| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <57A44A87.3000806@rock-chips.com>
Date: Fri, 5 Aug 2016 16:12:55 +0800
From: Yakir Yang <ykk@...k-chips.com>
To: Tomeu Vizoso <tomeu.vizoso@...labora.com>,
linux-kernel@...r.kernel.org
Cc: Javier Martinez Canillas <javier@....samsung.com>,
Mika Kahola <mika.kahola@...el.com>,
Daniel Vetter <daniel.vetter@...el.com>,
David Airlie <airlied@...ux.ie>,
dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH] drm/bridge: analogix_dp: Remove duplicated code
Tomeu,
Nice job ! Have a few nits bellow. ;)
On 08/04/2016 02:23 PM, Tomeu Vizoso wrote:
> Remove code for reading the EDID and DPCD fields and use the helpers
> instead.
>
> Besides the obvious code reduction, other helpers are being added to the
> core that could be used in this driver and will be good to be able to
> use them instead of duplicating them.
>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@...labora.com>
> Cc: Javier Martinez Canillas <javier@....samsung.com>
> Cc: Mika Kahola <mika.kahola@...el.com>
> Cc: Yakir Yang <ykk@...k-chips.com>
> Cc: Daniel Vetter <daniel.vetter@...el.com>
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 390 +++++++++++----------
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 39 +--
> drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 324 -----------------
> 3 files changed, 201 insertions(+), 552 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 32715daf73cb..c81cb37e56b6 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -31,6 +31,7 @@
> #include <drm/bridge/analogix_dp.h>
>
> #include "analogix_dp_core.h"
> +#include "analogix_dp_reg.h"
>
> #define to_dp(nm) container_of(nm, struct analogix_dp_device, nm)
>
> @@ -97,150 +98,21 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device *dp)
> return 0;
> }
>
> -static unsigned char analogix_dp_calc_edid_check_sum(unsigned char *edid_data)
> -{
> - int i;
> - unsigned char sum = 0;
> -
> - for (i = 0; i < EDID_BLOCK_LENGTH; i++)
> - sum = sum + edid_data[i];
> -
> - return sum;
> -}
> -
> -static int analogix_dp_read_edid(struct analogix_dp_device *dp)
> -{
> - unsigned char *edid = dp->edid;
> - unsigned int extend_block = 0;
> - unsigned char sum;
> - unsigned char test_vector;
> - int retval;
> -
> - /*
> - * EDID device address is 0x50.
> - * However, if necessary, you must have set upper address
> - * into E-EDID in I2C device, 0x30.
> - */
> -
> - /* Read Extension Flag, Number of 128-byte EDID extension blocks */
> - retval = analogix_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
> - EDID_EXTENSION_FLAG,
> - &extend_block);
> - if (retval)
> - return retval;
> -
> - if (extend_block > 0) {
> - dev_dbg(dp->dev, "EDID data includes a single extension!\n");
> -
> - /* Read EDID data */
> - retval = analogix_dp_read_bytes_from_i2c(dp,
> - I2C_EDID_DEVICE_ADDR,
> - EDID_HEADER_PATTERN,
> - EDID_BLOCK_LENGTH,
> - &edid[EDID_HEADER_PATTERN]);
> - if (retval != 0) {
> - dev_err(dp->dev, "EDID Read failed!\n");
> - return -EIO;
> - }
> - sum = analogix_dp_calc_edid_check_sum(edid);
> - if (sum != 0) {
> - dev_err(dp->dev, "EDID bad checksum!\n");
> - return -EIO;
> - }
> -
> - /* Read additional EDID data */
> - retval = analogix_dp_read_bytes_from_i2c(dp,
> - I2C_EDID_DEVICE_ADDR,
> - EDID_BLOCK_LENGTH,
> - EDID_BLOCK_LENGTH,
> - &edid[EDID_BLOCK_LENGTH]);
> - if (retval != 0) {
> - dev_err(dp->dev, "EDID Read failed!\n");
> - return -EIO;
> - }
> - sum = analogix_dp_calc_edid_check_sum(&edid[EDID_BLOCK_LENGTH]);
> - if (sum != 0) {
> - dev_err(dp->dev, "EDID bad checksum!\n");
> - return -EIO;
> - }
> -
> - analogix_dp_read_byte_from_dpcd(dp, DP_TEST_REQUEST,
> - &test_vector);
> - if (test_vector & DP_TEST_LINK_EDID_READ) {
> - analogix_dp_write_byte_to_dpcd(dp,
> - DP_TEST_EDID_CHECKSUM,
> - edid[EDID_BLOCK_LENGTH + EDID_CHECKSUM]);
> - analogix_dp_write_byte_to_dpcd(dp,
> - DP_TEST_RESPONSE,
> - DP_TEST_EDID_CHECKSUM_WRITE);
> - }
> - } else {
> - dev_info(dp->dev, "EDID data does not include any extensions.\n");
> -
> - /* Read EDID data */
> - retval = analogix_dp_read_bytes_from_i2c(dp,
> - I2C_EDID_DEVICE_ADDR, EDID_HEADER_PATTERN,
> - EDID_BLOCK_LENGTH, &edid[EDID_HEADER_PATTERN]);
> - if (retval != 0) {
> - dev_err(dp->dev, "EDID Read failed!\n");
> - return -EIO;
> - }
> - sum = analogix_dp_calc_edid_check_sum(edid);
> - if (sum != 0) {
> - dev_err(dp->dev, "EDID bad checksum!\n");
> - return -EIO;
> - }
> -
> - analogix_dp_read_byte_from_dpcd(dp, DP_TEST_REQUEST,
> - &test_vector);
> - if (test_vector & DP_TEST_LINK_EDID_READ) {
> - analogix_dp_write_byte_to_dpcd(dp,
> - DP_TEST_EDID_CHECKSUM, edid[EDID_CHECKSUM]);
> - analogix_dp_write_byte_to_dpcd(dp,
> - DP_TEST_RESPONSE, DP_TEST_EDID_CHECKSUM_WRITE);
> - }
> - }
> -
> - dev_dbg(dp->dev, "EDID Read success!\n");
> - return 0;
> -}
> -
> -static int analogix_dp_handle_edid(struct analogix_dp_device *dp)
> -{
> - u8 buf[12];
> - int i;
> - int retval;
> -
> - /* Read DPCD DP_DPCD_REV~RECEIVE_PORT1_CAP_1 */
> - retval = analogix_dp_read_bytes_from_dpcd(dp, DP_DPCD_REV, 12, buf);
> - if (retval)
> - return retval;
> -
> - /* Read EDID */
> - for (i = 0; i < 3; i++) {
> - retval = analogix_dp_read_edid(dp);
> - if (!retval)
> - break;
> - }
> -
> - return retval;
> -}
> -
> static void
> analogix_dp_enable_rx_to_enhanced_mode(struct analogix_dp_device *dp,
> bool enable)
> {
> u8 data;
>
> - analogix_dp_read_byte_from_dpcd(dp, DP_LANE_COUNT_SET, &data);
> + drm_dp_dpcd_readb(&dp->aux, DP_LANE_COUNT_SET, &data);
>
> if (enable)
> - analogix_dp_write_byte_to_dpcd(dp, DP_LANE_COUNT_SET,
> - DP_LANE_COUNT_ENHANCED_FRAME_EN |
> - DPCD_LANE_COUNT_SET(data));
> + drm_dp_dpcd_writeb(&dp->aux, DP_LANE_COUNT_SET,
> + DP_LANE_COUNT_ENHANCED_FRAME_EN |
> + DPCD_LANE_COUNT_SET(data));
> else
> - analogix_dp_write_byte_to_dpcd(dp, DP_LANE_COUNT_SET,
> - DPCD_LANE_COUNT_SET(data));
> + drm_dp_dpcd_writeb(&dp->aux, DP_LANE_COUNT_SET,
> + DPCD_LANE_COUNT_SET(data));
> }
>
> static int analogix_dp_is_enhanced_mode_available(struct analogix_dp_device *dp)
> @@ -248,7 +120,7 @@ static int analogix_dp_is_enhanced_mode_available(struct analogix_dp_device *dp)
> u8 data;
> int retval;
>
> - analogix_dp_read_byte_from_dpcd(dp, DP_MAX_LANE_COUNT, &data);
> + drm_dp_dpcd_readb(&dp->aux, DP_MAX_LANE_COUNT, &data);
> retval = DPCD_ENHANCED_FRAME_CAP(data);
>
> return retval;
> @@ -267,8 +139,8 @@ static void analogix_dp_training_pattern_dis(struct analogix_dp_device *dp)
> {
> analogix_dp_set_training_pattern(dp, DP_NONE);
>
> - analogix_dp_write_byte_to_dpcd(dp, DP_TRAINING_PATTERN_SET,
> - DP_TRAINING_PATTERN_DISABLE);
> + drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
> + DP_TRAINING_PATTERN_DISABLE);
> }
>
> static void
> @@ -313,8 +185,8 @@ static int analogix_dp_link_start(struct analogix_dp_device *dp)
> /* Setup RX configuration */
> buf[0] = dp->link_train.link_rate;
> buf[1] = dp->link_train.lane_count;
> - retval = analogix_dp_write_bytes_to_dpcd(dp, DP_LINK_BW_SET, 2, buf);
> - if (retval)
> + retval = drm_dp_dpcd_write(&dp->aux, DP_LINK_BW_SET, buf, 2);
> + if (retval < 0)
> return retval;
>
> /* Set TX pre-emphasis to minimum */
> @@ -338,20 +210,22 @@ static int analogix_dp_link_start(struct analogix_dp_device *dp)
> analogix_dp_set_training_pattern(dp, TRAINING_PTN1);
>
> /* Set RX training pattern */
> - retval = analogix_dp_write_byte_to_dpcd(dp,
> - DP_TRAINING_PATTERN_SET,
> - DP_LINK_SCRAMBLING_DISABLE | DP_TRAINING_PATTERN_1);
> - if (retval)
> + retval = drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
> + DP_LINK_SCRAMBLING_DISABLE |
> + DP_TRAINING_PATTERN_1);
'DP_TRAINING_PATTERN_1' need align with 'DP_LINK_SCRAMBLING_DISABLE'
> + if (retval < 0)
> return retval;
>
> for (lane = 0; lane < lane_count; lane++)
> buf[lane] = DP_TRAIN_PRE_EMPH_LEVEL_0 |
> DP_TRAIN_VOLTAGE_SWING_LEVEL_0;
>
> - retval = analogix_dp_write_bytes_to_dpcd(dp, DP_TRAINING_LANE0_SET,
> - lane_count, buf);
> + retval = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET, buf,
> + lane_count);
> + if (retval < 0)
> + return retval;
>
> - return retval;
> + return 0;
> }
>
> static unsigned char analogix_dp_get_lane_status(u8 link_status[2], int lane)
> @@ -503,25 +377,23 @@ static int analogix_dp_process_clock_recovery(struct analogix_dp_device *dp)
>
> lane_count = dp->link_train.lane_count;
>
> - retval = analogix_dp_read_bytes_from_dpcd(dp,
> - DP_LANE0_1_STATUS, 2, link_status);
> - if (retval)
> + retval = drm_dp_dpcd_read(&dp->aux, DP_LANE0_1_STATUS, link_status, 2);
> + if (retval < 0)
> return retval;
>
> - retval = analogix_dp_read_bytes_from_dpcd(dp,
> - DP_ADJUST_REQUEST_LANE0_1, 2, adjust_request);
> - if (retval)
> + retval = drm_dp_dpcd_read(&dp->aux, DP_ADJUST_REQUEST_LANE0_1,
> + adjust_request, 2);
> + if (retval < 0)
> return retval;
>
> if (analogix_dp_clock_recovery_ok(link_status, lane_count) == 0) {
> /* set training pattern 2 for EQ */
> analogix_dp_set_training_pattern(dp, TRAINING_PTN2);
>
> - retval = analogix_dp_write_byte_to_dpcd(dp,
> - DP_TRAINING_PATTERN_SET,
> - DP_LINK_SCRAMBLING_DISABLE |
> - DP_TRAINING_PATTERN_2);
> - if (retval)
> + retval = drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
> + DP_LINK_SCRAMBLING_DISABLE |
> + DP_TRAINING_PATTERN_2);
ditto
> + if (retval < 0)
> return retval;
>
> dev_info(dp->dev, "Link Training Clock Recovery success\n");
> @@ -559,13 +431,12 @@ static int analogix_dp_process_clock_recovery(struct analogix_dp_device *dp)
> analogix_dp_set_lane_link_training(dp,
> dp->link_train.training_lane[lane], lane);
>
> - retval = analogix_dp_write_bytes_to_dpcd(dp,
> - DP_TRAINING_LANE0_SET, lane_count,
> - dp->link_train.training_lane);
> - if (retval)
> + retval = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET,
> + dp->link_train.training_lane, lane_count);
> + if (retval < 0)
> return retval;
>
> - return retval;
> + return 0;
> }
>
> static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp)
> @@ -578,9 +449,8 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp)
>
> lane_count = dp->link_train.lane_count;
>
> - retval = analogix_dp_read_bytes_from_dpcd(dp,
> - DP_LANE0_1_STATUS, 2, link_status);
> - if (retval)
> + retval = drm_dp_dpcd_read(&dp->aux, DP_LANE0_1_STATUS, link_status, 2);
> + if (retval < 0)
> return retval;
>
> if (analogix_dp_clock_recovery_ok(link_status, lane_count)) {
> @@ -588,14 +458,13 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp)
> return -EIO;
> }
>
> - retval = analogix_dp_read_bytes_from_dpcd(dp,
> - DP_ADJUST_REQUEST_LANE0_1, 2, adjust_request);
> - if (retval)
> + retval = drm_dp_dpcd_read(&dp->aux, DP_ADJUST_REQUEST_LANE0_1, adjust_request, 2);
> + if (retval < 0)
> return retval;
>
> - retval = analogix_dp_read_byte_from_dpcd(dp,
> - DP_LANE_ALIGN_STATUS_UPDATED, &link_align);
> - if (retval)
> + retval = drm_dp_dpcd_readb(&dp->aux, DP_LANE_ALIGN_STATUS_UPDATED,
> + &link_align);
> + if (retval < 0)
> return retval;
>
> analogix_dp_get_adjust_training_lane(dp, adjust_request);
> @@ -636,10 +505,12 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp)
> analogix_dp_set_lane_link_training(dp,
> dp->link_train.training_lane[lane], lane);
>
> - retval = analogix_dp_write_bytes_to_dpcd(dp, DP_TRAINING_LANE0_SET,
> - lane_count, dp->link_train.training_lane);
> + retval = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET,
> + dp->link_train.training_lane, lane_count);
> + if (retval < 0)
> + return retval;
>
> - return retval;
> + return 0;
> }
>
> static void analogix_dp_get_max_rx_bandwidth(struct analogix_dp_device *dp,
> @@ -653,7 +524,7 @@ static void analogix_dp_get_max_rx_bandwidth(struct analogix_dp_device *dp,
> * For DP rev.1.2, Maximum link rate of Main Link lanes
> * 0x06 = 1.62 Gbps, 0x0a = 2.7 Gbps, 0x14 = 5.4Gbps
> */
> - analogix_dp_read_byte_from_dpcd(dp, DP_MAX_LINK_RATE, &data);
> + drm_dp_dpcd_readb(&dp->aux, DP_MAX_LINK_RATE, &data);
> *bandwidth = data;
> }
>
> @@ -666,7 +537,7 @@ static void analogix_dp_get_max_rx_lane_count(struct analogix_dp_device *dp,
> * For DP rev.1.1, Maximum number of Main Link lanes
> * 0x01 = 1 lane, 0x02 = 2 lanes, 0x04 = 4 lanes
> */
> - analogix_dp_read_byte_from_dpcd(dp, DP_MAX_LANE_COUNT, &data);
> + drm_dp_dpcd_readb(&dp->aux, DP_MAX_LANE_COUNT, &data);
> *lane_count = DPCD_MAX_LANE_COUNT(data);
> }
>
> @@ -835,19 +706,15 @@ static void analogix_dp_enable_scramble(struct analogix_dp_device *dp,
> if (enable) {
> analogix_dp_enable_scrambling(dp);
>
> - analogix_dp_read_byte_from_dpcd(dp, DP_TRAINING_PATTERN_SET,
> - &data);
> - analogix_dp_write_byte_to_dpcd(dp,
> - DP_TRAINING_PATTERN_SET,
> - (u8)(data & ~DP_LINK_SCRAMBLING_DISABLE));
> + drm_dp_dpcd_readb(&dp->aux, DP_TRAINING_PATTERN_SET, &data);
> + drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
> + (u8)(data & ~DP_LINK_SCRAMBLING_DISABLE));
> } else {
> analogix_dp_disable_scrambling(dp);
>
> - analogix_dp_read_byte_from_dpcd(dp, DP_TRAINING_PATTERN_SET,
> - &data);
> - analogix_dp_write_byte_to_dpcd(dp,
> - DP_TRAINING_PATTERN_SET,
> - (u8)(data | DP_LINK_SCRAMBLING_DISABLE));
> + drm_dp_dpcd_readb(&dp->aux, DP_TRAINING_PATTERN_SET, &data);
> + drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
> + (u8)(data | DP_LINK_SCRAMBLING_DISABLE));
> }
> }
>
> @@ -926,12 +793,11 @@ static void analogix_dp_commit(struct analogix_dp_device *dp)
> int analogix_dp_get_modes(struct drm_connector *connector)
> {
> struct analogix_dp_device *dp = to_dp(connector);
> - struct edid *edid = (struct edid *)dp->edid;
> int num_modes = 0;
>
> - if (analogix_dp_handle_edid(dp) == 0) {
> - drm_mode_connector_update_edid_property(&dp->connector, edid);
> - num_modes += drm_add_edid_modes(&dp->connector, edid);
> + if ((dp->edid = drm_get_edid(connector, &dp->aux.ddc))) {
You could remove the '->edid' variable from struct analogix_dp_device,
cause we don't need this information at other places. You need to free
the edid after parsed it, cause each time when you call drm_get_edid()
helper, it would malloc a new space for 'edid'. Besides I would prefer
move the statement out of the condition.
struct edid *edid;
edid = drm_get_edid(connector, &dp->aux.ddc);
if (edid) {
drm_mode_connector_update_edid_property(&dp->connector, edid);
num_modes += drm_add_edid_modes(&dp->connector, edid);
kfree(edid);
}
> + drm_mode_connector_update_edid_property(&dp->connector, dp->edid);
> + num_modes += drm_add_edid_modes(&dp->connector, dp->edid);
> }
>
> if (dp->plat_data->panel)
> @@ -984,6 +850,138 @@ static const struct drm_connector_funcs analogix_dp_connector_funcs = {
> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> };
>
> +static ssize_t analogix_dpaux_transfer(struct drm_dp_aux *aux,
> + struct drm_dp_aux_msg *msg)
> +{
> + struct analogix_dp_device *dp = to_dp(aux);
> + u32 reg;
> + u8 *buffer = msg->buffer;
> + int timeout_loop = 0;
> + unsigned int i;
> + int retval = 0;
> +
> + /* Buffer size of AUX CH is 16 bytes */
> + if (WARN_ON(msg->size > 16))
> + return -E2BIG;
> +
> + /* Clear AUX CH data buffer */
> + reg = BUF_CLR;
> + writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
> +
> + switch (msg->request & ~DP_AUX_I2C_MOT) {
> + case DP_AUX_I2C_WRITE:
> + reg = AUX_TX_COMM_WRITE | AUX_TX_COMM_I2C_TRANSACTION;
> + if (msg->request & DP_AUX_I2C_MOT)
> + reg |= AUX_TX_COMM_MOT;
> +
> + break;
> +
> + case DP_AUX_I2C_READ:
> + reg = AUX_TX_COMM_READ | AUX_TX_COMM_I2C_TRANSACTION;
> + if (msg->request & DP_AUX_I2C_MOT)
> + reg |= AUX_TX_COMM_MOT;
> +
> + break;
> +
> + case DP_AUX_NATIVE_WRITE:
> + reg = AUX_TX_COMM_WRITE | AUX_TX_COMM_DP_TRANSACTION;
> + break;
> +
> + case DP_AUX_NATIVE_READ:
> + reg = AUX_TX_COMM_READ | AUX_TX_COMM_DP_TRANSACTION;
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + reg |= AUX_LENGTH(msg->size);
> + writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
> +
> + /* Select DPCD device address */
> + reg = AUX_ADDR_7_0(msg->address);
> + writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0);
> + reg = AUX_ADDR_15_8(msg->address);
> + writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8);
> + reg = AUX_ADDR_19_16(msg->address);
> + writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16);
> +
> + if ((msg->request & DP_AUX_I2C_READ) == 0) {
> + for (i = 0; i < msg->size; i++) {
> + reg = buffer[i];
> + writel(reg, dp->reg_base + ANALOGIX_DP_BUF_DATA_0 +
> + 4 * i);
> + retval++;
> + }
> + }
> +
> + /* Enable AUX CH operation */
> + reg = 0;
> +
> + /* Zero-sized messages specify address-only transactions. */
> + if (msg->size < 1)
> + reg |= ADDR_ONLY;
> +
> + reg |= AUX_EN;
> + writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
> +
> + /* Is AUX CH command reply received? */
> + /* TODO: Wait for an interrupt instead of looping? */
> + reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
> + while (!(reg & RPLY_RECEIV)) {
> + timeout_loop++;
> + if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
> + dev_err(dp->dev, "AUX CH command reply failed!\n");
> + return -ETIMEDOUT;
> + }
> + reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
> + usleep_range(10, 11);
> + }
> +
> + /* Clear interrupt source for AUX CH command reply */
> + writel(RPLY_RECEIV, dp->reg_base + ANALOGIX_DP_INT_STA);
> +
> + /* Clear interrupt source for AUX CH access error */
> + reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
> + if (reg & AUX_ERR) {
> + writel(AUX_ERR, dp->reg_base + ANALOGIX_DP_INT_STA);
> + return -EREMOTEIO;
> + }
> +
> + /* Check AUX CH error access status */
> + reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_STA);
> + if ((reg & AUX_STATUS_MASK) != 0) {
> + dev_err(dp->dev, "AUX CH error happens: %d\n\n",
> + reg & AUX_STATUS_MASK);
> + return -EREMOTEIO;
> + }
> +
> + if (msg->request & DP_AUX_I2C_READ) {
> + for (i = 0; i < msg->size; i++) {
> + reg = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0 +
> + 4 * i);
> + buffer[i] = (unsigned char)reg;
> + retval++;
> + }
> + }
> +
> + /* Check if Rx sends defer */
> + reg = readl(dp->reg_base + ANALOGIX_DP_AUX_RX_COMM);
> + if (reg == AUX_RX_COMM_AUX_DEFER)
> + msg->reply = DP_AUX_NATIVE_REPLY_DEFER;
> + else if (reg == AUX_RX_COMM_I2C_DEFER)
> + msg->reply = DP_AUX_I2C_REPLY_DEFER;
> + else if ((msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_I2C_WRITE ||
> + (msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_I2C_READ)
> + msg->reply = DP_AUX_I2C_REPLY_ACK;
> + else if ((msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_NATIVE_WRITE ||
> + (msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_NATIVE_READ)
> + msg->reply = DP_AUX_NATIVE_REPLY_ACK;
> +
> + return retval;
> +}
Would you like to move this function to 'analogix_dp_reg.c' ?
> +
> +
> static int analogix_dp_bridge_attach(struct drm_bridge *bridge)
> {
> struct analogix_dp_device *dp = bridge->driver_private;
> @@ -1355,6 +1353,14 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
> dp->drm_dev = drm_dev;
> dp->encoder = dp->plat_data->encoder;
>
> + dp->aux.name = "DP-AUX";
> + dp->aux.transfer = analogix_dpaux_transfer;
> + dp->aux.dev = &pdev->dev;
> +
> + ret = drm_dp_aux_register(&dp->aux);
> + if (ret)
> + goto err_disable_pm_runtime;
> +
> ret = analogix_dp_create_bridge(drm_dev, dp);
> if (ret) {
> DRM_ERROR("failed to create bridge (%d)\n", ret);
> @@ -1384,6 +1390,8 @@ void analogix_dp_unbind(struct device *dev, struct device *master,
> }
>
> pm_runtime_disable(dev);
> +
> + kfree(dp->edid);
Could remove this free code
Thanks,
- Yakir
> }
> EXPORT_SYMBOL_GPL(analogix_dp_unbind);
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index b45638043ec4..27eb27a71802 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -20,15 +20,6 @@
> #define MAX_CR_LOOP 5
> #define MAX_EQ_LOOP 5
>
> -/* I2C EDID Chip ID, Slave Address */
> -#define I2C_EDID_DEVICE_ADDR 0x50
> -#define I2C_E_EDID_DEVICE_ADDR 0x30
> -
> -#define EDID_BLOCK_LENGTH 0x80
> -#define EDID_HEADER_PATTERN 0x00
> -#define EDID_EXTENSION_FLAG 0x7e
> -#define EDID_CHECKSUM 0x7f
> -
> /* DP_MAX_LANE_COUNT */
> #define DPCD_ENHANCED_FRAME_CAP(x) (((x) >> 7) & 0x1)
> #define DPCD_MAX_LANE_COUNT(x) ((x) & 0x1f)
> @@ -166,6 +157,7 @@ struct analogix_dp_device {
> struct drm_device *drm_dev;
> struct drm_connector connector;
> struct drm_bridge *bridge;
> + struct drm_dp_aux aux;
> struct clk *clock;
> unsigned int irq;
> void __iomem *reg_base;
> @@ -176,7 +168,7 @@ struct analogix_dp_device {
> int dpms_mode;
> int hpd_gpio;
> bool force_hpd;
> - unsigned char edid[EDID_BLOCK_LENGTH * 2];
> + struct edid *edid;
>
> struct analogix_dp_plat_data *plat_data;
> };
> @@ -206,33 +198,6 @@ void analogix_dp_reset_aux(struct analogix_dp_device *dp);
> void analogix_dp_init_aux(struct analogix_dp_device *dp);
> int analogix_dp_get_plug_in_status(struct analogix_dp_device *dp);
> void analogix_dp_enable_sw_function(struct analogix_dp_device *dp);
> -int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp);
> -int analogix_dp_write_byte_to_dpcd(struct analogix_dp_device *dp,
> - unsigned int reg_addr,
> - unsigned char data);
> -int analogix_dp_read_byte_from_dpcd(struct analogix_dp_device *dp,
> - unsigned int reg_addr,
> - unsigned char *data);
> -int analogix_dp_write_bytes_to_dpcd(struct analogix_dp_device *dp,
> - unsigned int reg_addr,
> - unsigned int count,
> - unsigned char data[]);
> -int analogix_dp_read_bytes_from_dpcd(struct analogix_dp_device *dp,
> - unsigned int reg_addr,
> - unsigned int count,
> - unsigned char data[]);
> -int analogix_dp_select_i2c_device(struct analogix_dp_device *dp,
> - unsigned int device_addr,
> - unsigned int reg_addr);
> -int analogix_dp_read_byte_from_i2c(struct analogix_dp_device *dp,
> - unsigned int device_addr,
> - unsigned int reg_addr,
> - unsigned int *data);
> -int analogix_dp_read_bytes_from_i2c(struct analogix_dp_device *dp,
> - unsigned int device_addr,
> - unsigned int reg_addr,
> - unsigned int count,
> - unsigned char edid[]);
> void analogix_dp_set_link_bandwidth(struct analogix_dp_device *dp, u32 bwtype);
> void analogix_dp_get_link_bandwidth(struct analogix_dp_device *dp, u32 *bwtype);
> void analogix_dp_set_lane_count(struct analogix_dp_device *dp, u32 count);
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index 48030f0cf497..eac61aa4224f 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -585,330 +585,6 @@ int analogix_dp_write_byte_to_dpcd(struct analogix_dp_device *dp,
> return retval;
> }
>
> -int analogix_dp_read_byte_from_dpcd(struct analogix_dp_device *dp,
> - unsigned int reg_addr,
> - unsigned char *data)
> -{
> - u32 reg;
> - int i;
> - int retval;
> -
> - for (i = 0; i < 3; i++) {
> - /* Clear AUX CH data buffer */
> - reg = BUF_CLR;
> - writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
> -
> - /* Select DPCD device address */
> - reg = AUX_ADDR_7_0(reg_addr);
> - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0);
> - reg = AUX_ADDR_15_8(reg_addr);
> - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8);
> - reg = AUX_ADDR_19_16(reg_addr);
> - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16);
> -
> - /*
> - * Set DisplayPort transaction and read 1 byte
> - * If bit 3 is 1, DisplayPort transaction.
> - * If Bit 3 is 0, I2C transaction.
> - */
> - reg = AUX_TX_COMM_DP_TRANSACTION | AUX_TX_COMM_READ;
> - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
> -
> - /* Start AUX transaction */
> - retval = analogix_dp_start_aux_transaction(dp);
> - if (retval == 0)
> - break;
> -
> - dev_dbg(dp->dev, "%s: Aux Transaction fail!\n", __func__);
> - }
> -
> - /* Read data buffer */
> - reg = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0);
> - *data = (unsigned char)(reg & 0xff);
> -
> - return retval;
> -}
> -
> -int analogix_dp_write_bytes_to_dpcd(struct analogix_dp_device *dp,
> - unsigned int reg_addr,
> - unsigned int count,
> - unsigned char data[])
> -{
> - u32 reg;
> - unsigned int start_offset;
> - unsigned int cur_data_count;
> - unsigned int cur_data_idx;
> - int i;
> - int retval = 0;
> -
> - /* Clear AUX CH data buffer */
> - reg = BUF_CLR;
> - writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
> -
> - start_offset = 0;
> - while (start_offset < count) {
> - /* Buffer size of AUX CH is 16 * 4bytes */
> - if ((count - start_offset) > 16)
> - cur_data_count = 16;
> - else
> - cur_data_count = count - start_offset;
> -
> - for (i = 0; i < 3; i++) {
> - /* Select DPCD device address */
> - reg = AUX_ADDR_7_0(reg_addr + start_offset);
> - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0);
> - reg = AUX_ADDR_15_8(reg_addr + start_offset);
> - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8);
> - reg = AUX_ADDR_19_16(reg_addr + start_offset);
> - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16);
> -
> - for (cur_data_idx = 0; cur_data_idx < cur_data_count;
> - cur_data_idx++) {
> - reg = data[start_offset + cur_data_idx];
> - writel(reg, dp->reg_base +
> - ANALOGIX_DP_BUF_DATA_0 +
> - 4 * cur_data_idx);
> - }
> -
> - /*
> - * Set DisplayPort transaction and write
> - * If bit 3 is 1, DisplayPort transaction.
> - * If Bit 3 is 0, I2C transaction.
> - */
> - reg = AUX_LENGTH(cur_data_count) |
> - AUX_TX_COMM_DP_TRANSACTION | AUX_TX_COMM_WRITE;
> - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
> -
> - /* Start AUX transaction */
> - retval = analogix_dp_start_aux_transaction(dp);
> - if (retval == 0)
> - break;
> -
> - dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
> - __func__);
> - }
> -
> - start_offset += cur_data_count;
> - }
> -
> - return retval;
> -}
> -
> -int analogix_dp_read_bytes_from_dpcd(struct analogix_dp_device *dp,
> - unsigned int reg_addr,
> - unsigned int count,
> - unsigned char data[])
> -{
> - u32 reg;
> - unsigned int start_offset;
> - unsigned int cur_data_count;
> - unsigned int cur_data_idx;
> - int i;
> - int retval = 0;
> -
> - /* Clear AUX CH data buffer */
> - reg = BUF_CLR;
> - writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
> -
> - start_offset = 0;
> - while (start_offset < count) {
> - /* Buffer size of AUX CH is 16 * 4bytes */
> - if ((count - start_offset) > 16)
> - cur_data_count = 16;
> - else
> - cur_data_count = count - start_offset;
> -
> - /* AUX CH Request Transaction process */
> - for (i = 0; i < 3; i++) {
> - /* Select DPCD device address */
> - reg = AUX_ADDR_7_0(reg_addr + start_offset);
> - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0);
> - reg = AUX_ADDR_15_8(reg_addr + start_offset);
> - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8);
> - reg = AUX_ADDR_19_16(reg_addr + start_offset);
> - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16);
> -
> - /*
> - * Set DisplayPort transaction and read
> - * If bit 3 is 1, DisplayPort transaction.
> - * If Bit 3 is 0, I2C transaction.
> - */
> - reg = AUX_LENGTH(cur_data_count) |
> - AUX_TX_COMM_DP_TRANSACTION | AUX_TX_COMM_READ;
> - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
> -
> - /* Start AUX transaction */
> - retval = analogix_dp_start_aux_transaction(dp);
> - if (retval == 0)
> - break;
> -
> - dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
> - __func__);
> - }
> -
> - for (cur_data_idx = 0; cur_data_idx < cur_data_count;
> - cur_data_idx++) {
> - reg = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0
> - + 4 * cur_data_idx);
> - data[start_offset + cur_data_idx] =
> - (unsigned char)reg;
> - }
> -
> - start_offset += cur_data_count;
> - }
> -
> - return retval;
> -}
> -
> -int analogix_dp_select_i2c_device(struct analogix_dp_device *dp,
> - unsigned int device_addr,
> - unsigned int reg_addr)
> -{
> - u32 reg;
> - int retval;
> -
> - /* Set EDID device address */
> - reg = device_addr;
> - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0);
> - writel(0x0, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8);
> - writel(0x0, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16);
> -
> - /* Set offset from base address of EDID device */
> - writel(reg_addr, dp->reg_base + ANALOGIX_DP_BUF_DATA_0);
> -
> - /*
> - * Set I2C transaction and write address
> - * If bit 3 is 1, DisplayPort transaction.
> - * If Bit 3 is 0, I2C transaction.
> - */
> - reg = AUX_TX_COMM_I2C_TRANSACTION | AUX_TX_COMM_MOT |
> - AUX_TX_COMM_WRITE;
> - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
> -
> - /* Start AUX transaction */
> - retval = analogix_dp_start_aux_transaction(dp);
> - if (retval != 0)
> - dev_dbg(dp->dev, "%s: Aux Transaction fail!\n", __func__);
> -
> - return retval;
> -}
> -
> -int analogix_dp_read_byte_from_i2c(struct analogix_dp_device *dp,
> - unsigned int device_addr,
> - unsigned int reg_addr,
> - unsigned int *data)
> -{
> - u32 reg;
> - int i;
> - int retval;
> -
> - for (i = 0; i < 3; i++) {
> - /* Clear AUX CH data buffer */
> - reg = BUF_CLR;
> - writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
> -
> - /* Select EDID device */
> - retval = analogix_dp_select_i2c_device(dp, device_addr,
> - reg_addr);
> - if (retval != 0)
> - continue;
> -
> - /*
> - * Set I2C transaction and read data
> - * If bit 3 is 1, DisplayPort transaction.
> - * If Bit 3 is 0, I2C transaction.
> - */
> - reg = AUX_TX_COMM_I2C_TRANSACTION |
> - AUX_TX_COMM_READ;
> - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
> -
> - /* Start AUX transaction */
> - retval = analogix_dp_start_aux_transaction(dp);
> - if (retval == 0)
> - break;
> -
> - dev_dbg(dp->dev, "%s: Aux Transaction fail!\n", __func__);
> - }
> -
> - /* Read data */
> - if (retval == 0)
> - *data = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0);
> -
> - return retval;
> -}
> -
> -int analogix_dp_read_bytes_from_i2c(struct analogix_dp_device *dp,
> - unsigned int device_addr,
> - unsigned int reg_addr,
> - unsigned int count,
> - unsigned char edid[])
> -{
> - u32 reg;
> - unsigned int i, j;
> - unsigned int cur_data_idx;
> - unsigned int defer = 0;
> - int retval = 0;
> -
> - for (i = 0; i < count; i += 16) {
> - for (j = 0; j < 3; j++) {
> - /* Clear AUX CH data buffer */
> - reg = BUF_CLR;
> - writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
> -
> - /* Set normal AUX CH command */
> - reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
> - reg &= ~ADDR_ONLY;
> - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
> -
> - /*
> - * If Rx sends defer, Tx sends only reads
> - * request without sending address
> - */
> - if (!defer)
> - retval = analogix_dp_select_i2c_device(dp,
> - device_addr, reg_addr + i);
> - else
> - defer = 0;
> -
> - if (retval == 0) {
> - /*
> - * Set I2C transaction and write data
> - * If bit 3 is 1, DisplayPort transaction.
> - * If Bit 3 is 0, I2C transaction.
> - */
> - reg = AUX_LENGTH(16) |
> - AUX_TX_COMM_I2C_TRANSACTION |
> - AUX_TX_COMM_READ;
> - writel(reg, dp->reg_base +
> - ANALOGIX_DP_AUX_CH_CTL_1);
> -
> - /* Start AUX transaction */
> - retval = analogix_dp_start_aux_transaction(dp);
> - if (retval == 0)
> - break;
> -
> - dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
> - __func__);
> - }
> - /* Check if Rx sends defer */
> - reg = readl(dp->reg_base + ANALOGIX_DP_AUX_RX_COMM);
> - if (reg == AUX_RX_COMM_AUX_DEFER ||
> - reg == AUX_RX_COMM_I2C_DEFER) {
> - dev_err(dp->dev, "Defer: %d\n\n", reg);
> - defer = 1;
> - }
> - }
> -
> - for (cur_data_idx = 0; cur_data_idx < 16; cur_data_idx++) {
> - reg = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0
> - + 4 * cur_data_idx);
> - edid[i + cur_data_idx] = (unsigned char)reg;
> - }
> - }
> -
> - return retval;
> -}
> -
> void analogix_dp_set_link_bandwidth(struct analogix_dp_device *dp, u32 bwtype)
> {
> u32 reg;
Powered by blists - more mailing lists