Re: [PATCH v3 07/11] drm/mediatek: Add secure layer config support for ovl
Re: [PATCH v3 09/11] drm/mediatek: Add secure flow support to mediatek-drm
Re: [v3 4/6] drm/vs: Add KMS crtc
在 2023-12-07星期四的 19:31 +0800，Keith Zhao写道：
>
>
> On 2023/12/7 16:41, Icenowy Zheng wrote:
> > 在 2023-12-04星期一的 20:33 +0800，Keith Zhao写道：
> > *snip*
> >
> > > +static void update_cursor_plane(struct vs_dc *dc, struct
> > > vs_plane
> > > *plane,
> > > + struct drm_plane *drm_plane,
> > > + struct drm_atomic_state
> > > *drm_state)
> > > +{
> > > + struct drm_plane_state *state =
> > > drm_atomic_get_new_plane_state(drm_state,
> > > +
> > >
> > > drm_plane);
> > > + struct vs_plane_state *plane_state =
> > > to_vs_plane_state(state);
> > > + struct drm_framebuffer *drm_fb = state->fb;
> > > + struct dc_hw_cursor cursor;
> > > +
> > > + cursor.address = plane_state->dma_addr[0];
> > > + cursor.x = state->crtc_x;
> > > + cursor.y = state->crtc_y;
> >
> > From my experiments on poking with registers on T-Head TH1520 (also
> > uses DC8200 display controller and a similar driver), the DC8200
> > hardware have a different definition of cursor position X and Y
> > with
> > the CRTC plane state.
> >
> > For CRTC plane state, hot_x and hot_y are only provided as
> > reference,
> > and the cursor should be displayed with its (0,0) drawn to (crtc_x,
> > crtc_y) ([XY]_crtc are values specified in CRTC state, the right
> > part
> > of the assignments here), when the cursor is moved to (0,0) but the
> > hot
> > point is not (0,0), it could be negative.
> >
> > However, for DC8200 registers definition, cursor XY position could
> > not
> > be negative -- the cursor will disappear then; because in its
> > definition, the cursor XY position should be where the cursor is
> > pointing to, instead of its (0,0). DC8200 will draw (0,0) of the
> > cursor
> > to (x - hot_x, y - hot_y). So to met the expectation of the KMS
> > plane
> > settings, the DC8200 position should be set to (crtc_x + hot_x,
> > crtc_y
> > + hot_y) instead. Thus these two lines of code should be:
> >
> > ```
> > cursor.x = state->crtc_x + drm_fb->hot_x;
> > cursor.y = state->crtc_y + drm_fb->hot_y;
> > ```
Well I realized that this change is not correct too: when moving the
mouse cursor with the screen rotated, the mouse cursor will disappear
at some screen border.
My current idea is:
As the CRTC hot point is just a reference, we can just ignore it, and
use the HW hot point to implement negative cursor position.
The patch is like the follow:
-cursor.x = state->crtc_x;
-cursor.y = state->crtc_y;
-cursor.hot_x = drm_fb->hot_x;
-cursor.hot_y = drm_fb->hot_y;
+if (state->crtc_x > 0) {
+cursor.x = state->crtc_x;
+cursor.hot_x = 0;
+} else {
+cursor.hot_x = -state->crtc_x;
+cursor.x = 0;
+}
+if (state->crtc_y > 0) {
+cursor.y = state->crtc_y;
+cursor.hot_y = 0;
+} else {
+cursor.hot_y = -state->crtc_y;
+cursor.y = 0;
+}
drm_fb could just be removed in this function then because it's no
longer needed (it's used to get the cursor's hot point, which we
ignored now).
> >
> >
> > > + cursor.hot_x = drm_fb->hot_x;
> > > + cursor.hot_y = drm_fb->hot_y;
> > > + cursor.display_id = to_vs_display_id(dc, state->crtc);
> > > + update_cursor_size(state, );
> > > + cursor.enable = true;
> > > +
> > > + dc_hw_update_cursor(>hw, cursor.display_id, );
> > > +}
> > *snip
> hello Icenowy:
> you are deep understanding on dc8200.
> by the way of practice
> I tested this change on the debian desktop, is there a way to compare
> the cursor behavior change?
> Thanks
>
>
>
Re: [PATCH v3 08/11] drm/mediatek: Add secure layer config support for ovl_adaptor
Re: [PATCH v3 06/11] drm/mediatek: Add get_sec_port interface to mtk_ddp_comp
Re: [PATCH v3 01/11] drm/mediatek: Add interface to allocate MediaTek GEM buffer.
Re: [PATCH v2 0/1] Implementation of resource_query_layout
On Thu, Dec 21, 2023 at 2:01 AM Julia Zhang wrote: > Hi all, > > Sorry to late reply. This is v2 of the implementation of > resource_query_layout. This adds a new ioctl to let guest query information > of host resource, which is originally from Daniel Stone. We add some > changes to support query the correct stride of host resource before it's > created, which is to support to blit data from dGPU to virtio iGPU for dGPU > prime feature. > > Changes from v1 to v2: > -Squash two patches to a single patch. > -A small modification of VIRTIO_GPU_F_RESOURCE_QUERY_LAYOUT > > > Below is description of v1: > This add implementation of resource_query_layout to get the information of > how the host has actually allocated the buffer. This function is now used > to query the stride for guest linear resource for dGPU prime on guest VMs. > You can use a context specific protocol or even the virgl capabilities [for a linear strided resource]. For example, Sommelier does the following: https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/vm_tools/sommelier/virtualization/virtgpu_channel.cc#549 i.e, you should be able to avoid extra ioctl + hypercall. > > v1 of kernel side: > https: > // > lore.kernel.org/xen-devel/20231110074027.24862-1-julia.zh...@amd.com/T/#t > > v1 of qemu side: > https: > // > lore.kernel.org/qemu-devel/20231110074027.24862-1-julia.zh...@amd.com/T/#t > > Daniel Stone (1): > drm/virtio: Implement RESOURCE_GET_LAYOUT ioctl > > drivers/gpu/drm/virtio/virtgpu_drv.c | 1 + > drivers/gpu/drm/virtio/virtgpu_drv.h | 22 - > drivers/gpu/drm/virtio/virtgpu_ioctl.c | 66 ++ > drivers/gpu/drm/virtio/virtgpu_kms.c | 8 +++- > drivers/gpu/drm/virtio/virtgpu_vq.c| 63 > include/uapi/drm/virtgpu_drm.h | 21 > include/uapi/linux/virtio_gpu.h| 30 > 7 files changed, 208 insertions(+), 3 deletions(-) > > -- > 2.34.1 > >
Re: [PATCH v2 2/4] drm/panel: Add driver for BOE TH101MB31IG002-28A panel
On 12/23/2023 7:20 AM, Manuel Traut wrote:
From: Alexander Warnecke
The BOE TH101MB31IG002-28A panel is a WXGA panel.
It is used in Pine64 Pinetab2 and PinetabV.
Signed-off-by: Alexander Warnecke
Signed-off-by: Manuel Traut
---
drivers/gpu/drm/panel/Kconfig | 11 +
drivers/gpu/drm/panel/Makefile | 1 +
.../gpu/drm/panel/panel-boe-th101mb31ig002-28a.c | 348 +
3 files changed, 360 insertions(+)
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 99e14dc212ec..927ddd10e688 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -67,6 +67,17 @@ config DRM_PANEL_BOE_HIMAX8279D
24 bit RGB per pixel. It provides a MIPI DSI interface to
the host and has a built-in LED backlight.
+config DRM_PANEL_BOE_TH101MB31UIG002_28A
+ tristate "Boe TH101MB31UIG002-28A panel"
+ depends on OF
+ depends on DRM_MIPI_DSI
+ depends on BACKLIGHT_CLASS_DEVICE
+ help
+ Say Y here if you want to enable support for Boe
+ TH101MB31UIG002-28A TFT-LCD modules. The panel has a 800x1280
+ resolution and uses 24 bit RGB per pixel. It provides a MIPI DSI
+ interface to the host and has a built-in LED backlight.
+
config DRM_PANEL_BOE_TV101WUM_NL6
tristate "BOE TV101WUM and AUO KD101N80 45NA 1200x1920 panel"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index d10c3de51c6d..dd6e1ac9d0a2 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_DRM_PANEL_ASUS_Z00T_TM5P5_NT35596) +=
panel-asus-z00t-tm5p5-n35596.
obj-$(CONFIG_DRM_PANEL_AUO_A030JTN01) += panel-auo-a030jtn01.o
obj-$(CONFIG_DRM_PANEL_BOE_BF060Y8M_AJ0) += panel-boe-bf060y8m-aj0.o
obj-$(CONFIG_DRM_PANEL_BOE_HIMAX8279D) += panel-boe-himax8279d.o
+obj-$(CONFIG_DRM_PANEL_BOE_TH101MB31UIG002_28A) +=
panel-boe-th101mb31ig002-28a.o
obj-$(CONFIG_DRM_PANEL_BOE_TV101WUM_NL6) += panel-boe-tv101wum-nl6.o
obj-$(CONFIG_DRM_PANEL_DSI_CM) += panel-dsi-cm.o
obj-$(CONFIG_DRM_PANEL_LVDS) += panel-lvds.o
diff --git a/drivers/gpu/drm/panel/panel-boe-th101mb31ig002-28a.c
b/drivers/gpu/drm/panel/panel-boe-th101mb31ig002-28a.c
new file mode 100644
index ..ffe4047b7434
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-boe-th101mb31ig002-28a.c
@@ -0,0 +1,348 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2023 Alexander Warnecke
+ * Copyright (c) 2023 Manuel Traut
+ * Copyright (c) 2023 Dang Huynh
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+struct boe_th101mb31ig002 {
+ struct drm_panel panel;
+ bool enabled;
+ bool prepared;
Hi Manuel,
Sorry, I responded to the v1 instead of the latest version. Carrying my
comment over to here:
If I remember correctly, commit d2aacaf07395bd798373cbec6af05fff4147aff3
should have introduced prepared/enabled do the drm_panel struct.
Thanks,
Jessica Zhang
+
+ struct mipi_dsi_device *dsi;
+
+ struct regulator *power;
+ struct gpio_desc *enable;
+ struct gpio_desc *reset;
+
+ enum drm_panel_orientation orientation;
+};
+
+static void boe_th101mb31ig002_reset(struct boe_th101mb31ig002 *ctx)
+{
+ gpiod_direction_output(ctx->reset, 0);
+ usleep_range(10, 100);
+ gpiod_direction_output(ctx->reset, 1);
+ usleep_range(10, 100);
+ gpiod_direction_output(ctx->reset, 0);
+ usleep_range(5000, 6000);
+}
+
+static int boe_th101mb31ig002_enable(struct drm_panel *panel)
+{
+ struct boe_th101mb31ig002 *ctx = container_of(panel,
+ struct boe_th101mb31ig002,
+ panel);
+ struct mipi_dsi_device *dsi = ctx->dsi;
+ struct device *dev = >dev;
+ int ret;
+
+ if (ctx->enabled)
+ return 0;
+
+ mipi_dsi_dcs_write_seq(dsi, 0xE0, 0xAB, 0xBA);
+ mipi_dsi_dcs_write_seq(dsi, 0xE1, 0xBA, 0xAB);
+ mipi_dsi_dcs_write_seq(dsi, 0xB1, 0x10, 0x01, 0x47, 0xFF);
+ mipi_dsi_dcs_write_seq(dsi, 0xB2, 0x0C, 0x14, 0x04, 0x50, 0x50, 0x14);
+ mipi_dsi_dcs_write_seq(dsi, 0xB3, 0x56, 0x53, 0x00);
+ mipi_dsi_dcs_write_seq(dsi, 0xB4, 0x33, 0x30, 0x04);
+ mipi_dsi_dcs_write_seq(dsi, 0xB6, 0xB0, 0x00, 0x00, 0x10, 0x00, 0x10,
+ 0x00);
+ mipi_dsi_dcs_write_seq(dsi, 0xB8, 0x05, 0x12, 0x29, 0x49, 0x48, 0x00,
+ 0x00);
+ mipi_dsi_dcs_write_seq(dsi, 0xB9, 0x7C, 0x65, 0x55, 0x49, 0x46, 0x36,
+ 0x3B, 0x24, 0x3D, 0x3C, 0x3D, 0x5C, 0x4C,
+ 0x55, 0x47, 0x46, 0x39, 0x26, 0x06, 0x7C,
+ 0x65, 0x55, 0x49, 0x46, 0x36, 0x3B, 0x24,
+
Re: [PATCH 2/6] drm/panel: Add driver for BOE TH101MB31IG002-28A panel
On 12/22/2023 3:05 AM, Manuel Traut wrote:
From: Segfault
The BOE TH101MB31IG002-28A panel is a WXGA panel.
It is used in Pine64 Pinetab2 and PinetabV.
Signed-off-by: Segfault
Signed-off-by: Manuel Traut
---
drivers/gpu/drm/panel/Kconfig | 11 +
drivers/gpu/drm/panel/Makefile | 1 +
.../gpu/drm/panel/panel-boe-th101mb31ig002-28a.c | 307 +
3 files changed, 319 insertions(+)
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 99e14dc212ec..927ddd10e688 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -67,6 +67,17 @@ config DRM_PANEL_BOE_HIMAX8279D
24 bit RGB per pixel. It provides a MIPI DSI interface to
the host and has a built-in LED backlight.
+config DRM_PANEL_BOE_TH101MB31UIG002_28A
+ tristate "Boe TH101MB31UIG002-28A panel"
+ depends on OF
+ depends on DRM_MIPI_DSI
+ depends on BACKLIGHT_CLASS_DEVICE
+ help
+ Say Y here if you want to enable support for Boe
+ TH101MB31UIG002-28A TFT-LCD modules. The panel has a 800x1280
+ resolution and uses 24 bit RGB per pixel. It provides a MIPI DSI
+ interface to the host and has a built-in LED backlight.
+
config DRM_PANEL_BOE_TV101WUM_NL6
tristate "BOE TV101WUM and AUO KD101N80 45NA 1200x1920 panel"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index d10c3de51c6d..dd6e1ac9d0a2 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_DRM_PANEL_ASUS_Z00T_TM5P5_NT35596) +=
panel-asus-z00t-tm5p5-n35596.
obj-$(CONFIG_DRM_PANEL_AUO_A030JTN01) += panel-auo-a030jtn01.o
obj-$(CONFIG_DRM_PANEL_BOE_BF060Y8M_AJ0) += panel-boe-bf060y8m-aj0.o
obj-$(CONFIG_DRM_PANEL_BOE_HIMAX8279D) += panel-boe-himax8279d.o
+obj-$(CONFIG_DRM_PANEL_BOE_TH101MB31UIG002_28A) +=
panel-boe-th101mb31ig002-28a.o
obj-$(CONFIG_DRM_PANEL_BOE_TV101WUM_NL6) += panel-boe-tv101wum-nl6.o
obj-$(CONFIG_DRM_PANEL_DSI_CM) += panel-dsi-cm.o
obj-$(CONFIG_DRM_PANEL_LVDS) += panel-lvds.o
diff --git a/drivers/gpu/drm/panel/panel-boe-th101mb31ig002-28a.c
b/drivers/gpu/drm/panel/panel-boe-th101mb31ig002-28a.c
new file mode 100644
index ..ac1dc99a0300
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-boe-th101mb31ig002-28a.c
@@ -0,0 +1,307 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2023 Alexander Warnecke
+ * Copyright (c) 2023 Manuel Traut
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+struct boe_th101mb31ig002 {
+ struct drm_panel panel;
+ bool enabled;
+ bool prepared;
Hi Manuel,
If I remember correctly, commit
d2aacaf07395bd798373cbec6af05fff4147aff3 should have introduced
prepared/enabled do the drm_panel struct.
Thanks,
Jessica Zhang
+
+ struct mipi_dsi_device *dsi;
+
+ struct regulator *power;
+ struct gpio_desc *enable;
+ struct gpio_desc *reset;
+
+ enum drm_panel_orientation orientation;
+};
+
+static int boe_th101mb31ig002_disable(struct drm_panel *panel)
+{
+ struct boe_th101mb31ig002 *ctx = container_of(panel,
+ struct boe_th101mb31ig002,
+ panel);
+
+ if (!ctx->enabled)
+ return 0;
+
+ mipi_dsi_dcs_set_display_off(ctx->dsi);
+ msleep(120);
+ ctx->enabled = false;
+
+ return 0;
+}
+
+static int boe_th101mb31ig002_unprepare(struct drm_panel *panel)
+{
+ struct boe_th101mb31ig002 *ctx = container_of(panel,
+ struct boe_th101mb31ig002,
+ panel);
+
+ if (!ctx->prepared)
+ return 0;
+
+ mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);
+ msleep(220);
+ gpiod_set_value_cansleep(ctx->reset, 1);
+ gpiod_set_value_cansleep(ctx->enable, 0);
+ regulator_disable(ctx->power);
+ ctx->prepared = false;
+
+ return 0;
+}
+
+static int boe_th101mb31ig002_prepare(struct drm_panel *panel)
+{
+ struct boe_th101mb31ig002 *ctx = container_of(panel,
+ struct boe_th101mb31ig002,
+ panel);
+ struct mipi_dsi_device *dsi = ctx->dsi;
+ int ret;
+
+ if (ctx->prepared)
+ return 0;
+
+ ret = regulator_enable(ctx->power);
+ if (ret) {
+ dev_err(>dev, "Failed to enable power supply: %d\n", ret);
+ return ret;
+ }
+
+ gpiod_set_value_cansleep(ctx->enable, 1);
+ msleep(120);
+ gpiod_direction_output(ctx->reset, 1);
+ msleep(120);
+ gpiod_direction_output(ctx->reset, 0);
+ msleep(120);
+
+
[PATCH v2 3/4] drm/i915/guc: reconcile Excess struct member kernel-doc warnings
Document nested struct members with full names as described in
Documentation/doc-guide/kernel-doc.rst.
intel_guc.h:305: warning: Excess struct member 'lock' description in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'guc_ids' description in
'intel_guc'
intel_guc.h:305: warning: Excess struct member 'num_guc_ids' description in
'intel_guc'
intel_guc.h:305: warning: Excess struct member 'guc_ids_bitmap' description in
'intel_guc'
intel_guc.h:305: warning: Excess struct member 'guc_id_list' description in
'intel_guc'
intel_guc.h:305: warning: Excess struct member 'guc_ids_in_use' description in
'intel_guc'
intel_guc.h:305: warning: Excess struct member 'destroyed_contexts' description
in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'destroyed_worker' description
in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'reset_fail_worker' description
in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'reset_fail_mask' description in
'intel_guc'
intel_guc.h:305: warning: Excess struct member 'sched_disable_delay_ms'
description in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'sched_disable_gucid_threshold'
description in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'lock' description in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'gt_stamp' description in
'intel_guc'
intel_guc.h:305: warning: Excess struct member 'ping_delay' description in
'intel_guc'
intel_guc.h:305: warning: Excess struct member 'work' description in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'shift' description in
'intel_guc'
intel_guc.h:305: warning: Excess struct member 'last_stat_jiffies' description
in 'intel_guc'
18 warnings as Errors
Signed-off-by: Randy Dunlap
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: intel-...@lists.freedesktop.org
Cc: Jonathan Corbet
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Rodrigo Vivi
---
v2: add Reviewed-by: Rodrigo
rebase and resend due to (i915) patchwork being down
drivers/gpu/drm/i915/gt/uc/intel_guc.h | 75 ---
1 file changed, 42 insertions(+), 33 deletions(-)
diff -- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -105,61 +105,67 @@ struct intel_guc {
*/
struct {
/**
-* @lock: protects everything in submission_state,
-* ce->guc_id.id, and ce->guc_id.ref when transitioning in and
-* out of zero
+* @submission_state.lock: protects everything in
+* submission_state, ce->guc_id.id, and ce->guc_id.ref
+* when transitioning in and out of zero
*/
spinlock_t lock;
/**
-* @guc_ids: used to allocate new guc_ids, single-lrc
+* @submission_state.guc_ids: used to allocate new
+* guc_ids, single-lrc
*/
struct ida guc_ids;
/**
-* @num_guc_ids: Number of guc_ids, selftest feature to be able
-* to reduce this number while testing.
+* @submission_state.num_guc_ids: Number of guc_ids, selftest
+* feature to be able to reduce this number while testing.
*/
int num_guc_ids;
/**
-* @guc_ids_bitmap: used to allocate new guc_ids, multi-lrc
+* @submission_state.guc_ids_bitmap: used to allocate
+* new guc_ids, multi-lrc
*/
unsigned long *guc_ids_bitmap;
/**
-* @guc_id_list: list of intel_context with valid guc_ids but no
-* refs
+* @submission_state.guc_id_list: list of intel_context
+* with valid guc_ids but no refs
*/
struct list_head guc_id_list;
/**
-* @guc_ids_in_use: Number single-lrc guc_ids in use
+* @submission_state.guc_ids_in_use: Number single-lrc
+* guc_ids in use
*/
unsigned int guc_ids_in_use;
/**
-* @destroyed_contexts: list of contexts waiting to be destroyed
-* (deregistered with the GuC)
+* @submission_state.destroyed_contexts: list of contexts
+* waiting to be destroyed (deregistered with the GuC)
*/
struct list_head destroyed_contexts;
/**
-* @destroyed_worker: worker to deregister contexts, need as we
-* need to take a GT PM reference and can't from destroy
-* function as it might be in an atomic context (no sleeping)
+*
[PATCH v2 4/4] drm/i915/perf: reconcile Excess struct member kernel-doc warnings
Document nested struct members with full names as described in
Documentation/doc-guide/kernel-doc.rst.
i915_perf_types.h:341: warning: Excess struct member 'ptr_lock' description in
'i915_perf_stream'
i915_perf_types.h:341: warning: Excess struct member 'head' description in
'i915_perf_stream'
i915_perf_types.h:341: warning: Excess struct member 'tail' description in
'i915_perf_stream'
3 warnings as Errors
Signed-off-by: Randy Dunlap
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: intel-...@lists.freedesktop.org
Cc: Jonathan Corbet
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Rodrigo Vivi
---
v2: add Reviewed-by: Rodrigo
rebase and resend due to (i915) patchwork being down
drivers/gpu/drm/i915/i915_perf_types.h |9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff -- a/drivers/gpu/drm/i915/i915_perf_types.h
b/drivers/gpu/drm/i915/i915_perf_types.h
--- a/drivers/gpu/drm/i915/i915_perf_types.h
+++ b/drivers/gpu/drm/i915/i915_perf_types.h
@@ -291,7 +291,8 @@ struct i915_perf_stream {
int size_exponent;
/**
-* @ptr_lock: Locks reads and writes to all head/tail state
+* @oa_buffer.ptr_lock: Locks reads and writes to all
+* head/tail state
*
* Consider: the head and tail pointer state needs to be read
* consistently from a hrtimer callback (atomic context) and
@@ -313,7 +314,8 @@ struct i915_perf_stream {
spinlock_t ptr_lock;
/**
-* @head: Although we can always read back the head pointer
register,
+* @oa_buffer.head: Although we can always read back
+* the head pointer register,
* we prefer to avoid trusting the HW state, just to avoid any
* risk that some hardware condition could * somehow bump the
* head pointer unpredictably and cause us to forward the wrong
@@ -322,7 +324,8 @@ struct i915_perf_stream {
u32 head;
/**
-* @tail: The last verified tail that can be read by userspace.
+* @oa_buffer.tail: The last verified tail that can be
+* read by userspace.
*/
u32 tail;
} oa_buffer;
[PATCH v2 2/4] drm/i915/gt: reconcile Excess struct member kernel-doc warnings
Document nested struct members with full names as described in
Documentation/doc-guide/kernel-doc.rst.
intel_gsc.h:34: warning: Excess struct member 'gem_obj' description in
'intel_gsc'
Also add missing field member descriptions.
Signed-off-by: Randy Dunlap
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: intel-...@lists.freedesktop.org
Cc: Jonathan Corbet
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Rodrigo Vivi
---
v2: add Reviewed-by: Rodrigo
rebase and resend due to (i915) patchwork being down
drivers/gpu/drm/i915/gt/intel_gsc.h |7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff -- a/drivers/gpu/drm/i915/gt/intel_gsc.h
b/drivers/gpu/drm/i915/gt/intel_gsc.h
--- a/drivers/gpu/drm/i915/gt/intel_gsc.h
+++ b/drivers/gpu/drm/i915/gt/intel_gsc.h
@@ -21,8 +21,11 @@ struct mei_aux_device;
/**
* struct intel_gsc - graphics security controller
*
- * @gem_obj: scratch memory GSC operations
- * @intf : gsc interface
+ * @intf : gsc interface
+ * @intf.adev :MEI aux. device for this @intf
+ * @intf.gem_obj : scratch memory GSC operations
+ * @intf.irq : IRQ for this device (%-1 for no IRQ)
+ * @intf.id : this interface's id number/index
*/
struct intel_gsc {
struct intel_gsc_intf {
[PATCH v2 1/4] drm/i915/gem: reconcile Excess struct member kernel-doc warnings
Document nested struct members with full names as described in
Documentation/doc-guide/kernel-doc.rst.
i915_gem_context_types.h:420: warning: Excess struct member 'lock' description
in 'i915_gem_context'
Signed-off-by: Randy Dunlap
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: intel-...@lists.freedesktop.org
Cc: Jonathan Corbet
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Rodrigo Vivi
---
v2: add Reviewed-by: Rodrigo
rebase and resend due to (i915) patchwork being down
drivers/gpu/drm/i915/gem/i915_gem_context_types.h |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff -- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
--- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
@@ -412,9 +412,9 @@ struct i915_gem_context {
/** @stale: tracks stale engines to be destroyed */
struct {
- /** @lock: guards engines */
+ /** @stale.lock: guards engines */
spinlock_t lock;
- /** @engines: list of stale engines */
+ /** @stale.engines: list of stale engines */
struct list_head engines;
} stale;
};
Re: [PATCH 4/4] drm/i915/perf: reconcile Excess struct member kernel-doc warnings
On 12/26/23 09:05, Rodrigo Vivi wrote: > On Wed, Dec 20, 2023 at 07:20:29PM -0800, Randy Dunlap wrote: >> Document nested struct members with full names as described in >> Documentation/doc-guide/kernel-doc.rst. >> >> i915_perf_types.h:341: warning: Excess struct member 'ptr_lock' description >> in 'i915_perf_stream' >> i915_perf_types.h:341: warning: Excess struct member 'head' description in >> 'i915_perf_stream' >> i915_perf_types.h:341: warning: Excess struct member 'tail' description in >> 'i915_perf_stream' >> 3 warnings as Errors >> >> Signed-off-by: Randy Dunlap >> Cc: Jani Nikula >> Cc: Joonas Lahtinen >> Cc: Rodrigo Vivi >> Cc: Tvrtko Ursulin >> Cc: intel-...@lists.freedesktop.org >> Cc: Jonathan Corbet >> Cc: dri-devel@lists.freedesktop.org > > > for the series: > Reviewed-by: Rodrigo Vivi > > I'm afraid patchwork was down when you sent this out. > Could you please rebase and resend? Just to ensure > our CI doesn't complain and then we push it. > Will do. Thanks. -- #Randy
Re: [PATCH] nightly.conf: Add the xe repo to drm-tip
On Fri, Dec 22, 2023 at 12:36:39PM +0100, Thomas Hellström wrote: > Add the xe repo to drm-tip and the dim tools. > For now use the sha1 of the first drm-xe-next pull request for drm-tip, > since that branch tip is currently adapted for our CI testing. > > Cc: Rodrigo Vivi > Cc: Lucas De Marchi > Cc: Oded Gabbay > Cc: daniel.vet...@ffwll.ch > Cc: Maarten Lankhorst > Cc: dim-to...@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org > Cc: intel-...@lists.freedesktop.org > Signed-off-by: Thomas Hellström > --- > nightly.conf | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/nightly.conf b/nightly.conf > index 24126b61b797..accd3ff2cc39 100644 > --- a/nightly.conf > +++ b/nightly.conf > @@ -24,6 +24,10 @@ git://anongit.freedesktop.org/drm-tip > https://anongit.freedesktop.org/git/drm/drm-tip > https://anongit.freedesktop.org/git/drm/drm-tip.git > " > +drm_tip_repos[drm-xe]=" > +ssh://g...@gitlab.freedesktop.org/drm/xe/kernel.git > +https://gitlab.freedesktop.org/drm/xe/kernel.git > +" > drm_tip_repos[drm-intel]=" > ssh://git.freedesktop.org/git/drm/drm-intel > ssh://git.freedesktop.org/git/drm-intel > @@ -65,14 +69,17 @@ drm_tip_config=( > "drmdrm-fixes" > "drm-misc drm-misc-fixes" > "drm-intel drm-intel-fixes" > + "drm-xe drm-xe-fixes" > > "drmdrm-next" > "drm-misc drm-misc-next-fixes" > "drm-intel drm-intel-next-fixes" > + "drm-xe drm-xe-next-fixes" > > "drm-misc drm-misc-next" > "drm-intel drm-intel-next" > "drm-intel drm-intel-gt-next" > + "drm-xe drm-xe-next b6e1b7081768" yeap, up to this commit nothing else should change, but then we will need an extra rebase of the rest on top of drm/drm-next. But then we need to decide where these following patches will live: 880277f31cc69 drm/xe/guc: define LNL FW 2cfc5ae1b8267 drm/xe/guc: define PVC FW 52383b58eb8cf mei/hdcp: Also enable for XE bea27d7369855 mei: gsc: add support for auxiliary device created by Xe driver fcb3410197f05 fault-inject: Include linux/types.h by default. 8ebd9cd71f8ac drm/xe: Add PVC's PCI device IDs Will it be the topic/core-for-CI? or topic/xe-extras? or what? Anyway, for the inclusion like this, after our CI is ready: Acked-by: Rodrigo Vivi > > "drm-intel topic/core-for-CI" > "drm-misc topic/i915-ttm" > -- > 2.42.0 >
[PULL] drm-xe-next-fixes
Hi Dave and Sima, Here goes the fixes that I had promised last week. With these in place we should be good now with the all yes config W=1 and different compilers. Thanks for already include that one that disables the 32-bit. I had just noticed when I was trying to cherry-pick it to the -next-fixes branch. Thanks, Rodrigo. The following changes since commit 92242716ee92d2aa3c38c736b53d8910d443566d: Merge tag 'drm-habanalabs-next-2023-12-19' of https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/linux into drm-next (2023-12-22 14:52:04 +1000) are available in the Git repository at: https://gitlab.freedesktop.org/drm/xe/kernel.git tags/drm-xe-next-fixes-2023-12-26 for you to fetch changes up to 315acff5196f4e2f84a2a2d093000e0c6b0b4d1c: drm/xe: Fix warning on impossible condition (2023-12-26 12:53:26 -0500) - Fix couple unfined behavior cases to calm UBSAN and clang (Matt Brost, Lucas) Lucas De Marchi (1): drm/xe: Fix warning on impossible condition Matthew Brost (1): drm/xe: Fix UBSAN splat in add_preempt_fences() drivers/gpu/drm/xe/xe_vm.c | 3 +++ drivers/gpu/drm/xe/xe_wait_user_fence.c | 1 + 2 files changed, 4 insertions(+)
[PATCH] drm/vc4: Improve exception handling in vc4_validate_shader()
From: Markus Elfring
Date: Tue, 26 Dec 2023 18:22:54 +0100
Memory releases were triggered during error handling only over
the label “fail”.
1. Return directly after a call of the function “kcalloc” failed
at the beginning.
2. Reduce the data processing at the end to a required kfree() call order
also by adjusting jump targets.
3. Delete an initialisation for the variable “validated_shader”
which became unnecessary with this refactoring.
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/vc4/vc4_validate_shaders.c | 42 +++---
1 file changed, 21 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_validate_shaders.c
b/drivers/gpu/drm/vc4/vc4_validate_shaders.c
index 9745f8810eca..d76787bb7d51 100644
--- a/drivers/gpu/drm/vc4/vc4_validate_shaders.c
+++ b/drivers/gpu/drm/vc4/vc4_validate_shaders.c
@@ -783,7 +783,7 @@ vc4_validate_shader(struct drm_gem_dma_object *shader_obj)
int shader_end_ip = 0;
uint32_t last_thread_switch_ip = -3;
uint32_t ip;
- struct vc4_validated_shader_info *validated_shader = NULL;
+ struct vc4_validated_shader_info *validated_shader;
struct vc4_shader_validation_state validation_state;
if (WARN_ON_ONCE(vc4->is_vc5))
@@ -799,14 +799,14 @@ vc4_validate_shader(struct drm_gem_dma_object *shader_obj)
kcalloc(BITS_TO_LONGS(validation_state.max_ip),
sizeof(unsigned long), GFP_KERNEL);
if (!validation_state.branch_targets)
- goto fail;
+ return NULL;
validated_shader = kcalloc(1, sizeof(*validated_shader), GFP_KERNEL);
if (!validated_shader)
- goto fail;
+ goto free_targets;
if (!vc4_validate_branches(_state))
- goto fail;
+ goto free_shader;
for (ip = 0; ip < validation_state.max_ip; ip++) {
uint64_t inst = validation_state.shader[ip];
@@ -815,7 +815,7 @@ vc4_validate_shader(struct drm_gem_dma_object *shader_obj)
validation_state.ip = ip;
if (!vc4_handle_branch_target(_state))
- goto fail;
+ goto free_offsets;
if (ip == last_thread_switch_ip + 3) {
/* Reset r0-r3 live clamp data */
@@ -842,12 +842,12 @@ vc4_validate_shader(struct drm_gem_dma_object *shader_obj)
if (!check_instruction_writes(validated_shader,
_state)) {
DRM_DEBUG("Bad write at ip %d\n", ip);
- goto fail;
+ goto free_offsets;
}
if (!check_instruction_reads(validated_shader,
_state))
- goto fail;
+ goto free_offsets;
if (sig == QPU_SIG_PROG_END) {
found_shader_end = true;
@@ -861,7 +861,7 @@ vc4_validate_shader(struct drm_gem_dma_object *shader_obj)
if (ip < last_thread_switch_ip + 3) {
DRM_DEBUG("Thread switch too soon after
"
"last switch at ip %d\n", ip);
- goto fail;
+ goto free_offsets;
}
last_thread_switch_ip = ip;
}
@@ -872,26 +872,26 @@ vc4_validate_shader(struct drm_gem_dma_object *shader_obj)
if (!check_instruction_writes(validated_shader,
_state)) {
DRM_DEBUG("Bad LOAD_IMM write at ip %d\n", ip);
- goto fail;
+ goto free_offsets;
}
break;
case QPU_SIG_BRANCH:
if (!check_branch(inst, validated_shader,
_state, ip))
- goto fail;
+ goto free_offsets;
if (ip < last_thread_switch_ip + 3) {
DRM_DEBUG("Branch in thread switch at ip %d",
ip);
- goto fail;
+ goto free_offsets;
}
break;
default:
DRM_DEBUG("Unsupported QPU signal %d at "
"instruction %d\n", sig, ip);
- goto fail;
+ goto free_offsets;
}
/* There are two delay slots after program end is signaled
@@
Re: [PATCH 4/4] drm/i915/perf: reconcile Excess struct member kernel-doc warnings
On Wed, Dec 20, 2023 at 07:20:29PM -0800, Randy Dunlap wrote:
> Document nested struct members with full names as described in
> Documentation/doc-guide/kernel-doc.rst.
>
> i915_perf_types.h:341: warning: Excess struct member 'ptr_lock' description
> in 'i915_perf_stream'
> i915_perf_types.h:341: warning: Excess struct member 'head' description in
> 'i915_perf_stream'
> i915_perf_types.h:341: warning: Excess struct member 'tail' description in
> 'i915_perf_stream'
> 3 warnings as Errors
>
> Signed-off-by: Randy Dunlap
> Cc: Jani Nikula
> Cc: Joonas Lahtinen
> Cc: Rodrigo Vivi
> Cc: Tvrtko Ursulin
> Cc: intel-...@lists.freedesktop.org
> Cc: Jonathan Corbet
> Cc: dri-devel@lists.freedesktop.org
for the series:
Reviewed-by: Rodrigo Vivi
I'm afraid patchwork was down when you sent this out.
Could you please rebase and resend? Just to ensure
our CI doesn't complain and then we push it.
Thanks,
Rodrigo.
> ---
> drivers/gpu/drm/i915/i915_perf_types.h |9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff -- a/drivers/gpu/drm/i915/i915_perf_types.h
> b/drivers/gpu/drm/i915/i915_perf_types.h
> --- a/drivers/gpu/drm/i915/i915_perf_types.h
> +++ b/drivers/gpu/drm/i915/i915_perf_types.h
> @@ -291,7 +291,8 @@ struct i915_perf_stream {
> int size_exponent;
>
> /**
> - * @ptr_lock: Locks reads and writes to all head/tail state
> + * @oa_buffer.ptr_lock: Locks reads and writes to all
> + * head/tail state
>*
>* Consider: the head and tail pointer state needs to be read
>* consistently from a hrtimer callback (atomic context) and
> @@ -313,7 +314,8 @@ struct i915_perf_stream {
> spinlock_t ptr_lock;
>
> /**
> - * @head: Although we can always read back the head pointer
> register,
> + * @oa_buffer.head: Although we can always read back
> + * the head pointer register,
>* we prefer to avoid trusting the HW state, just to avoid any
>* risk that some hardware condition could * somehow bump the
>* head pointer unpredictably and cause us to forward the wrong
> @@ -322,7 +324,8 @@ struct i915_perf_stream {
> u32 head;
>
> /**
> - * @tail: The last verified tail that can be read by userspace.
> + * @oa_buffer.tail: The last verified tail that can be
> + * read by userspace.
>*/
> u32 tail;
> } oa_buffer;
Re: [PATCH v2 0/4] arm64: rockchip: Pine64 pinetab2 support
On Tuesday, December 26, 2023 1:32:32 PM UTC Diederik de Haas wrote: > [5.570059] dwmmc_rockchip fe2c.mmc: could not set regulator OCR > (-22) [5.570835] dwmmc_rockchip fe2c.mmc: failed to enable vmmc > regulator [5.616373] dwmmc_rockchip fe2c.mmc: could not set > regulator OCR (-22) [5.621903] dwmmc_rockchip fe2c.mmc: failed to > enable vmmc regulator This is the WLAN SDIO, the error shows up when the device is not powered on. > Both also have an error wrt `goodix_911_cfg.bin` firmware, but the error > could be because Debian kernel 'upgraded' a warning to an error. > I've searched online for that filename, but haven't found anything. > The touchscreen works, so I guess that one can be ignored. This can be ignored, it's for touch panels where it's config is not on flash (typically touch panel calibration data).
[PATCH 2/2] drm/sched: Return an error code only as a constant in drm_sched_init()
From: Markus Elfring
Date: Tue, 26 Dec 2023 16:37:37 +0100
Return an error code without storing it in an intermediate variable.
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/scheduler/sched_main.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index b99d4e9ff109..1abbcdf38430 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -1249,7 +1249,7 @@ int drm_sched_init(struct drm_gpu_scheduler *sched,
long timeout, struct workqueue_struct *timeout_wq,
atomic_t *score, const char *name, struct device *dev)
{
- int i, ret;
+ int i;
sched->ops = ops;
sched->credit_limit = credit_limit;
@@ -1285,7 +1285,7 @@ int drm_sched_init(struct drm_gpu_scheduler *sched,
sched->own_submit_wq = true;
}
- ret = -ENOMEM;
+
sched->sched_rq = kmalloc_array(num_rqs, sizeof(*sched->sched_rq),
GFP_KERNEL | __GFP_ZERO);
if (!sched->sched_rq)
@@ -1321,7 +1321,7 @@ int drm_sched_init(struct drm_gpu_scheduler *sched,
if (sched->own_submit_wq)
destroy_workqueue(sched->submit_wq);
drm_err(sched, "%s: Failed to setup GPU scheduler--out of memory\n",
__func__);
- return ret;
+ return -ENOMEM;
}
EXPORT_SYMBOL(drm_sched_init);
--
2.43.0
[PATCH 1/2] drm/sched: One function call less in drm_sched_init() after error detection
From: Markus Elfring
Date: Tue, 26 Dec 2023 16:30:25 +0100
The kfree() function was called in one case by the
drm_sched_init() function during error handling
even if the passed data structure member contained a null pointer.
This issue was detected by using the Coccinelle software.
Thus adjust a jump target.
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/scheduler/sched_main.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index 550492a7a031..b99d4e9ff109 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -1289,7 +1289,7 @@ int drm_sched_init(struct drm_gpu_scheduler *sched,
sched->sched_rq = kmalloc_array(num_rqs, sizeof(*sched->sched_rq),
GFP_KERNEL | __GFP_ZERO);
if (!sched->sched_rq)
- goto Out_free;
+ goto Out_check_own;
sched->num_rqs = num_rqs;
for (i = DRM_SCHED_PRIORITY_KERNEL; i < sched->num_rqs; i++) {
sched->sched_rq[i] = kzalloc(sizeof(*sched->sched_rq[i]),
GFP_KERNEL);
@@ -1314,9 +1314,10 @@ int drm_sched_init(struct drm_gpu_scheduler *sched,
Out_unroll:
for (--i ; i >= DRM_SCHED_PRIORITY_KERNEL; i--)
kfree(sched->sched_rq[i]);
-Out_free:
+
kfree(sched->sched_rq);
sched->sched_rq = NULL;
+Out_check_own:
if (sched->own_submit_wq)
destroy_workqueue(sched->submit_wq);
drm_err(sched, "%s: Failed to setup GPU scheduler--out of memory\n",
__func__);
--
2.43.0
[PATCH 0/2] drm/sched: Adjustments for drm_sched_init()
From: Markus Elfring Date: Tue, 26 Dec 2023 16:48:48 +0100 A few update suggestions were taken into account from static source code analysis. Markus Elfring (2): One function call less after error detection Return an error code only as a constant drivers/gpu/drm/scheduler/sched_main.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) -- 2.43.0
Re: [PATCH v5 0/8] iio: new DMABUF based API, v5
On Thu, 21 Dec 2023 18:56:52 +0100 Paul Cercueil wrote: > Hi Jonathan, > > Le jeudi 21 décembre 2023 à 16:30 +, Jonathan Cameron a écrit : > > On Tue, 19 Dec 2023 18:50:01 +0100 > > Paul Cercueil wrote: > > > > > [V4 was: "iio: Add buffer write() support"][1] > > > > > > Hi Jonathan, > > > > > Hi Paul, > > > > > This is a respin of the V3 of my patchset that introduced a new > > > interface based on DMABUF objects [2]. > > > > Great to see this moving forwards. > > > > > > > > The V4 was a split of the patchset, to attempt to upstream buffer > > > write() support first. But since there is no current user upstream, > > > it > > > was not merged. This V5 is about doing the opposite, and contains > > > the > > > new DMABUF interface, without adding the buffer write() support. It > > > can > > > already be used with the upstream adi-axi-adc driver. > > > > Seems like a sensible path in the short term. > > > > > > > > In user-space, Libiio uses it to transfer back and forth blocks of > > > samples between the hardware and the applications, without having > > > to > > > copy the data. > > > > > > On a ZCU102 with a FMComms3 daughter board, running Libiio from the > > > pcercuei/dev-new-dmabuf-api branch [3], compiled with > > > WITH_LOCAL_DMABUF_API=OFF (so that it uses fileio): > > > sudo utils/iio_rwdev -b 4096 -B cf-ad9361-lpc > > > Throughput: 116 MiB/s > > > > > > Same hardware, with the DMABUF API (WITH_LOCAL_DMABUF_API=ON): > > > sudo utils/iio_rwdev -b 4096 -B cf-ad9361-lpc > > > Throughput: 475 MiB/s > > > > > > This benchmark only measures the speed at which the data can be > > > fetched > > > to iio_rwdev's internal buffers, and does not actually try to read > > > the > > > data (e.g. to pipe it to stdout). It shows that fetching the data > > > is > > > more than 4x faster using the new interface. > > > > > > When actually reading the data, the performance difference isn't > > > that > > > impressive (maybe because in case of DMABUF the data is not in > > > cache): > > > > This needs a bit more investigation ideally. Perhaps perf counters > > can be > > used to establish that cache misses are the main different between > > dropping it on the floor and actually reading the data. > > Yes, we'll work on it. The other big difference is that fileio uses > dma_alloc_coherent() while the DMABUFs use non-coherent mappings. I > guess coherent memory is faster for the typical access pattern (which > is "read/write everything sequentially once"). Long time since I last worked much with a platform that wasn't always IO coherent, so I've forgotten how all this works (all ends up as no-ops on platforms I tend to use these days!) Good luck, I'll be interested to see what this turns out to be. > > > > > > > WITH_LOCAL_DMABUF_API=OFF (so that it uses fileio): > > > sudo utils/iio_rwdev -b 4096 cf-ad9361-lpc | dd of=/dev/zero > > > status=progress > > > 2446422528 bytes (2.4 GB, 2.3 GiB) copied, 22 s, 111 MB/s > > > > > > WITH_LOCAL_DMABUF_API=ON: > > > sudo utils/iio_rwdev -b 4096 cf-ad9361-lpc | dd of=/dev/zero > > > status=progress > > > 2334388736 bytes (2.3 GB, 2.2 GiB) copied, 21 s, 114 MB/s > > > > > > One interesting thing to note is that fileio is (currently) > > > actually > > > faster than the DMABUF interface if you increase a lot the buffer > > > size. > > > My explanation is that the cache invalidation routine takes more > > > and > > > more time the bigger the DMABUF gets. This is because the DMABUF is > > > backed by small-size pages, so a (e.g.) 64 MiB DMABUF is backed by > > > up > > > to 16 thousands pages, that have to be invalidated one by one. This > > > can > > > be addressed by using huge pages, but the udmabuf driver does not > > > (yet) > > > support creating DMABUFs backed by huge pages. > > > > I'd imagine folios of reasonable size will help sort of a huge page > > as then hopefully it will use the flush by va range instructions if > > available. > > > > > > > > Anyway, the real benefits happen when the DMABUFs are either shared > > > between IIO devices, or between the IIO subsystem and another > > > filesystem. In that case, the DMABUFs are simply passed around > > > drivers, > > > without the data being copied at any moment. > > > > > > We use that feature to transfer samples from our transceivers to > > > USB, > > > using a DMABUF interface to FunctionFS [4]. > > > > > > This drastically increases the throughput, to about 274 MiB/s over > > > a > > > USB3 link, vs. 127 MiB/s using IIO's fileio interface + write() to > > > the > > > FunctionFS endpoints, for a lower CPU usage (0.85 vs. 0.65 load > > > avg.). > > > > This is a nice example. Where are you with getting the patch merged? > > I'll send a new version (mostly a [RESEND]...) in the coming days. As > you can see from the review on my last attempt, the main blocker is > that nobody wants to merge a new interface if the rest of the kernel > bits aren't
Re: [PATCH v5 7/8] iio: buffer-dmaengine: Support new DMABUF based userspace API
On Fri, 22 Dec 2023 09:58:29 +0100
Nuno Sá wrote:
> On Thu, 2023-12-21 at 18:30 +0100, Paul Cercueil wrote:
> > Hi Jonathan,
> >
> > Le jeudi 21 décembre 2023 à 16:12 +, Jonathan Cameron a écrit :
> > > On Tue, 19 Dec 2023 18:50:08 +0100
> > > Paul Cercueil wrote:
> > >
> > > > Use the functions provided by the buffer-dma core to implement the
> > > > DMABUF userspace API in the buffer-dmaengine IIO buffer
> > > > implementation.
> > > >
> > > > Since we want to be able to transfer an arbitrary number of bytes
> > > > and
> > > > not necesarily the full DMABUF, the associated scatterlist is
> > > > converted
> > > > to an array of DMA addresses + lengths, which is then passed to
> > > > dmaengine_prep_slave_dma_array().
> > > >
> > > > Signed-off-by: Paul Cercueil
> > > One question inline. Otherwise looks fine to me.
> > >
> > > J
> > > >
> > > > ---
> > > > v3: Use the new dmaengine_prep_slave_dma_array(), and adapt the
> > > > code to
> > > > work with the new functions introduced in industrialio-buffer-
> > > > dma.c.
> > > >
> > > > v5: - Use the new dmaengine_prep_slave_dma_vec().
> > > > - Restrict to input buffers, since output buffers are not yet
> > > > supported by IIO buffers.
> > > > ---
> > > > .../buffer/industrialio-buffer-dmaengine.c | 52
> > > > ---
> > > > 1 file changed, 46 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > > > b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > > > index 5f85ba38e6f6..825d76a24a67 100644
> > > > --- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > > > +++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > > > @@ -64,15 +64,51 @@ static int
> > > > iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue
> > > > *queue,
> > > > struct dmaengine_buffer *dmaengine_buffer =
> > > > iio_buffer_to_dmaengine_buffer(>buffer);
> > > > struct dma_async_tx_descriptor *desc;
> > > > + unsigned int i, nents;
> > > > + struct scatterlist *sgl;
> > > > + struct dma_vec *vecs;
> > > > + size_t max_size;
> > > > dma_cookie_t cookie;
> > > > + size_t len_total;
> > > >
> > > > - block->bytes_used = min(block->size, dmaengine_buffer-
> > > > > max_size);
> > > > - block->bytes_used = round_down(block->bytes_used,
> > > > - dmaengine_buffer->align);
> > > > + if (queue->buffer.direction != IIO_BUFFER_DIRECTION_IN) {
> > > > + /* We do not yet support output buffers. */
> > > > + return -EINVAL;
> > > > + }
> > > >
> > > > - desc = dmaengine_prep_slave_single(dmaengine_buffer->chan,
> > > > - block->phys_addr, block->bytes_used,
> > > > DMA_DEV_TO_MEM,
> > > > - DMA_PREP_INTERRUPT);
> > > > + if (block->sg_table) {
> > > > + sgl = block->sg_table->sgl;
> > > > + nents = sg_nents_for_len(sgl, block->bytes_used);
> > >
> > > Are we guaranteed the length in the sglist is enough? If not this
> > > can return an error code.
> >
> > The length of the sglist will always be enough, the
> > iio_buffer_enqueue_dmabuf() function already checks that block-
> > > bytes_used is equal or smaller than the size of the DMABUF.
> >
> > It is quite a few functions above in the call stack though, so I can
> > handle the errors of sg_nents_for_len() here if you think makes sense.
>
> Maybe putting something like the above in a comment?
Either comment, or an explicit check so we don't need the comment is
fine as far as I'm concerned.
Jonathan
>
> - Nuno Sá
>
>
Re: [PATCH v5 6/8] iio: buffer-dma: Enable support for DMABUFs
> > > +struct iio_dma_buffer_block *
> > > +iio_dma_buffer_attach_dmabuf(struct iio_buffer *buffer,
> > > + struct dma_buf_attachment *attach)
> > > +{
> > > + struct iio_dma_buffer_queue *queue = iio_buffer_to_queue(buffer);
> > > + struct iio_dma_buffer_block *block;
> > > + int err;
> > > +
> > > + mutex_lock(>lock);
> >
> > guard(mutex)(>lock);
>
> I'm also a big fan of this new stuff but shouldn't we have a prep patch
> making the
> transition to that? Otherwise we'll end up with a mix of styles. I'm happy to
> clean
> up stuff with follow up patches (even some coding style could be improved
> IIRC) but
> typically that's not how we handle things like this I believe...
Ideally yes, I think a prep patch would make sense - but I'm not that fussed
about it and follow up patches would be fine here.
There are cases for using this that are much easier to justify than
others, so I don't really mind if simple
mutex_lock();
do_something
mutex_unlock();
cases are left for ever not using guard(), though I also don't mind if people
want to use
scoped_guard() or guard for these just to be consistent. Either way is pretty
easy to read. We'll probably also continue to gain new uses of this logic such
as the conditional guard stuff that is queued up for the next merge window and
whilst that is going on we are going to have a bit of mixed style.
Jonathan
>
> - Nuno Sá
> >
>
Re: [PATCH v8 2/2] drm/i915/guc: Close deregister-context race against CT-loss
On Wed, Dec 20, 2023 at 11:08:59PM +, Teres Alexis, Alan Previn wrote: > On Wed, 2023-12-13 at 16:23 -0500, Vivi, Rodrigo wrote: > > On Tue, Dec 12, 2023 at 08:57:16AM -0800, Alan Previn wrote: > > > If we are at the end of suspend or very early in resume > > > its possible an async fence signal (via rcu_call) is triggered > > > to free_engines which could lead us to the execution of > > > the context destruction worker (after a prior worker flush). > alan:snip > > > > > Thus, do an unroll in guc_lrc_desc_unpin and deregister_destroyed_- > > > contexts if guc_lrc_desc_unpin fails due to CT send falure. > > > When unrolling, keep the context in the GuC's destroy-list so > > > it can get picked up on the next destroy worker invocation > > > (if suspend aborted) or get fully purged as part of a GuC > > > sanitization (end of suspend) or a reset flow. > > > > > > Signed-off-by: Alan Previn > > > Signed-off-by: Anshuman Gupta > > > Tested-by: Mousumi Jana > > > Acked-by: Daniele Ceraolo Spurio > > > > Thanks for all the explanations, patience and great work! > > > > Reviewed-by: Rodrigo Vivi > > alan: Thanks Rodrigo for the RB last week, just quick update: > > I've cant reproduce the BAT failures that seem to be intermittent > on platform and test - however, a noticable number of failures > do keep occuring on i915_selftest @live @requests where the > last test leaked a wakeref and the failing test hangs waiting > for gt to idle before starting its test. > > i have to debug this further although from code inspection > is unrelated to the patches in this series. > Hopefully its a different issue. Yeap, likely not related. Anyway, I'm sorry for not merging this sooner. Could you please send a rebased version? This on is not applying cleanly anymore.
Re: [PATCH v2 0/4] arm64: rockchip: Pine64 pinetab2 support
On Saturday, 23 December 2023 16:20:14 CET Manuel Traut wrote: > This adds support for the BOE TH101MB31IG002 LCD Panel used in Pinetab2 [1] > and Pinetab-V [2] as well as the devictrees for the Pinetab2 v0.1 and v2.0. I have a PineTab2 4GB/64GB model and I run a Debian Trixie system *from an SDcard* (danctnix system is on eMMC). On it, I have a 6.6 kernel with my own patch set and a 6.7-rc7 kernel with this patch set. Everything seems to work properly. I've now also tested the HDMI output connector and that worked too :) So based on that, you can add my Tested-By: Diederik de Haas I do see an error in dmesg wrt mmc. I had one with my own patch set too, but a different one and it seems the mainline patch set fixes that, but then runs into another issue: [5.570059] dwmmc_rockchip fe2c.mmc: could not set regulator OCR (-22) [5.570835] dwmmc_rockchip fe2c.mmc: failed to enable vmmc regulator [5.616373] dwmmc_rockchip fe2c.mmc: could not set regulator OCR (-22) [5.621903] dwmmc_rockchip fe2c.mmc: failed to enable vmmc regulator Both also have an error wrt `goodix_911_cfg.bin` firmware, but the error could be because Debian kernel 'upgraded' a warning to an error. I've searched online for that filename, but haven't found anything. The touchscreen works, so I guess that one can be ignored. kernel 6.7-rc7 + mainline patch set: === # uname -a Linux pinetab2 6.7-rc7+unreleased-arm64 #1 SMP Debian 6.7~rc7-1~pine64 (2023-12-24) aarch64 GNU/Linux # dmesg --level emerg,alert,crit # dmesg --level emerg,alert,crit,err [5.570059] dwmmc_rockchip fe2c.mmc: could not set regulator OCR (-22) [5.570835] dwmmc_rockchip fe2c.mmc: failed to enable vmmc regulator [5.616373] dwmmc_rockchip fe2c.mmc: could not set regulator OCR (-22) [5.621903] dwmmc_rockchip fe2c.mmc: failed to enable vmmc regulator [ 19.569452] Goodix-TS 1-005d: firmware: failed to load goodix_911_cfg.bin (-2) [ 19.575504] firmware_class: See https://wiki.debian.org/Firmware for information about missing firmware [ 19.613497] Goodix-TS 1-005d: firmware: failed to load goodix_911_cfg.bin (-2) # dmesg | grep mmc [3.782844] dwmmc_rockchip fe2c.mmc: IDMAC supports 32-bit address mode. [3.783027] dwmmc_rockchip fe2c.mmc: Using internal DMA controller. [3.783132] dwmmc_rockchip fe2c.mmc: Version ID is 270a [3.783314] dwmmc_rockchip fe2c.mmc: DW MMC controller at irq 24,32 bit host data width,256 deep fifo [3.853034] dwmmc_rockchip fe2b.mmc: IDMAC supports 32-bit address mode. [3.853859] dwmmc_rockchip fe2b.mmc: Using internal DMA controller. [3.854492] dwmmc_rockchip fe2b.mmc: Version ID is 270a [3.856863] dwmmc_rockchip fe2b.mmc: DW MMC controller at irq 31,32 bit host data width,256 deep fifo [3.861731] dwmmc_rockchip fe2c.mmc: IDMAC supports 32-bit address mode. [3.862459] dwmmc_rockchip fe2c.mmc: Using internal DMA controller. [3.863088] dwmmc_rockchip fe2c.mmc: Version ID is 270a [3.863656] dwmmc_rockchip fe2c.mmc: DW MMC controller at irq 24,32 bit host data width,256 deep fifo [3.870506] dwmmc_rockchip fe2c.mmc: Failed getting OCR mask: -22 [4.015316] dwmmc_rockchip fe2b.mmc: IDMAC supports 32-bit address mode. [4.016042] dwmmc_rockchip fe2b.mmc: Using internal DMA controller. [4.016660] dwmmc_rockchip fe2b.mmc: Version ID is 270a [4.017223] dwmmc_rockchip fe2b.mmc: DW MMC controller at irq 31,32 bit host data width,256 deep fifo [4.034954] dwmmc_rockchip fe2c.mmc: IDMAC supports 32-bit address mode. [4.035772] dwmmc_rockchip fe2c.mmc: Using internal DMA controller. [4.037119] dwmmc_rockchip fe2c.mmc: Version ID is 270a [4.039868] dwmmc_rockchip fe2c.mmc: DW MMC controller at irq 24,32 bit host data width,256 deep fifo [4.041248] dwmmc_rockchip fe2c.mmc: Failed getting OCR mask: -22 [5.172937] dwmmc_rockchip fe2b.mmc: IDMAC supports 32-bit address mode. [5.175108] dwmmc_rockchip fe2b.mmc: Using internal DMA controller. [5.175750] dwmmc_rockchip fe2b.mmc: Version ID is 270a [5.176311] dwmmc_rockchip fe2b.mmc: DW MMC controller at irq 31,32 bit host data width,256 deep fifo [5.179484] dwmmc_rockchip fe2b.mmc: Got CD GPIO [5.193439] mmc_host mmc0: Bus speed (slot 0) = 375000Hz (slot req 40Hz, actual 375000HZ div = 0) [5.259322] mmc_host mmc0: Bus speed (slot 0) = 15000Hz (slot req 15000Hz, actual 15000HZ div = 0) [5.332873] dwmmc_rockchip fe2b.mmc: Successfully tuned phase to 254 [5.332922] mmc0: new ultra high speed SDR104 SDHC card at address 59b4 [5.335025] mmcblk0: mmc0:59b4 0 14.9 GiB [5.345735] mmcblk0: p1 p2 [5.563710] dwmmc_rockchip fe2c.mmc: IDMAC supports 32-bit address mode. [5.564608] dwmmc_rockchip fe2c.mmc: Using internal DMA
Re: [PATCH v2 2/4] drm/mediatek: dsi: Cleanup functions mtk_dsi_ps_control{_vact}()
Hi Angelo,
On Wed, Dec 20, 2023 at 9:57 PM AngeloGioacchino Del Regno
wrote:
>
> Function mtk_dsi_ps_control() is a subset of mtk_dsi_ps_control_vact():
> merge the two in one mtk_dsi_ps_control() function by adding one
> function parameter `config_vact` which, when true, writes the VACT
> related registers.
>
> Signed-off-by: AngeloGioacchino Del Regno
>
> ---
> drivers/gpu/drm/mediatek/mtk_dsi.c | 76 +-
> 1 file changed, 23 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
> b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 23d2c5be8dbb..b618e2e31022 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -352,40 +352,6 @@ static void mtk_dsi_set_vm_cmd(struct mtk_dsi *dsi)
> mtk_dsi_mask(dsi, DSI_VM_CMD_CON, TS_VFP_EN, TS_VFP_EN);
> }
>
> -static void mtk_dsi_ps_control_vact(struct mtk_dsi *dsi)
> -{
> - struct videomode *vm = >vm;
> - u32 dsi_buf_bpp, ps_wc;
> - u32 ps_bpp_mode;
> -
> - if (dsi->format == MIPI_DSI_FMT_RGB565)
> - dsi_buf_bpp = 2;
> - else
> - dsi_buf_bpp = 3;
> -
> - ps_wc = vm->hactive * dsi_buf_bpp;
> - ps_bpp_mode = ps_wc;
> -
> - switch (dsi->format) {
> - case MIPI_DSI_FMT_RGB888:
> - ps_bpp_mode |= PACKED_PS_24BIT_RGB888;
> - break;
> - case MIPI_DSI_FMT_RGB666:
> - ps_bpp_mode |= PACKED_PS_18BIT_RGB666;
> - break;
> - case MIPI_DSI_FMT_RGB666_PACKED:
> - ps_bpp_mode |= LOOSELY_PS_18BIT_RGB666;
> - break;
> - case MIPI_DSI_FMT_RGB565:
> - ps_bpp_mode |= PACKED_PS_16BIT_RGB565;
> - break;
> - }
> -
> - writel(vm->vactive, dsi->regs + DSI_VACT_NL);
> - writel(ps_bpp_mode, dsi->regs + DSI_PSCTRL);
> - writel(ps_wc, dsi->regs + DSI_HSTX_CKL_WC);
> -}
> -
> static void mtk_dsi_rxtx_control(struct mtk_dsi *dsi)
> {
> u32 tmp_reg;
> @@ -417,36 +383,40 @@ static void mtk_dsi_rxtx_control(struct mtk_dsi *dsi)
> writel(tmp_reg, dsi->regs + DSI_TXRX_CTRL);
> }
>
> -static void mtk_dsi_ps_control(struct mtk_dsi *dsi)
> +static void mtk_dsi_ps_control(struct mtk_dsi *dsi, bool config_vact)
> {
> - u32 dsi_tmp_buf_bpp;
> - u32 tmp_reg;
> + struct videomode *vm = >vm;
> + u32 dsi_buf_bpp, ps_wc;
> + u32 ps_bpp_mode;
> +
> + if (dsi->format == MIPI_DSI_FMT_RGB565)
> + dsi_buf_bpp = 2;
> + else
> + dsi_buf_bpp = 3;
The same is also in mtk_dsi_config_vdo_timing(). Given this is a
cleanup series, I think it'd be a good chance to add another patch
and integrate those usages. Just a thought. :)
>
> +
> + ps_wc = vm->hactive * dsi_buf_bpp;
I noticed the "& DSI_PS_WC" part was dropped (but perhaps with awareness?).
While the outcome seems to always fall within the range of
DSI_PS_WC so we should be fine in practice, I think it doesn't hurt to
keep the value masked to save readers some time to check if this would
ever be possible to overflow and write undesired bits down the line.
WDYT?
Regardless of that, I didn't find obvious issue in this patch, so
Reviewed-by: Fei Shao
Regards,
Fei
>
> + ps_bpp_mode = ps_wc;
>
> switch (dsi->format) {
> case MIPI_DSI_FMT_RGB888:
> - tmp_reg = PACKED_PS_24BIT_RGB888;
> - dsi_tmp_buf_bpp = 3;
> + ps_bpp_mode |= PACKED_PS_24BIT_RGB888;
> break;
> case MIPI_DSI_FMT_RGB666:
> - tmp_reg = LOOSELY_PS_18BIT_RGB666;
> - dsi_tmp_buf_bpp = 3;
> + ps_bpp_mode |= PACKED_PS_18BIT_RGB666;
> break;
> case MIPI_DSI_FMT_RGB666_PACKED:
> - tmp_reg = PACKED_PS_18BIT_RGB666;
> - dsi_tmp_buf_bpp = 3;
> + ps_bpp_mode |= LOOSELY_PS_18BIT_RGB666;
> break;
> case MIPI_DSI_FMT_RGB565:
> - tmp_reg = PACKED_PS_16BIT_RGB565;
> - dsi_tmp_buf_bpp = 2;
> - break;
> - default:
>
> - tmp_reg = PACKED_PS_24BIT_RGB888;
> - dsi_tmp_buf_bpp = 3;
> + ps_bpp_mode |= PACKED_PS_16BIT_RGB565;
> break;
> }
>
> - tmp_reg += dsi->vm.hactive * dsi_tmp_buf_bpp & DSI_PS_WC;
>
> - writel(tmp_reg, dsi->regs + DSI_PSCTRL);
> + if (config_vact) {
> + writel(vm->vactive, dsi->regs + DSI_VACT_NL);
> + writel(ps_wc, dsi->regs + DSI_HSTX_CKL_WC);
> + }
> + writel(ps_bpp_mode, dsi->regs + DSI_PSCTRL);
> }
>
> static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
> @@ -522,7 +492,7 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
> writel(horizontal_backporch_byte, dsi->regs + DSI_HBP_WC);
> writel(horizontal_frontporch_byte, dsi->regs + DSI_HFP_WC);
>
>
Re: [PATCH v2 1/4] drm/mediatek: dsi: Use GENMASK() for register mask definitions
Hi Angelo, On Wed, Dec 20, 2023 at 9:57 PM AngeloGioacchino Del Regno wrote: > > Change magic numerical masks with usage of the GENMASK() macro > to improve readability. > > This commit brings no functional changes. > > Signed-off-by: AngeloGioacchino Del Regno > > --- > drivers/gpu/drm/mediatek/mtk_dsi.c | 46 -- > 1 file changed, 24 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c > b/drivers/gpu/drm/mediatek/mtk_dsi.c > index a2fdfc8ddb15..23d2c5be8dbb 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > @@ -58,18 +58,18 @@ > > #define DSI_TXRX_CTRL 0x18 > #define VC_NUM BIT(1) > -#define LANE_NUM (0xf << 2) > +#define LANE_NUM GENMASK(5, 2) > #define DIS_EOTBIT(6) > #define NULL_ENBIT(7) > #define TE_FREERUN BIT(8) > #define EXT_TE_EN BIT(9) > #define EXT_TE_EDGEBIT(10) > -#define MAX_RTN_SIZE (0xf << 12) > +#define MAX_RTN_SIZE GENMASK(15, 12) > #define HSTX_CKLP_EN BIT(16) > > #define DSI_PSCTRL 0x1c > -#define DSI_PS_WC 0x3fff > -#define DSI_PS_SEL (3 << 16) > +#define DSI_PS_WC GENMASK(14, 0) > +#define DSI_PS_SEL GENMASK(19, 16) GENMASK(17, 16) > > #define PACKED_PS_16BIT_RGB565 (0 << 16) > #define LOOSELY_PS_18BIT_RGB666(1 << 16) > #define PACKED_PS_18BIT_RGB666 (2 << 16) > @@ -109,26 +109,27 @@ > #define LD0_WAKEUP_EN BIT(2) > > #define DSI_PHY_TIMECON0 0x110 > -#define LPX(0xff << 0) > -#define HS_PREP(0xff << 8) > -#define HS_ZERO(0xff << 16) > -#define HS_TRAIL (0xff << 24) > +#define LPXGENMASK(7, 0) > +#define HS_PREPGENMASK(15, 8) > +#define HS_ZEROGENMASK(23, 16) > +#define HS_TRAIL GENMASK(31, 24) > > #define DSI_PHY_TIMECON1 0x114 > -#define TA_GO (0xff << 0) > -#define TA_SURE(0xff << 8) > -#define TA_GET (0xff << 16) > -#define DA_HS_EXIT (0xff << 24) > +#define TA_GO GENMASK(7, 0) > +#define TA_SUREGENMASK(15, 8) > +#define TA_GET GENMASK(23, 16) > +#define DA_HS_EXIT GENMASK(31, 24) > > #define DSI_PHY_TIMECON2 0x118 > -#define CONT_DET (0xff << 0) > -#define CLK_ZERO (0xff << 16) > -#define CLK_TRAIL (0xff << 24) > +#define CONT_DET GENMASK(7, 0) > +#define DA_HS_SYNC GENMASK(15, 8) This is new, so please introduce it in a separate patch if intended. The rest looks good to me. Regards, Fei > > +#define CLK_ZERO GENMASK(23, 16) > +#define CLK_TRAIL GENMASK(31, 24) > > #define DSI_PHY_TIMECON3 0x11c > -#define CLK_HS_PREP(0xff << 0) > -#define CLK_HS_POST(0xff << 8) > -#define CLK_HS_EXIT(0xff << 16) > +#define CLK_HS_PREPGENMASK(7, 0) > +#define CLK_HS_POSTGENMASK(15, 8) > +#define CLK_HS_EXITGENMASK(23, 16) > > #define DSI_VM_CMD_CON 0x130 > #define VM_CMD_EN BIT(0) > @@ -138,13 +139,14 @@ > #define FORCE_COMMIT BIT(0) > #define BYPASS_SHADOW BIT(1) > > -#define CONFIG (0xff << 0) > +/* CMDQ related bits */ > +#define CONFIG GENMASK(7, 0) > #define SHORT_PACKET 0 > #define LONG_PACKET2 > #define BTABIT(2) > -#define DATA_ID(0xff << 8) > -#define DATA_0 (0xff << 16) > -#define DATA_1 (0xff << 24) > +#define DATA_IDGENMASK(15, 8) > +#define DATA_0 GENMASK(23, 16) > +#define DATA_1 GENMASK(31, 24) > > #define NS_TO_CYCLE(n, c)((n) / (c) + (((n) % (c)) ? 1 : 0)) > > -- > 2.43.0 > >
Re: [PATCH 3/3] drm: property: Improve four size determinations
The whole series is: Reviewed-by: Simon Ser
[PATCH 1/3] drm: property: One function call less in drm_property_create() after error detection
From: Markus Elfring Date: Tue, 26 Dec 2023 08:44:37 +0100 The kfree() function was called in one case by the drm_property_create() function during error handling even if the passed data structure member contained a null pointer. This issue was detected by using the Coccinelle software. Thus use another label. Signed-off-by: Markus Elfring --- drivers/gpu/drm/drm_property.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c index 596272149a35..3440f4560e6e 100644 --- a/drivers/gpu/drm/drm_property.c +++ b/drivers/gpu/drm/drm_property.c @@ -117,7 +117,7 @@ struct drm_property *drm_property_create(struct drm_device *dev, property->values = kcalloc(num_values, sizeof(uint64_t), GFP_KERNEL); if (!property->values) - goto fail; + goto free_property; } ret = drm_mode_object_add(dev, >base, DRM_MODE_OBJECT_PROPERTY); @@ -135,6 +135,7 @@ struct drm_property *drm_property_create(struct drm_device *dev, return property; fail: kfree(property->values); +free_property: kfree(property); return NULL; } -- 2.43.0
[PATCH 3/3] drm: property: Improve four size determinations
From: Markus Elfring Date: Tue, 26 Dec 2023 09:45:36 +0100 Replace the specification of data structures by pointer dereferences as the parameter for the operator “sizeof” to make the corresponding size determination a bit safer according to the Linux coding style convention. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/gpu/drm/drm_property.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c index ea365f00e890..1fe54cbf1c83 100644 --- a/drivers/gpu/drm/drm_property.c +++ b/drivers/gpu/drm/drm_property.c @@ -107,7 +107,7 @@ struct drm_property *drm_property_create(struct drm_device *dev, if (WARN_ON(strlen(name) >= DRM_PROP_NAME_LEN)) return NULL; - property = kzalloc(sizeof(struct drm_property), GFP_KERNEL); + property = kzalloc(sizeof(*property), GFP_KERNEL); if (!property) return NULL; @@ -418,7 +418,7 @@ int drm_property_add_enum(struct drm_property *property, if (WARN_ON(index >= property->num_values)) return -EINVAL; - prop_enum = kzalloc(sizeof(struct drm_property_enum), GFP_KERNEL); + prop_enum = kzalloc(sizeof(*prop_enum), GFP_KERNEL); if (!prop_enum) return -ENOMEM; @@ -560,10 +560,10 @@ drm_property_create_blob(struct drm_device *dev, size_t length, struct drm_property_blob *blob; int ret; - if (!length || length > INT_MAX - sizeof(struct drm_property_blob)) + if (!length || length > INT_MAX - sizeof(*blob)) return ERR_PTR(-EINVAL); - blob = kvzalloc(sizeof(struct drm_property_blob)+length, GFP_KERNEL); + blob = kvzalloc(sizeof(*blob) + length, GFP_KERNEL); if (!blob) return ERR_PTR(-ENOMEM); -- 2.43.0
[PATCH 2/3] drm: property: Delete an unnecessary initialisation in drm_property_create()
From: Markus Elfring
Date: Tue, 26 Dec 2023 08:46:12 +0100
The variable “property” will eventually be set to an appropriate pointer
a bit later. Thus omit the explicit initialisation at the beginning.
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/drm_property.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
index 3440f4560e6e..ea365f00e890 100644
--- a/drivers/gpu/drm/drm_property.c
+++ b/drivers/gpu/drm/drm_property.c
@@ -98,7 +98,7 @@ struct drm_property *drm_property_create(struct drm_device
*dev,
u32 flags, const char *name,
int num_values)
{
- struct drm_property *property = NULL;
+ struct drm_property *property;
int ret;
if (WARN_ON(!drm_property_flags_valid(flags)))
--
2.43.0
[PATCH 0/3] drm: property: Adjustments for three function implementations
From: Markus Elfring Date: Tue, 26 Dec 2023 10:26:36 +0100 A few update suggestions were taken into account from static source code analysis. Markus Elfring (3): One function call less in drm_property_create() after error detection Delete an unnecessary initialisation in drm_property_create() Improve four size determinations drivers/gpu/drm/drm_property.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) -- 2.43.0
