[Nouveau] [PATCH] drm: Only select I2C_ALGOBIT for drivers that actually need it
While working on a drm driver that doesn't need the i2c algobit stuff I noticed that DRM selects this code even tough only 8 drivers actually use it. While also only some drivers use i2c, keep the select for I2C for the next cleanup patch. Still prepare this already by also selecting I2C for the individual drivers. Signed-off-by: Uwe Kleine-König --- drivers/gpu/drm/Kconfig | 5 - drivers/gpu/drm/ast/Kconfig | 2 ++ drivers/gpu/drm/gma500/Kconfig | 2 ++ drivers/gpu/drm/hisilicon/hibmc/Kconfig | 2 ++ drivers/gpu/drm/i915/Kconfig| 2 ++ drivers/gpu/drm/mgag200/Kconfig | 2 ++ drivers/gpu/drm/nouveau/Kconfig | 2 ++ 7 files changed, 16 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 3c16bd1afd87..351ea617c498 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -12,7 +12,6 @@ menuconfig DRM select HDMI select FB_CMDLINE select I2C - select I2C_ALGOBIT select DMA_SHARED_BUFFER select SYNC_FILE # gallium uses SYS_kcmp for os_same_file_description() to de-duplicate @@ -233,6 +232,8 @@ config DRM_RADEON select DRM_KMS_HELPER select DRM_TTM select DRM_TTM_HELPER + select I2C + select I2C_ALGOBIT select POWER_SUPPLY select HWMON select BACKLIGHT_CLASS_DEVICE @@ -254,6 +255,8 @@ config DRM_AMDGPU select DRM_SCHED select DRM_TTM select DRM_TTM_HELPER + select I2C + select I2C_ALGOBIT select POWER_SUPPLY select HWMON select BACKLIGHT_CLASS_DEVICE diff --git a/drivers/gpu/drm/ast/Kconfig b/drivers/gpu/drm/ast/Kconfig index fbcf2f45cef5..bcc25decd485 100644 --- a/drivers/gpu/drm/ast/Kconfig +++ b/drivers/gpu/drm/ast/Kconfig @@ -6,6 +6,8 @@ config DRM_AST select DRM_VRAM_HELPER select DRM_TTM select DRM_TTM_HELPER + select I2C + select I2C_ALGOBIT help Say yes for experimental AST GPU driver. Do not enable this driver without having a working -modesetting, diff --git a/drivers/gpu/drm/gma500/Kconfig b/drivers/gpu/drm/gma500/Kconfig index 0cff20265f97..e26c3a24955d 100644 --- a/drivers/gpu/drm/gma500/Kconfig +++ b/drivers/gpu/drm/gma500/Kconfig @@ -3,6 +3,8 @@ config DRM_GMA500 tristate "Intel GMA500/600/3600/3650 KMS Framebuffer" depends on DRM && PCI && X86 && MMU select DRM_KMS_HELPER + select I2C + select I2C_ALGOBIT # GMA500 depends on ACPI_VIDEO when ACPI is enabled, just like i915 select ACPI_VIDEO if ACPI select BACKLIGHT_CLASS_DEVICE if ACPI diff --git a/drivers/gpu/drm/hisilicon/hibmc/Kconfig b/drivers/gpu/drm/hisilicon/hibmc/Kconfig index 43943e980203..ac8c42dc79f6 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/Kconfig +++ b/drivers/gpu/drm/hisilicon/hibmc/Kconfig @@ -6,6 +6,8 @@ config DRM_HISI_HIBMC select DRM_VRAM_HELPER select DRM_TTM select DRM_TTM_HELPER + select I2C + select I2C_ALGOBIT help Choose this option if you have a Hisilicon Hibmc soc chipset. If M is selected the module will be called hibmc-drm. diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index 69f57ca9c68d..b3bb6f7cfbbc 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -13,6 +13,8 @@ config DRM_I915 select DRM_PANEL select DRM_MIPI_DSI select RELAY + select I2C + select I2C_ALGOBIT select IRQ_WORK # i915 depends on ACPI_VIDEO when ACPI is enabled # but for select to work, need to select ACPI_VIDEO's dependencies, ick diff --git a/drivers/gpu/drm/mgag200/Kconfig b/drivers/gpu/drm/mgag200/Kconfig index eec59658a938..b28c5e4828f4 100644 --- a/drivers/gpu/drm/mgag200/Kconfig +++ b/drivers/gpu/drm/mgag200/Kconfig @@ -4,6 +4,8 @@ config DRM_MGAG200 depends on DRM && PCI && MMU select DRM_GEM_SHMEM_HELPER select DRM_KMS_HELPER + select I2C + select I2C_ALGOBIT help This is a KMS driver for Matrox G200 chips. It supports the original MGA G200 desktop chips and the server variants. It requires 0.3.0 diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig index 9436310d0854..8823f0b24c73 100644 --- a/drivers/gpu/drm/nouveau/Kconfig +++ b/drivers/gpu/drm/nouveau/Kconfig @@ -7,6 +7,8 @@ config DRM_NOUVEAU select DRM_KMS_HELPER select DRM_TTM select DRM_TTM_HELPER + select I2C + select I2C_ALGOBIT select BACKLIGHT_CLASS_DEVICE if DRM_NOUVEAU_BACKLIGHT select ACPI_VIDEO if ACPI && X86 && BACKLIGHT_CLASS_DEVICE && INPUT select X86_PLATFORM_DEVICES if ACPI && X86 base-commit: 315d99318179b9cd5077ccc9f7f26a164c9fa998 -- 2.
Re: [Nouveau] [PATCH] drm: Only select I2C_ALGOBIT for drivers that actually need it
Hello, On Fri, May 14, 2021 at 12:01:42PM +0200, Uwe Kleine-König wrote: > While working on a drm driver that doesn't need the i2c algobit stuff I > noticed that DRM selects this code even tough only 8 drivers actually use > it. While also only some drivers use i2c, keep the select for I2C for the > next cleanup patch. Still prepare this already by also selecting I2C for > the individual drivers. > > Signed-off-by: Uwe Kleine-König This patch is already old but the issue is still valid and the patch even still applies to today's Linus' master branch. I didn't receive any human feedback. If you consider this patch worthwile I can recheck if it's still correct as is or needs adaption. Best regards Uwe > --- > drivers/gpu/drm/Kconfig | 5 - > drivers/gpu/drm/ast/Kconfig | 2 ++ > drivers/gpu/drm/gma500/Kconfig | 2 ++ > drivers/gpu/drm/hisilicon/hibmc/Kconfig | 2 ++ > drivers/gpu/drm/i915/Kconfig| 2 ++ > drivers/gpu/drm/mgag200/Kconfig | 2 ++ > drivers/gpu/drm/nouveau/Kconfig | 2 ++ > 7 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 3c16bd1afd87..351ea617c498 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -12,7 +12,6 @@ menuconfig DRM > select HDMI > select FB_CMDLINE > select I2C > - select I2C_ALGOBIT > select DMA_SHARED_BUFFER > select SYNC_FILE > # gallium uses SYS_kcmp for os_same_file_description() to de-duplicate > @@ -233,6 +232,8 @@ config DRM_RADEON > select DRM_KMS_HELPER > select DRM_TTM > select DRM_TTM_HELPER > + select I2C > + select I2C_ALGOBIT > select POWER_SUPPLY > select HWMON > select BACKLIGHT_CLASS_DEVICE > @@ -254,6 +255,8 @@ config DRM_AMDGPU > select DRM_SCHED > select DRM_TTM > select DRM_TTM_HELPER > + select I2C > + select I2C_ALGOBIT > select POWER_SUPPLY > select HWMON > select BACKLIGHT_CLASS_DEVICE > diff --git a/drivers/gpu/drm/ast/Kconfig b/drivers/gpu/drm/ast/Kconfig > index fbcf2f45cef5..bcc25decd485 100644 > --- a/drivers/gpu/drm/ast/Kconfig > +++ b/drivers/gpu/drm/ast/Kconfig > @@ -6,6 +6,8 @@ config DRM_AST > select DRM_VRAM_HELPER > select DRM_TTM > select DRM_TTM_HELPER > + select I2C > + select I2C_ALGOBIT > help >Say yes for experimental AST GPU driver. Do not enable >this driver without having a working -modesetting, > diff --git a/drivers/gpu/drm/gma500/Kconfig b/drivers/gpu/drm/gma500/Kconfig > index 0cff20265f97..e26c3a24955d 100644 > --- a/drivers/gpu/drm/gma500/Kconfig > +++ b/drivers/gpu/drm/gma500/Kconfig > @@ -3,6 +3,8 @@ config DRM_GMA500 > tristate "Intel GMA500/600/3600/3650 KMS Framebuffer" > depends on DRM && PCI && X86 && MMU > select DRM_KMS_HELPER > + select I2C > + select I2C_ALGOBIT > # GMA500 depends on ACPI_VIDEO when ACPI is enabled, just like i915 > select ACPI_VIDEO if ACPI > select BACKLIGHT_CLASS_DEVICE if ACPI > diff --git a/drivers/gpu/drm/hisilicon/hibmc/Kconfig > b/drivers/gpu/drm/hisilicon/hibmc/Kconfig > index 43943e980203..ac8c42dc79f6 100644 > --- a/drivers/gpu/drm/hisilicon/hibmc/Kconfig > +++ b/drivers/gpu/drm/hisilicon/hibmc/Kconfig > @@ -6,6 +6,8 @@ config DRM_HISI_HIBMC > select DRM_VRAM_HELPER > select DRM_TTM > select DRM_TTM_HELPER > + select I2C > + select I2C_ALGOBIT > help > Choose this option if you have a Hisilicon Hibmc soc chipset. > If M is selected the module will be called hibmc-drm. > diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig > index 69f57ca9c68d..b3bb6f7cfbbc 100644 > --- a/drivers/gpu/drm/i915/Kconfig > +++ b/drivers/gpu/drm/i915/Kconfig > @@ -13,6 +13,8 @@ config DRM_I915 > select DRM_PANEL > select DRM_MIPI_DSI > select RELAY > + select I2C > + select I2C_ALGOBIT > select IRQ_WORK > # i915 depends on ACPI_VIDEO when ACPI is enabled > # but for select to work, need to select ACPI_VIDEO's dependencies, ick > diff --git a/drivers/gpu/drm/mgag200/Kconfig b/drivers/gpu/drm/mgag200/Kconfig > index eec59658a938..b28c5e4828f4 100644 > --- a/drivers/gpu/drm/mgag200/Kconfig > +++ b/drivers/gpu/drm/mgag200/Kconfig > @@ -4,6 +4,8 @@ config DRM_MGAG200 > depends on DRM && PCI && MMU > select DRM_GEM_SHMEM_HELPER > select DRM_KMS_HELPER > + select I2C > + select I2C_ALGOBIT > help >
[Nouveau] [PATCH v3 14/16] drm/nouveau: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Reviewed-by: Thomas Zimmermann
Reviewed-by: Jyri Sarha
Signed-off-by: Uwe Kleine-König
---
drivers/gpu/drm/nouveau/nouveau_platform.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_platform.c
b/drivers/gpu/drm/nouveau/nouveau_platform.c
index 23cd43a7fd19..bf2dc7567ea4 100644
--- a/drivers/gpu/drm/nouveau/nouveau_platform.c
+++ b/drivers/gpu/drm/nouveau/nouveau_platform.c
@@ -43,11 +43,10 @@ static int nouveau_platform_probe(struct platform_device
*pdev)
return 0;
}
-static int nouveau_platform_remove(struct platform_device *pdev)
+static void nouveau_platform_remove(struct platform_device *pdev)
{
struct drm_device *dev = platform_get_drvdata(pdev);
nouveau_drm_device_remove(dev);
- return 0;
}
#if IS_ENABLED(CONFIG_OF)
@@ -93,5 +92,5 @@ struct platform_driver nouveau_platform_driver = {
.of_match_table = of_match_ptr(nouveau_platform_match),
},
.probe = nouveau_platform_probe,
- .remove = nouveau_platform_remove,
+ .remove_new = nouveau_platform_remove,
};
--
2.42.0
[Nouveau] [PATCH v3 00/16] drm: Convert to platform remove callback returning void
Hello,
this series converts all platform drivers below drivers/gpu/drm to use
.remove_new(). It starts with a fix for a problem that potentially might
crash the kernel that I stumbled over while implementing the conversion.
Some of the conversion patches following this fix were already send in
earlier series:
https://lore.kernel.org/dri-devel/20230801110239.831099-1-u.kleine-koe...@pengutronix.de
https://lore.kernel.org/dri-devel/20230318190804.234610-1-u.kleine-koe...@pengutronix.de
and three patches (bridge/tpd12s015, exynos + tilcdc) are new. Parts of
the above series were picked up, the patches resend here are not.
See commit 5c5a7680e67b ("platform: Provide a remove callback that
returns no value") for an extended explanation and the eventual goal.
Compared to the earlier submissions this patch series is rebased to
today's drm-misc-next. Otherwise there is no relevant change.
Best regards
Uwe
Uwe Kleine-König (16):
drm/bridge: tpd12s015: Drop buggy __exit annotation for remove
function
drm/arcpgu: Convert to platform remove callback returning void
drm/armada: Convert to platform remove callback returning void
drm/bridge: cdns-mhdp8546: Improve error reporting in remove callback
drm/bridge: cdns-mhdp8546: Convert to platform remove callback
returning void
drm/bridge: tpd12s015: Convert to platform remove callback returning
void
drm/etnaviv: Convert to platform remove callback returning void
drm/exynos: Convert to platform remove callback returning void
drm/imx/dcss: Convert to platform remove callback returning void
drm/imx: lcdc: Convert to platform remove callback returning void
drm/kmb: Convert to platform remove callback returning void
drm/mediatek: Convert to platform remove callback returning void
drm/meson: Convert to platform remove callback returning void
drm/nouveau: Convert to platform remove callback returning void
drm/sprd: Convert to platform remove callback returning void
drm/tilcdc: Convert to platform remove callback returning void
drivers/gpu/drm/armada/armada_crtc.c | 5 ++---
drivers/gpu/drm/armada/armada_drv.c | 5 ++---
.../drm/bridge/cadence/cdns-mhdp8546-core.c | 22 +--
drivers/gpu/drm/bridge/ti-tpd12s015.c | 6 ++---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 6 ++---
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 5 ++---
drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 6 ++---
drivers/gpu/drm/exynos/exynos7_drm_decon.c| 6 ++---
drivers/gpu/drm/exynos/exynos_dp.c| 6 ++---
drivers/gpu/drm/exynos/exynos_drm_drv.c | 5 ++---
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 6 ++---
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 6 ++---
drivers/gpu/drm/exynos/exynos_drm_g2d.c | 6 ++---
drivers/gpu/drm/exynos/exynos_drm_gsc.c | 6 ++---
drivers/gpu/drm/exynos/exynos_drm_mic.c | 6 ++---
drivers/gpu/drm/exynos/exynos_drm_rotator.c | 6 ++---
drivers/gpu/drm/exynos/exynos_drm_scaler.c| 6 ++---
drivers/gpu/drm/exynos/exynos_drm_vidi.c | 6 ++---
drivers/gpu/drm/exynos/exynos_hdmi.c | 6 ++---
drivers/gpu/drm/exynos/exynos_mixer.c | 6 ++---
drivers/gpu/drm/imx/dcss/dcss-drv.c | 6 ++---
drivers/gpu/drm/imx/lcdc/imx-lcdc.c | 6 ++---
drivers/gpu/drm/kmb/kmb_drv.c | 5 ++---
.../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c | 5 ++---
drivers/gpu/drm/mediatek/mtk_ethdr.c | 5 ++---
drivers/gpu/drm/meson/meson_dw_mipi_dsi.c | 6 ++---
drivers/gpu/drm/nouveau/nouveau_platform.c| 5 ++---
drivers/gpu/drm/sprd/sprd_dpu.c | 6 ++---
drivers/gpu/drm/sprd/sprd_drm.c | 5 ++---
drivers/gpu/drm/sprd/sprd_dsi.c | 6 ++---
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 9
drivers/gpu/drm/tiny/arcpgu.c | 6 ++---
32 files changed, 74 insertions(+), 128 deletions(-)
base-commit: 6fd9487147c4f18ad77eea00bd8c9189eec74a3e
--
2.42.0
[Nouveau] [PATCH v3] drm: Only select I2C_ALGOBIT for drivers that actually need it
While working on a drm driver that doesn't need the i2c algobit stuff I noticed that DRM selects this code even though only 8 drivers actually use it. While also only some drivers use i2c, keep the select for I2C for the next cleanup patch. Still prepare this already by also selecting I2C for the individual drivers. Signed-off-by: Uwe Kleine-König --- drivers/gpu/drm/Kconfig | 1 - drivers/gpu/drm/amd/amdgpu/Kconfig | 2 ++ drivers/gpu/drm/ast/Kconfig | 2 ++ drivers/gpu/drm/gma500/Kconfig | 2 ++ drivers/gpu/drm/hisilicon/hibmc/Kconfig | 2 ++ drivers/gpu/drm/i915/Kconfig| 2 ++ drivers/gpu/drm/mgag200/Kconfig | 2 ++ drivers/gpu/drm/nouveau/Kconfig | 2 ++ drivers/gpu/drm/radeon/Kconfig | 2 ++ 9 files changed, 16 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 315cbdf61979..93c732a8f870 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -12,7 +12,6 @@ menuconfig DRM select HDMI select FB_CMDLINE select I2C - select I2C_ALGOBIT select DMA_SHARED_BUFFER select SYNC_FILE # gallium uses SYS_kcmp for os_same_file_description() to de-duplicate diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig index 5fcd510f1abb..5341b6b242c3 100644 --- a/drivers/gpu/drm/amd/amdgpu/Kconfig +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig @@ -13,6 +13,8 @@ config DRM_AMDGPU select DRM_TTM_HELPER select POWER_SUPPLY select HWMON + select I2C + select I2C_ALGOBIT select BACKLIGHT_CLASS_DEVICE select INTERVAL_TREE select DRM_BUDDY diff --git a/drivers/gpu/drm/ast/Kconfig b/drivers/gpu/drm/ast/Kconfig index d367a90cd3de..563fa7a3b546 100644 --- a/drivers/gpu/drm/ast/Kconfig +++ b/drivers/gpu/drm/ast/Kconfig @@ -4,6 +4,8 @@ config DRM_AST depends on DRM && PCI && MMU select DRM_GEM_SHMEM_HELPER select DRM_KMS_HELPER + select I2C + select I2C_ALGOBIT help Say yes for experimental AST GPU driver. Do not enable this driver without having a working -modesetting, diff --git a/drivers/gpu/drm/gma500/Kconfig b/drivers/gpu/drm/gma500/Kconfig index 807b989e3c77..2efc0eb41c64 100644 --- a/drivers/gpu/drm/gma500/Kconfig +++ b/drivers/gpu/drm/gma500/Kconfig @@ -3,6 +3,8 @@ config DRM_GMA500 tristate "Intel GMA500/600/3600/3650 KMS Framebuffer" depends on DRM && PCI && X86 && MMU select DRM_KMS_HELPER + select I2C + select I2C_ALGOBIT # GMA500 depends on ACPI_VIDEO when ACPI is enabled, just like i915 select ACPI_VIDEO if ACPI select BACKLIGHT_CLASS_DEVICE if ACPI diff --git a/drivers/gpu/drm/hisilicon/hibmc/Kconfig b/drivers/gpu/drm/hisilicon/hibmc/Kconfig index 4e41c144a290..126504318a4f 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/Kconfig +++ b/drivers/gpu/drm/hisilicon/hibmc/Kconfig @@ -7,6 +7,8 @@ config DRM_HISI_HIBMC select DRM_VRAM_HELPER select DRM_TTM select DRM_TTM_HELPER + select I2C + select I2C_ALGOBIT help Choose this option if you have a Hisilicon Hibmc soc chipset. If M is selected the module will be called hibmc-drm. diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index 3efce05d7b57..c6e3792622f2 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -18,6 +18,8 @@ config DRM_I915 select DRM_PANEL select DRM_MIPI_DSI select RELAY + select I2C + select I2C_ALGOBIT select IRQ_WORK # i915 depends on ACPI_VIDEO when ACPI is enabled # but for select to work, need to select ACPI_VIDEO's dependencies, ick diff --git a/drivers/gpu/drm/mgag200/Kconfig b/drivers/gpu/drm/mgag200/Kconfig index eec59658a938..b28c5e4828f4 100644 --- a/drivers/gpu/drm/mgag200/Kconfig +++ b/drivers/gpu/drm/mgag200/Kconfig @@ -4,6 +4,8 @@ config DRM_MGAG200 depends on DRM && PCI && MMU select DRM_GEM_SHMEM_HELPER select DRM_KMS_HELPER + select I2C + select I2C_ALGOBIT help This is a KMS driver for Matrox G200 chips. It supports the original MGA G200 desktop chips and the server variants. It requires 0.3.0 diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig index 03d12caf9e26..a0bb3987bf63 100644 --- a/drivers/gpu/drm/nouveau/Kconfig +++ b/drivers/gpu/drm/nouveau/Kconfig @@ -10,6 +10,8 @@ config DRM_NOUVEAU select DRM_KMS_HELPER select DRM_TTM select DRM_TTM_HELPER + select I2C + select I2C_ALGOBIT select BACKLIGHT_CLASS_DEVICE if DRM_NOUVEAU_BACKLIGHT select X86_PLATFORM_DEVICES if ACPI && X86 select ACPI_WMI if ACPI && X86 diff --git a/drivers/gpu/drm/radeo
[Nouveau] [PATCH RFC v1 26/52] drm/nouveau: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev
Prepare dropping the alias "dev" for struct drm_crtc::drm_dev. "drm_dev"
is the better name as "dev" is usually a struct device pointer.
No semantic changes.
Signed-off-by: Uwe Kleine-König
---
drivers/gpu/drm/nouveau/dispnv04/crtc.c | 58 +++--
drivers/gpu/drm/nouveau/dispnv04/cursor.c | 10 ++--
drivers/gpu/drm/nouveau/dispnv50/atom.h | 2 +-
drivers/gpu/drm/nouveau/dispnv50/crc.c | 30 +--
drivers/gpu/drm/nouveau/dispnv50/crc907d.c | 6 +--
drivers/gpu/drm/nouveau/dispnv50/crcc37d.c | 6 +--
drivers/gpu/drm/nouveau/dispnv50/crcc57d.c | 2 +-
drivers/gpu/drm/nouveau/dispnv50/disp.c | 5 +-
drivers/gpu/drm/nouveau/dispnv50/head.c | 4 +-
drivers/gpu/drm/nouveau/dispnv50/head507d.c | 26 -
drivers/gpu/drm/nouveau/dispnv50/head827d.c | 10 ++--
drivers/gpu/drm/nouveau/dispnv50/head907d.c | 26 -
drivers/gpu/drm/nouveau/dispnv50/head917d.c | 6 +--
drivers/gpu/drm/nouveau/dispnv50/headc37d.c | 18 +++
drivers/gpu/drm/nouveau/dispnv50/headc57d.c | 10 ++--
drivers/gpu/drm/nouveau/nouveau_connector.h | 2 +-
drivers/gpu/drm/nouveau/nouveau_display.c | 2 +-
17 files changed, 113 insertions(+), 110 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index a6f2e681bde9..fad5c6dc2cf7 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -56,18 +56,18 @@ nv04_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
static void
crtc_wr_cio_state(struct drm_crtc *crtc, struct nv04_crtc_reg *crtcstate, int
index)
{
- NVWriteVgaCrtc(crtc->dev, nouveau_crtc(crtc)->index, index,
+ NVWriteVgaCrtc(crtc->drm_dev, nouveau_crtc(crtc)->index, index,
crtcstate->CRTC[index]);
}
static void nv_crtc_set_digital_vibrance(struct drm_crtc *crtc, int level)
{
struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
- struct drm_device *dev = crtc->dev;
+ struct drm_device *dev = crtc->drm_dev;
struct nv04_crtc_reg *regp =
_display(dev)->mode_reg.crtc_reg[nv_crtc->index];
regp->CRTC[NV_CIO_CRE_CSB] = nv_crtc->saturation = level;
- if (nv_crtc->saturation && nv_gf4_disp_arch(crtc->dev)) {
+ if (nv_crtc->saturation && nv_gf4_disp_arch(crtc->drm_dev)) {
regp->CRTC[NV_CIO_CRE_CSB] = 0x80;
regp->CRTC[NV_CIO_CRE_5B] = nv_crtc->saturation << 2;
crtc_wr_cio_state(crtc, regp, NV_CIO_CRE_5B);
@@ -78,14 +78,15 @@ static void nv_crtc_set_digital_vibrance(struct drm_crtc
*crtc, int level)
static void nv_crtc_set_image_sharpening(struct drm_crtc *crtc, int level)
{
struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
- struct drm_device *dev = crtc->dev;
+ struct drm_device *dev = crtc->drm_dev;
struct nv04_crtc_reg *regp =
_display(dev)->mode_reg.crtc_reg[nv_crtc->index];
nv_crtc->sharpness = level;
if (level < 0) /* blur is in hw range 0x3f -> 0x20 */
level += 0x40;
regp->ramdac_634 = level;
- NVWriteRAMDAC(crtc->dev, nv_crtc->index, NV_PRAMDAC_634,
regp->ramdac_634);
+ NVWriteRAMDAC(crtc->drm_dev, nv_crtc->index, NV_PRAMDAC_634,
+ regp->ramdac_634);
}
#define PLLSEL_VPLL1_MASK \
@@ -116,7 +117,7 @@ static void nv_crtc_set_image_sharpening(struct drm_crtc
*crtc, int level)
static void nv_crtc_calc_state_ext(struct drm_crtc *crtc, struct
drm_display_mode * mode, int dot_clock)
{
- struct drm_device *dev = crtc->dev;
+ struct drm_device *dev = crtc->drm_dev;
struct nouveau_drm *drm = nouveau_drm(dev);
struct nvkm_bios *bios = nvxx_bios(>client.device);
struct nvkm_clk *clk = nvxx_clk(>client.device);
@@ -175,7 +176,7 @@ static void
nv_crtc_dpms(struct drm_crtc *crtc, int mode)
{
struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
- struct drm_device *dev = crtc->dev;
+ struct drm_device *dev = crtc->drm_dev;
struct nouveau_drm *drm = nouveau_drm(dev);
unsigned char seq1 = 0, crtc17 = 0;
unsigned char crtc1A;
@@ -236,7 +237,7 @@ nv_crtc_dpms(struct drm_crtc *crtc, int mode)
static void
nv_crtc_mode_set_vga(struct drm_crtc *crtc, struct drm_display_mode *mode)
{
- struct drm_device *dev = crtc->dev;
+ struct drm_device *dev = crtc->drm_dev;
struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
struct nv04_crtc_reg *regp =
_display(dev)->mode_reg.crtc_reg[nv_crtc->index];
struct drm_framebuffer *fb = crtc->primary->fb;
@@ -460,7 +461,7 @@ nv_crtc_mode_set_vga(struct drm_crtc *crtc, struct
drm_display_mode *mode)
static void
nv_crtc_mode_set_regs(struct drm_crtc *crtc, struct dr
[Nouveau] [PATCH v2 09/12] drm/nouveau: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Reviewed-by: Thomas Zimmermann
Reviewed-by: Jyri Sarha
Signed-off-by: Uwe Kleine-König
---
drivers/gpu/drm/nouveau/nouveau_platform.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_platform.c
b/drivers/gpu/drm/nouveau/nouveau_platform.c
index 23cd43a7fd19..bf2dc7567ea4 100644
--- a/drivers/gpu/drm/nouveau/nouveau_platform.c
+++ b/drivers/gpu/drm/nouveau/nouveau_platform.c
@@ -43,11 +43,10 @@ static int nouveau_platform_probe(struct platform_device
*pdev)
return 0;
}
-static int nouveau_platform_remove(struct platform_device *pdev)
+static void nouveau_platform_remove(struct platform_device *pdev)
{
struct drm_device *dev = platform_get_drvdata(pdev);
nouveau_drm_device_remove(dev);
- return 0;
}
#if IS_ENABLED(CONFIG_OF)
@@ -93,5 +92,5 @@ struct platform_driver nouveau_platform_driver = {
.of_match_table = of_match_ptr(nouveau_platform_match),
},
.probe = nouveau_platform_probe,
- .remove = nouveau_platform_remove,
+ .remove_new = nouveau_platform_remove,
};
--
2.39.2
[Nouveau] [PATCH v2 00/12] drm: Convert to platform remove callback returning void
Hello, (implicit) v1 of this series can be found at https://lore.kernel.org/dri-devel/20230507162616.1368908-1-u.kleine-koe...@pengutronix.de Back then the series contained 53 patches. A big bunch was already applied to drm-misc, this is the remainder; with only little changes compared to v1: - rebased to todays drm-misc-next - Squashed together the two mediatek patches - Adapted the subject prefix for the arcpgu as pointed out by Thomas Zimmermann. (This affected two patches originally, one of them was merged already before anyhow (next-20230801~41^2~34^2~179).) All these patches are pairwise independant of each other and so can be applied individually to their respective maintainer trees. I'm open to get these all in together via drm-misc, but each maintainer picking the individual patches that they are repsonsible for is maybe the easier approach?! Best regards Uwe Uwe Kleine-König (12): drm/armada: Convert to platform remove callback returning void drm/etnaviv: Convert to platform remove callback returning void drm/imx/dcss: Convert to platform remove callback returning void drm/imx/ipuv3: Convert to platform remove callback returning void drm/ingenic: Convert to platform remove callback returning void drm/kmb: Convert to platform remove callback returning void drm/mediatek: Convert to platform remove callback returning void drm/msm: Convert to platform remove callback returning void drm/nouveau: Convert to platform remove callback returning void drm/shmobile: Convert to platform remove callback returning void drm/sprd: Convert to platform remove callback returning void drm/arcpgu: Convert to platform remove callback returning void drivers/gpu/drm/armada/armada_crtc.c | 5 ++--- drivers/gpu/drm/armada/armada_drv.c | 5 ++--- drivers/gpu/drm/etnaviv/etnaviv_drv.c| 6 ++ drivers/gpu/drm/etnaviv/etnaviv_gpu.c| 5 ++--- drivers/gpu/drm/imx/dcss/dcss-drv.c | 6 ++ drivers/gpu/drm/imx/ipuv3/dw_hdmi-imx.c | 6 ++ drivers/gpu/drm/imx/ipuv3/imx-drm-core.c | 5 ++--- drivers/gpu/drm/imx/ipuv3/imx-ldb.c | 5 ++--- drivers/gpu/drm/imx/ipuv3/imx-tve.c | 5 ++--- drivers/gpu/drm/imx/ipuv3/ipuv3-crtc.c | 5 ++--- drivers/gpu/drm/imx/ipuv3/parallel-display.c | 6 ++ drivers/gpu/drm/ingenic/ingenic-drm-drv.c| 6 ++ drivers/gpu/drm/ingenic/ingenic-ipu.c| 5 ++--- drivers/gpu/drm/kmb/kmb_drv.c| 5 ++--- drivers/gpu/drm/mediatek/mtk_cec.c | 5 ++--- drivers/gpu/drm/mediatek/mtk_disp_aal.c | 6 ++ drivers/gpu/drm/mediatek/mtk_disp_ccorr.c| 6 ++ drivers/gpu/drm/mediatek/mtk_disp_color.c| 6 ++ drivers/gpu/drm/mediatek/mtk_disp_gamma.c| 6 ++ drivers/gpu/drm/mediatek/mtk_disp_merge.c| 6 ++ drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 6 ++ drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 6 ++ drivers/gpu/drm/mediatek/mtk_dp.c| 6 ++ drivers/gpu/drm/mediatek/mtk_dpi.c | 6 ++ drivers/gpu/drm/mediatek/mtk_drm_drv.c | 6 ++ drivers/gpu/drm/mediatek/mtk_dsi.c | 6 ++ drivers/gpu/drm/mediatek/mtk_hdmi.c | 5 ++--- drivers/gpu/drm/mediatek/mtk_hdmi_ddc.c | 6 ++ drivers/gpu/drm/mediatek/mtk_mdp_rdma.c | 5 ++--- drivers/gpu/drm/msm/adreno/adreno_device.c | 5 ++--- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 6 ++ drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 6 ++ drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 5 ++--- drivers/gpu/drm/msm/dp/dp_display.c | 6 ++ drivers/gpu/drm/msm/dsi/dsi.c| 6 ++ drivers/gpu/drm/msm/hdmi/hdmi.c | 6 ++ drivers/gpu/drm/msm/hdmi/hdmi_phy.c | 6 ++ drivers/gpu/drm/msm/msm_drv.c| 6 ++ drivers/gpu/drm/msm/msm_mdss.c | 6 ++ drivers/gpu/drm/nouveau/nouveau_platform.c | 5 ++--- drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c | 6 ++ drivers/gpu/drm/sprd/sprd_dpu.c | 6 ++ drivers/gpu/drm/sprd/sprd_drm.c | 5 ++--- drivers/gpu/drm/sprd/sprd_dsi.c | 6 ++ drivers/gpu/drm/tiny/arcpgu.c| 6 ++ 45 files changed, 90 insertions(+), 164 deletions(-) base-commit: 290cdd7959a734a0ef20ec096af7810177c4b9f8 -- 2.39.2
Re: [Nouveau] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
Hello Jani,
On Wed, Jul 12, 2023 at 05:34:28PM +0300, Jani Nikula wrote:
> On Wed, 12 Jul 2023, Uwe Kleine-König wrote:
> > Hello,
> >
> > while I debugged an issue in the imx-lcdc driver I was constantly
> > irritated about struct drm_device pointer variables being named "dev"
> > because with that name I usually expect a struct device pointer.
> >
> > I think there is a big benefit when these are all renamed to "drm_dev".
> > I have no strong preference here though, so "drmdev" or "drm" are fine
> > for me, too. Let the bikesheding begin!
> >
> > Some statistics:
> >
> > $ git grep -ohE 'struct drm_device *\* *[^ (),;]*' v6.5-rc1 | sort | uniq
> > -c | sort -n
> > 1 struct drm_device *adev_to_drm
> > 1 struct drm_device *drm_
> > 1 struct drm_device *drm_dev
> > 1 struct drm_device*drm_dev
> > 1 struct drm_device *pdev
> > 1 struct drm_device *rdev
> > 1 struct drm_device *vdev
> > 2 struct drm_device *dcss_drv_dev_to_drm
> > 2 struct drm_device **ddev
> > 2 struct drm_device *drm_dev_alloc
> > 2 struct drm_device *mock
> > 2 struct drm_device *p_ddev
> > 5 struct drm_device *device
> > 9 struct drm_device * dev
> > 25 struct drm_device *d
> > 95 struct drm_device *
> > 216 struct drm_device *ddev
> > 234 struct drm_device *drm_dev
> > 611 struct drm_device *drm
> >4190 struct drm_device *dev
> >
> > This series starts with renaming struct drm_crtc::dev to drm_dev. If
> > it's not only me and others like the result of this effort it should be
> > followed up by adapting the other structs and the individual usages in
> > the different drivers.
>
> I think this is an unnecessary change. In drm, a dev is usually a drm
> device, i.e. struct drm_device *.
Well, unless it's not. Prominently there is
struct drm_device {
...
struct device *dev;
...
};
which yields quite a few code locations using dev->dev which is
IMHO unnecessary irritating:
$ git grep '\dev' v6.5-rc1 drivers/gpu/drm | wc -l
1633
Also the functions that deal with both a struct device and a struct
drm_device often use "dev" for the struct device and then "ddev" for
the drm_device (see for example amdgpu_device_get_pcie_replay_count()).
> If folks insist on following through with this anyway, I'm firmly in the
> camp the name should be "drm" and nothing else.
Up to now positive feedback is in the majority.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
Re: [Nouveau] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
Hello Maxime, On Wed, Jul 12, 2023 at 02:52:38PM +0200, Maxime Ripard wrote: > On Wed, Jul 12, 2023 at 01:02:53PM +0200, Uwe Kleine-König wrote: > > > Background is that this makes merge conflicts easier to handle and detect. > > > > Really? > > FWIW, I agree with Christian here. > > > Each file (apart from include/drm/drm_crtc.h) is only touched once. So > > unless I'm missing something you don't get less or easier conflicts by > > doing it all in a single patch. But you gain the freedom to drop a > > patch for one driver without having to drop the rest with it. > > Not really, because the last patch removed the union anyway. So you have > to revert both the last patch, plus that driver one. And then you need > to add a TODO to remove that union eventually. Yes, with a single patch you have only one revert (but 194 files changed, 1264 insertions(+), 1296 deletions(-)) instead of two (one of them: 1 file changed, 9 insertions(+), 1 deletion(-); the other maybe a bit bigger). (And maybe you get away with just reverting the last patch.) With a single patch the TODO after a revert is "redo it all again (and prepare for a different set of conflicts)" while with the split series it's only "fix that one driver that was forgotten/borked" + reapply that 10 line patch. As the one who gets that TODO, I prefer the latter. So in sum: If your metric is "small count of reverted commits", you're right. If however your metric is: Better get 95% of this series' change in than maybe 0%, the split series is the way to do it. With me having spend ~3h on this series' changes, it's maybe understandable that I did it the way I did. FTR: This series was created on top of v6.5-rc1. If you apply it to drm-misc-next you get a (trivial) conflict in patch #2. If I consider to be the responsible maintainer who applies this series, I like being able to just do git am --skip then. FTR#2: In drm-misc-next is a new driver (drivers/gpu/drm/loongson/lsdc_crtc.c) so skipping the last patch for now might indeed be a good idea. > > So I still like the split version better, but I'm open to a more > > verbose reasoning from your side. > > You're doing only one thing here, really: you change the name of a > structure field. If it was shared between multiple maintainers, then > sure, splitting that up is easier for everyone, but this will go through > drm-misc, so I can't see the benefit it brings. I see your argument, but I think mine weights more. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Re: [Nouveau] [Freedreno] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
hello Sean, On Wed, Jul 12, 2023 at 02:31:02PM -0400, Sean Paul wrote: > I'd really prefer this patch (series or single) is not accepted. This > will cause problems for everyone cherry-picking patches to a > downstream kernel (LTS or distro tree). I usually wouldn't expect > sympathy here, but the questionable benefit does not outweigh the cost > IM[biased]O. I agree that for backports this isn't so nice. However with the split approach (that was argumented against here) it's not soo bad. Patch #1 (and similar changes for the other affected structures) could be trivially backported and with that it doesn't matter if you write dev or drm (or whatever name is chosen in the end); both work in the same way. But even with the one-patch-per-rename approach I'd consider the renaming a net win, because ease of understanding code has a big value. It's value is not so easy measurable as "conflicts when backporting", but it also matters in say two years from now, while backporting shouldn't be an issue then any more. Thanks for your input, best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Re: [Nouveau] [Freedreno] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
On Thu, Jul 13, 2023 at 10:41:45AM -0400, Sean Paul wrote: > On Thu, Jul 13, 2023 at 9:04 AM Uwe Kleine-König > > But even with the one-patch-per-rename approach I'd consider the > > renaming a net win, because ease of understanding code has a big value. > > It's value is not so easy measurable as "conflicts when backporting", > > but it also matters in say two years from now, while backporting > > shouldn't be an issue then any more. > > You've rightly identified the conjecture in your statement. I've been > on both sides of the argument, having written/maintained drm code > upstream and cherry-picked changes to a downstream kernel. Perhaps > it's because drm's definition of dev is ingrained in my muscle memory, > or maybe it's because I don't do a lot of upstream development these > days, but I just have a hard time seeing the benefit here. I see that my change doesn't immediately benefit those who are already at home in drivers/gpu/drm. So it's quite understandable that someone in a senior role (long time contributor or maintainer) doesn't see a big upside. In contrast I think my change (with its indisputable cost) lowers the bar for new contributors to drm. IMHO that's something a maintainer has to have on their radar, too, and that's easily undervalued in my experience. Of course in the end it's about weighting the ups and downs. Thanks Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Re: [Nouveau] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
On Wed, Jul 12, 2023 at 12:46:33PM +0200, Christian König wrote: > Am 12.07.23 um 11:46 schrieb Uwe Kleine-König: > > Hello, > > > > while I debugged an issue in the imx-lcdc driver I was constantly > > irritated about struct drm_device pointer variables being named "dev" > > because with that name I usually expect a struct device pointer. > > > > I think there is a big benefit when these are all renamed to "drm_dev". > > I have no strong preference here though, so "drmdev" or "drm" are fine > > for me, too. Let the bikesheding begin! > > > > Some statistics: > > > > $ git grep -ohE 'struct drm_device *\* *[^ (),;]*' v6.5-rc1 | sort | uniq > > -c | sort -n > >1 struct drm_device *adev_to_drm > >1 struct drm_device *drm_ > >1 struct drm_device *drm_dev > >1 struct drm_device*drm_dev > >1 struct drm_device *pdev > >1 struct drm_device *rdev > >1 struct drm_device *vdev > >2 struct drm_device *dcss_drv_dev_to_drm > >2 struct drm_device **ddev > >2 struct drm_device *drm_dev_alloc > >2 struct drm_device *mock > >2 struct drm_device *p_ddev > >5 struct drm_device *device > >9 struct drm_device * dev > > 25 struct drm_device *d > > 95 struct drm_device * > > 216 struct drm_device *ddev > > 234 struct drm_device *drm_dev > > 611 struct drm_device *drm > > 4190 struct drm_device *dev > > > > This series starts with renaming struct drm_crtc::dev to drm_dev. If > > it's not only me and others like the result of this effort it should be > > followed up by adapting the other structs and the individual usages in > > the different drivers. > > > > To make this series a bit easier handleable, I first added an alias for > > drm_crtc::dev, then converted the drivers one after another and the last > > patch drops the "dev" name. This has the advantage of being easier to > > review, and if I should have missed an instance only the last patch must > > be dropped/reverted. Also this series might conflict with other patches, > > in this case the remaining patches can still go in (apart from the last > > one of course). Maybe it also makes sense to delay applying the last > > patch by one development cycle? > > When you automatically generate the patch (with cocci for example) I usually > prefer a single patch instead. Maybe I'm too stupid, but only parts of this patch were created by coccinelle. I failed to convert code like - spin_lock_irq(>dev->event_lock); + spin_lock_irq(>drm_dev->event_lock); Added Julia to Cc, maybe she has a hint?! (Up to now it's only @@ struct drm_crtc *crtc; @@ -crtc->dev +crtc->drm_dev ) > Background is that this makes merge conflicts easier to handle and detect. Really? Each file (apart from include/drm/drm_crtc.h) is only touched once. So unless I'm missing something you don't get less or easier conflicts by doing it all in a single patch. But you gain the freedom to drop a patch for one driver without having to drop the rest with it. So I still like the split version better, but I'm open to a more verbose reasoning from your side. > When you have multiple patches and a merge conflict because of some added > lines using the old field the build breaks only on the last patch which > removes the old field. Then you can revert/drop the last patch without having to respin the whole single patch and thus caring for still more conflicts that arise until the new version is sent. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
[Nouveau] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
Hello, while I debugged an issue in the imx-lcdc driver I was constantly irritated about struct drm_device pointer variables being named "dev" because with that name I usually expect a struct device pointer. I think there is a big benefit when these are all renamed to "drm_dev". I have no strong preference here though, so "drmdev" or "drm" are fine for me, too. Let the bikesheding begin! Some statistics: $ git grep -ohE 'struct drm_device *\* *[^ (),;]*' v6.5-rc1 | sort | uniq -c | sort -n 1 struct drm_device *adev_to_drm 1 struct drm_device *drm_ 1 struct drm_device *drm_dev 1 struct drm_device*drm_dev 1 struct drm_device *pdev 1 struct drm_device *rdev 1 struct drm_device *vdev 2 struct drm_device *dcss_drv_dev_to_drm 2 struct drm_device **ddev 2 struct drm_device *drm_dev_alloc 2 struct drm_device *mock 2 struct drm_device *p_ddev 5 struct drm_device *device 9 struct drm_device * dev 25 struct drm_device *d 95 struct drm_device * 216 struct drm_device *ddev 234 struct drm_device *drm_dev 611 struct drm_device *drm 4190 struct drm_device *dev This series starts with renaming struct drm_crtc::dev to drm_dev. If it's not only me and others like the result of this effort it should be followed up by adapting the other structs and the individual usages in the different drivers. To make this series a bit easier handleable, I first added an alias for drm_crtc::dev, then converted the drivers one after another and the last patch drops the "dev" name. This has the advantage of being easier to review, and if I should have missed an instance only the last patch must be dropped/reverted. Also this series might conflict with other patches, in this case the remaining patches can still go in (apart from the last one of course). Maybe it also makes sense to delay applying the last patch by one development cycle? The series was compile tested for arm, arm64, powerpc and amd64 using an allmodconfig (though I only build drivers/gpu/). Best regards Uwe Uwe Kleine-König (52): drm/crtc: Start renaming struct drm_crtc::dev to drm_dev drm/core: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/amd: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/armada: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/arm: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/aspeed: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/ast: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/atmel-hlcdc: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/exynos: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/fsl-dcu: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/gma500: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/gud: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/hisilicon: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/hyperv: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/i915: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/imx: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/ingenic: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/kmb: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/logicvc: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/mcde: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/mediatek: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/meson: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/mgag200: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/msm: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/mxsfb: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/nouveau: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/omapdrm: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/panel-ili9341: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/pl111: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/qxl: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/radeon: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/renesas: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/rockchip: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/solomon: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/sprd: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/sti: Use struct drm_crtc::drm_dev instead of struct drm_crtc::dev drm/stm: Use struct drm_crtc::drm_dev instead of str
Re: [Nouveau] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
On Thu, Jul 13, 2023 at 12:03:05PM +0300, Jani Nikula wrote:
> On Wed, 12 Jul 2023, Uwe Kleine-König wrote:
> > Hello Jani,
> >
> > On Wed, Jul 12, 2023 at 05:34:28PM +0300, Jani Nikula wrote:
> >> On Wed, 12 Jul 2023, Uwe Kleine-König
> >> wrote:
> >> > Hello,
> >> >
> >> > while I debugged an issue in the imx-lcdc driver I was constantly
> >> > irritated about struct drm_device pointer variables being named "dev"
> >> > because with that name I usually expect a struct device pointer.
> >> >
> >> > I think there is a big benefit when these are all renamed to "drm_dev".
> >> > I have no strong preference here though, so "drmdev" or "drm" are fine
> >> > for me, too. Let the bikesheding begin!
> >> >
> >> > Some statistics:
> >> >
> >> > $ git grep -ohE 'struct drm_device *\* *[^ (),;]*' v6.5-rc1 | sort |
> >> > uniq -c | sort -n
> >> > 1 struct drm_device *adev_to_drm
> >> > 1 struct drm_device *drm_
> >> > 1 struct drm_device *drm_dev
> >> > 1 struct drm_device*drm_dev
> >> > 1 struct drm_device *pdev
> >> > 1 struct drm_device *rdev
> >> > 1 struct drm_device *vdev
> >> > 2 struct drm_device *dcss_drv_dev_to_drm
> >> > 2 struct drm_device **ddev
> >> > 2 struct drm_device *drm_dev_alloc
> >> > 2 struct drm_device *mock
> >> > 2 struct drm_device *p_ddev
> >> > 5 struct drm_device *device
> >> > 9 struct drm_device * dev
> >> > 25 struct drm_device *d
> >> > 95 struct drm_device *
> >> > 216 struct drm_device *ddev
> >> > 234 struct drm_device *drm_dev
> >> > 611 struct drm_device *drm
> >> >4190 struct drm_device *dev
> >> >
> >> > This series starts with renaming struct drm_crtc::dev to drm_dev. If
> >> > it's not only me and others like the result of this effort it should be
> >> > followed up by adapting the other structs and the individual usages in
> >> > the different drivers.
> >>
> >> I think this is an unnecessary change. In drm, a dev is usually a drm
> >> device, i.e. struct drm_device *.
> >
> > Well, unless it's not. Prominently there is
> >
> > struct drm_device {
> > ...
> > struct device *dev;
> > ...
> > };
> >
> > which yields quite a few code locations using dev->dev which is
> > IMHO unnecessary irritating:
> >
> > $ git grep '\dev' v6.5-rc1 drivers/gpu/drm | wc -l
> > 1633
> >
> > Also the functions that deal with both a struct device and a struct
> > drm_device often use "dev" for the struct device and then "ddev" for
> > the drm_device (see for example amdgpu_device_get_pcie_replay_count()).
>
> Why is specifically struct drm_device *dev so irritating to you?
>
> You lead us to believe it's an outlier in kernel, something that goes
> against common kernel style, but it's really not:
>
> $ git grep -how "struct [A-Za-z0-9_]\+ \*dev" | sort | uniq -c | sort -rn |
> head -20
> 38494 struct device *dev
> 16388 struct net_device *dev
>4184 struct drm_device *dev
>2780 struct pci_dev *dev
>1916 struct comedi_device *dev
>1510 struct mlx5_core_dev *dev
>1057 struct mlx4_dev *dev
> 894 struct b43_wldev *dev
> 762 struct input_dev *dev
> 623 struct usbnet *dev
> 561 struct mlx5_ib_dev *dev
> 525 struct mt76_dev *dev
> 465 struct mt76x02_dev *dev
> 435 struct platform_device *dev
> 431 struct usb_device *dev
> 411 struct mt7915_dev *dev
> 398 struct cx231xx *dev
> 378 struct mei_device *dev
> 363 struct ksz_device *dev
> 359 struct mthca_dev *dev
>
> A good portion of the above also have a dev member.
Yeah, other subsystems and drivers have the same problem. You're lucky
that I noticed drm first and invested some effort to improve it. IMHO
other subsystems being bad shouldn't stop drm to improve here.
And note that for example for pci_dev there are 5794 instances that are
named "pdev" and there are 9971 struct platform_device that are called
"pdev", too. So the majority for these does it better. And agreed,
net_device and others are also inconsistent. If you want an area that is
better, look at the naming of i2c_clie
Re: [Nouveau] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
On Thu, Jul 13, 2023 at 08:52:12AM +0200, Geert Uytterhoeven wrote:
> Hi Uwe,
>
> Let's add some fuel to keep the thread alive ;-)
>
> On Wed, Jul 12, 2023 at 6:13 PM Uwe Kleine-König
> wrote:
> > On Wed, Jul 12, 2023 at 05:34:28PM +0300, Jani Nikula wrote:
> > > I think this is an unnecessary change. In drm, a dev is usually a drm
> > > device, i.e. struct drm_device *.
> >
> > Well, unless it's not. Prominently there is
> >
> > struct drm_device {
> > ...
> > struct device *dev;
> > ...
> > };
> >
> > which yields quite a few code locations using dev->dev which is
> > IMHO unnecessary irritating:
> >
> > $ git grep '\dev' v6.5-rc1 drivers/gpu/drm | wc -l
> > 1633
>
> I find that irritating as well...
>
> Same for e.g. crtc->crtc.
>
> Hence that's why I had sent patches to rename the base members in the
> shmob_drm-specific subclasses of drm_{crtc,connector,plane} to "base".
> https://lore.kernel.org/dri-devel/b3daca80f82625ba14e3aeaf2fca6dcefa056e47.1687423204.git.geert+rene...@glider.be
>
> > Also the functions that deal with both a struct device and a struct
> > drm_device often use "dev" for the struct device and then "ddev" for
> > the drm_device (see for example amdgpu_device_get_pcie_replay_count()).
>
> I guess you considered "drm_dev", because it is still a short name?
I considered drm_dev because it is still moderately short and a good
approximation of "drm_device". Other than that the main driving force to
pick "drm_dev" was that it's unique enough that I could have done
s/\/$nameofchoice/ on the initial patch and get it mostly
right.
> Code dealing with platform devices usually uses "pdev" and "dev".
> Same for PCI drivers (despite "pci_dev" being a short name).
pci_dev and platform_device both typlically using pdev already annoyed
me in the past. However less than drm_device *dev because for pci_dev +
platform_device there is little overlap.
> So my personal preference goes to "ddev".
I sticked to "drm" for the new series. I think this provides less fuel.
Best regards and thanks for your thoughts,
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
Re: [Nouveau] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
Hello Thomas, On Wed, Jul 12, 2023 at 12:19:37PM +0200, Thomas Zimmermann wrote: > Am 12.07.23 um 11:46 schrieb Uwe Kleine-König: > > Hello, > > > > while I debugged an issue in the imx-lcdc driver I was constantly > > irritated about struct drm_device pointer variables being named "dev" > > because with that name I usually expect a struct device pointer. > > > > I think there is a big benefit when these are all renamed to "drm_dev". > > If you rename drm_crtc.dev, you should also address *all* other data > structures. Yes. Changing drm_crtc::dev was some effort, so I thought to send that one out before doing the same to drm_dp_mst_topology_mgr drm_atomic_state drm_master drm_bridge drm_client_dev drm_connector drm_debugfs_entry drm_encoder drm_fb_helper drm_minor drm_framebuffer drm_gem_object drm_plane drm_property drm_property_blob drm_vblank_crtc when in the end the intention isn't welcome. > > I have no strong preference here though, so "drmdev" or "drm" are fine > > for me, too. Let the bikesheding begin! > > We've discussed this to death. IIRC 'drm' would be the prefered choice. "drm" at least has the advantage to be the 2nd most common name. With Paul Kocialkowski prefering "drm_dev" there is no clear favourite yet. Maybe all the other people with strong opinions are dead if this was "discussed to death" before? :-) Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
[Nouveau] patches dropped from drm-misc-next [Was: Re: [PATCH 00/53] drm: Convert to platform remove callback returning] void
[expanding recipents by the other affected persons]
On Thu, Jun 08, 2023 at 09:08:15AM -0700, Doug Anderson wrote:
> On Thu, Jun 1, 2023 at 8:40 AM Uwe Kleine-König
> wrote:
> >
> > Hello,
> >
> > On Sun, May 07, 2023 at 06:25:23PM +0200, Uwe Kleine-König wrote:
> > > this patch series adapts the platform drivers below drivers/gpu/drm
> > > to use the .remove_new() callback. Compared to the traditional .remove()
> > > callback .remove_new() returns no value. This is a good thing because
> > > the driver core doesn't (and cannot) cope for errors during remove. The
> > > only effect of a non-zero return value in .remove() is that the driver
> > > core emits a warning. The device is removed anyhow and an early return
> > > from .remove() usually yields a resource leak.
> > >
> > > By changing the remove callback to return void driver authors cannot
> > > reasonably (but wrongly) assume any more that there happens some kind of
> > > cleanup later.
> >
> > I wonder if someone would volunteer to add the whole series to
> > drm-misc-next?!
>
> It looks as if Neil applied quite a few of them already, so I looked
> at what was left...
>
> I'm a little hesitant to just apply the whole kit-and-caboodle to
> drm-misc-next since there are specific DRM trees for a bunch of them
> and it would be better if they landed there. ...so I went through all
> the patches that still applied to drm-misc-next, then used
> 'scripts/get_maintainer.pl --scm' to check if they were maintained
> through drm-misc. That still left quite a few patches. I've applied
> those ones and pushed to drm-misc-next:
>
> 71722685cd17 drm/xlnx/zynqmp_dpsub: Convert to platform remove
> callback returning void
> 1ed54a19f3b3 drm/vc4: Convert to platform remove callback returning void
> b957812839f8 drm/v3d: Convert to platform remove callback returning void
> e2fd3192e267 drm/tve200: Convert to platform remove callback returning void
> 84e6da7ad553 drm/tiny: Convert to platform remove callback returning void
> 34cdd1f691ad drm/tidss: Convert to platform remove callback returning void
> d665e3c9d37a drm/sun4i: Convert to platform remove callback returning void
> 0c259ab19146 drm/stm: Convert to platform remove callback returning void
> 9a865e45884a drm/sti: Convert to platform remove callback returning void
> 3c855610840e drm/rockchip: Convert to platform remove callback returning void
> e41977a83b71 drm/panfrost: Convert to platform remove callback returning void
> cef3776d0b5a drm/panel: Convert to platform remove callback returning void
> bd296a594e87 drm/mxsfb: Convert to platform remove callback returning void
> 38ca2d93d323 drm/meson: Convert to platform remove callback returning void
> fd1457d84bae drm/mcde: Convert to platform remove callback returning void
> 41a56a18615c drm/logicvc: Convert to platform remove callback returning void
> 980ec6444372 drm/lima: Convert to platform remove callback returning void
> 82a2c0cc1a22 drm/hisilicon: Convert to platform remove callback returning void
> c3b28b29ac0a drm/fsl-dcu: Convert to platform remove callback returning void
> a118fc6e71f9 drm/atmel-hlcdc: Convert to platform remove callback returning
> void
> 9a32dd324c46 drm/aspeed: Convert to platform remove callback returning void
> 2c7d291c498c drm/arm/malidp: Convert to platform remove callback returning
> void
> a920028df679 drm/arm/hdlcd: Convert to platform remove callback returning void
> 1bf3d76a7d15 drm/komeda: Convert to platform remove callback returning void
Together with the patches that were applied later the topmost commit
from this series is c2807ecb5290 ("drm/omap: Convert to platform remove
callback returning void"). This commit was part for the following next
tags:
$ git tag -l --contains c2807ecb5290
next-20230609
next-20230613
next-20230614
next-20230615
However in next-20230616 they are missing. In next-20230616
drm-misc/for-linux-next was cf683e8870bd4be0fd6b98639286700a35088660.
Compared to c2807ecb5290 this adds 1149 patches but drops 37 (that are
also not included with a different commit id). The 37 patches dropped
are 13cdd12a9f934158f4ec817cf048fcb4384aa9dc..c2807ecb5290:
$ git shortlog -s 13cdd12a9f934158f4ec817cf048fcb4384aa9dc..c2807ecb5290
1 Christophe JAILLET
2 Jessica Zhang
5 Karol Wachowski
1 Laura Nao
27 Uwe Kleine-König
1 Wang Jianzheng
I guess this was done by mistake because nobody told me about dropping
my/these patches? Can c2807ecb5290 please be merged into drm-misc-next
again?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
Re: [Nouveau] patches dropped from drm-misc-next [Was: Re: [PATCH 00/53] drm: Convert to platform remove callback returning] void
Hello Doug,
On Sat, Jun 17, 2023 at 10:57:23AM -0700, Doug Anderson wrote:
> On Sat, Jun 17, 2023 at 9:15 AM Uwe Kleine-König
> wrote:
> >
> > [expanding recipents by the other affected persons]
> >
> > On Thu, Jun 08, 2023 at 09:08:15AM -0700, Doug Anderson wrote:
> > > On Thu, Jun 1, 2023 at 8:40 AM Uwe Kleine-König
> > > wrote:
> > > >
> > > > Hello,
> > > >
> > > > On Sun, May 07, 2023 at 06:25:23PM +0200, Uwe Kleine-König wrote:
> > > > > this patch series adapts the platform drivers below drivers/gpu/drm
> > > > > to use the .remove_new() callback. Compared to the traditional
> > > > > .remove()
> > > > > callback .remove_new() returns no value. This is a good thing because
> > > > > the driver core doesn't (and cannot) cope for errors during remove.
> > > > > The
> > > > > only effect of a non-zero return value in .remove() is that the driver
> > > > > core emits a warning. The device is removed anyhow and an early return
> > > > > from .remove() usually yields a resource leak.
> > > > >
> > > > > By changing the remove callback to return void driver authors cannot
> > > > > reasonably (but wrongly) assume any more that there happens some kind
> > > > > of
> > > > > cleanup later.
> > > >
> > > > I wonder if someone would volunteer to add the whole series to
> > > > drm-misc-next?!
> > >
> > > It looks as if Neil applied quite a few of them already, so I looked
> > > at what was left...
> > >
> > > I'm a little hesitant to just apply the whole kit-and-caboodle to
> > > drm-misc-next since there are specific DRM trees for a bunch of them
> > > and it would be better if they landed there. ...so I went through all
> > > the patches that still applied to drm-misc-next, then used
> > > 'scripts/get_maintainer.pl --scm' to check if they were maintained
> > > through drm-misc. That still left quite a few patches. I've applied
> > > those ones and pushed to drm-misc-next:
> > >
> > > 71722685cd17 drm/xlnx/zynqmp_dpsub: Convert to platform remove
> > > callback returning void
> > > 1ed54a19f3b3 drm/vc4: Convert to platform remove callback returning void
> > > b957812839f8 drm/v3d: Convert to platform remove callback returning void
> > > e2fd3192e267 drm/tve200: Convert to platform remove callback returning
> > > void
> > > 84e6da7ad553 drm/tiny: Convert to platform remove callback returning void
> > > 34cdd1f691ad drm/tidss: Convert to platform remove callback returning void
> > > d665e3c9d37a drm/sun4i: Convert to platform remove callback returning void
> > > 0c259ab19146 drm/stm: Convert to platform remove callback returning void
> > > 9a865e45884a drm/sti: Convert to platform remove callback returning void
> > > 3c855610840e drm/rockchip: Convert to platform remove callback returning
> > > void
> > > e41977a83b71 drm/panfrost: Convert to platform remove callback returning
> > > void
> > > cef3776d0b5a drm/panel: Convert to platform remove callback returning void
> > > bd296a594e87 drm/mxsfb: Convert to platform remove callback returning void
> > > 38ca2d93d323 drm/meson: Convert to platform remove callback returning void
> > > fd1457d84bae drm/mcde: Convert to platform remove callback returning void
> > > 41a56a18615c drm/logicvc: Convert to platform remove callback returning
> > > void
> > > 980ec6444372 drm/lima: Convert to platform remove callback returning void
> > > 82a2c0cc1a22 drm/hisilicon: Convert to platform remove callback returning
> > > void
> > > c3b28b29ac0a drm/fsl-dcu: Convert to platform remove callback returning
> > > void
> > > a118fc6e71f9 drm/atmel-hlcdc: Convert to platform remove callback
> > > returning void
> > > 9a32dd324c46 drm/aspeed: Convert to platform remove callback returning
> > > void
> > > 2c7d291c498c drm/arm/malidp: Convert to platform remove callback
> > > returning void
> > > a920028df679 drm/arm/hdlcd: Convert to platform remove callback returning
> > > void
> > > 1bf3d76a7d15 drm/komeda: Convert to platform remove callback returning
> > > void
> >
> > Together with the patches that were applied later the topmost commit
> > from this series is c2807ecb5290 ("drm/omap: Convert to platform remove
> > callback returning void"). This commit was part for th
Re: [Nouveau] patches dropped from drm-misc-next [Was: Re: [PATCH 00/53] drm: Convert to platform remove callback returning] void
Hello Maxime,
On Sun, Jun 18, 2023 at 04:32:55PM +0200, Maxime Ripard wrote:
> On Sun, Jun 18, 2023 at 02:39:15PM +0200, Uwe Kleine-König wrote:
> > On Sat, Jun 17, 2023 at 10:57:23AM -0700, Doug Anderson wrote:
> > > On Sat, Jun 17, 2023 at 9:15 AM Uwe Kleine-König
> > > wrote:
> > > > Together with the patches that were applied later the topmost commit
> > > > from this series is c2807ecb5290 ("drm/omap: Convert to platform remove
> > > > callback returning void"). This commit was part for the following next
> > > > tags:
> > > >
> > > > $ git tag -l --contains c2807ecb5290
> > > > next-20230609
> > > > next-20230613
> > > > next-20230614
> > > > next-20230615
> > > >
> > > > However in next-20230616 they are missing. In next-20230616
> > > > drm-misc/for-linux-next was cf683e8870bd4be0fd6b98639286700a35088660.
> > > > Compared to c2807ecb5290 this adds 1149 patches but drops 37 (that are
> > > > also not included with a different commit id). The 37 patches dropped
> > > > are 13cdd12a9f934158f4ec817cf048fcb4384aa9dc..c2807ecb5290:
> > > >
> > > > $ git shortlog -s
> > > > 13cdd12a9f934158f4ec817cf048fcb4384aa9dc..c2807ecb5290
> > > > 1 Christophe JAILLET
> > > > 2 Jessica Zhang
> > > > 5 Karol Wachowski
> > > > 1 Laura Nao
> > > > 27 Uwe Kleine-König
> > > > 1 Wang Jianzheng
> > > >
> > > >
> > > > I guess this was done by mistake because nobody told me about dropping
> > > > my/these patches? Can c2807ecb5290 please be merged into drm-misc-next
> > > > again?
> > >
> > > Actually, it was probably a mistake that these patches got merged to
> > > linuxnext during the 4 days that you noticed. However, your patches
> > > aren't dropped and are still present in drm-misc-next.
> > >
> > > drm-misc has a bit of a unique model and it's documented fairly well here:
> > >
> > > https://drm.pages.freedesktop.org/maintainer-tools/drm-misc.html
> >
> > Is there a flaw then in this unique model (or its implementation) when
> > drm-misc/for-linux-next moves in a non-fast-forward manner? This isn't
> > expected, is it?
>
> There's no expectation afaik. Any tree merged in linux-next can be
> rebased, drop a patch, amend one, etc. without any concern.
I agree that there are no rules broken for a tree that is included in
next and a maintainer is free to rewrite their tree independant of the
tree being included in next.
Still I think that shouldn't be used as an excuse. For me, if a
maintainer puts some patch into next that's a statement saying
(approximately) "I think this patch is fine and I intend to send it to
Linus during the next merge window.". So my expectation is that if a
patch is dropped again from next, there was a problem and it would be
fair if the maintainer tells the author/submitter about this problem and
that the patch was dropped.
So my concern is not about rule breaking, but about the strange signal
that is sent to contributors by including their work in next for some
time and then dropping it again without comment.
> It's also why linux-next is rebuilt entirely every day.
>
> > > The key is that committers can commit to drm-misc-next _at any time_
> > > regardless of the merge window. The drm-misc merge strategy makes this
> > > OK. Specifically, when it's late in the linux cycle then drm-misc-next
> > > is supposed to stop merging to linuxnext. Then, shortly after the
> > > merge window closes, patches will start flowing again.
> > >
> > > So basically your patches are landed and should even keep the same git
> > > hashes when they eventually make it to Linux. They just won't land for
> > > another release cycle of Linux.
> >
> > OK, c2807ecb5290 is still included in drm-misc-next. So while I don't
> > understand the whole model, the patches at least seem to be scheduled to
> > go in during the next merge window.
>
> It's not that complicated.
>
> drm-misc-next is always open, and we start targeting release X + 2 when
> X-rc6 is released.
>
> This is due to Linus wanting all the commits sent as part of the PR in
> linux-next for two weeks.
>
> In order to converge towards (X + 1)-rc1 in linux-next, as soon as X-rc6
> is released, we remove drm-misc-next from the linux-next branch. All the
> patches in drm-misc-next that were targetting X + 1 are in drm/next by
> then, so it's not a concern.
So if I were a maintainer of drm-misc, I'd want that no commit from
drm-misc-next migrates to next after -rc6.
Also note that the branch head in question (i.e. c2807ecb5290) was
included in next-20230609, while v6.4-rc6 was tagged on Jun 11. So
according to the rules you described c2807ecb5290 could have been
qualified to go into v6.5-rc1?!
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
Re: [Nouveau] [PATCH 00/53] drm: Convert to platform remove callback returning void
On Mon, May 15, 2023 at 04:50:57PM +0900, Inki Dae wrote: > Hi, > > 2023년 5월 8일 (월) 오전 1:32, Uwe Kleine-König 님이 > 작성: > > > > Hello, > > > > this patch series adapts the platform drivers below drivers/gpu/drm > > to use the .remove_new() callback. Compared to the traditional .remove() > > callback .remove_new() returns no value. This is a good thing because > > First of all, I apologize for the delay in providing my review comments. > > Not related to this patch but seems that the "remove_new" callback > naming implicitly implies that there is no need to return anything > since its return type is void. To help users understand the intended > behavior based on the callback name, how about considering a modified > naming convention like "remove_no_return" or something similar? > > The relevant patch has already been merged as outlined below, > author Uwe Kleine-König 2022-12-09 > 16:09:14 +0100 > committer Greg Kroah-Hartman 2023-01-17 > 19:04:17 +0100 > commit 5c5a7680e67ba6fbbb5f4d79fa41485450c1985c (patch) > tree 0b6dbc003a6bb4a3f7fb084d31326bbfa3ba3f7c > parent 7bbb89b420d9e290cb34864832de8fcdf2c140dc (diff) > download linux-5c5a7680e67ba6fbbb5f4d79fa41485450c1985c.tar.gz > platform: Provide a remove callback that returns no value > > Maybe a trivial thing but how about renaming it? I think the postfix, > 'new', is a very generic word. I think you could introduce another > patch for it if you think it's reasonable. .remove_new is only a temporary name. Once all drivers are converted, .remove is changed to return void and then all drivers are converted back. While "remove_new" might not be a brilliant name choice, touching all already converted drivers again just to improve the temporary measures doesn't sound right. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Re: [Nouveau] patches dropped from drm-misc-next [Was: Re: [PATCH 00/53] drm: Convert to platform remove callback returning] void
On Mon, Jun 19, 2023 at 11:45:37AM +0200, Maxime Ripard wrote:
> On Sun, Jun 18, 2023 at 06:29:50PM +0200, Uwe Kleine-König wrote:
> > Hello Maxime,
> >
> > On Sun, Jun 18, 2023 at 04:32:55PM +0200, Maxime Ripard wrote:
> > > On Sun, Jun 18, 2023 at 02:39:15PM +0200, Uwe Kleine-König wrote:
> > > > On Sat, Jun 17, 2023 at 10:57:23AM -0700, Doug Anderson wrote:
> > > > > On Sat, Jun 17, 2023 at 9:15 AM Uwe Kleine-König
> > > > > wrote:
> > > > > > Together with the patches that were applied later the topmost commit
> > > > > > from this series is c2807ecb5290 ("drm/omap: Convert to platform
> > > > > > remove
> > > > > > callback returning void"). This commit was part for the following
> > > > > > next
> > > > > > tags:
> > > > > >
> > > > > > $ git tag -l --contains c2807ecb5290
> > > > > > next-20230609
> > > > > > next-20230613
> > > > > > next-20230614
> > > > > > next-20230615
> > > > > >
> > > > > > However in next-20230616 they are missing. In next-20230616
> > > > > > drm-misc/for-linux-next was
> > > > > > cf683e8870bd4be0fd6b98639286700a35088660.
> > > > > > Compared to c2807ecb5290 this adds 1149 patches but drops 37 (that
> > > > > > are
> > > > > > also not included with a different commit id). The 37 patches
> > > > > > dropped
> > > > > > are 13cdd12a9f934158f4ec817cf048fcb4384aa9dc..c2807ecb5290:
> > > > > >
> > > > > > $ git shortlog -s
> > > > > > 13cdd12a9f934158f4ec817cf048fcb4384aa9dc..c2807ecb5290
> > > > > > 1 Christophe JAILLET
> > > > > > 2 Jessica Zhang
> > > > > > 5 Karol Wachowski
> > > > > > 1 Laura Nao
> > > > > > 27 Uwe Kleine-König
> > > > > > 1 Wang Jianzheng
> > > > > >
> > > > > >
> > > > > > I guess this was done by mistake because nobody told me about
> > > > > > dropping
> > > > > > my/these patches? Can c2807ecb5290 please be merged into
> > > > > > drm-misc-next
> > > > > > again?
> > > > >
> > > > > Actually, it was probably a mistake that these patches got merged to
> > > > > linuxnext during the 4 days that you noticed. However, your patches
> > > > > aren't dropped and are still present in drm-misc-next.
> > > > >
> > > > > drm-misc has a bit of a unique model and it's documented fairly well
> > > > > here:
> > > > >
> > > > > https://drm.pages.freedesktop.org/maintainer-tools/drm-misc.html
> > > >
> > > > Is there a flaw then in this unique model (or its implementation) when
> > > > drm-misc/for-linux-next moves in a non-fast-forward manner? This isn't
> > > > expected, is it?
> > >
> > > There's no expectation afaik. Any tree merged in linux-next can be
> > > rebased, drop a patch, amend one, etc. without any concern.
> >
> > I agree that there are no rules broken for a tree that is included in
> > next and a maintainer is free to rewrite their tree independant of the
> > tree being included in next.
> >
> > Still I think that shouldn't be used as an excuse.
>
> As an excuse for what?
Just because the rules for trees in next allow the merged branch to be
rewritten, shouldn't be used to justify rewriting the branch.
IMHO you still should ensure that only commits make it into any next
snapshot via your tree before X-rc1 for some X (e.g. v6.5) that you
intend to be included in X-rc1.
> > For me, if a maintainer puts some patch into next that's a statement
> > saying (approximately) "I think this patch is fine and I intend to
> > send it to Linus during the next merge window.".
>
> I mean, that's what we're saying and doing?
No, on 2023-06-09 I assumed that my patches will go into v6.5-rc1 (as it
was part of next-20230609). A few days later however the patches were
dropped.
The two options that would have made the experience smoother for me are:
a) keep c2807ecb5290 in next and send it for v6.5-rc1; or
b) keep c2807ecb5290 in a branch that doesn't result it entering next
before v6.5-rc1.
> > So my expectation is that if a patch is dropped again from next, there
> > was a problem and it would be fair if the maintainer tells the
> > author/submitter about this problem and that the patch was dropped.
>
> But it wasn't dropped,
From my POV it was dropped from next as it was part of next between
next-20230609 and next-20230615 but not any more since next-20230616.
You talk about (not) being dropped from some branch in drm-misc, that's
irrelevant for the thing I'm complaining about.
> it's still very much to be sent to Linus during the next merge window.
"next merge window" as in the one leading to 6.5-rc1? Either we mean
different things when we say "next merge window", or there is a
misunderstanding I don't see yet.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
[Nouveau] [PATCH 00/53] drm: Convert to platform remove callback returning void
Hello, this patch series adapts the platform drivers below drivers/gpu/drm to use the .remove_new() callback. Compared to the traditional .remove() callback .remove_new() returns no value. This is a good thing because the driver core doesn't (and cannot) cope for errors during remove. The only effect of a non-zero return value in .remove() is that the driver core emits a warning. The device is removed anyhow and an early return from .remove() usually yields a resource leak. By changing the remove callback to return void driver authors cannot reasonably (but wrongly) assume any more that there happens some kind of cleanup later. Best regards Uwe Uwe Kleine-König (53): drm/komeda: Convert to platform remove callback returning void drm/arm/hdlcd: Convert to platform remove callback returning void drm/arm/malidp: Convert to platform remove callback returning void drm/armada: Convert to platform remove callback returning void drm/aspeed: Convert to platform remove callback returning void drm/atmel-hlcdc: Convert to platform remove callback returning void drm/bridge: cdns-dsi: Convert to platform remove callback returning void drm/bridge: display-connector: Convert to platform remove callback returning void drm/bridge: fsl-ldb: Convert to platform remove callback returning void drm/imx/imx8*: Convert to platform remove callback returning void drm/bridge: lvds-codec: Convert to platform remove callback returning void drm/bridge: nwl-dsi: Convert to platform remove callback returning void drm/bridge: simple-bridge: Convert to platform remove callback returning void drm/bridge: synopsys: Convert to platform remove callback returning void drm/bridge: thc63lvd1024: Convert to platform remove callback returning void drm/bridge: tfp410: Convert to platform remove callback returning void drm/etnaviv: Convert to platform remove callback returning void drm/exynos: Convert to platform remove callback returning void drm/fsl-dcu: Convert to platform remove callback returning void drm/hisilicon: Convert to platform remove callback returning void drm/imx/dcss: Convert to platform remove callback returning void drm/imx/ipuv3: Convert to platform remove callback returning void drm/ingenic: Convert to platform remove callback returning void drm/kmb: Convert to platform remove callback returning void drm/lima: Convert to platform remove callback returning void drm/logicvc: Convert to platform remove callback returning void drm/mcde: Convert to platform remove callback returning void drm/mediatek: Convert to platform remove callback returning void drm/mediatek: Convert to platform remove callback returning void drm/meson: Convert to platform remove callback returning void drm/msm: Convert to platform remove callback returning void drm/mxsfb: Convert to platform remove callback returning void drm/nouveau: Convert to platform remove callback returning void drm/omap: Convert to platform remove callback returning void drm/panel: Convert to platform remove callback returning void drm/panfrost: Convert to platform remove callback returning void drm/rcar-du: Convert to platform remove callback returning void drm/rockchip: Convert to platform remove callback returning void drm/shmobile: Convert to platform remove callback returning void drm/sprd: Convert to platform remove callback returning void drm/sti: Convert to platform remove callback returning void drm/stm: Convert to platform remove callback returning void drm/sun4i: Convert to platform remove callback returning void drm/tegra: Convert to platform remove callback returning void drm/tests: helpers: Convert to platform remove callback returning void drm/tidss: Convert to platform remove callback returning void drm/tilcdc: Convert to platform remove callback returning void drm/tiny: Convert to platform remove callback returning void drm/tiny: Convert to platform remove callback returning void drm/tve200: Convert to platform remove callback returning void drm/v3d: Convert to platform remove callback returning void drm/vc4: Convert to platform remove callback returning void drm/xlnx/zynqmp_dpsub: Convert to platform remove callback returning void drivers/gpu/drm/arm/display/komeda/komeda_drv.c | 5 ++--- drivers/gpu/drm/arm/hdlcd_drv.c | 5 ++--- drivers/gpu/drm/arm/malidp_drv.c| 5 ++--- drivers/gpu/drm/armada/armada_crtc.c| 5 ++--- drivers/gpu/drm/armada/armada_drv.c | 5 ++--- drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 6 ++ drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c| 6 ++ drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 6 ++ drivers/gpu/drm/bridge/display-connector.c | 6 ++ drivers/gpu/drm/bridge/fsl-ldb.c| 6 ++ drivers/gpu/drm/bridge/imx/imx8qm-ldb-drv.c | 6 ++ drivers
Re: [Nouveau] [PATCH 00/53] drm: Convert to platform remove callback returning void
[A few addressed bounced and my script to find the recipents for a patch series broke and invented some addresses. I fixed all the problem I'm aware of in this mail.] On Mon, May 08, 2023 at 09:06:27AM +0200, Thomas Zimmermann wrote: > for the whole series: > > Reviewed-by: Thomas Zimmermann > > Please see my comment on the patches to tiny/. > > Let me know if you want me to merge this patchset into drm-misc-next. Thanks, I'd wait a bit for more acks/reviews to come in and then plan to resend later, also addressing the feedback you sent. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Re: [Nouveau] patches dropped from drm-misc-next [Was: Re: [PATCH 00/53] drm: Convert to platform remove callback returning] void
Hello again,
On Sun, Jun 18, 2023 at 02:39:15PM +0200, Uwe Kleine-König wrote:
> > Actually, it was probably a mistake that these patches got merged to
> > linuxnext during the 4 days that you noticed. However, your patches
> > aren't dropped and are still present in drm-misc-next.
> >
> > drm-misc has a bit of a unique model and it's documented fairly well here:
> >
> > https://drm.pages.freedesktop.org/maintainer-tools/drm-misc.html
>
> Is there a flaw then in this unique model (or its implementation) when
> drm-misc/for-linux-next moves in a non-fast-forward manner? This isn't
> expected, is it?
FTR, I checked historic next trees to find other trees where
for-linux-next were moved in a non-fast-forward manner[1]. I found:
$ git for-each-ref --format='%(refname)' refs/tags/next-* | while read
n; do hn=$(git show $n:Next/SHA1s 2>/dev/null | awk '$1 == "drm-misc" { print
$2 }'); if test -z "$hn"; then continue; fi; if test -n "$prevhn" && ! git
merge-base --is-ancestor "$prevhn" "$hn"; then if git merge-base --is-ancestor
"$hn" linus/master; then merged=x; else merged="-"; fi; echo "$n ($prevhn ->
$hn) [$merged]"; fi; prevhn=$hn; done
refs/tags/next-20151231 (e8b4855b5dd3e285d0ec18ed15468025abc1be9a ->
e112e593b215c394c0303dbf0534db0928e87967) [x]
refs/tags/next-20160212 (e24bfdd5052ca65e99fb835838da9d64b36ddc88 ->
382ab95d1af85381d8a5dff09b16a80c7e492534) [x]
refs/tags/next-20170613 (18e51064c42ca3945b94dd4652156b62457962bc ->
2d7b56378d32b0cf006f8944cbba4046df45dd25) [x]
refs/tags/next-20180406 (3ae7fb202d86b7847f237daa474f3946bdc3b0c6 ->
41613a1a7df27a0aa34bf77d51278bbe8e108a8f) [x]
refs/tags/next-20180517 (3131f209468d1514af378dd46eb34123d0af84ff ->
2045b22461c07a88dc3d2bab3cbfc6dc0c602fd4) [x]
refs/tags/next-20190320 (e552f0851070fe4975d610a99910be4e9bf5d7bd ->
29054230f3e11ea818eccfa7bb4e4b3e89544164) [x]
refs/tags/next-20190617 (9f9b25593ab4197318e3621201588ad8cd525c9b ->
b1622cb3be4557fd086831ca7426eafe5f1acc2e) [x]
refs/tags/next-20190723 (7aaddd96d5febcf5b24357a326b3038d49a20532 ->
d808097627e51d53cf9b1aa13239b5c4a6adaefb) [x]
refs/tags/next-20190829 (66c2dee4ae10a2d841c40b9dd9c7141eb23eee76 ->
578d2342ec702e5fb8a77983fabb3754ae3e9660) [x]
refs/tags/next-20191004 (d7d44b6fe40a98e960be92ea8617954c2596d140 ->
4092de1ba34eb376791809fb366bc15f8a9e0b7c) [x]
refs/tags/next-20191106 (8a537de0f3d8b655cb901c948ed863bf0b23277b ->
cea35f5ad5ffac06ea29e0d7a7f748683e1f1b7d) [x]
refs/tags/next-20191216 (0a5239985a3bc084738851afdf3fceb7d5651b0c ->
d4e6a62d3769ef09bfe116b261a61ef871dea4f9) [x]
refs/tags/next-20200123 (73896f60d4865657740c64821a7b18825a9bf96c ->
db735fc4036bbe1fbe606819b5f0ff26cc76cdff) [x]
refs/tags/next-20200415 (152cce0006abf7e17dfb7dc94896b044bda4e588 ->
74aae1c42f4a7f69934762f9e9f90a3ec335fef2) [x]
refs/tags/next-20200521 (5bebaeadb30e8d1ed694bd9b63d4e424d333fe36 ->
0df3ff451287d71c620384eb7bb2cd3a8106412c) [x]
refs/tags/next-20200617 (291ddeb621e4a9f1ced8302a777fbd7fbda058c6 ->
cfe28f909ddd6ca854568870a7a9b46454e52b6f) [x]
refs/tags/next-20200724 (9fadd6d1e2977bbd449d4fb99cde41ed6f71f668 ->
206739119508d5ab4b42ab480ff61a7e6cd72d7c) [x]
refs/tags/next-20200924 (ad44c03208e46b83e4ae3269e32c9e524aa71cf8 ->
de194561359788871f7d8f5f7797557a2a166b4e) [x]
refs/tags/next-20201027 (2580a493a97da4a302cb66251b558bfc04c16e68 ->
70bb9193728627e84e02eb0960b0aa138ae2cef5) [x]
refs/tags/next-20201214 (c365d304d69ab2a38e1431323d17a216b7930e32 ->
05faf1559de52465f1e753e31883aa294e6179c1) [x]
refs/tags/next-20210211 (5ceeb328637a01e0e54e2618cff129c6a1c31dba ->
e2183fb135a7f62d317aa1c61eb3d1919080edba) [x]
refs/tags/next-20210319 (5fd3de7a51855e086d9ce9d2d752489e9c15b850 ->
4cf1d8719aab0ad89690abb1d37ecf4552778420) [x]
refs/tags/next-20210409 (167b400217121338a2beb78e09e2c77bd95491f5 ->
9c0fed84d5750e1eea6c664e073ffa2534a17743) [x]
refs/tags/next-20210615 (a3a5f9d0fb15da90820254ba735491887cc12099 ->
1bd8a7dc28c1c410f1ceefae1f2a97c06d1a67c2) [x]
refs/tags/next-20210714 (34bd46bcf3de72cbffcdc42d3fa67e543d1c869b ->
35d283658a6196b2057be562096610c6793e1219) [x]
refs/tags/next-20210715 (35d283658a6196b2057be562096610c6793e1219 ->
85fd4a8a84316166640102676a356755ddec80e0) [-]
refs/tags/next-20210726 (85fd4a8a84316166640102676a356755ddec80e0 ->
03b7c552d081b73ba814eefc257c704b4d096d93) [x]
refs/tags/next-20210727 (03b7c552d081b73ba814eefc257c704b4d096d93 ->
87360168759879d68550b0c052bbcc2a0339ff74) [-]
refs/tags/next-20210817 (87360168759879d68550b0c052bbcc2a0339ff74 ->
80cbd880
Re: [Nouveau] patches dropped from drm-misc-next [Was: Re: [PATCH 00/53] drm: Convert to platform remove callback returning] void
Hello Maxime, On Mon, Jun 19, 2023 at 02:47:09PM +0200, Maxime Ripard wrote: > On Mon, Jun 19, 2023 at 12:53:42PM +0200, Uwe Kleine-König wrote: > > IMHO you still should ensure that only commits make it into any next > > snapshot via your tree before X-rc1 for some X (e.g. v6.5) that you > > intend to be included in X-rc1. > > That's never been a next rule either. Rust support has been in next for > almost a year without being sent as a PR for example. It seems not to be rigorously enforced, but it exists. See for example https://lore.kernel.org/all/20230510092313.16693...@canb.auug.org.au/ . @Stephen: you wrote there You will need to ensure that the patches/commits in your tree/series have been [...] destined for the current or next Linux merge window. This is a bit ambiguous because (AFAIK) during a merge window no patches should be added that are supposed to go in during the next one, right? Maybe adapt your template to read: [...] destined to be included in the next -rc1 release. which is more precise? Even if others don't adhere to it, IMHO it's still an opportunity to improve. Also there is a difference between a patch that is included in next that doesn't make it in during the next merge window and a patch that disappears from next. The latter (up to now) only happened to me when there was a problem with the patch and the maintainer who first thought the patch to be fine changed their opinion. > > > > For me, if a maintainer puts some patch into next that's a statement > > > > saying (approximately) "I think this patch is fine and I intend to > > > > send it to Linus during the next merge window.". > > > > > > I mean, that's what we're saying and doing? > > > > No, on 2023-06-09 I assumed that my patches will go into v6.5-rc1 (as it > > was part of next-20230609). A few days later however the patches were > > dropped. > > > > The two options that would have made the experience smoother for me are: > > > > a) keep c2807ecb5290 in next and send it for v6.5-rc1; or > > That's not an option. You were simply too late for v6.5-rc1, unless you > expect us to get rid of timezones and work on week-ends. But surely you > don't. We're mixing two things here. One is: "When will my patches be merged?". I have no problem being patient here and b) is fine for me. The other is "The patches first being included in next and then later not anymore is a thing that just waits to be misinterpreted". This latter is the one I care about here and that I think should be fixed for the future. > > b) keep c2807ecb5290 in a branch that doesn't result it entering next > > before v6.5-rc1. > > All the drm-misc committers use dim. If that's a concern for you, feel > free to send a patch addressing this to dim. Not sure this is sensible given that I neither use nor know dim. Also I think it should be the drm-misc maintainers who should care here given that it's them who create this unfortunate situation again and again. > > > > So my expectation is that if a patch is dropped again from next, there > > > > was a problem and it would be fair if the maintainer tells the > > > > author/submitter about this problem and that the patch was dropped. > > > > > > But it wasn't dropped, > > > > From my POV it was dropped from next as it was part of next between > > next-20230609 and next-20230615 but not any more since next-20230616. > > You talk about (not) being dropped from some branch in drm-misc, that's > > irrelevant for the thing I'm complaining about. > > You were never told that they were merged in linux-next, but in > drm-misc-next. That's nitpicking and little helpful here. In your bubble where only or mostly drm-misc matters it's ok to only look at drm-misc. But for a contributor who sends patches for dozens of subsystems next is the more useful place to look and each subsystem that is special is an obstacle. > If they did, it's mostly an unfortunate artifact. I see some progress in this discussion as you seem to agree this is unfortunate. Actually that's all I intend to achieve. > We have a documentation that explains the process and how drm-misc-next > works. If that's still confusing somehow, feel free to amend it to make > it clearer. > > > > it's still very much to be sent to Linus during the next merge window. > > > > "next merge window" as in the one leading to 6.5-rc1? Either we mean > > different things when we say "next merge window", or there is a > > misunderstanding I don't see yet. > > Linus doesn't want to receive in a PR patches that haven't been in > linux-next for at least two
Re: [Nouveau] [PATCH 00/53] drm: Convert to platform remove callback returning void
Hello, On Sun, May 07, 2023 at 06:25:23PM +0200, Uwe Kleine-König wrote: > this patch series adapts the platform drivers below drivers/gpu/drm > to use the .remove_new() callback. Compared to the traditional .remove() > callback .remove_new() returns no value. This is a good thing because > the driver core doesn't (and cannot) cope for errors during remove. The > only effect of a non-zero return value in .remove() is that the driver > core emits a warning. The device is removed anyhow and an early return > from .remove() usually yields a resource leak. > > By changing the remove callback to return void driver authors cannot > reasonably (but wrongly) assume any more that there happens some kind of > cleanup later. I wonder if someone would volunteer to add the whole series to drm-misc-next?! Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
[Nouveau] [PATCH 33/53] drm/nouveau: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/gpu/drm/nouveau/nouveau_platform.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_platform.c
b/drivers/gpu/drm/nouveau/nouveau_platform.c
index 23cd43a7fd19..bf2dc7567ea4 100644
--- a/drivers/gpu/drm/nouveau/nouveau_platform.c
+++ b/drivers/gpu/drm/nouveau/nouveau_platform.c
@@ -43,11 +43,10 @@ static int nouveau_platform_probe(struct platform_device
*pdev)
return 0;
}
-static int nouveau_platform_remove(struct platform_device *pdev)
+static void nouveau_platform_remove(struct platform_device *pdev)
{
struct drm_device *dev = platform_get_drvdata(pdev);
nouveau_drm_device_remove(dev);
- return 0;
}
#if IS_ENABLED(CONFIG_OF)
@@ -93,5 +92,5 @@ struct platform_driver nouveau_platform_driver = {
.of_match_table = of_match_ptr(nouveau_platform_match),
},
.probe = nouveau_platform_probe,
- .remove = nouveau_platform_remove,
+ .remove_new = nouveau_platform_remove,
};
--
2.39.2
Re: [Nouveau] [PATCH v3 00/16] drm: Convert to platform remove callback returning void
[Dropped a few people from To that resulted in bounces before.] On Thu, Nov 02, 2023 at 05:56:41PM +0100, Uwe Kleine-König wrote: > Hello, > > this series converts all platform drivers below drivers/gpu/drm to use > .remove_new(). It starts with a fix for a problem that potentially might > crash the kernel that I stumbled over while implementing the conversion. > > Some of the conversion patches following this fix were already send in > earlier series: > > > https://lore.kernel.org/dri-devel/20230801110239.831099-1-u.kleine-koe...@pengutronix.de > > https://lore.kernel.org/dri-devel/20230318190804.234610-1-u.kleine-koe...@pengutronix.de > > and three patches (bridge/tpd12s015, exynos + tilcdc) are new. Parts of > the above series were picked up, the patches resend here are not. Apart from a Reviewed-by: by Toni Valkeinen for patch #16 and Inki Dae who wrote to have taken patch #8 (but that didn't appear in neither next nor drm-misc-next yet). Also in v2 they didn't result in euphoric replies. Can someone who cares about drm as a whole please care for this series apply it? Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
