All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Zimmermann <tzimmermann-l3A5Bk7waGM@public.gmane.org>
To: Sam Ravnborg <sam-uyr5N9Q2VtJg9hUCZPvPmw@public.gmane.org>,
	Daniel Vetter <daniel-/w4YWyX8dFk@public.gmane.org>,
	Laurent Pinchart
	<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Cc: Alexandre Belloni
	<alexandre.belloni-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org>,
	David Airlie <airlied-cv59FeDIM0c@public.gmane.org>,
	Jose Roberto de Souza
	<jose.souza-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
	Gerd Hoffmann <kraxel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	linux-stm32-XDFAJ8BFU24N7RejjzZ/Li2xQDfSxrLKVpNB7YpNyf8@public.gmane.org,
	Emil Velikov
	<emil.velikov-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Ludovic Desroches
	<ludovic.desroches-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>,
	NXP Linux Team <linux-imx-3arQi8VN3Tc@public.gmane.org>,
	Dave Airlie <airlied-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Boris Brezillon
	<bbrezillon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Nicolas Ferre
	<nicolas.ferre-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>,
	Andrzej
Subject: Re: [PATCH v1 0/3] drm: drm_encoder_init() => drm_encoder_init_funcs()
Date: Tue, 24 Mar 2020 09:33:23 +0100	[thread overview]
Message-ID: <c93bb45d-7922-56dc-061f-6ef70493a358@suse.de> (raw)
In-Reply-To: <20200313201744.19773-1-sam-uyr5N9Q2VtJg9hUCZPvPmw@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 10320 bytes --]

Hi Sam

Am 13.03.20 um 21:17 schrieb Sam Ravnborg:
> Thomas Zimmermann had made a nice patch-set that introduced
> drm_simple_encoder_init() which is already present in drm-misc-next.
> 
> While looking at this it was suddenly obvious to me that
> this was functionalty that really should be included in drm_encoder.c
> The case where the core could handle the callback is pretty
> common and not part of the simple pipe line.

The original patchset put the new function into the core implementation
and was shot down for this. So it ended up in the simple-KMS helpers.

> 
> So after some dialog on dri-devel the conclusion was to go for
> a change like this:
> 
>     drm_encoder_init_funcs() for all users that specified a
>     drm_encoder_funcs to extend the functionality.
> 
>     drm_encoder_init() for all users that did not
>     need to extend the basic functionality with
>     drm_encoder_funcs.

TBH, my take-away was to keep the core as it is ans maybe rename
drm_simple_encoder_init() to some better name.

> 
> A similar approach with a _funcs() prefix is used elsewhere in drm/

IMHO, there are a few things to consider:

From grepping, I could only find drm_gem_fb_create_with_funcs(). And the
proposed change would make the encoder's function name inconsistent with
drm_connector_init(), drm_crtc_init(), and others. Finally,
drm_connector_init_with_ddc() was criticiced for being mid-layerish and
could lead to many combinations of postfixes (e.g., _with_funcs(),
with_ddc(), _with_ddc_and_funcs(), etc).

If there is consent that the drm_simple_encoder_init() should go away,
I'd propose to change drm_encoder_init(). It could use a default
implementation for funcs, if no funcs argument has been specified. We
already have such behavior for some GEM callbacks. In later patches,
drm_gem_fb_create_with_funcs() and drm_connector_init_with_ddc() could
go away as well.

Best regards
Thomas


> 
> This required a rename of the existing users, and
> a follow-up patch that moves drm_simple_encoder_init()
> to drm_encoder.c
> 
> Patches 3 in this set demonstrate the use of drm_encoder_init().
> There are many more drivers that can be converted as Thomas
> has already demonstrated.
> 
> This is all based on work done by Thomas Zimmermann,
> I just wanted to implement my suggestion so
> we could select the best way forward.
> 
> Note: Daniel Vetter has hinted the approach implemented
> here smelled like middle-layer.
> IMO this is not so, it is just a way to handle cleanup
> for the simple cases.
> 
> 	Sam
> 
> 
> Sam Ravnborg (3):
>       drm: drm_encoder_init() => drm_encoder_init_funcs()
>       drm: drm_simple_encoder_init() => drm_encoder_init()
>       drm/atmel-hlcdc: Use drm_encoder_init()
> 
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c             | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c             | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c              | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c              | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.c           |  4 +-
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 10 ++---
>  .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c    | 10 ++---
>  drivers/gpu/drm/arc/arcpgu_hdmi.c                  |  4 +-
>  drivers/gpu/drm/arc/arcpgu_sim.c                   |  4 +-
>  drivers/gpu/drm/ast/ast_mode.c                     |  3 +-
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  8 +---
>  drivers/gpu/drm/drm_encoder.c                      | 49 +++++++++++++++++++---
>  drivers/gpu/drm/drm_encoder_slave.c                |  2 +-
>  drivers/gpu/drm/drm_simple_kms_helper.c            | 45 +-------------------
>  drivers/gpu/drm/drm_writeback.c                    |  6 +--
>  drivers/gpu/drm/exynos/exynos_dp.c                 |  4 +-
>  drivers/gpu/drm/exynos/exynos_drm_dpi.c            |  4 +-
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c            |  4 +-
>  drivers/gpu/drm/exynos/exynos_drm_vidi.c           |  4 +-
>  drivers/gpu/drm/exynos/exynos_hdmi.c               |  4 +-
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c          |  4 +-
>  drivers/gpu/drm/gma500/cdv_intel_crt.c             |  5 ++-
>  drivers/gpu/drm/gma500/cdv_intel_dp.c              |  4 +-
>  drivers/gpu/drm/gma500/cdv_intel_hdmi.c            |  4 +-
>  drivers/gpu/drm/gma500/cdv_intel_lvds.c            |  6 +--
>  drivers/gpu/drm/gma500/mdfld_dsi_dpi.c             |  7 ++--
>  drivers/gpu/drm/gma500/oaktrail_hdmi.c             |  6 +--
>  drivers/gpu/drm/gma500/oaktrail_lvds.c             |  4 +-
>  drivers/gpu/drm/gma500/psb_intel_lvds.c            |  6 +--
>  drivers/gpu/drm/gma500/psb_intel_sdvo.c            |  4 +-
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c   |  4 +-
>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c       |  4 +-
>  drivers/gpu/drm/i2c/tda998x_drv.c                  |  5 ++-
>  drivers/gpu/drm/i915/display/icl_dsi.c             |  4 +-
>  drivers/gpu/drm/i915/display/intel_crt.c           |  5 ++-
>  drivers/gpu/drm/i915/display/intel_ddi.c           |  6 ++-
>  drivers/gpu/drm/i915/display/intel_dp.c            |  6 +--
>  drivers/gpu/drm/i915/display/intel_dp_mst.c        |  6 ++-
>  drivers/gpu/drm/i915/display/intel_dvo.c           |  6 +--
>  drivers/gpu/drm/i915/display/intel_hdmi.c          |  6 +--
>  drivers/gpu/drm/i915/display/intel_lvds.c          |  4 +-
>  drivers/gpu/drm/i915/display/intel_sdvo.c          |  6 +--
>  drivers/gpu/drm/i915/display/intel_tv.c            |  4 +-
>  drivers/gpu/drm/i915/display/vlv_dsi.c             |  5 ++-
>  drivers/gpu/drm/imx/dw_hdmi-imx.c                  |  4 +-
>  drivers/gpu/drm/imx/imx-ldb.c                      |  4 +-
>  drivers/gpu/drm/imx/imx-tve.c                      |  4 +-
>  drivers/gpu/drm/imx/parallel-display.c             |  4 +-
>  drivers/gpu/drm/ingenic/ingenic-drm.c              |  5 ++-
>  drivers/gpu/drm/mediatek/mtk_dpi.c                 |  5 ++-
>  drivers/gpu/drm/mediatek/mtk_dsi.c                 |  4 +-
>  drivers/gpu/drm/meson/meson_dw_hdmi.c              |  5 ++-
>  drivers/gpu/drm/meson/meson_venc_cvbs.c            |  5 ++-
>  drivers/gpu/drm/mgag200/mgag200_mode.c             |  7 +---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c        |  4 +-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c   |  4 +-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c   |  4 +-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c  |  4 +-
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c       |  3 +-
>  drivers/gpu/drm/nouveau/dispnv04/dac.c             |  4 +-
>  drivers/gpu/drm/nouveau/dispnv04/dfp.c             |  3 +-
>  drivers/gpu/drm/nouveau/dispnv04/tvnv04.c          |  4 +-
>  drivers/gpu/drm/nouveau/dispnv04/tvnv17.c          |  4 +-
>  drivers/gpu/drm/nouveau/dispnv50/disp.c            | 16 +++----
>  drivers/gpu/drm/omapdrm/omap_encoder.c             |  4 +-
>  drivers/gpu/drm/qxl/qxl_display.c                  |  7 +---
>  drivers/gpu/drm/radeon/atombios_encoders.c         | 40 +++++++++---------
>  drivers/gpu/drm/radeon/radeon_dp_mst.c             |  4 +-
>  drivers/gpu/drm/radeon/radeon_legacy_encoders.c    | 20 ++++-----
>  drivers/gpu/drm/rcar-du/rcar_du_encoder.c          |  4 +-
>  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c    |  5 ++-
>  drivers/gpu/drm/rockchip/cdn-dp-core.c             |  4 +-
>  drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c    |  5 ++-
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        |  4 +-
>  drivers/gpu/drm/rockchip/inno_hdmi.c               |  4 +-
>  drivers/gpu/drm/rockchip/rk3066_hdmi.c             |  4 +-
>  drivers/gpu/drm/rockchip/rockchip_lvds.c           |  5 ++-
>  drivers/gpu/drm/rockchip/rockchip_rgb.c            |  5 ++-
>  drivers/gpu/drm/shmobile/shmob_drm_crtc.c          |  4 +-
>  drivers/gpu/drm/sti/sti_tvout.c                    | 16 +++----
>  drivers/gpu/drm/stm/ltdc.c                         |  4 +-
>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c             | 10 ++---
>  drivers/gpu/drm/sun4i/sun4i_lvds.c                 | 10 ++---
>  drivers/gpu/drm/sun4i/sun4i_rgb.c                  | 10 ++---
>  drivers/gpu/drm/sun4i/sun4i_tv.c                   | 10 ++---
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c             | 10 ++---
>  drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c              |  4 +-
>  drivers/gpu/drm/tegra/dsi.c                        |  6 +--
>  drivers/gpu/drm/tegra/hdmi.c                       |  5 ++-
>  drivers/gpu/drm/tegra/rgb.c                        |  4 +-
>  drivers/gpu/drm/tegra/sor.c                        |  4 +-
>  drivers/gpu/drm/tidss/tidss_encoder.c              |  4 +-
>  drivers/gpu/drm/tilcdc/tilcdc_external.c           |  8 ++--
>  drivers/gpu/drm/tilcdc/tilcdc_panel.c              |  4 +-
>  drivers/gpu/drm/vboxvideo/vbox_mode.c              |  4 +-
>  drivers/gpu/drm/vc4/vc4_dpi.c                      |  4 +-
>  drivers/gpu/drm/vc4/vc4_dsi.c                      |  4 +-
>  drivers/gpu/drm/vc4/vc4_hdmi.c                     |  4 +-
>  drivers/gpu/drm/vc4/vc4_vec.c                      |  4 +-
>  drivers/gpu/drm/virtio/virtgpu_display.c           |  4 +-
>  drivers/gpu/drm/vkms/vkms_output.c                 |  4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c                |  4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c               |  5 ++-
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c               |  4 +-
>  drivers/gpu/drm/zte/zx_hdmi.c                      |  4 +-
>  drivers/gpu/drm/zte/zx_tvenc.c                     |  4 +-
>  drivers/gpu/drm/zte/zx_vga.c                       |  4 +-
>  include/drm/drm_encoder.h                          |  9 ++--
>  include/drm/drm_simple_kms_helper.h                |  4 --
>  109 files changed, 394 insertions(+), 395 deletions(-)
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

