[Intel-gfx] [PATCH v6 1/3] drm: Add CRTC background color property (v5)
maarten.lankhorst at linux.intel.com
Thu Apr 25 10:45:33 UTC 2019
Op 26-02-2019 om 17:17 schreef Matt Roper:
> On Tue, Feb 26, 2019 at 08:26:36AM +0100, Maarten Lankhorst wrote:
>> Op 21-02-2019 om 01:28 schreef Matt Roper:
>>> Some display controllers can be programmed to present non-black colors
>>> for pixels not covered by any plane (or pixels covered by the
>>> transparent regions of higher planes). Compositors that want a UI with
>>> a solid color background can potentially save memory bandwidth by
>>> setting the CRTC background property and using smaller planes to display
>>> the rest of the content.
>>> To avoid confusion between different ways of encoding RGB data, we
>>> define a standard 64-bit format that should be used for this property's
>>> value. Helper functions and macros are provided to generate and dissect
>>> values in this standard format with varying component precision values.
>>> - Swap internal representation's blue and red bits to make it easier
>>> to read if printed out. (Ville)
>>> - Document bgcolor property in drm_blend.c. (Sean Paul)
>>> - s/background_color/bgcolor/ for consistency between property name and
>>> value storage field. (Sean Paul)
>>> - Add a convenience function to attach property to a given crtc.
>>> - Restructure ARGB component extraction macros to be easier to
>>> understand and enclose the parameters in () to avoid calculations
>>> if expressions are passed. (Sean Paul)
>>> - s/rgba/argb/ in helper function/macro names. Even though the idea is
>>> to not worry about the internal representation of the u64, it can
>>> still be confusing to look at code that uses 'rgba' terminology, but
>>> stores values with argb ordering. (Ville)
>>> - Drop the bgcolor_changed flag. (Ville, Brian Starkey)
>>> - Clarify in kerneldoc that background color is expected to undergo the
>>> same pipe-level degamma/csc/gamma transformations that planes do.
>>> (Brian Starkey)
>>> - Update kerneldoc to indicate non-opaque colors are allowed, but are
>>> generally only useful in special cases such as when writeback
>>> connectors are used (Brian Starkey / Eric Anholt)
>>> - Set crtc->state->bgcolor to solid black inside
>>> drm_crtc_add_bgcolor_property() in case drivers don't do this
>>> themselves. (Ville)
>>> - Add kerneldoc to drm_crtc_add_bgcolor_property() function.
>>> Cc: dri-devel at lists.freedesktop.org
>>> Cc: wei.c.li at intel.com
>>> Cc: harish.krupo.kps at intel.com
>>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>> Cc: Sean Paul <sean at poorly.run>
>>> Cc: Brian Starkey <Brian.Starkey at arm.com>
>>> Cc: Eric Anholt <eric at anholt.net>
>>> Cc: Stéphane Marchesin <marcheu at chromium.org>
>>> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
>>> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
>>> Reviewed-by(v2): Sean Paul <sean at poorly.run>
>>> Reviewed-by: Brian Starkey <brian.starkey at arm.com>
>> I like how bgcolor is a u64 now, but there is an issue with setting
>> crtc->state->bgcolor in attaching the property.
>> We should do it in atomic core init instead, like we already do for
>> connector/plane properties..
>> See for example https://patchwork.freedesktop.org/series/52363/
>> This was specificallly for the background color proposal, but we
>> probalby have to split out the non-trivial conversions of
> Makes sense. What's the status of that patch? It looks like it has a
> lot of a-b's/r-b's but hasn't landed yet. Is there anything blocking
> it? If you're still driving it forward, I can wait for that to land and
> then build on top of it.
I've split out the patch and landed some of the preparation patches, it should be good enough to unblock this patch series at least now. :)
More information about the Intel-gfx