[PATCH -next 0/2] media: aspeed: refine mode detection flow

[PATCH -next 0/2] media: aspeed: refine mode detection flow

[Jae Hyun Yoo]

This patch series refines mode detection flow by adding and fixing of
hsync/vsync polarity setting register handling. Please review this change.

Thanks,
-Jae

Jae Hyun Yoo (2):
  media: aspeed: refine hsync/vsync polarity setting logic
  media: aspeed: set hsync and vsync polarities to normal before
    starting mode detection

 drivers/media/platform/aspeed-video.c | 45 ++++++++++++++-------------
 1 file changed, 23 insertions(+), 22 deletions(-)

-- 
2.23.0

[PATCH -next 1/2] media: aspeed: refine hsync/vsync polarity setting logic

[Jae Hyun Yoo]

This commit refines hsync/vsync polarity setting logic by making
also clearing register bits possible based on probed sync state
accordingly.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 drivers/media/platform/aspeed-video.c | 43 +++++++++++++--------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index eb12f3793062..8f77079da55a 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -614,7 +614,7 @@ static void aspeed_video_check_and_set_polarity(struct aspeed_video *video)
 	int i;
 	int hsync_counter = 0;
 	int vsync_counter = 0;
-	u32 sts;
+	u32 sts, ctrl;
 
 	for (i = 0; i < NUM_POLARITY_CHECKS; ++i) {
 		sts = aspeed_video_read(video, VE_MODE_DETECT_STATUS);
@@ -629,30 +629,29 @@ static void aspeed_video_check_and_set_polarity(struct aspeed_video *video)
 			hsync_counter++;
 	}
 
-	if (hsync_counter < 0 || vsync_counter < 0) {
-		u32 ctrl = 0;
+	ctrl = aspeed_video_read(video, VE_CTRL);
 
-		if (hsync_counter < 0) {
-			ctrl = VE_CTRL_HSYNC_POL;
-			video->detected_timings.polarities &=
-				~V4L2_DV_HSYNC_POS_POL;
-		} else {
-			video->detected_timings.polarities |=
-				V4L2_DV_HSYNC_POS_POL;
-		}
-
-		if (vsync_counter < 0) {
-			ctrl = VE_CTRL_VSYNC_POL;
-			video->detected_timings.polarities &=
-				~V4L2_DV_VSYNC_POS_POL;
-		} else {
-			video->detected_timings.polarities |=
-				V4L2_DV_VSYNC_POS_POL;
-		}
+	if (hsync_counter < 0) {
+		ctrl |= VE_CTRL_HSYNC_POL;
+		video->detected_timings.polarities &=
+			~V4L2_DV_HSYNC_POS_POL;
+	} else {
+		ctrl &= ~VE_CTRL_HSYNC_POL;
+		video->detected_timings.polarities |=
+			V4L2_DV_HSYNC_POS_POL;
+	}
 
-		if (ctrl)
-			aspeed_video_update(video, VE_CTRL, 0, ctrl);
+	if (vsync_counter < 0) {
+		ctrl |= VE_CTRL_VSYNC_POL;
+		video->detected_timings.polarities &=
+			~V4L2_DV_VSYNC_POS_POL;
+	} else {
+		ctrl &= ~VE_CTRL_VSYNC_POL;
+		video->detected_timings.polarities |=
+			V4L2_DV_VSYNC_POS_POL;
 	}
+
+	aspeed_video_write(video, VE_CTRL, ctrl);
 }
 
 static bool aspeed_video_alloc_buf(struct aspeed_video *video,
-- 
2.23.0

Re: [PATCH -next 1/2] media: aspeed: refine hsync/vsync polarity setting logic

[Andrew Jeffery]

On Wed, 11 Sep 2019, at 04:37, Jae Hyun Yoo wrote:
> This commit refines hsync/vsync polarity setting logic by making
> also clearing register bits possible based on probed sync state
> accordingly.

That was tough to parse, but I think I understand. Trying to rephrase:

Enable clearing of hsync/vsync plarity bits based on probed sync state.

What was the issue that drove the change? Do you know why it was done
the way it was prior to this patch?

Andrew

Re: [PATCH -next 1/2] media: aspeed: refine hsync/vsync polarity setting logic

[Jae Hyun Yoo]

Hi Andrew,

On 9/11/2019 10:33 PM, Andrew Jeffery wrote:
> On Wed, 11 Sep 2019, at 04:37, Jae Hyun Yoo wrote:
>> This commit refines hsync/vsync polarity setting logic by making
>> also clearing register bits possible based on probed sync state
>> accordingly.
> 
> That was tough to parse, but I think I understand. Trying to rephrase:
> 
> Enable clearing of hsync/vsync plarity bits based on probed sync state.

Correct.

> What was the issue that drove the change? Do you know why it was done
> the way it was prior to this patch?

Because this driver detects weird resolutions sometimes. Investigated
that it uses
     aspeed_video_update(video, VE_CTRL, 0, ctrl);
so only setting of polarity bits is available. Means that clearing of
the bits isn't available so it can't set back the polarities to normal.

To fix the issue, this patch makes it use
     aspeed_video_write(video, VE_CTRL, ctrl);
instead of above one with adding bit masking code changes.

Thanks,
Jae

[PATCH -next 2/2] media: aspeed: set hsync and vsync polarities to normal before starting mode detection

[Jae Hyun Yoo]

Sometimes it detects a weird resolution such as 1024x287 when the
actual resolution is 1024x768. To resolve such an issue, this
commit adds clearing for hsync and vsync polarity register bits
at the beginning of the first mode detection. This is recommended
in the datasheet.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 drivers/media/platform/aspeed-video.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 8f77079da55a..929b3a5b8849 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -740,6 +740,8 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
 		}
 
 		set_bit(VIDEO_RES_DETECT, &video->flags);
