[PATCH v3 10/20] drm: omapdrm: Only commit planes on active CRTCs

[Tomi Valkeinen]

On 13/12/16 00:53, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Tuesday 20 Sep 2016 16:51:11 Tomi Valkeinen wrote:
>> On 19/09/16 15:27, Laurent Pinchart wrote:
>>> The DRM core supports skipping plane update for inactive CRTCs for
>>> hardware that don't need it or can't cope with it. That's our case, so
>>> use the DRM core infrastructure instead of reinventing it.
>>
>> I don't follow this desc. What is omapdrm reinventing? At least this
>> patch does not remove any of the "reinvention".
> 
> There used to be one, but it got removed when I rebased the patch series. I'll 
> reword the commit message.
> 
>> What does DRM_PLANE_COMMIT_ACTIVE_ONLY do? Skips plane HW configuration
>> for planes on crtcs that are disabled? The plane HW config will still be
>> done when the crtc is about to be enabled, right?
> 
> It skips plane update (atomic_begin, atomic_disable, atomic_flush) for 
> disabled CRTCs. The CRTC .begin() operation is still called for those CRTCs, 
> only plane update is skipped.

I didn't catch that. Skips atomic_begin, but .begin is still called?

> Now that I wrote that, I'm not quite sure this change is right. It looks like 
> disabling a plane is shadowed, and without an atomic_flush call the GO bit 
> will never be set. However, the problem predates this patch, as the GO bit 
> will only be set if dispc_mgr_is_enabled() returns true, which shouldn't be 
> the case for disabled CRTCs.
> 
> How is this supposed to work, how is plane disable supposed to be synchronized 
> at the hardware level ?

If a crtc is currently disabled (in the HW), all the new settings
(including plane enable/disable bit) can be programmed and when the crtc
is enabled, those settings are taken into use. GO bit is not necessary.

When the crtc is active, you need to set the GO bit to get the new
settings into use at next vblank.

If a crtc is to be disabled, we can just disable the crtc, GO bit is not
necessary.

 Tomi

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161213/edf409e9/attachment.sig>