Re: [PATCH 1/3] drm/i915: Remove early/pre-production Haswell code

[Zanoni, Paulo R]

On Fri, 2023-10-06 at 09:31 +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin 
> 
> It is not our policy to keep pre-production hardware support for this long
> so I guess this one was just forgotten.

Wouldn't it make sense to also remove the PCI IDs if they never made it
to the real production world*? Couldn't these IDs end up getting reused
for something else (maybe not even graphics) at some point in the
future?

*: I can't confirm this is the case.

> 
> Signed-off-by: Tvrtko Ursulin 
> ---
>  drivers/gpu/drm/i915/i915_driver.c | 1 -
>  drivers/gpu/drm/i915/i915_drv.h| 2 --
>  2 files changed, 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_driver.c 
> b/drivers/gpu/drm/i915/i915_driver.c
> index ccbb2834cde0..78a42c8a8509 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -175,7 +175,6 @@ static void intel_detect_preproduction_hw(struct 
> drm_i915_private *dev_priv)
>  {
>   bool pre = false;
>  
> - pre |= IS_HASWELL_EARLY_SDV(dev_priv);
>   pre |= IS_SKYLAKE(dev_priv) && INTEL_REVID(dev_priv) < 0x6;
>   pre |= IS_BROXTON(dev_priv) && INTEL_REVID(dev_priv) < 0xA;
>   pre |= IS_KABYLAKE(dev_priv) && INTEL_REVID(dev_priv) < 0x1;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index cb60fc9cf873..9d493ff1685a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -590,8 +590,6 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>   IS_SUBPLATFORM(i915, INTEL_ALDERLAKE_P, INTEL_SUBPLATFORM_RPL)
>  #define IS_RAPTORLAKE_U(i915) \
>   IS_SUBPLATFORM(i915, INTEL_ALDERLAKE_P, INTEL_SUBPLATFORM_RPLU)
> -#define IS_HASWELL_EARLY_SDV(i915) (IS_HASWELL(i915) && \
> - (INTEL_DEVID(i915) & 0xFF00) == 0x0C00)
>  #define IS_BROADWELL_ULT(i915) \
>   IS_SUBPLATFORM(i915, INTEL_BROADWELL, INTEL_SUBPLATFORM_ULT)
>  #define IS_BROADWELL_ULX(i915) \

Re: [PATCH v7] Documentation/gpu: Add a VM_BIND async document

[Zanoni, Paulo R]

On Mon, 2023-09-11 at 14:47 +0200, Thomas Hellström wrote:
> Add a motivation for and description of asynchronous VM_BIND operation
> 
> v2:
> - Fix typos (Nirmoy Das)
> - Improve the description of a memory fence (Oak Zeng)
> - Add a reference to the document in the Xe RFC.
> - Add pointers to sample uAPI suggestions
> v3:
> - Address review comments (Danilo Krummrich)
> - Formatting fixes
> v4:
> - Address typos (Francois Dugast)
> - Explain why in-fences are not allowed for VM_BIND operations for long-
>   running workloads (Matthew Brost)
> v5:
> - More typo- and style fixing
> - Further clarify the implications of disallowing in-fences for VM_BIND
>   operations for long-running workloads (Matthew Brost)
> v6:
> - Point out that a gpu_vm is a virtual GPU Address space.
>   (Danilo Krummrich)
> - For an explanation of dma-fences point to the dma-fence documentation.
>   (Paolo Zanoni)
> - Clarify that VM_BIND errors are reported synchronously. (Paulo Zanoni)
> - Use an rst doc reference when pointing to the async vm_bind document
>   from the xe merge plan.
> - Add the VM_BIND documentation to the drm documentation table-of-content,
>   using an intermediate "Misc DRM driver uAPI- and feature implementation
>   guidelines"
> v7:
> - Update the error handling documentation to remove the VM error state.
> 
> Cc: Paulo R Zanoni 

I was asked to give my input (ack or nack) here, since I'm working on
the Sparse implementation for Anv, which makes use of vm_bind (mesa MR
23045).

I understand that this text is mostly "implementation guidelines for
drivers". While I understand what's written in the text, I think it's
way too vague for me - wearing my user-space user-of-these-interfaces
hat - to make sense of:  I still don't know what exactly I'm supposed
to do with it, especially the error handling paths.

I was waiting to see if a proposal for xe.ko implementation would
appear before I could actually make full sense of this text and then
ack or nack anything. That's still my plan. More below.


> Signed-off-by: Thomas Hellström 
> Acked-by: Nirmoy Das 
> Reviewed-by: Danilo Krummrich 
> Reviewed-by: Matthew Brost 
> Reviewed-by: Rodrigo Vivi 
> ---
>  Documentation/gpu/drm-vm-bind-async.rst   | 155 ++
>  .../gpu/implementation_guidelines.rst |   9 +
>  Documentation/gpu/index.rst   |   1 +
>  Documentation/gpu/rfc/xe.rst  |   4 +-
>  4 files changed, 167 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/gpu/drm-vm-bind-async.rst
>  create mode 100644 Documentation/gpu/implementation_guidelines.rst
> 
> diff --git a/Documentation/gpu/drm-vm-bind-async.rst 
> b/Documentation/gpu/drm-vm-bind-async.rst
> new file mode 100644
> index ..f12f794408b9
> --- /dev/null
> +++ b/Documentation/gpu/drm-vm-bind-async.rst
> @@ -0,0 +1,155 @@
> +.. SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +
> +
> +Asynchronous VM_BIND
> +
> +
> +Nomenclature:
> +=
> +
> +* ``VRAM``: On-device memory. Sometimes referred to as device local memory.
> +
> +* ``gpu_vm``: A virtual GPU address space. Typically per process, but
> +  can be shared by multiple processes.
> +
> +* ``VM_BIND``: An operation or a list of operations to modify a gpu_vm using
> +  an IOCTL. The operations include mapping and unmapping system- or
> +  VRAM memory.
> +
> +* ``syncobj``: A container that abstracts synchronization objects. The
> +  synchronization objects can be either generic, like dma-fences or
> +  driver specific. A syncobj typically indicates the type of the
> +  underlying synchronization object.
> +
> +* ``in-syncobj``: Argument to a VM_BIND IOCTL, the VM_BIND operation waits
> +  for these before starting.
> +
> +* ``out-syncobj``: Argument to a VM_BIND_IOCTL, the VM_BIND operation
> +  signals these when the bind operation is complete.
> +
> +* ``dma-fence``: A cross-driver synchronization object. A basic
> +  understanding of dma-fences is required to digest this
> +  document. Please refer to the ``DMA Fences`` section of the
> +  :doc:`dma-buf doc `.
> +
> +* ``memory fence``: A synchronization object, different from a dma-fence.
> +  A memory fence uses the value of a specified memory location to determine
> +  signaled status. A memory fence can be awaited and signaled by both
> +  the GPU and CPU. Memory fences are sometimes referred to as
> +  user-fences, userspace-fences or gpu futexes and do not necessarily obey
> +  the dma-fence rule of signaling within a "reasonable amount of time".
> +  The kernel should thus avoid waiting for memory fences with locks held.
> +
> +* ``long-running workload``: A workload that may take more than the
> +  current stipulated dma-fence maximum signal delay to complete and
> +  which therefore needs to set the gpu_vm or the GPU execution context in
> +  a certain mode that disallows completion dma-fences.
> +
> +* ``exec function``: An exec function is a function that 

Re: [Intel-xe] [PATCH v5] Documentation/gpu: Add a VM_BIND async draft document

[Zanoni, Paulo R]

