[Intel-gfx] [PATCH] drm/i915: Tile F plane format support

[Matt Roper]

On Mon, Sep 27, 2021 at 09:29:07PM +0300, Ville Syrjälä wrote:
> On Mon, Sep 27, 2021 at 11:23:35AM -0700, Matt Roper wrote:
> > On Thu, Sep 23, 2021 at 06:49:59PM +0300, Ville Syrjälä wrote:
> > > On Thu, Sep 23, 2021 at 11:48:58AM +0300, Stanislav Lisovskiy wrote:
> > > > TileF(Tile4 in bspec) format is 4K tile organized into
> > > > 64B subtiles with same basic shape as for legacy TileY
> > > > which will be supported by Display13.
> > > 
> > > Why we still haven't done the F->tile64 rename?
> > >
> > > This is the last chance to fix this before we bake 
> > > this into the uapi and are stuck with a name that doesn't
> > > match the spec and will just confuse everyone.
> > 
> > I think you're confusing the formats here.  The bspec uses both terms
> > "TileF" and "Tile4" for the same format in different places.  There's a
> > completely different format that's referred to as both "TileS" and
> > "Tile64" in the bspec that we don't use at the moment.  So tile64
> > wouldn't be a correct rename, but tile4 could be.
> 
> Right, tile64 is the macro tile variant I think. So like Ys
> which we never bothered implementing, so I guess we''l not bother
> with tile64 either.
> 
> > 
> > In general Tile4 is much more common in the bspec than TileF is (TileF
> > terminology is mostly found in the media sections).  And bspec 44917 is
> > the most authoritative bspec page on the subject, and it refers to it as
> > Tile4, so I agree that switching over "Tile4" would probably be a good
> > move.
> > 
> > > 
> > > > 
> > ...
> > > > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> > > > index bde5860b3686..d7dc421c6134 100644
> > > > --- a/include/uapi/drm/i915_drm.h
> > > > +++ b/include/uapi/drm/i915_drm.h
> > > > @@ -1522,7 +1522,8 @@ struct drm_i915_gem_caching {
> > > >  #define I915_TILING_NONE	0
> > > >  #define I915_TILING_X		1
> > > >  #define I915_TILING_Y		2
> > > > -#define I915_TILING_LAST	I915_TILING_Y
> > > > +#define I915_TILING_F		3
> > > > +#define I915_TILING_LAST	I915_TILING_F
> > > 
> > > fences...
> > 
> > Recognizing TileF/Tile4 separately from TileY is important to code
> > outside of display as well.  There are blitter instructions that require
> > different settings for TileY vs Tile4/F so if we drop the tracking of
> > this as a unique tiling type, it will break the blitting/copying and
> > some of the upcoming local memory support for Xe_HP-based platforms.
> 
> These are uapi definitions for set_tiling(). You are not meant to add
> anything there. Just like we didn't add anything for Yf.

Yeah, I think that's the real problem --- we define some values here in
the uapi header, but we also wind up using the same set of values for
driver-internal non-uapi purposes too rather than having a separate enum
(containing a superset of the uapi values) that can be used for those
other things.  Display code can use FB modifiers for some things, but
core/lmem code needs a way to refer to Tile4 and such and doesn't have a
good way to do that today.

I think most (all?) of the non-display code that's relying on a
definition of I915_TILING_F is in various selftests that are still being
prepared for upstreaming, so maybe there's a better way to handle the
selection of possible formats specifically in the selftest code itself.
That's really the only area of the kernel code that should need to be
aware of the specific internal layout of various buffers.


Matt

> 
> -- 
> Ville Syrjälä
> Intel

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795