All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Do not do link training fallback or prune modes for eDP
@ 2017-08-17  6:58 Manasi Navare
  2017-08-17  7:01 ` Rodrigo Vivi
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Manasi Navare @ 2017-08-17  6:58 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

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)
 {
 	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,
-- 
2.1.4

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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Do not do link training fallback or prune modes for eDP
  2017-08-17  6:58 [PATCH] drm/i915: Do not do link training fallback or prune modes for eDP Manasi Navare
@ 2017-08-17  7:01 ` Rodrigo Vivi
  2017-08-17 19:46   ` Manasi Navare
  2017-08-17  7:21 ` ✓ Fi.CI.BAT: success for " Patchwork
  2017-08-17  9:51 ` [PATCH] " Jani Nikula
  2 siblings, 1 reply; 15+ messages in thread
From: Rodrigo Vivi @ 2017-08-17  7:01 UTC (permalink / raw)
  To: Manasi Navare, intel-gfx; +Cc: Daniel Vetter


[-- Attachment #1.1: Type: text/plain, Size: 3328 bytes --]

On Wed, Aug 16, 2017 at 11:51 PM 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.


What about downclock modes?!

>

> Also since its a panel, link training should not fail dynamically
> based on cable conditions like in  case of DP.


Is there any bug associated with this patch?


>
> 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)
>  {
>         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,
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

[-- Attachment #1.2: Type: text/html, Size: 4958 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Do not do link training fallback or prune modes for eDP
  2017-08-17  6:58 [PATCH] drm/i915: Do not do link training fallback or prune modes for eDP Manasi Navare
  2017-08-17  7:01 ` Rodrigo Vivi
@ 2017-08-17  7:21 ` Patchwork
  2017-08-17  9:51 ` [PATCH] " Jani Nikula
  2 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2017-08-17  7:21 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Do not do link training fallback or prune modes for eDP
URL   : https://patchwork.freedesktop.org/series/28900/
State : success

== Summary ==

Series 28900v1 drm/i915: Do not do link training fallback or prune modes for eDP
https://patchwork.freedesktop.org/api/1.0/series/28900/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-byt-n2820) fdo#101705

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:453s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:443s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:359s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:550s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:520s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:523s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:507s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:609s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:445s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:422s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:423s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:506s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:477s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:480s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:597s
fi-kbl-r         total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:596s
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:530s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:477s
fi-skl-6700k     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:482s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:486s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:440s
fi-skl-x1585l    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:503s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:553s
fi-snb-2600      total:279  pass:249  dwarn:0   dfail:0   fail:1   skip:29  time:409s

ada53b43f81fe618f3f0f1dfbd3dd776bb277323 drm-tip: 2017y-08m-16d-15h-18m-56s UTC integration manifest
8f6b3cf6104d drm/i915: Do not do link training fallback or prune modes for eDP

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5422/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Do not do link training fallback or prune modes for eDP
  2017-08-17  6:58 [PATCH] drm/i915: Do not do link training fallback or prune modes for eDP Manasi Navare
  2017-08-17  7:01 ` Rodrigo Vivi
  2017-08-17  7:21 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-08-17  9:51 ` Jani Nikula
  2017-08-17 19:29   ` Manasi Navare
  2 siblings, 1 reply; 15+ messages in thread
From: Jani Nikula @ 2017-08-17  9:51 UTC (permalink / raw)
  To: Manasi Navare, intel-gfx; +Cc: Daniel Vetter

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

>  {
>  	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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Do not do link training fallback or prune modes for eDP
  2017-08-17 19:29   ` Manasi Navare
@ 2017-08-17 19:28     ` Pandiyan, Dhinakaran
  2017-08-17 19:42       ` Manasi Navare
  0 siblings, 1 reply; 15+ messages in thread
From: Pandiyan, Dhinakaran @ 2017-08-17 19:28 UTC (permalink / raw)
  To: Navare, Manasi D; +Cc: Vetter, Daniel, intel-gfx

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() 