On Thu, 2023-08-10 at 21:49 +0200, Thomas Hellström wrote:
> Hi, Paulo.
> 
> Thanks for reviewing. I'd like to try give some answers below.
> 
> On 8/4/23 23:30, Zanoni, Paulo R wrote:
> > On Fri, 2023-08-04 at 16:02 -0400, Rodrigo Vivi wrote:
> > > On Wed, Jul 19, 2023 at 07:50:21PM +, Zanoni, Paulo R wrote:
> > > > On Sat, 2023-07-15 at 17:45 +0200, Thomas Hellström wrote:
> > > > > Add a motivation for and description of asynchronous VM_BIND operation
> > > > I think I may have missed some other documentation, which would explain
> > > > some of my questions below, so please be patient with my
> > > > misunderstandings. But here's a review from the POV of a UMD person.
> > > > 
> > > > 
> > > > > v2:
> > > > > - Fix typos (Nirmoy Das)
> > > > > - Improve the description of a memory fence (Oak Zeng)
> > > > > - Add a reference to the document in the Xe RFC.
> > > > > - Add pointers to sample uAPI suggestions
> > > > > v3:
> > > > > - Address review comments (Danilo Krummrich)
> > > > > - Formatting fixes
> > > > > v4:
> > > > > - Address typos (Francois Dugast)
> > > > > - Explain why in-fences are not allowed for VM_BIND operations for 
> > > > > long-
> > > > >running workloads (Matthew Brost)
> > > > > v5:
> > > > > - More typo- and style fixing
> > > > > - Further clarify the implications of disallowing in-fences for 
> > > > > VM_BIND
> > > > >operations for long-running workloads (Matthew Brost)
> > > > > 
> > > > > Signed-off-by: Thomas Hellström 
> > > > > Acked-by: Nirmoy Das 
> > > > > ---
> > > > >   Documentation/gpu/drm-vm-bind-async.rst | 171 
> > > > > 
> > > > >   Documentation/gpu/rfc/xe.rst|   4 +-
> > > > >   2 files changed, 173 insertions(+), 2 deletions(-)
> > > > >   create mode 100644 Documentation/gpu/drm-vm-bind-async.rst
> > > > > 
> > > > > diff --git a/Documentation/gpu/drm-vm-bind-async.rst 
> > > > > b/Documentation/gpu/drm-vm-bind-async.rst
> > > > > new file mode 100644
> > > > > index ..d2b02a38198a
> > > > > --- /dev/null
> > > > > +++ b/Documentation/gpu/drm-vm-bind-async.rst
> > > > > @@ -0,0 +1,171 @@
> > > > > +
> > > > > +Asynchronous VM_BIND
> > > > > +
> > > > > +
> > > > > +Nomenclature:
> > > > > +=
> > > > > +
> > > > > +* ``VRAM``: On-device memory. Sometimes referred to as device local 
> > > > > memory.
> > > > > +
> > > > > +* ``gpu_vm``: A GPU address space. Typically per process, but can be 
> > > > > shared by
> > > > > +  multiple processes.
> > > > > +
> > > > > +* ``VM_BIND``: An operation or a list of operations to modify a 
> > > > > gpu_vm using
> > > > > +  an IOCTL. The operations include mapping and unmapping system- or
> > > > > +  VRAM memory.
> > > > > +
> > > > > +* ``syncobj``: A container that abstracts synchronization objects. 
> > > > > The
> > > > > +  synchronization objects can be either generic, like dma-fences or
> > > > > +  driver specific. A syncobj typically indicates the type of the
> > > > > +  underlying synchronization object.
> > > > > +
> > > > > +* ``in-syncobj``: Argument to a VM_BIND IOCTL, the VM_BIND operation 
> > > > > waits
> > > > > +  for these before starting.
> > > > > +
> > > > > +* ``out-syncobj``: Argument to a VM_BIND_IOCTL, the VM_BIND operation
> > > > > +  signals these when the bind operation is complete.
> > > > > +
> > > > > +* ``memory fence``: A synchronization object, different from a 
> > > > > dma-fence.
> > > > Since you've mentioned it twice in this document already, for
> > > > completeness would you mind also giving a definition for dma-fence in
> > > > what it relates/contrasts to the rest of the text?
> > > Maybe worth a link to the dma-fence doc itself?
> > > (somehow making sphinx to point out 

Re: [Intel-xe] [PATCH v5] Documentation/gpu: Add a VM_BIND async draft document

[Zanoni, Paulo R]

On Fri, 2023-08-04 at 16:02 -0400, Rodrigo Vivi wrote:
> On Wed, Jul 19, 2023 at 07:50:21PM +0000, Zanoni, Paulo R wrote:
> > On Sat, 2023-07-15 at 17:45 +0200, Thomas Hellström wrote:
> > > Add a motivation for and description of asynchronous VM_BIND operation
> > 
> > I think I may have missed some other documentation, which would explain
> > some of my questions below, so please be patient with my
> > misunderstandings. But here's a review from the POV of a UMD person.
> > 
> > 
> > > 
> > > v2:
> > > - Fix typos (Nirmoy Das)
> > > - Improve the description of a memory fence (Oak Zeng)
> > > - Add a reference to the document in the Xe RFC.
> > > - Add pointers to sample uAPI suggestions
> > > v3:
> > > - Address review comments (Danilo Krummrich)
> > > - Formatting fixes
> > > v4:
> > > - Address typos (Francois Dugast)
> > > - Explain why in-fences are not allowed for VM_BIND operations for long-
> > >   running workloads (Matthew Brost)
> > > v5:
> > > - More typo- and style fixing
> > > - Further clarify the implications of disallowing in-fences for VM_BIND
> > >   operations for long-running workloads (Matthew Brost)
> > > 
> > > Signed-off-by: Thomas Hellström 
> > > Acked-by: Nirmoy Das 
> > > ---
> > >  Documentation/gpu/drm-vm-bind-async.rst | 171 
> > >  Documentation/gpu/rfc/xe.rst|   4 +-
> > >  2 files changed, 173 insertions(+), 2 deletions(-)
> > >  create mode 100644 Documentation/gpu/drm-vm-bind-async.rst
> > > 
> > > diff --git a/Documentation/gpu/drm-vm-bind-async.rst 
> > > b/Documentation/gpu/drm-vm-bind-async.rst
> > > new file mode 100644
> > > index ..d2b02a38198a
> > > --- /dev/null
> > > +++ b/Documentation/gpu/drm-vm-bind-async.rst
> > > @@ -0,0 +1,171 @@
> > > +
> > > +Asynchronous VM_BIND
> > > +
> > > +
> > > +Nomenclature:
> > > +=
> > > +
> > > +* ``VRAM``: On-device memory. Sometimes referred to as device local 
> > > memory.
> > > +
> > > +* ``gpu_vm``: A GPU address space. Typically per process, but can be 
> > > shared by
> > > +  multiple processes.
> > > +
> > > +* ``VM_BIND``: An operation or a list of operations to modify a gpu_vm 
> > > using
> > > +  an IOCTL. The operations include mapping and unmapping system- or
> > > +  VRAM memory.
> > > +
> > > +* ``syncobj``: A container that abstracts synchronization objects. The
> > > +  synchronization objects can be either generic, like dma-fences or
> > > +  driver specific. A syncobj typically indicates the type of the
> > > +  underlying synchronization object.
> > > +
> > > +* ``in-syncobj``: Argument to a VM_BIND IOCTL, the VM_BIND operation 
> > > waits
> > > +  for these before starting.
> > > +
> > > +* ``out-syncobj``: Argument to a VM_BIND_IOCTL, the VM_BIND operation
> > > +  signals these when the bind operation is complete.
> > > +
> > > +* ``memory fence``: A synchronization object, different from a dma-fence.
> > 
> > Since you've mentioned it twice in this document already, for
> > completeness would you mind also giving a definition for dma-fence in
> > what it relates/contrasts to the rest of the text?
> 
> Maybe worth a link to the dma-fence doc itself?
> (somehow making sphinx to point out to driver-api/dma-buf.html#dma-fences)
> 
> But the differences are written below Paulo:
> 
> > 
> > 
> > > +  A memory fence uses the value of a specified memory location to 
> > > determine
> > > +  signaled status. A memory fence can be awaited and signaled by both
> > > +  the GPU and CPU. Memory fences are sometimes referred to as
> > > +  user-fences, userspace-fences or gpu futexes and do not necessarily 
> > > obey
> > > +  the dma-fence rule of signaling within a "reasonable amount of time".
> > > +  The kernel should thus avoid waiting for memory fences with locks held.
> 
> ^
> 
> > > +
> > > +* ``long-running workload``: A workload that may take more than the
> > > +  current stipulated dma-fence maximum signal delay to complete and
> > 
> > Where is this delay defined? Is this the same as the gpuhang timer?
> 
> dma-fence defines it in a very "cool" way: "reasonable amount

Re: [Intel-xe] [PATCH v5] Documentation/gpu: Add a VM_BIND async draft document

[Zanoni, Paulo R]

On Sat, 2023-07-15 at 17:45 +0200, Thomas Hellström wrote:
> Add a motivation for and description of asynchronous VM_BIND operation

I think I may have missed some other documentation, which would explain
some of my questions below, so please be patient with my
misunderstandings. But here's a review from the POV of a UMD person.


> 
> v2:
> - Fix typos (Nirmoy Das)
> - Improve the description of a memory fence (Oak Zeng)
> - Add a reference to the document in the Xe RFC.
> - Add pointers to sample uAPI suggestions
> v3:
> - Address review comments (Danilo Krummrich)
> - Formatting fixes
> v4:
> - Address typos (Francois Dugast)
> - Explain why in-fences are not allowed for VM_BIND operations for long-
>   running workloads (Matthew Brost)
> v5:
> - More typo- and style fixing
> - Further clarify the implications of disallowing in-fences for VM_BIND
>   operations for long-running workloads (Matthew Brost)
> 
> Signed-off-by: Thomas Hellström 
> Acked-by: Nirmoy Das 
> ---
>  Documentation/gpu/drm-vm-bind-async.rst | 171 
>  Documentation/gpu/rfc/xe.rst|   4 +-
>  2 files changed, 173 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/gpu/drm-vm-bind-async.rst
> 
> diff --git a/Documentation/gpu/drm-vm-bind-async.rst 
> b/Documentation/gpu/drm-vm-bind-async.rst
> new file mode 100644
> index ..d2b02a38198a
> --- /dev/null
> +++ b/Documentation/gpu/drm-vm-bind-async.rst
> @@ -0,0 +1,171 @@
> +
> +Asynchronous VM_BIND
> +
> +
> +Nomenclature:
> +=
> +
> +* ``VRAM``: On-device memory. Sometimes referred to as device local memory.
> +
> +* ``gpu_vm``: A GPU address space. Typically per process, but can be shared 
> by
> +  multiple processes.
> +
> +* ``VM_BIND``: An operation or a list of operations to modify a gpu_vm using
> +  an IOCTL. The operations include mapping and unmapping system- or
> +  VRAM memory.
> +
> +* ``syncobj``: A container that abstracts synchronization objects. The
> +  synchronization objects can be either generic, like dma-fences or
> +  driver specific. A syncobj typically indicates the type of the
> +  underlying synchronization object.
> +
> +* ``in-syncobj``: Argument to a VM_BIND IOCTL, the VM_BIND operation waits
> +  for these before starting.
> +
> +* ``out-syncobj``: Argument to a VM_BIND_IOCTL, the VM_BIND operation
> +  signals these when the bind operation is complete.
> +
> +* ``memory fence``: A synchronization object, different from a dma-fence.

Since you've mentioned it twice in this document already, for
completeness would you mind also giving a definition for dma-fence in
what it relates/contrasts to the rest of the text?


> +  A memory fence uses the value of a specified memory location to determine
> +  signaled status. A memory fence can be awaited and signaled by both
> +  the GPU and CPU. Memory fences are sometimes referred to as
> +  user-fences, userspace-fences or gpu futexes and do not necessarily obey
> +  the dma-fence rule of signaling within a "reasonable amount of time".
> +  The kernel should thus avoid waiting for memory fences with locks held.
> +
> +* ``long-running workload``: A workload that may take more than the
> +  current stipulated dma-fence maximum signal delay to complete and

Where is this delay defined? Is this the same as the gpuhang timer?


> +  which therefore needs to set the gpu_vm or the GPU execution context in
> +  a certain mode that disallows completion dma-fences.
> +
> +* ``exec function``: An exec function is a function that revalidates all
> +  affected gpu_vmas, submits a GPU command batch and registers the
> +  dma_fence representing the GPU command's activity with all affected
> +  dma_resvs. For completeness, although not covered by this document,
> +  it's worth mentioning that an exec function may also be the
> +  revalidation worker that is used by some drivers in compute /
> +  long-running mode.
> +
> +* ``bind context``: A context identifier used for the VM_BIND
> +  operation. VM_BIND operations that use the same bind context can be
> +  assumed, where it matters, to complete in order of submission. No such
> +  assumptions can be made for VM_BIND operations using separate bind 
> contexts.
> +
> +* ``UMD``: User-mode driver.
> +
> +* ``KMD``: Kernel-mode driver.
> +
> +
> +Synchronous / Asynchronous VM_BIND operation
> +
> +
> +Synchronous VM_BIND
> +___
> +With Synchronous VM_BIND, the VM_BIND operations all complete before the
> +IOCTL returns. A synchronous VM_BIND takes neither in-fences nor
> +out-fences. Synchronous VM_BIND may block and wait for GPU operations;
> +for example swap-in or clearing, or even previous binds.
> +
> +Asynchronous VM_BIND
> +
> +Asynchronous VM_BIND accepts both in-syncobjs and out-syncobjs. While the
> +IOCTL may return immediately, the VM_BIND operations wait for the in-syncobjs
> 

Re: [PATCH v2] drm/i915: Make IRQ reset and postinstall multi-gt aware

[Zanoni, Paulo R]

On Thu, 2023-04-13 at 11:20 +0200, Andi Shyti wrote:
> From: Paulo Zanoni 
Hi

https://en.wikipedia.org/wiki/Ship_of_Theseus

My original patch was written in 2018. Since then, the implementation
has been rebased and changed multiple times, the commit message has
been changed, the subject line has been changed, yet none of that is
documented in the patch's revision history: it was all removed and it
now looks like I'm the author of the version that was submitted this
month. I never liked this "erase the internal patch's changelog before
submitting it upstream for the first time" process, I think it erases
crucial information and misleads people.

I know I said something different earlier in private, but after further
reflection, I concluded I do not feel comfortable having my name as the
Author or as the Signed-off-by in this patch. Please remove it. You can
add a "Based-on-patch-by: Paulo Zanoni " if
you want, but that's not necessary.

This should also help in case some bug is bisected to this patch, then
I won't need to spend time researching who I should forward the emails
to.

Thanks,
Paulo

> 
> In multitile systems IRQ need to be reset and enabled per GT.
> 
> Although in MTL the GUnit misc interrupts register set are
> available only in GT-0, we need to loop through all the GT's
> in order to initialize the media engine which lies on a different
> GT.
> 
> Signed-off-by: Paulo Zanoni 
> Cc: Tvrtko Ursulin 
> Signed-off-by: Andi Shyti 
> ---
> Hi,
> 
> proposing again this patch, apparently GuC needs this patch to
> initialize the media GT.
> 
> Andi
> 
> Changelog
> =
> v1 -> v2
>  - improve description in the commit log.
> 
>  drivers/gpu/drm/i915/i915_irq.c | 28 ++--
>  1 file changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index d24bdea65a3dc..524d64bf5d186 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2764,14 +2764,19 @@ static void dg1_irq_reset(struct drm_i915_private 
> *dev_priv)
>  {
>   struct intel_gt *gt = to_gt(dev_priv);
>   struct intel_uncore *uncore = gt->uncore;
> + unsigned int i;
>  
> 
> 
> 
>   dg1_master_intr_disable(dev_priv->uncore.regs);
>  
> 
> 
> 
> - gen11_gt_irq_reset(gt);
> - gen11_display_irq_reset(dev_priv);
> + for_each_gt(gt, dev_priv, i) {
> + gen11_gt_irq_reset(gt);
>  
> 
> 
> 
> - GEN3_IRQ_RESET(uncore, GEN11_GU_MISC_);
> - GEN3_IRQ_RESET(uncore, GEN8_PCU_);
> + uncore = gt->uncore;
> + GEN3_IRQ_RESET(uncore, GEN11_GU_MISC_);
> + GEN3_IRQ_RESET(uncore, GEN8_PCU_);
> + }
> +
> + gen11_display_irq_reset(dev_priv);
>  }
>  
> 
> 
> 
>  void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
> @@ -3425,13 +3430,16 @@ static void gen11_irq_postinstall(struct 
> drm_i915_private *dev_priv)
>  
> 
> 
> 
>  static void dg1_irq_postinstall(struct drm_i915_private *dev_priv)
>  {
> - struct intel_gt *gt = to_gt(dev_priv);
> - struct intel_uncore *uncore = gt->uncore;
>   u32 gu_misc_masked = GEN11_GU_MISC_GSE;
> + struct intel_gt *gt;
> + unsigned int i;
>  
> 
> 
> 
> - gen11_gt_irq_postinstall(gt);
> + for_each_gt(gt, dev_priv, i) {
> + gen11_gt_irq_postinstall(gt);
>  
> 
> 
> 
> - GEN3_IRQ_INIT(uncore, GEN11_GU_MISC_, ~gu_misc_masked, gu_misc_masked);
> + GEN3_IRQ_INIT(gt->uncore, GEN11_GU_MISC_, ~gu_misc_masked,
> +   gu_misc_masked);
> + }
>  
> 
> 
> 
>   if (HAS_DISPLAY(dev_priv)) {
>   icp_irq_postinstall(dev_priv);
> @@ -3440,8 +3448,8 @@ static void dg1_irq_postinstall(struct drm_i915_private 
> *dev_priv)
>  GEN11_DISPLAY_IRQ_ENABLE);
>   }
>  
> 
> 
> 
> - dg1_master_intr_enable(uncore->regs);
> - intel_uncore_posting_read(uncore, DG1_MSTR_TILE_INTR);
> + dg1_master_intr_enable(to_gt(dev_priv)->uncore->regs);
> + intel_uncore_posting_read(to_gt(dev_priv)->uncore, DG1_MSTR_TILE_INTR);
>  }
>  
> 
> 
> 
>  static void cherryview_irq_postinstall(struct drm_i915_private *dev_priv)

Re: [PATCH v10 00/23] drm/i915/vm_bind: Add VM_BIND functionality

[Zanoni, Paulo R]

On Tue, 2023-01-17 at 23:15 -0800, Niranjana Vishwanathapura wrote:
> DRM_I915_GEM_VM_BIND/UNBIND ioctls allows UMD to bind/unbind GEM
> buffer objects (BOs) or sections of a BOs at specified GPU virtual
> addresses on a specified address space (VM). Multiple mappings can map
> to the same physical pages of an object (aliasing). These mappings (also
> referred to as persistent mappings) will be persistent across multiple
> GPU submissions (execbuf calls) issued by the UMD, without user having
> to provide a list of all required mappings during each submission (as
> required by older execbuf mode).
> 
> This patch series support VM_BIND version 1, as described by the param
> I915_PARAM_VM_BIND_VERSION.
> 
> Add new execbuf3 ioctl (I915_GEM_EXECBUFFER3) which only works in
> vm_bind mode. The vm_bind mode only works with this new execbuf3 ioctl.
> The new execbuf3 ioctl will not have any execlist support and all the
> legacy support like relocations etc., are removed.
> 
> NOTEs:
> * It is based on below VM_BIND design+uapi rfc.
>   Documentation/gpu/rfc/i915_vm_bind.rst
> 
> * The IGT RFC series is posted as,
>   [PATCH i-g-t v10 0/19] vm_bind: Add VM_BIND validation support

FYI, I created a Draft MR for the Mesa implementation:

https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/21057

A Draft MR should be easier to track than simply a branch on a personal
tree. Feel free to put this link in the next cover letters for v11 and
above.

> 
> v2: Address various review comments
> v3: Address review comments and other fixes
> v4: Remove vm_unbind out fence uapi which is not supported yet,
> replace vm->vm_bind_mode check with i915_gem_vm_is_vm_bind_mode()
> v5: Render kernel-doc, use PIN_NOEVICT, limit vm_bind support to
> non-recoverable faults
> v6: Rebased, minor fixes, add reserved fields to drm_i915_gem_vm_bind,
> add new patch for async vm_unbind support
> v7: Rebased, minor cleanups as per review feedback
> v8: Rebased, add capture support
> v9: Address capture support feedback from v8
> v10: Properly handle vma->resource for mappings with capture request
> 
> Test-with: 20230118071350.17498-1-niranjana.vishwanathap...@intel.com
> 
> Signed-off-by: Niranjana Vishwanathapura 
> 
> Niranjana Vishwanathapura (23):
>   drm/i915/vm_bind: Expose vm lookup function
>   drm/i915/vm_bind: Add __i915_sw_fence_await_reservation()
>   drm/i915/vm_bind: Expose i915_gem_object_max_page_size()
>   drm/i915/vm_bind: Support partially mapped vma resource
>   drm/i915/vm_bind: Add support to create persistent vma
>   drm/i915/vm_bind: Implement bind and unbind of object
>   drm/i915/vm_bind: Support for VM private BOs
>   drm/i915/vm_bind: Add support to handle object evictions
>   drm/i915/vm_bind: Support persistent vma activeness tracking
>   drm/i915/vm_bind: Add out fence support
>   drm/i915/vm_bind: Abstract out common execbuf functions
>   drm/i915/vm_bind: Use common execbuf functions in execbuf path
>   drm/i915/vm_bind: Implement I915_GEM_EXECBUFFER3 ioctl
>   drm/i915/vm_bind: Update i915_vma_verify_bind_complete()
>   drm/i915/vm_bind: Expose i915_request_await_bind()
>   drm/i915/vm_bind: Handle persistent vmas in execbuf3
>   drm/i915/vm_bind: userptr dma-resv changes
>   drm/i915/vm_bind: Limit vm_bind mode to non-recoverable contexts
>   drm/i915/vm_bind: Add uapi for user to enable vm_bind_mode
>   drm/i915/vm_bind: Render VM_BIND documentation
>   drm/i915/vm_bind: Async vm_unbind support
>   drm/i915/vm_bind: Properly build persistent map sg table
>   drm/i915/vm_bind: Support capture of persistent mappings
> 
>  Documentation/gpu/i915.rst|  78 +-
>  drivers/gpu/drm/i915/Makefile |   3 +
>  drivers/gpu/drm/i915/gem/i915_gem_context.c   |  43 +-
>  drivers/gpu/drm/i915/gem/i915_gem_context.h   |  17 +
>  drivers/gpu/drm/i915/gem/i915_gem_create.c|  72 +-
>  drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c|   6 +
>  .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 522 +--
>  .../gpu/drm/i915/gem/i915_gem_execbuffer3.c   | 872 ++
>  .../drm/i915/gem/i915_gem_execbuffer_common.c | 671 ++
>  .../drm/i915/gem/i915_gem_execbuffer_common.h |  76 ++
>  drivers/gpu/drm/i915/gem/i915_gem_ioctls.h|   2 +
>  drivers/gpu/drm/i915/gem/i915_gem_object.c|   3 +
>  drivers/gpu/drm/i915/gem/i915_gem_object.h|   2 +
>  .../gpu/drm/i915/gem/i915_gem_object_types.h  |   6 +
>  drivers/gpu/drm/i915/gem/i915_gem_userptr.c   |  19 +
>  drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h   |  30 +
>  .../drm/i915/gem/i915_gem_vm_bind_object.c| 463 ++
>  drivers/gpu/drm/i915/gt/intel_gtt.c   |  22 +
>  drivers/gpu/drm/i915/gt/intel_gtt.h   |  28 +
>  drivers/gpu/drm/i915/i915_driver.c|   4 +
>  drivers/gpu/drm/i915/i915_drv.h   |   2 +
>  drivers/gpu/drm/i915/i915_gem.c   |  14 +-
>  drivers/gpu/drm/i915/i915_gem_gtt.c   |  39 +
>  

Re: [PATCH v9 10/23] drm/i915/vm_bind: Add out fence support

[Zanoni, Paulo R]

On Mon, 2022-12-12 at 15:15 -0800, Niranjana Vishwanathapura wrote:
> Add support for handling out fence for vm_bind call.
> 
> v2: Reset vma->vm_bind_fence.syncobj to NULL at the end
> of vm_bind call.
> v3: Remove vm_unbind out fence uapi which is not supported yet.
> v4: Return error if I915_TIMELINE_FENCE_WAIT fence flag is set.
> Wait for bind to complete iff I915_TIMELINE_FENCE_SIGNAL is
> not specified.
> v5: Ensure __I915_TIMELINE_FENCE_UNKNOWN_FLAGS are not set.
> 
> Reviewed-by: Matthew Auld 
> Signed-off-by: Niranjana Vishwanathapura 
> Signed-off-by: Andi Shyti 
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h   |  4 +
>  .../drm/i915/gem/i915_gem_vm_bind_object.c| 98 ++-
>  drivers/gpu/drm/i915/i915_vma.c   |  7 +-
>  drivers/gpu/drm/i915/i915_vma_types.h |  7 ++
>  include/uapi/drm/i915_drm.h   | 58 ++-
>  5 files changed, 165 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h 
> b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h
> index 36262a6357b5..b70e900e35ab 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h
> @@ -8,6 +8,7 @@
>  
> 
> 
> 
>  #include 
>  
> 
> 
> 
> +struct dma_fence;
>  struct drm_device;
>  struct drm_file;
>  struct i915_address_space;
> @@ -23,4 +24,7 @@ int i915_gem_vm_unbind_ioctl(struct drm_device *dev, void 
> *data,
>  
> 
> 
> 
>  void i915_gem_vm_unbind_all(struct i915_address_space *vm);
>  
> 
> 
> 
> +void i915_vm_bind_signal_fence(struct i915_vma *vma,
> +struct dma_fence * const fence);
> +
>  #endif /* __I915_GEM_VM_BIND_H */
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c 
> b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
> index dc738677466b..fd1d82ce99e6 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
> @@ -7,6 +7,8 @@
>  
> 
> 
> 
>  #include 
>  
> 
> 
> 
> +#include 
> +
>  #include "gem/i915_gem_context.h"
>  #include "gem/i915_gem_vm_bind.h"
>  
> 
> 
> 
> @@ -101,6 +103,77 @@ static void i915_gem_vm_bind_remove(struct i915_vma 
> *vma, bool release_obj)
>   i915_gem_object_put(vma->obj);
>  }
>  
> 
> 
> 
> +static int i915_vm_bind_add_fence(struct drm_file *file, struct i915_vma 
> *vma,
> +   u32 handle, u64 point)
> +{
> + struct drm_syncobj *syncobj;
> +
> + syncobj = drm_syncobj_find(file, handle);
> + if (!syncobj) {
> + drm_dbg(>vm->i915->drm,
> + "Invalid syncobj handle provided\n");
> + return -ENOENT;
> + }
> +
> + /*
> +  * For timeline syncobjs we need to preallocate chains for
> +  * later signaling.
> +  */
> + if (point) {
> + vma->vm_bind_fence.chain_fence = dma_fence_chain_alloc();
> + if (!vma->vm_bind_fence.chain_fence) {
> + drm_syncobj_put(syncobj);
> + return -ENOMEM;
> + }
> + } else {
> + vma->vm_bind_fence.chain_fence = NULL;
> + }
> + vma->vm_bind_fence.syncobj = syncobj;
> + vma->vm_bind_fence.value = point;
> +
> + return 0;
> +}
> +
> +static void i915_vm_bind_put_fence(struct i915_vma *vma)
> +{
> + if (!vma->vm_bind_fence.syncobj)
> + return;
> +
> + drm_syncobj_put(vma->vm_bind_fence.syncobj);
> + dma_fence_chain_free(vma->vm_bind_fence.chain_fence);
> + vma->vm_bind_fence.syncobj = NULL;
> +}
> +
> +/**
> + * i915_vm_bind_signal_fence() - Add fence to vm_bind syncobj
> + * @vma: vma mapping requiring signaling
> + * @fence: fence to be added
> + *
> + * Associate specified @fence with the @vma's syncobj to be
> + * signaled after the @fence work completes.
> + */
> +void i915_vm_bind_signal_fence(struct i915_vma *vma,
> +struct dma_fence * const fence)
> +{
> + struct drm_syncobj *syncobj = vma->vm_bind_fence.syncobj;
> +
> + if (!syncobj)
> + return;
> +
> + if (vma->vm_bind_fence.chain_fence) {
> + drm_syncobj_add_point(syncobj,
> +   vma->vm_bind_fence.chain_fence,
> +   fence, vma->vm_bind_fence.value);
> + /*
> +  * The chain's ownership is transferred to the
> +  * timeline.
> +  */
> + vma->vm_bind_fence.chain_fence = NULL;
> + } else {
> + drm_syncobj_replace_fence(syncobj, fence);
> + }
> +}
> +
>  static int i915_gem_vm_unbind_vma(struct i915_address_space *vm,
>     struct drm_i915_gem_vm_unbind *va)
>  {
> @@ -206,6 +279,11 @@ static int i915_gem_vm_bind_obj(struct 
> i915_address_space *vm,
>   if (!va->length || !IS_ALIGNED(va->start, I915_GTT_PAGE_SIZE))
>   ret = -EINVAL;
>  
> 
> 
> 
> + /* In 

Re: [Intel-gfx] [PATCH 0/2] drm/i915: Remove frontbuffer tracking from gem.

[Zanoni, Paulo R]

Hi

I was given a link to https://patchwork.freedesktop.org/series/111494/
but can't seem to find it on the mailing list, so I'll reply here.

On Thu, 2022-08-25 at 08:46 +0200, Maarten Lankhorst wrote:
> Frontbuffer tracking in gem is used in old drivers, but nowadays everyone
> calls dirtyfb explicitly. Remove frontbuffer tracking from gem, and
> isolate it to display only.

Ok, but then how can the Kernel know if the user space running is an
"old driver" or a new one? Assuming everybody is following the new
policy is at least risky.

(crazy idea: have an ioctl for user space to tell whether it knows how
to DirtyFB and another where it can even say it will never be touching
frontbuffers at all)

Also, when I wrote igt/kms_frontbuffer_tracking, it was agreed that
whatever was there was a representation of the "rules" of the
frontbfuffer writing contract/API. And I see on the link above that the
basic tests of kms_frontbuffer_tracking fail. If
kms_frontbuffer_tracking requires change due to i915.ko having changed
the frontbuffer writing rules, then we should do it, and possibly have
those rules written somewhere.

But then, having the Kernel change expectations like that is not
exactly what I learned we should be doing, because it's the recipe to
break user space.

I'm confused, please clarify us a little more. More sentences in the
commit messages would be absolutely great.

Thanks,
Paulo

> 
> Maarten Lankhorst (2):
>   drm/i915: Remove gem and overlay frontbuffer tracking
>   drm/i915: Remove special frontbuffer type
> 
>  drivers/gpu/drm/i915/display/intel_cursor.c   |   6 +-
>  drivers/gpu/drm/i915/display/intel_display.c  |   4 +-
>  .../drm/i915/display/intel_display_types.h|   8 +-
>  drivers/gpu/drm/i915/display/intel_fb.c   |  11 +-
>  drivers/gpu/drm/i915/display/intel_fbdev.c|   7 +-
>  .../gpu/drm/i915/display/intel_frontbuffer.c  | 103 ++
>  .../gpu/drm/i915/display/intel_frontbuffer.h  |  65 ++-
>  drivers/gpu/drm/i915/display/intel_overlay.c  |  14 ---
>  .../drm/i915/display/intel_plane_initial.c|   3 +-
>  drivers/gpu/drm/i915/display/intel_psr.c  |   1 +
>  drivers/gpu/drm/i915/gem/i915_gem_clflush.c   |   4 -
>  drivers/gpu/drm/i915/gem/i915_gem_domain.c|   7 --
>  .../gpu/drm/i915/gem/i915_gem_execbuffer.c|   2 -
>  drivers/gpu/drm/i915/gem/i915_gem_object.c|  25 -
>  drivers/gpu/drm/i915/gem/i915_gem_object.h|  22 
>  drivers/gpu/drm/i915/gem/i915_gem_phys.c  |   4 -
>  drivers/gpu/drm/i915/i915_driver.c|   1 +
>  drivers/gpu/drm/i915/i915_drv.h   |   1 -
>  drivers/gpu/drm/i915/i915_gem.c   |   8 --
>  drivers/gpu/drm/i915/i915_gem_gtt.c   |   1 -
>  drivers/gpu/drm/i915/i915_vma.c   |  12 --
>  21 files changed, 35 insertions(+), 274 deletions(-)
> 

Re: [PATCH v7 00/20] drm/i915/vm_bind: Add VM_BIND functionality

[Zanoni, Paulo R]

On Sat, 2022-11-12 at 23:57 -0800, Niranjana Vishwanathapura wrote:
> DRM_I915_GEM_VM_BIND/UNBIND ioctls allows UMD to bind/unbind GEM
> buffer objects (BOs) or sections of a BOs at specified GPU virtual
> addresses on a specified address space (VM). Multiple mappings can map
> to the same physical pages of an object (aliasing). These mappings (also
> referred to as persistent mappings) will be persistent across multiple
> GPU submissions (execbuf calls) issued by the UMD, without user having
> to provide a list of all required mappings during each submission (as
> required by older execbuf mode).
> 
> This patch series support VM_BIND version 1, as described by the param
> I915_PARAM_VM_BIND_VERSION.
> 
> Add new execbuf3 ioctl (I915_GEM_EXECBUFFER3) which only works in
> vm_bind mode. The vm_bind mode only works with this new execbuf3 ioctl.
> The new execbuf3 ioctl will not have any execlist support and all the
> legacy support like relocations etc., are removed.
> 
> NOTEs:
> * It is based on below VM_BIND design+uapi rfc.
>   Documentation/gpu/rfc/i915_vm_bind.rst

Just as a FYI to everyone, there is a Mesa Iris implementation that
makes use of this series here:

https://gitlab.freedesktop.org/pzanoni/mesa/-/commits/upstream-vmbind/

Some notes on it:

- Tested on TGL and Alchemist (aka DG2).

- The code still has a lot of TODOs and some FIXMEs.

- It was somewhat tested with the common Linux benchmarks (Dota 2,
Manhattan, Xonotic, etc.) and survived most of what I threw at it. The
only problems I saw so far are:

  - Sometimes you get a random GPU hang with Dota 2, but it seems to go
away if you use INTEL_DEBUG=nofc . I'm investigating it right now.

  - Very very rarely DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD returns EINVAL and
we don't properly recover. I am still trying to understand this one,
it's once in a blue moon that it happens.

- Performance seems to be mostly equivalent to non-vm-bind, but I
haven't spent a lot of time really gathering numbers since I'm mostly
debugging things. Using VM_PRIVATE BOs is the key here, you cut Dota's
performance by almost half if you don't use private BOs. Considering
how ABYSMALLY slower things get, I would assume there's probably
something the Kernel could do here to handle things a little faster.
The 'perf' tool shows a lot of i915 fence-related functions wasting CPU
time when we don't use private BOs.

- I am not really sure whether the implicit tracking code actually
works or not. It doesn't really seem to make much of a difference if I
remove it entirely, but I'm still planning to spend more time analyzing
this. If anybody knows of a workload that will absolutely fail if we
get this wrong, please tell me.

- There's the whole frontbuffer tracking discussion from v6 that's
still pending resolution.

- The Vulkan implementation will come later. I wanted to sort all the
GL details later so I don't make the same mistakes twice.

- I really dislike how we have to handle the lack of implicit tracking
support. The code is excessive and racy. See export_bo_sync_state(),
import_bo_sync_state() and their caller from
src/gallium/drivers/iris/iris_batch.c. Suggestions for Mesa
improvements are welcome, but I would really really really prefer to
have a way to just tell execbuffer3 to handle implicit tracking for
these buffers for me in an atomic way.

- I kinda wish execbuffer3 still accepted a pointer to struct
drm_i915_gem_exec_fence in addition to struct
drm_i915_gem_timeline_fence, since we already have to keep a list of
exec_fences for execbuf2, and then in the execbuf3 we convert them to
the new format. We could also do the opposite and leave execbuf2 with
the slower path. But I could live without this, no problem.

- Credits to Ken, Jason, Lionel, Niranjana, Nanley, Daniel and
everybody else who helped me sort things out here.


Is this ready to be merged to the Kernel? Maybe, but I'd like us to
sort these things out first:

1. Get conclusion regarding the frontbuffer tracking issue first.

2. Get some validation from more experienced people (*winks at Jason*)
that our approach with implicit tracking is correct here. Or convince
Niranjana to add a way to pass buffers for implicit tracking so the
Kernel can atomically inside execbuf3 what we're trying to do with 8
ioctls.

3. Fix all the Mesa bugs so we're 100% sure they're not design flaws of
the Kernel.

But that's just my humble opinion.

Thanks,
Paulo

> 
> * The IGT RFC series is posted as,
>   [PATCH i-g-t v7 0/13] vm_bind: Add VM_BIND validation support
> 
> v2: Address various review comments
> v3: Address review comments and other fixes
> v4: Remove vm_unbind out fence uapi which is not supported yet,
> replace vm->vm_bind_mode check with i915_gem_vm_is_vm_bind_mode()
> v5: Render kernel-doc, use PIN_NOEVICT, limit vm_bind support to
> non-recoverable faults
> v6: Rebased, minor fixes, add reserved fields to drm_i915_gem_vm_bind,
> add new patch for async vm_unbind support
> v7: Rebased, minor 

Re: [Intel-gfx] [PATCH v6 00/20] drm/i915/vm_bind: Add VM_BIND functionality

[Zanoni, Paulo R]

On Thu, 2022-11-10 at 15:05 +, Matthew Auld wrote:
> On 10/11/2022 14:47, Tvrtko Ursulin wrote:
> > 
> > On 10/11/2022 05:49, Niranjana Vishwanathapura wrote:
> > > On Wed, Nov 09, 2022 at 04:16:25PM -0800, Zanoni, Paulo R wrote:
> > > > On Mon, 2022-11-07 at 00:51 -0800, Niranjana Vishwanathapura wrote:
> > > > > DRM_I915_GEM_VM_BIND/UNBIND ioctls allows UMD to bind/unbind GEM
> > > > > buffer objects (BOs) or sections of a BOs at specified GPU virtual
> > > > > addresses on a specified address space (VM). Multiple mappings can map
> > > > > to the same physical pages of an object (aliasing). These mappings 
> > > > > (also
> > > > > referred to as persistent mappings) will be persistent across multiple
> > > > > GPU submissions (execbuf calls) issued by the UMD, without user having
> > > > > to provide a list of all required mappings during each submission (as
> > > > > required by older execbuf mode).
> > > > > 
> > > > > This patch series support VM_BIND version 1, as described by the param
> > > > > I915_PARAM_VM_BIND_VERSION.
> > > > > 
> > > > > Add new execbuf3 ioctl (I915_GEM_EXECBUFFER3) which only works in
> > > > > vm_bind mode. The vm_bind mode only works with this new execbuf3 
> > > > > ioctl.
> > > > > The new execbuf3 ioctl will not have any execlist support and all the
> > > > > legacy support like relocations etc., are removed.
> > > > > 
> > > > > NOTEs:
> > > > > * It is based on below VM_BIND design+uapi rfc.
> > > > >   Documentation/gpu/rfc/i915_vm_bind.rst
> > > > 
> > > > Hi
> > > > 
> > > > One difference for execbuf3 that I noticed that is not mentioned in the
> > > > RFC document is that we now don't have a way to signal
> > > > EXEC_OBJECT_WRITE. When looking at the Kernel code, some there are some
> > > > pieces that check for this flag:
> > > > 
> > > > - there's code that deals with frontbuffer rendering
> > > > - there's code that deals with fences
> > > > - there's code that prevents self-modifying batches
> > > > - another that seems related to waiting for objects
> > > > 
> > > > Are there any new rules regarding frontbuffer rendering when we use
> > > > execbuf3? Any other behavior changes related to the other places that
> > > > we should expect when using execbuf3?
> > > > 
> > > 
> > > Paulo,
> > > Most of the EXEC_OBJECT_WRITE check in execbuf path is related to
> > > implicit dependency tracker which execbuf3 does not support. The
> > > frontbuffer related updated is the only exception and I don't
> > > remember the rationale to not require this on execbuf3.
> > > 
> > > Matt, Tvrtko, Daniel, can you please comment here?
> > 
> > Does not ring a bell to me. Looking at the code it certainly looks like 
> > it would be silently failing to handle it properly.
> > 
> > I'll let people with more experience in this area answer, but from my 
> > point of view, if it is decided that it can be left unsupported, then we 
> > probably need a way of failing the ioctl is used against a frontbuffer, 
> > or something, instead of having display corruption.

There's no way for the ioctl to even know we're writing to
frontbuffers. Unless of course it decides to parse the whole
batchbuffer and understand everything that's going on there, which
sounds insane.


> 
> Maybe it's a coincidence but there is:
> https://patchwork.freedesktop.org/series/110715/
> 
> Which looks relevant. Maarten, any hints here?

Can we pretty please have the rules of frontbuffer tracking written
anywhere? I had major trouble trying to understand this back when I was
working on FBC, and now I regret not having written it back then
because I just forgot how it's supposed to work.

My first guess when looking at that patch is that it would completely
break FBC, but hey so many years have passed since I worked on this
that maybe things changed completely. At least I wrote tests to cover
this.

> 
> > 
> > Regards,
> > 
> > Tvrtko

Re: [PATCH v6 05/20] drm/i915/vm_bind: Implement bind and unbind of object

[Zanoni, Paulo R]

On Thu, 2022-11-10 at 08:32 -0800, Niranjana Vishwanathapura wrote:
> On Wed, Nov 09, 2022 at 05:28:59PM -0800, Zanoni, Paulo R wrote:
> > On Mon, 2022-11-07 at 00:51 -0800, Niranjana Vishwanathapura wrote:
> > > Add uapi and implement support for bind and unbind of an
> > > object at the specified GPU virtual addresses.
> > > 
> > > The vm_bind mode is not supported in legacy execbuf2 ioctl.
> > > It will be supported only in the newer execbuf3 ioctl.
> > > 
> > > v2: On older platforms ctx->vm is not set, check for it.
> > > In vm_bind call, add vma to vm_bind_list.
> > > Add more input validity checks.
> > > Update some documentation.
> > > v3: In vm_bind call, add vma to vm_bound_list as user can
> > > request a fence and pass to execbuf3 as input fence.
> > > Remove short term pinning with PIN_VALIDATE flag.
> > > v4: Replace vm->vm_bind_mode check with i915_gem_vm_is_vm_bind_mode().
> > > v5: Ensure all reserved fields are 0, use PIN_NOEVICT.
> > > v6: Add reserved fields to drm_i915_gem_vm_bind.
> > > 
> > > Reviewed-by: Matthew Auld 
> > > Signed-off-by: Niranjana Vishwanathapura 
> > > 
> > > Signed-off-by: Prathap Kumar Valsan 
> > > Signed-off-by: Andi Shyti 
> > > ---
> > >  drivers/gpu/drm/i915/Makefile |   1 +
> > >  drivers/gpu/drm/i915/gem/i915_gem_context.h   |  15 +
> > >  .../gpu/drm/i915/gem/i915_gem_execbuffer.c|   5 +
> > >  drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h   |  26 ++
> > >  .../drm/i915/gem/i915_gem_vm_bind_object.c| 324 ++
> > >  drivers/gpu/drm/i915/gt/intel_gtt.c   |  10 +
> > >  drivers/gpu/drm/i915/gt/intel_gtt.h   |   9 +
> > >  drivers/gpu/drm/i915/i915_driver.c|   3 +
> > >  drivers/gpu/drm/i915/i915_vma.c   |   1 +
> > >  drivers/gpu/drm/i915/i915_vma_types.h |  14 +
> > >  include/uapi/drm/i915_drm.h   |  99 ++
> > >  11 files changed, 507 insertions(+)
> > >  create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h
> > >  create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
> > > 
> > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > > index 51704b54317c..b731f3ac80da 100644
> > > --- a/drivers/gpu/drm/i915/Makefile
> > > +++ b/drivers/gpu/drm/i915/Makefile
> > > @@ -166,6 +166,7 @@ gem-y += \
> > >   gem/i915_gem_ttm_move.o \
> > >   gem/i915_gem_ttm_pm.o \
> > >   gem/i915_gem_userptr.o \
> > > + gem/i915_gem_vm_bind_object.o \
> > >   gem/i915_gem_wait.o \
> > >   gem/i915_gemfs.o
> > >  i915-y += \
> > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.h 
> > > b/drivers/gpu/drm/i915/gem/i915_gem_context.h
> > > index 899fa8f1e0fe..e8b41aa8f8c4 100644
> > > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.h
> > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.h
> > > @@ -139,6 +139,21 @@ int i915_gem_context_setparam_ioctl(struct 
> > > drm_device *dev, void *data,
> > >  int i915_gem_context_reset_stats_ioctl(struct drm_device *dev, void 
> > > *data,
> > >  struct drm_file *file);
> > >  
> > > 
> > > 
> > > 
> > > +/**
> > > + * i915_gem_vm_is_vm_bind_mode() - Check if address space is in vm_bind 
> > > mode
> > > + * @vm: the address space
> > > + *
> > > + * Returns:
> > > + * true: @vm is in vm_bind mode; allows only vm_bind method of binding.
> > > + * false: @vm is not in vm_bind mode; allows only legacy execbuff method
> > > + *of binding.
> > > + */
> > > +static inline bool i915_gem_vm_is_vm_bind_mode(struct i915_address_space 
> > > *vm)
> > > +{
> > > + /* No support to enable vm_bind mode yet */
> > > + return false;
> > > +}
> > > +
> > >  struct i915_address_space *
> > >  i915_gem_vm_lookup(struct drm_i915_file_private *file_priv, u32 id);
> > >  
> > > 
> > > 
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c 
> > > b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > > index 1160723c9d2d..c5bc9f6e887f 100644
> > > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > > @@ -781,6 +7

Re: [PATCH v6 05/20] drm/i915/vm_bind: Implement bind and unbind of object

[Zanoni, Paulo R]

On Mon, 2022-11-07 at 00:51 -0800, Niranjana Vishwanathapura wrote:
> Add uapi and implement support for bind and unbind of an
> object at the specified GPU virtual addresses.
> 
> The vm_bind mode is not supported in legacy execbuf2 ioctl.
> It will be supported only in the newer execbuf3 ioctl.
> 
> v2: On older platforms ctx->vm is not set, check for it.
> In vm_bind call, add vma to vm_bind_list.
> Add more input validity checks.
> Update some documentation.
> v3: In vm_bind call, add vma to vm_bound_list as user can
> request a fence and pass to execbuf3 as input fence.
> Remove short term pinning with PIN_VALIDATE flag.
> v4: Replace vm->vm_bind_mode check with i915_gem_vm_is_vm_bind_mode().
> v5: Ensure all reserved fields are 0, use PIN_NOEVICT.
> v6: Add reserved fields to drm_i915_gem_vm_bind.
> 
> Reviewed-by: Matthew Auld 
> Signed-off-by: Niranjana Vishwanathapura 
> Signed-off-by: Prathap Kumar Valsan 
> Signed-off-by: Andi Shyti 
> ---
>  drivers/gpu/drm/i915/Makefile |   1 +
>  drivers/gpu/drm/i915/gem/i915_gem_context.h   |  15 +
>  .../gpu/drm/i915/gem/i915_gem_execbuffer.c|   5 +
>  drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h   |  26 ++
>  .../drm/i915/gem/i915_gem_vm_bind_object.c| 324 ++
>  drivers/gpu/drm/i915/gt/intel_gtt.c   |  10 +
>  drivers/gpu/drm/i915/gt/intel_gtt.h   |   9 +
>  drivers/gpu/drm/i915/i915_driver.c|   3 +
>  drivers/gpu/drm/i915/i915_vma.c   |   1 +
>  drivers/gpu/drm/i915/i915_vma_types.h |  14 +
>  include/uapi/drm/i915_drm.h   |  99 ++
>  11 files changed, 507 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h
>  create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 51704b54317c..b731f3ac80da 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -166,6 +166,7 @@ gem-y += \
>   gem/i915_gem_ttm_move.o \
>   gem/i915_gem_ttm_pm.o \
>   gem/i915_gem_userptr.o \
> + gem/i915_gem_vm_bind_object.o \
>   gem/i915_gem_wait.o \
>   gem/i915_gemfs.o
>  i915-y += \
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.h 
> b/drivers/gpu/drm/i915/gem/i915_gem_context.h
> index 899fa8f1e0fe..e8b41aa8f8c4 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.h
> @@ -139,6 +139,21 @@ int i915_gem_context_setparam_ioctl(struct drm_device 
> *dev, void *data,
>  int i915_gem_context_reset_stats_ioctl(struct drm_device *dev, void *data,
>  struct drm_file *file);
>  
> 
> 
> 
> +/**
> + * i915_gem_vm_is_vm_bind_mode() - Check if address space is in vm_bind mode
> + * @vm: the address space
> + *
> + * Returns:
> + * true: @vm is in vm_bind mode; allows only vm_bind method of binding.
> + * false: @vm is not in vm_bind mode; allows only legacy execbuff method
> + *of binding.
> + */
> +static inline bool i915_gem_vm_is_vm_bind_mode(struct i915_address_space *vm)
> +{
> + /* No support to enable vm_bind mode yet */
> + return false;
> +}
> +
>  struct i915_address_space *
>  i915_gem_vm_lookup(struct drm_i915_file_private *file_priv, u32 id);
>  
> 
> 
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c 
> b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index 1160723c9d2d..c5bc9f6e887f 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -781,6 +781,11 @@ static int eb_select_context(struct i915_execbuffer *eb)
>   if (unlikely(IS_ERR(ctx)))
>   return PTR_ERR(ctx);
>  
> 
> 
> 
> + if (ctx->vm && i915_gem_vm_is_vm_bind_mode(ctx->vm)) {
> + i915_gem_context_put(ctx);
> + return -EOPNOTSUPP;
> + }
> +
>   eb->gem_context = ctx;
>   if (i915_gem_context_has_full_ppgtt(ctx))
>   eb->invalid_flags |= EXEC_OBJECT_NEEDS_GTT;
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h 
> b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h
> new file mode 100644
> index ..36262a6357b5
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#ifndef __I915_GEM_VM_BIND_H
> +#define __I915_GEM_VM_BIND_H
> +
> +#include 
> +
> +struct drm_device;
> +struct drm_file;
> +struct i915_address_space;
> +struct i915_vma;
> +
> +struct i915_vma *
> +i915_gem_vm_bind_lookup_vma(struct i915_address_space *vm, u64 va);
> +
> +int i915_gem_vm_bind_ioctl(struct drm_device *dev, void *data,
> +struct drm_file *file);
> +int i915_gem_vm_unbind_ioctl(struct drm_device *dev, void *data,
> +  struct drm_file *file);
> +
> +void i915_gem_vm_unbind_all(struct 

Re: [PATCH v6 00/20] drm/i915/vm_bind: Add VM_BIND functionality

[Zanoni, Paulo R]

On Mon, 2022-11-07 at 00:51 -0800, Niranjana Vishwanathapura wrote:
> DRM_I915_GEM_VM_BIND/UNBIND ioctls allows UMD to bind/unbind GEM
> buffer objects (BOs) or sections of a BOs at specified GPU virtual
> addresses on a specified address space (VM). Multiple mappings can map
> to the same physical pages of an object (aliasing). These mappings (also
> referred to as persistent mappings) will be persistent across multiple
> GPU submissions (execbuf calls) issued by the UMD, without user having
> to provide a list of all required mappings during each submission (as
> required by older execbuf mode).
> 
> This patch series support VM_BIND version 1, as described by the param
> I915_PARAM_VM_BIND_VERSION.
> 
> Add new execbuf3 ioctl (I915_GEM_EXECBUFFER3) which only works in
> vm_bind mode. The vm_bind mode only works with this new execbuf3 ioctl.
> The new execbuf3 ioctl will not have any execlist support and all the
> legacy support like relocations etc., are removed.
> 
> NOTEs:
> * It is based on below VM_BIND design+uapi rfc.
>   Documentation/gpu/rfc/i915_vm_bind.rst

Hi

One difference for execbuf3 that I noticed that is not mentioned in the
RFC document is that we now don't have a way to signal
EXEC_OBJECT_WRITE. When looking at the Kernel code, some there are some
pieces that check for this flag:

- there's code that deals with frontbuffer rendering 
- there's code that deals with fences
- there's code that prevents self-modifying batches
- another that seems related to waiting for objects

Are there any new rules regarding frontbuffer rendering when we use
execbuf3? Any other behavior changes related to the other places that
we should expect when using execbuf3?

Thanks,
Paulo

> 
> * The IGT RFC series is posted as,
>   [PATCH i-g-t v5 0/12] vm_bind: Add VM_BIND validation support
> 
> v2: Address various review comments
> v3: Address review comments and other fixes
> v4: Remove vm_unbind out fence uapi which is not supported yet,
> replace vm->vm_bind_mode check with i915_gem_vm_is_vm_bind_mode()
> v5: Render kernel-doc, use PIN_NOEVICT, limit vm_bind support to
> non-recoverable faults
> v6: Rebased, minor fixes, add reserved fields to drm_i915_gem_vm_bind,
> add new patch for async vm_unbind support
> 
> Signed-off-by: Niranjana Vishwanathapura 
> 
> Niranjana Vishwanathapura (20):
>   drm/i915/vm_bind: Expose vm lookup function
>   drm/i915/vm_bind: Add __i915_sw_fence_await_reservation()
>   drm/i915/vm_bind: Expose i915_gem_object_max_page_size()
>   drm/i915/vm_bind: Add support to create persistent vma
>   drm/i915/vm_bind: Implement bind and unbind of object
>   drm/i915/vm_bind: Support for VM private BOs
>   drm/i915/vm_bind: Add support to handle object evictions
>   drm/i915/vm_bind: Support persistent vma activeness tracking
>   drm/i915/vm_bind: Add out fence support
>   drm/i915/vm_bind: Abstract out common execbuf functions
>   drm/i915/vm_bind: Use common execbuf functions in execbuf path
>   drm/i915/vm_bind: Implement I915_GEM_EXECBUFFER3 ioctl
>   drm/i915/vm_bind: Update i915_vma_verify_bind_complete()
>   drm/i915/vm_bind: Expose i915_request_await_bind()
>   drm/i915/vm_bind: Handle persistent vmas in execbuf3
>   drm/i915/vm_bind: userptr dma-resv changes
>   drm/i915/vm_bind: Limit vm_bind mode to non-recoverable contexts
>   drm/i915/vm_bind: Add uapi for user to enable vm_bind_mode
>   drm/i915/vm_bind: Render VM_BIND documentation
>   drm/i915/vm_bind: Async vm_unbind support
> 
>  Documentation/gpu/i915.rst|  78 +-
>  drivers/gpu/drm/i915/Makefile |   3 +
>  drivers/gpu/drm/i915/gem/i915_gem_context.c   |  43 +-
>  drivers/gpu/drm/i915/gem/i915_gem_context.h   |  17 +
>  drivers/gpu/drm/i915/gem/i915_gem_create.c|  72 +-
>  drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c|   6 +
>  .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 516 +--
>  .../gpu/drm/i915/gem/i915_gem_execbuffer3.c   | 871 ++
>  .../drm/i915/gem/i915_gem_execbuffer_common.c | 666 +
>  .../drm/i915/gem/i915_gem_execbuffer_common.h |  74 ++
>  drivers/gpu/drm/i915/gem/i915_gem_ioctls.h|   2 +
>  drivers/gpu/drm/i915/gem/i915_gem_object.c|   3 +
>  drivers/gpu/drm/i915/gem/i915_gem_object.h|   2 +
>  .../gpu/drm/i915/gem/i915_gem_object_types.h  |   6 +
>  drivers/gpu/drm/i915/gem/i915_gem_userptr.c   |  19 +
>  drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h   |  30 +
>  .../drm/i915/gem/i915_gem_vm_bind_object.c| 449 +
>  drivers/gpu/drm/i915/gt/intel_gtt.c   |  17 +
>  drivers/gpu/drm/i915/gt/intel_gtt.h   |  21 +
>  drivers/gpu/drm/i915/i915_driver.c|   4 +
>  drivers/gpu/drm/i915/i915_drv.h   |   2 +
>  drivers/gpu/drm/i915/i915_gem_gtt.c   |  39 +
>  drivers/gpu/drm/i915/i915_gem_gtt.h   |   3 +
>  drivers/gpu/drm/i915/i915_getparam.c  |   3 +
>  drivers/gpu/drm/i915/i915_sw_fence.c  |  28 +-
>  

Re: [PATCH v6 20/20] drm/i915/vm_bind: Async vm_unbind support

[Zanoni, Paulo R]

On Mon, 2022-11-07 at 00:52 -0800, Niranjana Vishwanathapura wrote:
> Asynchronously unbind the vma upon vm_unbind call.
> Fall back to synchronous unbind if backend doesn't support
> async unbind or if async unbind fails.
> 
> No need for vm_unbind out fence support as i915 will internally
> handle all sequencing and user need not try to sequence any
> operation with the unbind completion.

Can you please provide some more details on how this works from the
user space point of view? I want to be able to know with 100% certainty
if an unbind has already happened, so I can reuse that vma or whatever
else I may decide to do. I see the interface does not provide any sort
of drm_syncobjs for me to wait on the async unbind. So, when does the
unbind really happen? When can I be sure it's past so I can do stuff
with it? Why would you provide an async ioctl and provide no means for
user space to wait on it?

Thanks,
Paulo

> 
> Signed-off-by: Niranjana Vishwanathapura 
> ---
>  drivers/gpu/drm/i915/i915_vma.c | 51 ++---
>  drivers/gpu/drm/i915/i915_vma.h |  1 +
>  2 files changed, 48 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> index 08218e3a2f12..03c966fad87b 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -42,6 +42,8 @@
>  #include "i915_vma.h"
>  #include "i915_vma_resource.h"
>  
> 
> 
> 
> +static struct dma_fence *__i915_vma_unbind_async(struct i915_vma *vma);
> +
>  static inline void assert_vma_held_evict(const struct i915_vma *vma)
>  {
>   /*
> @@ -1711,7 +1713,7 @@ void i915_vma_reopen(struct i915_vma *vma)
>   spin_unlock_irq(>closed_lock);
>  }
>  
> 
> 
> 
> -static void force_unbind(struct i915_vma *vma)
> +static void force_unbind(struct i915_vma *vma, bool async)
>  {
>   if (!drm_mm_node_allocated(>node))
>   return;
> @@ -1725,7 +1727,21 @@ static void force_unbind(struct i915_vma *vma)
>   i915_vma_set_purged(vma);
>  
> 
> 
> 
>   atomic_and(~I915_VMA_PIN_MASK, >flags);
> - WARN_ON(__i915_vma_unbind(vma));
> + if (async) {
> + struct dma_fence *fence;
> +
> + fence = __i915_vma_unbind_async(vma);
> + if (IS_ERR_OR_NULL(fence)) {
> + async = false;
> + } else {
> + dma_resv_add_fence(vma->obj->base.resv, fence,
> +DMA_RESV_USAGE_READ);
> + dma_fence_put(fence);
> + }
> + }
> +
> + if (!async)
> + WARN_ON(__i915_vma_unbind(vma));
>   GEM_BUG_ON(drm_mm_node_allocated(>node));
>  }
>  
> 
> 
> 
> @@ -1785,7 +1801,7 @@ void i915_vma_destroy_locked(struct i915_vma *vma)
>  {
>   lockdep_assert_held(>vm->mutex);
>  
> 
> 
> 
> - force_unbind(vma);
> + force_unbind(vma, false);
>   list_del_init(>vm_link);
>   release_references(vma, vma->vm->gt, false);
>  }
> @@ -1796,7 +1812,34 @@ void i915_vma_destroy(struct i915_vma *vma)
>   bool vm_ddestroy;
>  
> 
> 
> 
>   mutex_lock(>vm->mutex);
> - force_unbind(vma);
> + force_unbind(vma, false);
> + list_del_init(>vm_link);
> + vm_ddestroy = vma->vm_ddestroy;
> + vma->vm_ddestroy = false;
> +
> + /* vma->vm may be freed when releasing vma->vm->mutex. */
> + gt = vma->vm->gt;
> + mutex_unlock(>vm->mutex);
> + release_references(vma, gt, vm_ddestroy);
> +}
> +
> +void i915_vma_destroy_async(struct i915_vma *vma)
> +{
> + bool vm_ddestroy, async = vma->obj->mm.rsgt;
> + struct intel_gt *gt;
> +
> + if (dma_resv_reserve_fences(vma->obj->base.resv, 1))
> + async = false;
> +
> + mutex_lock(>vm->mutex);
> + /*
> +  * Ensure any asynchronous binding is complete while using
> +  * async unbind as we will be releasing the vma here.
> +  */
> + if (async && i915_active_wait(>active))
> + async = false;
> +
> + force_unbind(vma, async);
>   list_del_init(>vm_link);
>   vm_ddestroy = vma->vm_ddestroy;
>   vma->vm_ddestroy = false;
> diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
> index 737ef310d046..25f15965dab8 100644
> --- a/drivers/gpu/drm/i915/i915_vma.h
> +++ b/drivers/gpu/drm/i915/i915_vma.h
> @@ -272,6 +272,7 @@ void i915_vma_reopen(struct i915_vma *vma);
>  
> 
> 
> 
>  void i915_vma_destroy_locked(struct i915_vma *vma);
>  void i915_vma_destroy(struct i915_vma *vma);
> +void i915_vma_destroy_async(struct i915_vma *vma);
>  
> 
> 
> 
>  #define assert_vma_held(vma) dma_resv_assert_held((vma)->obj->base.resv)
>  
> 
> 
> 

Re: [PATCH v6 3/3] drm/doc/rfc: VM_BIND uapi definition

[Zanoni, Paulo R]

On Thu, 2022-06-30 at 09:18 -0700, Niranjana Vishwanathapura wrote:
> On Wed, Jun 29, 2022 at 11:39:52PM -0700, Zanoni, Paulo R wrote:
> > On Wed, 2022-06-29 at 23:08 -0700, Niranjana Vishwanathapura wrote:
> > > On Wed, Jun 29, 2022 at 05:33:49PM -0700, Zanoni, Paulo R wrote:
> > > > On Sat, 2022-06-25 at 18:49 -0700, Niranjana Vishwanathapura wrote:
> > > > > VM_BIND and related uapi definitions
> > > > > 
> > > > > v2: Reduce the scope to simple Mesa use case.
> > > > > v3: Expand VM_UNBIND documentation and add
> > > > > I915_GEM_VM_BIND/UNBIND_FENCE_VALID
> > > > > and I915_GEM_VM_BIND_TLB_FLUSH flags.
> > > > > v4: Remove I915_GEM_VM_BIND_TLB_FLUSH flag and add additional
> > > > > documentation for vm_bind/unbind.
> > > > > v5: Remove TLB flush requirement on VM_UNBIND.
> > > > > Add version support to stage implementation.
> > > > > v6: Define and use drm_i915_gem_timeline_fence structure for
> > > > > all timeline fences.
> > > > > v7: Rename I915_PARAM_HAS_VM_BIND to I915_PARAM_VM_BIND_VERSION.
> > > > > Update documentation on async vm_bind/unbind and versioning.
> > > > > Remove redundant vm_bind/unbind FENCE_VALID flag, execbuf3
> > > > > batch_count field and I915_EXEC3_SECURE flag.
> > > > > 
> > > > > Signed-off-by: Niranjana Vishwanathapura 
> > > > > 
> > > > > Reviewed-by: Daniel Vetter 
> > > > > ---
> > > > >  Documentation/gpu/rfc/i915_vm_bind.h | 280 
> > > > > +++
> > > > >  1 file changed, 280 insertions(+)
> > > > >  create mode 100644 Documentation/gpu/rfc/i915_vm_bind.h
> > > > > 
> > > > > diff --git a/Documentation/gpu/rfc/i915_vm_bind.h 
> > > > > b/Documentation/gpu/rfc/i915_vm_bind.h
> > > > > new file mode 100644
> > > > > index ..a93e08bceee6
> > > > > --- /dev/null
> > > > > +++ b/Documentation/gpu/rfc/i915_vm_bind.h
> > > > > @@ -0,0 +1,280 @@
> > > > > +/* SPDX-License-Identifier: MIT */
> > > > > +/*
> > > > > + * Copyright © 2022 Intel Corporation
> > > > > + */
> > > > > +
> > > > > +/**
> > > > > + * DOC: I915_PARAM_VM_BIND_VERSION
> > > > > + *
> > > > > + * VM_BIND feature version supported.
> > > > > + * See typedef drm_i915_getparam_t param.
> > > > > + *
> > > > > + * Specifies the VM_BIND feature version supported.
> > > > > + * The following versions of VM_BIND have been defined:
> > > > > + *
> > > > > + * 0: No VM_BIND support.
> > > > > + *
> > > > > + * 1: In VM_UNBIND calls, the UMD must specify the exact mappings 
> > > > > created
> > > > > + *previously with VM_BIND, the ioctl will not support unbinding 
> > > > > multiple
> > > > > + *mappings or splitting them. Similarly, VM_BIND calls will not 
> > > > > replace
> > > > > + *any existing mappings.
> > > > > + *
> > > > > + * 2: The restrictions on unbinding partial or multiple mappings is
> > > > > + *lifted, Similarly, binding will replace any mappings in the 
> > > > > given range.
> > > > > + *
> > > > > + * See struct drm_i915_gem_vm_bind and struct drm_i915_gem_vm_unbind.
> > > > > + */
> > > > > +#define I915_PARAM_VM_BIND_VERSION   57
> > > > > +
> > > > > +/**
> > > > > + * DOC: I915_VM_CREATE_FLAGS_USE_VM_BIND
> > > > > + *
> > > > > + * Flag to opt-in for VM_BIND mode of binding during VM creation.
> > > > > + * See struct drm_i915_gem_vm_control flags.
> > > > > + *
> > > > > + * The older execbuf2 ioctl will not support VM_BIND mode of 
> > > > > operation.
> > > > > + * For VM_BIND mode, we have new execbuf3 ioctl which will not 
> > > > > accept any
> > > > > + * execlist (See struct drm_i915_gem_execbuffer3 for more details).
> > > > > + */
> > > > > +#define I915_VM_CREATE_FLAGS_USE_VM_BIND (1 << 0)
> > > > > +
> > > > > +/* VM_BIND related ioctls */
> > > > > +#d

Re: [PATCH v6 3/3] drm/doc/rfc: VM_BIND uapi definition

[Zanoni, Paulo R]

On Wed, 2022-06-29 at 23:08 -0700, Niranjana Vishwanathapura wrote:
> On Wed, Jun 29, 2022 at 05:33:49PM -0700, Zanoni, Paulo R wrote:
> > On Sat, 2022-06-25 at 18:49 -0700, Niranjana Vishwanathapura wrote:
> > > VM_BIND and related uapi definitions
> > > 
> > > v2: Reduce the scope to simple Mesa use case.
> > > v3: Expand VM_UNBIND documentation and add
> > > I915_GEM_VM_BIND/UNBIND_FENCE_VALID
> > > and I915_GEM_VM_BIND_TLB_FLUSH flags.
> > > v4: Remove I915_GEM_VM_BIND_TLB_FLUSH flag and add additional
> > > documentation for vm_bind/unbind.
> > > v5: Remove TLB flush requirement on VM_UNBIND.
> > > Add version support to stage implementation.
> > > v6: Define and use drm_i915_gem_timeline_fence structure for
> > > all timeline fences.
> > > v7: Rename I915_PARAM_HAS_VM_BIND to I915_PARAM_VM_BIND_VERSION.
> > > Update documentation on async vm_bind/unbind and versioning.
> > > Remove redundant vm_bind/unbind FENCE_VALID flag, execbuf3
> > > batch_count field and I915_EXEC3_SECURE flag.
> > > 
> > > Signed-off-by: Niranjana Vishwanathapura 
> > > 
> > > Reviewed-by: Daniel Vetter 
> > > ---
> > >  Documentation/gpu/rfc/i915_vm_bind.h | 280 +++
> > >  1 file changed, 280 insertions(+)
> > >  create mode 100644 Documentation/gpu/rfc/i915_vm_bind.h
> > > 
> > > diff --git a/Documentation/gpu/rfc/i915_vm_bind.h 
> > > b/Documentation/gpu/rfc/i915_vm_bind.h
> > > new file mode 100644
> > > index ..a93e08bceee6
> > > --- /dev/null
> > > +++ b/Documentation/gpu/rfc/i915_vm_bind.h
> > > @@ -0,0 +1,280 @@
> > > +/* SPDX-License-Identifier: MIT */
> > > +/*
> > > + * Copyright © 2022 Intel Corporation
> > > + */
> > > +
> > > +/**
> > > + * DOC: I915_PARAM_VM_BIND_VERSION
> > > + *
> > > + * VM_BIND feature version supported.
> > > + * See typedef drm_i915_getparam_t param.
> > > + *
> > > + * Specifies the VM_BIND feature version supported.
> > > + * The following versions of VM_BIND have been defined:
> > > + *
> > > + * 0: No VM_BIND support.
> > > + *
> > > + * 1: In VM_UNBIND calls, the UMD must specify the exact mappings created
> > > + *previously with VM_BIND, the ioctl will not support unbinding 
> > > multiple
> > > + *mappings or splitting them. Similarly, VM_BIND calls will not 
> > > replace
> > > + *any existing mappings.
> > > + *
> > > + * 2: The restrictions on unbinding partial or multiple mappings is
> > > + *lifted, Similarly, binding will replace any mappings in the given 
> > > range.
> > > + *
> > > + * See struct drm_i915_gem_vm_bind and struct drm_i915_gem_vm_unbind.
> > > + */
> > > +#define I915_PARAM_VM_BIND_VERSION   57
> > > +
> > > +/**
> > > + * DOC: I915_VM_CREATE_FLAGS_USE_VM_BIND
> > > + *
> > > + * Flag to opt-in for VM_BIND mode of binding during VM creation.
> > > + * See struct drm_i915_gem_vm_control flags.
> > > + *
> > > + * The older execbuf2 ioctl will not support VM_BIND mode of operation.
> > > + * For VM_BIND mode, we have new execbuf3 ioctl which will not accept any
> > > + * execlist (See struct drm_i915_gem_execbuffer3 for more details).
> > > + */
> > > +#define I915_VM_CREATE_FLAGS_USE_VM_BIND (1 << 0)
> > > +
> > > +/* VM_BIND related ioctls */
> > > +#define DRM_I915_GEM_VM_BIND 0x3d
> > > +#define DRM_I915_GEM_VM_UNBIND   0x3e
> > > +#define DRM_I915_GEM_EXECBUFFER3 0x3f
> > > +
> > > +#define DRM_IOCTL_I915_GEM_VM_BIND   DRM_IOWR(DRM_COMMAND_BASE + 
> > > DRM_I915_GEM_VM_BIND, struct drm_i915_gem_vm_bind)
> > > +#define DRM_IOCTL_I915_GEM_VM_UNBIND DRM_IOWR(DRM_COMMAND_BASE + 
> > > DRM_I915_GEM_VM_UNBIND, struct drm_i915_gem_vm_bind)
> > > +#define DRM_IOCTL_I915_GEM_EXECBUFFER3   
> > > DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_EXECBUFFER3, struct 
> > > drm_i915_gem_execbuffer3)
> > > +
> > > +/**
> > > + * struct drm_i915_gem_timeline_fence - An input or output timeline 
> > > fence.
> > > + *
> > > + * The operation will wait for input fence to signal.
> > > + *
> > > + * The returned output fence will be signaled after the completion of the
> > > + 

Re: [PATCH v6 1/3] drm/doc/rfc: VM_BIND feature design document

[Zanoni, Paulo R]

On Sat, 2022-06-25 at 18:49 -0700, Niranjana Vishwanathapura wrote:
> VM_BIND design document with description of intended use cases.
> 
> v2: Reduce the scope to simple Mesa use case.
> v3: Expand documentation on dma-resv usage, TLB flushing and
> execbuf3.
> v4: Remove vm_bind tlb flush request support.
> v5: Update TLB flushing documentation.
> v6: Update out of order completion documentation.
> 
> Signed-off-by: Niranjana Vishwanathapura 
> ---
>  Documentation/gpu/rfc/i915_vm_bind.rst | 246 +
>  Documentation/gpu/rfc/index.rst|   4 +
>  2 files changed, 250 insertions(+)
>  create mode 100644 Documentation/gpu/rfc/i915_vm_bind.rst
> 
> diff --git a/Documentation/gpu/rfc/i915_vm_bind.rst 
> b/Documentation/gpu/rfc/i915_vm_bind.rst
> new file mode 100644
> index ..032ee32b885c
> --- /dev/null
> +++ b/Documentation/gpu/rfc/i915_vm_bind.rst
> @@ -0,0 +1,246 @@
> +==
> +I915 VM_BIND feature design and use cases
> +==
> +
> +VM_BIND feature
> +
> +DRM_I915_GEM_VM_BIND/UNBIND ioctls allows UMD to bind/unbind GEM buffer
> +objects (BOs) or sections of a BOs at specified GPU virtual addresses on a
> +specified address space (VM). These mappings (also referred to as persistent
> +mappings) will be persistent across multiple GPU submissions (execbuf calls)
> +issued by the UMD, without user having to provide a list of all required
> +mappings during each submission (as required by older execbuf mode).
> +
> +The VM_BIND/UNBIND calls allow UMDs to request a timeline out fence for
> +signaling the completion of bind/unbind operation.
> +
> +VM_BIND feature is advertised to user via I915_PARAM_HAS_VM_BIND.

I915_PARAM_VM_BIND_VERSION


> +User has to opt-in for VM_BIND mode of binding for an address space (VM)
> +during VM creation time via I915_VM_CREATE_FLAGS_USE_VM_BIND extension.
> +
> +VM_BIND/UNBIND ioctl calls executed on different CPU threads concurrently are
> +not ordered. Furthermore, parts of the VM_BIND/UNBIND operations can be done
> +asynchronously, when valid out fence is specified.
> +
> +VM_BIND features include:
> +
> +* Multiple Virtual Address (VA) mappings can map to the same physical pages
> +  of an object (aliasing).
> +* VA mapping can map to a partial section of the BO (partial binding).
> +* Support capture of persistent mappings in the dump upon GPU error.
> +* Support for userptr gem objects (no special uapi is required for this).
> +
> +TLB flush consideration
> +
> +The i915 driver flushes the TLB for each submission and when an object's
> +pages are released. The VM_BIND/UNBIND operation will not do any additional
> +TLB flush. Any VM_BIND mapping added will be in the working set for 
> subsequent
> +submissions on that VM and will not be in the working set for currently 
> running
> +batches (which would require additional TLB flushes, which is not supported).
> +
> +Execbuf ioctl in VM_BIND mode
> +---
> +A VM in VM_BIND mode will not support older execbuf mode of binding.
> +The execbuf ioctl handling in VM_BIND mode differs significantly from the
> +older execbuf2 ioctl (See struct drm_i915_gem_execbuffer2).
> +Hence, a new execbuf3 ioctl has been added to support VM_BIND mode. (See
> +struct drm_i915_gem_execbuffer3). The execbuf3 ioctl will not accept any
> +execlist. Hence, no support for implicit sync. It is expected that the below
> +work will be able to support requirements of object dependency setting in all
> +use cases:
> +
> +"dma-buf: Add an API for exporting sync files"
> +(https://lwn.net/Articles/859290/)
> +
> +The new execbuf3 ioctl only works in VM_BIND mode and the VM_BIND mode only
> +works with execbuf3 ioctl for submission. All BOs mapped on that VM (through
> +VM_BIND call) at the time of execbuf3 call are deemed required for that
> +submission.
> +
> +The execbuf3 ioctl directly specifies the batch addresses instead of as
> +object handles as in execbuf2 ioctl. The execbuf3 ioctl will also not
> +support many of the older features like in/out/submit fences, fence array,
> +default gem context and many more (See struct drm_i915_gem_execbuffer3).

Just as a note: both Iris and Vulkan use some of these features, so
some rework will be required. From what I can see, all current behavior
we depend on will be supported in some way or another, so hopefully
we'll be fine.


> +
> +In VM_BIND mode, VA allocation is completely managed by the user instead of
> +the i915 driver. Hence all VA assignment, eviction are not applicable in
> +VM_BIND mode. Also, for determining object activeness, VM_BIND mode will not
> +be using the i915_vma active reference tracking. It will instead use dma-resv
> +object for that (See `VM_BIND dma_resv usage`_).
> +
> +So, a lot of existing code supporting execbuf2 ioctl, like relocations, VA
> +evictions, vma lookup table, implicit sync, vma 

Re: [PATCH v6 3/3] drm/doc/rfc: VM_BIND uapi definition

[Zanoni, Paulo R]

On Sat, 2022-06-25 at 18:49 -0700, Niranjana Vishwanathapura wrote:
> VM_BIND and related uapi definitions
> 
> v2: Reduce the scope to simple Mesa use case.
> v3: Expand VM_UNBIND documentation and add
> I915_GEM_VM_BIND/UNBIND_FENCE_VALID
> and I915_GEM_VM_BIND_TLB_FLUSH flags.
> v4: Remove I915_GEM_VM_BIND_TLB_FLUSH flag and add additional
> documentation for vm_bind/unbind.
> v5: Remove TLB flush requirement on VM_UNBIND.
> Add version support to stage implementation.
> v6: Define and use drm_i915_gem_timeline_fence structure for
> all timeline fences.
> v7: Rename I915_PARAM_HAS_VM_BIND to I915_PARAM_VM_BIND_VERSION.
> Update documentation on async vm_bind/unbind and versioning.
> Remove redundant vm_bind/unbind FENCE_VALID flag, execbuf3
> batch_count field and I915_EXEC3_SECURE flag.
> 
> Signed-off-by: Niranjana Vishwanathapura 
> Reviewed-by: Daniel Vetter 
> ---
>  Documentation/gpu/rfc/i915_vm_bind.h | 280 +++
>  1 file changed, 280 insertions(+)
>  create mode 100644 Documentation/gpu/rfc/i915_vm_bind.h
> 
> diff --git a/Documentation/gpu/rfc/i915_vm_bind.h 
> b/Documentation/gpu/rfc/i915_vm_bind.h
> new file mode 100644
> index ..a93e08bceee6
> --- /dev/null
> +++ b/Documentation/gpu/rfc/i915_vm_bind.h
> @@ -0,0 +1,280 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +/**
> + * DOC: I915_PARAM_VM_BIND_VERSION
> + *
> + * VM_BIND feature version supported.
> + * See typedef drm_i915_getparam_t param.
> + *
> + * Specifies the VM_BIND feature version supported.
> + * The following versions of VM_BIND have been defined:
> + *
> + * 0: No VM_BIND support.
> + *
> + * 1: In VM_UNBIND calls, the UMD must specify the exact mappings created
> + *previously with VM_BIND, the ioctl will not support unbinding multiple
> + *mappings or splitting them. Similarly, VM_BIND calls will not replace
> + *any existing mappings.
> + *
> + * 2: The restrictions on unbinding partial or multiple mappings is
> + *lifted, Similarly, binding will replace any mappings in the given 
> range.
> + *
> + * See struct drm_i915_gem_vm_bind and struct drm_i915_gem_vm_unbind.
> + */
> +#define I915_PARAM_VM_BIND_VERSION   57
> +
> +/**
> + * DOC: I915_VM_CREATE_FLAGS_USE_VM_BIND
> + *
> + * Flag to opt-in for VM_BIND mode of binding during VM creation.
> + * See struct drm_i915_gem_vm_control flags.
> + *
> + * The older execbuf2 ioctl will not support VM_BIND mode of operation.
> + * For VM_BIND mode, we have new execbuf3 ioctl which will not accept any
> + * execlist (See struct drm_i915_gem_execbuffer3 for more details).
> + */
> +#define I915_VM_CREATE_FLAGS_USE_VM_BIND (1 << 0)
> +
> +/* VM_BIND related ioctls */
> +#define DRM_I915_GEM_VM_BIND 0x3d
> +#define DRM_I915_GEM_VM_UNBIND   0x3e
> +#define DRM_I915_GEM_EXECBUFFER3 0x3f
> +
> +#define DRM_IOCTL_I915_GEM_VM_BIND   DRM_IOWR(DRM_COMMAND_BASE + 
> DRM_I915_GEM_VM_BIND, struct drm_i915_gem_vm_bind)
> +#define DRM_IOCTL_I915_GEM_VM_UNBIND DRM_IOWR(DRM_COMMAND_BASE + 
> DRM_I915_GEM_VM_UNBIND, struct drm_i915_gem_vm_bind)
> +#define DRM_IOCTL_I915_GEM_EXECBUFFER3   
> DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_EXECBUFFER3, struct 
> drm_i915_gem_execbuffer3)
> +
> +/**
> + * struct drm_i915_gem_timeline_fence - An input or output timeline fence.
> + *
> + * The operation will wait for input fence to signal.
> + *
> + * The returned output fence will be signaled after the completion of the
> + * operation.
> + */
> +struct drm_i915_gem_timeline_fence {
> + /** @handle: User's handle for a drm_syncobj to wait on or signal. */
> + __u32 handle;
> +
> + /**
> +  * @flags: Supported flags are:
> +  *
> +  * I915_TIMELINE_FENCE_WAIT:
> +  * Wait for the input fence before the operation.
> +  *
> +  * I915_TIMELINE_FENCE_SIGNAL:
> +  * Return operation completion fence as output.
> +  */
> + __u32 flags;
> +#define I915_TIMELINE_FENCE_WAIT(1 << 0)
> +#define I915_TIMELINE_FENCE_SIGNAL  (1 << 1)
> +#define __I915_TIMELINE_FENCE_UNKNOWN_FLAGS (-(I915_TIMELINE_FENCE_SIGNAL << 
> 1))
> +
> + /**
> +  * @value: A point in the timeline.
> +  * Value must be 0 for a binary drm_syncobj. A Value of 0 for a
> +  * timeline drm_syncobj is invalid as it turns a drm_syncobj into a
> +  * binary one.
> +  */
> + __u64 value;
> +};
> +
> +/**
> + * struct drm_i915_gem_vm_bind - VA to object mapping to bind.
> + *
> + * This structure is passed to VM_BIND ioctl and specifies the mapping of GPU
> + * virtual address (VA) range to the section of an object that should be 
> bound
> + * in the device page table of the specified address space (VM).
> + * The VA range specified must be unique (ie., not currently bound) and can
> + * be mapped to whole object or a section of the object (partial binding).
> + * Multiple 

Re: [Intel-gfx] [RFC v3 3/3] drm/doc/rfc: VM_BIND uapi definition

[Zanoni, Paulo R]

On Tue, 2022-05-17 at 11:32 -0700, Niranjana Vishwanathapura wrote:
> VM_BIND and related uapi definitions
> 
> v2: Ensure proper kernel-doc formatting with cross references.
> Also add new uapi and documentation as per review comments
> from Daniel.
> 
> Signed-off-by: Niranjana Vishwanathapura 
> ---
>  Documentation/gpu/rfc/i915_vm_bind.h | 399 +++
>  1 file changed, 399 insertions(+)
>  create mode 100644 Documentation/gpu/rfc/i915_vm_bind.h
> 
> diff --git a/Documentation/gpu/rfc/i915_vm_bind.h 
> b/Documentation/gpu/rfc/i915_vm_bind.h
> new file mode 100644
> index ..589c0a009107
> --- /dev/null
> +++ b/Documentation/gpu/rfc/i915_vm_bind.h
> @@ -0,0 +1,399 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +/**
> + * DOC: I915_PARAM_HAS_VM_BIND
> + *
> + * VM_BIND feature availability.
> + * See typedef drm_i915_getparam_t param.
> + */
> +#define I915_PARAM_HAS_VM_BIND   57
> +
> +/**
> + * DOC: I915_VM_CREATE_FLAGS_USE_VM_BIND
> + *
> + * Flag to opt-in for VM_BIND mode of binding during VM creation.
> + * See struct drm_i915_gem_vm_control flags.
> + *
> + * A VM in VM_BIND mode will not support the older execbuff mode of binding.
> + * In VM_BIND mode, execbuff ioctl will not accept any execlist (ie., the
> + * _i915_gem_execbuffer2.buffer_count must be 0).
> + * Also, _i915_gem_execbuffer2.batch_start_offset and
> + * _i915_gem_execbuffer2.batch_len must be 0.
> + * DRM_I915_GEM_EXECBUFFER_EXT_BATCH_ADDRESSES extension must be provided
> + * to pass in the batch buffer addresses.
> + *
> + * Additionally, I915_EXEC_NO_RELOC, I915_EXEC_HANDLE_LUT and
> + * I915_EXEC_BATCH_FIRST of _i915_gem_execbuffer2.flags must be 0
> + * (not used) in VM_BIND mode. I915_EXEC_USE_EXTENSIONS flag must always be
> + * set (See struct drm_i915_gem_execbuffer_ext_batch_addresses).
> + * The buffers_ptr, buffer_count, batch_start_offset and batch_len fields
> + * of struct drm_i915_gem_execbuffer2 are also not used and must be 0.
> + */

