drivers/analogix/anx7625: Fix edid_read()

The current implementations of edid_read() and segments_edid_read() have
a few problems:

1. The type of variable `c` is incorrect, not matching the return type
   of sp_tx_aux_rd(). In addition, the meaning of `c` is unknown.
2. It is pointless to do `cnt++` when sp_tx_aux_rd() fails.
3. These two functions ignore the return value of
   anx7625_reg_block_read().
4. In segments_edid_read(), anx7625_reg_write() might return a positive
   value on failure.

Fix all of the 4 issues, and modify the code to be closer to kernel
5.10's implementation (drivers/gpu/drm/bridge/analogix/anx7625.c). Note
that, however, unlike in kernel, anx7625_reg_block_read() here doesn't
return the number of bytes. On success, 0 is returned instead.

In addition, following coreboot's convention, always return negative
error codes. In particular, change the return value to -1 for
edid_read() and segments_edid_read() on failure.

BUG=b:207055969
TEST=emerge-asurada coreboot
BRANCH=none

Change-Id: Ife9d7d97df2926b4581ba519a152c9efed8cd969
Signed-off-by: Yu-Ping Wu <yupingso@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/59540
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
This commit is contained in:
Yu-Ping Wu 2021-11-25 14:31:21 +08:00 committed by Hung-Te Lin
parent 74033df2bd
commit 0c9b1deb63
1 changed files with 76 additions and 68 deletions

View File

