From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> To: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> Cc: linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: Re: [PATCH v3 2/3] v4l: vsp1: Extend VSP1 module API to allow DRM callbacks Date: Sun, 05 Mar 2017 23:58:33 +0200 [thread overview] Message-ID: <16730350.NvnRv331tg@avalon> (raw) In-Reply-To: <200127cf978c8d8904e43ab2b28a225e8a786f6e.1488729419.git-series.kieran.bingham+renesas@ideasonboard.com> Hi Kieran, Thank you for the patch. On Sunday 05 Mar 2017 16:00:03 Kieran Bingham wrote: > To be able to perform page flips in DRM without flicker we need to be > able to notify the rcar-du module when the VSP has completed its > processing. > > We must not have bidirectional dependencies on the two components to > maintain support for loadable modules, thus we extend the API to allow > a callback to be registered within the VSP DRM interface. > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > --- > drivers/media/platform/vsp1/vsp1_drm.c | 17 +++++++++++++++++ > drivers/media/platform/vsp1/vsp1_drm.h | 11 +++++++++++ > include/media/vsp1.h | 13 +++++++++++++ > 3 files changed, 41 insertions(+) > > diff --git a/drivers/media/platform/vsp1/vsp1_drm.c > b/drivers/media/platform/vsp1/vsp1_drm.c index 4ee437c7ff0c..d93bf7d3a39e > 100644 > --- a/drivers/media/platform/vsp1/vsp1_drm.c > +++ b/drivers/media/platform/vsp1/vsp1_drm.c > @@ -37,6 +37,14 @@ void vsp1_drm_display_start(struct vsp1_device *vsp1) > vsp1_dlm_irq_display_start(vsp1->drm->pipe.output->dlm); > } > > +static void vsp1_du_pipeline_frame_end(struct vsp1_pipeline *pipe) > +{ > + struct vsp1_drm *drm = to_vsp1_drm(pipe); > + > + if (drm->du_complete) > + drm->du_complete(drm->du_private); > +} > + > /* ------------------------------------------------------------------------ > * DU Driver API > */ > @@ -96,6 +104,7 @@ int vsp1_du_setup_lif(struct device *dev, const struct > vsp1_du_lif_config *cfg) } > > pipe->num_inputs = 0; > + vsp1->drm->du_complete = NULL; > > vsp1_dlm_reset(pipe->output->dlm); > vsp1_device_put(vsp1); > @@ -200,6 +209,13 @@ int vsp1_du_setup_lif(struct device *dev, const struct > vsp1_du_lif_config *cfg) if (ret < 0) > return ret; > > + /* > + * Register a callback to allow us to notify the DRM framework of frame s/framework/driver/ > + * completion events. > + */ > + vsp1->drm->du_complete = cfg->callback; > + vsp1->drm->du_private = cfg->callback_data; > + > ret = media_pipeline_start(&pipe->output->entity.subdev.entity, > &pipe->pipe); > if (ret < 0) { > @@ -607,6 +623,7 @@ int vsp1_drm_init(struct vsp1_device *vsp1) > pipe->lif = &vsp1->lif->entity; > pipe->output = vsp1->wpf[0]; > pipe->output->pipe = pipe; > + pipe->frame_end = vsp1_du_pipeline_frame_end; > > return 0; > } > diff --git a/drivers/media/platform/vsp1/vsp1_drm.h > b/drivers/media/platform/vsp1/vsp1_drm.h index c8d2f88fc483..3de2095cb0ce > 100644 > --- a/drivers/media/platform/vsp1/vsp1_drm.h > +++ b/drivers/media/platform/vsp1/vsp1_drm.h > @@ -23,6 +23,8 @@ > * @num_inputs: number of active pipeline inputs at the beginning of an > update > * @inputs: source crop rectangle, destination compose rectangle and > z-order > * position for every input > + * @du_complete: frame completion callback for the DU driver (optional) > + * @du_private: data to be passed to the du_complete callback > */ > struct vsp1_drm { > struct vsp1_pipeline pipe; > @@ -33,8 +35,17 @@ struct vsp1_drm { > struct v4l2_rect compose; > unsigned int zpos; > } inputs[VSP1_MAX_RPF]; > + > + /* Frame syncronisation */ s/syncronisation/synchronisation/ > + void (*du_complete)(void *); > + void *du_private; > }; > > +static inline struct vsp1_drm *to_vsp1_drm(struct vsp1_pipeline *pipe) > +{ > + return container_of(pipe, struct vsp1_drm, pipe); > +} > + > int vsp1_drm_init(struct vsp1_device *vsp1); > void vsp1_drm_cleanup(struct vsp1_device *vsp1); > int vsp1_drm_create_links(struct vsp1_device *vsp1); > diff --git a/include/media/vsp1.h b/include/media/vsp1.h > index bfc701f04f3f..d59d0adf560d 100644 > --- a/include/media/vsp1.h > +++ b/include/media/vsp1.h > @@ -20,9 +20,22 @@ struct device; > > int vsp1_du_init(struct device *dev); > > +/** > + * struct vsp1_du_lif_config - VSP LIF configuration > + * @width: output frame width > + * @height: output frame height > + * @callback: frame completion callback function (optional) > + * @callback_data: data to be passed to the frame completion callback > + * > + * When the optional callback is provided to the VSP1, the VSP1 must > guarantee > + * that one completion callback is performed after every > vsp1_du_atomic_flush() This paragraph should be part of the @callback documentation. I would phrase it as * @callback: frame completion callback function (optional). When a callback * is provided, the VSP driver guarantees that it will be called * once and only once for each vsp1_du_atomic_flush() call. With this fixed, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> If you're fine with the above changes there's no need to resubmit, I'll fix when applying the patch. > + */ > struct vsp1_du_lif_config { > unsigned int width; > unsigned int height; > + > + void (*callback)(void *); > + void *callback_data; > }; > > int vsp1_du_setup_lif(struct device *dev, const struct vsp1_du_lif_config > *cfg); -- Regards, Laurent Pinchart
WARNING: multiple messages have this Message-ID (diff)
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> To: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> Cc: linux-renesas-soc@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org Subject: Re: [PATCH v3 2/3] v4l: vsp1: Extend VSP1 module API to allow DRM callbacks Date: Sun, 05 Mar 2017 23:58:33 +0200 [thread overview] Message-ID: <16730350.NvnRv331tg@avalon> (raw) In-Reply-To: <200127cf978c8d8904e43ab2b28a225e8a786f6e.1488729419.git-series.kieran.bingham+renesas@ideasonboard.com> Hi Kieran, Thank you for the patch. On Sunday 05 Mar 2017 16:00:03 Kieran Bingham wrote: > To be able to perform page flips in DRM without flicker we need to be > able to notify the rcar-du module when the VSP has completed its > processing. > > We must not have bidirectional dependencies on the two components to > maintain support for loadable modules, thus we extend the API to allow > a callback to be registered within the VSP DRM interface. > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > --- > drivers/media/platform/vsp1/vsp1_drm.c | 17 +++++++++++++++++ > drivers/media/platform/vsp1/vsp1_drm.h | 11 +++++++++++ > include/media/vsp1.h | 13 +++++++++++++ > 3 files changed, 41 insertions(+) > > diff --git a/drivers/media/platform/vsp1/vsp1_drm.c > b/drivers/media/platform/vsp1/vsp1_drm.c index 4ee437c7ff0c..d93bf7d3a39e > 100644 > --- a/drivers/media/platform/vsp1/vsp1_drm.c > +++ b/drivers/media/platform/vsp1/vsp1_drm.c > @@ -37,6 +37,14 @@ void vsp1_drm_display_start(struct vsp1_device *vsp1) > vsp1_dlm_irq_display_start(vsp1->drm->pipe.output->dlm); > } > > +static void vsp1_du_pipeline_frame_end(struct vsp1_pipeline *pipe) > +{ > + struct vsp1_drm *drm = to_vsp1_drm(pipe); > + > + if (drm->du_complete) > + drm->du_complete(drm->du_private); > +} > + > /* ------------------------------------------------------------------------ > * DU Driver API > */ > @@ -96,6 +104,7 @@ int vsp1_du_setup_lif(struct device *dev, const struct > vsp1_du_lif_config *cfg) } > > pipe->num_inputs = 0; > + vsp1->drm->du_complete = NULL; > > vsp1_dlm_reset(pipe->output->dlm); > vsp1_device_put(vsp1); > @@ -200,6 +209,13 @@ int vsp1_du_setup_lif(struct device *dev, const struct > vsp1_du_lif_config *cfg) if (ret < 0) > return ret; > > + /* > + * Register a callback to allow us to notify the DRM framework of frame s/framework/driver/ > + * completion events. > + */ > + vsp1->drm->du_complete = cfg->callback; > + vsp1->drm->du_private = cfg->callback_data; > + > ret = media_pipeline_start(&pipe->output->entity.subdev.entity, > &pipe->pipe); > if (ret < 0) { > @@ -607,6 +623,7 @@ int vsp1_drm_init(struct vsp1_device *vsp1) > pipe->lif = &vsp1->lif->entity; > pipe->output = vsp1->wpf[0]; > pipe->output->pipe = pipe; > + pipe->frame_end = vsp1_du_pipeline_frame_end; > > return 0; > } > diff --git a/drivers/media/platform/vsp1/vsp1_drm.h > b/drivers/media/platform/vsp1/vsp1_drm.h index c8d2f88fc483..3de2095cb0ce > 100644 > --- a/drivers/media/platform/vsp1/vsp1_drm.h > +++ b/drivers/media/platform/vsp1/vsp1_drm.h > @@ -23,6 +23,8 @@ > * @num_inputs: number of active pipeline inputs at the beginning of an > update > * @inputs: source crop rectangle, destination compose rectangle and > z-order > * position for every input > + * @du_complete: frame completion callback for the DU driver (optional) > + * @du_private: data to be passed to the du_complete callback > */ > struct vsp1_drm { > struct vsp1_pipeline pipe; > @@ -33,8 +35,17 @@ struct vsp1_drm { > struct v4l2_rect compose; > unsigned int zpos; > } inputs[VSP1_MAX_RPF]; > + > + /* Frame syncronisation */ s/syncronisation/synchronisation/ > + void (*du_complete)(void *); > + void *du_private; > }; > > +static inline struct vsp1_drm *to_vsp1_drm(struct vsp1_pipeline *pipe) > +{ > + return container_of(pipe, struct vsp1_drm, pipe); > +} > + > int vsp1_drm_init(struct vsp1_device *vsp1); > void vsp1_drm_cleanup(struct vsp1_device *vsp1); > int vsp1_drm_create_links(struct vsp1_device *vsp1); > diff --git a/include/media/vsp1.h b/include/media/vsp1.h > index bfc701f04f3f..d59d0adf560d 100644 > --- a/include/media/vsp1.h > +++ b/include/media/vsp1.h > @@ -20,9 +20,22 @@ struct device; > > int vsp1_du_init(struct device *dev); > > +/** > + * struct vsp1_du_lif_config - VSP LIF configuration > + * @width: output frame width > + * @height: output frame height > + * @callback: frame completion callback function (optional) > + * @callback_data: data to be passed to the frame completion callback > + * > + * When the optional callback is provided to the VSP1, the VSP1 must > guarantee > + * that one completion callback is performed after every > vsp1_du_atomic_flush() This paragraph should be part of the @callback documentation. I would phrase it as * @callback: frame completion callback function (optional). When a callback * is provided, the VSP driver guarantees that it will be called * once and only once for each vsp1_du_atomic_flush() call. With this fixed, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> If you're fine with the above changes there's no need to resubmit, I'll fix when applying the patch. > + */ > struct vsp1_du_lif_config { > unsigned int width; > unsigned int height; > + > + void (*callback)(void *); > + void *callback_data; > }; > > int vsp1_du_setup_lif(struct device *dev, const struct vsp1_du_lif_config > *cfg); -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2017-03-05 21:57 UTC|newest] Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-03-05 16:00 [PATCH v3 0/3] RCAR-DU, VSP1: Prevent pre-emptive frame flips on VSP1-DRM pipelines Kieran Bingham 2017-03-05 16:00 ` [PATCH v3 1/3] v4l: vsp1: Postpone frame end handling in event of display list race Kieran Bingham 2017-03-05 21:58 ` Laurent Pinchart 2017-03-05 21:58 ` Laurent Pinchart 2017-03-05 16:00 ` [PATCH v3 2/3] v4l: vsp1: Extend VSP1 module API to allow DRM callbacks Kieran Bingham 2017-03-05 21:58 ` Laurent Pinchart [this message] 2017-03-05 21:58 ` Laurent Pinchart 2017-03-05 22:01 ` Kieran Bingham 2017-03-05 16:00 ` [PATCH v3 3/3] drm: rcar-du: Register a completion callback with VSP1 Kieran Bingham 2017-03-05 16:57 ` Sergei Shtylyov 2017-03-05 22:01 ` Laurent Pinchart
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=16730350.NvnRv331tg@avalon \ --to=laurent.pinchart@ideasonboard.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=kieran.bingham+renesas@ideasonboard.com \ --cc=linux-media@vger.kernel.org \ --cc=linux-renesas-soc@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.