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

[Alex Hung]

On 10/4/24 05:43, Louis Chauvet wrote:
> 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"?


The bypass is used in each color transform. For example, it is used in 
pre_blend_color_transform() of vkms_composer.c, before calling 
apply_colorop().

Similarly, it is checked before calling each color transformation in 
amdgpu_dm_color.c, such as __set_colorop_in_tf_1d_curve(), 
__set_dm_plane_colorop_3x4_matrix() and etc.


> 
>>   
>> 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
>>