[PATCH 0/8] video: tc358767: fixes and improvements

[Andrey Gusakov]

Hi Barebox,

This set of patches fixes several issues that was found during testing
tc358767 with several desktop DP displays.

Andrey Gusakov (8):
video: tc358767: do not fail if sink supports more than 2 lanes
video: tc358767: support newer DPCD revisions and higher data rates
video: tc358767: fix EDID read for DP displays
video: tc358767: fix DP0_MISC register set
video: tc358767: fix timing calculation
video: tc358767: fix AUXDATAn registers access
video: tc358767: filter out modes with too high pixelclock
video: tc358767: accept any hsync and vsync polatiry

drivers/video/tc358767.c | 218 ++++++++++++++++++++++++++++++++++++++---------
1 file changed, 179 insertions(+), 39 deletions(-)

[Andrey Gusakov]

Display should support lower lane count for backward compatibility
---
drivers/video/tc358767.c | 36 +++++++++++++++++++++++++++++++-----
1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/drivers/video/tc358767.c b/drivers/video/tc358767.c
index ab2e86ec4..eaf3baad3 100644
--- a/drivers/video/tc358767.c
+++ b/drivers/video/tc358767.c
@@ -36,6 +36,7 @@

#define DP_LINK_BW_SET 0x100
#define DP_TRAINING_PATTERN_SET 0x102
+#define TRAINING_LANE0_SET 0x103

#define DP_DOWNSPREAD_CTRL 0x107
#define DP_SPREAD_AMP_0_5 (1 << 4)
@@ -681,6 +682,12 @@ static int tc_get_display_props(struct tc_data *tc)
tc->link.rev = tmp[0];
tc->link.rate = tmp[1];
tc->link.lanes = tmp[2] & 0x0f;
+ if (tc->link.lanes > 2) {
+ dev_dbg(tc->dev, "Display supports %d lanes, host only 2 max. "
+ "Tryin 2-lane mode.\n",
+ tc->link.lanes);
+ tc->link.lanes = 2;
+ }
tc->link.enhanced = !!(tmp[2] & 0x80);
tc->link.spread = tmp[3] & 0x01;
tc->link.coding8b10b = tmp[6] & 0x01;
@@ -985,14 +992,30 @@ static int tc_main_link_setup(struct tc_data *tc)
if (ret)
goto err_dpcd_write;

+ /* TRAINING_LANE0_SET .. TRAINING_LANE3_SET */
+ tmp[0] = 0x00;
+ tmp[1] = 0x00;
+ tmp[2] = 0x00;
+ tmp[3] = 0x00;
/* DOWNSPREAD_CTRL */
- tmp[0] = tc->link.spread ? DP_SPREAD_AMP_0_5 : 0x00;
+ tmp[4] = tc->link.spread ? DP_SPREAD_AMP_0_5 : 0x00;
/* MAIN_LINK_CHANNEL_CODING_SET */
- tmp[1] = tc->link.coding8b10b ? DP_SET_ANSI_8B10B : 0x00;
- ret = tc_aux_write(tc, DP_DOWNSPREAD_CTRL, tmp, 2);
+ tmp[5] = tc->link.coding8b10b ? DP_SET_ANSI_8B10B : 0x00;
+ ret = tc_aux_write(tc, TRAINING_LANE0_SET, tmp, 6);
if (ret)
goto err_dpcd_write;

+ /* check lanes */
+ ret = tc_aux_read(tc, DP_LINK_BW_SET, tmp, 2);
+ if (ret)
+ goto err_dpcd_read;
+
+ if ((tmp[1] & 0x0f) != tc->link.lanes) {
+ dev_err(dev, "Failed to set lanes = %d on display side\n",
+ tc->link.lanes);
+ goto err;
+ }
+
ret = tc_link_training(tc, DP_TRAINING_PATTERN_1);
if (ret)
goto err;
@@ -1019,8 +1042,11 @@ static int tc_main_link_setup(struct tc_data *tc)
ret = tc_aux_read(tc, 0x200, tmp, 7);
if (ret)
goto err_dpcd_read;
- ready = (tmp[2] == ((DP_CHANNEL_EQ_BITS << 4) | /* Lane1 */
- DP_CHANNEL_EQ_BITS)); /* Lane0 */
+ if (tc->link.lanes == 1)
+ ready = (tmp[2] == DP_CHANNEL_EQ_BITS); /* Lane0 */
+ else
+ ready = (tmp[2] == ((DP_CHANNEL_EQ_BITS << 4) | /* Lane1 */
+ DP_CHANNEL_EQ_BITS)); /* Lane0 */
aligned = tmp[4] & DP_INTERLANE_ALIGN_DONE;
} while ((--timeout) && !(ready && aligned));

