Re: [PATCH] drm: bridge: samsung-dsim: Fix waiting for empty cmd transfer FIFO on older Exynos

2023-07-18 Thread Marek Vasut

On 7/18/23 15:18, Marek Szyprowski wrote:

Samsung DSIM used in older Exynos SoCs (like Exynos 4210, 4x12, 3250)
doesn't report empty level of packer header FIFO. In case of those SoCs,
use the old way of waiting for empty command tranfsfer FIFO, removed
recently by commit 14806c641582 ("Drain command transfer FIFO before
transfer").

Fixes: 14806c641582 ("Drain command transfer FIFO before transfer")
Signed-off-by: Marek Szyprowski 
---
If I remember right, the problem with waiting for the empty command
transfer FIFO was there from the begging of the Exynos DSI driver and
Tomash Figa and Andrzej Hajda used a workaround based on waiting until
the DSIM_SFR_HEADER_FULL bit gets cleared. So far it worked well enough
on the older Exynos SoCs, but indeed there is no point using it on the
properly working hardware, like Exynos 5433 or IMX.

Marek Szyprowski
---
  drivers/gpu/drm/bridge/samsung-dsim.c | 11 +--
  include/drm/bridge/samsung-dsim.h |  1 +
  2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 9b7a00bafeaa..80eb268d5868 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -412,6 +412,7 @@ static const struct samsung_dsim_driver_data 
exynos3_dsi_driver_data = {
.m_min = 41,
.m_max = 125,
.min_freq = 500,
+   .has_broken_fifoctrl_emptyhdr = 1,
  };
  
  static const struct samsung_dsim_driver_data exynos4_dsi_driver_data = {

@@ -428,6 +429,7 @@ static const struct samsung_dsim_driver_data 
exynos4_dsi_driver_data = {
.m_min = 41,
.m_max = 125,
.min_freq = 500,
+   .has_broken_fifoctrl_emptyhdr = 1,
  };
  
  static const struct samsung_dsim_driver_data exynos5_dsi_driver_data = {

@@ -1009,8 +1011,13 @@ static int samsung_dsim_wait_for_hdr_fifo(struct 
samsung_dsim *dsi)
do {
u32 reg = samsung_dsim_read(dsi, DSIM_FIFOCTRL_REG);
  
-		if (reg & DSIM_SFR_HEADER_EMPTY)

-   return 0;
+   if (!dsi->driver_data->has_broken_fifoctrl_emptyhdr) {
+   if (reg & DSIM_SFR_HEADER_EMPTY)
+   return 0;
+   } else {
+   if (!(reg & DSIM_SFR_HEADER_FULL))
+   return 0;


Would it make sense to at least wait a little bit on the old hardware , 
so the data can actually leave the FIFO and be put on the line ?


I mean, imagine you have e.g. a byte of the FIFO space available (i.e. 
FIFO is not full), and you attempt to put in two bytes. The FIFO would 
overflow. If you wait a bit, then there is a chance this overflow 
condition is avoided.


[PATCH] drm: bridge: samsung-dsim: Fix waiting for empty cmd transfer FIFO on older Exynos

2023-07-18 Thread Marek Szyprowski
Samsung DSIM used in older Exynos SoCs (like Exynos 4210, 4x12, 3250)
doesn't report empty level of packer header FIFO. In case of those SoCs,
use the old way of waiting for empty command tranfsfer FIFO, removed
recently by commit 14806c641582 ("Drain command transfer FIFO before
transfer").

Fixes: 14806c641582 ("Drain command transfer FIFO before transfer")
Signed-off-by: Marek Szyprowski 
---
If I remember right, the problem with waiting for the empty command
transfer FIFO was there from the begging of the Exynos DSI driver and
Tomash Figa and Andrzej Hajda used a workaround based on waiting until
the DSIM_SFR_HEADER_FULL bit gets cleared. So far it worked well enough
on the older Exynos SoCs, but indeed there is no point using it on the
properly working hardware, like Exynos 5433 or IMX.

Marek Szyprowski
---
 drivers/gpu/drm/bridge/samsung-dsim.c | 11 +--
 include/drm/bridge/samsung-dsim.h |  1 +
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 9b7a00bafeaa..80eb268d5868 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -412,6 +412,7 @@ static const struct samsung_dsim_driver_data 
exynos3_dsi_driver_data = {
.m_min = 41,
.m_max = 125,
.min_freq = 500,
+   .has_broken_fifoctrl_emptyhdr = 1,
 };
 
 static const struct samsung_dsim_driver_data exynos4_dsi_driver_data = {
@@ -428,6 +429,7 @@ static const struct samsung_dsim_driver_data 
exynos4_dsi_driver_data = {
.m_min = 41,
.m_max = 125,
.min_freq = 500,
+   .has_broken_fifoctrl_emptyhdr = 1,
 };
 
 static const struct samsung_dsim_driver_data exynos5_dsi_driver_data = {
@@ -1009,8 +1011,13 @@ static int samsung_dsim_wait_for_hdr_fifo(struct 
samsung_dsim *dsi)
do {
u32 reg = samsung_dsim_read(dsi, DSIM_FIFOCTRL_REG);
 
-   if (reg & DSIM_SFR_HEADER_EMPTY)
-   return 0;
+   if (!dsi->driver_data->has_broken_fifoctrl_emptyhdr) {
+   if (reg & DSIM_SFR_HEADER_EMPTY)
+   return 0;
+   } else {
+   if (!(reg & DSIM_SFR_HEADER_FULL))
+   return 0;
+   }
 
if (!cond_resched())
usleep_range(950, 1050);
diff --git a/include/drm/bridge/samsung-dsim.h 
b/include/drm/bridge/samsung-dsim.h
index 05100e91ecb9..18017b3e5d9e 100644
--- a/include/drm/bridge/samsung-dsim.h
+++ b/include/drm/bridge/samsung-dsim.h
@@ -62,6 +62,7 @@ struct samsung_dsim_driver_data {
const unsigned int *reg_values;
u16 m_min;
u16 m_max;
+   unsigned int has_broken_fifoctrl_emptyhdr;
 };
 
 struct samsung_dsim_host_ops {
-- 
2.34.1