From that description, it seems we have:

struct drm_i915_gem_execbuffer2 {
__u64 buffers_ptr;  -> must be 0 (new)
__u32 buffer_count; -> must be 0 (new)
__u32 batch_start_offset;   -> must be 0 (new)
__u32 batch_len;-> must be 0 (new)
__u32 DR1;  -> must be 0 (old)
__u32 DR4;  -> must be 0 (old)
__u32 num_cliprects; (fences)   -> must be 0 since using extensions
__u64 cliprects_ptr; (fences, extensions) -> contains an actual pointer!
__u64 flags;-> some flags must be 0 (new)
__u64 rsvd1; (context info) -> repurposed field (old)
__u64 rsvd2;-> unused
};

Based on that, why can't we just get drm_i915_gem_execbuffer3 instead
of adding even more complexity to an already abused interface? While
the Vulkan-like extension thing is really nice, I don't think what
we're doing here is extending the ioctl usage, we're completely
changing how the base struct should be interpreted based on how the VM
was created (which is an entirely different ioctl).

From Rusty Russel's API Design grading, drm_i915_gem_execbuffer2 is
already at -6 without these changes. I think after vm_bind we'll need
to create a -11 entry just to deal with this ioctl.


+#define I915_VM_CREATE_FLAGS_USE_VM_BIND   (1 << 0)
+
+/**
+ * DOC: I915_CONTEXT_CREATE_FLAGS_LONG_RUNNING
+ *
+ * Flag to declare context as long running.
+ * See struct drm_i915_gem_context_create_ext flags.
+ *
+ * Usage of dma-fence expects that they complete in reasonable amount of time.
+ * Compute on the other hand can be long running. Hence it is not appropriate
+ * for compute contexts to export request completion dma-fence to user.
+ * The dma-fence usage will be limited to in-kernel consumption only.
+ * Compute contexts need to use user/memory fence.
+ *
+ * So, long running contexts do not support output fences. Hence,
+ * I915_EXEC_FENCE_OUT (See _i915_gem_execbuffer2.flags and
+ * I915_EXEC_FENCE_SIGNAL (See _i915_gem_exec_fence.flags) are expected
+ * to be not used.
+ *
+ * DRM_I915_GEM_WAIT ioctl call is also not supported for objects mapped
+ * to long running contexts.
+ */
+#define I915_CONTEXT_CREATE_FLAGS_LONG_RUNNING   (1u << 2)
+
+/* VM_BIND related ioctls */
+#define DRM_I915_GEM_VM_BIND   0x3d
+#define DRM_I915_GEM_VM_UNBIND 0x3e
+#define DRM_I915_GEM_WAIT_USER_FENCE   0x3f
+
+#define DRM_IOCTL_I915_GEM_VM_BIND DRM_IOWR(DRM_COMMAND_BASE + 
DRM_I915_GEM_VM_BIND, struct drm_i915_gem_vm_bind)
+#define DRM_IOCTL_I915_GEM_VM_UNBIND   DRM_IOWR(DRM_COMMAND_BASE + 
DRM_I915_GEM_VM_UNBIND, struct drm_i915_gem_vm_bind)
+#define DRM_IOCTL_I915_GEM_WAIT_USER_FENCE DRM_IOWR(DRM_COMMAND_BASE + 
DRM_I915_GEM_WAIT_USER_FENCE, struct drm_i915_gem_wait_user_fence)
+
+/**
+ * struct 

Re: [RFC v3 1/3] drm/doc/rfc: VM_BIND feature design document

[Zanoni, Paulo R]

On Tue, 2022-05-17 at 11:32 -0700, Niranjana Vishwanathapura wrote:
> VM_BIND design document with description of intended use cases.
> 
> v2: Add more documentation and format as per review comments
> from Daniel.
> 
> Signed-off-by: Niranjana Vishwanathapura 
> ---
> 
> diff --git a/Documentation/gpu/rfc/i915_vm_bind.rst 
> b/Documentation/gpu/rfc/i915_vm_bind.rst
> new file mode 100644
> index ..f1be560d313c
> --- /dev/null
> +++ b/Documentation/gpu/rfc/i915_vm_bind.rst
> @@ -0,0 +1,304 @@
> +==
> +I915 VM_BIND feature design and use cases
> +==
> +
> +VM_BIND feature
> +
> +DRM_I915_GEM_VM_BIND/UNBIND ioctls allows UMD to bind/unbind GEM buffer
> +objects (BOs) or sections of a BOs at specified GPU virtual addresses on a
> +specified address space (VM). These mappings (also referred to as persistent
> +mappings) will be persistent across multiple GPU submissions (execbuff calls)
> +issued by the UMD, without user having to provide a list of all required
> +mappings during each submission (as required by older execbuff mode).
> +
> +VM_BIND/UNBIND ioctls will support 'in' and 'out' fences to allow userpace
> +to specify how the binding/unbinding should sync with other operations
> +like the GPU job submission. These fences will be timeline 'drm_syncobj's
> +for non-Compute contexts (See struct drm_i915_vm_bind_ext_timeline_fences).
> +For Compute contexts, they will be user/memory fences (See struct
> +drm_i915_vm_bind_ext_user_fence).
> +
> +VM_BIND feature is advertised to user via I915_PARAM_HAS_VM_BIND.
> +User has to opt-in for VM_BIND mode of binding for an address space (VM)
> +during VM creation time via I915_VM_CREATE_FLAGS_USE_VM_BIND extension.
> +
> +VM_BIND/UNBIND ioctl will immediately start binding/unbinding the mapping in 
> an
> +async worker. The binding and unbinding will work like a special GPU engine.
> +The binding and unbinding operations are serialized and will wait on 
> specified
> +input fences before the operation and will signal the output fences upon the
> +completion of the operation. Due to serialization, completion of an operation
> +will also indicate that all previous operations are also complete.
> +
> +VM_BIND features include:
> +
> +* Multiple Virtual Address (VA) mappings can map to the same physical pages
> +  of an object (aliasing).
> +* VA mapping can map to a partial section of the BO (partial binding).
> +* Support capture of persistent mappings in the dump upon GPU error.
> +* TLB is flushed upon unbind completion. Batching of TLB flushes in some
> +  use cases will be helpful.
> +* Asynchronous vm_bind and vm_unbind support with 'in' and 'out' fences.
> +* Support for userptr gem objects (no special uapi is required for this).
> +
> +Execbuff ioctl in VM_BIND mode
> +---
> +The execbuff ioctl handling in VM_BIND mode differs significantly from the
> +older method. A VM in VM_BIND mode will not support older execbuff mode of
> +binding. In VM_BIND mode, execbuff ioctl will not accept any execlist. Hence,
> +no support for implicit sync. It is expected that the below work will be able
> +to support requirements of object dependency setting in all use cases:
> +
> +"dma-buf: Add an API for exporting sync files"
> +(https://lwn.net/Articles/859290/)

I would really like to have more details here. The link provided points
to new ioctls and we're not very familiar with those yet, so I think
you should really clarify the interaction between the new additions
here. Having some sample code would be really nice too.

For Mesa at least (and I believe for the other drivers too) we always
have a few exported buffers in every execbuf call, and we rely on the
implicit synchronization provided by execbuf to make sure everything
works. The execbuf ioctl also has some code to flush caches during
implicit synchronization AFAIR, so I would guess we rely on it too and
whatever else the Kernel does. Is that covered by the new ioctls?

In addition, as far as I remember, one of the big improvements of
vm_bind was that it would help reduce ioctl latency and cpu overhead.
But if making execbuf faster comes at the cost of requiring additional
ioctls calls for implicit synchronization, which is required on ever
execbuf call, then I wonder if we'll even get any faster at all.
Comparing old execbuf vs plain new execbuf without the new required
ioctls won't make sense.

But maybe I'm wrong and we won't need to call these new ioctls around
every single execbuf ioctl we submit? Again, more clarification and
some code examples here would be really nice. This is a big change on
an important part of the API, we should clarify the new expected usage.

> +
> +This also means, we need an execbuff extension to pass in the batch
> +buffer addresses (See struct drm_i915_gem_execbuffer_ext_batch_addresses).
> +
> +If at all execlist support in execbuff ioctl 

[Intel-gfx] Skylake graphics regression: projector failure with 4.8-rc3

[Zanoni, Paulo R]

Hi

Em Qua, 2016-08-31 às 14:43 -0700, James Bottomley escreveu:
> On Wed, 2016-08-31 at 11:23 -0700, James Bottomley wrote:
> > 
> > On Fri, 2016-08-26 at 09:10 -0400, James Bottomley wrote:
> > > 
> > > We seem to have an xrandr regression with skylake now.  What's
> > > happening is that I can get output on to a projector, but the 
> > > system is losing video when I change the xrandr sessions (like 
> > > going from a --above b to a --same-as b).  The main screen goes 
> > > blank, which is basically a reboot situation.  Unfortunately, I 
> > > can't seem to get the logs out of systemd to see if there was a 
> > > dump to dmesg (the system was definitely responding).
> > > 
> > > I fell back to 4.6.2 which worked perfectly, so this is
> > > definitely 
> > > some sort of regression.  I'll be able to debug more fully when
> > > I 
> > > get back home from the Linux Security Summit.
> > 
> > I'm home now.  Unfortunately, my monitor isn't as problematic as
> > the
> > projector, but by flipping between various modes and separating and
> > overlaying the panels with --above and --same-as (xrandr), I can
> > eventually get it to the point where the main LCD panel goes black 
> > and can only be restarted by specifying a different mode.
> > 
> > This seems to be associated with these lines in the X
> > 
> > [ 14714.389] (EE) intel(0): failed to set mode: Invalid argument
> > [22]
> > 
> > But the curious thing is that even if this fails with the error 
> > message once, it may succeed a second time, so it looks to be a 
> > transient error translation problem from the kernel driver.
> > 
> > I've attached the full log below.
> > 
> > This is only with a VGA output.  I currently don't have a HDMI 
> > dongle, but I'm in the process of acquiring one.
> 
> After more playing around, I'm getting thousands of these in the
> kernel
> log (possibly millions: the log wraps very fast):
> 
> [23504.873606] [drm:intel_dp_start_link_train [i915]] *ERROR* failed
> to train DP, aborting
> 
> And then finally it gives up with 
> 
> [25023.770951] [drm:intel_cpu_fifo_underrun_irq_handler [i915]]
> *ERROR* CPU pipe B FIFO underrun
> [25561.926075] [drm:intel_cpu_fifo_underrun_irq_handler [i915]]
> *ERROR* CPU pipe A FIFO underrun
> 
> And the crtc for the VGA output becomes non-responsive to any
> configuration command.  This requires a reboot and sometimes a UEFI
> variable reset before it comes back.

Please see this discussion:
https://patchwork.freedesktop.org/patch/103237/

Do you have this patch on your tree? Does the problem go away if you
revert it?

Thanks,
Paulo

> 
> James
> 
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v10] drm/i915/skl: Add support for the SAGV, fix underrun hangs

[Zanoni, Paulo R]

Em Ter, 2016-08-09 às 14:44 +0200, Maarten Lankhorst escreveu:
> Hey,
> 
> Op 08-08-16 om 23:03 schreef Lyude:
> > 
> > Since the watermark calculations for Skylake are still broken,
> > we're apt
> > to hitting underruns very easily under multi-monitor
> > configurations.
> > While it would be lovely if this was fixed, it's not. Another
> > problem
> > that's been coming from this however, is the mysterious issue of
> > underruns causing full system hangs. An easy way to reproduce this
> > with
> > a skylake system:
> > 
> > - Get a laptop with a skylake GPU, and hook up two external
> > monitors to
> >   it
> > - Move the cursor from the built-in LCD to one of the external
> > displays
> >   as quickly as you can
> > - You'll get a few pipe underruns, and eventually the entire system
> > will
> >   just freeze.
> > 
> > After doing a lot of investigation and reading through the bspec, I
> > found the existence of the SAGV, which is responsible for adjusting
> > the
> > system agent voltage and clock frequencies depending on how much
> > power
> > we need. According to the bspec:
> > 
> > "The display engine access to system memory is blocked during the
> >  adjustment time. SAGV defaults to enabled. Software must use the
> >  GT-driver pcode mailbox to disable SAGV when the display engine is
> > not
> >  able to tolerate the blocking time."
> > 
> > The rest of the bspec goes on to explain that software can simply
> > leave
> > the SAGV enabled, and disable it when we use interlaced pipes/have
> > more
> > then one pipe active.
> > 
> > Sure enough, with this patchset the system hangs resulting from
> > pipe
> > underruns on Skylake have completely vanished on my T460s.
> > Additionally,
> > the bspec mentions turning off the SAGV with more then one
> > pipe enabled
> > as a workaround for display underruns. While this patch doesn't
> > entirely
> > fix that, it looks like it does improve the situation a little bit
> > so
> > it's likely this is going to be required to make watermarks on
> > Skylake
> > fully functional.
> 
> I think this patch goes with v9 6/6 and v8 2-5/6. If you're only
> updating a single patch it might be better to send it in reply to the
> original patch.
> 
> I'm testing the whole series on my prerelease skylake, and running
> into this:
> 
> [ 2794.933149] kms_cursor_legacy: starting subtest 2x-flip-vs-cursor-
> legacy
> [ 2795.813970] [drm:skl_disable_sagv [i915]] *ERROR* Request to
> disable SAGV timed out
> 
> Value returned from skl_do_sagv_disable is always 0 for me, even when
> I bump the timeout to 15.

Yesterday I started testing this series, and I also noticed some visual
corruption: while browsing moderately-heavy websites on a maximized
Firefox, I could see the desktop background sort of "blinking" in the
screen (the background was not supposed to be visible). It looks like
the problem was introduced by patch 4, but I can't be 100% sure since
sometimes it's a little harder to reproduce it. Still, this is better
than the current "X doesn't work" state that we have without the
series.

> 
> ~Maarten
> 
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] i915/fbc: Disable on HSW by default for now

[Zanoni, Paulo R]

Em Sex, 2016-06-10 às 08:44 +0200, Stefan Richter escreveu:
> On Jun 09 Zanoni, Paulo R wrote:
> > 
> > Em Qui, 2016-06-09 às 11:58 -0400, Lyude escreveu:
> > > 
> > > From https://bugs.freedesktop.org/show_bug.cgi?id=96461 :
> > > 
> > > This was kind of a difficult bug to track down. If you're using a
> > > Haswell system running GNOME and you have fbc completely enabled
> > > and
> > > working, playing videos can result in video artifacts.
> [...]
> > 
> > > 
> > > For the time being, disable FBC for Haswell by default.  
> > In case we want to improve the commit message:
> > 
> > We also got reports from Steven Honeyman on openbox+roxterm, and
> > from
> > Stefan Richter.
> Perhaps also worth mentioning in the changelog:
> Stefan Richter reported kernel freezes (no video artifacts) when fbc
> is on.
> (E3-1245 v3 with HD P4600; openbox and some KDE and LXDE
> applications,
> thread begins at https://lkml.org/lkml/2016/4/26/813)

Patch merged with small additions to the commit message.

Thanks everybody involved,
Paulo

[PATCH] i915/fbc: Disable on HSW by default for now

[Zanoni, Paulo R]

Em Qui, 2016-06-09 às 11:58 -0400, Lyude escreveu:
> From https://bugs.freedesktop.org/show_bug.cgi?id=96461 :
> 
> This was kind of a difficult bug to track down. If you're using a
> Haswell system running GNOME and you have fbc completely enabled and
> working, playing videos can result in video artifacts. Steps to
> reproduce:
> 
> - Run GNOME
> - Ensure FBC is enabled and active
> - Download a movie, I used the ogg version of Big Buck Bunny for this
> - Run `gst-launch-1.0 filesrc location='some_movie.ogg' ! decodebin !
>   glimagesink` in a terminal
> - Watch for about over a minute, you'll see small horizontal lines go
>   down the screen.

If you "dmesg | grep -i underrun", do you see anything (even if it
doesn't happen during the moments of corruption)?

Does the problem still happen if you apply these patches?
 - https://patchwork.freedesktop.org/patch/79567/
 - https://patchwork.freedesktop.org/patch/80857/
 - https://patchwork.freedesktop.org/patch/92634/

> 
> For the time being, disable FBC for Haswell by default.

In case we want to improve the commit message:

We also got reports from Steven Honeyman on openbox+roxterm, and from
Stefan Richter.

Anyway, there's a regression that needs to be fixed, so:

Reviewed-by: Paulo Zanoni 

We can always try again later.

> 
> Signed-off-by: Lyude 
> Cc: stable at vger.kernel.org
> ---
>  drivers/gpu/drm/i915/intel_fbc.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c
> b/drivers/gpu/drm/i915/intel_fbc.c
> index 0f0492f..28f4407 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -823,8 +823,7 @@ static bool intel_fbc_can_choose(struct
> intel_crtc *crtc)
>  {
>  struct drm_i915_private *dev_priv = crtc->base.dev-
> >dev_private;
>  struct intel_fbc *fbc = _priv->fbc;
> - bool enable_by_default = IS_HASWELL(dev_priv) ||
> -  IS_BROADWELL(dev_priv);
> + bool enable_by_default = IS_BROADWELL(dev_priv);
>  
>  if (intel_vgpu_active(dev_priv->dev)) {
>  fbc->no_fbc_reason = "VGPU is active";

Regression of v4.6-rc vs. v4.5 bisected: a98ee79317b4 "drm/i915/fbc: enable FBC by default on HSW and BDW"

[Zanoni, Paulo R]

Em Sex, 2016-05-06 às 00:54 +0200, Stefan Richter escreveu:
> On May 05 Zanoni, Paulo R wrote:
> > 
> > Em Qui, 2016-05-05 às 19:45 +0200, Stefan Richter escreveu:
> > > 
> > >     Oh, and in case you - the person reading this commit message
> > > - found
> > >     this commit through git bisect, please do the following:
> > >      - Check your dmesg and see if there are error messages
> > > mentioning
> > >        underruns around the time your problem started happening.
> > > 
> > > Well, I always had the followings lines in dmesg:
> > > [drm:intel_set_cpu_fifo_underrun_reporting] *ERROR* uncleared
> > > fifo underrun on pipe A
> > > [drm:intel_cpu_fifo_underrun_irq_handler] *ERROR* CPU pipe A FIFO
> > > underrun  
> > Oh, well... I had a patch that would just disable FBC in case we
> > saw a
> > FIFO underrun, but it was rejected. Maybe this is the time to think
> > about it again? Otherwise, I can't think of much besides disabling
> > FBC
> > on HSW until all the underruns and watermarks regressions are fixed
> > forever.
> Just to be clear though, I know that these messages are emitted when
> the
> monitor is switched on, and when sddm is being shut down --- but I do
> not
> know whether there is any sort of underrun when I get the FBC related
> freeze (since I just don't get any kernel messages at that point).

The fact that underruns have occurred earlier is enough to know that
something is wrong (most probably, bad watermarks): we stop reporting
underruns once we get the first one. In addition, we already know that
FBC has the tendency to amplify apparently-harmless FIFO underruns into
black screens, and I wouldn't be surprised to learn that it could also
cause full machine lockups.

> 
> Is there a chance that a serial console would fare better than
> netconsole?  This board and another PC in its vicinity have got
> onboard
> serial ports but I don't have cables at the moment.

In the past, for some specific cases not related to FBC, I had more
luck with serial console than with netconsole. But if this is really
caused by FBC and watermarks, I don't think you'll be able to grab any
specific message at the time of the machine hang. OTOH, if something
actually shows up, it could help invalidate our current assumption of
the relationship between the problem and FBC and underruns.

> 
> > 
> > > 
> > >      - Download intel-gpu-tools, compile it, and run:
> > >        $ sudo ./tests/kms_frontbuffer_tracking --run-subtest
> > > '*fbc-*' 2>&1 | tee fbc.txt  
> > >        Then send us the fbc.txt file, especially if you get a
> > > failure.
> > >        This will really maximize your chances of getting the bug
> > > fixed
> > >        quickly.
> > > 
> > > Do you need this while FBC is enabled, or can I run it while FBC
> > > is
> > > disabled?  
> > FBC enabled. Considering your description, my hope is that maybe
> > some
> > specific subtest will be able to hang your machine, so testing this
> > again will require only running the specific subtest instead of
> > waiting
> > 18 hours.
> The kms_frontbuffer_tracking runs from which I posted output two
> hours
> ago did not trigger a lockup.
> 
> (I ran them while X11 was shut down because otherwise
> kms_frontbuffer_tracking would skip all tests with "Can't become DRM
> master, please check if no other DRM client is running.")

Yes, this is the correct way.

> 
> > 
> > > 
> > > PS:
> > > I am mentioning the following just in case that it has any
> > > relationship
> > > with the FBC related kernel freezes.  Maybe it doesn't...  There
> > > is
> > > another recent regression on this PC, but I have not yet figured
> > > out
> > > whether it was introduced by any particular kernel version.  The
> > > regression is:  When switching from X11 to text console by
> > > [Ctrl][Alt][Fx]
> > > or by shutting down sddm, I often only get a blank screen.  I
> > > suspect
> > > that this regression was introduced when I replaced kdm by sddm,
> > > but
> > > I am not sure about that.  
> > Maybe there is some relationship, since this operation involves a
> > mode
> > change. You can also try checking dmesg to see if there are
> > underruns
> > right when you do the change.
> Yes, this is accompanied by
> [drm:intel_set_cpu_fifo_underrun_reporting] *ERROR* uncleared fifo
> underrun on pipe A
> [drm:intel_cpu_fifo_underrun_irq_handler] *ERROR* CPU pipe A FIFO
> underrun  

Regression of v4.6-rc vs. v4.5 bisected: a98ee79317b4 "drm/i915/fbc: enable FBC by default on HSW and BDW"

[Zanoni, Paulo R]

Em Qui, 2016-05-05 às 19:45 +0200, Stefan Richter escreveu:
> On Apr 30 Stefan Richter wrote:
> > 
> > On Apr 29 Stefan Richter wrote:
> > > 
> > > On Apr 26 Stefan Richter wrote:  
> > > > 
> > > > v4.6-rc solidly hangs after a short while after boot, login to
> > > > X11, and
> > > > doing nothing much remarkable on the just brought up X desktop.
> > > > 
> > > > Hardware: x86-64, E3-1245 v3 (Haswell),
> > > >           mainboard Supermicro X10SAE,
> > > >           using integrated Intel graphics (HD P4600, i915
> > > > driver),
> > > >           C226 PCH's AHCI and USB 2/3, ASMedia ASM1062 AHCI,
> > > >           Intel LAN (i217, igb driver),
> > > >           several IEEE 1394 controllers, some of them behind
> > > >           PCIe bridges (IDT, PLX) or PCIe-to-PCI bridges (TI,
> > > > Tundra)
> > > >           and one PCI-to-CardBus bridge (Ricoh)
> > > > 
> > > > kernel.org kernel, Gentoo Linux userland
> > > > 
> > > > 1. known good:  v4.5-rc5 (gcc 4.9.3)
> > > >    known bad:   v4.6-rc2 (gcc 4.9.3), only tried one time
> > > > 
> > > > 2. known good:  v4.5.2 (gcc 5.2.0)
> > > >    known bad:   v4.6-rc5 (gcc 5.2.0), only tried one time
> > > > 
> > > > I will send my linux-4.6-rc5/.config in a follow-up message.  
> >  .config: http://www.spinics.net/lists/kernel/msg2243444.html
> >    lspci: http://www.spinics.net/lists/kernel/msg2243447.html
> > 
> > Some userland package versions, in case these have any bearing:
> > x11-base/xorg-drivers-1.17
> > x11-base/xorg-server-1.17.4
> > x11-bas/xorg-x11-7.4-r2
> Furthermore, there is a single display hooked up via DisplayPort.
> 
> > 
> > > 
> > > After it proved impossible to capture an oops through netconsole,
> > > I
> > > started git bisect.  This will apparently take almost a week, as
> > > git
> > > estimated 13 bisection steps and I will be allowing about 12
> > > hours of
> > > uptime as a sign for a good kernel.  (In my four or five tests of
> > > bad
> > > kernels before I started bisection, they hung after 3
> > > minutes...5.5 hours
> > > uptime, with no discernible difference in workload.  Maybe 12 h
> > > cutoff is
> > > even too short...)  
> I took at least 18 hours uptime (usually 24 hours) as a sign for good
> kernels.  During the bisection, bad kernels hung after 3 h, 2 h, 9
> min,
> 45 min, and 4 min uptime.  Thus I arrived at a98ee79317b4
> "drm/i915/fbc:
> enable FBC by default on HSW and BDW" as the point where the hangs
> are
> introduced.
> 
> Quoting the changelog of the commit:

Thanks for following the instructions on the commit message! :)

> 
>     Oh, and in case you - the person reading this commit message -
> found
>     this commit through git bisect, please do the following:
>      - Check your dmesg and see if there are error messages
> mentioning
>        underruns around the time your problem started happening.
> 
> Well, I always had the followings lines in dmesg:
> [drm:intel_set_cpu_fifo_underrun_reporting] *ERROR* uncleared fifo
> underrun on pipe A
> [drm:intel_cpu_fifo_underrun_irq_handler] *ERROR* CPU pipe A FIFO
> underrun

Oh, well... I had a patch that would just disable FBC in case we saw a
FIFO underrun, but it was rejected. Maybe this is the time to think
about it again? Otherwise, I can't think of much besides disabling FBC
on HSW until all the underruns and watermarks regressions are fixed
forever.

> 
> I always got these when I switch on the DisplayPort attached monitor.
> Recently I changed userland from kdm to sddm and noticed that I
> apparently get these when sddm shuts down.  I am not aware of whether
> or not this also already happened with kdm.
> 
> However, "around the time your problem started happening" there is
> nothing in dmesg, because "your problem" is a complete hang without
> possibility of disk IO and without netconsole output.
> 
>      - Download intel-gpu-tools, compile it, and run:
>        $ sudo ./tests/kms_frontbuffer_tracking --run-subtest '*fbc-*' 
> 2>&1 | tee fbc.txt
>        Then send us the fbc.txt file, especially if you get a
> failure.
>        This will really maximize your chances of getting the bug
> fixed
>        quickly.
> 
> Do you need this while FBC is enabled, or can I run it while FBC is
> disabled?

FBC enabled. Considering your description, my hope is that maybe some
specific subtest will be able to hang your machine, so testing this
again will require only running the specific subtest instead of waiting
18 hours.

> 
>      - Try to find a reliable way to reproduce the problem, and tell
> us.
> 
> The reliable way is to just wait for the kernel to hang after about
> 3 minutes to 5.5 hours.  I have not identified any special activity
> which would trigger the hang.
> 
>      - Boot with drm.debug=0xe, reproduce the problem, then send us
> the
>        dmesg file.
> 
> I can 

[Intel-gfx] [PATCH 4/4] drm/i915: Determine DP++ type 1 DVI adaptor presence based on VBT

[Zanoni, Paulo R]

Em Sex, 2016-04-01 às 17:45 +0300, Ville Syrjälä escreveu:
> On Thu, Mar 31, 2016 at 10:06:37PM +0000, Zanoni, Paulo R wrote:
> > 
> > Em Ter, 2016-02-23 às 18:46 +0200, ville.syrjala at linux.intel.com
> > escreveu:
> > > 
> > > From: Ville Syrjälä 
> > > 
> > > DP dual mode type 1 DVI adaptors aren't required to implement any
> > > registers, so it's a bit hard to detect them. The best way would
> > > be to check the state of the CONFIG1 pin, but we have no way to
> > > do that. So as a last resort, check the VBT to see if the HDMI
> > > port is in fact a dual mode capable DP port.
> > > 
> > > Signed-off-by: Ville Syrjälä 
> > > ---
> > >  drivers/gpu/drm/i915/intel_bios.h |  3 +++
> > >  drivers/gpu/drm/i915/intel_dp.c   | 28
> > > 
> > >  drivers/gpu/drm/i915/intel_drv.h  |  1 +
> > >  drivers/gpu/drm/i915/intel_hdmi.c | 23 +--
> > >  4 files changed, 53 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_bios.h
> > > b/drivers/gpu/drm/i915/intel_bios.h
> > > index 350d4e0f75a4..50d1659efe47 100644
> > > --- a/drivers/gpu/drm/i915/intel_bios.h
> > > +++ b/drivers/gpu/drm/i915/intel_bios.h
> > > @@ -730,6 +730,7 @@ struct bdb_psr {
> > >  #define  DEVICE_TYPE_INT_TV0x1009
> > >  #define  DEVICE_TYPE_HDMI  0x60D2
> > >  #define  DEVICE_TYPE_DP0x68C6
> > > +#define   DEVICE_TYPE_DP_DUAL_MODE  0x60D6
> > >  #define  DEVICE_TYPE_eDP   0x78C6
> > >  
> > >  #define  DEVICE_TYPE_CLASS_EXTENSION  (1 << 15)
> > > @@ -764,6 +765,8 @@ struct bdb_psr {
> > >   DEVICE_TYPE_DISPLAYPORT_OUTPUT | \
> > >   DEVICE_TYPE_ANALOG_OUTPUT)
> > >  
> > > +#define DEVICE_TYPE_DP_DUAL_MODE_BITS
> > > ~DEVICE_TYPE_NOT_HDMI_OUTPUT
> > > +
> > >  /* define the DVO port for HDMI output type */
> > >  #define DVO_B   1
> > >  #define DVO_C   2
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index cbc06596659a..f3edacf517ac 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -5104,6 +5104,34 @@ bool intel_dp_is_edp(struct drm_device
> > > *dev,
> > > enum port port)
> > >  return false;
> > >  }
> > >  
> > > +bool intel_dp_is_dual_mode(struct drm_i915_private *dev_priv,
> > > enum
> > > port port)
> > > +{
> > > + const union child_device_config *p_child;
> > > + int i;
> > > + static const short port_mapping[] = {
> > > + [PORT_B] = DVO_PORT_DPB,
> > > + [PORT_C] = DVO_PORT_DPC,
> > > + [PORT_D] = DVO_PORT_DPD,
> > > + [PORT_E] = DVO_PORT_DPE,
> > > + };
> > > +
> > > + if (port == PORT_A || port >= ARRAY_SIZE(port_mapping))
> > > + return false;
> > > +
> > > + if (!dev_priv->vbt.child_dev_num)
> > > + return false;
> > > +
> > > + for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> > > + p_child = _priv->vbt.child_dev[i];
> > > +
> > > + if (p_child->common.dvo_port ==
> > > port_mapping[port]
> > > &&
> > > +     (p_child->common.device_type &
> > > DEVICE_TYPE_DP_DUAL_MODE_BITS) ==
> > > +     (DEVICE_TYPE_DP_DUAL_MODE &
> > > DEVICE_TYPE_DP_DUAL_MODE_BITS))
> > > + return true;
> > > + }
> > Some thoughts:
> > 
> > This is going to be implemented for all VBT versions. Since there's
> > no
> > real history about anything before version 155, is this really what
> > we
> > want? A huge part of the "we don't trust the VBT" culture we have
> > on
> > our team is because of those old versions being completely
> > unreliable.
> > If this is the case, we could make this implementation just be a
> > small
> > patch in parse_ddi_port(). I'm kinda afraid we may somehow break
> > old
> > machines yet again.
> I don't think it matters much. ILK being the oldest platform with
> DP++
> capable of >165MHz, and at least my ILK here already has VBT version
> 163. Also this device type stuff was there before 155 already

