diff mbox

drm/exynos: add cursor plane support

Message ID 1441311297-5083-1-git-send-email-gustavo@padovan.org (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Gustavo Padovan Sept. 3, 2015, 8:14 p.m. UTC
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(-)

Comments

Inki Dae Sept. 4, 2015, 4:05 a.m. UTC | #1
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

> +



--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Vetter Sept. 4, 2015, 7:19 a.m. UTC | #2
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
Inki Dae Sept. 4, 2015, 11:16 a.m. UTC | #3
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
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gustavo Padovan Sept. 4, 2015, 10:07 p.m. UTC | #4
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
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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);