drm/exynos: two small fixes

drm/exynos: two small fixes

[Tobias Jakobi]

Hello,

I've dropped the remaining patches. Going to resend them once Gustavo's
cleanups have landed.

So this leaves just the small fry. Series is still based on [1].

With best wishes,
Tobias

[1] http://www.spinics.net/lists/linux-samsung-soc/msg43103.html

P.S.: I'm not sure if the two remaining patches from [1] still
apply cleanly to exynos-drm-next.

[PATCH v2 1/2] drm/exynos: mixer: move pixelformat defines

[Tobias Jakobi]

Move the defines for the pixelformats that the mixer supports out
of mixer_graph_buffer() to the top of the source.
Also add handling of RGB565 and exit if the pixelformat is not
supported.

Reviewed-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
---
 drivers/gpu/drm/exynos/exynos_mixer.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 3e07f04..9c398d5 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -44,6 +44,11 @@
 #define MIXER_WIN_NR		3
 #define MIXER_DEFAULT_WIN	0
 
+#define MIXER_PIXELFORMAT_RGB565 4
+#define MIXER_PIXELFORMAT_ARGB1555 5
+#define MIXER_PIXELFORMAT_ARGB4444 6
+#define MIXER_PIXELFORMAT_ARGB8888 7
+
 struct mixer_resources {
 	int			irq;
 	void __iomem		*mixer_regs;
@@ -536,31 +541,30 @@ static void mixer_graph_buffer(struct mixer_context *ctx, int win)
 
 	plane = &ctx->planes[win];
 
-	#define RGB565 4
-	#define ARGB1555 5
-	#define ARGB4444 6
-	#define ARGB8888 7
-
 	switch (plane->pixel_format) {
 	case DRM_FORMAT_ARGB4444:
-		fmt = ARGB4444;
+		fmt = MIXER_PIXELFORMAT_ARGB4444;
 		blend = 1;
 		break;
 
 	case DRM_FORMAT_ARGB8888:
-		fmt = ARGB8888;
+		fmt = MIXER_PIXELFORMAT_ARGB8888;
 		blend = 1;
 		break;
 
 	case DRM_FORMAT_XRGB8888:
-		fmt = ARGB8888;
+		fmt = MIXER_PIXELFORMAT_ARGB8888;
 		blend = 0;
 		break;
 
-	default:
-		fmt = ARGB8888;
+	case DRM_FORMAT_RGB565:
+		fmt = MIXER_PIXELFORMAT_RGB565;
 		blend = 0;
 		break;
+
+	default:
+		DRM_DEBUG_KMS("pixelformat unsupported by mixer\n");
+		return;
 	}
 
 	/* check if mixer supports requested scaling setup */
-- 
2.0.5

[PATCH v2 1/2] drm/exynos: mixer: move pixelformat defines

[Tobias Jakobi]

Move the defines for the pixelformats that the mixer supports out
of mixer_graph_buffer() to the top of the source.
Also add handling of RGB565 and exit if the pixelformat is not
supported.

Reviewed-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
---
 drivers/gpu/drm/exynos/exynos_mixer.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 3e07f04..9c398d5 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -44,6 +44,11 @@
 #define MIXER_WIN_NR		3
 #define MIXER_DEFAULT_WIN	0
 
+#define MIXER_PIXELFORMAT_RGB565 4
+#define MIXER_PIXELFORMAT_ARGB1555 5
+#define MIXER_PIXELFORMAT_ARGB4444 6
+#define MIXER_PIXELFORMAT_ARGB8888 7
+
 struct mixer_resources {
 	int			irq;
 	void __iomem		*mixer_regs;
@@ -536,31 +541,30 @@ static void mixer_graph_buffer(struct mixer_context *ctx, int win)
 
 	plane = &ctx->planes[win];
 
-	#define RGB565 4
-	#define ARGB1555 5
-	#define ARGB4444 6
-	#define ARGB8888 7
-
 	switch (plane->pixel_format) {
 	case DRM_FORMAT_ARGB4444:
-		fmt = ARGB4444;
+		fmt = MIXER_PIXELFORMAT_ARGB4444;
 		blend = 1;
 		break;
 
 	case DRM_FORMAT_ARGB8888:
-		fmt = ARGB8888;
+		fmt = MIXER_PIXELFORMAT_ARGB8888;
 		blend = 1;
 		break;
 
 	case DRM_FORMAT_XRGB8888:
-		fmt = ARGB8888;
+		fmt = MIXER_PIXELFORMAT_ARGB8888;
 		blend = 0;
 		break;
 
-	default:
-		fmt = ARGB8888;
+	case DRM_FORMAT_RGB565:
+		fmt = MIXER_PIXELFORMAT_RGB565;
 		blend = 0;
 		break;
+
+	default:
+		DRM_DEBUG_KMS("pixelformat unsupported by mixer\n");
+		return;
 	}
 
 	/* check if mixer supports requested scaling setup */
-- 
2.0.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/2] drm/exynos: remove unused 'activated' field from exynos_drm_plane

[Tobias Jakobi]

No component of Exynos DRM uses this field. Perhaps it was
once meant to provide more fine-grained information in
addition to the status stored in the 'enabled' field.

Reviewed-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
---
 drivers/gpu/drm/exynos/exynos_drm_drv.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 6a849cf..4c14a89 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -77,7 +77,6 @@ extern void exynos4412_qos(u8 tm, u8 ac);
  * @color_key: color key on or off.
  * @local_path: in case of lcd type, local path mode on or off.
  * @transparency: transparency on or off.
- * @activated: activated or not.
  * @enabled: enabled or not.
  * @resume: to resume or not.
  *
@@ -112,7 +111,6 @@ struct exynos_drm_plane {
 	bool color_key:1;
 	bool local_path:1;
 	bool transparency:1;
-	bool activated:1;
 	bool enabled:1;
 	bool resume:1;
 };
-- 
2.0.5

Re: [PATCH v2 2/2] drm/exynos: remove unused 'activated' field from exynos_drm_plane

[Joonyoung Shim]

Hi Tobias,

On 04/16/2015 04:54 AM, Tobias Jakobi wrote:
> No component of Exynos DRM uses this field. Perhaps it was
> once meant to provide more fine-grained information in
> addition to the status stored in the 'enabled' field.
> 
> Reviewed-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_drv.h | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> index 6a849cf..4c14a89 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> @@ -77,7 +77,6 @@ extern void exynos4412_qos(u8 tm, u8 ac);
>   * @color_key: color key on or off.
>   * @local_path: in case of lcd type, local path mode on or off.
>   * @transparency: transparency on or off.
> - * @activated: activated or not.
>   * @enabled: enabled or not.
>   * @resume: to resume or not.
>   *
> @@ -112,7 +111,6 @@ struct exynos_drm_plane {
>  	bool color_key:1;
>  	bool local_path:1;
>  	bool transparency:1;
> -	bool activated:1;
>  	bool enabled:1;
>  	bool resume:1;
>  };
> 

The following fields also are unused in exynos drm driver,
- default win, color_key, local_path, transparency

Inki, how about remove unused fields?

Thanks.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/2] drm/exynos: remove unused 'activated' field from exynos_drm_plane

