Re: [PATCH v1 2/3] dt-bindings: arm: mediatek: mmsys: Add OF graph support for board path

2024-04-08 Thread Chen-Yu Tsai
On Mon, Apr 8, 2024 at 6:16 PM AngeloGioacchino Del Regno
 wrote:
>
> Il 08/04/24 05:20, Chen-Yu Tsai ha scritto:
> > On Thu, Apr 4, 2024 at 4:16 PM AngeloGioacchino Del Regno
> >  wrote:
> >>
> >> Document OF graph on MMSYS/VDOSYS: this supports up to three DDP paths
> >> per HW instance (so potentially up to six displays for multi-vdo SoCs).
> >>
> >> The MMSYS or VDOSYS is always the first component in the DDP pipeline,
> >> so it only supports an output port with multiple endpoints - where each
> >> endpoint defines the starting point for one of the (currently three)
> >> possible hardware paths.
> >>
> >> Signed-off-by: AngeloGioacchino Del Regno 
> >> 
> >> ---
> >>   .../bindings/arm/mediatek/mediatek,mmsys.yaml | 23 +++
> >>   1 file changed, 23 insertions(+)
> >>
> >> diff --git 
> >> a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml 
> >> b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
> >> index b3c6888c1457..90758bb5bcb1 100644
> >> --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
> >> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
> >> @@ -93,6 +93,29 @@ properties:
> >> '#reset-cells':
> >>   const: 1
> >>
> >> +  port:
> >> +$ref: /schemas/graph.yaml#/properties/port
> >> +description:
> >> +  Output port node. This port connects the MMSYS/VDOSYS output to
> >> +  the first component of one display pipeline, for example one of
> >> +  the available OVL or RDMA blocks.
> >> +  Some MediaTek SoCs support up to three display outputs per MMSYS.
> >> +properties:
> >> +  endpoint@0:
> >> +$ref: /schemas/graph.yaml#/properties/endpoint
> >> +description: Output to the primary display pipeline
> >> +
> >> +  endpoint@1:
> >> +$ref: /schemas/graph.yaml#/properties/endpoint
> >> +description: Output to the secondary display pipeline
> >> +
> >> +  endpoint@2:
> >> +$ref: /schemas/graph.yaml#/properties/endpoint
> >> +description: Output to the tertiary display pipeline
> >> +
> >> +  required:
> >> +- endpoint@0
> >> +
> >
> > Technically the mmsys device serves as an glue layer for the display
> > pipeline, providing things like clock control and signal routing; the
> > device itself is not part of the pipeline, and probably shouldn't be
> > part of the graph?
> >
>
> That is (only) partially true: in the case of older SoCs, the MMSYS can only
> connect to a single first IP of the pipeline, but in the case of newer ones,
> and especially (but not limited to) MT8195 onwards having multiple instances
> of VDOSYS, that really becomes part of the pipeline.
>
> This is not because of the possible different first IP in the pipeline, but
> because of support for dual-interface (DSI and DP) that, in even newer SoCs,
> can be done with cross-mmsys (cross-vdosys, actually...) as some of those do
> have the two in different VDOs.
>
> So yes, this can be done without the graph in MMSYS *in this precise moment in
> time*, but we'll anyway end up adding it sooner than later - and I'm doing 
> this
> right now, instead of later, because it's also simplifying the implementation
> so like that I'm "catching two birds with one stone" :-)

I see. Thanks for sorting it out. We had something similar on Allwinner
platforms but it was never as complex or flexible as this.

ChenYu

> Cheers,
> Angelo
>
> > ChenYu
> >
> >>   required:
> >> - compatible
> >> - reg
> >> --
> >> 2.44.0
> >>
>
>


[git pull] drm urgent fix for 6.9-rc4

2024-04-08 Thread Dave Airlie
Hi Linus,

A previous fix to nouveau devinit on the GSP paths fixed the Turing
but broke Ampere, I did some more digging and found the proper fix.
Sending it early as I want to make sure it makes the next 6.8 stable
kernels to fix the regression.

Regular fixes will be at end of week as usual.

Thanks,
Dave.

drm-fixes-2024-04-09:
drm nouveau fix for 6.9-rc4

nouveau:
- regression fix for GSP display enable.
The following changes since commit fec50db7033ea478773b159e0e2efb135270e3b7:

  Linux 6.9-rc3 (2024-04-07 13:22:46 -0700)

are available in the Git repository at:

  https://gitlab.freedesktop.org/drm/kernel.git tags/drm-fixes-2024-04-09

for you to fetch changes up to 718c4fb221dbeff9072810841b949413c5ffc345:

  nouveau: fix devinit paths to only handle display on GSP.
(2024-04-09 13:14:13 +1000)


drm nouveau fix for 6.9-rc4

nouveau:
- regression fix for GSP display enable.


Dave Airlie (1):
  nouveau: fix devinit paths to only handle display on GSP.

 drivers/gpu/drm/nouveau/nvkm/subdev/devinit/gm107.c | 12 
 drivers/gpu/drm/nouveau/nvkm/subdev/devinit/r535.c  |  1 +
 2 files changed, 9 insertions(+), 4 deletions(-)


Re: [PATCH] nouveau: fix devinit paths to only handle display on GSP.

2024-04-08 Thread Dave Airlie
On Mon, 8 Apr 2024 at 23:05, Timur Tabi  wrote:
>
> On Mon, 2024-04-08 at 16:42 +1000, Dave Airlie wrote:
> > This reverts:
> > nouveau/gsp: don't check devinit disable on GSP.
> > and applies a further fix.
> >
> > It turns out the open gpu driver, checks this register, but only for
> > display.
> >
> > Match that behaviour and only disable displays on turings.
>
> Why only on Turings?

The only is for disabling displays, not for the turings. The ampere
devinit path only handle displays, it never tries to disable other
engines so should be fine.

Dave.


Re: [PATCH 4.19.y] drm/vkms: call drm_atomic_helper_shutdown before drm_dev_put()

2024-04-08 Thread guomengqi (A)



在 2024/4/5 17:30, Greg KH 写道:

On Wed, Apr 03, 2024 at 05:47:16PM +0800, Guo Mengqi wrote:

