Re: [PATCH v3 04/21] drm/i915/xe2hpd: Skip CCS modifiers

[Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>]

On 15.4.2024 19.06, Matt Roper wrote:
> On Mon, Apr 15, 2024 at 01:44:06PM +0530, Balasubramani Vivekanandan wrote:
>> Framebuffer format modifiers are used to indicate the existence of
>> auxillary surface in the plane, containing the CCS data.  But on
> 
> s/auxillary/auxiliary/ in a few places in this commit message.  Although
> I don't think this statement is 100% true.  DG2 use FlatCCS rather than
> AuxCCS, but still needs to use framebuffer modifiers because the region
> of the FlatCCS that corresponds to the buffer may not be
> initialized/correct if the buffer contents were generated in a
> non-compressed manner.  We have to use framebuffer modifiers to pass
> information through the software stack as to whether the FlatCCS data
> for the buffer is usable and should be consulted by consumers of the
> buffer.
> 
> As I understand it, the big change in Xe2, is that compression is now
> controlled by the PAT setting in the PTEs and even in cases where an
> "uncompressed" PAT index is used to generate content in the buffers, the
> corresponding FlatCCS area still gets initialized to whatever metadata
> code corresponds to "this bloc is uncompressed."  So that means that
> it's always safe for consumers like display to treat the buffer as if it
> were compressed (e.g., setting the decompression flag in PLANE_CTL) ---
> the CCS metadata for ever single block in the buffer will properly
> indicate that no compression is actually present.

Adding to what Matt commented above, issue which is being fixed here 
should already be taken care by

--
commit cf48bddd31deefb9ab07de9a4d0150da6610198a
Author: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Date:   Wed Feb 28 16:02:25 2024 +0200

     drm/i915/display: Disable AuxCCS framebuffers if built for Xe
--

/Juha-Pekka


>> Xe2_HPD, the CCS data is stored in a fixed reserved memory area and not
>> part of the plane. It contains no auxillary surface.
>> Also in Xe2, the compression is configured via PAT settings in the
>> pagetable mappings. Decompression is enabled by default in the
>> PLANE_CTL. Based on whether valid CCS data exists for the plane, display
>> hardware decides whether compression is necessary or not.
>> So there is no need for format modifiers to indicate if compression is
>> enabled or not.
>>
>> v2:
>> * Improved the commit description with more details
>> * Removed the redundant display IP version check for 20. Display version
>>    check for each modifier above would take care of it.
>>
>> CC: Juha-Pekka Heikkilä <juha-pekka.heikkila@intel.com>
>> CC: Matt Roper <matthew.d.roper@intel.com>
>> Signed-off-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_fb.c | 16 +++++++++++++---
>>   1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c
>> index 86b443433e8b..7234ce36b6a4 100644
>> --- a/drivers/gpu/drm/i915/display/intel_fb.c
>> +++ b/drivers/gpu/drm/i915/display/intel_fb.c
>> @@ -431,9 +431,19 @@ static bool plane_has_modifier(struct drm_i915_private *i915,
>>   	 * Separate AuxCCS and Flat CCS modifiers to be run only on platforms
>>   	 * where supported.
>>   	 */
>> -	if (intel_fb_is_ccs_modifier(md->modifier) &&
>> -	    HAS_FLAT_CCS(i915) != !md->ccs.packed_aux_planes)
>> -		return false;
>> +	if (intel_fb_is_ccs_modifier(md->modifier)) {
>> +
>> +		/*
>> +		 * There is no need for CCS format modifiers for Xe2_HPD, as
>> +		 * there is no support of AuxCCS and the FlatCCS is configured
>> +		 * usign PAT index in the page table mappings
>> +		 */
>> +		if (DISPLAY_VER_FULL(i915) == IP_VER(14, 1))
>> +			return false;
>> +
>> +		if (HAS_FLAT_CCS(i915) != !md->ccs.packed_aux_planes)
>> +			return false;
>> +	}
>>   
>>   	return true;
>>   }
>> -- 
>> 2.25.1
>>
>