| Message ID | 20160926130104.8977-2-marex@denx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am Montag, den 26.09.2016, 15:01 +0200 schrieb Marek Vasut: > Remove the common code from the driver and use the drm_fb_cma_setup_fence() > helper instead. Moveover, call the helper from prepare_fb() plane hook . > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Lucas Stach <l.stach@pengutronix.de> One nit inline, otherwise looks good and is: Tested-by: Lucas Stach <l.stach@pengutronix.de> > --- > drivers/gpu/drm/imx/imx-drm-core.c | 30 +----------------------------- > drivers/gpu/drm/imx/ipuv3-plane.c | 8 ++++++++ > 2 files changed, 9 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c > index 98df09c..c7faa1f 100644 > --- a/drivers/gpu/drm/imx/imx-drm-core.c > +++ b/drivers/gpu/drm/imx/imx-drm-core.c > @@ -18,7 +18,6 @@ > #include <linux/dma-buf.h> > #include <linux/module.h> > #include <linux/platform_device.h> > -#include <linux/reservation.h> > #include <drm/drmP.h> > #include <drm/drm_atomic.h> > #include <drm/drm_atomic_helper.h> > @@ -151,38 +150,11 @@ static int imx_drm_atomic_check(struct drm_device *dev, > return ret; > } > > -static int imx_drm_atomic_commit(struct drm_device *dev, > - struct drm_atomic_state *state, > - bool nonblock) > -{ > - struct drm_plane_state *plane_state; > - struct drm_plane *plane; > - struct dma_buf *dma_buf; > - int i; > - > - /* > - * If the plane fb has an dma-buf attached, fish out the exclusive > - * fence for the atomic helper to wait on. > - */ > - for_each_plane_in_state(state, plane, plane_state, i) { > - if ((plane->state->fb != plane_state->fb) && plane_state->fb) { > - dma_buf = drm_fb_cma_get_gem_obj(plane_state->fb, > - 0)->base.dma_buf; > - if (!dma_buf) > - continue; > - plane_state->fence = > - reservation_object_get_excl_rcu(dma_buf->resv); > - } > - } > - > - return drm_atomic_helper_commit(dev, state, nonblock); > -} > - > static const struct drm_mode_config_funcs imx_drm_mode_config_funcs = { > .fb_create = drm_fb_cma_create, > .output_poll_changed = imx_drm_output_poll_changed, > .atomic_check = imx_drm_atomic_check, > - .atomic_commit = imx_drm_atomic_commit, > + .atomic_commit = drm_atomic_helper_commit, > }; > > static void imx_drm_atomic_commit_tail(struct drm_atomic_state *state) > diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c > index ce22d0a..50615e3 100644 > --- a/drivers/gpu/drm/imx/ipuv3-plane.c > +++ b/drivers/gpu/drm/imx/ipuv3-plane.c > @@ -250,6 +250,13 @@ static const struct drm_plane_funcs ipu_plane_funcs = { > .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, > }; > > +static int ipu_plane_prepare_fb(struct drm_plane *plane, > + struct drm_plane_state *state) > +{ > + drm_fb_cma_setup_fence(plane, state); Blank line here to match the style in this driver. > + return 0; > +} > + > static int ipu_plane_atomic_check(struct drm_plane *plane, > struct drm_plane_state *state) > { > @@ -442,6 +449,7 @@ static void ipu_plane_atomic_update(struct drm_plane *plane, > } > > static const struct drm_plane_helper_funcs ipu_plane_helper_funcs = { > + .prepare_fb = ipu_plane_prepare_fb, > .atomic_check = ipu_plane_atomic_check, > .atomic_disable = ipu_plane_atomic_disable, > .atomic_update = ipu_plane_atomic_update,
On 09/28/2016 03:57 PM, Lucas Stach wrote: > Am Montag, den 26.09.2016, 15:01 +0200 schrieb Marek Vasut: >> Remove the common code from the driver and use the drm_fb_cma_setup_fence() >> helper instead. Moveover, call the helper from prepare_fb() plane hook . >> >> Signed-off-by: Marek Vasut <marex@denx.de> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >> Cc: Lucas Stach <l.stach@pengutronix.de> > > One nit inline, otherwise looks good and is: > > Tested-by: Lucas Stach <l.stach@pengutronix.de> > >> --- >> drivers/gpu/drm/imx/imx-drm-core.c | 30 +----------------------------- >> drivers/gpu/drm/imx/ipuv3-plane.c | 8 ++++++++ >> 2 files changed, 9 insertions(+), 29 deletions(-) >> >> diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c >> index 98df09c..c7faa1f 100644 >> --- a/drivers/gpu/drm/imx/imx-drm-core.c >> +++ b/drivers/gpu/drm/imx/imx-drm-core.c >> @@ -18,7 +18,6 @@ >> #include <linux/dma-buf.h> >> #include <linux/module.h> >> #include <linux/platform_device.h> >> -#include <linux/reservation.h> >> #include <drm/drmP.h> >> #include <drm/drm_atomic.h> >> #include <drm/drm_atomic_helper.h> >> @@ -151,38 +150,11 @@ static int imx_drm_atomic_check(struct drm_device *dev, >> return ret; >> } >> >> -static int imx_drm_atomic_commit(struct drm_device *dev, >> - struct drm_atomic_state *state, >> - bool nonblock) >> -{ >> - struct drm_plane_state *plane_state; >> - struct drm_plane *plane; >> - struct dma_buf *dma_buf; >> - int i; >> - >> - /* >> - * If the plane fb has an dma-buf attached, fish out the exclusive >> - * fence for the atomic helper to wait on. >> - */ >> - for_each_plane_in_state(state, plane, plane_state, i) { >> - if ((plane->state->fb != plane_state->fb) && plane_state->fb) { >> - dma_buf = drm_fb_cma_get_gem_obj(plane_state->fb, >> - 0)->base.dma_buf; >> - if (!dma_buf) >> - continue; >> - plane_state->fence = >> - reservation_object_get_excl_rcu(dma_buf->resv); >> - } >> - } >> - >> - return drm_atomic_helper_commit(dev, state, nonblock); >> -} >> - >> static const struct drm_mode_config_funcs imx_drm_mode_config_funcs = { >> .fb_create = drm_fb_cma_create, >> .output_poll_changed = imx_drm_output_poll_changed, >> .atomic_check = imx_drm_atomic_check, >> - .atomic_commit = imx_drm_atomic_commit, >> + .atomic_commit = drm_atomic_helper_commit, >> }; >> >> static void imx_drm_atomic_commit_tail(struct drm_atomic_state *state) >> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c >> index ce22d0a..50615e3 100644 >> --- a/drivers/gpu/drm/imx/ipuv3-plane.c >> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c >> @@ -250,6 +250,13 @@ static const struct drm_plane_funcs ipu_plane_funcs = { >> .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, >> }; >> >> +static int ipu_plane_prepare_fb(struct drm_plane *plane, >> + struct drm_plane_state *state) >> +{ >> + drm_fb_cma_setup_fence(plane, state); > > Blank line here to match the style in this driver. Done, thanks. >> + return 0; >> +} >> + >> static int ipu_plane_atomic_check(struct drm_plane *plane, >> struct drm_plane_state *state) >> { >> @@ -442,6 +449,7 @@ static void ipu_plane_atomic_update(struct drm_plane *plane, >> } >> >> static const struct drm_plane_helper_funcs ipu_plane_helper_funcs = { >> + .prepare_fb = ipu_plane_prepare_fb, >> .atomic_check = ipu_plane_atomic_check, >> .atomic_disable = ipu_plane_atomic_disable, >> .atomic_update = ipu_plane_atomic_update, > >
On Mon, Sep 26, 2016 at 03:01:04PM +0200, Marek Vasut wrote: > Remove the common code from the driver and use the drm_fb_cma_setup_fence() > helper instead. Moveover, call the helper from prepare_fb() plane hook . > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Lucas Stach <l.stach@pengutronix.de> > --- > drivers/gpu/drm/imx/imx-drm-core.c | 30 +----------------------------- > drivers/gpu/drm/imx/ipuv3-plane.c | 8 ++++++++ > 2 files changed, 9 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c > index 98df09c..c7faa1f 100644 > --- a/drivers/gpu/drm/imx/imx-drm-core.c > +++ b/drivers/gpu/drm/imx/imx-drm-core.c > @@ -18,7 +18,6 @@ > #include <linux/dma-buf.h> > #include <linux/module.h> > #include <linux/platform_device.h> > -#include <linux/reservation.h> > #include <drm/drmP.h> > #include <drm/drm_atomic.h> > #include <drm/drm_atomic_helper.h> > @@ -151,38 +150,11 @@ static int imx_drm_atomic_check(struct drm_device *dev, > return ret; > } > > -static int imx_drm_atomic_commit(struct drm_device *dev, > - struct drm_atomic_state *state, > - bool nonblock) > -{ > - struct drm_plane_state *plane_state; > - struct drm_plane *plane; > - struct dma_buf *dma_buf; > - int i; > - > - /* > - * If the plane fb has an dma-buf attached, fish out the exclusive > - * fence for the atomic helper to wait on. > - */ > - for_each_plane_in_state(state, plane, plane_state, i) { > - if ((plane->state->fb != plane_state->fb) && plane_state->fb) { > - dma_buf = drm_fb_cma_get_gem_obj(plane_state->fb, > - 0)->base.dma_buf; > - if (!dma_buf) > - continue; > - plane_state->fence = > - reservation_object_get_excl_rcu(dma_buf->resv); > - } > - } > - > - return drm_atomic_helper_commit(dev, state, nonblock); > -} > - > static const struct drm_mode_config_funcs imx_drm_mode_config_funcs = { > .fb_create = drm_fb_cma_create, > .output_poll_changed = imx_drm_output_poll_changed, > .atomic_check = imx_drm_atomic_check, > - .atomic_commit = imx_drm_atomic_commit, > + .atomic_commit = drm_atomic_helper_commit, > }; > > static void imx_drm_atomic_commit_tail(struct drm_atomic_state *state) > diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c > index ce22d0a..50615e3 100644 > --- a/drivers/gpu/drm/imx/ipuv3-plane.c > +++ b/drivers/gpu/drm/imx/ipuv3-plane.c > @@ -250,6 +250,13 @@ static const struct drm_plane_funcs ipu_plane_funcs = { > .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, > }; > > +static int ipu_plane_prepare_fb(struct drm_plane *plane, > + struct drm_plane_state *state) > +{ > + drm_fb_cma_setup_fence(plane, state); > + return 0; > +} > + > static int ipu_plane_atomic_check(struct drm_plane *plane, > struct drm_plane_state *state) > { > @@ -442,6 +449,7 @@ static void ipu_plane_atomic_update(struct drm_plane *plane, > } > > static const struct drm_plane_helper_funcs ipu_plane_helper_funcs = { > + .prepare_fb = ipu_plane_prepare_fb, See my coment about the function naming, but imo drm_fb_cma_prepare_fb should have a function type matching prepare_fb. Otherwise every driver needs a dummy wrapper function like ipu_plane_prepare_fb, which imo would be pointless. -Daniel > .atomic_check = ipu_plane_atomic_check, > .atomic_disable = ipu_plane_atomic_disable, > .atomic_update = ipu_plane_atomic_update, > -- > 2.9.3 >
On 09/29/2016 11:37 AM, Daniel Vetter wrote: > On Mon, Sep 26, 2016 at 03:01:04PM +0200, Marek Vasut wrote: >> Remove the common code from the driver and use the drm_fb_cma_setup_fence() >> helper instead. Moveover, call the helper from prepare_fb() plane hook . >> >> Signed-off-by: Marek Vasut <marex@denx.de> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >> Cc: Lucas Stach <l.stach@pengutronix.de> >> --- >> drivers/gpu/drm/imx/imx-drm-core.c | 30 +----------------------------- >> drivers/gpu/drm/imx/ipuv3-plane.c | 8 ++++++++ >> 2 files changed, 9 insertions(+), 29 deletions(-) >> >> diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c >> index 98df09c..c7faa1f 100644 >> --- a/drivers/gpu/drm/imx/imx-drm-core.c >> +++ b/drivers/gpu/drm/imx/imx-drm-core.c >> @@ -18,7 +18,6 @@ >> #include <linux/dma-buf.h> >> #include <linux/module.h> >> #include <linux/platform_device.h> >> -#include <linux/reservation.h> >> #include <drm/drmP.h> >> #include <drm/drm_atomic.h> >> #include <drm/drm_atomic_helper.h> >> @@ -151,38 +150,11 @@ static int imx_drm_atomic_check(struct drm_device *dev, >> return ret; >> } >> >> -static int imx_drm_atomic_commit(struct drm_device *dev, >> - struct drm_atomic_state *state, >> - bool nonblock) >> -{ >> - struct drm_plane_state *plane_state; >> - struct drm_plane *plane; >> - struct dma_buf *dma_buf; >> - int i; >> - >> - /* >> - * If the plane fb has an dma-buf attached, fish out the exclusive >> - * fence for the atomic helper to wait on. >> - */ >> - for_each_plane_in_state(state, plane, plane_state, i) { >> - if ((plane->state->fb != plane_state->fb) && plane_state->fb) { >> - dma_buf = drm_fb_cma_get_gem_obj(plane_state->fb, >> - 0)->base.dma_buf; >> - if (!dma_buf) >> - continue; >> - plane_state->fence = >> - reservation_object_get_excl_rcu(dma_buf->resv); >> - } >> - } >> - >> - return drm_atomic_helper_commit(dev, state, nonblock); >> -} >> - >> static const struct drm_mode_config_funcs imx_drm_mode_config_funcs = { >> .fb_create = drm_fb_cma_create, >> .output_poll_changed = imx_drm_output_poll_changed, >> .atomic_check = imx_drm_atomic_check, >> - .atomic_commit = imx_drm_atomic_commit, >> + .atomic_commit = drm_atomic_helper_commit, >> }; >> >> static void imx_drm_atomic_commit_tail(struct drm_atomic_state *state) >> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c >> index ce22d0a..50615e3 100644 >> --- a/drivers/gpu/drm/imx/ipuv3-plane.c >> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c >> @@ -250,6 +250,13 @@ static const struct drm_plane_funcs ipu_plane_funcs = { >> .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, >> }; >> >> +static int ipu_plane_prepare_fb(struct drm_plane *plane, >> + struct drm_plane_state *state) >> +{ >> + drm_fb_cma_setup_fence(plane, state); >> + return 0; >> +} >> + >> static int ipu_plane_atomic_check(struct drm_plane *plane, >> struct drm_plane_state *state) >> { >> @@ -442,6 +449,7 @@ static void ipu_plane_atomic_update(struct drm_plane *plane, >> } >> >> static const struct drm_plane_helper_funcs ipu_plane_helper_funcs = { >> + .prepare_fb = ipu_plane_prepare_fb, > > See my coment about the function naming, but imo drm_fb_cma_prepare_fb You mean drm_fb_cma_setup_fence() , so it can be used directly in .prepare_fb ? > should have a function type matching prepare_fb. Otherwise every driver > needs a dummy wrapper function like ipu_plane_prepare_fb, which imo would > be pointless. Indeed. > -Daniel > >> .atomic_check = ipu_plane_atomic_check, >> .atomic_disable = ipu_plane_atomic_disable, >> .atomic_update = ipu_plane_atomic_update, >> -- >> 2.9.3 >> >
diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c index 98df09c..c7faa1f 100644 --- a/drivers/gpu/drm/imx/imx-drm-core.c +++ b/drivers/gpu/drm/imx/imx-drm-core.c @@ -18,7 +18,6 @@ #include <linux/dma-buf.h> #include <linux/module.h> #include <linux/platform_device.h> -#include <linux/reservation.h> #include <drm/drmP.h> #include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> @@ -151,38 +150,11 @@ static int imx_drm_atomic_check(struct drm_device *dev, return ret; } -static int imx_drm_atomic_commit(struct drm_device *dev, - struct drm_atomic_state *state, - bool nonblock) -{ - struct drm_plane_state *plane_state; - struct drm_plane *plane; - struct dma_buf *dma_buf; - int i; - - /* - * If the plane fb has an dma-buf attached, fish out the exclusive - * fence for the atomic helper to wait on. - */ - for_each_plane_in_state(state, plane, plane_state, i) { - if ((plane->state->fb != plane_state->fb) && plane_state->fb) { - dma_buf = drm_fb_cma_get_gem_obj(plane_state->fb, - 0)->base.dma_buf; - if (!dma_buf) - continue; - plane_state->fence = - reservation_object_get_excl_rcu(dma_buf->resv); - } - } - - return drm_atomic_helper_commit(dev, state, nonblock); -} - static const struct drm_mode_config_funcs imx_drm_mode_config_funcs = { .fb_create = drm_fb_cma_create, .output_poll_changed = imx_drm_output_poll_changed, .atomic_check = imx_drm_atomic_check, - .atomic_commit = imx_drm_atomic_commit, + .atomic_commit = drm_atomic_helper_commit, }; static void imx_drm_atomic_commit_tail(struct drm_atomic_state *state) diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c index ce22d0a..50615e3 100644 --- a/drivers/gpu/drm/imx/ipuv3-plane.c +++ b/drivers/gpu/drm/imx/ipuv3-plane.c @@ -250,6 +250,13 @@ static const struct drm_plane_funcs ipu_plane_funcs = { .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, }; +static int ipu_plane_prepare_fb(struct drm_plane *plane, + struct drm_plane_state *state) +{ + drm_fb_cma_setup_fence(plane, state); + return 0; +} + static int ipu_plane_atomic_check(struct drm_plane *plane, struct drm_plane_state *state) { @@ -442,6 +449,7 @@ static void ipu_plane_atomic_update(struct drm_plane *plane, } static const struct drm_plane_helper_funcs ipu_plane_helper_funcs = { + .prepare_fb = ipu_plane_prepare_fb, .atomic_check = ipu_plane_atomic_check, .atomic_disable = ipu_plane_atomic_disable, .atomic_update = ipu_plane_atomic_update,
Remove the common code from the driver and use the drm_fb_cma_setup_fence() helper instead. Moveover, call the helper from prepare_fb() plane hook . Signed-off-by: Marek Vasut <marex@denx.de> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Lucas Stach <l.stach@pengutronix.de> --- drivers/gpu/drm/imx/imx-drm-core.c | 30 +----------------------------- drivers/gpu/drm/imx/ipuv3-plane.c | 8 ++++++++ 2 files changed, 9 insertions(+), 29 deletions(-)