date:20211208

Hi Matt and Ram,

On Wed, Dec 08, 2021 at 07:46:13PM +0530, Ramalingam C wrote:
> From: Matthew Auld 
> 
> If the device needs 64K minimum GTT pages for device local-memory,
> like on XEHPSDV, then we need to fail the allocation if we can't
> meet it, instead of falling back to 4K pages, otherwise we can't
> safely support the insertion of device local-memory pages for
> this vm, since the HW expects the correct physical alignment and
> size for every PTE, if we mark the page-table as 64K GTT mode.
> 
> v2: s/userpsace/userspace [Thomas]
> 
> Signed-off-by: Matthew Auld 
> Signed-off-by: Ramalingam C 
> Reviewed-by: Thomas Hellström 

Reviewed-by: Andi Shyti 

Andi

Re: [Intel-gfx] [PATCH 3/4] drm/i915/gtt/xehpsdv: move scratch page to system memory

Hi Ram and Matt,

> On some platforms the hw has dropped support for 4K GTT pages when
> dealing with LMEM, and due to the design of 64K GTT pages in the hw, we
> can only mark the *entire* page-table as operating in 64K GTT mode,
> since the enable bit is still on the pde, and not the pte. And since we
> we still need to allow 4K GTT pages for SMEM objects, we can't have a
> "normal" 4K page-table with scratch pointing to LMEM, since that's
> undefined from the hw pov. The simplest solution is to just move the 64K
> scratch page to SMEM on such platforms and call it a day, since that
> should work for all configurations.
> 
> Signed-off-by: Matthew Auld 
> Signed-off-by: Ramalingam C 
> Reviewed-by: Thomas Hellstrom 

Reviewed-by: Andi Shyti 

Andi

Re: [Intel-gfx] [PATCH 2/4] drm/i915/xehpsdv: set min page-size to 64K

Hi Ram,

On Wed, Dec 08, 2021 at 07:46:11PM +0530, Ramalingam C wrote:
> From: Matthew Auld 
> 
> LMEM should be allocated at 64K granularity, since 4K page support will
> eventually be dropped for LMEM when using the PPGTT.
> 
> Signed-off-by: Matthew Auld 
> Signed-off-by: Stuart Summers 
> Signed-off-by: Ramalingam C 
> Cc: Joonas Lahtinen 
> Cc: Rodrigo Vivi 
> Reviewed-by: Lucas De Marchi 
> Reviewed-by: Thomas Hellstrom 

Reviewed-by: Andi Shyti 

Thanks,
Andi

Re: [Intel-gfx] [PATCH 1/4] drm/i915: Add has_64k_pages flag

Hi Ram,

Reviewed-by: Andi Shyti 

but just two notes on the patchstyle, no need to resend:

1. would be nice to have [PATCH v2...] otherwise it's difficult
   to see if I'm reading the correct version. (I don't see the
   difficulty 'git format-patch -v 2...')

> Add a new platform flag, has_64k_pages, to mark the requirement of 64K
> GTT page sizes or larger for device local memory access.
> 
> Also implies that we require or at least support the compact PT layout
> for the ppGTT when using 64K GTT pages.
> 
> v2: More explanation for the flag [Thomas]
> 
> Signed-off-by: Stuart Summers 
> Signed-off-by: Ramalingam C 
> Reviewed-by: Lucas De Marchi 

2. the tag part has a temporal meaning: Stuart has written the
   patch, Lucas has reviewed it and you are sending it,
   therefore, the correct order should be:

  Signed-off-by: Stuart Summers 
  Reviewed-by: Lucas De Marchi 
  Signed-off-by: Ramalingam C 

Thanks,
Andi

[PATCH 2/4] drm/i915/xehpsdv: set min page-size to 64K

From: Matthew Auld 

Conditionally allocate LMEM with 64K granularity, since 4K page support
for LMEM will be dropped on some platforms when using the PPGTT.

v2:
  updated commit msg [Thomas]

Signed-off-by: Matthew Auld 
Signed-off-by: Stuart Summers 
Signed-off-by: Ramalingam C 
Cc: Joonas Lahtinen 
Cc: Rodrigo Vivi 
Reviewed-by: Lucas De Marchi 
Reviewed-by: Thomas Hellstrom 
---
 drivers/gpu/drm/i915/gem/i915_gem_stolen.c  | 6 +-
 drivers/gpu/drm/i915/gt/intel_region_lmem.c | 5 -
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index bce03d74a0b4..ba90ab47d838 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -780,6 +780,7 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, 
u16 type,
struct intel_uncore *uncore = >uncore;
struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
struct intel_memory_region *mem;
+   resource_size_t min_page_size;
resource_size_t io_start;
resource_size_t lmem_size;
u64 lmem_base;
@@ -791,8 +792,11 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, 
u16 type,
lmem_size = pci_resource_len(pdev, 2) - lmem_base;
io_start = pci_resource_start(pdev, 2) + lmem_base;
 
+   min_page_size = HAS_64K_PAGES(i915) ? I915_GTT_PAGE_SIZE_64K :
+   I915_GTT_PAGE_SIZE_4K;
+
mem = intel_memory_region_create(i915, lmem_base, lmem_size,
-I915_GTT_PAGE_SIZE_4K, io_start,
+min_page_size, io_start,
 type, instance,
 _region_stolen_lmem_ops);
if (IS_ERR(mem))
diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c 
b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
index 9ea49e0a27c0..fde2dcb59809 100644
--- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
+++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
@@ -197,6 +197,7 @@ static struct intel_memory_region *setup_lmem(struct 
intel_gt *gt)
struct intel_uncore *uncore = gt->uncore;
struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
struct intel_memory_region *mem;
+   resource_size_t min_page_size;
resource_size_t io_start;
resource_size_t lmem_size;
int err;
@@ -211,10 +212,12 @@ static struct intel_memory_region *setup_lmem(struct 
intel_gt *gt)
if (GEM_WARN_ON(lmem_size > pci_resource_len(pdev, 2)))
return ERR_PTR(-ENODEV);
 
+   min_page_size = HAS_64K_PAGES(i915) ? I915_GTT_PAGE_SIZE_64K :
+   I915_GTT_PAGE_SIZE_4K;
mem = intel_memory_region_create(i915,
 0,
 lmem_size,
-I915_GTT_PAGE_SIZE_4K,
+min_page_size,
 io_start,
 INTEL_MEMORY_LOCAL,
 0,
-- 
2.20.1

Re: [PATCH v2 6/6] Documentation/gpu: Add amdgpu and dc glossary

2021-12-08 Thread Rodrigo Siqueira Jordao





On 2021-12-07 2:49 p.m., Yann Dirson wrote:



On Thu, Dec 02, 2021 at 11:01:32AM -0500, Rodrigo Siqueira wrote:

In the DC driver, we have multiple acronyms that are not obvious
most of
the time; the same idea is valid for amdgpu. This commit introduces
a DC
and amdgpu glossary in order to make it easier to navigate through
our
driver.

Changes since V1:
  - Yann: Divide glossary based on driver context.
  - Alex: Make terms more consistent and update CPLIB
  - Add new acronyms to the glossary


If you're rerolling, it could be a good time to include the additional
(and detailed) entries from Alex's answer to "Looking for clarifications
around gfx/kcq/kiq".  Finding a way to fit the other details not
fitting directly in the glossary will likely take more rounds, though,
so we can wait for the first round to be merged before dealing with them.


Hi Yann,

I will send another version to address Daniel's comment, and I'll also 
expand the amdgpu acronyms glossary based on your mail thread with Alex. 
However, I don't want to add more details about that discussion in this 
series because I don't want to lose focus in this patchset since my main 
goal is to start to expand display documentation.


By the way, I think you could consider writing a kernel-doc based on 
your discussion with Alex. This way, you can try to consolidate what you 
discover and get reviews in the content.


Thanks
Siqueira






Signed-off-by: Rodrigo Siqueira 
---
  Documentation/gpu/amdgpu/amdgpu-glossary.rst  |  47 
  .../gpu/amdgpu/display/dc-glossary.rst| 243
  ++
  Documentation/gpu/amdgpu/display/index.rst|   1 +
  Documentation/gpu/amdgpu/index.rst|   7 +
  4 files changed, 298 insertions(+)
  create mode 100644 Documentation/gpu/amdgpu/amdgpu-glossary.rst
  create mode 100644
  Documentation/gpu/amdgpu/display/dc-glossary.rst

diff --git a/Documentation/gpu/amdgpu/amdgpu-glossary.rst
b/Documentation/gpu/amdgpu/amdgpu-glossary.rst
new file mode 100644
index ..e635851025e7
--- /dev/null
+++ b/Documentation/gpu/amdgpu/amdgpu-glossary.rst
@@ -0,0 +1,47 @@
+===
+AMDGPU Glossary
+===
+
+Here you can find some generic acronyms used in the amdgpu driver.
Notice that
+we have a dedicated glossary for Display Core.


Maybe add a link to that here so it's easier to find? sphinx
autogenerates
header targets so pretty easy (if the heading is unique at least).
-Daniel


+
+.. glossary::
+
+CPLIB
+  Content Protection Library
+
+DFS
+  Digital Frequency Synthesizer
+
+ECP
+  Enhanced Content Protection
+
+EOP
+  End Of Pipe/Pipeline
+
+HQD
+  Hardware Queue Descriptor
+
+KCQ
+  Kernel Compute Queue
+
+KGQ
+  Kernel Graphics Queue
+
+KIQ
+  Kernel Interface Queue
+
+MQD
+  Memory Queue Descriptor
+
+PPLib
+  PowerPlay Library - PowerPlay is the power management
component.
+
+SMU
+  System Management Unit
+
+VCE
+  Video Compression Engine
+
+VCN
+  Video Codec Next
diff --git a/Documentation/gpu/amdgpu/display/dc-glossary.rst
b/Documentation/gpu/amdgpu/display/dc-glossary.rst
new file mode 100644
index ..547c0bfbb3e2
--- /dev/null
+++ b/Documentation/gpu/amdgpu/display/dc-glossary.rst
@@ -0,0 +1,243 @@
+===
+DC Glossary
+===
+
+On this page, we try to keep track of acronyms related to the
display
+component. If you do not find what you are looking for, look at
the amdgpu
+glossary; if you cannot find it anywhere, consider asking in the
amdgfx and
+update this page.
+
+.. glossary::
+
+ABM
+  Adaptive Backlight Modulation
+
+APU
+  Accelerated Processing Unit
+
+ASIC
+  Application-Specific Integrated Circuit
+
+ASSR
+  Alternate Scrambler Seed Reset
+
+AZ
+  Azalia (HD audio DMA engine)
+
+BPC
+  Bits Per Colour/Component
+
+BPP
+  Bits Per Pixel
+
+Clocks
+  * PCLK: Pixel Clock
+  * SYMCLK: Symbol Clock
+  * SOCCLK: GPU Engine Clock
+  * DISPCLK: Display Clock
+  * DPPCLK: DPP Clock
+  * DCFCLK: Display Controller Fabric Clock
+  * REFCLK: Real Time Reference Clock
+  * PPLL: Pixel PLL
+  * FCLK: Fabric Clock
+  * MCLK: Memory Clock
+
+CRC
+  Cyclic Redundancy Check
+
+CRTC
+  Cathode Ray Tube Controller - commonly called "Controller" -
Generates
+  raw stream of pixels, clocked at pixel clock
+
+CVT
+  Coordinated Video Timings
+
+DAL
+  Display Abstraction layer
+
+DC (Software)
+  Display Core
+
+DC (Hardware)
+  Display Controller
+
+DCC
+  Delta Colour Compression
+
+DCE
+  Display Controller Engine
+
+DCHUB
+  Display Controller HUB
+
+ARB
+  Arbiter
+
+VTG
+  Vertical Timing Generator
+
+DCN
+  Display Core Next
+
+DCCG
+  Display Clock Generator block
+
+DDC
+  Display Data Channel
+
+DIO
+  Display IO

Re: [Intel-gfx] [PATCH 2/3] drm/i915: Sanitycheck device iomem on probe

On 2021-12-08 at 17:23:26 +0200, Andi Shyti wrote:
> Hi Ram,
> 
> > +static int intel_memory_region_memtest(struct intel_memory_region *mem,
> > +  void *caller)
> > +{
> > +   struct drm_i915_private *i915 = mem->i915;
> > +   int err = 0;
> > +
> > +   if (!mem->io_start)
> > +   return 0;
> > +
> > +   if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
> > +   err = iomemtest(mem, caller);
> 
> I don't understand the debugging part of the iomemtest, if memory
> is failing we wouldn't relise if DEBUG_GEM is not enabled.
Thanks for the review Andi!

Since this is in the system init path, running the time taking mem
test only (on demand basis) when the DEBUG_GEM is set or mod_param
is set as per the next patch.

Ram
> 
> In any case,
> 
> Reviewed-by: Andi Shyti 
> 
> Andi

[PATCH 3/3] drm/i915: Test all device memory on probing

From: Chris Wilson 

This extends the previous sanitychecking of device memory to read/write
all the memory on the device during the device probe, ala memtest86,
as an optional module parameter: i915.memtest=1. This is not expected to
be fast, but a reasonably thorough verfification that the device memory
is accessible and doesn't return bit errors.

v2: Rebased.

Suggested-by: Matthew Auld 
Signed-off-by: Chris Wilson 
Cc: Matthew Auld 
Signed-off-by: Ramalingam C 
Reviewed-by: Matthew Auld 
Reviewed-by: Andi Shyti 
---
 drivers/gpu/drm/i915/i915_params.c |  3 ++
 drivers/gpu/drm/i915/i915_params.h |  1 +
 drivers/gpu/drm/i915/intel_memory_region.c | 36 ++
 3 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c 
b/drivers/gpu/drm/i915/i915_params.c
index e07f4cfea63a..525ae832aa9a 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -140,6 +140,9 @@ i915_param_named_unsafe(invert_brightness, int, 0400,
 i915_param_named(disable_display, bool, 0400,
"Disable display (default: false)");
 
+i915_param_named(memtest, bool, 0400,
+   "Perform a read/write test of all device memory on module load 
(default: off)");
+
 i915_param_named(mmio_debug, int, 0400,
"Enable the MMIO debug code for the first N failures (default: off). "
"This may negatively affect performance.");
diff --git a/drivers/gpu/drm/i915/i915_params.h 
b/drivers/gpu/drm/i915/i915_params.h
index 8d725b64592d..c9d53ff910a0 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -64,6 +64,7 @@ struct drm_printer;
param(char *, guc_firmware_path, NULL, 0400) \
param(char *, huc_firmware_path, NULL, 0400) \
param(char *, dmc_firmware_path, NULL, 0400) \
+   param(bool, memtest, false, 0400) \
param(int, mmio_debug, -IS_ENABLED(CONFIG_DRM_I915_DEBUG_MMIO), 0600) \
param(int, edp_vswing, 0, 0400) \
param(unsigned int, reset, 3, 0600) \
diff --git a/drivers/gpu/drm/i915/intel_memory_region.c 
b/drivers/gpu/drm/i915/intel_memory_region.c
index 458afc648772..bab5b5caa6d8 100644
--- a/drivers/gpu/drm/i915/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/intel_memory_region.c
@@ -93,9 +93,12 @@ static resource_size_t random_page(resource_size_t last)
return prandom_u32_max(last >> PAGE_SHIFT) << PAGE_SHIFT;
 }
 
-static int iomemtest(struct intel_memory_region *mem, const void *caller)
+static int iomemtest(struct intel_memory_region *mem,
+bool test_all,
+const void *caller)
 {
resource_size_t last = resource_size(>region) - PAGE_SIZE;
+   resource_size_t page;
int err;
 
/*
@@ -109,17 +112,25 @@ static int iomemtest(struct intel_memory_region *mem, 
const void *caller)
 * a random offset within as a quick spot check for bad memory.
 */
 
-   err = iopagetest(mem, 0, caller);
-   if (err)
-   return err;
+   if (test_all) {
+   for (page = 0; page <= last; page += PAGE_SIZE) {
+   err = iopagetest(mem, page, caller);
+   if (err)
+   return err;
+   }
+   } else {
+   err = iopagetest(mem, 0, caller);
+   if (err)
+   return err;
 
-   err = iopagetest(mem, last, caller);
-   if (err)
-   return err;
+   err = iopagetest(mem, last, caller);
+   if (err)
+   return err;
 
-   err = iopagetest(mem, random_page(last), caller);
-   if (err)
-   return err;
+   err = iopagetest(mem, random_page(last), caller);
+   if (err)
+   return err;
+   }
 
return 0;
 }
@@ -188,13 +199,14 @@ void intel_memory_region_debug(struct intel_memory_region 
*mr,
 static int intel_memory_region_memtest(struct intel_memory_region *mem,
   void *caller)
 {
+   struct drm_i915_private *i915 = mem->i915;
int err = 0;
 
if (!mem->io_start)
return 0;
 
-   if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
-   err = iomemtest(mem, caller);
+   if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) || i915->params.memtest)
+   err = iomemtest(mem, i915->params.memtest, caller);
 
return err;
 }
-- 
2.20.1

[PATCH 2/3] drm/i915: Sanitycheck device iomem on probe

From: Chris Wilson 

As we setup the memory regions for the device, give each a quick test to
verify that we can read and write to the full iomem range. This ensures
that our physical addressing for the device's memory is correct, and
some reassurance that the memory is functional.

v2: wrapper for memtest [Chris]

v3: Removed the unused ptr i915 [Chris]

Signed-off-by: Chris Wilson 
Cc: Matthew Auld 
Signed-off-by: Ramalingam C 
Reviewed-by: Matthew Auld 
Reviewed-by: Andi Shyti 
---
 drivers/gpu/drm/i915/intel_memory_region.c | 116 +
 1 file changed, 116 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_memory_region.c 
b/drivers/gpu/drm/i915/intel_memory_region.c
index b43121609e25..458afc648772 100644
--- a/drivers/gpu/drm/i915/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/intel_memory_region.c
@@ -3,6 +3,8 @@
  * Copyright © 2019 Intel Corporation
  */
 
+#include 
+
 #include "intel_memory_region.h"
 #include "i915_drv.h"
 #include "i915_ttm_buddy_manager.h"
@@ -29,6 +31,99 @@ static const struct {
},
 };
 
+static int __iopagetest(struct intel_memory_region *mem,
+   u8 __iomem *va, int pagesize,
+   u8 value, resource_size_t offset,
+   const void *caller)
+{
+   int byte = prandom_u32_max(pagesize);
+   u8 result[3];
+
+   memset_io(va, value, pagesize); /* or GPF! */
+   wmb();
+
+   result[0] = ioread8(va);
+   result[1] = ioread8(va + byte);
+   result[2] = ioread8(va + pagesize - 1);
+   if (memchr_inv(result, value, sizeof(result))) {
+   dev_err(mem->i915->drm.dev,
+   "Failed to read back from memory region:%pR at [%pa + 
%pa] for %ps; wrote %x, read (%x, %x, %x)\n",
+   >region, >io_start, , caller,
+   value, result[0], result[1], result[2]);
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
+static int iopagetest(struct intel_memory_region *mem,
+ resource_size_t offset,
+ const void *caller)
+{
+   const u8 val[] = { 0x0, 0xa5, 0xc3, 0xf0 };
+   void __iomem *va;
+   int err;
+   int i;
+
+   va = ioremap_wc(mem->io_start + offset, PAGE_SIZE);
+   if (!va) {
+   dev_err(mem->i915->drm.dev,
+   "Failed to ioremap memory region [%pa + %px] for %ps\n",
+   >io_start, , caller);
+   return -EFAULT;
+   }
+
+   for (i = 0; i < ARRAY_SIZE(val); i++) {
+   err = __iopagetest(mem, va, PAGE_SIZE, val[i], offset, caller);
+   if (err)
+   break;
+
+   err = __iopagetest(mem, va, PAGE_SIZE, ~val[i], offset, caller);
+   if (err)
+   break;
+   }
+
+   iounmap(va);
+   return err;
+}
+
+static resource_size_t random_page(resource_size_t last)
+{
+   /* Limited to low 44b (16TiB), but should suffice for a spot check */
+   return prandom_u32_max(last >> PAGE_SHIFT) << PAGE_SHIFT;
+}
+
+static int iomemtest(struct intel_memory_region *mem, const void *caller)
+{
+   resource_size_t last = resource_size(>region) - PAGE_SIZE;
+   int err;
+
+   /*
+* Quick test to check read/write access to the iomap (backing store).
+*
+* Write a byte, read it back. If the iomapping fails, we expect
+* a GPF preventing further execution. If the backing store does not
+* exist, the read back will return garbage. We check a couple of pages,
+* the first and last of the specified region to confirm the backing
+* store + iomap does cover the entire memory region; and we check
+* a random offset within as a quick spot check for bad memory.
+*/
+
+   err = iopagetest(mem, 0, caller);
+   if (err)
+   return err;
+
+   err = iopagetest(mem, last, caller);
+   if (err)
+   return err;
+
+   err = iopagetest(mem, random_page(last), caller);
+   if (err)
+   return err;
+
+   return 0;
+}
+
 struct intel_memory_region *
 intel_memory_region_lookup(struct drm_i915_private *i915,
   u16 class, u16 instance)
@@ -90,6 +185,20 @@ void intel_memory_region_debug(struct intel_memory_region 
*mr,
   >total, >avail);
 }
 
+static int intel_memory_region_memtest(struct intel_memory_region *mem,
+  void *caller)
+{
+   int err = 0;
+
+   if (!mem->io_start)
+   return 0;
+
+   if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
+   err = iomemtest(mem, caller);
+
+   return err;
+}
+
 struct intel_memory_region *
 intel_memory_region_create(struct drm_i915_private *i915,
   resource_size_t start,
@@ -126,8 +235,15 @@ intel_memory_region_create(struct drm_i915_private *i915,
goto

[PATCH 1/3] drm/i915: Exclude reserved stolen from driver use

From: Chris Wilson 

Remove the portion of stolen memory reserved for private use from driver
access.

Signed-off-by: Chris Wilson 
cc: Matthew Auld 
Signed-off-by: Ramalingam C 
Reviewed-by: Matthew Auld 
Reviewed-by: Andi Shyti 
---
 drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index bce03d74a0b4..6ea3ca21cdf3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -488,6 +488,9 @@ static int i915_gem_init_stolen(struct intel_memory_region 
*mem)
return 0;
}
 
+   /* Exclude the reserved region from driver use */
+   mem->region.end = reserved_base - 1;
+
/* It is possible for the reserved area to end before the end of stolen
 * memory, so just consider the start. */
reserved_total = stolen_top - reserved_base;
-- 
2.20.1

[PATCH 0/3] drm/i915: Sanity Check for device memory region

Changes for introducing the quick test on the device memory range and
also a test of detailed validation for each addr of the range with read
and write.

Detailed testing is optionally enabled with a modparam i915.memtest=1

And third patch fixes the driver accessible stolen memory.

v2: Adding a wrapper for the memtest [Chris]
v3: Handling a bisecting issue.

Chris Wilson (3):
  drm/i915: Exclude reserved stolen from driver use
  drm/i915: Sanitycheck device iomem on probe
  drm/i915: Test all device memory on probing

 drivers/gpu/drm/i915/gem/i915_gem_stolen.c |   3 +
 drivers/gpu/drm/i915/i915_params.c |   3 +
 drivers/gpu/drm/i915/i915_params.h |   1 +
 drivers/gpu/drm/i915/intel_memory_region.c | 128 +
 4 files changed, 135 insertions(+)

-- 
2.20.1

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Test all device memory on probing

Hi Ram and Chris,

>   param(char *, guc_firmware_path, NULL, 0400) \
>   param(char *, huc_firmware_path, NULL, 0400) \
>   param(char *, dmc_firmware_path, NULL, 0400) \
> + param(bool, memtest, false, 0400) \

this partially answers my previous question...

[...]

> - if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
> - err = iomemtest(mem, caller);
> + if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) || i915->params.memtest)
> + err = iomemtest(mem, i915->params.memtest, caller);

