openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] media: aspeed: Use full swing as JFIF to fix incorrect color
@ 2022-02-09  8:42 Jammy Huang
  2022-02-09  9:06 ` Paul Menzel
  0 siblings, 1 reply; 3+ messages in thread
From: Jammy Huang @ 2022-02-09  8:42 UTC (permalink / raw)
  To: eajames, mchehab, joel, andrew, linux-media, openbmc,
	linux-arm-kernel, linux-aspeed, linux-kernel

Current settings for video capture rgb-2-yuv is BT.601(studio swing),
but JFIF uses BT.601(full swing) to deocde. This mismatch will lead
to incorrect color. For example, input RGB value, (0, 0, 255), will
become (16, 16, 235) after jpg decoded.

Add an enum, aspeed_video_capture_format, to define VR008[7:6]
capture format and correct default settings for video capture to fix
the problem.

Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
---
v2:
 - update subject from 'media: aspeed: Fix-incorrect-color' to
   'media: aspeed: Use full swing as JFIF to fix incorrect'
 - update commit message
 - add enum, aspeed_video_capture_format, to define VR008[7:6]
---
 drivers/media/platform/aspeed-video.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index eb9c17ac0e14..5bcf60b4628b 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -86,8 +86,6 @@
 #define  VE_CTRL_SOURCE			BIT(2)
 #define  VE_CTRL_INT_DE			BIT(4)
 #define  VE_CTRL_DIRECT_FETCH		BIT(5)
-#define  VE_CTRL_YUV			BIT(6)
-#define  VE_CTRL_RGB			BIT(7)
 #define  VE_CTRL_CAPTURE_FMT		GENMASK(7, 6)
 #define  VE_CTRL_AUTO_OR_CURSOR		BIT(8)
 #define  VE_CTRL_CLK_INVERSE		BIT(11)
@@ -202,6 +200,15 @@ enum {
 	VIDEO_CLOCKS_ON,
 };
 
+// for VE_CTRL_CAPTURE_FMT
+enum aspeed_video_capture_format {
+	VIDEO_CAP_FMT_YUV_STUDIO = 0,
+	VIDEO_CAP_FMT_YUV_FULL,
+	VIDEO_CAP_FMT_RGB,
+	VIDEO_CAP_FMT_GRAY,
+	VIDEO_CAP_FMT_MAX
+};
+
 struct aspeed_video_addr {
 	unsigned int size;
 	dma_addr_t dma;
@@ -1089,7 +1096,8 @@ static void aspeed_video_init_regs(struct aspeed_video *video)
 	u32 comp_ctrl = VE_COMP_CTRL_RSVD |
 		FIELD_PREP(VE_COMP_CTRL_DCT_LUM, video->jpeg_quality) |
 		FIELD_PREP(VE_COMP_CTRL_DCT_CHR, video->jpeg_quality | 0x10);
-	u32 ctrl = VE_CTRL_AUTO_OR_CURSOR;
+	u32 ctrl = VE_CTRL_AUTO_OR_CURSOR |
+		FIELD_PREP(VE_CTRL_CAPTURE_FMT, VIDEO_CAP_FMT_YUV_FULL);
 	u32 seq_ctrl = video->jpeg_mode;
 
 	if (video->frame_rate)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] media: aspeed: Use full swing as JFIF to fix incorrect color
  2022-02-09  8:42 [PATCH v2] media: aspeed: Use full swing as JFIF to fix incorrect color Jammy Huang
@ 2022-02-09  9:06 ` Paul Menzel
  2022-02-10  1:03   ` Jammy Huang
  0 siblings, 1 reply; 3+ messages in thread
From: Paul Menzel @ 2022-02-09  9:06 UTC (permalink / raw)
  To: Jammy Huang
  Cc: linux-aspeed, andrew, openbmc, eajames, linux-kernel, mchehab,
	linux-arm-kernel, linux-media

Dear Jammy,


Am 09.02.22 um 09:42 schrieb Jammy Huang:
> Current settings for video capture rgb-2-yuv is BT.601(studio swing),
> but JFIF uses BT.601(full swing) to deocde. This mismatch will lead
> to incorrect color. For example, input RGB value, (0, 0, 255), will
> become (16, 16, 235) after jpg decoded.
> 
> Add an enum, aspeed_video_capture_format, to define VR008[7:6]
> capture format and correct default settings for video capture to fix
> the problem.

Maybe quote the datasheet:

VR008[7:6] will decide the data format for video capture:
00: CCIR601 studio swing compliant YUV format
01: CCIR601 full swing compliant YUV format
10: RGB format
11: Gray color mode

> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
> ---
> v2:
>   - update subject from 'media: aspeed: Fix-incorrect-color' to
>     'media: aspeed: Use full swing as JFIF to fix incorrect'
>   - update commit message
>   - add enum, aspeed_video_capture_format, to define VR008[7:6]
> ---
>   drivers/media/platform/aspeed-video.c | 14 +++++++++++---
>   1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
> index eb9c17ac0e14..5bcf60b4628b 100644
> --- a/drivers/media/platform/aspeed-video.c
> +++ b/drivers/media/platform/aspeed-video.c
> @@ -86,8 +86,6 @@
>   #define  VE_CTRL_SOURCE			BIT(2)
>   #define  VE_CTRL_INT_DE			BIT(4)
>   #define  VE_CTRL_DIRECT_FETCH		BIT(5)
> -#define  VE_CTRL_YUV			BIT(6)
> -#define  VE_CTRL_RGB			BIT(7)
>   #define  VE_CTRL_CAPTURE_FMT		GENMASK(7, 6)
>   #define  VE_CTRL_AUTO_OR_CURSOR		BIT(8)
>   #define  VE_CTRL_CLK_INVERSE		BIT(11)
> @@ -202,6 +200,15 @@ enum {
>   	VIDEO_CLOCKS_ON,
>   };
>   
> +// for VE_CTRL_CAPTURE_FMT
> +enum aspeed_video_capture_format {
> +	VIDEO_CAP_FMT_YUV_STUDIO = 0,

Maybe also append `_SWING`?

> +	VIDEO_CAP_FMT_YUV_FULL,
> +	VIDEO_CAP_FMT_RGB,
> +	VIDEO_CAP_FMT_GRAY,
> +	VIDEO_CAP_FMT_MAX
> +};
> +
>   struct aspeed_video_addr {
>   	unsigned int size;
>   	dma_addr_t dma;
> @@ -1089,7 +1096,8 @@ static void aspeed_video_init_regs(struct aspeed_video *video)
>   	u32 comp_ctrl = VE_COMP_CTRL_RSVD |
>   		FIELD_PREP(VE_COMP_CTRL_DCT_LUM, video->jpeg_quality) |
>   		FIELD_PREP(VE_COMP_CTRL_DCT_CHR, video->jpeg_quality | 0x10);
> -	u32 ctrl = VE_CTRL_AUTO_OR_CURSOR;
> +	u32 ctrl = VE_CTRL_AUTO_OR_CURSOR |
> +		FIELD_PREP(VE_CTRL_CAPTURE_FMT, VIDEO_CAP_FMT_YUV_FULL);
>   	u32 seq_ctrl = video->jpeg_mode;
>   
>   	if (video->frame_rate)

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] media: aspeed: Use full swing as JFIF to fix incorrect color
  2022-02-09  9:06 ` Paul Menzel