Btw I remember seeing Jim's psr patch making this function
global(non-static?) too.


> > >  {
> > >  	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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Do not do link training fallback or prune modes for eDP
  2017-08-17  9:51 ` [PATCH] " Jani Nikula
@ 2017-08-17 19:29   ` Manasi Navare
  2017-08-17 19:28     ` Pandiyan, Dhinakaran
  0 siblings, 1 reply; 15+ messages in thread
From: Manasi Navare @ 2017-08-17 19:29 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Daniel Vetter, intel-gfx

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

> >  {
> >  	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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Do not do link training fallback or prune modes for eDP
  2017-08-17 19:28     ` Pandiyan, Dhinakaran
@ 2017-08-17 19:42       ` Manasi Navare
  0 siblings, 0 replies; 15+ messages in thread
From: Manasi Navare @ 2017-08-17 19:42 UTC (permalink / raw)
  To: Pandiyan, Dhinakaran; +Cc: Vetter, Daniel, intel-gfx

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Do not do link training fallback or prune modes for eDP
  2017-08-17  7:01 ` Rodrigo Vivi
@ 2017-08-17 19:46   ` Manasi Navare
  2017-08-17 20:11     ` Pandiyan, Dhinakaran
  2017-08-18  8:08     ` Daniel Vetter
  0 siblings, 2 replies; 15+ messages in thread
From: Manasi Navare @ 2017-08-17 19:46 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: Daniel Vetter, intel-gfx

On Thu, Aug 17, 2017 at 07:01:05AM +0000, Rodrigo Vivi wrote:
>    On Wed, Aug 16, 2017 at 11:51 PM Manasi Navare
>    <[1]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.
> 
>    What about downclock modes?!

What are the downclock modes? We have seen an issue with pruning modes
in case of eDP panel where we end up pruning the mode due to link train
fallback and then we get an error saying "No modes available for the
connector"

> 
>      Also since its a panel, link training should not fail dynamically
>      based on cable conditions like in  case of DP.
> 
>    Is there any bug associated with this patch?

Yes this is the bug:
https://bugs.freedesktop.org/show_bug.cgi?id=101518
But the reason I didnt have this in the Fixes tab is because
this patch will not fix the bug, it will just isolate to it being
a bug with AUX Timeouts warning.

Regards
Manasi

> 
>      Cc: Jani Nikula <[2]jani.nikula@linux.intel.com>
>      Cc: Jim Bride <[3]jim.bride@linux.intel.com>
>      Cc: Ville Syrjälä <[4]ville.syrjala@linux.intel.com>
>      Cc: Daniel Vetter <[5]daniel.vetter@intel.com>
>      Signed-off-by: Manasi Navare <[6]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)
>       {
>              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->[7]base.base.id,
>                            intel_connector->[8]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,
>      --
>      2.1.4
>      _______________________________________________
>      Intel-gfx mailing list
>      [9]Intel-gfx@lists.freedesktop.org
>      [10]https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> References
> 
>    1. mailto:manasi.d.navare@intel.com
>    2. mailto:jani.nikula@linux.intel.com
>    3. mailto:jim.bride@linux.intel.com
>    4. mailto:ville.syrjala@linux.intel.com
>    5. mailto:daniel.vetter@intel.com
>    6. mailto:manasi.d.navare@intel.com
>    7. http://base.base.id/
>    8. http://base.name/
>    9. mailto:Intel-gfx@lists.freedesktop.org
>   10. 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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Do not do link training fallback or prune modes for eDP
  2017-08-17 19:46   ` Manasi Navare
@ 2017-08-17 20:11     ` Pandiyan, Dhinakaran
  2017-08-17 20:44       ` Rodrigo Vivi
  2017-08-17 21:01       ` Manasi Navare
  2017-08-18  8:08     ` Daniel Vetter
  1 sibling, 2 replies; 15+ messages in thread
From: Pandiyan, Dhinakaran @ 2017-08-17 20:11 UTC (permalink / raw)
  To: Navare, Manasi D; +Cc: intel-gfx, Vetter, Daniel

On Thu, 2017-08-17 at 12:46 -0700, Manasi Navare wrote:
> On Thu, Aug 17, 2017 at 07:01:05AM +0000, Rodrigo Vivi wrote:
> >    On Wed, Aug 16, 2017 at 11:51 PM Manasi Navare
> >    <[1]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.
> > 
> >    What about downclock modes?!
> 
> What are the downclock modes? We have seen an issue with pruning modes
> in case of eDP panel where we end up pruning the mode due to link train
> fallback and then we get an error saying "No modes available for the
> connector"
> 

fwiw I've got two laptops with eDP's having more than ten modes.

> > 
> >      Also since its a panel, link training should not fail dynamically
> >      based on cable conditions like in  case of DP.
> > 
> >    Is there any bug associated with this patch?
> 
> Yes this is the bug:
> https://bugs.freedesktop.org/show_bug.cgi?id=101518
> But the reason I didnt have this in the Fixes tab is because
> this patch will not fix the bug, it will just isolate to it being
> a bug with AUX Timeouts warning.
> 
> Regards
> Manasi
> 
> > 
> >      Cc: Jani Nikula <[2]jani.nikula@linux.intel.com>
> >      Cc: Jim Bride <[3]jim.bride@linux.intel.com>
> >      Cc: Ville Syrjälä <[4]ville.syrjala@linux.intel.com>
> >      Cc: Daniel Vetter <[5]daniel.vetter@intel.com>
> >      Signed-off-by: Manasi Navare <[6]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)
> >       {
> >              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->[7]base.base.id,
> >                            intel_connector->[8]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,
> >      --
> >      2.1.4
> >      _______________________________________________
> >      Intel-gfx mailing list
> >      [9]Intel-gfx@lists.freedesktop.org
> >      [10]https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > References
> > 
> >    1. mailto:manasi.d.navare@intel.com
> >    2. mailto:jani.nikula@linux.intel.com
> >    3. mailto:jim.bride@linux.intel.com
> >    4. mailto:ville.syrjala@linux.intel.com
> >    5. mailto:daniel.vetter@intel.com
> >    6. mailto:manasi.d.navare@intel.com
> >    7. http://base.base.id/
> >    8. http://base.name/
> >    9. mailto:Intel-gfx@lists.freedesktop.org
> >   10. 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Do not do link training fallback or prune modes for eDP
  2017-08-17 20:11     ` Pandiyan, Dhinakaran
@ 2017-08-17 20:44       ` Rodrigo Vivi
  2017-08-17 22:10         ` Manasi Navare
  2017-08-18  4:00         ` Dhinakaran Pandiyan
  2017-08-17 21:01       ` Manasi Navare
  1 sibling, 2 replies; 15+ messages in thread
From: Rodrigo Vivi @ 2017-08-17 20:44 UTC (permalink / raw)
  To: Pandiyan, Dhinakaran; +Cc: Vetter, Daniel, intel-gfx

On Thu, Aug 17, 2017 at 1:11 PM, Pandiyan, Dhinakaran
<dhinakaran.pandiyan@intel.com> wrote:
> On Thu, 2017-08-17 at 12:46 -0700, Manasi Navare wrote:
>> On Thu, Aug 17, 2017 at 07:01:05AM +0000, Rodrigo Vivi wrote:
>> >    On Wed, Aug 16, 2017 at 11:51 PM Manasi Navare
>> >    <[1]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.
>> >
>> >    What about downclock modes?!
>>
>> What are the downclock modes? We have seen an issue with pruning modes
>> in case of eDP panel where we end up pruning the mode due to link train
>> fallback and then we get an error saying "No modes available for the
>> connector"
>>
>
> fwiw I've got two laptops with eDP's having more than ten modes.

those ten are probably a list of modes with scaler right?!

Anyways, there are eDP panels that supports multiple modes like same
resolution but different clocks.
Like the ones with 60Hz and 48Hz where we cannot get PSR on 60 and can
get on 48 one.

This made Jim to do this patch:
https://patchwork.freedesktop.org/patch/msgid/1502308133-26892-1-git-send-email-jim.bride@linux.intel.com

that now is merged already and allow we select different modes on eDP
per request.

this case proves that there are eDP panels out there where this bug
wouldn't occur.

So I think we need to find a different way to handle this case where
there is no other mode
or fix the link status somehow....

>
>> >
>> >      Also since its a panel, link training should not fail dynamically
>> >      based on cable conditions like in  case of DP.
>> >
>> >    Is there any bug associated with this patch?
>>
>> Yes this is the bug:
>> https://bugs.freedesktop.org/show_bug.cgi?id=101518

Please add this for reference in a future patch or version:

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101518

>> But the reason I didnt have this in the Fixes tab is because
>> this patch will not fix the bug, it will just isolate to it being
>> a bug with AUX Timeouts warning.
>>
>> Regards
>> Manasi
>>
>> >
>> >      Cc: Jani Nikula <[2]jani.nikula@linux.intel.com>
>> >      Cc: Jim Bride <[3]jim.bride@linux.intel.com>
>> >      Cc: Ville Syrjälä <[4]ville.syrjala@linux.intel.com>
>> >      Cc: Daniel Vetter <[5]daniel.vetter@intel.com>
>> >      Signed-off-by: Manasi Navare <[6]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)
>> >       {
>> >              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->[7]base.base.id,
>> >                            intel_connector->[8]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,
>> >      --
>> >      2.1.4
>> >      _______________________________________________
>> >      Intel-gfx mailing list
>> >      [9]Intel-gfx@lists.freedesktop.org
>> >      [10]https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >
>> > References
>> >
>> >    1. mailto:manasi.d.navare@intel.com
>> >    2. mailto:jani.nikula@linux.intel.com
>> >    3. mailto:jim.bride@linux.intel.com
>> >    4. mailto:ville.syrjala@linux.intel.com
>> >    5. mailto:daniel.vetter@intel.com
>> >    6. mailto:manasi.d.navare@intel.com
>> >    7. http://base.base.id/
>> >    8. http://base.name/
>> >    9. mailto:Intel-gfx@lists.freedesktop.org
>> >   10. 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



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Do not do link training fallback or prune modes for eDP
  2017-08-17 20:11     ` Pandiyan, Dhinakaran
  2017-08-17 20:44       ` Rodrigo Vivi
@ 2017-08-17 21:01       ` Manasi Navare
  2017-08-17 21:16         ` Manasi Navare
  1 sibling, 1 reply; 15+ messages in thread
From: Manasi Navare @ 2017-08-17 21:01 UTC (permalink / raw)
  To: Pandiyan, Dhinakaran; +Cc: intel-gfx, Vetter, Daniel

On Thu, Aug 17, 2017 at 01:11:00PM -0700, Pandiyan, Dhinakaran wrote:
> On Thu, 2017-08-17 at 12:46 -0700, Manasi Navare wrote:
> > On Thu, Aug 17, 2017 at 07:01:05AM +0000, Rodrigo Vivi wrote:
> > >    On Wed, Aug 16, 2017 at 11:51 PM Manasi Navare
> > >    <[1]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.
> > > 
> > >    What about downclock modes?!
> > 
> > What are the downclock modes? We have seen an issue with pruning modes
> > in case of eDP panel where we end up pruning the mode due to link train
> > fallback and then we get an error saying "No modes available for the
> > connector"
> > 
> 
> fwiw I've got two laptops with eDP's having more than ten modes.
>

Yes but link training is not supposed to fail for eDP since its a
fixed connection and so after discussing with Daniel and Jani, this is
the consensus that we shouldnt be falling back and retraining in case
of eDP since most eDP panels are going to have 1 or 2 modes.

Manasi
 
> > > 
> > >      Also since its a panel, link training should not fail dynamically
> > >      based on cable conditions like in  case of DP.
> > > 
> > >    Is there any bug associated with this patch?
> > 
> > Yes this is the bug:
> > https://bugs.freedesktop.org/show_bug.cgi?id=101518
> > But the reason I didnt have this in the Fixes tab is because
> > this patch will not fix the bug, it will just isolate to it being
> > a bug with AUX Timeouts warning.
> > 
> > Regards
> > Manasi
> > 
> > > 
> > >      Cc: Jani Nikula <[2]jani.nikula@linux.intel.com>
> > >      Cc: Jim Bride <[3]jim.bride@linux.intel.com>
> > >      Cc: Ville Syrjälä <[4]ville.syrjala@linux.intel.com>
> > >      Cc: Daniel Vetter <[5]daniel.vetter@intel.com>
> > >      Signed-off-by: Manasi Navare <[6]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)
> > >       {
> > >              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->[7]base.base.id,
> > >                            intel_connector->[8]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,
> > >      --
> > >      2.1.4
> > >      _______________________________________________
> > >      Intel-gfx mailing list
> > >      [9]Intel-gfx@lists.freedesktop.org
> > >      [10]https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > 
> > > References
> > > 
> > >    1. mailto:manasi.d.navare@intel.com
> > >    2. mailto:jani.nikula@linux.intel.com
> > >    3. mailto:jim.bride@linux.intel.com
> > >    4. mailto:ville.syrjala@linux.intel.com
> > >    5. mailto:daniel.vetter@intel.com
> > >    6. mailto:manasi.d.navare@intel.com
> > >    7. http://base.base.id/
> > >    8. http://base.name/
> > >    9. mailto:Intel-gfx@lists.freedesktop.org
> > >   10. 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Do not do link training fallback or prune modes for eDP
  2017-08-17 21:01       ` Manasi Navare
@ 2017-08-17 21:16         ` Manasi Navare
  0 siblings, 0 replies; 15+ messages in thread
From: Manasi Navare @ 2017-08-17 21:16 UTC (permalink / raw)
  To: Pandiyan, Dhinakaran; +Cc: Vetter, Daniel, intel-gfx

On Thu, Aug 17, 2017 at 02:01:05PM -0700, Manasi Navare wrote:
> On Thu, Aug 17, 2017 at 01:11:00PM -0700, Pandiyan, Dhinakaran wrote:
> > On Thu, 2017-08-17 at 12:46 -0700, Manasi Navare wrote:
> > > On Thu, Aug 17, 2017 at 07:01:05AM +0000, Rodrigo Vivi wrote:
> > > >    On Wed, Aug 16, 2017 at 11:51 PM Manasi Navare
> > > >    <[1]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.
> > > > 
> > > >    What about downclock modes?!
> > > 
> > > What are the downclock modes? We have seen an issue with pruning modes
> > > in case of eDP panel where we end up pruning the mode due to link train
> > > fallback and then we get an error saying "No modes available for the
> > > connector"
> > > 
> > 
> > fwiw I've got two laptops with eDP's having more than ten modes.
> >

Hmm, I have always dealt with laptops with eDPs with a fixed mode
and so are the systems in CI which are giving this "no mode
left on the connector error".

Manasi

> 
> Yes but link training is not supposed to fail for eDP since its a
> fixed connection and so after discussing with Daniel and Jani, this is
> the consensus that we shouldnt be falling back and retraining in case
> of eDP since most eDP panels are going to have 1 or 2 modes.
> 
> Manasi
>  
> > > > 
> > > >      Also since its a panel, link training should not fail dynamically
> > > >      based on cable conditions like in  case of DP.
> > > > 
> > > >    Is there any bug associated with this patch?
> > > 
> > > Yes this is the bug:
> > > https://bugs.freedesktop.org/show_bug.cgi?id=101518
> > > But the reason I didnt have this in the Fixes tab is because
> > > this patch will not fix the bug, it will just isolate to it being
> > > a bug with AUX Timeouts warning.
> > > 
> > > Regards
> > > Manasi
> > > 
> > > > 
> > > >      Cc: Jani Nikula <[2]jani.nikula@linux.intel.com>
> > > >      Cc: Jim Bride <[3]jim.bride@linux.intel.com>
> > > >      Cc: Ville Syrjälä <[4]ville.syrjala@linux.intel.com>
> > > >      Cc: Daniel Vetter <[5]daniel.vetter@intel.com>
> > > >      Signed-off-by: Manasi Navare <[6]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)
> > > >       {
> > > >              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->[7]base.base.id,
> > > >                            intel_connector->[8]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,
> > > >      --
> > > >      2.1.4
> > > >      _______________________________________________
> > > >      Intel-gfx mailing list
> > > >      [9]Intel-gfx@lists.freedesktop.org
> > > >      [10]https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > 
> > > > References
> > > > 
> > > >    1. mailto:manasi.d.navare@intel.com
> > > >    2. mailto:jani.nikula@linux.intel.com
> > > >    3. mailto:jim.bride@linux.intel.com
> > > >    4. mailto:ville.syrjala@linux.intel.com
> > > >    5. mailto:daniel.vetter@intel.com
> > > >    6. mailto:manasi.d.navare@intel.com
> > > >    7. http://base.base.id/
> > > >    8. http://base.name/
> > > >    9. mailto:Intel-gfx@lists.freedesktop.org
> > > >   10. 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
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Do not do link training fallback or prune modes for eDP
  2017-08-17 20:44       ` Rodrigo Vivi
@ 2017-08-17 22:10         ` Manasi Navare
  2017-08-18  4:00         ` Dhinakaran Pandiyan
  1 sibling, 0 replies; 15+ messages in thread
From: Manasi Navare @ 2017-08-17 22:10 UTC (permalink / raw)
  To: Rodrigo Vivi, jani.nikula, ville.syrjala
  Cc: Vetter, Daniel, intel-gfx, Pandiyan, Dhinakaran

On Thu, Aug 17, 2017 at 01:44:14PM -0700, Rodrigo Vivi wrote:
> On Thu, Aug 17, 2017 at 1:11 PM, Pandiyan, Dhinakaran
> <dhinakaran.pandiyan@intel.com> wrote:
> > On Thu, 2017-08-17 at 12:46 -0700, Manasi Navare wrote:
> >> On Thu, Aug 17, 2017 at 07:01:05AM +0000, Rodrigo Vivi wrote:
> >> >    On Wed, Aug 16, 2017 at 11:51 PM Manasi Navare
> >> >    <[1]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.
> >> >
> >> >    What about downclock modes?!
> >>
> >> What are the downclock modes? We have seen an issue with pruning modes
> >> in case of eDP panel where we end up pruning the mode due to link train
> >> fallback and then we get an error saying "No modes available for the
> >> connector"
> >>
> >
> > fwiw I've got two laptops with eDP's having more than ten modes.
> 
> those ten are probably a list of modes with scaler right?!
> 
> Anyways, there are eDP panels that supports multiple modes like same
> resolution but different clocks.
> Like the ones with 60Hz and 48Hz where we cannot get PSR on 60 and can
> get on 48 one.
> 
> This made Jim to do this patch:
> https://patchwork.freedesktop.org/patch/msgid/1502308133-26892-1-git-send-email-jim.bride@linux.intel.com
> 
> that now is merged already and allow we select different modes on eDP
> per request.
> 
> this case proves that there are eDP panels out there where this bug
> wouldn't occur.
> 
> So I think we need to find a different way to handle this case where
> there is no other mode
> or fix the link status somehow....
>

Thanks Rodrigo for this explanation. Yes those modes advertised to the
userspace would be the modes with scalar. But the HW would still only
support the fixed mode ot the alternate DRRS mode. Infact in case of link training
fallback, when the userspace triggers a new modeset, it will call mode_valid() hook
where if the requested mode's hdisplay or vdisplay are greater than the fixed mode's
hdisplay or vdisplay then it returns error MODE_PANEL instead of MODE_OK.
Also Jim brought up a point that for some eDP panels only 2 lanes could be wired in
which case we do want link train fallback in order to retry modeset with fewer lanes.
So we need fallback I suppose but handle this no mode situation differently.

Daniel, Jani any thoughts on how this could be handled?

Regards
Manasi

> >
> >> >
> >> >      Also since its a panel, link training should not fail dynamically
> >> >      based on cable conditions like in  case of DP.
> >> >
> >> >    Is there any bug associated with this patch?
> >>
> >> Yes this is the bug:
> >> https://bugs.freedesktop.org/show_bug.cgi?id=101518
> 
> Please add this for reference in a future patch or version:
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101518
> 
> >> But the reason I didnt have this in the Fixes tab is because
> >> this patch will not fix the bug, it will just isolate to it being
> >> a bug with AUX Timeouts warning.
> >>
> >> Regards
> >> Manasi
> >>
> >> >
> >> >      Cc: Jani Nikula <[2]jani.nikula@linux.intel.com>
> >> >      Cc: Jim Bride <[3]jim.bride@linux.intel.com>
> >> >      Cc: Ville Syrjälä <[4]ville.syrjala@linux.intel.com>
> >> >      Cc: Daniel Vetter <[5]daniel.vetter@intel.com>
> >> >      Signed-off-by: Manasi Navare <[6]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)
> >> >       {
> >> >              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->[7]base.base.id,
> >> >                            intel_connector->[8]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,
> >> >      --
> >> >      2.1.4
> >> >      _______________________________________________
> >> >      Intel-gfx mailing list
> >> >      [9]Intel-gfx@lists.freedesktop.org
> >> >      [10]https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> >
> >> > References
> >> >
> >> >    1. mailto:manasi.d.navare@intel.com
> >> >    2. mailto:jani.nikula@linux.intel.com
> >> >    3. mailto:jim.bride@linux.intel.com
> >> >    4. mailto:ville.syrjala@linux.intel.com
> >> >    5. mailto:daniel.vetter@intel.com
> >> >    6. mailto:manasi.d.navare@intel.com
> >> >    7. http://base.base.id/
> >> >    8. http://base.name/
> >> >    9. mailto:Intel-gfx@lists.freedesktop.org
> >> >   10. 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
> 
> 
> 
> -- 
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Do not do link training fallback or prune modes for eDP
  2017-08-17 20:44       ` Rodrigo Vivi
  2017-08-17 22:10         ` Manasi Navare
@ 2017-08-18  4:00         ` Dhinakaran Pandiyan
  1 sibling, 0 replies; 15+ messages in thread
From: Dhinakaran Pandiyan @ 2017-08-18  4:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: Pandiyan, Dhinakaran, Vetter, Daniel

On Thursday, August 17, 2017 1:44:14 PM PDT Rodrigo Vivi wrote:
> On Thu, Aug 17, 2017 at 1:11 PM, Pandiyan, Dhinakaran
> 
> <dhinakaran.pandiyan@intel.com> wrote:
> > On Thu, 2017-08-17 at 12:46 -0700, Manasi Navare wrote:
> >> On Thu, Aug 17, 2017 at 07:01:05AM +0000, Rodrigo Vivi wrote:
> >> >    On Wed, Aug 16, 2017 at 11:51 PM Manasi Navare
> >> >    
> >> >    <[1]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.
> >> >    
> >> >    What about downclock modes?!
> >> 
> >> What are the downclock modes? We have seen an issue with pruning modes
> >> in case of eDP panel where we end up pruning the mode due to link train
> >> fallback and then we get an error saying "No modes available for the
> >> connector"
> > 
> > fwiw I've got two laptops with eDP's having more than ten modes.
> 
> those ten are probably a list of modes with scaler right?!

Yeah, Clint very graciously explained that to me after I sent the email.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Do not do link training fallback or prune modes for eDP
  2017-08-17 19:46   ` Manasi Navare
  2017-08-17 20:11     ` Pandiyan, Dhinakaran
@ 2017-08-18  8:08     ` Daniel Vetter
  1 sibling, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2017-08-18  8:08 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx, Daniel Vetter

On Thu, Aug 17, 2017 at 12:46:53PM -0700, Manasi Navare wrote:
> On Thu, Aug 17, 2017 at 07:01:05AM +0000, Rodrigo Vivi wrote:
> >    On Wed, Aug 16, 2017 at 11:51 PM Manasi Navare
> >    <[1]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.
> > 
> >    What about downclock modes?!
> 
> What are the downclock modes? We have seen an issue with pruning modes
> in case of eDP panel where we end up pruning the mode due to link train
> fallback and then we get an error saying "No modes available for the
> connector"
> 
> > 
> >      Also since its a panel, link training should not fail dynamically
> >      based on cable conditions like in  case of DP.
> > 
> >    Is there any bug associated with this patch?
> 
> Yes this is the bug:
> https://bugs.freedesktop.org/show_bug.cgi?id=101518
> But the reason I didnt have this in the Fixes tab is because
> this patch will not fix the bug, it will just isolate to it being
> a bug with AUX Timeouts warning.

We use References: for that. Fixes: is for when you fix a bug in a
previous patch, and since this fixes a regression you need to add a Fixes:
line that references your original link training failure fallback.
-Daniel


> 
> Regards
> Manasi
> 
> > 
> >      Cc: Jani Nikula <[2]jani.nikula@linux.intel.com>
> >      Cc: Jim Bride <[3]jim.bride@linux.intel.com>
> >      Cc: Ville Syrjälä <[4]ville.syrjala@linux.intel.com>
> >      Cc: Daniel Vetter <[5]daniel.vetter@intel.com>
> >      Signed-off-by: Manasi Navare <[6]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)
> >       {
> >              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->[7]base.base.id,
> >                            intel_connector->[8]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,
> >      --
> >      2.1.4
> >      _______________________________________________
> >      Intel-gfx mailing list
> >      [9]Intel-gfx@lists.freedesktop.org
> >      [10]https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > References
> > 
> >    1. mailto:manasi.d.navare@intel.com
> >    2. mailto:jani.nikula@linux.intel.com
> >    3. mailto:jim.bride@linux.intel.com
> >    4. mailto:ville.syrjala@linux.intel.com
> >    5. mailto:daniel.vetter@intel.com
> >    6. mailto:manasi.d.navare@intel.com
> >    7. http://base.base.id/
> >    8. http://base.name/
> >    9. mailto:Intel-gfx@lists.freedesktop.org
> >   10. 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

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2017-08-18  8:08 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-17  6:58 [PATCH] drm/i915: Do not do link training fallback or prune modes for eDP Manasi Navare
2017-08-17  7:01 ` Rodrigo Vivi
2017-08-17 19:46   ` Manasi Navare
2017-08-17 20:11     ` Pandiyan, Dhinakaran
2017-08-17 20:44       ` Rodrigo Vivi
2017-08-17 22:10         ` Manasi Navare
2017-08-18  4:00         ` Dhinakaran Pandiyan
2017-08-17 21:01       ` Manasi Navare
2017-08-17 21:16         ` Manasi Navare
2017-08-18  8:08     ` Daniel Vetter
2017-08-17  7:21 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-08-17  9:51 ` [PATCH] " Jani Nikula
2017-08-17 19:29   ` Manasi Navare
2017-08-17 19:28     ` Pandiyan, Dhinakaran
2017-08-17 19:42       ` Manasi Navare

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.