Re: [PATCH 5/9] drm/bridge: synopsys: dw-mipi-dsi: Use pixel clock rate to calculate lbcc
Hi,
Am Dienstag, 17. Oktober 2023, 12:15:11 CEST schrieb Ying Liu:
> On Tuesday, October 17, 2023 2:15 AM, Heiko Stübner wrote:
> > Am Montag, 17. Juli 2023, 08:18:27 CEST schrieb Liu Ying:
> > > To get better accuration, use pixel clock rate to calculate lbcc instead
> > > of
> > > lane_mbps since the pixel clock rate is in KHz while lane_mbps is in MHz.
> > > Without this, distorted image can be seen on a HDMI monitor connected
> > with
> > > i.MX93 11x11 EVK through ADV7535 DSI to HDMI bridge in 1920x1080p@60
> > video
> > > mode.
> > >
> > > Signed-off-by: Liu Ying
> >
> > looks like I'm late to the party, but this change breaks the display output
> > my px30 minievb with the xinpeng xpp055c272 dsi display [0].
>
> Hmm, I asked for a test, but anyway sorry for the breakage.
I'm often way behind with looking at drm-related changes, sorry about that.
So thanks a lot for taking the time to look into the problem.
> The panel driver sets MIPI_DSI_MODE_VIDEO_BURST.
> And, it seems that rockchip dsi driver [1] only supports the burst mode,
> because it takes 1/0.8 = 1.25 faster lane_mbps than "bandwidth of RGB".
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c#n568
>
> >
> > Found this commit via git bisection and added a bit of debug output to
> > compare the value differences for the old and new calculation:
> >
> > [ 34.810722] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 810 * 480 * 1000
> > / 8
> > [ 34.810749] dw_mipi_dsi_get_hcomponent_lbcc: new lbcc: 810 * 64000 *
> > 24 / (4 * 8)
> > [ 34.810756] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 4860, new
> > lbcc: 3888
> > [ 34.810762] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 10 * 480 * 1000 /
> > 8
> > [ 34.810767] dw_mipi_dsi_get_hcomponent_lbcc: new lbcc: 10 * 64000 * 24
> > / (4 * 8)
> > [ 34.810773] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 60, new lbcc:
> > 48
> > [ 34.810778] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 40 * 480 * 1000 /
> > 8
> > [ 34.810783] dw_mipi_dsi_get_hcomponent_lbcc: new lbcc: 40 * 64000 * 24
> > / (4 * 8)
> > [ 34.810789] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 240, new
> > lbcc: 192
>
> Old lbcc / new lbcc is always 1.25.
>
> The new lbcc is for non-burst modes(sync pulse/sync event), IIUC.
> At least, it works for i.MX93 with the RM67191 panel and ADV7535 in
> sync pulse mode.
>
> >
> > With the new lbcc I get a blank dsi panel and just going back to the old
> > calculation of lbcc restores the image.
> >
> > I don't have that much in-depth knowledge about dsi stuff and the original
> > panel times also "just" came from the vendor tree, but I really would like
> > to keep that display working ;-) .
> >
> > Do you have any idea which way to go to fix this?
>
> Can you please test the below patch for your case?
The patch below does fix the display on the device. After applying it
I do get a working display again.
> 8<-
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -774,13 +774,19 @@ static u32 dw_mipi_dsi_get_hcomponent_lbcc(struct
> dw_mipi_dsi *dsi,
> u32 frac, lbcc, minimum_lbcc;
> int bpp;
>
> - bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> - if (bpp < 0) {
> - dev_err(dsi->dev, "failed to get bpp\n");
> - return 0;
> - }
> + if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
> + /* lbcc based on lane_mbps */
> + lbcc = hcomponent * dsi->lane_mbps * MSEC_PER_SEC / 8;
> + } else {
> + /* lbcc based on pixel clock */
> + bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> + if (bpp < 0) {
> + dev_err(dsi->dev, "failed to get bpp\n");
> + return 0;
> + }
>
> - lbcc = div_u64((u64)hcomponent * mode->clock * bpp, dsi->lanes * 8);
> + lbcc = div_u64((u64)hcomponent * mode->clock * bpp,
> dsi->lanes * 8);
> + }
>
> frac = lbcc % mode->clock;
> lbcc = lbcc / mode->clock;
> 8<-
>
> It kind of keeps the old lbcc for burst mode, except for the minimum lbcc
> check
> I introduced.
>
> It seems that meson supports non-burst modes only and stm supports both
> non-burst modes and burst mode. With the patch, I still worry about non-burst
> modes for stm, assuming the minimum lbcc check is ok and everything works
> for meson since I guess Neil has already tested the patch set on meson.
>
> Should we go with the above patch? If yes, I may send it out.
In my mind, definitly :-) .
But maybe Neil as the other reviewer also wants to
RE: [PATCH 5/9] drm/bridge: synopsys: dw-mipi-dsi: Use pixel clock rate to calculate lbcc
On Tuesday, October 17, 2023 2:15 AM, Heiko Stübner wrote:
> Hi,
Hi,
>
> Am Montag, 17. Juli 2023, 08:18:27 CEST schrieb Liu Ying:
> > To get better accuration, use pixel clock rate to calculate lbcc instead of
> > lane_mbps since the pixel clock rate is in KHz while lane_mbps is in MHz.
> > Without this, distorted image can be seen on a HDMI monitor connected
> with
> > i.MX93 11x11 EVK through ADV7535 DSI to HDMI bridge in 1920x1080p@60
> video
> > mode.
> >
> > Signed-off-by: Liu Ying
>
> looks like I'm late to the party, but this change breaks the display output
> my px30 minievb with the xinpeng xpp055c272 dsi display [0].
Hmm, I asked for a test, but anyway sorry for the breakage.
The panel driver sets MIPI_DSI_MODE_VIDEO_BURST.
And, it seems that rockchip dsi driver [1] only supports the burst mode,
because it takes 1/0.8 = 1.25 faster lane_mbps than "bandwidth of RGB".
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c#n568
>
> Found this commit via git bisection and added a bit of debug output to
> compare the value differences for the old and new calculation:
>
> [ 34.810722] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 810 * 480 * 1000
> / 8
> [ 34.810749] dw_mipi_dsi_get_hcomponent_lbcc: new lbcc: 810 * 64000 *
> 24 / (4 * 8)
> [ 34.810756] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 4860, new
> lbcc: 3888
> [ 34.810762] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 10 * 480 * 1000 /
> 8
> [ 34.810767] dw_mipi_dsi_get_hcomponent_lbcc: new lbcc: 10 * 64000 * 24
> / (4 * 8)
> [ 34.810773] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 60, new lbcc:
> 48
> [ 34.810778] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 40 * 480 * 1000 /
> 8
> [ 34.810783] dw_mipi_dsi_get_hcomponent_lbcc: new lbcc: 40 * 64000 * 24
> / (4 * 8)
> [ 34.810789] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 240, new
> lbcc: 192
Old lbcc / new lbcc is always 1.25.
The new lbcc is for non-burst modes(sync pulse/sync event), IIUC.
At least, it works for i.MX93 with the RM67191 panel and ADV7535 in
sync pulse mode.
>
> With the new lbcc I get a blank dsi panel and just going back to the old
> calculation of lbcc restores the image.
>
> I don't have that much in-depth knowledge about dsi stuff and the original
> panel times also "just" came from the vendor tree, but I really would like
> to keep that display working ;-) .
>
> Do you have any idea which way to go to fix this?
Can you please test the below patch for your case?
8<-
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -774,13 +774,19 @@ static u32 dw_mipi_dsi_get_hcomponent_lbcc(struct
dw_mipi_dsi *dsi,
u32 frac, lbcc, minimum_lbcc;
int bpp;
- bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
- if (bpp < 0) {
- dev_err(dsi->dev, "failed to get bpp\n");
- return 0;
- }
+ if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
+ /* lbcc based on lane_mbps */
+ lbcc = hcomponent * dsi->lane_mbps * MSEC_PER_SEC / 8;
+ } else {
+ /* lbcc based on pixel clock */
+ bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
+ if (bpp < 0) {
+ dev_err(dsi->dev, "failed to get bpp\n");
+ return 0;
+ }
- lbcc = div_u64((u64)hcomponent * mode->clock * bpp, dsi->lanes * 8);
+ lbcc = div_u64((u64)hcomponent * mode->clock * bpp, dsi->lanes
* 8);
+ }
frac = lbcc % mode->clock;
lbcc = lbcc / mode->clock;
8<-
It kind of keeps the old lbcc for burst mode, except for the minimum lbcc check
I introduced.
It seems that meson supports non-burst modes only and stm supports both
non-burst modes and burst mode. With the patch, I still worry about non-burst
modes for stm, assuming the minimum lbcc check is ok and everything works
for meson since I guess Neil has already tested the patch set on meson.
Should we go with the above patch? If yes, I may send it out.
Regards,
Liu Ying
>
>
> Thanks
> Heiko
>
>
> [0]
> https://git.ker/
> nel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2F
> tree%2Farch%2Farm64%2Fboot%2Fdts%2Frockchip%2Fpx30-
> evb.dts%23n138=05%7C01%7Cvictor.liu%40nxp.com%7C8f712ad41720
> 4ba7411808dbce73ce63%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> %7C638330769044424464%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLj
> AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%
> 7C%7C=effPCbPOk3GGuO8mR%2FSlcjFJfDUEZmq082simvjkux0%3D
> eserved=0
> https://git.ker/
>
Re: [PATCH 5/9] drm/bridge: synopsys: dw-mipi-dsi: Use pixel clock rate to calculate lbcc
Hi,
Am Montag, 17. Juli 2023, 08:18:27 CEST schrieb Liu Ying:
> To get better accuration, use pixel clock rate to calculate lbcc instead of
> lane_mbps since the pixel clock rate is in KHz while lane_mbps is in MHz.
> Without this, distorted image can be seen on a HDMI monitor connected with
> i.MX93 11x11 EVK through ADV7535 DSI to HDMI bridge in 1920x1080p@60 video
> mode.
>
> Signed-off-by: Liu Ying
looks like I'm late to the party, but this change breaks the display output
my px30 minievb with the xinpeng xpp055c272 dsi display [0].
Found this commit via git bisection and added a bit of debug output to
compare the value differences for the old and new calculation:
[ 34.810722] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 810 * 480 * 1000 / 8
[ 34.810749] dw_mipi_dsi_get_hcomponent_lbcc: new lbcc: 810 * 64000 * 24 / (4
* 8)
[ 34.810756] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 4860, new lbcc:
3888
[ 34.810762] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 10 * 480 * 1000 / 8
[ 34.810767] dw_mipi_dsi_get_hcomponent_lbcc: new lbcc: 10 * 64000 * 24 / (4
* 8)
[ 34.810773] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 60, new lbcc:
48
[ 34.810778] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 40 * 480 * 1000 / 8
[ 34.810783] dw_mipi_dsi_get_hcomponent_lbcc: new lbcc: 40 * 64000 * 24 / (4
* 8)
[ 34.810789] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 240, new lbcc:
192
With the new lbcc I get a blank dsi panel and just going back to the old
calculation of lbcc restores the image.
I don't have that much in-depth knowledge about dsi stuff and the original
panel times also "just" came from the vendor tree, but I really would like
to keep that display working ;-) .
Do you have any idea which way to go to fix this?
Thanks
Heiko
[0]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/px30-evb.dts#n138
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
> ---
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index c754d55f71d1..332388fd86da 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -12,6 +12,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -762,8 +763,15 @@ static u32 dw_mipi_dsi_get_hcomponent_lbcc(struct
> dw_mipi_dsi *dsi,
> u32 hcomponent)
> {
> u32 frac, lbcc;
> + int bpp;
>
> - lbcc = hcomponent * dsi->lane_mbps * MSEC_PER_SEC / 8;
> + bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> + if (bpp < 0) {
> + dev_err(dsi->dev, "failed to get bpp\n");
> + return 0;
> + }
> +
> + lbcc = div_u64((u64)hcomponent * mode->clock * bpp, dsi->lanes * 8);
Re: [PATCH 5/9] drm/bridge: synopsys: dw-mipi-dsi: Use pixel clock rate to calculate lbcc
On 17/07/2023 08:18, Liu Ying wrote:
To get better accuration, use pixel clock rate to calculate lbcc instead of
lane_mbps since the pixel clock rate is in KHz while lane_mbps is in MHz.
Without this, distorted image can be seen on a HDMI monitor connected with
i.MX93 11x11 EVK through ADV7535 DSI to HDMI bridge in 1920x1080p@60 video
mode.
Signed-off-by: Liu Ying
---
drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index c754d55f71d1..332388fd86da 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -762,8 +763,15 @@ static u32 dw_mipi_dsi_get_hcomponent_lbcc(struct
dw_mipi_dsi *dsi,
u32 hcomponent)
{
u32 frac, lbcc;
+ int bpp;
- lbcc = hcomponent * dsi->lane_mbps * MSEC_PER_SEC / 8;
+ bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
+ if (bpp < 0) {
+ dev_err(dsi->dev, "failed to get bpp\n");
+ return 0;
+ }
+
+ lbcc = div_u64((u64)hcomponent * mode->clock * bpp, dsi->lanes * 8);
frac = lbcc % mode->clock;
lbcc = lbcc / mode->clock;
Reviewed-by: Neil Armstrong
[PATCH 5/9] drm/bridge: synopsys: dw-mipi-dsi: Use pixel clock rate to calculate lbcc
To get better accuration, use pixel clock rate to calculate lbcc instead of
lane_mbps since the pixel clock rate is in KHz while lane_mbps is in MHz.
Without this, distorted image can be seen on a HDMI monitor connected with
i.MX93 11x11 EVK through ADV7535 DSI to HDMI bridge in 1920x1080p@60 video
mode.
Signed-off-by: Liu Ying
---
drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index c754d55f71d1..332388fd86da 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -762,8 +763,15 @@ static u32 dw_mipi_dsi_get_hcomponent_lbcc(struct
dw_mipi_dsi *dsi,
u32 hcomponent)
{
u32 frac, lbcc;
+ int bpp;
- lbcc = hcomponent * dsi->lane_mbps * MSEC_PER_SEC / 8;
+ bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
+ if (bpp < 0) {
+ dev_err(dsi->dev, "failed to get bpp\n");
+ return 0;
+ }
+
+ lbcc = div_u64((u64)hcomponent * mode->clock * bpp, dsi->lanes * 8);
frac = lbcc % mode->clock;
lbcc = lbcc / mode->clock;
--
2.37.1
