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

[Maxime Ripard]

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.

>  {
>  	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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/freedreno/attachments/20231204/ab6ec116/attachment-0001.sig>