[RFC,v1,08/19] media: renesas: vsp1: Store RPF partition configuration per RPF instance

Message ID 20231122043009.2741-9-laurent.pinchart+renesas@ideasonboard.com (mailing list archive)
State New
Headers
Series media: renesas: vsp1: Conversion to subdev active state |

Commit Message

Laurent Pinchart Nov. 22, 2023, 4:29 a.m. UTC
  The vsp1_partition structure stores the RPF partition configuration in a
single field for all RPF instances, while each RPF can have its own
configuration. Fix it by storing the configuration separately for each
RPF instance.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/vsp1/vsp1_pipe.h | 2 +-
 drivers/media/platform/renesas/vsp1/vsp1_rpf.c  | 8 +++++---
 2 files changed, 6 insertions(+), 4 deletions(-)
  

Comments

Jacopo Mondi June 18, 2024, 10:32 a.m. UTC | #1
Hi Laurent

On Wed, Nov 22, 2023 at 06:29:58AM GMT, Laurent Pinchart wrote:
> The vsp1_partition structure stores the RPF partition configuration in a
> single field for all RPF instances, while each RPF can have its own
> configuration. Fix it by storing the configuration separately for each
> RPF instance.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Is this worth a Fixes: tag ?
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

> ---
>  drivers/media/platform/renesas/vsp1/vsp1_pipe.h | 2 +-
>  drivers/media/platform/renesas/vsp1/vsp1_rpf.c  | 8 +++++---
>  2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
> index 02e98d843730..840fd3288efb 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
> @@ -73,7 +73,7 @@ struct vsp1_partition_window {
>   * @wpf: The WPF partition window configuration
>   */
>  struct vsp1_partition {
> -	struct vsp1_partition_window rpf;
> +	struct vsp1_partition_window rpf[VSP1_MAX_RPF];
>  	struct vsp1_partition_window uds_sink;
>  	struct vsp1_partition_window uds_source;
>  	struct vsp1_partition_window sru;
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> index 3e62638fdce6..42b0c5655404 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> @@ -311,8 +311,8 @@ static void rpf_configure_partition(struct vsp1_entity *entity,
>  	 * 'width' need to be adjusted.
>  	 */
>  	if (pipe->partitions > 1) {
> -		crop.width = pipe->partition->rpf.width;
> -		crop.left += pipe->partition->rpf.left;
> +		crop.width = pipe->partition->rpf[rpf->entity.index].width;
> +		crop.left += pipe->partition->rpf[rpf->entity.index].left;
>  	}
>
>  	if (pipe->interlaced) {
> @@ -367,7 +367,9 @@ static void rpf_partition(struct vsp1_entity *entity,
>  			  unsigned int partition_idx,
>  			  struct vsp1_partition_window *window)
>  {
> -	partition->rpf = *window;
> +	struct vsp1_rwpf *rpf = to_rwpf(&entity->subdev);
> +
> +	partition->rpf[rpf->entity.index] = *window;
>  }
>
>  static const struct vsp1_entity_operations rpf_entity_ops = {
> --
> Regards,
>
> Laurent Pinchart
>
>
  
Laurent Pinchart June 18, 2024, 4:09 p.m. UTC | #2
On Tue, Jun 18, 2024 at 12:32:47PM +0200, Jacopo Mondi wrote:
> Hi Laurent
> 
> On Wed, Nov 22, 2023 at 06:29:58AM GMT, Laurent Pinchart wrote:
> > The vsp1_partition structure stores the RPF partition configuration in a
> > single field for all RPF instances, while each RPF can have its own
> > configuration. Fix it by storing the configuration separately for each
> > RPF instance.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> Is this worth a Fixes: tag ?

I'll add

Fixes: ab45e8585182 ("media: v4l: vsp1: Allow entities to participate in the partition algorithm")

This was introduced in v5.2, I'm not sure if the fix can be meaningfully
backported.

> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> 
> > ---
> >  drivers/media/platform/renesas/vsp1/vsp1_pipe.h | 2 +-
> >  drivers/media/platform/renesas/vsp1/vsp1_rpf.c  | 8 +++++---
> >  2 files changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
> > index 02e98d843730..840fd3288efb 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
> > @@ -73,7 +73,7 @@ struct vsp1_partition_window {
> >   * @wpf: The WPF partition window configuration
> >   */
> >  struct vsp1_partition {
> > -	struct vsp1_partition_window rpf;
> > +	struct vsp1_partition_window rpf[VSP1_MAX_RPF];
> >  	struct vsp1_partition_window uds_sink;
> >  	struct vsp1_partition_window uds_source;
> >  	struct vsp1_partition_window sru;
> > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> > index 3e62638fdce6..42b0c5655404 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> > @@ -311,8 +311,8 @@ static void rpf_configure_partition(struct vsp1_entity *entity,
> >  	 * 'width' need to be adjusted.
> >  	 */
> >  	if (pipe->partitions > 1) {
> > -		crop.width = pipe->partition->rpf.width;
> > -		crop.left += pipe->partition->rpf.left;
> > +		crop.width = pipe->partition->rpf[rpf->entity.index].width;
> > +		crop.left += pipe->partition->rpf[rpf->entity.index].left;
> >  	}
> >
> >  	if (pipe->interlaced) {
> > @@ -367,7 +367,9 @@ static void rpf_partition(struct vsp1_entity *entity,
> >  			  unsigned int partition_idx,
> >  			  struct vsp1_partition_window *window)
> >  {
> > -	partition->rpf = *window;
> > +	struct vsp1_rwpf *rpf = to_rwpf(&entity->subdev);
> > +
> > +	partition->rpf[rpf->entity.index] = *window;
> >  }
> >
> >  static const struct vsp1_entity_operations rpf_entity_ops = {
  

Patch

diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
index 02e98d843730..840fd3288efb 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
@@ -73,7 +73,7 @@  struct vsp1_partition_window {
  * @wpf: The WPF partition window configuration
  */
 struct vsp1_partition {
-	struct vsp1_partition_window rpf;
+	struct vsp1_partition_window rpf[VSP1_MAX_RPF];
 	struct vsp1_partition_window uds_sink;
 	struct vsp1_partition_window uds_source;
 	struct vsp1_partition_window sru;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
index 3e62638fdce6..42b0c5655404 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
@@ -311,8 +311,8 @@  static void rpf_configure_partition(struct vsp1_entity *entity,
 	 * 'width' need to be adjusted.
 	 */
 	if (pipe->partitions > 1) {
-		crop.width = pipe->partition->rpf.width;
-		crop.left += pipe->partition->rpf.left;
+		crop.width = pipe->partition->rpf[rpf->entity.index].width;
+		crop.left += pipe->partition->rpf[rpf->entity.index].left;
 	}
 
 	if (pipe->interlaced) {
@@ -367,7 +367,9 @@  static void rpf_partition(struct vsp1_entity *entity,
 			  unsigned int partition_idx,
 			  struct vsp1_partition_window *window)
 {
-	partition->rpf = *window;
+	struct vsp1_rwpf *rpf = to_rwpf(&entity->subdev);
+
+	partition->rpf[rpf->entity.index] = *window;
 }
 
 static const struct vsp1_entity_operations rpf_entity_ops = {