+		aspeed_video_update(video, VE_CTRL,
+				    VE_CTRL_VSYNC_POL | VE_CTRL_HSYNC_POL, 0);
 		aspeed_video_enable_mode_detect(video);
 
 		rc = wait_event_interruptible_timeout(video->wait,
-- 
2.23.0

Re: [PATCH -next 2/2] media: aspeed: set hsync and vsync polarities to normal before starting mode detection

[Andrew Jeffery]

On Wed, 11 Sep 2019, at 04:37, Jae Hyun Yoo wrote:
> Sometimes it detects a weird resolution such as 1024x287 when the
> actual resolution is 1024x768. To resolve such an issue, this
> commit adds clearing for hsync and vsync polarity register bits
> at the beginning of the first mode detection. This is recommended
> in the datasheet.

I guess this answers my question on the previous patch's commit
message. Maybe it should be in both?

Andrew

Re: [PATCH -next 2/2] media: aspeed: set hsync and vsync polarities to normal before starting mode detection

[Jae Hyun Yoo]

On 9/11/2019 10:39 PM, Andrew Jeffery wrote:
> 
> 
> On Wed, 11 Sep 2019, at 04:37, Jae Hyun Yoo wrote:
>> Sometimes it detects a weird resolution such as 1024x287 when the
>> actual resolution is 1024x768. To resolve such an issue, this
>> commit adds clearing for hsync and vsync polarity register bits
>> at the beginning of the first mode detection. This is recommended
>> in the datasheet.
> 
> I guess this answers my question on the previous patch's commit
> message. Maybe it should be in both?

I think the previous patch is a bug fix and this one is an enhancement
patch. Better splitting them.

Thanks,
Jae

Re: [PATCH -next 2/2] media: aspeed: set hsync and vsync polarities to normal before starting mode detection

[Andrew Jeffery]

On Fri, 13 Sep 2019, at 02:36, Jae Hyun Yoo wrote:
> On 9/11/2019 10:39 PM, Andrew Jeffery wrote:
> > 
> > 
> > On Wed, 11 Sep 2019, at 04:37, Jae Hyun Yoo wrote:
> >> Sometimes it detects a weird resolution such as 1024x287 when the
> >> actual resolution is 1024x768. To resolve such an issue, this
> >> commit adds clearing for hsync and vsync polarity register bits
> >> at the beginning of the first mode detection. This is recommended
> >> in the datasheet.
> > 
> > I guess this answers my question on the previous patch's commit
> > message. Maybe it should be in both?
> 
> I think the previous patch is a bug fix and this one is an enhancement
> patch. Better splitting them.

I wasn't suggesting squashing the patches, I was suggesting updating
the commit message of the first patch to better justify/explain the
change.

Andrew

Re: [PATCH -next 2/2] media: aspeed: set hsync and vsync polarities to normal before starting mode detection

[Jae Hyun Yoo]

On 9/12/2019 5:18 PM, Andrew Jeffery wrote:
> 
> 
> On Fri, 13 Sep 2019, at 02:36, Jae Hyun Yoo wrote:
>> On 9/11/2019 10:39 PM, Andrew Jeffery wrote:
>>>
>>>
>>> On Wed, 11 Sep 2019, at 04:37, Jae Hyun Yoo wrote:
>>>> Sometimes it detects a weird resolution such as 1024x287 when the
>>>> actual resolution is 1024x768. To resolve such an issue, this
>>>> commit adds clearing for hsync and vsync polarity register bits
>>>> at the beginning of the first mode detection. This is recommended
>>>> in the datasheet.
>>>
>>> I guess this answers my question on the previous patch's commit
>>> message. Maybe it should be in both?
>>
>> I think the previous patch is a bug fix and this one is an enhancement
>> patch. Better splitting them.
> 
> I wasn't suggesting squashing the patches, I was suggesting updating
> the commit message of the first patch to better justify/explain the
> change.

Okay. Will update the commit message of the first patch.

Thanks,
Jae