Re:[PATCH dev-5.1 4/4] media: aspeed: add a workaround to fix a silicon bug (Jae Hyun Yoo)

Re:[PATCH dev-5.1 4/4] media: aspeed: add a workaround to fix a silicon bug (Jae Hyun Yoo)

[xiuzhi]

[-- Attachment #1: Type: text/plain, Size: 2988 bytes --]

Hi  Jae,
   I tested this patch ,it works on 1680*1050,but I found an issue:

The ikvm webpage will be black screen when you reboot the host after switching solution between 1680*1050 and 800*600 on chrome browser (my chrome version is 65).
you can reproduce it:
1, Set host screen solution to 1680*1050 and save it . ikvm webpage works 
2,Set host screen solution to 800*600 and save it. ikvm webpage works
3,Reset host screen solution to   1680*1050 and save it,
4,reboot the host,
5, the ikvm webpage is black screen when boot to Centos GUI 
Best,
xiuzhi

AST2500 silicon revision A1 and A2 have a silicon bug which causes
extremly long capturing time on specific resolutions (1680 width).
To fix the bug, this commit adjusts the capturing window register
setting to 1728 if detected width is 1680. The compression window
register setting will be kept as the original width so output
result will be the same.

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

diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index da20e93f58d3..c2d4a2e6f20f 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -826,8 +826,27 @@ static void aspeed_video_set_resolution(struct aspeed_video *video)
 	struct v4l2_bt_timings *act = &video->active_timings;
 	unsigned int size = act->width * act->height;
 
+	/* Set capture/compression frame sizes */
 	aspeed_video_calc_compressed_size(video, size);
 
+	if (video->active_timings.width == 1680) {
+		/*
+		 * This is a workaround to fix a silicon bug on A1 and A2
+		 * revisions. Since it doesn't break capturing operation on A0
+		 * revision, use it for all revisions without checking the
+		 * revision ID.
+		 */
+		aspeed_video_write(video, VE_CAP_WINDOW,
+				   1728 << 16 | act->height);
+		size += (1728 - 1680) * video->active_timings.height;
+	} else {
+		aspeed_video_write(video, VE_CAP_WINDOW,
+				   act->width << 16 | act->height);
+	}
+	aspeed_video_write(video, VE_COMP_WINDOW,
+			   act->width << 16 | act->height);
+	aspeed_video_write(video, VE_SRC_SCANLINE_OFFSET, act->width * 4);
+
 	/* Don't use direct mode below 1024 x 768 (irqs don't fire) */
 	if (size < DIRECT_FETCH_THRESHOLD) {
 		aspeed_video_write(video, VE_TGS_0,
@@ -844,13 +863,6 @@ static void aspeed_video_set_resolution(struct aspeed_video *video)
 		aspeed_video_update(video, VE_CTRL, 0, VE_CTRL_DIRECT_FETCH);
 	}
 
-	/* Set capture/compression frame sizes */
-	aspeed_video_write(video, VE_CAP_WINDOW,
-			   act->width << 16 | act->height);
-	aspeed_video_write(video, VE_COMP_WINDOW,
-			   act->width << 16 | act->height);
-	aspeed_video_write(video, VE_SRC_SCANLINE_OFFSET, act->width * 4);
-
 	size *= 4;
 
 	if (size == video->srcs[0].size / 2) {
-- 
2.21.0

[-- Attachment #2: Type: text/html, Size: 3388 bytes --]

Re: [PATCH dev-5.1 4/4] media: aspeed: add a workaround to fix a silicon bug (Jae Hyun Yoo)

[Jae Hyun Yoo]

On 5/31/2019 4:21 AM, xiuzhi wrote:
> Hi  Jae,
>     I tested this patch ,it works on 1680*1050,but I found an issue:
> 
> The ikvm webpage will be black screen when you reboot the host after 
> switching solution between 1680*1050 and 800*600 on chrome browser (my 
> chrome version is 65).
> you can reproduce it:
> 1, Set host screen solution to 1680*1050 and save it . ikvm webpage works
> 2,Set host screen solution to 800*600 and save it. ikvm webpage works
> 3,Reset host screen solution to   1680*1050 and save it,
> 4,reboot the host,
> 5, the ikvm webpage is black screen when boot to Centos GUI

Hi Xiuzhi,

Checked that the issue isn't related this patch. Actually, that is
caused by the first patch of this patch series. Since the patch removes
source buffer allocation before mode detection, remaining allocation
logic should be changed accordingly, otherwise it can't allocate buffers
and eventually causes the issue. I'll fix the first patch in the next
spin. In the meantime, you can apply below quick fix on top of what you
tested the issue.

Thanks,
Jae


diff --git a/drivers/media/platform/aspeed-video.c 
b/drivers/media/platform/aspeed-video.c
index c2d4a2e6f20f..034f2f436d76 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -865,20 +865,14 @@ static void aspeed_video_set_resolution(struct 
aspeed_video *video)

         size *= 4;

-       if (size == video->srcs[0].size / 2) {
-               aspeed_video_write(video, VE_SRC1_ADDR,
-                                  video->srcs[0].dma + size);
-       } else if (size == video->srcs[0].size) {
-               if (!aspeed_video_alloc_buf(video, &video->srcs[1], size))
-                       goto err_mem;
-
-               aspeed_video_write(video, VE_SRC1_ADDR, video->srcs[1].dma);
-       } else {
-               aspeed_video_free_buf(video, &video->srcs[0]);
+       if (size != video->srcs[0].size) {
+               if (video->srcs[0].size)
+                       aspeed_video_free_buf(video, &video->srcs[0]);
+               if (video->srcs[1].size)
+                       aspeed_video_free_buf(video, &video->srcs[1]);

                 if (!aspeed_video_alloc_buf(video, &video->srcs[0], size))
                         goto err_mem;
-
                 if (!aspeed_video_alloc_buf(video, &video->srcs[1], size))
                         goto err_mem;

Re: [PATCH dev-5.1 4/4] media: aspeed: add a workaround to fix asilicon bug (Jae Hyun Yoo)

[xiuzhi]

[-- Attachment #1: Type: text/plain, Size: 2960 bytes --]

Hi Jae,
    This new  patch works very well so far,
 I will test it more cases next week.
Best,
xiuzhi




------------------ Original ------------------
From:  "jae.hyun.yoo";<jae.hyun.yoo@linux.intel.com>;
Date:  Jun 1, 2019
To:  "xiuzhi"<1450335857@qq.com>; "openbmc"<openbmc@lists.ozlabs.org>; "eajames"<eajames@linux.ibm.com>; "joel"<joel@jms.id.au>; "andrew"<andrew@aj.id.au>; 

Subject:  Re: [PATCH dev-5.1 4/4] media: aspeed: add a workaround to fix asilicon bug (Jae Hyun Yoo)



On 5/31/2019 4:21 AM, xiuzhi wrote:
> Hi  Jae,
>     I tested this patch ,it works on 1680*1050,but I found an issue:
> 
> The ikvm webpage will be black screen when you reboot the host after 
> switching solution between 1680*1050 and 800*600 on chrome browser (my 
> chrome version is 65).
> you can reproduce it:
> 1, Set host screen solution to 1680*1050 and save it . ikvm webpage works
> 2,Set host screen solution to 800*600 and save it. ikvm webpage works
> 3,Reset host screen solution to   1680*1050 and save it,
> 4,reboot the host,
> 5, the ikvm webpage is black screen when boot to Centos GUI

Hi Xiuzhi,

Checked that the issue isn't related this patch. Actually, that is
caused by the first patch of this patch series. Since the patch removes
source buffer allocation before mode detection, remaining allocation
logic should be changed accordingly, otherwise it can't allocate buffers
and eventually causes the issue. I'll fix the first patch in the next
spin. In the meantime, you can apply below quick fix on top of what you
tested the issue.

Thanks,
Jae


diff --git a/drivers/media/platform/aspeed-video.c 
b/drivers/media/platform/aspeed-video.c
index c2d4a2e6f20f..034f2f436d76 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -865,20 +865,14 @@ static void aspeed_video_set_resolution(struct 
aspeed_video *video)

         size *= 4;

-       if (size == video->srcs[0].size / 2) {
-               aspeed_video_write(video, VE_SRC1_ADDR,
-                                  video->srcs[0].dma + size);
-       } else if (size == video->srcs[0].size) {
-               if (!aspeed_video_alloc_buf(video, &video->srcs[1], size))
-                       goto err_mem;
-
-               aspeed_video_write(video, VE_SRC1_ADDR, video->srcs[1].dma);
-       } else {
-               aspeed_video_free_buf(video, &video->srcs[0]);
+       if (size != video->srcs[0].size) {
+               if (video->srcs[0].size)
+                       aspeed_video_free_buf(video, &video->srcs[0]);
+               if (video->srcs[1].size)
+                       aspeed_video_free_buf(video, &video->srcs[1]);

                 if (!aspeed_video_alloc_buf(video, &video->srcs[0], size))
                         goto err_mem;
-
                 if (!aspeed_video_alloc_buf(video, &video->srcs[1], size))
                         goto err_mem;

[-- Attachment #2: Type: text/html, Size: 5051 bytes --]

Re: [PATCH dev-5.1 4/4] media: aspeed: add a workaround to fix asilicon bug (Jae Hyun Yoo)

[Jae Hyun Yoo]

Hi Xiuzhi,

Thanks a lot for sharing the test result. Please let us update if you
find any issue in the further test.

Regards,
Jae

On 6/1/2019 7:46 PM, xiuzhi wrote:
> Hi Jae,
>      This new  patch works very well so far,
>   I will test it more cases next week.
> Best,
> xiuzhi
> 
> 
> ------------------ Original ------------------
> *From: * "jae.hyun.yoo";<jae.hyun.yoo@linux.intel.com>;
> *Date: * Jun 1, 2019
> *To: * "xiuzhi"<1450335857@qq.com>; "openbmc"<openbmc@lists.ozlabs.org>; 
> "eajames"<eajames@linux.ibm.com>; "joel"<joel@jms.id.au>; 
> "andrew"<andrew@aj.id.au>;
> *Subject: * Re: [PATCH dev-5.1 4/4] media: aspeed: add a workaround to 
> fix asilicon bug (Jae Hyun Yoo)
> 
> On 5/31/2019 4:21 AM, xiuzhi wrote:
>  > Hi  Jae,
>  >     I tested this patch ,it works on 1680*1050,but I found an issue:
>  >
>  > The ikvm webpage will be black screen when you reboot the host after
>  > switching solution between 1680*1050 and 800*600 on chrome browser (my
>  > chrome version is 65).
>  > you can reproduce it:
>  > 1, Set host screen solution to 1680*1050 and save it . ikvm webpage works
>  > 2,Set host screen solution to 800*600 and save it. ikvm webpage works
>  > 3,Reset host screen solution to   1680*1050 and save it,
>  > 4,reboot the host,
>  > 5, the ikvm webpage is black screen when boot to Centos GUI
> 
> Hi Xiuzhi,
> 
> Checked that the issue isn't related this patch. Actually, that is
> caused by the first patch of this patch series. Since the patch removes
> source buffer allocation before mode detection, remaining allocation
> logic should be changed accordingly, otherwise it can't allocate buffers
> and eventually causes the issue. I'll fix the first patch in the next
> spin. In the meantime, you can apply below quick fix on top of what you
> tested the issue.
> 
> Thanks,
> Jae
> 
> 
> diff --git a/drivers/media/platform/aspeed-video.c
> b/drivers/media/platform/aspeed-video.c
> index c2d4a2e6f20f..034f2f436d76 100644
> --- a/drivers/media/platform/aspeed-video.c
> +++ b/drivers/media/platform/aspeed-video.c
> @@ -865,20 +865,14 @@ static void aspeed_video_set_resolution(struct
> aspeed_video *video)
> 
>           size *= 4;
> 
> -       if (size == video->srcs[0].size / 2) {
> -               aspeed_video_write(video, VE_SRC1_ADDR,
> -                                  video->srcs[0].dma + size);
> -       } else if (size == video->srcs[0].size) {
> -               if (!aspeed_video_alloc_buf(video, &video->srcs[1], size))
> -                       goto err_mem;
> -
> -               aspeed_video_write(video, VE_SRC1_ADDR, video->srcs[1].dma);
> -       } else {
> -               aspeed_video_free_buf(video, &video->srcs[0]);
> +       if (size != video->srcs[0].size) {
> +               if (video->srcs[0].size)
> +                       aspeed_video_free_buf(video, &video->srcs[0]);
> +               if (video->srcs[1].size)
> +                       aspeed_video_free_buf(video, &video->srcs[1]);
> 
>                   if (!aspeed_video_alloc_buf(video, &video->srcs[0], size))
>                           goto err_mem;
> -
>                   if (!aspeed_video_alloc_buf(video, &video->srcs[1], size))
>                           goto err_mem;
>