WARNING: multiple messages have this Message-ID
From: Thomas Zimmermann <tzimmermann@suse.de>
To: Sam Ravnborg <sam@ravnborg.org>, Daniel Vetter <daniel@ffwll.ch>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	dri-devel@lists.freedesktop.org
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>,
	David Airlie <airlied@linux.ie>,
	Jose Roberto de Souza <jose.souza@intel.com>,
	virtualization@lists.linux-foundation.org,
	Gerd Hoffmann <kraxel@redhat.com>,
	linux-stm32@st-md-mailman.stormreply.com,
	Emil Velikov <emil.velikov@collabora.com>,
	linux-samsung-soc@vger.kernel.org, amd-gfx@lists.freedesktop.org,
	linux-rockchip@lists.infradead.org,
	Ludovic Desroches <ludovic.desroches@microchip.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	Dave Airlie <airlied@redhat.com>,
	linux-mediatek@lists.infradead.org, linux-tegra@vger.kernel.org,
	linux-amlogic@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	Boris Brezillon <bbrezillon@kernel.org>,
	Nicolas Ferre <nicolas.ferre@microchip.com>,
	Andrzej Pietrasiewicz <andrzej.p@collabora.com>,
	linux-renesas-soc@vger.kernel.org
Subject: Re: [PATCH v1 0/3] drm: drm_encoder_init() => drm_encoder_init_funcs()
Date: Tue, 24 Mar 2020 09:33:23 +0100	[thread overview]
Message-ID: <c93bb45d-7922-56dc-061f-6ef70493a358@suse.de> (raw)
In-Reply-To: <20200313201744.19773-1-sam@ravnborg.org>


