[PATCH 1/2] drm/exynos: mixer: simplify mixer_cfg_rgb_fmt()

[Tobias Jakobi]

Hello Andrzej,

Andrzej Hajda wrote:
> Hi Tobias,
> 
> On 03.03.2017 14:40, Tobias Jakobi wrote:
>> Convert if-statements to switch statement. Removes
>> duplicated code.
>>
>> Signed-off-by: Tobias Jakobi <tjakobi at math.uni-bielefeld.de>
>> ---
>>  drivers/gpu/drm/exynos/exynos_mixer.c | 30 ++++++++----------------------
>>  1 file changed, 8 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
>> index 72143ac..41d0c36 100644
>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>> @@ -382,29 +382,14 @@ static void mixer_cfg_rgb_fmt(struct mixer_context *ctx, unsigned int height)
>>  	struct mixer_resources *res = &ctx->mixer_res;
>>  	u32 val;
>>  
>> -	if (height == 480) {
>> +	switch (height) {
>> +	case 480:
>> +	case 576:
>>  		val = MXR_CFG_RGB601_0_255;
>> -	} else if (height == 576) {
>> -		val = MXR_CFG_RGB601_0_255;
>> -	} else if (height == 720) {
>> -		val = MXR_CFG_RGB709_16_235;
>> -		mixer_reg_write(res, MXR_CM_COEFF_Y,
>> -				(1 << 30) | (94 << 20) | (314 << 10) |
>> -				(32 << 0));
>> -		mixer_reg_write(res, MXR_CM_COEFF_CB,
>> -				(972 << 20) | (851 << 10) | (225 << 0));
>> -		mixer_reg_write(res, MXR_CM_COEFF_CR,
>> -				(225 << 20) | (820 << 10) | (1004 << 0));
>> -	} else if (height == 1080) {
>> -		val = MXR_CFG_RGB709_16_235;
>> -		mixer_reg_write(res, MXR_CM_COEFF_Y,
>> -				(1 << 30) | (94 << 20) | (314 << 10) |
>> -				(32 << 0));
>> -		mixer_reg_write(res, MXR_CM_COEFF_CB,
>> -				(972 << 20) | (851 << 10) | (225 << 0));
>> -		mixer_reg_write(res, MXR_CM_COEFF_CR,
>> -				(225 << 20) | (820 << 10) | (1004 << 0));
>> -	} else {
>> +		break;
>> +	case 720:
>> +	case 1080:
>> +	default:
>>  		val = MXR_CFG_RGB709_16_235;
>>  		mixer_reg_write(res, MXR_CM_COEFF_Y,
>>  				(1 << 30) | (94 << 20) | (314 << 10) |
>> @@ -413,6 +398,7 @@ static void mixer_cfg_rgb_fmt(struct mixer_context *ctx, unsigned int height)
>>  				(972 << 20) | (851 << 10) | (225 << 0));
>>  		mixer_reg_write(res, MXR_CM_COEFF_CR,
>>  				(225 << 20) | (820 << 10) | (1004 << 0));
>> +		break;
>>  	}
> 
> Good change.
> Reviewed-by: Andrzej Hajda <a.hajda at samsung.com>
Thanks for the review.

> 
> One small nitpick.
> As I see this conditional/switch is to decide about BT standard
> depending on the height. The similar problem is addressed in exynos-gsc
> patches [1].
> It would be good to have the same criteria to distinguish SD/HD mode. I
> think ((height > 576) || (width > 720)) is more generic, in this case
> even (height > 576) looks better, but as this changes logic of the code
> it could be in separate patch.
I'm not submitting functional changes anymore. You probably still
remember how that ended up last time. It's just not worth my time, sorry.

With best wishes,
Tobias


> [1]: https://lkml.org/lkml/2017/2/21/864
> 
> Regards
> Andrzej
> 
>>  
>>  	mixer_reg_writemask(res, MXR_CFG, val, MXR_CFG_RGB_FMT_MASK);
> 
>