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
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
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
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
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
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
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
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
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