Re: [v8 14/15] drm/i915/color: Add 3D LUT to color pipeline

["Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com>]

On 12/18/2025 9:45 PM, Simon Ser wrote:
> On Monday, December 15th, 2025 at 09:43, Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com> wrote:
> 
>> On 12/12/2025 11:55 PM, Simon Ser wrote:
>>
>>> On Friday, December 12th, 2025 at 18:47, Borah, Chaitanya Kumar chaitanya.kumar.borah@intel.com wrote:
>>>
>>>>>> +void intel_color_plane_commit_arm(struct intel_dsb *dsb,
>>>>>> + const struct intel_plane_state *plane_state)
>>>>>> +{
>>>>>> + struct intel_display *display = to_intel_display(plane_state);
>>>>>> + struct intel_crtc *crtc = to_intel_crtc(plane_state->uapi.crtc);
>>>>>> +
>>>>>> + if (crtc && intel_color_crtc_has_3dlut(display, crtc->pipe))
>>>>>> + glk_lut_3d_commit(dsb, crtc, !!plane_state->hw.lut_3d);
>>>>>> ^^^^^^^^^^^^
>>>>>
>>>>> And this looks like a pretty major fail. Why is the 3D LUT stored in
>>>>> the plane state when it's a pipe level thing?
>>>>
>>>> With DISPLAY_VER(display) >= 35, 3DLUT can be attached to a plane.
>>>>
>>>> (Bits[23:22] in 3DLUT_CTL). This is the only way we are exposing the HW
>>>> to the userspace right now (through the new plane color pipeline uapi).
>>>> Therefore, it lies in the plane state.
>>>>
>>>> However, there are (soonish)plans to adopt the color pipeline for crtcs
>>>> too. Once that happens, it needs to be handled a bit more carefully. A
>>>> potential approach is to allow userspace to program the block with a
>>>> first come first served semantics and fail the commit if it tries to set
>>>> 3DLUT both on plane and crtc in the same commit.
>>>
>>> The plane 3D LUT must only be used before blending. Any pipe-level
>>> post-blending 3D LUT hardware block is not suitable to implement plane
>>> colorops.
>>
>> Same 3D LUT block is shared across pipe and planes. When we do end up
>> implementing the pipe color pipeline we would like the 3DLUT exposed at
>> the pipe stage too.
> 
> Ah, I see! And there is a switch in the hw to configure whether it's
> applied pre- or post-blending?
> 

Yes, that's correct.

>> However, there is no good way to do it in the current color pipeline
>> UAPI (atleast that I know of). One suggestion from Harry (discussed in
>> the hackfest) was to list the pipelines in order of preference of the
>> driver.
>>
>> Considering we prefer the pre-blend 3DLUT over a post blend one, it
>> would mean that we don't expose the 3DLUT on the first pipeline on the
>> crtc but do it in the second one. (I am not sure how well it scales though)
>>
>> I have considered other solutions like introducing a new property say
>> "muxed" which could be used with Bypass to indicate if the current color
>> block is being used in another part of the pipeline.
> 
> I think two pipelines + making commits fail when they use conflicting
> colorops is the way to go.
> 
> If it turns out to be a generalized issue with more hardware and the
> above solution isn't enough for user-space, we can think of introducing
> a way to describe the limitation.

IIRC, AMD also had similar HW semantics with some block. But a problem 
for another day, I guess.

==
Chaitanya