* [PATCH v2 1/4] media: aspeed: Correct value for h-total-pixels
2021-12-22 8:21 [PATCH 0/4] Correct timing report Jammy Huang
@ 2021-12-22 8:21 ` Jammy Huang
2021-12-22 8:21 ` [PATCH v2 2/4] media: aspeed: Use FIELD_GET to improve readability Jammy Huang
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Jammy Huang @ 2021-12-22 8:21 UTC (permalink / raw)
To: eajames, mchehab, joel, andrew, linux-media, openbmc,
linux-arm-kernel, linux-aspeed, linux-kernel
Previous reg-field, 0x98[11:0], stands for the period of the detected
hsync signal.
Use the correct reg, 0xa0, to get h-total in pixels.
Fixes: d2b4387f3bdf ("media: platform: Add Aspeed Video Engine driver")
Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
v2:
- no update
---
drivers/media/platform/aspeed-video.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index b388bc56ce81..d5f77b205175 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -166,7 +166,7 @@
#define VE_SRC_TB_EDGE_DET_BOT GENMASK(28, VE_SRC_TB_EDGE_DET_BOT_SHF)
#define VE_MODE_DETECT_STATUS 0x098
-#define VE_MODE_DETECT_H_PIXELS GENMASK(11, 0)
+#define VE_MODE_DETECT_H_PERIOD GENMASK(11, 0)
#define VE_MODE_DETECT_V_LINES_SHF 16
#define VE_MODE_DETECT_V_LINES GENMASK(27, VE_MODE_DETECT_V_LINES_SHF)
#define VE_MODE_DETECT_STATUS_VSYNC BIT(28)
@@ -177,6 +177,8 @@
#define VE_SYNC_STATUS_VSYNC_SHF 16
#define VE_SYNC_STATUS_VSYNC GENMASK(27, VE_SYNC_STATUS_VSYNC_SHF)
+#define VE_H_TOTAL_PIXELS 0x0A0
+
#define VE_INTERRUPT_CTRL 0x304
#define VE_INTERRUPT_STATUS 0x308
#define VE_INTERRUPT_MODE_DETECT_WD BIT(0)
@@ -938,6 +940,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
u32 src_lr_edge;
u32 src_tb_edge;
u32 sync;
+ u32 htotal;
struct v4l2_bt_timings *det = &video->detected_timings;
det->width = MIN_WIDTH;
@@ -983,6 +986,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
src_tb_edge = aspeed_video_read(video, VE_SRC_TB_EDGE_DET);
mds = aspeed_video_read(video, VE_MODE_DETECT_STATUS);
sync = aspeed_video_read(video, VE_SYNC_STATUS);
+ htotal = aspeed_video_read(video, VE_H_TOTAL_PIXELS);
video->frame_bottom = (src_tb_edge & VE_SRC_TB_EDGE_DET_BOT) >>
VE_SRC_TB_EDGE_DET_BOT_SHF;
@@ -999,8 +1003,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
VE_SRC_LR_EDGE_DET_RT_SHF;
video->frame_left = src_lr_edge & VE_SRC_LR_EDGE_DET_LEFT;
det->hfrontporch = video->frame_left;
- det->hbackporch = (mds & VE_MODE_DETECT_H_PIXELS) -
- video->frame_right;
+ det->hbackporch = htotal - video->frame_right;
det->hsync = sync & VE_SYNC_STATUS_HSYNC;
if (video->frame_left > video->frame_right)
continue;
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v2 2/4] media: aspeed: Use FIELD_GET to improve readability
2021-12-22 8:21 [PATCH 0/4] Correct timing report Jammy Huang
2021-12-22 8:21 ` [PATCH v2 1/4] media: aspeed: Correct value for h-total-pixels Jammy Huang
@ 2021-12-22 8:21 ` Jammy Huang
2021-12-22 8:21 ` [PATCH v2 3/4] media: aspeed: Correct values for detected timing Jammy Huang
2021-12-22 8:21 ` [PATCH v2 4/4] media: aspeed: Fix timing polarity incorrect Jammy Huang
3 siblings, 0 replies; 12+ messages in thread
From: Jammy Huang @ 2021-12-22 8:21 UTC (permalink / raw)
To: eajames, mchehab, joel, andrew, linux-media, openbmc,
linux-arm-kernel, linux-aspeed, linux-kernel
Use the bitfield macro FIELD_GET, and GENMASK to do the shift and mask in
one go for reg values.
Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
v2:
- Put some codes on one line
---
drivers/media/platform/aspeed-video.c | 31 +++++++++++----------------
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index d5f77b205175..c241038ee27c 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -156,26 +156,22 @@
#define VE_SRC_LR_EDGE_DET_NO_H BIT(13)
#define VE_SRC_LR_EDGE_DET_NO_DISP BIT(14)
#define VE_SRC_LR_EDGE_DET_NO_CLK BIT(15)
-#define VE_SRC_LR_EDGE_DET_RT_SHF 16
-#define VE_SRC_LR_EDGE_DET_RT GENMASK(27, VE_SRC_LR_EDGE_DET_RT_SHF)
+#define VE_SRC_LR_EDGE_DET_RT GENMASK(27, 16)
#define VE_SRC_LR_EDGE_DET_INTERLACE BIT(31)
#define VE_SRC_TB_EDGE_DET 0x094
#define VE_SRC_TB_EDGE_DET_TOP GENMASK(12, 0)
-#define VE_SRC_TB_EDGE_DET_BOT_SHF 16
-#define VE_SRC_TB_EDGE_DET_BOT GENMASK(28, VE_SRC_TB_EDGE_DET_BOT_SHF)
+#define VE_SRC_TB_EDGE_DET_BOT GENMASK(28, 16)
#define VE_MODE_DETECT_STATUS 0x098
#define VE_MODE_DETECT_H_PERIOD GENMASK(11, 0)
-#define VE_MODE_DETECT_V_LINES_SHF 16
-#define VE_MODE_DETECT_V_LINES GENMASK(27, VE_MODE_DETECT_V_LINES_SHF)
+#define VE_MODE_DETECT_V_LINES GENMASK(27, 16)
#define VE_MODE_DETECT_STATUS_VSYNC BIT(28)
#define VE_MODE_DETECT_STATUS_HSYNC BIT(29)
#define VE_SYNC_STATUS 0x09c
#define VE_SYNC_STATUS_HSYNC GENMASK(11, 0)
-#define VE_SYNC_STATUS_VSYNC_SHF 16
-#define VE_SYNC_STATUS_VSYNC GENMASK(27, VE_SYNC_STATUS_VSYNC_SHF)
+#define VE_SYNC_STATUS_VSYNC GENMASK(27, 16)
#define VE_H_TOTAL_PIXELS 0x0A0
@@ -988,23 +984,20 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
sync = aspeed_video_read(video, VE_SYNC_STATUS);
htotal = aspeed_video_read(video, VE_H_TOTAL_PIXELS);
- video->frame_bottom = (src_tb_edge & VE_SRC_TB_EDGE_DET_BOT) >>
- VE_SRC_TB_EDGE_DET_BOT_SHF;
- video->frame_top = src_tb_edge & VE_SRC_TB_EDGE_DET_TOP;
+ video->frame_bottom = FIELD_GET(VE_SRC_TB_EDGE_DET_BOT, src_tb_edge);
+ video->frame_top = FIELD_GET(VE_SRC_TB_EDGE_DET_TOP, src_tb_edge);
det->vfrontporch = video->frame_top;
- det->vbackporch = ((mds & VE_MODE_DETECT_V_LINES) >>
- VE_MODE_DETECT_V_LINES_SHF) - video->frame_bottom;
- det->vsync = (sync & VE_SYNC_STATUS_VSYNC) >>
- VE_SYNC_STATUS_VSYNC_SHF;
+ det->vbackporch = FIELD_GET(VE_MODE_DETECT_V_LINES, mds) -
+ video->frame_bottom;
+ det->vsync = FIELD_GET(VE_SYNC_STATUS_VSYNC, sync);
if (video->frame_top > video->frame_bottom)
continue;
- video->frame_right = (src_lr_edge & VE_SRC_LR_EDGE_DET_RT) >>
- VE_SRC_LR_EDGE_DET_RT_SHF;
- video->frame_left = src_lr_edge & VE_SRC_LR_EDGE_DET_LEFT;
+ video->frame_right = FIELD_GET(VE_SRC_LR_EDGE_DET_RT, src_lr_edge);
+ video->frame_left = FIELD_GET(VE_SRC_LR_EDGE_DET_LEFT, src_lr_edge);
det->hfrontporch = video->frame_left;
det->hbackporch = htotal - video->frame_right;
- det->hsync = sync & VE_SYNC_STATUS_HSYNC;
+ det->hsync = FIELD_GET(VE_SYNC_STATUS_HSYNC, sync);
if (video->frame_left > video->frame_right)
continue;
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 3/4] media: aspeed: Correct values for detected timing
2021-12-22 8:21 [PATCH 0/4] Correct timing report Jammy Huang
2021-12-22 8:21 ` [PATCH v2 1/4] media: aspeed: Correct value for h-total-pixels Jammy Huang
2021-12-22 8:21 ` [PATCH v2 2/4] media: aspeed: Use FIELD_GET to improve readability Jammy Huang
@ 2021-12-22 8:21 ` Jammy Huang
2022-01-20 12:31 ` Hans Verkuil
2021-12-22 8:21 ` [PATCH v2 4/4] media: aspeed: Fix timing polarity incorrect Jammy Huang
3 siblings, 1 reply; 12+ messages in thread
From: Jammy Huang @ 2021-12-22 8:21 UTC (permalink / raw)
To: eajames, mchehab, joel, andrew, linux-media, openbmc,
linux-arm-kernel, linux-aspeed, linux-kernel
Correct timing's fp/sync/bp value based on the information below.
It should be noticed that the calculation formula should be changed
per sync polarity.
The sequence of signal: sync - backporch - video data - frontporch
The following registers start counting from sync's rising edge:
1. VR090: frame edge's left and right
2. VR094: frame edge's top and bottom
3. VR09C: counting from sync's rising edge to falling edge
[Vertical timing]
+--+ +-------------------+ +--+
| | | v i d e o | | |
+--+ +-----+ +-----+ +---+
vsync+--+
frame_top+--------+
frame_bottom+----------------------------+
+-------------------+
| v i d e o |
+--+ +-----+ +-----+ +---+
| | | |
+--+ +--+
vsync+-------------------------------+
frame_top+-----+
frame_bottom+-------------------------+
[Horizontal timing]
+--+ +-------------------+ +--+
| | | v i d e o | | |
+--+ +-----+ +-----+ +---+
hsync+--+
frame_left+--------+
frame_right+----------------------------+
+-------------------+
| v i d e o |
+--+ +-----+ +-----+ +---+
| | | |
+--+ +--+
hsync+-------------------------------+
frame_left+-----+
frame_right+-------------------------+
Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
---
v2:
- Code refined per Joel's suggestion
- Update commit message to have name matching variable
---
drivers/media/platform/aspeed-video.c | 30 ++++++++++++++++++++-------
1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index c241038ee27c..7c50567f5ab0 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -936,7 +936,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
u32 src_lr_edge;
u32 src_tb_edge;
u32 sync;
- u32 htotal;
+ u32 htotal, vtotal, vsync, hsync;
struct v4l2_bt_timings *det = &video->detected_timings;
det->width = MIN_WIDTH;
@@ -983,21 +983,35 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
mds = aspeed_video_read(video, VE_MODE_DETECT_STATUS);
sync = aspeed_video_read(video, VE_SYNC_STATUS);
htotal = aspeed_video_read(video, VE_H_TOTAL_PIXELS);
+ vtotal = FIELD_GET(VE_MODE_DETECT_V_LINES, mds);
+ vsync = FIELD_GET(VE_SYNC_STATUS_VSYNC, sync);
+ hsync = FIELD_GET(VE_SYNC_STATUS_HSYNC, sync);
video->frame_bottom = FIELD_GET(VE_SRC_TB_EDGE_DET_BOT, src_tb_edge);
video->frame_top = FIELD_GET(VE_SRC_TB_EDGE_DET_TOP, src_tb_edge);
- det->vfrontporch = video->frame_top;
- det->vbackporch = FIELD_GET(VE_MODE_DETECT_V_LINES, mds) -
- video->frame_bottom;
- det->vsync = FIELD_GET(VE_SYNC_STATUS_VSYNC, sync);
+ if (det->polarities & V4L2_DV_VSYNC_POS_POL) {
+ det->vbackporch = video->frame_top - vsync;
+ det->vfrontporch = vtotal - video->frame_bottom;
+ det->vsync = vsync;
+ } else {
+ det->vbackporch = video->frame_top;
+ det->vfrontporch = vsync - video->frame_bottom;
+ det->vsync = vtotal - vsync;
+ }
if (video->frame_top > video->frame_bottom)
continue;
video->frame_right = FIELD_GET(VE_SRC_LR_EDGE_DET_RT, src_lr_edge);
video->frame_left = FIELD_GET(VE_SRC_LR_EDGE_DET_LEFT, src_lr_edge);
- det->hfrontporch = video->frame_left;
- det->hbackporch = htotal - video->frame_right;
- det->hsync = FIELD_GET(VE_SYNC_STATUS_HSYNC, sync);
+ if (det->polarities & V4L2_DV_HSYNC_POS_POL) {
+ det->hbackporch = video->frame_left - hsync;
+ det->hfrontporch = htotal - video->frame_right;
+ det->hsync = hsync;
+ } else {
+ det->hbackporch = video->frame_left;
+ det->hfrontporch = hsync - video->frame_right;
+ det->hsync = htotal - hsync;
+ }
if (video->frame_left > video->frame_right)
continue;
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 3/4] media: aspeed: Correct values for detected timing
2021-12-22 8:21 ` [PATCH v2 3/4] media: aspeed: Correct values for detected timing Jammy Huang
@ 2022-01-20 12:31 ` Hans Verkuil
2022-01-21 6:01 ` Jammy Huang
0 siblings, 1 reply; 12+ messages in thread
From: Hans Verkuil @ 2022-01-20 12:31 UTC (permalink / raw)
To: Jammy Huang, eajames, mchehab, joel, andrew, linux-media,
openbmc, linux-arm-kernel, linux-aspeed, linux-kernel
Hi Jammy,
I just want to double check this: I assume you have tested this with the
various polarity combinations?
I ask because I've never seen this before in any other hardware. The
sync and porch values reported by hardware are always independent from the
polarity, so that's why I am surprised to see this.
Same for the next patch (4/4).
Regards,
Hans
On 12/22/21 09:21, Jammy Huang wrote:
> Correct timing's fp/sync/bp value based on the information below.
> It should be noticed that the calculation formula should be changed
> per sync polarity.
>
> The sequence of signal: sync - backporch - video data - frontporch
>
> The following registers start counting from sync's rising edge:
> 1. VR090: frame edge's left and right
> 2. VR094: frame edge's top and bottom
> 3. VR09C: counting from sync's rising edge to falling edge
>
> [Vertical timing]
> +--+ +-------------------+ +--+
> | | | v i d e o | | |
> +--+ +-----+ +-----+ +---+
>
> vsync+--+
> frame_top+--------+
> frame_bottom+----------------------------+
>
> +-------------------+
> | v i d e o |
> +--+ +-----+ +-----+ +---+
> | | | |
> +--+ +--+
> vsync+-------------------------------+
> frame_top+-----+
> frame_bottom+-------------------------+
>
> [Horizontal timing]
> +--+ +-------------------+ +--+
> | | | v i d e o | | |
> +--+ +-----+ +-----+ +---+
>
> hsync+--+
> frame_left+--------+
> frame_right+----------------------------+
>
> +-------------------+
> | v i d e o |
> +--+ +-----+ +-----+ +---+
> | | | |
> +--+ +--+
> hsync+-------------------------------+
> frame_left+-----+
> frame_right+-------------------------+
>
> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
> ---
> v2:
> - Code refined per Joel's suggestion
> - Update commit message to have name matching variable
> ---
> drivers/media/platform/aspeed-video.c | 30 ++++++++++++++++++++-------
> 1 file changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
> index c241038ee27c..7c50567f5ab0 100644
> --- a/drivers/media/platform/aspeed-video.c
> +++ b/drivers/media/platform/aspeed-video.c
> @@ -936,7 +936,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
> u32 src_lr_edge;
> u32 src_tb_edge;
> u32 sync;
> - u32 htotal;
> + u32 htotal, vtotal, vsync, hsync;
> struct v4l2_bt_timings *det = &video->detected_timings;
>
> det->width = MIN_WIDTH;
> @@ -983,21 +983,35 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
> mds = aspeed_video_read(video, VE_MODE_DETECT_STATUS);
> sync = aspeed_video_read(video, VE_SYNC_STATUS);
> htotal = aspeed_video_read(video, VE_H_TOTAL_PIXELS);
> + vtotal = FIELD_GET(VE_MODE_DETECT_V_LINES, mds);
> + vsync = FIELD_GET(VE_SYNC_STATUS_VSYNC, sync);
> + hsync = FIELD_GET(VE_SYNC_STATUS_HSYNC, sync);
>
> video->frame_bottom = FIELD_GET(VE_SRC_TB_EDGE_DET_BOT, src_tb_edge);
> video->frame_top = FIELD_GET(VE_SRC_TB_EDGE_DET_TOP, src_tb_edge);
> - det->vfrontporch = video->frame_top;
> - det->vbackporch = FIELD_GET(VE_MODE_DETECT_V_LINES, mds) -
> - video->frame_bottom;
> - det->vsync = FIELD_GET(VE_SYNC_STATUS_VSYNC, sync);
> + if (det->polarities & V4L2_DV_VSYNC_POS_POL) {
> + det->vbackporch = video->frame_top - vsync;
> + det->vfrontporch = vtotal - video->frame_bottom;
> + det->vsync = vsync;
> + } else {
> + det->vbackporch = video->frame_top;
> + det->vfrontporch = vsync - video->frame_bottom;
> + det->vsync = vtotal - vsync;
> + }
> if (video->frame_top > video->frame_bottom)
> continue;
>
> video->frame_right = FIELD_GET(VE_SRC_LR_EDGE_DET_RT, src_lr_edge);
> video->frame_left = FIELD_GET(VE_SRC_LR_EDGE_DET_LEFT, src_lr_edge);
> - det->hfrontporch = video->frame_left;
> - det->hbackporch = htotal - video->frame_right;
> - det->hsync = FIELD_GET(VE_SYNC_STATUS_HSYNC, sync);
> + if (det->polarities & V4L2_DV_HSYNC_POS_POL) {
> + det->hbackporch = video->frame_left - hsync;
> + det->hfrontporch = htotal - video->frame_right;
> + det->hsync = hsync;
> + } else {
> + det->hbackporch = video->frame_left;
> + det->hfrontporch = hsync - video->frame_right;
> + det->hsync = htotal - hsync;
> + }
> if (video->frame_left > video->frame_right)
> continue;
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2 3/4] media: aspeed: Correct values for detected timing
2022-01-20 12:31 ` Hans Verkuil
@ 2022-01-21 6:01 ` Jammy Huang
2022-01-21 7:28 ` Hans Verkuil
0 siblings, 1 reply; 12+ messages in thread
From: Jammy Huang @ 2022-01-21 6:01 UTC (permalink / raw)
To: Hans Verkuil, eajames, mchehab, joel, andrew, linux-media,
openbmc, linux-arm-kernel, linux-aspeed, linux-kernel
Hi Hans,
Yes, this is a weird part of our hardware.
Because it uses the rising edge of the sync to start counting, an
additional calculation
is needed to get the exact value of the timings.
This problem was found when I was debugging the timing detection
unstable problem.
Reards,
Jammy
On 2022/1/20 下午 08:31, Hans Verkuil wrote:
> Hi Jammy,
>
> I just want to double check this: I assume you have tested this with the
> various polarity combinations?
>
> I ask because I've never seen this before in any other hardware. The
> sync and porch values reported by hardware are always independent from the
> polarity, so that's why I am surprised to see this.
>
> Same for the next patch (4/4).
>
> Regards,
>
> Hans
>
> On 12/22/21 09:21, Jammy Huang wrote:
>> Correct timing's fp/sync/bp value based on the information below.
>> It should be noticed that the calculation formula should be changed
>> per sync polarity.
>>
>> The sequence of signal: sync - backporch - video data - frontporch
>>
>> The following registers start counting from sync's rising edge:
>> 1. VR090: frame edge's left and right
>> 2. VR094: frame edge's top and bottom
>> 3. VR09C: counting from sync's rising edge to falling edge
>>
>> [Vertical timing]
>> +--+ +-------------------+ +--+
>> | | | v i d e o | | |
>> +--+ +-----+ +-----+ +---+
>>
>> vsync+--+
>> frame_top+--------+
>> frame_bottom+----------------------------+
>>
>> +-------------------+
>> | v i d e o |
>> +--+ +-----+ +-----+ +---+
>> | | | |
>> +--+ +--+
>> vsync+-------------------------------+
>> frame_top+-----+
>> frame_bottom+-------------------------+
>>
>> [Horizontal timing]
>> +--+ +-------------------+ +--+
>> | | | v i d e o | | |
>> +--+ +-----+ +-----+ +---+
>>
>> hsync+--+
>> frame_left+--------+
>> frame_right+----------------------------+
>>
>> +-------------------+
>> | v i d e o |
>> +--+ +-----+ +-----+ +---+
>> | | | |
>> +--+ +--+
>> hsync+-------------------------------+
>> frame_left+-----+
>> frame_right+-------------------------+
>>
>> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
>> ---
>> v2:
>> - Code refined per Joel's suggestion
>> - Update commit message to have name matching variable
>> ---
>> drivers/media/platform/aspeed-video.c | 30 ++++++++++++++++++++-------
>> 1 file changed, 22 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
>> index c241038ee27c..7c50567f5ab0 100644
>> --- a/drivers/media/platform/aspeed-video.c
>> +++ b/drivers/media/platform/aspeed-video.c
>> @@ -936,7 +936,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
>> u32 src_lr_edge;
>> u32 src_tb_edge;
>> u32 sync;
>> - u32 htotal;
>> + u32 htotal, vtotal, vsync, hsync;
>> struct v4l2_bt_timings *det = &video->detected_timings;
>>
>> det->width = MIN_WIDTH;
>> @@ -983,21 +983,35 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
>> mds = aspeed_video_read(video, VE_MODE_DETECT_STATUS);
>> sync = aspeed_video_read(video, VE_SYNC_STATUS);
>> htotal = aspeed_video_read(video, VE_H_TOTAL_PIXELS);
>> + vtotal = FIELD_GET(VE_MODE_DETECT_V_LINES, mds);
>> + vsync = FIELD_GET(VE_SYNC_STATUS_VSYNC, sync);
>> + hsync = FIELD_GET(VE_SYNC_STATUS_HSYNC, sync);
>>
>> video->frame_bottom = FIELD_GET(VE_SRC_TB_EDGE_DET_BOT, src_tb_edge);
>> video->frame_top = FIELD_GET(VE_SRC_TB_EDGE_DET_TOP, src_tb_edge);
>> - det->vfrontporch = video->frame_top;
>> - det->vbackporch = FIELD_GET(VE_MODE_DETECT_V_LINES, mds) -
>> - video->frame_bottom;
>> - det->vsync = FIELD_GET(VE_SYNC_STATUS_VSYNC, sync);
>> + if (det->polarities & V4L2_DV_VSYNC_POS_POL) {
>> + det->vbackporch = video->frame_top - vsync;
>> + det->vfrontporch = vtotal - video->frame_bottom;
>> + det->vsync = vsync;
>> + } else {
>> + det->vbackporch = video->frame_top;
>> + det->vfrontporch = vsync - video->frame_bottom;
>> + det->vsync = vtotal - vsync;
>> + }
>> if (video->frame_top > video->frame_bottom)
>> continue;
>>
>> video->frame_right = FIELD_GET(VE_SRC_LR_EDGE_DET_RT, src_lr_edge);
>> video->frame_left = FIELD_GET(VE_SRC_LR_EDGE_DET_LEFT, src_lr_edge);
>> - det->hfrontporch = video->frame_left;
>> - det->hbackporch = htotal - video->frame_right;
>> - det->hsync = FIELD_GET(VE_SYNC_STATUS_HSYNC, sync);
>> + if (det->polarities & V4L2_DV_HSYNC_POS_POL) {
>> + det->hbackporch = video->frame_left - hsync;
>> + det->hfrontporch = htotal - video->frame_right;
>> + det->hsync = hsync;
>> + } else {
>> + det->hbackporch = video->frame_left;
>> + det->hfrontporch = hsync - video->frame_right;
>> + det->hsync = htotal - hsync;
>> + }
>> if (video->frame_left > video->frame_right)
>> continue;
>>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2 3/4] media: aspeed: Correct values for detected timing
2022-01-21 6:01 ` Jammy Huang
@ 2022-01-21 7:28 ` Hans Verkuil
0 siblings, 0 replies; 12+ messages in thread
From: Hans Verkuil @ 2022-01-21 7:28 UTC (permalink / raw)
To: Jammy Huang, eajames, mchehab, joel, andrew, linux-media,
openbmc, linux-arm-kernel, linux-aspeed, linux-kernel
Hi Jammy,
On 1/21/22 07:01, Jammy Huang wrote:
> Hi Hans,
>
> Yes, this is a weird part of our hardware.
> Because it uses the rising edge of the sync to start counting, an additional calculation
> is needed to get the exact value of the timings.
>
> This problem was found when I was debugging the timing detection unstable problem.
OK, thank you for the confirmation.
I would like a v3 where basically the explanation you made in the cover letter
is copied to the code, probably as a comment block just before aspeed_video_get_resolution().
This is unusual enough that it should be documented well in the code.
Regards,
Hans
>
> Reards,
>
> Jammy
>
> On 2022/1/20 下午 08:31, Hans Verkuil wrote:
>> Hi Jammy,
>>
>> I just want to double check this: I assume you have tested this with the
>> various polarity combinations?
>>
>> I ask because I've never seen this before in any other hardware. The
>> sync and porch values reported by hardware are always independent from the
>> polarity, so that's why I am surprised to see this.
>>
>> Same for the next patch (4/4).
>>
>> Regards,
>>
>> Hans
>>
>> On 12/22/21 09:21, Jammy Huang wrote:
>>> Correct timing's fp/sync/bp value based on the information below.
>>> It should be noticed that the calculation formula should be changed
>>> per sync polarity.
>>>
>>> The sequence of signal: sync - backporch - video data - frontporch
>>>
>>> The following registers start counting from sync's rising edge:
>>> 1. VR090: frame edge's left and right
>>> 2. VR094: frame edge's top and bottom
>>> 3. VR09C: counting from sync's rising edge to falling edge
>>>
>>> [Vertical timing]
>>> +--+ +-------------------+ +--+
>>> | | | v i d e o | | |
>>> +--+ +-----+ +-----+ +---+
>>>
>>> vsync+--+
>>> frame_top+--------+
>>> frame_bottom+----------------------------+
>>>
>>> +-------------------+
>>> | v i d e o |
>>> +--+ +-----+ +-----+ +---+
>>> | | | |
>>> +--+ +--+
>>> vsync+-------------------------------+
>>> frame_top+-----+
>>> frame_bottom+-------------------------+
>>>
>>> [Horizontal timing]
>>> +--+ +-------------------+ +--+
>>> | | | v i d e o | | |
>>> +--+ +-----+ +-----+ +---+
>>>
>>> hsync+--+
>>> frame_left+--------+
>>> frame_right+----------------------------+
>>>
>>> +-------------------+
>>> | v i d e o |
>>> +--+ +-----+ +-----+ +---+
>>> | | | |
>>> +--+ +--+
>>> hsync+-------------------------------+
>>> frame_left+-----+
>>> frame_right+-------------------------+
>>>
>>> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
>>> ---
>>> v2:
>>> - Code refined per Joel's suggestion
>>> - Update commit message to have name matching variable
>>> ---
>>> drivers/media/platform/aspeed-video.c | 30 ++++++++++++++++++++-------
>>> 1 file changed, 22 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
>>> index c241038ee27c..7c50567f5ab0 100644
>>> --- a/drivers/media/platform/aspeed-video.c
>>> +++ b/drivers/media/platform/aspeed-video.c
>>> @@ -936,7 +936,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
>>> u32 src_lr_edge;
>>> u32 src_tb_edge;
>>> u32 sync;
>>> - u32 htotal;
>>> + u32 htotal, vtotal, vsync, hsync;
>>> struct v4l2_bt_timings *det = &video->detected_timings;
>>> det->width = MIN_WIDTH;
>>> @@ -983,21 +983,35 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
>>> mds = aspeed_video_read(video, VE_MODE_DETECT_STATUS);
>>> sync = aspeed_video_read(video, VE_SYNC_STATUS);
>>> htotal = aspeed_video_read(video, VE_H_TOTAL_PIXELS);
>>> + vtotal = FIELD_GET(VE_MODE_DETECT_V_LINES, mds);
>>> + vsync = FIELD_GET(VE_SYNC_STATUS_VSYNC, sync);
>>> + hsync = FIELD_GET(VE_SYNC_STATUS_HSYNC, sync);
>>> video->frame_bottom = FIELD_GET(VE_SRC_TB_EDGE_DET_BOT, src_tb_edge);
>>> video->frame_top = FIELD_GET(VE_SRC_TB_EDGE_DET_TOP, src_tb_edge);
>>> - det->vfrontporch = video->frame_top;
>>> - det->vbackporch = FIELD_GET(VE_MODE_DETECT_V_LINES, mds) -
>>> - video->frame_bottom;
>>> - det->vsync = FIELD_GET(VE_SYNC_STATUS_VSYNC, sync);
>>> + if (det->polarities & V4L2_DV_VSYNC_POS_POL) {
>>> + det->vbackporch = video->frame_top - vsync;
>>> + det->vfrontporch = vtotal - video->frame_bottom;
>>> + det->vsync = vsync;
>>> + } else {
>>> + det->vbackporch = video->frame_top;
>>> + det->vfrontporch = vsync - video->frame_bottom;
>>> + det->vsync = vtotal - vsync;
>>> + }
>>> if (video->frame_top > video->frame_bottom)
>>> continue;
>>> video->frame_right = FIELD_GET(VE_SRC_LR_EDGE_DET_RT, src_lr_edge);
>>> video->frame_left = FIELD_GET(VE_SRC_LR_EDGE_DET_LEFT, src_lr_edge);
>>> - det->hfrontporch = video->frame_left;
>>> - det->hbackporch = htotal - video->frame_right;
>>> - det->hsync = FIELD_GET(VE_SYNC_STATUS_HSYNC, sync);
>>> + if (det->polarities & V4L2_DV_HSYNC_POS_POL) {
>>> + det->hbackporch = video->frame_left - hsync;
>>> + det->hfrontporch = htotal - video->frame_right;
>>> + det->hsync = hsync;
>>> + } else {
>>> + det->hbackporch = video->frame_left;
>>> + det->hfrontporch = hsync - video->frame_right;
>>> + det->hsync = htotal - hsync;
>>> + }
>>> if (video->frame_left > video->frame_right)
>>> continue;
>>>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 4/4] media: aspeed: Fix timing polarity incorrect
2021-12-22 8:21 [PATCH 0/4] Correct timing report Jammy Huang
` (2 preceding siblings ...)
2021-12-22 8:21 ` [PATCH v2 3/4] media: aspeed: Correct values for detected timing Jammy Huang
@ 2021-12-22 8:21 ` Jammy Huang
2022-01-21 7:30 ` Hans Verkuil
3 siblings, 1 reply; 12+ messages in thread
From: Jammy Huang @ 2021-12-22 8:21 UTC (permalink / raw)
To: eajames, mchehab, joel, andrew, linux-media, openbmc,
linux-arm-kernel, linux-aspeed, linux-kernel
This is a workaround for sync polarity unstable.
Sync value get by VR09C counts from sync's rising edge, which means
sync's polarity is negative if sync value is bigger than total/2.
Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
---
v2:
- Use 'total/2' rather than 'total<<1'
- Update comment
---
drivers/media/platform/aspeed-video.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 7c50567f5ab0..c3e3343d91e1 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -989,6 +989,16 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
video->frame_bottom = FIELD_GET(VE_SRC_TB_EDGE_DET_BOT, src_tb_edge);
video->frame_top = FIELD_GET(VE_SRC_TB_EDGE_DET_TOP, src_tb_edge);
+
+ /*
+ * This is a workaround for polarity detection when the sync
+ * value is larger than half.
+ */
+ if (vsync > (vtotal / 2))
+ det->polarities &= ~V4L2_DV_VSYNC_POS_POL;
+ else
+ det->polarities |= V4L2_DV_VSYNC_POS_POL;
+
if (det->polarities & V4L2_DV_VSYNC_POS_POL) {
det->vbackporch = video->frame_top - vsync;
det->vfrontporch = vtotal - video->frame_bottom;
@@ -1003,6 +1013,16 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
video->frame_right = FIELD_GET(VE_SRC_LR_EDGE_DET_RT, src_lr_edge);
video->frame_left = FIELD_GET(VE_SRC_LR_EDGE_DET_LEFT, src_lr_edge);
+
+ /*
+ * This is a workaround for polarity detection when the sync
+ * value is larger than half.
+ */
+ if (hsync > (htotal / 2))
+ det->polarities &= ~V4L2_DV_HSYNC_POS_POL;
+ else
+ det->polarities |= V4L2_DV_HSYNC_POS_POL;
+
if (det->polarities & V4L2_DV_HSYNC_POS_POL) {
det->hbackporch = video->frame_left - hsync;
det->hfrontporch = htotal - video->frame_right;
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 4/4] media: aspeed: Fix timing polarity incorrect
2021-12-22 8:21 ` [PATCH v2 4/4] media: aspeed: Fix timing polarity incorrect Jammy Huang
@ 2022-01-21 7:30 ` Hans Verkuil
2022-01-24 2:29 ` Jammy Huang
0 siblings, 1 reply; 12+ messages in thread
From: Hans Verkuil @ 2022-01-21 7:30 UTC (permalink / raw)
To: Jammy Huang, eajames, mchehab, joel, andrew, linux-media,
openbmc, linux-arm-kernel, linux-aspeed, linux-kernel
On 12/22/21 09:21, Jammy Huang wrote:
> This is a workaround for sync polarity unstable.
> Sync value get by VR09C counts from sync's rising edge, which means
> sync's polarity is negative if sync value is bigger than total/2.
Do you have an example of such a format, or is this mostly theoretical?
Either provide the example or make a note that it is theoretical.
Regards,
Hans
>
> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
> ---
> v2:
> - Use 'total/2' rather than 'total<<1'
> - Update comment
> ---
> drivers/media/platform/aspeed-video.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
> index 7c50567f5ab0..c3e3343d91e1 100644
> --- a/drivers/media/platform/aspeed-video.c
> +++ b/drivers/media/platform/aspeed-video.c
> @@ -989,6 +989,16 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
>
> video->frame_bottom = FIELD_GET(VE_SRC_TB_EDGE_DET_BOT, src_tb_edge);
> video->frame_top = FIELD_GET(VE_SRC_TB_EDGE_DET_TOP, src_tb_edge);
> +
> + /*
> + * This is a workaround for polarity detection when the sync
> + * value is larger than half.
> + */
> + if (vsync > (vtotal / 2))
> + det->polarities &= ~V4L2_DV_VSYNC_POS_POL;
> + else
> + det->polarities |= V4L2_DV_VSYNC_POS_POL;
> +
> if (det->polarities & V4L2_DV_VSYNC_POS_POL) {
> det->vbackporch = video->frame_top - vsync;
> det->vfrontporch = vtotal - video->frame_bottom;
> @@ -1003,6 +1013,16 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
>
> video->frame_right = FIELD_GET(VE_SRC_LR_EDGE_DET_RT, src_lr_edge);
> video->frame_left = FIELD_GET(VE_SRC_LR_EDGE_DET_LEFT, src_lr_edge);
> +
> + /*
> + * This is a workaround for polarity detection when the sync
> + * value is larger than half.
> + */
> + if (hsync > (htotal / 2))
> + det->polarities &= ~V4L2_DV_HSYNC_POS_POL;
> + else
> + det->polarities |= V4L2_DV_HSYNC_POS_POL;
> +
> if (det->polarities & V4L2_DV_HSYNC_POS_POL) {
> det->hbackporch = video->frame_left - hsync;
> det->hfrontporch = htotal - video->frame_right;
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2 4/4] media: aspeed: Fix timing polarity incorrect
2022-01-21 7:30 ` Hans Verkuil
@ 2022-01-24 2:29 ` Jammy Huang
2022-01-24 10:22 ` Hans Verkuil
0 siblings, 1 reply; 12+ messages in thread
From: Jammy Huang @ 2022-01-24 2:29 UTC (permalink / raw)
To: Hans Verkuil, eajames, mchehab, joel, andrew, linux-media,
openbmc, linux-arm-kernel, linux-aspeed, linux-kernel
On 2022/1/21 下午 03:30, Hans Verkuil wrote:
> On 12/22/21 09:21, Jammy Huang wrote:
>> This is a workaround for sync polarity unstable.
>> Sync value get by VR09C counts from sync's rising edge, which means
>> sync's polarity is negative if sync value is bigger than total/2.
> Do you have an example of such a format, or is this mostly theoretical?
>
> Either provide the example or make a note that it is theoretical.
OK, I will update an example in next patch. Let me explain first.
This is a must-be result. Please refer to the graph below as I sent in
3/4 of this
series. For negative sync, sync width equals to back porch + data enable
+ front porch.
Thus, sync would be bigger than 90% of total in most cases.
+-------------------+
| v i d e o |
+--+ +-----+ +-----+ +---+
| | | |
+--+ +--+
vsync+-------------------------------+
frame_top+-----+
frame_bottom+-------------------------+
Following registers are what I got for 1920x1200@60.
1e700090: 07ee206f 04c9001a c4d3efff 04cc001f
1e7000a0: 0000081f 00000000 00000000 00000000
vertical total = 0x4D3 (VR098[27:16]) = 1235
vertical sync = 0x4CC (VR09C[27:16]) = 1228
>
> Regards,
>
> Hans
>
>> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
>> ---
>> v2:
>> - Use 'total/2' rather than 'total<<1'
>> - Update comment
>> ---
>> drivers/media/platform/aspeed-video.c | 20 ++++++++++++++++++++
>> 1 file changed, 20 insertions(+)
>>
>> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
>> index 7c50567f5ab0..c3e3343d91e1 100644
>> --- a/drivers/media/platform/aspeed-video.c
>> +++ b/drivers/media/platform/aspeed-video.c
>> @@ -989,6 +989,16 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
>>
>> video->frame_bottom = FIELD_GET(VE_SRC_TB_EDGE_DET_BOT, src_tb_edge);
>> video->frame_top = FIELD_GET(VE_SRC_TB_EDGE_DET_TOP, src_tb_edge);
>> +
>> + /*
>> + * This is a workaround for polarity detection when the sync
>> + * value is larger than half.
>> + */
>> + if (vsync > (vtotal / 2))
>> + det->polarities &= ~V4L2_DV_VSYNC_POS_POL;
>> + else
>> + det->polarities |= V4L2_DV_VSYNC_POS_POL;
>> +
>> if (det->polarities & V4L2_DV_VSYNC_POS_POL) {
>> det->vbackporch = video->frame_top - vsync;
>> det->vfrontporch = vtotal - video->frame_bottom;
>> @@ -1003,6 +1013,16 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
>>
>> video->frame_right = FIELD_GET(VE_SRC_LR_EDGE_DET_RT, src_lr_edge);
>> video->frame_left = FIELD_GET(VE_SRC_LR_EDGE_DET_LEFT, src_lr_edge);
>> +
>> + /*
>> + * This is a workaround for polarity detection when the sync
>> + * value is larger than half.
>> + */
>> + if (hsync > (htotal / 2))
>> + det->polarities &= ~V4L2_DV_HSYNC_POS_POL;
>> + else
>> + det->polarities |= V4L2_DV_HSYNC_POS_POL;
>> +
>> if (det->polarities & V4L2_DV_HSYNC_POS_POL) {
>> det->hbackporch = video->frame_left - hsync;
>> det->hfrontporch = htotal - video->frame_right;
--
Best Regards
Jammy
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2 4/4] media: aspeed: Fix timing polarity incorrect
2022-01-24 2:29 ` Jammy Huang
@ 2022-01-24 10:22 ` Hans Verkuil
2022-01-25 1:37 ` Jammy Huang
0 siblings, 1 reply; 12+ messages in thread
From: Hans Verkuil @ 2022-01-24 10:22 UTC (permalink / raw)
To: Jammy Huang, eajames, mchehab, joel, andrew, linux-media,
openbmc, linux-arm-kernel, linux-aspeed, linux-kernel
On 24/01/2022 03:29, Jammy Huang wrote:
>
> On 2022/1/21 下午 03:30, Hans Verkuil wrote:
>> On 12/22/21 09:21, Jammy Huang wrote:
>>> This is a workaround for sync polarity unstable.
>>> Sync value get by VR09C counts from sync's rising edge, which means
>>> sync's polarity is negative if sync value is bigger than total/2.
>> Do you have an example of such a format, or is this mostly theoretical?
>>
>> Either provide the example or make a note that it is theoretical.
>
> OK, I will update an example in next patch. Let me explain first.
>
> This is a must-be result. Please refer to the graph below as I sent in 3/4 of this
> series. For negative sync, sync width equals to back porch + data enable + front porch.
> Thus, sync would be bigger than 90% of total in most cases.
Right, I suspected that might be the case.
I think it would be better to combine patches 3 and 4 into a single
patch, since they are closely related and it is actually easier to
understand if it's just a single patch.
Regards,
Hans
>
> +-------------------+
> | v i d e o |
> +--+ +-----+ +-----+ +---+
> | | | |
> +--+ +--+
> vsync+-------------------------------+
> frame_top+-----+
> frame_bottom+-------------------------+
>
>
> Following registers are what I got for 1920x1200@60.
>
> 1e700090: 07ee206f 04c9001a c4d3efff 04cc001f
>
> 1e7000a0: 0000081f 00000000 00000000 00000000
>
> vertical total = 0x4D3 (VR098[27:16]) = 1235
> vertical sync = 0x4CC (VR09C[27:16]) = 1228
>
>>
>> Regards,
>>
>> Hans
>>
>>> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
>>> ---
>>> v2:
>>> - Use 'total/2' rather than 'total<<1'
>>> - Update comment
>>> ---
>>> drivers/media/platform/aspeed-video.c | 20 ++++++++++++++++++++
>>> 1 file changed, 20 insertions(+)
>>>
>>> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
>>> index 7c50567f5ab0..c3e3343d91e1 100644
>>> --- a/drivers/media/platform/aspeed-video.c
>>> +++ b/drivers/media/platform/aspeed-video.c
>>> @@ -989,6 +989,16 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
>>> video->frame_bottom = FIELD_GET(VE_SRC_TB_EDGE_DET_BOT, src_tb_edge);
>>> video->frame_top = FIELD_GET(VE_SRC_TB_EDGE_DET_TOP, src_tb_edge);
>>> +
>>> + /*
>>> + * This is a workaround for polarity detection when the sync
>>> + * value is larger than half.
>>> + */
>>> + if (vsync > (vtotal / 2))
>>> + det->polarities &= ~V4L2_DV_VSYNC_POS_POL;
>>> + else
>>> + det->polarities |= V4L2_DV_VSYNC_POS_POL;
>>> +
>>> if (det->polarities & V4L2_DV_VSYNC_POS_POL) {
>>> det->vbackporch = video->frame_top - vsync;
>>> det->vfrontporch = vtotal - video->frame_bottom;
>>> @@ -1003,6 +1013,16 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
>>> video->frame_right = FIELD_GET(VE_SRC_LR_EDGE_DET_RT, src_lr_edge);
>>> video->frame_left = FIELD_GET(VE_SRC_LR_EDGE_DET_LEFT, src_lr_edge);
>>> +
>>> + /*
>>> + * This is a workaround for polarity detection when the sync
>>> + * value is larger than half.
>>> + */
>>> + if (hsync > (htotal / 2))
>>> + det->polarities &= ~V4L2_DV_HSYNC_POS_POL;
>>> + else
>>> + det->polarities |= V4L2_DV_HSYNC_POS_POL;
>>> +
>>> if (det->polarities & V4L2_DV_HSYNC_POS_POL) {
>>> det->hbackporch = video->frame_left - hsync;
>>> det->hfrontporch = htotal - video->frame_right;
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2 4/4] media: aspeed: Fix timing polarity incorrect
2022-01-24 10:22 ` Hans Verkuil
@ 2022-01-25 1:37 ` Jammy Huang
0 siblings, 0 replies; 12+ messages in thread
From: Jammy Huang @ 2022-01-25 1:37 UTC (permalink / raw)
To: Hans Verkuil, eajames, mchehab, joel, andrew, linux-media,
openbmc, linux-arm-kernel, linux-aspeed, linux-kernel
On 2022/1/24 下午 06:22, Hans Verkuil wrote:
> On 24/01/2022 03:29, Jammy Huang wrote:
>> On 2022/1/21 下午 03:30, Hans Verkuil wrote:
>>> On 12/22/21 09:21, Jammy Huang wrote:
>>>> This is a workaround for sync polarity unstable.
>>>> Sync value get by VR09C counts from sync's rising edge, which means
>>>> sync's polarity is negative if sync value is bigger than total/2.
>>> Do you have an example of such a format, or is this mostly theoretical?
>>>
>>> Either provide the example or make a note that it is theoretical.
>> OK, I will update an example in next patch. Let me explain first.
>>
>> This is a must-be result. Please refer to the graph below as I sent in 3/4 of this
>> series. For negative sync, sync width equals to back porch + data enable + front porch.
>> Thus, sync would be bigger than 90% of total in most cases.
> Right, I suspected that might be the case.
>
> I think it would be better to combine patches 3 and 4 into a single
> patch, since they are closely related and it is actually easier to
> understand if it's just a single patch.
OK, I will combine 3/4 into a single patch in next series.
Thanks.
Best Regards,
Jammy
>
> Regards,
>
> Hans
>
>> +-------------------+
>> | v i d e o |
>> +--+ +-----+ +-----+ +---+
>> | | | |
>> +--+ +--+
>> vsync+-------------------------------+
>> frame_top+-----+
>> frame_bottom+-------------------------+
>>
>>
>> Following registers are what I got for 1920x1200@60.
>>
>> 1e700090: 07ee206f 04c9001a c4d3efff 04cc001f
>>
>> 1e7000a0: 0000081f 00000000 00000000 00000000
>>
>> vertical total = 0x4D3 (VR098[27:16]) = 1235
>> vertical sync = 0x4CC (VR09C[27:16]) = 1228
>>
>>> Regards,
>>>
>>> Hans
>>>
>>>> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
>>>> ---
>>>> v2:
>>>> - Use 'total/2' rather than 'total<<1'
>>>> - Update comment
>>>> ---
>>>> drivers/media/platform/aspeed-video.c | 20 ++++++++++++++++++++
>>>> 1 file changed, 20 insertions(+)
>>>>
>>>> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
>>>> index 7c50567f5ab0..c3e3343d91e1 100644
>>>> --- a/drivers/media/platform/aspeed-video.c
>>>> +++ b/drivers/media/platform/aspeed-video.c
>>>> @@ -989,6 +989,16 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
>>>> video->frame_bottom = FIELD_GET(VE_SRC_TB_EDGE_DET_BOT, src_tb_edge);
>>>> video->frame_top = FIELD_GET(VE_SRC_TB_EDGE_DET_TOP, src_tb_edge);
>>>> +
>>>> + /*
>>>> + * This is a workaround for polarity detection when the sync
>>>> + * value is larger than half.
>>>> + */
>>>> + if (vsync > (vtotal / 2))
>>>> + det->polarities &= ~V4L2_DV_VSYNC_POS_POL;
>>>> + else
>>>> + det->polarities |= V4L2_DV_VSYNC_POS_POL;
>>>> +
>>>> if (det->polarities & V4L2_DV_VSYNC_POS_POL) {
>>>> det->vbackporch = video->frame_top - vsync;
>>>> det->vfrontporch = vtotal - video->frame_bottom;
>>>> @@ -1003,6 +1013,16 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
>>>> video->frame_right = FIELD_GET(VE_SRC_LR_EDGE_DET_RT, src_lr_edge);
>>>> video->frame_left = FIELD_GET(VE_SRC_LR_EDGE_DET_LEFT, src_lr_edge);
>>>> +
>>>> + /*
>>>> + * This is a workaround for polarity detection when the sync
>>>> + * value is larger than half.
>>>> + */
>>>> + if (hsync > (htotal / 2))
>>>> + det->polarities &= ~V4L2_DV_HSYNC_POS_POL;
>>>> + else
>>>> + det->polarities |= V4L2_DV_HSYNC_POS_POL;
>>>> +
>>>> if (det->polarities & V4L2_DV_HSYNC_POS_POL) {
>>>> det->hbackporch = video->frame_left - hsync;
>>>> det->hfrontporch = htotal - video->frame_right;
^ permalink raw reply [flat|nested] 12+ messages in thread