| Message ID | 20240415081423.495834-5-balasubramani.vivekanandan@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Enable display support for Battlemage | expand |
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. Matt > 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 >
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 >> >
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; }
Framebuffer format modifiers are used to indicate the existence of auxillary surface in the plane, containing the CCS data. But on 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(-)