[-- Attachment #1.1: Type: text/plain, Size: 10292 bytes --]

Hi Sam

Am 13.03.20 um 21:17 schrieb Sam Ravnborg:
> Thomas Zimmermann had made a nice patch-set that introduced
> drm_simple_encoder_init() which is already present in drm-misc-next.
> 
> While looking at this it was suddenly obvious to me that
> this was functionalty that really should be included in drm_encoder.c
> The case where the core could handle the callback is pretty
> common and not part of the simple pipe line.

The original patchset put the new function into the core implementation
and was shot down for this. So it ended up in the simple-KMS helpers.

> 
> So after some dialog on dri-devel the conclusion was to go for
> a change like this:
> 
>     drm_encoder_init_funcs() for all users that specified a
>     drm_encoder_funcs to extend the functionality.
> 
>     drm_encoder_init() for all users that did not
>     need to extend the basic functionality with
>     drm_encoder_funcs.

TBH, my take-away was to keep the core as it is ans maybe rename
drm_simple_encoder_init() to some better name.

> 
> A similar approach with a _funcs() prefix is used elsewhere in drm/

IMHO, there are a few things to consider:

From grepping, I could only find drm_gem_fb_create_with_funcs(). And the
proposed change would make the encoder's function name inconsistent with
drm_connector_init(), drm_crtc_init(), and others. Finally,
drm_connector_init_with_ddc() was criticiced for being mid-layerish and
could lead to many combinations of postfixes (e.g., _with_funcs(),
with_ddc(), _with_ddc_and_funcs(), etc).

If there is consent that the drm_simple_encoder_init() should go away,
I'd propose to change drm_encoder_init(). It could use a default
implementation for funcs, if no funcs argument has been specified. We
already have such behavior for some GEM callbacks. In later patches,
drm_gem_fb_create_with_funcs() and drm_connector_init_with_ddc() could
go away as well.

Best regards
Thomas


> 
> This required a rename of the existing users, and
> a follow-up patch that moves drm_simple_encoder_init()
> to drm_encoder.c
> 
> Patches 3 in this set demonstrate the use of drm_encoder_init().
> There are many more drivers that can be converted as Thomas
> has already demonstrated.
> 
> This is all based on work done by Thomas Zimmermann,
> I just wanted to implement my suggestion so
> we could select the best way forward.
> 
> Note: Daniel Vetter has hinted the approach implemented
> here smelled like middle-layer.
> IMO this is not so, it is just a way to handle cleanup
> for the simple cases.
> 
> 	Sam
> 
> 
> Sam Ravnborg (3):
>       drm: drm_encoder_init() => drm_encoder_init_funcs()
>       drm: drm_simple_encoder_init() => drm_encoder_init()
>       drm/atmel-hlcdc: Use drm_encoder_init()
> 
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c             | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c             | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c              | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c              | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.c           |  4 +-
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 10 ++---
>  .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c    | 10 ++---
>  drivers/gpu/drm/arc/arcpgu_hdmi.c                  |  4 +-
>  drivers/gpu/drm/arc/arcpgu_sim.c                   |  4 +-
>  drivers/gpu/drm/ast/ast_mode.c                     |  3 +-
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  8 +---
>  drivers/gpu/drm/drm_encoder.c                      | 49 +++++++++++++++++++---
>  drivers/gpu/drm/drm_encoder_slave.c                |  2 +-
>  drivers/gpu/drm/drm_simple_kms_helper.c            | 45 +-------------------
>  drivers/gpu/drm/drm_writeback.c                    |  6 +--
>  drivers/gpu/drm/exynos/exynos_dp.c                 |  4 +-
>  drivers/gpu/drm/exynos/exynos_drm_dpi.c            |  4 +-
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c            |  4 +-
>  drivers/gpu/drm/exynos/exynos_drm_vidi.c           |  4 +-
>  drivers/gpu/drm/exynos/exynos_hdmi.c               |  4 +-
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c          |  4 +-
>  drivers/gpu/drm/gma500/cdv_intel_crt.c             |  5 ++-
>  drivers/gpu/drm/gma500/cdv_intel_dp.c              |  4 +-
>  drivers/gpu/drm/gma500/cdv_intel_hdmi.c            |  4 +-
>  drivers/gpu/drm/gma500/cdv_intel_lvds.c            |  6 +--
>  drivers/gpu/drm/gma500/mdfld_dsi_dpi.c             |  7 ++--
>  drivers/gpu/drm/gma500/oaktrail_hdmi.c             |  6 +--
>  drivers/gpu/drm/gma500/oaktrail_lvds.c             |  4 +-
>  drivers/gpu/drm/gma500/psb_intel_lvds.c            |  6 +--
>  drivers/gpu/drm/gma500/psb_intel_sdvo.c            |  4 +-
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c   |  4 +-
>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c       |  4 +-
>  drivers/gpu/drm/i2c/tda998x_drv.c                  |  5 ++-
>  drivers/gpu/drm/i915/display/icl_dsi.c             |  4 +-
>  drivers/gpu/drm/i915/display/intel_crt.c           |  5 ++-
>  drivers/gpu/drm/i915/display/intel_ddi.c           |  6 ++-
>  drivers/gpu/drm/i915/display/intel_dp.c            |  6 +--
>  drivers/gpu/drm/i915/display/intel_dp_mst.c        |  6 ++-
>  drivers/gpu/drm/i915/display/intel_dvo.c           |  6 +--
>  drivers/gpu/drm/i915/display/intel_hdmi.c          |  6 +--
>  drivers/gpu/drm/i915/display/intel_lvds.c          |  4 +-
>  drivers/gpu/drm/i915/display/intel_sdvo.c          |  6 +--
>  drivers/gpu/drm/i915/display/intel_tv.c            |  4 +-
>  drivers/gpu/drm/i915/display/vlv_dsi.c             |  5 ++-
>  drivers/gpu/drm/imx/dw_hdmi-imx.c                  |  4 +-
>  drivers/gpu/drm/imx/imx-ldb.c                      |  4 +-
>  drivers/gpu/drm/imx/imx-tve.c                      |  4 +-
>  drivers/gpu/drm/imx/parallel-display.c             |  4 +-
>  drivers/gpu/drm/ingenic/ingenic-drm.c              |  5 ++-
>  drivers/gpu/drm/mediatek/mtk_dpi.c                 |  5 ++-
>  drivers/gpu/drm/mediatek/mtk_dsi.c                 |  4 +-
>  drivers/gpu/drm/meson/meson_dw_hdmi.c              |  5 ++-
>  drivers/gpu/drm/meson/meson_venc_cvbs.c            |  5 ++-
>  drivers/gpu/drm/mgag200/mgag200_mode.c             |  7 +---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c        |  4 +-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c   |  4 +-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c   |  4 +-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c  |  4 +-
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c       |  3 +-
>  drivers/gpu/drm/nouveau/dispnv04/dac.c             |  4 +-
>  drivers/gpu/drm/nouveau/dispnv04/dfp.c             |  3 +-
>  drivers/gpu/drm/nouveau/dispnv04/tvnv04.c          |  4 +-
>  drivers/gpu/drm/nouveau/dispnv04/tvnv17.c          |  4 +-
>  drivers/gpu/drm/nouveau/dispnv50/disp.c            | 16 +++----
>  drivers/gpu/drm/omapdrm/omap_encoder.c             |  4 +-
>  drivers/gpu/drm/qxl/qxl_display.c                  |  7 +---
>  drivers/gpu/drm/radeon/atombios_encoders.c         | 40 +++++++++---------
>  drivers/gpu/drm/radeon/radeon_dp_mst.c             |  4 +-
>  drivers/gpu/drm/radeon/radeon_legacy_encoders.c    | 20 ++++-----
>  drivers/gpu/drm/rcar-du/rcar_du_encoder.c          |  4 +-
>  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c    |  5 ++-
>  drivers/gpu/drm/rockchip/cdn-dp-core.c             |  4 +-
>  drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c    |  5 ++-
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        |  4 +-
>  drivers/gpu/drm/rockchip/inno_hdmi.c               |  4 +-
>  drivers/gpu/drm/rockchip/rk3066_hdmi.c             |  4 +-
>  drivers/gpu/drm/rockchip/rockchip_lvds.c           |  5 ++-
>  drivers/gpu/drm/rockchip/rockchip_rgb.c            |  5 ++-
>  drivers/gpu/drm/shmobile/shmob_drm_crtc.c          |  4 +-
>  drivers/gpu/drm/sti/sti_tvout.c                    | 16 +++----
>  drivers/gpu/drm/stm/ltdc.c                         |  4 +-
>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c             | 10 ++---
>  drivers/gpu/drm/sun4i/sun4i_lvds.c                 | 10 ++---
>  drivers/gpu/drm/sun4i/sun4i_rgb.c                  | 10 ++---
>  drivers/gpu/drm/sun4i/sun4i_tv.c                   | 10 ++---
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c             | 10 ++---
>  drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c              |  4 +-
>  drivers/gpu/drm/tegra/dsi.c                        |  6 +--
>  drivers/gpu/drm/tegra/hdmi.c                       |  5 ++-
>  drivers/gpu/drm/tegra/rgb.c                        |  4 +-
>  drivers/gpu/drm/tegra/sor.c                        |  4 +-
>  drivers/gpu/drm/tidss/tidss_encoder.c              |  4 +-
>  drivers/gpu/drm/tilcdc/tilcdc_external.c           |  8 ++--
>  drivers/gpu/drm/tilcdc/tilcdc_panel.c              |  4 +-
>  drivers/gpu/drm/vboxvideo/vbox_mode.c              |  4 +-
>  drivers/gpu/drm/vc4/vc4_dpi.c                      |  4 +-
>  drivers/gpu/drm/vc4/vc4_dsi.c                      |  4 +-
>  drivers/gpu/drm/vc4/vc4_hdmi.c                     |  4 +-
>  drivers/gpu/drm/vc4/vc4_vec.c                      |  4 +-
>  drivers/gpu/drm/virtio/virtgpu_display.c           |  4 +-
>  drivers/gpu/drm/vkms/vkms_output.c                 |  4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c                |  4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c               |  5 ++-
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c               |  4 +-
>  drivers/gpu/drm/zte/zx_hdmi.c                      |  4 +-
>  drivers/gpu/drm/zte/zx_tvenc.c                     |  4 +-
>  drivers/gpu/drm/zte/zx_vga.c                       |  4 +-
>  include/drm/drm_encoder.h                          |  9 ++--
>  include/drm/drm_simple_kms_helper.h                |  4 --
>  109 files changed, 394 insertions(+), 395 deletions(-)
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

WARNING: multiple messages have this Message-ID
From: Thomas Zimmermann <tzimmermann@suse.de>
To: Sam Ravnborg <sam@ravnborg.org>, Daniel Vetter <daniel@ffwll.ch>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	dri-devel@lists.freedesktop.org
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>,
	linux-samsung-soc@vger.kernel.org,
	Boris Brezillon <bbrezillon@kernel.org>,
	David Airlie <airlied@linux.ie>,
	Andrzej Pietrasiewicz <andrzej.p@collabora.com>,
	Nicolas Ferre <nicolas.ferre@microchip.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	Jose Roberto de Souza <jose.souza@intel.com>,
	virtualization@lists.linux-foundation.org,
	linux-renesas-soc@vger.kernel.org,
	linux-rockchip@lists.infradead.org,
	Ludovic Desroches <ludovic.desroches@microchip.com>,
	linux-mediatek@lists.infradead.org,
	Gerd Hoffmann <kraxel@redhat.com>,
	linux-amlogic@lists.infradead.org, linux-tegra@vger.kernel.org,
	Dave Airlie <airlied@redhat.com>,
	amd-gfx@lists.freedesktop.org,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org,
	Emil Velikov <emil.velikov@collabora.com>
Subject: Re: [PATCH v1 0/3] drm: drm_encoder_init() => drm_encoder_init_funcs()
Date: Tue, 24 Mar 2020 09:33:23 +0100	[thread overview]
Message-ID: <c93bb45d-7922-56dc-061f-6ef70493a358@suse.de> (raw)
In-Reply-To: <20200313201744.19773-1-sam@ravnborg.org>


[-- Attachment #1.1.1: Type: text/plain, Size: 10292 bytes --]

Hi Sam

Am 13.03.20 um 21:17 schrieb Sam Ravnborg:
> Thomas Zimmermann had made a nice patch-set that introduced
> drm_simple_encoder_init() which is already present in drm-misc-next.
> 
> While looking at this it was suddenly obvious to me that
> this was functionalty that really should be included in drm_encoder.c
> The case where the core could handle the callback is pretty
> common and not part of the simple pipe line.

The original patchset put the new function into the core implementation
and was shot down for this. So it ended up in the simple-KMS helpers.

> 
> So after some dialog on dri-devel the conclusion was to go for
> a change like this:
> 
>     drm_encoder_init_funcs() for all users that specified a
>     drm_encoder_funcs to extend the functionality.
> 
>     drm_encoder_init() for all users that did not
>     need to extend the basic functionality with
>     drm_encoder_funcs.

TBH, my take-away was to keep the core as it is ans maybe rename
drm_simple_encoder_init() to some better name.

> 
> A similar approach with a _funcs() prefix is used elsewhere in drm/

IMHO, there are a few things to consider:

From grepping, I could only find drm_gem_fb_create_with_funcs(). And the
proposed change would make the encoder's function name inconsistent with
drm_connector_init(), drm_crtc_init(), and others. Finally,
drm_connector_init_with_ddc() was criticiced for being mid-layerish and
could lead to many combinations of postfixes (e.g., _with_funcs(),
with_ddc(), _with_ddc_and_funcs(), etc).

If there is consent that the drm_simple_encoder_init() should go away,
I'd propose to change drm_encoder_init(). It could use a default
implementation for funcs, if no funcs argument has been specified. We
already have such behavior for some GEM callbacks. In later patches,
drm_gem_fb_create_with_funcs() and drm_connector_init_with_ddc() could
go away as well.

Best regards
Thomas


> 
> This required a rename of the existing users, and
> a follow-up patch that moves drm_simple_encoder_init()
> to drm_encoder.c
> 
> Patches 3 in this set demonstrate the use of drm_encoder_init().
> There are many more drivers that can be converted as Thomas
> has already demonstrated.
> 
> This is all based on work done by Thomas Zimmermann,
> I just wanted to implement my suggestion so
> we could select the best way forward.
> 
> Note: Daniel Vetter has hinted the approach implemented
> here smelled like middle-layer.
> IMO this is not so, it is just a way to handle cleanup
> for the simple cases.
> 
> 	Sam
> 
> 
> Sam Ravnborg (3):
>       drm: drm_encoder_init() => drm_encoder_init_funcs()
>       drm: drm_simple_encoder_init() => drm_encoder_init()
>       drm/atmel-hlcdc: Use drm_encoder_init()
> 
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c             | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c             | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c              | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c              | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.c           |  4 +-
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 10 ++---
>  .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c    | 10 ++---
>  drivers/gpu/drm/arc/arcpgu_hdmi.c                  |  4 +-
>  drivers/gpu/drm/arc/arcpgu_sim.c                   |  4 +-
>  drivers/gpu/drm/ast/ast_mode.c                     |  3 +-
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  8 +---
>  drivers/gpu/drm/drm_encoder.c                      | 49 +++++++++++++++++++---
>  drivers/gpu/drm/drm_encoder_slave.c                |  2 +-
>  drivers/gpu/drm/drm_simple_kms_helper.c            | 45 +-------------------
>  drivers/gpu/drm/drm_writeback.c                    |  6 +--
>  drivers/gpu/drm/exynos/exynos_dp.c                 |  4 +-
>  drivers/gpu/drm/exynos/exynos_drm_dpi.c            |  4 +-
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c            |  4 +-
>  drivers/gpu/drm/exynos/exynos_drm_vidi.c           |  4 +-
>  drivers/gpu/drm/exynos/exynos_hdmi.c               |  4 +-
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c          |  4 +-
>  drivers/gpu/drm/gma500/cdv_intel_crt.c             |  5 ++-
>  drivers/gpu/drm/gma500/cdv_intel_dp.c              |  4 +-
>  drivers/gpu/drm/gma500/cdv_intel_hdmi.c            |  4 +-
>  drivers/gpu/drm/gma500/cdv_intel_lvds.c            |  6 +--
>  drivers/gpu/drm/gma500/mdfld_dsi_dpi.c             |  7 ++--
>  drivers/gpu/drm/gma500/oaktrail_hdmi.c             |  6 +--
>  drivers/gpu/drm/gma500/oaktrail_lvds.c             |  4 +-
>  drivers/gpu/drm/gma500/psb_intel_lvds.c            |  6 +--
>  drivers/gpu/drm/gma500/psb_intel_sdvo.c            |  4 +-
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c   |  4 +-
>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c       |  4 +-
>  drivers/gpu/drm/i2c/tda998x_drv.c                  |  5 ++-
>  drivers/gpu/drm/i915/display/icl_dsi.c             |  4 +-
>  drivers/gpu/drm/i915/display/intel_crt.c           |  5 ++-
>  drivers/gpu/drm/i915/display/intel_ddi.c           |  6 ++-
>  drivers/gpu/drm/i915/display/intel_dp.c            |  6 +--
>  drivers/gpu/drm/i915/display/intel_dp_mst.c        |  6 ++-
>  drivers/gpu/drm/i915/display/intel_dvo.c           |  6 +--
>  drivers/gpu/drm/i915/display/intel_hdmi.c          |  6 +--
>  drivers/gpu/drm/i915/display/intel_lvds.c          |  4 +-
>  drivers/gpu/drm/i915/display/intel_sdvo.c          |  6 +--
>  drivers/gpu/drm/i915/display/intel_tv.c            |  4 +-
>  drivers/gpu/drm/i915/display/vlv_dsi.c             |  5 ++-
>  drivers/gpu/drm/imx/dw_hdmi-imx.c                  |  4 +-
>  drivers/gpu/drm/imx/imx-ldb.c                      |  4 +-
>  drivers/gpu/drm/imx/imx-tve.c                      |  4 +-
>  drivers/gpu/drm/imx/parallel-display.c             |  4 +-
>  drivers/gpu/drm/ingenic/ingenic-drm.c              |  5 ++-
>  drivers/gpu/drm/mediatek/mtk_dpi.c                 |  5 ++-
>  drivers/gpu/drm/mediatek/mtk_dsi.c                 |  4 +-
>  drivers/gpu/drm/meson/meson_dw_hdmi.c              |  5 ++-
>  drivers/gpu/drm/meson/meson_venc_cvbs.c            |  5 ++-
>  drivers/gpu/drm/mgag200/mgag200_mode.c             |  7 +---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c        |  4 +-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c   |  4 +-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c   |  4 +-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c  |  4 +-
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c       |  3 +-
>  drivers/gpu/drm/nouveau/dispnv04/dac.c             |  4 +-
>  drivers/gpu/drm/nouveau/dispnv04/dfp.c             |  3 +-
>  drivers/gpu/drm/nouveau/dispnv04/tvnv04.c          |  4 +-
>  drivers/gpu/drm/nouveau/dispnv04/tvnv17.c          |  4 +-
>  drivers/gpu/drm/nouveau/dispnv50/disp.c            | 16 +++----
>  drivers/gpu/drm/omapdrm/omap_encoder.c             |  4 +-
>  drivers/gpu/drm/qxl/qxl_display.c                  |  7 +---
>  drivers/gpu/drm/radeon/atombios_encoders.c         | 40 +++++++++---------
>  drivers/gpu/drm/radeon/radeon_dp_mst.c             |  4 +-
>  drivers/gpu/drm/radeon/radeon_legacy_encoders.c    | 20 ++++-----
>  drivers/gpu/drm/rcar-du/rcar_du_encoder.c          |  4 +-
>  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c    |  5 ++-
>  drivers/gpu/drm/rockchip/cdn-dp-core.c             |  4 +-
>  drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c    |  5 ++-
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        |  4 +-
>  drivers/gpu/drm/rockchip/inno_hdmi.c               |  4 +-
>  drivers/gpu/drm/rockchip/rk3066_hdmi.c             |  4 +-
>  drivers/gpu/drm/rockchip/rockchip_lvds.c           |  5 ++-
>  drivers/gpu/drm/rockchip/rockchip_rgb.c            |  5 ++-
>  drivers/gpu/drm/shmobile/shmob_drm_crtc.c          |  4 +-
>  drivers/gpu/drm/sti/sti_tvout.c                    | 16 +++----
>  drivers/gpu/drm/stm/ltdc.c                         |  4 +-
>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c             | 10 ++---
>  drivers/gpu/drm/sun4i/sun4i_lvds.c                 | 10 ++---
>  drivers/gpu/drm/sun4i/sun4i_rgb.c                  | 10 ++---
>  drivers/gpu/drm/sun4i/sun4i_tv.c                   | 10 ++---
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c             | 10 ++---
>  drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c              |  4 +-
>  drivers/gpu/drm/tegra/dsi.c                        |  6 +--
>  drivers/gpu/drm/tegra/hdmi.c                       |  5 ++-
>  drivers/gpu/drm/tegra/rgb.c                        |  4 +-
>  drivers/gpu/drm/tegra/sor.c                        |  4 +-
>  drivers/gpu/drm/tidss/tidss_encoder.c              |  4 +-
>  drivers/gpu/drm/tilcdc/tilcdc_external.c           |  8 ++--
>  drivers/gpu/drm/tilcdc/tilcdc_panel.c              |  4 +-
>  drivers/gpu/drm/vboxvideo/vbox_mode.c              |  4 +-
>  drivers/gpu/drm/vc4/vc4_dpi.c                      |  4 +-
>  drivers/gpu/drm/vc4/vc4_dsi.c                      |  4 +-
>  drivers/gpu/drm/vc4/vc4_hdmi.c                     |  4 +-
>  drivers/gpu/drm/vc4/vc4_vec.c                      |  4 +-
>  drivers/gpu/drm/virtio/virtgpu_display.c           |  4 +-
>  drivers/gpu/drm/vkms/vkms_output.c                 |  4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c                |  4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c               |  5 ++-
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c               |  4 +-
>  drivers/gpu/drm/zte/zx_hdmi.c                      |  4 +-
>  drivers/gpu/drm/zte/zx_tvenc.c                     |  4 +-
>  drivers/gpu/drm/zte/zx_vga.c                       |  4 +-
>  include/drm/drm_encoder.h                          |  9 ++--
>  include/drm/drm_simple_kms_helper.h                |  4 --
>  109 files changed, 394 insertions(+), 395 deletions(-)
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

WARNING: multiple messages have this Message-ID
From: Thomas Zimmermann <tzimmermann@suse.de>
To: Sam Ravnborg <sam@ravnborg.org>, Daniel Vetter <daniel@ffwll.ch>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	dri-devel@lists.freedesktop.org
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>,
	linux-samsung-soc@vger.kernel.org,
	Boris Brezillon <bbrezillon@kernel.org>,
	David Airlie <airlied@linux.ie>,
	Andrzej Pietrasiewicz <andrzej.p@collabora.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	Jose Roberto de Souza <jose.souza@intel.com>,
	virtualization@lists.linux-foundation.org,
	linux-renesas-soc@vger.kernel.org,
	linux-rockchip@lists.infradead.org,
	Ludovic Desroches <ludovic.desroches@microchip.com>,
	linux-mediatek@lists.infradead.org,
	Gerd Hoffmann <kraxel@redhat.com>,
	linux-amlogic@lists.infradead.org, linux-tegra@vger.kernel.org,
	Dave Airlie <airlied@redhat.com>,
	amd-gfx@lists.freedesktop.org,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org,
	Emil Velikov <emil.velikov@collabora.com>
Subject: Re: [PATCH v1 0/3] drm: drm_encoder_init() => drm_encoder_init_funcs()
Date: Tue, 24 Mar 2020 09:33:23 +0100	[thread overview]
Message-ID: <c93bb45d-7922-56dc-061f-6ef70493a358@suse.de> (raw)
In-Reply-To: <20200313201744.19773-1-sam@ravnborg.org>


[-- Attachment #1.1.1: Type: text/plain, Size: 10292 bytes --]

Hi Sam

Am 13.03.20 um 21:17 schrieb Sam Ravnborg:
> Thomas Zimmermann had made a nice patch-set that introduced
> drm_simple_encoder_init() which is already present in drm-misc-next.
> 
> While looking at this it was suddenly obvious to me that
> this was functionalty that really should be included in drm_encoder.c
> The case where the core could handle the callback is pretty
> common and not part of the simple pipe line.

The original patchset put the new function into the core implementation
and was shot down for this. So it ended up in the simple-KMS helpers.

> 
> So after some dialog on dri-devel the conclusion was to go for
> a change like this:
> 
>     drm_encoder_init_funcs() for all users that specified a
>     drm_encoder_funcs to extend the functionality.
> 
>     drm_encoder_init() for all users that did not
>     need to extend the basic functionality with
>     drm_encoder_funcs.

TBH, my take-away was to keep the core as it is ans maybe rename
drm_simple_encoder_init() to some better name.

> 
> A similar approach with a _funcs() prefix is used elsewhere in drm/

IMHO, there are a few things to consider:

From grepping, I could only find drm_gem_fb_create_with_funcs(). And the
proposed change would make the encoder's function name inconsistent with
drm_connector_init(), drm_crtc_init(), and others. Finally,
drm_connector_init_with_ddc() was criticiced for being mid-layerish and
could lead to many combinations of postfixes (e.g., _with_funcs(),
with_ddc(), _with_ddc_and_funcs(), etc).

If there is consent that the drm_simple_encoder_init() should go away,
I'd propose to change drm_encoder_init(). It could use a default
implementation for funcs, if no funcs argument has been specified. We
already have such behavior for some GEM callbacks. In later patches,
drm_gem_fb_create_with_funcs() and drm_connector_init_with_ddc() could
go away as well.

Best regards
Thomas


> 
> This required a rename of the existing users, and
> a follow-up patch that moves drm_simple_encoder_init()
> to drm_encoder.c
> 
> Patches 3 in this set demonstrate the use of drm_encoder_init().
> There are many more drivers that can be converted as Thomas
> has already demonstrated.
> 
> This is all based on work done by Thomas Zimmermann,
> I just wanted to implement my suggestion so
> we could select the best way forward.
> 
> Note: Daniel Vetter has hinted the approach implemented
> here smelled like middle-layer.
> IMO this is not so, it is just a way to handle cleanup
> for the simple cases.
> 
> 	Sam
> 
> 
> Sam Ravnborg (3):
>       drm: drm_encoder_init() => drm_encoder_init_funcs()
>       drm: drm_simple_encoder_init() => drm_encoder_init()
>       drm/atmel-hlcdc: Use drm_encoder_init()
> 
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c             | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c             | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c              | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c              | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.c           |  4 +-
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 10 ++---
>  .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c    | 10 ++---
>  drivers/gpu/drm/arc/arcpgu_hdmi.c                  |  4 +-
>  drivers/gpu/drm/arc/arcpgu_sim.c                   |  4 +-
>  drivers/gpu/drm/ast/ast_mode.c                     |  3 +-
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  8 +---
>  drivers/gpu/drm/drm_encoder.c                      | 49 +++++++++++++++++++---
>  drivers/gpu/drm/drm_encoder_slave.c                |  2 +-
>  drivers/gpu/drm/drm_simple_kms_helper.c            | 45 +-------------------
>  drivers/gpu/drm/drm_writeback.c                    |  6 +--
>  drivers/gpu/drm/exynos/exynos_dp.c                 |  4 +-
>  drivers/gpu/drm/exynos/exynos_drm_dpi.c            |  4 +-
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c            |  4 +-
>  drivers/gpu/drm/exynos/exynos_drm_vidi.c           |  4 +-
>  drivers/gpu/drm/exynos/exynos_hdmi.c               |  4 +-
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c          |  4 +-
>  drivers/gpu/drm/gma500/cdv_intel_crt.c             |  5 ++-
>  drivers/gpu/drm/gma500/cdv_intel_dp.c              |  4 +-
>  drivers/gpu/drm/gma500/cdv_intel_hdmi.c            |  4 +-
>  drivers/gpu/drm/gma500/cdv_intel_lvds.c            |  6 +--
>  drivers/gpu/drm/gma500/mdfld_dsi_dpi.c             |  7 ++--
>  drivers/gpu/drm/gma500/oaktrail_hdmi.c             |  6 +--
>  drivers/gpu/drm/gma500/oaktrail_lvds.c             |  4 +-
>  drivers/gpu/drm/gma500/psb_intel_lvds.c            |  6 +--
>  drivers/gpu/drm/gma500/psb_intel_sdvo.c            |  4 +-
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c   |  4 +-
>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c       |  4 +-
>  drivers/gpu/drm/i2c/tda998x_drv.c                  |  5 ++-
>  drivers/gpu/drm/i915/display/icl_dsi.c             |  4 +-
>  drivers/gpu/drm/i915/display/intel_crt.c           |  5 ++-
>  drivers/gpu/drm/i915/display/intel_ddi.c           |  6 ++-
>  drivers/gpu/drm/i915/display/intel_dp.c            |  6 +--
>  drivers/gpu/drm/i915/display/intel_dp_mst.c        |  6 ++-
>  drivers/gpu/drm/i915/display/intel_dvo.c           |  6 +--
>  drivers/gpu/drm/i915/display/intel_hdmi.c          |  6 +--
>  drivers/gpu/drm/i915/display/intel_lvds.c          |  4 +-
>  drivers/gpu/drm/i915/display/intel_sdvo.c          |  6 +--
>  drivers/gpu/drm/i915/display/intel_tv.c            |  4 +-
>  drivers/gpu/drm/i915/display/vlv_dsi.c             |  5 ++-
>  drivers/gpu/drm/imx/dw_hdmi-imx.c                  |  4 +-
>  drivers/gpu/drm/imx/imx-ldb.c                      |  4 +-
>  drivers/gpu/drm/imx/imx-tve.c                      |  4 +-
>  drivers/gpu/drm/imx/parallel-display.c             |  4 +-
>  drivers/gpu/drm/ingenic/ingenic-drm.c              |  5 ++-
>  drivers/gpu/drm/mediatek/mtk_dpi.c                 |  5 ++-
>  drivers/gpu/drm/mediatek/mtk_dsi.c                 |  4 +-
>  drivers/gpu/drm/meson/meson_dw_hdmi.c              |  5 ++-
>  drivers/gpu/drm/meson/meson_venc_cvbs.c            |  5 ++-
>  drivers/gpu/drm/mgag200/mgag200_mode.c             |  7 +---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c        |  4 +-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c   |  4 +-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c   |  4 +-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c  |  4 +-
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c       |  3 +-
>  drivers/gpu/drm/nouveau/dispnv04/dac.c             |  4 +-
>  drivers/gpu/drm/nouveau/dispnv04/dfp.c             |  3 +-
>  drivers/gpu/drm/nouveau/dispnv04/tvnv04.c          |  4 +-
>  drivers/gpu/drm/nouveau/dispnv04/tvnv17.c          |  4 +-
>  drivers/gpu/drm/nouveau/dispnv50/disp.c            | 16 +++----
>  drivers/gpu/drm/omapdrm/omap_encoder.c             |  4 +-
>  drivers/gpu/drm/qxl/qxl_display.c                  |  7 +---
>  drivers/gpu/drm/radeon/atombios_encoders.c         | 40 +++++++++---------
>  drivers/gpu/drm/radeon/radeon_dp_mst.c             |  4 +-
>  drivers/gpu/drm/radeon/radeon_legacy_encoders.c    | 20 ++++-----
>  drivers/gpu/drm/rcar-du/rcar_du_encoder.c          |  4 +-
>  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c    |  5 ++-
>  drivers/gpu/drm/rockchip/cdn-dp-core.c             |  4 +-
>  drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c    |  5 ++-
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        |  4 +-
>  drivers/gpu/drm/rockchip/inno_hdmi.c               |  4 +-
>  drivers/gpu/drm/rockchip/rk3066_hdmi.c             |  4 +-
>  drivers/gpu/drm/rockchip/rockchip_lvds.c           |  5 ++-
>  drivers/gpu/drm/rockchip/rockchip_rgb.c            |  5 ++-
>  drivers/gpu/drm/shmobile/shmob_drm_crtc.c          |  4 +-
>  drivers/gpu/drm/sti/sti_tvout.c                    | 16 +++----
>  drivers/gpu/drm/stm/ltdc.c                         |  4 +-
>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c             | 10 ++---
>  drivers/gpu/drm/sun4i/sun4i_lvds.c                 | 10 ++---
>  drivers/gpu/drm/sun4i/sun4i_rgb.c                  | 10 ++---
>  drivers/gpu/drm/sun4i/sun4i_tv.c                   | 10 ++---
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c             | 10 ++---
>  drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c              |  4 +-
>  drivers/gpu/drm/tegra/dsi.c                        |  6 +--
>  drivers/gpu/drm/tegra/hdmi.c                       |  5 ++-
>  drivers/gpu/drm/tegra/rgb.c                        |  4 +-
>  drivers/gpu/drm/tegra/sor.c                        |  4 +-
>  drivers/gpu/drm/tidss/tidss_encoder.c              |  4 +-
>  drivers/gpu/drm/tilcdc/tilcdc_external.c           |  8 ++--
>  drivers/gpu/drm/tilcdc/tilcdc_panel.c              |  4 +-
>  drivers/gpu/drm/vboxvideo/vbox_mode.c              |  4 +-
>  drivers/gpu/drm/vc4/vc4_dpi.c                      |  4 +-
>  drivers/gpu/drm/vc4/vc4_dsi.c                      |  4 +-
>  drivers/gpu/drm/vc4/vc4_hdmi.c                     |  4 +-
>  drivers/gpu/drm/vc4/vc4_vec.c                      |  4 +-
>  drivers/gpu/drm/virtio/virtgpu_display.c           |  4 +-
>  drivers/gpu/drm/vkms/vkms_output.c                 |  4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c                |  4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c               |  5 ++-
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c               |  4 +-
>  drivers/gpu/drm/zte/zx_hdmi.c                      |  4 +-
>  drivers/gpu/drm/zte/zx_tvenc.c                     |  4 +-
>  drivers/gpu/drm/zte/zx_vga.c                       |  4 +-
>  include/drm/drm_encoder.h                          |  9 ++--
>  include/drm/drm_simple_kms_helper.h                |  4 --
>  109 files changed, 394 insertions(+), 395 deletions(-)
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID
From: Thomas Zimmermann <tzimmermann@suse.de>
To: Sam Ravnborg <sam@ravnborg.org>, Daniel Vetter <daniel@ffwll.ch>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	dri-devel@lists.freedesktop.org
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>,
	linux-samsung-soc@vger.kernel.org,
	Boris Brezillon <bbrezillon@kernel.org>,
	David Airlie <airlied@linux.ie>,
	Andrzej Pietrasiewicz <andrzej.p@collabora.com>,
	Nicolas Ferre <nicolas.ferre@microchip.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	Jose Roberto de Souza <jose.souza@intel.com>,
	virtualization@lists.linux-foundation.org,
	linux-renesas-soc@vger.kernel.org,
	linux-rockchip@lists.infradead.org,
	Ludovic Desroches <ludovic.desroches@microchip.com>,
	linux-mediatek@lists.infradead.org,
	Gerd Hoffmann <kraxel@redhat.com>,
	linux-amlogic@lists.infradead.org, linux-tegra@vger.kernel.org,
	Dave Airlie <airlied@redhat.com>,
	amd-gfx@lists.freedesktop.org,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org,
	Emil Velikov <emil.velikov@collabora.com>
Subject: Re: [PATCH v1 0/3] drm: drm_encoder_init() => drm_encoder_init_funcs()
Date: Tue, 24 Mar 2020 09:33:23 +0100	[thread overview]
Message-ID: <c93bb45d-7922-56dc-061f-6ef70493a358@suse.de> (raw)
In-Reply-To: <20200313201744.19773-1-sam@ravnborg.org>


[-- Attachment #1.1.1: Type: text/plain, Size: 10292 bytes --]

Hi Sam

Am 13.03.20 um 21:17 schrieb Sam Ravnborg:
> Thomas Zimmermann had made a nice patch-set that introduced
> drm_simple_encoder_init() which is already present in drm-misc-next.
> 
> While looking at this it was suddenly obvious to me that
> this was functionalty that really should be included in drm_encoder.c
> The case where the core could handle the callback is pretty
> common and not part of the simple pipe line.

The original patchset put the new function into the core implementation
and was shot down for this. So it ended up in the simple-KMS helpers.

> 
> So after some dialog on dri-devel the conclusion was to go for
> a change like this:
> 
>     drm_encoder_init_funcs() for all users that specified a
>     drm_encoder_funcs to extend the functionality.
> 
>     drm_encoder_init() for all users that did not
>     need to extend the basic functionality with
>     drm_encoder_funcs.

TBH, my take-away was to keep the core as it is ans maybe rename
drm_simple_encoder_init() to some better name.

> 
> A similar approach with a _funcs() prefix is used elsewhere in drm/

IMHO, there are a few things to consider:

From grepping, I could only find drm_gem_fb_create_with_funcs(). And the
proposed change would make the encoder's function name inconsistent with
drm_connector_init(), drm_crtc_init(), and others. Finally,
drm_connector_init_with_ddc() was criticiced for being mid-layerish and
could lead to many combinations of postfixes (e.g., _with_funcs(),
with_ddc(), _with_ddc_and_funcs(), etc).

If there is consent that the drm_simple_encoder_init() should go away,
I'd propose to change drm_encoder_init(). It could use a default
implementation for funcs, if no funcs argument has been specified. We
already have such behavior for some GEM callbacks. In later patches,
drm_gem_fb_create_with_funcs() and drm_connector_init_with_ddc() could
go away as well.

Best regards
Thomas


> 
> This required a rename of the existing users, and
> a follow-up patch that moves drm_simple_encoder_init()
> to drm_encoder.c
> 
> Patches 3 in this set demonstrate the use of drm_encoder_init().
> There are many more drivers that can be converted as Thomas
> has already demonstrated.
> 
> This is all based on work done by Thomas Zimmermann,
> I just wanted to implement my suggestion so
> we could select the best way forward.
> 
> Note: Daniel Vetter has hinted the approach implemented
> here smelled like middle-layer.
> IMO this is not so, it is just a way to handle cleanup
> for the simple cases.
> 
> 	Sam
> 
> 
> Sam Ravnborg (3):
>       drm: drm_encoder_init() => drm_encoder_init_funcs()
>       drm: drm_simple_encoder_init() => drm_encoder_init()
>       drm/atmel-hlcdc: Use drm_encoder_init()
> 
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c             | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c             | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c              | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c              | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.c           |  4 +-
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 10 ++---
>  .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c    | 10 ++---
>  drivers/gpu/drm/arc/arcpgu_hdmi.c                  |  4 +-
>  drivers/gpu/drm/arc/arcpgu_sim.c                   |  4 +-
>  drivers/gpu/drm/ast/ast_mode.c                     |  3 +-
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  8 +---
>  drivers/gpu/drm/drm_encoder.c                      | 49 +++++++++++++++++++---
>  drivers/gpu/drm/drm_encoder_slave.c                |  2 +-
>  drivers/gpu/drm/drm_simple_kms_helper.c            | 45 +-------------------
>  drivers/gpu/drm/drm_writeback.c                    |  6 +--
>  drivers/gpu/drm/exynos/exynos_dp.c                 |  4 +-
>  drivers/gpu/drm/exynos/exynos_drm_dpi.c            |  4 +-
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c            |  4 +-
>  drivers/gpu/drm/exynos/exynos_drm_vidi.c           |  4 +-
>  drivers/gpu/drm/exynos/exynos_hdmi.c               |  4 +-
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c          |  4 +-
>  drivers/gpu/drm/gma500/cdv_intel_crt.c             |  5 ++-
>  drivers/gpu/drm/gma500/cdv_intel_dp.c              |  4 +-
>  drivers/gpu/drm/gma500/cdv_intel_hdmi.c            |  4 +-
>  drivers/gpu/drm/gma500/cdv_intel_lvds.c            |  6 +--
>  drivers/gpu/drm/gma500/mdfld_dsi_dpi.c             |  7 ++--
>  drivers/gpu/drm/gma500/oaktrail_hdmi.c             |  6 +--
>  drivers/gpu/drm/gma500/oaktrail_lvds.c             |  4 +-
>  drivers/gpu/drm/gma500/psb_intel_lvds.c            |  6 +--
>  drivers/gpu/drm/gma500/psb_intel_sdvo.c            |  4 +-
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c   |  4 +-
>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c       |  4 +-
>  drivers/gpu/drm/i2c/tda998x_drv.c                  |  5 ++-
>  drivers/gpu/drm/i915/display/icl_dsi.c             |  4 +-
>  drivers/gpu/drm/i915/display/intel_crt.c           |  5 ++-
>  drivers/gpu/drm/i915/display/intel_ddi.c           |  6 ++-
>  drivers/gpu/drm/i915/display/intel_dp.c            |  6 +--
>  drivers/gpu/drm/i915/display/intel_dp_mst.c        |  6 ++-
>  drivers/gpu/drm/i915/display/intel_dvo.c           |  6 +--
>  drivers/gpu/drm/i915/display/intel_hdmi.c          |  6 +--
>  drivers/gpu/drm/i915/display/intel_lvds.c          |  4 +-
>  drivers/gpu/drm/i915/display/intel_sdvo.c          |  6 +--
>  drivers/gpu/drm/i915/display/intel_tv.c            |  4 +-
>  drivers/gpu/drm/i915/display/vlv_dsi.c             |  5 ++-
>  drivers/gpu/drm/imx/dw_hdmi-imx.c                  |  4 +-
>  drivers/gpu/drm/imx/imx-ldb.c                      |  4 +-
>  drivers/gpu/drm/imx/imx-tve.c                      |  4 +-
>  drivers/gpu/drm/imx/parallel-display.c             |  4 +-
>  drivers/gpu/drm/ingenic/ingenic-drm.c              |  5 ++-
>  drivers/gpu/drm/mediatek/mtk_dpi.c                 |  5 ++-
>  drivers/gpu/drm/mediatek/mtk_dsi.c                 |  4 +-
>  drivers/gpu/drm/meson/meson_dw_hdmi.c              |  5 ++-
>  drivers/gpu/drm/meson/meson_venc_cvbs.c            |  5 ++-
>  drivers/gpu/drm/mgag200/mgag200_mode.c             |  7 +---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c        |  4 +-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c   |  4 +-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c   |  4 +-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c  |  4 +-
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c       |  3 +-
>  drivers/gpu/drm/nouveau/dispnv04/dac.c             |  4 +-
>  drivers/gpu/drm/nouveau/dispnv04/dfp.c             |  3 +-
>  drivers/gpu/drm/nouveau/dispnv04/tvnv04.c          |  4 +-
>  drivers/gpu/drm/nouveau/dispnv04/tvnv17.c          |  4 +-
>  drivers/gpu/drm/nouveau/dispnv50/disp.c            | 16 +++----
>  drivers/gpu/drm/omapdrm/omap_encoder.c             |  4 +-
>  drivers/gpu/drm/qxl/qxl_display.c                  |  7 +---
>  drivers/gpu/drm/radeon/atombios_encoders.c         | 40 +++++++++---------
>  drivers/gpu/drm/radeon/radeon_dp_mst.c             |  4 +-
>  drivers/gpu/drm/radeon/radeon_legacy_encoders.c    | 20 ++++-----
>  drivers/gpu/drm/rcar-du/rcar_du_encoder.c          |  4 +-
>  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c    |  5 ++-
>  drivers/gpu/drm/rockchip/cdn-dp-core.c             |  4 +-
>  drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c    |  5 ++-
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        |  4 +-
>  drivers/gpu/drm/rockchip/inno_hdmi.c               |  4 +-
>  drivers/gpu/drm/rockchip/rk3066_hdmi.c             |  4 +-
>  drivers/gpu/drm/rockchip/rockchip_lvds.c           |  5 ++-
>  drivers/gpu/drm/rockchip/rockchip_rgb.c            |  5 ++-
>  drivers/gpu/drm/shmobile/shmob_drm_crtc.c          |  4 +-
>  drivers/gpu/drm/sti/sti_tvout.c                    | 16 +++----
>  drivers/gpu/drm/stm/ltdc.c                         |  4 +-
>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c             | 10 ++---
>  drivers/gpu/drm/sun4i/sun4i_lvds.c                 | 10 ++---
>  drivers/gpu/drm/sun4i/sun4i_rgb.c                  | 10 ++---
>  drivers/gpu/drm/sun4i/sun4i_tv.c                   | 10 ++---
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c             | 10 ++---
>  drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c              |  4 +-
>  drivers/gpu/drm/tegra/dsi.c                        |  6 +--
>  drivers/gpu/drm/tegra/hdmi.c                       |  5 ++-
>  drivers/gpu/drm/tegra/rgb.c                        |  4 +-
>  drivers/gpu/drm/tegra/sor.c                        |  4 +-
>  drivers/gpu/drm/tidss/tidss_encoder.c              |  4 +-
>  drivers/gpu/drm/tilcdc/tilcdc_external.c           |  8 ++--
>  drivers/gpu/drm/tilcdc/tilcdc_panel.c              |  4 +-
>  drivers/gpu/drm/vboxvideo/vbox_mode.c              |  4 +-
>  drivers/gpu/drm/vc4/vc4_dpi.c                      |  4 +-
>  drivers/gpu/drm/vc4/vc4_dsi.c                      |  4 +-
>  drivers/gpu/drm/vc4/vc4_hdmi.c                     |  4 +-
>  drivers/gpu/drm/vc4/vc4_vec.c                      |  4 +-
>  drivers/gpu/drm/virtio/virtgpu_display.c           |  4 +-
>  drivers/gpu/drm/vkms/vkms_output.c                 |  4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c                |  4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c               |  5 ++-
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c               |  4 +-
>  drivers/gpu/drm/zte/zx_hdmi.c                      |  4 +-
>  drivers/gpu/drm/zte/zx_tvenc.c                     |  4 +-
>  drivers/gpu/drm/zte/zx_vga.c                       |  4 +-
>  include/drm/drm_encoder.h                          |  9 ++--
>  include/drm/drm_simple_kms_helper.h                |  4 --
>  109 files changed, 394 insertions(+), 395 deletions(-)
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID
From: Thomas Zimmermann <tzimmermann@suse.de>
To: Sam Ravnborg <sam@ravnborg.org>, Daniel Vetter <daniel@ffwll.ch>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	dri-devel@lists.freedesktop.org
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>,
	linux-samsung-soc@vger.kernel.org,
	Boris Brezillon <bbrezillon@kernel.org>,
	David Airlie <airlied@linux.ie>,
	Andrzej Pietrasiewicz <andrzej.p@collabora.com>,
	Nicolas Ferre <nicolas.ferre@microchip.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	Jose Roberto de Souza <jose.souza@intel.com>,
	virtualization@lists.linux-foundation.org,
	linux-renesas-soc@vger.kernel.org,
	linux-rockchip@lists.infradead.org,
	Ludovic Desroches <ludovic.desroches@microchip.com>,
	linux-mediatek@lists.infradead.org,
	Gerd Hoffmann <kraxel@redhat.com>,
	linux-amlogic@lists.infradead.org, linux-tegra@vger.kernel.org,
	Dave Airlie <airlied@redhat.com>,
	amd-gfx@lists.freedesktop.org,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org,
	Emil Velikov <emil.velikov@collabora.com>
Subject: Re: [PATCH v1 0/3] drm: drm_encoder_init() => drm_encoder_init_funcs()
Date: Tue, 24 Mar 2020 09:33:23 +0100	[thread overview]
Message-ID: <c93bb45d-7922-56dc-061f-6ef70493a358@suse.de> (raw)
In-Reply-To: <20200313201744.19773-1-sam@ravnborg.org>


[-- Attachment #1.1.1: Type: text/plain, Size: 10292 bytes --]

Hi Sam

Am 13.03.20 um 21:17 schrieb Sam Ravnborg:
> Thomas Zimmermann had made a nice patch-set that introduced
> drm_simple_encoder_init() which is already present in drm-misc-next.
> 
> While looking at this it was suddenly obvious to me that
> this was functionalty that really should be included in drm_encoder.c
> The case where the core could handle the callback is pretty
> common and not part of the simple pipe line.

The original patchset put the new function into the core implementation
and was shot down for this. So it ended up in the simple-KMS helpers.

> 
> So after some dialog on dri-devel the conclusion was to go for
> a change like this:
> 
>     drm_encoder_init_funcs() for all users that specified a
>     drm_encoder_funcs to extend the functionality.
> 
>     drm_encoder_init() for all users that did not
>     need to extend the basic functionality with
>     drm_encoder_funcs.

TBH, my take-away was to keep the core as it is ans maybe rename
drm_simple_encoder_init() to some better name.

> 
> A similar approach with a _funcs() prefix is used elsewhere in drm/

IMHO, there are a few things to consider:

From grepping, I could only find drm_gem_fb_create_with_funcs(). And the
proposed change would make the encoder's function name inconsistent with
drm_connector_init(), drm_crtc_init(), and others. Finally,
drm_connector_init_with_ddc() was criticiced for being mid-layerish and
could lead to many combinations of postfixes (e.g., _with_funcs(),
with_ddc(), _with_ddc_and_funcs(), etc).

If there is consent that the drm_simple_encoder_init() should go away,
I'd propose to change drm_encoder_init(). It could use a default
implementation for funcs, if no funcs argument has been specified. We
already have such behavior for some GEM callbacks. In later patches,
drm_gem_fb_create_with_funcs() and drm_connector_init_with_ddc() could
go away as well.

Best regards
Thomas


> 
> This required a rename of the existing users, and
> a follow-up patch that moves drm_simple_encoder_init()
> to drm_encoder.c
> 
> Patches 3 in this set demonstrate the use of drm_encoder_init().
> There are many more drivers that can be converted as Thomas
> has already demonstrated.
> 
> This is all based on work done by Thomas Zimmermann,
> I just wanted to implement my suggestion so
> we could select the best way forward.
> 
> Note: Daniel Vetter has hinted the approach implemented
> here smelled like middle-layer.
> IMO this is not so, it is just a way to handle cleanup
> for the simple cases.
> 
> 	Sam
> 
> 
> Sam Ravnborg (3):
>       drm: drm_encoder_init() => drm_encoder_init_funcs()
>       drm: drm_simple_encoder_init() => drm_encoder_init()
>       drm/atmel-hlcdc: Use drm_encoder_init()
> 
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c             | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c             | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c              | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c              | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.c           |  4 +-
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 10 ++---
>  .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c    | 10 ++---
>  drivers/gpu/drm/arc/arcpgu_hdmi.c                  |  4 +-
>  drivers/gpu/drm/arc/arcpgu_sim.c                   |  4 +-
>  drivers/gpu/drm/ast/ast_mode.c                     |  3 +-
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  8 +---
>  drivers/gpu/drm/drm_encoder.c                      | 49 +++++++++++++++++++---
>  drivers/gpu/drm/drm_encoder_slave.c                |  2 +-
>  drivers/gpu/drm/drm_simple_kms_helper.c            | 45 +-------------------
>  drivers/gpu/drm/drm_writeback.c                    |  6 +--
>  drivers/gpu/drm/exynos/exynos_dp.c                 |  4 +-
>  drivers/gpu/drm/exynos/exynos_drm_dpi.c            |  4 +-
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c            |  4 +-
>  drivers/gpu/drm/exynos/exynos_drm_vidi.c           |  4 +-
>  drivers/gpu/drm/exynos/exynos_hdmi.c               |  4 +-
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c          |  4 +-
>  drivers/gpu/drm/gma500/cdv_intel_crt.c             |  5 ++-
>  drivers/gpu/drm/gma500/cdv_intel_dp.c              |  4 +-
>  drivers/gpu/drm/gma500/cdv_intel_hdmi.c            |  4 +-
>  drivers/gpu/drm/gma500/cdv_intel_lvds.c            |  6 +--
>  drivers/gpu/drm/gma500/mdfld_dsi_dpi.c             |  7 ++--
>  drivers/gpu/drm/gma500/oaktrail_hdmi.c             |  6 +--
>  drivers/gpu/drm/gma500/oaktrail_lvds.c             |  4 +-
>  drivers/gpu/drm/gma500/psb_intel_lvds.c            |  6 +--
>  drivers/gpu/drm/gma500/psb_intel_sdvo.c            |  4 +-
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c   |  4 +-
>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c       |  4 +-
>  drivers/gpu/drm/i2c/tda998x_drv.c                  |  5 ++-
>  drivers/gpu/drm/i915/display/icl_dsi.c             |  4 +-
>  drivers/gpu/drm/i915/display/intel_crt.c           |  5 ++-
>  drivers/gpu/drm/i915/display/intel_ddi.c           |  6 ++-
>  drivers/gpu/drm/i915/display/intel_dp.c            |  6 +--
>  drivers/gpu/drm/i915/display/intel_dp_mst.c        |  6 ++-
>  drivers/gpu/drm/i915/display/intel_dvo.c           |  6 +--
>  drivers/gpu/drm/i915/display/intel_hdmi.c          |  6 +--
>  drivers/gpu/drm/i915/display/intel_lvds.c          |  4 +-
>  drivers/gpu/drm/i915/display/intel_sdvo.c          |  6 +--
>  drivers/gpu/drm/i915/display/intel_tv.c            |  4 +-
>  drivers/gpu/drm/i915/display/vlv_dsi.c             |  5 ++-
>  drivers/gpu/drm/imx/dw_hdmi-imx.c                  |  4 +-
>  drivers/gpu/drm/imx/imx-ldb.c                      |  4 +-
>  drivers/gpu/drm/imx/imx-tve.c                      |  4 +-
>  drivers/gpu/drm/imx/parallel-display.c             |  4 +-
>  drivers/gpu/drm/ingenic/ingenic-drm.c              |  5 ++-
>  drivers/gpu/drm/mediatek/mtk_dpi.c                 |  5 ++-
>  drivers/gpu/drm/mediatek/mtk_dsi.c                 |  4 +-
>  drivers/gpu/drm/meson/meson_dw_hdmi.c              |  5 ++-
>  drivers/gpu/drm/meson/meson_venc_cvbs.c            |  5 ++-
>  drivers/gpu/drm/mgag200/mgag200_mode.c             |  7 +---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c        |  4 +-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c   |  4 +-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c   |  4 +-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c  |  4 +-
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c       |  3 +-
>  drivers/gpu/drm/nouveau/dispnv04/dac.c             |  4 +-
>  drivers/gpu/drm/nouveau/dispnv04/dfp.c             |  3 +-
>  drivers/gpu/drm/nouveau/dispnv04/tvnv04.c          |  4 +-
>  drivers/gpu/drm/nouveau/dispnv04/tvnv17.c          |  4 +-
>  drivers/gpu/drm/nouveau/dispnv50/disp.c            | 16 +++----
>  drivers/gpu/drm/omapdrm/omap_encoder.c             |  4 +-
>  drivers/gpu/drm/qxl/qxl_display.c                  |  7 +---
>  drivers/gpu/drm/radeon/atombios_encoders.c         | 40 +++++++++---------
>  drivers/gpu/drm/radeon/radeon_dp_mst.c             |  4 +-
>  drivers/gpu/drm/radeon/radeon_legacy_encoders.c    | 20 ++++-----
>  drivers/gpu/drm/rcar-du/rcar_du_encoder.c          |  4 +-
>  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c    |  5 ++-
>  drivers/gpu/drm/rockchip/cdn-dp-core.c             |  4 +-
>  drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c    |  5 ++-
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        |  4 +-
>  drivers/gpu/drm/rockchip/inno_hdmi.c               |  4 +-
>  drivers/gpu/drm/rockchip/rk3066_hdmi.c             |  4 +-
>  drivers/gpu/drm/rockchip/rockchip_lvds.c           |  5 ++-
>  drivers/gpu/drm/rockchip/rockchip_rgb.c            |  5 ++-
>  drivers/gpu/drm/shmobile/shmob_drm_crtc.c          |  4 +-
>  drivers/gpu/drm/sti/sti_tvout.c                    | 16 +++----
>  drivers/gpu/drm/stm/ltdc.c                         |  4 +-
>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c             | 10 ++---
>  drivers/gpu/drm/sun4i/sun4i_lvds.c                 | 10 ++---
>  drivers/gpu/drm/sun4i/sun4i_rgb.c                  | 10 ++---
>  drivers/gpu/drm/sun4i/sun4i_tv.c                   | 10 ++---
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c             | 10 ++---
>  drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c              |  4 +-
>  drivers/gpu/drm/tegra/dsi.c                        |  6 +--
>  drivers/gpu/drm/tegra/hdmi.c                       |  5 ++-
>  drivers/gpu/drm/tegra/rgb.c                        |  4 +-
>  drivers/gpu/drm/tegra/sor.c                        |  4 +-
>  drivers/gpu/drm/tidss/tidss_encoder.c              |  4 +-
>  drivers/gpu/drm/tilcdc/tilcdc_external.c           |  8 ++--
>  drivers/gpu/drm/tilcdc/tilcdc_panel.c              |  4 +-
>  drivers/gpu/drm/vboxvideo/vbox_mode.c              |  4 +-
>  drivers/gpu/drm/vc4/vc4_dpi.c                      |  4 +-
>  drivers/gpu/drm/vc4/vc4_dsi.c                      |  4 +-
>  drivers/gpu/drm/vc4/vc4_hdmi.c                     |  4 +-
>  drivers/gpu/drm/vc4/vc4_vec.c                      |  4 +-
>  drivers/gpu/drm/virtio/virtgpu_display.c           |  4 +-
>  drivers/gpu/drm/vkms/vkms_output.c                 |  4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c                |  4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c               |  5 ++-
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c               |  4 +-
>  drivers/gpu/drm/zte/zx_hdmi.c                      |  4 +-
>  drivers/gpu/drm/zte/zx_tvenc.c                     |  4 +-
>  drivers/gpu/drm/zte/zx_vga.c                       |  4 +-
>  include/drm/drm_encoder.h                          |  9 ++--
>  include/drm/drm_simple_kms_helper.h                |  4 --
>  109 files changed, 394 insertions(+), 395 deletions(-)
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

WARNING: multiple messages have this Message-ID
From: Thomas Zimmermann <tzimmermann@suse.de>
To: Sam Ravnborg <sam@ravnborg.org>, Daniel Vetter <daniel@ffwll.ch>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	dri-devel@lists.freedesktop.org
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>,
	linux-samsung-soc@vger.kernel.org,
	Boris Brezillon <bbrezillon@kernel.org>,
	David Airlie <airlied@linux.ie>,
	Andrzej Pietrasiewicz <andrzej.p@collabora.com>,
	Nicolas Ferre <nicolas.ferre@microchip.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	Jose Roberto de Souza <jose.souza@intel.com>,
	virtualization@lists.linux-foundation.org,
	linux-renesas-soc@vger.kernel.org,
	linux-rockchip@lists.infradead.org,
	Ludovic Desroches <ludovic.desroches@microchip.com>,
	linux-mediatek@lists.infradead.org,
	Gerd Hoffmann <kraxel@redhat.com>,
	linux-amlogic@lists.infradead.org, linux-tegra@vger.kernel.org,
	Dave Airlie <airlied@redhat.com>,
	amd-gfx@lists.freedesktop.org,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org,
	Emil Velikov <emil.velikov@collabora.com>
Subject: Re: [PATCH v1 0/3] drm: drm_encoder_init() => drm_encoder_init_funcs()
Date: Tue, 24 Mar 2020 09:33:23 +0100	[thread overview]
Message-ID: <c93bb45d-7922-56dc-061f-6ef70493a358@suse.de> (raw)
In-Reply-To: <20200313201744.19773-1-sam@ravnborg.org>


[-- Attachment #1.1.1: Type: text/plain, Size: 10292 bytes --]

Hi Sam

Am 13.03.20 um 21:17 schrieb Sam Ravnborg:
> Thomas Zimmermann had made a nice patch-set that introduced
> drm_simple_encoder_init() which is already present in drm-misc-next.
> 
> While looking at this it was suddenly obvious to me that
> this was functionalty that really should be included in drm_encoder.c
> The case where the core could handle the callback is pretty
> common and not part of the simple pipe line.

The original patchset put the new function into the core implementation
and was shot down for this. So it ended up in the simple-KMS helpers.

> 
> So after some dialog on dri-devel the conclusion was to go for
> a change like this:
> 
>     drm_encoder_init_funcs() for all users that specified a
>     drm_encoder_funcs to extend the functionality.
> 
>     drm_encoder_init() for all users that did not
>     need to extend the basic functionality with
>     drm_encoder_funcs.

TBH, my take-away was to keep the core as it is ans maybe rename
drm_simple_encoder_init() to some better name.

> 
> A similar approach with a _funcs() prefix is used elsewhere in drm/

IMHO, there are a few things to consider:

From grepping, I could only find drm_gem_fb_create_with_funcs(). And the
proposed change would make the encoder's function name inconsistent with
drm_connector_init(), drm_crtc_init(), and others. Finally,
drm_connector_init_with_ddc() was criticiced for being mid-layerish and
could lead to many combinations of postfixes (e.g., _with_funcs(),
with_ddc(), _with_ddc_and_funcs(), etc).

If there is consent that the drm_simple_encoder_init() should go away,
I'd propose to change drm_encoder_init(). It could use a default
implementation for funcs, if no funcs argument has been specified. We
already have such behavior for some GEM callbacks. In later patches,
drm_gem_fb_create_with_funcs() and drm_connector_init_with_ddc() could
go away as well.

Best regards
Thomas


> 
> This required a rename of the existing users, and
> a follow-up patch that moves drm_simple_encoder_init()
> to drm_encoder.c
> 
> Patches 3 in this set demonstrate the use of drm_encoder_init().
> There are many more drivers that can be converted as Thomas
> has already demonstrated.
> 
> This is all based on work done by Thomas Zimmermann,
> I just wanted to implement my suggestion so
> we could select the best way forward.
> 
> Note: Daniel Vetter has hinted the approach implemented
> here smelled like middle-layer.
> IMO this is not so, it is just a way to handle cleanup
> for the simple cases.
> 
> 	Sam
> 
> 
> Sam Ravnborg (3):
>       drm: drm_encoder_init() => drm_encoder_init_funcs()
>       drm: drm_simple_encoder_init() => drm_encoder_init()
>       drm/atmel-hlcdc: Use drm_encoder_init()
> 
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c             | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c             | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c              | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c              | 28 ++++++-------
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.c           |  4 +-
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 10 ++---
>  .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c    | 10 ++---
>  drivers/gpu/drm/arc/arcpgu_hdmi.c                  |  4 +-
>  drivers/gpu/drm/arc/arcpgu_sim.c                   |  4 +-
>  drivers/gpu/drm/ast/ast_mode.c                     |  3 +-
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  8 +---
>  drivers/gpu/drm/drm_encoder.c                      | 49 +++++++++++++++++++---
>  drivers/gpu/drm/drm_encoder_slave.c                |  2 +-
>  drivers/gpu/drm/drm_simple_kms_helper.c            | 45 +-------------------
>  drivers/gpu/drm/drm_writeback.c                    |  6 +--
>  drivers/gpu/drm/exynos/exynos_dp.c                 |  4 +-
>  drivers/gpu/drm/exynos/exynos_drm_dpi.c            |  4 +-
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c            |  4 +-
>  drivers/gpu/drm/exynos/exynos_drm_vidi.c           |  4 +-
>  drivers/gpu/drm/exynos/exynos_hdmi.c               |  4 +-
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c          |  4 +-
>  drivers/gpu/drm/gma500/cdv_intel_crt.c             |  5 ++-
>  drivers/gpu/drm/gma500/cdv_intel_dp.c              |  4 +-
>  drivers/gpu/drm/gma500/cdv_intel_hdmi.c            |  4 +-
>  drivers/gpu/drm/gma500/cdv_intel_lvds.c            |  6 +--
>  drivers/gpu/drm/gma500/mdfld_dsi_dpi.c             |  7 ++--
>  drivers/gpu/drm/gma500/oaktrail_hdmi.c             |  6 +--
>  drivers/gpu/drm/gma500/oaktrail_lvds.c             |  4 +-
>  drivers/gpu/drm/gma500/psb_intel_lvds.c            |  6 +--
>  drivers/gpu/drm/gma500/psb_intel_sdvo.c            |  4 +-
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c   |  4 +-
>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c       |  4 +-
>  drivers/gpu/drm/i2c/tda998x_drv.c                  |  5 ++-
>  drivers/gpu/drm/i915/display/icl_dsi.c             |  4 +-
>  drivers/gpu/drm/i915/display/intel_crt.c           |  5 ++-
>  drivers/gpu/drm/i915/display/intel_ddi.c           |  6 ++-
>  drivers/gpu/drm/i915/display/intel_dp.c            |  6 +--
>  drivers/gpu/drm/i915/display/intel_dp_mst.c        |  6 ++-
>  drivers/gpu/drm/i915/display/intel_dvo.c           |  6 +--
>  drivers/gpu/drm/i915/display/intel_hdmi.c          |  6 +--
>  drivers/gpu/drm/i915/display/intel_lvds.c          |  4 +-
>  drivers/gpu/drm/i915/display/intel_sdvo.c          |  6 +--
>  drivers/gpu/drm/i915/display/intel_tv.c            |  4 +-
>  drivers/gpu/drm/i915/display/vlv_dsi.c             |  5 ++-
>  drivers/gpu/drm/imx/dw_hdmi-imx.c                  |  4 +-
>  drivers/gpu/drm/imx/imx-ldb.c                      |  4 +-
>  drivers/gpu/drm/imx/imx-tve.c                      |  4 +-
>  drivers/gpu/drm/imx/parallel-display.c             |  4 +-
>  drivers/gpu/drm/ingenic/ingenic-drm.c              |  5 ++-
>  drivers/gpu/drm/mediatek/mtk_dpi.c                 |  5 ++-
>  drivers/gpu/drm/mediatek/mtk_dsi.c                 |  4 +-
>  drivers/gpu/drm/meson/meson_dw_hdmi.c              |  5 ++-
>  drivers/gpu/drm/meson/meson_venc_cvbs.c            |  5 ++-
>  drivers/gpu/drm/mgag200/mgag200_mode.c             |  7 +---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c        |  4 +-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c   |  4 +-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c   |  4 +-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c  |  4 +-
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c       |  3 +-
>  drivers/gpu/drm/nouveau/dispnv04/dac.c             |  4 +-
>  drivers/gpu/drm/nouveau/dispnv04/dfp.c             |  3 +-
>  drivers/gpu/drm/nouveau/dispnv04/tvnv04.c          |  4 +-
>  drivers/gpu/drm/nouveau/dispnv04/tvnv17.c          |  4 +-
>  drivers/gpu/drm/nouveau/dispnv50/disp.c            | 16 +++----
>  drivers/gpu/drm/omapdrm/omap_encoder.c             |  4 +-
>  drivers/gpu/drm/qxl/qxl_display.c                  |  7 +---
>  drivers/gpu/drm/radeon/atombios_encoders.c         | 40 +++++++++---------
>  drivers/gpu/drm/radeon/radeon_dp_mst.c             |  4 +-
>  drivers/gpu/drm/radeon/radeon_legacy_encoders.c    | 20 ++++-----
>  drivers/gpu/drm/rcar-du/rcar_du_encoder.c          |  4 +-
>  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c    |  5 ++-
>  drivers/gpu/drm/rockchip/cdn-dp-core.c             |  4 +-
>  drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c    |  5 ++-
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        |  4 +-
>  drivers/gpu/drm/rockchip/inno_hdmi.c               |  4 +-
>  drivers/gpu/drm/rockchip/rk3066_hdmi.c             |  4 +-
>  drivers/gpu/drm/rockchip/rockchip_lvds.c           |  5 ++-
>  drivers/gpu/drm/rockchip/rockchip_rgb.c            |  5 ++-
>  drivers/gpu/drm/shmobile/shmob_drm_crtc.c          |  4 +-
>  drivers/gpu/drm/sti/sti_tvout.c                    | 16 +++----
>  drivers/gpu/drm/stm/ltdc.c                         |  4 +-
>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c             | 10 ++---
>  drivers/gpu/drm/sun4i/sun4i_lvds.c                 | 10 ++---
>  drivers/gpu/drm/sun4i/sun4i_rgb.c                  | 10 ++---
>  drivers/gpu/drm/sun4i/sun4i_tv.c                   | 10 ++---
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c             | 10 ++---
>  drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c              |  4 +-
>  drivers/gpu/drm/tegra/dsi.c                        |  6 +--
>  drivers/gpu/drm/tegra/hdmi.c                       |  5 ++-
>  drivers/gpu/drm/tegra/rgb.c                        |  4 +-
>  drivers/gpu/drm/tegra/sor.c                        |  4 +-
>  drivers/gpu/drm/tidss/tidss_encoder.c              |  4 +-
>  drivers/gpu/drm/tilcdc/tilcdc_external.c           |  8 ++--
>  drivers/gpu/drm/tilcdc/tilcdc_panel.c              |  4 +-
>  drivers/gpu/drm/vboxvideo/vbox_mode.c              |  4 +-
>  drivers/gpu/drm/vc4/vc4_dpi.c                      |  4 +-
>  drivers/gpu/drm/vc4/vc4_dsi.c                      |  4 +-
>  drivers/gpu/drm/vc4/vc4_hdmi.c                     |  4 +-
>  drivers/gpu/drm/vc4/vc4_vec.c                      |  4 +-
>  drivers/gpu/drm/virtio/virtgpu_display.c           |  4 +-
>  drivers/gpu/drm/vkms/vkms_output.c                 |  4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c                |  4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c               |  5 ++-
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c               |  4 +-
>  drivers/gpu/drm/zte/zx_hdmi.c                      |  4 +-
>  drivers/gpu/drm/zte/zx_tvenc.c                     |  4 +-
>  drivers/gpu/drm/zte/zx_vga.c                       |  4 +-
>  include/drm/drm_encoder.h                          |  9 ++--
>  include/drm/drm_simple_kms_helper.h                |  4 --
>  109 files changed, 394 insertions(+), 395 deletions(-)
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

  parent reply	other threads:[~2020-03-24  8:33 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-13 20:17 Sam Ravnborg
2020-03-13 20:17 ` Sam Ravnborg
2020-03-13 20:17 ` Sam Ravnborg
2020-03-13 20:17 ` Sam Ravnborg
2020-03-13 20:17 ` Sam Ravnborg
2020-03-13 20:17 ` Sam Ravnborg
2020-03-13 20:17 ` Sam Ravnborg
2020-03-13 20:17 ` [PATCH v1 3/3] drm/atmel-hlcdc: Use drm_encoder_init() Sam Ravnborg
2020-03-13 20:17   ` Sam Ravnborg
2020-03-13 20:17   ` Sam Ravnborg
2020-03-13 20:17   ` Sam Ravnborg
2020-03-13 20:17   ` Sam Ravnborg
2020-03-13 20:17   ` Sam Ravnborg
2020-03-13 20:17   ` Sam Ravnborg
     [not found] ` <20200313201744.19773-1-sam-uyr5N9Q2VtJg9hUCZPvPmw@public.gmane.org>
2020-03-13 20:17   ` [PATCH v1 1/3] drm: drm_encoder_init() => drm_encoder_init_funcs() Sam Ravnborg
2020-03-13 20:17     ` Sam Ravnborg
2020-03-13 20:17     ` Sam Ravnborg
2020-03-13 20:17     ` Sam Ravnborg
2020-03-13 20:17     ` Sam Ravnborg
2020-03-13 20:17     ` Sam Ravnborg
2020-03-13 20:17     ` Sam Ravnborg
2020-03-13 20:17   ` [PATCH v1 2/3] drm: drm_simple_encoder_init() => drm_encoder_init() Sam Ravnborg
2020-03-13 20:17     ` Sam Ravnborg
2020-03-13 20:17     ` Sam Ravnborg
2020-03-13 20:17     ` Sam Ravnborg
2020-03-13 20:17     ` Sam Ravnborg
2020-03-13 20:17     ` Sam Ravnborg
2020-03-13 20:17     ` Sam Ravnborg
2020-03-19 14:19   ` [PATCH v1 0/3] drm: drm_encoder_init() => drm_encoder_init_funcs() Sam Ravnborg
2020-03-19 14:19     ` Sam Ravnborg
2020-03-19 14:19     ` Sam Ravnborg
2020-03-19 14:19     ` Sam Ravnborg
2020-03-19 14:19     ` Sam Ravnborg
2020-03-19 14:19     ` Sam Ravnborg
2020-03-19 14:19     ` Sam Ravnborg
     [not found]     ` <20200319141954.GA25036-uyr5N9Q2VtJg9hUCZPvPmw@public.gmane.org>
2020-03-19 17:05       ` Sam Ravnborg
2020-03-19 17:05         ` Sam Ravnborg
2020-03-19 17:05         ` Sam Ravnborg
2020-03-19 17:05         ` Sam Ravnborg
2020-03-19 17:05         ` Sam Ravnborg
2020-03-19 17:05         ` Sam Ravnborg
2020-03-19 17:05         ` Sam Ravnborg
2020-03-24  8:33   ` Thomas Zimmermann [this message]
2020-03-24  8:33     ` Thomas Zimmermann
2020-03-24  8:33     ` Thomas Zimmermann
2020-03-24  8:33     ` Thomas Zimmermann
2020-03-24  8:33     ` Thomas Zimmermann
2020-03-24  8:33     ` Thomas Zimmermann
2020-03-24  8:33     ` Thomas Zimmermann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c93bb45d-7922-56dc-061f-6ef70493a358@suse.de \
    --to=tzimmermann-l3a5bk7wagm@public.gmane.org \
    --cc=airlied-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=airlied-cv59FeDIM0c@public.gmane.org \
    --cc=alexandre.belloni-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org \
    --cc=amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
    --cc=bbrezillon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=daniel-/w4YWyX8dFk@public.gmane.org \
    --cc=dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
    --cc=emil.velikov-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org \
    --cc=jose.souza-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    --cc=kraxel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org \
    --cc=linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-imx-3arQi8VN3Tc@public.gmane.org \
    --cc=linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-stm32-XDFAJ8BFU24N7RejjzZ/Li2xQDfSxrLKVpNB7YpNyf8@public.gmane.org \
    --cc=linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=ludovic.desroches-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org \
    --cc=nicolas.ferre-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org \
    --cc=sam-uyr5N9Q2VtJg9hUCZPvPmw@public.gmane.org \
    --cc=virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --subject='Re: [PATCH v1 0/3] drm: drm_encoder_init() => drm_encoder_init_funcs()' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.