[Intel-gfx] [PATCH 4/4] drm/i915: Determine DP++ type 1 DVI adaptor presence based on VBT

[Zanoni, Paulo R]

Em Ter, 2016-02-23 às 18:46 +0200, ville.syrjala at linux.intel.com
escreveu:
> From: Ville Syrjälä 
> 
> DP dual mode type 1 DVI adaptors aren't required to implement any
> registers, so it's a bit hard to detect them. The best way would
> be to check the state of the CONFIG1 pin, but we have no way to
> do that. So as a last resort, check the VBT to see if the HDMI
> port is in fact a dual mode capable DP port.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_bios.h |  3 +++
>  drivers/gpu/drm/i915/intel_dp.c   | 28 
>  drivers/gpu/drm/i915/intel_drv.h  |  1 +
>  drivers/gpu/drm/i915/intel_hdmi.c | 23 +--
>  4 files changed, 53 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.h
> b/drivers/gpu/drm/i915/intel_bios.h
> index 350d4e0f75a4..50d1659efe47 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -730,6 +730,7 @@ struct bdb_psr {
>  #define  DEVICE_TYPE_INT_TV0x1009
>  #define  DEVICE_TYPE_HDMI  0x60D2
>  #define  DEVICE_TYPE_DP0x68C6
> +#define   DEVICE_TYPE_DP_DUAL_MODE  0x60D6
>  #define  DEVICE_TYPE_eDP   0x78C6
>  
>  #define  DEVICE_TYPE_CLASS_EXTENSION  (1 << 15)
> @@ -764,6 +765,8 @@ struct bdb_psr {
>   DEVICE_TYPE_DISPLAYPORT_OUTPUT | \
>   DEVICE_TYPE_ANALOG_OUTPUT)
>  
> +#define DEVICE_TYPE_DP_DUAL_MODE_BITS ~DEVICE_TYPE_NOT_HDMI_OUTPUT
> +
>  /* define the DVO port for HDMI output type */
>  #define DVO_B   1
>  #define DVO_C   2
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index cbc06596659a..f3edacf517ac 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5104,6 +5104,34 @@ bool intel_dp_is_edp(struct drm_device *dev,
> enum port port)
>  return false;
>  }
>  
> +bool intel_dp_is_dual_mode(struct drm_i915_private *dev_priv, enum
> port port)
> +{
> + const union child_device_config *p_child;
> + int i;
> + static const short port_mapping[] = {
> + [PORT_B] = DVO_PORT_DPB,
> + [PORT_C] = DVO_PORT_DPC,
> + [PORT_D] = DVO_PORT_DPD,
> + [PORT_E] = DVO_PORT_DPE,
> + };
> +
> + if (port == PORT_A || port >= ARRAY_SIZE(port_mapping))
> + return false;
> +
> + if (!dev_priv->vbt.child_dev_num)
> + return false;
> +
> + for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> + p_child = _priv->vbt.child_dev[i];
> +
> + if (p_child->common.dvo_port == port_mapping[port]
> &&
> +     (p_child->common.device_type &
> DEVICE_TYPE_DP_DUAL_MODE_BITS) ==
> +     (DEVICE_TYPE_DP_DUAL_MODE &
> DEVICE_TYPE_DP_DUAL_MODE_BITS))
> + return true;
> + }

