Re: [PATCH] drm/bridge: analogix_dp: Remove duplicated code v2
From: Tomeu Vizoso
Date: Mon Aug 08 2016 - 07:21:29 EST
On 8 August 2016 at 12:43, Yakir Yang <ykk@xxxxxxxxxxxxxx> wrote:
> + Archit
>
> Tomeu,
>
> Thanks for the update :-)
>
> But you have missed three tiny align problems, please see my inline notes,
> wish you could fix them. After that this patch looks good to me, so:
>
> Reviewed-by: Yakir Yang <ykk@xxxxxxxxxxxxxx>
Hi Yakir,
thanks for remembering the formatting issues and sorry for having
missed them before. I don't care much myself, but as the second line
isn't an argument to the function, I found it could be misleading if
it was aligned as if it was. A quick grep in the DRM sources showed
there isn't uniformity on this, so I'm happy to change it to whatever
the maintainer prefers.
> I guess this patch should go through Archit's drm_bridge tree, so I added
> him into the CC list.
Cool, thanks again. Archit, do you want me to submit a patch to
MAINTAINERS so the tools pick up your address for the right patches?
Thanks,
Tomeu
>
> - Yakir
>
>
>
> On 08/05/2016 08:59 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@xxxxxxxxxxxxx>
>> Tested-by: Javier Martinez Canillas <javier@xxxxxxxxxxxxxxx>
>> Tested-by: Sean Paul <seanpaul@xxxxxxxxxxxx>
>> Cc: Javier Martinez Canillas <javier@xxxxxxxxxxxxxxx>
>> Cc: Mika Kahola <mika.kahola@xxxxxxxxx>
>> Cc: Yakir Yang <ykk@xxxxxxxxxxxxxx>
>> Cc: Daniel Vetter <daniel.vetter@xxxxxxxxx>
>>
>> v2:
>> - A bunch of good fixes from Sean and Yakir
>> - Moved the transfer function to analogix_dp_reg.c
>> - Removed reference to the EDID from the dp struct
>> ---
>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 263 ++++--------
>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 40 +-
>> drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 451
>> ++++++---------------
>> 3 files changed, 204 insertions(+), 550 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..624fc4f44450 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));
>
> Note: "DPCD_LANE_COUNT_SET" haven't aligned with
> "DP_LANE_COUNT_ENHANCED_FRAME_EN"
>
>> 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);
>
> Note: "DP_TRAINING_PATTERN_1 " haven't aligned 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);
>
> Note: "DP_TRAINING_PATTERN_2" haven't aligned with
> "DP_LINK_SCRAMBLING_DISABLE "
>
>> + 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,14 @@ 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;
>> + struct edid *edid;
>> int num_modes = 0;
>> - if (analogix_dp_handle_edid(dp) == 0) {
>> + 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);
>> }
>> if (dp->plat_data->panel)
>> @@ -1231,6 +1100,14 @@ static int analogix_dp_dt_parse_pdata(struct
>> analogix_dp_device *dp)
>> return 0;
>> }
>> +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);
>> +
>> + return analogix_dp_transfer(dp, msg);
>> +}
>> +
>> int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
>> struct analogix_dp_plat_data *plat_data)
>> {
>> @@ -1355,6 +1232,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);
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> index b45638043ec4..3da338cd3178 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,6 @@ struct analogix_dp_device {
>> int dpms_mode;
>> int hpd_gpio;
>> bool force_hpd;
>> - unsigned char edid[EDID_BLOCK_LENGTH * 2];
>> struct analogix_dp_plat_data *plat_data;
>> };
>> @@ -206,33 +197,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);
>> @@ -278,4 +242,6 @@ int analogix_dp_is_video_stream_on(struct
>> analogix_dp_device *dp);
>> void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
>> void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
>> void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
>> +ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>> + struct drm_dp_aux_msg *msg);
>> #endif /* _ANALOGIX_DP_CORE_H */
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> index 48030f0cf497..7ac4caa6bc1c 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;
>> @@ -1322,3 +998,130 @@ void analogix_dp_disable_scrambling(struct
>> analogix_dp_device *dp)
>> reg |= SCRAMBLING_DISABLE;
>> writel(reg, dp->reg_base + ANALOGIX_DP_TRAINING_PTN_SET);
>> }
>> +
>> +ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>> + struct drm_dp_aux_msg *msg)
>> +{
>> + u32 reg;
>> + u8 *buffer = msg->buffer;
>> + int timeout_loop = 0;
>> + unsigned int i;
>> + int num_transferred = 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)) {
>> + for (i = 0; i < msg->size; i++) {
>> + reg = buffer[i];
>> + writel(reg, dp->reg_base + ANALOGIX_DP_BUF_DATA_0
>> +
>> + 4 * i);
>> + num_transferred++;
>> + }
>> + }
>> +
>> + /* Enable AUX CH operation */
>> + reg = AUX_EN;
>> +
>> + /* Zero-sized messages specify address-only transactions. */
>> + if (msg->size < 1)
>> + reg |= ADDR_ONLY;
>> +
>> + 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 (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {
>> + 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)) {
>> + dev_err(dp->dev, "AUX CH error happened: %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;
>> + num_transferred++;
>> + }
>> + }
>> +
>> + /* 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 num_transferred;
>> +}
>
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel