| Message ID | 20241214-drm-connector-mode-valid-const-v2-0-4f9498a4c822@linaro.org |
|---|---|
| Headers | show |
| Series | drm/connector: make mode_valid() callback accept const mode pointer | expand |
On Sat, Dec 14, 2024 at 03:37:04PM +0200, Dmitry Baryshkov wrote: > While working on the generic mode_valid() implementation for the HDMI > Connector framework I noticed that unlike other DRM objects > drm_connector accepts non-const pointer to struct drm_display_mode, > while obviously mode_valid() isn't expected to modify the argument. > > Mass-change the DRM framework code to pass const argument to that > callback. > > The series has been compile-tested with defconfig for x86-64, arm and > arm64. > > Note: yes, I understand that this change might be hard to review and > merge. The only viable option that I foresee is to add new callback, > having the const argument and migrate drivers into using it one by one. Colleagues, I'd like to graciously ping regarding this series. Should it be merged as is (possibly requiring more R-B's)? Or should I rework it adding something like .mode_valid_new() callback which takes const argument? > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > Changes in v2: > - Rebased on top of linux-next > - Replaced 'accept const argument' with 'take a const arugment' > (Laurent) > - Link to v1: https://lore.kernel.org/r/20241115-drm-connector-mode-valid-const-v1-0-b1b523156f71@linaro.org > > --- > Dmitry Baryshkov (5): > drm/encoder_slave: make mode_valid accept const struct drm_display_mode > drm/amdgpu: don't change mode in amdgpu_dm_connector_mode_valid() > drm/sti: hda: pass const struct drm_display_mode* to hda_get_mode_idx() > drm/connector: make mode_valid_ctx take a const struct drm_display_mode > drm/connector: make mode_valid take a const struct drm_display_mode > > drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 8 ++++---- > drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 2 +- > drivers/gpu/drm/amd/amdgpu/atombios_dp.h | 2 +- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 +++++++++--- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 +- > drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c | 2 +- > drivers/gpu/drm/arm/malidp_mw.c | 2 +- > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- > drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 2 +- > drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c | 7 ------- > drivers/gpu/drm/display/drm_bridge_connector.c | 2 +- > drivers/gpu/drm/display/drm_hdmi_state_helper.c | 2 +- > drivers/gpu/drm/drm_crtc_helper_internal.h | 2 +- > drivers/gpu/drm/drm_probe_helper.c | 2 +- > drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +- > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 2 +- > drivers/gpu/drm/gma500/cdv_intel_crt.c | 2 +- > drivers/gpu/drm/gma500/cdv_intel_dp.c | 2 +- > drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 2 +- > drivers/gpu/drm/gma500/cdv_intel_lvds.c | 2 +- > drivers/gpu/drm/gma500/oaktrail_hdmi.c | 2 +- > drivers/gpu/drm/gma500/psb_intel_drv.h | 2 +- > drivers/gpu/drm/gma500/psb_intel_lvds.c | 2 +- > drivers/gpu/drm/gma500/psb_intel_sdvo.c | 2 +- > drivers/gpu/drm/i2c/ch7006_drv.c | 2 +- > drivers/gpu/drm/i2c/sil164_drv.c | 2 +- > drivers/gpu/drm/i915/display/dvo_ch7017.c | 2 +- > drivers/gpu/drm/i915/display/dvo_ch7xxx.c | 2 +- > drivers/gpu/drm/i915/display/dvo_ivch.c | 2 +- > drivers/gpu/drm/i915/display/dvo_ns2501.c | 2 +- > drivers/gpu/drm/i915/display/dvo_sil164.c | 2 +- > drivers/gpu/drm/i915/display/dvo_tfp410.c | 2 +- > drivers/gpu/drm/i915/display/icl_dsi.c | 2 +- > drivers/gpu/drm/i915/display/intel_crt.c | 2 +- > drivers/gpu/drm/i915/display/intel_dp.c | 2 +- > drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +- > drivers/gpu/drm/i915/display/intel_dsi.c | 2 +- > drivers/gpu/drm/i915/display/intel_dsi.h | 2 +- > drivers/gpu/drm/i915/display/intel_dvo.c | 2 +- > drivers/gpu/drm/i915/display/intel_dvo_dev.h | 2 +- > drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +- > drivers/gpu/drm/i915/display/intel_lvds.c | 2 +- > drivers/gpu/drm/i915/display/intel_sdvo.c | 2 +- > drivers/gpu/drm/i915/display/intel_tv.c | 2 +- > drivers/gpu/drm/i915/display/vlv_dsi.c | 2 +- > drivers/gpu/drm/imx/ipuv3/imx-tve.c | 2 +- > drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c | 2 +- > drivers/gpu/drm/nouveau/dispnv04/tvnv17.c | 2 +- > drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +- > drivers/gpu/drm/nouveau/nouveau_connector.c | 2 +- > drivers/gpu/drm/qxl/qxl_display.c | 2 +- > drivers/gpu/drm/radeon/atombios_dp.c | 2 +- > drivers/gpu/drm/radeon/radeon_connectors.c | 10 +++++----- > drivers/gpu/drm/radeon/radeon_mode.h | 2 +- > drivers/gpu/drm/rockchip/cdn-dp-core.c | 2 +- > drivers/gpu/drm/rockchip/inno_hdmi.c | 4 ++-- > drivers/gpu/drm/rockchip/rk3066_hdmi.c | 2 +- > drivers/gpu/drm/sti/sti_dvo.c | 2 +- > drivers/gpu/drm/sti/sti_hda.c | 12 ++++++------ > drivers/gpu/drm/sti/sti_hdmi.c | 2 +- > drivers/gpu/drm/tegra/dsi.c | 2 +- > drivers/gpu/drm/tegra/hdmi.c | 2 +- > drivers/gpu/drm/tegra/sor.c | 2 +- > drivers/gpu/drm/vc4/vc4_txp.c | 2 +- > drivers/gpu/drm/virtio/virtgpu_display.c | 2 +- > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +- > drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 2 +- > drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 2 +- > include/drm/display/drm_hdmi_state_helper.h | 2 +- > include/drm/drm_encoder_slave.h | 2 +- > include/drm/drm_modeset_helper_vtables.h | 4 ++-- > 71 files changed, 92 insertions(+), 93 deletions(-) > --- > base-commit: 4176cf5c5651c33769de83bb61b0287f4ec7719f > change-id: 20241115-drm-connector-mode-valid-const-ae3db0ef6cb7 > > Best regards, > -- > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >
On Mon, Jan 06, 2025 at 12:47:07AM +0200, Dmitry Baryshkov wrote: > On Sat, Dec 14, 2024 at 03:37:04PM +0200, Dmitry Baryshkov wrote: > > While working on the generic mode_valid() implementation for the HDMI > > Connector framework I noticed that unlike other DRM objects > > drm_connector accepts non-const pointer to struct drm_display_mode, > > while obviously mode_valid() isn't expected to modify the argument. > > > > Mass-change the DRM framework code to pass const argument to that > > callback. > > > > The series has been compile-tested with defconfig for x86-64, arm and > > arm64. > > > > Note: yes, I understand that this change might be hard to review and > > merge. The only viable option that I foresee is to add new callback, > > having the const argument and migrate drivers into using it one by one. > > Colleagues, I'd like to graciously ping regarding this series. Should it > be merged as is (possibly requiring more R-B's)? Or should I rework it > adding something like .mode_valid_new() callback which takes const > argument? I personally like the end result. The diffstat isn't very big, I don't think it's that hard to review. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > Changes in v2: > > - Rebased on top of linux-next > > - Replaced 'accept const argument' with 'take a const arugment' > > (Laurent) > > - Link to v1: https://lore.kernel.org/r/20241115-drm-connector-mode-valid-const-v1-0-b1b523156f71@linaro.org > > > > --- > > Dmitry Baryshkov (5): > > drm/encoder_slave: make mode_valid accept const struct drm_display_mode > > drm/amdgpu: don't change mode in amdgpu_dm_connector_mode_valid() > > drm/sti: hda: pass const struct drm_display_mode* to hda_get_mode_idx() > > drm/connector: make mode_valid_ctx take a const struct drm_display_mode > > drm/connector: make mode_valid take a const struct drm_display_mode > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 8 ++++---- > > drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 2 +- > > drivers/gpu/drm/amd/amdgpu/atombios_dp.h | 2 +- > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 +++++++++--- > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 +- > > drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c | 2 +- > > drivers/gpu/drm/arm/malidp_mw.c | 2 +- > > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- > > drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 2 +- > > drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c | 7 ------- > > drivers/gpu/drm/display/drm_bridge_connector.c | 2 +- > > drivers/gpu/drm/display/drm_hdmi_state_helper.c | 2 +- > > drivers/gpu/drm/drm_crtc_helper_internal.h | 2 +- > > drivers/gpu/drm/drm_probe_helper.c | 2 +- > > drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +- > > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 2 +- > > drivers/gpu/drm/gma500/cdv_intel_crt.c | 2 +- > > drivers/gpu/drm/gma500/cdv_intel_dp.c | 2 +- > > drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 2 +- > > drivers/gpu/drm/gma500/cdv_intel_lvds.c | 2 +- > > drivers/gpu/drm/gma500/oaktrail_hdmi.c | 2 +- > > drivers/gpu/drm/gma500/psb_intel_drv.h | 2 +- > > drivers/gpu/drm/gma500/psb_intel_lvds.c | 2 +- > > drivers/gpu/drm/gma500/psb_intel_sdvo.c | 2 +- > > drivers/gpu/drm/i2c/ch7006_drv.c | 2 +- > > drivers/gpu/drm/i2c/sil164_drv.c | 2 +- > > drivers/gpu/drm/i915/display/dvo_ch7017.c | 2 +- > > drivers/gpu/drm/i915/display/dvo_ch7xxx.c | 2 +- > > drivers/gpu/drm/i915/display/dvo_ivch.c | 2 +- > > drivers/gpu/drm/i915/display/dvo_ns2501.c | 2 +- > > drivers/gpu/drm/i915/display/dvo_sil164.c | 2 +- > > drivers/gpu/drm/i915/display/dvo_tfp410.c | 2 +- > > drivers/gpu/drm/i915/display/icl_dsi.c | 2 +- > > drivers/gpu/drm/i915/display/intel_crt.c | 2 +- > > drivers/gpu/drm/i915/display/intel_dp.c | 2 +- > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +- > > drivers/gpu/drm/i915/display/intel_dsi.c | 2 +- > > drivers/gpu/drm/i915/display/intel_dsi.h | 2 +- > > drivers/gpu/drm/i915/display/intel_dvo.c | 2 +- > > drivers/gpu/drm/i915/display/intel_dvo_dev.h | 2 +- > > drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +- > > drivers/gpu/drm/i915/display/intel_lvds.c | 2 +- > > drivers/gpu/drm/i915/display/intel_sdvo.c | 2 +- > > drivers/gpu/drm/i915/display/intel_tv.c | 2 +- > > drivers/gpu/drm/i915/display/vlv_dsi.c | 2 +- > > drivers/gpu/drm/imx/ipuv3/imx-tve.c | 2 +- > > drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c | 2 +- > > drivers/gpu/drm/nouveau/dispnv04/tvnv17.c | 2 +- > > drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +- > > drivers/gpu/drm/nouveau/nouveau_connector.c | 2 +- > > drivers/gpu/drm/qxl/qxl_display.c | 2 +- > > drivers/gpu/drm/radeon/atombios_dp.c | 2 +- > > drivers/gpu/drm/radeon/radeon_connectors.c | 10 +++++----- > > drivers/gpu/drm/radeon/radeon_mode.h | 2 +- > > drivers/gpu/drm/rockchip/cdn-dp-core.c | 2 +- > > drivers/gpu/drm/rockchip/inno_hdmi.c | 4 ++-- > > drivers/gpu/drm/rockchip/rk3066_hdmi.c | 2 +- > > drivers/gpu/drm/sti/sti_dvo.c | 2 +- > > drivers/gpu/drm/sti/sti_hda.c | 12 ++++++------ > > drivers/gpu/drm/sti/sti_hdmi.c | 2 +- > > drivers/gpu/drm/tegra/dsi.c | 2 +- > > drivers/gpu/drm/tegra/hdmi.c | 2 +- > > drivers/gpu/drm/tegra/sor.c | 2 +- > > drivers/gpu/drm/vc4/vc4_txp.c | 2 +- > > drivers/gpu/drm/virtio/virtgpu_display.c | 2 +- > > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +- > > drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 2 +- > > drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 2 +- > > include/drm/display/drm_hdmi_state_helper.h | 2 +- > > include/drm/drm_encoder_slave.h | 2 +- > > include/drm/drm_modeset_helper_vtables.h | 4 ++-- > > 71 files changed, 92 insertions(+), 93 deletions(-) > > --- > > base-commit: 4176cf5c5651c33769de83bb61b0287f4ec7719f > > change-id: 20241115-drm-connector-mode-valid-const-ae3db0ef6cb7
On Mon, Jan 06, 2025 at 12:47:07AM +0200, Dmitry Baryshkov wrote: > On Sat, Dec 14, 2024 at 03:37:04PM +0200, Dmitry Baryshkov wrote: > > While working on the generic mode_valid() implementation for the HDMI > > Connector framework I noticed that unlike other DRM objects > > drm_connector accepts non-const pointer to struct drm_display_mode, > > while obviously mode_valid() isn't expected to modify the argument. > > > > Mass-change the DRM framework code to pass const argument to that > > callback. > > > > The series has been compile-tested with defconfig for x86-64, arm and > > arm64. > > > > Note: yes, I understand that this change might be hard to review and > > merge. The only viable option that I foresee is to add new callback, > > having the const argument and migrate drivers into using it one by one. > > Colleagues, I'd like to graciously ping regarding this series. Should it > be merged as is (possibly requiring more R-B's)? Or should I rework it > adding something like .mode_valid_new() callback which takes const > argument? I think your patch is fine, and you can add my Reviewed-by: Maxime Ripard <mripard@kernel.org> We seem to lack an Acked-by for amdgpu though? Maxime
Hi Am 05.01.25 um 23:47 schrieb Dmitry Baryshkov: > On Sat, Dec 14, 2024 at 03:37:04PM +0200, Dmitry Baryshkov wrote: >> While working on the generic mode_valid() implementation for the HDMI >> Connector framework I noticed that unlike other DRM objects >> drm_connector accepts non-const pointer to struct drm_display_mode, >> while obviously mode_valid() isn't expected to modify the argument. >> >> Mass-change the DRM framework code to pass const argument to that >> callback. >> >> The series has been compile-tested with defconfig for x86-64, arm and >> arm64. >> >> Note: yes, I understand that this change might be hard to review and >> merge. The only viable option that I foresee is to add new callback, >> having the const argument and migrate drivers into using it one by one. > Colleagues, I'd like to graciously ping regarding this series. Should it > be merged as is (possibly requiring more R-B's)? Or should I rework it > adding something like .mode_valid_new() callback which takes const > argument? Please merge as-is. No need to complicate things. For the whole series: Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> Thanks for doing this. Best regards Thomas > >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- >> Changes in v2: >> - Rebased on top of linux-next >> - Replaced 'accept const argument' with 'take a const arugment' >> (Laurent) >> - Link to v1: https://lore.kernel.org/r/20241115-drm-connector-mode-valid-const-v1-0-b1b523156f71@linaro.org >> >> --- >> Dmitry Baryshkov (5): >> drm/encoder_slave: make mode_valid accept const struct drm_display_mode >> drm/amdgpu: don't change mode in amdgpu_dm_connector_mode_valid() >> drm/sti: hda: pass const struct drm_display_mode* to hda_get_mode_idx() >> drm/connector: make mode_valid_ctx take a const struct drm_display_mode >> drm/connector: make mode_valid take a const struct drm_display_mode >> >> drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 8 ++++---- >> drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 2 +- >> drivers/gpu/drm/amd/amdgpu/atombios_dp.h | 2 +- >> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 +++++++++--- >> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 +- >> drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c | 2 +- >> drivers/gpu/drm/arm/malidp_mw.c | 2 +- >> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- >> drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 2 +- >> drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c | 7 ------- >> drivers/gpu/drm/display/drm_bridge_connector.c | 2 +- >> drivers/gpu/drm/display/drm_hdmi_state_helper.c | 2 +- >> drivers/gpu/drm/drm_crtc_helper_internal.h | 2 +- >> drivers/gpu/drm/drm_probe_helper.c | 2 +- >> drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +- >> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 2 +- >> drivers/gpu/drm/gma500/cdv_intel_crt.c | 2 +- >> drivers/gpu/drm/gma500/cdv_intel_dp.c | 2 +- >> drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 2 +- >> drivers/gpu/drm/gma500/cdv_intel_lvds.c | 2 +- >> drivers/gpu/drm/gma500/oaktrail_hdmi.c | 2 +- >> drivers/gpu/drm/gma500/psb_intel_drv.h | 2 +- >> drivers/gpu/drm/gma500/psb_intel_lvds.c | 2 +- >> drivers/gpu/drm/gma500/psb_intel_sdvo.c | 2 +- >> drivers/gpu/drm/i2c/ch7006_drv.c | 2 +- >> drivers/gpu/drm/i2c/sil164_drv.c | 2 +- >> drivers/gpu/drm/i915/display/dvo_ch7017.c | 2 +- >> drivers/gpu/drm/i915/display/dvo_ch7xxx.c | 2 +- >> drivers/gpu/drm/i915/display/dvo_ivch.c | 2 +- >> drivers/gpu/drm/i915/display/dvo_ns2501.c | 2 +- >> drivers/gpu/drm/i915/display/dvo_sil164.c | 2 +- >> drivers/gpu/drm/i915/display/dvo_tfp410.c | 2 +- >> drivers/gpu/drm/i915/display/icl_dsi.c | 2 +- >> drivers/gpu/drm/i915/display/intel_crt.c | 2 +- >> drivers/gpu/drm/i915/display/intel_dp.c | 2 +- >> drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +- >> drivers/gpu/drm/i915/display/intel_dsi.c | 2 +- >> drivers/gpu/drm/i915/display/intel_dsi.h | 2 +- >> drivers/gpu/drm/i915/display/intel_dvo.c | 2 +- >> drivers/gpu/drm/i915/display/intel_dvo_dev.h | 2 +- >> drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +- >> drivers/gpu/drm/i915/display/intel_lvds.c | 2 +- >> drivers/gpu/drm/i915/display/intel_sdvo.c | 2 +- >> drivers/gpu/drm/i915/display/intel_tv.c | 2 +- >> drivers/gpu/drm/i915/display/vlv_dsi.c | 2 +- >> drivers/gpu/drm/imx/ipuv3/imx-tve.c | 2 +- >> drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c | 2 +- >> drivers/gpu/drm/nouveau/dispnv04/tvnv17.c | 2 +- >> drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +- >> drivers/gpu/drm/nouveau/nouveau_connector.c | 2 +- >> drivers/gpu/drm/qxl/qxl_display.c | 2 +- >> drivers/gpu/drm/radeon/atombios_dp.c | 2 +- >> drivers/gpu/drm/radeon/radeon_connectors.c | 10 +++++----- >> drivers/gpu/drm/radeon/radeon_mode.h | 2 +- >> drivers/gpu/drm/rockchip/cdn-dp-core.c | 2 +- >> drivers/gpu/drm/rockchip/inno_hdmi.c | 4 ++-- >> drivers/gpu/drm/rockchip/rk3066_hdmi.c | 2 +- >> drivers/gpu/drm/sti/sti_dvo.c | 2 +- >> drivers/gpu/drm/sti/sti_hda.c | 12 ++++++------ >> drivers/gpu/drm/sti/sti_hdmi.c | 2 +- >> drivers/gpu/drm/tegra/dsi.c | 2 +- >> drivers/gpu/drm/tegra/hdmi.c | 2 +- >> drivers/gpu/drm/tegra/sor.c | 2 +- >> drivers/gpu/drm/vc4/vc4_txp.c | 2 +- >> drivers/gpu/drm/virtio/virtgpu_display.c | 2 +- >> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +- >> drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 2 +- >> drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 2 +- >> include/drm/display/drm_hdmi_state_helper.h | 2 +- >> include/drm/drm_encoder_slave.h | 2 +- >> include/drm/drm_modeset_helper_vtables.h | 4 ++-- >> 71 files changed, 92 insertions(+), 93 deletions(-) >> --- >> base-commit: 4176cf5c5651c33769de83bb61b0287f4ec7719f >> change-id: 20241115-drm-connector-mode-valid-const-ae3db0ef6cb7 >> >> Best regards, >> -- >> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>
On Mon, 6 Jan 2025 at 10:55, Maxime Ripard <mripard@kernel.org> wrote: > > On Mon, Jan 06, 2025 at 12:47:07AM +0200, Dmitry Baryshkov wrote: > > On Sat, Dec 14, 2024 at 03:37:04PM +0200, Dmitry Baryshkov wrote: > > > While working on the generic mode_valid() implementation for the HDMI > > > Connector framework I noticed that unlike other DRM objects > > > drm_connector accepts non-const pointer to struct drm_display_mode, > > > while obviously mode_valid() isn't expected to modify the argument. > > > > > > Mass-change the DRM framework code to pass const argument to that > > > callback. > > > > > > The series has been compile-tested with defconfig for x86-64, arm and > > > arm64. > > > > > > Note: yes, I understand that this change might be hard to review and > > > merge. The only viable option that I foresee is to add new callback, > > > having the const argument and migrate drivers into using it one by one. > > > > Colleagues, I'd like to graciously ping regarding this series. Should it > > be merged as is (possibly requiring more R-B's)? Or should I rework it > > adding something like .mode_valid_new() callback which takes const > > argument? > > I think your patch is fine, and you can add my > > Reviewed-by: Maxime Ripard <mripard@kernel.org> > > We seem to lack an Acked-by for amdgpu though? Yes. I think the AMD is the only one missing
On 2025-01-06 04:45, Dmitry Baryshkov wrote: > On Mon, 6 Jan 2025 at 10:55, Maxime Ripard <mripard@kernel.org> wrote: >> >> On Mon, Jan 06, 2025 at 12:47:07AM +0200, Dmitry Baryshkov wrote: >>> On Sat, Dec 14, 2024 at 03:37:04PM +0200, Dmitry Baryshkov wrote: >>>> While working on the generic mode_valid() implementation for the HDMI >>>> Connector framework I noticed that unlike other DRM objects >>>> drm_connector accepts non-const pointer to struct drm_display_mode, >>>> while obviously mode_valid() isn't expected to modify the argument. >>>> >>>> Mass-change the DRM framework code to pass const argument to that >>>> callback. >>>> >>>> The series has been compile-tested with defconfig for x86-64, arm and >>>> arm64. >>>> >>>> Note: yes, I understand that this change might be hard to review and >>>> merge. The only viable option that I foresee is to add new callback, >>>> having the const argument and migrate drivers into using it one by one. >>> >>> Colleagues, I'd like to graciously ping regarding this series. Should it >>> be merged as is (possibly requiring more R-B's)? Or should I rework it >>> adding something like .mode_valid_new() callback which takes const >>> argument? >> >> I think your patch is fine, and you can add my >> >> Reviewed-by: Maxime Ripard <mripard@kernel.org> >> >> We seem to lack an Acked-by for amdgpu though? > > Yes. I think the AMD is the only one missing > > For the amdgpu patch: Reviewed-by: Harry Wentland <harry.wentland@amd.com> Harry
On Sat, 14 Dec 2024 15:37:04 +0200, Dmitry Baryshkov wrote: > While working on the generic mode_valid() implementation for the HDMI > Connector framework I noticed that unlike other DRM objects > drm_connector accepts non-const pointer to struct drm_display_mode, > while obviously mode_valid() isn't expected to modify the argument. > > Mass-change the DRM framework code to pass const argument to that > callback. > > [...] Applied to drm-misc-next, thanks! [1/5] drm/encoder_slave: make mode_valid accept const struct drm_display_mode commit: 7a5cd45fab0a2671aa4ea6d8fb80cea268387176 [2/5] drm/amdgpu: don't change mode in amdgpu_dm_connector_mode_valid() commit: b255ce4388e09f14311e7912d0ccd45a14a08d66 [3/5] drm/sti: hda: pass const struct drm_display_mode* to hda_get_mode_idx() commit: 5f011b442006ccb29044263df10843de80fc0b14 [4/5] drm/connector: make mode_valid_ctx take a const struct drm_display_mode commit: 66df9debcb29d14802912ed79a9cf9ba721b51a4 [5/5] drm/connector: make mode_valid take a const struct drm_display_mode commit: 26d6fd81916e62d2b0568d9756e5f9c33f0f9b7a Best regards,
Hi Dmitry, On Tue, Jan 7, 2025 at 12:31 PM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > On Sat, 14 Dec 2024 15:37:04 +0200, Dmitry Baryshkov wrote: > > While working on the generic mode_valid() implementation for the HDMI > > Connector framework I noticed that unlike other DRM objects > > drm_connector accepts non-const pointer to struct drm_display_mode, > > while obviously mode_valid() isn't expected to modify the argument. > > > > Mass-change the DRM framework code to pass const argument to that > > callback. > > > > [...] > > Applied to drm-misc-next, thanks! > > [1/5] drm/encoder_slave: make mode_valid accept const struct drm_display_mode > commit: 7a5cd45fab0a2671aa4ea6d8fb80cea268387176 > [2/5] drm/amdgpu: don't change mode in amdgpu_dm_connector_mode_valid() > commit: b255ce4388e09f14311e7912d0ccd45a14a08d66 > [3/5] drm/sti: hda: pass const struct drm_display_mode* to hda_get_mode_idx() > commit: 5f011b442006ccb29044263df10843de80fc0b14 > [4/5] drm/connector: make mode_valid_ctx take a const struct drm_display_mode > commit: 66df9debcb29d14802912ed79a9cf9ba721b51a4 > [5/5] drm/connector: make mode_valid take a const struct drm_display_mode > commit: 26d6fd81916e62d2b0568d9756e5f9c33f0f9b7a I cannot find these in drm-misc or drm-next, but they are in drm-tip? The last one due to commit 2bdc721917cf141f ("Merge remote-tracking branch 'drm-misc/drm-misc-next' into drm-tip"). What am I missing? Thanks! P.S. Sima: noticed while resolving a merge conflict using drm-tip. Thx! Gr{oetje,eeting}s, Geert
On Tue, 21 Jan 2025 at 11:13, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Dmitry, > > On Tue, Jan 7, 2025 at 12:31 PM Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: > > On Sat, 14 Dec 2024 15:37:04 +0200, Dmitry Baryshkov wrote: > > > While working on the generic mode_valid() implementation for the HDMI > > > Connector framework I noticed that unlike other DRM objects > > > drm_connector accepts non-const pointer to struct drm_display_mode, > > > while obviously mode_valid() isn't expected to modify the argument. > > > > > > Mass-change the DRM framework code to pass const argument to that > > > callback. > > > > > > [...] > > > > Applied to drm-misc-next, thanks! > > > > [1/5] drm/encoder_slave: make mode_valid accept const struct drm_display_mode > > commit: 7a5cd45fab0a2671aa4ea6d8fb80cea268387176 > > [2/5] drm/amdgpu: don't change mode in amdgpu_dm_connector_mode_valid() > > commit: b255ce4388e09f14311e7912d0ccd45a14a08d66 > > [3/5] drm/sti: hda: pass const struct drm_display_mode* to hda_get_mode_idx() > > commit: 5f011b442006ccb29044263df10843de80fc0b14 > > [4/5] drm/connector: make mode_valid_ctx take a const struct drm_display_mode > > commit: 66df9debcb29d14802912ed79a9cf9ba721b51a4 > > [5/5] drm/connector: make mode_valid take a const struct drm_display_mode > > commit: 26d6fd81916e62d2b0568d9756e5f9c33f0f9b7a > > I cannot find these in drm-misc or drm-next, but they are in drm-tip? These are in drm-misc/drm-misc-next, the commit IDs are a part of the Git history. > The last one due to commit 2bdc721917cf141f ("Merge remote-tracking > branch 'drm-misc/drm-misc-next' into drm-tip"). > > What am I missing? > Thanks! It might be some kind of misinteraction between drm-misc-next vs drm-misc-next-fixes vs merge window. Let me recheck dim rebuild-tip. > > P.S. Sima: noticed while resolving a merge conflict using drm-tip. Thx!
Hi Dmitry, CC sfr On Tue, Jan 21, 2025 at 11:44 AM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > On Tue, 21 Jan 2025 at 11:13, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Tue, Jan 7, 2025 at 12:31 PM Dmitry Baryshkov > > <dmitry.baryshkov@linaro.org> wrote: > > > On Sat, 14 Dec 2024 15:37:04 +0200, Dmitry Baryshkov wrote: > > > > While working on the generic mode_valid() implementation for the HDMI > > > > Connector framework I noticed that unlike other DRM objects > > > > drm_connector accepts non-const pointer to struct drm_display_mode, > > > > while obviously mode_valid() isn't expected to modify the argument. > > > > > > > > Mass-change the DRM framework code to pass const argument to that > > > > callback. > > > > > > > > [...] > > > > > > Applied to drm-misc-next, thanks! > > > > > > [1/5] drm/encoder_slave: make mode_valid accept const struct drm_display_mode > > > commit: 7a5cd45fab0a2671aa4ea6d8fb80cea268387176 > > > [2/5] drm/amdgpu: don't change mode in amdgpu_dm_connector_mode_valid() > > > commit: b255ce4388e09f14311e7912d0ccd45a14a08d66 > > > [3/5] drm/sti: hda: pass const struct drm_display_mode* to hda_get_mode_idx() > > > commit: 5f011b442006ccb29044263df10843de80fc0b14 > > > [4/5] drm/connector: make mode_valid_ctx take a const struct drm_display_mode > > > commit: 66df9debcb29d14802912ed79a9cf9ba721b51a4 > > > [5/5] drm/connector: make mode_valid take a const struct drm_display_mode > > > commit: 26d6fd81916e62d2b0568d9756e5f9c33f0f9b7a > > > > I cannot find these in drm-misc or drm-next, but they are in drm-tip? > > These are in drm-misc/drm-misc-next, the commit IDs are a part of the > Git history. > > > The last one due to commit 2bdc721917cf141f ("Merge remote-tracking > > branch 'drm-misc/drm-misc-next' into drm-tip"). > > > > What am I missing? > > Thanks! > > It might be some kind of misinteraction between drm-misc-next vs > drm-misc-next-fixes vs merge window. Let me recheck dim rebuild-tip. I indeed see the commit in https://gitlab.freedesktop.org/drm/misc/kernel/-/blob/drm-misc-next/include/drm/drm_modeset_helper_vtables.h?ref_type=heads [diving deeper] So I missed the change from the for-linux-next to the drm-misc-next branch. Hence I fetched only the former, and was using a stale version of the latter. Apparently Stephen is also using the old branches for linux-next: drm-misc-fixes git https://gitlab.freedesktop.org/drm/misc/kernel.git#for-linux-next-fixes drm-misc git https://gitlab.freedesktop.org/drm/misc/kernel.git#for-linux-next I believe the latter should be drm-misc-next. Should the former be drm-misc-fixes or drm-misc-next-fixes? Or both? Thanks! Gr{oetje,eeting}s, Geert
On Tue, Jan 21, 2025 at 12:10:25PM +0100, Geert Uytterhoeven wrote: > Hi Dmitry, > > CC sfr > > On Tue, Jan 21, 2025 at 11:44 AM Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: > > On Tue, 21 Jan 2025 at 11:13, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > On Tue, Jan 7, 2025 at 12:31 PM Dmitry Baryshkov > > > <dmitry.baryshkov@linaro.org> wrote: > > > > On Sat, 14 Dec 2024 15:37:04 +0200, Dmitry Baryshkov wrote: > > > > > While working on the generic mode_valid() implementation for the HDMI > > > > > Connector framework I noticed that unlike other DRM objects > > > > > drm_connector accepts non-const pointer to struct drm_display_mode, > > > > > while obviously mode_valid() isn't expected to modify the argument. > > > > > > > > > > Mass-change the DRM framework code to pass const argument to that > > > > > callback. > > > > > > > > > > [...] > > > > > > > > Applied to drm-misc-next, thanks! > > > > > > > > [1/5] drm/encoder_slave: make mode_valid accept const struct drm_display_mode > > > > commit: 7a5cd45fab0a2671aa4ea6d8fb80cea268387176 > > > > [2/5] drm/amdgpu: don't change mode in amdgpu_dm_connector_mode_valid() > > > > commit: b255ce4388e09f14311e7912d0ccd45a14a08d66 > > > > [3/5] drm/sti: hda: pass const struct drm_display_mode* to hda_get_mode_idx() > > > > commit: 5f011b442006ccb29044263df10843de80fc0b14 > > > > [4/5] drm/connector: make mode_valid_ctx take a const struct drm_display_mode > > > > commit: 66df9debcb29d14802912ed79a9cf9ba721b51a4 > > > > [5/5] drm/connector: make mode_valid take a const struct drm_display_mode > > > > commit: 26d6fd81916e62d2b0568d9756e5f9c33f0f9b7a > > > > > > I cannot find these in drm-misc or drm-next, but they are in drm-tip? > > > > These are in drm-misc/drm-misc-next, the commit IDs are a part of the > > Git history. > > > > > The last one due to commit 2bdc721917cf141f ("Merge remote-tracking > > > branch 'drm-misc/drm-misc-next' into drm-tip"). > > > > > > What am I missing? > > > Thanks! > > > > It might be some kind of misinteraction between drm-misc-next vs > > drm-misc-next-fixes vs merge window. Let me recheck dim rebuild-tip. > > I indeed see the commit in > https://gitlab.freedesktop.org/drm/misc/kernel/-/blob/drm-misc-next/include/drm/drm_modeset_helper_vtables.h?ref_type=heads > > [diving deeper] > > So I missed the change from the for-linux-next to the drm-misc-next > branch. Hence I fetched only the former, and was using a stale > version of the latter. > > Apparently Stephen is also using the old branches for linux-next: > > drm-misc-fixes git > https://gitlab.freedesktop.org/drm/misc/kernel.git#for-linux-next-fixes > drm-misc git > https://gitlab.freedesktop.org/drm/misc/kernel.git#for-linux-next > > I believe the latter should be drm-misc-next. > Should the former be drm-misc-fixes or drm-misc-next-fixes? Or both? No. Both branches are correct. This is how the drm-misc tree managed development process: during the merge window (and several preceeding weeks) the drm-misc-next branch is open for the commits. However those commits are not targeted the forthcoming -rc1. Thus the for-linux-next branch is diverted to point to drm-misc-next-fixes. This is all being by the dim tool. Respective mode_valid changes were applied too late in the cycle, so they are not going to land into 6.14-rc1 (and are not a part of the for-linux-next branch). Once 6.14-rc1 is released and we start working towards 6.15, for-linux-next will again point to drm-misc-next.
While working on the generic mode_valid() implementation for the HDMI Connector framework I noticed that unlike other DRM objects drm_connector accepts non-const pointer to struct drm_display_mode, while obviously mode_valid() isn't expected to modify the argument. Mass-change the DRM framework code to pass const argument to that callback. The series has been compile-tested with defconfig for x86-64, arm and arm64. Note: yes, I understand that this change might be hard to review and merge. The only viable option that I foresee is to add new callback, having the const argument and migrate drivers into using it one by one. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- Changes in v2: - Rebased on top of linux-next - Replaced 'accept const argument' with 'take a const arugment' (Laurent) - Link to v1: https://lore.kernel.org/r/20241115-drm-connector-mode-valid-const-v1-0-b1b523156f71@linaro.org --- Dmitry Baryshkov (5): drm/encoder_slave: make mode_valid accept const struct drm_display_mode drm/amdgpu: don't change mode in amdgpu_dm_connector_mode_valid() drm/sti: hda: pass const struct drm_display_mode* to hda_get_mode_idx() drm/connector: make mode_valid_ctx take a const struct drm_display_mode drm/connector: make mode_valid take a const struct drm_display_mode drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 8 ++++---- drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 2 +- drivers/gpu/drm/amd/amdgpu/atombios_dp.h | 2 +- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 +++++++++--- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 +- drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c | 2 +- drivers/gpu/drm/arm/malidp_mw.c | 2 +- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 2 +- drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c | 7 ------- drivers/gpu/drm/display/drm_bridge_connector.c | 2 +- drivers/gpu/drm/display/drm_hdmi_state_helper.c | 2 +- drivers/gpu/drm/drm_crtc_helper_internal.h | 2 +- drivers/gpu/drm/drm_probe_helper.c | 2 +- drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 2 +- drivers/gpu/drm/gma500/cdv_intel_crt.c | 2 +- drivers/gpu/drm/gma500/cdv_intel_dp.c | 2 +- drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 2 +- drivers/gpu/drm/gma500/cdv_intel_lvds.c | 2 +- drivers/gpu/drm/gma500/oaktrail_hdmi.c | 2 +- drivers/gpu/drm/gma500/psb_intel_drv.h | 2 +- drivers/gpu/drm/gma500/psb_intel_lvds.c | 2 +- drivers/gpu/drm/gma500/psb_intel_sdvo.c | 2 +- drivers/gpu/drm/i2c/ch7006_drv.c | 2 +- drivers/gpu/drm/i2c/sil164_drv.c | 2 +- drivers/gpu/drm/i915/display/dvo_ch7017.c | 2 +- drivers/gpu/drm/i915/display/dvo_ch7xxx.c | 2 +- drivers/gpu/drm/i915/display/dvo_ivch.c | 2 +- drivers/gpu/drm/i915/display/dvo_ns2501.c | 2 +- drivers/gpu/drm/i915/display/dvo_sil164.c | 2 +- drivers/gpu/drm/i915/display/dvo_tfp410.c | 2 +- drivers/gpu/drm/i915/display/icl_dsi.c | 2 +- drivers/gpu/drm/i915/display/intel_crt.c | 2 +- drivers/gpu/drm/i915/display/intel_dp.c | 2 +- drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +- drivers/gpu/drm/i915/display/intel_dsi.c | 2 +- drivers/gpu/drm/i915/display/intel_dsi.h | 2 +- drivers/gpu/drm/i915/display/intel_dvo.c | 2 +- drivers/gpu/drm/i915/display/intel_dvo_dev.h | 2 +- drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +- drivers/gpu/drm/i915/display/intel_lvds.c | 2 +- drivers/gpu/drm/i915/display/intel_sdvo.c | 2 +- drivers/gpu/drm/i915/display/intel_tv.c | 2 +- drivers/gpu/drm/i915/display/vlv_dsi.c | 2 +- drivers/gpu/drm/imx/ipuv3/imx-tve.c | 2 +- drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c | 2 +- drivers/gpu/drm/nouveau/dispnv04/tvnv17.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +- drivers/gpu/drm/nouveau/nouveau_connector.c | 2 +- drivers/gpu/drm/qxl/qxl_display.c | 2 +- drivers/gpu/drm/radeon/atombios_dp.c | 2 +- drivers/gpu/drm/radeon/radeon_connectors.c | 10 +++++----- drivers/gpu/drm/radeon/radeon_mode.h | 2 +- drivers/gpu/drm/rockchip/cdn-dp-core.c | 2 +- drivers/gpu/drm/rockchip/inno_hdmi.c | 4 ++-- drivers/gpu/drm/rockchip/rk3066_hdmi.c | 2 +- drivers/gpu/drm/sti/sti_dvo.c | 2 +- drivers/gpu/drm/sti/sti_hda.c | 12 ++++++------ drivers/gpu/drm/sti/sti_hdmi.c | 2 +- drivers/gpu/drm/tegra/dsi.c | 2 +- drivers/gpu/drm/tegra/hdmi.c | 2 +- drivers/gpu/drm/tegra/sor.c | 2 +- drivers/gpu/drm/vc4/vc4_txp.c | 2 +- drivers/gpu/drm/virtio/virtgpu_display.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 2 +- include/drm/display/drm_hdmi_state_helper.h | 2 +- include/drm/drm_encoder_slave.h | 2 +- include/drm/drm_modeset_helper_vtables.h | 4 ++-- 71 files changed, 92 insertions(+), 93 deletions(-) --- base-commit: 4176cf5c5651c33769de83bb61b0287f4ec7719f change-id: 20241115-drm-connector-mode-valid-const-ae3db0ef6cb7 Best regards,