Re: [PATCH] drm/rockchip: dsi: Open-code drm_simple_encoder_init()

[Heiko Stuebner <heiko@sntech.de>]

Hi Diego,

Am Donnerstag, 4. Juni 2026, 14:32:25 Mitteleuropäische Sommerzeit schrieb Diogo Silva:
> Remove the dependency on drm_simple_kms_helper by open-coding the
> drm_simple_encoder_init call.

this description is missing a rationale.

Looking at the drm git history, I guess here you could add a second
paragraph, with something like:

----------- 8< -----------
The helpers have been deprecated for years as they only add an an
intermediate layer between atomic modesetting and the DRM driver.
----------- 8< -----------

Shamelessly stolen from the Todo item ;-)


> Signed-off-by: Diogo Silva <diogompaissilva@gmail.com>
> ---
>  drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 9 +++++++--

Any reason for only changing the DSI driver?

Looking at [0] a number of the Rockchip drivers use the same pattern:
- drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
- drivers/gpu/drm/rockchip/cdn-dp-core.c
- drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
- drivers/gpu/drm/rockchip/dw-mipi-dsi2-rockchip.c
- drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
- drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
- drivers/gpu/drm/rockchip/rk3066_hdmi.c
- drivers/gpu/drm/rockchip/rockchip_lvds.c
- drivers/gpu/drm/rockchip/rockchip_rgb.c

You can just do all Rockchip ones - even in one patch I think :-) .


Thanks
Heiko

[0] https://elixir.bootlin.com/linux/v7.1-rc6/A/ident/drm_simple_encoder_init


>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> index 3547d91b25d3..a09b382d208e 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> @@ -25,7 +25,6 @@
>  #include <drm/drm_mipi_dsi.h>
>  #include <drm/drm_of.h>
>  #include <drm/drm_print.h>
> -#include <drm/drm_simple_kms_helper.h>
>  
>  #include "rockchip_drm_drv.h"
>  
> @@ -825,6 +824,10 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
>  	clk_disable_unprepare(dsi->grf_clk);
>  }
>  
> +static const struct drm_encoder_funcs dw_mipi_dsi_encoder_funcs = {
> +	.destroy = drm_encoder_cleanup,
> +};
> +
>  static const struct drm_encoder_helper_funcs
>  dw_mipi_dsi_encoder_helper_funcs = {
>  	.atomic_check = dw_mipi_dsi_encoder_atomic_check,
> @@ -840,7 +843,9 @@ static int rockchip_dsi_drm_create_encoder(struct dw_mipi_dsi_rockchip *dsi,
>  	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev,
>  							     dsi->dev->of_node);
>  
> -	ret = drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_DSI);
> +	ret = drm_encoder_init(drm_dev, encoder,
> +				&dw_mipi_dsi_encoder_funcs,
> +				DRM_MODE_ENCODER_DSI, NULL);
>  	if (ret) {
>  		DRM_ERROR("Failed to initialize encoder with drm\n");
>  		return ret;
>