Re: [RFC 1/5] drm/exynos: mixer: refactor layer setup

Hi Tobias,

2015-04-30 Tobias Jakobi tjakobi@math.uni-bielefeld.de:

First step in allowing a more generic way to setup complex blending for the different layers.

Signed-off-by: Tobias Jakobi tjakobi@math.uni-bielefeld.de

drivers/gpu/drm/exynos/exynos_mixer.c | 74 +++++++++++++++++++++++++++++------ 1 file changed, 63 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 4155f43..a06b8e2 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -63,6 +63,12 @@ struct mixer_resources { struct clk *mout_mixer; };

+struct layer_config {

• unsigned int index;
• unsigned int priority;
• u32 cfg;

+};

I don't see why you are creating this struct, index and priority are never used in this patch series.

enum mixer_version_id { MXR_VER_0_0_0_16, MXR_VER_16_0_33_0, @@ -75,6 +81,8 @@ struct mixer_context { struct drm_device *drm_dev; struct exynos_drm_crtc *crtc; struct exynos_drm_plane planes[MIXER_WIN_NR];

• const struct layer_config *layer_config;
• unsigned int num_layer; int pipe; bool interlace; bool powered;

@@ -95,6 +103,40 @@ struct mixer_drv_data { bool has_sclk; };

+/*

• • The default layer priorities. A higher priority means that
• • the layer is at the top of layer stack.
• • The current configuration assumes the following usage scenario:
• • layer1: OSD [top]
• • layer0: main framebuffer
• • video layer: video overlay [bottom]
• • Note that the video layer is only usable when the
• • video processor is available.
• */

+static const struct layer_config default_layer_config[] = {

• {

• .index = 0, .priority = 1, /* layer0 */
  

• .cfg = MXR_LAYER_CFG_GRP0_VAL(1)
  

• }, {

• .index = 1, .priority = 2, /* layer1 */
  

• .cfg = MXR_LAYER_CFG_GRP1_VAL(2)
  

• }

+};

+static const struct layer_config vp_layer_config[] = {

• {

• .index = 2, .priority = 1, /* video layer */
  

• .cfg = MXR_LAYER_CFG_VP_VAL(1)
  

• }, {

• .index = 0, .priority = 2, /* layer0 */
  

• .cfg = MXR_LAYER_CFG_GRP0_VAL(2)
  

• }, {

• .index = 1, .priority = 3, /* layer1 */
  

• .cfg = MXR_LAYER_CFG_GRP1_VAL(3)
  

• }

+};

static const u8 filter_y_horiz_tap8[] = { 0, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 0, 0, 0, @@ -253,6 +295,17 @@ static void vp_default_filter(struct mixer_resources *res) filter_cr_horiz_tap4, sizeof(filter_cr_horiz_tap4)); }

+static void mixer_layer_priority(struct mixer_context *ctx) +{

• u32 val = 0;
• unsigned int i;
• for (i = 0; i < ctx->num_layer; ++i)

• val |= ctx->layer_config[i].cfg;
  

• mixer_reg_write(&ctx->mixer_res, MXR_LAYER_CFG, val);

+}

static void mixer_vsync_set_update(struct mixer_context *ctx, bool enable) { struct mixer_resources *res = &ctx->mixer_res; @@ -655,17 +708,7 @@ static void mixer_win_reset(struct mixer_context *ctx) mixer_reg_writemask(res, MXR_STATUS, MXR_STATUS_16_BURST, MXR_STATUS_BURST_MASK);

• /* setting default layer priority: layer1 > layer0 > video

• * because typical usage scenario would be
  

• * layer1 - OSD
  

• * layer0 - framebuffer
  

• * video - video overlay
  

• */
  

• val = MXR_LAYER_CFG_GRP1_VAL(3);
• val |= MXR_LAYER_CFG_GRP0_VAL(2);
• if (ctx->vp_enabled)

• val |= MXR_LAYER_CFG_VP_VAL(1);
  

• mixer_reg_write(res, MXR_LAYER_CFG, val);

I would move this exaclty piece of code into mixer_layer_priority().

• mixer_layer_priority(ctx);

  /* setting background color */ mixer_reg_write(res, MXR_BG_COLOR0, 0x008080);

@@ -1274,6 +1317,15 @@ static int mixer_probe(struct platform_device *pdev) ctx->vp_enabled = drv->is_vp_enabled; ctx->has_sclk = drv->has_sclk; ctx->mxr_ver = drv->version;

• if (drv->is_vp_enabled) {

• ctx->layer_config = vp_layer_config;
  

• ctx->num_layer = ARRAY_SIZE(vp_layer_config);
  

• } else {

• ctx->layer_config = default_layer_config;
  

• ctx->num_layer = ARRAY_SIZE(default_layer_config);
  

• }

Then this piece of code is useless.

Gustavo