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@xxxxxxxxxxxxxxx>
>>>
>>> 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@xxxxxxxxxxxxxxx>
>>> ---
>>> 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
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
