Re: [PATCH v6,02/24] v4l2: handle restricted memory flags in queue setup
Hi Andrzej,
Thanks for your help to review this patch.
Your suggestion is very reasonable, I will change it later.
Best Regards,
Yunfei Dong
On Wed, 2024-05-22 at 14:20 +0200, Andrzej Pietrasiewicz wrote:
> Hi Yunfei & Jeffrey,
>
> W dniu 16.05.2024 o 14:20, Yunfei Dong pisze:
> > From: Jeffrey Kardatzke
> >
> > Validates the restricted memory flags when setting up a queue and
> > ensures the queue has the proper capability.
> >
> > Signed-off-by: Jeffrey Kardatzke
> > Signed-off-by: Yunfei Dong
> > ---
> > .../media/common/videobuf2/videobuf2-core.c | 21
> > +++
> > .../media/common/videobuf2/videobuf2-v4l2.c | 4 +++-
> > 2 files changed, 24 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
> > b/drivers/media/common/videobuf2/videobuf2-core.c
> > index 358f1fe42975..fe4c0594ab81 100644
> > --- a/drivers/media/common/videobuf2/videobuf2-core.c
> > +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> > @@ -831,6 +831,15 @@ static bool verify_coherency_flags(struct
> > vb2_queue *q, bool non_coherent_mem)
> > return true;
> > }
> >
> > +static bool verify_restricted_mem_flags(struct vb2_queue *q, bool
> > restricted_mem)
> > +{
> > + if (restricted_mem != q->restricted_mem) {
> > + dprintk(q, 1, "restricted memory model mismatch\n");
> > + return false;
> > + }
> > + return true;
> > +}
> > +
> > static int vb2_core_allocated_buffers_storage(struct vb2_queue
> > *q)
> > {
> > if (!q->bufs)
> > @@ -864,6 +873,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum
> > vb2_memory memory,
> > unsigned int q_num_bufs = vb2_get_num_buffers(q);
> > unsigned plane_sizes[VB2_MAX_PLANES] = { };
> > bool non_coherent_mem = flags & V4L2_MEMORY_FLAG_NON_COHERENT;
> > + bool restricted_mem = flags & V4L2_MEMORY_FLAG_RESTRICTED;
> > unsigned int i, first_index;
> > int ret = 0;
> >
> > @@ -907,6 +917,9 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum
> > vb2_memory memory,
> > return 0;
> > }
> >
> > + if (restricted_mem && (!q->allow_restricted_mem || memory !=
> > VB2_MEMORY_DMABUF))
> > + return -EINVAL;
> > +
> > /*
> > * Make sure the requested values and current defaults are
> > sane.
> > */
> > @@ -924,6 +937,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum
> > vb2_memory memory,
> > if (ret)
> > return ret;
> > set_queue_coherency(q, non_coherent_mem);
> > + q->restricted_mem = restricted_mem;
> >
> > /*
> > * Ask the driver how many buffers and planes per buffer it
> > requires.
> > @@ -1032,6 +1046,7 @@ int vb2_core_create_bufs(struct vb2_queue *q,
> > enum vb2_memory memory,
> > unsigned plane_sizes[VB2_MAX_PLANES] = { };
> > bool non_coherent_mem = flags & V4L2_MEMORY_FLAG_NON_COHERENT;
> > unsigned int q_num_bufs = vb2_get_num_buffers(q);
> > + bool restricted_mem = flags & V4L2_MEMORY_FLAG_RESTRICTED;
> > bool no_previous_buffers = !q_num_bufs;
> > int ret = 0;
> >
> > @@ -1040,6 +1055,9 @@ int vb2_core_create_bufs(struct vb2_queue *q,
> > enum vb2_memory memory,
> > return -ENOBUFS;
> > }
> >
> > + if (restricted_mem && (!q->allow_restricted_mem || memory !=
> > VB2_MEMORY_DMABUF))
> > + return -EINVAL;
> > +
>
> This condition is repeated in another place. If it is ever to be
> changed, the person changing it must remember to look at both
> places. Maybe:
>
> static inline int restricted_mem_mismatch(bool restricted_mem,
> struct vb2_queue *q, enum vb2_memory
> memory)
> {
> return restricted_mem &&
> (!q->allow_restricted_mem || memory !=
> VB2_MEMORY_DMABUF) ?
> -1 : 0;
> }
>
> (you probably want to clean up line breaks)
>
> and:
>
> if (restricted_mem_mismatch(restricted_mem, q, memory))
> return -EINVAL;
>
> Regards,
>
> Andrzej
>
> > if (no_previous_buffers) {
> > if (q->waiting_in_dqbuf && *count) {
> > dprintk(q, 1, "another dup()ped fd is waiting
> > for a buffer\n");
> > @@ -1058,6 +1076,7 @@ int vb2_core_create_bufs(struct vb2_queue *q,
> > enum vb2_memory memory,
> > return ret;
> > q->waiting_for_buffers = !q->is_output;
> > set_queue_coherency(q, non_coherent_mem);
> > + q->restricted_mem = restricted_mem;
> > } else {
> > if (q->memory != memory) {
> > dprintk(q, 1, "memory model mismatch\n");
> > @@ -1065,6 +1084,8 @@ int vb2_core_create_bufs(struct vb2_queue *q,
> > enum vb2_memory memory,
> > }
> > if (!verify_coherency_flags(q, non_coherent_mem))
> > return -EINVAL;
> > + if (!verify_restricted_mem_flags(q, restricted_mem))
> > + return -EINVAL;
> > }
> >
> > num_buffers = min(*count, q->max_num_buf
回复：WARNING in drm_mode_create_lease_ioctl
> Cc'ing dri-devel > Does > https://lore.kernel.org/dri-devel/20240617035258.2774032-1-airl...@gmail.com/T/#u > help? > Thanks, > Dave. Hi. The issue does not appear after applied the patch. > On Mon, 17 Jun 2024 at 13:12, Ubisectech Sirius > wrote: >> >> Hello. >> We are Ubisectech Sirius Team, the vulnerability lab of China ValiantSec. >> Recently, our team has discovered a issue in Linux kernel 6.8. Attached to >> the email were a PoC file of the issue. >> >> Stack dump: >> >> [ cut here ] >> WARNING: CPU: 1 PID: 18929 at mm/page_alloc.c:4545 >> __alloc_pages+0x402/0x21b0 mm/page_alloc.c:4545 >> Modules linked in: >> CPU: 1 PID: 18929 Comm: syz-executor.3 Not tainted 6.8.0 #1 >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 >> 04/01/2014 >> RIP: 0010:__alloc_pages+0x402/0x21b0 mm/page_alloc.c:4545 >> Code: ff 00 0f 84 15 fe ff ff 80 ce 01 e9 0d fe ff ff 83 fe 0a 0f 86 0e fd >> ff ff 80 3d c7 cf 6a 0d 00 75 0b c6 05 be cf 6a 0d 01 90 <0f> 0b 90 45 31 e4 >> e9 87 fe ff ff e8 5e 3e 9b ff 84 c0 0f 85 7a fe >> RSP: 0018:c90001cc7808 EFLAGS: 00010246 >> RAX: RBX: RCX: 192000398f14 >> RDX: 0001 RSI: 000b RDI: 00040dc0 >> RBP: c90001cc7ab8 R08: 0007 R09: >> R10: R11: R12: 007a1200 >> R13: 000b R14: 00040dc0 R15: 000b >> FS: 7f1717ba5640() GS:88807ec0() knlGS: >> CS: 0010 DS: ES: CR0: 80050033 >> CR2: 203d CR3: 1f5c CR4: 00750ef0 >> DR0: DR1: DR2: >> DR3: DR6: fffe0ff0 DR7: 0400 >> PKRU: 5554 >> Call Trace: >> >> __alloc_pages_node include/linux/gfp.h:238 [inline] >> alloc_pages_node include/linux/gfp.h:261 [inline] >> __kmalloc_large_node+0x88/0x1a0 mm/slub.c:3926 >> __do_kmalloc_node mm/slub.c:3969 [inline] >> __kmalloc+0x370/0x480 mm/slub.c:3994 >> kmalloc_array include/linux/slab.h:627 [inline] >> kcalloc include/linux/slab.h:658 [inline] >> fill_object_idr drivers/gpu/drm/drm_lease.c:389 [inline] >> drm_mode_create_lease_ioctl+0x4ca/0x1f70 drivers/gpu/drm/drm_lease.c:522 >> drm_ioctl_kernel+0x1eb/0x3f0 drivers/gpu/drm/drm_ioctl.c:744 >> drm_ioctl+0x582/0xb70 drivers/gpu/drm/drm_ioctl.c:841 >> vfs_ioctl fs/ioctl.c:51 [inline] >> __do_sys_ioctl fs/ioctl.c:871 [inline] >> __se_sys_ioctl fs/ioctl.c:857 [inline] >> __x64_sys_ioctl+0x1a1/0x210 fs/ioctl.c:857 >> do_syscall_x64 arch/x86/entry/common.c:52 [inline] >> do_syscall_64+0xd5/0x270 arch/x86/entry/common.c:83 >> entry_SYSCALL_64_after_hwframe+0x6f/0x77 >> RIP: 0033:0x7f1716e8eeed >> Code: c3 e8 97 2b 00 00 0f 1f 80 00 00 00 00 f3 0f 1e fa 48 89 f8 48 89 f7 >> 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff >> 73 01 c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48 >> RSP: 002b:7f1717ba5028 EFLAGS: 0246 ORIG_RAX: 0010 >> RAX: ffda RBX: 7f1716fe3f80 RCX: 7f1716e8eeed >> RDX: 23c0 RSI: c01864c6 RDI: 0003 >> RBP: 7f1716f13014 R08: R09: > R10: R11: 0246 R12: > R13: 000b R14: 7f1716fe3f80 R15: 7f1717b85000 >> >> >> >> Thank you for taking the time to read this email and we look forward to >> working with you further. >> >> >> >> >> >> >> >> >> >>
Re: [PATCH RESEND, v6 6/8] mailbox: mediatek: Add CMDQ secure mailbox driver
Re: [PATCH v6,08/24] media: mediatek: vcodec: add tee client interface to communiate with optee-os
Hi Andrzej,
Thanks for your help to review this patch.
On Wed, 2024-05-22 at 14:21 +0200, Andrzej Pietrasiewicz wrote:
> Hi Yunfei & Jeffrey,
>
> W dniu 16.05.2024 o 14:20, Yunfei Dong pisze:
> > Open tee context to initialize the environment in order to
> > communication
> > with optee-os, then open tee session as the communication pipeline
> > for
> > lat and core to send data for hardware decode.
> >
> > Signed-off-by: Yunfei Dong
> > ---
> > .../platform/mediatek/vcodec/decoder/Makefile | 1 +
> > .../vcodec/decoder/mtk_vcodec_dec_drv.h | 5 +
> > .../vcodec/decoder/mtk_vcodec_dec_optee.c | 165
> > ++
> > .../vcodec/decoder/mtk_vcodec_dec_optee.h | 73
> > 4 files changed, 244 insertions(+)
> > create mode 100644
> > drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee
> > .c
> > create mode 100644
> > drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee
> > .h
> >
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/decoder/Makefile
> > b/drivers/media/platform/mediatek/vcodec/decoder/Makefile
> > index 904cd22def84..1624933dfd5e 100644
> > --- a/drivers/media/platform/mediatek/vcodec/decoder/Makefile
> > +++ b/drivers/media/platform/mediatek/vcodec/decoder/Makefile
> > @@ -21,5 +21,6 @@ mtk-vcodec-dec-y := vdec/vdec_h264_if.o \
> > mtk_vcodec_dec_stateful.o \
> > mtk_vcodec_dec_stateless.o \
> > mtk_vcodec_dec_pm.o \
> > + mtk_vcodec_dec_optee.o \
> >
> > mtk-vcodec-dec-hw-y := mtk_vcodec_dec_hw.o
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
> > .h
> > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
> > .h
> > index f975db4293da..76a0323f993c 100644
> > ---
> > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
> > .h
> > +++
> > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
> > .h
> > @@ -11,6 +11,7 @@
> > #include "../common/mtk_vcodec_dbgfs.h"
> > #include "../common/mtk_vcodec_fw_priv.h"
> > #include "../common/mtk_vcodec_util.h"
> > +#include "mtk_vcodec_dec_optee.h"
> > #include "vdec_msg_queue.h"
> >
> > #define MTK_VCODEC_DEC_NAME "mtk-vcodec-dec"
> > @@ -261,6 +262,8 @@ struct mtk_vcodec_dec_ctx {
> >* @dbgfs: debug log related information
> >*
> >* @chip_name: used to distinguish platforms and select the
> > correct codec configuration values
> > + *
> > + * @optee_private: optee private data
> >*/
> > struct mtk_vcodec_dec_dev {
> > struct v4l2_device v4l2_dev;
> > @@ -303,6 +306,8 @@ struct mtk_vcodec_dec_dev {
> > struct mtk_vcodec_dbgfs dbgfs;
> >
> > enum mtk_vcodec_dec_chip_name chip_name;
> > +
> > + struct mtk_vdec_optee_private *optee_private;
> > };
> >
> > static inline struct mtk_vcodec_dec_ctx *fh_to_dec_ctx(struct
> > v4l2_fh *fh)
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_opt
> > ee.c
> > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_opt
> > ee.c
> > new file mode 100644
> > index ..38d9c1c1785a
> > --- /dev/null
> > +++
> > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_opt
> > ee.c
> > @@ -0,0 +1,165 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2023 MediaTek Inc.
> > + * Author: Yunfei Dong
> > + */
> > +
> > +#include "mtk_vcodec_dec_drv.h"
> > +#include "mtk_vcodec_dec_optee.h"
> > +
> > +/*
> > + * Randomly generated, and must correspond to the GUID on the TA
> > side.
> > + */
> > +static const uuid_t mtk_vdec_lat_uuid =
> > + UUID_INIT(0xBC50D971, 0xD4C9, 0x42C4,
> > + 0x82, 0xCB, 0x34, 0x3F, 0xB7, 0xF3, 0x78, 0x90);
> > +
> > +static const uuid_t mtk_vdec_core_uuid =
> > + UUID_INIT(0xBC50D971, 0xD4C9, 0x42C4,
> > + 0x82, 0xCB, 0x34, 0x3F, 0xB7, 0xF3, 0x78, 0x91);
> > +
> > +/*
> > + * Check whether this driver supports decoder TA in the TEE
> > instance,
> > + * represented by the params (ver/data) of this function.
> > + */
> > +static int mtk_vcodec_dec_optee_match(struct
> > tee_ioctl_version_data *ver_data, const void *not_used)
> > +{
> > + if (ver_data->impl_id == TEE_IMPL_ID_OPTEE)
> > + return 1;
> > + else
> > + return 0;
>
> maybe:
>
> return ver_data->impl_id == TEE_IMPL_ID_OPTEE;
>
> > +}
> > +
> > +int mtk_vcodec_dec_optee_private_init(struct mtk_vcodec_dec_dev
> > *vcodec_dev)
> > +{
> > + vcodec_dev->optee_private = devm_kzalloc(&vcodec_dev->plat_dev-
> > >dev,
> > +sizeof(*vcodec_dev-
> > >optee_private),
> > +GFP_KERNEL);
> > + if (!vcodec_dev->optee_private)
> > + return -ENOMEM;
> > +
> > + vcodec_dev->optee_private->vcodec_dev = vcodec_dev;
> > +
> > + atomic_set(&vcodec_dev->optee_private->tee_active_cnt, 0);
> > + mutex_init(&vcodec_dev->optee_private->tee_mute
Re: [PATCH v2 5/7] drm/panic: Convert to drm_fb_clip_offset()
On 16/06/2024 11:12, Geert Uytterhoeven wrote:
On Sun, Jun 16, 2024 at 11:08 AM Geert Uytterhoeven
wrote:
On Sat, Jun 15, 2024 at 12:55 PM kernel test robot wrote:
kernel test robot noticed the following build errors:
[auto build test ERROR on drm-misc/drm-misc-next]
[cannot apply to linus/master v6.10-rc3 next-20240613]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/Geert-Uytterhoeven/drm-panic-Fix-uninitialized-drm_scanout_buffer-set_pixel-crash/20240614-032053
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:
https://lore.kernel.org/r/3121082eb4beb461773ebb6f656ed9b4286967ee.1718305355.git.geert%2Brenesas%40glider.be
patch subject: [PATCH v2 5/7] drm/panic: Convert to drm_fb_clip_offset()
config: x86_64-randconfig-003-20240615
(https://download.01.org/0day-ci/archive/20240615/202406151811.yeiz6203-...@intel.com/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20240615/202406151811.yeiz6203-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202406151811.yeiz6203-...@intel.com/
All errors (new ones prefixed by >>):
depmod: ERROR: Cycle detected: drm -> drm_kms_helper -> drm
depmod: ERROR: Found 2 modules in dependency cycles!
Oops, so DRM core cannot call any of the helpers, and DRM_PANIC
selecting DRM_KMS_HELPER was wrong in the first place?
Q: So how does this work with DRM_PANIC calling
drm_fb_helper_emergency_disable()?
A: drm_fb_helper_emergency_disable() is a dummy if
!CONFIG_DRM_FBDEV_EMULATION, so I guess no one tried to build
a failing randconfig with CONFIG_DRM_FBDEV_EMULATION=y yet.
drm_fb_helper_emergency_disable() is part of the series
https://patchwork.freedesktop.org/series/132720/
which, after discussing it on IRC with sima and Javier, is not a good
solution, and is abandoned.
I think the "select DRM_KMS_HELPER" is a leftover from earlier version
of drm_panic, that used the color conversion function from
drm_kms_helper, but that has changed in v10 and later.
drm_panic is called from the drm_core code, so in fact it can't depends
on the kms helper.
I don't see a good solution to workaround this circular dependency,
maybe depends on DRM_KMS_HELPER being built-in ? (so that means drm_core
will also be built-in). But that means platform that build drm core as
module, won't be able to use drm_panic.
There are a few of them in the kernel tree:
rg -l CONFIG_DRM=m
arch/powerpc/configs/85xx/stx_gp3_defconfig
arch/powerpc/configs/ppc6xx_defconfig
arch/powerpc/configs/skiroot_defconfig
arch/powerpc/configs/pmac32_defconfig
arch/mips/configs/ci20_defconfig
arch/arc/configs/axs101_defconfig
arch/arc/configs/axs103_smp_defconfig
arch/riscv/configs/defconfig
arch/parisc/configs/generic-32bit_defconfig
arch/arm/configs/davinci_all_defconfig
arch/arm/configs/omap2plus_defconfig
arch/arm/configs/pxa_defconfig
arch/arm64/configs/defconfig
Gr{oetje,eeting}s,
Geert
--
Jocelyn
Re: [PATCH] drm: zynqmp_dpsub: Fix an error handling path in zynqmp_dpsub_probe()
Hi,
On 16/06/2024 21:43, Laurent Pinchart wrote:
On Thu, Jun 13, 2024 at 11:05:01AM -0400, Sean Anderson wrote:
On 5/20/24 11:05, Sean Anderson wrote:
On 5/20/24 05:40, Christophe JAILLET wrote:
If zynqmp_dpsub_drm_init() fails, we must undo the previous
drm_bridge_add() call.
Fixes: be3f3042391d ("drm: zynqmp_dpsub: Always register bridge")
Signed-off-by: Christophe JAILLET
---
Compile tested only
---
drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
index face8d6b2a6f..f5781939de9c 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
@@ -269,6 +269,7 @@ static int zynqmp_dpsub_probe(struct platform_device *pdev)
return 0;
err_disp:
+ drm_bridge_remove(dpsub->bridge);
zynqmp_disp_remove(dpsub);
err_dp:
zynqmp_dp_remove(dpsub);
Reviewed-by: Sean Anderson
Will this be applied soon? The patch it fixes has made its way into
the stable tree already.
If someone can merge it in drm-misc that would be the fastest way.
Otherwise I'll send a pull request at some point, but I'm overworked at
the moment.
Thanks, I have pushed this to drm-misc-next.
Tomi
Re: [PATCH v2] drm: xlnx: zynqmp_dpsub: Enable plane in atomic update
Hi,
On 24/05/2024 02:49, Anatoliy Klymenko wrote:
Unconditionally enable the DPSUB layer in the corresponding atomic plane
update callback. Setting the new display mode may require disabling and
re-enabling the CRTC. This effectively resets DPSUB to the default state
with all layers disabled. The original implementation of the plane atomic
update enables the corresponding DPSUB layer only if the framebuffer
format has changed. This would leave the layer disabled after switching to
a different display mode with the same framebuffer format.
Signed-off-by: Anatoliy Klymenko
---
Changes in v2:
- Added comment around DPSUB layer enablement explaining why it should be
done unconditionally.
- Link to v1:
https://lore.kernel.org/r/20240520-dp-layer-enable-v1-1-c9b481209...@amd.com
---
drivers/gpu/drm/xlnx/zynqmp_kms.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c
b/drivers/gpu/drm/xlnx/zynqmp_kms.c
index 43bf416b33d5..0b57ab5451a9 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_kms.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c
@@ -120,9 +120,13 @@ static void zynqmp_dpsub_plane_atomic_update(struct
drm_plane *plane,
zynqmp_disp_blend_set_global_alpha(dpsub->disp, true,
plane->state->alpha >> 8);
- /* Enable or re-enable the plane if the format has changed. */
- if (format_changed)
- zynqmp_disp_layer_enable(layer);
+ /*
+* Unconditionally enable the layer, as it may have been disabled
+* previously either explicitly to reconfigure layer format, or
+* implicitly after DPSUB reset during display mode change. DRM
+* framework calls this callback for enabled planes only.
+*/
+ zynqmp_disp_layer_enable(layer);
}
static const struct drm_plane_helper_funcs zynqmp_dpsub_plane_helper_funcs = {
---
base-commit: 673087d8b023faf34b84e8faf63bbeea3da87bab
change-id: 20240520-dp-layer-enable-7b561af29ca8
Best regards,
Thanks, I have pushed this to drm-misc-next.
Tomi
Re: [PATCH v6, 10/24] media: mediatek: vcodec: send share memory data to optee
Hi Andrzej,
Thanks for your help to review this patch.
On Wed, 2024-05-22 at 14:22 +0200, Andrzej Pietrasiewicz wrote:
> Hi Yunfei & Jeffrey,
>
> W dniu 16.05.2024 o 14:20, Yunfei Dong pisze:
> > Setting msg and vsi information to shared buffer, then call tee
> > invoke
> > function to send it to optee-os.
> >
> > Signed-off-by: Yunfei Dong
> > ---
> > .../vcodec/decoder/mtk_vcodec_dec_optee.c | 140
> > ++
> > .../vcodec/decoder/mtk_vcodec_dec_optee.h | 51 +++
> > 2 files changed, 191 insertions(+)
> >
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_opt
> > ee.c
> > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_opt
> > ee.c
> > index 611fb0e56480..f29a8d143fee 100644
> > ---
> > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_opt
> > ee.c
> > +++
> > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_opt
> > ee.c
> > @@ -241,3 +241,143 @@ void mtk_vcodec_dec_optee_release(struct
> > mtk_vdec_optee_private *optee_private)
> > mutex_unlock(&optee_private->tee_mutex);
> > }
> > EXPORT_SYMBOL_GPL(mtk_vcodec_dec_optee_release);
> > +
> > +static int mtk_vcodec_dec_optee_fill_shm(struct tee_param
> > *command_params,
> > +struct
> > mtk_vdec_optee_shm_memref *shm_memref,
> > +struct
> > mtk_vdec_optee_data_to_shm *data,
> > +int index, struct device *dev)
> > +{
> > + if (!data->msg_buf_size[index] || !data->msg_buf[index]) {
> > + pr_err(MTK_DBG_VCODEC_STR "tee invalid buf param:
> > %d.\n", index);
> > + return -EINVAL;
> > + }
> > +
> > + *command_params = (struct tee_param) {
> > + .attr = shm_memref->param_type,
> > + .u.memref = {
> > + .shm = shm_memref->msg_shm,
> > + .size = data->msg_buf_size[index],
> > + .shm_offs = 0,
> > + },
> > + };
> > +
> > + if (!shm_memref->copy_to_ta) {
> > + dev_dbg(dev, MTK_DBG_VCODEC_STR "share memref data:
> > 0x%x param_type:%llu.\n",
> > + *((unsigned int *)shm_memref->msg_shm_ca_buf),
> > shm_memref->param_type);
> > + return 0;
> > + }
> > +
> > + memset(shm_memref->msg_shm_ca_buf, 0, shm_memref-
> > >msg_shm_size);
> > + memcpy(shm_memref->msg_shm_ca_buf, data->msg_buf[index], data-
> > >msg_buf_size[index]);
> > +
> > + dev_dbg(dev, MTK_DBG_VCODEC_STR "share memref data => msg
> > id:0x%x 0x%x param_type:%llu.\n",
> > + *((unsigned int *)data->msg_buf[index]),
> > + *((unsigned int *)shm_memref->msg_shm_ca_buf),
> > + shm_memref->param_type);
> > +
> > + return 0;
> > +}
> > +
> > +void mtk_vcodec_dec_optee_set_data(struct
> > mtk_vdec_optee_data_to_shm *data,
> > + void *buf, int buf_size,
> > + enum mtk_vdec_optee_data_index
> > index)
> > +{
> > + data->msg_buf[index] = buf;
> > + data->msg_buf_size[index] = buf_size;
> > +}
> > +EXPORT_SYMBOL_GPL(mtk_vcodec_dec_optee_set_data);
> > +
> > +int mtk_vcodec_dec_optee_invokd_cmd(struct mtk_vdec_optee_private
> > *optee_private,
> > + enum mtk_vdec_hw_id hw_id,
> > + struct mtk_vdec_optee_data_to_shm
> > *data)
> > +{
> > + struct device *dev = &optee_private->vcodec_dev->plat_dev->dev;
> > + struct tee_ioctl_invoke_arg trans_args;
> > + struct tee_param command_params[MTK_OPTEE_MAX_TEE_PARAMS];
> > + struct mtk_vdec_optee_ca_info *ca_info;
> > + struct mtk_vdec_optee_shm_memref *shm_memref;
> > + int ret, index;
> > +
> > + if (hw_id == MTK_VDEC_LAT0)
> > + ca_info = &optee_private->lat_ca;
> > + else
> > + ca_info = &optee_private->core_ca;
>
> You seem to be using this in several places. Maybe create a helper?
>
> static inline struct mtk_vdec_optee_ca_info *get_ca_info(
> struct mtk_vdec_optee_private *optee_private,
> enum mtk_vdec_hw_id hw_id)
> {
> return hw_id == MTK_VDEC_LAT0 ?
> &optee_private->lat_ca : &optee_private->core_ca;
> }
>
> (you want to clean up the line breaks in this suggested function)
>
> and then
>
> ca_info = get_ca_info(optee_private, hw_id);
>
> > +
> > + memset(&trans_args, 0, sizeof(trans_args));
> > + memset(command_params, 0, sizeof(command_params));
> > +
> > + trans_args = (struct tee_ioctl_invoke_arg) {
> > + .func = ca_info->vdec_session_func,
> > + .session = ca_info->vdec_session_id,
> > + .num_params = MTK_OPTEE_MAX_TEE_PARAMS,
> > + };
> > +
> > + /* Fill msg command parameters */
> > + for (index = 0; index < MTK_OPTEE_MAX_TEE_PARAMS; index++) {
> > + shm_memref = &ca_info->shm_memref[index];
> > +
> > + if (shm_memref->param_type ==
> > TEE_IOCTL_PARAM_ATTR_TYPE_NONE ||
> > + da
Re: [PATCH v5 00/10] drm: zynqmp_dp: IRQ cleanups and debugfs support
Hi Sean, On 03/05/2024 22:29, Sean Anderson wrote: This series cleans up the zyqnmp_dp IRQ and locking situation. Once that's done, it adds debugfs support. The intent is to enable compliance testing or to help debug signal-integrity issues. Last time I discussed converting the HPD work(s) to a threaded IRQ. I did not end up doing that for this series since the steps would be - Add locking - Move link retraining to a work function - Harden the IRQ - Merge the works into a threaded IRQ (omitted) Which with the exception of the final step is the same as leaving those works as-is. Conversion to a threaded IRQ can be done as a follow-up. I tested this, and the "drm: zynqmp_dp: Convert to a hard IRQ" causes a hang for me when unloading the drivers. Unfortunately I'm not in the condition to debug it at the moment. I have picked the first three patches into drm-misc-next, though, to decrease the number of patches in the series a bit. They looked independent and safe enough to apply. Tomi Changes in v5: - Fix AUX bus not getting unregistered - Rebase onto drm-misc/drm-misc-next Changes in v4: - Rebase onto drm/drm-next Changes in v3: - Don't delay work - Convert to a hard IRQ - Use AUX IRQs instead of polling - Take dp->lock in zynqmp_dp_hpd_work_func Changes in v2: - Rearrange zynqmp_dp for better padding - Split off the HPD IRQ work into another commit - Expand the commit message - Document hpd_irq_work - Document debugfs files - Add ignore_aux_errors and ignore_hpd debugfs files to replace earlier implicit functionality - Attempt to fix unreproducable, spurious build warning - Drop "Optionally ignore DPCD errors" in favor of a debugfs file directly affecting zynqmp_dp_aux_transfer. Sean Anderson (10): drm: zynqmp_kms: Fix AUX bus not getting unregistered drm: zynqmp_dp: Rearrange zynqmp_dp for better padding drm: zynqmp_dp: Don't delay work drm: zynqmp_dp: Add locking drm: zynqmp_dp: Don't retrain the link in our IRQ drm: zynqmp_dp: Convert to a hard IRQ drm: zynqmp_dp: Use AUX IRQs instead of polling drm: zynqmp_dp: Split off several helper functions drm: zynqmp_dp: Take dp->lock in zynqmp_dp_hpd_work_func drm: zynqmp_dp: Add debugfs interface for compliance testing Documentation/gpu/drivers.rst | 1 + Documentation/gpu/zynqmp.rst | 149 + MAINTAINERS | 1 + drivers/gpu/drm/xlnx/zynqmp_dp.c | 883 +++--- drivers/gpu/drm/xlnx/zynqmp_kms.c | 12 +- 5 files changed, 977 insertions(+), 69 deletions(-) create mode 100644 Documentation/gpu/zynqmp.rst
Re: [PATCH v6,12/24] media: mediatek: vcodec: add interface to allocate/free secure memory
Hi Andrzej,
Thanks for your help to review this patch.
On Wed, 2024-05-22 at 14:25 +0200, Andrzej Pietrasiewicz wrote:
> Hi Yunfei,
>
> W dniu 16.05.2024 o 14:20, Yunfei Dong pisze:
> > Need to call dma heap interface to allocate/free secure memory when
> > playing
> > secure video.
> >
> > Signed-off-by: Yunfei Dong
> > ---
> > .../media/platform/mediatek/vcodec/Kconfig| 1 +
> > .../mediatek/vcodec/common/mtk_vcodec_util.c | 122
> > +-
> > .../mediatek/vcodec/common/mtk_vcodec_util.h | 3 +
> > 3 files changed, 123 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/platform/mediatek/vcodec/Kconfig
> > b/drivers/media/platform/mediatek/vcodec/Kconfig
> > index bc8292232530..707865703e61 100644
> > --- a/drivers/media/platform/mediatek/vcodec/Kconfig
> > +++ b/drivers/media/platform/mediatek/vcodec/Kconfig
> > @@ -17,6 +17,7 @@ config VIDEO_MEDIATEK_VCODEC
> > depends on VIDEO_MEDIATEK_VPU || !VIDEO_MEDIATEK_VPU
> > depends on MTK_SCP || !MTK_SCP
> > depends on MTK_SMI || (COMPILE_TEST && MTK_SMI=n)
> > + depends on DMABUF_HEAPS
> > select VIDEOBUF2_DMA_CONTIG
> > select V4L2_MEM2MEM_DEV
> > select VIDEO_MEDIATEK_VCODEC_VPU if VIDEO_MEDIATEK_VPU
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.c
> > b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.c
> > index c60e4c193b25..5958dcd7965a 100644
> > ---
> > a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.c
> > +++
> > b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.c
> > @@ -5,9 +5,11 @@
> > * Tiffany Lin
> > */
> >
> > +#include
> > #include
> > #include
> > #include
> > +#include
> >
> > #include "../decoder/mtk_vcodec_dec_drv.h"
> > #include "../encoder/mtk_vcodec_enc_drv.h"
> > @@ -45,7 +47,7 @@ int mtk_vcodec_write_vdecsys(struct
> > mtk_vcodec_dec_ctx *ctx, unsigned int reg,
> > }
> > EXPORT_SYMBOL(mtk_vcodec_write_vdecsys);
> >
> > -int mtk_vcodec_mem_alloc(void *priv, struct mtk_vcodec_mem *mem)
> > +static int mtk_vcodec_mem_alloc_nor(void *priv, struct
> > mtk_vcodec_mem *mem)
> > {
> > enum mtk_instance_type inst_type = *((unsigned int *)priv);
> > struct platform_device *plat_dev;
> > @@ -75,9 +77,71 @@ int mtk_vcodec_mem_alloc(void *priv, struct
> > mtk_vcodec_mem *mem)
> >
> > return 0;
> > }
> > -EXPORT_SYMBOL(mtk_vcodec_mem_alloc);
> >
> > -void mtk_vcodec_mem_free(void *priv, struct mtk_vcodec_mem *mem)
> > +static int mtk_vcodec_mem_alloc_sec(struct mtk_vcodec_dec_ctx
> > *ctx, struct mtk_vcodec_mem *mem)
> > +{
> > + struct device *dev = &ctx->dev->plat_dev->dev;
> > + struct dma_buf *dma_buffer;
> > + struct dma_heap *vdec_heap;
> > + struct dma_buf_attachment *attach;
> > + struct sg_table *sgt;
> > + unsigned long size = mem->size;
> > + int ret = 0;
> > +
> > + if (!size)
> > + return -EINVAL;
> > +
> > + vdec_heap = dma_heap_find("restricted_mtk_cma");
> > + if (!vdec_heap) {
> > + mtk_v4l2_vdec_err(ctx, "dma heap find failed!");
> > + return -EPERM;
> > + }
> > +
> > + dma_buffer = dma_heap_buffer_alloc(vdec_heap, size,
> > DMA_HEAP_VALID_FD_FLAGS,
> > + DMA_HEAP_VALID_HEAP_FLAGS);
> > + if (IS_ERR_OR_NULL(dma_buffer)) {
> > + mtk_v4l2_vdec_err(ctx, "dma heap alloc size=0x%lx
> > failed!", size);
> > + return PTR_ERR(dma_buffer);
> > + }
> > +
> > + attach = dma_buf_attach(dma_buffer, dev);
> > + if (IS_ERR_OR_NULL(attach)) {
> > + mtk_v4l2_vdec_err(ctx, "dma attach size=0x%lx failed!",
> > size);
> > + ret = PTR_ERR(attach);
> > + goto err_attach;
> > + }
> > +
> > + sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
> > + if (IS_ERR_OR_NULL(sgt)) {
> > + mtk_v4l2_vdec_err(ctx, "dma map attach size=0x%lx
> > failed!", size);
> > + ret = PTR_ERR(sgt);
> > + goto err_sgt;
> > + }
> > +
> > + mem->va = dma_buffer;
> > + mem->dma_addr = (dma_addr_t)sg_dma_address((sgt)->sgl);
> > +
> > + if (!mem->va || !mem->dma_addr) {
> > + mtk_v4l2_vdec_err(ctx, "dma buffer size=0x%lx failed!",
> > size);
> > + ret = -EPERM;
> > + goto err_addr;
> > + }
> > +
> > + mem->attach = attach;
> > + mem->sgt = sgt;
> > +
> > + return 0;
> > +err_addr:
> > + dma_buf_unmap_attachment(attach, sgt, DMA_BIDIRECTIONAL);
> > +err_sgt:
> > + dma_buf_detach(dma_buffer, attach);
> > +err_attach:
> > + dma_buf_put(dma_buffer);
> > +
> > + return ret;
> > +}
> > +
> > +static void mtk_vcodec_mem_free_nor(void *priv, struct
> > mtk_vcodec_mem *mem)
> > {
> > enum mtk_instance_type inst_type = *((unsigned int *)priv);
> > struct platform_device *plat_dev;
> > @@ -110,6 +174,57 @@ void mtk_vcodec_mem_free(void *priv, struct
> > mtk_vcodec_mem *mem)
> > mem->dma_addr = 0;
> > mem->size = 0;
> > }
> > +
>
Re: [PATCH 09/13] drm/mediatek: Fix XRGB setting error in OVL
Re: [PATCH RESEND v5 00/16] Add audio support for the MediaTek Genio 350-evk board
On 14/06/2024 11:31, Mark Brown wrote: On Fri, Jun 14, 2024 at 09:27:43AM +0200, Alexandre Mergnat wrote: This serie aim to add the following audio support for the Genio 350-evk: - Playback - 2ch Headset Jack (Earphone) - 1ch Line-out Jack (Speaker) - 8ch HDMI Tx I seem to remember you had review comments that needed addressing from AngeloGioacchino, why resend without addressing those? I don't see any comment: https://lore.kernel.org/lkml/20240226-audio-i350-v5-0-e7e2569df...@baylibre.com/ -- Regards, Alexandre
Re: [PATCH v2 1/2] drm/bridge: tc358767: Add format negotiation hooks for DPI/DSI to (e)DP
On Wed, Nov 08, 2023 at 01:27:22PM GMT, Tomi Valkeinen wrote: > From: Aradhya Bhatia > > With new connector model, tc358767 will not create the connector, when > DRM_BRIDGE_ATTACH_NO_CONNECTOR is set and display-controller driver will > rely on format negotiation to setup the encoder format. > > Add the missing bus format negotiation hooks in the > drm_bridge_funcs to complete DRM_BRIDGE_ATTACH_NO_CONNECTOR support. > > Output format, for DPI/DSI to DP, is selected to > MEDIA_BUS_FMT_RGB888_1X24 as default, keeping in mind what the older > model used to support. > > Reported-by: Jan Kiszka > Closes: > https://lore.kernel.org/all/24282420-b4dd-45b3-bb1c-fc37fe4a8...@siemens.com/ > Signed-off-by: Aradhya Bhatia > Signed-off-by: Tomi Valkeinen > --- > drivers/gpu/drm/bridge/tc358767.c | 25 + > 1 file changed, 25 insertions(+) > If this is a fix, where is the Fixes tag? -- With best wishes Dmitry
Re: [PATCH v2 0/2] drm/bridge: tc358767: Fix DRM_BRIDGE_ATTACH_NO_CONNECTOR case
On Mon, Jun 17, 2024 at 07:40:32AM GMT, Jan Kiszka wrote: > On 16.02.24 15:57, Marek Vasut wrote: > > On 2/16/24 10:10, Tomi Valkeinen wrote: > >> Ok. Does anyone have a worry that these patches make the situation > >> worse for the DSI case than it was before? Afaics, if the DSI lanes > >> are not set up early enough by the DSI host, the driver would break > >> with and without these patches. > >> > >> These do fix the driver for DRM_BRIDGE_ATTACH_NO_CONNECTOR and DPI, so > >> I'd like to merge these unless these cause a regression with the DSI > >> case. > > > > 1/2 looks good to me, go ahead and apply . > > My local patches still apply on top of 6.10-rc4, so I don't think this > ever happened. What's still holding up this long-pending fix (at least > for our devices)? Neither of the patches contains Fixes tags. If the first patch fixes an issue in previous kernels, please consider following the stable process. If we are unsure about the second patch, please send the first patch separately, adding proper tags. -- With best wishes Dmitry
Re: [PATCH] drm/panic: depends on !VT_CONSOLE
On 16/06/2024 14:43, Javier Martinez Canillas wrote: Jocelyn Falempe writes: Hello Jocelyn, The race condition between fbcon and drm_panic can only occurs if VT_CONSOLE is set. So update drm_panic dependency accordingly. This will make it easier for Linux distributions to enable drm_panic by disabling VT_CONSOLE, and keeping fbcon terminal. The only drawback is that fbcon won't display the boot kmsg, so it should rely on userspace to do that. At least plymouth already handle this case with https://gitlab.freedesktop.org/plymouth/plymouth/-/merge_requests/224 Suggested-by: Daniel Vetter Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index a9df94291622..f5c989aed7e9 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -107,7 +107,7 @@ config DRM_KMS_HELPER config DRM_PANIC bool "Display a user-friendly message when a kernel panic occurs" - depends on DRM && !FRAMEBUFFER_CONSOLE + depends on DRM && !(FRAMEBUFFER_CONSOLE && VT_CONSOLE) I thought the idea was to only make it depend on !VT_CONSOLE, so that distros could also enable fbcon / VT but prevent the race condition to happen due the VT not being a system console for the kernel to print messages ? Yes, but when writing the patch, I thought that if you have VT_CONSOLE=y and FRAMEBUFFER_CONSOLE=n, then there won't be any race condition, and drm_panic can be enabled safely. I don't know if that really matters, and if VT_CONSOLE has any usage apart from fbcon. In other words, my understanding from the discussion with Sima was that this should be instead: + depends on DRM && !VT_CONSOLE I've tested that and at least I see that a framebuffer console is present and `echo c > /proc/sysrq-trigger` triggers the DRM panic handler message (but don't know if the race exists and is just that I was not hitting it). If my understanding is correct and should only be a depends on !VT_CONSOLE then feel free to add my: Tested-by: Javier Martinez Canillas Best regards, -- Jocelyn
Re: [PATCH v9 10/13] PCI: Give pci_intx() its own devres callback
On Fri, 2024-06-14 at 11:14 -0500, Bjorn Helgaas wrote: > On Fri, Jun 14, 2024 at 10:09:46AM +0200, Philipp Stanner wrote: > > On Thu, 2024-06-13 at 16:06 -0500, Bjorn Helgaas wrote: > > > On Thu, Jun 13, 2024 at 01:50:23PM +0200, Philipp Stanner wrote: > > > > pci_intx() is one of the functions that have "hybrid mode" > > > > (i.e., > > > > sometimes managed, sometimes not). Providing a separate > > > > pcim_intx() > > > > function with its own device resource and cleanup callback > > > > allows > > > > for > > > > removing further large parts of the legacy PCI devres > > > > implementation. > > > > > > > > As in the region-request-functions, pci_intx() has to call into > > > > its > > > > managed counterpart for backwards compatibility. > > > > > > > > As pci_intx() is an outdated function, pcim_intx() shall not be > > > > made > > > > visible to drivers via a public API. > > > > > > What makes pci_intx() outdated? If it's outdated, we should > > > mention > > > why and what the 30+ callers (including a couple in drivers/pci/) > > > should use instead. > > > > That is 100% based on Andy Shevchenko's (+CC) statement back from > > January 2024 a.D. [1] > > > > Apparently INTx is "old IRQ management" and should be done through > > pci_alloc_irq_vectors() nowadays. > > Do we have pcim_ support for pci_alloc_irq_vectors()? Nope. All PCI devres functions that exist are now in pci/devres.c, except for the hybrid functions (pci_intx() and everything calling __pci_request_region()) in pci.c P. > > > [1] > > https://lore.kernel.org/all/ZabyY3csP0y-p7lb@surfacebook.localdomain/ >
Re: [PATCH] drm/panic: depends on !VT_CONSOLE
Hi Jocelyn,
On Mon, Jun 17, 2024 at 10:16 AM Jocelyn Falempe wrote:
> On 16/06/2024 14:43, Javier Martinez Canillas wrote:
> > Jocelyn Falempe writes:
> >> The race condition between fbcon and drm_panic can only occurs if
> >> VT_CONSOLE is set. So update drm_panic dependency accordingly.
> >> This will make it easier for Linux distributions to enable drm_panic
> >> by disabling VT_CONSOLE, and keeping fbcon terminal.
> >> The only drawback is that fbcon won't display the boot kmsg, so it
> >> should rely on userspace to do that.
> >> At least plymouth already handle this case with
> >> https://gitlab.freedesktop.org/plymouth/plymouth/-/merge_requests/224
> >>
> >> Suggested-by: Daniel Vetter
> >> Signed-off-by: Jocelyn Falempe
> >> ---
> >> drivers/gpu/drm/Kconfig | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> >> index a9df94291622..f5c989aed7e9 100644
> >> --- a/drivers/gpu/drm/Kconfig
> >> +++ b/drivers/gpu/drm/Kconfig
> >> @@ -107,7 +107,7 @@ config DRM_KMS_HELPER
> >>
> >> config DRM_PANIC
> >> bool "Display a user-friendly message when a kernel panic occurs"
> >> -depends on DRM && !FRAMEBUFFER_CONSOLE
> >> +depends on DRM && !(FRAMEBUFFER_CONSOLE && VT_CONSOLE)
> >
> > I thought the idea was to only make it depend on !VT_CONSOLE, so that
> > distros could also enable fbcon / VT but prevent the race condition to
> > happen due the VT not being a system console for the kernel to print
> > messages ?
>
> Yes, but when writing the patch, I thought that if you have VT_CONSOLE=y
> and FRAMEBUFFER_CONSOLE=n, then there won't be any race condition, and
> drm_panic can be enabled safely.
> I don't know if that really matters, and if VT_CONSOLE has any usage
> apart from fbcon.
It is used for any kind of virtual terminal, so also for vgacon.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH] drm/panic: depends on !VT_CONSOLE
Jocelyn Falempe writes: > On 16/06/2024 14:43, Javier Martinez Canillas wrote: >> Jocelyn Falempe writes: >> >> Hello Jocelyn, >> >>> The race condition between fbcon and drm_panic can only occurs if >>> VT_CONSOLE is set. So update drm_panic dependency accordingly. >>> This will make it easier for Linux distributions to enable drm_panic >>> by disabling VT_CONSOLE, and keeping fbcon terminal. >>> The only drawback is that fbcon won't display the boot kmsg, so it >>> should rely on userspace to do that. >>> At least plymouth already handle this case with >>> https://gitlab.freedesktop.org/plymouth/plymouth/-/merge_requests/224 >>> >>> Suggested-by: Daniel Vetter >>> Signed-off-by: Jocelyn Falempe >>> --- >>> drivers/gpu/drm/Kconfig | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >>> index a9df94291622..f5c989aed7e9 100644 >>> --- a/drivers/gpu/drm/Kconfig >>> +++ b/drivers/gpu/drm/Kconfig >>> @@ -107,7 +107,7 @@ config DRM_KMS_HELPER >>> >>> config DRM_PANIC >>> bool "Display a user-friendly message when a kernel panic occurs" >>> - depends on DRM && !FRAMEBUFFER_CONSOLE >>> + depends on DRM && !(FRAMEBUFFER_CONSOLE && VT_CONSOLE) >> >> I thought the idea was to only make it depend on !VT_CONSOLE, so that >> distros could also enable fbcon / VT but prevent the race condition to >> happen due the VT not being a system console for the kernel to print >> messages ? > > Yes, but when writing the patch, I thought that if you have VT_CONSOLE=y > and FRAMEBUFFER_CONSOLE=n, then there won't be any race condition, and > drm_panic can be enabled safely. Ah, I see what you mean. > I don't know if that really matters, and if VT_CONSOLE has any usage > apart from fbcon. > Right. I don't know... -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
[PATCH RESEND] drm/display: Drop obsolete dependency on COMPILE_TEST
Since commit 0166dc11be91 ("of: make CONFIG_OF user selectable"), it
is possible to test-build any driver which depends on OF on any
architecture by explicitly selecting OF. Therefore depending on
COMPILE_TEST as an alternative is no longer needed.
Signed-off-by: Jean Delvare
Reviewed-by: Javier Martinez Canillas
Cc: David Airlie
Cc: Daniel Vetter
---
Already sent on: 2022-11-21, 2023-01-27, 2023-12-02
This is one of the only 3 remaining occurrences of this deprecated
construct.
Who can pick this patch and get it upstream?
drivers/gpu/drm/display/Kconfig |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-6.6.orig/drivers/gpu/drm/display/Kconfig
+++ linux-6.6/drivers/gpu/drm/display/Kconfig
@@ -3,7 +3,7 @@
config DRM_DP_AUX_BUS
tristate
depends on DRM
- depends on OF || COMPILE_TEST
+ depends on OF
config DRM_DISPLAY_HELPER
tristate
--
Jean Delvare
SUSE L3 Support
[PATCH RESEND] drm/logicvc: Drop obsolete dependency on COMPILE_TEST
Since commit 0166dc11be91 ("of: make CONFIG_OF user selectable"), it
is possible to test-build any driver which depends on OF on any
architecture by explicitly selecting OF. Therefore depending on
COMPILE_TEST as an alternative is no longer needed.
Signed-off-by: Jean Delvare
Reviewed-by: Paul Kocialkowski
Cc: David Airlie
Cc: Daniel Vetter
---
Already sent on: 2022-11-21, 2023-01-27, 2023-12-02
This is one of the only 3 remaining occurrences of this deprecated
construct.
Who can pick this patch and get it upstream?
drivers/gpu/drm/logicvc/Kconfig |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-6.6.orig/drivers/gpu/drm/logicvc/Kconfig
+++ linux-6.6/drivers/gpu/drm/logicvc/Kconfig
@@ -1,7 +1,7 @@
config DRM_LOGICVC
tristate "LogiCVC DRM"
depends on DRM
- depends on OF || COMPILE_TEST
+ depends on OF
select DRM_KMS_HELPER
select DRM_KMS_DMA_HELPER
select DRM_GEM_DMA_HELPER
--
Jean Delvare
SUSE L3 Support
Re: [PATCH 3/3] drm/panel: simple: Add PrimeView PM070WL4 support
On Thu, Jun 6, 2024 at 9:28 AM Primoz Fiser wrote: > Add support for PrimeView PM070WL4 7.0" (800x480) TFT-LCD panel. > Datasheet can be found at [1]. > > [1] https://www.beyondinfinite.com/lcd/Library/Pvi/PM070WL4-V1.0.pdf > > Signed-off-by: Primoz Fiser Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH] drm: zynqmp_dpsub: Fix an error handling path in zynqmp_dpsub_probe()
On Mon, Jun 17, 2024 at 10:43:44AM +0300, Tomi Valkeinen wrote:
> On 16/06/2024 21:43, Laurent Pinchart wrote:
> > On Thu, Jun 13, 2024 at 11:05:01AM -0400, Sean Anderson wrote:
> >> On 5/20/24 11:05, Sean Anderson wrote:
> >>> On 5/20/24 05:40, Christophe JAILLET wrote:
> If zynqmp_dpsub_drm_init() fails, we must undo the previous
> drm_bridge_add() call.
>
> Fixes: be3f3042391d ("drm: zynqmp_dpsub: Always register bridge")
> Signed-off-by: Christophe JAILLET
> ---
> Compile tested only
> ---
> drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
> b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
> index face8d6b2a6f..f5781939de9c 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
> @@ -269,6 +269,7 @@ static int zynqmp_dpsub_probe(struct platform_device
> *pdev)
> return 0;
>
> err_disp:
> +drm_bridge_remove(dpsub->bridge);
> zynqmp_disp_remove(dpsub);
> err_dp:
> zynqmp_dp_remove(dpsub);
> >>>
> >>> Reviewed-by: Sean Anderson
> >>
> >> Will this be applied soon? The patch it fixes has made its way into
> >> the stable tree already.
> >
> > If someone can merge it in drm-misc that would be the fastest way.
> > Otherwise I'll send a pull request at some point, but I'm overworked at
> > the moment.
>
> Thanks, I have pushed this to drm-misc-next.
Thank you Tomi, much appreciated.
--
Regards,
Laurent Pinchart
Re: [PATCH v2] drm/mediatek: Call drm_atomic_helper_shutdown() at shutdown time
On Tue, Jun 11, 2024 at 7:28 PM Douglas Anderson wrote:
> Based on grepping through the source code this driver appears to be
> missing a call to drm_atomic_helper_shutdown() at system shutdown
> time. Among other things, this means that if a panel is in use that it
> won't be cleanly powered off at system shutdown time.
>
> The fact that we should call drm_atomic_helper_shutdown() in the case
> of OS shutdown/restart comes straight out of the kernel doc "driver
> instance overview" in drm_drv.c.
>
> This driver users the component model and shutdown happens in the base
> driver. The "drvdata" for this driver will always be valid if
> shutdown() is called and as of commit 2a073968289d
> ("drm/atomic-helper: drm_atomic_helper_shutdown(NULL) should be a
> noop") we don't need to confirm that "drm" is non-NULL.
>
> Suggested-by: Maxime Ripard
> Reviewed-by: Maxime Ripard
> Reviewed-by: Fei Shao
> Tested-by: Fei Shao
> Signed-off-by: Douglas Anderson
Reviewed-by: Linus Walleij
Yours,
Linus Walleij
Re: [PATCH 2/2] iio: dac: Fix dependencies of AD9739A
On Sat, Jun 15, 2024 at 06:25:11PM -0700, Alexey Makhalov wrote:
> 0-DAY CI Kernel Test automation reported an issue:
>
>ld: drivers/base/regmap/regmap-spi.o: in function `regmap_spi_read':
>regmap-spi.c:(.text+0xf): undefined reference to `spi_write_then_read'
>ld: drivers/base/regmap/regmap-spi.o: in function
> `regmap_spi_gather_write':
>regmap-spi.c:(.text+0x2b4): undefined reference to `spi_sync'
>ld: drivers/base/regmap/regmap-spi.o: in function
> `spi_sync_transfer.constprop.0':
>regmap-spi.c:(.text+0x337): undefined reference to `spi_sync'
>ld: drivers/base/regmap/regmap-spi.o: in function `regmap_spi_async_write':
>regmap-spi.c:(.text+0x445): undefined reference to `spi_async'
>ld: drivers/iio/dac/ad9739a.o: in function `ad9739a_driver_init':
>ad9739a.c:(.init.text+0x10): undefined reference to `__spi_register_driver'
>
> Kconfig warnings: (for reference only)
>WARNING: unmet direct dependencies detected for REGMAP_SPI
>Depends on [n]: SPI [=n]
>Selected by [y]:
>- AD9739A [=y] && IIO [=y] && (SPI [=n] || COMPILE_TEST [=y])
>
> The issue is caused by CONFIG_AD9739A=y when CONFIG_SPI is not set.
>
> Add explicit dependency on SPI and conditional selection of REGMAP_SPI.
>
> Fixes: e77603d5468b ("iio: dac: support the ad9739a RF DAC")
> Reported-by: kernel test robot
> Closes:
> https://lore.kernel.org/oe-kbuild-all/202406152104.fxakp1mb-...@intel.com/
> Signed-off-by: Alexey Makhalov
> ---
> drivers/iio/dac/Kconfig | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index 3c2bf620f00f..d095f4d26e49 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -133,8 +133,8 @@ config AD5624R_SPI
>
> config AD9739A
> tristate "Analog Devices AD9739A RF DAC spi driver"
> - depends on SPI || COMPILE_TEST
> - select REGMAP_SPI
> + depends on SPI
> + select REGMAP_SPI if SPI_MASTER
> select IIO_BACKEND
> help
> Say yes here to build support for Analog Devices AD9739A Digital-to
> --
FWIW, I appreciate it you fixing other breakages. However, there's a patch for
that already, on its way:
https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?h=fixes-togreg&id=75183e461ce033605c3e85518a9f3d4e4ef848a3
Don't get discouraged, though, when fixing something that is not in our
immediate area of interest!
:-)
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: Design session notes: GPU acceleration in Xen
On 14.06.2024 18:35, Demi Marie Obenour wrote: > On Fri, Jun 14, 2024 at 08:38:51AM +0200, Jan Beulich wrote: >> On 13.06.2024 20:43, Demi Marie Obenour wrote: >>> 2. Add support for `XEN_DOMCTL_memory_mapping` to use system RAM, not >>>just IOMEM. Mappings made with `XEN_DOMCTL_memory_mapping` are >>>guaranteed to be able to be successfully revoked with >>>`XEN_DOMCTL_memory_mapping`, so all operations that would create >>>extra references to the mapped memory must be forbidden. These >>>include, but may not be limited to: >>> >>>1. Granting the pages to the same or other domains. >>>2. Mapping into another domain using `XEN_DOMCTL_memory_mapping`. >>>3. Another domain accessing the pages using the foreign memory APIs, >>> unless it is privileged over the domain that owns the pages. >> >> All of which may call for actually converting the memory to kind-of-MMIO, >> with a means to later convert it back. > > Would this support the case where the mapping domain is not fully > priviliged, and where it might be a PV guest? I suppose that should be a goal. Jan
Re: [PATCH 1/2] drm/vmwgfx: Fix missing HYPERVISOR_GUEST dependency
On Sat, Jun 15, 2024 at 06:25:10PM -0700, Alexey Makhalov wrote: > VMWARE_HYPERCALL alternative will not work as intended without > VMware guest code initialization. > > Reported-by: kernel test robot > Closes: > https://lore.kernel.org/oe-kbuild-all/202406152104.fxakp1mb-...@intel.com/ > Signed-off-by: Alexey Makhalov > --- > drivers/gpu/drm/vmwgfx/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/vmwgfx/Kconfig b/drivers/gpu/drm/vmwgfx/Kconfig > index faddae3d6ac2..6f1ac940cbae 100644 > --- a/drivers/gpu/drm/vmwgfx/Kconfig > +++ b/drivers/gpu/drm/vmwgfx/Kconfig > @@ -2,7 +2,7 @@ > config DRM_VMWGFX > tristate "DRM driver for VMware Virtual GPU" > depends on DRM && PCI && MMU > - depends on X86 || ARM64 > + depends on (X86 && HYPERVISOR_GUEST) || ARM64 > select DRM_TTM > select DRM_TTM_HELPER > select MAPPING_DIRTY_HELPERS > -- Right, I'll queue this soon but it doesn't reproduce here with gcc-11 or gcc-13. This must be something gcc-9 specific or so... -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] drm/panic: depends on !VT_CONSOLE
On 17/06/2024 10:25, Geert Uytterhoeven wrote:
Hi Jocelyn,
On Mon, Jun 17, 2024 at 10:16 AM Jocelyn Falempe wrote:
On 16/06/2024 14:43, Javier Martinez Canillas wrote:
Jocelyn Falempe writes:
The race condition between fbcon and drm_panic can only occurs if
VT_CONSOLE is set. So update drm_panic dependency accordingly.
This will make it easier for Linux distributions to enable drm_panic
by disabling VT_CONSOLE, and keeping fbcon terminal.
The only drawback is that fbcon won't display the boot kmsg, so it
should rely on userspace to do that.
At least plymouth already handle this case with
https://gitlab.freedesktop.org/plymouth/plymouth/-/merge_requests/224
Suggested-by: Daniel Vetter
Signed-off-by: Jocelyn Falempe
---
drivers/gpu/drm/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index a9df94291622..f5c989aed7e9 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -107,7 +107,7 @@ config DRM_KMS_HELPER
config DRM_PANIC
bool "Display a user-friendly message when a kernel panic occurs"
-depends on DRM && !FRAMEBUFFER_CONSOLE
+depends on DRM && !(FRAMEBUFFER_CONSOLE && VT_CONSOLE)
I thought the idea was to only make it depend on !VT_CONSOLE, so that
distros could also enable fbcon / VT but prevent the race condition to
happen due the VT not being a system console for the kernel to print
messages ?
Yes, but when writing the patch, I thought that if you have VT_CONSOLE=y
and FRAMEBUFFER_CONSOLE=n, then there won't be any race condition, and
drm_panic can be enabled safely.
I don't know if that really matters, and if VT_CONSOLE has any usage
apart from fbcon.
It is used for any kind of virtual terminal, so also for vgacon.
Ah thanks, but I think vgacon is safe to use with drm_panic.
The problem with fbcon, is that it can also uses the fbdev emulation
from the drm driver, and in this case, drm_panic will write to the same
framebuffer as fbcon during a panic, leading to corrupted output.
This can't occur with other graphical console, so !(FRAMEBUFFER_CONSOLE
&& VT_CONSOLE) is probably good.
--
Jocelyn
Gr{oetje,eeting}s,
Geert
Re: Linux 6.10-rc1
Hi! > > Let's bring in the actual gpu people.. Dave/Jani/others - does any of > > this sound familiar? Pavel says things have gotten much slower in > > 6.10: "something was very wrong with the performance, likely to do > > with graphics" > > Actually, maybe it's not graphics at all. Rafael just sent me a pull > request that fixes a "turbo is disabled at boot, but magically enabled > at runtime by firmware" issue. > > The 6.10-rc1 kernel would notice that turbo was disabled, and stopped > noticing that it magically got re-enabled. > > Pavel, that was with a very different laptop, but who knows... That > would match the "laptop is much slower" thing. > > So current -git might be worth checking. So... I went to (then) current -git and I don't want to replace my machine any more. So the problem should not exist in current mainline. (I did not have good objective data, so I'm not 100% sure problem was real in the first place. More like 90% sure.) Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH] drm/panic: depends on !VT_CONSOLE
Jocelyn Falempe writes: > On 17/06/2024 10:25, Geert Uytterhoeven wrote: >> Hi Jocelyn, >> >> On Mon, Jun 17, 2024 at 10:16 AM Jocelyn Falempe wrote: >>> On 16/06/2024 14:43, Javier Martinez Canillas wrote: Jocelyn Falempe writes: > The race condition between fbcon and drm_panic can only occurs if > VT_CONSOLE is set. So update drm_panic dependency accordingly. > This will make it easier for Linux distributions to enable drm_panic > by disabling VT_CONSOLE, and keeping fbcon terminal. > The only drawback is that fbcon won't display the boot kmsg, so it > should rely on userspace to do that. > At least plymouth already handle this case with > https://gitlab.freedesktop.org/plymouth/plymouth/-/merge_requests/224 > > Suggested-by: Daniel Vetter > Signed-off-by: Jocelyn Falempe > --- >drivers/gpu/drm/Kconfig | 2 +- >1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index a9df94291622..f5c989aed7e9 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -107,7 +107,7 @@ config DRM_KMS_HELPER > >config DRM_PANIC > bool "Display a user-friendly message when a kernel panic occurs" > -depends on DRM && !FRAMEBUFFER_CONSOLE > +depends on DRM && !(FRAMEBUFFER_CONSOLE && VT_CONSOLE) I thought the idea was to only make it depend on !VT_CONSOLE, so that distros could also enable fbcon / VT but prevent the race condition to happen due the VT not being a system console for the kernel to print messages ? >>> >>> Yes, but when writing the patch, I thought that if you have VT_CONSOLE=y >>> and FRAMEBUFFER_CONSOLE=n, then there won't be any race condition, and >>> drm_panic can be enabled safely. >>> I don't know if that really matters, and if VT_CONSOLE has any usage >>> apart from fbcon. >> >> It is used for any kind of virtual terminal, so also for vgacon. > > Ah thanks, but I think vgacon is safe to use with drm_panic. > > The problem with fbcon, is that it can also uses the fbdev emulation > from the drm driver, and in this case, drm_panic will write to the same > framebuffer as fbcon during a panic, leading to corrupted output. > This can't occur with other graphical console, so !(FRAMEBUFFER_CONSOLE > && VT_CONSOLE) is probably good. > Yeah, I agre that !(FRAMEBUFFER_CONSOLE && VT_CONSOLE) as you have in your patch is what makes sense. !VT_CONSOLE alone as I argued in my first email isn't correct since as you pointed out, it shouldn't affect other consoles besides fbcon. So please feel free to also add: Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
[PATCH] drm/nouveau: Use kmemdup_array() instead of kmemdup()
Use kmemdup_array() because we're allocating an array.
The main difference between kmemdup() and kmemdup_array() is that the
kmemdup_array() function has integer overflow checking built it. The
"args->in_sync.count" variable is a u32 so integer overflows would only
be a concern on 32bit systems. Fortunately, however, the u_memcpya()
function has integer overflow checking which means that it is not an
issue.
Still using kmemdup_array() is more appropriate and makes auditing the
code easier.
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/nouveau/nouveau_sched.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_sched.c
b/drivers/gpu/drm/nouveau/nouveau_sched.c
index 32fa2e273965..53d8b0584a56 100644
--- a/drivers/gpu/drm/nouveau/nouveau_sched.c
+++ b/drivers/gpu/drm/nouveau/nouveau_sched.c
@@ -45,10 +45,10 @@ nouveau_job_init(struct nouveau_job *job,
if (job->sync)
return -EINVAL;
- job->in_sync.data = kmemdup(args->in_sync.s,
-sizeof(*args->in_sync.s) *
-args->in_sync.count,
-GFP_KERNEL);
+ job->in_sync.data = kmemdup_array(args->in_sync.s,
+ args->in_sync.count,
+ sizeof(*args->in_sync.s),
+ GFP_KERNEL);
if (!job->in_sync.data)
return -ENOMEM;
}
@@ -60,10 +60,10 @@ nouveau_job_init(struct nouveau_job *job,
goto err_free_in_sync;
}
- job->out_sync.data = kmemdup(args->out_sync.s,
- sizeof(*args->out_sync.s) *
- args->out_sync.count,
- GFP_KERNEL);
+ job->out_sync.data = kmemdup_array(args->out_sync.s,
+ args->out_sync.count,
+ sizeof(*args->out_sync.s),
+ GFP_KERNEL);
if (!job->out_sync.data) {
ret = -ENOMEM;
goto err_free_in_sync;
--
2.43.0
Re: [PATCH v6,14/24] media: mediatek: vcodec: Add capture format to support one plane memory
Hi Andrzej,
Thanks for your help to review this patch.
On Wed, 2024-05-22 at 14:26 +0200, Andrzej Pietrasiewicz wrote:
> Hi Yunfei,
>
> W dniu 16.05.2024 o 14:20, Yunfei Dong pisze:
> > Define one uncompressed capture format V4L2_PIX_FMT_MS21 in order
> > to
> > support one plane memory. The buffer size is luma + chroma, luma is
> > stored at the start and chrome is stored at the end.
> >
> > Signed-off-by: Yunfei Dong
> > ---
> > Documentation/userspace-api/media/v4l/pixfmt-reserved.rst | 8
> >
> > drivers/media/v4l2-core/v4l2-common.c | 2 ++
> > drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
> > include/uapi/linux/videodev2.h| 1 +
> > 4 files changed, 12 insertions(+)
> >
> > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-
> > reserved.rst b/Documentation/userspace-api/media/v4l/pixfmt-
> > reserved.rst
> > index 886ba7b08d6b..6ec899649d50 100644
> > --- a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst
> > +++ b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst
> > @@ -295,6 +295,14 @@ please make a proposal on the linux-media
> > mailing list.
> > - Compressed format used by Nuvoton NPCM video driver. This
> > format is
> > defined in Remote Framebuffer Protocol (RFC 6143, chapter
> > 7.7.4 Hextile
> > Encoding).
> > +* .. _V4L2-PIX-FMT-MS21:
> > +
> > + - ``V4L2_PIX_FMT_MS21``
> > + - 'MS21'
> > + - This format has one plane, luma and chroma are stored in a
> > contiguous
>
> Maybe s/one/single ?
>
> > +memory. Luma pixel in 16x32 tiles at the start, chroma
> > pixel in 16x16
>
> maybe the word "pixel" is reduntant here? What else than pixels could
> tile sizes mean?
> Any padding between luma and chroma?
>
The secure memory only has one plane: Luma data + Chrome data.
Luma data is in order of 16x32 block, chrome is in order of 16x16
block. Width and height is aligned with fixed value.
I need to re-write the description again.
Best Regards,
Yunfei Dong
> > +tiles at the end. The image height must be aligned with 32
> > and the image
> > +width must be aligned with 16.
>
> Maybe aligned to?
>
> > .. raw:: latex
> >
> > \normalsize
> > diff --git a/drivers/media/v4l2-core/v4l2-common.c
> > b/drivers/media/v4l2-core/v4l2-common.c
> > index 4165c815faef..5ae54cf48dc7 100644
> > --- a/drivers/media/v4l2-core/v4l2-common.c
> > +++ b/drivers/media/v4l2-core/v4l2-common.c
> > @@ -271,6 +271,8 @@ const struct v4l2_format_info
> > *v4l2_format_info(u32 format)
> > .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0
> > }},
> > { .format = V4L2_PIX_FMT_MT2110R, .pixel_enc =
> > V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 5,
> > 10, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 2, .vdiv = 2,
> > .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0
> > }},
> > + { .format = V4L2_PIX_FMT_MS21, pixel_enc =
> > V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1,
> > 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2,
> > + .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0
> > }},
> >
> > /* YUV planar formats */
> > { .format = V4L2_PIX_FMT_NV12,.pixel_enc =
> > V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1,
> > 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> > b/drivers/media/v4l2-core/v4l2-ioctl.c
> > index 4c76d17b4629..3a68f2b9e7a4 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -1529,6 +1529,7 @@ static void v4l_fill_fmtdesc(struct
> > v4l2_fmtdesc *fmt)
> > case V4L2_PIX_FMT_MT2110T: descr = "Mediatek 10bit
> > Tile Mode"; break;
> > case V4L2_PIX_FMT_MT2110R: descr = "Mediatek 10bit
> > Raster Mode"; break;
> > case V4L2_PIX_FMT_HEXTILE: descr = "Hextile Compressed
> > Format"; break;
> > + case V4L2_PIX_FMT_MS21: descr = "MediaTek
> > One Plane Format"; break;
>
> s/One/Single ?
>
> Regards,
>
> Andrzej
>
> > default:
> > if (fmt->description[0])
> > return;
> > diff --git a/include/uapi/linux/videodev2.h
> > b/include/uapi/linux/videodev2.h
> > index 89eb1a3c6555..7aff2f2c8f9c 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -800,6 +800,7 @@ struct v4l2_pix_format {
> > #define V4L2_PIX_FMT_MM21 v4l2_fourcc('M', 'M', '2', '1') /*
> > Mediatek 8-bit block mode, two non-contiguous planes */
> > #define V4L2_PIX_FMT_MT2110T v4l2_fourcc('M', 'T', '2', 'T') /*
> > Mediatek 10-bit block tile mode */
> > #define V4L2_PIX_FMT_MT2110R v4l2_fourcc('M', 'T', '2', 'R') /*
> > Mediatek 10-bit block raster mode */
> > +#define V4L2_PIX_FMT_MS
Re: [PATCH RESEND] drm/display: Drop obsolete dependency on COMPILE_TEST
On Mon, Jun 17, 2024 at 10:30:30AM GMT, Jean Delvare wrote:
> Since commit 0166dc11be91 ("of: make CONFIG_OF user selectable"), it
> is possible to test-build any driver which depends on OF on any
> architecture by explicitly selecting OF. Therefore depending on
> COMPILE_TEST as an alternative is no longer needed.
The goal of this clause is to allow build-testing the driver with OF
being disabled (meaning that some of OF functions are stubbed and some
might disappear). I don't see how user-selectable OF provides the same
result.
>
> Signed-off-by: Jean Delvare
> Reviewed-by: Javier Martinez Canillas
> Cc: David Airlie
> Cc: Daniel Vetter
> ---
> Already sent on: 2022-11-21, 2023-01-27, 2023-12-02
>
> This is one of the only 3 remaining occurrences of this deprecated
> construct.
>
> Who can pick this patch and get it upstream?
>
> drivers/gpu/drm/display/Kconfig |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- linux-6.6.orig/drivers/gpu/drm/display/Kconfig
> +++ linux-6.6/drivers/gpu/drm/display/Kconfig
> @@ -3,7 +3,7 @@
> config DRM_DP_AUX_BUS
> tristate
> depends on DRM
> - depends on OF || COMPILE_TEST
> + depends on OF
>
> config DRM_DISPLAY_HELPER
> tristate
>
>
> --
> Jean Delvare
> SUSE L3 Support
--
With best wishes
Dmitry
Re: [PATCH RESEND] drm/logicvc: Drop obsolete dependency on COMPILE_TEST
On Mon, Jun 17, 2024 at 10:33:36AM GMT, Jean Delvare wrote:
> Since commit 0166dc11be91 ("of: make CONFIG_OF user selectable"), it
> is possible to test-build any driver which depends on OF on any
> architecture by explicitly selecting OF. Therefore depending on
> COMPILE_TEST as an alternative is no longer needed.
>
> Signed-off-by: Jean Delvare
> Reviewed-by: Paul Kocialkowski
> Cc: David Airlie
> Cc: Daniel Vetter
> ---
> Already sent on: 2022-11-21, 2023-01-27, 2023-12-02
>
> This is one of the only 3 remaining occurrences of this deprecated
> construct.
The same comment as the one for your drm/display patch.
>
> Who can pick this patch and get it upstream?
>
> drivers/gpu/drm/logicvc/Kconfig |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- linux-6.6.orig/drivers/gpu/drm/logicvc/Kconfig
> +++ linux-6.6/drivers/gpu/drm/logicvc/Kconfig
> @@ -1,7 +1,7 @@
> config DRM_LOGICVC
> tristate "LogiCVC DRM"
> depends on DRM
> - depends on OF || COMPILE_TEST
> + depends on OF
> select DRM_KMS_HELPER
> select DRM_KMS_DMA_HELPER
> select DRM_GEM_DMA_HELPER
>
>
> --
> Jean Delvare
> SUSE L3 Support
--
With best wishes
Dmitry
Re: [PATCH v2 7/7] drm/panic: Add support for drawing a monochrome graphical logo
On 13/06/2024 21:18, Geert Uytterhoeven wrote:
Re-use the existing support for boot-up logos to draw a monochrome
graphical logo in the DRM panic handler. When no suitable graphical
logo is available, the code falls back to the ASCII art penguin logo.
Note that all graphical boot-up logos are freed during late kernel
initialization, hence a copy must be made for later use.
Signed-off-by: Geert Uytterhoeven
---
v2:
- Rebased,
- Inline trivial draw_logo_mono().
---
drivers/gpu/drm/drm_panic.c | 65 +
drivers/video/logo/Kconfig | 2 ++
2 files changed, 60 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index f7e22b69bb25d3be..af30f243b2802ad7 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -7,11 +7,15 @@
*/
#include
+#include
#include
#include
#include
+#include
#include
+#include
#include
+#include
#include
#include
@@ -88,6 +92,42 @@ static const struct drm_panic_line logo_ascii[] = {
PANIC_LINE(" \\___)=(___/"),
};
+#ifdef CONFIG_LOGO
+static const struct linux_logo *logo_mono;
+
+static int drm_panic_setup_logo(void)
+{
+ const struct linux_logo *logo = fb_find_logo(1);
+ const unsigned char *logo_data;
+ struct linux_logo *logo_dup;
+
+ if (!logo || logo->type != LINUX_LOGO_MONO)
+ return 0;
+
+ /* The logo is __init, so we must make a copy for later use */
+ logo_data = kmemdup(logo->data,
+ size_mul(DIV_ROUND_UP(logo->width, BITS_PER_BYTE),
logo->height),
+ GFP_KERNEL);
+ if (!logo_data)
+ return -ENOMEM;
+
+ logo_dup = kmemdup(logo, sizeof(*logo), GFP_KERNEL);
+ if (!logo_dup) {
+ kfree(logo_data);
+ return -ENOMEM;
+ }
+
+ logo_dup->data = logo_data;
+ logo_mono = logo_dup;
+
+ return 0;
+}
+
+device_initcall(drm_panic_setup_logo);
+#else
+#define logo_mono ((const struct linux_logo *)NULL)
+#endif
I didn't notice that the first time, but the core drm can be built as a
module.
That means this will leak memory each time the module is removed.
But to solve the circular dependency between drm_kms_helper and
drm_panic, one solution is to depends on drm core to be built-in.
In this case there won't be a leak.
So depending on how we solve the circular dependency, it can be acceptable.
--
Jocelyn
Re: [PATCH 1/2] drm/vmwgfx: Fix missing HYPERVISOR_GUEST dependency
On Mon, Jun 17, 2024 at 11:07:09AM +0200, Borislav Petkov wrote: > On Sat, Jun 15, 2024 at 06:25:10PM -0700, Alexey Makhalov wrote: > > VMWARE_HYPERCALL alternative will not work as intended without > > VMware guest code initialization. > > > > Reported-by: kernel test robot > > Closes: > > https://lore.kernel.org/oe-kbuild-all/202406152104.fxakp1mb-...@intel.com/ > > Signed-off-by: Alexey Makhalov > > --- > > drivers/gpu/drm/vmwgfx/Kconfig | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/vmwgfx/Kconfig b/drivers/gpu/drm/vmwgfx/Kconfig > > index faddae3d6ac2..6f1ac940cbae 100644 > > --- a/drivers/gpu/drm/vmwgfx/Kconfig > > +++ b/drivers/gpu/drm/vmwgfx/Kconfig > > @@ -2,7 +2,7 @@ > > config DRM_VMWGFX > > tristate "DRM driver for VMware Virtual GPU" > > depends on DRM && PCI && MMU > > - depends on X86 || ARM64 > > + depends on (X86 && HYPERVISOR_GUEST) || ARM64 > > select DRM_TTM > > select DRM_TTM_HELPER > > select MAPPING_DIRTY_HELPERS > > -- > > Right, I'll queue this soon but it doesn't reproduce here with gcc-11 or > gcc-13. > This must be something gcc-9 specific or so... Actually, that's a DRM patch. Folks in To: ok to carry this though the tip tree? Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
[PATCH v3 01/10] drm/bridge: cdns-dsi: Fix OF node pointer
Fix the OF node pointer passed to the of_drm_find_bridge() call to find
the next bridge in the display chain.
To find the next bridge in the pipeline, we need to pass "np" - the OF
node pointer of the next entity in the devicetree chain. Passing
"of_node" to of_drm_find_bridge will make the function try to fetch the
bridge for the cdns-dsi which is not what's required.
Fix that.
Fixes: e19233955d9e ("drm/bridge: Add Cadence DSI driver")
Signed-off-by: Aradhya Bhatia
---
drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
index 7457d38622b0..b016f2ba06bb 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
@@ -952,7 +952,7 @@ static int cdns_dsi_attach(struct mipi_dsi_host *host,
bridge = drm_panel_bridge_add_typed(panel,
DRM_MODE_CONNECTOR_DSI);
} else {
- bridge = of_drm_find_bridge(dev->dev.of_node);
+ bridge = of_drm_find_bridge(np);
if (!bridge)
bridge = ERR_PTR(-EINVAL);
}
--
2.34.1
[PATCH v3 00/10] drm/bridge: cdns-dsi: Fix the color-shift issue
Hello all, This series provides some crucial fixes and improvements for the Cadence's DSI TX (cdns-dsi) controller found commonly in Texas Instruments' J7 family of SoCs as well as in AM62P. Along with that, this series aims to fix the color-shift issue that has been going on with the DSI controller. This controller requires to be enabled before the previous entity enables its stream[0]. It's a strict requirement which, if not followed, causes the colors to "shift" on the display. The fix happens in 2 steps. 1. The bridge pre_enable calls have been shifted before the crtc_enable and the bridge post_disable calls have been shifted after the crtc_disable. This has been done as per the definition of bridge pre_enable. "The display pipe (i.e. clocks and timing signals) feeding this bridge will not yet be running when this callback is called". Since CRTC is also a source feeding the bridge, it should not be enabled before the bridges in the pipeline are pre_enabled. The sequence of enable after this patch will look like: bridge[n]_pre_enable ... bridge[1]_pre_enable crtc_enable encoder_enable bridge[1]_enable ... bridge[n]_enable and vice-versa for the bridge chain disable sequence. 2. The cdns-dsi enable / disable sequences have now been moved to pre_enable and post_disable sequences. This is the only way to have cdns-dsi drivers be up and ready before the previous entity is enables its streaming. The DSI also spec requires the Clock and Data Lanes be ready before the DSI TX enables its stream[0]. A patch has been added to make the code wait for that to happen. Going ahead with further DSI (and DSS configuration), while the lanes are not ready, has been found to be another reason for shift in colors. All these patches have been tested on TI's vendor tree kernel with more devices, but for the mainline, these patches have been tested with J721E based BeagleboneAI64 along with a RaspberryPi 7" DSI panel. The extra patches can be found in the "next_dsi-v2-tests" branch of my github fork[1] for anyone who would like to test them. Thanks, Aradhya [0]: Section 12.6.5.7.3: "Start-up Procedure" [For DSI TX controller] in TDA4VM Technical Reference Manual https://www.ti.com/lit/zip/spruil1 [1]: https://github.com/aradhya07/linux-ab/tree/next_dsi-v3-finals-test Change Log: - Changes in v3: - Reword the commit message for patch "drm/bridge: cdns-dsi: Fix OF node pointer". - Add a new helper API to figure out DSI host input pixel format in patch "drm/mipi-dsi: Add helper to find input format". - Use a common function for bridge pre-enable and enable, and bridge disable and post-disable, to avoid code duplication. - Add T-b tag from Dominik Haller in patch 5/10. (Missed to add it in v2). - Add R-b tag from Dmitry Baryshkov for patch 8/10. - Changes in v2: - Drop patch "drm/tidss: Add CRTC mode_fixup" - Split patch "drm/bridge: cdns-dsi: Fix minor bugs" into 4 separate ones - Drop support for early_enable/late_disable APIs and instead re-order the pre_enable / post_disable APIs to be called before / after crtc_enable / crtc_disable. - Drop support for early_enable/late_disable in cdns-dsi and use pre_enable/post_disable APIs instead to do bridge enable/disable. Previous versions: v1: https://lore.kernel.org/all/20240511153051.1355825-1-a-bhat...@ti.com/ v2: https://lore.kernel.org/all/20240530093621.1925863-1-a-bhat...@ti.com/ Aradhya Bhatia (10): drm/bridge: cdns-dsi: Fix OF node pointer drm/bridge: cdns-dsi: Fix the phy_initialized variable drm/bridge: cdns-dsi: Fix the link and phy init order drm/bridge: cdns-dsi: Fix the clock variable for mode_valid() drm/bridge: cdns-dsi: Wait for Clk and Data Lanes to be ready drm/bridge: cdns-dsi: Reset the DCS write FIFO drm/mipi-dsi: Add helper to find input format drm/bridge: cdns-dsi: Support atomic bridge APIs drm/atomic-helper: Re-order bridge chain pre-enable and post-disable drm/bridge: cdns-dsi: Use pre_enable/post_disable to enable/disable .../gpu/drm/bridge/cadence/cdns-dsi-core.c| 72 +--- drivers/gpu/drm/drm_atomic_helper.c | 165 -- drivers/gpu/drm/drm_mipi_dsi.c| 37 include/drm/drm_atomic_helper.h | 7 + include/drm/drm_mipi_dsi.h| 1 + 5 files changed, 201 insertions(+), 81 deletions(-) base-commit: 6906a84c482f098d31486df8dc98cead21cce2d0 -- 2.34.1
[PATCH v3 05/10] drm/bridge: cdns-dsi: Wait for Clk and Data Lanes to be ready
Once the DSI Link and DSI Phy are initialized, the code needs to wait
for Clk and Data Lanes to be ready, before continuing configuration.
This is in accordance with the DSI Start-up procedure, found in the
Technical Reference Manual of Texas Instrument's J721E SoC[0] which
houses this DSI TX controller.
If the previous bridge (or crtc/encoder) are configured pre-maturely,
the input signal FIFO gets corrupt. This introduces a color-shift on the
display.
Allow the driver to wait for the clk and data lanes to get ready during
DSI enable.
[0]: See section 12.6.5.7.3 "Start-up Procedure" in J721E SoC TRM
TRM Link: http://www.ti.com/lit/pdf/spruil1
Fixes: e19233955d9e ("drm/bridge: Add Cadence DSI driver")
Tested-by: Dominik Haller
Signed-off-by: Aradhya Bhatia
---
drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
index 557b037bbc67..05d2f4cc50da 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
@@ -761,7 +761,7 @@ static void cdns_dsi_bridge_enable(struct drm_bridge
*bridge)
struct phy_configure_opts_mipi_dphy *phy_cfg =
&output->phy_opts.mipi_dphy;
unsigned long tx_byte_period;
struct cdns_dsi_cfg dsi_cfg;
- u32 tmp, reg_wakeup, div;
+ u32 tmp, reg_wakeup, div, status;
int nlanes;
if (WARN_ON(pm_runtime_get_sync(dsi->base.dev) < 0))
@@ -778,6 +778,17 @@ static void cdns_dsi_bridge_enable(struct drm_bridge
*bridge)
cdns_dsi_init_link(dsi);
cdns_dsi_hs_init(dsi);
+ /*
+* Now that the DSI Link and DSI Phy are initialized,
+* wait for the CLK and Data Lanes to be ready.
+*/
+ tmp = CLK_LANE_RDY;
+ for (int i = 0; i < nlanes; i++)
+ tmp |= DATA_LANE_RDY(i);
+
+ WARN_ON_ONCE(readl_poll_timeout(dsi->regs + MCTL_MAIN_STS, status,
+ status & tmp, 100, 0));
+
writel(HBP_LEN(dsi_cfg.hbp) | HSA_LEN(dsi_cfg.hsa),
dsi->regs + VID_HSIZE1);
writel(HFP_LEN(dsi_cfg.hfp) | HACT_LEN(dsi_cfg.hact),
--
2.34.1
[PATCH v3 03/10] drm/bridge: cdns-dsi: Fix the link and phy init order
The order of init of DSI link and DSI phy is wrong. The DSI link needs
to be configured before the DSI phy is getting configured. Otherwise,
the D-Phy is unable to lock in on the incoming PLL Reference clock[0].
Fix the order of inits.
[0]: See section 12.6.5.7.3 "Start-up Procedure" in J721E SoC TRM
TRM Link: http://www.ti.com/lit/pdf/spruil1
Fixes: fced5a364dee ("drm/bridge: cdns: Convert to phy framework")
Signed-off-by: Aradhya Bhatia
---
drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
index 42565e253b2d..371a3453970c 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
@@ -775,8 +775,8 @@ static void cdns_dsi_bridge_enable(struct drm_bridge
*bridge)
WARN_ON_ONCE(cdns_dsi_check_conf(dsi, mode, &dsi_cfg, false));
- cdns_dsi_hs_init(dsi);
cdns_dsi_init_link(dsi);
+ cdns_dsi_hs_init(dsi);
writel(HBP_LEN(dsi_cfg.hbp) | HSA_LEN(dsi_cfg.hsa),
dsi->regs + VID_HSIZE1);
--
2.34.1
[PATCH v3 02/10] drm/bridge: cdns-dsi: Fix the phy_initialized variable
Update the Phy initialized state to "not initialized" when the driver
(and the hardware by extension) gets suspended. This will allow the Phy
to get initialized again after resume.
Fixes: e19233955d9e ("drm/bridge: Add Cadence DSI driver")
Signed-off-by: Aradhya Bhatia
---
drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
index b016f2ba06bb..42565e253b2d 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
@@ -1153,6 +1153,7 @@ static int __maybe_unused cdns_dsi_suspend(struct device
*dev)
clk_disable_unprepare(dsi->dsi_p_clk);
reset_control_assert(dsi->dsi_p_rst);
dsi->link_initialized = false;
+ dsi->phy_initialized = false;
return 0;
}
--
2.34.1
[PATCH v3 04/10] drm/bridge: cdns-dsi: Fix the clock variable for mode_valid()
Allow the D-Phy config checks to use mode->clock instead of
mode->crtc_clock during mode_valid checks, like everywhere else in the
driver.
Fixes: fced5a364dee ("drm/bridge: cdns: Convert to phy framework")
Signed-off-by: Aradhya Bhatia
---
drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
index 371a3453970c..557b037bbc67 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
@@ -574,7 +574,7 @@ static int cdns_dsi_check_conf(struct cdns_dsi *dsi,
if (ret)
return ret;
- phy_mipi_dphy_get_default_config(mode->crtc_clock * 1000,
+ phy_mipi_dphy_get_default_config((mode_valid_check ? mode->clock :
mode->crtc_clock) * 1000,
mipi_dsi_pixel_format_to_bpp(output->dev->format),
nlanes, phy_cfg);
--
2.34.1
[PATCH v3 09/10] drm/atomic-helper: Re-order bridge chain pre-enable and post-disable
Move the bridge pre_enable call before crtc enable, and the bridge
post_disable call after the crtc disable.
The sequence of enable after this patch will look like:
bridge[n]_pre_enable
...
bridge[1]_pre_enable
crtc_enable
encoder_enable
bridge[1]_enable
...
bridge[n]__enable
and vice-versa for the bridge chain disable sequence.
The definition of bridge pre_enable hook says that,
"The display pipe (i.e. clocks and timing signals) feeding this bridge
will not yet be running when this callback is called".
Since CRTC is also a source feeding the bridge, it should not be enabled
before the bridges in the pipeline are pre_enabled. Fix that by
re-ordering the sequence of bridge pre_enable and bridge post_disable.
Signed-off-by: Aradhya Bhatia
---
drivers/gpu/drm/drm_atomic_helper.c | 165 ++--
include/drm/drm_atomic_helper.h | 7 ++
2 files changed, 114 insertions(+), 58 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index fb97b51b38f1..e8ad08634f58 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -74,6 +74,7 @@
* also shares the &struct drm_plane_helper_funcs function table with the plane
* helpers.
*/
+
static void
drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
struct drm_plane_state *old_plane_state,
@@ -1122,11 +1123,11 @@ crtc_needs_disable(struct drm_crtc_state *old_state,
}
static void
-disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
+disable_encoder_brige_chain(struct drm_device *dev, struct drm_atomic_state
*old_state,
+ enum bridge_chain_operation_type op_type)
{
struct drm_connector *connector;
struct drm_connector_state *old_conn_state, *new_conn_state;
- struct drm_crtc *crtc;
struct drm_crtc_state *old_crtc_state, *new_crtc_state;
int i;
@@ -1163,32 +1164,55 @@ disable_outputs(struct drm_device *dev, struct
drm_atomic_state *old_state)
if (WARN_ON(!encoder))
continue;
- funcs = encoder->helper_private;
-
- drm_dbg_atomic(dev, "disabling [ENCODER:%d:%s]\n",
- encoder->base.id, encoder->name);
-
/*
* Each encoder has at most one connector (since we always steal
* it away), so we won't call disable hooks twice.
*/
bridge = drm_bridge_chain_get_first_bridge(encoder);
- drm_atomic_bridge_chain_disable(bridge, old_state);
- /* Right function depends upon target state. */
- if (funcs) {
- if (funcs->atomic_disable)
- funcs->atomic_disable(encoder, old_state);
- else if (new_conn_state->crtc && funcs->prepare)
- funcs->prepare(encoder);
- else if (funcs->disable)
- funcs->disable(encoder);
- else if (funcs->dpms)
- funcs->dpms(encoder, DRM_MODE_DPMS_OFF);
- }
+ switch (op_type) {
+ case DRM_ENCODER_BRIDGE_DISABLE:
+ funcs = encoder->helper_private;
+
+ drm_dbg_atomic(dev, "disabling [ENCODER:%d:%s]\n",
+ encoder->base.id, encoder->name);
+
+ drm_atomic_bridge_chain_disable(bridge, old_state);
+
+ /* Right function depends upon target state. */
+ if (funcs) {
+ if (funcs->atomic_disable)
+ funcs->atomic_disable(encoder,
old_state);
+ else if (new_conn_state->crtc && funcs->prepare)
+ funcs->prepare(encoder);
+ else if (funcs->disable)
+ funcs->disable(encoder);
+ else if (funcs->dpms)
+ funcs->dpms(encoder, DRM_MODE_DPMS_OFF);
+ }
+
+ break;
+
+ case DRM_BRIDGE_POST_DISABLE:
+ drm_atomic_bridge_chain_post_disable(bridge, old_state);
- drm_atomic_bridge_chain_post_disable(bridge, old_state);
+ break;
+
+ default:
+ drm_err(dev, "Unrecognized Encoder/Bridge Operation
(%d).\n", op_type);
+ break;
+ }
}
+}
+
+static void
+disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
+{
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *old_crtc_state, *new_c
[PATCH v3 08/10] drm/bridge: cdns-dsi: Support atomic bridge APIs
Change the existing (and deprecated) bridge hooks, to the bridge
atomic APIs.
Add drm helpers for duplicate_state, destroy_state, and bridge_reset
bridge hooks.
Further add support for the input format negotiation hook.
Reviewed-by: Dmitry Baryshkov
Signed-off-by: Aradhya Bhatia
---
.../gpu/drm/bridge/cadence/cdns-dsi-core.c| 51 ---
1 file changed, 43 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
index 87fdd07ca0bc..acbd4007b38c 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
@@ -655,7 +655,8 @@ cdns_dsi_bridge_mode_valid(struct drm_bridge *bridge,
return MODE_OK;
}
-static void cdns_dsi_bridge_disable(struct drm_bridge *bridge)
+static void cdns_dsi_bridge_atomic_disable(struct drm_bridge *bridge,
+ struct drm_bridge_state
*old_bridge_state)
{
struct cdns_dsi_input *input = bridge_to_cdns_dsi_input(bridge);
struct cdns_dsi *dsi = input_to_dsi(input);
@@ -675,7 +676,8 @@ static void cdns_dsi_bridge_disable(struct drm_bridge
*bridge)
pm_runtime_put(dsi->base.dev);
}
-static void cdns_dsi_bridge_post_disable(struct drm_bridge *bridge)
+static void cdns_dsi_bridge_atomic_post_disable(struct drm_bridge *bridge,
+ struct drm_bridge_state
*old_bridge_state)
{
struct cdns_dsi_input *input = bridge_to_cdns_dsi_input(bridge);
struct cdns_dsi *dsi = input_to_dsi(input);
@@ -752,7 +754,8 @@ static void cdns_dsi_init_link(struct cdns_dsi *dsi)
dsi->link_initialized = true;
}
-static void cdns_dsi_bridge_enable(struct drm_bridge *bridge)
+static void cdns_dsi_bridge_atomic_enable(struct drm_bridge *bridge,
+ struct drm_bridge_state
*old_bridge_state)
{
struct cdns_dsi_input *input = bridge_to_cdns_dsi_input(bridge);
struct cdns_dsi *dsi = input_to_dsi(input);
@@ -903,7 +906,8 @@ static void cdns_dsi_bridge_enable(struct drm_bridge
*bridge)
writel(tmp, dsi->regs + MCTL_MAIN_EN);
}
-static void cdns_dsi_bridge_pre_enable(struct drm_bridge *bridge)
+static void cdns_dsi_bridge_atomic_pre_enable(struct drm_bridge *bridge,
+ struct drm_bridge_state
*old_bridge_state)
{
struct cdns_dsi_input *input = bridge_to_cdns_dsi_input(bridge);
struct cdns_dsi *dsi = input_to_dsi(input);
@@ -915,13 +919,44 @@ static void cdns_dsi_bridge_pre_enable(struct drm_bridge
*bridge)
cdns_dsi_hs_init(dsi);
}
+static u32 *cdns_dsi_bridge_get_input_bus_fmts(struct drm_bridge *bridge,
+ struct drm_bridge_state
*bridge_state,
+ struct drm_crtc_state
*crtc_state,
+ struct drm_connector_state
*conn_state,
+ u32 output_fmt,
+ unsigned int *num_input_fmts)
+{
+ struct cdns_dsi_input *input = bridge_to_cdns_dsi_input(bridge);
+ struct cdns_dsi *dsi = input_to_dsi(input);
+ struct cdns_dsi_output *output = &dsi->output;
+ u32 *input_fmts;
+
+ *num_input_fmts = 0;
+
+ input_fmts = kzalloc(sizeof(*input_fmts), GFP_KERNEL);
+ if (!input_fmts)
+ return NULL;
+
+ input_fmts[0] = drm_mipi_dsi_get_input_bus_fmt(output->dev->format);
+ if (!input_fmts[0])
+ return NULL;
+
+ *num_input_fmts = 1;
+
+ return input_fmts;
+}
+
static const struct drm_bridge_funcs cdns_dsi_bridge_funcs = {
.attach = cdns_dsi_bridge_attach,
.mode_valid = cdns_dsi_bridge_mode_valid,
- .disable = cdns_dsi_bridge_disable,
- .pre_enable = cdns_dsi_bridge_pre_enable,
- .enable = cdns_dsi_bridge_enable,
- .post_disable = cdns_dsi_bridge_post_disable,
+ .atomic_disable = cdns_dsi_bridge_atomic_disable,
+ .atomic_pre_enable = cdns_dsi_bridge_atomic_pre_enable,
+ .atomic_enable = cdns_dsi_bridge_atomic_enable,
+ .atomic_post_disable = cdns_dsi_bridge_atomic_post_disable,
+ .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+ .atomic_reset = drm_atomic_helper_bridge_reset,
+ .atomic_get_input_bus_fmts = cdns_dsi_bridge_get_input_bus_fmts,
};
static int cdns_dsi_attach(struct mipi_dsi_host *host,
--
2.34.1
[PATCH v3 10/10] drm/bridge: cdns-dsi: Use pre_enable/post_disable to enable/disable
The cdns-dsi controller requires that it be turned on completely before
the input DPI's source has begun streaming[0]. Not having that, allows
for a small window before cdns-dsi enable and after cdns-dsi disable
where the previous entity (in this case tidss's videoport) to continue
streaming DPI video signals. This small window where cdns-dsi is
disabled but is still receiving signals causes the input FIFO of
cdns-dsi to get corrupted. This causes the colors to shift on the output
display. The colors can either shift by one color component (R->G, G->B,
B->R), or by two color components (R->B, G->R, B->G).
Since tidss's videoport starts streaming via crtc enable hooks, we need
cdns-dsi to be up and running before that. Now that the bridges are
pre_enabled before crtc is enabled, and post_disabled after crtc is
disabled, use the pre_enable and post_disable hooks to get cdns-dsi
ready and running before the tidss videoport to get pass the color shift
issues.
[0]: See section 12.6.5.7.3 "Start-up Procedure" in J721E SoC TRM
TRM Link: http://www.ti.com/lit/pdf/spruil1
Signed-off-by: Aradhya Bhatia
---
.../gpu/drm/bridge/cadence/cdns-dsi-core.c| 32 +++
1 file changed, 4 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
index acbd4007b38c..a92814c3e0d6 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
@@ -655,8 +655,8 @@ cdns_dsi_bridge_mode_valid(struct drm_bridge *bridge,
return MODE_OK;
}
-static void cdns_dsi_bridge_atomic_disable(struct drm_bridge *bridge,
- struct drm_bridge_state
*old_bridge_state)
+static void cdns_dsi_bridge_atomic_post_disable(struct drm_bridge *bridge,
+ struct drm_bridge_state
*old_bridge_state)
{
struct cdns_dsi_input *input = bridge_to_cdns_dsi_input(bridge);
struct cdns_dsi *dsi = input_to_dsi(input);
@@ -676,15 +676,6 @@ static void cdns_dsi_bridge_atomic_disable(struct
drm_bridge *bridge,
pm_runtime_put(dsi->base.dev);
}
-static void cdns_dsi_bridge_atomic_post_disable(struct drm_bridge *bridge,
- struct drm_bridge_state
*old_bridge_state)
-{
- struct cdns_dsi_input *input = bridge_to_cdns_dsi_input(bridge);
- struct cdns_dsi *dsi = input_to_dsi(input);
-
- pm_runtime_put(dsi->base.dev);
-}
-
static void cdns_dsi_hs_init(struct cdns_dsi *dsi)
{
struct cdns_dsi_output *output = &dsi->output;
@@ -754,8 +745,8 @@ static void cdns_dsi_init_link(struct cdns_dsi *dsi)
dsi->link_initialized = true;
}
-static void cdns_dsi_bridge_atomic_enable(struct drm_bridge *bridge,
- struct drm_bridge_state
*old_bridge_state)
+static void cdns_dsi_bridge_atomic_pre_enable(struct drm_bridge *bridge,
+ struct drm_bridge_state
*old_bridge_state)
{
struct cdns_dsi_input *input = bridge_to_cdns_dsi_input(bridge);
struct cdns_dsi *dsi = input_to_dsi(input);
@@ -906,19 +897,6 @@ static void cdns_dsi_bridge_atomic_enable(struct
drm_bridge *bridge,
writel(tmp, dsi->regs + MCTL_MAIN_EN);
}
-static void cdns_dsi_bridge_atomic_pre_enable(struct drm_bridge *bridge,
- struct drm_bridge_state
*old_bridge_state)
-{
- struct cdns_dsi_input *input = bridge_to_cdns_dsi_input(bridge);
- struct cdns_dsi *dsi = input_to_dsi(input);
-
- if (WARN_ON(pm_runtime_get_sync(dsi->base.dev) < 0))
- return;
-
- cdns_dsi_init_link(dsi);
- cdns_dsi_hs_init(dsi);
-}
-
static u32 *cdns_dsi_bridge_get_input_bus_fmts(struct drm_bridge *bridge,
struct drm_bridge_state
*bridge_state,
struct drm_crtc_state
*crtc_state,
@@ -949,9 +927,7 @@ static u32 *cdns_dsi_bridge_get_input_bus_fmts(struct
drm_bridge *bridge,
static const struct drm_bridge_funcs cdns_dsi_bridge_funcs = {
.attach = cdns_dsi_bridge_attach,
.mode_valid = cdns_dsi_bridge_mode_valid,
- .atomic_disable = cdns_dsi_bridge_atomic_disable,
.atomic_pre_enable = cdns_dsi_bridge_atomic_pre_enable,
- .atomic_enable = cdns_dsi_bridge_atomic_enable,
.atomic_post_disable = cdns_dsi_bridge_atomic_post_disable,
.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
--
2.34.1
[PATCH v3 07/10] drm/mipi-dsi: Add helper to find input format
Add a helper API that can be used by the DSI hosts to find the required
input bus format for the given output dsi pixel format.
Signed-off-by: Aradhya Bhatia
---
drivers/gpu/drm/drm_mipi_dsi.c | 37 ++
include/drm/drm_mipi_dsi.h | 1 +
2 files changed, 38 insertions(+)
diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index 795001bb7ff1..70ca6678fec2 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -36,6 +36,8 @@
#include
#include
+#include
+
#include
/**
@@ -810,6 +812,41 @@ ssize_t mipi_dsi_generic_read(struct mipi_dsi_device *dsi,
const void *params,
}
EXPORT_SYMBOL(mipi_dsi_generic_read);
+/**
+ * drm_mipi_dsi_get_input_bus_fmt() - Get the required MEDIA_BUS_FMT_* based
+ * input pixel format for a given DSI output
+ * pixel format
+ * @dsi_format: pixel format that a DSI host needs to output
+ *
+ * Various DSI hosts can use this function during their
+ * &drm_bridge_funcs.atomic_get_input_bus_fmts operation to ascertain
+ * the MEDIA_BUS_FMT_* pixel format required as input.
+ *
+ * RETURNS:
+ * a 32-bit MEDIA_BUS_FMT_* value on success or 0 in case of failure.
+ */
+u32 drm_mipi_dsi_get_input_bus_fmt(enum mipi_dsi_pixel_format dsi_format)
+{
+ switch (dsi_format) {
+ case MIPI_DSI_FMT_RGB888:
+ return MEDIA_BUS_FMT_RGB888_1X24;
+
+ case MIPI_DSI_FMT_RGB666:
+ return MEDIA_BUS_FMT_RGB666_1X24_CPADHI;
+
+ case MIPI_DSI_FMT_RGB666_PACKED:
+ return MEDIA_BUS_FMT_RGB666_1X18;
+
+ case MIPI_DSI_FMT_RGB565:
+ return MEDIA_BUS_FMT_RGB565_1X16;
+
+ default:
+ /* Unsupported DSI Format */
+ return 0;
+ }
+}
+EXPORT_SYMBOL(drm_mipi_dsi_get_input_bus_fmt);
+
/**
* mipi_dsi_dcs_write_buffer() - transmit a DCS command with payload
* @dsi: DSI peripheral device
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 82b1cc434ea3..12ed7f51fe69 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -258,6 +258,7 @@ ssize_t mipi_dsi_generic_write(struct mipi_dsi_device *dsi,
const void *payload,
size_t size);
ssize_t mipi_dsi_generic_read(struct mipi_dsi_device *dsi, const void *params,
size_t num_params, void *data, size_t size);
+u32 drm_mipi_dsi_get_input_bus_fmt(enum mipi_dsi_pixel_format dsi_format);
/**
* enum mipi_dsi_dcs_tear_mode - Tearing Effect Output Line mode
--
2.34.1
[PATCH v3 06/10] drm/bridge: cdns-dsi: Reset the DCS write FIFO
Allow the DCS Write FIFO in the cdns-dsi controller to reset before any DCS packet is transmitted to the DSI sink device. The DCS FIFO reset is optional. Not all panels require it. But at least one of the DSI based panel that uses Ilitek ILI9881C (DSI to DPI bridge) doesn't work with without this reset. Signed-off-by: Aradhya Bhatia --- drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c index 05d2f4cc50da..87fdd07ca0bc 100644 --- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c +++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c @@ -1037,6 +1037,9 @@ static ssize_t cdns_dsi_transfer(struct mipi_dsi_host *host, cdns_dsi_init_link(dsi); + /* Reset the DCS Write FIFO */ + writel(0x00, dsi->regs + DIRECT_CMD_FIFO_RST); + ret = mipi_dsi_create_packet(&packet, msg); if (ret) goto out; -- 2.34.1
Re: [PATCH v6] drm/ci: add tests on vkms
On 14/06/2024 13:54, Helen Koike wrote:
On 14/06/2024 13:18, Vignesh Raman wrote:
Add job that runs igt on top of vkms.
Acked-by: Maíra Canal
Acked-by: Helen Koike
Signed-off-by: Vignesh Raman
Acked-by: Jessica Zhang
Tested-by: Jessica Zhang
Acked-by: Maxime Ripard
Signed-off-by: Helen Koike
---
v2:
- do not mv modules to /lib/modules in the job definition, leave it to
crosvm-runner.sh
v3:
- Enable CONFIG_DRM_VKMS in x86_64.config and update xfails
v4:
- Build vkms as module and test with latest IGT.
This patch depends on
https://lore.kernel.org/dri-devel/20240130150340.687871-1-vignesh.ra...@collabora.com/
v5:
- Test with the updated IGT and update xfails
v6:
- Add metadata header for each flake test. Update skips file.
https://gitlab.freedesktop.org/vigneshraman/linux/-/pipelines/1201477
Thanks!
If there are no more comments until monday I'm applying this.
Thanks all for reviewing and testing.
Regards,
Helen
Applied to drm-misc-next.
Thanks
Helen
---
MAINTAINERS | 1 +
drivers/gpu/drm/ci/build.sh | 1 -
drivers/gpu/drm/ci/gitlab-ci.yml | 1 +
drivers/gpu/drm/ci/igt_runner.sh | 6 +-
drivers/gpu/drm/ci/image-tags.yml | 2 +-
drivers/gpu/drm/ci/test.yml | 24 +++-
drivers/gpu/drm/ci/x86_64.config | 1 +
drivers/gpu/drm/ci/xfails/vkms-none-fails.txt | 57 +
.../gpu/drm/ci/xfails/vkms-none-flakes.txt | 69 ++
drivers/gpu/drm/ci/xfails/vkms-none-skips.txt | 119 ++
10 files changed, 275 insertions(+), 6 deletions(-)
create mode 100644 drivers/gpu/drm/ci/xfails/vkms-none-fails.txt
create mode 100644 drivers/gpu/drm/ci/xfails/vkms-none-flakes.txt
create mode 100644 drivers/gpu/drm/ci/xfails/vkms-none-skips.txt
diff --git a/MAINTAINERS b/MAINTAINERS
index 8aee861d18f9..94065f5028cf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7036,6 +7036,7 @@ L: dri-devel@lists.freedesktop.org
S: Maintained
T: git https://gitlab.freedesktop.org/drm/misc/kernel.git
F: Documentation/gpu/vkms.rst
+F: drivers/gpu/drm/ci/xfails/vkms*
F: drivers/gpu/drm/vkms/
DRM DRIVER FOR VIRTUALBOX VIRTUAL GPU
diff --git a/drivers/gpu/drm/ci/build.sh b/drivers/gpu/drm/ci/build.sh
index a67871fdcd3f..e938074ac8e7 100644
--- a/drivers/gpu/drm/ci/build.sh
+++ b/drivers/gpu/drm/ci/build.sh
@@ -157,7 +157,6 @@ fi
mkdir -p artifacts/install/lib
mv install/* artifacts/install/.
-rm -rf artifacts/install/modules
ln -s common artifacts/install/ci-common
cp .config artifacts/${CI_JOB_NAME}_config
diff --git a/drivers/gpu/drm/ci/gitlab-ci.yml
b/drivers/gpu/drm/ci/gitlab-ci.yml
index 1b29c3b6406b..80fb0f57ae46 100644
--- a/drivers/gpu/drm/ci/gitlab-ci.yml
+++ b/drivers/gpu/drm/ci/gitlab-ci.yml
@@ -123,6 +123,7 @@ stages:
- msm
- rockchip
- virtio-gpu
+ - software-driver
# YAML anchors for rule conditions
#
diff --git a/drivers/gpu/drm/ci/igt_runner.sh
b/drivers/gpu/drm/ci/igt_runner.sh
index d49ad434b580..79f41d7da772 100755
--- a/drivers/gpu/drm/ci/igt_runner.sh
+++ b/drivers/gpu/drm/ci/igt_runner.sh
@@ -30,10 +30,10 @@ case "$DRIVER_NAME" in
export IGT_FORCE_DRIVER="panfrost"
fi
;;
- amdgpu)
+ amdgpu|vkms)
# Cannot use HWCI_KERNEL_MODULES as at that point we don't
have the module in /lib
- mv /install/modules/lib/modules/* /lib/modules/.
- modprobe amdgpu
+ mv /install/modules/lib/modules/* /lib/modules/. || true
+ modprobe --first-time $DRIVER_NAME
;;
esac
diff --git a/drivers/gpu/drm/ci/image-tags.yml
b/drivers/gpu/drm/ci/image-tags.yml
index 60323ebc7304..13eda37bdf05 100644
--- a/drivers/gpu/drm/ci/image-tags.yml
+++ b/drivers/gpu/drm/ci/image-tags.yml
@@ -4,7 +4,7 @@ variables:
DEBIAN_BASE_TAG: "${CONTAINER_TAG}"
DEBIAN_X86_64_BUILD_IMAGE_PATH: "debian/x86_64_build"
- DEBIAN_BUILD_TAG: "2023-10-08-config"
+ DEBIAN_BUILD_TAG: "2024-06-10-vkms"
KERNEL_ROOTFS_TAG: "2023-10-06-amd"
diff --git a/drivers/gpu/drm/ci/test.yml b/drivers/gpu/drm/ci/test.yml
index 322cce714657..ee908b66aad2 100644
--- a/drivers/gpu/drm/ci/test.yml
+++ b/drivers/gpu/drm/ci/test.yml
@@ -338,7 +338,7 @@ meson:g12b:
RUNNER_TAG: mesa-ci-x86-64-lava-meson-g12b-a311d-khadas-vim3
virtio_gpu:none:
- stage: virtio-gpu
+ stage: software-driver
variables:
CROSVM_GALLIUM_DRIVER: llvmpipe
DRIVER_NAME: virtio_gpu
@@ -358,3 +358,25 @@ virtio_gpu:none:
- debian/x86_64_test-gl
- testing:x86_64
- igt:x86_64
+
+vkms:none:
+ stage: software-driver
+ variables:
+ DRIVER_NAME: vkms
+ GPU_VERSION: none
+ extends:
+ - .test-gl
+ - .test-rules
+ tags:
+ - kvm
+ script:
+ - ln -sf $CI_PROJECT_DIR/install /install
+ - mv install/bzImage /lava-files/bzImage
+ - mkdir -p /lib/modules
+ - mkdir -p $CI_PR
[PATCH v2] fbdev: vesafb: Detect VGA compatibility from screen info's VESA attributes
Test the vesa_attributes field in struct screen_info for compatibility
with VGA hardware. Vesafb currently tests bit 1 in screen_info's
capabilities field, It sets the framebuffer address size and is
unrelated to VGA.
Section 4.4 of the Vesa VBE 2.0 specifications defines that bit 5 in
the mode's attributes field signals VGA compatibility. The mode is
compatible with VGA hardware if the bit is clear. In that case, the
driver can access VGA state of the VBE's underlying hardware. The
vesafb driver uses this feature to program the color LUT in palette
modes. Without, colors might be incorrect.
The problem got introduced in commit 89ec4c238e7a ("[PATCH] vesafb: Fix
incorrect logo colors in x86_64"). It incorrectly stores the mode
attributes in the screen_info's capabilities field and updates vesafb
accordingly. Later, commit 5e8ddcbe8692 ("Video mode probing support for
the new x86 setup code") fixed the screen_info, but did not update vesafb.
Color output still tends to work, because bit 1 in capabilities is
usually 0.
Besides fixing the bug in vesafb, this commit introduces a helper that
reads the correct bit from screen_info.
v2:
- clarify comment on non-VGA modes (Helge)
Signed-off-by: Thomas Zimmermann
Fixes: 5e8ddcbe8692 ("Video mode probing support for the new x86 setup code")
Reviewed-by: Javier Martinez Canillas
Cc: # v2.6.23+
---
drivers/video/fbdev/vesafb.c | 2 +-
include/linux/screen_info.h | 10 ++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c
index 8ab64ae4cad3e..5a161750a3aee 100644
--- a/drivers/video/fbdev/vesafb.c
+++ b/drivers/video/fbdev/vesafb.c
@@ -271,7 +271,7 @@ static int vesafb_probe(struct platform_device *dev)
if (si->orig_video_isVGA != VIDEO_TYPE_VLFB)
return -ENODEV;
- vga_compat = (si->capabilities & 2) ? 0 : 1;
+ vga_compat = !__screen_info_vbe_mode_nonvga(si);
vesafb_fix.smem_start = si->lfb_base;
vesafb_defined.bits_per_pixel = si->lfb_depth;
if (15 == vesafb_defined.bits_per_pixel)
diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h
index 75303c126285a..d21f8e4e9f4a4 100644
--- a/include/linux/screen_info.h
+++ b/include/linux/screen_info.h
@@ -49,6 +49,16 @@ static inline u64 __screen_info_lfb_size(const struct
screen_info *si, unsigned
return lfb_size;
}
+static inline bool __screen_info_vbe_mode_nonvga(const struct screen_info *si)
+{
+ /*
+* VESA modes typically run on VGA hardware. Set bit 5 signal that this
+* is not the case. Drivers can then not make use of VGA resources. See
+* Sec 4.4 of the VBE 2.0 spec.
+*/
+ return si->vesa_attributes & BIT(5);
+}
+
static inline unsigned int __screen_info_video_type(unsigned int type)
{
switch (type) {
--
2.45.2
[PATCH 0/2] Fixes of AMD GFX7/8 hang on Loongson platforms
This patchset tries to fix some workaround code in amdgpu/radeon driver, that makes Loongson 3A+7A platform suffering from GPU crashes. Icenowy Zheng (2): drm/amdgpu: make duplicated EOP packet for GFX7/8 have real content drm/radeon: repeat the same EOP packet for EOP workaround on CIK drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 12 +--- drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 12 drivers/gpu/drm/radeon/cik.c | 7 ++- 3 files changed, 11 insertions(+), 20 deletions(-) -- 2.45.1
[PATCH 1/2] drm/amdgpu: make duplicated EOP packet for GFX7/8 have real content
The duplication of EOP packets for GFX7/8, with the former one have
seq-1 written and the latter one have seq written, seems to confuse some
hardware platform (e.g. Loongson 7A series PCIe controllers).
Make the content of the duplicated EOP packet the same with the real
one, only masking any possible interrupts.
Fixes: bf26da927a1c ("drm/amdgpu: add cache flush workaround to gfx8
emit_fence")
Fixes: a2e73f56fa62 ("drm/amdgpu: Add support for CIK parts")
Signed-off-by: Icenowy Zheng
---
drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 12 +---
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 12
2 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index 541dbd70d8c75..778f27f1a34fe 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -2117,9 +2117,8 @@ static void gfx_v7_0_ring_emit_fence_gfx(struct
amdgpu_ring *ring, u64 addr,
{
bool write64bit = flags & AMDGPU_FENCE_FLAG_64BIT;
bool int_sel = flags & AMDGPU_FENCE_FLAG_INT;
- /* Workaround for cache flush problems. First send a dummy EOP
-* event down the pipe with seq one below.
-*/
+
+ /* Workaround for cache flush problems, send EOP twice. */
amdgpu_ring_write(ring, PACKET3(PACKET3_EVENT_WRITE_EOP, 4));
amdgpu_ring_write(ring, (EOP_TCL1_ACTION_EN |
EOP_TC_ACTION_EN |
@@ -2127,11 +2126,10 @@ static void gfx_v7_0_ring_emit_fence_gfx(struct
amdgpu_ring *ring, u64 addr,
EVENT_INDEX(5)));
amdgpu_ring_write(ring, addr & 0xfffc);
amdgpu_ring_write(ring, (upper_32_bits(addr) & 0x) |
- DATA_SEL(1) | INT_SEL(0));
- amdgpu_ring_write(ring, lower_32_bits(seq - 1));
- amdgpu_ring_write(ring, upper_32_bits(seq - 1));
+ DATA_SEL(write64bit ? 2 : 1) | INT_SEL(0));
+ amdgpu_ring_write(ring, lower_32_bits(seq));
+ amdgpu_ring_write(ring, upper_32_bits(seq));
- /* Then send the real EOP event down the pipe. */
amdgpu_ring_write(ring, PACKET3(PACKET3_EVENT_WRITE_EOP, 4));
amdgpu_ring_write(ring, (EOP_TCL1_ACTION_EN |
EOP_TC_ACTION_EN |
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 2f0e72caee1af..39a7d60f1fd69 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -6153,9 +6153,7 @@ static void gfx_v8_0_ring_emit_fence_gfx(struct
amdgpu_ring *ring, u64 addr,
bool write64bit = flags & AMDGPU_FENCE_FLAG_64BIT;
bool int_sel = flags & AMDGPU_FENCE_FLAG_INT;
- /* Workaround for cache flush problems. First send a dummy EOP
-* event down the pipe with seq one below.
-*/
+ /* Workaround for cache flush problems, send EOP twice. */
amdgpu_ring_write(ring, PACKET3(PACKET3_EVENT_WRITE_EOP, 4));
amdgpu_ring_write(ring, (EOP_TCL1_ACTION_EN |
EOP_TC_ACTION_EN |
@@ -6164,12 +6162,10 @@ static void gfx_v8_0_ring_emit_fence_gfx(struct
amdgpu_ring *ring, u64 addr,
EVENT_INDEX(5)));
amdgpu_ring_write(ring, addr & 0xfffc);
amdgpu_ring_write(ring, (upper_32_bits(addr) & 0x) |
- DATA_SEL(1) | INT_SEL(0));
- amdgpu_ring_write(ring, lower_32_bits(seq - 1));
- amdgpu_ring_write(ring, upper_32_bits(seq - 1));
+ DATA_SEL(write64bit ? 2 : 1) | INT_SEL(0));
+ amdgpu_ring_write(ring, lower_32_bits(seq));
+ amdgpu_ring_write(ring, upper_32_bits(seq));
- /* Then send the real EOP event down the pipe:
-* EVENT_WRITE_EOP - flush caches, send int */
amdgpu_ring_write(ring, PACKET3(PACKET3_EVENT_WRITE_EOP, 4));
amdgpu_ring_write(ring, (EOP_TCL1_ACTION_EN |
EOP_TC_ACTION_EN |
--
2.45.1
[PATCH 2/2] drm/radeon: repeat the same EOP packet for EOP workaround on CIK
Ths first EOP packet with a sequence number as seq-1 seems to confuse
some PCIe hardware (e.g. Loongson 7A PCHs).
Use the real sequence number instead.
Fixes: a9c73a0e022c ("drm/radeon: workaround for CP HW bug on CIK")
Signed-off-by: Icenowy Zheng
---
drivers/gpu/drm/radeon/cik.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
index 8275eeba0b349..9d203054f922a 100644
--- a/drivers/gpu/drm/radeon/cik.c
+++ b/drivers/gpu/drm/radeon/cik.c
@@ -3543,9 +3543,7 @@ void cik_fence_gfx_ring_emit(struct radeon_device *rdev,
struct radeon_ring *ring = &rdev->ring[fence->ring];
u64 addr = rdev->fence_drv[fence->ring].gpu_addr;
- /* Workaround for cache flush problems. First send a dummy EOP
-* event down the pipe with seq one below.
-*/
+ /* Workaround for cache flush problems by sending the EOP event twice */
radeon_ring_write(ring, PACKET3(PACKET3_EVENT_WRITE_EOP, 4));
radeon_ring_write(ring, (EOP_TCL1_ACTION_EN |
EOP_TC_ACTION_EN |
@@ -3554,10 +3552,9 @@ void cik_fence_gfx_ring_emit(struct radeon_device *rdev,
radeon_ring_write(ring, addr & 0xfffc);
radeon_ring_write(ring, (upper_32_bits(addr) & 0x) |
DATA_SEL(1) | INT_SEL(0));
- radeon_ring_write(ring, fence->seq - 1);
+ radeon_ring_write(ring, fence->seq);
radeon_ring_write(ring, 0);
- /* Then send the real EOP event down the pipe. */
radeon_ring_write(ring, PACKET3(PACKET3_EVENT_WRITE_EOP, 4));
radeon_ring_write(ring, (EOP_TCL1_ACTION_EN |
EOP_TC_ACTION_EN |
--
2.45.1
Re: [PATCH 10/13] drm/mediatek: Fix XRGB setting error in Mixer
Il 16/06/24 10:29, Hsiao Chien Sung via B4 Relay ha scritto:
From: Hsiao Chien Sung
Although the alpha channel in XRGB formats can be ignored, ALPHA_CON
must be configured accordingly when using XRGB formats or it will still
affects CRC generation.
Fixes: d886c0009bd0 ("drm/mediatek: Add ETHDR support for MT8195")
Signed-off-by: Hsiao Chien Sung
Reviewed-by: CK Hu
Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH 11/13] drm/mediatek: Add new color format MACROs in OVL
Il 16/06/24 10:29, Hsiao Chien Sung via B4 Relay ha scritto: From: Hsiao Chien Sung Define new color formats to hide the bit operation in the MACROs to make the switch statement more concise. Change the MACROs to align the naming rule in DRM. Signed-off-by: Hsiao Chien Sung Reviewed-by: CK Hu Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH 12/13] drm/mediatek: Support DRM plane alpha in OVL
Il 16/06/24 10:29, Hsiao Chien Sung via B4 Relay ha scritto: From: Hsiao Chien Sung Set the plane alpha according to DRM plane property. Signed-off-by: Hsiao Chien Sung Reviewed-by: CK Hu Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH 13/13] drm/mediatek: Support DRM plane alpha in Mixer
Il 16/06/24 10:29, Hsiao Chien Sung via B4 Relay ha scritto: From: Hsiao Chien Sung Set the plane alpha according to DRM plane property. Signed-off-by: Hsiao Chien Sung Reviewed-by: CK Hu Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH 09/13] drm/mediatek: Fix XRGB setting error in OVL
Il 16/06/24 10:29, Hsiao Chien Sung via B4 Relay ha scritto:
From: Hsiao Chien Sung
CONST_BLD must be enabled for XRGB formats although the alpha channel
can be ignored, or OVL will still read the value from memory.
This error only affects CRC generation.
Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
Signed-off-by: Hsiao Chien Sung
Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH RESEND] drm/display: Drop obsolete dependency on COMPILE_TEST
Hi Dmitry,
Thanks for your feedback.
On Mon, 17 Jun 2024 12:57:19 +0300, Dmitry Baryshkov wrote:
> On Mon, Jun 17, 2024 at 10:30:30AM GMT, Jean Delvare wrote:
> > Since commit 0166dc11be91 ("of: make CONFIG_OF user selectable"), it
> > is possible to test-build any driver which depends on OF on any
> > architecture by explicitly selecting OF. Therefore depending on
> > COMPILE_TEST as an alternative is no longer needed.
>
> The goal of this clause is to allow build-testing the driver with OF
> being disabled (meaning that some of OF functions are stubbed and some
> might disappear). I don't see how user-selectable OF provides the same
> result.
Historically, the goal of this clause *was* to allow build-testing the
driver on architectures which did not support OF, and that did make
sense back then. As I understand it, building the driver without OF
support was never a goal per se (if it was, then the driver wouldn't be
set to depend on OF in the first place).
Some of my other submissions include the following explanation which
you might find useful (I ended up stripping it on resubmission after
being told I was being too verbose, but maybe it was needed after all):
It is actually better to always build such drivers with OF enabled,
so that the test builds are closer to how each driver will actually be
built on its intended target. Building them without OF may not test
much as the compiler will optimize out potentially large parts of the
code. In the worst case, this could even pop false positive warnings.
Dropping COMPILE_TEST here improves the quality of our testing and
avoids wasting time on non-existent issues.
--
Jean Delvare
SUSE L3 Support
Re: [PATCH v2] fbdev: vesafb: Detect VGA compatibility from screen info's VESA attributes
Am 17.06.24 um 13:06 schrieb Thomas Zimmermann:
Test the vesa_attributes field in struct screen_info for compatibility
with VGA hardware. Vesafb currently tests bit 1 in screen_info's
capabilities field, It sets the framebuffer address size and is
unrelated to VGA.
Section 4.4 of the Vesa VBE 2.0 specifications defines that bit 5 in
the mode's attributes field signals VGA compatibility. The mode is
compatible with VGA hardware if the bit is clear. In that case, the
driver can access VGA state of the VBE's underlying hardware. The
vesafb driver uses this feature to program the color LUT in palette
modes. Without, colors might be incorrect.
The problem got introduced in commit 89ec4c238e7a ("[PATCH] vesafb: Fix
incorrect logo colors in x86_64"). It incorrectly stores the mode
attributes in the screen_info's capabilities field and updates vesafb
accordingly. Later, commit 5e8ddcbe8692 ("Video mode probing support for
the new x86 setup code") fixed the screen_info, but did not update vesafb.
Color output still tends to work, because bit 1 in capabilities is
usually 0.
Besides fixing the bug in vesafb, this commit introduces a helper that
reads the correct bit from screen_info.
v2:
- clarify comment on non-VGA modes (Helge)
Signed-off-by: Thomas Zimmermann
Fixes: 5e8ddcbe8692 ("Video mode probing support for the new x86 setup code")
Reviewed-by: Javier Martinez Canillas
Cc: # v2.6.23+
---
drivers/video/fbdev/vesafb.c | 2 +-
include/linux/screen_info.h | 10 ++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c
index 8ab64ae4cad3e..5a161750a3aee 100644
--- a/drivers/video/fbdev/vesafb.c
+++ b/drivers/video/fbdev/vesafb.c
@@ -271,7 +271,7 @@ static int vesafb_probe(struct platform_device *dev)
if (si->orig_video_isVGA != VIDEO_TYPE_VLFB)
return -ENODEV;
- vga_compat = (si->capabilities & 2) ? 0 : 1;
+ vga_compat = !__screen_info_vbe_mode_nonvga(si);
vesafb_fix.smem_start = si->lfb_base;
vesafb_defined.bits_per_pixel = si->lfb_depth;
if (15 == vesafb_defined.bits_per_pixel)
diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h
index 75303c126285a..d21f8e4e9f4a4 100644
--- a/include/linux/screen_info.h
+++ b/include/linux/screen_info.h
@@ -49,6 +49,16 @@ static inline u64 __screen_info_lfb_size(const struct
screen_info *si, unsigned
return lfb_size;
}
+static inline bool __screen_info_vbe_mode_nonvga(const struct screen_info *si)
+{
+ /*
+* VESA modes typically run on VGA hardware. Set bit 5 signal that this
'signals'
+* is not the case. Drivers can then not make use of VGA resources. See
+* Sec 4.4 of the VBE 2.0 spec.
+*/
+ return si->vesa_attributes & BIT(5);
+}
+
static inline unsigned int __screen_info_video_type(unsigned int type)
{
switch (type) {
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
Re: [PATCH] drm/tidss: Add drm_panic support
On 15/06/2024 10:53, Javier Martinez Canillas wrote:
Add support for the drm_panic module, which displays a pretty user
friendly message on the screen when a Linux kernel panic occurs.
Thanks for enabling drm panic on another hardware.
It looks good to me.
Reviewed-by: Jocelyn Falempe
Signed-off-by: Javier Martinez Canillas
---
Tested on an AM625 BeaglePlay board by triggering a panic using the
`echo c > /proc/sysrq-trigger` command.
drivers/gpu/drm/tidss/tidss_plane.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tidss/tidss_plane.c
b/drivers/gpu/drm/tidss/tidss_plane.c
index 68fed531f6a7..a5d86822c9e3 100644
--- a/drivers/gpu/drm/tidss/tidss_plane.c
+++ b/drivers/gpu/drm/tidss/tidss_plane.c
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -166,6 +167,14 @@ static const struct drm_plane_helper_funcs
tidss_plane_helper_funcs = {
.atomic_disable = tidss_plane_atomic_disable,
};
+static const struct drm_plane_helper_funcs tidss_primary_plane_helper_funcs = {
+ .atomic_check = tidss_plane_atomic_check,
+ .atomic_update = tidss_plane_atomic_update,
+ .atomic_enable = tidss_plane_atomic_enable,
+ .atomic_disable = tidss_plane_atomic_disable,
+ .get_scanout_buffer = drm_fb_dma_get_scanout_buffer,
+};
+
static const struct drm_plane_funcs tidss_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
@@ -211,7 +220,10 @@ struct tidss_plane *tidss_plane_create(struct tidss_device
*tidss,
if (ret < 0)
goto err;
- drm_plane_helper_add(&tplane->plane, &tidss_plane_helper_funcs);
+ if (type == DRM_PLANE_TYPE_PRIMARY)
+ drm_plane_helper_add(&tplane->plane,
&tidss_primary_plane_helper_funcs);
+ else
+ drm_plane_helper_add(&tplane->plane, &tidss_plane_helper_funcs);
drm_plane_create_zpos_property(&tplane->plane, tidss->num_planes, 0,
num_planes - 1);
Re: [PATCH 01/15] net: hbl_cn: add habanalabs Core Network driver
On Mon, Jun 17, 2024 at 08:08:26AM +, Omer Shpigelman wrote: > On 6/13/24 16:01, Przemek Kitszel wrote: > > [Some people who received this message don't often get email from > > przemyslaw.kits...@intel.com. Learn why this is important at > > https://aka.ms/LearnAboutSenderIdentification ] > > > > On 6/13/24 10:21, Omer Shpigelman wrote: > >> Add the hbl_cn driver which will serve both Ethernet and InfiniBand > >> drivers. > >> hbl_cn is the layer which is used by the satellite drivers for many shared > >> operations that are needed by both EN and IB subsystems like QPs, CQs etc. > >> The CN driver is initialized via auxiliary bus by the habanalabs driver. > >> > >> Signed-off-by: Omer Shpigelman > >> Co-developed-by: Abhilash K V > >> Signed-off-by: Abhilash K V > >> Co-developed-by: Andrey Agranovich > >> Signed-off-by: Andrey Agranovich > >> Co-developed-by: Bharat Jauhari > >> Signed-off-by: Bharat Jauhari > >> Co-developed-by: David Meriin > >> Signed-off-by: David Meriin > >> Co-developed-by: Sagiv Ozeri > >> Signed-off-by: Sagiv Ozeri > >> Co-developed-by: Zvika Yehudai > >> Signed-off-by: Zvika Yehudai > >> --- > >> .../device_drivers/ethernet/index.rst |1 + > >> .../device_drivers/ethernet/intel/hbl.rst | 82 + > >> MAINTAINERS | 11 + > >> drivers/net/ethernet/intel/Kconfig| 20 + > >> drivers/net/ethernet/intel/Makefile |1 + > >> drivers/net/ethernet/intel/hbl_cn/Makefile|9 + > >> .../net/ethernet/intel/hbl_cn/common/Makefile |3 + > >> .../net/ethernet/intel/hbl_cn/common/hbl_cn.c | 5954 + > >> .../net/ethernet/intel/hbl_cn/common/hbl_cn.h | 1627 + > >> .../ethernet/intel/hbl_cn/common/hbl_cn_drv.c | 220 + > >> .../intel/hbl_cn/common/hbl_cn_memory.c | 40 + > >> .../ethernet/intel/hbl_cn/common/hbl_cn_phy.c | 33 + > >> .../ethernet/intel/hbl_cn/common/hbl_cn_qp.c | 13 + > >> include/linux/habanalabs/cpucp_if.h | 125 +- > >> include/linux/habanalabs/hl_boot_if.h |9 +- > >> include/linux/net/intel/cn.h | 474 ++ > >> include/linux/net/intel/cn_aux.h | 298 + > >> include/linux/net/intel/cni.h | 636 ++ > >> 18 files changed, 9545 insertions(+), 11 deletions(-) > > > > this is a very big patch, it asks for a split; what's worse, it's > > proportional to the size of this series: > > 146 files changed, 148514 insertions(+), 70 deletions(-) > > which is just too big > > > > [...] > > > > Yeah, well I'm limited to 15 patches per patch set according to the kernel > doc so I had to have this big patch. > Our changes are contained in 4 different drivers and all of the changes > should be merged together so the HW will be operational. > Hence I had to squeeze some code to a big patch. Submit your code in multiple steps. One driver at a time. Thanks
Re: [PATCH RESEND] drm/display: Drop obsolete dependency on COMPILE_TEST
On Mon, Jun 17, 2024 at 01:23:48PM GMT, Jean Delvare wrote:
> Hi Dmitry,
>
> Thanks for your feedback.
>
> On Mon, 17 Jun 2024 12:57:19 +0300, Dmitry Baryshkov wrote:
> > On Mon, Jun 17, 2024 at 10:30:30AM GMT, Jean Delvare wrote:
> > > Since commit 0166dc11be91 ("of: make CONFIG_OF user selectable"), it
> > > is possible to test-build any driver which depends on OF on any
> > > architecture by explicitly selecting OF. Therefore depending on
> > > COMPILE_TEST as an alternative is no longer needed.
> >
> > The goal of this clause is to allow build-testing the driver with OF
> > being disabled (meaning that some of OF functions are stubbed and some
> > might disappear). I don't see how user-selectable OF provides the same
> > result.
>
> Historically, the goal of this clause *was* to allow build-testing the
> driver on architectures which did not support OF, and that did make
> sense back then. As I understand it, building the driver without OF
> support was never a goal per se (if it was, then the driver wouldn't be
> set to depend on OF in the first place).
>
> Some of my other submissions include the following explanation which
> you might find useful (I ended up stripping it on resubmission after
> being told I was being too verbose, but maybe it was needed after all):
>
> It is actually better to always build such drivers with OF enabled,
> so that the test builds are closer to how each driver will actually be
> built on its intended target. Building them without OF may not test
> much as the compiler will optimize out potentially large parts of the
> code. In the worst case, this could even pop false positive warnings.
> Dropping COMPILE_TEST here improves the quality of our testing and
> avoids wasting time on non-existent issues.
This doesn't seem to match the COMPILE_TEST usage that I observe in
other places. For example, we frequently use 'depends on ARCH_QCOM ||
COMPILE_TEST'. Which means that the driver itself doesn't make sense
without ARCH_QCOM, but we want for it to be tested on non-ARCH_QCOM
cases. I think the same logic applies to 'depends on OF ||
COMPILE_TEST' clauses. The driver (DP AUX bus) depends on OF to be fully
functional, but it should be compilable even without OF case.
I'll let Douglas (in cc) comment if my understanding is incorrect.
--
With best wishes
Dmitry
Re: [PATCH v3 01/10] drm/bridge: cdns-dsi: Fix OF node pointer
On Mon, Jun 17, 2024 at 04:23:02PM GMT, Aradhya Bhatia wrote:
> Fix the OF node pointer passed to the of_drm_find_bridge() call to find
> the next bridge in the display chain.
>
> To find the next bridge in the pipeline, we need to pass "np" - the OF
> node pointer of the next entity in the devicetree chain. Passing
> "of_node" to of_drm_find_bridge will make the function try to fetch the
> bridge for the cdns-dsi which is not what's required.
>
> Fix that.
>
> Fixes: e19233955d9e ("drm/bridge: Add Cadence DSI driver")
> Signed-off-by: Aradhya Bhatia
> ---
> drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Dmitry Baryshkov
Nit: consider switching to devm_drm_of_get_bridge(), which does the
same.
--
With best wishes
Dmitry
Re: [PATCH v3 02/10] drm/bridge: cdns-dsi: Fix the phy_initialized variable
On Mon, Jun 17, 2024 at 04:23:03PM GMT, Aradhya Bhatia wrote:
> Update the Phy initialized state to "not initialized" when the driver
> (and the hardware by extension) gets suspended. This will allow the Phy
> to get initialized again after resume.
>
> Fixes: e19233955d9e ("drm/bridge: Add Cadence DSI driver")
> Signed-off-by: Aradhya Bhatia
> ---
> drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
> b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
> index b016f2ba06bb..42565e253b2d 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
> @@ -1153,6 +1153,7 @@ static int __maybe_unused cdns_dsi_suspend(struct
> device *dev)
> clk_disable_unprepare(dsi->dsi_p_clk);
> reset_control_assert(dsi->dsi_p_rst);
> dsi->link_initialized = false;
Most likely you should also call phy_exit() here. And in _remove() too.
> + dsi->phy_initialized = false;
> return 0;
> }
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
Re: [PATCH v3 07/10] drm/mipi-dsi: Add helper to find input format
On Mon, Jun 17, 2024 at 04:23:08PM GMT, Aradhya Bhatia wrote: > Add a helper API that can be used by the DSI hosts to find the required > input bus format for the given output dsi pixel format. > > Signed-off-by: Aradhya Bhatia > --- > drivers/gpu/drm/drm_mipi_dsi.c | 37 ++ > include/drm/drm_mipi_dsi.h | 1 + > 2 files changed, 38 insertions(+) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v3 06/10] drm/bridge: cdns-dsi: Reset the DCS write FIFO
On Mon, Jun 17, 2024 at 04:23:07PM GMT, Aradhya Bhatia wrote: > Allow the DCS Write FIFO in the cdns-dsi controller to reset before any > DCS packet is transmitted to the DSI sink device. > > The DCS FIFO reset is optional. Not all panels require it. But at > least one of the DSI based panel that uses Ilitek ILI9881C (DSI to DPI > bridge) doesn't work with without this reset. Could you please be more specific, why doesn't it work. Are there any leftover bytes in the FIFO? Is there any additional delay? > > Signed-off-by: Aradhya Bhatia > --- > drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c > b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c > index 05d2f4cc50da..87fdd07ca0bc 100644 > --- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c > +++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c > @@ -1037,6 +1037,9 @@ static ssize_t cdns_dsi_transfer(struct mipi_dsi_host > *host, > > cdns_dsi_init_link(dsi); > > + /* Reset the DCS Write FIFO */ > + writel(0x00, dsi->regs + DIRECT_CMD_FIFO_RST); > + > ret = mipi_dsi_create_packet(&packet, msg); > if (ret) > goto out; > -- > 2.34.1 > -- With best wishes Dmitry
Re: [PATCH RFC 2/5] ASoC: hdmi-codec: pass data to get_dai_id too
On Sat, Jun 15, 2024 at 08:53:31PM +0300, Dmitry Baryshkov wrote: > The upcoming DRM connector HDMI codec implementation is going to use > codec-specific data in the .get_dai_id to get drm_connector. Pass data > to the callback, as it is done with other hdmi_codec_ops callbacks. Acked-by: Mark Brown signature.asc Description: PGP signature
Re: [PATCH 2/6] drm/ttm: Store the bo_kmap_type in struct iosys_map
Hi
Am 14.06.24 um 16:31 schrieb Christian König:
Am 14.06.24 um 15:21 schrieb Thomas Zimmermann:
For each instances of struct iosys_map set up by ttm_bo_vmap(), store
the type of allocation in the instance. Use this information to unmap
the memory in ttm_bo_vunmap(). This change simplifies the unmap code
and puts the complicated logic entirely into the map code.
I'm not sure that's a good idea.
The mapping information should already be available in the resource
and storing it in the iosys_map structures duplicates that information.
So we might run into the issue that the resource has changed and so we
need a different approach now, but the iosys_map will say that we
should unmap things for example.
Patches 1 and 2 are only here to make patch 4 (add the kmap special
case) work. How can I distinguish between vmap'ed and kmap'ed memory?
It's not clear to me, whether there is a benefit from patch 4. The xe
driver makes it sound like that, but the rest of the kernel appears to
disagree.
Best regards
Thomas
Regards,
Christian.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/ttm/ttm_bo_util.c | 46 +--
1 file changed, 31 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 0b3f4267130c4..a9df0deff2deb 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -36,6 +36,7 @@
#include
#include
+#include
struct ttm_transfer_obj {
struct ttm_buffer_object base;
@@ -479,24 +480,29 @@ int ttm_bo_vmap(struct ttm_buffer_object *bo,
struct iosys_map *map)
if (mem->bus.is_iomem) {
void __iomem *vaddr_iomem;
+ u16 alloc_flags;
- if (mem->bus.addr)
+ if (mem->bus.addr) {
vaddr_iomem = (void __iomem *)mem->bus.addr;
- else if (mem->bus.caching == ttm_write_combined)
- vaddr_iomem = ioremap_wc(mem->bus.offset,
- bo->base.size);
+ alloc_flags = ttm_bo_map_premapped;
+ } else if (mem->bus.caching == ttm_write_combined) {
+ vaddr_iomem = ioremap_wc(mem->bus.offset, bo->base.size);
+ alloc_flags = ttm_bo_map_iomap;
#ifdef CONFIG_X86
- else if (mem->bus.caching == ttm_cached)
- vaddr_iomem = ioremap_cache(mem->bus.offset,
- bo->base.size);
+ } else if (mem->bus.caching == ttm_cached) {
+ vaddr_iomem = ioremap_cache(mem->bus.offset,
bo->base.size);
+ alloc_flags = ttm_bo_map_iomap;
#endif
- else
+ } else {
vaddr_iomem = ioremap(mem->bus.offset, bo->base.size);
+ alloc_flags = ttm_bo_map_iomap;
+ }
if (!vaddr_iomem)
return -ENOMEM;
iosys_map_set_vaddr_iomem(map, vaddr_iomem);
+ map->alloc_flags = alloc_flags;
} else {
struct ttm_operation_ctx ctx = {
@@ -506,6 +512,7 @@ int ttm_bo_vmap(struct ttm_buffer_object *bo,
struct iosys_map *map)
struct ttm_tt *ttm = bo->ttm;
pgprot_t prot;
void *vaddr;
+ u16 alloc_flags;
ret = ttm_tt_populate(bo->bdev, ttm, &ctx);
if (ret)
@@ -519,8 +526,10 @@ int ttm_bo_vmap(struct ttm_buffer_object *bo,
struct iosys_map *map)
vaddr = vmap(ttm->pages, ttm->num_pages, 0, prot);
if (!vaddr)
return -ENOMEM;
+ alloc_flags = ttm_bo_map_vmap;
iosys_map_set_vaddr(map, vaddr);
+ map->alloc_flags = alloc_flags;
}
return 0;
@@ -537,20 +546,27 @@ EXPORT_SYMBOL(ttm_bo_vmap);
*/
void ttm_bo_vunmap(struct ttm_buffer_object *bo, struct iosys_map
*map)
{
- struct ttm_resource *mem = bo->resource;
-
dma_resv_assert_held(bo->base.resv);
if (iosys_map_is_null(map))
return;
- if (!map->is_iomem)
- vunmap(map->vaddr);
- else if (!mem->bus.addr)
+ switch (map->alloc_flags) {
+ case ttm_bo_map_iomap:
iounmap(map->vaddr_iomem);
- iosys_map_clear(map);
-
+ break;
+ case ttm_bo_map_vmap:
+ vunmap(map->vaddr);
+ break;
+ case ttm_bo_map_premapped:
+ break;
+ default:
+ drm_err(bo->base.dev, "Unsupported alloc_flags 0x%x\n",
map->alloc_flags);
+ return;
+ }
ttm_mem_io_free(bo->bdev, bo->resource);
+
+ iosys_map_clear(map);
}
EXPORT_SYMBOL(ttm_bo_vunmap);
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
Re: [PATCH 00/15] Introduce HabanaLabs network drivers
From: Omer Shpigelman Date: Thu, 13 Jun 2024 11:21:53 +0300 > This patch set implements the HabanaLabs network drivers for Gaudi2 ASIC > which is designed for scaling of AI neural networks training. > The patch set includes the common code which is shared by all Gaudi ASICs > and the Gaudi2 ASIC specific code. Newer ASICs code will be followed. > All of these network drivers are modeled as an auxiliary devices to the > parent driver. > > The newly added drivers are Core Network (CN), Ethernet and InfiniBand. > All of these drivers are based on the existing habanalabs driver which > serves as the compute driver and the entire platform. > The habanalabs driver probes the network drivers which configure the > relevant NIC HW of the device. In addition, it continuously communicates > with the CN driver for providing some services which are not NIC specific > e.g. PCI, MMU, FW communication etc. > > See the drivers scheme at: > Documentation/networking/device_drivers/ethernet/intel/hbl.rst > > The CN driver is both a parent and a son driver. It serves as the common > layer of many shared operations that are required by both EN and IB > drivers. > > The Gaudi2 NIC HW is composed of 48 physical lanes, 56Gbps each. Each pair > of lanes represent a 100Gbps logical port. > > The NIC HW was designed specifically for scaling AI training. > Hence it basically functions as a regular NIC device but it is tuned for > its dedicated purpose. As a result, the NIC HW supports Ethernet traffic > and RDMA over modified ROCEv2 protocol. > For example, with respect to the IB driver, the HW supports a single > context and a single PD. The reason for this is that the operational use > case of AI training for Gaudi2 consists of a single user > application/process. > Another example related to the IB driver is the lack of MR since a single > application/process can share the entire MMU with the compute device. > Moreover, the memory allocation of user data buffers which are used for > RDMA communication is done via the habanalabs compute driver uAPI. > With respect to the Ethernet driver, since the Ethernet flow is used > mainly for control, the HW is not performance tuned e.g. it assumes a > contiguous memory for the Rx buffers. Thus the EN driver needs to copy the > Rx packets from the Rx buffer into the skb memory. > > The first 8 patches implement the CN driver. > The next 2 patches implement the EN driver. > The next 2 patches implement the IB driver. > The last 3 patches modify the compute driver to support the CN driver. > > The patches are rebased on v6.10-rc3 tag: > https://github.com/torvalds/linux/releases/tag/v6.10-rc3 > > The patches are also available at: > https://github.com/HabanaAI/drivers.gpu.linux-nic.kernel/tree/hbl_next > > The user-mode of the driver is being reviewed at: > https://github.com/linux-rdma/rdma-core/pull/1472 > > Any feedback, comment or question is welcome. > > Thanks, > Omer > > Omer Shpigelman (15): > net: hbl_cn: add habanalabs Core Network driver > net: hbl_cn: memory manager component > net: hbl_cn: physical layer support > net: hbl_cn: QP state machine > net: hbl_cn: memory trace events > net: hbl_cn: debugfs support > net: hbl_cn: gaudi2: ASIC register header files > net: hbl_cn: gaudi2: ASIC specific support > net: hbl_en: add habanalabs Ethernet driver > net: hbl_en: gaudi2: ASIC specific support > RDMA/hbl: add habanalabs RDMA driver > RDMA/hbl: direct verbs support > accel/habanalabs: network scaling support > accel/habanalabs/gaudi2: CN registers header files > accel/habanalabs/gaudi2: network scaling support > > .../ABI/testing/debugfs-driver-habanalabs_cn | 195 + > .../device_drivers/ethernet/index.rst | 1 + > .../device_drivers/ethernet/intel/hbl.rst |82 + > MAINTAINERS |33 + > drivers/accel/habanalabs/Kconfig | 1 + > drivers/accel/habanalabs/Makefile | 3 + > drivers/accel/habanalabs/cn/Makefile | 2 + > drivers/accel/habanalabs/cn/cn.c | 815 + > drivers/accel/habanalabs/cn/cn.h | 133 + > .../habanalabs/common/command_submission.c| 2 +- > drivers/accel/habanalabs/common/device.c |23 + > drivers/accel/habanalabs/common/firmware_if.c |20 + > drivers/accel/habanalabs/common/habanalabs.h |43 +- > .../accel/habanalabs/common/habanalabs_drv.c |37 +- > .../habanalabs/common/habanalabs_ioctl.c | 2 + > drivers/accel/habanalabs/common/memory.c | 123 + > drivers/accel/habanalabs/gaudi/gaudi.c|14 +- > drivers/accel/habanalabs/gaudi2/Makefile | 2 +- > drivers/accel/habanalabs/gaudi2/gaudi2.c | 440 +- > drivers/accel/habanalabs/gaudi2/gaudi2P.h |41 +- > drivers/accel/habanalabs/gaudi2/gaudi2_cn.c | 424 + > drivers/accel/habanalabs/gaudi2/gaudi2_cn.h |42 + > .../habanalabs/gaudi2/gaudi2_coresight.c |
Re: [PATCH 1/2] drm/amdgpu: make duplicated EOP packet for GFX7/8 have real content
Am 17.06.24 um 12:58 schrieb Icenowy Zheng:
The duplication of EOP packets for GFX7/8, with the former one have
seq-1 written and the latter one have seq written, seems to confuse some
hardware platform (e.g. Loongson 7A series PCIe controllers).
Make the content of the duplicated EOP packet the same with the real
one, only masking any possible interrupts.
Well completely NAK to that, exactly that disables the workaround.
The CPU needs to see two different values written here.
Regards,
Christian.
Fixes: bf26da927a1c ("drm/amdgpu: add cache flush workaround to gfx8
emit_fence")
Fixes: a2e73f56fa62 ("drm/amdgpu: Add support for CIK parts")
Signed-off-by: Icenowy Zheng
---
drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 12 +---
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 12
2 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index 541dbd70d8c75..778f27f1a34fe 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -2117,9 +2117,8 @@ static void gfx_v7_0_ring_emit_fence_gfx(struct
amdgpu_ring *ring, u64 addr,
{
bool write64bit = flags & AMDGPU_FENCE_FLAG_64BIT;
bool int_sel = flags & AMDGPU_FENCE_FLAG_INT;
- /* Workaround for cache flush problems. First send a dummy EOP
-* event down the pipe with seq one below.
-*/
+
+ /* Workaround for cache flush problems, send EOP twice. */
amdgpu_ring_write(ring, PACKET3(PACKET3_EVENT_WRITE_EOP, 4));
amdgpu_ring_write(ring, (EOP_TCL1_ACTION_EN |
EOP_TC_ACTION_EN |
@@ -2127,11 +2126,10 @@ static void gfx_v7_0_ring_emit_fence_gfx(struct
amdgpu_ring *ring, u64 addr,
EVENT_INDEX(5)));
amdgpu_ring_write(ring, addr & 0xfffc);
amdgpu_ring_write(ring, (upper_32_bits(addr) & 0x) |
- DATA_SEL(1) | INT_SEL(0));
- amdgpu_ring_write(ring, lower_32_bits(seq - 1));
- amdgpu_ring_write(ring, upper_32_bits(seq - 1));
+ DATA_SEL(write64bit ? 2 : 1) | INT_SEL(0));
+ amdgpu_ring_write(ring, lower_32_bits(seq));
+ amdgpu_ring_write(ring, upper_32_bits(seq));
- /* Then send the real EOP event down the pipe. */
amdgpu_ring_write(ring, PACKET3(PACKET3_EVENT_WRITE_EOP, 4));
amdgpu_ring_write(ring, (EOP_TCL1_ACTION_EN |
EOP_TC_ACTION_EN |
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 2f0e72caee1af..39a7d60f1fd69 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -6153,9 +6153,7 @@ static void gfx_v8_0_ring_emit_fence_gfx(struct
amdgpu_ring *ring, u64 addr,
bool write64bit = flags & AMDGPU_FENCE_FLAG_64BIT;
bool int_sel = flags & AMDGPU_FENCE_FLAG_INT;
- /* Workaround for cache flush problems. First send a dummy EOP
-* event down the pipe with seq one below.
-*/
+ /* Workaround for cache flush problems, send EOP twice. */
amdgpu_ring_write(ring, PACKET3(PACKET3_EVENT_WRITE_EOP, 4));
amdgpu_ring_write(ring, (EOP_TCL1_ACTION_EN |
EOP_TC_ACTION_EN |
@@ -6164,12 +6162,10 @@ static void gfx_v8_0_ring_emit_fence_gfx(struct
amdgpu_ring *ring, u64 addr,
EVENT_INDEX(5)));
amdgpu_ring_write(ring, addr & 0xfffc);
amdgpu_ring_write(ring, (upper_32_bits(addr) & 0x) |
- DATA_SEL(1) | INT_SEL(0));
- amdgpu_ring_write(ring, lower_32_bits(seq - 1));
- amdgpu_ring_write(ring, upper_32_bits(seq - 1));
+ DATA_SEL(write64bit ? 2 : 1) | INT_SEL(0));
+ amdgpu_ring_write(ring, lower_32_bits(seq));
+ amdgpu_ring_write(ring, upper_32_bits(seq));
- /* Then send the real EOP event down the pipe:
-* EVENT_WRITE_EOP - flush caches, send int */
amdgpu_ring_write(ring, PACKET3(PACKET3_EVENT_WRITE_EOP, 4));
amdgpu_ring_write(ring, (EOP_TCL1_ACTION_EN |
EOP_TC_ACTION_EN |
Re: [PATCH] drm/tidss: Add drm_panic support
Hi Javier, I tested the patch, and it was good to see the panic screen on SK-AM62. Thanks for adding this feature in tidss. =) On 15/06/24 14:23, Javier Martinez Canillas wrote: > Add support for the drm_panic module, which displays a pretty user > friendly message on the screen when a Linux kernel panic occurs. > > Signed-off-by: Javier Martinez Canillas Reviewed-by: Aradhya Bhatia > --- > Tested on an AM625 BeaglePlay board by triggering a panic using the > `echo c > /proc/sysrq-trigger` command. > > drivers/gpu/drm/tidss/tidss_plane.c | 14 +- > 1 file changed, 13 insertions(+), 1 deletion(-) > [...] -- Regards Aradhya
[PATCH] drm/i915/gem: Return SIGBUS for wrong mapping parameters
We normally issue a warning when incorrect memory parameters are provided. Typically, providing erroneous addresses to mmap, results in a segmentation fault, and the default behavior is to return VM_FAULT_SIGBUS. This can happen for example when remap_io_mapping() or remap_io_sg() return -EINVAL. Because VM_FAULT_SIGBUS is already returned when memory boundaries are improperly handled and numerous warnings are already generated, avoid redundant logging to prevent overprinting by translating -EINVAL to VM_FAULT_SIGBUS in the i915_error_to_vmf_fault() helper. Signed-off-by: Andi Shyti --- drivers/gpu/drm/i915/gem/i915_gem_mman.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c index a2195e28b625..698ff42b004a 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c @@ -223,6 +223,7 @@ static vm_fault_t i915_error_to_vmf_fault(int err) default: WARN_ONCE(err, "unhandled error in %s: %i\n", __func__, err); fallthrough; + case -EINVAL: /* the memory parameters provided are wrong */ case -EIO: /* shmemfs failure from swap device */ case -EFAULT: /* purged object */ case -ENODEV: /* bad object, how did you get here! */ -- 2.45.1
Re: [PATCH v2] fbdev: vesafb: Detect VGA compatibility from screen info's VESA attributes
On 6/17/24 13:30, Thomas Zimmermann wrote:
Am 17.06.24 um 13:06 schrieb Thomas Zimmermann:
Test the vesa_attributes field in struct screen_info for compatibility
with VGA hardware. Vesafb currently tests bit 1 in screen_info's
capabilities field, It sets the framebuffer address size and is
unrelated to VGA.
Section 4.4 of the Vesa VBE 2.0 specifications defines that bit 5 in
the mode's attributes field signals VGA compatibility. The mode is
compatible with VGA hardware if the bit is clear. In that case, the
driver can access VGA state of the VBE's underlying hardware. The
vesafb driver uses this feature to program the color LUT in palette
modes. Without, colors might be incorrect.
The problem got introduced in commit 89ec4c238e7a ("[PATCH] vesafb: Fix
incorrect logo colors in x86_64"). It incorrectly stores the mode
attributes in the screen_info's capabilities field and updates vesafb
accordingly. Later, commit 5e8ddcbe8692 ("Video mode probing support for
the new x86 setup code") fixed the screen_info, but did not update vesafb.
Color output still tends to work, because bit 1 in capabilities is
usually 0.
Besides fixing the bug in vesafb, this commit introduces a helper that
reads the correct bit from screen_info.
v2:
- clarify comment on non-VGA modes (Helge)
Signed-off-by: Thomas Zimmermann
Fixes: 5e8ddcbe8692 ("Video mode probing support for the new x86 setup code")
Reviewed-by: Javier Martinez Canillas
Cc: # v2.6.23+
---
drivers/video/fbdev/vesafb.c | 2 +-
include/linux/screen_info.h | 10 ++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c
index 8ab64ae4cad3e..5a161750a3aee 100644
--- a/drivers/video/fbdev/vesafb.c
+++ b/drivers/video/fbdev/vesafb.c
@@ -271,7 +271,7 @@ static int vesafb_probe(struct platform_device *dev)
if (si->orig_video_isVGA != VIDEO_TYPE_VLFB)
return -ENODEV;
- vga_compat = (si->capabilities & 2) ? 0 : 1;
+ vga_compat = !__screen_info_vbe_mode_nonvga(si);
vesafb_fix.smem_start = si->lfb_base;
vesafb_defined.bits_per_pixel = si->lfb_depth;
if (15 == vesafb_defined.bits_per_pixel)
diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h
index 75303c126285a..d21f8e4e9f4a4 100644
--- a/include/linux/screen_info.h
+++ b/include/linux/screen_info.h
@@ -49,6 +49,16 @@ static inline u64 __screen_info_lfb_size(const struct
screen_info *si, unsigned
return lfb_size;
}
+static inline bool __screen_info_vbe_mode_nonvga(const struct screen_info *si)
+{
+ /*
+ * VESA modes typically run on VGA hardware. Set bit 5 signal that this
'signals'
I've fixed this up in your patch and applied it to the fbdev git tree.
No need to send new patch...
Thanks!
Helge
Re: [PATCH v2] fbdev: vesafb: Detect VGA compatibility from screen info's VESA attributes
Hi
Am 17.06.24 um 14:42 schrieb Helge Deller:
On 6/17/24 13:30, Thomas Zimmermann wrote:
Am 17.06.24 um 13:06 schrieb Thomas Zimmermann:
Test the vesa_attributes field in struct screen_info for compatibility
with VGA hardware. Vesafb currently tests bit 1 in screen_info's
capabilities field, It sets the framebuffer address size and is
unrelated to VGA.
Section 4.4 of the Vesa VBE 2.0 specifications defines that bit 5 in
the mode's attributes field signals VGA compatibility. The mode is
compatible with VGA hardware if the bit is clear. In that case, the
driver can access VGA state of the VBE's underlying hardware. The
vesafb driver uses this feature to program the color LUT in palette
modes. Without, colors might be incorrect.
The problem got introduced in commit 89ec4c238e7a ("[PATCH] vesafb: Fix
incorrect logo colors in x86_64"). It incorrectly stores the mode
attributes in the screen_info's capabilities field and updates vesafb
accordingly. Later, commit 5e8ddcbe8692 ("Video mode probing support
for
the new x86 setup code") fixed the screen_info, but did not update
vesafb.
Color output still tends to work, because bit 1 in capabilities is
usually 0.
Besides fixing the bug in vesafb, this commit introduces a helper that
reads the correct bit from screen_info.
v2:
- clarify comment on non-VGA modes (Helge)
Signed-off-by: Thomas Zimmermann
Fixes: 5e8ddcbe8692 ("Video mode probing support for the new x86
setup code")
Reviewed-by: Javier Martinez Canillas
Cc: # v2.6.23+
---
drivers/video/fbdev/vesafb.c | 2 +-
include/linux/screen_info.h | 10 ++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/vesafb.c
b/drivers/video/fbdev/vesafb.c
index 8ab64ae4cad3e..5a161750a3aee 100644
--- a/drivers/video/fbdev/vesafb.c
+++ b/drivers/video/fbdev/vesafb.c
@@ -271,7 +271,7 @@ static int vesafb_probe(struct platform_device
*dev)
if (si->orig_video_isVGA != VIDEO_TYPE_VLFB)
return -ENODEV;
- vga_compat = (si->capabilities & 2) ? 0 : 1;
+ vga_compat = !__screen_info_vbe_mode_nonvga(si);
vesafb_fix.smem_start = si->lfb_base;
vesafb_defined.bits_per_pixel = si->lfb_depth;
if (15 == vesafb_defined.bits_per_pixel)
diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h
index 75303c126285a..d21f8e4e9f4a4 100644
--- a/include/linux/screen_info.h
+++ b/include/linux/screen_info.h
@@ -49,6 +49,16 @@ static inline u64 __screen_info_lfb_size(const
struct screen_info *si, unsigned
return lfb_size;
}
+static inline bool __screen_info_vbe_mode_nonvga(const struct
screen_info *si)
+{
+ /*
+ * VESA modes typically run on VGA hardware. Set bit 5 signal
that this
'signals'
I've fixed this up in your patch and applied it to the fbdev git tree.
No need to send new patch...
Great, thank you so much.
Best regards
Thomas
Thanks!
Helge
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
Re: [PATCH v2 7/7] drm/panic: Add support for drawing a monochrome graphical logo
Hi Jocelyn,
On Mon, Jun 17, 2024 at 11:59 AM Jocelyn Falempe wrote:
> On 13/06/2024 21:18, Geert Uytterhoeven wrote:
> > Re-use the existing support for boot-up logos to draw a monochrome
> > graphical logo in the DRM panic handler. When no suitable graphical
> > logo is available, the code falls back to the ASCII art penguin logo.
> >
> > Note that all graphical boot-up logos are freed during late kernel
> > initialization, hence a copy must be made for later use.
> >
> > Signed-off-by: Geert Uytterhoeven
> > --- a/drivers/gpu/drm/drm_panic.c
> > +++ b/drivers/gpu/drm/drm_panic.c
> > PANIC_LINE(" \\___)=(___/"),
> > };
> >
> > +#ifdef CONFIG_LOGO
> > +static const struct linux_logo *logo_mono;
> > +
> > +static int drm_panic_setup_logo(void)
> > +{
> > + const struct linux_logo *logo = fb_find_logo(1);
> > + const unsigned char *logo_data;
> > + struct linux_logo *logo_dup;
> > +
> > + if (!logo || logo->type != LINUX_LOGO_MONO)
> > + return 0;
> > +
> > + /* The logo is __init, so we must make a copy for later use */
> > + logo_data = kmemdup(logo->data,
> > + size_mul(DIV_ROUND_UP(logo->width,
> > BITS_PER_BYTE), logo->height),
> > + GFP_KERNEL);
> > + if (!logo_data)
> > + return -ENOMEM;
> > +
> > + logo_dup = kmemdup(logo, sizeof(*logo), GFP_KERNEL);
> > + if (!logo_dup) {
> > + kfree(logo_data);
> > + return -ENOMEM;
> > + }
> > +
> > + logo_dup->data = logo_data;
> > + logo_mono = logo_dup;
> > +
> > + return 0;
> > +}
> > +
> > +device_initcall(drm_panic_setup_logo);
> > +#else
> > +#define logo_mono((const struct linux_logo *)NULL)
> > +#endif
>
> I didn't notice that the first time, but the core drm can be built as a
> module.
> That means this will leak memory each time the module is removed.
While I hadn't considered a modular DRM core, there is no memory leak:
after the logos are freed (from late_initcall_sync()), fb_find_logo()
returns NULL. This does mean there won't be a graphical logo on panic,
though.
> But to solve the circular dependency between drm_kms_helper and
> drm_panic, one solution is to depends on drm core to be built-in.
> In this case there won't be a leak.
> So depending on how we solve the circular dependency, it can be acceptable.
So far there is no reason to select DRM_KMS_HELPER, right?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 1/2] drm/i915/gem: Return -EINVAL instead of '0'
On Sun, Jun 16, 2024 at 09:03:48AM GMT, Andi Shyti wrote:
Commit 05da7d9f717b ("drm/i915/gem: Downgrade stolen lmem setup
warning") returns '0' from i915_gem_stolen_lmem_setup(), but it's
supposed to return a pointer to the intel_memory_region
structure.
Sparse complains with the following message:
drivers/gpu/drm/i915/gem/i915_gem_stolen.c:943:32: sparse: sparse:
Using plain integer as NULL pointer
The caller checks for errors, and if no error is returned, it
stores the address of the stolen memory. Therefore, we can't
return NULL. Since we are handling a case of out-of-bounds, it's
appropriate to treat the "lmem_size < dsm_base" case as an error.
which completely invalidates the point of the commit that introduced this
regression. That was commit was supposed to do "let's continue, just
disabling stolen". Apparently we should just revert that patch instead
since it introduced 2 new issues and didn't solve what it was supposed
to... for probe failures, we are completely fine leaving the warning
there.
Lucas De Marchi
Return -EINVAL embedded in a pointer instead of '0' (or NULL).
This way, we avoid a potential NULL pointer dereference.
Since we are returning an error, it makes sense to print an error
message with drm_err() instead of a debug message using
drm_dbg().
Fixes: 05da7d9f717b ("drm/i915/gem: Downgrade stolen lmem setup warning")
Signed-off-by: Andi Shyti
Cc: Jonathan Cavitt
---
drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index 004471f60117..bd774ce713cf 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -937,10 +937,10 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915,
u16 type,
/* Use DSM base address instead for stolen memory */
dsm_base = intel_uncore_read64(uncore, GEN6_DSMBASE) &
GEN11_BDSM_MASK;
if (lmem_size < dsm_base) {
- drm_dbg(&i915->drm,
+ drm_err(&i915->drm,
"Disabling stolen memory support due to OOB
placement: lmem_size = %lli vs dsm_base = %lli\n",
lmem_size, dsm_base);
- return 0;
+ return ERR_PTR(-EINVAL);
}
dsm_size = ALIGN_DOWN(lmem_size - dsm_base, SZ_1M);
}
--
2.45.1
Re: [PATCH 2/6] drm/ttm: Store the bo_kmap_type in struct iosys_map
Hi,
Am 17.06.24 um 14:32 schrieb Thomas Zimmermann:
Hi
Am 14.06.24 um 16:31 schrieb Christian König:
Am 14.06.24 um 15:21 schrieb Thomas Zimmermann:
For each instances of struct iosys_map set up by ttm_bo_vmap(), store
the type of allocation in the instance. Use this information to unmap
the memory in ttm_bo_vunmap(). This change simplifies the unmap code
and puts the complicated logic entirely into the map code.
I'm not sure that's a good idea.
The mapping information should already be available in the resource
and storing it in the iosys_map structures duplicates that information.
So we might run into the issue that the resource has changed and so
we need a different approach now, but the iosys_map will say that we
should unmap things for example.
Patches 1 and 2 are only here to make patch 4 (add the kmap special
case) work. How can I distinguish between vmap'ed and kmap'ed memory?
It's not clear to me, whether there is a benefit from patch 4. The xe
driver makes it sound like that, but the rest of the kernel appears to
disagree.
Yeah, exactly that's the point.
The last time we talked about that we came to the conclusion that the
kmap approach of mapping only a single page or range of pages isn't that
useful in general.
The only use case where you actually need this is the
ttm_bo_vm_access_kmap() helper and that is static and internal to TTM.
So what exactly is the use case xe tries to handle here?
Regards,
Christian.
Best regards
Thomas
Regards,
Christian.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/ttm/ttm_bo_util.c | 46
+--
1 file changed, 31 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 0b3f4267130c4..a9df0deff2deb 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -36,6 +36,7 @@
#include
#include
+#include
struct ttm_transfer_obj {
struct ttm_buffer_object base;
@@ -479,24 +480,29 @@ int ttm_bo_vmap(struct ttm_buffer_object *bo,
struct iosys_map *map)
if (mem->bus.is_iomem) {
void __iomem *vaddr_iomem;
+ u16 alloc_flags;
- if (mem->bus.addr)
+ if (mem->bus.addr) {
vaddr_iomem = (void __iomem *)mem->bus.addr;
- else if (mem->bus.caching == ttm_write_combined)
- vaddr_iomem = ioremap_wc(mem->bus.offset,
- bo->base.size);
+ alloc_flags = ttm_bo_map_premapped;
+ } else if (mem->bus.caching == ttm_write_combined) {
+ vaddr_iomem = ioremap_wc(mem->bus.offset, bo->base.size);
+ alloc_flags = ttm_bo_map_iomap;
#ifdef CONFIG_X86
- else if (mem->bus.caching == ttm_cached)
- vaddr_iomem = ioremap_cache(mem->bus.offset,
- bo->base.size);
+ } else if (mem->bus.caching == ttm_cached) {
+ vaddr_iomem = ioremap_cache(mem->bus.offset,
bo->base.size);
+ alloc_flags = ttm_bo_map_iomap;
#endif
- else
+ } else {
vaddr_iomem = ioremap(mem->bus.offset, bo->base.size);
+ alloc_flags = ttm_bo_map_iomap;
+ }
if (!vaddr_iomem)
return -ENOMEM;
iosys_map_set_vaddr_iomem(map, vaddr_iomem);
+ map->alloc_flags = alloc_flags;
} else {
struct ttm_operation_ctx ctx = {
@@ -506,6 +512,7 @@ int ttm_bo_vmap(struct ttm_buffer_object *bo,
struct iosys_map *map)
struct ttm_tt *ttm = bo->ttm;
pgprot_t prot;
void *vaddr;
+ u16 alloc_flags;
ret = ttm_tt_populate(bo->bdev, ttm, &ctx);
if (ret)
@@ -519,8 +526,10 @@ int ttm_bo_vmap(struct ttm_buffer_object *bo,
struct iosys_map *map)
vaddr = vmap(ttm->pages, ttm->num_pages, 0, prot);
if (!vaddr)
return -ENOMEM;
+ alloc_flags = ttm_bo_map_vmap;
iosys_map_set_vaddr(map, vaddr);
+ map->alloc_flags = alloc_flags;
}
return 0;
@@ -537,20 +546,27 @@ EXPORT_SYMBOL(ttm_bo_vmap);
*/
void ttm_bo_vunmap(struct ttm_buffer_object *bo, struct iosys_map
*map)
{
- struct ttm_resource *mem = bo->resource;
-
dma_resv_assert_held(bo->base.resv);
if (iosys_map_is_null(map))
return;
- if (!map->is_iomem)
- vunmap(map->vaddr);
- else if (!mem->bus.addr)
+ switch (map->alloc_flags) {
+ case ttm_bo_map_iomap:
iounmap(map->vaddr_iomem);
- iosys_map_clear(map);
-
+ break;
+ case ttm_bo_map_vmap:
+ vunmap(map->vaddr);
+ break;
+ case ttm_bo_map_premapped:
+ break;
+ default:
+ drm_err(bo->base.dev, "Unsupported alloc_flags 0x%x\n",
map->alloc_flags);
+ return;
+ }
ttm_mem_io_free(bo->bdev, bo->resource);
+
+ iosys_map_clear(map);
}
EXPORT_SYMBOL(ttm_bo_vunmap);
Re: [PATCH v5 1/4] drm/sched: add device name to the drm_sched_process_job event
Hi,
Le 14/06/2024 à 11:08, Christian König a écrit :
Am 14.06.24 um 10:16 schrieb Pierre-Eric Pelloux-Prayer:
Until the switch from kthread to workqueue, a userspace application could
determine the source device from the pid of the thread sending the event.
With workqueues this is not possible anymore, so the event needs to
contain
the dev_name() to identify the device.
Signed-off-by: Pierre-Eric Pelloux-Prayer
---
drivers/gpu/drm/scheduler/gpu_scheduler_trace.h | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
b/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
index c75302ca3427..1f9c5a884487 100644
--- a/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
+++ b/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
@@ -42,6 +42,7 @@ DECLARE_EVENT_CLASS(drm_sched_job,
__field(uint64_t, id)
__field(u32, job_count)
__field(int, hw_job_count)
+ __string(dev, dev_name(sched_job->sched->dev))
),
TP_fast_assign(
@@ -52,6 +53,7 @@ DECLARE_EVENT_CLASS(drm_sched_job,
__entry->job_count =
spsc_queue_count(&entity->job_queue);
__entry->hw_job_count = atomic_read(
&sched_job->sched->credit_count);
+ __assign_str(dev);
),
TP_printk("entity=%p, id=%llu, fence=%p, ring=%s, job
count:%u, hw job count:%d",
__entry->entity, __entry->id,
@@ -64,9 +66,13 @@ DEFINE_EVENT(drm_sched_job, drm_sched_job,
TP_ARGS(sched_job, entity)
);
-DEFINE_EVENT(drm_sched_job, drm_run_job,
+DEFINE_EVENT_PRINT(drm_sched_job, drm_run_job,
TP_PROTO(struct drm_sched_job *sched_job, struct
drm_sched_entity *entity),
- TP_ARGS(sched_job, entity)
+ TP_ARGS(sched_job, entity),
+ TP_printk("dev=%s, entity=%p id=%llu, fence=%p, ring=%s, job
count:%u, hw job count:%d",
+ __get_str(dev), __entry->entity, __entry->id,
+ __entry->fence, __get_str(name),
+ __entry->job_count, __entry->hw_job_count)
Why not doing that in the TP_printk() of the drm_sched_job event class?
I didn't because drm_sched_job + drm_run_job usually work in pair so the
ring+device information will be available anyway.
But maybe you're right and it's better to add it in the event the job
never gets scheduled.
For the other events it's not necessary IMO: the first event will always
be drm_sched_job, so it's possible to figure out which the device/ring
for a given job.
Regards,
Pierre-eric
The device should now be available for all trace events of that class.
Regards,
Christian.
);
TRACE_EVENT(drm_sched_process_job,
Re: [PATCH v5 1/4] drm/sched: add device name to the drm_sched_process_job event
Am 17.06.24 um 14:54 schrieb Pierre-Eric Pelloux-Prayer:
Hi,
Le 14/06/2024 à 11:08, Christian König a écrit :
Am 14.06.24 um 10:16 schrieb Pierre-Eric Pelloux-Prayer:
Until the switch from kthread to workqueue, a userspace application
could
determine the source device from the pid of the thread sending the
event.
With workqueues this is not possible anymore, so the event needs to
contain
the dev_name() to identify the device.
Signed-off-by: Pierre-Eric Pelloux-Prayer
---
drivers/gpu/drm/scheduler/gpu_scheduler_trace.h | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
b/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
index c75302ca3427..1f9c5a884487 100644
--- a/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
+++ b/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
@@ -42,6 +42,7 @@ DECLARE_EVENT_CLASS(drm_sched_job,
__field(uint64_t, id)
__field(u32, job_count)
__field(int, hw_job_count)
+ __string(dev, dev_name(sched_job->sched->dev))
),
TP_fast_assign(
@@ -52,6 +53,7 @@ DECLARE_EVENT_CLASS(drm_sched_job,
__entry->job_count =
spsc_queue_count(&entity->job_queue);
__entry->hw_job_count = atomic_read(
&sched_job->sched->credit_count);
+ __assign_str(dev);
),
TP_printk("entity=%p, id=%llu, fence=%p, ring=%s, job
count:%u, hw job count:%d",
__entry->entity, __entry->id,
@@ -64,9 +66,13 @@ DEFINE_EVENT(drm_sched_job, drm_sched_job,
TP_ARGS(sched_job, entity)
);
-DEFINE_EVENT(drm_sched_job, drm_run_job,
+DEFINE_EVENT_PRINT(drm_sched_job, drm_run_job,
TP_PROTO(struct drm_sched_job *sched_job, struct
drm_sched_entity *entity),
- TP_ARGS(sched_job, entity)
+ TP_ARGS(sched_job, entity),
+ TP_printk("dev=%s, entity=%p id=%llu, fence=%p, ring=%s,
job count:%u, hw job count:%d",
+ __get_str(dev), __entry->entity, __entry->id,
+ __entry->fence, __get_str(name),
+ __entry->job_count, __entry->hw_job_count)
Why not doing that in the TP_printk() of the drm_sched_job event class?
I didn't because drm_sched_job + drm_run_job usually work in pair so
the ring+device information will be available anyway.
But maybe you're right and it's better to add it in the event the job
never gets scheduled.
For the other events it's not necessary IMO: the first event will
always be drm_sched_job, so it's possible to figure out which the
device/ring for a given job.
Yeah, but we have that strace in the trace event even when we don't
print it.
I would just go ahead and always print it even when it is available
somehow else.
Regards,
Christian.
Regards,
Pierre-eric
The device should now be available for all trace events of that class.
Regards,
Christian.
);
TRACE_EVENT(drm_sched_process_job,
Re: [PATCH 3/6] drm/ttm: Support partial buffer mappings for ttm_bo_vmap()
Hi
Am 14.06.24 um 16:33 schrieb Christian König:
Am 14.06.24 um 15:21 schrieb Thomas Zimmermann:
Add offset and size parameters to ttm_bo_vmap() to allow for partial
mappings of a buffer object. This brings the functionality on par with
ttm_bo_kmap().
Well the long term plan was to remove this functionality from
ttm_bo_kmap() and nuke that function sooner or later.
What exactly is the use case for partial mappings?
I don't have a use case; except for streamlining the DRM internals.
For the drivers, xe uses partial mappings in
i915_gem_object_read_from_page(). But struct iosys_map to map the whole
buffer and do memcpys within the range. See patch 6 for that. I think it
could do without partial mappings. Vmwgfx uses a local mappings at [1],
but as the nearby comment says, this could be a TTM helper. Or maybe TTM
could provide a dedicated ttm_bo_kmap_local_page() for temporary
mappings like this one. There's also [2], which looks like a candidate
for a possible ttm_bo_kmap_local_page(). And in vmwgfx, there's the
memcpy at [3], which could be replaced with ttm_bo_vmap() plus iosys_map
helpers. I think that's it. Best regards Thomas [1]
https://elixir.bootlin.com/linux/v6.9/source/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c#L457
[2]
https://elixir.bootlin.com/linux/v6.9/source/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c#L418
[3]
https://elixir.bootlin.com/linux/v6.9/source/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c#L430
Regards,
Christian.
Callers pass the byte offset and size within the buffer object and
receive a page-aligned mapping of the buffer object's memory for the
specified area.
Also update all callers of ttm_bo_vmap() for the new parameters. As
before, existing callers map the buffer object's complete memory.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_gem_ttm_helper.c | 2 +-
drivers/gpu/drm/drm_gem_vram_helper.c | 2 +-
drivers/gpu/drm/loongson/lsdc_gem.c | 2 +-
drivers/gpu/drm/qxl/qxl_object.c | 2 +-
drivers/gpu/drm/ttm/ttm_bo_util.c | 21 +++--
drivers/gpu/drm/xe/xe_lrc.c | 2 +-
drivers/gpu/drm/xe/xe_vm.c | 2 +-
include/drm/ttm/ttm_bo.h | 4 +++-
8 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c
b/drivers/gpu/drm/drm_gem_ttm_helper.c
index 3734aa2d1c5b5..f26b7c9077a68 100644
--- a/drivers/gpu/drm/drm_gem_ttm_helper.c
+++ b/drivers/gpu/drm/drm_gem_ttm_helper.c
@@ -67,7 +67,7 @@ int drm_gem_ttm_vmap(struct drm_gem_object *gem,
{
struct ttm_buffer_object *bo = drm_gem_ttm_of_gem(gem);
- return ttm_bo_vmap(bo, map);
+ return ttm_bo_vmap(bo, 0, gem->size, map);
}
EXPORT_SYMBOL(drm_gem_ttm_vmap);
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
b/drivers/gpu/drm/drm_gem_vram_helper.c
index 6027584406af6..1670f9a459a9d 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -398,7 +398,7 @@ int drm_gem_vram_vmap(struct drm_gem_vram_object
*gbo, struct iosys_map *map)
* no mapping present.
*/
if (iosys_map_is_null(&gbo->map)) {
- ret = ttm_bo_vmap(&gbo->bo, &gbo->map);
+ ret = ttm_bo_vmap(&gbo->bo, 0, gbo->bo.base.size, &gbo->map);
if (ret)
return ret;
}
diff --git a/drivers/gpu/drm/loongson/lsdc_gem.c
b/drivers/gpu/drm/loongson/lsdc_gem.c
index a720d8f532093..f709960c781b9 100644
--- a/drivers/gpu/drm/loongson/lsdc_gem.c
+++ b/drivers/gpu/drm/loongson/lsdc_gem.c
@@ -77,7 +77,7 @@ static int lsdc_gem_object_vmap(struct
drm_gem_object *obj, struct iosys_map *ma
return ret;
}
- ret = ttm_bo_vmap(tbo, &lbo->map);
+ ret = ttm_bo_vmap(tbo, 0, tbo->base.size, &lbo->map);
if (ret) {
drm_err(obj->dev, "ttm bo vmap failed\n");
lsdc_bo_unpin(lbo);
diff --git a/drivers/gpu/drm/qxl/qxl_object.c
b/drivers/gpu/drm/qxl/qxl_object.c
index 5893e27a7ae50..9f06d5e26a32c 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -164,7 +164,7 @@ int qxl_bo_vmap_locked(struct qxl_bo *bo, struct
iosys_map *map)
goto out;
}
- r = ttm_bo_vmap(&bo->tbo, &bo->map);
+ r = ttm_bo_vmap(&bo->tbo, 0, bo->tbo.base.size, &bo->map);
if (r) {
qxl_bo_unpin_locked(bo);
return r;
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index a9df0deff2deb..31f9772f05dac 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -457,17 +457,23 @@ EXPORT_SYMBOL(ttm_bo_kunmap);
* ttm_bo_vmap
*
* @bo: The buffer object.
+ * @offset: Byte offset into the buffer.
+ * @size: Number of bytes to map.
* @map: pointer to a struct iosys_map representing the map.
*
* Sets up a kernel virtual mapping, using ioremap or vmap to the
* data in the buffer object. The parameter @map returns the virtual
* address as struct iosys_map. Unmap the buffe
Re: [PATCH V2] drm/bridge: adv7511: Fix Intermittent EDID failures
[CCing the regression list, as it should be in the loop for regressions:
https://docs.kernel.org/admin-guide/reporting-regressions.html]
Hi! Top-posting for once, to make this easily accessible to everyone.
Hmm, seem nobody took a look at below fix for a regression that seems to
be caused by f3d9683346d6b1 ("drm/bridge: adv7511: Allow IRQ to share
GPIO pins") [which went into v6.10-rc1].
Adam and Dimitry, what are your stances on this patch from Adam? I'm
asking, as you authored respectively committed the culprit?
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
#regzbot poke
On 01.06.24 15:24, Adam Ford wrote:
> In the process of adding support for shared IRQ pins, a scenario
> was accidentally created where adv7511_irq_process returned
> prematurely causing the EDID to fail randomly.
>
> Since the interrupt handler is broken up into two main helper functions,
> update both of them to treat the helper functions as IRQ handlers. These
> IRQ routines process their respective tasks as before, but if they
> determine that actual work was done, mark the respective IRQ status
> accordingly, and delay the check until everything has been processed.
>
> This should guarantee the helper functions don't return prematurely
> while still returning proper values of either IRQ_HANDLED or IRQ_NONE.
>
> Reported-by: Liu Ying
> Fixes: f3d9683346d6 ("drm/bridge: adv7511: Allow IRQ to share GPIO pins")
> Signed-off-by: Adam Ford
> Tested-by: Liu Ying # i.MX8MP EVK ADV7535 EDID retrieval
> w/o IRQ
> ---
> V2: Fix uninitialized cec_status
> Cut back a little on error handling to return either IRQ_NONE or
> IRQ_HANDLED.
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> index ea271f62b214..ec0b7f3d889c 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -401,7 +401,7 @@ struct adv7511 {
>
> #ifdef CONFIG_DRM_I2C_ADV7511_CEC
> int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511);
> -void adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1);
> +int adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1);
> #else
> static inline int adv7511_cec_init(struct device *dev, struct adv7511
> *adv7511)
> {
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
> index 44451a9658a3..651fb1dde780 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
> @@ -119,7 +119,7 @@ static void adv7511_cec_rx(struct adv7511 *adv7511, int
> rx_buf)
> cec_received_msg(adv7511->cec_adap, &msg);
> }
>
> -void adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1)
> +int adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1)
> {
> unsigned int offset = adv7511->info->reg_cec_offset;
> const u32 irq_tx_mask = ADV7511_INT1_CEC_TX_READY |
> @@ -130,17 +130,21 @@ void adv7511_cec_irq_process(struct adv7511 *adv7511,
> unsigned int irq1)
> ADV7511_INT1_CEC_RX_READY3;
> unsigned int rx_status;
> int rx_order[3] = { -1, -1, -1 };
> - int i;
> + int i, ret = 0;
> + int irq_status = IRQ_NONE;
>
> - if (irq1 & irq_tx_mask)
> + if (irq1 & irq_tx_mask) {
> adv_cec_tx_raw_status(adv7511, irq1);
> + irq_status = IRQ_HANDLED;
> + }
>
> if (!(irq1 & irq_rx_mask))
> - return;
> + return irq_status;
>
> - if (regmap_read(adv7511->regmap_cec,
> - ADV7511_REG_CEC_RX_STATUS + offset, &rx_status))
> - return;
> + ret = regmap_read(adv7511->regmap_cec,
> + ADV7511_REG_CEC_RX_STATUS + offset, &rx_status);
> + if (ret < 0)
> + return irq_status;
>
> /*
>* ADV7511_REG_CEC_RX_STATUS[5:0] contains the reception order of RX
> @@ -172,6 +176,8 @@ void adv7511_cec_irq_process(struct adv7511 *adv7511,
> unsigned int irq1)
>
> adv7511_cec_rx(adv7511, rx_buf);
> }
> +
> + return IRQ_HANDLED;
> }
>
> static int adv7511_cec_adap_enable(struct cec_adapter *adap, bool enable)
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index 66ccb61e2a66..c8d2c4a157b2 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -469,6 +469,8 @@ static int adv7511_irq_process(struct adv7511 *adv7511,
> bool process_hpd)
> {
> unsigned int irq0, irq1;
> int ret;
> + int cec_status = IRQ_NONE;
> + int irq_status = IRQ_NONE;
>
> ret = regmap_read(adv7511->regmap
Re: [PATCH 1/2] drm/amdgpu: make duplicated EOP packet for GFX7/8 have real content
在 2024-06-17星期一的 14:35 +0200，Christian König写道：
> Am 17.06.24 um 12:58 schrieb Icenowy Zheng:
> > The duplication of EOP packets for GFX7/8, with the former one have
> > seq-1 written and the latter one have seq written, seems to confuse
> > some
> > hardware platform (e.g. Loongson 7A series PCIe controllers).
> >
> > Make the content of the duplicated EOP packet the same with the
> > real
> > one, only masking any possible interrupts.
>
> Well completely NAK to that, exactly that disables the workaround.
>
> The CPU needs to see two different values written here.
Why do the CPU need to see two different values here? Only the second
packet will raise an interrupt before and after applying this patch,
and the first packet's result should just be overriden on ordinary
platforms. The CPU won't see the first one, until it's polling for the
address for a very short interval, so short that the GPU CP couldn't
execute 2 commands.
Or do you mean the GPU needs to see two different values being written,
or they will be merged into only one write request?
Please give out more information about this workaround, otherwise the
GPU hang problem on Loongson platforms will persist.
>
> Regards,
> Christian.
>
> >
> > Fixes: bf26da927a1c ("drm/amdgpu: add cache flush workaround to
> > gfx8 emit_fence")
> > Fixes: a2e73f56fa62 ("drm/amdgpu: Add support for CIK parts")
> > Signed-off-by: Icenowy Zheng
> > ---
> > drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 12 +---
> > drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 12
> > 2 files changed, 9 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > index 541dbd70d8c75..778f27f1a34fe 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > @@ -2117,9 +2117,8 @@ static void
> > gfx_v7_0_ring_emit_fence_gfx(struct amdgpu_ring *ring, u64 addr,
> > {
> > bool write64bit = flags & AMDGPU_FENCE_FLAG_64BIT;
> > bool int_sel = flags & AMDGPU_FENCE_FLAG_INT;
> > - /* Workaround for cache flush problems. First send a dummy
> > EOP
> > - * event down the pipe with seq one below.
> > - */
> > +
> > + /* Workaround for cache flush problems, send EOP twice. */
> > amdgpu_ring_write(ring, PACKET3(PACKET3_EVENT_WRITE_EOP,
> > 4));
> > amdgpu_ring_write(ring, (EOP_TCL1_ACTION_EN |
> > EOP_TC_ACTION_EN |
> > @@ -2127,11 +2126,10 @@ static void
> > gfx_v7_0_ring_emit_fence_gfx(struct amdgpu_ring *ring, u64 addr,
> > EVENT_INDEX(5)));
> > amdgpu_ring_write(ring, addr & 0xfffc);
> > amdgpu_ring_write(ring, (upper_32_bits(addr) & 0x) |
> > - DATA_SEL(1) | INT_SEL(0));
> > - amdgpu_ring_write(ring, lower_32_bits(seq - 1));
> > - amdgpu_ring_write(ring, upper_32_bits(seq - 1));
> > + DATA_SEL(write64bit ? 2 : 1) |
> > INT_SEL(0));
> > + amdgpu_ring_write(ring, lower_32_bits(seq));
> > + amdgpu_ring_write(ring, upper_32_bits(seq));
> >
> > - /* Then send the real EOP event down the pipe. */
> > amdgpu_ring_write(ring, PACKET3(PACKET3_EVENT_WRITE_EOP,
> > 4));
> > amdgpu_ring_write(ring, (EOP_TCL1_ACTION_EN |
> > EOP_TC_ACTION_EN |
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > index 2f0e72caee1af..39a7d60f1fd69 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > @@ -6153,9 +6153,7 @@ static void
> > gfx_v8_0_ring_emit_fence_gfx(struct amdgpu_ring *ring, u64 addr,
> > bool write64bit = flags & AMDGPU_FENCE_FLAG_64BIT;
> > bool int_sel = flags & AMDGPU_FENCE_FLAG_INT;
> >
> > - /* Workaround for cache flush problems. First send a dummy
> > EOP
> > - * event down the pipe with seq one below.
> > - */
> > + /* Workaround for cache flush problems, send EOP twice. */
> > amdgpu_ring_write(ring, PACKET3(PACKET3_EVENT_WRITE_EOP,
> > 4));
> > amdgpu_ring_write(ring, (EOP_TCL1_ACTION_EN |
> > EOP_TC_ACTION_EN |
> > @@ -6164,12 +6162,10 @@ static void
> > gfx_v8_0_ring_emit_fence_gfx(struct amdgpu_ring *ring, u64 addr,
> > EVENT_INDEX(5)));
> > amdgpu_ring_write(ring, addr & 0xfffc);
> > amdgpu_ring_write(ring, (upper_32_bits(addr) & 0x) |
> > - DATA_SEL(1) | INT_SEL(0));
> > - amdgpu_ring_write(ring, lower_32_bits(seq - 1));
> > - amdgpu_ring_write(ring, upper_32_bits(seq - 1));
> > + DATA_SEL(write64bit ? 2 : 1) |
> > INT_SEL(0));
> > + amdgpu_ring_write(ring, lower_32_bits(seq));
> > + amdgpu_ring_write(ring, upper_32_bits(seq));
> >
Re: [PATCH v2 7/7] drm/panic: Add support for drawing a monochrome graphical logo
On 17/06/2024 14:49, Geert Uytterhoeven wrote:
Hi Jocelyn,
On Mon, Jun 17, 2024 at 11:59 AM Jocelyn Falempe wrote:
On 13/06/2024 21:18, Geert Uytterhoeven wrote:
Re-use the existing support for boot-up logos to draw a monochrome
graphical logo in the DRM panic handler. When no suitable graphical
logo is available, the code falls back to the ASCII art penguin logo.
Note that all graphical boot-up logos are freed during late kernel
initialization, hence a copy must be made for later use.
Signed-off-by: Geert Uytterhoeven
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
PANIC_LINE(" \\___)=(___/"),
};
+#ifdef CONFIG_LOGO
+static const struct linux_logo *logo_mono;
+
+static int drm_panic_setup_logo(void)
+{
+ const struct linux_logo *logo = fb_find_logo(1);
+ const unsigned char *logo_data;
+ struct linux_logo *logo_dup;
+
+ if (!logo || logo->type != LINUX_LOGO_MONO)
+ return 0;
+
+ /* The logo is __init, so we must make a copy for later use */
+ logo_data = kmemdup(logo->data,
+ size_mul(DIV_ROUND_UP(logo->width, BITS_PER_BYTE),
logo->height),
+ GFP_KERNEL);
+ if (!logo_data)
+ return -ENOMEM;
+
+ logo_dup = kmemdup(logo, sizeof(*logo), GFP_KERNEL);
+ if (!logo_dup) {
+ kfree(logo_data);
+ return -ENOMEM;
+ }
+
+ logo_dup->data = logo_data;
+ logo_mono = logo_dup;
+
+ return 0;
+}
+
+device_initcall(drm_panic_setup_logo);
+#else
+#define logo_mono((const struct linux_logo *)NULL)
+#endif
I didn't notice that the first time, but the core drm can be built as a
module.
That means this will leak memory each time the module is removed.
While I hadn't considered a modular DRM core, there is no memory leak:
after the logos are freed (from late_initcall_sync()), fb_find_logo()
returns NULL. This does mean there won't be a graphical logo on panic,
though.
Yes, you're right, thanks for the precision.
But to solve the circular dependency between drm_kms_helper and
drm_panic, one solution is to depends on drm core to be built-in.
In this case there won't be a leak.
So depending on how we solve the circular dependency, it can be acceptable.
So far there is no reason to select DRM_KMS_HELPER, right?
The current version doesn't need DRM_KMS_HELPER.
But for example, it uses struct drm_rect, and a few functions
(drm_rect_width()) that are defined in .h, but other drm_rect_*
functions are defined in DRM_KMS_HELPER.
And as you pointed out in your patch, it duplicates the
drm_fb_clip_offset(). So I'm not sure if it can avoid the dependency on
DRM_KMS_HELPER in the long term.
Gr{oetje,eeting}s,
Geert
Re: [PATCH 1/2] drm/amdgpu: make duplicated EOP packet for GFX7/8 have real content
Am 17.06.24 um 15:03 schrieb Icenowy Zheng:
在 2024-06-17星期一的 14:35 +0200，Christian König写道：
Am 17.06.24 um 12:58 schrieb Icenowy Zheng:
The duplication of EOP packets for GFX7/8, with the former one have
seq-1 written and the latter one have seq written, seems to confuse
some
hardware platform (e.g. Loongson 7A series PCIe controllers).
Make the content of the duplicated EOP packet the same with the
real
one, only masking any possible interrupts.
Well completely NAK to that, exactly that disables the workaround.
The CPU needs to see two different values written here.
Why do the CPU need to see two different values here? Only the second
packet will raise an interrupt before and after applying this patch,
and the first packet's result should just be overriden on ordinary
platforms. The CPU won't see the first one, until it's polling for the
address for a very short interval, so short that the GPU CP couldn't
execute 2 commands.
Yes exactly that. We need to make two writes, one with the old value
(seq - 1) and a second with the real value (seq).
Otherwise it is possible that a polling CPU would see the sequence
before the second EOP is issued with results in incoherent view of memory.
Or do you mean the GPU needs to see two different values being written,
or they will be merged into only one write request?
Please give out more information about this workaround, otherwise the
GPU hang problem on Loongson platforms will persist.
Well if Loongson can't handle two consecutive write operations to the
same address with different values then you have a massive platform bug.
That is something which can happen all the time throughout the operation.
Regards,
Christian.
Regards,
Christian.
Fixes: bf26da927a1c ("drm/amdgpu: add cache flush workaround to
gfx8 emit_fence")
Fixes: a2e73f56fa62 ("drm/amdgpu: Add support for CIK parts")
Signed-off-by: Icenowy Zheng
---
drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 12 +---
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 12
2 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index 541dbd70d8c75..778f27f1a34fe 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -2117,9 +2117,8 @@ static void
gfx_v7_0_ring_emit_fence_gfx(struct amdgpu_ring *ring, u64 addr,
{
bool write64bit = flags & AMDGPU_FENCE_FLAG_64BIT;
bool int_sel = flags & AMDGPU_FENCE_FLAG_INT;
- /* Workaround for cache flush problems. First send a dummy
EOP
- * event down the pipe with seq one below.
- */
+
+ /* Workaround for cache flush problems, send EOP twice. */
amdgpu_ring_write(ring, PACKET3(PACKET3_EVENT_WRITE_EOP,
4));
amdgpu_ring_write(ring, (EOP_TCL1_ACTION_EN |
EOP_TC_ACTION_EN |
@@ -2127,11 +2126,10 @@ static void
gfx_v7_0_ring_emit_fence_gfx(struct amdgpu_ring *ring, u64 addr,
EVENT_INDEX(5)));
amdgpu_ring_write(ring, addr & 0xfffc);
amdgpu_ring_write(ring, (upper_32_bits(addr) & 0x) |
- DATA_SEL(1) | INT_SEL(0));
- amdgpu_ring_write(ring, lower_32_bits(seq - 1));
- amdgpu_ring_write(ring, upper_32_bits(seq - 1));
+ DATA_SEL(write64bit ? 2 : 1) |
INT_SEL(0));
+ amdgpu_ring_write(ring, lower_32_bits(seq));
+ amdgpu_ring_write(ring, upper_32_bits(seq));
- /* Then send the real EOP event down the pipe. */
amdgpu_ring_write(ring, PACKET3(PACKET3_EVENT_WRITE_EOP,
4));
amdgpu_ring_write(ring, (EOP_TCL1_ACTION_EN |
EOP_TC_ACTION_EN |
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 2f0e72caee1af..39a7d60f1fd69 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -6153,9 +6153,7 @@ static void
gfx_v8_0_ring_emit_fence_gfx(struct amdgpu_ring *ring, u64 addr,
bool write64bit = flags & AMDGPU_FENCE_FLAG_64BIT;
bool int_sel = flags & AMDGPU_FENCE_FLAG_INT;
- /* Workaround for cache flush problems. First send a dummy
EOP
- * event down the pipe with seq one below.
- */
+ /* Workaround for cache flush problems, send EOP twice. */
amdgpu_ring_write(ring, PACKET3(PACKET3_EVENT_WRITE_EOP,
4));
amdgpu_ring_write(ring, (EOP_TCL1_ACTION_EN |
EOP_TC_ACTION_EN |
@@ -6164,12 +6162,10 @@ static void
gfx_v8_0_ring_emit_fence_gfx(struct amdgpu_ring *ring, u64 addr,
EVENT_INDEX(5)));
amdgpu_ring_write(ring, addr & 0xfffc);
amdgpu_ring_write(ring, (upper_32_bits(addr) & 0x) |
- DATA_SEL(1) | INT_SEL(0));
- amdgpu_ring_write(ring, lower_32_bits(seq - 1));
- amdgpu_
Re: [PATCH V2] drm/bridge: adv7511: Fix Intermittent EDID failures
On Mon, Jun 17, 2024 at 8:00 AM Linux regression tracking (Thorsten
Leemhuis) wrote:
>
> [CCing the regression list, as it should be in the loop for regressions:
> https://docs.kernel.org/admin-guide/reporting-regressions.html]
>
> Hi! Top-posting for once, to make this easily accessible to everyone.
>
> Hmm, seem nobody took a look at below fix for a regression that seems to
> be caused by f3d9683346d6b1 ("drm/bridge: adv7511: Allow IRQ to share
> GPIO pins") [which went into v6.10-rc1].
>
> Adam and Dimitry, what are your stances on this patch from Adam? I'm
> asking, as you authored respectively committed the culprit?
>
I learned of the regression from Liu Ying when he posted a proposed
fix [1] which then resulted in some further discussion on how to
better solve the issue. I felt like I should be the one to fix the
issue, since I accidentally caused it when trying to allow for shared
GPIO IRQ's. The shared GPIO IRQ was required on the imx8mp-beacon-kit
because the interrupt GPIO for the hot-plug-detect IRQ is shared with
a GPIO expander which also serves as an interrupt controller.
Dimitry had given me some suggestions, and from that, I posted a V1.
Dmitry had some more followup suggestions [2] which resulted in the
V2.
As far as I know, Liu was satisfied that this addressed the regression
he reported.
adam
[1] -
https://lore.kernel.org/linux-kernel/2f41a890-915b-4859-8ac9-5436da14f...@nxp.com/T/
[2] -
https://lore.kernel.org/all/7bb4d582-5d90-465e-a241-1ee8439a5...@nxp.com/T/
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
>
> #regzbot poke
>
> On 01.06.24 15:24, Adam Ford wrote:
> > In the process of adding support for shared IRQ pins, a scenario
> > was accidentally created where adv7511_irq_process returned
> > prematurely causing the EDID to fail randomly.
> >
> > Since the interrupt handler is broken up into two main helper functions,
> > update both of them to treat the helper functions as IRQ handlers. These
> > IRQ routines process their respective tasks as before, but if they
> > determine that actual work was done, mark the respective IRQ status
> > accordingly, and delay the check until everything has been processed.
> >
> > This should guarantee the helper functions don't return prematurely
> > while still returning proper values of either IRQ_HANDLED or IRQ_NONE.
> >
> > Reported-by: Liu Ying
> > Fixes: f3d9683346d6 ("drm/bridge: adv7511: Allow IRQ to share GPIO pins")
> > Signed-off-by: Adam Ford
> > Tested-by: Liu Ying # i.MX8MP EVK ADV7535 EDID
> > retrieval w/o IRQ
> > ---
> > V2: Fix uninitialized cec_status
> > Cut back a little on error handling to return either IRQ_NONE or
> > IRQ_HANDLED.
> >
> > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> > b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> > index ea271f62b214..ec0b7f3d889c 100644
> > --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> > @@ -401,7 +401,7 @@ struct adv7511 {
> >
> > #ifdef CONFIG_DRM_I2C_ADV7511_CEC
> > int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511);
> > -void adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1);
> > +int adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1);
> > #else
> > static inline int adv7511_cec_init(struct device *dev, struct adv7511
> > *adv7511)
> > {
> > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
> > b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
> > index 44451a9658a3..651fb1dde780 100644
> > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
> > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
> > @@ -119,7 +119,7 @@ static void adv7511_cec_rx(struct adv7511 *adv7511, int
> > rx_buf)
> > cec_received_msg(adv7511->cec_adap, &msg);
> > }
> >
> > -void adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1)
> > +int adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1)
> > {
> > unsigned int offset = adv7511->info->reg_cec_offset;
> > const u32 irq_tx_mask = ADV7511_INT1_CEC_TX_READY |
> > @@ -130,17 +130,21 @@ void adv7511_cec_irq_process(struct adv7511 *adv7511,
> > unsigned int irq1)
> > ADV7511_INT1_CEC_RX_READY3;
> > unsigned int rx_status;
> > int rx_order[3] = { -1, -1, -1 };
> > - int i;
> > + int i, ret = 0;
> > + int irq_status = IRQ_NONE;
> >
> > - if (irq1 & irq_tx_mask)
> > + if (irq1 & irq_tx_mask) {
> > adv_cec_tx_raw_status(adv7511, irq1);
> > + irq_status = IRQ_HANDLED;
> > + }
> >
> > if (!(irq1 & irq_rx_mask))
> > - return;
> > + return irq_status;
> >
> > - if (regmap_read(adv7511->regmap_cec,
> > - A
Re: [PATCH 04/15] net: hbl_cn: QP state machine
On Thu, Jun 13, 2024 at 11:21:57AM +0300, Omer Shpigelman wrote:
> Add a common QP state machine which handles the moving for a QP from one
> state to another including performing necessary checks, draining
> in-flight transactions, invalidating caches and error reporting.
>
> Signed-off-by: Omer Shpigelman
> Co-developed-by: Abhilash K V
> Signed-off-by: Abhilash K V
> Co-developed-by: Andrey Agranovich
> Signed-off-by: Andrey Agranovich
> Co-developed-by: Bharat Jauhari
> Signed-off-by: Bharat Jauhari
> Co-developed-by: David Meriin
> Signed-off-by: David Meriin
> Co-developed-by: Sagiv Ozeri
> Signed-off-by: Sagiv Ozeri
> Co-developed-by: Zvika Yehudai
> Signed-off-by: Zvika Yehudai
> ---
> .../ethernet/intel/hbl_cn/common/hbl_cn_qp.c | 480 +-
> 1 file changed, 479 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/hbl_cn/common/hbl_cn_qp.c
> b/drivers/net/ethernet/intel/hbl_cn/common/hbl_cn_qp.c
> index 9ddc23bf8194..26ebdf448193 100644
> --- a/drivers/net/ethernet/intel/hbl_cn/common/hbl_cn_qp.c
> +++ b/drivers/net/ethernet/intel/hbl_cn/common/hbl_cn_qp.c
> @@ -6,8 +6,486 @@
<...>
> +/* The following table represents the (valid) operations that can be
> performed on
> + * a QP in order to move it from one state to another
> + * For example: a QP in RTR state can be moved to RTS state using the
> CN_QP_OP_RTR_2RTS
> + * operation.
> + */
> +static const enum hbl_cn_qp_state_op
> qp_valid_state_op[CN_QP_NUM_STATE][CN_QP_NUM_STATE] = {
> + [CN_QP_STATE_RESET] = {
> + [CN_QP_STATE_RESET] = CN_QP_OP_2RESET,
> + [CN_QP_STATE_INIT] = CN_QP_OP_RST_2INIT,
> + [CN_QP_STATE_SQD] = CN_QP_OP_NOP,
> + [CN_QP_STATE_QPD] = CN_QP_OP_NOP,
> + },
> + [CN_QP_STATE_INIT] = {
> + [CN_QP_STATE_RESET] = CN_QP_OP_2RESET,
> + [CN_QP_STATE_ERR] = CN_QP_OP_2ERR,
> + [CN_QP_STATE_INIT] = CN_QP_OP_NOP,
> + [CN_QP_STATE_RTR] = CN_QP_OP_INIT_2RTR,
> + [CN_QP_STATE_SQD] = CN_QP_OP_NOP,
> + [CN_QP_STATE_QPD] = CN_QP_OP_NOP,
> + },
> + [CN_QP_STATE_RTR] = {
> + [CN_QP_STATE_RESET] = CN_QP_OP_2RESET,
> + [CN_QP_STATE_ERR] = CN_QP_OP_2ERR,
> + [CN_QP_STATE_RTR] = CN_QP_OP_RTR_2RTR,
> + [CN_QP_STATE_RTS] = CN_QP_OP_RTR_2RTS,
> + [CN_QP_STATE_SQD] = CN_QP_OP_NOP,
> + [CN_QP_STATE_QPD] = CN_QP_OP_RTR_2QPD,
> + },
> + [CN_QP_STATE_RTS] = {
> + [CN_QP_STATE_RESET] = CN_QP_OP_2RESET,
> + [CN_QP_STATE_ERR] = CN_QP_OP_2ERR,
> + [CN_QP_STATE_RTS] = CN_QP_OP_RTS_2RTS,
> + [CN_QP_STATE_SQD] = CN_QP_OP_RTS_2SQD,
> + [CN_QP_STATE_QPD] = CN_QP_OP_RTS_2QPD,
> + [CN_QP_STATE_SQERR] = CN_QP_OP_RTS_2SQERR,
> + },
> + [CN_QP_STATE_SQD] = {
> + [CN_QP_STATE_RESET] = CN_QP_OP_2RESET,
> + [CN_QP_STATE_ERR] = CN_QP_OP_2ERR,
> + [CN_QP_STATE_SQD] = CN_QP_OP_SQD_2SQD,
> + [CN_QP_STATE_RTS] = CN_QP_OP_SQD_2RTS,
> + [CN_QP_STATE_QPD] = CN_QP_OP_SQD_2QPD,
> + [CN_QP_STATE_SQERR] = CN_QP_OP_SQD_2SQ_ERR,
> + },
> + [CN_QP_STATE_QPD] = {
> + [CN_QP_STATE_RESET] = CN_QP_OP_2RESET,
> + [CN_QP_STATE_ERR] = CN_QP_OP_2ERR,
> + [CN_QP_STATE_SQD] = CN_QP_OP_NOP,
> + [CN_QP_STATE_QPD] = CN_QP_OP_NOP,
> + [CN_QP_STATE_RTR] = CN_QP_OP_QPD_2RTR,
> + },
> + [CN_QP_STATE_SQERR] = {
> + [CN_QP_STATE_RESET] = CN_QP_OP_2RESET,
> + [CN_QP_STATE_ERR] = CN_QP_OP_2ERR,
> + [CN_QP_STATE_SQD] = CN_QP_OP_SQ_ERR_2SQD,
> + [CN_QP_STATE_SQERR] = CN_QP_OP_NOP,
> + },
> + [CN_QP_STATE_ERR] = {
> + [CN_QP_STATE_RESET] = CN_QP_OP_2RESET,
> + [CN_QP_STATE_ERR] = CN_QP_OP_2ERR,
> + }
> +};
I don't understand why IBTA QP state machine is declared in ETH driver
and not in IB driver.
> +
<...>
> + /* Release lock while we wait before retry.
> + * Note, we can assert that we are already locked.
> + */
> + port_funcs->cfg_unlock(cn_port);
> +
> + msleep(20);
> +
> + port_funcs->cfg_lock(cn_port);
lock/unlock through ops pointer doesn't look like a good idea.
Thanks
Re: [PATCH net-next v12 01/13] netdev: add netdev_rx_queue_restart()
On 6/13/24 02:35, Mina Almasry wrote:
Add netdev_rx_queue_restart() function to netdev_rx_queue.h
see nit below
Reviewed-by: Pavel Begunkov
Signed-off-by: David Wei
Signed-off-by: Mina Almasry
---
v11:
- Fix not checking dev->queue_mgmt_ops (Pavel).
- Fix ndo_queue_mem_free call that passed the wrong pointer (David).
v9: https://lore.kernel.org/all/20240502045410.3524155-4...@davidwei.uk/
(submitted by David).
- fixed SPDX license identifier (Simon).
- Rebased on top of merged queue API definition, and changed
implementation to match that.
- Replace rtnl_lock() with rtnl_is_locked() to make it useable from my
netlink code where rtnl is already locked.
---
include/net/netdev_rx_queue.h | 3 ++
net/core/Makefile | 1 +
net/core/netdev_rx_queue.c| 74 +++
3 files changed, 78 insertions(+)
create mode 100644 net/core/netdev_rx_queue.c
diff --git a/include/net/netdev_rx_queue.h b/include/net/netdev_rx_queue.h
index aa1716fb0e53c..e78ca52d67fbf 100644
--- a/include/net/netdev_rx_queue.h
+++ b/include/net/netdev_rx_queue.h
@@ -54,4 +54,7 @@ get_netdev_rx_queue_index(struct netdev_rx_queue *queue)
return index;
}
#endif
+
+int netdev_rx_queue_restart(struct net_device *dev, unsigned int rxq);
+
#endif
diff --git a/net/core/Makefile b/net/core/Makefile
index 62be9aef25285..f82232b358a2c 100644
--- a/net/core/Makefile
+++ b/net/core/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_NETDEV_ADDR_LIST_TEST) += dev_addr_lists_test.o
obj-y += net-sysfs.o
obj-y += hotdata.o
+obj-y += netdev_rx_queue.o
obj-$(CONFIG_PAGE_POOL) += page_pool.o page_pool_user.o
obj-$(CONFIG_PROC_FS) += net-procfs.o
obj-$(CONFIG_NET_PKTGEN) += pktgen.o
diff --git a/net/core/netdev_rx_queue.c b/net/core/netdev_rx_queue.c
new file mode 100644
index 0..de0575cf6df5d
--- /dev/null
+++ b/net/core/netdev_rx_queue.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include
+#include
+#include
+
+int netdev_rx_queue_restart(struct net_device *dev, unsigned int rxq_idx)
+{
+ void *new_mem, *old_mem;
+ int err;
+
+ if (!dev->queue_mgmt_ops || !dev->queue_mgmt_ops->ndo_queue_stop ||
+ !dev->queue_mgmt_ops->ndo_queue_mem_free ||
+ !dev->queue_mgmt_ops->ndo_queue_mem_alloc ||
+ !dev->queue_mgmt_ops->ndo_queue_start)
+ return -EOPNOTSUPP;
+
+ DEBUG_NET_WARN_ON_ONCE(!rtnl_is_locked());
+
+ new_mem = kvzalloc(dev->queue_mgmt_ops->ndo_queue_mem_size, GFP_KERNEL);
+ if (!new_mem)
+ return -ENOMEM;
+
+ old_mem = kvzalloc(dev->queue_mgmt_ops->ndo_queue_mem_size, GFP_KERNEL);
+ if (!old_mem) {
+ err = -ENOMEM;
+ goto err_free_new_mem;
+ }
+
+ err = dev->queue_mgmt_ops->ndo_queue_mem_alloc(dev, new_mem, rxq_idx);
+ if (err)
+ goto err_free_old_mem;
+
+ err = dev->queue_mgmt_ops->ndo_queue_stop(dev, old_mem, rxq_idx);
+ if (err)
+ goto err_free_new_queue_mem;
+
+ err = dev->queue_mgmt_ops->ndo_queue_start(dev, new_mem, rxq_idx);
+ if (err)
+ goto err_start_queue;
+
+ dev->queue_mgmt_ops->ndo_queue_mem_free(dev, old_mem);
+
+ kvfree(old_mem);
+ kvfree(new_mem);
+
+ return 0;
+
+err_start_queue:
+ /* Restarting the queue with old_mem should be successful as we haven't
+* changed any of the queue configuration, and there is not much we can
+* do to recover from a failure here.
+*
+* WARN if the we fail to recover the old rx queue, and at least free
nit "if the we"
+* old_mem so we don't also leak that.
+*/
+ if (dev->queue_mgmt_ops->ndo_queue_start(dev, old_mem, rxq_idx)) {
+ WARN(1,
+"Failed to restart old queue in error path. RX queue %d may be
unhealthy.",
+rxq_idx);
+ dev->queue_mgmt_ops->ndo_queue_mem_free(dev, old_mem);
+ }
+
+err_free_new_queue_mem:
+ dev->queue_mgmt_ops->ndo_queue_mem_free(dev, new_mem);
+
+err_free_old_mem:
+ kvfree(old_mem);
+
+err_free_new_mem:
+ kvfree(new_mem);
+
+ return err;
+}
--
Pavel Begunkov
[PATCH AUTOSEL 6.9 38/44] drm/amdgpu/pptable: Fix UBSAN array-index-out-of-bounds
From: Tasos Sahanidis
[ Upstream commit c6c4dd54012551cce5cde408b35468f2c62b0cce ]
Flexible arrays used [1] instead of []. Replace the former with the latter
to resolve multiple UBSAN warnings observed on boot with a BONAIRE card.
In addition, use the __counted_by attribute where possible to hint the
length of the arrays to the compiler and any sanitizers.
Signed-off-by: Tasos Sahanidis
Signed-off-by: Alex Deucher
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/amd/include/pptable.h | 91 ++-
1 file changed, 49 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/amd/include/pptable.h
b/drivers/gpu/drm/amd/include/pptable.h
index 2e8e6c9875f6c..f83ace2d7ec30 100644
--- a/drivers/gpu/drm/amd/include/pptable.h
+++ b/drivers/gpu/drm/amd/include/pptable.h
@@ -477,31 +477,30 @@ typedef struct _ATOM_PPLIB_STATE_V2
} ATOM_PPLIB_STATE_V2;
typedef struct _StateArray{
-//how many states we have
-UCHAR ucNumEntries;
-
-ATOM_PPLIB_STATE_V2 states[1];
+ //how many states we have
+ UCHAR ucNumEntries;
+
+ ATOM_PPLIB_STATE_V2 states[] /* __counted_by(ucNumEntries) */;
}StateArray;
typedef struct _ClockInfoArray{
-//how many clock levels we have
-UCHAR ucNumEntries;
-
-//sizeof(ATOM_PPLIB_CLOCK_INFO)
-UCHAR ucEntrySize;
-
-UCHAR clockInfo[];
+ //how many clock levels we have
+ UCHAR ucNumEntries;
+
+ //sizeof(ATOM_PPLIB_CLOCK_INFO)
+ UCHAR ucEntrySize;
+
+ UCHAR clockInfo[];
}ClockInfoArray;
typedef struct _NonClockInfoArray{
+ //how many non-clock levels we have. normally should be same as number
of states
+ UCHAR ucNumEntries;
+ //sizeof(ATOM_PPLIB_NONCLOCK_INFO)
+ UCHAR ucEntrySize;
-//how many non-clock levels we have. normally should be same as number of
states
-UCHAR ucNumEntries;
-//sizeof(ATOM_PPLIB_NONCLOCK_INFO)
-UCHAR ucEntrySize;
-
-ATOM_PPLIB_NONCLOCK_INFO nonClockInfo[];
+ ATOM_PPLIB_NONCLOCK_INFO nonClockInfo[] __counted_by(ucNumEntries);
}NonClockInfoArray;
typedef struct _ATOM_PPLIB_Clock_Voltage_Dependency_Record
@@ -513,8 +512,10 @@ typedef struct _ATOM_PPLIB_Clock_Voltage_Dependency_Record
typedef struct _ATOM_PPLIB_Clock_Voltage_Dependency_Table
{
-UCHAR ucNumEntries;//
Number of entries.
-ATOM_PPLIB_Clock_Voltage_Dependency_Record entries[1]; //
Dynamically allocate entries.
+ // Number of entries.
+ UCHAR ucNumEntries;
+ // Dynamically allocate entries.
+ ATOM_PPLIB_Clock_Voltage_Dependency_Record entries[]
__counted_by(ucNumEntries);
}ATOM_PPLIB_Clock_Voltage_Dependency_Table;
typedef struct _ATOM_PPLIB_Clock_Voltage_Limit_Record
@@ -529,8 +530,10 @@ typedef struct _ATOM_PPLIB_Clock_Voltage_Limit_Record
typedef struct _ATOM_PPLIB_Clock_Voltage_Limit_Table
{
-UCHAR ucNumEntries;//
Number of entries.
-ATOM_PPLIB_Clock_Voltage_Limit_Record entries[1]; //
Dynamically allocate entries.
+ // Number of entries.
+ UCHAR ucNumEntries;
+ // Dynamically allocate entries.
+ ATOM_PPLIB_Clock_Voltage_Limit_Record entries[]
__counted_by(ucNumEntries);
}ATOM_PPLIB_Clock_Voltage_Limit_Table;
union _ATOM_PPLIB_CAC_Leakage_Record
@@ -553,8 +556,10 @@ typedef union _ATOM_PPLIB_CAC_Leakage_Record
ATOM_PPLIB_CAC_Leakage_Record;
typedef struct _ATOM_PPLIB_CAC_Leakage_Table
{
-UCHAR ucNumEntries; //
Number of entries.
-ATOM_PPLIB_CAC_Leakage_Record entries[1]; //
Dynamically allocate entries.
+ // Number of entries.
+ UCHAR ucNumEntries;
+ // Dynamically allocate entries.
+ ATOM_PPLIB_CAC_Leakage_Record entries[] __counted_by(ucNumEntries);
}ATOM_PPLIB_CAC_Leakage_Table;
typedef struct _ATOM_PPLIB_PhaseSheddingLimits_Record
@@ -568,8 +573,10 @@ typedef struct _ATOM_PPLIB_PhaseSheddingLimits_Record
typedef struct _ATOM_PPLIB_PhaseSheddingLimits_Table
{
-UCHAR ucNumEntries; //
Number of entries.
-ATOM_PPLIB_PhaseSheddingLimits_Record entries[1]; //
Dynamically allocate entries.
+ // Number of entries.
+ UCHAR ucNumEntries;
+ // Dynamically allocate entries.
+ ATOM_PPLIB_PhaseSheddingLimits_Record entries[]
__counted_by(ucNumEntries);
}ATOM_PPLIB_PhaseSheddingLimits_Table;
typedef struct _VCEClockInfo{
@@ -580,8 +587,8 @@ typedef struct _VCEClockInfo{
}VCEClockInfo;
typedef struct _VCEClockInfoArray{
-UCHAR ucNumEntries;
-VCEClockInfo entries[1];
+ UCHAR ucNumEntries;
+ VCEClockInfo entries[] __counted_by(ucNumEntries);
}VCEClockInfoArray;
typedef struct _ATOM_PPLIB_VCE_Clock_Voltage_Limit_Record
@@ -592,8 +599,8 @@ typedef struct _ATOM_PPLIB_VCE_Clock_Voltage_Li
Re: [PATCH net-next v12 03/13] netdev: support binding dma-buf to netdevice
On 6/13/24 02:35, Mina Almasry wrote:
Add a netdev_dmabuf_binding struct which represents the
dma-buf-to-netdevice binding. The netlink API will bind the dma-buf to
rx queues on the netdevice. On the binding, the dma_buf_attach
& dma_buf_map_attachment will occur. The entries in the sg_table from
mapping will be inserted into a genpool to make it ready
for allocation.
The chunks in the genpool are owned by a dmabuf_chunk_owner struct which
holds the dma-buf offset of the base of the chunk and the dma_addr of
the chunk. Both are needed to use allocations that come from this chunk.
We create a new type that represents an allocation from the genpool:
net_iov. We setup the net_iov allocation size in the
genpool to PAGE_SIZE for simplicity: to match the PAGE_SIZE normally
allocated by the page pool and given to the drivers.
The user can unbind the dmabuf from the netdevice by closing the netlink
socket that established the binding. We do this so that the binding is
automatically unbound even if the userspace process crashes.
The binding and unbinding leaves an indicator in struct netdev_rx_queue
that the given queue is bound, but the binding doesn't take effect until
the driver actually reconfigures its queues, and re-initializes its page
pool.
The netdev_dmabuf_binding struct is refcounted, and releases its
resources only when all the refs are released.
Signed-off-by: Willem de Bruijn
Signed-off-by: Kaiyuan Zhang
Signed-off-by: Mina Almasry
Apart from the comment below
Reviewed-by: Pavel Begunkov # excluding netlink
diff --git a/include/net/devmem.h b/include/net/devmem.h
new file mode 100644
index 0..eaf3fd965d7a8
...
diff --git a/net/core/dev.c b/net/core/dev.c
index c361a7b69da86..84c9f96a6c9bf 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -158,6 +158,9 @@
#include
#include
#include
+#include
+#include
+#include
#include "dev.h"
#include "net-sysfs.h"
diff --git a/net/core/devmem.c b/net/core/devmem.c
new file mode 100644
index 0..951a06004c430
--- /dev/null
+++ b/net/core/devmem.c
@@ -0,0 +1,252 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Devmem TCP
+ *
+ * Authors: Mina Almasry
+ * Willem de Bruijn
+ * Kaiyuan Zhang
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/* Device memory support */
+
+#if defined(CONFIG_DMA_SHARED_BUFFER) && defined(CONFIG_GENERIC_ALLOCATOR)
+static void net_devmem_dmabuf_free_chunk_owner(struct gen_pool *genpool,
+ struct gen_pool_chunk *chunk,
+ void *not_used)
+{
+ struct dmabuf_genpool_chunk_owner *owner = chunk->owner;
+
+ kvfree(owner->niovs);
+ kfree(owner);
+}
+
+void __net_devmem_dmabuf_binding_free(struct net_devmem_dmabuf_binding
*binding)
+{
+ size_t size, avail;
+
+ gen_pool_for_each_chunk(binding->chunk_pool,
+ net_devmem_dmabuf_free_chunk_owner, NULL);
+
+ size = gen_pool_size(binding->chunk_pool);
+ avail = gen_pool_avail(binding->chunk_pool);
+
+ if (!WARN(size != avail, "can't destroy genpool. size=%zu, avail=%zu",
+ size, avail))
+ gen_pool_destroy(binding->chunk_pool);
+
+ dma_buf_unmap_attachment(binding->attachment, binding->sgt,
+DMA_FROM_DEVICE);
It's unmapped here as DMA_FROM_DEVICE, a fail path does DMA_BIDIRECTIONAL,
dma_buf_map_attachment() passes BIDIRECTIONAL.
+ dma_buf_detach(binding->dmabuf, binding->attachment);
+ dma_buf_put(binding->dmabuf);
+ xa_destroy(&binding->bound_rxq_list);
+ kfree(binding);
+}
+
+/* Protected by rtnl_lock() */
+static DEFINE_XARRAY_FLAGS(net_devmem_dmabuf_bindings, XA_FLAGS_ALLOC1);
+
...
--
Pavel Begunkov
Re: [PATCH v7 3/3] drm/mediatek: Implement OF graphs support for display paths
Hi Angelo,
> +/**
> + * mtk_drm_of_ddp_path_build_one - Build a Display HW Pipeline for a CRTC
> Path
> + * @dev: The mediatek-drm device
> + * @cpath:CRTC Path relative to a VDO or MMSYS
> + * @out_path: Pointer to an array that will contain the new pipeline
> + * @out_path_len: Number of entries in the pipeline array
> + *
> + * MediaTek SoCs can use different DDP hardware pipelines (or paths)
> depending
> + * on the board-specific desired display configuration; this function walks
> + * through all of the output endpoints starting from a VDO or MMSYS hardware
> + * instance and builds the right pipeline as specified in device trees.
> + *
> + * Return:
> + * * %0 - Display HW Pipeline successfully built and validated
> + * * %-ENOENT - Display pipeline was not specified in device tree
> + * * %-EINVAL - Display pipeline built but validation failed
> + * * %-ENOMEM - Failure to allocate pipeline array to pass to the caller
> + */
> +static int mtk_drm_of_ddp_path_build_one(struct device *dev, enum
> mtk_crtc_path cpath,
> + const unsigned int **out_path,
> + unsigned int *out_path_len)
> +{
> + struct device_node *next, *prev, *vdo = dev->parent->of_node;
> + unsigned int temp_path[DDP_COMPONENT_DRM_ID_MAX] = { 0 };
> + unsigned int *final_ddp_path;
> + unsigned short int idx = 0;
> + bool ovl_adaptor_comp_added = false;
> + int ret;
> +
> + /* Get the first entry for the temp_path array */
> + ret = mtk_drm_of_get_ddp_ep_cid(vdo, 0, cpath, &next, &temp_path[idx]);
> + if (ret) {
> + if (next && temp_path[idx] == DDP_COMPONENT_DRM_OVL_ADAPTOR) {
> + dev_err(dev, "Adding OVL Adaptor for %pOF\n", next);
> + ovl_adaptor_comp_added = true;
> + } else {
> + if (next)
> + dev_err(dev, "Invalid component %pOF\n", next);
> + else
> + dev_err(dev, "Cannot find first endpoint for
> path %d\n", cpath);
> +
> + return ret;
> + }
> + }
> + idx++;
> +
> + /*
> + * Walk through port outputs until we reach the last valid mediatek-drm
> component.
> + * To be valid, this must end with an "invalid" component that is a
> display node.
> + */
> + do {
> + prev = next;
> + ret = mtk_drm_of_get_ddp_ep_cid(next, 1, cpath, &next,
> &temp_path[idx]);
> + of_node_put(prev);
> + if (ret) {
> + of_node_put(next);
> + break;
> + }
> +
> + /*
> + * If this is an OVL adaptor exclusive component and one of
> those
> + * was already added, don't add another instance of the generic
> + * DDP_COMPONENT_OVL_ADAPTOR, as this is used only to decide
> whether
> + * to probe that component master driver of which only one
> instance
> + * is needed and possible.
> + */
> + if (temp_path[idx] == DDP_COMPONENT_DRM_OVL_ADAPTOR) {
> + if (!ovl_adaptor_comp_added)
> + ovl_adaptor_comp_added = true;
> + else
> + idx--;
> + }
> + } while (++idx < DDP_COMPONENT_DRM_ID_MAX);
> +
> + /*
> + * The device component might not be disabled: in that case, don't
Sorry there was a typo in my proposal, This should either be
"not be enabled" or "be disabled".
> + * check the last entry and just report that the device is missing.
> + */
> + if (ret == -ENODEV)
> + return ret;
> +
..
> +static int mtk_drm_of_ddp_path_build(struct device *dev, struct device_node
> *node,
> + struct mtk_mmsys_driver_data *data)
> +{
> + struct device_node *ep_node;
> + struct of_endpoint of_ep;
> + bool output_present[MAX_CRTC] = { false };
> + bool valid_output_found = false;
> + int ret;
> +
> + for_each_endpoint_of_node(node, ep_node) {
> + ret = of_graph_parse_endpoint(ep_node, &of_ep);
> + if (ret) {
> + dev_err_probe(dev, ret, "Cannot parse endpoint\n");
> + break;
> + }
> +
> + if (of_ep.id >= MAX_CRTC) {
> + ret = dev_err_probe(dev, -EINVAL,
> + "Invalid endpoint%u number\n",
> of_ep.port);
> + break;
> + }
> +
> + output_present[of_ep.id] = true;
> + }
> +
> + if (ret) {
> + of_node_put(ep_node);
> + return ret;
> + }
> +
> + if (output_present[CRTC_MAIN]) {
> + ret = mtk_drm_of_ddp_path_build_one(dev, CRTC_MAIN,
> + &da
Re: drm/bridge/imx8mp-hdmi-tx: Allow inexact pixel clock frequencies (Was: [PATCH V8 10/12] drm/bridge: imx: add bridge wrapper driver for i.MX8MP DWC HDMI)
On Mon, Jun 17, 2024 at 1:17 AM Dominique MARTINET
wrote:
>
> Adam Ford wrote on Sat, Feb 03, 2024 at 10:52:50AM -0600:
> > From: Lucas Stach
> >
> > Add a simple wrapper driver for the DWC HDMI bridge driver that
> > implements the few bits that are necessary to abstract the i.MX8MP
> > SoC integration.
>
> Hi Lucas, Adam,
> (trimmed ccs a bit)
>
> First, thank you for the effort of upstreaming all of this!! It's really
> appreciated, and with display working I'll really be wanting to upstream
> our DTS as well as soon as I have time (which is going to be a while,
> but better late than never ?)
>
> Until then, it's been a few months but I've got a question on this bit:
>
> > diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> > b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> > new file mode 100644
> > index ..89fc432ac611
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> > +static enum drm_mode_status
> > +imx8mp_hdmi_mode_valid(struct dw_hdmi *dw_hdmi, void *data,
> > +const struct drm_display_info *info,
> > +const struct drm_display_mode *mode)
> > +{
> > + struct imx8mp_hdmi *hdmi = (struct imx8mp_hdmi *)data;
> > +
> > + if (mode->clock < 13500)
> > + return MODE_CLOCK_LOW;
> > +
> > + if (mode->clock > 297000)
> > + return MODE_CLOCK_HIGH;
> > +
> > + if (clk_round_rate(hdmi->pixclk, mode->clock * 1000) !=
> > + mode->clock * 1000)
> > + return MODE_CLOCK_RANGE;
>
> Do you know why such a check is here?
I didn't write the original code, so I'll defer to Lucas here. I just
tried to edit/fix issues as they were identified to get it pushed
upstream.
>
> When plugging in a screen with no frequency identically supported in its
> EDID this check causes the screen to stay black, and we've been telling
> customers to override the EDID but it's a huge pain.
>
> Commit 6ad082bee902 ("phy: freescale: add Samsung HDMI PHY") already
> "fixed" the samsung hdmi phy driver to return the next frequency if an
> exact match hasn't been found (NXP tree's match frequencies exactly, but
> this gets the first clock with pixclk <= rate), so if this check is also
> relaxed our displays would work out of the box.
Are you proposing to replace 'return MODE_CLOCK_RANGE' with a printed warning?
>
> I also don't see any other bridge doing this kind of check.
> drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c has a similar check with a
> 0.5% leeway, and all the other drivers don't check anything.
> If you want to add some level of safety, I think we could make this work
> with a 5% margin easily... Printing a warning in dmesg could work if
> you're worried about artifacts, but litteraly anything is better than a
> black screen with no error message in my opinion.
>
>
> In practice the screen I'm looking at has an EDID which only supports
> 51.2MHz and the closest frequency supported by the Samsung HDMI phy is
> 50.4MHz, so that's a ~1.5% difference and it'd be great if it could work
> out of the box.
I wonder if the HDMI PHY could be improved to better dynamically
calculate values instead of the look tables.
adam
>
> For reference, the output of edid-decode is as follow:
> ---
> edid-decode /sys/devices/platform/display-subsystem/drm/car
> d1/card1-HDMI-A-1/edid
> edid-decode (hex):
>
> 00 ff ff ff ff ff ff 00 3a 49 03 00 01 00 00 00
> 20 1e 01 03 80 10 09 00 0a 00 00 00 00 00 00 00
> 00 00 00 00 00 00 01 01 01 01 01 01 01 01 01 01
> 01 01 01 01 01 01 00 14 00 40 41 58 23 20 a0 20
> c8 00 9a 56 00 00 00 18 00 00 00 10 00 00 00 00
> 00 00 00 00 00 00 00 00 00 00 00 00 00 10 00 00
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 10
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 9a
>
>
>
> Block 0, Base EDID:
> EDID Structure Version & Revision: 1.3
> Vendor & Product Identification:
> Manufacturer: NRI
> Model: 3
> Serial Number: 1
> Made in: week 32 of 2020
> Basic Display Parameters & Features:
> Digital display
> Maximum image size: 16 cm x 9 cm
> Gamma: 1.00
> RGB color display
> First detailed timing is the preferred timing
> Color Characteristics:
> Red : 0., 0.
> Green: 0., 0.
> Blue : 0., 0.
> White: 0., 0.
> Established Timings I & II: none
> Standard Timings: none
> Detailed Timing Descriptors:
> DTD 1: 1024x60059.993 Hz 128:75 38.095 kHz 51.200 MHz (154 mm x
> 86 m
> m)
> Hfront 160 Hsync 32 Hback 128 Hpol N
> Vfront 12 Vsync 8 Vback 15 Vpol N
> Dummy Descriptor:
> Dummy Descriptor:
> Dummy Descriptor:
> Checksum: 0x9a
> ---
>
>
> Thanks,
> --
> Dominique Martinet
>
>
>
> --
> linux-phy mailing list
> linux-...@lists.infradead.org
> https://lists.infradead.org/mailman/listinfo/linux-phy
Re: [PATCH V2] drm/bridge: adv7511: Fix Intermittent EDID failures
On 17.06.24 15:14, Adam Ford wrote:
> On Mon, Jun 17, 2024 at 8:00 AM Linux regression tracking (Thorsten
> Leemhuis) wrote:
>>
>> [CCing the regression list, as it should be in the loop for regressions:
>> https://docs.kernel.org/admin-guide/reporting-regressions.html]
>>
>> Hi! Top-posting for once, to make this easily accessible to everyone.
>>
>> Hmm, seem nobody took a look at below fix for a regression that seems to
>> be caused by f3d9683346d6b1 ("drm/bridge: adv7511: Allow IRQ to share
>> GPIO pins") [which went into v6.10-rc1].
>>
>> Adam and Dimitry, what are your stances on this patch from Adam? I'm
>> asking, as you authored respectively committed the culprit?
>
> I learned of the regression from Liu Ying [...]
Ohh, I'm very sorry, stupid me somehow missed that the Adam that was
posting the fix was the same Adam that authored the culprit. :-( Seems I
definitely need more coffee (or green tea in my case) or reduce the
number or regressions on the stack. Please accept my apologies.
Thx for the update anyway.
> Dimitry had given me some suggestions, and from that, I posted a V1.
> Dmitry had some more followup suggestions [2] which resulted in the
> V2.
>> As far as I know, Liu was satisfied that this addressed the regression
> he reported.
So in that case the main question afaics is why this fix did not make
any progress for more than two weeks now (at least afaics -- or did I
miss something in that area, too?).
Ciao, Thorsten
>> On 01.06.24 15:24, Adam Ford wrote:
>>> In the process of adding support for shared IRQ pins, a scenario
>>> was accidentally created where adv7511_irq_process returned
>>> prematurely causing the EDID to fail randomly.
>>>
>>> Since the interrupt handler is broken up into two main helper functions,
>>> update both of them to treat the helper functions as IRQ handlers. These
>>> IRQ routines process their respective tasks as before, but if they
>>> determine that actual work was done, mark the respective IRQ status
>>> accordingly, and delay the check until everything has been processed.
>>>
>>> This should guarantee the helper functions don't return prematurely
>>> while still returning proper values of either IRQ_HANDLED or IRQ_NONE.
>>>
>>> Reported-by: Liu Ying
>>> Fixes: f3d9683346d6 ("drm/bridge: adv7511: Allow IRQ to share GPIO pins")
>>> Signed-off-by: Adam Ford
>>> Tested-by: Liu Ying # i.MX8MP EVK ADV7535 EDID
>>> retrieval w/o IRQ
>>> ---
>>> V2: Fix uninitialized cec_status
>>> Cut back a little on error handling to return either IRQ_NONE or
>>> IRQ_HANDLED.
>>>
>>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
>>> b/drivers/gpu/drm/bridge/adv7511/adv7511.h
>>> index ea271f62b214..ec0b7f3d889c 100644
>>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
>>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
>>> @@ -401,7 +401,7 @@ struct adv7511 {
>>>
>>> #ifdef CONFIG_DRM_I2C_ADV7511_CEC
>>> int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511);
>>> -void adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1);
>>> +int adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1);
>>> #else
>>> static inline int adv7511_cec_init(struct device *dev, struct adv7511
>>> *adv7511)
>>> {
>>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
>>> b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
>>> index 44451a9658a3..651fb1dde780 100644
>>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
>>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
>>> @@ -119,7 +119,7 @@ static void adv7511_cec_rx(struct adv7511 *adv7511, int
>>> rx_buf)
>>> cec_received_msg(adv7511->cec_adap, &msg);
>>> }
>>>
>>> -void adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1)
>>> +int adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1)
>>> {
>>> unsigned int offset = adv7511->info->reg_cec_offset;
>>> const u32 irq_tx_mask = ADV7511_INT1_CEC_TX_READY |
>>> @@ -130,17 +130,21 @@ void adv7511_cec_irq_process(struct adv7511 *adv7511,
>>> unsigned int irq1)
>>> ADV7511_INT1_CEC_RX_READY3;
>>> unsigned int rx_status;
>>> int rx_order[3] = { -1, -1, -1 };
>>> - int i;
>>> + int i, ret = 0;
>>> + int irq_status = IRQ_NONE;
>>>
>>> - if (irq1 & irq_tx_mask)
>>> + if (irq1 & irq_tx_mask) {
>>> adv_cec_tx_raw_status(adv7511, irq1);
>>> + irq_status = IRQ_HANDLED;
>>> + }
>>>
>>> if (!(irq1 & irq_rx_mask))
>>> - return;
>>> + return irq_status;
>>>
>>> - if (regmap_read(adv7511->regmap_cec,
>>> - ADV7511_REG_CEC_RX_STATUS + offset, &rx_status))
>>> - return;
>>> + ret = regmap_read(adv7511->regmap_cec,
>>> + ADV7511_REG_CEC_RX_STATUS + offset, &rx_status);
>>> + if (ret < 0)
>>> + return irq_status;
>>>
>>> /*
>>>* ADV7511_REG_CEC_RX_STATUS[5:0] contains the reception o
Re: [PATCH V2] drm/bridge: adv7511: Fix Intermittent EDID failures
On Mon, Jun 17, 2024 at 8:29 AM Linux regression tracking (Thorsten
Leemhuis) wrote:
>
> On 17.06.24 15:14, Adam Ford wrote:
> > On Mon, Jun 17, 2024 at 8:00 AM Linux regression tracking (Thorsten
> > Leemhuis) wrote:
> >>
> >> [CCing the regression list, as it should be in the loop for regressions:
> >> https://docs.kernel.org/admin-guide/reporting-regressions.html]
> >>
> >> Hi! Top-posting for once, to make this easily accessible to everyone.
> >>
> >> Hmm, seem nobody took a look at below fix for a regression that seems to
> >> be caused by f3d9683346d6b1 ("drm/bridge: adv7511: Allow IRQ to share
> >> GPIO pins") [which went into v6.10-rc1].
> >>
> >> Adam and Dimitry, what are your stances on this patch from Adam? I'm
> >> asking, as you authored respectively committed the culprit?
> >
> > I learned of the regression from Liu Ying [...]
>
> Ohh, I'm very sorry, stupid me somehow missed that the Adam that was
> posting the fix was the same Adam that authored the culprit. :-( Seems I
> definitely need more coffee (or green tea in my case) or reduce the
> number or regressions on the stack. Please accept my apologies.
>
> Thx for the update anyway.
No problem. Sent out a few e-mails and/or patches while tired and I
when I read them again when I was awake, I had to ask myself 'what
what was I thinking'
>
> > Dimitry had given me some suggestions, and from that, I posted a V1.
> > Dmitry had some more followup suggestions [2] which resulted in the
> > V2.
> >> As far as I know, Liu was satisfied that this addressed the regression
> > he reported.
>
> So in that case the main question afaics is why this fix did not make
> any progress for more than two weeks now (at least afaics -- or did I
> miss something in that area, too?).
I have not seen anything either which is why I sent out the gentle
nudge last week.
adam
>
> Ciao, Thorsten
>
> >> On 01.06.24 15:24, Adam Ford wrote:
> >>> In the process of adding support for shared IRQ pins, a scenario
> >>> was accidentally created where adv7511_irq_process returned
> >>> prematurely causing the EDID to fail randomly.
> >>>
> >>> Since the interrupt handler is broken up into two main helper functions,
> >>> update both of them to treat the helper functions as IRQ handlers. These
> >>> IRQ routines process their respective tasks as before, but if they
> >>> determine that actual work was done, mark the respective IRQ status
> >>> accordingly, and delay the check until everything has been processed.
> >>>
> >>> This should guarantee the helper functions don't return prematurely
> >>> while still returning proper values of either IRQ_HANDLED or IRQ_NONE.
> >>>
> >>> Reported-by: Liu Ying
> >>> Fixes: f3d9683346d6 ("drm/bridge: adv7511: Allow IRQ to share GPIO pins")
> >>> Signed-off-by: Adam Ford
> >>> Tested-by: Liu Ying # i.MX8MP EVK ADV7535 EDID
> >>> retrieval w/o IRQ
> >>> ---
> >>> V2: Fix uninitialized cec_status
> >>> Cut back a little on error handling to return either IRQ_NONE or
> >>> IRQ_HANDLED.
> >>>
> >>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> >>> b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> >>> index ea271f62b214..ec0b7f3d889c 100644
> >>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> >>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> >>> @@ -401,7 +401,7 @@ struct adv7511 {
> >>>
> >>> #ifdef CONFIG_DRM_I2C_ADV7511_CEC
> >>> int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511);
> >>> -void adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1);
> >>> +int adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1);
> >>> #else
> >>> static inline int adv7511_cec_init(struct device *dev, struct adv7511
> >>> *adv7511)
> >>> {
> >>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
> >>> b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
> >>> index 44451a9658a3..651fb1dde780 100644
> >>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
> >>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
> >>> @@ -119,7 +119,7 @@ static void adv7511_cec_rx(struct adv7511 *adv7511,
> >>> int rx_buf)
> >>> cec_received_msg(adv7511->cec_adap, &msg);
> >>> }
> >>>
> >>> -void adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1)
> >>> +int adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1)
> >>> {
> >>> unsigned int offset = adv7511->info->reg_cec_offset;
> >>> const u32 irq_tx_mask = ADV7511_INT1_CEC_TX_READY |
> >>> @@ -130,17 +130,21 @@ void adv7511_cec_irq_process(struct adv7511
> >>> *adv7511, unsigned int irq1)
> >>> ADV7511_INT1_CEC_RX_READY3;
> >>> unsigned int rx_status;
> >>> int rx_order[3] = { -1, -1, -1 };
> >>> - int i;
> >>> + int i, ret = 0;
> >>> + int irq_status = IRQ_NONE;
> >>>
> >>> - if (irq1 & irq_tx_mask)
> >>> + if (irq1 & irq_tx_mask) {
> >>> adv_cec_tx_raw_status(adv7511, irq1);
> >>> + irq_status = IRQ_HANDLED
Re: [PATCH net-next v12 04/13] netdev: netdevice devmem allocator
On 6/13/24 02:35, Mina Almasry wrote: Implement netdev devmem allocator. The allocator takes a given struct netdev_dmabuf_binding as input and allocates net_iov from that binding. The allocation simply delegates to the binding's genpool for the allocation logic and wraps the returned memory region in a net_iov struct. Reviewed-by: Pavel Begunkov Signed-off-by: Willem de Bruijn Signed-off-by: Kaiyuan Zhang Signed-off-by: Mina Almasry --- -- Pavel Begunkov
Re: [PATCH 1/2] drm/amdgpu: make duplicated EOP packet for GFX7/8 have real content
在 2024-06-17星期一的 15:09 +0200，Christian König写道：
> Am 17.06.24 um 15:03 schrieb Icenowy Zheng:
> > 在 2024-06-17星期一的 14:35 +0200，Christian König写道：
> > > Am 17.06.24 um 12:58 schrieb Icenowy Zheng:
> > > > The duplication of EOP packets for GFX7/8, with the former one
> > > > have
> > > > seq-1 written and the latter one have seq written, seems to
> > > > confuse
> > > > some
> > > > hardware platform (e.g. Loongson 7A series PCIe controllers).
> > > >
> > > > Make the content of the duplicated EOP packet the same with the
> > > > real
> > > > one, only masking any possible interrupts.
> > > Well completely NAK to that, exactly that disables the
> > > workaround.
> > >
> > > The CPU needs to see two different values written here.
> > Why do the CPU need to see two different values here? Only the
> > second
> > packet will raise an interrupt before and after applying this
> > patch,
> > and the first packet's result should just be overriden on ordinary
> > platforms. The CPU won't see the first one, until it's polling for
> > the
> > address for a very short interval, so short that the GPU CP
> > couldn't
> > execute 2 commands.
>
> Yes exactly that. We need to make two writes, one with the old value
> (seq - 1) and a second with the real value (seq).
>
> Otherwise it is possible that a polling CPU would see the sequence
> before the second EOP is issued with results in incoherent view of
> memory.
In this case shouldn't we write seq-1 before any work, and then write
seq after work, like what is done in Mesa?
As what I see, Mesa uses another command buffer to emit a
EVENT_WRITE_EOP writing 0, and commit this command buffer before the
real command buffer.
>
> > Or do you mean the GPU needs to see two different values being
> > written,
> > or they will be merged into only one write request?
> >
> > Please give out more information about this workaround, otherwise
> > the
> > GPU hang problem on Loongson platforms will persist.
>
> Well if Loongson can't handle two consecutive write operations to the
> same address with different values then you have a massive platform
> bug.
I think the issue is triggered when two consecutive write operations
and one IRQ is present, which is exactly the case of this function.
>
> That is something which can happen all the time throughout the
> operation.
>
> Regards,
> Christian.
>
> >
> > > Regards,
> > > Christian.
> > >
> > > > Fixes: bf26da927a1c ("drm/amdgpu: add cache flush workaround to
> > > > gfx8 emit_fence")
> > > > Fixes: a2e73f56fa62 ("drm/amdgpu: Add support for CIK parts")
> > > > Signed-off-by: Icenowy Zheng
> > > > ---
> > > > drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 12 +---
> > > > drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 12
> > > > 2 files changed, 9 insertions(+), 15 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > > > b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > > > index 541dbd70d8c75..778f27f1a34fe 100644
> > > > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > > > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > > > @@ -2117,9 +2117,8 @@ static void
> > > > gfx_v7_0_ring_emit_fence_gfx(struct amdgpu_ring *ring, u64
> > > > addr,
> > > > {
> > > > bool write64bit = flags & AMDGPU_FENCE_FLAG_64BIT;
> > > > bool int_sel = flags & AMDGPU_FENCE_FLAG_INT;
> > > > - /* Workaround for cache flush problems. First send a
> > > > dummy
> > > > EOP
> > > > - * event down the pipe with seq one below.
> > > > - */
> > > > +
> > > > + /* Workaround for cache flush problems, send EOP twice.
> > > > */
> > > > amdgpu_ring_write(ring,
> > > > PACKET3(PACKET3_EVENT_WRITE_EOP,
> > > > 4));
> > > > amdgpu_ring_write(ring, (EOP_TCL1_ACTION_EN |
> > > > EOP_TC_ACTION_EN |
> > > > @@ -2127,11 +2126,10 @@ static void
> > > > gfx_v7_0_ring_emit_fence_gfx(struct amdgpu_ring *ring, u64
> > > > addr,
> > > > EVENT_INDEX(5)));
> > > > amdgpu_ring_write(ring, addr & 0xfffc);
> > > > amdgpu_ring_write(ring, (upper_32_bits(addr) & 0x)
> > > > |
> > > > - DATA_SEL(1) | INT_SEL(0));
> > > > - amdgpu_ring_write(ring, lower_32_bits(seq - 1));
> > > > - amdgpu_ring_write(ring, upper_32_bits(seq - 1));
> > > > + DATA_SEL(write64bit ? 2 : 1) |
> > > > INT_SEL(0));
> > > > + amdgpu_ring_write(ring, lower_32_bits(seq));
> > > > + amdgpu_ring_write(ring, upper_32_bits(seq));
> > > >
> > > > - /* Then send the real EOP event down the pipe. */
> > > > amdgpu_ring_write(ring,
> > > > PACKET3(PACKET3_EVENT_WRITE_EOP,
> > > > 4));
> > > > amdgpu_ring_write(ring, (EOP_TCL1_ACTION_EN |
> > > > EOP_TC_ACTION_EN |
> > > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > > > b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > >
Re: [PATCH 1/2] drm/vmwgfx: Fix missing HYPERVISOR_GUEST dependency
On Mon, Jun 17, 2024 at 6:02 AM Borislav Petkov wrote: > > On Mon, Jun 17, 2024 at 11:07:09AM +0200, Borislav Petkov wrote: > > On Sat, Jun 15, 2024 at 06:25:10PM -0700, Alexey Makhalov wrote: > > > VMWARE_HYPERCALL alternative will not work as intended without > > > VMware guest code initialization. > > > > > > Reported-by: kernel test robot > > > Closes: > > > https://lore.kernel.org/oe-kbuild-all/202406152104.fxakp1mb-...@intel.com/ > > > Signed-off-by: Alexey Makhalov > > > --- > > > drivers/gpu/drm/vmwgfx/Kconfig | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/vmwgfx/Kconfig > > > b/drivers/gpu/drm/vmwgfx/Kconfig > > > index faddae3d6ac2..6f1ac940cbae 100644 > > > --- a/drivers/gpu/drm/vmwgfx/Kconfig > > > +++ b/drivers/gpu/drm/vmwgfx/Kconfig > > > @@ -2,7 +2,7 @@ > > > config DRM_VMWGFX > > > tristate "DRM driver for VMware Virtual GPU" > > > depends on DRM && PCI && MMU > > > - depends on X86 || ARM64 > > > + depends on (X86 && HYPERVISOR_GUEST) || ARM64 > > > select DRM_TTM > > > select DRM_TTM_HELPER > > > select MAPPING_DIRTY_HELPERS > > > -- > > > > Right, I'll queue this soon but it doesn't reproduce here with gcc-11 or > > gcc-13. > > This must be something gcc-9 specific or so... > > Actually, that's a DRM patch. > > Folks in To: ok to carry this though the tip tree? That's fine with me. Thanks. z
Correct sequencing of usage of DRM writeback connector
Hi, There is a discussion ongoing over in the compositor world about the implication of this cautionary wording found in the documentation for the DRM_MODE_CONNECTOR_WRITEBACK connectors: > * "WRITEBACK_OUT_FENCE_PTR": > *Userspace can use this property to provide a pointer for the kernel to > *fill with a sync_file file descriptor, which will signal once the > *writeback is finished. The value should be the address of a 32-bit > *signed integer, cast to a u64. > *Userspace should wait for this fence to signal before making another > *commit affecting any of the same CRTCs, Planes or Connectors. > ***Failure to do so will result in undefined behaviour.** > *For this reason it is strongly recommended that all userspace > *applications making use of writeback connectors *always* retrieve an > *out-fence for the commit and use it appropriately. > *From userspace, this property will always read as zero. The question is whether it's realistic to hope that a DRM writeback connector can produce results on every frame, and do so without dragging down the frame-rate for the connector. The wording in the documentation above suggests that it is very likely the fence fd won't signal userspace until after the vblank following the scanout during which the writeback was applied (call that frame N). This would mean that the compositor driving the connector would typically be unable to legally queue a page flip for frame N+1. Is this the right interpretation? Is the writeback hardware typically even designed with a streaming use-case in mind? Maybe it's just intended for occasional static screenshots. Matt Hoosier
Re: [RFC] GPU driver with separate "core" and "DRM" modules
On Fri, Jun 14, 2024 at 03:02:09AM +1000, Ben Skeggs wrote: > NVIDIA has been exploring ways to better support the effort for an > upstream kernel mode driver for GPUs that are capable of running GSP-RM > firmware, since the introduction[1] to Nova. > > Use cases have been identified for which separating the core GPU > programming out of the full DRM driver stack is a strong requirement > from our key customers. > > An upstreamed NVIDIA GPU driver should be able to support current and > emerging customer use cases for vGPU hosts. NVIDIA's vGPU deployments > to date do not support compute or graphics functionality within the > hypervisor host, and have no dependency on the Linux graphics subsystem, > instead implementing the minimal functionality required to run vGPU > guest VMs. > > For security-sensitive environments such as cloud infrastructure, it's > important to continue support for running a minimal footprint vGPU host > driver in a stripped-down / barebones kernel environment. > > This can be achieved by supporting both VFIO and DRM drivers as clients > of a core driver, without requiring a full-fledged DRM driver (or the > DRM subsystem itself) to be built into the host kernel. > > A core driver would be responsible for booting and communicating with > GSP-RM, enumeration of HW configuration, shared/partitioned resource > management, exception handling, and event dispatch. > > The DRM driver would do all the standard things a DRM driver does, and > implement GPU memory management (TTM/HMM), KMS, command submission etc, > as well as providing UAPI for userspace clients. These features would > be implemented using HW resources allocated from a core driver, rather > than the DRM driver being directly responsible for HW programming. > > As Nouveau's KMD is already split (in the logical sense) along similar > lines, we're using it here for the purposes of this RFC to demonstrate > the feasibility of such an architecture, and open it up for discussion. Sounds reasonable. Only bikeshed I have to add is that the blessed way (according to the cool kernel maintainers at least or something) to structure this is using auxbus. Definitely when you end up with more than one driver binding to the core (like maybe some system management interface thing, or perhaps a special compute-only kernel driver). https://dri.freedesktop.org/docs/drm/driver-api/auxiliary_bus.html Cheers, Sima > > A link[2] to a tree containing the patches is below. > > [1] > https://lore.kernel.org/all/3ed356488c9b0ca93845501425d427309f4cf616.ca...@redhat.com/ > [2] https://gitlab.freedesktop.org/bskeggs/nouveau/-/tree/00.03-module > > *** BLURB HERE *** > > Ben Skeggs (2): > drm/nouveau/nvkm: export symbols needed by the drm driver > drm/nouveau/nvkm: separate out into nvkm.ko > > drivers/gpu/drm/nouveau/Kbuild | 4 ++-- > drivers/gpu/drm/nouveau/include/nvkm/core/module.h | 3 --- > drivers/gpu/drm/nouveau/nouveau_drm.c | 10 +- > drivers/gpu/drm/nouveau/nvkm/core/driver.c | 1 + > drivers/gpu/drm/nouveau/nvkm/core/gpuobj.c | 2 ++ > drivers/gpu/drm/nouveau/nvkm/core/mm.c | 4 > drivers/gpu/drm/nouveau/nvkm/device/acpi.c | 1 + > drivers/gpu/drm/nouveau/nvkm/engine/gr/base.c | 1 + > drivers/gpu/drm/nouveau/nvkm/module.c | 8 ++-- > drivers/gpu/drm/nouveau/nvkm/subdev/bios/init.c | 1 + > drivers/gpu/drm/nouveau/nvkm/subdev/bios/pll.c | 1 + > drivers/gpu/drm/nouveau/nvkm/subdev/fb/base.c | 3 +++ > drivers/gpu/drm/nouveau/nvkm/subdev/gpio/base.c | 3 +++ > drivers/gpu/drm/nouveau/nvkm/subdev/i2c/base.c | 2 ++ > drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c | 1 + > drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c | 1 + > drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c| 1 + > drivers/gpu/drm/nouveau/nvkm/subdev/therm/fan.c | 1 + > drivers/gpu/drm/nouveau/nvkm/subdev/volt/base.c | 1 + > 19 files changed, 33 insertions(+), 16 deletions(-) > > -- > 2.44.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: UAPI Re: [PATCH 1/3] drm: Add DRM_MODE_TV_MODE_MONOCHROME
On Fri, Jun 14, 2024 at 11:01:45AM +0200, Daniel Vetter wrote: > On Thu, May 23, 2024 at 04:51:25PM +0200, Maxime Ripard wrote: > > Hi, > > > > Reviving this thread because I'm not sure what the outcome was. > > > > On Thu, Feb 29, 2024 at 11:52:12AM GMT, Daniel Vetter wrote: > > > > The only thing I'm saying is that this breaks the usual DRM > > > > requirements. > > > > If, as a maintainer, you're fine with breaking the rules and have a good > > > > motivation to do so, that's fine by me. Rules are meant to be broken > > > > from > > > > time to time depending on the situation. But please don't pretend that > > > > modetest/xrandr is valid user-space to pass the rules. > > > > > > I think it bends it pretty badly, because people running native Xorg are > > > slowly going away, and the modetest hack does not clear the bar for "is it > > > a joke/test/demo hack" for me. > > > > > > I think some weston (or whatever compositor you like) config file support > > > to set a bunch of "really only way to configure is by hand" output > > > properties would clear the bar here for me. Because that is a feature I > > > already mentioned that xrandr _does_ have, and which modetest hackery very > > > much does not. > > > > The expectation (and general usage) for that property was that it was > > set by the kernel command line and then was forgotten about. Old TVs > > require one mode and that's it, so it doesn't make much sense to change > > it while the system is live, you just want the default to work. > > > > So it's not really a matter of "the user-space code should be open" > > here, there's no user-space code, and there will likely never be given > > that it's mostly used to deal with decades-old systems at this point. > > Yeah if this is being used just with the kernel cmdline, then I guess > that's somewhat ok-ish. And TVs are horrible, so "massage your kernel > cmdline" is comparitively not a horrible interface :-P > > Anyway, I guess this makes this an ack. To clarify, since people asked about this on irc and why it's not a terrible precedence. This is very much just for very, very dead old tech like analog TVs. If people try this escape hatch for hdmi or dp screens, the answer will be completely different :-) -Sima -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 1/2] drm/amdgpu: make duplicated EOP packet for GFX7/8 have real content
Am 17.06.24 um 15:43 schrieb Icenowy Zheng:
在 2024-06-17星期一的 15:09 +0200，Christian König写道：
Am 17.06.24 um 15:03 schrieb Icenowy Zheng:
在 2024-06-17星期一的 14:35 +0200，Christian König写道：
Am 17.06.24 um 12:58 schrieb Icenowy Zheng:
The duplication of EOP packets for GFX7/8, with the former one
have
seq-1 written and the latter one have seq written, seems to
confuse
some
hardware platform (e.g. Loongson 7A series PCIe controllers).
Make the content of the duplicated EOP packet the same with the
real
one, only masking any possible interrupts.
Well completely NAK to that, exactly that disables the
workaround.
The CPU needs to see two different values written here.
Why do the CPU need to see two different values here? Only the
second
packet will raise an interrupt before and after applying this
patch,
and the first packet's result should just be overriden on ordinary
platforms. The CPU won't see the first one, until it's polling for
the
address for a very short interval, so short that the GPU CP
couldn't
execute 2 commands.
Yes exactly that. We need to make two writes, one with the old value
(seq - 1) and a second with the real value (seq).
Otherwise it is possible that a polling CPU would see the sequence
before the second EOP is issued with results in incoherent view of
memory.
In this case shouldn't we write seq-1 before any work, and then write
seq after work, like what is done in Mesa?
No. This hw workaround requires that two consecutive write operations
happen directly behind each other on the PCIe bus with two different values.
To make the software logic around that work without any changes we use
the values seq - 1 and seq because those are guaranteed to be different
and not trigger any unwanted software behavior.
Only then we can guarantee that we have a coherent view of system memory.
As what I see, Mesa uses another command buffer to emit a
EVENT_WRITE_EOP writing 0, and commit this command buffer before the
real command buffer.
Or do you mean the GPU needs to see two different values being
written,
or they will be merged into only one write request?
Please give out more information about this workaround, otherwise
the
GPU hang problem on Loongson platforms will persist.
Well if Loongson can't handle two consecutive write operations to the
same address with different values then you have a massive platform
bug.
I think the issue is triggered when two consecutive write operations
and one IRQ is present, which is exactly the case of this function.
Well then you have a massive platform bug.
Two consecutive writes to the same bus address are perfectly legal from
the PCIe specification and can happen all the time, even without this
specific hw workaround.
Regards,
Christian.
That is something which can happen all the time throughout the
operation.
Regards,
Christian.
Regards,
Christian.
Fixes: bf26da927a1c ("drm/amdgpu: add cache flush workaround to
gfx8 emit_fence")
Fixes: a2e73f56fa62 ("drm/amdgpu: Add support for CIK parts")
Signed-off-by: Icenowy Zheng
---
drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 12 +---
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 12
2 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index 541dbd70d8c75..778f27f1a34fe 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -2117,9 +2117,8 @@ static void
gfx_v7_0_ring_emit_fence_gfx(struct amdgpu_ring *ring, u64
addr,
{
bool write64bit = flags & AMDGPU_FENCE_FLAG_64BIT;
bool int_sel = flags & AMDGPU_FENCE_FLAG_INT;
- /* Workaround for cache flush problems. First send a
dummy
EOP
- * event down the pipe with seq one below.
- */
+
+ /* Workaround for cache flush problems, send EOP twice.
*/
amdgpu_ring_write(ring,
PACKET3(PACKET3_EVENT_WRITE_EOP,
4));
amdgpu_ring_write(ring, (EOP_TCL1_ACTION_EN |
EOP_TC_ACTION_EN |
@@ -2127,11 +2126,10 @@ static void
gfx_v7_0_ring_emit_fence_gfx(struct amdgpu_ring *ring, u64
addr,
EVENT_INDEX(5)));
amdgpu_ring_write(ring, addr & 0xfffc);
amdgpu_ring_write(ring, (upper_32_bits(addr) & 0x)
|
- DATA_SEL(1) | INT_SEL(0));
- amdgpu_ring_write(ring, lower_32_bits(seq - 1));
- amdgpu_ring_write(ring, upper_32_bits(seq - 1));
+ DATA_SEL(write64bit ? 2 : 1) |
INT_SEL(0));
+ amdgpu_ring_write(ring, lower_32_bits(seq));
+ amdgpu_ring_write(ring, upper_32_bits(seq));
- /* Then send the real EOP event down the pipe. */
amdgpu_ring_write(ring,
PACKET3(PACKET3_EVENT_WRITE_EOP,
4));
amdgpu_ring_write(ring, (EOP_TCL1_ACTION_EN |
EOP_TC_ACTION_EN |
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
b/drivers/
Re: [PATCH] drm/fbdev-dma: fix getting smem_start
On Thu, Jun 13, 2024 at 12:18:55PM +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 13.06.24 um 12:14 schrieb Thomas Zimmermann:
> > Hi
> >
> > Am 12.06.24 um 17:00 schrieb Daniel Vetter:
> > > On Wed, Jun 12, 2024 at 10:37:14AM +0200, Thomas Zimmermann wrote:
> > > > Hi Javier
> > > >
> > > > Am 12.06.24 um 09:49 schrieb Javier Martinez Canillas:
> > > > > Thomas Zimmermann writes:
> > > > >
> > > > > Hello Thomas,
> > > > >
> > > > > > Hi
> > > > > >
> > > > > > Am 10.06.24 um 10:47 schrieb Thomas Zimmermann:
> > > > > > > Hi
> > > > > > >
> > > > > > > Am 04.06.24 um 10:03 schrieb Peng Fan (OSS):
> > > > > > > > From: Peng Fan
> > > > > > > >
> > > > > > > > If 'info->screen_buffer' locates in vmalloc
> > > > > > > > address space, virt_to_page
> > > > > > > > will not be able to get correct results. With CONFIG_DEBUG_VM
> > > > > > > > and
> > > > > > > > CONFIG_DEBUG_VIRTUAL enabled on ARM64, there is dump below:
> > > > > > > Which graphics driver triggers this bug?
> > > > > > >
> > > > > > > > [ 3.536043] [ cut here ]
> > > > > > > > [ 3.540716] virt_to_phys used for non-linear address:
> > > > > > > > 7fc4f540 (0x800086001000)
> > > > > > > > [ 3.552628] WARNING: CPU: 4 PID: 61 at
> > > > > > > > arch/arm64/mm/physaddr.c:12
> > > > > > > > __virt_to_phys+0x68/0x98
> > > > > > > > [ 3.565455] Modules linked in:
> > > > > > > > [ 3.568525] CPU: 4 PID: 61 Comm: kworker/u12:5 Not tainted
> > > > > > > > 6.6.23-06226-g4986cc3e1b75-dirty #250
> > > > > > > > [ 3.577310] Hardware name: NXP i.MX95 19X19 board (DT)
> > > > > > > > [ 3.582452] Workqueue: events_unbound
> > > > > > > > deferred_probe_work_func
> > > > > > > > [ 3.588291] pstate: 6049 (nZCv daif +PAN
> > > > > > > > -UAO -TCO -DIT -SSBS
> > > > > > > > BTYPE=--)
> > > > > > > > [ 3.595233] pc : __virt_to_phys+0x68/0x98
> > > > > > > > [ 3.599246] lr : __virt_to_phys+0x68/0x98
> > > > > > > > [ 3.603276] sp : 800083603990
> > > > > > > > [ 3.677939] Call trace:
> > > > > > > > [ 3.680393] __virt_to_phys+0x68/0x98
> > > > > > > > [ 3.684067] drm_fbdev_dma_helper_fb_probe+0x138/0x238
> > > > > > > > [ 3.689214]
> > > > > > > > __drm_fb_helper_initial_config_and_unlock+0x2b0/0x4c0
> > > > > > > > [ 3.695385] drm_fb_helper_initial_config+0x4c/0x68
> > > > > > > > [ 3.700264] drm_fbdev_dma_client_hotplug+0x8c/0xe0
> > > > > > > > [ 3.705161] drm_client_register+0x60/0xb0
> > > > > > > > [ 3.709269] drm_fbdev_dma_setup+0x94/0x148
> > > > > > > >
> > > > > > > > So add a check 'is_vmalloc_addr'.
> > > > > > > >
> > > > > > > > Fixes: b79fe9abd58b ("drm/fbdev-dma: Implement fbdev emulation
> > > > > > > > for
> > > > > > > > GEM DMA helpers")
> > > > > > > > Signed-off-by: Peng Fan
> > > > > > > Reviewed-by: Thomas Zimmermann
> > > > > > I'm taking back my r-b. The memory is expected to by be physically
> > > > > > contiguous and vmalloc() won't guarantee that.
> > > > > >
> > > > > Agreed.
> > > > These smem_ fields are clearly designed for PCI BARs of
> > > > traditional graphics
> > > > cards. So can we even assume contiguous memory for DMA? That was my
> > > > assumption, but with IOMMUs it might not be the case. Fbdev-dma
> > > > only sets
> > > > smem_start to support a single old userspace driver. Maybe we
> > > > should further
> > > > restrict usage of this field by making it opt-in for each driver. Best
> > > > regards Thomas
> > > We could make it all conditional on CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM, and
> > > remove the FBINFO_HIDE_SMEM_START flag. The reason I've done the flag is
> > > that with the old fb_mmap code we had to always fill out smem_start to
> > > make mmap work. But now that the various drm fbdev helpers have all
> > > their
> > > own mmap implementation, we could make this a lot cleaner.
> >
> > Enabling CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM would still crash the NXP
> > driver. I think I'll add a flag to drm_fbdev_dma_setup() to set
> > smem_start from within lima, which is the only driver that requires
> > it.I'd like to remove CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM and all that, but
> > I fear that it would break someone's setup. Best regards Thomas
Yeah we'd always need to make this conditional on the memory not being in
the vmalloc range, or things will blow up.
> I've been looking at
>
> https://lore.kernel.org/dri-devel/20240318-dark-mongoose-of-camouflage-7ac6ed@houat/
>
> and I'm now confused to find that lima doesn't even set up fbdev support.
The mali driver here was the out-of-tree proprietary mali driver as the
consumer of such buffers.
The "exporters" was just any random fbdev driver, and with the DRM option
to set the smem, also drm drivers could play in this role. It at least
seems to have helped a few people to move away from out-of-tree fbdev
drivers to upstream drm drivers (but still with the out-of-tree mali gpu
driver). Which means we've needed this for any display driver that
happens to have
