[PATCH 10/23] drm: omapdrm: Use atomic state instead of local device state

[Laurent Pinchart]

Hi Tomi,

On Monday 06 Jun 2016 13:37:13 Tomi Valkeinen wrote:
> On 06/06/16 04:14, Laurent Pinchart wrote:
> >>> If the DRM core doesn't track whether a CRTC HW is enabled at the
> >>> moment, maybe omapdrm should? I guess the above works, but that if()
> >>> makes me a bit uneasy, as it's not really obvious, and the logic behind
> >>> it could possibly change later...
> >>> 
> >>> A "if (crtc->is_hw_enabled)" would be much more readable.
> > 
> > The whole point of this patch is to remove local state and rely on DRM
> > core state, so I'd like to avoid that if possible.
> 
> Yep, but if DRM core doesn't give that information...
> 
> Using drm_atomic_crtc_needs_modeset() to check if a crtc is enabled at a
> particular point in the commit sequence feels a bit risky to me.
> 
> You do explain it in the comment, so it's not that bad, but I'd still
> rather see an 'if (is-the-hw-enabled-or-not)' than looking at seemingly
> unrelated information, and deducing from that if the hw is enabled or not.

I agree, but I'd like that "is-the-hw-enabled-or-not" state to be provided by 
the DRM core, not the omapdrm driver. I'll see how we can get there, but I'd 
rather do that separately from this patch series as it will require very 
careful design and review.

-- 
Regards,

Laurent Pinchart