| Message ID | 1441311297-5083-1-git-send-email-gustavo@padovan.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@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@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 > +
On Fri, Sep 04, 2015 at 01:05:35PM +0900, Inki Dae wrote: > 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@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@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. I think cursor should by default be the top layer (if the hw can do it). Otherwise it will be really confusing to compositors I fear. -Daniel
On 2015? 09? 04? 16:19, Daniel Vetter wrote: > On Fri, Sep 04, 2015 at 01:05:35PM +0900, Inki Dae wrote: >> 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@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@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. > > I think cursor should by default be the top layer (if the hw can do it). > Otherwise it will be really confusing to compositors I fear. I also prefer first one. Thanks, Inki Dae > -Daniel >
Hi Inki, 2015-09-04 Inki Dae <inki.dae@samsung.com>: > 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@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@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. I've just sent a v2 adjusting the cursor window to be the on the higher layer. However I couldn't check if that is enough for all SoC versions. Gustavo
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 + #define to_exynos_crtc(x) container_of(x, struct exynos_drm_crtc, base) #define to_exynos_plane(x) container_of(x, struct exynos_drm_plane, base) diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 750a9e6..2697ebc 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -154,7 +154,6 @@ struct fimd_context { struct clk *lcd_clk; void __iomem *regs; struct regmap *sysreg; - unsigned int default_win; unsigned long irq_flags; u32 vidcon0; u32 vidcon1; @@ -953,7 +952,6 @@ static int fimd_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; @@ -961,16 +959,14 @@ static int fimd_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, fimd_formats, + 1 << ctx->pipe, fimd_formats, ARRAY_SIZE(fimd_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, &fimd_crtc_ops, ctx); diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c index 7148224..d2aab4d 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_plane.c +++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c @@ -210,12 +210,24 @@ static void exynos_plane_attach_zpos_property(struct drm_plane *plane, int exynos_plane_init(struct drm_device *dev, struct exynos_drm_plane *exynos_plane, - unsigned long possible_crtcs, enum drm_plane_type type, - const uint32_t *formats, unsigned int fcount, - unsigned int zpos) + unsigned long possible_crtcs, const uint32_t *formats, + unsigned int fcount, unsigned int zpos) { + enum drm_plane_type type; int err; + switch (zpos) { + case DEFAULT_WIN: + type = DRM_PLANE_TYPE_PRIMARY; + break; + case CURSOR_WIN: + type = DRM_PLANE_TYPE_CURSOR; + break; + default: + type = DRM_PLANE_TYPE_OVERLAY; + break; + }; + err = drm_universal_plane_init(dev, &exynos_plane->base, possible_crtcs, &exynos_plane_funcs, formats, fcount, type); diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.h b/drivers/gpu/drm/exynos/exynos_drm_plane.h index 476c934..0ab5acf 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_plane.h +++ b/drivers/gpu/drm/exynos/exynos_drm_plane.h @@ -11,6 +11,5 @@ int exynos_plane_init(struct drm_device *dev, struct exynos_drm_plane *exynos_plane, - unsigned long possible_crtcs, enum drm_plane_type type, - const uint32_t *formats, unsigned int fcount, - unsigned int zpos); + unsigned long possible_crtcs, const uint32_t *formats, + unsigned int fcount, unsigned int zpos); diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c index 75718e1..4499117 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c @@ -42,7 +42,6 @@ struct vidi_context { struct exynos_drm_plane planes[WINDOWS_NR]; struct edid *raw_edid; unsigned int clkdiv; - unsigned int default_win; unsigned long irq_flags; unsigned int connected; bool vblank_on; @@ -439,23 +438,20 @@ static int vidi_bind(struct device *dev, struct device *master, void *data) struct drm_device *drm_dev = data; struct drm_encoder *encoder = &ctx->encoder; struct exynos_drm_plane *exynos_plane; - enum drm_plane_type type; unsigned int zpos; int pipe, ret; vidi_ctx_initialize(ctx, drm_dev); 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, formats, + 1 << ctx->pipe, formats, ARRAY_SIZE(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_VIDI, &vidi_crtc_ops, ctx); @@ -507,7 +503,6 @@ static int vidi_probe(struct platform_device *pdev) if (!ctx) return -ENOMEM; - ctx->default_win = 0; ctx->pdev = pdev; INIT_WORK(&ctx->work, vidi_fake_vblank_handler); diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 7f81cce..2484277 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -600,7 +600,7 @@ static void mixer_graph_buffer(struct mixer_context *ctx, /* setup display size */ if (ctx->mxr_ver == MXR_VER_128_0_0_184 && - win == MIXER_DEFAULT_WIN) { + win == DEFAULT_WIN) { val = MXR_MXR_RES_HEIGHT(mode->vdisplay); val |= MXR_MXR_RES_WIDTH(mode->hdisplay); mixer_reg_write(res, MXR_RESOLUTION, val); @@ -1193,12 +1193,9 @@ static int mixer_bind(struct device *dev, struct device *manager, void *data) return ret; for (zpos = 0; zpos < MIXER_WIN_NR; zpos++) { - enum drm_plane_type type; const uint32_t *formats; unsigned int fcount; - type = (zpos == MIXER_DEFAULT_WIN) ? DRM_PLANE_TYPE_PRIMARY : - DRM_PLANE_TYPE_OVERLAY; if (zpos < VP_DEFAULT_WIN) { formats = mixer_formats; fcount = ARRAY_SIZE(mixer_formats); @@ -1208,13 +1205,12 @@ static int mixer_bind(struct device *dev, struct device *manager, void *data) } ret = exynos_plane_init(drm_dev, &ctx->planes[zpos], - 1 << ctx->pipe, type, formats, fcount, - zpos); + 1 << ctx->pipe, formats, fcount, zpos); if (ret) return ret; } - exynos_plane = &ctx->planes[MIXER_DEFAULT_WIN]; + exynos_plane = &ctx->planes[DEFAULT_WIN]; ctx->crtc = exynos_drm_crtc_create(drm_dev, &exynos_plane->base, ctx->pipe, EXYNOS_DISPLAY_TYPE_HDMI, &mixer_crtc_ops, ctx);