[Andrey Gusakov]

Do not fail on newer DPCD revision. Assume it backwards compatible.
Try to fall back to supported data rate in case display reports too
high data rate.
---
drivers/video/tc358767.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/video/tc358767.c b/drivers/video/tc358767.c
index eaf3baad3..3b9b4b8ad 100644
--- a/drivers/video/tc358767.c
+++ b/drivers/video/tc358767.c
@@ -674,13 +674,25 @@ static int tc_get_display_props(struct tc_data *tc)
ret = tc_aux_read(tc, 0x000, tmp, 8);
if (ret)
goto err_dpcd_read;
- /* check rev 1.0 or 1.1 */
- if ((tmp[1] != 0x06) && (tmp[1] != 0x0a))
- goto err_dpcd_inval;

tc->assr = !(tc->rev & 0x02);
+
+ /* check DPCD rev */
+ if (tmp[0] < 0x10) {
+ dev_err(tc->dev, "Too low DPCD revision 0x%02x\n", tmp[0]);
+ goto err_dpcd_inval;
+ }
+ if ((tmp[0] != 0x10) && (tmp[0] != 0x11))
+ dev_err(tc->dev, "Unknown DPCD revision 0x%02x\n", tmp[0]);
tc->link.rev = tmp[0];
- tc->link.rate = tmp[1];
+
+ /* check rate */
+ if ((tmp[1] == 0x06) || (tmp[1] == 0x0a)) {
+ tc->link.rate = tmp[1];
+ } else {
+ dev_err(tc->dev, "Unknown link rate 0x%02x, falling to 2.7Gbps\n", tmp[1]);
+ tc->link.rate = 0x0a;
+ }
tc->link.lanes = tmp[2] & 0x0f;
if (tc->link.lanes > 2) {
dev_dbg(tc->dev, "Display supports %d lanes, host only 2 max. "

[Andrey Gusakov]

Desktop displays do not always support E-DDC. So do not fall on
EDID segment index write operation.
---
drivers/video/tc358767.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/video/tc358767.c b/drivers/video/tc358767.c
index 3b9b4b8ad..3d38f86ba 100644
--- a/drivers/video/tc358767.c
+++ b/drivers/video/tc358767.c
@@ -1146,13 +1146,16 @@ static int tc_read_edid(struct tc_data *tc)
unsigned char start = 0;
unsigned char segment = 0;

- struct i2c_msg msgs[] = {
+ struct i2c_msg msg_segment[] = {
{
.addr = DDC_SEGMENT_ADDR,
.flags = 0,
.len = 1,
.buf = &segment,
- }, {
+ }
+ };
+ struct i2c_msg msgs[] = {
+ {
.addr = DDC_ADDR,
.flags = 0,
.len = 1,
@@ -1164,13 +1167,21 @@ static int tc_read_edid(struct tc_data *tc)
};
tc->edid = xmalloc(EDID_LENGTH);

+ /*
+ * Some displays supports E-EDID some not
+ * Just reset segment address to 0x0
+ * This transfer can fail on non E-DCC displays
+ * Ignore error
+ */
+ i2c_transfer(&tc->adapter, msg_segment, 1);
+
do {
block = min(DDC_BLOCK_READ, EDID_LENGTH - i);

- msgs[2].buf = tc->edid + i;
- msgs[2].len = block;
+ msgs[1].buf = tc->edid + i;
+ msgs[1].len = block;

- ret = i2c_transfer(&tc->adapter, msgs, 3);
+ ret = i2c_transfer(&tc->adapter, msgs, 2);
if (ret < 0)
goto err;

[Andrey Gusakov]

Remove shift from TU_SIZE_RECOMMENDED define as it used to
calculate max_tu_symbols.
---
drivers/video/tc358767.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/tc358767.c b/drivers/video/tc358767.c
index 3d38f86ba..033da4fb2 100644
--- a/drivers/video/tc358767.c
+++ b/drivers/video/tc358767.c
@@ -122,7 +122,7 @@
#define DP0_ACTIVEVAL 0x0650
#define DP0_SYNCVAL 0x0654
#define DP0_MISC 0x0658
-#define TU_SIZE_RECOMMENDED (0x3f << 16) /* LSCLK cycles per TU */
+#define TU_SIZE_RECOMMENDED (63) /* LSCLK cycles per TU */
#define BPC_6 (0 << 5)
#define BPC_8 (1 << 5)

@@ -799,7 +799,7 @@ static int tc_set_video_mode(struct tc_data *tc, struct fb_videomode *mode)
* Must be less than tu_size.
*/
max_tu_symbol = TU_SIZE_RECOMMENDED - 1;
- tc_write(DP0_MISC, (max_tu_symbol << 23) | TU_SIZE_RECOMMENDED | BPC_8);
+ tc_write(DP0_MISC, (max_tu_symbol << 23) | (TU_SIZE_RECOMMENDED << 16) | BPC_8);

return 0;
err:

[Andrey Gusakov]

Also optimize DPCD writes
---
drivers/video/tc358767.c | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/video/tc358767.c b/drivers/video/tc358767.c
index d2c94c616..1541b18c9 100644
--- a/drivers/video/tc358767.c
+++ b/drivers/video/tc358767.c
@@ -35,6 +35,7 @@
#include <asm-generic/div64.h>

#define DP_LINK_BW_SET 0x100
+#define DP_ENHANCED_FRAME_EN (1 << 7)
#define DP_TRAINING_PATTERN_SET 0x102
#define TRAINING_LANE0_SET 0x103

@@ -389,7 +390,7 @@ static int tc_aux_write(struct tc_data *tc, int reg, char *data, int size)
i++;
if (((i % 4) == 0) ||
(i == size)) {
- tc_write(DP0_AUXWDATA(i >> 2), tmp);
+ tc_write(DP0_AUXWDATA((i - 1) >> 2), tmp);
tmp = 0;
}
}
@@ -477,7 +478,7 @@ static int tc_aux_i2c_write(struct tc_data *tc, struct i2c_msg *msg)
i++;
if (((i % 4) == 0) ||
(i == msg->len)) {
- tc_write(DP0_AUXWDATA(i >> 2), tmp);
+ tc_write(DP0_AUXWDATA((i - 1) >> 2), tmp);
tmp = 0;
}
}
@@ -903,7 +904,7 @@ static int tc_main_link_setup(struct tc_data *tc)
bool ready;
u32 value;
int ret;
- u8 tmp[8];
+ u8 tmp[16];

/* display mode should be set at this point */
if (!tc->mode)
@@ -1004,21 +1005,22 @@ static int tc_main_link_setup(struct tc_data *tc)
/* LANE_COUNT_SET */
tmp[1] = tc->link.lanes;
if (tc->link.enhanced)
- tmp[1] |= (1 << 7);
- ret = tc_aux_write(tc, DP_LINK_BW_SET, tmp, 2);
- if (ret)
- goto err_dpcd_write;
+ tmp[1] |= DP_ENHANCED_FRAME_EN;

- /* TRAINING_LANE0_SET .. TRAINING_LANE3_SET */
- tmp[0] = 0x00;
- tmp[1] = 0x00;
+ /* TRAINING_PATTERN_SET */
tmp[2] = 0x00;
+ /* TRAINING_LANE0_SET .. TRAINING_LANE3_SET */
tmp[3] = 0x00;
+ tmp[4] = 0x00;
+ tmp[5] = 0x00;
+ tmp[6] = 0x00;
/* DOWNSPREAD_CTRL */
- tmp[4] = tc->link.spread ? DP_SPREAD_AMP_0_5 : 0x00;
+ tmp[7] = tc->link.spread ? DP_SPREAD_AMP_0_5 : 0x00;
/* MAIN_LINK_CHANNEL_CODING_SET */
- tmp[5] = tc->link.coding8b10b ? DP_SET_ANSI_8B10B : 0x00;
- ret = tc_aux_write(tc, TRAINING_LANE0_SET, tmp, 6);
+ tmp[8] = tc->link.coding8b10b ? DP_SET_ANSI_8B10B : 0x00;
+
+ /* DP_LINK_BW_SET .. MAIN_LINK_CHANNEL_CODING_SET */
+ ret = tc_aux_write(tc, DP_LINK_BW_SET, tmp, 9);
if (ret)
goto err_dpcd_write;

[Lucas Stach]

Post by Andrey Gusakov
Also optimize DPCD writes

The word "also" in a commit message is a pretty strong hint that this
should be in a separate patch. The message of this commit should explain
what the fix for the AUXDATA access is without me needing to read the
code.

Regards,
Lucas

Post by Andrey Gusakov
---
drivers/video/tc358767.c | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/drivers/video/tc358767.c b/drivers/video/tc358767.c
index d2c94c616..1541b18c9 100644
--- a/drivers/video/tc358767.c
+++ b/drivers/video/tc358767.c
@@ -35,6 +35,7 @@
#include <asm-generic/div64.h>
#define DP_LINK_BW_SET 0x100
+#define DP_ENHANCED_FRAME_EN (1 << 7)
#define DP_TRAINING_PATTERN_SET 0x102
#define TRAINING_LANE0_SET 0x103
@@ -389,7 +390,7 @@ static int tc_aux_write(struct tc_data *tc, int reg, char *data, int size)
i++;
if (((i % 4) == 0) ||
(i == size)) {
- tc_write(DP0_AUXWDATA(i >> 2), tmp);
+ tc_write(DP0_AUXWDATA((i - 1) >> 2), tmp);
tmp = 0;
}
}
@@ -477,7 +478,7 @@ static int tc_aux_i2c_write(struct tc_data *tc, struct i2c_msg *msg)
i++;
if (((i % 4) == 0) ||
(i == msg->len)) {
- tc_write(DP0_AUXWDATA(i >> 2), tmp);
+ tc_write(DP0_AUXWDATA((i - 1) >> 2), tmp);
tmp = 0;
}
}
@@ -903,7 +904,7 @@ static int tc_main_link_setup(struct tc_data *tc)
bool ready;
u32 value;
int ret;
- u8 tmp[8];
+ u8 tmp[16];
/* display mode should be set at this point */
if (!tc->mode)
@@ -1004,21 +1005,22 @@ static int tc_main_link_setup(struct tc_data *tc)
/* LANE_COUNT_SET */
tmp[1] = tc->link.lanes;
if (tc->link.enhanced)
- tmp[1] |= (1 << 7);
- ret = tc_aux_write(tc, DP_LINK_BW_SET, tmp, 2);
- if (ret)
- goto err_dpcd_write;
+ tmp[1] |= DP_ENHANCED_FRAME_EN;
- /* TRAINING_LANE0_SET .. TRAINING_LANE3_SET */
- tmp[0] = 0x00;
- tmp[1] = 0x00;
+ /* TRAINING_PATTERN_SET */
tmp[2] = 0x00;
+ /* TRAINING_LANE0_SET .. TRAINING_LANE3_SET */
tmp[3] = 0x00;
+ tmp[4] = 0x00;
+ tmp[5] = 0x00;
+ tmp[6] = 0x00;
/* DOWNSPREAD_CTRL */
- tmp[4] = tc->link.spread ? DP_SPREAD_AMP_0_5 : 0x00;
+ tmp[7] = tc->link.spread ? DP_SPREAD_AMP_0_5 : 0x00;
/* MAIN_LINK_CHANNEL_CODING_SET */
- tmp[5] = tc->link.coding8b10b ? DP_SET_ANSI_8B10B : 0x00;
- ret = tc_aux_write(tc, TRAINING_LANE0_SET, tmp, 6);
+ tmp[8] = tc->link.coding8b10b ? DP_SET_ANSI_8B10B : 0x00;
+
+ /* DP_LINK_BW_SET .. MAIN_LINK_CHANNEL_CODING_SET */
+ ret = tc_aux_write(tc, DP_LINK_BW_SET, tmp, 9);
if (ret)
goto err_dpcd_write;

[Andrey Gusakov]

Fields in HTIM01 and HTIM02 regs should be even.
Recomended thresh_dly value is max_tu_symbol.
---
drivers/video/tc358767.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/video/tc358767.c b/drivers/video/tc358767.c
index 033da4fb2..d2c94c616 100644
--- a/drivers/video/tc358767.c
+++ b/drivers/video/tc358767.c
@@ -730,13 +730,15 @@ err_dpcd_inval:
return -EINVAL;
}