Some thoughts:

This is going to be implemented for all VBT versions. Since there's no
real history about anything before version 155, is this really what we
want? A huge part of the "we don't trust the VBT" culture we have on
our team is because of those old versions being completely unreliable.
If this is the case, we could make this implementation just be a small
patch in parse_ddi_port(). I'm kinda afraid we may somehow break old
machines yet again.

- Instead of creating these complicated bit masks, why don't we just
specifically check "if bit 2 and bit 4 are enabled, we're using an
adaptor"? Much simpler IMHO.

- Jani's recent patch suggests you may want to move this function to
intel_bios.c in order to avoid including intel_vbt_defs.h from
intel_hdmi.c. Anyway, you'll have to rebase.

> + return false;
> +}
> +
>  void
>  intel_dp_add_properties(struct intel_dp *intel_dp, struct
> drm_connector *connector)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 3ca29a181e64..c7d1ea4dbe42 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1248,6 +1248,7 @@ int intel_dp_sink_crc(struct intel_dp
> *intel_dp, u8 *crc);
>  bool intel_dp_compute_config(struct intel_encoder *encoder,
>       struct intel_crtc_state *pipe_config);
>  bool intel_dp_is_edp(struct drm_device *dev, enum port port);
> +bool intel_dp_is_dual_mode(struct drm_i915_private *dev_priv, enum
> port port);
>  enum irqreturn intel_dp_hpd_pulse(struct intel_digital_port
> *intel_dig_port,
>    bool long_hpd);
>  void intel_edp_backlight_on(struct intel_dp *intel_dp);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 660a65f48fd8..1476f3afb7e2 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1390,14 +1390,33 @@ intel_hdmi_unset_edid(struct drm_connector
> *connector)
>  }
>  
>  static void
> 

[Intel-gfx] [PATCH v2 3/4] drm/i915: Enable/disable TMDS output buffers in DP++ adaptor as needed

[Zanoni, Paulo R]

Em Qui, 2016-02-25 às 14:51 +0200, ville.syrjala at linux.intel.com
escreveu:
> From: Ville Syrjälä 
> 
> To save a bit of power, let's try to turn off the TMDS output buffers
> in DP++ adaptors when we're not driving the port.
> 
> v2: Let's not forget DDI, toss in a debug message while at it
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_ddi.c  | 12 
>  drivers/gpu/drm/i915/intel_drv.h  |  2 ++
>  drivers/gpu/drm/i915/intel_hdmi.c | 31
> +--
>  3 files changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 21a9b83f3bfc..458c41788b80 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2301,6 +2301,12 @@ static void intel_ddi_pre_enable(struct
> intel_encoder *intel_encoder)
>  enum port port = intel_ddi_get_encoder_port(intel_encoder);
>  int type = intel_encoder->type;
>  
> + if (type == INTEL_OUTPUT_HDMI) {
> + struct intel_hdmi *intel_hdmi =
> enc_to_intel_hdmi(encoder);
> +
> + intel_dp_dual_mode_set_tmds_output(intel_hdmi,
> true);
> + }
> +
>  intel_prepare_ddi_buffer(intel_encoder);
>  
>  if (type == INTEL_OUTPUT_EDP) {
> @@ -2367,6 +2373,12 @@ static void intel_ddi_post_disable(struct
> intel_encoder *intel_encoder)
>  DPLL_CTRL2_DDI_CLK_OFF(port)
> ));
>  else if (INTEL_INFO(dev)->gen < 9)
>  I915_WRITE(PORT_CLK_SEL(port), PORT_CLK_SEL_NONE);
> +
> + if (type == INTEL_OUTPUT_HDMI) {
> + struct intel_hdmi *intel_hdmi =
> enc_to_intel_hdmi(encoder);
> +
> + intel_dp_dual_mode_set_tmds_output(intel_hdmi,
> false);
> + }
>  }
>  
>  static void intel_enable_ddi(struct intel_encoder *intel_encoder)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 944eacbfb6a9..2e4fa4337c59 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -706,6 +706,7 @@ struct intel_hdmi {
>  int ddc_bus;
>  struct {
>  int max_tmds_clock;
> + bool tmds_output_control;
>  } dp_dual_mode;
>  bool limited_color_range;
>  bool color_range_auto;
> @@ -1355,6 +1356,7 @@ void intel_hdmi_init_connector(struct
> intel_digital_port *intel_dig_port,
>  struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder);
>  bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>         struct intel_crtc_state
> *pipe_config);
> +void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi,
> bool enable);
>  
>  
>  /* intel_lvds.c */
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 1e8cfdb18dc7..4b528a669f8e 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -837,6 +837,20 @@ static void hsw_set_infoframes(struct
> drm_encoder *encoder,
>  intel_hdmi_set_hdmi_infoframe(encoder, adjusted_mode);
>  }
>  
> +void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi,
> bool enable)
> +{
> + if (hdmi->dp_dual_mode.tmds_output_control) {

Or you could just do an early return here and save an indentation level
:)

> + struct drm_i915_private *dev_priv =
> to_i915(intel_hdmi_to_dev(hdmi));
> + struct i2c_adapter *adapter =
> + intel_gmbus_get_adapter(dev_priv, hdmi-
> >ddc_bus);
> +
> + DRM_DEBUG_KMS("%s DP dual mode adaptor TMDS
> output\n",
> +       enable ? "Enabling" : "Disabling");
> +
> + drm_dp_dual_mode_set_tmds_output(adapter, enable);
> + }
> +}
> +
>  static void intel_hdmi_prepare(struct intel_encoder *encoder)
>  {
>  struct drm_device *dev = encoder->base.dev;
> @@ -846,6 +860,8 @@ static void intel_hdmi_prepare(struct
> intel_encoder *encoder)
>  const struct drm_display_mode *adjusted_mode = 
> >config->base.adjusted_mode;
>  u32 hdmi_val;
>  
> + intel_dp_dual_mode_set_tmds_output(intel_hdmi, true);
> +
>  hdmi_val = SDVO_ENCODING_HDMI;
>  if (!HAS_PCH_SPLIT(dev) && crtc->config-
> >limited_color_range)
>  hdmi_val |= HDMI_COLOR_RANGE_16_235;
> @@ -1144,6 +1160,8 @@ static void intel_disable_hdmi(struct
> intel_encoder *encoder)
>  }
>  
>  intel_hdmi->set_infoframes(>base, false, NULL);
> +
> + intel_dp_dual_mode_set_tmds_output(intel_hdmi, false);
>  }
>  
>  static void g4x_disable_hdmi(struct intel_encoder *encoder)
> @@ -1369,6 +1387,7 @@ intel_hdmi_unset_edid(struct drm_connector
> *connector)
>  intel_hdmi->rgb_quant_range_selectable = false;
>  
>  intel_hdmi->dp_dual_mode.max_tmds_clock = 0;
> + intel_hdmi->dp_dual_mode.tmds_output_control = false;
>  
>  kfree(to_intel_connector(connector)->detect_edid);
>  

