Re: [1/8] drm/plane-helper: Move drm_plane_helper_atomic_check() into udl

[Sui Jingfeng <sui.jingfeng@linux.dev>]

Hi,


On 2023/12/1 16:22, Thomas Zimmermann wrote:
> Hi
>
> Am 01.12.23 um 03:36 schrieb Sui Jingfeng:
>> Hi,
>>
>>
>> On 2023/11/28 18:45, Thomas Zimmermann wrote:
>>> The udl driver is the only caller of drm_plane_helper_atomic_check().
>>> Move the function into the driver. No functional changes.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> ---
>>>   drivers/gpu/drm/drm_plane_helper.c | 32 
>>> ------------------------------
>>>   drivers/gpu/drm/udl/udl_modeset.c  | 19 ++++++++++++++++--
>>>   include/drm/drm_plane_helper.h     |  2 --
>>>   3 files changed, 17 insertions(+), 36 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_plane_helper.c 
>>> b/drivers/gpu/drm/drm_plane_helper.c
>>> index 5e95089676ff8..7982be4b0306d 100644
>>> --- a/drivers/gpu/drm/drm_plane_helper.c
>>> +++ b/drivers/gpu/drm/drm_plane_helper.c
>>> @@ -279,35 +279,3 @@ void drm_plane_helper_destroy(struct drm_plane 
>>> *plane)
>>>       kfree(plane);
>>>   }
>>>   EXPORT_SYMBOL(drm_plane_helper_destroy);
>>> -
>>> -/**
>>> - * drm_plane_helper_atomic_check() - Helper to check plane 
>>> atomic-state
>>> - * @plane: plane to check
>>> - * @state: atomic state object
>>> - *
>>> - * Provides a default plane-state check handler for planes whose 
>>> atomic-state
>>> - * scale and positioning are not expected to change since the plane 
>>> is always
>>> - * a fullscreen scanout buffer.
>>> - *
>>> - * This is often the case for the primary plane of simple 
>>> framebuffers. See
>>> - * also drm_crtc_helper_atomic_check() for the respective 
>>> CRTC-state check
>>> - * helper function.
>>> - *
>>> - * RETURNS:
>>> - * Zero on success, or an errno code otherwise.
>>> - */
>>> -int drm_plane_helper_atomic_check(struct drm_plane *plane, struct 
>>> drm_atomic_state *state)
>>> -{
>>> -    struct drm_plane_state *new_plane_state = 
>>> drm_atomic_get_new_plane_state(state, plane);
>>> -    struct drm_crtc *new_crtc = new_plane_state->crtc;
>>> -    struct drm_crtc_state *new_crtc_state = NULL;
>>> -
>>> -    if (new_crtc)
>>> -        new_crtc_state = drm_atomic_get_new_crtc_state(state, 
>>> new_crtc);
>>> -
>>> -    return drm_atomic_helper_check_plane_state(new_plane_state, 
>>> new_crtc_state,
>>> -                           DRM_PLANE_NO_SCALING,
>>> -                           DRM_PLANE_NO_SCALING,
>>> -                           false, false);
>>> -}
>>> -EXPORT_SYMBOL(drm_plane_helper_atomic_check);
>>
>>
>> Since this function is removed, does the comments of the 
>> drm_crtc_helper_atomic_check()
>> function (in the drm_crtc_helper.c) need to update as well? I'm ask 
>> because I see the
>> comments of the drm_crtc_helper_atomic_check() still referencing this 
>> function.
>
> Good point. I'll update that comment. Thanks for reviewing.
>
OK,  with this trivial problem solved:


Acked-by: Sui Jingfeng <suijingfeng@loongson.cn>


> Best regards
> Thomas
>
>>
>>
>>> diff --git a/drivers/gpu/drm/udl/udl_modeset.c 
>>> b/drivers/gpu/drm/udl/udl_modeset.c
>>> index 40876bcdd79a4..7702359c90c22 100644
>>> --- a/drivers/gpu/drm/udl/udl_modeset.c
>>> +++ b/drivers/gpu/drm/udl/udl_modeset.c
>>> @@ -21,7 +21,6 @@
>>>   #include <drm/drm_gem_framebuffer_helper.h>
>>>   #include <drm/drm_gem_shmem_helper.h>
>>>   #include <drm/drm_modeset_helper_vtables.h>
>>> -#include <drm/drm_plane_helper.h>
>>>   #include <drm/drm_probe_helper.h>
>>>   #include <drm/drm_vblank.h>
>>> @@ -261,6 +260,22 @@ static const uint64_t 
>>> udl_primary_plane_fmtmods[] = {
>>>       DRM_FORMAT_MOD_INVALID
>>>   };
>>> +static int udl_primary_plane_helper_atomic_check(struct drm_plane 
>>> *plane,
>>> +                         struct drm_atomic_state *state)
>>> +{
>>> +    struct drm_plane_state *new_plane_state = 
>>> drm_atomic_get_new_plane_state(state, plane);
>>> +    struct drm_crtc *new_crtc = new_plane_state->crtc;
>>> +    struct drm_crtc_state *new_crtc_state = NULL;
>>> +
>>> +    if (new_crtc)
>>> +        new_crtc_state = drm_atomic_get_new_crtc_state(state, 
>>> new_crtc);
>>> +
>>> +    return drm_atomic_helper_check_plane_state(new_plane_state, 
>>> new_crtc_state,
>>> +                           DRM_PLANE_NO_SCALING,
>>> +                           DRM_PLANE_NO_SCALING,
>>> +                           false, false);
>>> +}
>>> +
>>>   static void udl_primary_plane_helper_atomic_update(struct 
>>> drm_plane *plane,
>>>                              struct drm_atomic_state *state)
>>>   {
>>> @@ -296,7 +311,7 @@ static void 
>>> udl_primary_plane_helper_atomic_update(struct drm_plane *plane,
>>>   static const struct drm_plane_helper_funcs 
>>> udl_primary_plane_helper_funcs = {
>>>       DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
>>> -    .atomic_check = drm_plane_helper_atomic_check,
>>> +    .atomic_check = udl_primary_plane_helper_atomic_check,
>>>       .atomic_update = udl_primary_plane_helper_atomic_update,
>>>   };
>>> diff --git a/include/drm/drm_plane_helper.h 
>>> b/include/drm/drm_plane_helper.h
>>> index 3a574e8cd22f4..75f9c4830564a 100644
>>> --- a/include/drm/drm_plane_helper.h
>>> +++ b/include/drm/drm_plane_helper.h
>>> @@ -26,7 +26,6 @@
>>>   #include <linux/types.h>
>>> -struct drm_atomic_state;
>>>   struct drm_crtc;
>>>   struct drm_framebuffer;
>>>   struct drm_modeset_acquire_ctx;
>>> @@ -42,7 +41,6 @@ int drm_plane_helper_update_primary(struct 
>>> drm_plane *plane, struct drm_crtc *cr
>>>   int drm_plane_helper_disable_primary(struct drm_plane *plane,
>>>                        struct drm_modeset_acquire_ctx *ctx);
>>>   void drm_plane_helper_destroy(struct drm_plane *plane);
>>> -int drm_plane_helper_atomic_check(struct drm_plane *plane, struct 
>>> drm_atomic_state *state);
>>>   /**
>>>    * DRM_PLANE_NON_ATOMIC_FUNCS - Default plane functions for 
>>> non-atomic drivers
>