Messages in this thread | | | Subject | Re: [PATCH 5/5] drm/vc4: fix fb references in async update | From | Helen Koike <> | Date | Mon, 11 Mar 2019 20:26:34 -0300 |
| |
On 3/11/19 6:56 AM, Boris Brezillon wrote: > +Eric (the VC4 driver maintainer) > > Hello Helen, > > On Mon, 4 Mar 2019 11:49:09 -0300 > Helen Koike <helen.koike@collabora.com> wrote: > >> Async update callbacks are expected to set the old_fb in the new_state >> so prepare/cleanup framebuffers are balanced. >> >> Calling drm_atomic_set_fb_for_plane() (which gets a reference of the new >> fb and put the old fb) is not required, as it's taken care by >> drm_mode_cursor_universal() when calling drm_atomic_helper_update_plane(). >> >> Cc: <stable@vger.kernel.org> # v4.19+: 25dc194b34dd: drm: Block fb changes for async plane updates >> Cc: <stable@vger.kernel.org> # v4.19+: 8105bbaf9afd: drm: don't block fb changes for async plane updates > > Hm, the commit hash you give here will change when applied to the DRM > tree. I think there's a standard way to express dependencies between > patches that needs to be applied to stable, but I'm not sure you need > to describe that since Greg picks patches in the order they appear in > Linus' tree and those patches will be applied in the right order.
right
> > Another option if you want to keep things simple is to squash all > changes in a single patch ;).
I was thinking about that, but some of them don't need to be picked by Greg (rockchip changes won't apply to stable for example), and I think it's easier to get tested-by/reviewed-by tags if I separate them and send them to the proper mailing list for the respective architecture.
> >> Fixes: 25dc194b34dd ("drm: Block fb changes for async plane updates") > > Nitpicking: this Fixes tag is a bit of lie since you're actually fixing a > mistake that was introduced when async update support was added to VC4. > Commit 25dc194b34dd only added a new constraint to fix the initial > problem. > > So I'd suggest: > > Fixes: 539c320bfa97 ("drm/vc4: update cursors asynchronously through atomic") > > BTW, the same applies to other patches in this series. > >> Suggested-by: Boris Brezillon <boris.brezillon@collabora.com> > > Other than that, > > Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Thanks! Helen
> > Regards, > > Boris > >> Signed-off-by: Helen Koike <helen.koike@collabora.com> >> >> --- >> Hello, >> >> As mentioned in the cover letter, >> I tested on the rockchip and on i915 using igt plane_cursor_legacy and >> kms_cursor_legacy and I didn't see any regressions. >> But I couldn't test on VC4. I have a Raspberry pi model B rev2, when >> FB_SIMPLE is running I can see output on the screen, but when vc4 is >> loaded my hdmi display is not detected anymore, I am still debugging >> this, probably some config in the firmware, but I would appreciate if >> anyone could help me testing it. >> >> Also the Cc statble commit hash dependency needs to be updated once the >> refered commit is merged. >> >> Thanks! >> Helen >> >> drivers/gpu/drm/vc4/vc4_plane.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c >> index 1babfeca0c92..1235e53b22a3 100644 >> --- a/drivers/gpu/drm/vc4/vc4_plane.c >> +++ b/drivers/gpu/drm/vc4/vc4_plane.c >> @@ -968,7 +968,7 @@ static void vc4_plane_atomic_async_update(struct drm_plane *plane, >> { >> struct vc4_plane_state *vc4_state, *new_vc4_state; >> >> - drm_atomic_set_fb_for_plane(plane->state, state->fb); >> + swap(plane->state->fb, state->fb); >> plane->state->crtc_x = state->crtc_x; >> plane->state->crtc_y = state->crtc_y; >> plane->state->crtc_w = state->crtc_w; > >
| |