Re: [PATCH 0/8] Cleanup CRTC .enable()/.disable() cargo-cult

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Daniel,

On Wednesday 28 Jun 2017 10:20:33 Daniel Vetter wrote:
> On Wed, Jun 28, 2017 at 10:16:14AM +0200, Daniel Vetter wrote:
> > On Tue, Jun 27, 2017 at 11:38:35PM +0300, Laurent Pinchart wrote:
> >> Hello,
> >> 
> >> The atomic helpers favour the .enable() and .atomic_disable() CRTC
> >> helper operations, but still support the legacy .prepare(), .commit(),
> >> .disable() and .dpms() operations. Some drivers have been updated, but
> >> most still use various combination of new and legacy operations,
> >> leading to confusion among new developers when they read the code.
> >> 
> >> To avoid cargo-cult use of the legacy operations, this patch series
> >> mass-updates all atomic drivers to use the new CRTC atomic helper
> >> operations.
> >> 
> >> In addition, patch 7/8 adds an old state pointer argument to the CRTC
> >> .enable() helper operation, and rename it to .atomic_enable() for
> >> consistency with .atomic_disable(). These two changes could have been
> >> split in separate patch, but as they are simple and touch the same
> >> large number of files, keeping the two changes as one is simpler and
> >> less error-prone.
> >> 
> >> The patches are based on top of the drm-misc-next branch and have been
> >> compile-tested only except for rcar-du-drm that has been tested on real
> >> hardware. Given the high risk of conflicts I would like to get them
> > > merged as soon as possible (after, of course, proper review and
> >> testing).
> > 
> > Makes sense, so I went ahead and pulled in patches 1-6. I think 7&8 need a
> > bit more soaking to gather acks, if only for aweareness. Plus a bit more
> > kernel-doc polish imo.
> 
> On the kernel-doc polish: Grepping for drm_crtc_helper_funcs\.enable and
> \.disable brought up a few references that need to be updated in your
> patches 7&8.

I had just noticed that too, and will fix it in v2 of 7/8 and 8/8. I'll wait 
for more acks before reposting.

> >> Laurent Pinchart (8):
> >>   drm: arcpgu: Remove CRTC .commit() helper operation
> >>   drm: arcpgu: Remove CRTC .prepare() helper operation
> >>   drm: qxl: Remove unused CRTC .dpms() helper operation
> >>   drm: qxl: Replace CRTC .commit() helper operation with .enable()
> >>   drm: vmwgfx: Remove unneeded CRTC .prepare() helper operation
> >>   drm: vmwgfx: Replace CRTC .commit() helper operation with .enable()
> >>   drm: Add old state pointer to CRTC .enable() helper function
> >>   drm: Convert atomic drivers from CRTC .disable() to .atomic_disable()
> >>  
> >>  drivers/gpu/drm/arc/arcpgu_crtc.c               | 12 ++++----
> >>  drivers/gpu/drm/arm/hdlcd_crtc.c                | 10 ++++---
> >>  drivers/gpu/drm/arm/malidp_crtc.c               | 10 ++++---
> >>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c  | 10 ++++---
> >>  drivers/gpu/drm/drm_atomic_helper.c             |  7 +++--
> >>  drivers/gpu/drm/drm_simple_kms_helper.c         | 10 ++++---
> >>  drivers/gpu/drm/exynos/exynos_drm_crtc.c        | 10 ++++---
> >>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c      |  5 ++--
> >>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c  | 10 ++++---
> >>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 10 ++++---
> >>  drivers/gpu/drm/imx/ipuv3-crtc.c                |  5 ++--
> >>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c         | 10 ++++---
> >>  drivers/gpu/drm/meson/meson_crtc.c              | 10 ++++---
> >>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c        | 10 ++++---
> >>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c        | 10 ++++---
> >>  drivers/gpu/drm/omapdrm/omap_crtc.c             | 10 ++++---
> >>  drivers/gpu/drm/qxl/qxl_display.c               | 15 ++++------
> >>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c          | 10 ++++---
> >>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c     | 10 ++++---
> >>  drivers/gpu/drm/sti/sti_crtc.c                  | 10 ++++---
> >>  drivers/gpu/drm/stm/ltdc.c                      | 10 ++++---
> >>  drivers/gpu/drm/sun4i/sun4i_crtc.c              | 10 ++++---
> >>  drivers/gpu/drm/tegra/dc.c                      | 10 ++++---
> >>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c            | 16 +++++++++--
> >>  drivers/gpu/drm/vc4/vc4_crtc.c                  | 10 ++++---
> >>  drivers/gpu/drm/virtio/virtgpu_display.c        | 10 ++++---
> >>  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c             | 27 ++++++------------
> >>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c            | 14 +++++----
> >>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c            | 10 ++++---
> >>  drivers/gpu/drm/zte/zx_vou.c                    | 10 ++++---
> >>  include/drm/drm_modeset_helper_vtables.h        | 38 +++++++++---------
> >>  31 files changed, 204 insertions(+), 155 deletions(-)

-- 
Regards,

Laurent Pinchart

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