[Tobias Jakobi]

Hello Joonyoung,

On 2015-04-17 08:30, Joonyoung Shim wrote:
> Hi Tobias,
> 
> On 04/16/2015 04:54 AM, Tobias Jakobi wrote:
>> No component of Exynos DRM uses this field. Perhaps it was
>> once meant to provide more fine-grained information in
>> addition to the status stored in the 'enabled' field.
>> 
>> Reviewed-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_drv.h | 2 --
>>  1 file changed, 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h 
>> b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> index 6a849cf..4c14a89 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> @@ -77,7 +77,6 @@ extern void exynos4412_qos(u8 tm, u8 ac);
>>   * @color_key: color key on or off.
>>   * @local_path: in case of lcd type, local path mode on or off.
>>   * @transparency: transparency on or off.
>> - * @activated: activated or not.
>>   * @enabled: enabled or not.
>>   * @resume: to resume or not.
>>   *
>> @@ -112,7 +111,6 @@ struct exynos_drm_plane {
>>  	bool color_key:1;
>>  	bool local_path:1;
>>  	bool transparency:1;
>> -	bool activated:1;
>>  	bool enabled:1;
>>  	bool resume:1;
>>  };
>> 
> 
> The following fields also are unused in exynos drm driver,
> - default win, color_key, local_path, transparency
Yeah, looks like it. I just stumbled upon 'activated' because I was 
wondering what the difference between 'enabled' and 'activated' was. 
Grepping than revealed that nothing was using that field. I didn't check 
any other fields.

