[PATCH v17 1/1] drm/i915: Allow user to set cache at BO creation

[Zhang, Carl]

> -----Original Message-----
> From: Andi Shyti <andi.shyti at linux.intel.com>
> Sent: Wednesday, June 7, 2023 1:11 PM
> To: Zhang, Carl <carl.zhang at intel.com>
> Cc: Andi Shyti <andi.shyti at linux.intel.com>; Joonas Lahtinen
> <joonas.lahtinen at linux.intel.com>; Tvrtko Ursulin
> <tvrtko.ursulin at linux.intel.com>; Yang, Fei <fei.yang at intel.com>; Chris
> Wilson <chris at chris-wilson.co.uk>; Roper, Matthew D
> <matthew.d.roper at intel.com>; Justen, Jordan L <jordan.l.justen at intel.com>;
> Gu, Lihao <lihao.gu at intel.com>; Intel GFX <intel-gfx at lists.freedesktop.org>;
> DRI Devel <dri-devel at lists.freedesktop.org>
> Subject: Re: [PATCH v17 1/1] drm/i915: Allow user to set cache at BO creation
> 
> Hi Carl,
> 
> On Wed, Jun 07, 2023 at 03:40:20AM +0000, Zhang, Carl wrote:
> > Media driver reverted previous patches, and file a new  PR
> > https://github.com/intel/media-driver/pull/1680
> > will hold this PR until the uapi changes appear in drm_next.
> 
> That's great, thanks a lot for the quick actions here.
> 
> Before pushing I am going to replace the Media part in the commit log with
> the following sentence:
> 
> "
> The media driver supprt has bin submitted in this merge request:
> https://github.com/intel/media-driver/pull/1680
> "
> 
> > besides this, ask a dumb question.
> > How we retrieve the pat_index from a shared resource though dma_buf fd?
> > maybe we need to know whether it could be CPU cached if we want map it.
> > Of course, looks there are no real usage to access it though CPU.
> > Just use it directly without any pat related options ?
> 
> I am not understanding. Do you want to ask the PAT table to the driver? Are
> you referring to the CPU PAT index?
> 
> In any case, if I understood correctly, you don't necessarily always need to
> set the PAT options and the cache options will fall into the default values.
> 
> Please let me know if I haven't answered the question.
> 

If mesa create a resource , then use DRM_IOCTL_PRIME_HANDLE_TO_FD convert it to a dma fd. 
Then share it to media, media use DRM_IOCTL_PRIME_FD_TO_HANDLE convert it to a gem bo. 
But media does not know the PAT index , because mesa create it and set it. 
So, if media want to call DRM_IOCTL_I915_GEM_MMAP_OFFSET, media does not know whether it could be WB.

> Andi
> 
> > Thanks
> > Carl
> >
> > > -----Original Message-----
> > > From: Andi Shyti <andi.shyti at linux.intel.com>
> > > Sent: Tuesday, June 6, 2023 7:15 PM
> > > To: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > > Cc: Andi Shyti <andi.shyti at linux.intel.com>; Tvrtko Ursulin
> > > <tvrtko.ursulin at linux.intel.com>; Yang, Fei <fei.yang at intel.com>;
> > > Chris Wilson <chris at chris-wilson.co.uk>; Roper, Matthew D
> > > <matthew.d.roper at intel.com>; Justen, Jordan L
> > > <jordan.l.justen at intel.com>; Zhang, Carl <carl.zhang at intel.com>; Gu,
> > > Lihao <lihao.gu at intel.com>; Intel GFX
> > > <intel-gfx at lists.freedesktop.org>; DRI Devel <dri-
> > > devel at lists.freedesktop.org>
> > > Subject: Re: [PATCH v17 1/1] drm/i915: Allow user to set cache at BO
> > > creation
> > >
> > > > > > > To comply with the design that buffer objects shall have
> > > > > > > immutable cache setting through out their life cycle, {set,
> > > > > > > get}_caching ioctl's are no longer supported from MTL onward.
> > > > > > > With that change caching policy can only be set at object
> > > > > > > creation time. The current code applies a default (platform
> > > > > > > dependent)
> > > cache setting for all objects.
> > > > > > > However this is not optimal for performance tuning. The
> > > > > > > patch extends the existing gem_create uAPI to let user set
> > > > > > > PAT index for the object at creation time.
> > > > > > > The new extension is platform independent, so UMD's can
> > > > > > > switch to using this extension for older platforms as well,
> > > > > > > while {set, get}_caching are still supported on these legacy
> > > > > > > paltforms for
> > > compatibility reason.
> > > > > > > However, since PAT index was not clearly defined for
> > > > > > > platforms prior to
> > > > > > > GEN12 (TGL), so we are limiting this externsion to GEN12+
> > > > > > > platforms only. See ext_set_pat() in for the implementation details.
> > > > > > >
> > > > > > > Note: The documentation related to the PAT/MOCS tables is
> > > > > > > currently available for Tiger Lake here:
> > > > > > > https://www.intel.com/content/www/us/en/docs/graphics-for-li
> > > > > > > nux/ developer-reference/1-0/tiger-lake.html
> > > > > > >
> > > > > > > BSpec: 45101
> > > > > > >
> > > > > > > Mesa support has been submitted in this merge request:
> > > > > > > https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22
> > > > > > > 878
> > > > > > >
> > > > > > > The media driver is supported by the following commits:
> > > > > > > https://github.com/intel/media-
> > > driver/commit/92c00a857433ebb34ec
> > > > > > > 575e9834f473c6fcb6341
> > > > > > > https://github.com/intel/media-driver/commit/fd375cf2c5e1f6b
> > > > > > > f6b4
> > > > > > > 3258ff797b3134aadc9fd
> > > > > > > https://github.com/intel/media-
> > > driver/commit/08dd244b22484770a33
> > > > > > > 464c2c8ae85430e548000
> > > >
> > > > Andi, let's still get these corrected before merging once the
> > > > media-driver revert is completed.
> > >
> > > Sure!
> > >
> > > At least this doesn't need a new version to be respinned.
> > >
> > > Please, Carl, link the new pull request and I will update the commit log.
> > >
> > > Andi