Re: [PATCH] RFC: drm/drm_plane: Expose the plane capability and interoperability
On Mon, Jul 29, 2024 at 04:59:14AM GMT, Murthy, Arun R wrote: > Gentle Reminder! > Any comments? First of all, the format is underdocumented. Second, there is a usual requirement for new uAPI: please provide a pointer to IGT patch and to the userspace utilising the property. > > Thanks and Regards, > Arun R Murthy > > > > -Original Message- > > From: Murthy, Arun R > > Sent: Tuesday, July 9, 2024 1:17 PM > > To: dri-de...@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > > Cc: Murthy, Arun R > > Subject: [PATCH] RFC: drm/drm_plane: Expose the plane capability and > > interoperability > > > > Each plane has its own capability or interoperability based on the harware > > restrictions. If this is exposed to the user, then user can read it once on > > boot > > and store this. Later can be used as a lookup table to check a corresponding > > capability is supported by plane then only go ahead with the framebuffer > > creation/calling atomic_ioctl. > > > > For Ex: There are few restiction as to async flip doesnt support all the > > formats/modifiers. Similar restrictions on scaling. With the availabililty > > of this > > info to user, failures in atomic_check can be avoided as these are more the > > hardware capabilities. > > > > There are two options on how this can be acheived. > > Option 1: > > > > Add a new element to struct drm_mode_get_plane that holds the addr to the > > array of a new struct. This new struct consists of the formats supported > > and the > > corresponding plane capabilities. > > > > Option 2: > > > > These can be exposed to user as a plane property so that the user can get to > > know the limitation ahead and avoid failures in atomic_check. > > > > Usually atomic_get_property is controlled over the state struct for the > > parameters/elements that can change. But here these capabilities or the > > interoperabilities are rather hardware restrictions and wont change over > > flips. > > Hence having as a plane_property may not make much sense. > > On the other hand, Option 1 include changes in the uapi struct > > drm_mode_get_plane. Shouldnt have impact on backward compatibility, but if > > userspace has some implementation so as to check the size of the struct, > > then it > > might a challenge. > > > > Signed-off-by: Arun R Murthy > > --- > > drivers/gpu/drm/drm_atomic_uapi.c | 3 +++ > > include/drm/drm_plane.h | 8 > > include/uapi/drm/drm_mode.h | 20 > > 3 files changed, 31 insertions(+) > > > > =Option 2 > > > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c > > b/drivers/gpu/drm/drm_atomic_uapi.c > > index 22bbb2d83e30..b46177d5fc8c 100644 > > --- a/drivers/gpu/drm/drm_atomic_uapi.c > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > > @@ -631,6 +631,9 @@ drm_atomic_plane_get_property(struct drm_plane > > *plane, > > *val = state->hotspot_x; > > } else if (property == plane->hotspot_y_property) { > > *val = state->hotspot_y; > > + } else if (property == config->prop_plane_caps) { > > + *val = (state->plane_caps) ? > > + state->plane_caps->base.id : 0; > > } else { > > drm_dbg_atomic(dev, > >"[PLANE:%d:%s] unknown property > > [PROP:%d:%s]\n", diff --git a/include/drm/drm_plane.h > > b/include/drm/drm_plane.h index dd718c62ac31..dfe931677d0a 100644 > > --- a/include/drm/drm_plane.h > > +++ b/include/drm/drm_plane.h > > @@ -260,6 +260,14 @@ struct drm_plane_state { > > * flow. > > */ > > bool color_mgmt_changed : 1; > > + > > + /** > > +* @plane_caps: > > +* > > +* Blob representing plane capcabilites and interoperability. > > +* This element is a pointer to the array of struct drm_format_blob. > > +*/ > > + struct drm_property_blob *plane_caps; > > }; > > > > static inline struct drm_rect > > > > =Option 1 > > > > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > > index d390011b89b4..0b5c1b65ef63 100644 > > --- a/include/uapi/drm/drm_mode.h > > +++ b/include/uapi/drm/drm_mode.h > > @@ -312,6 +312,20 @@ struct drm_mode_set_plane { > > __u32 src_w; > > }; > > > > +#define DRM_FORMAT_PLANE_CAP_LINEAR_TILE BIT(0) > > +#define DRM_FORMAT_PLANE_CAP_X_TILEBIT(1) > > +#define DRM_FORMAT_PLANE_CAP_Y_TILEBIT(2) > > +#define DRM_FORMAT_PLANE_CAP_Yf_TILE BIT(3) > > +#define DRM_FORMAT_PLANE_CAP_ASYNC_FLIPBIT(4) > > +#define DRM_FORMAT_PLANE_CAP_FBC BIT(5) > > +#define DRM_FORMAT_PLANE_CAP_RCBIT(6) > > + > > +struct drm_format_blob { > > + __u64 modifier; > > + __u32 plane_caps; > > + > > +}; > > + > > /** > > * struct drm_mode_get_plane - Get plane metadata. > > * > > @@ -355,6 +369,12 @@ struct drm_mode_get_plane { > > * supported by the plane. These
Re: DisplayPort: handling of HPD events / link training
On Tue, Jul 16, 2024 at 06:48:12PM GMT, Thomas Zimmermann wrote: > Hi > > Am 16.07.24 um 18:35 schrieb Dmitry Baryshkov: > > On Tue, 16 Jul 2024 at 18:58, Thomas Zimmermann wrote: > > > Hi > > > > > > Am 27.02.24 um 23:40 schrieb Dmitry Baryshkov: > > > > Hello, > > > > > > > > We are currently looking at checking and/or possibly redesigning the > > > > way the MSM DRM driver handles the HPD events and link training. > > > > > > > > After a quick glance at the drivers implementing DP support, I noticed > > > > following main approaches: > > > > - Perform link training at the atomic_enable time, don't report > > > > failures (mtk, analogix, zynqmp, tegra, nouveau) > > > > - Perform link training at the atomic_enable time, report errors using > > > > link_status property (i915, mhdp8546) > > > > - Perform link training on the plug event (msm, it8605). > > > > - Perform link training from the DPMS handler, also calling it from > > > > the enable callback (AMDGPU, radeon). > > > > > > > > It looks like the majority wins and we should move HPD to > > > > atomic_enable time. Is that assumption correct? > > > Did you ever receive an answer to this question? I currently investigate > > > ast's DP code, which does link training as part of detecting the > > > connector state (in detect_ctx). But most other drivers do this in > > > atomic_enable. I wonder if ast should follow. > > Short answer: yes, the only proper place to do it is atomic_enable(). > > Thanks. > > > > > Long answer: I don't see a way to retrigger link training in ast_dp.c > > Without such change you are just shifting things around. The > > end-result of moving link-training to atomic_enable() is that each > > enable can trigger link training, possibly lowering the link rate, > > etc. if link training is just a status bit from the firmware that we > > don't control, it doesn't make real-real sense to move it. > > I have to think about what to do. People tend to copy existing drivers, > which alone might be a good argument for using atomic_enable. The link > training is indeed just a flag that is set by the firmware. I think it's > possible to re-trigger training by powering the port down and up again. > atomic_enable could likely do that. The hardware is also somewhat buggy and > not fully standard conformant. It stil looks like having an explicit comment ('check LT here becasue handled by firmware') might be better. -- With best wishes Dmitry
Re: DisplayPort: handling of HPD events / link training
On Tue, 16 Jul 2024 at 18:58, Thomas Zimmermann wrote: > > Hi > > Am 27.02.24 um 23:40 schrieb Dmitry Baryshkov: > > Hello, > > > > We are currently looking at checking and/or possibly redesigning the > > way the MSM DRM driver handles the HPD events and link training. > > > > After a quick glance at the drivers implementing DP support, I noticed > > following main approaches: > > - Perform link training at the atomic_enable time, don't report > > failures (mtk, analogix, zynqmp, tegra, nouveau) > > - Perform link training at the atomic_enable time, report errors using > > link_status property (i915, mhdp8546) > > - Perform link training on the plug event (msm, it8605). > > - Perform link training from the DPMS handler, also calling it from > > the enable callback (AMDGPU, radeon). > > > > It looks like the majority wins and we should move HPD to > > atomic_enable time. Is that assumption correct? > > Did you ever receive an answer to this question? I currently investigate > ast's DP code, which does link training as part of detecting the > connector state (in detect_ctx). But most other drivers do this in > atomic_enable. I wonder if ast should follow. Short answer: yes, the only proper place to do it is atomic_enable(). Long answer: I don't see a way to retrigger link training in ast_dp.c Without such change you are just shifting things around. The end-result of moving link-training to atomic_enable() is that each enable can trigger link training, possibly lowering the link rate, etc. if link training is just a status bit from the firmware that we don't control, it doesn't make real-real sense to move it. > > Best regards > Thomas > > > > > Also two related questions: > > - Is there a plan to actually make use of the link_status property? > > Intel presented it at FOSDEM 2018, but since that time it was not > > picked up by other drivers. > > > > - Is there any plan to create generic DP link training helpers? After > > glancing through the DP drivers there is a lot of similar code in the > > link training functions, with minor differences here and there. And > > it's those minor differences that bug me. It means that drivers might > > respond differently to similar devices. Or that there might be minor > > bugs here and there. > > > > -- > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Frankenstrasse 146, 90461 Nuernberg, Germany > GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman > HRB 36809 (AG Nuernberg) > -- With best wishes Dmitry
[PATCH v6] drm/display: split DSC helpers from DP helpers
Currently the DRM DSC functions are selected by the DRM_DISPLAY_DP_HELPER Kconfig symbol. This is not optimal, since the DSI code (both panel and host drivers) end up selecting the seemingly irrelevant DP helpers. Split the DSC code to be guarded by the separate DRM_DISPLAY_DSC_HELPER Kconfig symbol. Reviewed-by: Jessica Zhang Reviewed-by: Marijn Suijten Acked-by: Rodrigo Vivi #i915 Signed-off-by: Dmitry Baryshkov --- Changes in v6: - Moved the Makefile entry to follow the sorting order (Thomas Zimmermann) - Link to v5: https://lore.kernel.org/r/20240623-panel-sw43408-fix-v5-1-5401ab61e...@linaro.org Changes in v5: - Drop applied patches - Link to v4: https://lore.kernel.org/r/20240528-panel-sw43408-fix-v4-0-330b42445...@linaro.org Changes in v4: - Reoder patches so that fixes come first, to be able to land them to drm-misc-fixes - Link to v3: https://lore.kernel.org/r/20240522-panel-sw43408-fix-v3-0-6902285ad...@linaro.org Changes in v3: - Split DRM_DISPLAY_DSC_HELPER from DRM_DISPLAY_DP_HELPER - Added missing Fixes tags - Link to v2: https://lore.kernel.org/r/20240510-panel-sw43408-fix-v2-0-d1ef91ee1...@linaro.org Changes in v2: - use SELECT instead of DEPEND to follow the reverted Kconfig changes - Link to v1: https://lore.kernel.org/r/20240420-panel-sw43408-fix-v1-0-b282ff725...@linaro.org --- drivers/gpu/drm/amd/amdgpu/Kconfig | 1 + drivers/gpu/drm/display/Kconfig| 6 ++ drivers/gpu/drm/display/Makefile | 5 +++-- drivers/gpu/drm/i915/Kconfig | 1 + drivers/gpu/drm/msm/Kconfig| 1 + drivers/gpu/drm/panel/Kconfig | 6 +++--- 6 files changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig index 0051fb1b437f..fc3237da8090 100644 --- a/drivers/gpu/drm/amd/amdgpu/Kconfig +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig @@ -6,6 +6,7 @@ config DRM_AMDGPU depends on !UML select FW_LOADER select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DSC_HELPER select DRM_DISPLAY_HDMI_HELPER select DRM_DISPLAY_HDCP_HELPER select DRM_DISPLAY_HELPER diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig index 479e62690d75..a2e42014ffe0 100644 --- a/drivers/gpu/drm/display/Kconfig +++ b/drivers/gpu/drm/display/Kconfig @@ -59,6 +59,12 @@ config DRM_DISPLAY_DP_TUNNEL_STATE_DEBUG If in doubt, say "N". +config DRM_DISPLAY_DSC_HELPER + bool + depends on DRM_DISPLAY_HELPER + help + DRM display helpers for VESA DSC (used by DSI and DisplayPort). + config DRM_DISPLAY_HDCP_HELPER bool depends on DRM_DISPLAY_HELPER diff --git a/drivers/gpu/drm/display/Makefile b/drivers/gpu/drm/display/Makefile index 629df2f4d322..a023f72fa139 100644 --- a/drivers/gpu/drm/display/Makefile +++ b/drivers/gpu/drm/display/Makefile @@ -6,10 +6,11 @@ drm_display_helper-y := drm_display_helper_mod.o drm_display_helper-$(CONFIG_DRM_DISPLAY_DP_HELPER) += \ drm_dp_dual_mode_helper.o \ drm_dp_helper.o \ - drm_dp_mst_topology.o \ - drm_dsc_helper.o + drm_dp_mst_topology.o drm_display_helper-$(CONFIG_DRM_DISPLAY_DP_TUNNEL) += \ drm_dp_tunnel.o +drm_display_helper-$(CONFIG_DRM_DISPLAY_DSC_HELPER) += \ + drm_dsc_helper.o drm_display_helper-$(CONFIG_DRM_DISPLAY_HDCP_HELPER) += drm_hdcp_helper.o drm_display_helper-$(CONFIG_DRM_DISPLAY_HDMI_HELPER) += \ drm_hdmi_helper.o \ diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index faa253b27664..db400aad88fa 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -11,6 +11,7 @@ config DRM_I915 select SHMEM select TMPFS select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DSC_HELPER select DRM_DISPLAY_HDCP_HELPER select DRM_DISPLAY_HDMI_HELPER select DRM_DISPLAY_HELPER diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig index 26a4c71da63a..420385c47193 100644 --- a/drivers/gpu/drm/msm/Kconfig +++ b/drivers/gpu/drm/msm/Kconfig @@ -112,6 +112,7 @@ config DRM_MSM_DSI depends on DRM_MSM select DRM_PANEL select DRM_MIPI_DSI + select DRM_DISPLAY_DSC_HELPER default y help Choose this option if you have a need for MIPI DSI connector diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 9f49b0189d3b..dac01ade7e2e 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -369,7 +369,7 @@ config DRM_PANEL_LG_SW43408 depends on OF depends on DRM_MIPI_DSI depends on BACKLIGHT_CLASS_DEVICE - select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DSC_HELPER select DRM_DISPLAY_HELPER help Say Y here if you want to enable support for LG sw43408 panel. @@ -578,7 +578,7 @@ config DRM_PANEL_RAYDIUM_RM692E5 depends on OF depends on DR
Re: [PATCH v1] drm/ci: uprev IGT
On Thu, Jul 04, 2024 at 02:52:02PM GMT, Vignesh Raman wrote: > Uprev IGT to the latest version, which includes a fix for the > writeback tests issue on MSM devices. Enable debugging for > igt-runner to log output such as 'Begin test' and 'End test'. > This will help identify which test causes system freeze or hangs. > Update xfails and add metadata header for each flake test. > > Signed-off-by: Vignesh Raman > --- > > v1: > - https://gitlab.freedesktop.org/vigneshraman/linux/-/pipelines/1216850 > > --- > drivers/gpu/drm/ci/gitlab-ci.yml | 2 +- > drivers/gpu/drm/ci/igt_runner.sh | 1 + > .../gpu/drm/ci/xfails/amdgpu-stoney-fails.txt | 1 + > .../drm/ci/xfails/amdgpu-stoney-flakes.txt| 14 +- > .../gpu/drm/ci/xfails/amdgpu-stoney-skips.txt | 4 +- > drivers/gpu/drm/ci/xfails/i915-amly-fails.txt | 12 +- > .../gpu/drm/ci/xfails/i915-amly-flakes.txt| 41 - > drivers/gpu/drm/ci/xfails/i915-amly-skips.txt | 5 +- > drivers/gpu/drm/ci/xfails/i915-apl-flakes.txt | 2 +- > drivers/gpu/drm/ci/xfails/i915-apl-skips.txt | 4 +- > drivers/gpu/drm/ci/xfails/i915-cml-fails.txt | 14 +- > drivers/gpu/drm/ci/xfails/i915-cml-flakes.txt | 9 +- > drivers/gpu/drm/ci/xfails/i915-cml-skips.txt | 5 +- > drivers/gpu/drm/ci/xfails/i915-glk-fails.txt | 24 +-- > drivers/gpu/drm/ci/xfails/i915-glk-flakes.txt | 8 +- > drivers/gpu/drm/ci/xfails/i915-glk-skips.txt | 4 +- > drivers/gpu/drm/ci/xfails/i915-kbl-fails.txt | 2 + > drivers/gpu/drm/ci/xfails/i915-kbl-flakes.txt | 2 +- > drivers/gpu/drm/ci/xfails/i915-kbl-skips.txt | 4 +- > drivers/gpu/drm/ci/xfails/i915-tgl-fails.txt | 25 +-- > drivers/gpu/drm/ci/xfails/i915-tgl-skips.txt | 4 +- > drivers/gpu/drm/ci/xfails/i915-whl-fails.txt | 17 +- > drivers/gpu/drm/ci/xfails/i915-whl-flakes.txt | 2 +- > drivers/gpu/drm/ci/xfails/i915-whl-skips.txt | 5 +- > .../drm/ci/xfails/mediatek-mt8173-fails.txt | 9 +- > .../drm/ci/xfails/mediatek-mt8173-flakes.txt | 32 +++- > .../drm/ci/xfails/mediatek-mt8173-skips.txt | 4 +- > .../drm/ci/xfails/mediatek-mt8183-fails.txt | 2 +- > .../drm/ci/xfails/mediatek-mt8183-skips.txt | 2 +- > .../gpu/drm/ci/xfails/meson-g12b-fails.txt| 2 +- > .../gpu/drm/ci/xfails/meson-g12b-skips.txt| 2 +- > .../gpu/drm/ci/xfails/msm-apq8016-fails.txt | 5 +- > .../gpu/drm/ci/xfails/msm-apq8016-skips.txt | 2 +- > .../gpu/drm/ci/xfails/msm-apq8096-flakes.txt | 2 +- > .../gpu/drm/ci/xfails/msm-apq8096-skips.txt | 4 +- Acked-by: Dmitry Baryshkov # msm tests > .../msm-sc7180-trogdor-kingoftown-fails.txt | 145 -- > .../msm-sc7180-trogdor-kingoftown-flakes.txt | 18 ++- > .../msm-sc7180-trogdor-kingoftown-skips.txt | 5 +- > ...sm-sc7180-trogdor-lazor-limozeen-fails.txt | 145 -- > ...m-sc7180-trogdor-lazor-limozeen-flakes.txt | 11 +- > ...sm-sc7180-trogdor-lazor-limozeen-skips.txt | 2 +- > .../gpu/drm/ci/xfails/msm-sdm845-flakes.txt | 105 - > .../gpu/drm/ci/xfails/msm-sdm845-skips.txt| 4 +- > .../drm/ci/xfails/rockchip-rk3288-fails.txt | 2 +- > .../drm/ci/xfails/rockchip-rk3288-skips.txt | 2 +- > .../drm/ci/xfails/rockchip-rk3399-fails.txt | 2 +- > .../drm/ci/xfails/rockchip-rk3399-flakes.txt | 4 +- > .../drm/ci/xfails/rockchip-rk3399-skips.txt | 2 +- > .../drm/ci/xfails/virtio_gpu-none-fails.txt | 64 > .../drm/ci/xfails/virtio_gpu-none-skips.txt | 4 +- > drivers/gpu/drm/ci/xfails/vkms-none-fails.txt | 4 - > .../gpu/drm/ci/xfails/vkms-none-flakes.txt| 21 +++ > drivers/gpu/drm/ci/xfails/vkms-none-skips.txt | 105 - > 53 files changed, 527 insertions(+), 395 deletions(-) > > diff --git a/drivers/gpu/drm/ci/gitlab-ci.yml > b/drivers/gpu/drm/ci/gitlab-ci.yml > index 80fb0f57ae46..b09976c3d2c2 100644 > --- a/drivers/gpu/drm/ci/gitlab-ci.yml > +++ b/drivers/gpu/drm/ci/gitlab-ci.yml > @@ -5,7 +5,7 @@ variables: >UPSTREAM_REPO: git://anongit.freedesktop.org/drm/drm >TARGET_BRANCH: drm-next > > - IGT_VERSION: 0df7b9b97f9da0e364f5ee30fe331004b8c86b56 > + IGT_VERSION: f13702b8e4e847c56da3ef6f0969065d686049c5 > >DEQP_RUNNER_GIT_URL: https://gitlab.freedesktop.org/anholt/deqp-runner.git >DEQP_RUNNER_GIT_TAG: v0.15.0 -- With best wishes Dmitry
[PATCH v5] drm/display: split DSC helpers from DP helpers
Currently the DRM DSC functions are selected by the DRM_DISPLAY_DP_HELPER Kconfig symbol. This is not optimal, since the DSI code (both panel and host drivers) end up selecting the seemingly irrelevant DP helpers. Split the DSC code to be guarded by the separate DRM_DISPLAY_DSC_HELPER Kconfig symbol. Reviewed-by: Jessica Zhang Reviewed-by: Marijn Suijten Signed-off-by: Dmitry Baryshkov --- To: Alex Deucher To: Christian König To: Pan, Xinhui To: David Airlie To: Daniel Vetter To: Maarten Lankhorst To: Maxime Ripard To: Thomas Zimmermann To: Jani Nikula To: Joonas Lahtinen To: Rodrigo Vivi To: Tvrtko Ursulin To: Rob Clark To: Abhinav Kumar To: Sean Paul To: Marijn Suijten To: Neil Armstrong To: Jessica Zhang Cc: amd-...@lists.freedesktop.org Cc: dri-de...@lists.freedesktop.org Cc: linux-ker...@vger.kernel.org Cc: intel-gfx@lists.freedesktop.org Cc: linux-arm-...@vger.kernel.org Cc: freedr...@lists.freedesktop.org Signed-off-by: Dmitry Baryshkov Changes in v5: - Drop applied patches - Link to v4: https://lore.kernel.org/r/20240528-panel-sw43408-fix-v4-0-330b42445...@linaro.org Changes in v4: - Reoder patches so that fixes come first, to be able to land them to drm-misc-fixes - Link to v3: https://lore.kernel.org/r/20240522-panel-sw43408-fix-v3-0-6902285ad...@linaro.org Changes in v3: - Split DRM_DISPLAY_DSC_HELPER from DRM_DISPLAY_DP_HELPER - Added missing Fixes tags - Link to v2: https://lore.kernel.org/r/20240510-panel-sw43408-fix-v2-0-d1ef91ee1...@linaro.org Changes in v2: - use SELECT instead of DEPEND to follow the reverted Kconfig changes - Link to v1: https://lore.kernel.org/r/20240420-panel-sw43408-fix-v1-0-b282ff725...@linaro.org --- drivers/gpu/drm/amd/amdgpu/Kconfig | 1 + drivers/gpu/drm/display/Kconfig| 6 ++ drivers/gpu/drm/display/Makefile | 3 ++- drivers/gpu/drm/i915/Kconfig | 1 + drivers/gpu/drm/msm/Kconfig| 1 + drivers/gpu/drm/panel/Kconfig | 6 +++--- 6 files changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig index 4232ab27f990..5933ca8c6b96 100644 --- a/drivers/gpu/drm/amd/amdgpu/Kconfig +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig @@ -6,6 +6,7 @@ config DRM_AMDGPU depends on !UML select FW_LOADER select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DSC_HELPER select DRM_DISPLAY_HDMI_HELPER select DRM_DISPLAY_HDCP_HELPER select DRM_DISPLAY_HELPER diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig index 479e62690d75..a2e42014ffe0 100644 --- a/drivers/gpu/drm/display/Kconfig +++ b/drivers/gpu/drm/display/Kconfig @@ -59,6 +59,12 @@ config DRM_DISPLAY_DP_TUNNEL_STATE_DEBUG If in doubt, say "N". +config DRM_DISPLAY_DSC_HELPER + bool + depends on DRM_DISPLAY_HELPER + help + DRM display helpers for VESA DSC (used by DSI and DisplayPort). + config DRM_DISPLAY_HDCP_HELPER bool depends on DRM_DISPLAY_HELPER diff --git a/drivers/gpu/drm/display/Makefile b/drivers/gpu/drm/display/Makefile index 629df2f4d322..df8f22c7e916 100644 --- a/drivers/gpu/drm/display/Makefile +++ b/drivers/gpu/drm/display/Makefile @@ -6,7 +6,8 @@ drm_display_helper-y := drm_display_helper_mod.o drm_display_helper-$(CONFIG_DRM_DISPLAY_DP_HELPER) += \ drm_dp_dual_mode_helper.o \ drm_dp_helper.o \ - drm_dp_mst_topology.o \ + drm_dp_mst_topology.o +drm_display_helper-$(CONFIG_DRM_DISPLAY_DSC_HELPER) += \ drm_dsc_helper.o drm_display_helper-$(CONFIG_DRM_DISPLAY_DP_TUNNEL) += \ drm_dp_tunnel.o diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index faa253b27664..db400aad88fa 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -11,6 +11,7 @@ config DRM_I915 select SHMEM select TMPFS select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DSC_HELPER select DRM_DISPLAY_HDCP_HELPER select DRM_DISPLAY_HDMI_HELPER select DRM_DISPLAY_HELPER diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig index 1931ecf73e32..6dcd26180611 100644 --- a/drivers/gpu/drm/msm/Kconfig +++ b/drivers/gpu/drm/msm/Kconfig @@ -111,6 +111,7 @@ config DRM_MSM_DSI depends on DRM_MSM select DRM_PANEL select DRM_MIPI_DSI + select DRM_DISPLAY_DSC_HELPER default y help Choose this option if you have a need for MIPI DSI connector diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index bf4eadfe21cb..afae8b130e9a 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -349,7 +349,7 @@ config DRM_PANEL_LG_SW43408 depends on OF depends on DRM_MIPI_DSI depends on BACKLIGHT_CLASS_DEVICE - select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DSC_HELPER select DRM_DISPLAY_HELPER
Re: [PATCH v7 2/9] drm: Support per-plane async flip configuration
On Tue, Jun 18, 2024 at 01:18:10PM GMT, André Almeida wrote: > Em 18/06/2024 07:07, Dmitry Baryshkov escreveu: > > On Tue, 18 Jun 2024 at 12:38, Jani Nikula > > wrote: > > > > > > On Tue, 18 Jun 2024, André Almeida wrote: > > > > Drivers have different capabilities on what plane types they can or > > > > cannot perform async flips. Create a plane::async_flip field so each > > > > driver can choose which planes they allow doing async flips. > > > > > > > > Signed-off-by: André Almeida > > > > --- > > > > include/drm/drm_plane.h | 5 + > > > > 1 file changed, 5 insertions(+) > > > > > > > > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h > > > > index 9507542121fa..0bebc72af5c3 100644 > > > > --- a/include/drm/drm_plane.h > > > > +++ b/include/drm/drm_plane.h > > > > @@ -786,6 +786,11 @@ struct drm_plane { > > > > * @kmsg_panic: Used to register a panic notifier for this plane > > > > */ > > > >struct kmsg_dumper kmsg_panic; > > > > + > > > > + /** > > > > + * @async_flip: indicates if a plane can do async flips > > > > + */ > > > > > > When is it okay to set or change the value of this member? > > > > > > If you don't document it, people will find creative uses for this. > > > > Maybe it's better to have a callback instead of a static field? This > > way it becomes clear that it's only relevant at the time of the > > atomic_check(). > > > > So we would have something like bool (*async_flip) for struct > drm_plane_funcs I suppose. Then each driver will implement this function and > check on runtime if it should flip or not, right? > > I agree that it makes more clear, but as far as I can see this is not > something that is subject to being changed at runtime at all, so it seems a > bit overkill to me to encapsulate a static information like that. I prefer > to improve the documentation on the struct member to see if this solves the > problem. What do you think of the following comment: It looks like I keep on mixing async_flips as handled via the DRM_MODE_PAGE_FLIP_ASYNC and the plane flips that are governed by .atomic_async_check / .atomic_async_update / drm_atomic_helper_check() and which end up being used just for legacy cursor updates. So, yes, those are two different code paths, but with your changes I think it becomes even easier to get confused between atomic_async_check() and .async_flip member. > /** > * @async_flip: indicates if a plane can perform async flips. The > * driver should set this true only for planes that the hardware > * supports flipping asynchronously. It may not be changed during > * runtime. This field is checked inside drm_mode_atomic_ioctl() to > * allow only the correct planes to go with DRM_MODE_PAGE_FLIP_ASYNC. > */ -- With best wishes Dmitry
Re: [PATCH v7 2/9] drm: Support per-plane async flip configuration
On Tue, 18 Jun 2024 at 12:38, Jani Nikula wrote: > > On Tue, 18 Jun 2024, André Almeida wrote: > > Drivers have different capabilities on what plane types they can or > > cannot perform async flips. Create a plane::async_flip field so each > > driver can choose which planes they allow doing async flips. > > > > Signed-off-by: André Almeida > > --- > > include/drm/drm_plane.h | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h > > index 9507542121fa..0bebc72af5c3 100644 > > --- a/include/drm/drm_plane.h > > +++ b/include/drm/drm_plane.h > > @@ -786,6 +786,11 @@ struct drm_plane { > >* @kmsg_panic: Used to register a panic notifier for this plane > >*/ > > struct kmsg_dumper kmsg_panic; > > + > > + /** > > + * @async_flip: indicates if a plane can do async flips > > + */ > > When is it okay to set or change the value of this member? > > If you don't document it, people will find creative uses for this. Maybe it's better to have a callback instead of a static field? This way it becomes clear that it's only relevant at the time of the atomic_check(). > > + bool async_flip; > > }; > > > > #define obj_to_plane(x) container_of(x, struct drm_plane, base) > > -- > Jani Nikula, Intel -- With best wishes Dmitry
Re: [PATCH v6 0/8] drm: Support per-plane async flip configuration
On Fri, Jun 14, 2024 at 12:35:27PM GMT, André Almeida wrote: > AMD hardware can do async flips with overlay planes, but currently there's no > easy way to enable that in DRM. To solve that, this patchset creates a new > drm_plane field, bool async_flip, that allows drivers to choose which plane > can > or cannot do async flips. This is latter used on drm_atomic_set_property when > users want to do async flips. > > Patch 1 allows async commits with IN_FENCE_ID in any driver. > > Patches 2 to 7 have no function change. As per current code, every driver that > allows async page flips using the atomic API, allows doing it only in the > primary plane. Those patches then enable it for every driver. > > Patch 8 finally enables async flip on overlay planes for amdgpu. > > Changes from v5: > - Instead of enabling plane->async_flip in the common code, move it to driver > code. > - Enable primary plane async flip on every driver > https://lore.kernel.org/dri-devel/20240612193713.167448-1-andrealm...@igalia.com/ > > André Almeida (8): > drm/atomic: Allow userspace to use explicit sync with atomic async > flips > drm: Support per-plane async flip configuration > drm/amdgpu: Enable async flips on the primary plane > drm: atmel-hlcdc: Enable async flips on the primary plane > drm/i915: Enable async flips on the primary plane > drm/nouveau: Enable async flips on the primary plane > drm/vc4: Enable async flips on the primary plane > drm/amdgpu: Make it possible to async flip overlay planes > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 2 ++ > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 3 +++ > drivers/gpu/drm/drm_atomic_uapi.c | 8 +--- > drivers/gpu/drm/i915/display/i9xx_plane.c | 3 +++ > drivers/gpu/drm/nouveau/dispnv04/crtc.c | 4 > drivers/gpu/drm/nouveau/dispnv50/wndw.c | 4 > drivers/gpu/drm/vc4/vc4_plane.c | 4 +++- The main question is why only these drivers were updated. > include/drm/drm_plane.h | 5 + > 8 files changed, 29 insertions(+), 4 deletions(-) > > -- > 2.45.2 > -- With best wishes Dmitry
Re: [PATCH v6 2/8] drm: Support per-plane async flip configuration
On Fri, Jun 14, 2024 at 12:35:29PM GMT, André Almeida wrote: > Drivers have different capabilities on what plane types they can or > cannot perform async flips. Create a plane::async_flip field so each > driver can choose which planes they allow doing async flips. > > Signed-off-by: André Almeida > --- > drivers/gpu/drm/drm_atomic_uapi.c | 4 ++-- > include/drm/drm_plane.h | 5 + > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c > b/drivers/gpu/drm/drm_atomic_uapi.c > index 2e1d9391febe..ed1af3455477 100644 > --- a/drivers/gpu/drm/drm_atomic_uapi.c > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > @@ -1079,9 +1079,9 @@ int drm_atomic_set_property(struct drm_atomic_state > *state, > break; > } > > - if (async_flip && plane_state->plane->type != > DRM_PLANE_TYPE_PRIMARY) { > + if (async_flip && !plane->async_flip) { So, after this patch async flips becomes disabled until the driver enables that manually. Whether that's desired or not is a separate topic, but this definitely should be explicitly mentioned in the commit message. > drm_dbg_atomic(prop->dev, > -"[OBJECT:%d] Only primary planes can be > changed during async flip\n", > +"[PLANE:%d] does not support async > flips\n", > obj->id); > ret = -EINVAL; > break; > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h > index 9507542121fa..0bebc72af5c3 100644 > --- a/include/drm/drm_plane.h > +++ b/include/drm/drm_plane.h > @@ -786,6 +786,11 @@ struct drm_plane { >* @kmsg_panic: Used to register a panic notifier for this plane >*/ > struct kmsg_dumper kmsg_panic; > + > + /** > + * @async_flip: indicates if a plane can do async flips > + */ > + bool async_flip; > }; > > #define obj_to_plane(x) container_of(x, struct drm_plane, base) > -- > 2.45.2 > -- With best wishes Dmitry
Re: (subset) [PATCH v4 0/3] drm/panel: two fixes for lg-sw43408
On Tue, 28 May 2024 22:39:17 +0300, Dmitry Baryshkov wrote: > Fix two issues with the panel-lg-sw43408 driver reported by the kernel > test robot. > > Applied to drm-misc-fixes, thanks! [1/3] drm/panel/lg-sw43408: select CONFIG_DRM_DISPLAY_DP_HELPER commit: 33defcacd207196a6b35857087e6335590adad62 [2/3] drm/panel/lg-sw43408: mark sw43408_backlight_ops as static commit: 8c318cb70c88aa02068db7518e852b909c9b400f Best regards, -- With best wishes Dmitry
Re: [PATCH v3 4/6] drm/ci: uprev IGT
On Wed, May 29, 2024 at 08:10:47AM +0530, Vignesh Raman wrote: > test-list.txt and test-list-full.txt are not generated for > cross-builds and they are required by drm-ci for testing > arm32 targets. This is fixed in igt-gpu-tools. So uprev > IGT to include the commit which fixes this issue. Also > disable building xe driver tests for non-intel platforms. > > Reviewed-by: Dmitry Baryshkov > Signed-off-by: Vignesh Raman > --- > > v2: > - Split IGT uprev to seperate patch. > > v3: > - No changes. > > --- > drivers/gpu/drm/ci/build-igt.sh | 4 > drivers/gpu/drm/ci/gitlab-ci.yml | 2 +- > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/ci/build-igt.sh b/drivers/gpu/drm/ci/build-igt.sh > index b7d2a49a6db3..eddb5f782a5e 100644 > --- a/drivers/gpu/drm/ci/build-igt.sh > +++ b/drivers/gpu/drm/ci/build-igt.sh > @@ -45,6 +45,10 @@ MESON_OPTIONS="-Doverlay=disabled\ > -Dlibunwind=enabled \ > -Dprefix=/igt" > > +if [[ "$KERNEL_ARCH" = "arm64" ]] || [[ "$KERNEL_ARCH" = "arm" ]]; then > +MESON_OPTIONS="$MESON_OPTIONS -Dxe_driver=disabled" > +fi > + > mkdir -p /igt > meson build $MESON_OPTIONS $EXTRA_MESON_ARGS > ninja -C build -j${FDO_CI_CONCURRENT:-4} || ninja -C build -j 1 > diff --git a/drivers/gpu/drm/ci/gitlab-ci.yml > b/drivers/gpu/drm/ci/gitlab-ci.yml > index 8f32de63d92e..1b29c3b6406b 100644 > --- a/drivers/gpu/drm/ci/gitlab-ci.yml > +++ b/drivers/gpu/drm/ci/gitlab-ci.yml > @@ -5,7 +5,7 @@ variables: >UPSTREAM_REPO: git://anongit.freedesktop.org/drm/drm >TARGET_BRANCH: drm-next > > - IGT_VERSION: d2af13d9f5be5ce23d996e4afd3e45990f5ab977 > + IGT_VERSION: 0df7b9b97f9da0e364f5ee30fe331004b8c86b56 Let's land this, then I'll ask to uprev to dc2d7fb4f978048b87707ea9ec32da748b01b378, which fixes an issue with the writeback tests on MSM devices. > >DEQP_RUNNER_GIT_URL: https://gitlab.freedesktop.org/anholt/deqp-runner.git >DEQP_RUNNER_GIT_TAG: v0.15.0 > -- > 2.40.1 > -- With best wishes Dmitry
Re: [PATCH v3 6/6] drm/ci: update xfails for the new testlist
On Wed, May 29, 2024 at 08:10:49AM +0530, Vignesh Raman wrote: > Now the testlist is used from IGT build, so update > xfails with the new testlist. > > Set the timeout of all i915 jobs to 1h30m since some jobs > takes more than 1 hour to complete. > > Reviewed-by: Dmitry Baryshkov This had an explicit '# msm testlist' at the end. Please don't drop important parts of tags. I didn't review fails/flakes for other platforms. > Signed-off-by: Vignesh Raman > --- > > v2: > - Set the timeout of all i915 jobs to 1h30m and updated expectations file. > > v3: > - Add a link to the email reporting the flaky tests to the maintainers. > > --- > drivers/gpu/drm/ci/test.yml | 6 +- > .../gpu/drm/ci/xfails/amdgpu-stoney-fails.txt | 41 ++-- > .../drm/ci/xfails/amdgpu-stoney-flakes.txt| 7 + > .../gpu/drm/ci/xfails/amdgpu-stoney-skips.txt | 18 ++ > drivers/gpu/drm/ci/xfails/i915-amly-fails.txt | 31 > .../gpu/drm/ci/xfails/i915-amly-flakes.txt| 9 + > drivers/gpu/drm/ci/xfails/i915-amly-skips.txt | 11 ++ > drivers/gpu/drm/ci/xfails/i915-apl-fails.txt | 46 +++-- > drivers/gpu/drm/ci/xfails/i915-apl-flakes.txt | 6 + > drivers/gpu/drm/ci/xfails/i915-apl-skips.txt | 15 ++ > drivers/gpu/drm/ci/xfails/i915-cml-fails.txt | 38 > drivers/gpu/drm/ci/xfails/i915-cml-flakes.txt | 6 + > drivers/gpu/drm/ci/xfails/i915-cml-skips.txt | 14 ++ > drivers/gpu/drm/ci/xfails/i915-glk-fails.txt | 41 +++- > drivers/gpu/drm/ci/xfails/i915-glk-flakes.txt | 7 + > drivers/gpu/drm/ci/xfails/i915-glk-skips.txt | 15 ++ > drivers/gpu/drm/ci/xfails/i915-kbl-fails.txt | 42 ++--- > drivers/gpu/drm/ci/xfails/i915-kbl-flakes.txt | 7 +- > drivers/gpu/drm/ci/xfails/i915-kbl-skips.txt | 25 +++ > drivers/gpu/drm/ci/xfails/i915-tgl-fails.txt | 77 > drivers/gpu/drm/ci/xfails/i915-tgl-skips.txt | 17 ++ > drivers/gpu/drm/ci/xfails/i915-whl-fails.txt | 63 --- > drivers/gpu/drm/ci/xfails/i915-whl-flakes.txt | 6 + > drivers/gpu/drm/ci/xfails/i915-whl-skips.txt | 11 ++ > .../drm/ci/xfails/mediatek-mt8173-fails.txt | 30 ++- > .../drm/ci/xfails/mediatek-mt8173-flakes.txt | 11 ++ > .../drm/ci/xfails/mediatek-mt8173-skips.txt | 4 + > .../drm/ci/xfails/mediatek-mt8183-fails.txt | 21 +-- > .../drm/ci/xfails/mediatek-mt8183-skips.txt | 4 + > .../gpu/drm/ci/xfails/meson-g12b-fails.txt| 24 +-- > .../gpu/drm/ci/xfails/meson-g12b-skips.txt| 4 + > .../gpu/drm/ci/xfails/msm-apq8016-fails.txt | 12 +- > .../gpu/drm/ci/xfails/msm-apq8016-skips.txt | 4 + > .../gpu/drm/ci/xfails/msm-apq8096-fails.txt | 7 + > .../gpu/drm/ci/xfails/msm-apq8096-flakes.txt | 6 + > .../gpu/drm/ci/xfails/msm-apq8096-skips.txt | 12 ++ > .../msm-sc7180-trogdor-kingoftown-fails.txt | 175 +- > .../msm-sc7180-trogdor-kingoftown-flakes.txt | 8 + > .../msm-sc7180-trogdor-kingoftown-skips.txt | 7 + > ...sm-sc7180-trogdor-lazor-limozeen-fails.txt | 175 +- > ...m-sc7180-trogdor-lazor-limozeen-flakes.txt | 6 + > ...sm-sc7180-trogdor-lazor-limozeen-skips.txt | 4 + > .../gpu/drm/ci/xfails/msm-sdm845-fails.txt| 38 +--- > .../gpu/drm/ci/xfails/msm-sdm845-flakes.txt | 25 ++- > .../gpu/drm/ci/xfails/msm-sdm845-skips.txt| 7 + > .../drm/ci/xfails/rockchip-rk3288-fails.txt | 62 +-- > .../drm/ci/xfails/rockchip-rk3288-skips.txt | 4 + > .../drm/ci/xfails/rockchip-rk3399-fails.txt | 83 + > .../drm/ci/xfails/rockchip-rk3399-flakes.txt | 13 +- > .../drm/ci/xfails/rockchip-rk3399-skips.txt | 4 + > drivers/gpu/drm/ci/xfails/update-xfails.py| 4 +- > .../drm/ci/xfails/virtio_gpu-none-fails.txt | 94 +++--- > .../drm/ci/xfails/virtio_gpu-none-skips.txt | 4 + > 53 files changed, 1023 insertions(+), 388 deletions(-) > create mode 100644 drivers/gpu/drm/ci/xfails/i915-amly-flakes.txt > create mode 100644 drivers/gpu/drm/ci/xfails/i915-apl-flakes.txt > create mode 100644 drivers/gpu/drm/ci/xfails/i915-cml-flakes.txt > create mode 100644 drivers/gpu/drm/ci/xfails/i915-glk-flakes.txt > create mode 100644 drivers/gpu/drm/ci/xfails/i915-whl-flakes.txt > create mode 100644 drivers/gpu/drm/ci/xfails/mediatek-mt8173-flakes.txt > create mode 100644 drivers/gpu/drm/ci/xfails/msm-apq8096-flakes.txt > create mode 100644 > drivers/gpu/drm/ci/xfails/msm-sc7180-trogdor-kingoftown-flakes.txt > create mode 100644 > drivers/gpu/drm/ci/xfails/msm-sc7180-trogdor-lazor-limozeen-flakes.txt > > diff --git a/drivers/gpu/drm/ci/test.yml b/drivers/gpu/drm/ci/test.yml > index 2615f67f6aa3..322cce714657 100644 > --- a/drivers/gpu/drm/ci/test.yml > +++ b/drivers/gpu/drm/ci/test.yml > @@ -19
Re: [PATCH v3 3/6] drm/ci: generate testlist from build
On Wed, May 29, 2024 at 08:10:46AM +0530, Vignesh Raman wrote: > Stop vendoring the testlist into the kernel. Instead, use the > testlist from the IGT build to ensure we do not miss renamed > or newly added tests. > > Signed-off-by: Vignesh Raman > --- > > v2: > - Fix testlist generation for arm and arm64 builds. > > v3: > - Rename generated testlist file to ci-testlist. > > --- > drivers/gpu/drm/ci/build-igt.sh | 35 + > drivers/gpu/drm/ci/igt_runner.sh |9 +- > drivers/gpu/drm/ci/testlist.txt | 2761 -- > 3 files changed, 40 insertions(+), 2765 deletions(-) > delete mode 100644 drivers/gpu/drm/ci/testlist.txt > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v3 2/6] drm/ci: add farm variable
On Wed, May 29, 2024 at 08:10:45AM +0530, Vignesh Raman wrote: > Mesa uses structured logs for logging and debug purpose, > https://mesa.pages.freedesktop.org/-/mesa/-/jobs/59165650/artifacts/results/job_detail.json > > Since drm-ci uses the mesa scripts, add the farm variable > and update the device type for missing jobs. > > Signed-off-by: Vignesh Raman > --- > > v3: > - New commit to add farm variable and update device type variable. > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v3 1/6] drm/ci: uprev mesa version
On Wed, May 29, 2024 at 08:10:44AM +0530, Vignesh Raman wrote: > zlib.net is not allowing tarball download anymore and results > in below error in kernel+rootfs_arm32 container build, > urllib.error.HTTPError: HTTP Error 403: Forbidden > urllib.error.HTTPError: HTTP Error 415: Unsupported Media Type > > Uprev mesa to latest version which includes a fix for this issue. > https://gitlab.freedesktop.org/mesa/mesa/-/commit/908f444e > > Use id_tokens for JWT authentication. Since s3 bucket is migrated to > mesa-rootfs, update the variables accordingly. Also copy helper scripts > to install, so that the ci jobs can use these scripts for logging. > > Signed-off-by: Vignesh Raman > --- > > v2: > - Uprev to recent version and use id_tokens for JWT authentication > > v3: > - Move adding farm variable and updating device type variable to seperate > commit > > --- > drivers/gpu/drm/ci/build-igt.sh | 2 +- > drivers/gpu/drm/ci/build.sh | 6 +++-- > drivers/gpu/drm/ci/container.yml | 12 +++-- > drivers/gpu/drm/ci/gitlab-ci.yml | 44 +-- > drivers/gpu/drm/ci/image-tags.yml | 2 +- > drivers/gpu/drm/ci/lava-submit.sh | 4 +-- > 6 files changed, 42 insertions(+), 28 deletions(-) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
[PATCH v4 3/3] drm/display: split DSC helpers from DP helpers
Currently the DRM DSC functions are selected by the DRM_DISPLAY_DP_HELPER Kconfig symbol. This is not optimal, since the DSI code (both panel and host drivers) end up selecting the seemingly irrelevant DP helpers. Split the DSC code to be guarded by the separate DRM_DISPLAY_DSC_HELPER Kconfig symbol. Reviewed-by: Jessica Zhang Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/amd/amdgpu/Kconfig | 1 + drivers/gpu/drm/display/Kconfig| 6 ++ drivers/gpu/drm/display/Makefile | 3 ++- drivers/gpu/drm/i915/Kconfig | 1 + drivers/gpu/drm/msm/Kconfig| 1 + drivers/gpu/drm/panel/Kconfig | 6 +++--- 6 files changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig index 4232ab27f990..5933ca8c6b96 100644 --- a/drivers/gpu/drm/amd/amdgpu/Kconfig +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig @@ -6,6 +6,7 @@ config DRM_AMDGPU depends on !UML select FW_LOADER select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DSC_HELPER select DRM_DISPLAY_HDMI_HELPER select DRM_DISPLAY_HDCP_HELPER select DRM_DISPLAY_HELPER diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig index 864a6488bfdf..f524cf95dec3 100644 --- a/drivers/gpu/drm/display/Kconfig +++ b/drivers/gpu/drm/display/Kconfig @@ -59,6 +59,12 @@ config DRM_DISPLAY_DP_TUNNEL_STATE_DEBUG If in doubt, say "N". +config DRM_DISPLAY_DSC_HELPER + bool + depends on DRM_DISPLAY_HELPER + help + DRM display helpers for VESA DSC (used by DSI and DisplayPort). + config DRM_DISPLAY_HDCP_HELPER bool depends on DRM_DISPLAY_HELPER diff --git a/drivers/gpu/drm/display/Makefile b/drivers/gpu/drm/display/Makefile index 17d2cc73ff56..2ec71e15c3cb 100644 --- a/drivers/gpu/drm/display/Makefile +++ b/drivers/gpu/drm/display/Makefile @@ -6,7 +6,8 @@ drm_display_helper-y := drm_display_helper_mod.o drm_display_helper-$(CONFIG_DRM_DISPLAY_DP_HELPER) += \ drm_dp_dual_mode_helper.o \ drm_dp_helper.o \ - drm_dp_mst_topology.o \ + drm_dp_mst_topology.o +drm_display_helper-$(CONFIG_DRM_DISPLAY_DSC_HELPER) += \ drm_dsc_helper.o drm_display_helper-$(CONFIG_DRM_DISPLAY_DP_TUNNEL) += \ drm_dp_tunnel.o diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index 5932024f8f95..117b84260b1c 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -11,6 +11,7 @@ config DRM_I915 select SHMEM select TMPFS select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DSC_HELPER select DRM_DISPLAY_HDCP_HELPER select DRM_DISPLAY_HDMI_HELPER select DRM_DISPLAY_HELPER diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig index 1931ecf73e32..6dcd26180611 100644 --- a/drivers/gpu/drm/msm/Kconfig +++ b/drivers/gpu/drm/msm/Kconfig @@ -111,6 +111,7 @@ config DRM_MSM_DSI depends on DRM_MSM select DRM_PANEL select DRM_MIPI_DSI + select DRM_DISPLAY_DSC_HELPER default y help Choose this option if you have a need for MIPI DSI connector diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 2ae0eb0638f3..3e3f63479544 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -340,7 +340,7 @@ config DRM_PANEL_LG_SW43408 depends on OF depends on DRM_MIPI_DSI depends on BACKLIGHT_CLASS_DEVICE - select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DSC_HELPER select DRM_DISPLAY_HELPER help Say Y here if you want to enable support for LG sw43408 panel. @@ -549,7 +549,7 @@ config DRM_PANEL_RAYDIUM_RM692E5 depends on OF depends on DRM_MIPI_DSI depends on BACKLIGHT_CLASS_DEVICE - select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DSC_HELPER select DRM_DISPLAY_HELPER help Say Y here if you want to enable support for Raydium RM692E5-based @@ -907,7 +907,7 @@ config DRM_PANEL_VISIONOX_R66451 depends on OF depends on DRM_MIPI_DSI depends on BACKLIGHT_CLASS_DEVICE - select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DSC_HELPER select DRM_DISPLAY_HELPER help Say Y here if you want to enable support for Visionox -- 2.39.2
[PATCH v4 2/3] drm/panel/lg-sw43408: mark sw43408_backlight_ops as static
Fix sparse warning regarding symbol 'sw43408_backlight_ops' not being declared. Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202404200739.hbwzvohr-...@intel.com/ Reviewed-by: Neil Armstrong Fixes: 069a6c0e94f9 ("drm: panel: Add LG sw43408 panel driver") Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/panel/panel-lg-sw43408.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/panel/panel-lg-sw43408.c b/drivers/gpu/drm/panel/panel-lg-sw43408.c index 115f4702d59f..2b3a73696dce 100644 --- a/drivers/gpu/drm/panel/panel-lg-sw43408.c +++ b/drivers/gpu/drm/panel/panel-lg-sw43408.c @@ -182,7 +182,7 @@ static int sw43408_backlight_update_status(struct backlight_device *bl) return mipi_dsi_dcs_set_display_brightness_large(dsi, brightness); } -const struct backlight_ops sw43408_backlight_ops = { +static const struct backlight_ops sw43408_backlight_ops = { .update_status = sw43408_backlight_update_status, }; -- 2.39.2
[PATCH v4 0/3] drm/panel: two fixes for lg-sw43408
Fix two issues with the panel-lg-sw43408 driver reported by the kernel test robot. Signed-off-by: Dmitry Baryshkov --- Changes in v4: - Reoder patches so that fixes come first, to be able to land them to drm-misc-fixes - Link to v3: https://lore.kernel.org/r/20240522-panel-sw43408-fix-v3-0-6902285ad...@linaro.org Changes in v3: - Split DRM_DISPLAY_DSC_HELPER from DRM_DISPLAY_DP_HELPER - Added missing Fixes tags - Link to v2: https://lore.kernel.org/r/20240510-panel-sw43408-fix-v2-0-d1ef91ee1...@linaro.org Changes in v2: - use SELECT instead of DEPEND to follow the reverted Kconfig changes - Link to v1: https://lore.kernel.org/r/20240420-panel-sw43408-fix-v1-0-b282ff725...@linaro.org --- Dmitry Baryshkov (3): drm/panel/lg-sw43408: select CONFIG_DRM_DISPLAY_DP_HELPER drm/panel/lg-sw43408: mark sw43408_backlight_ops as static drm/display: split DSC helpers from DP helpers drivers/gpu/drm/amd/amdgpu/Kconfig | 1 + drivers/gpu/drm/display/Kconfig | 6 ++ drivers/gpu/drm/display/Makefile | 3 ++- drivers/gpu/drm/i915/Kconfig | 1 + drivers/gpu/drm/msm/Kconfig | 1 + drivers/gpu/drm/panel/Kconfig| 6 -- drivers/gpu/drm/panel/panel-lg-sw43408.c | 2 +- 7 files changed, 16 insertions(+), 4 deletions(-) --- base-commit: 6dc544b66971c7f9909ff038b62149105272d26a change-id: 20240420-panel-sw43408-fix-ff6549c121be Best regards, -- Dmitry Baryshkov
[PATCH v4 1/3] drm/panel/lg-sw43408: select CONFIG_DRM_DISPLAY_DP_HELPER
This panel driver uses DSC PPS functions and as such depends on the DRM_DISPLAY_DP_HELPER. Select this symbol to make required functions available to the driver. Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202404200800.kysryyli-...@intel.com/ Fixes: 069a6c0e94f9 ("drm: panel: Add LG sw43408 panel driver") Reviewed-by: Neil Armstrong Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/panel/Kconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 982324ef5a41..2ae0eb0638f3 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -340,6 +340,8 @@ config DRM_PANEL_LG_SW43408 depends on OF depends on DRM_MIPI_DSI depends on BACKLIGHT_CLASS_DEVICE + select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_HELPER help Say Y here if you want to enable support for LG sw43408 panel. The panel has a 1080x2160@60Hz resolution and uses 24 bit RGB per -- 2.39.2
Re: [PATCH v2 1/6] drm/ci: uprev mesa version
On Thu, 23 May 2024 at 09:07, Vignesh Raman wrote: > > Hi Dmitry, > > On 20/05/24 16:13, Dmitry Baryshkov wrote: > > On Fri, May 17, 2024 at 02:54:57PM +0530, Vignesh Raman wrote: > >> zlib.net is not allowing tarball download anymore and results > >> in below error in kernel+rootfs_arm32 container build, > >> urllib.error.HTTPError: HTTP Error 403: Forbidden > >> urllib.error.HTTPError: HTTP Error 415: Unsupported Media Type > >> > >> Uprev mesa to latest version which includes a fix for this issue. > >> https://gitlab.freedesktop.org/mesa/mesa/-/commit/908f444e > >> > >> Use id_tokens for JWT authentication. Since s3 bucket is migrated to > >> mesa-rootfs, update the variables accordingly. Also copy helper scripts > >> to install, so that the ci jobs can use these scripts for logging. > >> > >> Signed-off-by: Vignesh Raman > >> --- > >> > >> v2: > >>- Uprev to recent version and use id_tokens for JWT authentication > >> > >> --- > >> drivers/gpu/drm/ci/build-igt.sh | 2 +- > >> drivers/gpu/drm/ci/build.sh | 6 +++-- > >> drivers/gpu/drm/ci/container.yml | 12 +++-- > >> drivers/gpu/drm/ci/gitlab-ci.yml | 44 +-- > >> drivers/gpu/drm/ci/image-tags.yml | 2 +- > >> drivers/gpu/drm/ci/lava-submit.sh | 4 +-- > >> drivers/gpu/drm/ci/test.yml | 2 ++ > >> 7 files changed, 44 insertions(+), 28 deletions(-) > >> > > > > [skipped] > > > >> diff --git a/drivers/gpu/drm/ci/test.yml b/drivers/gpu/drm/ci/test.yml > >> index 8bc63912fddb..612c9ede3507 100644 > >> --- a/drivers/gpu/drm/ci/test.yml > >> +++ b/drivers/gpu/drm/ci/test.yml > >> @@ -150,6 +150,8 @@ msm:sdm845: > >> BM_KERNEL: https://${PIPELINE_ARTIFACTS_BASE}/arm64/cheza-kernel > >> GPU_VERSION: sdm845 > >> RUNNER_TAG: google-freedreno-cheza > >> +DEVICE_TYPE: sdm845-cheza-r3 > >> +FARM: google > > > > I see that this is the only user of the FARM: tag. Is it correct? > > No, we need to add FARM variable for other jobs as well. Why? Even if we have to, we don't have them now and the change doesn't seem to be related to the uprev'ing of mesa. So this probably should go to a separate commit. > > > Also we miss DEVICE_TYPE for several other boards. Should we be adding > > them? > > Yes, device type needs to be added for msm:apq8016, msm:apq8096, virtio_gpu. > > I will add this. Thanks. I'd guess, separate commit too. > > Regards, > Vignesh > > > > >> script: > >> - ./install/bare-metal/cros-servo.sh > >> > >> -- > >> 2.40.1 > >> > > -- With best wishes Dmitry
[PATCH v3 2/3] drm/panel/lg-sw43408: select CONFIG_DRM_DISPLAY_DP_HELPER
This panel driver uses DSC PPS functions and as such depends on the DRM_DISPLAY_DP_HELPER. Select this symbol to make required functions available to the driver. Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202404200800.kysryyli-...@intel.com/ Fixes: 069a6c0e94f9 ("drm: panel: Add LG sw43408 panel driver") Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/panel/Kconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 4a2f621433ef..3e3f63479544 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -340,6 +340,8 @@ config DRM_PANEL_LG_SW43408 depends on OF depends on DRM_MIPI_DSI depends on BACKLIGHT_CLASS_DEVICE + select DRM_DISPLAY_DSC_HELPER + select DRM_DISPLAY_HELPER help Say Y here if you want to enable support for LG sw43408 panel. The panel has a 1080x2160@60Hz resolution and uses 24 bit RGB per -- 2.39.2
[PATCH v3 3/3] drm/panel/lg-sw43408: mark sw43408_backlight_ops as static
Fix sparse warning regarding symbol 'sw43408_backlight_ops' not being declared. Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202404200739.hbwzvohr-...@intel.com/ Reviewed-by: Neil Armstrong Fixes: 069a6c0e94f9 ("drm: panel: Add LG sw43408 panel driver") Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/panel/panel-lg-sw43408.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/panel/panel-lg-sw43408.c b/drivers/gpu/drm/panel/panel-lg-sw43408.c index 115f4702d59f..2b3a73696dce 100644 --- a/drivers/gpu/drm/panel/panel-lg-sw43408.c +++ b/drivers/gpu/drm/panel/panel-lg-sw43408.c @@ -182,7 +182,7 @@ static int sw43408_backlight_update_status(struct backlight_device *bl) return mipi_dsi_dcs_set_display_brightness_large(dsi, brightness); } -const struct backlight_ops sw43408_backlight_ops = { +static const struct backlight_ops sw43408_backlight_ops = { .update_status = sw43408_backlight_update_status, }; -- 2.39.2
[PATCH v3 1/3] drm/display: split DSC helpers from DP helpers
Currently the DRM DSC functions are selected by the DRM_DISPLAY_DP_HELPER Kconfig symbol. This is not optimal, since the DSI code (both panel and host drivers) end up selecting the seemingly irrelevant DP helpers. Split the DSC code to be guarded by the separate DRM_DISPLAY_DSC_HELPER Kconfig symbol. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/amd/amdgpu/Kconfig | 1 + drivers/gpu/drm/display/Kconfig| 6 ++ drivers/gpu/drm/display/Makefile | 3 ++- drivers/gpu/drm/i915/Kconfig | 1 + drivers/gpu/drm/msm/Kconfig| 1 + drivers/gpu/drm/panel/Kconfig | 4 ++-- 6 files changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig index 22d88f8ef527..b69d5c4a5367 100644 --- a/drivers/gpu/drm/amd/amdgpu/Kconfig +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig @@ -6,6 +6,7 @@ config DRM_AMDGPU depends on !UML select FW_LOADER select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DSC_HELPER select DRM_DISPLAY_HDMI_HELPER select DRM_DISPLAY_HDCP_HELPER select DRM_DISPLAY_HELPER diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig index 864a6488bfdf..f524cf95dec3 100644 --- a/drivers/gpu/drm/display/Kconfig +++ b/drivers/gpu/drm/display/Kconfig @@ -59,6 +59,12 @@ config DRM_DISPLAY_DP_TUNNEL_STATE_DEBUG If in doubt, say "N". +config DRM_DISPLAY_DSC_HELPER + bool + depends on DRM_DISPLAY_HELPER + help + DRM display helpers for VESA DSC (used by DSI and DisplayPort). + config DRM_DISPLAY_HDCP_HELPER bool depends on DRM_DISPLAY_HELPER diff --git a/drivers/gpu/drm/display/Makefile b/drivers/gpu/drm/display/Makefile index 17d2cc73ff56..2ec71e15c3cb 100644 --- a/drivers/gpu/drm/display/Makefile +++ b/drivers/gpu/drm/display/Makefile @@ -6,7 +6,8 @@ drm_display_helper-y := drm_display_helper_mod.o drm_display_helper-$(CONFIG_DRM_DISPLAY_DP_HELPER) += \ drm_dp_dual_mode_helper.o \ drm_dp_helper.o \ - drm_dp_mst_topology.o \ + drm_dp_mst_topology.o +drm_display_helper-$(CONFIG_DRM_DISPLAY_DSC_HELPER) += \ drm_dsc_helper.o drm_display_helper-$(CONFIG_DRM_DISPLAY_DP_TUNNEL) += \ drm_dp_tunnel.o diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index 5932024f8f95..117b84260b1c 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -11,6 +11,7 @@ config DRM_I915 select SHMEM select TMPFS select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DSC_HELPER select DRM_DISPLAY_HDCP_HELPER select DRM_DISPLAY_HDMI_HELPER select DRM_DISPLAY_HELPER diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig index 1931ecf73e32..6dcd26180611 100644 --- a/drivers/gpu/drm/msm/Kconfig +++ b/drivers/gpu/drm/msm/Kconfig @@ -111,6 +111,7 @@ config DRM_MSM_DSI depends on DRM_MSM select DRM_PANEL select DRM_MIPI_DSI + select DRM_DISPLAY_DSC_HELPER default y help Choose this option if you have a need for MIPI DSI connector diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 982324ef5a41..4a2f621433ef 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -547,7 +547,7 @@ config DRM_PANEL_RAYDIUM_RM692E5 depends on OF depends on DRM_MIPI_DSI depends on BACKLIGHT_CLASS_DEVICE - select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DSC_HELPER select DRM_DISPLAY_HELPER help Say Y here if you want to enable support for Raydium RM692E5-based @@ -905,7 +905,7 @@ config DRM_PANEL_VISIONOX_R66451 depends on OF depends on DRM_MIPI_DSI depends on BACKLIGHT_CLASS_DEVICE - select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DSC_HELPER select DRM_DISPLAY_HELPER help Say Y here if you want to enable support for Visionox -- 2.39.2
[PATCH v3 0/3] drm/panel: two fixes for lg-sw43408
Fix two issues with the panel-lg-sw43408 driver reported by the kernel test robot. To: Neil Armstrong To: Jessica Zhang To: Sam Ravnborg To: Maarten Lankhorst To: Maxime Ripard To: Thomas Zimmermann To: David Airlie To: Daniel Vetter To: Sumit Semwal To: Caleb Connolly To: Alex Deucher To: Christian König To: Pan, Xinhui To: Jani Nikula To: Joonas Lahtinen To: Rodrigo Vivi To: Tvrtko Ursulin To: Rob Clark To: Abhinav Kumar To: Sean Paul To: Marijn Suijten To: Vinod Koul To: Caleb Connolly Cc: dri-de...@lists.freedesktop.org Cc: linux-ker...@vger.kernel.org Cc: amd-...@lists.freedesktop.org Cc: intel-gfx@lists.freedesktop.org Cc: linux-arm-...@vger.kernel.org Cc: freedr...@lists.freedesktop.org Signed-off-by: Dmitry Baryshkov Changes in v3: - Split DRM_DISPLAY_DSC_HELPER from DRM_DISPLAY_DP_HELPER - Added missing Fixes tags - Link to v2: https://lore.kernel.org/r/20240510-panel-sw43408-fix-v2-0-d1ef91ee1...@linaro.org Changes in v2: - use SELECT instead of DEPEND to follow the reverted Kconfig changes - Link to v1: https://lore.kernel.org/r/20240420-panel-sw43408-fix-v1-0-b282ff725...@linaro.org --- Dmitry Baryshkov (3): drm/display: split DSC helpers from DP helpers drm/panel/lg-sw43408: select CONFIG_DRM_DISPLAY_DP_HELPER drm/panel/lg-sw43408: mark sw43408_backlight_ops as static drivers/gpu/drm/amd/amdgpu/Kconfig | 1 + drivers/gpu/drm/display/Kconfig | 6 ++ drivers/gpu/drm/display/Makefile | 3 ++- drivers/gpu/drm/i915/Kconfig | 1 + drivers/gpu/drm/msm/Kconfig | 1 + drivers/gpu/drm/panel/Kconfig| 6 -- drivers/gpu/drm/panel/panel-lg-sw43408.c | 2 +- 7 files changed, 16 insertions(+), 4 deletions(-) --- base-commit: 8314289a8d50a4e05d8ece1ae0445a3b57bb4d3b change-id: 20240420-panel-sw43408-fix-ff6549c121be Best regards, -- Dmitry Baryshkov
Re: [PATCH v2 6/6] drm/ci: update xfails for the new testlist
On Fri, May 17, 2024 at 02:55:02PM +0530, Vignesh Raman wrote: > Now the testlist is used from IGT build, so update > xfails with the new testlist. > > Set the timeout of all i915 jobs to 1h30m since some jobs > takes more than 1 hour to complete. > > Signed-off-by: Vignesh Raman > --- > > v2: > - Set the timeout of all i915 jobs to 1h30m and updated expectations file. > > --- > drivers/gpu/drm/ci/test.yml | 6 +- > .../gpu/drm/ci/xfails/amdgpu-stoney-fails.txt | 41 ++-- > .../drm/ci/xfails/amdgpu-stoney-flakes.txt| 6 + > .../gpu/drm/ci/xfails/amdgpu-stoney-skips.txt | 18 ++ > drivers/gpu/drm/ci/xfails/i915-amly-fails.txt | 31 > .../gpu/drm/ci/xfails/i915-amly-flakes.txt| 8 + > drivers/gpu/drm/ci/xfails/i915-amly-skips.txt | 11 ++ > drivers/gpu/drm/ci/xfails/i915-apl-fails.txt | 46 +++-- > drivers/gpu/drm/ci/xfails/i915-apl-flakes.txt | 5 + > drivers/gpu/drm/ci/xfails/i915-apl-skips.txt | 15 ++ > drivers/gpu/drm/ci/xfails/i915-cml-fails.txt | 38 > drivers/gpu/drm/ci/xfails/i915-cml-flakes.txt | 5 + > drivers/gpu/drm/ci/xfails/i915-cml-skips.txt | 14 ++ > drivers/gpu/drm/ci/xfails/i915-glk-fails.txt | 41 +++- > drivers/gpu/drm/ci/xfails/i915-glk-flakes.txt | 6 + > drivers/gpu/drm/ci/xfails/i915-glk-skips.txt | 15 ++ > drivers/gpu/drm/ci/xfails/i915-kbl-fails.txt | 42 ++--- > drivers/gpu/drm/ci/xfails/i915-kbl-flakes.txt | 6 +- > drivers/gpu/drm/ci/xfails/i915-kbl-skips.txt | 25 +++ > drivers/gpu/drm/ci/xfails/i915-tgl-fails.txt | 77 > drivers/gpu/drm/ci/xfails/i915-tgl-skips.txt | 16 ++ > drivers/gpu/drm/ci/xfails/i915-whl-fails.txt | 63 --- > drivers/gpu/drm/ci/xfails/i915-whl-flakes.txt | 5 + > drivers/gpu/drm/ci/xfails/i915-whl-skips.txt | 11 ++ > .../drm/ci/xfails/mediatek-mt8173-fails.txt | 30 ++- > .../drm/ci/xfails/mediatek-mt8173-flakes.txt | 10 + > .../drm/ci/xfails/mediatek-mt8173-skips.txt | 4 + > .../drm/ci/xfails/mediatek-mt8183-fails.txt | 21 +-- > .../drm/ci/xfails/mediatek-mt8183-skips.txt | 4 + > .../gpu/drm/ci/xfails/meson-g12b-fails.txt| 24 +-- > .../gpu/drm/ci/xfails/meson-g12b-skips.txt| 4 + > .../gpu/drm/ci/xfails/msm-apq8016-fails.txt | 12 +- > .../gpu/drm/ci/xfails/msm-apq8016-skips.txt | 4 + > .../gpu/drm/ci/xfails/msm-apq8096-fails.txt | 7 + > .../gpu/drm/ci/xfails/msm-apq8096-flakes.txt | 5 + > .../gpu/drm/ci/xfails/msm-apq8096-skips.txt | 12 ++ > .../msm-sc7180-trogdor-kingoftown-fails.txt | 175 +- > .../msm-sc7180-trogdor-kingoftown-flakes.txt | 7 + > .../msm-sc7180-trogdor-kingoftown-skips.txt | 7 + > ...sm-sc7180-trogdor-lazor-limozeen-fails.txt | 175 +- > ...m-sc7180-trogdor-lazor-limozeen-flakes.txt | 5 + > ...sm-sc7180-trogdor-lazor-limozeen-skips.txt | 4 + > .../gpu/drm/ci/xfails/msm-sdm845-fails.txt| 38 +--- > .../gpu/drm/ci/xfails/msm-sdm845-flakes.txt | 26 ++- > .../gpu/drm/ci/xfails/msm-sdm845-skips.txt| 7 + Reviewed-by: Dmitry Baryshkov # msm testlists We'd need to triage why the tests are failing, but at least it looks logical from my POV, no more full-test skips, etc. > .../drm/ci/xfails/rockchip-rk3288-fails.txt | 62 +-- > .../drm/ci/xfails/rockchip-rk3288-skips.txt | 4 + > .../drm/ci/xfails/rockchip-rk3399-fails.txt | 83 + > .../drm/ci/xfails/rockchip-rk3399-flakes.txt | 12 +- > .../drm/ci/xfails/rockchip-rk3399-skips.txt | 4 + > drivers/gpu/drm/ci/xfails/update-xfails.py| 4 +- > .../drm/ci/xfails/virtio_gpu-none-fails.txt | 94 +++--- > .../drm/ci/xfails/virtio_gpu-none-skips.txt | 4 + > 53 files changed, 1010 insertions(+), 389 deletions(-) > create mode 100644 drivers/gpu/drm/ci/xfails/i915-amly-flakes.txt > create mode 100644 drivers/gpu/drm/ci/xfails/i915-apl-flakes.txt > create mode 100644 drivers/gpu/drm/ci/xfails/i915-cml-flakes.txt > create mode 100644 drivers/gpu/drm/ci/xfails/i915-glk-flakes.txt > create mode 100644 drivers/gpu/drm/ci/xfails/i915-whl-flakes.txt > create mode 100644 drivers/gpu/drm/ci/xfails/mediatek-mt8173-flakes.txt > create mode 100644 drivers/gpu/drm/ci/xfails/msm-apq8096-flakes.txt > create mode 100644 > drivers/gpu/drm/ci/xfails/msm-sc7180-trogdor-kingoftown-flakes.txt > create mode 100644 > drivers/gpu/drm/ci/xfails/msm-sc7180-trogdor-lazor-limozeen-flakes.txt -- With best wishes Dmitry
Re: [PATCH v2 5/6] drm/ci: skip driver specific tests
On Fri, May 17, 2024 at 02:55:01PM +0530, Vignesh Raman wrote: > Skip driver specific tests and skip kms tests for > panfrost driver since it is not a kms driver. > > Signed-off-by: Vignesh Raman > --- I didn't perform a through check, but generally looks good. Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v2 4/6] drm/ci: uprev IGT
On Fri, May 17, 2024 at 02:55:00PM +0530, Vignesh Raman wrote: > test-list.txt and test-list-full.txt are not generated for > cross-builds and they are required by drm-ci for testing > arm32 targets. > > This is fixed in igt-gpu-tools. So uprev IGT to include the > commit which fixes this issue. Disable building xe driver > tests for non-intel platforms. > > Signed-off-by: Vignesh Raman > --- > > v2: > - Split IGT uprev to seperate patch. > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v2 3/6] drm/ci: build virtual GPU driver as module
On Fri, May 17, 2024 at 02:54:59PM +0530, Vignesh Raman wrote: > With latest IGT, the tests tries to load the module and it > fails. So build the virtual GPU driver for virtio as module. Why? If the test fails on module loading (if the driver is built-in) then it's the test that needs to be fixed, not the kerenel config. It's fine as a temporal workaround, but please include a link to the patch posted to fix the issue. > > Signed-off-by: Vignesh Raman > --- > > v2: > - No changes. > > --- > drivers/gpu/drm/ci/build.sh | 1 - > drivers/gpu/drm/ci/igt_runner.sh | 6 +++--- > drivers/gpu/drm/ci/image-tags.yml | 4 ++-- > drivers/gpu/drm/ci/test.yml | 1 + > drivers/gpu/drm/ci/x86_64.config | 2 +- > 5 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/ci/build.sh b/drivers/gpu/drm/ci/build.sh > index a67871fdcd3f..e938074ac8e7 100644 > --- a/drivers/gpu/drm/ci/build.sh > +++ b/drivers/gpu/drm/ci/build.sh > @@ -157,7 +157,6 @@ fi > > mkdir -p artifacts/install/lib > mv install/* artifacts/install/. > -rm -rf artifacts/install/modules > ln -s common artifacts/install/ci-common > cp .config artifacts/${CI_JOB_NAME}_config > > diff --git a/drivers/gpu/drm/ci/igt_runner.sh > b/drivers/gpu/drm/ci/igt_runner.sh > index 20026612a9bd..55532f79fbdc 100755 > --- a/drivers/gpu/drm/ci/igt_runner.sh > +++ b/drivers/gpu/drm/ci/igt_runner.sh > @@ -30,10 +30,10 @@ case "$DRIVER_NAME" in > export IGT_FORCE_DRIVER="panfrost" > fi > ;; > -amdgpu) > +amdgpu|virtio_gpu) > # Cannot use HWCI_KERNEL_MODULES as at that point we don't have the > module in /lib > -mv /install/modules/lib/modules/* /lib/modules/. > -modprobe amdgpu > +mv /install/modules/lib/modules/* /lib/modules/. || true > +modprobe --first-time $DRIVER_NAME > ;; > esac > > diff --git a/drivers/gpu/drm/ci/image-tags.yml > b/drivers/gpu/drm/ci/image-tags.yml > index 60323ebc7304..328f5c560742 100644 > --- a/drivers/gpu/drm/ci/image-tags.yml > +++ b/drivers/gpu/drm/ci/image-tags.yml > @@ -4,9 +4,9 @@ variables: > DEBIAN_BASE_TAG: "${CONTAINER_TAG}" > > DEBIAN_X86_64_BUILD_IMAGE_PATH: "debian/x86_64_build" > - DEBIAN_BUILD_TAG: "2023-10-08-config" > + DEBIAN_BUILD_TAG: "2024-05-09-virtio" > > - KERNEL_ROOTFS_TAG: "2023-10-06-amd" > + KERNEL_ROOTFS_TAG: "2024-05-09-virtio" > > DEBIAN_X86_64_TEST_BASE_IMAGE: "debian/x86_64_test-base" > DEBIAN_X86_64_TEST_IMAGE_GL_PATH: "debian/x86_64_test-gl" > diff --git a/drivers/gpu/drm/ci/test.yml b/drivers/gpu/drm/ci/test.yml > index 612c9ede3507..864ac3809d84 100644 > --- a/drivers/gpu/drm/ci/test.yml > +++ b/drivers/gpu/drm/ci/test.yml > @@ -350,6 +350,7 @@ virtio_gpu:none: >script: > - ln -sf $CI_PROJECT_DIR/install /install > - mv install/bzImage /lava-files/bzImage > +- mkdir -p /lib/modules Is it necessary to create it manually here? > - mkdir -p $CI_PROJECT_DIR/results > - ln -sf $CI_PROJECT_DIR/results /results > - install/crosvm-runner.sh install/igt_runner.sh > diff --git a/drivers/gpu/drm/ci/x86_64.config > b/drivers/gpu/drm/ci/x86_64.config > index 1cbd49a5b23a..78479f063e8e 100644 > --- a/drivers/gpu/drm/ci/x86_64.config > +++ b/drivers/gpu/drm/ci/x86_64.config > @@ -91,7 +91,7 @@ CONFIG_KVM=y > CONFIG_KVM_GUEST=y > CONFIG_VIRT_DRIVERS=y > CONFIG_VIRTIO_FS=y > -CONFIG_DRM_VIRTIO_GPU=y > +CONFIG_DRM_VIRTIO_GPU=m > CONFIG_SERIAL_8250_CONSOLE=y > CONFIG_VIRTIO_NET=y > CONFIG_VIRTIO_CONSOLE=y > -- > 2.40.1 > -- With best wishes Dmitry
Re: [PATCH v2 2/6] drm/ci: generate testlist from build
On Fri, May 17, 2024 at 02:54:58PM +0530, Vignesh Raman wrote: > Stop vendoring the testlist into the kernel. Instead, use the > testlist from the IGT build to ensure we do not miss renamed > or newly added tests. > > Signed-off-by: Vignesh Raman > --- > > v2: > - Fix testlist generation for arm and arm64 builds. > > --- > drivers/gpu/drm/ci/build-igt.sh | 34 + > drivers/gpu/drm/ci/igt_runner.sh |9 +- > drivers/gpu/drm/ci/testlist.txt | 2761 -- > 3 files changed, 39 insertions(+), 2765 deletions(-) > delete mode 100644 drivers/gpu/drm/ci/testlist.txt > > diff --git a/drivers/gpu/drm/ci/build-igt.sh b/drivers/gpu/drm/ci/build-igt.sh > index 7859554756c4..e62244728613 100644 > --- a/drivers/gpu/drm/ci/build-igt.sh > +++ b/drivers/gpu/drm/ci/build-igt.sh [...] > @@ -26,6 +50,16 @@ meson build $MESON_OPTIONS $EXTRA_MESON_ARGS > ninja -C build -j${FDO_CI_CONCURRENT:-4} || ninja -C build -j 1 > ninja -C build install > > +if [[ "$KERNEL_ARCH" = "arm64" ]]; then > +export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/igt/lib/aarch64-linux-gnu > +elif [[ "$KERNEL_ARCH" = "arm" ]]; then > +export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/igt/lib > +else > +export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/igt/lib64 Could you please clarify this part? The arm64 vs arm don't look logical from my point of view. The rest LGTM. > +fi > + > +generate_testlist > + > mkdir -p artifacts/ > tar -cf artifacts/igt.tar /igt > -- With best wishes Dmitry
Re: [PATCH v2 1/6] drm/ci: uprev mesa version
On Fri, May 17, 2024 at 02:54:57PM +0530, Vignesh Raman wrote: > zlib.net is not allowing tarball download anymore and results > in below error in kernel+rootfs_arm32 container build, > urllib.error.HTTPError: HTTP Error 403: Forbidden > urllib.error.HTTPError: HTTP Error 415: Unsupported Media Type > > Uprev mesa to latest version which includes a fix for this issue. > https://gitlab.freedesktop.org/mesa/mesa/-/commit/908f444e > > Use id_tokens for JWT authentication. Since s3 bucket is migrated to > mesa-rootfs, update the variables accordingly. Also copy helper scripts > to install, so that the ci jobs can use these scripts for logging. > > Signed-off-by: Vignesh Raman > --- > > v2: > - Uprev to recent version and use id_tokens for JWT authentication > > --- > drivers/gpu/drm/ci/build-igt.sh | 2 +- > drivers/gpu/drm/ci/build.sh | 6 +++-- > drivers/gpu/drm/ci/container.yml | 12 +++-- > drivers/gpu/drm/ci/gitlab-ci.yml | 44 +-- > drivers/gpu/drm/ci/image-tags.yml | 2 +- > drivers/gpu/drm/ci/lava-submit.sh | 4 +-- > drivers/gpu/drm/ci/test.yml | 2 ++ > 7 files changed, 44 insertions(+), 28 deletions(-) > [skipped] > diff --git a/drivers/gpu/drm/ci/test.yml b/drivers/gpu/drm/ci/test.yml > index 8bc63912fddb..612c9ede3507 100644 > --- a/drivers/gpu/drm/ci/test.yml > +++ b/drivers/gpu/drm/ci/test.yml > @@ -150,6 +150,8 @@ msm:sdm845: > BM_KERNEL: https://${PIPELINE_ARTIFACTS_BASE}/arm64/cheza-kernel > GPU_VERSION: sdm845 > RUNNER_TAG: google-freedreno-cheza > +DEVICE_TYPE: sdm845-cheza-r3 > +FARM: google I see that this is the only user of the FARM: tag. Is it correct? Also we miss DEVICE_TYPE for several other boards. Should we be adding them? >script: > - ./install/bare-metal/cros-servo.sh > > -- > 2.40.1 > -- With best wishes Dmitry
Re: [PATCH 09/11] drm/msm: Use fbdev client helpers
On Tue, May 07, 2024 at 01:58:30PM +0200, Thomas Zimmermann wrote: > Implement struct drm_client_funcs with the respective helpers and > remove the custom code from the emulation. The generic helpers are > equivalent in functionality. > > Signed-off-by: Thomas Zimmermann > --- > drivers/gpu/drm/msm/msm_fbdev.c | 58 ++--- > 1 file changed, 3 insertions(+), 55 deletions(-) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v1 2/5] drm/ci: generate testlist from build
On Tue, Apr 30, 2024 at 02:41:18PM +0530, Vignesh Raman wrote: > Stop vendoring the testlist into the kernel. Instead, use the > testlist from the IGT build to ensure we do not miss renamed > or newly added tests. > > Signed-off-by: Vignesh Raman > --- > drivers/gpu/drm/ci/build-igt.sh | 23 + > drivers/gpu/drm/ci/igt_runner.sh |9 +- > drivers/gpu/drm/ci/testlist.txt | 2761 -- > 3 files changed, 28 insertions(+), 2765 deletions(-) > delete mode 100644 drivers/gpu/drm/ci/testlist.txt > > diff --git a/drivers/gpu/drm/ci/build-igt.sh b/drivers/gpu/drm/ci/build-igt.sh > index 500fa4f5c30a..cedc62baba1e 100644 > --- a/drivers/gpu/drm/ci/build-igt.sh > +++ b/drivers/gpu/drm/ci/build-igt.sh > @@ -26,6 +26,29 @@ meson build $MESON_OPTIONS $EXTRA_MESON_ARGS > ninja -C build -j${FDO_CI_CONCURRENT:-4} || ninja -C build -j 1 > ninja -C build install > > +set +ex > +export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/igt/lib64 > +while read -r line; do > +if [ "$line" = "TESTLIST" ] || [ "$line" = "END TESTLIST" ]; then > +continue > +fi > + > +tests=$(echo "$line" | tr ' ' '\n') > + > +for test in $tests; do > +output=$(/igt/libexec/igt-gpu-tools/"$test" --list-subtests) > + > +if [ -z "$output" ]; then > +echo "$test" > +else > +echo "$output" | while read -r subtest; do > +echo "$test@$subtest" > +done > +fi > +done > +done < /igt/libexec/igt-gpu-tools/test-list.txt > > /igt/libexec/igt-gpu-tools/testlist.txt > +set -ex Is the list in sync between x86 and arm/arm64 IGT builds? Is there a chance of having a safety net here? > + > mkdir -p artifacts/ > tar -cf artifacts/igt.tar /igt > > diff --git a/drivers/gpu/drm/ci/igt_runner.sh > b/drivers/gpu/drm/ci/igt_runner.sh > index f1a08b9b146f..20026612a9bd 100755 > --- a/drivers/gpu/drm/ci/igt_runner.sh > +++ b/drivers/gpu/drm/ci/igt_runner.sh > @@ -59,25 +59,26 @@ fi > > curl -L --retry 4 -f --retry-all-errors --retry-delay 60 -s > ${FDO_HTTP_CACHE_URI:-}$PIPELINE_ARTIFACTS_BASE/$ARCH/igt.tar.gz | tar --zstd > -v -x -C / > > +TESTLIST="/igt/libexec/igt-gpu-tools/testlist.txt" > > # If the job is parallel at the gitab job level, take the corresponding > fraction > # of the caselist. > if [ -n "$CI_NODE_INDEX" ]; then > -sed -ni $CI_NODE_INDEX~$CI_NODE_TOTAL"p" /install/testlist.txt > +sed -ni $CI_NODE_INDEX~$CI_NODE_TOTAL"p" $TESTLIST > fi > > # core_getversion checks if the driver is loaded and probed correctly > # so run it in all shards > -if ! grep -q "core_getversion" /install/testlist.txt; then > +if ! grep -q "core_getversion" $TESTLIST; then > # Add the line to the file > -echo "core_getversion" >> /install/testlist.txt > +echo "core_getversion" >> $TESTLIST > fi > > set +e > igt-runner \ > run \ > --igt-folder /igt/libexec/igt-gpu-tools \ > ---caselist /install/testlist.txt \ > +--caselist $TESTLIST \ > --output /results \ > $IGT_SKIPS \ > $IGT_FLAKES \ -- With best wishes Dmitry
Re: [PATCH v1 5/5] drm/ci: update xfails for the new testlist
On Tue, Apr 30, 2024 at 02:41:21PM +0530, Vignesh Raman wrote: > Now the testlist is used from IGT build, so update > xfails with the new testlist. > > Signed-off-by: Vignesh Raman > --- > .../gpu/drm/ci/xfails/amdgpu-stoney-fails.txt | 47 +++ > .../drm/ci/xfails/amdgpu-stoney-flakes.txt| 8 +- > .../gpu/drm/ci/xfails/amdgpu-stoney-skips.txt | 15 > drivers/gpu/drm/ci/xfails/i915-amly-fails.txt | 22 - > .../gpu/drm/ci/xfails/i915-amly-flakes.txt| 8 ++ > drivers/gpu/drm/ci/xfails/i915-amly-skips.txt | 8 ++ > drivers/gpu/drm/ci/xfails/i915-apl-fails.txt | 45 +- > drivers/gpu/drm/ci/xfails/i915-apl-flakes.txt | 5 ++ > drivers/gpu/drm/ci/xfails/i915-apl-skips.txt | 12 +++ > drivers/gpu/drm/ci/xfails/i915-cml-fails.txt | 26 +- > drivers/gpu/drm/ci/xfails/i915-cml-flakes.txt | 6 ++ > drivers/gpu/drm/ci/xfails/i915-cml-skips.txt | 8 ++ > drivers/gpu/drm/ci/xfails/i915-glk-fails.txt | 28 +-- > drivers/gpu/drm/ci/xfails/i915-glk-skips.txt | 12 +++ > drivers/gpu/drm/ci/xfails/i915-kbl-fails.txt | 39 - > drivers/gpu/drm/ci/xfails/i915-kbl-flakes.txt | 10 ++- > drivers/gpu/drm/ci/xfails/i915-kbl-skips.txt | 21 + > drivers/gpu/drm/ci/xfails/i915-tgl-fails.txt | 75 + > drivers/gpu/drm/ci/xfails/i915-tgl-skips.txt | 13 +++ > drivers/gpu/drm/ci/xfails/i915-whl-fails.txt | 46 +-- > drivers/gpu/drm/ci/xfails/i915-whl-skips.txt | 8 ++ > .../drm/ci/xfails/mediatek-mt8173-fails.txt | 47 +++ > .../drm/ci/xfails/mediatek-mt8183-fails.txt | 17 +--- > .../drm/ci/xfails/mediatek-mt8183-flakes.txt | 5 ++ > .../gpu/drm/ci/xfails/meson-g12b-fails.txt| 20 + > .../gpu/drm/ci/xfails/meson-g12b-flakes.txt | 5 ++ > .../gpu/drm/ci/xfails/msm-apq8016-fails.txt | 26 ++ > .../gpu/drm/ci/xfails/msm-apq8016-flakes.txt | 5 ++ > .../gpu/drm/ci/xfails/msm-apq8096-fails.txt | 5 +- > .../gpu/drm/ci/xfails/msm-apq8096-flakes.txt | 5 ++ > .../gpu/drm/ci/xfails/msm-apq8096-skips.txt | 67 +++ > .../msm-sc7180-trogdor-kingoftown-fails.txt | 34 > .../msm-sc7180-trogdor-kingoftown-flakes.txt | 5 ++ > ...sm-sc7180-trogdor-lazor-limozeen-fails.txt | 34 > ...m-sc7180-trogdor-lazor-limozeen-flakes.txt | 5 ++ > .../gpu/drm/ci/xfails/msm-sdm845-fails.txt| 75 - > .../gpu/drm/ci/xfails/msm-sdm845-flakes.txt | 26 ++ > .../drm/ci/xfails/rockchip-rk3288-fails.txt | 54 > .../drm/ci/xfails/rockchip-rk3399-fails.txt | 80 ++ > .../drm/ci/xfails/rockchip-rk3399-flakes.txt | 7 -- > .../drm/ci/xfails/virtio_gpu-none-fails.txt | 82 +-- > .../drm/ci/xfails/virtio_gpu-none-skips.txt | 3 + > 42 files changed, 574 insertions(+), 495 deletions(-) > create mode 100644 drivers/gpu/drm/ci/xfails/i915-amly-flakes.txt > create mode 100644 drivers/gpu/drm/ci/xfails/i915-apl-flakes.txt > create mode 100644 drivers/gpu/drm/ci/xfails/i915-cml-flakes.txt > create mode 100644 drivers/gpu/drm/ci/xfails/mediatek-mt8183-flakes.txt > create mode 100644 drivers/gpu/drm/ci/xfails/meson-g12b-flakes.txt > create mode 100644 drivers/gpu/drm/ci/xfails/msm-apq8016-flakes.txt > create mode 100644 drivers/gpu/drm/ci/xfails/msm-apq8096-flakes.txt > create mode 100644 > drivers/gpu/drm/ci/xfails/msm-sc7180-trogdor-kingoftown-flakes.txt > create mode 100644 > drivers/gpu/drm/ci/xfails/msm-sc7180-trogdor-lazor-limozeen-flakes.txt > delete mode 100644 drivers/gpu/drm/ci/xfails/rockchip-rk3288-fails.txt > delete mode 100644 drivers/gpu/drm/ci/xfails/rockchip-rk3399-flakes.txt > [skipped] > diff --git a/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt > b/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt > index 44a5c62dedad..96e9faf0e607 100644 > --- a/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt > +++ b/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt > @@ -1,19 +1,9 @@ > +core_setmaster_vs_auth,Fail > +device_reset,Fail > +dumb_buffer,Fail This doesn't look correct, core tests should be passing. > kms_3d,Fail > -kms_addfb_basic@addfb25-bad-modifier,Fail > -kms_cursor_legacy@all-pipes-forked-bo,Fail > -kms_cursor_legacy@all-pipes-forked-move,Fail > -kms_cursor_legacy@all-pipes-single-bo,Fail > -kms_cursor_legacy@all-pipes-single-move,Fail > -kms_cursor_legacy@all-pipes-torture-bo,Fail > -kms_cursor_legacy@all-pipes-torture-move,Fail > -kms_cursor_legacy@pipe-A-forked-bo,Fail > -kms_cursor_legacy@pipe-A-forked-move,Fail > -kms_cursor_legacy@pipe-A-single-bo,Fail > -kms_cursor_legacy@pipe-A-single-move,Fail > -kms_cursor_legacy@pipe-A-torture-bo,Fail > -kms_cursor_legacy@pipe-A-torture-move,Fail > -kms_force_connector_basic@force-edid,Fail > -kms_hdmi_inject@inject-4k,Fail > -kms_selftest@drm_format,Timeout > -kms_selftest@drm_format_helper,Timeout Fine, kms_cursor_legacy tests were migrated to -flakes. But what happened with the rest of the failures? > -msm_mapping@ring,Fail >
Re: [PATCH v1 4/5] drm/ci: skip driver specific tests
On Tue, Apr 30, 2024 at 02:41:20PM +0530, Vignesh Raman wrote: > Skip driver specific tests and skip kms tests for > panfrost driver since it is not a kms driver. > > Signed-off-by: Vignesh Raman > --- > .../gpu/drm/ci/xfails/amdgpu-stoney-skips.txt | 14 +- > drivers/gpu/drm/ci/xfails/i915-amly-skips.txt | 14 +- > drivers/gpu/drm/ci/xfails/i915-apl-skips.txt| 14 +- > drivers/gpu/drm/ci/xfails/i915-cml-skips.txt| 12 > drivers/gpu/drm/ci/xfails/i915-glk-skips.txt| 14 +- > drivers/gpu/drm/ci/xfails/i915-kbl-skips.txt| 14 +- > drivers/gpu/drm/ci/xfails/i915-tgl-skips.txt| 14 +- > drivers/gpu/drm/ci/xfails/i915-whl-skips.txt| 14 +- > .../gpu/drm/ci/xfails/mediatek-mt8173-skips.txt | 12 > .../gpu/drm/ci/xfails/mediatek-mt8183-skips.txt | 14 ++ > drivers/gpu/drm/ci/xfails/meson-g12b-skips.txt | 14 ++ > drivers/gpu/drm/ci/xfails/msm-apq8016-skips.txt | 14 ++ > drivers/gpu/drm/ci/xfails/msm-apq8096-skips.txt | 14 +- Reviewed-by: Dmitry Baryshkov # msm skips > .../msm-sc7180-trogdor-kingoftown-skips.txt | 15 +++ > .../msm-sc7180-trogdor-lazor-limozeen-skips.txt | 15 +++ > drivers/gpu/drm/ci/xfails/msm-sdm845-skips.txt | 15 +++ > .../gpu/drm/ci/xfails/rockchip-rk3288-skips.txt | 17 - > .../gpu/drm/ci/xfails/rockchip-rk3399-skips.txt | 15 +++ > .../gpu/drm/ci/xfails/virtio_gpu-none-skips.txt | 15 ++- > 19 files changed, 260 insertions(+), 10 deletions(-) > create mode 100644 drivers/gpu/drm/ci/xfails/mediatek-mt8173-skips.txt > create mode 100644 drivers/gpu/drm/ci/xfails/mediatek-mt8183-skips.txt > create mode 100644 drivers/gpu/drm/ci/xfails/meson-g12b-skips.txt > create mode 100644 drivers/gpu/drm/ci/xfails/msm-apq8016-skips.txt > -- With best wishes Dmitry
Re: [PATCH v1 3/4] drm/ci: uprev IGT and generate testlist from build
On Tue, 23 Apr 2024 at 13:24, Maíra Canal wrote: > > On 4/23/24 01:02, Vignesh Raman wrote: > > Uprev IGT to the latest version and stop vendoring the > > testlist into the kernel. Instead, use the testlist from > > the IGT build to ensure we do not miss renamed or newly > > added tests. > > Nitpick: wouldn't it be better to (1) stop vendoring the > testlist into the kernel in one patch and then (2) uprev > IGT to the latest version? I feel that this patch is changing > a lot of different stuff. Definitely. Otherwise it's hard to understand whether a change to fails/flakes is caused by the uprev or by the testlist change. E.g. in several cases the list of failing subtests seems to be replaced with the test itself. > > Best Regards, > - Maíra > > > > > Skip kms tests for panfrost driver since it is not a kms > > driver and skip driver-specific tests. Update xfails with > > the latest testlist. > > > > Increase the timoout of i915 jobs to 2h30m since some jobs > > takes more than 2 hours to complete. > > > > Signed-off-by: Vignesh Raman > > --- -- With best wishes Dmitry
Re: [PATCH 1/2] drm/print: drop include debugfs.h and include where needed
On Mon, Apr 22, 2024 at 03:10:10PM +0300, Jani Nikula wrote: > Surprisingly many places depend on debugfs.h to be included via > drm_print.h. Fix them. > > v3: Also fix armada, ite-it6505, imagination, msm, sti, vc4, and xe > > v2: Also fix ivpu and vmwgfx > > Reviewed-by: Andrzej Hajda > Acked-by: Maxime Ripard > Link: > https://patchwork.freedesktop.org/patch/msgid/20240410141434.157908-1-jani.nik...@intel.com > Signed-off-by: Jani Nikula > > --- > > Cc: Jacek Lawrynowicz > Cc: Stanislaw Gruszka > Cc: Oded Gabbay > Cc: Russell King > Cc: David Airlie > Cc: Daniel Vetter > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Robert Foss > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Thomas Zimmermann > Cc: Jani Nikula > Cc: Rodrigo Vivi > Cc: Joonas Lahtinen > Cc: Tvrtko Ursulin > Cc: Frank Binns > Cc: Matt Coster > Cc: Rob Clark > Cc: Abhinav Kumar > Cc: Dmitry Baryshkov > Cc: Sean Paul > Cc: Marijn Suijten > Cc: Karol Herbst > Cc: Lyude Paul > Cc: Danilo Krummrich > Cc: Alex Deucher > Cc: "Christian König" > Cc: "Pan, Xinhui" > Cc: Alain Volmat > Cc: Huang Rui > Cc: Zack Rusin > Cc: Broadcom internal kernel review list > > Cc: Lucas De Marchi > Cc: "Thomas Hellström" > Cc: dri-de...@lists.freedesktop.org > Cc: intel-gfx@lists.freedesktop.org > Cc: intel...@lists.freedesktop.org > Cc: linux-arm-...@vger.kernel.org > Cc: freedr...@lists.freedesktop.org > Cc: nouv...@lists.freedesktop.org > Cc: amd-...@lists.freedesktop.org > --- > drivers/accel/ivpu/ivpu_debugfs.c | 2 ++ > drivers/gpu/drm/armada/armada_debugfs.c | 1 + > drivers/gpu/drm/bridge/ite-it6505.c | 1 + > drivers/gpu/drm/bridge/panel.c | 2 ++ > drivers/gpu/drm/drm_print.c | 6 +++--- > drivers/gpu/drm/i915/display/intel_dmc.c| 1 + > drivers/gpu/drm/imagination/pvr_fw_trace.c | 1 + > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 2 ++ Acked-by: Dmitry Baryshkov # drm/msm > drivers/gpu/drm/nouveau/dispnv50/crc.c | 2 ++ > drivers/gpu/drm/radeon/r100.c | 1 + > drivers/gpu/drm/radeon/r300.c | 1 + > drivers/gpu/drm/radeon/r420.c | 1 + > drivers/gpu/drm/radeon/r600.c | 3 ++- > drivers/gpu/drm/radeon/radeon_fence.c | 1 + > drivers/gpu/drm/radeon/radeon_gem.c | 1 + > drivers/gpu/drm/radeon/radeon_ib.c | 2 ++ > drivers/gpu/drm/radeon/radeon_pm.c | 1 + > drivers/gpu/drm/radeon/radeon_ring.c| 2 ++ > drivers/gpu/drm/radeon/radeon_ttm.c | 1 + > drivers/gpu/drm/radeon/rs400.c | 1 + > drivers/gpu/drm/radeon/rv515.c | 1 + > drivers/gpu/drm/sti/sti_drv.c | 1 + > drivers/gpu/drm/ttm/ttm_device.c| 1 + > drivers/gpu/drm/ttm/ttm_resource.c | 3 ++- > drivers/gpu/drm/ttm/ttm_tt.c| 5 +++-- > drivers/gpu/drm/vc4/vc4_drv.h | 1 + > drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 2 ++ > drivers/gpu/drm/xe/xe_debugfs.c | 1 + > drivers/gpu/drm/xe/xe_gt_debugfs.c | 2 ++ > drivers/gpu/drm/xe/xe_uc_debugfs.c | 2 ++ > include/drm/drm_print.h | 2 +- > 31 files changed, 46 insertions(+), 8 deletions(-) -- With best wishes Dmitry
Re: [PATCH 1/5] drm/vblank: Introduce drm_crtc_vblank_crtc()
On Mon, Apr 08, 2024 at 10:06:07PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > Make life easier by providing a function that hands > out the the correct drm_vblank_crtc for a given a drm_crtc. > > Also abstract the lower level internals of the vblank > code in a similar fashion. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/drm_vblank.c | 58 ++- > drivers/gpu/drm/drm_vblank_work.c | 2 +- > include/drm/drm_vblank.h | 1 + > 3 files changed, 36 insertions(+), 25 deletions(-) Reviewed-by: Dmitry Baryshkov I'm looking forward to using this in drm/msm > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > index 702a12bc93bd..cc3571e25a9a 100644 > --- a/drivers/gpu/drm/drm_vblank.c > +++ b/drivers/gpu/drm/drm_vblank.c > @@ -166,11 +166,24 @@ module_param_named(timestamp_precision_usec, > drm_timestamp_precision, int, 0600) > MODULE_PARM_DESC(vblankoffdelay, "Delay until vblank irq auto-disable > [msecs] (0: never disable, <0: disable immediately)"); > MODULE_PARM_DESC(timestamp_precision_usec, "Max. error on timestamps > [usecs]"); > > +static struct drm_vblank_crtc * > +drm_vblank_crtc(struct drm_device *dev, unsigned int pipe) > +{ > + return >vblank[pipe]; > +} > + > +struct drm_vblank_crtc * > +drm_crtc_vblank_crtc(struct drm_crtc *crtc) > +{ > + return drm_vblank_crtc(crtc->dev, drm_crtc_index(crtc)); > +} > +EXPORT_SYMBOL(drm_crtc_vblank_crtc); > + > static void store_vblank(struct drm_device *dev, unsigned int pipe, >u32 vblank_count_inc, >ktime_t t_vblank, u32 last) > { > - struct drm_vblank_crtc *vblank = >vblank[pipe]; > + struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, pipe); > > assert_spin_locked(>vblank_time_lock); > > @@ -184,7 +197,7 @@ static void store_vblank(struct drm_device *dev, unsigned > int pipe, > > static u32 drm_max_vblank_count(struct drm_device *dev, unsigned int pipe) > { > - struct drm_vblank_crtc *vblank = >vblank[pipe]; > + struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, pipe); > > return vblank->max_vblank_count ?: dev->max_vblank_count; > } > @@ -273,7 +286,7 @@ static void drm_reset_vblank_timestamp(struct drm_device > *dev, unsigned int pipe > static void drm_update_vblank_count(struct drm_device *dev, unsigned int > pipe, > bool in_vblank_irq) > { > - struct drm_vblank_crtc *vblank = >vblank[pipe]; > + struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, pipe); > u32 cur_vblank, diff; > bool rc; > ktime_t t_vblank; > @@ -364,7 +377,7 @@ static void drm_update_vblank_count(struct drm_device > *dev, unsigned int pipe, > > u64 drm_vblank_count(struct drm_device *dev, unsigned int pipe) > { > - struct drm_vblank_crtc *vblank = >vblank[pipe]; > + struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, pipe); > u64 count; > > if (drm_WARN_ON(dev, pipe >= dev->num_crtcs)) > @@ -438,7 +451,7 @@ static void __disable_vblank(struct drm_device *dev, > unsigned int pipe) > */ > void drm_vblank_disable_and_save(struct drm_device *dev, unsigned int pipe) > { > - struct drm_vblank_crtc *vblank = >vblank[pipe]; > + struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, pipe); > unsigned long irqflags; > > assert_spin_locked(>vbl_lock); > @@ -600,7 +613,7 @@ void drm_calc_timestamping_constants(struct drm_crtc > *crtc, > { > struct drm_device *dev = crtc->dev; > unsigned int pipe = drm_crtc_index(crtc); > - struct drm_vblank_crtc *vblank = >vblank[pipe]; > + struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc); > int linedur_ns = 0, framedur_ns = 0; > int dotclock = mode->crtc_clock; > > @@ -930,7 +943,7 @@ EXPORT_SYMBOL(drm_crtc_vblank_count); > static u64 drm_vblank_count_and_time(struct drm_device *dev, unsigned int > pipe, >ktime_t *vblanktime) > { > - struct drm_vblank_crtc *vblank = >vblank[pipe]; > + struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, pipe); > u64 vblank_count; > unsigned int seq; > > @@ -985,7 +998,6 @@ EXPORT_SYMBOL(drm_crtc_vblank_count_and_time); > */ > int drm_crtc_next_vblank_start(struct drm_crtc *crtc, ktime_t *vblanktime) > { > - unsigned int pipe = drm_crtc_index(crtc); > struct drm_vblank_crtc *vblank; > struct drm_display_mode *mode; > u64 vblank_start; > @@ -993,7 +10
Re: [PATCH 01/12] drm/client: Fully protect modes[] with dev->mode_config.mutex
On Fri, 5 Apr 2024 at 22:17, Ville Syrjälä wrote: > > On Fri, Apr 05, 2024 at 06:24:01AM +0300, Dmitry Baryshkov wrote: > > On Thu, Apr 04, 2024 at 11:33:25PM +0300, Ville Syrjala wrote: > > > From: Ville Syrjälä > > > > > > The modes[] array contains pointers to modes on the connectors' > > > mode lists, which are protected by dev->mode_config.mutex. > > > Thus we need to extend modes[] the same protection or by the > > > time we use it the elements may already be pointing to > > > freed/reused memory. > > > > > > Cc: sta...@vger.kernel.org > > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10583 > > > Signed-off-by: Ville Syrjälä > > > > Reviewed-by: Dmitry Baryshkov > > > > I tried looking for the proper Fixes tag, but it looks like it might be > > something like 386516744ba4 ("drm/fb: fix fbdev object model + cleanup > > properly.") > > The history is rather messy. I think it was originally completely > lockless and broken, and got fixed piecemeal later in these: > commit 7394371d8569 ("drm: Take lock around probes for > drm_fb_helper_hotplug_event") > commit 966a6a13c666 ("drm: Hold mode_config.lock to prevent hotplug whilst > setting up crtcs") > > commit e13a05831050 ("drm/fb-helper: Stop using mode_config.mutex for > internals") > looks to me like where the race might have been re-introduced. > But didn't do a thorough analysis so not 100% sure. It's all > rather ancient history by now so a Fixes tag doesn't seem all > that useful anyway. Well, you have added stable to cc list, so you expect to have this patch backported. Then it should either have a kernel version as a 'starting' point or a Fixes tag to assist the sable team. -- With best wishes Dmitry
Re: [PATCH] drm/dp: correct struct member name in documentation
On Fri, Apr 05, 2024 at 12:21:59PM +0530, Mitul Golani wrote: > Correct struct member name to 'mode' instead of 'operation mode' > in 'drm_dp_as_sdp' structure description. > > Fixes: 0bbb8f594e33 ("drm/dp: Add Adaptive Sync SDP logging") > Cc: Mitul Golani > Cc: Ankit Nautiyal > Cc: Jani Nikula > Signed-off-by: Mitul Golani > --- > include/drm/display/drm_dp_helper.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 05/12] drm/client: Nuke outdated fastboot comment
On Thu, Apr 04, 2024 at 11:33:29PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > Remove the tall tale about fastboot vs. user mode vs. > adjusted mode. crtc->mode == crtc->state->mode, so none > of this makes any sense. I suppose it may have been true > long ago in the past. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/drm_client_modeset.c | 10 -- > 1 file changed, 10 deletions(-) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 04/12] drm/client: Add a FIXME around crtc->mode usage
On Thu, Apr 04, 2024 at 11:33:28PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > crtc->mode is legacy junk and shouldn't really be used with > atomic drivers. > > Most (all?) atomic drivers do end up still calling > drm_atomic_helper_update_legacy_modeset_state() at some > point, so crtc->mode does still get populated, and this > does work for now. But eventually would be nice to eliminate > all the legacy stuff from atomic drivers. > > Switching to crtc->state->mode would require some bigger > changes however, as we currently drop the crtc->mutex > before we're done using the mode. So leave the junk in > for now and just add a FIXME to remind us that this > needs fixing. What about using allocated duplicate modes to fill modes[] array? This requires additional allocations, but it will solve most if not all modes lifetime issues. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/drm_client_modeset.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/drm_client_modeset.c > b/drivers/gpu/drm/drm_client_modeset.c > index 2b7d0be04911..8ef03608b424 100644 > --- a/drivers/gpu/drm/drm_client_modeset.c > +++ b/drivers/gpu/drm/drm_client_modeset.c > @@ -699,6 +699,10 @@ static bool drm_client_firmware_config(struct > drm_client_dev *client, >* >* This is crtc->mode and not crtc->state->mode for the >* fastboot check to work correctly. > + * > + * FIXME using legacy crtc->mode with atomic drivers > + * is dodgy. Switch to crtc->state->mode, after taking > + * care of the resulting locking/lifetime issues. >*/ > DRM_DEBUG_KMS("looking for current mode on connector > %s\n", > connector->name); > -- > 2.43.2 > -- With best wishes Dmitry
Re: [PATCH 03/12] drm/client: Use drm_mode_destroy()
On Thu, Apr 04, 2024 at 11:33:27PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > Prefer drm_mode_destroy() over bare kfree(), for consistency > and setting a good example. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/drm_client_modeset.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 02/12] drm/client: s/drm_connector_has_preferred_mode/drm_connector_preferred_mode/
On Thu, Apr 04, 2024 at 11:33:26PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > Drop the "has" from drm_connector_has_preferred_mode() to better > describe what it does. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/drm_client_modeset.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 01/12] drm/client: Fully protect modes[] with dev->mode_config.mutex
On Thu, Apr 04, 2024 at 11:33:25PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > The modes[] array contains pointers to modes on the connectors' > mode lists, which are protected by dev->mode_config.mutex. > Thus we need to extend modes[] the same protection or by the > time we use it the elements may already be pointing to > freed/reused memory. > > Cc: sta...@vger.kernel.org > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10583 > Signed-off-by: Ville Syrjälä Reviewed-by: Dmitry Baryshkov I tried looking for the proper Fixes tag, but it looks like it might be something like 386516744ba4 ("drm/fb: fix fbdev object model + cleanup properly.") > --- > drivers/gpu/drm/drm_client_modeset.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_client_modeset.c > b/drivers/gpu/drm/drm_client_modeset.c > index 871e4e2129d6..0683a129b362 100644 > --- a/drivers/gpu/drm/drm_client_modeset.c > +++ b/drivers/gpu/drm/drm_client_modeset.c > @@ -777,6 +777,7 @@ int drm_client_modeset_probe(struct drm_client_dev > *client, unsigned int width, > unsigned int total_modes_count = 0; > struct drm_client_offset *offsets; > unsigned int connector_count = 0; > + /* points to modes protected by mode_config.mutex */ > struct drm_display_mode **modes; > struct drm_crtc **crtcs; > int i, ret = 0; > @@ -845,7 +846,6 @@ int drm_client_modeset_probe(struct drm_client_dev > *client, unsigned int width, > drm_client_pick_crtcs(client, connectors, connector_count, > crtcs, modes, 0, width, height); > } > - mutex_unlock(>mode_config.mutex); > > drm_client_modeset_release(client); > > @@ -875,6 +875,7 @@ int drm_client_modeset_probe(struct drm_client_dev > *client, unsigned int width, > modeset->y = offset->y; > } > } > + mutex_unlock(>mode_config.mutex); > > mutex_unlock(>modeset_mutex); > out: > -- > 2.43.2 > -- With best wishes Dmitry
Re: [PATCH 7/8] drm/bridge: lt9611uxc: use int for holding number of modes
On Fri, 8 Mar 2024 at 18:04, Jani Nikula wrote: > > lt9611uxc_connector_get_modes() propagates the return value of > drm_edid_connector_add_modes() but stores the int temporarily in an > unsigned int. Use the correct type. > > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Robert Foss > Signed-off-by: Jani Nikula > --- > drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
DisplayPort: handling of HPD events / link training
Hello, We are currently looking at checking and/or possibly redesigning the way the MSM DRM driver handles the HPD events and link training. After a quick glance at the drivers implementing DP support, I noticed following main approaches: - Perform link training at the atomic_enable time, don't report failures (mtk, analogix, zynqmp, tegra, nouveau) - Perform link training at the atomic_enable time, report errors using link_status property (i915, mhdp8546) - Perform link training on the plug event (msm, it8605). - Perform link training from the DPMS handler, also calling it from the enable callback (AMDGPU, radeon). It looks like the majority wins and we should move HPD to atomic_enable time. Is that assumption correct? Also two related questions: - Is there a plan to actually make use of the link_status property? Intel presented it at FOSDEM 2018, but since that time it was not picked up by other drivers. - Is there any plan to create generic DP link training helpers? After glancing through the DP drivers there is a lot of similar code in the link training functions, with minor differences here and there. And it's those minor differences that bug me. It means that drivers might respond differently to similar devices. Or that there might be minor bugs here and there. -- With best wishes Dmitry
Re: [PATCH v3 1/3] bits: introduce fixed-type genmasks
On Wed, 21 Feb 2024 at 22:30, Dmitry Baryshkov wrote: > > On Thu, 8 Feb 2024 at 09:45, Lucas De Marchi wrote: > > > > From: Yury Norov > > > > Generalize __GENMASK() to support different types, and implement > > fixed-types versions of GENMASK() based on it. The fixed-type version > > allows more strict checks to the min/max values accepted, which is > > useful for defining registers like implemented by i915 and xe drivers > > with their REG_GENMASK*() macros. > > > > The strict checks rely on shift-count-overflow compiler check to > > fail the build if a number outside of the range allowed is passed. > > Example: > > > > #define FOO_MASK GENMASK_U32(33, 4) > > > > will generate a warning like: > > > > ../include/linux/bits.h:41:31: error: left shift count >= width of > > type [-Werror=shift-count-overflow] > >41 | (((t)~0ULL - ((t)(1) << (l)) + 1) & \ > > | ^~ > > > > Signed-off-by: Yury Norov > > Signed-off-by: Lucas De Marchi > > Acked-by: Jani Nikula > > --- > > include/linux/bitops.h | 1 - > > include/linux/bits.h | 32 ++-- > > 2 files changed, 22 insertions(+), 11 deletions(-) > > > > diff --git a/include/linux/bitops.h b/include/linux/bitops.h > > index 2ba557e067fe..1db50c69cfdb 100644 > > --- a/include/linux/bitops.h > > +++ b/include/linux/bitops.h > > @@ -15,7 +15,6 @@ > > # define aligned_byte_mask(n) (~0xffUL << (BITS_PER_LONG - 8 - 8*(n))) > > #endif > > > > -#define BITS_PER_TYPE(type)(sizeof(type) * BITS_PER_BYTE) > > #define BITS_TO_LONGS(nr) __KERNEL_DIV_ROUND_UP(nr, > > BITS_PER_TYPE(long)) > > #define BITS_TO_U64(nr)__KERNEL_DIV_ROUND_UP(nr, > > BITS_PER_TYPE(u64)) > > #define BITS_TO_U32(nr)__KERNEL_DIV_ROUND_UP(nr, > > BITS_PER_TYPE(u32)) > > diff --git a/include/linux/bits.h b/include/linux/bits.h > > index 7c0cf5031abe..bd56f32de44e 100644 > > --- a/include/linux/bits.h > > +++ b/include/linux/bits.h > > @@ -6,6 +6,8 @@ > > #include > > #include > > > > +#define BITS_PER_TYPE(type)(sizeof(type) * BITS_PER_BYTE) > > + > > #define BIT_MASK(nr) (UL(1) << ((nr) % BITS_PER_LONG)) > > #define BIT_WORD(nr) ((nr) / BITS_PER_LONG) > > #define BIT_ULL_MASK(nr) (ULL(1) << ((nr) % BITS_PER_LONG_LONG)) > > @@ -30,16 +32,26 @@ > > #define GENMASK_INPUT_CHECK(h, l) 0 > > #endif > > > > -#define __GENMASK(h, l) \ > > - (((~UL(0)) - (UL(1) << (l)) + 1) & \ > > -(~UL(0) >> (BITS_PER_LONG - 1 - (h > > -#define GENMASK(h, l) \ > > - (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l)) > > +/* > > + * Generate a mask for the specified type @t. Additional checks are made to > > + * guarantee the value returned fits in that type, relying on > > + * shift-count-overflow compiler check to detect incompatible arguments. > > + * For example, all these create build errors or warnings: > > + * > > + * - GENMASK(15, 20): wrong argument order > > + * - GENMASK(72, 15): doesn't fit unsigned long > > + * - GENMASK_U32(33, 15): doesn't fit in a u32 > > + */ > > +#define __GENMASK(t, h, l) \ > > + (GENMASK_INPUT_CHECK(h, l) + \ > > +(((t)~0ULL - ((t)(1) << (l)) + 1) & \ > > +((t)~0ULL >> (BITS_PER_TYPE(t) - 1 - (h) > > > > -#define __GENMASK_ULL(h, l) \ > > - (((~ULL(0)) - (ULL(1) << (l)) + 1) & \ > > -(~ULL(0) >> (BITS_PER_LONG_LONG - 1 - (h > > -#define GENMASK_ULL(h, l) \ > > - (GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l)) > > +#define GENMASK(h, l) __GENMASK(unsigned long, h, l) > > +#define GENMASK_ULL(h, l) __GENMASK(unsigned long long, h, l) > > +#define GENMASK_U8(h, l) __GENMASK(u8, h, l) > > +#define GENMASK_U16(h, l) __GENMASK(u16, h, l) > > +#define GENMASK_U32(h, l) __GENMASK(u32, h, l) > > +#define GENMASK_U64(h, l) __GENMASK(u64, h, l) > > This breaks drm-tip on arm64 architecture: > Excuse me, it seems a c from gitlab didn't work as expected. arch/arm64/kernel/entry-fpsimd.S: Assembler messages: arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')' arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')' arch/arm64/kernel/entry-fpsimd.S:66: Info: m
Re: [PATCH v3 1/3] bits: introduce fixed-type genmasks
On Thu, 8 Feb 2024 at 09:45, Lucas De Marchi wrote: > > From: Yury Norov > > Generalize __GENMASK() to support different types, and implement > fixed-types versions of GENMASK() based on it. The fixed-type version > allows more strict checks to the min/max values accepted, which is > useful for defining registers like implemented by i915 and xe drivers > with their REG_GENMASK*() macros. > > The strict checks rely on shift-count-overflow compiler check to > fail the build if a number outside of the range allowed is passed. > Example: > > #define FOO_MASK GENMASK_U32(33, 4) > > will generate a warning like: > > ../include/linux/bits.h:41:31: error: left shift count >= width of > type [-Werror=shift-count-overflow] >41 | (((t)~0ULL - ((t)(1) << (l)) + 1) & \ > | ^~ > > Signed-off-by: Yury Norov > Signed-off-by: Lucas De Marchi > Acked-by: Jani Nikula > --- > include/linux/bitops.h | 1 - > include/linux/bits.h | 32 ++-- > 2 files changed, 22 insertions(+), 11 deletions(-) > > diff --git a/include/linux/bitops.h b/include/linux/bitops.h > index 2ba557e067fe..1db50c69cfdb 100644 > --- a/include/linux/bitops.h > +++ b/include/linux/bitops.h > @@ -15,7 +15,6 @@ > # define aligned_byte_mask(n) (~0xffUL << (BITS_PER_LONG - 8 - 8*(n))) > #endif > > -#define BITS_PER_TYPE(type)(sizeof(type) * BITS_PER_BYTE) > #define BITS_TO_LONGS(nr) __KERNEL_DIV_ROUND_UP(nr, BITS_PER_TYPE(long)) > #define BITS_TO_U64(nr)__KERNEL_DIV_ROUND_UP(nr, > BITS_PER_TYPE(u64)) > #define BITS_TO_U32(nr)__KERNEL_DIV_ROUND_UP(nr, > BITS_PER_TYPE(u32)) > diff --git a/include/linux/bits.h b/include/linux/bits.h > index 7c0cf5031abe..bd56f32de44e 100644 > --- a/include/linux/bits.h > +++ b/include/linux/bits.h > @@ -6,6 +6,8 @@ > #include > #include > > +#define BITS_PER_TYPE(type)(sizeof(type) * BITS_PER_BYTE) > + > #define BIT_MASK(nr) (UL(1) << ((nr) % BITS_PER_LONG)) > #define BIT_WORD(nr) ((nr) / BITS_PER_LONG) > #define BIT_ULL_MASK(nr) (ULL(1) << ((nr) % BITS_PER_LONG_LONG)) > @@ -30,16 +32,26 @@ > #define GENMASK_INPUT_CHECK(h, l) 0 > #endif > > -#define __GENMASK(h, l) \ > - (((~UL(0)) - (UL(1) << (l)) + 1) & \ > -(~UL(0) >> (BITS_PER_LONG - 1 - (h > -#define GENMASK(h, l) \ > - (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l)) > +/* > + * Generate a mask for the specified type @t. Additional checks are made to > + * guarantee the value returned fits in that type, relying on > + * shift-count-overflow compiler check to detect incompatible arguments. > + * For example, all these create build errors or warnings: > + * > + * - GENMASK(15, 20): wrong argument order > + * - GENMASK(72, 15): doesn't fit unsigned long > + * - GENMASK_U32(33, 15): doesn't fit in a u32 > + */ > +#define __GENMASK(t, h, l) \ > + (GENMASK_INPUT_CHECK(h, l) + \ > +(((t)~0ULL - ((t)(1) << (l)) + 1) & \ > +((t)~0ULL >> (BITS_PER_TYPE(t) - 1 - (h) > > -#define __GENMASK_ULL(h, l) \ > - (((~ULL(0)) - (ULL(1) << (l)) + 1) & \ > -(~ULL(0) >> (BITS_PER_LONG_LONG - 1 - (h > -#define GENMASK_ULL(h, l) \ > - (GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l)) > +#define GENMASK(h, l) __GENMASK(unsigned long, h, l) > +#define GENMASK_ULL(h, l) __GENMASK(unsigned long long, h, l) > +#define GENMASK_U8(h, l) __GENMASK(u8, h, l) > +#define GENMASK_U16(h, l) __GENMASK(u16, h, l) > +#define GENMASK_U32(h, l) __GENMASK(u32, h, l) > +#define GENMASK_U64(h, l) __GENMASK(u64, h, l) This breaks drm-tip on arm64 architecture: arch/arm64/kernel/entry-fpsimd.S: Assembler messages: 465arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')' 466arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here 467arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')' 468arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here 469arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')' 470arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here 471arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')' 472arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here 473arch/arm64/kernel/entry-fpsimd.S:271: Error: unexpected characters following instruction at operand 3 -- `bic x2,x1,(0+(((unsigned long)~0ULL-((unsigned long)(1)<<(0))+1)&((unsigned long)~0ULL>>((sizeof(unsigned long)*8)-1-(3)' 474arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here 475arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', expected: ')' 476arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here 477arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', expected: ')' 478arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here 479arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l',
Re: [PATCH v3 2/2] drm/dp: drop the size parameter from drm_dp_vsc_sdp_pack()
On Tue, 20 Feb 2024 at 21:54, Abhinav Kumar wrote: > > Currently the size parameter of drm_dp_vsc_sdp_pack() is always > the size of struct dp_sdp. Hence lets drop this parameter and > use sizeof() directly. > > Suggested-by: Dmitry Baryshkov > Signed-off-by: Abhinav Kumar > --- > drivers/gpu/drm/display/drm_dp_helper.c | 8 ++-- > drivers/gpu/drm/i915/display/intel_dp.c | 3 +-- > include/drm/display/drm_dp_helper.h | 3 +-- > 3 files changed, 4 insertions(+), 10 deletions(-) Thank you! Acked-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v2] drm/dp: move intel_dp_vsc_sdp_pack() to generic helper
On Tue, 20 Feb 2024 at 21:05, Dmitry Baryshkov wrote: > > On Tue, 20 Feb 2024 at 20:53, Abhinav Kumar wrote: > > > > > > > > On 2/20/2024 10:49 AM, Dmitry Baryshkov wrote: > > > On Thu, 15 Feb 2024 at 21:08, Abhinav Kumar > > > wrote: > > >> > > >> intel_dp_vsc_sdp_pack() can be re-used by other DRM drivers as well. > > >> Lets move this to drm_dp_helper to achieve this. > > >> > > >> changes in v2: > > >> - rebased on top of drm-tip > > >> > > >> Acked-by: Dmitry Baryshkov > > > > > > v1 had an explicit comment before the ack: > > > > > > > Yes, I remember the comment. I did not make any changes to v2 other than > > just rebasing it on drm-tip to get the ack from i915 folks. > > > > >> From my side, with the promise of the size fixup. > > > > > > However I observe neither a second patch removing the size argument > > > nor it being dropped as a part of this patch. > > > > > > > Yes, now that in v2 we got the ack for this patch, I can spin a v3 with > > the addition of the next patch to remove the size OR as another series > > so as to not block the main series which needs this patch. > > > > I would prefer the latter. > > It doesn't work this way. The comment should have been fixed for v2. This probably deserves some explanation. Currently there is only one user of this function. So it is easy to fix it. Once there are several users, you have to fix all of them at the same time, patching different drm subtrees. That complicates the life of maintainers. -- With best wishes Dmitry
Re: [PATCH v2] drm/dp: move intel_dp_vsc_sdp_pack() to generic helper
On Tue, 20 Feb 2024 at 21:09, Abhinav Kumar wrote: > > > > On 2/20/2024 11:05 AM, Dmitry Baryshkov wrote: > > On Tue, 20 Feb 2024 at 20:53, Abhinav Kumar > > wrote: > >> > >> > >> > >> On 2/20/2024 10:49 AM, Dmitry Baryshkov wrote: > >>> On Thu, 15 Feb 2024 at 21:08, Abhinav Kumar > >>> wrote: > >>>> > >>>> intel_dp_vsc_sdp_pack() can be re-used by other DRM drivers as well. > >>>> Lets move this to drm_dp_helper to achieve this. > >>>> > >>>> changes in v2: > >>>> - rebased on top of drm-tip > >>>> > >>>> Acked-by: Dmitry Baryshkov > >>> > >>> v1 had an explicit comment before the ack: > >>> > >> > >> Yes, I remember the comment. I did not make any changes to v2 other than > >> just rebasing it on drm-tip to get the ack from i915 folks. > >> > >>>> From my side, with the promise of the size fixup. > >>> > >>> However I observe neither a second patch removing the size argument > >>> nor it being dropped as a part of this patch. > >>> > >> > >> Yes, now that in v2 we got the ack for this patch, I can spin a v3 with > >> the addition of the next patch to remove the size OR as another series > >> so as to not block the main series which needs this patch. > >> > >> I would prefer the latter. > > > > It doesn't work this way. The comment should have been fixed for v2. > > > > Ack, I can post a v3 now by adding the removal in patch 2 of this series. Yes, please. -- With best wishes Dmitry
Re: [PATCH v2] drm/dp: move intel_dp_vsc_sdp_pack() to generic helper
On Tue, 20 Feb 2024 at 20:53, Abhinav Kumar wrote: > > > > On 2/20/2024 10:49 AM, Dmitry Baryshkov wrote: > > On Thu, 15 Feb 2024 at 21:08, Abhinav Kumar > > wrote: > >> > >> intel_dp_vsc_sdp_pack() can be re-used by other DRM drivers as well. > >> Lets move this to drm_dp_helper to achieve this. > >> > >> changes in v2: > >> - rebased on top of drm-tip > >> > >> Acked-by: Dmitry Baryshkov > > > > v1 had an explicit comment before the ack: > > > > Yes, I remember the comment. I did not make any changes to v2 other than > just rebasing it on drm-tip to get the ack from i915 folks. > > >> From my side, with the promise of the size fixup. > > > > However I observe neither a second patch removing the size argument > > nor it being dropped as a part of this patch. > > > > Yes, now that in v2 we got the ack for this patch, I can spin a v3 with > the addition of the next patch to remove the size OR as another series > so as to not block the main series which needs this patch. > > I would prefer the latter. It doesn't work this way. The comment should have been fixed for v2. -- With best wishes Dmitry
Re: [PATCH v2] drm/dp: move intel_dp_vsc_sdp_pack() to generic helper
On Thu, 15 Feb 2024 at 21:08, Abhinav Kumar wrote: > > intel_dp_vsc_sdp_pack() can be re-used by other DRM drivers as well. > Lets move this to drm_dp_helper to achieve this. > > changes in v2: > - rebased on top of drm-tip > > Acked-by: Dmitry Baryshkov v1 had an explicit comment before the ack: >From my side, with the promise of the size fixup. However I observe neither a second patch removing the size argument nor it being dropped as a part of this patch. > Signed-off-by: Abhinav Kumar > --- > drivers/gpu/drm/display/drm_dp_helper.c | 78 + > drivers/gpu/drm/i915/display/intel_dp.c | 71 +- > include/drm/display/drm_dp_helper.h | 3 + > 3 files changed, 83 insertions(+), 69 deletions(-) > > diff --git a/drivers/gpu/drm/display/drm_dp_helper.c > b/drivers/gpu/drm/display/drm_dp_helper.c > index 8d6ce46471ae..6c91f400ecb1 100644 > --- a/drivers/gpu/drm/display/drm_dp_helper.c > +++ b/drivers/gpu/drm/display/drm_dp_helper.c > @@ -2913,6 +2913,84 @@ void drm_dp_vsc_sdp_log(struct drm_printer *p, const > struct drm_dp_vsc_sdp *vsc) > } > EXPORT_SYMBOL(drm_dp_vsc_sdp_log); > > +/** > + * drm_dp_vsc_sdp_pack() - pack a given vsc sdp into generic dp_sdp > + * @vsc: vsc sdp initialized according to its purpose as defined in > + * table 2-118 - table 2-120 in DP 1.4a specification > + * @sdp: valid handle to the generic dp_sdp which will be packed > + * @size: valid size of the passed sdp handle > + * > + * Returns length of sdp on success and error code on failure > + */ > +ssize_t drm_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc, > + struct dp_sdp *sdp, size_t size) > +{ > + size_t length = sizeof(struct dp_sdp); > + > + if (size < length) > + return -ENOSPC; > + > + memset(sdp, 0, size); > + > + /* > +* Prepare VSC Header for SU as per DP 1.4a spec, Table 2-119 > +* VSC SDP Header Bytes > +*/ > + sdp->sdp_header.HB0 = 0; /* Secondary-Data Packet ID = 0 */ > + sdp->sdp_header.HB1 = vsc->sdp_type; /* Secondary-data Packet Type */ > + sdp->sdp_header.HB2 = vsc->revision; /* Revision Number */ > + sdp->sdp_header.HB3 = vsc->length; /* Number of Valid Data Bytes */ > + > + if (vsc->revision == 0x6) { > + sdp->db[0] = 1; > + sdp->db[3] = 1; > + } > + > + /* > +* Revision 0x5 and revision 0x7 supports Pixel Encoding/Colorimetry > +* Format as per DP 1.4a spec and DP 2.0 respectively. > +*/ > + if (!(vsc->revision == 0x5 || vsc->revision == 0x7)) > + goto out; > + > + /* VSC SDP Payload for DB16 through DB18 */ > + /* Pixel Encoding and Colorimetry Formats */ > + sdp->db[16] = (vsc->pixelformat & 0xf) << 4; /* DB16[7:4] */ > + sdp->db[16] |= vsc->colorimetry & 0xf; /* DB16[3:0] */ > + > + switch (vsc->bpc) { > + case 6: > + /* 6bpc: 0x0 */ > + break; > + case 8: > + sdp->db[17] = 0x1; /* DB17[3:0] */ > + break; > + case 10: > + sdp->db[17] = 0x2; > + break; > + case 12: > + sdp->db[17] = 0x3; > + break; > + case 16: > + sdp->db[17] = 0x4; > + break; > + default: > + WARN(1, "Missing case %d\n", vsc->bpc); > + return -EINVAL; > + } > + > + /* Dynamic Range and Component Bit Depth */ > + if (vsc->dynamic_range == DP_DYNAMIC_RANGE_CTA) > + sdp->db[17] |= 0x80; /* DB17[7] */ > + > + /* Content Type */ > + sdp->db[18] = vsc->content_type & 0x7; > + > +out: > + return length; > +} > +EXPORT_SYMBOL(drm_dp_vsc_sdp_pack); > + > /** > * drm_dp_get_pcon_max_frl_bw() - maximum frl supported by PCON > * @dpcd: DisplayPort configuration data > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index 217196196e50..a9458df475e2 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -4089,73 +4089,6 @@ intel_dp_needs_vsc_sdp(const struct intel_crtc_state > *crtc_state, > return false; > } > > -static ssize_t intel_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc, > -struct dp_sdp *sdp, size_t size) > -{ > - size_t length = sizeof(struct dp_sdp);
Re: [PATCH] drm/dp: move intel_dp_vsc_sdp_pack() to generic helper
On Wed, 14 Feb 2024 at 20:08, Abhinav Kumar wrote: > > > > On 2/14/2024 10:02 AM, Ville Syrjälä wrote: > > On Wed, Feb 14, 2024 at 09:17:34AM -0800, Abhinav Kumar wrote: > >> > >> > >> On 2/14/2024 12:15 AM, Dmitry Baryshkov wrote: > >>> On Wed, 14 Feb 2024 at 01:45, Abhinav Kumar > >>> wrote: > >>>> > >>>> intel_dp_vsc_sdp_pack() can be re-used by other DRM drivers as well. > >>>> Lets move this to drm_dp_helper to achieve this. > >>>> > >>>> Signed-off-by: Abhinav Kumar > >>> > >>> My preference would be to have packing functions in > >>> drivers/video/hdmi.c, as we already have > >>> hdmi_audio_infoframe_pack_for_dp() there. > >>> > >> > >> My preference is drm_dp_helper because it already has some VSC SDP stuff > >> and after discussion with Ville on IRC, I decided to post it this way. > >> > >> hdmi_audio_infoframe_pack_for_dp() is an exception from my PoV as the > >> hdmi audio infoframe fields were re-used and packed into a DP SDP > >> thereby re-using the existing struct hdmi_audio_infoframe . > >> > >> This is not like that. Here we pack from struct drm_dp_vsc_sdp to struct > >> dp_sdp both of which had prior usages already in this file. > >> > >> So it all adds up and makes sense to me to be in this file. > >> > >> I will let the other DRM core maintainers comment on this. > >> > >> Ville, Jani? > > > > Yeah, I'm not sure bloating the (poorly named) hdmi.c with all > > SDP stuff is a great idea. Since other related stuff already > > lives in the drm_dp_helper.c that seems reasonable to me at this > > time. And if we get a decent amount of this then probably all > > DP SDP stuff should be extracted into its own file. > > > > Yes, thanks. > > > There are of course a few overlaps here andthere (the audio SDP > > I guess, and the CTA infoframe SDP). But I'm not sure that actually > > needs any SDP specific stuff in hdmi.c, or could we just let hdmi.c > > deal with the actual CTA-861 stuff and then have the DP SDP code > > wrap that up in its own thing externally? Dunno, haven't really > > looked at the details. > > > > Thats a good way to look at it. this packing is from DP spec and not CTA > so makes more sense to be in this file. > > In that case, R-b? > > >> > >>>> --- > >>>>drivers/gpu/drm/display/drm_dp_helper.c | 78 + > >>>>drivers/gpu/drm/i915/display/intel_dp.c | 73 +-- > >>>>include/drm/display/drm_dp_helper.h | 3 + > >>>>3 files changed, 84 insertions(+), 70 deletions(-) > >>>> > >>>> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c > >>>> b/drivers/gpu/drm/display/drm_dp_helper.c > >>>> index b1ca3a1100da..066cfbbf7a91 100644 > >>>> --- a/drivers/gpu/drm/display/drm_dp_helper.c > >>>> +++ b/drivers/gpu/drm/display/drm_dp_helper.c > >>>> @@ -2916,6 +2916,84 @@ void drm_dp_vsc_sdp_log(const char *level, struct > >>>> device *dev, > >>>>} > >>>>EXPORT_SYMBOL(drm_dp_vsc_sdp_log); > >>>> > >>>> +/** > >>>> + * drm_dp_vsc_sdp_pack() - pack a given vsc sdp into generic dp_sdp > >>>> + * @vsc: vsc sdp initialized according to its purpose as defined in > >>>> + * table 2-118 - table 2-120 in DP 1.4a specification > >>>> + * @sdp: valid handle to the generic dp_sdp which will be packed > >>>> + * @size: valid size of the passed sdp handle > >>>> + * > >>>> + * Returns length of sdp on success and error code on failure > >>>> + */ > >>>> +ssize_t drm_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc, > >>>> + struct dp_sdp *sdp, size_t size) > >>> > >>> I know that you are just moving the function. Maybe there can be > >>> patch#2, which drops the size argument? The struct dp_sdp already has > >>> a defined size. The i915 driver just passes sizeof(sdp), which is more > >>> or less useless. > >>> > >> > >> Yes this is a valid point, I also noticed this. I can post it on top of > >> this once we get an agreement and ack on this patch first. > >> >From my side, with the promise of the size fixup. Acked-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH] drm/dp: move intel_dp_vsc_sdp_pack() to generic helper
On Wed, 14 Feb 2024 at 01:45, Abhinav Kumar wrote: > > intel_dp_vsc_sdp_pack() can be re-used by other DRM drivers as well. > Lets move this to drm_dp_helper to achieve this. > > Signed-off-by: Abhinav Kumar My preference would be to have packing functions in drivers/video/hdmi.c, as we already have hdmi_audio_infoframe_pack_for_dp() there. > --- > drivers/gpu/drm/display/drm_dp_helper.c | 78 + > drivers/gpu/drm/i915/display/intel_dp.c | 73 +-- > include/drm/display/drm_dp_helper.h | 3 + > 3 files changed, 84 insertions(+), 70 deletions(-) > > diff --git a/drivers/gpu/drm/display/drm_dp_helper.c > b/drivers/gpu/drm/display/drm_dp_helper.c > index b1ca3a1100da..066cfbbf7a91 100644 > --- a/drivers/gpu/drm/display/drm_dp_helper.c > +++ b/drivers/gpu/drm/display/drm_dp_helper.c > @@ -2916,6 +2916,84 @@ void drm_dp_vsc_sdp_log(const char *level, struct > device *dev, > } > EXPORT_SYMBOL(drm_dp_vsc_sdp_log); > > +/** > + * drm_dp_vsc_sdp_pack() - pack a given vsc sdp into generic dp_sdp > + * @vsc: vsc sdp initialized according to its purpose as defined in > + * table 2-118 - table 2-120 in DP 1.4a specification > + * @sdp: valid handle to the generic dp_sdp which will be packed > + * @size: valid size of the passed sdp handle > + * > + * Returns length of sdp on success and error code on failure > + */ > +ssize_t drm_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc, > + struct dp_sdp *sdp, size_t size) I know that you are just moving the function. Maybe there can be patch#2, which drops the size argument? The struct dp_sdp already has a defined size. The i915 driver just passes sizeof(sdp), which is more or less useless. > +{ > + size_t length = sizeof(struct dp_sdp); > + > + if (size < length) > + return -ENOSPC; > + > + memset(sdp, 0, size); > + > + /* > +* Prepare VSC Header for SU as per DP 1.4a spec, Table 2-119 > +* VSC SDP Header Bytes > +*/ > + sdp->sdp_header.HB0 = 0; /* Secondary-Data Packet ID = 0 */ > + sdp->sdp_header.HB1 = vsc->sdp_type; /* Secondary-data Packet Type */ > + sdp->sdp_header.HB2 = vsc->revision; /* Revision Number */ > + sdp->sdp_header.HB3 = vsc->length; /* Number of Valid Data Bytes */ > + > + if (vsc->revision == 0x6) { > + sdp->db[0] = 1; > + sdp->db[3] = 1; > + } > + > + /* > +* Revision 0x5 and revision 0x7 supports Pixel Encoding/Colorimetry > +* Format as per DP 1.4a spec and DP 2.0 respectively. > +*/ > + if (!(vsc->revision == 0x5 || vsc->revision == 0x7)) > + goto out; > + > + /* VSC SDP Payload for DB16 through DB18 */ > + /* Pixel Encoding and Colorimetry Formats */ > + sdp->db[16] = (vsc->pixelformat & 0xf) << 4; /* DB16[7:4] */ > + sdp->db[16] |= vsc->colorimetry & 0xf; /* DB16[3:0] */ > + > + switch (vsc->bpc) { > + case 6: > + /* 6bpc: 0x0 */ > + break; > + case 8: > + sdp->db[17] = 0x1; /* DB17[3:0] */ > + break; > + case 10: > + sdp->db[17] = 0x2; > + break; > + case 12: > + sdp->db[17] = 0x3; > + break; > + case 16: > + sdp->db[17] = 0x4; > + break; > + default: > + WARN(1, "Missing case %d\n", vsc->bpc); > + return -EINVAL; > + } > + > + /* Dynamic Range and Component Bit Depth */ > + if (vsc->dynamic_range == DP_DYNAMIC_RANGE_CTA) > + sdp->db[17] |= 0x80; /* DB17[7] */ > + > + /* Content Type */ > + sdp->db[18] = vsc->content_type & 0x7; > + > +out: > + return length; > +} > +EXPORT_SYMBOL(drm_dp_vsc_sdp_pack); > + > /** > * drm_dp_get_pcon_max_frl_bw() - maximum frl supported by PCON > * @dpcd: DisplayPort configuration data > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index f5ef95da5534..e94db51aeeb7 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -4110,73 +4110,6 @@ intel_dp_needs_vsc_sdp(const struct intel_crtc_state > *crtc_state, > return false; > } > > -static ssize_t intel_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc, > -struct dp_sdp *sdp, size_t size) > -{ > - size_t length = sizeof(struct dp_sdp); > - > - if (size < length) > - return -ENOSPC; > - > - memset(sdp, 0, size); > - > - /* > -* Prepare VSC Header for SU as per DP 1.4a spec, Table 2-119 > -* VSC SDP Header Bytes > -*/ > - sdp->sdp_header.HB0 = 0; /* Secondary-Data Packet ID = 0 */ > - sdp->sdp_header.HB1 = vsc->sdp_type; /* Secondary-data Packet Type */ > - sdp->sdp_header.HB2 = vsc->revision; /*
Re: [PATCH] drm/atomic-helpers: remove legacy_cursor_update hacks
On Wed, 31 Jan 2024 at 11:11, Daniel Vetter wrote: > > On Wed, Jan 31, 2024 at 05:17:08AM +, Jason-JH Lin (林睿祥) wrote: > > On Thu, 2024-01-25 at 19:17 +0100, Daniel Vetter wrote: > > > > > > External email : Please do not click links or open attachments until > > > you have verified the sender or the content. > > > On Tue, Jan 23, 2024 at 06:09:05AM +, Jason-JH Lin (林睿祥) wrote: > > > > Hi Maxime, Daniel, > > > > > > > > We encountered similar issue with mediatek SoCs. > > > > > > > > We have found that in drm_atomic_helper_commit_rpm(), when > > > disabling > > > > the cursor plane, the old_state->legacy_cursor_update in > > > > drm_atomic_wait_for_vblank() is set to true. > > > > As the result, we are not actually waiting for a vlbank to wait for > > > our > > > > hardware to close the cursor plane. Subsequently, the execution > > > > proceeds to drm_atomic_helper_cleanup_planes() to free the cursor > > > > buffer. This can lead to use-after-free issues with our hardware. > > > > > > > > Could you please apply this patch to fix our problem? > > > > Or are there any considerations for not applying this patch? > > > > > > Mostly it needs someone to collect a pile of acks/tested-by and then > > > land > > > it. > > > > > > > Got it. I would add tested-by tag for mediatek SoC. > > > > > I'd be _very_ happy if someone else can take care of that ... > > > > > > There's also the potential issue that it might slow down some of the > > > legacy X11 use-cases that really needed a non-blocking cursor, but I > > > think > > > all the drivers where this matters have switched over to the async > > > plane > > > update stuff meanwhile. So hopefully that's good. > > > > > > > I think all the drivers should have switched to async plane update. > > > > Can we add the checking condition to see if atomic_async_update/check > > function are implemented? > > Pretty sure not all have done that, so really it boils down to whether we > break a real user's use-case. Which pretty much can only be checked by > merging the patch (hence the requirement to get as many acks as possible > from display drivers) and then being willing to handle any fallout that's > reported as regressions for a specific driver. > > It's a pile of work, at least when it goes south, that's why I'm looking > for volunteers. I can check this on all sensible msm generations, including mdp4, but it will be next week, after the FOSDEM. BTW, for technical reasons one of the msm platforms still has the legacy cursor implementation might it be related? > > Note that handling the fallout doesn't mean you have to fix that specific > driver, the only realistic option might be to reinstate the legacy cursor > behaviour, but as an explicit opt-in that only that specific driver > enables. > > So maybe for next round of that patch it might be good to have a 2nd patch > which implements this fallback plan in the shared atomic modeset code? > > Cheers, Sima -- With best wishes Dmitry
Re: [PATCH 10/12] drm/panelreplay: dpcd register definition for panelreplay SU
On Thu, 4 Jan 2024 at 12:49, Jouni Högander wrote: > > Add definitions for panel replay selective update > > Cc: dri-de...@lists.freedesktop.org > 1) This CC should not be necessary. It is already a part of maintainers entry for this file 2) It probably doesn't work as expected. It is separated with the blank link from the rest of the trailers, so most of the tools will skip it. 3) You have skipped the rest of the maintainers for this file. Please use ./scripts/get_maintainers.pl and pass corresponding options to git send-email. > Signed-off-by: Jouni Högander > --- > include/drm/display/drm_dp.h | 6 ++ > 1 file changed, 6 insertions(+) The patch itself looks good to me. > > diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h > index 3731828825bd..6a59d30b7b25 100644 > --- a/include/drm/display/drm_dp.h > +++ b/include/drm/display/drm_dp.h > @@ -548,6 +548,12 @@ > # define DP_PANEL_REPLAY_SUPPORT(1 << 0) > # define DP_PANEL_REPLAY_SU_SUPPORT (1 << 1) > > +#define DP_PANEL_PANEL_REPLAY_CAPABILITY 0xb1 > +# define DP_PANEL_PANEL_REPLAY_SU_GRANULARITY_REQUIRED (1 << 5) > + > +#define DP_PANEL_PANEL_REPLAY_X_GRANULARITY0xb2 > +#define DP_PANEL_PANEL_REPLAY_Y_GRANULARITY0xb4 > + > /* Link Configuration */ > #defineDP_LINK_BW_SET 0x100 > # define DP_LINK_RATE_TABLE0x00/* eDP 1.4 */ > -- > 2.34.1 > -- With best wishes Dmitry
Re: [Intel-gfx] [RFC] drm: enable W=1 warnings by default across the subsystem
On Thu, 30 Nov 2023 at 11:18, Maxime Ripard wrote: > > Hi, > > On Thu, Nov 30, 2023 at 10:52:17AM +0200, Jani Nikula wrote: > > On Wed, 29 Nov 2023, Hamza Mahfooz wrote: > > > Cc: Nathan Chancellor > > > > > > On 11/29/23 13:12, Jani Nikula wrote: > > >> At least the i915 and amd drivers enable a bunch more compiler warnings > > >> than the kernel defaults. > > >> > > >> Extend the W=1 warnings to the entire drm subsystem by default. Use the > > >> copy-pasted warnings from scripts/Makefile.extrawarn with > > >> s/KBUILD_CFLAGS/subdir-ccflags-y/ to make it easier to compare and keep > > >> up with them in the future. > > >> > > >> This is similar to the approach currently used in i915. > > >> > > >> Some of the -Wextra warnings do need to be disabled, just like in > > >> Makefile.extrawarn, but take care to not disable them for W=2 or W=3 > > >> builds, depending on the warning. > > > > > > I think this should go in after drm-misc-next has a clean build (for > > > COMPILE_TEST builds) with this patch applied. Otherwise, it will break a > > > lot of build configs. > > > > Oh, I'm absolutely not suggesting this should be merged before known > > warnings have been addressed one way or another. Either by fixing them > > or by disabling said warning in driver local Makefiles, depending on the > > case. > > I'm all for it, but yeah, we need some easy way to opt-in/opt-out. Some > drivers are pretty much unmaintained now and are likely to never fix > those warnings. Then maybe they should have the same fate as other undermaintained drivers: either they get fixed, or they get dropped? -- With best wishes Dmitry
[Intel-gfx] [PATCH v7 3/3 RESEND] drm/bridge_connector: implement oob_hotplug_event
Implement the oob_hotplug_event() callback. Translate it to the HPD notification sent to the HPD bridge in the chain. Reviewed-by: Janne Grunau Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/drm_bridge_connector.c | 31 +++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c index 70582491d955..8239ad43aed5 100644 --- a/drivers/gpu/drm/drm_bridge_connector.c +++ b/drivers/gpu/drm/drm_bridge_connector.c @@ -5,6 +5,8 @@ #include #include +#include +#include #include #include @@ -107,10 +109,9 @@ static void drm_bridge_connector_hpd_notify(struct drm_connector *connector, } } -static void drm_bridge_connector_hpd_cb(void *cb_data, - enum drm_connector_status status) +static void drm_bridge_connector_handle_hpd(struct drm_bridge_connector *drm_bridge_connector, + enum drm_connector_status status) { - struct drm_bridge_connector *drm_bridge_connector = cb_data; struct drm_connector *connector = _bridge_connector->base; struct drm_device *dev = connector->dev; @@ -123,6 +124,21 @@ static void drm_bridge_connector_hpd_cb(void *cb_data, drm_kms_helper_connector_hotplug_event(connector); } +static void drm_bridge_connector_hpd_cb(void *cb_data, + enum drm_connector_status status) +{ + drm_bridge_connector_handle_hpd(cb_data, status); +} + +static void drm_bridge_connector_oob_hotplug_event(struct drm_connector *connector, + enum drm_connector_status status) +{ + struct drm_bridge_connector *bridge_connector = + to_drm_bridge_connector(connector); + + drm_bridge_connector_handle_hpd(bridge_connector, status); +} + static void drm_bridge_connector_enable_hpd(struct drm_connector *connector) { struct drm_bridge_connector *bridge_connector = @@ -191,6 +207,8 @@ static void drm_bridge_connector_destroy(struct drm_connector *connector) drm_connector_unregister(connector); drm_connector_cleanup(connector); + fwnode_handle_put(connector->fwnode); + kfree(bridge_connector); } @@ -216,6 +234,7 @@ static const struct drm_connector_funcs drm_bridge_connector_funcs = { .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, .debugfs_init = drm_bridge_connector_debugfs_init, + .oob_hotplug_event = drm_bridge_connector_oob_hotplug_event, }; /* - @@ -352,6 +371,12 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, if (!drm_bridge_get_next_bridge(bridge)) connector_type = bridge->type; +#ifdef CONFIG_OF + if (!drm_bridge_get_next_bridge(bridge) && + bridge->of_node) + connector->fwnode = fwnode_handle_get(of_fwnode_handle(bridge->of_node)); +#endif + if (bridge->ddc) ddc = bridge->ddc; -- 2.39.2
[Intel-gfx] [PATCH v7 1/3 RESEND] drm: Add HPD state to drm_connector_oob_hotplug_event()
From: Bjorn Andersson In some implementations, such as the Qualcomm platforms, the display driver has no way to query the current HPD state and as such it's impossible to distinguish between disconnect and attention events. Add a parameter to drm_connector_oob_hotplug_event() to pass the HPD state. Also push the test for unchanged state in the displayport altmode driver into the i915 driver, to allow other drivers to act upon each update. Signed-off-by: Bjorn Andersson Reviewed-by: Hans de Goede Acked-by: Heikki Krogerus Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/drm_connector.c | 6 -- .../gpu/drm/i915/display/intel_display_core.h | 3 +++ drivers/gpu/drm/i915/display/intel_dp.c | 17 ++--- drivers/usb/typec/altmodes/displayport.c| 17 + include/drm/drm_connector.h | 6 -- 5 files changed, 34 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 9d4c7b0c5c05..c3725086f413 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -3060,6 +3060,7 @@ struct drm_connector *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode) /** * drm_connector_oob_hotplug_event - Report out-of-band hotplug event to connector * @connector_fwnode: fwnode_handle to report the event on + * @status: hot plug detect logical state * * On some hardware a hotplug event notification may come from outside the display * driver / device. An example of this is some USB Type-C setups where the hardware @@ -3069,7 +3070,8 @@ struct drm_connector *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode) * This function can be used to report these out-of-band events after obtaining * a drm_connector reference through calling drm_connector_find_by_fwnode(). */ -void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode) +void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode, +enum drm_connector_status status) { struct drm_connector *connector; @@ -3078,7 +3080,7 @@ void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode) return; if (connector->funcs->oob_hotplug_event) - connector->funcs->oob_hotplug_event(connector); + connector->funcs->oob_hotplug_event(connector, status); drm_connector_put(connector); } diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h index 53e5c33e08c3..ccfe27630fb6 100644 --- a/drivers/gpu/drm/i915/display/intel_display_core.h +++ b/drivers/gpu/drm/i915/display/intel_display_core.h @@ -175,6 +175,9 @@ struct intel_hotplug { /* Whether or not to count short HPD IRQs in HPD storms */ u8 hpd_short_storm_enabled; + /* Last state reported by oob_hotplug_event for each encoder */ + unsigned long oob_hotplug_last_state; + /* * if we get a HPD irq from DP and a HPD irq from non-DP * the non-DP HPD could block the workqueue on a mode config diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 0ef7cb8134b6..4f6835a7578e 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -5723,15 +5723,26 @@ static int intel_dp_connector_atomic_check(struct drm_connector *conn, return intel_modeset_synced_crtcs(state, conn); } -static void intel_dp_oob_hotplug_event(struct drm_connector *connector) +static void intel_dp_oob_hotplug_event(struct drm_connector *connector, + enum drm_connector_status hpd_state) { struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector)); struct drm_i915_private *i915 = to_i915(connector->dev); + bool hpd_high = hpd_state == connector_status_connected; + unsigned int hpd_pin = encoder->hpd_pin; + bool need_work = false; spin_lock_irq(>irq_lock); - i915->display.hotplug.event_bits |= BIT(encoder->hpd_pin); + if (hpd_high != test_bit(hpd_pin, >display.hotplug.oob_hotplug_last_state)) { + i915->display.hotplug.event_bits |= BIT(hpd_pin); + + __assign_bit(hpd_pin, >display.hotplug.oob_hotplug_last_state, hpd_high); + need_work = true; + } spin_unlock_irq(>irq_lock); - queue_delayed_work(i915->unordered_wq, >display.hotplug.hotplug_work, 0); + + if (need_work) + queue_delayed_work(i915->unordered_wq, >display.hotplug.hotplug_work, 0); } static const struct drm_connector_funcs intel_dp_connector_funcs = { diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c index f503cb4cd721..c6dd35cec
[Intel-gfx] [PATCH v7 2/3 RESEND] drm/bridge_connector: stop filtering events in drm_bridge_connector_hpd_cb()
In some cases the bridge drivers would like to receive hotplug events even in the case new status is equal to the old status. In the DP case this is used to deliver "attention" messages to the DP host. Stop filtering the events in the drm_bridge_connector_hpd_cb() and let drivers decide whether they would like to receive the event or not. Reviewed-by: Janne Grunau Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/drm_bridge_connector.c | 5 - 1 file changed, 5 deletions(-) diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c index 31baf1f5ff81..70582491d955 100644 --- a/drivers/gpu/drm/drm_bridge_connector.c +++ b/drivers/gpu/drm/drm_bridge_connector.c @@ -113,16 +113,11 @@ static void drm_bridge_connector_hpd_cb(void *cb_data, struct drm_bridge_connector *drm_bridge_connector = cb_data; struct drm_connector *connector = _bridge_connector->base; struct drm_device *dev = connector->dev; - enum drm_connector_status old_status; mutex_lock(>mode_config.mutex); - old_status = connector->status; connector->status = status; mutex_unlock(>mode_config.mutex); - if (old_status == status) - return; - drm_bridge_connector_hpd_notify(connector, status); drm_kms_helper_connector_hotplug_event(connector); -- 2.39.2
[Intel-gfx] [PATCH v7 0/3 RESEND] drm/bridge_connector: implement OOB HPD handling
This is a resend, since the previous submission got no responses. The patches have been reviewed/acked by several maintainers. Can we please gain some attention and either get it merged or understand what should be changed / improved. This series is required to delivere HPD events from altmode driver to the MSM DP driver in the sane way. Note, numbering for this series starts from v5, since there were several revisions for this patchset under a different series title ([1]). USB altmodes code would send OOB notifications to the drm_connector specified in the device tree. However as the MSM DP driver uses drm_bridge_connector, there is no way to receive these event directly. Implement a bridge between oob_hotplug_event and drm_bridge's hpd_notify. Merge strategy: since this series touches i915 code, it might make sense to merge all three patches through drm-intel. [1] https://patchwork.freedesktop.org/series/103449/ Changes since v6: - Rebased on top of linux-next. Fixed the freshly added new drm_connector_oob_hotplug_event() call. Changes since v5: - Fixed checkpatch warning in the first patch (noted by intel-gfx CI). Changes since v4: - Picked up the patchset - Dropped msm-specific patches - Changed drm_bridge_connector_oob_hotplug_event to call connector's HPD callback directly, rather than going through the last bridge's hpd_notify - Added proper fwnode for the drm_bridge_connector Bjorn Andersson (1): drm: Add HPD state to drm_connector_oob_hotplug_event() Dmitry Baryshkov (2): drm/bridge_connector: stop filtering events in drm_bridge_connector_hpd_cb() drm/bridge_connector: implement oob_hotplug_event drivers/gpu/drm/drm_bridge_connector.c| 36 ++- drivers/gpu/drm/drm_connector.c | 6 ++-- .../gpu/drm/i915/display/intel_display_core.h | 3 ++ drivers/gpu/drm/i915/display/intel_dp.c | 17 +++-- drivers/usb/typec/altmodes/displayport.c | 17 - include/drm/drm_connector.h | 6 ++-- 6 files changed, 62 insertions(+), 23 deletions(-) -- 2.39.2
Re: [Intel-gfx] [PATCH v7 0/3] drm/bridge_connector: implement OOB HPD handling
On 25/08/2023 02:56, Dmitry Baryshkov wrote: Note, numbering for this series starts from v5, since there were several revisions for this patchset under a different series title ([1]). USB altmodes code would send OOB notifications to the drm_connector specified in the device tree. However as the MSM DP driver uses drm_bridge_connector, there is no way to receive these event directly. Implement a bridge between oob_hotplug_event and drm_bridge's hpd_notify. Merge strategy: since this series touches i915 code, it might make sense to merge all three patches through drm-intel. Dear drm-misc and drm-intel maintainers. Since the merge window has ended and the trees are fully open for the patches, I'd like to massage this patch series. We have R-B on all three patches. Heikki has acked the first patch, so it seems to be fine from the i915 point of view. Is it fine to be merged via drm-misc? Would you like to pick it up into drm-intel? [1] https://patchwork.freedesktop.org/series/103449/ Changes since v6: - Rebased on top of linux-next. Fixed the freshly added new drm_connector_oob_hotplug_event() call. Changes since v5: - Fixed checkpatch warning in the first patch (noted by intel-gfx CI). Changes since v4: - Picked up the patchset - Dropped msm-specific patches - Changed drm_bridge_connector_oob_hotplug_event to call connector's HPD callback directly, rather than going through the last bridge's hpd_notify - Added proper fwnode for the drm_bridge_connector Bjorn Andersson (1): drm: Add HPD state to drm_connector_oob_hotplug_event() Dmitry Baryshkov (2): drm/bridge_connector: stop filtering events in drm_bridge_connector_hpd_cb() drm/bridge_connector: implement oob_hotplug_event drivers/gpu/drm/drm_bridge_connector.c| 34 ++- drivers/gpu/drm/drm_connector.c | 6 ++-- .../gpu/drm/i915/display/intel_display_core.h | 3 ++ drivers/gpu/drm/i915/display/intel_dp.c | 17 -- drivers/usb/typec/altmodes/displayport.c | 17 +- include/drm/drm_connector.h | 6 ++-- 6 files changed, 60 insertions(+), 23 deletions(-) -- With best wishes Dmitry
[Intel-gfx] [PATCH v7 3/3] drm/bridge_connector: implement oob_hotplug_event
Implement the oob_hotplug_event() callback. Translate it to the HPD notification sent to the HPD bridge in the chain. Reviewed-by: Janne Grunau Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/drm_bridge_connector.c | 31 +++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c index 10b52224db37..bf73960c2c2a 100644 --- a/drivers/gpu/drm/drm_bridge_connector.c +++ b/drivers/gpu/drm/drm_bridge_connector.c @@ -5,6 +5,8 @@ #include #include +#include +#include #include #include @@ -107,10 +109,9 @@ static void drm_bridge_connector_hpd_notify(struct drm_connector *connector, } } -static void drm_bridge_connector_hpd_cb(void *cb_data, - enum drm_connector_status status) +static void drm_bridge_connector_handle_hpd(struct drm_bridge_connector *drm_bridge_connector, + enum drm_connector_status status) { - struct drm_bridge_connector *drm_bridge_connector = cb_data; struct drm_connector *connector = _bridge_connector->base; struct drm_device *dev = connector->dev; @@ -123,6 +124,21 @@ static void drm_bridge_connector_hpd_cb(void *cb_data, drm_kms_helper_connector_hotplug_event(connector); } +static void drm_bridge_connector_hpd_cb(void *cb_data, + enum drm_connector_status status) +{ + drm_bridge_connector_handle_hpd(cb_data, status); +} + +static void drm_bridge_connector_oob_hotplug_event(struct drm_connector *connector, + enum drm_connector_status status) +{ + struct drm_bridge_connector *bridge_connector = + to_drm_bridge_connector(connector); + + drm_bridge_connector_handle_hpd(bridge_connector, status); +} + static void drm_bridge_connector_enable_hpd(struct drm_connector *connector) { struct drm_bridge_connector *bridge_connector = @@ -191,6 +207,8 @@ static void drm_bridge_connector_destroy(struct drm_connector *connector) drm_connector_unregister(connector); drm_connector_cleanup(connector); + fwnode_handle_put(connector->fwnode); + kfree(bridge_connector); } @@ -216,6 +234,7 @@ static const struct drm_connector_funcs drm_bridge_connector_funcs = { .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, .debugfs_init = drm_bridge_connector_debugfs_init, + .oob_hotplug_event = drm_bridge_connector_oob_hotplug_event, }; /* - @@ -352,6 +371,12 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, if (!drm_bridge_get_next_bridge(bridge)) connector_type = bridge->type; +#ifdef CONFIG_OF + if (!drm_bridge_get_next_bridge(bridge) && + bridge->of_node) + connector->fwnode = fwnode_handle_get(of_fwnode_handle(bridge->of_node)); +#endif + if (bridge->ddc) ddc = bridge->ddc; -- 2.39.2
[Intel-gfx] [PATCH v7 2/3] drm/bridge_connector: stop filtering events in drm_bridge_connector_hpd_cb()
In some cases the bridge drivers would like to receive hotplug events even in the case new status is equal to the old status. In the DP case this is used to deliver "attention" messages to the DP host. Stop filtering the events in the drm_bridge_connector_hpd_cb() and let drivers decide whether they would like to receive the event or not. Reviewed-By: Janne Grunau Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/drm_bridge_connector.c | 5 - 1 file changed, 5 deletions(-) diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c index 1da93d5a1f61..10b52224db37 100644 --- a/drivers/gpu/drm/drm_bridge_connector.c +++ b/drivers/gpu/drm/drm_bridge_connector.c @@ -113,16 +113,11 @@ static void drm_bridge_connector_hpd_cb(void *cb_data, struct drm_bridge_connector *drm_bridge_connector = cb_data; struct drm_connector *connector = _bridge_connector->base; struct drm_device *dev = connector->dev; - enum drm_connector_status old_status; mutex_lock(>mode_config.mutex); - old_status = connector->status; connector->status = status; mutex_unlock(>mode_config.mutex); - if (old_status == status) - return; - drm_bridge_connector_hpd_notify(connector, status); drm_kms_helper_connector_hotplug_event(connector); -- 2.39.2
[Intel-gfx] [PATCH v7 1/3] drm: Add HPD state to drm_connector_oob_hotplug_event()
From: Bjorn Andersson In some implementations, such as the Qualcomm platforms, the display driver has no way to query the current HPD state and as such it's impossible to distinguish between disconnect and attention events. Add a parameter to drm_connector_oob_hotplug_event() to pass the HPD state. Also push the test for unchanged state in the displayport altmode driver into the i915 driver, to allow other drivers to act upon each update. Signed-off-by: Bjorn Andersson Reviewed-by: Hans de Goede Acked-by: Heikki Krogerus Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/drm_connector.c | 6 -- .../gpu/drm/i915/display/intel_display_core.h | 3 +++ drivers/gpu/drm/i915/display/intel_dp.c | 17 ++--- drivers/usb/typec/altmodes/displayport.c| 17 + include/drm/drm_connector.h | 6 -- 5 files changed, 34 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index c44d5bcf1284..05fc29a54801 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -3051,6 +3051,7 @@ struct drm_connector *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode) /** * drm_connector_oob_hotplug_event - Report out-of-band hotplug event to connector * @connector_fwnode: fwnode_handle to report the event on + * @status: hot plug detect logical state * * On some hardware a hotplug event notification may come from outside the display * driver / device. An example of this is some USB Type-C setups where the hardware @@ -3060,7 +3061,8 @@ struct drm_connector *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode) * This function can be used to report these out-of-band events after obtaining * a drm_connector reference through calling drm_connector_find_by_fwnode(). */ -void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode) +void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode, +enum drm_connector_status status) { struct drm_connector *connector; @@ -3069,7 +3071,7 @@ void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode) return; if (connector->funcs->oob_hotplug_event) - connector->funcs->oob_hotplug_event(connector); + connector->funcs->oob_hotplug_event(connector, status); drm_connector_put(connector); } diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h index 53e5c33e08c3..ccfe27630fb6 100644 --- a/drivers/gpu/drm/i915/display/intel_display_core.h +++ b/drivers/gpu/drm/i915/display/intel_display_core.h @@ -175,6 +175,9 @@ struct intel_hotplug { /* Whether or not to count short HPD IRQs in HPD storms */ u8 hpd_short_storm_enabled; + /* Last state reported by oob_hotplug_event for each encoder */ + unsigned long oob_hotplug_last_state; + /* * if we get a HPD irq from DP and a HPD irq from non-DP * the non-DP HPD could block the workqueue on a mode config diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 12bd2f322e62..2af815b5c22b 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -5253,15 +5253,26 @@ static int intel_dp_connector_atomic_check(struct drm_connector *conn, return intel_modeset_synced_crtcs(state, conn); } -static void intel_dp_oob_hotplug_event(struct drm_connector *connector) +static void intel_dp_oob_hotplug_event(struct drm_connector *connector, + enum drm_connector_status hpd_state) { struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector)); struct drm_i915_private *i915 = to_i915(connector->dev); + bool hpd_high = hpd_state == connector_status_connected; + unsigned int hpd_pin = encoder->hpd_pin; + bool need_work = false; spin_lock_irq(>irq_lock); - i915->display.hotplug.event_bits |= BIT(encoder->hpd_pin); + if (hpd_high != test_bit(hpd_pin, >display.hotplug.oob_hotplug_last_state)) { + i915->display.hotplug.event_bits |= BIT(hpd_pin); + + __assign_bit(hpd_pin, >display.hotplug.oob_hotplug_last_state, hpd_high); + need_work = true; + } spin_unlock_irq(>irq_lock); - queue_delayed_work(i915->unordered_wq, >display.hotplug.hotplug_work, 0); + + if (need_work) + queue_delayed_work(i915->unordered_wq, >display.hotplug.hotplug_work, 0); } static const struct drm_connector_funcs intel_dp_connector_funcs = { diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c index 426c88a516e5..29f40e658
[Intel-gfx] [PATCH v7 0/3] drm/bridge_connector: implement OOB HPD handling
Note, numbering for this series starts from v5, since there were several revisions for this patchset under a different series title ([1]). USB altmodes code would send OOB notifications to the drm_connector specified in the device tree. However as the MSM DP driver uses drm_bridge_connector, there is no way to receive these event directly. Implement a bridge between oob_hotplug_event and drm_bridge's hpd_notify. Merge strategy: since this series touches i915 code, it might make sense to merge it through drm-intel. [1] https://patchwork.freedesktop.org/series/103449/ Changes since v6: - Fixed the fwnode refcount in drm/bridge-connector Changes since v5: - Fixed checkpatch warning in the first patch (noted by intel-gfx CI). Changes since v4: - Picked up the patchset - Dropped msm-specific patches - Changed drm_bridge_connector_oob_hotplug_event to call connector's HPD callback directly, rather than going through the last bridge's hpd_notify - Added proper fwnode for the drm_bridge_connector Bjorn Andersson (1): drm: Add HPD state to drm_connector_oob_hotplug_event() Dmitry Baryshkov (2): drm/bridge_connector: stop filtering events in drm_bridge_connector_hpd_cb() drm/bridge_connector: implement oob_hotplug_event drivers/gpu/drm/drm_bridge_connector.c| 36 ++- drivers/gpu/drm/drm_connector.c | 6 ++-- .../gpu/drm/i915/display/intel_display_core.h | 3 ++ drivers/gpu/drm/i915/display/intel_dp.c | 17 +++-- drivers/usb/typec/altmodes/displayport.c | 17 - include/drm/drm_connector.h | 6 ++-- 6 files changed, 62 insertions(+), 23 deletions(-) -- 2.39.2
[Intel-gfx] [PATCH v7 1/3] drm: Add HPD state to drm_connector_oob_hotplug_event()
From: Bjorn Andersson In some implementations, such as the Qualcomm platforms, the display driver has no way to query the current HPD state and as such it's impossible to distinguish between disconnect and attention events. Add a parameter to drm_connector_oob_hotplug_event() to pass the HPD state. Also push the test for unchanged state in the displayport altmode driver into the i915 driver, to allow other drivers to act upon each update. Signed-off-by: Bjorn Andersson Reviewed-by: Hans de Goede Acked-by: Heikki Krogerus Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/drm_connector.c | 6 -- .../gpu/drm/i915/display/intel_display_core.h | 3 +++ drivers/gpu/drm/i915/display/intel_dp.c | 17 ++--- drivers/usb/typec/altmodes/displayport.c| 17 + include/drm/drm_connector.h | 6 -- 5 files changed, 34 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index bf8371dc2a61..a3d3e7dc08b2 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -3049,6 +3049,7 @@ struct drm_connector *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode) /** * drm_connector_oob_hotplug_event - Report out-of-band hotplug event to connector * @connector_fwnode: fwnode_handle to report the event on + * @status: hot plug detect logical state * * On some hardware a hotplug event notification may come from outside the display * driver / device. An example of this is some USB Type-C setups where the hardware @@ -3058,7 +3059,8 @@ struct drm_connector *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode) * This function can be used to report these out-of-band events after obtaining * a drm_connector reference through calling drm_connector_find_by_fwnode(). */ -void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode) +void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode, +enum drm_connector_status status) { struct drm_connector *connector; @@ -3067,7 +3069,7 @@ void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode) return; if (connector->funcs->oob_hotplug_event) - connector->funcs->oob_hotplug_event(connector); + connector->funcs->oob_hotplug_event(connector, status); drm_connector_put(connector); } diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h index 53e5c33e08c3..ccfe27630fb6 100644 --- a/drivers/gpu/drm/i915/display/intel_display_core.h +++ b/drivers/gpu/drm/i915/display/intel_display_core.h @@ -175,6 +175,9 @@ struct intel_hotplug { /* Whether or not to count short HPD IRQs in HPD storms */ u8 hpd_short_storm_enabled; + /* Last state reported by oob_hotplug_event for each encoder */ + unsigned long oob_hotplug_last_state; + /* * if we get a HPD irq from DP and a HPD irq from non-DP * the non-DP HPD could block the workqueue on a mode config diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 12bd2f322e62..2af815b5c22b 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -5253,15 +5253,26 @@ static int intel_dp_connector_atomic_check(struct drm_connector *conn, return intel_modeset_synced_crtcs(state, conn); } -static void intel_dp_oob_hotplug_event(struct drm_connector *connector) +static void intel_dp_oob_hotplug_event(struct drm_connector *connector, + enum drm_connector_status hpd_state) { struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector)); struct drm_i915_private *i915 = to_i915(connector->dev); + bool hpd_high = hpd_state == connector_status_connected; + unsigned int hpd_pin = encoder->hpd_pin; + bool need_work = false; spin_lock_irq(>irq_lock); - i915->display.hotplug.event_bits |= BIT(encoder->hpd_pin); + if (hpd_high != test_bit(hpd_pin, >display.hotplug.oob_hotplug_last_state)) { + i915->display.hotplug.event_bits |= BIT(hpd_pin); + + __assign_bit(hpd_pin, >display.hotplug.oob_hotplug_last_state, hpd_high); + need_work = true; + } spin_unlock_irq(>irq_lock); - queue_delayed_work(i915->unordered_wq, >display.hotplug.hotplug_work, 0); + + if (need_work) + queue_delayed_work(i915->unordered_wq, >display.hotplug.hotplug_work, 0); } static const struct drm_connector_funcs intel_dp_connector_funcs = { diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c index 426c88a516e5..29f40e658
[Intel-gfx] [PATCH v7 3/3] drm/bridge_connector: implement oob_hotplug_event
Implement the oob_hotplug_event() callback. Translate it to the HPD notification sent to the HPD bridge in the chain. Reviewed-by: Janne Grunau Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/drm_bridge_connector.c | 29 +++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c index 10b52224db37..3aa129b3f8e9 100644 --- a/drivers/gpu/drm/drm_bridge_connector.c +++ b/drivers/gpu/drm/drm_bridge_connector.c @@ -5,6 +5,8 @@ #include #include +#include +#include #include #include @@ -107,10 +109,9 @@ static void drm_bridge_connector_hpd_notify(struct drm_connector *connector, } } -static void drm_bridge_connector_hpd_cb(void *cb_data, - enum drm_connector_status status) +static void drm_bridge_connector_handle_hpd(struct drm_bridge_connector *drm_bridge_connector, + enum drm_connector_status status) { - struct drm_bridge_connector *drm_bridge_connector = cb_data; struct drm_connector *connector = _bridge_connector->base; struct drm_device *dev = connector->dev; @@ -123,6 +124,21 @@ static void drm_bridge_connector_hpd_cb(void *cb_data, drm_kms_helper_connector_hotplug_event(connector); } +static void drm_bridge_connector_hpd_cb(void *cb_data, + enum drm_connector_status status) +{ + drm_bridge_connector_handle_hpd(cb_data, status); +} + +static void drm_bridge_connector_oob_hotplug_event(struct drm_connector *connector, + enum drm_connector_status status) +{ + struct drm_bridge_connector *bridge_connector = + to_drm_bridge_connector(connector); + + drm_bridge_connector_handle_hpd(bridge_connector, status); +} + static void drm_bridge_connector_enable_hpd(struct drm_connector *connector) { struct drm_bridge_connector *bridge_connector = @@ -216,6 +232,7 @@ static const struct drm_connector_funcs drm_bridge_connector_funcs = { .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, .debugfs_init = drm_bridge_connector_debugfs_init, + .oob_hotplug_event = drm_bridge_connector_oob_hotplug_event, }; /* - @@ -352,6 +369,12 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, if (!drm_bridge_get_next_bridge(bridge)) connector_type = bridge->type; +#ifdef CONFIG_OF + if (!drm_bridge_get_next_bridge(bridge) && + bridge->of_node) + connector->fwnode = fwnode_handle_get(of_fwnode_handle(bridge->of_node)); +#endif + if (bridge->ddc) ddc = bridge->ddc; -- 2.39.2
[Intel-gfx] [PATCH v7 2/3] drm/bridge_connector: stop filtering events in drm_bridge_connector_hpd_cb()
In some cases the bridge drivers would like to receive hotplug events even in the case new status is equal to the old status. In the DP case this is used to deliver "attention" messages to the DP host. Stop filtering the events in the drm_bridge_connector_hpd_cb() and let drivers decide whether they would like to receive the event or not. Reviewed-By: Janne Grunau Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/drm_bridge_connector.c | 5 - 1 file changed, 5 deletions(-) diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c index 1da93d5a1f61..10b52224db37 100644 --- a/drivers/gpu/drm/drm_bridge_connector.c +++ b/drivers/gpu/drm/drm_bridge_connector.c @@ -113,16 +113,11 @@ static void drm_bridge_connector_hpd_cb(void *cb_data, struct drm_bridge_connector *drm_bridge_connector = cb_data; struct drm_connector *connector = _bridge_connector->base; struct drm_device *dev = connector->dev; - enum drm_connector_status old_status; mutex_lock(>mode_config.mutex); - old_status = connector->status; connector->status = status; mutex_unlock(>mode_config.mutex); - if (old_status == status) - return; - drm_bridge_connector_hpd_notify(connector, status); drm_kms_helper_connector_hotplug_event(connector); -- 2.39.2
[Intel-gfx] [PATCH v7 0/3] drm/bridge_connector: implement OOB HPD handling
Note, numbering for this series starts from v5, since there were several revisions for this patchset under a different series title ([1]). USB altmodes code would send OOB notifications to the drm_connector specified in the device tree. However as the MSM DP driver uses drm_bridge_connector, there is no way to receive these event directly. Implement a bridge between oob_hotplug_event and drm_bridge's hpd_notify. Merge strategy: since this series touches i915 code, it might make sense to merge all three patches through drm-intel. [1] https://patchwork.freedesktop.org/series/103449/ Changes since v6: - Rebased on top of linux-next. Fixed the freshly added new drm_connector_oob_hotplug_event() call. Changes since v5: - Fixed checkpatch warning in the first patch (noted by intel-gfx CI). Changes since v4: - Picked up the patchset - Dropped msm-specific patches - Changed drm_bridge_connector_oob_hotplug_event to call connector's HPD callback directly, rather than going through the last bridge's hpd_notify - Added proper fwnode for the drm_bridge_connector Bjorn Andersson (1): drm: Add HPD state to drm_connector_oob_hotplug_event() Dmitry Baryshkov (2): drm/bridge_connector: stop filtering events in drm_bridge_connector_hpd_cb() drm/bridge_connector: implement oob_hotplug_event drivers/gpu/drm/drm_bridge_connector.c| 34 ++- drivers/gpu/drm/drm_connector.c | 6 ++-- .../gpu/drm/i915/display/intel_display_core.h | 3 ++ drivers/gpu/drm/i915/display/intel_dp.c | 17 -- drivers/usb/typec/altmodes/displayport.c | 17 +- include/drm/drm_connector.h | 6 ++-- 6 files changed, 60 insertions(+), 23 deletions(-) -- 2.39.2
Re: [Intel-gfx] [PATCH 3/4] drm/uapi: document the USB subconnector type
On 18/08/2023 09:24, Simon Ser wrote: On Thursday, August 17th, 2023 at 21:33, Dmitry Baryshkov wrote: We have been looking for a way to document that the corresponding DP port is represented by the USB connector on the device. Consequently, I believe the best way to document it, would be to use DisplayPort / USB, when there is no dongle connected, switching to DisplayPort / HDMI, DisplayPort / VGA, DisplayPort / DisplayPort, etc. when the actual dongle / display is connected and then switching back to the DisplayPort / USB when it gets disconnected. If this sounds good to all parties, I'll post v2, adding this explanation to the cover letter. But how can user-space discover that the port is USB-C when it's connected? That information is lost at this point. Yes, unfortunately. (In addition, this clashes with the existing semantics of the subconnector prop as discussed before: USB-C is not sub-, it's super-.) Ok. How do we proceed then? Is it fine to add another property for DP case? Do you have any particular property name in mind? I will follow with addition of this property then. -- With best wishes Dmitry
Re: [Intel-gfx] [PATCH 3/4] drm/uapi: document the USB subconnector type
Simon, Laurent, On 03/08/2023 23:46, Simon Ser wrote: On Thursday, August 3rd, 2023 at 22:44, Laurent Pinchart wrote: On Thu, Aug 03, 2023 at 03:31:16PM +, Simon Ser wrote: On Thursday, August 3rd, 2023 at 17:22, Simon Ser cont...@emersion.fr wrote: The KMS docs describe "subconnector" to be defined as "downstream port" for DP. Can USB-C (or USB) be seen as a DP downstream port? To expand on this a bit: I'm wondering if we're mixing apples and oranges here. The current values of "subconnector" typically describe the lower-level protocol tunneled inside DP. For instance, VGA can be tunneled inside the DP cable when using DP → VGA adapter. Doesn't this contradict the example use case you gave in your previous e-mail, with wlroots stating "DP-3 via DVI-D" ? I understand that as DP carried over a DVI-D physical connector, did I get it wrong ? No, this is DVI carried over DP. DP cannot be carried over VGA/DVI/HDMI, but VGA/DVI/HDMI can be carried over DP. Please excuse me for the long delay, I was on vacation. Several notes on the subconnector topic. For TV and DVI-I we are really identifying a connector (or a part of the connector pins) present on the device. So, we can have e.g. following combinations (type / subtype): DVI-I / DVI-D (digital part of DVI connector) DVI-I / DVI-A (analog part of DVI connector) TV / S-Video (full S-Video connector) TV / Composite (either a separate Composite connector, or shared with S-Video) etc. For DP unfortunately we have mixed everything together. The physical connector present on the device can be DP / miniDP or USB-C (or micro-USB for SlimPort). The physical protocol can be DP or DVI / HDMI (but only for dual-mode DP ports). Over USB-C link the DP can be transferred using DP or USB signal levels. And last, but not least, we have the dongle / display connector type, which can be VGA (for active DP -> VGA converters), HDMI, DVI, DP, etc. If we were designing this from the scratch, I'd say that we should encode physical connector type to DRM connector type and the dongle type to subconnector. However AMD and Intel drivers have already reused DisplayPort connector type for USB-C connections. Subconnector type represents (if known) the type of downstream / dongle port. I'm not going to judge whether this was correct or not. We have to live with this and behave in a similar way. We have been looking for a way to document that the corresponding DP port is represented by the USB connector on the device. Consequently, I believe the best way to document it, would be to use DisplayPort / USB, when there is no dongle connected, switching to DisplayPort / HDMI, DisplayPort / VGA, DisplayPort / DisplayPort, etc. when the actual dongle / display is connected and then switching back to the DisplayPort / USB when it gets disconnected. If this sounds good to all parties, I'll post v2, adding this explanation to the cover letter. -- With best wishes Dmitry
Re: [Intel-gfx] [PATCH v2 2/5] drm: Add an HPD poll helper to reschedule the poll work
On Thu, 20 Jul 2023 at 15:54, Imre Deak wrote: > > Add a helper to reschedule drm_mode_config::output_poll_work after > polling has been enabled for a connector (and needing a reschedule, > since previously polling was disabled for all connectors and hence > output_poll_work was not running). > > This is needed by the next patch fixing HPD polling on i915. > > Cc: Dmitry Baryshkov > Cc: dri-de...@lists.freedesktop.org > Signed-off-by: Imre Deak Reviewed-by: Dmitry Baryshkov > --- > drivers/gpu/drm/drm_probe_helper.c | 68 -- > include/drm/drm_probe_helper.h | 1 + > 2 files changed, 47 insertions(+), 22 deletions(-) I support merging these patches through drm-intel rather than drm-misc. -- With best wishes Dmitry
Re: [Intel-gfx] [PATCH 3/4] drm/uapi: document the USB subconnector type
On Thu, 3 Aug 2023 at 23:47, Simon Ser wrote: > > On Thursday, August 3rd, 2023 at 22:44, Laurent Pinchart > wrote: > > > On Thu, Aug 03, 2023 at 03:31:16PM +, Simon Ser wrote: > > > > > On Thursday, August 3rd, 2023 at 17:22, Simon Ser cont...@emersion.fr > > > wrote: > > > > > > > The KMS docs describe "subconnector" to be defined as "downstream port" > > > > for DP. > > > > Can USB-C (or USB) be seen as a DP downstream port? > > > > > > To expand on this a bit: I'm wondering if we're mixing apples and > > > oranges here. The current values of "subconnector" typically describe > > > the lower-level protocol tunneled inside DP. For instance, VGA can be > > > tunneled inside the DP cable when using DP → VGA adapter. > > > > Doesn't this contradict the example use case you gave in your previous > > e-mail, with wlroots stating "DP-3 via DVI-D" ? I understand that as DP > > carried over a DVI-D physical connector, did I get it wrong ? > > No, this is DVI carried over DP. DP cannot be carried over VGA/DVI/HDMI, > but VGA/DVI/HDMI can be carried over DP. Well, not quite. It means that the sink (display) connected to this port identifies itself as an VGA / DVI / HDMI monitor. E.g. on if connect HDMI/DVI monitor through the DP-DVI dongle (native DP connector), AMD driver still identifies it as 'subconnector HDMI', despite dongle a DVI connector on the other side. -- With best wishes Dmitry
Re: [Intel-gfx] [PATCH 3/4] drm/uapi: document the USB subconnector type
On Thu, 3 Aug 2023 at 18:43, Simon Ser wrote: > > On Thursday, August 3rd, 2023 at 17:36, Dmitry Baryshkov > wrote: > > > On Thu, 3 Aug 2023 at 18:31, Simon Ser cont...@emersion.fr wrote: > > > > > On Thursday, August 3rd, 2023 at 17:22, Simon Ser cont...@emersion.fr > > > wrote: > > > > > > > The KMS docs describe "subconnector" to be defined as "downstream port" > > > > for DP. > > > > Can USB-C (or USB) be seen as a DP downstream port? > > > > > > To expand on this a bit: I'm wondering if we're mixing apples and > > > oranges here. The current values of "subconnector" typically describe > > > the lower-level protocol tunneled inside DP. For instance, VGA can be > > > tunneled inside the DP cable when using DP → VGA adapter. > > > > My opinion hasn't changed: I think this should be the USB connector > > with proper DP / DVI / HDMI / etc. subconnector type (or lack of it). > > In the end, the physical connector on the side of laptop is USB-C. > > - Even if the connector is USB-C, the protocol used for display is > still DP. There's also the case of Thunderbolt. Yes. But the connector type is not about the protocol. > - This is inconsistent with existing drivers. i915 and amdgpu expose > DP ports for their USB-C ports. Changing that isn't possible without > causing user-space regressions (compositor config files use the > connector type). Yes, I know. Consider my phrase as a personal opinion or minority report. I think that using DisplayPort for USB-C connectors was a mistake, which we now have to cope with somehow. -- With best wishes Dmitry
Re: [Intel-gfx] [PATCH 3/4] drm/uapi: document the USB subconnector type
On Thu, 3 Aug 2023 at 18:31, Simon Ser wrote: > > On Thursday, August 3rd, 2023 at 17:22, Simon Ser wrote: > > > The KMS docs describe "subconnector" to be defined as "downstream port" for > > DP. > > Can USB-C (or USB) be seen as a DP downstream port? > > To expand on this a bit: I'm wondering if we're mixing apples and > oranges here. The current values of "subconnector" typically describe > the lower-level protocol tunneled inside DP. For instance, VGA can be > tunneled inside the DP cable when using DP → VGA adapter. My opinion hasn't changed: I think this should be the USB connector with proper DP / DVI / HDMI / etc. subconnector type (or lack of it). In the end, the physical connector on the side of laptop is USB-C. If we want to make it different from GUD, we might want to define a USB-DP connector type (which would also include SlimPort). > > However, in the USB-C case, DP itself is tunneled inside USB-C. And you > might use a USB-C → DP adapter. So it's not really *sub*connector, it's > more of a *super*connector, right? > > I think [1] is somewhat related, since it also allows user-space to > discover whether a connector uses USB-C. But relying on sysfs to figure > this out isn't super optimal perhaps. > > [1]: > https://lore.kernel.org/dri-devel/20221108185004.2263578-1-wonch...@google.com/ -- With best wishes Dmitry
Re: [Intel-gfx] [PATCH 3/4] drm/uapi: document the USB subconnector type
2 августа 2023 г. 22:13:51 GMT+03:00, Laurent Pinchart пишет: >On Wed, Aug 02, 2023 at 10:01:19PM +0300, Dmitry Baryshkov wrote: >> On 02/08/2023 21:55, Laurent Pinchart wrote: >> > Hi Dmitry, >> > >> > Thank you for the patch. >> > >> > On Sat, Jul 29, 2023 at 03:49:12AM +0300, Dmitry Baryshkov wrote: >> >> To properly define the USB-C DP altmode connectors, add the USB >> >> subconnector type. >> >> >> >> Suggested-by: Simon Ser >> >> Signed-off-by: Dmitry Baryshkov >> >> --- >> >> drivers/gpu/drm/drm_connector.c | 1 + >> >> include/uapi/drm/drm_mode.h | 1 + >> >> 2 files changed, 2 insertions(+) >> >> >> >> diff --git a/drivers/gpu/drm/drm_connector.c >> >> b/drivers/gpu/drm/drm_connector.c >> >> index a6066e4a5e9a..9e96b038f5d0 100644 >> >> --- a/drivers/gpu/drm/drm_connector.c >> >> +++ b/drivers/gpu/drm/drm_connector.c >> >> @@ -1050,6 +1050,7 @@ static const struct drm_prop_enum_list >> >> drm_dp_subconnector_enum_list[] = { >> >> { DRM_MODE_SUBCONNECTOR_DisplayPort, "DP"}, /* DP */ >> >> { DRM_MODE_SUBCONNECTOR_Wireless,"Wireless" }, /* DP */ >> >> { DRM_MODE_SUBCONNECTOR_Native, "Native"}, /* DP */ >> >> + { DRM_MODE_SUBCONNECTOR_USB, "USB" }, /* DP */ >> > >> > Should this be DRM_MODE_SUBCONNECTOR_USB_C and "USB-C", in case we get >> > another USB type later ? >> >> Hmm, which id should I use for micro-USB then? (consider anx7808, >> SlimPort). I thought about using DRM_MODE_SUBCONNECTOR_USB for both of >> them. But maybe I should add another subtype for SlimPort. > >I suppose it depends on whether userspace needs a way to differentiate >those. Do you have a good visibility on the userspace use cases ? No. I'm not even sure, which userspace handles subtypes properly. For the reference, SlimPort is mostly legacy hardware, think about Nexus 4, 5, 6, 7 (2013) > >> >> }; >> >> >> >> DRM_ENUM_NAME_FN(drm_get_dp_subconnector_name, >> >> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h >> >> index 92d96a2b6763..0f74918b011c 100644 >> >> --- a/include/uapi/drm/drm_mode.h >> >> +++ b/include/uapi/drm/drm_mode.h >> >> @@ -398,6 +398,7 @@ enum drm_mode_subconnector { >> >> DRM_MODE_SUBCONNECTOR_HDMIA = 11, /*DP */ >> >> DRM_MODE_SUBCONNECTOR_Native = 15, /*DP */ >> >> DRM_MODE_SUBCONNECTOR_Wireless= 18, /*DP */ >> >> + DRM_MODE_SUBCONNECTOR_USB = 20, /*DP */ >> >> }; >> >> >> >> #define DRM_MODE_CONNECTOR_Unknown 0 >
Re: [Intel-gfx] [PATCH 1/4] drm: allow specifying default subtype for the DP subconnector property
On 02/08/2023 21:54, Laurent Pinchart wrote: Hi Dmitry, Thank you for the patch. On Sat, Jul 29, 2023 at 03:49:10AM +0300, Dmitry Baryshkov wrote: In the embedded usecases the default subtype depends on the bridge chain, so it is easier to specify the subtype at the proprety attachment s/proprety/property/ type rather than specifying it later. Did you mean s/type/time/ ? I think I understand why you need this, looking at patch 2/4, but the commit message isn't very clear. It would benefit from being reworded. Ack, thanks for the feedback. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 3 ++- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 3 ++- drivers/gpu/drm/drm_connector.c | 6 -- drivers/gpu/drm/i915/display/intel_dp.c | 3 ++- include/drm/drm_connector.h | 3 ++- 5 files changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c index d34037b85cf8..c18459ecd4be 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c @@ -2022,7 +2022,8 @@ amdgpu_connector_add(struct amdgpu_device *adev, if (connector_type == DRM_MODE_CONNECTOR_DisplayPort || connector_type == DRM_MODE_CONNECTOR_eDP) { - drm_connector_attach_dp_subconnector_property(_connector->base); + drm_connector_attach_dp_subconnector_property(_connector->base, + DRM_MODE_SUBCONNECTOR_Unknown); } return; diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c index 943959012d04..297321f0199e 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c @@ -759,7 +759,8 @@ void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm, drm_dp_mst_topology_mgr_init(>mst_mgr, adev_to_drm(dm->adev), >dm_dp_aux.aux, 16, 4, aconnector->connector_id); - drm_connector_attach_dp_subconnector_property(>base); + drm_connector_attach_dp_subconnector_property(>base, + DRM_MODE_SUBCONNECTOR_Unknown); } int dm_mst_get_pbn_divider(struct dc_link *link) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index a3d3e7dc08b2..a6066e4a5e9a 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1577,10 +1577,12 @@ EXPORT_SYMBOL(drm_mode_create_dvi_i_properties); /** * drm_connector_attach_dp_subconnector_property - create subconnector property for DP * @connector: drm_connector to attach property + * @subtype: initial value for the subconnector type * * Called by a driver when DP connector is created. */ -void drm_connector_attach_dp_subconnector_property(struct drm_connector *connector) +void drm_connector_attach_dp_subconnector_property(struct drm_connector *connector, + enum drm_mode_subconnector subtype) { struct drm_mode_config *mode_config = >dev->mode_config; @@ -1594,7 +1596,7 @@ void drm_connector_attach_dp_subconnector_property(struct drm_connector *connect drm_object_attach_property(>base, mode_config->dp_subconnector_property, - DRM_MODE_SUBCONNECTOR_Unknown); + subtype); } EXPORT_SYMBOL(drm_connector_attach_dp_subconnector_property); diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 474785110662..5819105187f6 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -5391,7 +5391,8 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect enum port port = dp_to_dig_port(intel_dp)->base.port; if (!intel_dp_is_edp(intel_dp)) - drm_connector_attach_dp_subconnector_property(connector); + drm_connector_attach_dp_subconnector_property(connector, + DRM_MODE_SUBCONNECTOR_Unknown); if (!IS_G4X(dev_priv) && port != PORT_A) intel_attach_force_audio_property(connector); diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 5a8115dca359..a130a78f6e0f 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -1990,7 +1990,8 @@ const char *drm_get_hdcp_content_type_name(int val); int drm_get_tv_mode_from_name(const char *name, size_t len); int drm_mode
Re: [Intel-gfx] [PATCH 3/4] drm/uapi: document the USB subconnector type
On 02/08/2023 21:55, Laurent Pinchart wrote: Hi Dmitry, Thank you for the patch. On Sat, Jul 29, 2023 at 03:49:12AM +0300, Dmitry Baryshkov wrote: To properly define the USB-C DP altmode connectors, add the USB subconnector type. Suggested-by: Simon Ser Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/drm_connector.c | 1 + include/uapi/drm/drm_mode.h | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index a6066e4a5e9a..9e96b038f5d0 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1050,6 +1050,7 @@ static const struct drm_prop_enum_list drm_dp_subconnector_enum_list[] = { { DRM_MODE_SUBCONNECTOR_DisplayPort, "DP"}, /* DP */ { DRM_MODE_SUBCONNECTOR_Wireless,"Wireless" }, /* DP */ { DRM_MODE_SUBCONNECTOR_Native, "Native"}, /* DP */ + { DRM_MODE_SUBCONNECTOR_USB, "USB" }, /* DP */ Should this be DRM_MODE_SUBCONNECTOR_USB_C and "USB-C", in case we get another USB type later ? Hmm, which id should I use for micro-USB then? (consider anx7808, SlimPort). I thought about using DRM_MODE_SUBCONNECTOR_USB for both of them. But maybe I should add another subtype for SlimPort. }; DRM_ENUM_NAME_FN(drm_get_dp_subconnector_name, diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 92d96a2b6763..0f74918b011c 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -398,6 +398,7 @@ enum drm_mode_subconnector { DRM_MODE_SUBCONNECTOR_HDMIA = 11, /*DP */ DRM_MODE_SUBCONNECTOR_Native = 15, /*DP */ DRM_MODE_SUBCONNECTOR_Wireless= 18, /*DP */ + DRM_MODE_SUBCONNECTOR_USB = 20, /*DP */ }; #define DRM_MODE_CONNECTOR_Unknown 0 -- With best wishes Dmitry
Re: [Intel-gfx] [PATCH 2/4] drm/bridge-connector: handle subconnector types
On 02/08/2023 21:46, Laurent Pinchart wrote: On Wed, Aug 02, 2023 at 12:05:50PM +0300, Dmitry Baryshkov wrote: On Wed, 2 Aug 2023 at 11:35, Neil Armstrong wrote: On 29/07/2023 02:49, Dmitry Baryshkov wrote: If the created connector type supports subconnector type property, create and attach corresponding it. The default subtype value is 0, which maps to the DRM_MODE_SUBCONNECTOR_Unknown type. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/drm_bridge_connector.c | 33 +- include/drm/drm_bridge.h | 4 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c index 07b5930b1282..a7b92f0d2430 100644 --- a/drivers/gpu/drm/drm_bridge_connector.c +++ b/drivers/gpu/drm/drm_bridge_connector.c @@ -329,7 +329,9 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, struct drm_connector *connector; struct i2c_adapter *ddc = NULL; struct drm_bridge *bridge, *panel_bridge = NULL; + enum drm_mode_subconnector subconnector; int connector_type; + int ret; bridge_connector = kzalloc(sizeof(*bridge_connector), GFP_KERNEL); if (!bridge_connector) @@ -365,8 +367,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, if (bridge->ops & DRM_BRIDGE_OP_MODES) bridge_connector->bridge_modes = bridge; - if (!drm_bridge_get_next_bridge(bridge)) + if (!drm_bridge_get_next_bridge(bridge)) { connector_type = bridge->type; + subconnector = bridge->subtype; + } #ifdef CONFIG_OF if (!drm_bridge_get_next_bridge(bridge) && @@ -399,6 +403,33 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, if (panel_bridge) drm_panel_bridge_set_orientation(connector, panel_bridge); + if (connector_type == DRM_MODE_CONNECTOR_DisplayPort) { + drm_connector_attach_dp_subconnector_property(connector, subconnector); + } else if (connector_type == DRM_MODE_CONNECTOR_DVII) { + ret = drm_mode_create_dvi_i_properties(drm); + if (ret) + return ERR_PTR(ret); + + drm_object_attach_property(>base, + drm->mode_config.dvi_i_subconnector_property, +subconnector); + } else if (connector_type == DRM_MODE_CONNECTOR_TV) { + ret = drm_mode_create_tv_properties(drm, + BIT(DRM_MODE_TV_MODE_NTSC) | + BIT(DRM_MODE_TV_MODE_NTSC_443) | + BIT(DRM_MODE_TV_MODE_NTSC_J) | + BIT(DRM_MODE_TV_MODE_PAL) | + BIT(DRM_MODE_TV_MODE_PAL_M) | + BIT(DRM_MODE_TV_MODE_PAL_N) | + BIT(DRM_MODE_TV_MODE_SECAM)); + if (ret) + return ERR_PTR(ret); I don't think this is right, this should be called from the appropriate encoder device depending on the analog tv mode capabilities. Good question. My logic was the following: the DRM device can have different TV out ports with different capabilities (yeah, pure theoretical construct). In this case it might be impossible to create a single subset of values. Thus it is more correct to create the property listing all possible values. The property is immutable anyway (and so the user doesn't have control over the value). Those ports would correspond to different connectors, so I agree with Neil, I don't think it's right to create a single property with all modes and attach it to all analog output connectors. I agree that this case is mishandled currently (as current design assumes a single property that fits for the complete device). If you want to support multiple analog outputs that have different capabilities, this will need changes to drm_mode_create_tv_properties() to allow creating multiple properties. If you don't want to do so now, and prefer limiting support to devices where all ports support the same modes (which includes devices with a single analog output), then the modes should reflect what the device supports. Ack, I'll drop the create call and check for the property existence before attaching it. + + drm_object_attach_property(>base, + drm->mode_config.tv_subconnector_property, +subconnector); Here, only add the property if drm->mode_config.tv_subconnector_property exists, and perhaps add a warning if not. This property is created in the previ
Re: [Intel-gfx] [PATCH 2/4] drm/bridge-connector: handle subconnector types
On Wed, 2 Aug 2023 at 11:35, Neil Armstrong wrote: > > On 29/07/2023 02:49, Dmitry Baryshkov wrote: > > If the created connector type supports subconnector type property, > > create and attach corresponding it. The default subtype value is 0, > > which maps to the DRM_MODE_SUBCONNECTOR_Unknown type. > > > > Signed-off-by: Dmitry Baryshkov > > --- > > drivers/gpu/drm/drm_bridge_connector.c | 33 +- > > include/drm/drm_bridge.h | 4 > > 2 files changed, 36 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_bridge_connector.c > > b/drivers/gpu/drm/drm_bridge_connector.c > > index 07b5930b1282..a7b92f0d2430 100644 > > --- a/drivers/gpu/drm/drm_bridge_connector.c > > +++ b/drivers/gpu/drm/drm_bridge_connector.c > > @@ -329,7 +329,9 @@ struct drm_connector *drm_bridge_connector_init(struct > > drm_device *drm, > > struct drm_connector *connector; > > struct i2c_adapter *ddc = NULL; > > struct drm_bridge *bridge, *panel_bridge = NULL; > > + enum drm_mode_subconnector subconnector; > > int connector_type; > > + int ret; > > > > bridge_connector = kzalloc(sizeof(*bridge_connector), GFP_KERNEL); > > if (!bridge_connector) > > @@ -365,8 +367,10 @@ struct drm_connector *drm_bridge_connector_init(struct > > drm_device *drm, > > if (bridge->ops & DRM_BRIDGE_OP_MODES) > > bridge_connector->bridge_modes = bridge; > > > > - if (!drm_bridge_get_next_bridge(bridge)) > > + if (!drm_bridge_get_next_bridge(bridge)) { > > connector_type = bridge->type; > > + subconnector = bridge->subtype; > > + } > > > > #ifdef CONFIG_OF > > if (!drm_bridge_get_next_bridge(bridge) && > > @@ -399,6 +403,33 @@ struct drm_connector *drm_bridge_connector_init(struct > > drm_device *drm, > > if (panel_bridge) > > drm_panel_bridge_set_orientation(connector, panel_bridge); > > > > + if (connector_type == DRM_MODE_CONNECTOR_DisplayPort) { > > + drm_connector_attach_dp_subconnector_property(connector, > > subconnector); > > + } else if (connector_type == DRM_MODE_CONNECTOR_DVII) { > > + ret = drm_mode_create_dvi_i_properties(drm); > > + if (ret) > > + return ERR_PTR(ret); > > + > > + drm_object_attach_property(>base, > > + > > drm->mode_config.dvi_i_subconnector_property, > > +subconnector); > > + } else if (connector_type == DRM_MODE_CONNECTOR_TV) { > > + ret = drm_mode_create_tv_properties(drm, > > + > > BIT(DRM_MODE_TV_MODE_NTSC) | > > + > > BIT(DRM_MODE_TV_MODE_NTSC_443) | > > + > > BIT(DRM_MODE_TV_MODE_NTSC_J) | > > + BIT(DRM_MODE_TV_MODE_PAL) > > | > > + > > BIT(DRM_MODE_TV_MODE_PAL_M) | > > + > > BIT(DRM_MODE_TV_MODE_PAL_N) | > > + > > BIT(DRM_MODE_TV_MODE_SECAM)); > > + if (ret) > > + return ERR_PTR(ret); > > I don't think this is right, this should be called from the appropriate > encoder > device depending on the analog tv mode capabilities. Good question. My logic was the following: the DRM device can have different TV out ports with different capabilities (yeah, pure theoretical construct). In this case it might be impossible to create a single subset of values. Thus it is more correct to create the property listing all possible values. The property is immutable anyway (and so the user doesn't have control over the value). > > + > > + drm_object_attach_property(>base, > > + > > drm->mode_config.tv_subconnector_property, > > +subconnector); > > Here, only add the property if drm->mode_config.tv_subconnector_property > exists, > and perhaps add a warning if not. This property is created in the previous call, drm_mode_create_tv_properties() -> drm_mode_create_tv_properties_legacy(). > > AFAI
[Intel-gfx] [PATCH 4/4] soc: qcom: pmic_glink: properly describe the DP connector
During the discussion of the DP connectors, it was suggested that USB-C DisplayPort connectors should have DRM_MODE_CONNECTOR_DisplayPort connector type. This follows the example provided by other drivers (AMDGPU, Intel). To distinguish them from native DP ports, they should have the freshly defined USB as a subconnector type. Suggested-by: Simon Ser Cc: Neil Armstrong Signed-off-by: Dmitry Baryshkov --- drivers/soc/qcom/pmic_glink_altmode.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/soc/qcom/pmic_glink_altmode.c b/drivers/soc/qcom/pmic_glink_altmode.c index 1dedacc52aea..9094944c6cc0 100644 --- a/drivers/soc/qcom/pmic_glink_altmode.c +++ b/drivers/soc/qcom/pmic_glink_altmode.c @@ -417,7 +417,8 @@ static int pmic_glink_altmode_probe(struct auxiliary_device *adev, alt_port->bridge.funcs = _glink_altmode_bridge_funcs; alt_port->bridge.of_node = to_of_node(fwnode); alt_port->bridge.ops = DRM_BRIDGE_OP_HPD; - alt_port->bridge.type = DRM_MODE_CONNECTOR_USB; + alt_port->bridge.type = DRM_MODE_CONNECTOR_DisplayPort; + alt_port->bridge.subtype = DRM_MODE_SUBCONNECTOR_USB; ret = devm_drm_bridge_add(dev, _port->bridge); if (ret) -- 2.39.2
[Intel-gfx] [PATCH 3/4] drm/uapi: document the USB subconnector type
To properly define the USB-C DP altmode connectors, add the USB subconnector type. Suggested-by: Simon Ser Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/drm_connector.c | 1 + include/uapi/drm/drm_mode.h | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index a6066e4a5e9a..9e96b038f5d0 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1050,6 +1050,7 @@ static const struct drm_prop_enum_list drm_dp_subconnector_enum_list[] = { { DRM_MODE_SUBCONNECTOR_DisplayPort, "DP"}, /* DP */ { DRM_MODE_SUBCONNECTOR_Wireless,"Wireless" }, /* DP */ { DRM_MODE_SUBCONNECTOR_Native, "Native"}, /* DP */ + { DRM_MODE_SUBCONNECTOR_USB, "USB" }, /* DP */ }; DRM_ENUM_NAME_FN(drm_get_dp_subconnector_name, diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 92d96a2b6763..0f74918b011c 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -398,6 +398,7 @@ enum drm_mode_subconnector { DRM_MODE_SUBCONNECTOR_HDMIA = 11, /*DP */ DRM_MODE_SUBCONNECTOR_Native = 15, /*DP */ DRM_MODE_SUBCONNECTOR_Wireless= 18, /*DP */ + DRM_MODE_SUBCONNECTOR_USB = 20, /*DP */ }; #define DRM_MODE_CONNECTOR_Unknown 0 -- 2.39.2
[Intel-gfx] [PATCH 2/4] drm/bridge-connector: handle subconnector types
If the created connector type supports subconnector type property, create and attach corresponding it. The default subtype value is 0, which maps to the DRM_MODE_SUBCONNECTOR_Unknown type. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/drm_bridge_connector.c | 33 +- include/drm/drm_bridge.h | 4 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c index 07b5930b1282..a7b92f0d2430 100644 --- a/drivers/gpu/drm/drm_bridge_connector.c +++ b/drivers/gpu/drm/drm_bridge_connector.c @@ -329,7 +329,9 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, struct drm_connector *connector; struct i2c_adapter *ddc = NULL; struct drm_bridge *bridge, *panel_bridge = NULL; + enum drm_mode_subconnector subconnector; int connector_type; + int ret; bridge_connector = kzalloc(sizeof(*bridge_connector), GFP_KERNEL); if (!bridge_connector) @@ -365,8 +367,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, if (bridge->ops & DRM_BRIDGE_OP_MODES) bridge_connector->bridge_modes = bridge; - if (!drm_bridge_get_next_bridge(bridge)) + if (!drm_bridge_get_next_bridge(bridge)) { connector_type = bridge->type; + subconnector = bridge->subtype; + } #ifdef CONFIG_OF if (!drm_bridge_get_next_bridge(bridge) && @@ -399,6 +403,33 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, if (panel_bridge) drm_panel_bridge_set_orientation(connector, panel_bridge); + if (connector_type == DRM_MODE_CONNECTOR_DisplayPort) { + drm_connector_attach_dp_subconnector_property(connector, subconnector); + } else if (connector_type == DRM_MODE_CONNECTOR_DVII) { + ret = drm_mode_create_dvi_i_properties(drm); + if (ret) + return ERR_PTR(ret); + + drm_object_attach_property(>base, + drm->mode_config.dvi_i_subconnector_property, + subconnector); + } else if (connector_type == DRM_MODE_CONNECTOR_TV) { + ret = drm_mode_create_tv_properties(drm, + BIT(DRM_MODE_TV_MODE_NTSC) | + BIT(DRM_MODE_TV_MODE_NTSC_443) | + BIT(DRM_MODE_TV_MODE_NTSC_J) | + BIT(DRM_MODE_TV_MODE_PAL) | + BIT(DRM_MODE_TV_MODE_PAL_M) | + BIT(DRM_MODE_TV_MODE_PAL_N) | + BIT(DRM_MODE_TV_MODE_SECAM)); + if (ret) + return ERR_PTR(ret); + + drm_object_attach_property(>base, + drm->mode_config.tv_subconnector_property, + subconnector); + } + return connector; } EXPORT_SYMBOL_GPL(drm_bridge_connector_init); diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index bf964cdfb330..68b14ac5ac0d 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -739,6 +739,10 @@ struct drm_bridge { * identifies the type of connected display. */ int type; + /** +* @subtype: the subtype of the connector for the DP/TV/DVI-I cases. +*/ + enum drm_mode_subconnector subtype; /** * @interlace_allowed: Indicate that the bridge can handle interlaced * modes. -- 2.39.2
[Intel-gfx] [PATCH 1/4] drm: allow specifying default subtype for the DP subconnector property
In the embedded usecases the default subtype depends on the bridge chain, so it is easier to specify the subtype at the proprety attachment type rather than specifying it later. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 3 ++- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 3 ++- drivers/gpu/drm/drm_connector.c | 6 -- drivers/gpu/drm/i915/display/intel_dp.c | 3 ++- include/drm/drm_connector.h | 3 ++- 5 files changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c index d34037b85cf8..c18459ecd4be 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c @@ -2022,7 +2022,8 @@ amdgpu_connector_add(struct amdgpu_device *adev, if (connector_type == DRM_MODE_CONNECTOR_DisplayPort || connector_type == DRM_MODE_CONNECTOR_eDP) { - drm_connector_attach_dp_subconnector_property(_connector->base); + drm_connector_attach_dp_subconnector_property(_connector->base, + DRM_MODE_SUBCONNECTOR_Unknown); } return; diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c index 943959012d04..297321f0199e 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c @@ -759,7 +759,8 @@ void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm, drm_dp_mst_topology_mgr_init(>mst_mgr, adev_to_drm(dm->adev), >dm_dp_aux.aux, 16, 4, aconnector->connector_id); - drm_connector_attach_dp_subconnector_property(>base); + drm_connector_attach_dp_subconnector_property(>base, + DRM_MODE_SUBCONNECTOR_Unknown); } int dm_mst_get_pbn_divider(struct dc_link *link) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index a3d3e7dc08b2..a6066e4a5e9a 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1577,10 +1577,12 @@ EXPORT_SYMBOL(drm_mode_create_dvi_i_properties); /** * drm_connector_attach_dp_subconnector_property - create subconnector property for DP * @connector: drm_connector to attach property + * @subtype: initial value for the subconnector type * * Called by a driver when DP connector is created. */ -void drm_connector_attach_dp_subconnector_property(struct drm_connector *connector) +void drm_connector_attach_dp_subconnector_property(struct drm_connector *connector, + enum drm_mode_subconnector subtype) { struct drm_mode_config *mode_config = >dev->mode_config; @@ -1594,7 +1596,7 @@ void drm_connector_attach_dp_subconnector_property(struct drm_connector *connect drm_object_attach_property(>base, mode_config->dp_subconnector_property, - DRM_MODE_SUBCONNECTOR_Unknown); + subtype); } EXPORT_SYMBOL(drm_connector_attach_dp_subconnector_property); diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 474785110662..5819105187f6 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -5391,7 +5391,8 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect enum port port = dp_to_dig_port(intel_dp)->base.port; if (!intel_dp_is_edp(intel_dp)) - drm_connector_attach_dp_subconnector_property(connector); + drm_connector_attach_dp_subconnector_property(connector, + DRM_MODE_SUBCONNECTOR_Unknown); if (!IS_G4X(dev_priv) && port != PORT_A) intel_attach_force_audio_property(connector); diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 5a8115dca359..a130a78f6e0f 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -1990,7 +1990,8 @@ const char *drm_get_hdcp_content_type_name(int val); int drm_get_tv_mode_from_name(const char *name, size_t len); int drm_mode_create_dvi_i_properties(struct drm_device *dev); -void drm_connector_attach_dp_subconnector_property(struct drm_connector *connector); +void drm_connector_attach_dp_subconnector_property(struct drm_connector *connector, + enum drm_mode_subconnector subtype); int drm_mode_create_tv_margin_properties(struct drm_device *dev);
[Intel-gfx] [PATCH 0/4] drm/bridge-connector: simplify handling of USB-C DP
During the discussion of DP connetors, it was pointed out that existing DP drivers supporting USB-C altmode (AMDGPU, Intel) use DRM_MODE_CONNECTOR_DisplayPort for such connectors rather than DRM_MODE_CONNECTOR_USB. This patchset attempts to solve this issue. It adds USB to the enum drm_dp_subconnector and then provides a simple yet efficient way to define DP-in-USB subconnector type for the drivers that use drm_bridge_connector. Dmitry Baryshkov (4): drm: allow specifying default subtype for the DP subconnector property drm/bridge-connector: handle subconnector types drm/uapi: document the USB subconnector type soc: qcom: pmic_glink: properly describe the DP connector .../gpu/drm/amd/amdgpu/amdgpu_connectors.c| 3 +- .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 3 +- drivers/gpu/drm/drm_bridge_connector.c| 33 ++- drivers/gpu/drm/drm_connector.c | 7 ++-- drivers/gpu/drm/i915/display/intel_dp.c | 3 +- drivers/soc/qcom/pmic_glink_altmode.c | 3 +- include/drm/drm_bridge.h | 4 +++ include/drm/drm_connector.h | 3 +- include/uapi/drm/drm_mode.h | 1 + 9 files changed, 52 insertions(+), 8 deletions(-) -- 2.39.2
[Intel-gfx] [PATCH v6 2/3] drm/bridge_connector: stop filtering events in drm_bridge_connector_hpd_cb()
In some cases the bridge drivers would like to receive hotplug events even in the case new status is equal to the old status. In the DP case this is used to deliver "attention" messages to the DP host. Stop filtering the events in the drm_bridge_connector_hpd_cb() and let drivers decide whether they would like to receive the event or not. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/drm_bridge_connector.c | 5 - 1 file changed, 5 deletions(-) diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c index 19ae4a177ac3..84d8d310ef04 100644 --- a/drivers/gpu/drm/drm_bridge_connector.c +++ b/drivers/gpu/drm/drm_bridge_connector.c @@ -113,16 +113,11 @@ static void drm_bridge_connector_hpd_cb(void *cb_data, struct drm_bridge_connector *drm_bridge_connector = cb_data; struct drm_connector *connector = _bridge_connector->base; struct drm_device *dev = connector->dev; - enum drm_connector_status old_status; mutex_lock(>mode_config.mutex); - old_status = connector->status; connector->status = status; mutex_unlock(>mode_config.mutex); - if (old_status == status) - return; - drm_bridge_connector_hpd_notify(connector, status); drm_kms_helper_hotplug_event(dev); -- 2.39.2
[Intel-gfx] [PATCH v6 3/3] drm/bridge_connector: implement oob_hotplug_event
Implement the oob_hotplug_event() callback. Translate it to the HPD notification sent to the HPD bridge in the chain. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/drm_bridge_connector.c | 29 +++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c index 84d8d310ef04..364f6e37fbdc 100644 --- a/drivers/gpu/drm/drm_bridge_connector.c +++ b/drivers/gpu/drm/drm_bridge_connector.c @@ -5,6 +5,8 @@ #include #include +#include +#include #include #include @@ -107,10 +109,9 @@ static void drm_bridge_connector_hpd_notify(struct drm_connector *connector, } } -static void drm_bridge_connector_hpd_cb(void *cb_data, - enum drm_connector_status status) +static void drm_bridge_connector_handle_hpd(struct drm_bridge_connector *drm_bridge_connector, + enum drm_connector_status status) { - struct drm_bridge_connector *drm_bridge_connector = cb_data; struct drm_connector *connector = _bridge_connector->base; struct drm_device *dev = connector->dev; @@ -123,6 +124,21 @@ static void drm_bridge_connector_hpd_cb(void *cb_data, drm_kms_helper_hotplug_event(dev); } +static void drm_bridge_connector_hpd_cb(void *cb_data, + enum drm_connector_status status) +{ + drm_bridge_connector_handle_hpd(cb_data, status); +} + +static void drm_bridge_connector_oob_hotplug_event(struct drm_connector *connector, + enum drm_connector_status status) +{ + struct drm_bridge_connector *bridge_connector = + to_drm_bridge_connector(connector); + + drm_bridge_connector_handle_hpd(bridge_connector, status); +} + static void drm_bridge_connector_enable_hpd(struct drm_connector *connector) { struct drm_bridge_connector *bridge_connector = @@ -216,6 +232,7 @@ static const struct drm_connector_funcs drm_bridge_connector_funcs = { .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, .debugfs_init = drm_bridge_connector_debugfs_init, + .oob_hotplug_event = drm_bridge_connector_oob_hotplug_event, }; /* - @@ -351,6 +368,12 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, if (!drm_bridge_get_next_bridge(bridge)) connector_type = bridge->type; +#ifdef CONFIG_OF + if (!drm_bridge_get_next_bridge(bridge) && + bridge->of_node) + connector->fwnode = fwnode_handle_get(of_fwnode_handle(bridge->of_node)); +#endif + if (bridge->ddc) ddc = bridge->ddc; -- 2.39.2
[Intel-gfx] [PATCH v6 1/3] drm: Add HPD state to drm_connector_oob_hotplug_event()
From: Bjorn Andersson In some implementations, such as the Qualcomm platforms, the display driver has no way to query the current HPD state and as such it's impossible to distinguish between disconnect and attention events. Add a parameter to drm_connector_oob_hotplug_event() to pass the HPD state. Also push the test for unchanged state in the displayport altmode driver into the i915 driver, to allow other drivers to act upon each update. Signed-off-by: Bjorn Andersson Reviewed-by: Hans de Goede Acked-by: Heikki Krogerus Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/drm_connector.c | 6 -- .../gpu/drm/i915/display/intel_display_core.h | 3 +++ drivers/gpu/drm/i915/display/intel_dp.c | 17 ++--- drivers/usb/typec/altmodes/displayport.c| 14 +++--- include/drm/drm_connector.h | 6 -- 5 files changed, 32 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 3ed4cfcb350c..1128149c74f2 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -3049,6 +3049,7 @@ struct drm_connector *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode) /** * drm_connector_oob_hotplug_event - Report out-of-band hotplug event to connector * @connector_fwnode: fwnode_handle to report the event on + * @status: hot plug detect logical state * * On some hardware a hotplug event notification may come from outside the display * driver / device. An example of this is some USB Type-C setups where the hardware @@ -3058,7 +3059,8 @@ struct drm_connector *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode) * This function can be used to report these out-of-band events after obtaining * a drm_connector reference through calling drm_connector_find_by_fwnode(). */ -void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode) +void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode, +enum drm_connector_status status) { struct drm_connector *connector; @@ -3067,7 +3069,7 @@ void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode) return; if (connector->funcs->oob_hotplug_event) - connector->funcs->oob_hotplug_event(connector); + connector->funcs->oob_hotplug_event(connector, status); drm_connector_put(connector); } diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h index 8d2243c71dd8..419efee5df74 100644 --- a/drivers/gpu/drm/i915/display/intel_display_core.h +++ b/drivers/gpu/drm/i915/display/intel_display_core.h @@ -175,6 +175,9 @@ struct intel_hotplug { /* Whether or not to count short HPD IRQs in HPD storms */ u8 hpd_short_storm_enabled; + /* Last state reported by oob_hotplug_event for each encoder */ + unsigned long oob_hotplug_last_state; + /* * if we get a HPD irq from DP and a HPD irq from non-DP * the non-DP HPD could block the workqueue on a mode config diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 9f40da20e88d..cf633f0df6ff 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -5244,15 +5244,26 @@ static int intel_dp_connector_atomic_check(struct drm_connector *conn, return intel_modeset_synced_crtcs(state, conn); } -static void intel_dp_oob_hotplug_event(struct drm_connector *connector) +static void intel_dp_oob_hotplug_event(struct drm_connector *connector, + enum drm_connector_status hpd_state) { struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector)); struct drm_i915_private *i915 = to_i915(connector->dev); + bool hpd_high = hpd_state == connector_status_connected; + unsigned int hpd_pin = encoder->hpd_pin; + bool need_work = false; spin_lock_irq(>irq_lock); - i915->display.hotplug.event_bits |= BIT(encoder->hpd_pin); + if (hpd_high != test_bit(hpd_pin, >display.hotplug.oob_hotplug_last_state)) { + i915->display.hotplug.event_bits |= BIT(hpd_pin); + + __assign_bit(hpd_pin, >display.hotplug.oob_hotplug_last_state, hpd_high); + need_work = true; + } spin_unlock_irq(>irq_lock); - queue_delayed_work(i915->unordered_wq, >display.hotplug.hotplug_work, 0); + + if (need_work) + queue_delayed_work(i915->unordered_wq, >display.hotplug.hotplug_work, 0); } static const struct drm_connector_funcs intel_dp_connector_funcs = { diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c index 66de880b28d0..dc2d3a83d
[Intel-gfx] [PATCH v6 0/3] drm/bridge_connector: implement OOB HPD handling
Note, numbering for this series starts from v5, since there were several revisions for this patchset under a different series title ([1]). USB altmodes code would send OOB notifications to the drm_connector specified in the device tree. However as the MSM DP driver uses drm_bridge_connector, there is no way to receive these event directly. Implement a bridge between oob_hotplug_event and drm_bridge's hpd_notify. Merge strategy: since this series touches i915 code, it might make sense to merge it through drm-intel. [1] https://patchwork.freedesktop.org/series/103449/ Changes since v5: - Fixed checkpatch warning in the first patch (noted by intel-gfx CI). Changes since v4: - Picked up the patchset - Dropped msm-specific patches - Changed drm_bridge_connector_oob_hotplug_event to call connector's HPD callback directly, rather than going through the last bridge's hpd_notify - Added proper fwnode for the drm_bridge_connector Bjorn Andersson (1): drm: Add HPD state to drm_connector_oob_hotplug_event() Dmitry Baryshkov (2): drm/bridge_connector: stop filtering events in drm_bridge_connector_hpd_cb() drm/bridge_connector: implement oob_hotplug_event drivers/gpu/drm/drm_bridge_connector.c| 34 ++- drivers/gpu/drm/drm_connector.c | 6 ++-- .../gpu/drm/i915/display/intel_display_core.h | 3 ++ drivers/gpu/drm/i915/display/intel_dp.c | 17 -- drivers/usb/typec/altmodes/displayport.c | 14 include/drm/drm_connector.h | 6 ++-- 6 files changed, 58 insertions(+), 22 deletions(-) -- 2.39.2
[Intel-gfx] [PATCH v5 1/3] drm: Add HPD state to drm_connector_oob_hotplug_event()
From: Bjorn Andersson In some implementations, such as the Qualcomm platforms, the display driver has no way to query the current HPD state and as such it's impossible to distinguish between disconnect and attention events. Add a parameter to drm_connector_oob_hotplug_event() to pass the HPD state. Also push the test for unchanged state in the displayport altmode driver into the i915 driver, to allow other drivers to act upon each update. Signed-off-by: Bjorn Andersson Reviewed-by: Hans de Goede Acked-by: Heikki Krogerus Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/drm_connector.c | 6 -- .../gpu/drm/i915/display/intel_display_core.h | 3 +++ drivers/gpu/drm/i915/display/intel_dp.c | 17 ++--- drivers/usb/typec/altmodes/displayport.c| 13 ++--- include/drm/drm_connector.h | 6 -- 5 files changed, 31 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 3ed4cfcb350c..1128149c74f2 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -3049,6 +3049,7 @@ struct drm_connector *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode) /** * drm_connector_oob_hotplug_event - Report out-of-band hotplug event to connector * @connector_fwnode: fwnode_handle to report the event on + * @status: hot plug detect logical state * * On some hardware a hotplug event notification may come from outside the display * driver / device. An example of this is some USB Type-C setups where the hardware @@ -3058,7 +3059,8 @@ struct drm_connector *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode) * This function can be used to report these out-of-band events after obtaining * a drm_connector reference through calling drm_connector_find_by_fwnode(). */ -void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode) +void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode, +enum drm_connector_status status) { struct drm_connector *connector; @@ -3067,7 +3069,7 @@ void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode) return; if (connector->funcs->oob_hotplug_event) - connector->funcs->oob_hotplug_event(connector); + connector->funcs->oob_hotplug_event(connector, status); drm_connector_put(connector); } diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h index 8d2243c71dd8..419efee5df74 100644 --- a/drivers/gpu/drm/i915/display/intel_display_core.h +++ b/drivers/gpu/drm/i915/display/intel_display_core.h @@ -175,6 +175,9 @@ struct intel_hotplug { /* Whether or not to count short HPD IRQs in HPD storms */ u8 hpd_short_storm_enabled; + /* Last state reported by oob_hotplug_event for each encoder */ + unsigned long oob_hotplug_last_state; + /* * if we get a HPD irq from DP and a HPD irq from non-DP * the non-DP HPD could block the workqueue on a mode config diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 9f40da20e88d..cf633f0df6ff 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -5244,15 +5244,26 @@ static int intel_dp_connector_atomic_check(struct drm_connector *conn, return intel_modeset_synced_crtcs(state, conn); } -static void intel_dp_oob_hotplug_event(struct drm_connector *connector) +static void intel_dp_oob_hotplug_event(struct drm_connector *connector, + enum drm_connector_status hpd_state) { struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector)); struct drm_i915_private *i915 = to_i915(connector->dev); + bool hpd_high = hpd_state == connector_status_connected; + unsigned int hpd_pin = encoder->hpd_pin; + bool need_work = false; spin_lock_irq(>irq_lock); - i915->display.hotplug.event_bits |= BIT(encoder->hpd_pin); + if (hpd_high != test_bit(hpd_pin, >display.hotplug.oob_hotplug_last_state)) { + i915->display.hotplug.event_bits |= BIT(hpd_pin); + + __assign_bit(hpd_pin, >display.hotplug.oob_hotplug_last_state, hpd_high); + need_work = true; + } spin_unlock_irq(>irq_lock); - queue_delayed_work(i915->unordered_wq, >display.hotplug.hotplug_work, 0); + + if (need_work) + queue_delayed_work(i915->unordered_wq, >display.hotplug.hotplug_work, 0); } static const struct drm_connector_funcs intel_dp_connector_funcs = { diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c index 66de880b28d0..4e5aa17ce4c8 10
[Intel-gfx] [PATCH v5 0/3] drm/bridge_connector: implement OOB HPD handling
Note, this is sent as v5, since there were several revisions for this patchset under a different series title ([1]). USB altmodes code would send OOB notifications to the drm_connector specified in the device tree. However as the MSM DP driver uses drm_bridge_connector, there is no way to receive these event directly. Implement a bridge between oob_hotplug_event and drm_bridge's hpd_notify. Merge strategy: since this series touches i915 code, it might make sense to merge it through drm-intel. [1] https://patchwork.freedesktop.org/series/103449/ Changes since v4: - Picked up the patchset - Dropped msm-specific patches - Changed drm_bridge_connector_oob_hotplug_event to call connector's HPD callback directly, rather than going through the last bridge's hpd_notify - Added proper fwnode for the drm_bridge_connector Bjorn Andersson (1): drm: Add HPD state to drm_connector_oob_hotplug_event() Dmitry Baryshkov (2): drm/bridge_connector: stop filtering events in drm_bridge_connector_hpd_cb() drm/bridge_connector: implement oob_hotplug_event drivers/gpu/drm/drm_bridge_connector.c| 34 ++- drivers/gpu/drm/drm_connector.c | 6 ++-- .../gpu/drm/i915/display/intel_display_core.h | 3 ++ drivers/gpu/drm/i915/display/intel_dp.c | 17 -- drivers/usb/typec/altmodes/displayport.c | 13 --- include/drm/drm_connector.h | 6 ++-- 6 files changed, 57 insertions(+), 22 deletions(-) -- 2.39.2
[Intel-gfx] [PATCH v5 2/3] drm/bridge_connector: stop filtering events in drm_bridge_connector_hpd_cb()
In some cases the bridge drivers would like to receive hotplug events even in the case new status is equal to the old status. In the DP case this is used to deliver "attention" messages to the DP host. Stop filtering the events in the drm_bridge_connector_hpd_cb() and let drivers decide whether they would like to receive the event or not. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/drm_bridge_connector.c | 5 - 1 file changed, 5 deletions(-) diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c index 19ae4a177ac3..84d8d310ef04 100644 --- a/drivers/gpu/drm/drm_bridge_connector.c +++ b/drivers/gpu/drm/drm_bridge_connector.c @@ -113,16 +113,11 @@ static void drm_bridge_connector_hpd_cb(void *cb_data, struct drm_bridge_connector *drm_bridge_connector = cb_data; struct drm_connector *connector = _bridge_connector->base; struct drm_device *dev = connector->dev; - enum drm_connector_status old_status; mutex_lock(>mode_config.mutex); - old_status = connector->status; connector->status = status; mutex_unlock(>mode_config.mutex); - if (old_status == status) - return; - drm_bridge_connector_hpd_notify(connector, status); drm_kms_helper_hotplug_event(dev); -- 2.39.2
[Intel-gfx] [PATCH v5 3/3] drm/bridge_connector: implement oob_hotplug_event
Implement the oob_hotplug_event() callback. Translate it to the HPD notification sent to the HPD bridge in the chain. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/drm_bridge_connector.c | 29 +++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c index 84d8d310ef04..364f6e37fbdc 100644 --- a/drivers/gpu/drm/drm_bridge_connector.c +++ b/drivers/gpu/drm/drm_bridge_connector.c @@ -5,6 +5,8 @@ #include #include +#include +#include #include #include @@ -107,10 +109,9 @@ static void drm_bridge_connector_hpd_notify(struct drm_connector *connector, } } -static void drm_bridge_connector_hpd_cb(void *cb_data, - enum drm_connector_status status) +static void drm_bridge_connector_handle_hpd(struct drm_bridge_connector *drm_bridge_connector, + enum drm_connector_status status) { - struct drm_bridge_connector *drm_bridge_connector = cb_data; struct drm_connector *connector = _bridge_connector->base; struct drm_device *dev = connector->dev; @@ -123,6 +124,21 @@ static void drm_bridge_connector_hpd_cb(void *cb_data, drm_kms_helper_hotplug_event(dev); } +static void drm_bridge_connector_hpd_cb(void *cb_data, + enum drm_connector_status status) +{ + drm_bridge_connector_handle_hpd(cb_data, status); +} + +static void drm_bridge_connector_oob_hotplug_event(struct drm_connector *connector, + enum drm_connector_status status) +{ + struct drm_bridge_connector *bridge_connector = + to_drm_bridge_connector(connector); + + drm_bridge_connector_handle_hpd(bridge_connector, status); +} + static void drm_bridge_connector_enable_hpd(struct drm_connector *connector) { struct drm_bridge_connector *bridge_connector = @@ -216,6 +232,7 @@ static const struct drm_connector_funcs drm_bridge_connector_funcs = { .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, .debugfs_init = drm_bridge_connector_debugfs_init, + .oob_hotplug_event = drm_bridge_connector_oob_hotplug_event, }; /* - @@ -351,6 +368,12 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, if (!drm_bridge_get_next_bridge(bridge)) connector_type = bridge->type; +#ifdef CONFIG_OF + if (!drm_bridge_get_next_bridge(bridge) && + bridge->of_node) + connector->fwnode = fwnode_handle_get(of_fwnode_handle(bridge->of_node)); +#endif + if (bridge->ddc) ddc = bridge->ddc; -- 2.39.2
Re: [Intel-gfx] [Freedreno] [PATCH v3 11/12] drm/fbdev-generic: Implement dedicated fbdev I/O helpers
On Mon, 22 May 2023 at 15:22, Thomas Zimmermann wrote: > > Implement dedicated fbdev helpers for framebuffer I/O instead > of using DRM's helpers. Fbdev-generic was the only caller of the > DRM helpers, so remove them from the helper module. > > v2: > * use FB_SYS_HELPERS_DEFERRED option > > Signed-off-by: Thomas Zimmermann > --- > drivers/gpu/drm/Kconfig | 6 +- > drivers/gpu/drm/drm_fb_helper.c | 107 > drivers/gpu/drm/drm_fbdev_generic.c | 47 ++-- > include/drm/drm_fb_helper.h | 41 --- > 4 files changed, 43 insertions(+), 158 deletions(-) Looking at this patch makes me wonder if we should have implemented fb_dirty for the MSM driver. We have drm_framebuffer_funcs::dirty() implemented (by wrapping the drm_atomic_helper_dirtyfb()). > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 77fb10ddd8a2..92a782827b7b 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -95,6 +95,7 @@ config DRM_KUNIT_TEST > config DRM_KMS_HELPER > tristate > depends on DRM > + select FB_SYS_HELPERS_DEFERRED if DRM_FBDEV_EMULATION > help > CRTC helpers for KMS drivers. > > @@ -135,11 +136,6 @@ config DRM_FBDEV_EMULATION > select FB_CFB_FILLRECT > select FB_CFB_COPYAREA > select FB_CFB_IMAGEBLIT > - select FB_DEFERRED_IO > - select FB_SYS_FOPS > - select FB_SYS_FILLRECT > - select FB_SYS_COPYAREA > - select FB_SYS_IMAGEBLIT > select FRAMEBUFFER_CONSOLE if !EXPERT > select FRAMEBUFFER_CONSOLE_DETECT_PRIMARY if FRAMEBUFFER_CONSOLE > default y > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index 8724e08c518b..ba0a808f14ee 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -729,113 +729,6 @@ void drm_fb_helper_deferred_io(struct fb_info *info, > struct list_head *pagerefli > } > EXPORT_SYMBOL(drm_fb_helper_deferred_io); > > -/** > - * drm_fb_helper_sys_read - Implements struct _ops.fb_read for system > memory > - * @info: fb_info struct pointer > - * @buf: userspace buffer to read from framebuffer memory > - * @count: number of bytes to read from framebuffer memory > - * @ppos: read offset within framebuffer memory > - * > - * Returns: > - * The number of bytes read on success, or an error code otherwise. > - */ > -ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf, > - size_t count, loff_t *ppos) > -{ > - return fb_sys_read(info, buf, count, ppos); > -} > -EXPORT_SYMBOL(drm_fb_helper_sys_read); > - > -/** > - * drm_fb_helper_sys_write - Implements struct _ops.fb_write for system > memory > - * @info: fb_info struct pointer > - * @buf: userspace buffer to write to framebuffer memory > - * @count: number of bytes to write to framebuffer memory > - * @ppos: write offset within framebuffer memory > - * > - * Returns: > - * The number of bytes written on success, or an error code otherwise. > - */ > -ssize_t drm_fb_helper_sys_write(struct fb_info *info, const char __user *buf, > - size_t count, loff_t *ppos) > -{ > - struct drm_fb_helper *helper = info->par; > - loff_t pos = *ppos; > - ssize_t ret; > - struct drm_rect damage_area; > - > - ret = fb_sys_write(info, buf, count, ppos); > - if (ret <= 0) > - return ret; > - > - if (helper->funcs->fb_dirty) { > - drm_fb_helper_memory_range_to_clip(info, pos, ret, > _area); > - drm_fb_helper_damage(helper, damage_area.x1, damage_area.y1, > -drm_rect_width(_area), > -drm_rect_height(_area)); > - } > - > - return ret; > -} > -EXPORT_SYMBOL(drm_fb_helper_sys_write); > - > -/** > - * drm_fb_helper_sys_fillrect - wrapper around sys_fillrect > - * @info: fbdev registered by the helper > - * @rect: info about rectangle to fill > - * > - * A wrapper around sys_fillrect implemented by fbdev core > - */ > -void drm_fb_helper_sys_fillrect(struct fb_info *info, > - const struct fb_fillrect *rect) > -{ > - struct drm_fb_helper *helper = info->par; > - > - sys_fillrect(info, rect); > - > - if (helper->funcs->fb_dirty) > - drm_fb_helper_damage(helper, rect->dx, rect->dy, rect->width, > rect->height); > -} > -EXPORT_SYMBOL(drm_fb_helper_sys_fillrect); > - > -/** > - * drm_fb_helper_sys_copyarea - wrapper around sys_copyarea > - * @info: fbdev registered by the helper > - * @area: info about area to copy > - * > - * A wrapper around sys_copyarea implemented by fbdev core > - */ > -void drm_fb_helper_sys_copyarea(struct fb_info *info, > - const struct fb_copyarea *area) > -{ > - struct drm_fb_helper *helper = info->par; > - > -
Re: [Intel-gfx] [PATCH v10 00/10] drm/hdcp: Pull HDCP auth/exchange/check into helpers
On 20/05/2023 00:16, Rodrigo Vivi wrote: On Fri, May 19, 2023 at 07:55:47PM +0300, Dmitry Baryshkov wrote: On 19/04/2023 18:43, Mark Yacoub wrote: Hi all, This is v10 of the HDCP patches. The patches are authored by Sean Paul. I rebased and addressed the review comments in v6-v10. Main change in v10 is handling the kernel test bot warnings. Patches 1-4 focus on moving the common HDCP helpers to common DRM. This introduces a slight change in the original intel flow as it splits the unique driver protocol from the generic implementation. Patches 5-7 split the HDCP flow on the i915 driver to make use of the common DRM helpers. Patches 8-10 implement HDCP on MSM driver. Thanks, -Mark Yacoub Sean Paul (10): drm/hdcp: Add drm_hdcp_atomic_check() drm/hdcp: Avoid changing crtc state in hdcp atomic check drm/hdcp: Update property value on content type and user changes drm/hdcp: Expand HDCP helper library for enable/disable/check drm/i915/hdcp: Consolidate HDCP setup/state cache drm/i915/hdcp: Retain hdcp_capable return codes drm/i915/hdcp: Use HDCP helpers for i915 dt-bindings: msm/dp: Add bindings for HDCP registers arm64: dts: qcom: sc7180: Add support for HDCP in dp-controller Dear i915 maintainers, I wanted to ping you regarding this patch series. If there are no comments for the series from you side, would it be possible to land Intel-specific and generic patches into drm-intel tree? We will continue working on the msm specific parts and merge them through the msm tree. pushed to drm-intel-next. should be propagated in a few weeks to drm-next on our next pull request. Probably there is some kind of confusion here. You've pushed the DSC patches, while the response was sent to the HDCP series. -- With best wishes Dmitry
Re: [Intel-gfx] [PATCH v10 00/10] drm/hdcp: Pull HDCP auth/exchange/check into helpers
On 19/04/2023 18:43, Mark Yacoub wrote: Hi all, This is v10 of the HDCP patches. The patches are authored by Sean Paul. I rebased and addressed the review comments in v6-v10. Main change in v10 is handling the kernel test bot warnings. Patches 1-4 focus on moving the common HDCP helpers to common DRM. This introduces a slight change in the original intel flow as it splits the unique driver protocol from the generic implementation. Patches 5-7 split the HDCP flow on the i915 driver to make use of the common DRM helpers. Patches 8-10 implement HDCP on MSM driver. Thanks, -Mark Yacoub Sean Paul (10): drm/hdcp: Add drm_hdcp_atomic_check() drm/hdcp: Avoid changing crtc state in hdcp atomic check drm/hdcp: Update property value on content type and user changes drm/hdcp: Expand HDCP helper library for enable/disable/check drm/i915/hdcp: Consolidate HDCP setup/state cache drm/i915/hdcp: Retain hdcp_capable return codes drm/i915/hdcp: Use HDCP helpers for i915 dt-bindings: msm/dp: Add bindings for HDCP registers arm64: dts: qcom: sc7180: Add support for HDCP in dp-controller Dear i915 maintainers, I wanted to ping you regarding this patch series. If there are no comments for the series from you side, would it be possible to land Intel-specific and generic patches into drm-intel tree? We will continue working on the msm specific parts and merge them through the msm tree. drm/msm: Implement HDCP 1.x using the new drm HDCP helpers .../bindings/display/msm/dp-controller.yaml |7 +- arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi |8 + drivers/gpu/drm/display/drm_hdcp_helper.c | 1224 + drivers/gpu/drm/i915/display/intel_atomic.c |8 +- drivers/gpu/drm/i915/display/intel_ddi.c | 32 +- .../drm/i915/display/intel_display_debugfs.c | 12 +- .../drm/i915/display/intel_display_types.h| 51 +- drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 352 ++--- drivers/gpu/drm/i915/display/intel_dp_mst.c | 16 +- drivers/gpu/drm/i915/display/intel_hdcp.c | 1060 +++--- drivers/gpu/drm/i915/display/intel_hdcp.h | 48 +- drivers/gpu/drm/i915/display/intel_hdmi.c | 267 ++-- drivers/gpu/drm/msm/Kconfig |1 + drivers/gpu/drm/msm/Makefile |1 + drivers/gpu/drm/msm/dp/dp_catalog.c | 156 +++ drivers/gpu/drm/msm/dp/dp_catalog.h | 18 + drivers/gpu/drm/msm/dp/dp_debug.c | 46 +- drivers/gpu/drm/msm/dp/dp_debug.h | 11 +- drivers/gpu/drm/msm/dp/dp_display.c | 39 +- drivers/gpu/drm/msm/dp/dp_display.h |5 + drivers/gpu/drm/msm/dp/dp_drm.c | 39 +- drivers/gpu/drm/msm/dp/dp_drm.h |7 + drivers/gpu/drm/msm/dp/dp_hdcp.c | 389 ++ drivers/gpu/drm/msm/dp/dp_hdcp.h | 33 + drivers/gpu/drm/msm/dp/dp_parser.c| 14 + drivers/gpu/drm/msm/dp/dp_parser.h|4 + drivers/gpu/drm/msm/dp/dp_reg.h | 30 +- drivers/gpu/drm/msm/msm_atomic.c | 19 + include/drm/display/drm_hdcp.h| 296 include/drm/display/drm_hdcp_helper.h | 23 + 30 files changed, 2867 insertions(+), 1349 deletions(-) create mode 100644 drivers/gpu/drm/msm/dp/dp_hdcp.c create mode 100644 drivers/gpu/drm/msm/dp/dp_hdcp.h -- With best wishes Dmitry
Re: [Intel-gfx] [PATCH v7 6/8] drm/display/dsc: split DSC 1.2 and DSC 1.1 (pre-SCR) parameters
On 17/05/2023 13:42, Kandpal, Suraj wrote: The array of rc_parameters contains a mixture of parameters from DSC 1.1 and DSC 1.2 standards. Split these tow configuration arrays in preparation to adding more configuration data. Signed-off-by: Dmitry Baryshkov LGTM. Reviewed-by: Suraj Kandpal Just to note, this is not a proper R-B tag, it doesn't contain your email: 313685b15740 drm/display/dsc: split DSC 1.2 and DSC 1.1 (pre-SCR) parameters -:12: ERROR:BAD_SIGN_OFF: Unrecognized email address: 'Suraj Kandpal' #12: Reviewed-by: Suraj Kandpal total: 1 errors, 0 warnings, 0 checks, 239 lines checked --- drivers/gpu/drm/display/drm_dsc_helper.c | 139 ++ drivers/gpu/drm/i915/display/intel_vdsc.c | 10 +- include/drm/display/drm_dsc_helper.h | 7 +- 3 files changed, 129 insertions(+), 27 deletions(-) [skipped the patch] -- With best wishes Dmitry