... but still I am missing the debugging part.

Reviewed-by: Andi Shyti 

Andi

Re: [Intel-gfx] [PATCH 2/3] drm/i915: Sanitycheck device iomem on probe

Hi Ram,

> +static int intel_memory_region_memtest(struct intel_memory_region *mem,
> +void *caller)
> +{
> + struct drm_i915_private *i915 = mem->i915;
> + int err = 0;
> +
> + if (!mem->io_start)
> + return 0;
> +
> + if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
> + err = iomemtest(mem, caller);

I don't understand the debugging part of the iomemtest, if memory
is failing we wouldn't relise if DEBUG_GEM is not enabled.

In any case,

Reviewed-by: Andi Shyti 

Andi

Re: [Intel-gfx] [PATCH 1/3] drm/i915: Exclude reserved stolen from driver use

Hi Ram,

On Wed, Dec 08, 2021 at 08:27:58PM +0530, Ramalingam C wrote:
> From: Chris Wilson 
> 
> Remove the portion of stolen memory reserved for private use from driver
> access.
> 
> Signed-off-by: Chris Wilson 
> cc: Matthew Auld 
> Signed-off-by: Ramalingam C 
> Reviewed-by: Matthew Auld 

Reviewed-by: Andi Shyti 

Thanks,
Andi

Re: [Intel-gfx] [PATCH 1/3] drm/i915: Exclude reserved stolen from driver use

On Wed, Dec 08, 2021 at 08:27:58PM +0530, Ramalingam C wrote:
> From: Chris Wilson 
> 
> Remove the portion of stolen memory reserved for private use from driver
> access.
> 
> Signed-off-by: Chris Wilson 
> cc: Matthew Auld 
> Signed-off-by: Ramalingam C 
> Reviewed-by: Matthew Auld 
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c 
> b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> index bce03d74a0b4..6ea3ca21cdf3 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> @@ -488,6 +488,9 @@ static int i915_gem_init_stolen(struct 
> intel_memory_region *mem)
>   return 0;
>   }
>  
> + /* Exclude the reserved region from driver use */
> + mem->region.end = reserved_base - 1;
> +
>   /* It is possible for the reserved area to end before the end of stolen
>* memory, so just consider the start. */
>   reserved_total = stolen_top - reserved_base;
> -- 
> 2.20.1

[PATCH 07/18] dt-bindings: display: rockchip: dw-hdmi: Allow "ref" as clock name

"vpll" is a misnomer. A clock input to a device should be named after
the usage in the device, not after the clock that drives it. On the
rk3568 the same clock is driven by the HPLL.
To fix that, this patch renames the vpll clock to ref clock.  The clock
name "vpll" is left for compatibility to old device trees.

Signed-off-by: Sascha Hauer 
---
 .../bindings/display/rockchip/rockchip,dw-hdmi.yaml | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git 
a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml 
b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
index 6e09dd2ee05ac..2ab6578033da2 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
@@ -48,10 +48,14 @@ properties:
   - cec
   - grf
   - vpll
+  - ref
   - enum:
   - grf
   - vpll
-  - const: vpll
+  - ref
+  - enum:
+  - vpll
+  - ref
 
   ddc-i2c-bus:
 $ref: /schemas/types.yaml#/definitions/phandle
-- 
2.30.2

[PATCH v2 00/18] drm/rockchip: RK356x VOP2 support

This is the second round of the vop2 series. There are still some issues open,
but I thought it's about time to let people see and test it. I integrated the
review feedback I got from v1. Other changes include:

All framesync waiting is gone from the driver which makes it more straight
forward. To accomplish this the port_mux setup is now static in the driver.
This means each video port has a fixed maximum number of planes which is less
flexible but much easier to handle.

I also removed much of the register mapping and shadow register handling around
struct vop_reg. This basically resembles regmap and can eventually replaced by
regmap. Some places are still left in the driver, I plan to remove those in
later versions.

I think I have found the issue why only 1080p resolutions work, this seems to
be an issue in the way the clock tree is arranged. See the last patch in this
series which points to the problem, so far I don't have a good solution for it.

As usual, all comments and feedback welcome.

Sascha

Changes since v1:
- drop all unnecessary waiting for frames within atomic modeset and plane update
- Cluster subwin support removed
- gamma support removed
- unnecessary irq_lock removed
- interrupt handling simplified
- simplified zpos handling
- drop is_alpha_support(), use fb->format->has_alpha instead
- use devm_regulator_get() rather than devm_regulator_get_optional() for hdmi 
regulators
- Use fixed number of planes per video port
- Drop homegrown regmap code from vop2 driver (not complete yet)
- Add separate include file for vop2 driver to not pollute the vop include

Andy Yan (1):
  drm: rockchip: Add VOP2 driver

Benjamin Gaignard (1):
  dt-bindings: display: rockchip: dw-hdmi: Add compatible for rk3568
HDMI

Michael Riesch (1):
  arm64: dts: rockchip: enable vop2 and hdmi tx on quartz64a

Sascha Hauer (15):
  drm/rockchip: dw_hdmi: Do not leave clock enabled in error case
  drm/rockchip: dw_hdmi: rename vpll clock to reference clock
  drm/rockchip: dw_hdmi: add rk3568 support
  drm/rockchip: dw_hdmi: add regulator support
  dt-bindings: display: rockchip: dw-hdmi: Make unwedge pinctrl optional
  dt-bindings: display: rockchip: dw-hdmi: Allow "ref" as clock name
  dt-bindings: display: rockchip: dw-hdmi: Add regulator support
  arm64: dts: rockchip: rk3399: reorder hmdi clocks
  dt-bindings: display: rockchip: Add binding for VOP2
  arm64: dts: rockchip: rk356x: Add VOP2 nodes
  arm64: dts: rockchip: rk356x: Add HDMI nodes
  arm64: dts: rockchip: rk3568-evb: Enable VOP2 and hdmi
  drm/encoder: Add of_graph port to struct drm_encoder
  drm/rockchip: Make VOP driver optional
  [HACK, RFC] clk: rk3568: do not divide dclk_vop0

 .../display/rockchip/rockchip,dw-hdmi.yaml|   14 +-
 .../display/rockchip/rockchip-vop2.yaml   |  118 +
 arch/arm64/boot/dts/rockchip/rk3399.dtsi  |6 +-
 .../boot/dts/rockchip/rk3566-quartz64-a.dts   |   31 +
 arch/arm64/boot/dts/rockchip/rk3566.dtsi  |4 +
 .../boot/dts/rockchip/rk3568-evb1-v10.dts |   31 +
 arch/arm64/boot/dts/rockchip/rk3568.dtsi  |4 +
 arch/arm64/boot/dts/rockchip/rk356x.dtsi  |   75 +
 drivers/clk/rockchip/clk-rk3568.c |4 +-
 drivers/gpu/drm/rockchip/Kconfig  |   14 +
 drivers/gpu/drm/rockchip/Makefile |4 +-
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c   |  107 +-
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c   |3 +-
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h   |7 +-
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c|2 +
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h   |   15 +
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c  | 2636 +
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.h  |  625 
 drivers/gpu/drm/rockchip/rockchip_vop2_reg.c  |  505 
 include/drm/drm_encoder.h |2 +
 include/dt-bindings/soc/rockchip,vop2.h   |   14 +
 21 files changed, 4193 insertions(+), 28 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
 create mode 100644 include/dt-bindings/soc/rockchip,vop2.h

-- 
2.30.2

[PATCH 06/18] dt-bindings: display: rockchip: dw-hdmi: Make unwedge pinctrl optional

None of the upstream device tree files has a "unwedge" pinctrl
specified. Make it optional.

Signed-off-by: Sascha Hauer 
---
 .../devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml   | 1 +
 1 file changed, 1 insertion(+)

diff --git 
a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml 
b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
index 45cae4f57a1c1..6e09dd2ee05ac 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
@@ -73,6 +73,7 @@ properties:
   The unwedge pinctrl entry shall drive the DDC SDA line low. This is
   intended to work around a hardware errata that can cause the DDC I2C
   bus to be wedged.
+minItems: 1
 items:
   - const: default
   - const: unwedge
-- 
2.30.2

[PATCH 14/18] arm64: dts: rockchip: enable vop2 and hdmi tx on quartz64a

From: Michael Riesch 

Enable the RK356x Video Output Processor (VOP) 2 on the Pine64
Quartz64 Model A.

Signed-off-by: Michael Riesch 
Signed-off-by: Sascha Hauer 
---
 .../boot/dts/rockchip/rk3566-quartz64-a.dts   | 31 +++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts 
b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
index 4d4b2a301b1a4..ccebd6bb19cea 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
@@ -4,6 +4,7 @@
 
 #include 
 #include 
+#include 
 #include "rk3566.dtsi"
 
 / {
@@ -205,6 +206,12 @@ _clkinout
status = "okay";
 };
 
