max_tu_symbol was programmed to TU_SIZE_RECOMMENDED - 1, which is not what the spec says. The spec says:
roundup ((input active video bandwidth in bytes/output active video bandwidth in bytes) * tu_size)
It is not quite clear what the above means, but calculating max_tu_symbol = (input Bps / output Bps) * tu_size seems to work and fixes the issues seen.
This fixes artifacts in some videomodes (e.g. 1024x768@60 on 2-lanes & 1.62Gbps was pretty bad for me).
Signed-off-by: Tomi Valkeinen tomi.valkeinen@ti.com --- drivers/gpu/drm/bridge/tc358767.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c index 13ade28a36a8..b6aa1bd47e1d 100644 --- a/drivers/gpu/drm/bridge/tc358767.c +++ b/drivers/gpu/drm/bridge/tc358767.c @@ -677,6 +677,8 @@ static int tc_set_video_mode(struct tc_data *tc, int upper_margin = mode->vtotal - mode->vsync_end; int lower_margin = mode->vsync_start - mode->vdisplay; int vsync_len = mode->vsync_end - mode->vsync_start; + u32 bits_per_pixel = 24; + u32 in_bw, out_bw;
/* * Recommended maximum number of symbols transferred in a transfer unit: @@ -684,7 +686,10 @@ static int tc_set_video_mode(struct tc_data *tc, * (output active video bandwidth in bytes)) * Must be less than tu_size. */ - max_tu_symbol = TU_SIZE_RECOMMENDED - 1; + + in_bw = mode->clock * bits_per_pixel / 8; + out_bw = tc->link.base.num_lanes * tc->link.base.rate; + max_tu_symbol = DIV_ROUND_UP(in_bw * TU_SIZE_RECOMMENDED, out_bw);
dev_dbg(tc->dev, "set mode %dx%d\n", mode->hdisplay, mode->vdisplay);
On 24/09/2019 16:17, Tomi Valkeinen wrote:
max_tu_symbol was programmed to TU_SIZE_RECOMMENDED - 1, which is not what the spec says. The spec says:
roundup ((input active video bandwidth in bytes/output active video bandwidth in bytes) * tu_size)
It is not quite clear what the above means, but calculating max_tu_symbol = (input Bps / output Bps) * tu_size seems to work and fixes the issues seen.
This fixes artifacts in some videomodes (e.g. 1024x768@60 on 2-lanes & 1.62Gbps was pretty bad for me).
Signed-off-by: Tomi Valkeinen tomi.valkeinen@ti.com
I could reproduce the problem and see that the patch fixes it, so:
Tested-by: Jyri Sarha jsarha@ti.com
drivers/gpu/drm/bridge/tc358767.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c index 13ade28a36a8..b6aa1bd47e1d 100644 --- a/drivers/gpu/drm/bridge/tc358767.c +++ b/drivers/gpu/drm/bridge/tc358767.c @@ -677,6 +677,8 @@ static int tc_set_video_mode(struct tc_data *tc, int upper_margin = mode->vtotal - mode->vsync_end; int lower_margin = mode->vsync_start - mode->vdisplay; int vsync_len = mode->vsync_end - mode->vsync_start;
u32 bits_per_pixel = 24;
u32 in_bw, out_bw;
/*
- Recommended maximum number of symbols transferred in a transfer unit:
@@ -684,7 +686,10 @@ static int tc_set_video_mode(struct tc_data *tc, * (output active video bandwidth in bytes)) * Must be less than tu_size. */
- max_tu_symbol = TU_SIZE_RECOMMENDED - 1;
in_bw = mode->clock * bits_per_pixel / 8;
out_bw = tc->link.base.num_lanes * tc->link.base.rate;
max_tu_symbol = DIV_ROUND_UP(in_bw * TU_SIZE_RECOMMENDED, out_bw);
dev_dbg(tc->dev, "set mode %dx%d\n", mode->hdisplay, mode->vdisplay);
On 24.09.2019 15:17, Tomi Valkeinen wrote:
max_tu_symbol was programmed to TU_SIZE_RECOMMENDED - 1, which is not what the spec says. The spec says:
roundup ((input active video bandwidth in bytes/output active video bandwidth in bytes) * tu_size)
It is not quite clear what the above means, but calculating max_tu_symbol = (input Bps / output Bps) * tu_size seems to work and fixes the issues seen.
This fixes artifacts in some videomodes (e.g. 1024x768@60 on 2-lanes & 1.62Gbps was pretty bad for me).
Signed-off-by: Tomi Valkeinen tomi.valkeinen@ti.com
Queued to fixes.
Regards
Andrzej
drivers/gpu/drm/bridge/tc358767.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c index 13ade28a36a8..b6aa1bd47e1d 100644 --- a/drivers/gpu/drm/bridge/tc358767.c +++ b/drivers/gpu/drm/bridge/tc358767.c @@ -677,6 +677,8 @@ static int tc_set_video_mode(struct tc_data *tc, int upper_margin = mode->vtotal - mode->vsync_end; int lower_margin = mode->vsync_start - mode->vdisplay; int vsync_len = mode->vsync_end - mode->vsync_start;
u32 bits_per_pixel = 24;
u32 in_bw, out_bw;
/*
- Recommended maximum number of symbols transferred in a transfer unit:
@@ -684,7 +686,10 @@ static int tc_set_video_mode(struct tc_data *tc, * (output active video bandwidth in bytes)) * Must be less than tu_size. */
- max_tu_symbol = TU_SIZE_RECOMMENDED - 1;
in_bw = mode->clock * bits_per_pixel / 8;
out_bw = tc->link.base.num_lanes * tc->link.base.rate;
max_tu_symbol = DIV_ROUND_UP(in_bw * TU_SIZE_RECOMMENDED, out_bw);
dev_dbg(tc->dev, "set mode %dx%d\n", mode->hdisplay, mode->vdisplay);
Hi Andrzej,
On 10/10/2019 12:19, Andrzej Hajda wrote:
On 24.09.2019 15:17, Tomi Valkeinen wrote:
max_tu_symbol was programmed to TU_SIZE_RECOMMENDED - 1, which is not what the spec says. The spec says:
roundup ((input active video bandwidth in bytes/output active video bandwidth in bytes) * tu_size)
It is not quite clear what the above means, but calculating max_tu_symbol = (input Bps / output Bps) * tu_size seems to work and fixes the issues seen.
This fixes artifacts in some videomodes (e.g. 1024x768@60 on 2-lanes & 1.62Gbps was pretty bad for me).
Signed-off-by: Tomi Valkeinen tomi.valkeinen@ti.com
Queued to fixes.
If you didn't push this yet, can you drop it for now? This works for all the video modes I have tested, but as I mention above, the calculation is really not quite clear to me.
I've sent queries to Toshiba about how to calculate this, and I'm hoping to get a reply soon.
If you did push it already, that's fine too, as it does improve things.
Tomi
On 10/10/2019 12:25, Tomi Valkeinen wrote:
Hi Andrzej,
On 10/10/2019 12:19, Andrzej Hajda wrote:
On 24.09.2019 15:17, Tomi Valkeinen wrote:
max_tu_symbol was programmed to TU_SIZE_RECOMMENDED - 1, which is not what the spec says. The spec says:
roundup ((input active video bandwidth in bytes/output active video bandwidth in bytes) * tu_size)
It is not quite clear what the above means, but calculating max_tu_symbol = (input Bps / output Bps) * tu_size seems to work and fixes the issues seen.
This fixes artifacts in some videomodes (e.g. 1024x768@60 on 2-lanes & 1.62Gbps was pretty bad for me).
Signed-off-by: Tomi Valkeinen tomi.valkeinen@ti.com
Queued to fixes.
If you didn't push this yet, can you drop it for now? This works for all the video modes I have tested, but as I mention above, the calculation is really not quite clear to me.
I've sent queries to Toshiba about how to calculate this, and I'm hoping to get a reply soon.
If you did push it already, that's fine too, as it does improve things.
Just for the record, got a reply from Toshiba, and the patch is correct.
Tomi
dri-devel@lists.freedesktop.org