Re: [PATCH 03/10] drm/i915/uapi: expose the avail tracking
On 25/05/2022 19:43, Matthew Auld wrote:
Vulkan would like to have a rough measure of how much device memory can
in theory be allocated. Also add unallocated_cpu_visible_size to track
the visible portion, in case the device is using small BAR.
I have concerns here that it isn't useful and could even be
counter-productive. If we give unprivileged access it may be considered
a side channel, but if we "lie" (report total region size) to
unprivileged clients (like in this patch), then they don't co-operate
well and end trashing.
Is Vulkan really sure it wants this and why?
Regards,
Tvrtko
Testcase: igt@i915_query@query-regions-unallocated
Testcase: igt@i915_query@query-regions-sanity-check
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Lionel Landwerlin
Cc: Tvrtko Ursulin
Cc: Jon Bloomfield
Cc: Daniel Vetter
Cc: Jordan Justen
Cc: Kenneth Graunke
Cc: Akeem G Abodunrin
---
drivers/gpu/drm/i915/i915_query.c | 10 +-
drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 20
drivers/gpu/drm/i915/i915_ttm_buddy_manager.h | 3 ++
drivers/gpu/drm/i915/intel_memory_region.c| 14 +
drivers/gpu/drm/i915/intel_memory_region.h| 3 ++
include/uapi/drm/i915_drm.h | 31 ++-
6 files changed, 79 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_query.c
b/drivers/gpu/drm/i915/i915_query.c
index 9aa0b28aa6ee..e095c55f4d4b 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -502,7 +502,15 @@ static int query_memregion_info(struct drm_i915_private
*i915,
else
info.probed_cpu_visible_size = mr->total;
- info.unallocated_size = mr->avail;
+ if (perfmon_capable()) {
+ intel_memory_region_avail(mr,
+ &info.unallocated_size,
+
&info.unallocated_cpu_visible_size);
+ } else {
+ info.unallocated_size = info.probed_size;
+ info.unallocated_cpu_visible_size =
+ info.probed_cpu_visible_size;
+ }
if (__copy_to_user(info_ptr, &info, sizeof(info)))
return -EFAULT;
diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
index a5109548abc0..aa5c91e44438 100644
--- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
+++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
@@ -365,6 +365,26 @@ u64 i915_ttm_buddy_man_visible_size(struct
ttm_resource_manager *man)
return bman->visible_size;
}
+/**
+ * i915_ttm_buddy_man_visible_size - Query the avail tracking for the manager.
+ *
+ * @man: The buddy allocator ttm manager
+ * @avail: The total available memory in pages for the entire manager.
+ * @visible_avail: The total available memory in pages for the CPU visible
+ * portion. Note that this will always give the same value as @avail on
+ * configurations that don't have a small BAR.
+ */
+void i915_ttm_buddy_man_avail(struct ttm_resource_manager *man,
+u64 *avail, u64 *visible_avail)
+{
+ struct i915_ttm_buddy_manager *bman = to_buddy_manager(man);
+
+ mutex_lock(&bman->lock);
+ *avail = bman->mm.avail >> PAGE_SHIFT;
+ *visible_avail = bman->visible_avail;
+ mutex_unlock(&bman->lock);
+}
+
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
void i915_ttm_buddy_man_force_visible_size(struct ttm_resource_manager *man,
u64 size)
diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h
b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h
index 52d9586d242c..d64620712830 100644
--- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h
+++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h
@@ -61,6 +61,9 @@ int i915_ttm_buddy_man_reserve(struct ttm_resource_manager
*man,
u64 i915_ttm_buddy_man_visible_size(struct ttm_resource_manager *man);
+void i915_ttm_buddy_man_avail(struct ttm_resource_manager *man,
+ u64 *avail, u64 *avail_visible);
+
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
void i915_ttm_buddy_man_force_visible_size(struct ttm_resource_manager *man,
u64 size);
diff --git a/drivers/gpu/drm/i915/intel_memory_region.c
b/drivers/gpu/drm/i915/intel_memory_region.c
index e38d2db1c3e3..94ee26e99549 100644
--- a/drivers/gpu/drm/i915/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/intel_memory_region.c
@@ -279,6 +279,20 @@ void intel_memory_region_set_name(struct
intel_memory_region *mem,
va_end(ap);
}
+void intel_memory_region_avail(struct intel_memory_region *mr,
+ u64 *avail, u64 *visible_avail)
+{
+ if (mr->type == INTEL_MEMORY_LOCAL) {
+ i915_ttm_buddy_man_avail(mr->region_private,
+
Re: [PATCH 1/2] dt-bindings: backlight: rt4831: Add the new property for ocp level selection
On 26/05/2022 05:16, cy_huang wrote: > From: ChiYuan Huang > > Add the new property for ocp level selection. > > Signed-off-by: ChiYuan Huang > --- > .../bindings/leds/backlight/richtek,rt4831-backlight.yaml | 8 > > include/dt-bindings/leds/rt4831-backlight.h | 5 + > 2 files changed, 13 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > > b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > index e0ac686..c1c59de 100644 > --- > a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > +++ > b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > @@ -47,6 +47,14 @@ properties: > minimum: 0 > maximum: 3 > > + richtek,bled-ocp-sel: Skip "sel" as it is a shortcut of selection. Name instead: "richtek,backlight-ocp" > +description: | > + Backlight OCP level selection, currently support 0.9A/1.2A/1.5A/1.8A Could you explain here what is OCP (unfold the acronym)? Best regards, Krzysztof
Re: [PATCH 03/10] drm/i915/uapi: expose the avail tracking
On 26/05/2022 08:58, Tvrtko Ursulin wrote:
On 25/05/2022 19:43, Matthew Auld wrote:
Vulkan would like to have a rough measure of how much device memory can
in theory be allocated. Also add unallocated_cpu_visible_size to track
the visible portion, in case the device is using small BAR.
I have concerns here that it isn't useful and could even be
counter-productive. If we give unprivileged access it may be considered
a side channel, but if we "lie" (report total region size) to
unprivileged clients (like in this patch), then they don't co-operate
well and end trashing.
Is Vulkan really sure it wants this and why?
Lionel pointed out:
https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/VK_EXT_memory_budget.html
Also note that the existing behaviour was to lie. I'm not sure what's
the best option here.
Regards,
Tvrtko
Testcase: igt@i915_query@query-regions-unallocated
Testcase: igt@i915_query@query-regions-sanity-check
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Lionel Landwerlin
Cc: Tvrtko Ursulin
Cc: Jon Bloomfield
Cc: Daniel Vetter
Cc: Jordan Justen
Cc: Kenneth Graunke
Cc: Akeem G Abodunrin
---
drivers/gpu/drm/i915/i915_query.c | 10 +-
drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 20
drivers/gpu/drm/i915/i915_ttm_buddy_manager.h | 3 ++
drivers/gpu/drm/i915/intel_memory_region.c | 14 +
drivers/gpu/drm/i915/intel_memory_region.h | 3 ++
include/uapi/drm/i915_drm.h | 31 ++-
6 files changed, 79 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_query.c
b/drivers/gpu/drm/i915/i915_query.c
index 9aa0b28aa6ee..e095c55f4d4b 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -502,7 +502,15 @@ static int query_memregion_info(struct
drm_i915_private *i915,
else
info.probed_cpu_visible_size = mr->total;
- info.unallocated_size = mr->avail;
+ if (perfmon_capable()) {
+ intel_memory_region_avail(mr,
+ &info.unallocated_size,
+ &info.unallocated_cpu_visible_size);
+ } else {
+ info.unallocated_size = info.probed_size;
+ info.unallocated_cpu_visible_size =
+ info.probed_cpu_visible_size;
+ }
if (__copy_to_user(info_ptr, &info, sizeof(info)))
return -EFAULT;
diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
index a5109548abc0..aa5c91e44438 100644
--- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
+++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
@@ -365,6 +365,26 @@ u64 i915_ttm_buddy_man_visible_size(struct
ttm_resource_manager *man)
return bman->visible_size;
}
+/**
+ * i915_ttm_buddy_man_visible_size - Query the avail tracking for the
manager.
+ *
+ * @man: The buddy allocator ttm manager
+ * @avail: The total available memory in pages for the entire manager.
+ * @visible_avail: The total available memory in pages for the CPU
visible
+ * portion. Note that this will always give the same value as @avail on
+ * configurations that don't have a small BAR.
+ */
+void i915_ttm_buddy_man_avail(struct ttm_resource_manager *man,
+ u64 *avail, u64 *visible_avail)
+{
+ struct i915_ttm_buddy_manager *bman = to_buddy_manager(man);
+
+ mutex_lock(&bman->lock);
+ *avail = bman->mm.avail >> PAGE_SHIFT;
+ *visible_avail = bman->visible_avail;
+ mutex_unlock(&bman->lock);
+}
+
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
void i915_ttm_buddy_man_force_visible_size(struct
ttm_resource_manager *man,
u64 size)
diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h
b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h
index 52d9586d242c..d64620712830 100644
--- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h
+++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h
@@ -61,6 +61,9 @@ int i915_ttm_buddy_man_reserve(struct
ttm_resource_manager *man,
u64 i915_ttm_buddy_man_visible_size(struct ttm_resource_manager *man);
+void i915_ttm_buddy_man_avail(struct ttm_resource_manager *man,
+ u64 *avail, u64 *avail_visible);
+
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
void i915_ttm_buddy_man_force_visible_size(struct
ttm_resource_manager *man,
u64 size);
diff --git a/drivers/gpu/drm/i915/intel_memory_region.c
b/drivers/gpu/drm/i915/intel_memory_region.c
index e38d2db1c3e3..94ee26e99549 100644
--- a/drivers/gpu/drm/i915/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/intel_memory_region.c
@@ -279,6 +279,20 @@ void intel_memory_region_set_name(struct
intel_memory_region *mem,
va_end(ap);
}
+void intel_memory_region_avail(struct intel_memory_region *mr,
+ u64 *avail, u64 *visible_avail)
+{
+ if (mr->type == INTEL_MEMORY_LOCAL) {
+ i915_ttm_buddy_man_avail(mr
Re: [PATCH 1/2] dt-bindings: backlight: rt4831: Add the new property for ocp level selection
Krzysztof Kozlowski 於 2022年5月26日 週四 下午4:06寫道： > > On 26/05/2022 05:16, cy_huang wrote: > > From: ChiYuan Huang > > > > Add the new property for ocp level selection. > > > > Signed-off-by: ChiYuan Huang > > --- > > .../bindings/leds/backlight/richtek,rt4831-backlight.yaml | 8 > > > > include/dt-bindings/leds/rt4831-backlight.h | 5 + > > 2 files changed, 13 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > > > > b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > > index e0ac686..c1c59de 100644 > > --- > > a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > > +++ > > b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > > @@ -47,6 +47,14 @@ properties: > > minimum: 0 > > maximum: 3 > > > > + richtek,bled-ocp-sel: > > Skip "sel" as it is a shortcut of selection. Name instead: > "richtek,backlight-ocp" > OK, if so, do I need to rename all properties from 'bled' to 'backlight' ? If only this property is naming as 'backlight'. it may conflict with the others like as "richtek,bled-ovp-sel". > > > +description: | > > + Backlight OCP level selection, currently support 0.9A/1.2A/1.5A/1.8A > > Could you explain here what is OCP (unfold the acronym)? Yes. And the full name is 'over current protection'. > > > Best regards, > Krzysztof
Re: [linux-next:master] BUILD REGRESSION 8cb8311e95e3bb58bd84d6350365f14a718faa6d
On Wed, May 25, 2022 at 02:50:56PM -0700, Andrew Morton wrote: > On Thu, 26 May 2022 05:35:20 +0800 kernel test robot wrote: > > > tree/branch: > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master > > branch HEAD: 8cb8311e95e3bb58bd84d6350365f14a718faa6d Add linux-next > > specific files for 20220525 > > > > Error/Warning reports: > > > > ... > > > > Unverified Error/Warning (likely false positive, please contact us if > > interested): > > Could be so. > > > mm/shmem.c:1948 shmem_getpage_gfp() warn: should '(((1) << 12) / 512) << > > folio_order(folio)' be a 64 bit type? > > I've been seeing this one for a while. And from this report I can't > figure out what tool emitted it. Clang? This is a Smatch warning. I normally look over Smatch warnings before forwarding kbuild-bot emails but this email is a grab bag of static checker warnings from different tools. This warning has a high rate of false positives so I'm going to disable it by default. > > > > > ... > > > > |-- i386-randconfig-m021 > > | `-- > > mm-shmem.c-shmem_getpage_gfp()-warn:should-((()-)-)-folio_order(folio)-be-a-bit-type > > If you're going to use randconfig then shouldn't you make the config > available? Or maybe quote the KCONFIG_SEED - presumably there's a way > for others to regenerate. > > Anyway, the warning seems wrong to me. > > > #define PAGE_SIZE (_AC(1,UL) << PAGE_SHIFT) > > #define BLOCKS_PER_PAGE (PAGE_SIZE/512) > > inode->i_blocks += BLOCKS_PER_PAGE << folio_order(folio); > > so the RHS here should have unsigned long type. Being able to generate > the cpp output would be helpful. That requires the .config. The heuristic is that "inode->i_blocks" is a u64 but this .config must be for a 32bit CPU. I'm just going to turn off all these warnings until I can figure out a better heuristic. regards, dan carpenter
Re: [linux-next:master] BUILD REGRESSION 8cb8311e95e3bb58bd84d6350365f14a718faa6d
On Wed, May 25, 2022 at 11:35 PM kernel test robot wrote: > .__mulsi3.o.cmd: No such file or directory > Makefile:686: arch/h8300/Makefile: No such file or directory > Makefile:765: arch/h8300/Makefile: No such file or directory > arch/Kconfig:10: can't open file "arch/h8300/Kconfig" Please stop building h8300 after the asm-generic tree is merged, the architecture is getting removed. Arnd
Re: [PATCH 03/10] drm/i915/uapi: expose the avail tracking
On 26/05/2022 09:10, Matthew Auld wrote:
On 26/05/2022 08:58, Tvrtko Ursulin wrote:
On 25/05/2022 19:43, Matthew Auld wrote:
Vulkan would like to have a rough measure of how much device memory can
in theory be allocated. Also add unallocated_cpu_visible_size to track
the visible portion, in case the device is using small BAR.
I have concerns here that it isn't useful and could even be
counter-productive. If we give unprivileged access it may be
considered a side channel, but if we "lie" (report total region size)
to unprivileged clients (like in this patch), then they don't
co-operate well and end trashing.
Is Vulkan really sure it wants this and why?
Lionel pointed out:
https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/VK_EXT_memory_budget.html
So this query would provide
VkPhysicalDeviceMemoryBudgetPropertiesEXT::heapBudget. Apart that it
wouldn't since we thought to lie. And apart that it's text says:
"""
...how much total memory from each heap the current process can use at
any given time, before allocations may start failing or causing
performance degradation. The values may change based on other activity
in the system that is outside the scope and control of the Vulkan
implementation.
"""
It acknowledges itself in the second sentence that the first sentence is
questionable.
And VkPhysicalDeviceMemoryBudgetPropertiesEXT::heapUsage would be still
missing and would maybe come via fdinfo? Or you plan to add it to this
same query later?
I like to source knowledge of best practices from the long established
world of CPU scheduling and process memory management. Is anyone aware
of this kind of techniques there - applications actively looking at free
memory data from /proc/meminfo and dynamically adjusting their runtime
behaviour based on it? And that they are not single application on a
dedicated system type of things.
Or perhaps this Vk extension is envisaged for exactly those kind of
scenarios? However if so then userspace can know all this data from
probed size and the data set it created.
Also note that the existing behaviour was to lie. I'm not sure what's
the best option here.
Uapi reserved -1 for unknown so we could do that?
Regards,
Tvrtko
Regards,
Tvrtko
Testcase: igt@i915_query@query-regions-unallocated
Testcase: igt@i915_query@query-regions-sanity-check
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Lionel Landwerlin
Cc: Tvrtko Ursulin
Cc: Jon Bloomfield
Cc: Daniel Vetter
Cc: Jordan Justen
Cc: Kenneth Graunke
Cc: Akeem G Abodunrin
---
drivers/gpu/drm/i915/i915_query.c | 10 +-
drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 20
drivers/gpu/drm/i915/i915_ttm_buddy_manager.h | 3 ++
drivers/gpu/drm/i915/intel_memory_region.c | 14 +
drivers/gpu/drm/i915/intel_memory_region.h | 3 ++
include/uapi/drm/i915_drm.h | 31 ++-
6 files changed, 79 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_query.c
b/drivers/gpu/drm/i915/i915_query.c
index 9aa0b28aa6ee..e095c55f4d4b 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -502,7 +502,15 @@ static int query_memregion_info(struct
drm_i915_private *i915,
else
info.probed_cpu_visible_size = mr->total;
- info.unallocated_size = mr->avail;
+ if (perfmon_capable()) {
+ intel_memory_region_avail(mr,
+ &info.unallocated_size,
+ &info.unallocated_cpu_visible_size);
+ } else {
+ info.unallocated_size = info.probed_size;
+ info.unallocated_cpu_visible_size =
+ info.probed_cpu_visible_size;
+ }
if (__copy_to_user(info_ptr, &info, sizeof(info)))
return -EFAULT;
diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
index a5109548abc0..aa5c91e44438 100644
--- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
+++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
@@ -365,6 +365,26 @@ u64 i915_ttm_buddy_man_visible_size(struct
ttm_resource_manager *man)
return bman->visible_size;
}
+/**
+ * i915_ttm_buddy_man_visible_size - Query the avail tracking for
the manager.
+ *
+ * @man: The buddy allocator ttm manager
+ * @avail: The total available memory in pages for the entire manager.
+ * @visible_avail: The total available memory in pages for the CPU
visible
+ * portion. Note that this will always give the same value as @avail on
+ * configurations that don't have a small BAR.
+ */
+void i915_ttm_buddy_man_avail(struct ttm_resource_manager *man,
+ u64 *avail, u64 *visible_avail)
+{
+ struct i915_ttm_buddy_manager *bman = to_buddy_manager(man);
+
+ mutex_lock(&bman->lock);
+ *avail = bman->mm.avail >> PAGE_SHIFT;
+ *visible_avail = bman->visible_avail;
+ mutex_unlock(&bman-
Re: [PATCH v2 8/9] drm/panfrost: Add Mali-G57 "Natt" support
On 25/05/2022 15:57, Alyssa Rosenzweig wrote:
> Add the features, issues, and GPU ID for Mali-G57, a first-generation
> Valhall GPU. Other first- and second-generation Valhall GPUs should be
> similar.
>
> v2: Split out issue list for r0p0 from newer Natt GPUs, as TTRX_3485 was
> fixed in r0p1. Unfortunately, MT8192 has a r0p0, so we do need to handle
> TTRX_3485.
>
> Signed-off-by: Alyssa Rosenzweig
> ---
> drivers/gpu/drm/panfrost/panfrost_features.h | 12
> drivers/gpu/drm/panfrost/panfrost_gpu.c | 3 +++
> drivers/gpu/drm/panfrost/panfrost_issues.h | 9 +
> 3 files changed, 24 insertions(+)
>
> diff --git a/drivers/gpu/drm/panfrost/panfrost_features.h
> b/drivers/gpu/drm/panfrost/panfrost_features.h
> index 1a8bdebc86a3..7ed0cd3ea2d4 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_features.h
> +++ b/drivers/gpu/drm/panfrost/panfrost_features.h
> @@ -106,6 +106,18 @@ enum panfrost_hw_feature {
> BIT_ULL(HW_FEATURE_TLS_HASHING) | \
> BIT_ULL(HW_FEATURE_3BIT_EXT_RW_L2_MMU_CONFIG))
>
> +#define hw_features_g57 (\
> + BIT_ULL(HW_FEATURE_JOBCHAIN_DISAMBIGUATION) | \
> + BIT_ULL(HW_FEATURE_PWRON_DURING_PWROFF_TRANS) | \
> + BIT_ULL(HW_FEATURE_XAFFINITY) | \
> + BIT_ULL(HW_FEATURE_FLUSH_REDUCTION) | \
> + BIT_ULL(HW_FEATURE_PROTECTED_MODE) | \
> + BIT_ULL(HW_FEATURE_PROTECTED_DEBUG_MODE) | \
> + BIT_ULL(HW_FEATURE_COHERENCY_REG) | \
> + BIT_ULL(HW_FEATURE_AARCH64_MMU) | \
> + BIT_ULL(HW_FEATURE_IDVS_GROUP_SIZE) | \
> + BIT_ULL(HW_FEATURE_CLEAN_ONLY_SAFE))
> +
> static inline bool panfrost_has_hw_feature(struct panfrost_device *pfdev,
> enum panfrost_hw_feature feat)
> {
> diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c
> b/drivers/gpu/drm/panfrost/panfrost_gpu.c
> index e1a6e763d0dc..6452e4e900dd 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
> @@ -201,6 +201,9 @@ static const struct panfrost_model gpu_models[] = {
> GPU_MODEL(g52, 0x7002),
> GPU_MODEL(g31, 0x7003,
> GPU_REV(g31, 1, 0)),
> +
> + GPU_MODEL(g57, 0x9001,
> + GPU_REV(g57, 0, 0)),
> };
>
> static void panfrost_gpu_init_features(struct panfrost_device *pfdev)
> diff --git a/drivers/gpu/drm/panfrost/panfrost_issues.h
> b/drivers/gpu/drm/panfrost/panfrost_issues.h
> index 4d41e0a13867..c5fa9e897a35 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_issues.h
> +++ b/drivers/gpu/drm/panfrost/panfrost_issues.h
> @@ -258,6 +258,15 @@ enum panfrost_hw_issue {
>
> #define hw_issues_g76 0
>
> +#define hw_issues_g57 (\
> + BIT_ULL(HW_ISSUE_TTRX_2968_TTRX_3162) | \
> + BIT_ULL(HW_ISSUE_TTRX_3076))
> +
> +#define hw_issues_g57_r0p0 (\
> + BIT_ULL(HW_ISSUE_TTRX_2968_TTRX_3162) | \
> + BIT_ULL(HW_ISSUE_TTRX_3076) | \
> + BIT_ULL(HW_ISSUE_TTRX_3485))
There's no need to repeat the issues that are generic for g57 in the
r0p0 list. So this can be simplified to:
#define hw_issues_g57_r0p0 (\
BIT_ULL(HW_ISSUE_TTRX_3485))
With that fixed:
Reviewed-by: Steven Price
> +
> static inline bool panfrost_has_hw_issue(const struct panfrost_device *pfdev,
>enum panfrost_hw_issue issue)
> {
Re: [PATCH v2 1/9] dt-bindings: Add compatible for Mali Valhall (JM)
On 25/05/2022 15:57, Alyssa Rosenzweig wrote: > From the kernel's perspective, (pre-CSF, "Job Manager") Valhall is more > or less compatible with Bifrost, although they differ to userspace. Add > a compatible for Valhall to the existing Bifrost bindings documentation. > > As the first SoC with a Valhall GPU receiving mainline support, add a > specific compatible for the MediaTek MT8192, which instantiates a > Mali-G57. > > v2: Change compatible to arm,mali-valhall-jm (Daniel Stone). > > Signed-off-by: Alyssa Rosenzweig > CC: devicet...@vger.kernel.org Reviewed-by: Steven Price > --- > .../bindings/gpu/arm,mali-bifrost.yaml| 25 +++ > 1 file changed, 15 insertions(+), 10 deletions(-) > > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > index 85f8d4764740..78964c140b46 100644 > --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > @@ -14,16 +14,21 @@ properties: > pattern: '^gpu@[a-f0-9]+$' > >compatible: > -items: > - - enum: > - - amlogic,meson-g12a-mali > - - mediatek,mt8183-mali > - - realtek,rtd1619-mali > - - renesas,r9a07g044-mali > - - renesas,r9a07g054-mali > - - rockchip,px30-mali > - - rockchip,rk3568-mali > - - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully > discoverable > +oneOf: > + - items: > + - enum: > + - amlogic,meson-g12a-mali > + - mediatek,mt8183-mali > + - realtek,rtd1619-mali > + - renesas,r9a07g044-mali > + - renesas,r9a07g054-mali > + - rockchip,px30-mali > + - rockchip,rk3568-mali > + - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is > fully discoverable > + - items: > + - enum: > + - mediatek,mt8192-mali > + - const: arm,mali-valhall-jm # Mali Valhall GPU model/revision is > fully discoverable > >reg: > maxItems: 1
Re: [linux-next:master] BUILD REGRESSION 8cb8311e95e3bb58bd84d6350365f14a718faa6d
On Thu, May 26, 2022 at 02:16:34AM +0100, Matthew Wilcox wrote: > Bizarre this started showing up now. The recent patch was: > > - info->alloced += compound_nr(page); > - inode->i_blocks += BLOCKS_PER_PAGE << compound_order(page); > + info->alloced += folio_nr_pages(folio); > + inode->i_blocks += BLOCKS_PER_PAGE << folio_order(folio); > > so it could tell that compound_order() was small, but folio_order() > might be large? The old code also generates a warning on my test system. Smatch thinks both compound_order() and folio_order() are 0-255. I guess because of the "unsigned char compound_order;" in the struct page. regards, dan carpenter
[PULL] drm-misc-fixes
Hi Daniel, Dave, Here's this week drm-misc-fixes PR. Maxime drm-misc-fixes-2022-05-26: A use-after-free fix for panfrost, and a DT invalid configuration fix for ti-sn65dsi83 The following changes since commit 6e03b13cc7d9427c2c77feed1549191015615202: drm/dp/mst: fix a possible memory leak in fetch_monitor_name() (2022-05-17 15:56:18 -0400) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2022-05-26 for you to fetch changes up to 6e516faf04317db2c46cbec4e3b78b4653a5b109: drm/panfrost: Job should reference MMU not file_priv (2022-05-25 09:14:22 +0100) A use-after-free fix for panfrost, and a DT invalid configuration fix for ti-sn65dsi83 Marek Vasut (1): drm/bridge: ti-sn65dsi83: Handle dsi_lanes == 0 as invalid Steven Price (1): drm/panfrost: Job should reference MMU not file_priv drivers/gpu/drm/bridge/ti-sn65dsi83.c | 2 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 5 +++-- drivers/gpu/drm/panfrost/panfrost_job.c | 6 +++--- drivers/gpu/drm/panfrost/panfrost_job.h | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) signature.asc Description: PGP signature
Re: [PATCH 2/2] backlight: rt4831: Add the property parsing for ocp level selection
On Thu, May 26, 2022 at 11:16:35AM +0800, cy_huang wrote:
> From: ChiYuan Huang
>
> Add the property parsing for ocp level selection.
Isn't this just restating the Subject: line?
It would be better to provide information useful to the reviewer here.
Something like:
"Currently this driver simply inherits whatever over-current protection
level is programmed into the hardware when it is handed over. Typically
this will be the reset value, A, although the bootloader could
have established a different value.
Allow the correct OCP value to be provided by the DT."
BTW please don't uncritically copy the above into the patch header. It is
just made something up as an example and I did no fact checking...
>
> Reported-by: Lucas Tsai
> Signed-off-by: ChiYuan Huang
> ---
> drivers/video/backlight/rt4831-backlight.c | 13 +
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/video/backlight/rt4831-backlight.c
> b/drivers/video/backlight/rt4831-backlight.c
> index 42155c7..c81f7d9 100644
> --- a/drivers/video/backlight/rt4831-backlight.c
> +++ b/drivers/video/backlight/rt4831-backlight.c
> @@ -12,6 +12,7 @@
> #define RT4831_REG_BLCFG 0x02
> #define RT4831_REG_BLDIML0x04
> #define RT4831_REG_ENABLE0x08
> +#define RT4831_REG_BLOPT20x11
>
> #define RT4831_BLMAX_BRIGHTNESS 2048
>
> @@ -23,6 +24,8 @@
> #define RT4831_BLDIML_MASK GENMASK(2, 0)
> #define RT4831_BLDIMH_MASK GENMASK(10, 3)
> #define RT4831_BLDIMH_SHIFT 3
> +#define RT4831_BLOCP_MASKGENMASK(1, 0)
> +#define RT4831_BLOCP_SHIFT 0
>
> struct rt4831_priv {
> struct device *dev;
> @@ -120,6 +123,16 @@ static int rt4831_parse_backlight_properties(struct
> rt4831_priv *priv,
> if (ret)
> return ret;
>
> + ret = device_property_read_u8(dev, "richtek,bled-ocp-sel", &propval);
> + if (ret)
> + propval = RT4831_BLOCPLVL_1P2A;
Is 1.2A the reset value for the register?
Additionally, it looks like adding a hard-coded default would cause
problems for existing platforms where the bootloader doesn't use
richtek,bled-ocp-sel and pre-configures a different value itself.
Would it be safer (in terms of working nicely with older bootloaders)
to only write to the RT4831_BLOCP_MASK if the new property is set?
Daniel.
> +
> + propval = min_t(u8, propval, RT4831_BLOCPLVL_1P8A);
> + ret = regmap_update_bits(priv->regmap, RT4831_REG_BLOPT2,
> RT4831_BLOCP_MASK,
> + propval << RT4831_BLOCP_SHIFT);
> + if (ret)
> + return ret;
> +
> ret = device_property_read_u8(dev, "richtek,channel-use", &propval);
> if (ret) {
> dev_err(dev, "richtek,channel-use DT property missing\n");
> --
> 2.7.4
>
Re: [PATCH] drm/i915/dg2: Catch and log more unexpected values in DG1_MSTR_TILE_INTR
On 25/05/2022 19:05, Matt Roper wrote:
On Wed, May 25, 2022 at 05:03:13PM +0100, Tvrtko Ursulin wrote:
On 24/05/2022 18:51, Matt Roper wrote:
On Tue, May 24, 2022 at 10:43:39AM +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
Catch and log any garbage in the register, including no tiles marked, or
multiple tiles marked.
Signed-off-by: Tvrtko Ursulin
Cc: Matt Roper
---
We caught garbage in DG1_MSTR_TILE_INTR with DG2 (actual value 0xF9D2C008)
during glmark and more badness. So I thought lets log all possible failure
modes from here and also use per device logging.
---
drivers/gpu/drm/i915/i915_irq.c | 33 ++---
drivers/gpu/drm/i915/i915_reg.h | 1 +
2 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 73cebc6aa650..79853d3fc1ed 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2778,24 +2778,30 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg)
u32 gu_misc_iir;
if (!intel_irqs_enabled(i915))
- return IRQ_NONE;
+ goto none;
master_tile_ctl = dg1_master_intr_disable(regs);
- if (!master_tile_ctl) {
- dg1_master_intr_enable(regs);
- return IRQ_NONE;
+ if (!master_tile_ctl)
+ goto enable_none;
+
+ if (master_tile_ctl & ~(DG1_MSTR_IRQ | DG1_MSTR_TILE_MASK)) {
+ drm_warn(&i915->drm, "Garbage in master_tile_ctl: 0x%08x!\n",
+master_tile_ctl);
I know we have a bunch of them already, but shouldn't we be avoiding
printk-based stuff like this inside interrupt handlers? Should we be
migrating all these error messages over to trace_printk or something
similar that's safer to use?
Not sure - I kind of think some really unexpected and worrying situations
should be loud and on by default. Risk is then spam if not ratelimited.
Maybe we should instead ratelimit most errors/warnings coming for irq
handlers?
It's not the risk of spam that's the problem, but rather that
printk-based stuff eventually calls into the console code to flush its
buffers. That's way more overhead than you want in an interrupt handler
so it's bad on its own, but if you're using something slow like a serial
console, it becomes even more of a problem.
Is it a problem for messages which we never expect to see?
While the unexpected bits in the master tile register are strange and
may point to a bigger problem somewhere else, they're also harmless on
their own since we should just ignore those bits and only process the
valid tiles.
Yes, I was expecting that a patch belonging to multi-tile enablement
would be incoming soon, which would be changing:
+ if (REG_FIELD_GET(DG1_MSTR_TILE_MASK, master_tile_ctl) !=
+ DG1_MSTR_TILE(0)) {
+ drm_warn(&i915->drm, "Unexpected irq from tile %u!\n",
+ilog2(REG_FIELD_GET(DG1_MSTR_TILE_MASK,
+master_tile_ctl)));
+ goto enable_none;
}
From this patch, into something completely different like walking bit
by bit, handling the present tiles, and warning on unexpected ones. What
should remain though is warning on no tiles signaled (which what we saw,
together with garbage in reserved bits).
In this particular case at least DRM_ERROR with no device info is the odd
one out in the entire file so I'd suggest changing at least that, if the
rest of my changes is of questionable benefit.
Changing DRM_ERROR -> drm_err would probably be fine in the short term
since it doesn't really make us any worse off. Changing to drm_warn
might not be great since we're generating a lot more lines of output and
Sorry I don't follow - why does replacing drm_err with drm_warn generate
(a lot) more lines of output?
But it can be drm_err for all I care, I don't think we really have
consistent story between errors and warnings in this area.
probably multiplying the already bad overhead that shouldn't be
happening in an interrupt handler. But if we could update the interrupt
handler to just save away the details and do the actual drm_warn later,
outside the interrupt handler code, that would be okay. We should
probably work toward something like that for all of our interrupt
handler warning/error messages.
Not sure I agree - for messages which we don't expect to see it doesn't
really matter that there will be overhead when they are hit. Presumably
bad things are already happening there so spending effort to optimise
those path is questionable.
Regards,
Tvrtko
Re: [PATCH] drm/i915/dg2: Catch and log more unexpected values in DG1_MSTR_TILE_INTR
On 25/05/2022 19:14, Lucas De Marchi wrote:
On Wed, May 25, 2022 at 05:03:13PM +0100, Tvrtko Ursulin wrote:
On 24/05/2022 18:51, Matt Roper wrote:
On Tue, May 24, 2022 at 10:43:39AM +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
Catch and log any garbage in the register, including no tiles
marked, or
multiple tiles marked.
Signed-off-by: Tvrtko Ursulin
Cc: Matt Roper
---
We caught garbage in DG1_MSTR_TILE_INTR with DG2 (actual value
0xF9D2C008)
during glmark and more badness. So I thought lets log all possible
failure
modes from here and also use per device logging.
---
drivers/gpu/drm/i915/i915_irq.c | 33 ++---
drivers/gpu/drm/i915/i915_reg.h | 1 +
2 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c
index 73cebc6aa650..79853d3fc1ed 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2778,24 +2778,30 @@ static irqreturn_t dg1_irq_handler(int irq,
void *arg)
u32 gu_misc_iir;
if (!intel_irqs_enabled(i915))
- return IRQ_NONE;
+ goto none;
master_tile_ctl = dg1_master_intr_disable(regs);
- if (!master_tile_ctl) {
- dg1_master_intr_enable(regs);
- return IRQ_NONE;
+ if (!master_tile_ctl)
+ goto enable_none;
+
+ if (master_tile_ctl & ~(DG1_MSTR_IRQ | DG1_MSTR_TILE_MASK)) {
+ drm_warn(&i915->drm, "Garbage in master_tile_ctl: 0x%08x!\n",
+ master_tile_ctl);
I know we have a bunch of them already, but shouldn't we be avoiding
printk-based stuff like this inside interrupt handlers? Should we be
migrating all these error messages over to trace_printk or something
similar that's safer to use?
Not sure - I kind of think some really unexpected and worrying
situations should be loud and on by default. Risk is then spam if not
ratelimited. Maybe we should instead ratelimit most errors/warnings
coming for irq handlers?
In this particular case at least DRM_ERROR with no device info is the
odd one out in the entire file so I'd suggest changing at least that,
if the rest of my changes is of questionable benefit.
I'd rather remove the printk's from irq rather than adding more. At the
very
least, they should be the _once variant or ratelimited. One of the few
cases to even deserve a unlikely(), even to document this shouldn't
really be happening.
I would support ratelimited for all the unexpected bits set, no bits
set, or similar conditions. I wouldn't remove such printks to
micro-optimize things. That would potentially lose important feedback in
cases when we hit truly unexpected situations.
But annotating them as unlikely would be a good thing.
Our irq handlers (particularly on dgfx and multi-gt) are already too
long running... I don't like making them any onger or slower.
How come? I mean which interrupts are a problem there? Isn't GuC
supposed to be taking on that load on itself, isn't that one of the main
selling points?
Regards,
Tvrtko
Re: [PATCH 1/2] dt-bindings: backlight: rt4831: Add the new property for ocp level selection
On 26/05/2022 10:13, ChiYuan Huang wrote: > Krzysztof Kozlowski 於 2022年5月26日 週四 下午4:06寫道： >> >> On 26/05/2022 05:16, cy_huang wrote: >>> From: ChiYuan Huang >>> >>> Add the new property for ocp level selection. >>> >>> Signed-off-by: ChiYuan Huang >>> --- >>> .../bindings/leds/backlight/richtek,rt4831-backlight.yaml | 8 >>> >>> include/dt-bindings/leds/rt4831-backlight.h | 5 + >>> 2 files changed, 13 insertions(+) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml >>> >>> b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml >>> index e0ac686..c1c59de 100644 >>> --- >>> a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml >>> +++ >>> b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml >>> @@ -47,6 +47,14 @@ properties: >>> minimum: 0 >>> maximum: 3 >>> >>> + richtek,bled-ocp-sel: >> >> Skip "sel" as it is a shortcut of selection. Name instead: >> "richtek,backlight-ocp" >> > OK, if so, do I need to rename all properties from 'bled' to 'backlight' ? > If only this property is naming as 'backlight'. it may conflict with > the others like as "richtek,bled-ovp-sel". Ah, no, no need. >> >>> +description: | >>> + Backlight OCP level selection, currently support 0.9A/1.2A/1.5A/1.8A >> >> Could you explain here what is OCP (unfold the acronym)? > Yes. And the full name is 'over current protection'. Thanks and this leads to second thing - you encode register value instead of logical value. This must be a logical value in mA, so "richtek,bled-ocp-microamp". I see you already did some register-style for voltage. It's wrong but it was done, so let it be. But let's don't make that a pattern... >> >> >> Best regards, >> Krzysztof Best regards, Krzysztof
Re: [PATCH v4 12/13] drm/msm: Utilize gpu scheduler priorities
On 26/05/2022 04:37, Rob Clark wrote:
On Wed, May 25, 2022 at 9:22 AM Tvrtko Ursulin
wrote:
On 25/05/2022 14:41, Rob Clark wrote:
On Wed, May 25, 2022 at 2:46 AM Tvrtko Ursulin
wrote:
On 24/05/2022 15:50, Rob Clark wrote:
On Tue, May 24, 2022 at 6:45 AM Tvrtko Ursulin
wrote:
On 23/05/2022 23:53, Rob Clark wrote:
On Mon, May 23, 2022 at 7:45 AM Tvrtko Ursulin
wrote:
Hi Rob,
On 28/07/2021 02:06, Rob Clark wrote:
From: Rob Clark
The drm/scheduler provides additional prioritization on top of that
provided by however many number of ringbuffers (each with their own
priority level) is supported on a given generation. Expose the
additional levels of priority to userspace and map the userspace
priority back to ring (first level of priority) and schedular priority
(additional priority levels within the ring).
Signed-off-by: Rob Clark
Acked-by: Christian König
---
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 4 +-
drivers/gpu/drm/msm/msm_gem_submit.c| 4 +-
drivers/gpu/drm/msm/msm_gpu.h | 58 -
drivers/gpu/drm/msm/msm_submitqueue.c | 35 +++
include/uapi/drm/msm_drm.h | 14 +-
5 files changed, 88 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index bad4809b68ef..748665232d29 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -261,8 +261,8 @@ int adreno_get_param(struct msm_gpu *gpu, uint32_t param,
uint64_t *value)
return ret;
}
return -EINVAL;
- case MSM_PARAM_NR_RINGS:
- *value = gpu->nr_rings;
+ case MSM_PARAM_PRIORITIES:
+ *value = gpu->nr_rings * NR_SCHED_PRIORITIES;
return 0;
case MSM_PARAM_PP_PGTABLE:
*value = 0;
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
b/drivers/gpu/drm/msm/msm_gem_submit.c
index 450efe59abb5..c2ecec5b11c4 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -59,7 +59,7 @@ static struct msm_gem_submit *submit_create(struct drm_device
*dev,
submit->gpu = gpu;
submit->cmd = (void *)&submit->bos[nr_bos];
submit->queue = queue;
- submit->ring = gpu->rb[queue->prio];
+ submit->ring = gpu->rb[queue->ring_nr];
submit->fault_dumped = false;
INIT_LIST_HEAD(&submit->node);
@@ -749,7 +749,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
/* Get a unique identifier for the submission for logging purposes */
submitid = atomic_inc_return(&ident) - 1;
- ring = gpu->rb[queue->prio];
+ ring = gpu->rb[queue->ring_nr];
trace_msm_gpu_submit(pid_nr(pid), ring->id, submitid,
args->nr_bos, args->nr_cmds);
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index b912cacaecc0..0e4b45bff2e6 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -250,6 +250,59 @@ struct msm_gpu_perfcntr {
const char *name;
};
+/*
+ * The number of priority levels provided by drm gpu scheduler. The
+ * DRM_SCHED_PRIORITY_KERNEL priority level is treated specially in some
+ * cases, so we don't use it (no need for kernel generated jobs).
+ */
+#define NR_SCHED_PRIORITIES (1 + DRM_SCHED_PRIORITY_HIGH -
DRM_SCHED_PRIORITY_MIN)
+
+/**
+ * msm_gpu_convert_priority - Map userspace priority to ring # and sched
priority
+ *
+ * @gpu:the gpu instance
+ * @prio: the userspace priority level
+ * @ring_nr:[out] the ringbuffer the userspace priority maps to
+ * @sched_prio: [out] the gpu scheduler priority level which the userspace
+ * priority maps to
+ *
+ * With drm/scheduler providing it's own level of prioritization, our total
+ * number of available priority levels is (nr_rings * NR_SCHED_PRIORITIES).
+ * Each ring is associated with it's own scheduler instance. However, our
+ * UABI is that lower numerical values are higher priority. So mapping the
+ * single userspace priority level into ring_nr and sched_prio takes some
+ * care. The userspace provided priority (when a submitqueue is created)
+ * is mapped to ring nr and scheduler priority as such:
+ *
+ * ring_nr= userspace_prio / NR_SCHED_PRIORITIES
+ * sched_prio = NR_SCHED_PRIORITIES -
+ *(userspace_prio % NR_SCHED_PRIORITIES) - 1
+ *
+ * This allows generations without preemption (nr_rings==1) to have some
+ * amount of prioritization, and provides more priority levels for gens
+ * that do have preemption.
I am exploring how different drivers handle priority levels and this
caught my eye.
Is the implication of the last paragraphs that on hw with nr_rings > 1,
ring + 1 preempts ring?
Other way around, at least from the uabi standpoint. Ie. ring[0]
preempts ring[1]
Ah yes, I
Re: [PATCH] radeon: Fix spelling typo in comment
On 5/26/22 08:27, pengfuyuan wrote: > Fix spelling typo in comment. > > Signed-off-by: pengfuyuan applied to the fbdev git tree. Thanks! Helge > --- > include/video/radeon.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/video/radeon.h b/include/video/radeon.h > index 005eae19ec09..72f94ccfa725 100644 > --- a/include/video/radeon.h > +++ b/include/video/radeon.h > @@ -750,7 +750,7 @@ > #define WAIT_DMA_GUI_IDLE (1 << 9) > #define WAIT_2D_IDLECLEAN (1 << 16) > > -/* SURFACE_CNTL bit consants */ > +/* SURFACE_CNTL bit constants */ > #define SURF_TRANSLATION_DIS(1 << 8) > #define NONSURF_AP0_SWP_16BPP (1 << 20) > #define NONSURF_AP0_SWP_32BPP (1 << 21)
[PATCH 00/31] OPP: Add new configuration interface: dev_pm_opp_set_config()
Hello, We have too many configuration specific APIs currently, six of them already, like dev_pm_opp_set_regulators(). This makes it complex/messy for both the OPP core and its users to manage. There is also code redundancy in these API, in the way they add/manage the OPP table specific stuff. This patch series is an attempt to simplify these interfaces by adding a single interface, dev_pm_opp_set_config(), which replaces all the existing ones. This also migrates the users to the new API. The first two patches help get the API in place, followed by patches to migrate the end users. Once all the users are migrated, the last few patches remove the now unused interfaces. I have lightly tested this on Hikey960 for now and also getting help from various build/boot bots, gitlab and lkp, to get these tested. It would be helpful if someone with access to the affected platforms can give it a try. This is pushed here: git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git opp/config The entire patchset shall get merged via the OPP tree in 5.20-rc1, please do not merge individual patches. Thanks. -- Viresh Viresh Kumar (31): OPP: Track if clock name is configured by platform OPP: Add dev_pm_opp_set_config() and friends cpufreq: dt: Migrate to dev_pm_opp_set_config() cpufreq: imx: Migrate to dev_pm_opp_set_config() cpufreq: qcom-nvmem: Migrate to dev_pm_opp_set_config() cpufreq: sti: Migrate to dev_pm_opp_set_config() cpufreq: sun50i: Migrate to dev_pm_opp_set_config() cpufreq: tegra20: Migrate to dev_pm_opp_set_config() cpufreq: ti: Migrate to dev_pm_opp_set_config() devfreq: exynos: Migrate to dev_pm_opp_set_config() devfreq: sun8i: Migrate to dev_pm_opp_set_config() devfreq: tegra30: Migrate to dev_pm_opp_set_config() drm/lima: Migrate to dev_pm_opp_set_config() drm/msm: Migrate to dev_pm_opp_set_config() drm/panfrost: Migrate to dev_pm_opp_set_config() drm/tegra: Migrate to dev_pm_opp_set_config() media: venus: Migrate to dev_pm_opp_set_config() media: tegra: Migrate to dev_pm_opp_set_config() mmc: sdhci-msm: Migrate to dev_pm_opp_set_config() OPP: ti: Migrate to dev_pm_opp_set_config() soc/tegra: Remove the call to devm_pm_opp_set_clkname() soc/tegra: Migrate to dev_pm_opp_set_config() spi: qcom: Migrate to dev_pm_opp_set_config() serial: qcom: Migrate to dev_pm_opp_set_config() OPP: Remove dev_pm_opp_set_regulators() and friends OPP: Remove dev_pm_opp_set_supported_hw() and friends OPP: Remove dev_pm_opp_set_clkname() and friends OPP: Remove dev_pm_opp_register_set_opp_helper() and friends OPP: Remove dev_pm_opp_attach_genpd() and friends OPP: Remove dev_pm_opp_set_prop_name() and friends OPP: Rearrange dev_pm_opp_set_config() and friends drivers/cpufreq/cpufreq-dt.c | 14 +- drivers/cpufreq/imx-cpufreq-dt.c | 12 +- drivers/cpufreq/qcom-cpufreq-nvmem.c | 107 +--- drivers/cpufreq/sti-cpufreq.c | 22 +- drivers/cpufreq/sun50i-cpufreq-nvmem.c| 11 +- drivers/cpufreq/tegra20-cpufreq.c | 12 +- drivers/cpufreq/ti-cpufreq.c | 38 +- drivers/devfreq/exynos-bus.c | 14 +- drivers/devfreq/sun8i-a33-mbus.c | 7 +- drivers/devfreq/tegra30-devfreq.c | 8 +- drivers/gpu/drm/lima/lima_devfreq.c | 11 +- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 8 +- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 10 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 5 +- drivers/gpu/drm/msm/dp/dp_ctrl.c | 5 +- drivers/gpu/drm/msm/dsi/dsi_host.c| 5 +- drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 +- drivers/gpu/drm/tegra/gr3d.c | 6 +- .../media/platform/qcom/venus/pm_helpers.c| 16 +- drivers/memory/tegra/tegra124-emc.c | 14 +- drivers/mmc/host/sdhci-msm.c | 5 +- drivers/opp/core.c| 540 +++--- drivers/opp/opp.h | 2 + drivers/opp/ti-opp-supply.c | 6 +- drivers/soc/tegra/common.c| 14 +- drivers/soc/tegra/pmc.c | 8 +- drivers/spi/spi-geni-qcom.c | 5 +- drivers/spi/spi-qcom-qspi.c | 5 +- drivers/tty/serial/qcom_geni_serial.c | 5 +- include/linux/pm_opp.h| 118 ++-- 30 files changed, 444 insertions(+), 598 deletions(-) -- 2.31.1.272.g89b43f80a514
[PATCH 13/31] drm/lima: Migrate to dev_pm_opp_set_config()
The OPP core now provides a unified API for setting all configuration
types, i.e. dev_pm_opp_set_config().
Lets start using it.
Signed-off-by: Viresh Kumar
---
drivers/gpu/drm/lima/lima_devfreq.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/lima/lima_devfreq.c
b/drivers/gpu/drm/lima/lima_devfreq.c
index 8989e215dfc9..e792ab5cd76a 100644
--- a/drivers/gpu/drm/lima/lima_devfreq.c
+++ b/drivers/gpu/drm/lima/lima_devfreq.c
@@ -111,6 +111,11 @@ int lima_devfreq_init(struct lima_device *ldev)
struct dev_pm_opp *opp;
unsigned long cur_freq;
int ret;
+ struct dev_pm_opp_config config = {
+ .regulator_names = (const char *[]){ "mali" },
+ .regulator_count = 1,
+ .clk_name = "core",
+ };
if (!device_property_present(dev, "operating-points-v2"))
/* Optional, continue without devfreq */
@@ -118,11 +123,7 @@ int lima_devfreq_init(struct lima_device *ldev)
spin_lock_init(&ldevfreq->lock);
- ret = devm_pm_opp_set_clkname(dev, "core");
- if (ret)
- return ret;
-
- ret = devm_pm_opp_set_regulators(dev, (const char *[]){ "mali" }, 1);
+ ret = devm_pm_opp_set_config(dev, &config);
if (ret) {
/* Continue if the optional regulator is missing */
if (ret != -ENODEV)
--
2.31.1.272.g89b43f80a514
[PATCH 14/31] drm/msm: Migrate to dev_pm_opp_set_config()
The OPP core now provides a unified API for setting all configuration
types, i.e. dev_pm_opp_set_config().
Lets start using it.
Signed-off-by: Viresh Kumar
---
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 8 ++--
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 10 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 5 -
drivers/gpu/drm/msm/dp/dp_ctrl.c| 5 -
drivers/gpu/drm/msm/dsi/dsi_host.c | 5 -
5 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index 407f50a15faa..c39fb085a762 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -1728,10 +1728,14 @@ static void check_speed_bin(struct device *dev)
{
struct nvmem_cell *cell;
u32 val;
+ struct dev_pm_opp_config config = {
+ .supported_hw = &val,
+ .supported_hw_count = 1,
+ };
/*
* If the OPP table specifies a opp-supported-hw property then we have
-* to set something with dev_pm_opp_set_supported_hw() or the table
+* to set something with dev_pm_opp_set_config() or the table
* doesn't get populated so pick an arbitrary value that should
* ensure the default frequencies are selected but not conflict with any
* actual bins
@@ -1753,7 +1757,7 @@ static void check_speed_bin(struct device *dev)
nvmem_cell_put(cell);
}
- devm_pm_opp_set_supported_hw(dev, &val, 1);
+ devm_pm_opp_set_config(dev, &config);
}
struct msm_gpu *a5xx_gpu_init(struct drm_device *dev)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 83c31b2ad865..ddb2812b1ff7 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1805,6 +1805,10 @@ static int a6xx_set_supported_hw(struct device *dev,
struct adreno_rev rev)
u32 supp_hw = UINT_MAX;
u32 speedbin;
int ret;
+ struct dev_pm_opp_config config = {
+ .supported_hw = &supp_hw,
+ .supported_hw_count = 1,
+ };
ret = adreno_read_speedbin(dev, &speedbin);
/*
@@ -1823,11 +1827,7 @@ static int a6xx_set_supported_hw(struct device *dev,
struct adreno_rev rev)
supp_hw = fuse_to_supp_hw(dev, rev, speedbin);
done:
- ret = devm_pm_opp_set_supported_hw(dev, &supp_hw, 1);
- if (ret)
- return ret;
-
- return 0;
+ return devm_pm_opp_set_config(dev, &config);
}
static const struct adreno_gpu_funcs funcs = {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index e29796c4f27b..43f943fdfde5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1203,12 +1203,15 @@ static int dpu_bind(struct device *dev, struct device
*master, void *data)
struct drm_device *ddev = priv->dev;
struct dpu_kms *dpu_kms;
int ret = 0;
+ struct dev_pm_opp_config config = {
+ .clk_name = "core",
+ };
dpu_kms = devm_kzalloc(&pdev->dev, sizeof(*dpu_kms), GFP_KERNEL);
if (!dpu_kms)
return -ENOMEM;
- ret = devm_pm_opp_set_clkname(dev, "core");
+ ret = devm_pm_opp_set_config(dev, &config);
if (ret)
return ret;
/* OPP table is optional */
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 53568567e05b..54bdb33eef45 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1974,6 +1974,9 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct
dp_link *link,
{
struct dp_ctrl_private *ctrl;
int ret;
+ struct dev_pm_opp_config config = {
+ .clk_name = "ctrl_link",
+ };
if (!dev || !panel || !aux ||
!link || !catalog) {
@@ -1987,7 +1990,7 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct
dp_link *link,
return ERR_PTR(-ENOMEM);
}
- ret = devm_pm_opp_set_clkname(dev, "ctrl_link");
+ ret = devm_pm_opp_set_config(dev, &config);
if (ret) {
dev_err(dev, "invalid DP OPP table in device tree\n");
/* caller do PTR_ERR(opp_table) */
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index d51e70fab93d..7d5b027629d2 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -1801,6 +1801,9 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi)
struct msm_dsi_host *msm_host = NULL;
struct platform_device *pdev = msm_dsi->pdev;
int ret;
+ struct dev_pm_opp_config config = {
+ .clk_name = "byte",
+ };
msm_host = devm_kzalloc(&pdev->dev, sizeof(*msm_host), GFP_KERNEL);
if (!msm_host) {
@@ -1862,7 +186
[PATCH 15/31] drm/panfrost: Migrate to dev_pm_opp_set_config()
The OPP core now provides a unified API for setting all configuration
types, i.e. dev_pm_opp_set_config().
Lets start using it.
Signed-off-by: Viresh Kumar
---
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index 194af7f607a6..7826d9366d35 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -91,6 +91,10 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
struct devfreq *devfreq;
struct thermal_cooling_device *cooling;
struct panfrost_devfreq *pfdevfreq = &pfdev->pfdevfreq;
+ struct dev_pm_opp_config config = {
+ .regulator_names = pfdev->comp->supply_names,
+ .regulator_count = pfdev->comp->num_supplies,
+ };
if (pfdev->comp->num_supplies > 1) {
/*
@@ -101,13 +105,12 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
return 0;
}
- ret = devm_pm_opp_set_regulators(dev, pfdev->comp->supply_names,
-pfdev->comp->num_supplies);
+ ret = devm_pm_opp_set_config(dev, &config);
if (ret) {
/* Continue if the optional regulator is missing */
if (ret != -ENODEV) {
if (ret != -EPROBE_DEFER)
- DRM_DEV_ERROR(dev, "Couldn't set OPP
regulators\n");
+ DRM_DEV_ERROR(dev, "Couldn't set OPP config\n");
return ret;
}
}
--
2.31.1.272.g89b43f80a514
[PATCH 16/31] drm/tegra: Migrate to dev_pm_opp_set_config()
The OPP core now provides a unified API for setting all configuration
types, i.e. dev_pm_opp_set_config().
Lets start using it.
Signed-off-by: Viresh Kumar
---
drivers/gpu/drm/tegra/gr3d.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tegra/gr3d.c b/drivers/gpu/drm/tegra/gr3d.c
index a1fd3113ea96..05c45c104e13 100644
--- a/drivers/gpu/drm/tegra/gr3d.c
+++ b/drivers/gpu/drm/tegra/gr3d.c
@@ -389,6 +389,10 @@ static int gr3d_init_power(struct device *dev, struct gr3d
*gr3d)
struct device_link *link;
unsigned int i;
int err;
+ struct dev_pm_opp_config config = {
+ .genpd_names = opp_genpd_names,
+ .virt_devs = &opp_virt_devs,
+ };
err = of_count_phandle_with_args(dev->of_node, "power-domains",
"#power-domain-cells");
@@ -421,7 +425,7 @@ static int gr3d_init_power(struct device *dev, struct gr3d
*gr3d)
if (dev->pm_domain)
return 0;
- err = devm_pm_opp_attach_genpd(dev, opp_genpd_names, &opp_virt_devs);
+ err = devm_pm_opp_set_config(dev, &config);
if (err)
return err;
--
2.31.1.272.g89b43f80a514
Re: [GIT PULL] drm/msm: drm-msm-fixes-2022-05-19
On 19/05/2022 19:21, Abhinav Kumar wrote: Hi Rob Here is the pull request for the fixes for 5.19. Just a few more changes on top of msm-fixes-staging. Mainly it has the foll fixes: - Limiting WB modes to max sspp linewidth - Fixing the supported rotations to add 180 back for IGT - Fix to handle pm_runtime_get_sync() errors to avoid unclocked access in the bind() path for dpu driver - Fix the irq_free() without request issue which was a big-time hitter in the CI-runs. Acked-by: Dmitry Baryshkov Thanks Abhinav The following changes since commit 947a844bb3ebff0f4736d244d792ce129f6700d7: drm: msm: fix possible memory leak in mdp5_crtc_cursor_set() (2022-05-18 11:05:21 -0700) are available in the git repository at: https://gitlab.freedesktop.org/abhinavk/msm.git/ tags/msm-next-5.19-fixes for you to fetch changes up to 64b22a0da12adb571c01edd671ee43634ebd7e41: drm/msm/dpu: handle pm_runtime_get_sync() errors in bind path (2022-05-18 18:32:03 -0700) 5.19 fixes for msm-next Signed-off-by: Abhinav Kumar Abhinav Kumar (3): drm/msm/dpu: limit writeback modes according to max_linewidth drm/msm/dpu: add DRM_MODE_ROTATE_180 back to supported rotations drm/msm/dpu: handle pm_runtime_get_sync() errors in bind path Dmitry Baryshkov (1): drm/msm: don't free the IRQ if it was not requested drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c | 4 +++- drivers/gpu/drm/msm/msm_drv.c | 7 ++- drivers/gpu/drm/msm/msm_kms.h | 1 + 5 files changed, 14 insertions(+), 4 deletions(-) -- With best wishes Dmitry
Re: [PATCH] drm: bridge: adv7511: Move CEC definitions to adv7511_cec.c
On Thu, 26 May 2022 at 01:50, Alvin Šipraga wrote:
>
> On Wed, May 25, 2022 at 06:53:16PM -0300, Fabio Estevam wrote:
> > ADV7511_REG_CEC_RX_FRAME_HDR[] and ADV7511_REG_CEC_RX_FRAME_LEN[]
> > are only used inside adv7511_cec.c.
> >
> > Move their definitions to this file to avoid the following build
> > warnings when CONFIG_DRM_I2C_ADV7511_CEC is not selected:
> >
> > drivers/gpu/drm/bridge/adv7511/adv7511.h:229:17: warning:
> > 'ADV7511_REG_CEC_RX_FRAME_HDR' defined but not used
> > [-Wunused-const-variable=]
> > drivers/gpu/drm/bridge/adv7511/adv7511.h:235:17: warning:
> > 'ADV7511_REG_CEC_RX_FRAME_LEN' defined but not used
> > [-Wunused-const-variable=]
> >
> > Reported-by: kernel test robot
> > Fixes: ab0af093bf90 ("drm: bridge: adv7511: use non-legacy mode for CEC RX")
> > Signed-off-by: Fabio Estevam
> > ---
>
> Thank you for fixing this.
>
> Reviewed-by: Alvin Šipraga
>
>
> > drivers/gpu/drm/bridge/adv7511/adv7511.h | 12
> > drivers/gpu/drm/bridge/adv7511/adv7511_cec.c | 12
> > 2 files changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> > b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> > index 9e3bb8a8ee40..a031a0cd1f18 100644
> > --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> > @@ -226,18 +226,6 @@
> > #define ADV7511_REG_CEC_CLK_DIV 0x4e
> > #define ADV7511_REG_CEC_SOFT_RESET 0x50
> >
> > -static const u8 ADV7511_REG_CEC_RX_FRAME_HDR[] = {
> > - ADV7511_REG_CEC_RX1_FRAME_HDR,
> > - ADV7511_REG_CEC_RX2_FRAME_HDR,
> > - ADV7511_REG_CEC_RX3_FRAME_HDR,
> > -};
> > -
> > -static const u8 ADV7511_REG_CEC_RX_FRAME_LEN[] = {
> > - ADV7511_REG_CEC_RX1_FRAME_LEN,
> > - ADV7511_REG_CEC_RX2_FRAME_LEN,
> > - ADV7511_REG_CEC_RX3_FRAME_LEN,
> > -};
> > -
> > #define ADV7533_REG_CEC_OFFSET 0x70
> >
> > enum adv7511_input_clock {
> > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
> > b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
> > index 399f625a50c8..0b266f28f150 100644
> > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
> > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
> > @@ -15,6 +15,18 @@
> >
> > #include "adv7511.h"
> >
> > +static const u8 ADV7511_REG_CEC_RX_FRAME_HDR[] = {
> > + ADV7511_REG_CEC_RX1_FRAME_HDR,
> > + ADV7511_REG_CEC_RX2_FRAME_HDR,
> > + ADV7511_REG_CEC_RX3_FRAME_HDR,
> > +};
> > +
> > +static const u8 ADV7511_REG_CEC_RX_FRAME_LEN[] = {
> > + ADV7511_REG_CEC_RX1_FRAME_LEN,
> > + ADV7511_REG_CEC_RX2_FRAME_LEN,
> > + ADV7511_REG_CEC_RX3_FRAME_LEN,
> > +};
> > +
> > #define ADV7511_INT1_CEC_MASK \
> > (ADV7511_INT1_CEC_TX_READY | ADV7511_INT1_CEC_TX_ARBIT_LOST | \
> >ADV7511_INT1_CEC_TX_RETRY_TIMEOUT | ADV7511_INT1_CEC_RX_READY1 | \
> > --
> > 2.25.1
> >
Applied to drm-misc-next
Re: [PATCH] dt-bindings: display: ingenic,jz4780-hdmi: Drop undocumented 'ddc-i2c-bus'
On Thu, 26 May 2022 at 00:34, Laurent Pinchart wrote: > > Hi Rob, > > Thank you for the patch. > > On Wed, May 25, 2022 at 03:56:26PM -0500, Rob Herring wrote: > > While 'ddc-i2c-bus' is a common property, it should be in a connector > > node rather than the HDMI bridge node as the I2C bus goes to a > > connector and not the HDMI block. Drop it from the example. > > > > Signed-off-by: Rob Herring > > Reviewed-by: Laurent Pinchart > > > --- > > .../devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git > > a/Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml > > b/Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml > > index b8219eab4475..89490fdffeb0 100644 > > --- > > a/Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml > > +++ > > b/Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml > > @@ -55,7 +55,6 @@ examples: > > compatible = "ingenic,jz4780-dw-hdmi"; > > reg = <0x1018 0x8000>; > > reg-io-width = <4>; > > -ddc-i2c-bus = <&i2c4>; > > interrupt-parent = <&intc>; > > interrupts = <3>; > > clocks = <&cgu JZ4780_CLK_AHB0>, <&cgu JZ4780_CLK_HDMI>; > > -- > Regards, > > Laurent Pinchart Applied to drm-misc-next
Re: [PATCH v2] drm: bridge: icn6211: Adjust clock phase using SYS_CTRL_1
On Mon, 23 May 2022 at 15:25, Marek Vasut wrote: > > On 5/23/22 15:20, Jonathan Liu wrote: > > Hi Marek, > > > > On Mon, 23 May 2022 at 23:15, Marek Vasut wrote: > >> > >> On 5/23/22 15:01, Jonathan Liu wrote: > >>> The code from [1] sets SYS_CTRL_1 to different values depending on the > >>> desired clock phase (0, 1/4, 1/2 or 3/4). A clock phase of 0 aligns the > >>> positive edge of the clock with the pixel data while other values delay > >>> the clock by a fraction of the clock period. A clock phase of 1/2 aligns > >>> the negative edge of the clock with the pixel data. > >>> > >>> The driver currently hard codes SYS_CTRL_1 to 0x88 which corresponds to > >>> aligning the positive edge of the clock with the pixel data. This won't > >>> work correctly for panels that require aligning the negative edge of the > >>> clock with the pixel data. > >>> > >>> Adjust the clock phase to 0 if DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE is > >>> present in bus_flags, otherwise adjust the clock phase to 1/2 as > >>> appropriate for DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE. > >>> > >>> [1] https://github.com/tdjastrzebski/ICN6211-Configurator > >>> > >>> Signed-off-by: Jonathan Liu > >>> --- > >>> V2: Use GENMASK and FIELD_PREP macros > >>> --- > >>>drivers/gpu/drm/bridge/chipone-icn6211.c | 18 -- > >>>1 file changed, 16 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/bridge/chipone-icn6211.c > >>> b/drivers/gpu/drm/bridge/chipone-icn6211.c > >>> index 47dea657a752..f1538fb5f8a9 100644 > >>> --- a/drivers/gpu/drm/bridge/chipone-icn6211.c > >>> +++ b/drivers/gpu/drm/bridge/chipone-icn6211.c > >>> @@ -9,6 +9,8 @@ > >>>#include > >>>#include > >>> > >>> +#include > >>> +#include > >>>#include > >>>#include > >>>#include > >>> @@ -26,6 +28,11 @@ > >>>#define PD_CTRL(n) (0x0a + ((n) & 0x3)) /* 0..3 */ > >>>#define RST_CTRL(n) (0x0e + ((n) & 0x1)) /* 0..1 */ > >>>#define SYS_CTRL(n) (0x10 + ((n) & 0x7)) /* 0..4 */ > >>> +#define SYS_CTRL_1_CLK_PHASE_MSK GENMASK(5, 4) > >> > >> This should be GENMASK(7, 6) , no ? > > > > Clock phase 0 = 0b_1000_1000 = 0x88 > > Clock phase 1/4 = 0b_1001_1000 = 0x98 > > Clock phase 1/2 = 0b_1010_1000 = 0xA8 > > Clock phase 3/4 = 0b_1011_1000 = 0xB8 > > > > The clock phase bits are 5:4 not 7:6. The upper 2 bits and lower 4 > > bits are unknown. > > Doh, you're right. > > Reviewed-by: Marek Vasut Applied to drm-misc-next.
Re: [PATCH 2/2] dma-buf: Add an API for importing sync files (v9)
On Friday, May 6th, 2022 at 20:02, Jason Ekstrand wrote:
> diff --git a/include/uapi/linux/dma-buf.h b/include/uapi/linux/dma-buf.h
> index 46f1e3e98b02..70e213a0d7d9 100644
> --- a/include/uapi/linux/dma-buf.h
> +++ b/include/uapi/linux/dma-buf.h
> @@ -96,6 +96,24 @@ struct dma_buf_sync {
> * dma-buf for waiting later instead of waiting immediately. This is
> * useful for modern graphics APIs such as Vulkan which assume an explicit
> * synchronization model but still need to inter-operate with dma-buf.
> + *
> + * The intended usage pattern is the following:
> + *
> + * 1. Export a sync_file with flags corresponding to the expected GPU usage
> + * via DMA_BUF_IOCTL_EXPORT_SYNC_FILE.
> + *
> + * 2. Submit rendering work which uses the dma-buf. The work should wait on
> + * the exported sync file before rendering and produce another sync_file
> + * when complete.
> + *
> + * 3. Import the rendering-complete sync_file into the dma-buf with flags
> + * corresponding to the GPU usage via DMA_BUF_IOCTL_EXPORT_SYNC_FILE.
This should read DMA_BUF_IOCTL_IMPORT_SYNC_FILE I think?
> + * Unlike doing implicit synchronization via a GPU kernel driver's exec
> ioctl,
> + * the above is not a single atomic operation. If userspace wants to ensure
> + * ordering via these fences, it is the respnosibility of userspace to use
> + * locks or other mechanisms to ensure that no other context adds fences or
> + * submits work between steps 1 and 3 above.
> */
> struct dma_buf_export_sync_file {
> /**
Re: [PATCH v4 12/13] drm/msm: Utilize gpu scheduler priorities
On 26/05/2022 04:15, Rob Clark wrote: On Wed, May 25, 2022 at 9:11 AM Tvrtko Ursulin wrote: On 24/05/2022 15:57, Rob Clark wrote: On Tue, May 24, 2022 at 6:45 AM Tvrtko Ursulin wrote: On 23/05/2022 23:53, Rob Clark wrote: btw, one fun (but unrelated) issue I'm hitting with scheduler... I'm trying to add an igt test to stress shrinker/eviction, similar to the existing tests/i915/gem_shrink.c. But we hit an unfortunate combination of circumstances: 1. Pinning memory happens in the synchronous part of the submit ioctl, before enqueuing the job for the kthread to handle. 2. The first run_job() callback incurs a slight delay (~1.5ms) while resuming the GPU 3. Because of that delay, userspace has a chance to queue up enough more jobs to require locking/pinning more than the available system RAM.. Is that one or multiple threads submitting jobs? In this case multiple.. but I think it could also happen with a single thread (provided it didn't stall on a fence, directly or indirectly, from an earlier submit), because of how resume and actual job submission happens from scheduler kthread. I'm not sure if we want a way to prevent userspace from getting *too* far ahead of the kthread. Or maybe at some point the shrinker should sleep on non-idle buffers? On the direct reclaim path when invoked from the submit ioctl? In i915 we only shrink idle objects on direct reclaim and leave active ones for the swapper. It depends on how your locking looks like whether you could do them, whether there would be coupling of locks and fs-reclaim context. I think the locking is more or less ok, although lockdep is unhappy about one thing[1] which is I think a false warning (ie. not recognizing that we'd already successfully acquired the obj lock via trylock). We can already reclaim idle bo's in this path. But the problem with a bunch of submits queued up in the scheduler, is that they are already considered pinned and active. So at some point we need to sleep (hopefully interruptabley) until they are no longer active, ie. to throttle userspace trying to shove in more submits until some of the enqueued ones have a chance to run and complete. Odd I did not think trylock could trigger that. Looking at your code it indeed seems two trylocks. I am pretty sure we use the same trylock trick to avoid it. I am confused.. The sequence is, 1. kref_get_unless_zero() 2. trylock, which succeeds 3. attempt to evict or purge (which may or may not have succeeded) 4. unlock ... meanwhile this has raced with submit (aka execbuf) finishing and retiring and dropping *other* remaining reference to bo... 5. drm_gem_object_put() which triggers drm_gem_object_free() 6. in our free path we acquire the obj lock again and then drop it. Which arguably is unnecessary and only serves to satisfy some GEM_WARN_ON(!msm_gem_is_locked(obj)) in code paths that are also used elsewhere lockdep doesn't realize the previously successful trylock+unlock sequence so it assumes that the code that triggered recursion into shrinker could be holding the objects lock. Ah yes, missed that lock after trylock in msm_gem_shrinker/scan(). Well i915 has the same sequence in our shrinker, but the difference is we use delayed work to actually free, _and_ use trylock in the delayed worker. It does feel a bit inelegant (objects with no reference count which cannot be trylocked?!), but as this is the code recently refactored by Maarten so I think best try and sync with him for the full story. Otherwise if you can afford to sleep you can of course throttle organically via direct reclaim. Unless I am forgetting some key gotcha - it's been a while I've been active in this area. So, one thing that is awkward about sleeping in this path is that there is no way to propagate back -EINTR, so we end up doing an uninterruptible sleep in something that could be called indirectly from userspace syscall.. i915 seems to deal with this by limiting it to shrinker being called from kswapd. I think in the shrinker we want to know whether it is ok to sleep (ie. not syscall trigggered codepath, and whether we are under enough memory pressure to justify sleeping). For the syscall path, I'm playing with something that lets me pass __GFP_RETRY_MAYFAIL | __GFP_NOWARN to shmem_read_mapping_page_gfp(), and then stall after the shrinker has failed, somewhere where we can make it interruptable. Ofc, that doesn't help with all the other random memory allocations which can fail, so not sure if it will turn out to be a good approach or not. But I guess pinning the GEM bo's is the single biggest potential consumer of pages in the submit path, so maybe it will be better than nothing. We play similar games, although by a quick look I am not sure we quite manage to honour/propagate signals. This has certainly been a historically fiddly area. If you first ask for no reclaim allocations and invoke the shrinker manually first, then falling back to a bigger hammer, you shou
Re: [PATCH 15/31] drm/panfrost: Migrate to dev_pm_opp_set_config()
On 26/05/2022 12:42, Viresh Kumar wrote:
> The OPP core now provides a unified API for setting all configuration
> types, i.e. dev_pm_opp_set_config().
>
> Lets start using it.
>
> Signed-off-by: Viresh Kumar
Acked-by: Steven Price
> ---
> drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> index 194af7f607a6..7826d9366d35 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> @@ -91,6 +91,10 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
> struct devfreq *devfreq;
> struct thermal_cooling_device *cooling;
> struct panfrost_devfreq *pfdevfreq = &pfdev->pfdevfreq;
> + struct dev_pm_opp_config config = {
> + .regulator_names = pfdev->comp->supply_names,
> + .regulator_count = pfdev->comp->num_supplies,
> + };
>
> if (pfdev->comp->num_supplies > 1) {
> /*
> @@ -101,13 +105,12 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
> return 0;
> }
>
> - ret = devm_pm_opp_set_regulators(dev, pfdev->comp->supply_names,
> - pfdev->comp->num_supplies);
> + ret = devm_pm_opp_set_config(dev, &config);
> if (ret) {
> /* Continue if the optional regulator is missing */
> if (ret != -ENODEV) {
> if (ret != -EPROBE_DEFER)
> - DRM_DEV_ERROR(dev, "Couldn't set OPP
> regulators\n");
> + DRM_DEV_ERROR(dev, "Couldn't set OPP config\n");
> return ret;
> }
> }
Re: [linux-next:master] BUILD REGRESSION 8cb8311e95e3bb58bd84d6350365f14a718faa6d
On Thu, May 26, 2022 at 11:48:32AM +0300, Dan Carpenter wrote: > On Thu, May 26, 2022 at 02:16:34AM +0100, Matthew Wilcox wrote: > > Bizarre this started showing up now. The recent patch was: > > > > - info->alloced += compound_nr(page); > > - inode->i_blocks += BLOCKS_PER_PAGE << compound_order(page); > > + info->alloced += folio_nr_pages(folio); > > + inode->i_blocks += BLOCKS_PER_PAGE << folio_order(folio); > > > > so it could tell that compound_order() was small, but folio_order() > > might be large? > > The old code also generates a warning on my test system. Smatch thinks > both compound_order() and folio_order() are 0-255. I guess because of > the "unsigned char compound_order;" in the struct page. It'd be nice if we could annotate that as "contains a value between 1 and BITS_PER_LONG - PAGE_SHIFT". Then be able to optionally enable a checker that ensures that's true on loads/stores. Maybe we need a language that isn't C :-P Ada can do this ... I don't think Rust can.
Re: [PATCH] drm/amd: Fix spelling typo in comments
Applied. Thanks!
Alex
On Thu, May 26, 2022 at 5:29 AM <1064094...@qq.com> wrote:
>
> From: pengfuyuan
>
> Fix spelling typo in comments.
>
> Reported-by: k2ci
> Signed-off-by: pengfuyuan
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +-
> drivers/gpu/drm/amd/amdgpu/mes_api_def.h | 2 +-
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
> drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c | 4 ++--
> drivers/gpu/drm/amd/display/modules/vmid/vmid.c | 2 +-
> drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/arcturus_ppsmc.h | 2 +-
> 6 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 7606e3b6361e..35c303c865be 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -271,7 +271,7 @@ extern int amdgpu_vcnfw_log;
> #define CIK_CURSOR_WIDTH 128
> #define CIK_CURSOR_HEIGHT 128
>
> -/* smasrt shift bias level limits */
> +/* smart shift bias level limits */
> #define AMDGPU_SMARTSHIFT_MAX_BIAS (100)
> #define AMDGPU_SMARTSHIFT_MIN_BIAS (-100)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/mes_api_def.h
> b/drivers/gpu/drm/amd/amdgpu/mes_api_def.h
> index 3f4fca5fd1da..c31abf554878 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mes_api_def.h
> +++ b/drivers/gpu/drm/amd/amdgpu/mes_api_def.h
> @@ -33,7 +33,7 @@
> */
> enum { API_FRAME_SIZE_IN_DWORDS = 64 };
>
> -/* To avoid command in scheduler context to be overwritten whenenver mutilple
> +/* To avoid command in scheduler context to be overwritten whenever multiple
> * interrupts come in, this creates another queue.
> */
> enum { API_NUMBER_OF_COMMAND_MAX = 32 };
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 62139ff35476..8855a75dc75e 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -6705,7 +6705,7 @@ static void dm_disable_vblank(struct drm_crtc *crtc)
> dm_set_vblank(crtc, false);
> }
>
> -/* Implemented only the options currently availible for the driver */
> +/* Implemented only the options currently available for the driver */
> static const struct drm_crtc_funcs amdgpu_dm_crtc_funcs = {
> .reset = dm_crtc_reset_state,
> .destroy = amdgpu_dm_crtc_destroy,
> diff --git a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c
> b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c
> index 4385d19bc489..733f99a6e8e6 100644
> --- a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c
> +++ b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c
> @@ -619,7 +619,7 @@ static int get_max_dsc_slices(union dsc_enc_slice_caps
> slice_caps)
> }
>
>
> -// Increment sice number in available sice numbers stops if possible, or
> just increment if not
> +// Increment slice number in available slice numbers stops if possible, or
> just increment if not
> static int inc_num_slices(union dsc_enc_slice_caps slice_caps, int
> num_slices)
> {
> // Get next bigger num slices available in common caps
> @@ -650,7 +650,7 @@ static int inc_num_slices(union dsc_enc_slice_caps
> slice_caps, int num_slices)
> }
>
>
> -// Decrement sice number in available sice numbers stops if possible, or
> just decrement if not. Stop at zero.
> +// Decrement slice number in available slice numbers stops if possible, or
> just decrement if not. Stop at zero.
> static int dec_num_slices(union dsc_enc_slice_caps slice_caps, int
> num_slices)
> {
> // Get next bigger num slices available in common caps
> diff --git a/drivers/gpu/drm/amd/display/modules/vmid/vmid.c
> b/drivers/gpu/drm/amd/display/modules/vmid/vmid.c
> index 61ee4be35d27..2c40212d86da 100644
> --- a/drivers/gpu/drm/amd/display/modules/vmid/vmid.c
> +++ b/drivers/gpu/drm/amd/display/modules/vmid/vmid.c
> @@ -66,7 +66,7 @@ static void evict_vmids(struct core_vmid *core_vmid)
> }
> }
>
> -// Return value of -1 indicates vmid table unitialized or ptb dne in the
> table
> +// Return value of -1 indicates vmid table uninitialized or ptb dne in the
> table
> static int get_existing_vmid_for_ptb(struct core_vmid *core_vmid, uint64_t
> ptb)
> {
> int i;
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/arcturus_ppsmc.h
> b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/arcturus_ppsmc.h
> index 45f5d29bc705..15b313baa0ee 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/arcturus_ppsmc.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/arcturus_ppsmc.h
> @@ -120,7 +120,7 @@
> #define PPSMC_MSG_ReadSerialNumTop320x40
> #define PPSMC_MSG_ReadSerialNumBottom32 0x41
>
> -/* paramater for MSG_LightSBR
> +/* parameter for MSG_LightSBR
> * 1 -- Enable light secondary bus reset, only do nbio respond without
> further handling,
> * leave driver to handle the real reset
> * 0 -- Disable LightSBR, d
Re: [PATCH 0/2] dma-buf: Add an API for exporting sync files (v14)
Acked-by: Bas Nieuwenhuizen Didn't test the latest version of everything, but I can confirm the UAPI worked fine for what we'd want to use it for with radv. On Thu, May 26, 2022 at 8:47 AM Jason Ekstrand wrote: > > On Wed, May 25, 2022 at 5:02 AM Daniel Stone wrote: >> >> On Sat, 7 May 2022 at 14:18, Jason Ekstrand wrote: >> > This patch series actually contains two new ioctls. There is the export >> > one >> > mentioned above as well as an RFC for an import ioctl which provides the >> > other >> > half. The intention is to land the export ioctl since it seems like >> > there's >> > no real disagreement on that one. The import ioctl, however, has a lot of >> > debate around it so it's intended to be RFC-only for now. >> >> Errr, I think we're good with this one now right? > > > Yeah, I dropped the RFC from the patch, just not the description in the cover > letter, apparently. > >> >> From the uAPI point of view, having looked through the Mesa MR, both are: >> Acked-by: Daniel Stone > > > For reference: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4037 > > Yes, I agree it's gotten sufficient review at this point that I think we can > call the uAPI reviewed. I'm good with landing now. Sorry that took so long > but the original version I had only used half of the new API and I wanted to > make sure both halves got good testing. > > --Jason > > >> >> Cheers, >> Daniel
Re: [PATCH v2 -next] drm/display: Fix build error without CONFIG_OF
Hi Linus, Yue,
On 23/05/2022 15:54, Linus Walleij wrote:
On Mon, May 23, 2022 at 2:46 PM Linus Walleij wrote:
On Fri, May 6, 2022 at 2:33 PM YueHaibing wrote:
While CONFIG_OF is n but COMPILE_TEST is y, we got this:
WARNING: unmet direct dependencies detected for DRM_DP_AUX_BUS
Depends on [n]: HAS_IOMEM [=y] && DRM [=y] && OF [=n]
Selected by [y]:
- DRM_MSM [=y] && HAS_IOMEM [=y] && DRM [=y] && (ARCH_QCOM || SOC_IMX5 || COMPILE_TEST [=y]) && COMMON_CLK
[=y] && IOMMU_SUPPORT [=y] && (QCOM_OCMEM [=n] || QCOM_OCMEM [=n]=n) && (QCOM_LLCC [=y] || QCOM_LLCC [=y]=n) &&
(QCOM_COMMAND_DB [=n] || QCOM_COMMAND_DB [=n]=n)
Make DRM_DP_AUX_BUS depends on OF || COMPILE_TEST to fix this warning.
Fixes: f5d01644921b ("drm/msm: select DRM_DP_AUX_BUS for the AUX bus support")
Signed-off-by: YueHaibing
Patch applied to the DRM tree.
Nope, failed:
$ dim push-branch drm-misc-next
dim: ac890b9eeb9b ("drm/display: Fix build error without CONFIG_OF"):
Fixes: SHA1 in not pointing at an ancestor:
dim: f5d01644921b ("drm/msm: select DRM_DP_AUX_BUS for the AUX bus support")
dim: ERROR: issues in commits detected, aborting
After a second thought, I think the patch contains wrong Fixes tag. It
should be:
Fixes: 1e0f66420b13 ("drm/display: Introduce a DRM display-helper module")
With that in place would we be able to merge it through drm-misc? Does
it needs to be resubmitted?
I don't know what to do with this, sorry. The other committers are maybe better
with this kind of situations. I think it is designed to stop me from
shooting myself
in the foot.
Yours,
Linus Walleij
--
With best wishes
Dmitry
Re: [linux-next:master] BUILD REGRESSION 8cb8311e95e3bb58bd84d6350365f14a718faa6d
On Thu, May 26, 2022 at 03:28:25PM +0100, Matthew Wilcox wrote: > On Thu, May 26, 2022 at 11:48:32AM +0300, Dan Carpenter wrote: > > On Thu, May 26, 2022 at 02:16:34AM +0100, Matthew Wilcox wrote: > > > Bizarre this started showing up now. The recent patch was: > > > > > > - info->alloced += compound_nr(page); > > > - inode->i_blocks += BLOCKS_PER_PAGE << compound_order(page); > > > + info->alloced += folio_nr_pages(folio); > > > + inode->i_blocks += BLOCKS_PER_PAGE << folio_order(folio); > > > > > > so it could tell that compound_order() was small, but folio_order() > > > might be large? > > > > The old code also generates a warning on my test system. Smatch thinks > > both compound_order() and folio_order() are 0-255. I guess because of > > the "unsigned char compound_order;" in the struct page. > > It'd be nice if we could annotate that as "contains a value between > 1 and BITS_PER_LONG - PAGE_SHIFT". Then be able to optionally enable > a checker that ensures that's true on loads/stores. Maybe we need a > language that isn't C :-P Ada can do this ... I don't think Rust can. Machine Parsable Comments. It's a matter of figuring out the best format and writing the code. In Smatch, I have table of hard coded return values in the format: https://github.com/error27/smatch/blob/master/smatch_data/db/kernel.return_fixes I don't have code to handle something like BITS_PER_LONG or PAGE_SHIFT. To be honest, Smatch code always assumes that PAGE_SIZE is 4096 but I should actually look it up... It's not impossible to do. The GFP_KERNEL values changed enough so that I eventually made that look up the actual defines. I also have a table in the database where I could edit the values of (struct page)->compound_order. regards, dan carpenter
Re: [PATCH v3] drm/probe-helper: Make 640x480 first if no EDID
On Thu, 26 May 2022 at 03:28, Sean Paul wrote: > > On Wed, May 25, 2022 at 9:26 AM Daniel Vetter wrote: > > > > On Mon, May 23, 2022 at 05:59:02PM -0700, Doug Anderson wrote: > > > Hi, > > > > > > On Fri, May 20, 2022 at 5:01 PM Doug Anderson > > > wrote: > > > > > > > > Hi, > > > > > > > > On Mon, May 16, 2022 at 3:28 AM Thomas Zimmermann > > > > wrote: > > > > > > > > > > Hi Douglas, > > > > > > > > > > I understand that you're trying to tell userspace that the modelist > > > > > has > > > > > been made up, but it's not something that should be done via fragile > > > > > heuristics IMHO. > > > > > > > > > > I looked at the Chromium source code that you linked, but I cannot say > > > > > whether it's doing the correct thing. It all depends on what your > > > > > program needs. > > > > > > > > > > In that function, you could also search for 'DRM_MODE_TYPE_USERDEF'. > > > > > It's the mode that the user specified on the kernel command line. If > > > > > Chromium's automatic mode selection fails, you'd give your users > > > > > direct > > > > > control over it. > > > > > > > > That doesn't really work for Chrome OS. Certainly a kernel hacker > > > > could do this, but it's not something I could imagine us exposing to > > > > an average user of a Chromebook. > > > > > > > > > > > > > When there's no flagged mode or if > > > > > /sys/class/drm/card<...>/status contains "unconnected", you can assume > > > > > that the modelist is artificial and try the modes in an appropriate > > > > > order. > > > > > > > > So "no flagged" means that nothing is marked as preferred, correct? > > > > > > > > ...so I guess what you're suggesting is that the order that the kernel > > > > is presenting the modes to userspace is not ABI. If there are no > > > > preferred modes then userspace shouldn't necessarily assume that the > > > > first mode returned is the best mode. Instead it should assume that if > > > > there is no preferred mode then the mode list is made up and it should > > > > make its own decisions about the best mode to start with. If this is > > > > the ABI from the kernel then plausibly I could convince people to > > > > change userspace to pick 640x480 first in this case. > > > > > > > > > If we really want the kernel to give additional guarantees, we should > > > > > have a broader discussion about this topic IMHO. > > > > > > > > Sure. I've added Stéphane Marchesin to this thread in case he wants to > > > > chime in about anything. > > > > > > > > Overall, my take on the matter: > > > > > > > > * Mostly I got involved because, apparently, a DP compliance test was > > > > failing. The compliance test was upset that when it presented us with > > > > no EDID that we didn't default to 640x480. There was a push to make a > > > > fix for this in the Qualcomm specific driver but that didn't sit right > > > > with me. > > > > > > > > * On all devices I'm currently working with (laptops), the DP is a > > > > secondary display. If a user was trying to plug in a display with a > > > > bad EDID and the max mode (1024x768) didn't work, they could just use > > > > the primary display to choose a different resolution. It seems > > > > unlikely a user would truly be upset and would probably be happy they > > > > could get their broken display to work at all. Even if this is a > > > > primary display, I believe there are documented key combos to change > > > > the resolution of the primary display even if you can't see anything. > > > > > > > > * That all being said, defaulting to 640x480 when there's no EDID made > > > > sense to me, especially since it's actually defined in the DP spec. So > > > > I'm trying to do the right thing and solve this corner case. That > > > > being said, if it's truly controversial I can just drop it. > > > > > > > > > > > > So I guess my plan will be to give Stéphane a little while in case he > > > > wants to chime in. If not then I guess I'll try a Chrome patch... > > > > ...and if that doesn't work, I'll just drop it. > > > > > > OK, this userspace code seems to work: > > > > > > https://crrev.com/c/3662501 - ozone/drm: Try 640x480 before picking > > > the first mode if no EDID > > > > > > ...so we'll see how review of that goes. :-) > > Mirroring some of my comments on that review here :-) > > IMO, this should be addressed in the kernel, or not at all. The kernel > ensures other aspects of DisplayPort implementation are compliant, so > I don't think this would be any exception. Further, the kernel is the > one creating the "safe" mode list, so it seems odd that userspace > would override that. Finally, relying on every userspace to do the > right thing is asking for trouble (we have 3 places which would need > this logic in CrOS). Oh I missed the part that this is defined in the DP spec as _the_ fallback mode. I think the probe helpers could check whether it's a DP connector and then dtrt per DP spec? I think that should have a solid chance of avoiding the regression mess, since the
Re: [PATCH 0/2] dma-buf: Add an API for exporting sync files (v14)
The uAPI looks good to me as well, if that helps. Acked-by: Simon Ser
Re: [PATCH v3] drm/probe-helper: Make 640x480 first if no EDID
Hi, On Thu, May 26, 2022 at 8:42 AM Daniel Vetter wrote: > > On Thu, 26 May 2022 at 03:28, Sean Paul wrote: > > > > On Wed, May 25, 2022 at 9:26 AM Daniel Vetter wrote: > > > > > > On Mon, May 23, 2022 at 05:59:02PM -0700, Doug Anderson wrote: > > > > Hi, > > > > > > > > On Fri, May 20, 2022 at 5:01 PM Doug Anderson > > > > wrote: > > > > > > > > > > Hi, > > > > > > > > > > On Mon, May 16, 2022 at 3:28 AM Thomas Zimmermann > > > > > wrote: > > > > > > > > > > > > Hi Douglas, > > > > > > > > > > > > I understand that you're trying to tell userspace that the modelist > > > > > > has > > > > > > been made up, but it's not something that should be done via fragile > > > > > > heuristics IMHO. > > > > > > > > > > > > I looked at the Chromium source code that you linked, but I cannot > > > > > > say > > > > > > whether it's doing the correct thing. It all depends on what your > > > > > > program needs. > > > > > > > > > > > > In that function, you could also search for 'DRM_MODE_TYPE_USERDEF'. > > > > > > It's the mode that the user specified on the kernel command line. If > > > > > > Chromium's automatic mode selection fails, you'd give your users > > > > > > direct > > > > > > control over it. > > > > > > > > > > That doesn't really work for Chrome OS. Certainly a kernel hacker > > > > > could do this, but it's not something I could imagine us exposing to > > > > > an average user of a Chromebook. > > > > > > > > > > > > > > > > When there's no flagged mode or if > > > > > > /sys/class/drm/card<...>/status contains "unconnected", you can > > > > > > assume > > > > > > that the modelist is artificial and try the modes in an appropriate > > > > > > order. > > > > > > > > > > So "no flagged" means that nothing is marked as preferred, correct? > > > > > > > > > > ...so I guess what you're suggesting is that the order that the kernel > > > > > is presenting the modes to userspace is not ABI. If there are no > > > > > preferred modes then userspace shouldn't necessarily assume that the > > > > > first mode returned is the best mode. Instead it should assume that if > > > > > there is no preferred mode then the mode list is made up and it should > > > > > make its own decisions about the best mode to start with. If this is > > > > > the ABI from the kernel then plausibly I could convince people to > > > > > change userspace to pick 640x480 first in this case. > > > > > > > > > > > If we really want the kernel to give additional guarantees, we > > > > > > should > > > > > > have a broader discussion about this topic IMHO. > > > > > > > > > > Sure. I've added Stéphane Marchesin to this thread in case he wants to > > > > > chime in about anything. > > > > > > > > > > Overall, my take on the matter: > > > > > > > > > > * Mostly I got involved because, apparently, a DP compliance test was > > > > > failing. The compliance test was upset that when it presented us with > > > > > no EDID that we didn't default to 640x480. There was a push to make a > > > > > fix for this in the Qualcomm specific driver but that didn't sit right > > > > > with me. > > > > > > > > > > * On all devices I'm currently working with (laptops), the DP is a > > > > > secondary display. If a user was trying to plug in a display with a > > > > > bad EDID and the max mode (1024x768) didn't work, they could just use > > > > > the primary display to choose a different resolution. It seems > > > > > unlikely a user would truly be upset and would probably be happy they > > > > > could get their broken display to work at all. Even if this is a > > > > > primary display, I believe there are documented key combos to change > > > > > the resolution of the primary display even if you can't see anything. > > > > > > > > > > * That all being said, defaulting to 640x480 when there's no EDID made > > > > > sense to me, especially since it's actually defined in the DP spec. So > > > > > I'm trying to do the right thing and solve this corner case. That > > > > > being said, if it's truly controversial I can just drop it. > > > > > > > > > > > > > > > So I guess my plan will be to give Stéphane a little while in case he > > > > > wants to chime in. If not then I guess I'll try a Chrome patch... > > > > > ...and if that doesn't work, I'll just drop it. > > > > > > > > OK, this userspace code seems to work: > > > > > > > > https://crrev.com/c/3662501 - ozone/drm: Try 640x480 before picking > > > > the first mode if no EDID > > > > > > > > ...so we'll see how review of that goes. :-) > > > > Mirroring some of my comments on that review here :-) > > > > IMO, this should be addressed in the kernel, or not at all. The kernel > > ensures other aspects of DisplayPort implementation are compliant, so > > I don't think this would be any exception. Further, the kernel is the > > one creating the "safe" mode list, so it seems odd that userspace > > would override that. Finally, relying on every userspace to do the > > right thing is asking for trouble (we have 3 plac
Re: [PATCH v3 02/13] mm: handling Non-LRU pages returned by vm_normal_pages
On 5/24/2022 11:11 PM, Alistair Popple wrote:
Alex Sierra writes:
With DEVICE_COHERENT, we'll soon have vm_normal_pages() return
device-managed anonymous pages that are not LRU pages. Although they
behave like normal pages for purposes of mapping in CPU page, and for
COW. They do not support LRU lists, NUMA migration or THP.
We also introduced a FOLL_LRU flag that adds the same behaviour to
follow_page and related APIs, to allow callers to specify that they
expect to put pages on an LRU list.
Continuing the follow up from the thread for v2:
This means by default GUP can return non-LRU pages. I didn't see
anywhere that would be a problem but I didn't check everything. Did you
check this or is there some other reason I've missed that makes this not
a problem?
I have double checked all gup and pin_user_pages callers and none of them seem
to have interaction with LRU APIs.
And actually if I'm understanding things correctly callers of
GUP/PUP/follow_page_pte() should already expect to get non-LRU pages
returned:
page = vm_normal_page(vma, address, pte);
if ((flags & FOLL_LRU) && page && is_zone_device_page(page))
page = NULL;
if (!page && pte_devmap(pte) && (flags & (FOLL_GET | FOLL_PIN))) {
/*
* Only return device mapping pages in the FOLL_GET or FOLL_PIN
* case since they are only valid while holding the pgmap
* reference.
*/
*pgmap = get_dev_pagemap(pte_pfn(pte), *pgmap);
if (*pgmap)
page = pte_page(pte);
Which I think makes FOLL_LRU confusing, because if understand correctly
even with FOLL_LRU it is still possible for follow_page_pte() to return
a non-LRU page. Could we do something like this to make it consistent:
if ((flags & FOLL_LRU) && (page && is_zone_device_page(page) ||
!page && pte_devmap(pte)))
Hi Alistair,
Not sure if this suggestion is a replacement for the first or the second
condition in the snip code above. We know device coherent type will not
be set with devmap. So we could do the following:
if ((flags & FOLL_LRU) && page && is_zone_device_page(page))
- page = NULL;
+ goto no_page;
Regards,
Alex Sierra
Looking at callers that currently use FOLL_LRU I don't think this would
change any behaviour as they already filter out devmap through various
other means.
Signed-off-by: Alex Sierra
Acked-by: Felix Kuehling
---
fs/proc/task_mmu.c | 2 +-
include/linux/mm.h | 3 ++-
mm/gup.c | 2 ++
mm/huge_memory.c | 2 +-
mm/khugepaged.c| 9 ++---
mm/ksm.c | 6 +++---
mm/madvise.c | 4 ++--
mm/memory.c| 9 -
mm/mempolicy.c | 2 +-
mm/migrate.c | 4 ++--
mm/mlock.c | 2 +-
mm/mprotect.c | 2 +-
12 files changed, 30 insertions(+), 17 deletions(-)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index f46060eb91b5..5d620733f173 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -1785,7 +1785,7 @@ static struct page *can_gather_numa_stats(pte_t pte,
struct vm_area_struct *vma,
return NULL;
page = vm_normal_page(vma, addr, pte);
- if (!page)
+ if (!page || is_zone_device_page(page))
return NULL;
if (PageReserved(page))
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 9f44254af8ce..d7f253a0c41e 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -601,7 +601,7 @@ struct vm_operations_struct {
#endif
/*
* Called by vm_normal_page() for special PTEs to find the
-* page for @addr. This is useful if the default behavior
+* page for @addr. This is useful if the default behavior
* (using pte_page()) would not find the correct page.
*/
struct page *(*find_special_page)(struct vm_area_struct *vma,
@@ -2929,6 +2929,7 @@ struct page *follow_page(struct vm_area_struct *vma,
unsigned long address,
#define FOLL_NUMA 0x200 /* force NUMA hinting page fault */
#define FOLL_MIGRATION0x400 /* wait for page to replace migration
entry */
#define FOLL_TRIED0x800 /* a retry, previous pass started an IO */
+#define FOLL_LRU0x1000 /* return only LRU (anon or page cache) */
#define FOLL_REMOTE 0x2000 /* we are working on non-current tsk/mm */
#define FOLL_COW 0x4000 /* internal GUP flag */
#define FOLL_ANON 0x8000 /* don't do file mappings */
diff --git a/mm/gup.c b/mm/gup.c
index 501bc150792c..c9cbac06bcc5 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -479,6 +479,8 @@ static struct page *follow_page_pte(struct vm_area_struct
*vma,
}
page = vm_normal_page(vma, address, pte);
+ if ((flags & FOLL_LRU) && page && is_zone_device_page(page))
+ page = NULL;
if (!page && pte_devmap(pte) && (flags & (FOLL_GET | FOLL_PIN))) {
/*
* Only return device mapping pages in the FOLL_GET or FOLL_PIN
diff --git a/mm/huge_memory.
[PATCH] dma-buf: document how to find size, mention kernel versions
Document how to obtain the size of a DMA-BUF. This is what
Wayland compositors are doing.
Mention the kernel version numbers from which DMA-BUF features are
available.
Signed-off-by: Simon Ser
Cc: Daniel Vetter
Cc: Jason Ekstrand
---
include/uapi/linux/dma-buf.h | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/include/uapi/linux/dma-buf.h b/include/uapi/linux/dma-buf.h
index 8e4a2ca0bcbf..c95f6d3457d2 100644
--- a/include/uapi/linux/dma-buf.h
+++ b/include/uapi/linux/dma-buf.h
@@ -47,9 +47,13 @@
*
* If the driver or API with which the client is interacting uses implicit
* synchronization, waiting for prior work to complete can be done via
- * poll() on the DMA buffer file descriptor. If the driver or API requires
- * explicit synchronization, the client may have to wait on a sync_file or
- * other synchronization primitive outside the scope of the DMA buffer API.
+ * poll() on the DMA buffer file descriptor from kernel version 3.17. If the
+ * driver or API requires explicit synchronization, the client may have to wait
+ * on a sync_file or other synchronization primitive outside the scope of the
+ * DMA buffer API.
+ *
+ * From kernel version 3.12, user-space can use llseek(2) with the ``SEEK_END``
+ * whence to obtain the size of a DMA-BUF.
*/
struct dma_buf_sync {
/**
--
2.36.1
Re: [PATCH] dma-buf: document how to find size, mention kernel versions
On Thu, 26 May 2022 at 18:30, Simon Ser wrote:
>
> Document how to obtain the size of a DMA-BUF. This is what
> Wayland compositors are doing.
>
> Mention the kernel version numbers from which DMA-BUF features are
> available.
>
> Signed-off-by: Simon Ser
> Cc: Daniel Vetter
> Cc: Jason Ekstrand
> ---
> include/uapi/linux/dma-buf.h | 10 +++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/include/uapi/linux/dma-buf.h b/include/uapi/linux/dma-buf.h
> index 8e4a2ca0bcbf..c95f6d3457d2 100644
> --- a/include/uapi/linux/dma-buf.h
> +++ b/include/uapi/linux/dma-buf.h
> @@ -47,9 +47,13 @@
> *
> * If the driver or API with which the client is interacting uses implicit
> * synchronization, waiting for prior work to complete can be done via
> - * poll() on the DMA buffer file descriptor. If the driver or API requires
> - * explicit synchronization, the client may have to wait on a sync_file or
> - * other synchronization primitive outside the scope of the DMA buffer API.
> + * poll() on the DMA buffer file descriptor from kernel version 3.17. If the
> + * driver or API requires explicit synchronization, the client may have to
> wait
> + * on a sync_file or other synchronization primitive outside the scope of the
> + * DMA buffer API.
This looks good, but you missed the DOC: implicit fence polling in
dma-buf.c. Probably good to update that too.
> + *
> + * From kernel version 3.12, user-space can use llseek(2) with the
> ``SEEK_END``
> + * whence to obtain the size of a DMA-BUF.
This feels misplaced, especially since this ends up under the "DMA
Buffer ioctls" heading.
Maybe simplest to just put a "DMA Buffer misc uAPI" section directly
into dma-buf.rst and put that section there? Also probably good to add
that the size is invariant.
Or if you don't want a new heading, put it somewhere sensible in the
mmap section? Also just noticed that the DOC: cpu access section could
perhaps link to the relevant ioctl docs here to connect things better.
-Daniel
> */
> struct dma_buf_sync {
> /**
> --
> 2.36.1
>
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH] drm/msm/dp: force link training for display resolution change
During display resolution changes display have to be disabled first
followed by display enable with new resolution. This patch force
main link always be retrained during display enable procedure to
simplify implementation instead of manually kicking of irq_hpd
handle.
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/dp/dp_ctrl.c| 6 +++---
drivers/gpu/drm/msm/dp/dp_ctrl.h| 2 +-
drivers/gpu/drm/msm/dp/dp_display.c | 13 ++---
3 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 5ddb4e8..8b71013 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1545,7 +1545,7 @@ static int dp_ctrl_process_phy_test_request(struct
dp_ctrl_private *ctrl)
ret = dp_ctrl_on_link(&ctrl->dp_ctrl);
if (!ret)
- ret = dp_ctrl_on_stream(&ctrl->dp_ctrl);
+ ret = dp_ctrl_on_stream(&ctrl->dp_ctrl, false);
else
DRM_ERROR("failed to enable DP link controller\n");
@@ -1802,7 +1802,7 @@ static int dp_ctrl_link_retrain(struct dp_ctrl_private
*ctrl)
return dp_ctrl_setup_main_link(ctrl, &training_step);
}
-int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
+int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl, bool force_link_train)
{
int ret = 0;
bool mainlink_ready = false;
@@ -1827,7 +1827,7 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
}
}
- if (!dp_ctrl_channel_eq_ok(ctrl))
+ if (force_link_train || !dp_ctrl_channel_eq_ok(ctrl))
dp_ctrl_link_retrain(ctrl);
/* stop txing train pattern to end link training */
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
index 2433edb..dcc7af21 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
@@ -20,7 +20,7 @@ struct dp_ctrl {
};
int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl);
-int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl);
+int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl, bool force_link_train);
int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl);
int dp_ctrl_off(struct dp_ctrl *dp_ctrl);
void dp_ctrl_push_idle(struct dp_ctrl *dp_ctrl);
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index bfc6581..9246421 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -892,7 +892,7 @@ static int dp_display_enable(struct dp_display_private *dp,
u32 data)
return 0;
}
- rc = dp_ctrl_on_stream(dp->ctrl);
+ rc = dp_ctrl_on_stream(dp->ctrl, data);
if (!rc)
dp_display->power_on = true;
@@ -1594,6 +1594,7 @@ int msm_dp_display_enable(struct msm_dp *dp, struct
drm_encoder *encoder)
int rc = 0;
struct dp_display_private *dp_display;
u32 state;
+ bool force_link_train = false;
dp_display = container_of(dp, struct dp_display_private, dp_display);
if (!dp_display->dp_mode.drm_mode.clock) {
@@ -1622,10 +1623,12 @@ int msm_dp_display_enable(struct msm_dp *dp, struct
drm_encoder *encoder)
state = dp_display->hpd_state;
- if (state == ST_DISPLAY_OFF)
+ if (state == ST_DISPLAY_OFF) {
dp_display_host_phy_init(dp_display);
+ force_link_train = 1;
+ }
- dp_display_enable(dp_display, 0);
+ dp_display_enable(dp_display, force_link_train);
rc = dp_display_post_enable(dp);
if (rc) {
@@ -1634,10 +1637,6 @@ int msm_dp_display_enable(struct msm_dp *dp, struct
drm_encoder *encoder)
dp_display_unprepare(dp);
}
- /* manual kick off plug event to train link */
- if (state == ST_DISPLAY_OFF)
- dp_add_event(dp_display, EV_IRQ_HPD_INT, 0, 0);
-
/* completed connection */
dp_display->hpd_state = ST_CONNECTED;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
[RFC PATCH] dma-buf: Add a capabilities directory
To discover support for new DMA-BUF IOCTLs, user-space has no
choice but to try to perform the IOCTL on an existing DMA-BUF.
However, user-space may want to figure out whether or not the
IOCTL is available before it has a DMA-BUF at hand, e.g. at
initialization time in a Wayland compositor.
Add a /sys/kernel/dmabuf/caps directory which allows the DMA-BUF
subsystem to advertise supported features. Add a
sync_file_import_export entry which indicates that importing and
exporting sync_files from/to DMA-BUFs is supported.
Signed-off-by: Simon Ser
Cc: Jason Ekstrand
Cc: Daniel Vetter
Cc: Bas Nieuwenhuizen
Cc: Christian König
---
This depends on:
https://patchwork.freedesktop.org/series/103715/
.../ABI/testing/sysfs-kernel-dmabuf-buffers | 14 ++
drivers/dma-buf/Makefile | 2 +-
drivers/dma-buf/dma-buf-sysfs-stats.c | 13 +-
drivers/dma-buf/dma-buf-sysfs-stats.h | 6 ++-
drivers/dma-buf/dma-buf.c | 43 +--
include/uapi/linux/dma-buf.h | 6 +++
6 files changed, 66 insertions(+), 18 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
index 5d3bc997dc64..682d313689d8 100644
--- a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
+++ b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
@@ -22,3 +22,17 @@ KernelVersion: v5.13
Contact: Hridya Valsaraju
Description: This file is read-only and specifies the size of the DMA-BUF in
bytes.
+
+What: /sys/kernel/dmabuf/caps
+Date: May 2022
+KernelVersion: v5.19
+Contact: Simon Ser
+Description: This directory advertises DMA-BUF capabilities supported by the
+ kernel.
+
+What: /sys/kernel/dmabuf/caps/sync_file_import_export
+Date: May 2022
+KernelVersion: v5.19
+Contact: Simon Ser
+Description: This file is read-only and advertises support for importing and
+ exporting sync_files from/to DMA-BUFs.
diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile
index 4c9eb53ba3f8..afc874272710 100644
--- a/drivers/dma-buf/Makefile
+++ b/drivers/dma-buf/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-only
obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \
-dma-resv.o
+dma-resv.o dma-buf-sysfs-caps.o
obj-$(CONFIG_DMABUF_HEAPS) += dma-heap.o
obj-$(CONFIG_DMABUF_HEAPS) += heaps/
obj-$(CONFIG_SYNC_FILE)+= sync_file.o
diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.c
b/drivers/dma-buf/dma-buf-sysfs-stats.c
index 2bba0babcb62..09e43c8891d6 100644
--- a/drivers/dma-buf/dma-buf-sysfs-stats.c
+++ b/drivers/dma-buf/dma-buf-sysfs-stats.c
@@ -141,21 +141,13 @@ static const struct kset_uevent_ops
dmabuf_sysfs_no_uevent_ops = {
.filter = dmabuf_sysfs_uevent_filter,
};
-static struct kset *dma_buf_stats_kset;
static struct kset *dma_buf_per_buffer_stats_kset;
-int dma_buf_init_sysfs_statistics(void)
+int dma_buf_init_sysfs_statistics(struct kset *kset)
{
- dma_buf_stats_kset = kset_create_and_add("dmabuf",
-&dmabuf_sysfs_no_uevent_ops,
-kernel_kobj);
- if (!dma_buf_stats_kset)
- return -ENOMEM;
-
dma_buf_per_buffer_stats_kset = kset_create_and_add("buffers",
&dmabuf_sysfs_no_uevent_ops,
-
&dma_buf_stats_kset->kobj);
+ &kset->kobj);
if (!dma_buf_per_buffer_stats_kset) {
- kset_unregister(dma_buf_stats_kset);
return -ENOMEM;
}
@@ -165,7 +157,6 @@ int dma_buf_init_sysfs_statistics(void)
void dma_buf_uninit_sysfs_statistics(void)
{
kset_unregister(dma_buf_per_buffer_stats_kset);
- kset_unregister(dma_buf_stats_kset);
}
int dma_buf_stats_setup(struct dma_buf *dmabuf)
diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.h
b/drivers/dma-buf/dma-buf-sysfs-stats.h
index a49c6e2650cc..798c54fb8ee3 100644
--- a/drivers/dma-buf/dma-buf-sysfs-stats.h
+++ b/drivers/dma-buf/dma-buf-sysfs-stats.h
@@ -8,9 +8,11 @@
#ifndef _DMA_BUF_SYSFS_STATS_H
#define _DMA_BUF_SYSFS_STATS_H
+struct kset;
+
#ifdef CONFIG_DMABUF_SYSFS_STATS
-int dma_buf_init_sysfs_statistics(void);
+int dma_buf_init_sysfs_statistics(struct kset *kset);
void dma_buf_uninit_sysfs_statistics(void);
int dma_buf_stats_setup(struct dma_buf *dmabuf);
@@ -18,7 +20,7 @@ int dma_buf_stats_setup(struct dma_buf *dmabuf);
void dma_buf_stats_teardown(struct dma_buf *dmabuf);
#else
-static inline int dma_buf_init_sysfs_statistics(void)
+static inline int dma_buf_init_sysfs_statistics(struct kset *kset)
{
return 0;
}
diff --git a/drivers/dma-buf/dma-buf.c b/drivers
[RFC PATCH v2] dma-buf: Add a capabilities directory
To discover support for new DMA-BUF IOCTLs, user-space has no
choice but to try to perform the IOCTL on an existing DMA-BUF.
However, user-space may want to figure out whether or not the
IOCTL is available before it has a DMA-BUF at hand, e.g. at
initialization time in a Wayland compositor.
Add a /sys/kernel/dmabuf/caps directory which allows the DMA-BUF
subsystem to advertise supported features. Add a
sync_file_import_export entry which indicates that importing and
exporting sync_files from/to DMA-BUFs is supported.
Signed-off-by: Simon Ser
Cc: Jason Ekstrand
Cc: Daniel Vetter
Cc: Bas Nieuwenhuizen
Cc: Christian König
---
Oops, I forgot to check in new files after spliting a commit.
Fixed.
This depends on:
https://patchwork.freedesktop.org/series/103715/
.../ABI/testing/sysfs-kernel-dmabuf-buffers | 14 +
drivers/dma-buf/Makefile | 2 +-
drivers/dma-buf/dma-buf-sysfs-caps.c | 51 +++
drivers/dma-buf/dma-buf-sysfs-caps.h | 16 ++
drivers/dma-buf/dma-buf-sysfs-stats.c | 13 +
drivers/dma-buf/dma-buf-sysfs-stats.h | 6 ++-
drivers/dma-buf/dma-buf.c | 43 ++--
include/uapi/linux/dma-buf.h | 6 +++
8 files changed, 133 insertions(+), 18 deletions(-)
create mode 100644 drivers/dma-buf/dma-buf-sysfs-caps.c
create mode 100644 drivers/dma-buf/dma-buf-sysfs-caps.h
diff --git a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
index 5d3bc997dc64..682d313689d8 100644
--- a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
+++ b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
@@ -22,3 +22,17 @@ KernelVersion: v5.13
Contact: Hridya Valsaraju
Description: This file is read-only and specifies the size of the DMA-BUF in
bytes.
+
+What: /sys/kernel/dmabuf/caps
+Date: May 2022
+KernelVersion: v5.19
+Contact: Simon Ser
+Description: This directory advertises DMA-BUF capabilities supported by the
+ kernel.
+
+What: /sys/kernel/dmabuf/caps/sync_file_import_export
+Date: May 2022
+KernelVersion: v5.19
+Contact: Simon Ser
+Description: This file is read-only and advertises support for importing and
+ exporting sync_files from/to DMA-BUFs.
diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile
index 4c9eb53ba3f8..afc874272710 100644
--- a/drivers/dma-buf/Makefile
+++ b/drivers/dma-buf/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-only
obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \
-dma-resv.o
+dma-resv.o dma-buf-sysfs-caps.o
obj-$(CONFIG_DMABUF_HEAPS) += dma-heap.o
obj-$(CONFIG_DMABUF_HEAPS) += heaps/
obj-$(CONFIG_SYNC_FILE)+= sync_file.o
diff --git a/drivers/dma-buf/dma-buf-sysfs-caps.c
b/drivers/dma-buf/dma-buf-sysfs-caps.c
new file mode 100644
index ..c760e55353bc
--- /dev/null
+++ b/drivers/dma-buf/dma-buf-sysfs-caps.c
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * DMA-BUF sysfs capabilities.
+ *
+ * Copyright (C) 2022 Simon Ser
+ */
+
+#include
+#include
+
+#include "dma-buf-sysfs-caps.h"
+
+static ssize_t sync_file_import_export_show(struct kobject *kobj,
+ struct kobj_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "1\n");
+}
+
+static struct kobj_attribute dma_buf_sync_file_import_export_attr =
+ __ATTR_RO(sync_file_import_export);
+
+static struct attribute *dma_buf_caps_attrs[] = {
+ &dma_buf_sync_file_import_export_attr.attr,
+ NULL,
+};
+
+static const struct attribute_group dma_buf_caps_attr_group = {
+ .attrs = dma_buf_caps_attrs,
+};
+
+static struct kobject *dma_buf_caps_kobj;
+
+int dma_buf_init_sysfs_capabilities(struct kset *kset)
+{
+ int ret;
+
+ dma_buf_caps_kobj = kobject_create_and_add("caps", &kset->kobj);
+ if (!dma_buf_caps_kobj)
+ return -ENOMEM;
+
+ ret = sysfs_create_group(dma_buf_caps_kobj, &dma_buf_caps_attr_group);
+ if (ret)
+ kobject_put(dma_buf_caps_kobj);
+ return ret;
+}
+
+void dma_buf_uninit_sysfs_capabilities(void)
+{
+ kobject_put(dma_buf_caps_kobj);
+}
diff --git a/drivers/dma-buf/dma-buf-sysfs-caps.h
b/drivers/dma-buf/dma-buf-sysfs-caps.h
new file mode 100644
index ..d7bcef490b31
--- /dev/null
+++ b/drivers/dma-buf/dma-buf-sysfs-caps.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * DMA-BUF sysfs capabilities.
+ *
+ * Copyright (C) 2022 Simon Ser
+ */
+
+#ifndef _DMA_BUF_SYSFS_CAPS_H
+#define _DMA_BUF_SYSFS_CAPS_H
+
+struct kset;
+
+int dma_buf_init_sysfs_capabilities(struct kset *kset);
+void dma_buf_uninit_sysfs_capabilities(void);
+
+#endif // _DMA_BUF_SYSFS_CAPS_H
diff --git a/drivers/dma-buf/dma-buf-
Re: [RFC PATCH v2] dma-buf: Add a capabilities directory
On Thursday, May 26th, 2022 at 19:40, Simon Ser wrote: > diff --git a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers > b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers > index 5d3bc997dc64..682d313689d8 100644 > --- a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers > +++ b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers > @@ -22,3 +22,17 @@ KernelVersion: v5.13 > Contact: Hridya Valsaraju > Description: This file is read-only and specifies the size of the DMA-BUF in > bytes. > + > +What:/sys/kernel/dmabuf/caps > +Date:May 2022 > +KernelVersion: v5.19 > +Contact: Simon Ser > +Description: This directory advertises DMA-BUF capabilities supported by the > + kernel. > + > +What:/sys/kernel/dmabuf/caps/sync_file_import_export > +Date:May 2022 > +KernelVersion: v5.19 > +Contact: Simon Ser > +Description: This file is read-only and advertises support for importing and > + exporting sync_files from/to DMA-BUFs. I now realize these entries should probably be in their own file: Documentation/ABI/testing/sysfs-kernel-dmabuf-caps. Also Daniel has noticed this is for 5.20, not 5.19.
[PATCH] fbdev: vesafb: Fix a use-after-free due early fb_info cleanup
Commit b3c9a924aab6 ("fbdev: vesafb: Cleanup fb_info in .fb_destroy rather
than .remove") fixed a use-after-free error due the vesafb driver freeing
the fb_info in the .remove handler instead of doing it in .fb_destroy.
This can happen if the .fb_destroy callback is executed after the .remove
callback, since the former tries to access a pointer freed by the latter.
But that change didn't take into account that another possible scenario is
that .fb_destroy is called before the .remove callback. For example, if no
process has the fbdev chardev opened by the time the driver is removed.
If that's the case, fb_info will be freed when unregister_framebuffer() is
called, making the fb_info pointer accessed in vesafb_remove() after that
to no longer be valid.
To prevent that, move the expression containing the info->par to happen
before the unregister_framebuffer() function call.
Fixes: b3c9a924aab6 ("fbdev: vesafb: Cleanup fb_info in .fb_destroy rather than
.remove")
Reported-by: Pascal Ernster
Signed-off-by: Javier Martinez Canillas
---
drivers/video/fbdev/vesafb.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c
index e25e8de5ff67..929d4775cb4b 100644
--- a/drivers/video/fbdev/vesafb.c
+++ b/drivers/video/fbdev/vesafb.c
@@ -490,11 +490,12 @@ static int vesafb_remove(struct platform_device *pdev)
{
struct fb_info *info = platform_get_drvdata(pdev);
- /* vesafb_destroy takes care of info cleanup */
- unregister_framebuffer(info);
if (((struct vesafb_par *)(info->par))->region)
release_region(0x3c0, 32);
+ /* vesafb_destroy takes care of info cleanup */
+ unregister_framebuffer(info);
+
return 0;
}
--
2.36.1
[pull] amdgpu, amdkfd, radeon drm-next-5.19
Hi Dave, Daniel, Fixes for 5.19. The size is largely due to updates to new IPs which were added in this cycle. The following changes since commit c4955d9cd2fc56c43e78c908dad4e2cac7cc9073: Merge tag 'drm-intel-next-fixes-2022-05-24' of git://anongit.freedesktop.org/drm/drm-intel into drm-next (2022-05-25 12:03:41 +1000) are available in the Git repository at: https://gitlab.freedesktop.org/agd5f/linux.git tags/amd-drm-next-5.19-2022-05-26 for you to fetch changes up to 62e9bd20035b53ff6c679499c08546d96c6c60a7: drm/amdgpu: add beige goby PCI ID (2022-05-26 14:56:33 -0400) amd-drm-next-5.19-2022-05-26: amdgpu: - Link training fixes - DPIA fixes - Misc code cleanups - Aux fixes - Hotplug fixes - More FP clean up - Misc GFX9/10 fixes - Fix a possible memory leak in SMU shutdown - SMU 13 updates - RAS fixes - TMZ fixes - GC 11 updates - SMU 11 metrics fixes - Fix coverage blend mode for overlay plane - Note DDR vs LPDDR memory - Fuzz fix for CS IOCTL - Add new PCI DID amdkfd: - Clean up hive setup - Misc fixes radeon: - Fix a possible NULL pointer dereference Alan Liu (1): drm/amd/display: Add HDMI_ACP_SEND register Alex Deucher (3): drm/amdgpu/discovery: validate VCN and SDMA instances drm/amdgpu: differentiate between LP and non-LP DDR memory drm/amdgpu: add beige goby PCI ID Alvin Lee (1): drm/amd/display: Clean up code in dc Aric Cyr (1): drm/amd/display: 3.2.186 Bhawanpreet Lakha (1): drm/amd/display: Fic incorrect pipe being used for clk update Candice Li (1): drm/amdgpu: Resolve pcie_bif RAS recovery bug Christian König (1): drm/amdgpu: cleanup ctx implementation Dan Carpenter (2): drm/amdgpu/pm: smu_v13_0_4: delete duplicate condition drm/amdgpu: Off by one in dm_dmub_outbox1_low_irq() Dave Airlie (1): drm/amdgpu/cs: make commands with 0 chunks illegal behaviour. David Galiffi (1): drm/amd/display: Check if modulo is 0 before dividing. Derek Lai (1): drm/amd/display: Allow individual control of eDP hotplug support Eric Huang (1): drm/amdkfd: port cwsr trap handler from dkms branch Evan Quan (8): drm/amd/pm: enable more dpm features for SMU 13.0.0 drm/amd/pm: skip dpm disablement on suspend for SMU 13.0.0 drm/amd/pm: update SMU 13.0.0 driver_if header drm/amd/pm: correct the softpptable ids used for SMU 13.0.0 drm/amd/pm: enable more dpm features for SMU 13.0.0 drm/amd/pm: enable memory temp reading for SMU 13.0.0 drm/amd/pm: correct the metrics version for SMU 11.0.11/12/13 drm/amdgpu: suppress some compile warnings Gong Yuanjun (2): drm/radeon: fix a possible null pointer dereference drm/amd/pm: fix a potential gpu_metrics_table memory leak Haohui Mai (3): drm/amdgpu: Clean up of initializing doorbells for gfx_v9 and gfx_v10 drm/amdgpu: Set CP_HQD_PQ_CONTROL.RPTR_BLOCK_SIZE correctly drm/amdgpu/gfx10: rework KIQ programming Jasdeep Dhillon (1): drm/amd/display: Move FPU associated DCN30 code to DML folder Jay Cornwall (1): drm/amdkfd: Add gfx11 trap handler Jimmy Kizito (2): drm/amd/display: Update link training fallback behaviour. drm/amd/display: Query DPIA HPD status. Jonathan Kim (1): drm/amdkfd: simplify cpu hive assignment Julia Lawall (2): drm/amdgpu/gfx: fix typos in comments drm/amdkfd: fix typo in comment Lijo Lazar (2): drm/amd/pm: Fix missing thermal throttler status drm/amd/pm: Return auto perf level, if unsupported Michael Strauss (1): Revert "drm/amd/display: Refactor LTTPR cap retrieval" Nicholas Kazlauskas (1): drm/amd/display: Check zero planes for OTG disable W/A on clock change Paul Hsieh (1): drm/amd/display: clear request when release aux engine Prike Liang (1): drm/amdgpu: clean up asd on the ta_firmware_header_v2_0 Stanley.Yang (1): drm/amdgpu: support ras on SRIOV Sung Joon Kim (1): drm/amd/display: add Coverage blend mode for overlay plane Sunil Khatri (3): drm/amdgpu: move amdgpu_gmc_tmz_set after ip_version populated drm/amdgpu: change code name to ip version for tmz set drm/amdgpu: add support of tmz for GC 10.3.7 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c |2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c |8 +- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c |4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c| 46 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h| 11 +- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 +- drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 19 +- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c|1 + drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c| 29 +- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c|2 +- driver
Re: [PATCH v2 -next] drm/display: Fix build error without CONFIG_OF
On Wed, May 25, 2022 at 3:36 PM Dmitry Baryshkov
wrote:
> On Mon, 23 May 2022 at 15:55, Linus Walleij wrote:
> > Nope, failed:
> >
> > $ dim push-branch drm-misc-next
> > dim: ac890b9eeb9b ("drm/display: Fix build error without CONFIG_OF"):
> > Fixes: SHA1 in not pointing at an ancestor:
> > dim: f5d01644921b ("drm/msm: select DRM_DP_AUX_BUS for the AUX bus
> > support")
> > dim: ERROR: issues in commits detected, aborting
> >
> > I don't know what to do with this, sorry. The other committers are maybe
> > better
> > with this kind of situations. I think it is designed to stop me from
> > shooting myself
> > in the foot.
>
> Linus, can we get an ack from you (or anybody else from DRM core) to
> merge it through drm/msm tree?
Acked-by: Linus Walleij
> After a second thought, I think the patch contains wrong Fixes tag. It
> should be:
>
> Fixes: 1e0f66420b13 ("drm/display: Introduce a DRM display-helper module")
>
> With that in place would we be able to merge it through drm-misc? Does
> it needs to be resubmitted?
But it doesn't apply to drm-misc... that's my problem :/
Yours,
Linus Walleij
Re: [1/2] dma-buf: Add an API for exporting sync files (v14)
Acked-by: Lionel Landwerlin
Re: [2/2] dma-buf: Add an API for importing sync files (v9)
Just noticed a small nit on this one : ordering via these fences, it is the respnosibility of userspace to use -> responsibility Acked-by: Lionel Landwerlin Cheers, -Lionel
Re: [PATCH 05/25] drm/msm/dpu: move pipe_hw to dpu_plane_state
On 5/13/2022 11:37 PM, Dmitry Baryshkov wrote:
On 04/05/2022 01:32, Abhinav Kumar wrote:
On 2/9/2022 9:25 AM, Dmitry Baryshkov wrote:
In preparation to adding fully virtualized planes, move struct
dpu_hw_pipe instance from struct dpu_plane to struct dpu_plane_state, as
it will become a part of state (allocated during atomic check) rather
than part of a plane (allocated during boot).
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 104 --
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 2 +
2 files changed, 58 insertions(+), 48 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 37742f74a7bf..0247ff8a67a2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -100,7 +100,6 @@ struct dpu_plane {
enum dpu_sspp pipe;
- struct dpu_hw_pipe *pipe_hw;
uint32_t color_fill;
bool is_error;
bool is_rt_pipe;
@@ -300,6 +299,7 @@ static void _dpu_plane_set_qos_lut(struct
drm_plane *plane,
struct drm_framebuffer *fb, struct dpu_hw_pipe_cfg *pipe_cfg)
{
struct dpu_plane *pdpu = to_dpu_plane(plane);
+ struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
const struct dpu_format *fmt = NULL;
u64 qos_lut;
u32 total_fl = 0, lut_usage;
@@ -331,7 +331,7 @@ static void _dpu_plane_set_qos_lut(struct
drm_plane *plane,
fmt ? (char *)&fmt->base.pixel_format : NULL,
pdpu->is_rt_pipe, total_fl, qos_lut);
- pdpu->pipe_hw->ops.setup_creq_lut(pdpu->pipe_hw, qos_lut);
+ pstate->pipe_hw->ops.setup_creq_lut(pstate->pipe_hw, qos_lut);
}
/**
@@ -343,6 +343,7 @@ static void _dpu_plane_set_danger_lut(struct
drm_plane *plane,
struct drm_framebuffer *fb)
{
struct dpu_plane *pdpu = to_dpu_plane(plane);
+ struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
const struct dpu_format *fmt = NULL;
u32 danger_lut, safe_lut;
@@ -382,7 +383,7 @@ static void _dpu_plane_set_danger_lut(struct
drm_plane *plane,
danger_lut,
safe_lut);
- pdpu->pipe_hw->ops.setup_danger_safe_lut(pdpu->pipe_hw,
+ pstate->pipe_hw->ops.setup_danger_safe_lut(pstate->pipe_hw,
danger_lut, safe_lut);
}
@@ -396,14 +397,15 @@ static void _dpu_plane_set_qos_ctrl(struct
drm_plane *plane,
bool enable, u32 flags)
{
struct dpu_plane *pdpu = to_dpu_plane(plane);
+ struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
struct dpu_hw_pipe_qos_cfg pipe_qos_cfg;
memset(&pipe_qos_cfg, 0, sizeof(pipe_qos_cfg));
if (flags & DPU_PLANE_QOS_VBLANK_CTRL) {
- pipe_qos_cfg.creq_vblank =
pdpu->pipe_hw->cap->sblk->creq_vblank;
+ pipe_qos_cfg.creq_vblank =
pstate->pipe_hw->cap->sblk->creq_vblank;
pipe_qos_cfg.danger_vblank =
- pdpu->pipe_hw->cap->sblk->danger_vblank;
+ pstate->pipe_hw->cap->sblk->danger_vblank;
pipe_qos_cfg.vblank_en = enable;
}
@@ -429,7 +431,7 @@ static void _dpu_plane_set_qos_ctrl(struct
drm_plane *plane,
pipe_qos_cfg.danger_vblank,
pdpu->is_rt_pipe);
- pdpu->pipe_hw->ops.setup_qos_ctrl(pdpu->pipe_hw,
+ pstate->pipe_hw->ops.setup_qos_ctrl(pstate->pipe_hw,
&pipe_qos_cfg);
}
@@ -443,18 +445,19 @@ static void _dpu_plane_set_ot_limit(struct
drm_plane *plane,
struct drm_crtc *crtc, struct dpu_hw_pipe_cfg *pipe_cfg)
{
struct dpu_plane *pdpu = to_dpu_plane(plane);
+ struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
struct dpu_vbif_set_ot_params ot_params;
struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
memset(&ot_params, 0, sizeof(ot_params));
- ot_params.xin_id = pdpu->pipe_hw->cap->xin_id;
- ot_params.num = pdpu->pipe_hw->idx - SSPP_NONE;
+ ot_params.xin_id = pstate->pipe_hw->cap->xin_id;
+ ot_params.num = pstate->pipe_hw->idx - SSPP_NONE;
ot_params.width = drm_rect_width(&pipe_cfg->src_rect);
ot_params.height = drm_rect_height(&pipe_cfg->src_rect);
ot_params.is_wfd = !pdpu->is_rt_pipe;
ot_params.frame_rate = drm_mode_vrefresh(&crtc->mode);
ot_params.vbif_idx = VBIF_RT;
- ot_params.clk_ctrl = pdpu->pipe_hw->cap->clk_ctrl;
+ ot_params.clk_ctrl = pstate->pipe_hw->cap->clk_ctrl;
ot_params.rd = true;
dpu_vbif_set_ot_limit(dpu_kms, &ot_params);
@@ -467,14 +470,15 @@ static void _dpu_plane_set_ot_limit(struct
drm_plane *plane,
static void _dpu_plane_set_qos_remap(struct drm_plane *plane)
{
struct dpu_plane *pdpu = to_dpu_plane(plane);
+ struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
struct dpu_vbif_set_qos_params qos_params;
struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
memset(&qos_params, 0, sizeof(qos_params));
qos_params.vbif_idx = VBIF_RT;
- qos_params.cl
[PATCH] drm/i2c/sil164: Drop no-op remove function
A remove callback that just returns 0 is equivalent to no callback at all
as can be seen in i2c_device_remove(). So simplify accordingly.
Signed-off-by: Uwe Kleine-König
---
drivers/gpu/drm/i2c/sil164_drv.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/gpu/drm/i2c/sil164_drv.c b/drivers/gpu/drm/i2c/sil164_drv.c
index 741886b54419..1bc0b5de4499 100644
--- a/drivers/gpu/drm/i2c/sil164_drv.c
+++ b/drivers/gpu/drm/i2c/sil164_drv.c
@@ -370,12 +370,6 @@ sil164_probe(struct i2c_client *client, const struct
i2c_device_id *id)
return 0;
}
-static int
-sil164_remove(struct i2c_client *client)
-{
- return 0;
-}
-
static struct i2c_client *
sil164_detect_slave(struct i2c_client *client)
{
@@ -427,7 +421,6 @@ MODULE_DEVICE_TABLE(i2c, sil164_ids);
static struct drm_i2c_encoder_driver sil164_driver = {
.i2c_driver = {
.probe = sil164_probe,
- .remove = sil164_remove,
.driver = {
.name = "sil164",
},
base-commit: 4b0986a3613c92f4ec1bdc7f60ec66fea135991f
--
2.36.1
[pull] amdgpu drm-next-5.19 (bonus)
Hi Dave, Daniel, This is an additional pull on top of the one I send a few minutes ago. It adds a minor UAPI change and updates the fdinfo format to match the generic drm fdinfo format. They are pretty small, but if you'd prefer, I can wait until next cycle. The following changes since commit 62e9bd20035b53ff6c679499c08546d96c6c60a7: drm/amdgpu: add beige goby PCI ID (2022-05-26 14:56:33 -0400) are available in the Git repository at: https://gitlab.freedesktop.org/agd5f/linux.git tags/amd-drm-next-5.19-2022-05-26-2 for you to fetch changes up to 9bdc1992c925a35c6f7200e8abe54e3f00ce7719: drm/amdgpu: add drm-client-id to fdinfo v2 (2022-05-26 14:56:34 -0400) amd-drm-next-5.19-2022-05-26-2: amdgpu: - Update fdinfo to the common drm format UAPI: - Add VM_NOALLOC GPUVM attribute to prevent buffers for going into the MALL Add AMDGPU_GEM_CREATE_DISCARDABLE flag to create buffers that can be discarded on eviction Mesa code which uses these: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/16466 Christian König (5): drm/amdgpu: add AMDGPU_GEM_CREATE_DISCARDABLE drm/amdgpu: add AMDGPU_VM_NOALLOC v2 drm/amdgpu: bump minor version number drm/amdgpu: Convert to common fdinfo format v5 drm/amdgpu: add drm-client-id to fdinfo v2 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c| 177 +++-- drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h| 12 +- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c| 5 +- drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c | 68 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c| 9 +- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 - drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 2 +- drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 3 + drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c | 3 + drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 2 +- include/uapi/drm/amdgpu_drm.h | 6 + 12 files changed, 154 insertions(+), 135 deletions(-)
[PATCH] drm/nouveau/kms: Fix failure path for creating DP connectors
It looks like that when we moved nouveau over to using drm_dp_aux_init()
and registering it's aux bus during late connector registration, we totally
forgot to fix the failure codepath in nouveau_connector_create() - as it
still seems to assume that drm_dp_aux_init() can fail (it can't).
So, let's fix that and also add a missing check to ensure that we've
properly allocated nv_connector->aux.name while we're at it.
Signed-off-by: Lyude Paul
Fixes: fd43ad9d47e7 ("drm/nouveau/kms/nv50-: Move AUX adapter reg to connector
late register/early unregister")
Cc: # v5.14+
---
drivers/gpu/drm/nouveau/nouveau_connector.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c
b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 22b83a6577eb..df83c4654e26 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1361,13 +1361,11 @@ nouveau_connector_create(struct drm_device *dev,
snprintf(aux_name, sizeof(aux_name), "sor-%04x-%04x",
dcbe->hasht, dcbe->hashm);
nv_connector->aux.name = kstrdup(aux_name, GFP_KERNEL);
- drm_dp_aux_init(&nv_connector->aux);
- if (ret) {
- NV_ERROR(drm, "Failed to init AUX adapter for
sor-%04x-%04x: %d\n",
-dcbe->hasht, dcbe->hashm, ret);
+ if (!nv_connector->aux.name) {
kfree(nv_connector);
- return ERR_PTR(ret);
+ return ERR_PTR(-ENOMEM);
}
+ drm_dp_aux_init(&nv_connector->aux);
fallthrough;
default:
funcs = &nouveau_connector_funcs;
--
2.35.3
[PATCH 1/2] drm/nouveau/acpi: Don't print error when we get -EINPROGRESS from pm_runtime
Since this isn't actually a failure.
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/nouveau/nouveau_display.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c
b/drivers/gpu/drm/nouveau/nouveau_display.c
index 2cd0932b3d68..9f5a45f24e5b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -537,7 +537,7 @@ nouveau_display_acpi_ntfy(struct notifier_block *nb,
unsigned long val,
* it's own hotplug events.
*/
pm_runtime_put_autosuspend(drm->dev->dev);
- } else if (ret == 0) {
+ } else if (ret == 0 || ret == -EINPROGRESS) {
/* We've started resuming the GPU already, so
* it will handle scheduling a full reprobe
* itself
--
2.35.3
[PATCH 2/2] drm/nouveau: Don't pm_runtime_put_sync(), only pm_runtime_put_autosuspend()
While trying to fix another issue, it occurred to me that I don't actually
think there is any situation where we want pm_runtime_put() in nouveau to
be synchronous. In fact, this kind of just seems like it would cause
issues where we may unexpectedly block a thread we don't expect to be
blocked.
So, let's only use pm_runtime_put_autosuspend().
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/nouveau/nouveau_display.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_fbcon.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c
b/drivers/gpu/drm/nouveau/nouveau_display.c
index 9f5a45f24e5b..103bfb11acb9 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -515,7 +515,7 @@ nouveau_display_hpd_work(struct work_struct *work)
pm_runtime_mark_last_busy(drm->dev->dev);
noop:
- pm_runtime_put_sync(drm->dev->dev);
+ pm_runtime_put(drm->dev->dev);
}
#ifdef CONFIG_ACPI
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 4f9b3aa5deda..20ac1ce2c0f1 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -466,7 +466,7 @@ nouveau_fbcon_set_suspend_work(struct work_struct *work)
if (state == FBINFO_STATE_RUNNING) {
nouveau_fbcon_hotplug_resume(drm->fbcon);
pm_runtime_mark_last_busy(drm->dev->dev);
- pm_runtime_put_sync(drm->dev->dev);
+ pm_runtime_put_autosuspend(drm->dev->dev);
}
}
--
2.35.3
Re: [RFC PATCH v2] dma-buf: Add a capabilities directory
On Thu, May 26, 2022 at 12:40 PM Simon Ser wrote:
> To discover support for new DMA-BUF IOCTLs, user-space has no
> choice but to try to perform the IOCTL on an existing DMA-BUF.
> However, user-space may want to figure out whether or not the
> IOCTL is available before it has a DMA-BUF at hand, e.g. at
> initialization time in a Wayland compositor.
>
> Add a /sys/kernel/dmabuf/caps directory which allows the DMA-BUF
> subsystem to advertise supported features. Add a
> sync_file_import_export entry which indicates that importing and
> exporting sync_files from/to DMA-BUFs is supported.
>
> Signed-off-by: Simon Ser
> Cc: Jason Ekstrand
> Cc: Daniel Vetter
> Cc: Bas Nieuwenhuizen
> Cc: Christian König
> ---
>
> Oops, I forgot to check in new files after spliting a commit.
> Fixed.
>
> This depends on:
> https://patchwork.freedesktop.org/series/103715/
>
> .../ABI/testing/sysfs-kernel-dmabuf-buffers | 14 +
> drivers/dma-buf/Makefile | 2 +-
> drivers/dma-buf/dma-buf-sysfs-caps.c | 51 +++
> drivers/dma-buf/dma-buf-sysfs-caps.h | 16 ++
> drivers/dma-buf/dma-buf-sysfs-stats.c | 13 +
> drivers/dma-buf/dma-buf-sysfs-stats.h | 6 ++-
> drivers/dma-buf/dma-buf.c | 43 ++--
> include/uapi/linux/dma-buf.h | 6 +++
> 8 files changed, 133 insertions(+), 18 deletions(-)
> create mode 100644 drivers/dma-buf/dma-buf-sysfs-caps.c
> create mode 100644 drivers/dma-buf/dma-buf-sysfs-caps.h
>
> diff --git a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
> b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
> index 5d3bc997dc64..682d313689d8 100644
> --- a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
> +++ b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
> @@ -22,3 +22,17 @@ KernelVersion: v5.13
> Contact: Hridya Valsaraju
> Description: This file is read-only and specifies the size of the
> DMA-BUF in
> bytes.
> +
> +What: /sys/kernel/dmabuf/caps
> +Date: May 2022
> +KernelVersion: v5.19
> +Contact: Simon Ser
> +Description: This directory advertises DMA-BUF capabilities supported
> by the
> + kernel.
> +
> +What: /sys/kernel/dmabuf/caps/sync_file_import_export
> +Date: May 2022
> +KernelVersion: v5.19
> +Contact: Simon Ser
> +Description: This file is read-only and advertises support for
> importing and
> + exporting sync_files from/to DMA-BUFs.
> diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile
> index 4c9eb53ba3f8..afc874272710 100644
> --- a/drivers/dma-buf/Makefile
> +++ b/drivers/dma-buf/Makefile
> @@ -1,6 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0-only
> obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \
> -dma-resv.o
> +dma-resv.o dma-buf-sysfs-caps.o
> obj-$(CONFIG_DMABUF_HEAPS) += dma-heap.o
> obj-$(CONFIG_DMABUF_HEAPS) += heaps/
> obj-$(CONFIG_SYNC_FILE)+= sync_file.o
> diff --git a/drivers/dma-buf/dma-buf-sysfs-caps.c
> b/drivers/dma-buf/dma-buf-sysfs-caps.c
> new file mode 100644
> index ..c760e55353bc
> --- /dev/null
> +++ b/drivers/dma-buf/dma-buf-sysfs-caps.c
> @@ -0,0 +1,51 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * DMA-BUF sysfs capabilities.
> + *
> + * Copyright (C) 2022 Simon Ser
> + */
> +
> +#include
> +#include
> +
> +#include "dma-buf-sysfs-caps.h"
> +
> +static ssize_t sync_file_import_export_show(struct kobject *kobj,
> + struct kobj_attribute *attr,
> + char *buf)
> +{
> + return sysfs_emit(buf, "1\n");
> +}
> +
> +static struct kobj_attribute dma_buf_sync_file_import_export_attr =
> + __ATTR_RO(sync_file_import_export);
> +
> +static struct attribute *dma_buf_caps_attrs[] = {
> + &dma_buf_sync_file_import_export_attr.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group dma_buf_caps_attr_group = {
> + .attrs = dma_buf_caps_attrs,
> +};
> +
> +static struct kobject *dma_buf_caps_kobj;
> +
> +int dma_buf_init_sysfs_capabilities(struct kset *kset)
> +{
> + int ret;
> +
> + dma_buf_caps_kobj = kobject_create_and_add("caps", &kset->kobj);
> + if (!dma_buf_caps_kobj)
> + return -ENOMEM;
> +
> + ret = sysfs_create_group(dma_buf_caps_kobj,
> &dma_buf_caps_attr_group);
> + if (ret)
> + kobject_put(dma_buf_caps_kobj);
> + return ret;
> +}
> +
> +void dma_buf_uninit_sysfs_capabilities(void)
> +{
> + kobject_put(dma_buf_caps_kobj);
> +}
> diff --git a/drivers/dma-buf/dma-buf-sysfs-caps.h
> b/drivers/dma-buf/dma-buf-sysfs-caps.h
> new file mode 100644
> index ..d7bcef490b31
> --- /dev/null
> +++ b/drivers/dma-buf/dma-buf-sysfs-caps.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + *
Re: [PATCH 17/25] drm/msm/dpu: drop src_split and multirect check from dpu_crtc_atomic_check
On 2/9/2022 9:25 AM, Dmitry Baryshkov wrote:
Neither source split nor multirect are properly supported at this
moment. Both of these checks depend on normalized_zpos being equal for
several planes (which is never the case for normalized zpos).
Drop these checks to simplify dpu_crtc_atomic_check(). The actual
support for either of these features is not removed from the backend
code (sspp, ctl, etc).
Signed-off-by: Dmitry Baryshkov
This is true that current implementation of these features is not really
compatible with any of the existing opensource compositors.
Till we have a better way to utilize this, I am okay to drop this code.
When we do re-visit this implementation especially for src split,
atleast we have a reference of this PW link of what has been removed and
can take it from there.
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 166 ++-
1 file changed, 12 insertions(+), 154 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 751c64012058..cbd0e50c2bd3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1046,13 +1046,6 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
drm_crtc_vblank_on(crtc);
}
-struct plane_state {
- struct dpu_plane_state *dpu_pstate;
- const struct drm_plane_state *drm_pstate;
- int stage;
- u32 pipe_id;
-};
-
static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
struct drm_atomic_state *state)
{
@@ -1060,28 +1053,21 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
crtc);
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc_state);
- struct plane_state *pstates;
const struct drm_plane_state *pstate;
struct drm_plane *plane;
struct drm_display_mode *mode;
- int cnt = 0, rc = 0, mixer_width = 0, i, z_pos;
+ int rc = 0;
- struct dpu_multirect_plane_states multirect_plane[DPU_STAGE_MAX * 2];
- int multirect_count = 0;
- const struct drm_plane_state *pipe_staged[SSPP_MAX];
- int left_zpos_cnt = 0, right_zpos_cnt = 0;
struct drm_rect crtc_rect = { 0 };
- pstates = kzalloc(sizeof(*pstates) * DPU_STAGE_MAX * 4, GFP_KERNEL);
-
if (!crtc_state->enable || !crtc_state->active) {
DRM_DEBUG_ATOMIC("crtc%d -> enable %d, active %d, skip
atomic_check\n",
crtc->base.id, crtc_state->enable,
crtc_state->active);
memset(&cstate->new_perf, 0, sizeof(cstate->new_perf));
- goto end;
+ return 0;
}
mode = &crtc_state->adjusted_mode;
@@ -1091,13 +1077,8 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
if (crtc_state->active_changed)
crtc_state->mode_changed = true;
- memset(pipe_staged, 0, sizeof(pipe_staged));
-
- if (cstate->num_mixers) {
- mixer_width = mode->hdisplay / cstate->num_mixers;
-
+ if (cstate->num_mixers)
_dpu_crtc_setup_lm_bounds(crtc, crtc_state);
- }
crtc_rect.x2 = mode->hdisplay;
crtc_rect.y2 = mode->vdisplay;
@@ -1105,33 +1086,16 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
/* get plane state for all drm planes associated with crtc state */
drm_atomic_crtc_state_for_each_plane_state(plane, pstate, crtc_state) {
struct drm_rect dst, clip = crtc_rect;
+ int z_pos;
if (IS_ERR_OR_NULL(pstate)) {
rc = PTR_ERR(pstate);
DPU_ERROR("%s: failed to get plane%d state, %d\n",
dpu_crtc->name, plane->base.id, rc);
- goto end;
- }
- if (cnt >= DPU_STAGE_MAX * 4)
- continue;
-
- pstates[cnt].dpu_pstate = to_dpu_plane_state(pstate);
- pstates[cnt].drm_pstate = pstate;
- pstates[cnt].stage = pstate->normalized_zpos;
- pstates[cnt].pipe_id =
to_dpu_plane_state(pstate)->pipe.sspp->idx;
-
- if (pipe_staged[pstates[cnt].pipe_id]) {
- multirect_plane[multirect_count].r0 =
- pipe_staged[pstates[cnt].pipe_id];
- multirect_plane[multirect_count].r1 = pstate;
- multirect_count++;
-
- pipe_staged[pstates[cnt].pipe_id] = NULL;
- } else {
- pipe_staged[pstates[cnt].pipe_id] = pstate;
+ return rc;
}
- cnt++;
+ dpu_plane_clear_multirect(pstate);
dst = drm_plane_state_dest(pstate);
if
[PATCH v2] drm/msm/dp: force link training for display resolution change
During display resolution changes display have to be disabled first
followed by display enable with new resolution. This patch force
main link always be retrained during display enable procedure to
simplify implementation instead of manually kicking of irq_hpd
handle.
Changes in v2:
-- set force_link_train flag on DP only (is_edp == false)
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/dp/dp_ctrl.c| 6 +++---
drivers/gpu/drm/msm/dp/dp_ctrl.h| 2 +-
drivers/gpu/drm/msm/dp/dp_display.c | 15 ---
3 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index af7a80c..ac226f5 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1551,7 +1551,7 @@ static int dp_ctrl_process_phy_test_request(struct
dp_ctrl_private *ctrl)
ret = dp_ctrl_on_link(&ctrl->dp_ctrl);
if (!ret)
- ret = dp_ctrl_on_stream(&ctrl->dp_ctrl);
+ ret = dp_ctrl_on_stream(&ctrl->dp_ctrl, false);
else
DRM_ERROR("failed to enable DP link controller\n");
@@ -1807,7 +1807,7 @@ static int dp_ctrl_link_retrain(struct dp_ctrl_private
*ctrl)
return dp_ctrl_setup_main_link(ctrl, &training_step);
}
-int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
+int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl, bool force_link_train)
{
int ret = 0;
bool mainlink_ready = false;
@@ -1848,7 +1848,7 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
return 0;
}
- if (!dp_ctrl_channel_eq_ok(ctrl))
+ if (force_link_tarin || !dp_ctrl_channel_eq_ok(ctrl))
dp_ctrl_link_retrain(ctrl);
/* stop txing train pattern to end link training */
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
index 0745fde..b563e2e 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
@@ -21,7 +21,7 @@ struct dp_ctrl {
};
int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl);
-int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl);
+int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl, bool force_link_train);
int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl);
int dp_ctrl_off_link(struct dp_ctrl *dp_ctrl);
int dp_ctrl_off(struct dp_ctrl *dp_ctrl);
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index c388323..370348d 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -872,7 +872,7 @@ static int dp_display_enable(struct dp_display_private *dp,
u32 data)
return 0;
}
- rc = dp_ctrl_on_stream(dp->ctrl);
+ rc = dp_ctrl_on_stream(dp->ctrl, data);
if (!rc)
dp_display->power_on = true;
@@ -1654,6 +1654,7 @@ void dp_bridge_enable(struct drm_bridge *drm_bridge)
int rc = 0;
struct dp_display_private *dp_display;
u32 state;
+ bool force_link_train = false;
dp_display = container_of(dp, struct dp_display_private, dp_display);
if (!dp_display->dp_mode.drm_mode.clock) {
@@ -1688,10 +1689,14 @@ void dp_bridge_enable(struct drm_bridge *drm_bridge)
state = dp_display->hpd_state;
- if (state == ST_DISPLAY_OFF)
+ if (state == ST_DISPLAY_OFF) {
dp_display_host_phy_init(dp_display);
- dp_display_enable(dp_display, 0);
+ if (!dp->is_edp)
+ force_link_train = true;
+ }
+
+ dp_display_enable(dp_display, force_link_train);
rc = dp_display_post_enable(dp);
if (rc) {
@@ -1700,10 +1705,6 @@ void dp_bridge_enable(struct drm_bridge *drm_bridge)
dp_display_unprepare(dp);
}
- /* manual kick off plug event to train link */
- if (state == ST_DISPLAY_OFF)
- dp_add_event(dp_display, EV_IRQ_HPD_INT, 0, 0);
-
/* completed connection */
dp_display->hpd_state = ST_CONNECTED;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: Exynos vblank timeout issue
Hello again, I tried to dig around a bit to unearth some more information. What I'm seeing is that it works just fine in the beginning, planes are updated a couple of times and suddenly, after one of the plane updates, the interrupt handler in the FIMD driver is no longer called. The screen goes dark but the device is still operational, e.g. ADB works fine, I can connect and execute commands. Trying to figure out what is called when and curious about the state of the registers, I littered the code with print statements and it looks like vsync is still active, no other code calls into disabling it. All registers are as expected, e.g. VIDINTCON0 has the interrupt bit set. I also had a look at the interrupt combiner, this too has the corresponding lcd0-1 interrupt enabled at all times and there is no interrupt pending, even after FIMD stopped receiving them. Looking at the wiki at https://exynos.wiki.kernel.org/todo_tasks I found issue #9. It's about trashed display or DMA freeze if planes are too narrow and I was wondering if this could be related. So I had a look at the drm debug output and planes are indeed getting very small. This happens exactly when the animation that is triggering the issue is playing, so this would match. Looking a bit closer at the position and size of the planes, I could see that the last working vsync was right after one of the planes was exactly 1 pixel in width and vsync only stopped working one update later. Here are the plane updates from the logs: - Planes getting smaller and smaller with each update: plane : offset_x/y(0,0), width/height(4,800) plane : offset_x/y(4,0), width/height(1276,800) plane : offset_x/y(0,0), width/height(1280,800) plane : offset_x/y(0,776), width/height(1280,24) plane : offset_x/y(0,0), width/height(2,800) plane : offset_x/y(2,0), width/height(1278,800) plane : offset_x/y(0,0), width/height(1280,800) plane : offset_x/y(0,776), width/height(1280,24) plane : offset_x/y(0,0), width/height(1,800) plane : offset_x/y(1,0), width/height(1279,800) plane : offset_x/y(0,0), width/height(1280,800) plane : offset_x/y(0,776), width/height(1280,24) Still got a vsync in between those two. But after the following update, it's dead: plane : offset_x/y(0,0), width/height(1280,800) plane : offset_x/y(0,0), width/height(1280,24) plane : offset_x/y(0,740), width/height(1280,60) plane : offset_x/y(0,0), width/height(1280,800) -> vsync timeout comes here - I have no idea how to analyze this further on the kernel side. I'll try to write an executable that triggers this bug next. If you have any ideas on that, I'd be very grateful. Kind Regards Martin On Sun, May 22, 2022 at 12:06:39PM +0200, Martin Jücker wrote: > On Sun, May 22, 2022 at 09:45:51AM +0200, Krzysztof Kozlowski wrote: > > On 22/05/2022 02:02, Martin Jücker wrote: > > > Hello, > > > > > > I'm trying to get Android 12 up and running on my Galaxy Note 10.1 which > > > is based on Exynos 4412 with a Mali GPU. For Android 11, I had no issues > > > with graphics but after upgrading and building Android 12, I'm getting a > > > vblank wait timeout shortly after starting the device setup, which in > > > turn leads to my display turning black and SurfaceFlinger hanging. This > > > can be reliably reproduced after every reboot, so much so that it's > > > basically always on the exact same step of the setup. > > > > > > I'm using the following setup: > > > > > > * 5.10.101 Android Common Kernel with some patches to get > > > the Note 10.1 up and running > > > > It's Android kernel, so not upstream. It is perfectly fine to use > > downstream kernels, but with the issues you also go to downstream folks. > > I have no clue what Android did to Exynos. > > Hi Krzysztof, > > indeed, that was my mistake. Should have done that on mainline first. > > I rebased some patches on top of v5.17.9 and tried again, same result. > There are no Android patches in there, only p4note related things. You > can have a look here: > > https://github.com/Viciouss/linux/commits/v5.17.9-android > > The behaviour is exactly the same, as soon as I try to advance in the > setup process, it suddenly turns the screen all black. > > Here is the warning again, just in case there are any differences. > > [ 77.651495] [ cut here ] > [ 77.651527] WARNING: CPU: 2 PID: 8 at > ../drivers/gpu/drm/drm_atomic_helper.c:1530 > drm_atomic_helper_wait_for_vblanks.part.1+0x2b0/0x2b4 > [ 77.651593] [CRTC:49:crtc-0] vblank wait timed out > [ 77.651608] Modules linked in: s5p_mfc s5p_jpeg v4l2_mem2mem > videobuf2_dma_contig videobuf2_memops videobuf2_v4l2 videobuf2_common > rfcomm kheaders hidp hci_uart cpufreq_userspace cpufreq_powersave > cpufreq_conservative btbcm brcmfmac brcmutil bnep bluetooth atmel_mxt_ts > [ 77.651789] CPU: 2 PID: 8 Comm: kworker/u8:0 Not tainted 5.17.9+ #3 > [ 77.651813] Hardware name: Samsung Exynos (Flattened Device Tree) > [ 77.651828] Workqueue: events_unbound commit_work > [ 77.6
[PATCH v6 00/22] Add generic memory shrinker to VirtIO-GPU and Panfrost DRM drivers
Hello, This patchset introduces memory shrinker for the VirtIO-GPU DRM driver and adds memory purging and eviction support to VirtIO-GPU driver. The new dma-buf locking convention is introduced here as well. During OOM, the shrinker will release BOs that are marked as "not needed" by userspace using the new madvise IOCTL, it will also evict idling BOs to SWAP. The userspace in this case is the Mesa VirGL driver, it will mark the cached BOs as "not needed", allowing kernel driver to release memory of the cached shmem BOs on lowmem situations, preventing OOM kills. The Panfrost driver is switched to use generic memory shrinker. This patchset includes improvements and fixes for various things that I found while was working on the shrinker. The Mesa and IGT patches will be kept on hold until this kernel series will be approved and merged. This patchset was tested using Qemu and crosvm, including both cases of IOMMU off/on. Mesa: https://gitlab.freedesktop.org/digetx/mesa/-/commits/virgl-madvise IGT: https://gitlab.freedesktop.org/digetx/igt-gpu-tools/-/commits/virtio-madvise https://gitlab.freedesktop.org/digetx/igt-gpu-tools/-/commits/panfrost-madvise Changelog: v6: - Added new VirtIO-related fix patch that previously was sent separately and didn't get much attention: drm/gem: Properly annotate WW context on drm_gem_lock_reservations() error - Added new patch that fixes mapping of imported dma-bufs for Tegra DRM and other affected drivers. It's also handy to have it for switching to the new dma-buf locking convention scheme: drm/gem: Move mapping of imported dma-bufs to drm_gem_mmap_obj() - Added new patch that fixes shrinker list corruption for stable Panfrost driver: drm/panfrost: Fix shrinker list corruption by madvise IOCTL - Added new minor patch-fix for drm-shmem: drm/shmem-helper: Add missing vunmap on error - Added fixes tag to the "Put mapping ..." patch like was suggested by Steven Price. - Added new VirtIO-GPU driver improvement patch: drm/virtio: Return proper error codes instead of -1 - Reworked shrinker patches like was suggested by Daniel Vetter: - Introduced the new locking convention for dma-bufs. Tested on VirtIO-GPU, Panfrost, Lima, Tegra and Intel selftests. - Dropped separate purge() callback. Now single evict() does everything. - Dropped swap_in() callback from drm-shmem objects. DRM drivers now could and should restore only the required mappings. - Dropped dynamic counting of evictable pages. This simplifies code in exchange to *potentially* burning more CPU time on OOM. v5: - Added new for-stable patch "drm/panfrost: Put mapping instead of shmem obj on panfrost_mmu_map_fault_addr() error" that corrects GEM's refcounting in case of error. - The drm_gem_shmem_v[un]map() now takes a separate vmap_lock for imported GEMs to avoid recursive locking of DMA reservations. This addresses v4 comment from Thomas Zimmermann about the potential deadlocking of vmapping. - Added ack from Thomas Zimmermann to "drm/shmem-helper: Correct doc-comment of drm_gem_shmem_get_sg_table()" patch. - Dropped explicit shmem states from the generic shrinker patch as was requested by Thomas Zimmermann. - Improved variable names and comments of the generic shrinker code. - Extended drm_gem_shmem_print_info() with the shrinker-state info in the "drm/virtio: Support memory shrinking" patch. - Moved evict()/swap_in()/purge() callbacks from drm_gem_object_funcs to drm_gem_shmem_object in the generic shrinker patch, for more consistency. - Corrected bisectability of the patches that was broken in v4 by accident. - The virtio_gpu_plane_prepare_fb() now uses drm_gem_shmem_pin() instead of drm_gem_shmem_set_unpurgeable_and_unevictable() and does it only for shmem BOs in the "drm/virtio: Support memory shrinking" patch. - Made more functions private to drm_gem_shmem_helper.c as was requested by Thomas Zimmermann. This minimizes number of the public shmem helpers. v4: - Corrected minor W=1 warnings reported by kernel test robot for v3. - Renamed DRM_GEM_SHMEM_PAGES_STATE_ACTIVE/INACTIVE to PINNED/UNPINNED, for more clarity. v3: - Hardened shrinker's count() with usage of READ_ONCE() since we don't use atomic type for counting and technically compiler is free to re-fetch counter's variable. - "Correct drm_gem_shmem_get_sg_table() error handling" now uses PTR_ERR_OR_ZERO(), fixing typo that was made in v2. - Removed obsoleted shrinker from the Panfrost driver, which I missed to do in v2 by accident and Alyssa Rosenzweig managed to notice it. - CCed stable kernels in all patches that make fixes, even the minor ones, like was suggested by Emil Veliko
[PATCH v6 01/22] drm/gem: Properly annotate WW context on drm_gem_lock_reservations() error
Use ww_acquire_fini() in the error code paths. Otherwise lockdep
thinks that lock is held when lock's memory is freed after the
drm_gem_lock_reservations() error. The WW needs to be annotated
as "freed", which fixes the noisy "WARNING: held lock freed!" splat
of VirtIO-GPU driver with CONFIG_DEBUG_MUTEXES=y and enabled lockdep.
Cc: sta...@vger.kernel.org
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index eb0c2d041f13..86d670c71286 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1226,7 +1226,7 @@ drm_gem_lock_reservations(struct drm_gem_object **objs,
int count,
ret = dma_resv_lock_slow_interruptible(obj->resv,
acquire_ctx);
if (ret) {
- ww_acquire_done(acquire_ctx);
+ ww_acquire_fini(acquire_ctx);
return ret;
}
}
@@ -1251,7 +1251,7 @@ drm_gem_lock_reservations(struct drm_gem_object **objs,
int count,
goto retry;
}
- ww_acquire_done(acquire_ctx);
+ ww_acquire_fini(acquire_ctx);
return ret;
}
}
--
2.35.3
[PATCH v6 02/22] drm/gem: Move mapping of imported dma-bufs to drm_gem_mmap_obj()
Drivers that use drm_gem_mmap() and drm_gem_mmap_obj() helpers don't
handle imported dma-bufs properly, which results in mapping of something
else than the imported dma-buf. For example, on NVIDIA Tegra we get a hard
lockup when userspace writes to the memory mapping of a dma-buf that was
imported into Tegra's DRM GEM.
To fix this bug, move mapping of imported dma-bufs to drm_gem_mmap_obj().
Now mmaping of imported dma-bufs works properly for all DRM drivers.
Cc: sta...@vger.kernel.org
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem.c | 3 +++
drivers/gpu/drm/drm_gem_shmem_helper.c | 9 -
drivers/gpu/drm/tegra/gem.c| 4
3 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 86d670c71286..7c0b025508e4 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1038,6 +1038,9 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned
long obj_size,
if (obj_size < vma->vm_end - vma->vm_start)
return -EINVAL;
+ if (obj->import_attach)
+ return dma_buf_mmap(obj->dma_buf, vma, 0);
+
/* Take a ref for this mapping of the object, so that the fault
* handler can dereference the mmap offset's pointer to the object.
* This reference is cleaned up by the corresponding vm_close
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 8ad0e02991ca..6190f5018986 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -609,17 +609,8 @@ EXPORT_SYMBOL_GPL(drm_gem_shmem_vm_ops);
*/
int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct
vm_area_struct *vma)
{
- struct drm_gem_object *obj = &shmem->base;
int ret;
- if (obj->import_attach) {
- /* Drop the reference drm_gem_mmap_obj() acquired.*/
- drm_gem_object_put(obj);
- vma->vm_private_data = NULL;
-
- return dma_buf_mmap(obj->dma_buf, vma, 0);
- }
-
ret = drm_gem_shmem_get_pages(shmem);
if (ret) {
drm_gem_vm_close(vma);
diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index 7c7dd84e6db8..f92aa20d63bb 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -564,6 +564,10 @@ int __tegra_gem_mmap(struct drm_gem_object *gem, struct
vm_area_struct *vma)
{
struct tegra_bo *bo = to_tegra_bo(gem);
+ /* imported dmu-buf is mapped by drm_gem_mmap_obj() */
+ if (gem->import_attach)
+ return 0;
+
if (!bo->pages) {
unsigned long vm_pgoff = vma->vm_pgoff;
int err;
--
2.35.3
[PATCH v6 04/22] drm/panfrost: Fix shrinker list corruption by madvise IOCTL
Calling madvise IOCTL twice on BO causes memory shrinker list corruption
and crashes kernel because BO is already on the list and it's added to
the list again, while BO should be removed from from the list before it's
re-added. Fix it.
Cc: sta...@vger.kernel.org
Fixes: 013b65101315 ("drm/panfrost: Add madvise and shrinker support")
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/panfrost/panfrost_drv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 087e69b98d06..b1e6d238674f 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -433,8 +433,8 @@ static int panfrost_ioctl_madvise(struct drm_device *dev,
void *data,
if (args->retained) {
if (args->madv == PANFROST_MADV_DONTNEED)
- list_add_tail(&bo->base.madv_list,
- &pfdev->shrinker_list);
+ list_move_tail(&bo->base.madv_list,
+ &pfdev->shrinker_list);
else if (args->madv == PANFROST_MADV_WILLNEED)
list_del_init(&bo->base.madv_list);
}
--
2.35.3
[PATCH v6 03/22] drm/panfrost: Put mapping instead of shmem obj on panfrost_mmu_map_fault_addr() error
When panfrost_mmu_map_fault_addr() fails, the BO's mapping should be
unreferenced and not the shmem object which backs the mapping.
Cc: sta...@vger.kernel.org
Fixes: bdefca2d8dc0 ("drm/panfrost: Add the panfrost_gem_mapping concept")
Reviewed-by: Steven Price
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/panfrost/panfrost_mmu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c
b/drivers/gpu/drm/panfrost/panfrost_mmu.c
index d3f82b26a631..b285a8001b1d 100644
--- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
@@ -518,7 +518,7 @@ static int panfrost_mmu_map_fault_addr(struct
panfrost_device *pfdev, int as,
err_pages:
drm_gem_shmem_put_pages(&bo->base);
err_bo:
- drm_gem_object_put(&bo->base.base);
+ panfrost_gem_mapping_put(bomapping);
return ret;
}
--
2.35.3
[PATCH v6 06/22] drm/virtio: Check whether transferred 2D BO is shmem
Transferred 2D BO always must be a shmem BO. Add check for that to prevent NULL dereference if userspace passes a VRAM BO. Cc: sta...@vger.kernel.org Reviewed-by: Emil Velikov Signed-off-by: Dmitry Osipenko --- drivers/gpu/drm/virtio/virtgpu_vq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index 7c052efe8836..2edf31806b74 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -595,7 +595,7 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev, bool use_dma_api = !virtio_has_dma_quirk(vgdev->vdev); struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo); - if (use_dma_api) + if (virtio_gpu_is_shmem(bo) && use_dma_api) dma_sync_sgtable_for_device(vgdev->vdev->dev.parent, shmem->pages, DMA_TO_DEVICE); -- 2.35.3
[PATCH v6 07/22] drm/virtio: Unlock reservations on virtio_gpu_object_shmem_init() error
Unlock reservations in the error code path of virtio_gpu_object_create()
to silence debug warning splat produced by ww_mutex_destroy(&obj->lock)
when GEM is released with the held lock.
Cc: sta...@vger.kernel.org
Reviewed-by: Emil Velikov
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/virtio/virtgpu_object.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c
b/drivers/gpu/drm/virtio/virtgpu_object.c
index 3d0c8d4d1c20..21c19cdedce0 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -250,6 +250,8 @@ int virtio_gpu_object_create(struct virtio_gpu_device
*vgdev,
ret = virtio_gpu_object_shmem_init(vgdev, bo, &ents, &nents);
if (ret != 0) {
+ if (fence)
+ virtio_gpu_array_unlock_resv(objs);
virtio_gpu_array_put_free(objs);
virtio_gpu_free_object(&shmem_obj->base);
return ret;
--
2.35.3
[PATCH v6 09/22] drm/virtio: Use appropriate atomic state in virtio_gpu_plane_cleanup_fb()
Make virtio_gpu_plane_cleanup_fb() to clean the state which DRM core
wants to clean up and not the current plane's state. Normally the older
atomic state is cleaned up, but the newer state could also be cleaned up
in case of aborted commits.
Cc: sta...@vger.kernel.org
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/virtio/virtgpu_plane.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c
b/drivers/gpu/drm/virtio/virtgpu_plane.c
index 6d3cc9e238a4..7148f3813d8b 100644
--- a/drivers/gpu/drm/virtio/virtgpu_plane.c
+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
@@ -266,14 +266,14 @@ static int virtio_gpu_plane_prepare_fb(struct drm_plane
*plane,
}
static void virtio_gpu_plane_cleanup_fb(struct drm_plane *plane,
- struct drm_plane_state *old_state)
+ struct drm_plane_state *state)
{
struct virtio_gpu_framebuffer *vgfb;
- if (!plane->state->fb)
+ if (!state->fb)
return;
- vgfb = to_virtio_gpu_framebuffer(plane->state->fb);
+ vgfb = to_virtio_gpu_framebuffer(state->fb);
if (vgfb->fence) {
dma_fence_put(&vgfb->fence->f);
vgfb->fence = NULL;
--
2.35.3
[PATCH v6 11/22] drm/shmem-helper: Correct doc-comment of drm_gem_shmem_get_sg_table()
drm_gem_shmem_get_sg_table() never returns NULL on error, but a ERR_PTR.
Correct the doc comment which says that it returns NULL on error.
Acked-by: Thomas Zimmermann
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 54b0ba28aa0a..7232e321fdb4 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -654,7 +654,8 @@ EXPORT_SYMBOL(drm_gem_shmem_print_info);
* drm_gem_shmem_get_pages_sgt() instead.
*
* Returns:
- * A pointer to the scatter/gather table of pinned pages or NULL on failure.
+ * A pointer to the scatter/gather table of pinned pages or an
ERR_PTR()-encoded
+ * error code on failure.
*/
struct sg_table *drm_gem_shmem_get_sg_table(struct drm_gem_shmem_object *shmem)
{
@@ -680,7 +681,8 @@ EXPORT_SYMBOL_GPL(drm_gem_shmem_get_sg_table);
* drm_gem_shmem_get_sg_table() should not be directly called by drivers.
*
* Returns:
- * A pointer to the scatter/gather table of pinned pages or errno on failure.
+ * A pointer to the scatter/gather table of pinned pages or an
ERR_PTR()-encoded
+ * error code on failure.
*/
struct sg_table *drm_gem_shmem_get_pages_sgt(struct drm_gem_shmem_object
*shmem)
{
--
2.35.3
[PATCH v6 10/22] drm/shmem-helper: Add missing vunmap on error
The vmapping of dma-buf may succeed, but DRM SHMEM rejects the iomem
mappings, and thus, drm_gem_shmem_vmap_locked() should unvmap the iomem
before erroring out.
Cc: sta...@vger.kernel.org
Fixes: 49a3f51dfeee ("drm/gem: Use struct dma_buf_map in GEM vmap ops and
convert GEM backends")
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 6190f5018986..54b0ba28aa0a 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -302,6 +302,7 @@ static int drm_gem_shmem_vmap_locked(struct
drm_gem_shmem_object *shmem,
ret = dma_buf_vmap(obj->import_attach->dmabuf, map);
if (!ret) {
if (WARN_ON(map->is_iomem)) {
+ dma_buf_vunmap(obj->import_attach->dmabuf, map);
ret = -EIO;
goto err_put_pages;
}
--
2.35.3
[PATCH v6 08/22] drm/virtio: Unlock reservations on dma_resv_reserve_fences() error
Unlock reservations on dma_resv_reserve_fences() error to fix recursive
locking of the reservations when this error happens.
Cc: sta...@vger.kernel.org
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/virtio/virtgpu_gem.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c
b/drivers/gpu/drm/virtio/virtgpu_gem.c
index 580a78809836..7db48d17ee3a 100644
--- a/drivers/gpu/drm/virtio/virtgpu_gem.c
+++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
@@ -228,8 +228,10 @@ int virtio_gpu_array_lock_resv(struct
virtio_gpu_object_array *objs)
for (i = 0; i < objs->nents; ++i) {
ret = dma_resv_reserve_fences(objs->objs[i]->resv, 1);
- if (ret)
+ if (ret) {
+ virtio_gpu_array_unlock_resv(objs);
return ret;
+ }
}
return ret;
}
--
2.35.3
[PATCH v6 05/22] drm/virtio: Correct drm_gem_shmem_get_sg_table() error handling
drm_gem_shmem_get_sg_table() never ever returned NULL on error. Correct
the error handling to avoid crash on OOM.
Cc: sta...@vger.kernel.org
Reviewed-by: Emil Velikov
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/virtio/virtgpu_object.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c
b/drivers/gpu/drm/virtio/virtgpu_object.c
index f293e6ad52da..3d0c8d4d1c20 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -168,9 +168,11 @@ static int virtio_gpu_object_shmem_init(struct
virtio_gpu_device *vgdev,
* since virtio_gpu doesn't support dma-buf import from other devices.
*/
shmem->pages = drm_gem_shmem_get_sg_table(&bo->base);
- if (!shmem->pages) {
+ ret = PTR_ERR_OR_ZERO(shmem->pages);
+ if (ret) {
drm_gem_shmem_unpin(&bo->base);
- return -EINVAL;
+ shmem->pages = NULL;
+ return ret;
}
if (use_dma_api) {
--
2.35.3
[PATCH v6 13/22] drm/virtio: Improve DMA API usage for shmem BOs
DRM API requires the DRM's driver to be backed with the device that can
be used for generic DMA operations. The VirtIO-GPU device can't perform
DMA operations if it uses PCI transport because PCI device driver creates
a virtual VirtIO-GPU device that isn't associated with the PCI. Use PCI's
GPU device for the DRM's device instead of the VirtIO-GPU device and drop
DMA-related hacks from the VirtIO-GPU driver.
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/virtio/virtgpu_drv.c| 51 ++
drivers/gpu/drm/virtio/virtgpu_drv.h| 5 +--
drivers/gpu/drm/virtio/virtgpu_kms.c| 7 ++--
drivers/gpu/drm/virtio/virtgpu_object.c | 56 +
drivers/gpu/drm/virtio/virtgpu_vq.c | 13 +++---
5 files changed, 32 insertions(+), 100 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c
b/drivers/gpu/drm/virtio/virtgpu_drv.c
index 5f25a8d15464..0141b7df97ec 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -46,12 +46,11 @@ static int virtio_gpu_modeset = -1;
MODULE_PARM_DESC(modeset, "Disable/Enable modesetting");
module_param_named(modeset, virtio_gpu_modeset, int, 0400);
-static int virtio_gpu_pci_quirk(struct drm_device *dev, struct virtio_device
*vdev)
+static int virtio_gpu_pci_quirk(struct drm_device *dev)
{
- struct pci_dev *pdev = to_pci_dev(vdev->dev.parent);
+ struct pci_dev *pdev = to_pci_dev(dev->dev);
const char *pname = dev_name(&pdev->dev);
bool vga = (pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA;
- char unique[20];
int ret;
DRM_INFO("pci: %s detected at %s\n",
@@ -63,39 +62,7 @@ static int virtio_gpu_pci_quirk(struct drm_device *dev,
struct virtio_device *vd
return ret;
}
- /*
-* Normally the drm_dev_set_unique() call is done by core DRM.
-* The following comment covers, why virtio cannot rely on it.
-*
-* Unlike the other virtual GPU drivers, virtio abstracts the
-* underlying bus type by using struct virtio_device.
-*
-* Hence the dev_is_pci() check, used in core DRM, will fail
-* and the unique returned will be the virtio_device "virtio0",
-* while a "pci:..." one is required.
-*
-* A few other ideas were considered:
-* - Extend the dev_is_pci() check [in drm_set_busid] to
-* consider virtio.
-* Seems like a bigger hack than what we have already.
-*
-* - Point drm_device::dev to the parent of the virtio_device
-* Semantic changes:
-* * Using the wrong device for i2c, framebuffer_alloc and
-* prime import.
-* Visual changes:
-* * Helpers such as DRM_DEV_ERROR, dev_info, drm_printer,
-* will print the wrong information.
-*
-* We could address the latter issues, by introducing
-* drm_device::bus_dev, ... which would be used solely for this.
-*
-* So for the moment keep things as-is, with a bulky comment
-* for the next person who feels like removing this
-* drm_dev_set_unique() quirk.
-*/
- snprintf(unique, sizeof(unique), "pci:%s", pname);
- return drm_dev_set_unique(dev, unique);
+ return 0;
}
static int virtio_gpu_probe(struct virtio_device *vdev)
@@ -109,18 +76,24 @@ static int virtio_gpu_probe(struct virtio_device *vdev)
if (virtio_gpu_modeset == 0)
return -EINVAL;
- dev = drm_dev_alloc(&driver, &vdev->dev);
+ /*
+* The virtio-gpu device is a virtual device that doesn't have DMA
+* ops assigned to it, nor DMA mask set and etc. Its parent device
+* is actual GPU device we want to use it for the DRM's device in
+* order to benefit from using generic DRM APIs.
+*/
+ dev = drm_dev_alloc(&driver, vdev->dev.parent);
if (IS_ERR(dev))
return PTR_ERR(dev);
vdev->priv = dev;
if (!strcmp(vdev->dev.parent->bus->name, "pci")) {
- ret = virtio_gpu_pci_quirk(dev, vdev);
+ ret = virtio_gpu_pci_quirk(dev);
if (ret)
goto err_free;
}
- ret = virtio_gpu_init(dev);
+ ret = virtio_gpu_init(vdev, dev);
if (ret)
goto err_free;
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 0a194aaad419..b2d93cb12ebf 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -100,8 +100,6 @@ struct virtio_gpu_object {
struct virtio_gpu_object_shmem {
struct virtio_gpu_object base;
- struct sg_table *pages;
- uint32_t mapped;
};
struct virtio_gpu_object_vram {
@@ -214,7 +212,6 @@ struct virtio_gpu_drv_cap_cache {
};
struct virtio_gpu_device {
- struct device *dev;
struct drm_device *ddev;
[PATCH v6 14/22] dma-buf: Introduce new locking convention
All dma-bufs have dma-reservation lock that allows drivers to perform
exclusive operations over shared dma-bufs. Today's dma-buf API has
incomplete locking specification, which creates dead lock situation
for dma-buf importers and exporters that don't coordinate theirs locks.
This patch introduces new locking convention for dma-buf users. From now
on all dma-buf importers are responsible for holding dma-buf reservation
lock around operations performed over dma-bufs.
This patch implements the new dma-buf locking convention by:
1. Making dma-buf API functions to take the reservation lock.
2. Adding new locked variants of the dma-buf API functions for drivers
that need to manage imported dma-bufs under the held lock.
3. Converting all drivers to the new locking scheme.
Signed-off-by: Dmitry Osipenko
---
drivers/dma-buf/dma-buf.c | 270 +++---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 6 +-
drivers/gpu/drm/drm_client.c | 4 +-
drivers/gpu/drm/drm_gem.c | 33 +++
drivers/gpu/drm/drm_gem_framebuffer_helper.c | 6 +-
drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c| 10 +-
drivers/gpu/drm/qxl/qxl_object.c | 17 +-
drivers/gpu/drm/qxl/qxl_prime.c | 4 +-
.../common/videobuf2/videobuf2-dma-contig.c | 11 +-
.../media/common/videobuf2/videobuf2-dma-sg.c | 11 +-
.../common/videobuf2/videobuf2-vmalloc.c | 11 +-
include/drm/drm_gem.h | 3 +
include/linux/dma-buf.h | 14 +-
13 files changed, 241 insertions(+), 159 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 32f55640890c..64a9909ccfa2 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -552,7 +552,6 @@ struct dma_buf *dma_buf_export(const struct
dma_buf_export_info *exp_info)
file->f_mode |= FMODE_LSEEK;
dmabuf->file = file;
- mutex_init(&dmabuf->lock);
INIT_LIST_HEAD(&dmabuf->attachments);
mutex_lock(&db_list.lock);
@@ -737,14 +736,14 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct
device *dev,
attach->importer_ops = importer_ops;
attach->importer_priv = importer_priv;
+ dma_resv_lock(dmabuf->resv, NULL);
+
if (dmabuf->ops->attach) {
ret = dmabuf->ops->attach(dmabuf, attach);
if (ret)
goto err_attach;
}
- dma_resv_lock(dmabuf->resv, NULL);
list_add(&attach->node, &dmabuf->attachments);
- dma_resv_unlock(dmabuf->resv);
/* When either the importer or the exporter can't handle dynamic
* mappings we cache the mapping here to avoid issues with the
@@ -755,7 +754,6 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct
device *dev,
struct sg_table *sgt;
if (dma_buf_is_dynamic(attach->dmabuf)) {
- dma_resv_lock(attach->dmabuf->resv, NULL);
ret = dmabuf->ops->pin(attach);
if (ret)
goto err_unlock;
@@ -768,15 +766,16 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct
device *dev,
ret = PTR_ERR(sgt);
goto err_unpin;
}
- if (dma_buf_is_dynamic(attach->dmabuf))
- dma_resv_unlock(attach->dmabuf->resv);
attach->sgt = sgt;
attach->dir = DMA_BIDIRECTIONAL;
}
+ dma_resv_unlock(dmabuf->resv);
+
return attach;
err_attach:
+ dma_resv_unlock(attach->dmabuf->resv);
kfree(attach);
return ERR_PTR(ret);
@@ -785,10 +784,10 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct
device *dev,
dmabuf->ops->unpin(attach);
err_unlock:
- if (dma_buf_is_dynamic(attach->dmabuf))
- dma_resv_unlock(attach->dmabuf->resv);
+ dma_resv_unlock(dmabuf->resv);
dma_buf_detach(dmabuf, attach);
+
return ERR_PTR(ret);
}
EXPORT_SYMBOL_NS_GPL(dma_buf_dynamic_attach, DMA_BUF);
@@ -832,24 +831,23 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct
dma_buf_attachment *attach)
if (WARN_ON(!dmabuf || !attach))
return;
- if (attach->sgt) {
- if (dma_buf_is_dynamic(attach->dmabuf))
- dma_resv_lock(attach->dmabuf->resv, NULL);
+ if (WARN_ON(dmabuf != attach->dmabuf))
+ return;
+ dma_resv_lock(dmabuf->resv, NULL);
+
+ if (attach->sgt) {
__unmap_dma_buf(attach, attach->sgt, attach->dir);
- if (dma_buf_is_dynamic(attach->dmabuf)) {
+ if (dma_buf_is_dynamic(attach->dmabuf))
dmabuf->ops->unpin(attach);
- dma_resv_unlock(attach->dmabuf->resv);
- }
}
- dma_resv_lock(dmabuf->resv, NULL)
[PATCH v6 12/22] drm/virtio: Simplify error handling of virtio_gpu_object_create()
Change the order of SHMEM initialization and reservation locking
to make code cleaner and to prepare for transitioning of the common
GEM SHMEM code to use the GEM's reservation lock instead of the
shmem.page_lock.
There is no need to lock reservation during allocation of the SHMEM pages
because the lock is needed only to avoid racing with the async host-side
allocation. Hence we can safely move the SHMEM initialization out of the
reservation lock.
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/virtio/virtgpu_object.c | 13 -
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c
b/drivers/gpu/drm/virtio/virtgpu_object.c
index 21c19cdedce0..18f70ef6b4d0 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -236,6 +236,10 @@ int virtio_gpu_object_create(struct virtio_gpu_device
*vgdev,
bo->dumb = params->dumb;
+ ret = virtio_gpu_object_shmem_init(vgdev, bo, &ents, &nents);
+ if (ret != 0)
+ goto err_put_id;
+
if (fence) {
ret = -ENOMEM;
objs = virtio_gpu_array_alloc(1);
@@ -248,15 +252,6 @@ int virtio_gpu_object_create(struct virtio_gpu_device
*vgdev,
goto err_put_objs;
}
- ret = virtio_gpu_object_shmem_init(vgdev, bo, &ents, &nents);
- if (ret != 0) {
- if (fence)
- virtio_gpu_array_unlock_resv(objs);
- virtio_gpu_array_put_free(objs);
- virtio_gpu_free_object(&shmem_obj->base);
- return ret;
- }
-
if (params->blob) {
if (params->blob_mem == VIRTGPU_BLOB_MEM_GUEST)
bo->guest_blob = true;
--
2.35.3
[PATCH v6 15/22] drm/shmem-helper: Don't use vmap_use_count for dma-bufs
There is no need to refcount vmappings of dma-bufs because dma-buf core
has its own refcounting. Drop the refcounting of dma-bufs. This will ease
replacing of all drm-shmem locks with a single dma-buf reservation lock,
preparing drm-shmem code for addition of the generic drm-shmem shrinker.
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 35 +++---
1 file changed, 20 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 7232e321fdb4..fd2647690bf7 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -293,24 +293,22 @@ static int drm_gem_shmem_vmap_locked(struct
drm_gem_shmem_object *shmem,
struct drm_gem_object *obj = &shmem->base;
int ret = 0;
- if (shmem->vmap_use_count++ > 0) {
- iosys_map_set_vaddr(map, shmem->vaddr);
- return 0;
- }
-
if (obj->import_attach) {
ret = dma_buf_vmap(obj->import_attach->dmabuf, map);
if (!ret) {
if (WARN_ON(map->is_iomem)) {
dma_buf_vunmap(obj->import_attach->dmabuf, map);
- ret = -EIO;
- goto err_put_pages;
+ return -EIO;
}
- shmem->vaddr = map->vaddr;
}
} else {
pgprot_t prot = PAGE_KERNEL;
+ if (shmem->vmap_use_count++ > 0) {
+ iosys_map_set_vaddr(map, shmem->vaddr);
+ return 0;
+ }
+
ret = drm_gem_shmem_get_pages(shmem);
if (ret)
goto err_zero_use;
@@ -376,15 +374,15 @@ static void drm_gem_shmem_vunmap_locked(struct
drm_gem_shmem_object *shmem,
{
struct drm_gem_object *obj = &shmem->base;
- if (WARN_ON_ONCE(!shmem->vmap_use_count))
- return;
-
- if (--shmem->vmap_use_count > 0)
- return;
-
if (obj->import_attach) {
dma_buf_vunmap(obj->import_attach->dmabuf, map);
} else {
+ if (WARN_ON_ONCE(!shmem->vmap_use_count))
+ return;
+
+ if (--shmem->vmap_use_count > 0)
+ return;
+
vunmap(shmem->vaddr);
drm_gem_shmem_put_pages(shmem);
}
@@ -637,7 +635,14 @@ void drm_gem_shmem_print_info(const struct
drm_gem_shmem_object *shmem,
struct drm_printer *p, unsigned int indent)
{
drm_printf_indent(p, indent, "pages_use_count=%u\n",
shmem->pages_use_count);
- drm_printf_indent(p, indent, "vmap_use_count=%u\n",
shmem->vmap_use_count);
+
+ if (shmem->base.import_attach)
+ drm_printf_indent(p, indent, "vmap_use_count=%u\n",
+ shmem->base.dma_buf->vmapping_counter);
+ else
+ drm_printf_indent(p, indent, "vmap_use_count=%u\n",
+ shmem->vmap_use_count);
+
drm_printf_indent(p, indent, "vaddr=%p\n", shmem->vaddr);
}
EXPORT_SYMBOL(drm_gem_shmem_print_info);
--
2.35.3
[PATCH v6 17/22] drm/shmem-helper: Add generic memory shrinker
Introduce a common DRM SHMEM shrinker framework that allows to reduce
code duplication among DRM drivers by replacing theirs custom shrinker
implementations with the generic shrinker.
In order to start using DRM SHMEM shrinker drivers should:
1. Implement new evict() shmem object callback.
2. Register shrinker using drm_gem_shmem_shrinker_register(drm_device).
3. Use drm_gem_shmem_set_purgeable(shmem) and alike API functions to
activate shrinking of shmem GEMs.
This patch is based on a ideas borrowed from Rob's Clark MSM shrinker,
Thomas' Zimmermann variant of SHMEM shrinker and Intel's i915 shrinker.
Signed-off-by: Daniel Almeida
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c| 540 --
.../gpu/drm/panfrost/panfrost_gem_shrinker.c | 9 +-
drivers/gpu/drm/virtio/virtgpu_drv.h | 3 +
include/drm/drm_device.h | 4 +
include/drm/drm_gem_shmem_helper.h| 87 ++-
5 files changed, 594 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 555fe212bd98..4cd0b5913492 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -126,6 +126,42 @@ struct drm_gem_shmem_object *drm_gem_shmem_create(struct
drm_device *dev, size_t
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_create);
+static bool drm_gem_shmem_is_evictable(struct drm_gem_shmem_object *shmem)
+{
+ return (shmem->madv >= 0) && shmem->evict &&
+ shmem->eviction_enabled && shmem->pages_use_count &&
+ !shmem->pages_pin_count && !shmem->base.dma_buf &&
+ !shmem->base.import_attach && shmem->sgt && !shmem->evicted;
+}
+
+static void
+drm_gem_shmem_update_pages_state(struct drm_gem_shmem_object *shmem)
+{
+ struct drm_gem_object *obj = &shmem->base;
+ struct drm_gem_shmem_shrinker *gem_shrinker = obj->dev->shmem_shrinker;
+
+ dma_resv_assert_held(shmem->base.resv);
+
+ if (!gem_shrinker || obj->import_attach)
+ return;
+
+ mutex_lock(&gem_shrinker->lock);
+
+ if (drm_gem_shmem_is_evictable(shmem) ||
+ drm_gem_shmem_is_purgeable(shmem))
+ list_move_tail(&shmem->madv_list, &gem_shrinker->lru_evictable);
+ else if (shmem->madv < 0)
+ list_del_init(&shmem->madv_list);
+ else if (shmem->evicted)
+ list_move_tail(&shmem->madv_list, &gem_shrinker->lru_evicted);
+ else if (!shmem->pages)
+ list_del_init(&shmem->madv_list);
+ else
+ list_move_tail(&shmem->madv_list, &gem_shrinker->lru_pinned);
+
+ mutex_unlock(&gem_shrinker->lock);
+}
+
/**
* drm_gem_shmem_free - Free resources associated with a shmem GEM object
* @shmem: shmem GEM object to free
@@ -142,6 +178,9 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem)
} else {
dma_resv_lock(shmem->base.resv, NULL);
+ /* take out shmem GEM object from the memory shrinker */
+ drm_gem_shmem_madvise(shmem, -1);
+
WARN_ON(shmem->vmap_use_count);
if (shmem->sgt) {
@@ -150,7 +189,7 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem)
sg_free_table(shmem->sgt);
kfree(shmem->sgt);
}
- if (shmem->pages)
+ if (shmem->pages_use_count)
drm_gem_shmem_put_pages(shmem);
WARN_ON(shmem->pages_use_count);
@@ -163,18 +202,82 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object
*shmem)
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_free);
-static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem)
+/**
+ * drm_gem_shmem_set_evictable() - Make GEM evictable by memory shrinker
+ * @shmem: shmem GEM object
+ *
+ * Tell memory shrinker that this GEM can be evicted. Initially eviction is
+ * disabled for all GEMs. If GEM was purged, then -ENOMEM is returned.
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ */
+int drm_gem_shmem_set_evictable(struct drm_gem_shmem_object *shmem)
+{
+ dma_resv_lock(shmem->base.resv, NULL);
+
+ if (shmem->madv < 0)
+ return -ENOMEM;
+
+ shmem->eviction_enabled = true;
+
+ dma_resv_unlock(shmem->base.resv);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(drm_gem_shmem_set_evictable);
+
+/**
+ * drm_gem_shmem_set_purgeable() - Make GEM purgeable by memory shrinker
+ * @shmem: shmem GEM object
+ *
+ * Tell memory shrinker that this GEM can be purged. Initially purging is
+ * disabled for all GEMs. If GEM was purged, then -ENOMEM is returned.
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ */
+int drm_gem_shmem_set_purgeable(struct drm_gem_shmem_object *shmem)
+{
+ dma_resv_lock(shmem->base.resv, NULL);
+
+ if (shmem->madv < 0)
+ return -ENOMEM;
+
+
[PATCH v6 16/22] drm/shmem-helper: Use reservation lock
Replace drm_gem_shmem locks with GEM reservation lock to make drm-shmem
locks consistent with the new locking convention of dma-bufs which tells
that dma-buf importers are responsible for holding reservation lock for
all operations performed over dma-bufs. This prepares drm-shmem code for
addition of the generic shmem shrinker framework.
Suggested-by: Daniel Vetter
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c| 181 +++---
drivers/gpu/drm/lima/lima_gem.c | 8 +-
drivers/gpu/drm/lima/lima_sched.c | 4 +-
drivers/gpu/drm/panfrost/panfrost_drv.c | 7 +-
.../gpu/drm/panfrost/panfrost_gem_shrinker.c | 6 +-
drivers/gpu/drm/panfrost/panfrost_mmu.c | 19 +-
drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 6 +-
include/drm/drm_gem_shmem_helper.h| 14 +-
8 files changed, 97 insertions(+), 148 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index fd2647690bf7..555fe212bd98 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -86,8 +86,6 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t size,
bool private)
if (ret)
goto err_release;
- mutex_init(&shmem->pages_lock);
- mutex_init(&shmem->vmap_lock);
INIT_LIST_HEAD(&shmem->madv_list);
if (!private) {
@@ -139,11 +137,13 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object
*shmem)
{
struct drm_gem_object *obj = &shmem->base;
- WARN_ON(shmem->vmap_use_count);
-
if (obj->import_attach) {
drm_prime_gem_destroy(obj, shmem->sgt);
} else {
+ dma_resv_lock(shmem->base.resv, NULL);
+
+ WARN_ON(shmem->vmap_use_count);
+
if (shmem->sgt) {
dma_unmap_sgtable(obj->dev->dev, shmem->sgt,
DMA_BIDIRECTIONAL, 0);
@@ -152,18 +152,18 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object
*shmem)
}
if (shmem->pages)
drm_gem_shmem_put_pages(shmem);
- }
- WARN_ON(shmem->pages_use_count);
+ WARN_ON(shmem->pages_use_count);
+
+ dma_resv_unlock(shmem->base.resv);
+ }
drm_gem_object_release(obj);
- mutex_destroy(&shmem->pages_lock);
- mutex_destroy(&shmem->vmap_lock);
kfree(shmem);
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_free);
-static int drm_gem_shmem_get_pages_locked(struct drm_gem_shmem_object *shmem)
+static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem)
{
struct drm_gem_object *obj = &shmem->base;
struct page **pages;
@@ -194,35 +194,17 @@ static int drm_gem_shmem_get_pages_locked(struct
drm_gem_shmem_object *shmem)
}
/*
- * drm_gem_shmem_get_pages - Allocate backing pages for a shmem GEM object
+ * drm_gem_shmem_put_pages - Decrease use count on the backing pages for a
shmem GEM object
* @shmem: shmem GEM object
*
- * This function makes sure that backing pages exists for the shmem GEM object
- * and increases the use count.
- *
- * Returns:
- * 0 on success or a negative error code on failure.
+ * This function decreases the use count and puts the backing pages when use
drops to zero.
*/
-int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem)
-{
- int ret;
-
- WARN_ON(shmem->base.import_attach);
-
- ret = mutex_lock_interruptible(&shmem->pages_lock);
- if (ret)
- return ret;
- ret = drm_gem_shmem_get_pages_locked(shmem);
- mutex_unlock(&shmem->pages_lock);
-
- return ret;
-}
-EXPORT_SYMBOL(drm_gem_shmem_get_pages);
-
-static void drm_gem_shmem_put_pages_locked(struct drm_gem_shmem_object *shmem)
+void drm_gem_shmem_put_pages(struct drm_gem_shmem_object *shmem)
{
struct drm_gem_object *obj = &shmem->base;
+ dma_resv_assert_held(shmem->base.resv);
+
if (WARN_ON_ONCE(!shmem->pages_use_count))
return;
@@ -239,19 +221,6 @@ static void drm_gem_shmem_put_pages_locked(struct
drm_gem_shmem_object *shmem)
shmem->pages_mark_accessed_on_put);
shmem->pages = NULL;
}
-
-/*
- * drm_gem_shmem_put_pages - Decrease use count on the backing pages for a
shmem GEM object
- * @shmem: shmem GEM object
- *
- * This function decreases the use count and puts the backing pages when use
drops to zero.
- */
-void drm_gem_shmem_put_pages(struct drm_gem_shmem_object *shmem)
-{
- mutex_lock(&shmem->pages_lock);
- drm_gem_shmem_put_pages_locked(shmem);
- mutex_unlock(&shmem->pages_lock);
-}
EXPORT_SYMBOL(drm_gem_shmem_put_pages);
/**
@@ -266,6 +235,8 @@ EXPORT_SYMBOL(drm_gem_shmem_put_pages);
*/
int drm_gem_shmem_pin(struct drm_gem_shmem_object *shmem)
{
+ dma_resv_assert_held(shmem->base.resv);
+
WARN_ON(shmem->base.import
[PATCH v6 18/22] drm/gem: Add drm_gem_pin_unlocked()
Add unlocked variants of drm_gem_un/pin() functions and make them public.
These new helpers will take care of GEM dma-reservation locking for DRM
drivers.
We are going to add memory shrinking support to the VirtIO-GPU driver
that will need to pin framebuffers explicitly to prevent eviction of the
actively used buffers by the shrinker. VirtIO-GPU driver will use these
new generic helpers to pin shmem framebuffers.
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem.c | 29 +
include/drm/drm_gem.h | 3 +++
2 files changed, 32 insertions(+)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index c61674887582..c909c935cfda 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1179,6 +1179,35 @@ void drm_gem_unpin(struct drm_gem_object *obj)
obj->funcs->unpin(obj);
}
+int drm_gem_pin_unlocked(struct drm_gem_object *obj)
+{
+ int ret;
+
+ if (!obj->funcs->pin)
+ return 0;
+
+ ret = dma_resv_lock_interruptible(obj->resv, NULL);
+ if (ret)
+ return ret;
+
+ ret = obj->funcs->pin(obj);
+ dma_resv_unlock(obj->resv);
+
+ return ret;
+}
+EXPORT_SYMBOL(drm_gem_pin_unlocked);
+
+void drm_gem_unpin_unlocked(struct drm_gem_object *obj)
+{
+ if (!obj->funcs->unpin)
+ return;
+
+ dma_resv_lock(obj->resv, NULL);
+ obj->funcs->unpin(obj);
+ dma_resv_unlock(obj->resv);
+}
+EXPORT_SYMBOL(drm_gem_unpin_unlocked);
+
int drm_gem_vmap(struct drm_gem_object *obj, struct iosys_map *map)
{
int ret;
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 0b427939f466..870d81e7a104 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -413,4 +413,7 @@ int drm_gem_dumb_map_offset(struct drm_file *file, struct
drm_device *dev,
int drm_gem_vmap_unlocked(struct drm_gem_object *obj, struct iosys_map *map);
void drm_gem_vunmap_unlocked(struct drm_gem_object *obj, struct iosys_map
*map);
+int drm_gem_pin_unlocked(struct drm_gem_object *obj);
+void drm_gem_unpin_unlocked(struct drm_gem_object *obj);
+
#endif /* __DRM_GEM_H__ */
--
2.35.3
[PATCH v6 19/22] drm/virtio: Support memory shrinking
Support generic drm-shmem memory shrinker and add new madvise IOCTL to
the VirtIO-GPU driver. BO cache manager of Mesa driver will mark BOs as
"don't need" using the new IOCTL to let shrinker purge the marked BOs on
OOM, the shrinker will also evict unpurgeable shmem BOs from memory if
guest supports SWAP file or partition. Altogether this allows to prevent
OOM kills of guest applications that use VirGL by lowering memory pressure.
Signed-off-by: Daniel Almeida
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/virtio/virtgpu_drv.h| 15 ++-
drivers/gpu/drm/virtio/virtgpu_gem.c| 55 ++
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 37 +++
drivers/gpu/drm/virtio/virtgpu_kms.c| 9 ++
drivers/gpu/drm/virtio/virtgpu_object.c | 138 +++-
drivers/gpu/drm/virtio/virtgpu_plane.c | 22 +++-
drivers/gpu/drm/virtio/virtgpu_vq.c | 40 +++
include/uapi/drm/virtgpu_drm.h | 14 +++
8 files changed, 300 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 81bacc7e1873..26b570029940 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -275,7 +275,7 @@ struct virtio_gpu_fpriv {
};
/* virtgpu_ioctl.c */
-#define DRM_VIRTIO_NUM_IOCTLS 12
+#define DRM_VIRTIO_NUM_IOCTLS 13
extern struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS];
void virtio_gpu_create_context(struct drm_device *dev, struct drm_file *file);
@@ -311,6 +311,10 @@ void virtio_gpu_array_put_free(struct
virtio_gpu_object_array *objs);
void virtio_gpu_array_put_free_delayed(struct virtio_gpu_device *vgdev,
struct virtio_gpu_object_array *objs);
void virtio_gpu_array_put_free_work(struct work_struct *work);
+int virtio_gpu_array_prepare(struct virtio_gpu_device *vgdev,
+struct virtio_gpu_object_array *objs);
+int virtio_gpu_gem_host_mem_release(struct virtio_gpu_object *bo);
+int virtio_gpu_gem_madvise(struct virtio_gpu_object *obj, int madv);
/* virtgpu_vq.c */
int virtio_gpu_alloc_vbufs(struct virtio_gpu_device *vgdev);
@@ -322,6 +326,8 @@ void virtio_gpu_cmd_create_resource(struct
virtio_gpu_device *vgdev,
struct virtio_gpu_fence *fence);
void virtio_gpu_cmd_unref_resource(struct virtio_gpu_device *vgdev,
struct virtio_gpu_object *bo);
+int virtio_gpu_cmd_release_resource(struct virtio_gpu_device *vgdev,
+ struct virtio_gpu_object *bo);
void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev,
uint64_t offset,
uint32_t width, uint32_t height,
@@ -342,6 +348,9 @@ void virtio_gpu_object_attach(struct virtio_gpu_device
*vgdev,
struct virtio_gpu_object *obj,
struct virtio_gpu_mem_entry *ents,
unsigned int nents);
+void virtio_gpu_object_detach(struct virtio_gpu_device *vgdev,
+ struct virtio_gpu_object *obj,
+ struct virtio_gpu_fence *fence);
int virtio_gpu_attach_status_page(struct virtio_gpu_device *vgdev);
int virtio_gpu_detach_status_page(struct virtio_gpu_device *vgdev);
void virtio_gpu_cursor_ping(struct virtio_gpu_device *vgdev,
@@ -486,4 +495,8 @@ void virtio_gpu_vram_unmap_dma_buf(struct device *dev,
struct sg_table *sgt,
enum dma_data_direction dir);
+/* virtgpu_gem_shrinker.c */
+int virtio_gpu_gem_shrinker_init(struct virtio_gpu_device *vgdev);
+void virtio_gpu_gem_shrinker_fini(struct virtio_gpu_device *vgdev);
+
#endif
diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c
b/drivers/gpu/drm/virtio/virtgpu_gem.c
index 7db48d17ee3a..6c5d98e0f071 100644
--- a/drivers/gpu/drm/virtio/virtgpu_gem.c
+++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
@@ -294,3 +294,58 @@ void virtio_gpu_array_put_free_work(struct work_struct
*work)
}
spin_unlock(&vgdev->obj_free_lock);
}
+
+int virtio_gpu_array_prepare(struct virtio_gpu_device *vgdev,
+struct virtio_gpu_object_array *objs)
+{
+ struct virtio_gpu_object *bo;
+ int ret = 0;
+ u32 i;
+
+ for (i = 0; i < objs->nents; i++) {
+ bo = gem_to_virtio_gpu_obj(objs->objs[i]);
+
+ if (virtio_gpu_is_shmem(bo) && bo->detached) {
+ ret = virtio_gpu_reattach_shmem_object(bo);
+ if (ret)
+ break;
+ }
+ }
+
+ return ret;
+}
+
+int virtio_gpu_gem_madvise(struct virtio_gpu_object *bo, int madv)
+{
+ int ret;
+
+ /*
+* For now we support only purging BOs that are backed by guest's
+* memory.
+*/
+ if (!virtio_gpu_is_
[PATCH v6 20/22] drm/virtio: Use dev_is_pci()
Use common dev_is_pci() helper to replace the strcmp("pci") used by driver.
Suggested-by: Robin Murphy
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/virtio/virtgpu_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c
b/drivers/gpu/drm/virtio/virtgpu_drv.c
index 0141b7df97ec..0035affc3e59 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -87,7 +87,7 @@ static int virtio_gpu_probe(struct virtio_device *vdev)
return PTR_ERR(dev);
vdev->priv = dev;
- if (!strcmp(vdev->dev.parent->bus->name, "pci")) {
+ if (dev_is_pci(vdev->dev.parent)) {
ret = virtio_gpu_pci_quirk(dev);
if (ret)
goto err_free;
--
2.35.3
[PATCH v6 21/22] drm/virtio: Return proper error codes instead of -1
Don't return -1 in error cases, return proper error code. The returned
error codes propagate to error messages and to userspace and it's always
good to have a meaningful error number for debugging purposes.
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/virtio/virtgpu_vq.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 2a04dad1ae89..40402367d593 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -320,7 +320,7 @@ static int virtio_gpu_queue_ctrl_sgs(struct
virtio_gpu_device *vgdev,
if (fence && vbuf->objs)
virtio_gpu_array_unlock_resv(vbuf->objs);
free_vbuf(vgdev, vbuf);
- return -1;
+ return -ENODEV;
}
if (vgdev->has_indirect)
@@ -384,7 +384,7 @@ static int virtio_gpu_queue_fenced_ctrl_buffer(struct
virtio_gpu_device *vgdev,
if (!sgt) {
if (fence && vbuf->objs)
virtio_gpu_array_unlock_resv(vbuf->objs);
- return -1;
+ return -ENOMEM;
}
elemcnt += sg_ents;
@@ -750,7 +750,7 @@ static int virtio_get_edid_block(void *data, u8 *buf,
size_t start = block * EDID_LENGTH;
if (start + len > le32_to_cpu(resp->size))
- return -1;
+ return -EINVAL;
memcpy(buf, resp->edid + start, len);
return 0;
}
--
2.35.3
[PATCH v6 22/22] drm/panfrost: Switch to generic memory shrinker
Replace Panfrost's memory shrinker with a generic drm-shmem memory
shrinker.
Tested-by: Steven Price
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/panfrost/Makefile | 1 -
drivers/gpu/drm/panfrost/panfrost_device.h| 4 -
drivers/gpu/drm/panfrost/panfrost_drv.c | 19 +--
drivers/gpu/drm/panfrost/panfrost_gem.c | 33 +++--
drivers/gpu/drm/panfrost/panfrost_gem.h | 9 --
.../gpu/drm/panfrost/panfrost_gem_shrinker.c | 129 --
drivers/gpu/drm/panfrost/panfrost_job.c | 18 ++-
7 files changed, 42 insertions(+), 171 deletions(-)
delete mode 100644 drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
diff --git a/drivers/gpu/drm/panfrost/Makefile
b/drivers/gpu/drm/panfrost/Makefile
index b71935862417..ecf0864cb515 100644
--- a/drivers/gpu/drm/panfrost/Makefile
+++ b/drivers/gpu/drm/panfrost/Makefile
@@ -5,7 +5,6 @@ panfrost-y := \
panfrost_device.o \
panfrost_devfreq.o \
panfrost_gem.o \
- panfrost_gem_shrinker.o \
panfrost_gpu.o \
panfrost_job.o \
panfrost_mmu.o \
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h
b/drivers/gpu/drm/panfrost/panfrost_device.h
index 8b25278f34c8..fe04b21fc044 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.h
+++ b/drivers/gpu/drm/panfrost/panfrost_device.h
@@ -115,10 +115,6 @@ struct panfrost_device {
atomic_t pending;
} reset;
- struct mutex shrinker_lock;
- struct list_head shrinker_list;
- struct shrinker shrinker;
-
struct panfrost_devfreq pfdevfreq;
};
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 859e240161d1..b77c99ba2475 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -160,7 +160,6 @@ panfrost_lookup_bos(struct drm_device *dev,
break;
}
- atomic_inc(&bo->gpu_usecount);
job->mappings[i] = mapping;
}
@@ -392,7 +391,6 @@ static int panfrost_ioctl_madvise(struct drm_device *dev,
void *data,
{
struct panfrost_file_priv *priv = file_priv->driver_priv;
struct drm_panfrost_madvise *args = data;
- struct panfrost_device *pfdev = dev->dev_private;
struct drm_gem_object *gem_obj;
struct panfrost_gem_object *bo;
int ret = 0;
@@ -409,7 +407,6 @@ static int panfrost_ioctl_madvise(struct drm_device *dev,
void *data,
if (ret)
goto out_put_object;
- mutex_lock(&pfdev->shrinker_lock);
mutex_lock(&bo->mappings.lock);
if (args->madv == PANFROST_MADV_DONTNEED) {
struct panfrost_gem_mapping *first;
@@ -435,17 +432,8 @@ static int panfrost_ioctl_madvise(struct drm_device *dev,
void *data,
args->retained = drm_gem_shmem_madvise(&bo->base, args->madv);
- if (args->retained) {
- if (args->madv == PANFROST_MADV_DONTNEED)
- list_move_tail(&bo->base.madv_list,
- &pfdev->shrinker_list);
- else if (args->madv == PANFROST_MADV_WILLNEED)
- list_del_init(&bo->base.madv_list);
- }
-
out_unlock_mappings:
mutex_unlock(&bo->mappings.lock);
- mutex_unlock(&pfdev->shrinker_lock);
dma_resv_unlock(bo->base.base.resv);
out_put_object:
drm_gem_object_put(gem_obj);
@@ -577,9 +565,6 @@ static int panfrost_probe(struct platform_device *pdev)
ddev->dev_private = pfdev;
pfdev->ddev = ddev;
- mutex_init(&pfdev->shrinker_lock);
- INIT_LIST_HEAD(&pfdev->shrinker_list);
-
err = panfrost_device_init(pfdev);
if (err) {
if (err != -EPROBE_DEFER)
@@ -601,7 +586,7 @@ static int panfrost_probe(struct platform_device *pdev)
if (err < 0)
goto err_out1;
- panfrost_gem_shrinker_init(ddev);
+ drm_gem_shmem_shrinker_register(ddev);
return 0;
@@ -619,8 +604,8 @@ static int panfrost_remove(struct platform_device *pdev)
struct panfrost_device *pfdev = platform_get_drvdata(pdev);
struct drm_device *ddev = pfdev->ddev;
+ drm_gem_shmem_shrinker_unregister(ddev);
drm_dev_unregister(ddev);
- panfrost_gem_shrinker_cleanup(ddev);
pm_runtime_get_sync(pfdev->dev);
pm_runtime_disable(pfdev->dev);
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c
b/drivers/gpu/drm/panfrost/panfrost_gem.c
index 293e799e2fe8..f1436405e3a0 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
@@ -19,16 +19,6 @@ static void panfrost_gem_free_object(struct drm_gem_object
*obj)
struct panfrost_gem_object *bo = to_panfrost_bo(obj);
struct panfrost_device *pfdev = obj->dev->dev_private;
- /*
-* Make sure the BO is no longer inserted in the shrinker list before
-* taking
Re: [PATCH] drm/msm/dp: force link training for display resolution change
Quoting Kuogee Hsieh (2022-05-26 10:26:18)
> During display resolution changes display have to be disabled first
> followed by display enable with new resolution. This patch force
> main link always be retrained during display enable procedure to
> simplify implementation instead of manually kicking of irq_hpd
> handle.
How is that better? Can you please describe how it improves things? I
suppose it avoids some round trips through the event queue just to turn
on the display?
>
> Signed-off-by: Kuogee Hsieh
Any Fixes tag? Or it's not fixing anything, just making things simpler?
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 5ddb4e8..8b71013 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h
> b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> index 2433edb..dcc7af21 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> @@ -20,7 +20,7 @@ struct dp_ctrl {
> };
>
> int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl);
> -int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl);
> +int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl, bool force_link_train);
Can we have another API like dp_ctrl_on_stream_retrain()? Or name
'force_link_train' to 'retrain'?
> int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl);
> int dp_ctrl_off(struct dp_ctrl *dp_ctrl);
> void dp_ctrl_push_idle(struct dp_ctrl *dp_ctrl);
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index bfc6581..9246421 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -892,7 +892,7 @@ static int dp_display_enable(struct dp_display_private
> *dp, u32 data)
Any reason why dp_display_enable() is forward declared and why it takes
a u32 data argument? It's not part of the eventqueue calling code from
what I can tell so we should be able to rename 'data' to 'retrain_link'?
> return 0;
> }
>
> - rc = dp_ctrl_on_stream(dp->ctrl);
> + rc = dp_ctrl_on_stream(dp->ctrl, data);
> if (!rc)
> dp_display->power_on = true;
>
> @@ -1594,6 +1594,7 @@ int msm_dp_display_enable(struct msm_dp *dp, struct
> drm_encoder *encoder)
> int rc = 0;
> struct dp_display_private *dp_display;
> u32 state;
> + bool force_link_train = false;
>
> dp_display = container_of(dp, struct dp_display_private, dp_display);
> if (!dp_display->dp_mode.drm_mode.clock) {
> @@ -1622,10 +1623,12 @@ int msm_dp_display_enable(struct msm_dp *dp, struct
> drm_encoder *encoder)
>
> state = dp_display->hpd_state;
>
> - if (state == ST_DISPLAY_OFF)
> + if (state == ST_DISPLAY_OFF) {
> dp_display_host_phy_init(dp_display);
> + force_link_train = 1;
Use true instead of 1 if the type is a bool please.
> + }
>
> - dp_display_enable(dp_display, 0);
> + dp_display_enable(dp_display, force_link_train);
>
> rc = dp_display_post_enable(dp);
> if (rc) {
Re: [PATCH v13 1/3] phy: qcom-edp: add regulator_set_load to edp phy
Quoting Kuogee Hsieh (2022-05-25 14:02:18) > This patch add regulator_set_load() before enable regulator at > eDP phy driver. > > Signed-off-by: Kuogee Hsieh > Reviewed-by: Douglas Anderson > --- Reviewed-by: Stephen Boyd
Re: [PATCH 18/25] drm/msm/dpu: move the rest of plane checks to dpu_plane_atomic_check()
On 2/9/2022 9:25 AM, Dmitry Baryshkov wrote:
Remove plane checks/state updates from dpu_crtc_atomic_check(). The
belong to dpu_plane_atomic_check().
The --> They
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 44 ---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 18 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 6
3 files changed, 10 insertions(+), 58 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index cbd0e50c2bd3..fa279f0358d6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1054,14 +1054,10 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc_state);
- const struct drm_plane_state *pstate;
- struct drm_plane *plane;
struct drm_display_mode *mode;
int rc = 0;
- struct drm_rect crtc_rect = { 0 };
-
if (!crtc_state->enable || !crtc_state->active) {
DRM_DEBUG_ATOMIC("crtc%d -> enable %d, active %d, skip
atomic_check\n",
crtc->base.id, crtc_state->enable,
@@ -1080,46 +1076,6 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
if (cstate->num_mixers)
_dpu_crtc_setup_lm_bounds(crtc, crtc_state);
- crtc_rect.x2 = mode->hdisplay;
- crtc_rect.y2 = mode->vdisplay;
-
-/* get plane state for all drm planes associated with crtc state */
- drm_atomic_crtc_state_for_each_plane_state(plane, pstate, crtc_state) {
- struct drm_rect dst, clip = crtc_rect;
- int z_pos;
-
- if (IS_ERR_OR_NULL(pstate)) {
- rc = PTR_ERR(pstate);
- DPU_ERROR("%s: failed to get plane%d state, %d\n",
- dpu_crtc->name, plane->base.id, rc);
- return rc;
- }
-
- dpu_plane_clear_multirect(pstate);
-
- dst = drm_plane_state_dest(pstate);
- if (!drm_rect_intersect(&clip, &dst)) {
- DPU_ERROR("invalid vertical/horizontal destination\n");
- DPU_ERROR("display: " DRM_RECT_FMT " plane: "
- DRM_RECT_FMT "\n", DRM_RECT_ARG(&crtc_rect),
- DRM_RECT_ARG(&dst));
- return -E2BIG;
- }
Doesnt this check also have to be moved? We need to make sure dest
rectangle is within CRTC bounds?
-
- z_pos = pstate->normalized_zpos;
-
- /* verify z_pos setting before using it */
- if (z_pos >= DPU_STAGE_MAX - DPU_STAGE_0) {
- DPU_ERROR("> %d plane stages assigned\n",
- DPU_STAGE_MAX - DPU_STAGE_0);
- return -EINVAL;
- }
-
- to_dpu_plane_state(pstate)->stage = z_pos + DPU_STAGE_0;
- DRM_DEBUG_ATOMIC("%s: zpos %d\n", dpu_crtc->name, z_pos);
-
- }
-
atomic_inc(&_dpu_crtc_get_kms(crtc)->bandwidth_ref);
rc = dpu_core_perf_crtc_check(crtc, crtc_state);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index b2395f02f6d3..637d164667e9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -702,14 +702,6 @@ static int _dpu_plane_color_fill(struct dpu_plane *pdpu,
return 0;
}
-void dpu_plane_clear_multirect(const struct drm_plane_state *drm_state)
-{
- struct dpu_plane_state *pstate = to_dpu_plane_state(drm_state);
-
- pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
- pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
-}
-
int dpu_plane_validate_multirect_v2(struct dpu_multirect_plane_states *plane)
{
struct dpu_plane_state *pstate[R_MAX];
@@ -931,6 +923,16 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
if (!new_plane_state->visible)
return 0;
+ pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
+ pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+
+ pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
+ if (pstate->stage >= DPU_STAGE_MAX) {
+ DPU_ERROR("> %d plane stages assigned\n",
+ DPU_STAGE_MAX - DPU_STAGE_0);
+ return -EINVAL;
+ }
+
src.x1 = new_plane_state->src_x >> 16;
src.y1 = new_plane_state->src_y >> 16;
src.x2 = src.x1 + (new_plane_state->src_w >> 16);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
index b6fd6f856d6a..e61c57b045ea 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane
Re: [PATCH 2/2] drm/tiny: Add ofdrm for Open Firmware framebuffers
On Wed, 2022-05-25 at 18:45 +0200, Thomas Zimmermann wrote: > > > The palette handling is useful when using a real Open Firmware > > implementation which tends to boot in 8-bit mode, so without palette > > things will look ... bad. > > > > It's not necessary when using 16/32 bpp framebuffers which is typically > > ... what BootX provides :-) > > Maybe the odd color formats can be tested via qemu. > > I don't mind adding DRM support for BootX displays, but getting the > necessary test HW with a suitable Linux seems to be laborious. Would a > G4 Powerbook work? My point was that it's the non-BootX case that cares about the palette hacks :-) Cheers, Ben.
Re: [PATCH 2/2] drm/tiny: Add ofdrm for Open Firmware framebuffers
On Wed, 2022-05-25 at 18:45 +0200, Thomas Zimmermann wrote: > I don't mind adding DRM support for BootX displays, but getting the > necessary test HW with a suitable Linux seems to be laborious. Would > a G4 Powerbook work? Probably not unfortunately... it's going to be tricky. I might sitll have some old BootX-based machines somewhere in storage I could try to dig out, but it might not be worth bothering too much... Cheers, Ben.
Re: [PATCH v3 02/13] mm: handling Non-LRU pages returned by vm_normal_pages
"Sierra Guiza, Alejandro (Alex)" writes:
> On 5/24/2022 11:11 PM, Alistair Popple wrote:
>> Alex Sierra writes:
>>
>>> With DEVICE_COHERENT, we'll soon have vm_normal_pages() return
>>> device-managed anonymous pages that are not LRU pages. Although they
>>> behave like normal pages for purposes of mapping in CPU page, and for
>>> COW. They do not support LRU lists, NUMA migration or THP.
>>>
>>> We also introduced a FOLL_LRU flag that adds the same behaviour to
>>> follow_page and related APIs, to allow callers to specify that they
>>> expect to put pages on an LRU list.
>> Continuing the follow up from the thread for v2:
>>
This means by default GUP can return non-LRU pages. I didn't see
anywhere that would be a problem but I didn't check everything. Did you
check this or is there some other reason I've missed that makes this not
a problem?
>>> I have double checked all gup and pin_user_pages callers and none of them
>>> seem
>>> to have interaction with LRU APIs.
>> And actually if I'm understanding things correctly callers of
>> GUP/PUP/follow_page_pte() should already expect to get non-LRU pages
>> returned:
>>
>> page = vm_normal_page(vma, address, pte);
>> if ((flags & FOLL_LRU) && page && is_zone_device_page(page))
>> page = NULL;
>> if (!page && pte_devmap(pte) && (flags & (FOLL_GET | FOLL_PIN))) {
>> /*
>> * Only return device mapping pages in the FOLL_GET or FOLL_PIN
>> * case since they are only valid while holding the pgmap
>> * reference.
>> */
>> *pgmap = get_dev_pagemap(pte_pfn(pte), *pgmap);
>> if (*pgmap)
>> page = pte_page(pte);
>>
>> Which I think makes FOLL_LRU confusing, because if understand correctly
>> even with FOLL_LRU it is still possible for follow_page_pte() to return
>> a non-LRU page. Could we do something like this to make it consistent:
>>
>> if ((flags & FOLL_LRU) && (page && is_zone_device_page(page) ||
>> !page && pte_devmap(pte)))
>
> Hi Alistair,
> Not sure if this suggestion is a replacement for the first or the second
> condition in the snip code above. We know device coherent type will not
> be set with devmap. So we could do the following:
Sorry, I must not have been clear enough. My understanding is if the
following condition is true:
>> if (!page && pte_devmap(pte) && (flags & (FOLL_GET | FOLL_PIN))) {
Then follow_page_pte() could return a non-LRU page even when FOLL_LRU is
specified (because I think a devmap page is a non-LRU page). That seems
confusing, so for consistency I was suggesting we should not return
devmap pages for FOLL_LRU.
To be clear I don't think there is an actual problem here atm, but the
inconsistency could easily lead to one in future.
> if ((flags & FOLL_LRU) && page && is_zone_device_page(page))
> - page = NULL;
> + goto no_page;
>
> Regards,
> Alex Sierra
>
>>
>> Looking at callers that currently use FOLL_LRU I don't think this would
>> change any behaviour as they already filter out devmap through various
>> other means.
>>
>>> Signed-off-by: Alex Sierra
>>> Acked-by: Felix Kuehling
>>> ---
>>> fs/proc/task_mmu.c | 2 +-
>>> include/linux/mm.h | 3 ++-
>>> mm/gup.c | 2 ++
>>> mm/huge_memory.c | 2 +-
>>> mm/khugepaged.c| 9 ++---
>>> mm/ksm.c | 6 +++---
>>> mm/madvise.c | 4 ++--
>>> mm/memory.c| 9 -
>>> mm/mempolicy.c | 2 +-
>>> mm/migrate.c | 4 ++--
>>> mm/mlock.c | 2 +-
>>> mm/mprotect.c | 2 +-
>>> 12 files changed, 30 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
>>> index f46060eb91b5..5d620733f173 100644
>>> --- a/fs/proc/task_mmu.c
>>> +++ b/fs/proc/task_mmu.c
>>> @@ -1785,7 +1785,7 @@ static struct page *can_gather_numa_stats(pte_t pte,
>>> struct vm_area_struct *vma,
>>> return NULL;
>>>
>>> page = vm_normal_page(vma, addr, pte);
>>> - if (!page)
>>> + if (!page || is_zone_device_page(page))
>>> return NULL;
>>>
>>> if (PageReserved(page))
>>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>>> index 9f44254af8ce..d7f253a0c41e 100644
>>> --- a/include/linux/mm.h
>>> +++ b/include/linux/mm.h
>>> @@ -601,7 +601,7 @@ struct vm_operations_struct {
>>> #endif
>>> /*
>>> * Called by vm_normal_page() for special PTEs to find the
>>> -* page for @addr. This is useful if the default behavior
>>> +* page for @addr. This is useful if the default behavior
>>> * (using pte_page()) would not find the correct page.
>>> */
>>> struct page *(*find_special_page)(struct vm_area_struct *vma,
>>> @@ -2929,6 +2929,7 @@ struct page *follow_page(struct vm_area_struct *vma,
>>> unsigned long address,
>>> #define FOLL_NUMA 0x200 /* force NUMA hinting page fault */
>>> #define FOLL_MIGRATION0x400 /* wait for page to replace migration
>>> entry */
>>> #define FOLL_
Re: [PATCH 2/2] backlight: rt4831: Add the property parsing for ocp level selection
Daniel Thompson 於 2022年5月26日 週四 下午6:05寫道：
>
> On Thu, May 26, 2022 at 11:16:35AM +0800, cy_huang wrote:
> > From: ChiYuan Huang
> >
> > Add the property parsing for ocp level selection.
>
> Isn't this just restating the Subject: line?
>
Ah, that's my fault. I didn't state too much in the patch comment.
I only left it in the cover letter.
> It would be better to provide information useful to the reviewer here.
> Something like:
>
> "Currently this driver simply inherits whatever over-current protection
> level is programmed into the hardware when it is handed over. Typically
> this will be the reset value, A, although the bootloader could
> have established a different value.
>
> Allow the correct OCP value to be provided by the DT."
>
> BTW please don't uncritically copy the above into the patch header. It is
> just made something up as an example and I did no fact checking...
>
OK, got it.
>
> >
> > Reported-by: Lucas Tsai
> > Signed-off-by: ChiYuan Huang
> > ---
> > drivers/video/backlight/rt4831-backlight.c | 13 +
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/drivers/video/backlight/rt4831-backlight.c
> > b/drivers/video/backlight/rt4831-backlight.c
> > index 42155c7..c81f7d9 100644
> > --- a/drivers/video/backlight/rt4831-backlight.c
> > +++ b/drivers/video/backlight/rt4831-backlight.c
> > @@ -12,6 +12,7 @@
> > #define RT4831_REG_BLCFG 0x02
> > #define RT4831_REG_BLDIML0x04
> > #define RT4831_REG_ENABLE0x08
> > +#define RT4831_REG_BLOPT20x11
> >
> > #define RT4831_BLMAX_BRIGHTNESS 2048
> >
> > @@ -23,6 +24,8 @@
> > #define RT4831_BLDIML_MASK GENMASK(2, 0)
> > #define RT4831_BLDIMH_MASK GENMASK(10, 3)
> > #define RT4831_BLDIMH_SHIFT 3
> > +#define RT4831_BLOCP_MASKGENMASK(1, 0)
> > +#define RT4831_BLOCP_SHIFT 0
> >
> > struct rt4831_priv {
> > struct device *dev;
> > @@ -120,6 +123,16 @@ static int rt4831_parse_backlight_properties(struct
> > rt4831_priv *priv,
> > if (ret)
> > return ret;
> >
> > + ret = device_property_read_u8(dev, "richtek,bled-ocp-sel", &propval);
> > + if (ret)
> > + propval = RT4831_BLOCPLVL_1P2A;
>
> Is 1.2A the reset value for the register?
Yes, it's the HW default value.
>
> Additionally, it looks like adding a hard-coded default would cause
> problems for existing platforms where the bootloader doesn't use
> richtek,bled-ocp-sel and pre-configures a different value itself.
>
> Would it be safer (in terms of working nicely with older bootloaders)
> to only write to the RT4831_BLOCP_MASK if the new property is set?
>
Ah, my excuse. I really didn't consider the case that you mentioned.
It seems it's better to do the judgement here for two cases.
1) property not exist, keep the current HW value
2) property exist, clamp align and update the suitable selector to HW.
Thanks.
>
> Daniel.
>
>
>
> > +
> > + propval = min_t(u8, propval, RT4831_BLOCPLVL_1P8A);
> > + ret = regmap_update_bits(priv->regmap, RT4831_REG_BLOPT2,
> > RT4831_BLOCP_MASK,
> > + propval << RT4831_BLOCP_SHIFT);
> > + if (ret)
> > + return ret;
> > +
> > ret = device_property_read_u8(dev, "richtek,channel-use", &propval);
> > if (ret) {
> > dev_err(dev, "richtek,channel-use DT property missing\n");
> > --
> > 2.7.4
> >
Re: [Freedreno] [PATCH v4 12/13] drm/msm: Utilize gpu scheduler priorities
On Thu, May 26, 2022 at 4:38 AM Tvrtko Ursulin
wrote:
>
>
> On 26/05/2022 04:37, Rob Clark wrote:
> > On Wed, May 25, 2022 at 9:22 AM Tvrtko Ursulin
> > wrote:
> >>
> >>
> >> On 25/05/2022 14:41, Rob Clark wrote:
> >>> On Wed, May 25, 2022 at 2:46 AM Tvrtko Ursulin
> >>> wrote:
>
>
> On 24/05/2022 15:50, Rob Clark wrote:
> > On Tue, May 24, 2022 at 6:45 AM Tvrtko Ursulin
> > wrote:
> >>
> >>
> >> On 23/05/2022 23:53, Rob Clark wrote:
> >>> On Mon, May 23, 2022 at 7:45 AM Tvrtko Ursulin
> >>> wrote:
>
>
> Hi Rob,
>
> On 28/07/2021 02:06, Rob Clark wrote:
> > From: Rob Clark
> >
> > The drm/scheduler provides additional prioritization on top of that
> > provided by however many number of ringbuffers (each with their own
> > priority level) is supported on a given generation. Expose the
> > additional levels of priority to userspace and map the userspace
> > priority back to ring (first level of priority) and schedular
> > priority
> > (additional priority levels within the ring).
> >
> > Signed-off-by: Rob Clark
> > Acked-by: Christian König
> > ---
> > drivers/gpu/drm/msm/adreno/adreno_gpu.c | 4 +-
> > drivers/gpu/drm/msm/msm_gem_submit.c| 4 +-
> > drivers/gpu/drm/msm/msm_gpu.h | 58
> > -
> > drivers/gpu/drm/msm/msm_submitqueue.c | 35 +++
> > include/uapi/drm/msm_drm.h | 14 +-
> > 5 files changed, 88 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > index bad4809b68ef..748665232d29 100644
> > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > @@ -261,8 +261,8 @@ int adreno_get_param(struct msm_gpu *gpu,
> > uint32_t param, uint64_t *value)
> > return ret;
> > }
> > return -EINVAL;
> > - case MSM_PARAM_NR_RINGS:
> > - *value = gpu->nr_rings;
> > + case MSM_PARAM_PRIORITIES:
> > + *value = gpu->nr_rings * NR_SCHED_PRIORITIES;
> > return 0;
> > case MSM_PARAM_PP_PGTABLE:
> > *value = 0;
> > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> > b/drivers/gpu/drm/msm/msm_gem_submit.c
> > index 450efe59abb5..c2ecec5b11c4 100644
> > --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> > @@ -59,7 +59,7 @@ static struct msm_gem_submit
> > *submit_create(struct drm_device *dev,
> > submit->gpu = gpu;
> > submit->cmd = (void *)&submit->bos[nr_bos];
> > submit->queue = queue;
> > - submit->ring = gpu->rb[queue->prio];
> > + submit->ring = gpu->rb[queue->ring_nr];
> > submit->fault_dumped = false;
> >
> > INIT_LIST_HEAD(&submit->node);
> > @@ -749,7 +749,7 @@ int msm_ioctl_gem_submit(struct drm_device
> > *dev, void *data,
> > /* Get a unique identifier for the submission for logging
> > purposes */
> > submitid = atomic_inc_return(&ident) - 1;
> >
> > - ring = gpu->rb[queue->prio];
> > + ring = gpu->rb[queue->ring_nr];
> > trace_msm_gpu_submit(pid_nr(pid), ring->id, submitid,
> > args->nr_bos, args->nr_cmds);
> >
> > diff --git a/drivers/gpu/drm/msm/msm_gpu.h
> > b/drivers/gpu/drm/msm/msm_gpu.h
> > index b912cacaecc0..0e4b45bff2e6 100644
> > --- a/drivers/gpu/drm/msm/msm_gpu.h
> > +++ b/drivers/gpu/drm/msm/msm_gpu.h
> > @@ -250,6 +250,59 @@ struct msm_gpu_perfcntr {
> > const char *name;
> > };
> >
> > +/*
> > + * The number of priority levels provided by drm gpu scheduler.
> > The
> > + * DRM_SCHED_PRIORITY_KERNEL priority level is treated specially
> > in some
> > + * cases, so we don't use it (no need for kernel generated jobs).
> > + */
> > +#define NR_SCHED_PRIORITIES (1 + DRM_SCHED_PRIORITY_HIGH -
> > DRM_SCHED_PRIORITY_MIN)
> > +
> > +/**
> > + * msm_gpu_convert_priority - Map userspace priority to ring # and
> > sched priority
> > + *
> > + * @gpu:the gpu instance
> > + * @prio: the userspace priority level
> > + * @ring_nr:[out] th
Re: [PATCH v4 12/13] drm/msm: Utilize gpu scheduler priorities
On Thu, May 26, 2022 at 6:29 AM Tvrtko Ursulin wrote: > > > On 26/05/2022 04:15, Rob Clark wrote: > > On Wed, May 25, 2022 at 9:11 AM Tvrtko Ursulin > > wrote: > >> > >> > >> On 24/05/2022 15:57, Rob Clark wrote: > >>> On Tue, May 24, 2022 at 6:45 AM Tvrtko Ursulin > >>> wrote: > > On 23/05/2022 23:53, Rob Clark wrote: > > > > btw, one fun (but unrelated) issue I'm hitting with scheduler... I'm > > trying to add an igt test to stress shrinker/eviction, similar to the > > existing tests/i915/gem_shrink.c. But we hit an unfortunate > > combination of circumstances: > > 1. Pinning memory happens in the synchronous part of the submit ioctl, > > before enqueuing the job for the kthread to handle. > > 2. The first run_job() callback incurs a slight delay (~1.5ms) while > > resuming the GPU > > 3. Because of that delay, userspace has a chance to queue up enough > > more jobs to require locking/pinning more than the available system > > RAM.. > > Is that one or multiple threads submitting jobs? > >>> > >>> In this case multiple.. but I think it could also happen with a single > >>> thread (provided it didn't stall on a fence, directly or indirectly, > >>> from an earlier submit), because of how resume and actual job > >>> submission happens from scheduler kthread. > >>> > > I'm not sure if we want a way to prevent userspace from getting *too* > > far ahead of the kthread. Or maybe at some point the shrinker should > > sleep on non-idle buffers? > > On the direct reclaim path when invoked from the submit ioctl? In i915 > we only shrink idle objects on direct reclaim and leave active ones for > the swapper. It depends on how your locking looks like whether you could > do them, whether there would be coupling of locks and fs-reclaim context. > >>> > >>> I think the locking is more or less ok, although lockdep is unhappy > >>> about one thing[1] which is I think a false warning (ie. not > >>> recognizing that we'd already successfully acquired the obj lock via > >>> trylock). We can already reclaim idle bo's in this path. But the > >>> problem with a bunch of submits queued up in the scheduler, is that > >>> they are already considered pinned and active. So at some point we > >>> need to sleep (hopefully interruptabley) until they are no longer > >>> active, ie. to throttle userspace trying to shove in more submits > >>> until some of the enqueued ones have a chance to run and complete. > >> > >> Odd I did not think trylock could trigger that. Looking at your code it > >> indeed seems two trylocks. I am pretty sure we use the same trylock > >> trick to avoid it. I am confused.. > > > > The sequence is, > > > > 1. kref_get_unless_zero() > > 2. trylock, which succeeds > > 3. attempt to evict or purge (which may or may not have succeeded) > > 4. unlock > > > > ... meanwhile this has raced with submit (aka execbuf) finishing and > > retiring and dropping *other* remaining reference to bo... > > > > 5. drm_gem_object_put() which triggers drm_gem_object_free() > > 6. in our free path we acquire the obj lock again and then drop it. > > Which arguably is unnecessary and only serves to satisfy some > > GEM_WARN_ON(!msm_gem_is_locked(obj)) in code paths that are also used > > elsewhere > > > > lockdep doesn't realize the previously successful trylock+unlock > > sequence so it assumes that the code that triggered recursion into > > shrinker could be holding the objects lock. > > Ah yes, missed that lock after trylock in msm_gem_shrinker/scan(). Well > i915 has the same sequence in our shrinker, but the difference is we use > delayed work to actually free, _and_ use trylock in the delayed worker. > It does feel a bit inelegant (objects with no reference count which > cannot be trylocked?!), but as this is the code recently refactored by > Maarten so I think best try and sync with him for the full story. ahh, we used to use delayed work for free, but realized that was causing janks where we'd get a bunch of bo's queued up to free and at some point that would cause us to miss deadlines I suppose instead we could have used an unbound wq for free instead of the same one we used (at the time, since transitioned to kthread worker to avoid being preempted by RT SF threads) for retiring submits > >> Otherwise if you can afford to sleep you can of course throttle > >> organically via direct reclaim. Unless I am forgetting some key gotcha - > >> it's been a while I've been active in this area. > > > > So, one thing that is awkward about sleeping in this path is that > > there is no way to propagate back -EINTR, so we end up doing an > > uninterruptible sleep in something that could be called indirectly > > from userspace syscall.. i915 seems to deal with this by limiting it > > to shrinker being called from kswapd. I think in the shrinker we want > > to know whether it is ok to sleep (ie. not syscall trigggered > > codepat
Re: [PATCH v3 02/13] mm: handling Non-LRU pages returned by vm_normal_pages
Am 2022-05-25 um 00:11 schrieb Alistair Popple:
Alex Sierra writes:
With DEVICE_COHERENT, we'll soon have vm_normal_pages() return
device-managed anonymous pages that are not LRU pages. Although they
behave like normal pages for purposes of mapping in CPU page, and for
COW. They do not support LRU lists, NUMA migration or THP.
We also introduced a FOLL_LRU flag that adds the same behaviour to
follow_page and related APIs, to allow callers to specify that they
expect to put pages on an LRU list.
Continuing the follow up from the thread for v2:
This means by default GUP can return non-LRU pages. I didn't see
anywhere that would be a problem but I didn't check everything. Did you
check this or is there some other reason I've missed that makes this not
a problem?
I have double checked all gup and pin_user_pages callers and none of them seem
to have interaction with LRU APIs.
And actually if I'm understanding things correctly callers of
GUP/PUP/follow_page_pte() should already expect to get non-LRU pages
returned:
page = vm_normal_page(vma, address, pte);
if ((flags & FOLL_LRU) && page && is_zone_device_page(page))
page = NULL;
if (!page && pte_devmap(pte) && (flags & (FOLL_GET | FOLL_PIN))) {
/*
* Only return device mapping pages in the FOLL_GET or FOLL_PIN
* case since they are only valid while holding the pgmap
* reference.
*/
*pgmap = get_dev_pagemap(pte_pfn(pte), *pgmap);
if (*pgmap)
page = pte_page(pte);
Which I think makes FOLL_LRU confusing, because if understand correctly
even with FOLL_LRU it is still possible for follow_page_pte() to return
a non-LRU page. Could we do something like this to make it consistent:
if ((flags & FOLL_LRU) && (page && is_zone_device_page(page) ||
!page && pte_devmap(pte)))
This alone won't help if it still goes into the if (!page &&
pte_devmap(pte) ...) afterwards. I think what you're suggesting is:
+ if ((flags & FOLL_LRU) && (page && is_zone_device_page(page) ||
+ !page && pte_devmap(pte)))
+ page = NULL;
- |if (!page && pte_devmap(pte) && (flags & (FOLL_GET | FOLL_PIN))) { +
else if (!page && pte_devmap(pte) && (flags & (FOLL_GET | FOLL_PIN))) { |
Is that what you meant?
Regards,
Felix
Looking at callers that currently use FOLL_LRU I don't think this would
change any behaviour as they already filter out devmap through various
other means.
Signed-off-by: Alex Sierra
Acked-by: Felix Kuehling
---
fs/proc/task_mmu.c | 2 +-
include/linux/mm.h | 3 ++-
mm/gup.c | 2 ++
mm/huge_memory.c | 2 +-
mm/khugepaged.c| 9 ++---
mm/ksm.c | 6 +++---
mm/madvise.c | 4 ++--
mm/memory.c| 9 -
mm/mempolicy.c | 2 +-
mm/migrate.c | 4 ++--
mm/mlock.c | 2 +-
mm/mprotect.c | 2 +-
12 files changed, 30 insertions(+), 17 deletions(-)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index f46060eb91b5..5d620733f173 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -1785,7 +1785,7 @@ static struct page *can_gather_numa_stats(pte_t pte,
struct vm_area_struct *vma,
return NULL;
page = vm_normal_page(vma, addr, pte);
- if (!page)
+ if (!page || is_zone_device_page(page))
return NULL;
if (PageReserved(page))
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 9f44254af8ce..d7f253a0c41e 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -601,7 +601,7 @@ struct vm_operations_struct {
#endif
/*
* Called by vm_normal_page() for special PTEs to find the
-* page for @addr. This is useful if the default behavior
+* page for @addr. This is useful if the default behavior
* (using pte_page()) would not find the correct page.
*/
struct page *(*find_special_page)(struct vm_area_struct *vma,
@@ -2929,6 +2929,7 @@ struct page *follow_page(struct vm_area_struct *vma,
unsigned long address,
#define FOLL_NUMA 0x200 /* force NUMA hinting page fault */
#define FOLL_MIGRATION0x400 /* wait for page to replace migration
entry */
#define FOLL_TRIED0x800 /* a retry, previous pass started an IO */
+#define FOLL_LRU0x1000 /* return only LRU (anon or page cache) */
#define FOLL_REMOTE 0x2000 /* we are working on non-current tsk/mm */
#define FOLL_COW 0x4000 /* internal GUP flag */
#define FOLL_ANON 0x8000 /* don't do file mappings */
diff --git a/mm/gup.c b/mm/gup.c
index 501bc150792c..c9cbac06bcc5 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -479,6 +479,8 @@ static struct page *follow_page_pte(struct vm_area_struct
*vma,
}
page = vm_normal_page(vma, address, pte);
+ if ((flags & FOLL_LRU) && page && is_zone_device_page(page))
+ page = NULL;
if (!page && pte_devmap(pte) && (flags & (FO
Re: [PATCH v2] drm/msm/dp: force link training for display resolution change
Hi Kuogee,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm/drm-next]
[also build test ERROR on next-20220526]
[cannot apply to v5.18]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/intel-lab-lkp/linux/commits/Kuogee-Hsieh/drm-msm-dp-force-link-training-for-display-resolution-change/20220527-071202
base: git://anongit.freedesktop.org/drm/drm drm-next
config: sparc-allmodconfig
(https://download.01.org/0day-ci/archive/20220527/202205271424.apfrmbam-...@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/intel-lab-lkp/linux/commit/42816831cad7ced23cdedbb77ef2ebf13247c623
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
Kuogee-Hsieh/drm-msm-dp-force-link-training-for-display-resolution-change/20220527-071202
git checkout 42816831cad7ced23cdedbb77ef2ebf13247c623
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1
O=build_dir ARCH=sparc SHELL=/bin/bash drivers/gpu/drm/msm/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
drivers/gpu/drm/msm/dp/dp_ctrl.c: In function 'dp_ctrl_on_stream':
>> drivers/gpu/drm/msm/dp/dp_ctrl.c:1852:13: error: 'force_link_tarin'
>> undeclared (first use in this function); did you mean 'force_link_train'?
1852 | if (force_link_tarin || !dp_ctrl_channel_eq_ok(ctrl))
| ^~~~
| force_link_train
drivers/gpu/drm/msm/dp/dp_ctrl.c:1852:13: note: each undeclared identifier
is reported only once for each function it appears in
vim +1852 drivers/gpu/drm/msm/dp/dp_ctrl.c
1810
1811 int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl, bool force_link_train)
1812 {
1813 int ret = 0;
1814 bool mainlink_ready = false;
1815 struct dp_ctrl_private *ctrl;
1816 unsigned long pixel_rate_orig;
1817
1818 if (!dp_ctrl)
1819 return -EINVAL;
1820
1821 ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
1822
1823 ctrl->dp_ctrl.pixel_rate = ctrl->panel->dp_mode.drm_mode.clock;
1824
1825 pixel_rate_orig = ctrl->dp_ctrl.pixel_rate;
1826 if (dp_ctrl->wide_bus_en)
1827 ctrl->dp_ctrl.pixel_rate >>= 1;
1828
1829 drm_dbg_dp(ctrl->drm_dev, "rate=%d, num_lanes=%d,
pixel_rate=%d\n",
1830 ctrl->link->link_params.rate,
1831 ctrl->link->link_params.num_lanes,
ctrl->dp_ctrl.pixel_rate);
1832
1833 if (!dp_power_clk_status(ctrl->power, DP_CTRL_PM)) { /* link
clk is off */
1834 ret = dp_ctrl_enable_mainlink_clocks(ctrl);
1835 if (ret) {
1836 DRM_ERROR("Failed to start link clocks.
ret=%d\n", ret);
1837 goto end;
1838 }
1839 }
1840
1841 ret = dp_ctrl_enable_stream_clocks(ctrl);
1842 if (ret) {
1843 DRM_ERROR("Failed to start pixel clocks. ret=%d\n",
ret);
1844 goto end;
1845 }
1846
1847 if (ctrl->link->sink_request & DP_TEST_LINK_PHY_TEST_PATTERN) {
1848 dp_ctrl_send_phy_test_pattern(ctrl);
1849 return 0;
1850 }
1851
> 1852 if (force_link_tarin || !dp_ctrl_channel_eq_ok(ctrl))
1853 dp_ctrl_link_retrain(ctrl);
1854
1855 /* stop txing train pattern to end link training */
1856 dp_ctrl_clear_training_pattern(ctrl);
1857
1858 /*
1859 * Set up transfer unit values and set controller state to send
1860 * video.
1861 */
1862 reinit_completion(&ctrl->video_comp);
1863
1864 dp_ctrl_configure_source_params(ctrl);
1865
1866 dp_catalog_ctrl_config_msa(ctrl->catalog,
1867 ctrl->link->link_params.rate,
1868 pixel_rate_orig, dp_ctrl_use_fixed_nvid(ctrl));
1869
1870 dp_ctrl_setup_tr_unit(ctrl);
1871
1872 dp_catalog_ctrl_state_ctrl(ctrl->catalog,
DP_STATE_CTRL_SEND_VIDEO);
1873
1874 ret = dp_ctr