+#define EVEN_HI(val) ((val + 1) & (~0x01))
+
static int tc_set_video_mode(struct tc_data *tc, struct fb_videomode *mode)
{
int ret;
int htotal;
int vtotal;
int vid_sync_dly;
- int max_tu_symbol;
+ int max_tu_symbol = TU_SIZE_RECOMMENDED - 1;

htotal = mode->hsync_len + mode->left_margin + mode->xres +
mode->right_margin;
@@ -750,14 +752,18 @@ static int tc_set_video_mode(struct tc_data *tc, struct fb_videomode *mode)
mode->upper_margin, mode->lower_margin, mode->vsync_len);
dev_dbg(tc->dev, "total: %dx%d\n", htotal, vtotal);

-
- /* LCD Ctl Frame Size */
- tc_write(VPCTRL0, (0x40 << 20) /* VSDELAY */ |
+ /*
+ * Datasheet is not clear of vsdelay in case of DPI
+ * assume we do not need any delay when DPI is a source of
+ * sync signals
+ */
+ tc_write(VPCTRL0, (0 << 20) /* VSDELAY */ |
OPXLFMT_RGB888 | FRMSYNC_DISABLED | MSF_DISABLED);
- tc_write(HTIM01, (mode->left_margin << 16) | /* H back porch */
- (mode->hsync_len << 0)); /* Hsync */
- tc_write(HTIM02, (mode->right_margin << 16) | /* H front porch */
- (mode->xres << 0)); /* width */
+ /* LCD Ctl Frame Size */
+ tc_write(HTIM01, (EVEN_HI(mode->left_margin) << 16) | /* H back porch */
+ (EVEN_HI(mode->hsync_len) << 0)); /* Hsync */
+ tc_write(HTIM02, (EVEN_HI(mode->right_margin) << 16) | /* H front porch */
+ (EVEN_HI(mode->xres) << 0)); /* width */
tc_write(VTIM01, (mode->upper_margin << 16) | /* V back porch */
(mode->vsync_len << 0)); /* Vsync */
tc_write(VTIM02, (mode->lower_margin << 16) | /* V front porch */
@@ -776,7 +782,7 @@ static int tc_set_video_mode(struct tc_data *tc, struct fb_videomode *mode)
/* DP Main Stream Attributes */
vid_sync_dly = mode->hsync_len + mode->left_margin + mode->xres;
tc_write(DP0_VIDSYNCDELAY,
- (0x003e << 16) | /* thresh_dly */
+ (max_tu_symbol << 16) | /* thresh_dly */
(vid_sync_dly << 0));

tc_write(DP0_TOTALVAL, (vtotal << 16) | (htotal));
@@ -798,7 +804,6 @@ static int tc_set_video_mode(struct tc_data *tc, struct fb_videomode *mode)
* (output active video bandwidth in bytes))
* Must be less than tu_size.
*/
- max_tu_symbol = TU_SIZE_RECOMMENDED - 1;
tc_write(DP0_MISC, (max_tu_symbol << 23) | (TU_SIZE_RECOMMENDED << 16) | BPC_8);

return 0;

[Lucas Stach]

Post by Andrey Gusakov
Fields in HTIM01 and HTIM02 regs should be even.
Recomended thresh_dly value is max_tu_symbol.
---
drivers/video/tc358767.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/video/tc358767.c b/drivers/video/tc358767.c
index 033da4fb2..d2c94c616 100644
--- a/drivers/video/tc358767.c
+++ b/drivers/video/tc358767.c
return -EINVAL;
}
+#define EVEN_HI(val) ((val + 1) & (~0x01))
+

Please use the commonly used ALIGN instead of defining this on your own.

Post by Andrey Gusakov
static int tc_set_video_mode(struct tc_data *tc, struct fb_videomode *mode)
{
int ret;
int htotal;
int vtotal;
int vid_sync_dly;
- int max_tu_symbol;
+ int max_tu_symbol = TU_SIZE_RECOMMENDED - 1;
htotal = mode->hsync_len + mode->left_margin + mode->xres +
mode->right_margin;
@@ -750,14 +752,18 @@ static int tc_set_video_mode(struct tc_data *tc, struct fb_videomode *mode)
mode->upper_margin, mode->lower_margin, mode->vsync_len);
dev_dbg(tc->dev, "total: %dx%d\n", htotal, vtotal);
-
- /* LCD Ctl Frame Size */
- tc_write(VPCTRL0, (0x40 << 20) /* VSDELAY */ |
+ /*
+ * Datasheet is not clear of vsdelay in case of DPI
+ * assume we do not need any delay when DPI is a source of
+ * sync signals
+ */
+ tc_write(VPCTRL0, (0 << 20) /* VSDELAY */ |
OPXLFMT_RGB888 | FRMSYNC_DISABLED | MSF_DISABLED);
- tc_write(HTIM01, (mode->left_margin << 16) | /* H back porch */
- (mode->hsync_len << 0)); /* Hsync */
- tc_write(HTIM02, (mode->right_margin << 16) | /* H front porch */
- (mode->xres << 0)); /* width */
+ /* LCD Ctl Frame Size */
+ tc_write(HTIM01, (EVEN_HI(mode->left_margin) << 16) | /* H back porch */
+ (EVEN_HI(mode->hsync_len) << 0)); /* Hsync */
+ tc_write(HTIM02, (EVEN_HI(mode->right_margin) << 16) | /* H front porch */
+ (EVEN_HI(mode->xres) << 0)); /* width */
tc_write(VTIM01, (mode->upper_margin << 16) | /* V back porch */
(mode->vsync_len << 0)); /* Vsync */
tc_write(VTIM02, (mode->lower_margin << 16) | /* V front porch */
@@ -776,7 +782,7 @@ static int tc_set_video_mode(struct tc_data *tc, struct fb_videomode *mode)
/* DP Main Stream Attributes */
vid_sync_dly = mode->hsync_len + mode->left_margin + mode->xres;
tc_write(DP0_VIDSYNCDELAY,
- (0x003e << 16) | /* thresh_dly */
+ (max_tu_symbol << 16) | /* thresh_dly */
(vid_sync_dly << 0));
tc_write(DP0_TOTALVAL, (vtotal << 16) | (htotal));
@@ -798,7 +804,6 @@ static int tc_set_video_mode(struct tc_data *tc, struct fb_videomode *mode)
* (output active video bandwidth in bytes))
* Must be less than tu_size.
*/
- max_tu_symbol = TU_SIZE_RECOMMENDED - 1;
tc_write(DP0_MISC, (max_tu_symbol << 23) | (TU_SIZE_RECOMMENDED << 16) | BPC_8);
return 0;

[Andrey Gusakov]

Minimum pixel clock period is 6.5 nS for DPI. Remove modes with
lower pixel clock period. Also sort modes in decreasing order
because currently first resolution in list is picked.
---
drivers/video/tc358767.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 83 insertions(+)

diff --git a/drivers/video/tc358767.c b/drivers/video/tc358767.c
index 1541b18c9..61695b61c 100644
--- a/drivers/video/tc358767.c
+++ b/drivers/video/tc358767.c
@@ -1210,6 +1210,82 @@ err:
return ret;
}

