Re: [PATCH 4/7] drm/omap: Implement CTM property for CRTC using OVL managers CPR matrix

[Pekka Paalanen <ppaalanen@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3524 bytes --]

On Mon, 21 Sep 2020 14:08:48 +0300
Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

> Hi,
> 
> On 04/09/2019 23:20, Ilia Mirkin wrote:
> 
> >>>>>> Implement CTM color management property for OMAP CRTC using DSS
> >>>>>> overlay manager's Color Phase Rotation matrix. The CPR matrix does not
> >>>>>> exactly match the CTM property documentation. On DSS the CPR matrix is
> >>>>>> applied after gamma table look up. However, it seems stupid to add a
> >>>>>> custom property just for that.  
> >>>>>
> >>>>> In that case the DRM documentation should be updated to mention that
> >>>>> both options are allowed.  
> >>>>
> >>>> Ok, if that is alright. But if we do that, then I guess all the drivers
> >>>> implementing CTM should document the point where it is applied in the
> >>>> pipeline.  
> >>>
> >>> Whatever solution we end up picking, I think it should at least be
> >>> discussed with a broader upstream audience and not just swept under the
> >>> omapdrm carpet :-)
> >>>  
> >>
> >> I'll try to write something and send the next series to wider audience.
> >> Let's see what jury says.  
> > 
> > In case it's useful ... the pipeline normally goes degamma -> ctm ->
> > gamma. If your ctm is applied after gamma, perhaps you can just rename
> > "gamma" to "degamma" and be done? (There's the unfortunate case of
> > legacy gamma which does end up in "GAMMA" when using atomic helpers.
> > But in such a case, you won't have a CTM.)  
> 
> Waking up old thread, as I started looking at these patches again. So the problem was that DRM
> defines the output color transformations as:
> 
> degamma -> ctm -> gamma -> out
> 
> whereas OMAP DSS has
> 
> gamma -> ctm -> out
> 
> The omapdrm driver could declare the gamma table as degamma, as suggested by Ilia, and for the
> legacy drmModeCrtcSetGamma, the omapdrm driver could implement its own version, and use the degamma
> table internally (with no ctm).
> 
> For legacy, that would work fine and as expected, but I think the atomic version would be a bit odd,
> with only degamma, and no gamma.
> 
> Quick grep for drm_crtc_enable_color_mgmt shows that there are other drivers that have CTM and
> gamma, but no degamma. I wonder if all those have ctm -> gamma -> out, or are they similar to OMAP DSS.
> 
> Any thoughts on how to proceed with this?
> 
> Should we have a property that describes the order? Or new property name for gamma before ctm
> (PREGAMMA)? Or just have it as degamma, and let the userspace deal with it (i.e. figure out there's
> no gamma, but there's degamma, so use degamma)?

Hi,

would it not be simplest if KMS UAPI specification defined the abstract
color pipeline with all the blocks that may be exposed with
driver-agnostic UAPI, and then just say that if a block is not present,
it behaves as pass-through, a no-op?

Each block would be represented by standardised KMS properties, that
either exist or don't.

I think that would be fairly easy for userspace to grasp, but I don't
know if the abstract model itself would be feasible considering all the
hardware out there.

If we happened to be limited to

FB -> plane-degamma -> plane-CTM -> plane-gamma -> (blending) ->
degamma -> CTM -> gamma -> encoder -> wire

it would still be tractable.

No funny business with new KMS properties changing how old KMS
properties behave. Old userspace understands and uses old KMS
properties but not new KMS properties, so it wouldn't even work.


Thanks,
pq

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel