linux-kernel - Re: [PATCH 3/3] drm/crtc-helper: Add a drm_crtc_helper_atomic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 11 Oct 2022 15:26:08 +0200
From:   Javier Martinez Canillas <javierm@...hat.com>
To:     Thomas Zimmermann <tzimmermann@...e.de>,
        linux-kernel@...r.kernel.org
Cc:     Daniel Vetter <daniel@...ll.ch>, David Airlie <airlied@...ux.ie>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH 3/3] drm/crtc-helper: Add a drm_crtc_helper_atomic_check()
 helper

On 10/11/22 15:21, Thomas Zimmermann wrote:
> Hi
> 
> Am 10.10.22 um 19:02 schrieb Javier Martinez Canillas:
>> Provides a default CRTC state check handler for CRTCs that only have one
>> primary plane attached.
>>
>> There are some drivers that duplicate this logic in their helpers, such as
>> simpledrm and ssd130x. Factor out this common code into a CRTC helper and
>> make drivers use it.
>>
>> Signed-off-by: Javier Martinez Canillas <javierm@...hat.com>
> 
> Reviewed-by: Thomas Zimmermann <tzimmermann@...e.de>
> 
> There really isn't much here for now. I suspect that there are more 
> drivers that could use this helper. If you merge this before ofdrm, I'll 
> rebase ofdrm on top.
>

Sure. I probably won't post a v2 until tomorrow and I believe ofdrm is
ready to be merged, so I'll just rebase this series on top of that once
just push it.

> Please also see my comment below.
> 
> 

[...]

>> +/**
>> + * drm_crtc_helper_atomic_check() - Helper to check CRTC atomic-state
>> + * @crtc: CRTC to check
>> + * @state: atomic state object
>> + *
>> + * Provides a default CRTC-state check handler for CRTCs that only have
>> + * one primary plane attached to it.
>> + *
>> + * This is often the case for the CRTC of simple framebuffers.
> 
> I'd add a reference to drm_plane_helper_atomic_check() to this 
> paragraph. Like
> 
>    See drm_plane_helper_atomic_check() for the respective plane helpers.
> 
> And also reference back from the plane-check helper to the CRTC-check 
> helper.
>

Good idea, I'll do that. Thanks for your review.
 
> Best regards
> Thomas
> 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat