lkml.org 
[lkml]   [2021]   [Jan]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 09/11] drm/atomic: Pass the full state to planes atomic disable and update
    Hi Maxime,

    Thank you for the patch.

    On Thu, Jan 21, 2021 at 05:35:34PM +0100, Maxime Ripard wrote:
    > The current atomic helpers have either their object state being passed as
    > an argument or the full atomic state.
    >
    > The former is the pattern that was done at first, before switching to the
    > latter for new hooks or when it was needed.
    >
    > Let's convert the remaining helpers to provide a consistent interface,
    > this time with the planes atomic_update and atomic_disable.
    >
    > The conversion was done using the coccinelle script below, built tested on
    > all the drivers.
    >
    > @@
    > identifier plane, plane_state;
    > symbol state;
    > @@
    >
    > struct drm_plane_helper_funcs {
    > ...
    > void (*atomic_update)(struct drm_plane *plane,
    > - struct drm_plane_state *plane_state);
    > + struct drm_atomic_state *state);
    > ...
    > }
    >
    > @@
    > identifier plane, plane_state;
    > symbol state;
    > @@
    >
    > struct drm_plane_helper_funcs {
    > ...
    > void (*atomic_disable)(struct drm_plane *plane,
    > - struct drm_plane_state *plane_state);
    > + struct drm_atomic_state *state);
    > ...
    > }
    >
    > @ plane_atomic_func @
    > identifier helpers;
    > identifier func;
    > @@
    >
    > (
    > static const struct drm_plane_helper_funcs helpers = {
    > ...,
    > .atomic_update = func,
    > ...,
    > };
    > |
    > static const struct drm_plane_helper_funcs helpers = {
    > ...,
    > .atomic_disable = func,
    > ...,
    > };
    > )
    >
    > @@
    > struct drm_plane_helper_funcs *FUNCS;
    > identifier f;
    > identifier crtc_state;
    > identifier plane, plane_state, state;
    > expression e;
    > @@
    >
    > f(struct drm_crtc_state *crtc_state)
    > {
    > ...
    > struct drm_atomic_state *state = e;
    > <+...
    > (
    > - FUNCS->atomic_disable(plane, plane_state)
    > + FUNCS->atomic_disable(plane, state)
    > |
    > - FUNCS->atomic_update(plane, plane_state)
    > + FUNCS->atomic_update(plane, state)
    > )
    > ...+>
    > }
    >
    > @@
    > identifier plane_atomic_func.func;
    > identifier plane;
    > symbol state;
    > @@
    >
    > func(struct drm_plane *plane,
    > - struct drm_plane_state *state)
    > + struct drm_plane_state *old_plane_state)
    > {
    > <...
    > - state
    > + old_plane_state
    > ...>
    > }
    >
    > @ ignores_old_state @
    > identifier plane_atomic_func.func;
    > identifier plane, old_state;
    > @@
    >
    > func(struct drm_plane *plane, struct drm_plane_state *old_state)
    > {
    > ... when != old_state
    > }
    >
    > @ adds_old_state depends on plane_atomic_func && !ignores_old_state @
    > identifier plane_atomic_func.func;
    > identifier plane, plane_state;
    > @@
    >
    > func(struct drm_plane *plane, struct drm_plane_state *plane_state)
    > {
    > + struct drm_plane_state *plane_state = drm_atomic_get_old_plane_state(state, plane);
    > ...
    > }
    >
    > @ depends on plane_atomic_func @
    > identifier plane_atomic_func.func;
    > identifier plane, plane_state;
    > @@
    >
    > func(struct drm_plane *plane,
    > - struct drm_plane_state *plane_state
    > + struct drm_atomic_state *state
    > )
    > { ... }
    >
    > @ include depends on adds_old_state @
    > @@
    >
    > #include <drm/drm_atomic.h>
    >
    > @ no_include depends on !include && adds_old_state @
    > @@
    >
    > + #include <drm/drm_atomic.h>
    > #include <drm/...>
    >
    > @@
    > identifier plane_atomic_func.func;
    > identifier plane, state;
    > identifier plane_state;
    > @@
    >
    > func(struct drm_plane *plane, struct drm_atomic_state *state) {
    > ...
    > struct drm_plane_state *plane_state = drm_atomic_get_old_plane_state(state, plane);
    > <+...
    > - plane_state->state
    > + state
    > ...+>
    > }
    >
    > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
    >
    > ---
    >
    > Changes from v1:
    > - Reintroduce the old_plane_state check in zynqmp_disp_crtc_atomic_disable
    > ---
    > drivers/gpu/drm/drm_atomic_helper.c | 8 ++++----
    > drivers/gpu/drm/drm_simple_kms_helper.c | 4 +++-
    > drivers/gpu/drm/mxsfb/mxsfb_kms.c | 6 ++++--
    > drivers/gpu/drm/omapdrm/omap_plane.c | 4 ++--
    > drivers/gpu/drm/rcar-du/rcar_du_plane.c | 4 +++-
    > drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 4 +++-
    > drivers/gpu/drm/tidss/tidss_plane.c | 4 ++--
    > drivers/gpu/drm/tilcdc/tilcdc_plane.c | 2 +-
    > drivers/gpu/drm/xlnx/zynqmp_disp.c | 15 ++++++++-------
    > include/drm/drm_modeset_helper_vtables.h | 4 ++--

    For these,

    Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

    > 58 files changed, 211 insertions(+), 123 deletions(-)

    --
    Regards,

    Laurent Pinchart

    \
     
     \ /
      Last update: 2021-01-22 00:26    [W:9.153 / U:0.072 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site