On Thu, Sep 4, 2025 at 7:56 PM Matt Atwood <matthew.s.atw...@intel.com> wrote:
>
> On Thu, Sep 04, 2025 at 01:53:01PM -0300, Gustavo Sousa wrote:
> > Quoting Matt Atwood (2025-09-03 14:08:21-03:00)
> > >The checks in plane_has_modifier() should check against display version
> > >instead of graphics version.
> > >
> > >Bspec: 67165, 70815
> > >
> > >Signed-off-by: Matt Atwood <matthew.s.atw...@intel.com>
> > >---
> > > drivers/gpu/drm/i915/display/intel_fb.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > >diff --git a/drivers/gpu/drm/i915/display/intel_fb.c
> > >b/drivers/gpu/drm/i915/display/intel_fb.c
> > >index b210c3250501..1e4cee857d7b 100644
> > >--- a/drivers/gpu/drm/i915/display/intel_fb.c
> > >+++ b/drivers/gpu/drm/i915/display/intel_fb.c
> > >@@ -563,11 +563,11 @@ static bool plane_has_modifier(struct intel_display
> > >*display,
> > > return false;
> > >
> > > if (md->modifier == I915_FORMAT_MOD_4_TILED_BMG_CCS &&
> > >- (GRAPHICS_VER(i915) < 20 || !display->platform.dgfx))
> > >+ (DISPLAY_VER(display) < 14 || !display->platform.dgfx))
> > > return false;
> > >
> > > if (md->modifier == I915_FORMAT_MOD_4_TILED_LNL_CCS &&
> > >- (GRAPHICS_VER(i915) < 20 || display->platform.dgfx))
> > >+ (DISPLAY_VER(display) < 20 || display->platform.dgfx))
> > > return false;
> >
> > Hm... Maybe using GRAPHICS_VER() here was intentional? The checks on
> > display version are already covered by the entries in intel_modifiers.
> >
> > If we look at commit fca0abb23447 ("drm/i915/display: allow creation of
> > Xe2 ccs framebuffers"), we'll see that the respective entries were added
> > to intel_modifiers *and* the checks on GRAPHICS_VER() were added as
> > well. Perhaps there are indeed restrictions on the graphics side to be
> > able to use the format?
> >
> > --
> > Gustavo Sousa
> Honestly, I tried looking for reasons. I couldn't find anything in the
> documentation to support. I decided to send upstream to see if it broke
> stuff to not do the checks that way. CI seems very clean. Hoping Jani or
> Juha-Pekka will chime in if it is indeed an issue.
Using GRAPHICS_VER here was intentional. Jani didn't like it but these
xe2 ccs don't follow display versioning but gt versioning.
Proposed change look ok but I'll need to dig in to documentation
before I can say for sure. I remember we had discussion about this
with Jani but can't remember what convinced Jani I needed to use
GRAPHICS_VER at that time.
/Juha-Pekka
> >
> > >
> > > return true;
> > >--
> > >2.50.1
> > >
