Re: [PATCH v3 11/13] drm/msm: edp: Avoid drm_dp_link helpers

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Oct 21, 2019 at 04:34:35PM +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> 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 <treding@nvidia.com>
> ---
>  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, &dp_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(&ctrl->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, &ctrl->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, &value);
> +		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, &ctrl->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 = drm_dp_dpcd_readb(ctrl->drm_aux, DP_SET_POWER, &value);
> +		if (ret > 0) {
> +			value &= ~DP_SET_POWER_MASK;
> +			value |= DP_SET_POWER_D3;
> +
> +			drm_dp_dpcd_writeb(ctrl->drm_aux, DP_SET_POWER, value);
> +		}
> +	}
>  
>  	edp_ctrl_irq_enable(ctrl, 0);
>  
> @@ -1225,14 +1259,8 @@ int msm_edp_ctrl_get_panel_info(struct edp_ctrl *ctrl,
>  		edp_ctrl_irq_enable(ctrl, 1);
>  	}
>  
> -	ret = drm_dp_link_probe(ctrl->drm_aux, &ctrl->dp_link);
> -	if (ret) {
> -		pr_err("%s: read dpcd cap failed, %d\n", __func__, ret);
> -		goto disable_ret;
> -	}
> -
>  	/* Initialize link rate as panel max link rate */
> -	ctrl->link_rate = drm_dp_link_rate_to_bw_code(ctrl->dp_link.rate);
> +	ctrl->link_rate = ctrl->dpcd[DP_MAX_LINK_RATE];
>  
>  	ctrl->edid = drm_get_edid(connector, &ctrl->drm_aux->ddc);
>  	if (!ctrl->edid) {

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> -- 
> 2.23.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel