[PATCH] drm/amdgpu/dc: Simplify drm_crtc_state::active checks

Michel Dänzer michel at daenzer.net
Wed Jul 22 14:25:09 UTC 2020


On 2020-07-22 3:10 p.m., Kazlauskas, Nicholas wrote:
> On 2020-07-22 8:51 a.m., Daniel Vetter wrote:
>> On Wed, Jul 22, 2020 at 2:38 PM Michel Dänzer <michel at daenzer.net> wrote:
>>>
>>> From: Michel Dänzer <mdaenzer at redhat.com>
>>>
>>> drm_atomic_crtc_check enforces that ::active can only be true if
>>> ::enable is as well.
>>>
>>> Signed-off-by: Michel Dänzer <mdaenzer at redhat.com>
> 
> Looks fine to me. The check is sufficiently old enough that I don't mind
> relying on the core for this either.
> 
> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
> 
>>
>> modeset vs modereset is a bit an inglorious name choice ... since this
>> seems to be glue code and not part of core dc, maybe rename to
>> enable_required/disable_required to keep it consistent with the
>> wording atomic helpers use? DC also seems to use reset for a lot of
>> other things already (state reset, like atomic, or gpu reset like
>> drm/scheduler's td_r_), so I think this would also help clarity from a
>> DC perspective.
>>
>> Patch itself is good, above just an idea for another patch on top.
>>
>> Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

Thanks for the reviews! I assume this will get picked up by a DC
developer or Alex/Christian.


> That sounds like a reasonable idea to me. These are used more as a
> stream_changed / stream_removed flag, but I don't think these helpers
> really need to exist at all.
> 
> That could come as a follow up patch.

Yeah, I'm leaving that to you guys. :)


-- 
Earthling Michel Dänzer               |               https://redhat.com
Libre software enthusiast             |             Mesa and X developer


More information about the dri-devel mailing list