[PATCH RESEND] drm/atomic-helper: rename drm_atomic_helper_check_wb_encoder_state

[Dmitry Baryshkov]

On 04/12/2023 10:38, Maxime Ripard wrote:
> On Sat, Dec 02, 2023 at 12:07:49AM +0200, Dmitry Baryshkov wrote:
>> The drm_atomic_helper_check_wb_encoder_state() function doesn't use
>> encoder for anything other than getting the drm_device instance. The
>> function's description talks about checking the writeback connector
>> state, not the encoder state. Moreover, there is no such thing as an
>> encoder state, encoders generally do not have a state on their own.
>>
>> Drop the first argument and rename the function to
>> drm_atomic_helper_check_wb_connector_state().
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>> ---
>>
>> Resending, no reaction for two months
>>
>> ---
>>   drivers/gpu/drm/drm_atomic_helper.c   | 10 ++++------
>>   drivers/gpu/drm/vkms/vkms_writeback.c |  2 +-
>>   include/drm/drm_atomic_helper.h       |  3 +--
>>   3 files changed, 6 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>> index 2444fc33dd7c..d69591381f00 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -795,8 +795,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>>   EXPORT_SYMBOL(drm_atomic_helper_check_modeset);
>>   
>>   /**
>> - * drm_atomic_helper_check_wb_encoder_state() - Check writeback encoder state
>> - * @encoder: encoder state to check
>> + * drm_atomic_helper_check_wb_connector_state() - Check writeback connector state
>>    * @conn_state: connector state to check
>>    *
>>    * Checks if the writeback connector state is valid, and returns an error if it
>> @@ -806,8 +805,7 @@ EXPORT_SYMBOL(drm_atomic_helper_check_modeset);
>>    * Zero for success or -errno
>>    */
>>   int
>> -drm_atomic_helper_check_wb_encoder_state(struct drm_encoder *encoder,
>> -					 struct drm_connector_state *conn_state)
>> +drm_atomic_helper_check_wb_connector_state(struct drm_connector_state *conn_state)
> 
> AFAIK, all the helpers take the object as first argument, so I'm fine
> with the name change but it should take a drm_connector too. And ideally
> a drm_atomic_state pointer instead of drm_connector_state too.

I think we then might take even further step and pass 
drm_writeback_connector to this function. I'll send this as a part of v2.

> 
>>   {
>>   	struct drm_writeback_job *wb_job = conn_state->writeback_job;
>>   	struct drm_property_blob *pixel_format_blob;
>> @@ -827,11 +825,11 @@ drm_atomic_helper_check_wb_encoder_state(struct drm_encoder *encoder,
>>   		if (fb->format->format == formats[i])
>>   			return 0;
>>   
>> -	drm_dbg_kms(encoder->dev, "Invalid pixel format %p4cc\n", &fb->format->format);
>> +	drm_dbg_kms(conn_state->connector->dev, "Invalid pixel format %p4cc\n", &fb->format->format);
> 
> Which would also avoid the checkpatch warning there.
> 
> Maxime

-- 
With best wishes
Dmitry