Re: [PATCH] drm/bridge: analogix_dp: Remove duplicated code v2

[Sean Paul]

On Fri, Aug 5, 2016 at 8:59 AM, Tomeu Vizoso <tomeu.vizoso@xxxxxxxxxxxxx> 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>
>


Thanks for the respin.

Reviewed-by: Sean Paul <seanpaul@xxxxxxxxxxxx>

> 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));
>         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);
> +       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);
> +               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;
> +}
> --
> 2.7.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel