[PATCH] drm/xe/display: Disable aux ccs framebuffers

Juha-Pekka Heikkila juhapekka.heikkila at gmail.com
Tue Jan 23 18:33:16 UTC 2024


On 23.1.2024 19.57, Jani Nikula wrote:
> On Tue, 23 Jan 2024, Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com> wrote:
>> On 23.1.2024 12.49, Jani Nikula wrote:
>>> On Tue, 02 Jan 2024, Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com> wrote:
>>>> Aux ccs framebuffers don't work on Xe driver hence disable them
>>>> from plane capabilities until they are fixed. Flat ccs framebuffers
>>>> work and they are left enabled. Here is separated plane capabilities
>>>> check on i915 so it can behave differencly depending on the driver.
>>>
>>> I still think there's too much going on in this one patch. It refactors
>>> i915 and modifies xe behaviour in one go.
>>>
>>> It adds intel_plane_caps.[ch] in i915, but extracts them from skl+
>>> specific functions and files. xe uses the .h but adds the code in
>>> existing xe_plane_initial.c. There's also intel_plane_initial.c i915
>>> side, but that's not where the functions get moved in i915 side.
>>
>> I was never against splitting it, I can do that.
>>
>>>
>>> I'm trying to look at the actual functional change, and I'm wondering if
>>> it isn't just this:
>>>
>>> index 511dc1544854..8bba6c2e5098 100644
>>> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
>>> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
>>> @@ -2290,6 +2290,9 @@ static u8 skl_get_plane_caps(struct drm_i915_private *i915,
>>>    	if (HAS_4TILE(i915))
>>>    		caps |= INTEL_PLANE_CAP_TILING_4;
>>>    
>>> +	if (!IS_ENABLED(I915) && !HAS_FLAT_CCS(i915))
>>> +		return caps;
>>> +
>>>    	if (skl_plane_has_rc_ccs(i915, pipe, plane_id)) {
>>>    		caps |= INTEL_PLANE_CAP_CCS_RC;
>>>    		if (DISPLAY_VER(i915) >= 12)
>>>
>>> I'm not saying that's exactly pretty, either, but IIUC this is supposed
>>> to be a temporary measure ("until they are fixed"), I'd rather take this
>>> small thing instead.
>>>
>>
>> Would that work when both i915 and Xe are being built?
> 
> IS_ENABLED(I915) will be false for xe build, true for i915 build. And
> HAS_FLAT_CCS() is defined for both, in different ways.
> 
> It's essentially the same as #ifdef but much less of an eyesore.
> 

ack. I'll put that into a patch that will replace this patch.

/Juha-Pekka



More information about the Intel-gfx mailing list