diff mbox series

drm/tidss: remove AM65x PG1 YUV erratum code

Message ID 20200429121022.3871-1-tomi.valkeinen@ti.com (mailing list archive)
State New, archived
Headers show
Series drm/tidss: remove AM65x PG1 YUV erratum code | expand

Commit Message

Tomi Valkeinen April 29, 2020, 12:10 p.m. UTC
AM65x PG1 has a HW issue with YUV pixel formats, resulting in wrong
colors on the screen. This issue is fixed in PG2 hardware.

The driver currently has code to hide YUV pixel formats from the
userspace. To support PG2, we would need to add code to detect the SoC
version and hide the YUV formats based on that.

However, as PG1 will be phased out and PG2 will be the main platform, a
much simpler solution is just to drop the code in question. The downside
is that the users will be able to use YUV formats on PG1, getting wrong
colors on the screen. On the other hand, that may also be a plus, as the
same applications will now work on PG1 and PG2, even if the colors are
wrong on PG1.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/tidss/tidss_dispc.c | 11 ++---------
 drivers/gpu/drm/tidss/tidss_dispc.h |  5 -----
 2 files changed, 2 insertions(+), 14 deletions(-)

Comments

Jyri Sarha May 4, 2020, 10:51 a.m. UTC | #1
On 29/04/2020 15:10, Tomi Valkeinen wrote:
> AM65x PG1 has a HW issue with YUV pixel formats, resulting in wrong
> colors on the screen. This issue is fixed in PG2 hardware.
> 
> The driver currently has code to hide YUV pixel formats from the
> userspace. To support PG2, we would need to add code to detect the SoC
> version and hide the YUV formats based on that.
> 
> However, as PG1 will be phased out and PG2 will be the main platform, a
> much simpler solution is just to drop the code in question. The downside
> is that the users will be able to use YUV formats on PG1, getting wrong
> colors on the screen. On the other hand, that may also be a plus, as the
> same applications will now work on PG1 and PG2, even if the colors are
> wrong on PG1.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

One minor nitpick bellow, but otherwise:

Reviewed-by: Jyri Sarha <jsarha@ti.com>

> ---
>  drivers/gpu/drm/tidss/tidss_dispc.c | 11 ++---------
>  drivers/gpu/drm/tidss/tidss_dispc.h |  5 -----
>  2 files changed, 2 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
> index 29f42768e294..629dd06393f6 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
> @@ -181,10 +181,6 @@ const struct dispc_features dispc_am65x_feats = {
>  	.vid_name = { "vid", "vidl1" },
>  	.vid_lite = { false, true, },
>  	.vid_order = { 1, 0 },
> -
> -	.errata = {
> -		.i2000 = true,
> -	},
>  };
>  
>  static const u16 tidss_j721e_common_regs[DISPC_COMMON_REG_TABLE_LEN] = {
> @@ -2674,12 +2670,9 @@ int dispc_init(struct tidss_device *tidss)
>  		return -ENOMEM;
>  
>  	num_fourccs = 0;
> -	for (i = 0; i < ARRAY_SIZE(dispc_color_formats); ++i) {
> -		if (feat->errata.i2000 &&
> -		    dispc_fourcc_is_yuv(dispc_color_formats[i].fourcc))
> -			continue;
> +	for (i = 0; i < ARRAY_SIZE(dispc_color_formats); ++i)
>  		dispc->fourccs[num_fourccs++] = dispc_color_formats[i].fourcc;
> -	}
> +
>  	dispc->num_fourccs = num_fourccs;
>  	dispc->tidss = tidss;
>  	dispc->dev = dev;
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h b/drivers/gpu/drm/tidss/tidss_dispc.h
> index a4a68249e44b..1228b166b255 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.h
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.h
> @@ -46,10 +46,6 @@ struct dispc_features_scaling {
>  	u32 xinc_max;
>  };
>  
> -struct dispc_errata {
> -	bool i2000; /* DSS Does Not Support YUV Pixel Data Formats */
> -};
> -
>  enum dispc_vp_bus_type {
>  	DISPC_VP_DPI,		/* DPI output */
>  	DISPC_VP_OLDI,		/* OLDI (LVDS) output */
> @@ -84,7 +80,6 @@ struct dispc_features {
>  	bool vid_lite[TIDSS_MAX_PLANES];
>  	u32 vid_order[TIDSS_MAX_PLANES];
>  

Shouldn't we remove the above empty line too?

> -	struct dispc_errata errata;
>  };
>  
>  extern const struct dispc_features dispc_k2g_feats;
>
Tomi Valkeinen May 4, 2020, 10:54 a.m. UTC | #2
On 04/05/2020 13:51, Jyri Sarha wrote:
> On 29/04/2020 15:10, Tomi Valkeinen wrote:
>> AM65x PG1 has a HW issue with YUV pixel formats, resulting in wrong
>> colors on the screen. This issue is fixed in PG2 hardware.
>>
>> The driver currently has code to hide YUV pixel formats from the
>> userspace. To support PG2, we would need to add code to detect the SoC
>> version and hide the YUV formats based on that.
>>
>> However, as PG1 will be phased out and PG2 will be the main platform, a
>> much simpler solution is just to drop the code in question. The downside
>> is that the users will be able to use YUV formats on PG1, getting wrong
>> colors on the screen. On the other hand, that may also be a plus, as the
>> same applications will now work on PG1 and PG2, even if the colors are
>> wrong on PG1.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
> One minor nitpick bellow, but otherwise:
> 
> Reviewed-by: Jyri Sarha <jsarha@ti.com>
> 
>> ---
>>   drivers/gpu/drm/tidss/tidss_dispc.c | 11 ++---------
>>   drivers/gpu/drm/tidss/tidss_dispc.h |  5 -----
>>   2 files changed, 2 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
>> index 29f42768e294..629dd06393f6 100644
>> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
>> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
>> @@ -181,10 +181,6 @@ const struct dispc_features dispc_am65x_feats = {
>>   	.vid_name = { "vid", "vidl1" },
>>   	.vid_lite = { false, true, },
>>   	.vid_order = { 1, 0 },
>> -
>> -	.errata = {
>> -		.i2000 = true,
>> -	},
>>   };
>>   
>>   static const u16 tidss_j721e_common_regs[DISPC_COMMON_REG_TABLE_LEN] = {
>> @@ -2674,12 +2670,9 @@ int dispc_init(struct tidss_device *tidss)
>>   		return -ENOMEM;
>>   
>>   	num_fourccs = 0;
>> -	for (i = 0; i < ARRAY_SIZE(dispc_color_formats); ++i) {
>> -		if (feat->errata.i2000 &&
>> -		    dispc_fourcc_is_yuv(dispc_color_formats[i].fourcc))
>> -			continue;
>> +	for (i = 0; i < ARRAY_SIZE(dispc_color_formats); ++i)
>>   		dispc->fourccs[num_fourccs++] = dispc_color_formats[i].fourcc;
>> -	}
>> +
>>   	dispc->num_fourccs = num_fourccs;
>>   	dispc->tidss = tidss;
>>   	dispc->dev = dev;
>> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h b/drivers/gpu/drm/tidss/tidss_dispc.h
>> index a4a68249e44b..1228b166b255 100644
>> --- a/drivers/gpu/drm/tidss/tidss_dispc.h
>> +++ b/drivers/gpu/drm/tidss/tidss_dispc.h
>> @@ -46,10 +46,6 @@ struct dispc_features_scaling {
>>   	u32 xinc_max;
>>   };
>>   
>> -struct dispc_errata {
>> -	bool i2000; /* DSS Does Not Support YUV Pixel Data Formats */
>> -};
>> -
>>   enum dispc_vp_bus_type {
>>   	DISPC_VP_DPI,		/* DPI output */
>>   	DISPC_VP_OLDI,		/* OLDI (LVDS) output */
>> @@ -84,7 +80,6 @@ struct dispc_features {
>>   	bool vid_lite[TIDSS_MAX_PLANES];
>>   	u32 vid_order[TIDSS_MAX_PLANES];
>>   
> 
> Shouldn't we remove the above empty line too?

Indeed, I'll do the change. Thanks!

  Tomi
diff mbox series

Patch

diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
index 29f42768e294..629dd06393f6 100644
--- a/drivers/gpu/drm/tidss/tidss_dispc.c
+++ b/drivers/gpu/drm/tidss/tidss_dispc.c
@@ -181,10 +181,6 @@  const struct dispc_features dispc_am65x_feats = {
 	.vid_name = { "vid", "vidl1" },
 	.vid_lite = { false, true, },
 	.vid_order = { 1, 0 },
-
-	.errata = {
-		.i2000 = true,
-	},
 };
 
 static const u16 tidss_j721e_common_regs[DISPC_COMMON_REG_TABLE_LEN] = {
@@ -2674,12 +2670,9 @@  int dispc_init(struct tidss_device *tidss)
 		return -ENOMEM;
 
 	num_fourccs = 0;
-	for (i = 0; i < ARRAY_SIZE(dispc_color_formats); ++i) {
-		if (feat->errata.i2000 &&
-		    dispc_fourcc_is_yuv(dispc_color_formats[i].fourcc))
-			continue;
+	for (i = 0; i < ARRAY_SIZE(dispc_color_formats); ++i)
 		dispc->fourccs[num_fourccs++] = dispc_color_formats[i].fourcc;
-	}
+
 	dispc->num_fourccs = num_fourccs;
 	dispc->tidss = tidss;
 	dispc->dev = dev;
diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h b/drivers/gpu/drm/tidss/tidss_dispc.h
index a4a68249e44b..1228b166b255 100644
--- a/drivers/gpu/drm/tidss/tidss_dispc.h
+++ b/drivers/gpu/drm/tidss/tidss_dispc.h
@@ -46,10 +46,6 @@  struct dispc_features_scaling {
 	u32 xinc_max;
 };
 
-struct dispc_errata {
-	bool i2000; /* DSS Does Not Support YUV Pixel Data Formats */
-};
-
 enum dispc_vp_bus_type {
 	DISPC_VP_DPI,		/* DPI output */
 	DISPC_VP_OLDI,		/* OLDI (LVDS) output */
@@ -84,7 +80,6 @@  struct dispc_features {
 	bool vid_lite[TIDSS_MAX_PLANES];
 	u32 vid_order[TIDSS_MAX_PLANES];
 
-	struct dispc_errata errata;
 };
 
 extern const struct dispc_features dispc_k2g_feats;