Re: [PATCH V2] treewide: Add missing semicolons to __assign_str uses

2021-06-12 Thread Steven Rostedt
On Sat, 12 Jun 2021 08:42:27 -0700
Joe Perches  wrote:

> The __assign_str macro has an unusual ending semicolon but the vast
> majority of uses of the macro already have semicolon termination.
> 
> $ git grep -P '\b__assign_str\b' | wc -l
> 551
> $ git grep -P '\b__assign_str\b.*;' | wc -l
> 480
> 
> Add semicolons to the __assign_str() uses without semicolon termination
> and all the other uses without semicolon termination via additional defines
> that are equivalent to __assign_str() with the eventual goal of removing
> the semicolon from the __assign_str() macro definition.
> 
> Link: 
> https://lore.kernel.org/lkml/1e068d21106bb6db05b735b4916bb420e6c9842a.ca...@perches.com/

FYI, please send new patches as new threads. Otherwise it is likely to
be missed.

-- Steve


[Bug 213391] AMDGPU retries page fault with some specific processes amdgpu and sometimes followed [gfxhub0] retry page fault until *ERROR* ring gfx timeout, but soft recovered

2021-06-12 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=213391

--- Comment #11 from Lahfa Samy (s...@lahfa.xyz) ---
Hi Dimitris, what is your current kernel version under Fedora, or the output of
this command "uname --kernel-release" in a terminal, I cannot try the patch
given however I haven't run into the issue again, I haven't had the time to put
my RAM under heavy load.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[PATCH V2] treewide: Add missing semicolons to __assign_str uses

2021-06-12 Thread Joe Perches
The __assign_str macro has an unusual ending semicolon but the vast
majority of uses of the macro already have semicolon termination.

$ git grep -P '\b__assign_str\b' | wc -l
551
$ git grep -P '\b__assign_str\b.*;' | wc -l
480

Add semicolons to the __assign_str() uses without semicolon termination
and all the other uses without semicolon termination via additional defines
that are equivalent to __assign_str() with the eventual goal of removing
the semicolon from the __assign_str() macro definition.

Link: 
https://lore.kernel.org/lkml/1e068d21106bb6db05b735b4916bb420e6c9842a.ca...@perches.com/

Signed-off-by: Joe Perches 
---

V2: Remove semicolon addition to #define VIF_ASSIGN as every use of
this macro already has a semicolon termination.

Compiled x84-64 allyesconfig

 drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h  | 14 
 drivers/gpu/drm/lima/lima_trace.h  |  2 +-
 drivers/infiniband/hw/hfi1/trace_misc.h|  4 +--
 drivers/infiniband/hw/hfi1/trace_rc.h  |  4 +--
 drivers/infiniband/hw/hfi1/trace_tid.h |  6 ++--
 drivers/infiniband/hw/hfi1/trace_tx.h  |  8 ++---
 drivers/infiniband/sw/rdmavt/trace_cq.h|  4 +--
 drivers/infiniband/sw/rdmavt/trace_mr.h|  2 +-
 drivers/infiniband/sw/rdmavt/trace_qp.h|  4 +--
 drivers/infiniband/sw/rdmavt/trace_rc.h|  2 +-
 drivers/infiniband/sw/rdmavt/trace_tx.h|  4 +--
 drivers/misc/mei/mei-trace.h   |  6 ++--
 .../net/ethernet/marvell/octeontx2/af/rvu_trace.h  | 12 +++
 drivers/net/fjes/fjes_trace.h  |  4 +--
 drivers/usb/cdns3/cdnsp-trace.h|  2 +-
 fs/nfs/nfs4trace.h |  6 ++--
 fs/nfs/nfstrace.h  |  4 +--
 include/trace/events/btrfs.h   |  2 +-
 include/trace/events/dma_fence.h   |  4 +--
 include/trace/events/rpcgss.h  |  4 +--
 include/trace/events/sunrpc.h  | 40 +++---
 21 files changed, 69 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
index 0527772fe1b80..d855cb53c7e09 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
@@ -176,10 +176,10 @@ TRACE_EVENT(amdgpu_cs_ioctl,
 
TP_fast_assign(
   __entry->sched_job_id = job->base.id;
-  __assign_str(timeline, 
AMDGPU_JOB_GET_TIMELINE_NAME(job))
+  __assign_str(timeline, 
AMDGPU_JOB_GET_TIMELINE_NAME(job));
   __entry->context = 
job->base.s_fence->finished.context;
   __entry->seqno = job->base.s_fence->finished.seqno;
-  __assign_str(ring, 
to_amdgpu_ring(job->base.sched)->name)
+  __assign_str(ring, 
to_amdgpu_ring(job->base.sched)->name);
   __entry->num_ibs = job->num_ibs;
   ),
TP_printk("sched_job=%llu, timeline=%s, context=%u, seqno=%u, 
ring_name=%s, num_ibs=%u",
@@ -201,10 +201,10 @@ TRACE_EVENT(amdgpu_sched_run_job,
 
TP_fast_assign(
   __entry->sched_job_id = job->base.id;
-  __assign_str(timeline, 
AMDGPU_JOB_GET_TIMELINE_NAME(job))
+  __assign_str(timeline, 
AMDGPU_JOB_GET_TIMELINE_NAME(job));
   __entry->context = 
job->base.s_fence->finished.context;
   __entry->seqno = job->base.s_fence->finished.seqno;
-  __assign_str(ring, 
to_amdgpu_ring(job->base.sched)->name)
+  __assign_str(ring, 
to_amdgpu_ring(job->base.sched)->name);
   __entry->num_ibs = job->num_ibs;
   ),
TP_printk("sched_job=%llu, timeline=%s, context=%u, seqno=%u, 
ring_name=%s, num_ibs=%u",
@@ -229,7 +229,7 @@ TRACE_EVENT(amdgpu_vm_grab_id,
 
TP_fast_assign(
   __entry->pasid = vm->pasid;
-  __assign_str(ring, ring->name)
+  __assign_str(ring, ring->name);
   __entry->vmid = job->vmid;
   __entry->vm_hub = ring->funcs->vmhub,
   __entry->pd_addr = job->vm_pd_addr;
@@ -424,7 +424,7 @@ TRACE_EVENT(amdgpu_vm_flush,
 ),
 
TP_fast_assign(
-  __assign_str(ring, ring->name)
+  __assign_str(ring, ring->name);
   __entry->vmid = vmid;
   __entry->vm_hub = ring->funcs->vmhub;
   __entry->pd_addr = pd_addr;
@@ -525,7 +525,7 @@ TRACE_EVENT(amdgpu_ib_pipe_sync,
  

vc4_bo_create: Failed to allocate from CMA

2021-06-12 Thread Stefan Wahren
Hi,

while testing the mainline kernel (arm64, defconfig) on Raspberry Pi 3 B
Plus with Raspberry Pi OS - 64 bit, sometimes X doesn't start into
desktop properly (unexpected and unusable login screen instead of auto
login or mouse pointer is show shorty and than switch back to black
screen in a loop). In that case dmesg shows the following:

[   74.737106] [drm:vc4_bo_create [vc4]] *ERROR* Failed to allocate from
CMA:
[   74.737558] vc4-drm soc:gpu: [drm]    V3D: 
28976kb BOs (10)
[   74.737602] vc4-drm soc:gpu: [drm] V3D
shader: 44kb BOs (11)
[   74.737632] vc4-drm soc:gpu: [drm]   dumb:  
4564kb BOs (5)
[   74.737664] vc4-drm soc:gpu: [drm] binner: 
16384kb BOs (1)
[   74.737697] vc4-drm soc:gpu: [drm]    total purged
BO:  4kb BOs (1)
[   74.739039] [drm:vc4_bo_create [vc4]] *ERROR* Failed to allocate from
CMA:
[   74.739466] vc4-drm soc:gpu: [drm]    V3D: 
28972kb BOs (9)
[   74.739512] vc4-drm soc:gpu: [drm] V3D
shader: 44kb BOs (11)
[   74.739541] vc4-drm soc:gpu: [drm]   dumb:  
4564kb BOs (5)
[   74.739570] vc4-drm soc:gpu: [drm] binner: 
16384kb BOs (1)
[   74.739602] vc4-drm soc:gpu: [drm]    total purged
BO:  4kb BOs (1)
[   74.740718] [drm:vc4_bo_create [vc4]] *ERROR* Failed to allocate from
CMA:
[   74.741138] vc4-drm soc:gpu: [drm]    V3D: 
28972kb BOs (9)
[   74.741171] vc4-drm soc:gpu: [drm] V3D
shader: 44kb BOs (11)
[   74.741202] vc4-drm soc:gpu: [drm]   dumb:  
4564kb BOs (5)
[   74.741231] vc4-drm soc:gpu: [drm] binner: 
16384kb BOs (1)
[   74.741263] vc4-drm soc:gpu: [drm]    total purged
BO:  4kb BOs (1)
...

I have only seen this issue on arm64 with latest mainline kernel
(5.13.0-rc5-00130-gf21b807c3cf8), but also with older kernel versions.
So it's not a regression. It seems 64 bit needs more CMA.

In case X started properly i was also able to reproduce these errors
above by dis- and reconneting HDMI.

So i increased CMA in bcm283x.dtsi and the problem disappeared:

iff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
index b83a864..d1304cb 100644
--- a/arch/arm/boot/dts/bcm283x.dtsi
+++ b/arch/arm/boot/dts/bcm283x.dtsi
@@ -37,7 +37,7 @@
 
     cma: linux,cma {
         compatible = "shared-dma-pool";
-            size = <0x400>; /* 64MB */
+            size = <0x600>; /* 96MB */
         reusable;
         linux,cma-default;
     };

The questions are:

Is this the right way (tm) to fix this problem?

And what is a sensible value (don't have a 4K display to test)?

Best regards
Stefan



[PATCH 2/2] drm: Protect drm_master pointers in drm_lease.c

2021-06-12 Thread Desmond Cheong Zhi Xi
This patch ensures that the device's master mutex is acquired before
accessing pointers to struct drm_master that are subsequently
dereferenced. Without the mutex, the struct drm_master may be freed
concurrently by another process calling drm_setmaster_ioctl(). This
could then lead to use-after-free errors.

Reported-by: Daniel Vetter 
Signed-off-by: Desmond Cheong Zhi Xi 
---
 drivers/gpu/drm/drm_lease.c | 58 +++--
 1 file changed, 43 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
index da4f085fc09e..f2422f1c4915 100644
--- a/drivers/gpu/drm/drm_lease.c
+++ b/drivers/gpu/drm/drm_lease.c
@@ -107,10 +107,16 @@ static bool _drm_has_leased(struct drm_master *master, 
int id)
  */
 bool _drm_lease_held(struct drm_file *file_priv, int id)
 {
+   bool ret;
+
if (!file_priv || !file_priv->master)
return true;
 
-   return _drm_lease_held_master(file_priv->master, id);
+   mutex_lock(&file_priv->master->dev->master_mutex);
+   ret = _drm_lease_held_master(file_priv->master, id);
+   mutex_unlock(&file_priv->master->dev->master_mutex);
+
+   return ret;
 }
 
 /**
@@ -132,10 +138,12 @@ bool drm_lease_held(struct drm_file *file_priv, int id)
if (!file_priv || !file_priv->master || !file_priv->master->lessor)
return true;
 
+   mutex_lock(&file_priv->master->dev->master_mutex);
master = file_priv->master;
mutex_lock(&master->dev->mode_config.idr_mutex);
ret = _drm_lease_held_master(master, id);
mutex_unlock(&master->dev->mode_config.idr_mutex);
+   mutex_unlock(&file_priv->master->dev->master_mutex);
return ret;
 }
 
@@ -158,6 +166,7 @@ uint32_t drm_lease_filter_crtcs(struct drm_file *file_priv, 
uint32_t crtcs_in)
if (!file_priv || !file_priv->master || !file_priv->master->lessor)
return crtcs_in;
 
+   mutex_lock(&file_priv->master->dev->master_mutex);
master = file_priv->master;
dev = master->dev;
 
@@ -177,6 +186,7 @@ uint32_t drm_lease_filter_crtcs(struct drm_file *file_priv, 
uint32_t crtcs_in)
count_in++;
}
mutex_unlock(&master->dev->mode_config.idr_mutex);
+   mutex_unlock(&file_priv->master->dev->master_mutex);
return crtcs_out;
 }
 
@@ -490,7 +500,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;
struct drm_master *lessee = NULL;
struct file *lessee_file = NULL;
struct file *lessor_file = lessor_priv->filp;
@@ -502,12 +512,6 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
if (!drm_core_check_feature(dev, DRIVER_MODESET))
return -EOPNOTSUPP;
 
-   /* Do not allow sub-leases */
-   if (lessor->lessor) {
-   DRM_DEBUG_LEASE("recursive leasing not allowed\n");
-   return -EINVAL;
-   }
-
/* need some objects */
if (cl->object_count == 0) {
DRM_DEBUG_LEASE("no objects in lease\n");
@@ -519,12 +523,23 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
return -EINVAL;
}
 
+   mutex_lock(&dev->master_mutex);
+   lessor = lessor_priv->master;
+   /* Do not allow sub-leases */
+   if (lessor->lessor) {
+   DRM_DEBUG_LEASE("recursive leasing not allowed\n");
+   ret = -EINVAL;
+   goto unlock;
+   }
+
object_count = cl->object_count;
 
object_ids = memdup_user(u64_to_user_ptr(cl->object_ids),
array_size(object_count, sizeof(__u32)));
-   if (IS_ERR(object_ids))
-   return PTR_ERR(object_ids);
+   if (IS_ERR(object_ids)) {
+   ret = PTR_ERR(object_ids);
+   goto unlock;
+   }
 
idr_init(&leases);
 
@@ -535,14 +550,15 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
if (ret) {
DRM_DEBUG_LEASE("lease object lookup failed: %i\n", ret);
idr_destroy(&leases);
-   return ret;
+   goto unlock;
}
 
/* Allocate a file descriptor for the lease */
fd = get_unused_fd_flags(cl->flags & (O_CLOEXEC | O_NONBLOCK));
if (fd < 0) {
idr_destroy(&leases);
-   return fd;
+   ret = fd;
+   goto unlock;
}
 
DRM_DEBUG_LEASE("Creating lease\n");
@@ -578,6 +594,7 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
/* Hook up the fd */
fd_install(fd, lessee_file);
 
+   mutex_unlock(&dev->master_mutex);
DRM_DEBUG_LEASE("drm_mode_create_lease_ioctl succeeded\n");
return 0;
 
@@ -587,6 +604,8 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
 out_leases:
 

[PATCH 1/2] drm: Add a locked version of drm_is_current_master

2021-06-12 Thread Desmond Cheong Zhi Xi
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 
---
 drivers/gpu/drm/drm_auth.c | 23 +++
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 232abbba3686..c6bf52c310a9 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -61,6 +61,8 @@
  * trusted clients.
  */
 
+static bool drm_is_current_master_locked(struct drm_file *fpriv);
+
 int drm_getmagic(struct drm_device *dev, void *data, struct drm_file 
*file_priv)
 {
struct drm_auth *auth = data;
@@ -223,7 +225,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 +274,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 +323,7 @@ void drm_master_release(struct drm_file *file_priv)
if (file_priv->magic)
idr_remove(&file_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,6 +344,13 @@ void drm_master_release(struct drm_file *file_priv)
mutex_unlock(&dev->master_mutex);
 }
 
+static bool drm_is_current_master_locked(struct drm_file *fpriv)
+{
+   lockdep_assert_held_once(&fpriv->master->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
@@ -354,7 +363,13 @@ void drm_master_release(struct drm_file *file_priv)
  */
 bool drm_is_current_master(struct drm_file *fpriv)
 {
-   return fpriv->is_master && drm_lease_owner(fpriv->master) == 
fpriv->minor->dev->master;
+   bool ret;
+
+   mutex_lock(&fpriv->master->dev->master_mutex);
+   ret = drm_is_current_master_locked(fpriv);
+   mutex_unlock(&fpriv->master->dev->master_mutex);
+
+   return ret;
 }
 EXPORT_SYMBOL(drm_is_current_master);
 
-- 
2.25.1



[PATCH 0/2] drm: Address potential UAF bugs with drm_master ptrs

2021-06-12 Thread Desmond Cheong Zhi Xi
This patch 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 two patches:

1. Implement a locked version of drm_is_current_master() function that's used 
within drm_auth.c

2. Identify areas in drm_lease.c where pointers to struct drm_master are 
dereferenced, and ensure that the master pointers are protected by a mutex

Desmond Cheong Zhi Xi (2):
  drm: Add a locked version of drm_is_current_master
  drm: Protect drm_master pointers in drm_lease.c

 drivers/gpu/drm/drm_auth.c  | 23 ---
 drivers/gpu/drm/drm_lease.c | 58 +++--
 2 files changed, 62 insertions(+), 19 deletions(-)

-- 
2.25.1



[v6 2/5] drm/panel-simple: Support DP AUX backlight

2021-06-12 Thread Rajeev Nandan
If there is no backlight specified in the device tree and the panel
has access to the DP AUX channel then create a DP AUX backlight if
supported by the panel.

Signed-off-by: Rajeev Nandan 
Reviewed-by: Douglas Anderson 
---

(no changes since v5)

This patch depends on the previous patch (2/5) of this series.

Changes in v4:
- New

Changes in v5:
- Address review comments and move backlight functions to drm_panel.c (Douglas)
- Create and register DP AUX backlight if there is no backlight specified in the
  device tree and panel has the DP AUX channel. (Douglas)
- The new drm_panel_dp_aux_backlight() will do the 
drm_edp_backlight_supported() check.

 drivers/gpu/drm/panel/panel-simple.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index df6fbd1..26555ec 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -800,6 +800,12 @@ static int panel_simple_probe(struct device *dev, const 
struct panel_desc *desc,
if (err)
goto disable_pm_runtime;
 
+   if (!panel->base.backlight && panel->aux) {
+   err = drm_panel_dp_aux_backlight(&panel->base, panel->aux);
+   if (err)
+   goto disable_pm_runtime;
+   }
+
drm_panel_add(&panel->base);
 
return 0;
-- 
2.7.4



[v6 5/5] drm/panel-simple: Add Samsung ATNA33XC20

2021-06-12 Thread Rajeev Nandan
Add Samsung 13.3" FHD eDP AMOLED panel.

Signed-off-by: Rajeev Nandan 
Reviewed-by: Douglas Anderson 
---

(no changes since v5)

Changes in v4:
- New

Changes in v5:
- Remove "uses_dpcd_backlight" property, not required now. (Douglas)

 drivers/gpu/drm/panel/panel-simple.c | 33 +
 1 file changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index 86e5a45..23242fc 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -3562,6 +3562,36 @@ static const struct panel_desc rocktech_rk101ii01d_ct = {
.connector_type = DRM_MODE_CONNECTOR_LVDS,
 };
 
+static const struct drm_display_mode samsung_atna33xc20_mode = {
+   .clock = 138770,
+   .hdisplay = 1920,
+   .hsync_start = 1920 + 48,
+   .hsync_end = 1920 + 48 + 32,
+   .htotal = 1920 + 48 + 32 + 80,
+   .vdisplay = 1080,
+   .vsync_start = 1080 + 8,
+   .vsync_end = 1080 + 8 + 8,
+   .vtotal = 1080 + 8 + 8 + 16,
+   .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC,
+};
+
+static const struct panel_desc samsung_atna33xc20 = {
+   .modes = &samsung_atna33xc20_mode,
+   .num_modes = 1,
+   .bpc = 10,
+   .size = {
+   .width = 294,
+   .height = 165,
+   },
+   .delay = {
+   .disable_to_power_off = 150,
+   .power_to_enable = 150,
+   .hpd_absent_delay = 200,
+   .unprepare = 500,
+   },
+   .connector_type = DRM_MODE_CONNECTOR_eDP,
+};
+
 static const struct drm_display_mode samsung_lsn122dl01_c01_mode = {
.clock = 271560,
.hdisplay = 2560,
@@ -4563,6 +4593,9 @@ static const struct of_device_id platform_of_match[] = {
.compatible = "rocktech,rk101ii01d-ct",
.data = &rocktech_rk101ii01d_ct,
}, {
+   .compatible = "samsung,atna33xc20",
+   .data = &samsung_atna33xc20,
+   }, {
.compatible = "samsung,lsn122dl01-c01",
.data = &samsung_lsn122dl01_c01,
}, {
-- 
2.7.4



[v6 1/5] drm/panel: add basic DP AUX backlight support

2021-06-12 Thread Rajeev Nandan
Some panels support backlight control over DP AUX channel using
VESA's standard backlight control interface.
Using new DRM eDP backlight helpers, add support to create and
register a backlight for those panels in drm_panel to simplify
the panel drivers.

The panel driver with access to "struct drm_dp_aux" can create and
register a backlight device using following code snippet in its
probe() function:

err = drm_panel_dp_aux_backlight(panel, aux);
if (err)
return err;

Then drm_panel will handle backlight_(enable|disable) calls
similar to the case when drm_panel_of_backlight() is used.

Currently, we are not supporting one feature where the source
device can combine the backlight brightness levels set through
DP AUX and the BL_PWM_DIM eDP connector pin. Since it's not
required for the basic backlight controls, it can be added later.

Signed-off-by: Rajeev Nandan 
Reviewed-by: Douglas Anderson 
---

Changes in v5:
- New

Changes in v6:
- Fixed ordering of memory allocation (Douglas)
- Updated word wrapping in a comment (Douglas)

 drivers/gpu/drm/drm_panel.c | 108 
 include/drm/drm_panel.h |  15 --
 2 files changed, 119 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index f634371..9e65342 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -26,12 +26,20 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 
 static DEFINE_MUTEX(panel_lock);
 static LIST_HEAD(panel_list);
 
+struct dp_aux_backlight {
+   struct backlight_device *base;
+   struct drm_dp_aux *aux;
+   struct drm_edp_backlight_info info;
+   bool enabled;
+};
+
 /**
  * DOC: drm panel
  *
@@ -342,6 +350,106 @@ int drm_panel_of_backlight(struct drm_panel *panel)
return 0;
 }
 EXPORT_SYMBOL(drm_panel_of_backlight);
+
+static int dp_aux_backlight_update_status(struct backlight_device *bd)
+{
+   struct dp_aux_backlight *bl = bl_get_data(bd);
+   u16 brightness = backlight_get_brightness(bd);
+   int ret = 0;
+
+   if (brightness > 0) {
+   if (!bl->enabled) {
+   drm_edp_backlight_enable(bl->aux, &bl->info, 
brightness);
+   bl->enabled = true;
+   return 0;
+   }
+   ret = drm_edp_backlight_set_level(bl->aux, &bl->info, 
brightness);
+   } else {
+   if (bl->enabled) {
+   drm_edp_backlight_disable(bl->aux, &bl->info);
+   bl->enabled = false;
+   }
+   }
+
+   return ret;
+}
+
+static const struct backlight_ops dp_aux_bl_ops = {
+   .update_status = dp_aux_backlight_update_status,
+};
+
+/**
+ * drm_panel_dp_aux_backlight - create and use DP AUX backlight
+ * @panel: DRM panel
+ * @aux: The DP AUX channel to use
+ *
+ * Use this function to create and handle backlight if your panel
+ * supports backlight control over DP AUX channel using DPCD
+ * registers as per VESA's standard backlight control interface.
+ *
+ * When the panel is enabled backlight will be enabled after a
+ * successful call to &drm_panel_funcs.enable()
+ *
+ * When the panel is disabled backlight will be disabled before the
+ * call to &drm_panel_funcs.disable().
+ *
+ * A typical implementation for a panel driver supporting backlight
+ * control over DP AUX will call this function at probe time.
+ * Backlight will then be handled transparently without requiring
+ * any intervention from the driver.
+ *
+ * drm_panel_dp_aux_backlight() must be called after the call to 
drm_panel_init().
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux)
+{
+   struct dp_aux_backlight *bl;
+   struct backlight_properties props = { 0 };
+   u16 current_level;
+   u8 current_mode;
+   u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
+   int ret;
+
+   if (!panel || !panel->dev || !aux)
+   return -EINVAL;
+
+   ret = drm_dp_dpcd_read(aux, DP_EDP_DPCD_REV, edp_dpcd,
+  EDP_DISPLAY_CTL_CAP_SIZE);
+   if (ret < 0)
+   return ret;
+
+   if (!drm_edp_backlight_supported(edp_dpcd)) {
+   DRM_DEV_INFO(panel->dev, "DP AUX backlight is not supported\n");
+   return 0;
+   }
+
+   bl = devm_kzalloc(panel->dev, sizeof(*bl), GFP_KERNEL);
+   if (!bl)
+   return -ENOMEM;
+
+   bl->aux = aux;
+
+   ret = drm_edp_backlight_init(aux, &bl->info, 0, edp_dpcd,
+¤t_level, ¤t_mode);
+   if (ret < 0)
+   return ret;
+
+   props.type = BACKLIGHT_RAW;
+   props.brightness = current_level;
+   props.max_brightness = bl->info.max;
+
+   bl->base = devm_backlight_device_register(panel->dev, 
"dp_aux_backlight",
+ 

[v6 4/5] dt-bindings: display: simple: Add Samsung ATNA33XC20

2021-06-12 Thread Rajeev Nandan
Add Samsung 13.3" FHD eDP AMOLED panel.

Signed-off-by: Rajeev Nandan 
Reviewed-by: Douglas Anderson 
Acked-by: Rob Herring 
---

(no changes since v4)

Changes in v4:
- New

 Documentation/devicetree/bindings/display/panel/panel-simple.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml 
b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
index 4a0a5e1..f5acfd6 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
@@ -242,6 +242,8 @@ properties:
   - rocktech,rk101ii01d-ct
 # Rocktech Display Ltd. RK070ER9427 800(RGB)x480 TFT LCD panel
   - rocktech,rk070er9427
+# Samsung 13.3" FHD (1920x1080 pixels) eDP AMOLED panel
+  - samsung,atna33xc20
 # Samsung 12.2" (2560x1600 pixels) TFT LCD panel
   - samsung,lsn122dl01-c01
 # Samsung Electronics 10.1" WSVGA TFT LCD panel
-- 
2.7.4



[v6 3/5] drm/panel-simple: Support for delays between GPIO & regulator

2021-06-12 Thread Rajeev Nandan
Some panels datasheets may specify a delay between the enable GPIO and
the regulator. Support this in panel-simple.

Signed-off-by: Rajeev Nandan 
Reviewed-by: Douglas Anderson 
---

Changes in v4:
- New

Changes in v5:
- Update description (Douglas)
- Warn if "power_to_enable" or "disable_to_power_off" is non-zero and 
panel->enable_gpio
  is NULL (Douglas)

Changes in v6:
- Update warning message to make it more meaningful. (Douglas)

 drivers/gpu/drm/panel/panel-simple.c | 28 
 1 file changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index 26555ec..86e5a45 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -133,6 +133,22 @@ struct panel_desc {
unsigned int prepare_to_enable;
 
/**
+* @delay.power_to_enable: Time for the power to enable the 
display on.
+*
+* The time (in milliseconds) to wait after powering up the 
display
+* before asserting its enable pin.
+*/
+   unsigned int power_to_enable;
+
+   /**
+* @delay.disable_to_power_off: Time for the disable to power 
the display off.
+*
+* The time (in milliseconds) to wait before powering off the 
display
+* after deasserting its enable pin.
+*/
+   unsigned int disable_to_power_off;
+
+   /**
 * @delay.enable: Time for the panel to display a valid frame.
 *
 * The time (in milliseconds) that it takes for the panel to
@@ -347,6 +363,10 @@ static int panel_simple_suspend(struct device *dev)
struct panel_simple *p = dev_get_drvdata(dev);
 
gpiod_set_value_cansleep(p->enable_gpio, 0);
+
+   if (p->desc->delay.disable_to_power_off)
+   msleep(p->desc->delay.disable_to_power_off);
+
regulator_disable(p->supply);
p->unprepared_time = ktime_get();
 
@@ -407,6 +427,9 @@ static int panel_simple_prepare_once(struct panel_simple *p)
return err;
}
 
+   if (p->desc->delay.power_to_enable)
+   msleep(p->desc->delay.power_to_enable);
+
gpiod_set_value_cansleep(p->enable_gpio, 1);
 
delay = p->desc->delay.prepare;
@@ -782,6 +805,11 @@ static int panel_simple_probe(struct device *dev, const 
struct panel_desc *desc,
break;
}
 
+   if (!panel->enable_gpio && desc->delay.disable_to_power_off)
+   dev_warn(dev, "Need a delay after disabling panel GPIO, but a 
GPIO wasn't provided\n");
+   if (!panel->enable_gpio && desc->delay.power_to_enable)
+   dev_warn(dev, "Need a delay before enabling panel GPIO, but a 
GPIO wasn't provided\n");
+
dev_set_drvdata(dev, panel);
 
/*
-- 
2.7.4



[v6 0/5] drm: Support basic DPCD backlight in panel-simple and add a new panel ATNA33XC20

2021-06-12 Thread Rajeev Nandan
This series adds the support for the eDP panel that needs the backlight
controlling over the DP AUX channel using DPCD registers of the panel
as per the VESA's standard.

This series also adds support for the Samsung eDP AMOLED panel that
needs DP AUX to control the backlight, and introduces new delays in the
@panel_desc.delay to support this panel.

This patch series depends on the following two series:
- Doug's series [1], exposed the DP AUX channel to the panel-simple.
- Lyude's series [2], introduced new drm helper functions for DPCD
  backlight.

This series is the logical successor to the series [3].

Changes in v1:
- Created dpcd backlight helper with very basic functionality, added
  backlight registration in the ti-sn65dsi86 bridge driver.

Changes in v2:
- Created a new DisplayPort aux backlight driver and moved the code from
  drm_dp_aux_backlight.c (v1) to the new driver.

Changes in v3:
- Fixed module compilation (kernel test bot).

Changes in v4:
- Added basic DPCD backlight support in panel-simple.
- Added support for a new Samsung panel ATNA33XC20 that needs DPCD
  backlight controlling and has a requirement of delays between enable
  GPIO and regulator.

Changes in v5:
Addressed review suggestions from Douglas:
- Created a new API drm_panel_dp_aux_backlight() in drm_panel.c
- Moved DP AUX backlight functions from panel-simple.c to drm_panel.c
- panel-simple probe() calls drm_panel_dp_aux_backlight() to create
  backlight when the backlight phandle is not specified in panel DT
  and DP AUX channel is present.
- Added check for drm_edp_backlight_supported() before registering.
- Removed the @uses_dpcd_backlight flag from panel_desc as this
  should be auto-detected.
- Updated comments/descriptions.

Changes in v6:
- Rebased
- Updated wanrning messages, fixed word wrapping in comments.
- Fixed ordering of memory allocation

[1] 
https://lore.kernel.org/dri-devel/20210525000159.3384921-1-diand...@chromium.org/
[2] https://lore.kernel.org/dri-devel/20210514181504.565252-1-ly...@redhat.com/
[3] 
https://lore.kernel.org/dri-devel/1619416756-3533-1-git-send-email-rajee...@codeaurora.org/

Rajeev Nandan (5):
  drm/panel: add basic DP AUX backlight support
  drm/panel-simple: Support DP AUX backlight
  drm/panel-simple: Support for delays between GPIO & regulator
  dt-bindings: display: simple: Add Samsung ATNA33XC20
  drm/panel-simple: Add Samsung ATNA33XC20

 .../bindings/display/panel/panel-simple.yaml   |   2 +
 drivers/gpu/drm/drm_panel.c| 108 +
 drivers/gpu/drm/panel/panel-simple.c   |  67 +
 include/drm/drm_panel.h|  15 ++-
 4 files changed, 188 insertions(+), 4 deletions(-)

-- 
2.7.4



Re: [PATCH] fix for checkpatch.pl

2021-06-12 Thread Greg KH
On Sat, Jun 12, 2021 at 05:15:05PM +0530, arjuncr wrote:
> Signed-off-by: arjuncr 
> ---
>  drivers/staging/fbtft/fbtft-bus.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/fbtft/fbtft-bus.c 
> b/drivers/staging/fbtft/fbtft-bus.c
> index 63c65dd67..cdb451dd3 100644
> --- a/drivers/staging/fbtft/fbtft-bus.c
> +++ b/drivers/staging/fbtft/fbtft-bus.c
> @@ -62,9 +62,9 @@ out:
>   \
>  }
>  \
>  EXPORT_SYMBOL(func);
>  
> -define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8, )
> +define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8,)
>  define_fbtft_write_reg(fbtft_write_reg16_bus8, __be16, u16, cpu_to_be16)
> -define_fbtft_write_reg(fbtft_write_reg16_bus16, u16, u16, )
> +define_fbtft_write_reg(fbtft_write_reg16_bus16, u16, u16,)
>  
>  void fbtft_write_reg8_bus9(struct fbtft_par *par, int len, ...)
>  {
> -- 
> 2.25.1
> 
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- You did not specify a description of why the patch is needed, or
  possibly, any description at all, in the email body.  Please read the
  section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what is needed in order to
  properly describe the change.

- You did not write a descriptive Subject: for the patch, allowing Greg,
  and everyone else, to know what this patch is all about.  Please read
  the section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what a proper Subject: line should
  look like.

- It looks like you did not use your "real" name for the patch on either
  the Signed-off-by: line, or the From: line (both of which have to
  match).  Please read the kernel file, Documentation/SubmittingPatches
  for how to do this correctly.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot


Re: vc4: hdmi: audio: ASoC: error at snd_soc_dai_startup on fef00700.hdmi

2021-06-12 Thread Stefan Wahren
Hi Maxime,

Am 04.06.21 um 11:02 schrieb Maxime Ripard:
> Hi Stefan,
>
> I would assume it's due to this:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/vc4/vc4_hdmi.c#n1083
>
> It pre-dates my time working on the vc4 driver so I'm not really sure
> what this is supposed to prevent, but my guess is that it's there to
> avoid someone using the audio card before we have a display detected and
> connected, and its capabilities known (the first and more obvious one
> being does it support audio in the first place).
>
> It's nothing new though, maybe it's the error printing itself that is?

i'm sorry, i forgot about this discussion here:

https://lists.freedesktop.org/archives/dri-devel/2020-December/292701.html

Regards
Stefan

>
> Maxime


Re: [PATCH v2] drm/msm/dpu: Avoid ABBA deadlock between IRQ modules

2021-06-12 Thread Dmitry Baryshkov

On 11/06/2021 20:00, Bjorn Andersson wrote:

Handling of the interrupt callback lists is done in dpu_core_irq.c,
under the "cb_lock" spinlock. When these operations results in the need
for enableing or disabling the IRQ in the hardware the code jumps to
dpu_hw_interrupts.c, which protects its operations with "irq_lock"
spinlock.

When an interrupt fires, dpu_hw_intr_dispatch_irq() inspects the
hardware state while holding the "irq_lock" spinlock and jumps to
dpu_core_irq_callback_handler() to invoke the registered handlers, which
traverses the callback list under the "cb_lock" spinlock.

As such, in the event that these happens concurrently we'll end up with
a deadlock.

Prior to '1c1e7763a6d4 ("drm/msm/dpu: simplify IRQ enabling/disabling")'
the enable/disable of the hardware interrupt was done outside the
"cb_lock" region, optimitically by using an atomic enable-counter for
each interrupt and an warning print if someone changed the list between
the atomic_read and the time the operation concluded.

Rather than re-introducing the large array of atomics, this change
embraces the fact that dpu_core_irq and dpu_hw_interrupts are deeply
entangled and make them share the single "irq_lock".

Following this step it's suggested that we squash the two parts into a
single irq handling thing.

Fixes: 1c1e7763a6d4 ("drm/msm/dpu: simplify IRQ enabling/disabling")
Signed-off-by: Bjorn Andersson 


Reviewed-by: Dmitry Baryshkov 


---

Changes since v1:
- Make dpu_core_irq use dpu_hw_interrupts' irq_lock instead of adding another
   mutex.

  drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c  | 27 -
  .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 60 +++
  .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 20 ++-
  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h   |  2 -
  4 files changed, 63 insertions(+), 46 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 4f110c428b60..18557b9713b6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
@@ -22,7 +22,6 @@ static void dpu_core_irq_callback_handler(void *arg, int 
irq_idx)
struct dpu_kms *dpu_kms = arg;
struct dpu_irq *irq_obj = &dpu_kms->irq_obj;
struct dpu_irq_callback *cb;
-   unsigned long irq_flags;
  
  	pr_debug("irq_idx=%d\n", irq_idx);
  
@@ -34,11 +33,9 @@ static void dpu_core_irq_callback_handler(void *arg, int irq_idx)

/*
 * Perform registered function callback
 */
-   spin_lock_irqsave(&dpu_kms->irq_obj.cb_lock, irq_flags);
list_for_each_entry(cb, &irq_obj->irq_cb_tbl[irq_idx], list)
if (cb->func)
cb->func(cb->arg, irq_idx);
-   spin_unlock_irqrestore(&dpu_kms->irq_obj.cb_lock, irq_flags);
  }
  
  u32 dpu_core_irq_read(struct dpu_kms *dpu_kms, int irq_idx, bool clear)

@@ -82,22 +79,21 @@ int dpu_core_irq_register_callback(struct dpu_kms *dpu_kms, 
int irq_idx,
  
  	DPU_DEBUG("[%pS] irq_idx=%d\n", __builtin_return_address(0), irq_idx);
  
-	spin_lock_irqsave(&dpu_kms->irq_obj.cb_lock, irq_flags);

+   irq_flags = dpu_kms->hw_intr->ops.lock(dpu_kms->hw_intr);
trace_dpu_core_irq_register_callback(irq_idx, register_irq_cb);
list_del_init(®ister_irq_cb->list);
list_add_tail(®ister_irq_cb->list,
&dpu_kms->irq_obj.irq_cb_tbl[irq_idx]);
if (list_is_first(®ister_irq_cb->list,
&dpu_kms->irq_obj.irq_cb_tbl[irq_idx])) {
-   int ret = dpu_kms->hw_intr->ops.enable_irq(
+   int ret = dpu_kms->hw_intr->ops.enable_irq_locked(
dpu_kms->hw_intr,
irq_idx);
if (ret)
DPU_ERROR("Fail to enable IRQ for irq_idx:%d\n",
irq_idx);
}
-
-   spin_unlock_irqrestore(&dpu_kms->irq_obj.cb_lock, irq_flags);
+   dpu_kms->hw_intr->ops.unlock(dpu_kms->hw_intr, irq_flags);
  
  	return 0;

  }
@@ -127,12 +123,12 @@ int dpu_core_irq_unregister_callback(struct dpu_kms 
*dpu_kms, int irq_idx,
  
  	DPU_DEBUG("[%pS] irq_idx=%d\n", __builtin_return_address(0), irq_idx);
  
-	spin_lock_irqsave(&dpu_kms->irq_obj.cb_lock, irq_flags);

+   irq_flags = dpu_kms->hw_intr->ops.lock(dpu_kms->hw_intr);
trace_dpu_core_irq_unregister_callback(irq_idx, register_irq_cb);
list_del_init(®ister_irq_cb->list);
/* empty callback list but interrupt is still enabled */
if (list_empty(&dpu_kms->irq_obj.irq_cb_tbl[irq_idx])) {
-   int ret = dpu_kms->hw_intr->ops.disable_irq(
+   int ret = dpu_kms->hw_intr->ops.disable_irq_locked(
dpu_kms->hw_intr,
irq_idx);
if (ret)
@@ -140,7 +136,7 @@ int dpu_core_irq_unregister_callback(struct dpu_kms 
*dpu_kms, int irq_idx,

Re: [Intel-gfx] [PATCH v2 2/4] drm/i915/ttm: Adjust gem flags and caching settings after a move

2021-06-12 Thread Thomas Hellström
On Fri, 2021-06-11 at 17:29 +0100, Matthew Auld wrote:
> On Fri, 11 Jun 2021 at 15:55, Thomas Hellström
>  wrote:
> > 
> > After a TTM move we need to update the i915 gem flags and caching
> > settings to reflect the new placement.
> > Also introduce gpu_binds_iomem() and cpu_maps_iomem() to clean up
> > the
> > various ways we previously used to detect this.
> > Finally, initialize the TTM object reserved to be able to update
> > flags and caching before anyone else gets hold of the object.
> 
> Hmm, why do we need to update it after a move? Is it not static? i.e
> we just consider the mm.placements/region to determine the correct
> domain and cache tracking? Or maybe it doesn't really matter either
> way?

Flags are not static, currently. If migrating from LMEM to SYSTEM, they
need to be updated. Caching and domains should remain unchanged for now
because of the rule that we don't want to change caching mode when
migrating from LMEM to SYSTEM for buffers that support both, and that
rule is enforced by setting the ttm_tt caching mode accordingly.
However, I figure if we need to change that rule moving forward because
we decide we can't rely on the TTM shinker for WC system pages, or
because allocating WC system pages is too expensive, It would be good
if we don't need to audit all the code to find places where an updated
caching policy needs changes.

/Thomas