[PATCH v6 41/44] drm/colorop: allow non-bypass colorops

[Louis Chauvet]

On 03/10/24 - 16:01, Harry Wentland wrote:
> Not all HW will be able to do bypass on all color
> operations. Introduce an 'allow_bypass' boolean for
> all colorop init functions and only create the BYPASS
> property when it's true.
> 
> Signed-off-by: Harry Wentland <harry.wentland at amd.com>
> ---
>  .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 22 +++++---
>  drivers/gpu/drm/drm_atomic.c                  |  3 +-
>  drivers/gpu/drm/drm_colorop.c                 | 51 ++++++++++++-------
>  drivers/gpu/drm/vkms/vkms_colorop.c           |  8 +--
>  include/drm/drm_colorop.h                     | 10 ++--
>  5 files changed, 60 insertions(+), 34 deletions(-)
> 

[...]

> --- a/drivers/gpu/drm/vkms/vkms_colorop.c
> +++ b/drivers/gpu/drm/vkms/vkms_colorop.c
> @@ -31,7 +31,7 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
>  		goto cleanup;
>  	}
>  
> -	ret = drm_colorop_curve_1d_init(dev, ops[i], plane, supported_tfs);
> +	ret = drm_colorop_curve_1d_init(dev, ops[i], plane, supported_tfs, true);
>  	if (ret)
>  		goto cleanup;
>  
> @@ -48,7 +48,7 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
>  		goto cleanup;
>  	}
>  
> -	ret = drm_colorop_ctm_3x4_init(dev, ops[i], plane);
> +	ret = drm_colorop_ctm_3x4_init(dev, ops[i], plane, true);
>  	if (ret)
>  		goto cleanup;
>  
> @@ -64,7 +64,7 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
>  		goto cleanup;
>  	}
>  
> -	ret = drm_colorop_ctm_3x4_init(dev, ops[i], plane);
> +	ret = drm_colorop_ctm_3x4_init(dev, ops[i], plane, true);
>  	if (ret)
>  		goto cleanup;
>  
> @@ -80,7 +80,7 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
>  		goto cleanup;
>  	}
>  
> -	ret = drm_colorop_curve_1d_init(dev, ops[i], plane, supported_tfs);
> +	ret = drm_colorop_curve_1d_init(dev, ops[i], plane, supported_tfs, true);
>  	if (ret)
>  		goto cleanup;

You allow the bypass here, but you forgot to add a check in apply_colorop 
to bypass the colorop when this is set. It seems to be the case in the AMD 
driver too. Or maybe you wanted to pass false in "allow_bypass"?

>  
> diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
> index d3a00296973d..b8c1c4da3444 100644
> --- a/include/drm/drm_colorop.h
> +++ b/include/drm/drm_colorop.h
> @@ -333,14 +333,16 @@ static inline struct drm_colorop *drm_colorop_find(struct drm_device *dev,
>  }
>  
>  int drm_colorop_curve_1d_init(struct drm_device *dev, struct drm_colorop *colorop,
> -			      struct drm_plane *plane, u64 supported_tfs);
> +			      struct drm_plane *plane, u64 supported_tfs,
> +			      bool allow_bypass);
>  int drm_colorop_curve_1d_lut_init(struct drm_device *dev, struct drm_colorop *colorop,
>  				  struct drm_plane *plane, uint32_t lut_size,
> -				  enum drm_colorop_lut1d_interpolation_type lut1d_interpolation);
> +				  enum drm_colorop_lut1d_interpolation_type lut1d_interpolation,
> +				  bool allow_bypass);
>  int drm_colorop_ctm_3x4_init(struct drm_device *dev, struct drm_colorop *colorop,
> -			     struct drm_plane *plane);
> +			     struct drm_plane *plane, bool allow_bypass);
>  int drm_colorop_mult_init(struct drm_device *dev, struct drm_colorop *colorop,
> -			      struct drm_plane *plane);
> +			      struct drm_plane *plane, bool allow_bypass);
>  
>  struct drm_colorop_state *
>  drm_atomic_helper_colorop_duplicate_state(struct drm_colorop *colorop);
> -- 
> 2.46.2
>