* [PATCH 1/2] drm/fb_cma_helper: Add drm_fb_cma_setup_fence() helper @ 2016-09-26 13:01 Marek Vasut 2016-09-26 13:01 ` [PATCH 2/2] drm/imx: Switch to " Marek Vasut 2016-09-28 13:55 ` [PATCH 1/2] drm/fb_cma_helper: Add " Lucas Stach 0 siblings, 2 replies; 14+ messages in thread From: Marek Vasut @ 2016-09-26 13:01 UTC (permalink / raw) To: dri-devel; +Cc: Marek Vasut, Daniel Vetter Add new drm_fb_cma_setup_fence() helper function extracted from the imx-drm driver. This function checks if the plane has DMABUF attached to it and if so, sets up the fence on which the atomic helper can wait. 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/drm_fb_cma_helper.c | 26 ++++++++++++++++++++++++++ include/drm/drm_fb_cma_helper.h | 3 +++ 2 files changed, 29 insertions(+) diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c index 1fd6eac..2441707 100644 --- a/drivers/gpu/drm/drm_fb_cma_helper.c +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -23,8 +23,10 @@ #include <drm/drm_crtc_helper.h> #include <drm/drm_gem_cma_helper.h> #include <drm/drm_fb_cma_helper.h> +#include <linux/dma-buf.h> #include <linux/dma-mapping.h> #include <linux/module.h> +#include <linux/reservation.h> #define DEFAULT_FBDEFIO_DELAY_MS 50 @@ -265,6 +267,30 @@ struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb, } EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_obj); +/** + * drm_fb_cma_setup_fence() - Set up the fence for atomic helper to wait on + * @plane: Which plane + * @state: Plane state to check + * + * If the plane fb has an dma-buf attached, fish out the exclusive + * fence for the atomic helper to wait on. + */ +void drm_fb_cma_setup_fence(struct drm_plane *plane, + struct drm_plane_state *state) +{ + struct dma_buf *dma_buf; + + if ((plane->state->fb == state->fb) || !state->fb) + return; + + dma_buf = drm_fb_cma_get_gem_obj(state->fb, 0)->base.dma_buf; + if (!dma_buf) + return; + + state->fence = reservation_object_get_excl_rcu(dma_buf->resv); +} +EXPORT_SYMBOL_GPL(drm_fb_cma_setup_fence); + #ifdef CONFIG_DEBUG_FS static void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m) { diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h index f313211..fc122d3 100644 --- a/include/drm/drm_fb_cma_helper.h +++ b/include/drm/drm_fb_cma_helper.h @@ -41,6 +41,9 @@ struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev, struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb, unsigned int plane); +void drm_fb_cma_setup_fence(struct drm_plane *plane, + struct drm_plane_state *state); + #ifdef CONFIG_DEBUG_FS struct seq_file; -- 2.9.3 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] drm/imx: Switch to drm_fb_cma_setup_fence() helper 2016-09-26 13:01 [PATCH 1/2] drm/fb_cma_helper: Add drm_fb_cma_setup_fence() helper Marek Vasut @ 2016-09-26 13:01 ` Marek Vasut 2016-09-28 13:57 ` Lucas Stach 2016-09-29 9:37 ` Daniel Vetter 2016-09-28 13:55 ` [PATCH 1/2] drm/fb_cma_helper: Add " Lucas Stach 1 sibling, 2 replies; 14+ messages in thread From: Marek Vasut @ 2016-09-26 13:01 UTC (permalink / raw) To: dri-devel; +Cc: Marek Vasut, Daniel Vetter 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, .atomic_check = ipu_plane_atomic_check, .atomic_disable = ipu_plane_atomic_disable, .atomic_update = ipu_plane_atomic_update, -- 2.9.3 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] drm/imx: Switch to drm_fb_cma_setup_fence() helper 2016-09-26 13:01 ` [PATCH 2/2] drm/imx: Switch to " Marek Vasut @ 2016-09-28 13:57 ` Lucas Stach 2016-09-28 20:09 ` Marek Vasut 2016-09-29 9:37 ` Daniel Vetter 1 sibling, 1 reply; 14+ messages in thread From: Lucas Stach @ 2016-09-28 13:57 UTC (permalink / raw) To: Marek Vasut; +Cc: Daniel Vetter, dri-devel 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, _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] drm/imx: Switch to drm_fb_cma_setup_fence() helper 2016-09-28 13:57 ` Lucas Stach @ 2016-09-28 20:09 ` Marek Vasut 0 siblings, 0 replies; 14+ messages in thread From: Marek Vasut @ 2016-09-28 20:09 UTC (permalink / raw) To: Lucas Stach; +Cc: Daniel Vetter, dri-devel 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, > > -- Best regards, Marek Vasut _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] drm/imx: Switch to drm_fb_cma_setup_fence() helper 2016-09-26 13:01 ` [PATCH 2/2] drm/imx: Switch to " Marek Vasut 2016-09-28 13:57 ` Lucas Stach @ 2016-09-29 9:37 ` Daniel Vetter 2016-09-29 21:40 ` Marek Vasut 1 sibling, 1 reply; 14+ messages in thread From: Daniel Vetter @ 2016-09-29 9:37 UTC (permalink / raw) To: Marek Vasut; +Cc: Daniel Vetter, dri-devel 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 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] drm/imx: Switch to drm_fb_cma_setup_fence() helper 2016-09-29 9:37 ` Daniel Vetter @ 2016-09-29 21:40 ` Marek Vasut 0 siblings, 0 replies; 14+ messages in thread From: Marek Vasut @ 2016-09-29 21:40 UTC (permalink / raw) To: Daniel Vetter; +Cc: Daniel Vetter, dri-devel 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 >> > -- Best regards, Marek Vasut _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] drm/fb_cma_helper: Add drm_fb_cma_setup_fence() helper 2016-09-26 13:01 [PATCH 1/2] drm/fb_cma_helper: Add drm_fb_cma_setup_fence() helper Marek Vasut 2016-09-26 13:01 ` [PATCH 2/2] drm/imx: Switch to " Marek Vasut @ 2016-09-28 13:55 ` Lucas Stach 2016-09-28 20:08 ` Marek Vasut 1 sibling, 1 reply; 14+ messages in thread From: Lucas Stach @ 2016-09-28 13:55 UTC (permalink / raw) To: Marek Vasut; +Cc: Daniel Vetter, dri-devel Hi Marek, Am Montag, den 26.09.2016, 15:01 +0200 schrieb Marek Vasut: > Add new drm_fb_cma_setup_fence() helper function extracted from the > imx-drm driver. This function checks if the plane has DMABUF attached > to it and if so, sets up the fence on which the atomic helper can wait. > > 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/drm_fb_cma_helper.c | 26 ++++++++++++++++++++++++++ > include/drm/drm_fb_cma_helper.h | 3 +++ > 2 files changed, 29 insertions(+) > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c > index 1fd6eac..2441707 100644 > --- a/drivers/gpu/drm/drm_fb_cma_helper.c > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c > @@ -23,8 +23,10 @@ > #include <drm/drm_crtc_helper.h> > #include <drm/drm_gem_cma_helper.h> > #include <drm/drm_fb_cma_helper.h> > +#include <linux/dma-buf.h> > #include <linux/dma-mapping.h> > #include <linux/module.h> > +#include <linux/reservation.h> > > #define DEFAULT_FBDEFIO_DELAY_MS 50 > > @@ -265,6 +267,30 @@ struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb, > } > EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_obj); > > +/** > + * drm_fb_cma_setup_fence() - Set up the fence for atomic helper to wait on I don't really like the naming of the helper. It's not setting up any fence, it's either extracting it from the plane and/or attaching it to the planestate, so I would have expected the name to include extract or attach. > + * @plane: Which plane > + * @state: Plane state to check s/check/attach fence to > + * > + * If the plane fb has an dma-buf attached, fish out the exclusive > + * fence for the atomic helper to wait on. > + */ > +void drm_fb_cma_setup_fence(struct drm_plane *plane, > + struct drm_plane_state *state) > +{ > + struct dma_buf *dma_buf; > + > + if ((plane->state->fb == state->fb) || !state->fb) > + return; > + > + dma_buf = drm_fb_cma_get_gem_obj(state->fb, 0)->base.dma_buf; > + if (!dma_buf) > + return; > + > + state->fence = reservation_object_get_excl_rcu(dma_buf->resv); > +} > +EXPORT_SYMBOL_GPL(drm_fb_cma_setup_fence); > + > #ifdef CONFIG_DEBUG_FS > static void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m) > { > diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h > index f313211..fc122d3 100644 > --- a/include/drm/drm_fb_cma_helper.h > +++ b/include/drm/drm_fb_cma_helper.h > @@ -41,6 +41,9 @@ struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev, > struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb, > unsigned int plane); > > +void drm_fb_cma_setup_fence(struct drm_plane *plane, > + struct drm_plane_state *state); > + > #ifdef CONFIG_DEBUG_FS > struct seq_file; > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] drm/fb_cma_helper: Add drm_fb_cma_setup_fence() helper 2016-09-28 13:55 ` [PATCH 1/2] drm/fb_cma_helper: Add " Lucas Stach @ 2016-09-28 20:08 ` Marek Vasut 2016-09-29 9:36 ` Daniel Vetter 0 siblings, 1 reply; 14+ messages in thread From: Marek Vasut @ 2016-09-28 20:08 UTC (permalink / raw) To: Lucas Stach; +Cc: Daniel Vetter, dri-devel On 09/28/2016 03:55 PM, Lucas Stach wrote: > Hi Marek, > > Am Montag, den 26.09.2016, 15:01 +0200 schrieb Marek Vasut: >> Add new drm_fb_cma_setup_fence() helper function extracted from the >> imx-drm driver. This function checks if the plane has DMABUF attached >> to it and if so, sets up the fence on which the atomic helper can wait. >> >> 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/drm_fb_cma_helper.c | 26 ++++++++++++++++++++++++++ >> include/drm/drm_fb_cma_helper.h | 3 +++ >> 2 files changed, 29 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c >> index 1fd6eac..2441707 100644 >> --- a/drivers/gpu/drm/drm_fb_cma_helper.c >> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c >> @@ -23,8 +23,10 @@ >> #include <drm/drm_crtc_helper.h> >> #include <drm/drm_gem_cma_helper.h> >> #include <drm/drm_fb_cma_helper.h> >> +#include <linux/dma-buf.h> >> #include <linux/dma-mapping.h> >> #include <linux/module.h> >> +#include <linux/reservation.h> >> >> #define DEFAULT_FBDEFIO_DELAY_MS 50 >> >> @@ -265,6 +267,30 @@ struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb, >> } >> EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_obj); >> >> +/** >> + * drm_fb_cma_setup_fence() - Set up the fence for atomic helper to wait on > > I don't really like the naming of the helper. It's not setting up any > fence, it's either extracting it from the plane and/or attaching it to > the planestate, so I would have expected the name to include extract or > attach. > >> + * @plane: Which plane >> + * @state: Plane state to check > > s/check/attach fence to Both fixed in V2, thanks. >> + * >> + * If the plane fb has an dma-buf attached, fish out the exclusive >> + * fence for the atomic helper to wait on. >> + */ >> +void drm_fb_cma_setup_fence(struct drm_plane *plane, >> + struct drm_plane_state *state) >> +{ >> + struct dma_buf *dma_buf; >> + >> + if ((plane->state->fb == state->fb) || !state->fb) >> + return; >> + >> + dma_buf = drm_fb_cma_get_gem_obj(state->fb, 0)->base.dma_buf; >> + if (!dma_buf) >> + return; >> + >> + state->fence = reservation_object_get_excl_rcu(dma_buf->resv); >> +} >> +EXPORT_SYMBOL_GPL(drm_fb_cma_setup_fence); >> + >> #ifdef CONFIG_DEBUG_FS >> static void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m) >> { >> diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h >> index f313211..fc122d3 100644 >> --- a/include/drm/drm_fb_cma_helper.h >> +++ b/include/drm/drm_fb_cma_helper.h >> @@ -41,6 +41,9 @@ struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev, >> struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb, >> unsigned int plane); >> >> +void drm_fb_cma_setup_fence(struct drm_plane *plane, >> + struct drm_plane_state *state); >> + >> #ifdef CONFIG_DEBUG_FS >> struct seq_file; >> > > -- Best regards, Marek Vasut _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] drm/fb_cma_helper: Add drm_fb_cma_setup_fence() helper 2016-09-28 20:08 ` Marek Vasut @ 2016-09-29 9:36 ` Daniel Vetter 2016-09-29 21:44 ` Marek Vasut 0 siblings, 1 reply; 14+ messages in thread From: Daniel Vetter @ 2016-09-29 9:36 UTC (permalink / raw) To: Marek Vasut; +Cc: Daniel Vetter, dri-devel On Wed, Sep 28, 2016 at 10:08:21PM +0200, Marek Vasut wrote: > On 09/28/2016 03:55 PM, Lucas Stach wrote: > > Hi Marek, > > > > Am Montag, den 26.09.2016, 15:01 +0200 schrieb Marek Vasut: > >> Add new drm_fb_cma_setup_fence() helper function extracted from the > >> imx-drm driver. This function checks if the plane has DMABUF attached > >> to it and if so, sets up the fence on which the atomic helper can wait. > >> > >> 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/drm_fb_cma_helper.c | 26 ++++++++++++++++++++++++++ > >> include/drm/drm_fb_cma_helper.h | 3 +++ > >> 2 files changed, 29 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c > >> index 1fd6eac..2441707 100644 > >> --- a/drivers/gpu/drm/drm_fb_cma_helper.c > >> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c > >> @@ -23,8 +23,10 @@ > >> #include <drm/drm_crtc_helper.h> > >> #include <drm/drm_gem_cma_helper.h> > >> #include <drm/drm_fb_cma_helper.h> > >> +#include <linux/dma-buf.h> > >> #include <linux/dma-mapping.h> > >> #include <linux/module.h> > >> +#include <linux/reservation.h> > >> > >> #define DEFAULT_FBDEFIO_DELAY_MS 50 > >> > >> @@ -265,6 +267,30 @@ struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb, > >> } > >> EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_obj); > >> > >> +/** > >> + * drm_fb_cma_setup_fence() - Set up the fence for atomic helper to wait on > > > > I don't really like the naming of the helper. It's not setting up any > > fence, it's either extracting it from the plane and/or attaching it to > > the planestate, so I would have expected the name to include extract or > > attach. Imo for helpers that should be put into a specific vhook like this here, it'd go with that vhooks name. So here drm_fb_cma_prepare_fb. And then in the kerneldoc also mention where it should be put (both for drivers using atomic helpers, and those using simple pipe helpers), e.g. "This should be put into the prepare_fb hook of struct &drm_plane_helper_funcs." Please run $ make htmldocs to make sure the docs look pretty and the generated links are correct. -Daniel > > > >> + * @plane: Which plane > >> + * @state: Plane state to check > > > > s/check/attach fence to > > Both fixed in V2, thanks. > > >> + * > >> + * If the plane fb has an dma-buf attached, fish out the exclusive > >> + * fence for the atomic helper to wait on. > >> + */ > >> +void drm_fb_cma_setup_fence(struct drm_plane *plane, > >> + struct drm_plane_state *state) > >> +{ > >> + struct dma_buf *dma_buf; > >> + > >> + if ((plane->state->fb == state->fb) || !state->fb) > >> + return; > >> + > >> + dma_buf = drm_fb_cma_get_gem_obj(state->fb, 0)->base.dma_buf; > >> + if (!dma_buf) > >> + return; > >> + > >> + state->fence = reservation_object_get_excl_rcu(dma_buf->resv); > >> +} > >> +EXPORT_SYMBOL_GPL(drm_fb_cma_setup_fence); > >> + > >> #ifdef CONFIG_DEBUG_FS > >> static void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m) > >> { > >> diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h > >> index f313211..fc122d3 100644 > >> --- a/include/drm/drm_fb_cma_helper.h > >> +++ b/include/drm/drm_fb_cma_helper.h > >> @@ -41,6 +41,9 @@ struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev, > >> struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb, > >> unsigned int plane); > >> > >> +void drm_fb_cma_setup_fence(struct drm_plane *plane, > >> + struct drm_plane_state *state); > >> + > >> #ifdef CONFIG_DEBUG_FS > >> struct seq_file; > >> > > > > > > > -- > Best regards, > Marek Vasut -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] drm/fb_cma_helper: Add drm_fb_cma_setup_fence() helper 2016-09-29 9:36 ` Daniel Vetter @ 2016-09-29 21:44 ` Marek Vasut 2016-09-30 9:53 ` Daniel Vetter 0 siblings, 1 reply; 14+ messages in thread From: Marek Vasut @ 2016-09-29 21:44 UTC (permalink / raw) To: Daniel Vetter; +Cc: Daniel Vetter, dri-devel On 09/29/2016 11:36 AM, Daniel Vetter wrote: > On Wed, Sep 28, 2016 at 10:08:21PM +0200, Marek Vasut wrote: >> On 09/28/2016 03:55 PM, Lucas Stach wrote: >>> Hi Marek, >>> >>> Am Montag, den 26.09.2016, 15:01 +0200 schrieb Marek Vasut: >>>> Add new drm_fb_cma_setup_fence() helper function extracted from the >>>> imx-drm driver. This function checks if the plane has DMABUF attached >>>> to it and if so, sets up the fence on which the atomic helper can wait. >>>> >>>> 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/drm_fb_cma_helper.c | 26 ++++++++++++++++++++++++++ >>>> include/drm/drm_fb_cma_helper.h | 3 +++ >>>> 2 files changed, 29 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c >>>> index 1fd6eac..2441707 100644 >>>> --- a/drivers/gpu/drm/drm_fb_cma_helper.c >>>> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c >>>> @@ -23,8 +23,10 @@ >>>> #include <drm/drm_crtc_helper.h> >>>> #include <drm/drm_gem_cma_helper.h> >>>> #include <drm/drm_fb_cma_helper.h> >>>> +#include <linux/dma-buf.h> >>>> #include <linux/dma-mapping.h> >>>> #include <linux/module.h> >>>> +#include <linux/reservation.h> >>>> >>>> #define DEFAULT_FBDEFIO_DELAY_MS 50 >>>> >>>> @@ -265,6 +267,30 @@ struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb, >>>> } >>>> EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_obj); >>>> >>>> +/** >>>> + * drm_fb_cma_setup_fence() - Set up the fence for atomic helper to wait on >>> >>> I don't really like the naming of the helper. It's not setting up any >>> fence, it's either extracting it from the plane and/or attaching it to >>> the planestate, so I would have expected the name to include extract or >>> attach. > > Imo for helpers that should be put into a specific vhook like this here, > it'd go with that vhooks name. So here drm_fb_cma_prepare_fb. I have the following right now, I think that's more descriptive as this function is not preparing the FB in any way. /** * drm_fb_cma_extract_and_attach_fence() - Extract fence from plane and attach to planestate * @plane: Which plane * @state: Plane state attach fence to * * If the plane fb has an dma-buf attached, fish out the exclusive * fence and attach it to plane state for the atomic helper to wait * on. */ > And then in the kerneldoc also mention where it should be put (both for > drivers using atomic helpers, and those using simple pipe helpers), e.g. > "This should be put into the prepare_fb hook of struct > &drm_plane_helper_funcs." > > Please run $ make htmldocs to make sure the docs look pretty and the > generated links are correct. > -Daniel > >>> >>>> + * @plane: Which plane >>>> + * @state: Plane state to check >>> >>> s/check/attach fence to >> >> Both fixed in V2, thanks. >> >>>> + * >>>> + * If the plane fb has an dma-buf attached, fish out the exclusive >>>> + * fence for the atomic helper to wait on. >>>> + */ >>>> +void drm_fb_cma_setup_fence(struct drm_plane *plane, >>>> + struct drm_plane_state *state) >>>> +{ >>>> + struct dma_buf *dma_buf; >>>> + >>>> + if ((plane->state->fb == state->fb) || !state->fb) >>>> + return; >>>> + >>>> + dma_buf = drm_fb_cma_get_gem_obj(state->fb, 0)->base.dma_buf; >>>> + if (!dma_buf) >>>> + return; >>>> + >>>> + state->fence = reservation_object_get_excl_rcu(dma_buf->resv); >>>> +} >>>> +EXPORT_SYMBOL_GPL(drm_fb_cma_setup_fence); >>>> + >>>> #ifdef CONFIG_DEBUG_FS >>>> static void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m) >>>> { >>>> diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h >>>> index f313211..fc122d3 100644 >>>> --- a/include/drm/drm_fb_cma_helper.h >>>> +++ b/include/drm/drm_fb_cma_helper.h >>>> @@ -41,6 +41,9 @@ struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev, >>>> struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb, >>>> unsigned int plane); >>>> >>>> +void drm_fb_cma_setup_fence(struct drm_plane *plane, >>>> + struct drm_plane_state *state); >>>> + >>>> #ifdef CONFIG_DEBUG_FS >>>> struct seq_file; >>>> >>> >>> >> >> >> -- >> Best regards, >> Marek Vasut > -- Best regards, Marek Vasut _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] drm/fb_cma_helper: Add drm_fb_cma_setup_fence() helper 2016-09-29 21:44 ` Marek Vasut @ 2016-09-30 9:53 ` Daniel Vetter 2016-10-02 17:19 ` Marek Vasut 0 siblings, 1 reply; 14+ messages in thread From: Daniel Vetter @ 2016-09-30 9:53 UTC (permalink / raw) To: Marek Vasut; +Cc: dri-devel On Thu, Sep 29, 2016 at 11:44 PM, Marek Vasut <marex@denx.de> wrote: > I have the following right now, I think that's more descriptive as this > function is not preparing the FB in any way. > > /** > * drm_fb_cma_extract_and_attach_fence() - Extract fence from plane and > attach to planestate > * @plane: Which plane > * @state: Plane state attach fence to > * > * If the plane fb has an dma-buf attached, fish out the exclusive > * fence and attach it to plane state for the atomic helper to wait > * on. > */ That choice of color is ok with me too, but then you need to have a pile of text to explain where it should be used (i.e. directly as the prepare_fb hook). And it is a bit inconsistent with all the other helpers that can be put into hooks directly. Also, extract_and_attach_fence _is_ officially part of what a driver should do in their ->prepare_fb hook. Hence I'm still leaning towards that (and it's shorter!). And with cma there's nothing else to do (dma memory is always pinned, which is the other thing prepare_fb should do), so really it's not even a lie: Your function fully prapares a cma fb for display as expected by th atomic helpers. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] drm/fb_cma_helper: Add drm_fb_cma_setup_fence() helper 2016-09-30 9:53 ` Daniel Vetter @ 2016-10-02 17:19 ` Marek Vasut 2016-10-03 13:54 ` Daniel Vetter 0 siblings, 1 reply; 14+ messages in thread From: Marek Vasut @ 2016-10-02 17:19 UTC (permalink / raw) To: Daniel Vetter; +Cc: dri-devel On 09/30/2016 11:53 AM, Daniel Vetter wrote: > On Thu, Sep 29, 2016 at 11:44 PM, Marek Vasut <marex@denx.de> wrote: >> I have the following right now, I think that's more descriptive as this >> function is not preparing the FB in any way. >> >> /** >> * drm_fb_cma_extract_and_attach_fence() - Extract fence from plane and >> attach to planestate >> * @plane: Which plane >> * @state: Plane state attach fence to >> * >> * If the plane fb has an dma-buf attached, fish out the exclusive >> * fence and attach it to plane state for the atomic helper to wait >> * on. >> */ > > That choice of color is ok with me too, but then you need to have a > pile of text to explain where it should be used (i.e. directly as the > prepare_fb hook). There can be more stuff in the prepare_fb hook though. > And it is a bit inconsistent with all the other > helpers that can be put into hooks directly. Also, > extract_and_attach_fence _is_ officially part of what a driver should > do in their ->prepare_fb hook. Hence I'm still leaning towards that > (and it's shorter!). And with cma there's nothing else to do (dma > memory is always pinned, which is the other thing prepare_fb should > do), so really it's not even a lie: Your function fully prapares a cma > fb for display as expected by th atomic helpers. OK, I have no good argument not to go for this one. Unless someone has, I will redo it. > -Daniel > -- Best regards, Marek Vasut _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] drm/fb_cma_helper: Add drm_fb_cma_setup_fence() helper 2016-10-02 17:19 ` Marek Vasut @ 2016-10-03 13:54 ` Daniel Vetter 2016-10-05 14:50 ` Marek Vasut 0 siblings, 1 reply; 14+ messages in thread From: Daniel Vetter @ 2016-10-03 13:54 UTC (permalink / raw) To: Marek Vasut; +Cc: dri-devel On Sun, Oct 2, 2016 at 7:19 PM, Marek Vasut <marex@denx.de> wrote: >>> /** >>> * drm_fb_cma_extract_and_attach_fence() - Extract fence from plane and >>> attach to planestate >>> * @plane: Which plane >>> * @state: Plane state attach fence to >>> * >>> * If the plane fb has an dma-buf attached, fish out the exclusive >>> * fence and attach it to plane state for the atomic helper to wait >>> * on. >>> */ >> >> That choice of color is ok with me too, but then you need to have a >> pile of text to explain where it should be used (i.e. directly as the >> prepare_fb hook). > > There can be more stuff in the prepare_fb hook though. There's 3 things prepare/cleanup_fb should do: - pin/upin the backing storage. CMA memory is always pinned, so nothing to do. - setup/tear down iommu mappings: Already done when allocating CMA memory, again nothing to do. - grab fences in prepare. So for plain cma drivers this hook is indeed the complete implementation they need for prepare_fb. I guess you could mention in the kernel-doc that cma based drivers don't need a cleanup_fb, hence why there is none. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] drm/fb_cma_helper: Add drm_fb_cma_setup_fence() helper 2016-10-03 13:54 ` Daniel Vetter @ 2016-10-05 14:50 ` Marek Vasut 0 siblings, 0 replies; 14+ messages in thread From: Marek Vasut @ 2016-10-05 14:50 UTC (permalink / raw) To: Daniel Vetter; +Cc: dri-devel On 10/03/2016 03:54 PM, Daniel Vetter wrote: > On Sun, Oct 2, 2016 at 7:19 PM, Marek Vasut <marex@denx.de> wrote: >>>> /** >>>> * drm_fb_cma_extract_and_attach_fence() - Extract fence from plane and >>>> attach to planestate >>>> * @plane: Which plane >>>> * @state: Plane state attach fence to >>>> * >>>> * If the plane fb has an dma-buf attached, fish out the exclusive >>>> * fence and attach it to plane state for the atomic helper to wait >>>> * on. >>>> */ >>> >>> That choice of color is ok with me too, but then you need to have a >>> pile of text to explain where it should be used (i.e. directly as the >>> prepare_fb hook). >> >> There can be more stuff in the prepare_fb hook though. > > There's 3 things prepare/cleanup_fb should do: > - pin/upin the backing storage. CMA memory is always pinned, so nothing to do. > - setup/tear down iommu mappings: Already done when allocating CMA > memory, again nothing to do. > - grab fences in prepare. > > So for plain cma drivers this hook is indeed the complete > implementation they need for prepare_fb. I guess you could mention in > the kernel-doc that cma based drivers don't need a cleanup_fb, hence > why there is none. OK, understood. I'll roll V3 shortly. -- Best regards, Marek Vasut _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-10-05 15:15 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-09-26 13:01 [PATCH 1/2] drm/fb_cma_helper: Add drm_fb_cma_setup_fence() helper Marek Vasut 2016-09-26 13:01 ` [PATCH 2/2] drm/imx: Switch to " Marek Vasut 2016-09-28 13:57 ` Lucas Stach 2016-09-28 20:09 ` Marek Vasut 2016-09-29 9:37 ` Daniel Vetter 2016-09-29 21:40 ` Marek Vasut 2016-09-28 13:55 ` [PATCH 1/2] drm/fb_cma_helper: Add " Lucas Stach 2016-09-28 20:08 ` Marek Vasut 2016-09-29 9:36 ` Daniel Vetter 2016-09-29 21:44 ` Marek Vasut 2016-09-30 9:53 ` Daniel Vetter 2016-10-02 17:19 ` Marek Vasut 2016-10-03 13:54 ` Daniel Vetter 2016-10-05 14:50 ` Marek Vasut
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).