Re: [PATCH v2 5/5] drm/omap: Enable COLOR_ENCODING and COLOR_RANGE properties for planes

On 30/11/2020 16:19, Laurent Pinchart wrote:

...

...

...

• switch (color_encoding) {
• case DRM_COLOR_YCBCR_BT601:

• if (color_range == DRM_COLOR_YCBCR_FULL_RANGE)
  

• 	csc = &coefs_yuv2rgb_bt601_full;
  

• else
  

• 	csc = &coefs_yuv2rgb_bt601_lim;
  

• break;
  

• case DRM_COLOR_YCBCR_BT709:

• if (color_range == DRM_COLOR_YCBCR_FULL_RANGE)
  

• 	csc = &coefs_yuv2rgb_bt709_full;
  

• else
  

• 	csc = &coefs_yuv2rgb_bt709_lim;
  

• break;
  

• default:

• DSSERR("Unsupported CSC mode %d for plane %d\n",
  

•        color_encoding, plane);
  

• return -EINVAL;
  

Can this happen, given that the properties are created with only the above four combinations being allowed ?

No, it shouldn't happen. Are you just asking, or are you suggesting that we shouldn't check if color_encoding is valid here?

I don't usually check parameters which we can expect to be correct, but with we use switch/if with the parameter, we have to deal with the "else" case too.

I use a default in that case, especially if it can avoid turning the function from void to int.

Yes, that's true. I'll do the change.

Tomi