Re: [PATCH] MAINTAINERS: add Melissa to V3D maintainers
Acked-by: Iago Toral Quiroga On Fri, 2022-04-29 at 18:33 -0100, Melissa Wen wrote: > I've been contributing to v3d through improvements, reviews, testing, > debugging, etc. So, I'm adding myself as a co-maintainer of the V3D > driver. > > Signed-off-by: Melissa Wen > --- > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 9b2b0dc44506..0a854b7f2491 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6714,6 +6714,7 @@ F: drivers/gpu/drm/omapdrm/ > > DRM DRIVERS FOR V3D > M: Emma Anholt > +M: Melissa Wen > S: Supported > T: git git://anongit.freedesktop.org/drm/drm-misc > F: Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
[PATCH 3/3] drm/i915: Enabling WD Transcoder
Adding support for writeback transcoder to start capturing frames using
interrupt mechanism
Signed-off-by: Suraj Kandpal
---
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/display/intel_acpi.c | 1 +
drivers/gpu/drm/i915/display/intel_display.c | 89 +-
drivers/gpu/drm/i915/display/intel_display.h | 9 +
.../drm/i915/display/intel_display_types.h| 13 +
drivers/gpu/drm/i915/display/intel_dpll.c | 3 +
drivers/gpu/drm/i915/display/intel_opregion.c | 3 +
drivers/gpu/drm/i915/display/intel_wd.c | 978 ++
drivers/gpu/drm/i915/display/intel_wd.h | 82 ++
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/i915_irq.c | 8 +-
drivers/gpu/drm/i915/i915_pci.c | 7 +-
drivers/gpu/drm/i915/i915_reg.h | 137 +++
13 files changed, 1330 insertions(+), 3 deletions(-)
create mode 100644 drivers/gpu/drm/i915/display/intel_wd.c
create mode 100644 drivers/gpu/drm/i915/display/intel_wd.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 8bd1dd01dc81..7a665d95e2b2 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -299,6 +299,7 @@ i915-y += \
display/intel_vdsc.o \
display/intel_vrr.o \
display/intel_wb_connector.o\
+ display/intel_wd.o\
display/vlv_dsi.o \
display/vlv_dsi_pll.o
diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c
b/drivers/gpu/drm/i915/display/intel_acpi.c
index e78430001f07..ae08db164f73 100644
--- a/drivers/gpu/drm/i915/display/intel_acpi.c
+++ b/drivers/gpu/drm/i915/display/intel_acpi.c
@@ -247,6 +247,7 @@ static u32 acpi_display_type(struct intel_connector
*connector)
case DRM_MODE_CONNECTOR_LVDS:
case DRM_MODE_CONNECTOR_eDP:
case DRM_MODE_CONNECTOR_DSI:
+ case DRM_MODE_CONNECTOR_WRITEBACK:
display_type = ACPI_DISPLAY_TYPE_INTERNAL_DIGITAL;
break;
case DRM_MODE_CONNECTOR_Unknown:
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 0decf3d24237..451dbc02c69b 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -113,6 +113,7 @@
#include "intel_sprite.h"
#include "intel_tc.h"
#include "intel_vga.h"
+#include "intel_wd.h"
#include "i9xx_plane.h"
#include "skl_scaler.h"
#include "skl_universal_plane.h"
@@ -1551,6 +1552,72 @@ static void intel_encoders_update_complete(struct
intel_atomic_state *state)
}
}
+static void intel_queue_writeback_job(struct intel_atomic_state *state,
+ struct intel_crtc *intel_crtc, struct intel_crtc_state
*crtc_state)
+{
+ struct drm_connector_state *new_conn_state;
+ struct drm_connector *connector;
+ struct drm_i915_private *i915 = to_i915(intel_crtc->base.dev);
+ struct intel_wd *intel_wd;
+ struct intel_connector *intel_connector;
+ struct intel_digital_connector_state *intel_conn_state;
+ struct intel_encoder *encoder;
+ int i;
+
+ for_each_intel_encoder_with_wd(>drm, encoder) {
+ intel_wd = enc_to_intel_wd(encoder);
+
+ if (intel_wd->wd_crtc != intel_crtc)
+ return;
+
+ }
+
+ for_each_new_connector_in_state(>base, connector, new_conn_state,
+ i) {
+ intel_conn_state =
to_intel_digital_connector_state(new_conn_state);
+ if (!intel_conn_state->job)
+ continue;
+ intel_connector = to_intel_connector(connector);
+ intel_writeback_queue_job(_connector->wb_conn,
new_conn_state);
+ drm_dbg_kms(>drm, "queueing writeback job\n");
+ }
+}
+
+static void intel_find_writeback_connector(struct intel_atomic_state *state,
+ struct intel_crtc *intel_crtc, struct intel_crtc_state
*crtc_state)
+{
+ struct drm_connector_state *new_conn_state;
+ struct drm_connector *connector;
+ struct drm_i915_private *i915 = to_i915(intel_crtc->base.dev);
+ struct intel_wd *intel_wd;
+ struct intel_encoder *encoder;
+ int i;
+
+ for_each_intel_encoder_with_wd(>drm, encoder) {
+ intel_wd = enc_to_intel_wd(encoder);
+
+ if (intel_wd->wd_crtc != intel_crtc)
+ return;
+
+ }
+
+ for_each_new_connector_in_state(>base, connector, new_conn_state,
+ i) {
+ struct intel_connector *intel_connector;
+
+ intel_connector = to_intel_connector(connector);
+ drm_dbg_kms(>drm, "[CONNECTOR:%d:%s]: status: %s\n",
+ connector->base.id, connector->name,
+
drm_get_connector_status_name(connector->status));
+ encoder =
[PATCH 2/3] drm/i915: Define WD trancoder for i915
Adding WD Types, WD transcoder to enum list and WD Transcoder offsets
Signed-off-by: Suraj Kandpal
---
drivers/gpu/drm/i915/display/intel_display.h | 6 ++
drivers/gpu/drm/i915/display/intel_display_types.h | 1 +
drivers/gpu/drm/i915/i915_reg.h| 2 ++
3 files changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_display.h
b/drivers/gpu/drm/i915/display/intel_display.h
index 187910d94ec6..62dc26b3bf76 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -119,6 +119,8 @@ enum transcoder {
TRANSCODER_DSI_1,
TRANSCODER_DSI_A = TRANSCODER_DSI_0,/* legacy DSI */
TRANSCODER_DSI_C = TRANSCODER_DSI_1,/* legacy DSI */
+ TRANSCODER_WD_0,
+ TRANSCODER_WD_1,
I915_MAX_TRANSCODERS
};
@@ -140,6 +142,10 @@ static inline const char *transcoder_name(enum transcoder
transcoder)
return "DSI A";
case TRANSCODER_DSI_C:
return "DSI C";
+ case TRANSCODER_WD_0:
+ return "WD 0";
+ case TRANSCODER_WD_1:
+ return "WD 1";
default:
return "";
}
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index c4ccb03424f1..be47f7ba592c 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -79,6 +79,7 @@ enum intel_output_type {
INTEL_OUTPUT_DSI = 9,
INTEL_OUTPUT_DDI = 10,
INTEL_OUTPUT_DP_MST = 11,
+ INTEL_OUTPUT_WD = 12,
};
enum hdmi_force_audio {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9ccb67eec1bd..e9f6301a6394 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2044,6 +2044,8 @@
#define TRANSCODER_EDP_OFFSET 0x6f000
#define TRANSCODER_DSI0_OFFSET 0x6b000
#define TRANSCODER_DSI1_OFFSET 0x6b800
+#define TRANSCODER_WD0_OFFSET 0x6e000
+#define TRANSCODER_WD1_OFFSET 0x6e800
#define HTOTAL(trans) _MMIO_TRANS2(trans, _HTOTAL_A)
#define HBLANK(trans) _MMIO_TRANS2(trans, _HBLANK_A)
--
2.35.1
[PATCH 1/3] drm/i915: Creating writeback pipeline to bypass drm_writeback framework
Changes to create a i915 private pipeline to enable the WD transcoder
without relying on the current drm_writeback framework.
Signed-off-by: Suraj Kandpal
---
drivers/gpu/drm/i915/Makefile | 1 +
.../drm/i915/display/intel_display_types.h| 4 +
.../gpu/drm/i915/display/intel_wb_connector.c | 296 ++
.../gpu/drm/i915/display/intel_wb_connector.h | 99 ++
drivers/gpu/drm/i915/i915_drv.h | 3 +
5 files changed, 403 insertions(+)
create mode 100644 drivers/gpu/drm/i915/display/intel_wb_connector.c
create mode 100644 drivers/gpu/drm/i915/display/intel_wb_connector.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index d2b18f03a33c..8bd1dd01dc81 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -298,6 +298,7 @@ i915-y += \
display/intel_tv.o \
display/intel_vdsc.o \
display/intel_vrr.o \
+ display/intel_wb_connector.o\
display/vlv_dsi.o \
display/vlv_dsi_pll.o
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 408152f9f46a..c4ccb03424f1 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -52,6 +52,7 @@
#include "intel_display_power.h"
#include "intel_dpll_mgr.h"
#include "intel_pm_types.h"
+#include "intel_wb_connector.h"
struct drm_printer;
struct __intel_global_objs_state;
@@ -537,11 +538,14 @@ struct intel_connector {
struct work_struct modeset_retry_work;
struct intel_hdcp hdcp;
+
+ struct intel_writeback_connector wb_conn;
};
struct intel_digital_connector_state {
struct drm_connector_state base;
+ struct intel_writeback_job *job;
enum hdmi_force_audio force_audio;
int broadcast_rgb;
};
diff --git a/drivers/gpu/drm/i915/display/intel_wb_connector.c
b/drivers/gpu/drm/i915/display/intel_wb_connector.c
new file mode 100644
index ..65f4abef53d0
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_wb_connector.c
@@ -0,0 +1,296 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright © 2022 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ * Suraj Kandpal
+ * Arun Murthy
+ *
+ */
+
+
+#include
+
+#include
+#include
+#include
+#include
+#include
+
+#include "i915_drv.h"
+#include "intel_wb_connector.h"
+#include "intel_display_types.h"
+
+#define fence_to_wb_connector(x) container_of(x->lock, \
+ struct intel_writeback_connector,
\
+ fence_lock)
+
+static const char *intel_writeback_fence_get_driver_name(struct dma_fence
*fence)
+{
+ struct intel_writeback_connector *wb_connector =
+ fence_to_wb_connector(fence);
+
+ return wb_connector->base->dev->driver->name;
+}
+
+static const char *
+intel_writeback_fence_get_timeline_name(struct dma_fence *fence)
+{
+ struct intel_writeback_connector *wb_connector =
+ fence_to_wb_connector(fence);
+
+ return wb_connector->timeline_name;
+}
+
+static bool intel_writeback_fence_enable_signaling(struct dma_fence *fence)
+{
+ return true;
+}
+
+static const struct dma_fence_ops intel_writeback_fence_ops = {
+ .get_driver_name = intel_writeback_fence_get_driver_name,
+ .get_timeline_name = intel_writeback_fence_get_timeline_name,
+ .enable_signaling = intel_writeback_fence_enable_signaling,
+};
+
+static int intel_create_writeback_properties(struct drm_device *dev)
+{
+ struct drm_property *prop;
+ struct drm_i915_private *i915 = to_i915(dev);
+
+ if (!i915->wb_fb_id_property) {
+ prop = drm_property_create_object(dev, DRM_MODE_PROP_ATOMIC,
+
[PATCH 0/3] i915 private writeback framework
A patch series was floated in the drm mailing list which aimed to change the drm_connector and drm_encoder fields to pointer in the drm_connector_writeback structure, this received a huge pushback from the community but since i915 expects each connector present in the drm_device list to be a intel_connector but drm_writeback framework. [1] https://patchwork.kernel.org/project/dri-devel/patch/20220202081702.22119-1-suraj.kand...@intel.com/ [2] https://patchwork.kernel.org/project/dri-devel/patch/20220202085429.22261-6-suraj.kand...@intel.com/ This forces us to use a drm_connector which is not embedded in intel_connector the current drm_writeback framework becomes very unfeasible to us as it would mean a lot of checks at a lot of places to take into account the above issue.Since no one had an issue with encoder field being changed into a pointer it was decided to break the connector and encoder pointer changes into two different series.The encoder field changes is currently being worked upon by Abhinav Kumar [3]https://patchwork.kernel.org/project/dri-devel/list/?series=633565 In the meantime for i915 to start using the writeback functionality we came up with a interim solution to own writeback pipeline bypassing one provided by drm which is what these patches do. Note: these are temp patches till we figure out how we can either change drm core writeback to work with our intel_connector structure or find a different solution which allows us to work with the current Suraj Kandpal (3): drm/i915: Creating writeback pipeline to bypass drm_writeback framework drm/i915: Define WD trancoder for i915 drm/i915: Enabling WD Transcoder drivers/gpu/drm/i915/Makefile | 2 + drivers/gpu/drm/i915/display/intel_acpi.c | 1 + drivers/gpu/drm/i915/display/intel_display.c | 89 +- drivers/gpu/drm/i915/display/intel_display.h | 15 + .../drm/i915/display/intel_display_types.h| 18 + drivers/gpu/drm/i915/display/intel_dpll.c | 3 + drivers/gpu/drm/i915/display/intel_opregion.c | 3 + .../gpu/drm/i915/display/intel_wb_connector.c | 296 ++ .../gpu/drm/i915/display/intel_wb_connector.h | 99 ++ drivers/gpu/drm/i915/display/intel_wd.c | 978 ++ drivers/gpu/drm/i915/display/intel_wd.h | 82 ++ drivers/gpu/drm/i915/i915_drv.h | 5 + drivers/gpu/drm/i915/i915_irq.c | 8 +- drivers/gpu/drm/i915/i915_pci.c | 7 +- drivers/gpu/drm/i915/i915_reg.h | 139 +++ 15 files changed, 1742 insertions(+), 3 deletions(-) create mode 100644 drivers/gpu/drm/i915/display/intel_wb_connector.c create mode 100644 drivers/gpu/drm/i915/display/intel_wb_connector.h create mode 100644 drivers/gpu/drm/i915/display/intel_wd.c create mode 100644 drivers/gpu/drm/i915/display/intel_wd.h -- 2.35.1
[PATCH] drm/msm/dpu: add missing break statement for update_pending_flush_wb()
Add missing break statement for dpu_hw_ctl_update_pending_flush_wb().
Otherwise this leads to below build warning.
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c:273:2:
warning: unannotated fall-through between switch labels
default:
^
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c:273:2:
note: insert 'break;' to avoid fall-through
default:
^
break;
1 warning generated.
Fixes: 2e0086d8c61d ("drm/msm/dpu: add changes to support writeback in hw_ctl")
Reported-by: kernel test robot
Signed-off-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index 254fdf06bb42..c33e7ef611a6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -270,6 +270,7 @@ static void dpu_hw_ctl_update_pending_flush_wb(struct
dpu_hw_ctl *ctx,
case WB_1:
case WB_2:
ctx->pending_flush_mask |= BIT(WB_IDX);
+ break;
default:
break;
}
--
2.7.4
Re: [PATCH 0/3] HDR aux backlight range calculation
On Fri, 2022-04-29 at 19:34 -0400, Lyude Paul wrote: > Cool! Tested this on three different laptops, and it seems to work > great on > all of them. so, this series is: > > Tested-by: Lyude Paul Thank you all for review/testing support. I will come back with updated patch set later. > > Would review, but I basically have the same comments as jani > > On Tue, 2022-04-26 at 15:30 +0300, Jouni Högander wrote: > > This patch set splits out static hdr metadata backlight range > > parsing > > from gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c into gpu/drm/drm- > > edid.c as > > a new function. This new function is then used in admgpu_dm.c and > > intel_dp_aux_backlight.c > > > > Cc: Maarten Lankhorst > > Cc: Rodrigo Siqueira > > Cc: Harry Wentland > > Cc: Lyude Paul > > Cc: Mika Kahola > > Cc: Jani Nikula > > > > Jouni Högander (3): > > drm: New function to get luminance range based on static hdr > > metadata > > drm/amdgpu_dm: Use split out luminance calculation function > > drm/i915: Use luminance range from static hdr metadata > > > > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 35 ++-- > > drivers/gpu/drm/drm_edid.c| 55 > > +++ > > .../drm/i915/display/intel_dp_aux_backlight.c | 9 ++- > > include/drm/drm_edid.h| 4 ++ > > 4 files changed, 70 insertions(+), 33 deletions(-) > >
[linux-next:master] BUILD REGRESSION 5469f0c06732a077c70a759a81f2a1f00b277694
dgpu-soc21.c:warning:no-previous-prototype-for-soc21_grbm_select `-- xtensa-allyesconfig |-- drivers-dma-buf-st-dma-fence-unwrap.c:warning:variable-err-set-but-not-used |-- drivers-gpu-drm-amd-amdgpu-amdgpu_discovery.c:warning:no-previous-prototype-for-amdgpu_discovery_get_mall_info `-- drivers-gpu-drm-amd-amdgpu-soc21.c:warning:no-previous-prototype-for-soc21_grbm_select clang_recent_errors |-- arm-randconfig-c002-20220428 | |-- drivers-gpu-drm-amd-amdgpu-amdgpu_discovery.c:warning:no-previous-prototype-for-function-amdgpu_discovery_get_mall_info | |-- drivers-gpu-drm-display-drm_dp_helper.c:warning:Undefined-or-garbage-value-returned-to-caller-clang-analyzer-core.uninitialized.UndefReturn | `-- drivers-gpu-drm-solomon-ssd13.c:warning:Undefined-or-garbage-value-returned-to-caller-clang-analyzer-core.uninitialized.UndefReturn |-- hexagon-randconfig-r041-20220428 | `-- drivers-hid-wacom_wac.c:warning:format-specifies-type-unsigned-short-but-the-argument-has-type-int |-- i386-allmodconfig | `-- drivers-hid-wacom_wac.c:warning:format-specifies-type-unsigned-short-but-the-argument-has-type-int |-- i386-randconfig-a013-20220502 | |-- drivers-gpu-drm-i915-gt-intel_gt_sysfs_pm.c:error:call-to-undeclared-function-sysfs_gt_attribute_r_max_func-ISO-C99-and-later-do-not-support-implicit-function-declarations | |-- drivers-gpu-drm-i915-gt-intel_gt_sysfs_pm.c:error:call-to-undeclared-function-sysfs_gt_attribute_r_min_func-ISO-C99-and-later-do-not-support-implicit-function-declarations | `-- drivers-gpu-drm-i915-gt-intel_gt_sysfs_pm.c:error:call-to-undeclared-function-sysfs_gt_attribute_w_func-ISO-C99-and-later-do-not-support-implicit-function-declarations |-- i386-randconfig-a015 | `-- drivers-hid-wacom_wac.c:warning:format-specifies-type-unsigned-short-but-the-argument-has-type-int |-- i386-randconfig-c001 | |-- kernel-module-main.c:warning:Call-to-function-sprintf-is-insecure-as-it-does-not-provide-bounding-of-the-memory-buffer-or-security-checks-introduced-in-the-C11-standard.-Replace-with-analogous-functio | |-- kernel-module-main.c:warning:Null-pointer-passed-as-1st-argument-to-memory-copy-function-clang-analyzer-unix.cstring.NullArg | `-- net-ipv4-tcp_cong.c:warning:Division-by-zero-clang-analyzer-core.DivideZero |-- riscv-buildonly-randconfig-r005-20220428 | |-- drivers-gpu-drm-amd-amdgpu-amdgpu_discovery.c:warning:no-previous-prototype-for-function-amdgpu_discovery_get_mall_info | `-- drivers-gpu-drm-amd-amdgpu-soc21.c:warning:no-previous-prototype-for-function-soc21_grbm_select |-- riscv-randconfig-c006-20220501 | `-- mm-page_io.c:warning:Value-stored-to-page-during-its-initialization-is-never-read-clang-analyzer-deadcode.DeadStores |-- x86_64-randconfig-a003 | `-- drivers-hid-wacom_wac.c:warning:format-specifies-type-unsigned-short-but-the-argument-has-type-int |-- x86_64-randconfig-a014 | `-- drivers-hid-wacom_wac.c:warning:format-specifies-type-unsigned-short-but-the-argument-has-type-int |-- x86_64-randconfig-a016 | `-- drivers-hid-wacom_wac.c:warning:format-specifies-type-unsigned-short-but-the-argument-has-type-int `-- x86_64-randconfig-c007 |-- kernel-module-main.c:warning:Call-to-function-sprintf-is-insecure-as-it-does-not-provide-bounding-of-the-memory-buffer-or-security-checks-introduced-in-the-C11-standard.-Replace-with-analogous-functio |-- kernel-module-main.c:warning:Null-pointer-passed-as-1st-argument-to-memory-copy-function-clang-analyzer-unix.cstring.NullArg |-- mm-sparse-vmemmap.c:warning:Value-stored-to-next-during-its-initialization-is-never-read-clang-analyzer-deadcode.DeadStores `-- net-ipv4-tcp_cong.c:warning:Division-by-zero-clang-analyzer-core.DivideZero elapsed time: 4104m configs tested: 133 configs skipped: 3 gcc tested configs: arm64 defconfig arm allmodconfig arm allyesconfig arm defconfig arm64allyesconfig mips allyesconfig riscvallyesconfig um x86_64_defconfig riscvallmodconfig um i386_defconfig mips allmodconfig powerpc allmodconfig m68k allyesconfig s390 allmodconfig m68k allmodconfig powerpc allyesconfig s390 allyesconfig sparcallyesconfig parisc allyesconfig sh allmodconfig h8300allyesconfig xtensa allyesconfig arc allyesconfig alphaallyesconfig nios2allyesconfig i386
Re: [Freedreno] [PATCH] drm/msm/disp/dpu1: avoid clearing hw interrupts if hw_intr is null during drm uninit
Looks like our new CI has given all the answers we need :) which is a
great win for the CI in my opinion.
Take a look at this report :
https://gitlab.freedesktop.org/drm/msm/-/jobs/22015361
This issue seems to be because this change
https://github.com/torvalds/linux/commit/169466d4e59ca204683998b7f45673ebf0eb2de6
is missing in our tree.
Without this change, what happens is that we are not hitting the return
0 because we check for ENODEV.
/*
* External bridges are mandatory for eDP interfaces: one has to
* provide at least an eDP panel (which gets wrapped into
panel-bridge).
*
* For DisplayPort interfaces external bridges are optional, so
* silently ignore an error if one is not present (-ENODEV).
*/
rc = dp_parser_find_next_bridge(dp_priv->parser);
if (!dp->is_edp && rc == -ENODEV)
return 0;
So, I think we should do both:
1) Since we are running CI on the tree, backport this change so that
this error path doesnt hit?
2) Add this protection as well because this shows that we can indeed hit
this path in EDEFER cases causing this crash.
Thanks
Abhinav
On 4/27/2022 3:53 AM, Dmitry Baryshkov wrote:
On 27/04/2022 00:50, Stephen Boyd wrote:
Quoting Vinod Polimera (2022-04-25 23:02:11)
Avoid clearing irqs and derefernce hw_intr when hw_intr is null.
Presumably this is only the case when the display driver doesn't fully
probe and something probe defers? Can you clarify how this situation
happens?
BUG: Unable to handle kernel NULL pointer dereference at virtual
address
Call trace:
dpu_core_irq_uninstall+0x50/0xb0
dpu_irq_uninstall+0x18/0x24
msm_drm_uninit+0xd8/0x16c
msm_drm_bind+0x580/0x5fc
try_to_bring_up_master+0x168/0x1c0
__component_add+0xb4/0x178
component_add+0x1c/0x28
dp_display_probe+0x38c/0x400
platform_probe+0xb0/0xd0
really_probe+0xcc/0x2c8
__driver_probe_device+0xbc/0xe8
driver_probe_device+0x48/0xf0
__device_attach_driver+0xa0/0xc8
bus_for_each_drv+0x8c/0xd8
__device_attach+0xc4/0x150
device_initial_probe+0x1c/0x28
Fixes: a73033619ea ("drm/msm/dpu: squash dpu_core_irq into
dpu_hw_interrupts")
The fixes tag looks odd. In dpu_core_irq_uninstall() at that commit it
is dealing with 'irq_obj' which isn't a pointer. After commit
f25f656608e3 ("drm/msm/dpu: merge struct dpu_irq into struct
dpu_hw_intr") dpu_core_irq_uninstall() starts using 'hw_intr' which is
allocated on the heap. If we backported this patch to a place that had
a73033619ea without f25f656608e3 it wouldn't make any sense.
I'd agree here. The following tag would be correct:
Fixes: f25f656608e3 ("drm/msm/dpu: merge struct dpu_irq into struct
dpu_hw_intr")
Signed-off-by: Vinod Polimera
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
index c515b7c..ab28577 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
@@ -599,6 +599,9 @@ void dpu_core_irq_uninstall(struct dpu_kms *dpu_kms)
{
int i;
+ if (!dpu_kms->hw_intr)
+ return;
+
pm_runtime_get_sync(_kms->pdev->dev);
for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++)
Re: [Freedreno] [PULL v3] drm/msm: display pull request for 5.19
Adding one more item: On 5/1/2022 4:41 PM, Dmitry Baryshkov wrote: Hi Rob, This is a pull request over the patches accumulated, reviewed and tested for the 5.19 merge window. This pull request contains following changes: - DPU: DSC (Display Stream Compression) support - DPU: Writeback support on SM8250 - DPU: inline rotation support on SC7280 - DPU: update DP timings to follow vendor recommendations - DP, DPU: add support for wide bus (on newer chipsets) - DP: eDP support - Merge DPU1 and MDP5 MDSS driver, make dpu/mdp device the master component - MDSS: optionally reset the IP block at the bootup to drop bootloader state - Properly register and unregister internal bridges in the DRM framework - Complete DPU IRQ cleanup - DP: conversion to use drm_bridge and drm_bridge_connector - eDP: drop old eDP parts again - Misc small fixes This pull request incorporates both patches from first and second pull requests. It was flattened to ease fixing the Fixes tags issues reported by Stephen Rothwell. The following changes since commit 78f815c1cf8fc5f05dc5cec29eb1895cb53470e9: drm/msm: return the average load over the polling period (2022-04-21 15:05:23 -0700) are available in the Git repository at: https://gitlab.freedesktop.org/lumag/msm.git msm-next-lumag for you to fetch changes up to f1fc2b87de4719cfa8e193e0746cc524dd9f7472: drm/msm: drop old eDP block support (again) (2022-05-02 02:39:35 +0300) Abhinav Kumar (21): drm/msm: remove unused hotplug and edid macros from msm_drv.h drm: allow passing possible_crtcs to drm_writeback_connector_init() drm: introduce drm_writeback_connector_init_with_encoder() API drm/msm/dpu: add writeback blocks to the sm8250 DPU catalog drm/msm/dpu: add reset_intf_cfg operation for dpu_hw_ctl drm/msm/dpu: rename dpu_hw_pipe_cdp_cfg to dpu_hw_cdp_cfg drm/msm/dpu: add dpu_hw_wb abstraction for writeback blocks drm/msm/dpu: add writeback blocks to DPU RM drm/msm/dpu: add changes to support writeback in hw_ctl drm/msm/dpu: add an API to reset the encoder related hw blocks drm/msm/dpu: make changes to dpu_encoder to support virtual encoder drm/msm/dpu: add encoder operations to prepare/cleanup wb job drm/msm/dpu: move _dpu_plane_get_qos_lut to dpu_hw_util file drm/msm/dpu: introduce the dpu_encoder_phys_* for writeback drm/msm/dpu: add the writeback connector layer drm/msm/dpu: initialize dpu encoder and connector for writeback drm/msm/dpu: gracefully handle null fb commits for writeback drm/msm/dpu: add writeback blocks to the display snapshot drm/msm/dpu: add wb_idx to existing DRM prints in dpu_encoder drm/msm/dpu: add wb_idx to DRM traces in dpu_encoder drm/msm/dpu: remove unused refcount for encoder_phys_wb Bjorn Andersson (2): dt-bindings: display: msm: Add optional resets drm/msm/dpu: Issue MDSS reset during initialization Dmitry Baryshkov (32): drm/msm: unify MDSS drivers drm/msm: remove extra indirection for msm_mdss drm/msm: split the main platform driver drm/msm: stop using device's match data pointer drm/msm: allow compile time selection of driver components drm/msm: make mdp5/dpu devices master components drm/msm: properly add and remove internal bridges drm/msm/dpu: remove manual destruction of DRM objects drm/msm: loop over encoders using drm_for_each_encoder() drm/msm: don't store created planes, connectors and encoders drm/msm: remove unused plane_property field from msm_drm_private drm/msm/dpu: don't use merge_3d if DSC merge topology is used drm/msm/dpu: remove extra wrappers around dpu_core_irq drm/msm/dpu: remove always-true argument of dpu_core_irq_read() drm/msm/dpu: allow just single IRQ callback drm/msm/dpu: get rid of dpu_encoder_helper_(un)register_irq drm/msm/dpu: remove struct dpu_encoder_irq drm/msm/dpu: pass irq to dpu_encoder_helper_wait_for_irq() drm/msm/dpu: document INTF_EDP/INTF_DP difference drm/msm/dpu: drop INTF_TYPE_MAX symbol drm/msm/dpu: drop obsolete INTF_EDP comment drm/msm/dpu: drop INTF_EDP from interface type conditions drm/msm/dp: replace dp_connector with drm_bridge_connector drm/msm/dp: remove extra wrappers and public functions drm/msm/dp: drop dp_mode argument from dp_panel_get_modes() drm/msm/dp: simplify dp_connector_get_modes() drm/msm/dp: remove max_pclk_khz field from dp_panel/dp_display drm/msm: select DRM_DP_AUX_BUS for the AUX bus support drm/msm/dsi: fix error checks and return values for DSI xmit functions drm/msm/dsi: use RMW cycles in dsi_update_dsc_timing drm/msm: add missing include to msm_drv.c drm/msm: drop old eDP
Re: [Freedreno] [PATCH v2] drm/msm/dsi: use RMW cycles in dsi_update_dsc_timing
On 5/1/2022 1:06 PM, Marijn Suijten wrote:
On 2022-04-30 12:25:57, Abhinav Kumar wrote:
On 4/30/2022 11:58 AM, Marijn Suijten wrote:
On 2022-04-30 20:55:33, Dmitry Baryshkov wrote:
The downstream uses read-modify-write for updating command mode
compression registers. Let's follow this approach. This also fixes the
following warning:
drivers/gpu/drm/msm/dsi/dsi_host.c:918:23: warning: variable 'reg_ctrl' set but
not used [-Wunused-but-set-variable]
Reported-by: kernel test robot
Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration")
Signed-off-by: Dmitry Baryshkov
I pointed this out in review multiple times, so you'll obviously get my:
Reviewed-by: Marijn Suijten
(But are you sure there's nothing else to clear in the 1st CTRL
register, only the lowest 16 bits? That should mean `reg` never
contains anything in 0x)
The top 16 bits contain information for stream 1.
Stream 1 is unused. And whatever is the reset value we should retain that.
So this patch is correct.
I was not debating the correctness of this patch, quite the contrary:
it's already much better than it was before.
I'm simply asking whether we should prevent `reg` (not `reg_ctrl`!)
from having anything in the top 16 bits, which would overwrite the reset
value for stream 1 which you correctly say should be retained as it is.
It seems unlikely that this happens, but better be safe than sorry?
Wouln't this macro already make sure that 'reg' doesnt have anything in
the top 16 bits? Its doing a & with 0x3f00
#define DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__MASK
0x3f00
#define DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__SHIFT 8
static inline uint32_t
DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE(uint32_t val)
{
return ((val) <<
DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__SHIFT) &
DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__MASK;
}
Did you have anything else in mind? If so, please suggest.
Looking at the DSI register definition for DSC [1] again, I wonder if
there's support for defining a common bitfield layout and reusing it
thrice: the two channels for CMD mode and a single use for VIDEO. Don't
think that'd make the kernel code better though if even possible at all.
We can have a common bitfield layout for the two channels for command mode.
So we can do something like below for common fields:
-static inline uint32_t
DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE(uint32_t val)
+static inline uint32_t
DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM_DATATYPE(uint32_t val, uint32_t
stream_id)
{
- return ((val) <<
DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__SHIFT) &
DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__MASK;
+ return ((val) <<
(DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__SHIFT + (stream_id
* 16)) & DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__MASK;
}
Video mode can also use all of these except for WC as that field is not
present for cmd modes.
This can go as a separate change .
I can push this and perhaps get a Tested-by from Vinod as I dont have a
setup to re-validate this.
Thanks
Abhinav
[1]: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/14967/diffs
- Marijn
However, this seems to indicate that the DSC patch series has been
approved and merged somehow??
---
Changes since v1:
- Fix c error and apply mask clear to reg_ctrl2 instead of reg_ctrl
(Abhinav)
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index c983698d1384..a95d5df52653 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -961,10 +961,13 @@ static void dsi_update_dsc_timing(struct msm_dsi_host
*msm_host, bool is_cmd_mod
reg_ctrl = dsi_read(msm_host,
REG_DSI_COMMAND_COMPRESSION_MODE_CTRL);
reg_ctrl2 = dsi_read(msm_host,
REG_DSI_COMMAND_COMPRESSION_MODE_CTRL2);
+ reg_ctrl &= ~0x;
reg_ctrl |= reg;
+
+ reg_ctrl2 &=
~DSI_COMMAND_COMPRESSION_MODE_CTRL2_STREAM0_SLICE_WIDTH__MASK;
reg_ctrl2 |=
DSI_COMMAND_COMPRESSION_MODE_CTRL2_STREAM0_SLICE_WIDTH(bytes_in_slice);
- dsi_write(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL, reg);
+ dsi_write(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL,
reg_ctrl);
dsi_write(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL2,
reg_ctrl2);
} else {
dsi_write(msm_host, REG_DSI_VIDEO_COMPRESSION_MODE_CTRL, reg);
--
2.35.1
Re: [PATCH v3] drm: mxsfb: Implement LCDIF scanout CRC32 support
On 4/29/22 23:23, Marek Vasut wrote: The LCDIF controller as present in i.MX28/i.MX6SX/i.MX8M Mini/Nano has CRC_STAT register, which contains CRC32 of the frame as it was clocked out of the DPI interface of the LCDIF. This is most likely meant as a functional safety feature. Unfortunately, there is zero documentation on how the CRC32 is calculated, there is no documentation of the polynomial, the init value, nor on which data is the checksum applied. By applying brute-force on 8 pixel / 2 line frame, which is the minimum size LCDIF would work with, it turns out the polynomial is CRC32_POLY_LE 0xedb88320 , init value is 0x , the input data are bitrev32() of the entire frame and the resulting CRC has to be also bitrev32()ed. Doing this calculation in kernel for each frame is unrealistic due to the CPU demand, so attach the CRC collected from hardware to a frame instead. The DRM subsystem already has an interface for this purpose and the CRC can be accessed e.g. via debugfs: " $ echo auto > /sys/kernel/debug/dri/1/crtc-0/crc/control $ cat /sys/kernel/debug/dri/1/crtc-0/crc/data 0x408c 0xa4e5cdd8 0x408d 0x72f537b4 " The per-frame CRC can be used by userspace e.g. during automated testing, to verify that whatever buffer was sent to be scanned out was actually scanned out of the LCDIF correctly. If this is OK with everyone, I will apply this soon.
[PULL v3] drm/msm: display pull request for 5.19
Hi Rob, This is a pull request over the patches accumulated, reviewed and tested for the 5.19 merge window. This pull request contains following changes: - DPU: DSC (Display Stream Compression) support - DPU: inline rotation support on SC7280 - DPU: update DP timings to follow vendor recommendations - DP, DPU: add support for wide bus (on newer chipsets) - DP: eDP support - Merge DPU1 and MDP5 MDSS driver, make dpu/mdp device the master component - MDSS: optionally reset the IP block at the bootup to drop bootloader state - Properly register and unregister internal bridges in the DRM framework - Complete DPU IRQ cleanup - DP: conversion to use drm_bridge and drm_bridge_connector - eDP: drop old eDP parts again - Misc small fixes This pull request incorporates both patches from first and second pull requests. It was flattened to ease fixing the Fixes tags issues reported by Stephen Rothwell. The following changes since commit 78f815c1cf8fc5f05dc5cec29eb1895cb53470e9: drm/msm: return the average load over the polling period (2022-04-21 15:05:23 -0700) are available in the Git repository at: https://gitlab.freedesktop.org/lumag/msm.git msm-next-lumag for you to fetch changes up to f1fc2b87de4719cfa8e193e0746cc524dd9f7472: drm/msm: drop old eDP block support (again) (2022-05-02 02:39:35 +0300) Abhinav Kumar (21): drm/msm: remove unused hotplug and edid macros from msm_drv.h drm: allow passing possible_crtcs to drm_writeback_connector_init() drm: introduce drm_writeback_connector_init_with_encoder() API drm/msm/dpu: add writeback blocks to the sm8250 DPU catalog drm/msm/dpu: add reset_intf_cfg operation for dpu_hw_ctl drm/msm/dpu: rename dpu_hw_pipe_cdp_cfg to dpu_hw_cdp_cfg drm/msm/dpu: add dpu_hw_wb abstraction for writeback blocks drm/msm/dpu: add writeback blocks to DPU RM drm/msm/dpu: add changes to support writeback in hw_ctl drm/msm/dpu: add an API to reset the encoder related hw blocks drm/msm/dpu: make changes to dpu_encoder to support virtual encoder drm/msm/dpu: add encoder operations to prepare/cleanup wb job drm/msm/dpu: move _dpu_plane_get_qos_lut to dpu_hw_util file drm/msm/dpu: introduce the dpu_encoder_phys_* for writeback drm/msm/dpu: add the writeback connector layer drm/msm/dpu: initialize dpu encoder and connector for writeback drm/msm/dpu: gracefully handle null fb commits for writeback drm/msm/dpu: add writeback blocks to the display snapshot drm/msm/dpu: add wb_idx to existing DRM prints in dpu_encoder drm/msm/dpu: add wb_idx to DRM traces in dpu_encoder drm/msm/dpu: remove unused refcount for encoder_phys_wb Bjorn Andersson (2): dt-bindings: display: msm: Add optional resets drm/msm/dpu: Issue MDSS reset during initialization Dmitry Baryshkov (32): drm/msm: unify MDSS drivers drm/msm: remove extra indirection for msm_mdss drm/msm: split the main platform driver drm/msm: stop using device's match data pointer drm/msm: allow compile time selection of driver components drm/msm: make mdp5/dpu devices master components drm/msm: properly add and remove internal bridges drm/msm/dpu: remove manual destruction of DRM objects drm/msm: loop over encoders using drm_for_each_encoder() drm/msm: don't store created planes, connectors and encoders drm/msm: remove unused plane_property field from msm_drm_private drm/msm/dpu: don't use merge_3d if DSC merge topology is used drm/msm/dpu: remove extra wrappers around dpu_core_irq drm/msm/dpu: remove always-true argument of dpu_core_irq_read() drm/msm/dpu: allow just single IRQ callback drm/msm/dpu: get rid of dpu_encoder_helper_(un)register_irq drm/msm/dpu: remove struct dpu_encoder_irq drm/msm/dpu: pass irq to dpu_encoder_helper_wait_for_irq() drm/msm/dpu: document INTF_EDP/INTF_DP difference drm/msm/dpu: drop INTF_TYPE_MAX symbol drm/msm/dpu: drop obsolete INTF_EDP comment drm/msm/dpu: drop INTF_EDP from interface type conditions drm/msm/dp: replace dp_connector with drm_bridge_connector drm/msm/dp: remove extra wrappers and public functions drm/msm/dp: drop dp_mode argument from dp_panel_get_modes() drm/msm/dp: simplify dp_connector_get_modes() drm/msm/dp: remove max_pclk_khz field from dp_panel/dp_display drm/msm: select DRM_DP_AUX_BUS for the AUX bus support drm/msm/dsi: fix error checks and return values for DSI xmit functions drm/msm/dsi: use RMW cycles in dsi_update_dsc_timing drm/msm: add missing include to msm_drv.c drm/msm: drop old eDP block support (again) Guo Zhengkui (1): drm/msm: fix returnvar.cocci warning Haowen Bai (1): drm/msm/mdp5: Eliminate useless code Jessica Zhang (1): drm/msm/dpu:
Re: [PATCH v20 8/8] soc: mediatek: remove DDP_DOMPONENT_DITHER from enum
Hi, Matthias:
Matthias Brugger 於 2022年4月22日 週五 下午8:42寫道：
>
>
>
> On 19/04/2022 11:41, jason-jh.lin wrote:
> > After mmsys and drm change DITHER enum to DDP_COMPONENT_DITHER0,
> > mmsys header can remove the useless DDP_COMPONENT_DITHER enum.
> >
> > Signed-off-by: jason-jh.lin
> > Reviewed-by: AngeloGioacchino Del Regno
> >
>
> Acked-by: Matthias Brugger
>
> Chun-Kuang, I think it would make sense to take that through your tree as it
> depends on the previous patches.
>
> I provide you a stable tag so that you can take it:
> v5.18-next-vdso0-stable-tag
After I take this tag, I find one checkpatch warning:
WARNING: DT compatible string "mediatek,mt8195-mmsys" appears
un-documented -- check ./Documentation/devicetree/bindings/
#670: FILE: drivers/soc/mediatek/mtk-mmsys.c:390:
+ .compatible = "mediatek,mt8195-mmsys",
I think this tag lost one binding patch, it's better that this tag has
no this warning.
Regards,
Chun-Kuang.
>
> Regards,
> Matthias
>
> > ---
> > include/linux/soc/mediatek/mtk-mmsys.h | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/include/linux/soc/mediatek/mtk-mmsys.h
> > b/include/linux/soc/mediatek/mtk-mmsys.h
> > index 59117d970daf..fb719fd1281c 100644
> > --- a/include/linux/soc/mediatek/mtk-mmsys.h
> > +++ b/include/linux/soc/mediatek/mtk-mmsys.h
> > @@ -16,8 +16,7 @@ enum mtk_ddp_comp_id {
> > DDP_COMPONENT_CCORR,
> > DDP_COMPONENT_COLOR0,
> > DDP_COMPONENT_COLOR1,
> > - DDP_COMPONENT_DITHER,
> > - DDP_COMPONENT_DITHER0 = DDP_COMPONENT_DITHER,
> > + DDP_COMPONENT_DITHER0,
> > DDP_COMPONENT_DITHER1,
> > DDP_COMPONENT_DP_INTF0,
> > DDP_COMPONENT_DP_INTF1,
Re: [PATCH v2] drm/msm/dsi: use RMW cycles in dsi_update_dsc_timing
On 01/05/2022 23:41, Marijn Suijten wrote:
On 2022-04-30 22:28:42, Dmitry Baryshkov wrote:
On 30/04/2022 21:58, Marijn Suijten wrote:
On 2022-04-30 20:55:33, Dmitry Baryshkov wrote:
The downstream uses read-modify-write for updating command mode
compression registers. Let's follow this approach. This also fixes the
following warning:
drivers/gpu/drm/msm/dsi/dsi_host.c:918:23: warning: variable 'reg_ctrl' set but
not used [-Wunused-but-set-variable]
Reported-by: kernel test robot
Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration")
Signed-off-by: Dmitry Baryshkov
I pointed this out in review multiple times, so you'll obviously get my:
I think I might have also pointed this out once (and then forgot to
check that the issue was fixed by Vinod).
Reviewed-by: Marijn Suijten
(But are you sure there's nothing else to clear in the 1st CTRL
register, only the lowest 16 bits? That should mean `reg` never
contains anything in 0x)
Judging from the downstream the upper half conains the same fields, but
used for other virtual channel. I didn't research what's the difference
yet. All the dtsi files that I have here at hand use
'qcom,mdss-dsi-virtual-channel-id = <0>;'
As replied to Abhinav I'm simply asking whether we should be strict
and add `reg & 0x` to prevent accidentally writing the top 16 bits,
which are stream 1. It doesn't seem like the current code can hit that
though, with all the macros using masks internally already; but it's
still a little scary since we're assuming the registers for VIDEO are
identical to CMD (as mentioned in the reply to Abhinav: I wonder if it's
possible to declare a a pair of bitfields as a single layout in the XML,
and reuse that across CMD's stream 0/1 and the VIDEO register).
However, this seems to indicate that the DSC patch series has been
approved and merged somehow??
Pending inclusion, yes. If Vinod missed or ignored any other review
points, please excuse Abhinav and me not noticing that.
Vinod replied to most of the comments so I'll double-check if they were
applied in the way requested. Note that I don't always post a full
review up-front if it gets too noisy: I'll simply start out with the
most glaring issues and go in more detail in further revisions to
prevent drowning everyone in comments.
Can you please take a look at the latest revision posted, if there are
any other missing points. Let's decide if there are grave issues or we
can work them through.
Thanks, I'll queue that up this week. One of my thus-far-unaddressed
issues with the patches which can't be addressed in hindsight is the
relatively lackluster commit messages: most happen to be repeating the
title in a slightly different way without any additional clarification,
which doesn't fit the upstream spirit at all.
I understand that the reference manuals can't be quoted nor am I asking
to, but a little more insight in the process and details of each patch
goes a very long way. Explain how certain calculations work or came to
be, link to public sources detailing the protocol, explain design
decisions or document how to use/test the feature and describe possible
limitations.
I usually link contributors to [1], but it's a bit of an odd read at
times.
[1]:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
In any case, given that you've already sent this patch and another three
patches [2] fixing/cleaning up the series tells me it's far from ready.
Most of this should just be handled - or have been handled - in review
and amended?
During the review time we agreed that [2] would come as a separate
change It is an API change that would make using panel-bridge easier,
but isn't otherwise required.
I have been working towards more logical drm_bridge/drm_bridge_connector
chains employing panel-bridge and display-connector where required, [2]
is a part of that effort (as well as few other patches that hit
dri-devel in the last few days).
[2]:
https://lore.kernel.org/linux-arm-msm/20220501151220.3999164-1-dmitry.barysh...@linaro.org/T/#t
I'll look through v14 again this week and let you know.
- Marijn
---
Changes since v1:
- Fix c error and apply mask clear to reg_ctrl2 instead of reg_ctrl
(Abhinav)
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index c983698d1384..a95d5df52653 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -961,10 +961,13 @@ static void dsi_update_dsc_timing(struct msm_dsi_host
*msm_host, bool is_cmd_mod
reg_ctrl = dsi_read(msm_host,
REG_DSI_COMMAND_COMPRESSION_MODE_CTRL);
reg_ctrl2 = dsi_read(msm_host,
REG_DSI_COMMAND_COMPRESSION_MODE_CTRL2);
+ reg_ctrl &= ~0x;
reg_ctrl |= reg;
+
+ reg_ctrl2 &=
Re: [PATCH 25/48] ARM: pxa: zylonite: use gpio lookup instead mfp header
On Tue, Apr 19, 2022 at 6:42 PM Arnd Bergmann wrote: > From: Arnd Bergmann > > The mach/mfp.h header is only used by this one driver > for hardcoded gpio numbers. Change that to use a lookup > table instead. > > Cc: Dmitry Torokhov > Cc: linux-in...@vger.kernel.org > Acked-by: Robert Jarzmik > Acked-by: Linus Walleij > Signed-off-by: Arnd Bergmann Looks good to me! Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH 24/48] ARM: pxa: mainstone-wm97xx: use gpio lookup table
On Tue, Apr 19, 2022 at 6:42 PM Arnd Bergmann wrote: > From: Arnd Bergmann > > This driver hardcodes gpio numbers without a header file. > Use lookup tables instead. > > Cc: Marek Vasut > Acked-by: Dmitry Torokhov > Acked-by: Robert Jarzmik > Reviewed-by: Linus Walleij > Cc: linux-in...@vger.kernel.org > Signed-off-by: Arnd Bergmann Looks good to me! Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH 23/48] ARM: pxa: magician: use platform driver for audio
On Tue, Apr 19, 2022 at 6:41 PM Arnd Bergmann wrote: > From: Arnd Bergmann > > The magician audio driver creates a codec device and gets > data from a board specific header file, both of which is > a bit suspicious. Move these into the board file itself, > using a gpio lookup table. > > Acked-by: Mark Brown > Acked-by: Robert Jarzmik > Cc: alsa-de...@alsa-project.org > Signed-off-by: Arnd Bergmann Looks good to me! Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH 22/48] ARM: pxa: z2: use gpio lookup for audio device
On Tue, Apr 19, 2022 at 6:41 PM Arnd Bergmann wrote: > From: Arnd Bergmann > > The audio device is allocated by the audio driver, and it uses a gpio > number from the mach/z2.h header file. > > Change it to use a gpio lookup table for the device allocated by the > driver to keep the header file local to the machine. > > Acked-by: Mark Brown > Cc: alsa-de...@alsa-project.org > Reviewed-by: Linus Walleij > Acked-by: Robert Jarzmik > Signed-off-by: Arnd Bergmann Looks good to me! Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH 21/48] ARM: pxa: eseries: use gpio lookup for audio
On Tue, Apr 19, 2022 at 6:41 PM Arnd Bergmann wrote: > From: Arnd Bergmann > > The three eseries machines have very similar drivers for audio, all > using the mach/eseries-gpio.h header for finding the gpio numbers. > > Change these to use gpio descriptors to avoid the header file > dependency. > > I convert the _OFF gpio numbers into GPIO_ACTIVE_LOW ones for > consistency here. > > Acked-by: Mark Brown > Acked-by: Robert Jarzmik > Reviewed-by: Linus Walleij > Cc: alsa-de...@alsa-project.org > Signed-off-by: Arnd Bergmann Nice use of active low. Maybe I would simply have dropped the _OFF suffix on these GPIO lines as it can be confusing now that their active level is encoded but no big deal. Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH 20/48] ARM: pxa: spitz: use gpio descriptors for audio
On Tue, Apr 19, 2022 at 6:41 PM Arnd Bergmann wrote: > From: Arnd Bergmann > > The audio driver should not use a hardwired gpio number > from the header. Change it to use a lookup table. > > Acked-by: Mark Brown > Cc: alsa-de...@alsa-project.org > Signed-off-by: Arnd Bergmann Looks good to me! Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH 18/48] ARM: pxa: hx4700: use gpio descriptors for audio
On Tue, Apr 19, 2022 at 6:41 PM Arnd Bergmann wrote:
> From: Arnd Bergmann
>
> The audio driver should not use a hardwired gpio number
> from the header. Change it to use a lookup table.
>
> Cc: Philipp Zabel
> Cc: Paul Parsons
> Acked-by: Mark Brown
> Acked-by: Robert Jarzmik
> Cc: alsa-de...@alsa-project.org
> Signed-off-by: Arnd Bergmann
(...)
> +static struct gpiod_lookup_table hx4700_audio_gpio_table = {
> + .dev_id = "hx4700-audio",
> + .table = {
> + GPIO_LOOKUP("gpio-pxa", GPIO75_HX4700_EARPHONE_nDET,
> + "earphone-ndet", GPIO_ACTIVE_HIGH),
This looks wrong. The n in nDET in the end of the name of the GPIO line
means active low does it not?
What I usually do when I see this is to properly set it to
GPIO_ACTIVE_LOW in the descriptor table, then invert the logic
where it's getting used.
Also rename to earphone-det instead of -ndet
> + GPIO_LOOKUP("gpio-pxa", GPIO92_HX4700_HP_DRIVER,
> + "hp-driver", GPIO_ACTIVE_HIGH),
> + GPIO_LOOKUP("gpio-pxa", GPIO107_HX4700_SPK_nSD,
> + "spk-nsd", GPIO_ACTIVE_HIGH),
Same here. Rename spk-sd
Yours,
Linus Walleij
Re: [PATCH 17/48] ARM: pxa: corgi: use gpio descriptors for audio
On Tue, Apr 19, 2022 at 6:41 PM Arnd Bergmann wrote: > From: Arnd Bergmann > > The audio driver should not use a hardwired gpio number > from the header. Change it to use a lookup table. > > Acked-by: Mark Brown > Cc: alsa-de...@alsa-project.org > Acked-by: Robert Jarzmik > Signed-off-by: Arnd Bergmann Looks good to me! Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH 15/48] ARM: pxa: tosa: use gpio descriptor for audio
On Tue, Apr 19, 2022 at 6:40 PM Arnd Bergmann wrote: > From: Arnd Bergmann > > The audio driver should not use a hardwired gpio number > from the header. Change it to use a lookup table. > > Acked-by: Mark Brown > Acked-by: Robert Jarzmik > Cc: alsa-de...@alsa-project.org > Signed-off-by: Arnd Bergmann Looks good to me! Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH v2] drm/msm/dsi: use RMW cycles in dsi_update_dsc_timing
On 2022-04-30 22:28:42, Dmitry Baryshkov wrote:
> On 30/04/2022 21:58, Marijn Suijten wrote:
> > On 2022-04-30 20:55:33, Dmitry Baryshkov wrote:
> >> The downstream uses read-modify-write for updating command mode
> >> compression registers. Let's follow this approach. This also fixes the
> >> following warning:
> >>
> >> drivers/gpu/drm/msm/dsi/dsi_host.c:918:23: warning: variable 'reg_ctrl'
> >> set but not used [-Wunused-but-set-variable]
> >>
> >> Reported-by: kernel test robot
> >> Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration")
> >> Signed-off-by: Dmitry Baryshkov
> >
> > I pointed this out in review multiple times, so you'll obviously get my:
>
> I think I might have also pointed this out once (and then forgot to
> check that the issue was fixed by Vinod).
>
> >
> > Reviewed-by: Marijn Suijten
> >
> > (But are you sure there's nothing else to clear in the 1st CTRL
> > register, only the lowest 16 bits? That should mean `reg` never
> > contains anything in 0x)
>
> Judging from the downstream the upper half conains the same fields, but
> used for other virtual channel. I didn't research what's the difference
> yet. All the dtsi files that I have here at hand use
> 'qcom,mdss-dsi-virtual-channel-id = <0>;'
As replied to Abhinav I'm simply asking whether we should be strict
and add `reg & 0x` to prevent accidentally writing the top 16 bits,
which are stream 1. It doesn't seem like the current code can hit that
though, with all the macros using masks internally already; but it's
still a little scary since we're assuming the registers for VIDEO are
identical to CMD (as mentioned in the reply to Abhinav: I wonder if it's
possible to declare a a pair of bitfields as a single layout in the XML,
and reuse that across CMD's stream 0/1 and the VIDEO register).
> > However, this seems to indicate that the DSC patch series has been
> > approved and merged somehow??
>
> Pending inclusion, yes. If Vinod missed or ignored any other review
> points, please excuse Abhinav and me not noticing that.
Vinod replied to most of the comments so I'll double-check if they were
applied in the way requested. Note that I don't always post a full
review up-front if it gets too noisy: I'll simply start out with the
most glaring issues and go in more detail in further revisions to
prevent drowning everyone in comments.
> Can you please take a look at the latest revision posted, if there are
> any other missing points. Let's decide if there are grave issues or we
> can work them through.
Thanks, I'll queue that up this week. One of my thus-far-unaddressed
issues with the patches which can't be addressed in hindsight is the
relatively lackluster commit messages: most happen to be repeating the
title in a slightly different way without any additional clarification,
which doesn't fit the upstream spirit at all.
I understand that the reference manuals can't be quoted nor am I asking
to, but a little more insight in the process and details of each patch
goes a very long way. Explain how certain calculations work or came to
be, link to public sources detailing the protocol, explain design
decisions or document how to use/test the feature and describe possible
limitations.
I usually link contributors to [1], but it's a bit of an odd read at
times.
[1]:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
In any case, given that you've already sent this patch and another three
patches [2] fixing/cleaning up the series tells me it's far from ready.
Most of this should just be handled - or have been handled - in review
and amended?
[2]:
https://lore.kernel.org/linux-arm-msm/20220501151220.3999164-1-dmitry.barysh...@linaro.org/T/#t
I'll look through v14 again this week and let you know.
- Marijn
> >
> >> ---
> >>
> >> Changes since v1:
> >> - Fix c error and apply mask clear to reg_ctrl2 instead of reg_ctrl
> >> (Abhinav)
> >>
> >> ---
> >> drivers/gpu/drm/msm/dsi/dsi_host.c | 5 -
> >> 1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> >> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> >> index c983698d1384..a95d5df52653 100644
> >> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> >> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> >> @@ -961,10 +961,13 @@ static void dsi_update_dsc_timing(struct
> >> msm_dsi_host *msm_host, bool is_cmd_mod
> >>reg_ctrl = dsi_read(msm_host,
> >> REG_DSI_COMMAND_COMPRESSION_MODE_CTRL);
> >>reg_ctrl2 = dsi_read(msm_host,
> >> REG_DSI_COMMAND_COMPRESSION_MODE_CTRL2);
> >>
> >> + reg_ctrl &= ~0x;
> >>reg_ctrl |= reg;
> >> +
> >> + reg_ctrl2 &=
> >> ~DSI_COMMAND_COMPRESSION_MODE_CTRL2_STREAM0_SLICE_WIDTH__MASK;
> >>reg_ctrl2 |=
> >> DSI_COMMAND_COMPRESSION_MODE_CTRL2_STREAM0_SLICE_WIDTH(bytes_in_slice);
> >>
> >> - dsi_write(msm_host,
Re: [Freedreno] [PATCH v2] drm/msm/dsi: use RMW cycles in dsi_update_dsc_timing
On 2022-04-30 12:25:57, Abhinav Kumar wrote:
>
>
> On 4/30/2022 11:58 AM, Marijn Suijten wrote:
> > On 2022-04-30 20:55:33, Dmitry Baryshkov wrote:
> >> The downstream uses read-modify-write for updating command mode
> >> compression registers. Let's follow this approach. This also fixes the
> >> following warning:
> >>
> >> drivers/gpu/drm/msm/dsi/dsi_host.c:918:23: warning: variable 'reg_ctrl'
> >> set but not used [-Wunused-but-set-variable]
> >>
> >> Reported-by: kernel test robot
> >> Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration")
> >> Signed-off-by: Dmitry Baryshkov
> >
> > I pointed this out in review multiple times, so you'll obviously get my:
> >
> > Reviewed-by: Marijn Suijten
> >
> > (But are you sure there's nothing else to clear in the 1st CTRL
> > register, only the lowest 16 bits? That should mean `reg` never
> > contains anything in 0x)
>
> The top 16 bits contain information for stream 1.
>
> Stream 1 is unused. And whatever is the reset value we should retain that.
>
> So this patch is correct.
I was not debating the correctness of this patch, quite the contrary:
it's already much better than it was before.
I'm simply asking whether we should prevent `reg` (not `reg_ctrl`!)
from having anything in the top 16 bits, which would overwrite the reset
value for stream 1 which you correctly say should be retained as it is.
It seems unlikely that this happens, but better be safe than sorry?
Looking at the DSI register definition for DSC [1] again, I wonder if
there's support for defining a common bitfield layout and reusing it
thrice: the two channels for CMD mode and a single use for VIDEO. Don't
think that'd make the kernel code better though if even possible at all.
[1]: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/14967/diffs
- Marijn
> >
> > However, this seems to indicate that the DSC patch series has been
> > approved and merged somehow??
> >
> >> ---
> >>
> >> Changes since v1:
> >> - Fix c error and apply mask clear to reg_ctrl2 instead of reg_ctrl
> >> (Abhinav)
> >>
> >> ---
> >> drivers/gpu/drm/msm/dsi/dsi_host.c | 5 -
> >> 1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> >> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> >> index c983698d1384..a95d5df52653 100644
> >> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> >> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> >> @@ -961,10 +961,13 @@ static void dsi_update_dsc_timing(struct
> >> msm_dsi_host *msm_host, bool is_cmd_mod
> >>reg_ctrl = dsi_read(msm_host,
> >> REG_DSI_COMMAND_COMPRESSION_MODE_CTRL);
> >>reg_ctrl2 = dsi_read(msm_host,
> >> REG_DSI_COMMAND_COMPRESSION_MODE_CTRL2);
> >>
> >> + reg_ctrl &= ~0x;
> >>reg_ctrl |= reg;
> >> +
> >> + reg_ctrl2 &=
> >> ~DSI_COMMAND_COMPRESSION_MODE_CTRL2_STREAM0_SLICE_WIDTH__MASK;
> >>reg_ctrl2 |=
> >> DSI_COMMAND_COMPRESSION_MODE_CTRL2_STREAM0_SLICE_WIDTH(bytes_in_slice);
> >>
> >> - dsi_write(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL, reg);
> >> + dsi_write(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL,
> >> reg_ctrl);
> >>dsi_write(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL2,
> >> reg_ctrl2);
> >>} else {
> >>dsi_write(msm_host, REG_DSI_VIDEO_COMPRESSION_MODE_CTRL, reg);
> >> --
> >> 2.35.1
> >>
[PATCH] drm/msm/dsi: pll_7nm: remove unsupported dividers for DSI pixel clock
Remove dividers that are not recommended for DSI DPHY mode when setting
up the clock tree for the DSI pixel clock.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
index 6e506feb111f..66ed1919a1db 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
@@ -586,7 +586,7 @@ static int dsi_7nm_set_usecase(struct msm_dsi_phy *phy)
static int pll_7nm_register(struct dsi_pll_7nm *pll_7nm, struct clk_hw
**provided_clocks)
{
char clk_name[32], parent[32], vco_name[32];
- char parent2[32], parent3[32], parent4[32];
+ char parent2[32];
struct clk_init_data vco_init = {
.parent_data = &(const struct clk_parent_data) {
.fw_name = "ref",
@@ -687,15 +687,13 @@ static int pll_7nm_register(struct dsi_pll_7nm *pll_7nm,
struct clk_hw **provide
snprintf(clk_name, 32, "dsi%d_pclk_mux", pll_7nm->phy->id);
snprintf(parent, 32, "dsi%d_pll_bit_clk", pll_7nm->phy->id);
snprintf(parent2, 32, "dsi%d_pll_by_2_bit_clk",
pll_7nm->phy->id);
- snprintf(parent3, 32, "dsi%d_pll_out_div_clk",
pll_7nm->phy->id);
- snprintf(parent4, 32, "dsi%d_pll_post_out_div_clk",
pll_7nm->phy->id);
hw = devm_clk_hw_register_mux(dev, clk_name,
((const char *[]){
- parent, parent2, parent3, parent4
- }), 4, 0, pll_7nm->phy->base +
+ parent, parent2,
+ }), 2, 0, pll_7nm->phy->base +
REG_DSI_7nm_PHY_CMN_CLK_CFG1,
- 0, 2, 0, NULL);
+ 0, 1, 0, NULL);
if (IS_ERR(hw)) {
ret = PTR_ERR(hw);
goto fail;
--
2.35.1
[PATCH 3/3] drm/panel: drop DSC pps pointer
Complete the move of DSC data pointer from struct drm_panel to struct
mipi_dsi_device.
Signed-off-by: Dmitry Baryshkov
---
include/drm/drm_panel.h | 7 ---
1 file changed, 7 deletions(-)
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index d279ee455f01..1ba2d424a53f 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -179,13 +179,6 @@ struct drm_panel {
* Panel entry in registry.
*/
struct list_head list;
-
- /**
-* @dsc:
-*
-* Panel DSC pps payload to be sent
-*/
- struct drm_dsc_config *dsc;
};
void drm_panel_init(struct drm_panel *panel, struct device *dev,
--
2.35.1
[PATCH 2/3] drm/msm/dsi: fetch DSC pps payload from struct mipi_dsi_device
Now that struct mipi_dsi_device provides DSC data, fetch it from the
mentioned struct rather than from the struct drm_panel itself. This
would allow supporting MIPI DSI bridges handling DSC on their input
side.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 25 +++--
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index a95d5df52653..173df1fd3692 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -1690,6 +1690,17 @@ static int dsi_host_attach(struct mipi_dsi_host *host,
msm_host->lanes = dsi->lanes;
msm_host->format = dsi->format;
msm_host->mode_flags = dsi->mode_flags;
+ if (dsi->dsc) {
+ struct msm_display_dsc_config *dsc = msm_host->dsc;
+
+ if (!dsc) {
+ dsc = devm_kzalloc(_host->pdev->dev, sizeof(*dsc),
GFP_KERNEL);
+ if (!dsc)
+ return -ENOMEM;
+ dsc->drm = dsi->dsc;
+ msm_host->dsc = dsc;
+ }
+ }
/* Some gpios defined in panel DT need to be controlled by host */
ret = dsi_host_init_panel_gpios(msm_host, >dev);
@@ -2164,23 +2175,9 @@ int msm_dsi_host_modeset_init(struct mipi_dsi_host *host,
{
struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd;
- struct drm_panel *panel;
int ret;
msm_host->dev = dev;
- panel = msm_dsi_host_get_panel(_host->base);
-
- if (!IS_ERR(panel) && panel->dsc) {
- struct msm_display_dsc_config *dsc = msm_host->dsc;
-
- if (!dsc) {
- dsc = devm_kzalloc(_host->pdev->dev, sizeof(*dsc),
GFP_KERNEL);
- if (!dsc)
- return -ENOMEM;
- dsc->drm = panel->dsc;
- msm_host->dsc = dsc;
- }
- }
ret = cfg_hnd->ops->tx_buf_alloc(msm_host, SZ_4K);
if (ret) {
--
2.35.1
[PATCH 1/3] drm/mipi-dsi: pass DSC data through the struct mipi_dsi_device
The commit 0f40ba48de3b ("drm/msm/dsi: Pass DSC params to drm_panel")
added a pointer to the DSC data to the struct drm_panel. However DSC
support is not limited to the DSI panels. MIPI DSI bridges can also
consume DSC command streams. Thus add struct drm_dsc_config pointer to
the struct mipi_dsi_device.
Signed-off-by: Dmitry Baryshkov
---
include/drm/drm_mipi_dsi.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 147e51b6d241..8b1c9be9b2a7 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -177,6 +177,7 @@ struct mipi_dsi_device_info {
* @lp_rate: maximum lane frequency for low power mode in hertz, this should
* be set to the real limits of the hardware, zero is only accepted for
* legacy drivers
+ * @dsc: panel/bridge DSC pps payload to be sent
*/
struct mipi_dsi_device {
struct mipi_dsi_host *host;
@@ -189,6 +190,7 @@ struct mipi_dsi_device {
unsigned long mode_flags;
unsigned long hs_rate;
unsigned long lp_rate;
+ struct drm_dsc_config *dsc;
};
#define MIPI_DSI_MODULE_PREFIX "mipi-dsi:"
--
2.35.1
[PATCH 0/3] drm: move dsc data pointer from drm_panel to mipi_dsi_device
To properly support DSC the sink driver (panel) has to pass DSC pps data
to the source (DSI host). The commit 0f40ba48de3b ("drm/msm/dsi: Pass
DSC params to drm_panel") added a pointer to the DSC data to the struct
drm_panel. However this is not the ideal solution.
First, this leaves DSC-supporting DSI sink bridges (like ANX7625 which
support DSC decoding on the MIPI DSI inputs).
Second, this does not play well with the panel_bridge. Drivers depending
solely on the bridge chains will still have to lookup panel and fetch
data from it.
Last, but not least, the DSC data is not relevant for the wide variety
of panels including DPI and LVDS panels.
To solve all these problems, move struct drm_dsc_config pointer from
struct drm_panel to struct mipi_host_device. This way MIPI DSI host
driver receives DSC data during attach callback without additional
lookups.
Dependencies: this depends on the MSM DRM DSC patchset [1] being pulled
in through the MSM DRM tree.
[1] https://patchwork.freedesktop.org/series/102262/
Dmitry Baryshkov (3):
drm/mipi-dsi: pass DSC data through the struct mipi_dsi_device
drm/msm/dsi: fetch DSC pps payload from struct mipi_dsi_device
drm/panel: drop DSC pps pointer
drivers/gpu/drm/msm/dsi/dsi_host.c | 25 +++--
include/drm/drm_mipi_dsi.h | 2 ++
include/drm/drm_panel.h| 7 ---
3 files changed, 13 insertions(+), 21 deletions(-)
--
2.35.1
RE: [PATCH v2 1/2] module: update dependencies at try_module_get()
From: Mauro Carvalho Chehab > Sent: 30 April 2022 14:38 > > Em Sat, 30 Apr 2022 14:04:59 +0200 > Greg KH escreveu: > > > On Sat, Apr 30, 2022 at 11:30:58AM +0100, Mauro Carvalho Chehab wrote: > > > Did you run checkpatch on this? Please do :) > > > > > + > > > + if (mod == this) > > > + return 0; > > > > How can this happen? > > When people mistakenly call try_module_get(THIS_MODULE)? > > Yes. There are lots of place where this is happening: > > $ git grep try_module_get\(THIS_MODULE|wc -l > 82 > > > We should > > throw up a big warning when that happens anyway as that's always wrong. > > > > But that's a different issue from this change, sorry for the noise. > > It sounds very weird to use try_module_get(THIS_MODULE). > > We could add a WARN_ON() there - or something similar - but I would do it > on a separate patch. You could add a compile-time check. But a run-time one seems unnecessary. Clearly try_module_get(THIS_MODULE) usually succeeds. I think I can invent a case where it can fail: The module count must be zero, and a module unload in progress. The thread doing the unload is blocked somewhere. Another thread makes a callback into the module for some request that (for instance) would need to create a kernel thread. It tries to get a reference for the thread. So try_module_get(THIS_MODULE) is the right call - and will fail here. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
[PATCH 2/3] drm/msm/mdp5: move iommu_domain_alloc() call close to its usage
Move iommu_domain_alloc() in front of adress space/IOMMU initialization.
This allows us to drop final bits of struct mdp5_cfg_platform which
remained from the pre-DT days.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c | 16
drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h | 6 --
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 6 --
3 files changed, 4 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
index 1bf9ff5dbabc..714effb967ff 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
@@ -1248,8 +1248,6 @@ static const struct mdp5_cfg_handler cfg_handlers_v3[] = {
{ .revision = 3, .config = { .hw = _config } },
};
-static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev);
-
const struct mdp5_cfg_hw *mdp5_cfg_get_hw_config(struct mdp5_cfg_handler
*cfg_handler)
{
return cfg_handler->config.hw;
@@ -1274,10 +1272,8 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms
*mdp5_kms,
uint32_t major, uint32_t minor)
{
struct drm_device *dev = mdp5_kms->dev;
- struct platform_device *pdev = to_platform_device(dev->dev);
struct mdp5_cfg_handler *cfg_handler;
const struct mdp5_cfg_handler *cfg_handlers;
- struct mdp5_cfg_platform *pconfig;
int i, ret = 0, num_handlers;
cfg_handler = kzalloc(sizeof(*cfg_handler), GFP_KERNEL);
@@ -1320,9 +1316,6 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms
*mdp5_kms,
cfg_handler->revision = minor;
cfg_handler->config.hw = mdp5_cfg;
- pconfig = mdp5_get_config(pdev);
- memcpy(_handler->config.platform, pconfig, sizeof(*pconfig));
-
DBG("MDP5: %s hw config selected", mdp5_cfg->name);
return cfg_handler;
@@ -1333,12 +1326,3 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms
*mdp5_kms,
return ERR_PTR(ret);
}
-
-static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev)
-{
- static struct mdp5_cfg_platform config = {};
-
- config.iommu = iommu_domain_alloc(_bus_type);
-
- return
-}
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
index 6b03d7899309..c2502cc33864 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
@@ -104,14 +104,8 @@ struct mdp5_cfg_hw {
uint32_t max_clk;
};
-/* platform config data (ie. from DT, or pdata) */
-struct mdp5_cfg_platform {
- struct iommu_domain *iommu;
-};
-
struct mdp5_cfg {
const struct mdp5_cfg_hw *hw;
- struct mdp5_cfg_platform platform;
};
struct mdp5_kms;
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 9b7bbc3adb97..1c67c2c828cd 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -558,6 +558,7 @@ static int mdp5_kms_init(struct drm_device *dev)
struct msm_gem_address_space *aspace;
int irq, i, ret;
struct device *iommu_dev;
+ struct iommu_domain *iommu;
ret = mdp5_init(to_platform_device(dev->dev), dev);
@@ -601,14 +602,15 @@ static int mdp5_kms_init(struct drm_device *dev)
}
mdelay(16);
- if (config->platform.iommu) {
+ iommu = iommu_domain_alloc(_bus_type);
+ if (iommu) {
struct msm_mmu *mmu;
iommu_dev = >dev;
if (!dev_iommu_fwspec_get(iommu_dev))
iommu_dev = iommu_dev->parent;
- mmu = msm_iommu_new(iommu_dev, config->platform.iommu);
+ mmu = msm_iommu_new(iommu_dev, iommu);
aspace = msm_gem_address_space_create(mmu, "mdp5",
0x1000, 0x1 - 0x1000);
--
2.35.1
[PATCH 1/3] drm/msm/dpu: check both DPU and MDSS devices for the IOMMU
Follow the lead of MDP5 driver and check both DPU and MDSS devices for
the IOMMU specifiers.
Historically DPU devices had IOMMU specified in the MDSS device tree
node, but as some of MDP5 devices are being converted to the supported
by the DPU driver, the driver should adapt and check both devices.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 143d6643be53..5ccda0766f6c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1004,14 +1004,22 @@ static int _dpu_kms_mmu_init(struct dpu_kms *dpu_kms)
struct msm_mmu *mmu;
struct device *dpu_dev = dpu_kms->dev->dev;
struct device *mdss_dev = dpu_dev->parent;
+ struct device *iommu_dev;
domain = iommu_domain_alloc(_bus_type);
if (!domain)
return 0;
- /* IOMMUs are a part of MDSS device tree binding, not the
-* MDP/DPU device. */
- mmu = msm_iommu_new(mdss_dev, domain);
+ /*
+* IOMMUs can be a part of MDSS device tree binding, or the
+* MDP/DPU device.
+*/
+ if (dev_iommu_fwspec_get(dpu_dev))
+ iommu_dev = dpu_dev;
+ else
+ iommu_dev = mdss_dev;
+
+ mmu = msm_iommu_new(iommu_dev, domain);
if (IS_ERR(mmu)) {
iommu_domain_free(domain);
return PTR_ERR(mmu);
--
2.35.1
[PATCH 3/3] drm/msm: Stop using iommu_present()
Even if some IOMMU has registered itself on the platform "bus", that
doesn't necessarily mean it provides translation for the device we
care about. Replace iommu_present() with a more appropriate check.
On Qualcomm platforms the IOMMU can be specified either for the MDP/DPU
device or for its parent MDSS device depending on the actual platform.
Check both of them, since that is how both DPU and MDP5 drivers work.
Co-developed-by: Robin Murphy
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/msm_drv.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 4a3dda23e3e0..a37a3bbc04d9 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -266,8 +266,14 @@ bool msm_use_mmu(struct drm_device *dev)
{
struct msm_drm_private *priv = dev->dev_private;
- /* a2xx comes with its own MMU */
- return priv->is_a2xx || iommu_present(_bus_type);
+ /*
+* a2xx comes with its own MMU
+* On other platforms IOMMU can be declared specified either for the
+* MDP/DPU device or for its parent, MDSS device.
+*/
+ return priv->is_a2xx ||
+ device_iommu_mapped(dev->dev) ||
+ device_iommu_mapped(dev->dev->parent);
}
static int msm_init_vram(struct drm_device *dev)
--
2.35.1
[PATCH 0/3] drm/msm: fixes for KMS iommu handling
This series started from the applied and then reverted [2] patch by Robin Murphy [1]. After the MDSS rework [3] has landed it is now possible to reapply the extended version of the original patch. While we are at it, also rework the IOMMU init code for DPU and MDP5 drivers. For MDP5 this moves iommu_domain_alloc() call and removes struct mdp5_cfg_platform remains. For DPU this allows specifying the iommus = <...> either in the DPU device (like all DPU devices do) or in the MDSS device (like MDP5 devices do). [1] https://patchwork.freedesktop.org/patch/480707/ [2] https://patchwork.freedesktop.org/patch/482453/ [3] https://patchwork.freedesktop.org/series/98525/ Dmitry Baryshkov (3): drm/msm/dpu: check both DPU and MDSS devices for the IOMMU drm/msm/mdp5: move iommu_domain_alloc() call close to its usage drm/msm: Stop using iommu_present() drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 14 +++--- drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c | 16 drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h | 6 -- drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 6 -- drivers/gpu/drm/msm/msm_drv.c| 10 -- 5 files changed, 23 insertions(+), 29 deletions(-) -- 2.35.1
Re: [PATCHv5] drm/amdgpu: vi: disable ASPM on Intel Alder Lake based systems
Dear Richard,
Am 29.04.22 um 18:06 schrieb Richard Gong:
Active State Power Management (ASPM) feature is enabled since kernel 5.14.
There are some AMD Volcanic Islands (VI) GFX cards, such as the WX3200 and
RX640, that do not work with ASPM-enabled Intel Alder Lake based systems.
Using these GFX cards as video/display output, Intel Alder Lake based
systems will freeze after suspend/resume.
As replied in v4 just now, “freeze” is misleading if you can still run
`dmesg` after resume.
Kind regards,
Paul
The issue was originally reported on one system (Dell Precision 3660 with
BIOS version 0.14.81), but was later confirmed to affect at least 4
pre-production Alder Lake based systems.
Add an extra check to disable ASPM on Intel Alder Lake based systems with
the problematic AMD Volcanic Islands GFX cards.
Fixes: 0064b0ce85bb ("drm/amd/pm: enable ASPM by default")
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1885
Reported-by: kernel test robot
Signed-off-by: Richard Gong
---
v5: added vi to commit header and updated commit message
rolled back guard with the preprocessor as did in v2 to correct build
error on non-x86 systems
v4: s/CONFIG_X86_64/CONFIG_X86
enhanced check logic
v3: s/intel_core_aspm_chk/aspm_support_quirk_check
correct build error with W=1 option
v2: correct commit description
move the check from chip family to problematic platform
---
drivers/gpu/drm/amd/amdgpu/vi.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c b/drivers/gpu/drm/amd/amdgpu/vi.c
index 039b90cdc3bc..45f0188c4273 100644
--- a/drivers/gpu/drm/amd/amdgpu/vi.c
+++ b/drivers/gpu/drm/amd/amdgpu/vi.c
@@ -81,6 +81,10 @@
#include "mxgpu_vi.h"
#include "amdgpu_dm.h"
+#if IS_ENABLED(CONFIG_X86)
+#include
+#endif
+
#define ixPCIE_LC_L1_PM_SUBSTATE 0x100100C6
#define PCIE_LC_L1_PM_SUBSTATE__LC_L1_SUBSTATES_OVERRIDE_EN_MASK
0x0001L
#define PCIE_LC_L1_PM_SUBSTATE__LC_PCI_PM_L1_2_OVERRIDE_MASK 0x0002L
@@ -1134,13 +1138,24 @@ static void vi_enable_aspm(struct amdgpu_device *adev)
WREG32_PCIE(ixPCIE_LC_CNTL, data);
}
+static bool aspm_support_quirk_check(void)
+{
+#if IS_ENABLED(CONFIG_X86)
+ struct cpuinfo_x86 *c = _data(0);
+
+ return !(c->x86 == 6 && c->x86_model == INTEL_FAM6_ALDERLAKE);
+#else
+ return true;
+#endif
+}
+
static void vi_program_aspm(struct amdgpu_device *adev)
{
u32 data, data1, orig;
bool bL1SS = false;
bool bClkReqSupport = true;
- if (!amdgpu_device_should_use_aspm(adev))
+ if (!amdgpu_device_should_use_aspm(adev) || !aspm_support_quirk_check())
return;
if (adev->flags & AMD_IS_APU ||
Re: [PATCHv4] drm/amdgpu: disable ASPM on Intel Alder Lake based systems
Dear Richard, Sorry for the late reply. Am 26.04.22 um 15:53 schrieb Gong, Richard: On 4/21/2022 12:35 AM, Paul Menzel wrote: Am 21.04.22 um 03:12 schrieb Gong, Richard: On 4/20/2022 3:29 PM, Paul Menzel wrote: Am 19.04.22 um 23:46 schrieb Gong, Richard: On 4/14/2022 2:52 AM, Paul Menzel wrote: [Cc: -kernel test robot ] […] Am 13.04.22 um 15:00 schrieb Alex Deucher: On Wed, Apr 13, 2022 at 3:43 AM Paul Menzel wrote: Thank you for sending out v4. Am 12.04.22 um 23:50 schrieb Richard Gong: […] I am still not clear, what “hang during suspend/resume” means. I guess suspending works fine? During resume (S3 or S0ix?), where does it hang? The system is functional, but there are only display problems? System freeze after suspend/resume. But you see certain messages still? At what point does it freeze exactly? In the bug report you posted Linux messages. No, the system freeze then users have to recycle power to recover. Then I misread the issue? Did you capture the messages over serial log then? I think so. We captured dmesg log. Then the (whole) system did *not* freeze, if you could still log in (maybe over network) and execute `dmesg`. Please also paste the amdgpu(?) error logs in the commit message. As mentioned early we need support from Intel on how to get ASPM working for VI generation on Intel Alder Lake, but we don't know where things currently stand. Who is working on this, and knows? Kind regards, Paul
