[PATCH 1/2] drm/exynos: mixer: cleanup pixelformat handling

[Tobias Jakobi]

On 2015-04-24 03:48, Joonyoung Shim wrote:
> Hi Tobias,
> 
> On 04/23/2015 09:12 PM, Tobias Jakobi wrote:
>> Move the defines for the pixelformats that the mixer supports out
>> of mixer_graph_buffer() to the top of the source.
>> Then select the mixer pixelformat (pf) in mixer_graph_buffer() based 
>> on
>> the plane's pf (and not bpp).
>> Also add handling of RGB565 and XRGB1555 to the switch statement and
>> exit early if the plane has an unsupported pf.
>> 
>> Partially based on 'drm/exynos: enable/disable blend based on pixel
>> format' by Gustavo Padovan <gustavo.padovan at collabora.co.uk>.
>> 
>> Signed-off-by: Tobias Jakobi <tjakobi at math.uni-bielefeld.de>
>> ---
>>  drivers/gpu/drm/exynos/exynos_mixer.c | 32 
>> ++++++++++++++++++++++----------
>>  1 file changed, 22 insertions(+), 10 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
>> b/drivers/gpu/drm/exynos/exynos_mixer.c
>> index fbec750..1bd23ee 100644
>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>> @@ -44,6 +44,12 @@
>>  #define MIXER_WIN_NR		3
>>  #define MIXER_DEFAULT_WIN	0
>> 
>> +/* The pixelformats that are natively supported by the mixer. */
>> +#define MIXER_PIXELFORMAT_RGB565 4
>> +#define MIXER_PIXELFORMAT_ARGB1555 5
>> +#define MIXER_PIXELFORMAT_ARGB4444 6
>> +#define MIXER_PIXELFORMAT_ARGB8888 7
>> +
> 
> Seems be long, how about s/PIXELFORMAT/FORMAT or just MIXER_RGB565 ... 
> ?
> Please use tab between define name and value.
How about MXR_FORMAT_XYZ, to stay consistent with the regs-mixer header?


>>  struct mixer_resources {
>>  	int			irq;
>>  	void __iomem		*mixer_regs;
>> @@ -531,20 +537,26 @@ 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_XRGB4444:
>> +		fmt = MIXER_PIXELFORMAT_ARGB4444;
>> +		break;
>> +
>> +	case DRM_FORMAT_XRGB1555:
>> +		fmt = MIXER_PIXELFORMAT_ARGB1555;
>> +		break;
>> 
>> -	switch (plane->bpp) {
>> -	case 16:
>> -		fmt = ARGB4444;
>> +	case DRM_FORMAT_RGB565:
>> +		fmt = MIXER_PIXELFORMAT_RGB565;
>>  		break;
>> -	case 32:
>> -		fmt = ARGB8888;
>> +
>> +	case DRM_FORMAT_XRGB8888:
>> +		fmt = MIXER_PIXELFORMAT_ARGB8888;
>>  		break;
>> +
>>  	default:
>> -		fmt = ARGB8888;
>> +		DRM_DEBUG_KMS("pixelformat unsupported by mixer\n");
>> +		return;
>>  	}
>> 
>>  	/* check if mixer supports requested scaling setup */
>> 
> 
> Hmm missing formats having alpha?
> DRM_FORMAT_ARGB4444
> DRM_FORMAT_ARGB1555
> DRM_FORMAT_ARGB8888
Should be obvious from my other mail. Blending/alpha is currently 
screwed up, so we shouldn't add these formats at this point. At least 
not before figuring out how to properly solve the issue.



> Thanks.

With best wishes,
Tobias