Re: [PATCH v2 04/10] drm/aperture: Add infrastructure for aperture ownership
Hi
Am 08.04.21 um 11:48 schrieb Daniel Vetter:
Maybe just me, but to avoid overstretching the attention spawn of doc
readers I'd avoid this example here. And maybe make the recommendation
stronger, e.g. "PCI device drivers can avoid open-coding
remove_conflicting_framebuffers() by calling
drm_fb_helper_remove_conflicting_pci_framebuffers()."
It's a tutorial. In my expectation, everyone just copies the tutorial
code and fills the gaps.
+ *
+ * .. code-block:: c
+ *
+ * static int probe(struct pci_dev *pdev)
+ * {
+ * int ret;
+ *
+ * // Remove any generic drivers...
+ * ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev,
"example driver");
+ * if (ret)
+ * return ret;
+ *
+ * // ... and initialize the hardware.
+ * ...
+ *
+ * drm_dev_register();
+ *
+ * return 0;
+ * }
+ *
+ * Drivers that are susceptible to being removed be other drivers, such as
+ * generic EFI or VESA drivers, have to register themselves as owners of their
+ * given framebuffer memory. Ownership of the framebuffer memory is achived
+ * by calling devm_aperture_acquire(). On success, the driver is the owner
+ * of the framebuffer range. The function fails if the framebuffer is already
+ * by another driver. See below for an example.
+ *
+ * .. code-block:: c
+ *
+ * static struct drm_aperture_funcs ap_funcs = {
+ * .detach = ...
Is there really value in allowing/forcing drivers to set up their own
detach ops? You already make this specific to struct drm_device, an
implementation that just calls drm_dev_unplug feels like the right thing
to do?
Is it that easy? simepldrm's detach function has code to synchronize
with concurrent hotplug removals. If we can use drm_dev_unplug() for
everything, I'm all for it.
Best regards
Thomas
Or maybe we should tie this more into the struct device mode and force an
unload that way? That way devm cleanup would work as one expects, and
avoid the need for anything specific (hopefully) in this detach callback.
Just feels a bit like we're reinventing half of the driver model here,
badly.
+ * };
+ *
+ * static int acquire_framebuffers(struct drm_device *dev, struct pci_dev
*pdev)
+ * {
+ * resource_size_t start, len;
+ * struct drm_aperture *ap;
+ *
+ * base = pci_resource_start(pdev, 0);
+ * size = pci_resource_len(pdev, 0);
+ *
+ * ap = devm_acquire_aperture(dev, base, size, &ap_funcs);
+ * if (IS_ERR(ap))
+ * return PTR_ERR(ap);
+ *
+ * return 0;
+ * }
+ *
+ * static int probe(struct pci_dev *pdev)
+ * {
+ * struct drm_device *dev;
+ * int ret;
+ *
+ * // ... Initialize the device...
+ * dev = devm_drm_dev_alloc();
+ * ...
+ *
+ * // ... and acquire ownership of the framebuffer.
+ * ret = acquire_framebuffers(dev, pdev);
+ * if (ret)
+ * return ret;
+ *
+ * drm_dev_register();
+ *
+ * return 0;
+ * }
+ *
+ * The generic driver is now subject to forced removal by other drivers. This
+ * is when the detach function in struct &drm_aperture_funcs comes into play.
+ * When a driver calls drm_fb_helper_remove_conflicting_framebuffers() et al
+ * for the registered framebuffer range, the DRM core calls struct
+ * &drm_aperture_funcs.detach and the generic driver has to onload itself. It
+ * may not access the device's registers, framebuffer memory, ROM, etc after
+ * detach returned. If the driver supports hotplugging, detach can be treated
+ * like an unplug event.
+ *
+ * .. code-block:: c
+ *
+ * static void detach_from_device(struct drm_device *dev,
+ *resource_size_t base,
+ *resource_size_t size)
+ * {
+ * // Signal unplug
+ * drm_dev_unplug(dev);
+ *
+ * // Maybe do other clean-up operations
+ * ...
+ * }
+ *
+ * static struct drm_aperture_funcs ap_funcs = {
+ * .detach = detach_from_device,
+ * };
+ */
+
+/**
+ * struct drm_aperture - Represents a DRM framebuffer aperture
+ *
+ * This structure has no public fields.
+ */
+struct drm_aperture {
+ struct drm_device *dev;
+ resource_size_t base;
+ resource_size_t size;
+
+ const struct drm_aperture_funcs *funcs;
+
+ struct list_head lh;
+};
+
+static LIST_HEAD(drm_apertures);
+
+static DEFINE_MUTEX(drm_apertures_lock);
+
+static bool overlap(resource_size_t base1, resource_size_t end1,
+ resource_size_t base2, resource_size_t end2)
+{
+ return (base1 < end2) && (end1 > base2);
+}
+
+static void devm_aperture_acquire_release(void *data)
+{
+ struct drm_aperture *ap = data;
+ bool detached = !ap->dev;
+
Re: [PATCH v2 03/10] drm/aperture: Move fbdev conflict helpers into drm_aperture.h
Hi
Am 08.04.21 um 11:50 schrieb Daniel Vetter:
On Thu, Mar 18, 2021 at 11:29:14AM +0100, Thomas Zimmermann wrote:
Fbdev's helpers for handling conflicting framebuffers are related to
framebuffer apertures, not console emulation. Therefore move them into a
drm_aperture.h, which will contain the interfaces for the new aperture
helpers.
Signed-off-by: Thomas Zimmermann
Tested-by: nerdopolis
---
Documentation/gpu/drm-internals.rst | 6 +++
include/drm/drm_aperture.h | 60 +
include/drm/drm_fb_helper.h | 56 ++-
3 files changed, 69 insertions(+), 53 deletions(-)
create mode 100644 include/drm/drm_aperture.h
diff --git a/Documentation/gpu/drm-internals.rst
b/Documentation/gpu/drm-internals.rst
index 12272b168580..4c7642d2ca34 100644
--- a/Documentation/gpu/drm-internals.rst
+++ b/Documentation/gpu/drm-internals.rst
@@ -75,6 +75,12 @@ update it, its value is mostly useless. The DRM core prints
it to the
kernel log at initialization time and passes it to userspace through the
DRM_IOCTL_VERSION ioctl.
+Managing Ownership of the Framebuffer Aperture
+--
+
+.. kernel-doc:: include/drm/drm_aperture.h
+ :internal:
+
Device Instance and Driver Handling
---
diff --git a/include/drm/drm_aperture.h b/include/drm/drm_aperture.h
new file mode 100644
index ..13766efe9517
--- /dev/null
+++ b/include/drm/drm_aperture.h
@@ -0,0 +1,60 @@
+/* SPDX-License-Identifier: MIT */
+
+#ifndef _DRM_APERTURE_H_
+#define _DRM_APERTURE_H_
+
+#include
+#include
+
+/**
+ * drm_fb_helper_remove_conflicting_framebuffers - remove firmware-configured
framebuffers
Annoying bikeshed, but I'd give them drm_aperture_ prefixes, for ocd
consistency. Also make them real functions, they're quite big and will
grow more in the next patch.
I'm also not super happy about the naming here but oh well.
The original name for this was platform helpers, which was worse. So
it's not like we're not improving. :)
I'll take this patch + some docs from patch 4 + your feedback and turn
it into a separate patchset. It should be useful even without simpledrm.
Best regards
Thomas
Either way: Acked-by: Daniel Vetter
+ * @a: memory range, users of which are to be removed
+ * @name: requesting driver name
+ * @primary: also kick vga16fb if present
+ *
+ * This function removes framebuffer devices (initialized by
firmware/bootloader)
+ * which use memory range described by @a. If @a is NULL all such devices are
+ * removed.
+ */
+static inline int
+drm_fb_helper_remove_conflicting_framebuffers(struct apertures_struct *a,
+ const char *name, bool primary)
+{
+#if IS_REACHABLE(CONFIG_FB)
+ return remove_conflicting_framebuffers(a, name, primary);
+#else
+ return 0;
+#endif
+}
+
+/**
+ * drm_fb_helper_remove_conflicting_pci_framebuffers - remove
firmware-configured
+ * framebuffers for PCI
devices
+ * @pdev: PCI device
+ * @name: requesting driver name
+ *
+ * This function removes framebuffer devices (eg. initialized by firmware)
+ * using memory range configured for any of @pdev's memory bars.
+ *
+ * The function assumes that PCI device with shadowed ROM drives a primary
+ * display and so kicks out vga16fb.
+ */
+static inline int
+drm_fb_helper_remove_conflicting_pci_framebuffers(struct pci_dev *pdev,
+ const char *name)
+{
+ int ret = 0;
+
+ /*
+* WARNING: Apparently we must kick fbdev drivers before vgacon,
+* otherwise the vga fbdev driver falls over.
+*/
+#if IS_REACHABLE(CONFIG_FB)
+ ret = remove_conflicting_pci_framebuffers(pdev, name);
+#endif
+ if (ret == 0)
+ ret = vga_remove_vgacon(pdev);
+ return ret;
+}
+
+#endif
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 3b273f9ca39a..d06a3942fddb 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -30,13 +30,13 @@
#ifndef DRM_FB_HELPER_H
#define DRM_FB_HELPER_H
-struct drm_fb_helper;
-
+#include
#include
#include
#include
#include
-#include
+
+struct drm_fb_helper;
enum mode_set_atomic {
LEAVE_ATOMIC_MODE_SET,
@@ -451,54 +451,4 @@ drm_fbdev_generic_setup(struct drm_device *dev, unsigned
int preferred_bpp)
#endif
-/**
- * drm_fb_helper_remove_conflicting_framebuffers - remove firmware-configured
framebuffers
- * @a: memory range, users of which are to be removed
- * @name: requesting driver name
- * @primary: also kick vga16fb if present
- *
- * This function removes framebuffer devices (initialized by
firmware/bootloader)
- * which use memory range described by @a. If @a is NULL all such devices are
- * removed.
- */
-static inline int
-drm_fb_helper_remove_conflicting_framebuffers(struct ape
Re: [PATCH 01/18] drm: Introduce new HDMI helpers
Hi
with my comments addressed:
Acked-by: Thomas Zimmermann
Am 17.03.21 um 16:43 schrieb Maxime Ripard:
The new bridge rework to support the input and output formats introduced
some boilerplate code that will need to be shared across drivers.
Since dw-hdmi is the only driver so far, let's introduce those helpers
based on that code.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/Makefile | 2 +-
drivers/gpu/drm/drm_hdmi.c | 170 +
include/drm/drm_hdmi.h | 24 ++
3 files changed, 195 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/drm_hdmi.c
create mode 100644 include/drm/drm_hdmi.h
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 5eb5bf7c16e3..1b77bd64a37e 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -17,7 +17,7 @@ drm-y :=drm_auth.o drm_cache.o \
drm_plane.o drm_color_mgmt.o drm_print.o \
drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \
drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o \
- drm_client_modeset.o drm_atomic_uapi.o drm_hdcp.o \
+ drm_client_modeset.o drm_atomic_uapi.o drm_hdmi.o drm_hdcp.o \
drm_managed.o drm_vblank_work.o
drm-$(CONFIG_DRM_LEGACY) += drm_bufs.o drm_context.o drm_dma.o drm_legacy_misc.o drm_lock.o \
diff --git a/drivers/gpu/drm/drm_hdmi.c b/drivers/gpu/drm/drm_hdmi.c
new file mode 100644
index ..3834d5dd6d88
--- /dev/null
+++ b/drivers/gpu/drm/drm_hdmi.c
@@ -0,0 +1,170 @@
The SPDX tag is missing from this file.
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+/**
+ * drm_hdmi_bus_fmt_is_rgb() - Is the media bus format an RGB format?
+ * @bus_format: MEDIA_BUS_FMT* to test
+ *
+ * Checks if the media bus format is an RGB one
+ *
+ * RETURNS:
Just a question on this. I always use 'Returns:' Is this supposed to be
in capital letters? And does it make a difference?
+ * True if the format is an RGB one, false otherwise
+ */
+bool drm_hdmi_bus_fmt_is_rgb(u32 bus_format)
+{
+ switch (bus_format) {
+ case MEDIA_BUS_FMT_RGB888_1X24:
+ case MEDIA_BUS_FMT_RGB101010_1X30:
+ case MEDIA_BUS_FMT_RGB121212_1X36:
+ case MEDIA_BUS_FMT_RGB161616_1X48:
+ return true;
+
No empty line here and in similar places.
+ default:
+ return false;
+ }
+}
+EXPORT_SYMBOL(drm_hdmi_bus_fmt_is_rgb);
+
+/**
+ * drm_hdmi_bus_fmt_is_yuv444() - Is the media bus format an YUV444 format?
+ * @bus_format: MEDIA_BUS_FMT* to test
+ *
+ * Checks if the media bus format is an YUV444 one
+ *
+ * RETURNS:
+ * True if the format is an YUV444 one, false otherwise
+ */
+bool drm_hdmi_bus_fmt_is_yuv444(u32 bus_format)
+{
+ switch (bus_format) {
+ case MEDIA_BUS_FMT_YUV8_1X24:
+ case MEDIA_BUS_FMT_YUV10_1X30:
+ case MEDIA_BUS_FMT_YUV12_1X36:
+ case MEDIA_BUS_FMT_YUV16_1X48:
+ return true;
+
+ default:
+ return false;
+ }
+}
+EXPORT_SYMBOL(drm_hdmi_bus_fmt_is_yuv444);
+
+/**
+ * drm_hdmi_bus_fmt_is_yuv422() - Is the media bus format an YUV422 format?
+ * @bus_format: MEDIA_BUS_FMT* to test
+ *
+ * Checks if the media bus format is an YUV422 one
+ *
+ * RETURNS:
+ * True if the format is an YUV422 one, false otherwise
+ */
+bool drm_hdmi_bus_fmt_is_yuv422(u32 bus_format)
+{
+ switch (bus_format) {
+ case MEDIA_BUS_FMT_UYVY8_1X16:
+ case MEDIA_BUS_FMT_UYVY10_1X20:
+ case MEDIA_BUS_FMT_UYVY12_1X24:
+ return true;
+
+ default:
+ return false;
+ }
+}
+EXPORT_SYMBOL(drm_hdmi_bus_fmt_is_yuv422);
+
+/**
+ * drm_hdmi_bus_fmt_is_yuv420() - Is the media bus format an YUV420 format?
+ * @bus_format: MEDIA_BUS_FMT* to test
+ *
+ * Checks if the media bus format is an YUV420 one
+ *
+ * RETURNS:
+ * True if the format is an YUV420 one, false otherwise
+ */
+bool drm_hdmi_bus_fmt_is_yuv420(u32 bus_format)
+{
+ switch (bus_format) {
+ case MEDIA_BUS_FMT_UYYVYY8_0_5X24:
+ case MEDIA_BUS_FMT_UYYVYY10_0_5X30:
+ case MEDIA_BUS_FMT_UYYVYY12_0_5X36:
+ case MEDIA_BUS_FMT_UYYVYY16_0_5X48:
+ return true;
+
+ default:
+ return false;
+ }
+}
+EXPORT_SYMBOL(drm_hdmi_bus_fmt_is_yuv420);
+
+/**
+ * drm_hdmi_bus_fmt_color_depth() - Returns the color depth in bits
+ * @bus_format: MEDIA_BUS_FMT* to test
+ *
+ * Computes the number of bits per color for a given media bus format
+ *
+ * RETURNS:
+ * The number of bits per color
+ */
+int drm_hdmi_bus_fmt_color_depth(u32 bus_format)
+{
+ switch (bus_format) {
+ case MEDIA_BUS_FMT_RGB888_1X24:
+ case MEDIA_BUS_FMT_YUV8_1X24:
+ case MEDIA_BUS_FMT_UYVY8_1X16:
+ case MEDIA_BUS_FMT_UYYVYY8_0_5X24:
+ return 8;
+
+ case MEDIA_BUS_FMT_RGB101010_1X30:
+ case MEDIA_BUS_FMT_YUV10_1X30:
+ case MEDIA_BU
[PATCH 1/2] mm/vmscan: add sync_shrinkers function
To be able to switch to a spinlock and reduce lock contention in the TTM
shrinker we don't want to hold a mutex while unmapping and freeing pages
from the pool.
But then we somehow need to prevent a race between (for example) the shrinker
trying to free pages and hotplug trying to remove the device which those pages
belong to.
Taking and releasing the shrinker semaphore on the write side after
unmapping and freeing all pages should make sure that no shrinker is running in
paralell any more.
Signed-off-by: Christian König
---
include/linux/shrinker.h | 1 +
mm/vmscan.c | 10 ++
2 files changed, 11 insertions(+)
diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h
index 0f80123650e2..6b75dc372fce 100644
--- a/include/linux/shrinker.h
+++ b/include/linux/shrinker.h
@@ -92,4 +92,5 @@ extern void register_shrinker_prepared(struct shrinker
*shrinker);
extern int register_shrinker(struct shrinker *shrinker);
extern void unregister_shrinker(struct shrinker *shrinker);
extern void free_prealloced_shrinker(struct shrinker *shrinker);
+extern void sync_shrinkers(void);
#endif
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 562e87cbd7a1..46cd9c215d73 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -408,6 +408,16 @@ void unregister_shrinker(struct shrinker *shrinker)
}
EXPORT_SYMBOL(unregister_shrinker);
+/**
+ * sync_shrinker - Wait for all running shrinkers to complete.
+ */
+void sync_shrinkers(void)
+{
+ down_write(&shrinker_rwsem);
+ up_write(&shrinker_rwsem);
+}
+EXPORT_SYMBOL(sync_shrinkers);
+
#define SHRINK_BATCH 128
static unsigned long do_shrink_slab(struct shrink_control *shrinkctl,
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/ttm: optimize the pool shrinker a bit
Switch back to using a spinlock again by moving the IOMMU unmap outside
of the locked region.
Signed-off-by: Christian König
---
drivers/gpu/drm/ttm/ttm_pool.c | 40 +++---
1 file changed, 18 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
index cb38b1a17b09..a8b4abe687ce 100644
--- a/drivers/gpu/drm/ttm/ttm_pool.c
+++ b/drivers/gpu/drm/ttm/ttm_pool.c
@@ -70,7 +70,7 @@ static struct ttm_pool_type global_uncached[MAX_ORDER];
static struct ttm_pool_type global_dma32_write_combined[MAX_ORDER];
static struct ttm_pool_type global_dma32_uncached[MAX_ORDER];
-static struct mutex shrinker_lock;
+static spinlock_t shrinker_lock;
static struct list_head shrinker_list;
static struct shrinker mm_shrinker;
@@ -263,9 +263,9 @@ static void ttm_pool_type_init(struct ttm_pool_type *pt,
struct ttm_pool *pool,
spin_lock_init(&pt->lock);
INIT_LIST_HEAD(&pt->pages);
- mutex_lock(&shrinker_lock);
+ spin_lock(&shrinker_lock);
list_add_tail(&pt->shrinker_list, &shrinker_list);
- mutex_unlock(&shrinker_lock);
+ spin_unlock(&shrinker_lock);
}
/* Remove a pool_type from the global shrinker list and free all pages */
@@ -273,9 +273,9 @@ static void ttm_pool_type_fini(struct ttm_pool_type *pt)
{
struct page *p;
- mutex_lock(&shrinker_lock);
+ spin_lock(&shrinker_lock);
list_del(&pt->shrinker_list);
- mutex_unlock(&shrinker_lock);
+ spin_unlock(&shrinker_lock);
while ((p = ttm_pool_type_take(pt)))
ttm_pool_free_page(pt->pool, pt->caching, pt->order, p);
@@ -313,24 +313,19 @@ static struct ttm_pool_type *ttm_pool_select_type(struct
ttm_pool *pool,
static unsigned int ttm_pool_shrink(void)
{
struct ttm_pool_type *pt;
- unsigned int num_freed;
struct page *p;
- mutex_lock(&shrinker_lock);
+ spin_lock(&shrinker_lock);
pt = list_first_entry(&shrinker_list, typeof(*pt), shrinker_list);
+ list_move_tail(&pt->shrinker_list, &shrinker_list);
+ spin_unlock(&shrinker_lock);
p = ttm_pool_type_take(pt);
- if (p) {
- ttm_pool_free_page(pt->pool, pt->caching, pt->order, p);
- num_freed = 1 << pt->order;
- } else {
- num_freed = 0;
- }
-
- list_move_tail(&pt->shrinker_list, &shrinker_list);
- mutex_unlock(&shrinker_lock);
+ if (!p)
+ return 0;
- return num_freed;
+ ttm_pool_free_page(pt->pool, pt->caching, pt->order, p);
+ return 1 << pt->order;
}
/* Return the allocation order based for a page */
@@ -530,6 +525,7 @@ void ttm_pool_fini(struct ttm_pool *pool)
for (j = 0; j < MAX_ORDER; ++j)
ttm_pool_type_fini(&pool->caching[i].orders[j]);
}
+ sync_shrinkers();
}
/* As long as pages are available make sure to release at least one */
@@ -604,7 +600,7 @@ static int ttm_pool_debugfs_globals_show(struct seq_file
*m, void *data)
{
ttm_pool_debugfs_header(m);
- mutex_lock(&shrinker_lock);
+ spin_lock(&shrinker_lock);
seq_puts(m, "wc\t:");
ttm_pool_debugfs_orders(global_write_combined, m);
seq_puts(m, "uc\t:");
@@ -613,7 +609,7 @@ static int ttm_pool_debugfs_globals_show(struct seq_file
*m, void *data)
ttm_pool_debugfs_orders(global_dma32_write_combined, m);
seq_puts(m, "uc 32\t:");
ttm_pool_debugfs_orders(global_dma32_uncached, m);
- mutex_unlock(&shrinker_lock);
+ spin_unlock(&shrinker_lock);
ttm_pool_debugfs_footer(m);
@@ -640,7 +636,7 @@ int ttm_pool_debugfs(struct ttm_pool *pool, struct seq_file
*m)
ttm_pool_debugfs_header(m);
- mutex_lock(&shrinker_lock);
+ spin_lock(&shrinker_lock);
for (i = 0; i < TTM_NUM_CACHING_TYPES; ++i) {
seq_puts(m, "DMA ");
switch (i) {
@@ -656,7 +652,7 @@ int ttm_pool_debugfs(struct ttm_pool *pool, struct seq_file
*m)
}
ttm_pool_debugfs_orders(pool->caching[i].orders, m);
}
- mutex_unlock(&shrinker_lock);
+ spin_unlock(&shrinker_lock);
ttm_pool_debugfs_footer(m);
return 0;
@@ -693,7 +689,7 @@ int ttm_pool_mgr_init(unsigned long num_pages)
if (!page_pool_size)
page_pool_size = num_pages;
- mutex_init(&shrinker_lock);
+ spin_lock_init(&shrinker_lock);
INIT_LIST_HEAD(&shrinker_list);
for (i = 0; i < MAX_ORDER; ++i) {
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/vmwgfx: Make sure unpinning handles reservations
Hi, Zack,
On 4/8/21 7:22 PM, Zack Rusin wrote:
Quite often it's a little hard to tell if reservations are already held
in code paths that unpin bo's. While our pinning/unpinning code should
be more explicit that requires a substential amount of work so instead
we can avoid the issues by making sure we try to reserve before unpinning.
Because we unpin those bo's only on destruction/error paths just that check
tells us if we're already reserved or not and allows to cleanly unpin.
Reviewed-by: Martin Krastev
Reviewed-by: Roland Scheidegger
Fixes: d1a73c641afd ("drm/vmwgfx: Make sure we unpin no longer needed buffers")
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Zack Rusin
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 17 -
drivers/gpu/drm/vmwgfx/vmwgfx_mob.c | 8
2 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 8087a9013455..03bef9c17e56 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -1517,6 +1517,21 @@ static inline struct vmw_surface
*vmw_surface_reference(struct vmw_surface *srf)
return srf;
}
+/*
+ * vmw_bo_unpin_safe - currently pinning requires a reservation to be held
+ * but sometimes it's hard to tell if we're in a callback whose parent
+ * is already holding a reservation, to avoid deadloacks we have to try
+ * to get a reservation explicitly to also try to avoid messing up the
+ * internal ttm lru bo list
+ */
+static inline void vmw_bo_unpin_safe(struct ttm_buffer_object *bo)
+{
+ bool locked = dma_resv_trylock(bo->base.resv);
Isn't there a chance another thread is holding the lock and releasing it
at this position?
+ ttm_bo_unpin(bo);
+ if (locked)
+ dma_resv_unlock(bo->base.resv);
+}
+
static inline void vmw_bo_unreference(struct vmw_buffer_object **buf)
{
struct vmw_buffer_object *tmp_buf = *buf;
@@ -1524,7 +1539,7 @@ static inline void vmw_bo_unreference(struct
vmw_buffer_object **buf)
*buf = NULL;
if (tmp_buf != NULL) {
if (tmp_buf->base.pin_count > 0)
- ttm_bo_unpin(&tmp_buf->base);
+ vmw_bo_unpin_safe(&tmp_buf->base);
Hmm. If execbuf is referencing a buffer that someone else has pinned,
wouldn't execbuf incorrectly unpin that buffer when calling unreference?
Would it perhaps be possible to if needed, use the TTM release_notify
callback to unpin any leaking pins similar to what's done in
ttm_bo_release? Although that I guess goes somewhat against that
recently added WARN_ON_ONCE.
ttm_bo_put(&tmp_buf->base);
}
}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
index a0b53141dded..23ffeb2dd6e0 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
@@ -277,7 +277,7 @@ static int vmw_otable_batch_setup(struct vmw_private
*dev_priv,
&batch->otables[i]);
}
- ttm_bo_unpin(batch->otable_bo);
+ vmw_bo_unpin_safe(batch->otable_bo);
Could it be we're the only user here? If so safe to reserve and unpin.
ttm_bo_put(batch->otable_bo);
batch->otable_bo = NULL;
return ret;
@@ -343,7 +343,7 @@ static void vmw_otable_batch_takedown(struct vmw_private
*dev_priv,
vmw_bo_fence_single(bo, NULL);
ttm_bo_unreserve(bo);
- ttm_bo_unpin(batch->otable_bo);
+ vmw_bo_unpin_safe(batch->otable_bo);
Would it be possible to just move ttm_bo_unpin() above the
ttm_bo_unreserve() above?
ttm_bo_put(batch->otable_bo);
batch->otable_bo = NULL;
}
@@ -530,7 +530,7 @@ static void vmw_mob_pt_setup(struct vmw_mob *mob,
void vmw_mob_destroy(struct vmw_mob *mob)
{
if (mob->pt_bo) {
- ttm_bo_unpin(mob->pt_bo);
+ vmw_bo_unpin_safe(mob->pt_bo);
ttm_bo_put(mob->pt_bo);
mob->pt_bo = NULL;
}
@@ -646,7 +646,7 @@ int vmw_mob_bind(struct vmw_private *dev_priv,
out_no_cmd_space:
vmw_fifo_resource_dec(dev_priv);
if (pt_set_up) {
- ttm_bo_unpin(mob->pt_bo);
+ vmw_bo_unpin_safe(mob->pt_bo);
Perhaps the same here?
ttm_bo_put(mob->pt_bo);
mob->pt_bo = NULL;
}
/Thomas
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/vmwgfx: Make sure unpinning handles reservations
On Thu, Apr 08, 2021 at 01:22:45PM -0400, Zack Rusin wrote:
> Quite often it's a little hard to tell if reservations are already held
> in code paths that unpin bo's. While our pinning/unpinning code should
> be more explicit that requires a substential amount of work so instead
> we can avoid the issues by making sure we try to reserve before unpinning.
> Because we unpin those bo's only on destruction/error paths just that check
> tells us if we're already reserved or not and allows to cleanly unpin.
>
> Reviewed-by: Martin Krastev
> Reviewed-by: Roland Scheidegger
> Fixes: d1a73c641afd ("drm/vmwgfx: Make sure we unpin no longer needed
> buffers")
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Zack Rusin
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 17 -
> drivers/gpu/drm/vmwgfx/vmwgfx_mob.c | 8
> 2 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> index 8087a9013455..03bef9c17e56 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> @@ -1517,6 +1517,21 @@ static inline struct vmw_surface
> *vmw_surface_reference(struct vmw_surface *srf)
> return srf;
> }
>
> +/*
> + * vmw_bo_unpin_safe - currently pinning requires a reservation to be held
> + * but sometimes it's hard to tell if we're in a callback whose parent
> + * is already holding a reservation, to avoid deadloacks we have to try
> + * to get a reservation explicitly to also try to avoid messing up the
> + * internal ttm lru bo list
> + */
> +static inline void vmw_bo_unpin_safe(struct ttm_buffer_object *bo)
> +{
> + bool locked = dma_resv_trylock(bo->base.resv);
> + ttm_bo_unpin(bo);
> + if (locked)
> + dma_resv_unlock(bo->base.resv);
> +}
> +
> static inline void vmw_bo_unreference(struct vmw_buffer_object **buf)
> {
> struct vmw_buffer_object *tmp_buf = *buf;
> @@ -1524,7 +1539,7 @@ static inline void vmw_bo_unreference(struct
> vmw_buffer_object **buf)
> *buf = NULL;
> if (tmp_buf != NULL) {
> if (tmp_buf->base.pin_count > 0)
> - ttm_bo_unpin(&tmp_buf->base);
> + vmw_bo_unpin_safe(&tmp_buf->base);
So in the unreference callback I understand it might be tricky and you
need this, but do all the others below really don't know whether the bo is
locked or not?
Also _trylock is a bit much yolo locking here, I'd minimally put a comment
there that we don't actually care about races, it's just to shut up ttm
locking checks. Whether that's true or not is another question I think.
And if it's just this case here, maybe inline the trylock, and for the
others do a vmw_bo_unpin_unlocked which unconditionally grabs the lock?
-Daniel
> ttm_bo_put(&tmp_buf->base);
> }
> }
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
> index a0b53141dded..23ffeb2dd6e0 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
> @@ -277,7 +277,7 @@ static int vmw_otable_batch_setup(struct vmw_private
> *dev_priv,
>&batch->otables[i]);
> }
>
> - ttm_bo_unpin(batch->otable_bo);
> + vmw_bo_unpin_safe(batch->otable_bo);
> ttm_bo_put(batch->otable_bo);
> batch->otable_bo = NULL;
> return ret;
> @@ -343,7 +343,7 @@ static void vmw_otable_batch_takedown(struct vmw_private
> *dev_priv,
> vmw_bo_fence_single(bo, NULL);
> ttm_bo_unreserve(bo);
>
> - ttm_bo_unpin(batch->otable_bo);
> + vmw_bo_unpin_safe(batch->otable_bo);
> ttm_bo_put(batch->otable_bo);
> batch->otable_bo = NULL;
> }
> @@ -530,7 +530,7 @@ static void vmw_mob_pt_setup(struct vmw_mob *mob,
> void vmw_mob_destroy(struct vmw_mob *mob)
> {
> if (mob->pt_bo) {
> - ttm_bo_unpin(mob->pt_bo);
> + vmw_bo_unpin_safe(mob->pt_bo);
> ttm_bo_put(mob->pt_bo);
> mob->pt_bo = NULL;
> }
> @@ -646,7 +646,7 @@ int vmw_mob_bind(struct vmw_private *dev_priv,
> out_no_cmd_space:
> vmw_fifo_resource_dec(dev_priv);
> if (pt_set_up) {
> - ttm_bo_unpin(mob->pt_bo);
> + vmw_bo_unpin_safe(mob->pt_bo);
> ttm_bo_put(mob->pt_bo);
> mob->pt_bo = NULL;
> }
> --
> 2.27.0
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: optimize the pool shrinker a bit
On Thu, Apr 08, 2021 at 02:44:16PM +0200, Christian König wrote:
> Am 08.04.21 um 13:31 schrieb Daniel Vetter:
> > On Thu, Apr 08, 2021 at 01:17:32PM +0200, Christian König wrote:
> > > Am 08.04.21 um 13:08 schrieb Daniel Vetter:
> > > > On Thu, Apr 01, 2021 at 03:54:13PM +0200, Christian König wrote:
> > > > > [SNIP]
> > > > >EXPORT_SYMBOL(unregister_shrinker);
> > > > > +/**
> > > > > + * sync_shrinker - Wait for all running shrinkers to complete.
> > > > > + */
> > > > > +void sync_shrinkers(void)
> > > > This one should probably be in its own patch, with a bit more commit
> > > > message about why we need it and all that. I'd assume that just
> > > > unregistering the shrinker should sync everything we needed to sync
> > > > already, and for other sync needs we can do locking within our own
> > > > shrinker?
> > > Correct. Reason why we need the barrier is that we need to destroy the
> > > device (during hotplug) before the shrinker is unregistered (during module
> > > unload).
> > >
> > > Going to separate that, write something up in the commit message and send
> > > it
> > > to the appropriate audience.
> > Hm why do we need that?
>
> When the shrinker runs in parallel with (for example) a hotplug event and
> unmaps pages from the devices IOMMU I must make sure that you can't destroy
> the device or pool structure at the same time.
>
> Previously holding the mutex while updating the IOMMU would take care of
> that, but now we need to prevent this otherwise.
>
> Could be that this is also handled somewhere else, but I'm better save than
> sorry here and grabbing/releasing write side of the shrinker_rwsem is rather
> lightweight.
I forgot that we don't have a per-pool (or at least per-device) shrinker,
but one global one for all ttm device. So yeah with that design a
sync_shrinker is needed.
-Daniel
>
> > Either way sounds like an orthogonal series for
> > the hotunplug work, not just shrinker optimization.
>
> It is unrelated to the hotplug work in general.
>
> Regards,
> Christian.
>
> > -Daniel
> >
> > > Thanks,
> > > Christian.
> > >
> > > > -Daniel
> > > >
> > > > > +{
> > > > > + down_write(&shrinker_rwsem);
> > > > > + up_write(&shrinker_rwsem);
> > > > > +}
> > > > > +EXPORT_SYMBOL(sync_shrinkers);
> > > > > +
> > > > >#define SHRINK_BATCH 128
> > > > >static unsigned long do_shrink_slab(struct shrink_control
> > > > > *shrinkctl,
> > > > > --
> > > > > 2.25.1
> > > > >
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/virtio: Create Dumb BOs as guest Blobs (v2)
Hi, > > IIRC the VIRTGPU_BLOB_FLAG_USE_SHAREABLE flag means that the host *can* > > create a shared mapping (i.e. the host seeing guest-side changes without > > explicit transfer doesn't cause problems for the guest). It doesn not > > mean the host *must* create a shared mapping (note that there is no > > negotiation whenever the host supports shared mappings or not). > > > > VIRTGPU_BLOB_FLAG_USE_SHAREABLE means guest userspace intends to share the > blob resource with another virtgpu driver instance via drmPrimeHandleToFd. > It's a rough analogue to VkExportMemoryAllocateInfoKHR or > PIPE_BIND_USE_SHARED. Oh. My memory was failing me then. We should *really* clarify the spec for BLOB_MEM_GUEST. So shared mappings are allowed for all BLOB_MEM_GUEST resources, right? > > So the transfer calls are still needed, and the host can decide to > > shortcut them in case it can create a shared mapping. In case there is > > no shared mapping (say due to missing udmabuf support) the host can > > fallback to copying. > > Transfers are a bit under-defined for BLOB_MEM_GUEST. Even without udmabuf > on the host, there is no host side resource for guest-only blobs? Before > blob resources, the dumb flow was: > > 1) update guest side resource > 2) TRANSFER_TO_HOST_2D to copy guest side contents to host side private > resource [Pixman??] > 3) RESOURCE_FLUSH to copy the host-side contents to the framebuffer and > page-flip Yes. > At least for crosvm, this is possible: > > 1) update guest side resource > 2) RESOURCE_FLUSH to copy the guest-side contents to the framebuffer and > pageflip > > With implicit udmabuf, it may be possible to do this: > > 1) update guest side resource > 2) RESOURCE_FLUSH to page-flip > > > So I think crosvm should be fixed to not consider transfer commands for > > VIRTGPU_BLOB_MEM_GUEST resources an error. > > It's a simple change to make and we can definitely do it, if TRANSFER_2D is > helpful for the QEMU case. I haven't looked at the QEMU side patches. Well, we have two different cases: (1) No udmabuf available. qemu will have a host-side shadow then and the workflow will be largely identical to the non-blob resource workflow. (2) With udmabuf support. qemu can create udmabufs for the resources, mmap() the dmabuf to get a linear mapping, create a pixman buffer backed by that dmabuf (no copying needed then). Depending on capabilities pass either the pixman image (gl=off) or the dmabuf handle (gl=on) to the UI code to actually show the guest display. The guest doesn't need to know any of this, it'll just send transfer and flush commands. In case (1) qemu must process the transfer commands and for case (2) qemu can simply ignore them. > For the PCI-passthrough + guest blob case, the end goal is to share it with > the host compositor. If there is no guarantee the guest memory can be > converted to an OS-handle (to share with the host compositor), then I think > the guest user space should fallback to another technique involving > memcpy() to share the memory. This is what happens today (using non-blob resources). > So essentially, thinking for two new protocol additions: > > F_CREATE_GUEST_HANDLE (or F_HANDLE_FROM_GUEST) --> means an OS-specific > udmabuf-like mechanism exists on the host. > > BLOB_FLAG_CREATE_GUEST_HANDLE (or BLOB_FLAG_HANDLE_FROM_GUEST)--> tells > host userspace "you must create a udmabuf" [or OS-specific equivalent] upon > success Again: Why do we actually need that? Is there any benefit other than the guest knowing it doesn't need to send transfer commands? I see the whole udmabuf thing as a host-side performance optimization and I think this should be fully transparent to the guest as the host can easily just ignore the transfer commands. Given we batch commands the extra commands don't lead to extra context switches, so there shouldn't be much overhead. If we really want make the guest aware of the hosts udmabuf state I think this should be designed the other way around: Add some way for the host to tell the guest transfer commands are not needed for a specific BLOB_MEM_GUEST resource. take care, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 04/10] drm/aperture: Add infrastructure for aperture ownership
Hi Am 08.04.21 um 11:48 schrieb Daniel Vetter: On Thu, Mar 18, 2021 at 11:29:15AM +0100, Thomas Zimmermann wrote: Platform devices might operate on firmware framebuffers, such as VESA or EFI. Before a native driver for the graphics hardware can take over the device, it has to remove any platform driver that operates on the firmware framebuffer. Aperture helpers provide the infrastructure for platform drivers to acquire firmware framebuffers, and for native drivers to remove them later on. It works similar to the related fbdev mechanism. During initialization, the platform driver acquires the firmware framebuffer's I/O memory and provides a callback to be removed. The native driver later uses this information to remove any platform driver for it's framebuffer I/O memory. The aperture removal code is integrated into the existing code for removing conflicting framebuffers, so native drivers use it automatically. v2: * rename plaform helpers to aperture helpers * tie to device lifetime with devm_ functions * removed unsued remove() callback * rename kickout to detach * make struct drm_aperture private * rebase onto existing drm_aperture.h header file * use MIT license only for simplicity * documentation Signed-off-by: Thomas Zimmermann Tested-by: nerdopolis Bunch of bikesheds for your considerations below, but overall lgtm. Acked-by: Daniel Vetter Cheers, Daniel --- Documentation/gpu/drm-internals.rst | 6 + drivers/gpu/drm/Kconfig | 7 + drivers/gpu/drm/Makefile| 1 + drivers/gpu/drm/drm_aperture.c | 287 include/drm/drm_aperture.h | 38 +++- 5 files changed, 338 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/drm_aperture.c diff --git a/Documentation/gpu/drm-internals.rst b/Documentation/gpu/drm-internals.rst index 4c7642d2ca34..06af044c882f 100644 --- a/Documentation/gpu/drm-internals.rst +++ b/Documentation/gpu/drm-internals.rst @@ -78,9 +78,15 @@ DRM_IOCTL_VERSION ioctl. Managing Ownership of the Framebuffer Aperture -- +.. kernel-doc:: drivers/gpu/drm/drm_aperture.c + :doc: overview + .. kernel-doc:: include/drm/drm_aperture.h :internal: +.. kernel-doc:: drivers/gpu/drm/drm_aperture.c + :export: + Device Instance and Driver Handling --- diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 1461652921be..b9d3fb91d22d 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -221,6 +221,13 @@ config DRM_SCHED tristate depends on DRM +config DRM_APERTURE + bool + depends on DRM + help + Controls ownership of graphics apertures. Required to + synchronize with firmware-based drivers. Uh I'm not a big fan of Kconfig and .ko modules for every little helper code. Imo just stuff this into the drm kms helpers and done. Or stuff it into drm core code, I think either is a good case for this. Everything is its own module means we need to EXPORT_SYMBOL more stuff, and then drivers get funny ideas about using these internals ... The code lives in the DRM core module. There's no extra ko file. But I'd like to keep the Kconfig option. The aperture helpers will only be required if there are generic drivers in the kernel and for many systems this is not the case. Best regards Thomas + source "drivers/gpu/drm/i2c/Kconfig" source "drivers/gpu/drm/arm/Kconfig" diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 5eb5bf7c16e3..c9ecb02df0f3 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -32,6 +32,7 @@ drm-$(CONFIG_AGP) += drm_agpsupport.o drm-$(CONFIG_PCI) += drm_pci.o drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o +drm-$(CONFIG_DRM_APERTURE) += drm_aperture.o drm_vram_helper-y := drm_gem_vram_helper.o obj-$(CONFIG_DRM_VRAM_HELPER) += drm_vram_helper.o diff --git a/drivers/gpu/drm/drm_aperture.c b/drivers/gpu/drm/drm_aperture.c new file mode 100644 index ..4b02b5fed0a1 --- /dev/null +++ b/drivers/gpu/drm/drm_aperture.c @@ -0,0 +1,287 @@ +// SPDX-License-Identifier: MIT + +#include +#include +#include +#include +#include + +#include +#include +#include + +/** + * DOC: overview + * + * A graphics device might be supported by different drivers, but only one + * driver can be active at any given time. Many systems load a generic + * graphics drivers, such as EFI-GOP or VESA, early during the boot process. + * During later boot stages, they replace the generic driver with a dedicated, + * hardware-specific driver. To take over the device the dedicated driver + * first has to remove the generic driver. DRM aperture functions manage + * ownership of DRM framebuffer memory and hand-over between drivers. + * + *
[PATCH -next] drm/hisilicon/kirin: Remove redundant DRM_ERROR call in dsi_parse_dt()
There is a error message within devm_ioremap_resource
already, so remove the DRM_ERROR call to avoid redundant
error message.
Signed-off-by: Chen Hui
---
drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
index 00e87c290796..bc19ce318c62 100644
--- a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
+++ b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
@@ -836,10 +836,8 @@ static int dsi_parse_dt(struct platform_device *pdev,
struct dw_dsi *dsi)
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
ctx->base = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(ctx->base)) {
- DRM_ERROR("failed to remap dsi io region\n");
+ if (IS_ERR(ctx->base))
return PTR_ERR(ctx->base);
- }
return 0;
}
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 02/18] drm/bridge: Add HDMI output fmt helper
Hi
Am 17.03.21 um 16:43 schrieb Maxime Ripard:
The atomic_get_output_bus_fmts bridge callback is there to list the
available formats for output by decreasing order of preference.
On HDMI controllers, we have a fairly static list that will depend on
what the HDMI sink is capable of and the BPC our controller can output.
The dw-hdmi driver already has that code done in a fairly generic
manner, so let's turn that code into an helper for all the HDMI
controllers to reuse.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 127 --
drivers/gpu/drm/drm_bridge.c | 118
include/drm/drm_bridge.h | 6 +
3 files changed, 124 insertions(+), 127 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index dda4fa9a1a08..d010c9c525d9 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2514,133 +2514,6 @@ static int dw_hdmi_connector_create(struct dw_hdmi
*hdmi)
* DRM Bridge Operations
*/
-/*
- * Possible output formats :
- * - MEDIA_BUS_FMT_UYYVYY16_0_5X48,
- * - MEDIA_BUS_FMT_UYYVYY12_0_5X36,
- * - MEDIA_BUS_FMT_UYYVYY10_0_5X30,
- * - MEDIA_BUS_FMT_UYYVYY8_0_5X24,
- * - MEDIA_BUS_FMT_YUV16_1X48,
- * - MEDIA_BUS_FMT_RGB161616_1X48,
- * - MEDIA_BUS_FMT_UYVY12_1X24,
- * - MEDIA_BUS_FMT_YUV12_1X36,
- * - MEDIA_BUS_FMT_RGB121212_1X36,
- * - MEDIA_BUS_FMT_UYVY10_1X20,
- * - MEDIA_BUS_FMT_YUV10_1X30,
- * - MEDIA_BUS_FMT_RGB101010_1X30,
- * - MEDIA_BUS_FMT_UYVY8_1X16,
- * - MEDIA_BUS_FMT_YUV8_1X24,
- * - MEDIA_BUS_FMT_RGB888_1X24,
- */
-
-/* Can return a maximum of 11 possible output formats for a mode/connector */
-#define MAX_OUTPUT_SEL_FORMATS 11
-
-static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge
*bridge,
- struct drm_bridge_state *bridge_state,
- struct drm_crtc_state *crtc_state,
- struct drm_connector_state *conn_state,
- unsigned int *num_output_fmts)
-{
- struct drm_connector *conn = conn_state->connector;
- struct drm_display_info *info = &conn->display_info;
- struct drm_display_mode *mode = &crtc_state->mode;
- u8 max_bpc = conn_state->max_requested_bpc;
- bool is_hdmi2_sink = info->hdmi.scdc.supported ||
-(info->color_formats & DRM_COLOR_FORMAT_YCRCB420);
- u32 *output_fmts;
- unsigned int i = 0;
-
- *num_output_fmts = 0;
-
- output_fmts = kcalloc(MAX_OUTPUT_SEL_FORMATS, sizeof(*output_fmts),
- GFP_KERNEL);
- if (!output_fmts)
- return NULL;
-
- /* If dw-hdmi is the only bridge, avoid negociating with ourselves */
- if (list_is_singular(&bridge->encoder->bridge_chain)) {
- *num_output_fmts = 1;
- output_fmts[0] = MEDIA_BUS_FMT_FIXED;
-
- return output_fmts;
- }
-
- /*
-* If the current mode enforces 4:2:0, force the output but format
-* to 4:2:0 and do not add the YUV422/444/RGB formats
-*/
- if (conn->ycbcr_420_allowed &&
- (drm_mode_is_420_only(info, mode) ||
-(is_hdmi2_sink && drm_mode_is_420_also(info, mode {
-
- /* Order bus formats from 16bit to 8bit if supported */
- if (max_bpc >= 16 && info->bpc == 16 &&
- (info->hdmi.y420_dc_modes & DRM_EDID_YCBCR420_DC_48))
- output_fmts[i++] = MEDIA_BUS_FMT_UYYVYY16_0_5X48;
-
- if (max_bpc >= 12 && info->bpc >= 12 &&
- (info->hdmi.y420_dc_modes & DRM_EDID_YCBCR420_DC_36))
- output_fmts[i++] = MEDIA_BUS_FMT_UYYVYY12_0_5X36;
-
- if (max_bpc >= 10 && info->bpc >= 10 &&
- (info->hdmi.y420_dc_modes & DRM_EDID_YCBCR420_DC_30))
- output_fmts[i++] = MEDIA_BUS_FMT_UYYVYY10_0_5X30;
-
- /* Default 8bit fallback */
- output_fmts[i++] = MEDIA_BUS_FMT_UYYVYY8_0_5X24;
-
- *num_output_fmts = i;
-
- return output_fmts;
- }
-
- /*
-* Order bus formats from 16bit to 8bit and from YUV422 to RGB
-* if supported. In any case the default RGB888 format is added
-*/
-
- if (max_bpc >= 16 && info->bpc == 16) {
- if (info->color_formats & DRM_COLOR_FORMAT_YCRCB444)
- output_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
-
- output_fmts[i++] = MEDIA_BUS_FMT_RGB161616_1X48;
- }
-
- if (max_bpc >= 12 && info->bpc >= 12) {
- if (info->color_formats & DRM_COLOR_FORMAT_YCRCB422)
- output_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
-
- if (info->color_formats & DRM_COLOR_FORMAT_YCRCB444)
Re: [PATCH 03/18] drm/bridge: dw-hdmi: Use helpers
There was some objection in patch 2, but at least this conversion patch
looks good.
Acked-by: Thomas Zimmremann
Am 17.03.21 um 16:43 schrieb Maxime Ripard:
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 141 +-
1 file changed, 28 insertions(+), 113 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index d010c9c525d9..39b380453183 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -801,92 +802,6 @@ void dw_hdmi_audio_disable(struct dw_hdmi *hdmi)
}
EXPORT_SYMBOL_GPL(dw_hdmi_audio_disable);
-static bool hdmi_bus_fmt_is_rgb(unsigned int bus_format)
-{
- switch (bus_format) {
- case MEDIA_BUS_FMT_RGB888_1X24:
- case MEDIA_BUS_FMT_RGB101010_1X30:
- case MEDIA_BUS_FMT_RGB121212_1X36:
- case MEDIA_BUS_FMT_RGB161616_1X48:
- return true;
-
- default:
- return false;
- }
-}
-
-static bool hdmi_bus_fmt_is_yuv444(unsigned int bus_format)
-{
- switch (bus_format) {
- case MEDIA_BUS_FMT_YUV8_1X24:
- case MEDIA_BUS_FMT_YUV10_1X30:
- case MEDIA_BUS_FMT_YUV12_1X36:
- case MEDIA_BUS_FMT_YUV16_1X48:
- return true;
-
- default:
- return false;
- }
-}
-
-static bool hdmi_bus_fmt_is_yuv422(unsigned int bus_format)
-{
- switch (bus_format) {
- case MEDIA_BUS_FMT_UYVY8_1X16:
- case MEDIA_BUS_FMT_UYVY10_1X20:
- case MEDIA_BUS_FMT_UYVY12_1X24:
- return true;
-
- default:
- return false;
- }
-}
-
-static bool hdmi_bus_fmt_is_yuv420(unsigned int bus_format)
-{
- switch (bus_format) {
- case MEDIA_BUS_FMT_UYYVYY8_0_5X24:
- case MEDIA_BUS_FMT_UYYVYY10_0_5X30:
- case MEDIA_BUS_FMT_UYYVYY12_0_5X36:
- case MEDIA_BUS_FMT_UYYVYY16_0_5X48:
- return true;
-
- default:
- return false;
- }
-}
-
-static int hdmi_bus_fmt_color_depth(unsigned int bus_format)
-{
- switch (bus_format) {
- case MEDIA_BUS_FMT_RGB888_1X24:
- case MEDIA_BUS_FMT_YUV8_1X24:
- case MEDIA_BUS_FMT_UYVY8_1X16:
- case MEDIA_BUS_FMT_UYYVYY8_0_5X24:
- return 8;
-
- case MEDIA_BUS_FMT_RGB101010_1X30:
- case MEDIA_BUS_FMT_YUV10_1X30:
- case MEDIA_BUS_FMT_UYVY10_1X20:
- case MEDIA_BUS_FMT_UYYVYY10_0_5X30:
- return 10;
-
- case MEDIA_BUS_FMT_RGB121212_1X36:
- case MEDIA_BUS_FMT_YUV12_1X36:
- case MEDIA_BUS_FMT_UYVY12_1X24:
- case MEDIA_BUS_FMT_UYYVYY12_0_5X36:
- return 12;
-
- case MEDIA_BUS_FMT_RGB161616_1X48:
- case MEDIA_BUS_FMT_YUV16_1X48:
- case MEDIA_BUS_FMT_UYYVYY16_0_5X48:
- return 16;
-
- default:
- return 0;
- }
-}
-
/*
* this submodule is responsible for the video data synchronization.
* for example, for RGB 4:4:4 input, the data map is defined as
@@ -967,8 +882,8 @@ static int is_color_space_conversion(struct dw_hdmi *hdmi)
struct hdmi_data_info *hdmi_data = &hdmi->hdmi_data;
bool is_input_rgb, is_output_rgb;
- is_input_rgb = hdmi_bus_fmt_is_rgb(hdmi_data->enc_in_bus_format);
- is_output_rgb = hdmi_bus_fmt_is_rgb(hdmi_data->enc_out_bus_format);
+ is_input_rgb = drm_hdmi_bus_fmt_is_rgb(hdmi_data->enc_in_bus_format);
+ is_output_rgb = drm_hdmi_bus_fmt_is_rgb(hdmi_data->enc_out_bus_format);
return (is_input_rgb != is_output_rgb) ||
(is_input_rgb && is_output_rgb && hdmi_data->rgb_limited_range);
@@ -976,11 +891,11 @@ static int is_color_space_conversion(struct dw_hdmi *hdmi)
static int is_color_space_decimation(struct dw_hdmi *hdmi)
{
- if (!hdmi_bus_fmt_is_yuv422(hdmi->hdmi_data.enc_out_bus_format))
+ if (!drm_hdmi_bus_fmt_is_yuv422(hdmi->hdmi_data.enc_out_bus_format))
return 0;
- if (hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_in_bus_format) ||
- hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_in_bus_format))
+ if (drm_hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_in_bus_format) ||
+ drm_hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_in_bus_format))
return 1;
return 0;
@@ -988,11 +903,11 @@ static int is_color_space_decimation(struct dw_hdmi *hdmi)
static int is_color_space_interpolation(struct dw_hdmi *hdmi)
{
- if (!hdmi_bus_fmt_is_yuv422(hdmi->hdmi_data.enc_in_bus_format))
+ if (!drm_hdmi_bus_fmt_is_yuv422(hdmi->hdmi_data.enc_in_bus_format))
return 0;
- if (hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_out_bus_format) ||
- hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format))
+ if (drm_hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_out_bus_format) ||
+
Re: [PATCH v1 2/2] drivers/gpu/drm: don't select DMA_CMA or CMA from aspeed or etnaviv
On 08.04.21 15:19, Arnd Bergmann wrote: On Thu, Apr 8, 2021 at 2:50 PM Linus Walleij wrote: On Thu, Apr 8, 2021 at 2:01 PM David Hildenbrand wrote: This is something you could do using a hidden helper symbol like config DRMA_ASPEED_GFX bool "Aspeed display driver" select DRM_WANT_CMA config DRM_WANT_CMA bool help Select this from any driver that benefits from CMA being enabled config DMA_CMA bool "Use CMA helpers for DRM" default DRM_WANT_CMA Arnd That's precisely what I had first, with an additional "WANT_CMA" -- but looking at the number of such existing options (I was able to spot 1 !) If you do this it probably makes sense to fix a few other drivers Kconfig in the process. It's not just a problem with your driver. "my" drivers: drivers/gpu/drm/mcde/Kconfig drivers/gpu/drm/pl111/Kconfig drivers/gpu/drm/tve200/Kconfig certainly needs this as well, and pretty much anything that is selecting DRM_KMS_CMA_HELPER or DRM_GEM_CMA_HELPER "wants" DMA_CMA. Are there any that don't select either of the helpers and still want CMA? If not, it would be easy to just add default DRM_KMS_CMA_HELPER || DRM_GEM_CMA_HELPER and skipt the extra symbol. That sounds like a reasonable thing to do. I'll look into that. -- Thanks, David / dhildenb ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/18] drm/vc4: txp: Properly set the possible_crtcs mask
Am 17.03.21 um 16:43 schrieb Maxime Ripard:
The current code does a binary or on the possible_crtcs variable of the
s/binary or/binary OR/
I had to read this twice to get it. Otherwise
Acked-by: Thomas Zimmermann
TXP encoder, while we want to set it to that value instead.
Fixes: 39fcb2808376 ("drm/vc4: txp: Turn the TXP into a CRTC of its own")
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_txp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c
index c0122d83b651..2fc7f4b5fa09 100644
--- a/drivers/gpu/drm/vc4/vc4_txp.c
+++ b/drivers/gpu/drm/vc4/vc4_txp.c
@@ -507,7 +507,7 @@ static int vc4_txp_bind(struct device *dev, struct device
*master, void *data)
return ret;
encoder = &txp->connector.encoder;
- encoder->possible_crtcs |= drm_crtc_mask(crtc);
+ encoder->possible_crtcs = drm_crtc_mask(crtc);
ret = devm_request_irq(dev, irq, vc4_txp_interrupt, 0,
dev_name(dev), txp);
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
OpenPGP_signature
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 2/2] drivers/gpu/drm: don't select DMA_CMA or CMA from aspeed or etnaviv
On 08.04.21 22:29, Arnd Bergmann wrote: On Thu, Apr 8, 2021 at 6:45 PM David Hildenbrand wrote: On 08.04.21 14:49, Linus Walleij wrote: On Thu, Apr 8, 2021 at 2:01 PM David Hildenbrand wrote: This is something you could do using a hidden helper symbol like config DRMA_ASPEED_GFX bool "Aspeed display driver" select DRM_WANT_CMA config DRM_WANT_CMA bool help Select this from any driver that benefits from CMA being enabled config DMA_CMA bool "Use CMA helpers for DRM" default DRM_WANT_CMA Arnd That's precisely what I had first, with an additional "WANT_CMA" -- but looking at the number of such existing options (I was able to spot 1 !) If you do this it probably makes sense to fix a few other drivers Kconfig in the process. It's not just a problem with your driver. "my" drivers: :) I actually wanted to convert them to "depends on DMA_CMA" but ran into recursive dependencies ... drivers/gpu/drm/mcde/Kconfig drivers/gpu/drm/pl111/Kconfig drivers/gpu/drm/tve200/Kconfig Right, this is the main problem caused by using 'select' to force-enable symbols that other drivers depend on. Usually, the answer is to be consistent about the use of 'select' and 'depends on', using the former only to enable symbols that are hidden, while using 'depends on' for anything that is an actual build time dependency. I was assuming these are "real" dependencies. Will it also work without DMA_CMA? I think in this case, it is fairly likely to work without DMA_CMA when the probe function gets called during a fresh boot, but fairly likely to fail if it gets called after the system has run for long enough to fragment the free memory. The point of DMA_CMA is to make it work reliably. Right, and even at runtime there is no guarantee that DMA_CMA will do anything -- especially if we don't reserve a CMA region (e.g., "cma=X"). So this really sounds like a "desires DMA_CMA" and achieving that via an additional symbol or via "default y if ..." for DMA_CMA sounds reasonable. Thanks! -- Thanks, David / dhildenb ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 05/18] drm/vc4: crtc: Skip the TXP
Am 17.03.21 um 16:43 schrieb Maxime Ripard:
The vc4_set_crtc_possible_masks is meant to run over all the encoders
and then set their possible_crtcs mask to their associated pixelvalve.
However, since the commit 39fcb2808376 ("drm/vc4: txp: Turn the TXP into
a CRTC of its own"), the TXP has been turned to a CRTC and encoder of
its own, and while it does indeed register an encoder, it no longer has
an associated pixelvalve. The code will thus run over the TXP encoder
and set a bogus possible_crtcs mask, overriding the one set in the TXP
bind function.
In order to fix this, let's skip any virtual encoder.
Fixes: 39fcb2808376 ("drm/vc4: txp: Turn the TXP into a CRTC of its own")
Signed-off-by: Maxime Ripard
dim fixes 39fcb2808376
Fixes: 39fcb2808376 ("drm/vc4: txp: Turn the TXP into a CRTC of its own")
Cc: Maxime Ripard
Cc: Eric Anholt
Cc: Maxime Ripard
Cc: # v5.9+
At least the CC: stable line should be there.
Acked-by: Thomas Zimmermann
---
drivers/gpu/drm/vc4/vc4_crtc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 269390bc586e..f1f2e8cbce79 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -1018,6 +1018,9 @@ static void vc4_set_crtc_possible_masks(struct drm_device
*drm,
struct vc4_encoder *vc4_encoder;
int i;
+ if (encoder->encoder_type == DRM_MODE_ENCODER_VIRTUAL)
+ continue;
+
vc4_encoder = to_vc4_encoder(encoder);
for (i = 0; i < ARRAY_SIZE(pv_data->encoder_types); i++) {
if (vc4_encoder->type == encoder_types[i]) {
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
OpenPGP_signature
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/18] drm/vc4: txp: Properly set the possible_crtcs mask
Am 17.03.21 um 16:43 schrieb Maxime Ripard:
The current code does a binary or on the possible_crtcs variable of the
TXP encoder, while we want to set it to that value instead.
Fixes: 39fcb2808376 ("drm/vc4: txp: Turn the TXP into a CRTC of its own")
Signed-off-by: Maxime Ripard
Cc: # v5.9+
---
drivers/gpu/drm/vc4/vc4_txp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c
index c0122d83b651..2fc7f4b5fa09 100644
--- a/drivers/gpu/drm/vc4/vc4_txp.c
+++ b/drivers/gpu/drm/vc4/vc4_txp.c
@@ -507,7 +507,7 @@ static int vc4_txp_bind(struct device *dev, struct device
*master, void *data)
return ret;
encoder = &txp->connector.encoder;
- encoder->possible_crtcs |= drm_crtc_mask(crtc);
+ encoder->possible_crtcs = drm_crtc_mask(crtc);
ret = devm_request_irq(dev, irq, vc4_txp_interrupt, 0,
dev_name(dev), txp);
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
OpenPGP_signature
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH 1/3] drm/msm/mdp5: Configure PP_SYNC_HEIGHT to double the vtotal
Hi Abhinav, Angelo, Rob, On 4/9/21 2:08 AM, Rob Clark wrote: On Thu, Apr 8, 2021 at 4:16 PM AngeloGioacchino Del Regno wrote: Il gio 8 apr 2021, 21:05 Rob Clark ha scritto: On Wed, Apr 7, 2021 at 12:11 PM AngeloGioacchino Del Regno wrote: Il 07/04/21 20:19, abhin...@codeaurora.org ha scritto: Hi Marijn On 2021-04-06 14:47, Marijn Suijten wrote: Leaving this at a close-to-maximum register value 0xFFF0 means it takes very long for the MDSS to generate a software vsync interrupt when the hardware TE interrupt doesn't arrive. Configuring this to double the vtotal (like some downstream kernels) leads to a frame to take at most twice before the vsync signal, until hardware TE comes up. In this case the hardware interrupt responsible for providing this signal - "disp-te" gpio - is not hooked up to the mdp5 vsync/pp logic at all. This solves severe panel update issues observed on at least the Xperia Loire and Tone series, until said gpio is properly hooked up to an irq. The reason the CONFIG_HEIGHT was at such a high value is to make sure that we always get the TE only from the panel vsync and not false positives coming from the tear check logic itself. Correct, most CAF kernels mention such behaviour in comments, with fallbacks to vtotal*2 for safety or vtotal when an emulated panel does not support hardware TE at all. We don't seem to (need to) support the latter case but one might at some point want to configure the tearcheck logic to emit a signal for every frame, by means of a DT property or automatically when disp-te is not defined. When you say that disp-te gpio is not hooked up, is it something incorrect with the schematic OR panel is not generating the TE correctly? The GPIO is currently not used at all by this kernel driver besides a call to devm_gpiod_get_optional. As mentioned in the cover letter we have patches in progress to hook up this interrupt line to the pp_done infrastructure and complete vsync requests that way instead of waiting for this "simulated" interrupt from the MDP. Currently I have this patch in msm-next-staging but I guess we need to decide in the next day or so whether to drop it or smash in a comment? Marijn, can you please urgently throw a comment in, reminding that these timers are interacting with TE and send a fast V2? Or just reply on list w/ a comment to smash in, if that is easier How about a comment along the lines of: Tearcheck emits a blanking signal every vclks_line * vtotal * 2 ticks on the vsync_clk equating to roughly half the desired panel refresh rate. This is only necessary as stability fallback if interrupts from the panel arrive too late or not at all, but is currently used by default because these panel interrupts are not wired up yet. I'd place this comment right above REG_MDP5_PP_SYNC_CONFIG_VSYNC, and perhaps add a newline after REG_MDP5_PP_SYNC_CONFIG_HEIGHT to make it clear this applies to those two registers specifically. We can also involve MDP5_PP_SYNC_CONFIG_VSYNC_COUNT(vclks_line) in the mix. Then, when the panel TE is wired up we can be smarter about the situation and print warnings when the user has disp-te hooked up but is receiving interrupts from the MDSS block instead of the panel directly (except if incidentally). This likely means that SET_TEAR_ON was not sent to the panel or the GPIO is wrong. In that sense this patch (together with x100 removal) is concealing configuration mistakes, but we might also revert back to 0xfff0 if the GPIO is specified in DT and accept the timeout+recovery which did not seem to hamper devices on downstream kernels anyway. - Marijn ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-misc-fixes
drm-misc-fixes-2021-04-09: drm-misc-fixes for v5.12-rc7: - Fix use-after-free in xen. - Reduce fifo threshold on hvs4 to fix a fifo full error. - Disable TE support for Droid4 and N950. - Small compiler fixes. The following changes since commit 50891bead80bc79871528c2962d65c781c02330b: drm/etnaviv: User FOLL_LONGTERM in userptr (2021-03-19 20:15:48 +0100) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2021-04-09 for you to fetch changes up to eb9dfdd1ed40357b99a4201c8534c58c562e48c9: drm/vc4: crtc: Reduce PV fifo threshold on hvs4 (2021-04-08 14:55:02 +0200) drm-misc-fixes for v5.12-rc7: - Fix use-after-free in xen. - Reduce fifo threshold on hvs4 to fix a fifo full error. - Disable TE support for Droid4 and N950. - Small compiler fixes. Dom Cobley (1): drm/vc4: crtc: Reduce PV fifo threshold on hvs4 Lv Yunlong (1): gpu/xen: Fix a use after free in xen_drm_drv_init Maxime Ripard (1): drm/vc4: plane: Remove redundant assignment Sebastian Reichel (1): drm/panel: panel-dsi-cm: disable TE for now Wan Jiabing (1): drivers: gpu: drm: xen_drm_front_drm_info is declared twice drivers/gpu/drm/panel/panel-dsi-cm.c | 12 +--- drivers/gpu/drm/vc4/vc4_crtc.c | 17 + drivers/gpu/drm/vc4/vc4_plane.c | 1 - drivers/gpu/drm/xen/xen_drm_front.c | 6 -- drivers/gpu/drm/xen/xen_drm_front_conn.h | 1 - 5 files changed, 30 insertions(+), 7 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 5/6] dt-bindings: display: add Unisoc's mipi dsi controller bindings
Maxime Ripard 于2021年4月7日周三 下午6:46写道： > On Wed, Mar 31, 2021 at 09:49:14AM +0800, Kevin Tang wrote: > > Hi Maxime, > > > > Maxime Ripard 于2021年3月24日周三 下午7:13写道： > > > > > On Mon, Feb 22, 2021 at 09:28:21PM +0800, Kevin Tang wrote: > > > > From: Kevin Tang > > > > > > > > Adds MIPI DSI Controller > > > > support for Unisoc's display subsystem. > > > > > > > > Cc: Orson Zhai > > > > Cc: Chunyan Zhang > > > > Signed-off-by: Kevin Tang > > > > Reviewed-by: Rob Herring > > > > --- > > > > .../display/sprd/sprd,sharkl3-dsi-host.yaml | 102 > ++ > > > > 1 file changed, 102 insertions(+) > > > > create mode 100644 > > > > Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dsi-host.yaml > > > > > > > > diff --git > > > > a/Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dsi-host.yaml > > > > b/Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dsi-host.yaml > > > > new file mode 100644 > > > > index 0..d439f688f > > > > --- /dev/null > > > > +++ > > > > b/Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dsi-host.yaml > > > > @@ -0,0 +1,102 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > +%YAML 1.2 > > > > +--- > > > > +$id: > > > http://devicetree.org/schemas/display/sprd/sprd,sharkl3-dsi-host.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: Unisoc MIPI DSI Controller > > > > + > > > > +maintainers: > > > > + - Kevin Tang > > > > + > > > > +properties: > > > > + compatible: > > > > +const: sprd,sharkl3-dsi-host > > > > + > > > > + reg: > > > > +maxItems: 1 > > > > + > > > > + interrupts: > > > > +maxItems: 2 > > > > + > > > > + clocks: > > > > +minItems: 1 > > > > + > > > > + clock-names: > > > > +items: > > > > + - const: clk_src_96m > > > > + > > > > + power-domains: > > > > +maxItems: 1 > > > > + > > > > + ports: > > > > +type: object > > > > + > > > > +properties: > > > > + "#address-cells": > > > > +const: 1 > > > > + > > > > + "#size-cells": > > > > +const: 0 > > > > + > > > > + port@0: > > > > +type: object > > > > +description: > > > > + A port node with endpoint definitions as defined in > > > > + > Documentation/devicetree/bindings/media/video-interfaces.txt. > > > > + That port should be the input endpoint, usually coming > from > > > > + the associated DPU. > > > > + port@1: > > > > +type: object > > > > +description: > > > > + A port node with endpoint definitions as defined in > > > > + > Documentation/devicetree/bindings/media/video-interfaces.txt. > > > > + That port should be the output endpoint, usually output to > > > > + the associated panel. > > > > > > The DSI generic binding asks that peripherals that are controlled > > > through a DCS be a subnode of the MIPI-DSI bus, not through a port > > > endpoint. > > > > > Our DSI controller don't support DCS now... > > I'm not sure I follow you, you mentionned in the patch 4 that you were > testing for a device to be in command mode, how would that work without > DCS support? > Sorry, I see DCS as DSC, pls ignore my previous comments. dsi input node is display controller and dsi output node is panel, I still don't understand what it has to do with dcs? and it seems that other vendors also like this. can you help provide some cases? > > Maxime > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] gpu/drm: mediatek: hdmi: check for valid modes on MT8167
On MT8167, only CEA modes and anything using a clock below 148500 is
supported for HDMI. This change adds some checks to make sure the
video format is OK for MT8167.
Signed-off-by: Fabien Parent
Signed-off-by: Neil Armstrong
---
drivers/gpu/drm/mediatek/mtk_hdmi.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c
b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index 8ee55f9e2954..991e2e935b93 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -148,6 +148,8 @@ struct hdmi_audio_param {
struct mtk_hdmi_conf {
bool tz_disabled;
+ unsigned long max_mode_clock;
+ bool cea_modes_only;
};
struct mtk_hdmi {
@@ -1259,6 +1261,13 @@ static int mtk_hdmi_conn_mode_valid(struct drm_connector
*conn,
return MODE_BAD;
}
+ if (hdmi->conf->cea_modes_only && !drm_match_cea_mode(mode))
+ return MODE_BAD;
+
+ if (hdmi->conf->max_mode_clock &&
+ mode->clock > hdmi->conf->max_mode_clock)
+ return MODE_CLOCK_HIGH;
+
if (mode->clock < 27000)
return MODE_CLOCK_LOW;
if (mode->clock > 297000)
@@ -1810,10 +1819,18 @@ static const struct mtk_hdmi_conf mtk_hdmi_conf_mt2701
= {
.tz_disabled = true,
};
+static const struct mtk_hdmi_conf mtk_hdmi_conf_mt8167 = {
+ .max_mode_clock = 148500,
+ .cea_modes_only = true,
+};
+
static const struct of_device_id mtk_drm_hdmi_of_ids[] = {
{ .compatible = "mediatek,mt2701-hdmi",
.data = &mtk_hdmi_conf_mt2701,
},
+ { .compatible = "mediatek,mt8167-hdmi",
+ .data = &mtk_hdmi_conf_mt8167,
+ },
{ .compatible = "mediatek,mt8173-hdmi",
},
{}
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 06/18] drm/vc4: Rework the encoder retrieval code
Hi
Am 17.03.21 um 16:43 schrieb Maxime Ripard:
Due to a FIFO that cannot be flushed between the pixelvalve and the HDMI
controllers on BCM2711, we need to carefully disable both at boot time
if they were left enabled by the firmware.
However, at the time we're running that code, the struct drm_connector
encoder pointer isn't set yet, and thus we cannot retrieve the encoder
associated to our CRTC.
We can however make use of the fact that we have a less flexible setup
than what DRM allows where we have a 1:1 relationship between our CRTCs
and encoders (and connectors), and thus store the crtc associated to our
encoder at boot time.
We cannot do that at the time the encoders are probed though, since the
CRTCs won't be probed yet and thus we don't know at that time which CRTC
index we're going to get, so let's do this in two passes: we can first
bind all the components and then once they all are bound, we can iterate
over all the encoders to find their associated CRTC and set the pointer.
This is similar to what we're doing to set the possible_crtcs field.
Fixes: 875a4d536842 ("drm/vc4: drv: Disable the CRTC at boot time")
Cc: # v5.10+
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_crtc.c | 25 +--
drivers/gpu/drm/vc4/vc4_drv.c | 36 ++
drivers/gpu/drm/vc4/vc4_drv.h | 1 +
3 files changed, 60 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index f1f2e8cbce79..e2607e1f2520 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -255,6 +255,19 @@ static u32 vc4_crtc_get_fifo_full_level_bits(struct
vc4_crtc *vc4_crtc,
PV_CONTROL_FIFO_LEVEL);
}
+struct drm_encoder *vc4_get_connector_encoder(struct drm_connector *connector)
+{
+ struct drm_encoder *encoder;
+
+ if (WARN_ON(hweight32(connector->possible_encoders) != 1))
drm_WARN_ON
+ return NULL;
+
+ drm_connector_for_each_possible_encoder(connector, encoder)
+ return encoder;
+
+ return NULL;
+}
+
/*
* Returns the encoder attached to the CRTC.
*
@@ -269,9 +282,17 @@ static struct drm_encoder *vc4_get_crtc_encoder(struct
drm_crtc *crtc)
drm_connector_list_iter_begin(crtc->dev, &conn_iter);
drm_for_each_connector_iter(connector, &conn_iter) {
- if (connector->state->crtc == crtc) {
+ struct drm_encoder *encoder;
+ struct vc4_encoder *vc4_encoder;
+
+ encoder = vc4_get_connector_encoder(connector);
+ if (!encoder)
+ continue;
+
+ vc4_encoder = to_vc4_encoder(encoder);
+ if (vc4_encoder->crtc == crtc) {
drm_connector_list_iter_end(&conn_iter);
- return connector->encoder;
+ return encoder;
}
}
drm_connector_list_iter_end(&conn_iter);
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 556ad0f02a0d..cd1fb75c66a7 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -199,6 +199,41 @@ static int compare_dev(struct device *dev, void *data)
return dev == data;
}
+static struct drm_crtc *vc4_drv_find_crtc(struct drm_device *drm,
+ struct drm_encoder *encoder)
+{
+ struct drm_crtc *crtc;
+
+ if (WARN_ON(hweight32(encoder->possible_crtcs) != 1))
+ return NULL;
+
+ drm_for_each_crtc(crtc, drm) {
+ if (!drm_encoder_crtc_ok(encoder, crtc))
+ continue;
+
+ return crtc;
+ }
+
+ return NULL;
+}
+
+static void vc4_drv_set_encoder_data(struct drm_device *drm)
+{
+ struct drm_encoder *encoder;
+
+ drm_for_each_encoder(encoder, drm) {
+ struct vc4_encoder *vc4_encoder;
+ struct drm_crtc *crtc;
+
+ crtc = vc4_drv_find_crtc(drm, encoder);
+ if (WARN_ON(!crtc))
+ return;
+
+ vc4_encoder = to_vc4_encoder(encoder);
+ vc4_encoder->crtc = crtc;
+ }
+}
+
static void vc4_match_add_drivers(struct device *dev,
struct component_match **match,
struct platform_driver *const *drivers,
@@ -261,6 +296,7 @@ static int vc4_drm_bind(struct device *dev)
ret = component_bind_all(dev, drm);
if (ret)
return ret;
+ vc4_drv_set_encoder_data(drm);
ret = vc4_plane_create_additional_planes(drm);
if (ret)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index a7500716cf3f..1b569dcc2154 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -438,6 +438,7 @@ enum vc4_encoder_type {
struct vc4_encoder {
st
[PULL] drm-intel-fixes
Hi Dave and Daniel, Only one last minute fix targeting stable to fix a null dereference. Here goes drm-intel-fixes-2021-04-09: - Fix invalid access to ACPI _DSM objects (Takashi) Thanks, Rodrigo. The following changes since commit e49d033bddf5b565044e2abe4241353959bc9120: Linux 5.12-rc6 (2021-04-04 14:15:36 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2021-04-09 for you to fetch changes up to b6a37a93c9ac3900987c79b726d0bb3699d8db4e: drm/i915: Fix invalid access to ACPI _DSM objects (2021-04-07 19:07:44 -0400) - Fix invalid access to ACPI _DSM objects (Takashi) Takashi Iwai (1): drm/i915: Fix invalid access to ACPI _DSM objects drivers/gpu/drm/i915/display/intel_acpi.c | 22 -- 1 file changed, 20 insertions(+), 2 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-misc-next
Hi Dave, Daniel, Like you asked, here's this week drm-misc-next PR Maxime drm-misc-next-2021-04-09: drm-misc-next for 5.13: UAPI Changes: Cross-subsystem Changes: Core Changes: - bridge: Fix Kconfig dependency - cmdline: Refuse zero width/height mode - ttm: Ignore signaled move fences, ioremap buffer according to mem caching settins Driver Changes: - Conversions to sysfs_emit - tegra: Don't register DP AUX channels before connectors - zynqmp: Fix for an out-of-bound (but within struct padding) memset The following changes since commit 6c744983004ebc66756e582294672f8b991288d5: drm/bridge: anx7625: disable regulators when power off (2021-04-01 10:38:02 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-2021-04-09 for you to fetch changes up to e8b8b0df8694e39ea6bbbdb9e2fcfa78a61e2e42: drm/panel: Convert sysfs sprintf/snprintf family to sysfs_emit (2021-04-08 20:41:38 -0400) drm-misc-next for 5.13: UAPI Changes: Cross-subsystem Changes: Core Changes: - bridge: Fix Kconfig dependency - cmdline: Refuse zero width/height mode - ttm: Ignore signaled move fences, ioremap buffer according to mem caching settins Driver Changes: - Conversions to sysfs_emit - tegra: Don't register DP AUX channels before connectors - zynqmp: Fix for an out-of-bound (but within struct padding) memset Carsten Haitzler (1): drm/komeda: Fix bit check to import to value of proper type Christian König (1): drm/sched: add missing member documentation Dafna Hirschfeld (1): drm/bridge: fix typo in Kconfig Dan Carpenter (1): drm: xlnx: zynqmp: fix a memset in zynqmp_dp_train() David Stevens (1): drm/syncobj: use newly allocated stub fences Felix Kuehling (1): drm/ttm: Ignore signaled move fences Guobin Huang (1): gma500: Use DEFINE_SPINLOCK() for spinlock Julian Braha (1): drivers: gpu: drm: bridge: fix kconfig dependency on DRM_KMS_HELPER Lyude Paul (4): drm/dp: Fixup kernel docs for struct drm_dp_aux drm/tegra: Don't register DP AUX channels before connectors drm/print: Fixup DRM_DEBUG_KMS_RATELIMITED() drm/dp_mst: Drop DRM_ERROR() on kzalloc() fail in drm_dp_mst_handle_up_req() Oak Zeng (1): drm/ttm: ioremap buffer according to TTM mem caching setting Tian Tao (2): drm/komeda: Convert sysfs sprintf/snprintf family to sysfs_emit drm/panel: Convert sysfs sprintf/snprintf family to sysfs_emit Ville Syrjälä (2): drm: Refuse to create zero width/height cmdline modes drm/vblank: Do not store a new vblank timestamp in drm_vblank_restore() Wan Jiabing (1): drm/drm_internal.h: Remove repeated struct declaration Zhang Jianhua (1): drm/bridge: lt8912b: Add header file drivers/dma-buf/dma-fence.c| 27 - drivers/gpu/drm/arm/display/include/malidp_utils.h | 3 -- drivers/gpu/drm/arm/display/komeda/komeda_dev.c| 6 +-- .../gpu/drm/arm/display/komeda/komeda_pipeline.c | 16 +--- .../drm/arm/display/komeda/komeda_pipeline_state.c | 19 ++ drivers/gpu/drm/bridge/Kconfig | 3 +- drivers/gpu/drm/bridge/lontium-lt8912b.c | 1 + drivers/gpu/drm/drm_dp_mst_topology.c | 5 +-- drivers/gpu/drm/drm_internal.h | 1 - drivers/gpu/drm/drm_modes.c| 3 ++ drivers/gpu/drm/drm_syncobj.c | 25 +--- drivers/gpu/drm/drm_vblank.c | 3 +- drivers/gpu/drm/gma500/power.c | 3 +- drivers/gpu/drm/panel/panel-tpo-td043mtea1.c | 4 +- drivers/gpu/drm/tegra/dpaux.c | 11 +++--- drivers/gpu/drm/ttm/ttm_bo.c | 3 +- drivers/gpu/drm/ttm/ttm_bo_util.c | 14 +++ drivers/gpu/drm/xlnx/zynqmp_dp.c | 2 +- include/drm/drm_dp_helper.h| 44 +++--- include/drm/drm_print.h| 20 ++ include/drm/gpu_scheduler.h| 1 + include/linux/dma-fence.h | 1 + 22 files changed, 142 insertions(+), 73 deletions(-) signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/1] drm/bridge: anx7625: send DPCD command to downstream
Hey Xin,
On Fri, 9 Apr 2021 at 07:35, Xin Ji wrote:
>
> Send DPCD command to downstream before anx7625 power down,
> tell downstream into standby mode.
>
> Signed-off-by: Xin Ji
> ---
> drivers/gpu/drm/bridge/analogix/anx7625.c | 75 +++
> 1 file changed, 75 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
> b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index 65cc05982f82..53d2f0d0ee30 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -124,6 +124,23 @@ static int anx7625_reg_write(struct anx7625_data *ctx,
> return ret;
> }
>
> +static int anx7625_reg_block_write(struct anx7625_data *ctx,
> + struct i2c_client *client,
> + u8 reg_addr, u8 len, u8 *buf)
> +{
> + int ret;
> + struct device *dev = &client->dev;
> +
> + i2c_access_workaround(ctx, client);
> +
> + ret = i2c_smbus_write_i2c_block_data(client, reg_addr, len, buf);
> + if (ret < 0)
> + DRM_DEV_ERROR(dev, "write i2c block failed id=%x\n:%x",
> + client->addr, reg_addr);
> +
> + return ret;
> +}
> +
> static int anx7625_write_or(struct anx7625_data *ctx,
> struct i2c_client *client,
> u8 offset, u8 mask)
> @@ -195,6 +212,55 @@ static int wait_aux_op_finish(struct anx7625_data *ctx)
> return val;
> }
>
> +static int anx7625_aux_dpcd_write(struct anx7625_data *ctx,
> + u8 addrh, u8 addrm, u8 addrl,
> + u8 len, u8 *buf)
> +{
> + struct device *dev = &ctx->client->dev;
> + int ret;
> + u8 cmd;
> +
> + if (len > MAX_DPCD_BUFFER_SIZE) {
> + DRM_DEV_ERROR(dev, "exceed aux buffer len.\n");
> + return -EINVAL;
> + }
> +
> + cmd = ((len - 1) << 4) | 0x08;
> +
> + /* Set command and length */
> + ret = anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
> + AP_AUX_COMMAND, cmd);
> +
> + /* Set aux access address */
> + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
> +AP_AUX_ADDR_7_0, addrl);
> + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
> +AP_AUX_ADDR_15_8, addrm);
> + ret |= anx7625_write_and(ctx, ctx->i2c.rx_p0_client,
> +AP_AUX_ADDR_19_16, addrh);
> +
> + /* Set write data */
> + ret |= anx7625_reg_block_write(ctx, ctx->i2c.rx_p0_client,
> + AP_AUX_BUFF_START, len, buf);
> + /* Enable aux access */
> + ret |= anx7625_write_or(ctx, ctx->i2c.rx_p0_client,
> + AP_AUX_CTRL_STATUS, AP_AUX_CTRL_OP_EN);
> + if (ret < 0) {
> + DRM_DEV_ERROR(dev, "cannot access aux related register.\n");
> + return -EIO;
> + }
> +
> + usleep_range(2000, 2100);
> +
> + ret = wait_aux_op_finish(ctx);
> + if (ret) {
> + DRM_DEV_ERROR(dev, "aux IO error: wait aux op finish.\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static int anx7625_video_mute_control(struct anx7625_data *ctx,
> u8 status)
> {
> @@ -617,6 +683,7 @@ static void anx7625_dp_stop(struct anx7625_data *ctx)
> {
> struct device *dev = &ctx->client->dev;
> int ret;
> + u8 data;
>
> DRM_DEV_DEBUG_DRIVER(dev, "stop dp output\n");
>
> @@ -628,8 +695,16 @@ static void anx7625_dp_stop(struct anx7625_data *ctx)
> ret |= anx7625_write_and(ctx, ctx->i2c.tx_p2_client, 0x08, 0x7f);
>
> ret |= anx7625_video_mute_control(ctx, 1);
> +
> + DRM_DEV_DEBUG_DRIVER(dev, "notify downstream enter into standby\n");
> +
> + /* Downstream monitor enter into standby mode */
> + data = 2;
> + ret |= anx7625_aux_dpcd_write(ctx, 0x00, 0x06, 0x00, 1, &data);
> if (ret < 0)
> DRM_DEV_ERROR(dev, "IO error : mute video fail\n");
> +
> + return;
> }
Reviewed-by: Robert Foss
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [pull] amdgpu, radeon, ttm, sched drm-next-5.13
Am 08.04.21 um 15:03 schrieb Alex Deucher: On Thu, Apr 8, 2021 at 6:28 AM Christian König wrote: Am 08.04.21 um 09:13 schrieb Christian König: Am 07.04.21 um 21:04 schrieb Alex Deucher: On Wed, Apr 7, 2021 at 3:23 AM Dave Airlie wrote: On Wed, 7 Apr 2021 at 06:54, Alex Deucher wrote: On Fri, Apr 2, 2021 at 12:22 PM Christian König wrote: Hey Alex, the TTM and scheduler changes should already be in the drm-misc-next branch (not 100% sure about the TTM patch, need to double check next week). The TTM change is not in drm-misc yet. Could that cause problems when both are merged into drm-next? Dave, Daniel, how do you want to handle this? The duplicated patch is this one: https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ac4eb83ab255de9c31184df51fd1534ba36fd212 amdgpu has changes which depend on it. The same patch is included in this PR. Ouch not sure how best to sync up here, maybe get misc-next into my tree then rebase your tree on top of it? I can do that. Please let me double check later today that we have everything we need in drm-misc-next. There where two patch for TTM (one from Felix and one from Oak) which still needed to be pushed to drm-misc-next. I've done that just a minute ago. They were included in this PR. Then we have this patch which fixes a bug in code removed on drm-misc-next. I think it should be dropped when amd-staging-drm-next is based on drm-next/drm-misc-next. Author: xinhui pan Date: Wed Feb 24 11:28:08 2021 +0800 drm/ttm: Do not add non-system domain BO into swap list Ok. I've also found the following patch which is problematic as well: commit c8a921d49443025e10794342d4433b3f29616409 Author: Jack Zhang Date: Mon Mar 8 12:41:27 2021 +0800 drm/amd/amdgpu implement tdr advanced mode [Why] Previous tdr design treats the first job in job_timeout as the bad job. But sometimes a later bad compute job can block a good gfx job and cause an unexpected gfx job timeout because gfx and compute ring share internal GC HW mutually. [How] This patch implements an advanced tdr mode.It involves an additinal synchronous pre-resubmit step(Step0 Resubmit) before normal resubmit step in order to find the real bad job. 1. At Step0 Resubmit stage, it synchronously submits and pends for the first job being signaled. If it gets timeout, we identify it as guilty and do hw reset. After that, we would do the normal resubmit step to resubmit left jobs. 2. For whole gpu reset(vram lost), do resubmit as the old way. Signed-off-by: Jack Zhang Reviewed-by: Andrey Grodzovsky That one is modifying both amdgpu as well as the scheduler code. IIRC I actually requested that the patch is split into two, but that was somehow not done. How should we proceed here? Should I separate the patch, push the changes to drm-misc-next and then we merge with drm-next and rebase amd-staging-drm-next on top of that? That's most likely the cleanest option approach as far as I can see. That's fine with me. We could have included them in my PR. Now we have wait for drm-misc-next to be merged again before we can merge the amdgpu code. Well I'm not sure, but the patches are identical on both branches. As far as I can see git then just ignores that it gets the patches from both sides of the merge. But I'm not an expert if that doesn't has some bad consequences somewhere else. On the other hand I've decided to keep the TDR patch as it is for now. Regards, Christian. Is anyone planning to do another drm-misc merge at this point? Alex Thanks, Christian. Regards, Christian. Alex Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 04/10] drm/aperture: Add infrastructure for aperture ownership
On Fri, Apr 09, 2021 at 09:06:56AM +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 08.04.21 um 11:48 schrieb Daniel Vetter:
> >
> > Maybe just me, but to avoid overstretching the attention spawn of doc
> > readers I'd avoid this example here. And maybe make the recommendation
> > stronger, e.g. "PCI device drivers can avoid open-coding
> > remove_conflicting_framebuffers() by calling
> > drm_fb_helper_remove_conflicting_pci_framebuffers()."
>
> It's a tutorial. In my expectation, everyone just copies the tutorial code
> and fills the gaps.
Sure, but we also have default functions for most common cases, so most
people just end up copypasting the single function call. Feels like
overkill to have a tutorial for that.
Imo tutorial/pseudo-code are good if there's more involved code flow that
many places need to copypaste and customize. Or to show how different
functions work together collectively. This doesn't quite feel like it's
clearing that bar.
And please don't get me wrong, solid docs is great. It's just that I think
we need to have reader's attention span in mind too (and mine personally
might be on the extremely short side here) to make sure our docs are
effective at conveying information.
> > > + *
> > > + * .. code-block:: c
> > > + *
> > > + * static int probe(struct pci_dev *pdev)
> > > + * {
> > > + * int ret;
> > > + *
> > > + * // Remove any generic drivers...
> > > + * ret =
> > > drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, "example driver");
> > > + * if (ret)
> > > + * return ret;
> > > + *
> > > + * // ... and initialize the hardware.
> > > + * ...
> > > + *
> > > + * drm_dev_register();
> > > + *
> > > + * return 0;
> > > + * }
> > > + *
> > > + * Drivers that are susceptible to being removed be other drivers, such
> > > as
> > > + * generic EFI or VESA drivers, have to register themselves as owners of
> > > their
> > > + * given framebuffer memory. Ownership of the framebuffer memory is
> > > achived
> > > + * by calling devm_aperture_acquire(). On success, the driver is the
> > > owner
> > > + * of the framebuffer range. The function fails if the framebuffer is
> > > already
> > > + * by another driver. See below for an example.
> > > + *
> > > + * .. code-block:: c
> > > + *
> > > + * static struct drm_aperture_funcs ap_funcs = {
> > > + * .detach = ...
> >
> > Is there really value in allowing/forcing drivers to set up their own
> > detach ops? You already make this specific to struct drm_device, an
> > implementation that just calls drm_dev_unplug feels like the right thing
> > to do?
>
> Is it that easy? simepldrm's detach function has code to synchronize with
> concurrent hotplug removals. If we can use drm_dev_unplug() for everything,
> I'm all for it.
Uh, I should have looked at the code instead of just asking silly
questions :-)
Now I'm even more scared, and also more convinced that we're recreating a
bad version of some of the core driver model concepts.
I think the ideal option here would be if drm_aperture could unload
(unbind really) the platform driver for us, through the driver model. Then
there's only one place that keeps track whether the driver is unbound or
not. I'm not sure whether this can be done fully generic on a struct
device, or whether we need special code for each type. Since atm we only
have simpledrm we can just specialize on platform_device and it's good
enough.
I think best here would be to Cc: gregkh on this patch and the simpledrm
->detach implementatation, and ask for his feedback as driver model
maintainer. Maybe if you could hack together the platform_device unbind
path as proof of concept would be even better.
Either way, this is really tricky.
-Daniel
>
> Best regards
> Thomas
>
> >
> > Or maybe we should tie this more into the struct device mode and force an
> > unload that way? That way devm cleanup would work as one expects, and
> > avoid the need for anything specific (hopefully) in this detach callback.
> >
> > Just feels a bit like we're reinventing half of the driver model here,
> > badly.
> >
> > > + * };
> > > + *
> > > + * static int acquire_framebuffers(struct drm_device *dev, struct
> > > pci_dev *pdev)
> > > + * {
> > > + * resource_size_t start, len;
> > > + * struct drm_aperture *ap;
> > > + *
> > > + * base = pci_resource_start(pdev, 0);
> > > + * size = pci_resource_len(pdev, 0);
> > > + *
> > > + * ap = devm_acquire_aperture(dev, base, size, &ap_funcs);
> > > + * if (IS_ERR(ap))
> > > + * return PTR_ERR(ap);
> > > + *
> > > + * return 0;
> > > + * }
> > > + *
> > > + * static int probe(struct pci_dev *pdev)
> > > + * {
> > > + * struct drm_device *dev;
> > > + *
Re: [PATCH v2 04/10] drm/aperture: Add infrastructure for aperture ownership
On Fri, Apr 09, 2021 at 09:54:03AM +0200, Thomas Zimmermann wrote: > Hi > > Am 08.04.21 um 11:48 schrieb Daniel Vetter: > > On Thu, Mar 18, 2021 at 11:29:15AM +0100, Thomas Zimmermann wrote: > > > Platform devices might operate on firmware framebuffers, such as VESA or > > > EFI. Before a native driver for the graphics hardware can take over the > > > device, it has to remove any platform driver that operates on the firmware > > > framebuffer. Aperture helpers provide the infrastructure for platform > > > drivers to acquire firmware framebuffers, and for native drivers to remove > > > them later on. > > > > > > It works similar to the related fbdev mechanism. During initialization, > > > the > > > platform driver acquires the firmware framebuffer's I/O memory and > > > provides > > > a callback to be removed. The native driver later uses this information to > > > remove any platform driver for it's framebuffer I/O memory. > > > > > > The aperture removal code is integrated into the existing code for > > > removing > > > conflicting framebuffers, so native drivers use it automatically. > > > > > > v2: > > > * rename plaform helpers to aperture helpers > > > * tie to device lifetime with devm_ functions > > > * removed unsued remove() callback > > > * rename kickout to detach > > > * make struct drm_aperture private > > > * rebase onto existing drm_aperture.h header file > > > * use MIT license only for simplicity > > > * documentation > > > > > > Signed-off-by: Thomas Zimmermann > > > Tested-by: nerdopolis > > > > Bunch of bikesheds for your considerations below, but overall lgtm. > > > > Acked-by: Daniel Vetter > > > > Cheers, Daniel > > > > > --- > > > Documentation/gpu/drm-internals.rst | 6 + > > > drivers/gpu/drm/Kconfig | 7 + > > > drivers/gpu/drm/Makefile| 1 + > > > drivers/gpu/drm/drm_aperture.c | 287 > > > include/drm/drm_aperture.h | 38 +++- > > > 5 files changed, 338 insertions(+), 1 deletion(-) > > > create mode 100644 drivers/gpu/drm/drm_aperture.c > > > > > > diff --git a/Documentation/gpu/drm-internals.rst > > > b/Documentation/gpu/drm-internals.rst > > > index 4c7642d2ca34..06af044c882f 100644 > > > --- a/Documentation/gpu/drm-internals.rst > > > +++ b/Documentation/gpu/drm-internals.rst > > > @@ -78,9 +78,15 @@ DRM_IOCTL_VERSION ioctl. > > > Managing Ownership of the Framebuffer Aperture > > > -- > > > +.. kernel-doc:: drivers/gpu/drm/drm_aperture.c > > > + :doc: overview > > > + > > > .. kernel-doc:: include/drm/drm_aperture.h > > > :internal: > > > +.. kernel-doc:: drivers/gpu/drm/drm_aperture.c > > > + :export: > > > + > > > Device Instance and Driver Handling > > > --- > > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > > > index 1461652921be..b9d3fb91d22d 100644 > > > --- a/drivers/gpu/drm/Kconfig > > > +++ b/drivers/gpu/drm/Kconfig > > > @@ -221,6 +221,13 @@ config DRM_SCHED > > > tristate > > > depends on DRM > > > +config DRM_APERTURE > > > + bool > > > + depends on DRM > > > + help > > > + Controls ownership of graphics apertures. Required to > > > + synchronize with firmware-based drivers. > > > > Uh I'm not a big fan of Kconfig and .ko modules for every little helper > > code. Imo just stuff this into the drm kms helpers and done. Or stuff it > > into drm core code, I think either is a good case for this. Everything is > > its own module means we need to EXPORT_SYMBOL more stuff, and then drivers > > get funny ideas about using these internals ... > > The code lives in the DRM core module. There's no extra ko file. But I'd > like to keep the Kconfig option. The aperture helpers will only be required > if there are generic drivers in the kernel and for many systems this is not > the case. Imo this kind of optimization is what LTO is for. Having hundreds of Kconfig symbols just to shave of 2 functions, or something like that, in each case just doesn't feel like it's justified spending of effort and complexity. Configuring out entire subsystems, sure, but not individual pieces like this. So minimally a new Kconfig like this needs to show in a a) real world config b) actual relevant savings in terms of bytes. Otherwise it's really just cargo culting. I also feel like Kconfig symbols for everything is an appeasement tactic to sneak code int that has seen some resistance about potential overhead and all that. The cost in maintenance and complexity in keeping all the combinations working is much, much bigger though. Just look at the absolute endless amounts of pain that disabling CONFIG_BACKLIGHT is causing drm drivers. We do not want more of that, except if it's really solidly justified. And for the "this saves memory" justification, we've done that for i915 to kick out support code for old platforms. LTO is what gives you actu
[PATCH -next] ompfb: video: use DEFINE_SPINLOCK() instead of spin_lock_init() in apply.c
spinlock can be initialized automatically with DEFINE_SPINLOCK()
rather than explicitly calling spin_lock_init().
Signed-off-by: Yu Jiahua
---
drivers/video/fbdev/omap2/omapfb/dss/apply.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/video/fbdev/omap2/omapfb/dss/apply.c
b/drivers/video/fbdev/omap2/omapfb/dss/apply.c
index c71021091828..acca991c7540 100644
--- a/drivers/video/fbdev/omap2/omapfb/dss/apply.c
+++ b/drivers/video/fbdev/omap2/omapfb/dss/apply.c
@@ -108,7 +108,7 @@ static struct {
} dss_data;
/* protects dss_data */
-static spinlock_t data_lock;
+static DEFINE_SPINLOCK(data_lock);
/* lock for blocking functions */
static DEFINE_MUTEX(apply_lock);
static DECLARE_COMPLETION(extra_updated_completion);
@@ -131,8 +131,6 @@ static void apply_init_priv(void)
struct mgr_priv_data *mp;
int i;
- spin_lock_init(&data_lock);
-
for (i = 0; i < num_ovls; ++i) {
struct ovl_priv_data *op;
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/doc: emphasize difference between plane formats and IN_FORMATS blob
On Thu, 8 Apr 2021 17:39:22 +0300 Ville Syrjälä wrote: > On Thu, Apr 08, 2021 at 04:57:51PM +0300, Pekka Paalanen wrote: > > On Thu, 8 Apr 2021 13:30:16 +0200 > > Daniel Vetter wrote: > > > > > > Is it also so that passing MOD_INVALID to the explicit modifier uAPI > > > > (ADDFB2) is invalid argument? Do we have that documented? > > > > > > We'd need to check that, currently it's an out-of-band flag in the struct. > > > Atm DRM_FORMAT_MOD_INVALID is entirely an internal sentinel value to > > > denote end-of-array entries. > > > > > > In practice it wont pass because we validate the modifiers against the > > > advertised list. > > We don't actually. If the driver provides the .format_mod_supported() > hook then it's up to the driver to validate the modifier in said hook. > This was done so that people can embed metadata inside the modifier > while only having the base modifier on the modifier list. How userspace > is supposed to figure out which values for this extra metadata are valid > I have no idea. Maybe it's the difference between generic userspace and userspace drivers? I've been having the feeling that these two have different "rules". Maybe that distinction should be formalised in documentation somewhere? Generic userspace never looks into modifiers, it just relays them and compares them as opaque 64-bit words. Userspace drivers are allowed to look into what a modifier actually means and fiddle with it. Thanks, pq pgpaPufxrckBf.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/18] drm/vc4: hdmi: Add Support for the YUV output
On 18/03/2021 19:16, Jernej Škrabec wrote: > Hi! > > Dne sreda, 17. marec 2021 ob 16:43:34 CET je Maxime Ripard napisal(a): >> Hi, >> >> Here's an attempt at support the HDMI YUV output on the BCM2711 SoC found on >> the RaspberryPi4. >> >> I took the same approach than what dw-hdmi did already, turning a bunch of >> functions found in that driver into helpers since they were fairly generic. >> >> However, it feels a bit clunky overall and there's a few rough edges that >> should be addressed in a generic manner: >> >> - while the format negociation makes sense for a bridge, it feels a bit >> over-engineered for a simple encoder where that setting could be a > simple >> switch (and possibly a property?) > > Property could work, but possible values should be then limited to cross > section of HW and connected display capabilities. > >> >> - more importantly, whether we're choosing an YUV output or not is > completely >> hidden away from the userspace even though it might have some effect on > the >> visual quality output (thinking about YUV420 and YUV422 here mostly). > > IMO driver should select highest achievable quality. So in case of YUV420 and > YUV422, later should be selected. This should be the case even if the > property > is implemented. > > Best regards, > Jernej > >> >> - Similarly, the list we report is static and the userspace cannot change > or >> force one mode over the other. We will always pick YUV444 over RGB444 if >> both are available for example. >> >> While the first one might just be due to a lack of helpers, the second and >> third ones are also feeling a bit inconsistent with how we're handling the >> 10/12 bit output for example Another points for YUV422 and YUV420 are: - mandatory YUV420 for pre-HDMI2 displays to achieve 4k60 with HDMI1.4 max TDMS - possibility to achieve factorial frequencies for 10/12bits, it's not the case for YUV422, it's the same TMDS character rate for 8, 19, 12 and 16bits - selecting YUV422 instead of YUV444 for 10/12/16 for 4k60 in HDMI2.0 Today we do not take in account the SCDC feedback from the display, but at some point we should monitor the Scrambling_Status and Character Error Detection to lower down from YUV444 to 422 and 420 for example. Neil >> >> Let me know what you think, >> Maxime >> >> Maxime Ripard (18): >> drm: Introduce new HDMI helpers >> drm/bridge: Add HDMI output fmt helper >> drm/bridge: dw-hdmi: Use helpers >> drm/vc4: txp: Properly set the possible_crtcs mask >> drm/vc4: crtc: Skip the TXP >> drm/vc4: Rework the encoder retrieval code >> drm/vc4: hdmi: Add full range RGB helper >> drm/vc4: hdmi: Use full range helper in csc functions >> drm/vc4: hdmi: Remove limited_rgb_range >> drm/vc4: hdmi: Convert to bridge >> drm/vc4: hdmi: Move XBAR setup to csc_setup >> drm/vc4: hdmi: Replace CSC_CTL hardcoded value by defines >> drm/vc4: hdmi: Define colorspace matrices >> drm/vc4: hdmi: Change CSC callback prototype >> drm/vc4: hdmi: Rework the infoframe prototype >> drm/vc4: hdmi: Support HDMI YUV output >> drm/vc4: hdmi: Move the pixel rate calculation to a helper >> drm/vc4: hdmi: Force YUV422 if the rate is too high >> >> drivers/gpu/drm/Makefile | 2 +- >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 268 ++- >> drivers/gpu/drm/drm_bridge.c | 118 +++ >> drivers/gpu/drm/drm_hdmi.c| 170 + >> drivers/gpu/drm/vc4/vc4_crtc.c| 59 +++- >> drivers/gpu/drm/vc4/vc4_drv.c | 41 +++ >> drivers/gpu/drm/vc4/vc4_drv.h | 26 +- >> drivers/gpu/drm/vc4/vc4_hdmi.c| 399 +++--- >> drivers/gpu/drm/vc4/vc4_hdmi.h| 13 +- >> drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 6 + >> drivers/gpu/drm/vc4/vc4_regs.h| 19 ++ >> drivers/gpu/drm/vc4/vc4_txp.c | 2 +- >> include/drm/drm_bridge.h | 6 + >> include/drm/drm_hdmi.h| 24 ++ >> 14 files changed, 770 insertions(+), 383 deletions(-) >> create mode 100644 drivers/gpu/drm/drm_hdmi.c >> create mode 100644 include/drm/drm_hdmi.h >> >> -- >> 2.30.2 >> >> > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] mm/vmscan: add sync_shrinkers function
On 4/9/21 9:17 AM, Christian König wrote:
> To be able to switch to a spinlock and reduce lock contention in the TTM
> shrinker we don't want to hold a mutex while unmapping and freeing pages
> from the pool.
Does using spinlock instead of mutex really reduce lock contention?
> But then we somehow need to prevent a race between (for example) the shrinker
> trying to free pages and hotplug trying to remove the device which those pages
> belong to.
>
> Taking and releasing the shrinker semaphore on the write side after
> unmapping and freeing all pages should make sure that no shrinker is running
> in
> paralell any more.
So you explain this in this commit log for adding the function, but then the
next patch just adds a sync_shrinkers() call without any comment. I would expect
there a comment explaining why it's done there - what it protects against, as
it's not an obvious pattern IMHO.
> Signed-off-by: Christian König
> ---
> include/linux/shrinker.h | 1 +
> mm/vmscan.c | 10 ++
> 2 files changed, 11 insertions(+)
>
> diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h
> index 0f80123650e2..6b75dc372fce 100644
> --- a/include/linux/shrinker.h
> +++ b/include/linux/shrinker.h
> @@ -92,4 +92,5 @@ extern void register_shrinker_prepared(struct shrinker
> *shrinker);
> extern int register_shrinker(struct shrinker *shrinker);
> extern void unregister_shrinker(struct shrinker *shrinker);
> extern void free_prealloced_shrinker(struct shrinker *shrinker);
> +extern void sync_shrinkers(void);
> #endif
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 562e87cbd7a1..46cd9c215d73 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -408,6 +408,16 @@ void unregister_shrinker(struct shrinker *shrinker)
> }
> EXPORT_SYMBOL(unregister_shrinker);
>
> +/**
> + * sync_shrinker - Wait for all running shrinkers to complete.
> + */
> +void sync_shrinkers(void)
> +{
> + down_write(&shrinker_rwsem);
> + up_write(&shrinker_rwsem);
> +}
> +EXPORT_SYMBOL(sync_shrinkers);
> +
> #define SHRINK_BATCH 128
>
> static unsigned long do_shrink_slab(struct shrink_control *shrinkctl,
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] mm/vmscan: add sync_shrinkers function
Am 09.04.21 um 13:00 schrieb Vlastimil Babka:
On 4/9/21 9:17 AM, Christian König wrote:
To be able to switch to a spinlock and reduce lock contention in the TTM
shrinker we don't want to hold a mutex while unmapping and freeing pages
from the pool.
Does using spinlock instead of mutex really reduce lock contention?
Well using the spinlock instead of the mutex is only the cherry on the cake.
The real improvement for the contention is the fact that we just grab
the next pool and drop the lock again instead of doing the whole IOMMU
unmap and flushing of the CPU TLB dance while holding the lock.
But then we somehow need to prevent a race between (for example) the shrinker
trying to free pages and hotplug trying to remove the device which those pages
belong to.
Taking and releasing the shrinker semaphore on the write side after
unmapping and freeing all pages should make sure that no shrinker is running in
paralell any more.
So you explain this in this commit log for adding the function, but then the
next patch just adds a sync_shrinkers() call without any comment. I would expect
there a comment explaining why it's done there - what it protects against, as
it's not an obvious pattern IMHO.
Good point, going to add a comment.
Thanks,
Christian.
Signed-off-by: Christian König
---
include/linux/shrinker.h | 1 +
mm/vmscan.c | 10 ++
2 files changed, 11 insertions(+)
diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h
index 0f80123650e2..6b75dc372fce 100644
--- a/include/linux/shrinker.h
+++ b/include/linux/shrinker.h
@@ -92,4 +92,5 @@ extern void register_shrinker_prepared(struct shrinker
*shrinker);
extern int register_shrinker(struct shrinker *shrinker);
extern void unregister_shrinker(struct shrinker *shrinker);
extern void free_prealloced_shrinker(struct shrinker *shrinker);
+extern void sync_shrinkers(void);
#endif
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 562e87cbd7a1..46cd9c215d73 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -408,6 +408,16 @@ void unregister_shrinker(struct shrinker *shrinker)
}
EXPORT_SYMBOL(unregister_shrinker);
+/**
+ * sync_shrinker - Wait for all running shrinkers to complete.
+ */
+void sync_shrinkers(void)
+{
+ down_write(&shrinker_rwsem);
+ up_write(&shrinker_rwsem);
+}
+EXPORT_SYMBOL(sync_shrinkers);
+
#define SHRINK_BATCH 128
static unsigned long do_shrink_slab(struct shrink_control *shrinkctl,
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/ttm: make global mutex and use count static
Only needed during device hot plug and remove and not exported. Signed-off-by: Christian König Suggested-by: Bernard --- drivers/gpu/drm/ttm/ttm_device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c index 9b787b3caeb5..1f2024164d72 100644 --- a/drivers/gpu/drm/ttm/ttm_device.c +++ b/drivers/gpu/drm/ttm/ttm_device.c @@ -39,8 +39,8 @@ /** * ttm_global_mutex - protecting the global state */ -DEFINE_MUTEX(ttm_global_mutex); -unsigned ttm_glob_use_count; +static DEFINE_MUTEX(ttm_global_mutex); +static unsigned ttm_glob_use_count; struct ttm_global ttm_glob; EXPORT_SYMBOL(ttm_glob); -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: cleanup coding style a bit
Am 01.04.21 um 03:59 schrieb Bernard: From: "Christian König" Date: 2021-03-31 21:15:22 To: Bernard Zhao ,Huang Rui ,David Airlie ,Daniel Vetter ,dri-devel@lists.freedesktop.org,linux-ker...@vger.kernel.org Cc: opensource.ker...@vivo.com Subject: Re: [PATCH] drm/ttm: cleanup coding style a bit>Am 31.03.21 um 15:12 schrieb Bernard Zhao: Fix sparse warning: drivers/gpu/drm/ttm/ttm_bo.c:52:1: warning: symbol 'ttm_global_mutex' was not declared. Should it be static? drivers/gpu/drm/ttm/ttm_bo.c:53:10: warning: symbol 'ttm_bo_glob_use_count' was not declared. Should it be static? Signed-off-by: Bernard Zhao You are based on an outdated branch, please rebase on top of drm-misc-next. Hi Sure, thanks for your review！ I will fix this and resubmit this patch. Found some time today to do it myself. Please review the patch I've just send to you. Thanks, Christian. BR//Bernard Regards, Christian. --- drivers/gpu/drm/ttm/ttm_bo.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 101a68dc615b..eab21643edfb 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -49,8 +49,8 @@ static void ttm_bo_global_kobj_release(struct kobject *kobj); /* * ttm_global_mutex - protecting the global BO state */ -DEFINE_MUTEX(ttm_global_mutex); -unsigned ttm_bo_glob_use_count; +static DEFINE_MUTEX(ttm_global_mutex); +static unsigned int ttm_bo_glob_use_count; struct ttm_bo_global ttm_bo_glob; EXPORT_SYMBOL(ttm_bo_glob); ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3] drivers: introduce and use WANT_DMA_CMA for soft dependencies on DMA_CMA
Random drivers should not override a user configuration of core knobs
(e.g., CONFIG_DMA_CMA=n). Applicable drivers would like to use DMA_CMA,
which depends on CMA, if possible; however, these drivers also have to
tolerate if DMA_CMA is not available/functioning, for example, if no CMA
area for DMA_CMA use has been setup via "cma=X". In the worst case, the
driver cannot do it's job properly in some configurations.
For example, commit 63f5677544b3 ("drm/etnaviv: select CMA and DMA_CMA if
available") documents
While this is no build dependency, etnaviv will only work correctly
on most systems if CMA and DMA_CMA are enabled. Select both options
if available to avoid users ending up with a non-working GPU due to
a lacking kernel config.
So etnaviv really wants to have DMA_CMA, however, can deal with some cases
where it is not available.
Let's introduce WANT_DMA_CMA and use it in most cases where drivers
select CMA/DMA_CMA, or depend on DMA_CMA (in a wrong way via CMA because
of recursive dependency issues).
We'll assume that any driver that selects DRM_GEM_CMA_HELPER or
DRM_KMS_CMA_HELPER would like to use DMA_CMA if possible.
With this change, distributions can disable CONFIG_CMA or
CONFIG_DMA_CMA, without it silently getting enabled again by random
drivers. Also, we'll now automatically try to enabled both, CONFIG_CMA
and CONFIG_DMA_CMA if they are unspecified and any driver is around that
selects WANT_DMA_CMA -- also implicitly via DRM_GEM_CMA_HELPER or
DRM_KMS_CMA_HELPER.
For example, if any driver selects WANT_DMA_CMA and we do a
"make olddefconfig":
1. With "# CONFIG_CMA is not set" and no specification of
"CONFIG_DMA_CMA"
-> CONFIG_DMA_CMA won't be part of .config
2. With no specification of CONFIG_CMA or CONFIG_DMA_CMA
Contiguous Memory Allocator (CMA) [Y/n/?] (NEW)
DMA Contiguous Memory Allocator (DMA_CMA) [Y/n/?] (NEW)
3. With "# CONFIG_CMA is not set" and "# CONFIG_DMA_CMA is not set"
-> CONFIG_DMA_CMA will be removed from .config
Note: drivers/remoteproc seems to be special; commit c51e882cd711
("remoteproc/davinci: Update Kconfig to depend on DMA_CMA") explains that
there is a real dependency to DMA_CMA for it to work; leave that dependency
in place and don't convert it to a soft dependency.
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: David Airlie
Cc: Daniel Vetter
Cc: Joel Stanley
Cc: Andrew Jeffery
Cc: Lucas Stach
Cc: Russell King
Cc: Christian Gmeiner
Cc: Paul Cercueil
Cc: Linus Walleij
Cc: Christoph Hellwig
Cc: Marek Szyprowski
Cc: Robin Murphy
Cc: Andrew Morton
Cc: Mike Rapoport
Cc: Arnd Bergmann
Cc: Bartlomiej Zolnierkiewicz
Cc: Eric Anholt
Cc: Michal Simek
Cc: Masahiro Yamada
Cc: "Alexander A. Klimov"
Cc: Peter Collingbourne
Cc: Suman Anna
Cc: Jason Gunthorpe
Cc: dri-devel@lists.freedesktop.org
Cc: linux-asp...@lists.ozlabs.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: etna...@lists.freedesktop.org
Cc: linux-m...@vger.kernel.org
Cc: linux-fb...@vger.kernel.org
Cc: io...@lists.linux-foundation.org
Signed-off-by: David Hildenbrand
---
Let's see if this approach is better for soft dependencies (and if we
actually have some hard dependencies in there). This is the follow-up
of
https://lkml.kernel.org/r/20210408092011.52763-1-da...@redhat.com
https://lkml.kernel.org/r/20210408100523.63356-1-da...@redhat.com
I was wondering if it would make sense in some drivers to warn if either
CONFIG_DMA_CMA is not available or if DRM_CMA has not been configured
properly - just to give people a heads up that something might more likely
go wrong; that would, however, be future work.
v2 -> v3:
- Don't use "imply" but instead use a new WANT_DMA_CMA and make the default
of CMA and DMA_CMA depend on it.
- Also adjust ingenic, mcde, tve200; these sound like soft dependencies as
well (although DMA_CMA is really desired)
v1 -> v2:
- Fix DRM_CMA -> DMA_CMA
---
drivers/gpu/drm/Kconfig | 2 ++
drivers/gpu/drm/aspeed/Kconfig | 2 --
drivers/gpu/drm/etnaviv/Kconfig | 3 +--
drivers/gpu/drm/ingenic/Kconfig | 1 -
drivers/gpu/drm/mcde/Kconfig| 1 -
drivers/gpu/drm/tve200/Kconfig | 1 -
drivers/video/fbdev/Kconfig | 2 +-
kernel/dma/Kconfig | 7 +++
mm/Kconfig | 1 +
9 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 85b79a7fee63..6f9989adfa93 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -201,12 +201,14 @@ config DRM_TTM_HELPER
config DRM_GEM_CMA_HELPER
bool
depends on DRM
+ select WANT_DMA_CMA
help
Choose this if you need the GEM CMA helper functions
config DRM_KMS_CMA_HELPER
bool
depends on DRM
+ select WANT_DMA_CMA
select DRM_GEM_CMA_HELPER
help
Choose this if you need the KMS CMA helper functions
diff --git a/drivers/gpu/drm/aspeed/Kconfig b/drivers/gpu/drm/aspeed/Kconfig
i
Re: [Intel-gfx] [PATCH] drm/i915/pmu: Do not report 100% RC6 if not supported
On 06/04/2021 12:19, Andi Shyti wrote:
Hi Tvrtko,
We use GT parked status to estimate RC6 while not in use, however if RC6
is not supported to start with that does not work very well and produces a
false 100% RC6 readout.
Fix by not advancing the estimated RC6 counter when feature is not
supported.
Signed-off-by: Tvrtko Ursulin
Fixes: 1fe699e30113 ("drm/i915/pmu: Fix sleep under atomic in RC6 readout")
Reported-by: Eero T Tamminen
---
drivers/gpu/drm/i915/i915_pmu.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 41651ac255fa..02fe0d22c470 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -191,7 +191,10 @@ static u64 get_rc6(struct intel_gt *gt)
* on top of the last known real value, as the approximated RC6
* counter value.
*/
- val = ktime_since_raw(pmu->sleep_last);
+ if (gt->rc6.supported)
+ val = ktime_since_raw(pmu->sleep_last);
+ else
+ val = 0;
if rc6 is not supported, why are we here?
There is another flavour of this patch which indeed prevents us from
getting here if rc6 is not enabled. (By not exposing the counter if not
supported.)
Did you mean rc6.enabled ?
Yeah, I did not see that one initially at all! But it doesn't matter
since this patch is not going in anyway.
Regards,
Tvrtko
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: make global mutex and use count static
On Fri, Apr 09, 2021 at 01:07:30PM +0200, Christian König wrote: > Only needed during device hot plug and remove and not exported. > > Signed-off-by: Christian König > Suggested-by: Bernard Acked-by: Daniel Vetter > --- > drivers/gpu/drm/ttm/ttm_device.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_device.c > b/drivers/gpu/drm/ttm/ttm_device.c > index 9b787b3caeb5..1f2024164d72 100644 > --- a/drivers/gpu/drm/ttm/ttm_device.c > +++ b/drivers/gpu/drm/ttm/ttm_device.c > @@ -39,8 +39,8 @@ > /** > * ttm_global_mutex - protecting the global state > */ > -DEFINE_MUTEX(ttm_global_mutex); > -unsigned ttm_glob_use_count; > +static DEFINE_MUTEX(ttm_global_mutex); > +static unsigned ttm_glob_use_count; > struct ttm_global ttm_glob; > EXPORT_SYMBOL(ttm_glob); > > -- > 2.25.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] pwm: Rename pwm_get_state() to better reflect its semantic
On Tue, Apr 06, 2021 at 03:43:56PM +0200, Uwe Kleine-König wrote: > Hello Thierry, > > On Tue, Apr 06, 2021 at 01:16:31PM +0200, Thierry Reding wrote: > > On Tue, Apr 06, 2021 at 09:30:36AM +0200, Uwe Kleine-König wrote: > > > Given that lowlevel drivers usually cannot implement exactly what a > > > consumer requests with pwm_apply_state() there is some rounding involved. > > > > > > pwm_get_state() traditionally returned the setting that was requested most > > > recently by the consumer (opposed to what was actually implemented in > > > hardware in reply to the last request). To make this semantic obvious > > > rename the function. > > > > > > Signed-off-by: Uwe Kleine-König > > > --- > > > Documentation/driver-api/pwm.rst | 6 +++- > > > drivers/clk/clk-pwm.c | 2 +- > > > drivers/gpu/drm/i915/display/intel_panel.c | 4 +-- > > > drivers/input/misc/da7280.c| 2 +- > > > drivers/input/misc/pwm-beeper.c| 2 +- > > > drivers/input/misc/pwm-vibra.c | 4 +-- > > > drivers/pwm/core.c | 4 +-- > > > drivers/pwm/pwm-atmel-hlcdc.c | 2 +- > > > drivers/pwm/pwm-atmel.c| 2 +- > > > drivers/pwm/pwm-imx27.c| 2 +- > > > drivers/pwm/pwm-rockchip.c | 2 +- > > > drivers/pwm/pwm-stm32-lp.c | 4 +-- > > > drivers/pwm/pwm-sun4i.c| 2 +- > > > drivers/pwm/sysfs.c| 18 ++-- > > > drivers/regulator/pwm-regulator.c | 4 +-- > > > drivers/video/backlight/pwm_bl.c | 10 +++ > > > include/linux/pwm.h| 34 ++ > > > 17 files changed, 59 insertions(+), 45 deletions(-) > > > > Honestly, I don't think this is worth the churn. If you think people > > will easily get confused by this then a better solution might be to more > > explicitly document the pwm_get_state() function to say exactly what it > > returns. > > I'm not so optimistic that people become aware of the semantic just > because there is documentation describing it and I strongly believe that > a good name for functions is more important than accurate documentation. > > If you don't agree, what do you think about the updated wording in > Documentation/driver-api/pwm.rst? Yeah, that clarifies this a bit. I can apply that hunk of the patch separately. > > But there's no need to make life difficult for everyone by > > renaming this to something as cumbersome as this. > > I don't expect any merge conflicts (and if still a problem occurs > resolving should be trivial enough). So I obviously don't agree to your > weighing. I wasn't talking about merge conflicts but instead about the extra churn of changing all consumers and having to type all these extra characters for no benefit. Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drivers: introduce and use WANT_DMA_CMA for soft dependencies on DMA_CMA
Am Freitag, dem 09.04.2021 um 13:20 +0200 schrieb David Hildenbrand:
> Random drivers should not override a user configuration of core knobs
> (e.g., CONFIG_DMA_CMA=n). Applicable drivers would like to use DMA_CMA,
> which depends on CMA, if possible; however, these drivers also have to
> tolerate if DMA_CMA is not available/functioning, for example, if no CMA
> area for DMA_CMA use has been setup via "cma=X". In the worst case, the
> driver cannot do it's job properly in some configurations.
>
> For example, commit 63f5677544b3 ("drm/etnaviv: select CMA and DMA_CMA if
> available") documents
> While this is no build dependency, etnaviv will only work correctly
> on most systems if CMA and DMA_CMA are enabled. Select both options
> if available to avoid users ending up with a non-working GPU due to
> a lacking kernel config.
> So etnaviv really wants to have DMA_CMA, however, can deal with some cases
> where it is not available.
>
> Let's introduce WANT_DMA_CMA and use it in most cases where drivers
> select CMA/DMA_CMA, or depend on DMA_CMA (in a wrong way via CMA because
> of recursive dependency issues).
>
> We'll assume that any driver that selects DRM_GEM_CMA_HELPER or
> DRM_KMS_CMA_HELPER would like to use DMA_CMA if possible.
>
> With this change, distributions can disable CONFIG_CMA or
> CONFIG_DMA_CMA, without it silently getting enabled again by random
> drivers. Also, we'll now automatically try to enabled both, CONFIG_CMA
> and CONFIG_DMA_CMA if they are unspecified and any driver is around that
> selects WANT_DMA_CMA -- also implicitly via DRM_GEM_CMA_HELPER or
> DRM_KMS_CMA_HELPER.
>
> For example, if any driver selects WANT_DMA_CMA and we do a
> "make olddefconfig":
>
> 1. With "# CONFIG_CMA is not set" and no specification of
> "CONFIG_DMA_CMA"
>
> -> CONFIG_DMA_CMA won't be part of .config
>
> 2. With no specification of CONFIG_CMA or CONFIG_DMA_CMA
>
> Contiguous Memory Allocator (CMA) [Y/n/?] (NEW)
> DMA Contiguous Memory Allocator (DMA_CMA) [Y/n/?] (NEW)
>
> 3. With "# CONFIG_CMA is not set" and "# CONFIG_DMA_CMA is not set"
>
> -> CONFIG_DMA_CMA will be removed from .config
>
> Note: drivers/remoteproc seems to be special; commit c51e882cd711
> ("remoteproc/davinci: Update Kconfig to depend on DMA_CMA") explains that
> there is a real dependency to DMA_CMA for it to work; leave that dependency
> in place and don't convert it to a soft dependency.
Hm, to me this sounds much like the reasoning for the etnaviv
dependency. There is no actual build dependency, as the allocations are
done through the DMA API, but for the allocations to succeed you most
likely want CMA to be enabled. But that's just an observation from the
outside, I have no real clue about the remoteproc drivers.
As far as the etnaviv changes are concerned:
Acked-by: Lucas Stach
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: Thomas Zimmermann
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Joel Stanley
> Cc: Andrew Jeffery
> Cc: Lucas Stach
> Cc: Russell King
> Cc: Christian Gmeiner
> Cc: Paul Cercueil
> Cc: Linus Walleij
> Cc: Christoph Hellwig
> Cc: Marek Szyprowski
> Cc: Robin Murphy
> Cc: Andrew Morton
> Cc: Mike Rapoport
> Cc: Arnd Bergmann
> Cc: Bartlomiej Zolnierkiewicz
> Cc: Eric Anholt
> Cc: Michal Simek
> Cc: Masahiro Yamada
> Cc: "Alexander A. Klimov"
> Cc: Peter Collingbourne
> Cc: Suman Anna
> Cc: Jason Gunthorpe
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-asp...@lists.ozlabs.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: etna...@lists.freedesktop.org
> Cc: linux-m...@vger.kernel.org
> Cc: linux-fb...@vger.kernel.org
> Cc: io...@lists.linux-foundation.org
> Signed-off-by: David Hildenbrand
> ---
>
> Let's see if this approach is better for soft dependencies (and if we
> actually have some hard dependencies in there). This is the follow-up
> of
> https://lkml.kernel.org/r/20210408092011.52763-1-da...@redhat.com
> https://lkml.kernel.org/r/20210408100523.63356-1-da...@redhat.com
>
> I was wondering if it would make sense in some drivers to warn if either
> CONFIG_DMA_CMA is not available or if DRM_CMA has not been configured
> properly - just to give people a heads up that something might more likely
> go wrong; that would, however, be future work.
>
> v2 -> v3:
> - Don't use "imply" but instead use a new WANT_DMA_CMA and make the default
> of CMA and DMA_CMA depend on it.
> - Also adjust ingenic, mcde, tve200; these sound like soft dependencies as
> well (although DMA_CMA is really desired)
>
> v1 -> v2:
> - Fix DRM_CMA -> DMA_CMA
>
> ---
> drivers/gpu/drm/Kconfig | 2 ++
> drivers/gpu/drm/aspeed/Kconfig | 2 --
> drivers/gpu/drm/etnaviv/Kconfig | 3 +--
> drivers/gpu/drm/ingenic/Kconfig | 1 -
> drivers/gpu/drm/mcde/Kconfig| 1 -
> drivers/gpu/drm/tve200/Kconfig | 1 -
> drivers/video/fbdev/Kconfig | 2 +-
> kernel/dma/Kconfig | 7 +++
> mm/Kconfig | 1 +
> 9 fil
Re: [PATCH v2 4/5] drm/vc4: hdmi: Enable the scrambler
Hi Dave,
On Thu, Apr 01, 2021 at 12:30:45PM +0100, Dave Stevenson wrote:
> > Signed-off-by: Maxime Ripard
> > ---
> > drivers/gpu/drm/vc4/vc4_hdmi.c | 56 +
> > drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 3 ++
> > 2 files changed, 59 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > index 0924a1b9e186..530c83097b1a 100644
> > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > @@ -35,6 +35,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -76,6 +77,8 @@
> > #define VC5_HDMI_VERTB_VSPO_SHIFT 16
> > #define VC5_HDMI_VERTB_VSPO_MASK VC4_MASK(29, 16)
> >
> > +#define VC5_HDMI_SCRAMBLER_CTL_ENABLE BIT(0)
> > +
> > #define VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_SHIFT 8
> > #define VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_MASK
> > VC4_MASK(10, 8)
> >
> > @@ -457,6 +460,56 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder
> > *encoder)
> > vc4_hdmi_set_audio_infoframe(encoder);
> > }
> >
> > +static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder,
> > +struct drm_display_mode *mode)
> > +{
> > + struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
> > + struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
> > + struct drm_display_info *display =
> > &vc4_hdmi->connector.display_info;
> > +
> > + if (!vc4_encoder->hdmi_monitor)
> > + return false;
> > +
> > + if (!display->hdmi.scdc.supported ||
> > + !display->hdmi.scdc.scrambling.supported)
> > + return false;
> > +
>
> I think I made this comment last time, but possibly not very clearly.
I must have missed it then, sorry :/
> Up to this point in the function is whether the display/hdmi interface
> *supports* scrambling. The bit after is whether it is *required* to be
> enabled by the mode.
Thomas made a suggestion to move the mode clock check to a helper, so
we'll end up with essentially a helper about whether we support
scrambling or not and if the mode requires it.
> At the moment, if the display/interface supports scrambling but the
> mode doesn't need it, then the scrambling setup is never touched. That
> makes a few assumptions about the default settings.
> Boot with the firmware selecting 4k60 (ie scrambling on), but
> video=HDMI-1:1920x1080 in the kernel command line and you'll have a
> mess with scrambling enabled but not signalled.
>
> I'd be happier if the display/interface says scrambling is supported
> then we always call drm_scdc_set_high_tmds_clock_ratio,
> drm_scdc_set_scrambling and set the VC5_HDMI_SCRAMBLER_CTL_ENABLE
> register bit appropriately for the mode. Feel free to disagree with me
> though.
I think part of it is due to our custom helpers never being called
currently during the boot process. Once that is fixed, the disable
helpers will be called and will disable the scrambling so we should be
good there.
This creates another issue though. That function takes the mode as the
argument and at boot time the mode pointer will be null. I think we can
work around it by assuming that we need to disable it at boot all the
time (and thus ignore the test if our pointer is null).
Would that work for you?
Maxime
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drivers: introduce and use WANT_DMA_CMA for soft dependencies on DMA_CMA
On Fri, Apr 9, 2021 at 1:20 PM David Hildenbrand wrote: > Random drivers should not override a user configuration of core knobs > (e.g., CONFIG_DMA_CMA=n). Applicable drivers would like to use DMA_CMA, > which depends on CMA, if possible; however, these drivers also have to > tolerate if DMA_CMA is not available/functioning, for example, if no CMA > area for DMA_CMA use has been setup via "cma=X". In the worst case, the > driver cannot do it's job properly in some configurations. Looks good to me. At least a lot better than what we have. Reviewed-by: Linus Walleij > Let's see if this approach is better for soft dependencies (and if we > actually have some hard dependencies in there). This is the follow-up > of > https://lkml.kernel.org/r/20210408092011.52763-1-da...@redhat.com > https://lkml.kernel.org/r/20210408100523.63356-1-da...@redhat.com You can just add these to the commit message with Link: when applying so people can easily find the discussion from the commit. > I was wondering if it would make sense in some drivers to warn if either > CONFIG_DMA_CMA is not available or if DRM_CMA has not been configured > properly - just to give people a heads up that something might more likely > go wrong; that would, however, be future work. I think the frameworks (DRM_*_CMA_HELPER) should pr_info something about it so the individual drivers don't have to sanity check their entire world. Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/ttm: re-add debugs tt_shrink file
That got lost when we moved back to a static limit.
Signed-off-by: Christian König
---
drivers/gpu/drm/ttm/ttm_tt.c | 20
1 file changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 7d479095dcf8..4d8498a3d642 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -389,6 +389,21 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, struct
ttm_tt *ttm)
ttm->page_flags &= ~TTM_PAGE_FLAG_PRIV_POPULATED;
}
+#ifdef CONFIG_DEBUG_FS
+
+/* Test the shrinker functions and dump the result */
+static int ttm_tt_debugfs_shrink_show(struct seq_file *m, void *data)
+{
+ struct ttm_operation_ctx ctx = { false, false };
+
+ seq_printf(m, "%d\n", ttm_global_swapout(&ctx, GFP_KERNEL));
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(ttm_tt_debugfs_shrink);
+
+#endif
+
+
/**
* ttm_tt_mgr_init - register with the MM shrinker
*
@@ -396,6 +411,11 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, struct
ttm_tt *ttm)
*/
void ttm_tt_mgr_init(unsigned long num_pages, unsigned long num_dma32_pages)
{
+#ifdef CONFIG_DEBUG_FS
+ debugfs_create_file("tt_shrink", 0400, ttm_debugfs_root, NULL,
+ &ttm_tt_debugfs_shrink_fops);
+#endif
+
if (!ttm_pages_limit)
ttm_pages_limit = num_pages;
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/ttm: fix return value check
The function returns the number of swapped pages here. Only abort when we get
a negative error code.
Signed-off-by: Christian König
---
drivers/gpu/drm/ttm/ttm_tt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 7dcd3fb69495..7d479095dcf8 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -326,7 +326,7 @@ int ttm_tt_populate(struct ttm_device *bdev,
ttm_dma32_pages_limit) {
ret = ttm_global_swapout(ctx, GFP_KERNEL);
- if (ret)
+ if (ret < 0)
goto error;
}
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH -next] drm/etnaviv: remove unneeded if-null-free check
Eliminate the following coccicheck warning: drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c:616:2-8: WARNING: NULL check before some freeing functions is not needed. drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c:618:2-8: WARNING: NULL check before some freeing functions is not needed. drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c:620:2-8: WARNING: NULL check before some freeing functions is not needed. drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c:622:2-8: WARNING: NULL check before some freeing functions is not needed. Signed-off-by: Qiheng Lin --- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index d05c35994579..bd0d66ebf314 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -612,14 +612,10 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, err_submit_cmds: if (ret && (out_fence_fd >= 0)) put_unused_fd(out_fence_fd); - if (stream) - kvfree(stream); - if (bos) - kvfree(bos); - if (relocs) - kvfree(relocs); - if (pmrs) - kvfree(pmrs); + kvfree(stream); + kvfree(bos); + kvfree(relocs); + kvfree(pmrs); return ret; } -- 2.31.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH -next] video: fbdev: vermilion: use DEFINE_MUTEX() for mutex lock and LIST_HEAD() for list head
mutex lock can be initialized automatically with DEFINE_MUTEX() rather than explicitly calling mutex_init(). list head can be initialized automatically with LIST_HEAD() rather than explicitly calling INIT_LIST_HEAD(). Reported-by: Hulk Robot Signed-off-by: Ye Bin --- drivers/video/fbdev/vermilion/vermilion.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/video/fbdev/vermilion/vermilion.c b/drivers/video/fbdev/vermilion/vermilion.c index ff61605b8764..689170d25bc8 100644 --- a/drivers/video/fbdev/vermilion/vermilion.c +++ b/drivers/video/fbdev/vermilion/vermilion.c @@ -35,9 +35,9 @@ #define VML_TOHW(_val, _width) _val) << (_width)) + 0x7FFF - (_val)) >> 16) -static struct mutex vml_mutex; -static struct list_head global_no_mode; -static struct list_head global_has_mode; +static DEFINE_MUTEX(vml_mutex); +static LIST_HEAD(global_no_mode); +static LIST_HEAD(global_has_mode); static struct fb_ops vmlfb_ops; static struct vml_sys *subsys = NULL; static char *vml_default_mode = "1024x768@60"; @@ -1058,9 +1058,6 @@ static int __init vmlfb_init(void) #endif printk(KERN_DEBUG MODULE_NAME ": initializing\n"); - mutex_init(&vml_mutex); - INIT_LIST_HEAD(&global_no_mode); - INIT_LIST_HEAD(&global_has_mode); return pci_register_driver(&vmlfb_pci_driver); } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/1] kernel.h: Split out panic and oops helpers
kernel.h is being used as a dump for all kinds of stuff for a long time.
Here is the attempt to start cleaning it up by splitting out panic and
oops helpers.
There are several purposes of doing this:
- dropping dependency in bug.h
- dropping a loop by moving out panic_notifier.h
- unload kernel.h from something which has its own domain
At the same time convert users tree-wide to use new headers, although
for the time being include new header back to kernel.h to avoid twisted
indirected includes for existing users.
Signed-off-by: Andy Shevchenko
Reviewed-by: Bjorn Andersson
Acked-by: Mike Rapoport
Acked-by: Corey Minyard
Acked-by: Christian Brauner
Acked-by: Arnd Bergmann
Acked-by: Kees Cook
Acked-by: Wei Liu
Acked-by: Rasmus Villemoes
Signed-off-by: Andrew Morton
---
v2:
- fixed all errors with allmodconfig on x86_64 (Andrew)
- checked with allyesconfig on x86_64
- additionally grepped source code for panic notifier list usage
and converted all users
- elaborated commit message (Luis)
- collected given tags (incl. Andrew's SoB, see below)
I added Andrew's SoB since part of the fixes I took from him. Andrew,
feel free to amend or tell me how you want me to do.
arch/alpha/kernel/setup.c | 2 +-
arch/arm64/kernel/setup.c | 1 +
arch/mips/kernel/relocate.c | 1 +
arch/mips/sgi-ip22/ip22-reset.c | 1 +
arch/mips/sgi-ip32/ip32-reset.c | 1 +
arch/parisc/kernel/pdc_chassis.c | 1 +
arch/powerpc/kernel/setup-common.c| 1 +
arch/s390/kernel/ipl.c| 1 +
arch/sparc/kernel/sstate.c| 1 +
arch/um/drivers/mconsole_kern.c | 1 +
arch/um/kernel/um_arch.c | 1 +
arch/x86/include/asm/desc.h | 1 +
arch/x86/kernel/cpu/mshyperv.c| 1 +
arch/x86/kernel/setup.c | 1 +
arch/x86/purgatory/purgatory.c| 2 +
arch/x86/xen/enlighten.c | 1 +
arch/xtensa/platforms/iss/setup.c | 1 +
drivers/bus/brcmstb_gisb.c| 1 +
drivers/char/ipmi/ipmi_msghandler.c | 1 +
drivers/clk/analogbits/wrpll-cln28hpc.c | 4 +
drivers/edac/altera_edac.c| 1 +
drivers/firmware/google/gsmi.c| 1 +
drivers/hv/vmbus_drv.c| 1 +
.../hwtracing/coresight/coresight-cpu-debug.c | 1 +
drivers/leds/trigger/ledtrig-activity.c | 1 +
drivers/leds/trigger/ledtrig-heartbeat.c | 1 +
drivers/leds/trigger/ledtrig-panic.c | 1 +
drivers/misc/bcm-vk/bcm_vk_dev.c | 1 +
drivers/misc/ibmasm/heartbeat.c | 1 +
drivers/misc/pvpanic/pvpanic.c| 1 +
drivers/net/ipa/ipa_smp2p.c | 1 +
drivers/parisc/power.c| 1 +
drivers/power/reset/ltc2952-poweroff.c| 1 +
drivers/remoteproc/remoteproc_core.c | 1 +
drivers/s390/char/con3215.c | 1 +
drivers/s390/char/con3270.c | 1 +
drivers/s390/char/sclp.c | 1 +
drivers/s390/char/sclp_con.c | 1 +
drivers/s390/char/sclp_vt220.c| 1 +
drivers/s390/char/zcore.c | 1 +
drivers/soc/bcm/brcmstb/pm/pm-arm.c | 1 +
drivers/staging/olpc_dcon/olpc_dcon.c | 1 +
drivers/video/fbdev/hyperv_fb.c | 1 +
include/asm-generic/bug.h | 3 +-
include/linux/kernel.h| 84 +---
include/linux/panic.h | 98 +++
include/linux/panic_notifier.h| 12 +++
kernel/hung_task.c| 1 +
kernel/kexec_core.c | 1 +
kernel/panic.c| 1 +
kernel/rcu/tree.c | 2 +
kernel/sysctl.c | 1 +
kernel/trace/trace.c | 1 +
53 files changed, 167 insertions(+), 85 deletions(-)
create mode 100644 include/linux/panic.h
create mode 100644 include/linux/panic_notifier.h
diff --git a/arch/alpha/kernel/setup.c b/arch/alpha/kernel/setup.c
index 03dda3beb3bd..5d1296534682 100644
--- a/arch/alpha/kernel/setup.c
+++ b/arch/alpha/kernel/setup.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -46,7 +47,6 @@
#include
#include
-extern struct atomic_notifier_head panic_notifier_list;
static int alpha_panic_event(struct notifier_block *, unsigned long, void *);
static struct notifier_block alpha_panic_block = {
alpha_panic_event,
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 61845c0821d9..787bc0f601b3 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#incl
[PATCH -next] drm/i915/display: remove redundant NULL check
Fix the following coccicheck warning:
drivers/gpu/drm/i915/display/intel_psr.c:1530:29-31: WARNING
!A || A && B is equivalent to !A || B
Signed-off-by: Qiheng Lin
---
drivers/gpu/drm/i915/display/intel_psr.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
b/drivers/gpu/drm/i915/display/intel_psr.c
index 32d3d56259c2..4cec6b4d7fb9 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -1526,8 +1526,7 @@ void intel_psr_wait_for_idle(const struct
intel_crtc_state *new_crtc_state)
u32 psr_status;
mutex_lock(&intel_dp->psr.lock);
- if (!intel_dp->psr.enabled ||
- (intel_dp->psr.enabled && intel_dp->psr.psr2_enabled)) {
+ if (!intel_dp->psr.enabled || intel_dp->psr.psr2_enabled) {
mutex_unlock(&intel_dp->psr.lock);
continue;
}
--
2.31.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/1] kernel.h: Split out panic and oops helpers
Hi, On Fri, Apr 09, 2021 at 01:02:50PM +0300, Andy Shevchenko wrote: > kernel.h is being used as a dump for all kinds of stuff for a long time. > Here is the attempt to start cleaning it up by splitting out panic and > oops helpers. > > There are several purposes of doing this: > - dropping dependency in bug.h > - dropping a loop by moving out panic_notifier.h > - unload kernel.h from something which has its own domain > > At the same time convert users tree-wide to use new headers, although > for the time being include new header back to kernel.h to avoid twisted > indirected includes for existing users. > > Signed-off-by: Andy Shevchenko > Reviewed-by: Bjorn Andersson > Acked-by: Mike Rapoport > Acked-by: Corey Minyard > Acked-by: Christian Brauner > Acked-by: Arnd Bergmann > Acked-by: Kees Cook > Acked-by: Wei Liu > Acked-by: Rasmus Villemoes > Signed-off-by: Andrew Morton > --- > v2: > - fixed all errors with allmodconfig on x86_64 (Andrew) > - checked with allyesconfig on x86_64 > - additionally grepped source code for panic notifier list usage >and converted all users > - elaborated commit message (Luis) > - collected given tags (incl. Andrew's SoB, see below) > > I added Andrew's SoB since part of the fixes I took from him. Andrew, > feel free to amend or tell me how you want me to do. > > [...] > drivers/power/reset/ltc2952-poweroff.c| 1 + > [...] Acked-by: Sebastian Reichel -- Sebastian signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 4/6] drm/sprd: add Unisoc's drm display controller driver
Maxime Ripard 于2021年4月7日周三 下午6:45写道：
> Hi,
>
> Adding Jörg, Will and Robin,
>
> On Wed, Mar 31, 2021 at 09:21:19AM +0800, Kevin Tang wrote:
> > > > +static u32 check_mmu_isr(struct sprd_dpu *dpu, u32 reg_val)
> > > > +{
> > > > + struct dpu_context *ctx = &dpu->ctx;
> > > > + u32 mmu_mask = BIT_DPU_INT_MMU_VAOR_RD |
> > > > + BIT_DPU_INT_MMU_VAOR_WR |
> > > > + BIT_DPU_INT_MMU_INV_RD |
> > > > + BIT_DPU_INT_MMU_INV_WR;
> > > > + u32 val = reg_val & mmu_mask;
> > > > + int i;
> > > > +
> > > > + if (val) {
> > > > + drm_err(dpu->drm, "--- iommu interrupt err: 0x%04x
> ---\n",
> > > val);
> > > > +
> > > > + if (val & BIT_DPU_INT_MMU_INV_RD)
> > > > + drm_err(dpu->drm, "iommu invalid read error,
> addr:
> > > 0x%08x\n",
> > > > + readl(ctx->base +
> REG_MMU_INV_ADDR_RD));
> > > > + if (val & BIT_DPU_INT_MMU_INV_WR)
> > > > + drm_err(dpu->drm, "iommu invalid write error,
> > > addr: 0x%08x\n",
> > > > + readl(ctx->base +
> REG_MMU_INV_ADDR_WR));
> > > > + if (val & BIT_DPU_INT_MMU_VAOR_RD)
> > > > + drm_err(dpu->drm, "iommu va out of range read
> > > error, addr: 0x%08x\n",
> > > > + readl(ctx->base +
> REG_MMU_VAOR_ADDR_RD));
> > > > + if (val & BIT_DPU_INT_MMU_VAOR_WR)
> > > > + drm_err(dpu->drm, "iommu va out of range write
> > > error, addr: 0x%08x\n",
> > > > + readl(ctx->base +
> REG_MMU_VAOR_ADDR_WR));
> > >
> > > Is that the IOMMU page fault interrupt? I would expect it to be in the
> > > iommu driver.
> >
> > Our iommu driver is indeed an separate driver, and also in upstreaming,
> > but iommu fault interrupts reporting by display controller, not iommu
> > itself,
> > if use iommu_set_fault_handler() to hook in our reporting function, there
> > must be cross-module access to h/w registers.
>
> Can you explain a bit more the design of the hardware then? Each device
> connected to the IOMMU has a status register (and an interrupt) that
> reports when there's a fault?
>
> I'd like to get an ack at least from the IOMMU maintainers and
> reviewers.
>
Chunyan has helped explain it.
>
> > > > +static void sprd_dpi_init(struct sprd_dpu *dpu)
> > > > +{
> > > > + struct dpu_context *ctx = &dpu->ctx;
> > > > + u32 int_mask = 0;
> > > > + u32 reg_val;
> > > > +
> > > > + if (ctx->if_type == SPRD_DPU_IF_DPI) {
> > > > + /* use dpi as interface */
> > > > + dpu_reg_clr(ctx, REG_DPU_CFG0, BIT_DPU_IF_EDPI);
> > > > + /* disable Halt function for SPRD DSI */
> > > > + dpu_reg_clr(ctx, REG_DPI_CTRL, BIT_DPU_DPI_HALT_EN);
> > > > + /* select te from external pad */
> > > > + dpu_reg_set(ctx, REG_DPI_CTRL,
> > > BIT_DPU_EDPI_FROM_EXTERNAL_PAD);
> > > > +
> > > > + /* set dpi timing */
> > > > + reg_val = ctx->vm.hsync_len << 0 |
> > > > + ctx->vm.hback_porch << 8 |
> > > > + ctx->vm.hfront_porch << 20;
> > > > + writel(reg_val, ctx->base + REG_DPI_H_TIMING);
> > > > +
> > > > + reg_val = ctx->vm.vsync_len << 0 |
> > > > + ctx->vm.vback_porch << 8 |
> > > > + ctx->vm.vfront_porch << 20;
> > > > + writel(reg_val, ctx->base + REG_DPI_V_TIMING);
> > > > +
> > > > + if (ctx->vm.vsync_len + ctx->vm.vback_porch < 32)
> > > > + drm_warn(dpu->drm, "Warning: (vsync + vbp) <
> 32, "
> > > > + "underflow risk!\n");
> > >
> > > I don't think a warning is appropriate here. Maybe we should just
> > > outright reject any mode that uses it?
> > >
> > This issue has been fixed on the new soc, maybe I should remove it.
>
> If it still requires a workaround on older SoC, you can definitely add
> it but we should prevent any situation where the underflow might occur
> instead of reporting it once we're there.
>
Thks, porch parameter is obtained from the panel driver, so I think it
would be
better to check on the panel side. I will remove it here.
>
> > > > +static enum drm_mode_status sprd_crtc_mode_valid(struct drm_crtc
> *crtc,
> > > > + const struct drm_display_mode
> > > *mode)
> > > > +{
> > > > + struct sprd_dpu *dpu = to_sprd_crtc(crtc);
> > > > +
> > > > + drm_dbg(dpu->drm, "%s() mode: "DRM_MODE_FMT"\n", __func__,
> > > DRM_MODE_ARG(mode));
> > > > +
> > > > + if (mode->type & DRM_MODE_TYPE_PREFERRED) {
> > > > + drm_display_mode_to_videomode(mode, &dpu->ctx.vm);
> > >
> > > You don't seem to use that anywhere else? And that's a bit fragile,
> > > nothing really guarantees that it's the mode you're going to use, and
> > > even then it can be adjusted
Re: [PATCH v3] drivers: introduce and use WANT_DMA_CMA for soft dependencies on DMA_CMA
On Fri, Apr 9, 2021 at 1:21 PM David Hildenbrand wrote:
>
> Random drivers should not override a user configuration of core knobs
> (e.g., CONFIG_DMA_CMA=n). Applicable drivers would like to use DMA_CMA,
> which depends on CMA, if possible; however, these drivers also have to
> tolerate if DMA_CMA is not available/functioning, for example, if no CMA
> area for DMA_CMA use has been setup via "cma=X". In the worst case, the
> driver cannot do it's job properly in some configurations.
>
> For example, commit 63f5677544b3 ("drm/etnaviv: select CMA and DMA_CMA if
> available") documents
> While this is no build dependency, etnaviv will only work correctly
> on most systems if CMA and DMA_CMA are enabled. Select both options
> if available to avoid users ending up with a non-working GPU due to
> a lacking kernel config.
> So etnaviv really wants to have DMA_CMA, however, can deal with some cases
> where it is not available.
>
> Let's introduce WANT_DMA_CMA and use it in most cases where drivers
> select CMA/DMA_CMA, or depend on DMA_CMA (in a wrong way via CMA because
> of recursive dependency issues).
>
> We'll assume that any driver that selects DRM_GEM_CMA_HELPER or
> DRM_KMS_CMA_HELPER would like to use DMA_CMA if possible.
>
> With this change, distributions can disable CONFIG_CMA or
> CONFIG_DMA_CMA, without it silently getting enabled again by random
> drivers. Also, we'll now automatically try to enabled both, CONFIG_CMA
> and CONFIG_DMA_CMA if they are unspecified and any driver is around that
> selects WANT_DMA_CMA -- also implicitly via DRM_GEM_CMA_HELPER or
> DRM_KMS_CMA_HELPER.
>
> For example, if any driver selects WANT_DMA_CMA and we do a
> "make olddefconfig":
>
> 1. With "# CONFIG_CMA is not set" and no specification of
>"CONFIG_DMA_CMA"
>
> -> CONFIG_DMA_CMA won't be part of .config
>
> 2. With no specification of CONFIG_CMA or CONFIG_DMA_CMA
>
> Contiguous Memory Allocator (CMA) [Y/n/?] (NEW)
> DMA Contiguous Memory Allocator (DMA_CMA) [Y/n/?] (NEW)
>
> 3. With "# CONFIG_CMA is not set" and "# CONFIG_DMA_CMA is not set"
>
> -> CONFIG_DMA_CMA will be removed from .config
>
> Note: drivers/remoteproc seems to be special; commit c51e882cd711
> ("remoteproc/davinci: Update Kconfig to depend on DMA_CMA") explains that
> there is a real dependency to DMA_CMA for it to work; leave that dependency
> in place and don't convert it to a soft dependency.
I don't think this dependency is fundamentally different from the others,
though davinci machines tend to have less memory than a lot of the
other machines, so it's more likely to fail without CMA.
Regardless of this,
Reviewed-by: Arnd Bergmann
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drivers: introduce and use WANT_DMA_CMA for soft dependencies on DMA_CMA
On 09.04.21 15:35, Arnd Bergmann wrote:
On Fri, Apr 9, 2021 at 1:21 PM David Hildenbrand wrote:
Random drivers should not override a user configuration of core knobs
(e.g., CONFIG_DMA_CMA=n). Applicable drivers would like to use DMA_CMA,
which depends on CMA, if possible; however, these drivers also have to
tolerate if DMA_CMA is not available/functioning, for example, if no CMA
area for DMA_CMA use has been setup via "cma=X". In the worst case, the
driver cannot do it's job properly in some configurations.
For example, commit 63f5677544b3 ("drm/etnaviv: select CMA and DMA_CMA if
available") documents
While this is no build dependency, etnaviv will only work correctly
on most systems if CMA and DMA_CMA are enabled. Select both options
if available to avoid users ending up with a non-working GPU due to
a lacking kernel config.
So etnaviv really wants to have DMA_CMA, however, can deal with some cases
where it is not available.
Let's introduce WANT_DMA_CMA and use it in most cases where drivers
select CMA/DMA_CMA, or depend on DMA_CMA (in a wrong way via CMA because
of recursive dependency issues).
We'll assume that any driver that selects DRM_GEM_CMA_HELPER or
DRM_KMS_CMA_HELPER would like to use DMA_CMA if possible.
With this change, distributions can disable CONFIG_CMA or
CONFIG_DMA_CMA, without it silently getting enabled again by random
drivers. Also, we'll now automatically try to enabled both, CONFIG_CMA
and CONFIG_DMA_CMA if they are unspecified and any driver is around that
selects WANT_DMA_CMA -- also implicitly via DRM_GEM_CMA_HELPER or
DRM_KMS_CMA_HELPER.
For example, if any driver selects WANT_DMA_CMA and we do a
"make olddefconfig":
1. With "# CONFIG_CMA is not set" and no specification of
"CONFIG_DMA_CMA"
-> CONFIG_DMA_CMA won't be part of .config
2. With no specification of CONFIG_CMA or CONFIG_DMA_CMA
Contiguous Memory Allocator (CMA) [Y/n/?] (NEW)
DMA Contiguous Memory Allocator (DMA_CMA) [Y/n/?] (NEW)
3. With "# CONFIG_CMA is not set" and "# CONFIG_DMA_CMA is not set"
-> CONFIG_DMA_CMA will be removed from .config
Note: drivers/remoteproc seems to be special; commit c51e882cd711
("remoteproc/davinci: Update Kconfig to depend on DMA_CMA") explains that
there is a real dependency to DMA_CMA for it to work; leave that dependency
in place and don't convert it to a soft dependency.
I don't think this dependency is fundamentally different from the others,
though davinci machines tend to have less memory than a lot of the
other machines, so it's more likely to fail without CMA.
I was also unsure - and Lucas had similar thoughts. If you want, I can
send a v4 also taking care of this.
Thanks!
Regardless of this,
Reviewed-by: Arnd Bergmann
--
Thanks,
David / dhildenb
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 2/6] drm/sprd: add Unisoc's drm kms master
Hi Thomas,
First of all, thanks for comments.
Thomas Zimmermann 于2021年4月8日周四 下午6:21写道：
> Hi,
>
> I have just a few nits below plus the points that others made.
>
> Am 22.02.21 um 14:28 schrieb Kevin Tang:
> > Adds drm support for the Unisoc's display subsystem.
> >
> > This is drm kms driver, this driver provides support for the
> > application framework in Android, Yocto and more.
> >
> > Application framework can access Unisoc's display internel
> > peripherals through libdrm or libkms, it's test ok by modetest
> > (DRM/KMS test tool) and Android HWComposer.
> >
> > Cc: Orson Zhai
> > Cc: Chunyan Zhang
> > Signed-off-by: Kevin Tang
> >
> > v4:
> >- Move the devm_drm_dev_alloc to master_ops->bind function.
> >- The managed drmm_mode_config_init() it is no longer necessary for
> drivers to explicitly call drm_mode_config_cleanup, so delete it.
> > ---
> > drivers/gpu/drm/Kconfig | 2 +
> > drivers/gpu/drm/Makefile| 1 +
> > drivers/gpu/drm/sprd/Kconfig| 12 ++
> > drivers/gpu/drm/sprd/Makefile | 5 +
> > drivers/gpu/drm/sprd/sprd_drm.c | 217
> > drivers/gpu/drm/sprd/sprd_drm.h | 16 +++
> > 6 files changed, 253 insertions(+)
> > create mode 100644 drivers/gpu/drm/sprd/Kconfig
> > create mode 100644 drivers/gpu/drm/sprd/Makefile
> > create mode 100644 drivers/gpu/drm/sprd/sprd_drm.c
> > create mode 100644 drivers/gpu/drm/sprd/sprd_drm.h
> >
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index 8bf103de1..9d6ce2867 100644
> > --- a/drivers/gpu/drm/Kconfig
> > +++ b/drivers/gpu/drm/Kconfig
> > @@ -382,6 +382,8 @@ source "drivers/gpu/drm/tidss/Kconfig"
> >
> > source "drivers/gpu/drm/xlnx/Kconfig"
> >
> > +source "drivers/gpu/drm/sprd/Kconfig"
> > +
> > # Keep legacy drivers last
> >
> > menuconfig DRM_LEGACY
> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > index 02c229392..42d211d9c 100644
> > --- a/drivers/gpu/drm/Makefile
> > +++ b/drivers/gpu/drm/Makefile
> > @@ -126,3 +126,4 @@ obj-$(CONFIG_DRM_ASPEED_GFX) += aspeed/
> > obj-$(CONFIG_DRM_MCDE) += mcde/
> > obj-$(CONFIG_DRM_TIDSS) += tidss/
> > obj-y += xlnx/
> > +obj-$(CONFIG_DRM_SPRD) += sprd/
> > diff --git a/drivers/gpu/drm/sprd/Kconfig b/drivers/gpu/drm/sprd/Kconfig
> > new file mode 100644
> > index 0..6e80cc9f3
> > --- /dev/null
> > +++ b/drivers/gpu/drm/sprd/Kconfig
> > @@ -0,0 +1,12 @@
> > +config DRM_SPRD
> > + tristate "DRM Support for Unisoc SoCs Platform"
> > + depends on ARCH_SPRD || COMPILE_TEST
> > + depends on DRM && OF
> > + select DRM_KMS_HELPER
> > + select DRM_GEM_CMA_HELPER
> > + select DRM_KMS_CMA_HELPER
> > + select DRM_MIPI_DSI
>
> Maybe keep the selects sorted by alphabet to make it more readable.
>
Thks, i will be fix it.
>
> > + help
> > + Choose this option if you have a Unisoc chipset.
> > + If M is selected the module will be called sprd_drm.
> > +
> > diff --git a/drivers/gpu/drm/sprd/Makefile
> b/drivers/gpu/drm/sprd/Makefile
> > new file mode 100644
> > index 0..86d95d93a
> > --- /dev/null
> > +++ b/drivers/gpu/drm/sprd/Makefile
> > @@ -0,0 +1,5 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +subdir-ccflags-y += -I$(srctree)/$(src)
> > +
> > +obj-y := sprd_drm.o
> > diff --git a/drivers/gpu/drm/sprd/sprd_drm.c
> b/drivers/gpu/drm/sprd/sprd_drm.c
> > new file mode 100644
> > index 0..a1d3ed655
> > --- /dev/null
> > +++ b/drivers/gpu/drm/sprd/sprd_drm.c
> > @@ -0,0 +1,217 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2020 Unisoc Inc.
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include "sprd_drm.h"
> > +
> > +#define DRIVER_NAME "sprd"
> > +#define DRIVER_DESC "Spreadtrum SoCs' DRM Driver"
> > +#define DRIVER_DATE "20200201"
> > +#define DRIVER_MAJOR 1
> > +#define DRIVER_MINOR 0
> > +
> > +static const struct drm_mode_config_helper_funcs
> sprd_drm_mode_config_helper = {
> > + .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
> > +};
> > +
> > +static const struct drm_mode_config_funcs sprd_drm_mode_config_funcs = {
> > + .fb_create = drm_gem_fb_create,
> > + .atomic_check = drm_atomic_helper_check,
> > + .atomic_commit = drm_atomic_helper_commit,
> > +};
> > +
> > +static void sprd_drm_mode_config_init(struct drm_device *drm)
> > +{
> > + drm->mode_config.min_width = 0;
> > + drm->mode_config.min_height = 0;
> > + drm->mode_config.max_width = 8192;
> > + drm->mode_config.max_height = 8192;
> > + drm->mode_config.allow_fb_modifiers = true;
> > +
> > + drm->mode_config.funcs = &sprd_drm_mode_config_funcs;
> > + drm->mode_config.helper_private = &sprd_drm_mode_config_helper;
> > +}
> > +
> > +DEFINE_DRM_G
Re: [PATCH v4 2/6] drm/sprd: add Unisoc's drm kms master
Hi
Am 09.04.21 um 15:50 schrieb Kevin Tang:
> +static int __init sprd_drm_init(void)
> +{
> + int ret;
I think ret should just go away.
Like this?
"return platform_register_drivers(sprd_drm_drivers,
ARRAY_SIZE(sprd_drm_drivers));"
Sure.
Best regards
Thomas
if so, i will fix it, thks.
Acked-by: Thomas Zimmermann mailto:tzimmerm...@suse.de>>
> +
> + ret = platform_register_drivers(sprd_drm_drivers,
> + ARRAY_SIZE(sprd_drm_drivers));
> + return ret;
> +}
> +
> +static void __exit sprd_drm_exit(void)
> +{
> + platform_unregister_drivers(sprd_drm_drivers,
> + ARRAY_SIZE(sprd_drm_drivers));
> +}
> +
> +module_init(sprd_drm_init);
> +module_exit(sprd_drm_exit);
> +
> +MODULE_AUTHOR("Leon He mailto:leon...@unisoc.com>>");
> +MODULE_AUTHOR("Kevin Tang mailto:kevin.t...@unisoc.com>>");
> +MODULE_DESCRIPTION("Unisoc DRM KMS Master Driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/gpu/drm/sprd/sprd_drm.h
b/drivers/gpu/drm/sprd/sprd_drm.h
> new file mode 100644
> index 0..9781fd591
> --- /dev/null
> +++ b/drivers/gpu/drm/sprd/sprd_drm.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2020 Unisoc Inc.
> + */
> +
> +#ifndef _SPRD_DRM_H_
> +#define _SPRD_DRM_H_
> +
> +#include
> +#include
> +
> +struct sprd_drm {
> + struct drm_device drm;
> +};
> +
> +#endif /* _SPRD_DRM_H_ */
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
OpenPGP_signature
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/ttm: fix return value check
On Fri, 9 Apr 2021 at 14:01, Christian König wrote: > > The function returns the number of swapped pages here. Only abort when we get > a negative error code. > > Signed-off-by: Christian König Reviewed-by: Matthew Auld ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 1/3] fpga: region: Add fpga-region property 'fpga-config-from-dmabuf'
On Fri, Apr 02, 2021 at 02:39:31PM +0530, Nava kishore Manne wrote: > Add "fpga-config-from-dmabuf" property to allow the bitstream > configuration from pre-allocated dma-buffer. > > Signed-off-by: Nava kishore Manne > --- > Documentation/devicetree/bindings/fpga/fpga-region.txt | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/fpga/fpga-region.txt > b/Documentation/devicetree/bindings/fpga/fpga-region.txt > index 969ca53bb65e..c573cf258d60 100644 > --- a/Documentation/devicetree/bindings/fpga/fpga-region.txt > +++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt > @@ -177,6 +177,8 @@ Optional properties: > it indicates that the FPGA has already been programmed with this image. > If this property is in an overlay targeting a FPGA region, it is a > request to program the FPGA with that image. > +- fpga-config-from-dmabuf : boolean, set if the FPGA configured done from the > + pre-allocated dma-buffer. Sounds like an implementation detail of the OS. Doesn't belong in DT. Rob > - fpga-bridges : should contain a list of phandles to FPGA Bridges that must > be > controlled during FPGA programming along with the parent FPGA bridge. > This property is optional if the FPGA Manager handles the bridges. > -- > 2.18.0 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/ttm: re-add debugs tt_shrink file
On Fri, 9 Apr 2021 at 14:01, Christian König wrote: > > That got lost when we moved back to a static limit. > > Signed-off-by: Christian König Reviewed-by: Matthew Auld ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/atmel-hlcdc: Allow async page flips
On Tue, Mar 30, 2021 at 08:17:20AM -0700, Dan Sneddon wrote: > The driver is capable of doing async page flips so we need to tell the > core to allow them. > > Signed-off-by: Dan Sneddon Tested-by: Ludovic Desroches Thanks, Ludovic > --- > > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > index 871293d1aeeb..f6c3d8809fd8 100644 > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > @@ -686,6 +686,7 @@ static int atmel_hlcdc_dc_modeset_init(struct drm_device > *dev) > dev->mode_config.max_width = dc->desc->max_width; > dev->mode_config.max_height = dc->desc->max_height; > dev->mode_config.funcs = &mode_config_funcs; > + dev->mode_config.async_page_flip = true; > > return 0; > } > -- > 2.17.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next] drm/etnaviv: remove unneeded if-null-free check
Am Fr., 9. Apr. 2021 um 14:23 Uhr schrieb Qiheng Lin : > > Eliminate the following coccicheck warning: > > drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c:616:2-8: WARNING: > NULL check before some freeing functions is not needed. > drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c:618:2-8: WARNING: > NULL check before some freeing functions is not needed. > drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c:620:2-8: WARNING: > NULL check before some freeing functions is not needed. > drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c:622:2-8: WARNING: > NULL check before some freeing functions is not needed. > > Signed-off-by: Qiheng Lin Reviewed-by: Christian Gmeiner -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/panel: Add DT bindings for Samsung LMS397KF04
On Tue, 06 Apr 2021 01:47:12 +0200, Linus Walleij wrote: > This adds device tree bindings for the Samsung LMS397KF04 > RGB DPI display panel. > > Cc: devicet...@vger.kernel.org > Signed-off-by: Linus Walleij > --- > .../display/panel/samsung,lms397kf04.yaml | 74 +++ > 1 file changed, 74 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/samsung,lms397kf04.yaml > Reviewed-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 1/2] dt-bindings: display: add google,cros-ec-anx7688.yaml
ChromeOS EC ANX7688 is a display bridge that converts HDMI 2.0 to
DisplayPort 1.3 Ultra-HDi (4096x2160p60). It is an Analogix ANX7688 chip
which is connected to and operated by the ChromeOS Embedded Controller
(See google,cros-ec.yaml). It is accessed using I2C tunneling through
the EC and therefore its node should be a child of an EC I2C tunnel node
(See google,cros-ec-i2c-tunnel.yaml).
ChromOS EC ANX7688 is found on Acer Chromebook R13 (elm)
Signed-off-by: Dafna Hirschfeld
---
.../bridge/google,cros-ec-anx7688.yaml| 82 +++
1 file changed, 82 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/bridge/google,cros-ec-anx7688.yaml
diff --git
a/Documentation/devicetree/bindings/display/bridge/google,cros-ec-anx7688.yaml
b/Documentation/devicetree/bindings/display/bridge/google,cros-ec-anx7688.yaml
new file mode 100644
index ..9f7cc6b757cb
--- /dev/null
+++
b/Documentation/devicetree/bindings/display/bridge/google,cros-ec-anx7688.yaml
@@ -0,0 +1,82 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/google,cros-ec-anx7688.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ChromeOS EC ANX7688 HDMI to DP Converter through Type-C Port
+
+maintainers:
+ - Nicolas Boichat
+ - Enric Balletbo i Serra
+
+description: |
+ ChromeOS EC ANX7688 is a display bridge that converts HDMI 2.0 to
+ DisplayPort 1.3 Ultra-HDi (4096x2160p60). It is an Analogix ANX7688 chip
+ which is connected to and operated by the ChromeOS Embedded Controller
+ (See google,cros-ec.yaml). It is accessed using I2C tunneling through
+ the EC and therefore its node should be a child of an EC I2C tunnel node
+ (See google,cros-ec-i2c-tunnel.yaml).
+
+properties:
+ compatible:
+const: google,cros-ec-anx7688
+
+ reg:
+maxItems: 1
+description: I2C address of the device.
+
+ ports:
+$ref: /schemas/graph.yaml#/properties/ports
+
+properties:
+ port@0:
+$ref: /schemas/graph.yaml#/properties/port
+description: Video port for HDMI input.
+
+ port@1:
+$ref: /schemas/graph.yaml#/properties/port
+description: USB Type-c connector.
+
+required:
+ - port@0
+ - port@1
+
+required:
+ - compatible
+ - reg
+ - ports
+
+additionalProperties: false
+
+examples:
+ - |
+i2c_tunnel_b: i2c-tunnel1 {
+compatible = "google,cros-ec-i2c-tunnel";
+google,remote-bus = <1>;
+#address-cells = <1>;
+#size-cells = <0>;
+
+anx7688: anx7688@2c {
+compatible = "google,cros-ec-anx7688";
+reg = <0x2c>;
+
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+port@0 {
+reg = <0>;
+anx7688_in: endpoint {
+remote-endpoint = <&hdmi0_out>;
+};
+};
+port@1 {
+reg = <1>;
+anx7688_out: endpoint {
+remote-endpoint = <&typec_connector>;
+};
+};
+};
+};
+};
+
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 0/2] Add support for ANX7688
ANX7688 is a typec port controller that also converts HDMI to DP. ANX7688 is found on Acer Chromebook R13 (elm) and on Pine64 PinePhone. On Acer Chromebook R13 (elm), the device is powered-up and controller by the Embedded Controller. Therefore its operation is transparent to the SoC. It is used in elm only as a display bridge driver. The bridge driver only reads some values using i2c and use them to implement the mode_fixup cb. On v5 we added the full dt-binding of the generic Analogix anx7688 device. The problem is that for elm, most of the fields are not needed since the anx7688 sits behind the EC. After a discussion on v5 (see [1]) we decided to go back to the original approach and send the dt binding as specific to the elm. So in this version we rename the device to cros_ec_anx7688 and use the compatible 'google,cros-ec-anx7688'. [1] https://patchwork.kernel.org/project/dri-devel/patch/20210305124351.15079-3-dafna.hirschf...@collabora.com/ Changes since v5: * treat the device as a specific combination of an ANX7688 behind the EC and call it 'cros-ec-anx7688' Changes since v4: In v4 of this set, the device was added as an 'mfd' device and an additional 'bridge' device for the HDMI-DP conversion, see [2]. [2] https://lkml.org/lkml/2020/3/18/64 Dafna Hirschfeld (1): dt-bindings: display: add google,cros-ec-anx7688.yaml Enric Balletbo i Serra (1): drm/bridge: Add ChromeOS EC ANX7688 bridge driver support .../bridge/google,cros-ec-anx7688.yaml| 82 drivers/gpu/drm/bridge/Kconfig| 12 ++ drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/cros-ec-anx7688.c | 191 ++ 4 files changed, 286 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/google,cros-ec-anx7688.yaml create mode 100644 drivers/gpu/drm/bridge/cros-ec-anx7688.c -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 2/2] drm/bridge: Add ChromeOS EC ANX7688 bridge driver support
From: Enric Balletbo i Serra
This driver adds support for the ChromeOS EC ANX7688 HDMI to DP converter
For our use case, the only reason the Linux kernel driver is necessary is
to reject resolutions that require more bandwidth than what is available
on the DP side. DP bandwidth and lane count are reported by the bridge via
2 registers and, as far as we know, only chips that have a firmware
version greater than 0.85 support these two registers.
Signed-off-by: Nicolas Boichat
Signed-off-by: Hsin-Yi Wang
[The driver is OF only so should depends on CONFIG_OF]
Reported-by: kbuild test robot
Signed-off-by: Enric Balletbo i Serra
[convert to i2c driver, rename to cros_ec_anx7688, add err checks]
Signed-off-by: Dafna Hirschfeld
Reviewed-by: Laurent Pinchart
---
drivers/gpu/drm/bridge/Kconfig | 12 ++
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/cros-ec-anx7688.c | 191 +++
3 files changed, 204 insertions(+)
create mode 100644 drivers/gpu/drm/bridge/cros-ec-anx7688.c
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 998dcda44f70..9f991f0551ce 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -37,6 +37,18 @@ config DRM_CHRONTEL_CH7033
If in doubt, say "N".
+config DRM_CROS_EC_ANX7688
+ tristate "ChromeOS EC ANX7688 bridge"
+ depends on OF
+ select DRM_KMS_HELPER
+ select REGMAP_I2C
+ help
+ ChromeOS EC ANX7688 is an ultra-low power
+ 4K Ultra-HD (4096x2160p60) mobile HD transmitter
+ designed for ChromeOS devices. It converts HDMI
+ 2.0 to DisplayPort 1.3 Ultra-HD. It is connected
+ to the ChromeOS Embedded Controller.
+
config DRM_DISPLAY_CONNECTOR
tristate "Display connector support"
depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 0fc37a8e38d0..a6261b89087c 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o
obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o
+obj-$(CONFIG_DRM_CROS_EC_ANX7688) += cros-ec-anx7688.o
obj-$(CONFIG_DRM_DISPLAY_CONNECTOR) += display-connector.o
obj-$(CONFIG_DRM_LONTIUM_LT9611) += lontium-lt9611.o
obj-$(CONFIG_DRM_LONTIUM_LT9611UXC) += lontium-lt9611uxc.o
diff --git a/drivers/gpu/drm/bridge/cros-ec-anx7688.c
b/drivers/gpu/drm/bridge/cros-ec-anx7688.c
new file mode 100644
index ..0f6d907432e3
--- /dev/null
+++ b/drivers/gpu/drm/bridge/cros-ec-anx7688.c
@@ -0,0 +1,191 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * CrOS EC ANX7688 HDMI->DP bridge driver
+ *
+ * Copyright 2020 Google LLC
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+/* Register addresses */
+#define ANX7688_VENDOR_ID_REG 0x00
+#define ANX7688_DEVICE_ID_REG 0x02
+
+#define ANX7688_FW_VERSION_REG 0x80
+
+#define ANX7688_DP_BANDWIDTH_REG 0x85
+#define ANX7688_DP_LANE_COUNT_REG 0x86
+
+#define ANX7688_VENDOR_ID 0x1f29
+#define ANX7688_DEVICE_ID 0x7688
+
+/* First supported firmware version (0.85) */
+#define ANX7688_MINIMUM_FW_VERSION 0x0085
+
+static const struct regmap_config cros_ec_anx7688_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+};
+
+struct cros_ec_anx7688 {
+ struct i2c_client *client;
+ struct regmap *regmap;
+ struct drm_bridge bridge;
+ bool filter;
+};
+
+static inline struct cros_ec_anx7688 *
+bridge_to_cros_ec_anx7688(struct drm_bridge *bridge)
+{
+ return container_of(bridge, struct cros_ec_anx7688, bridge);
+}
+
+static bool cros_ec_anx7688_bridge_mode_fixup(struct drm_bridge *bridge,
+ const struct drm_display_mode
*mode,
+ struct drm_display_mode
*adjusted_mode)
+{
+ struct cros_ec_anx7688 *anx = bridge_to_cros_ec_anx7688(bridge);
+ int totalbw, requiredbw;
+ u8 dpbw, lanecount;
+ u8 regs[2];
+ int ret;
+
+ if (!anx->filter)
+ return true;
+
+ /* Read both regs 0x85 (bandwidth) and 0x86 (lane count). */
+ ret = regmap_bulk_read(anx->regmap, ANX7688_DP_BANDWIDTH_REG, regs, 2);
+ if (ret < 0) {
+ DRM_ERROR("Failed to read bandwidth/lane count\n");
+ return false;
+ }
+ dpbw = regs[0];
+ lanecount = regs[1];
+
+ /* Maximum 0x19 bandwidth (6.75 Gbps Turbo mode), 2 lanes */
+ if (dpbw > 0x19 || lanecount > 2) {
+ DRM_ERROR("Invalid bandwidth/lane count (%02x/%d)\n", dpbw,
+ lanecount);
+ return false;
+ }
+
+ /* Compute available bandwidth (kHz) */
+ totalbw = dpbw * lanecount * 27 * 8 / 10;
+
+ /* Required bandwidth (8 bpc, kHz) */
+ requ
Re: [PATCH v2] drm/bridge/sii8620: fix dependency on extcon
On 4/8/21 6:07 AM, Robert Foss wrote: > The DRM_SIL_SII8620 kconfig has a weak `imply` dependency > on EXTCON, which causes issues when sii8620 is built > as a builtin and EXTCON is built as a module. > > The symptoms are 'undefined reference' errors caused > by the symbols in EXTCON not being available > to the sii8620 driver. > > Signed-off-by: Robert Foss > Reported-by: kernel test robot > --- > > Changes since v1: > - Fix typo on comment > > drivers/gpu/drm/bridge/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > index 22a467abd3e9..2289b44613af 100644 > --- a/drivers/gpu/drm/bridge/Kconfig > +++ b/drivers/gpu/drm/bridge/Kconfig > @@ -169,7 +169,7 @@ config DRM_SIL_SII8620 > tristate "Silicon Image SII8620 HDMI/MHL bridge" > depends on OF > select DRM_KMS_HELPER > - imply EXTCON > + depends on EXTCON || !EXTCON # if EXTCON=m, this cannot be built-in > depends on RC_CORE || !RC_CORE > help > Silicon Image SII8620 HDMI/MHL bridge chip driver. > That gives me: (on linux-next 20210409) drivers/gpu/drm/Kconfig:77:error: recursive dependency detected! drivers/gpu/drm/Kconfig:77: symbol DRM_KMS_HELPER is selected by DRM_SIL_SII8620 drivers/gpu/drm/bridge/Kconfig:168: symbol DRM_SIL_SII8620 depends on EXTCON drivers/extcon/Kconfig:2: symbol EXTCON is selected by CHARGER_MANAGER drivers/power/supply/Kconfig:499: symbol CHARGER_MANAGER depends on POWER_SUPPLY drivers/power/supply/Kconfig:2: symbol POWER_SUPPLY is selected by OLPC_XO1_SCI arch/x86/Kconfig:2757: symbol OLPC_XO1_SCI depends on GPIO_CS5535 drivers/gpio/Kconfig:1156: symbol GPIO_CS5535 depends on GPIOLIB drivers/gpio/Kconfig:14:symbol GPIOLIB is selected by I2C_MUX_LTC4306 drivers/i2c/muxes/Kconfig:47: symbol I2C_MUX_LTC4306 depends on I2C drivers/i2c/Kconfig:8: symbol I2C is selected by FB_DDC drivers/video/fbdev/Kconfig:63: symbol FB_DDC depends on FB drivers/video/fbdev/Kconfig:12: symbol FB is selected by DRM_KMS_FB_HELPER drivers/gpu/drm/Kconfig:83: symbol DRM_KMS_FB_HELPER depends on DRM_KMS_HELPER For a resolution refer to Documentation/kbuild/kconfig-language.rst subsection "Kconfig recursive dependency limitations" -- ~Randy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 1/1] video: ssd1307fb: Drop OF dependency
After the commit 72915994e028 ("video: ssd1307fb: Make use of device
properties") driver does not depend on OF, drop unneeded dependency.
Signed-off-by: Andy Shevchenko
---
drivers/video/fbdev/Kconfig | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 4f02db65dede..7506b5949eb0 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -2209,7 +2209,6 @@ config FB_SIMPLE
config FB_SSD1307
tristate "Solomon SSD1307 framebuffer support"
depends on FB && I2C
- depends on OF
depends on GPIOLIB || COMPILE_TEST
select FB_SYS_FOPS
select FB_SYS_FILLRECT
--
2.30.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[git pull] drm fixes for 5.12-rc7
Hey Linus, Was relatively quiet this week, but still a few pulls came in, pretty much small fixes across the board, a couple of regression fixes in the amdgpu/radeon code, msm has a few minor fixes across the board, a panel regression fix also. I'm out all next week, so Daniel will do any last minute fixes for the final release, assuming things stick to schedule. I'll be back for the merge window but might towards the end of the first week before I get my MR lined up. Dave. drm-fixes-2021-04-10: drm fixes for 5.12-rc7 amdgpu: - DCN3 fix - Fix CAC setting regression for TOPAZ - Fix ttm regression radeon: - Fix ttm regression msm: - a5xx/a6xx timestamp fix - microcode version check - fail path fix - block programming fix - error removal fix. i915: - Fix invalid access to ACPI _DSM objects xen: - Fix use-after-free in xen. - minor duplicate definition cleanup vc4: - Reduce fifo threshold on hvs4 to fix a fifo full error. - minor redundant assignment cleanup panel: - Disable TE support for Droid4 and N950. The following changes since commit e49d033bddf5b565044e2abe4241353959bc9120: Linux 5.12-rc6 (2021-04-04 14:15:36 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2021-04-10 for you to fetch changes up to bd119f471299c8692a00b2f5e9bba8e3b81c3466: Merge tag 'drm-intel-fixes-2021-04-09' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes (2021-04-10 05:18:35 +1000) drm fixes for 5.12-rc7 amdgpu: - DCN3 fix - Fix CAC setting regression for TOPAZ - Fix ttm regression radeon: - Fix ttm regression msm: - a5xx/a6xx timestamp fix - microcode version check - fail path fix - block programming fix - error removal fix. i915: - Fix invalid access to ACPI _DSM objects xen: - Fix use-after-free in xen. - minor duplicate defintion cleanup vc4: - Reduce fifo threshold on hvs4 to fix a fifo full error. - minor redunantant assignment cleanup panel: - Disable TE support for Droid4 and N950. Alex Deucher (1): drm/amdgpu/smu7: fix CAC setting on TOPAZ Dave Airlie (4): Merge tag 'amd-drm-fixes-5.12-2021-04-08' of https://gitlab.freedesktop.org/agd5f/linux into drm-fixes Merge tag 'drm-msm-fixes-2021-04-02' of https://gitlab.freedesktop.org/drm/msm into drm-fixes Merge tag 'drm-misc-fixes-2021-04-09' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes Merge tag 'drm-intel-fixes-2021-04-09' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes Dmitry Baryshkov (1): drm/msm: a6xx: fix version check for the A650 SQE microcode Dom Cobley (1): drm/vc4: crtc: Reduce PV fifo threshold on hvs4 John Stultz (1): drm/msm: Fix removal of valid error case when checking speed_bin Kalyan Thota (1): drm/msm/disp/dpu1: program 3d_merge only if block is attached Lv Yunlong (1): gpu/xen: Fix a use after free in xen_drm_drv_init Maxime Ripard (1): drm/vc4: plane: Remove redundant assignment Qingqing Zhuo (1): drm/amd/display: Add missing mask for DCN3 Rob Clark (1): drm/msm: Fix a5xx/a6xx timestamps Sebastian Reichel (1): drm/panel: panel-dsi-cm: disable TE for now Stephen Boyd (1): drm/msm: Set drvdata to NULL when msm_drm_init() fails Takashi Iwai (1): drm/i915: Fix invalid access to ACPI _DSM objects Wan Jiabing (1): drivers: gpu: drm: xen_drm_front_drm_info is declared twice xinhui pan (2): drm/amdgpu: Fix size overflow drm/radeon: Fix size overflow drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 2 +- drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hubp.h | 1 + .../gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c| 3 ++- drivers/gpu/drm/i915/display/intel_acpi.c | 22 -- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 ++-- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 18 -- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 4 +++- drivers/gpu/drm/msm/msm_drv.c | 1 + drivers/gpu/drm/panel/panel-dsi-cm.c | 12 +--- drivers/gpu/drm/radeon/radeon_ttm.c| 4 ++-- drivers/gpu/drm/vc4/vc4_crtc.c | 17 + drivers/gpu/drm/vc4/vc4_plane.c| 1 - drivers/gpu/drm/xen/xen_drm_front.c| 6 -- drivers/gpu/drm/xen/xen_drm_front_conn.h | 1 - 14 files changed, 74 insertions(+), 22 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [pull] amdgpu, radeon, ttm, sched drm-next-5.13
On Fri, 9 Apr 2021 at 19:07, Christian König wrote: > > Am 08.04.21 um 15:03 schrieb Alex Deucher: > > On Thu, Apr 8, 2021 at 6:28 AM Christian König > > wrote: > >> Am 08.04.21 um 09:13 schrieb Christian König: > >>> Am 07.04.21 um 21:04 schrieb Alex Deucher: > On Wed, Apr 7, 2021 at 3:23 AM Dave Airlie wrote: > > On Wed, 7 Apr 2021 at 06:54, Alex Deucher > > wrote: > >> On Fri, Apr 2, 2021 at 12:22 PM Christian König > >> wrote: > >>> Hey Alex, > >>> > >>> the TTM and scheduler changes should already be in the drm-misc-next > >>> branch (not 100% sure about the TTM patch, need to double check > >>> next week). > >>> > >> The TTM change is not in drm-misc yet. > >> > >>> Could that cause problems when both are merged into drm-next? > >> Dave, Daniel, how do you want to handle this? The duplicated patch > >> is this one: > >> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ac4eb83ab255de9c31184df51fd1534ba36fd212 > >> > >> amdgpu has changes which depend on it. The same patch is included > >> in this PR. > > Ouch not sure how best to sync up here, maybe get misc-next into my > > tree then rebase your tree on top of it? > I can do that. > >>> Please let me double check later today that we have everything we need > >>> in drm-misc-next. > >> There where two patch for TTM (one from Felix and one from Oak) which > >> still needed to be pushed to drm-misc-next. I've done that just a minute > >> ago. > >> > > They were included in this PR. > > > >> Then we have this patch which fixes a bug in code removed on > >> drm-misc-next. I think it should be dropped when amd-staging-drm-next is > >> based on drm-next/drm-misc-next. > >> > >> Author: xinhui pan > >> Date: Wed Feb 24 11:28:08 2021 +0800 > >> > >> drm/ttm: Do not add non-system domain BO into swap list > >> > > Ok. > > > >> I've also found the following patch which is problematic as well: > >> > >> commit c8a921d49443025e10794342d4433b3f29616409 > >> Author: Jack Zhang > >> Date: Mon Mar 8 12:41:27 2021 +0800 > >> > >> drm/amd/amdgpu implement tdr advanced mode > >> > >> [Why] > >> Previous tdr design treats the first job in job_timeout as the bad > >> job. > >> But sometimes a later bad compute job can block a good gfx job and > >> cause an unexpected gfx job timeout because gfx and compute ring > >> share > >> internal GC HW mutually. > >> > >> [How] > >> This patch implements an advanced tdr mode.It involves an additinal > >> synchronous pre-resubmit step(Step0 Resubmit) before normal resubmit > >> step in order to find the real bad job. > >> > >> 1. At Step0 Resubmit stage, it synchronously submits and pends for > >> the > >> first job being signaled. If it gets timeout, we identify it as > >> guilty > >> and do hw reset. After that, we would do the normal resubmit step to > >> resubmit left jobs. > >> > >> 2. For whole gpu reset(vram lost), do resubmit as the old way. > >> > >> Signed-off-by: Jack Zhang > >> Reviewed-by: Andrey Grodzovsky > >> > >> That one is modifying both amdgpu as well as the scheduler code. IIRC I > >> actually requested that the patch is split into two, but that was > >> somehow not done. > >> > >> How should we proceed here? Should I separate the patch, push the > >> changes to drm-misc-next and then we merge with drm-next and rebase > >> amd-staging-drm-next on top of that? > >> > >> That's most likely the cleanest option approach as far as I can see. > > That's fine with me. We could have included them in my PR. Now we > > have wait for drm-misc-next to be merged again before we can merge the > > amdgpu code. > > Well I'm not sure, but the patches are identical on both branches. > > As far as I can see git then just ignores that it gets the patches from > both sides of the merge. No this is one of the biggest no-nos. Don't ever merge a patch via multiple trees, it ends badly. (you might get away with it once or twice depending, but longer term bad things result, esp around merge conflicts with other trees). If we have patches we need in multiple trees, we have to create a stable topic branch and pull that into both trees. drm-misc-next is backmerged into drm-next now. Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm fixes for 5.12-rc7
The pull request you sent on Sat, 10 Apr 2021 05:46:21 +1000: > git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2021-04-10 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/a85f165e1f38c0a5a6e671ce8126a8cafe35af09 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/3] drm/msm/dpu: add support to dump dpu registers
On Thu, Apr 8, 2021 at 7:28 PM Abhinav Kumar wrote:
>
> Add the dpu_dbg module which adds supports to dump dpu registers
> which can be used in case of error conditions.
>
> changes in v3:
> - Get rid of registration mechanism for sub-modules and instead get
>this information from the dpu catalog itself
> - Get rid of global dpu_dbg struct and instead store it in dpu_kms
> - delegate the power management of the sub-modules to the resp drivers
> - refactor and remove the linked list logic and simplify it to have
>just an array
>
> Change-Id: Ide975ecf5d7952ae44daaa6eb611e27d09630be5
> Reported-by: kernel test robot
> Signed-off-by: Abhinav Kumar
> ---
> drivers/gpu/drm/msm/Makefile | 2 +
> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c| 221 +
> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h| 200 +++
> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c | 257
> +
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 86 +
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h| 5 +
> drivers/gpu/drm/msm/dp/dp_catalog.c| 10 +
> drivers/gpu/drm/msm/dp/dp_catalog.h| 5 +
> drivers/gpu/drm/msm/dp/dp_display.c| 37
> drivers/gpu/drm/msm/dp/dp_display.h| 1 +
> drivers/gpu/drm/msm/dsi/dsi.c | 5 +
> drivers/gpu/drm/msm/dsi/dsi.h | 4 +
> drivers/gpu/drm/msm/dsi/dsi_host.c | 25 +++
> drivers/gpu/drm/msm/msm_drv.c | 29 ++-
> drivers/gpu/drm/msm/msm_drv.h | 2 +
> 16 files changed, 889 insertions(+), 2 deletions(-)
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c
>
[snip]
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h
> new file mode 100644
> index 000..302205a
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h
> @@ -0,0 +1,200 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved.
> + */
> +
> +#ifndef DPU_DBG_H_
> +#define DPU_DBG_H_
> +
> +#include
> +#include
> +#include "../../../drm_crtc_internal.h"
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include "dpu_hw_catalog.h"
> +#include "dpu_kms.h"
> +#include "dsi.h"
> +
> +#define DPU_DBG_DUMP_DATA_LIMITER (NULL)
> +
> +enum dpu_dbg_dump_flag {
> + DPU_DBG_DUMP_IN_LOG = BIT(0),
> + DPU_DBG_DUMP_IN_MEM = BIT(1),
> + DPU_DBG_DUMP_IN_COREDUMP = BIT(2),
> +};
overall, I like this better, but..
I'm not completely convinced about the need for
DUMP_IN_LOG/DUMP_IN_MEM.. we haven't really needed it on the GPU side
of things, and the only case I can think of where it might be useful
is if you can't boot far enough to get to some minimal userspace..
once you have at least some minimal userspace, you can just pull out
and clear the devcore dump via sysfs. That said, if state snapshot
and printing were better separated it would just take a few lines of
code to use a different drm_printer to print the state snapshot to
dmesg.
[snip]
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index ab281cb..d1675ee 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -2489,3 +2489,28 @@ struct drm_bridge *msm_dsi_host_get_bridge(struct
> mipi_dsi_host *host)
>
> return of_drm_find_bridge(msm_host->device_node);
> }
> +
> +void msm_dsi_host_dump_regs(struct mipi_dsi_host *host)
> +{
> + struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
> + struct drm_device *dev = msm_host->dev;
> + struct dpu_dbg_base *dpu_dbg;
> +
> + dpu_dbg = dpu_dbg_get(dev);
> +
> + if (dpu_dbg_is_drm_printer_needed(dpu_dbg) &&
> + !dpu_dbg->dpu_dbg_printer) {
> + pr_err("invalid drm printer\n");
> + return;
> + }
for example, here ^^^
why should the other blocks even care? All they should know is that
they've been asked to snapshot their state..
> + if (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM)
> + pm_runtime_get_sync(&msm_host->pdev->dev);
> +
> + dpu_dbg_dump_regs(&dpu_dbg->dsi_ctrl_regs[msm_host->id],
> + msm_iomap_size(msm_host->pdev, "dsi_ctrl"),
> msm_host->ctrl_base,
> + dpu_dbg->reg_dump_method, dpu_dbg->dpu_dbg_printer);
> +
> + if (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM)
> + pm_runtime_put_sync(&msm_host->pdev->dev);
> +}
I
Re: [PATCH v3 2/3] drm/msm/dpu: add support to dump dpu registers
Hi Rob
Thank you for the review.
On 2021-04-09 13:38, Rob Clark wrote:
On Thu, Apr 8, 2021 at 7:28 PM Abhinav Kumar
wrote:
Add the dpu_dbg module which adds supports to dump dpu registers
which can be used in case of error conditions.
changes in v3:
- Get rid of registration mechanism for sub-modules and instead get
this information from the dpu catalog itself
- Get rid of global dpu_dbg struct and instead store it in dpu_kms
- delegate the power management of the sub-modules to the resp
drivers
- refactor and remove the linked list logic and simplify it to have
just an array
Change-Id: Ide975ecf5d7952ae44daaa6eb611e27d09630be5
Reported-by: kernel test robot
Signed-off-by: Abhinav Kumar
---
drivers/gpu/drm/msm/Makefile | 2 +
drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c| 221
+
drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h| 200
+++
drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c | 257
+
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 86 +
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h| 5 +
drivers/gpu/drm/msm/dp/dp_catalog.c| 10 +
drivers/gpu/drm/msm/dp/dp_catalog.h| 5 +
drivers/gpu/drm/msm/dp/dp_display.c| 37
drivers/gpu/drm/msm/dp/dp_display.h| 1 +
drivers/gpu/drm/msm/dsi/dsi.c | 5 +
drivers/gpu/drm/msm/dsi/dsi.h | 4 +
drivers/gpu/drm/msm/dsi/dsi_host.c | 25 +++
drivers/gpu/drm/msm/msm_drv.c | 29 ++-
drivers/gpu/drm/msm/msm_drv.h | 2 +
16 files changed, 889 insertions(+), 2 deletions(-)
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c
[snip]
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h
new file mode 100644
index 000..302205a
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h
@@ -0,0 +1,200 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2020-2021, The Linux Foundation. All rights
reserved.
+ */
+
+#ifndef DPU_DBG_H_
+#define DPU_DBG_H_
+
+#include
+#include
+#include "../../../drm_crtc_internal.h"
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "dpu_hw_catalog.h"
+#include "dpu_kms.h"
+#include "dsi.h"
+
+#define DPU_DBG_DUMP_DATA_LIMITER (NULL)
+
+enum dpu_dbg_dump_flag {
+ DPU_DBG_DUMP_IN_LOG = BIT(0),
+ DPU_DBG_DUMP_IN_MEM = BIT(1),
+ DPU_DBG_DUMP_IN_COREDUMP = BIT(2),
+};
overall, I like this better, but..
I'm not completely convinced about the need for
DUMP_IN_LOG/DUMP_IN_MEM.. we haven't really needed it on the GPU side
of things, and the only case I can think of where it might be useful
is if you can't boot far enough to get to some minimal userspace..
once you have at least some minimal userspace, you can just pull out
and clear the devcore dump via sysfs. That said, if state snapshot
and printing were better separated it would just take a few lines of
code to use a different drm_printer to print the state snapshot to
dmesg.
[snip]
Yes, dumping the registers to log is just a debug feature and also
useful if
CONFIG_DEVCOREDUMP is not enabled.
Its also useful for developers who can just view the dumps in the logs.
Yes, the way this is written it is already generic enough to just take a
different drm_printer.
For the log i use this:
+ if (DPU_DBG_DUMP_IN_CONSOLE) {
+ p = drm_info_printer(dpu_dbg->drm_dev->dev);
+ dpu_dbg->dpu_dbg_printer = &p;
+ dpu_dbg_print_regs(dpu_dbg->drm_dev, DPU_DBG_DUMP_IN_LOG);
+ }
For the coredump I use this,
p = drm_coredump_printer(&iter);
+
+ drm_printf(&p, "---\n");
+
+ drm_printf(&p, "module: " KBUILD_MODNAME "\n");
+ drm_printf(&p, "dpu devcoredump\n");
+ drm_printf(&p, "timestamp %lld\n", ktime_to_ns(dpu_dbg->timestamp));
+
+ dpu_dbg->dpu_dbg_printer = &p;
+
+ dpu_dbg_print_regs(dpu_dbg->drm_dev, DPU_DBG_DUMP_IN_COREDUMP);
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index ab281cb..d1675ee 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -2489,3 +2489,28 @@ struct drm_bridge
*msm_dsi_host_get_bridge(struct mipi_dsi_host *host)
return of_drm_find_bridge(msm_host->device_node);
}
+
+void msm_dsi_host_dump_regs(struct mipi_dsi_host *host)
+{
+ struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
+ struct drm_device *dev = msm_host->dev;
+ struct dpu_dbg_base *dpu_dbg;
+
+ dpu_dbg = dpu_dbg_get(dev);
+
+ if (dpu_
Re: [PATCH] gpu/drm: mediatek: hdmi: check for valid modes on MT8167
Hi, Neil:
Neil Armstrong 於 2021年4月9日 週五 下午4:43寫道：
>
> On MT8167, only CEA modes and anything using a clock below 148500 is
> supported for HDMI. This change adds some checks to make sure the
> video format is OK for MT8167.
I think this patch should be separated to 3 patches. check CEA mode,
check clock, add mt8167 hdmi support.
>
> Signed-off-by: Fabien Parent
> Signed-off-by: Neil Armstrong
> ---
> drivers/gpu/drm/mediatek/mtk_hdmi.c | 17 +
> 1 file changed, 17 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> index 8ee55f9e2954..991e2e935b93 100644
> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> @@ -148,6 +148,8 @@ struct hdmi_audio_param {
>
> struct mtk_hdmi_conf {
> bool tz_disabled;
> + unsigned long max_mode_clock;
> + bool cea_modes_only;
> };
>
> struct mtk_hdmi {
> @@ -1259,6 +1261,13 @@ static int mtk_hdmi_conn_mode_valid(struct
> drm_connector *conn,
> return MODE_BAD;
> }
>
> + if (hdmi->conf->cea_modes_only && !drm_match_cea_mode(mode))
> + return MODE_BAD;
> +
> + if (hdmi->conf->max_mode_clock &&
> + mode->clock > hdmi->conf->max_mode_clock)
> + return MODE_CLOCK_HIGH;
> +
> if (mode->clock < 27000)
> return MODE_CLOCK_LOW;
> if (mode->clock > 297000)
> @@ -1810,10 +1819,18 @@ static const struct mtk_hdmi_conf
> mtk_hdmi_conf_mt2701 = {
> .tz_disabled = true,
> };
>
> +static const struct mtk_hdmi_conf mtk_hdmi_conf_mt8167 = {
> + .max_mode_clock = 148500,
> + .cea_modes_only = true,
> +};
> +
> static const struct of_device_id mtk_drm_hdmi_of_ids[] = {
> { .compatible = "mediatek,mt2701-hdmi",
> .data = &mtk_hdmi_conf_mt2701,
> },
> + { .compatible = "mediatek,mt8167-hdmi",
"mediatek,mt8167-hdmi" does not exist in binding document, so add this
to binding document first.
In addition, could you also transfer mediatek,hdmi.txt to yaml format?
Regards,
Chun-Kuang.
> + .data = &mtk_hdmi_conf_mt8167,
> + },
> { .compatible = "mediatek,mt8173-hdmi",
> },
> {}
> --
> 2.25.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC 1/3] drm/msm/dpu: merge dpu_hw_intr_get_interrupt_statuses into dpu_hw_intr_dispatch_irqs
There is little sense in reading interrupt statuses and right after that
going after the array of statuses to dispatch them. Merge both loops
into single function doing read and dispatch.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c | 10 +--
.../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 66 ++-
.../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 8 ---
3 files changed, 20 insertions(+), 64 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
index 84ea09d9692f..c977e8484174 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
@@ -376,15 +376,6 @@ void dpu_core_irq_uninstall(struct dpu_kms *dpu_kms)
irqreturn_t dpu_core_irq(struct dpu_kms *dpu_kms)
{
- /*
-* Read interrupt status from all sources. Interrupt status are
-* stored within hw_intr.
-* Function will also clear the interrupt status after reading.
-* Individual interrupt status bit will only get stored if it
-* is enabled.
-*/
- dpu_kms->hw_intr->ops.get_interrupt_statuses(dpu_kms->hw_intr);
-
/*
* Dispatch to HW driver to handle interrupt lookup that is being
* fired. When matching interrupt is located, HW driver will call to
@@ -392,6 +383,7 @@ irqreturn_t dpu_core_irq(struct dpu_kms *dpu_kms)
* dpu_core_irq_callback_handler will perform the registered function
* callback, and do the interrupt status clearing once the registered
* callback is finished.
+* Function will also clear the interrupt status after reading.
*/
dpu_kms->hw_intr->ops.dispatch_irqs(
dpu_kms->hw_intr,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
index 5c521de71567..a8d463a8e8fe 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
@@ -780,6 +780,7 @@ static void dpu_hw_intr_dispatch_irq(struct dpu_hw_intr
*intr,
int start_idx;
int end_idx;
u32 irq_status;
+ u32 enable_mask;
unsigned long irq_flags;
if (!intr)
@@ -792,8 +793,6 @@ static void dpu_hw_intr_dispatch_irq(struct dpu_hw_intr
*intr,
*/
spin_lock_irqsave(&intr->irq_lock, irq_flags);
for (reg_idx = 0; reg_idx < ARRAY_SIZE(dpu_intr_set); reg_idx++) {
- irq_status = intr->save_irq_status[reg_idx];
-
/*
* Each Interrupt register has a range of 32 indexes, and
* that is static for dpu_irq_map.
@@ -805,6 +804,20 @@ static void dpu_hw_intr_dispatch_irq(struct dpu_hw_intr
*intr,
start_idx >= ARRAY_SIZE(dpu_irq_map))
continue;
+ /* Read interrupt status */
+ irq_status = DPU_REG_READ(&intr->hw,
dpu_intr_set[reg_idx].status_off);
+
+ /* Read enable mask */
+ enable_mask = DPU_REG_READ(&intr->hw,
dpu_intr_set[reg_idx].en_off);
+
+ /* and clear the interrupt */
+ if (irq_status)
+ DPU_REG_WRITE(&intr->hw, dpu_intr_set[reg_idx].clr_off,
+irq_status);
+
+ /* Finally update IRQ status based on enable mask */
+ irq_status &= enable_mask;
+
/*
* Search through matching intr status from irq map.
* start_idx and end_idx defined the search range in
@@ -836,6 +849,10 @@ static void dpu_hw_intr_dispatch_irq(struct dpu_hw_intr
*intr,
irq_status &= ~dpu_irq_map[irq_idx].irq_mask;
}
}
+
+ /* ensure register writes go through */
+ wmb();
+
spin_unlock_irqrestore(&intr->irq_lock, irq_flags);
}
@@ -987,41 +1004,6 @@ static int dpu_hw_intr_disable_irqs(struct dpu_hw_intr
*intr)
return 0;
}
-static void dpu_hw_intr_get_interrupt_statuses(struct dpu_hw_intr *intr)
-{
- int i;
- u32 enable_mask;
- unsigned long irq_flags;
-
- if (!intr)
- return;
-
- spin_lock_irqsave(&intr->irq_lock, irq_flags);
- for (i = 0; i < ARRAY_SIZE(dpu_intr_set); i++) {
- if (!test_bit(i, &intr->irq_mask))
- continue;
-
- /* Read interrupt status */
- intr->save_irq_status[i] = DPU_REG_READ(&intr->hw,
- dpu_intr_set[i].status_off);
-
- /* Read enable mask */
- enable_mask = DPU_REG_READ(&intr->hw, dpu_intr_set[i].en_off);
-
- /* and clear the interrupt */
- if (intr->save_irq_status[i])
- DPU_REG_WRITE(&intr->hw, dpu_intr_set[i].clr_off,
- intr-
[RFC 2/3] drm/msm/dpu: hw_intr: always call dpu_hw_intr_clear_intr_status_nolock
Always call dpu_hw_intr_clear_intr_status_nolock() from the
dpu_hw_intr_dispatch_irqs(). This simplifies the callback function
and enforces clearing the hw interrupt status.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c | 9 -
.../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 39 +--
.../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 9 -
3 files changed, 18 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
index c977e8484174..dadb4103b0eb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
@@ -41,15 +41,6 @@ static void dpu_core_irq_callback_handler(void *arg, int
irq_idx)
if (cb->func)
cb->func(cb->arg, irq_idx);
spin_unlock_irqrestore(&dpu_kms->irq_obj.cb_lock, irq_flags);
-
- /*
-* Clear pending interrupt status in HW.
-* NOTE: dpu_core_irq_callback_handler is protected by top-level
-* spinlock, so it is safe to clear any interrupt status here.
-*/
- dpu_kms->hw_intr->ops.clear_intr_status_nolock(
- dpu_kms->hw_intr,
- irq_idx);
}
int dpu_core_irq_idx_lookup(struct dpu_kms *dpu_kms,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
index a8d463a8e8fe..3d48ad69c901 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
@@ -771,6 +771,22 @@ static int dpu_hw_intr_irqidx_lookup(enum dpu_intr_type
intr_type,
return -EINVAL;
}
+static void dpu_hw_intr_clear_intr_status_nolock(struct dpu_hw_intr *intr,
+ int irq_idx)
+{
+ int reg_idx;
+
+ if (!intr)
+ return;
+
+ reg_idx = dpu_irq_map[irq_idx].reg_idx;
+ DPU_REG_WRITE(&intr->hw, dpu_intr_set[reg_idx].clr_off,
+ dpu_irq_map[irq_idx].irq_mask);
+
+ /* ensure register writes go through */
+ wmb();
+}
+
static void dpu_hw_intr_dispatch_irq(struct dpu_hw_intr *intr,
void (*cbfunc)(void *, int),
void *arg)
@@ -837,9 +853,8 @@ static void dpu_hw_intr_dispatch_irq(struct dpu_hw_intr
*intr,
*/
if (cbfunc)
cbfunc(arg, irq_idx);
- else
- intr->ops.clear_intr_status_nolock(
- intr, irq_idx);
+
+ dpu_hw_intr_clear_intr_status_nolock(intr,
irq_idx);
/*
* When callback finish, clear the irq_status
@@ -1004,23 +1019,6 @@ static int dpu_hw_intr_disable_irqs(struct dpu_hw_intr
*intr)
return 0;
}
-
-static void dpu_hw_intr_clear_intr_status_nolock(struct dpu_hw_intr *intr,
- int irq_idx)
-{
- int reg_idx;
-
- if (!intr)
- return;
-
- reg_idx = dpu_irq_map[irq_idx].reg_idx;
- DPU_REG_WRITE(&intr->hw, dpu_intr_set[reg_idx].clr_off,
- dpu_irq_map[irq_idx].irq_mask);
-
- /* ensure register writes go through */
- wmb();
-}
-
static u32 dpu_hw_intr_get_interrupt_status(struct dpu_hw_intr *intr,
int irq_idx, bool clear)
{
@@ -1062,7 +1060,6 @@ static void __setup_intr_ops(struct dpu_hw_intr_ops *ops)
ops->dispatch_irqs = dpu_hw_intr_dispatch_irq;
ops->clear_all_irqs = dpu_hw_intr_clear_irqs;
ops->disable_all_irqs = dpu_hw_intr_disable_irqs;
- ops->clear_intr_status_nolock = dpu_hw_intr_clear_intr_status_nolock;
ops->get_interrupt_status = dpu_hw_intr_get_interrupt_status;
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
index d8b9d5fe6b8c..8d005687b265 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
@@ -141,15 +141,6 @@ struct dpu_hw_intr_ops {
void (*cbfunc)(void *arg, int irq_idx),
void *arg);
- /**
-* clear_intr_status_nolock() - clears the HW interrupts without lock
-* @intr: HW interrupt handle
-* @irq_idx:Lookup irq index return from irq_idx_lookup
-*/
- void (*clear_intr_status_nolock)(
- struct dpu_hw_intr *intr,
- int irq_idx);
-
/**
* get_interrupt_status - Gets HW interrupt status, and clear if set,
*based on given lookup IRQ index.
--
2.30.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedes
[RFC 3/3] drm/msm/dpu: simplify interrupt managing
Change huge lookup table to contain just sensible entries. IRQ index is
now not an index in the table, but just register id (multiplied by 32,
the amount of IRQs in the register) plus offset in the register. This
allows us to remove all the "reserved" entries from dpu_irq_map. The
table is now only used for lookups, individual functions calculate
register and mask using the irq_idx.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c | 10 +-
.../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 535 +-
.../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 4 +-
3 files changed, 141 insertions(+), 408 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
index dadb4103b0eb..bc93eb55b8f9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
@@ -70,7 +70,7 @@ static int _dpu_core_irq_enable(struct dpu_kms *dpu_kms, int
irq_idx)
return -EINVAL;
}
- if (irq_idx < 0 || irq_idx >= dpu_kms->hw_intr->irq_idx_tbl_size) {
+ if (irq_idx < 0 || irq_idx >= dpu_kms->hw_intr->total_irqs) {
DPU_ERROR("invalid IRQ index: [%d]\n", irq_idx);
return -EINVAL;
}
@@ -133,7 +133,7 @@ static int _dpu_core_irq_disable(struct dpu_kms *dpu_kms,
int irq_idx)
return -EINVAL;
}
- if (irq_idx < 0 || irq_idx >= dpu_kms->hw_intr->irq_idx_tbl_size) {
+ if (irq_idx < 0 || irq_idx >= dpu_kms->hw_intr->total_irqs) {
DPU_ERROR("invalid IRQ index: [%d]\n", irq_idx);
return -EINVAL;
}
@@ -208,7 +208,7 @@ int dpu_core_irq_register_callback(struct dpu_kms *dpu_kms,
int irq_idx,
return -EINVAL;
}
- if (irq_idx < 0 || irq_idx >= dpu_kms->hw_intr->irq_idx_tbl_size) {
+ if (irq_idx < 0 || irq_idx >= dpu_kms->hw_intr->total_irqs) {
DPU_ERROR("invalid IRQ index: [%d]\n", irq_idx);
return -EINVAL;
}
@@ -243,7 +243,7 @@ int dpu_core_irq_unregister_callback(struct dpu_kms
*dpu_kms, int irq_idx,
return -EINVAL;
}
- if (irq_idx < 0 || irq_idx >= dpu_kms->hw_intr->irq_idx_tbl_size) {
+ if (irq_idx < 0 || irq_idx >= dpu_kms->hw_intr->total_irqs) {
DPU_ERROR("invalid IRQ index: [%d]\n", irq_idx);
return -EINVAL;
}
@@ -328,7 +328,7 @@ void dpu_core_irq_preinstall(struct dpu_kms *dpu_kms)
spin_lock_init(&dpu_kms->irq_obj.cb_lock);
/* Create irq callbacks for all possible irq_idx */
- dpu_kms->irq_obj.total_irqs = dpu_kms->hw_intr->irq_idx_tbl_size;
+ dpu_kms->irq_obj.total_irqs = dpu_kms->hw_intr->total_irqs;
dpu_kms->irq_obj.irq_cb_tbl = kcalloc(dpu_kms->irq_obj.total_irqs,
sizeof(struct list_head), GFP_KERNEL);
dpu_kms->irq_obj.enable_counts = kcalloc(dpu_kms->irq_obj.total_irqs,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
index 3d48ad69c901..09e1d3182611 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
@@ -29,140 +29,140 @@
/**
* WB interrupt status bit definitions
*/
-#define DPU_INTR_WB_0_DONE BIT(0)
-#define DPU_INTR_WB_1_DONE BIT(1)
-#define DPU_INTR_WB_2_DONE BIT(4)
+#define DPU_INTR_WB_0_DONE 0
+#define DPU_INTR_WB_1_DONE 1
+#define DPU_INTR_WB_2_DONE 4
/**
* WDOG timer interrupt status bit definitions
*/
-#define DPU_INTR_WD_TIMER_0_DONE BIT(2)
-#define DPU_INTR_WD_TIMER_1_DONE BIT(3)
-#define DPU_INTR_WD_TIMER_2_DONE BIT(5)
-#define DPU_INTR_WD_TIMER_3_DONE BIT(6)
-#define DPU_INTR_WD_TIMER_4_DONE BIT(7)
+#define DPU_INTR_WD_TIMER_0_DONE 2
+#define DPU_INTR_WD_TIMER_1_DONE 3
+#define DPU_INTR_WD_TIMER_2_DONE 5
+#define DPU_INTR_WD_TIMER_3_DONE 6
+#define DPU_INTR_WD_TIMER_4_DONE 7
/**
* Pingpong interrupt status bit definitions
*/
-#define DPU_INTR_PING_PONG_0_DONE BIT(8)
-#define DPU_INTR_PING_PONG_1_DONE BIT(9)
-#define DPU_INTR_PING_PONG_2_DONE BIT(10)
-#define DPU_INTR_PING_PONG_3_DONE BIT(11)
-#define DPU_INTR_PING_PONG_0_RD_PTR BIT(12)
-#define DPU_INTR_PING_PONG_1_RD_PTR BIT(13)
-#define DPU_INTR_PING_PONG_2_RD_PTR BIT(14)
-#define DPU_INTR_PING_PONG_3_RD_PTR BIT(15)
-#define DPU_INTR_PING_PONG_0_WR_PTR BIT(16)
-#define DPU_INTR_PING_PONG_1_WR_PTR BIT(17)
-#define DPU_INTR_PING_PONG_2_WR_PTR BIT(18)
-#define DPU_INTR_PING_PONG_3_WR_PTR BIT(19)
-#define DPU_INTR_PING_PONG_0_AUTOREFRESH_DONE BIT(20)
-#define DPU_INTR_PING_PONG_1_AUTOREFRESH_DONE BIT(21)
-#define DPU_INTR_PING_PONG_2_AUTOREFRESH_DONE BIT(22)
-#define DPU_INTR_PING_PONG_3_AUTOREFRESH_DONE BIT(23)
+#define DPU_INTR_PING_PONG_0_DONE 8
+#define DPU_INTR_PING_PONG_1_DONE 9
+#define DPU_INTR_PING_PONG_2_DONE 10
+#define DPU_INTR_PING_PONG
[Bug 212635] New: nouveau 0000:04:00.0: fifo: fault 00 [READ] at 0000000000380000 engine 00 [GR] client 14 [HUB/SCC] reason 02 [PTE] on channel 5 [007fabf000 X[570]]
https://bugzilla.kernel.org/show_bug.cgi?id=212635 Bug ID: 212635 Summary: nouveau :04:00.0: fifo: fault 00 [READ] at 0038 engine 00 [GR] client 14 [HUB/SCC] reason 02 [PTE] on channel 5 [007fabf000 X[570]] Product: Drivers Version: 2.5 Kernel Version: 5.11.12 Hardware: x86-64 OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: erhar...@mailbox.org Regression: No Created attachment 296315 --> https://bugzilla.kernel.org/attachment.cgi?id=296315&action=edit dmesg (kernel 5.11.12) Happened during browsing on Firefox, WebGL conformance tests were running in the background (https://www.khronos.org/registry/webgl/sdk/tests/webgl-conformance-tests.html?version=2.0.1). The screen went blank, but I got to desktop back again and the machine stayed usable. [...] nouveau :04:00.0: fifo: fault 00 [READ] at 0038 engine 00 [GR] client 14 [HUB/SCC] reason 02 [PTE] on channel 5 [007fabf000 X[570]] nouveau :04:00.0: fifo: channel 5: killed nouveau :04:00.0: fifo: runlist 0: scheduled for recovery nouveau :04:00.0: fifo: engine 0: scheduled for recovery nouveau :04:00.0: X[570]: channel 5 killed! nouveau :04:00.0: fifo: fault 00 [READ] at 0038 engine 00 [GR] client 14 [HUB/SCC] reason 02 [PTE] on channel 5 [007fabf000 X[570]] nouveau :04:00.0: fifo: channel 5: killed nouveau :04:00.0: fifo: runlist 0: scheduled for recovery nouveau :04:00.0: fifo: engine 0: scheduled for recovery nouveau :04:00.0: X[570]: channel 5 killed! nouveau :04:00.0: fifo: fault 00 [READ] at 0038 engine 00 [GR] client 14 [HUB/SCC] reason 02 [PTE] on channel 5 [007fabf000 X[570]] nouveau :04:00.0: fifo: channel 5: killed nouveau :04:00.0: fifo: runlist 0: scheduled for recovery nouveau :04:00.0: fifo: engine 0: scheduled for recovery nouveau :04:00.0: X[570]: channel 5 killed! nouveau :04:00.0: fifo: fault 00 [READ] at 0038 engine 00 [GR] client 14 [HUB/SCC] reason 02 [PTE] on channel 5 [007fabf000 X[570]] nouveau :04:00.0: fifo: channel 5: killed nouveau :04:00.0: fifo: runlist 0: scheduled for recovery nouveau :04:00.0: fifo: engine 0: scheduled for recovery nouveau :04:00.0: X[570]: channel 5 killed! # inxi -b System:Kernel: 5.11.12-gentoo-Excavator x86_64 bits: 64 Desktop: MATE 1.24.0 Distro: Gentoo Base System release 2.7 Machine: Type: Desktop Mobo: ASRock model: A320M-HDV R3.0 serial: M80-BA024200938 UEFI: American Megatrends v: P3.10 date: 06/26/2019 CPU: Info: Quad Core AMD A10-9700E RADEON R7 10 COMPUTE CORES 4C+6G [MCP] speed: 3300 MHz min/max: 800/3000 MHz Graphics: Device-1: NVIDIA GK208B [GeForce GT 710] driver: nouveau v: kernel Display: x11 server: X.Org 1.20.10 driver: modesetting resolution: 1920x1080~60Hz OpenGL: renderer: NV106 v: 4.3 Mesa 20.3.5 Network: Device-1: Realtek RTL8111/8168/8411 PCI Express Gigabit Ethernet driver: r8169 # lspci 00:00.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 15h (Models 60h-6fh) Processor Root Complex 00:00.2 IOMMU: Advanced Micro Devices, Inc. [AMD] Family 15h (Models 60h-6fh) I/O Memory Management Unit 00:02.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 15h (Models 60h-6fh) Host Bridge 00:02.4 PCI bridge: Advanced Micro Devices, Inc. [AMD] Family 15h (Models 60h-6fh) Processor Root Port 00:02.5 PCI bridge: Advanced Micro Devices, Inc. [AMD] Family 15h (Models 60h-6fh) Processor Root Port 00:03.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 15h (Models 60h-6fh) Host Bridge 00:03.1 PCI bridge: Advanced Micro Devices, Inc. [AMD] Family 15h (Models 60h-6fh) Processor Root Port 00:08.0 Encryption controller: Advanced Micro Devices, Inc. [AMD] Carrizo Platform Security Processor 00:09.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Carrizo Audio Dummy Host Bridge 00:09.2 Audio device: Advanced Micro Devices, Inc. [AMD] Family 15h (Models 60h-6fh) Audio Controller 00:10.0 USB controller: Advanced Micro Devices, Inc. [AMD] FCH USB XHCI Controller (rev 20) 00:11.0 SATA controller: Advanced Micro Devices, Inc. [AMD] FCH SATA Controller [AHCI mode] (rev 49) 00:12.0 USB controller: Advanced Micro Devices, Inc. [AMD] FCH USB EHCI Controller (rev 49) 00:14.0 SMBus: Advanced Micro Devices, Inc. [AMD] FCH SMBus Controller (rev 4a) 00:14.3 ISA bridge: Advanced Micro Devices, Inc. [AMD] FCH LPC Bridge (rev 11) 00:18.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 15h (Models 60h-6fh) Processor Function 0 00:18.1 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 15h (Models 60h-6fh) Processor Function 1 00:18.2 Host bridge: Advanced Micro Devices, I
[Bug 212635] nouveau 0000:04:00.0: fifo: fault 00 [READ] at 0000000000380000 engine 00 [GR] client 14 [HUB/SCC] reason 02 [PTE] on channel 5 [007fabf000 X[570]]
https://bugzilla.kernel.org/show_bug.cgi?id=212635 --- Comment #1 from Erhard F. (erhar...@mailbox.org) --- Created attachment 296317 --> https://bugzilla.kernel.org/attachment.cgi?id=296317&action=edit kernel .config (kernel 5.11.12) -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re:Re: [PATCH v1] drm/radeon: Fix a missing check bug in radeon_dp_mst_detect()
My pleasure!
At 2021-04-09 04:17:36, "Alex Deucher" wrote:
>Applied. Thanks!
>
>Alex
>
>On Wed, Apr 7, 2021 at 2:23 AM wrote:
>>
>> From: Yingjie Wang
>>
>> In radeon_dp_mst_detect(), We should check whether or not @connector
>> has been unregistered from userspace. If the connector is unregistered,
>> we should return disconnected status.
>>
>> Fixes: 9843ead08f18 ("drm/radeon: add DisplayPort MST support (v2)")
>> Signed-off-by: Yingjie Wang
>> ---
>> drivers/gpu/drm/radeon/radeon_dp_mst.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c
>> b/drivers/gpu/drm/radeon/radeon_dp_mst.c
>> index 2c32186c4acd..4e4c937c36c6 100644
>> --- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
>> +++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
>> @@ -242,6 +242,9 @@ radeon_dp_mst_detect(struct drm_connector *connector,
>> to_radeon_connector(connector);
>> struct radeon_connector *master = radeon_connector->mst_port;
>>
>> + if (drm_connector_is_unregistered(connector))
>> + return connector_status_disconnected;
>> +
>> return drm_dp_mst_detect_port(connector, ctx, &master->mst_mgr,
>> radeon_connector->port);
>> }
>> --
>> 2.7.4
>>
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm/dsi: dsi_phy_28nm_8960: fix uninitialized variable access
The parent_name initialization was lost in refactoring, restore it now.
Fixes: 5d13459650b3 ("drm/msm/dsi: push provided clocks handling into a generic
code")
Reported-by: kernel test robot
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
index 582b1428f971..86e40a0d41a3 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
@@ -405,6 +405,10 @@ static int pll_28nm_register(struct dsi_pll_28nm
*pll_28nm, struct clk_hw **prov
if (!vco_name)
return -ENOMEM;
+ parent_name = devm_kzalloc(dev, 32, GFP_KERNEL);
+ if (!parent_name)
+ return -ENOMEM;
+
clk_name = devm_kzalloc(dev, 32, GFP_KERNEL);
if (!clk_name)
return -ENOMEM;
--
2.30.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/etnaviv: Remove useless error message
Am Do., 25. März 2021 um 13:33 Uhr schrieb Tian Tao : > > Fix the following coccicheck report: > > drivers/gpu/drm/etnaviv/etnaviv_gpu.c:1775:2-9: > line 1775 is redundant because platform_get_irq() already prints an error > > Remove dev_err() messages after platform_get_irq() failures. > > Signed-off-by: Tian Tao > Signed-off-by: Zihao Tang > Signed-off-by: Jay Fang Reviewed-by: Christian Gmeiner -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