@ -54,9 +54,11 @@ static int i2c_access_workaround(uint8_t bus, uint8_t saddr)
} }
ret = i2c_writeb(bus, saddr, offset, 0x00); ret = i2c_writeb(bus, saddr, offset, 0x00);
if (ret < 0) if (ret < 0) {
ANXERROR("Failed to access %#x:%#x\n", saddr, offset); ANXERROR("Failed to access %#x:%#x\n", saddr, offset);
return ret; return ret;
}
return 0;
} }
static int anx7625_reg_read(uint8_t bus, uint8_t saddr, uint8_t offset, static int anx7625_reg_read(uint8_t bus, uint8_t saddr, uint8_t offset,
@ -70,7 +72,7 @@ static int anx7625_reg_read(uint8_t bus, uint8_t saddr, uint8_t offset,
ANXERROR("Failed to read i2c reg=%#x:%#x\n", saddr, offset); ANXERROR("Failed to read i2c reg=%#x:%#x\n", saddr, offset);
return ret; return ret;
} }
return *val; return 0;
} }
static int anx7625_reg_block_read(uint8_t bus, uint8_t saddr, uint8_t reg_addr, static int anx7625_reg_block_read(uint8_t bus, uint8_t saddr, uint8_t reg_addr,
@ -80,10 +82,12 @@ static int anx7625_reg_block_read(uint8_t bus, uint8_t saddr, uint8_t reg_addr,
i2c_access_workaround(bus, saddr); i2c_access_workaround(bus, saddr);
ret = i2c_read_bytes(bus, saddr, reg_addr, buf, len); ret = i2c_read_bytes(bus, saddr, reg_addr, buf, len);
if (ret < 0) if (ret < 0) {
ANXERROR("Failed to read i2c block=%#x:%#x[len=%#x]\n", saddr, ANXERROR("Failed to read i2c block=%#x:%#x[len=%#x]\n", saddr,
reg_addr, len); reg_addr, len);
return ret; return ret;
}
return 0;
} }
static int anx7625_reg_write(uint8_t bus, uint8_t saddr, uint8_t reg_addr, static int anx7625_reg_write(uint8_t bus, uint8_t saddr, uint8_t reg_addr,
@ -93,10 +97,11 @@ static int anx7625_reg_write(uint8_t bus, uint8_t saddr, uint8_t reg_addr,
i2c_access_workaround(bus, saddr); i2c_access_workaround(bus, saddr);
ret = i2c_writeb(bus, saddr, reg_addr, reg_val); ret = i2c_writeb(bus, saddr, reg_addr, reg_val);
if (ret < 0) if (ret < 0) {
ANXERROR("Failed to write i2c id=%#x:%#x\n", saddr, reg_addr); ANXERROR("Failed to write i2c id=%#x:%#x\n", saddr, reg_addr);
return ret;
return ret; }
return 0;
} }
static int anx7625_write_or(uint8_t bus, uint8_t saddr, uint8_t offset, static int anx7625_write_or(uint8_t bus, uint8_t saddr, uint8_t offset,
@ -128,30 +133,31 @@ static int anx7625_write_and(uint8_t bus, uint8_t saddr, uint8_t offset,
static int wait_aux_op_finish(uint8_t bus) static int wait_aux_op_finish(uint8_t bus)
{ {
uint8_t val; uint8_t val;
int ret = -1;
int loop; int loop;
int success = 0;
int ret;
for (loop = 0; loop < 150; loop++) { for (loop = 0; loop < 150; loop++) {
mdelay(2); mdelay(2);
anx7625_reg_read(bus, RX_P0_ADDR, AP_AUX_CTRL_STATUS, &val); anx7625_reg_read(bus, RX_P0_ADDR, AP_AUX_CTRL_STATUS, &val);
if (!(val & AP_AUX_CTRL_OP_EN)) { if (!(val & AP_AUX_CTRL_OP_EN)) {
ret = 0; success = 1;
break; break;
} }
} }
if (ret != 0) { if (!success) {
ANXERROR("Timed out waiting aux operation.\n"); ANXERROR("Timed out waiting aux operation.\n");
return ret; return -1;
} }
ret = anx7625_reg_read(bus, RX_P0_ADDR, AP_AUX_CTRL_STATUS, &val); ret = anx7625_reg_read(bus, RX_P0_ADDR, AP_AUX_CTRL_STATUS, &val);
if (ret < 0 || val & 0x0F) { if (ret < 0 || val & 0x0F) {
ANXDEBUG("aux status %02x\n", val); ANXDEBUG("aux status %02x\n", val);
ret = -1; return -1;
} }
return ret; return 0;
} }
static unsigned long gcd(unsigned long a, unsigned long b) static unsigned long gcd(unsigned long a, unsigned long b)
@ -210,7 +216,7 @@ static int anx7625_calculate_m_n(u32 pixelclock,
ANXERROR("pixelclock %u higher than %lu, " ANXERROR("pixelclock %u higher than %lu, "
"output may be unstable\n", "output may be unstable\n",
pixelclock, PLL_OUT_FREQ_ABS_MAX / POST_DIVIDER_MIN); pixelclock, PLL_OUT_FREQ_ABS_MAX / POST_DIVIDER_MIN);
return 1; return -1;
} }
if (pixelclock < PLL_OUT_FREQ_ABS_MIN / POST_DIVIDER_MAX) { if (pixelclock < PLL_OUT_FREQ_ABS_MIN / POST_DIVIDER_MAX) {
@ -218,7 +224,7 @@ static int anx7625_calculate_m_n(u32 pixelclock,
ANXERROR("pixelclock %u lower than %lu, " ANXERROR("pixelclock %u lower than %lu, "
"output may be unstable\n", "output may be unstable\n",
pixelclock, PLL_OUT_FREQ_ABS_MIN / POST_DIVIDER_MAX); pixelclock, PLL_OUT_FREQ_ABS_MIN / POST_DIVIDER_MAX);
return 1; return -1;
} }
post_divider = 1; post_divider = 1;
@ -237,7 +243,7 @@ static int anx7625_calculate_m_n(u32 pixelclock,
if (post_divider > POST_DIVIDER_MAX) { if (post_divider > POST_DIVIDER_MAX) {
ANXERROR("cannot find property post_divider(%d)\n", ANXERROR("cannot find property post_divider(%d)\n",
post_divider); post_divider);
return 1; return -1;
} }
} }
@ -256,7 +262,7 @@ static int anx7625_calculate_m_n(u32 pixelclock,
if (pixelclock * post_divider > PLL_OUT_FREQ_ABS_MAX) { if (pixelclock * post_divider > PLL_OUT_FREQ_ABS_MAX) {
ANXINFO("act clock(%u) large than maximum(%lu)\n", ANXINFO("act clock(%u) large than maximum(%lu)\n",
pixelclock * post_divider, PLL_OUT_FREQ_ABS_MAX); pixelclock * post_divider, PLL_OUT_FREQ_ABS_MAX);
return 1; return -1;
} }
*m = pixelclock; *m = pixelclock;
@ -292,10 +298,12 @@ static int anx7625_odfc_config(uint8_t bus, uint8_t post_divider)
ret |= anx7625_write_or(bus, RX_P1_ADDR, MIPI_DIGITAL_PLL_7, ret |= anx7625_write_or(bus, RX_P1_ADDR, MIPI_DIGITAL_PLL_7,
MIPI_PLL_RESET_N); MIPI_PLL_RESET_N);
if (ret < 0) if (ret < 0) {
ANXERROR("IO error.\n"); ANXERROR("IO error.\n");
return ret;
}
return ret; return 0;
} }
static int anx7625_dsi_video_config(uint8_t bus, struct display_timing *dt) static int anx7625_dsi_video_config(uint8_t bus, struct display_timing *dt)
@ -305,10 +313,8 @@ static int anx7625_dsi_video_config(uint8_t bus, struct display_timing *dt)
int ret; int ret;
uint8_t post_divider = 0; uint8_t post_divider = 0;
ret = anx7625_calculate_m_n(dt->pixelclock * 1000, &m, &n, if (anx7625_calculate_m_n(dt->pixelclock * 1000, &m, &n,
&post_divider); &post_divider) < 0) {
if (ret != 0) {
ANXERROR("cannot get property m n value.\n"); ANXERROR("cannot get property m n value.\n");
return -1; return -1;
} }
@ -385,10 +391,12 @@ static int anx7625_dsi_video_config(uint8_t bus, struct display_timing *dt)
ret |= anx7625_odfc_config(bus, post_divider - 1); ret |= anx7625_odfc_config(bus, post_divider - 1);
if (ret < 0) if (ret < 0) {
ANXERROR("mipi dsi setup IO error.\n"); ANXERROR("mipi dsi setup IO error.\n");
return ret;
}
return ret; return 0;
} }
static int anx7625_swap_dsi_lane3(uint8_t bus) static int anx7625_swap_dsi_lane3(uint8_t bus)
@ -400,7 +408,7 @@ static int anx7625_swap_dsi_lane3(uint8_t bus)
ret = anx7625_reg_read(bus, RX_P1_ADDR, MIPI_SWAP, &val); ret = anx7625_reg_read(bus, RX_P1_ADDR, MIPI_SWAP, &val);
if (ret < 0) { if (ret < 0) {
ANXERROR("IO error: access MIPI_SWAP.\n"); ANXERROR("IO error: access MIPI_SWAP.\n");
return -1; return ret;
} }
val |= (1 << MIPI_SWAP_CH3); val |= (1 << MIPI_SWAP_CH3);
@ -461,10 +469,12 @@ static int anx7625_api_dsi_config(uint8_t bus, struct display_timing *dt)
ret |= anx7625_reg_write(bus, RX_P1_ADDR, MIPI_LANE_CTRL_10, 0x00); ret |= anx7625_reg_write(bus, RX_P1_ADDR, MIPI_LANE_CTRL_10, 0x00);
ret |= anx7625_reg_write(bus, RX_P1_ADDR, MIPI_LANE_CTRL_10, 0x80); ret |= anx7625_reg_write(bus, RX_P1_ADDR, MIPI_LANE_CTRL_10, 0x80);
if (ret < 0) if (ret < 0) {
ANXERROR("IO error: mipi dsi enable init failed.\n"); ANXERROR("IO error: mipi dsi enable init failed.\n");
return ret;
}
return ret; return 0;
} }
static int anx7625_dsi_config(uint8_t bus, struct display_timing *dt) static int anx7625_dsi_config(uint8_t bus, struct display_timing *dt)
@ -487,12 +497,13 @@ static int anx7625_dsi_config(uint8_t bus, struct display_timing *dt)
/* clear mute flag */ /* clear mute flag */
ret |= anx7625_write_and(bus, RX_P0_ADDR, AP_AV_STATUS, ~AP_MIPI_MUTE); ret |= anx7625_write_and(bus, RX_P0_ADDR, AP_AV_STATUS, ~AP_MIPI_MUTE);
if (ret < 0) if (ret < 0) {
ANXERROR("IO error: enable mipi rx failed.\n"); ANXERROR("IO error: enable mipi rx failed.\n");
else return ret;
ANXINFO("success to config DSI\n"); }
return ret; ANXINFO("success to config DSI\n");
return 0;
} }
static int sp_tx_rst_aux(uint8_t bus) static int sp_tx_rst_aux(uint8_t bus)
@ -549,34 +560,32 @@ static int sp_tx_get_edid_block(uint8_t bus)
static int edid_read(uint8_t bus, uint8_t offset, uint8_t *pblock_buf) static int edid_read(uint8_t bus, uint8_t offset, uint8_t *pblock_buf)
{ {
uint8_t c, cnt = 0; int ret, cnt;
c = 0;
for (cnt = 0; cnt < 3; cnt++) { for (cnt = 0; cnt < 3; cnt++) {
sp_tx_aux_wr(bus, offset); sp_tx_aux_wr(bus, offset);
/* set I2C read com 0x01 mot = 0 and read 16 bytes */ /* set I2C read com 0x01 mot = 0 and read 16 bytes */
c = sp_tx_aux_rd(bus, 0xf1); ret = sp_tx_aux_rd(bus, 0xf1);
if (c == 1) { if (ret < 0) {
sp_tx_rst_aux(bus); sp_tx_rst_aux(bus);
ANXERROR("edid read failed, reset!\n"); ANXERROR("edid read failed, reset!\n");
cnt++;
} else { } else {
anx7625_reg_block_read(bus, RX_P0_ADDR, if (anx7625_reg_block_read(bus, RX_P0_ADDR,
AP_AUX_BUFF_START, AP_AUX_BUFF_START,
MAX_DPCD_BUFFER_SIZE, pblock_buf); MAX_DPCD_BUFFER_SIZE,
return 0; pblock_buf) >= 0)
return 0;
} }
} }
return 1; return -1;
} }
static int segments_edid_read(uint8_t bus, uint8_t segment, uint8_t *buf, static int segments_edid_read(uint8_t bus, uint8_t segment, uint8_t *buf,
uint8_t offset) uint8_t offset)
{ {
uint8_t c, cnt = 0; int ret, cnt;
int ret;
/* write address only */ /* write address only */
ret = anx7625_reg_write(bus, RX_P0_ADDR, AP_AUX_ADDR_7_0, 0x30); ret = anx7625_reg_write(bus, RX_P0_ADDR, AP_AUX_ADDR_7_0, 0x30);
@ -598,21 +607,21 @@ static int segments_edid_read(uint8_t bus, uint8_t segment, uint8_t *buf,
for (cnt = 0; cnt < 3; cnt++) { for (cnt = 0; cnt < 3; cnt++) {
sp_tx_aux_wr(bus, offset); sp_tx_aux_wr(bus, offset);
/* set I2C read com 0x01 mot = 0 and read 16 bytes */ /* set I2C read com 0x01 mot = 0 and read 16 bytes */
c = sp_tx_aux_rd(bus, 0xf1); ret = sp_tx_aux_rd(bus, 0xf1);
if (c == 1) { if (ret < 0) {
ret = sp_tx_rst_aux(bus); sp_tx_rst_aux(bus);
ANXERROR("segment read failed, reset!\n"); ANXERROR("segment read failed, reset!\n");
cnt++;
} else { } else {
ret = anx7625_reg_block_read(bus, RX_P0_ADDR, if (anx7625_reg_block_read(bus, RX_P0_ADDR,
AP_AUX_BUFF_START, AP_AUX_BUFF_START,
MAX_DPCD_BUFFER_SIZE, buf); MAX_DPCD_BUFFER_SIZE,
return ret; buf) >= 0)
return 0;
} }
} }
return ret; return -1;
} }
static int sp_tx_edid_read(uint8_t bus, uint8_t *pedid_blocks_buf, static int sp_tx_edid_read(uint8_t bus, uint8_t *pedid_blocks_buf,
@ -621,9 +630,7 @@ static int sp_tx_edid_read(uint8_t bus, uint8_t *pedid_blocks_buf,
uint8_t offset, edid_pos; uint8_t offset, edid_pos;
int count, blocks_num; int count, blocks_num;
uint8_t pblock_buf[MAX_DPCD_BUFFER_SIZE]; uint8_t pblock_buf[MAX_DPCD_BUFFER_SIZE];
uint8_t i; int i, ret, g_edid_break = 0;
uint8_t g_edid_break = 0;
int ret;
/* address initial */ /* address initial */
ret = anx7625_reg_write(bus, RX_P0_ADDR, AP_AUX_ADDR_7_0, 0x50); ret = anx7625_reg_write(bus, RX_P0_ADDR, AP_AUX_ADDR_7_0, 0x50);
@ -637,7 +644,7 @@ static int sp_tx_edid_read(uint8_t bus, uint8_t *pedid_blocks_buf,
blocks_num = sp_tx_get_edid_block(bus); blocks_num = sp_tx_get_edid_block(bus);
if (blocks_num < 0) if (blocks_num < 0)
return blocks_num; return -1;
count = 0; count = 0;
@ -647,10 +654,10 @@ static int sp_tx_edid_read(uint8_t bus, uint8_t *pedid_blocks_buf,
case 1: case 1:
for (i = 0; i < 8; i++) { for (i = 0; i < 8; i++) {
offset = (i + count * 8) * MAX_DPCD_BUFFER_SIZE; offset = (i + count * 8) * MAX_DPCD_BUFFER_SIZE;
g_edid_break = edid_read(bus, offset, g_edid_break = !!edid_read(bus, offset,
pblock_buf); pblock_buf);
if (g_edid_break == 1) if (g_edid_break)
break; break;
if (offset <= size - MAX_DPCD_BUFFER_SIZE) if (offset <= size - MAX_DPCD_BUFFER_SIZE)
@ -668,11 +675,11 @@ static int sp_tx_edid_read(uint8_t bus, uint8_t *pedid_blocks_buf,
edid_pos = (i + count * 8) * edid_pos = (i + count * 8) *
MAX_DPCD_BUFFER_SIZE; MAX_DPCD_BUFFER_SIZE;
if (g_edid_break == 1) if (g_edid_break)
break; break;
segments_edid_read(bus, count / 2, segments_edid_read(bus, count / 2,
pblock_buf, offset); pblock_buf, offset);
if (edid_pos <= size - MAX_DPCD_BUFFER_SIZE) if (edid_pos <= size - MAX_DPCD_BUFFER_SIZE)
memcpy(&pedid_blocks_buf[edid_pos], memcpy(&pedid_blocks_buf[edid_pos],
pblock_buf, pblock_buf,
@ -834,12 +841,13 @@ int anx7625_dp_start(uint8_t bus, const struct edid *edid)
anx7625_parse_edid(edid, &dt); anx7625_parse_edid(edid, &dt);
ret = anx7625_dsi_config(bus, &dt); ret = anx7625_dsi_config(bus, &dt);
if (ret < 0) if (ret < 0) {
ANXERROR("MIPI phy setup error.\n"); ANXERROR("MIPI phy setup error.\n");
else return ret;
ANXINFO("MIPI phy setup OK.\n"); }
return ret; ANXINFO("MIPI phy setup OK.\n");
return 0;
} }
int anx7625_dp_get_edid(uint8_t bus, struct edid *out) int anx7625_dp_get_edid(uint8_t bus, struct edid *out)
@ -869,7 +877,7 @@ int anx7625_init(uint8_t bus)
int retry_power_on = 3; int retry_power_on = 3;
while (--retry_power_on) { while (--retry_power_on) {
if (anx7625_power_on_init(bus) == 0) if (anx7625_power_on_init(bus) >= 0)
break; break;
} }
if (!retry_power_on) { if (!retry_power_on) {