+static int tc_filter_videomodes(struct tc_data *tc, struct display_timings *timings)
+{
+ int i;
+ int num_modes = 0;
+ struct fb_videomode *mode, *valid_modes;
+
+ valid_modes = xzalloc(timings->num_modes * sizeof(struct fb_videomode));
+
+ /* first filter modes with too high pclock */
+ for (i = 0; i < timings->num_modes; i++) {
+ mode = &timings->modes[i];
+
+ /* minimum Pixel Clock Period for DPI is 6.5 nS = 6500 pS */
+ if (mode->pixclock < 6500) {
+ dev_dbg(tc->dev, "%dx%d@%d (%d KHz, flags 0x%08x, sync 0x%08x) skipped\n",
+ mode->xres, mode->yres, mode->refresh,
+ (int)PICOS2KHZ(mode->pixclock), mode->display_flags,
+ mode->sync);
+ /* remove from list */
+ mode->xres = mode->yres = 0;
+ }
+ }
+
+ /* then sort from hi to low */
+ do {
+ int index = -1;
+
+ /* find higest resolution */
+ for (i = 0; i < timings->num_modes; i++) {
+ mode = &timings->modes[i];
+ if (!(mode->xres && mode->yres))
+ continue;
+ if (index == -1) {
+ index = i;
+ } else {
+ /* compare square */
+ if (timings->modes[index].xres * timings->modes[index].yres <
+ mode->xres * mode->yres)
+ index = i;
+ }
+ }
+
+ /* nothing left */
+ if (index == -1)
+ break;
+
+ /* copy to output list */
+ mode = &timings->modes[index];
+ memcpy(&valid_modes[num_modes], mode, sizeof(struct fb_videomode));
+ mode->xres = mode->yres = 0;
+ num_modes++;
+ } while (1);
+
+ free(timings->modes);
+ timings->modes = NULL;
+
+ if (!num_modes) {
+ free(valid_modes);
+ return -EINVAL;
+ }
+
+ timings->num_modes = num_modes;
+ timings->modes = valid_modes;
+
+ dev_dbg(tc->dev, "Valid modes (%d):\n", num_modes);
+ for (i = 0; i < timings->num_modes; i++) {
+ mode = &timings->modes[i];
+ dev_dbg(tc->dev, "%dx%d@%d (%d KHz, flags 0x%08x, sync 0x%08x)\n",
+ mode->xres, mode->yres, mode->refresh,
+ (int)PICOS2KHZ(mode->pixclock), mode->display_flags,
+ mode->sync);
+ }
+
+ return 0;
+}
+
static int tc_get_videomodes(struct tc_data *tc, struct display_timings *timings)
{
int ret;
@@ -1229,6 +1305,13 @@ static int tc_get_videomodes(struct tc_data *tc, struct display_timings *timings
return ret;
}

+ /* filter out unsupported due to high pixelxlock */
+ ret = tc_filter_videomodes(tc, timings);
+ if (ret < 0) {
+ dev_err(tc->dev, "No supported modes found\n");
+ return ret;
+ }
+
/* hsync, vsync active low */
timings->modes->sync &= ~(FB_SYNC_HOR_HIGH_ACT |
FB_SYNC_VERT_HIGH_ACT);

[Andrey Gusakov]

Do not fix modes. Instead set any sync polarity passed through
VPL_PREPARE and fb_videomode.
---
drivers/video/tc358767.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/video/tc358767.c b/drivers/video/tc358767.c
index 61695b61c..32509e34c 100644
--- a/drivers/video/tc358767.c
+++ b/drivers/video/tc358767.c
@@ -736,6 +736,7 @@ err_dpcd_inval:
static int tc_set_video_mode(struct tc_data *tc, struct fb_videomode *mode)
{
int ret;
+ u32 reg;
int htotal;
int vtotal;
int vid_sync_dly;
@@ -796,8 +797,12 @@ static int tc_set_video_mode(struct tc_data *tc, struct fb_videomode *mode)

tc_write(DP0_SYNCVAL, (mode->vsync_len << 16) | (mode->hsync_len << 0));

- tc_write(DPIPXLFMT, VS_POL_ACTIVE_LOW | HS_POL_ACTIVE_LOW |
- DE_POL_ACTIVE_HIGH | SUB_CFG_TYPE_CONFIG1 | DPI_BPP_RGB888);
+ reg = DE_POL_ACTIVE_HIGH | SUB_CFG_TYPE_CONFIG1 | DPI_BPP_RGB888;
+ if (!(mode->sync & FB_SYNC_VERT_HIGH_ACT))
+ reg |= VS_POL_ACTIVE_LOW;
+ if (!(mode->sync & FB_SYNC_HOR_HIGH_ACT))
+ reg |= HS_POL_ACTIVE_LOW;
+ tc_write(DPIPXLFMT, reg);

/*
* Recommended maximum number of symbols transferred in a transfer unit:
@@ -1312,10 +1317,6 @@ static int tc_get_videomodes(struct tc_data *tc, struct display_timings *timings
return ret;
}

- /* hsync, vsync active low */
- timings->modes->sync &= ~(FB_SYNC_HOR_HIGH_ACT |
- FB_SYNC_VERT_HIGH_ACT);
-
return ret;
}

[Lucas Stach]

Hi Andrey,

Post by Andrey Gusakov
Hi Barebox,
This set of patches fixes several issues that was found during testing
tc358767 with several desktop DP displays.
video: tc358767: do not fail if sink supports more than 2 lanes
video: tc358767: support newer DPCD revisions and higher data rates
video: tc358767: fix EDID read for DP displays
video: tc358767: fix DP0_MISC register set
video: tc358767: fix timing calculation
video: tc358767: fix AUXDATAn registers access
video: tc358767: filter out modes with too high pixelclock
video: tc358767: accept any hsync and vsync polatiry
drivers/video/tc358767.c | 218 ++++++++++++++++++++++++++++++++++++++---------
1 file changed, 179 insertions(+), 39 deletions(-)

All those patches are missing a signed-off-by line, so I can not apply
them. Please fix this for the next revision.

Regards,
Lucas