@ 2022-02-10  1:03   ` Jammy Huang
  0 siblings, 0 replies; 3+ messages in thread
From: Jammy Huang @ 2022-02-10  1:03 UTC (permalink / raw)
  To: Paul Menzel
  Cc: linux-aspeed, andrew, openbmc, eajames, linux-kernel, mchehab,
	linux-arm-kernel, linux-media

Dear Paul,

OK, I will update a new patch per your suggestion.

Thanks for your help.

On 2022/2/9 下午 05:06, Paul Menzel wrote:
> Dear Jammy,
>
>
> Am 09.02.22 um 09:42 schrieb Jammy Huang:
>> Current settings for video capture rgb-2-yuv is BT.601(studio swing),
>> but JFIF uses BT.601(full swing) to deocde. This mismatch will lead
>> to incorrect color. For example, input RGB value, (0, 0, 255), will
>> become (16, 16, 235) after jpg decoded.
>>
>> Add an enum, aspeed_video_capture_format, to define VR008[7:6]
>> capture format and correct default settings for video capture to fix
>> the problem.
> Maybe quote the datasheet:
>
> VR008[7:6] will decide the data format for video capture:
> 00: CCIR601 studio swing compliant YUV format
> 01: CCIR601 full swing compliant YUV format
> 10: RGB format
> 11: Gray color mode
>
>> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
>> ---
>> v2:
>>    - update subject from 'media: aspeed: Fix-incorrect-color' to
>>      'media: aspeed: Use full swing as JFIF to fix incorrect'
>>    - update commit message
>>    - add enum, aspeed_video_capture_format, to define VR008[7:6]
>> ---
>>    drivers/media/platform/aspeed-video.c | 14 +++++++++++---
>>    1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
>> index eb9c17ac0e14..5bcf60b4628b 100644
>> --- a/drivers/media/platform/aspeed-video.c
>> +++ b/drivers/media/platform/aspeed-video.c
>> @@ -86,8 +86,6 @@
>>    #define  VE_CTRL_SOURCE			BIT(2)
>>    #define  VE_CTRL_INT_DE			BIT(4)
>>    #define  VE_CTRL_DIRECT_FETCH		BIT(5)
>> -#define  VE_CTRL_YUV			BIT(6)
>> -#define  VE_CTRL_RGB			BIT(7)
>>    #define  VE_CTRL_CAPTURE_FMT		GENMASK(7, 6)
>>    #define  VE_CTRL_AUTO_OR_CURSOR		BIT(8)
>>    #define  VE_CTRL_CLK_INVERSE		BIT(11)
>> @@ -202,6 +200,15 @@ enum {
>>    	VIDEO_CLOCKS_ON,
>>    };
>>    
>> +// for VE_CTRL_CAPTURE_FMT
>> +enum aspeed_video_capture_format {
>> +	VIDEO_CAP_FMT_YUV_STUDIO = 0,
> Maybe also append `_SWING`?
>
>> +	VIDEO_CAP_FMT_YUV_FULL,
>> +	VIDEO_CAP_FMT_RGB,
>> +	VIDEO_CAP_FMT_GRAY,
>> +	VIDEO_CAP_FMT_MAX
>> +};
>> +
>>    struct aspeed_video_addr {
>>    	unsigned int size;
>>    	dma_addr_t dma;
>> @@ -1089,7 +1096,8 @@ static void aspeed_video_init_regs(struct aspeed_video *video)
>>    	u32 comp_ctrl = VE_COMP_CTRL_RSVD |
>>    		FIELD_PREP(VE_COMP_CTRL_DCT_LUM, video->jpeg_quality) |
>>    		FIELD_PREP(VE_COMP_CTRL_DCT_CHR, video->jpeg_quality | 0x10);
>> -	u32 ctrl = VE_CTRL_AUTO_OR_CURSOR;
>> +	u32 ctrl = VE_CTRL_AUTO_OR_CURSOR |
>> +		FIELD_PREP(VE_CTRL_CAPTURE_FMT, VIDEO_CAP_FMT_YUV_FULL);
>>    	u32 seq_ctrl = video->jpeg_mode;
>>    
>>    	if (video->frame_rate)
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
>
>
> Kind regards,
>
> Paul

-- 
Best Regards
Jammy


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-02-10  1:04 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-09  8:42 [PATCH v2] media: aspeed: Use full swing as JFIF to fix incorrect color Jammy Huang
2022-02-09  9:06 ` Paul Menzel
2022-02-10  1:03   ` Jammy Huang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).