Re: [PATCH] drm/i915/display: Disable AuxCCS framebuffers if built for Xe - Juha-Pekka Heikkila

From: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
To: "Souza, Jose" <jose.souza@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>
Cc: "jani.nikula@linux.intel.com" <jani.nikula@linux.intel.com>
Subject: Re: [PATCH] drm/i915/display: Disable AuxCCS framebuffers if built for Xe
Date: Mon, 5 Feb 2024 12:00:15 +0200	[thread overview]
Message-ID: <620670df-2f56-416f-9197-798e99d595cb@gmail.com> (raw)
In-Reply-To: <17a867f9176dac4c29cbc4c78785884fed5199ac.camel@intel.com>

On 2.2.2024 22.51, Souza, Jose wrote:
> On Thu, 2024-01-25 at 08:07 -0800, José Roberto de Souza wrote:
>> On Thu, 2024-01-25 at 17:56 +0200, Juha-Pekka Heikkila wrote:
>>> On 25.1.2024 17.28, Souza, Jose wrote:
>>>> On Thu, 2024-01-25 at 17:25 +0200, Juha-Pekka Heikkila wrote:
>>>>> AuxCCS framebuffers don't work on Xe driver hence disable them
>>>>> from plane capabilities until they are fixed. FlatCCS framebuffers
>>>>> work and they are left enabled. CCS is left untouched for i915
>>>>> deriver.
>>>>>
>>>>> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/933
>>>>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>>>>> ---
>>>>>    drivers/gpu/drm/i915/display/skl_universal_plane.c | 8 ++++++++
>>>>>    1 file changed, 8 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
>>>>> index 511dc1544854..1521d829525a 100644
>>>>> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
>>>>> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
>>>>> @@ -2290,6 +2290,14 @@ static u8 skl_get_plane_caps(struct drm_i915_private *i915,
>>>>>    	if (HAS_4TILE(i915))
>>>>>    		caps |= INTEL_PLANE_CAP_TILING_4;
>>>>>    
>>>>> +	/*
>>>>> +	 * FIXME: Below if(IS_ENABLED(CONFIG_I915)..) is because Xe driver
>>>>> +	 * can't use AuxCCS framebuffers. Once they are fixed this need to be
>>>>> +	 * removed.
>>>>> +	 */
>>>>> +	if (!IS_ENABLED(CONFIG_I915) && !HAS_FLAT_CCS(i915))
>>>>> +		return caps;
>>>>> +
>>>>
>>>> functional but looks odd.
>>>> would rather add a check inside of gen12_plane_has_mc_ccs() or 'if ((!IS_ENABLED(CONFIG_I915) && !HAS_FLAT_CCS(i915)) && gen12_plane_has_mc_ccs(i915,
>>>> plane_id))'
>>>
>>> Hi Jose,
>>>
>>> not sure I understood your idea. Here need to be disabled all versions
>>> of aux ccs for Xe, not just mc ccs.
>>
>> ah misplace this this...please ignore.
>>
>> this is fine for something that will be fixed soon.
>>
>> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> 
> ping!
> something is holding from being merged?

Patch results showed things didn't go as Jani had suggested. This would 
just break ccs for i915. I'll need to rewrite my original patch 
splitting it into smaller parts and see if I can take into account 
Ville's comment there.

/Juha-Pekka