[Intel-gfx] [PATCH 1/4] drm: Add helper for DP++ adaptors

[Zanoni, Paulo R]

Em Ter, 2016-02-23 às 18:46 +0200, ville.syrjala at linux.intel.com
escreveu:
> From: Ville Syrjälä 
> 
> Add a helper which aids in he identification of DP dual mode (aka.
> DP++)
> adaptors. There are several types of adaptors specified:
> type 1 DVI, type 1 HDMI, type 2 DVI, type 2 HDMI
> 
> Type 1 adaptors have a max TMDS clock limit of 165MHz, type 2
> adaptors
> may go as high as 300MHz and they provide a register informing the
> source device what the actual limit is. Supposedly also type 1
> adaptors
> may optionally implement this register. This TMDS clock limit is the
> main reason why we need to identify these adaptors.
> 
> Type 1 adaptors provide access to their internal registers and the
> sink
> DDC bus through I2C. Type 2 adaptors provide this access both via I2C
> and I2C-over-AUX. A type 2 source device may choose to implement
> either
> or both of these methods. If a source device implements only the
> I2C-over-AUX method, then the driver will obviously need specific
> support for such adaptors since the port is driven like an HDMI port,
> but DDC communication happes over the AUX channel.
> 
> This helper should be enough to identify the adaptor type (some
> type 1 DVI adaptors may be a slight exception) and the maximum TMDS
> clock limit. Another feature that may be available is control over
> the TMDS output buffers on the adaptor, possibly allowing for some
> power saving when the TMDS link is down.
> 
> Other user controllable features that may be available in the
> adaptors
> are downstream i2c bus speed control when using i2c-over-aux, and
> some control over the CEC pin. I chose not to provide any helper
> functions for those since I have no use for them in i915 at this
> time.
> The rest of the registers in the adaptor are mostly just information,
> eg. IEEE OUI, hardware and firmware revision, etc.

