Re: [linux-sunxi] [PATCH v6 05/13] drm/sun4i: abstract a engine type

[icenowy@xxxxxxx]

在 2017-05-05 10:56，Chen-Yu Tsai 写道：
> On Thu, May 4, 2017 at 7:48 PM, Icenowy Zheng <icenowy@xxxxxxx> wrote:
> > As we are going to add support for the Allwinner DE2 engine in 
> > sun4i-drm
> > driver, we will finally have two types of display engines -- the DE1
> > backend and the DE2 mixer. They both do some display blending and feed
> > graphics data to TCON, so I choose to call them both "engine" here.
>
> These engines composite different layers into a final image which is
> then sent out to the TCONs. As such, "compositor" would be an accurate
> name.
>
> However, "engine" is OK, since Allwinner calls this stuff Display 
> Engine
> 1.0 and 2.0. Hope there won't be a 3.0 ...
>
> Maybe you should note that in your commit message. That is justifies 
> the name.
>
> > Abstract the engine type to a new struct with an ops struct, which 
> > contains
> > functions that should be called outside the engine-specified code (in
> > TCON, CRTC or TV Encoder code).
> >
> > Signed-off-by: Icenowy Zheng <icenowy@xxxxxxx>
> > ---
> > Changes in v6:
> > - Rebased on wens's multi-pipeline patchset.
> > - Split out Makefile changes.
> > Changes in v5:
> > - Really made a sunxi_engine struct type, and moved ops pointer
> >   into it.
> > - Added checked ops wrappers.
> > - Changed the second parameter of layers_init from crtc to engine.
> > Changes in v4:
> > - Comments to tag the color correction functions as optional.
> > - Check before calling the optional functions.
> > - Change layers_init to satisfy new PATCH v4 04/11.
> >
> >  drivers/gpu/drm/sun4i/sun4i_backend.c |  68 ++++++++++++---------
> >  drivers/gpu/drm/sun4i/sun4i_backend.h |  17 +++---
> >  drivers/gpu/drm/sun4i/sun4i_crtc.c    |  11 ++--
> >  drivers/gpu/drm/sun4i/sun4i_crtc.h    |   4 +-
> >  drivers/gpu/drm/sun4i/sun4i_drv.c     |   2 +-
> >  drivers/gpu/drm/sun4i/sun4i_drv.h     |   2 +-
> >  drivers/gpu/drm/sun4i/sun4i_layer.c   |   8 +--
> >  drivers/gpu/drm/sun4i/sun4i_layer.h   |   5 +-
> >  drivers/gpu/drm/sun4i/sun4i_tcon.c    |  36 ++++++-----
> >  drivers/gpu/drm/sun4i/sun4i_tv.c      |   9 ++-
> >  drivers/gpu/drm/sun4i/sunxi_engine.h  | 112 
> > ++++++++++++++++++++++++++++++++++
> >  11 files changed, 198 insertions(+), 76 deletions(-)
> >  create mode 100644 drivers/gpu/drm/sun4i/sunxi_engine.h
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c 
> > b/drivers/gpu/drm/sun4i/sun4i_backend.c
> > index e53107418add..611cdcb9c182 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> > @@ -25,6 +25,8 @@
> >
> >  #include "sun4i_backend.h"
> >  #include "sun4i_drv.h"
> > +#include "sun4i_layer.h"
> > +#include "sunxi_engine.h"
> >
> >  static const u32 sunxi_rgb2yuv_coef[12] = {
> >         0x00000107, 0x00000204, 0x00000064, 0x00000108,
> > @@ -32,41 +34,38 @@ static const u32 sunxi_rgb2yuv_coef[12] = {
> >         0x000001c1, 0x00003e88, 0x00003fb8, 0x00000808
> >  };
> >
> > -void sun4i_backend_apply_color_correction(struct sun4i_backend 
> > *backend)
> > +static void sun4i_backend_apply_color_correction(struct sunxi_engine 
> > *engine)
> >  {
> >         int i;
> >
> >         DRM_DEBUG_DRIVER("Applying RGB to YUV color correction\n");
> >
> >         /* Set color correction */
> > -       regmap_write(backend->regs, SUN4I_BACKEND_OCCTL_REG,
> > +       regmap_write(engine->regs, SUN4I_BACKEND_OCCTL_REG,
> >                      SUN4I_BACKEND_OCCTL_ENABLE);
> >
> >         for (i = 0; i < 12; i++)
> > -               regmap_write(backend->regs, 
> > SUN4I_BACKEND_OCRCOEF_REG(i),
> > +               regmap_write(engine->regs, 
> > SUN4I_BACKEND_OCRCOEF_REG(i),
> >                              sunxi_rgb2yuv_coef[i]);
> >  }
> > -EXPORT_SYMBOL(sun4i_backend_apply_color_correction);
> >
> > -void sun4i_backend_disable_color_correction(struct sun4i_backend 
> > *backend)
> > +static void sun4i_backend_disable_color_correction(struct 
> > sunxi_engine *engine)
> >  {
> >         DRM_DEBUG_DRIVER("Disabling color correction\n");
> >
> >         /* Disable color correction */
> > -       regmap_update_bits(backend->regs, SUN4I_BACKEND_OCCTL_REG,
> > +       regmap_update_bits(engine->regs, SUN4I_BACKEND_OCCTL_REG,
> >                            SUN4I_BACKEND_OCCTL_ENABLE, 0);
> >  }
> > -EXPORT_SYMBOL(sun4i_backend_disable_color_correction);
> >
> > -void sun4i_backend_commit(struct sun4i_backend *backend)
> > +static void sun4i_backend_commit(struct sunxi_engine *engine)
> >  {
> >         DRM_DEBUG_DRIVER("Committing changes\n");
> >
> > -       regmap_write(backend->regs, SUN4I_BACKEND_REGBUFFCTL_REG,
> > +       regmap_write(engine->regs, SUN4I_BACKEND_REGBUFFCTL_REG,
> >                      SUN4I_BACKEND_REGBUFFCTL_AUTOLOAD_DIS |
> >                      SUN4I_BACKEND_REGBUFFCTL_LOADCTL);
> >  }
> > -EXPORT_SYMBOL(sun4i_backend_commit);
> >
> >  void sun4i_backend_layer_enable(struct sun4i_backend *backend,
> >                                 int layer, bool enable)
> > @@ -81,7 +80,7 @@ void sun4i_backend_layer_enable(struct sun4i_backend 
> > *backend,
> >         else
> >                 val = 0;
> >
> > -       regmap_update_bits(backend->regs, SUN4I_BACKEND_MODCTL_REG,
> > +       regmap_update_bits(backend->engine.regs, 
> > SUN4I_BACKEND_MODCTL_REG,
> >                            SUN4I_BACKEND_MODCTL_LAY_EN(layer), val);
> >  }
> >  EXPORT_SYMBOL(sun4i_backend_layer_enable);
> > @@ -144,27 +143,28 @@ int sun4i_backend_update_layer_coord(struct 
> > sun4i_backend *backend,
> >         if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
> >                 DRM_DEBUG_DRIVER("Primary layer, updating global size 
> > W: %u H: %u\n",
> >                                  state->crtc_w, state->crtc_h);
> > -               regmap_write(backend->regs, SUN4I_BACKEND_DISSIZE_REG,
> > +               regmap_write(backend->engine.regs, 
> > SUN4I_BACKEND_DISSIZE_REG,
> >                              SUN4I_BACKEND_DISSIZE(state->crtc_w,
> >                                                    state->crtc_h));
> >         }
> >
> >         /* Set the line width */
> >         DRM_DEBUG_DRIVER("Layer line width: %d bits\n", fb->pitches[0] 
> > * 8);
> > -       regmap_write(backend->regs, 
> > SUN4I_BACKEND_LAYLINEWIDTH_REG(layer),
> > +       regmap_write(backend->engine.regs,
> > +                    SUN4I_BACKEND_LAYLINEWIDTH_REG(layer),
> >                      fb->pitches[0] * 8);
> >
> >         /* Set height and width */
> >         DRM_DEBUG_DRIVER("Layer size W: %u H: %u\n",
> >                          state->crtc_w, state->crtc_h);
> > -       regmap_write(backend->regs, SUN4I_BACKEND_LAYSIZE_REG(layer),
> > +       regmap_write(backend->engine.regs, 
> > SUN4I_BACKEND_LAYSIZE_REG(layer),
> >                      SUN4I_BACKEND_LAYSIZE(state->crtc_w,
> >                                            state->crtc_h));
> >
> >         /* Set base coordinates */
> >         DRM_DEBUG_DRIVER("Layer coordinates X: %d Y: %d\n",
> >                          state->crtc_x, state->crtc_y);
> > -       regmap_write(backend->regs, SUN4I_BACKEND_LAYCOOR_REG(layer),
> > +       regmap_write(backend->engine.regs, 
> > SUN4I_BACKEND_LAYCOOR_REG(layer),
> >                      SUN4I_BACKEND_LAYCOOR(state->crtc_x,
> >                                            state->crtc_y));
> >
> > @@ -185,7 +185,7 @@ int sun4i_backend_update_layer_formats(struct 
> > sun4i_backend *backend,
> >                 interlaced = 
> > plane->state->crtc->state->adjusted_mode.flags
> >                         & DRM_MODE_FLAG_INTERLACE;
> >
> > -       regmap_update_bits(backend->regs, SUN4I_BACKEND_MODCTL_REG,
> > +       regmap_update_bits(backend->engine.regs, 
> > SUN4I_BACKEND_MODCTL_REG,
> >                            SUN4I_BACKEND_MODCTL_ITLMOD_EN,
> >                            interlaced ? SUN4I_BACKEND_MODCTL_ITLMOD_EN 
> > : 0);
> >
> > @@ -199,7 +199,8 @@ int sun4i_backend_update_layer_formats(struct 
> > sun4i_backend *backend,
> >                 return ret;
> >         }
> >
> > -       regmap_update_bits(backend->regs, 
> > SUN4I_BACKEND_ATTCTL_REG1(layer),
> > +       regmap_update_bits(backend->engine.regs,
> > +                          SUN4I_BACKEND_ATTCTL_REG1(layer),
> >                            SUN4I_BACKEND_ATTCTL_REG1_LAY_FBFMT, val);
> >
> >         return 0;
> > @@ -232,13 +233,14 @@ int sun4i_backend_update_layer_buffer(struct 
> > sun4i_backend *backend,
> >         /* Write the 32 lower bits of the address (in bits) */
> >         lo_paddr = paddr << 3;
> >         DRM_DEBUG_DRIVER("Setting address lower bits to 0x%x\n", 
> > lo_paddr);
> > -       regmap_write(backend->regs, 
> > SUN4I_BACKEND_LAYFB_L32ADD_REG(layer),
> > +       regmap_write(backend->engine.regs,
> > +                    SUN4I_BACKEND_LAYFB_L32ADD_REG(layer),
> >                      lo_paddr);
> >
> >         /* And the upper bits */
> >         hi_paddr = paddr >> 29;
> >         DRM_DEBUG_DRIVER("Setting address high bits to 0x%x\n", 
> > hi_paddr);
> > -       regmap_update_bits(backend->regs, 
> > SUN4I_BACKEND_LAYFB_H4ADD_REG,
> > +       regmap_update_bits(backend->engine.regs, 
> > SUN4I_BACKEND_LAYFB_H4ADD_REG,
> >                            SUN4I_BACKEND_LAYFB_H4ADD_MSK(layer),
> >                            SUN4I_BACKEND_LAYFB_H4ADD(layer, 
> > hi_paddr));
> >
> > @@ -330,6 +332,13 @@ static int sun4i_backend_of_get_id(struct 
> > device_node *node)
> >         return ret;
> >  }
> >
> > +static const struct sunxi_engine_ops sun4i_backend_engine_ops = {
> > +       .commit                         = sun4i_backend_commit,
> > +       .layers_init                    = sun4i_layers_init,
> > +       .apply_color_correction         = 
> > sun4i_backend_apply_color_correction,
> > +       .disable_color_correction       = 
> > sun4i_backend_disable_color_correction,
> > +};
> > +
> >  static struct regmap_config sun4i_backend_regmap_config = {
> >         .reg_bits       = 32,
> >         .val_bits       = 32,
> > @@ -353,7 +362,8 @@ static int sun4i_backend_bind(struct device *dev, 
> > struct device *master,
> >                 return -ENOMEM;
> >         dev_set_drvdata(dev, backend);
> >
> > -       backend->node = dev->of_node;
> > +       backend->engine.node = dev->of_node;
> > +       backend->engine.ops = &sun4i_backend_engine_ops;
> >         backend->id = sun4i_backend_of_get_id(dev->of_node);
> >         if (backend->id < 0)
> >                 return backend->id;
> > @@ -363,11 +373,11 @@ static int sun4i_backend_bind(struct device 
> > *dev, struct device *master,
> >         if (IS_ERR(regs))
> >                 return PTR_ERR(regs);
> >
> > -       backend->regs = devm_regmap_init_mmio(dev, regs,
> > -                                             
> > &sun4i_backend_regmap_config);
> > -       if (IS_ERR(backend->regs)) {
> > +       backend->engine.regs = devm_regmap_init_mmio(dev, regs,
> > +                                                    
> > &sun4i_backend_regmap_config);
> > +       if (IS_ERR(backend->engine.regs)) {
> >                 dev_err(dev, "Couldn't create the backend regmap\n");
> > -               return PTR_ERR(backend->regs);
> > +               return PTR_ERR(backend->engine.regs);
> >         }
> >
> >         backend->reset = devm_reset_control_get(dev, NULL);
> > @@ -415,18 +425,18 @@ static int sun4i_backend_bind(struct device 
> > *dev, struct device *master,
> >                 }
> >         }
> >
> > -       list_add_tail(&backend->list, &drv->backend_list);
> > +       list_add_tail(&backend->engine.list, &drv->engine_list);
> >
> >         /* Reset the registers */
> >         for (i = 0x800; i < 0x1000; i += 4)
> > -               regmap_write(backend->regs, i, 0);
> > +               regmap_write(backend->engine.regs, i, 0);
> >
> >         /* Disable registers autoloading */
> > -       regmap_write(backend->regs, SUN4I_BACKEND_REGBUFFCTL_REG,
> > +       regmap_write(backend->engine.regs, 
> > SUN4I_BACKEND_REGBUFFCTL_REG,
> >                      SUN4I_BACKEND_REGBUFFCTL_AUTOLOAD_DIS);
> >
> >         /* Enable the backend */
> > -       regmap_write(backend->regs, SUN4I_BACKEND_MODCTL_REG,
> > +       regmap_write(backend->engine.regs, SUN4I_BACKEND_MODCTL_REG,
> >                      SUN4I_BACKEND_MODCTL_DEBE_EN |
> >                      SUN4I_BACKEND_MODCTL_START_CTL);
> >
> > @@ -448,7 +458,7 @@ static void sun4i_backend_unbind(struct device 
> > *dev, struct device *master,
> >  {
> >         struct sun4i_backend *backend = dev_get_drvdata(dev);
> >
> > -       list_del(&backend->list);
> > +       list_del(&backend->engine.list);
> >
> >         if (of_device_is_compatible(dev->of_node,
> >                                     
> > "allwinner,sun8i-a33-display-backend"))
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h 
> > b/drivers/gpu/drm/sun4i/sun4i_backend.h
> > index 6327a2985fe6..b022a37e8e5b 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_backend.h
> > +++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
> > @@ -19,6 +19,8 @@
> >  #include <linux/regmap.h>
> >  #include <linux/reset.h>
> >
> > +#include "sunxi_engine.h"
> > +
> >  #define SUN4I_BACKEND_MODCTL_REG               0x800
> >  #define SUN4I_BACKEND_MODCTL_LINE_SEL                  BIT(29)
> >  #define SUN4I_BACKEND_MODCTL_ITLMOD_EN                 BIT(28)
> > @@ -141,8 +143,7 @@
> >  #define SUN4I_BACKEND_PIPE_OFF(p)              (0x5000 + (0x400 * 
> > (p)))
> >
> >  struct sun4i_backend {
> > -       struct device_node      *node;
> > -       struct regmap           *regs;
> > +       struct sunxi_engine     engine;
> >
> >         struct reset_control    *reset;
> >
> > @@ -154,15 +155,13 @@ struct sun4i_backend {
> >         struct reset_control    *sat_reset;
> >
> >         int                     id;
> > -
> > -       /* Backend list management */
> > -       struct list_head        list;
> >  };
> >
> > -void sun4i_backend_apply_color_correction(struct sun4i_backend 
> > *backend);
> > -void sun4i_backend_disable_color_correction(struct sun4i_backend 
> > *backend);
> > -
> > -void sun4i_backend_commit(struct sun4i_backend *backend);
> > +static inline struct sun4i_backend *
> > +engine_to_sun4i_backend(struct sunxi_engine *engine)
> > +{
> > +       return container_of(engine, struct sun4i_backend, engine);
> > +}
> >
> >  void sun4i_backend_layer_enable(struct sun4i_backend *backend,
> >                                 int layer, bool enable);
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_crtc.c 
> > b/drivers/gpu/drm/sun4i/sun4i_crtc.c
> > index 708b3543d4e9..f8c70439d1e2 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_crtc.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_crtc.c
> > @@ -25,10 +25,9 @@
> >
> >  #include <video/videomode.h>
> >
> > -#include "sun4i_backend.h"
> >  #include "sun4i_crtc.h"
> >  #include "sun4i_drv.h"
> > -#include "sun4i_layer.h"
> > +#include "sunxi_engine.h"
> >  #include "sun4i_tcon.h"
> >
> >  static void sun4i_crtc_atomic_begin(struct drm_crtc *crtc,
> > @@ -56,7 +55,7 @@ static void sun4i_crtc_atomic_flush(struct drm_crtc 
> > *crtc,
> >
> >         DRM_DEBUG_DRIVER("Committing plane changes\n");
> >
> > -       sun4i_backend_commit(scrtc->backend);
> > +       sunxi_engine_commit(scrtc->engine);
> >
> >         if (event) {
> >                 crtc->state->event = NULL;
> > @@ -135,7 +134,7 @@ static const struct drm_crtc_funcs 
> > sun4i_crtc_funcs = {
> >  };
> >
> >  struct sun4i_crtc *sun4i_crtc_init(struct drm_device *drm,
> > -                                  struct sun4i_backend *backend,
> > +                                  struct sunxi_engine *engine,
> >                                    struct sun4i_tcon *tcon)
> >  {
> >         struct sun4i_crtc *scrtc;
> > @@ -146,11 +145,11 @@ struct sun4i_crtc *sun4i_crtc_init(struct 
> > drm_device *drm,
> >         scrtc = devm_kzalloc(drm->dev, sizeof(*scrtc), GFP_KERNEL);
> >         if (!scrtc)
> >                 return ERR_PTR(-ENOMEM);
> > -       scrtc->backend = backend;
> > +       scrtc->engine = engine;
> >         scrtc->tcon = tcon;
> >
> >         /* Create our layers */
> > -       planes = sun4i_layers_init(drm, scrtc);
> > +       planes = sunxi_engine_layers_init(drm, engine);
> >         if (IS_ERR(planes)) {
> >                 dev_err(drm->dev, "Couldn't create the planes\n");
> >                 return NULL;
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_crtc.h 
> > b/drivers/gpu/drm/sun4i/sun4i_crtc.h
> > index 4dae3508424a..bf0ce36eb518 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_crtc.h
> > +++ b/drivers/gpu/drm/sun4i/sun4i_crtc.h
> > @@ -17,7 +17,7 @@ struct sun4i_crtc {
> >         struct drm_crtc                 crtc;
> >         struct drm_pending_vblank_event *event;
> >
> > -       struct sun4i_backend            *backend;
> > +       struct sunxi_engine             *engine;
> >         struct sun4i_tcon               *tcon;
> >  };
> >
> > @@ -27,7 +27,7 @@ static inline struct sun4i_crtc 
> > *drm_crtc_to_sun4i_crtc(struct drm_crtc *crtc)
> >  }
> >
> >  struct sun4i_crtc *sun4i_crtc_init(struct drm_device *drm,
> > -                                  struct sun4i_backend *backend,
> > +                                  struct sunxi_engine *engine,
> >                                    struct sun4i_tcon *tcon);
> >
> >  #endif /* _SUN4I_CRTC_H_ */
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c 
> > b/drivers/gpu/drm/sun4i/sun4i_drv.c
> > index 89c51fd6e9af..12ede8682b5c 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> > @@ -101,7 +101,7 @@ static int sun4i_drv_bind(struct device *dev)
> >                 goto free_drm;
> >         }
> >         drm->dev_private = drv;
> > -       INIT_LIST_HEAD(&drv->backend_list);
> > +       INIT_LIST_HEAD(&drv->engine_list);
> >         INIT_LIST_HEAD(&drv->tcon_list);
> >
> >         ret = of_reserved_mem_device_init(dev);
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.h 
> > b/drivers/gpu/drm/sun4i/sun4i_drv.h
> > index 250c29017ef5..a960c89270cc 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_drv.h
> > +++ b/drivers/gpu/drm/sun4i/sun4i_drv.h
> > @@ -18,7 +18,7 @@
> >  #include <linux/regmap.h>
> >
> >  struct sun4i_drv {
> > -       struct list_head        backend_list;
> > +       struct list_head        engine_list;
> >         struct list_head        tcon_list;
> >
> >         struct drm_fbdev_cma    *fbdev;
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c 
> > b/drivers/gpu/drm/sun4i/sun4i_layer.c
> > index e1f03e1cc0ac..ab33e4d06782 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_layer.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
> > @@ -11,12 +11,10 @@
> >   */
> >
> >  #include <drm/drm_atomic_helper.h>
> > -#include <drm/drm_crtc.h>
> >  #include <drm/drm_plane_helper.h>
> >  #include <drm/drmP.h>
> >
> >  #include "sun4i_backend.h"
> > -#include "sun4i_crtc.h"
> >  #include "sun4i_layer.h"
>
> You should also include sun4i_engine.h directly.
>
> >  struct sun4i_plane_desc {
> > @@ -130,10 +128,10 @@ static struct sun4i_layer 
> > *sun4i_layer_init_one(struct drm_device *drm,
> >  }
> >
> >  struct drm_plane **sun4i_layers_init(struct drm_device *drm,
> > -                                    struct sun4i_crtc *crtc)
> > +                                    struct sunxi_engine *engine)
> >  {
> >         struct drm_plane **planes;
> > -       struct sun4i_backend *backend = crtc->backend;
> > +       struct sun4i_backend *backend = 
> > engine_to_sun4i_backend(engine);
> >         int i;
> >
> >         planes = devm_kcalloc(drm->dev, 
> > ARRAY_SIZE(sun4i_backend_planes) + 1,
> > @@ -175,7 +173,7 @@ struct drm_plane **sun4i_layers_init(struct 
> > drm_device *drm,
> >
> >                 DRM_DEBUG_DRIVER("Assigning %s plane to pipe %d\n",
> >                                  i ? "overlay" : "primary", 
> > plane->pipe);
> > -               regmap_update_bits(backend->regs, 
> > SUN4I_BACKEND_ATTCTL_REG0(i),
> > +               regmap_update_bits(engine->regs, 
> > SUN4I_BACKEND_ATTCTL_REG0(i),
> >                                    
> > SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL_MASK,
> >                                    
> > SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL(plane->pipe));
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.h 
> > b/drivers/gpu/drm/sun4i/sun4i_layer.h
> > index 5ea5c994d6ea..004b7cfe8ffb 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_layer.h
> > +++ b/drivers/gpu/drm/sun4i/sun4i_layer.h
> > @@ -13,6 +13,8 @@
> >  #ifndef _SUN4I_LAYER_H_
> >  #define _SUN4I_LAYER_H_
> >
> > +struct sunxi_engine;
> > +
> >  struct sun4i_layer {
> >         struct drm_plane        plane;
> >         struct sun4i_drv        *drv;
> > @@ -27,6 +29,5 @@ plane_to_sun4i_layer(struct drm_plane *plane)
> >  }
> >
> >  struct drm_plane **sun4i_layers_init(struct drm_device *drm,
> > -                                    struct sun4i_crtc *crtc);
> > -
> > +                                    struct sunxi_engine *engine);
>
> Please keep the newline.
>
> >  #endif /* _SUN4I_LAYER_H_ */
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c 
> > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > index 29fd829aa54c..c48135a10fda 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > @@ -26,12 +26,12 @@
> >  #include <linux/regmap.h>
> >  #include <linux/reset.h>
> >
> > -#include "sun4i_backend.h"
> >  #include "sun4i_crtc.h"
> >  #include "sun4i_dotclock.h"
> >  #include "sun4i_drv.h"
> >  #include "sun4i_rgb.h"
> >  #include "sun4i_tcon.h"
> > +#include "sunxi_engine.h"
>
> Please keep the headers in alphabetical order.

sunxi is of course after sun4i.

> >  void sun4i_tcon_disable(struct sun4i_tcon *tcon)
> >  {
> > @@ -488,12 +488,16 @@ struct drm_bridge *sun4i_tcon_find_bridge(struct 
> > device_node *node)
> >   * means maintaining a large list of them. Or, since the backend is
> >   * registered and binded before the TCON, we can just go through the
> >   * list of registered backends and compare the device node.
> > + *
> > + * As the structures now store engines instead of backends, here this
> > + * function in fact searches the corresponding engine, and the ID is
> > + * requested via the get_id function of the engine.
> >   */
> > -static struct sun4i_backend *sun4i_tcon_find_backend(struct sun4i_drv 
> > *drv,
> > +static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv 
> > *drv,
> >                                                      struct 
> > device_node *node)
> >  {
> >         struct device_node *port, *ep, *remote;
> > -       struct sun4i_backend *backend;
> > +       struct sunxi_engine *engine;
> >
> >         port = of_graph_get_port_by_id(node, 0);
> >         if (!port)
> > @@ -504,21 +508,21 @@ static struct sun4i_backend 
> > *sun4i_tcon_find_backend(struct sun4i_drv *drv,
> >                 if (!remote)
> >                         continue;
> >
> > -               /* does this node match any registered backends? */
> > -               list_for_each_entry(backend, &drv->backend_list, list) 
> > {
> > -                       if (remote == backend->node) {
> > +               /* does this node match any registered engines? */
> > +               list_for_each_entry(engine, &drv->engine_list, list) {
> > +                       if (remote == engine->node) {
> >                                 of_node_put(remote);
> >                                 of_node_put(port);
> > -                               return backend;
> > +                               return engine;
> >                         }
> >                 }
> >
> >                 /* keep looking through upstream ports */
> > -               backend = sun4i_tcon_find_backend(drv, remote);
> > -               if (!IS_ERR(backend)) {
> > +               engine = sun4i_tcon_find_engine(drv, remote);
> > +               if (!IS_ERR(engine)) {
> >                         of_node_put(remote);
> >                         of_node_put(port);
> > -                       return backend;
> > +                       return engine;
> >                 }
> >         }
> >
> > @@ -530,13 +534,13 @@ static int sun4i_tcon_bind(struct device *dev, 
> > struct device *master,
> >  {
> >         struct drm_device *drm = data;
> >         struct sun4i_drv *drv = drm->dev_private;
> > -       struct sun4i_backend *backend;
> > +       struct sunxi_engine *engine;
> >         struct sun4i_tcon *tcon;
> >         int ret;
> >
> > -       backend = sun4i_tcon_find_backend(drv, dev->of_node);
> > -       if (IS_ERR(backend)) {
> > -               dev_err(dev, "Couldn't find matching backend\n");
> > +       engine = sun4i_tcon_find_engine(drv, dev->of_node);
> > +       if (IS_ERR(engine)) {
> > +               dev_err(dev, "Couldn't find matching engine\n");
> >                 return -EPROBE_DEFER;
> >         }
> >
> > @@ -546,7 +550,7 @@ static int sun4i_tcon_bind(struct device *dev, 
> > struct device *master,
> >         dev_set_drvdata(dev, tcon);
> >         tcon->drm = drm;
> >         tcon->dev = dev;
> > -       tcon->id = backend->id;
> > +       tcon->id = sunxi_engine_get_id(engine);
> >         tcon->quirks = of_device_get_match_data(dev);
> >
> >         tcon->lcd_rst = devm_reset_control_get(dev, "lcd");
> > @@ -589,7 +593,7 @@ static int sun4i_tcon_bind(struct device *dev, 
> > struct device *master,
> >                 goto err_free_dotclock;
> >         }
> >
> > -       tcon->crtc = sun4i_crtc_init(drm, backend, tcon);
> > +       tcon->crtc = sun4i_crtc_init(drm, engine, tcon);
> >         if (IS_ERR(tcon->crtc)) {
> >                 dev_err(dev, "Couldn't create our CRTC\n");
> >                 ret = PTR_ERR(tcon->crtc);
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c 
> > b/drivers/gpu/drm/sun4i/sun4i_tv.c
> > index 542da220818b..a9cad00d4ee8 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_tv.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_tv.c
> > @@ -22,10 +22,10 @@
> >  #include <drm/drm_of.h>
> >  #include <drm/drm_panel.h>
> >
> > -#include "sun4i_backend.h"
> >  #include "sun4i_crtc.h"
> >  #include "sun4i_drv.h"
> >  #include "sun4i_tcon.h"
> > +#include "sunxi_engine.h"
> >
> >  #define SUN4I_TVE_EN_REG               0x000
> >  #define SUN4I_TVE_EN_DAC_MAP_MASK              GENMASK(19, 4)
> > @@ -353,7 +353,6 @@ static void sun4i_tv_disable(struct drm_encoder 
> > *encoder)
> >         struct sun4i_tv *tv = drm_encoder_to_sun4i_tv(encoder);
> >         struct sun4i_crtc *crtc = 
> > drm_crtc_to_sun4i_crtc(encoder->crtc);
> >         struct sun4i_tcon *tcon = crtc->tcon;
> > -       struct sun4i_backend *backend = crtc->backend;
> >
> >         DRM_DEBUG_DRIVER("Disabling the TV Output\n");
> >
> > @@ -362,7 +361,8 @@ static void sun4i_tv_disable(struct drm_encoder 
> > *encoder)
> >         regmap_update_bits(tv->regs, SUN4I_TVE_EN_REG,
> >                            SUN4I_TVE_EN_ENABLE,
> >                            0);
> > -       sun4i_backend_disable_color_correction(backend);
> > +
> > +       sunxi_engine_disable_color_correction(crtc->engine);
> >  }
> >
> >  static void sun4i_tv_enable(struct drm_encoder *encoder)
> > @@ -370,11 +370,10 @@ static void sun4i_tv_enable(struct drm_encoder 
> > *encoder)
> >         struct sun4i_tv *tv = drm_encoder_to_sun4i_tv(encoder);
> >         struct sun4i_crtc *crtc = 
> > drm_crtc_to_sun4i_crtc(encoder->crtc);
> >         struct sun4i_tcon *tcon = crtc->tcon;
> > -       struct sun4i_backend *backend = crtc->backend;
> >
> >         DRM_DEBUG_DRIVER("Enabling the TV Output\n");
> >
> > -       sun4i_backend_apply_color_correction(backend);
> > +       sunxi_engine_apply_color_correction(crtc->engine);
> >
> >         regmap_update_bits(tv->regs, SUN4I_TVE_EN_REG,
> >                            SUN4I_TVE_EN_ENABLE,
> > diff --git a/drivers/gpu/drm/sun4i/sunxi_engine.h 
> > b/drivers/gpu/drm/sun4i/sunxi_engine.h
> > new file mode 100644
> > index 000000000000..b3c6e6148568
> > --- /dev/null
> > +++ b/drivers/gpu/drm/sun4i/sunxi_engine.h
> > @@ -0,0 +1,112 @@
> > +/*
> > + * Copyright (C) 2017 Icenowy Zheng <icenowy@xxxxxxx>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation; either version 2 of
> > + * the License, or (at your option) any later version.
> > + */
> > +
> > +#ifndef _SUNXI_ENGINE_H_
> > +#define _SUNXI_ENGINE_H_
> > +
> > +struct sun4i_crtc;
>
> This is not used. Please remove it.
>
>
> The rest looks good. Thanks for working this out. Once the minor 
> comments
> are fixed, please add my
>
> Reviewed-by: Chen-Yu Tsai <wens@xxxxxxxx>
>
> > +struct drm_plane;
> > +struct drm_device;
> > +
> > +struct sunxi_engine;
> > +
> > +struct sunxi_engine_ops {
> > +       void (*commit)(struct sunxi_engine *engine);
> > +       struct drm_plane **(*layers_init)(struct drm_device *drm,
> > +                                         struct sunxi_engine 
> > *engine);
> > +
> > +       void (*apply_color_correction)(struct sunxi_engine *engine);
> > +       void (*disable_color_correction)(struct sunxi_engine *engine);
> > +       int (*get_id)(struct sunxi_engine *engine);
> > +};
> > +
> > +/**
> > + * struct sunxi_engine - the common parts of an engine for sun4i-drm 
> > driver
> > + * @ops:       the operations of the engine
> > + * @regs:      the regmap of the engine
> > + */
> > +struct sunxi_engine {
> > +       const struct sunxi_engine_ops   *ops;
> > +
> > +       struct device_node              *node;
> > +       struct regmap                   *regs;
> > +
> > +       /* Engine list management */
> > +       struct list_head                list;
> > +};
> > +
> > +/**
> > + * sunxi_engine_commit() - commit all changes of the engine
> > + * @engine:    pointer to the engine
> > + */
> > +static inline void
> > +sunxi_engine_commit(struct sunxi_engine *engine)
> > +{
> > +       if (engine->ops && engine->ops->commit)
> > +               engine->ops->commit(engine);
> > +}
> > +
> > +/**
> > + * sunxi_engine_layers_init() - Create planes (layers) for the engine
> > + * @drm:       pointer to the drm_device for which planes will be 
> > created
> > + * @engine:    pointer to the engine
> > + */
> > +static inline struct drm_plane **
> > +sunxi_engine_layers_init(struct drm_device *drm, struct sunxi_engine 
> > *engine)
> > +{
> > +       if (engine->ops && engine->ops->layers_init)
> > +               return engine->ops->layers_init(drm, engine);
> > +       return ERR_PTR(-ENOSYS);
> > +}
> > +
> > +/**
> > + * sunxi_engine_apply_color_correction - Apply the RGB2YUV color 
> > correction
> > + * @engine:    pointer to the engine
> > + *
> > + * This functionality is optional for an engine, however, if the 
> > engine is
> > + * intended to be used with TV Encoder, the output will be incorrect
> > + * without the color correction, due to TV Encoder expects the engine 
> > to
> > + * output directly YUV signal.
> > + */
> > +static inline void
> > +sunxi_engine_apply_color_correction(struct sunxi_engine *engine)
> > +{
> > +       if (engine->ops && engine->ops->apply_color_correction)
> > +               engine->ops->apply_color_correction(engine);
> > +}
> > +
> > +/**
> > + * sunxi_engine_disable_color_correction - Disable the color space 
> > correction
> > + * @engine:    pointer to the engine
> > + *
> > + * This function is paired with apply_color_correction().
> > + */
> > +static inline void
> > +sunxi_engine_disable_color_correction(struct sunxi_engine *engine)
> > +{
> > +       if (engine->ops && engine->ops->disable_color_correction)
> > +               engine->ops->disable_color_correction(engine);
> > +}
> > +
> > +/**
> > + * sunxi_engine_get_id - Get the ID of the engine.
> > + * @engine:    pointer to the engine
> > + *
> > + * If the ID is not necessary, just do not implement it in 
> > sunxi_engine_ops,
> > + * and a default -1 will be returned.
> > + */
> > +static inline int
> > +sunxi_engine_get_id(struct sunxi_engine *engine)
> > +{
> > +       if (engine->ops && engine->ops->get_id)
> > +               return engine->ops->get_id(engine);
> > +
> > +       return -1;
> > +}
> > +#endif /* _SUNXI_ENGINE_H_ */
> > --
> > 2.12.2
> >
> > --
> > You received this message because you are subscribed to the Google 
> > Groups "linux-sunxi" group.
> > To unsubscribe from this group and stop receiving emails from it, send 
> > an email to linux-sunxi+unsubscribe@xxxxxxxxxxxxxxxx.
> > For more options, visit https://groups.google.com/d/optout.
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html