[v8, PATCH 0/3] force hsa hbp hfp packets multiple of lanenum to avoid screen shift

[v8, PATCH 0/3] force hsa hbp hfp packets multiple of lanenum to avoid screen shift

[Rex-BC Chen]

Changes since v7:
 - Rebase to kernel 5.16
 - Add tags of reviewed-by and acked-by.
 - Add detailed commit message for flag "hs_packet_end_aligned" in DSI common driver.

Changes since v6:
 - Add "bool hs_packet_end_aligned" in "struct mipi_dsi_device" to control the dsi aligned.
 - Config the "hs_packet_end_aligned" in ANX7725 .attach().

Changes since v5:
 - Search the anx7625 compatible as flag to control dsi output aligned.

Changes since v4:
 - Move "dt-bindings: drm/bridge: anx7625: add force_dsi_end_without_null" before
   "drm/mediatek: force hsa hbp hfp packets multiple of lanenum to avoid".
 - Retitle "dt-bindings: drm/bridge: anx7625: add force_dsi_end_without_null".

Rex-BC Chen (3):
  drm/dsi: transfer DSI HS packets ending at the same time
  drm/mediatek: implement the DSI hs packets aligned
  drm/bridge: anx7625: config hs packets end aligned to avoid screen shift

 drivers/gpu/drm/bridge/analogix/anx7625.c |  1 +
 drivers/gpu/drm/mediatek/mtk_dsi.c        | 10 ++++++++++
 include/drm/drm_mipi_dsi.h                |  3 +++
 3 files changed, 14 insertions(+)

-- 
2.18.0

[v8, PATCH 1/3] drm/dsi: transfer DSI HS packets ending at the same time

[Rex-BC Chen]

Since a HS transmission is composed of an arbitrary number
of bytes that may not be an integer multiple of lanes, some
lanes may run out of data before others.
(Defined in 6.1.3 of mipi_DSI_specification_v.01-02-00)

However, for some DSI RX devices (for example, anx7625),
there is a limitation that packet number should be the same
on all DSI lanes. In other words, they need to end a HS at
the same time.

Because this limitation is for some specific DSI RX devices,
it is more reasonable to put the enable control in these
DSI RX drivers. If DSI TX driver knows the information,
they can adjust the setting for this situation.

Therefore, add a flag to control this situation beacuse the
mipi DSI specification is not forbidden this situation.

Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
Reviewed-by: Chun-Kuang Hu <chunkuang.hu@kernel.org>
---
 include/drm/drm_mipi_dsi.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 147e51b6d241..df4d15345326 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -177,6 +177,8 @@ struct mipi_dsi_device_info {
  * @lp_rate: maximum lane frequency for low power mode in hertz, this should
  * be set to the real limits of the hardware, zero is only accepted for
  * legacy drivers
+ * @hs_packet_end_aligned: transfer DSI HS packets ending at the same time
+ * for all DSI lanes
  */
 struct mipi_dsi_device {
 	struct mipi_dsi_host *host;
@@ -189,6 +191,7 @@ struct mipi_dsi_device {
 	unsigned long mode_flags;
 	unsigned long hs_rate;
 	unsigned long lp_rate;
+	bool hs_packet_end_aligned;
 };
 
 #define MIPI_DSI_MODULE_PREFIX "mipi-dsi:"
-- 
2.18.0

[v8, PATCH 2/3] drm/mediatek: implement the DSI hs packets aligned

[Rex-BC Chen]

Some DSI RX devices require the packets on all lanes aligned at the end.
Otherwise, there will be some issues of shift or scroll for screen.

Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
Reviewed-by: Chun-Kuang Hu <chunkuang.hu@kernel.org>
---
 drivers/gpu/drm/mediatek/mtk_dsi.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 5d90d2eb0019..2f3ff9b595a4 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -195,6 +195,8 @@ struct mtk_dsi {
 	struct clk *hs_clk;
 
 	u32 data_rate;
+	/* force dsi line end without dsi_null data */
+	bool hs_packet_end_aligned;
 
 	unsigned long mode_flags;
 	enum mipi_dsi_pixel_format format;
@@ -500,6 +502,13 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
 		DRM_WARN("HFP + HBP less than d-phy, FPS will under 60Hz\n");
 	}
 
+	if (dsi->hs_packet_end_aligned) {
+		horizontal_sync_active_byte = roundup(horizontal_sync_active_byte, dsi->lanes) - 2;
+		horizontal_frontporch_byte = roundup(horizontal_frontporch_byte, dsi->lanes) - 2;
+		horizontal_backporch_byte = roundup(horizontal_backporch_byte, dsi->lanes) - 2;
+		horizontal_backporch_byte -= (vm->hactive * dsi_tmp_buf_bpp + 2) % dsi->lanes;
+	}
+
 	writel(horizontal_sync_active_byte, dsi->regs + DSI_HSA_WC);
 	writel(horizontal_backporch_byte, dsi->regs + DSI_HBP_WC);
 	writel(horizontal_frontporch_byte, dsi->regs + DSI_HFP_WC);
@@ -794,6 +803,7 @@ static int mtk_dsi_host_attach(struct mipi_dsi_host *host,
 	dsi->lanes = device->lanes;
 	dsi->format = device->format;
 	dsi->mode_flags = device->mode_flags;
+	dsi->hs_packet_end_aligned = device->hs_packet_end_aligned;
 
 	return 0;
 }
-- 
2.18.0

[v8, PATCH 3/3] drm/bridge: anx7625: config hs packets end aligned to avoid screen shift

[Rex-BC Chen]

This device requires the packets on lanes aligned at the end to fix
screen shift or scroll.

Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
Reviewed-by: Xin Ji <xji@analogixsemi.com>
Acked-by: Robert Foss <robert.foss@linaro.org>
---
 drivers/gpu/drm/bridge/analogix/anx7625.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 2346dbcc505f..672705a68dae 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -1674,6 +1674,7 @@ static int anx7625_attach_dsi(struct anx7625_data *ctx)
 	dsi->mode_flags = MIPI_DSI_MODE_VIDEO	|
 		MIPI_DSI_MODE_VIDEO_SYNC_PULSE	|
 		MIPI_DSI_MODE_VIDEO_HSE;
+	dsi->hs_packet_end_aligned = true;
 
 	ret = devm_mipi_dsi_attach(dev, dsi);
 	if (ret) {
-- 
2.18.0

Re: [v8, PATCH 1/3] drm/dsi: transfer DSI HS packets ending at the same time

[Andrzej Hajda]

Hi,

On 12.01.2022 16:36, Rex-BC Chen wrote:
> Since a HS transmission is composed of an arbitrary number
> of bytes that may not be an integer multiple of lanes, some
> lanes may run out of data before others.
> (Defined in 6.1.3 of mipi_DSI_specification_v.01-02-00)
>
> However, for some DSI RX devices (for example, anx7625),
> there is a limitation that packet number should be the same
> on all DSI lanes. In other words, they need to end a HS at
> the same time.


Is it documented in anx7625 manual? Is it confirmed with hw team?

If not, how it was detected? Have you tried to find workaround for it by 
inspecting registers, maybe it is just matter of clock gating deferral, 
timings or sth similar ???.

>
> Because this limitation is for some specific DSI RX devices,
> it is more reasonable to put the enable control in these
> DSI RX drivers. If DSI TX driver knows the information,
> they can adjust the setting for this situation.
>
> Therefore, add a flag to control this situation beacuse the
> mipi DSI specification is not forbidden this situation.


I am not sure what you mean here.

I have an impression (according t 6.1.3 of spec) that devices should 
allow transmission of arbitrary number of bytes, so this is bug in hw/fw.

The question if it can be fixed. If not patches are welcome.


>
> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> Reviewed-by: Chun-Kuang Hu <chunkuang.hu@kernel.org>
> ---
>   include/drm/drm_mipi_dsi.h | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> index 147e51b6d241..df4d15345326 100644
> --- a/include/drm/drm_mipi_dsi.h
> +++ b/include/drm/drm_mipi_dsi.h
> @@ -177,6 +177,8 @@ struct mipi_dsi_device_info {
>    * @lp_rate: maximum lane frequency for low power mode in hertz, this should
>    * be set to the real limits of the hardware, zero is only accepted for
>    * legacy drivers
> + * @hs_packet_end_aligned: transfer DSI HS packets ending at the same time
> + * for all DSI lanes
>    */
>   struct mipi_dsi_device {
>   	struct mipi_dsi_host *host;
> @@ -189,6 +191,7 @@ struct mipi_dsi_device {
>   	unsigned long mode_flags;
>   	unsigned long hs_rate;
>   	unsigned long lp_rate;
> +	bool hs_packet_end_aligned;


Maybe it would be better to add another mode_flag.


Regards

Andrzej



>   };
>   
>   #define MIPI_DSI_MODULE_PREFIX "mipi-dsi:"

Re: [v8, PATCH 1/3] drm/dsi: transfer DSI HS packets ending at the same time

[Xin Ji]

Hi Andrzej Hajda,

On Thu, Jan 13, 2022 at 01:14:14AM +0100, Andrzej Hajda wrote:
> Hi,
> 
> On 12.01.2022 16:36, Rex-BC Chen wrote:
> > Since a HS transmission is composed of an arbitrary number
> > of bytes that may not be an integer multiple of lanes, some
> > lanes may run out of data before others.
> > (Defined in 6.1.3 of mipi_DSI_specification_v.01-02-00)
> > 
> > However, for some DSI RX devices (for example, anx7625),
> > there is a limitation that packet number should be the same
> > on all DSI lanes. In other words, they need to end a HS at
> > the same time.
> 
> 
> Is it documented in anx7625 manual? Is it confirmed with hw team?
We have application note, and it is confirmed by designer, hw team.
> 
> If not, how it was detected? Have you tried to find workaround for it by
> inspecting registers, maybe it is just matter of clock gating deferral,
> timings or sth similar ???.
> 
> > 
> > Because this limitation is for some specific DSI RX devices,
> > it is more reasonable to put the enable control in these
> > DSI RX drivers. If DSI TX driver knows the information,
> > they can adjust the setting for this situation.
> > 
> > Therefore, add a flag to control this situation beacuse the
> > mipi DSI specification is not forbidden this situation.
> 
> 
> I am not sure what you mean here.
> 
> I have an impression (according t 6.1.3 of spec) that devices should allow
> transmission of arbitrary number of bytes, so this is bug in hw/fw.
This is anx7625 bridge chip design limitation.

> 
> The question if it can be fixed. If not patches are welcome.
> 
> 
> > 
> > Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> > Reviewed-by: Chun-Kuang Hu <chunkuang.hu@kernel.org>
> > ---
> >   include/drm/drm_mipi_dsi.h | 2 ++
> >   1 file changed, 2 insertions(+)
> > 
> > diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> > index 147e51b6d241..df4d15345326 100644
> > --- a/include/drm/drm_mipi_dsi.h
> > +++ b/include/drm/drm_mipi_dsi.h
> > @@ -177,6 +177,8 @@ struct mipi_dsi_device_info {
> >    * @lp_rate: maximum lane frequency for low power mode in hertz, this should
> >    * be set to the real limits of the hardware, zero is only accepted for
> >    * legacy drivers
> > + * @hs_packet_end_aligned: transfer DSI HS packets ending at the same time
> > + * for all DSI lanes
> >    */
> >   struct mipi_dsi_device {
> >   	struct mipi_dsi_host *host;
> > @@ -189,6 +191,7 @@ struct mipi_dsi_device {
> >   	unsigned long mode_flags;
> >   	unsigned long hs_rate;
> >   	unsigned long lp_rate;
> > +	bool hs_packet_end_aligned;
> 
> 
> Maybe it would be better to add another mode_flag.
mode_flag should be OK.

Thanks,
Xin
> 
> 
> Regards
> 
> Andrzej
> 
> 
> 
> >   };
> >   #define MIPI_DSI_MODULE_PREFIX "mipi-dsi:"