Also I just noticed that due to how git formats the patches, this one 
doesn't apply to any of Inki's branches (due to the 'extern void 
exynos4412_qos(u8 tm, u8 ac);' line).

Should I respin this, and if yes, should I also drop the other fields 
mentioned? Question is more directed to Gustavo, since he's cleaning 
'exynos_drm_plane' anyway.


> Inki, how about remove unused fields?
> 
> Thanks.


With best wishes,
Tobias

Re: [PATCH v2 2/2] drm/exynos: remove unused 'activated' field from exynos_drm_plane

[Joonyoung Shim]

Hi Tobias,

On 04/17/2015 05:04 PM, Tobias Jakobi wrote:
> Hello Joonyoung,
> 
> On 2015-04-17 08:30, Joonyoung Shim wrote:
>> Hi Tobias,
>>
>> On 04/16/2015 04:54 AM, Tobias Jakobi wrote:
>>> No component of Exynos DRM uses this field. Perhaps it was
>>> once meant to provide more fine-grained information in
>>> addition to the status stored in the 'enabled' field.
>>>
>>> Reviewed-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>>> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
>>> ---
>>>  drivers/gpu/drm/exynos/exynos_drm_drv.h | 2 --
>>>  1 file changed, 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>>> index 6a849cf..4c14a89 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>>> @@ -77,7 +77,6 @@ extern void exynos4412_qos(u8 tm, u8 ac);
>>>   * @color_key: color key on or off.
>>>   * @local_path: in case of lcd type, local path mode on or off.
>>>   * @transparency: transparency on or off.
>>> - * @activated: activated or not.
>>>   * @enabled: enabled or not.
>>>   * @resume: to resume or not.
>>>   *
>>> @@ -112,7 +111,6 @@ struct exynos_drm_plane {
>>>      bool color_key:1;
>>>      bool local_path:1;
>>>      bool transparency:1;
>>> -    bool activated:1;
>>>      bool enabled:1;
>>>      bool resume:1;
>>>  };
>>>
>>
>> The following fields also are unused in exynos drm driver,
>> - default win, color_key, local_path, transparency
> Yeah, looks like it. I just stumbled upon 'activated' because I was wondering what the difference between 'enabled' and 'activated' was. Grepping than revealed that nothing was using that field. I didn't check any other fields.
> 

Actually i don't know about activated field but i think maybe activated
field and other fields seem be just reserved for any features later when
exynos drm driver was posted first.

> Also I just noticed that due to how git formats the patches, this one doesn't apply to any of Inki's branches (due to the 'extern void exynos4412_qos(u8 tm, u8 ac);' line).
> 
> Should I respin this, and if yes, should I also drop the other fields mentioned? Question is more directed to Gustavo, since he's cleaning 'exynos_drm_plane' anyway.
> 

I think you can post patch based on latest branch(exynos-drm-next) of
Inki with removing other fields.

Thanks.

> 
>> Inki, how about remove unused fields?
>>
>> Thanks.
> 
> 
> With best wishes,
> Tobias
> 
> 
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/2] drm/exynos: remove unused 'activated' field from exynos_drm_plane

[Inki Dae]

