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

From: Tomi Valkeinen <tomi.valkeinen@ti.com>
To: "Ilia Mirkin" <imirkin@alum.mit.edu>,
	"Laurent Pinchart" <laurent.pinchart@ideasonboard.com>,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	"Daniel Vetter" <daniel@ffwll.ch>
Cc: dri-devel <dri-devel@lists.freedesktop.org>, Jyri Sarha <jsarha@ti.com>
Subject: Re: [PATCH 4/7] drm/omap: Implement CTM property for CRTC using OVL managers CPR matrix
Date: Mon, 21 Sep 2020 14:08:48 +0300	[thread overview]
Message-ID: <5beadfb2-86a5-a782-ff88-ce77c727ecfe@ti.com> (raw)
In-Reply-To: <CAKb7Uvhcvi5AQZq21ky8V07wjeNfdpV0y+5j=O=S1PTp=4o5Bw@mail.gmail.com>

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)?

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel