Hi Gustavo,
I had already a review but I didn't give any comment to you. Sorry about
that. This patch looks good to me but one thing isn't clear. Below is my
comment.
On 2015ë
09ì 04ì¼ 05:14, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
>
> Set one of the planes for each crtc driver as a cursor plane enabled
> window managers to fully work on exynos.
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
> ---
> drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 9 ++-------
> drivers/gpu/drm/exynos/exynos7_drm_decon.c | 8 ++------
> drivers/gpu/drm/exynos/exynos_drm_drv.h | 3 +++
> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 8 ++------
> drivers/gpu/drm/exynos/exynos_drm_plane.c | 18 +++++++++++++++---
> drivers/gpu/drm/exynos/exynos_drm_plane.h | 5 ++---
> drivers/gpu/drm/exynos/exynos_drm_vidi.c | 9 ++-------
> drivers/gpu/drm/exynos/exynos_mixer.c | 10 +++-------
> 8 files changed, 31 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> index b3c7307..79b2b22 100644
> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> @@ -33,7 +33,6 @@ struct decon_context {
> struct exynos_drm_plane planes[WINDOWS_NR];
> void __iomem *addr;
> struct clk *clks[6];
> - unsigned int default_win;
> unsigned long irq_flags;
> int pipe;
> bool suspended;
> @@ -493,7 +492,6 @@ static int decon_bind(struct device *dev, struct device
> *master, void *data)
> struct drm_device *drm_dev = data;
> struct exynos_drm_private *priv = drm_dev->dev_private;
> struct exynos_drm_plane *exynos_plane;
> - enum drm_plane_type type;
> unsigned int zpos;
> int ret;
>
> @@ -501,16 +499,14 @@ static int decon_bind(struct device *dev, struct device
> *master, void *data)
> ctx->pipe = priv->pipe++;
>
> for (zpos = 0; zpos < WINDOWS_NR; zpos++) {
> - type = (zpos == ctx->default_win) ? DRM_PLANE_TYPE_PRIMARY :
> - DRM_PLANE_TYPE_OVERLAY;
> ret = exynos_plane_init(drm_dev, &ctx->planes[zpos],
> - 1 << ctx->pipe, type, decon_formats,
> + 1 << ctx->pipe, decon_formats,
> ARRAY_SIZE(decon_formats), zpos);
> if (ret)
> return ret;
> }
>
> - exynos_plane = &ctx->planes[ctx->default_win];
> + exynos_plane = &ctx->planes[DEFAULT_WIN];
> ctx->crtc = exynos_drm_crtc_create(drm_dev, &exynos_plane->base,
> ctx->pipe, EXYNOS_DISPLAY_TYPE_LCD,
> &decon_crtc_ops, ctx);
> @@ -607,7 +603,6 @@ static int exynos5433_decon_probe(struct platform_device
> *pdev)
> if (!ctx)
> return -ENOMEM;
>
> - ctx->default_win = 0;
> ctx->suspended = true;
> ctx->dev = dev;
> if (of_get_child_by_name(dev->of_node, "i80-if-timings"))
> diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> index cbdb78e..f3826dc 100644
> --- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> +++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> @@ -52,7 +52,6 @@ struct decon_context {
> struct clk *eclk;
> struct clk *vclk;
> void __iomem *regs;
> - unsigned int default_win;
> unsigned long irq_flags;
> bool i80_if;
> bool suspended;
> @@ -691,7 +690,6 @@ static int decon_bind(struct device *dev, struct device
> *master, void *data)
> struct decon_context *ctx = dev_get_drvdata(dev);
> struct drm_device *drm_dev = data;
> struct exynos_drm_plane *exynos_plane;
> - enum drm_plane_type type;
> unsigned int zpos;
> int ret;
>
> @@ -702,16 +700,14 @@ static int decon_bind(struct device *dev, struct device
> *master, void *data)
> }
>
> for (zpos = 0; zpos < WINDOWS_NR; zpos++) {
> - type = (zpos == ctx->default_win) ? DRM_PLANE_TYPE_PRIMARY :
> - DRM_PLANE_TYPE_OVERLAY;
> ret = exynos_plane_init(drm_dev, &ctx->planes[zpos],
> - 1 << ctx->pipe, type, decon_formats,
> + 1 << ctx->pipe, decon_formats,
> ARRAY_SIZE(decon_formats), zpos);
> if (ret)
> return ret;
> }
>
> - exynos_plane = &ctx->planes[ctx->default_win];
> + exynos_plane = &ctx->planes[DEFAULT_WIN];
> ctx->crtc = exynos_drm_crtc_create(drm_dev, &exynos_plane->base,
> ctx->pipe, EXYNOS_DISPLAY_TYPE_LCD,
> &decon_crtc_ops, ctx);
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> index b7ba21d..cc56c3d 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> @@ -22,6 +22,9 @@
> #define MAX_PLANE 5
> #define MAX_FB_BUFFER 4
>
> +#define DEFAULT_WIN 0
> +#define CURSOR_WIN 1
You fixed overlay number for cursor with 1. However, Display controllers
of Exynos SoC have fixed overlay priority like this,
win 4 > win 3 > win 2 > win 1 > win 0 so if we fix the overlay number
for cursor and we use another overlay - which has a higher layer than
cursor - to display caption or UI then the we couldn't see the cursor on
screen without additional work such as color key or alpha channel.
So before that, you need to check how many overlays can be used
according to Exynos SoC versions, and which way is better - one is for
top layer to be fixed for cursor, other is for one given layer to be
fixed by user-space for cursor.
Thanks,
Inki Dae
> +
