dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Kieran Bingham <kieran.bingham@ideasonboard.com>
To: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>,
	dri-devel@lists.freedesktop.org
Cc: linux-renesas-soc@vger.kernel.org,
	Liviu Dudau <Liviu.Dudau@arm.com>,
	linux-media@vger.kernel.org
Subject: Re: [PATCH v6 06/18] media: vsp1: Add vsp1_dl_list argument to .configure_stream() operation
Date: Wed, 13 Mar 2019 10:36:35 +0000	[thread overview]
Message-ID: <cbc85e93-a701-e482-8f53-1f2f8440ef30@ideasonboard.com> (raw)
In-Reply-To: <20190313000532.7087-7-laurent.pinchart+renesas@ideasonboard.com>

Hi Laurent,

On 13/03/2019 00:05, Laurent Pinchart wrote:
> The WPF needs access to the current display list to configure writeback.
> Add a display list pointer to the VSP1 entity .configure_stream()
> operation.
> 
> Only display pipelines can make use of the display list there as
> mem-to-mem pipelines don't have access to a display list at stream
> configuration time. This is not an issue as writeback is only used for
> display pipelines.


As we purposefully inject NULL DL variables, Do we need to add NULL
checks in to these functions

I think I'm happy leaving the null-checks out - because as you state
above - the only time this variable is used, it will not be NULL, and it
is documented as such in the code.

So,

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>


> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/vsp1/vsp1_brx.c    | 1 +
>  drivers/media/platform/vsp1/vsp1_clu.c    | 1 +
>  drivers/media/platform/vsp1/vsp1_drm.c    | 2 +-
>  drivers/media/platform/vsp1/vsp1_entity.c | 3 ++-
>  drivers/media/platform/vsp1/vsp1_entity.h | 7 +++++--
>  drivers/media/platform/vsp1/vsp1_hgo.c    | 1 +
>  drivers/media/platform/vsp1/vsp1_hgt.c    | 1 +
>  drivers/media/platform/vsp1/vsp1_hsit.c   | 1 +
>  drivers/media/platform/vsp1/vsp1_lif.c    | 1 +
>  drivers/media/platform/vsp1/vsp1_lut.c    | 1 +
>  drivers/media/platform/vsp1/vsp1_rpf.c    | 1 +
>  drivers/media/platform/vsp1/vsp1_sru.c    | 1 +
>  drivers/media/platform/vsp1/vsp1_uds.c    | 1 +
>  drivers/media/platform/vsp1/vsp1_uif.c    | 1 +
>  drivers/media/platform/vsp1/vsp1_video.c  | 3 ++-
>  drivers/media/platform/vsp1/vsp1_wpf.c    | 1 +
>  16 files changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_brx.c b/drivers/media/platform/vsp1/vsp1_brx.c
> index 5e50178b057d..43468bc8fb56 100644
> --- a/drivers/media/platform/vsp1/vsp1_brx.c
> +++ b/drivers/media/platform/vsp1/vsp1_brx.c
> @@ -283,6 +283,7 @@ static const struct v4l2_subdev_ops brx_ops = {
>  
>  static void brx_configure_stream(struct vsp1_entity *entity,
>  				 struct vsp1_pipeline *pipe,
> +				 struct vsp1_dl_list *dl,
>  				 struct vsp1_dl_body *dlb)
>  {
>  	struct vsp1_brx *brx = to_brx(&entity->subdev);
> diff --git a/drivers/media/platform/vsp1/vsp1_clu.c b/drivers/media/platform/vsp1/vsp1_clu.c
> index 942fc14c19d1..a47b23bf5abf 100644
> --- a/drivers/media/platform/vsp1/vsp1_clu.c
> +++ b/drivers/media/platform/vsp1/vsp1_clu.c
> @@ -171,6 +171,7 @@ static const struct v4l2_subdev_ops clu_ops = {
>  
>  static void clu_configure_stream(struct vsp1_entity *entity,
>  				 struct vsp1_pipeline *pipe,
> +				 struct vsp1_dl_list *dl,
>  				 struct vsp1_dl_body *dlb)
>  {
>  	struct vsp1_clu *clu = to_clu(&entity->subdev);
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c
> index 89773d3a916c..4f1bc51d1ef4 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -558,7 +558,7 @@ static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe)
>  		}
>  
>  		vsp1_entity_route_setup(entity, pipe, dlb);
> -		vsp1_entity_configure_stream(entity, pipe, dlb);
> +		vsp1_entity_configure_stream(entity, pipe, dl, dlb);
>  		vsp1_entity_configure_frame(entity, pipe, dl, dlb);
>  		vsp1_entity_configure_partition(entity, pipe, dl, dlb);
>  	}
> diff --git a/drivers/media/platform/vsp1/vsp1_entity.c b/drivers/media/platform/vsp1/vsp1_entity.c
> index a54ab528b060..aa9d2286056e 100644
> --- a/drivers/media/platform/vsp1/vsp1_entity.c
> +++ b/drivers/media/platform/vsp1/vsp1_entity.c
> @@ -71,10 +71,11 @@ void vsp1_entity_route_setup(struct vsp1_entity *entity,
>  
>  void vsp1_entity_configure_stream(struct vsp1_entity *entity,
>  				  struct vsp1_pipeline *pipe,
> +				  struct vsp1_dl_list *dl,
>  				  struct vsp1_dl_body *dlb)
>  {
>  	if (entity->ops->configure_stream)
> -		entity->ops->configure_stream(entity, pipe, dlb);
> +		entity->ops->configure_stream(entity, pipe, dl, dlb);
>  }
>  
>  void vsp1_entity_configure_frame(struct vsp1_entity *entity,
> diff --git a/drivers/media/platform/vsp1/vsp1_entity.h b/drivers/media/platform/vsp1/vsp1_entity.h
> index 97acb7795cf1..a1ceb37bb837 100644
> --- a/drivers/media/platform/vsp1/vsp1_entity.h
> +++ b/drivers/media/platform/vsp1/vsp1_entity.h
> @@ -67,7 +67,9 @@ struct vsp1_route {
>   * struct vsp1_entity_operations - Entity operations
>   * @destroy:	Destroy the entity.
>   * @configure_stream:	Setup the hardware parameters for the stream which do
> - *			not vary between frames (pipeline, formats).
> + *			not vary between frames (pipeline, formats). Note that
> + *			the vsp1_dl_list argument is only valid for display
> + *			pipeline and will be NULL for mem-to-mem pipelines.
>   * @configure_frame:	Configure the runtime parameters for each frame.
>   * @configure_partition: Configure partition specific parameters.
>   * @max_width:	Return the max supported width of data that the entity can
> @@ -78,7 +80,7 @@ struct vsp1_route {
>  struct vsp1_entity_operations {
>  	void (*destroy)(struct vsp1_entity *);
>  	void (*configure_stream)(struct vsp1_entity *, struct vsp1_pipeline *,
> -				 struct vsp1_dl_body *);
> +				 struct vsp1_dl_list *, struct vsp1_dl_body *);
>  	void (*configure_frame)(struct vsp1_entity *, struct vsp1_pipeline *,
>  				struct vsp1_dl_list *, struct vsp1_dl_body *);
>  	void (*configure_partition)(struct vsp1_entity *,
> @@ -155,6 +157,7 @@ void vsp1_entity_route_setup(struct vsp1_entity *entity,
>  
>  void vsp1_entity_configure_stream(struct vsp1_entity *entity,
>  				  struct vsp1_pipeline *pipe,
> +				  struct vsp1_dl_list *dl,
>  				  struct vsp1_dl_body *dlb);
>  
>  void vsp1_entity_configure_frame(struct vsp1_entity *entity,
> diff --git a/drivers/media/platform/vsp1/vsp1_hgo.c b/drivers/media/platform/vsp1/vsp1_hgo.c
> index 827373c25351..bf3f981f93a1 100644
> --- a/drivers/media/platform/vsp1/vsp1_hgo.c
> +++ b/drivers/media/platform/vsp1/vsp1_hgo.c
> @@ -131,6 +131,7 @@ static const struct v4l2_ctrl_config hgo_num_bins_control = {
>  
>  static void hgo_configure_stream(struct vsp1_entity *entity,
>  				 struct vsp1_pipeline *pipe,
> +				 struct vsp1_dl_list *dl,
>  				 struct vsp1_dl_body *dlb)
>  {
>  	struct vsp1_hgo *hgo = to_hgo(&entity->subdev);
> diff --git a/drivers/media/platform/vsp1/vsp1_hgt.c b/drivers/media/platform/vsp1/vsp1_hgt.c
> index bb6ce6fdd5f4..aa1c718e0453 100644
> --- a/drivers/media/platform/vsp1/vsp1_hgt.c
> +++ b/drivers/media/platform/vsp1/vsp1_hgt.c
> @@ -127,6 +127,7 @@ static const struct v4l2_ctrl_config hgt_hue_areas = {
>  
>  static void hgt_configure_stream(struct vsp1_entity *entity,
>  				 struct vsp1_pipeline *pipe,
> +				 struct vsp1_dl_list *dl,
>  				 struct vsp1_dl_body *dlb)
>  {
>  	struct vsp1_hgt *hgt = to_hgt(&entity->subdev);
> diff --git a/drivers/media/platform/vsp1/vsp1_hsit.c b/drivers/media/platform/vsp1/vsp1_hsit.c
> index 39ab2e0c7c18..d5ebd9d08c8a 100644
> --- a/drivers/media/platform/vsp1/vsp1_hsit.c
> +++ b/drivers/media/platform/vsp1/vsp1_hsit.c
> @@ -129,6 +129,7 @@ static const struct v4l2_subdev_ops hsit_ops = {
>  
>  static void hsit_configure_stream(struct vsp1_entity *entity,
>  				  struct vsp1_pipeline *pipe,
> +				  struct vsp1_dl_list *dl,
>  				  struct vsp1_dl_body *dlb)
>  {
>  	struct vsp1_hsit *hsit = to_hsit(&entity->subdev);
> diff --git a/drivers/media/platform/vsp1/vsp1_lif.c b/drivers/media/platform/vsp1/vsp1_lif.c
> index 8b0a26335d70..14ed5d7bd061 100644
> --- a/drivers/media/platform/vsp1/vsp1_lif.c
> +++ b/drivers/media/platform/vsp1/vsp1_lif.c
> @@ -84,6 +84,7 @@ static const struct v4l2_subdev_ops lif_ops = {
>  
>  static void lif_configure_stream(struct vsp1_entity *entity,
>  				 struct vsp1_pipeline *pipe,
> +				 struct vsp1_dl_list *dl,
>  				 struct vsp1_dl_body *dlb)
>  {
>  	const struct v4l2_mbus_framefmt *format;
> diff --git a/drivers/media/platform/vsp1/vsp1_lut.c b/drivers/media/platform/vsp1/vsp1_lut.c
> index 64c48d9459b0..9f88842d7048 100644
> --- a/drivers/media/platform/vsp1/vsp1_lut.c
> +++ b/drivers/media/platform/vsp1/vsp1_lut.c
> @@ -147,6 +147,7 @@ static const struct v4l2_subdev_ops lut_ops = {
>  
>  static void lut_configure_stream(struct vsp1_entity *entity,
>  				 struct vsp1_pipeline *pipe,
> +				 struct vsp1_dl_list *dl,
>  				 struct vsp1_dl_body *dlb)
>  {
>  	struct vsp1_lut *lut = to_lut(&entity->subdev);
> diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c b/drivers/media/platform/vsp1/vsp1_rpf.c
> index 616afa7e165f..85587c1b6a37 100644
> --- a/drivers/media/platform/vsp1/vsp1_rpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_rpf.c
> @@ -57,6 +57,7 @@ static const struct v4l2_subdev_ops rpf_ops = {
>  
>  static void rpf_configure_stream(struct vsp1_entity *entity,
>  				 struct vsp1_pipeline *pipe,
> +				 struct vsp1_dl_list *dl,
>  				 struct vsp1_dl_body *dlb)
>  {
>  	struct vsp1_rwpf *rpf = to_rwpf(&entity->subdev);
> diff --git a/drivers/media/platform/vsp1/vsp1_sru.c b/drivers/media/platform/vsp1/vsp1_sru.c
> index b1617cb1f2b9..2b65457ee12f 100644
> --- a/drivers/media/platform/vsp1/vsp1_sru.c
> +++ b/drivers/media/platform/vsp1/vsp1_sru.c
> @@ -269,6 +269,7 @@ static const struct v4l2_subdev_ops sru_ops = {
>  
>  static void sru_configure_stream(struct vsp1_entity *entity,
>  				 struct vsp1_pipeline *pipe,
> +				 struct vsp1_dl_list *dl,
>  				 struct vsp1_dl_body *dlb)
>  {
>  	const struct vsp1_sru_param *param;
> diff --git a/drivers/media/platform/vsp1/vsp1_uds.c b/drivers/media/platform/vsp1/vsp1_uds.c
> index 27012af973b2..5fc04c082d1a 100644
> --- a/drivers/media/platform/vsp1/vsp1_uds.c
> +++ b/drivers/media/platform/vsp1/vsp1_uds.c
> @@ -257,6 +257,7 @@ static const struct v4l2_subdev_ops uds_ops = {
>  
>  static void uds_configure_stream(struct vsp1_entity *entity,
>  				 struct vsp1_pipeline *pipe,
> +				 struct vsp1_dl_list *dl,
>  				 struct vsp1_dl_body *dlb)
>  {
>  	struct vsp1_uds *uds = to_uds(&entity->subdev);
> diff --git a/drivers/media/platform/vsp1/vsp1_uif.c b/drivers/media/platform/vsp1/vsp1_uif.c
> index 4b58d51df231..467d1072577b 100644
> --- a/drivers/media/platform/vsp1/vsp1_uif.c
> +++ b/drivers/media/platform/vsp1/vsp1_uif.c
> @@ -192,6 +192,7 @@ static const struct v4l2_subdev_ops uif_ops = {
>  
>  static void uif_configure_stream(struct vsp1_entity *entity,
>  				 struct vsp1_pipeline *pipe,
> +				 struct vsp1_dl_list *dl,
>  				 struct vsp1_dl_body *dlb)
>  {
>  	struct vsp1_uif *uif = to_uif(&entity->subdev);
> diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
> index 9ae20982604a..fd98e483b2f4 100644
> --- a/drivers/media/platform/vsp1/vsp1_video.c
> +++ b/drivers/media/platform/vsp1/vsp1_video.c
> @@ -825,7 +825,8 @@ static int vsp1_video_setup_pipeline(struct vsp1_pipeline *pipe)
>  
>  	list_for_each_entry(entity, &pipe->entities, list_pipe) {
>  		vsp1_entity_route_setup(entity, pipe, pipe->stream_config);
> -		vsp1_entity_configure_stream(entity, pipe, pipe->stream_config);
> +		vsp1_entity_configure_stream(entity, pipe, NULL,
> +					     pipe->stream_config);
>  	}
>  
>  	return 0;
> diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c b/drivers/media/platform/vsp1/vsp1_wpf.c
> index 18c49e3a7875..fc5c1b0f6633 100644
> --- a/drivers/media/platform/vsp1/vsp1_wpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_wpf.c
> @@ -234,6 +234,7 @@ static void vsp1_wpf_destroy(struct vsp1_entity *entity)
>  
>  static void wpf_configure_stream(struct vsp1_entity *entity,
>  				 struct vsp1_pipeline *pipe,
> +				 struct vsp1_dl_list *dl,
>  				 struct vsp1_dl_body *dlb)
>  {
>  	struct vsp1_rwpf *wpf = to_rwpf(&entity->subdev);
> 

-- 
Regards
--
Kieran
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2019-03-13 10:36 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-13  0:05 [PATCH v6 00/18] R-Car DU display writeback support Laurent Pinchart
2019-03-13  0:05 ` [PATCH v6 01/18] Revert "[media] v4l: vsp1: Supply frames to the DU continuously" Laurent Pinchart
2019-03-13  0:05 ` [PATCH v6 02/18] media: vsp1: wpf: Fix partition configuration for display pipelines Laurent Pinchart
2019-03-13  0:05 ` [PATCH v6 03/18] media: vsp1: Replace leftover occurrence of fragment with body Laurent Pinchart
2019-03-13  0:05 ` [PATCH v6 04/18] media: vsp1: Fix addresses of display-related registers for VSP-DL Laurent Pinchart
2019-03-13  0:05 ` [PATCH v6 05/18] media: vsp1: Replace the display list internal flag with a flags field Laurent Pinchart
2019-03-13  0:05 ` [PATCH v6 06/18] media: vsp1: Add vsp1_dl_list argument to .configure_stream() operation Laurent Pinchart
2019-03-13 10:36   ` Kieran Bingham [this message]
2019-03-13  0:05 ` [PATCH v6 07/18] media: vsp1: dl: Allow chained display lists for display pipelines Laurent Pinchart
2019-03-13 11:07   ` Kieran Bingham
2019-03-13  0:05 ` [PATCH v6 08/18] media: vsp1: wpf: Add writeback support Laurent Pinchart
2019-03-13 10:59   ` Kieran Bingham
2019-03-13 11:15     ` Laurent Pinchart
2019-03-13  0:05 ` [PATCH v6 09/18] media: vsp1: drm: Split RPF format setting to separate function Laurent Pinchart
2019-03-13 11:12   ` Kieran Bingham
2019-03-13 11:17     ` Laurent Pinchart
2019-03-13  0:05 ` [PATCH v6 10/18] media: vsp1: drm: Extend frame completion API to the DU driver Laurent Pinchart
2019-03-13 11:26   ` Kieran Bingham
2019-03-13 15:50     ` Laurent Pinchart
2019-03-13  0:05 ` [PATCH v6 11/18] media: vsp1: drm: Implement writeback support Laurent Pinchart
2019-03-13 11:42   ` Kieran Bingham
2019-03-13 15:56     ` Laurent Pinchart
2019-03-14  8:28       ` Kieran Bingham
2019-03-14 12:09         ` Laurent Pinchart
2019-03-13  0:05 ` [PATCH v6 12/18] drm: writeback: Cleanup job ownership handling when queuing job Laurent Pinchart
2019-03-13 11:45   ` Kieran Bingham
2019-03-13  0:05 ` [PATCH v6 13/18] drm: writeback: Fix leak of writeback job Laurent Pinchart
2019-03-13  0:05 ` [PATCH v6 14/18] drm: writeback: Add job prepare and cleanup operations Laurent Pinchart
2019-03-15 17:54   ` Liviu Dudau
2019-03-13  0:05 ` [PATCH v6 15/18] drm: rcar-du: Fix rcar_du_crtc structure documentation Laurent Pinchart
2019-03-13  0:05 ` [PATCH v6 16/18] drm: rcar-du: Store V4L2 fourcc in rcar_du_format_info structure Laurent Pinchart
2019-03-13  0:05 ` [PATCH v6 17/18] drm: rcar-du: vsp: Extract framebuffer (un)mapping to separate functions Laurent Pinchart
2019-03-13 11:54   ` Kieran Bingham
2019-03-13  0:05 ` [PATCH v6 18/18] drm: rcar-du: Add writeback support for R-Car Gen3 Laurent Pinchart
2019-03-13 12:06   ` Kieran Bingham
2019-03-13 16:08     ` 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=cbc85e93-a701-e482-8f53-1f2f8440ef30@ideasonboard.com \
    --to=kieran.bingham@ideasonboard.com \
    --cc=Liviu.Dudau@arm.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=laurent.pinchart+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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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).