Plane transitional helper prototypes mismatch their methods

[Russell King - ARM Linux]

Ping.

On Mon, Jun 25, 2018 at 04:52:16PM +0100, Russell King - ARM Linux wrote:
> Hi,
> 
> Currently, the transitional plane helpers have prototypes that
> do not have the drm_modeset_acquire_ctx argument:
> 
> int drm_plane_helper_update(struct drm_plane *plane, struct drm_crtc *crtc,
>                             struct drm_framebuffer *fb,
>                             int crtc_x, int crtc_y,
>                             unsigned int crtc_w, unsigned int crtc_h,
>                             uint32_t src_x, uint32_t src_y,
>                             uint32_t src_w, uint32_t src_h);
> int drm_plane_helper_disable(struct drm_plane *plane);
> 
> However, the helper methods have this as the last argument:
> 
>         int (*update_plane)(struct drm_plane *plane,
>                             struct drm_crtc *crtc, struct drm_framebuffer *fb,
>                             int crtc_x, int crtc_y,
>                             unsigned int crtc_w, unsigned int crtc_h,
>                             uint32_t src_x, uint32_t src_y,
>                             uint32_t src_w, uint32_t src_h,
>                             struct drm_modeset_acquire_ctx *ctx);
>         int (*disable_plane)(struct drm_plane *plane,
>                              struct drm_modeset_acquire_ctx *ctx);
> 
> Are these transitional helpers supposed to be plugged into these
> methods directly?  Looking back in the history, the following commit
> to i915 seems to suggest that they were designed to be plugged
> directly into these methods:
> 
> commit ea2c67bb4affa84080c616920f3899f123786e56
> Author: Matt Roper <matthew.d.roper at intel.com>
> Date:   Tue Dec 23 10:41:52 2014 -0800
> 
>     drm/i915: Move to atomic plane helpers (v9)
> 
> It seems easy to add this argument to drm_plane_helper_update() as
> it has no current users, but the drm_plane_helper_disable()
> transitional helper seems to be used extensively by what are
> otherwise fully atomic modeset drivers, despite being described as
> only a transitional helper.
> 
> drivers/gpu/drm/arm/hdlcd_crtc.c:   drm_plane_helper_disable(plane);
> drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c: drm_plane_helper_disable(plane);
> drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c: drm_plane_helper_disable(plane);
> drivers/gpu/drm/arc/arcpgu_crtc.c:  drm_plane_helper_disable(plane);
> drivers/gpu/drm/sti/sti_hqvdp.c:    drm_plane_helper_disable(drm_plane);
> drivers/gpu/drm/sti/sti_gdp.c:      drm_plane_helper_disable(drm_plane);
> drivers/gpu/drm/sti/sti_cursor.c:   drm_plane_helper_disable(drm_plane);
> drivers/gpu/drm/vc4/vc4_plane.c:    drm_plane_helper_disable(plane);
> drivers/gpu/drm/zte/zx_plane.c:     drm_plane_helper_disable(plane);
> 
> Are these drivers buggy using the transitional helper, which won't do
> anything but change the state of that single plane, leaving the CRTC,
> encoders, bridges, etc all active?
> 
> -- 
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
> According to speedtest.net: 8.21Mbps down 510kbps up

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up