Re: [PATCH] drm/i915: Do not do link training fallback or prune modes for eDP

[Manasi Navare <manasi.d.navare@intel.com>]

On Thu, Aug 17, 2017 at 12:28:56PM -0700, Pandiyan, Dhinakaran wrote:
> On Thu, 2017-08-17 at 12:29 -0700, Manasi Navare wrote:
> > On Thu, Aug 17, 2017 at 12:51:27PM +0300, Jani Nikula wrote:
> > > On Wed, 16 Aug 2017, Manasi Navare <manasi.d.navare@intel.com> wrote:
> > > > In case of eDP because the panel has a fixed mode we cannot
> > > > link train fallback and prune modes since this results in
> > > > no modes available for eDP connector.
> > > > Also since its a panel, link training should not fail dynamically
> > > > based on cable conditions like in  case of DP.
> > > >
> > > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > > Cc: Jim Bride <jim.bride@linux.intel.com>
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dp.c               | 2 +-
> > > >  drivers/gpu/drm/i915/intel_dp_link_training.c | 3 ++-
> > > >  drivers/gpu/drm/i915/intel_drv.h              | 1 +
> > > >  3 files changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > index 4fd4853..edac0c8 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -109,7 +109,7 @@ static const int default_rates[] = { 162000, 270000, 540000 };
> > > >   * If a CPU or PCH DP output is attached to an eDP panel, this function
> > > >   * will return true, and false otherwise.
> > > >   */
> > > > -static bool is_edp(struct intel_dp *intel_dp)
> > > > +bool is_edp(struct intel_dp *intel_dp)
> > > 
> > > Make that intel_dp_is_edp when you expose it outside of intel_dp.c
> > > 
> > > BR,
> > > Jani
> > > 
> > 
> > Yea renaming it as intel_dp_is_edp() makes sense after making it a non static function.
> > So should I make a seperate patch for this change and changing the name at all places
> > that call is_edp() currently?
> > 
> > Regards
> > Manasi
> > 
> 
> Don't we already have a intel_dp_is_edp() that checks VBT? That'll need
> to be renamed if is_edp() is going to become intel_dp_is_edp() 
>

Yes just noticed we do have that intel_dp_is_edp().
But I agree that we need to add intel_dp prefix if we are going
to expose the is_edp() function.
Do you have any suggestions for the name?
 
> Btw I remember seeing Jim's psr patch making this function
> global(non-static?) too.
>

Yes I have looked at it and that patch is still in review, 
so this review comment applies to his patch as well.
So either he makes this change in his or it happens here.
I plan to add this comment to Jim's PSR patch review too.

Regards
Manasi
> 
> > > >  {
> > > >  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > >  
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > > > index 05907fa..18ec61f 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > > > @@ -332,7 +332,8 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
> > > >  		      intel_connector->base.base.id,
> > > >  		      intel_connector->base.name,
> > > >  		      intel_dp->link_rate, intel_dp->lane_count);
> > > > -	if (!intel_dp_get_link_train_fallback_values(intel_dp,
> > > > +	/* Dont fallback and prune modes if its eDP */
> > > > +	if (!is_edp(intel_dp) && !intel_dp_get_link_train_fallback_values(intel_dp,
> > > >  						     intel_dp->link_rate,
> > > >  						     intel_dp->lane_count))
> > > >  		/* Schedule a Hotplug Uevent to userspace to start modeset */
> > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > > index fa47285..9800a15 100644
> > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > @@ -1548,6 +1548,7 @@ static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
> > > >  }
> > > >  
> > > >  bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> > > > +bool is_edp(struct intel_dp *intel_dp);
> > > >  int intel_dp_link_required(int pixel_clock, int bpp);
> > > >  int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
> > > >  bool intel_digital_port_connected(struct drm_i915_private *dev_priv,
> > > 
> > > -- 
> > > Jani Nikula, Intel Open Source Technology Center
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx