[PATCHv3 0/4] Add USB typeC based DP support for BXT platform

[PATCHv3 0/4] Add USB typeC based DP support for BXT platform

[Durgadoss R]

This patch series adds upfront link training support to enable
USB type C based DP on BXT platform.

To support USB type C alternate DP mode, the display driver needs to
know the number of lanes required by the DP panel as well as number
of lanes that can be supported by the type-C cable. Sometimes, the
type-C cable may limit the bandwidth even if Panel can support
more lanes.

The goal is to find out the number of lanes which can be supported
using a particular cable so that we can cap 'max_available_lanes'
to that number during modeset.

Patch   1/4   :Moves finding unused crtc to a common function
Patch   2,3/4 :Update pll config's crtc_mask/dpll hw_state to
                use in upfront link train
Patch   4/4   :Upfront implementation for DDI platforms
                (for now, tested on BXT B1).

Changes from v2:
* Rebased on top of intel_dpll_mgr.c code.
* Use drm_atomic_commit instead of atomic_helper_dpms, since the
  later uses crtc->acquire_ctx used also in legacy paths.
* Using a drm_atomic_state helps in using the shared_dpll APIs
  as is, thus keeping the upfront code away from changing pll
  internals directly.
* Fixed locking/retry/backoff logic in upfront according to
  atomic implementation.
Changes from v1:
* Using atomic_helper_dpms() to do DPMS off for upfront link
  training, instead of using load detect functions.
* Made intel_get_shared_dpll handle non-atomic paths by
  duplicating the required shared_dpll_config and then working
  on top of it. This helps in upfront link train code not
  directly touch the 'pll/pll->config' internals.
* Fixed the link_bw update logic in intel_dp_get_link_retry_params()
  for non-HBR2 panels.
* As per comments on earlier version, merged few patches
  (that added new functions) with the upfront link train patch,
  to facilitate easy review.

Link for v2:    https://patchwork.freedesktop.org/patch/72207/
Link for v1:    https://patchwork.freedesktop.org/patch/67784/
Link for RFCv2: https://patchwork.freedesktop.org/patch/61776/
Link for RFCv1: https://patchwork.freedesktop.org/patch/59589/

Durgadoss R (4):
  drm/i915: Make finding unused crtc as a generic function
  drm/i915: Store the dpll config in crtc_state->shared_dpll
  drm/i915: Update dpll_hw_state if mask is same
  drm/i915/dp: Enable Upfront link training for typeC DP support on BXT

 drivers/gpu/drm/i915/intel_ddi.c      |  46 +++++++++
 drivers/gpu/drm/i915/intel_display.c  |  74 ++++++++------
 drivers/gpu/drm/i915/intel_dp.c       | 180 +++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_dpll_mgr.c |   8 +-
 drivers/gpu/drm/i915/intel_drv.h      |   7 ++
 5 files changed, 279 insertions(+), 36 deletions(-)

-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCHv3 1/4] drm/i915: Make finding unused crtc as a generic function

[Durgadoss R]

Looping over the crtc list and finding an unused crtc
has other users like upfront link training. Hence move it to
a common function to re-use the logic.

v3:
* Added kernel Doc and removed an invalid comment (Ander)
* Rebased on top of latest code which includes locking
  for state->enable usage.
v2:
* Made this as a separate function instead of having it
  inside upfront link train code.

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 74 +++++++++++++++++++++---------------
 drivers/gpu/drm/i915/intel_drv.h     |  2 +
 2 files changed, 45 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e6b5ee5..3c1fbcd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10366,6 +10366,43 @@ static int intel_modeset_setup_plane_state(struct drm_atomic_state *state,
 	return 0;
 }
 
+/**
+ * intel_get_unused_crtc - Find an unused crtc for the given encoder
+ * @encoder: drm_encoder structure
+ * @ctx: ctx pointer to use with locking
+ *
+ * This function tries to find an unused crtc that can drive
+ * the given encoder. Returns NULL on failure.
+ */
+struct drm_crtc *intel_get_unused_crtc(struct drm_encoder *encoder,
+				struct drm_modeset_acquire_ctx *ctx)
+{
+	struct drm_crtc *possible_crtc;
+	struct drm_crtc *crtc = NULL;
+	struct drm_device *dev = encoder->dev;
+	int ret, i = -1;
+
+	for_each_crtc(dev, possible_crtc) {
+		i++;
+		if (!(encoder->possible_crtcs & (1 << i)))
+			continue;
+
+		ret = drm_modeset_lock(&possible_crtc->mutex, ctx);
+		if (ret)
+			return ERR_PTR(ret);
+
+		if (possible_crtc->state->enable) {
+			drm_modeset_unlock(&possible_crtc->mutex);
+			continue;
+		}
+
+		crtc = possible_crtc;
+		break;
+	}
+
+	return crtc;
+}
+
 bool intel_get_load_detect_pipe(struct drm_connector *connector,
 				struct drm_display_mode *mode,
 				struct intel_load_detect_pipe *old,
@@ -10374,7 +10411,6 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
 	struct intel_crtc *intel_crtc;
 	struct intel_encoder *intel_encoder =
 		intel_attached_encoder(connector);
-	struct drm_crtc *possible_crtc;
 	struct drm_encoder *encoder = &intel_encoder->base;
 	struct drm_crtc *crtc = NULL;
 	struct drm_device *dev = encoder->dev;
@@ -10383,7 +10419,7 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
 	struct drm_atomic_state *state = NULL, *restore_state = NULL;
 	struct drm_connector_state *connector_state;
 	struct intel_crtc_state *crtc_state;
-	int ret, i = -1;
+	int ret;
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
 		      connector->base.id, connector->name,
@@ -10413,39 +10449,15 @@ retry:
 		ret = drm_modeset_lock(&crtc->mutex, ctx);
 		if (ret)
 			goto fail;
-
-		/* Make sure the crtc and connector are running */
-		goto found;
-	}
-
-	/* Find an unused one (if possible) */
-	for_each_crtc(dev, possible_crtc) {
-		i++;
-		if (!(encoder->possible_crtcs & (1 << i)))
-			continue;
-
-		ret = drm_modeset_lock(&possible_crtc->mutex, ctx);
-		if (ret)
+	} else {
+		crtc = intel_get_unused_crtc(encoder, ctx);
+		if (IS_ERR_OR_NULL(crtc)) {
+			ret = PTR_ERR_OR_ZERO(crtc);
+			DRM_DEBUG_KMS("no pipe available for load-detect\n");
 			goto fail;
-
-		if (possible_crtc->state->enable) {
-			drm_modeset_unlock(&possible_crtc->mutex);
-			continue;
 		}
-
-		crtc = possible_crtc;
-		break;
-	}
-
-	/*
-	 * If we didn't find an unused CRTC, don't use any.
-	 */
-	if (!crtc) {
-		DRM_DEBUG_KMS("no pipe available for load-detect\n");
-		goto fail;
 	}
 
-found:
 	intel_crtc = to_intel_crtc(crtc);
 
 	ret = drm_modeset_lock(&crtc->primary->mutex, ctx);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9255b56..3873af5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1152,6 +1152,8 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
 void intel_release_load_detect_pipe(struct drm_connector *connector,
 				    struct intel_load_detect_pipe *old,
 				    struct drm_modeset_acquire_ctx *ctx);
+struct drm_crtc *intel_get_unused_crtc(struct drm_encoder *encoder,
+				struct drm_modeset_acquire_ctx *ctx);
 int intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
 			       unsigned int rotation);
 struct drm_framebuffer *
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCHv3 1/4] drm/i915: Make finding unused crtc as a generic function

[Ander Conselvan De Oliveira]

On Wed, 2016-04-06 at 17:23 +0530, Durgadoss R wrote:
> Looping over the crtc list and finding an unused crtc
> has other users like upfront link training. Hence move it to
> a common function to re-use the logic.
> 
> v3:
> * Added kernel Doc and removed an invalid comment (Ander)
> * Rebased on top of latest code which includes locking
>   for state->enable usage.
> v2:
> * Made this as a separate function instead of having it
>   inside upfront link train code.
> 
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 74 +++++++++++++++++++++--------------
> -
>  drivers/gpu/drm/i915/intel_drv.h     |  2 +
>  2 files changed, 45 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index e6b5ee5..3c1fbcd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10366,6 +10366,43 @@ static int intel_modeset_setup_plane_state(struct
> drm_atomic_state *state,
>  	return 0;
>  }
>  
> +/**
> + * intel_get_unused_crtc - Find an unused crtc for the given encoder
> + * @encoder: drm_encoder structure
> + * @ctx: ctx pointer to use with locking
> + *
> + * This function tries to find an unused crtc that can drive
> + * the given encoder. Returns NULL on failure.

This doesn't match the code below, since it now returns an unused crtc or an
ERR_PTR. It never returns NULL.

Ander

> + */
> +struct drm_crtc *intel_get_unused_crtc(struct drm_encoder *encoder,
> +				struct drm_modeset_acquire_ctx *ctx)
> +{
> +	struct drm_crtc *possible_crtc;
> +	struct drm_crtc *crtc = NULL;
> +	struct drm_device *dev = encoder->dev;
> +	int ret, i = -1;
> +
> +	for_each_crtc(dev, possible_crtc) {
> +		i++;
> +		if (!(encoder->possible_crtcs & (1 << i)))
> +			continue;
> +
> +		ret = drm_modeset_lock(&possible_crtc->mutex, ctx);
> +		if (ret)
> +			return ERR_PTR(ret);
> +
> +		if (possible_crtc->state->enable) {
> +			drm_modeset_unlock(&possible_crtc->mutex);
> +			continue;
> +		}
> +
> +		crtc = possible_crtc;
> +		break;
> +	}
> +
> +	return crtc;
> +}
> +
>  bool intel_get_load_detect_pipe(struct drm_connector *connector,
>  				struct drm_display_mode *mode,
>  				struct intel_load_detect_pipe *old,
> @@ -10374,7 +10411,6 @@ bool intel_get_load_detect_pipe(struct drm_connector
> *connector,
>  	struct intel_crtc *intel_crtc;
>  	struct intel_encoder *intel_encoder =
>  		intel_attached_encoder(connector);
> -	struct drm_crtc *possible_crtc;
>  	struct drm_encoder *encoder = &intel_encoder->base;
>  	struct drm_crtc *crtc = NULL;
>  	struct drm_device *dev = encoder->dev;
> @@ -10383,7 +10419,7 @@ bool intel_get_load_detect_pipe(struct drm_connector
> *connector,
>  	struct drm_atomic_state *state = NULL, *restore_state = NULL;
>  	struct drm_connector_state *connector_state;
>  	struct intel_crtc_state *crtc_state;
> -	int ret, i = -1;
> +	int ret;
>  
>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
>  		      connector->base.id, connector->name,
> @@ -10413,39 +10449,15 @@ retry:
>  		ret = drm_modeset_lock(&crtc->mutex, ctx);
>  		if (ret)
>  			goto fail;
> -
> -		/* Make sure the crtc and connector are running */
> -		goto found;
> -	}
> -
> -	/* Find an unused one (if possible) */
> -	for_each_crtc(dev, possible_crtc) {
> -		i++;
> -		if (!(encoder->possible_crtcs & (1 << i)))
> -			continue;
> -
> -		ret = drm_modeset_lock(&possible_crtc->mutex, ctx);
> -		if (ret)
> +	} else {
> +		crtc = intel_get_unused_crtc(encoder, ctx);
> +		if (IS_ERR_OR_NULL(crtc)) {
> +			ret = PTR_ERR_OR_ZERO(crtc);
> +			DRM_DEBUG_KMS("no pipe available for load-detect\n");
>  			goto fail;
> -
> -		if (possible_crtc->state->enable) {
> -			drm_modeset_unlock(&possible_crtc->mutex);
> -			continue;
>  		}
> -
> -		crtc = possible_crtc;
> -		break;
> -	}
> -
> -	/*
> -	 * If we didn't find an unused CRTC, don't use any.
> -	 */
> -	if (!crtc) {
> -		DRM_DEBUG_KMS("no pipe available for load-detect\n");
> -		goto fail;
>  	}
>  
> -found:
>  	intel_crtc = to_intel_crtc(crtc);
>  
>  	ret = drm_modeset_lock(&crtc->primary->mutex, ctx);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 9255b56..3873af5 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1152,6 +1152,8 @@ bool intel_get_load_detect_pipe(struct drm_connector
> *connector,
>  void intel_release_load_detect_pipe(struct drm_connector *connector,
>  				    struct intel_load_detect_pipe *old,
>  				    struct drm_modeset_acquire_ctx *ctx);
> +struct drm_crtc *intel_get_unused_crtc(struct drm_encoder *encoder,
> +				struct drm_modeset_acquire_ctx *ctx);
>  int intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
>  			       unsigned int rotation);
>  struct drm_framebuffer *
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCHv3 1/4] drm/i915: Make finding unused crtc as a generic function

[R, Durgadoss]

>-----Original Message-----
>From: Ander Conselvan De Oliveira [mailto:conselvan2@gmail.com]
>Sent: Monday, April 11, 2016 6:06 PM
>To: R, Durgadoss <durgadoss.r@intel.com>; intel-gfx@lists.freedesktop.org
>Cc: ville.syrjala@linux.intel.com
>Subject: Re: [PATCHv3 1/4] drm/i915: Make finding unused crtc as a generic function
>
>On Wed, 2016-04-06 at 17:23 +0530, Durgadoss R wrote:
>> Looping over the crtc list and finding an unused crtc
>> has other users like upfront link training. Hence move it to
>> a common function to re-use the logic.
>>
>> v3:
>> * Added kernel Doc and removed an invalid comment (Ander)
>> * Rebased on top of latest code which includes locking
>>   for state->enable usage.
>> v2:
>> * Made this as a separate function instead of having it
>>   inside upfront link train code.
>>
>> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 74 +++++++++++++++++++++--------------
>> -
>>  drivers/gpu/drm/i915/intel_drv.h     |  2 +
>>  2 files changed, 45 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index e6b5ee5..3c1fbcd 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -10366,6 +10366,43 @@ static int intel_modeset_setup_plane_state(struct
>> drm_atomic_state *state,
>>  	return 0;
>>  }
>>
>> +/**
>> + * intel_get_unused_crtc - Find an unused crtc for the given encoder
>> + * @encoder: drm_encoder structure
>> + * @ctx: ctx pointer to use with locking
>> + *
>> + * This function tries to find an unused crtc that can drive
>> + * the given encoder. Returns NULL on failure.
>
>This doesn't match the code below, since it now returns an unused crtc or an
>ERR_PTR. It never returns NULL.

If the 'state->enable'  is set for all crtcs, then I think it can return a NULL.
Anyway, will fix the comment so say "Null or an error pointer" on failure.

Thanks,
Durga

>
>Ander
>
>> + */
>> +struct drm_crtc *intel_get_unused_crtc(struct drm_encoder *encoder,
>> +				struct drm_modeset_acquire_ctx *ctx)
>> +{
>> +	struct drm_crtc *possible_crtc;
>> +	struct drm_crtc *crtc = NULL;
>> +	struct drm_device *dev = encoder->dev;
>> +	int ret, i = -1;
>> +
>> +	for_each_crtc(dev, possible_crtc) {
>> +		i++;
>> +		if (!(encoder->possible_crtcs & (1 << i)))
>> +			continue;
>> +
>> +		ret = drm_modeset_lock(&possible_crtc->mutex, ctx);
>> +		if (ret)
>> +			return ERR_PTR(ret);
>> +
>> +		if (possible_crtc->state->enable) {
>> +			drm_modeset_unlock(&possible_crtc->mutex);
>> +			continue;
>> +		}
>> +
>> +		crtc = possible_crtc;
>> +		break;
>> +	}
>> +
>> +	return crtc;
>> +}
>> +
>>  bool intel_get_load_detect_pipe(struct drm_connector *connector,
>>  				struct drm_display_mode *mode,
>>  				struct intel_load_detect_pipe *old,
>> @@ -10374,7 +10411,6 @@ bool intel_get_load_detect_pipe(struct drm_connector
>> *connector,
>>  	struct intel_crtc *intel_crtc;
>>  	struct intel_encoder *intel_encoder =
>>  		intel_attached_encoder(connector);
>> -	struct drm_crtc *possible_crtc;
>>  	struct drm_encoder *encoder = &intel_encoder->base;
>>  	struct drm_crtc *crtc = NULL;
>>  	struct drm_device *dev = encoder->dev;
>> @@ -10383,7 +10419,7 @@ bool intel_get_load_detect_pipe(struct drm_connector
>> *connector,
>>  	struct drm_atomic_state *state = NULL, *restore_state = NULL;
>>  	struct drm_connector_state *connector_state;
>>  	struct intel_crtc_state *crtc_state;
>> -	int ret, i = -1;
>> +	int ret;
>>
>>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
>>  		      connector->base.id, connector->name,
>> @@ -10413,39 +10449,15 @@ retry:
>>  		ret = drm_modeset_lock(&crtc->mutex, ctx);
>>  		if (ret)
>>  			goto fail;
>> -
>> -		/* Make sure the crtc and connector are running */
>> -		goto found;
>> -	}
>> -
>> -	/* Find an unused one (if possible) */
>> -	for_each_crtc(dev, possible_crtc) {
>> -		i++;
>> -		if (!(encoder->possible_crtcs & (1 << i)))
>> -			continue;
>> -
>> -		ret = drm_modeset_lock(&possible_crtc->mutex, ctx);
>> -		if (ret)
>> +	} else {
>> +		crtc = intel_get_unused_crtc(encoder, ctx);
>> +		if (IS_ERR_OR_NULL(crtc)) {
>> +			ret = PTR_ERR_OR_ZERO(crtc);
>> +			DRM_DEBUG_KMS("no pipe available for load-detect\n");
>>  			goto fail;
>> -
>> -		if (possible_crtc->state->enable) {
>> -			drm_modeset_unlock(&possible_crtc->mutex);
>> -			continue;
>>  		}
>> -
>> -		crtc = possible_crtc;
>> -		break;
>> -	}
>> -
>> -	/*
>> -	 * If we didn't find an unused CRTC, don't use any.
>> -	 */
>> -	if (!crtc) {
>> -		DRM_DEBUG_KMS("no pipe available for load-detect\n");
>> -		goto fail;
>>  	}
>>
>> -found:
>>  	intel_crtc = to_intel_crtc(crtc);
>>
>>  	ret = drm_modeset_lock(&crtc->primary->mutex, ctx);
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index 9255b56..3873af5 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1152,6 +1152,8 @@ bool intel_get_load_detect_pipe(struct drm_connector
>> *connector,
>>  void intel_release_load_detect_pipe(struct drm_connector *connector,
>>  				    struct intel_load_detect_pipe *old,
>>  				    struct drm_modeset_acquire_ctx *ctx);
>> +struct drm_crtc *intel_get_unused_crtc(struct drm_encoder *encoder,
>> +				struct drm_modeset_acquire_ctx *ctx);
>>  int intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
>>  			       unsigned int rotation);
>>  struct drm_framebuffer *
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCHv3 2/4] drm/i915: Store the dpll config in crtc_state->shared_dpll

[Durgadoss R]

Currently, the required shared dpll is saved in the crtc_state.
Similarly, this patch saves the dpll config values also, so that
these values (through crtc_state->shared_dpll->config.hw_state)
can be used for upfront link training.

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 1175eeb..cad10f2 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -248,6 +248,7 @@ intel_reference_shared_dpll(struct intel_shared_dpll *pll,
 			 pipe_name(crtc->pipe));
 
 	intel_shared_dpll_config_get(shared_dpll, pll, crtc);
+	crtc_state->shared_dpll->config = shared_dpll[i];
 }
 
 void intel_shared_dpll_commit(struct drm_atomic_state *state)
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCHv3 2/4] drm/i915: Store the dpll config in crtc_state->shared_dpll

[Conselvan De Oliveira, Ander]

On Wed, 2016-04-06 at 17:23 +0530, Durgadoss R wrote:
> Currently, the required shared dpll is saved in the crtc_state.
> Similarly, this patch saves the dpll config values also, so that
> these values (through crtc_state->shared_dpll->config.hw_state)
> can be used for upfront link training.
> 
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index 1175eeb..cad10f2 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -248,6 +248,7 @@ intel_reference_shared_dpll(struct intel_shared_dpll *pll,
>  			 pipe_name(crtc->pipe));
>  
>  	intel_shared_dpll_config_get(shared_dpll, pll, crtc);
> +	crtc_state->shared_dpll->config = shared_dpll[i];

This overwrites the state stored in dev_priv->shared_dpll[i].config, so it means
we loose the current state set in the hardware. If the atomic check fails after
this, the software tracking of the hw state gets messed up.

Ander

>  }
>  
>  void intel_shared_dpll_commit(struct drm_atomic_state *state)
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCHv3 2/4] drm/i915: Store the dpll config in crtc_state->shared_dpll

[R, Durgadoss]

>-----Original Message-----
>From: Conselvan De Oliveira, Ander
>Sent: Monday, April 11, 2016 6:07 PM
>To: intel-gfx@lists.freedesktop.org; R, Durgadoss <durgadoss.r@intel.com>
>Cc: ville.syrjala@linux.intel.com
>Subject: Re: [PATCHv3 2/4] drm/i915: Store the dpll config in crtc_state->shared_dpll
>
>On Wed, 2016-04-06 at 17:23 +0530, Durgadoss R wrote:
>> Currently, the required shared dpll is saved in the crtc_state.
>> Similarly, this patch saves the dpll config values also, so that
>> these values (through crtc_state->shared_dpll->config.hw_state)
>> can be used for upfront link training.
>>
>> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
>> b/drivers/gpu/drm/i915/intel_dpll_mgr.c
>> index 1175eeb..cad10f2 100644
>> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
>> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
>> @@ -248,6 +248,7 @@ intel_reference_shared_dpll(struct intel_shared_dpll *pll,
>>  			 pipe_name(crtc->pipe));
>>
>>  	intel_shared_dpll_config_get(shared_dpll, pll, crtc);
>> +	crtc_state->shared_dpll->config = shared_dpll[i];
>
>This overwrites the state stored in dev_priv->shared_dpll[i].config, so it means
>we loose the current state set in the hardware. If the atomic check fails after
>this, the software tracking of the hw state gets messed up.

But we need the computed dpll_state and crtc_mask set for pll enabling.
So, the only other way I see is to call shared_dpll_commit() in ddi_upfront() code
after we do get_shared_dpll(). 
What do you think of this ?

Or any other options ?

Thanks,
Durga

>
>Ander
>
>>  }
>>
>>  void intel_shared_dpll_commit(struct drm_atomic_state *state)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCHv3 3/4] drm/i915: Update dpll_hw_state if mask is same

[Durgadoss R]

Currently, the dpll_hw_state of a particular pll config
is not updated if the crtc_mask is non-zero, indicating
possibly shared dpll. But for things like upfront link
training, dpll_hw_state of a pll config needs to be
updated multiple times (for every new link_clock
calculation). This patch does that by letting the
update happen as long as the crtc_mask stays same.

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index cad10f2..f1ca753 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -235,13 +235,14 @@ intel_reference_shared_dpll(struct intel_shared_dpll *pll,
 {
 	struct intel_shared_dpll_config *shared_dpll;
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	unsigned int crtc_mask = 1 << drm_crtc_index(&crtc->base);
 	enum intel_dpll_id i = pll->id;
 
 	shared_dpll = intel_atomic_get_shared_dpll_state(crtc_state->base.state);
 
-	if (shared_dpll[i].crtc_mask == 0)
-		shared_dpll[i].hw_state =
-			crtc_state->dpll_hw_state;
+	if (shared_dpll[i].crtc_mask == 0 ||
+				shared_dpll[i].crtc_mask == crtc_mask)
+		shared_dpll[i].hw_state = crtc_state->dpll_hw_state;
 
 	crtc_state->shared_dpll = pll;
 	DRM_DEBUG_DRIVER("using %s for pipe %c\n", pll->name,
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCHv3 4/4] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT

[Durgadoss R]

To support USB type C alternate DP mode, the display driver needs to
know the number of lanes required by the DP panel as well as number
of lanes that can be supported by the type-C cable. Sometimes, the
type-C cable may limit the bandwidth even if Panel can support
more lanes. To address these scenarios, the display driver will
start link training with max lanes, and if that fails, the driver
falls back to x2 lanes; and repeats this procedure for all
bandwidth/lane configurations.

* Since link training is done before modeset only the port
  (and not pipe/planes) and its associated PLLs are enabled.
* On DP hotplug: Directly start link training on the crtc
  associated with the DP encoder.
* On Connected boot scenarios: When booted with an LFP and a DP,
  typically, BIOS brings up DP. In these cases, we disable the
  crtc and then do upfront link training; and let the subsequent
  modeset re-enable the crtc.
* All local changes made for upfront link training are reset
  to their previous values once it is done; so that the
  subsequent modeset is not aware of these changes.

Changes since v2:
* Rebased on top of latest dpll_mgr.c code and
  latest HPD related clean ups.
* Corrected return values from upfront (Ander)
* Corrected atomic locking for upfront in intel_dp.c (Ville)
Changes since v1:
*  all pll related functions inside ddi.c

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c |  46 ++++++++++
 drivers/gpu/drm/i915/intel_dp.c  | 180 ++++++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h |   5 ++
 3 files changed, 229 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 1e08385..0add10a 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2087,6 +2087,52 @@ intel_ddi_init_hdmi_connector(struct intel_digital_port *intel_dig_port)
 	return connector;
 }
 
+int intel_ddi_upfront_link_train(struct intel_dp *intel_dp,
+				struct intel_crtc *crtc)
+{
+	struct intel_connector *connector = intel_dp->attached_connector;
+	struct intel_encoder *encoder = connector->encoder;
+	struct intel_shared_dpll *pll = NULL;
+	uint8_t link_bw, lane_count;
+
+	if (WARN_ON(!crtc))
+		return -EINVAL;
+
+	/* Initialize with Max Link rate & lane count supported by panel */
+	link_bw =  intel_dp->dpcd[DP_MAX_LINK_RATE];
+	lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
+
+	do {
+		crtc->config->port_clock = drm_dp_bw_code_to_link_rate(link_bw);
+		crtc->config->lane_count = lane_count;
+
+		pll = intel_get_shared_dpll(crtc, crtc->config, encoder);
+		if (!pll) {
+			DRM_ERROR("Could not get shared DPLL\n");
+			return -EINVAL;
+		}
+
+		/* Enable PLL followed by port */
+		intel_enable_shared_dpll(crtc);
+		encoder->pre_enable(encoder);
+
+		/* Check if link training passed; if so update DPCD */
+		if (intel_dp->train_set_valid)
+			intel_dp_update_dpcd_params(intel_dp);
+
+		/* Disable port followed by PLL for next retry/clean up */
+		encoder->post_disable(encoder);
+		intel_disable_shared_dpll(crtc);
+
+	} while (!intel_dp->train_set_valid &&
+		intel_dp_get_link_retry_params(intel_dp, &lane_count, &link_bw));
+
+	DRM_DEBUG_KMS("Upfront link train %s: lanes:%d bw:%d\n",
+	intel_dp->train_set_valid ? "Passed" : "Failed", lane_count, link_bw);
+
+	return intel_dp->train_set_valid ? 0 : -1;
+}
+
 void intel_ddi_init(struct drm_device *dev, enum port port)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index da0c3d2..b8a42b8 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1595,6 +1595,41 @@ void intel_dp_set_link_params(struct intel_dp *intel_dp,
 	intel_dp->lane_count = pipe_config->lane_count;
 }
 
+void intel_dp_update_dpcd_params(struct intel_dp *intel_dp)
+{
+	intel_dp->dpcd[DP_MAX_LANE_COUNT] &= ~DP_MAX_LANE_COUNT_MASK;
+	intel_dp->dpcd[DP_MAX_LANE_COUNT] |=
+			intel_dp->lane_count & DP_MAX_LANE_COUNT_MASK;
+
+	intel_dp->dpcd[DP_MAX_LINK_RATE] =
+			drm_dp_link_rate_to_bw_code(intel_dp->link_rate);
+}
+
+bool intel_dp_get_link_retry_params(struct intel_dp *intel_dp,
+			uint8_t *lane_count, uint8_t *link_bw)
+{
+	/*
+	 * As per DP1.3 Spec, retry all link rates for a particular
+	 * lane count value, before reducing number of lanes.
+	 */
+	if (*link_bw == DP_LINK_BW_5_4) {
+		*link_bw = DP_LINK_BW_2_7;
+	} else if (*link_bw == DP_LINK_BW_2_7) {
+		*link_bw = DP_LINK_BW_1_62;
+	} else if (*lane_count == 4) {
+		*lane_count = 2;
+		*link_bw = intel_dp_max_link_bw(intel_dp);
+	} else if (*lane_count == 2) {
+		*lane_count = 1;
+		*link_bw = intel_dp_max_link_bw(intel_dp);
+	} else {
+		/* Tried all combinations, so exit */
+		return false;
+	}
+
+	return true;
+}
+
 static void intel_dp_prepare(struct intel_encoder *encoder)
 {
 	struct drm_device *dev = encoder->base.dev;
@@ -4578,6 +4613,135 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
 	intel_dp->has_audio = false;
 }
 
+static struct intel_crtc_state *intel_dp_upfront_get_crtc_state(
+				struct intel_crtc *crtc,
+				struct drm_modeset_acquire_ctx *ctx)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_atomic_state *state;
+	struct intel_crtc_state *crtc_state;
+
+	state = drm_atomic_state_alloc(dev);
+	if (!state)
+		return ERR_PTR(-ENOMEM);
+
+	state->acquire_ctx = ctx;
+
+	crtc_state = intel_atomic_get_crtc_state(state, crtc);
+	if (IS_ERR(crtc_state)) {
+		drm_atomic_state_free(state);
+		state = NULL;
+	}
+
+	return crtc_state;
+}
+
+static int intel_dp_upfront_commit(struct intel_crtc *crtc,
+				struct drm_modeset_acquire_ctx *ctx)
+{
+	int ret;
+	struct intel_crtc_state *crtc_state;
+	enum pipe pipe = crtc->pipe;
+
+	crtc_state = intel_dp_upfront_get_crtc_state(crtc, ctx);
+	if (IS_ERR(crtc_state))
+		return PTR_ERR(crtc_state);
+
+	DRM_DEBUG_KMS("Disabling crtc %c for upfront link train\n", pipe_name(pipe));
+
+	crtc_state->base.active = false;
+	ret = drm_atomic_commit(crtc_state->base.state);
+	if (ret) {
+		drm_atomic_state_free(crtc_state->base.state);
+		crtc_state->base.state = NULL;
+	}
+	return ret;
+}
+
+static int intel_dp_upfront_link_train(struct drm_connector *connector)
+{
+	struct intel_dp *intel_dp = intel_attached_dp(connector);
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct intel_encoder *intel_encoder = &intel_dig_port->base;
+	struct drm_device *dev = intel_encoder->base.dev;
+	struct drm_mode_config *config = &dev->mode_config;
+	struct drm_modeset_acquire_ctx ctx;
+	struct intel_crtc_state *crtc_state, *tmp_crtc_config;
+	struct intel_crtc *intel_crtc;
+	struct drm_crtc *crtc = NULL;
+	bool crtc_enabled = false;
+	int ret, status = 0;
+
+	if (!IS_BROXTON(dev))
+		return true;
+
+	drm_modeset_acquire_init(&ctx, 0);
+retry:
+	ret = drm_modeset_lock(&config->connection_mutex, &ctx);
+	if (ret)
+		goto exit_fail;
+
+	if (connector->state->crtc) {
+		crtc = connector->state->crtc;
+
+		ret = drm_modeset_lock(&crtc->mutex, &ctx);
+		if (ret)
+			goto exit_fail;
+
+		crtc_enabled = true;
+	} else {
+		crtc = intel_get_unused_crtc(&intel_encoder->base, &ctx);
+		if (IS_ERR_OR_NULL(crtc)) {
+			ret = PTR_ERR_OR_ZERO(crtc);
+			DRM_DEBUG_KMS("No pipe available for upfront link train:%d\n", ret);
+			goto exit_fail;
+		}
+		intel_encoder->base.crtc = crtc;
+	}
+
+	intel_crtc = to_intel_crtc(crtc);
+	DRM_DEBUG_KMS("Using pipe %c for upfront link training\n",
+					pipe_name(intel_crtc->pipe));
+
+	ret = drm_modeset_lock(&crtc->primary->mutex, &ctx);
+	if (ret)
+		goto exit_fail;
+
+	if (crtc_enabled) {
+		ret = intel_dp_upfront_commit(intel_crtc, &ctx);
+		if (ret)
+			goto exit_fail;
+	}
+
+	crtc_state = intel_dp_upfront_get_crtc_state(intel_crtc, &ctx);
+	if (IS_ERR(crtc_state)) {
+		ret = PTR_ERR(crtc_state);
+		goto exit_fail;
+	}
+
+	/* Save the existing config */
+	tmp_crtc_config = intel_crtc->config;
+	intel_crtc->config = crtc_state;
+
+	if (HAS_DDI(dev))
+		status = intel_ddi_upfront_link_train(intel_dp, intel_crtc);
+		/* Other platforms upfront link train call goes here..*/
+
+	/* Restore the saved config */
+	intel_crtc->config = tmp_crtc_config;
+	intel_encoder->base.crtc = crtc_enabled ? crtc : NULL;
+	drm_atomic_state_free(crtc_state->base.state);
+
+exit_fail:
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry;
+	}
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+	return status;
+}
+
 static void
 intel_dp_long_pulse(struct intel_connector *intel_connector)
 {
@@ -4588,7 +4752,7 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 	struct drm_device *dev = connector->dev;
 	enum drm_connector_status status;
 	enum intel_display_power_domain power_domain;
-	bool ret;
+	bool ret, do_upfront_link_train;
 	u8 sink_irq_vector;
 
 	power_domain = intel_display_port_aux_power_domain(intel_encoder);
@@ -4634,7 +4798,11 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 		drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
 		intel_dp_check_link_status(intel_dp);
 		drm_modeset_unlock(&dev->mode_config.connection_mutex);
-		goto out;
+		/*
+		 * If we are here, we have (re)read DPCD above and hence
+		 * need to do upfront link train again.
+		 */
+		goto upfront;
 	}
 
 	/*
@@ -4664,6 +4832,14 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
 	}
 
+upfront:
+	/* Do not do upfront link train, if it is a compliance request */
+	do_upfront_link_train =
+		intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT &&
+		intel_dp->compliance_test_type != DP_TEST_LINK_TRAINING;
+
+	if (do_upfront_link_train && intel_dp_upfront_link_train(connector))
+		status = connector_status_disconnected;
 out:
 	if (status != connector_status_connected) {
 		intel_dp_unset_edid(intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3873af5..ecd5fd3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1077,6 +1077,8 @@ void intel_ddi_clock_get(struct intel_encoder *encoder,
 			 struct intel_crtc_state *pipe_config);
 void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state);
 uint32_t ddi_signal_levels(struct intel_dp *intel_dp);
+int intel_ddi_upfront_link_train(struct intel_dp *intel_dp,
+				struct intel_crtc *crtc);
 
 /* intel_frontbuffer.c */
 void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
@@ -1275,6 +1277,9 @@ bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 			     struct intel_connector *intel_connector);
 void intel_dp_set_link_params(struct intel_dp *intel_dp,
 			      const struct intel_crtc_state *pipe_config);
