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@xxxxxxxxxx>
> ---
>
> 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
Attachment:
signature.asc
Description: PGP signature
