[PATCH v8 5/5] drm: protect drm_master pointers in drm_lease.c
drm_file->master pointers should be protected by
drm_device.master_mutex or drm_file.master_lookup_lock when being
dereferenced.
However, in drm_lease.c, there are multiple instances where
drm_file->master is accessed and dereferenced while neither lock is
held. This makes drm_lease.c vulnerable to use-after-free bugs.
We address this issue in 2 ways:
1. Add a new drm_file_get_master() function that calls drm_master_get
on drm_file->master while holding on to
drm_file.master_lookup_lock. Since drm_master_get increments the
reference count of master, this prevents master from being freed until
we unreference it with drm_master_put.
2. In each case where drm_file->master is directly accessed and
eventually dereferenced in drm_lease.c, we wrap the access in a call
to the new drm_file_get_master function, then unreference the master
pointer once we are done using it.
Reported-by: Daniel Vetter
Signed-off-by: Desmond Cheong Zhi Xi
Reviewed-by: Emil Velikov
---
drivers/gpu/drm/drm_auth.c | 25
drivers/gpu/drm/drm_lease.c | 81 -
include/drm/drm_auth.h | 1 +
include/drm/drm_file.h | 6 +++
4 files changed, 93 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 30a239901b36..f00354bec3fb 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -389,6 +389,31 @@ struct drm_master *drm_master_get(struct drm_master
*master)
}
EXPORT_SYMBOL(drm_master_get);
+/**
+ * drm_file_get_master - reference _file.master of @file_priv
+ * @file_priv: DRM file private
+ *
+ * Increments the reference count of @file_priv's _file.master and returns
+ * the _file.master. If @file_priv has no _file.master, returns NULL.
+ *
+ * Master pointers returned from this function should be unreferenced using
+ * drm_master_put().
+ */
+struct drm_master *drm_file_get_master(struct drm_file *file_priv)
+{
+ struct drm_master *master = NULL;
+
+ spin_lock(_priv->master_lookup_lock);
+ if (!file_priv->master)
+ goto unlock;
+ master = drm_master_get(file_priv->master);
+
+unlock:
+ spin_unlock(_priv->master_lookup_lock);
+ return master;
+}
+EXPORT_SYMBOL(drm_file_get_master);
+
static void drm_master_destroy(struct kref *kref)
{
struct drm_master *master = container_of(kref, struct drm_master,
refcount);
diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
index 00fb433bcef1..92eac73d9001 100644
--- a/drivers/gpu/drm/drm_lease.c
+++ b/drivers/gpu/drm/drm_lease.c
@@ -106,10 +106,19 @@ static bool _drm_has_leased(struct drm_master *master,
int id)
*/
bool _drm_lease_held(struct drm_file *file_priv, int id)
{
- if (!file_priv || !file_priv->master)
+ bool ret;
+ struct drm_master *master;
+
+ if (!file_priv)
return true;
- return _drm_lease_held_master(file_priv->master, id);
+ master = drm_file_get_master(file_priv);
+ if (!master)
+ return true;
+ ret = _drm_lease_held_master(master, id);
+ drm_master_put();
+
+ return ret;
}
/**
@@ -128,13 +137,22 @@ bool drm_lease_held(struct drm_file *file_priv, int id)
struct drm_master *master;
bool ret;
- if (!file_priv || !file_priv->master || !file_priv->master->lessor)
+ if (!file_priv)
return true;
- master = file_priv->master;
+ master = drm_file_get_master(file_priv);
+ if (!master)
+ return true;
+ if (!master->lessor) {
+ ret = true;
+ goto out;
+ }
mutex_lock(>dev->mode_config.idr_mutex);
ret = _drm_lease_held_master(master, id);
mutex_unlock(>dev->mode_config.idr_mutex);
+
+out:
+ drm_master_put();
return ret;
}
@@ -154,10 +172,16 @@ uint32_t drm_lease_filter_crtcs(struct drm_file
*file_priv, uint32_t crtcs_in)
int count_in, count_out;
uint32_t crtcs_out = 0;
- if (!file_priv || !file_priv->master || !file_priv->master->lessor)
+ if (!file_priv)
return crtcs_in;
- master = file_priv->master;
+ master = drm_file_get_master(file_priv);
+ if (!master)
+ return crtcs_in;
+ if (!master->lessor) {
+ crtcs_out = crtcs_in;
+ goto out;
+ }
dev = master->dev;
count_in = count_out = 0;
@@ -176,6 +200,9 @@ uint32_t drm_lease_filter_crtcs(struct drm_file *file_priv,
uint32_t crtcs_in)
count_in++;
}
mutex_unlock(>dev->mode_config.idr_mutex);
+
+out:
+ drm_master_put();
return crtcs_out;
}
@@ -489,7 +516,7 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
size_t object_count;
int ret = 0;
struct idr leases;
- struct drm_master *lessor = lessor_priv->master;
+ struct drm_master *lessor;
[PATCH v8 4/5] drm: serialize drm_file.master with a new spinlock
Currently, drm_file.master pointers should be protected by
drm_device.master_mutex when being dereferenced. This is because
drm_file.master is not invariant for the lifetime of drm_file. If
drm_file is not the creator of master, then drm_file.is_master is
false, and a call to drm_setmaster_ioctl will invoke
drm_new_set_master, which then allocates a new master for drm_file and
puts the old master.
Thus, without holding drm_device.master_mutex, the old value of
drm_file.master could be freed while it is being used by another
concurrent process.
However, it is not always possible to lock drm_device.master_mutex to
dereference drm_file.master. Through the fbdev emulation code, this
might occur in a deep nest of other locks. But drm_device.master_mutex
is also the outermost lock in the nesting hierarchy, so this leads to
potential deadlocks.
To address this, we introduce a new spin lock at the bottom of the
lock hierarchy that only serializes drm_file.master. With this change,
the value of drm_file.master changes only when both
drm_device.master_mutex and drm_file.master_lookup_lock are
held. Hence, any process holding either of those locks can ensure that
the value of drm_file.master will not change concurrently.
Since no lock depends on the new drm_file.master_lookup_lock, when
drm_file.master is dereferenced, but drm_device.master_mutex cannot be
held, we can safely protect the master pointer with
drm_file.master_lookup_lock.
Reported-by: Daniel Vetter
Signed-off-by: Desmond Cheong Zhi Xi
---
drivers/gpu/drm/drm_auth.c | 17 +++--
drivers/gpu/drm/drm_file.c | 1 +
include/drm/drm_file.h | 12 +---
3 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index ab1863c5a5a0..30a239901b36 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -164,16 +164,18 @@ static void drm_set_master(struct drm_device *dev, struct
drm_file *fpriv,
static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv)
{
struct drm_master *old_master;
+ struct drm_master *new_master;
lockdep_assert_held_once(>master_mutex);
WARN_ON(fpriv->is_master);
old_master = fpriv->master;
- fpriv->master = drm_master_create(dev);
- if (!fpriv->master) {
- fpriv->master = old_master;
+ new_master = drm_master_create(dev);
+ if (!new_master)
return -ENOMEM;
- }
+ spin_lock(>master_lookup_lock);
+ fpriv->master = new_master;
+ spin_unlock(>master_lookup_lock);
fpriv->is_master = 1;
fpriv->authenticated = 1;
@@ -332,10 +334,13 @@ int drm_master_open(struct drm_file *file_priv)
* any master object for render clients
*/
mutex_lock(>master_mutex);
- if (!dev->master)
+ if (!dev->master) {
ret = drm_new_set_master(dev, file_priv);
- else
+ } else {
+ spin_lock(_priv->master_lookup_lock);
file_priv->master = drm_master_get(dev->master);
+ spin_unlock(_priv->master_lookup_lock);
+ }
mutex_unlock(>master_mutex);
return ret;
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index d4f0bac6f8f8..ceb1a9723855 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -176,6 +176,7 @@ struct drm_file *drm_file_alloc(struct drm_minor *minor)
init_waitqueue_head(>event_wait);
file->event_space = 4096; /* set aside 4k for event buffer */
+ spin_lock_init(>master_lookup_lock);
mutex_init(>event_read_lock);
if (drm_core_check_feature(dev, DRIVER_GEM))
diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index b81b3bfb08c8..9b82988e3427 100644
--- a/include/drm/drm_file.h
+++ b/include/drm/drm_file.h
@@ -226,15 +226,21 @@ struct drm_file {
/**
* @master:
*
-* Master this node is currently associated with. Only relevant if
-* drm_is_primary_client() returns true. Note that this only
-* matches _device.master if the master is the currently active one.
+* Master this node is currently associated with. Protected by struct
+* _device.master_mutex, and serialized by @master_lookup_lock.
+*
+* Only relevant if drm_is_primary_client() returns true. Note that
+* this only matches _device.master if the master is the currently
+* active one.
*
* See also @authentication and @is_master and the :ref:`section on
* primary nodes and authentication `.
*/
struct drm_master *master;
+ /** @master_lock: Serializes @master. */
+ spinlock_t master_lookup_lock;
+
/** @pid: Process that opened this file. */
struct pid *pid;
--
2.25.1
[PATCH v8 3/5] drm: add a locked version of drm_is_current_master
While checking the master status of the DRM file in
drm_is_current_master(), the device's master mutex should be
held. Without the mutex, the pointer fpriv->master may be freed
concurrently by another process calling drm_setmaster_ioctl(). This
could lead to use-after-free errors when the pointer is subsequently
dereferenced in drm_lease_owner().
The callers of drm_is_current_master() from drm_auth.c hold the
device's master mutex, but external callers do not. Hence, we implement
drm_is_current_master_locked() to be used within drm_auth.c, and
modify drm_is_current_master() to grab the device's master mutex
before checking the master status.
Reported-by: Daniel Vetter
Signed-off-by: Desmond Cheong Zhi Xi
Reviewed-by: Emil Velikov
---
drivers/gpu/drm/drm_auth.c | 51 --
1 file changed, 32 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index f00e5abdbbf4..ab1863c5a5a0 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -61,6 +61,35 @@
* trusted clients.
*/
+static bool drm_is_current_master_locked(struct drm_file *fpriv)
+{
+ lockdep_assert_held_once(>minor->dev->master_mutex);
+
+ return fpriv->is_master && drm_lease_owner(fpriv->master) ==
fpriv->minor->dev->master;
+}
+
+/**
+ * drm_is_current_master - checks whether @priv is the current master
+ * @fpriv: DRM file private
+ *
+ * Checks whether @fpriv is current master on its device. This decides whether
a
+ * client is allowed to run DRM_MASTER IOCTLs.
+ *
+ * Most of the modern IOCTL which require DRM_MASTER are for kernel modesetting
+ * - the current master is assumed to own the non-shareable display hardware.
+ */
+bool drm_is_current_master(struct drm_file *fpriv)
+{
+ bool ret;
+
+ mutex_lock(>minor->dev->master_mutex);
+ ret = drm_is_current_master_locked(fpriv);
+ mutex_unlock(>minor->dev->master_mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL(drm_is_current_master);
+
int drm_getmagic(struct drm_device *dev, void *data, struct drm_file
*file_priv)
{
struct drm_auth *auth = data;
@@ -223,7 +252,7 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data,
if (ret)
goto out_unlock;
- if (drm_is_current_master(file_priv))
+ if (drm_is_current_master_locked(file_priv))
goto out_unlock;
if (dev->master) {
@@ -272,7 +301,7 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data,
if (ret)
goto out_unlock;
- if (!drm_is_current_master(file_priv)) {
+ if (!drm_is_current_master_locked(file_priv)) {
ret = -EINVAL;
goto out_unlock;
}
@@ -321,7 +350,7 @@ void drm_master_release(struct drm_file *file_priv)
if (file_priv->magic)
idr_remove(_priv->master->magic_map, file_priv->magic);
- if (!drm_is_current_master(file_priv))
+ if (!drm_is_current_master_locked(file_priv))
goto out;
drm_legacy_lock_master_cleanup(dev, master);
@@ -342,22 +371,6 @@ void drm_master_release(struct drm_file *file_priv)
mutex_unlock(>master_mutex);
}
-/**
- * drm_is_current_master - checks whether @priv is the current master
- * @fpriv: DRM file private
- *
- * Checks whether @fpriv is current master on its device. This decides whether
a
- * client is allowed to run DRM_MASTER IOCTLs.
- *
- * Most of the modern IOCTL which require DRM_MASTER are for kernel modesetting
- * - the current master is assumed to own the non-shareable display hardware.
- */
-bool drm_is_current_master(struct drm_file *fpriv)
-{
- return fpriv->is_master && drm_lease_owner(fpriv->master) ==
fpriv->minor->dev->master;
-}
-EXPORT_SYMBOL(drm_is_current_master);
-
/**
* drm_master_get - reference a master pointer
* @master: drm_master
--
2.25.1
[PATCH v8 2/5] drm: avoid blocking in drm_clients_info's rcu section
Inside drm_clients_info, the rcu_read_lock is held to lock
pid_task()->comm. However, within this protected section, a call to
drm_is_current_master is made, which involves a mutex lock in a future
patch. However, this is illegal because the mutex lock might block
while in the RCU read-side critical section.
Since drm_is_current_master isn't protected by rcu_read_lock, we avoid
this by moving it out of the RCU critical section.
The following report came from intel-gfx ci's
igt@debugfs_test@read_all_entries testcase:
=
[ BUG: Invalid wait context ]
5.13.0-CI-Patchwork_20515+ #1 Tainted: GW
-
debugfs_test/1101 is trying to lock:
888132d901a8 (>master_mutex){+.+.}-{3:3}, at:
drm_is_current_master+0x1e/0x50
other info that might help us debug this:
context-{4:4}
3 locks held by debugfs_test/1101:
#0: 88810fdffc90 (>lock){+.+.}-{3:3}, at:
seq_read_iter+0x53/0x3b0
#1: 888132d90240 (>filelist_mutex){+.+.}-{3:3}, at:
drm_clients_info+0x63/0x2a0
#2: 82734220 (rcu_read_lock){}-{1:2}, at:
drm_clients_info+0x1b1/0x2a0
stack backtrace:
CPU: 8 PID: 1101 Comm: debugfs_test Tainted: GW
5.13.0-CI-Patchwork_20515+ #1
Hardware name: Intel Corporation CometLake Client Platform/CometLake S
UDIMM (ERB/CRB), BIOS CMLSFWR1.R00.1263.D00.1906260926 06/26/2019
Call Trace:
dump_stack+0x7f/0xad
__lock_acquire.cold.78+0x2af/0x2ca
lock_acquire+0xd3/0x300
? drm_is_current_master+0x1e/0x50
? __mutex_lock+0x76/0x970
? lockdep_hardirqs_on+0xbf/0x130
__mutex_lock+0xab/0x970
? drm_is_current_master+0x1e/0x50
? drm_is_current_master+0x1e/0x50
? drm_is_current_master+0x1e/0x50
drm_is_current_master+0x1e/0x50
drm_clients_info+0x107/0x2a0
seq_read_iter+0x178/0x3b0
seq_read+0x104/0x150
full_proxy_read+0x4e/0x80
vfs_read+0xa5/0x1b0
ksys_read+0x5a/0xd0
do_syscall_64+0x39/0xb0
entry_SYSCALL_64_after_hwframe+0x44/0xae
Signed-off-by: Desmond Cheong Zhi Xi
---
drivers/gpu/drm/drm_debugfs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 3d7182001004..b0a826489488 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -91,6 +91,7 @@ static int drm_clients_info(struct seq_file *m, void *data)
mutex_lock(>filelist_mutex);
list_for_each_entry_reverse(priv, >filelist, lhead) {
struct task_struct *task;
+ bool is_current_master = drm_is_current_master(priv);
rcu_read_lock(); /* locks pid_task()->comm */
task = pid_task(priv->pid, PIDTYPE_PID);
@@ -99,7 +100,7 @@ static int drm_clients_info(struct seq_file *m, void *data)
task ? task->comm : "",
pid_vnr(priv->pid),
priv->minor->index,
- drm_is_current_master(priv) ? 'y' : 'n',
+ is_current_master ? 'y' : 'n',
priv->authenticated ? 'y' : 'n',
from_kuid_munged(seq_user_ns(m), uid),
priv->magic);
--
2.25.1
[PATCH v8 1/5] drm: avoid circular locks in drm_mode_getconnector
In preparation for a future patch to take a lock on
drm_device.master_mutex inside drm_is_current_master(), we first move
the call to drm_is_current_master() in drm_mode_getconnector out from the
section locked by >mode_config.mutex. This avoids creating a
circular lock dependency.
Failing to avoid this lock dependency produces the following lockdep
splat:
==
WARNING: possible circular locking dependency detected
5.13.0-rc7-CI-CI_DRM_10254+ #1 Not tainted
--
kms_frontbuffer/1087 is trying to acquire lock:
88810dcd01a8 (>master_mutex){+.+.}-{3:3}, at:
drm_is_current_master+0x1b/0x40
but task is already holding lock:
88810dcd0488 (>mode_config.mutex){+.+.}-{3:3}, at:
drm_mode_getconnector+0x1c6/0x4a0
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #2 (>mode_config.mutex){+.+.}-{3:3}:
__mutex_lock+0xab/0x970
drm_client_modeset_probe+0x22e/0xca0
__drm_fb_helper_initial_config_and_unlock+0x42/0x540
intel_fbdev_initial_config+0xf/0x20 [i915]
async_run_entry_fn+0x28/0x130
process_one_work+0x26d/0x5c0
worker_thread+0x37/0x380
kthread+0x144/0x170
ret_from_fork+0x1f/0x30
-> #1 (>modeset_mutex){+.+.}-{3:3}:
__mutex_lock+0xab/0x970
drm_client_modeset_commit_locked+0x1c/0x180
drm_client_modeset_commit+0x1c/0x40
__drm_fb_helper_restore_fbdev_mode_unlocked+0x88/0xb0
drm_fb_helper_set_par+0x34/0x40
intel_fbdev_set_par+0x11/0x40 [i915]
fbcon_init+0x270/0x4f0
visual_init+0xc6/0x130
do_bind_con_driver+0x1e5/0x2d0
do_take_over_console+0x10e/0x180
do_fbcon_takeover+0x53/0xb0
register_framebuffer+0x22d/0x310
__drm_fb_helper_initial_config_and_unlock+0x36c/0x540
intel_fbdev_initial_config+0xf/0x20 [i915]
async_run_entry_fn+0x28/0x130
process_one_work+0x26d/0x5c0
worker_thread+0x37/0x380
kthread+0x144/0x170
ret_from_fork+0x1f/0x30
-> #0 (>master_mutex){+.+.}-{3:3}:
__lock_acquire+0x151e/0x2590
lock_acquire+0xd1/0x3d0
__mutex_lock+0xab/0x970
drm_is_current_master+0x1b/0x40
drm_mode_getconnector+0x37e/0x4a0
drm_ioctl_kernel+0xa8/0xf0
drm_ioctl+0x1e8/0x390
__x64_sys_ioctl+0x6a/0xa0
do_syscall_64+0x39/0xb0
entry_SYSCALL_64_after_hwframe+0x44/0xae
other info that might help us debug this:
Chain exists of: >master_mutex --> >modeset_mutex -->
>mode_config.mutex
Possible unsafe locking scenario:
CPU0CPU1
lock(>mode_config.mutex);
lock(>modeset_mutex);
lock(>mode_config.mutex);
lock(>master_mutex);
*** DEADLOCK ***
1 lock held by kms_frontbuffer/1087:
#0: 88810dcd0488 (>mode_config.mutex){+.+.}-{3:3}, at:
drm_mode_getconnector+0x1c6/0x4a0
stack backtrace:
CPU: 7 PID: 1087 Comm: kms_frontbuffer Not tainted 5.13.0-rc7-CI-CI_DRM_10254+
#1
Hardware name: Intel Corporation Ice Lake Client Platform/IceLake U DDR4 SODIMM
PD RVP TLC, BIOS ICLSFWR1.R00.3234.A01.1906141750 06/14/2019
Call Trace:
dump_stack+0x7f/0xad
check_noncircular+0x12e/0x150
__lock_acquire+0x151e/0x2590
lock_acquire+0xd1/0x3d0
__mutex_lock+0xab/0x970
drm_is_current_master+0x1b/0x40
drm_mode_getconnector+0x37e/0x4a0
drm_ioctl_kernel+0xa8/0xf0
drm_ioctl+0x1e8/0x390
__x64_sys_ioctl+0x6a/0xa0
do_syscall_64+0x39/0xb0
entry_SYSCALL_64_after_hwframe+0x44/0xae
Reported-by: Daniel Vetter
Signed-off-by: Desmond Cheong Zhi Xi
Reviewed-by: Emil Velikov
---
drivers/gpu/drm/drm_connector.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index da39e7ff6965..2ba257b1ae20 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2414,6 +2414,7 @@ int drm_mode_getconnector(struct drm_device *dev, void
*data,
struct drm_mode_modeinfo u_mode;
struct drm_mode_modeinfo __user *mode_ptr;
uint32_t __user *encoder_ptr;
+ bool is_current_master;
if (!drm_core_check_feature(dev, DRIVER_MODESET))
return -EOPNOTSUPP;
@@ -2444,9 +2445,11 @@ int drm_mode_getconnector(struct drm_device *dev, void
*data,
out_resp->connector_type = connector->connector_type;
out_resp->connector_type_id = connector->connector_type_id;
+ is_current_master = drm_is_current_master(file_priv);
+
mutex_lock(>mode_config.mutex);
if (out_resp->count_modes == 0) {
- if (drm_is_current_master(file_priv))
+ if (is_current_master)
connector->funcs->fill_modes(connector,
dev->mode_config.max_width,
[PATCH v8 0/5] drm: address potential UAF bugs with drm_master ptrs
Hi, In the previous thread on this series we decided to remove a patch that was violating a lockdep requirement in drm_lease. In addition to this change, I took a closer look at the CI logs for the Basic Acceptance Tests and noticed that another regression was introduced. The new patch 2 is a response to this. Overall, this series addresses potential use-after-free errors when dereferencing pointers to struct drm_master. These were identified after one such bug was caught by Syzbot in drm_getunique(): https://syzkaller.appspot.com/bug?id=148d2f1dfac64af52ffd27b661981a540724f803 The series is broken up into five patches: 1. Move a call to drm_is_current_master() out from a section locked by >mode_config.mutex in drm_mode_getconnector(). This patch does not apply to stable. 2. Move a call to drm_is_current_master() out from the RCU read-side critical section in drm_clients_info(). 3. Implement a locked version of drm_is_current_master() function that's used within drm_auth.c. 4. Serialize drm_file.master by introducing a new spinlock that's held whenever the value of drm_file.master changes. 5. Identify areas in drm_lease.c where pointers to struct drm_master are dereferenced, and ensure that the master pointers are not freed during use. v7 -> v8: - Remove the patch that moves the call to _drm_lease_held out from the section locked by >mode_config.idr_mutex in __drm_mode_object_find. This patch violated an existing lockdep requirement as reported by the intel-gfx CI. - Added a new patch that moves a call to drm_is_current_master out from the RCU critical section in drm_clients_info. This was reported by the intel-gfx CI. v6 -> v7: - Modify code alignment as suggested by the intel-gfx CI. - Add a new patch to the series that adds a new lock to serialize drm_file.master, in response to the lockdep splat by the intel-gfx CI. - Update drm_file_get_master to use the new drm_file.master_lock instead of drm_device.master_mutex, in response to the lockdep splat by the intel-gfx CI. v5 -> v6: - Add a new patch to the series that moves the call to _drm_lease_held out from the section locked by >mode_config.idr_mutex in __drm_mode_object_find. - Clarify the kerneldoc for dereferencing drm_file.master, as suggested by Daniel Vetter. - Refactor error paths with goto labels so that each function only has a single drm_master_put(), as suggested by Emil Velikov. - Modify comparisons to NULL into "!master", as suggested by the intel-gfx CI. v4 -> v5: - Add a new patch to the series that moves the call to drm_is_current_master in drm_mode_getconnector out from the section locked by >mode_config.mutex. - Additionally, added a missing semicolon to the patch, caught by the intel-gfx CI. v3 -> v4: - Move the call to drm_is_current_master in drm_mode_getconnector out from the section locked by >mode_config.mutex. As suggested by Daniel Vetter. This avoids a circular lock lock dependency as reported here https://patchwork.freedesktop.org/patch/440406/ - Inside drm_is_current_master, instead of grabbing >master->dev->master_mutex, we grab >minor->dev->master_mutex to avoid dereferencing a null ptr if fpriv->master is not set. - Modify kerneldoc formatting for drm_file.master, as suggested by Daniel Vetter. - Additionally, add a file_priv->master NULL check inside drm_file_get_master, and handle the NULL result accordingly in drm_lease.c. As suggested by Daniel Vetter. v2 -> v3: - Move the definition of drm_is_current_master and the _locked version higher up in drm_auth.c to avoid needing a forward declaration of drm_is_current_master_locked. As suggested by Daniel Vetter. - Instead of leaking drm_device.master_mutex into drm_lease.c to protect drm_master pointers, add a new drm_file_get_master() function that returns drm_file->master while increasing its reference count, to prevent drm_file->master from being freed. As suggested by Daniel Vetter. v1 -> v2: - Move the lock and assignment before the DRM_DEBUG_LEASE in drm_mode_get_lease_ioctl, as suggested by Emil Velikov. Desmond Cheong Zhi Xi (5): drm: avoid circular locks in drm_mode_getconnector drm: avoid blocking in drm_clients_info's rcu section drm: add a locked version of drm_is_current_master drm: serialize drm_file.master with a new spinlock drm: protect drm_master pointers in drm_lease.c drivers/gpu/drm/drm_auth.c | 93 - drivers/gpu/drm/drm_connector.c | 5 +- drivers/gpu/drm/drm_debugfs.c | 3 +- drivers/gpu/drm/drm_file.c | 1 + drivers/gpu/drm/drm_lease.c | 81 +--- include/drm/drm_auth.h | 1 + include/drm/drm_file.h | 18 +-- 7 files changed, 152 insertions(+), 50 deletions(-) -- 2.25.1
Re: [v1] drm/msm/disp/dpu1: add safe lut config in dpu driver
On 2021-07-09 16:10, Kalyan Thota wrote:
Add safe lut configuration for all the targets in dpu
driver as per QOS recommendation.
Issue reported on SC7280:
With wait-for-safe feature in smmu enabled, RT client
buffer levels are checked to be safe before smmu invalidation.
Since display was always set to unsafe it was delaying the
invalidaiton process thus impacting the performance on NRT clients
such as eMMC and NVMe.
Validated this change on SC7280, With this change eMMC performance
has improved significantly.
Signed-off-by: Kalyan Thota
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index d01c4c9..2e482cd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -974,6 +974,7 @@ static const struct dpu_perf_cfg sdm845_perf_data =
{
.amortizable_threshold = 25,
.min_prefill_lines = 24,
.danger_lut_tbl = {0xf, 0x, 0x0},
+ .safe_lut_tbl = {0xfff0, 0xf000, 0x},
.qos_lut_tbl = {
{.nentry = ARRAY_SIZE(sdm845_qos_linear),
.entries = sdm845_qos_linear
@@ -1001,6 +1002,7 @@ static const struct dpu_perf_cfg sc7180_perf_data
= {
.min_dram_ib = 160,
.min_prefill_lines = 24,
.danger_lut_tbl = {0xff, 0x, 0x0},
+ .safe_lut_tbl = {0xfff0, 0xff00, 0x},
.qos_lut_tbl = {
{.nentry = ARRAY_SIZE(sc7180_qos_linear),
.entries = sc7180_qos_linear
@@ -1028,6 +1030,7 @@ static const struct dpu_perf_cfg sm8150_perf_data
= {
.min_dram_ib = 80,
.min_prefill_lines = 24,
.danger_lut_tbl = {0xf, 0x, 0x0},
+ .safe_lut_tbl = {0xfff8, 0xf000, 0x},
.qos_lut_tbl = {
{.nentry = ARRAY_SIZE(sm8150_qos_linear),
.entries = sm8150_qos_linear
@@ -1056,6 +1059,7 @@ static const struct dpu_perf_cfg sm8250_perf_data
= {
.min_dram_ib = 80,
.min_prefill_lines = 35,
.danger_lut_tbl = {0xf, 0x, 0x0},
+ .safe_lut_tbl = {0xfff0, 0xff00, 0x},
.qos_lut_tbl = {
{.nentry = ARRAY_SIZE(sc7180_qos_linear),
.entries = sc7180_qos_linear
@@ -1084,6 +1088,7 @@ static const struct dpu_perf_cfg sc7280_perf_data
= {
.min_dram_ib = 160,
.min_prefill_lines = 24,
.danger_lut_tbl = {0x, 0x, 0x0},
+ .safe_lut_tbl = {0xff00, 0xff00, 0x},
.qos_lut_tbl = {
{.nentry = ARRAY_SIZE(sc7180_qos_macrotile),
.entries = sc7180_qos_macrotile
Tested-by: Sai Prakash Ranjan
(sc7280, sc7180)
This will need fixes and stable tag and I think this should also fix the
wait-for-safe issue with sdm845 (ufs/usb speed slowdown with display
active)
which we have in arm-smmu-qcom.
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
Re: drm/amd/display: Simplify hdcp validate_bksv
Hi Joe, I love your patch! Yet something to improve: [auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on drm-tip/drm-tip drm-exynos/exynos-drm-next linus/master v5.14-rc1 next-20210709] [cannot apply to kees/for-next/pstore tegra-drm/drm/tegra/for-next drm/drm-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Joe-Perches/drm-amd-display-Simplify-hdcp-validate_bksv/20210712-034708 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: arm64-allyesconfig (attached as .config) compiler: aarch64-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/66fae2c1becdcb71c95f2c6a6413de4dfe1deb51 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Joe-Perches/drm-amd-display-Simplify-hdcp-validate_bksv/20210712-034708 git checkout 66fae2c1becdcb71c95f2c6a6413de4dfe1deb51 # save the attached .config to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross O=build_dir ARCH=arm64 SHELL=/bin/bash If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): aarch64-linux-ld: Unexpected GOT/PLT entries detected! aarch64-linux-ld: Unexpected run-time procedure linkages detected! aarch64-linux-ld: drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.o: in function `validate_bksv': >> hdcp1_execution.c:(.text+0x5ac): undefined reference to `__popcountdi2' --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
[PATCH v2, 0/3] drm/mediatek: Separate aal module
Chnage since v1: - seprate patch - keep gamma register setting with cpu Yongqiang Niu (3): drm/mediatek: Separate aal module drm/mediatek: add mt8183 aal support arm64: dts: mt8183: refine aal compatible name arch/arm64/boot/dts/mediatek/mt8183.dtsi| 3 +- drivers/gpu/drm/mediatek/Makefile | 3 +- drivers/gpu/drm/mediatek/mtk_disp_aal.c | 167 drivers/gpu/drm/mediatek/mtk_disp_drv.h | 9 ++ drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 39 +-- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 8 +- drivers/gpu/drm/mediatek/mtk_drm_drv.h | 1 + 7 files changed, 188 insertions(+), 42 deletions(-) create mode 100644 drivers/gpu/drm/mediatek/mtk_disp_aal.c -- 1.8.1.1.dirty
[PATCH v2, 2/3] drm/mediatek: add mt8183 aal support
This patch add mt8183 private data
Signed-off-by: Yongqiang Niu
---
drivers/gpu/drm/mediatek/mtk_disp_aal.c | 1 +
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
index fb212e96..64b4528 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
@@ -151,6 +151,7 @@ static int mtk_disp_aal_remove(struct platform_device *pdev)
static const struct of_device_id mtk_disp_aal_driver_dt_match[] = {
{ .compatible = "mediatek,mt8173-disp-aal",
.data = _aal_driver_data},
+ { .compatible = "mediatek,mt8183-disp-aal"},
{},
};
MODULE_DEVICE_TABLE(of, mtk_disp_aal_driver_dt_match);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 67a585e..143ba24 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -420,6 +420,8 @@ static void mtk_drm_unbind(struct device *dev)
.data = (void *)MTK_DISP_COLOR },
{ .compatible = "mediatek,mt8173-disp-aal",
.data = (void *)MTK_DISP_AAL},
+ { .compatible = "mediatek,mt8183-disp-aal",
+ .data = (void *)MTK_DISP_AAL},
{ .compatible = "mediatek,mt8173-disp-gamma",
.data = (void *)MTK_DISP_GAMMA, },
{ .compatible = "mediatek,mt8183-disp-gamma",
--
1.8.1.1.dirty
[PATCH v2, 1/3] drm/mediatek: Separate aal module
mt8183 aal has no gamma function
Signed-off-by: Yongqiang Niu
---
drivers/gpu/drm/mediatek/Makefile | 3 +-
drivers/gpu/drm/mediatek/mtk_disp_aal.c | 166
drivers/gpu/drm/mediatek/mtk_disp_drv.h | 9 ++
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 39 +--
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 6 +-
drivers/gpu/drm/mediatek/mtk_drm_drv.h | 1 +
6 files changed, 184 insertions(+), 40 deletions(-)
create mode 100644 drivers/gpu/drm/mediatek/mtk_disp_aal.c
diff --git a/drivers/gpu/drm/mediatek/Makefile
b/drivers/gpu/drm/mediatek/Makefile
index dc54a7a..29098d7 100644
--- a/drivers/gpu/drm/mediatek/Makefile
+++ b/drivers/gpu/drm/mediatek/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
-mediatek-drm-y := mtk_disp_ccorr.o \
+mediatek-drm-y := mtk_disp_aal.o \
+ mtk_disp_ccorr.o \
mtk_disp_color.o \
mtk_disp_gamma.o \
mtk_disp_ovl.o \
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
new file mode 100644
index 000..fb212e96
--- /dev/null
+++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
@@ -0,0 +1,166 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2021 MediaTek Inc.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "mtk_disp_drv.h"
+#include "mtk_drm_crtc.h"
+#include "mtk_drm_ddp_comp.h"
+
+#define DISP_AAL_EN0x
+#define AAL_EN BIT(0)
+#define DISP_AAL_SIZE 0x0030
+
+
+struct mtk_disp_aal_data {
+ bool has_gamma;
+};
+
+/**
+ * struct mtk_disp_aal - DISP_AAL driver structure
+ * @ddp_comp - structure containing type enum and hardware resources
+ * @crtc - associated crtc to report irq events to
+ */
+struct mtk_disp_aal {
+ struct clk *clk;
+ void __iomem *regs;
+ struct cmdq_client_reg cmdq_reg;
+ const struct mtk_disp_aal_data *data;
+};
+
+int mtk_aal_clk_enable(struct device *dev)
+{
+ struct mtk_disp_aal *aal = dev_get_drvdata(dev);
+
+ return clk_prepare_enable(aal->clk);
+}
+
+void mtk_aal_clk_disable(struct device *dev)
+{
+ struct mtk_disp_aal *aal = dev_get_drvdata(dev);
+
+ clk_disable_unprepare(aal->clk);
+}
+
+void mtk_aal_config(struct device *dev, unsigned int w,
+ unsigned int h, unsigned int vrefresh,
+ unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
+{
+ struct mtk_disp_aal *aal = dev_get_drvdata(dev);
+
+ mtk_ddp_write(cmdq_pkt, w << 16 | h, >cmdq_reg, aal->regs,
DISP_AAL_SIZE);
+}
+
+void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state)
+{
+ struct mtk_disp_aal *aal = dev_get_drvdata(dev);
+
+ if (aal->data && aal->data->has_gamma)
+ mtk_gamma_set_common(aal->regs, state);
+}
+
+void mtk_aal_start(struct device *dev)
+{
+ struct mtk_disp_aal *aal = dev_get_drvdata(dev);
+
+ writel(AAL_EN, aal->regs + DISP_AAL_EN);
+}
+
+void mtk_aal_stop(struct device *dev)
+{
+ struct mtk_disp_aal *aal = dev_get_drvdata(dev);
+
+ writel_relaxed(0x0, aal->regs + DISP_AAL_EN);
+}
+
+static int mtk_disp_aal_bind(struct device *dev, struct device *master,
+ void *data)
+{
+ return 0;
+}
+
+static void mtk_disp_aal_unbind(struct device *dev, struct device *master,
+ void *data)
+{
+}
+
+static const struct component_ops mtk_disp_aal_component_ops = {
+ .bind = mtk_disp_aal_bind,
+ .unbind = mtk_disp_aal_unbind,
+};
+
+static int mtk_disp_aal_probe(struct platform_device *pdev)
+{
+ struct device *dev = >dev;
+ struct mtk_disp_aal *priv;
+ struct resource *res;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(priv->clk)) {
+ dev_err(dev, "failed to get aal clk\n");
+ return PTR_ERR(priv->clk);
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ priv->regs = devm_ioremap_resource(dev, res);
+ if (IS_ERR(priv->regs)) {
+ dev_err(dev, "failed to ioremap aal\n");
+ return PTR_ERR(priv->regs);
+ }
+
+#if IS_REACHABLE(CONFIG_MTK_CMDQ)
+ ret = cmdq_dev_get_client_reg(dev, >cmdq_reg, 0);
+ if (ret)
+ dev_dbg(dev, "get mediatek,gce-client-reg fail!\n");
+#endif
+
+ priv->data = of_device_get_match_data(dev);
+ platform_set_drvdata(pdev, priv);
+
+ ret = component_add(dev, _disp_aal_component_ops);
+ if (ret)
+ dev_err(dev, "Failed to add component: %d\n", ret);
+
+ return ret;
+}
+
+static int mtk_disp_aal_remove(struct platform_device *pdev)
+{
+
[PATCH v2, 3/3] arm64: dts: mt8183: refine aal compatible name
mt8183 aal is different with mt8173
remove mt8173 compatible name for mt8183 aal
Signed-off-by: Yongqiang Niu
---
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index c5e822b..1ca5f9b 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -1204,8 +1204,7 @@
};
aal0: aal@1401 {
- compatible = "mediatek,mt8183-disp-aal",
-"mediatek,mt8173-disp-aal";
+ compatible = "mediatek,mt8183-disp-aal";
reg = <0 0x1401 0 0x1000>;
interrupts = ;
power-domains = < MT8183_POWER_DOMAIN_DISP>;
--
1.8.1.1.dirty
Re: drm/amd/display: Simplify hdcp validate_bksv
On Mon, 2021-07-12 at 07:02 +0800, kernel test robot wrote: > Hi Joe, > > I love your patch! Yet something to improve: > > [auto build test ERROR on drm-intel/for-linux-next] > [also build test ERROR on drm-exynos/exynos-drm-next linus/master > next-20210709] > [cannot apply to kees/for-next/pstore tegra-drm/drm/tegra/for-next > drm/drm-next v5.13] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch] > > url: > https://github.com/0day-ci/linux/commits/Joe-Perches/drm-amd-display-Simplify-hdcp-validate_bksv/20210712-034708 > base: git://anongit.freedesktop.org/drm-intel for-linux-next > config: i386-randconfig-a003-20210712 (attached as .config) > compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 > reproduce (this is a W=1 build): > # > https://github.com/0day-ci/linux/commit/66fae2c1becdcb71c95f2c6a6413de4dfe1deb51 > git remote add linux-review https://github.com/0day-ci/linux > git fetch --no-tags linux-review > Joe-Perches/drm-amd-display-Simplify-hdcp-validate_bksv/20210712-034708 > git checkout 66fae2c1becdcb71c95f2c6a6413de4dfe1deb51 > # save the attached .config to linux build tree > make W=1 ARCH=i386 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot > > All errors (new ones prefixed by >>, old ones prefixed by <<): > > > > ERROR: modpost: "__popcountdi2" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] > > > undefined! curious. Anyone know why?
Re: drm/amd/display: Simplify hdcp validate_bksv
Hi Joe, I love your patch! Yet something to improve: [auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on drm-exynos/exynos-drm-next linus/master next-20210709] [cannot apply to kees/for-next/pstore tegra-drm/drm/tegra/for-next drm/drm-next v5.13] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Joe-Perches/drm-amd-display-Simplify-hdcp-validate_bksv/20210712-034708 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: i386-randconfig-a003-20210712 (attached as .config) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 reproduce (this is a W=1 build): # https://github.com/0day-ci/linux/commit/66fae2c1becdcb71c95f2c6a6413de4dfe1deb51 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Joe-Perches/drm-amd-display-Simplify-hdcp-validate_bksv/20210712-034708 git checkout 66fae2c1becdcb71c95f2c6a6413de4dfe1deb51 # save the attached .config to linux build tree make W=1 ARCH=i386 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>, old ones prefixed by <<): >> ERROR: modpost: "__popcountdi2" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] >> undefined! --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH 0/4] vkms: Switch to shadow-buffered plane state
On 07/06, Thomas Zimmermann wrote: > Hi > > Am 05.07.21 um 23:29 schrieb Melissa Wen: > > On 07/05, Daniel Vetter wrote: > > > On Mon, Jul 05, 2021 at 12:05:28PM +0200, Thomas Zimmermann wrote: > > > > Hi > > > > > > > > Am 05.07.21 um 11:27 schrieb Daniel Vetter: > > > > > On Mon, Jul 05, 2021 at 09:46:29AM +0200, Thomas Zimmermann wrote: > > > > > > Vkms copies each plane's framebuffer into the output buffer; > > > > > > essentially > > > > > > using a shadow buffer. DRM provides struct drm_shadow_plane_state, > > > > > > which > > > > > > handles the details of mapping/unmapping shadow buffers into memory > > > > > > for > > > > > > active planes. > > > > > > > > > > > > Convert vkms to the helpers. Makes vkms use shared code and gives > > > > > > more > > > > > > test exposure to shadow-plane helpers. > > > > > > > > > > > > Thomas Zimmermann (4): > > > > > > drm/gem: Export implementation of shadow-plane helpers > > > > > > drm/vkms: Inherit plane state from struct drm_shadow_plane_state > > > > > > drm/vkms: Let shadow-plane helpers prepare the plane's FB > > > > > > drm/vkms: Use dma-buf mapping from shadow-plane state for > > > > > > composing > > > > > > > > > > So I think right now this fits, but I think it'll mismit going > > > > > forward: We > > > > > don't really have a shadow-plane that we then toss to the hw, it's a > > > > > shadow-crtc-area. Right now there's no difference, because we don't > > > > > support positioning/scaling the primary plane. But that's all kinda > > > > > stuff > > > > > that's on the table. > > > > > > > > > > But conceptually at least the compositioning buffer should bet part > > > > > of the > > > > > crtc, not of the primary plane. > > > > > > > > > > So not sure what to do, but also coffee hasn't kicked in yet, so > > > > > maybe I'm > > > > > just confused. > > > > > > > > I'm not sure if I understand your concern. Can you elaborate? The > > > > compositing output buffer is not affected by this patchset. Only the > > > > input > > > > frambuffers of the planes. Those are shadow buffers. AFAICT the composer > > > > code memcpy's the primary plane and then blends the other planes on top. > > > > Supporting transformation of the primary plane doesn't really change > > > > much > > > > wrt to the vmaping of input fbs. > > > > > > Yeah that's the current implementation, because that's easier. But > > > fundamentally we don't need a copy of the input shadow plane, we need a > > > scratch area that's sized for the crtc. > > > > Maybe I'm missing something, but I am not sure the relevance for vkms to > > switch to shadow-buffered plane. (?) > > It replaces the vkms code with shared code. Nothing else. For the shared > shadow-buffer code it means more testing. If vkms ever supports color > formats that use multiple planes, the new code will be ready. Hi, lgtm. For debug history, can you please include in the description of the third patch (shadow-plane helpers to prepare fb) that vmap failure is no longer displayed in the execution some IGT kms_flip testcases? For the series: Reviewed-by: Melissa Wen Thanks, Melissa > > Best regards > Thomas > > > -- > 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 >
drm/amd/display: Simplify hdcp validate_bksv
commit 06888d571b51 ("drm/amd/display: Avoid HDCP over-read and corruption")
fixed an overread with an invalid buffer length but added an unnecessary
buffer and copy.
Simplify the code by using a single uint64_t and __builtin_popcountll to
count the number of bits set in the original bksv buffer instead of a loop.
This also avoid a possible unaligned access of the temporary bksv.
Signed-off-by: Joe Perches
---
It seems quite odd 20 bits set is a magic number here.
Should it be a specific be/le value instead?
drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c
b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c
index de872e7958b06..78a4c6dd95d99 100644
--- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c
+++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c
@@ -28,17 +28,10 @@
static inline enum mod_hdcp_status validate_bksv(struct mod_hdcp *hdcp)
{
uint64_t n = 0;
- uint8_t count = 0;
- u8 bksv[sizeof(n)] = { };
- memcpy(bksv, hdcp->auth.msg.hdcp1.bksv,
sizeof(hdcp->auth.msg.hdcp1.bksv));
- n = *(uint64_t *)bksv;
+ memcpy(, hdcp->auth.msg.hdcp1.bksv,
sizeof(hdcp->auth.msg.hdcp1.bksv));
- while (n) {
- count++;
- n &= (n - 1);
- }
- return (count == 20) ? MOD_HDCP_STATUS_SUCCESS :
+ return (__builtin_popcountll(n) == 20) ? MOD_HDCP_STATUS_SUCCESS :
MOD_HDCP_STATUS_HDCP1_INVALID_BKSV;
}
Re: [RESEND PATCH v4] drm/panel: simple: add SGD GKTW70SDAD1SD
Hi Oliver.
On Sun, Jul 11, 2021 at 05:49:29PM +0200, Oliver Graute wrote:
> Add support for the Solomon Goldentek Display Model: GKTW70SDAD1SD
> to panel-simple.
>
> The panel spec from Variscite can be found at:
> https://www.variscite.com/wp-content/uploads/2017/12/VLCD-CAP-GLD-RGB.pdf
>
> Signed-off-by: Oliver Graute
> Reviewed-by: Marco Felsch
> Reviewed-by: Fabio Estevam
> ---
>
> v4:
>
> - added the datasheet labels
> - added Reviewed-by
>
> v3:
>
> - added flags
> - added delay
>
> v2:
>
> - changed bpc to 6
> - set max value of pixelclock
> - increased hfront_porch and hback_porch
> - dropped connector-type
connector_type is mandatory. It will cause a warnign if it is missing.
Please re-add.
>
> adding of bus_format = MEDIA_BUS_FMT_RGB666_1X18 results in wrong colors.
> omitting bus_format and using some default is better (Tux Pinguin is colored
> fine)
Likewise bus_format is mandatory. If default is better than
MEDIA_BUS_FMT_RGB666_1X18,
then specify whatever is default.
>
> drivers/gpu/drm/panel/panel-simple.c | 26 ++
> 1 file changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c
> index 2be358f..c63f6a8 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -3336,6 +3336,36 @@ static const struct panel_desc satoz_sat050at40h12r2 =
> {
> .connector_type = DRM_MODE_CONNECTOR_LVDS,
> };
>
> +static const struct display_timing sgd_gktw70sdad1sd_timing = {
> + .pixelclock = {3000, 3000, 4000},
> + .hactive = { 800, 800, 800},
> + .hfront_porch = {40, 40, 40},
> + .hback_porch = {40, 40, 40},
> + .hsync_len = {48, 48, 48},
> + .vactive = {480, 480, 480},
> + .vfront_porch = {13, 13, 13},
> + .vback_porch = {29, 29, 29},
> + .vsync_len = {3, 3, 3},
> + .flags = DISPLAY_FLAGS_HSYNC_LOW | DISPLAY_FLAGS_VSYNC_LOW |
> + DISPLAY_FLAGS_DE_HIGH | DISPLAY_FLAGS_PIXDATA_NEGEDGE,
> +};
> +
> +static const struct panel_desc sgd_gktw70sdad1sd = {
> + .timings = _gktw70sdad1sd_timing,
> + .num_timings = 1,
> + .bpc = 6,
> + .size = {
> + .width = 153,
> + .height = 86,
> + },
> + .delay = {
> + .prepare = 20 + 20 + 10 + 10, /* T0 + T2 + T3 + T4 */
> + .enable = 50, /* T5 */
> + .disable = 50, /* T5 */
> + .unprepare = 10 + 10 + 20 + 20, /* T4 + T3 + T2 + T0 */
> + },
> +};
> +
> static const struct drm_display_mode sharp_ld_d5116z01b_mode = {
> .clock = 168480,
> .hdisplay = 1920,
> @@ -4222,6 +4252,9 @@ static const struct of_device_id platform_of_match[] = {
> .compatible = "satoz,sat050at40h12r2",
> .data = _sat050at40h12r2,
> }, {
> + .compatible = "sgd,gktw70sdad1sd",
compatible needs to be documented - please add to the appropiate .yaml
file, or add a new bindings file.
Sorry for the push back, but if we do not get this fixed now we will
have to revisit it later.
Sam
Re: [PATCH] drm/amd/display: Fix identical code for different branches
On Sun, 2021-07-11 at 19:24 +0200, Len Baker wrote:
> The branches of the "if" statement are the same. So remove the
> unnecessary if and goto statements.
>
> Addresses-Coverity-ID: 1456916 ("Identical code for different branches")
> Fixes: 4c283fdac08ab ("drm/amd/display: Add HDCP module")
> Signed-off-by: Len Baker
I'm not a big fan of this type of change.
It's currently the same style used for six tests in this function
and changing this last one would just make it harder to see the
code blocks as consistent.
I doubt any reasonable compiler would produce different objects.
> diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c
> b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c
[]
> @@ -305,10 +305,8 @@ static enum mod_hdcp_status wait_for_ready(struct
> mod_hdcp *hdcp,
> hdcp, "bcaps_read"))
> goto out;
> }
> - if (!mod_hdcp_execute_and_set(check_ksv_ready,
> - >ready_check, ,
> - hdcp, "ready_check"))
> - goto out;
> + mod_hdcp_execute_and_set(check_ksv_ready, >ready_check, ,
> + hdcp, "ready_check");
> out:
> return status;
> }
> --
> 2.25.1
>
[PATCH] drm/amd/display: Fix identical code for different branches
The branches of the "if" statement are the same. So remove the
unnecessary if and goto statements.
Addresses-Coverity-ID: 1456916 ("Identical code for different branches")
Fixes: 4c283fdac08ab ("drm/amd/display: Add HDCP module")
Signed-off-by: Len Baker
---
drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c
b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c
index de872e7958b0..d0c565567102 100644
--- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c
+++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c
@@ -305,10 +305,8 @@ static enum mod_hdcp_status wait_for_ready(struct mod_hdcp
*hdcp,
hdcp, "bcaps_read"))
goto out;
}
- if (!mod_hdcp_execute_and_set(check_ksv_ready,
- >ready_check, ,
- hdcp, "ready_check"))
- goto out;
+ mod_hdcp_execute_and_set(check_ksv_ready, >ready_check, ,
+hdcp, "ready_check");
out:
return status;
}
--
2.25.1
Re: [RFC PATCH v2 1/4] drm_print.h: rewrap __DRM_DEFINE_DBG_RATELIMITED macro
On Sat, 2021-07-10 at 23:49 -0600, Jim Cromie wrote:
> whitespace only, to diff-minimize a later commit.
> no functional changes
[]
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
[]
> @@ -524,19 +524,24 @@ void __drm_err(const char *format, ...);
> #define DRM_DEBUG_DP(fmt, ...)
> \
> __drm_dbg(DRM_UT_DP, fmt, ## __VA_ARGS__)
>
>
> -#define __DRM_DEFINE_DBG_RATELIMITED(category, drm, fmt, ...)
> \
> -({
> \
> - static DEFINE_RATELIMIT_STATE(rs_, DEFAULT_RATELIMIT_INTERVAL,
> DEFAULT_RATELIMIT_BURST);\
> - const struct drm_device *drm_ = (drm);
> \
> -
> \
> - if (drm_debug_enabled(DRM_UT_ ## category) && __ratelimit(_))
> \
> - drm_dev_printk(drm_ ? drm_->dev : NULL, KERN_DEBUG, fmt, ##
> __VA_ARGS__); \
> +#define __DRM_DEFINE_DBG_RATELIMITED(category, drm, fmt, ...)
> \
> +({ \
> + static DEFINE_RATELIMIT_STATE(rs_, \
> + DEFAULT_RATELIMIT_INTERVAL, \
> + DEFAULT_RATELIMIT_BURST); \
> + const struct drm_device *drm_ = (drm); \
> + \
> + if (drm_debug_enabled(DRM_UT_ ## category) \
> + && __ratelimit(_)) \
Though I don't really see the need for the change, the typical style
has the logical continuation at the end of the test.
if (drm_debug_enabled(DRM_UT_ ## category) && \
__ratelimit(_)) \
[RESEND PATCH v4] drm/panel: simple: add SGD GKTW70SDAD1SD
Add support for the Solomon Goldentek Display Model: GKTW70SDAD1SD
to panel-simple.
The panel spec from Variscite can be found at:
https://www.variscite.com/wp-content/uploads/2017/12/VLCD-CAP-GLD-RGB.pdf
Signed-off-by: Oliver Graute
Reviewed-by: Marco Felsch
Reviewed-by: Fabio Estevam
---
v4:
- added the datasheet labels
- added Reviewed-by
v3:
- added flags
- added delay
v2:
- changed bpc to 6
- set max value of pixelclock
- increased hfront_porch and hback_porch
- dropped connector-type
adding of bus_format = MEDIA_BUS_FMT_RGB666_1X18 results in wrong colors.
omitting bus_format and using some default is better (Tux Pinguin is colored
fine)
drivers/gpu/drm/panel/panel-simple.c | 26 ++
1 file changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index 2be358f..c63f6a8 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -3336,6 +3336,36 @@ static const struct panel_desc satoz_sat050at40h12r2 = {
.connector_type = DRM_MODE_CONNECTOR_LVDS,
};
+static const struct display_timing sgd_gktw70sdad1sd_timing = {
+ .pixelclock = {3000, 3000, 4000},
+ .hactive = { 800, 800, 800},
+ .hfront_porch = {40, 40, 40},
+ .hback_porch = {40, 40, 40},
+ .hsync_len = {48, 48, 48},
+ .vactive = {480, 480, 480},
+ .vfront_porch = {13, 13, 13},
+ .vback_porch = {29, 29, 29},
+ .vsync_len = {3, 3, 3},
+ .flags = DISPLAY_FLAGS_HSYNC_LOW | DISPLAY_FLAGS_VSYNC_LOW |
+ DISPLAY_FLAGS_DE_HIGH | DISPLAY_FLAGS_PIXDATA_NEGEDGE,
+};
+
+static const struct panel_desc sgd_gktw70sdad1sd = {
+ .timings = _gktw70sdad1sd_timing,
+ .num_timings = 1,
+ .bpc = 6,
+ .size = {
+ .width = 153,
+ .height = 86,
+ },
+ .delay = {
+ .prepare = 20 + 20 + 10 + 10, /* T0 + T2 + T3 + T4 */
+ .enable = 50, /* T5 */
+ .disable = 50, /* T5 */
+ .unprepare = 10 + 10 + 20 + 20, /* T4 + T3 + T2 + T0 */
+ },
+};
+
static const struct drm_display_mode sharp_ld_d5116z01b_mode = {
.clock = 168480,
.hdisplay = 1920,
@@ -4222,6 +4252,9 @@ static const struct of_device_id platform_of_match[] = {
.compatible = "satoz,sat050at40h12r2",
.data = _sat050at40h12r2,
}, {
+ .compatible = "sgd,gktw70sdad1sd",
+ .data = _gktw70sdad1sd,
+ }, {
.compatible = "sharp,ld-d5116z01b",
.data = _ld_d5116z01b,
}, {
--
2.7.4
[PATCH v5 2/2] habanalabs: add support for dma-buf exporter
From: Tomer Tayar
Implement the calls to the dma-buf kernel api to create a dma-buf
object backed by FD.
We block the option to mmap the DMA-BUF object because we don't support
DIRECT_IO and implicit P2P. We only implement support for explicit P2P
through importing the FD of the DMA-BUF.
In the export phase, we provide to the DMA-BUF object an array of pages
that represent the device's memory area. During the map callback,
we convert the array of pages into an SGT. We split/merge the pages
according to the dma max segment size of the importer.
To get the DMA address of the PCI bar, we use the dma_map_resources()
kernel API, because our device memory is not backed by page struct
and this API doesn't need page struct to map the physical address to
a DMA address.
We set the orig_nents member of the SGT to be 0, to indicate to other
drivers that we don't support CPU mappings.
Note that in Habanalabs's ASICs, the device memory is pinned and
immutable. Therefore, there is no need for dynamic mappings and pinning
callbacks.
Also note that in GAUDI we don't have an MMU towards the device memory
and the user works on physical addresses. Therefore, the user doesn't
pass through the kernel driver to allocate memory there. As a result,
only for GAUDI we receive from the user a device memory physical address
(instead of a handle) and a size.
We check the p2p distance using pci_p2pdma_distance_many() and refusing
to map dmabuf in case the distance doesn't allow p2p.
Signed-off-by: Tomer Tayar
Reviewed-by: Oded Gabbay
Reviewed-by: Gal Pressman
Signed-off-by: Oded Gabbay
---
Changes in v5:
- Use dma_get_max_seg_size() to get the max segment size of the importer.
Use that value when we convert the pages array into an SGT, to make
sure each SG is not larger than that size.
- Set orig_nents to be 0 after initializing the SGT. This will hopefully
tell other drivers to avoid using CPU mapping with this SGT.
- Remove the allocation of pages array in case we receive an address
from the user (GAUDI use-case). This was redundant.
drivers/misc/habanalabs/Kconfig | 1 +
drivers/misc/habanalabs/common/habanalabs.h | 22 +
drivers/misc/habanalabs/common/memory.c | 522 +++-
drivers/misc/habanalabs/gaudi/gaudi.c | 1 +
drivers/misc/habanalabs/goya/goya.c | 1 +
5 files changed, 543 insertions(+), 4 deletions(-)
diff --git a/drivers/misc/habanalabs/Kconfig b/drivers/misc/habanalabs/Kconfig
index 293d79811372..c82d2e7b2035 100644
--- a/drivers/misc/habanalabs/Kconfig
+++ b/drivers/misc/habanalabs/Kconfig
@@ -8,6 +8,7 @@ config HABANA_AI
depends on PCI && HAS_IOMEM
select GENERIC_ALLOCATOR
select HWMON
+ select DMA_SHARED_BUFFER
help
Enables PCIe card driver for Habana's AI Processors (AIP) that are
designed to accelerate Deep Learning inference and training workloads.
diff --git a/drivers/misc/habanalabs/common/habanalabs.h
b/drivers/misc/habanalabs/common/habanalabs.h
index 9782bb50931a..4969495f4788 100644
--- a/drivers/misc/habanalabs/common/habanalabs.h
+++ b/drivers/misc/habanalabs/common/habanalabs.h
@@ -26,6 +26,7 @@
#include
#include
#include
+#include
#define HL_NAME"habanalabs"
@@ -1329,6 +1330,23 @@ struct hl_pending_cb {
u32 hw_queue_id;
};
+/**
+ * struct hl_dmabuf_wrapper - a dma-buf wrapper object.
+ * @dmabuf: pointer to dma-buf object.
+ * @ctx: pointer to the dma-buf owner's context.
+ * @phys_pg_pack: pointer to physical page pack if the dma-buf was exported for
+ *memory allocation handle.
+ * @device_address: physical address of the device's memory. Relevant only
+ * if phys_pg_pack is NULL (dma-buf was exported from
address).
+ * The total size can be taken from the dmabuf object.
+ */
+struct hl_dmabuf_wrapper {
+ struct dma_buf *dmabuf;
+ struct hl_ctx *ctx;
+ struct hl_vm_phys_pg_pack *phys_pg_pack;
+ uint64_tdevice_address;
+};
+
/**
* struct hl_ctx - user/kernel context.
* @mem_hash: holds mapping from virtual address to virtual memory area
@@ -1637,6 +1655,7 @@ struct hl_vm_hw_block_list_node {
* @npages: num physical pages in the pack.
* @total_size: total size of all the pages in this list.
* @mapping_cnt: number of shared mappings.
+ * @exporting_cnt: number of dma-buf exporting.
* @asid: the context related to this list.
* @page_size: size of each page in the pack.
* @flags: HL_MEM_* flags related to this list.
@@ -1651,6 +1670,7 @@ struct hl_vm_phys_pg_pack {
u64 npages;
u64 total_size;
atomic_tmapping_cnt;
+ u32 exporting_cnt;
u32 asid;
u32 page_size;
u32
[PATCH v5 1/2] habanalabs: define uAPI to export FD for DMA-BUF
User process might want to share the device memory with another
driver/device, and to allow it to access it over PCIe (P2P).
To enable this, we utilize the dma-buf mechanism and add a dma-buf
exporter support, so the other driver can import the device memory and
access it.
The device memory is allocated using our existing allocation uAPI,
where the user will get a handle that represents the allocation.
The user will then need to call the new
uAPI (HL_MEM_OP_EXPORT_DMABUF_FD) and give the handle as a parameter.
The driver will return a FD that represents the DMA-BUF object that
was created to match that allocation.
Signed-off-by: Oded Gabbay
Reviewed-by: Tomer Tayar
---
include/uapi/misc/habanalabs.h | 28 +++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/include/uapi/misc/habanalabs.h b/include/uapi/misc/habanalabs.h
index 18765eb75b65..c5cbd60696d7 100644
--- a/include/uapi/misc/habanalabs.h
+++ b/include/uapi/misc/habanalabs.h
@@ -808,6 +808,10 @@ union hl_wait_cs_args {
#define HL_MEM_OP_UNMAP3
/* Opcode to map a hw block */
#define HL_MEM_OP_MAP_BLOCK4
+/* Opcode to create DMA-BUF object for an existing device memory allocation
+ * and to export an FD of that DMA-BUF back to the caller
+ */
+#define HL_MEM_OP_EXPORT_DMABUF_FD 5
/* Memory flags */
#define HL_MEM_CONTIGUOUS 0x1
@@ -879,11 +883,26 @@ struct hl_mem_in {
/* Virtual address returned from HL_MEM_OP_MAP */
__u64 device_virt_addr;
} unmap;
+
+ /* HL_MEM_OP_EXPORT_DMABUF_FD */
+ struct {
+ /* Handle returned from HL_MEM_OP_ALLOC. In Gaudi,
+* where we don't have MMU for the device memory, the
+* driver expects a physical address (instead of
+* a handle) in the device memory space.
+*/
+ __u64 handle;
+ /* Size of memory allocation. Relevant only for GAUDI */
+ __u64 mem_size;
+ } export_dmabuf_fd;
};
/* HL_MEM_OP_* */
__u32 op;
- /* HL_MEM_* flags */
+ /* HL_MEM_* flags.
+* For the HL_MEM_OP_EXPORT_DMABUF_FD opcode, this field holds the
+* DMA-BUF file/FD flags.
+*/
__u32 flags;
/* Context ID - Currently not in use */
__u32 ctx_id;
@@ -920,6 +939,13 @@ struct hl_mem_out {
__u32 pad;
};
+
+ /* Returned in HL_MEM_OP_EXPORT_DMABUF_FD. Represents the
+* DMA-BUF object that was created to describe a memory
+* allocation on the device's memory space. The FD should be
+* passed to the importer driver
+*/
+ __u64 fd;
};
};
--
2.25.1
[PATCH v5 0/2] Add p2p via dmabuf to habanalabs
Hi, This is v5 of this patch-set following again a long email thread. It contains fixes to the implementation according to the review that Jason did on v4. Jason, I appreciate your feedback. If you can take another look to see I didn't miss anything that would be great. The details of the fixes are in the changelog in the commit message of the second patch. There was one issue with your proposal to set the orig_nents to 0. I did that, but I also had to restore it to nents before calling sg_free_table because that function uses orig_nents to iterate. Thanks, Oded Oded Gabbay (1): habanalabs: define uAPI to export FD for DMA-BUF Tomer Tayar (1): habanalabs: add support for dma-buf exporter drivers/misc/habanalabs/Kconfig | 1 + drivers/misc/habanalabs/common/habanalabs.h | 22 + drivers/misc/habanalabs/common/memory.c | 522 +++- drivers/misc/habanalabs/gaudi/gaudi.c | 1 + drivers/misc/habanalabs/goya/goya.c | 1 + include/uapi/misc/habanalabs.h | 28 +- 6 files changed, 570 insertions(+), 5 deletions(-) -- 2.25.1
[PATCH] drm/amd/display: Fix identical code for different branches
The ternary expression:
vrr->state == VRR_STATE_ACTIVE_VARIABLE ? max_refresh : max_refresh;
has identical then and else expressions. So, simplify the code.
Addresses-Coverity-ID: 1471122 ("Identical code for different branches")
Fixes: 9bc4162665827 ("drm/amd/display: Implement VSIF V3 extended refresh rate
feature")
Signed-off-by: Len Baker
---
drivers/gpu/drm/amd/display/modules/freesync/freesync.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
index 3f4f44b44e6a..54374c7d309b 100644
--- a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
+++ b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
@@ -613,9 +613,8 @@ static void build_vrr_infopacket_data_v3(const struct
mod_vrr_params *vrr,
(vrr->state == VRR_STATE_INACTIVE) ? min_refresh :
max_refresh; // Non-fs case, program nominal range
- max_programmed = (vrr->state == VRR_STATE_ACTIVE_FIXED) ? fixed_refresh
:
- (vrr->state == VRR_STATE_ACTIVE_VARIABLE) ? max_refresh
:
- max_refresh;// Non-fs case, program nominal range
+ max_programmed = (vrr->state == VRR_STATE_ACTIVE_FIXED) ?
+ fixed_refresh : max_refresh;
/* PB7 = FreeSync Minimum refresh rate (Hz) */
infopacket->sb[7] = min_programmed & 0xFF;
--
2.25.1
Re: [PATCH 1/1] drm: bridge: Mark deprecated operations in drm_bridge_funcs
On Sun, Jul 11, 2021 at 11:16:44AM +0200, Sam Ravnborg wrote: > drm_bridge_funcs includes several duplicated operations as atomic > variants has been added over time. > New bridge drivers shall use the atomic variants - mark the deprecated > operations to try to avoid usage in new bridge drivers. > > Signed-off-by: Sam Ravnborg > Cc: Laurent Pinchart > Cc: Andrzej Hajda > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Thomas Zimmermann > Cc: David Airlie > Cc: Daniel Vetter Ignore this patch - I managed to chain the wrong patch to the cover letter. Sam
[PATCH v1 2/2] drm/atmel-hlcdc: Convert to Linux IRQ interfaces
The drm mid-layer for irq's is not relevant for atomic display drivers,
and will be available only for legacy drivers in the future.
Drop usage in the atmel hlcdc driver.
Signed-off-by: Sam Ravnborg
Cc: Sam Ravnborg
Cc: Boris Brezillon
Cc: Nicolas Ferre
Cc: Alexandre Belloni
Cc: Ludovic Desroches
Cc: linux-arm-ker...@lists.infradead.org
---
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 19 ---
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index f67363188cbc..6b3f353e1cc7 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -22,7 +22,6 @@
#include
#include
#include
-#include
#include
#include
@@ -521,9 +520,8 @@ atmel_hlcdc_dc_mode_valid(struct atmel_hlcdc_dc *dc,
return MODE_OK;
}
-static int atmel_hlcdc_dc_irq_postinstall(struct drm_device *dev)
+static int atmel_hlcdc_dc_irq_postinstall(struct atmel_hlcdc_dc *dc)
{
- struct atmel_hlcdc_dc *dc = dev->dev_private;
unsigned int cfg = 0;
int i;
@@ -538,9 +536,8 @@ static int atmel_hlcdc_dc_irq_postinstall(struct drm_device
*dev)
return 0;
}
-static void atmel_hlcdc_dc_irq_uninstall(struct drm_device *dev)
+static void atmel_hlcdc_dc_irq_reset(struct atmel_hlcdc_dc *dc)
{
- struct atmel_hlcdc_dc *dc = dev->dev_private;
unsigned int isr;
regmap_write(dc->hlcdc->regmap, ATMEL_HLCDC_IDR, 0x);
@@ -672,12 +669,14 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
drm_mode_config_reset(dev);
pm_runtime_get_sync(dev->dev);
- ret = drm_irq_install(dev, dc->hlcdc->irq);
+ atmel_hlcdc_dc_irq_reset(dc);
+ ret = request_irq(dc->hlcdc->irq, atmel_hlcdc_dc_irq_handler, 0,
dev->driver->name, dev);
pm_runtime_put_sync(dev->dev);
if (ret < 0) {
dev_err(dev->dev, "failed to install IRQ handler\n");
goto err_periph_clk_disable;
}
+ atmel_hlcdc_dc_irq_postinstall(dc);
platform_set_drvdata(pdev, dev);
@@ -701,7 +700,9 @@ static void atmel_hlcdc_dc_unload(struct drm_device *dev)
drm_mode_config_cleanup(dev);
pm_runtime_get_sync(dev->dev);
- drm_irq_uninstall(dev);
+
+ atmel_hlcdc_dc_irq_reset(dc);
+ free_irq(dc->hlcdc->irq, dev);
pm_runtime_put_sync(dev->dev);
dev->dev_private = NULL;
@@ -714,10 +715,6 @@ DEFINE_DRM_GEM_CMA_FOPS(fops);
static const struct drm_driver atmel_hlcdc_dc_driver = {
.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
- .irq_handler = atmel_hlcdc_dc_irq_handler,
- .irq_preinstall = atmel_hlcdc_dc_irq_uninstall,
- .irq_postinstall = atmel_hlcdc_dc_irq_postinstall,
- .irq_uninstall = atmel_hlcdc_dc_irq_uninstall,
DRM_GEM_CMA_DRIVER_OPS,
.fops = ,
.name = "atmel-hlcdc",
--
2.30.2
[PATCH v1 1/2] drm/atmel-hlcdc: Move interrupt helper funtions
This is in preparation for removal of drm_irq use.
Functions are moved without any other changes.
Signed-off-by: Sam Ravnborg
Cc: Sam Ravnborg
Cc: Boris Brezillon
Cc: Nicolas Ferre
Cc: Alexandre Belloni
Cc: Ludovic Desroches
Cc: linux-arm-ker...@lists.infradead.org
---
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 52 ++--
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index 65af56e47129..f67363188cbc 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -521,6 +521,32 @@ atmel_hlcdc_dc_mode_valid(struct atmel_hlcdc_dc *dc,
return MODE_OK;
}
+static int atmel_hlcdc_dc_irq_postinstall(struct drm_device *dev)
+{
+ struct atmel_hlcdc_dc *dc = dev->dev_private;
+ unsigned int cfg = 0;
+ int i;
+
+ /* Enable interrupts on activated layers */
+ for (i = 0; i < ATMEL_HLCDC_MAX_LAYERS; i++) {
+ if (dc->layers[i])
+ cfg |= ATMEL_HLCDC_LAYER_STATUS(i);
+ }
+
+ regmap_write(dc->hlcdc->regmap, ATMEL_HLCDC_IER, cfg);
+
+ return 0;
+}
+
+static void atmel_hlcdc_dc_irq_uninstall(struct drm_device *dev)
+{
+ struct atmel_hlcdc_dc *dc = dev->dev_private;
+ unsigned int isr;
+
+ regmap_write(dc->hlcdc->regmap, ATMEL_HLCDC_IDR, 0x);
+ regmap_read(dc->hlcdc->regmap, ATMEL_HLCDC_ISR, );
+}
+
static void atmel_hlcdc_layer_irq(struct atmel_hlcdc_layer *layer)
{
if (!layer)
@@ -684,32 +710,6 @@ static void atmel_hlcdc_dc_unload(struct drm_device *dev)
clk_disable_unprepare(dc->hlcdc->periph_clk);
}
-static int atmel_hlcdc_dc_irq_postinstall(struct drm_device *dev)
-{
- struct atmel_hlcdc_dc *dc = dev->dev_private;
- unsigned int cfg = 0;
- int i;
-
- /* Enable interrupts on activated layers */
- for (i = 0; i < ATMEL_HLCDC_MAX_LAYERS; i++) {
- if (dc->layers[i])
- cfg |= ATMEL_HLCDC_LAYER_STATUS(i);
- }
-
- regmap_write(dc->hlcdc->regmap, ATMEL_HLCDC_IER, cfg);
-
- return 0;
-}
-
-static void atmel_hlcdc_dc_irq_uninstall(struct drm_device *dev)
-{
- struct atmel_hlcdc_dc *dc = dev->dev_private;
- unsigned int isr;
-
- regmap_write(dc->hlcdc->regmap, ATMEL_HLCDC_IDR, 0x);
- regmap_read(dc->hlcdc->regmap, ATMEL_HLCDC_ISR, );
-}
-
DEFINE_DRM_GEM_CMA_FOPS(fops);
static const struct drm_driver atmel_hlcdc_dc_driver = {
--
2.30.2
[PATCH v1 0/2] drm/atmel-hlcdc: drop use of drm_irq mid-layer
[Ignore the first mail - wrong patch was attached] Two small patches to drop the use of the drm irq mid-layer. My atmel hlcdc target has stopped working - so unfortunately un-tested :-( I picked up this to continue the work of Thomas Zimmermann. Sam Sam Ravnborg (2): drm/atmel-hlcdc: Move interrupt helper funtions drm/atmel-hlcdc: Convert to Linux IRQ interfaces drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 63 +--- 1 file changed, 30 insertions(+), 33 deletions(-)
[PATCH 1/1] drm: bridge: Mark deprecated operations in drm_bridge_funcs
drm_bridge_funcs includes several duplicated operations as atomic
variants has been added over time.
New bridge drivers shall use the atomic variants - mark the deprecated
operations to try to avoid usage in new bridge drivers.
Signed-off-by: Sam Ravnborg
Cc: Laurent Pinchart
Cc: Andrzej Hajda
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: David Airlie
Cc: Daniel Vetter
---
include/drm/drm_bridge.h | 28 ++--
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 2195daa289d2..6805898d70f5 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -171,6 +171,11 @@ struct drm_bridge_funcs {
* signals) feeding it is still running when this callback is called.
*
* The @disable callback is optional.
+*
+* NOTE:
+*
+* This is deprecated, do not use!
+* New drivers shall use _bridge_funcs.atomic_disable.
*/
void (*disable)(struct drm_bridge *bridge);
@@ -190,6 +195,11 @@ struct drm_bridge_funcs {
* called.
*
* The @post_disable callback is optional.
+*
+* NOTE:
+*
+* This is deprecated, do not use!
+* New drivers shall use _bridge_funcs.atomic_post_disable.
*/
void (*post_disable)(struct drm_bridge *bridge);
@@ -213,11 +223,15 @@ struct drm_bridge_funcs {
* For atomic drivers the adjusted_mode is the mode stored in
* _crtc_state.adjusted_mode.
*
-* NOTE:
-*
* If a need arises to store and access modes adjusted for other
* locations than the connection between the CRTC and the first bridge,
* the DRM framework will have to be extended with DRM bridge states.
+*
+* NOTE:
+*
+* This is deprecated, do not use!
+* New drivers shall set their mode in _bridge_funcs.atomic_enable
+* operation.
*/
void (*mode_set)(struct drm_bridge *bridge,
const struct drm_display_mode *mode,
@@ -239,6 +253,11 @@ struct drm_bridge_funcs {
* there is one) when this callback is called.
*
* The @pre_enable callback is optional.
+*
+* NOTE:
+*
+* This is deprecated, do not use!
+* New drivers shall use _bridge_funcs.atomic_pre_enable.
*/
void (*pre_enable)(struct drm_bridge *bridge);
@@ -259,6 +278,11 @@ struct drm_bridge_funcs {
* chain if there is one.
*
* The @enable callback is optional.
+*
+* NOTE:
+*
+* This is deprecated, do not use!
+* New drivers shall use _bridge_funcs.atomic_enable.
*/
void (*enable)(struct drm_bridge *bridge);
--
2.30.2
[PATCH v1 0/2] drm/atmel-hlcdc: drop use of drm_irq mid-layer
Two small patches to drop the use of the drm irq mid-layer. My atmel hlcdc target has stopped working - so unfortunately un-tested :-( I picked up this to continue the work of Thomas Zimmermann. Sam Sam Ravnborg (2): drm/atmel-hlcdc: Move interrupt helper funtions drm/atmel-hlcdc: Convert to Linux IRQ interfaces drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 63 +--- 1 file changed, 30 insertions(+), 33 deletions(-)
[PATCH] drivers:gpu:drm:selftests: cleanup __FUNCTION__ usage
__FUNCTION__ exists only for backwards compatibility reasons
with old gcc versions. Replace it with __func__.
Signed-off-by: Dwaipayan Ray
---
drivers/gpu/drm/selftests/test-drm_modeset_common.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/selftests/test-drm_modeset_common.h
b/drivers/gpu/drm/selftests/test-drm_modeset_common.h
index cfb51d8da2bc..a4e9d9bacc89 100644
--- a/drivers/gpu/drm/selftests/test-drm_modeset_common.h
+++ b/drivers/gpu/drm/selftests/test-drm_modeset_common.h
@@ -9,7 +9,7 @@
#define FAIL(test, msg, ...) \
do { \
if (test) { \
- pr_err("%s/%u: " msg, __FUNCTION__, __LINE__,
##__VA_ARGS__); \
+ pr_err("%s/%u: " msg, __func__, __LINE__,
##__VA_ARGS__); \
return -EINVAL; \
} \
} while (0)
--
2.28.0
[RFC PATCH v2 0/4] Allow using dyndbg to replace drm_debug_enabled
drm_debug_enabled() is called a lot to do unlikely bit-tests to control debug printing; this is a good job for dynamic-debug, IFF it is built with JUMP_LABEL. Enable the use of dynamic-debug to avoid drm_debug_enabled() overheads, opt in with CONFIG_DRM_USE_DYNAMIC_DEBUG=y. I have this patchset running bare-metal on an i915 laptop & an amdgpu desktop (both as loadable modules). I booted the amdgpu box with: BOOT_IMAGE=(hd2,gpt2)/vmlinuz-5.13.0-dd7-13692-g8def25788f56 \ root=UUID=mumble ro \ rootflags=subvol=root00 rhgb \ dynamic_debug.verbose=3 main.dyndbg=+p \ amdgpu.debug=1 amdgpu.test=1 \ "amdgpu.dyndbg=format ^[ +p" That last line activates ~1700 callsites with a format like '[DML' etc at boot, causing ~76k prdbgs in 409 seconds, before I turned them off with: echo module amdgpu -p > /proc/dynamic_debug/control [root@gandalf jimc]# journalctl -b-0 | grep -P '\[(DML|VBLANK|SURFACE|BIOS|BANDWIDTH)' | wc 68708 578503 5054437 [root@gandalf jimc]# journalctl -b-0 | grep -P '\[(DML|VBLANK|SURFACE|BIOS|BANDWIDTH|\w+)' | wc 76298 661176 6028087 IOW, things appear to hold up under some stress. this is on top of master @ v5.13-13688-gde5540965853 v1 is here: https://lore.kernel.org/lkml/20201204035318.332419-1-jim.cro...@gmail.com/ Jim Cromie (4): drm_print.h: rewrap __DRM_DEFINE_DBG_RATELIMITED macro drm: fixup comment spelling drm: RFC add choice to use dynamic debug in drm-debug i915: map gvt pr_debug categories to bits in parameters/debug_gvt drivers/gpu/drm/Kconfig| 13 drivers/gpu/drm/drm_print.c| 75 +- drivers/gpu/drm/i915/gvt/Makefile | 4 + drivers/gpu/drm/i915/i915_params.c | 76 ++ include/drm/drm_print.h| 119 - 5 files changed, 249 insertions(+), 38 deletions(-) -- 2.31.1
[RFC PATCH v2 2/4] drm: fixup comment spelling
s/prink/printk/ - no functional changes --- include/drm/drm_print.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h index 6419b4e7c5dc..4559583bc88b 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -327,7 +327,7 @@ static inline bool drm_debug_enabled(enum drm_debug_category category) /* * struct device based logging * - * Prefer drm_device based logging over device or prink based logging. + * Prefer drm_device based logging over device or printk based logging. */ __printf(3, 4) -- 2.31.1
[RFC PATCH v2 4/4] i915: map gvt pr_debug categories to bits in parameters/debug_gvt
The gvt component of this driver has ~120 pr_debugs, in 9 "classes".
Following the interface model of drm.debug, add a parameter to map
bits to these classes.
If CONFIG_DRM_USE_DYNAMIC_DEBUG=y (and CONFIG_DYNAMIC_DEBUG_CORE), add
-DDYNAMIC_DEBUG_MODULE into Makefile. TBD: maybe add a separate
CONFIG_I915_USE_DYNAMIC_DEBUG to more fully optionalize this.
In i915_params.c, add callback to map bits to queries.
TBD: the callback code should probably be moved to lib/dynamic_debug,
and given a declarative interface, with implied bit-numbering,
something like:
MOD_PARM_BITMAP_DESC(__gvt_debug,
"gvt: cmd: ", "command processing"
"gvt: core: ", "core help",
"gvt: dpy: ", "display help",
"gvt: el: ", "help",
"gvt: irq: ", "help",
"gvt: mm: ", "help",
"gvt: mmio: ", "help",
"gvt: render: ", "help",
"gvt: sched: " "help");
---
drivers/gpu/drm/i915/gvt/Makefile | 4 ++
drivers/gpu/drm/i915/i915_params.c | 76 ++
2 files changed, 80 insertions(+)
diff --git a/drivers/gpu/drm/i915/gvt/Makefile
b/drivers/gpu/drm/i915/gvt/Makefile
index ea8324abc784..846ba73b8de6 100644
--- a/drivers/gpu/drm/i915/gvt/Makefile
+++ b/drivers/gpu/drm/i915/gvt/Makefile
@@ -7,3 +7,7 @@ GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o
trace_points.o firmware.o \
ccflags-y += -I $(srctree)/$(src) -I
$(srctree)/$(src)/$(GVT_DIR)/
i915-y += $(addprefix $(GVT_DIR)/,
$(GVT_SOURCE))
+
+#ifdef CONFIG_DRM_USE_DYNAMIC_DEBUG
+ccflags-y += -DDYNAMIC_DEBUG_MODULE
+#endif
diff --git a/drivers/gpu/drm/i915/i915_params.c
b/drivers/gpu/drm/i915/i915_params.c
index e07f4cfea63a..e0d13aff5274 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -265,3 +265,79 @@ void i915_params_free(struct i915_params *params)
I915_PARAMS_FOR_EACH(FREE);
#undef FREE
}
+
+/* POC for callback -> dynamic_debug_exec_queries */
+unsigned long __gvt_debug;
+EXPORT_SYMBOL(__gvt_debug);
+
+static char *format_prefix_classes[] = {
+ "gvt: cmd: ",
+ "gvt: core: ",
+ "gvt: dpy: ",
+ "gvt: el: ",
+ "gvt: irq: ",
+ "gvt: mm: ",
+ "gvt: mmio: ",
+ "gvt: render: ",
+ "gvt: sched: "
+};
+#define NUM_CLASSESARRAY_SIZE(format_prefix_classes)
+#define OUR_QUERY_SIZE 128 /* we need about 20 */
+
+#include
+
+static int param_set_dyndbg(const char *instr, const struct kernel_param *kp)
+{
+ unsigned int val;
+ unsigned long changes, result;
+ int rc, chgct = 0, totct = 0, bitpos;
+ char query[OUR_QUERY_SIZE];
+
+ rc = kstrtouint(instr, 0, );
+ if (rc) {
+ pr_err("set_dyndbg: failed\n");
+ return -EINVAL;
+ }
+ result = val;
+ pr_info("set_dyndbg: result:0x%lx from %s\n", result, instr);
+
+ changes = result ^ __gvt_debug;
+
+ for_each_set_bit(bitpos, , NUM_CLASSES) {
+
+ sprintf(query, "format '^%s' %cp",
format_prefix_classes[bitpos],
+ test_bit(bitpos, ) ? '+' : '-');
+
+ chgct = dynamic_debug_exec_queries(query, "i915");
+
+ pr_info("%d changes on: %s\n", chgct, query);
+ totct += chgct;
+ }
+ pr_info("total changes: %d\n", totct);
+ __gvt_debug = result;
+ return 0;
+}
+static int param_get_dyndbg(char *buffer, const struct kernel_param *kp)
+{
+ return scnprintf(buffer, PAGE_SIZE, "%u\n",
+*((unsigned int *)kp->arg));
+}
+static const struct kernel_param_ops param_ops_dyndbg = {
+ .set = param_set_dyndbg,
+ .get = param_get_dyndbg,
+};
+
+#define info_ln(hexi, prefix) "\n\t0x" __stringify(hexi) "\t" prefix
+
+MODULE_PARM_DESC(debug_gvt, " gvt debug categories:"
+info_ln(1, "gvt: cmd:")
+info_ln(2, "gvt: core:")
+info_ln(4, "gvt: dpy:")
+info_ln(8, "gvt: el:")
+info_ln(10, "gvt: irq:")
+info_ln(20, "gvt: mm:")
+info_ln(40, "gvt: mmio:")
+info_ln(80, "gvt: render:")
+info_ln(100, "gvt: sched:"));
+
+module_param_cb(debug_gvt, _ops_dyndbg, &__gvt_debug, 0644);
--
2.31.1
[RFC PATCH v2 3/4] drm: RFC add choice to use dynamic debug in drm-debug
drm's debug system uses distinct categories of debug messages, encoded
in an enum (DRM_UT_), which are mapped to bits in drm.debug.
drm_debug_enabled() does a lot of unlikely bit-mask checks on
drm.debug; we can use dynamic debug instead, and get all that
static_key/jump_label goodness.
Dynamic debug has no concept of category, but we can map the DRM_UT_*
to a set of distinct prefixes; "drm:core:", "drm:kms:" etc, and
prepend them to the given formats. Then we can use
`echo module drm format ^drm:core: > control`
to select the whole category with one query. This new prefix changes
pr_debug's output, so is user visible, but it seems unlikely to cause
trouble for log watchers; they're not relying on the absence of class
prefix strings.
This conversion yields ~2100 new callsites on my i7 laptop:
dyndbg: 195 debug prints in module drm_kms_helper
dyndbg: 298 debug prints in module drm
dyndbg: 1630 debug prints in module i915
CONFIG_DRM_USE_DYNAMIC_DEBUG enables this, and is available if
CONFIG_DYNAMIC_DEBUG or CONFIG_DYNAMIC_DEBUG_CORE is chosen, and if
CONFIG_JUMP_LABEL is enabled; this because its required to get the
promised optimizations.
The indirection/switchover is layered into the macro scheme:
0.A new callback on drm.debug which calls dynamic_debug_exec_queries
to map those bits to specific query/commands
dynamic_debug_exec_queries("format ^drm:kms: +p", "drm*");
here for POC, this should be in dynamic_debug.c
with a MODULE_PARAM_DEBUG_BITMAP(__drm_debug, { "prefix-1", "desc-1" }+)
1.A "converted" or "classy" DRM_UT_* map
based on: DRM_UT_* ( symbol => bit-mask )
named it: cDRM_UT_* ( symbol => format-class-prefix-string )
So cDRM_UT_* is either:
legacy: cDRM_UT_* <-- DRM_UT_* ( !CONFIG_DRM_USE_DYNAMIC_DEBUG )
enabled:
#define cDRM_UT_KMS"drm:kms: "
#define cDRM_UT_PRIME "drm:prime: "
#define cDRM_UT_ATOMIC "drm:atomic: "
DRM_UT_* are unchanged, since theyre used in drm_debug_enabled() and
elsewhere.
2.drm_dev_dbg & drm_debug are renamed (prefixed with '_')
old names are now macros, calling either:
legacy: -> to renamed fn
enabled: -> dev_dbg & pr_debug, with cDRM-prefix prepended format.
3.names in (2) are called from DRM_DEBUG_ and
drm_dbg_. all these macros get "converted" to use
cDRM_UT_*, to get right token type.
4.simplification of __DRM_DEFINE_DBG_RATELIMITED macro
pass both DRM_UT & cDRM_UT, for drm_debug_enabled & drm_dev_dbg
remove DRM_UT_ ## KMS
Also reuse gist of:
commit 7911902129a8 ("drm/print: Handle potentially NULL drm_devices in
drm_dbg_*)
---
drivers/gpu/drm/Kconfig | 13 +
drivers/gpu/drm/drm_print.c | 75 --
include/drm/drm_print.h | 104 ++--
3 files changed, 159 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 7ff89690a976..e4524ccba040 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -57,6 +57,19 @@ config DRM_DEBUG_MM
If in doubt, say "N".
+config DRM_USE_DYNAMIC_DEBUG
+ bool "use dynamic debug to implement drm.debug"
+ default n
+ depends on DRM
+ depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE
+ depends on JUMP_LABEL
+ help
+ The drm debug category facility does a lot of unlikely bit-field
+ tests at runtime; while cheap individually, the cost accumulates.
+ This option uses dynamic debug facility (if configured and
+ using jump_label) to avoid those runtime checks, patching
+ the kernel when those debugs are desired.
+
config DRM_DEBUG_SELFTEST
tristate "kselftests for DRM"
depends on DRM
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 111b932cf2a9..e2acdfc7088b 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -52,8 +52,75 @@ MODULE_PARM_DESC(debug, "Enable debug output, where each bit
enables a debug cat
"\t\tBit 5 (0x20) will enable VBL messages (vblank code)\n"
"\t\tBit 7 (0x80) will enable LEASE messages (leasing code)\n"
"\t\tBit 8 (0x100) will enable DP messages (displayport code)");
+
+#ifndef CONFIG_DRM_USE_DYNAMIC_DEBUG
module_param_named(debug, __drm_debug, int, 0600);
+#else
+static char *format_class_prefixes[] = {
+ cDRM_UT_CORE,
+ cDRM_UT_DRIVER,
+ cDRM_UT_KMS,
+ cDRM_UT_PRIME,
+ cDRM_UT_ATOMIC,
+ cDRM_UT_VBL,
+ cDRM_UT_STATE,
+ cDRM_UT_LEASE,
+ cDRM_UT_DP,
+ cDRM_UT_DRMRES
+};
+
+#define OUR_QUERY_SIZE 64 /* > strlen "format '^%s' %cp" + longest prefix */
+
+static int param_set_dyndbg(const char *instr, const struct kernel_param *kp)
+{
+ unsigned int val;
+ unsigned long changes, result;
+ int rc, chgct = 0, totct = 0, bitpos;
+ char query[OUR_QUERY_SIZE];
+
+ rc = kstrtouint(instr, 0, );
+ if (rc) {
+ pr_err("%s: failed\n", __func__);
+
[RFC PATCH v2 1/4] drm_print.h: rewrap __DRM_DEFINE_DBG_RATELIMITED macro
whitespace only, to diff-minimize a later commit.
no functional changes
---
include/drm/drm_print.h | 21 +
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 9b66be54dd16..6419b4e7c5dc 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -524,19 +524,24 @@ void __drm_err(const char *format, ...);
#define DRM_DEBUG_DP(fmt, ...) \
__drm_dbg(DRM_UT_DP, fmt, ## __VA_ARGS__)
-#define __DRM_DEFINE_DBG_RATELIMITED(category, drm, fmt, ...)
\
-({
\
- static DEFINE_RATELIMIT_STATE(rs_, DEFAULT_RATELIMIT_INTERVAL,
DEFAULT_RATELIMIT_BURST);\
- const struct drm_device *drm_ = (drm);
\
-
\
- if (drm_debug_enabled(DRM_UT_ ## category) && __ratelimit(_))
\
- drm_dev_printk(drm_ ? drm_->dev : NULL, KERN_DEBUG, fmt, ##
__VA_ARGS__); \
+#define __DRM_DEFINE_DBG_RATELIMITED(category, drm, fmt, ...) \
+({ \
+ static DEFINE_RATELIMIT_STATE(rs_, \
+ DEFAULT_RATELIMIT_INTERVAL, \
+ DEFAULT_RATELIMIT_BURST); \
+ const struct drm_device *drm_ = (drm); \
+ \
+ if (drm_debug_enabled(DRM_UT_ ## category) \
+ && __ratelimit(_)) \
+ drm_dev_printk(drm_ ? drm_->dev : NULL, \
+ KERN_DEBUG, fmt, ## __VA_ARGS__);\
})
#define drm_dbg_kms_ratelimited(drm, fmt, ...) \
__DRM_DEFINE_DBG_RATELIMITED(KMS, drm, fmt, ## __VA_ARGS__)
-#define DRM_DEBUG_KMS_RATELIMITED(fmt, ...) drm_dbg_kms_ratelimited(NULL, fmt,
## __VA_ARGS__)
+#define DRM_DEBUG_KMS_RATELIMITED(fmt, ...) \
+ drm_dbg_kms_ratelimited(NULL, fmt, ## __VA_ARGS__)
/*
* struct drm_device based WARNs
--
2.31.1
[PATCHi v3] drm/mediatek: adjust rdma fifo threshold calculate formula
Change since v2: - add more commit message Yongqiang Niu (1): drm/mediatek: adjust rdma fifo threshold calculate formula drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) -- 1.8.1.1.dirty
[PATCH v3] drm/mediatek: adjust rdma fifo threshold calculate formula
the orginal formula will caused rdma fifo threshold config overflow and no one could come out a solution for all SoC, set threshold to 70% of max fifo size to make sure it will not overflow, and 70% is a empirical vlaue Signed-off-by: Yongqiang Niu --- drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c index f123fc0..f1f6a2e 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c @@ -164,10 +164,10 @@ void mtk_rdma_config(struct device *dev, unsigned int width, /* * Enable FIFO underflow since DSI and DPI can't be blocked. * Keep the FIFO pseudo size reset default of 8 KiB. Set the -* output threshold to 6 microseconds with 7/6 overhead to -* account for blanking, and with a pixel depth of 4 bytes: +* output threshold to 70% of max fifo size to make sure the +* threhold will not overflow */ - threshold = width * height * vrefresh * 4 * 7 / 100; + threshold = rdma_fifo_size * 7 / 10; reg = RDMA_FIFO_UNDERFLOW_EN | RDMA_FIFO_PSEUDO_SIZE(rdma_fifo_size) | RDMA_OUTPUT_VALID_FIFO_THRESHOLD(threshold); -- 1.8.1.1.dirty