+void intel_dp_update_dpcd_params(struct intel_dp *intel_dp);
+bool intel_dp_get_link_retry_params(struct intel_dp *intel_dp,
+				uint8_t *lane_count, uint8_t *link_bw);
 void intel_dp_start_link_train(struct intel_dp *intel_dp);
 void intel_dp_stop_link_train(struct intel_dp *intel_dp);
 void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for Add USB typeC based DP support for BXT platform (rev4)

[Patchwork]

== Series Details ==

Series: Add USB typeC based DP support for BXT platform (rev4)
URL   : https://patchwork.freedesktop.org/series/1731/
State : failure

== Summary ==

Series 1731v4 Add USB typeC based DP support for BXT platform
http://patchwork.freedesktop.org/api/1.0/series/1731/revisions/4/mbox/

Test drv_module_reload_basic:
                dmesg-warn -> PASS       (bsw-nuc-2)
                pass       -> SKIP       (skl-nuci5)
Test gem_ctx_param_basic:
        Subgroup invalid-ctx-set:
                dmesg-warn -> PASS       (bsw-nuc-2)
Test gem_storedw_loop:
        Subgroup basic-blt:
                dmesg-fail -> PASS       (bsw-nuc-2)
        Subgroup basic-bsd:
                skip       -> PASS       (bsw-nuc-2)