Please run a spell checker and do some proof-reading both in the commit
message and in the code comments. Multiple instances of "sizo",
"Hoever", "adator", "Identyfy", etc. I also spotted some typos in the
next commits.



> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/Makefile                  |   2 +-
>  drivers/gpu/drm/drm_dp_dual_mode_helper.c | 328
> ++
>  include/drm/drm_dp_dual_mode_helper.h     |  80 
>  3 files changed, 409 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/drm_dp_dual_mode_helper.c
>  create mode 100644 include/drm/drm_dp_dual_mode_helper.h
> 
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 6eb94fc561dc..8ef50f36 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -23,7 +23,7 @@ drm-$(CONFIG_AGP) += drm_agpsupport.o
>  
>  drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o
> drm_probe_helper.o \
>  drm_plane_helper.o drm_dp_mst_topology.o
> drm_atomic_helper.o \
> - drm_kms_helper_common.o
> + drm_kms_helper_common.o drm_dp_dual_mode_helper.o
>  
>  drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
>  drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
> diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> new file mode 100644
> index ..bfe511c09568
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> @@ -0,0 +1,328 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person
> obtaining a
> + * copy of this software and associated documentation files (the
> "Software"),
> + * to deal in the Software without restriction, including without
> limitation
> + * the rights to use, copy, modify, merge, publish, distribute,
> sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom
> the
> + * Software is furnished to do so, subject to the following
> conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
> EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM,
> DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/**
> + * DOC: DP dual mode (aka. DP++) adaptor helpers
> + *
> + * Helper functions to deal with DP dual mode adaptors.
> + *
> + * Type 1:
> + * Adaptor registers (if any) and the sink DDC bus may be accessed
> via I2C.
> + *
> + * Type 2:
> 

[Intel-gfx] [PATCH] drm/i915: Fix build warning on 32-bit

[Zanoni, Paulo R]

Em Sex, 2015-08-14 às 12:35 +0200, Thierry Reding escreveu:
> From: Thierry Reding 
> 
> The gtt.stolen_size field is of type size_t, and so should be printed
> using %zu to avoid build warnings on either 32-bit and 64-bit builds.

While the suggestion from Chris sounds good, this patch alone is
already a fix, so:
Reviewed-by: Paulo Zanoni 

> 
> Signed-off-by: Thierry Reding 
> ---
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
> b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index a36cb95ec798..f361c4a56995 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -348,7 +348,7 @@ int i915_gem_init_stolen(struct drm_device *dev)
>* memory, so just consider the start. */
>   reserved_total = stolen_top - reserved_base;
>  
> - DRM_DEBUG_KMS("Memory reserved for graphics device: %luK, 
> usable: %luK\n",
> + DRM_DEBUG_KMS("Memory reserved for graphics device: %zuK, 
> usable: %luK\n",
> dev_priv->gtt.stolen_size >> 10,
> (dev_priv->gtt.stolen_size - reserved_total) 
> >> 10);
>