Re: [PATCH] drm: Don't force all planes to be added to the state due to zpos

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Oct 10, 2016 at 2:19 PM,  <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> We don't want all planes to be added to the state whenever a
> plane with fixed zpos gets enabled/disabled. This is true
> especially for eg. cursor planes on i915, as we want cursor
> updates to go through w/o throttling. Same holds for drivers
> that don't support zpos at all (i915 actually falls into this
> category right now since we've not yet added zpos support).
>
> Allow drivers more freedom by letting them deal with zpos
> themselves instead of doing it in drm_atomic_helper_check_planes()
> unconditionally. Easiest solution seems to be to move the call
> up to drm_atomic_helper_check(). But as some drivers might want
> to use that function without the zpos handling, let's provide
> two variants: the normal one, and one that deals with zpos.
>
> Cc: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> Cc: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxx>
> Cc: Vincent Abriou <vincent.abriou@xxxxxx>
> Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Cc: Inki Dae <inki.dae@xxxxxxxxxxx>
> Cc: Joonyoung Shim <jy0922.shim@xxxxxxxxxxx>
> Cc: Seung-Woo Kim <sw0312.kim@xxxxxxxxxxx>
> Cc: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> Cc: Lyude <cpaul@xxxxxxxxxx>
> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 44d1240d006c ("drm: add generic zpos property")
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>

Seems a bit fragile, and then drivers still need to not overshot when
they do zpos (which we want eventually in i915 too). I think the
proper way is to keep track of a per-plane zpos changed (or compute
that ad-hoc, we have both states). And only grab more planes if a zpos
value changed.

That would fix the issue at the source, also work for us in the
future, and it should be contained to just the helper function itself.
Win all around ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]