| Message ID | 20191001080253.6135-4-icenowy@aosc.io (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/sun4i: dsi: misc timing fixes | expand |
Hi Wens, On Tue, Oct 1, 2019 at 1:34 PM Icenowy Zheng <icenowy@aosc.io> wrote: > > This reverts commit 62e7511a4f4dcf07f753893d3424decd9466c98b. > > This commit, although claimed as a refactor, in fact changed the > formula. > > By expanding the original formula, we can find that the const 10 is not > substracted, instead it's added to the value (because 10 is negative > when calculating hsa, and hsa itself is negative when calculating hblk). > This breaks the similar pattern to other formulas, so restoring the > original formula is more proper. > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > --- > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 9 ++------- > 1 file changed, 2 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > index 2d3e822a7739..cb5fd19c0d0d 100644 > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > @@ -577,14 +577,9 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi, > (mode->hsync_start - mode->hdisplay) * Bpp - HFP_PACKET_OVERHEAD); > > /* > - * The blanking is set using a sync event (4 bytes) > - * and a blanking packet (4 bytes + payload + 2 > - * bytes). Its minimal size is therefore 10 bytes. > + * hblk seems to be the line + porches length. > */ > -#define HBLK_PACKET_OVERHEAD 10 > - hblk = max((unsigned int)HBLK_PACKET_OVERHEAD, > - (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp - > - HBLK_PACKET_OVERHEAD); > + hblk = mode->htotal * Bpp - hsa; The original formula is correct according to BSP [1] and work with my panels which I have tested before. May be the horizontal timings on panels you have leads to negative value. [1] https://github.com/ayufan-pine64/linux-pine64/blob/my-hacks-1.2-with-drm/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c#L919
在 2019-10-03四的 09:53 +0530，Jagan Teki写道： > Hi Wens, > > On Tue, Oct 1, 2019 at 1:34 PM Icenowy Zheng <icenowy@aosc.io> wrote: > > This reverts commit 62e7511a4f4dcf07f753893d3424decd9466c98b. > > > > This commit, although claimed as a refactor, in fact changed the > > formula. > > > > By expanding the original formula, we can find that the const 10 is > > not > > substracted, instead it's added to the value (because 10 is > > negative > > when calculating hsa, and hsa itself is negative when calculating > > hblk). > > This breaks the similar pattern to other formulas, so restoring the > > original formula is more proper. > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > > --- > > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 9 ++------- > > 1 file changed, 2 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > index 2d3e822a7739..cb5fd19c0d0d 100644 > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > @@ -577,14 +577,9 @@ static void sun6i_dsi_setup_timings(struct > > sun6i_dsi *dsi, > > (mode->hsync_start - mode->hdisplay) * > > Bpp - HFP_PACKET_OVERHEAD); > > > > /* > > - * The blanking is set using a sync event (4 bytes) > > - * and a blanking packet (4 bytes + payload + 2 > > - * bytes). Its minimal size is therefore 10 bytes. > > + * hblk seems to be the line + porches length. > > */ > > -#define HBLK_PACKET_OVERHEAD 10 > > - hblk = max((unsigned int)HBLK_PACKET_OVERHEAD, > > - (mode->htotal - (mode->hsync_end - mode- > > >hsync_start)) * Bpp - > > - HBLK_PACKET_OVERHEAD); > > + hblk = mode->htotal * Bpp - hsa; > > The original formula is correct according to BSP [1] and work with my > panels which I have tested before. May be the horizontal timings on > panels you have leads to negative value. Do you tested the same timing with BSP kernel? It's quite difficult to get a negative value here, because the value is quite big (includes mode->hdisplay * Bpp). Strangely, only change the formula here back makes the timing translated from FEX file works (tested on PineTab and PinePhone production ver). The translation rule is from [1]. So I still insist on the patch because it's needed by experiment. [1] http://linux-sunxi.org/LCD > > [1] > https://github.com/ayufan-pine64/linux-pine64/blob/my-hacks-1.2-with-drm/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c#L919
在 2019-10-06日的 22:44 +0800，Icenowy Zheng写道： > 在 2019-10-03四的 09:53 +0530，Jagan Teki写道： > > Hi Wens, > > > > On Tue, Oct 1, 2019 at 1:34 PM Icenowy Zheng <icenowy@aosc.io> > > wrote: > > > This reverts commit 62e7511a4f4dcf07f753893d3424decd9466c98b. > > > > > > This commit, although claimed as a refactor, in fact changed the > > > formula. > > > > > > By expanding the original formula, we can find that the const 10 > > > is > > > not > > > substracted, instead it's added to the value (because 10 is > > > negative > > > when calculating hsa, and hsa itself is negative when calculating > > > hblk). > > > This breaks the similar pattern to other formulas, so restoring > > > the > > > original formula is more proper. > > > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > > > --- > > > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 9 ++------- > > > 1 file changed, 2 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > > b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > > index 2d3e822a7739..cb5fd19c0d0d 100644 > > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > > @@ -577,14 +577,9 @@ static void sun6i_dsi_setup_timings(struct > > > sun6i_dsi *dsi, > > > (mode->hsync_start - mode->hdisplay) * > > > Bpp - HFP_PACKET_OVERHEAD); > > > > > > /* > > > - * The blanking is set using a sync event (4 > > > bytes) > > > - * and a blanking packet (4 bytes + payload + 2 > > > - * bytes). Its minimal size is therefore 10 > > > bytes. > > > + * hblk seems to be the line + porches length. > > > */ > > > -#define HBLK_PACKET_OVERHEAD 10 > > > - hblk = max((unsigned int)HBLK_PACKET_OVERHEAD, > > > - (mode->htotal - (mode->hsync_end - > > > mode- > > > > hsync_start)) * Bpp - > > > - HBLK_PACKET_OVERHEAD); > > > + hblk = mode->htotal * Bpp - hsa; > > > > The original formula is correct according to BSP [1] and work with > > my > > panels which I have tested before. May be the horizontal timings on > > panels you have leads to negative value. > > Do you tested the same timing with BSP kernel? > > It's quite difficult to get a negative value here, because the value > is > quite big (includes mode->hdisplay * Bpp). By re-checking with the BSP source code, I found that the constant in the HFP formula is indeed wrong -- it should be 16, not 6. > > Strangely, only change the formula here back makes the timing > translated from FEX file works (tested on PineTab and PinePhone > production ver). The translation rule is from [1]. > > So I still insist on the patch because it's needed by experiment. > > [1] http://linux-sunxi.org/LCD > > > [1] > > https://github.com/ayufan-pine64/linux-pine64/blob/my-hacks-1.2-with-drm/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c#L919
HI Icenowy, On Sun, Oct 06, 2019 at 11:12:43PM +0800, Icenowy Zheng wrote: > 在 2019-10-06日的 22:44 +0800，Icenowy Zheng写道： > > 在 2019-10-03四的 09:53 +0530，Jagan Teki写道： > > > Hi Wens, > > > > > > On Tue, Oct 1, 2019 at 1:34 PM Icenowy Zheng <icenowy@aosc.io> > > > wrote: > > > > This reverts commit 62e7511a4f4dcf07f753893d3424decd9466c98b. > > > > > > > > This commit, although claimed as a refactor, in fact changed the > > > > formula. > > > > > > > > By expanding the original formula, we can find that the const 10 > > > > is > > > > not > > > > substracted, instead it's added to the value (because 10 is > > > > negative > > > > when calculating hsa, and hsa itself is negative when calculating > > > > hblk). > > > > This breaks the similar pattern to other formulas, so restoring > > > > the > > > > original formula is more proper. > > > > > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > > > > --- > > > > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 9 ++------- > > > > 1 file changed, 2 insertions(+), 7 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > > > b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > > > index 2d3e822a7739..cb5fd19c0d0d 100644 > > > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > > > @@ -577,14 +577,9 @@ static void sun6i_dsi_setup_timings(struct > > > > sun6i_dsi *dsi, > > > > (mode->hsync_start - mode->hdisplay) * > > > > Bpp - HFP_PACKET_OVERHEAD); > > > > > > > > /* > > > > - * The blanking is set using a sync event (4 > > > > bytes) > > > > - * and a blanking packet (4 bytes + payload + 2 > > > > - * bytes). Its minimal size is therefore 10 > > > > bytes. > > > > + * hblk seems to be the line + porches length. > > > > */ > > > > -#define HBLK_PACKET_OVERHEAD 10 > > > > - hblk = max((unsigned int)HBLK_PACKET_OVERHEAD, > > > > - (mode->htotal - (mode->hsync_end - > > > > mode- > > > > > hsync_start)) * Bpp - > > > > - HBLK_PACKET_OVERHEAD); > > > > + hblk = mode->htotal * Bpp - hsa; > > > > > > The original formula is correct according to BSP [1] and work with > > > my > > > panels which I have tested before. May be the horizontal timings on > > > panels you have leads to negative value. > > > > Do you tested the same timing with BSP kernel? > > > > It's quite difficult to get a negative value here, because the value > > is > > quite big (includes mode->hdisplay * Bpp). > > By re-checking with the BSP source code, I found that the constant in > the HFP formula is indeed wrong -- it should be 16, not 6. I'm not sure if it's relevant to the discussion, but I've recently found a LCD configuration manual for A10, that may contain some useful info: See this: https://github.com/pocketbook/Platform_A13/blob/master/Kernel/drivers/video/sun5i/lcd/a10_lcd_config_nanual_v1.0.pdf regards, o. > > > > Strangely, only change the formula here back makes the timing > > translated from FEX file works (tested on PineTab and PinePhone > > production ver). The translation rule is from [1]. > > > > So I still insist on the patch because it's needed by experiment. > > > > [1] http://linux-sunxi.org/LCD > > > > > [1] > > > https://github.com/ayufan-pine64/linux-pine64/blob/my-hacks-1.2-with-drm/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c#L919 > > -- > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. > To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/14da3ae768c439e387f6609553bd465e945d4a33.camel%40aosc.io.
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index 2d3e822a7739..cb5fd19c0d0d 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -577,14 +577,9 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi, (mode->hsync_start - mode->hdisplay) * Bpp - HFP_PACKET_OVERHEAD); /* - * The blanking is set using a sync event (4 bytes) - * and a blanking packet (4 bytes + payload + 2 - * bytes). Its minimal size is therefore 10 bytes. + * hblk seems to be the line + porches length. */ -#define HBLK_PACKET_OVERHEAD 10 - hblk = max((unsigned int)HBLK_PACKET_OVERHEAD, - (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp - - HBLK_PACKET_OVERHEAD); + hblk = mode->htotal * Bpp - hsa; /* * And I'm not entirely sure what vblk is about. The driver in
This reverts commit 62e7511a4f4dcf07f753893d3424decd9466c98b. This commit, although claimed as a refactor, in fact changed the formula. By expanding the original formula, we can find that the const 10 is not substracted, instead it's added to the value (because 10 is negative when calculating hsa, and hsa itself is negative when calculating hblk). This breaks the similar pattern to other formulas, so restoring the original formula is more proper. Signed-off-by: Icenowy Zheng <icenowy@aosc.io> --- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)