Re: [PATCH 1/3] drm/i915/display: Copy color pipeline from plane in the primary joiner pipe

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

On 4/1/2026 5:10 PM, Ville Syrjälä wrote:
> On Wed, Apr 01, 2026 at 02:08:39PM +0530, Chaitanya Kumar Borah wrote:
>> When copying plane color state in a joiner configuration, use the plane in
>> the primary joiner pipe since it carries the pipeline number selected by
>> the user-space.
>>
>> This assumes that all pipes in the joiner are symmetric in their plane
>> color capabilities.
>>
>> Cc: stable@vger.kernel.org # v6.19+
>> Fixes: a78f1b6baf4d ("drm/i915/color: Add framework to program CSC")
>> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_plane.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_plane.c b/drivers/gpu/drm/i915/display/intel_plane.c
>> index 5390ceb21ca4..82f445c83158 100644
>> --- a/drivers/gpu/drm/i915/display/intel_plane.c
>> +++ b/drivers/gpu/drm/i915/display/intel_plane.c
>> @@ -373,7 +373,7 @@ intel_plane_color_copy_uapi_to_hw_state(struct intel_plane_state *plane_state,
>>   	bool changed = false;
>>   	int i = 0;
>>   
>> -	iter_colorop = plane_state->uapi.color_pipeline;
>> +	iter_colorop = from_plane_state->uapi.color_pipeline;
>>   
>>   	while (iter_colorop) {
>>   		for_each_new_colorop_in_state(state, colorop, new_colorop_state, i) {
> 
> Hmm. This whole colorop thing seems a bit weird. So each plane/crtc/etc
> doesn't actually have its full state in its state, but rather it points
> to some other colorop state somewhere?
> 

Yes, colorops are unique in the DRM model. While they are DRM objects 
with their own states, they contain information about a plane/crtc's 
state. The plane/crtc property "COLOR PIPELINE" determines which chain 
of colorops is currently active.


> The mess here with the 'intel_atomic_state' here needs to get cleaned up.
> At the very least we need to pass the full atomic state from the caller
> instead of digging it out via the plane_state->uapi.state footgun.

To make the dependency on the intel_atomic_state explicit?
I will make the change.

> That thing should never be used, and ideally we'd just nuke it entirely.
> 

You mean the back pointer to the atomic state in the plane state?