All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] media: aspeed: Fix incorrect resolution detected
@ 2021-11-09  9:54 ` Jammy Huang
  0 siblings, 0 replies; 8+ messages in thread
From: Jammy Huang @ 2021-11-09  9:54 UTC (permalink / raw)
  To: eajames, mchehab, joel, andrew, linux-media, openbmc,
	linux-arm-kernel, linux-aspeed, linux-kernel

During the process of os-installation, detected resolution's height
could be less than the correct one.

Increase min-required-count of stable signal to fix the problem.

Fixes: d2b4387f3bdf ("media: platform: Add Aspeed Video Engine driver")
Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
---
 drivers/media/platform/aspeed-video.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 5da52646b298..625a77ddb479 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -196,6 +196,12 @@
 #define  VE_INTERRUPT_VSYNC_DESC	BIT(11)
 
 #define VE_MODE_DETECT			0x30c
+#define  VE_MODE_DT_HOR_TOLER		GENMASK(31, 28)
+#define  VE_MODE_DT_VER_TOLER		GENMASK(27, 24)
+#define  VE_MODE_DT_HOR_STABLE		GENMASK(23, 20)
+#define  VE_MODE_DT_VER_STABLE		GENMASK(19, 16)
+#define  VE_MODE_DT_EDG_THROD		GENMASK(15, 8)
+
 #define VE_MEM_RESTRICT_START		0x310
 #define VE_MEM_RESTRICT_END		0x314
 
@@ -1199,7 +1205,12 @@ static void aspeed_video_init_regs(struct aspeed_video *video)
 	aspeed_video_write(video, VE_SCALING_FILTER3, 0x00200000);
 
 	/* Set mode detection defaults */
-	aspeed_video_write(video, VE_MODE_DETECT, 0x22666500);
+	aspeed_video_write(video, VE_MODE_DETECT,
+			   FIELD_PREP(VE_MODE_DT_HOR_TOLER, 2) |
+			   FIELD_PREP(VE_MODE_DT_VER_TOLER, 2) |
+			   FIELD_PREP(VE_MODE_DT_HOR_STABLE, 10) |
+			   FIELD_PREP(VE_MODE_DT_VER_STABLE, 10) |
+			   FIELD_PREP(VE_MODE_DT_EDG_THROD, 0x65));
 
 	aspeed_video_write(video, VE_BCD_CTRL, 0);
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH] media: aspeed: Fix incorrect resolution detected
@ 2021-11-09  9:54 ` Jammy Huang
  0 siblings, 0 replies; 8+ messages in thread
From: Jammy Huang @ 2021-11-09  9:54 UTC (permalink / raw)
  To: eajames, mchehab, joel, andrew, linux-media, openbmc,
	linux-arm-kernel, linux-aspeed, linux-kernel

During the process of os-installation, detected resolution's height
could be less than the correct one.

Increase min-required-count of stable signal to fix the problem.

Fixes: d2b4387f3bdf ("media: platform: Add Aspeed Video Engine driver")
Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
---
 drivers/media/platform/aspeed-video.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 5da52646b298..625a77ddb479 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -196,6 +196,12 @@
 #define  VE_INTERRUPT_VSYNC_DESC	BIT(11)
 
 #define VE_MODE_DETECT			0x30c
+#define  VE_MODE_DT_HOR_TOLER		GENMASK(31, 28)
+#define  VE_MODE_DT_VER_TOLER		GENMASK(27, 24)
+#define  VE_MODE_DT_HOR_STABLE		GENMASK(23, 20)
+#define  VE_MODE_DT_VER_STABLE		GENMASK(19, 16)
+#define  VE_MODE_DT_EDG_THROD		GENMASK(15, 8)
+
 #define VE_MEM_RESTRICT_START		0x310
 #define VE_MEM_RESTRICT_END		0x314
 
@@ -1199,7 +1205,12 @@ static void aspeed_video_init_regs(struct aspeed_video *video)
 	aspeed_video_write(video, VE_SCALING_FILTER3, 0x00200000);
 
 	/* Set mode detection defaults */