+ {
+   avdd-0v9-supply = <_0v9>;
+   avdd-1v8-supply = <_1v8>;
+   status = "okay";
+};
+
  {
status = "okay";
 
@@ -546,3 +553,27 @@ bluetooth {
  {
status = "okay";
 };
+
+ {
+   assigned-clocks = < DCLK_VOP0>, < DCLK_VOP1>;
+   assigned-clock-parents = < PLL_HPLL>, < PLL_VPLL>;
+   status = "okay";
+};
+
+_mmu {
+   status = "okay";
+};
+
+_in {
+   hdmi_in_vp0: endpoint@0 {
+   reg = <0>;
+   remote-endpoint = <_out_hdmi>;
+   };
+};
+
+ {
+   vp0_out_hdmi: endpoint@RK3568_VOP2_EP_HDMI {
+   reg = ;
+   remote-endpoint = <_in_vp0>;
+   };
+};
-- 
2.30.2

[PATCH 03/18] drm/rockchip: dw_hdmi: add rk3568 support

Add a new dw_hdmi_plat_data struct and new compatible for rk3568.

Signed-off-by: Benjamin Gaignard 
Signed-off-by: Sascha Hauer 
---
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 31 +
 1 file changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c 
b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index e352e0404f772..262eef614cb12 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -50,6 +50,10 @@
 #define RK3399_GRF_SOC_CON20   0x6250
 #define RK3399_HDMI_LCDC_SEL   BIT(6)
 
+#define RK3568_GRF_VO_CON1 0x0364
+#define RK3568_HDMI_SDAIN_MSK  BIT(15)
+#define RK3568_HDMI_SCLIN_MSK  BIT(14)
+
 #define HIWORD_UPDATE(val, mask)   (val | (mask) << 16)
 
 /**
@@ -470,6 +474,19 @@ static const struct dw_hdmi_plat_data rk3399_hdmi_drv_data 
= {
.use_drm_infoframe = true,
 };
 
+static struct rockchip_hdmi_chip_data rk3568_chip_data = {
+   .lcdsel_grf_reg = -1,
+};
+
+static const struct dw_hdmi_plat_data rk3568_hdmi_drv_data = {
+   .mode_valid = dw_hdmi_rockchip_mode_valid,
+   .mpll_cfg   = rockchip_mpll_cfg,
+   .cur_ctr= rockchip_cur_ctr,
+   .phy_config = rockchip_phy_config,
+   .phy_data = _chip_data,
+   .use_drm_infoframe = true,
+};
+
 static const struct of_device_id dw_hdmi_rockchip_dt_ids[] = {
{ .compatible = "rockchip,rk3228-dw-hdmi",
  .data = _hdmi_drv_data
@@ -483,6 +500,9 @@ static const struct of_device_id dw_hdmi_rockchip_dt_ids[] 
= {
{ .compatible = "rockchip,rk3399-dw-hdmi",
  .data = _hdmi_drv_data
},
+   { .compatible = "rockchip,rk3568-dw-hdmi",
+ .data = _hdmi_drv_data
+   },
{},
 };
 MODULE_DEVICE_TABLE(of, dw_hdmi_rockchip_dt_ids);
@@ -517,6 +537,9 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct 
device *master,
encoder = >encoder;
 
encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
+
+   encoder->port = of_graph_get_port_by_id(dev->of_node, 0);
+
/*
 * If we failed to find the CRTC(s) which this encoder is
 * supposed to be connected to, it's because the CRTC has
@@ -547,6 +570,14 @@ static int dw_hdmi_rockchip_bind(struct device *dev, 
struct device *master,
return ret;
}
 
+   if (hdmi->chip_data == _chip_data) {
+   regmap_write(hdmi->regmap, RK3568_GRF_VO_CON1,
+HIWORD_UPDATE(RK3568_HDMI_SDAIN_MSK |
+  RK3568_HDMI_SCLIN_MSK,
+  RK3568_HDMI_SDAIN_MSK |
+  RK3568_HDMI_SCLIN_MSK));
+   }
+
drm_encoder_helper_add(encoder, _hdmi_rockchip_encoder_helper_funcs);
drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_TMDS);
 
-- 
2.30.2

[PATCH 09/18] arm64: dts: rockchip: rk3399: reorder hmdi clocks

The binding specifies the clock order to "cec", "grf", "vpll". Reorder
the clocks accordingly.

Signed-off-by: Sascha Hauer 
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index d3cdf6f42a303..080457a68e3c7 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1881,10 +1881,10 @@ hdmi: hdmi@ff94 {
interrupts = ;
clocks = < PCLK_HDMI_CTRL>,
 < SCLK_HDMI_SFR>,
-< PLL_VPLL>,
+< SCLK_HDMI_CEC>,
 < PCLK_VIO_GRF>,
-< SCLK_HDMI_CEC>;
-   clock-names = "iahb", "isfr", "vpll", "grf", "cec";
+< PLL_VPLL>;
+   clock-names = "iahb", "isfr", "cec", "grf", "vpll";
power-domains = < RK3399_PD_HDCP>;
reg-io-width = <4>;
rockchip,grf = <>;
-- 
2.30.2

[PATCH 16/18] drm/rockchip: Make VOP driver optional

With upcoming VOP2 support VOP won't be the only choice anymore, so make
the VOP driver optional.

Signed-off-by: Sascha Hauer 
---
 drivers/gpu/drm/rockchip/Kconfig| 8 
 drivers/gpu/drm/rockchip/Makefile   | 3 ++-
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 2 +-
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index 9f1ecefc39332..b9b156308460a 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -21,8 +21,16 @@ config DRM_ROCKCHIP
 
 if DRM_ROCKCHIP
 
+config ROCKCHIP_VOP
+   bool "Rockchip VOP driver"
+   default y
+   help
+ This selects support for the VOP driver. You should enable it
+ on all older SoCs up to RK3399.
+
 config ROCKCHIP_ANALOGIX_DP
bool "Rockchip specific extensions for Analogix DP driver"
+   depends on ROCKCHIP_VOP
help
  This selects support for Rockchip SoC specific extensions
  for the Analogix Core DP driver. If you want to enable DP
diff --git a/drivers/gpu/drm/rockchip/Makefile 
b/drivers/gpu/drm/rockchip/Makefile
index 17a9e7eb2130d..cd6e7bb5ce9c5 100644
--- a/drivers/gpu/drm/rockchip/Makefile
+++ b/drivers/gpu/drm/rockchip/Makefile
@@ -4,9 +4,10 @@
 # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
 
 rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \
-   rockchip_drm_gem.o rockchip_drm_vop.o rockchip_vop_reg.o
+   rockchip_drm_gem.o
 rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
 
+rockchipdrm-$(CONFIG_ROCKCHIP_VOP) += rockchip_drm_vop.o rockchip_vop_reg.o
 rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
 rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o
 rockchipdrm-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index e4ebe60b3cc1a..64fa5fd62c01a 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -473,7 +473,7 @@ static int __init rockchip_drm_init(void)
int ret;
 
num_rockchip_sub_drivers = 0;
-   ADD_ROCKCHIP_SUB_DRIVER(vop_platform_driver, CONFIG_DRM_ROCKCHIP);
+   ADD_ROCKCHIP_SUB_DRIVER(vop_platform_driver, CONFIG_ROCKCHIP_VOP);
ADD_ROCKCHIP_SUB_DRIVER(rockchip_lvds_driver,
CONFIG_ROCKCHIP_LVDS);
ADD_ROCKCHIP_SUB_DRIVER(rockchip_dp_driver,
-- 
2.30.2

[PATCH 13/18] arm64: dts: rockchip: rk3568-evb: Enable VOP2 and hdmi

This enabled the VOP2 display controller along with hdmi and the
required port routes which is enough to get a picture out of the
hdmi port of the board.

Signed-off-by: Sascha Hauer 
---
 .../boot/dts/rockchip/rk3568-evb1-v10.dts | 31 +++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts 
b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
index 184e2aa2416af..b1b0963fa8525 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
@@ -7,6 +7,7 @@
 /dts-v1/;
 #include 
 #include 
+#include 
 #include "rk3568.dtsi"
 
 / {
@@ -106,6 +107,12 @@ _rgmii_clk
status = "okay";
 };
 
+ {
+   status = "okay";
+   avdd-0v9-supply = <_image>;
+   avdd-1v8-supply = <_image>;
+};
+
  {
status = "okay";
 
@@ -390,3 +397,27 @@  {
  {
status = "okay";
 };
+
+ {
+   status = "okay";
+   assigned-clocks = < DCLK_VOP0>, < DCLK_VOP1>;
+   assigned-clock-parents = < PLL_HPLL>, < PLL_VPLL>;
+};
+
+_mmu {
+   status = "okay";
+};
+
+_in {
+   hdmi_in_vp0: endpoint@0 {
+   reg = <0>;
+   remote-endpoint = <_out_hdmi>;
+   };
+};
+
+ {
+   vp0_out_hdmi: endpoint@RK3568_VOP2_EP_HDMI {
+   reg = ;
+   remote-endpoint = <_in_vp0>;
+   };
+};
-- 
2.30.2

[PATCH 05/18] dt-bindings: display: rockchip: dw-hdmi: Add compatible for rk3568 HDMI

From: Benjamin Gaignard 

Define a new compatible for rk3568 HDMI.
This version of HDMI hardware block needs two new clocks hclk_vio and hclk
to provide phy reference clocks.

Signed-off-by: Benjamin Gaignard 
Reviewed-by: Rob Herring 
Link: 
https://lore.kernel.org/r/20210707120323.401785-2-benjamin.gaign...@collabora.com
Signed-off-by: Sascha Hauer 
---
 .../devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml   | 1 +
 1 file changed, 1 insertion(+)

diff --git 
a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml 
b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
index da3b889ad8fcd..45cae4f57a1c1 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
@@ -23,6 +23,7 @@ properties:
   - rockchip,rk3288-dw-hdmi
   - rockchip,rk3328-dw-hdmi
   - rockchip,rk3399-dw-hdmi
+  - rockchip,rk3568-dw-hdmi
 
   reg-io-width:
 const: 4
-- 
2.30.2

[PATCH 10/18] dt-bindings: display: rockchip: Add binding for VOP2

The VOP2 is found on newer Rockchip SoCs like the rk3568 or the rk3566.
The binding differs slightly from the existing VOP binding, so add a new
binding file for it.

Signed-off-by: Sascha Hauer 
---
 .../display/rockchip/rockchip-vop2.yaml   | 118 ++
 1 file changed, 118 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml

diff --git 
a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml 
b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
new file mode 100644
index 0..6533c4ae4ec3a
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
@@ -0,0 +1,118 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/rockchip/rockchip-vop2.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip SoC display controller (VOP2)
+
+description:
+  VOP2 (Video Output Processor v2) is the display controller for the Rockchip
+  series of SoCs which transfers the image data from a video memory
+  buffer to an external LCD interface.
+
+maintainers:
+  - Sandy Huang 
+  - Heiko Stuebner 
+
+properties:
+  compatible:
+enum:
+  - rockchip,rk3566-vop
+  - rockchip,rk3568-vop
+
+  reg:
+minItems: 1
+items:
+  - description:
+  Must contain one entry corresponding to the base address and length
+  of the register space.
+  - description:
+  Can optionally contain a second entry corresponding to
+  the CRTC gamma LUT address.
+
+  interrupts:
+maxItems: 1
+description:
+  The VOP interrupt is shared by several interrupt sources, such as
+  frame start (VSYNC), line flag and other status interrupts.
+
+  clocks:
+items:
+  - description: Clock for ddr buffer transfer.
+  - description: Clock for the ahb bus to R/W the phy regs.
+  - description: Pixel clock for video port 0.
+  - description: Pixel clock for video port 1.
+  - description: Pixel clock for video port 2.
+
+  clock-names:
+items:
+  - const: aclk_vop
+  - const: hclk_vop
+  - const: dclk_vp0
+  - const: dclk_vp1
+  - const: dclk_vp2
+
+  port:
+$ref: /schemas/graph.yaml#/properties/port
+
+  assigned-clocks:
+maxItems: 2
+
+  assigned-clock-rates:
+maxItems: 2
+
+  iommus:
+maxItems: 1
+
+  power-domains:
+maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+#include 
+#include 
+#include 
+bus {
+#address-cells = <2>;
+#size-cells = <2>;
+vop: vop@fe04 {
+compatible = "rockchip,rk3568-vop";
+reg = <0x0 0xfe04 0x0 0x3000>, <0x0 0xfe044000 0x0 0x1000>;
+interrupts = ;
+clocks = < ACLK_VOP>,
+ < HCLK_VOP>,
+ < DCLK_VOP0>,
+ < DCLK_VOP1>,
+ < DCLK_VOP2>;
+clock-names = "aclk_vop",
+  "hclk_vop",
+  "dclk_vp0",
+  "dclk_vp1",
+  "dclk_vp2";
+power-domains = < RK3568_PD_VO>;
+iommus = <_mmu>;
+vop_out: port {
+#address-cells = <1>;
+#size-cells = <0>;
+vp0_out_dsi0: endpoint@0 {
+reg = <0>;
+remote-endpoint = <_in_vp0>;
+};
+vp0_out_hdmi: endpoint@1 {
+reg = <1>;
+remote-endpoint = <_in_vp0>;
+};
+};
+};
+};
-- 
2.30.2

[PATCH 12/18] arm64: dts: rockchip: rk356x: Add HDMI nodes

Add support for the HDMI port found on RK3568.

Signed-off-by: Sascha Hauer 
---
 arch/arm64/boot/dts/rockchip/rk356x.dtsi | 26 +++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi 
b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index 3c1e65e851023..702065bdcf282 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -10,7 +10,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 
 / {
@@ -498,6 +497,31 @@ vop_mmu: iommu@fe043e00 {
status = "disabled";
};
 
+   hdmi: hdmi@fe0a {
+   compatible = "rockchip,rk3568-dw-hdmi";
+   reg = <0x0 0xfe0a 0x0 0x2>;
+   interrupts = ;
+   clocks = < PCLK_HDMI_HOST>,
+< CLK_HDMI_SFR>,
+< CLK_HDMI_CEC>,
+< HCLK_VOP>;
+   clock-names = "iahb", "isfr", "cec", "ref";
+   pinctrl-names = "default";
+   pinctrl-0 = <_scl _sda _cec>;
+   power-domains = < RK3568_PD_VO>;
+   reg-io-width = <4>;
+   rockchip,grf = <>;
+   #sound-dai-cells = <0>;
+   status = "disabled";
+
+   ports {
+   hdmi_in: port {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   };
+   };
+   };
+
qos_gpu: qos@fe128000 {
compatible = "rockchip,rk3568-qos", "syscon";
reg = <0x0 0xfe128000 0x0 0x20>;
-- 
2.30.2

[PATCH 11/18] arm64: dts: rockchip: rk356x: Add VOP2 nodes

The VOP2 is the display output controller on the RK3568. Add the node
for it to the dtsi file along with the required display-subsystem node
and the iommu node.

Signed-off-by: Sascha Hauer 
---
 arch/arm64/boot/dts/rockchip/rk3566.dtsi |  4 ++
 arch/arm64/boot/dts/rockchip/rk3568.dtsi |  4 ++
 arch/arm64/boot/dts/rockchip/rk356x.dtsi | 51 
 include/dt-bindings/soc/rockchip,vop2.h  | 14 +++
 4 files changed, 73 insertions(+)
 create mode 100644 include/dt-bindings/soc/rockchip,vop2.h

diff --git a/arch/arm64/boot/dts/rockchip/rk3566.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3566.dtsi
index 3839eef5e4f76..595fa2562cb8e 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3566.dtsi
@@ -18,3 +18,7 @@ power-domain@RK3568_PD_PIPE {
#power-domain-cells = <0>;
};
 };
+
+ {
+   compatible = "rockchip,rk3566-vop";
+};
diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
index 2fd313a295f8a..1e55efb6fcfde 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
@@ -95,3 +95,7 @@ power-domain@RK3568_PD_PIPE {
#power-domain-cells = <0>;
};
 };
+
+ {
+   compatible = "rockchip,rk3568-vop";
+};
diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi 
b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index 46d9552f60284..3c1e65e851023 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 / {
@@ -125,6 +126,11 @@ opp-18 {
};
};
 
+   display_subsystem: display-subsystem {
+   compatible = "rockchip,display-subsystem";
+   ports = <_out>;
+   };
+
firmware {
scmi: scmi {
compatible = "arm,scmi-smc";
@@ -447,6 +453,51 @@ gmac1_mtl_tx_setup: tx-queues-config {
};
};
 
+   vop: vop@fe04 {
+   reg = <0x0 0xfe04 0x0 0x3000>, <0x0 0xfe044000 0x0 0x1000>;
+   reg-names = "regs", "gamma_lut";
+   interrupts = ;
+   clocks = < ACLK_VOP>, < HCLK_VOP>, < DCLK_VOP0>, 
< DCLK_VOP1>, < DCLK_VOP2>;
+   clock-names = "aclk_vop", "hclk_vop", "dclk_vp0", "dclk_vp1", 
"dclk_vp2";
+   iommus = <_mmu>;
+   power-domains = < RK3568_PD_VO>;
+   rockchip,grf = <>;
+   status = "disabled";
+
+   vop_out: ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   vp0: port@0 {
+   reg = <0>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   };
+
+   vp1: port@1 {
+   reg = <1>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   };
+
+   vp2: port@2 {
+   reg = <2>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   };
+   };
+   };
+
+   vop_mmu: iommu@fe043e00 {
+   compatible = "rockchip,rk3568-iommu";
+   reg = <0x0 0xfe043e00 0x0 0x100>, <0x0 0xfe043f00 0x0 0x100>;
+   interrupts = ;
+   clocks = < ACLK_VOP>, < HCLK_VOP>;
+   clock-names = "aclk", "iface";
+   #iommu-cells = <0>;
+   status = "disabled";
+   };
+
qos_gpu: qos@fe128000 {
compatible = "rockchip,rk3568-qos", "syscon";
reg = <0x0 0xfe128000 0x0 0x20>;
diff --git a/include/dt-bindings/soc/rockchip,vop2.h 
b/include/dt-bindings/soc/rockchip,vop2.h
new file mode 100644
index 0..0a87bc90564a7
--- /dev/null
+++ b/include/dt-bindings/soc/rockchip,vop2.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */
+
+#ifndef __DT_BINDINGS_ROCKCHIP_VOP2_H
+#define __DT_BINDINGS_ROCKCHIP_VOP2_H
+
+#define RK3568_VOP2_EP_RGB 0
+#define RK3568_VOP2_EP_HDMI1
+#define RK3568_VOP2_EP_EDP 2
+#define RK3568_VOP2_EP_MIPI0   3
+#define RK3568_VOP2_EP_LVDS0   4
+#define RK3568_VOP2_EP_MIPI1   5
+#define RK3568_VOP2_EP_LVDS1   6
+
+#endif /* __DT_BINDINGS_ROCKCHIP_VOP2_H */
-- 
2.30.2

[PATCH 18/18] [HACK, RFC] clk: rk3568: do not divide dclk_vop0

On the rk3568 we have this (simplified) situation:

 .. .-..-.
-| hpll   |--.--| /n  ||dclk_vop0|-
 `´  |  `-´`-´
 |  .-..-.
 `--| /m  ||dclk_vop1|-
 |  `-´`-´
 | .-.
 `-|hdmi_ref |-
   `-´

hpll is the PLL that drives the HDMI reference clock and the pixel
clocks for the different CRTCs (dclk_vop0/1). Between the pixel clocks
and the hpll there are programmable dividers whereas the HDMI reference
clock is directly connected to the hpll.

For the HDMI output to work the pixel clock must be the same as the HDMI
reference clock, hence the dividers must be programmed to 1. Normally a
rate change on dclk_vop0/1 propagates through to the hpll and the clock
framework picks a suitable combination of hpll and divider settings. by
accident it picks a divider setting of 1 for the standard 1080p case,
but other divider settings for most other resolutions leaving the HDMI
port non working.

This patch is not a solution, it merely puts the finger in the wound. We
leave out the divider for the composite clock for dclk_vop0 which then
leaves the divider at the bootloader default setting of 1. I assume
the divider is disturbing only for the HDMI case, but needed for other
outputs. Any thoughts how this can be handled?

Signed-off-by: Sascha Hauer 
---
 drivers/clk/rockchip/clk-rk3568.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/rockchip/clk-rk3568.c 
b/drivers/clk/rockchip/clk-rk3568.c
index 69a9e8069a486..2d04d8253ca22 100644
--- a/drivers/clk/rockchip/clk-rk3568.c
+++ b/drivers/clk/rockchip/clk-rk3568.c
@@ -1038,8 +1038,8 @@ static struct rockchip_clk_branch rk3568_clk_branches[] 
__initdata = {
RK3568_CLKGATE_CON(20), 8, GFLAGS),
GATE(HCLK_VOP, "hclk_vop", "hclk_vo", 0,
RK3568_CLKGATE_CON(20), 9, GFLAGS),
-   COMPOSITE(DCLK_VOP0, "dclk_vop0", hpll_vpll_gpll_cpll_p, 
CLK_SET_RATE_PARENT | CLK_SET_RATE_NO_REPARENT,
-   RK3568_CLKSEL_CON(39), 10, 2, MFLAGS, 0, 8, DFLAGS,
+   COMPOSITE_NODIV(DCLK_VOP0, "dclk_vop0", hpll_vpll_gpll_cpll_p, 
CLK_SET_RATE_PARENT | CLK_SET_RATE_NO_REPARENT,
+   RK3568_CLKSEL_CON(39), 10, 2, MFLAGS,
RK3568_CLKGATE_CON(20), 10, GFLAGS),
COMPOSITE(DCLK_VOP1, "dclk_vop1", hpll_vpll_gpll_cpll_p, 
CLK_SET_RATE_PARENT | CLK_SET_RATE_NO_REPARENT,
RK3568_CLKSEL_CON(40), 10, 2, MFLAGS, 0, 8, DFLAGS,
-- 
2.30.2

[PATCH 17/18] drm: rockchip: Add VOP2 driver

From: Andy Yan 

The VOP2 unit is found on Rockchip SoCs beginning with rk3566/rk3568.
It replaces the VOP unit found in the older Rockchip SoCs.

This driver has been derived from the downstream Rockchip Kernel and
heavily modified:

- All nonstandard DRM properties have been removed
- dropped struct vop2_plane_state and pass around less data between
  functions
- Dropped all DRM_FORMAT_* not known on upstream
- rework register access to get rid of excessively used macros
- Drop all waiting for framesyncs

The driver is tested with HDMI and MIPI-DSI display on a RK3568-EVB
board. Overlay support is tested with the modetest utility. AFBC support
on the cluster windows is tested with weston-simple-dmabuf-egl on
weston using the (yet to be upstreamed) panfrost driver support.

Signed-off-by: Sascha Hauer 
---
 drivers/gpu/drm/rockchip/Kconfig |6 +
 drivers/gpu/drm/rockchip/Makefile|1 +
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c  |1 +
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h  |7 +-
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c   |2 +
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h  |   15 +
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 2636 ++
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.h |  625 +
 drivers/gpu/drm/rockchip/rockchip_vop2_reg.c |  505 
 9 files changed, 3797 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_vop2_reg.c

diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index b9b156308460a..4ff0043f0ee70 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -28,6 +28,12 @@ config ROCKCHIP_VOP
  This selects support for the VOP driver. You should enable it
  on all older SoCs up to RK3399.
 
+config ROCKCHIP_VOP2
+   bool "Rockchip VOP2 driver"
+   help
+ This selects support for the VOP2 driver. You should enable it
+ on all newer SoCs beginning form RK3568.
+
 config ROCKCHIP_ANALOGIX_DP
bool "Rockchip specific extensions for Analogix DP driver"
depends on ROCKCHIP_VOP
diff --git a/drivers/gpu/drm/rockchip/Makefile 
b/drivers/gpu/drm/rockchip/Makefile
index cd6e7bb5ce9c5..29848caef5c21 100644
--- a/drivers/gpu/drm/rockchip/Makefile
+++ b/drivers/gpu/drm/rockchip/Makefile
@@ -7,6 +7,7 @@ rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \
rockchip_drm_gem.o
 rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
 
+rockchipdrm-$(CONFIG_ROCKCHIP_VOP2) += rockchip_drm_vop2.o rockchip_vop2_reg.o
 rockchipdrm-$(CONFIG_ROCKCHIP_VOP) += rockchip_drm_vop.o rockchip_vop_reg.o
 rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
 rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index 64fa5fd62c01a..2bd9acb265e5a 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -474,6 +474,7 @@ static int __init rockchip_drm_init(void)
 
num_rockchip_sub_drivers = 0;
ADD_ROCKCHIP_SUB_DRIVER(vop_platform_driver, CONFIG_ROCKCHIP_VOP);
+   ADD_ROCKCHIP_SUB_DRIVER(vop2_platform_driver, CONFIG_ROCKCHIP_VOP2);
ADD_ROCKCHIP_SUB_DRIVER(rockchip_lvds_driver,
CONFIG_ROCKCHIP_LVDS);
ADD_ROCKCHIP_SUB_DRIVER(rockchip_dp_driver,
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h 
b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index aa0909e8edf93..fd6994f21817e 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -18,7 +18,7 @@
 
 #define ROCKCHIP_MAX_FB_BUFFER 3
 #define ROCKCHIP_MAX_CONNECTOR 2
-#define ROCKCHIP_MAX_CRTC  2
+#define ROCKCHIP_MAX_CRTC  4
 
 struct drm_device;
 struct drm_connector;
@@ -31,6 +31,9 @@ struct rockchip_crtc_state {
int output_bpc;
int output_flags;
bool enable_afbc;
+   uint32_t bus_format;
+   u32 bus_flags;
+   int color_space;
 };
 #define to_rockchip_crtc_state(s) \
container_of(s, struct rockchip_crtc_state, base)
@@ -65,4 +68,6 @@ extern struct platform_driver rockchip_dp_driver;
 extern struct platform_driver rockchip_lvds_driver;
 extern struct platform_driver vop_platform_driver;
 extern struct platform_driver rk3066_hdmi_driver;
+extern struct platform_driver vop2_platform_driver;
+
 #endif /* _ROCKCHIP_DRM_DRV_H_ */
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index 3aa37e177667e..0d2cb4f3922b8 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -134,4 +134,6 @@ void rockchip_drm_mode_config_init(struct drm_device *dev)

[PATCH 01/18] drm/rockchip: dw_hdmi: Do not leave clock enabled in error case

The driver returns an error when devm_phy_optional_get() fails leaving
the previously enabled clock turned on. Change order and enable the
clock only after the phy has been acquired.

Signed-off-by: Sascha Hauer 
---
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c 
b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 830bdd5e9b7ce..8677c82716784 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -529,13 +529,6 @@ static int dw_hdmi_rockchip_bind(struct device *dev, 
struct device *master,
return ret;
}
 
-   ret = clk_prepare_enable(hdmi->vpll_clk);
-   if (ret) {
-   DRM_DEV_ERROR(hdmi->dev, "Failed to enable HDMI vpll: %d\n",
- ret);
-   return ret;
-   }
-
hdmi->phy = devm_phy_optional_get(dev, "hdmi");
if (IS_ERR(hdmi->phy)) {
ret = PTR_ERR(hdmi->phy);
@@ -544,6 +537,13 @@ static int dw_hdmi_rockchip_bind(struct device *dev, 
struct device *master,
return ret;
}
 
+   ret = clk_prepare_enable(hdmi->vpll_clk);
+   if (ret) {
+   DRM_DEV_ERROR(hdmi->dev, "Failed to enable HDMI vpll: %d\n",
+ ret);
+   return ret;
+   }
+
drm_encoder_helper_add(encoder, _hdmi_rockchip_encoder_helper_funcs);
drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_TMDS);
 
-- 
2.30.2

[PATCH 04/18] drm/rockchip: dw_hdmi: add regulator support

The RK3568 has HDMI_TX_AVDD0V9 and HDMI_TX_AVDD_1V8 supply inputs needed
for the HDMI port. add support for these to the driver for boards which
have them supplied by switchable regulators.

Signed-off-by: Sascha Hauer 
---
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 41 +++--
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c 
b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 262eef614cb12..3d7c3f6fdf223 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -76,6 +77,8 @@ struct rockchip_hdmi {
struct clk *ref_clk;
struct clk *grf_clk;
struct dw_hdmi *hdmi;
+   struct regulator *avdd_0v9;
+   struct regulator *avdd_1v8;
struct phy *phy;
 };
 
@@ -223,6 +226,14 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi 
*hdmi)
return PTR_ERR(hdmi->grf_clk);
}
 
+   hdmi->avdd_0v9 = devm_regulator_get(hdmi->dev, "avdd-0v9");
+   if (IS_ERR(hdmi->avdd_0v9))
+   return PTR_ERR(hdmi->avdd_0v9);
+
+   hdmi->avdd_1v8 = devm_regulator_get(hdmi->dev, "avdd-1v8");
+   if (IS_ERR(hdmi->avdd_1v8))
+   return PTR_ERR(hdmi->avdd_1v8);
+
return 0;
 }
 
@@ -563,11 +574,23 @@ static int dw_hdmi_rockchip_bind(struct device *dev, 
struct device *master,
return ret;
}
 
+   ret = regulator_enable(hdmi->avdd_0v9);
+   if (ret) {
+   DRM_DEV_ERROR(hdmi->dev, "failed to enable avdd0v9: %d\n", ret);
+   goto err_avdd_0v9;
+   }
+
+   ret = regulator_enable(hdmi->avdd_1v8);
+   if (ret) {
+   DRM_DEV_ERROR(hdmi->dev, "failed to enable avdd1v8: %d\n", ret);
+   goto err_avdd_1v8;
+   }
+
ret = clk_prepare_enable(hdmi->ref_clk);
if (ret) {
DRM_DEV_ERROR(hdmi->dev, "Failed to enable HDMI reference 
clock: %d\n",
  ret);
-   return ret;
+   goto err_clk;
}
 
if (hdmi->chip_data == _chip_data) {
@@ -591,10 +614,19 @@ static int dw_hdmi_rockchip_bind(struct device *dev, 
struct device *master,
 */
if (IS_ERR(hdmi->hdmi)) {
ret = PTR_ERR(hdmi->hdmi);
-   drm_encoder_cleanup(encoder);
-   clk_disable_unprepare(hdmi->ref_clk);
+   goto err_bind;
}
 
+   return 0;
+
+err_bind:
+   clk_disable_unprepare(hdmi->ref_clk);
+   drm_encoder_cleanup(encoder);
+err_clk:
+   regulator_disable(hdmi->avdd_1v8);
+err_avdd_1v8:
+   regulator_disable(hdmi->avdd_0v9);
+err_avdd_0v9:
return ret;
 }
 
@@ -605,6 +637,9 @@ static void dw_hdmi_rockchip_unbind(struct device *dev, 
struct device *master,
 
dw_hdmi_unbind(hdmi->hdmi);
clk_disable_unprepare(hdmi->ref_clk);
+
+   regulator_disable(hdmi->avdd_1v8);
+   regulator_disable(hdmi->avdd_0v9);
 }
 
 static const struct component_ops dw_hdmi_rockchip_ops = {
-- 
2.30.2

[PATCH 08/18] dt-bindings: display: rockchip: dw-hdmi: Add regulator support

Signed-off-by: Sascha Hauer 
---
 .../bindings/display/rockchip/rockchip,dw-hdmi.yaml | 6 ++
 1 file changed, 6 insertions(+)

diff --git 
a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml 
b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
index 2ab6578033da2..b9dca49aa6e05 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
@@ -28,6 +28,12 @@ properties:
   reg-io-width:
 const: 4
 
+  avdd-0v9-supply:
+description: A 0.9V supply that powers up the SoC internal circuitry.
+
+  avdd-1v8-supply:
+description: A 1.8V supply that powers up the SoC internal circuitry.
+
   clocks:
 minItems: 2
 items:
-- 
2.30.2

[PATCH 02/18] drm/rockchip: dw_hdmi: rename vpll clock to reference clock

"vpll" is a misnomer. A clock input to a device should be named after
the usage in the device, not after the clock that drives it. On the
rk3568 the same clock is driven by the HPLL.
To fix that, this patch renames the vpll clock to ref clock. The clock
name "vpll" is left for compatibility to old device trees.

Signed-off-by: Sascha Hauer 
---
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 29 -
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c 
b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 8677c82716784..e352e0404f772 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -69,7 +69,7 @@ struct rockchip_hdmi {
struct regmap *regmap;
struct drm_encoder encoder;
const struct rockchip_hdmi_chip_data *chip_data;
-   struct clk *vpll_clk;
+   struct clk *ref_clk;
struct clk *grf_clk;
struct dw_hdmi *hdmi;
struct phy *phy;
@@ -196,14 +196,17 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi 
*hdmi)
return PTR_ERR(hdmi->regmap);
}
 
-   hdmi->vpll_clk = devm_clk_get(hdmi->dev, "vpll");
-   if (PTR_ERR(hdmi->vpll_clk) == -ENOENT) {
-   hdmi->vpll_clk = NULL;
-   } else if (PTR_ERR(hdmi->vpll_clk) == -EPROBE_DEFER) {
+   hdmi->ref_clk = devm_clk_get(hdmi->dev, "ref");
+   if (PTR_ERR(hdmi->ref_clk) == -ENOENT)
+   hdmi->ref_clk = devm_clk_get(hdmi->dev, "vpll");
+
+   if (PTR_ERR(hdmi->ref_clk) == -ENOENT) {
+   hdmi->ref_clk = NULL;
+   } else if (PTR_ERR(hdmi->ref_clk) == -EPROBE_DEFER) {
return -EPROBE_DEFER;
-   } else if (IS_ERR(hdmi->vpll_clk)) {
-   DRM_DEV_ERROR(hdmi->dev, "failed to get vpll clock\n");
-   return PTR_ERR(hdmi->vpll_clk);
+   } else if (IS_ERR(hdmi->ref_clk)) {
+   DRM_DEV_ERROR(hdmi->dev, "failed to get reference clock\n");
+   return PTR_ERR(hdmi->ref_clk);
}
 
hdmi->grf_clk = devm_clk_get(hdmi->dev, "grf");
@@ -257,7 +260,7 @@ static void dw_hdmi_rockchip_encoder_mode_set(struct 
drm_encoder *encoder,
 {
struct rockchip_hdmi *hdmi = to_rockchip_hdmi(encoder);
 
-   clk_set_rate(hdmi->vpll_clk, adj_mode->clock * 1000);
+   clk_set_rate(hdmi->ref_clk, adj_mode->clock * 1000);
 }
 
 static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder)
@@ -537,9 +540,9 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct 
device *master,
return ret;
}
 
-   ret = clk_prepare_enable(hdmi->vpll_clk);
+   ret = clk_prepare_enable(hdmi->ref_clk);
if (ret) {
-   DRM_DEV_ERROR(hdmi->dev, "Failed to enable HDMI vpll: %d\n",
+   DRM_DEV_ERROR(hdmi->dev, "Failed to enable HDMI reference 
clock: %d\n",
  ret);
return ret;
}
@@ -558,7 +561,7 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct 
device *master,
if (IS_ERR(hdmi->hdmi)) {
ret = PTR_ERR(hdmi->hdmi);
drm_encoder_cleanup(encoder);
-   clk_disable_unprepare(hdmi->vpll_clk);
+   clk_disable_unprepare(hdmi->ref_clk);
}
 
return ret;
@@ -570,7 +573,7 @@ static void dw_hdmi_rockchip_unbind(struct device *dev, 
struct device *master,
struct rockchip_hdmi *hdmi = dev_get_drvdata(dev);
 
dw_hdmi_unbind(hdmi->hdmi);
-   clk_disable_unprepare(hdmi->vpll_clk);
+   clk_disable_unprepare(hdmi->ref_clk);
 }
 
 static const struct component_ops dw_hdmi_rockchip_ops = {
-- 
2.30.2

[PATCH 15/18] drm/encoder: Add of_graph port to struct drm_encoder

Add a device node to drm_encoder which corresponds with the port node
in the DT description of the encoder. This allows drivers to find the
of_graph link between a crtc and an encoder.

Signed-off-by: Sascha Hauer 
---
 include/drm/drm_encoder.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
index 6e91a0280f31b..3acd054b1eb3e 100644
--- a/include/drm/drm_encoder.h
+++ b/include/drm/drm_encoder.h
@@ -99,6 +99,8 @@ struct drm_encoder {
struct drm_device *dev;
struct list_head head;
 
+   struct device_node *port;
+
struct drm_mode_object base;
char *name;
/**
-- 
2.30.2

Re: [PATCH 2/4] drm/i915/xehpsdv: set min page-size to 64K

2021-12-08 Thread Thomas Hellström




On 12/8/21 15:34, Matthew Auld wrote:

On Wed, 8 Dec 2021 at 14:16, Ramalingam C  wrote:

From: Matthew Auld 

LMEM should be allocated at 64K granularity, since 4K page support will
eventually be dropped for LMEM when using the PPGTT.

s/will eventually be dropped/has been dropped/ as per Thomas' suggestion.


Or perhaps something along the lines of

Conditionally allocate LMEM with 64K granularity, since 4K page support 
for LMEM will be dropped on some platforms when using the PPGTT.


/Thomas






Signed-off-by: Matthew Auld 
Signed-off-by: Stuart Summers 
Signed-off-by: Ramalingam C 
Cc: Joonas Lahtinen 
Cc: Rodrigo Vivi 
Reviewed-by: Lucas De Marchi 
Reviewed-by: Thomas Hellstrom 
---
  drivers/gpu/drm/i915/gem/i915_gem_stolen.c  | 6 +-
  drivers/gpu/drm/i915/gt/intel_region_lmem.c | 5 -
  2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index bce03d74a0b4..ba90ab47d838 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -780,6 +780,7 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, 
u16 type,
 struct intel_uncore *uncore = >uncore;
 struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
 struct intel_memory_region *mem;
+   resource_size_t min_page_size;
 resource_size_t io_start;
 resource_size_t lmem_size;
 u64 lmem_base;
@@ -791,8 +792,11 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, 
u16 type,
 lmem_size = pci_resource_len(pdev, 2) - lmem_base;
 io_start = pci_resource_start(pdev, 2) + lmem_base;

+   min_page_size = HAS_64K_PAGES(i915) ? I915_GTT_PAGE_SIZE_64K :
+   I915_GTT_PAGE_SIZE_4K;
+
 mem = intel_memory_region_create(i915, lmem_base, lmem_size,
-I915_GTT_PAGE_SIZE_4K, io_start,
+min_page_size, io_start,
  type, instance,
  _region_stolen_lmem_ops);
 if (IS_ERR(mem))
diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c 
b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
index 9ea49e0a27c0..fde2dcb59809 100644
--- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
+++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
@@ -197,6 +197,7 @@ static struct intel_memory_region *setup_lmem(struct 
intel_gt *gt)
 struct intel_uncore *uncore = gt->uncore;
 struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
 struct intel_memory_region *mem;
+   resource_size_t min_page_size;
 resource_size_t io_start;
 resource_size_t lmem_size;
 int err;
@@ -211,10 +212,12 @@ static struct intel_memory_region *setup_lmem(struct 
intel_gt *gt)
 if (GEM_WARN_ON(lmem_size > pci_resource_len(pdev, 2)))
 return ERR_PTR(-ENODEV);

+   min_page_size = HAS_64K_PAGES(i915) ? I915_GTT_PAGE_SIZE_64K :
+   I915_GTT_PAGE_SIZE_4K;
 mem = intel_memory_region_create(i915,
  0,
  lmem_size,
-I915_GTT_PAGE_SIZE_4K,
+min_page_size,
  io_start,
  INTEL_MEMORY_LOCAL,
  0,
--
2.20.1

[PATCH 3/3] drm/i915: Test all device memory on probing

From: Chris Wilson 

This extends the previous sanitychecking of device memory to read/write
all the memory on the device during the device probe, ala memtest86,
as an optional module parameter: i915.memtest=1. This is not expected to
be fast, but a reasonably thorough verfification that the device memory
is accessible and doesn't return bit errors.

v2: Rebased.

Suggested-by: Matthew Auld 
Signed-off-by: Chris Wilson 
Cc: Matthew Auld 
Signed-off-by: Ramalingam C 
Reviewed-by: Matthew Auld 
---
 drivers/gpu/drm/i915/i915_params.c |  3 ++
 drivers/gpu/drm/i915/i915_params.h |  1 +
 drivers/gpu/drm/i915/intel_memory_region.c | 35 ++
 3 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c 
b/drivers/gpu/drm/i915/i915_params.c
index e07f4cfea63a..525ae832aa9a 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -140,6 +140,9 @@ i915_param_named_unsafe(invert_brightness, int, 0400,
 i915_param_named(disable_display, bool, 0400,
"Disable display (default: false)");
 
+i915_param_named(memtest, bool, 0400,
+   "Perform a read/write test of all device memory on module load 
(default: off)");
+
 i915_param_named(mmio_debug, int, 0400,
"Enable the MMIO debug code for the first N failures (default: off). "
"This may negatively affect performance.");
diff --git a/drivers/gpu/drm/i915/i915_params.h 
b/drivers/gpu/drm/i915/i915_params.h
index 8d725b64592d..c9d53ff910a0 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -64,6 +64,7 @@ struct drm_printer;
param(char *, guc_firmware_path, NULL, 0400) \
param(char *, huc_firmware_path, NULL, 0400) \
param(char *, dmc_firmware_path, NULL, 0400) \
+   param(bool, memtest, false, 0400) \
param(int, mmio_debug, -IS_ENABLED(CONFIG_DRM_I915_DEBUG_MMIO), 0600) \
param(int, edp_vswing, 0, 0400) \
param(unsigned int, reset, 3, 0600) \
diff --git a/drivers/gpu/drm/i915/intel_memory_region.c 
b/drivers/gpu/drm/i915/intel_memory_region.c
index ed65b67e2d14..bab5b5caa6d8 100644
--- a/drivers/gpu/drm/i915/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/intel_memory_region.c
@@ -93,9 +93,12 @@ static resource_size_t random_page(resource_size_t last)
return prandom_u32_max(last >> PAGE_SHIFT) << PAGE_SHIFT;
 }
 
-static int iomemtest(struct intel_memory_region *mem, const void *caller)
+static int iomemtest(struct intel_memory_region *mem,
+bool test_all,
+const void *caller)
 {
resource_size_t last = resource_size(>region) - PAGE_SIZE;
+   resource_size_t page;
int err;
 
/*
@@ -109,17 +112,25 @@ static int iomemtest(struct intel_memory_region *mem, 
const void *caller)
 * a random offset within as a quick spot check for bad memory.
 */
 
-   err = iopagetest(mem, 0, caller);
-   if (err)
-   return err;
+   if (test_all) {
+   for (page = 0; page <= last; page += PAGE_SIZE) {
+   err = iopagetest(mem, page, caller);
+   if (err)
+   return err;
+   }
+   } else {
+   err = iopagetest(mem, 0, caller);
+   if (err)
+   return err;
 
-   err = iopagetest(mem, last, caller);
-   if (err)
-   return err;
+   err = iopagetest(mem, last, caller);
+   if (err)
+   return err;
 
-   err = iopagetest(mem, random_page(last), caller);
-   if (err)
-   return err;
+   err = iopagetest(mem, random_page(last), caller);
+   if (err)
+   return err;
+   }
 
return 0;
 }
@@ -194,8 +205,8 @@ static int intel_memory_region_memtest(struct 
intel_memory_region *mem,
if (!mem->io_start)
return 0;
 
-   if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
-   err = iomemtest(mem, caller);
+   if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) || i915->params.memtest)
+   err = iomemtest(mem, i915->params.memtest, caller);
 
return err;
 }
-- 
2.20.1

[PATCH 2/3] drm/i915: Sanitycheck device iomem on probe

From: Chris Wilson 

As we setup the memory regions for the device, give each a quick test to
verify that we can read and write to the full iomem range. This ensures
that our physical addressing for the device's memory is correct, and
some reassurance that the memory is functional.

v2: wrapper for memtest [Chris]

Signed-off-by: Chris Wilson 
Cc: Matthew Auld 
Signed-off-by: Ramalingam C 
Reviewed-by: Matthew Auld 
---
 drivers/gpu/drm/i915/intel_memory_region.c | 117 +
 1 file changed, 117 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_memory_region.c 
b/drivers/gpu/drm/i915/intel_memory_region.c
index b43121609e25..ed65b67e2d14 100644
--- a/drivers/gpu/drm/i915/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/intel_memory_region.c
@@ -3,6 +3,8 @@
  * Copyright © 2019 Intel Corporation
  */
 
+#include 
+
 #include "intel_memory_region.h"
 #include "i915_drv.h"
 #include "i915_ttm_buddy_manager.h"
@@ -29,6 +31,99 @@ static const struct {
},
 };
 
+static int __iopagetest(struct intel_memory_region *mem,
+   u8 __iomem *va, int pagesize,
+   u8 value, resource_size_t offset,
+   const void *caller)
+{
+   int byte = prandom_u32_max(pagesize);
+   u8 result[3];
+
+   memset_io(va, value, pagesize); /* or GPF! */
+   wmb();
+
+   result[0] = ioread8(va);
+   result[1] = ioread8(va + byte);
+   result[2] = ioread8(va + pagesize - 1);
+   if (memchr_inv(result, value, sizeof(result))) {
+   dev_err(mem->i915->drm.dev,
+   "Failed to read back from memory region:%pR at [%pa + 
%pa] for %ps; wrote %x, read (%x, %x, %x)\n",
+   >region, >io_start, , caller,
+   value, result[0], result[1], result[2]);
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
+static int iopagetest(struct intel_memory_region *mem,
+ resource_size_t offset,
+ const void *caller)
+{
+   const u8 val[] = { 0x0, 0xa5, 0xc3, 0xf0 };
+   void __iomem *va;
+   int err;
+   int i;
+
+   va = ioremap_wc(mem->io_start + offset, PAGE_SIZE);
+   if (!va) {
+   dev_err(mem->i915->drm.dev,
+   "Failed to ioremap memory region [%pa + %px] for %ps\n",
+   >io_start, , caller);
+   return -EFAULT;
+   }
+
+   for (i = 0; i < ARRAY_SIZE(val); i++) {
+   err = __iopagetest(mem, va, PAGE_SIZE, val[i], offset, caller);
+   if (err)
+   break;
+
+   err = __iopagetest(mem, va, PAGE_SIZE, ~val[i], offset, caller);
+   if (err)
+   break;
+   }
+
+   iounmap(va);
+   return err;
+}
+
+static resource_size_t random_page(resource_size_t last)
+{
+   /* Limited to low 44b (16TiB), but should suffice for a spot check */
+   return prandom_u32_max(last >> PAGE_SHIFT) << PAGE_SHIFT;
+}
+
+static int iomemtest(struct intel_memory_region *mem, const void *caller)
+{
+   resource_size_t last = resource_size(>region) - PAGE_SIZE;
+   int err;
+
+   /*
+* Quick test to check read/write access to the iomap (backing store).
+*
+* Write a byte, read it back. If the iomapping fails, we expect
+* a GPF preventing further execution. If the backing store does not
+* exist, the read back will return garbage. We check a couple of pages,
+* the first and last of the specified region to confirm the backing
+* store + iomap does cover the entire memory region; and we check
+* a random offset within as a quick spot check for bad memory.
+*/
+
+   err = iopagetest(mem, 0, caller);
+   if (err)
+   return err;
+
+   err = iopagetest(mem, last, caller);
+   if (err)
+   return err;
+
+   err = iopagetest(mem, random_page(last), caller);
+   if (err)
+   return err;
+
+   return 0;
+}
+
 struct intel_memory_region *
 intel_memory_region_lookup(struct drm_i915_private *i915,
   u16 class, u16 instance)
@@ -90,6 +185,21 @@ void intel_memory_region_debug(struct intel_memory_region 
*mr,
   >total, >avail);
 }
 
+static int intel_memory_region_memtest(struct intel_memory_region *mem,
+  void *caller)
+{
+   struct drm_i915_private *i915 = mem->i915;
+   int err = 0;
+
+   if (!mem->io_start)
+   return 0;
+
+   if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
+   err = iomemtest(mem, caller);
+
+   return err;
+}
+
 struct intel_memory_region *
 intel_memory_region_create(struct drm_i915_private *i915,
   resource_size_t start,
@@ -126,8 +236,15 @@ intel_memory_region_create(struct drm_i915_private *i915,
goto err_free;

[PATCH 1/3] drm/i915: Exclude reserved stolen from driver use

From: Chris Wilson 

Remove the portion of stolen memory reserved for private use from driver
access.

Signed-off-by: Chris Wilson 
cc: Matthew Auld 
Signed-off-by: Ramalingam C 
Reviewed-by: Matthew Auld 
---
 drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index bce03d74a0b4..6ea3ca21cdf3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -488,6 +488,9 @@ static int i915_gem_init_stolen(struct intel_memory_region 
*mem)
return 0;
}
 
+   /* Exclude the reserved region from driver use */
+   mem->region.end = reserved_base - 1;
+
/* It is possible for the reserved area to end before the end of stolen
 * memory, so just consider the start. */
reserved_total = stolen_top - reserved_base;
-- 
2.20.1

[PATCH 0/3] drm/i915: Sanity Check for device memory region

Changes for introducing the quick test on the device memory range and
also a test of detailed validation for each addr of the range with read
and write.

Detailed testing is optionally enabled with a modparam i915.memtest=1

And third patch fixes the driver accessible stolen memory.

v2: Adding a wrapper for the memtest [Chris]

Chris Wilson (3):
  drm/i915: Exclude reserved stolen from driver use
  drm/i915: Sanitycheck device iomem on probe
  drm/i915: Test all device memory on probing

 drivers/gpu/drm/i915/gem/i915_gem_stolen.c |   3 +
 drivers/gpu/drm/i915/i915_params.c |   3 +
 drivers/gpu/drm/i915/i915_params.h |   1 +
 drivers/gpu/drm/i915/intel_memory_region.c | 128 +
 4 files changed, 135 insertions(+)

-- 
2.20.1

Re: [PATCH 2/4] drm/i915/xehpsdv: set min page-size to 64K

On Wed, 8 Dec 2021 at 14:16, Ramalingam C  wrote:
>
> From: Matthew Auld 
>
> LMEM should be allocated at 64K granularity, since 4K page support will
> eventually be dropped for LMEM when using the PPGTT.

s/will eventually be dropped/has been dropped/ as per Thomas' suggestion.

>
> Signed-off-by: Matthew Auld 
> Signed-off-by: Stuart Summers 
> Signed-off-by: Ramalingam C 
> Cc: Joonas Lahtinen 
> Cc: Rodrigo Vivi 
> Reviewed-by: Lucas De Marchi 
> Reviewed-by: Thomas Hellstrom 
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_stolen.c  | 6 +-
>  drivers/gpu/drm/i915/gt/intel_region_lmem.c | 5 -
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c 
> b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> index bce03d74a0b4..ba90ab47d838 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> @@ -780,6 +780,7 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, 
> u16 type,
> struct intel_uncore *uncore = >uncore;
> struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
> struct intel_memory_region *mem;
> +   resource_size_t min_page_size;
> resource_size_t io_start;
> resource_size_t lmem_size;
> u64 lmem_base;
> @@ -791,8 +792,11 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private 
> *i915, u16 type,
> lmem_size = pci_resource_len(pdev, 2) - lmem_base;
> io_start = pci_resource_start(pdev, 2) + lmem_base;
>
> +   min_page_size = HAS_64K_PAGES(i915) ? I915_GTT_PAGE_SIZE_64K :
> +   I915_GTT_PAGE_SIZE_4K;
> +
> mem = intel_memory_region_create(i915, lmem_base, lmem_size,
> -I915_GTT_PAGE_SIZE_4K, io_start,
> +min_page_size, io_start,
>  type, instance,
>  _region_stolen_lmem_ops);
> if (IS_ERR(mem))
> diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c 
> b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
> index 9ea49e0a27c0..fde2dcb59809 100644
> --- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
> +++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
> @@ -197,6 +197,7 @@ static struct intel_memory_region *setup_lmem(struct 
> intel_gt *gt)
> struct intel_uncore *uncore = gt->uncore;
> struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
> struct intel_memory_region *mem;
> +   resource_size_t min_page_size;
> resource_size_t io_start;
> resource_size_t lmem_size;
> int err;
> @@ -211,10 +212,12 @@ static struct intel_memory_region *setup_lmem(struct 
> intel_gt *gt)
> if (GEM_WARN_ON(lmem_size > pci_resource_len(pdev, 2)))
> return ERR_PTR(-ENODEV);
>
> +   min_page_size = HAS_64K_PAGES(i915) ? I915_GTT_PAGE_SIZE_64K :
> +   I915_GTT_PAGE_SIZE_4K;
> mem = intel_memory_region_create(i915,
>  0,
>  lmem_size,
> -I915_GTT_PAGE_SIZE_4K,
> +min_page_size,
>  io_start,
>  INTEL_MEMORY_LOCAL,
>  0,
> --
> 2.20.1
>

[PATCH 4/4] drm/i915: enforce min page size for scratch

From: Matthew Auld 

If the device needs 64K minimum GTT pages for device local-memory,
like on XEHPSDV, then we need to fail the allocation if we can't
meet it, instead of falling back to 4K pages, otherwise we can't
safely support the insertion of device local-memory pages for
this vm, since the HW expects the correct physical alignment and
size for every PTE, if we mark the page-table as 64K GTT mode.

v2: s/userpsace/userspace [Thomas]

Signed-off-by: Matthew Auld 
Signed-off-by: Ramalingam C 
Reviewed-by: Thomas Hellström 
---
 drivers/gpu/drm/i915/gt/intel_gtt.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c 
b/drivers/gpu/drm/i915/gt/intel_gtt.c
index 1428e2b9075a..b30e4478f098 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -337,6 +337,18 @@ int setup_scratch_page(struct i915_address_space *vm)
if (size == I915_GTT_PAGE_SIZE_4K)
return -ENOMEM;
 
+   /*
+* If we need 64K minimum GTT pages for device local-memory,
+* like on XEHPSDV, then we need to fail the allocation here,
+* otherwise we can't safely support the insertion of
+* local-memory pages for this vm, since the HW expects the
+* correct physical alignment and size when the page-table is
+* operating in 64K GTT mode, which includes any scratch PTEs,
+* since userspace can still touch them.
+*/
+   if (HAS_64K_PAGES(vm->i915))
+   return -ENOMEM;
+
size = I915_GTT_PAGE_SIZE_4K;
} while (1);
 }
-- 
2.20.1

[PATCH 3/4] drm/i915/gtt/xehpsdv: move scratch page to system memory

From: Matthew Auld 

On some platforms the hw has dropped support for 4K GTT pages when
dealing with LMEM, and due to the design of 64K GTT pages in the hw, we
can only mark the *entire* page-table as operating in 64K GTT mode,
since the enable bit is still on the pde, and not the pte. And since we
we still need to allow 4K GTT pages for SMEM objects, we can't have a
"normal" 4K page-table with scratch pointing to LMEM, since that's
undefined from the hw pov. The simplest solution is to just move the 64K
scratch page to SMEM on such platforms and call it a day, since that
should work for all configurations.

Signed-off-by: Matthew Auld 
Signed-off-by: Ramalingam C 
Reviewed-by: Thomas Hellstrom 
---
 drivers/gpu/drm/i915/gt/gen6_ppgtt.c  |  1 +
 drivers/gpu/drm/i915/gt/gen8_ppgtt.c  | 23 +--
 drivers/gpu/drm/i915/gt/intel_ggtt.c  |  3 +++
 drivers/gpu/drm/i915/gt/intel_gtt.c   |  2 +-
 drivers/gpu/drm/i915/gt/intel_gtt.h   |  2 ++
 drivers/gpu/drm/i915/selftests/mock_gtt.c |  2 ++
 6 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c 
b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
index 4a166d25fe60..c0d149f04949 100644
--- a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
@@ -454,6 +454,7 @@ struct i915_ppgtt *gen6_ppgtt_create(struct intel_gt *gt)
ppgtt->base.vm.cleanup = gen6_ppgtt_cleanup;
 
ppgtt->base.vm.alloc_pt_dma = alloc_pt_dma;
+   ppgtt->base.vm.alloc_scratch_dma = alloc_pt_dma;
ppgtt->base.vm.pte_encode = ggtt->vm.pte_encode;
 
err = gen6_ppgtt_init_scratch(ppgtt);
diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c 
b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
index 95c02096a61b..b012c50f7ce7 100644
--- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
@@ -776,10 +776,29 @@ struct i915_ppgtt *gen8_ppgtt_create(struct intel_gt *gt,
 */
ppgtt->vm.has_read_only = !IS_GRAPHICS_VER(gt->i915, 11, 12);
 
-   if (HAS_LMEM(gt->i915))
+   if (HAS_LMEM(gt->i915)) {
ppgtt->vm.alloc_pt_dma = alloc_pt_lmem;
-   else
+
+   /*
+* On some platforms the hw has dropped support for 4K GTT pages
+* when dealing with LMEM, and due to the design of 64K GTT
+* pages in the hw, we can only mark the *entire* page-table as
+* operating in 64K GTT mode, since the enable bit is still on
+* the pde, and not the pte. And since we still need to allow
+* 4K GTT pages for SMEM objects, we can't have a "normal" 4K
+* page-table with scratch pointing to LMEM, since that's
+* undefined from the hw pov. The simplest solution is to just
+* move the 64K scratch page to SMEM on such platforms and call
+* it a day, since that should work for all configurations.
+*/
+   if (HAS_64K_PAGES(gt->i915))
+   ppgtt->vm.alloc_scratch_dma = alloc_pt_dma;
+   else
+   ppgtt->vm.alloc_scratch_dma = alloc_pt_lmem;
+   } else {
ppgtt->vm.alloc_pt_dma = alloc_pt_dma;
+   ppgtt->vm.alloc_scratch_dma = alloc_pt_dma;
+   }
 
err = gen8_init_scratch(>vm);
if (err)
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c 
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index cbc6d2b1fd9e..d85a1050f4a8 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -941,6 +941,7 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
size = gen8_get_total_gtt_size(snb_gmch_ctl);
 
ggtt->vm.alloc_pt_dma = alloc_pt_dma;
+   ggtt->vm.alloc_scratch_dma = alloc_pt_dma;
ggtt->vm.lmem_pt_obj_flags = I915_BO_ALLOC_PM_EARLY;
 
ggtt->vm.total = (size / sizeof(gen8_pte_t)) * I915_GTT_PAGE_SIZE;
@@ -1094,6 +1095,7 @@ static int gen6_gmch_probe(struct i915_ggtt *ggtt)
ggtt->vm.total = (size / sizeof(gen6_pte_t)) * I915_GTT_PAGE_SIZE;
 
ggtt->vm.alloc_pt_dma = alloc_pt_dma;
+   ggtt->vm.alloc_scratch_dma = alloc_pt_dma;
 
ggtt->vm.clear_range = nop_clear_range;
if (!HAS_FULL_PPGTT(i915) || intel_scanout_needs_vtd_wa(i915))
@@ -1146,6 +1148,7 @@ static int i915_gmch_probe(struct i915_ggtt *ggtt)
(struct resource)DEFINE_RES_MEM(gmadr_base, ggtt->mappable_end);
 
ggtt->vm.alloc_pt_dma = alloc_pt_dma;
+   ggtt->vm.alloc_scratch_dma = alloc_pt_dma;
 
if (needs_idle_maps(i915)) {
drm_notice(>drm,
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c 
b/drivers/gpu/drm/i915/gt/intel_gtt.c
index 0dd254cb1f69..1428e2b9075a 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -301,7 +301,7 @@ int setup_scratch_page(struct i915_address_space *vm)
do {
struct

[PATCH 2/4] drm/i915/xehpsdv: set min page-size to 64K

From: Matthew Auld 

LMEM should be allocated at 64K granularity, since 4K page support will
eventually be dropped for LMEM when using the PPGTT.

Signed-off-by: Matthew Auld 
Signed-off-by: Stuart Summers 
Signed-off-by: Ramalingam C 
Cc: Joonas Lahtinen 
Cc: Rodrigo Vivi 
Reviewed-by: Lucas De Marchi 
Reviewed-by: Thomas Hellstrom 
---
 drivers/gpu/drm/i915/gem/i915_gem_stolen.c  | 6 +-
 drivers/gpu/drm/i915/gt/intel_region_lmem.c | 5 -
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index bce03d74a0b4..ba90ab47d838 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -780,6 +780,7 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, 
u16 type,
struct intel_uncore *uncore = >uncore;
struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
struct intel_memory_region *mem;
+   resource_size_t min_page_size;
resource_size_t io_start;
resource_size_t lmem_size;
u64 lmem_base;
@@ -791,8 +792,11 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, 
u16 type,
lmem_size = pci_resource_len(pdev, 2) - lmem_base;
io_start = pci_resource_start(pdev, 2) + lmem_base;
 
+   min_page_size = HAS_64K_PAGES(i915) ? I915_GTT_PAGE_SIZE_64K :
+   I915_GTT_PAGE_SIZE_4K;
+
mem = intel_memory_region_create(i915, lmem_base, lmem_size,
-I915_GTT_PAGE_SIZE_4K, io_start,
+min_page_size, io_start,
 type, instance,
 _region_stolen_lmem_ops);
if (IS_ERR(mem))
diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c 
b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
index 9ea49e0a27c0..fde2dcb59809 100644
--- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
+++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
@@ -197,6 +197,7 @@ static struct intel_memory_region *setup_lmem(struct 
intel_gt *gt)
struct intel_uncore *uncore = gt->uncore;
struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
struct intel_memory_region *mem;
+   resource_size_t min_page_size;
resource_size_t io_start;
resource_size_t lmem_size;
int err;
@@ -211,10 +212,12 @@ static struct intel_memory_region *setup_lmem(struct 
intel_gt *gt)
if (GEM_WARN_ON(lmem_size > pci_resource_len(pdev, 2)))
return ERR_PTR(-ENODEV);
 
+   min_page_size = HAS_64K_PAGES(i915) ? I915_GTT_PAGE_SIZE_64K :
+   I915_GTT_PAGE_SIZE_4K;
mem = intel_memory_region_create(i915,
 0,
 lmem_size,
-I915_GTT_PAGE_SIZE_4K,
+min_page_size,
 io_start,
 INTEL_MEMORY_LOCAL,
 0,
-- 
2.20.1

[PATCH 1/4] drm/i915: Add has_64k_pages flag

From: Stuart Summers 

Add a new platform flag, has_64k_pages, to mark the requirement of 64K
GTT page sizes or larger for device local memory access.

Also implies that we require or at least support the compact PT layout
for the ppGTT when using 64K GTT pages.

v2: More explanation for the flag [Thomas]

Signed-off-by: Stuart Summers 
Signed-off-by: Ramalingam C 
Reviewed-by: Lucas De Marchi 
---
 drivers/gpu/drm/i915/i915_drv.h  | 8 
 drivers/gpu/drm/i915/i915_pci.c  | 2 ++
 drivers/gpu/drm/i915/intel_device_info.h | 1 +
 3 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 85bb8d3107f0..e63c62f69ec5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1528,6 +1528,14 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
 #define HAS_MSLICES(dev_priv) \
(INTEL_INFO(dev_priv)->has_mslices)
 
+/*
+ * Set this flag, when platform requires 64K GTT page sizes or larger for
+ * device local memory access. Also this flag implies that we require or
+ * at least support the compact PT layout for the ppGTT when using the 64K
+ * GTT pages.
+ */
+#define HAS_64K_PAGES(dev_priv) (INTEL_INFO(dev_priv)->has_64k_pages)
+
 #define HAS_IPC(dev_priv)   (INTEL_INFO(dev_priv)->display.has_ipc)
 
 #define HAS_REGION(i915, i) (INTEL_INFO(i915)->memory_regions & (i))
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 6aaa7c644c9b..634282edadb7 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -1029,6 +1029,7 @@ static const struct intel_device_info xehpsdv_info = {
DGFX_FEATURES,
PLATFORM(INTEL_XEHPSDV),
.display = { },
+   .has_64k_pages = 1,
.pipe_mask = 0,
.platform_engine_mask =
BIT(RCS0) | BIT(BCS0) |
@@ -1047,6 +1048,7 @@ static const struct intel_device_info dg2_info = {
.graphics.rel = 55,
.media.rel = 55,
PLATFORM(INTEL_DG2),
+   .has_64k_pages = 1,
.platform_engine_mask =
BIT(RCS0) | BIT(BCS0) |
BIT(VECS0) | BIT(VECS1) |
diff --git a/drivers/gpu/drm/i915/intel_device_info.h 
b/drivers/gpu/drm/i915/intel_device_info.h
index 669f0d26c3c3..f38ac5bd837b 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -123,6 +123,7 @@ enum intel_ppgtt_type {
func(is_dgfx); \
/* Keep has_* in alphabetical order */ \
func(has_64bit_reloc); \
+   func(has_64k_pages); \
func(gpu_reset_clobbers_display); \
func(has_reset_engine); \
func(has_global_mocs); \
-- 
2.20.1

[PATCH 0/4] drm/i915: Basic enabling of 64k page support

Preparational patches for 64k page support.

Matthew Auld (3):
  drm/i915/xehpsdv: set min page-size to 64K
  drm/i915/gtt/xehpsdv: move scratch page to system memory
  drm/i915: enforce min page size for scratch

Stuart Summers (1):
  drm/i915: Add has_64k_pages flag

 drivers/gpu/drm/i915/gem/i915_gem_stolen.c  |  6 +-
 drivers/gpu/drm/i915/gt/gen6_ppgtt.c|  1 +
 drivers/gpu/drm/i915/gt/gen8_ppgtt.c| 23 +++--
 drivers/gpu/drm/i915/gt/intel_ggtt.c|  3 +++
 drivers/gpu/drm/i915/gt/intel_gtt.c | 14 -
 drivers/gpu/drm/i915/gt/intel_gtt.h |  2 ++
 drivers/gpu/drm/i915/gt/intel_region_lmem.c |  5 -
 drivers/gpu/drm/i915/i915_drv.h |  8 +++
 drivers/gpu/drm/i915/i915_pci.c |  2 ++
 drivers/gpu/drm/i915/intel_device_info.h|  1 +
 drivers/gpu/drm/i915/selftests/mock_gtt.c   |  2 ++
 11 files changed, 62 insertions(+), 5 deletions(-)

-- 
2.20.1

Re: [PATCH v2 03/11] mm/gup: migrate PIN_LONGTERM dev coherent pages to system

2021-12-08 Thread Jason Gunthorpe

On Wed, Dec 08, 2021 at 10:31:58PM +1100, Alistair Popple wrote:
> On Tuesday, 7 December 2021 5:52:43 AM AEDT Alex Sierra wrote:
> > Avoid long term pinning for Coherent device type pages. This could
> > interfere with their own device memory manager.
> > If caller tries to get user device coherent pages with PIN_LONGTERM flag
> > set, those pages will be migrated back to system memory.
> > 
> > Signed-off-by: Alex Sierra 
> >  mm/gup.c | 32 ++--
> >  1 file changed, 30 insertions(+), 2 deletions(-)
> > 
> > diff --git a/mm/gup.c b/mm/gup.c
> > index 886d6148d3d0..1572eacf07f4 100644
> > +++ b/mm/gup.c
> > @@ -1689,17 +1689,37 @@ struct page *get_dump_page(unsigned long addr)
> >  #endif /* CONFIG_ELF_CORE */
> >  
> >  #ifdef CONFIG_MIGRATION
> > +static int migrate_device_page(unsigned long address,
> > +   struct page *page)
> > +{
> > +   struct vm_area_struct *vma = find_vma(current->mm, address);
> > +   struct vm_fault vmf = {
> > +   .vma = vma,
> > +   .address = address & PAGE_MASK,
> > +   .flags = FAULT_FLAG_USER,
> > +   .pgoff = linear_page_index(vma, address),
> > +   .gfp_mask = GFP_KERNEL,
> > +   .page = page,
> > +   };
> > +   if (page->pgmap && page->pgmap->ops->migrate_to_ram)
> > +   return page->pgmap->ops->migrate_to_ram();
> 
> How does this synchronise against pgmap being released? As I understand things
> at this point we're not holding a reference on either the page or pgmap, so
> the page and therefore the pgmap may have been freed.

For sure, this can't keep touching the pages[] array after it unpinned
them:

> > if (gup_flags & FOLL_PIN) {
> > unpin_user_pages(pages, nr_pages);
   ^^^

> > } else {
> > for (i = 0; i < nr_pages; i++)
> > put_page(pages[i]);
> > }
> > +   if (is_device_page(head))
> > +   return migrate_device_page(start + page_index * PAGE_SIZE, 
> > head);

It was safe before this patch as isolate_lru_page(head) has a
get_page() inside.

Also, please try hard not to turn this function into goto spaghetti

> I think a similar problem exists for device private fault handling as well and
> it has been on my list of things to fix for a while. I think the solution is 
> to
> call try_get_page(), except it doesn't work with device pages due to the whole
> refcount thing. That issue is blocking a fair bit of work now so I've started
> looking into it.

Where is this?

Jason

Re: [Intel-gfx] [PATCH v2 10/16] drm/i915: Make i915_gem_evict_vm work correctly for already locked objects

2021-12-08 Thread Maarten Lankhorst

On 08-12-2021 13:07, Matthew Auld wrote:
> On Mon, 29 Nov 2021 at 13:58, Maarten Lankhorst
>  wrote:
>> i915_gem_execbuf will call i915_gem_evict_vm() after failing to pin
>> all objects in the first round. We are about to remove those short-term
>> pins, but even without those the objects are still locked. Add a special
>> case to allow i915_gem_evict_vm to evict locked objects as well.
>>
>> This might also allow multiple objects sharing the same resv to be evicted.
>>
>> Signed-off-by: Maarten Lankhorst 
> Reviewed-by: Matthew Auld 
>
> Do we need similar treatment for stuff like evict_for_node etc?

Unfortunately not, we would risk evicting newly bound objects when we 
completely drop short term pinning

evict_vm is the exception, because you expect it to clean up the entire vm as 
much as possible, and is called explictly, not as a part of pinning. :)

>> ---
>>  drivers/gpu/drm/i915/i915_gem_evict.c | 23 ++-
>>  1 file changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c 
>> b/drivers/gpu/drm/i915/i915_gem_evict.c
>> index 24f5e3345e43..f502a617b35c 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_evict.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
>> @@ -410,21 +410,42 @@ int i915_gem_evict_vm(struct i915_address_space *vm, 
>> struct i915_gem_ww_ctx *ww)
>> do {
>> struct i915_vma *vma, *vn;
>> LIST_HEAD(eviction_list);
>> +   LIST_HEAD(locked_eviction_list);
>>
>> list_for_each_entry(vma, >bound_list, vm_link) {
>> if (i915_vma_is_pinned(vma))
>> continue;
>>
>> +   /*
>> +* If we already own the lock, trylock fails. In 
>> case the resv
>> +* is shared among multiple objects, we still need 
>> the object ref.
>> +*/
>> +   if (ww && (dma_resv_locking_ctx(vma->obj->base.resv) 
>> == >ctx)) {
>> +   __i915_vma_pin(vma);
>> +   list_add(>evict_link, 
>> _eviction_list);
>> +   continue;
>> +   }
>> +
>> if (!i915_gem_object_trylock(vma->obj, ww))
>> continue;
>>
>> __i915_vma_pin(vma);
>> list_add(>evict_link, _list);
>> }
>> -   if (list_empty(_list))
>> +   if (list_empty(_list) && 
>> list_empty(_eviction_list))
>> break;
>>
>> ret = 0;
>> +   /* Unbind locked objects first, before unlocking the 
>> eviction_list */
>> +   list_for_each_entry_safe(vma, vn, _eviction_list, 
>> evict_link) {
>> +   __i915_vma_unpin(vma);
>> +
>> +   if (ret == 0)
>> +   ret = __i915_vma_unbind(vma);
>> +   if (ret != -EINTR) /* "Get me out of here!" */
>> +   ret = 0;
>> +   }
>> +
>> list_for_each_entry_safe(vma, vn, _list, 
>> evict_link) {
>> __i915_vma_unpin(vma);
>> if (ret == 0)
>> --
>> 2.34.0
>>

Re: [Intel-gfx] [PATCH 4/4] drm/i915: enforce min page size for scratch

2021-12-08 Thread Thomas Hellström




On 12/7/21 17:51, Ramalingam C wrote:

From: Matthew Auld 

If the device needs 64K minimum GTT pages for device local-memory,
like on XEHPSDV, then we need to fail the allocation if we can't
meet it, instead of falling back to 4K pages, otherwise we can't
safely support the insertion of device local-memory pages for
this vm, since the HW expects the correct physical alignment and
size for every PTE, if we mark the page-table as 64K GTT mode.

Signed-off-by: Matthew Auld 
Signed-off-by: Ramalingam C 
---
  drivers/gpu/drm/i915/gt/intel_gtt.c | 12 
  1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c 
b/drivers/gpu/drm/i915/gt/intel_gtt.c
index 1428e2b9075a..869b771a5fdc 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -337,6 +337,18 @@ int setup_scratch_page(struct i915_address_space *vm)
if (size == I915_GTT_PAGE_SIZE_4K)
return -ENOMEM;
  
+		/*

+* If we need 64K minimum GTT pages for device local-memory,
+* like on XEHPSDV, then we need to fail the allocation here,
+* otherwise we can't safely support the insertion of
+* local-memory pages for this vm, since the HW expects the
+* correct physical alignment and size when the page-table is
+* operating in 64K GTT mode, which includes any scratch PTEs,
+* since userpsace can still touch them.


s/userpsace/userspace/

LGTM.

Reviewed-by: Thomas Hellström 


+*/
+   if (HAS_64K_PAGES(vm->i915))
+   return -ENOMEM;
+
size = I915_GTT_PAGE_SIZE_4K;
} while (1);
  }

Re: [Intel-gfx] [PATCH v2 07/16] drm/i915: Take trylock during eviction, v2.

2021-12-08 Thread Maarten Lankhorst

On 07-12-2021 12:01, Matthew Auld wrote:
> On Mon, 29 Nov 2021 at 13:57, Maarten Lankhorst
>  wrote:
>> Now that freeing objects takes the object lock when destroying the
>> backing pages, we can confidently take the object lock even for dead
>> objects.
> That looks to be a future patch, at least with non-TTM backend? Does
> something need to be re-ordered in the series?
>
>> Use this fact to take the object lock in the shrinker, without requiring
>> a reference to the object, so all calls to unbind take the object lock.
> Hmm, the previous patch was talking about "we don't evict when
> refcount = 0", but this looks to be doing something else?

Yeah, a careful inspection of eviction code does allow evicting when refcount = 
0 using trylocks, iff we take the object lock during destruction., in order to 
free all remaining backing.

However, you can't do a blocking lock for dead objects, because VM lock is the 
inner lock in the hierarchy. Lockdep will complain (rightfully).

To free dead objects to reclaim memory, you would need to call 
drain_freed_objects(), which will deadlock if you hold either vm lock or resv 
locks.

>> This is the last step to requiring the object lock for vma_unbind.
>>
>> Changes since v1:
>> - No longer require the refcount, as every freed object now holds the lock
>>   when unbinding VMA's.
>>
>> Signed-off-by: Maarten Lankhorst 
>> ---
>>  drivers/gpu/drm/i915/gem/i915_gem_shrinker.c |  6 
>>  drivers/gpu/drm/i915/i915_gem_evict.c| 34 +---
>>  2 files changed, 35 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c 
>> b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
>> index eebff4735781..ad2123369e0d 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
>> @@ -405,12 +405,18 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, 
>> unsigned long event, void *ptr
>> list_for_each_entry_safe(vma, next,
>>  >ggtt.vm.bound_list, vm_link) {
>> unsigned long count = vma->node.size >> PAGE_SHIFT;
>> +   struct drm_i915_gem_object *obj = vma->obj;
>>
>> if (!vma->iomap || i915_vma_is_active(vma))
>> continue;
>>
>> +   if (!i915_gem_object_trylock(obj))
>> +   continue;
>> +
>> if (__i915_vma_unbind(vma) == 0)
>> freed_pages += count;
>> +
>> +   i915_gem_object_unlock(obj);
>> }
>> mutex_unlock(>ggtt.vm.mutex);
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c 
>> b/drivers/gpu/drm/i915/i915_gem_evict.c
>> index 2b73ddb11c66..286efa462eca 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_evict.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
>> @@ -58,6 +58,9 @@ mark_free(struct drm_mm_scan *scan,
>> if (i915_vma_is_pinned(vma))
>> return false;
>>
>> +   if (!i915_gem_object_trylock(vma->obj))
>> +   return false;
>> +
>> list_add(>evict_link, unwind);
>> return drm_mm_scan_add_block(scan, >node);
>>  }
>> @@ -178,6 +181,7 @@ i915_gem_evict_something(struct i915_address_space *vm,
>> list_for_each_entry_safe(vma, next, _list, evict_link) {
>> ret = drm_mm_scan_remove_block(, >node);
>> BUG_ON(ret);
>> +   i915_gem_object_unlock(vma->obj);
>> }
>>
>> /*
>> @@ -222,10 +226,12 @@ i915_gem_evict_something(struct i915_address_space *vm,
>>  * of any of our objects, thus corrupting the list).
>>  */
>> list_for_each_entry_safe(vma, next, _list, evict_link) {
>> -   if (drm_mm_scan_remove_block(, >node))
>> +   if (drm_mm_scan_remove_block(, >node)) {
>> __i915_vma_pin(vma);
>> -   else
>> +   } else {
>> list_del(>evict_link);
>> +   i915_gem_object_unlock(vma->obj);
>> +   }
>> }
>>
>> /* Unbinding will emit any required flushes */
>> @@ -234,16 +240,22 @@ i915_gem_evict_something(struct i915_address_space *vm,
>> __i915_vma_unpin(vma);
>> if (ret == 0)
>> ret = __i915_vma_unbind(vma);
>> +
>> +   i915_gem_object_unlock(vma->obj);
>> }
>>
>> while (ret == 0 && (node = drm_mm_scan_color_evict())) {
>> vma = container_of(node, struct i915_vma, node);
>>
>> +
>> /* If we find any non-objects (!vma), we cannot evict them */
>> -   if (vma->node.color != I915_COLOR_UNEVICTABLE)
>> +   if (vma->node.color != I915_COLOR_UNEVICTABLE &&
>> +   i915_gem_object_trylock(vma->obj)) {
>> ret = __i915_vma_unbind(vma);
>> -   else
>> -   ret = -ENOSPC; /* XXX search failed,

Re: [Intel-gfx] [PATCH 3/4] drm/i915/gtt/xehpsdv: move scratch page to system memory

2021-12-08 Thread Thomas Hellström




On 12/7/21 17:51, Ramalingam C wrote:

From: Matthew Auld 

On some platforms the hw has dropped support for 4K GTT pages when
dealing with LMEM, and due to the design of 64K GTT pages in the hw, we
can only mark the *entire* page-table as operating in 64K GTT mode,
since the enable bit is still on the pde, and not the pte. And since we
we

we we

still need to allow 4K GTT pages for SMEM objects, we can't have a
"normal" 4K page-table with scratch pointing to LMEM, since that's
undefined from the hw pov. The simplest solution is to just move the 64K
scratch page to SMEM on such platforms and call it a day, since that
should work for all configurations.

Signed-off-by: Matthew Auld 
Signed-off-by: Ramalingam C 


LGTM.

Reviewed-by: Thomas Hellström 



---
  drivers/gpu/drm/i915/gt/gen6_ppgtt.c  |  1 +
  drivers/gpu/drm/i915/gt/gen8_ppgtt.c  | 23 +--
  drivers/gpu/drm/i915/gt/intel_ggtt.c  |  3 +++
  drivers/gpu/drm/i915/gt/intel_gtt.c   |  2 +-
  drivers/gpu/drm/i915/gt/intel_gtt.h   |  2 ++
  drivers/gpu/drm/i915/selftests/mock_gtt.c |  2 ++
  6 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c 
b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
index 4a166d25fe60..c0d149f04949 100644
--- a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
@@ -454,6 +454,7 @@ struct i915_ppgtt *gen6_ppgtt_create(struct intel_gt *gt)
ppgtt->base.vm.cleanup = gen6_ppgtt_cleanup;
  
  	ppgtt->base.vm.alloc_pt_dma = alloc_pt_dma;

+   ppgtt->base.vm.alloc_scratch_dma = alloc_pt_dma;
ppgtt->base.vm.pte_encode = ggtt->vm.pte_encode;
  
  	err = gen6_ppgtt_init_scratch(ppgtt);

diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c 
b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
index 95c02096a61b..b012c50f7ce7 100644
--- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
@@ -776,10 +776,29 @@ struct i915_ppgtt *gen8_ppgtt_create(struct intel_gt *gt,
 */
ppgtt->vm.has_read_only = !IS_GRAPHICS_VER(gt->i915, 11, 12);
  
-	if (HAS_LMEM(gt->i915))

+   if (HAS_LMEM(gt->i915)) {
ppgtt->vm.alloc_pt_dma = alloc_pt_lmem;
-   else
+
+   /*
+* On some platforms the hw has dropped support for 4K GTT pages
+* when dealing with LMEM, and due to the design of 64K GTT
+* pages in the hw, we can only mark the *entire* page-table as
+* operating in 64K GTT mode, since the enable bit is still on
+* the pde, and not the pte. And since we still need to allow
+* 4K GTT pages for SMEM objects, we can't have a "normal" 4K
+* page-table with scratch pointing to LMEM, since that's
+* undefined from the hw pov. The simplest solution is to just
+* move the 64K scratch page to SMEM on such platforms and call
+* it a day, since that should work for all configurations.
+*/
+   if (HAS_64K_PAGES(gt->i915))
+   ppgtt->vm.alloc_scratch_dma = alloc_pt_dma;
+   else
+   ppgtt->vm.alloc_scratch_dma = alloc_pt_lmem;
+   } else {
ppgtt->vm.alloc_pt_dma = alloc_pt_dma;
+   ppgtt->vm.alloc_scratch_dma = alloc_pt_dma;
+   }
  
  	err = gen8_init_scratch(>vm);

if (err)
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c 
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index cbc6d2b1fd9e..d85a1050f4a8 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -941,6 +941,7 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
size = gen8_get_total_gtt_size(snb_gmch_ctl);
  
  	ggtt->vm.alloc_pt_dma = alloc_pt_dma;

+   ggtt->vm.alloc_scratch_dma = alloc_pt_dma;
ggtt->vm.lmem_pt_obj_flags = I915_BO_ALLOC_PM_EARLY;
  
  	ggtt->vm.total = (size / sizeof(gen8_pte_t)) * I915_GTT_PAGE_SIZE;

@@ -1094,6 +1095,7 @@ static int gen6_gmch_probe(struct i915_ggtt *ggtt)
ggtt->vm.total = (size / sizeof(gen6_pte_t)) * I915_GTT_PAGE_SIZE;
  
  	ggtt->vm.alloc_pt_dma = alloc_pt_dma;

+   ggtt->vm.alloc_scratch_dma = alloc_pt_dma;
  
  	ggtt->vm.clear_range = nop_clear_range;

if (!HAS_FULL_PPGTT(i915) || intel_scanout_needs_vtd_wa(i915))
@@ -1146,6 +1148,7 @@ static int i915_gmch_probe(struct i915_ggtt *ggtt)
(struct resource)DEFINE_RES_MEM(gmadr_base, ggtt->mappable_end);
  
  	ggtt->vm.alloc_pt_dma = alloc_pt_dma;

+   ggtt->vm.alloc_scratch_dma = alloc_pt_dma;
  
  	if (needs_idle_maps(i915)) {

drm_notice(>drm,
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c 
b/drivers/gpu/drm/i915/gt/intel_gtt.c
index 0dd254cb1f69..1428e2b9075a 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -301,7 +301,7 @@ int setup_scratch_page(struct i915_address_space *vm)

Re: [Intel-gfx] [PATCH 1/4] drm/i915: Add has_64k_pages flag

2021-12-08 Thread Intel




On 12/8/21 13:59, Matthew Auld wrote:

On Wed, 8 Dec 2021 at 12:43, Thomas Hellström (Intel)
 wrote:

Hi,

On 12/7/21 17:51, Ramalingam C wrote:

From: Stuart Summers 

Add a new platform flag, has_64k_pages, for platforms supporting
base page sizes of 64k.

Signed-off-by: Stuart Summers 
Signed-off-by: Ramalingam C 
Reviewed-by: Lucas De Marchi 
---
   drivers/gpu/drm/i915/i915_drv.h  | 2 ++
   drivers/gpu/drm/i915/i915_pci.c  | 2 ++
   drivers/gpu/drm/i915/intel_device_info.h | 1 +
   3 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 85bb8d3107f0..6132163e1cb3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1528,6 +1528,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
   #define HAS_MSLICES(dev_priv) \
   (INTEL_INFO(dev_priv)->has_mslices)

+#define HAS_64K_PAGES(dev_priv) (INTEL_INFO(dev_priv)->has_64k_pages)
+

Could we please have some documentation of the exact meaning of this flag.
Like, smallest page size of LMEM is 64K. Hardware supports 64k pages etc?

Something like: "Set if the device requires 64K GTT page sizes or
larger for device local memory access. Also implies that we require or
at least support the compact PT layout for the ppGTT when using 64K
GTT pages."


Sounds great.

Thanks,

Thomas

Re: [PATCH v2 06/16] drm/i915: Ensure gem_contexts selftests work with unbind changes.

2021-12-08 Thread Maarten Lankhorst

On 07-12-2021 11:44, Matthew Auld wrote:
> On Mon, 29 Nov 2021 at 13:57, Maarten Lankhorst
>  wrote:
>> In the next commit, we don't evict when refcount = 0.
>>
>> igt_vm_isolation() continuously tries to pin/unpin at same address,
>> but also calls put() on the object, which means the object may not
>> be unpinned in time.
>>
>> Instead of this, re-use the same object over and over, so they can
>> be unbound as required.
>>
>> Signed-off-by: Maarten Lankhorst 
> Is this something to be worried about in the real world, outside of
> the selftests?

I don't think userspace could hit it because the race is small, it would need 
to free an object, then immediately try to softpin a new object in the same 
place.

It could be fixed, but it would require a massive rework of eviction. It could 
eventually be done, but requires fixing the entire vm locking. I don't think 
userspace

will hit it, except if it tried deliberately. If it does turn out to be a 
problem, a workaround would be only calling i915_gem_evict_vm() without locks, 
so it can call drain_freed_objects as needed. This requires some surgery

to make execbuf handle the case where we may drop all locks when evicting.


>> ---
>>  .../drm/i915/gem/selftests/i915_gem_context.c | 54 +++
>>  1 file changed, 32 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c 
>> b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
>> index b32f7fed2d9c..3fc595b57cf4 100644
>> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
>> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
>> @@ -1481,10 +1481,10 @@ static int check_scratch(struct i915_address_space 
>> *vm, u64 offset)
>>
>>  static int write_to_scratch(struct i915_gem_context *ctx,
>> struct intel_engine_cs *engine,
>> +   struct drm_i915_gem_object *obj,
>> u64 offset, u32 value)
>>  {
>> struct drm_i915_private *i915 = ctx->i915;
>> -   struct drm_i915_gem_object *obj;
>> struct i915_address_space *vm;
>> struct i915_request *rq;
>> struct i915_vma *vma;
>> @@ -1497,15 +1497,9 @@ static int write_to_scratch(struct i915_gem_context 
>> *ctx,
>> if (err)
>> return err;
>>
>> -   obj = i915_gem_object_create_internal(i915, PAGE_SIZE);
>> -   if (IS_ERR(obj))
>> -   return PTR_ERR(obj);
>> -
>> cmd = i915_gem_object_pin_map_unlocked(obj, I915_MAP_WB);
>> -   if (IS_ERR(cmd)) {
>> -   err = PTR_ERR(cmd);
>> -   goto out;
>> -   }
>> +   if (IS_ERR(cmd))
>> +   return PTR_ERR(cmd);
>>
>> *cmd++ = MI_STORE_DWORD_IMM_GEN4;
>> if (GRAPHICS_VER(i915) >= 8) {
>> @@ -1569,17 +1563,19 @@ static int write_to_scratch(struct i915_gem_context 
>> *ctx,
>> i915_vma_unpin(vma);
>>  out_vm:
>> i915_vm_put(vm);
>> -out:
>> -   i915_gem_object_put(obj);
>> +
>> +   if (!err)
>> +   err = i915_gem_object_wait(obj, 0, MAX_SCHEDULE_TIMEOUT);
>> +
>> return err;
>>  }
>>
>>  static int read_from_scratch(struct i915_gem_context *ctx,
>>  struct intel_engine_cs *engine,
>> +struct drm_i915_gem_object *obj,
>>  u64 offset, u32 *value)
>>  {
>> struct drm_i915_private *i915 = ctx->i915;
>> -   struct drm_i915_gem_object *obj;
>> struct i915_address_space *vm;
>> const u32 result = 0x100;
>> struct i915_request *rq;
>> @@ -1594,10 +1590,6 @@ static int read_from_scratch(struct i915_gem_context 
>> *ctx,
>> if (err)
>> return err;
>>
>> -   obj = i915_gem_object_create_internal(i915, PAGE_SIZE);
>> -   if (IS_ERR(obj))
>> -   return PTR_ERR(obj);
>> -
>> if (GRAPHICS_VER(i915) >= 8) {
>> const u32 GPR0 = engine->mmio_base + 0x600;
>>
>> @@ -1615,7 +1607,7 @@ static int read_from_scratch(struct i915_gem_context 
>> *ctx,
>> cmd = i915_gem_object_pin_map_unlocked(obj, I915_MAP_WB);
>> if (IS_ERR(cmd)) {
>> err = PTR_ERR(cmd);
>> -   goto out;
>> +   goto err_unpin;
>> }
>>
>> memset(cmd, POISON_INUSE, PAGE_SIZE);
>> @@ -1651,7 +1643,7 @@ static int read_from_scratch(struct i915_gem_context 
>> *ctx,
>> cmd = i915_gem_object_pin_map_unlocked(obj, I915_MAP_WB);
>> if (IS_ERR(cmd)) {
>> err = PTR_ERR(cmd);
>> -   goto out;
>> +   goto err_unpin;
>> }
>>
>> memset(cmd, POISON_INUSE, PAGE_SIZE);
>> @@ -1722,8 +1714,10 @@ static int read_from_scratch(struct i915_gem_context 
>> *ctx,
>> i915_vma_unpin(vma);
>>  out_vm:
>> i915_vm_put(vm);

Re: [PATCH linux-next] drm/i915/display: Delete redundant post_mask assignment

2021-12-08 Thread Jani Nikula

On Wed, 08 Dec 2021, Ville Syrjälä  wrote:
> On Wed, Dec 08, 2021 at 07:46:19AM +, cgel@gmail.com wrote:
>> From: luo penghao 
>> 
>> This value will be overwritten by the following if statement, even
>> if the if is not executed, the value will not be used
>> 
>> The clang_analyzer complains as follows:
>> 
>> Value stored to 'port_mask' is never read
>> 
>> Reported-by: Zeal Robot 
>> Signed-off-by: luo penghao 
>> ---
>>  drivers/gpu/drm/i915/display/intel_ddi.c | 1 -
>>  1 file changed, 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c 
>> b/drivers/gpu/drm/i915/display/intel_ddi.c
>> index bd18432..3aad0c3 100644
>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> @@ -2011,7 +2011,6 @@ void intel_ddi_sanitize_encoder_pll_mapping(struct 
>> intel_encoder *encoder)
>>  return;
>>  }
>>  
>> -port_mask = BIT(encoder->port);
>>  ddi_clk_needed = encoder->base.crtc;
>>  
>>  if (encoder->type == INTEL_OUTPUT_DSI) {
>
> 'port_mask' declaration could now be moved into this block.

I've basically given up on patches from cgel@gmail.com,
luo.peng...@zte.com.cn, zea...@zte.com.cn, etc. The patches are the
simplest possible changes to silence static analyzer warnings, with no
further analysis of the issues, and no follow-up on questions.

Until that changes, I think we're better off just regarding them static
as analyzer reports.


BR,
Jani.


>
>> -- 
>> 2.15.2
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [PATCH linux-next] drm/i915/display: Delete redundant post_mask assignment

2021-12-08 Thread Ville Syrjälä

On Wed, Dec 08, 2021 at 07:46:19AM +, cgel@gmail.com wrote:
> From: luo penghao 
> 
> This value will be overwritten by the following if statement, even
> if the if is not executed, the value will not be used
> 
> The clang_analyzer complains as follows:
> 
> Value stored to 'port_mask' is never read
> 
> Reported-by: Zeal Robot 
> Signed-off-by: luo penghao 
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c 
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index bd18432..3aad0c3 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -2011,7 +2011,6 @@ void intel_ddi_sanitize_encoder_pll_mapping(struct 
> intel_encoder *encoder)
>   return;
>   }
>  
> - port_mask = BIT(encoder->port);
>   ddi_clk_needed = encoder->base.crtc;
>  
>   if (encoder->type == INTEL_OUTPUT_DSI) {

'port_mask' declaration could now be moved into this block.

> -- 
> 2.15.2
> 

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 1/4] drm/i915: Add has_64k_pages flag

On Wed, 8 Dec 2021 at 12:43, Thomas Hellström (Intel)
 wrote:
>
> Hi,
>
> On 12/7/21 17:51, Ramalingam C wrote:
> > From: Stuart Summers 
> >
> > Add a new platform flag, has_64k_pages, for platforms supporting
> > base page sizes of 64k.
> >
> > Signed-off-by: Stuart Summers 
> > Signed-off-by: Ramalingam C 
> > Reviewed-by: Lucas De Marchi 
> > ---
> >   drivers/gpu/drm/i915/i915_drv.h  | 2 ++
> >   drivers/gpu/drm/i915/i915_pci.c  | 2 ++
> >   drivers/gpu/drm/i915/intel_device_info.h | 1 +
> >   3 files changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 85bb8d3107f0..6132163e1cb3 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1528,6 +1528,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
> >   #define HAS_MSLICES(dev_priv) \
> >   (INTEL_INFO(dev_priv)->has_mslices)
> >
> > +#define HAS_64K_PAGES(dev_priv) (INTEL_INFO(dev_priv)->has_64k_pages)
> > +
>
> Could we please have some documentation of the exact meaning of this flag.
> Like, smallest page size of LMEM is 64K. Hardware supports 64k pages etc?

Something like: "Set if the device requires 64K GTT page sizes or
larger for device local memory access. Also implies that we require or
at least support the compact PT layout for the ppGTT when using 64K
GTT pages."

>
> for future reference.
>
> /Thomas
>
>
>
>
>
> >   #define HAS_IPC(dev_priv)
> > (INTEL_INFO(dev_priv)->display.has_ipc)
> >
> >   #define HAS_REGION(i915, i) (INTEL_INFO(i915)->memory_regions & (i))
> > diff --git a/drivers/gpu/drm/i915/i915_pci.c 
> > b/drivers/gpu/drm/i915/i915_pci.c
> > index 6aaa7c644c9b..634282edadb7 100644
> > --- a/drivers/gpu/drm/i915/i915_pci.c
> > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > @@ -1029,6 +1029,7 @@ static const struct intel_device_info xehpsdv_info = {
> >   DGFX_FEATURES,
> >   PLATFORM(INTEL_XEHPSDV),
> >   .display = { },
> > + .has_64k_pages = 1,
> >   .pipe_mask = 0,
> >   .platform_engine_mask =
> >   BIT(RCS0) | BIT(BCS0) |
> > @@ -1047,6 +1048,7 @@ static const struct intel_device_info dg2_info = {
> >   .graphics.rel = 55,
> >   .media.rel = 55,
> >   PLATFORM(INTEL_DG2),
> > + .has_64k_pages = 1,
> >   .platform_engine_mask =
> >   BIT(RCS0) | BIT(BCS0) |
> >   BIT(VECS0) | BIT(VECS1) |
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.h 
> > b/drivers/gpu/drm/i915/intel_device_info.h
> > index 669f0d26c3c3..f38ac5bd837b 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.h
> > +++ b/drivers/gpu/drm/i915/intel_device_info.h
> > @@ -123,6 +123,7 @@ enum intel_ppgtt_type {
> >   func(is_dgfx); \
> >   /* Keep has_* in alphabetical order */ \
> >   func(has_64bit_reloc); \
> > + func(has_64k_pages); \
> >   func(gpu_reset_clobbers_display); \
> >   func(has_reset_engine); \
> >   func(has_global_mocs); \

Re: [Intel-gfx] [PATCH 2/4] drm/i915/xehpsdv: set min page-size to 64K

2021-12-08 Thread Intel




On 12/7/21 17:51, Ramalingam C wrote:

From: Matthew Auld 

LMEM should be allocated at 64K granularity, since 4K page support will
eventually be dropped for LMEM when using the PPGTT.

Signed-off-by: Matthew Auld 
Signed-off-by: Stuart Summers 
Signed-off-by: Ramalingam C 
Cc: Joonas Lahtinen 
Cc: Rodrigo Vivi 
Reviewed-by: Lucas De Marchi 



Reviewed-by: Thomas Hellstrom 



---
  drivers/gpu/drm/i915/gem/i915_gem_stolen.c  | 6 +-
  drivers/gpu/drm/i915/gt/intel_region_lmem.c | 5 -
  2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index bce03d74a0b4..ba90ab47d838 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -780,6 +780,7 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, 
u16 type,
struct intel_uncore *uncore = >uncore;
struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
struct intel_memory_region *mem;
+   resource_size_t min_page_size;
resource_size_t io_start;
resource_size_t lmem_size;
u64 lmem_base;
@@ -791,8 +792,11 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, 
u16 type,
lmem_size = pci_resource_len(pdev, 2) - lmem_base;
io_start = pci_resource_start(pdev, 2) + lmem_base;
  
+	min_page_size = HAS_64K_PAGES(i915) ? I915_GTT_PAGE_SIZE_64K :

+   I915_GTT_PAGE_SIZE_4K;
+
mem = intel_memory_region_create(i915, lmem_base, lmem_size,
-I915_GTT_PAGE_SIZE_4K, io_start,
+min_page_size, io_start,
 type, instance,
 _region_stolen_lmem_ops);
if (IS_ERR(mem))
diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c 
b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
index 9ea49e0a27c0..fde2dcb59809 100644
--- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
+++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
@@ -197,6 +197,7 @@ static struct intel_memory_region *setup_lmem(struct 
intel_gt *gt)
struct intel_uncore *uncore = gt->uncore;
struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
struct intel_memory_region *mem;
+   resource_size_t min_page_size;
resource_size_t io_start;
resource_size_t lmem_size;
int err;
@@ -211,10 +212,12 @@ static struct intel_memory_region *setup_lmem(struct 
intel_gt *gt)
if (GEM_WARN_ON(lmem_size > pci_resource_len(pdev, 2)))
return ERR_PTR(-ENODEV);
  
+	min_page_size = HAS_64K_PAGES(i915) ? I915_GTT_PAGE_SIZE_64K :

+   I915_GTT_PAGE_SIZE_4K;
mem = intel_memory_region_create(i915,
 0,
 lmem_size,
-I915_GTT_PAGE_SIZE_4K,
+min_page_size,
 io_start,
 INTEL_MEMORY_LOCAL,
 0,

Re: [Intel-gfx] [PATCH 1/4] drm/i915: Add has_64k_pages flag

2021-12-08 Thread Intel


Hi,

On 12/7/21 17:51, Ramalingam C wrote:

From: Stuart Summers 

Add a new platform flag, has_64k_pages, for platforms supporting
base page sizes of 64k.

Signed-off-by: Stuart Summers 
Signed-off-by: Ramalingam C 
Reviewed-by: Lucas De Marchi 
---
  drivers/gpu/drm/i915/i915_drv.h  | 2 ++
  drivers/gpu/drm/i915/i915_pci.c  | 2 ++
  drivers/gpu/drm/i915/intel_device_info.h | 1 +
  3 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 85bb8d3107f0..6132163e1cb3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1528,6 +1528,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
  #define HAS_MSLICES(dev_priv) \
(INTEL_INFO(dev_priv)->has_mslices)
  
+#define HAS_64K_PAGES(dev_priv) (INTEL_INFO(dev_priv)->has_64k_pages)

+


Could we please have some documentation of the exact meaning of this flag.
Like, smallest page size of LMEM is 64K. Hardware supports 64k pages etc?

for future reference.

/Thomas






  #define HAS_IPC(dev_priv)  (INTEL_INFO(dev_priv)->display.has_ipc)
  
  #define HAS_REGION(i915, i) (INTEL_INFO(i915)->memory_regions & (i))

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 6aaa7c644c9b..634282edadb7 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -1029,6 +1029,7 @@ static const struct intel_device_info xehpsdv_info = {
DGFX_FEATURES,
PLATFORM(INTEL_XEHPSDV),
.display = { },
+   .has_64k_pages = 1,
.pipe_mask = 0,
.platform_engine_mask =
BIT(RCS0) | BIT(BCS0) |
@@ -1047,6 +1048,7 @@ static const struct intel_device_info dg2_info = {
.graphics.rel = 55,
.media.rel = 55,
PLATFORM(INTEL_DG2),
+   .has_64k_pages = 1,
.platform_engine_mask =
BIT(RCS0) | BIT(BCS0) |
BIT(VECS0) | BIT(VECS1) |
diff --git a/drivers/gpu/drm/i915/intel_device_info.h 
b/drivers/gpu/drm/i915/intel_device_info.h
index 669f0d26c3c3..f38ac5bd837b 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -123,6 +123,7 @@ enum intel_ppgtt_type {
func(is_dgfx); \
/* Keep has_* in alphabetical order */ \
func(has_64bit_reloc); \
+   func(has_64k_pages); \
func(gpu_reset_clobbers_display); \
func(has_reset_engine); \
func(has_global_mocs); \

Re: [PATCH] drm: rcar-du: Use dev_err_probe() helper

2021-12-08 Thread Geert Uytterhoeven

Hi Kieran,

On Wed, Dec 8, 2021 at 12:57 PM Kieran Bingham
 wrote:
> Quoting Geert Uytterhoeven (2021-12-08 10:30:53)
> > Use the dev_err_probe() helper, instead of open-coding the same
> > operation.
> >
> > Signed-off-by: Geert Uytterhoeven 
> > ---
> >  drivers/gpu/drm/rcar-du/rcar_du_drv.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c 
> > b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > index 5612a9e7a9056cf7..86eeda769e2ebd10 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > @@ -661,9 +661,8 @@ static int rcar_du_probe(struct platform_device *pdev)
> > /* DRM/KMS objects */
> > ret = rcar_du_modeset_init(rcdu);
> > if (ret < 0) {
> > -   if (ret != -EPROBE_DEFER)
> > -   dev_err(>dev,
> > -   "failed to initialize DRM/KMS (%d)\n", ret);
> > +   dev_err_probe(>dev, ret,
> > + "failed to initialize DRM/KMS\n");
>
> I've just learned that dev_err_probe() sets a 'reason' for the deferral.
> Seems like a nice feature when exploring devices that are still waiting
> to probe. Is the message still appropriate enough in that case?
>
> I think it's probably fine, so

I have no idea why it could fail. So if you think the message is
fine, it must be fine ;-)

> Reviewed-by: Kieran Bingham 

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [Intel-gfx] [PATCH v2 10/16] drm/i915: Make i915_gem_evict_vm work correctly for already locked objects

On Mon, 29 Nov 2021 at 13:58, Maarten Lankhorst
 wrote:
>
> i915_gem_execbuf will call i915_gem_evict_vm() after failing to pin
> all objects in the first round. We are about to remove those short-term
> pins, but even without those the objects are still locked. Add a special
> case to allow i915_gem_evict_vm to evict locked objects as well.
>
> This might also allow multiple objects sharing the same resv to be evicted.
>
> Signed-off-by: Maarten Lankhorst 

Reviewed-by: Matthew Auld 

Do we need similar treatment for stuff like evict_for_node etc?

> ---
>  drivers/gpu/drm/i915/i915_gem_evict.c | 23 ++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c 
> b/drivers/gpu/drm/i915/i915_gem_evict.c
> index 24f5e3345e43..f502a617b35c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_evict.c
> +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
> @@ -410,21 +410,42 @@ int i915_gem_evict_vm(struct i915_address_space *vm, 
> struct i915_gem_ww_ctx *ww)
> do {
> struct i915_vma *vma, *vn;
> LIST_HEAD(eviction_list);
> +   LIST_HEAD(locked_eviction_list);
>
> list_for_each_entry(vma, >bound_list, vm_link) {
> if (i915_vma_is_pinned(vma))
> continue;
>
> +   /*
> +* If we already own the lock, trylock fails. In case 
> the resv
> +* is shared among multiple objects, we still need 
> the object ref.
> +*/
> +   if (ww && (dma_resv_locking_ctx(vma->obj->base.resv) 
> == >ctx)) {
> +   __i915_vma_pin(vma);
> +   list_add(>evict_link, 
> _eviction_list);
> +   continue;
> +   }
> +
> if (!i915_gem_object_trylock(vma->obj, ww))
> continue;
>
> __i915_vma_pin(vma);
> list_add(>evict_link, _list);
> }
> -   if (list_empty(_list))
> +   if (list_empty(_list) && 
> list_empty(_eviction_list))
> break;
>
> ret = 0;
> +   /* Unbind locked objects first, before unlocking the 
> eviction_list */
> +   list_for_each_entry_safe(vma, vn, _eviction_list, 
> evict_link) {
> +   __i915_vma_unpin(vma);
> +
> +   if (ret == 0)
> +   ret = __i915_vma_unbind(vma);
> +   if (ret != -EINTR) /* "Get me out of here!" */
> +   ret = 0;
> +   }
> +
> list_for_each_entry_safe(vma, vn, _list, evict_link) 
> {
> __i915_vma_unpin(vma);
> if (ret == 0)
> --
> 2.34.0
>

Re: [Intel-gfx] [PATCH v2 09/16] drm/i915: Ensure i915_vma tests do not get -ENOSPC with the locking changes.

On Wed, 8 Dec 2021 at 11:49, Matthew Auld
 wrote:
>
> On Mon, 29 Nov 2021 at 13:58, Maarten Lankhorst
>  wrote:
> >
> > Now that we require locking to evict, multiple vmas from the same object
> > might not be evicted. This is expected and required, because execbuf will
> > move to short-term pinning by using the lock only. This will cause these
> > tests to fail, because they create a ton of vma's for the same object.
> >
> > Unbind manually to prevent spurious -ENOSPC in those mock tests.
> >
> > Signed-off-by: Maarten Lankhorst 
>
> Hmm, do we need this? It looks like we should be able to handle such
> scenarios, with already locked objects sharing the same dma-resv? Or
> is something else going on here?

Oh, because trylock still "fails' in such cases? Do the later changes
to evict_vm, where it is able to handle already locked objects fix
this? Do we not need similar treatment for things like evict_for_node?

>
> > ---
> >  drivers/gpu/drm/i915/selftests/i915_vma.c | 17 -
> >  1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/selftests/i915_vma.c 
> > b/drivers/gpu/drm/i915/selftests/i915_vma.c
> > index 1f10fe36619b..5c5809dfe9b2 100644
> > --- a/drivers/gpu/drm/i915/selftests/i915_vma.c
> > +++ b/drivers/gpu/drm/i915/selftests/i915_vma.c
> > @@ -691,7 +691,11 @@ static int igt_vma_rotate_remap(void *arg)
> > }
> >
> > i915_vma_unpin(vma);
> > -
> > +   err = i915_vma_unbind(vma);
> > +   if (err) {
> > +   pr_err("Unbinding returned 
> > %i\n", err);
> > +   goto out_object;
> > +   }
> > cond_resched();
> > }
> > }
> > @@ -848,6 +852,11 @@ static int igt_vma_partial(void *arg)
> >
> > i915_vma_unpin(vma);
> > nvma++;
> > +   err = i915_vma_unbind(vma);
> > +   if (err) {
> > +   pr_err("Unbinding returned %i\n", 
> > err);
> > +   goto out_object;
> > +   }
> >
> > cond_resched();
> > }
> > @@ -882,6 +891,12 @@ static int igt_vma_partial(void *arg)
> >
> > i915_vma_unpin(vma);
> >
> > +   err = i915_vma_unbind(vma);
> > +   if (err) {
> > +   pr_err("Unbinding returned %i\n", err);
> > +   goto out_object;
> > +   }
> > +
> > count = 0;
> > list_for_each_entry(vma, >vma.list, obj_link)
> > count++;
> > --
> > 2.34.0
> >

Re: [PATCH] drm: rcar-du: Use dev_err_probe() helper

2021-12-08 Thread Kieran Bingham

Quoting Geert Uytterhoeven (2021-12-08 10:30:53)
> Use the dev_err_probe() helper, instead of open-coding the same
> operation.
> 
> Signed-off-by: Geert Uytterhoeven 
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c 
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index 5612a9e7a9056cf7..86eeda769e2ebd10 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -661,9 +661,8 @@ static int rcar_du_probe(struct platform_device *pdev)
> /* DRM/KMS objects */
> ret = rcar_du_modeset_init(rcdu);
> if (ret < 0) {
> -   if (ret != -EPROBE_DEFER)
> -   dev_err(>dev,
> -   "failed to initialize DRM/KMS (%d)\n", ret);
> +   dev_err_probe(>dev, ret,
> + "failed to initialize DRM/KMS\n");

I've just learned that dev_err_probe() sets a 'reason' for the deferral.
Seems like a nice feature when exploring devices that are still waiting
to probe. Is the message still appropriate enough in that case?

I think it's probably fine, so

Reviewed-by: Kieran Bingham 


> goto error;
> }
>  
> -- 
> 2.25.1
>

Re: [Intel-gfx] [PATCH v2 09/16] drm/i915: Ensure i915_vma tests do not get -ENOSPC with the locking changes.

On Mon, 29 Nov 2021 at 13:58, Maarten Lankhorst
 wrote:
>
> Now that we require locking to evict, multiple vmas from the same object
> might not be evicted. This is expected and required, because execbuf will
> move to short-term pinning by using the lock only. This will cause these
> tests to fail, because they create a ton of vma's for the same object.
>
> Unbind manually to prevent spurious -ENOSPC in those mock tests.
>
> Signed-off-by: Maarten Lankhorst 

Hmm, do we need this? It looks like we should be able to handle such
scenarios, with already locked objects sharing the same dma-resv? Or
is something else going on here?

> ---
>  drivers/gpu/drm/i915/selftests/i915_vma.c | 17 -
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/selftests/i915_vma.c 
> b/drivers/gpu/drm/i915/selftests/i915_vma.c
> index 1f10fe36619b..5c5809dfe9b2 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_vma.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_vma.c
> @@ -691,7 +691,11 @@ static int igt_vma_rotate_remap(void *arg)
> }
>
> i915_vma_unpin(vma);
> -
> +   err = i915_vma_unbind(vma);
> +   if (err) {
> +   pr_err("Unbinding returned 
> %i\n", err);
> +   goto out_object;
> +   }
> cond_resched();
> }
> }
> @@ -848,6 +852,11 @@ static int igt_vma_partial(void *arg)
>
> i915_vma_unpin(vma);
> nvma++;
> +   err = i915_vma_unbind(vma);
> +   if (err) {
> +   pr_err("Unbinding returned %i\n", 
> err);
> +   goto out_object;
> +   }
>
> cond_resched();
> }
> @@ -882,6 +891,12 @@ static int igt_vma_partial(void *arg)
>
> i915_vma_unpin(vma);
>
> +   err = i915_vma_unbind(vma);
> +   if (err) {
> +   pr_err("Unbinding returned %i\n", err);
> +   goto out_object;
> +   }
> +
> count = 0;
> list_for_each_entry(vma, >vma.list, obj_link)
> count++;
> --
> 2.34.0
>

Re: [PATCH v2 03/11] mm/gup: migrate PIN_LONGTERM dev coherent pages to system

2021-12-08 Thread Alistair Popple

On Tuesday, 7 December 2021 5:52:43 AM AEDT Alex Sierra wrote:
> Avoid long term pinning for Coherent device type pages. This could
> interfere with their own device memory manager.
> If caller tries to get user device coherent pages with PIN_LONGTERM flag
> set, those pages will be migrated back to system memory.
> 
> Signed-off-by: Alex Sierra 
> ---
>  mm/gup.c | 32 ++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/gup.c b/mm/gup.c
> index 886d6148d3d0..1572eacf07f4 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -1689,17 +1689,37 @@ struct page *get_dump_page(unsigned long addr)
>  #endif /* CONFIG_ELF_CORE */
>  
>  #ifdef CONFIG_MIGRATION
> +static int migrate_device_page(unsigned long address,
> + struct page *page)
> +{
> + struct vm_area_struct *vma = find_vma(current->mm, address);
> + struct vm_fault vmf = {
> + .vma = vma,
> + .address = address & PAGE_MASK,
> + .flags = FAULT_FLAG_USER,
> + .pgoff = linear_page_index(vma, address),
> + .gfp_mask = GFP_KERNEL,
> + .page = page,
> + };
> + if (page->pgmap && page->pgmap->ops->migrate_to_ram)
> + return page->pgmap->ops->migrate_to_ram();

How does this synchronise against pgmap being released? As I understand things
at this point we're not holding a reference on either the page or pgmap, so
the page and therefore the pgmap may have been freed.

I think a similar problem exists for device private fault handling as well and
it has been on my list of things to fix for a while. I think the solution is to
call try_get_page(), except it doesn't work with device pages due to the whole
refcount thing. That issue is blocking a fair bit of work now so I've started
looking into it.

> +
> + return -EBUSY;
> +}
> +
>  /*
>   * Check whether all pages are pinnable, if so return number of pages.  If 
> some
>   * pages are not pinnable, migrate them, and unpin all pages. Return zero if
>   * pages were migrated, or if some pages were not successfully isolated.
>   * Return negative error if migration fails.
>   */
> -static long check_and_migrate_movable_pages(unsigned long nr_pages,
> +static long check_and_migrate_movable_pages(unsigned long start,
> + unsigned long nr_pages,
>   struct page **pages,
>   unsigned int gup_flags)
>  {
>   unsigned long i;
> + unsigned long page_index;
>   unsigned long isolation_error_count = 0;
>   bool drain_allow = true;
>   LIST_HEAD(movable_page_list);
> @@ -1720,6 +1740,10 @@ static long check_and_migrate_movable_pages(unsigned 
> long nr_pages,
>* If we get a movable page, since we are going to be pinning
>* these entries, try to move them out if possible.
>*/
> + if (is_device_page(head)) {
> + page_index = i;
> + goto unpin_pages;
> + }
>   if (!is_pinnable_page(head)) {
>   if (PageHuge(head)) {
>   if (!isolate_huge_page(head, 
> _page_list))
> @@ -1750,12 +1774,16 @@ static long check_and_migrate_movable_pages(unsigned 
> long nr_pages,
>   if (list_empty(_page_list) && !isolation_error_count)
>   return nr_pages;
>  
> +unpin_pages:
>   if (gup_flags & FOLL_PIN) {
>   unpin_user_pages(pages, nr_pages);
>   } else {
>   for (i = 0; i < nr_pages; i++)
>   put_page(pages[i]);
>   }
> + if (is_device_page(head))
> + return migrate_device_page(start + page_index * PAGE_SIZE, 
> head);

This isn't very optimal - if a range contains more than one device page (which
seems likely) we will have to go around the whole gup/check_and_migrate loop
once for each device page which seems unnecessary. You should be able to either
build a list or migrate them as you go through the loop. I'm also currently
looking into how to extend migrate_pages() to support device pages which might
be useful here too.

> +
>   if (!list_empty(_page_list)) {
>   ret = migrate_pages(_page_list, alloc_migration_target,
>   NULL, (unsigned long), MIGRATE_SYNC,
> @@ -1798,7 +1826,7 @@ static long __gup_longterm_locked(struct mm_struct *mm,
>NULL, gup_flags);
>   if (rc <= 0)
>   break;
> - rc = check_and_migrate_movable_pages(rc, pages, gup_flags);
> + rc = check_and_migrate_movable_pages(start, rc, pages, 
> gup_flags);
>   } while (!rc);
>   memalloc_pin_restore(flags);
>  
>

Re: [PATCH] drm/dp: Actually read Adjust Request Post Cursor2 register

2021-12-08 Thread Jani Nikula

On Fri, 03 Dec 2021, Kees Cook  wrote:
> The link_status array was not large enough to read the Adjust Request
> Post Cursor2 register. Adjust the size to include it. Found with a
> -Warray-bounds build:
>
> drivers/gpu/drm/drm_dp_helper.c: In function 
> 'drm_dp_get_adjust_request_post_cursor':
> drivers/gpu/drm/drm_dp_helper.c:59:27: error: array subscript 10 is outside 
> array bounds of 'const u8[6]' {aka 'const unsigned char[6]'} 
> [-Werror=array-bounds]
>59 | return link_status[r - DP_LANE0_1_STATUS];
>   |~~~^~~
> drivers/gpu/drm/drm_dp_helper.c:147:51: note: while referencing 'link_status'
>   147 | u8 drm_dp_get_adjust_request_post_cursor(const u8 
> link_status[DP_LINK_STATUS_SIZE],
>   |  
> ~^~~~
>
> Fixes: 79465e0ffeb9 ("drm/dp: Add helper to get post-cursor adjustments")
> Signed-off-by: Kees Cook 

Using DP_ADJUST_REQUEST_POST_CURSOR2 has been deprecated since DP 1.3
published in 2014, and Tegra is the only user of
drm_dp_get_adjust_request_post_cursor().

Instead of bumping the link status read size from 6 to 11 for all
drivers I'd much rather see some other (maybe Tegra specific) solution
to this.


BR,
Jani.


> ---
>  include/drm/drm_dp_helper.h | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 472dac376284..277643d2fe2c 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1517,7 +1517,15 @@ enum drm_dp_phy {
>  #define DP_MST_LOGICAL_PORT_0 8
>  
>  #define DP_LINK_CONSTANT_N_VALUE 0x8000
> -#define DP_LINK_STATUS_SIZE 6
> +/*
> + * DPCD registers in link_status:
> + * Link Status:  0x202 through 0x204
> + * Sink Status:  0x205
> + * Adjust Request:   0x206 through 0x207
> + * Training Score:   0x208 through 0x20b
> + * AR Post Cursor2:  0x20c
> + */
> +#define DP_LINK_STATUS_SIZE 11
>  bool drm_dp_channel_eq_ok(const u8 link_status[DP_LINK_STATUS_SIZE],
> int lane_count);
>  bool drm_dp_clock_recovery_ok(const u8 link_status[DP_LINK_STATUS_SIZE],

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [PATCH 1/3] drm/i915: Sanitycheck device iomem on probe

On 2021-12-08 at 11:12:07 +, Matthew Auld wrote:
> On 08/12/2021 10:20, Ramalingam C wrote:
> > From: Chris Wilson 
> > 
> > As we setup the memory regions for the device, give each a quick test to
> > verify that we can read and write to the full iomem range. This ensures
> > that our physical addressing for the device's memory is correct, and
> > some reassurance that the memory is functional.
> > 
> > Signed-off-by: Chris Wilson 
> > Cc: Matthew Auld 
> > Cc: CQ Tang 
> > Signed-off-by: Ramalingam C 
> 
> For the series, assuming CI is happy now,
> Reviewed-by: Matthew Auld 
Thank you!
> 
> Also patch 3 should be moved to the start of the series.
Sure I will do it.

Ram.
> 
> > ---
> >   drivers/gpu/drm/i915/intel_memory_region.c | 104 +
> >   1 file changed, 104 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_memory_region.c 
> > b/drivers/gpu/drm/i915/intel_memory_region.c
> > index b43121609e25..c53e07f1d0c0 100644
> > --- a/drivers/gpu/drm/i915/intel_memory_region.c
> > +++ b/drivers/gpu/drm/i915/intel_memory_region.c
> > @@ -3,6 +3,8 @@
> >* Copyright © 2019 Intel Corporation
> >*/
> > +#include 
> > +
> >   #include "intel_memory_region.h"
> >   #include "i915_drv.h"
> >   #include "i915_ttm_buddy_manager.h"
> > @@ -29,6 +31,99 @@ static const struct {
> > },
> >   };
> > +static int __iopagetest(struct intel_memory_region *mem,
> > +   u8 __iomem *va, int pagesize,
> > +   u8 value, resource_size_t offset,
> > +   const void *caller)
> > +{
> > +   int byte = prandom_u32_max(pagesize);
> > +   u8 result[3];
> > +
> > +   memset_io(va, value, pagesize); /* or GPF! */
> > +   wmb();
> > +
> > +   result[0] = ioread8(va);
> > +   result[1] = ioread8(va + byte);
> > +   result[2] = ioread8(va + pagesize - 1);
> > +   if (memchr_inv(result, value, sizeof(result))) {
> > +   dev_err(mem->i915->drm.dev,
> > +   "Failed to read back from memory region:%pR at [%pa + 
> > %pa] for %ps; wrote %x, read (%x, %x, %x)\n",
> > +   >region, >io_start, , caller,
> > +   value, result[0], result[1], result[2]);
> > +   return -EINVAL;
> > +   }
> > +
> > +   return 0;
> > +}
> > +
> > +static int iopagetest(struct intel_memory_region *mem,
> > + resource_size_t offset,
> > + const void *caller)
> > +{
> > +   const u8 val[] = { 0x0, 0xa5, 0xc3, 0xf0 };
> > +   void __iomem *va;
> > +   int err;
> > +   int i;
> > +
> > +   va = ioremap_wc(mem->io_start + offset, PAGE_SIZE);
> > +   if (!va) {
> > +   dev_err(mem->i915->drm.dev,
> > +   "Failed to ioremap memory region [%pa + %px] for %ps\n",
> > +   >io_start, , caller);
> > +   return -EFAULT;
> > +   }
> > +
> > +   for (i = 0; i < ARRAY_SIZE(val); i++) {
> > +   err = __iopagetest(mem, va, PAGE_SIZE, val[i], offset, caller);
> > +   if (err)
> > +   break;
> > +
> > +   err = __iopagetest(mem, va, PAGE_SIZE, ~val[i], offset, caller);
> > +   if (err)
> > +   break;
> > +   }
> > +
> > +   iounmap(va);
> > +   return err;
> > +}
> > +
> > +static resource_size_t random_page(resource_size_t last)
> > +{
> > +   /* Limited to low 44b (16TiB), but should suffice for a spot check */
> > +   return prandom_u32_max(last >> PAGE_SHIFT) << PAGE_SHIFT;
> > +}
> > +
> > +static int iomemtest(struct intel_memory_region *mem, const void *caller)
> > +{
> > +   resource_size_t last = resource_size(>region) - PAGE_SIZE;
> > +   int err;
> > +
> > +   /*
> > +* Quick test to check read/write access to the iomap (backing store).
> > +*
> > +* Write a byte, read it back. If the iomapping fails, we expect
> > +* a GPF preventing further execution. If the backing store does not
> > +* exist, the read back will return garbage. We check a couple of pages,
> > +* the first and last of the specified region to confirm the backing
> > +* store + iomap does cover the entire memory region; and we check
> > +* a random offset within as a quick spot check for bad memory.
> > +*/
> > +
> > +   err = iopagetest(mem, 0, caller);
> > +   if (err)
> > +   return err;
> > +
> > +   err = iopagetest(mem, last, caller);
> > +   if (err)
> > +   return err;
> > +
> > +   err = iopagetest(mem, random_page(last), caller);
> > +   if (err)
> > +   return err;
> > +
> > +   return 0;
> > +}
> > +
> >   struct intel_memory_region *
> >   intel_memory_region_lookup(struct drm_i915_private *i915,
> >u16 class, u16 instance)
> > @@ -126,8 +221,17 @@ intel_memory_region_create(struct drm_i915_private 
> > *i915,
> > goto err_free;
> > }
> > +   if (io_start && IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)) {
> > +   err = iomemtest(mem, (void *)_RET_IP_);
> > +   if (err)
> > +

Re: [PATCH 1/3] drm/i915: Sanitycheck device iomem on probe


On 08/12/2021 10:20, Ramalingam C wrote:

From: Chris Wilson 

As we setup the memory regions for the device, give each a quick test to
verify that we can read and write to the full iomem range. This ensures
that our physical addressing for the device's memory is correct, and
some reassurance that the memory is functional.

Signed-off-by: Chris Wilson 
Cc: Matthew Auld 
Cc: CQ Tang 
Signed-off-by: Ramalingam C 


For the series, assuming CI is happy now,
Reviewed-by: Matthew Auld 

Also patch 3 should be moved to the start of the series.


---
  drivers/gpu/drm/i915/intel_memory_region.c | 104 +
  1 file changed, 104 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_memory_region.c 
b/drivers/gpu/drm/i915/intel_memory_region.c
index b43121609e25..c53e07f1d0c0 100644
--- a/drivers/gpu/drm/i915/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/intel_memory_region.c
@@ -3,6 +3,8 @@
   * Copyright © 2019 Intel Corporation
   */
  
+#include 

+
  #include "intel_memory_region.h"
  #include "i915_drv.h"
  #include "i915_ttm_buddy_manager.h"
@@ -29,6 +31,99 @@ static const struct {
},
  };
  
+static int __iopagetest(struct intel_memory_region *mem,

+   u8 __iomem *va, int pagesize,
+   u8 value, resource_size_t offset,
+   const void *caller)
+{
+   int byte = prandom_u32_max(pagesize);
+   u8 result[3];
+
+   memset_io(va, value, pagesize); /* or GPF! */
+   wmb();
+
+   result[0] = ioread8(va);
+   result[1] = ioread8(va + byte);
+   result[2] = ioread8(va + pagesize - 1);
+   if (memchr_inv(result, value, sizeof(result))) {
+   dev_err(mem->i915->drm.dev,
+   "Failed to read back from memory region:%pR at [%pa + %pa] 
for %ps; wrote %x, read (%x, %x, %x)\n",
+   >region, >io_start, , caller,
+   value, result[0], result[1], result[2]);
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
+static int iopagetest(struct intel_memory_region *mem,
+ resource_size_t offset,
+ const void *caller)
+{
+   const u8 val[] = { 0x0, 0xa5, 0xc3, 0xf0 };
+   void __iomem *va;
+   int err;
+   int i;
+
+   va = ioremap_wc(mem->io_start + offset, PAGE_SIZE);
+   if (!va) {
+   dev_err(mem->i915->drm.dev,
+   "Failed to ioremap memory region [%pa + %px] for %ps\n",
+   >io_start, , caller);
+   return -EFAULT;
+   }
+
+   for (i = 0; i < ARRAY_SIZE(val); i++) {
+   err = __iopagetest(mem, va, PAGE_SIZE, val[i], offset, caller);
+   if (err)
+   break;
+
+   err = __iopagetest(mem, va, PAGE_SIZE, ~val[i], offset, caller);
+   if (err)
+   break;
+   }
+
+   iounmap(va);
+   return err;
+}
+
+static resource_size_t random_page(resource_size_t last)
+{
+   /* Limited to low 44b (16TiB), but should suffice for a spot check */
+   return prandom_u32_max(last >> PAGE_SHIFT) << PAGE_SHIFT;
+}
+
+static int iomemtest(struct intel_memory_region *mem, const void *caller)
+{
+   resource_size_t last = resource_size(>region) - PAGE_SIZE;
+   int err;
+
+   /*
+* Quick test to check read/write access to the iomap (backing store).
+*
+* Write a byte, read it back. If the iomapping fails, we expect
+* a GPF preventing further execution. If the backing store does not
+* exist, the read back will return garbage. We check a couple of pages,
+* the first and last of the specified region to confirm the backing
+* store + iomap does cover the entire memory region; and we check
+* a random offset within as a quick spot check for bad memory.
+*/
+
+   err = iopagetest(mem, 0, caller);
+   if (err)
+   return err;
+
+   err = iopagetest(mem, last, caller);
+   if (err)
+   return err;
+
+   err = iopagetest(mem, random_page(last), caller);
+   if (err)
+   return err;
+
+   return 0;
+}
+
  struct intel_memory_region *
  intel_memory_region_lookup(struct drm_i915_private *i915,
   u16 class, u16 instance)
@@ -126,8 +221,17 @@ intel_memory_region_create(struct drm_i915_private *i915,
goto err_free;
}
  
+	if (io_start && IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)) {

+   err = iomemtest(mem, (void *)_RET_IP_);
+   if (err)
+   goto err_release;
+   }
+
return mem;
  
+err_release:

+   if (mem->ops->release)
+   mem->ops->release(mem);
  err_free:
kfree(mem);
return ERR_PTR(err);

[PATCH] drm: rcar-du: Use dev_err_probe() helper

2021-12-08 Thread Geert Uytterhoeven

Use the dev_err_probe() helper, instead of open-coding the same
operation.

Signed-off-by: Geert Uytterhoeven 
---
 drivers/gpu/drm/rcar-du/rcar_du_drv.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c 
b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index 5612a9e7a9056cf7..86eeda769e2ebd10 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -661,9 +661,8 @@ static int rcar_du_probe(struct platform_device *pdev)
/* DRM/KMS objects */
ret = rcar_du_modeset_init(rcdu);
if (ret < 0) {
-   if (ret != -EPROBE_DEFER)
-   dev_err(>dev,
-   "failed to initialize DRM/KMS (%d)\n", ret);
+   dev_err_probe(>dev, ret,
+ "failed to initialize DRM/KMS\n");
goto error;
}
 
-- 
2.25.1

[PATCH v3 1/3] dt-bindings: gpu: mali-bifrost: Document RZ/G2L support

2021-12-08 Thread Biju Das

The Renesas RZ/G2{L, LC} SoC (a.k.a R9A07G044) has a Bifrost Mali-G31 GPU,
add a compatible string for it.

Signed-off-by: Biju Das 
Reviewed-by: Lad Prabhakar 
---
v2->v3:
 * Moved optional clock-names and reset-names to SoC-specific conditional 
schemas.
 * minimum number of reset for the generic GPU is set to 1.
 * Documented number of clocks, resets, interrupts and interrupt-names in RZ/G2L
   SoC-specific conditional schemas.
v1->v2:
 * Updated minItems for resets as 2
 * Documented optional property reset-names
 * Documented reset-names as required property for RZ/G2L SoC.
---
 .../bindings/gpu/arm,mali-bifrost.yaml| 45 ++-
 1 file changed, 43 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml 
b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
index 6f98dd55fb4c..63a08f3f321d 100644
--- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
+++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
@@ -19,6 +19,7 @@ properties:
   - amlogic,meson-g12a-mali
   - mediatek,mt8183-mali
   - realtek,rtd1619-mali
+  - renesas,r9a07g044-mali
   - rockchip,px30-mali
   - rockchip,rk3568-mali
   - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully 
discoverable
@@ -27,19 +28,26 @@ properties:
 maxItems: 1
 
   interrupts:
+minItems: 3
 items:
   - description: Job interrupt
   - description: MMU interrupt
   - description: GPU interrupt
+  - description: Event interrupt
 
   interrupt-names:
+minItems: 3
 items:
   - const: job
   - const: mmu
   - const: gpu
+  - const: event
 
   clocks:
-maxItems: 1
+minItems: 1
+maxItems: 3
+
+  clock-names: true
 
   mali-supply: true
 
@@ -52,7 +60,10 @@ properties:
 maxItems: 3
 
   resets:
-maxItems: 2
+minItems: 1
+maxItems: 3
+
+  reset-names: true
 
   "#cooling-cells":
 const: 2
@@ -94,6 +105,36 @@ allOf:
 then:
   required:
 - resets
+  - if:
+  properties:
+compatible:
+  contains:
+const: renesas,r9a07g044-mali
+then:
+  properties:
+interrupts:
+  minItems: 4
+interrupt-names:
+  minItems: 4
+clocks:
+  minItems: 3
+clock-names:
+  items:
+- const: gpu
+- const: bus
+- const: bus_ace
+resets:
+  minItems: 3
+reset-names:
+  items:
+- const: rst
+- const: axi_rst
+- const: ace_rst
+  required:
+- clock-names
+- power-domains
+- resets
+- reset-names
   - if:
   properties:
 compatible:
-- 
2.17.1

[PATCH v3 0/3] Add Mali-G31 GPU support for RZ/G2L SoC

2021-12-08 Thread Biju Das

RZ/G2L SoC embeds Mali-G31 bifrost GPU.
This patch series aims to add support for the same

It is tested with latest drm-misc-next + mesa 21.3.0 + 
out of tree patch for (du + DSI) + 
platform specific mesa configuration for RZ/G2L.

Tested the kmscube application.

test logs:-
root@smarc-rzg2l:~# kmscube
Using display 0xdb6e7d30 with EGL version 1.4
===
EGL information:
  version: "1.4"
  vendor: "Mesa Project"
.
===
OpenGL ES 2.x information:
  version: "OpenGL ES 3.1 Mesa 21.3.0"
  shading language version: "OpenGL ES GLSL ES 3.10"
  vendor: "Panfrost"
  renderer: "Mali-G31 (Panfrost)"
  
===
^C

root@smarc-rzg2l:~# cat /proc/interrupts | grep panfrost
 82: 587287  0 GICv3 186 Level panfrost-job
 83:  2  0 GICv3 187 Level panfrost-mmu
 84:  8  0 GICv3 185 Level panfrost-gpu

root@smarc-rzg2l:~# cat /sys/class/devfreq/1184.gpu/trans_stat
 From  :   To
   :  5000  6250 1 12500 2 25000 
4 5   time(ms)
*  5000: 0 0 0 0 0 0
 0 072
   6250: 0 0 0 0 0 0
 0 0 0
  1: 0 0 0 0 0 0
 0 0 0
  12500: 0 0 0 0 0 0
 0 168
  2: 0 0 0 0 0 0
 0 168
  25000: 1 0 0 0 0 0
 0 084
  4: 0 0 0 0 0 0
 0 0 0
  5: 0 0 0 1 1 1
 0 0   736
Total transition : 6
root@smarc-rzg2l:~# kmscube
Using display 0xf7a421b0 with EGL version 1.4
===
EGL information:
  version: "1.4"
  vendor: "Mesa Project"
  .
===
OpenGL ES 2.x information:
  version: "OpenGL ES 3.1 Mesa 21.3.0"
  shading language version: "OpenGL ES GLSL ES 3.10"
  vendor: "Panfrost"
  renderer: "Mali-G31 (Panfrost)"
  ..
===

root@smarc-rzg2l:~#
root@smarc-rzg2l:~#
root@smarc-rzg2l:~# cat /sys/class/devfreq/1184.gpu/trans_stat
 From  :   To
   :  5000  6250 1 12500 2 25000 
4 5   time(ms)
*  5000: 0 0 0 0 0 0
 0 1   144
   6250: 0 0 0 0 0 0
 0 0 0
  1: 0 0 0 0 0 0
 0 9   524
  12500: 0 0 9 0 0 0
 0 3  2544
  2: 0 0 011 0 0
 046  3304
  25000: 1 0 0 033 0
 0 0  7496
  4: 0 0 0 01619
 0 0  2024
  5: 1 0 0 1 815
35 0  4032
Total transition : 208

Platform specific mesa configuration patch for RZ/G2L
-
src/gallium/targets/dri/meson.build
+   'rcar-du_dri.so',
src/gallium/targets/dri/target.c
+DEFINE_LOADER_DRM_ENTRYPOINT(rcar_du)

V2->V3:
 * Moved optional clock-names and reset-names to SoC-specific conditional 
schemas.
 * minimum number of reset for the generic GPU is set to 1.
 * Documented number of clocks, resets, interrupts and interrupt-names in RZ/G2L
   SoC-specific conditional schemas.
 * Updated commit description for patch#3
V1->V2:
 * Removed clock patches from this seies, as it is accepted for 5.17
 * Added Rb tag from Geert
 * Added reset-names required property for RZ/G2L and updated the board dtsi.

Biju Das (3):
  dt-bindings: gpu: mali-bifrost: Document RZ/G2L support
  arm64: dts: renesas: r9a07g044: Add Mali-G31 GPU node
  arm64: dts: renesas: rzg2l-smarc-som: Add vdd core regulator

 .../bindings/gpu/arm,mali-bifrost.yaml| 45 -
 arch/arm64/boot/dts/renesas/r9a07g044.dtsi| 65 +++
 .../boot/dts/renesas/rzg2l-smarc-som.dtsi | 13 
 3 files changed, 121 insertions(+), 2 deletions(-)

-- 
2.17.1

Re: [PATCH] drm: Return error codes from struct drm_driver.gem_create_object

2021-12-08 Thread Thomas Zimmermann

Am 07.12.21 um 10:24 schrieb Thomas Zimmermann:

Am 07.12.21 um 09:55 schrieb Dan Carpenter:

I appologize. This thread has been really frustrating. I got mixed up
because I recently sent patches for ingenic and vc4. Also we are
working against different trees so maybe that is part of the problem?

I'm looking at today's linux-next. Your patch has been applied.

Yes. You updated all the drivers. But somehow the vc4 chunk from your
patch was dropped. It was *NOT* dropped by Stephen Rothwell. It got
dropped earlier. I am including the `git format-patch -1 ` output
from the commit.

My vc4 change is in drm-misc-next, [1] but not in drm-tip. [2] Your vc4
patch went through drm-misc-fixes.

drm-tip is build automatically by our DRM tools from the various trees.
The tools took my patch from drm-misc-next and your patch from
drm-misc-fixes and the result was broken.

Thanks for bringing this up. I'll see what I can do about it.

FYI I fixed drm-tip to return a pointer-encoded error in vc4. [1]

Best regards
Thomas

[1]
https://cgit.freedesktop.org/drm/drm-tip/tree/drivers/gpu/drm/vc4/vc4_bo.c?id=cc3b9eb186e3c8dfb0bcc7d54fa4bfbe52c0b58b#n394

Best regards
Thomas

[1]
https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/vc4/vc4_bo.c#n394

[2]
https://cgit.freedesktop.org/drm/drm-tip/tree/drivers/gpu/drm/vc4/vc4_bo.c

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

OpenPGP_signature
Description: OpenPGP digital signature

[PATCH 3/3] drm/i915: Exclude reserved stolen from driver use