Re: [PATCH v3 11/13] drm/msm: edp: Avoid drm_dp_link helpers
On Mon, Oct 21, 2019 at 04:34:35PM +0200, Thierry Reding wrote:
> From: Thierry Reding
>
> During the discussion of patches that enhance the drm_dp_link helpers it
> was concluded that these helpers aren't very useful to begin with. Start
> pushing the equivalent code into individual drivers to ultimately remove
> them.
>
> Signed-off-by: Thierry Reding
> ---
> drivers/gpu/drm/msm/edp/edp_ctrl.c | 70 +-
> 1 file changed, 49 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/edp/edp_ctrl.c
> b/drivers/gpu/drm/msm/edp/edp_ctrl.c
> index 7f3dd3ffe2c9..0d9657cc70db 100644
> --- a/drivers/gpu/drm/msm/edp/edp_ctrl.c
> +++ b/drivers/gpu/drm/msm/edp/edp_ctrl.c
> @@ -89,7 +89,6 @@ struct edp_ctrl {
> /* edid raw data */
> struct edid *edid;
>
> - struct drm_dp_link dp_link;
> struct drm_dp_aux *drm_aux;
>
> /* dpcd raw data */
> @@ -403,7 +402,7 @@ static void edp_fill_link_cfg(struct edp_ctrl *ctrl)
> u32 prate;
> u32 lrate;
> u32 bpp;
> - u8 max_lane = ctrl->dp_link.num_lanes;
> + u8 max_lane = drm_dp_max_lane_count(ctrl->dpcd);
> u8 lane;
>
> prate = ctrl->pixel_rate;
> @@ -413,7 +412,7 @@ static void edp_fill_link_cfg(struct edp_ctrl *ctrl)
>* By default, use the maximum link rate and minimum lane count,
>* so that we can do rate down shift during link training.
>*/
> - ctrl->link_rate = drm_dp_link_rate_to_bw_code(ctrl->dp_link.rate);
> + ctrl->link_rate = ctrl->dpcd[DP_MAX_LINK_RATE];
>
> prate *= bpp;
> prate /= 8; /* in kByte */
> @@ -439,7 +438,7 @@ static void edp_config_ctrl(struct edp_ctrl *ctrl)
>
> data = EDP_CONFIGURATION_CTRL_LANES(ctrl->lane_cnt - 1);
>
> - if (ctrl->dp_link.capabilities & DP_LINK_CAP_ENHANCED_FRAMING)
> + if (drm_dp_enhanced_frame_cap(ctrl->dpcd))
> data |= EDP_CONFIGURATION_CTRL_ENHANCED_FRAMING;
>
> depth = EDP_6BIT;
> @@ -701,7 +700,7 @@ static int edp_link_rate_down_shift(struct edp_ctrl *ctrl)
>
> rate = ctrl->link_rate;
> lane = ctrl->lane_cnt;
> - max_lane = ctrl->dp_link.num_lanes;
> + max_lane = drm_dp_max_lane_count(ctrl->dpcd);
>
> bpp = ctrl->color_depth * 3;
> prate = ctrl->pixel_rate;
> @@ -751,18 +750,22 @@ static int edp_clear_training_pattern(struct edp_ctrl
> *ctrl)
>
> static int edp_do_link_train(struct edp_ctrl *ctrl)
> {
> + u8 values[2];
> int ret;
> - struct drm_dp_link dp_link;
>
> DBG("");
> /*
>* Set the current link rate and lane cnt to panel. They may have been
>* adjusted and the values are different from them in DPCD CAP
>*/
> - dp_link.num_lanes = ctrl->lane_cnt;
> - dp_link.rate = drm_dp_bw_code_to_link_rate(ctrl->link_rate);
> - dp_link.capabilities = ctrl->dp_link.capabilities;
> - if (drm_dp_link_configure(ctrl->drm_aux, _link) < 0)
> + values[0] = ctrl->lane_cnt;
> + values[1] = ctrl->link_rate;
> +
> + if (drm_dp_enhanced_frame_cap(ctrl->dpcd))
> + values[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
> +
> + if (drm_dp_dpcd_write(ctrl->drm_aux, DP_LINK_BW_SET, values,
> + sizeof(values)) < 0)
> return EDP_TRAIN_FAIL;
>
> ctrl->v_level = 0; /* start from default level */
> @@ -952,6 +955,7 @@ static void edp_ctrl_on_worker(struct work_struct *work)
> {
> struct edp_ctrl *ctrl = container_of(
> work, struct edp_ctrl, on_work);
> + u8 value;
> int ret;
>
> mutex_lock(>dev_mutex);
> @@ -965,9 +969,27 @@ static void edp_ctrl_on_worker(struct work_struct *work)
> edp_ctrl_link_enable(ctrl, 1);
>
> edp_ctrl_irq_enable(ctrl, 1);
> - ret = drm_dp_link_power_up(ctrl->drm_aux, >dp_link);
> - if (ret)
> - goto fail;
> +
> + /* DP_SET_POWER register is only available on DPCD v1.1 and later */
> + if (ctrl->dpcd[DP_DPCD_REV] >= 0x11) {
> + ret = drm_dp_dpcd_readb(ctrl->drm_aux, DP_SET_POWER, );
> + if (ret < 0)
> + goto fail;
> +
> + value &= ~DP_SET_POWER_MASK;
> + value |= DP_SET_POWER_D0;
> +
> + ret = drm_dp_dpcd_writeb(ctrl->drm_aux, DP_SET_POWER, value);
> + if (ret < 0)
> + goto fail;
> +
> + /*
> + * According to the DP 1.1 specification, a "Sink Device must
> + * exit the power saving state within 1 ms" (Section 2.5.3.1,
> + * Table 5-52, "Sink Control Field" (register 0x600).
> + */
> + usleep_range(1000, 2000);
> + }
>
> ctrl->power_on = true;
>
> @@ -1011,7 +1033,19 @@ static void edp_ctrl_off_worker(struct work_struct
> *work)
>
> edp_state_ctrl(ctrl, 0);
>
> - drm_dp_link_power_down(ctrl->drm_aux, >dp_link);
> + /* DP_SET_POWER register is only
[PATCH v3 11/13] drm/msm: edp: Avoid drm_dp_link helpers
From: Thierry Reding
During the discussion of patches that enhance the drm_dp_link helpers it
was concluded that these helpers aren't very useful to begin with. Start
pushing the equivalent code into individual drivers to ultimately remove
them.
Signed-off-by: Thierry Reding
---
drivers/gpu/drm/msm/edp/edp_ctrl.c | 70 +-
1 file changed, 49 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/msm/edp/edp_ctrl.c
b/drivers/gpu/drm/msm/edp/edp_ctrl.c
index 7f3dd3ffe2c9..0d9657cc70db 100644
--- a/drivers/gpu/drm/msm/edp/edp_ctrl.c
+++ b/drivers/gpu/drm/msm/edp/edp_ctrl.c
@@ -89,7 +89,6 @@ struct edp_ctrl {
/* edid raw data */
struct edid *edid;
- struct drm_dp_link dp_link;
struct drm_dp_aux *drm_aux;
/* dpcd raw data */
@@ -403,7 +402,7 @@ static void edp_fill_link_cfg(struct edp_ctrl *ctrl)
u32 prate;
u32 lrate;
u32 bpp;
- u8 max_lane = ctrl->dp_link.num_lanes;
+ u8 max_lane = drm_dp_max_lane_count(ctrl->dpcd);
u8 lane;
prate = ctrl->pixel_rate;
@@ -413,7 +412,7 @@ static void edp_fill_link_cfg(struct edp_ctrl *ctrl)
* By default, use the maximum link rate and minimum lane count,
* so that we can do rate down shift during link training.
*/
- ctrl->link_rate = drm_dp_link_rate_to_bw_code(ctrl->dp_link.rate);
+ ctrl->link_rate = ctrl->dpcd[DP_MAX_LINK_RATE];
prate *= bpp;
prate /= 8; /* in kByte */
@@ -439,7 +438,7 @@ static void edp_config_ctrl(struct edp_ctrl *ctrl)
data = EDP_CONFIGURATION_CTRL_LANES(ctrl->lane_cnt - 1);
- if (ctrl->dp_link.capabilities & DP_LINK_CAP_ENHANCED_FRAMING)
+ if (drm_dp_enhanced_frame_cap(ctrl->dpcd))
data |= EDP_CONFIGURATION_CTRL_ENHANCED_FRAMING;
depth = EDP_6BIT;
@@ -701,7 +700,7 @@ static int edp_link_rate_down_shift(struct edp_ctrl *ctrl)
rate = ctrl->link_rate;
lane = ctrl->lane_cnt;
- max_lane = ctrl->dp_link.num_lanes;
+ max_lane = drm_dp_max_lane_count(ctrl->dpcd);
bpp = ctrl->color_depth * 3;
prate = ctrl->pixel_rate;
@@ -751,18 +750,22 @@ static int edp_clear_training_pattern(struct edp_ctrl
*ctrl)
static int edp_do_link_train(struct edp_ctrl *ctrl)
{
+ u8 values[2];
int ret;
- struct drm_dp_link dp_link;
DBG("");
/*
* Set the current link rate and lane cnt to panel. They may have been
* adjusted and the values are different from them in DPCD CAP
*/
- dp_link.num_lanes = ctrl->lane_cnt;
- dp_link.rate = drm_dp_bw_code_to_link_rate(ctrl->link_rate);
- dp_link.capabilities = ctrl->dp_link.capabilities;
- if (drm_dp_link_configure(ctrl->drm_aux, _link) < 0)
+ values[0] = ctrl->lane_cnt;
+ values[1] = ctrl->link_rate;
+
+ if (drm_dp_enhanced_frame_cap(ctrl->dpcd))
+ values[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
+
+ if (drm_dp_dpcd_write(ctrl->drm_aux, DP_LINK_BW_SET, values,
+ sizeof(values)) < 0)
return EDP_TRAIN_FAIL;
ctrl->v_level = 0; /* start from default level */
@@ -952,6 +955,7 @@ static void edp_ctrl_on_worker(struct work_struct *work)
{
struct edp_ctrl *ctrl = container_of(
work, struct edp_ctrl, on_work);
+ u8 value;
int ret;
mutex_lock(>dev_mutex);
@@ -965,9 +969,27 @@ static void edp_ctrl_on_worker(struct work_struct *work)
edp_ctrl_link_enable(ctrl, 1);
edp_ctrl_irq_enable(ctrl, 1);
- ret = drm_dp_link_power_up(ctrl->drm_aux, >dp_link);
- if (ret)
- goto fail;
+
+ /* DP_SET_POWER register is only available on DPCD v1.1 and later */
+ if (ctrl->dpcd[DP_DPCD_REV] >= 0x11) {
+ ret = drm_dp_dpcd_readb(ctrl->drm_aux, DP_SET_POWER, );
+ if (ret < 0)
+ goto fail;
+
+ value &= ~DP_SET_POWER_MASK;
+ value |= DP_SET_POWER_D0;
+
+ ret = drm_dp_dpcd_writeb(ctrl->drm_aux, DP_SET_POWER, value);
+ if (ret < 0)
+ goto fail;
+
+ /*
+* According to the DP 1.1 specification, a "Sink Device must
+* exit the power saving state within 1 ms" (Section 2.5.3.1,
+* Table 5-52, "Sink Control Field" (register 0x600).
+*/
+ usleep_range(1000, 2000);
+ }
ctrl->power_on = true;
@@ -1011,7 +1033,19 @@ static void edp_ctrl_off_worker(struct work_struct *work)
edp_state_ctrl(ctrl, 0);
- drm_dp_link_power_down(ctrl->drm_aux, >dp_link);
+ /* DP_SET_POWER register is only available on DPCD v1.1 and later */
+ if (ctrl->dpcd[DP_DPCD_REV] >= 0x11) {
+ u8 value;
+ int ret;
+
+ ret =