-	aspeed_video_write(video, VE_MODE_DETECT, 0x22666500);
+	aspeed_video_write(video, VE_MODE_DETECT,
+			   FIELD_PREP(VE_MODE_DT_HOR_TOLER, 2) |
+			   FIELD_PREP(VE_MODE_DT_VER_TOLER, 2) |
+			   FIELD_PREP(VE_MODE_DT_HOR_STABLE, 10) |
+			   FIELD_PREP(VE_MODE_DT_VER_STABLE, 10) |
+			   FIELD_PREP(VE_MODE_DT_EDG_THROD, 0x65));
 
 	aspeed_video_write(video, VE_BCD_CTRL, 0);
 }
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH] media: aspeed: Fix incorrect resolution detected
  2021-11-09  9:54 ` Jammy Huang
  (?)
@ 2021-11-09 11:22   ` Paul Menzel
  -1 siblings, 0 replies; 8+ messages in thread
From: Paul Menzel @ 2021-11-09 11:22 UTC (permalink / raw)
  To: Jammy Huang
  Cc: eajames, mchehab, joel, andrew, linux-media, openbmc,
	linux-arm-kernel, linux-aspeed, linux-kernel

Dear Jammy,


Am 09.11.21 um 10:54 schrieb Jammy Huang:
> During the process of os-installation, detected resolution's height
> could be less than the correct one.

Can you please elaborate? What OS? What resolution was and what 
resolution should have been detected?

> Increase min-required-count of stable signal to fix the problem.

So you do two changes in the patch? First use the bitfield access 
macros, and then change VE_MODE_DT_HOR_STABLE and VE_MODE_DT_VER_STABLE 
from 6 to 10? Is that the amount of iterations? Why 10 and not 20?

How much time do four iterations add?

Sorry for my ignorance, but if you could make it two patches, that’d be 
great.

> Fixes: d2b4387f3bdf ("media: platform: Add Aspeed Video Engine driver")
> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
> ---
>   drivers/media/platform/aspeed-video.c | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
> index 5da52646b298..625a77ddb479 100644
> --- a/drivers/media/platform/aspeed-video.c
> +++ b/drivers/media/platform/aspeed-video.c
> @@ -196,6 +196,12 @@
>   #define  VE_INTERRUPT_VSYNC_DESC	BIT(11)
>   
>   #define VE_MODE_DETECT			0x30c
> +#define  VE_MODE_DT_HOR_TOLER		GENMASK(31, 28)
> +#define  VE_MODE_DT_VER_TOLER		GENMASK(27, 24)
> +#define  VE_MODE_DT_HOR_STABLE		GENMASK(23, 20)
> +#define  VE_MODE_DT_VER_STABLE		GENMASK(19, 16)
> +#define  VE_MODE_DT_EDG_THROD		GENMASK(15, 8)
> +
>   #define VE_MEM_RESTRICT_START		0x310
>   #define VE_MEM_RESTRICT_END		0x314
>   
> @@ -1199,7 +1205,12 @@ static void aspeed_video_init_regs(struct aspeed_video *video)
>   	aspeed_video_write(video, VE_SCALING_FILTER3, 0x00200000);
>   
>   	/* Set mode detection defaults */
> -	aspeed_video_write(video, VE_MODE_DETECT, 0x22666500);
> +	aspeed_video_write(video, VE_MODE_DETECT,
> +			   FIELD_PREP(VE_MODE_DT_HOR_TOLER, 2) |
> +			   FIELD_PREP(VE_MODE_DT_VER_TOLER, 2) |
> +			   FIELD_PREP(VE_MODE_DT_HOR_STABLE, 10) |
> +			   FIELD_PREP(VE_MODE_DT_VER_STABLE, 10) |
> +			   FIELD_PREP(VE_MODE_DT_EDG_THROD, 0x65));
>   
>   	aspeed_video_write(video, VE_BCD_CTRL, 0);
>   }
> 


Kind regards,

Paul

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] media: aspeed: Fix incorrect resolution detected
@ 2021-11-09 11:22   ` Paul Menzel
  0 siblings, 0 replies; 8+ messages in thread
From: Paul Menzel @ 2021-11-09 11:22 UTC (permalink / raw)
  To: Jammy Huang
  Cc: linux-aspeed, andrew, openbmc, eajames, linux-kernel, mchehab,
	linux-arm-kernel, linux-media

Dear Jammy,


Am 09.11.21 um 10:54 schrieb Jammy Huang:
> During the process of os-installation, detected resolution's height
> could be less than the correct one.

Can you please elaborate? What OS? What resolution was and what 
resolution should have been detected?

> Increase min-required-count of stable signal to fix the problem.

So you do two changes in the patch? First use the bitfield access 
macros, and then change VE_MODE_DT_HOR_STABLE and VE_MODE_DT_VER_STABLE 
from 6 to 10? Is that the amount of iterations? Why 10 and not 20?

How much time do four iterations add?

Sorry for my ignorance, but if you could make it two patches, that’d be 
great.

> Fixes: d2b4387f3bdf ("media: platform: Add Aspeed Video Engine driver")
> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
> ---
>   drivers/media/platform/aspeed-video.c | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
> index 5da52646b298..625a77ddb479 100644
> --- a/drivers/media/platform/aspeed-video.c
> +++ b/drivers/media/platform/aspeed-video.c
> @@ -196,6 +196,12 @@
>   #define  VE_INTERRUPT_VSYNC_DESC	BIT(11)
>   
>   #define VE_MODE_DETECT			0x30c
> +#define  VE_MODE_DT_HOR_TOLER		GENMASK(31, 28)
> +#define  VE_MODE_DT_VER_TOLER		GENMASK(27, 24)
> +#define  VE_MODE_DT_HOR_STABLE		GENMASK(23, 20)
> +#define  VE_MODE_DT_VER_STABLE		GENMASK(19, 16)
> +#define  VE_MODE_DT_EDG_THROD		GENMASK(15, 8)
> +
>   #define VE_MEM_RESTRICT_START		0x310
>   #define VE_MEM_RESTRICT_END		0x314
>   
> @@ -1199,7 +1205,12 @@ static void aspeed_video_init_regs(struct aspeed_video *video)
>   	aspeed_video_write(video, VE_SCALING_FILTER3, 0x00200000);
>   
>   	/* Set mode detection defaults */
> -	aspeed_video_write(video, VE_MODE_DETECT, 0x22666500);
> +	aspeed_video_write(video, VE_MODE_DETECT,
> +			   FIELD_PREP(VE_MODE_DT_HOR_TOLER, 2) |
> +			   FIELD_PREP(VE_MODE_DT_VER_TOLER, 2) |
> +			   FIELD_PREP(VE_MODE_DT_HOR_STABLE, 10) |
> +			   FIELD_PREP(VE_MODE_DT_VER_STABLE, 10) |
> +			   FIELD_PREP(VE_MODE_DT_EDG_THROD, 0x65));
>   
>   	aspeed_video_write(video, VE_BCD_CTRL, 0);
>   }
> 


Kind regards,

Paul

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] media: aspeed: Fix incorrect resolution detected
@ 2021-11-09 11:22   ` Paul Menzel
  0 siblings, 0 replies; 8+ messages in thread
From: Paul Menzel @ 2021-11-09 11:22 UTC (permalink / raw)
  To: Jammy Huang
  Cc: eajames, mchehab, joel, andrew, linux-media, openbmc,
	linux-arm-kernel, linux-aspeed, linux-kernel

Dear Jammy,


Am 09.11.21 um 10:54 schrieb Jammy Huang:
> During the process of os-installation, detected resolution's height
> could be less than the correct one.

Can you please elaborate? What OS? What resolution was and what 
resolution should have been detected?

> Increase min-required-count of stable signal to fix the problem.

So you do two changes in the patch? First use the bitfield access 
macros, and then change VE_MODE_DT_HOR_STABLE and VE_MODE_DT_VER_STABLE 
from 6 to 10? Is that the amount of iterations? Why 10 and not 20?

How much time do four iterations add?

Sorry for my ignorance, but if you could make it two patches, that’d be 
great.

> Fixes: d2b4387f3bdf ("media: platform: Add Aspeed Video Engine driver")
> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
> ---
>   drivers/media/platform/aspeed-video.c | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
> index 5da52646b298..625a77ddb479 100644
> --- a/drivers/media/platform/aspeed-video.c
> +++ b/drivers/media/platform/aspeed-video.c
> @@ -196,6 +196,12 @@
>   #define  VE_INTERRUPT_VSYNC_DESC	BIT(11)
>   
>   #define VE_MODE_DETECT			0x30c
> +#define  VE_MODE_DT_HOR_TOLER		GENMASK(31, 28)
> +#define  VE_MODE_DT_VER_TOLER		GENMASK(27, 24)
> +#define  VE_MODE_DT_HOR_STABLE		GENMASK(23, 20)
> +#define  VE_MODE_DT_VER_STABLE		GENMASK(19, 16)
> +#define  VE_MODE_DT_EDG_THROD		GENMASK(15, 8)
> +
>   #define VE_MEM_RESTRICT_START		0x310
>   #define VE_MEM_RESTRICT_END		0x314
>   
> @@ -1199,7 +1205,12 @@ static void aspeed_video_init_regs(struct aspeed_video *video)
>   	aspeed_video_write(video, VE_SCALING_FILTER3, 0x00200000);
>   
>   	/* Set mode detection defaults */
> -	aspeed_video_write(video, VE_MODE_DETECT, 0x22666500);
> +	aspeed_video_write(video, VE_MODE_DETECT,
> +			   FIELD_PREP(VE_MODE_DT_HOR_TOLER, 2) |
> +			   FIELD_PREP(VE_MODE_DT_VER_TOLER, 2) |
> +			   FIELD_PREP(VE_MODE_DT_HOR_STABLE, 10) |
> +			   FIELD_PREP(VE_MODE_DT_VER_STABLE, 10) |
> +			   FIELD_PREP(VE_MODE_DT_EDG_THROD, 0x65));
>   
>   	aspeed_video_write(video, VE_BCD_CTRL, 0);
>   }
> 


Kind regards,

Paul

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] media: aspeed: Fix incorrect resolution detected
  2021-11-09 11:22   ` Paul Menzel
  (?)
@ 2021-11-10  3:33     ` Jammy Huang
  -1 siblings, 0 replies; 8+ messages in thread
From: Jammy Huang @ 2021-11-10  3:33 UTC (permalink / raw)
  To: Paul Menzel
  Cc: eajames, mchehab, joel, andrew, linux-media, openbmc,
	linux-arm-kernel, linux-aspeed, linux-kernel

Dear Paul,

On 2021/11/9 下午 07:22, Paul Menzel wrote:
> Dear Jammy,
>
>
> Am 09.11.21 um 10:54 schrieb Jammy Huang:
>> During the process of os-installation, detected resolution's height
>> could be less than the correct one.
> Can you please elaborate? What OS? What resolution was and what
> resolution should have been detected?
My bad. This is the scenario our QA used to find this problem, but it 
could happen in
any scenario with successive timing-change condition. I will update to 
have a more
elaborate message
>
>> Increase min-required-count of stable signal to fix the problem.
> So you do two changes in the patch? First use the bitfield access
> macros, and then change VE_MODE_DT_HOR_STABLE and VE_MODE_DT_VER_STABLE
> from 6 to 10? Is that the amount of iterations? Why 10 and not 20?
> How much time do four iterations add?
VE_MODE_DT_HOR_STABLE means the min required count in detecting stable
HSYNC signal to set mode detection horizontal signal stable.
VE_MODE_DT_VER_STABLE means the min required count in detecting stable
VSYNC signal to set mode detection vertical signal stable.

How much time these iterations take varies by the input timing.
If VE_MODE_DT_VER_STABLE is 10, it means 10 successive stable vsync required
to make vertical signal stable flag raised. If the current timing is 60 
fps, it will takes
at least 10/60 second.
The suggested min acceptable value for these two are 3 and max is 15.
> Sorry for my ignorance, but if you could make it two patches, that’d be
> great.
Sure, I will make it two patches in next update.
>
>> Fixes: d2b4387f3bdf ("media: platform: Add Aspeed Video Engine driver")
>> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
>> ---
>>    drivers/media/platform/aspeed-video.c | 13 ++++++++++++-
>>    1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
>> index 5da52646b298..625a77ddb479 100644
>> --- a/drivers/media/platform/aspeed-video.c
>> +++ b/drivers/media/platform/aspeed-video.c
>> @@ -196,6 +196,12 @@
>>    #define  VE_INTERRUPT_VSYNC_DESC	BIT(11)
>>    
>>    #define VE_MODE_DETECT			0x30c
>> +#define  VE_MODE_DT_HOR_TOLER		GENMASK(31, 28)
>> +#define  VE_MODE_DT_VER_TOLER		GENMASK(27, 24)
>> +#define  VE_MODE_DT_HOR_STABLE		GENMASK(23, 20)
>> +#define  VE_MODE_DT_VER_STABLE		GENMASK(19, 16)
>> +#define  VE_MODE_DT_EDG_THROD		GENMASK(15, 8)
>> +
>>    #define VE_MEM_RESTRICT_START		0x310
>>    #define VE_MEM_RESTRICT_END		0x314
>>    
>> @@ -1199,7 +1205,12 @@ static void aspeed_video_init_regs(struct aspeed_video *video)
>>    	aspeed_video_write(video, VE_SCALING_FILTER3, 0x00200000);
>>    
>>    	/* Set mode detection defaults */
>> -	aspeed_video_write(video, VE_MODE_DETECT, 0x22666500);
>> +	aspeed_video_write(video, VE_MODE_DETECT,
>> +			   FIELD_PREP(VE_MODE_DT_HOR_TOLER, 2) |
>> +			   FIELD_PREP(VE_MODE_DT_VER_TOLER, 2) |
>> +			   FIELD_PREP(VE_MODE_DT_HOR_STABLE, 10) |
>> +			   FIELD_PREP(VE_MODE_DT_VER_STABLE, 10) |
>> +			   FIELD_PREP(VE_MODE_DT_EDG_THROD, 0x65));
>>    
>>    	aspeed_video_write(video, VE_BCD_CTRL, 0);
>>    }
>>
>
> Kind regards,
>
> Paul

-- 
Best Regards
Jammy


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] media: aspeed: Fix incorrect resolution detected
@ 2021-11-10  3:33     ` Jammy Huang
  0 siblings, 0 replies; 8+ messages in thread
From: Jammy Huang @ 2021-11-10  3:33 UTC (permalink / raw)
  To: Paul Menzel
  Cc: linux-aspeed, andrew, openbmc, eajames, linux-kernel, mchehab,
	linux-arm-kernel, linux-media

Dear Paul,

On 2021/11/9 下午 07:22, Paul Menzel wrote:
> Dear Jammy,
>
>
> Am 09.11.21 um 10:54 schrieb Jammy Huang:
>> During the process of os-installation, detected resolution's height
>> could be less than the correct one.
> Can you please elaborate? What OS? What resolution was and what
> resolution should have been detected?
My bad. This is the scenario our QA used to find this problem, but it 
could happen in
any scenario with successive timing-change condition. I will update to 
have a more
elaborate message
>
>> Increase min-required-count of stable signal to fix the problem.
> So you do two changes in the patch? First use the bitfield access
> macros, and then change VE_MODE_DT_HOR_STABLE and VE_MODE_DT_VER_STABLE
> from 6 to 10? Is that the amount of iterations? Why 10 and not 20?
> How much time do four iterations add?
VE_MODE_DT_HOR_STABLE means the min required count in detecting stable
HSYNC signal to set mode detection horizontal signal stable.
VE_MODE_DT_VER_STABLE means the min required count in detecting stable
VSYNC signal to set mode detection vertical signal stable.

How much time these iterations take varies by the input timing.
If VE_MODE_DT_VER_STABLE is 10, it means 10 successive stable vsync required
to make vertical signal stable flag raised. If the current timing is 60 
fps, it will takes
at least 10/60 second.
The suggested min acceptable value for these two are 3 and max is 15.
> Sorry for my ignorance, but if you could make it two patches, that’d be
> great.
Sure, I will make it two patches in next update.
>
>> Fixes: d2b4387f3bdf ("media: platform: Add Aspeed Video Engine driver")
>> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
>> ---
>>    drivers/media/platform/aspeed-video.c | 13 ++++++++++++-
>>    1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
>> index 5da52646b298..625a77ddb479 100644
>> --- a/drivers/media/platform/aspeed-video.c
>> +++ b/drivers/media/platform/aspeed-video.c
>> @@ -196,6 +196,12 @@
>>    #define  VE_INTERRUPT_VSYNC_DESC	BIT(11)
>>    
>>    #define VE_MODE_DETECT			0x30c
>> +#define  VE_MODE_DT_HOR_TOLER		GENMASK(31, 28)
>> +#define  VE_MODE_DT_VER_TOLER		GENMASK(27, 24)
>> +#define  VE_MODE_DT_HOR_STABLE		GENMASK(23, 20)
>> +#define  VE_MODE_DT_VER_STABLE		GENMASK(19, 16)
>> +#define  VE_MODE_DT_EDG_THROD		GENMASK(15, 8)
>> +
>>    #define VE_MEM_RESTRICT_START		0x310
>>    #define VE_MEM_RESTRICT_END		0x314
>>    
>> @@ -1199,7 +1205,12 @@ static void aspeed_video_init_regs(struct aspeed_video *video)
>>    	aspeed_video_write(video, VE_SCALING_FILTER3, 0x00200000);
>>    
>>    	/* Set mode detection defaults */
>> -	aspeed_video_write(video, VE_MODE_DETECT, 0x22666500);
>> +	aspeed_video_write(video, VE_MODE_DETECT,
>> +			   FIELD_PREP(VE_MODE_DT_HOR_TOLER, 2) |
>> +			   FIELD_PREP(VE_MODE_DT_VER_TOLER, 2) |
>> +			   FIELD_PREP(VE_MODE_DT_HOR_STABLE, 10) |
>> +			   FIELD_PREP(VE_MODE_DT_VER_STABLE, 10) |
>> +			   FIELD_PREP(VE_MODE_DT_EDG_THROD, 0x65));
>>    
>>    	aspeed_video_write(video, VE_BCD_CTRL, 0);
>>    }
>>
>
> Kind regards,
>
> Paul

-- 
Best Regards
Jammy


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] media: aspeed: Fix incorrect resolution detected
@ 2021-11-10  3:33     ` Jammy Huang
  0 siblings, 0 replies; 8+ messages in thread
From: Jammy Huang @ 2021-11-10  3:33 UTC (permalink / raw)
  To: Paul Menzel
  Cc: eajames, mchehab, joel, andrew, linux-media, openbmc,
	linux-arm-kernel, linux-aspeed, linux-kernel

Dear Paul,

On 2021/11/9 下午 07:22, Paul Menzel wrote:
> Dear Jammy,
>
>
> Am 09.11.21 um 10:54 schrieb Jammy Huang:
>> During the process of os-installation, detected resolution's height
>> could be less than the correct one.
> Can you please elaborate? What OS? What resolution was and what
> resolution should have been detected?
My bad. This is the scenario our QA used to find this problem, but it 
could happen in
any scenario with successive timing-change condition. I will update to 
have a more
elaborate message
>
>> Increase min-required-count of stable signal to fix the problem.
> So you do two changes in the patch? First use the bitfield access
> macros, and then change VE_MODE_DT_HOR_STABLE and VE_MODE_DT_VER_STABLE
> from 6 to 10? Is that the amount of iterations? Why 10 and not 20?
> How much time do four iterations add?
VE_MODE_DT_HOR_STABLE means the min required count in detecting stable
HSYNC signal to set mode detection horizontal signal stable.
VE_MODE_DT_VER_STABLE means the min required count in detecting stable
VSYNC signal to set mode detection vertical signal stable.

How much time these iterations take varies by the input timing.
If VE_MODE_DT_VER_STABLE is 10, it means 10 successive stable vsync required
to make vertical signal stable flag raised. If the current timing is 60 
fps, it will takes
at least 10/60 second.
The suggested min acceptable value for these two are 3 and max is 15.
> Sorry for my ignorance, but if you could make it two patches, that’d be
> great.
Sure, I will make it two patches in next update.
>
>> Fixes: d2b4387f3bdf ("media: platform: Add Aspeed Video Engine driver")
>> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
>> ---
>>    drivers/media/platform/aspeed-video.c | 13 ++++++++++++-
>>    1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
>> index 5da52646b298..625a77ddb479 100644
>> --- a/drivers/media/platform/aspeed-video.c
>> +++ b/drivers/media/platform/aspeed-video.c
>> @@ -196,6 +196,12 @@
>>    #define  VE_INTERRUPT_VSYNC_DESC	BIT(11)
>>    
>>    #define VE_MODE_DETECT			0x30c
>> +#define  VE_MODE_DT_HOR_TOLER		GENMASK(31, 28)
>> +#define  VE_MODE_DT_VER_TOLER		GENMASK(27, 24)
>> +#define  VE_MODE_DT_HOR_STABLE		GENMASK(23, 20)
>> +#define  VE_MODE_DT_VER_STABLE		GENMASK(19, 16)
>> +#define  VE_MODE_DT_EDG_THROD		GENMASK(15, 8)
>> +
>>    #define VE_MEM_RESTRICT_START		0x310
>>    #define VE_MEM_RESTRICT_END		0x314
>>    
>> @@ -1199,7 +1205,12 @@ static void aspeed_video_init_regs(struct aspeed_video *video)
>>    	aspeed_video_write(video, VE_SCALING_FILTER3, 0x00200000);
>>    
>>    	/* Set mode detection defaults */
>> -	aspeed_video_write(video, VE_MODE_DETECT, 0x22666500);
>> +	aspeed_video_write(video, VE_MODE_DETECT,
>> +			   FIELD_PREP(VE_MODE_DT_HOR_TOLER, 2) |
>> +			   FIELD_PREP(VE_MODE_DT_VER_TOLER, 2) |
>> +			   FIELD_PREP(VE_MODE_DT_HOR_STABLE, 10) |
>> +			   FIELD_PREP(VE_MODE_DT_VER_STABLE, 10) |
>> +			   FIELD_PREP(VE_MODE_DT_EDG_THROD, 0x65));
>>    
>>    	aspeed_video_write(video, VE_BCD_CTRL, 0);
>>    }
>>
>
> Kind regards,
>
> Paul

-- 
Best Regards
Jammy


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-11-10  4:07 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-09  9:54 [PATCH] media: aspeed: Fix incorrect resolution detected Jammy Huang
2021-11-09  9:54 ` Jammy Huang
2021-11-09 11:22 ` Paul Menzel
2021-11-09 11:22   ` Paul Menzel
2021-11-09 11:22   ` Paul Menzel
2021-11-10  3:33   ` Jammy Huang
2021-11-10  3:33     ` Jammy Huang
2021-11-10  3:33     ` Jammy Huang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.