Test kms_addfb_basic:
        Subgroup bad-pitch-128:
                dmesg-warn -> PASS       (bsw-nuc-2)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                dmesg-warn -> PASS       (bsw-nuc-2)
        Subgroup basic-flip-vs-modeset:
                dmesg-warn -> PASS       (ilk-hp8440p) UNSTABLE
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                pass       -> DMESG-WARN (bsw-nuc-2)

bdw-nuci7        total:196  pass:184  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:196  pass:175  dwarn:0   dfail:0   fail:0   skip:21 
bsw-nuc-2        total:196  pass:158  dwarn:1   dfail:0   fail:0   skip:37 
byt-nuc          total:196  pass:161  dwarn:0   dfail:0   fail:0   skip:35 
hsw-brixbox      total:196  pass:174  dwarn:0   dfail:0   fail:0   skip:22 
hsw-gt2          total:183  pass:165  dwarn:0   dfail:0   fail:0   skip:17 
ilk-hp8440p      total:196  pass:132  dwarn:0   dfail:0   fail:0   skip:64 
ivb-t430s        total:196  pass:171  dwarn:0   dfail:0   fail:0   skip:25 
skl-i7k-2        total:196  pass:173  dwarn:0   dfail:0   fail:0   skip:23 
skl-nuci5        total:196  pass:184  dwarn:0   dfail:0   fail:0   skip:12 
snb-x220t        total:196  pass:162  dwarn:0   dfail:0   fail:1   skip:33 
BOOT FAILED for snb-dellxps

Results at /archive/results/CI_IGT_test/Patchwork_1816/

ab13a48af1102b84bdb8dcdfbef1b5b44fab8d3d drm-intel-nightly: 2016y-04m-06d-11h-23m-30s UTC integration manifest
9436c4214ad89a10722251642efb2fd6927726de drm/i915/dp: Enable Upfront link training for typeC DP support on BXT
ae288069e18a87dd684ef1e23a8211b857340061 drm/i915: Update dpll_hw_state if mask is same
236693fc96ba3ec44220ff42cde46469608fde08 drm/i915: Store the dpll config in crtc_state->shared_dpll
8ccb190aa0610ac5e4120b755e6f04d058329a2e drm/i915: Make finding unused crtc as a generic function

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx