Re: WARNING in amdgpu_sync_keep_later / dma_fence_is_later should be rate limited

[Christian König]

Am 21.09.23 um 23:30 schrieb Alex Deucher:

On Thu, Sep 21, 2023 at 4:21 PM Rafał Miłecki  wrote:

On 21.09.2023 21:52, Deucher, Alexander wrote:

backporting commit 187916e6ed9d ("drm/amdgpu: install stub fence into
potential unused fence pointers") to stable kernels resulted in lots of
WARNINGs on some devices. In my case I was getting 3 WARNINGs per
second (~150 lines logged every second). Commit ended up being reverted for
stable but it exposed a potential problem. My messages log size was reaching
gigabytes and was running my /tmp/ out of space.

Could someone take a look at amdgpu_sync_keep_later / dma_fence_is_later
and make sure its logging is rate limited to avoid such situations in the 
future,
please?

Revert in linux-5.15.x:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=li
nux-5.15.y=fae2d591f3cb31f722c7f065acf586830eab8c2a

openSUSE bug report:
https://bugzilla.opensuse.org/show_bug.cgi?id=1215523

These patches were never intended for stable.  They were picked up by Sasha's 
stable autoselect tools and automatically applied to stable kernels.

Are you saying massive WARNINGs in dma_fence_is_later() can't happen
in any other case? I understand it was an incorrect backport action but
I thought we may learn from it and still add some rate limit.

All of the current places where that function is used check the
contexts before calling it so it should be safe as is in the tree.
That said, something like this could potentially happen again.  I
don't think using WARN_ON_RATELIMIT() would be a problem.


Yeah, but it also shouldn't be necessary.

When this triggers you have a major driver bug at hand, spamming the 
logs is then the least of your problems.


Christian.



Alex

Re: [PATCH 0/5] drm/amd/display: Remove migrate-disable and move memory allocation.

[Christian König]

Am 21.09.23 um 16:15 schrieb Sebastian Andrzej Siewior:

Hi,

I stumbled uppon the amdgpu driver via a bugzilla report. The actual fix
is #4 + #5 and the rest was made while looking at the code.


Oh, yes please :)

Rodrigo and I have been trying to sort those things out previously, but 
that's Sisyphean work.


In general the DC team needs to judge, but of hand it looks good to me.

Christian.



Sebastian

[Bug 201957] amdgpu: ring gfx timeout

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=201957

G OConnor (graham.ocon...@gmail.com) changed:

   What|Removed |Added

 CC||graham.ocon...@gmail.com

--- Comment #90 from G OConnor (graham.ocon...@gmail.com) ---
AMD Ryzen 3700U APU (Vega 10)

This issue has recently started happening, mostly when firing up games or
graphically intensive tasks. One case of lockup during normal desktop use.

Worked fine on 6.4.X series (currently running on 6.4.12). However, all kernels
in the 6.5 series cause the following:

[  112.727138] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx_low timeout,
signaled seq=9861, emitted seq=9863
[  112.728214] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information:
process Xwayland pid 919 thread Xwayland:cs0 pid 928
[  112.729270] amdgpu :04:00.0: amdgpu: GPU reset begin!
[  112.885652] amdgpu :04:00.0: amdgpu: MODE2 reset
[  112.885709] amdgpu :04:00.0: amdgpu: GPU reset succeeded, trying to
resume
[  112.886024] [drm] PCIE GART of 1024M enabled.
[  112.886027] [drm] PTB located at 0x00F400A0
[  112.886143] [drm] PSP is resuming...
[  112.906168] [drm] reserve 0x40 from 0xf47fc0 for PSP TMR
[  112.985033] amdgpu :04:00.0: amdgpu: RAS: optional ras ta ucode is not
available
[  112.992320] amdgpu :04:00.0: amdgpu: RAP: optional rap ta ucode is not
available
[  113.733685] [drm] kiq ring mec 2 pipe 1 q 0
[  113.998619] amdgpu :04:00.0: [drm:amdgpu_ring_test_helper [amdgpu]]
*ERROR* ring gfx test failed (-110)
[  113.999249] [drm:amdgpu_device_ip_resume_phase2 [amdgpu]] *ERROR* resume of
IP block  failed -110
[  113.57] amdgpu :04:00.0: amdgpu: GPU reset(2) failed
[  114.06] amdgpu :04:00.0: amdgpu: GPU reset end with ret = -110
[  114.10] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* GPU Recovery Failed:
-110

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

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

Re: [PATCH v2 3/6] drm: lcdif: rework runtime PM handling in the atomic commit

[kernel test robot]

Hi Lucas,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on linus/master v6.6-rc2 next-20230921]
[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#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Lucas-Stach/drm-lcdif-don-t-clear-unrelated-bits-in-CTRLDESCL0_5-when-setting-up-format/20230922-040438
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:
https://lore.kernel.org/r/20230921200312.3989073-3-l.stach%40pengutronix.de
patch subject: [PATCH v2 3/6] drm: lcdif: rework runtime PM handling in the 
atomic commit
config: m68k-allyesconfig 
(https://download.01.org/0day-ci/archive/20230922/202309220530.84slbdtu-...@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20230922/202309220530.84slbdtu-...@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202309220530.84slbdtu-...@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/mxsfb/lcdif_drv.c:39:6: warning: no previous prototype for 
>> 'lcdif_commit_tail' [-Wmissing-prototypes]
  39 | void lcdif_commit_tail(struct drm_atomic_state *old_state)
 |  ^


vim +/lcdif_commit_tail +39 drivers/gpu/drm/mxsfb/lcdif_drv.c

38  
  > 39  void lcdif_commit_tail(struct drm_atomic_state *old_state)
40  {
41  struct drm_device *drm = old_state->dev;
42  
43  pm_runtime_get_sync(drm->dev);
44  
45  drm_atomic_helper_commit_modeset_disables(drm, old_state);
46  drm_atomic_helper_commit_planes(drm, old_state,
47  DRM_PLANE_COMMIT_ACTIVE_ONLY);
48  drm_atomic_helper_commit_modeset_enables(drm, old_state);
49  
50  drm_atomic_helper_fake_vblank(old_state);
51  drm_atomic_helper_commit_hw_done(old_state);
52  drm_atomic_helper_wait_for_vblanks(drm, old_state);
53  
54  pm_runtime_put(drm->dev);
55  
56  drm_atomic_helper_cleanup_planes(drm, old_state);
57  }
58  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: WARNING in amdgpu_sync_keep_later / dma_fence_is_later should be rate limited

[Alex Deucher]

On Thu, Sep 21, 2023 at 4:21 PM Rafał Miłecki  wrote:
>
> On 21.09.2023 21:52, Deucher, Alexander wrote:
> >> backporting commit 187916e6ed9d ("drm/amdgpu: install stub fence into
> >> potential unused fence pointers") to stable kernels resulted in lots of
> >> WARNINGs on some devices. In my case I was getting 3 WARNINGs per
> >> second (~150 lines logged every second). Commit ended up being reverted for
> >> stable but it exposed a potential problem. My messages log size was 
> >> reaching
> >> gigabytes and was running my /tmp/ out of space.
> >>
> >> Could someone take a look at amdgpu_sync_keep_later / dma_fence_is_later
> >> and make sure its logging is rate limited to avoid such situations in the 
> >> future,
> >> please?
> >>
> >> Revert in linux-5.15.x:
> >> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=li
> >> nux-5.15.y=fae2d591f3cb31f722c7f065acf586830eab8c2a
> >>
> >> openSUSE bug report:
> >> https://bugzilla.opensuse.org/show_bug.cgi?id=1215523
> >
> > These patches were never intended for stable.  They were picked up by 
> > Sasha's stable autoselect tools and automatically applied to stable kernels.
>
> Are you saying massive WARNINGs in dma_fence_is_later() can't happen
> in any other case? I understand it was an incorrect backport action but
> I thought we may learn from it and still add some rate limit.

All of the current places where that function is used check the
contexts before calling it so it should be safe as is in the tree.
That said, something like this could potentially happen again.  I
don't think using WARN_ON_RATELIMIT() would be a problem.

Alex

[PATCH v2 5/6] drm: lcdif: move pitch setup to plane atomic update

[Lucas Stach]

The buffer pitch may change when switching the buffer on a
atomic update. As the register is double buffered it can be
safely changed while the display is active.

Signed-off-by: Lucas Stach 
Reviewed-by: Marek Vasut 
---
v2: no changes
---
 drivers/gpu/drm/mxsfb/lcdif_kms.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c 
b/drivers/gpu/drm/mxsfb/lcdif_kms.c
index 4e9284b0d12e..daf54ff2b7bd 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_kms.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c
@@ -327,19 +327,6 @@ static void lcdif_set_mode(struct lcdif_drm_private 
*lcdif, u32 bus_flags)
writel(CTRLDESCL0_1_HEIGHT(m->vdisplay) |
   CTRLDESCL0_1_WIDTH(m->hdisplay),
   lcdif->base + LCDC_V8_CTRLDESCL0_1);
-
-   /*
-* The P_SIZE and T_SIZE bitfields are only documented in the
-* downstream driver. Those bitfields control the AXI burst size.
-* As of now there are two known values:
-*  1 - 128Byte
-*  2 - 256Byte
-* Downstream sets this to 256B burst size to improve the memory access
-* efficiency so set it here too.
-*/
-   ctrl = CTRLDESCL0_3_P_SIZE(2) | CTRLDESCL0_3_T_SIZE(2) |
-  CTRLDESCL0_3_PITCH(lcdif->crtc.primary->state->fb->pitches[0]);
-   writel(ctrl, lcdif->base + LCDC_V8_CTRLDESCL0_3);
 }
 
 static void lcdif_enable_controller(struct lcdif_drm_private *lcdif)
@@ -694,6 +681,19 @@ static void lcdif_plane_primary_atomic_update(struct 
drm_plane *plane,
writel(CTRLDESCL_HIGH0_4_ADDR_HIGH(upper_32_bits(paddr)),
   lcdif->base + LCDC_V8_CTRLDESCL_HIGH0_4);
}
+
+   /*
+* The P_SIZE and T_SIZE bitfields are only documented in the
+* downstream driver. Those bitfields control the AXI burst size.
+* As of now there are two known values:
+*  1 - 128Byte
+*  2 - 256Byte
+* Downstream sets this to 256B burst size to improve the memory access
+* efficiency so set it here too.
+*/
+   writel(CTRLDESCL0_3_P_SIZE(2) | CTRLDESCL0_3_T_SIZE(2) |
+  CTRLDESCL0_3_PITCH(new_pstate->fb->pitches[0]),
+  lcdif->base + LCDC_V8_CTRLDESCL0_3);
 }
 
 static bool lcdif_format_mod_supported(struct drm_plane *plane,
-- 
2.39.2

[PATCH v2 1/6] drm: lcdif: improve burst size configuration comment

[Lucas Stach]

The comment regarding AXI bust size configuration is a bit hard
to read. Improve the wording somewhat.

Signed-off-by: Lucas Stach 
Reviewed-by: Marco Felsch 
Reviewed-by: Marek Vasut 
---
v2: Some more rewording.
---
 drivers/gpu/drm/mxsfb/lcdif_kms.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c 
b/drivers/gpu/drm/mxsfb/lcdif_kms.c
index 2541d2de4e45..07e343e01f3e 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_kms.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c
@@ -329,12 +329,12 @@ static void lcdif_set_mode(struct lcdif_drm_private 
*lcdif, u32 bus_flags)
   lcdif->base + LCDC_V8_CTRLDESCL0_1);
 
/*
-* Undocumented P_SIZE and T_SIZE register but those written in the
-* downstream kernel those registers control the AXI burst size. As of
-* now there are two known values:
+* The P_SIZE and T_SIZE bitfields are only documented in the
+* downstream driver. Those bitfields control the AXI burst size.
+* As of now there are two known values:
 *  1 - 128Byte
 *  2 - 256Byte
-* Downstream set it to 256B burst size to improve the memory
+* Downstream sets this to 256B burst size to improve the memory access
 * efficiency so set it here too.
 */
ctrl = CTRLDESCL0_3_P_SIZE(2) | CTRLDESCL0_3_T_SIZE(2) |
-- 
2.39.2

[PATCH v2 2/6] drm: lcdif: don't clear unrelated bits in CTRLDESCL0_5 when setting up format

[Lucas Stach]

The CTRLDESCL0_5 register also holds other bits that are not related to the
format, which should not be overwritten when the format is set up. Use a
proper RMW access in lcdif_set_formats().

Signed-off-by: Lucas Stach 
---
v2: new patch
---
 drivers/gpu/drm/mxsfb/lcdif_kms.c | 40 +++
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c 
b/drivers/gpu/drm/mxsfb/lcdif_kms.c
index 07e343e01f3e..e277592e5fa5 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_kms.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c
@@ -166,6 +166,7 @@ static void lcdif_set_formats(struct lcdif_drm_private 
*lcdif,
const u32 format = plane_state->fb->format->format;
bool in_yuv = false;
bool out_yuv = false;
+   u32 ctrl_desc_5;
 
switch (bus_format) {
case MEDIA_BUS_FMT_RGB565_1X16:
@@ -186,52 +187,49 @@ static void lcdif_set_formats(struct lcdif_drm_private 
*lcdif,
break;
}
 
+   ctrl_desc_5 = readl(lcdif->base + LCDC_V8_CTRLDESCL0_5) &
+ ~(CTRLDESCL0_5_BPP_MASK | CTRLDESCL0_5_YUV_FORMAT_MASK);
+
switch (format) {
/* RGB Formats */
case DRM_FORMAT_RGB565:
-   writel(CTRLDESCL0_5_BPP_16_RGB565,
-  lcdif->base + LCDC_V8_CTRLDESCL0_5);
+   ctrl_desc_5 |= CTRLDESCL0_5_BPP_16_RGB565;
break;
case DRM_FORMAT_RGB888:
-   writel(CTRLDESCL0_5_BPP_24_RGB888,
-  lcdif->base + LCDC_V8_CTRLDESCL0_5);
+   ctrl_desc_5 |= CTRLDESCL0_5_BPP_24_RGB888;
break;
case DRM_FORMAT_XRGB1555:
-   writel(CTRLDESCL0_5_BPP_16_ARGB1555,
-  lcdif->base + LCDC_V8_CTRLDESCL0_5);
+   ctrl_desc_5 |= CTRLDESCL0_5_BPP_16_ARGB1555;
break;
case DRM_FORMAT_XRGB:
-   writel(CTRLDESCL0_5_BPP_16_ARGB,
-  lcdif->base + LCDC_V8_CTRLDESCL0_5);
+   ctrl_desc_5 |= CTRLDESCL0_5_BPP_16_ARGB;
break;
case DRM_FORMAT_XBGR:
-   writel(CTRLDESCL0_5_BPP_32_ABGR,
-  lcdif->base + LCDC_V8_CTRLDESCL0_5);
+   ctrl_desc_5 |= CTRLDESCL0_5_BPP_32_ABGR;
break;
case DRM_FORMAT_XRGB:
-   writel(CTRLDESCL0_5_BPP_32_ARGB,
-  lcdif->base + LCDC_V8_CTRLDESCL0_5);
+   ctrl_desc_5 |= CTRLDESCL0_5_BPP_32_ARGB;
break;
 
/* YUV Formats */
case DRM_FORMAT_YUYV:
-   writel(CTRLDESCL0_5_BPP_YCbCr422 | 
CTRLDESCL0_5_YUV_FORMAT_VY2UY1,
-  lcdif->base + LCDC_V8_CTRLDESCL0_5);
+   ctrl_desc_5 |= CTRLDESCL0_5_BPP_YCbCr422 |
+  CTRLDESCL0_5_YUV_FORMAT_VY2UY1;
in_yuv = true;
break;
case DRM_FORMAT_YVYU:
-   writel(CTRLDESCL0_5_BPP_YCbCr422 | 
CTRLDESCL0_5_YUV_FORMAT_UY2VY1,
-  lcdif->base + LCDC_V8_CTRLDESCL0_5);
+   ctrl_desc_5 |= CTRLDESCL0_5_BPP_YCbCr422 |
+  CTRLDESCL0_5_YUV_FORMAT_UY2VY1;
in_yuv = true;
break;
case DRM_FORMAT_UYVY:
-   writel(CTRLDESCL0_5_BPP_YCbCr422 | 
CTRLDESCL0_5_YUV_FORMAT_Y2VY1U,
-  lcdif->base + LCDC_V8_CTRLDESCL0_5);
+   ctrl_desc_5 |= CTRLDESCL0_5_BPP_YCbCr422 |
+  CTRLDESCL0_5_YUV_FORMAT_Y2VY1U;
in_yuv = true;
break;
case DRM_FORMAT_VYUY:
-   writel(CTRLDESCL0_5_BPP_YCbCr422 | 
CTRLDESCL0_5_YUV_FORMAT_Y2UY1V,
-  lcdif->base + LCDC_V8_CTRLDESCL0_5);
+   ctrl_desc_5 |= CTRLDESCL0_5_BPP_YCbCr422 |
+  CTRLDESCL0_5_YUV_FORMAT_Y2UY1V;
in_yuv = true;
break;
 
@@ -240,6 +238,8 @@ static void lcdif_set_formats(struct lcdif_drm_private 
*lcdif,
break;
}
 
+   writel(ctrl_desc_5, lcdif->base + LCDC_V8_CTRLDESCL0_5);
+
/*
 * The CSC differentiates between "YCbCr" and "YUV", but the reference
 * manual doesn't detail how they differ. Experiments showed that the
-- 
2.39.2

[PATCH v2 4/6] drm: lcdif: remove superfluous setup of framebuffer DMA address

[Lucas Stach]

Now that the plane state is fully programmed into the hardware before
the scanout is started there is no need to program the plane framebuffer
DMA address from the CRTC atomic_enable anymore.

Signed-off-by: Lucas Stach 
Reviewed-by: Marek Vasut 
---
v2: no changes
---
 drivers/gpu/drm/mxsfb/lcdif_kms.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c 
b/drivers/gpu/drm/mxsfb/lcdif_kms.c
index ccee5e28f236..4e9284b0d12e 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_kms.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c
@@ -536,7 +536,6 @@ static void lcdif_crtc_atomic_enable(struct drm_crtc *crtc,

crtc->primary);
struct drm_display_mode *m = >crtc.state->adjusted_mode;
struct drm_device *drm = lcdif->drm;
-   dma_addr_t paddr;
 
clk_set_rate(lcdif->clk, m->crtc_clock * 1000);
 
@@ -548,14 +547,6 @@ static void lcdif_crtc_atomic_enable(struct drm_crtc *crtc,
 
lcdif_crtc_mode_set_nofb(new_cstate, new_pstate);
 
-   /* Write cur_buf as well to avoid an initial corrupt frame */
-   paddr = drm_fb_dma_get_gem_addr(new_pstate->fb, new_pstate, 0);
-   if (paddr) {
-   writel(lower_32_bits(paddr),
-  lcdif->base + LCDC_V8_CTRLDESCL_LOW0_4);
-   writel(CTRLDESCL_HIGH0_4_ADDR_HIGH(upper_32_bits(paddr)),
-  lcdif->base + LCDC_V8_CTRLDESCL_HIGH0_4);
-   }
lcdif_enable_controller(lcdif);
 
drm_crtc_vblank_on(crtc);
-- 
2.39.2

[PATCH v2 3/6] drm: lcdif: rework runtime PM handling in the atomic commit

[Lucas Stach]

drm_atomic_helper_commit_tail_rpm makes it hard for drivers to follow
the documented encoder/bridge enable flow, as it commits all CRTC enables
before the planes are fully set up, so drivers that can't enable the
display link without valid plane setup either need to do the plane setup
in the CRTC enable or violate the flow by enabling the display link after
the planes have been set up. Neither of those options seem like a good
idea.

For devices that only do coarse-grained runtime PM for the whole display
controller and not per CRTC, like the i.MX LCDIF, we can handle hardware
wakeup and suspend in the atomic_commit_tail. Add a commit tail which
follows the more conventional atomic commit flow of first diabling any
unused CRTCs, setting up all the active plane state, then enable all
active display pipes and also handles the device runtime PM at the
appropriate times.

Signed-off-by: Lucas Stach 
---
v2: new patch
---
 drivers/gpu/drm/mxsfb/lcdif_drv.c | 22 +-
 drivers/gpu/drm/mxsfb/lcdif_kms.c | 12 ++--
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c 
b/drivers/gpu/drm/mxsfb/lcdif_drv.c
index 18de2f17e249..205f6855fb1b 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
@@ -36,8 +36,28 @@ static const struct drm_mode_config_funcs 
lcdif_mode_config_funcs = {
.atomic_commit  = drm_atomic_helper_commit,
 };
 
+void lcdif_commit_tail(struct drm_atomic_state *old_state)
+{
+   struct drm_device *drm = old_state->dev;
+
+   pm_runtime_get_sync(drm->dev);
+
+   drm_atomic_helper_commit_modeset_disables(drm, old_state);
+   drm_atomic_helper_commit_planes(drm, old_state,
+   DRM_PLANE_COMMIT_ACTIVE_ONLY);
+   drm_atomic_helper_commit_modeset_enables(drm, old_state);
+
+   drm_atomic_helper_fake_vblank(old_state);
+   drm_atomic_helper_commit_hw_done(old_state);
+   drm_atomic_helper_wait_for_vblanks(drm, old_state);
+
+   pm_runtime_put(drm->dev);
+
+   drm_atomic_helper_cleanup_planes(drm, old_state);
+}
+
 static const struct drm_mode_config_helper_funcs lcdif_mode_config_helpers = {
-   .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
+   .atomic_commit_tail = lcdif_commit_tail,
 };
 
 static const struct drm_encoder_funcs lcdif_encoder_funcs = {
diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c 
b/drivers/gpu/drm/mxsfb/lcdif_kms.c
index e277592e5fa5..ccee5e28f236 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_kms.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c
@@ -540,7 +540,11 @@ static void lcdif_crtc_atomic_enable(struct drm_crtc *crtc,
 
clk_set_rate(lcdif->clk, m->crtc_clock * 1000);
 
-   pm_runtime_get_sync(drm->dev);
+   /*
+* Update the RPM usage count, actual resume already happened in
+* lcdif_commit_tail wrapping all the atomic update.
+*/
+   pm_runtime_get_noresume(drm->dev);
 
lcdif_crtc_mode_set_nofb(new_cstate, new_pstate);
 
@@ -576,7 +580,11 @@ static void lcdif_crtc_atomic_disable(struct drm_crtc 
*crtc,
}
spin_unlock_irq(>event_lock);
 
-   pm_runtime_put_sync(drm->dev);
+   /*
+* Update the RPM usage count, actual suspend happens in
+* lcdif_commit_tail wrapping all the atomic update.
+*/
+   pm_runtime_put(drm->dev);
 }
 
 static void lcdif_crtc_atomic_destroy_state(struct drm_crtc *crtc,
-- 
2.39.2

[PATCH v2 6/6] drm: lcdif: force modeset when FB format changes

[Lucas Stach]

Force a modeset if the new FB has a different format than the
currently active one. While it might be possible to change between
compatible formats without a full modeset as the format control is
also supposed to be double buffered, the colorspace conversion is
not, so when the CSC changes we need a modeset.

For now just always force a modeset when the FB format changes to
properly reconfigure all parts of the device for the new format.

Signed-off-by: Lucas Stach 
Reviewed-by: Marek Vasut 
---
v2: fix indentation
---
 drivers/gpu/drm/mxsfb/lcdif_drv.c | 18 +-
 drivers/gpu/drm/mxsfb/lcdif_kms.c | 26 --
 2 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c 
b/drivers/gpu/drm/mxsfb/lcdif_drv.c
index 205f6855fb1b..69a2a9323257 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
@@ -30,9 +30,25 @@
 #include "lcdif_drv.h"
 #include "lcdif_regs.h"
 
+static int lcdif_atomic_check(struct drm_device *dev,
+ struct drm_atomic_state *state)
+{
+   int ret;
+
+   ret = drm_atomic_helper_check(dev, state);
+   if (ret)
+   return ret;
+
+   /*
+* Check modeset again in case crtc_state->mode_changed is
+* updated in plane's ->atomic_check callback.
+*/
+   return drm_atomic_helper_check_modeset(dev, state);
+}
+
 static const struct drm_mode_config_funcs lcdif_mode_config_funcs = {
.fb_create  = drm_gem_fb_create,
-   .atomic_check   = drm_atomic_helper_check,
+   .atomic_check   = lcdif_atomic_check,
.atomic_commit  = drm_atomic_helper_commit,
 };
 
diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c 
b/drivers/gpu/drm/mxsfb/lcdif_kms.c
index daf54ff2b7bd..34386e4b31c4 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_kms.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c
@@ -652,18 +652,32 @@ static const struct drm_crtc_funcs lcdif_crtc_funcs = {
 static int lcdif_plane_atomic_check(struct drm_plane *plane,
struct drm_atomic_state *state)
 {
-   struct drm_plane_state *plane_state = 
drm_atomic_get_new_plane_state(state,
-
plane);
+   struct drm_plane_state *new_state = 
drm_atomic_get_new_plane_state(state,
+  
plane);
+   struct drm_plane_state *old_state = 
drm_atomic_get_old_plane_state(state,
+  
plane);
struct lcdif_drm_private *lcdif = to_lcdif_drm_private(plane->dev);
struct drm_crtc_state *crtc_state;
+   int ret;
+
+   /* always okay to disable the plane */
+   if (!new_state->fb)
+   return 0;
 
crtc_state = drm_atomic_get_new_crtc_state(state,
   >crtc);
 
-   return drm_atomic_helper_check_plane_state(plane_state, crtc_state,
-  DRM_PLANE_NO_SCALING,
-  DRM_PLANE_NO_SCALING,
-  false, true);
+   ret = drm_atomic_helper_check_plane_state(new_state, crtc_state,
+ DRM_PLANE_NO_SCALING,
+ DRM_PLANE_NO_SCALING,
+ false, true);
+   if (ret)
+   return ret;
+
+   if (old_state->fb && new_state->fb->format != old_state->fb->format)
+   crtc_state->mode_changed = true;
+
+   return 0;
 }
 
 static void lcdif_plane_primary_atomic_update(struct drm_plane *plane,
-- 
2.39.2

RE: WARNING in amdgpu_sync_keep_later / dma_fence_is_later should be rate limited

[Deucher, Alexander]

[Public]

> -Original Message-
> From: Rafał Miłecki 
> Sent: Thursday, September 21, 2023 3:41 PM
> To: Deucher, Alexander ; Koenig, Christian
> ; Pan, Xinhui ; amd-
> g...@lists.freedesktop.org; dri-devel ; Yu,
> Lang 
> Subject: WARNING in amdgpu_sync_keep_later / dma_fence_is_later should
> be rate limited
>
> Hi,
>
> backporting commit 187916e6ed9d ("drm/amdgpu: install stub fence into
> potential unused fence pointers") to stable kernels resulted in lots of
> WARNINGs on some devices. In my case I was getting 3 WARNINGs per
> second (~150 lines logged every second). Commit ended up being reverted for
> stable but it exposed a potential problem. My messages log size was reaching
> gigabytes and was running my /tmp/ out of space.
>
> Could someone take a look at amdgpu_sync_keep_later / dma_fence_is_later
> and make sure its logging is rate limited to avoid such situations in the 
> future,
> please?
>
> Revert in linux-5.15.x:
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=li
> nux-5.15.y=fae2d591f3cb31f722c7f065acf586830eab8c2a
>
> openSUSE bug report:
> https://bugzilla.opensuse.org/show_bug.cgi?id=1215523

These patches were never intended for stable.  They were picked up by Sasha's 
stable autoselect tools and automatically applied to stable kernels.

Alex

[RFT PATCH v2 12/12] drm/renesas/shmobile: Call drm_helper_force_disable_all() at shutdown/remove time

[Douglas Anderson]

Based on grepping through the source code, this driver appears to be
missing a call to drm_atomic_helper_shutdown(), or in this case the
non-atomic equivalent drm_helper_force_disable_all(), at system
shutdown time and at driver remove time. This is important because
drm_helper_force_disable_all() will cause panels to get disabled
cleanly which may be important for their power sequencing. Future
changes will remove any custom powering off in individual panel
drivers so the DRM drivers need to start getting this right.

The fact that we should call drm_atomic_helper_shutdown(), or in this
case the non-atomic equivalent drm_helper_force_disable_all(), in the
case of OS shutdown/restart comes straight out of the kernel doc
"driver instance overview" in drm_drv.c.

Suggested-by: Maxime Ripard 
Cc: Geert Uytterhoeven 
Signed-off-by: Douglas Anderson 
---
This commit is only compile-time tested.

As Geert pointed out in response to v1 [1], this patch conflicts with
the patches doing atomic conversion [2]. Since those patches don't
appear to be landed yet, I'm simply reposting v1. If those patches
land, I'm more than happy to re-post this one. I'm also more than
happy if someone wants to incorporate these changes into a different
patch.

[1] 
https://lore.kernel.org/r/CAMuHMdWOB7d-KE3F7aeZvVimNuy_U30uk=PND7=twmpzcd7...@mail.gmail.com
[2] 
https://lore.kernel.org/dri-devel/fd7a6702490bd431f314d6591551bb39e77e3304.1692178020.git.geert+rene...@glider.be/

Changes in v2:
- Rebased and resolved conflicts.

 drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c 
b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c
index e5db4e0095ba..8c4c9d17a79e 100644
--- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c
+++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -179,10 +180,18 @@ static void shmob_drm_remove(struct platform_device *pdev)
 
drm_dev_unregister(ddev);
drm_kms_helper_poll_fini(ddev);
+   drm_helper_force_disable_all(ddev);
free_irq(sdev->irq, ddev);
drm_dev_put(ddev);
 }
 
+static void shmob_drm_shutdown(struct platform_device *pdev)
+{
+   struct shmob_drm_device *sdev = platform_get_drvdata(pdev);
+
+   drm_helper_force_disable_all(sdev->ddev);
+}
+
 static int shmob_drm_probe(struct platform_device *pdev)
 {
struct shmob_drm_platform_data *pdata = pdev->dev.platform_data;
@@ -287,6 +296,7 @@ static int shmob_drm_probe(struct platform_device *pdev)
 static struct platform_driver shmob_drm_platform_driver = {
.probe  = shmob_drm_probe,
.remove_new = shmob_drm_remove,
+   .shutdown   = shmob_drm_shutdown,
.driver = {
.name   = "shmob-drm",
.pm = pm_sleep_ptr(_drm_pm_ops),
-- 
2.42.0.515.g380fc7ccd1-goog

[RFT PATCH v2 10/12] drm/gma500: Call drm_helper_force_disable_all() at shutdown/remove time

[Douglas Anderson]

Based on grepping through the source code, this driver appears to be
missing a call to drm_atomic_helper_shutdown(), or in this case the
non-atomic equivalent drm_helper_force_disable_all(), at system
shutdown time and at driver remove time. This is important because
drm_helper_force_disable_all() will cause panels to get disabled
cleanly which may be important for their power sequencing. Future
changes will remove any custom powering off in individual panel
drivers so the DRM drivers need to start getting this right.

The fact that we should call drm_atomic_helper_shutdown(), or in this
case the non-atomic equivalent drm_helper_force_disable_all(), in the
case of OS shutdown/restart comes straight out of the kernel doc
"driver instance overview" in drm_drv.c.

Suggested-by: Maxime Ripard 
Reviewed-by: Maxime Ripard 
Signed-off-by: Douglas Anderson 
---
This commit is only compile-time tested.

(no changes since v1)

 drivers/gpu/drm/gma500/psb_drv.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 8b64f61ffaf9..a5a399bbe8f5 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -20,6 +20,7 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -485,6 +486,12 @@ static void psb_pci_remove(struct pci_dev *pdev)
struct drm_device *dev = pci_get_drvdata(pdev);
 
drm_dev_unregister(dev);
+   drm_helper_force_disable_all(dev);
+}
+
+static void psb_pci_shutdown(struct pci_dev *pdev)
+{
+   drm_helper_force_disable_all(pci_get_drvdata(pdev));
 }
 
 static DEFINE_RUNTIME_DEV_PM_OPS(psb_pm_ops, gma_power_suspend, 
gma_power_resume, NULL);
@@ -521,6 +528,7 @@ static struct pci_driver psb_pci_driver = {
.id_table = pciidlist,
.probe = psb_pci_probe,
.remove = psb_pci_remove,
+   .shutdown = psb_pci_shutdown,
.driver.pm = _pm_ops,
 };
 
-- 
2.42.0.515.g380fc7ccd1-goog

[RFT PATCH v2 09/12] drm/exynos: Call drm_atomic_helper_shutdown() at shutdown/unbind time

[Douglas Anderson]

Based on grepping through the source code this driver appears to be
missing a call to drm_atomic_helper_shutdown() at system shutdown time
and at driver unbind time. Among other things, this means that if a
panel is in use that it won't be cleanly powered off at system
shutdown time.

The fact that we should call drm_atomic_helper_shutdown() in the case
of OS shutdown/restart and at driver remove (or unbind) time comes
straight out of the kernel doc "driver instance overview" in
drm_drv.c.

A few notes about this fix:
- When adding drm_atomic_helper_shutdown() to the unbind path, I added
  it after drm_kms_helper_poll_fini() since that's when other drivers
  seemed to have it.
- Technically with a previous patch, ("drm/atomic-helper:
  drm_atomic_helper_shutdown(NULL) should be a noop"), we don't
  actually need to check to see if our "drm" pointer is NULL before
  calling drm_atomic_helper_shutdown(). We'll leave the "if" test in,
  though, so that this patch can land without any dependencies. It
  could potentially be removed later.
- This patch also makes sure to set the drvdata to NULL in the case of
  bind errors to make sure that shutdown can't access freed data.

Suggested-by: Maxime Ripard 
Reviewed-by: Maxime Ripard 
Signed-off-by: Douglas Anderson 
---
This commit is only compile-time tested.

(no changes since v1)

 drivers/gpu/drm/exynos/exynos_drm_drv.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c 
b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 8399256cb5c9..5380fb6c55ae 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -300,6 +300,7 @@ static int exynos_drm_bind(struct device *dev)
drm_mode_config_cleanup(drm);
exynos_drm_cleanup_dma(drm);
kfree(private);
+   dev_set_drvdata(dev, NULL);
 err_free_drm:
drm_dev_put(drm);
 
@@ -313,6 +314,7 @@ static void exynos_drm_unbind(struct device *dev)
drm_dev_unregister(drm);
 
drm_kms_helper_poll_fini(drm);
+   drm_atomic_helper_shutdown(drm);
 
component_unbind_all(drm->dev, drm);
drm_mode_config_cleanup(drm);
@@ -350,9 +352,18 @@ static int exynos_drm_platform_remove(struct 
platform_device *pdev)
return 0;
 }
 
+static void exynos_drm_platform_shutdown(struct platform_device *pdev)
+{
+   struct drm_device *drm = platform_get_drvdata(pdev);
+
+   if (drm)
+   drm_atomic_helper_shutdown(drm);
+}
+
 static struct platform_driver exynos_drm_platform_driver = {
.probe  = exynos_drm_platform_probe,
.remove = exynos_drm_platform_remove,
+   .shutdown = exynos_drm_platform_shutdown,
.driver = {
.name   = "exynos-drm",
.pm = _drm_pm_ops,
-- 
2.42.0.515.g380fc7ccd1-goog

[RFT PATCH v2 11/12] drm/radeon: Call drm_helper_force_disable_all() at shutdown/remove time

[Douglas Anderson]

Based on grepping through the source code, this driver appears to be
missing a call to drm_atomic_helper_shutdown(), or in this case the
non-atomic equivalent drm_helper_force_disable_all(), at system
shutdown time and at driver remove time. This is important because
drm_helper_force_disable_all() will cause panels to get disabled
cleanly which may be important for their power sequencing. Future
changes will remove any custom powering off in individual panel
drivers so the DRM drivers need to start getting this right.

The fact that we should call drm_atomic_helper_shutdown(), or in this
case the non-atomic equivalent drm_helper_force_disable_all(), in the
case of OS shutdown/restart comes straight out of the kernel doc
"driver instance overview" in drm_drv.c.

NOTE: in order to get things inserted in the right place, I had to
replace the old/deprecated drm_put_dev() function with the equivalent
new calls.

Suggested-by: Maxime Ripard 
Reviewed-by: Maxime Ripard 
Signed-off-by: Douglas Anderson 
---
I honestly have no idea if I got this patch right. The shutdown()
function already had some special case logic for PPC, Loongson, and
VMs and I don't 100% for sure know how this interacts with
those. Everything here is just compile tested.

(no changes since v1)

 drivers/gpu/drm/radeon/radeon_drv.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
b/drivers/gpu/drm/radeon/radeon_drv.c
index 39cdede460b5..67995ea24852 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -38,6 +38,7 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -357,7 +358,9 @@ radeon_pci_remove(struct pci_dev *pdev)
 {
struct drm_device *dev = pci_get_drvdata(pdev);
 
-   drm_put_dev(dev);
+   drm_dev_unregister(dev);
+   drm_helper_force_disable_all(dev);
+   drm_dev_put(dev);
 }
 
 static void
@@ -368,6 +371,8 @@ radeon_pci_shutdown(struct pci_dev *pdev)
 */
if (radeon_device_is_virtual())
radeon_pci_remove(pdev);
+   else
+   drm_helper_force_disable_all(pci_get_drvdata(pdev));
 
 #if defined(CONFIG_PPC64) || defined(CONFIG_MACH_LOONGSON64)
/*
-- 
2.42.0.515.g380fc7ccd1-goog

[RFT PATCH v2 07/12] drm/amdgpu: Call drm_atomic_helper_shutdown() at shutdown time

[Douglas Anderson]

Based on grepping through the source code this driver appears to be
missing a call to drm_atomic_helper_shutdown() at system shutdown
time. Among other things, this means that if a panel is in use that it
won't be cleanly powered off at system shutdown time.

The fact that we should call drm_atomic_helper_shutdown() in the case
of OS shutdown/restart comes straight out of the kernel doc "driver
instance overview" in drm_drv.c.

Suggested-by: Maxime Ripard 
Signed-off-by: Douglas Anderson 
---
This commit is only compile-time tested.

...and further, I'd say that this patch is more of a plea for help
than a patch I think is actually right. I'm _fairly_ certain that
drm/amdgpu needs this call at shutdown time but the logic is a bit
hard for me to follow. I'd appreciate if anyone who actually knows
what this should look like could illuminate me, or perhaps even just
post a patch themselves!

(no changes since v1)

 drivers/gpu/drm/amd/amdgpu/amdgpu.h|  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c|  2 ++
 3 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 8f2255b3a38a..cfcff0b37466 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1104,6 +1104,7 @@ static inline struct amdgpu_device 
*amdgpu_ttm_adev(struct ttm_device *bdev)
 int amdgpu_device_init(struct amdgpu_device *adev,
   uint32_t flags);
 void amdgpu_device_fini_hw(struct amdgpu_device *adev);
+void amdgpu_device_shutdown_hw(struct amdgpu_device *adev);
 void amdgpu_device_fini_sw(struct amdgpu_device *adev);
 
 int amdgpu_gpu_wait_for_idle(struct amdgpu_device *adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index a2cdde0ca0a7..fa5925c2092d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4247,6 +4247,16 @@ void amdgpu_device_fini_hw(struct amdgpu_device *adev)
 
 }
 
+void amdgpu_device_shutdown_hw(struct amdgpu_device *adev)
+{
+   if (adev->mode_info.mode_config_initialized) {
+   if (!drm_drv_uses_atomic_modeset(adev_to_drm(adev)))
+   drm_helper_force_disable_all(adev_to_drm(adev));
+   else
+   drm_atomic_helper_shutdown(adev_to_drm(adev));
+   }
+}
+
 void amdgpu_device_fini_sw(struct amdgpu_device *adev)
 {
int idx;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index e90f730eb715..3a7cbff111d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2333,6 +2333,8 @@ amdgpu_pci_shutdown(struct pci_dev *pdev)
struct drm_device *dev = pci_get_drvdata(pdev);
struct amdgpu_device *adev = drm_to_adev(dev);
 
+   amdgpu_device_shutdown_hw(adev);
+
if (amdgpu_ras_intr_triggered())
return;
 
-- 
2.42.0.515.g380fc7ccd1-goog

[RFT PATCH v2 08/12] drm/sprd: Call drm_atomic_helper_shutdown() at remove time

[Douglas Anderson]

Based on grepping through the source code, this driver appears to be
missing a call to drm_atomic_helper_shutdown() at remove time. Let's
add it.

The fact that we should call drm_atomic_helper_shutdown() in the case
of OS driver remove comes straight out of the kernel doc "driver
instance overview" in drm_drv.c.

While at it, let's also fix it so that if the driver's bind fails or
if a driver gets unbound that the drvdata gets set to NULL. This will
make sure we can't get confused during a later shutdown().

Suggested-by: Maxime Ripard 
Reviewed-by: Maxime Ripard 
Signed-off-by: Douglas Anderson 
---
This commit is only compile-time tested.

(no changes since v1)

 drivers/gpu/drm/sprd/sprd_drm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sprd/sprd_drm.c b/drivers/gpu/drm/sprd/sprd_drm.c
index 0aa39156f2fa..86a175116140 100644
--- a/drivers/gpu/drm/sprd/sprd_drm.c
+++ b/drivers/gpu/drm/sprd/sprd_drm.c
@@ -114,6 +114,7 @@ static int sprd_drm_bind(struct device *dev)
drm_kms_helper_poll_fini(drm);
 err_unbind_all:
component_unbind_all(drm->dev, drm);
+   platform_set_drvdata(pdev, NULL);
return ret;
 }
 
@@ -122,10 +123,11 @@ static void sprd_drm_unbind(struct device *dev)
struct drm_device *drm = dev_get_drvdata(dev);
 
drm_dev_unregister(drm);
-
drm_kms_helper_poll_fini(drm);
+   drm_atomic_helper_shutdown(drm);
 
component_unbind_all(drm->dev, drm);
+   dev_set_drvdata(dev, NULL);
 }
 
 static const struct component_master_ops drm_component_ops = {
-- 
2.42.0.515.g380fc7ccd1-goog

[RFT PATCH v2 06/12] drm/arcpgu: Call drm_atomic_helper_shutdown() at shutdown time

[Douglas Anderson]

Based on grepping through the source code this driver appears to be
missing a call to drm_atomic_helper_shutdown() at system shutdown
time. Among other things, this means that if a panel is in use that it
won't be cleanly powered off at system shutdown time.

The fact that we should call drm_atomic_helper_shutdown() in the case
of OS shutdown/restart comes straight out of the kernel doc "driver
instance overview" in drm_drv.c.

Suggested-by: Maxime Ripard 
Reviewed-by: Maxime Ripard 
Signed-off-by: Douglas Anderson 
---
This commit is only compile-time tested.

(no changes since v1)

 drivers/gpu/drm/tiny/arcpgu.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/tiny/arcpgu.c b/drivers/gpu/drm/tiny/arcpgu.c
index e5b10e41554a..c1e851c982e4 100644
--- a/drivers/gpu/drm/tiny/arcpgu.c
+++ b/drivers/gpu/drm/tiny/arcpgu.c
@@ -414,6 +414,11 @@ static int arcpgu_remove(struct platform_device *pdev)
return 0;
 }
 
+static void arcpgu_shutdown(struct platform_device *pdev)
+{
+   drm_atomic_helper_shutdown(platform_get_drvdata(pdev));
+}
+
 static const struct of_device_id arcpgu_of_table[] = {
{.compatible = "snps,arcpgu"},
{}
@@ -424,6 +429,7 @@ MODULE_DEVICE_TABLE(of, arcpgu_of_table);
 static struct platform_driver arcpgu_platform_driver = {
.probe = arcpgu_probe,
.remove = arcpgu_remove,
+   .shutdown = arcpgu_shutdown,
.driver = {
   .name = "arcpgu",
   .of_match_table = arcpgu_of_table,
-- 
2.42.0.515.g380fc7ccd1-goog

[RFT PATCH v2 05/12] drm/tegra: Call drm_atomic_helper_shutdown() at shutdown time

[Douglas Anderson]

Based on grepping through the source code this driver appears to be
missing a call to drm_atomic_helper_shutdown() at system shutdown
time. Among other things, this means that if a panel is in use that it
won't be cleanly powered off at system shutdown time.

The fact that we should call drm_atomic_helper_shutdown() in the case
of OS shutdown/restart comes straight out of the kernel doc "driver
instance overview" in drm_drv.c.

Suggested-by: Maxime Ripard 
Reviewed-by: Maxime Ripard 
Signed-off-by: Douglas Anderson 
---
This commit is only compile-time tested.

(no changes since v1)

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

diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index ff36171c8fb7..ce2d4153f7bd 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -1312,6 +1312,11 @@ static int host1x_drm_remove(struct host1x_device *dev)
return 0;
 }
 
+static void host1x_drm_shutdown(struct host1x_device *dev)
+{
+   drm_atomic_helper_shutdown(dev_get_drvdata(>dev));
+}
+
 #ifdef CONFIG_PM_SLEEP
 static int host1x_drm_suspend(struct device *dev)
 {
@@ -1380,6 +1385,7 @@ static struct host1x_driver host1x_drm_driver = {
},
.probe = host1x_drm_probe,
.remove = host1x_drm_remove,
+   .shutdown = host1x_drm_shutdown,
.subdevs = host1x_drm_subdevs,
 };
 
-- 
2.42.0.515.g380fc7ccd1-goog

[RFT PATCH v2 03/12] drm/mediatek: Call drm_atomic_helper_shutdown() at shutdown time

[Douglas Anderson]

Based on grepping through the source code this driver appears to be
missing a call to drm_atomic_helper_shutdown() at system shutdown
time. Among other things, this means that if a panel is in use that it
won't be cleanly powered off at system shutdown time.

The fact that we should call drm_atomic_helper_shutdown() in the case
of OS shutdown/restart comes straight out of the kernel doc "driver
instance overview" in drm_drv.c.

This driver users the component model and shutdown happens in the base
driver. The "drvdata" for this driver will always be valid if
shutdown() is called and we know that if the "drm" pointer in our
private data is non-NULL then we need to call
drm_atomic_helper_shutdown(). Technically with a previous patch,
("drm/atomic-helper: drm_atomic_helper_shutdown(NULL) should be a
noop"), we don't actually need to check to see if our "drm" pointer is
NULL before calling drm_atomic_helper_shutdown(). We'll leave the "if"
test in, though, so that this patch can land without any
dependencies. It could potentially be removed later.

Suggested-by: Maxime Ripard 
Reviewed-by: Maxime Ripard 
Reviewed-by: Fei Shao 
Tested-by: Fei Shao 
Signed-off-by: Douglas Anderson 
---

Changes in v2:
- Rebased and resolved conflicts.

 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c 
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index d16cc8219105..6bab360c0c1a 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -919,6 +919,14 @@ static void mtk_drm_remove(struct platform_device *pdev)
of_node_put(private->comp_node[i]);
 }
 
+static void mtk_drm_shutdown(struct platform_device *pdev)
+{
+   struct mtk_drm_private *private = platform_get_drvdata(pdev);
+
+   if (private->drm)
+   drm_atomic_helper_shutdown(private->drm);
+}
+
 static int mtk_drm_sys_prepare(struct device *dev)
 {
struct mtk_drm_private *private = dev_get_drvdata(dev);
@@ -950,6 +958,7 @@ static const struct dev_pm_ops mtk_drm_pm_ops = {
 static struct platform_driver mtk_drm_platform_driver = {
.probe  = mtk_drm_probe,
.remove_new = mtk_drm_remove,
+   .shutdown = mtk_drm_shutdown,
.driver = {
.name   = "mediatek-drm",
.pm = _drm_pm_ops,
-- 
2.42.0.515.g380fc7ccd1-goog

[RFT PATCH v2 04/12] drm/nouveau: Call drm_atomic_helper_shutdown() or equiv at shutdown time

[Douglas Anderson]

Based on grepping through the source code this driver appears to be
missing a call to drm_atomic_helper_shutdown() (or
drm_helper_force_disable_all() if not using atomic) at system shutdown
time. Among other things, this means that if a panel is in use that it
won't be cleanly powered off at system shutdown time.

The fact that we should call drm_atomic_helper_shutdown() in the case
of OS shutdown/restart comes straight out of the kernel doc "driver
instance overview" in drm_drv.c.

Suggested-by: Maxime Ripard 
Reviewed-by: Maxime Ripard 
Signed-off-by: Douglas Anderson 
---
This commit is only compile-time tested. I made my best guess about
how to fit this into the existing code. If someone wishes a different
style, please yell.

(no changes since v1)

 drivers/gpu/drm/nouveau/nouveau_display.c  |  9 +
 drivers/gpu/drm/nouveau/nouveau_display.h  |  1 +
 drivers/gpu/drm/nouveau/nouveau_drm.c  | 13 +
 drivers/gpu/drm/nouveau/nouveau_drv.h  |  1 +
 drivers/gpu/drm/nouveau/nouveau_platform.c |  6 ++
 5 files changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c 
b/drivers/gpu/drm/nouveau/nouveau_display.c
index d8c92521226d..05c3688ccb76 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -642,6 +642,15 @@ nouveau_display_fini(struct drm_device *dev, bool suspend, 
bool runtime)
disp->fini(dev, runtime, suspend);
 }
 
+void
+nouveau_display_shutdown(struct drm_device *dev)
+{
+   if (drm_drv_uses_atomic_modeset(dev))
+   drm_atomic_helper_shutdown(dev);
+   else
+   drm_helper_force_disable_all(dev);
+}
+
 static void
 nouveau_display_create_properties(struct drm_device *dev)
 {
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h 
b/drivers/gpu/drm/nouveau/nouveau_display.h
index 2ab2ddb1eadf..9df62e833cda 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.h
+++ b/drivers/gpu/drm/nouveau/nouveau_display.h
@@ -47,6 +47,7 @@ void nouveau_display_destroy(struct drm_device *dev);
 int  nouveau_display_init(struct drm_device *dev, bool resume, bool runtime);
 void nouveau_display_hpd_resume(struct drm_device *dev);
 void nouveau_display_fini(struct drm_device *dev, bool suspend, bool runtime);
+void nouveau_display_shutdown(struct drm_device *dev);
 int  nouveau_display_suspend(struct drm_device *dev, bool runtime);
 void nouveau_display_resume(struct drm_device *dev, bool runtime);
 int  nouveau_display_vblank_enable(struct drm_crtc *crtc);
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c 
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 50589f982d1a..8ecfd66b7aab 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -879,6 +879,18 @@ nouveau_drm_remove(struct pci_dev *pdev)
pci_disable_device(pdev);
 }
 
+void
+nouveau_drm_device_shutdown(struct drm_device *dev)
+{
+   nouveau_display_shutdown(dev);
+}
+
+static void
+nouveau_drm_shutdown(struct pci_dev *pdev)
+{
+   nouveau_drm_device_shutdown(pci_get_drvdata(pdev));
+}
+
 static int
 nouveau_do_suspend(struct drm_device *dev, bool runtime)
 {
@@ -1346,6 +1358,7 @@ nouveau_drm_pci_driver = {
.id_table = nouveau_drm_pci_table,
.probe = nouveau_drm_probe,
.remove = nouveau_drm_remove,
+   .shutdown = nouveau_drm_shutdown,
.driver.pm = _pm_ops,
 };
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h 
b/drivers/gpu/drm/nouveau/nouveau_drv.h
index 3666a7403e47..aa936cabb6cf 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -327,6 +327,7 @@ struct drm_device *
 nouveau_platform_device_create(const struct nvkm_device_tegra_func *,
   struct platform_device *, struct nvkm_device **);
 void nouveau_drm_device_remove(struct drm_device *dev);
+void nouveau_drm_device_shutdown(struct drm_device *dev);
 
 #define NV_PRINTK(l,c,f,a...) do { 
\
struct nouveau_cli *_cli = (c);\
diff --git a/drivers/gpu/drm/nouveau/nouveau_platform.c 
b/drivers/gpu/drm/nouveau/nouveau_platform.c
index 23cd43a7fd19..b2e82a96411c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_platform.c
+++ b/drivers/gpu/drm/nouveau/nouveau_platform.c
@@ -50,6 +50,11 @@ static int nouveau_platform_remove(struct platform_device 
*pdev)
return 0;
 }
 
+static void nouveau_platform_shutdown(struct platform_device *pdev)
+{
+   nouveau_drm_device_shutdown(platform_get_drvdata(pdev));
+}
+
 #if IS_ENABLED(CONFIG_OF)
 static const struct nvkm_device_tegra_func gk20a_platform_data = {
.iommu_bit = 34,
@@ -94,4 +99,5 @@ struct platform_driver nouveau_platform_driver = {
},
.probe = nouveau_platform_probe,
.remove = nouveau_platform_remove,
+   .shutdown = nouveau_platform_shutdown,
 };
-- 
2.42.0.515.g380fc7ccd1-goog

[RFT PATCH v2 02/12] drm/kmb: Call drm_atomic_helper_shutdown() at shutdown time

[Douglas Anderson]

Based on grepping through the source code this driver appears to be
missing a call to drm_atomic_helper_shutdown() at system shutdown
time. Among other things, this means that if a panel is in use that it
won't be cleanly powered off at system shutdown time.

The fact that we should call drm_atomic_helper_shutdown() in the case
of OS shutdown/restart comes straight out of the kernel doc "driver
instance overview" in drm_drv.c.

Suggested-by: Maxime Ripard 
Reviewed-by: Maxime Ripard 
Signed-off-by: Douglas Anderson 
---
This commit is only compile-time tested.

(no changes since v1)

 drivers/gpu/drm/kmb/kmb_drv.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c
index 24035b53441c..af9bd34fefc0 100644
--- a/drivers/gpu/drm/kmb/kmb_drv.c
+++ b/drivers/gpu/drm/kmb/kmb_drv.c
@@ -476,6 +476,11 @@ static int kmb_remove(struct platform_device *pdev)
return 0;
 }
 
+static void kmb_shutdown(struct platform_device *pdev)
+{
+   drm_atomic_helper_shutdown(platform_get_drvdata(pdev));
+}
+
 static int kmb_probe(struct platform_device *pdev)
 {
struct device *dev = get_device(>dev);
@@ -622,6 +627,7 @@ static SIMPLE_DEV_PM_OPS(kmb_pm_ops, kmb_pm_suspend, 
kmb_pm_resume);
 static struct platform_driver kmb_platform_driver = {
.probe = kmb_probe,
.remove = kmb_remove,
+   .shutdown = kmb_shutdown,
.driver = {
.name = "kmb-drm",
.pm = _pm_ops,
-- 
2.42.0.515.g380fc7ccd1-goog

[RFT PATCH v2 01/12] drm/imx/dcss: Call drm_atomic_helper_shutdown() at shutdown time

[Douglas Anderson]

Based on grepping through the source code this driver appears to be
missing a call to drm_atomic_helper_shutdown() at system shutdown
time. Among other things, this means that if a panel is in use that it
won't be cleanly powered off at system shutdown time.

The fact that we should call drm_atomic_helper_shutdown() in the case
of OS shutdown/restart comes straight out of the kernel doc "driver
instance overview" in drm_drv.c.

Suggested-by: Maxime Ripard 
Reviewed-by: Maxime Ripard 
Signed-off-by: Douglas Anderson 
---
This commit is only compile-time tested.

(no changes since v1)

 drivers/gpu/drm/imx/dcss/dcss-drv.c | 8 
 drivers/gpu/drm/imx/dcss/dcss-kms.c | 7 +++
 drivers/gpu/drm/imx/dcss/dcss-kms.h | 1 +
 3 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/imx/dcss/dcss-drv.c 
b/drivers/gpu/drm/imx/dcss/dcss-drv.c
index c68b0d93ae9e..b61cec0cc79d 100644
--- a/drivers/gpu/drm/imx/dcss/dcss-drv.c
+++ b/drivers/gpu/drm/imx/dcss/dcss-drv.c
@@ -92,6 +92,13 @@ static int dcss_drv_platform_remove(struct platform_device 
*pdev)
return 0;
 }
 
+static void dcss_drv_platform_shutdown(struct platform_device *pdev)
+{
+   struct dcss_drv *mdrv = dev_get_drvdata(>dev);
+
+   dcss_kms_shutdown(mdrv->kms);
+}
+
 static struct dcss_type_data dcss_types[] = {
[DCSS_IMX8MQ] = {
.name = "DCSS_IMX8MQ",
@@ -114,6 +121,7 @@ MODULE_DEVICE_TABLE(of, dcss_of_match);
 static struct platform_driver dcss_platform_driver = {
.probe  = dcss_drv_platform_probe,
.remove = dcss_drv_platform_remove,
+   .shutdown = dcss_drv_platform_shutdown,
.driver = {
.name = "imx-dcss",
.of_match_table = dcss_of_match,
diff --git a/drivers/gpu/drm/imx/dcss/dcss-kms.c 
b/drivers/gpu/drm/imx/dcss/dcss-kms.c
index 896de946f8df..d0ea4e97cded 100644
--- a/drivers/gpu/drm/imx/dcss/dcss-kms.c
+++ b/drivers/gpu/drm/imx/dcss/dcss-kms.c
@@ -172,3 +172,10 @@ void dcss_kms_detach(struct dcss_kms_dev *kms)
dcss_crtc_deinit(>crtc, drm);
drm->dev_private = NULL;
 }
+
+void dcss_kms_shutdown(struct dcss_kms_dev *kms)
+{
+   struct drm_device *drm = >base;
+
+   drm_atomic_helper_shutdown(drm);
+}
diff --git a/drivers/gpu/drm/imx/dcss/dcss-kms.h 
b/drivers/gpu/drm/imx/dcss/dcss-kms.h
index dfe5dd99eea3..62521c1fd6d2 100644
--- a/drivers/gpu/drm/imx/dcss/dcss-kms.h
+++ b/drivers/gpu/drm/imx/dcss/dcss-kms.h
@@ -34,6 +34,7 @@ struct dcss_kms_dev {
 
 struct dcss_kms_dev *dcss_kms_attach(struct dcss_dev *dcss);
 void dcss_kms_detach(struct dcss_kms_dev *kms);
+void dcss_kms_shutdown(struct dcss_kms_dev *kms);
 int dcss_crtc_init(struct dcss_crtc *crtc, struct drm_device *drm);
 void dcss_crtc_deinit(struct dcss_crtc *crtc, struct drm_device *drm);
 struct dcss_plane *dcss_plane_init(struct drm_device *drm,
-- 
2.42.0.515.g380fc7ccd1-goog

[RFT PATCH v2 00/12] drm: call drm_atomic_helper_shutdown() at the right times

[Douglas Anderson]

This patch series came about after a _long_ discussion between me and
Maxime Ripard in response to a different patch I sent out [1]. As part
of that discussion, we realized that it would be good if DRM drivers
consistently called drm_atomic_helper_shutdown() properly at shutdown
and driver remove time as it's documented that they should do. The
eventual goal of this would be to enable removing some hacky code from
panel drivers where they had to hook into shutdown themselves because
the DRM driver wasn't calling them.

It turns out that quite a lot of drivers seemed to be missing
drm_atomic_helper_shutdown() in one or both places that it was
supposed to be. This patch series attempts to fix all the drivers that
I was able to identify.

NOTE: fixing this wasn't exactly cookie cutter. Each driver has its
own unique way of setting itself up and tearing itself down. Some
drivers also use the component model, which adds extra fun. I've made
my best guess at solving this and I've run a bunch of compile tests
(specifically, allmodconfig for amd64, arm64, and powerpc). That being
said, these code changes are not totally trivial and I've done zero
real testing on them. Making these patches was also a little mind
numbing and I'm certain my eyes glazed over at several points when
writing them. What I'm trying to say is to please double-check that I
didn't do anything too silly, like cast your driver's drvdata to the
wrong type. Even better, test these patches!

I've labeled this patch series as RFT (request for testing) to help
call attention to the fact that I didn't personally test any of these
patches.

I'd like to call out a few drivers that I _didn't_ fix in this series
and why. If any of these drivers should be fixed then please yell.
- DRM drivers backed by usb_driver (like gud, gm12u320, udl): I didn't
  add the call to drm_atomic_helper_shutdown() at shutdown time
  because there's no ".shutdown" callback for them USB drivers. Given
  that USB is hotpluggable, I'm assuming that they are robust against
  this and the special shutdown callback isn't needed.
- ofdrm and simpledrm: These didn't have drm_atomic_helper_shutdown()
  in either shutdown or remove, but I didn't add it. I think that's OK
  since they're sorta special and not really directly controlling
  hardware power sequencing.
- virtio, vkms, vmwgfx, xen: I believe these are all virtual (thus
  they wouldn't directly drive a panel) and adding the shutdown
  didn't look straightforward, so I skipped them.

I've let each patch in the series get CCed straight from
get_maintainer. That means not everyone will have received every patch
but everyone should be on the cover letter. I know some people dislike
this but when touching this many drivers there's not much
choice. dri-devel and lkml have been CCed and lore/lei exist, so
hopefully that's enough for folks. I'm happy to add people to the
whole series for future posts.

NOTE: I landed everything I could from v1 of the patch series [2] [3]
to drm-misc. This v2 is everyone that is still left. If you'd like me
to land one of the patches here to drm-misc for you, please say
so. Otherwise I will assume maintainers will pick patches for their
particular driver and land them. There are no dependencies.

[1] 
https://lore.kernel.org/lkml/20230804140605.RFC.4.I930069a32baab6faf46d6b234f89613b5cec0f14@changeid
[2] https://lore.kernel.org/r/20230901234015.566018-1-diand...@chromium.org
[3] https://lore.kernel.org/r/20230901234202.566951-1-diand...@chromium.org

Changes in v2:
- Rebased and resolved conflicts.

Douglas Anderson (12):
  drm/imx/dcss: Call drm_atomic_helper_shutdown() at shutdown time
  drm/kmb: Call drm_atomic_helper_shutdown() at shutdown time
  drm/mediatek: Call drm_atomic_helper_shutdown() at shutdown time
  drm/nouveau: Call drm_atomic_helper_shutdown() or equiv at shutdown
time
  drm/tegra: Call drm_atomic_helper_shutdown() at shutdown time
  drm/arcpgu: Call drm_atomic_helper_shutdown() at shutdown time
  drm/amdgpu: Call drm_atomic_helper_shutdown() at shutdown time
  drm/sprd: Call drm_atomic_helper_shutdown() at remove time
  drm/exynos: Call drm_atomic_helper_shutdown() at shutdown/unbind time
  drm/gma500: Call drm_helper_force_disable_all() at shutdown/remove
time
  drm/radeon: Call drm_helper_force_disable_all() at shutdown/remove
time
  drm/renesas/shmobile: Call drm_helper_force_disable_all() at
shutdown/remove time

 drivers/gpu/drm/amd/amdgpu/amdgpu.h  |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c   | 10 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c  |  2 ++
 drivers/gpu/drm/exynos/exynos_drm_drv.c  | 11 +++
 drivers/gpu/drm/gma500/psb_drv.c |  8 
 drivers/gpu/drm/imx/dcss/dcss-drv.c  |  8 
 drivers/gpu/drm/imx/dcss/dcss-kms.c  |  7 +++
 drivers/gpu/drm/imx/dcss/dcss-kms.h  |  1 +
 drivers/gpu/drm/kmb/kmb_drv.c|  6 

Re: [RFT PATCH 6/6] drm/hisilicon/kirin: Call drm_atomic_helper_shutdown() at shutdown/unbind time

[Doug Anderson]

Hi,

On Fri, Sep 1, 2023 at 4:41 PM Douglas Anderson  wrote:
>
> Based on grepping through the source code this driver appears to be
> missing a call to drm_atomic_helper_shutdown() at system shutdown time
> and at driver unbind time. Among other things, this means that if a
> panel is in use that it won't be cleanly powered off at system
> shutdown time.
>
> The fact that we should call drm_atomic_helper_shutdown() in the case
> of OS shutdown/restart and at driver remove (or unbind) time comes
> straight out of the kernel doc "driver instance overview" in
> drm_drv.c.
>
> I have attempted to put this in the right place at unbind time. In
> most other DRM drivers the call is made right after the call to
> drm_kms_helper_poll_fini(), so I've put it there. That means that this
> call will also be made in the case that we hit errors in bind, since
> kirin_drm_kms_cleanup() is called both in the bind error path and in
> unbind. I believe this is harmless even though it's not needed in the
> bind error path.
>
> For handling shutdown, we rely on the common technique of seeing if
> the drvdata is NULL to know whether we need to call
> drm_atomic_helper_shutdown(). This makes it important to make sure
> that the drvdata is NULL if bind failed or if unbind was called. We
> don't need the actual check for NULL and we'll rely on the patch
> ("drm/atomic-helper: drm_atomic_helper_shutdown(NULL) should be a
> noop").
>
> Suggested-by: Maxime Ripard 
> Signed-off-by: Douglas Anderson 
> ---
> This commit is only compile-time tested.
>
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 9 +
>  1 file changed, 9 insertions(+)

Landed in drm-misc-next:

918ce0906dcd drm/hisilicon/kirin: Call drm_atomic_helper_shutdown() at
shutdown/unbind time

Re: [RFT PATCH 5/6] drm: Call drm_atomic_helper_shutdown() at shutdown/remove time for misc drivers

[Doug Anderson]

Hi,

On Fri, Sep 1, 2023 at 4:41 PM Douglas Anderson  wrote:
>
> Based on grepping through the source code these drivers appear to be
> missing a call to drm_atomic_helper_shutdown() at system shutdown time
> and at driver remove (or unbind) time. Among other things, this means
> that if a panel is in use that it won't be cleanly powered off at
> system shutdown time.
>
> The fact that we should call drm_atomic_helper_shutdown() in the case
> of OS shutdown/restart and at driver remove (or unbind) time comes
> straight out of the kernel doc "driver instance overview" in
> drm_drv.c.
>
> A few notes about these fixes:
> - I confirmed that these drivers were all DRIVER_MODESET type drivers,
>   which I believe makes this relevant.
> - I confirmed that these drivers were all DRIVER_ATOMIC.
> - When adding drm_atomic_helper_shutdown() to the remove/unbind path,
>   I added it after drm_kms_helper_poll_fini() when the driver had
>   it. This seemed to be what other drivers did. If
>   drm_kms_helper_poll_fini() wasn't there I added it straight after
>   drm_dev_unregister().
> - This patch deals with drivers using the component model in similar
>   ways as the patch ("drm: Call drm_atomic_helper_shutdown() at
>   shutdown time for misc drivers")
> - These fixes rely on the patch ("drm/atomic-helper:
>   drm_atomic_helper_shutdown(NULL) should be a noop") to simplify
>   shutdown.
>
> Suggested-by: Maxime Ripard 
> Signed-off-by: Douglas Anderson 
> ---
>
>  drivers/gpu/drm/aspeed/aspeed_gfx_drv.c |  7 +++
>  drivers/gpu/drm/mgag200/mgag200_drv.c   |  8 
>  drivers/gpu/drm/pl111/pl111_drv.c   |  7 +++
>  drivers/gpu/drm/stm/drv.c   |  7 +++
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c | 11 ++-
>  drivers/gpu/drm/tve200/tve200_drv.c |  7 +++
>  drivers/gpu/drm/vboxvideo/vbox_drv.c| 10 ++
>  7 files changed, 56 insertions(+), 1 deletion(-)

Landed on drm-misc-next:

3c4babae3c4a drm: Call drm_atomic_helper_shutdown() at shutdown/remove
time for misc drivers

Re: [RFT PATCH 2/6] drm: Call drm_atomic_helper_shutdown() at shutdown time for misc drivers

[Doug Anderson]

Hi,

On Fri, Sep 1, 2023 at 4:40 PM Douglas Anderson  wrote:
>
> Based on grepping through the source code these drivers appear to be
> missing a call to drm_atomic_helper_shutdown() at system shutdown
> time. Among other things, this means that if a panel is in use that it
> won't be cleanly powered off at system shutdown time.
>
> The fact that we should call drm_atomic_helper_shutdown() in the case
> of OS shutdown/restart comes straight out of the kernel doc "driver
> instance overview" in drm_drv.c.
>
> All of the drivers in this patch were fairly straightforward to fix
> since they already had a call to drm_atomic_helper_shutdown() at
> remove/unbind time but were just lacking one at system shutdown. The
> only hitch is that some of these drivers use the component model to
> register/unregister their DRM devices. The shutdown callback is part
> of the original device. The typical solution here, based on how other
> DRM drivers do this, is to keep track of whether the device is bound
> based on drvdata. In most cases the drvdata is the drm_device, so we
> can just make sure it is NULL when the device is not bound. In some
> drivers, this required minor code changes. To make things simpler,
> drm_atomic_helper_shutdown() has been modified to consider a NULL
> drm_device as a noop in the patch ("drm/atomic-helper:
> drm_atomic_helper_shutdown(NULL) should be a noop").
>
> Suggested-by: Maxime Ripard 
> Signed-off-by: Douglas Anderson 
> ---
> This commit is only compile-time tested.
>
> Note that checkpatch yells that "drivers/gpu/drm/tiny/cirrus.c" is
> marked as 'obsolete', but it seems silly not to include the fix if
> it's already been written. If someone wants me to take that out,
> though, I can.
>
>  drivers/gpu/drm/arm/display/komeda/komeda_drv.c | 9 +
>  drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 7 +++
>  drivers/gpu/drm/arm/display/komeda/komeda_kms.h | 1 +
>  drivers/gpu/drm/arm/hdlcd_drv.c | 6 ++
>  drivers/gpu/drm/arm/malidp_drv.c| 6 ++
>  drivers/gpu/drm/ast/ast_drv.c   | 6 ++
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c| 6 ++
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c   | 8 
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 6 ++
>  drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 6 ++
>  drivers/gpu/drm/logicvc/logicvc_drm.c   | 9 +
>  drivers/gpu/drm/loongson/lsdc_drv.c | 6 ++
>  drivers/gpu/drm/mcde/mcde_drv.c | 9 +
>  drivers/gpu/drm/omapdrm/omap_drv.c  | 8 
>  drivers/gpu/drm/qxl/qxl_drv.c   | 7 +++
>  drivers/gpu/drm/sti/sti_drv.c   | 7 +++
>  drivers/gpu/drm/sun4i/sun4i_drv.c   | 6 ++
>  drivers/gpu/drm/tiny/bochs.c| 6 ++
>  drivers/gpu/drm/tiny/cirrus.c   | 6 ++
>  19 files changed, 125 insertions(+)

This has been about 3 weeks now and it feels like that's enough bake
time and several people have managed to test it (thanks!). Landing in
drm-misc-next:

ce3d99c83495 drm: Call drm_atomic_helper_shutdown() at shutdown time
for misc drivers

Re: [RFT PATCH 4/6] drm/ssd130x: Call drm_atomic_helper_shutdown() at remove time

[Doug Anderson]

Hi,

On Fri, Sep 1, 2023 at 4:40 PM Douglas Anderson  wrote:
>
> Based on grepping through the source code, this driver appears to be
> missing a call to drm_atomic_helper_shutdown() at remove time. Let's
> add it.
>
> The fact that we should call drm_atomic_helper_shutdown() in the case
> of OS driver remove comes straight out of the kernel doc "driver
> instance overview" in drm_drv.c.
>
> Suggested-by: Maxime Ripard 
> Signed-off-by: Douglas Anderson 
> ---
> This commit is only compile-time tested.
>
> NOTE: I'm not 100% sure this is the correct thing to do, but I _think_
> so. Please shout if you know better.
>
>  drivers/gpu/drm/solomon/ssd130x.c | 1 +
>  1 file changed, 1 insertion(+)

Landed this to drm-misc-next. Since I wasn't 100% sure, if someone
finds that this is bad after-the-fact, we can certainly revert.

10c8204c8b17 drm/ssd130x: Call drm_atomic_helper_shutdown() at remove time

Re: [RFT PATCH 3/6] drm/vc4: Call drm_atomic_helper_shutdown() at shutdown time

[Doug Anderson]

Hi,

On Fri, Sep 1, 2023 at 4:40 PM Douglas Anderson  wrote:
>
> Based on grepping through the source code these drivers appear to be
> missing a call to drm_atomic_helper_shutdown() at system shutdown
> time. Among other things, this means that if a panel is in use that it
> won't be cleanly powered off at system shutdown time.
>
> The fact that we should call drm_atomic_helper_shutdown() in the case
> of OS shutdown/restart comes straight out of the kernel doc "driver
> instance overview" in drm_drv.c.
>
> Suggested-by: Maxime Ripard 
> Signed-off-by: Douglas Anderson 
> ---
> This commit is only compile-time tested.
>
> Though this patch could be squashed into the patch ("drm: Call
> drm_atomic_helper_shutdown() at shutdown time for misc drivers"), I
> kept it separate to call attention to this driver. While writing this
> patch, I noticed that the bind() function is using "devm" and thus
> assumes it doesn't need to do much explicit error handling. That's
> actually a bug. As per kernel docs [1] "the lifetime of the aggregate
> driver does not align with any of the underlying struct device
> instances. Therefore devm cannot be used and all resources acquired or
> allocated in this callback must be explicitly released in the unbind
> callback". Fixing that is outside the scope of this commit.
>
> [1] https://docs.kernel.org/driver-api/component.html
>
>  drivers/gpu/drm/vc4/vc4_drv.c | 36 ++-
>  1 file changed, 23 insertions(+), 13 deletions(-)

Landed to drm-misc-next:

013d382d11a2 drm/vc4: Call drm_atomic_helper_shutdown() at shutdown time

Re: [RFT PATCH 01/15] drm/armada: Call drm_atomic_helper_shutdown() at shutdown time

[Doug Anderson]

Hi,

On Wed, Sep 20, 2023 at 11:58 AM Russell King (Oracle)
 wrote:
>
> On Wed, Sep 20, 2023 at 11:03:32AM -0700, Doug Anderson wrote:
> > Maxime,
> >
> > On Wed, Sep 13, 2023 at 8:34 AM Doug Anderson  wrote:
> > >
> > > Hi,
> > >
> > > On Tue, Sep 5, 2023 at 7:23 AM Doug Anderson  
> > > wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Sun, Sep 3, 2023 at 8:53 AM Russell King (Oracle)
> > > >  wrote:
> > > > >
> > > > > On Fri, Sep 01, 2023 at 04:41:12PM -0700, Douglas Anderson wrote:
> > > > > > Based on grepping through the source code this driver appears to be
> > > > > > missing a call to drm_atomic_helper_shutdown() at system shutdown
> > > > > > time. Among other things, this means that if a panel is in use that 
> > > > > > it
> > > > > > won't be cleanly powered off at system shutdown time.
> > > > > >
> > > > > > The fact that we should call drm_atomic_helper_shutdown() in the 
> > > > > > case
> > > > > > of OS shutdown/restart comes straight out of the kernel doc "driver
> > > > > > instance overview" in drm_drv.c.
> > > > > >
> > > > > > This driver was fairly easy to update. The drm_device is stored in 
> > > > > > the
> > > > > > drvdata so we just have to make sure the drvdata is NULL whenever 
> > > > > > the
> > > > > > device is not bound.
> > > > >
> > > > > ... and there I think you have a misunderstanding of the driver model.
> > > > > Please have a look at device_unbind_cleanup() which will be called if
> > > > > probe fails, or when the device is removed (in other words, when it is
> > > > > not bound to a driver.)
> > > >
> > > > ...and there I think you didn't read this patch closely enough and
> > > > perhaps that you have a misunderstanding of the component model.
> > > > Please have a look at the difference between armada_drm_unbind() and
> > > > armada_drm_remove() and also check which of those two functions is
> > > > being modified by my patch. Were this patch adding a call to
> > > > "dev_set_drvdata(dev, NULL)" in armada_drm_remove() then your NAK
> > > > would be justified. However, I am not aware of anything in the
> > > > component unbind path nor in the failure case of component bind that
> > > > would NULL the drvdata.
> > > >
> > > > Kindly look at the patch a second time with this in mind.
> > >
> > > Since I didn't see any further response, I'll assume that my
> > > explanation here has addressed your concerns. If not, I can re-post it
> > > without NULLing the "drvdata". While I still believe this is unsafe in
> > > some corner cases because of the component model used by this driver,
> > > at least it would get the shutdown call in.
> > >
> > > In any case, what's the process for landing patches to this driver?
> > > Running `./scripts/get_maintainer.pl --scm -f
> > > drivers/gpu/drm/armada/armada_drv.c` seems to indicate that this
> > > should go through the git tree:
> > >
> > > git git://git.armlinux.org.uk/~rmk/linux-arm.git drm-armada-devel
> > >
> > > ...but it doesn't appear that recent changes to this driver have gone
> > > that way. Should this land through drm-misc?
> >
> > Do you have any advice here? Should I land this through drm-misc-next,
> > put it on ice for a while, or resend without the calls to NULL our the
> > drvdata?
>
> Sorry, I haven't had a chance to look at it, but I think you're probably
> right, so I withdraw my objection. Please take it through drm-misc for
> the time being. Thanks.

Pushed to drm-misc-next:

c478768ce807 drm/armada: Call drm_atomic_helper_shutdown() at shutdown time

Re: [PATCH] drm/mipi-dsi: Fix detach call without attach

[Sebastian Reichel]

Hi,

On Thu, Sep 21, 2023 at 01:50:32PM +0300, Tomi Valkeinen wrote:
> It's been reported that DSI host driver's detach can be called without
> the attach ever happening:
> 
> https://lore.kernel.org/all/20230412073954.20601-1-t...@atomide.com/
> 
> After reading the code, I think this is what happens:
> 
> We have a DSI host defined in the device tree and a DSI peripheral under
> that host (i.e. an i2c device using the DSI as data bus doesn't exhibit
> this behavior).
> 
> The host driver calls mipi_dsi_host_register(), which causes (via a few
> functions) mipi_dsi_device_add() to be called for the DSI peripheral. So
> now we have a DSI device under the host, but attach hasn't been called.
> 
> Normally the probing of the devices continues, and eventually the DSI
> peripheral's driver will call mipi_dsi_attach(), attaching the
> peripheral.
> 
> However, if the host driver's probe encounters an error after calling
> mipi_dsi_host_register(), and before the peripheral has called
> mipi_dsi_attach(), the host driver will do cleanups and return an error
> from its probe function. The cleanups include calling
> mipi_dsi_host_unregister().
> 
> mipi_dsi_host_unregister() will call two functions for all its DSI
> peripheral devices: mipi_dsi_detach() and mipi_dsi_device_unregister().
> The latter makes sense, as the device exists, but the former may be
> wrong as attach has not necessarily been done.
> 
> To fix this, track the attached state of the peripheral, and only detach
> from mipi_dsi_host_unregister() if the peripheral was attached.
> 
> Note that I have only tested this with a board with an i2c DSI
> peripheral, not with a "pure" DSI peripheral.
> 
> However, slightly related, the unregister machinery still seems broken.
> E.g. if the DSI host driver is unbound, it'll detach and unregister the
> DSI peripherals. After that, when the DSI peripheral driver unbound
> it'll call detach either directly or using the devm variant, leading to
> a crash. And probably the driver will crash if it happens, for some
> reason, to try to send a message via the DSI bus.
> 
> But that's another topic.
> 
> Signed-off-by: Tomi Valkeinen 
> ---

Reviewed-by: Sebastian Reichel 

-- Sebastian

>  drivers/gpu/drm/drm_mipi_dsi.c | 17 +++--
>  include/drm/drm_mipi_dsi.h |  2 ++
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> index 14201f73aab1..843a6dbda93a 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -347,7 +347,8 @@ static int mipi_dsi_remove_device_fn(struct device *dev, 
> void *priv)
>  {
>   struct mipi_dsi_device *dsi = to_mipi_dsi_device(dev);
>  
> - mipi_dsi_detach(dsi);
> + if (dsi->attached)
> + mipi_dsi_detach(dsi);
>   mipi_dsi_device_unregister(dsi);
>  
>   return 0;
> @@ -370,11 +371,18 @@ EXPORT_SYMBOL(mipi_dsi_host_unregister);
>  int mipi_dsi_attach(struct mipi_dsi_device *dsi)
>  {
>   const struct mipi_dsi_host_ops *ops = dsi->host->ops;
> + int ret;
>  
>   if (!ops || !ops->attach)
>   return -ENOSYS;
>  
> - return ops->attach(dsi->host, dsi);
> + ret = ops->attach(dsi->host, dsi);
> + if (ret)
> + return ret;
> +
> + dsi->attached = true;
> +
> + return 0;
>  }
>  EXPORT_SYMBOL(mipi_dsi_attach);
>  
> @@ -386,9 +394,14 @@ int mipi_dsi_detach(struct mipi_dsi_device *dsi)
>  {
>   const struct mipi_dsi_host_ops *ops = dsi->host->ops;
>  
> + if (WARN_ON(!dsi->attached))
> + return -EINVAL;
> +
>   if (!ops || !ops->detach)
>   return -ENOSYS;
>  
> + dsi->attached = false;
> +
>   return ops->detach(dsi->host, dsi);
>  }
>  EXPORT_SYMBOL(mipi_dsi_detach);
> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> index c9df0407980c..c0aec0d4d664 100644
> --- a/include/drm/drm_mipi_dsi.h
> +++ b/include/drm/drm_mipi_dsi.h
> @@ -168,6 +168,7 @@ struct mipi_dsi_device_info {
>   * struct mipi_dsi_device - DSI peripheral device
>   * @host: DSI host for this peripheral
>   * @dev: driver model device node for this peripheral
> + * @attached: the DSI device has been successfully attached
>   * @name: DSI peripheral chip type
>   * @channel: virtual channel assigned to the peripheral
>   * @format: pixel format for video mode
> @@ -184,6 +185,7 @@ struct mipi_dsi_device_info {
>  struct mipi_dsi_device {
>   struct mipi_dsi_host *host;
>   struct device dev;
> + bool attached;
>  
>   char name[DSI_DEV_NAME_SIZE];
>   unsigned int channel;
> 
> ---
> base-commit: 9fc75c40faa29df14ba16066be6bdfaea9f39ce4
> change-id: 20230921-dsi-detach-fix-6736f7a48ba7
> 
> Best regards,
> -- 
> Tomi Valkeinen 
> 


signature.asc
Description: PGP signature

[PATCH] drm/i915/guc: Suppress 'ignoring reset notification' message

[John . C . Harrison]

From: John Harrison 

If an active context has been banned (e.g. Ctrl+C killed) then it is
likely to be reset as part of evicting it from the hardware. That
results in a 'ignoring context reset notification: banned = 1'
message at info level. This confuses/concerns people and makes them
thing something has gone wrong when it hasn't.

There is already a debug level message with essentially the same
information. So drop the 'ignore' info level one and just add the
'ignore' flag to the debug level one instead (which will therefore not
appear by default but will still show up in CI runs).

Signed-off-by: John Harrison 
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index cabdc645fcddb..da7331346df1f 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -4770,19 +4770,19 @@ static void guc_context_replay(struct intel_context *ce)
 static void guc_handle_context_reset(struct intel_guc *guc,
 struct intel_context *ce)
 {
+   bool capture = intel_context_is_schedulable(ce);
+
trace_intel_context_reset(ce);
 
-   guc_dbg(guc, "Got context reset notification: 0x%04X on %s, exiting = 
%s, banned = %s\n",
+   guc_dbg(guc, "%s context reset notification: 0x%04X on %s, exiting = 
%s, banned = %s\n",
+   capture ? "Got" : "Ignoring",
ce->guc_id.id, ce->engine->name,
str_yes_no(intel_context_is_exiting(ce)),
str_yes_no(intel_context_is_banned(ce)));
 
-   if (likely(intel_context_is_schedulable(ce))) {
+   if (capture) {
capture_error_state(guc, ce);
guc_context_replay(ce);
-   } else {
-   guc_info(guc, "Ignoring context reset notification of exiting 
context 0x%04X on %s",
-ce->guc_id.id, ce->engine->name);
}
 }
 
-- 
2.41.0

Re: [PATCH] MAINTAINERS: Update drm-misc entry to match all drivers

[Helen Koike]

On 21/09/2023 15:12, Helen Koike wrote:

Hi,

On 19/09/2023 10:12, Maxime Ripard wrote:

We've had a number of times when a patch slipped through and we couldn't
pick them up either because our MAINTAINERS entry only covers the
framework and thus we weren't Cc'd.

Let's take another approach where we match everything, and remove all
the drivers that are not maintained through drm-misc.

Signed-off-by: Maxime Ripard 
---
  MAINTAINERS | 23 ---
  1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 90f13281d297..757d4f33e158 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6860,12 +6860,29 @@ M:    Thomas Zimmermann 
  S:    Maintained
  W:
https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html

  T:    git git://anongit.freedesktop.org/drm/drm-misc
+F:    Documentation/devicetree/bindings/display/
+F:    Documentation/devicetree/bindings/gpu/
  F:    Documentation/gpu/
-F:    drivers/gpu/drm/*
+F:    drivers/gpu/drm/
  F:    drivers/gpu/vga/
-F:    include/drm/drm*
+F:    include/drm/drm
  F:    include/linux/vga*
-F:    include/uapi/drm/drm*
+F:    include/uapi/drm/
+X:    drivers/gpu/drm/amd/
+X:    drivers/gpu/drm/armada/
+X:    drivers/gpu/drm/etnaviv/
+X:    drivers/gpu/drm/exynos/
+X:    drivers/gpu/drm/gma500/
+X:    drivers/gpu/drm/i915/
+X:    drivers/gpu/drm/imx/
+X:    drivers/gpu/drm/ingenic/
+X:    drivers/gpu/drm/kmb/
+X:    drivers/gpu/drm/mediatek/
+X:    drivers/gpu/drm/msm/
+X:    drivers/gpu/drm/nouveau/
+X:    drivers/gpu/drm/radeon/
+X:    drivers/gpu/drm/renesas/
+X:    drivers/gpu/drm/tegra/


Should drivers/gpu/drm/ci/ be in the list too?


ops, please ignore this message, I misread the patch, it is already 
included by default (instead of excluded).


sorry for the noise.
Helen



Thanks,
Helen


  DRM DRIVERS FOR ALLWINNER A10
  M:    Maxime Ripard 

Re: [PATCH] MAINTAINERS: Update drm-misc entry to match all drivers

[Helen Koike]

Hi,

On 19/09/2023 10:12, Maxime Ripard wrote:

We've had a number of times when a patch slipped through and we couldn't
pick them up either because our MAINTAINERS entry only covers the
framework and thus we weren't Cc'd.

Let's take another approach where we match everything, and remove all
the drivers that are not maintained through drm-misc.

Signed-off-by: Maxime Ripard 
---
  MAINTAINERS | 23 ---
  1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 90f13281d297..757d4f33e158 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6860,12 +6860,29 @@ M:  Thomas Zimmermann 
  S:Maintained
  W:https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html
  T:git git://anongit.freedesktop.org/drm/drm-misc
+F: Documentation/devicetree/bindings/display/
+F: Documentation/devicetree/bindings/gpu/
  F:Documentation/gpu/
-F: drivers/gpu/drm/*
+F: drivers/gpu/drm/
  F:drivers/gpu/vga/
-F: include/drm/drm*
+F: include/drm/drm
  F:include/linux/vga*
-F: include/uapi/drm/drm*
+F: include/uapi/drm/
+X: drivers/gpu/drm/amd/
+X: drivers/gpu/drm/armada/
+X: drivers/gpu/drm/etnaviv/
+X: drivers/gpu/drm/exynos/
+X: drivers/gpu/drm/gma500/
+X: drivers/gpu/drm/i915/
+X: drivers/gpu/drm/imx/
+X: drivers/gpu/drm/ingenic/
+X: drivers/gpu/drm/kmb/
+X: drivers/gpu/drm/mediatek/
+X: drivers/gpu/drm/msm/
+X: drivers/gpu/drm/nouveau/
+X: drivers/gpu/drm/radeon/
+X: drivers/gpu/drm/renesas/
+X: drivers/gpu/drm/tegra/


Should drivers/gpu/drm/ci/ be in the list too?

Thanks,
Helen

  
  DRM DRIVERS FOR ALLWINNER A10

  M:Maxime Ripard 

Re: [PATCH RFC v2 00/37] drm/connector: Create HDMI Connector infrastructure

[Maxime Ripard]

Hi Hans,

On Thu, Sep 21, 2023 at 06:29:29PM +0200, Hans Verkuil wrote:
> On 20/09/2023 16:35, Maxime Ripard wrote:
> > Hi,
> > 
> > Here's a series that creates a subclass of drm_connector specifically
> > targeted at HDMI controllers.
> > 
> > The idea behind this series came from a recent discussion on IRC during
> > which we discussed infoframes generation of i915 vs everything else. 
> > 
> > Infoframes generation code still requires some decent boilerplate, with
> > each driver doing some variation of it.
> > 
> > In parallel, while working on vc4, we ended up converting a lot of i915
> > logic (mostly around format / bpc selection, and scrambler setup) to
> > apply on top of a driver that relies only on helpers.
> > 
> > While currently sitting in the vc4 driver, none of that logic actually
> > relies on any driver or hardware-specific behaviour.
> > 
> > The only missing piece to make it shareable are a bunch of extra
> > variables stored in a state (current bpc, format, RGB range selection,
> > etc.).
> > 
> > The initial implementation was relying on some generic subclass of
> > drm_connector to address HDMI connectors, with a bunch of helpers that
> > will take care of all the "HDMI Spec" related code. Scrambler setup is
> > missing at the moment but can easily be plugged in.
> > 
> > The feedback was that creating a connector subclass like was done for
> > writeback would prevent the adoption of those helpers since it couldn't
> > be used in all situations (like when the connector driver can implement
> > multiple output) and required more churn to cast between the
> > drm_connector and its subclass. The decision was thus to provide a set
> > of helper and to store the required variables in drm_connector and
> > drm_connector_state. This what has been implemented now.
> > 
> > Hans Verkuil also expressed interest in implementing a mechanism in v4l2
> > to retrieve infoframes from HDMI receiver and implementing an
> > infoframe-decode tool.
> 
> I'd love to get started on that, but...
> 
> > 
> > This series thus leverages the infoframe generation code to expose it
> > through debugfs.
> > 
> > This entire series is only build-tested at the moment. Let me know what
> > you think,
> 
> ...trying this series on my RPi4 gives me this during boot:
> 
> [2.361239] vc4-drm gpu: bound fe40.hvs (ops 0x800080cac6f8)
> [2.367834] Unable to handle kernel NULL pointer dereference at virtual 
> address 0090
> [2.376748] Mem abort info:
> [2.379570]   ESR = 0x9644
> [2.383367]   EC = 0x25: DABT (current EL), IL = 32 bits
> [2.388748]   SET = 0, FnV = 0
> [2.391835]   EA = 0, S1PTW = 0
> [2.395011]   FSC = 0x04: level 0 translation fault
> [2.399951] Data abort info:
> [2.402864]   ISV = 0, ISS = 0x0044, ISS2 = 0x
> [2.408420]   CM = 0, WnR = 1, TnD = 0, TagAccess = 0
> [2.413536]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [2.418916] [0090] user address but active_mm is swapper
> [2.425353] Internal error: Oops: 9644 [#1] PREEMPT SMP
> [2.431700] Modules linked in:
> [2.434791] CPU: 2 PID: 55 Comm: kworker/u8:3 Not tainted 
> 6.6.0-rc1-hdmi-dbg #245
> [2.442372] Hardware name: Raspberry Pi 4 Model B Rev 1.4 (DT)
> [2.448278] Workqueue: events_unbound deferred_probe_work_func
> [2.454193] pstate: 8005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [2.461245] pc : drm_connector_attach_max_bpc_property+0x48/0x90
> [2.467332] lr : drm_connector_attach_max_bpc_property+0x3c/0x90
> [2.473415] sp : 800081d038b0
> [2.476766] x29: 800081d038b0 x28:  x27: 
> 0001041968c0
> [2.483999] x26:  x25: 00010339d558 x24: 
> 000103399000
> [2.491231] x23: 800080caa3e8 x22: 800080e96a20 x21: 
> 000c
> [2.498463] x20: 000c x19: 00010339d558 x18: 
> 
> [2.505694] x17: 0001008e7650 x16: 800080d55500 x15: 
> 
> [2.512926] x14: 000105dda209 x13: 0006 x12: 
> 0001
> [2.520158] x11: 0101010101010101 x10: 00027effe219 x9 : 
> 0001
> [2.527389] x8 : 000105db8ad4 x7 : c0c0c0c0 x6 : 
> c0c0c0c0
> [2.534620] x5 :  x4 : 00010339d728 x3 : 
> 00010339d728
> [2.541852] x2 : 000c x1 :  x0 : 
> 
> [2.549083] Call trace:
> [2.551554]  drm_connector_attach_max_bpc_property+0x48/0x90
> [2.557285]  drmm_connector_hdmi_init+0x114/0x14c
> [2.562048]  vc4_hdmi_bind+0x320/0xa40
> [2.565842]  component_bind_all+0x114/0x23c
> [2.570077]  vc4_drm_bind+0x148/0x2c0
> [2.573784]  try_to_bring_up_aggregate_device+0x168/0x1d4
> [2.579253]  __component_add+0xa4/0x16c
> [2.583136]  component_add+0x14/0x20
> [2.586754]  vc4_hdmi_dev_probe+0x1c/0x28
> [2.590815]  

Re: [PATCH RFC v2 00/37] drm/connector: Create HDMI Connector infrastructure

[Hans Verkuil]

On 20/09/2023 16:35, Maxime Ripard wrote:
> Hi,
> 
> Here's a series that creates a subclass of drm_connector specifically
> targeted at HDMI controllers.
> 
> The idea behind this series came from a recent discussion on IRC during
> which we discussed infoframes generation of i915 vs everything else. 
> 
> Infoframes generation code still requires some decent boilerplate, with
> each driver doing some variation of it.
> 
> In parallel, while working on vc4, we ended up converting a lot of i915
> logic (mostly around format / bpc selection, and scrambler setup) to
> apply on top of a driver that relies only on helpers.
> 
> While currently sitting in the vc4 driver, none of that logic actually
> relies on any driver or hardware-specific behaviour.
> 
> The only missing piece to make it shareable are a bunch of extra
> variables stored in a state (current bpc, format, RGB range selection,
> etc.).
> 
> The initial implementation was relying on some generic subclass of
> drm_connector to address HDMI connectors, with a bunch of helpers that
> will take care of all the "HDMI Spec" related code. Scrambler setup is
> missing at the moment but can easily be plugged in.
> 
> The feedback was that creating a connector subclass like was done for
> writeback would prevent the adoption of those helpers since it couldn't
> be used in all situations (like when the connector driver can implement
> multiple output) and required more churn to cast between the
> drm_connector and its subclass. The decision was thus to provide a set
> of helper and to store the required variables in drm_connector and
> drm_connector_state. This what has been implemented now.
> 
> Hans Verkuil also expressed interest in implementing a mechanism in v4l2
> to retrieve infoframes from HDMI receiver and implementing an
> infoframe-decode tool.

I'd love to get started on that, but...

> 
> This series thus leverages the infoframe generation code to expose it
> through debugfs.
> 
> This entire series is only build-tested at the moment. Let me know what
> you think,

...trying this series on my RPi4 gives me this during boot:

[2.361239] vc4-drm gpu: bound fe40.hvs (ops 0x800080cac6f8)
[2.367834] Unable to handle kernel NULL pointer dereference at virtual 
address 0090
[2.376748] Mem abort info:
[2.379570]   ESR = 0x9644
[2.383367]   EC = 0x25: DABT (current EL), IL = 32 bits
[2.388748]   SET = 0, FnV = 0
[2.391835]   EA = 0, S1PTW = 0
[2.395011]   FSC = 0x04: level 0 translation fault
[2.399951] Data abort info:
[2.402864]   ISV = 0, ISS = 0x0044, ISS2 = 0x
[2.408420]   CM = 0, WnR = 1, TnD = 0, TagAccess = 0
[2.413536]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[2.418916] [0090] user address but active_mm is swapper
[2.425353] Internal error: Oops: 9644 [#1] PREEMPT SMP
[2.431700] Modules linked in:
[2.434791] CPU: 2 PID: 55 Comm: kworker/u8:3 Not tainted 6.6.0-rc1-hdmi-dbg 
#245
[2.442372] Hardware name: Raspberry Pi 4 Model B Rev 1.4 (DT)
[2.448278] Workqueue: events_unbound deferred_probe_work_func
[2.454193] pstate: 8005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[2.461245] pc : drm_connector_attach_max_bpc_property+0x48/0x90
[2.467332] lr : drm_connector_attach_max_bpc_property+0x3c/0x90
[2.473415] sp : 800081d038b0
[2.476766] x29: 800081d038b0 x28:  x27: 0001041968c0
[2.483999] x26:  x25: 00010339d558 x24: 000103399000
[2.491231] x23: 800080caa3e8 x22: 800080e96a20 x21: 000c
[2.498463] x20: 000c x19: 00010339d558 x18: 
[2.505694] x17: 0001008e7650 x16: 800080d55500 x15: 
[2.512926] x14: 000105dda209 x13: 0006 x12: 0001
[2.520158] x11: 0101010101010101 x10: 00027effe219 x9 : 0001
[2.527389] x8 : 000105db8ad4 x7 : c0c0c0c0 x6 : c0c0c0c0
[2.534620] x5 :  x4 : 00010339d728 x3 : 00010339d728
[2.541852] x2 : 000c x1 :  x0 : 
[2.549083] Call trace:
[2.551554]  drm_connector_attach_max_bpc_property+0x48/0x90
[2.557285]  drmm_connector_hdmi_init+0x114/0x14c
[2.562048]  vc4_hdmi_bind+0x320/0xa40
[2.565842]  component_bind_all+0x114/0x23c
[2.570077]  vc4_drm_bind+0x148/0x2c0
[2.573784]  try_to_bring_up_aggregate_device+0x168/0x1d4
[2.579253]  __component_add+0xa4/0x16c
[2.583136]  component_add+0x14/0x20
[2.586754]  vc4_hdmi_dev_probe+0x1c/0x28
[2.590815]  platform_probe+0x68/0xc4
[2.594522]  really_probe+0x148/0x2b0
[2.598228]  __driver_probe_device+0x78/0x12c
[2.602638]  driver_probe_device+0xd8/0x15c
[2.606873]  __device_attach_driver+0xb8/0x134
[2.611372]  bus_for_each_drv+0x80/0xdc
[2.615254]  __device_attach+0x9c/0x188

[PULL] drm-misc-fixes

[Thomas Zimmermann]

Hi Dave and Daniel,

this is the PR for drm-misc-fixes for this week.

Best regards
Thomas

drm-misc-fixes-2023-09-21:
Short summary of fixes pull:

 * DRM MM-test fixes
 * Fbdev Kconfig fixes

 * ivpu:
   * IRQ-handling fixes

 * meson:
   * Fix memory leak in HDMI EDID code

 * nouveau:
   * Correct type casting
   * Fix memory leak in scheduler
   * u_memcpya() fixes

 * virtio:
   * Fence cleanups
The following changes since commit 139a27854bf5ce93ff9805f9f7683b88c13074dc:

  drm/tests: helpers: Avoid a driver uaf (2023-09-14 13:57:58 +0200)

are available in the Git repository at:

  git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2023-09-21

for you to fetch changes up to f75f71b2c418a27a7c05139bb27a0c83adf88d19:

  fbdev/sh7760fb: Depend on FB=y (2023-09-21 10:33:49 +0200)


Short summary of fixes pull:

 * DRM MM-test fixes
 * Fbdev Kconfig fixes

 * ivpu:
   * IRQ-handling fixes

 * meson:
   * Fix memory leak in HDMI EDID code

 * nouveau:
   * Correct type casting
   * Fix memory leak in scheduler
   * u_memcpya() fixes

 * virtio:
   * Fence cleanups


Arnd Bergmann (1):
  drm: fix up fbdev Kconfig defaults

Dan Carpenter (1):
  nouveau/u_memcpya: fix NULL vs error pointer bug

Danilo Krummrich (2):
  drm/nouveau: fence: fix type cast warning in nouveau_fence_emit()
  drm/nouveau: sched: fix leaking memory of timedout job

Dave Airlie (1):
  nouveau/u_memcpya: use vmemdup_user

Jani Nikula (1):
  drm/meson: fix memory leak on ->hpd_notify callback

Janusz Krzysztofik (1):
  drm/tests: Fix incorrect argument in drm_test_mm_insert_range

José Pekkarinen (1):
  drm/virtio: clean out_fence on complete_submit

Karol Wachowski (1):
  accel/ivpu/40xx: Fix buttress interrupt handling

Thomas Zimmermann (1):
  fbdev/sh7760fb: Depend on FB=y

 drivers/accel/ivpu/ivpu_hw_40xx.c  |  9 -
 drivers/gpu/drm/Kconfig|  2 +-
 drivers/gpu/drm/meson/meson_encoder_hdmi.c |  2 ++
 drivers/gpu/drm/nouveau/nouveau_drv.h  | 19 +--
 drivers/gpu/drm/nouveau/nouveau_exec.c |  2 +-
 drivers/gpu/drm/nouveau/nouveau_fence.c|  2 +-
 drivers/gpu/drm/nouveau/nouveau_sched.c| 12 +---
 drivers/gpu/drm/tests/drm_mm_test.c|  2 +-
 drivers/gpu/drm/virtio/virtgpu_submit.c|  1 -
 drivers/video/console/Kconfig  |  1 +
 drivers/video/fbdev/Kconfig|  2 +-
 drivers/video/fbdev/core/Kconfig   |  2 +-
 12 files changed, 31 insertions(+), 25 deletions(-)

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[PATCH] drm/edid/firmware: drop drm_kms_helper.edid_firmware backward compat

[Jani Nikula]

Since the edid_firmware module parameter was moved from
drm_kms_helper.ko to drm.ko in v4.15, we've had a backwards
compatibility helper in place, with a DRM_NOTE() suggesting to migrate
to drm.edid_firmware. This was added in commit ac6c35a4d8c7 ("drm: add
backwards compatibility support for drm_kms_helper.edid_firmware").

More than five years and 30+ kernel releases later, see if we could drop
the backward compatibility. Leave some warnings in place for a while
longer.

Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/drm_edid_load.c | 16 
 drivers/gpu/drm/drm_kms_helper_common.c | 11 ++-
 include/drm/drm_edid.h  |  5 -
 3 files changed, 6 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
index 5d9ef267ebb3..60fcb80bce61 100644
--- a/drivers/gpu/drm/drm_edid_load.c
+++ b/drivers/gpu/drm/drm_edid_load.c
@@ -23,22 +23,6 @@ module_param_string(edid_firmware, edid_firmware, 
sizeof(edid_firmware), 0644);
 MODULE_PARM_DESC(edid_firmware, "Do not probe monitor, use specified EDID blob 
"
"from built-in data or /lib/firmware instead. ");
 
-/* Use only for backward compatibility with drm_kms_helper.edid_firmware */
-int __drm_set_edid_firmware_path(const char *path)
-{
-   scnprintf(edid_firmware, sizeof(edid_firmware), "%s", path);
-
-   return 0;
-}
-EXPORT_SYMBOL(__drm_set_edid_firmware_path);
-
-/* Use only for backward compatibility with drm_kms_helper.edid_firmware */
-int __drm_get_edid_firmware_path(char *buf, size_t bufsize)
-{
-   return scnprintf(buf, bufsize, "%s", edid_firmware);
-}
-EXPORT_SYMBOL(__drm_get_edid_firmware_path);
-
 #define GENERIC_EDIDS 6
 static const char * const generic_edid_name[GENERIC_EDIDS] = {
"edid/800x600.bin",
diff --git a/drivers/gpu/drm/drm_kms_helper_common.c 
b/drivers/gpu/drm/drm_kms_helper_common.c
index 0bf0fc1abf54..924e0f7bd5b7 100644
--- a/drivers/gpu/drm/drm_kms_helper_common.c
+++ b/drivers/gpu/drm/drm_kms_helper_common.c
@@ -38,17 +38,18 @@ MODULE_LICENSE("GPL and additional rights");
 
 #if IS_ENABLED(CONFIG_DRM_LOAD_EDID_FIRMWARE)
 
-/* Backward compatibility for drm_kms_helper.edid_firmware */
 static int edid_firmware_set(const char *val, const struct kernel_param *kp)
 {
-   DRM_NOTE("drm_kms_helper.edid_firmware is deprecated, please use 
drm.edid_firmware instead.\n");
+   pr_warn("drm_kms_helper.edid_firmware has been removed, please use 
drm.edid_firmware instead.\n");
 
-   return __drm_set_edid_firmware_path(val);
+   return -ENOENT;
 }
 
 static int edid_firmware_get(char *buffer, const struct kernel_param *kp)
 {
-   return __drm_get_edid_firmware_path(buffer, PAGE_SIZE);
+   pr_warn("drm_kms_helper.edid_firmware has been removed, please use 
drm.edid_firmware instead.\n");
+
+   return -ENOENT;
 }
 
 static const struct kernel_param_ops edid_firmware_ops = {
@@ -59,6 +60,6 @@ static const struct kernel_param_ops edid_firmware_ops = {
 module_param_cb(edid_firmware, _firmware_ops, NULL, 0644);
 __MODULE_PARM_TYPE(edid_firmware, "charp");
 MODULE_PARM_DESC(edid_firmware,
-"DEPRECATED. Use drm.edid_firmware module parameter instead.");
+"REMOVED. Use drm.edid_firmware module parameter instead.");
 
 #endif
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 882d2638708e..00f0a778ab62 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -387,11 +387,6 @@ int drm_edid_to_speaker_allocation(const struct edid 
*edid, u8 **sadb);
 int drm_av_sync_delay(struct drm_connector *connector,
  const struct drm_display_mode *mode);
 
-#ifdef CONFIG_DRM_LOAD_EDID_FIRMWARE
-int __drm_set_edid_firmware_path(const char *path);
-int __drm_get_edid_firmware_path(char *buf, size_t bufsize);
-#endif
-
 bool drm_edid_are_equal(const struct edid *edid1, const struct edid *edid2);
 
 int
-- 
2.39.2

Re: [PATCH drm-misc-next v4 4/8] drm/gpuvm: add common dma-resv per struct drm_gpuvm

[Danilo Krummrich]

On 9/21/23 16:34, Christian König wrote:



Am 21.09.23 um 16:25 schrieb Boris Brezillon:

On Thu, 21 Sep 2023 15:34:44 +0200
Danilo Krummrich  wrote:


On 9/21/23 09:39, Christian König wrote:

Am 20.09.23 um 16:42 schrieb Danilo Krummrich:

Provide a common dma-resv for GEM objects not being used outside of this
GPU-VM. This is used in a subsequent patch to generalize dma-resv,
external and evicted object handling and GEM validation.

Signed-off-by: Danilo Krummrich 
---
   drivers/gpu/drm/drm_gpuvm.c    |  9 +++--
   drivers/gpu/drm/nouveau/nouveau_uvmm.c |  2 +-
   include/drm/drm_gpuvm.h    | 17 -
   3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
index bfea4a8a19ec..cbf4b738a16c 100644
--- a/drivers/gpu/drm/drm_gpuvm.c
+++ b/drivers/gpu/drm/drm_gpuvm.c
@@ -655,6 +655,7 @@ drm_gpuva_range_valid(struct drm_gpuvm *gpuvm,
   /**
    * drm_gpuvm_init() - initialize a _gpuvm
    * @gpuvm: pointer to the _gpuvm to initialize
+ * @drm: the drivers _device
    * @name: the name of the GPU VA space
    * @start_offset: the start offset of the GPU VA space
    * @range: the size of the GPU VA space
@@ -668,7 +669,7 @@ drm_gpuva_range_valid(struct drm_gpuvm *gpuvm,
    *  is expected to be managed by the surrounding driver structures.
    */
   void
-drm_gpuvm_init(struct drm_gpuvm *gpuvm,
+drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device *drm,
  const char *name,
  u64 start_offset, u64 range,
  u64 reserve_offset, u64 reserve_range,
@@ -694,6 +695,8 @@ drm_gpuvm_init(struct drm_gpuvm *gpuvm,
    reserve_range)))
   __drm_gpuva_insert(gpuvm, >kernel_alloc_node);
   }
+
+    drm_gem_private_object_init(drm, >d_obj, 0);
   }
   EXPORT_SYMBOL_GPL(drm_gpuvm_init);
@@ -713,7 +716,9 @@ drm_gpuvm_destroy(struct drm_gpuvm *gpuvm)
   __drm_gpuva_remove(>kernel_alloc_node);
   WARN(!RB_EMPTY_ROOT(>rb.tree.rb_root),
- "GPUVA tree is not empty, potentially leaking memory.");
+ "GPUVA tree is not empty, potentially leaking memory.\n");
+
+    drm_gem_private_object_fini(>d_obj);
   }
   EXPORT_SYMBOL_GPL(drm_gpuvm_destroy);
diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c 
b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
index 6c86b64273c3..a80ac8767843 100644
--- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
@@ -1836,7 +1836,7 @@ nouveau_uvmm_init(struct nouveau_uvmm *uvmm, struct 
nouveau_cli *cli,
   uvmm->kernel_managed_addr = kernel_managed_addr;
   uvmm->kernel_managed_size = kernel_managed_size;
-    drm_gpuvm_init(>base, cli->name,
+    drm_gpuvm_init(>base, cli->drm->dev, cli->name,
  NOUVEAU_VA_SPACE_START,
  NOUVEAU_VA_SPACE_END,
  kernel_managed_addr, kernel_managed_size,
diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
index 0e802676e0a9..c07d7c3e 100644
--- a/include/drm/drm_gpuvm.h
+++ b/include/drm/drm_gpuvm.h
@@ -240,14 +240,29 @@ struct drm_gpuvm {
    * @ops: _gpuvm_ops providing the split/merge steps to drivers
    */
   const struct drm_gpuvm_ops *ops;
+
+    /**
+ * @d_obj: Dummy GEM object; used internally to pass the GPU VMs
+ * dma-resv to _exec. Provides the GPUVM's 
+ */
+    struct drm_gem_object d_obj;

Yeah, as pointed out in the other mail that won't work like this.

Which one? Seems that I missed it.


The GPUVM contains GEM objects and therefore should probably have a reference 
to those objects.

When those GEM objects now use the dma-resv object embedded inside the GPUVM 
then they also need a reference to the GPUVM to make sure the dma-resv object 
won't be freed before they are freed.

My assumption here is that GEM objects being local to a certain VM never 
out-live the VM. We never share it with anyone, otherwise it would be external 
and hence wouldn't carray the VM's dma-resv. The only references I see are from 
the VM itself (which is fine) and from userspace. The latter isn't a problem as 
long as all GEM handles are closed before the VM is destroyed on FD close.

But we don't want to rely on userspace doing the right thing (calling
GEM_CLOSE before releasing the VM), do we?

BTW, even though my private BOs have a ref to their exclusive VM, I just
ran into a bug because drm_gem_shmem_free() acquires the resv lock
(which is questionable, but that's not the topic :-)) and
I was calling vm_put(bo->exclusive_vm) before drm_gem_shmem_free(),
leading to a use-after-free when the gem->resv is acquired. This has
nothing to do with drm_gpuvm, but it proves that this sort of bug is
likely to happen if we don't pay attention.


Do I miss something? Do we have use cases where this isn't true?

The other case I can think of is GEM being v[un]map-ed (kernel
mapping) after the VM was released.


I think the file reference and the VM 

Re: [PATCH drm-misc-next v4 4/8] drm/gpuvm: add common dma-resv per struct drm_gpuvm

[Boris Brezillon]

On Thu, 21 Sep 2023 16:34:54 +0200
Christian König  wrote:

> Am 21.09.23 um 16:25 schrieb Boris Brezillon:
> > On Thu, 21 Sep 2023 15:34:44 +0200
> > Danilo Krummrich  wrote:
> >  
> >> On 9/21/23 09:39, Christian König wrote:  
> >>> Am 20.09.23 um 16:42 schrieb Danilo Krummrich:  
>  Provide a common dma-resv for GEM objects not being used outside of this
>  GPU-VM. This is used in a subsequent patch to generalize dma-resv,
>  external and evicted object handling and GEM validation.
> 
>  Signed-off-by: Danilo Krummrich 
>  ---
>     drivers/gpu/drm/drm_gpuvm.c    |  9 +++--
>     drivers/gpu/drm/nouveau/nouveau_uvmm.c |  2 +-
>     include/drm/drm_gpuvm.h    | 17 -
>     3 files changed, 24 insertions(+), 4 deletions(-)
> 
>  diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
>  index bfea4a8a19ec..cbf4b738a16c 100644
>  --- a/drivers/gpu/drm/drm_gpuvm.c
>  +++ b/drivers/gpu/drm/drm_gpuvm.c
>  @@ -655,6 +655,7 @@ drm_gpuva_range_valid(struct drm_gpuvm *gpuvm,
>     /**
>      * drm_gpuvm_init() - initialize a _gpuvm
>      * @gpuvm: pointer to the _gpuvm to initialize
>  + * @drm: the drivers _device
>      * @name: the name of the GPU VA space
>      * @start_offset: the start offset of the GPU VA space
>      * @range: the size of the GPU VA space
>  @@ -668,7 +669,7 @@ drm_gpuva_range_valid(struct drm_gpuvm *gpuvm,
>      *  is expected to be managed by the surrounding driver 
>  structures.
>      */
>     void
>  -drm_gpuvm_init(struct drm_gpuvm *gpuvm,
>  +drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device *drm,
>    const char *name,
>    u64 start_offset, u64 range,
>    u64 reserve_offset, u64 reserve_range,
>  @@ -694,6 +695,8 @@ drm_gpuvm_init(struct drm_gpuvm *gpuvm,
>      reserve_range)))
>     __drm_gpuva_insert(gpuvm, >kernel_alloc_node);
>     }
>  +
>  +    drm_gem_private_object_init(drm, >d_obj, 0);
>     }
>     EXPORT_SYMBOL_GPL(drm_gpuvm_init);
>  @@ -713,7 +716,9 @@ drm_gpuvm_destroy(struct drm_gpuvm *gpuvm)
>     __drm_gpuva_remove(>kernel_alloc_node);
>     WARN(!RB_EMPTY_ROOT(>rb.tree.rb_root),
>  - "GPUVA tree is not empty, potentially leaking memory.");
>  + "GPUVA tree is not empty, potentially leaking memory.\n");
>  +
>  +    drm_gem_private_object_fini(>d_obj);
>     }
>     EXPORT_SYMBOL_GPL(drm_gpuvm_destroy);
>  diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c 
>  b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
>  index 6c86b64273c3..a80ac8767843 100644
>  --- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
>  +++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
>  @@ -1836,7 +1836,7 @@ nouveau_uvmm_init(struct nouveau_uvmm *uvmm, 
>  struct nouveau_cli *cli,
>     uvmm->kernel_managed_addr = kernel_managed_addr;
>     uvmm->kernel_managed_size = kernel_managed_size;
>  -    drm_gpuvm_init(>base, cli->name,
>  +    drm_gpuvm_init(>base, cli->drm->dev, cli->name,
>    NOUVEAU_VA_SPACE_START,
>    NOUVEAU_VA_SPACE_END,
>    kernel_managed_addr, kernel_managed_size,
>  diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
>  index 0e802676e0a9..c07d7c3e 100644
>  --- a/include/drm/drm_gpuvm.h
>  +++ b/include/drm/drm_gpuvm.h
>  @@ -240,14 +240,29 @@ struct drm_gpuvm {
>      * @ops: _gpuvm_ops providing the split/merge steps to drivers
>      */
>     const struct drm_gpuvm_ops *ops;
>  +
>  +    /**
>  + * @d_obj: Dummy GEM object; used internally to pass the GPU VMs
>  + * dma-resv to _exec. Provides the GPUVM's 
>  + */
>  +    struct drm_gem_object d_obj;  
> >>> Yeah, as pointed out in the other mail that won't work like this.  
> >> Which one? Seems that I missed it.
> >>  
> >>> The GPUVM contains GEM objects and therefore should probably have a 
> >>> reference to those objects.
> >>>
> >>> When those GEM objects now use the dma-resv object embedded inside the 
> >>> GPUVM then they also need a reference to the GPUVM to make sure the 
> >>> dma-resv object won't be freed before they are freed.  
> >> My assumption here is that GEM objects being local to a certain VM never 
> >> out-live the VM. We never share it with anyone, otherwise it would be 
> >> external and hence wouldn't carray the VM's dma-resv. The only references 
> >> I see are from the VM itself (which is fine) and from userspace. The 
> >> latter isn't a problem as long as all GEM handles are closed before the VM 
> >> is destroyed on FD close.  
> > But we don't want to rely on userspace doing the right thing (calling
> > GEM_CLOSE before 

[PULL] drm-intel-fixes

[Rodrigo Vivi]

Hi Dave and Daniel,

Here goes drm-intel-fixes-2023-09-21:

- Prevent error pointer dereference (Dan Carpenter)
- Fix PMU busyness values when using GuC mode (Umesh)

Thanks,
Rodrigo.

The following changes since commit ce9ecca0238b140b88f43859b211c9fdfd8e5b70:

  Linux 6.6-rc2 (2023-09-17 14:40:24 -0700)

are available in the Git repository at:

  git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2023-09-21

for you to fetch changes up to c524cd40e8a2a1a36f4898eaf2024beefeb815f3:

  i915/pmu: Move execlist stats initialization to execlist specific setup 
(2023-09-20 10:55:37 -0400)


- Prevent error pointer dereference (Dan Carpenter)
- Fix PMU busyness values when using GuC mode (Umesh)


Dan Carpenter (1):
  drm/i915/gt: Prevent error pointer dereference

Umesh Nerlige Ramappa (1):
  i915/pmu: Move execlist stats initialization to execlist specific setup

 drivers/gpu/drm/i915/gt/intel_engine_cs.c| 1 -
 drivers/gpu/drm/i915/gt/intel_execlists_submission.c | 2 ++
 drivers/gpu/drm/i915/gt/intel_lrc.c  | 5 +++--
 3 files changed, 5 insertions(+), 3 deletions(-)

Re: [PATCH 01/15] dt-bindings: mailbox: Add property for CMDQ secure driver

[林睿祥]

Re: [PATCH 00/10] Add mediate-drm secure flow for SVP

[林睿祥]

Re: [PATCH 00/15] Add CMDQ secure driver for SVP

[林睿祥]

Re: [PATCH drm-misc-next v4 4/8] drm/gpuvm: add common dma-resv per struct drm_gpuvm

[Danilo Krummrich]

On 9/21/23 16:25, Boris Brezillon wrote:

On Thu, 21 Sep 2023 15:34:44 +0200
Danilo Krummrich  wrote:


On 9/21/23 09:39, Christian König wrote:

Am 20.09.23 um 16:42 schrieb Danilo Krummrich:

Provide a common dma-resv for GEM objects not being used outside of this
GPU-VM. This is used in a subsequent patch to generalize dma-resv,
external and evicted object handling and GEM validation.

Signed-off-by: Danilo Krummrich 
---
   drivers/gpu/drm/drm_gpuvm.c    |  9 +++--
   drivers/gpu/drm/nouveau/nouveau_uvmm.c |  2 +-
   include/drm/drm_gpuvm.h    | 17 -
   3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
index bfea4a8a19ec..cbf4b738a16c 100644
--- a/drivers/gpu/drm/drm_gpuvm.c
+++ b/drivers/gpu/drm/drm_gpuvm.c
@@ -655,6 +655,7 @@ drm_gpuva_range_valid(struct drm_gpuvm *gpuvm,
   /**
    * drm_gpuvm_init() - initialize a _gpuvm
    * @gpuvm: pointer to the _gpuvm to initialize
+ * @drm: the drivers _device
    * @name: the name of the GPU VA space
    * @start_offset: the start offset of the GPU VA space
    * @range: the size of the GPU VA space
@@ -668,7 +669,7 @@ drm_gpuva_range_valid(struct drm_gpuvm *gpuvm,
    *  is expected to be managed by the surrounding driver structures.
    */
   void
-drm_gpuvm_init(struct drm_gpuvm *gpuvm,
+drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device *drm,
  const char *name,
  u64 start_offset, u64 range,
  u64 reserve_offset, u64 reserve_range,
@@ -694,6 +695,8 @@ drm_gpuvm_init(struct drm_gpuvm *gpuvm,
    reserve_range)))
   __drm_gpuva_insert(gpuvm, >kernel_alloc_node);
   }
+
+    drm_gem_private_object_init(drm, >d_obj, 0);
   }
   EXPORT_SYMBOL_GPL(drm_gpuvm_init);
@@ -713,7 +716,9 @@ drm_gpuvm_destroy(struct drm_gpuvm *gpuvm)
   __drm_gpuva_remove(>kernel_alloc_node);
   WARN(!RB_EMPTY_ROOT(>rb.tree.rb_root),
- "GPUVA tree is not empty, potentially leaking memory.");
+ "GPUVA tree is not empty, potentially leaking memory.\n");
+
+    drm_gem_private_object_fini(>d_obj);
   }
   EXPORT_SYMBOL_GPL(drm_gpuvm_destroy);
diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c 
b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
index 6c86b64273c3..a80ac8767843 100644
--- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
@@ -1836,7 +1836,7 @@ nouveau_uvmm_init(struct nouveau_uvmm *uvmm, struct 
nouveau_cli *cli,
   uvmm->kernel_managed_addr = kernel_managed_addr;
   uvmm->kernel_managed_size = kernel_managed_size;
-    drm_gpuvm_init(>base, cli->name,
+    drm_gpuvm_init(>base, cli->drm->dev, cli->name,
  NOUVEAU_VA_SPACE_START,
  NOUVEAU_VA_SPACE_END,
  kernel_managed_addr, kernel_managed_size,
diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
index 0e802676e0a9..c07d7c3e 100644
--- a/include/drm/drm_gpuvm.h
+++ b/include/drm/drm_gpuvm.h
@@ -240,14 +240,29 @@ struct drm_gpuvm {
    * @ops: _gpuvm_ops providing the split/merge steps to drivers
    */
   const struct drm_gpuvm_ops *ops;
+
+    /**
+ * @d_obj: Dummy GEM object; used internally to pass the GPU VMs
+ * dma-resv to _exec. Provides the GPUVM's 
+ */
+    struct drm_gem_object d_obj;


Yeah, as pointed out in the other mail that won't work like this.


Which one? Seems that I missed it.



The GPUVM contains GEM objects and therefore should probably have a reference 
to those objects.

When those GEM objects now use the dma-resv object embedded inside the GPUVM 
then they also need a reference to the GPUVM to make sure the dma-resv object 
won't be freed before they are freed.


My assumption here is that GEM objects being local to a certain VM never 
out-live the VM. We never share it with anyone, otherwise it would be external 
and hence wouldn't carray the VM's dma-resv. The only references I see are from 
the VM itself (which is fine) and from userspace. The latter isn't a problem as 
long as all GEM handles are closed before the VM is destroyed on FD close.


But we don't want to rely on userspace doing the right thing (calling
GEM_CLOSE before releasing the VM), do we?


I assume VM's are typically released on postclose() and drm_gem_release() is
called previously. But yeah, I guess there are indeed other issues.



BTW, even though my private BOs have a ref to their exclusive VM, I just
ran into a bug because drm_gem_shmem_free() acquires the resv lock
(which is questionable, but that's not the topic :-)) and
I was calling vm_put(bo->exclusive_vm) before drm_gem_shmem_free(),
leading to a use-after-free when the gem->resv is acquired. This has
nothing to do with drm_gpuvm, but it proves that this sort of bug is
likely to happen if we don't pay attention.



Do I miss something? Do we have use cases where this isn't true?


The other case I can think of is GEM 

Re: [PATCH v8 2/7] phy: Add HDMI configuration options

[Vinod Koul]

On 21-09-23, 16:01, Vinod Koul wrote:
> On 22-08-23, 20:22, Dmitry Baryshkov wrote:
> > On 22/08/2023 16:54, Vinod Koul wrote:
> > > On 17-08-23, 13:05, Dmitry Baryshkov wrote:
> > >> On 08/08/2023 11:32, Sandor Yu wrote:
> > >>> Allow HDMI PHYs to be configured through the generic
> > >>> functions through a custom structure added to the generic union.
> > >>>
> > >>> The parameters added here are based on HDMI PHY
> > >>> implementation practices.  The current set of parameters
> > >>> should cover the potential users.
> > >>>
> > >>> Signed-off-by: Sandor Yu 
> > >>> ---
> > >>>include/linux/phy/phy-hdmi.h | 24 
> > >>>include/linux/phy/phy.h  |  7 ++-
> > >>>2 files changed, 30 insertions(+), 1 deletion(-)
> > >>>create mode 100644 include/linux/phy/phy-hdmi.h
> > >>
> > >> I think this looks good now, thank you!
> > >>
> > >> Reviewed-by: Dmitry Baryshkov 
> > >
> > > Should this go thru drm or phy...?
> > 
> > I'd say, PHY, together with the other PHY patches. If you can merge
> > them into an immutable branch, then it can also be merged into
> > drm-misc (?) to provide the dependency between drm and phy parts.
> 
> phy/topic/hdmi should be pushed out in a bit for that

Sorry we need the drm header, so best to merge thru drm tree:

Acked-by: Vinod Koul 

-- 
~Vinod

Re: [PATCH drm-misc-next v4 4/8] drm/gpuvm: add common dma-resv per struct drm_gpuvm

[Christian König]

Am 21.09.23 um 16:25 schrieb Boris Brezillon:

On Thu, 21 Sep 2023 15:34:44 +0200
Danilo Krummrich  wrote:


On 9/21/23 09:39, Christian König wrote:

Am 20.09.23 um 16:42 schrieb Danilo Krummrich:

Provide a common dma-resv for GEM objects not being used outside of this
GPU-VM. This is used in a subsequent patch to generalize dma-resv,
external and evicted object handling and GEM validation.

Signed-off-by: Danilo Krummrich 
---
   drivers/gpu/drm/drm_gpuvm.c    |  9 +++--
   drivers/gpu/drm/nouveau/nouveau_uvmm.c |  2 +-
   include/drm/drm_gpuvm.h    | 17 -
   3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
index bfea4a8a19ec..cbf4b738a16c 100644
--- a/drivers/gpu/drm/drm_gpuvm.c
+++ b/drivers/gpu/drm/drm_gpuvm.c
@@ -655,6 +655,7 @@ drm_gpuva_range_valid(struct drm_gpuvm *gpuvm,
   /**
    * drm_gpuvm_init() - initialize a _gpuvm
    * @gpuvm: pointer to the _gpuvm to initialize
+ * @drm: the drivers _device
    * @name: the name of the GPU VA space
    * @start_offset: the start offset of the GPU VA space
    * @range: the size of the GPU VA space
@@ -668,7 +669,7 @@ drm_gpuva_range_valid(struct drm_gpuvm *gpuvm,
    *  is expected to be managed by the surrounding driver structures.
    */
   void
-drm_gpuvm_init(struct drm_gpuvm *gpuvm,
+drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device *drm,
  const char *name,
  u64 start_offset, u64 range,
  u64 reserve_offset, u64 reserve_range,
@@ -694,6 +695,8 @@ drm_gpuvm_init(struct drm_gpuvm *gpuvm,
    reserve_range)))
   __drm_gpuva_insert(gpuvm, >kernel_alloc_node);
   }
+
+    drm_gem_private_object_init(drm, >d_obj, 0);
   }
   EXPORT_SYMBOL_GPL(drm_gpuvm_init);
@@ -713,7 +716,9 @@ drm_gpuvm_destroy(struct drm_gpuvm *gpuvm)
   __drm_gpuva_remove(>kernel_alloc_node);
   WARN(!RB_EMPTY_ROOT(>rb.tree.rb_root),
- "GPUVA tree is not empty, potentially leaking memory.");
+ "GPUVA tree is not empty, potentially leaking memory.\n");
+
+    drm_gem_private_object_fini(>d_obj);
   }
   EXPORT_SYMBOL_GPL(drm_gpuvm_destroy);
diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c 
b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
index 6c86b64273c3..a80ac8767843 100644
--- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
@@ -1836,7 +1836,7 @@ nouveau_uvmm_init(struct nouveau_uvmm *uvmm, struct 
nouveau_cli *cli,
   uvmm->kernel_managed_addr = kernel_managed_addr;
   uvmm->kernel_managed_size = kernel_managed_size;
-    drm_gpuvm_init(>base, cli->name,
+    drm_gpuvm_init(>base, cli->drm->dev, cli->name,
  NOUVEAU_VA_SPACE_START,
  NOUVEAU_VA_SPACE_END,
  kernel_managed_addr, kernel_managed_size,
diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
index 0e802676e0a9..c07d7c3e 100644
--- a/include/drm/drm_gpuvm.h
+++ b/include/drm/drm_gpuvm.h
@@ -240,14 +240,29 @@ struct drm_gpuvm {
    * @ops: _gpuvm_ops providing the split/merge steps to drivers
    */
   const struct drm_gpuvm_ops *ops;
+
+    /**
+ * @d_obj: Dummy GEM object; used internally to pass the GPU VMs
+ * dma-resv to _exec. Provides the GPUVM's 
+ */
+    struct drm_gem_object d_obj;

Yeah, as pointed out in the other mail that won't work like this.

Which one? Seems that I missed it.


The GPUVM contains GEM objects and therefore should probably have a reference 
to those objects.

When those GEM objects now use the dma-resv object embedded inside the GPUVM 
then they also need a reference to the GPUVM to make sure the dma-resv object 
won't be freed before they are freed.

My assumption here is that GEM objects being local to a certain VM never 
out-live the VM. We never share it with anyone, otherwise it would be external 
and hence wouldn't carray the VM's dma-resv. The only references I see are from 
the VM itself (which is fine) and from userspace. The latter isn't a problem as 
long as all GEM handles are closed before the VM is destroyed on FD close.

But we don't want to rely on userspace doing the right thing (calling
GEM_CLOSE before releasing the VM), do we?

BTW, even though my private BOs have a ref to their exclusive VM, I just
ran into a bug because drm_gem_shmem_free() acquires the resv lock
(which is questionable, but that's not the topic :-)) and
I was calling vm_put(bo->exclusive_vm) before drm_gem_shmem_free(),
leading to a use-after-free when the gem->resv is acquired. This has
nothing to do with drm_gpuvm, but it proves that this sort of bug is
likely to happen if we don't pay attention.


Do I miss something? Do we have use cases where this isn't true?

The other case I can think of is GEM being v[un]map-ed (kernel
mapping) after the VM was released.


I think the file reference and the VM stays around in those cases as 
well, but yes 

Re: (subset) [PATCH v9 0/7] Initial support Cadence MHDP8501(HDMI/DP) for i.MX8MQ

[Vinod Koul]

On Thu, 07 Sep 2023 09:05:27 +0800, Sandor Yu wrote:
> The patch set initial support Cadence MHDP8501(HDMI/DP) DRM bridge
> drivers and Cadence HDP-TX PHY(HDMI/DP) drivers for Freescale i.MX8MQ.
> 
> The patch set compose of DRM bridge drivers and PHY drivers.
> 
> Both of them need the followed two patches to pass build.
>   drm: bridge: Cadence: convert mailbox functions to macro functions
>   phy: Add HDMI configuration options
> 
> [...]

Applied, thanks!

[2/7] phy: Add HDMI configuration options
  commit: 7f90516edb5cbfa4108b92bb83cbc8ef35a4cccd
[6/7] phy: freescale: Add DisplayPort PHY driver for i.MX8MQ
  commit: a2717f1d7c64660679441c407b96103abb7c4a8c
[7/7] phy: freescale: Add HDMI PHY driver for i.MX8MQ
  commit: 8e36091a94d2d28e8dccb9bfda081b2e42e951ae

Best regards,
-- 
~Vinod

Re: [PATCH v6 0/8] Initial support for Cadence MHDP8501(HDMI/DP) for i.MX8MQ

[Vinod Koul]

On Thu, 15 Jun 2023 09:38:10 +0800, Sandor Yu wrote:
> The patch set initial support for Cadence MHDP8501(HDMI/DP) DRM bridge
> drivers and Cadence HDP-TX PHY(HDMI/DP) drivers for Freescale i.MX8MQ.
> 
> The patch set compose of DRM bridge drivers and PHY drivers.
> 
> Both of them need the followed two patches to pass build.
>   drm: bridge: Cadence: convert mailbox functions to macro functions
>   phy: Add HDMI configuration options
> 
> [...]

Applied, thanks!

[1/8] drm: bridge: Cadence: convert mailbox functions to macro functions
  (no commit info)
[2/8] dt-bindings: display: bridge: Add Cadence MHDP8501 HDMI and DP
  (no commit info)
[3/8] drm: bridge: Cadence: Add MHDP8501 DP driver
  (no commit info)
[4/8] phy: Add HDMI configuration options
  commit: 7f90516edb5cbfa4108b92bb83cbc8ef35a4cccd
[5/8] drm: bridge: Cadence: Add MHDP8501 HDMI driver
  (no commit info)
[6/8] dt-bindings: phy: Add Freescale iMX8MQ DP and HDMI PHY
  (no commit info)
[7/8] phy: freescale: Add DisplayPort PHY driver for i.MX8MQ
  commit: a2717f1d7c64660679441c407b96103abb7c4a8c
[8/8] phy: freescale: Add HDMI PHY driver for i.MX8MQ
  commit: 8e36091a94d2d28e8dccb9bfda081b2e42e951ae

Best regards,
-- 
~Vinod

Re: [PATCH 11/15] soc: mediatek: Add cmdq_insert_backup_cookie before EOC for secure pkt

[林睿祥]

Re: [PATCH 08/15] soc: mediatek: Add cmdq_pkt_finalize_loop to CMDQ driver

[林睿祥]

Re: [PATCH drm-misc-next v4 4/8] drm/gpuvm: add common dma-resv per struct drm_gpuvm

[Boris Brezillon]

On Thu, 21 Sep 2023 15:34:44 +0200
Danilo Krummrich  wrote:

> On 9/21/23 09:39, Christian König wrote:
> > Am 20.09.23 um 16:42 schrieb Danilo Krummrich:  
> >> Provide a common dma-resv for GEM objects not being used outside of this
> >> GPU-VM. This is used in a subsequent patch to generalize dma-resv,
> >> external and evicted object handling and GEM validation.
> >>
> >> Signed-off-by: Danilo Krummrich 
> >> ---
> >>   drivers/gpu/drm/drm_gpuvm.c    |  9 +++--
> >>   drivers/gpu/drm/nouveau/nouveau_uvmm.c |  2 +-
> >>   include/drm/drm_gpuvm.h    | 17 -
> >>   3 files changed, 24 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
> >> index bfea4a8a19ec..cbf4b738a16c 100644
> >> --- a/drivers/gpu/drm/drm_gpuvm.c
> >> +++ b/drivers/gpu/drm/drm_gpuvm.c
> >> @@ -655,6 +655,7 @@ drm_gpuva_range_valid(struct drm_gpuvm *gpuvm,
> >>   /**
> >>    * drm_gpuvm_init() - initialize a _gpuvm
> >>    * @gpuvm: pointer to the _gpuvm to initialize
> >> + * @drm: the drivers _device
> >>    * @name: the name of the GPU VA space
> >>    * @start_offset: the start offset of the GPU VA space
> >>    * @range: the size of the GPU VA space
> >> @@ -668,7 +669,7 @@ drm_gpuva_range_valid(struct drm_gpuvm *gpuvm,
> >>    *  is expected to be managed by the surrounding driver structures.
> >>    */
> >>   void
> >> -drm_gpuvm_init(struct drm_gpuvm *gpuvm,
> >> +drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device *drm,
> >>  const char *name,
> >>  u64 start_offset, u64 range,
> >>  u64 reserve_offset, u64 reserve_range,
> >> @@ -694,6 +695,8 @@ drm_gpuvm_init(struct drm_gpuvm *gpuvm,
> >>    reserve_range)))
> >>   __drm_gpuva_insert(gpuvm, >kernel_alloc_node);
> >>   }
> >> +
> >> +    drm_gem_private_object_init(drm, >d_obj, 0);
> >>   }
> >>   EXPORT_SYMBOL_GPL(drm_gpuvm_init);
> >> @@ -713,7 +716,9 @@ drm_gpuvm_destroy(struct drm_gpuvm *gpuvm)
> >>   __drm_gpuva_remove(>kernel_alloc_node);
> >>   WARN(!RB_EMPTY_ROOT(>rb.tree.rb_root),
> >> - "GPUVA tree is not empty, potentially leaking memory.");
> >> + "GPUVA tree is not empty, potentially leaking memory.\n");
> >> +
> >> +    drm_gem_private_object_fini(>d_obj);
> >>   }
> >>   EXPORT_SYMBOL_GPL(drm_gpuvm_destroy);
> >> diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c 
> >> b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> >> index 6c86b64273c3..a80ac8767843 100644
> >> --- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> >> +++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> >> @@ -1836,7 +1836,7 @@ nouveau_uvmm_init(struct nouveau_uvmm *uvmm, struct 
> >> nouveau_cli *cli,
> >>   uvmm->kernel_managed_addr = kernel_managed_addr;
> >>   uvmm->kernel_managed_size = kernel_managed_size;
> >> -    drm_gpuvm_init(>base, cli->name,
> >> +    drm_gpuvm_init(>base, cli->drm->dev, cli->name,
> >>  NOUVEAU_VA_SPACE_START,
> >>  NOUVEAU_VA_SPACE_END,
> >>  kernel_managed_addr, kernel_managed_size,
> >> diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
> >> index 0e802676e0a9..c07d7c3e 100644
> >> --- a/include/drm/drm_gpuvm.h
> >> +++ b/include/drm/drm_gpuvm.h
> >> @@ -240,14 +240,29 @@ struct drm_gpuvm {
> >>    * @ops: _gpuvm_ops providing the split/merge steps to drivers
> >>    */
> >>   const struct drm_gpuvm_ops *ops;
> >> +
> >> +    /**
> >> + * @d_obj: Dummy GEM object; used internally to pass the GPU VMs
> >> + * dma-resv to _exec. Provides the GPUVM's 
> >> + */
> >> +    struct drm_gem_object d_obj;  
> > 
> > Yeah, as pointed out in the other mail that won't work like this.  
> 
> Which one? Seems that I missed it.
> 
> > 
> > The GPUVM contains GEM objects and therefore should probably have a 
> > reference to those objects.
> > 
> > When those GEM objects now use the dma-resv object embedded inside the 
> > GPUVM then they also need a reference to the GPUVM to make sure the 
> > dma-resv object won't be freed before they are freed.  
> 
> My assumption here is that GEM objects being local to a certain VM never 
> out-live the VM. We never share it with anyone, otherwise it would be 
> external and hence wouldn't carray the VM's dma-resv. The only references I 
> see are from the VM itself (which is fine) and from userspace. The latter 
> isn't a problem as long as all GEM handles are closed before the VM is 
> destroyed on FD close.

But we don't want to rely on userspace doing the right thing (calling
GEM_CLOSE before releasing the VM), do we?

BTW, even though my private BOs have a ref to their exclusive VM, I just
ran into a bug because drm_gem_shmem_free() acquires the resv lock
(which is questionable, but that's not the topic :-)) and
I was calling vm_put(bo->exclusive_vm) before drm_gem_shmem_free(),
leading to a use-after-free when the gem->resv is acquired. This 

Re: [PATCH drm-misc-next v4 4/8] drm/gpuvm: add common dma-resv per struct drm_gpuvm

[Christian König]

Am 21.09.23 um 15:34 schrieb Danilo Krummrich:

On 9/21/23 09:39, Christian König wrote:

Am 20.09.23 um 16:42 schrieb Danilo Krummrich:
Provide a common dma-resv for GEM objects not being used outside of 
this

GPU-VM. This is used in a subsequent patch to generalize dma-resv,
external and evicted object handling and GEM validation.

Signed-off-by: Danilo Krummrich 
---
  drivers/gpu/drm/drm_gpuvm.c    |  9 +++--
  drivers/gpu/drm/nouveau/nouveau_uvmm.c |  2 +-
  include/drm/drm_gpuvm.h    | 17 -
  3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
index bfea4a8a19ec..cbf4b738a16c 100644
--- a/drivers/gpu/drm/drm_gpuvm.c
+++ b/drivers/gpu/drm/drm_gpuvm.c
@@ -655,6 +655,7 @@ drm_gpuva_range_valid(struct drm_gpuvm *gpuvm,
  /**
   * drm_gpuvm_init() - initialize a _gpuvm
   * @gpuvm: pointer to the _gpuvm to initialize
+ * @drm: the drivers _device
   * @name: the name of the GPU VA space
   * @start_offset: the start offset of the GPU VA space
   * @range: the size of the GPU VA space
@@ -668,7 +669,7 @@ drm_gpuva_range_valid(struct drm_gpuvm *gpuvm,
   *  is expected to be managed by the surrounding driver 
structures.

   */
  void
-drm_gpuvm_init(struct drm_gpuvm *gpuvm,
+drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device *drm,
 const char *name,
 u64 start_offset, u64 range,
 u64 reserve_offset, u64 reserve_range,
@@ -694,6 +695,8 @@ drm_gpuvm_init(struct drm_gpuvm *gpuvm,
   reserve_range)))
  __drm_gpuva_insert(gpuvm, >kernel_alloc_node);
  }
+
+    drm_gem_private_object_init(drm, >d_obj, 0);
  }
  EXPORT_SYMBOL_GPL(drm_gpuvm_init);
@@ -713,7 +716,9 @@ drm_gpuvm_destroy(struct drm_gpuvm *gpuvm)
__drm_gpuva_remove(>kernel_alloc_node);
  WARN(!RB_EMPTY_ROOT(>rb.tree.rb_root),
- "GPUVA tree is not empty, potentially leaking memory.");
+ "GPUVA tree is not empty, potentially leaking memory.\n");
+
+    drm_gem_private_object_fini(>d_obj);
  }
  EXPORT_SYMBOL_GPL(drm_gpuvm_destroy);
diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c 
b/drivers/gpu/drm/nouveau/nouveau_uvmm.c

index 6c86b64273c3..a80ac8767843 100644
--- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
@@ -1836,7 +1836,7 @@ nouveau_uvmm_init(struct nouveau_uvmm *uvmm, 
struct nouveau_cli *cli,

  uvmm->kernel_managed_addr = kernel_managed_addr;
  uvmm->kernel_managed_size = kernel_managed_size;
-    drm_gpuvm_init(>base, cli->name,
+    drm_gpuvm_init(>base, cli->drm->dev, cli->name,
 NOUVEAU_VA_SPACE_START,
 NOUVEAU_VA_SPACE_END,
 kernel_managed_addr, kernel_managed_size,
diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
index 0e802676e0a9..c07d7c3e 100644
--- a/include/drm/drm_gpuvm.h
+++ b/include/drm/drm_gpuvm.h
@@ -240,14 +240,29 @@ struct drm_gpuvm {
   * @ops: _gpuvm_ops providing the split/merge steps to 
drivers

   */
  const struct drm_gpuvm_ops *ops;
+
+    /**
+ * @d_obj: Dummy GEM object; used internally to pass the GPU VMs
+ * dma-resv to _exec. Provides the GPUVM's 
+ */
+    struct drm_gem_object d_obj;


Yeah, as pointed out in the other mail that won't work like this.


Which one? Seems that I missed it.



The GPUVM contains GEM objects and therefore should probably have a 
reference to those objects.


When those GEM objects now use the dma-resv object embedded inside 
the GPUVM then they also need a reference to the GPUVM to make sure 
the dma-resv object won't be freed before they are freed.


My assumption here is that GEM objects being local to a certain VM 
never out-live the VM. We never share it with anyone, otherwise it 
would be external and hence wouldn't carray the VM's dma-resv. The 
only references I see are from the VM itself (which is fine) and from 
userspace. The latter isn't a problem as long as all GEM handles are 
closed before the VM is destroyed on FD close.


Do I miss something? Do we have use cases where this isn't true?


There are multiple use cases where this isn't true. One example is 
memory management with TTM or drm_exec. The both grab references on the 
objects they lock.


Since this is eviction code it is perfectly possible that a GEM object 
is locked from a different VM then the one currently in use. So a single 
GEM object from a VM can live longer than the VM itself.


Another potential case is delayed delete where a GEM object might need 
to stay around a bit longer because of hw restrictions. This can simply 
be that we wait for shaders to end, but also hw workarounds where we 
need to wait some grace time before freeing things.







This is a circle reference dependency.

The simplest solution I can see is to let the driver provide the GEM 
object to use. Amdgpu uses the root page directory object for this.


Sure, we can do 

Re: [PATCH] drm/ssd130x: Drop _helper prefix from struct drm_*_helper_funcs callbacks

[Maxime Ripard]

On Thu, 14 Sep 2023 21:51:24 +0200, Javier Martinez Canillas wrote:
> The driver uses a naming convention where functions for struct drm_*_funcs
> callbacks are named ssd130x_$object_$operation, while the callbacks for
> struct drm_*_helper_funcs are named ssd130x_$object_helper_$operation.
> 
> The idea is that this helper_ prefix in the function names denote that are
> 
> [ ... ]

Reviewed-by: Maxime Ripard 

Thanks!
Maxime

[PATCH 5/5] drm/amd/display: Move the memory allocation out of dcn20_validate_bandwidth_fp().

[Sebastian Andrzej Siewior]

dcn20_validate_bandwidth_fp() is invoked while FPU access has been
enabled. FPU access requires disabling preemption even on PREEMPT_RT.
It is not possible to allocate memory with disabled preemption even with
GFP_ATOMIC on PREEMPT_RT.

Move the memory allocation before FPU access is enabled.
To preserve previous "clean" state of "pipes" add a memset() before the
second invocation of dcn20_validate_bandwidth_internal() where the
variable is used.

Signed-off-by: Sebastian Andrzej Siewior 
---
 .../drm/amd/display/dc/dcn20/dcn20_resource.c| 10 +-
 .../gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c | 16 +++-
 .../gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.h |  5 ++---
 3 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c 
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
index d587f807dfd7c..5036a3e608324 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
@@ -2141,9 +2141,17 @@ bool dcn20_validate_bandwidth(struct dc *dc, struct 
dc_state *context,
bool fast_validate)
 {
bool voltage_supported;
+   display_e2e_pipe_params_st *pipes;
+
+   pipes = kcalloc(dc->res_pool->pipe_count, 
sizeof(display_e2e_pipe_params_st), GFP_KERNEL);
+   if (!pipes)
+   return false;
+
DC_FP_START();
-   voltage_supported = dcn20_validate_bandwidth_fp(dc, context, 
fast_validate);
+   voltage_supported = dcn20_validate_bandwidth_fp(dc, context, 
fast_validate, pipes);
DC_FP_END();
+
+   kfree(pipes);
return voltage_supported;
 }
 
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
index 8b2038162a7e1..2ad92497b9bf2 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
@@ -1923,7 +1923,7 @@ void dcn20_patch_bounding_box(struct dc *dc, struct 
_vcs_dpi_soc_bounding_box_st
 }
 
 static bool dcn20_validate_bandwidth_internal(struct dc *dc, struct dc_state 
*context,
-   bool fast_validate)
+   bool fast_validate, display_e2e_pipe_params_st *pipes)
 {
bool out = false;
 
@@ -1932,7 +1932,6 @@ static bool dcn20_validate_bandwidth_internal(struct dc 
*dc, struct dc_state *co
int vlevel = 0;
int pipe_split_from[MAX_PIPES];
int pipe_cnt = 0;
-   display_e2e_pipe_params_st *pipes = kzalloc(dc->res_pool->pipe_count * 
sizeof(display_e2e_pipe_params_st), GFP_ATOMIC);
DC_LOGGER_INIT(dc->ctx->logger);
 
BW_VAL_TRACE_COUNT();
@@ -1967,16 +1966,14 @@ static bool dcn20_validate_bandwidth_internal(struct dc 
*dc, struct dc_state *co
out = false;
 
 validate_out:
-   kfree(pipes);
 
BW_VAL_TRACE_FINISH();
 
return out;
 }
 
-bool dcn20_validate_bandwidth_fp(struct dc *dc,
-struct dc_state *context,
-bool fast_validate)
+bool dcn20_validate_bandwidth_fp(struct dc *dc, struct dc_state *context,
+bool fast_validate, display_e2e_pipe_params_st 
*pipes)
 {
bool voltage_supported = false;
bool full_pstate_supported = false;
@@ -1995,11 +1992,11 @@ bool dcn20_validate_bandwidth_fp(struct dc *dc,
ASSERT(context != dc->current_state);
 
if (fast_validate) {
-   return dcn20_validate_bandwidth_internal(dc, context, true);
+   return dcn20_validate_bandwidth_internal(dc, context, true, 
pipes);
}
 
// Best case, we support full UCLK switch latency
-   voltage_supported = dcn20_validate_bandwidth_internal(dc, context, 
false);
+   voltage_supported = dcn20_validate_bandwidth_internal(dc, context, 
false, pipes);
full_pstate_supported = 
context->bw_ctx.bw.dcn.clk.p_state_change_support;
 
if (context->bw_ctx.dml.soc.dummy_pstate_latency_us == 0 ||
@@ -2011,7 +2008,8 @@ bool dcn20_validate_bandwidth_fp(struct dc *dc,
// Fallback: Try to only support G6 temperature read latency
context->bw_ctx.dml.soc.dram_clock_change_latency_us = 
context->bw_ctx.dml.soc.dummy_pstate_latency_us;
 
-   voltage_supported = dcn20_validate_bandwidth_internal(dc, context, 
false);
+   memset(pipes, 0, dc->res_pool->pipe_count * 
sizeof(display_e2e_pipe_params_st));
+   voltage_supported = dcn20_validate_bandwidth_internal(dc, context, 
false, pipes);
dummy_pstate_supported = 
context->bw_ctx.bw.dcn.clk.p_state_change_support;
 
if (voltage_supported && (dummy_pstate_supported || 
!(context->stream_count))) {
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.h 
b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.h
index a81a0b9e68842..b6c34198ddc86 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.h
+++ 

Re: [PATCH 06/15] mailbox: mediatek: Add cmdq_mbox_stop to disable GCE thread

[林睿祥]

[PATCH 4/5] drm/amd/display: Move the memory allocation out of dcn21_validate_bandwidth_fp().

[Sebastian Andrzej Siewior]

dcn21_validate_bandwidth_fp() is invoked while FPU access has been
enabled. FPU access requires disabling preemption even on PREEMPT_RT.
It is not possible to allocate memory with disabled preemption even with
GFP_ATOMIC on PREEMPT_RT.

Move the memory allocation before FPU access is enabled.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=217928
Signed-off-by: Sebastian Andrzej Siewior 
---
 drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c | 10 +-
 drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c  |  7 ++-
 drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.h  |  5 ++---
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c 
b/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c
index d1a25fe6c44fa..5674c3450fc36 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c
@@ -953,9 +953,17 @@ static bool dcn21_validate_bandwidth(struct dc *dc, struct 
dc_state *context,
bool fast_validate)
 {
bool voltage_supported;
+   display_e2e_pipe_params_st *pipes;
+
+   pipes = kcalloc(dc->res_pool->pipe_count, 
sizeof(display_e2e_pipe_params_st), GFP_KERNEL);
+   if (!pipes)
+   return false;
+
DC_FP_START();
-   voltage_supported = dcn21_validate_bandwidth_fp(dc, context, 
fast_validate);
+   voltage_supported = dcn21_validate_bandwidth_fp(dc, context, 
fast_validate, pipes);
DC_FP_END();
+
+   kfree(pipes);
return voltage_supported;
 }
 
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
index 5805fb02af14e..8b2038162a7e1 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
@@ -2216,9 +2216,8 @@ static void dcn21_calculate_wm(struct dc *dc, struct 
dc_state *context,
>bw_ctx.dml, pipes, 
pipe_cnt);
 }
 
-bool dcn21_validate_bandwidth_fp(struct dc *dc,
-struct dc_state *context,
-bool fast_validate)
+bool dcn21_validate_bandwidth_fp(struct dc *dc, struct dc_state *context,
+bool fast_validate, display_e2e_pipe_params_st 
*pipes)
 {
bool out = false;
 
@@ -2227,7 +2226,6 @@ bool dcn21_validate_bandwidth_fp(struct dc *dc,
int vlevel = 0;
int pipe_split_from[MAX_PIPES];
int pipe_cnt = 0;
-   display_e2e_pipe_params_st *pipes = kzalloc(dc->res_pool->pipe_count * 
sizeof(display_e2e_pipe_params_st), GFP_ATOMIC);
DC_LOGGER_INIT(dc->ctx->logger);
 
BW_VAL_TRACE_COUNT();
@@ -2267,7 +2265,6 @@ bool dcn21_validate_bandwidth_fp(struct dc *dc,
out = false;
 
 validate_out:
-   kfree(pipes);
 
BW_VAL_TRACE_FINISH();
 
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.h 
b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.h
index c51badf7b68a9..a81a0b9e68842 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.h
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.h
@@ -77,9 +77,8 @@ int dcn21_populate_dml_pipes_from_context(struct dc *dc,
  struct dc_state *context,
  display_e2e_pipe_params_st *pipes,
  bool fast_validate);
-bool dcn21_validate_bandwidth_fp(struct dc *dc,
-struct dc_state *context,
-bool fast_validate);
+bool dcn21_validate_bandwidth_fp(struct dc *dc, struct dc_state *context, bool
+fast_validate, display_e2e_pipe_params_st 
*pipes);
 void dcn21_update_bw_bounding_box(struct dc *dc, struct clk_bw_params 
*bw_params);
 
 void dcn21_clk_mgr_set_bw_params_wm_table(struct clk_bw_params *bw_params);
-- 
2.40.1

[PATCH 3/5] drm/amd/display: Add a warning if the FPU is used outside from task context.

[Sebastian Andrzej Siewior]

Add a warning if the FPU is used from any context other than task
context. This is only precaution since the code is not able to be used
from softirq while the API allows it on x86 for instance.

Signed-off-by: Sebastian Andrzej Siewior 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
index 8bd5926b47e06..4ae4720535a56 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
@@ -84,6 +84,7 @@ void dc_fpu_begin(const char *function_name, const int line)
 {
int depth;
 
+   WARN_ON_ONCE(!in_task());
preempt_disable();
depth = __this_cpu_inc_return(fpu_recursion_depth);
 
-- 
2.40.1

[PATCH 0/5] drm/amd/display: Remove migrate-disable and move memory allocation.

[Sebastian Andrzej Siewior]

Hi,

I stumbled uppon the amdgpu driver via a bugzilla report. The actual fix
is #4 + #5 and the rest was made while looking at the code.

Sebastian

[PATCH 1/5] drm/amd/display: Remove migrate_en/dis from dc_fpu_begin().

[Sebastian Andrzej Siewior]

This is a revert of the commit mentioned below while it is not wrong, as
in the kernel will explode, having migrate_disable() here it is
complete waste of resources.

Additionally commit message is plain wrong the review tag does not make
it any better. The migrate_disable() interface has a fat comment
describing it and it includes the word "undesired" in the headline which
should tickle people to read it before using it.
Initially I assumed it is worded too harsh but now I beg to differ.

The reviewer of the original commit, even not understanding what
migrate_disable() does should ask the following:

- migrate_disable() is added only to the CONFIG_X86 block and it claims
  to protect fpu_recursion_depth. Why are the other the architectures
  excluded?

- migrate_disable() is added after fpu_recursion_depth was modified.
  Shouldn't it be added before the modification or referencing takes
  place?

Moving on.
Disabling preemption DOES prevent CPU migration. A task, that can not be
pushed away from the CPU by the scheduler (due to disabled preemption)
can not be pushed or migrated to another CPU.

Disabling migration DOES NOT ensure consistency of per-CPU variables. It
only ensures that the task acts always on the same per-CPU variable. The
task remains preemptible meaning multiple tasks can access the same
per-CPU variable. This in turn leads to inconsistency for the statement

  *pcpu -= 1;

with two tasks on one CPU and a preemption point during the RMW
operation:

 Task A   Task B
 read pcpu to reg  # 0
 inc reg   # 0 -> 1
  read pcpu to reg  # 0
  inc reg   # 0 -> 1
  write reg to pcpu # 1
 write reg to pcpu # 1

At the end pcpu reads 1 but should read 2 instead. Boom.

get_cpu_ptr() already contains a preempt_disable() statement. That means
that the per-CPU variable can only be referenced by a single task which
is currently running. The only inconsistency that can occur if the
variable is additionally accessed from an interrupt.

Remove migrate_disable/enable() from dc_fpu_begin/end().

Cc: Tianci Yin 
Cc: Aurabindo Pillai 
Fixes: 0c316556d1249 ("drm/amd/display: Disable migration to ensure consistency 
of per-CPU variable")
Signed-off-by: Sebastian Andrzej Siewior 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
index 172aa10a8800f..86f4c0e046548 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
@@ -91,7 +91,6 @@ void dc_fpu_begin(const char *function_name, const int line)
 
if (*pcpu == 1) {
 #if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH)
-   migrate_disable();
kernel_fpu_begin();
 #elif defined(CONFIG_PPC64)
if (cpu_has_feature(CPU_FTR_VSX_COMP)) {
@@ -132,7 +131,6 @@ void dc_fpu_end(const char *function_name, const int line)
if (*pcpu <= 0) {
 #if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH)
kernel_fpu_end();
-   migrate_enable();
 #elif defined(CONFIG_PPC64)
if (cpu_has_feature(CPU_FTR_VSX_COMP)) {
disable_kernel_vsx();
-- 
2.40.1

[PATCH 2/5] drm/amd/display: Simplify the per-CPU usage.

[Sebastian Andrzej Siewior]

The fpu_recursion_depth counter is used to ensure that dc_fpu_begin()
can be invoked multiple times while the FPU-disable function itself is
only invoked once. Also the counter part (dc_fpu_end()) is ballanced
properly.

Instead of using the get_cpu_ptr() dance around the inc it is simpler to
increment the per-CPU variable directly. Also the per-CPU variable has
to be incremented and decremented on the same CPU. This is ensured by
the inner-part which disables preemption. This is kind of not obvious,
works and the preempt-counter is touched a few times for no reason.

Disable preemption before incrementing fpu_recursion_depth for the first
time. Keep preemption disabled until dc_fpu_end() where the counter is
decremented making it obvious that the preemption has to stay disabled
while the counter is non-zero.
Use simple inc/dec functions.
Remove the nested preempt_disable/enable functions which are now not
needed.

Signed-off-by: Sebastian Andrzej Siewior 
---
 .../gpu/drm/amd/display/amdgpu_dm/dc_fpu.c| 50 ---
 1 file changed, 20 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
index 86f4c0e046548..8bd5926b47e06 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
@@ -60,11 +60,9 @@ static DEFINE_PER_CPU(int, fpu_recursion_depth);
  */
 inline void dc_assert_fp_enabled(void)
 {
-   int *pcpu, depth = 0;
+   int depth;
 
-   pcpu = get_cpu_ptr(_recursion_depth);
-   depth = *pcpu;
-   put_cpu_ptr(_recursion_depth);
+   depth = __this_cpu_read(fpu_recursion_depth);
 
ASSERT(depth >= 1);
 }
@@ -84,32 +82,27 @@ inline void dc_assert_fp_enabled(void)
  */
 void dc_fpu_begin(const char *function_name, const int line)
 {
-   int *pcpu;
+   int depth;
 
-   pcpu = get_cpu_ptr(_recursion_depth);
-   *pcpu += 1;
+   preempt_disable();
+   depth = __this_cpu_inc_return(fpu_recursion_depth);
 
-   if (*pcpu == 1) {
+   if (depth == 1) {
 #if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH)
kernel_fpu_begin();
 #elif defined(CONFIG_PPC64)
-   if (cpu_has_feature(CPU_FTR_VSX_COMP)) {
-   preempt_disable();
+   if (cpu_has_feature(CPU_FTR_VSX_COMP))
enable_kernel_vsx();
-   } else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) {
-   preempt_disable();
+   else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP))
enable_kernel_altivec();
-   } else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) {
-   preempt_disable();
+   else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE))
enable_kernel_fp();
-   }
 #elif defined(CONFIG_ARM64)
kernel_neon_begin();
 #endif
}
 
-   TRACE_DCN_FPU(true, function_name, line, *pcpu);
-   put_cpu_ptr(_recursion_depth);
+   TRACE_DCN_FPU(true, function_name, line, depth);
 }
 
 /**
@@ -124,29 +117,26 @@ void dc_fpu_begin(const char *function_name, const int 
line)
  */
 void dc_fpu_end(const char *function_name, const int line)
 {
-   int *pcpu;
+   int depth;
 
-   pcpu = get_cpu_ptr(_recursion_depth);
-   *pcpu -= 1;
-   if (*pcpu <= 0) {
+   depth = __this_cpu_dec_return(fpu_recursion_depth);
+   if (depth == 0) {
 #if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH)
kernel_fpu_end();
 #elif defined(CONFIG_PPC64)
-   if (cpu_has_feature(CPU_FTR_VSX_COMP)) {
+   if (cpu_has_feature(CPU_FTR_VSX_COMP))
disable_kernel_vsx();
-   preempt_enable();
-   } else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) {
+   else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP))
disable_kernel_altivec();
-   preempt_enable();
-   } else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) {
+   else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE))
disable_kernel_fp();
-   preempt_enable();
-   }
 #elif defined(CONFIG_ARM64)
kernel_neon_end();
 #endif
+   } else {
+   WARN_ON_ONCE(depth < 0);
}
 
-   TRACE_DCN_FPU(false, function_name, line, *pcpu);
-   put_cpu_ptr(_recursion_depth);
+   TRACE_DCN_FPU(false, function_name, line, depth);
+   preempt_enable();
 }
-- 
2.40.1

Re: [PATCH v8 2/7] phy: Add HDMI configuration options

[Vinod Koul]

On 22-08-23, 20:22, Dmitry Baryshkov wrote:
> On 22/08/2023 16:54, Vinod Koul wrote:
> > On 17-08-23, 13:05, Dmitry Baryshkov wrote:
> >> On 08/08/2023 11:32, Sandor Yu wrote:
> >>> Allow HDMI PHYs to be configured through the generic
> >>> functions through a custom structure added to the generic union.
> >>>
> >>> The parameters added here are based on HDMI PHY
> >>> implementation practices.  The current set of parameters
> >>> should cover the potential users.
> >>>
> >>> Signed-off-by: Sandor Yu 
> >>> ---
> >>>include/linux/phy/phy-hdmi.h | 24 
> >>>include/linux/phy/phy.h  |  7 ++-
> >>>2 files changed, 30 insertions(+), 1 deletion(-)
> >>>create mode 100644 include/linux/phy/phy-hdmi.h
> >>
> >> I think this looks good now, thank you!
> >>
> >> Reviewed-by: Dmitry Baryshkov 
> >
> > Should this go thru drm or phy...?
> 
> I'd say, PHY, together with the other PHY patches. If you can merge
> them into an immutable branch, then it can also be merged into
> drm-misc (?) to provide the dependency between drm and phy parts.

phy/topic/hdmi should be pushed out in a bit for that

-- 
~Vinod

Re: [PATCH 08/31] phy: rockchip-inno-usb2: Split ID interrupt phy registers

[Vinod Koul]

On 29-08-23, 19:16, Alex Bee wrote:
> Commit 51a9b2c03dd3 ("phy: rockchip-inno-usb2: Handle ID IRQ") added ID
> detection interrupt registers. However the current implementation assumes
> that falling and rising edge interrupt are always enabled in registers
> spaning over subsequent bits.
> That is not the case for RK312x's version of the phy and this
> implementation can't be used as-is, since there are bits with different
> purpose in between.
> 
> This splits up the register definitions for id_det_en, id_det_en and
> id_det_clr registers in rising and falling edge variants.
> It's required as preparation to support RK312x's Innosilicon usb2 phy as
> well in this driver and matches pretty much to what the vendor does, so I'm
> not expecting issues for other SoCs with that change.

This fails to apply for phy/next

-- 
~Vinod

[PATCH v2] drm: renesas: rcar-du: use proper naming for R-Car

[Wolfram Sang]

Not RCAR, but R-Car.

Signed-off-by: Wolfram Sang 
Reviewed-by: Kieran Bingham 
---

Changes since v1:
* rebased to 6.6-rc2
* added tag from Kieran (Thanks!)

 drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.h 
b/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.h
index f9893d7d6dfc..e9e59c5e70d5 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.h
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.h
@@ -16,7 +16,7 @@ struct rcar_du_format_info;
 struct rcar_du_group;
 
 /*
- * The RCAR DU has 8 hardware planes, shared between primary and overlay 
planes.
+ * The R-Car DU has 8 hardware planes, shared between primary and overlay 
planes.
  * As using overlay planes requires at least one of the CRTCs being enabled, no
  * more than 7 overlay planes can be available. We thus create 1 primary plane
  * per CRTC and 7 overlay planes, for a total of up to 9 KMS planes.
-- 
2.35.1

Re: [PATCH drm-misc-next v4 4/8] drm/gpuvm: add common dma-resv per struct drm_gpuvm

[Danilo Krummrich]

On 9/21/23 09:39, Christian König wrote:

Am 20.09.23 um 16:42 schrieb Danilo Krummrich:

Provide a common dma-resv for GEM objects not being used outside of this
GPU-VM. This is used in a subsequent patch to generalize dma-resv,
external and evicted object handling and GEM validation.

Signed-off-by: Danilo Krummrich 
---
  drivers/gpu/drm/drm_gpuvm.c    |  9 +++--
  drivers/gpu/drm/nouveau/nouveau_uvmm.c |  2 +-
  include/drm/drm_gpuvm.h    | 17 -
  3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
index bfea4a8a19ec..cbf4b738a16c 100644
--- a/drivers/gpu/drm/drm_gpuvm.c
+++ b/drivers/gpu/drm/drm_gpuvm.c
@@ -655,6 +655,7 @@ drm_gpuva_range_valid(struct drm_gpuvm *gpuvm,
  /**
   * drm_gpuvm_init() - initialize a _gpuvm
   * @gpuvm: pointer to the _gpuvm to initialize
+ * @drm: the drivers _device
   * @name: the name of the GPU VA space
   * @start_offset: the start offset of the GPU VA space
   * @range: the size of the GPU VA space
@@ -668,7 +669,7 @@ drm_gpuva_range_valid(struct drm_gpuvm *gpuvm,
   *  is expected to be managed by the surrounding driver structures.
   */
  void
-drm_gpuvm_init(struct drm_gpuvm *gpuvm,
+drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device *drm,
 const char *name,
 u64 start_offset, u64 range,
 u64 reserve_offset, u64 reserve_range,
@@ -694,6 +695,8 @@ drm_gpuvm_init(struct drm_gpuvm *gpuvm,
   reserve_range)))
  __drm_gpuva_insert(gpuvm, >kernel_alloc_node);
  }
+
+    drm_gem_private_object_init(drm, >d_obj, 0);
  }
  EXPORT_SYMBOL_GPL(drm_gpuvm_init);
@@ -713,7 +716,9 @@ drm_gpuvm_destroy(struct drm_gpuvm *gpuvm)
  __drm_gpuva_remove(>kernel_alloc_node);
  WARN(!RB_EMPTY_ROOT(>rb.tree.rb_root),
- "GPUVA tree is not empty, potentially leaking memory.");
+ "GPUVA tree is not empty, potentially leaking memory.\n");
+
+    drm_gem_private_object_fini(>d_obj);
  }
  EXPORT_SYMBOL_GPL(drm_gpuvm_destroy);
diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c 
b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
index 6c86b64273c3..a80ac8767843 100644
--- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
@@ -1836,7 +1836,7 @@ nouveau_uvmm_init(struct nouveau_uvmm *uvmm, struct 
nouveau_cli *cli,
  uvmm->kernel_managed_addr = kernel_managed_addr;
  uvmm->kernel_managed_size = kernel_managed_size;
-    drm_gpuvm_init(>base, cli->name,
+    drm_gpuvm_init(>base, cli->drm->dev, cli->name,
 NOUVEAU_VA_SPACE_START,
 NOUVEAU_VA_SPACE_END,
 kernel_managed_addr, kernel_managed_size,
diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
index 0e802676e0a9..c07d7c3e 100644
--- a/include/drm/drm_gpuvm.h
+++ b/include/drm/drm_gpuvm.h
@@ -240,14 +240,29 @@ struct drm_gpuvm {
   * @ops: _gpuvm_ops providing the split/merge steps to drivers
   */
  const struct drm_gpuvm_ops *ops;
+
+    /**
+ * @d_obj: Dummy GEM object; used internally to pass the GPU VMs
+ * dma-resv to _exec. Provides the GPUVM's 
+ */
+    struct drm_gem_object d_obj;


Yeah, as pointed out in the other mail that won't work like this.


Which one? Seems that I missed it.



The GPUVM contains GEM objects and therefore should probably have a reference 
to those objects.

When those GEM objects now use the dma-resv object embedded inside the GPUVM 
then they also need a reference to the GPUVM to make sure the dma-resv object 
won't be freed before they are freed.


My assumption here is that GEM objects being local to a certain VM never 
out-live the VM. We never share it with anyone, otherwise it would be external 
and hence wouldn't carray the VM's dma-resv. The only references I see are from 
the VM itself (which is fine) and from userspace. The latter isn't a problem as 
long as all GEM handles are closed before the VM is destroyed on FD close.

Do I miss something? Do we have use cases where this isn't true?



This is a circle reference dependency.

The simplest solution I can see is to let the driver provide the GEM object to 
use. Amdgpu uses the root page directory object for this.


Sure, we can do that, if we see cases where VM local GEM objects can out-live 
the VM.



Apart from that I strongly think that we shouldn't let the GPUVM code create a 
driver GEM object. We did that in TTM for the ghost objects and it turned out 
to be a bad idea.


You mean let GPUVM create a dummy GEM based on the drm_device from the driver? 
What were the problems that were encountered?

- Danilo



Regards,
Christian.


  };
-void drm_gpuvm_init(struct drm_gpuvm *gpuvm, const char *name,
+void drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device *drm,
+    const char *name,
  u64 start_offset, u64 range,
  u64 reserve_offset, u64 reserve_range,

Re: [PATCH] drm/ssd130x: Drop _helper prefix from struct drm_*_helper_funcs callbacks

[Maxime Ripard]

Hi,

On Thu, Sep 21, 2023 at 10:52:14AM +0200, Thomas Zimmermann wrote:
> Am 21.09.23 um 09:44 schrieb Maxime Ripard:
> > On Mon, Sep 18, 2023 at 09:19:07AM +0200, Javier Martinez Canillas wrote:
> > > Thomas Zimmermann  writes:
> > > 
> > > > Hi
> > > > 
> > > > Am 14.09.23 um 21:51 schrieb Javier Martinez Canillas:
> > > > > The driver uses a naming convention where functions for struct 
> > > > > drm_*_funcs
> > > > > callbacks are named ssd130x_$object_$operation, while the callbacks 
> > > > > for
> > > > > struct drm_*_helper_funcs are named ssd130x_$object_helper_$operation.
> > > > > 
> > > > > The idea is that this helper_ prefix in the function names denote 
> > > > > that are
> > > > > for struct drm_*_helper_funcs callbacks. This convention was copied 
> > > > > from
> > > > > other drivers, when ssd130x was written but Maxime pointed out that 
> > > > > is the
> > > > > exception rather than the norm.
> > > > 
> > > > I guess you found this in my code. I want to point out that I use the
> > > > _helper infix to signal that these are callback for
> > > > drm_primary_plane_helper_funcs and *not* drm_primary_plane_funcs. The
> > > > naming is intentional.
> > > > 
> > > 
> > > Yes, that's what tried to say in the commit message and indeed I got the
> > > convention from drivers in drivers/gpu/drm/tiny. In fact I believe these
> > > function names are since first iteration of the driver, when was meant to
> > > be a tiny driver.
> > > 
> > > According to Maxime it's the exception rather than the rule and suggested
> > > to change it, I don't really have a strong opinion on either naming TBH.
> > 
> > Maybe that's just me, but the helper in the name indeed throws me off. In my
> > mind, it's supposed to be used only for helpers, not functions implementing 
> > the
> > helpers hooks.
> 
> Tying the function name to its _funcs structure makes perfect sense to me,
> as it helps to structure the driver code. So I always use the _helper_
> infix.
> 
> In contrast, the DRM helpers that implement certain functionality does not
> seem to follow any naming scheme. For example drm_atomic_helper_check()
> implements struct drm_mode_config_funcs.atomic_check. To me, it's not
> obvious that these two belong together.

Right, but then we end up with functions that have helpers in their name
and are indeed helpers, and then we have functions that have helpers in
their name but are not meant to help anyone or be reused anywhere else.

> And in the same structure, there's fb_create, which is provided by
> drm_gem_fb_create_with_dirty(). This one doesn't even mention that
> it's a helper.

We should fix that then I guess?

Anyway, we're way past bikeshedding there :)

Maxime


signature.asc
Description: PGP signature

Re: [PATCH 5.4 000/367] 5.4.257-rc1 review

[Sui Jingfeng]

Hi,


On 2023/9/21 21:10, Sui Jingfeng wrote:

return -ERR_PTR(-ENOMEM)



return ERR_PTR(-ENOMEM);

[PATCH v2] drm/dp_mst: Fix NULL deref in get_mst_branch_device_by_guid_helper()

[Lukasz Majczak]

As drm_dp_get_mst_branch_device_by_guid() is called from
drm_dp_get_mst_branch_device_by_guid(), we need to check mstb parameter,
Check mstb parameter, otherwise NULL dereference may occur in
the call to memcpy() and cause following:

[12579.365869] BUG: kernel NULL pointer dereference, address: 0049
[12579.365878] #PF: supervisor read access in kernel mode
[12579.365880] #PF: error_code(0x) - not-present page
[12579.365882] PGD 0 P4D 0
[12579.365887] Oops:  [#1] PREEMPT SMP NOPTI
...
[12579.365895] Workqueue: events_long drm_dp_mst_up_req_work
[12579.365899] RIP: 0010:memcmp+0xb/0x29
[12579.365921] Call Trace:
[12579.365927] get_mst_branch_device_by_guid_helper+0x22/0x64
[12579.365930] drm_dp_mst_up_req_work+0x137/0x416
[12579.365933] process_one_work+0x1d0/0x419
[12579.365935] worker_thread+0x11a/0x289
[12579.365938] kthread+0x13e/0x14f
[12579.365941] ? process_one_work+0x419/0x419
[12579.365943] ? kthread_blkcg+0x31/0x31
[12579.365946] ret_from_fork+0x1f/0x30

As get_mst_branch_device_by_guid_helper() is recursive, moving condition
to the first line allow to remove a similar one for step over of NULL elements
inside a loop.

Fixes: 5e93b8208d3c ("drm/dp/mst: move GUID storage from mgr, port to only mst 
branch")
Cc:  # 4.14+
Signed-off-by: Lukasz Majczak 
---
 drivers/gpu/drm/display/drm_dp_mst_topology.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c 
b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index ed96cfcfa304..8c929ef72c72 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -2574,14 +2574,14 @@ static struct drm_dp_mst_branch 
*get_mst_branch_device_by_guid_helper(
struct drm_dp_mst_branch *found_mstb;
struct drm_dp_mst_port *port;
 
+   if (!mstb)
+   return NULL;
+
if (memcmp(mstb->guid, guid, 16) == 0)
return mstb;
 
 
list_for_each_entry(port, >ports, next) {
-   if (!port->mstb)
-   continue;
-
found_mstb = get_mst_branch_device_by_guid_helper(port->mstb, 
guid);
 
if (found_mstb)
-- 
2.42.0.515.g380fc7ccd1-goog

Re: [PATCH 5.4 000/367] 5.4.257-rc1 review

[Sui Jingfeng]

Hi,


On 2023/9/21 20:08, Naresh Kamboju wrote:

On Wed, 20 Sept 2023 at 14:25, Greg Kroah-Hartman
 wrote:

This is the start of the stable review cycle for the 5.4.257 release.
There are 367 patches in this series, all will be posted as a response
to this one.  If anyone has any issues with these being applied, please
let me know.

Responses should be made by Fri, 22 Sep 2023 11:28:09 +.
Anything received after that time might be too late.

The whole patch series can be found in one patch at:
 
https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.4.257-rc1.gz
or in the git tree and branch at:
 
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
linux-5.4.y
and the diffstat can be found below.

thanks,

greg k-h

Following build warnings noticed while building arm64 with allmodconfig
on stable-rc 5.4 with gcc-8 and gcc-12 toolchains.

Reported-by: Linux Kernel Functional Testing 

drivers/gpu/drm/mediatek/mtk_drm_gem.c: In function 'mtk_drm_gem_prime_vmap':
drivers/gpu/drm/mediatek/mtk_drm_gem.c:273:10: warning: returning
'int' from a function with return type 'void *' makes pointer from
integer without a cast [-Wint-conversion]
return -ENOMEM;
   ^



Well, this is easy to solve.
For the Linux-5.4 kernel, we should use "return -ERR_PTR(-ENOMEM)" instead of 
"return -ENOMEM".
Since, newer version kernel prefer to return error code instead of error 
pointer.
See below commit for more information.

commit <7e542ff8b463>  ("drm/mediatek: Use struct dma_buf_map in GEM 
vmap ops")
commit <49a3f51dfeee> ("drm/gem: Use struct dma_buf_map in GEM vmap ops 
and convert GEM backends")




Links:
  - 
https://storage.tuxsuite.com/public/linaro/lkft/builds/2VfG47LmPH9MUEuIcMVftu6NsFy/


Following commit is causing this build warning.

drm/mediatek: Fix potential memory leak if vmap() fail
[ Upstream commit 379091e0f6d179d1a084c65de90fa44583b14a70 ]

--
Linaro LKFT
https://lkft.linaro.org

Re: [PATCH 7/8] drm/i915/dsc: Add debugfs entry to validate DSC fractional bpp

[Sharma, Swati2]

On 21-Sep-23 5:44 PM, Jani Nikula wrote:

On Thu, 21 Sep 2023, "Sharma, Swati2"  wrote:

On 21-Sep-23 1:30 PM, Jani Nikula wrote:

On Wed, 13 Sep 2023, Mitul Golani  wrote:

From: Swati Sharma 

DSC_Sink_BPP_Precision entry is added to i915_dsc_fec_support_show
to depict sink's precision.
Also, new debugfs entry is created to enforce fractional bpp.
If Force_DSC_Fractional_BPP_en is set then while iterating over
output bpp with fractional step size we will continue if output_bpp is
computed as integer. With this approach, we will be able to validate
DSC with fractional bpp.

v2:
Add drm_modeset_unlock to new line(Suraj)

Signed-off-by: Swati Sharma 
Signed-off-by: Ankit Nautiyal 
Signed-off-by: Mitul Golani 
Reviewed-by: Suraj Kandpal 
---
   .../drm/i915/display/intel_display_debugfs.c  | 83 +++
   .../drm/i915/display/intel_display_types.h|  1 +
   2 files changed, 84 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c 
b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index f05b52381a83..776ab96def1f 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1244,6 +1244,8 @@ static int i915_dsc_fec_support_show(struct seq_file *m, 
void *data)
  
DP_DSC_YCbCr420_Native)),
   
str_yes_no(drm_dp_dsc_sink_supports_format(intel_dp->dsc_dpcd,
  
DP_DSC_YCbCr444)));
+   seq_printf(m, "DSC_Sink_BPP_Precision: %d\n",
+  drm_dp_dsc_sink_bpp_incr(intel_dp->dsc_dpcd));
seq_printf(m, "Force_DSC_Enable: %s\n",
   str_yes_no(intel_dp->force_dsc_en));
if (!intel_dp_is_edp(intel_dp))
@@ -1436,6 +1438,84 @@ static const struct file_operations 
i915_dsc_output_format_fops = {
.write = i915_dsc_output_format_write
   };
   
+static int i915_dsc_fractional_bpp_show(struct seq_file *m, void *data)

+{
+   struct drm_connector *connector = m->private;
+   struct drm_device *dev = connector->dev;
+   struct drm_crtc *crtc;
+   struct intel_dp *intel_dp;
+   struct intel_encoder *encoder = 
intel_attached_encoder(to_intel_connector(connector));
+   int ret;
+
+   if (!encoder)
+   return -ENODEV;
+
+   ret = 
drm_modeset_lock_single_interruptible(>mode_config.connection_mutex);
+   if (ret)
+   return ret;
+
+   crtc = connector->state->crtc;
+   if (connector->status != connector_status_connected || !crtc) {
+   ret = -ENODEV;
+   goto out;
+   }
+
+   intel_dp = intel_attached_dp(to_intel_connector(connector));
+   seq_printf(m, "Force_DSC_Fractional_BPP_Enable: %s\n",
+  str_yes_no(intel_dp->force_dsc_fractional_bpp_en));


Why "Force_DSC_Fractional_BPP_Enable" in the output?

Usually debugfs files, like sysfs files, for stuff like this should be
attributes, one thing per file. Why print a long name for it, if the
name of the debugfs file is the name of the attribute?

And even if you print it for humans, why the underscores?


Hi Jani,
Followed same strategy as we are doing for other dsc scenarios like
force_dsc.
Even naming convention followed same as other dsc stuff like
Force_DSC_Enable, etc.
All DSC related enteries have underscores in its naming convention.


There's value in that, though maybe my comment highlights I'm not fond
of the existing stuff. ;)


Sure, I can work on cleanup part later.




May be i can consolidate other dsc debugfs enteries into
one as a cleanup task later. But it will impact IGT aswell. And i'm not
sure if we can break compatibility but since IGT (intel as only vendor)
is the only consumer, may be we change at both places and clean it up.


We can do what we want with debugfs, as long as we change both the
driver and igt.


Sure, will make corresponding changes in both IGT and KMD.








+
+out:
+   drm_modeset_unlock(>mode_config.connection_mutex);
+
+   return ret;
+}
+
+static ssize_t i915_dsc_fractional_bpp_write(struct file *file,
+const char __user *ubuf,
+size_t len, loff_t *offp)
+{
+   struct drm_connector *connector =
+   ((struct seq_file *)file->private_data)->private;


I know this is copy-pasted from elsewhere, but really it's nicer to
avoid the cast, and copy-paste from the places that get this right:

struct seq_file *m = file->private_data;
  struct drm_connector *connector = m->private;


Done.




+   struct intel_encoder *encoder = 
intel_attached_encoder(to_intel_connector(connector));
+   struct drm_i915_private *i915 = to_i915(encoder->base.dev);
+   struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+   bool 

Re: (subset) [PATCH] MAINTAINERS: Update gma500 git repo

[Maxime Ripard]

On Thu, 21 Sep 2023 13:00:38 +0200, Maxime Ripard wrote:
> The GMA500 driver has been handled through drm-misc for a while but the
> git repo hasn't been updated. Make sure it points to the right place.
> 
> 

Applied to drm/drm-misc (drm-misc-next).

Thanks!
Maxime

Re: [PATCH 7/8] drm/i915/dsc: Add debugfs entry to validate DSC fractional bpp

[Jani Nikula]

On Thu, 21 Sep 2023, "Sharma, Swati2"  wrote:
> On 21-Sep-23 1:30 PM, Jani Nikula wrote:
>> On Wed, 13 Sep 2023, Mitul Golani  
>> wrote:
>>> From: Swati Sharma 
>>>
>>> DSC_Sink_BPP_Precision entry is added to i915_dsc_fec_support_show
>>> to depict sink's precision.
>>> Also, new debugfs entry is created to enforce fractional bpp.
>>> If Force_DSC_Fractional_BPP_en is set then while iterating over
>>> output bpp with fractional step size we will continue if output_bpp is
>>> computed as integer. With this approach, we will be able to validate
>>> DSC with fractional bpp.
>>>
>>> v2:
>>> Add drm_modeset_unlock to new line(Suraj)
>>>
>>> Signed-off-by: Swati Sharma 
>>> Signed-off-by: Ankit Nautiyal 
>>> Signed-off-by: Mitul Golani 
>>> Reviewed-by: Suraj Kandpal 
>>> ---
>>>   .../drm/i915/display/intel_display_debugfs.c  | 83 +++
>>>   .../drm/i915/display/intel_display_types.h|  1 +
>>>   2 files changed, 84 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c 
>>> b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>>> index f05b52381a83..776ab96def1f 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>>> @@ -1244,6 +1244,8 @@ static int i915_dsc_fec_support_show(struct seq_file 
>>> *m, void *data)
>>>   
>>> DP_DSC_YCbCr420_Native)),
>>>
>>> str_yes_no(drm_dp_dsc_sink_supports_format(intel_dp->dsc_dpcd,
>>>   
>>> DP_DSC_YCbCr444)));
>>> +   seq_printf(m, "DSC_Sink_BPP_Precision: %d\n",
>>> +  drm_dp_dsc_sink_bpp_incr(intel_dp->dsc_dpcd));
>>> seq_printf(m, "Force_DSC_Enable: %s\n",
>>>str_yes_no(intel_dp->force_dsc_en));
>>> if (!intel_dp_is_edp(intel_dp))
>>> @@ -1436,6 +1438,84 @@ static const struct file_operations 
>>> i915_dsc_output_format_fops = {
>>> .write = i915_dsc_output_format_write
>>>   };
>>>   
>>> +static int i915_dsc_fractional_bpp_show(struct seq_file *m, void *data)
>>> +{
>>> +   struct drm_connector *connector = m->private;
>>> +   struct drm_device *dev = connector->dev;
>>> +   struct drm_crtc *crtc;
>>> +   struct intel_dp *intel_dp;
>>> +   struct intel_encoder *encoder = 
>>> intel_attached_encoder(to_intel_connector(connector));
>>> +   int ret;
>>> +
>>> +   if (!encoder)
>>> +   return -ENODEV;
>>> +
>>> +   ret = 
>>> drm_modeset_lock_single_interruptible(>mode_config.connection_mutex);
>>> +   if (ret)
>>> +   return ret;
>>> +
>>> +   crtc = connector->state->crtc;
>>> +   if (connector->status != connector_status_connected || !crtc) {
>>> +   ret = -ENODEV;
>>> +   goto out;
>>> +   }
>>> +
>>> +   intel_dp = intel_attached_dp(to_intel_connector(connector));
>>> +   seq_printf(m, "Force_DSC_Fractional_BPP_Enable: %s\n",
>>> +  str_yes_no(intel_dp->force_dsc_fractional_bpp_en));
>> 
>> Why "Force_DSC_Fractional_BPP_Enable" in the output?
>> 
>> Usually debugfs files, like sysfs files, for stuff like this should be
>> attributes, one thing per file. Why print a long name for it, if the
>> name of the debugfs file is the name of the attribute?
>> 
>> And even if you print it for humans, why the underscores?
>
> Hi Jani,
> Followed same strategy as we are doing for other dsc scenarios like
> force_dsc.
> Even naming convention followed same as other dsc stuff like 
> Force_DSC_Enable, etc.
> All DSC related enteries have underscores in its naming convention.

There's value in that, though maybe my comment highlights I'm not fond
of the existing stuff. ;)

> May be i can consolidate other dsc debugfs enteries into
> one as a cleanup task later. But it will impact IGT aswell. And i'm not 
> sure if we can break compatibility but since IGT (intel as only vendor) 
> is the only consumer, may be we change at both places and clean it up.

We can do what we want with debugfs, as long as we change both the
driver and igt.

>
>> 
>>> +
>>> +out:
>>> +   drm_modeset_unlock(>mode_config.connection_mutex);
>>> +
>>> +   return ret;
>>> +}
>>> +
>>> +static ssize_t i915_dsc_fractional_bpp_write(struct file *file,
>>> +const char __user *ubuf,
>>> +size_t len, loff_t *offp)
>>> +{
>>> +   struct drm_connector *connector =
>>> +   ((struct seq_file *)file->private_data)->private;
>> 
>> I know this is copy-pasted from elsewhere, but really it's nicer to
>> avoid the cast, and copy-paste from the places that get this right:
>> 
>>  struct seq_file *m = file->private_data;
>>  struct drm_connector *connector = m->private;
>
> Done.
>
>> 
>>> +   struct intel_encoder *encoder = 
>>> intel_attached_encoder(to_intel_connector(connector));
>>> +   struct drm_i915_private *i915 = 

Re: [PATCH] drm/bridge: Add 200ms delay to wait FW HPD status stable

[Laurent Pinchart]

The subject line is missing the driver name.

On Thu, Sep 21, 2023 at 03:09:10PM +0300, Jani Nikula wrote:
> On Thu, 21 Sep 2023, Xin Ji  wrote:
> > For the none-interrupt design(sink device is panel, polling HPD

s/none-interrupt/no-interrupt/ ?

s/design/design /

> > status when chip power on), anx7625 FW has more than 200ms HPD
> > de-bounce time in FW, for the safety to get HPD status, driver
> > better to wait 200ms before HPD detection after OS resume back.
> >
> > Signed-off-by: Xin Ji 
> > ---
> >  drivers/gpu/drm/bridge/analogix/anx7625.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c 
> > b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > index 51abe42c639e..833d6d50a03d 100644
> > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > @@ -1464,6 +1464,9 @@ static int _anx7625_hpd_polling(struct anx7625_data 
> > *ctx,
> > if (ctx->pdata.intp_irq)
> > return 0;
> >  
> > +   /* Delay 200ms for FW HPD de-bounce */
> > +   usleep_range(20, 201000);
> 
> If you need to sleep for 200 ms, maybe use msleep instead?

fsleep() could be a nice replacement.

> > +
> > ret = readx_poll_timeout(anx7625_read_hpd_status_p0,
> >  ctx, val,
> >  ((val & HPD_STATUS) || (val < 0)),

-- 
Regards,

Laurent Pinchart

Re: [PATCH] drm/bridge: Add 200ms delay to wait FW HPD status stable

[Jani Nikula]

On Thu, 21 Sep 2023, Xin Ji  wrote:
> For the none-interrupt design(sink device is panel, polling HPD
> status when chip power on), anx7625 FW has more than 200ms HPD
> de-bounce time in FW, for the safety to get HPD status, driver
> better to wait 200ms before HPD detection after OS resume back.
>
> Signed-off-by: Xin Ji 
> ---
>  drivers/gpu/drm/bridge/analogix/anx7625.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c 
> b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index 51abe42c639e..833d6d50a03d 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -1464,6 +1464,9 @@ static int _anx7625_hpd_polling(struct anx7625_data 
> *ctx,
>   if (ctx->pdata.intp_irq)
>   return 0;
>  
> + /* Delay 200ms for FW HPD de-bounce */
> + usleep_range(20, 201000);

If you need to sleep for 200 ms, maybe use msleep instead?

BR,
Jani.

> +
>   ret = readx_poll_timeout(anx7625_read_hpd_status_p0,
>ctx, val,
>((val & HPD_STATUS) || (val < 0)),

-- 
Jani Nikula, Intel

Re: [PATCH 5.4 000/367] 5.4.257-rc1 review

[Naresh Kamboju]

On Wed, 20 Sept 2023 at 14:25, Greg Kroah-Hartman
 wrote:
>
> This is the start of the stable review cycle for the 5.4.257 release.
> There are 367 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
>
> Responses should be made by Fri, 22 Sep 2023 11:28:09 +.
> Anything received after that time might be too late.
>
> The whole patch series can be found in one patch at:
> 
> https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.4.257-rc1.gz
> or in the git tree and branch at:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> linux-5.4.y
> and the diffstat can be found below.
>
> thanks,
>
> greg k-h

Following build warnings noticed while building arm64 with allmodconfig
on stable-rc 5.4 with gcc-8 and gcc-12 toolchains.

Reported-by: Linux Kernel Functional Testing 

drivers/gpu/drm/mediatek/mtk_drm_gem.c: In function 'mtk_drm_gem_prime_vmap':
drivers/gpu/drm/mediatek/mtk_drm_gem.c:273:10: warning: returning
'int' from a function with return type 'void *' makes pointer from
integer without a cast [-Wint-conversion]
   return -ENOMEM;
  ^

Links:
 - 
https://storage.tuxsuite.com/public/linaro/lkft/builds/2VfG47LmPH9MUEuIcMVftu6NsFy/


Following commit is causing this build warning.

drm/mediatek: Fix potential memory leak if vmap() fail
[ Upstream commit 379091e0f6d179d1a084c65de90fa44583b14a70 ]

--
Linaro LKFT
https://lkft.linaro.org

Re: [PATCH 7/8] drm/i915/dsc: Add debugfs entry to validate DSC fractional bpp

[Sharma, Swati2]

On 21-Sep-23 1:30 PM, Jani Nikula wrote:

On Wed, 13 Sep 2023, Mitul Golani  wrote:

From: Swati Sharma 

DSC_Sink_BPP_Precision entry is added to i915_dsc_fec_support_show
to depict sink's precision.
Also, new debugfs entry is created to enforce fractional bpp.
If Force_DSC_Fractional_BPP_en is set then while iterating over
output bpp with fractional step size we will continue if output_bpp is
computed as integer. With this approach, we will be able to validate
DSC with fractional bpp.

v2:
Add drm_modeset_unlock to new line(Suraj)

Signed-off-by: Swati Sharma 
Signed-off-by: Ankit Nautiyal 
Signed-off-by: Mitul Golani 
Reviewed-by: Suraj Kandpal 
---
  .../drm/i915/display/intel_display_debugfs.c  | 83 +++
  .../drm/i915/display/intel_display_types.h|  1 +
  2 files changed, 84 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c 
b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index f05b52381a83..776ab96def1f 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1244,6 +1244,8 @@ static int i915_dsc_fec_support_show(struct seq_file *m, 
void *data)
  
DP_DSC_YCbCr420_Native)),
   
str_yes_no(drm_dp_dsc_sink_supports_format(intel_dp->dsc_dpcd,
  
DP_DSC_YCbCr444)));
+   seq_printf(m, "DSC_Sink_BPP_Precision: %d\n",
+  drm_dp_dsc_sink_bpp_incr(intel_dp->dsc_dpcd));
seq_printf(m, "Force_DSC_Enable: %s\n",
   str_yes_no(intel_dp->force_dsc_en));
if (!intel_dp_is_edp(intel_dp))
@@ -1436,6 +1438,84 @@ static const struct file_operations 
i915_dsc_output_format_fops = {
.write = i915_dsc_output_format_write
  };
  
+static int i915_dsc_fractional_bpp_show(struct seq_file *m, void *data)

+{
+   struct drm_connector *connector = m->private;
+   struct drm_device *dev = connector->dev;
+   struct drm_crtc *crtc;
+   struct intel_dp *intel_dp;
+   struct intel_encoder *encoder = 
intel_attached_encoder(to_intel_connector(connector));
+   int ret;
+
+   if (!encoder)
+   return -ENODEV;
+
+   ret = 
drm_modeset_lock_single_interruptible(>mode_config.connection_mutex);
+   if (ret)
+   return ret;
+
+   crtc = connector->state->crtc;
+   if (connector->status != connector_status_connected || !crtc) {
+   ret = -ENODEV;
+   goto out;
+   }
+
+   intel_dp = intel_attached_dp(to_intel_connector(connector));
+   seq_printf(m, "Force_DSC_Fractional_BPP_Enable: %s\n",
+  str_yes_no(intel_dp->force_dsc_fractional_bpp_en));


Why "Force_DSC_Fractional_BPP_Enable" in the output?

Usually debugfs files, like sysfs files, for stuff like this should be
attributes, one thing per file. Why print a long name for it, if the
name of the debugfs file is the name of the attribute?

And even if you print it for humans, why the underscores?


Hi Jani,
Followed same strategy as we are doing for other dsc scenarios like
force_dsc.
Even naming convention followed same as other dsc stuff like 
Force_DSC_Enable, etc.

All DSC related enteries have underscores in its naming convention.

May be i can consolidate other dsc debugfs enteries into
one as a cleanup task later. But it will impact IGT aswell. And i'm not 
sure if we can break compatibility but since IGT (intel as only vendor) 
is the only consumer, may be we change at both places and clean it up.





+
+out:
+   drm_modeset_unlock(>mode_config.connection_mutex);
+
+   return ret;
+}
+
+static ssize_t i915_dsc_fractional_bpp_write(struct file *file,
+const char __user *ubuf,
+size_t len, loff_t *offp)
+{
+   struct drm_connector *connector =
+   ((struct seq_file *)file->private_data)->private;


I know this is copy-pasted from elsewhere, but really it's nicer to
avoid the cast, and copy-paste from the places that get this right:

struct seq_file *m = file->private_data;
 struct drm_connector *connector = m->private;


Done.




+   struct intel_encoder *encoder = 
intel_attached_encoder(to_intel_connector(connector));
+   struct drm_i915_private *i915 = to_i915(encoder->base.dev);
+   struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+   bool dsc_fractional_bpp_enable = false;
+   int ret;
+
+   if (len == 0)
+   return 0;


kstrtobool_from_user() has this covered.


Done.




+
+   drm_dbg(>drm,
+   "Copied %zu bytes from user to force fractional bpp for DSC\n", 
len);


That's useless.


Done.




+
+   ret = kstrtobool_from_user(ubuf, len, _fractional_bpp_enable);
+   if (ret < 0)
+   

[PATCH 5/5] drm/i915: Implement fdinfo memory stats printing

[Tvrtko Ursulin]

From: Tvrtko Ursulin 

Use the newly added drm_print_memory_stats helper to show memory
utilisation of our objects in drm/driver specific fdinfo output.

To collect the stats we walk the per memory regions object lists
and accumulate object size into the respective drm_memory_stats
categories.

Objects with multiple possible placements are reported in multiple
regions for total and shared sizes, while other categories are
counted only for the currently active region.

v2:
 * Only account against the active region.
 * Use DMA_RESV_USAGE_BOOKKEEP when testing for active. (Tejas)

Signed-off-by: Tvrtko Ursulin 
Cc: Aravind Iddamsetty 
Cc: Rob Clark 
Cc: Andi Shyti 
Cc: Tejas Upadhyay 
Reviewed-by: Andi Shyti  # v1
---
 drivers/gpu/drm/i915/i915_drm_client.c | 64 ++
 1 file changed, 64 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drm_client.c 
b/drivers/gpu/drm/i915/i915_drm_client.c
index a61356012df8..94abc2fb2ea6 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -45,6 +45,68 @@ void __i915_drm_client_free(struct kref *kref)
 }
 
 #ifdef CONFIG_PROC_FS
+static void
+obj_meminfo(struct drm_i915_gem_object *obj,
+   struct drm_memory_stats stats[INTEL_REGION_UNKNOWN])
+{
+   const enum intel_region_id id = obj->mm.region ?
+   obj->mm.region->id : INTEL_REGION_SMEM;
+   const u64 sz = obj->base.size;
+
+   if (obj->base.handle_count > 1)
+   stats[id].shared += sz;
+   else
+   stats[id].private += sz;
+
+   if (i915_gem_object_has_pages(obj)) {
+   stats[id].resident += sz;
+
+   if (!dma_resv_test_signaled(obj->base.resv,
+   DMA_RESV_USAGE_BOOKKEEP))
+   stats[id].active += sz;
+   else if (i915_gem_object_is_shrinkable(obj) &&
+obj->mm.madv == I915_MADV_DONTNEED)
+   stats[id].purgeable += sz;
+   }
+}
+
+static void show_meminfo(struct drm_printer *p, struct drm_file *file)
+{
+   struct drm_memory_stats stats[INTEL_REGION_UNKNOWN] = {};
+   struct drm_i915_file_private *fpriv = file->driver_priv;
+   struct i915_drm_client *client = fpriv->client;
+   struct drm_i915_private *i915 = fpriv->i915;
+   struct drm_i915_gem_object *obj;
+   struct intel_memory_region *mr;
+   struct list_head *pos;
+   unsigned int id;
+
+   /* Public objects. */
+   spin_lock(>table_lock);
+   idr_for_each_entry(>object_idr, obj, id)
+   obj_meminfo(obj, stats);
+   spin_unlock(>table_lock);
+
+   /* Internal objects. */
+   rcu_read_lock();
+   list_for_each_rcu(pos, >objects_list) {
+   obj = i915_gem_object_get_rcu(list_entry(pos, typeof(*obj),
+client_link));
+   if (!obj)
+   continue;
+   obj_meminfo(obj, stats);
+   i915_gem_object_put(obj);
+   }
+   rcu_read_unlock();
+
+   for_each_memory_region(mr, i915, id)
+   drm_print_memory_stats(p,
+  [id],
+  DRM_GEM_OBJECT_RESIDENT |
+  DRM_GEM_OBJECT_PURGEABLE,
+  mr->name);
+}
+
 static const char * const uabi_class_names[] = {
[I915_ENGINE_CLASS_RENDER] = "render",
[I915_ENGINE_CLASS_COPY] = "copy",
@@ -106,6 +168,8 @@ void i915_drm_client_fdinfo(struct drm_printer *p, struct 
drm_file *file)
 * **
 */
 
+   show_meminfo(p, file);
+
if (GRAPHICS_VER(i915) < 8)
return;
 
-- 
2.39.2

[PATCH 2/5] drm/i915: Record which client owns a VM

[Tvrtko Ursulin]

From: Tvrtko Ursulin 

To enable accounting of indirect client memory usage (such as page tables)
in the following patch, lets start recording the creator of each PPGTT.

Signed-off-by: Tvrtko Ursulin 
Reviewed-by: Aravind Iddamsetty 
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c   | 11 ---
 drivers/gpu/drm/i915/gem/i915_gem_context_types.h |  3 +++
 drivers/gpu/drm/i915/gem/selftests/mock_context.c |  4 ++--
 drivers/gpu/drm/i915/gt/intel_gtt.h   |  1 +
 4 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c 
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 9a9ff84c90d7..35cf6608180e 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -279,7 +279,8 @@ static int proto_context_set_protected(struct 
drm_i915_private *i915,
 }
 
 static struct i915_gem_proto_context *
-proto_context_create(struct drm_i915_private *i915, unsigned int flags)
+proto_context_create(struct drm_i915_file_private *fpriv,
+struct drm_i915_private *i915, unsigned int flags)
 {
struct i915_gem_proto_context *pc, *err;
 
@@ -287,6 +288,7 @@ proto_context_create(struct drm_i915_private *i915, 
unsigned int flags)
if (!pc)
return ERR_PTR(-ENOMEM);
 
+   pc->fpriv = fpriv;
pc->num_user_engines = -1;
pc->user_engines = NULL;
pc->user_flags = BIT(UCONTEXT_BANNABLE) |
@@ -1621,6 +1623,7 @@ i915_gem_create_context(struct drm_i915_private *i915,
err = PTR_ERR(ppgtt);
goto err_ctx;
}
+   ppgtt->vm.fpriv = pc->fpriv;
vm = >vm;
}
if (vm)
@@ -1740,7 +1743,7 @@ int i915_gem_context_open(struct drm_i915_private *i915,
/* 0 reserved for invalid/unassigned ppgtt */
xa_init_flags(_priv->vm_xa, XA_FLAGS_ALLOC1);
 
-   pc = proto_context_create(i915, 0);
+   pc = proto_context_create(file_priv, i915, 0);
if (IS_ERR(pc)) {
err = PTR_ERR(pc);
goto err;
@@ -1822,6 +1825,7 @@ int i915_gem_vm_create_ioctl(struct drm_device *dev, void 
*data,
 
GEM_BUG_ON(id == 0); /* reserved for invalid/unassigned ppgtt */
args->vm_id = id;
+   ppgtt->vm.fpriv = file_priv;
return 0;
 
 err_put:
@@ -2284,7 +2288,8 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, 
void *data,
return -EIO;
}
 
-   ext_data.pc = proto_context_create(i915, args->flags);
+   ext_data.pc = proto_context_create(file->driver_priv, i915,
+  args->flags);
if (IS_ERR(ext_data.pc))
return PTR_ERR(ext_data.pc);
 
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h 
b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
index cb78214a7dcd..c573c067779f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
@@ -188,6 +188,9 @@ struct i915_gem_proto_engine {
  * CONTEXT_CREATE_SET_PARAM during GEM_CONTEXT_CREATE.
  */
 struct i915_gem_proto_context {
+   /** @fpriv: Client which creates the context */
+   struct drm_i915_file_private *fpriv;
+
/** @vm: See _gem_context.vm */
struct i915_address_space *vm;
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_context.c 
b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
index 8ac6726ec16b..125584ada282 100644
--- a/drivers/gpu/drm/i915/gem/selftests/mock_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
@@ -83,7 +83,7 @@ live_context(struct drm_i915_private *i915, struct file *file)
int err;
u32 id;
 
-   pc = proto_context_create(i915, 0);
+   pc = proto_context_create(fpriv, i915, 0);
if (IS_ERR(pc))
return ERR_CAST(pc);
 
@@ -152,7 +152,7 @@ kernel_context(struct drm_i915_private *i915,
struct i915_gem_context *ctx;
struct i915_gem_proto_context *pc;
 
-   pc = proto_context_create(i915, 0);
+   pc = proto_context_create(NULL, i915, 0);
if (IS_ERR(pc))
return ERR_CAST(pc);
 
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h 
b/drivers/gpu/drm/i915/gt/intel_gtt.h
index 346ec8ec2edd..8cf62f5134a9 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
@@ -248,6 +248,7 @@ struct i915_address_space {
struct drm_mm mm;
struct intel_gt *gt;
struct drm_i915_private *i915;
+   struct drm_i915_file_private *fpriv;
struct device *dma;
u64 total;  /* size addr space maps (ex. 2GB for ggtt) */
u64 reserved;   /* size addr space reserved */
-- 
2.39.2

[PATCH 4/5] drm/i915: Account ring buffer and context state storage

[Tvrtko Ursulin]

From: Tvrtko Ursulin 

Account ring buffers and logical context space against the owning client
memory usage stats.

Signed-off-by: Tvrtko Ursulin 
Reviewed-by: Aravind Iddamsetty 
---
 drivers/gpu/drm/i915/gt/intel_context.c | 14 ++
 drivers/gpu/drm/i915/i915_drm_client.c  | 10 ++
 drivers/gpu/drm/i915/i915_drm_client.h  |  9 +
 3 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_context.c 
b/drivers/gpu/drm/i915/gt/intel_context.c
index a53b26178f0a..a2f1245741bb 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -6,6 +6,7 @@
 #include "gem/i915_gem_context.h"
 #include "gem/i915_gem_pm.h"
 
+#include "i915_drm_client.h"
 #include "i915_drv.h"
 #include "i915_trace.h"
 
@@ -50,6 +51,7 @@ intel_context_create(struct intel_engine_cs *engine)
 
 int intel_context_alloc_state(struct intel_context *ce)
 {
+   struct i915_gem_context *ctx;
int err = 0;
 
if (mutex_lock_interruptible(>pin_mutex))
@@ -66,6 +68,18 @@ int intel_context_alloc_state(struct intel_context *ce)
goto unlock;
 
set_bit(CONTEXT_ALLOC_BIT, >flags);
+
+   rcu_read_lock();
+   ctx = rcu_dereference(ce->gem_context);
+   if (ctx && !kref_get_unless_zero(>ref))
+   ctx = NULL;
+   rcu_read_unlock();
+   if (ctx) {
+   if (ctx->client)
+   i915_drm_client_add_context_objects(ctx->client,
+   ce);
+   i915_gem_context_put(ctx);
+   }
}
 
 unlock:
diff --git a/drivers/gpu/drm/i915/i915_drm_client.c 
b/drivers/gpu/drm/i915/i915_drm_client.c
index 2e5e69edc0f9..a61356012df8 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -144,4 +144,14 @@ bool i915_drm_client_remove_object(struct 
drm_i915_gem_object *obj)
 
return true;
 }
+
+void i915_drm_client_add_context_objects(struct i915_drm_client *client,
+struct intel_context *ce)
+{
+   if (ce->state)
+   i915_drm_client_add_object(client, ce->state->obj);
+
+   if (ce->ring != ce->engine->legacy.ring && ce->ring->vma)
+   i915_drm_client_add_object(client, ce->ring->vma->obj);
+}
 #endif
diff --git a/drivers/gpu/drm/i915/i915_drm_client.h 
b/drivers/gpu/drm/i915/i915_drm_client.h
index 5f58fdf7dcb8..69cedfcd3d69 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.h
+++ b/drivers/gpu/drm/i915/i915_drm_client.h
@@ -14,6 +14,7 @@
 
 #include "i915_file_private.h"
 #include "gem/i915_gem_object_types.h"
+#include "gt/intel_context_types.h"
 
 #define I915_LAST_UABI_ENGINE_CLASS I915_ENGINE_CLASS_COMPUTE
 
@@ -70,6 +71,8 @@ void i915_drm_client_fdinfo(struct drm_printer *p, struct 
drm_file *file);
 void i915_drm_client_add_object(struct i915_drm_client *client,
struct drm_i915_gem_object *obj);
 bool i915_drm_client_remove_object(struct drm_i915_gem_object *obj);
+void i915_drm_client_add_context_objects(struct i915_drm_client *client,
+struct intel_context *ce);
 #else
 static inline void i915_drm_client_add_object(struct i915_drm_client *client,
  struct drm_i915_gem_object *obj)
@@ -79,6 +82,12 @@ static inline void i915_drm_client_add_object(struct 
i915_drm_client *client,
 static inline bool i915_drm_client_remove_object(struct drm_i915_gem_object 
*obj)
 {
 }
+
+static inline void
+i915_drm_client_add_context_objects(struct i915_drm_client *client,
+   struct intel_context *ce)
+{
+}
 #endif
 
 #endif /* !__I915_DRM_CLIENT_H__ */
-- 
2.39.2

[PATCH 3/5] drm/i915: Track page table backing store usage

[Tvrtko Ursulin]

From: Tvrtko Ursulin 

Account page table backing store against the owning client memory usage
stats.

Signed-off-by: Tvrtko Ursulin 
Reviewed-by: Aravind Iddamsetty 
---
 drivers/gpu/drm/i915/gt/intel_gtt.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c 
b/drivers/gpu/drm/i915/gt/intel_gtt.c
index 13944a14ea2d..c3f2b379 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -58,6 +58,9 @@ struct drm_i915_gem_object *alloc_pt_lmem(struct 
i915_address_space *vm, int sz)
if (!IS_ERR(obj)) {
obj->base.resv = i915_vm_resv_get(vm);
obj->shares_resv_from = vm;
+
+   if (vm->fpriv)
+   i915_drm_client_add_object(vm->fpriv->client, obj);
}
 
return obj;
@@ -79,6 +82,9 @@ struct drm_i915_gem_object *alloc_pt_dma(struct 
i915_address_space *vm, int sz)
if (!IS_ERR(obj)) {
obj->base.resv = i915_vm_resv_get(vm);
obj->shares_resv_from = vm;
+
+   if (vm->fpriv)
+   i915_drm_client_add_object(vm->fpriv->client, obj);
}
 
return obj;
-- 
2.39.2

[PATCH 1/5] drm/i915: Add ability for tracking buffer objects per client

[Tvrtko Ursulin]

From: Tvrtko Ursulin 

In order to show per client memory usage lets add some infrastructure
which enables tracking buffer objects owned by clients.

We add a per client list protected by a new per client lock and to support
delayed destruction (post client exit) we make tracked objects hold
references to the owning client.

Also, object memory region teardown is moved to the existing RCU free
callback to allow safe dereference from the fdinfo RCU read section.

Signed-off-by: Tvrtko Ursulin 
Reviewed-by: Aravind Iddamsetty 
---
 drivers/gpu/drm/i915/gem/i915_gem_object.c| 13 +--
 .../gpu/drm/i915/gem/i915_gem_object_types.h  | 12 +++
 drivers/gpu/drm/i915/i915_drm_client.c| 36 +++
 drivers/gpu/drm/i915/i915_drm_client.h| 32 +
 4 files changed, 90 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c 
b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index c26d87555825..25eeeb863209 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -106,6 +106,10 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
 
INIT_LIST_HEAD(>mm.link);
 
+#ifdef CONFIG_PROC_FS
+   INIT_LIST_HEAD(>client_link);
+#endif
+
INIT_LIST_HEAD(>lut_list);
spin_lock_init(>lut_lock);
 
@@ -293,6 +297,10 @@ void __i915_gem_free_object_rcu(struct rcu_head *head)
container_of(head, typeof(*obj), rcu);
struct drm_i915_private *i915 = to_i915(obj->base.dev);
 
+   /* We need to keep this alive for RCU read access from fdinfo. */
+   if (obj->mm.n_placements > 1)
+   kfree(obj->mm.placements);
+
i915_gem_object_free(obj);
 
GEM_BUG_ON(!atomic_read(>mm.free_count));
@@ -389,9 +397,6 @@ void __i915_gem_free_object(struct drm_i915_gem_object *obj)
if (obj->ops->release)
obj->ops->release(obj);
 
-   if (obj->mm.n_placements > 1)
-   kfree(obj->mm.placements);
-
if (obj->shares_resv_from)
i915_vm_resv_put(obj->shares_resv_from);
 
@@ -442,6 +447,8 @@ static void i915_gem_free_object(struct drm_gem_object 
*gem_obj)
 
GEM_BUG_ON(i915_gem_object_is_framebuffer(obj));
 
+   i915_drm_client_remove_object(obj);
+
/*
 * Before we free the object, make sure any pure RCU-only
 * read-side critical sections are complete, e.g.
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h 
b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index 2292404007c8..0c5cdab278b6 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -302,6 +302,18 @@ struct drm_i915_gem_object {
 */
struct i915_address_space *shares_resv_from;
 
+#ifdef CONFIG_PROC_FS
+   /**
+* @client: @i915_drm_client which created the object
+*/
+   struct i915_drm_client *client;
+
+   /**
+* @client_link: Link into @i915_drm_client.objects_list
+*/
+   struct list_head client_link;
+#endif
+
union {
struct rcu_head rcu;
struct llist_node freed;
diff --git a/drivers/gpu/drm/i915/i915_drm_client.c 
b/drivers/gpu/drm/i915/i915_drm_client.c
index 2a44b3876cb5..2e5e69edc0f9 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -28,6 +28,10 @@ struct i915_drm_client *i915_drm_client_alloc(void)
kref_init(>kref);
spin_lock_init(>ctx_lock);
INIT_LIST_HEAD(>ctx_list);
+#ifdef CONFIG_PROC_FS
+   spin_lock_init(>objects_lock);
+   INIT_LIST_HEAD(>objects_list);
+#endif
 
return client;
 }
@@ -108,4 +112,36 @@ void i915_drm_client_fdinfo(struct drm_printer *p, struct 
drm_file *file)
for (i = 0; i < ARRAY_SIZE(uabi_class_names); i++)
show_client_class(p, i915, file_priv->client, i);
 }
+
+void i915_drm_client_add_object(struct i915_drm_client *client,
+   struct drm_i915_gem_object *obj)
+{
+   unsigned long flags;
+
+   GEM_WARN_ON(obj->client);
+   GEM_WARN_ON(!list_empty(>client_link));
+
+   spin_lock_irqsave(>objects_lock, flags);
+   obj->client = i915_drm_client_get(client);
+   list_add_tail_rcu(>client_link, >objects_list);
+   spin_unlock_irqrestore(>objects_lock, flags);
+}
+
+bool i915_drm_client_remove_object(struct drm_i915_gem_object *obj)
+{
+   struct i915_drm_client *client = fetch_and_zero(>client);
+   unsigned long flags;
+
+   /* Object may not be associated with a client. */
+   if (!client)
+   return false;
+
+   spin_lock_irqsave(>objects_lock, flags);
+   list_del_rcu(>client_link);
+   spin_unlock_irqrestore(>objects_lock, flags);
+
+   i915_drm_client_put(client);
+
+   return true;
+}
 #endif
diff --git a/drivers/gpu/drm/i915/i915_drm_client.h 

[PATCH v7 0/5] fdinfo memory stats

[Tvrtko Ursulin]

From: Tvrtko Ursulin 

A short series to enable fdinfo memory stats for i915.

I added tracking of most classes of objects (user objects, page tables, context
state, ring buffers) which contribute to client's memory footprint and am
accouting their memory use along the similar lines as in Rob's msm code, just
that with i915 specific code we can show a memory region breakdown and so
support discrete and multi-tile GPUs properly. And also reflect that our objects
can have multiple allowed backing stores.

The existing helper Rob added is then used to dump the per memory region stats
to fdinfo.

The basic objects-per-client infrastructure can later be extended to cover all
objects and so avoid needing to walk the IDR under the client's file table lock,
which would further avoid distburbing the running clients by parallel fdinfo
readers.

Example fdinfo format:

# cat /proc/1383/fdinfo/8
pos:0
flags:  0212
mnt_id: 21
ino:397
drm-driver: i915
drm-client-id:  18
drm-pdev:   :00:02.0
drm-total-system:   125 MiB
drm-shared-system:  16 MiB
drm-active-system:  110 MiB
drm-resident-system:125 MiB
drm-purgeable-system:   2 MiB
drm-total-stolen-system:0
drm-shared-stolen-system:   0
drm-active-stolen-system:   0
drm-resident-stolen-system: 0
drm-purgeable-stolen-system:0
drm-engine-render:  25662044495 ns
drm-engine-copy:0 ns
drm-engine-video:   0 ns
drm-engine-video-enhance:   0 ns

Example gputop output:

DRM minor 0
 PID SMEM  SMEMRSS   render copy videoNAME
1233 124M 124M |||||||| neverball
1130  59M  59M |█▌  ||||||| Xorg
1207  12M  12M |||||||| xfwm4

Or with Wayland:

DRM minor 0
 PID  MEM  RSSrendercopy videovideo-enhance NAME
2093 191M 191M |▊  ||   ||   ||   | 
gnome-shell
DRM minor 128
 PID  MEM  RSSrendercopy videovideo-enhance NAME
2551  71M  71M |██▉||   ||   ||   | 
neverball
2553  50M  50M |   ||   ||   ||   | 
Xwayland

v2:
 * Now actually per client.

v3:
 * Track imported dma-buf objects.

v4:
 * Rely on DRM GEM handles for tracking user objects.
 * Fix internal object accounting (no placements).

v5:
 * Fixed brain fart of overwriting the loop cursor.
 * Fixed object destruction racing with fdinfo reads.
 * Take reference to GEM context while using it.

v6:
 * Rebase, cover letter update.

v7:
 * Account against active region only.
 * Cover all dma_resv usage when testing for activity.

Test-with: 20230921114557.192629-1-tvrtko.ursu...@linux.intel.com

Tvrtko Ursulin (5):
  drm/i915: Add ability for tracking buffer objects per client
  drm/i915: Record which client owns a VM
  drm/i915: Track page table backing store usage
  drm/i915: Account ring buffer and context state storage
  drm/i915: Implement fdinfo memory stats printing

 drivers/gpu/drm/i915/gem/i915_gem_context.c   |  11 +-
 .../gpu/drm/i915/gem/i915_gem_context_types.h |   3 +
 drivers/gpu/drm/i915/gem/i915_gem_object.c|  13 ++-
 .../gpu/drm/i915/gem/i915_gem_object_types.h  |  12 ++
 .../gpu/drm/i915/gem/selftests/mock_context.c |   4 +-
 drivers/gpu/drm/i915/gt/intel_context.c   |  14 +++
 drivers/gpu/drm/i915/gt/intel_gtt.c   |   6 +
 drivers/gpu/drm/i915/gt/intel_gtt.h   |   1 +
 drivers/gpu/drm/i915/i915_drm_client.c| 110 ++
 drivers/gpu/drm/i915/i915_drm_client.h|  41 +++
 10 files changed, 207 insertions(+), 8 deletions(-)

-- 
2.39.2

Re: [PATCH] omap: dsi: do not WARN on detach if dsidev was never attached

[Tony Lindgren]

* Tomi Valkeinen  [230921 10:53]:
> I sent a patch to the DSI framework code,
> "[PATCH] drm/mipi-dsi: Fix detach call without attach".
> 
> If that fixes the issue (please test, I don't have a suitable platform),
> perhaps it's a better fix as detach really shouldn't be called if attach has
> not been called.

Yup that works for me, replied with my Tested-by on that thread.

Thanks,

Tony

Re: [PATCH] drm/mipi-dsi: Fix detach call without attach

[Tony Lindgren]

* Tomi Valkeinen  [230921 10:51]:
> mipi_dsi_host_unregister() will call two functions for all its DSI
> peripheral devices: mipi_dsi_detach() and mipi_dsi_device_unregister().
> The latter makes sense, as the device exists, but the former may be
> wrong as attach has not necessarily been done.
> 
> To fix this, track the attached state of the peripheral, and only detach
> from mipi_dsi_host_unregister() if the peripheral was attached.
> 
> Note that I have only tested this with a board with an i2c DSI
> peripheral, not with a "pure" DSI peripheral.

Thanks this fixes the deferred probe warning I've been seeing:

Tested-by: Tony Lindgren 

Re: [PATCH] MAINTAINERS: Update gma500 git repo

[Patrik Jakobsson]

On Thu, Sep 21, 2023 at 1:00 PM Maxime Ripard  wrote:
>
> The GMA500 driver has been handled through drm-misc for a while but the
> git repo hasn't been updated. Make sure it points to the right place.
>
> Signed-off-by: Maxime Ripard 

Acked-by: Patrik Jakobsson 

>
> ---
>
> Cc: Patrik Jakobsson 
> ---
>  MAINTAINERS | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1012402dada5..5ebcf85bcbdb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6970,7 +6970,7 @@ DRM DRIVERS FOR GMA500 (Poulsbo, Moorestown and 
> derivative chipsets)
>  M: Patrik Jakobsson 
>  L: dri-devel@lists.freedesktop.org
>  S: Maintained
> -T: git git://github.com/patjak/drm-gma500
> +T: git git://anongit.freedesktop.org/drm/drm-misc
>  F: drivers/gpu/drm/gma500/
>
>  DRM DRIVERS FOR HISILICON
> --
> 2.41.0
>

Re: [PATCH v2] MAINTAINERS: Update drm-misc entry to match all drivers

[Thomas Zimmermann]

Am 21.09.23 um 12:57 schrieb Maxime Ripard:

We've had a number of times when a patch slipped through and we couldn't
pick them up either because our MAINTAINERS entry only covers the
framework and thus we weren't Cc'd.

Let's take another approach where we match everything, and remove all
the drivers that are not maintained through drm-misc.

Acked-by: Jani Nikula 
Signed-off-by: Maxime Ripard 


Acked-by: Thomas Zimmermann 



---

Cc: Alex Deucher 
Cc: Christian König 
Cc: "Pan, Xinhui" 
Cc: Russell King 
Cc: Lucas Stach 
Cc: Christian Gmeiner 
Cc: Inki Dae 
Cc: Seung-Woo Kim 
Cc: Kyungmin Park 
Cc: Jani Nikula 
Cc: Joonas Lahtinen 
Cc: Rodrigo Vivi 
Cc: Tvrtko Ursulin 
Cc: Philipp Zabel 
Cc: Laurentiu Palcu 
Cc: Anitha Chrisanthus 
Cc: Edmund Dea 
Cc: Chun-Kuang Hu 
Cc: Rob Clark 
Cc: Abhinav Kumar 
Cc: Dmitry Baryshkov 
Cc: Sean Paul 
Cc: Marijn Suijten 
Cc: Ben Skeggs 
Cc: Karol Herbst 
Cc: Lyude Paul 
Cc: Laurent Pinchart 
Cc: Kieran Bingham 
Cc: Thierry Reding 
Cc: Mikko Perttunen 
Cc: dri-devel@lists.freedesktop.org
Cc: amd-...@lists.freedesktop.org
Cc: etna...@lists.freedesktop.org
Cc: intel-...@lists.freedesktop.org
Cc: nouv...@lists.freedesktop.org
Cc: linux-renesas-...@vger.kernel.org
Cc: linux-te...@vger.kernel.org

Changes from v1:
- Remove ingenic and gma500 from the excluded list
---
  MAINTAINERS | 21 ++---
  1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 90f13281d297..1012402dada5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6860,12 +6860,27 @@ M:  Thomas Zimmermann 
  S:Maintained
  W:https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html
  T:git git://anongit.freedesktop.org/drm/drm-misc
+F: Documentation/devicetree/bindings/display/
+F: Documentation/devicetree/bindings/gpu/
  F:Documentation/gpu/
-F: drivers/gpu/drm/*
+F: drivers/gpu/drm/
  F:drivers/gpu/vga/
-F: include/drm/drm*
+F: include/drm/drm
  F:include/linux/vga*
-F: include/uapi/drm/drm*
+F: include/uapi/drm/
+X: drivers/gpu/drm/amd/
+X: drivers/gpu/drm/armada/
+X: drivers/gpu/drm/etnaviv/
+X: drivers/gpu/drm/exynos/
+X: drivers/gpu/drm/i915/
+X: drivers/gpu/drm/imx/
+X: drivers/gpu/drm/kmb/
+X: drivers/gpu/drm/mediatek/
+X: drivers/gpu/drm/msm/
+X: drivers/gpu/drm/nouveau/
+X: drivers/gpu/drm/radeon/
+X: drivers/gpu/drm/renesas/
+X: drivers/gpu/drm/tegra/
  
  DRM DRIVERS FOR ALLWINNER A10

  M:Maxime Ripard 


--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)


OpenPGP_signature.asc
Description: OpenPGP digital signature

[PATCH v5 1/4] video: fbdev: atyfb: only use ioremap_uc() on i386 and ia64

[Baoquan He]

From: Arnd Bergmann 

ioremap_uc() is only meaningful on old x86-32 systems with the PAT
extension, and on ia64 with its slightly unconventional ioremap()
behavior, everywhere else this is the same as ioremap() anyway.

Change the only driver that still references ioremap_uc() to only do so
on x86-32/ia64 in order to allow removing that interface at some
point in the future for the other architectures.

On some architectures, ioremap_uc() just returns NULL, changing
the driver to call ioremap() means that they now have a chance
of working correctly.

Signed-off-by: Arnd Bergmann 
Signed-off-by: Baoquan He 
Reviewed-by: Luis Chamberlain 
Cc: Helge Deller 
Cc: Thomas Zimmermann 
Cc: Christophe Leroy 
Cc: linux-fb...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
---
 drivers/video/fbdev/aty/atyfb_base.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/video/fbdev/aty/atyfb_base.c 
b/drivers/video/fbdev/aty/atyfb_base.c
index 5c87817a4f4c..3dcf83f5e7b4 100644
--- a/drivers/video/fbdev/aty/atyfb_base.c
+++ b/drivers/video/fbdev/aty/atyfb_base.c
@@ -3440,11 +3440,15 @@ static int atyfb_setup_generic(struct pci_dev *pdev, 
struct fb_info *info,
}
 
info->fix.mmio_start = raddr;
+#if defined(__i386__) || defined(__ia64__)
/*
 * By using strong UC we force the MTRR to never have an
 * effect on the MMIO region on both non-PAT and PAT systems.
 */
par->ati_regbase = ioremap_uc(info->fix.mmio_start, 0x1000);
+#else
+   par->ati_regbase = ioremap(info->fix.mmio_start, 0x1000);
+#endif
if (par->ati_regbase == NULL)
return -ENOMEM;
 
-- 
2.41.0

[PATCH] MAINTAINERS: Update gma500 git repo

[Maxime Ripard]

The GMA500 driver has been handled through drm-misc for a while but the
git repo hasn't been updated. Make sure it points to the right place.

Signed-off-by: Maxime Ripard 

---

Cc: Patrik Jakobsson 
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 1012402dada5..5ebcf85bcbdb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6970,7 +6970,7 @@ DRM DRIVERS FOR GMA500 (Poulsbo, Moorestown and 
derivative chipsets)
 M: Patrik Jakobsson 
 L: dri-devel@lists.freedesktop.org
 S: Maintained
-T: git git://github.com/patjak/drm-gma500
+T: git git://anongit.freedesktop.org/drm/drm-misc
 F: drivers/gpu/drm/gma500/
 
 DRM DRIVERS FOR HISILICON
-- 
2.41.0

[PATCH v2] MAINTAINERS: Update drm-misc entry to match all drivers

[Maxime Ripard]

We've had a number of times when a patch slipped through and we couldn't
pick them up either because our MAINTAINERS entry only covers the
framework and thus we weren't Cc'd.

Let's take another approach where we match everything, and remove all
the drivers that are not maintained through drm-misc.

Acked-by: Jani Nikula 
Signed-off-by: Maxime Ripard 

---

Cc: Alex Deucher 
Cc: Christian König 
Cc: "Pan, Xinhui" 
Cc: Russell King 
Cc: Lucas Stach 
Cc: Christian Gmeiner 
Cc: Inki Dae 
Cc: Seung-Woo Kim 
Cc: Kyungmin Park 
Cc: Jani Nikula 
Cc: Joonas Lahtinen 
Cc: Rodrigo Vivi 
Cc: Tvrtko Ursulin 
Cc: Philipp Zabel 
Cc: Laurentiu Palcu 
Cc: Anitha Chrisanthus 
Cc: Edmund Dea 
Cc: Chun-Kuang Hu 
Cc: Rob Clark 
Cc: Abhinav Kumar 
Cc: Dmitry Baryshkov 
Cc: Sean Paul 
Cc: Marijn Suijten 
Cc: Ben Skeggs 
Cc: Karol Herbst 
Cc: Lyude Paul 
Cc: Laurent Pinchart 
Cc: Kieran Bingham 
Cc: Thierry Reding 
Cc: Mikko Perttunen 
Cc: dri-devel@lists.freedesktop.org
Cc: amd-...@lists.freedesktop.org
Cc: etna...@lists.freedesktop.org
Cc: intel-...@lists.freedesktop.org
Cc: nouv...@lists.freedesktop.org
Cc: linux-renesas-...@vger.kernel.org
Cc: linux-te...@vger.kernel.org

Changes from v1:
- Remove ingenic and gma500 from the excluded list
---
 MAINTAINERS | 21 ++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 90f13281d297..1012402dada5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6860,12 +6860,27 @@ M:  Thomas Zimmermann 
 S: Maintained
 W: https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html
 T: git git://anongit.freedesktop.org/drm/drm-misc
+F: Documentation/devicetree/bindings/display/
+F: Documentation/devicetree/bindings/gpu/
 F: Documentation/gpu/
-F: drivers/gpu/drm/*
+F: drivers/gpu/drm/
 F: drivers/gpu/vga/
-F: include/drm/drm*
+F: include/drm/drm
 F: include/linux/vga*
-F: include/uapi/drm/drm*
+F: include/uapi/drm/
+X: drivers/gpu/drm/amd/
+X: drivers/gpu/drm/armada/
+X: drivers/gpu/drm/etnaviv/
+X: drivers/gpu/drm/exynos/
+X: drivers/gpu/drm/i915/
+X: drivers/gpu/drm/imx/
+X: drivers/gpu/drm/kmb/
+X: drivers/gpu/drm/mediatek/
+X: drivers/gpu/drm/msm/
+X: drivers/gpu/drm/nouveau/
+X: drivers/gpu/drm/radeon/
+X: drivers/gpu/drm/renesas/
+X: drivers/gpu/drm/tegra/
 
 DRM DRIVERS FOR ALLWINNER A10
 M: Maxime Ripard 
-- 
2.41.0

Re: [PATCH] omap: dsi: do not WARN on detach if dsidev was never attached

[Tomi Valkeinen]

Hi,

On 19/09/2023 16:37, H. Nikolaus Schaller wrote:

dsi_init_output() called by dsi_probe() may fail. In that
case mipi_dsi_host_unregister() is called which may call
omap_dsi_host_detach() with uninitialized dsi->dsidev
because omap_dsi_host_attach() was never called before.

This happens if the panel driver asks for an EPROBE_DEFER.

So let's suppress the WARN() in this special case.

[7.416759] WARNING: CPU: 0 PID: 32 at 
drivers/gpu/drm/omapdrm/dss/dsi.c:4419 omap_dsi_host_detach+0x3c/0xbc [omapdrm]
[7.436053] Modules linked in: ina2xx_adc snd_soc_ts3a227e bq2429x_charger 
bq27xxx_battery_i2c(+) bq27xxx_battery ina2xx tca8418_keypad as5013(+) omapdrm 
hci_uart cec palmas_pwrbutton btbcm bmp280_spi palmas_gpadc bluetooth usb3503 
ecdh_generic bmc150_accel_i2c bmg160_i2c ecc bmc150_accel_core bmg160_core 
bmc150_magn_i2c bmp280_i2c bmc150_magn bno055 industrialio_triggered_buffer 
bmp280 kfifo_buf snd_soc_omap_aess display_connector drm_kms_helper syscopyarea 
snd_soc_omap_mcbsp snd_soc_ti_sdma sysfillrect ti_tpd12s015 sysimgblt 
fb_sys_fops wwan_on_off snd_soc_gtm601 generic_adc_battery drm 
snd_soc_w2cbw003_bt industrialio drm_panel_orientation_quirks pwm_bl 
pwm_omap_dmtimer ip_tables x_tables ipv6 autofs4
[7.507068] CPU: 0 PID: 32 Comm: kworker/u4:2 Tainted: GW  
6.1.0-rc3-letux-lpae+ #11107
[7.516964] Hardware name: Generic OMAP5 (Flattened Device Tree)
[7.523284] Workqueue: events_unbound deferred_probe_work_func
[7.529456]  unwind_backtrace from show_stack+0x10/0x14
[7.534972]  show_stack from dump_stack_lvl+0x40/0x4c
[7.540315]  dump_stack_lvl from __warn+0xb0/0x164
[7.545379]  __warn from warn_slowpath_fmt+0x70/0x9c
[7.550625]  warn_slowpath_fmt from omap_dsi_host_detach+0x3c/0xbc [omapdrm]
[7.558137]  omap_dsi_host_detach [omapdrm] from 
mipi_dsi_remove_device_fn+0x10/0x20
[7.566376]  mipi_dsi_remove_device_fn from device_for_each_child+0x60/0x94
[7.573729]  device_for_each_child from mipi_dsi_host_unregister+0x20/0x54
[7.580992]  mipi_dsi_host_unregister from dsi_probe+0x5d8/0x744 [omapdrm]
[7.588315]  dsi_probe [omapdrm] from platform_probe+0x58/0xa8
[7.594542]  platform_probe from really_probe+0x144/0x2ac
[7.600249]  really_probe from __driver_probe_device+0xc4/0xd8
[7.606411]  __driver_probe_device from driver_probe_device+0x3c/0xb8
[7.613216]  driver_probe_device from __device_attach_driver+0x58/0xbc
[7.620115]  __device_attach_driver from bus_for_each_drv+0xa0/0xb4
[7.626737]  bus_for_each_drv from __device_attach+0xdc/0x150
[7.632808]  __device_attach from bus_probe_device+0x28/0x80
[7.638792]  bus_probe_device from deferred_probe_work_func+0x84/0xa0
[7.645595]  deferred_probe_work_func from process_one_work+0x1a4/0x2d8
[7.652587]  process_one_work from worker_thread+0x214/0x2b8
[7.658567]  worker_thread from kthread+0xe4/0xf0
[7.663542]  kthread from ret_from_fork+0x14/0x1c
[7.668515] Exception stack(0xf01b5fb0 to 0xf01b5ff8)
[7.673827] 5fa0:   
 
[7.682435] 5fc0:       
 
[7.691038] 5fe0:     0013 

Signed-off-by: H. Nikolaus Schaller 
---
  drivers/gpu/drm/omapdrm/dss/dsi.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c 
b/drivers/gpu/drm/omapdrm/dss/dsi.c
index ea63c64d3a1ab..c37eb6b1b9a39 100644
--- a/drivers/gpu/drm/omapdrm/dss/dsi.c
+++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
@@ -4411,7 +4411,7 @@ static int omap_dsi_host_detach(struct mipi_dsi_host 
*host,
  {
struct dsi_data *dsi = host_to_omap(host);
  
-	if (WARN_ON(dsi->dsidev != client))

+   if (!dsi->dsidev || WARN_ON(dsi->dsidev != client))
return -EINVAL;
  
  	cancel_delayed_work_sync(>dsi_disable_work);


I sent a patch to the DSI framework code,
"[PATCH] drm/mipi-dsi: Fix detach call without attach".

If that fixes the issue (please test, I don't have a suitable platform), 
perhaps it's a better fix as detach really shouldn't be called if attach 
has not been called.


 Tomi

[PATCH] drm/mipi-dsi: Fix detach call without attach

[Tomi Valkeinen]

It's been reported that DSI host driver's detach can be called without
the attach ever happening:

https://lore.kernel.org/all/20230412073954.20601-1-t...@atomide.com/

After reading the code, I think this is what happens:

We have a DSI host defined in the device tree and a DSI peripheral under
that host (i.e. an i2c device using the DSI as data bus doesn't exhibit
this behavior).

The host driver calls mipi_dsi_host_register(), which causes (via a few
functions) mipi_dsi_device_add() to be called for the DSI peripheral. So
now we have a DSI device under the host, but attach hasn't been called.

Normally the probing of the devices continues, and eventually the DSI
peripheral's driver will call mipi_dsi_attach(), attaching the
peripheral.

However, if the host driver's probe encounters an error after calling
mipi_dsi_host_register(), and before the peripheral has called
mipi_dsi_attach(), the host driver will do cleanups and return an error
from its probe function. The cleanups include calling
mipi_dsi_host_unregister().

mipi_dsi_host_unregister() will call two functions for all its DSI
peripheral devices: mipi_dsi_detach() and mipi_dsi_device_unregister().
The latter makes sense, as the device exists, but the former may be
wrong as attach has not necessarily been done.

To fix this, track the attached state of the peripheral, and only detach
from mipi_dsi_host_unregister() if the peripheral was attached.

Note that I have only tested this with a board with an i2c DSI
peripheral, not with a "pure" DSI peripheral.

However, slightly related, the unregister machinery still seems broken.
E.g. if the DSI host driver is unbound, it'll detach and unregister the
DSI peripherals. After that, when the DSI peripheral driver unbound
it'll call detach either directly or using the devm variant, leading to
a crash. And probably the driver will crash if it happens, for some
reason, to try to send a message via the DSI bus.

But that's another topic.

Signed-off-by: Tomi Valkeinen 
---
 drivers/gpu/drm/drm_mipi_dsi.c | 17 +++--
 include/drm/drm_mipi_dsi.h |  2 ++
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index 14201f73aab1..843a6dbda93a 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -347,7 +347,8 @@ static int mipi_dsi_remove_device_fn(struct device *dev, 
void *priv)
 {
struct mipi_dsi_device *dsi = to_mipi_dsi_device(dev);
 
-   mipi_dsi_detach(dsi);
+   if (dsi->attached)
+   mipi_dsi_detach(dsi);
mipi_dsi_device_unregister(dsi);
 
return 0;
@@ -370,11 +371,18 @@ EXPORT_SYMBOL(mipi_dsi_host_unregister);
 int mipi_dsi_attach(struct mipi_dsi_device *dsi)
 {
const struct mipi_dsi_host_ops *ops = dsi->host->ops;
+   int ret;
 
if (!ops || !ops->attach)
return -ENOSYS;
 
-   return ops->attach(dsi->host, dsi);
+   ret = ops->attach(dsi->host, dsi);
+   if (ret)
+   return ret;
+
+   dsi->attached = true;
+
+   return 0;
 }
 EXPORT_SYMBOL(mipi_dsi_attach);
 
@@ -386,9 +394,14 @@ int mipi_dsi_detach(struct mipi_dsi_device *dsi)
 {
const struct mipi_dsi_host_ops *ops = dsi->host->ops;
 
+   if (WARN_ON(!dsi->attached))
+   return -EINVAL;
+
if (!ops || !ops->detach)
return -ENOSYS;
 
+   dsi->attached = false;
+
return ops->detach(dsi->host, dsi);
 }
 EXPORT_SYMBOL(mipi_dsi_detach);
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index c9df0407980c..c0aec0d4d664 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -168,6 +168,7 @@ struct mipi_dsi_device_info {
  * struct mipi_dsi_device - DSI peripheral device
  * @host: DSI host for this peripheral
  * @dev: driver model device node for this peripheral
+ * @attached: the DSI device has been successfully attached
  * @name: DSI peripheral chip type
  * @channel: virtual channel assigned to the peripheral
  * @format: pixel format for video mode
@@ -184,6 +185,7 @@ struct mipi_dsi_device_info {
 struct mipi_dsi_device {
struct mipi_dsi_host *host;
struct device dev;
+   bool attached;
 
char name[DSI_DEV_NAME_SIZE];
unsigned int channel;

---
base-commit: 9fc75c40faa29df14ba16066be6bdfaea9f39ce4
change-id: 20230921-dsi-detach-fix-6736f7a48ba7

Best regards,
-- 
Tomi Valkeinen 

Re: MAINTAINERS: Update drm-misc entry to match all drivers

[Maxime Ripard]

On Thu, Sep 21, 2023 at 05:09:07PM +0800, Sui Jingfeng wrote:
> On 2023/9/21 16:47, Maxime Ripard wrote:
> > Adding Paul in Cc
> > 
> > On Thu, Sep 21, 2023 at 04:25:50PM +0800, suijingfeng wrote:
> > > On 2023/9/19 21:12, Maxime Ripard wrote:
> > > > We've had a number of times when a patch slipped through and we couldn't
> > > > pick them up either because our MAINTAINERS entry only covers the
> > > > framework and thus we weren't Cc'd.
> > > > 
> > > > Let's take another approach where we match everything, and remove all
> > > > the drivers that are not maintained through drm-misc.
> > > > 
> > > > Signed-off-by: Maxime Ripard 
> > > > Acked-by: Jani Nikula 
> > > > ---
> > > >MAINTAINERS | 23 ---
> > > >1 file changed, 20 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index 90f13281d297..757d4f33e158 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -6860,12 +6860,29 @@ M:  Thomas Zimmermann 
> > > >S:   Maintained
> > > >W:   
> > > > https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html
> > > >T:   git git://anongit.freedesktop.org/drm/drm-misc
> > > > +F: Documentation/devicetree/bindings/display/
> > > > +F: Documentation/devicetree/bindings/gpu/
> > > >F:   Documentation/gpu/
> > > > -F: drivers/gpu/drm/*
> > > > +F: drivers/gpu/drm/
> > > >F:   drivers/gpu/vga/
> > > > -F: include/drm/drm*
> > > > +F: include/drm/drm
> > > >F:   include/linux/vga*
> > > > -F: include/uapi/drm/drm*
> > > > +F: include/uapi/drm/
> > > > +X: drivers/gpu/drm/amd/
> > > > +X: drivers/gpu/drm/armada/
> > > > +X: drivers/gpu/drm/etnaviv/
> > > > +X: drivers/gpu/drm/exynos/
> > > > +X: drivers/gpu/drm/gma500/
> > > > +X: drivers/gpu/drm/i915/
> > > > +X: drivers/gpu/drm/imx/
> > > > +X: drivers/gpu/drm/ingenic/
> > > > +X: drivers/gpu/drm/kmb/
> > > > +X: drivers/gpu/drm/mediatek/
> > > > +X: drivers/gpu/drm/msm/
> > > > +X: drivers/gpu/drm/nouveau/
> > > > +X: drivers/gpu/drm/radeon/
> > > > +X: drivers/gpu/drm/renesas/
> > > > +X: drivers/gpu/drm/tegra/
> > > >DRM DRIVERS FOR ALLWINNER A10
> > > >M:   Maxime Ripard 
> > > 
> > > Nice patch!
> > > 
> > > Well, I'm just curious about why the drm/ingenic and drm/gma500 are not 
> > > maintained through drm-misc?
> > > 
> > > As far as I know:
> > > 1) the drm/ingenic driver don't have a "T" annotation (location of the 
> > > link).
> > Yeah, I wasn't sure about that one indeed. I remained conservative since 
> > it's a
> > sensitive topic for some.
> > 
> > Paul, is drm/ingenic supposed to be maintained through drm-misc? Either way,
> > could you clarify which git tree is supposed to merge those patches in
> > MAINTAINERS?
> > 
> > > 2) the "T" of drm/gma500 is "git git://github.com/patjak/drm-gma500", but 
> > > the
> >code for this link is not up to date.
> > 
> > For gma500, I think it's mostly historical since it was there before 
> > drm-misc
> > was a thing.
> > 
> > > I think at least the drm/ingenic and drm/gma500 drivers are *actually*
> > > maintained through drm-misc, So perhaps, these two drivers should not be
> > > excluded. Am I correct?
> > It's likely :)
> > 
> > Either way, I think it can be solved/clarified later on
> 
> 
> OK, that's sound fairly enough. I will respect you and Paul's opinion.
> By the way, I also want to say that I think the drm/imb and various drm/imx 
> drivers
> are also belong to the drm-misc. They are also lack the "T" annotation.
> Hopes someone can help to check that. Thanks.
> Thanks for the patch.

As far as I know, imx is kind of in-between at the moment. Some patches
are going through drm-misc, but others are coming through their tree.
For kmb (if that's what you meant), then I would expect intel to do the
maintenance like they do for i915.

But given this discussion it could be a good idea to add all the
maintainers of those excluded drivers in Cc in the next revision.

Maxime


signature.asc
Description: PGP signature

[PATCH v3 1/2] drm/bridge: megachips-stdpxxxx-ge-b850v3-fw: switch to drm_do_get_edid()

[Ian Ray]

Migrate away from custom EDID parsing and validity checks.

Note:  This is a follow-up to the original RFC by Jani [1].  The first
submission in this series should have been marked v2.

[1] 
https://patchwork.freedesktop.org/patch/msgid/20230901102400.552254-1-jani.nik...@intel.com

Co-developed-by: Jani Nikula 
Signed-off-by: Jani Nikula 
Signed-off-by: Ian Ray 
---
 .../drm/bridge/megachips-stdp-ge-b850v3-fw.c   | 57 --
 1 file changed, 9 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c 
b/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c
index 460db3c..e93083b 100644
--- a/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c
+++ b/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c
@@ -65,12 +65,11 @@ struct ge_b850v3_lvds {
 
 static struct ge_b850v3_lvds *ge_b850v3_lvds_ptr;
 
-static u8 *stdp2690_get_edid(struct i2c_client *client)
+static int stdp2690_read_block(void *context, u8 *buf, unsigned int block, 
size_t len)
 {
+   struct i2c_client *client = context;
struct i2c_adapter *adapter = client->adapter;
-   unsigned char start = 0x00;
-   unsigned int total_size;
-   u8 *block = kmalloc(EDID_LENGTH, GFP_KERNEL);
+   unsigned char start = block * EDID_LENGTH;
 
struct i2c_msg msgs[] = {
{
@@ -81,53 +80,15 @@ static u8 *stdp2690_get_edid(struct i2c_client *client)
}, {
.addr   = client->addr,
.flags  = I2C_M_RD,
-   .len= EDID_LENGTH,
-   .buf= block,
+   .len= len,
+   .buf= buf,
}
};
 
-   if (!block)
-   return NULL;
+   if (i2c_transfer(adapter, msgs, 2) != 2)
+   return -1;
 
-   if (i2c_transfer(adapter, msgs, 2) != 2) {
-   DRM_ERROR("Unable to read EDID.\n");
-   goto err;
-   }
-
-   if (!drm_edid_block_valid(block, 0, false, NULL)) {
-   DRM_ERROR("Invalid EDID data\n");
-   goto err;
-   }
-
-   total_size = (block[EDID_EXT_BLOCK_CNT] + 1) * EDID_LENGTH;
-   if (total_size > EDID_LENGTH) {
-   kfree(block);
-   block = kmalloc(total_size, GFP_KERNEL);
-   if (!block)
-   return NULL;
-
-   /* Yes, read the entire buffer, and do not skip the first
-* EDID_LENGTH bytes.
-*/
-   start = 0x00;
-   msgs[1].len = total_size;
-   msgs[1].buf = block;
-
-   if (i2c_transfer(adapter, msgs, 2) != 2) {
-   DRM_ERROR("Unable to read EDID extension blocks.\n");
-   goto err;
-   }
-   if (!drm_edid_block_valid(block, 1, false, NULL)) {
-   DRM_ERROR("Invalid EDID data\n");
-   goto err;
-   }
-   }
-
-   return block;
-
-err:
-   kfree(block);
-   return NULL;
+   return 0;
 }
 
 static struct edid *ge_b850v3_lvds_get_edid(struct drm_bridge *bridge,
@@ -137,7 +98,7 @@ static struct edid *ge_b850v3_lvds_get_edid(struct 
drm_bridge *bridge,
 
client = ge_b850v3_lvds_ptr->stdp2690_i2c;
 
-   return (struct edid *)stdp2690_get_edid(client);
+   return drm_do_get_edid(connector, stdp2690_read_block, client);
 }
 
 static int ge_b850v3_lvds_get_modes(struct drm_connector *connector)
-- 
2.10.1

[PATCH v3 2/2] MAINTAINERS: Update entry for megachips-stdpxxxx-ge-b850v3-fw

[Ian Ray]

Replace Martin, who has left GE.

Signed-off-by: Ian Ray 
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index bf0f54c..31bb835 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13524,7 +13524,7 @@ F:  drivers/usb/mtu3/
 
 MEGACHIPS STDP-GE-B850V3-FW LVDS/DP++ BRIDGES
 M: Peter Senna Tschudin 
-M: Martin Donnelly 
+M: Ian Ray 
 M: Martyn Welch 
 S: Maintained
 F: 
Documentation/devicetree/bindings/display/bridge/megachips-stdp-ge-b850v3-fw.txt
-- 
2.10.1

Re: [Intel-gfx] [PATCH] drm/i915/gem: Allow users to disable waitboost

[Tvrtko Ursulin]

On 20/09/2023 22:56, Vinay Belgaumkar wrote:

Provide a bit to disable waitboost while waiting on a gem object.
Waitboost results in increased power consumption by requesting RP0
while waiting for the request to complete. Add a bit in the gem_wait()
IOCTL where this can be disabled.

This is related to the libva API change here -
Link: 
https://github.com/XinfengZhang/libva/commit/3d90d18c67609a73121bb71b20ee4776b54b61a7


This link does not appear to lead to userspace code using this uapi?



Cc: Rodrigo Vivi 
Signed-off-by: Vinay Belgaumkar 
---
  drivers/gpu/drm/i915/gem/i915_gem_wait.c | 9 ++---
  drivers/gpu/drm/i915/i915_request.c  | 3 ++-
  drivers/gpu/drm/i915/i915_request.h  | 1 +
  include/uapi/drm/i915_drm.h  | 1 +
  4 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_wait.c 
b/drivers/gpu/drm/i915/gem/i915_gem_wait.c
index d4b918fb11ce..955885ec859d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_wait.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_wait.c
@@ -72,7 +72,8 @@ i915_gem_object_wait_reservation(struct dma_resv *resv,
struct dma_fence *fence;
long ret = timeout ?: 1;
  
-	i915_gem_object_boost(resv, flags);

+   if (!(flags & I915_WAITBOOST_DISABLE))
+   i915_gem_object_boost(resv, flags);
  
  	dma_resv_iter_begin(, resv,

dma_resv_usage_rw(flags & I915_WAIT_ALL));
@@ -236,7 +237,7 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, 
struct drm_file *file)
ktime_t start;
long ret;
  
-	if (args->flags != 0)

+   if (args->flags != 0 || args->flags != I915_GEM_WAITBOOST_DISABLE)
return -EINVAL;
  
  	obj = i915_gem_object_lookup(file, args->bo_handle);

@@ -248,7 +249,9 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, 
struct drm_file *file)
ret = i915_gem_object_wait(obj,
   I915_WAIT_INTERRUPTIBLE |
   I915_WAIT_PRIORITY |
-  I915_WAIT_ALL,
+  I915_WAIT_ALL |
+  (args->flags & I915_GEM_WAITBOOST_DISABLE ?
+   I915_WAITBOOST_DISABLE : 0),
   to_wait_timeout(args->timeout_ns));
  
  	if (args->timeout_ns > 0) {

diff --git a/drivers/gpu/drm/i915/i915_request.c 
b/drivers/gpu/drm/i915/i915_request.c
index f59081066a19..2957409b4b2a 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -2044,7 +2044,8 @@ long i915_request_wait_timeout(struct i915_request *rq,
 * but at a cost of spending more power processing the workload
 * (bad for battery).
 */
-   if (flags & I915_WAIT_PRIORITY && !i915_request_started(rq))
+   if (!(flags & I915_WAITBOOST_DISABLE) && (flags & I915_WAIT_PRIORITY) &&
+   !i915_request_started(rq))
intel_rps_boost(rq);
  
  	wait.tsk = current;

diff --git a/drivers/gpu/drm/i915/i915_request.h 
b/drivers/gpu/drm/i915/i915_request.h
index 0ac55b2e4223..3cc00e8254dc 100644
--- a/drivers/gpu/drm/i915/i915_request.h
+++ b/drivers/gpu/drm/i915/i915_request.h
@@ -445,6 +445,7 @@ long i915_request_wait(struct i915_request *rq,
  #define I915_WAIT_INTERRUPTIBLE   BIT(0)
  #define I915_WAIT_PRIORITYBIT(1) /* small priority bump for the request */
  #define I915_WAIT_ALL BIT(2) /* used by i915_gem_object_wait() */
+#define I915_WAITBOOST_DISABLE BIT(3) /* used by i915_gem_object_wait() */
  
  void i915_request_show(struct drm_printer *m,

   const struct i915_request *rq,
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 7000e5910a1d..4adee70e39cf 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1928,6 +1928,7 @@ struct drm_i915_gem_wait {
/** Handle of BO we shall wait on */
__u32 bo_handle;
__u32 flags;
+#define I915_GEM_WAITBOOST_DISABLE  (1u<<0)


Probably would be good to avoid mentioning waitboost in the uapi since 
so far it wasn't an explicit feature/contract. Something like 
I915_GEM_WAIT_BACKGROUND_PRIORITY? Low priority?


I also wonder if there could be a possible angle to help Rob (+cc) 
upstream the syncobj/fence deadline code if our media driver might make 
use of that somehow.


Like if either we could wire up the deadline into GEM_WAIT (in a 
backward compatible manner), or if media could use sync fd wait instead. 
Assuming they have an out fence already, which may not be true.


Regards,

Tvrtko


/** Number of nanoseconds to wait, Returns time remaining. */
__s64 timeout_ns;
  };

Re: MAINTAINERS: Update drm-misc entry to match all drivers

[Patrik Jakobsson]

On Thu, Sep 21, 2023 at 10:47:58AM +0200, Maxime Ripard wrote:
> Hi,
> 
> Adding Paul in Cc
> 
> On Thu, Sep 21, 2023 at 04:25:50PM +0800, suijingfeng wrote:
> > On 2023/9/19 21:12, Maxime Ripard wrote:
> > > We've had a number of times when a patch slipped through and we couldn't
> > > pick them up either because our MAINTAINERS entry only covers the
> > > framework and thus we weren't Cc'd.
> > > 
> > > Let's take another approach where we match everything, and remove all
> > > the drivers that are not maintained through drm-misc.
> > > 
> > > Signed-off-by: Maxime Ripard 
> > > Acked-by: Jani Nikula 
> > > ---
> > >   MAINTAINERS | 23 ---
> > >   1 file changed, 20 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 90f13281d297..757d4f33e158 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -6860,12 +6860,29 @@ M:Thomas Zimmermann 
> > >   S:  Maintained
> > >   W:  
> > > https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html
> > >   T:  git git://anongit.freedesktop.org/drm/drm-misc
> > > +F:   Documentation/devicetree/bindings/display/
> > > +F:   Documentation/devicetree/bindings/gpu/
> > >   F:  Documentation/gpu/
> > > -F:   drivers/gpu/drm/*
> > > +F:   drivers/gpu/drm/
> > >   F:  drivers/gpu/vga/
> > > -F:   include/drm/drm*
> > > +F:   include/drm/drm
> > >   F:  include/linux/vga*
> > > -F:   include/uapi/drm/drm*
> > > +F:   include/uapi/drm/
> > > +X:   drivers/gpu/drm/amd/
> > > +X:   drivers/gpu/drm/armada/
> > > +X:   drivers/gpu/drm/etnaviv/
> > > +X:   drivers/gpu/drm/exynos/
> > > +X:   drivers/gpu/drm/gma500/
> > > +X:   drivers/gpu/drm/i915/
> > > +X:   drivers/gpu/drm/imx/
> > > +X:   drivers/gpu/drm/ingenic/
> > > +X:   drivers/gpu/drm/kmb/
> > > +X:   drivers/gpu/drm/mediatek/
> > > +X:   drivers/gpu/drm/msm/
> > > +X:   drivers/gpu/drm/nouveau/
> > > +X:   drivers/gpu/drm/radeon/
> > > +X:   drivers/gpu/drm/renesas/
> > > +X:   drivers/gpu/drm/tegra/
> > >   DRM DRIVERS FOR ALLWINNER A10
> > >   M:  Maxime Ripard 
> > 
> > 
> > Nice patch!
> > 
> > Well, I'm just curious about why the drm/ingenic and drm/gma500 are not 
> > maintained through drm-misc?
> > 
> > As far as I know:
> > 1) the drm/ingenic driver don't have a "T" annotation (location of the 
> > link).
> 
> Yeah, I wasn't sure about that one indeed. I remained conservative since it's 
> a
> sensitive topic for some.
> 
> Paul, is drm/ingenic supposed to be maintained through drm-misc? Either way,
> could you clarify which git tree is supposed to merge those patches in
> MAINTAINERS?
> 
> > 2) the "T" of drm/gma500 is "git git://github.com/patjak/drm-gma500", but 
> > the
>   code for this link is not up to date.
> 
> For gma500, I think it's mostly historical since it was there before drm-misc
> was a thing.

Yes, that's the reason. I used do PRs from my github before we had
drm-misc but now everything gma500 related goes through drm-misc.

> 
> > I think at least the drm/ingenic and drm/gma500 drivers are *actually*
> > maintained through drm-misc, So perhaps, these two drivers should not be
> > excluded. Am I correct?
> 
> It's likely :)
> 
> Either way, I think it can be solved/clarified later on
> 
> Maxime

Re: [PATCH 1/2] drm/bridge: megachips-stdpxxxx-ge-b850v3-fw: switch to drm_do_get_edid()

[Jani Nikula]

On Thu, 21 Sep 2023, Ian Ray  wrote:
> Migrate away from custom EDID parsing and validity checks.
>
> Signed-off-by: Jani Nikula 
> Signed-off-by: Ian Ray 

So this is v2 of [1]. For future reference, people can get really fussy
about preserving authorship. I don't really mind in this case, because
most of the work was actually following through with it, testing, etc.
But maybe add

Co-developed-by: Jani Nikula 

immediately above my Signed-off-by.

Thanks,
Jani.


[1] 
https://patchwork.freedesktop.org/patch/msgid/20230901102400.552254-1-jani.nik...@intel.com


> ---
>  .../drm/bridge/megachips-stdp-ge-b850v3-fw.c   | 57 
> --
>  1 file changed, 9 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c 
> b/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c
> index 460db3c..e93083b 100644
> --- a/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c
> +++ b/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c
> @@ -65,12 +65,11 @@ struct ge_b850v3_lvds {
>  
>  static struct ge_b850v3_lvds *ge_b850v3_lvds_ptr;
>  
> -static u8 *stdp2690_get_edid(struct i2c_client *client)
> +static int stdp2690_read_block(void *context, u8 *buf, unsigned int block, 
> size_t len)
>  {
> + struct i2c_client *client = context;
>   struct i2c_adapter *adapter = client->adapter;
> - unsigned char start = 0x00;
> - unsigned int total_size;
> - u8 *block = kmalloc(EDID_LENGTH, GFP_KERNEL);
> + unsigned char start = block * EDID_LENGTH;
>  
>   struct i2c_msg msgs[] = {
>   {
> @@ -81,53 +80,15 @@ static u8 *stdp2690_get_edid(struct i2c_client *client)
>   }, {
>   .addr   = client->addr,
>   .flags  = I2C_M_RD,
> - .len= EDID_LENGTH,
> - .buf= block,
> + .len= len,
> + .buf= buf,
>   }
>   };
>  
> - if (!block)
> - return NULL;
> + if (i2c_transfer(adapter, msgs, 2) != 2)
> + return -1;
>  
> - if (i2c_transfer(adapter, msgs, 2) != 2) {
> - DRM_ERROR("Unable to read EDID.\n");
> - goto err;
> - }
> -
> - if (!drm_edid_block_valid(block, 0, false, NULL)) {
> - DRM_ERROR("Invalid EDID data\n");
> - goto err;
> - }
> -
> - total_size = (block[EDID_EXT_BLOCK_CNT] + 1) * EDID_LENGTH;
> - if (total_size > EDID_LENGTH) {
> - kfree(block);
> - block = kmalloc(total_size, GFP_KERNEL);
> - if (!block)
> - return NULL;
> -
> - /* Yes, read the entire buffer, and do not skip the first
> -  * EDID_LENGTH bytes.
> -  */
> - start = 0x00;
> - msgs[1].len = total_size;
> - msgs[1].buf = block;
> -
> - if (i2c_transfer(adapter, msgs, 2) != 2) {
> - DRM_ERROR("Unable to read EDID extension blocks.\n");
> - goto err;
> - }
> - if (!drm_edid_block_valid(block, 1, false, NULL)) {
> - DRM_ERROR("Invalid EDID data\n");
> - goto err;
> - }
> - }
> -
> - return block;
> -
> -err:
> - kfree(block);
> - return NULL;
> + return 0;
>  }
>  
>  static struct edid *ge_b850v3_lvds_get_edid(struct drm_bridge *bridge,
> @@ -137,7 +98,7 @@ static struct edid *ge_b850v3_lvds_get_edid(struct 
> drm_bridge *bridge,
>  
>   client = ge_b850v3_lvds_ptr->stdp2690_i2c;
>  
> - return (struct edid *)stdp2690_get_edid(client);
> + return drm_do_get_edid(connector, stdp2690_read_block, client);
>  }
>  
>  static int ge_b850v3_lvds_get_modes(struct drm_connector *connector)

-- 
Jani Nikula, Intel

Re: MAINTAINERS: Update drm-misc entry to match all drivers

[Maxime Ripard]

Hi Paul,

On Thu, Sep 21, 2023 at 10:57:46AM +0200, Paul Cercueil wrote:
> Le jeudi 21 septembre 2023 à 10:47 +0200, Maxime Ripard a écrit :
> > On Thu, Sep 21, 2023 at 04:25:50PM +0800, suijingfeng wrote:
> > > On 2023/9/19 21:12, Maxime Ripard wrote:
> > > > We've had a number of times when a patch slipped through and we
> > > > couldn't
> > > > pick them up either because our MAINTAINERS entry only covers the
> > > > framework and thus we weren't Cc'd.
> > > > 
> > > > Let's take another approach where we match everything, and remove
> > > > all
> > > > the drivers that are not maintained through drm-misc.
> > > > 
> > > > Signed-off-by: Maxime Ripard 
> > > > Acked-by: Jani Nikula 
> > > > ---
> > > >   MAINTAINERS | 23 ---
> > > >   1 file changed, 20 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index 90f13281d297..757d4f33e158 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -6860,12 +6860,29 @@ M:  Thomas Zimmermann
> > > > 
> > > >   S:Maintained
> > > >  
> > > > W:https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-
> > > > misc.html
> > > >   T:git git://anongit.freedesktop.org/drm/drm-misc
> > > > +F: Documentation/devicetree/bindings/display/
> > > > +F: Documentation/devicetree/bindings/gpu/
> > > >   F:Documentation/gpu/
> > > > -F: drivers/gpu/drm/*
> > > > +F: drivers/gpu/drm/
> > > >   F:drivers/gpu/vga/
> > > > -F: include/drm/drm*
> > > > +F: include/drm/drm
> > > >   F:include/linux/vga*
> > > > -F: include/uapi/drm/drm*
> > > > +F: include/uapi/drm/
> > > > +X: drivers/gpu/drm/amd/
> > > > +X: drivers/gpu/drm/armada/
> > > > +X: drivers/gpu/drm/etnaviv/
> > > > +X: drivers/gpu/drm/exynos/
> > > > +X: drivers/gpu/drm/gma500/
> > > > +X: drivers/gpu/drm/i915/
> > > > +X: drivers/gpu/drm/imx/
> > > > +X: drivers/gpu/drm/ingenic/
> > > > +X: drivers/gpu/drm/kmb/
> > > > +X: drivers/gpu/drm/mediatek/
> > > > +X: drivers/gpu/drm/msm/
> > > > +X: drivers/gpu/drm/nouveau/
> > > > +X: drivers/gpu/drm/radeon/
> > > > +X: drivers/gpu/drm/renesas/
> > > > +X: drivers/gpu/drm/tegra/
> > > >   DRM DRIVERS FOR ALLWINNER A10
> > > >   M:Maxime Ripard 
> > > 
> > > 
> > > Nice patch!
> > > 
> > > Well, I'm just curious about why the drm/ingenic and drm/gma500 are
> > > not maintained through drm-misc?
> > > 
> > > As far as I know:
> > > 1) the drm/ingenic driver don't have a "T" annotation (location of
> > > the link).
> > 
> > Yeah, I wasn't sure about that one indeed. I remained conservative
> > since it's a
> > sensitive topic for some.
> > 
> > Paul, is drm/ingenic supposed to be maintained through drm-misc?
> > Either way,
> > could you clarify which git tree is supposed to merge those patches
> > in
> > MAINTAINERS?
> 
> drm/ingenic is maintained through drm-misc, yes.
> 
> Looking at the MAINTAINERS file, it seems to be one of the only DRM
> drivers without its own entry. I'll add one then.

Ack, thanks!
Maxime


signature.asc
Description: PGP signature

Re: [PATCH] MAINTAINERS: Update drm-misc entry to match all drivers

[Maxime Ripard]

Hi Thomas,

On Thu, Sep 21, 2023 at 10:57:57AM +0200, Thomas Zimmermann wrote:
> Am 19.09.23 um 15:12 schrieb Maxime Ripard:
> > We've had a number of times when a patch slipped through and we couldn't
> > pick them up either because our MAINTAINERS entry only covers the
> > framework and thus we weren't Cc'd.
> > 
> > Let's take another approach where we match everything, and remove all
> > the drivers that are not maintained through drm-misc.
> > 
> > Signed-off-by: Maxime Ripard 
> > ---
> >   MAINTAINERS | 23 ---
> >   1 file changed, 20 insertions(+), 3 deletions(-)
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 90f13281d297..757d4f33e158 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -6860,12 +6860,29 @@ M:  Thomas Zimmermann 
> >   S:Maintained
> >   W:
> > https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html
> >   T:git git://anongit.freedesktop.org/drm/drm-misc
> > +F: Documentation/devicetree/bindings/display/
> > +F: Documentation/devicetree/bindings/gpu/
> >   F:Documentation/gpu/
> > -F: drivers/gpu/drm/*
> > +F: drivers/gpu/drm/
> >   F:drivers/gpu/vga/
> > -F: include/drm/drm*
> > +F: include/drm/drm
> >   F:include/linux/vga*
> > -F: include/uapi/drm/drm*
> > +F: include/uapi/drm/
> > +X: drivers/gpu/drm/amd/
> > +X: drivers/gpu/drm/armada/
> > +X: drivers/gpu/drm/etnaviv/
> > +X: drivers/gpu/drm/exynos/
> 
> > +X: drivers/gpu/drm/gma500/
> 
> We always had gma500 in drm-misc. Where else would these go?

The maintainers entry has a git repo. I'll update it (and that patch too) then

Maxime


signature.asc
Description: PGP signature