commit 73a82b22963d ("drm/atomic: Fix potential use-after-free
in nonblocking commits") introduced drm_dev_get/put() to
drm_atomic_helper_shutdown(). And this cause problem in vkms driver exit
process.

vkms_exit()
   drm_dev_put()
 vkms_release()
   drm_atomic_helper_shutdown()
 drm_dev_get()
 drm_dev_put()
   vkms_release()-- null pointer access

Using 4.19 stable x86 image on qemu, below stacktrace can be triggered by
load and unload vkms.ko.

root:~ # insmod vkms.ko
[  142.135449] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[  142.138713] [drm] Driver supports precise vblank timestamp query.
[  142.142390] [drm] Initialized vkms 1.0.0 20180514 for virtual device on 
minor 0
root:~ # rmmod vkms.ko
[  144.093710] BUG: unable to handle kernel NULL pointer dereference at 
00a0
[  144.097491] PGD 80023624e067 P4D 80023624e067 PUD 22ab59067 PMD 0
[  144.100802] Oops:  [#1] SMP PTI
[  144.102502] CPU: 0 PID: 3615 Comm: rmmod Not tainted 4.19.310 #1
[  144.104452] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
[  144.107238] RIP: 0010:device_del+0x34/0x3a0
...
[  144.131323] Call Trace:
[  144.131962]  ? __die+0x7d/0xc0
[  144.132711]  ? no_context+0x152/0x3b0
[  144.133605]  ? wake_up_q+0x70/0x70
[  144.134436]  ? __do_page_fault+0x342/0x4b0
[  144.135445]  ? __switch_to_asm+0x41/0x70
[  144.136416]  ? __switch_to_asm+0x35/0x70
[  144.137366]  ? page_fault+0x1e/0x30
[  144.138214]  ? __drm_atomic_state_free+0x51/0x60
[  144.139331]  ? device_del+0x34/0x3a0
[  144.140197]  platform_device_del.part.14+0x19/0x70
[  144.141348]  platform_device_unregister+0xe/0x20
[  144.142458]  vkms_release+0x10/0x30 [vkms]
[  144.143449]  __drm_atomic_helper_disable_all.constprop.31+0x13b/0x150
[  144.144980]  drm_atomic_helper_shutdown+0x4b/0x90
[  144.146102]  vkms_release+0x18/0x30 [vkms]
[  144.147107]  vkms_exit+0x29/0x8ec [vkms]
[  144.148053]  __x64_sys_delete_module+0x155/0x220
[  144.149168]  do_syscall_64+0x43/0x100
[  144.150056]  entry_SYSCALL_64_after_hwframe+0x5c/0xc1

It seems that the proper unload sequence is:
drm_atomic_helper_shutdown();
drm_dev_put();

Just put drm_atomic_helper_shutdown() before drm_dev_put()
should solve the problem.

Note that vkms exit code is refactored by 53d77aaa3f76 ("drm/vkms: Use
devm_drm_dev_alloc") in tags/v5.10-rc1.

So this bug only exists on 4.19 and 5.4.

Do we also need this for 5.4?  If so, can you send a version for that
tree with the correct Fixes: information, and I will be glad to queue
both of these up.


I sent a patch to 5.4.y too. Please check it at 
https://lore.kernel.org/all/20240409022647.1821-1-guomeng...@huawei.com/T/#u


Thanks,

Mengqi


thanks,

greg k-h
.


[PATCH 5.4.y] drm/vkms: call drm_atomic_helper_shutdown before drm_dev_put()

2024-04-08 Thread Guo Mengqi
commit 73a82b22963d ("drm/atomic: Fix potential use-after-free
in nonblocking commits") introduced drm_dev_get/put() to
drm_atomic_helper_shutdown(). And this cause problem in vkms driver exit
process.

vkms_exit()
  drm_dev_put()
vkms_release()
  drm_atomic_helper_shutdown()
drm_dev_get()
drm_dev_put()
  vkms_release()-- use after free

Using 5.4 stable x86 image on qemu, below stacktrace can be triggered by
load and unload vkms.ko.

root:~ # insmod vkms.ko
[   76.957802] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[   76.961490] [drm] Driver supports precise vblank timestamp query.
[   76.964416] [drm] Initialized vkms 1.0.0 20180514 for vkms on minor 0
root:~ # rmmod vkms.ko
[   79.650202] refcount_t: addition on 0; use-after-free.
[   79.650249] WARNING: CPU: 2 PID: 3533 at ../lib/refcount.c:25 
refcount_warn_saturate+0xcf/0xf0
[   79.654241] Modules linked in: vkms(-)
[   79.654249] CPU: 2 PID: 3533 Comm: rmmod Not tainted 5.4.273 #4
[   79.654251] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
[   79.654262] RIP: 0010:refcount_warn_saturate+0xcf/0xf0
...
[   79.654296] Call Trace:
[   79.654462]  ? __warn+0x80/0xd0
[   79.654473]  ? refcount_warn_saturate+0xcf/0xf0
[   79.654481]  ? report_bug+0xb6/0x130
[   79.654484]  ? refcount_warn_saturate+0xcf/0xf0
[   79.654489]  ? fixup_bug.part.12+0x13/0x30
[   79.654492]  ? do_error_trap+0x90/0xb0
[   79.654495]  ? do_invalid_op+0x31/0x40
[   79.654497]  ? refcount_warn_saturate+0xcf/0xf0
[   79.654504]  ? invalid_op+0x1e/0x30
[   79.654508]  ? refcount_warn_saturate+0xcf/0xf0
[   79.654516]  drm_atomic_state_init+0x68/0xb0
[   79.654543]  drm_atomic_state_alloc+0x43/0x60
[   79.654551]  drm_atomic_helper_disable_all+0x13/0x180
[   79.654562]  drm_atomic_helper_shutdown+0x5f/0xb0
[   79.654571]  vkms_release+0x18/0x40 [vkms]
[   79.654575]  vkms_exit+0x29/0xc00 [vkms]
[   79.654582]  __x64_sys_delete_module+0x155/0x220
[   79.654592]  do_syscall_64+0x43/0x120
[   79.654603]  entry_SYSCALL_64_after_hwframe+0x5c/0xc1
[   79.654619] ---[ end trace ce0c02f57ea6bf73 ]---

It seems that the proper unload sequence is:
drm_atomic_helper_shutdown();
drm_dev_put();

Just put drm_atomic_helper_shutdown() before drm_dev_put()
should solve the problem.

Note that vkms exit code is refactored by commit 53d77aaa3f76
("drm/vkms: Use devm_drm_dev_alloc") in tags/v5.10-rc1.

So this bug only exists on 4.19 and 5.4.

Fixes: 380c7ceabdde ("drm/atomic: Fix potential use-after-free in nonblocking 
commits")
Fixes: 2ead1be54b22 ("drm/vkms: Fix connector leak at the module removal")
Signed-off-by: Guo Mengqi 
---
 drivers/gpu/drm/vkms/vkms_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 44ab9f8ef8be..86043d7c0e4b 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -60,7 +60,6 @@ static void vkms_release(struct drm_device *dev)
struct vkms_device *vkms = container_of(dev, struct vkms_device, drm);
 
platform_device_unregister(vkms->platform);
-   drm_atomic_helper_shutdown(>drm);
drm_mode_config_cleanup(>drm);
drm_dev_fini(>drm);
destroy_workqueue(vkms->output.composer_workq);
@@ -194,6 +193,7 @@ static void __exit vkms_exit(void)
}
 
drm_dev_unregister(_device->drm);
+   drm_atomic_helper_shutdown(_device->drm);
drm_dev_put(_device->drm);
 
kfree(vkms_device);
-- 
2.17.1



Re: [PATCH v3] drm/msm/dp: call dp_hpd_plug_handle()/unplug_handle() directly for external HPD

2024-04-08 Thread Bjorn Andersson
On Tue, Apr 09, 2024 at 01:07:57AM +0300, Dmitry Baryshkov wrote:
> On Tue, 9 Apr 2024 at 00:23, Abhinav Kumar  wrote:
> > On 4/8/2024 2:12 PM, Dmitry Baryshkov wrote:
> > > On Mon, 8 Apr 2024 at 22:43, Abhinav Kumar  
> > > wrote:
> > >> On 4/7/2024 11:48 AM, Bjorn Andersson wrote:
> > >>> On Fri, Apr 05, 2024 at 08:15:47PM -0700, Abhinav Kumar wrote:
> >  From: Kuogee Hsieh 
> > >>> [..]
> >  diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
> >  b/drivers/gpu/drm/msm/dp/dp_display.c
[..]
> > >>
> > >> I will need sometime to address that use-case as I need to see if we can
> > >> handle that better and then drop the the DISCONNECT_PENDING state to
> > >> address this fully. But it needs more testing.
> > >>
> > >> But, we will need this patch anyway because without this we will not be
> > >> able to fix even the most regular and commonly seen case of basic
> > >> connect/disconnect receiving complementary events.
> > >
> > > Hmm, no. We need to drop the HPD state machine, not to patch it. Once
> > > the driver has proper detect() callback, there will be no
> > > complementary events. That is a proper way to fix the code, not these
> > > kinds of band-aids patches.
> > >
> >
> > I had discussed this part too :)
> >
> > I totally agree we should fix .detect()'s behavior to just match cable
> > connect/disconnect and not link_ready status.
> >
> > But that alone would not have fixed this issue. If HPD thread does not
> > get scheduled and plug_handle() was not executed, .detect() would have
> > still returned old status as we will update the cable status only in
> > plug_handle() / unplug_handle() to have a common API between internal
> > and external hpd execution.
> 
> I think there should be just hpd_notify, which if the HPD is up,
> attempts to read the DPCD. No need for separate plug/unplug_handle.
> The detect() can be as simple as !drm_dp_is_branch() || sink_count != 0.
> 

What is detect() supposed to return in the event that we have external
HPD handler? The link state? While the external HPD bridge would return
the HPD state?

If a driver only drives the link inbetween atomic_enable() and
atomic_disable() will the "connected state" then ever be reported as
"connected"? (I'm sure I'm still missing pieces of this puzzle).

Regards,
Bjorn


Re: [PATCH 5/5] drm/vkms: Use drm_crtc_vblank_crtc()

2024-04-08 Thread Maíra Canal

On 4/8/24 16:06, Ville Syrjala wrote:

From: Ville Syrjälä 

Replace the open coded drm_crtc_vblank_crtc() with the real
thing.

Cc: Rodrigo Siqueira 
Cc: Melissa Wen 
Cc: "Maíra Canal" 
Cc: Haneen Mohammed 
Cc: Daniel Vetter 
Signed-off-by: Ville Syrjälä 


Reviewed-by: Maíra Canal 

Best Regards,
- Maíra


---
  drivers/gpu/drm/vkms/vkms_crtc.c | 7 ++-
  1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 61e500b8c9da..40b4d084e3ce 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -61,9 +61,7 @@ static enum hrtimer_restart vkms_vblank_simulate(struct 
hrtimer *timer)
  
  static int vkms_enable_vblank(struct drm_crtc *crtc)

  {
-   struct drm_device *dev = crtc->dev;
-   unsigned int pipe = drm_crtc_index(crtc);
-   struct drm_vblank_crtc *vblank = >vblank[pipe];
+   struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
  
  	drm_calc_timestamping_constants(crtc, >mode);

@@ -88,10 +86,9 @@ static bool vkms_get_vblank_timestamp(struct drm_crtc *crtc,
  bool in_vblank_irq)
  {
struct drm_device *dev = crtc->dev;
-   unsigned int pipe = crtc->index;
struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev);
struct vkms_output *output = >output;
-   struct drm_vblank_crtc *vblank = >vblank[pipe];
+   struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
  
  	if (!READ_ONCE(vblank->enabled)) {

*vblank_time = ktime_get();


Re: [PATCH v3 3/6] drm/msm/dsi: set VIDEO_COMPRESSION_MODE_CTRL_WC (fix video mode DSC)

2024-04-08 Thread Jun Nie
Marijn Suijten  于2024年4月9日周二 00:45写道:
>
> Can we drop (fix video mode DSC) from this patch title?  It looks like more
> patches are required to get this done, such a mention is more something for 
> the
> cover letter.
>
> We could also clarify further to "set Word Count for video-mode DSC".
>
Accepted. video-mode DSC is achieved with the patch set, not this
specific patch.


[PATCH] nouveau: fix instmem race condition around ptr stores

2024-04-08 Thread Dave Airlie
From: Dave Airlie 

Running a lot of VK CTS in parallel against nouveau, once every
few hours you might see something like this crash.

BUG: kernel NULL pointer dereference, address: 0008
PGD 800114e6e067 P4D 800114e6e067 PUD 109046067 PMD 0
Oops:  [#1] PREEMPT SMP PTI
CPU: 7 PID: 53891 Comm: deqp-vk Not tainted 6.8.0-rc6+ #27
Hardware name: Gigabyte Technology Co., Ltd. Z390 I AORUS PRO WIFI/Z390 I AORUS 
PRO WIFI-CF, BIOS F8 11/05/2021
RIP: 0010:gp100_vmm_pgt_mem+0xe3/0x180 [nouveau]
Code: c7 48 01 c8 49 89 45 58 85 d2 0f 84 95 00 00 00 41 0f b7 46 12 49 8b 7e 
08 89 da 42 8d 2c f8 48 8b 47 08 41 83 c7 01 48 89 ee <48> 8b 40 08 ff d0 0f 1f 
00 49 8b 7e 08 48 89 d9 48 8d 75 04 48 c1
RSP: :ac20c5857838 EFLAGS: 00010202
RAX:  RBX: 004d8001 RCX: 0001
RDX: 004d8001 RSI: 06d8 RDI: a07afe332180
RBP: 06d8 R08: ac20c5857ad0 R09: 0010
R10: 0001 R11: a07af27e2de0 R12: 001c
R13: ac20c5857ad0 R14: a07a96fe9040 R15: 001c
FS:  7fe395eed7c0() GS:a07e2c98() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 0008 CR3: 00011febe001 CR4: 003706f0
DR0:  DR1:  DR2: 
DR3:  DR6: fffe0ff0 DR7: 0400
Call Trace:

...

 ? gp100_vmm_pgt_mem+0xe3/0x180 [nouveau]
 ? gp100_vmm_pgt_mem+0x37/0x180 [nouveau]
 nvkm_vmm_iter+0x351/0xa20 [nouveau]
 ? __pfx_nvkm_vmm_ref_ptes+0x10/0x10 [nouveau]
 ? __pfx_gp100_vmm_pgt_mem+0x10/0x10 [nouveau]
 ? __pfx_gp100_vmm_pgt_mem+0x10/0x10 [nouveau]
 ? __lock_acquire+0x3ed/0x2170
 ? __pfx_gp100_vmm_pgt_mem+0x10/0x10 [nouveau]
 nvkm_vmm_ptes_get_map+0xc2/0x100 [nouveau]
 ? __pfx_nvkm_vmm_ref_ptes+0x10/0x10 [nouveau]
 ? __pfx_gp100_vmm_pgt_mem+0x10/0x10 [nouveau]
 nvkm_vmm_map_locked+0x224/0x3a0 [nouveau]

Adding any sort of useful debug usually makes it go away, so I hand
wrote the function in a line, and debugged the asm.

Every so often pt->memory->ptrs is NULL. This ptrs ptr is set in
the nv50_instobj_acquire called from nvkm_kmap.

If Thread A and Thread B both get to nv50_instobj_acquire around
the same time, and Thread A hits the refcount_set line, and in
lockstep thread B succeeds at refcount_inc_not_zero, there is a
chance the ptrs value won't have been stored since refcount_set
is unordered. Force a memory barrier here, I picked smp_mb, since
we want it on all CPUs and it's write followed by a read.

Cc: linux-stable
Signed-off-by: Dave Airlie 
---
 drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c
index a7f3fc342d87..cbacc7b11f8c 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c
@@ -250,6 +250,9 @@ nv50_instobj_acquire(struct nvkm_memory *memory)
iobj->base.memory.ptrs = _instobj_fast;
else
iobj->base.memory.ptrs = _instobj_slow;
+   /* barrier to ensure ptrs is written before another thread
+  does refcount_inc_not_zero successfully. */
+   smp_mb();
refcount_set(>maps, 1);
}
 
-- 
2.43.2



Re: [PATCH 3/3] drm/msm/dsi: simplify connector creation

2024-04-08 Thread Abhinav Kumar




On 3/9/2024 7:09 AM, Dmitry Baryshkov wrote:

Instead of having two functions, msm_dsi_manager_bridge_init()
and msm_dsi_manager_ext_bridge_init(), merge them into
msm_dsi_manager_connector_init(), moving drm_bridge_attach() to be
called from the bridge's attach callback (as most other bridges do).

Signed-off-by: Dmitry Baryshkov 
---
  drivers/gpu/drm/msm/dsi/dsi.c | 10 +
  drivers/gpu/drm/msm/dsi/dsi.h |  5 ++---
  drivers/gpu/drm/msm/dsi/dsi_manager.c | 41 +++
  3 files changed, 21 insertions(+), 35 deletions(-)



LGTM,

Reviewed-by: Abhinav Kumar 


Re: [PATCH 1/2] drm/nouveau/kms/nv50-: Disable AUX bus for disconnected DP ports

2024-04-08 Thread Dave Airlie
On Fri, 5 Apr 2024 at 09:37, Lyude Paul  wrote:
>
> GSP has its own state for keeping track of whether or not a given display
> connector is plugged in or not, and enforces this state on the driver. In
> particular, AUX transactions on a DisplayPort connector which GSP says is
> disconnected can never succeed - and can in some cases even cause
> unexpected timeouts, which can trickle up to cause other problems. A good
> example of this is runtime power management: where we can actually get
> stuck trying to resume the GPU if a userspace application like fwupd tries
> accessing a drm_aux_dev for a disconnected port. This was an issue I hit a
> few times with my Slimbook Executive 16 - where trying to offload something
> to the discrete GPU would wake it up, and then potentially cause it to
> timeout as fwupd tried to immediately access the dp_aux_dev nodes for
> nouveau.
>
> Likewise: we don't really have any cases I know of where we'd want to
> ignore this state and try an aux transaction anyway - and failing pointless
> aux transactions immediately can even speed things up. So - let's start
> enabling/disabling the aux bus in nouveau_dp_detect() to fix this. We
> enable the aux bus during connector probing, and leave it enabled if we
> discover something is actually on the connector. Otherwise, we just shut it
> off.
>
> This should fix some people's runtime PM issues (like myself), and also get
> rid of quite of a lot of GSP error spam in dmesg.
>
> Signed-off-by: Lyude Paul 


For the two patches,

Reviewed-by: Dave Airlie 

> ---
>  drivers/gpu/drm/nouveau/nouveau_dp.c | 10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_dp.c 
> b/drivers/gpu/drm/nouveau/nouveau_dp.c
> index fb06ee17d9e54..8b1be7dd64ebe 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_dp.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_dp.c
> @@ -232,6 +232,9 @@ nouveau_dp_detect(struct nouveau_connector *nv_connector,
> dpcd[DP_DPCD_REV] != 0)
> return NOUVEAU_DP_SST;
>
> +   // Ensure that the aux bus is enabled for probing
> +   drm_dp_dpcd_set_powered(_connector->aux, true);
> +
> mutex_lock(_encoder->dp.hpd_irq_lock);
> if (mstm) {
> /* If we're not ready to handle MST state changes yet, just
> @@ -293,6 +296,13 @@ nouveau_dp_detect(struct nouveau_connector *nv_connector,
> if (mstm && !mstm->suspended && ret != NOUVEAU_DP_MST)
> nv50_mstm_remove(mstm);
>
> +   /* GSP doesn't like when we try to do aux transactions on a port it 
> considers disconnected,
> +* and since we don't really have a usecase for that anyway - just 
> disable the aux bus here
> +* if we've decided the connector is disconnected
> +*/
> +   if (ret == NOUVEAU_DP_NONE)
> +   drm_dp_dpcd_set_powered(_connector->aux, false);
> +
> mutex_unlock(_encoder->dp.hpd_irq_lock);
> return ret;
>  }
> --
> 2.44.0
>


Re: [PATCH v3] drm/msm/dp: call dp_hpd_plug_handle()/unplug_handle() directly for external HPD

2024-04-08 Thread Dmitry Baryshkov
On Tue, 9 Apr 2024 at 00:17, Abhinav Kumar  wrote:
>
>
>
> On 4/8/2024 2:13 PM, Dmitry Baryshkov wrote:
> > On Tue, 9 Apr 2024 at 00:08, Abhinav Kumar  
> > wrote:
> >>
> >>
> >>
> >> On 4/8/2024 1:41 PM, Bjorn Andersson wrote:
> >>> On Mon, Apr 08, 2024 at 12:43:34PM -0700, Abhinav Kumar wrote:
> 
> 
>  On 4/7/2024 11:48 AM, Bjorn Andersson wrote:
> > On Fri, Apr 05, 2024 at 08:15:47PM -0700, Abhinav Kumar wrote:
> >> From: Kuogee Hsieh 
> > [..]
> >> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
> >> b/drivers/gpu/drm/msm/dp/dp_display.c
> >> index d80f89581760..bfb6dfff27e8 100644
> >> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> >> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> >> @@ -1665,7 +1665,7 @@ void dp_bridge_hpd_notify(struct drm_bridge 
> >> *bridge,
> >> return;
> >> if (!dp_display->link_ready && status == 
> >> connector_status_connected)
> >> -  dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
> >> +  dp_hpd_plug_handle(dp, 0);
> >
> > If I read the code correctly, and we get an external connect event
> > inbetween a previous disconnect and the related disable call, this
> > should result in a PLUG_INT being injected into the queue still.
> >
> > Will that not cause the same problem?
> >
> > Regards,
> > Bjorn
> >
> 
>  Yes, your observation is correct and I had asked the same question to 
>  kuogee
>  before taking over this change and posting.
> 
>  We will have to handle that case separately. I don't have a good solution
>  yet for it without requiring further rework or we drop the below snippet.
> 
>    if (state == ST_DISCONNECT_PENDING) {
>    /* wait until ST_DISCONNECTED */
>    dp_add_event(dp, EV_HPD_PLUG_INT, 0, 1); /* delay = 1 
>  */
>    mutex_unlock(>event_mutex);
>    return 0;
>    }
> 
>  I will need sometime to address that use-case as I need to see if we can
>  handle that better and then drop the the DISCONNECT_PENDING state to 
>  address
>  this fully. But it needs more testing.
> 
>  But, we will need this patch anyway because without this we will not be 
>  able
>  to fix even the most regular and commonly seen case of basic
>  connect/disconnect receiving complementary events.
> 
> >>>
> >>> I did some more testing on this patch. Connecting and disconnecting the
> >>> cable while in fbcon works reliably, except for:
> >>
> >> Thanks for the tests !
> >>
> >>> - edid/modes are not read before we bring up the link so I always end up
> >>> with 640x480
> >>>
> >>
> >> hmmm, I wonder why this should be affected due to this patch. We always
> >> read the EDID during hpd_connect() and the selected resolution will be
> >> programmed with the modeset. We will retry this with our x1e80100 and see.
> >
> > BTW, why is EDID read during HPD handling? I always supposed that it
> > can be read much later, when the DRM framework calls the get_modes /
> > get_edid callbacks.
> >
>
> Well, dp_panel_read_sink_caps() is in dp_display_process_hpd_high()
> currently. We read the edid there.

My question was, why is it done this way? Can it be dropped? There is
no need to store EDID in the driver data,  is it?

>
> get_modes(), parses the EDID and adds the modes using drm_add_edid_modes().
>
> >>
> >>> - if I run modetest -s : the link is brought up with the new
> >>> resolution and I get my test image on the screen.
> >>> But as we're shutting down the link for the resolution chance I end up
> >>> in dp_bridge_hpd_notify() with link_ready && state = disconnected.
> >>> This triggers an unplug which hangs on the event_mutex, such that as
> >>> soon as I get the test image, the state machine enters
> >>> DISCONNECT_PENDING. This is immediately followed by another
> >>> !link_ready && status = connected, which attempts to perform the plug
> >>> operation, but as we're in DISCONNECT_PENDING this is posted on the
> >>> event queue. From there I get a log entry from my PLUG_INT, every
> >>> 100ms stating that we're still in DISCONNECT_PENDING. If I exit
> >>> modetest the screen goes black, and no new mode can be selected,
> >>> because we're in DISCONNECT_PENDING. The only way out is to disconnect
> >>> the cable to complete the DISCONNECT_PENDING.
> >>>
> >>
> >> I am going to run this test-case and see what we can do.
> >>
> >>> Regards,
> >>> Bjorn
> >>>
> 
> >> else if (dp_display->link_ready && status == 
> >> connector_status_disconnected)
> >> -  dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
> >> +  dp_hpd_unplug_handle(dp, 0);
> >> }
> >> --
> >> 2.43.2
> >>
> >
> >
> >



-- 
With best wishes
Dmitry


Re: [PATCH v3] drm/msm/dp: call dp_hpd_plug_handle()/unplug_handle() directly for external HPD

2024-04-08 Thread Dmitry Baryshkov
On Tue, 9 Apr 2024 at 00:23, Abhinav Kumar  wrote:
>
>
>
> On 4/8/2024 2:12 PM, Dmitry Baryshkov wrote:
> > On Mon, 8 Apr 2024 at 22:43, Abhinav Kumar  
> > wrote:
> >>
> >>
> >>
> >> On 4/7/2024 11:48 AM, Bjorn Andersson wrote:
> >>> On Fri, Apr 05, 2024 at 08:15:47PM -0700, Abhinav Kumar wrote:
>  From: Kuogee Hsieh 
> >>> [..]
>  diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
>  b/drivers/gpu/drm/msm/dp/dp_display.c
>  index d80f89581760..bfb6dfff27e8 100644
>  --- a/drivers/gpu/drm/msm/dp/dp_display.c
>  +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>  @@ -1665,7 +1665,7 @@ void dp_bridge_hpd_notify(struct drm_bridge 
>  *bridge,
>    return;
> 
>    if (!dp_display->link_ready && status == 
>  connector_status_connected)
>  -dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
>  +dp_hpd_plug_handle(dp, 0);
> >>>
> >>> If I read the code correctly, and we get an external connect event
> >>> inbetween a previous disconnect and the related disable call, this
> >>> should result in a PLUG_INT being injected into the queue still.
> >>>
> >>> Will that not cause the same problem?
> >>>
> >>> Regards,
> >>> Bjorn
> >>>
> >>
> >> Yes, your observation is correct and I had asked the same question to
> >> kuogee before taking over this change and posting.
> >
> > Should it then have the Co-developed-by trailers?
> >
>
> hmmm, perhaps but that didnt result in any code change between v2 and
> v3, so I didnt add it.

So who authored v0 of it? From your text I had an impression that it
was Kuogee. Please excuse me if I was wrong.

>
> >> We will have to handle that case separately. I don't have a good
> >> solution yet for it without requiring further rework or we drop the
> >> below snippet.
> >>
> >>   if (state == ST_DISCONNECT_PENDING) {
> >>   /* wait until ST_DISCONNECTED */
> >>   dp_add_event(dp, EV_HPD_PLUG_INT, 0, 1); /* delay = 1 */
> >>   mutex_unlock(>event_mutex);
> >>   return 0;
> >>   }
> >>
> >> I will need sometime to address that use-case as I need to see if we can
> >> handle that better and then drop the the DISCONNECT_PENDING state to
> >> address this fully. But it needs more testing.
> >>
> >> But, we will need this patch anyway because without this we will not be
> >> able to fix even the most regular and commonly seen case of basic
> >> connect/disconnect receiving complementary events.
> >
> > Hmm, no. We need to drop the HPD state machine, not to patch it. Once
> > the driver has proper detect() callback, there will be no
> > complementary events. That is a proper way to fix the code, not these
> > kinds of band-aids patches.
> >
>
> I had discussed this part too :)
>
> I totally agree we should fix .detect()'s behavior to just match cable
> connect/disconnect and not link_ready status.
>
> But that alone would not have fixed this issue. If HPD thread does not
> get scheduled and plug_handle() was not executed, .detect() would have
> still returned old status as we will update the cable status only in
> plug_handle() / unplug_handle() to have a common API between internal
> and external hpd execution.

I think there should be just hpd_notify, which if the HPD is up,
attempts to read the DPCD. No need for separate plug/unplug_handle.
The detect() can be as simple as !drm_dp_is_branch() || sink_count != 0.

>
> So we need to do both, make .detect() return correct status AND drop hpd
> event thread processing.
>
> But, dropping the hpd event thread processing alone was fixing the
> complimentary events issue. So kuogee had only this part in this patch.

I'd prefer to wait for the final patchset then. Which has the HPD
thread completely removed.

>
>
>    else if (dp_display->link_ready && status == 
>  connector_status_disconnected)
>  -dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
>  +dp_hpd_unplug_handle(dp, 0);
> }
>  --
>  2.43.2
> 
> >
> >
> >



--
With best wishes
Dmitry


Re: [PATCH 2/3] drm/msm/dsi: move next bridge acquisition to dsi_bind

2024-04-08 Thread Abhinav Kumar




On 4/5/2024 11:15 AM, Dmitry Baryshkov wrote:

On Fri, 5 Apr 2024 at 20:35, Abhinav Kumar  wrote:




On 3/9/2024 7:09 AM, Dmitry Baryshkov wrote:

Currently the MSM DSI driver looks for the next bridge during
msm_dsi_modeset_init(). If the bridge is not registered at that point,
this might result in -EPROBE_DEFER, which can be problematic that late
during the device probe process. Move next bridge acquisition to the
dsi_bind state so that probe deferral is returned as early as possible.



But msm_dsi_modeset)init() is also called during msm_drm_bind() only.

What issue are you suggesting will be fixed by moving this from
msm_drm_bind() to dsi_bind()?


The goal is to return as early as possible as not not cause
probe-deferral loops. See commit fbc35b45f9f6 ("Add documentation on
meaning of -EPROBE_DEFER"). It discusses returning from probe() but
the same logic applies to bind().



Understood. I was trying to make sure the purpose of the patch is that 
"deferral in component_bind() is better than component_master_bind()"


But yes, overall that is better since the unbounding path will be more 
in the master case.



Signed-off-by: Dmitry Baryshkov 
---
   drivers/gpu/drm/msm/dsi/dsi.c | 16 
   drivers/gpu/drm/msm/dsi/dsi.h |  2 ++
   drivers/gpu/drm/msm/dsi/dsi_manager.c |  8 +---
   3 files changed, 19 insertions(+), 7 deletions(-)



Reviewed-by: Abhinav Kumar 


Re: [PATCH v2 6/6] drm/msm/dp: Use function arguments for audio operations

2024-04-08 Thread Abhinav Kumar




On 3/28/2024 7:40 AM, Bjorn Andersson wrote:

From: Bjorn Andersson 

The dp_audio read and write operations uses members in struct dp_catalog
for passing arguments and return values. This adds unnecessary
complexity to the implementation, as it turns out after detangling the
logic that no state is actually held in these variables.

Clean this up by using function arguments and return values for passing
the data.

Reviewed-by: Dmitry Baryshkov 
Signed-off-by: Bjorn Andersson 
---
  drivers/gpu/drm/msm/dp/dp_audio.c   | 20 +--
  drivers/gpu/drm/msm/dp/dp_catalog.c | 39 +
  drivers/gpu/drm/msm/dp/dp_catalog.h | 18 +
  3 files changed, 28 insertions(+), 49 deletions(-)



One quick question, was DP audio re-tested after this patch?


Re: [PATCH v3] drm/msm/dp: call dp_hpd_plug_handle()/unplug_handle() directly for external HPD

2024-04-08 Thread Abhinav Kumar




On 4/8/2024 2:12 PM, Dmitry Baryshkov wrote:

On Mon, 8 Apr 2024 at 22:43, Abhinav Kumar  wrote:




On 4/7/2024 11:48 AM, Bjorn Andersson wrote:

On Fri, Apr 05, 2024 at 08:15:47PM -0700, Abhinav Kumar wrote:

From: Kuogee Hsieh 

[..]

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
b/drivers/gpu/drm/msm/dp/dp_display.c
index d80f89581760..bfb6dfff27e8 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1665,7 +1665,7 @@ void dp_bridge_hpd_notify(struct drm_bridge *bridge,
  return;

  if (!dp_display->link_ready && status == connector_status_connected)
-dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
+dp_hpd_plug_handle(dp, 0);


If I read the code correctly, and we get an external connect event
inbetween a previous disconnect and the related disable call, this
should result in a PLUG_INT being injected into the queue still.

Will that not cause the same problem?

Regards,
Bjorn



Yes, your observation is correct and I had asked the same question to
kuogee before taking over this change and posting.


Should it then have the Co-developed-by trailers?



hmmm, perhaps but that didnt result in any code change between v2 and 
v3, so I didnt add it.



We will have to handle that case separately. I don't have a good
solution yet for it without requiring further rework or we drop the
below snippet.

  if (state == ST_DISCONNECT_PENDING) {
  /* wait until ST_DISCONNECTED */
  dp_add_event(dp, EV_HPD_PLUG_INT, 0, 1); /* delay = 1 */
  mutex_unlock(>event_mutex);
  return 0;
  }

I will need sometime to address that use-case as I need to see if we can
handle that better and then drop the the DISCONNECT_PENDING state to
address this fully. But it needs more testing.

But, we will need this patch anyway because without this we will not be
able to fix even the most regular and commonly seen case of basic
connect/disconnect receiving complementary events.


Hmm, no. We need to drop the HPD state machine, not to patch it. Once
the driver has proper detect() callback, there will be no
complementary events. That is a proper way to fix the code, not these
kinds of band-aids patches.



I had discussed this part too :)

I totally agree we should fix .detect()'s behavior to just match cable 
connect/disconnect and not link_ready status.


But that alone would not have fixed this issue. If HPD thread does not 
get scheduled and plug_handle() was not executed, .detect() would have 
still returned old status as we will update the cable status only in 
plug_handle() / unplug_handle() to have a common API between internal 
and external hpd execution.


So we need to do both, make .detect() return correct status AND drop hpd 
event thread processing.


But, dropping the hpd event thread processing alone was fixing the 
complimentary events issue. So kuogee had only this part in this patch.




  else if (dp_display->link_ready && status == 
connector_status_disconnected)
-dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
+dp_hpd_unplug_handle(dp, 0);
   }
--
2.43.2







Re: [PATCH 4/5] drm/nouveau: Use drm_crtc_vblank_crtc()

2024-04-08 Thread Lyude Paul
Reviewed-by: Lyude Paul 

On Mon, 2024-04-08 at 22:06 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Replace the open coded drm_crtc_vblank_crtc() with the real
> thing.
> 
> Cc: Karol Herbst 
> Cc: Lyude Paul 
> Cc: Danilo Krummrich 
> Cc: nouv...@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/nouveau/nouveau_display.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c
> b/drivers/gpu/drm/nouveau/nouveau_display.c
> index f28f9a857458..aed5d5b51b43 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> @@ -83,7 +83,7 @@ static bool
>  nouveau_display_scanoutpos_head(struct drm_crtc *crtc, int *vpos,
> int *hpos,
>   ktime_t *stime, ktime_t *etime)
>  {
> - struct drm_vblank_crtc *vblank = >dev-
> >vblank[drm_crtc_index(crtc)];
> + struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
>   struct nvif_head *head = _crtc(crtc)->head;
>   struct nvif_head_scanoutpos_v0 args;
>   int retry = 20;

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat



Re: [PATCH v3] drm/msm/dp: call dp_hpd_plug_handle()/unplug_handle() directly for external HPD

2024-04-08 Thread Abhinav Kumar




On 4/8/2024 2:13 PM, Dmitry Baryshkov wrote:

On Tue, 9 Apr 2024 at 00:08, Abhinav Kumar  wrote:




On 4/8/2024 1:41 PM, Bjorn Andersson wrote:

On Mon, Apr 08, 2024 at 12:43:34PM -0700, Abhinav Kumar wrote:



On 4/7/2024 11:48 AM, Bjorn Andersson wrote:

On Fri, Apr 05, 2024 at 08:15:47PM -0700, Abhinav Kumar wrote:

From: Kuogee Hsieh 

[..]

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
b/drivers/gpu/drm/msm/dp/dp_display.c
index d80f89581760..bfb6dfff27e8 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1665,7 +1665,7 @@ void dp_bridge_hpd_notify(struct drm_bridge *bridge,
return;
if (!dp_display->link_ready && status == connector_status_connected)
-  dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
+  dp_hpd_plug_handle(dp, 0);


If I read the code correctly, and we get an external connect event
inbetween a previous disconnect and the related disable call, this
should result in a PLUG_INT being injected into the queue still.

Will that not cause the same problem?

Regards,
Bjorn



Yes, your observation is correct and I had asked the same question to kuogee
before taking over this change and posting.

We will have to handle that case separately. I don't have a good solution
yet for it without requiring further rework or we drop the below snippet.

  if (state == ST_DISCONNECT_PENDING) {
  /* wait until ST_DISCONNECTED */
  dp_add_event(dp, EV_HPD_PLUG_INT, 0, 1); /* delay = 1 */
  mutex_unlock(>event_mutex);
  return 0;
  }

I will need sometime to address that use-case as I need to see if we can
handle that better and then drop the the DISCONNECT_PENDING state to address
this fully. But it needs more testing.

But, we will need this patch anyway because without this we will not be able
to fix even the most regular and commonly seen case of basic
connect/disconnect receiving complementary events.



I did some more testing on this patch. Connecting and disconnecting the
cable while in fbcon works reliably, except for:


Thanks for the tests !


- edid/modes are not read before we bring up the link so I always end up
with 640x480



hmmm, I wonder why this should be affected due to this patch. We always
read the EDID during hpd_connect() and the selected resolution will be
programmed with the modeset. We will retry this with our x1e80100 and see.


BTW, why is EDID read during HPD handling? I always supposed that it
can be read much later, when the DRM framework calls the get_modes /
get_edid callbacks.



Well, dp_panel_read_sink_caps() is in dp_display_process_hpd_high() 
currently. We read the edid there.


get_modes(), parses the EDID and adds the modes using drm_add_edid_modes().




- if I run modetest -s : the link is brought up with the new
resolution and I get my test image on the screen.
But as we're shutting down the link for the resolution chance I end up
in dp_bridge_hpd_notify() with link_ready && state = disconnected.
This triggers an unplug which hangs on the event_mutex, such that as
soon as I get the test image, the state machine enters
DISCONNECT_PENDING. This is immediately followed by another
!link_ready && status = connected, which attempts to perform the plug
operation, but as we're in DISCONNECT_PENDING this is posted on the
event queue. From there I get a log entry from my PLUG_INT, every
100ms stating that we're still in DISCONNECT_PENDING. If I exit
modetest the screen goes black, and no new mode can be selected,
because we're in DISCONNECT_PENDING. The only way out is to disconnect
the cable to complete the DISCONNECT_PENDING.



I am going to run this test-case and see what we can do.


Regards,
Bjorn




else if (dp_display->link_ready && status == 
connector_status_disconnected)
-  dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
+  dp_hpd_unplug_handle(dp, 0);
}
--
2.43.2







Re: [PATCH v3] drm/msm/dp: call dp_hpd_plug_handle()/unplug_handle() directly for external HPD

2024-04-08 Thread Dmitry Baryshkov
On Tue, 9 Apr 2024 at 00:08, Abhinav Kumar  wrote:
>
>
>
> On 4/8/2024 1:41 PM, Bjorn Andersson wrote:
> > On Mon, Apr 08, 2024 at 12:43:34PM -0700, Abhinav Kumar wrote:
> >>
> >>
> >> On 4/7/2024 11:48 AM, Bjorn Andersson wrote:
> >>> On Fri, Apr 05, 2024 at 08:15:47PM -0700, Abhinav Kumar wrote:
>  From: Kuogee Hsieh 
> >>> [..]
>  diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
>  b/drivers/gpu/drm/msm/dp/dp_display.c
>  index d80f89581760..bfb6dfff27e8 100644
>  --- a/drivers/gpu/drm/msm/dp/dp_display.c
>  +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>  @@ -1665,7 +1665,7 @@ void dp_bridge_hpd_notify(struct drm_bridge 
>  *bridge,
> return;
> if (!dp_display->link_ready && status == 
>  connector_status_connected)
>  -  dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
>  +  dp_hpd_plug_handle(dp, 0);
> >>>
> >>> If I read the code correctly, and we get an external connect event
> >>> inbetween a previous disconnect and the related disable call, this
> >>> should result in a PLUG_INT being injected into the queue still.
> >>>
> >>> Will that not cause the same problem?
> >>>
> >>> Regards,
> >>> Bjorn
> >>>
> >>
> >> Yes, your observation is correct and I had asked the same question to 
> >> kuogee
> >> before taking over this change and posting.
> >>
> >> We will have to handle that case separately. I don't have a good solution
> >> yet for it without requiring further rework or we drop the below snippet.
> >>
> >>  if (state == ST_DISCONNECT_PENDING) {
> >>  /* wait until ST_DISCONNECTED */
> >>  dp_add_event(dp, EV_HPD_PLUG_INT, 0, 1); /* delay = 1 */
> >>  mutex_unlock(>event_mutex);
> >>  return 0;
> >>  }
> >>
> >> I will need sometime to address that use-case as I need to see if we can
> >> handle that better and then drop the the DISCONNECT_PENDING state to 
> >> address
> >> this fully. But it needs more testing.
> >>
> >> But, we will need this patch anyway because without this we will not be 
> >> able
> >> to fix even the most regular and commonly seen case of basic
> >> connect/disconnect receiving complementary events.
> >>
> >
> > I did some more testing on this patch. Connecting and disconnecting the
> > cable while in fbcon works reliably, except for:
>
> Thanks for the tests !
>
> > - edid/modes are not read before we bring up the link so I always end up
> >with 640x480
> >
>
> hmmm, I wonder why this should be affected due to this patch. We always
> read the EDID during hpd_connect() and the selected resolution will be
> programmed with the modeset. We will retry this with our x1e80100 and see.

BTW, why is EDID read during HPD handling? I always supposed that it
can be read much later, when the DRM framework calls the get_modes /
get_edid callbacks.

>
> > - if I run modetest -s : the link is brought up with the new
> >resolution and I get my test image on the screen.
> >But as we're shutting down the link for the resolution chance I end up
> >in dp_bridge_hpd_notify() with link_ready && state = disconnected.
> >This triggers an unplug which hangs on the event_mutex, such that as
> >soon as I get the test image, the state machine enters
> >DISCONNECT_PENDING. This is immediately followed by another
> >!link_ready && status = connected, which attempts to perform the plug
> >operation, but as we're in DISCONNECT_PENDING this is posted on the
> >event queue. From there I get a log entry from my PLUG_INT, every
> >100ms stating that we're still in DISCONNECT_PENDING. If I exit
> >modetest the screen goes black, and no new mode can be selected,
> >because we're in DISCONNECT_PENDING. The only way out is to disconnect
> >the cable to complete the DISCONNECT_PENDING.
> >
>
> I am going to run this test-case and see what we can do.
>
> > Regards,
> > Bjorn
> >
> >>
> else if (dp_display->link_ready && status == 
>  connector_status_disconnected)
>  -  dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
>  +  dp_hpd_unplug_handle(dp, 0);
> }
>  --
>  2.43.2
> 



-- 
With best wishes
Dmitry


Re: [PATCH v3] drm/msm/dp: call dp_hpd_plug_handle()/unplug_handle() directly for external HPD

2024-04-08 Thread Dmitry Baryshkov
On Mon, 8 Apr 2024 at 22:43, Abhinav Kumar  wrote:
>
>
>
> On 4/7/2024 11:48 AM, Bjorn Andersson wrote:
> > On Fri, Apr 05, 2024 at 08:15:47PM -0700, Abhinav Kumar wrote:
> >> From: Kuogee Hsieh 
> > [..]
> >> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
> >> b/drivers/gpu/drm/msm/dp/dp_display.c
> >> index d80f89581760..bfb6dfff27e8 100644
> >> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> >> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> >> @@ -1665,7 +1665,7 @@ void dp_bridge_hpd_notify(struct drm_bridge *bridge,
> >>  return;
> >>
> >>  if (!dp_display->link_ready && status == connector_status_connected)
> >> -dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
> >> +dp_hpd_plug_handle(dp, 0);
> >
> > If I read the code correctly, and we get an external connect event
> > inbetween a previous disconnect and the related disable call, this
> > should result in a PLUG_INT being injected into the queue still.
> >
> > Will that not cause the same problem?
> >
> > Regards,
> > Bjorn
> >
>
> Yes, your observation is correct and I had asked the same question to
> kuogee before taking over this change and posting.

Should it then have the Co-developed-by trailers?

> We will have to handle that case separately. I don't have a good
> solution yet for it without requiring further rework or we drop the
> below snippet.
>
>  if (state == ST_DISCONNECT_PENDING) {
>  /* wait until ST_DISCONNECTED */
>  dp_add_event(dp, EV_HPD_PLUG_INT, 0, 1); /* delay = 1 */
>  mutex_unlock(>event_mutex);
>  return 0;
>  }
>
> I will need sometime to address that use-case as I need to see if we can
> handle that better and then drop the the DISCONNECT_PENDING state to
> address this fully. But it needs more testing.
>
> But, we will need this patch anyway because without this we will not be
> able to fix even the most regular and commonly seen case of basic
> connect/disconnect receiving complementary events.

Hmm, no. We need to drop the HPD state machine, not to patch it. Once
the driver has proper detect() callback, there will be no
complementary events. That is a proper way to fix the code, not these
kinds of band-aids patches.

> >>  else if (dp_display->link_ready && status == 
> >> connector_status_disconnected)
> >> -dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
> >> +dp_hpd_unplug_handle(dp, 0);
> >>   }
> >> --
> >> 2.43.2
> >>



-- 
With best wishes
Dmitry


[PATCH] drm/amdgpu: remove "num_pages" local variable in amdgpu_gtt_mgr_new

2024-04-08 Thread brolerliew
From: brolerliew 

amdgpu_gtt_mgr_new and ttm_range_man_alloc share similar logic, but
"num_pages" in amdgpu_gtt_mgr_new is defined as local variable which
is calculate directly in ttm_range_man_alloc.

Signed-off-by: brolerliew <575705...@qq.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 44367f03316f..0c56e4057d85 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -116,7 +116,6 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager 
*man,
  struct ttm_resource **res)
 {
struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
-   uint32_t num_pages = PFN_UP(tbo->base.size);
struct ttm_range_mgr_node *node;
int r;
 
@@ -134,7 +133,7 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager 
*man,
if (place->lpfn) {
spin_lock(>lock);
r = drm_mm_insert_node_in_range(>mm, >mm_nodes[0],
-   num_pages, tbo->page_alignment,
+   PFN_UP(node->base.size), 
tbo->page_alignment,
0, place->fpfn, place->lpfn,
DRM_MM_INSERT_BEST);
spin_unlock(>lock);
-- 
2.40.1



Re: [RFC PATCH net-next v8 14/14] selftests: add ncdevmem, netcat for devmem TCP

2024-04-08 Thread Cong Wang
On Tue, Apr 02, 2024 at 05:20:51PM -0700, Mina Almasry wrote:
> +static char *server_ip = "192.168.1.4";
> +static char *client_ip = "192.168.1.2";
> +static char *port = "5201";
> +static size_t do_validation;
> +static int start_queue = 8;
> +static int num_queues = 8;
> +static char *ifname = "eth1";
> +static unsigned int ifindex = 3;
> +static char *nic_pci_addr = ":06:00.0";

It seems this is set but never used.

Thanks.


Re: [RFC PATCH net-next v6 02/15] net: page_pool: create hooks for custom page providers

2024-04-08 Thread Cong Wang
On Mon, Apr 01, 2024 at 12:22:24PM -0700, Mina Almasry wrote:
> On Thu, Mar 28, 2024 at 12:31 AM Christoph Hellwig  wrote:
> >
> > On Tue, Mar 26, 2024 at 01:19:20PM -0700, Mina Almasry wrote:
> > >
> > > Are you envisioning that dmabuf support would be added to the block
> > > layer
> >
> > Yes.
> >
> > > (which I understand is part of the VFS and not driver specific),
> >
> > The block layer isn't really the VFS, it's just another core stack
> > like the network stack.
> >
> > > or as part of the specific storage driver (like nvme for example)? If
> > > we can add dmabuf support to the block layer itself that sounds
> > > awesome. We may then be able to do devmem TCP on all/most storage
> > > devices without having to modify each individual driver.
> >
> > I suspect we'll still need to touch the drivers to understand it,
> > but hopefully all the main infrastructure can live in the block layer.
> >
> > > In your estimation, is adding dmabuf support to the block layer
> > > something technically feasible & acceptable upstream? I notice you
> > > suggested it so I'm guessing yes to both, but I thought I'd confirm.
> >
> > I think so, and I know there has been quite some interest to at least
> > pre-register userspace memory so that the iommu overhead can be
> > pre-loaded.  It also is a much better interface for Peer to Peer
> > transfers than what we currently have.
> >

Thanks for copying me on this. This sounds really great. 

Also P2PDMA requires PCI root complex to support this kind of direct transfer,
and IIUC dmabuf does not have such hardware dependency.

> 
> I think this is positively thrilling news for me. I was worried that
> adding devmemTCP support to storage devices would involve using a
> non-dmabuf standard of buffer sharing like pci_p2pdma_
> (drivers/pci/p2pdma.c) and that would require messy changes to
> pci_p2pdma_ that would get nacked. Also it would require adding
> pci_p2pdma_ support to devmem TCP, which is a can of worms. If adding
> dma-buf support to storage devices is feasible and desirable, that's a
> much better approach IMO. (a) it will maybe work with devmem TCP
> without any changes needed on the netdev side of things and (b)
> dma-buf support may be generically useful and a good contribution even
> outside of devmem TCP.

I think the major difference is its interface, which exposes an mmap memory
region instead of fd: https://lwn.net/Articles/906092/.

> 
> I don't have a concrete user for devmem TCP for storage devices but
> the use case is very similar to GPU and I imagine the benefits in perf
> can be significant in some setups.

We have storage use cases at ByteDance, we use NVME SSD to cache videos
transferred through network, so moving data directly from SSD to NIC
would help a lot.

Thanks!


Re: [PATCH v3] drm/msm/dp: call dp_hpd_plug_handle()/unplug_handle() directly for external HPD

2024-04-08 Thread Abhinav Kumar




On 4/8/2024 1:41 PM, Bjorn Andersson wrote:

On Mon, Apr 08, 2024 at 12:43:34PM -0700, Abhinav Kumar wrote:



On 4/7/2024 11:48 AM, Bjorn Andersson wrote:

On Fri, Apr 05, 2024 at 08:15:47PM -0700, Abhinav Kumar wrote:

From: Kuogee Hsieh 

[..]

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
b/drivers/gpu/drm/msm/dp/dp_display.c
index d80f89581760..bfb6dfff27e8 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1665,7 +1665,7 @@ void dp_bridge_hpd_notify(struct drm_bridge *bridge,
return;
if (!dp_display->link_ready && status == connector_status_connected)
-   dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
+   dp_hpd_plug_handle(dp, 0);


If I read the code correctly, and we get an external connect event
inbetween a previous disconnect and the related disable call, this
should result in a PLUG_INT being injected into the queue still.

Will that not cause the same problem?

Regards,
Bjorn



Yes, your observation is correct and I had asked the same question to kuogee
before taking over this change and posting.

We will have to handle that case separately. I don't have a good solution
yet for it without requiring further rework or we drop the below snippet.

 if (state == ST_DISCONNECT_PENDING) {
 /* wait until ST_DISCONNECTED */
 dp_add_event(dp, EV_HPD_PLUG_INT, 0, 1); /* delay = 1 */
 mutex_unlock(>event_mutex);
 return 0;
 }

I will need sometime to address that use-case as I need to see if we can
handle that better and then drop the the DISCONNECT_PENDING state to address
this fully. But it needs more testing.

But, we will need this patch anyway because without this we will not be able
to fix even the most regular and commonly seen case of basic
connect/disconnect receiving complementary events.



I did some more testing on this patch. Connecting and disconnecting the
cable while in fbcon works reliably, except for:


Thanks for the tests !


- edid/modes are not read before we bring up the link so I always end up
   with 640x480



hmmm, I wonder why this should be affected due to this patch. We always 
read the EDID during hpd_connect() and the selected resolution will be 
programmed with the modeset. We will retry this with our x1e80100 and see.



- if I run modetest -s : the link is brought up with the new
   resolution and I get my test image on the screen.
   But as we're shutting down the link for the resolution chance I end up
   in dp_bridge_hpd_notify() with link_ready && state = disconnected.
   This triggers an unplug which hangs on the event_mutex, such that as
   soon as I get the test image, the state machine enters
   DISCONNECT_PENDING. This is immediately followed by another
   !link_ready && status = connected, which attempts to perform the plug
   operation, but as we're in DISCONNECT_PENDING this is posted on the
   event queue. From there I get a log entry from my PLUG_INT, every
   100ms stating that we're still in DISCONNECT_PENDING. If I exit
   modetest the screen goes black, and no new mode can be selected,
   because we're in DISCONNECT_PENDING. The only way out is to disconnect
   the cable to complete the DISCONNECT_PENDING.



I am going to run this test-case and see what we can do.


Regards,
Bjorn




else if (dp_display->link_ready && status == 
connector_status_disconnected)
-   dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
+   dp_hpd_unplug_handle(dp, 0);
   }
--
2.43.2



Re: [PATCH v3] drm/msm/dp: call dp_hpd_plug_handle()/unplug_handle() directly for external HPD

2024-04-08 Thread Bjorn Andersson
On Mon, Apr 08, 2024 at 12:43:34PM -0700, Abhinav Kumar wrote:
> 
> 
> On 4/7/2024 11:48 AM, Bjorn Andersson wrote:
> > On Fri, Apr 05, 2024 at 08:15:47PM -0700, Abhinav Kumar wrote:
> > > From: Kuogee Hsieh 
> > [..]
> > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
> > > b/drivers/gpu/drm/msm/dp/dp_display.c
> > > index d80f89581760..bfb6dfff27e8 100644
> > > --- a/drivers/gpu/drm/msm/dp/dp_display.c
> > > +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> > > @@ -1665,7 +1665,7 @@ void dp_bridge_hpd_notify(struct drm_bridge *bridge,
> > >   return;
> > >   if (!dp_display->link_ready && status == 
> > > connector_status_connected)
> > > - dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
> > > + dp_hpd_plug_handle(dp, 0);
> > 
> > If I read the code correctly, and we get an external connect event
> > inbetween a previous disconnect and the related disable call, this
> > should result in a PLUG_INT being injected into the queue still.
> > 
> > Will that not cause the same problem?
> > 
> > Regards,
> > Bjorn
> > 
> 
> Yes, your observation is correct and I had asked the same question to kuogee
> before taking over this change and posting.
> 
> We will have to handle that case separately. I don't have a good solution
> yet for it without requiring further rework or we drop the below snippet.
> 
> if (state == ST_DISCONNECT_PENDING) {
> /* wait until ST_DISCONNECTED */
> dp_add_event(dp, EV_HPD_PLUG_INT, 0, 1); /* delay = 1 */
> mutex_unlock(>event_mutex);
> return 0;
> }
> 
> I will need sometime to address that use-case as I need to see if we can
> handle that better and then drop the the DISCONNECT_PENDING state to address
> this fully. But it needs more testing.
> 
> But, we will need this patch anyway because without this we will not be able
> to fix even the most regular and commonly seen case of basic
> connect/disconnect receiving complementary events.
> 

I did some more testing on this patch. Connecting and disconnecting the
cable while in fbcon works reliably, except for:
- edid/modes are not read before we bring up the link so I always end up
  with 640x480

- if I run modetest -s : the link is brought up with the new
  resolution and I get my test image on the screen.
  But as we're shutting down the link for the resolution chance I end up
  in dp_bridge_hpd_notify() with link_ready && state = disconnected.
  This triggers an unplug which hangs on the event_mutex, such that as
  soon as I get the test image, the state machine enters
  DISCONNECT_PENDING. This is immediately followed by another
  !link_ready && status = connected, which attempts to perform the plug
  operation, but as we're in DISCONNECT_PENDING this is posted on the
  event queue. From there I get a log entry from my PLUG_INT, every
  100ms stating that we're still in DISCONNECT_PENDING. If I exit
  modetest the screen goes black, and no new mode can be selected,
  because we're in DISCONNECT_PENDING. The only way out is to disconnect
  the cable to complete the DISCONNECT_PENDING.

Regards,
Bjorn

> 
> > >   else if (dp_display->link_ready && status == 
> > > connector_status_disconnected)
> > > - dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
> > > + dp_hpd_unplug_handle(dp, 0);
> > >   }
> > > -- 
> > > 2.43.2
> > > 


Re: [PATCH -next] drm/amd/display: delete the redundant initialization in dcn3_51_soc

2024-04-08 Thread Alex Deucher
Acked-by: Alex Deucher 

Applied.

Thanks,

Alex

On Mon, Apr 8, 2024 at 3:45 AM Xiang Yang  wrote:
>
> the dram_clock_change_latency_us in dcn3_51_soc is initialized twice, so
> delete one of them.
>
> Signed-off-by: Xiang Yang 
> ---
>  drivers/gpu/drm/amd/display/dc/dml/dcn351/dcn351_fpu.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn351/dcn351_fpu.c 
> b/drivers/gpu/drm/amd/display/dc/dml/dcn351/dcn351_fpu.c
> index b3ffab77cf88..f1c0d5b77f1b 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn351/dcn351_fpu.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn351/dcn351_fpu.c
> @@ -237,7 +237,6 @@ struct _vcs_dpi_soc_bounding_box_st dcn3_51_soc = {
> .urgent_latency_adjustment_fabric_clock_component_us = 0,
> .urgent_latency_adjustment_fabric_clock_reference_mhz = 0,
> .num_chans = 4,
> -   .dram_clock_change_latency_us = 11.72,
> .dispclk_dppclk_vco_speed_mhz = 2400.0,
>  };
>
> --
> 2.34.1
>


Re: [PATCH v2] drm/radeon/radeon_display: Decrease the size of allocated memory

2024-04-08 Thread Alex Deucher
On Mon, Apr 1, 2024 at 8:35 AM Christian König  wrote:
>
> Am 30.03.24 um 17:34 schrieb Erick Archer:
> > This is an effort to get rid of all multiplications from allocation
> > functions in order to prevent integer overflows [1] [2].
> >
> > In this case, the memory allocated to store RADEONFB_CONN_LIMIT pointers
> > to "drm_connector" structures can be avoided. This is because this
> > memory area is never accessed.
> >
> > Also, in the kzalloc function, it is preferred to use sizeof(*pointer)
> > instead of sizeof(type) due to the type of the variable can change and
> > one needs not change the former (unlike the latter).
> >
> > At the same time take advantage to remove the "#if 0" block, the code
> > where the removed memory area was accessed, and the RADEONFB_CONN_LIMIT
> > constant due to now is never used.
> >
> > Link: 
> > https://www.kernel.org/doc/html/latest/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments
> >  [1]
> > Link: https://github.com/KSPP/linux/issues/160 [2]
> > Signed-off-by: Erick Archer 
>
> Well in general we don't do any new feature development any more for the
> radeon driver.
>
> But this cleanup looks so straight forward that the risk of breaking
> something is probably very low.
>
> Acked-by from my side, but Alex should probably take a look as well.

I can't remember why that was done that way.  Maybe some leftover from
the early KMS days before we finalized the fbdev interactions?
Anyway, patch applied.  Thanks.

Alex

>
> Regards,
> Christian.
>
> > ---
> > Changes in v2:
> > - Rebase against linux-next.
> >
> > Previous versions:
> > v1 -> 
> > https://lore.kernel.org/linux-hardening/20240222180431.7451-1-erick.arc...@gmx.com/
> >
> > Hi everyone,
> >
> > Any comments would be greatly appreciated. The first version was
> > not commented.
> >
> > Thanks,
> > Erick
> > ---
> >   drivers/gpu/drm/radeon/radeon.h | 1 -
> >   drivers/gpu/drm/radeon/radeon_display.c | 8 +---
> >   2 files changed, 1 insertion(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/radeon/radeon.h 
> > b/drivers/gpu/drm/radeon/radeon.h
> > index 3e5ff17e3caf..0999c8eaae94 100644
> > --- a/drivers/gpu/drm/radeon/radeon.h
> > +++ b/drivers/gpu/drm/radeon/radeon.h
> > @@ -132,7 +132,6 @@ extern int radeon_cik_support;
> >   /* RADEON_IB_POOL_SIZE must be a power of 2 */
> >   #define RADEON_IB_POOL_SIZE 16
> >   #define RADEON_DEBUGFS_MAX_COMPONENTS   32
> > -#define RADEONFB_CONN_LIMIT  4
> >   #define RADEON_BIOS_NUM_SCRATCH 8
> >
> >   /* internal ring indices */
> > diff --git a/drivers/gpu/drm/radeon/radeon_display.c 
> > b/drivers/gpu/drm/radeon/radeon_display.c
> > index efd18c8d84c8..5f1d24d3120c 100644
> > --- a/drivers/gpu/drm/radeon/radeon_display.c
> > +++ b/drivers/gpu/drm/radeon/radeon_display.c
> > @@ -683,7 +683,7 @@ static void radeon_crtc_init(struct drm_device *dev, 
> > int index)
> >   struct radeon_device *rdev = dev->dev_private;
> >   struct radeon_crtc *radeon_crtc;
> >
> > - radeon_crtc = kzalloc(sizeof(struct radeon_crtc) + 
> > (RADEONFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL);
> > + radeon_crtc = kzalloc(sizeof(*radeon_crtc), GFP_KERNEL);
> >   if (radeon_crtc == NULL)
> >   return;
> >
> > @@ -709,12 +709,6 @@ static void radeon_crtc_init(struct drm_device *dev, 
> > int index)
> >   dev->mode_config.cursor_width = radeon_crtc->max_cursor_width;
> >   dev->mode_config.cursor_height = radeon_crtc->max_cursor_height;
> >
> > -#if 0
> > - radeon_crtc->mode_set.crtc = _crtc->base;
> > - radeon_crtc->mode_set.connectors = (struct drm_connector 
> > **)(radeon_crtc + 1);
> > - radeon_crtc->mode_set.num_connectors = 0;
> > -#endif
> > -
> >   if (rdev->is_atom_bios && (ASIC_IS_AVIVO(rdev) || radeon_r4xx_atom))
> >   radeon_atombios_init_crtc(dev, radeon_crtc);
> >   else
>


Re: [PATCH v3] drm/msm/dp: call dp_hpd_plug_handle()/unplug_handle() directly for external HPD

2024-04-08 Thread Abhinav Kumar




On 4/7/2024 11:48 AM, Bjorn Andersson wrote:

On Fri, Apr 05, 2024 at 08:15:47PM -0700, Abhinav Kumar wrote:

From: Kuogee Hsieh 

[..]

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
b/drivers/gpu/drm/msm/dp/dp_display.c
index d80f89581760..bfb6dfff27e8 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1665,7 +1665,7 @@ void dp_bridge_hpd_notify(struct drm_bridge *bridge,
return;
  
  	if (!dp_display->link_ready && status == connector_status_connected)

-   dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
+   dp_hpd_plug_handle(dp, 0);


If I read the code correctly, and we get an external connect event
inbetween a previous disconnect and the related disable call, this
should result in a PLUG_INT being injected into the queue still.

Will that not cause the same problem?

Regards,
Bjorn



Yes, your observation is correct and I had asked the same question to 
kuogee before taking over this change and posting.


We will have to handle that case separately. I don't have a good 
solution yet for it without requiring further rework or we drop the 
below snippet.


if (state == ST_DISCONNECT_PENDING) {
/* wait until ST_DISCONNECTED */
dp_add_event(dp, EV_HPD_PLUG_INT, 0, 1); /* delay = 1 */
mutex_unlock(>event_mutex);
return 0;
}

I will need sometime to address that use-case as I need to see if we can 
handle that better and then drop the the DISCONNECT_PENDING state to 
address this fully. But it needs more testing.


But, we will need this patch anyway because without this we will not be 
able to fix even the most regular and commonly seen case of basic 
connect/disconnect receiving complementary events.




else if (dp_display->link_ready && status == 
connector_status_disconnected)
-   dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
+   dp_hpd_unplug_handle(dp, 0);
  }
--
2.43.2



[PATCH 5/5] drm/vkms: Use drm_crtc_vblank_crtc()

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

Replace the open coded drm_crtc_vblank_crtc() with the real
thing.

Cc: Rodrigo Siqueira 
Cc: Melissa Wen 
Cc: "Maíra Canal" 
Cc: Haneen Mohammed 
Cc: Daniel Vetter 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/vkms/vkms_crtc.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 61e500b8c9da..40b4d084e3ce 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -61,9 +61,7 @@ static enum hrtimer_restart vkms_vblank_simulate(struct 
hrtimer *timer)
 
 static int vkms_enable_vblank(struct drm_crtc *crtc)
 {
-   struct drm_device *dev = crtc->dev;
-   unsigned int pipe = drm_crtc_index(crtc);
-   struct drm_vblank_crtc *vblank = >vblank[pipe];
+   struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
 
drm_calc_timestamping_constants(crtc, >mode);
@@ -88,10 +86,9 @@ static bool vkms_get_vblank_timestamp(struct drm_crtc *crtc,
  bool in_vblank_irq)
 {
struct drm_device *dev = crtc->dev;
-   unsigned int pipe = crtc->index;
struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev);
struct vkms_output *output = >output;
-   struct drm_vblank_crtc *vblank = >vblank[pipe];
+   struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
 
if (!READ_ONCE(vblank->enabled)) {
*vblank_time = ktime_get();
-- 
2.43.2



[PATCH 4/5] drm/nouveau: Use drm_crtc_vblank_crtc()

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

Replace the open coded drm_crtc_vblank_crtc() with the real
thing.

Cc: Karol Herbst 
Cc: Lyude Paul 
Cc: Danilo Krummrich 
Cc: nouv...@lists.freedesktop.org
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/nouveau/nouveau_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c 
b/drivers/gpu/drm/nouveau/nouveau_display.c
index f28f9a857458..aed5d5b51b43 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -83,7 +83,7 @@ static bool
 nouveau_display_scanoutpos_head(struct drm_crtc *crtc, int *vpos, int *hpos,
ktime_t *stime, ktime_t *etime)
 {
-   struct drm_vblank_crtc *vblank = 
>dev->vblank[drm_crtc_index(crtc)];
+   struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
struct nvif_head *head = _crtc(crtc)->head;
struct nvif_head_scanoutpos_v0 args;
int retry = 20;
-- 
2.43.2



[PATCH 3/5] drm/i915: Use drm_crtc_vblank_crtc()

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

Replace the open coded drm_crtc_vblank_crtc() with the real
thing.

Cc: intel-...@lists.freedesktop.org
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/display/intel_crtc.c   |  3 +--
 drivers/gpu/drm/i915/display/intel_vblank.c | 16 +---
 2 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c 
b/drivers/gpu/drm/i915/display/intel_crtc.c
index 25593f6aae7d..339010384b86 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc.c
@@ -78,8 +78,7 @@ void intel_wait_for_vblank_if_active(struct drm_i915_private 
*i915,
 
 u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc)
 {
-   struct drm_device *dev = crtc->base.dev;
-   struct drm_vblank_crtc *vblank = 
>vblank[drm_crtc_index(>base)];
+   struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(>base);
 
if (!crtc->active)
return 0;
diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c 
b/drivers/gpu/drm/i915/display/intel_vblank.c
index baf7354cb6e2..951190bcbc50 100644
--- a/drivers/gpu/drm/i915/display/intel_vblank.c
+++ b/drivers/gpu/drm/i915/display/intel_vblank.c
@@ -132,8 +132,7 @@ u32 g4x_get_vblank_counter(struct drm_crtc *crtc)
 static u32 intel_crtc_scanlines_since_frame_timestamp(struct intel_crtc *crtc)
 {
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-   struct drm_vblank_crtc *vblank =
-   >base.dev->vblank[drm_crtc_index(>base)];
+   struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(>base);
const struct drm_display_mode *mode = >hwmode;
u32 htotal = mode->crtc_htotal;
u32 clock = mode->crtc_clock;
@@ -178,8 +177,7 @@ static u32 
intel_crtc_scanlines_since_frame_timestamp(struct intel_crtc *crtc)
  */
 static u32 __intel_get_crtc_scanline_from_timestamp(struct intel_crtc *crtc)
 {
-   struct drm_vblank_crtc *vblank =
-   >base.dev->vblank[drm_crtc_index(>base)];
+   struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(>base);
const struct drm_display_mode *mode = >hwmode;
u32 vblank_start = mode->crtc_vblank_start;
u32 vtotal = mode->crtc_vtotal;
@@ -200,17 +198,14 @@ static int __intel_get_crtc_scanline(struct intel_crtc 
*crtc)
 {
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
-   const struct drm_display_mode *mode;
-   struct drm_vblank_crtc *vblank;
+   struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(>base);
+   const struct drm_display_mode *mode = >hwmode;
enum pipe pipe = crtc->pipe;
int position, vtotal;
 
if (!crtc->active)
return 0;
 
-   vblank = >base.dev->vblank[drm_crtc_index(>base)];
-   mode = >hwmode;
-
if (crtc->mode_flags & I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP)
return __intel_get_crtc_scanline_from_timestamp(crtc);
 
@@ -254,8 +249,7 @@ static int __intel_get_crtc_scanline(struct intel_crtc 
*crtc)
 
 int intel_crtc_scanline_to_hw(struct intel_crtc *crtc, int scanline)
 {
-   const struct drm_vblank_crtc *vblank =
-   >base.dev->vblank[drm_crtc_index(>base)];
+   const struct drm_vblank_crtc *vblank = 
drm_crtc_vblank_crtc(>base);
const struct drm_display_mode *mode = >hwmode;
int vtotal;
 
-- 
2.43.2



[PATCH 2/5] drm/amdgpu: Use drm_crtc_vblank_crtc()

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

Replace the open coded drm_crtc_vblank_crtc() with the real
thing.

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: "Pan, Xinhui" 
Cc: amd-...@lists.freedesktop.org
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c  | 8 ++--
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
index 8baa2e0935cc..258703145161 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
@@ -65,9 +65,7 @@ static enum hrtimer_restart 
amdgpu_vkms_vblank_simulate(struct hrtimer *timer)
 
 static int amdgpu_vkms_enable_vblank(struct drm_crtc *crtc)
 {
-   struct drm_device *dev = crtc->dev;
-   unsigned int pipe = drm_crtc_index(crtc);
-   struct drm_vblank_crtc *vblank = >vblank[pipe];
+   struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
struct amdgpu_vkms_output *out = drm_crtc_to_amdgpu_vkms_output(crtc);
struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
 
@@ -91,10 +89,8 @@ static bool amdgpu_vkms_get_vblank_timestamp(struct drm_crtc 
*crtc,
 ktime_t *vblank_time,
 bool in_vblank_irq)
 {
-   struct drm_device *dev = crtc->dev;
-   unsigned int pipe = crtc->index;
struct amdgpu_vkms_output *output = 
drm_crtc_to_amdgpu_vkms_output(crtc);
-   struct drm_vblank_crtc *vblank = >vblank[pipe];
+   struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
 
if (!READ_ONCE(vblank->enabled)) {
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 71d2d44681b2..662d2d83473b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -528,7 +528,7 @@ static void dm_vupdate_high_irq(void *interrupt_params)
if (acrtc) {
vrr_active = amdgpu_dm_crtc_vrr_active_irq(acrtc);
drm_dev = acrtc->base.dev;
-   vblank = _dev->vblank[acrtc->base.index];
+   vblank = drm_crtc_vblank_crtc(>base);
previous_timestamp = 
atomic64_read(_params->previous_timestamp);
frame_duration_ns = vblank->time - previous_timestamp;
 
-- 
2.43.2



[PATCH 1/5] drm/vblank: Introduce drm_crtc_vblank_crtc()

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

Make life easier by providing a function that hands
out the the correct drm_vblank_crtc for a given a drm_crtc.

Also abstract the lower level internals of the vblank
code in a similar fashion.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/drm_vblank.c  | 58 ++-
 drivers/gpu/drm/drm_vblank_work.c |  2 +-
 include/drm/drm_vblank.h  |  1 +
 3 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 702a12bc93bd..cc3571e25a9a 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -166,11 +166,24 @@ module_param_named(timestamp_precision_usec, 
drm_timestamp_precision, int, 0600)
 MODULE_PARM_DESC(vblankoffdelay, "Delay until vblank irq auto-disable [msecs] 
(0: never disable, <0: disable immediately)");
 MODULE_PARM_DESC(timestamp_precision_usec, "Max. error on timestamps [usecs]");
 
+static struct drm_vblank_crtc *
+drm_vblank_crtc(struct drm_device *dev, unsigned int pipe)
+{
+   return >vblank[pipe];
+}
+
+struct drm_vblank_crtc *
+drm_crtc_vblank_crtc(struct drm_crtc *crtc)
+{
+   return drm_vblank_crtc(crtc->dev, drm_crtc_index(crtc));
+}
+EXPORT_SYMBOL(drm_crtc_vblank_crtc);
+
 static void store_vblank(struct drm_device *dev, unsigned int pipe,
 u32 vblank_count_inc,
 ktime_t t_vblank, u32 last)
 {
-   struct drm_vblank_crtc *vblank = >vblank[pipe];
+   struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, pipe);
 
assert_spin_locked(>vblank_time_lock);
 
@@ -184,7 +197,7 @@ static void store_vblank(struct drm_device *dev, unsigned 
int pipe,
 
 static u32 drm_max_vblank_count(struct drm_device *dev, unsigned int pipe)
 {
-   struct drm_vblank_crtc *vblank = >vblank[pipe];
+   struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, pipe);
 
return vblank->max_vblank_count ?: dev->max_vblank_count;
 }
@@ -273,7 +286,7 @@ static void drm_reset_vblank_timestamp(struct drm_device 
*dev, unsigned int pipe
 static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe,
bool in_vblank_irq)
 {
-   struct drm_vblank_crtc *vblank = >vblank[pipe];
+   struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, pipe);
u32 cur_vblank, diff;
bool rc;
ktime_t t_vblank;
@@ -364,7 +377,7 @@ static void drm_update_vblank_count(struct drm_device *dev, 
unsigned int pipe,
 
 u64 drm_vblank_count(struct drm_device *dev, unsigned int pipe)
 {
-   struct drm_vblank_crtc *vblank = >vblank[pipe];
+   struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, pipe);
u64 count;
 
if (drm_WARN_ON(dev, pipe >= dev->num_crtcs))
@@ -438,7 +451,7 @@ static void __disable_vblank(struct drm_device *dev, 
unsigned int pipe)
  */
 void drm_vblank_disable_and_save(struct drm_device *dev, unsigned int pipe)
 {
-   struct drm_vblank_crtc *vblank = >vblank[pipe];
+   struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, pipe);
unsigned long irqflags;
 
assert_spin_locked(>vbl_lock);
@@ -600,7 +613,7 @@ void drm_calc_timestamping_constants(struct drm_crtc *crtc,
 {
struct drm_device *dev = crtc->dev;
unsigned int pipe = drm_crtc_index(crtc);
-   struct drm_vblank_crtc *vblank = >vblank[pipe];
+   struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
int linedur_ns = 0, framedur_ns = 0;
int dotclock = mode->crtc_clock;
 
@@ -930,7 +943,7 @@ EXPORT_SYMBOL(drm_crtc_vblank_count);
 static u64 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe,
 ktime_t *vblanktime)
 {
-   struct drm_vblank_crtc *vblank = >vblank[pipe];
+   struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, pipe);
u64 vblank_count;
unsigned int seq;
 
@@ -985,7 +998,6 @@ EXPORT_SYMBOL(drm_crtc_vblank_count_and_time);
  */
 int drm_crtc_next_vblank_start(struct drm_crtc *crtc, ktime_t *vblanktime)
 {
-   unsigned int pipe = drm_crtc_index(crtc);
struct drm_vblank_crtc *vblank;
struct drm_display_mode *mode;
u64 vblank_start;
@@ -993,7 +1005,7 @@ int drm_crtc_next_vblank_start(struct drm_crtc *crtc, 
ktime_t *vblanktime)
if (!drm_dev_has_vblank(crtc->dev))
return -EINVAL;
 
-   vblank = >dev->vblank[pipe];
+   vblank = drm_crtc_vblank_crtc(crtc);
mode = >hwmode;
 
if (!vblank->framedur_ns || !vblank->linedur_ns)
@@ -1147,7 +1159,7 @@ static int __enable_vblank(struct drm_device *dev, 
unsigned int pipe)
 
 static int drm_vblank_enable(struct drm_device *dev, unsigned int pipe)
 {
-   struct drm_vblank_crtc *vblank = >vblank[pipe];
+   struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, pipe);
int ret = 0;
 
assert_spin_locked(>vbl_lock);
@@ -1185,7 +1197,7 @@ static int 

[PATCH 1/9] tools/include: Sync uapi/drm/i915_drm.h with the kernel sources

2024-04-08 Thread Namhyung Kim
To pick up changes from:

   b112364867499 ("drm/i915: Add GuC submission interface version query")
   5cf0fbf763741 ("drm/i915: Add some boring kerneldoc")

This should be used to beautify DRM syscall arguments and it addresses
these tools/perf build warnings:

  Warning: Kernel ABI header differences:
diff -u tools/include/uapi/drm/i915_drm.h include/uapi/drm/i915_drm.h

Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Thomas Zimmermann 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Namhyung Kim 
---
 tools/include/uapi/drm/i915_drm.h | 16 
 1 file changed, 16 insertions(+)

diff --git a/tools/include/uapi/drm/i915_drm.h 
b/tools/include/uapi/drm/i915_drm.h
index fd4f9574d177..2ee338860b7e 100644
--- a/tools/include/uapi/drm/i915_drm.h
+++ b/tools/include/uapi/drm/i915_drm.h
@@ -3013,6 +3013,7 @@ struct drm_i915_query_item {
 *  - %DRM_I915_QUERY_MEMORY_REGIONS (see struct 
drm_i915_query_memory_regions)
 *  - %DRM_I915_QUERY_HWCONFIG_BLOB (see `GuC HWCONFIG blob uAPI`)
 *  - %DRM_I915_QUERY_GEOMETRY_SUBSLICES (see struct 
drm_i915_query_topology_info)
+*  - %DRM_I915_QUERY_GUC_SUBMISSION_VERSION (see struct 
drm_i915_query_guc_submission_version)
 */
__u64 query_id;
 #define DRM_I915_QUERY_TOPOLOGY_INFO   1
@@ -3021,6 +3022,7 @@ struct drm_i915_query_item {
 #define DRM_I915_QUERY_MEMORY_REGIONS  4
 #define DRM_I915_QUERY_HWCONFIG_BLOB   5
 #define DRM_I915_QUERY_GEOMETRY_SUBSLICES  6
+#define DRM_I915_QUERY_GUC_SUBMISSION_VERSION  7
 /* Must be kept compact -- no holes and well documented */
 
/**
@@ -3566,6 +3568,20 @@ struct drm_i915_query_memory_regions {
struct drm_i915_memory_region_info regions[];
 };
 
+/**
+ * struct drm_i915_query_guc_submission_version - query GuC submission 
interface version
+ */
+struct drm_i915_query_guc_submission_version {
+   /** @branch: Firmware branch version. */
+   __u32 branch;
+   /** @major: Firmware major version. */
+   __u32 major;
+   /** @minor: Firmware minor version. */
+   __u32 minor;
+   /** @patch: Firmware patch version. */
+   __u32 patch;
+};
+
 /**
  * DOC: GuC HWCONFIG blob uAPI
  *
-- 
2.44.0.478.gd926399ef9-goog



RE: [PATCH v3 1/9] drm: xlnx: zynqmp_dpsub: Set layer mode during creation

2024-04-08 Thread Klymenko, Anatoliy


> -Original Message-
> From: Tomi Valkeinen 
> Sent: Friday, April 5, 2024 5:31 AM
> To: Klymenko, Anatoliy 
> Cc: dri-devel@lists.freedesktop.org; linux-arm-ker...@lists.infradead.org;
> linux-ker...@vger.kernel.org; devicet...@vger.kernel.org; linux-
> me...@vger.kernel.org; Laurent Pinchart
> ; Maarten Lankhorst
> ; Maxime Ripard
> ; Thomas Zimmermann ;
> David Airlie ; Daniel Vetter ;
> Simek, Michal ; Andrzej Hajda
> ; Neil Armstrong
> ; Robert Foss ; Jonas
> Karlman ; Jernej Skrabec
> ; Rob Herring ;
> Krzysztof Kozlowski ; Conor Dooley
> ; Mauro Carvalho Chehab
> 
> Subject: Re: [PATCH v3 1/9] drm: xlnx: zynqmp_dpsub: Set layer mode
> during creation
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> On 21/03/2024 22:43, Anatoliy Klymenko wrote:
> > Set layer mode of operation (live or dma-based) during layer creation.
> >
> > Each DPSUB layer mode of operation is defined by corresponding DT
> node port
> > connection, so it is possible to assign it during layer object creation.
> > Previously it was set in layer enable functions, although it is too late
> > as setting layer format depends on layer mode, and should be done
> before
> > given layer enabled.
> >
> > Signed-off-by: Anatoliy Klymenko 
> > Reviewed-by: Laurent Pinchart 
> > ---
> >   drivers/gpu/drm/xlnx/zynqmp_disp.c | 20 
> >   drivers/gpu/drm/xlnx/zynqmp_disp.h | 13 +
> >   drivers/gpu/drm/xlnx/zynqmp_dp.c   |  2 +-
> >   drivers/gpu/drm/xlnx/zynqmp_kms.c  |  2 +-
> >   4 files changed, 19 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > index 8a39b3accce5..e6d26ef60e89 100644
> > --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > @@ -64,6 +64,16 @@
> >
> >   #define ZYNQMP_DISP_MAX_NUM_SUB_PLANES  3
> >
> > +/**
> > + * enum zynqmp_dpsub_layer_mode - Layer mode
> > + * @ZYNQMP_DPSUB_LAYER_NONLIVE: non-live (memory) mode
> > + * @ZYNQMP_DPSUB_LAYER_LIVE: live (stream) mode
> > + */
> > +enum zynqmp_dpsub_layer_mode {
> > + ZYNQMP_DPSUB_LAYER_NONLIVE,
> > + ZYNQMP_DPSUB_LAYER_LIVE,
> > +};
> > +
> >   /**
> >* struct zynqmp_disp_format - Display subsystem format information
> >* @drm_fmt: DRM format (4CC)
> > @@ -902,15 +912,12 @@ u32
> *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
> >   /**
> >* zynqmp_disp_layer_enable - Enable a layer
> >* @layer: The layer
> > - * @mode: Operating mode of layer
> >*
> >* Enable the @layer in the audio/video buffer manager and the
> blender. DMA
> >* channels are started separately by zynqmp_disp_layer_update().
> >*/
> > -void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer,
> > -   enum zynqmp_dpsub_layer_mode mode)
> > +void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer)
> >   {
> > - layer->mode = mode;
> >   zynqmp_disp_avbuf_enable_video(layer->disp, layer);
> >   zynqmp_disp_blend_layer_enable(layer->disp, layer);
> >   }
> > @@ -1134,6 +1141,11 @@ static int zynqmp_disp_create_layers(struct
> zynqmp_disp *disp)
> >   layer->id = i;
> >   layer->disp = disp;
> >   layer->info = _info[i];
> > + /* For now assume dpsub works in either live or non-live
> mode for both layers.
> > +  * Hybrid mode is not supported yet.
> > +  */
> 
> This comment style is not according to the style guide, and in fact you
> fix it in the patch 4. So please fix it here instead.
> 

Thanks for catching it.

>   Tomi
> 
> > + layer->mode = disp->dpsub->dma_enabled ?
> ZYNQMP_DPSUB_LAYER_NONLIVE
> > +: 
> > ZYNQMP_DPSUB_LAYER_LIVE;
> >
> >   ret = zynqmp_disp_layer_request_dma(disp, layer);
> >   if (ret)
> > diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.h
> b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> > index 123cffac08be..9b8b202224d9 100644
> > --- a/drivers/gpu/drm/xlnx/zynqmp_disp.h
> > +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> > @@ -42,16 +42,6 @@ enum zynqmp_dpsub_layer_id {
> >   ZYNQMP_DPSUB_LAYER_GFX,
> >   };
> >
> > -/**
> > - * enum zynqmp_dpsub_layer_mode - Layer mode
> > - * @ZYNQMP_DPSUB_LAYER_NONLIVE: non-live (memory) mode
> > - * @ZYNQMP_DPSUB_LAYER_LIVE: live (stream) mode
> > - */
> > -enum zynqmp_dpsub_layer_mode {
> > - ZYNQMP_DPSUB_LAYER_NONLIVE,
> > - ZYNQMP_DPSUB_LAYER_LIVE,
> > -};
> > -
> >   void zynqmp_disp_enable(struct zynqmp_disp *disp);
> >   void zynqmp_disp_disable(struct zynqmp_disp *disp);
> >   int zynqmp_disp_setup_clock(struct zynqmp_disp *disp,
> > @@ -62,8 +52,7 @@ void zynqmp_disp_blend_set_global_alpha(struct
> zynqmp_disp *disp,
> >
> >   u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer
> *layer,
> >

RE: [PATCH v3 2/9] drm: xlnx: zynqmp_dpsub: Update live format defines

2024-04-08 Thread Klymenko, Anatoliy


> -Original Message-
> From: Tomi Valkeinen 
> Sent: Friday, April 5, 2024 5:10 AM
> To: Klymenko, Anatoliy 
> Cc: dri-devel@lists.freedesktop.org; linux-arm-ker...@lists.infradead.org;
> linux-ker...@vger.kernel.org; devicet...@vger.kernel.org; linux-
> me...@vger.kernel.org; Laurent Pinchart
> ; Maarten Lankhorst
> ; Maxime Ripard
> ; Thomas Zimmermann ;
> David Airlie ; Daniel Vetter ;
> Simek, Michal ; Andrzej Hajda
> ; Neil Armstrong
> ; Robert Foss ; Jonas
> Karlman ; Jernej Skrabec
> ; Rob Herring ;
> Krzysztof Kozlowski ; Conor Dooley
> ; Mauro Carvalho Chehab
> 
> Subject: Re: [PATCH v3 2/9] drm: xlnx: zynqmp_dpsub: Update live format
> defines
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> On 21/03/2024 22:43, Anatoliy Klymenko wrote:
> > Update live format defines to match DPSUB AV_BUF_LIVE_VID_CONFIG
> register
> > layout.
> 
> I think this description needs a bit more. Mention that the defines are
> not currently used,  so we can change them like this without any other
> change.
> 

Makes sense. I'll update this.

>   Tomi
> 
> > Reviewed-by: Laurent Pinchart 
> > Signed-off-by: Anatoliy Klymenko 
> > ---
> >   drivers/gpu/drm/xlnx/zynqmp_disp_regs.h | 8 
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
> b/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
> > index f92a006d5070..fa3935384834 100644
> > --- a/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
> > +++ b/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
> > @@ -165,10 +165,10 @@
> >   #define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_10   0x2
> >   #define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_12   0x3
> >   #define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_MASK
> GENMASK(2, 0)
> > -#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB   0x0
> > -#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV4440x1
> > -#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV4220x2
> > -#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YONLY 0x3
> > +#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB   (0x0
> << 4)
> > +#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV444(0x1 <<
> 4)
> > +#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV422(0x2 <<
> 4)
> > +#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YONLY (0x3 <<
> 4)
> >   #define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_MASK
> GENMASK(5, 4)
> >   #define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_CB_FIRST BIT(8)
> >   #define ZYNQMP_DISP_AV_BUF_PALETTE_MEMORY   0x400
> >



Re: [PATCH 1/4] drm/edid: add drm_edid_get_product_id()

2024-04-08 Thread Ville Syrjälä
On Thu, Mar 21, 2024 at 12:05:09PM +0200, Jani Nikula wrote:
> Add a struct drm_edid based function to get the vendor and product ID
> from an EDID. Add a separate struct for defining this part of the EDID,
> with defined byte order for product code and serial number.
> 
> Signed-off-by: Jani Nikula 
> ---
>  drivers/gpu/drm/drm_edid.c | 15 +++
>  include/drm/drm_edid.h | 25 -
>  2 files changed, 35 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index ea77577a3786..626a0e24e66a 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2756,6 +2756,21 @@ const struct drm_edid *drm_edid_read(struct 
> drm_connector *connector)
>  }
>  EXPORT_SYMBOL(drm_edid_read);
>  
> +/**
> + * drm_edid_get_product_id - Get the vendor and product identification
> + * @drm_edid: EDID
> + * @id: Where to place the product id
> + */
> +void drm_edid_get_product_id(const struct drm_edid *drm_edid,
> +  struct drm_edid_product_id *id)
> +{
> + if (drm_edid && drm_edid->edid && drm_edid->size >= EDID_LENGTH)
> + memcpy(id, _edid->edid->product_id, sizeof(*id));
> + else
> + memset(id, 0, sizeof(*id));
> +}
> +EXPORT_SYMBOL(drm_edid_get_product_id);
> +
>  /**
>   * drm_edid_get_panel_id - Get a panel's ID from EDID
>   * @drm_edid: EDID that contains panel ID.
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 6f65bbf655a1..7911a2f8a672 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -272,14 +272,27 @@ struct detailed_timing {
>  #define DRM_EDID_DSC_MAX_SLICES  0xf
>  #define DRM_EDID_DSC_TOTAL_CHUNK_KBYTES  0x3f
>  
> +struct drm_edid_product_id {
> + u8 manufacturer_name[2];

__be16?

> + __le16 product_code;
> + __le32 serial_number;
> + u8 week_of_manufacture;
> + u8 year_of_manufacture;
> +} __packed;
> +
>  struct edid {
>   u8 header[8];
>   /* Vendor & product info */
> - u8 mfg_id[2];
> - u8 prod_code[2];
> - u32 serial; /* FIXME: byte order */
> - u8 mfg_week;
> - u8 mfg_year;
> + union {
> + struct drm_edid_product_id product_id;
> + struct {
> + u8 mfg_id[2];
> + u8 prod_code[2];
> + u32 serial; /* FIXME: byte order */
> + u8 mfg_week;
> + u8 mfg_year;
> + } __packed;
> + } __packed;
>   /* EDID version */
>   u8 version;
>   u8 revision;
> @@ -466,6 +479,8 @@ int drm_edid_connector_update(struct drm_connector 
> *connector,
> const struct drm_edid *edid);
>  int drm_edid_connector_add_modes(struct drm_connector *connector);
>  bool drm_edid_is_digital(const struct drm_edid *drm_edid);
> +void drm_edid_get_product_id(const struct drm_edid *drm_edid,
> +  struct drm_edid_product_id *id);
>  
>  const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid,
> int ext_id, int *ext_index);
> -- 
> 2.39.2

-- 
Ville Syrjälä
Intel


Re: [PATCH v10 3/3] drm/tests: Add a test case for drm buddy clear allocation

2024-04-08 Thread Matthew Auld

On 08/04/2024 16:16, Arunpravin Paneer Selvam wrote:

Add a new test case for the drm buddy clear and dirty
allocation.

v2:(Matthew)
   - make size as u32
   - rename PAGE_SIZE with SZ_4K
   - dont fragment the address space for all the order allocation
 iterations. we can do it once and just increment and allocate
 the size.
   - create new mm with non power-of-two size to ensure the multi-root
 force_merge during fini.

Signed-off-by: Arunpravin Paneer Selvam 
Suggested-by: Matthew Auld 
---
  drivers/gpu/drm/tests/drm_buddy_test.c | 141 +
  1 file changed, 141 insertions(+)

diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c 
b/drivers/gpu/drm/tests/drm_buddy_test.c
index 4621a860cb05..b07f132f2835 100644
--- a/drivers/gpu/drm/tests/drm_buddy_test.c
+++ b/drivers/gpu/drm/tests/drm_buddy_test.c
@@ -224,6 +224,146 @@ static void drm_test_buddy_alloc_range_bias(struct kunit 
*test)
drm_buddy_fini();
  }
  
+static void drm_test_buddy_alloc_clear(struct kunit *test)

+{
+   unsigned long n_pages, total, i = 0;
+   const unsigned long ps = SZ_4K;
+   struct drm_buddy_block *block;
+   const int max_order = 12;
+   LIST_HEAD(allocated);
+   struct drm_buddy mm;
+   unsigned int order;
+   u32 mm_size, size;
+   LIST_HEAD(dirty);
+   LIST_HEAD(clean);
+
+   mm_size = SZ_4K << max_order;
+   KUNIT_EXPECT_FALSE(test, drm_buddy_init(, mm_size, ps));
+
+   KUNIT_EXPECT_EQ(test, mm.max_order, max_order);
+
+   /*
+* Idea is to allocate and free some random portion of the address 
space,
+* returning those pages as non-dirty and randomly alternate between
+* requesting dirty and non-dirty pages (not going over the limit
+* we freed as non-dirty), putting that into two separate lists.
+* Loop over both lists at the end checking that the dirty list
+* is indeed all dirty pages and vice versa. Free it all again,
+* keeping the dirty/clear status.
+*/
+   KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
+   5 * ps, ps, 
,
+   
DRM_BUDDY_TOPDOWN_ALLOCATION),
+   "buddy_alloc hit an error size=%lu\n", 5 * ps);
+   drm_buddy_free_list(, , DRM_BUDDY_CLEARED);
+
+   n_pages = 10;
+   do {
+   unsigned long flags;
+   struct list_head *list;
+   int slot = i % 2;
+
+   if (slot == 0) {
+   list = 
+   flags = 0;
+   } else {
+   list = 
+   flags = DRM_BUDDY_CLEAR_ALLOCATION;
+   }
+
+   KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, 
mm_size,
+   ps, ps, 
list,
+   flags),
+   "buddy_alloc hit an error size=%lu\n", 
ps);
+   } while (++i < n_pages);
+
+   list_for_each_entry(block, , link)
+   KUNIT_EXPECT_EQ(test, drm_buddy_block_is_clear(block), true);
+
+   list_for_each_entry(block, , link)
+   KUNIT_EXPECT_EQ(test, drm_buddy_block_is_clear(block), false);
+
+   drm_buddy_free_list(, , DRM_BUDDY_CLEARED);
+
+   /*
+* Trying to go over the clear limit for some allocation.
+* The allocation should never fail with reasonable page-size.
+*/
+   KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
+   10 * ps, ps, ,
+   
DRM_BUDDY_CLEAR_ALLOCATION),
+   "buddy_alloc hit an error size=%lu\n", 10 * ps);
+
+   drm_buddy_free_list(, , DRM_BUDDY_CLEARED);
+   drm_buddy_free_list(, , 0);
+   drm_buddy_fini();
+
+   KUNIT_EXPECT_FALSE(test, drm_buddy_init(, mm_size, ps));
+
+   /*
+* Create a new mm. Intentionally fragment the address space by creating
+* two alternating lists. Free both lists, one as dirty the other as 
clean.
+* Try to allocate double the previous size with matching 
min_page_size. The
+* allocation should never fail as it calls the force_merge. Also check 
that
+* the page is always dirty after force_merge. Free the page as dirty, 
then
+* repeat the whole thing, increment the order until we hit the 
max_order.
+*/
+
+   i = 0;
+   n_pages = mm_size / ps;
+   do {
+   struct list_head *list;
+   int slot = i % 2;
+
+   if (slot == 0)
+   list = 
+   else
+   list = 
+
+   KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, 
mm_size,
+

Re: [PATCH 2/4] drm/edid: add drm_edid_print_product_id()

2024-04-08 Thread Ville Syrjälä
On Thu, Mar 21, 2024 at 12:05:10PM +0200, Jani Nikula wrote:
> Add a function to print a decoded EDID vendor and product id to a drm
> printer, optinally with the raw data.
> 
> Signed-off-by: Jani Nikula 
> ---
>  drivers/gpu/drm/drm_edid.c | 35 +++
>  include/drm/drm_edid.h |  3 +++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 626a0e24e66a..198986f0eb8b 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -29,6 +29,7 @@
>   */
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -2771,6 +2772,40 @@ void drm_edid_get_product_id(const struct drm_edid 
> *drm_edid,
>  }
>  EXPORT_SYMBOL(drm_edid_get_product_id);
>  
> +/**
> + * drm_edid_print_product_id - Print decoded product id to printer
> + * @p: drm printer
> + * @id: EDID product id
> + * @raw: If true, also print the raw hex
> + */
> +void drm_edid_print_product_id(struct drm_printer *p,
> +const struct drm_edid_product_id *id, bool raw)
> +{
> + u16 mfg_id = id->manufacturer_name[0] << 8 | id->manufacturer_name[1];
> + char *date;
> + char vend[4];
> +
> + drm_edid_decode_mfg_id(mfg_id, vend);
> +
> + if (id->week_of_manufacture == 0xff)

Didn't realize this had a loaded meaning. Maybe we should also
skip the week printout if week==0? Otherwise people might think
week==0 means the first week.

> + date = kasprintf(GFP_KERNEL, "model year: %d",
> +  id->year_of_manufacture + 1990);
> + else
> + date = kasprintf(GFP_KERNEL, "week: %d, year of manufacture: 
> %d",

The "week: %d" part feels a bit left out here. Maybe this should be
formatted as "week/year of manufacture: %d/%d"? 

Not sure I like the kasprintf(). Maybe use an on-stack buffer?

> +  id->week_of_manufacture,
> +  id->year_of_manufacture + 1990);
> +
> + drm_printf(p, "manufacturer name: %s, product code: %u, serial number: 
> %u, %s\n",
> +vend, le16_to_cpu(id->product_code),
> +le32_to_cpu(id->serial_number), date ?: "");
> +
> + if (raw)
> + drm_printf(p, "raw product id: %*ph\n", (int)sizeof(*id), id);
> +
> + kfree(date);
> +}
> +EXPORT_SYMBOL(drm_edid_print_product_id);
> +
>  /**
>   * drm_edid_get_panel_id - Get a panel's ID from EDID
>   * @drm_edid: EDID that contains panel ID.
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 7911a2f8a672..c763ba1a0bbd 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -30,6 +30,7 @@ struct drm_connector;
>  struct drm_device;
>  struct drm_display_mode;
>  struct drm_edid;
> +struct drm_printer;
>  struct hdmi_avi_infoframe;
>  struct hdmi_vendor_infoframe;
>  struct i2c_adapter;
> @@ -481,6 +482,8 @@ int drm_edid_connector_add_modes(struct drm_connector 
> *connector);
>  bool drm_edid_is_digital(const struct drm_edid *drm_edid);
>  void drm_edid_get_product_id(const struct drm_edid *drm_edid,
>struct drm_edid_product_id *id);
> +void drm_edid_print_product_id(struct drm_printer *p,
> +const struct drm_edid_product_id *id, bool raw);
>  
>  const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid,
> int ext_id, int *ext_index);
> -- 
> 2.39.2

-- 
Ville Syrjälä
Intel


Re: [PATCH] drm/amdgpu: remove "num_pages" local variable in amdgpu_gtt_mgr_new

2024-04-08 Thread broler Liew
sorry, this patch has format problem. abandon. I send another email
use qq mail instead.

>
> amdgpu_gtt_mgr_new and ttm_range_man_alloc share similar logic, but
> "num_pages" in amdgpu_gtt_mgr_new is defined as local variable which
> is calculate directly in ttm_range_man_alloc.
>
> Signed-off-by: brolerliew 
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> index 44367f03316f..0c56e4057d85 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> @@ -116,7 +116,6 @@ static int amdgpu_gtt_mgr_new(struct
> ttm_resource_manager *man,
>  struct ttm_resource **res)
> {
>struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
> -   uint32_t num_pages = PFN_UP(tbo->base.size);
>struct ttm_range_mgr_node *node;
>int r;
>
> @@ -134,7 +133,7 @@ static int amdgpu_gtt_mgr_new(struct
> ttm_resource_manager *man,
>if (place->lpfn) {
>spin_lock(>lock);
>r = drm_mm_insert_node_in_range(>mm, >mm_nodes[0],
> -   num_pages, 
> tbo->page_alignment,
> +
> PFN_UP(node->base.size), tbo->page_alignment,
>0, place->fpfn, place->lpfn,
>DRM_MM_INSERT_BEST);
>spin_unlock(>lock);
> --
> 2.40.1


[PATCH] drm/ttm: Make ttm shrinkers NUMA aware

2024-04-08 Thread Rajneesh Bhardwaj
Otherwise the nid is always passed as 0 during memory reclaim so
make TTM shrinkers NUMA aware.

Signed-off-by: Rajneesh Bhardwaj 
---
 drivers/gpu/drm/ttm/ttm_pool.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
index dbc96984d331..514261f44b78 100644
--- a/drivers/gpu/drm/ttm/ttm_pool.c
+++ b/drivers/gpu/drm/ttm/ttm_pool.c
@@ -794,7 +794,7 @@ int ttm_pool_mgr_init(unsigned long num_pages)
_pool_debugfs_shrink_fops);
 #endif
 
-   mm_shrinker = shrinker_alloc(0, "drm-ttm_pool");
+   mm_shrinker = shrinker_alloc(SHRINKER_NUMA_AWARE, "drm-ttm_pool");
if (!mm_shrinker)
return -ENOMEM;
 
-- 
2.34.1



Re: [PATCH] drm/ttm: Print the memory decryption status just once

2024-04-08 Thread Zack Rusin
Sorry, apologies to everyone. By accident I replied off the list.
Redoing it now on the list. More below.

On Mon, Apr 8, 2024 at 12:10 PM Christian König
 wrote:
>
> Am 08.04.24 um 18:04 schrieb Zack Rusin:
> > On Mon, Apr 8, 2024 at 11:59 AM Christian König
> >  wrote:
> >> Am 08.04.24 um 17:56 schrieb Zack Rusin:
> >>> Stop printing the TT memory decryption status info each time tt is created
> >>> and instead print it just once.
> >>>
> >>> Reduces the spam in the system logs when running guests with SEV enabled.
> >> Do we then really need this in the first place?
> > Thomas asked for it just to have an indication when those paths are
> > being used because they could potentially break things pretty bad. I
> > think it is useful knowing that those paths are hit (but only once).
> > It makes it pretty easy for me to tell whether bug reports with people
> > who report black screen can be answered with "the kernel needs to be
> > upgraded" ;)
>
> Sounds reasonable, but my expectation was rather that we would print
> something on the device level.
>
> If that's not feasible for whatever reason than printing it once works
> as well of course.

TBH, I think it's pretty convenient to have the drm_info in the TT
just to make sure that when drivers request use_dma_alloc on SEV
systems TT turns decryption on correctly, i.e. it's a nice sanity
check when reading the logs. But if you'd prefer it in the driver I
can move this logic there as well.

z


Re: [PATCH 11/12] drm/client: Streamline mode selection debugs

2024-04-08 Thread Ville Syrjälä
On Mon, Apr 08, 2024 at 09:46:44AM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 05.04.24 um 21:58 schrieb Ville Syrjälä:
> > On Fri, Apr 05, 2024 at 09:57:07AM +0200, Thomas Zimmermann wrote:
> >> Hi
> >>
> >> Am 04.04.24 um 22:33 schrieb Ville Syrjala:
> >>> From: Ville Syrjälä 
> >>>
> >>> Get rid of all the redundant debugs and just wait until the end
> >>> to print which mode (and of which type) we picked.
> >>>
> >>> Signed-off-by: Ville Syrjälä 
> >>> ---
> >>>drivers/gpu/drm/drm_client_modeset.c | 65 +---
> >>>1 file changed, 31 insertions(+), 34 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/drm_client_modeset.c 
> >>> b/drivers/gpu/drm/drm_client_modeset.c
> >>> index 415d1799337b..ad88c11037d8 100644
> >>> --- a/drivers/gpu/drm/drm_client_modeset.c
> >>> +++ b/drivers/gpu/drm/drm_client_modeset.c
> >>> @@ -408,6 +408,8 @@ static bool drm_client_target_preferred(struct 
> >>> drm_device *dev,
> >>>
> >>>retry:
> >>>   for (i = 0; i < connector_count; i++) {
> >>> + const char *mode_type;
> >>> +
> >>>   connector = connectors[i];
> >>>
> >>>   if (conn_configured & BIT_ULL(i))
> >>> @@ -440,20 +442,20 @@ static bool drm_client_target_preferred(struct 
> >>> drm_device *dev,
> >>>   drm_client_get_tile_offsets(dev, connectors, 
> >>> connector_count, modes, offsets, i,
> >>>   
> >>> connector->tile_h_loc, connector->tile_v_loc);
> >>>   }
> >>> - drm_dbg_kms(dev, "looking for cmdline mode on 
> >>> [CONNECTOR:%d:%s]\n",
> >>> - connector->base.id, connector->name);
> >>>
> >>> - /* got for command line mode first */
> >>> + mode_type = "cmdline";
> >>>   modes[i] = drm_connector_pick_cmdline_mode(connector);
> >>> +
> >>>   if (!modes[i]) {
> >>> - drm_dbg_kms(dev, "looking for preferred mode on 
> >>> [CONNECTOR:%d:%s] (tile group: %d)\n",
> >>> - connector->base.id, connector->name,
> >>> - connector->tile_group ? 
> >>> connector->tile_group->id : 0);
> >>> + mode_type = "preferred";
> >>>   modes[i] = 
> >>> drm_connector_preferred_mode(connector, width, height);
> >>>   }
> >>> - /* No preferred modes, pick one off the list */
> >>> - if (!modes[i])
> >>> +
> >>> + if (!modes[i]) {
> >>> + mode_type = "first";
> >>>   modes[i] = drm_connector_first_mode(connector);
> >>> + }
> >>> +
> >>>   /*
> >>>* In case of tiled mode if all tiles not present 
> >>> fallback to
> >>>* first available non tiled mode.
> >>> @@ -468,16 +470,20 @@ static bool drm_client_target_preferred(struct 
> >>> drm_device *dev,
> >>>   (connector->tile_h_loc == 0 &&
> >>>connector->tile_v_loc == 0 &&
> >>>!drm_connector_get_tiled_mode(connector))) 
> >>> {
> >>> - drm_dbg_kms(dev, "Falling back to non tiled 
> >>> mode on [CONNECTOR:%d:%s]\n",
> >>> - connector->base.id, 
> >>> connector->name);
> >>> + mode_type = "non tiled";
> >>>   modes[i] = 
> >>> drm_connector_fallback_non_tiled_mode(connector);
> >>>   } else {
> >>> + mode_type = "tiled";
> >>>   modes[i] = 
> >>> drm_connector_get_tiled_mode(connector);
> >>>   }
> >>>   }
> >>>
> >>> - drm_dbg_kms(dev, "found mode %s\n",
> >>> - modes[i] ? modes[i]->name : "none");
> >>> + if (!modes[i])
> >>> + mode_type = "no";
> >>> +
> >>> + drm_dbg_kms(dev, "[CONNECTOR:%d:%s] found %s mode: %s\n",
> >>> + connector->base.id, connector->name,
> >>> + mode_type, modes[i] ? modes[i]->name : "none");
> >> Instead of tracking the whole mode_type thing, maybe just do
> >>
> >> if (!modes[i])
> >>       drm_dbg_kms(dev, "[CONNECTOR:%d:%s] found mode: " DRM_MODE_FMT,
> >> DRM_MODE_ARG(modes[i]) );
> >>
> >> to print the full mode.
> > The point of the mode_type is to indicate how we derived
> > that mode. Printing the full modeline doesn't help with that.
> 
> But do we care where the mode comes from? At least from my experience, 
> it's much more important to know which modes had been available.

The tiled vs. not-tiled at least could be quite interesting.
We know there are actual bugs in this code where some tiled 
connectors seem to incorrectly think they aren't tiled
while others correctly think they are tiled. Seeing that
spelled 

Re: [PATCH 19/21] drm/meson: Allow build with COMPILE_TEST=y

2024-04-08 Thread Martin Blumenstingl
On Mon, Apr 8, 2024 at 7:05 PM Ville Syrjala
 wrote:
>
> From: Ville Syrjälä 
>
> Allow meson to be built with COMPILE_TEST=y for greater
> coverage. Builds fine on x86/x86_64 at least.
>
> Cc: Neil Armstrong 
> Cc: linux-amlo...@lists.infradead.org
> Signed-off-by: Ville Syrjälä 
Thank you for spotting this Ville!

> ---
>  drivers/gpu/drm/meson/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/meson/Kconfig b/drivers/gpu/drm/meson/Kconfig
> index 5520b9e3f010..d8f67bd9c755 100644
> --- a/drivers/gpu/drm/meson/Kconfig
> +++ b/drivers/gpu/drm/meson/Kconfig
> @@ -1,7 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  config DRM_MESON
> tristate "DRM Support for Amlogic Meson Display Controller"
> -   depends on DRM && OF && (ARM || ARM64)
> +   depends on DRM && OF && (ARM || ARM64 || COMPILE_TEST)
Neil, I wonder if we should just have "depends on DRM && OF" here as
the next line already has:
> depends on ARCH_MESON || COMPILE_TEST
ARCH_MESON is only defined for ARM and ARM64


[PATCH] drm/amdgpu: remove "num_pages" local variable in amdgpu_gtt_mgr_new

2024-04-08 Thread broler Liew
amdgpu_gtt_mgr_new and ttm_range_man_alloc share similar logic, but
"num_pages" in amdgpu_gtt_mgr_new is defined as local variable which
is calculate directly in ttm_range_man_alloc.

Signed-off-by: brolerliew 
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 44367f03316f..0c56e4057d85 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -116,7 +116,6 @@ static int amdgpu_gtt_mgr_new(struct
ttm_resource_manager *man,
 struct ttm_resource **res)
{
   struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
-   uint32_t num_pages = PFN_UP(tbo->base.size);
   struct ttm_range_mgr_node *node;
   int r;

@@ -134,7 +133,7 @@ static int amdgpu_gtt_mgr_new(struct
ttm_resource_manager *man,
   if (place->lpfn) {
   spin_lock(>lock);
   r = drm_mm_insert_node_in_range(>mm, >mm_nodes[0],
-   num_pages, tbo->page_alignment,
+
PFN_UP(node->base.size), tbo->page_alignment,
   0, place->fpfn, place->lpfn,
   DRM_MM_INSERT_BEST);
   spin_unlock(>lock);
--
2.40.1


Re: [PATCH 15/21] drm/omap: Allow build with COMPILE_TEST=y

2024-04-08 Thread Ville Syrjälä
On Mon, Apr 08, 2024 at 08:04:20PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Allow omapdrm to be built with COMPILE_TEST=y for greater
> coverage.
> 
> FIXME: Still borked due to ?

In fact not borked anymore. Just forgot to update
the commit message.

> 
> Cc: Tomi Valkeinen 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/omapdrm/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/Kconfig b/drivers/gpu/drm/omapdrm/Kconfig
> index 6c49270cb290..85ed92042b74 100644
> --- a/drivers/gpu/drm/omapdrm/Kconfig
> +++ b/drivers/gpu/drm/omapdrm/Kconfig
> @@ -2,7 +2,7 @@
>  config DRM_OMAP
>   tristate "OMAP DRM"
>   depends on DRM && OF
> - depends on ARCH_OMAP2PLUS
> + depends on ARCH_OMAP2PLUS || COMPILE_TEST
>   select DRM_KMS_HELPER
>   select FB_DMAMEM_HELPERS_DEFERRED if DRM_FBDEV_EMULATION
>   select VIDEOMODE_HELPERS
> -- 
> 2.43.2

-- 
Ville Syrjälä
Intel


Re: [PATCH 11/21] drm/hisilicon/kirin: Allow build with COMPILE_TEST=y

2024-04-08 Thread John Stultz
On Mon, Apr 8, 2024 at 10:05 AM Ville Syrjala
 wrote:
>
> From: Ville Syrjälä 
>
> Allow kirin to be built with COMPILE_TEST=y for greater
> coverage. Builds fine on x86/x86_64 at least.
>
> Cc: Xinliang Liu 
> Cc: Tian Tao 
> Cc: Xinwei Kong 
> Cc: Sumit Semwal 
> Cc: Yongqin Liu 
> Cc: John Stultz 
> Signed-off-by: Ville Syrjälä 

Acked-by: John Stultz 


[linux-next:master] BUILD REGRESSION 11cb68ad52ac78c81e33b806b531f097e68edfa2

2024-04-08 Thread kernel test robot
tree/branch: 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
branch HEAD: 11cb68ad52ac78c81e33b806b531f097e68edfa2  Add linux-next specific 
files for 20240408

Error/Warning: (recently discovered and may have been fixed)

drivers/gpu/drm/drm_mm.c:152:1: error: unused function 
'drm_mm_interval_tree_insert' [-Werror,-Wunused-function]
drivers/gpu/drm/drm_mm.c:152:1: error: unused function 
'drm_mm_interval_tree_iter_next' [-Werror,-Wunused-function]
drivers/gpu/drm/lima/lima_drv.c:387:13: error: cast to smaller integer type 
'enum lima_gpu_id' from 'const void *' [-Werror,-Wvoid-pointer-to-enum-cast]
drivers/gpu/drm/pl111/pl111_versatile.c:488:24: error: cast to smaller integer 
type 'enum versatile_clcd' from 'const void *' 
[-Werror,-Wvoid-pointer-to-enum-cast]

Error/Warning ids grouped by kconfigs:

gcc_recent_errors
|-- alpha-allyesconfig
|   |-- 
drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and
|   `-- 
drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size
|-- arm-allmodconfig
|   |-- 
drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and
|   `-- 
drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size
|-- arm-allyesconfig
|   |-- 
drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and
|   `-- 
drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size
|-- csky-allmodconfig
|   |-- 
drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and
|   `-- 
drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size
|-- csky-allyesconfig
|   |-- 
drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and
|   `-- 
drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size
|-- loongarch-allmodconfig
|   |-- 
drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and
|   `-- 
drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size
|-- microblaze-allmodconfig
|   |-- 
drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and
|   `-- 
drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size
|-- microblaze-allyesconfig
|   |-- 
drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and
|   `-- 
drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size
|-- openrisc-allyesconfig
|   |-- 
drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and
|   `-- 
drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size
|-- parisc-allmodconfig
|   |-- 
drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and
|   |-- 
drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size
|   `-- 
drivers-gpu-drm-nouveau-nvif-object.c:error:memcpy-accessing-or-more-bytes-at-offsets-and-overlaps-bytes-at-offset
|-- parisc-allyesconfig
|   |-- 
drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and
|   |-- 
drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size
|   `-- 
drivers-gpu-drm-nouveau-nvif-object.c:error:memcpy-accessing-or-more-bytes-at-offsets-and-overlaps-bytes-at-offset
|-- powerpc-allmodconfig
|   |-- 
drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and
|   `-- 
drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size
|-- s390-allyesconfig
|   |-- 
drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and
|   `-- 
drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d

Re: [PATCH 10/21] drm/hisilicon/kirin: Fix MASK(32) on 32bit architectures

2024-04-08 Thread John Stultz
On Mon, Apr 8, 2024 at 10:05 AM Ville Syrjala
 wrote:
>
> From: Ville Syrjälä 
>
> BIT(32) is illegal when sizeof(long)==4. Use BIT_ULL(32)
> instead.
>
> Cc: Xinliang Liu 
> Cc: Tian Tao 
> Cc: Xinwei Kong 
> Cc: Sumit Semwal 
> Cc: Yongqin Liu 
> Cc: John Stultz 
> Signed-off-by: Ville Syrjälä 

Acked-by: John Stultz 


Re: [PATCH 09/21] drm/hisilicon/kirin: Fix 64bit divisions

2024-04-08 Thread John Stultz
On Mon, Apr 8, 2024 at 10:05 AM Ville Syrjala
 wrote:
>
> From: Ville Syrjälä 
>
> Use the appropriate 64bit division helpers to make the code
> build on 32bit architectures.
>
> Cc: Xinliang Liu 
> Cc: Tian Tao 
> Cc: Xinwei Kong 
> Cc: Sumit Semwal 
> Cc: Yongqin Liu 
> Cc: John Stultz 
> Signed-off-by: Ville Syrjälä 

Acked-by: John Stultz 


Re: [PATCH 08/21] drm/hisilicon/kirin: Include linux/io.h for readl()/writel()

2024-04-08 Thread John Stultz
On Mon, Apr 8, 2024 at 10:04 AM Ville Syrjala
 wrote:
>
> From: Ville Syrjälä 
>
> Include linux/io.h for readl()/writel().
>
> When built on x86_64 w/ COMPILE_TEST=y:
> ../drivers/gpu/drm/hisilicon/kirin/dw_dsi_reg.h:93:16: error: implicit 
> declaration of function ‘readl’ [-Werror=implicit-function-declaration]
>93 | orig = readl(addr);
>   |^
> ../drivers/gpu/drm/hisilicon/kirin/dw_dsi_reg.h:96:9: error: implicit 
> declaration of function ‘writel’ [-Werror=implicit-function-declaration]
>96 | writel(tmp, addr);
>   | ^~
>
> Cc: Xinliang Liu 
> Cc: Tian Tao 
> Cc: Xinwei Kong 
> Cc: Sumit Semwal 
> Cc: Yongqin Liu 
> Cc: John Stultz 
> Signed-off-by: Ville Syrjälä 

Acked-by: John Stultz 


[PATCH 17/21] drm/fsl-dcu: Allow build with COMPILE_TEST=y

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

Allow fsl-dcu to be built with COMPILE_TEST=y for greater
coverage. Builds fine on x86/x86_64 at least.

Cc: Stefan Agner 
Cc: Alison Wang 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/fsl-dcu/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/fsl-dcu/Kconfig b/drivers/gpu/drm/fsl-dcu/Kconfig
index 5ca71ef87325..9c9954a5e9bc 100644
--- a/drivers/gpu/drm/fsl-dcu/Kconfig
+++ b/drivers/gpu/drm/fsl-dcu/Kconfig
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config DRM_FSL_DCU
tristate "DRM Support for Freescale DCU"
-   depends on DRM && OF && ARM && COMMON_CLK
+   depends on DRM && OF && (ARM || COMPILE_TEST) && COMMON_CLK
select BACKLIGHT_CLASS_DEVICE
select DRM_GEM_DMA_HELPER
select DRM_KMS_HELPER
-- 
2.43.2



[PATCH 15/21] drm/omap: Allow build with COMPILE_TEST=y

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

Allow omapdrm to be built with COMPILE_TEST=y for greater
coverage.

FIXME: Still borked due to ?

Cc: Tomi Valkeinen 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/omapdrm/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/Kconfig b/drivers/gpu/drm/omapdrm/Kconfig
index 6c49270cb290..85ed92042b74 100644
--- a/drivers/gpu/drm/omapdrm/Kconfig
+++ b/drivers/gpu/drm/omapdrm/Kconfig
@@ -2,7 +2,7 @@
 config DRM_OMAP
tristate "OMAP DRM"
depends on DRM && OF
-   depends on ARCH_OMAP2PLUS
+   depends on ARCH_OMAP2PLUS || COMPILE_TEST
select DRM_KMS_HELPER
select FB_DMAMEM_HELPERS_DEFERRED if DRM_FBDEV_EMULATION
select VIDEOMODE_HELPERS
-- 
2.43.2



[PATCH 20/21] drm/rcar-du: Allow build with COMPILE_TEST=y

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

Allow rcar-du to be built with COMPILE_TEST=y for greater
coverage. Builds fine on x86/x86_64 at least.

Cc: Laurent Pinchart 
Cc: Kieran Bingham 
Cc: linux-renesas-...@vger.kernel.org
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/renesas/rcar-du/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/renesas/rcar-du/Kconfig 
b/drivers/gpu/drm/renesas/rcar-du/Kconfig
index 2dc739db2ba3..df8b08b1e537 100644
--- a/drivers/gpu/drm/renesas/rcar-du/Kconfig
+++ b/drivers/gpu/drm/renesas/rcar-du/Kconfig
@@ -2,7 +2,7 @@
 config DRM_RCAR_DU
tristate "DRM Support for R-Car Display Unit"
depends on DRM && OF
-   depends on ARM || ARM64
+   depends on ARM || ARM64 || COMPILE_TEST
depends on ARCH_RENESAS || COMPILE_TEST
select DRM_KMS_HELPER
select DRM_GEM_DMA_HELPER
-- 
2.43.2



[PATCH 21/21] drm/stm: Allow build with COMPILE_TEST=y

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

Allow stm to be built with COMPILE_TEST=y for greater
coverage. Builds fine on x86/x86_64 at least.

Cc: Yannick Fertre 
Cc: Raphael Gallais-Pou 
Cc: Philippe Cornu 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/stm/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/stm/Kconfig b/drivers/gpu/drm/stm/Kconfig
index fa49cde43bb2..4c906d602825 100644
--- a/drivers/gpu/drm/stm/Kconfig
+++ b/drivers/gpu/drm/stm/Kconfig
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config DRM_STM
tristate "DRM Support for STMicroelectronics SoC Series"
-   depends on DRM && ARCH_STM32
+   depends on DRM && (ARCH_STM32 || COMPILE_TEST)
select DRM_KMS_HELPER
select DRM_GEM_DMA_HELPER
select DRM_PANEL_BRIDGE
-- 
2.43.2



[PATCH 19/21] drm/meson: Allow build with COMPILE_TEST=y

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

Allow meson to be built with COMPILE_TEST=y for greater
coverage. Builds fine on x86/x86_64 at least.

Cc: Neil Armstrong 
Cc: linux-amlo...@lists.infradead.org
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/meson/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/meson/Kconfig b/drivers/gpu/drm/meson/Kconfig
index 5520b9e3f010..d8f67bd9c755 100644
--- a/drivers/gpu/drm/meson/Kconfig
+++ b/drivers/gpu/drm/meson/Kconfig
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config DRM_MESON
tristate "DRM Support for Amlogic Meson Display Controller"
-   depends on DRM && OF && (ARM || ARM64)
+   depends on DRM && OF && (ARM || ARM64 || COMPILE_TEST)
depends on ARCH_MESON || COMPILE_TEST
select DRM_KMS_HELPER
select DRM_GEM_DMA_HELPER
-- 
2.43.2



[PATCH 18/21] drm/mediatek: Allow build with COMPILE_TEST=y

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

Allow mediatek to be built with COMPILE_TEST=y for greater
coverage. Builds fine on x86/x86_64 at least.

Cc: Chun-Kuang Hu 
Cc: Philipp Zabel 
Cc: linux-media...@lists.infradead.org
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/mediatek/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/Kconfig b/drivers/gpu/drm/mediatek/Kconfig
index 6caab8d4d4e0..d770936e238b 100644
--- a/drivers/gpu/drm/mediatek/Kconfig
+++ b/drivers/gpu/drm/mediatek/Kconfig
@@ -2,9 +2,9 @@
 config DRM_MEDIATEK
tristate "DRM Support for Mediatek SoCs"
depends on DRM
-   depends on ARCH_MEDIATEK || (ARM && COMPILE_TEST)
+   depends on (ARCH_MEDIATEK && ARM) || COMPILE_TEST
depends on COMMON_CLK
-   depends on HAVE_ARM_SMCCC
+   depends on HAVE_ARM_SMCCC || COMPILE_TEST
depends on OF
depends on MTK_MMSYS
select DRM_KMS_HELPER
-- 
2.43.2



[PATCH 13/21] drm/tilcdc: Allow build with COMPILE_TEST=y

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

Allow tilcdc to be built with COMPILE_TEST=y for greater
coverage. Builds fine on x86/x86_64 at least.

Cc: Jyri Sarha 
Cc: Tomi Valkeinen 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/tilcdc/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tilcdc/Kconfig b/drivers/gpu/drm/tilcdc/Kconfig
index d3bd2d7a181e..1897ef91c70b 100644
--- a/drivers/gpu/drm/tilcdc/Kconfig
+++ b/drivers/gpu/drm/tilcdc/Kconfig
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config DRM_TILCDC
tristate "DRM Support for TI LCDC Display Controller"
-   depends on DRM && OF && ARM
+   depends on DRM && OF && (ARM || COMPILE_TEST)
select DRM_KMS_HELPER
select DRM_GEM_DMA_HELPER
select DRM_BRIDGE
-- 
2.43.2



[PATCH 14/21] drm/omap: Open code phys_to_page()

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

phys_to_page() is not available on most architectures.
Just open code it like msm does. Allows COMPILE_TEST=y
builds of omapdrm on other architectures.

Cc: Tomi Valkeinen 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/omapdrm/omap_gem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c 
b/drivers/gpu/drm/omapdrm/omap_gem.c
index 3421e8389222..c4454e7f1c94 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -1022,8 +1022,8 @@ struct sg_table *omap_gem_get_sg(struct drm_gem_object 
*obj,
 
if (addr) {
for_each_sg(sgt->sgl, sg, count, i) {
-   sg_set_page(sg, phys_to_page(addr), len,
-   offset_in_page(addr));
+   sg_set_page(sg, pfn_to_page(__phys_to_pfn(addr)),
+   len, offset_in_page(addr));
sg_dma_address(sg) = addr;
sg_dma_len(sg) = len;
 
-- 
2.43.2



[PATCH 16/21] drm/atmel-hlcdc: Allow build with COMPILE_TEST=y

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

Allow atmel-hlcdc to be built with COMPILE_TEST=y for greater
coverage. Builds fine on x86/x86_64 at least.

Cc: Sam Ravnborg 
Cc: Boris Brezillon 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/atmel-hlcdc/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/Kconfig 
b/drivers/gpu/drm/atmel-hlcdc/Kconfig
index 3bdbab3a6333..945f3aa7bb24 100644
--- a/drivers/gpu/drm/atmel-hlcdc/Kconfig
+++ b/drivers/gpu/drm/atmel-hlcdc/Kconfig
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config DRM_ATMEL_HLCDC
tristate "DRM Support for ATMEL HLCDC Display Controller"
-   depends on DRM && OF && COMMON_CLK && MFD_ATMEL_HLCDC && ARM
+   depends on DRM && OF && COMMON_CLK && ((MFD_ATMEL_HLCDC && ARM) || 
COMPILE_TEST)
select DRM_GEM_DMA_HELPER
select DRM_KMS_HELPER
select DRM_PANEL
-- 
2.43.2



[PATCH 10/21] drm/hisilicon/kirin: Fix MASK(32) on 32bit architectures

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

BIT(32) is illegal when sizeof(long)==4. Use BIT_ULL(32)
instead.

Cc: Xinliang Liu 
Cc: Tian Tao 
Cc: Xinwei Kong 
Cc: Sumit Semwal 
Cc: Yongqin Liu 
Cc: John Stultz 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/hisilicon/kirin/kirin_ade_reg.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_ade_reg.h 
b/drivers/gpu/drm/hisilicon/kirin/kirin_ade_reg.h
index be9e789c2d04..36f923cc7594 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_ade_reg.h
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_ade_reg.h
@@ -10,7 +10,7 @@
 /*
  * ADE Registers
  */
-#define MASK(x)(BIT(x) - 1)
+#define MASK(x)(BIT_ULL(x) - 1)
 
 #define ADE_CTRL   0x0004
 #define FRM_END_START_OFST 0
-- 
2.43.2



[PATCH 12/21] drm/tilcdc: Allow build without __iowmb()

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

__iowmb() isn't available on most architectures. Make
its use optional so that the driver can be built on
other architectures with COMPILE_TEST=y.

Cc: Jyri Sarha 
Cc: Tomi Valkeinen 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/tilcdc/tilcdc_regs.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_regs.h 
b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
index f90e2dc3457c..44e4ada30fba 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
@@ -125,7 +125,9 @@ static inline void tilcdc_write64(struct drm_device *dev, 
u32 reg, u64 data)
 #if defined(iowrite64) && !defined(iowrite64_is_nonatomic)
iowrite64(data, addr);
 #else
+#ifdef __iowmb
__iowmb();
+#endif
/* This compiles to strd (=64-bit write) on ARM7 */
*(volatile u64 __force *)addr = __cpu_to_le64(data);
 #endif
-- 
2.43.2



[PATCH 11/21] drm/hisilicon/kirin: Allow build with COMPILE_TEST=y

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

Allow kirin to be built with COMPILE_TEST=y for greater
coverage. Builds fine on x86/x86_64 at least.

Cc: Xinliang Liu 
Cc: Tian Tao 
Cc: Xinwei Kong 
Cc: Sumit Semwal 
Cc: Yongqin Liu 
Cc: John Stultz 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/hisilicon/kirin/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/hisilicon/kirin/Kconfig 
b/drivers/gpu/drm/hisilicon/kirin/Kconfig
index c5265675bf0c..0772f79567ef 100644
--- a/drivers/gpu/drm/hisilicon/kirin/Kconfig
+++ b/drivers/gpu/drm/hisilicon/kirin/Kconfig
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config DRM_HISI_KIRIN
tristate "DRM Support for Hisilicon Kirin series SoCs Platform"
-   depends on DRM && OF && ARM64
+   depends on DRM && OF && (ARM64 || COMPILE_TEST)
select DRM_KMS_HELPER
select DRM_GEM_DMA_HELPER
select DRM_MIPI_DSI
-- 
2.43.2



[PATCH 09/21] drm/hisilicon/kirin: Fix 64bit divisions

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

Use the appropriate 64bit division helpers to make the code
build on 32bit architectures.

Cc: Xinliang Liu 
Cc: Tian Tao 
Cc: Xinwei Kong 
Cc: Sumit Semwal 
Cc: Yongqin Liu 
Cc: John Stultz 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c 
b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
index 566de4658719..a39cc549c20b 100644
--- a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
+++ b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
@@ -157,8 +157,8 @@ static u32 dsi_calc_phy_rate(u32 req_kHz, struct 
mipi_phy_params *phy)
q_pll = 0x10 >> (7 - phy->hstx_ckg_sel);
 
temp = f_kHz * (u64)q_pll * (u64)ref_clk_ps;
-   m_n_int = temp / (u64)10;
-   m_n = (temp % (u64)10) / (u64)1;
+   m_n_int = div64_u64_rem(temp, 10, );
+   m_n = div_u64(temp, 1);
 
if (m_n_int % 2 == 0) {
if (m_n * 6 >= 50) {
@@ -229,9 +229,8 @@ static u32 dsi_calc_phy_rate(u32 req_kHz, struct 
mipi_phy_params *phy)
phy->pll_fbd_div5f = 1;
}
 
-   f_kHz = (u64)10 * (u64)m_pll /
-   ((u64)ref_clk_ps * (u64)n_pll * (u64)q_pll);
-
+   f_kHz = div64_u64((u64)10 * (u64)m_pll,
+ (u64)ref_clk_ps * (u64)n_pll * (u64)q_pll);
if (f_kHz >= req_kHz)
break;
 
@@ -490,7 +489,7 @@ static void dsi_set_mode_timing(void __iomem *base,
hsa_time = (hsw * lane_byte_clk_kHz) / pixel_clk_kHz;
hbp_time = (hbp * lane_byte_clk_kHz) / pixel_clk_kHz;
tmp = (u64)htot * (u64)lane_byte_clk_kHz;
-   hline_time = DIV_ROUND_UP(tmp, pixel_clk_kHz);
+   hline_time = DIV_ROUND_UP_ULL(tmp, pixel_clk_kHz);
 
/* all specified in byte-lane clocks */
writel(hsa_time, base + VID_HSA_TIME);
-- 
2.43.2



[PATCH 08/21] drm/hisilicon/kirin: Include linux/io.h for readl()/writel()

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

Include linux/io.h for readl()/writel().

When built on x86_64 w/ COMPILE_TEST=y:
../drivers/gpu/drm/hisilicon/kirin/dw_dsi_reg.h:93:16: error: implicit 
declaration of function ‘readl’ [-Werror=implicit-function-declaration]
   93 | orig = readl(addr);
  |^
../drivers/gpu/drm/hisilicon/kirin/dw_dsi_reg.h:96:9: error: implicit 
declaration of function ‘writel’ [-Werror=implicit-function-declaration]
   96 | writel(tmp, addr);
  | ^~

Cc: Xinliang Liu 
Cc: Tian Tao 
Cc: Xinwei Kong 
Cc: Sumit Semwal 
Cc: Yongqin Liu 
Cc: John Stultz 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/hisilicon/kirin/dw_dsi_reg.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/hisilicon/kirin/dw_dsi_reg.h 
b/drivers/gpu/drm/hisilicon/kirin/dw_dsi_reg.h
index d79fc031e53d..a87d1135856f 100644
--- a/drivers/gpu/drm/hisilicon/kirin/dw_dsi_reg.h
+++ b/drivers/gpu/drm/hisilicon/kirin/dw_dsi_reg.h
@@ -7,6 +7,8 @@
 #ifndef __DW_DSI_REG_H__
 #define __DW_DSI_REG_H__
 
+#include 
+
 #define MASK(x)(BIT(x) - 1)
 
 /*
-- 
2.43.2



[PATCH 07/21] drm/sti: Allow build with COMPILE_TEST=y

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

Allow sti to be built with COMPILE_TEST=y for greater
coverage. Builds fine on x86/x86_64 at least.

Cc: Alain Volmat 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/sti/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sti/Kconfig b/drivers/gpu/drm/sti/Kconfig
index 3c7a5feff8de..75c301aadcbc 100644
--- a/drivers/gpu/drm/sti/Kconfig
+++ b/drivers/gpu/drm/sti/Kconfig
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config DRM_STI
tristate "DRM Support for STMicroelectronics SoC stiH4xx Series"
-   depends on OF && DRM && ARCH_STI
+   depends on OF && DRM && (ARCH_STI || COMPILE_TEST)
select RESET_CONTROLLER
select DRM_KMS_HELPER
select DRM_GEM_DMA_HELPER
-- 
2.43.2



[PATCH 05/21] drm/imx/dcss: Allow build with COMPILE_TEST=y

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

Allow imx/dcss to be built with COMPILE_TEST=y for greater
coverage. Builds fine on x86/x86_64 at least.

Cc: Laurentiu Palcu 
Cc: Lucas Stach 
Signed-off-by: Ville Syrjälä 
dpiormw.cocci
---
 drivers/gpu/drm/imx/dcss/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/imx/dcss/Kconfig b/drivers/gpu/drm/imx/dcss/Kconfig
index 3ffc061d392b..502612747007 100644
--- a/drivers/gpu/drm/imx/dcss/Kconfig
+++ b/drivers/gpu/drm/imx/dcss/Kconfig
@@ -4,7 +4,7 @@ config DRM_IMX_DCSS
select DRM_KMS_HELPER
select DRM_GEM_DMA_HELPER
select VIDEOMODE_HELPERS
-   depends on DRM && ARCH_MXC && ARM64
+   depends on DRM && ((ARCH_MXC && ARM64) || COMPILE_TEST)
help
  Choose this if you have a NXP i.MX8MQ based system and want to use the
  Display Controller Subsystem. This option enables DCSS support.
-- 
2.43.2



[PATCH 06/21] drm/sti: Include linux/io.h for devm_ioremap()

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

Include linux/io.h for devm_ioremap().

When built on x86_64 w/ COMPILE_TEST=y:
../drivers/gpu/drm/sti/sti_dvo.c:531:21: error: implicit declaration of 
function ‘devm_ioremap’ [-Werror=implicit-function-declaration]
  531 | dvo->regs = devm_ioremap(dev, res->start,
  | ^~~~
../drivers/gpu/drm/sti/sti_dvo.c:531:19: error: assignment to ‘void *’ from 
‘int’ makes pointer from integer without a cast [-Werror=int-conversion]
  531 | dvo->regs = devm_ioremap(dev, res->start,
  |   ^

Cc: Alain Volmat 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/sti/sti_dvo.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c
index fd1df4ce3852..48a5d49fc131 100644
--- a/drivers/gpu/drm/sti/sti_dvo.c
+++ b/drivers/gpu/drm/sti/sti_dvo.c
@@ -7,6 +7,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
-- 
2.43.2



Re: [PATCH] drm: ci: fix the xfails for apq8016

2024-04-08 Thread Abhinav Kumar

Hi Helen

Gentle reminder on this.

If you are okay, we can land it via msm-next tree...

Thanks

Abhinav

On 4/1/2024 1:48 PM, Abhinav Kumar wrote:

After IGT migrating to dynamic sub-tests, the pipe prefixes
in the expected fails list are incorrect. Lets drop those
to accurately match the expected fails.

In addition, update the xfails list to match the current passing
list. This should have ideally failed in the CI run because some
tests were marked as fail even though they passed but due to the
mismatch in test names, the matching didn't correctly work and was
resulting in those failures not being seen.

Here is the passing pipeline for apq8016 with this change:

https://gitlab.freedesktop.org/drm/msm/-/jobs/57050562

Signed-off-by: Abhinav Kumar 
---
  drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt | 13 +
  1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt 
b/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt
index 44a5c62dedad..b14d4e884971 100644
--- a/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt
+++ b/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt
@@ -1,17 +1,6 @@
  kms_3d,Fail
  kms_addfb_basic@addfb25-bad-modifier,Fail
-kms_cursor_legacy@all-pipes-forked-bo,Fail
-kms_cursor_legacy@all-pipes-forked-move,Fail
-kms_cursor_legacy@all-pipes-single-bo,Fail
-kms_cursor_legacy@all-pipes-single-move,Fail
-kms_cursor_legacy@all-pipes-torture-bo,Fail
-kms_cursor_legacy@all-pipes-torture-move,Fail
-kms_cursor_legacy@pipe-A-forked-bo,Fail
-kms_cursor_legacy@pipe-A-forked-move,Fail
-kms_cursor_legacy@pipe-A-single-bo,Fail
-kms_cursor_legacy@pipe-A-single-move,Fail
-kms_cursor_legacy@pipe-A-torture-bo,Fail
-kms_cursor_legacy@pipe-A-torture-move,Fail
+kms_cursor_legacy@torture-bo,Fail
  kms_force_connector_basic@force-edid,Fail
  kms_hdmi_inject@inject-4k,Fail
  kms_selftest@drm_format,Timeout


[PATCH 04/21] drm/imx/dcss: Fix 64bit divisions

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

Use the appropriate 64bit division helpers to make the code
build on 32bit architectures.

Cc: Laurentiu Palcu 
Cc: Lucas Stach 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/imx/dcss/dcss-scaler.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/imx/dcss/dcss-scaler.c 
b/drivers/gpu/drm/imx/dcss/dcss-scaler.c
index 825728c356ff..32c3f46b21da 100644
--- a/drivers/gpu/drm/imx/dcss/dcss-scaler.c
+++ b/drivers/gpu/drm/imx/dcss/dcss-scaler.c
@@ -136,7 +136,7 @@ static int div_q(int A, int B)
else
temp -= B / 2;
 
-   result = (int)(temp / B);
+   result = div_s64(temp, B);
return result;
 }
 
@@ -239,7 +239,7 @@ static void dcss_scaler_gaussian_filter(int fc_q, bool 
use_5_taps,
ll_temp = coef[phase][i];
ll_temp <<= PSC_COEFF_PRECISION;
ll_temp += sum >> 1;
-   ll_temp /= sum;
+   ll_temp = div_s64(ll_temp, sum);
coef[phase][i] = (int)ll_temp;
}
}
-- 
2.43.2



[PATCH 03/21] drm/armada: Allow build with COMPILE_TEST=y

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

Allow armada to be built with COMPILE_TEST=y for greater
coverage. Builds fine on x86/x86_64 at least.

Cc: Russell King 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/armada/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/armada/Kconfig b/drivers/gpu/drm/armada/Kconfig
index e5597d7c9ae1..043ca103ab3f 100644
--- a/drivers/gpu/drm/armada/Kconfig
+++ b/drivers/gpu/drm/armada/Kconfig
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config DRM_ARMADA
tristate "DRM support for Marvell Armada SoCs"
-   depends on DRM && HAVE_CLK && ARM && MMU
+   depends on DRM && HAVE_CLK && MMU && (ARM || COMPILE_TEST)
select DRM_KMS_HELPER
select FB_IOMEM_HELPERS if DRM_FBDEV_EMULATION
help
-- 
2.43.2



[PATCH 02/21] drm/armada: Fix armada_debugfs_crtc_reg_write() return type

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

Change the armada_debugfs_crtc_reg_write() return type to
the correct ssize_t. This makes the code actually build on
certain architectures.

Cc: Russell King 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/armada/armada_debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/armada/armada_debugfs.c 
b/drivers/gpu/drm/armada/armada_debugfs.c
index 29f4b52e3c8d..338f0f6ca441 100644
--- a/drivers/gpu/drm/armada/armada_debugfs.c
+++ b/drivers/gpu/drm/armada/armada_debugfs.c
@@ -48,7 +48,7 @@ static int armada_debugfs_crtc_reg_open(struct inode *inode, 
struct file *file)
   inode->i_private);
 }
 
-static int armada_debugfs_crtc_reg_write(struct file *file,
+static ssize_t armada_debugfs_crtc_reg_write(struct file *file,
const char __user *ptr, size_t len, loff_t *off)
 {
struct armada_crtc *dcrtc;
-- 
2.43.2



[PATCH 01/21] drm/armada: Fix printk arguments

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

../drivers/gpu/drm/armada/armada_gem.c: In function ‘armada_gem_pwrite_ioctl’:
../drivers/gpu/drm/armada/armada_gem.c:367:27: warning: format ‘%u’ expects 
argument of type ‘unsigned int’, but argument 2 has type ‘size_t’ {aka ‘long 
unsigned int’} [-Wformat=]
  367 | DRM_ERROR("invalid size: object size %u\n", 
dobj->obj.size);
  |   ^~~~  
~~
  |  |
  |  
size_t {aka long unsigned int}

Cc: Russell King 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/armada/armada_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/armada/armada_gem.c 
b/drivers/gpu/drm/armada/armada_gem.c
index 26d10065d534..e9575ef5aaef 100644
--- a/drivers/gpu/drm/armada/armada_gem.c
+++ b/drivers/gpu/drm/armada/armada_gem.c
@@ -364,7 +364,7 @@ int armada_gem_pwrite_ioctl(struct drm_device *dev, void 
*data,
 
if (args->offset > dobj->obj.size ||
args->size > dobj->obj.size - args->offset) {
-   DRM_ERROR("invalid size: object size %u\n", dobj->obj.size);
+   DRM_ERROR("invalid size: object size %zu\n", dobj->obj.size);
ret = -EINVAL;
goto unref;
}
-- 
2.43.2



[PATCH 00/21] drm: Increase COMPILE_TEST=y coverage

2024-04-08 Thread Ville Syrjala
From: Ville Syrjälä 

I got fed up having to build multiple architectures when
doing subsystem wide refactoring. So I decided to attack
the low hanging COMPILE_TEST=y fruit. Here are the
results. All of these drivers now build with COMPILE_TEST=y
on x86/x86_64

Ville Syrjälä (21):
  drm/armada: Fix printk arguments
  drm/armada: Fix armada_debugfs_crtc_reg_write() return type
  drm/armada: Allow build with COMPILE_TEST=y
  drm/imx/dcss: Fix 64bit divisions
  drm/imx/dcss: Allow build with COMPILE_TEST=y
  drm/sti: Include linux/io.h for devm_ioremap()
  drm/sti: Allow build with COMPILE_TEST=y
  drm/hisilicon/kirin: Include linux/io.h for readl()/writel()
  drm/hisilicon/kirin: Fix 64bit divisions
  drm/hisilicon/kirin: Fix MASK(32) on 32bit architectures
  drm/hisilicon/kirin: Allow build with COMPILE_TEST=y
  drm/tilcdc: Allow build without __iowmb()
  drm/tilcdc: Allow build with COMPILE_TEST=y
  drm/omap: Open code phys_to_page()
  drm/omap: Allow build with COMPILE_TEST=y
  drm/atmel-hlcdc: Allow build with COMPILE_TEST=y
  drm/fsl-dcu: Allow build with COMPILE_TEST=y
  drm/mediatek: Allow build with COMPILE_TEST=y
  drm/meson: Allow build with COMPILE_TEST=y
  drm/rcar-du: Allow build with COMPILE_TEST=y
  drm/stm: Allow build with COMPILE_TEST=y

 drivers/gpu/drm/armada/Kconfig  |  2 +-
 drivers/gpu/drm/armada/armada_debugfs.c |  2 +-
 drivers/gpu/drm/armada/armada_gem.c |  2 +-
 drivers/gpu/drm/atmel-hlcdc/Kconfig |  2 +-
 drivers/gpu/drm/fsl-dcu/Kconfig |  2 +-
 drivers/gpu/drm/hisilicon/kirin/Kconfig |  2 +-
 drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c| 11 +--
 drivers/gpu/drm/hisilicon/kirin/dw_dsi_reg.h|  2 ++
 drivers/gpu/drm/hisilicon/kirin/kirin_ade_reg.h |  2 +-
 drivers/gpu/drm/imx/dcss/Kconfig|  2 +-
 drivers/gpu/drm/imx/dcss/dcss-scaler.c  |  4 ++--
 drivers/gpu/drm/mediatek/Kconfig|  4 ++--
 drivers/gpu/drm/meson/Kconfig   |  2 +-
 drivers/gpu/drm/omapdrm/Kconfig |  2 +-
 drivers/gpu/drm/omapdrm/omap_gem.c  |  4 ++--
 drivers/gpu/drm/renesas/rcar-du/Kconfig |  2 +-
 drivers/gpu/drm/sti/Kconfig |  2 +-
 drivers/gpu/drm/sti/sti_dvo.c   |  1 +
 drivers/gpu/drm/stm/Kconfig |  2 +-
 drivers/gpu/drm/tilcdc/Kconfig  |  2 +-
 drivers/gpu/drm/tilcdc/tilcdc_regs.h|  2 ++
 21 files changed, 30 insertions(+), 26 deletions(-)

-- 
2.43.2



Re: [PATCH] drm/msm/dpu: Add callback function pointer check before its call

2024-04-08 Thread Abhinav Kumar




On 4/8/2024 1:55 AM, Aleksandr Mishin wrote:

In dpu_core_irq_callback_handler() callback function pointer is compared to 
NULL,
but then callback function is unconditionally called by this pointer.
Fix this bug by adding conditional return.

Found by Linux Verification Center (linuxtesting.org) with SVACE.



Yes , as dmitry wrote, this should be Reported-by.

But rest LGTM.


Fixes: c929ac60b3ed ("drm/msm/dpu: allow just single IRQ callback")
Signed-off-by: Aleksandr Mishin 
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
index 946dd0135dff..03a16fbd4c99 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
@@ -223,9 +223,11 @@ static void dpu_core_irq_callback_handler(struct dpu_kms 
*dpu_kms, unsigned int
  
  	VERB("IRQ=[%d, %d]\n", DPU_IRQ_REG(irq_idx), DPU_IRQ_BIT(irq_idx));
  
-	if (!irq_entry->cb)

+   if (!irq_entry->cb) {
DRM_ERROR("no registered cb, IRQ=[%d, %d]\n",
  DPU_IRQ_REG(irq_idx), DPU_IRQ_BIT(irq_idx));
+   return;
+   }
  
  	atomic_inc(_entry->count);
  


Re: [PATCH v3 3/6] drm/msm/dsi: set VIDEO_COMPRESSION_MODE_CTRL_WC (fix video mode DSC)

2024-04-08 Thread Marijn Suijten
Can we drop (fix video mode DSC) from this patch title?  It looks like more
patches are required to get this done, such a mention is more something for the
cover letter.

We could also clarify further to "set Word Count for video-mode DSC".

- Marijn

On 2024-04-03 17:10:59, Jun Nie wrote:
> From: Jonathan Marek 
> 
> Video mode DSC won't work if this field is not set correctly. Set it to fix
> video mode DSC (for slice_per_pkt==1 cases at least).
> 
> Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration")
> Signed-off-by: Jonathan Marek 
> Reviewed-by: Dmitry Baryshkov 
> ---
>  drivers/gpu/drm/msm/dsi/dsi_host.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 2a0422cad6de..80ea4f1d8274 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -858,6 +858,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host 
> *msm_host, bool is_cmd_mod
>   u32 slice_per_intf, total_bytes_per_intf;
>   u32 pkt_per_line;
>   u32 eol_byte_num;
> + u32 bytes_per_pkt;
>  
>   /* first calculate dsc parameters and then program
>* compress mode registers
> @@ -865,6 +866,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host 
> *msm_host, bool is_cmd_mod
>   slice_per_intf = msm_dsc_get_slices_per_intf(dsc, hdisplay);
>  
>   total_bytes_per_intf = dsc->slice_chunk_size * slice_per_intf;
> + bytes_per_pkt = dsc->slice_chunk_size; /* * slice_per_pkt; */
>  
>   eol_byte_num = total_bytes_per_intf % 3;
>  
> @@ -902,6 +904,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host 
> *msm_host, bool is_cmd_mod
>   dsi_write(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL, 
> reg_ctrl);
>   dsi_write(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL2, 
> reg_ctrl2);
>   } else {
> + reg |= DSI_VIDEO_COMPRESSION_MODE_CTRL_WC(bytes_per_pkt);
>   dsi_write(msm_host, REG_DSI_VIDEO_COMPRESSION_MODE_CTRL, reg);
>   }
>  }
> 
> -- 
> 2.34.1
> 


Re: [PATCH v3 06/15] net: kunit: Suppress lock warning noise at end of dev_addr_lists tests

2024-04-08 Thread Guenter Roeck
On Wed, Apr 03, 2024 at 06:34:12PM -0700, Jakub Kicinski wrote:
> On Wed,  3 Apr 2024 06:19:27 -0700 Guenter Roeck wrote:
> > dev_addr_lists_test generates lock warning noise at the end of tests
> > if lock debugging is enabled. There are two sets of warnings.
> > 
> > WARNING: CPU: 0 PID: 689 at kernel/locking/mutex.c:923 
> > __mutex_unlock_slowpath.constprop.0+0x13c/0x368
> > DEBUG_LOCKS_WARN_ON(__owner_task(owner) != __get_current())
> > 
> > WARNING: kunit_try_catch/1336 still has locks held!
> > 
> > KUnit test cleanup is not guaranteed to run in the same thread as the test
> > itself. For this test, this means that rtnl_lock() and rtnl_unlock() may
> > be called from different threads. This triggers the warnings.
> > Suppress the warnings because they are irrelevant for the test and just
> > confusing and distracting.
> > 
> > The first warning can be suppressed by using START_SUPPRESSED_WARNING()
> > and END_SUPPRESSED_WARNING() around the call to rtnl_unlock(). To suppress
> > the second warning, it is necessary to set debug_locks_silent while the
> > rtnl lock is held.
> 
> Is it okay if I move the locking into the tests, instead?
> It's only 4 lines more and no magic required, seems to work fine.

I don't think it is that simple. Unit tests run in a kernel thread
and exit immediately if a test fails. While the unit test code _looks_
sequential, that isn't really the case. Every instance of KUNIT_ASSERT_x
or KUNIT_FAIL() results in immediate kernel thread termination. If
that happens, any rtnl_unlock() in the failed function would not be
executed. I am not aware of an equivalent of atexit() for kernel threads
which would fix the problem. My understanding is that the kunit system
doesn't support an equivalent either, but at least sometimes executes
the exit function in a different thread context.

Guenter


Re: [PATCH v3 5/6] drm/display: Add slice_per_pkt for dsc

2024-04-08 Thread Marijn Suijten
On 2024-04-08 17:58:29, Jun Nie wrote:
> Dmitry Baryshkov  于2024年4月3日周三 17:41写道:
> >
> > On Wed, 3 Apr 2024 at 12:11, Jun Nie  wrote:
> > >
> > > Add variable for slice number of a DSC compression bit stream packet.
> > > Its value shall be specified in panel driver, or default value can be set
> > > in display controller driver if panel driver does not set it.
> >
> > This is not a part of the standard. Please justify it.
> 
> Right, I read the standard but did not find any details of packet description.
> Looks like msm silicon support tuning of number of slice packing per 
> downstream
> code.
> The slice_per_pkt can be set in the downstream msm device tree. And I test the
> values 1 and 2 on vtdr6130 panel and both work. So I guess this is related to
> performance or something like that. I will have more test with different panel
> to check the impact.
> drivers/gpu/drm/panel/panel-raydium-rm692e5.c also mentions to pass new value
> to slice_per_pkt.
> 
> Hi Konrad,
> Do you remember why value 2 is TODO for slice_per_pkt for panel rm692e5?

Hi Jun,

I think I should indirectly answer that question, as I indirectly via "the"
MDSS panel generator place that comment there based on the suggested downstream
value:

https://github.com/msm8916-mainline/linux-mdss-dsi-panel-driver-generator/commit/5c82e613d987d05feca423412f6de625f9c99bae#diff-dba3766d7cec900b8de500f888c64a392cd9780f9baf00aae7e3f87a7d3fefc4R458

So I don't think Konrad's answer will be any different than "that's what
downstream does, and that's what the generator put there".

---

I was fairly certain that it used for performance reasons, but panels were found
(e.g. on the FairPhone 5) that don't seem to function without combining multiple
(2) slices in one packet at all?

- Marijn

> > > Signed-off-by: Jun Nie 
> > > ---
> > >  include/drm/display/drm_dsc.h | 4 
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/include/drm/display/drm_dsc.h b/include/drm/display/drm_dsc.h
> > > index bc90273d06a6..4fac0a2746ae 100644
> > > --- a/include/drm/display/drm_dsc.h
> > > +++ b/include/drm/display/drm_dsc.h
> > > @@ -82,6 +82,10 @@ struct drm_dsc_config {
> > >  * @bits_per_component: Bits per component to code (8/10/12)
> > >  */
> > > u8 bits_per_component;
> > > +   /**
> > > +* @slice_per_pkt: slice number per DSC bit stream packet
> > > +*/
> > > +   u8 slice_per_pkt;
> > > /**
> > >  * @convert_rgb:
> > >  * Flag to indicate if RGB - YCoCg conversion is needed
> > >
> > > --
> > > 2.34.1
> > >
> >
> >
> > --
> > With best wishes
> > Dmitry


Re: [PATCH] drm/ttm: Print the memory decryption status just once

2024-04-08 Thread Christian König

Am 08.04.24 um 17:56 schrieb Zack Rusin:

Stop printing the TT memory decryption status info each time tt is created
and instead print it just once.

Reduces the spam in the system logs when running guests with SEV enabled.


Do we then really need this in the first place?

Regards,
Christian.



Signed-off-by: Zack Rusin 
Fixes: 71ce046327cf ("drm/ttm: Make sure the mapped tt pages are decrypted when 
needed")
Cc: Thomas Hellström 
Cc: Christian König 
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
Cc:  # v5.14+
---
  drivers/gpu/drm/ttm/ttm_tt.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 578a7c37f00b..d776e3f87064 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -92,7 +92,7 @@ int ttm_tt_create(struct ttm_buffer_object *bo, bool 
zero_alloc)
 */
if (bdev->pool.use_dma_alloc && 
cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
page_flags |= TTM_TT_FLAG_DECRYPTED;
-   drm_info(ddev, "TT memory decryption enabled.");
+   drm_info_once(ddev, "TT memory decryption enabled.");
}
  
  	bo->ttm = bdev->funcs->ttm_tt_create(bo, page_flags);




[PATCH] drm/ttm: Print the memory decryption status just once

2024-04-08 Thread Zack Rusin
Stop printing the TT memory decryption status info each time tt is created
and instead print it just once.

Reduces the spam in the system logs when running guests with SEV enabled.

Signed-off-by: Zack Rusin 
Fixes: 71ce046327cf ("drm/ttm: Make sure the mapped tt pages are decrypted when 
needed")
Cc: Thomas Hellström 
Cc: Christian König 
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
Cc:  # v5.14+
---
 drivers/gpu/drm/ttm/ttm_tt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 578a7c37f00b..d776e3f87064 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -92,7 +92,7 @@ int ttm_tt_create(struct ttm_buffer_object *bo, bool 
zero_alloc)
 */
if (bdev->pool.use_dma_alloc && 
cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
page_flags |= TTM_TT_FLAG_DECRYPTED;
-   drm_info(ddev, "TT memory decryption enabled.");
+   drm_info_once(ddev, "TT memory decryption enabled.");
}
 
bo->ttm = bdev->funcs->ttm_tt_create(bo, page_flags);
-- 
2.40.1



Re: [PATCH v1 1/1] drm/msm/hdmi: Replace of_gpio.h by proper one

2024-04-08 Thread Dmitry Baryshkov
On Mon, 4 Mar 2024 at 19:51, Andy Shevchenko
 wrote:
>
> of_gpio.h is deprecated and subject to remove.
> The driver doesn't use it directly, replace it
> with what is really being used.
>
> Signed-off-by: Andy Shevchenko 
> ---
>  drivers/gpu/drm/msm/hdmi/hdmi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
> index c8ebd75176bb..24abcb7254cc 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
> @@ -5,8 +5,8 @@
>   * Author: Rob Clark 
>   */
>
> +#include 
>  #include 
> -#include 
>  #include 
>  #include 
>
> --
> 2.43.0.rc1.1.gbec44491f096
>

First one didn't reach the PW, let's try again:

Reviewed-by: Dmitry Baryshkov 


-- 
With best wishes
Dmitry


Re: [PATCH] drm: bridge: cdns-mhdp8546: Fix possible null pointer dereference

2024-04-08 Thread Robert Foss
On Mon, 8 Apr 2024 15:58:10 +0300, Aleksandr Mishin wrote:
> In cdns_mhdp_atomic_enable(), the return value of drm_mode_duplicate() is
> assigned to mhdp_state->current_mode, and there is a dereference of it in
> drm_mode_set_name(), which will lead to a NULL pointer dereference on
> failure of drm_mode_duplicate().
> 
> Fix this bug add a check of mhdp_state->current_mode.
> 
> [...]

Applied, thanks!

[1/1] drm: bridge: cdns-mhdp8546: Fix possible null pointer dereference
  https://cgit.freedesktop.org/drm/drm-misc/commit/?id=935a92a1c400



Rob



Re: [PATCH v10 1/3] drm/buddy: Implement tracking clear page feature

2024-04-08 Thread Paneer Selvam, Arunpravin

Hi Matthew,
Could you please review these changes as few clients are waiting for 
these patches.


Thanks,
Arun.
On 4/8/2024 8:46 PM, Arunpravin Paneer Selvam wrote:

- Add tracking clear page feature.

- Driver should enable the DRM_BUDDY_CLEARED flag if it
   successfully clears the blocks in the free path. On the otherhand,
   DRM buddy marks each block as cleared.

- Track the available cleared pages size

- If driver requests cleared memory we prefer cleared memory
   but fallback to uncleared if we can't find the cleared blocks.
   when driver requests uncleared memory we try to use uncleared but
   fallback to cleared memory if necessary.

- When a block gets freed we clear it and mark the freed block as cleared,
   when there are buddies which are cleared as well we can merge them.
   Otherwise, we prefer to keep the blocks as separated.

- Add a function to support defragmentation.

v1:
   - Depends on the flag check DRM_BUDDY_CLEARED, enable the block as
 cleared. Else, reset the clear flag for each block in the list(Christian)
   - For merging the 2 cleared blocks compare as below,
 drm_buddy_is_clear(block) != drm_buddy_is_clear(buddy)(Christian)
   - Defragment the memory beginning from min_order
 till the required memory space is available.

v2: (Matthew)
   - Add a wrapper drm_buddy_free_list_internal for the freeing of blocks
 operation within drm buddy.
   - Write a macro block_incompatible() to allocate the required blocks.
   - Update the xe driver for the drm_buddy_free_list change in arguments.
   - add a warning if the two blocks are incompatible on
 defragmentation
   - call full defragmentation in the fini() function
   - place a condition to test if min_order is equal to 0
   - replace the list with safe_reverse() variant as we might
 remove the block from the list.

v3:
   - fix Gitlab user reported lockup issue.
   - Keep DRM_BUDDY_HEADER_CLEAR define sorted(Matthew)
   - modify to pass the root order instead max_order in fini()
 function(Matthew)
   - change bool 1 to true(Matthew)
   - add check if min_block_size is power of 2(Matthew)
   - modify the min_block_size datatype to u64(Matthew)

v4:
   - rename the function drm_buddy_defrag with __force_merge.
   - Include __force_merge directly in drm buddy file and remove
 the defrag use in amdgpu driver.
   - Remove list_empty() check(Matthew)
   - Remove unnecessary space, headers and placement of new variables(Matthew)
   - Add a unit test case(Matthew)

v5:
   - remove force merge support to actual range allocation and not to bail
 out when contains && split(Matthew)
   - add range support to force merge function.

Signed-off-by: Arunpravin Paneer Selvam 
Signed-off-by: Matthew Auld 
Suggested-by: Christian König 
Suggested-by: Matthew Auld 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c  |   6 +-
  drivers/gpu/drm/drm_buddy.c   | 430 ++
  drivers/gpu/drm/i915/i915_ttm_buddy_manager.c |   6 +-
  drivers/gpu/drm/tests/drm_buddy_test.c|  28 +-
  drivers/gpu/drm/xe/xe_ttm_vram_mgr.c  |   4 +-
  include/drm/drm_buddy.h   |  16 +-
  6 files changed, 368 insertions(+), 122 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index 8db880244324..c0c851409241 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -571,7 +571,7 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager 
*man,
return 0;
  
  error_free_blocks:

-   drm_buddy_free_list(mm, >blocks);
+   drm_buddy_free_list(mm, >blocks, 0);
mutex_unlock(>lock);
  error_fini:
ttm_resource_fini(man, >base);
@@ -604,7 +604,7 @@ static void amdgpu_vram_mgr_del(struct ttm_resource_manager 
*man,
  
  	amdgpu_vram_mgr_do_reserve(man);
  
-	drm_buddy_free_list(mm, >blocks);

+   drm_buddy_free_list(mm, >blocks, 0);
mutex_unlock(>lock);
  
  	atomic64_sub(vis_usage, >vis_usage);

@@ -912,7 +912,7 @@ void amdgpu_vram_mgr_fini(struct amdgpu_device *adev)
kfree(rsv);
  
  	list_for_each_entry_safe(rsv, temp, >reserved_pages, blocks) {

-   drm_buddy_free_list(>mm, >allocated);
+   drm_buddy_free_list(>mm, >allocated, 0);
kfree(rsv);
}
if (!adev->gmc.is_app_apu)
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index 5ebdd6f8f36e..83dbe252f727 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -38,8 +38,8 @@ static void drm_block_free(struct drm_buddy *mm,
kmem_cache_free(slab_blocks, block);
  }
  
-static void list_insert_sorted(struct drm_buddy *mm,

-  struct drm_buddy_block *block)
+static void list_insert(struct drm_buddy *mm,
+   struct drm_buddy_block *block)
  {
struct drm_buddy_block *node;
struct list_head 

[PATCH v10 3/3] drm/tests: Add a test case for drm buddy clear allocation

2024-04-08 Thread Arunpravin Paneer Selvam
Add a new test case for the drm buddy clear and dirty
allocation.

v2:(Matthew)
  - make size as u32
  - rename PAGE_SIZE with SZ_4K
  - dont fragment the address space for all the order allocation
iterations. we can do it once and just increment and allocate
the size.
  - create new mm with non power-of-two size to ensure the multi-root
force_merge during fini.

Signed-off-by: Arunpravin Paneer Selvam 
Suggested-by: Matthew Auld 
---
 drivers/gpu/drm/tests/drm_buddy_test.c | 141 +
 1 file changed, 141 insertions(+)

diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c 
b/drivers/gpu/drm/tests/drm_buddy_test.c
index 4621a860cb05..b07f132f2835 100644
--- a/drivers/gpu/drm/tests/drm_buddy_test.c
+++ b/drivers/gpu/drm/tests/drm_buddy_test.c
@@ -224,6 +224,146 @@ static void drm_test_buddy_alloc_range_bias(struct kunit 
*test)
drm_buddy_fini();
 }
 
+static void drm_test_buddy_alloc_clear(struct kunit *test)
+{
+   unsigned long n_pages, total, i = 0;
+   const unsigned long ps = SZ_4K;
+   struct drm_buddy_block *block;
+   const int max_order = 12;
+   LIST_HEAD(allocated);
+   struct drm_buddy mm;
+   unsigned int order;
+   u32 mm_size, size;
+   LIST_HEAD(dirty);
+   LIST_HEAD(clean);
+
+   mm_size = SZ_4K << max_order;
+   KUNIT_EXPECT_FALSE(test, drm_buddy_init(, mm_size, ps));
+
+   KUNIT_EXPECT_EQ(test, mm.max_order, max_order);
+
+   /*
+* Idea is to allocate and free some random portion of the address 
space,
+* returning those pages as non-dirty and randomly alternate between
+* requesting dirty and non-dirty pages (not going over the limit
+* we freed as non-dirty), putting that into two separate lists.
+* Loop over both lists at the end checking that the dirty list
+* is indeed all dirty pages and vice versa. Free it all again,
+* keeping the dirty/clear status.
+*/
+   KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
+   5 * ps, ps, 
,
+   
DRM_BUDDY_TOPDOWN_ALLOCATION),
+   "buddy_alloc hit an error size=%lu\n", 5 * ps);
+   drm_buddy_free_list(, , DRM_BUDDY_CLEARED);
+
+   n_pages = 10;
+   do {
+   unsigned long flags;
+   struct list_head *list;
+   int slot = i % 2;
+
+   if (slot == 0) {
+   list = 
+   flags = 0;
+   } else {
+   list = 
+   flags = DRM_BUDDY_CLEAR_ALLOCATION;
+   }
+
+   KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, 
mm_size,
+   ps, ps, 
list,
+   flags),
+   "buddy_alloc hit an error size=%lu\n", 
ps);
+   } while (++i < n_pages);
+
+   list_for_each_entry(block, , link)
+   KUNIT_EXPECT_EQ(test, drm_buddy_block_is_clear(block), true);
+
+   list_for_each_entry(block, , link)
+   KUNIT_EXPECT_EQ(test, drm_buddy_block_is_clear(block), false);
+
+   drm_buddy_free_list(, , DRM_BUDDY_CLEARED);
+
+   /*
+* Trying to go over the clear limit for some allocation.
+* The allocation should never fail with reasonable page-size.
+*/
+   KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
+   10 * ps, ps, ,
+   
DRM_BUDDY_CLEAR_ALLOCATION),
+   "buddy_alloc hit an error size=%lu\n", 10 * ps);
+
+   drm_buddy_free_list(, , DRM_BUDDY_CLEARED);
+   drm_buddy_free_list(, , 0);
+   drm_buddy_fini();
+
+   KUNIT_EXPECT_FALSE(test, drm_buddy_init(, mm_size, ps));
+
+   /*
+* Create a new mm. Intentionally fragment the address space by creating
+* two alternating lists. Free both lists, one as dirty the other as 
clean.
+* Try to allocate double the previous size with matching 
min_page_size. The
+* allocation should never fail as it calls the force_merge. Also check 
that
+* the page is always dirty after force_merge. Free the page as dirty, 
then
+* repeat the whole thing, increment the order until we hit the 
max_order.
+*/
+
+   i = 0;
+   n_pages = mm_size / ps;
+   do {
+   struct list_head *list;
+   int slot = i % 2;
+
+   if (slot == 0)
+   list = 
+   else
+   list = 
+
+   KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, 
mm_size,
+   ps, 

[PATCH v10 1/3] drm/buddy: Implement tracking clear page feature

2024-04-08 Thread Arunpravin Paneer Selvam
- Add tracking clear page feature.

- Driver should enable the DRM_BUDDY_CLEARED flag if it
  successfully clears the blocks in the free path. On the otherhand,
  DRM buddy marks each block as cleared.

- Track the available cleared pages size

- If driver requests cleared memory we prefer cleared memory
  but fallback to uncleared if we can't find the cleared blocks.
  when driver requests uncleared memory we try to use uncleared but
  fallback to cleared memory if necessary.

- When a block gets freed we clear it and mark the freed block as cleared,
  when there are buddies which are cleared as well we can merge them.
  Otherwise, we prefer to keep the blocks as separated.

- Add a function to support defragmentation.

v1:
  - Depends on the flag check DRM_BUDDY_CLEARED, enable the block as
cleared. Else, reset the clear flag for each block in the list(Christian)
  - For merging the 2 cleared blocks compare as below,
drm_buddy_is_clear(block) != drm_buddy_is_clear(buddy)(Christian)
  - Defragment the memory beginning from min_order
till the required memory space is available.

v2: (Matthew)
  - Add a wrapper drm_buddy_free_list_internal for the freeing of blocks
operation within drm buddy.
  - Write a macro block_incompatible() to allocate the required blocks.
  - Update the xe driver for the drm_buddy_free_list change in arguments.
  - add a warning if the two blocks are incompatible on
defragmentation
  - call full defragmentation in the fini() function
  - place a condition to test if min_order is equal to 0
  - replace the list with safe_reverse() variant as we might
remove the block from the list.

v3:
  - fix Gitlab user reported lockup issue.
  - Keep DRM_BUDDY_HEADER_CLEAR define sorted(Matthew)
  - modify to pass the root order instead max_order in fini()
function(Matthew)
  - change bool 1 to true(Matthew)
  - add check if min_block_size is power of 2(Matthew)
  - modify the min_block_size datatype to u64(Matthew)

v4:
  - rename the function drm_buddy_defrag with __force_merge.
  - Include __force_merge directly in drm buddy file and remove
the defrag use in amdgpu driver.
  - Remove list_empty() check(Matthew)
  - Remove unnecessary space, headers and placement of new variables(Matthew)
  - Add a unit test case(Matthew)

v5:
  - remove force merge support to actual range allocation and not to bail
out when contains && split(Matthew)
  - add range support to force merge function.

Signed-off-by: Arunpravin Paneer Selvam 
Signed-off-by: Matthew Auld 
Suggested-by: Christian König 
Suggested-by: Matthew Auld 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c  |   6 +-
 drivers/gpu/drm/drm_buddy.c   | 430 ++
 drivers/gpu/drm/i915/i915_ttm_buddy_manager.c |   6 +-
 drivers/gpu/drm/tests/drm_buddy_test.c|  28 +-
 drivers/gpu/drm/xe/xe_ttm_vram_mgr.c  |   4 +-
 include/drm/drm_buddy.h   |  16 +-
 6 files changed, 368 insertions(+), 122 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index 8db880244324..c0c851409241 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -571,7 +571,7 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager 
*man,
return 0;
 
 error_free_blocks:
-   drm_buddy_free_list(mm, >blocks);
+   drm_buddy_free_list(mm, >blocks, 0);
mutex_unlock(>lock);
 error_fini:
ttm_resource_fini(man, >base);
@@ -604,7 +604,7 @@ static void amdgpu_vram_mgr_del(struct ttm_resource_manager 
*man,
 
amdgpu_vram_mgr_do_reserve(man);
 
-   drm_buddy_free_list(mm, >blocks);
+   drm_buddy_free_list(mm, >blocks, 0);
mutex_unlock(>lock);
 
atomic64_sub(vis_usage, >vis_usage);
@@ -912,7 +912,7 @@ void amdgpu_vram_mgr_fini(struct amdgpu_device *adev)
kfree(rsv);
 
list_for_each_entry_safe(rsv, temp, >reserved_pages, blocks) {
-   drm_buddy_free_list(>mm, >allocated);
+   drm_buddy_free_list(>mm, >allocated, 0);
kfree(rsv);
}
if (!adev->gmc.is_app_apu)
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index 5ebdd6f8f36e..83dbe252f727 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -38,8 +38,8 @@ static void drm_block_free(struct drm_buddy *mm,
kmem_cache_free(slab_blocks, block);
 }
 
-static void list_insert_sorted(struct drm_buddy *mm,
-  struct drm_buddy_block *block)
+static void list_insert(struct drm_buddy *mm,
+   struct drm_buddy_block *block)
 {
struct drm_buddy_block *node;
struct list_head *head;
@@ -57,6 +57,16 @@ static void list_insert_sorted(struct drm_buddy *mm,
__list_add(>link, node->link.prev, >link);
 }
 
+static void clear_reset(struct drm_buddy_block *block)
+{
+   

[PATCH v10 2/3] drm/amdgpu: Enable clear page functionality

2024-04-08 Thread Arunpravin Paneer Selvam
Add clear page support in vram memory region.

v1(Christian):
  - Dont handle clear page as TTM flag since when moving the BO back
in from GTT again we don't need that.
  - Make a specialized version of amdgpu_fill_buffer() which only
clears the VRAM areas which are not already cleared
  - Drop the TTM_PL_FLAG_WIPE_ON_RELEASE check in
amdgpu_object.c

v2:
  - Modify the function name amdgpu_ttm_* (Alex)
  - Drop the delayed parameter (Christian)
  - handle amdgpu_res_cleared() just above the size
calculation (Christian)
  - Use AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE for clearing the buffers
in the free path to properly wait for fences etc.. (Christian)

v3(Christian):
  - Remove buffer clear code in VRAM manager instead change the
AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE handling to set
the DRM_BUDDY_CLEARED flag.
  - Remove ! from amdgpu_res_cleared() check.

v4(Christian):
  - vres flag setting move to vram manager file
  - use dma_fence_get_stub in amdgpu_ttm_clear_buffer function
  - make fence a mandatory parameter and drop the if and the get/put dance

Signed-off-by: Arunpravin Paneer Selvam 
Suggested-by: Christian König 
Acked-by: Felix Kuehling 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c|  9 +--
 .../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h| 25 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c   | 70 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h   |  5 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c  |  6 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h  | 10 +++
 6 files changed, 116 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 8bc79924d171..e011a326fe86 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -39,6 +39,7 @@
 #include "amdgpu.h"
 #include "amdgpu_trace.h"
 #include "amdgpu_amdkfd.h"
+#include "amdgpu_vram_mgr.h"
 
 /**
  * DOC: amdgpu_object
@@ -601,8 +602,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (!amdgpu_bo_support_uswc(bo->flags))
bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
 
-   if (adev->ras_enabled)
-   bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
+   bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
 
bo->tbo.bdev = >mman.bdev;
if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA |
@@ -634,7 +634,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
bo->tbo.resource->mem_type == TTM_PL_VRAM) {
struct dma_fence *fence;
 
-   r = amdgpu_fill_buffer(bo, 0, bo->tbo.base.resv, , true);
+   r = amdgpu_ttm_clear_buffer(bo, bo->tbo.base.resv, );
if (unlikely(r))
goto fail_unreserve;
 
@@ -1365,8 +1365,9 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object 
*bo)
if (WARN_ON_ONCE(!dma_resv_trylock(bo->base.resv)))
return;
 
-   r = amdgpu_fill_buffer(abo, AMDGPU_POISON, bo->base.resv, , true);
+   r = amdgpu_fill_buffer(abo, 0, bo->base.resv, , true);
if (!WARN_ON(r)) {
+   amdgpu_vram_mgr_set_cleared(bo->resource);
amdgpu_bo_fence(abo, fence, false);
dma_fence_put(fence);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
index 381101d2bf05..50fcd86e1033 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
@@ -164,4 +164,29 @@ static inline void amdgpu_res_next(struct 
amdgpu_res_cursor *cur, uint64_t size)
}
 }
 
+/**
+ * amdgpu_res_cleared - check if blocks are cleared
+ *
+ * @cur: the cursor to extract the block
+ *
+ * Check if the @cur block is cleared
+ */
+static inline bool amdgpu_res_cleared(struct amdgpu_res_cursor *cur)
+{
+   struct drm_buddy_block *block;
+
+   switch (cur->mem_type) {
+   case TTM_PL_VRAM:
+   block = cur->node;
+
+   if (!amdgpu_vram_mgr_is_cleared(block))
+   return false;
+   break;
+   default:
+   return false;
+   }
+
+   return true;
+}
+
 #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index fc418e670fda..f0b42b567357 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -378,11 +378,12 @@ static int amdgpu_move_blit(struct ttm_buffer_object *bo,
(abo->flags & AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE)) {
struct dma_fence *wipe_fence = NULL;
 
-   r = amdgpu_fill_buffer(abo, AMDGPU_POISON, NULL, _fence,
-   false);
+   r = amdgpu_fill_buffer(abo, 0, NULL, _fence,
+  false);
if (r) {

Re: [PATCH] gpu: host1x: Do not setup DMA for virtual devices

2024-04-08 Thread Thierry Reding
On Wed Apr 3, 2024 at 12:07 PM CEST, Jon Hunter wrote:
> Hi Thierry,
>
> On 15/03/2024 11:25, Jon Hunter wrote:
> > 
> > On 14/03/2024 15:49, Thierry Reding wrote:
> >> From: Thierry Reding 
> >>
> >> The host1x devices are virtual compound devices and do not perform DMA
> >> accesses themselves, so they do not need to be set up for DMA.
> >>
> >> Ideally we would also not need to set up DMA masks for the virtual
> >> devices, but we currently still need those for legacy support on old
> >> hardware.
> >>
> >> Signed-off-by: Thierry Reding 
> >> ---
> >>   drivers/gpu/host1x/bus.c | 8 
> >>   1 file changed, 8 deletions(-)
> >>
> >> diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c
> >> index 783975d1384f..7c52757a89db 100644
> >> --- a/drivers/gpu/host1x/bus.c
> >> +++ b/drivers/gpu/host1x/bus.c
> >> @@ -351,11 +351,6 @@ static int host1x_device_uevent(const struct 
> >> device *dev,
> >>   return 0;
> >>   }
> >> -static int host1x_dma_configure(struct device *dev)
> >> -{
> >> -    return of_dma_configure(dev, dev->of_node, true);
> >> -}
> >> -
> >>   static const struct dev_pm_ops host1x_device_pm_ops = {
> >>   .suspend = pm_generic_suspend,
> >>   .resume = pm_generic_resume,
> >> @@ -369,7 +364,6 @@ const struct bus_type host1x_bus_type = {
> >>   .name = "host1x",
> >>   .match = host1x_device_match,
> >>   .uevent = host1x_device_uevent,
> >> -    .dma_configure = host1x_dma_configure,
> >>   .pm = _device_pm_ops,
> >>   };
> >> @@ -458,8 +452,6 @@ static int host1x_device_add(struct host1x *host1x,
> >>   device->dev.bus = _bus_type;
> >>   device->dev.parent = host1x->dev;
> >> -    of_dma_configure(>dev, host1x->dev->of_node, true);
> >> -
> >>   device->dev.dma_parms = >dma_parms;
> >>   dma_set_max_seg_size(>dev, UINT_MAX);
> > 
> > 
> > Tested-by: Jon Hunter 
> > Acked-by: Jon Hunter 
>
>
> I don't see this in -next yet?
>
> Ideally, if we don't see any issues with this we should pull this into 
> v6.8.y stable branch because I am now seeing the warning there. Should 
> we apply a fixes tag to this?

I was finally able to run some finally tests on this and pushed it to
drm-misc-fixes, so it should go into linux-next and then Linus' tree
sometime soon.

I decided against adding a Fixes tag because it's difficult to backport
this all the way to the release which contains the commit that added
the issue. Adding a Fixes tag to the commit that ended up exposing the
issue didn't seem right either, so let's get this into mainline first
and then manually ask stable maintainers to pick this up.

Thierry


signature.asc
Description: PGP signature


Re: [RFC PATCH v3 0/6] drm/amd/display: switch amdgpu_dm_connector to use struct drm_edid

2024-04-08 Thread Melissa Wen
On 03/28, Jani Nikula wrote:
> On Wed, 27 Mar 2024, Melissa Wen  wrote:
> > 2. Most of the edid data handled by `dm_helpers_parse_edid_caps()` are
> >in drm_edid halpers, but there are still a few that are not managed by
> >them yet. For example:
> >```
> > edid_caps->serial_number = edid_buf->serial;
> > edid_caps->manufacture_week = edid_buf->mfg_week;
> > edid_caps->manufacture_year = edid_buf->mfg_year;
> >```
> >AFAIU I see the same pending issue in i915/pnpid_get_panel_type() -
> >that still uses drm_edid_raw().
> 
> See https://lore.kernel.org/r/cover.1711015462.git.jani.nik...@intel.com

Thanks!
I'll work on top of your series.

Melissa

> 
> BR,
> Jani.
> 
> 
> -- 
> Jani Nikula, Intel


Re: [RFC PATCH v3 3/6] drm/amd/display: switch amdgpu_dm_connector to use struct drm_edid

2024-04-08 Thread Melissa Wen
On 03/28, Jani Nikula wrote:
> On Wed, 27 Mar 2024, Melissa Wen  wrote:
> > Replace raw edid handling (struct edid) with the opaque EDID type
> > (struct drm_edid) on amdgpu_dm_connector for consistency. It may also
> > prevent mismatch of approaches in different parts of the driver code.
> > Working in progress. It was only exercised with IGT tests.
> >
> > v2: use const to fix warnings (Alex Hung)
> > v3: fix general protection fault on mst
> >
> > Signed-off-by: Melissa Wen 
> > ---
> >  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 99 +--
> >  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  4 +-
> >  .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c |  8 +-
> >  .../display/amdgpu_dm/amdgpu_dm_mst_types.c   | 24 ++---
> >  4 files changed, 67 insertions(+), 68 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index 280562707cd0..bbbf9c9d40d5 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -3269,18 +3269,19 @@ void amdgpu_dm_update_connector_after_detect(
> > aconnector->dc_sink = sink;
> > dc_sink_retain(aconnector->dc_sink);
> > if (sink->dc_edid.length == 0) {
> > -   aconnector->edid = NULL;
> > +   drm_edid_free(aconnector->edid);
> 
> If aconnector->edid is used as some kind of cache, it might be prudent
> to still set it to NULL.

Makes sense. Thanks for pointing it out.
> 
> It's up to the amd maintainers, but personally I've renamed any ->edid
> fields to ->drm_edid when I've done these conversions to ensure every
> single place is covered, and also later stable backports will give a
> build failure if the change is not taken into account.

I see. Good points. I'll follow your advice and do this name change in
the next version.
> 
> > if (aconnector->dc_link->aux_mode) {
> > drm_dp_cec_unset_edid(
> > >dm_dp_aux.aux);
> > }
> > } else {
> > -   aconnector->edid =
> > -   (struct edid *)sink->dc_edid.raw_edid;
> > +   const struct edid *edid = (const struct edid 
> > *)sink->dc_edid.raw_edid;
> > +   aconnector->edid = drm_edid_alloc(edid, 
> > sink->dc_edid.length);
> >  
> > +   /* FIXME: Get rid of drm_edid_raw() */
> > if (aconnector->dc_link->aux_mode)
> > drm_dp_cec_set_edid(>dm_dp_aux.aux,
> > -   aconnector->edid);
> > +   
> > drm_edid_raw(aconnector->edid));
> 
> The goal should be to switch to use CEC functions that take the source
> physical address directly instead of parsing the EDID.

Yes, I understand I have the same goal as in the next patch:
- https://lore.kernel.org/dri-devel/20240327214953.367126-5-m...@igalia.com/

Am I missing something?

> 
> > }
> >  
> > if (!aconnector->timing_requested) {
> > @@ -3291,17 +3292,17 @@ void amdgpu_dm_update_connector_after_detect(
> > "failed to create 
> > aconnector->requested_timing\n");
> > }
> >  
> > -   drm_connector_update_edid_property(connector, aconnector->edid);
> > +   drm_edid_connector_update(connector, aconnector->edid);
> > amdgpu_dm_update_freesync_caps(connector, aconnector->edid);
> > update_connector_ext_caps(aconnector);
> > } else {
> > drm_dp_cec_unset_edid(>dm_dp_aux.aux);
> > amdgpu_dm_update_freesync_caps(connector, NULL);
> > -   drm_connector_update_edid_property(connector, NULL);
> > +   drm_edid_connector_update(connector, NULL);
> > aconnector->num_modes = 0;
> > dc_sink_release(aconnector->dc_sink);
> > aconnector->dc_sink = NULL;
> > -   aconnector->edid = NULL;
> > +   drm_edid_free(aconnector->edid);
> > kfree(aconnector->timing_requested);
> > aconnector->timing_requested = NULL;
> > /* Set CP to DESIRED if it was ENABLED, so we can re-enable it 
> > again on hotplug */
> > @@ -6661,13 +6662,7 @@ static void amdgpu_dm_connector_funcs_force(struct 
> > drm_connector *connector)
> > struct amdgpu_dm_connector *aconnector = 
> > to_amdgpu_dm_connector(connector);
> > struct dc_link *dc_link = aconnector->dc_link;
> > struct dc_sink *dc_em_sink = aconnector->dc_em_sink;
> > -   struct edid *edid;
> > -   struct i2c_adapter *ddc;
> > -
> > -   if (dc_link && dc_link->aux_mode)
> > -   ddc = >dm_dp_aux.aux.ddc;
> > -   else
> > -   ddc = >i2c->base;
> > +   const struct drm_edid *drm_edid;
> >  
> > /*
> >  * Note: drm_get_edid gets edid in 

Re: [PATCH 0/4] drm/edid & drm/i915: vendor and product id logging improvements

2024-04-08 Thread Jani Nikula
On Mon, 08 Apr 2024, Melissa Wen  wrote:
> On 04/08, Jani Nikula wrote:
>> On Mon, 08 Apr 2024, Melissa Wen  wrote:
>> > On 04/02, Jani Nikula wrote:
>> >> On Thu, 21 Mar 2024, Jani Nikula  wrote:
>> >> > Jani Nikula (4):
>> >> >   drm/edid: add drm_edid_get_product_id()
>> >> >   drm/edid: add drm_edid_print_product_id()
>> >> >   drm/i915/bios: switch to struct drm_edid and struct
>> >> > drm_edid_product_id
>> >> >   drm/i915/bios: return drm_edid_product_id from get_lvds_pnp_id()
>> >> 
>> >> Ping for reviews please? This should be helpful in eradicating one class
>> >> of drm_edid_raw() uses.
>> >
>> > Hi Jani,
>> >
>> > I took a look at the series. AFAIU your solution with
>> > `drm_edid_product_id` mostly fits AMD display driver needs, except that
>> > it needs the `product_code` split into two parts (like manufacturer
>> > name) because the driver handles prod_code parts to configure a register
>> > for audio, as in the path below:
>> >
>> > 1. 
>> > https://gitlab.freedesktop.org/drm/kernel/-/blob/drm-next/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c#L113
>> > 2. 
>> > https://gitlab.freedesktop.org/drm/kernel/-/blob/drm-next/drivers/gpu/drm/amd/display/dc/core/dc_stream.c#L90
>> > 3. 
>> > https://gitlab.freedesktop.org/drm/kernel/-/blob/drm-next/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c#L873
>> >
>> > What do you think on keeping `prod_code` split into two part in
>> > `drm_edid_product_id`?
>> 
>> I think having it as "__le16 product_code" is better and
>> self-documenting. This is what the spec says it is, so why split it to
>> two parts where you always need to wonder about the byte order?
>
> I have no strong opinion, I was only thinking that two parts would make
> this `edid_buf->prod_code[0] | edid_buf->prod_code[1] << 8` operation
> more intuitive.
>
> As you mentioned the currrent product_code format fits specs better, I
> understand we can get the same result on amd with:
> le16_to_cpu() --> split --> second part shift.

Wait, what? No. le16_to_cpu() directly gives you what you want. That's
the whole point. No splits, no shifts, no OR-ing. One macro that forces
the right byte order for you, as the member is defined __le16.

> Anyway, it's certainly not a blocker. The series LGTM.
>
> Acked-by: Melissa Wen 
>
>> 
>> This is how you'd use it:
>> 
>>  edid_caps->product_id = le16_to_cpu(id->product_code);

This is intended to be an example how to deal with your URL 1 above.

BR,
Jani.

>> 
>> BR,
>> Jani.
>> 
>> >
>> > (cc'ing some AMD devs that might have a better understanding of this use 
>> > case)
>> >
>> > Thanks a lot for addressing this pending issue!
>> >
>> > Melissa
>> >
>> >> 
>> >> BR,
>> >> Jani.
>> >> 
>> >> 
>> >> >
>> >> >  drivers/gpu/drm/drm_edid.c| 50 +++
>> >> >  drivers/gpu/drm/i915/display/intel_bios.c | 49 ++
>> >> >  include/drm/drm_edid.h| 28 ++---
>> >> >  3 files changed, 94 insertions(+), 33 deletions(-)
>> >> 
>> >> -- 
>> >> Jani Nikula, Intel
>> 
>> -- 
>> Jani Nikula, Intel

-- 
Jani Nikula, Intel


Re: [PATCH 0/4] drm/edid & drm/i915: vendor and product id logging improvements

2024-04-08 Thread Melissa Wen
On 04/08, Jani Nikula wrote:
> On Mon, 08 Apr 2024, Melissa Wen  wrote:
> > On 04/02, Jani Nikula wrote:
> >> On Thu, 21 Mar 2024, Jani Nikula  wrote:
> >> > Jani Nikula (4):
> >> >   drm/edid: add drm_edid_get_product_id()
> >> >   drm/edid: add drm_edid_print_product_id()
> >> >   drm/i915/bios: switch to struct drm_edid and struct
> >> > drm_edid_product_id
> >> >   drm/i915/bios: return drm_edid_product_id from get_lvds_pnp_id()
> >> 
> >> Ping for reviews please? This should be helpful in eradicating one class
> >> of drm_edid_raw() uses.
> >
> > Hi Jani,
> >
> > I took a look at the series. AFAIU your solution with
> > `drm_edid_product_id` mostly fits AMD display driver needs, except that
> > it needs the `product_code` split into two parts (like manufacturer
> > name) because the driver handles prod_code parts to configure a register
> > for audio, as in the path below:
> >
> > 1. 
> > https://gitlab.freedesktop.org/drm/kernel/-/blob/drm-next/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c#L113
> > 2. 
> > https://gitlab.freedesktop.org/drm/kernel/-/blob/drm-next/drivers/gpu/drm/amd/display/dc/core/dc_stream.c#L90
> > 3. 
> > https://gitlab.freedesktop.org/drm/kernel/-/blob/drm-next/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c#L873
> >
> > What do you think on keeping `prod_code` split into two part in
> > `drm_edid_product_id`?
> 
> I think having it as "__le16 product_code" is better and
> self-documenting. This is what the spec says it is, so why split it to
> two parts where you always need to wonder about the byte order?

I have no strong opinion, I was only thinking that two parts would make
this `edid_buf->prod_code[0] | edid_buf->prod_code[1] << 8` operation
more intuitive.

As you mentioned the currrent product_code format fits specs better, I
understand we can get the same result on amd with:
le16_to_cpu() --> split --> second part shift.

Anyway, it's certainly not a blocker. The series LGTM.

Acked-by: Melissa Wen 

> 
> This is how you'd use it:
> 
>   edid_caps->product_id = le16_to_cpu(id->product_code);
> 
> BR,
> Jani.
> 
> >
> > (cc'ing some AMD devs that might have a better understanding of this use 
> > case)
> >
> > Thanks a lot for addressing this pending issue!
> >
> > Melissa
> >
> >> 
> >> BR,
> >> Jani.
> >> 
> >> 
> >> >
> >> >  drivers/gpu/drm/drm_edid.c| 50 +++
> >> >  drivers/gpu/drm/i915/display/intel_bios.c | 49 ++
> >> >  include/drm/drm_edid.h| 28 ++---
> >> >  3 files changed, 94 insertions(+), 33 deletions(-)
> >> 
> >> -- 
> >> Jani Nikula, Intel
> 
> -- 
> Jani Nikula, Intel


Re: [PATCH] drm: nv04: Add check to avoid out of bounds access

2024-04-08 Thread Danilo Krummrich

On 4/5/24 22:05, Lyude Paul wrote:

On Fri, 2024-04-05 at 17:53 +0200, Danilo Krummrich wrote:

On 3/31/24 08:45, Mikhail Kobuk wrote:

Output Resource (dcb->or) value is not guaranteed to be non-zero
(i.e.
in drivers/gpu/drm/nouveau/nouveau_bios.c, in
'fabricate_dcb_encoder_table()'
'dcb->or' is assigned value '0' in call to
'fabricate_dcb_output()').


I don't really know much about the semantics of this code.

Looking at fabricate_dcb_output() though I wonder if the intention
was to assign
BIT(or) to entry->or.

@Lyude, can you help here?


This code is definitely a bit before my time as well - but I think
you're completely correct. Especially considering this bit I found in
nouveau_bios.h:


Thanks for confirming.

@Mikhail, I think we should rather fix this assignment then.

- Danilo



enum nouveau_or {
DCB_OUTPUT_A = (1 << 0),
DCB_OUTPUT_B = (1 << 1),
DCB_OUTPUT_C = (1 << 2)
};




Otherwise, for parsing the DCB entries, it seems that the bound
checks are
happening in olddcb_outp_foreach() [1].

[1]
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/nouveau/nouveau_bios.c#L1331



Add check to validate 'dcb->or' before it's used.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 2e5702aff395 ("drm/nouveau: fabricate DCB encoder table for
iMac G4")
Signed-off-by: Mikhail Kobuk 
---
   drivers/gpu/drm/nouveau/dispnv04/dac.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv04/dac.c
b/drivers/gpu/drm/nouveau/dispnv04/dac.c
index d6b8e0cce2ac..0c8d4fc95ff3 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/dac.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/dac.c
@@ -428,7 +428,7 @@ void nv04_dac_update_dacclk(struct drm_encoder
*encoder, bool enable)
    struct drm_device *dev = encoder->dev;
    struct dcb_output *dcb = nouveau_encoder(encoder)->dcb;
   
-	if (nv_gf4_disp_arch(dev)) {

+   if (nv_gf4_disp_arch(dev) && ffs(dcb->or)) {
    uint32_t *dac_users = _display(dev)-

dac_users[ffs(dcb->or) - 1];

    int dacclk_off = NV_PRAMDAC_DACCLK +
nv04_dac_output_offset(encoder);
    uint32_t dacclk = NVReadRAMDAC(dev, 0,
dacclk_off);
@@ -453,7 +453,7 @@ bool nv04_dac_in_use(struct drm_encoder
*encoder)
    struct drm_device *dev = encoder->dev;
    struct dcb_output *dcb = nouveau_encoder(encoder)->dcb;
   
-	return nv_gf4_disp_arch(encoder->dev) &&

+   return nv_gf4_disp_arch(encoder->dev) && ffs(dcb->or) &&
    (nv04_display(dev)->dac_users[ffs(dcb->or) - 1] &
~(1 << dcb->index));
   }
   






[PATCH] drm/msm/dpu: Add callback function pointer check before its call

2024-04-08 Thread Aleksandr Mishin
In dpu_core_irq_callback_handler() callback function pointer is compared to 
NULL,
but then callback function is unconditionally called by this pointer.
Fix this bug by adding conditional return.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: c929ac60b3ed ("drm/msm/dpu: allow just single IRQ callback")
Signed-off-by: Aleksandr Mishin 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
index 946dd0135dff..03a16fbd4c99 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
@@ -223,9 +223,11 @@ static void dpu_core_irq_callback_handler(struct dpu_kms 
*dpu_kms, unsigned int
 
VERB("IRQ=[%d, %d]\n", DPU_IRQ_REG(irq_idx), DPU_IRQ_BIT(irq_idx));
 
-   if (!irq_entry->cb)
+   if (!irq_entry->cb) {
DRM_ERROR("no registered cb, IRQ=[%d, %d]\n",
  DPU_IRQ_REG(irq_idx), DPU_IRQ_BIT(irq_idx));
+   return;
+   }
 
atomic_inc(_entry->count);
 
-- 
2.30.2



[PATCH] drm: bridge: cdns-mhdp8546: Fix possible null pointer dereference

2024-04-08 Thread Aleksandr Mishin
In cdns_mhdp_atomic_enable(), the return value of drm_mode_duplicate() is
assigned to mhdp_state->current_mode, and there is a dereference of it in
drm_mode_set_name(), which will lead to a NULL pointer dereference on
failure of drm_mode_duplicate().

Fix this bug add a check of mhdp_state->current_mode.

Fixes: fb43aa0acdfd ("drm: bridge: Add support for Cadence MHDP8546 DPI/DP 
bridge")
Signed-off-by: Aleksandr Mishin 
---
 drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c 
b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index e226acc5c15e..8a91ef0ae065 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -2059,6 +2059,9 @@ static void cdns_mhdp_atomic_enable(struct drm_bridge 
*bridge,
mhdp_state = to_cdns_mhdp_bridge_state(new_state);
 
mhdp_state->current_mode = drm_mode_duplicate(bridge->dev, mode);
+   if (!mhdp_state->current_mode)
+   return;
+
drm_mode_set_name(mhdp_state->current_mode);
 
dev_dbg(mhdp->dev, "%s: Enabling mode %s\n", __func__, mode->name);
-- 
2.30.2



Re: Khadas VIM3/3L New TS050 support patch

2024-04-08 Thread neil . armstrong

Hi Jacobe,

On 08/04/2024 15:16, Jacobe Zang wrote:

Subject: [PATCH] drm/panel: add New TS050 Panel support

Hello all,

I have made some changes to the panel-khadas-ts050.c. Here is a brief overview 
of what has been done:

- Add dt-bindings in document.
- Add New TS050 Panel timing sequence.
- Make it compatible with the old TS050 panel.
- The only difference between them is the timing, so I change the node in 
overlay to identify the specific panel.
- Changes in DTS doesn't push, because preceding commits have not been merged.

The proposed changes have been successfully run on Khadas VIM3/3L.

Please find the patch attached/inline.


You should read 
https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html
and perhaps use a tool like b4 (https://b4.docs.kernel.org/en/latest/) to send 
your patches in text only,
you can follow 
https://www.marcusfolkesson.se/blog/use-b4-for-kernel-contributions/ for 
example.

I'm afraid this patchset will be rejected and not reviewed if sent in this 
format with patches as attachments.

Thanks,
Neil



Jacobe Zang (2):
 drm/panel: add New TS050 panel support
 dt-bindings: panel-simple-dsi: add New Khadas TS050 panel bindings

drivers/gpu/drm/panel/panel-khadas-ts050.c | 1119 
-
Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml | 2 ++

---
Best Regards
Jacobe 臧介皓
Amazing Khadas, Always Amazes You!




Re: [PATCH 0/4] drm/edid & drm/i915: vendor and product id logging improvements

2024-04-08 Thread Jani Nikula
On Mon, 08 Apr 2024, Melissa Wen  wrote:
> On 04/02, Jani Nikula wrote:
>> On Thu, 21 Mar 2024, Jani Nikula  wrote:
>> > Jani Nikula (4):
>> >   drm/edid: add drm_edid_get_product_id()
>> >   drm/edid: add drm_edid_print_product_id()
>> >   drm/i915/bios: switch to struct drm_edid and struct
>> > drm_edid_product_id
>> >   drm/i915/bios: return drm_edid_product_id from get_lvds_pnp_id()
>> 
>> Ping for reviews please? This should be helpful in eradicating one class
>> of drm_edid_raw() uses.
>
> Hi Jani,
>
> I took a look at the series. AFAIU your solution with
> `drm_edid_product_id` mostly fits AMD display driver needs, except that
> it needs the `product_code` split into two parts (like manufacturer
> name) because the driver handles prod_code parts to configure a register
> for audio, as in the path below:
>
> 1. 
> https://gitlab.freedesktop.org/drm/kernel/-/blob/drm-next/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c#L113
> 2. 
> https://gitlab.freedesktop.org/drm/kernel/-/blob/drm-next/drivers/gpu/drm/amd/display/dc/core/dc_stream.c#L90
> 3. 
> https://gitlab.freedesktop.org/drm/kernel/-/blob/drm-next/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c#L873
>
> What do you think on keeping `prod_code` split into two part in
> `drm_edid_product_id`?

I think having it as "__le16 product_code" is better and
self-documenting. This is what the spec says it is, so why split it to
two parts where you always need to wonder about the byte order?

This is how you'd use it:

edid_caps->product_id = le16_to_cpu(id->product_code);

BR,
Jani.

>
> (cc'ing some AMD devs that might have a better understanding of this use case)
>
> Thanks a lot for addressing this pending issue!
>
> Melissa
>
>> 
>> BR,
>> Jani.
>> 
>> 
>> >
>> >  drivers/gpu/drm/drm_edid.c| 50 +++
>> >  drivers/gpu/drm/i915/display/intel_bios.c | 49 ++
>> >  include/drm/drm_edid.h| 28 ++---
>> >  3 files changed, 94 insertions(+), 33 deletions(-)
>> 
>> -- 
>> Jani Nikula, Intel

-- 
Jani Nikula, Intel


Re: [PATCH] nouveau: fix devinit paths to only handle display on GSP.

2024-04-08 Thread Timur Tabi
On Mon, 2024-04-08 at 16:42 +1000, Dave Airlie wrote:
> This reverts:
> nouveau/gsp: don't check devinit disable on GSP.
> and applies a further fix.
> 
> It turns out the open gpu driver, checks this register, but only for
> display.
> 
> Match that behaviour and only disable displays on turings.

Why only on Turings?


Re: [RFC PATCH 0/8] TTM shrinker helpers and xe buffer object shrinker

2024-04-08 Thread Thomas Hellström
Hi, Amaranath,

On Tue, 2024-04-02 at 17:25 +0530, Somalapuram, Amaranath wrote:
> 
> On 3/29/2024 8:26 PM, Thomas Hellström wrote:
> > This series implements TTM shrinker / eviction helpers and an xe bo
> > shrinker. It builds on two previous series. First
> > 
> > https://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg484425.html
> > 
> > for patch 1-4, which IMO still could be reviewed and pushed as a
> > separate series.
> > 
> > Second the previous TTM shrinker series 
> On the latest drm-misc good amount of conflicts on both patch series,
> able to re-base first one, Second on has 16 patches.
> If you have latest re-base patches, Please share it.

I was a bit unclear here. This series includes a rebased variant of the
first series, and the rest of this series is a rework of the remaining
patches in the second series below.

So this series is self-contained and the links are provided only for
reference and context. This series still seems to apply using git am -3
on drm-tip, and with a minor single conflict on drm-misc-next.
(Typically we send the patches against drm-tip since that's what CI is
using).

The below series has not been rebased but I have a version of this
series with a direct-swap-cache insertion backend lying around
somewhere. I can post it or share it on gitlab if you want.

Thanks,
/Thomas



> 
> Regards,
> S.Amarnath
> > https://lore.kernel.org/linux-mm/b7491378-defd-4f1c-31e2-29e4c77e2...@amd.com/T/
> > 
> > Where the comment about layering
> > https://lore.kernel.org/linux-mm/b7491378-defd-4f1c-31e2-29e4c77e2...@amd.com/T/#ma918844aa8a6efe8768fdcda0c6590d5c93850c9
> > 
> > now addressed, and this version also implements shmem objects for
> > backup
> > rather than direct swap-cache insertions, which was used in the
> > previuos
> > series. It turns out that with per-page backup / shrinking, shmem
> > objects
> > appears to work just as well as direct swap-cache insertions with
> > the
> > added benefit that was introduced in the previous TTM shrinker
> > series to
> > avoid running out of swap entries isn't really needed.
> > 
> > In any case, patch 1-4 are better described in their separate
> > series.
> > (RFC is removed for those).
> > 
> > Patch 5 could in theory be skipped but introduces a possibility to
> > easily
> > add or test multiple backup backends, like the direct swap-cache
> > insertion or even files into fast dedicated nvme storage for for
> > example.
> > 
> > Patch 6 introduces helpers in the ttm_pool code for page-by-page
> > shrinking
> > and recovery. It avoids having to temporarily allocate a huge
> > amount of
> > memory to be able to shrink a buffer object. It also introduces the
> > possibility to immediately write-back pages if needed, since that
> > tends
> > to be a bit delayed when left to kswapd.
> > 
> > Patch 7 introduces a LRU walk helper for eviction and shrinking.
> > It's
> > currently xe-only but not xe-specific and can easily be moved to
> > TTM when
> > used by more than one driver or when eviction is implemented using
> > it.
> > 
> > Patch 8 introduces a helper callback for shrinking (Also ready to
> > be
> > moved to TTM) and an xe-specific shrinker implementation.
> > 
> > Testing:
> > ATM I don't think we have good tests to cover the shrinking
> > functionality
> > The series has been tested with a hack that continously creates
> > TTM_TT buffer objects until system memory and swap space is
> > exhausted,
> > and then reads them back and frees them. However, these tests
> > seem to be very slow.
> > Ideally a similar test on a machine with very fast solid state or
> > similar storage should be set up. Ideally also verifying content
> > preservation.
> > 
> > Cc: Somalapuram Amaranath 
> > Cc: Christian König 
> > Cc: 
> > 
> > Thomas Hellström (8):
> >    drm/ttm: Allow TTM LRU list nodes of different types
> >    drm/ttm: Use LRU hitches
> >    drm/ttm, drm/amdgpu, drm/xe: Consider hitch moves within bulk
> > sublist
> >  moves
> >    drm/ttm: Allow continued swapout after -ENOSPC falure
> >    drm/ttm: Add a virtual base class for graphics memory backup
> >    drm/ttm/pool: Provide a helper to shrink pages.
> >    drm/xe, drm/ttm: Provide a generic LRU walker helper
> >    drm/xe: Add a shrinker for xe bos
> > 
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c |   4 +
> >   drivers/gpu/drm/ttm/Makefile   |   2 +-
> >   drivers/gpu/drm/ttm/ttm_backup_shmem.c | 137 +
> >   drivers/gpu/drm/ttm/ttm_bo.c   |   1 +
> >   drivers/gpu/drm/ttm/ttm_device.c   |  33 ++-
> >   drivers/gpu/drm/ttm/ttm_pool.c | 391
> > -
> >   drivers/gpu/drm/ttm/ttm_resource.c | 231 ---
> >   drivers/gpu/drm/ttm/ttm_tt.c   |  34 +++
> >   drivers/gpu/drm/xe/Makefile    |   2 +
> >   drivers/gpu/drm/xe/xe_bo.c | 123 ++--
> >   drivers/gpu/drm/xe/xe_bo.h |   3 +
> >   drivers/gpu/drm/xe/xe_device.c |   8 +
> >   

[PATCH 5.15 016/690] drm/vmwgfx: Fix possible null pointer derefence with invalid contexts

2024-04-08 Thread Greg Kroah-Hartman
5.15-stable review patch.  If anyone has any objections, please let me know.

--

From: Zack Rusin 

[ Upstream commit 517621b7060096e48e42f545fa6646fc00252eac ]

vmw_context_cotable can return either an error or a null pointer and its
usage sometimes went unchecked. Subsequent code would then try to access
either a null pointer or an error value.

The invalid dereferences were only possible with malformed userspace
apps which never properly initialized the rendering contexts.

Check the results of vmw_context_cotable to fix the invalid derefs.

Thanks:
ziming zhang(@ezrak1e) from Ant Group Light-Year Security Lab
who was the first person to discover it.
Niels De Graef who reported it and helped to track down the poc.

Fixes: 9c079b8ce8bf ("drm/vmwgfx: Adapt execbuf to the new validation api")
Cc:  # v4.20+
Reported-by: Niels De Graef  
Signed-off-by: Zack Rusin 
Cc: Martin Krastev 
Cc: Maaz Mombasawala 
Cc: Ian Forbes 
Cc: Broadcom internal kernel review list 
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Maaz Mombasawala 
Reviewed-by: Martin Krastev 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20240110200305.94086-1-zack.ru...@broadcom.com
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
index b91f8d17404d6..21134c7f18382 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
@@ -472,7 +472,7 @@ static int vmw_resource_context_res_add(struct vmw_private 
*dev_priv,
vmw_res_type(ctx) == vmw_res_dx_context) {
for (i = 0; i < cotable_max; ++i) {
res = vmw_context_cotable(ctx, i);
-   if (IS_ERR(res))
+   if (IS_ERR_OR_NULL(res))
continue;
 
ret = vmw_execbuf_res_noctx_val_add(sw_context, res,
@@ -1277,6 +1277,8 @@ static int vmw_cmd_dx_define_query(struct vmw_private 
*dev_priv,
return -EINVAL;
 
cotable_res = vmw_context_cotable(ctx_node->ctx, SVGA_COTABLE_DXQUERY);
+   if (IS_ERR_OR_NULL(cotable_res))
+   return cotable_res ? PTR_ERR(cotable_res) : -EINVAL;
ret = vmw_cotable_notify(cotable_res, cmd->body.queryId);
 
return ret;
@@ -2455,6 +2457,8 @@ static int vmw_cmd_dx_view_define(struct vmw_private 
*dev_priv,
return ret;
 
res = vmw_context_cotable(ctx_node->ctx, vmw_view_cotables[view_type]);
+   if (IS_ERR_OR_NULL(res))
+   return res ? PTR_ERR(res) : -EINVAL;
ret = vmw_cotable_notify(res, cmd->defined_id);
if (unlikely(ret != 0))
return ret;
@@ -2540,8 +2544,8 @@ static int vmw_cmd_dx_so_define(struct vmw_private 
*dev_priv,
 
so_type = vmw_so_cmd_to_type(header->id);
res = vmw_context_cotable(ctx_node->ctx, vmw_so_cotables[so_type]);
-   if (IS_ERR(res))
-   return PTR_ERR(res);
+   if (IS_ERR_OR_NULL(res))
+   return res ? PTR_ERR(res) : -EINVAL;
cmd = container_of(header, typeof(*cmd), header);
ret = vmw_cotable_notify(res, cmd->defined_id);
 
@@ -2660,6 +2664,8 @@ static int vmw_cmd_dx_define_shader(struct vmw_private 
*dev_priv,
return -EINVAL;
 
res = vmw_context_cotable(ctx_node->ctx, SVGA_COTABLE_DXSHADER);
+   if (IS_ERR_OR_NULL(res))
+   return res ? PTR_ERR(res) : -EINVAL;
ret = vmw_cotable_notify(res, cmd->body.shaderId);
if (ret)
return ret;
@@ -2981,6 +2987,8 @@ static int vmw_cmd_dx_define_streamoutput(struct 
vmw_private *dev_priv,
}
 
res = vmw_context_cotable(ctx_node->ctx, SVGA_COTABLE_STREAMOUTPUT);
+   if (IS_ERR_OR_NULL(res))
+   return res ? PTR_ERR(res) : -EINVAL;
ret = vmw_cotable_notify(res, cmd->body.soid);
if (ret)
return ret;
-- 
2.43.0





Re: [PATCH 2/8] drm/ttm: Use LRU hitches

2024-04-08 Thread Thomas Hellström
On Fri, 2024-04-05 at 14:41 +0200, Christian König wrote:
> Am 29.03.24 um 15:57 schrieb Thomas Hellström:
> > Have iterators insert themselves into the list they are iterating
> > over using hitch list nodes. Since only the iterator owner
> > can remove these list nodes from the list, it's safe to unlock
> > the list and when continuing, use them as a starting point. Due to
> > the way LRU bumping works in TTM, newly added items will not be
> > missed, and bumped items will be iterated over a second time before
> > reaching the end of the list.
> > 
> > The exception is list with bulk move sublists. When bumping a
> > sublist, a hitch that is part of that sublist will also be moved
> > and we might miss items if restarting from it. This will be
> > addressed in a later patch.
> > 
> > v2:
> > - Updated ttm_resource_cursor_fini() documentation.
> > 
> > Cc: Christian König 
> > Cc: Somalapuram Amaranath 
> > Cc: 
> > Signed-off-by: Thomas Hellström 
> > ---
> >   drivers/gpu/drm/ttm/ttm_bo.c   |  1 +
> >   drivers/gpu/drm/ttm/ttm_device.c   |  9 ++-
> >   drivers/gpu/drm/ttm/ttm_resource.c | 94 -
> > -
> >   include/drm/ttm/ttm_resource.h | 16 +++--
> >   4 files changed, 82 insertions(+), 38 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
> > b/drivers/gpu/drm/ttm/ttm_bo.c
> > index e059b1e1b13b..b6f75a0ff2e5 100644
> > --- a/drivers/gpu/drm/ttm/ttm_bo.c
> > +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> > @@ -622,6 +622,7 @@ int ttm_mem_evict_first(struct ttm_device
> > *bdev,
> >     if (locked)
> >     dma_resv_unlock(res->bo->base.resv);
> >     }
> > +   ttm_resource_cursor_fini_locked();
> >   
> >     if (!bo) {
> >     if (busy_bo && !ttm_bo_get_unless_zero(busy_bo))
> > diff --git a/drivers/gpu/drm/ttm/ttm_device.c
> > b/drivers/gpu/drm/ttm/ttm_device.c
> > index f27406e851e5..e8a6a1dab669 100644
> > --- a/drivers/gpu/drm/ttm/ttm_device.c
> > +++ b/drivers/gpu/drm/ttm/ttm_device.c
> > @@ -169,12 +169,17 @@ int ttm_device_swapout(struct ttm_device
> > *bdev, struct ttm_operation_ctx *ctx,
> >     num_pages = PFN_UP(bo->base.size);
> >     ret = ttm_bo_swapout(bo, ctx, gfp_flags);
> >     /* ttm_bo_swapout has dropped the lru_lock
> > */
> > -   if (!ret)
> > +   if (!ret) {
> > +   ttm_resource_cursor_fini();
> >     return num_pages;
> > -   if (ret != -EBUSY)
> > +   }
> > +   if (ret != -EBUSY) {
> > +   ttm_resource_cursor_fini();
> >     return ret;
> > +   }
> >     }
> >     }
> > +   ttm_resource_cursor_fini_locked();
> >     spin_unlock(>lru_lock);
> >     return 0;
> >   }
> > diff --git a/drivers/gpu/drm/ttm/ttm_resource.c
> > b/drivers/gpu/drm/ttm/ttm_resource.c
> > index 7aa5ca5c0e33..ccc31ad85e3b 100644
> > --- a/drivers/gpu/drm/ttm/ttm_resource.c
> > +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> > @@ -32,6 +32,37 @@
> >   
> >   #include 
> >   
> > +/**
> > + * ttm_resource_cursor_fini_locked() - Finalize the LRU list
> > cursor usage
> > + * @cursor: The struct ttm_resource_cursor to finalize.
> > + *
> > + * The function pulls the LRU list cursor off any lists it was
> > previusly
> > + * attached to. Needs to be called with the LRU lock held. The
> > function
> > + * can be called multiple times after eachother.
> > + */
> > +void ttm_resource_cursor_fini_locked(struct ttm_resource_cursor
> > *cursor)
> > +{
> > +   lockdep_assert_held(>man->bdev->lru_lock);
> > +   list_del_init(>hitch.link);
> > +}
> > +
> > +/**
> > + * ttm_resource_cursor_fini() - Finalize the LRU list cursor usage
> > + * @cursor: The struct ttm_resource_cursor to finalize.
> > + *
> > + * The function pulls the LRU list cursor off any lists it was
> > previusly
> > + * attached to. Needs to be called without the LRU list lock held.
> > The
> > + * function can be called multiple times after eachother.
> > + */
> > +void ttm_resource_cursor_fini(struct ttm_resource_cursor *cursor)
> > +{
> > +   spinlock_t *lru_lock = >man->bdev->lru_lock;
> > +
> > +   spin_lock(lru_lock);
> > +   ttm_resource_cursor_fini_locked(cursor);
> > +   spin_unlock(lru_lock);
> > +}
> > +
> >   /**
> >    * ttm_lru_bulk_move_init - initialize a bulk move structure
> >    * @bulk: the structure to init
> > @@ -484,62 +515,63 @@ void ttm_resource_manager_debug(struct
> > ttm_resource_manager *man,
> >   EXPORT_SYMBOL(ttm_resource_manager_debug);
> >   
> >   /**
> > - * ttm_resource_manager_first
> > - *
> > - * @man: resource manager to iterate over
> > + * ttm_resource_manager_next() - Continue iterating over the
> > resource manager
> > + * resources
> >    * @cursor: cursor to record the position
> >    *
> > - * Returns the first resource from the resource manager.
> > + * Return: The next resource from the 

  1   2   >