On 2015년 04월 17일 17:23, Joonyoung Shim wrote:
> Hi Tobias,
> 
> On 04/17/2015 05:04 PM, Tobias Jakobi wrote:
>> Hello Joonyoung,
>>
>> On 2015-04-17 08:30, Joonyoung Shim wrote:
>>> Hi Tobias,
>>>
>>> On 04/16/2015 04:54 AM, Tobias Jakobi wrote:
>>>> No component of Exynos DRM uses this field. Perhaps it was
>>>> once meant to provide more fine-grained information in
>>>> addition to the status stored in the 'enabled' field.
>>>>
>>>> Reviewed-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>>>> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
>>>> ---
>>>>  drivers/gpu/drm/exynos/exynos_drm_drv.h | 2 --
>>>>  1 file changed, 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>>>> index 6a849cf..4c14a89 100644
>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>>>> @@ -77,7 +77,6 @@ extern void exynos4412_qos(u8 tm, u8 ac);
>>>>   * @color_key: color key on or off.
>>>>   * @local_path: in case of lcd type, local path mode on or off.
>>>>   * @transparency: transparency on or off.
>>>> - * @activated: activated or not.
>>>>   * @enabled: enabled or not.
>>>>   * @resume: to resume or not.
>>>>   *
>>>> @@ -112,7 +111,6 @@ struct exynos_drm_plane {
>>>>      bool color_key:1;
>>>>      bool local_path:1;
>>>>      bool transparency:1;
>>>> -    bool activated:1;
>>>>      bool enabled:1;
>>>>      bool resume:1;
>>>>  };
>>>>
>>>
>>> The following fields also are unused in exynos drm driver,
>>> - default win, color_key, local_path, transparency
>> Yeah, looks like it. I just stumbled upon 'activated' because I was wondering what the difference between 'enabled' and 'activated' was. Grepping than revealed that nothing was using that field. I didn't check any other fields.
>>
> 
> Actually i don't know about activated field but i think maybe activated
> field and other fields seem be just reserved for any features later when
> exynos drm driver was posted first.

Exactly. These fields should be removed if they are not used anywhere.
They can be added again when used actually.

Thanks,
Inki Dae

> 
>> Also I just noticed that due to how git formats the patches, this one doesn't apply to any of Inki's branches (due to the 'extern void exynos4412_qos(u8 tm, u8 ac);' line).
>>
>> Should I respin this, and if yes, should I also drop the other fields mentioned? Question is more directed to Gustavo, since he's cleaning 'exynos_drm_plane' anyway.
>>
> 
> I think you can post patch based on latest branch(exynos-drm-next) of
> Inki with removing other fields.
> 
> Thanks.
> 
>>
>>> Inki, how about remove unused fields?
>>>
>>> Thanks.
>>
>>
>> With best wishes,
>> Tobias
>>
>>
>>
> 
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/2] drm/exynos: remove unused 'activated' field from exynos_drm_plane

[Gustavo Padovan]

2015-04-17 Tobias Jakobi <tjakobi@math.uni-bielefeld.de>:

> Hello Joonyoung,
> 
> On 2015-04-17 08:30, Joonyoung Shim wrote:
> >Hi Tobias,
> >
> >On 04/16/2015 04:54 AM, Tobias Jakobi wrote:
> >>No component of Exynos DRM uses this field. Perhaps it was
> >>once meant to provide more fine-grained information in
> >>addition to the status stored in the 'enabled' field.
> >>
> >>Reviewed-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> >>Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
> >>---
> >> drivers/gpu/drm/exynos/exynos_drm_drv.h | 2 --
> >> 1 file changed, 2 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> >>b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> >>index 6a849cf..4c14a89 100644
> >>--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> >>+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> >>@@ -77,7 +77,6 @@ extern void exynos4412_qos(u8 tm, u8 ac);
> >>  * @color_key: color key on or off.
> >>  * @local_path: in case of lcd type, local path mode on or off.
> >>  * @transparency: transparency on or off.
> >>- * @activated: activated or not.
> >>  * @enabled: enabled or not.
> >>  * @resume: to resume or not.
> >>  *
> >>@@ -112,7 +111,6 @@ struct exynos_drm_plane {
> >> 	bool color_key:1;
> >> 	bool local_path:1;
> >> 	bool transparency:1;
> >>-	bool activated:1;
> >> 	bool enabled:1;
> >> 	bool resume:1;
> >> };
> >>
> >
> >The following fields also are unused in exynos drm driver,
> >- default win, color_key, local_path, transparency
> Yeah, looks like it. I just stumbled upon 'activated' because I was
> wondering what the difference between 'enabled' and 'activated' was.
> Grepping than revealed that nothing was using that field. I didn't check any
> other fields.
> 
> Also I just noticed that due to how git formats the patches, this one
> doesn't apply to any of Inki's branches (due to the 'extern void
> exynos4412_qos(u8 tm, u8 ac);' line).
> 
> Should I respin this, and if yes, should I also drop the other fields
> mentioned? Question is more directed to Gustavo, since he's cleaning
> 'exynos_drm_plane' anyway.

Let's remove them now, that doesn't affect my work.

	Gustavo