Re: [PATCH v3 1/7] drm/exynos: rename zpos to index

[Inki Dae]

Hi Marek,

2015년 12월 28일 21:34에 Marek Szyprowski 이(가) 쓴 글:
> Hello,
> 
> On 2015-12-24 09:15, Inki Dae wrote:
>> Seems this patch could be more cleaned up.
>>
>> Each ctx object of each crtc driver has its own plane config object which 
>> includes already zpos value.
>> So I think we wouldn't need to keep zpos of the plane config and index of 
>> the plane object.
>> How about removing index from exynos plane structure and using zpos of 
>> exynos plane config structure instead?
>>
>> Below patch can be applied on top of your patch,
> 
> If you want I can move 'index' entry to config object, but the initial zpos 
> value
> should also be there. Please note that index is not always equal to the 
> initial zpos
> and having initial zpos configurable is also needed. There were already 
> concerns if
> mixer's dedicated video plane should be below or above the primary gfx plane 
> in the
> default configuration.

Merged all patches.

Thanks,
Inki Dae

> 
>> (...)
> 
> Best regards
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/7] drm/exynos: rename zpos to index

[Marek Szyprowski]

Hello,

On 2015-12-24 09:15, Inki Dae wrote:

Seems this patch could be more cleaned up.

Each ctx object of each crtc driver has its own plane config object which 
includes already zpos value.
So I think we wouldn't need to keep zpos of the plane config and index of the 
plane object.
How about removing index from exynos plane structure and using zpos of exynos 
plane config structure instead?

Below patch can be applied on top of your patch,


If you want I can move 'index' entry to config object, but the initial 
zpos value
should also be there. Please note that index is not always equal to the 
initial zpos
and having initial zpos configurable is also needed. There were already 
concerns if
mixer's dedicated video plane should be below or above the primary gfx 
plane in the

default configuration.

> (...)

Best regards
--
Marek Szyprowski, PhD
Samsung R Institute Poland

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/7] drm/exynos: rename zpos to index

[Inki Dae]

Hi Marek,

Seems this patch could be more cleaned up.

Each ctx object of each crtc driver has its own plane config object which 
includes already zpos value.
So I think we wouldn't need to keep zpos of the plane config and index of the 
plane object.
How about removing index from exynos plane structure and using zpos of exynos 
plane config structure instead?

Below patch can be applied on top of your patch,
diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c 
b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 88d022a..009fa18 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -256,7 +256,7 @@ static void decon_atomic_begin(struct exynos_drm_crtc *crtc,
if (test_bit(BIT_SUSPENDED, >flags))
return;
 
-   decon_shadow_protect_win(ctx, plane->index, true);
+   decon_shadow_protect_win(ctx, plane->config->index, true);
 }
 
 #define BIT_VAL(x, e, s) (((x) & ((1 << ((e) - (s) + 1)) - 1)) << (s))
@@ -270,7 +270,7 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
to_exynos_plane_state(plane->base.state);
struct decon_context *ctx = crtc->ctx;
struct drm_framebuffer *fb = state->base.fb;
-   unsigned int win = plane->index;
+   unsigned int win = plane->config->index;
unsigned int bpp = fb->bits_per_pixel >> 3;
unsigned int pitch = fb->pitches[0];
dma_addr_t dma_addr = exynos_drm_fb_dma_addr(fb, 0);
@@ -320,7 +320,7 @@ static void decon_disable_plane(struct exynos_drm_crtc 
*crtc,
struct exynos_drm_plane *plane)
 {
struct decon_context *ctx = crtc->ctx;
-   unsigned int win = plane->index;
+   unsigned int win = plane->config->index;
 
if (test_bit(BIT_SUSPENDED, >flags))
return;
@@ -344,7 +344,7 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc,
if (test_bit(BIT_SUSPENDED, >flags))
return;
 
-   decon_shadow_protect_win(ctx, plane->index, false);
+   decon_shadow_protect_win(ctx, plane->config->index, false);
 
if (ctx->out_type == IFTYPE_I80)
set_bit(BIT_WIN_UPDATED, >flags);
@@ -499,10 +499,10 @@ static int decon_bind(struct device *dev, struct device 
*master, void *data)
 
ctx->configs[win].pixel_formats = decon_formats;
ctx->configs[win].num_pixel_formats = ARRAY_SIZE(decon_formats);
-   ctx->configs[win].zpos = win;
+   ctx->configs[win].index = win;
ctx->configs[win].type = decon_win_types[tmp];
 
-   ret = exynos_plane_init(drm_dev, >planes[win], i,
+   ret = exynos_plane_init(drm_dev, >planes[win],
1 << ctx->pipe, >configs[win]);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c 
b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
index 8911f965..6f47b60 100644
--- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
@@ -393,7 +393,7 @@ static void decon_atomic_begin(struct exynos_drm_crtc *crtc,
if (ctx->suspended)
return;
 
-   decon_shadow_protect_win(ctx, plane->index, true);
+   decon_shadow_protect_win(ctx, plane->config->index, true);
 }
 
 static void decon_update_plane(struct exynos_drm_crtc *crtc,
@@ -407,7 +407,7 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
unsigned long val, alpha;
unsigned int last_x;
unsigned int last_y;
-   unsigned int win = plane->index;
+   unsigned int win = plane->config->index;
unsigned int bpp = fb->bits_per_pixel >> 3;
unsigned int pitch = fb->pitches[0];
 
@@ -498,7 +498,7 @@ static void decon_disable_plane(struct exynos_drm_crtc 
*crtc,
struct exynos_drm_plane *plane)
 {
struct decon_context *ctx = crtc->ctx;
-   unsigned int win = plane->index;
+   unsigned int win = plane->config->index;
u32 val;
 
if (ctx->suspended)
@@ -525,7 +525,7 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc,
if (ctx->suspended)
return;
 
-   decon_shadow_protect_win(ctx, plane->index, false);
+   decon_shadow_protect_win(ctx, plane->config->index, false);
 }
 
 static void decon_init(struct decon_context *ctx)
@@ -654,10 +654,10 @@ static int decon_bind(struct device *dev, struct device 
*master, void *data)
for (i = 0; i < WINDOWS_NR; i++) {
ctx->configs[i].pixel_formats = decon_formats;
ctx->configs[i].num_pixel_formats = ARRAY_SIZE(decon_formats);
-   ctx->configs[i].zpos = i;
+   ctx->configs[i].index = i;
ctx->configs[i].type = decon_win_types[i];
 
-   ret = exynos_plane_init(drm_dev, >planes[i], i,
+   

Re: [PATCH v3 1/7] drm/exynos: rename zpos to index

[Inki Dae]

Below just trivial issue,

2015년 12월 16일 21:21에 Marek Szyprowski 이(가) 쓴 글:
> This patch renames zpos entry to index, because in most places it is
> used as index for selecting hardware layer/window instead of
> configurable layer position. This will later enable to make the zpos
> property configurable.
> 
> Signed-off-by: Marek Szyprowski 
> ---
>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 10 +-
>  drivers/gpu/drm/exynos/exynos7_drm_decon.c| 10 +-
>  drivers/gpu/drm/exynos/exynos_drm_drv.h   |  4 ++--
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c  | 10 +-
>  drivers/gpu/drm/exynos/exynos_drm_plane.c |  4 ++--
>  drivers/gpu/drm/exynos/exynos_drm_plane.h |  2 +-
>  drivers/gpu/drm/exynos/exynos_drm_vidi.c  |  2 +-
>  drivers/gpu/drm/exynos/exynos_mixer.c | 14 +++---
>  8 files changed, 28 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c 
> b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> index c7362b99ce28..88d022ad5280 100644
> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> @@ -256,7 +256,7 @@ static void decon_atomic_begin(struct exynos_drm_crtc 
> *crtc,
>   if (test_bit(BIT_SUSPENDED, >flags))
>   return;
>  
> - decon_shadow_protect_win(ctx, plane->zpos, true);
> + decon_shadow_protect_win(ctx, plane->index, true);
>  }
>  
>  #define BIT_VAL(x, e, s) (((x) & ((1 << ((e) - (s) + 1)) - 1)) << (s))
> @@ -270,7 +270,7 @@ static void decon_update_plane(struct exynos_drm_crtc 
> *crtc,
>   to_exynos_plane_state(plane->base.state);
>   struct decon_context *ctx = crtc->ctx;
>   struct drm_framebuffer *fb = state->base.fb;
> - unsigned int win = plane->zpos;
> + unsigned int win = plane->index;
>   unsigned int bpp = fb->bits_per_pixel >> 3;
>   unsigned int pitch = fb->pitches[0];
>   dma_addr_t dma_addr = exynos_drm_fb_dma_addr(fb, 0);
> @@ -320,7 +320,7 @@ static void decon_disable_plane(struct exynos_drm_crtc 
> *crtc,
>   struct exynos_drm_plane *plane)
>  {
>   struct decon_context *ctx = crtc->ctx;
> - unsigned int win = plane->zpos;
> + unsigned int win = plane->index;
>  
>   if (test_bit(BIT_SUSPENDED, >flags))
>   return;
> @@ -344,7 +344,7 @@ static void decon_atomic_flush(struct exynos_drm_crtc 
> *crtc,
>   if (test_bit(BIT_SUSPENDED, >flags))
>   return;
>  
> - decon_shadow_protect_win(ctx, plane->zpos, false);
> + decon_shadow_protect_win(ctx, plane->index, false);
>  
>   if (ctx->out_type == IFTYPE_I80)
>   set_bit(BIT_WIN_UPDATED, >flags);
> @@ -502,7 +502,7 @@ static int decon_bind(struct device *dev, struct device 
> *master, void *data)
>   ctx->configs[win].zpos = win;
>   ctx->configs[win].type = decon_win_types[tmp];
>  
> - ret = exynos_plane_init(drm_dev, >planes[win],
> + ret = exynos_plane_init(drm_dev, >planes[win], i,

'i' isn't declared so you have to use win instead.
Please, post the patch set at least after build test.

Thanks,
Inki Dae 
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html