From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752680AbdEEIgI (ORCPT ); Fri, 5 May 2017 04:36:08 -0400 Received: from hermes.aosc.io ([199.195.250.187]:46968 "EHLO hermes.aosc.io" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751362AbdEEIgG (ORCPT ); Fri, 5 May 2017 04:36:06 -0400 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Date: Fri, 05 May 2017 16:36:01 +0800 From: icenowy@aosc.io To: Chen-Yu Tsai Cc: devicetree , linux-sunxi , linux-kernel , dri-devel , Rob Herring , Maxime Ripard , linux-clk , linux-arm-kernel Subject: Re: [linux-sunxi] [PATCH v6 05/13] drm/sun4i: abstract a engine type In-Reply-To: References: <20170504114858.9008-1-icenowy@aosc.io> <20170504114858.9008-6-icenowy@aosc.io> Message-ID: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 在 2017-05-05 10:56，Chen-Yu Tsai 写道： > On Thu, May 4, 2017 at 7:48 PM, Icenowy Zheng 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 >> --- >> 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 >> #include >> >> +#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