| Message ID | 20230523104234.7849-1-angelogioacchino.delregno@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: mediatek: mtk_dsi: Fix NO_EOT_PACKET settings/handling | expand |
Il 23/05/23 12:42, AngeloGioacchino Del Regno ha scritto: > Due to the initial confusion about MIPI_DSI_MODE_EOT_PACKET, properly > renamed to MIPI_DSI_MODE_NO_EOT_PACKET, reflecting its actual meaning, > both the DSI_TXRX_CON register setting for bit (HSTX_)DIS_EOT and the > later calculation for horizontal sync-active (HSA), back (HBP) and > front (HFP) porches got incorrect due to the logic being inverted. > > This means that a number of settings were wrong because....: > - DSI_TXRX_CON register setting: bit (HSTX_)DIS_EOT should be > set in order to disable the End of Transmission packet; > - Horizontal Sync and Back/Front porches: The delta used to > calculate all of HSA, HBP and HFP should account for the > additional EOT packet. > > Before this change... > - Bit (HSTX_)DIS_EOT was being set when EOT packet was enabled; > - For HSA/HBP/HFP delta... all three were wrong, as words were > added when EOT disabled, instead of when EOT packet enabled! > > Invert the logic around flag MIPI_DSI_MODE_NO_EOT_PACKET in the > MediaTek DSI driver to fix the aforementioned issues. > > Fixes: 8b2b99fd7931 ("drm/mediatek: dsi: Fine tune the line time caused by EOTp") > Fixes: 2d52bfba09d1 ("drm/mediatek: add non-continuous clock mode and EOT packet control") > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> Gentle ping for an important fix. Regards, Angelo > --- > drivers/gpu/drm/mediatek/mtk_dsi.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c > index 7d5250351193..b0ab38e59db9 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > @@ -407,7 +407,7 @@ static void mtk_dsi_rxtx_control(struct mtk_dsi *dsi) > if (dsi->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) > tmp_reg |= HSTX_CKLP_EN; > > - if (!(dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET)) > + if (dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET) > tmp_reg |= DIS_EOT; > > writel(tmp_reg, dsi->regs + DSI_TXRX_CTRL); > @@ -484,7 +484,7 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi) > timing->da_hs_zero + timing->da_hs_exit + 3; > > delta = dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST ? 18 : 12; > - delta += dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET ? 2 : 0; > + delta += dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET ? 0 : 2; > > horizontal_frontporch_byte = vm->hfront_porch * dsi_tmp_buf_bpp; > horizontal_front_back_byte = horizontal_frontporch_byte + horizontal_backporch_byte;
On 23/06/2023 11:47, AngeloGioacchino Del Regno wrote: > Il 23/05/23 12:42, AngeloGioacchino Del Regno ha scritto: >> Due to the initial confusion about MIPI_DSI_MODE_EOT_PACKET, properly >> renamed to MIPI_DSI_MODE_NO_EOT_PACKET, reflecting its actual meaning, >> both the DSI_TXRX_CON register setting for bit (HSTX_)DIS_EOT and the >> later calculation for horizontal sync-active (HSA), back (HBP) and >> front (HFP) porches got incorrect due to the logic being inverted. >> >> This means that a number of settings were wrong because....: >> - DSI_TXRX_CON register setting: bit (HSTX_)DIS_EOT should be >> set in order to disable the End of Transmission packet; >> - Horizontal Sync and Back/Front porches: The delta used to >> calculate all of HSA, HBP and HFP should account for the >> additional EOT packet. >> >> Before this change... >> - Bit (HSTX_)DIS_EOT was being set when EOT packet was enabled; >> - For HSA/HBP/HFP delta... all three were wrong, as words were >> added when EOT disabled, instead of when EOT packet enabled! >> >> Invert the logic around flag MIPI_DSI_MODE_NO_EOT_PACKET in the >> MediaTek DSI driver to fix the aforementioned issues. >> >> Fixes: 8b2b99fd7931 ("drm/mediatek: dsi: Fine tune the line time >> caused by EOTp") >> Fixes: 2d52bfba09d1 ("drm/mediatek: add non-continuous clock mode and >> EOT packet control") >> Signed-off-by: AngeloGioacchino Del Regno >> <angelogioacchino.delregno@collabora.com> > > Gentle ping for an important fix. > > Regards, > Angelo > >> --- >> drivers/gpu/drm/mediatek/mtk_dsi.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c >> b/drivers/gpu/drm/mediatek/mtk_dsi.c >> index 7d5250351193..b0ab38e59db9 100644 >> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c >> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c >> @@ -407,7 +407,7 @@ static void mtk_dsi_rxtx_control(struct mtk_dsi *dsi) >> if (dsi->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) >> tmp_reg |= HSTX_CKLP_EN; >> - if (!(dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET)) >> + if (dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET) >> tmp_reg |= DIS_EOT; >> writel(tmp_reg, dsi->regs + DSI_TXRX_CTRL); >> @@ -484,7 +484,7 @@ static void mtk_dsi_config_vdo_timing(struct >> mtk_dsi *dsi) >> timing->da_hs_zero + timing->da_hs_exit + 3; >> delta = dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST ? 18 : 12; >> - delta += dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET ? 2 : 0; >> + delta += dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET ? 0 : 2; >> horizontal_frontporch_byte = vm->hfront_porch * dsi_tmp_buf_bpp; >> horizontal_front_back_byte = horizontal_frontporch_byte + >> horizontal_backporch_byte; > > Sounds logic Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
Il 23/06/23 13:29, Alexandre Mergnat ha scritto: > > > On 23/06/2023 11:47, AngeloGioacchino Del Regno wrote: >> Il 23/05/23 12:42, AngeloGioacchino Del Regno ha scritto: >>> Due to the initial confusion about MIPI_DSI_MODE_EOT_PACKET, properly >>> renamed to MIPI_DSI_MODE_NO_EOT_PACKET, reflecting its actual meaning, >>> both the DSI_TXRX_CON register setting for bit (HSTX_)DIS_EOT and the >>> later calculation for horizontal sync-active (HSA), back (HBP) and >>> front (HFP) porches got incorrect due to the logic being inverted. >>> >>> This means that a number of settings were wrong because....: >>> - DSI_TXRX_CON register setting: bit (HSTX_)DIS_EOT should be >>> set in order to disable the End of Transmission packet; >>> - Horizontal Sync and Back/Front porches: The delta used to >>> calculate all of HSA, HBP and HFP should account for the >>> additional EOT packet. >>> >>> Before this change... >>> - Bit (HSTX_)DIS_EOT was being set when EOT packet was enabled; >>> - For HSA/HBP/HFP delta... all three were wrong, as words were >>> added when EOT disabled, instead of when EOT packet enabled! >>> >>> Invert the logic around flag MIPI_DSI_MODE_NO_EOT_PACKET in the >>> MediaTek DSI driver to fix the aforementioned issues. >>> >>> Fixes: 8b2b99fd7931 ("drm/mediatek: dsi: Fine tune the line time caused by EOTp") >>> Fixes: 2d52bfba09d1 ("drm/mediatek: add non-continuous clock mode and EOT packet >>> control") >>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> >> >> Gentle ping for an important fix. >> >> Regards, >> Angelo >> >>> --- >>> drivers/gpu/drm/mediatek/mtk_dsi.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c >>> b/drivers/gpu/drm/mediatek/mtk_dsi.c >>> index 7d5250351193..b0ab38e59db9 100644 >>> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c >>> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c >>> @@ -407,7 +407,7 @@ static void mtk_dsi_rxtx_control(struct mtk_dsi *dsi) >>> if (dsi->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) >>> tmp_reg |= HSTX_CKLP_EN; >>> - if (!(dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET)) >>> + if (dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET) >>> tmp_reg |= DIS_EOT; >>> writel(tmp_reg, dsi->regs + DSI_TXRX_CTRL); >>> @@ -484,7 +484,7 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi) >>> timing->da_hs_zero + timing->da_hs_exit + 3; >>> delta = dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST ? 18 : 12; >>> - delta += dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET ? 2 : 0; >>> + delta += dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET ? 0 : 2; >>> horizontal_frontporch_byte = vm->hfront_porch * dsi_tmp_buf_bpp; >>> horizontal_front_back_byte = horizontal_frontporch_byte + >>> horizontal_backporch_byte; >> >> > > Sounds logic > > Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> > Ping again. Regards, Angelo
Hi, Jitao: Do you have any comment? If you have no comment, I would apply this patch. Regards, Chun-Kuang. AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> 於 2023年5月23日 週二 下午6:42寫道： > > Due to the initial confusion about MIPI_DSI_MODE_EOT_PACKET, properly > renamed to MIPI_DSI_MODE_NO_EOT_PACKET, reflecting its actual meaning, > both the DSI_TXRX_CON register setting for bit (HSTX_)DIS_EOT and the > later calculation for horizontal sync-active (HSA), back (HBP) and > front (HFP) porches got incorrect due to the logic being inverted. > > This means that a number of settings were wrong because....: > - DSI_TXRX_CON register setting: bit (HSTX_)DIS_EOT should be > set in order to disable the End of Transmission packet; > - Horizontal Sync and Back/Front porches: The delta used to > calculate all of HSA, HBP and HFP should account for the > additional EOT packet. > > Before this change... > - Bit (HSTX_)DIS_EOT was being set when EOT packet was enabled; > - For HSA/HBP/HFP delta... all three were wrong, as words were > added when EOT disabled, instead of when EOT packet enabled! > > Invert the logic around flag MIPI_DSI_MODE_NO_EOT_PACKET in the > MediaTek DSI driver to fix the aforementioned issues. > > Fixes: 8b2b99fd7931 ("drm/mediatek: dsi: Fine tune the line time caused by EOTp") > Fixes: 2d52bfba09d1 ("drm/mediatek: add non-continuous clock mode and EOT packet control") > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > --- > drivers/gpu/drm/mediatek/mtk_dsi.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c > index 7d5250351193..b0ab38e59db9 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > @@ -407,7 +407,7 @@ static void mtk_dsi_rxtx_control(struct mtk_dsi *dsi) > if (dsi->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) > tmp_reg |= HSTX_CKLP_EN; > > - if (!(dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET)) > + if (dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET) > tmp_reg |= DIS_EOT; > > writel(tmp_reg, dsi->regs + DSI_TXRX_CTRL); > @@ -484,7 +484,7 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi) > timing->da_hs_zero + timing->da_hs_exit + 3; > > delta = dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST ? 18 : 12; > - delta += dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET ? 2 : 0; > + delta += dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET ? 0 : 2; > > horizontal_frontporch_byte = vm->hfront_porch * dsi_tmp_buf_bpp; > horizontal_front_back_byte = horizontal_frontporch_byte + horizontal_backporch_byte; > -- > 2.40.1 >
> Due to the initial confusion about MIPI_DSI_MODE_EOT_PACKET, properly > renamed to MIPI_DSI_MODE_NO_EOT_PACKET, reflecting its actual meaning, > both the DSI_TXRX_CON register setting for bit (HSTX_)DIS_EOT and the > later calculation for horizontal sync-active (HSA), back (HBP) and > front (HFP) porches got incorrect due to the logic being inverted. > > This means that a number of settings were wrong because....: > - DSI_TXRX_CON register setting: bit (HSTX_)DIS_EOT should be > set in order to disable the End of Transmission packet; > - Horizontal Sync and Back/Front porches: The delta used to > calculate all of HSA, HBP and HFP should account for the > additional EOT packet. > > Before this change... > - Bit (HSTX_)DIS_EOT was being set when EOT packet was enabled; > - For HSA/HBP/HFP delta... all three were wrong, as words were > added when EOT disabled, instead of when EOT packet enabled! > > Invert the logic around flag MIPI_DSI_MODE_NO_EOT_PACKET in the > MediaTek DSI driver to fix the aforementioned issues. > > Fixes: 8b2b99fd7931 ("drm/mediatek: dsi: Fine tune the line time caused by EOTp") > Fixes: 2d52bfba09d1 ("drm/mediatek: add non-continuous clock mode and EOT packet control") > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> Tested-by: Michael Walle <mwalle@kernel.org> Thanks, -michael
Il 15/09/23 12:11, Michael Walle ha scritto: >> Due to the initial confusion about MIPI_DSI_MODE_EOT_PACKET, properly >> renamed to MIPI_DSI_MODE_NO_EOT_PACKET, reflecting its actual meaning, >> both the DSI_TXRX_CON register setting for bit (HSTX_)DIS_EOT and the >> later calculation for horizontal sync-active (HSA), back (HBP) and >> front (HFP) porches got incorrect due to the logic being inverted. >> >> This means that a number of settings were wrong because....: >> - DSI_TXRX_CON register setting: bit (HSTX_)DIS_EOT should be >> set in order to disable the End of Transmission packet; >> - Horizontal Sync and Back/Front porches: The delta used to >> calculate all of HSA, HBP and HFP should account for the >> additional EOT packet. >> >> Before this change... >> - Bit (HSTX_)DIS_EOT was being set when EOT packet was enabled; >> - For HSA/HBP/HFP delta... all three were wrong, as words were >> added when EOT disabled, instead of when EOT packet enabled! >> >> Invert the logic around flag MIPI_DSI_MODE_NO_EOT_PACKET in the >> MediaTek DSI driver to fix the aforementioned issues. >> >> Fixes: 8b2b99fd7931 ("drm/mediatek: dsi: Fine tune the line time caused by EOTp") >> Fixes: 2d52bfba09d1 ("drm/mediatek: add non-continuous clock mode and EOT packet control") >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > Hello CK, can you please pick this fix? Thanks, Angelo
Hi, Angelo: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> 於 2023年10月18日 週三 下午6:21寫道： > > Il 15/09/23 12:11, Michael Walle ha scritto: > >> Due to the initial confusion about MIPI_DSI_MODE_EOT_PACKET, properly > >> renamed to MIPI_DSI_MODE_NO_EOT_PACKET, reflecting its actual meaning, > >> both the DSI_TXRX_CON register setting for bit (HSTX_)DIS_EOT and the > >> later calculation for horizontal sync-active (HSA), back (HBP) and > >> front (HFP) porches got incorrect due to the logic being inverted. > >> > >> This means that a number of settings were wrong because....: > >> - DSI_TXRX_CON register setting: bit (HSTX_)DIS_EOT should be > >> set in order to disable the End of Transmission packet; > >> - Horizontal Sync and Back/Front porches: The delta used to > >> calculate all of HSA, HBP and HFP should account for the > >> additional EOT packet. > >> > >> Before this change... > >> - Bit (HSTX_)DIS_EOT was being set when EOT packet was enabled; > >> - For HSA/HBP/HFP delta... all three were wrong, as words were > >> added when EOT disabled, instead of when EOT packet enabled! > >> > >> Invert the logic around flag MIPI_DSI_MODE_NO_EOT_PACKET in the > >> MediaTek DSI driver to fix the aforementioned issues. > >> > >> Fixes: 8b2b99fd7931 ("drm/mediatek: dsi: Fine tune the line time caused by EOTp") > >> Fixes: 2d52bfba09d1 ("drm/mediatek: add non-continuous clock mode and EOT packet control") > >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > > > > Hello CK, > > can you please pick this fix? Applied with the changing Fixes: 2d52bfba09d1 ("drm/mediatek: add non-continuous clock mode and EOT packet control") to Fixes: c87d1c4b5b9a ("drm/mediatek: dsi: Use symbolized register definition") Regards, Chun-Kuang. > > Thanks, > Angelo >
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 7d5250351193..b0ab38e59db9 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -407,7 +407,7 @@ static void mtk_dsi_rxtx_control(struct mtk_dsi *dsi) if (dsi->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) tmp_reg |= HSTX_CKLP_EN; - if (!(dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET)) + if (dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET) tmp_reg |= DIS_EOT; writel(tmp_reg, dsi->regs + DSI_TXRX_CTRL); @@ -484,7 +484,7 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi) timing->da_hs_zero + timing->da_hs_exit + 3; delta = dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST ? 18 : 12; - delta += dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET ? 2 : 0; + delta += dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET ? 0 : 2; horizontal_frontporch_byte = vm->hfront_porch * dsi_tmp_buf_bpp; horizontal_front_back_byte = horizontal_frontporch_byte + horizontal_backporch_byte;
Due to the initial confusion about MIPI_DSI_MODE_EOT_PACKET, properly renamed to MIPI_DSI_MODE_NO_EOT_PACKET, reflecting its actual meaning, both the DSI_TXRX_CON register setting for bit (HSTX_)DIS_EOT and the later calculation for horizontal sync-active (HSA), back (HBP) and front (HFP) porches got incorrect due to the logic being inverted. This means that a number of settings were wrong because....: - DSI_TXRX_CON register setting: bit (HSTX_)DIS_EOT should be set in order to disable the End of Transmission packet; - Horizontal Sync and Back/Front porches: The delta used to calculate all of HSA, HBP and HFP should account for the additional EOT packet. Before this change... - Bit (HSTX_)DIS_EOT was being set when EOT packet was enabled; - For HSA/HBP/HFP delta... all three were wrong, as words were added when EOT disabled, instead of when EOT packet enabled! Invert the logic around flag MIPI_DSI_MODE_NO_EOT_PACKET in the MediaTek DSI driver to fix the aforementioned issues. Fixes: 8b2b99fd7931 ("drm/mediatek: dsi: Fine tune the line time caused by EOTp") Fixes: 2d52bfba09d1 ("drm/mediatek: add non-continuous clock mode and EOT packet control") Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> --- drivers/gpu/drm/mediatek/mtk_dsi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)