Re: [Intel-gfx] [PATCH 0/5] Handle link training failure during modeset

2016-11-17 Thread Manasi Navare 
On Thu, Nov 17, 2016 at 02:29:30PM +0200, Jani Nikula wrote:
> On Tue, 15 Nov 2016, Manasi Navare  wrote:
> > Submitting new series that adds proper commit messages/cover letter
> > and kernel documentation. It also moved the set_link_status function
> > to drm core so other kernel drivers can make use of it.
> >
> > The idea presented in these patches is to address link training failure
> > in a way that:
> > a) changes the current happy day scenario as little as possible, to avoid
> > regressions, b) can be implemented the same way by all drm drivers, c)
> > is still opt-in for the drivers and userspace, and opting out doesn't
> > regress the user experience, d) doesn't prevent drivers from
> > implementing better or alternate approaches, possibly without userspace
> > involvement. And, of course, handles all the issues presented.
> >
> > The solution is to add a "link status" connector property. In the usual
> > happy day scenario, this is always "good". If something fails during or
> > after a mode set, the kernel driver can set the link status to "bad",
> > prune the mode list based on new information as necessary, and send a
> > hotplug uevent for userspace to have it re-check the valid modes through
> > getconnector, and try again. If the theoretical capabilities of the link
> > can't be reached, the mode list is trimmed based on that.
> >
> > If the userspace is not aware of the property, the user experience is
> > the same as it currently is. If the userspace is aware of the property,
> > it has a chance to improve user experience. If a drm driver does not
> > modify the property (it stays "good"), the user experience is the same
> > as it currently is. A drm driver can also choose to try to handle more
> > of the failures in kernel, hardware not limiting, or it can choose to
> > involve userspace more. Up to the drivers.
> >
> > The reason for adding the property is to handle link training failures,
> > but it is not limited to DP or link training. For example, if we
> > implement asynchronous setcrtc, we can use this to report any failures
> > in that.
> >
> > Finally, while DP CTS compliance is advertized (which is great, and
> > could be made to work similarly for all drm drivers), this can be used
> > for the more important goal of improving user experience on link
> > training failures, by avoiding black screens.
> 
> Since I went through the trouble of writing this, you might as well add
> it to patch 1/5 commit message so it benefits the posterity.
>

Yes, I have also added most of this to the kernel documentation
for the link status property. But I will also add this to Patch 1/5 commit 
message.
Should we explain the alternative approaches as well over there?

Manasi

 
> > Acked-by: Tony Cheng 
> > Acked-by: Harry Wentland 
> 
> These must go to patch 1/5 commit message.
> 
> BR,
> Jani.
> 
> 
> 
> >
> > Manasi Navare (5):
> >   drm: Add a new connector property for link status
> >   drm: Set DRM connector link status property
> >   drm/i915: Update CRTC state if connector link status property changed
> >   drm/i915: Find fallback link rate/lane count
> >   drm/i915: Implement Link Rate fallback on Link training failure
> >
> >  drivers/gpu/drm/drm_atomic_helper.c   |   7 ++
> >  drivers/gpu/drm/drm_connector.c   |  55 ++
> >  drivers/gpu/drm/i915/intel_ddi.c  |  21 +++-
> >  drivers/gpu/drm/i915/intel_dp.c   | 144 
> > +-
> >  drivers/gpu/drm/i915/intel_dp_link_training.c |  12 ++-
> >  drivers/gpu/drm/i915/intel_drv.h  |  10 +-
> >  include/drm/drm_connector.h   |   9 +-
> >  include/drm/drm_crtc.h|   5 +
> >  include/uapi/drm/drm_mode.h   |   4 +
> >  9 files changed, 257 insertions(+), 10 deletions(-)
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 0/5] Handle link training failure during modeset

2016-11-17 Thread Jani Nikula 
On Tue, 15 Nov 2016, Manasi Navare  wrote:
> Submitting new series that adds proper commit messages/cover letter
> and kernel documentation. It also moved the set_link_status function
> to drm core so other kernel drivers can make use of it.
>
> The idea presented in these patches is to address link training failure
> in a way that:
> a) changes the current happy day scenario as little as possible, to avoid
> regressions, b) can be implemented the same way by all drm drivers, c)
> is still opt-in for the drivers and userspace, and opting out doesn't
> regress the user experience, d) doesn't prevent drivers from
> implementing better or alternate approaches, possibly without userspace
> involvement. And, of course, handles all the issues presented.
>
> The solution is to add a "link status" connector property. In the usual
> happy day scenario, this is always "good". If something fails during or
> after a mode set, the kernel driver can set the link status to "bad",
> prune the mode list based on new information as necessary, and send a
> hotplug uevent for userspace to have it re-check the valid modes through
> getconnector, and try again. If the theoretical capabilities of the link
> can't be reached, the mode list is trimmed based on that.
>
> If the userspace is not aware of the property, the user experience is
> the same as it currently is. If the userspace is aware of the property,
> it has a chance to improve user experience. If a drm driver does not
> modify the property (it stays "good"), the user experience is the same
> as it currently is. A drm driver can also choose to try to handle more
> of the failures in kernel, hardware not limiting, or it can choose to
> involve userspace more. Up to the drivers.
>
> The reason for adding the property is to handle link training failures,
> but it is not limited to DP or link training. For example, if we
> implement asynchronous setcrtc, we can use this to report any failures
> in that.
>
> Finally, while DP CTS compliance is advertized (which is great, and
> could be made to work similarly for all drm drivers), this can be used
> for the more important goal of improving user experience on link
> training failures, by avoiding black screens.

Since I went through the trouble of writing this, you might as well add
it to patch 1/5 commit message so it benefits the posterity.

> Acked-by: Tony Cheng 
> Acked-by: Harry Wentland 

These must go to patch 1/5 commit message.

BR,
Jani.



>
> Manasi Navare (5):
>   drm: Add a new connector property for link status
>   drm: Set DRM connector link status property
>   drm/i915: Update CRTC state if connector link status property changed
>   drm/i915: Find fallback link rate/lane count
>   drm/i915: Implement Link Rate fallback on Link training failure
>
>  drivers/gpu/drm/drm_atomic_helper.c   |   7 ++
>  drivers/gpu/drm/drm_connector.c   |  55 ++
>  drivers/gpu/drm/i915/intel_ddi.c  |  21 +++-
>  drivers/gpu/drm/i915/intel_dp.c   | 144 
> +-
>  drivers/gpu/drm/i915/intel_dp_link_training.c |  12 ++-
>  drivers/gpu/drm/i915/intel_drv.h  |  10 +-
>  include/drm/drm_connector.h   |   9 +-
>  include/drm/drm_crtc.h|   5 +
>  include/uapi/drm/drm_mode.h   |   4 +
>  9 files changed, 257 insertions(+), 10 deletions(-)

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure during modeset

2016-11-14 Thread Harry Wentland 
Overall this patch set makes sense but I think it would be good to move 
some stuff to common code instead of leaving it in i915. I'm thinking 
about patch 2 (setting the link status property) in particular but also 
tend to agree with Ville and Daniel about their comments for patch 5.


That said, should another driver need it it shouldn't be hard to pull 
that out.


Patch 1: Reviewed-by: Harry Wentland 

Patch 2-5: Acked-by: Harry Wentland 

Harry


On 2016-11-09 11:42 PM, Manasi Navare wrote:

Link training failure is handled by lowering the link rate first
until it reaches the minimum and keeping the lane count maximum
and then lowering the lane count until it reaches minimim. These
fallback values are saved and hotplug uevent is sent to the userspace
after setting the connector link status property to BAD. Userspace
should triiger another modeset on a uevent and if link status property
is BAD. This will retrain the link at fallback values.
This is repeated until the link is successfully trained.

This has been validated to pass DP compliance.

Manasi Navare (5):
   drm: Add a new connector property for link status
   drm/i915: Set link status property for DP connector
   drm/i915: Update CRTC state if connector link status property changed
   drm/i915: Find fallback link rate/lane count
   drm/i915: Implement Link Rate fallback on Link training failure

  drivers/gpu/drm/drm_atomic_helper.c   |   7 ++
  drivers/gpu/drm/drm_connector.c   |  17 
  drivers/gpu/drm/i915/intel_ddi.c  |  21 +++-
  drivers/gpu/drm/i915/intel_dp.c   | 138 +-
  drivers/gpu/drm/i915/intel_dp_link_training.c |  12 ++-
  drivers/gpu/drm/i915/intel_drv.h  |  12 ++-
  include/drm/drm_connector.h   |   7 +-
  include/drm/drm_crtc.h|   5 +
  include/uapi/drm/drm_mode.h   |   4 +
  9 files changed, 214 insertions(+), 9 deletions(-)



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

[Intel-gfx] [PATCH 0/5] Handle link training failure during modeset

2016-11-14 Thread Manasi Navare 
Submitting new series that adds proper commit messages/cover letter
and kernel documentation. It also moved the set_link_status function
to drm core so other kernel drivers can make use of it.

The idea presented in these patches is to address link training failure
in a way that:
a) changes the current happy day scenario as little as possible, to avoid
regressions, b) can be implemented the same way by all drm drivers, c)
is still opt-in for the drivers and userspace, and opting out doesn't
regress the user experience, d) doesn't prevent drivers from
implementing better or alternate approaches, possibly without userspace
involvement. And, of course, handles all the issues presented.

The solution is to add a "link status" connector property. In the usual
happy day scenario, this is always "good". If something fails during or
after a mode set, the kernel driver can set the link status to "bad",
prune the mode list based on new information as necessary, and send a
hotplug uevent for userspace to have it re-check the valid modes through
getconnector, and try again. If the theoretical capabilities of the link
can't be reached, the mode list is trimmed based on that.

If the userspace is not aware of the property, the user experience is
the same as it currently is. If the userspace is aware of the property,
it has a chance to improve user experience. If a drm driver does not
modify the property (it stays "good"), the user experience is the same
as it currently is. A drm driver can also choose to try to handle more
of the failures in kernel, hardware not limiting, or it can choose to
involve userspace more. Up to the drivers.

The reason for adding the property is to handle link training failures,
but it is not limited to DP or link training. For example, if we
implement asynchronous setcrtc, we can use this to report any failures
in that.

Finally, while DP CTS compliance is advertized (which is great, and
could be made to work similarly for all drm drivers), this can be used
for the more important goal of improving user experience on link
training failures, by avoiding black screens.

Acked-by: Tony Cheng 
Acked-by: Harry Wentland 

Manasi Navare (5):
  drm: Add a new connector property for link status
  drm: Set DRM connector link status property
  drm/i915: Update CRTC state if connector link status property changed
  drm/i915: Find fallback link rate/lane count
  drm/i915: Implement Link Rate fallback on Link training failure

 drivers/gpu/drm/drm_atomic_helper.c   |   7 ++
 drivers/gpu/drm/drm_connector.c   |  55 ++
 drivers/gpu/drm/i915/intel_ddi.c  |  21 +++-
 drivers/gpu/drm/i915/intel_dp.c   | 144 +-
 drivers/gpu/drm/i915/intel_dp_link_training.c |  12 ++-
 drivers/gpu/drm/i915/intel_drv.h  |  10 +-
 include/drm/drm_connector.h   |   9 +-
 include/drm/drm_crtc.h|   5 +
 include/uapi/drm/drm_mode.h   |   4 +
 9 files changed, 257 insertions(+), 10 deletions(-)

-- 
1.9.1

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

Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure during modeset

2016-11-14 Thread Manasi Navare 
On Mon, Nov 14, 2016 at 08:07:39PM -0500, Harry Wentland wrote:
> Overall this patch set makes sense but I think it would be good to
> move some stuff to common code instead of leaving it in i915. I'm
> thinking about patch 2 (setting the link status property) in
> particular but also tend to agree with Ville and Daniel about their
> comments for patch 5.
> 
> That said, should another driver need it it shouldn't be hard to
> pull that out.
> 
> Patch 1: Reviewed-by: Harry Wentland 
> 
> Patch 2-5: Acked-by: Harry Wentland 
> 
> Harry
> 
>

Thanks for the review.
I am already moving Patch 2 set_link_status to the drm core as
per Daniel and Ville's suggestion. I will make those changes
and submit the new patches by EOD today.

Regards
Manasi
 
> On 2016-11-09 11:42 PM, Manasi Navare wrote:
> >Link training failure is handled by lowering the link rate first
> >until it reaches the minimum and keeping the lane count maximum
> >and then lowering the lane count until it reaches minimim. These
> >fallback values are saved and hotplug uevent is sent to the userspace
> >after setting the connector link status property to BAD. Userspace
> >should triiger another modeset on a uevent and if link status property
> >is BAD. This will retrain the link at fallback values.
> >This is repeated until the link is successfully trained.
> >
> >This has been validated to pass DP compliance.
> >
> >Manasi Navare (5):
> >   drm: Add a new connector property for link status
> >   drm/i915: Set link status property for DP connector
> >   drm/i915: Update CRTC state if connector link status property changed
> >   drm/i915: Find fallback link rate/lane count
> >   drm/i915: Implement Link Rate fallback on Link training failure
> >
> >  drivers/gpu/drm/drm_atomic_helper.c   |   7 ++
> >  drivers/gpu/drm/drm_connector.c   |  17 
> >  drivers/gpu/drm/i915/intel_ddi.c  |  21 +++-
> >  drivers/gpu/drm/i915/intel_dp.c   | 138 
> > +-
> >  drivers/gpu/drm/i915/intel_dp_link_training.c |  12 ++-
> >  drivers/gpu/drm/i915/intel_drv.h  |  12 ++-
> >  include/drm/drm_connector.h   |   7 +-
> >  include/drm/drm_crtc.h|   5 +
> >  include/uapi/drm/drm_mode.h   |   4 +
> >  9 files changed, 214 insertions(+), 9 deletions(-)
> >
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure during modeset

2016-11-14 Thread Manasi Navare 
Yes driver can chose to have the pre train for kernel hiding unsupported mode, 
that is completely still possible for the amd driver and it would never use the 
link_status connector property and no interaction with userspace.
Could you please give your ACKs for the patches if you are okay with this 
porposal?

Regards
Manasi



On Mon, Nov 14, 2016 at 02:45:55PM +, Cheng, Tony wrote:
> I see.  As long as amd can still just have kernel hiding unsupported mode by 
> doing a pre-train I am okay with the proposal.  Just to caution you the link 
> training fallback we implemented on windows in old dal architecture is very 
> painful to get right.  Windows 7, 8.1 and 10 all behave differently.  Not all 
> apps handles mode change failure gracefully and worse case we get stuck in 
> infinite mode set.  This is why for future generations asic on windows we are 
> also going to just hide unsupported mode by doing pre-train.
> 
> -Original Message-
> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Monday, November 14, 2016 3:04 AM
> To: Manasi Navare <manasi.d.nav...@intel.com>
> Cc: Cheng, Tony <tony.ch...@amd.com>; intel-gfx@lists.freedesktop.org; Peres, 
> Martin <martin.pe...@intel.com>; dri-de...@lists.freedesktop.org; Deucher, 
> Alexander <alexander.deuc...@amd.com>; Wentland, Harry 
> <harry.wentl...@amd.com>
> Subject: Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure during 
> modeset
> 
> On Sun, Nov 13, 2016 at 11:43:01PM -0800, Manasi Navare wrote:
> > On Fri, Nov 11, 2016 at 07:42:16PM +, Cheng, Tony wrote:
> > > In case of link training failure and requiring user space to set a lower 
> > > mode, would full mode set address it?  How do we make user mode select 
> > > lower resolution?
> > > 
> > > For example 4k@60Hz monitor, and link training at 4 lane HBR2 fails and 
> > > fallback to 4 lanes HBR, 4k@60 is no longer doable.  I would think 
> > > preventing user mode from seeing 4K@60Hz from the start is a easier and 
> > > more robust solution and requiring user mode to have logic to decide how 
> > > to fallback.
> > >
> > 
> > Hi Tony,
> > 
> > So in case of link training failure, we first call mode_valid that 
> > will use the lower link rate/lane count values based on the link 
> > training fallback to validate the modes and prune the modes that are 
> > higher than the available link. After this is done, then we send the uevent 
> > to userspace indicating that link status is BAD and it will redo a probe 
> > and trigger a modeset for next possible lower resolution.
> 
> Just in case it's not clear: This entire discussion here is only about the 
> userspace-visible abi changes. And I think for the worst-case we need 
> something like this (and Harry at least said on irc that on other os you do 
> something similar already). It of course does not preclude at all that you do 
> additional link-training/probe on initial hotplug. That part is entirely up 
> to drivers (and we might get there on some platforms, but on others it's a 
> real pain to get a link up unfortunately for training, since we need 
> to steal a crtc and do a full modeset).
> -Daniel
> 
> > 
> > Manasi
> > 
> >  
> > > -Original Message-
> > > From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> > > Sent: Friday, November 11, 2016 11:44 AM
> > > To: Cheng, Tony <tony.ch...@amd.com>
> > > Cc: Deucher, Alexander <alexander.deuc...@amd.com>; 'Jani Nikula' 
> > > <jani.nik...@linux.intel.com>; Manasi Navare 
> > > <manasi.d.nav...@intel.com>; dri-de...@lists.freedesktop.org; 
> > > intel-gfx@lists.freedesktop.org; Wentland, Harry 
> > > <harry.wentl...@amd.com>; Peres, Martin <martin.pe...@intel.com>
> > > Subject: Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure 
> > > during modeset
> > > 
> > > On Fri, Nov 11, 2016 at 04:21:58PM +, Cheng, Tony wrote:
> > > > For HDMI, you can yank the cable, plug back in, HDMI will light up 
> > > > without user mode or kernel mode doing anything.
> > > > 
> > > > For DP this is not possible, someone will have to retrain the link when 
> > > > plugging back in or DP will not light up.  We see that on Ubuntu if 
> > > > someone unplug display and plug it back into same connector, we do not 
> > > > get KMS so in this case.  It appears there is some optimizations 
> > > > somewhere in user mode stack which I am not familiar with yet.  dal 
> > > > added work

Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure during modeset

2016-11-14 Thread Cheng, Tony 
I see.  As long as amd can still just have kernel hiding unsupported mode by 
doing a pre-train I am okay with the proposal.  Just to caution you the link 
training fallback we implemented on windows in old dal architecture is very 
painful to get right.  Windows 7, 8.1 and 10 all behave differently.  Not all 
apps handles mode change failure gracefully and worse case we get stuck in 
infinite mode set.  This is why for future generations asic on windows we are 
also going to just hide unsupported mode by doing pre-train.

-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Monday, November 14, 2016 3:04 AM
To: Manasi Navare <manasi.d.nav...@intel.com>
Cc: Cheng, Tony <tony.ch...@amd.com>; intel-gfx@lists.freedesktop.org; Peres, 
Martin <martin.pe...@intel.com>; dri-de...@lists.freedesktop.org; Deucher, 
Alexander <alexander.deuc...@amd.com>; Wentland, Harry <harry.wentl...@amd.com>
Subject: Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure during modeset

On Sun, Nov 13, 2016 at 11:43:01PM -0800, Manasi Navare wrote:
> On Fri, Nov 11, 2016 at 07:42:16PM +, Cheng, Tony wrote:
> > In case of link training failure and requiring user space to set a lower 
> > mode, would full mode set address it?  How do we make user mode select 
> > lower resolution?
> > 
> > For example 4k@60Hz monitor, and link training at 4 lane HBR2 fails and 
> > fallback to 4 lanes HBR, 4k@60 is no longer doable.  I would think 
> > preventing user mode from seeing 4K@60Hz from the start is a easier and 
> > more robust solution and requiring user mode to have logic to decide how to 
> > fallback.
> >
> 
> Hi Tony,
> 
> So in case of link training failure, we first call mode_valid that 
> will use the lower link rate/lane count values based on the link 
> training fallback to validate the modes and prune the modes that are 
> higher than the available link. After this is done, then we send the uevent 
> to userspace indicating that link status is BAD and it will redo a probe and 
> trigger a modeset for next possible lower resolution.

Just in case it's not clear: This entire discussion here is only about the 
userspace-visible abi changes. And I think for the worst-case we need something 
like this (and Harry at least said on irc that on other os you do something 
similar already). It of course does not preclude at all that you do additional 
link-training/probe on initial hotplug. That part is entirely up to drivers 
(and we might get there on some platforms, but on others it's a real pain to 
get a link up unfortunately for training, since we need to steal a crtc 
and do a full modeset).
-Daniel

> 
> Manasi
> 
>  
> > -Original Message-
> > From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> > Sent: Friday, November 11, 2016 11:44 AM
> > To: Cheng, Tony <tony.ch...@amd.com>
> > Cc: Deucher, Alexander <alexander.deuc...@amd.com>; 'Jani Nikula' 
> > <jani.nik...@linux.intel.com>; Manasi Navare 
> > <manasi.d.nav...@intel.com>; dri-de...@lists.freedesktop.org; 
> > intel-gfx@lists.freedesktop.org; Wentland, Harry 
> > <harry.wentl...@amd.com>; Peres, Martin <martin.pe...@intel.com>
> > Subject: Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure 
> > during modeset
> > 
> > On Fri, Nov 11, 2016 at 04:21:58PM +, Cheng, Tony wrote:
> > > For HDMI, you can yank the cable, plug back in, HDMI will light up 
> > > without user mode or kernel mode doing anything.
> > > 
> > > For DP this is not possible, someone will have to retrain the link when 
> > > plugging back in or DP will not light up.  We see that on Ubuntu if 
> > > someone unplug display and plug it back into same connector, we do not 
> > > get KMS so in this case.  It appears there is some optimizations 
> > > somewhere in user mode stack which I am not familiar with yet.  dal added 
> > > workaround to retrain the link to light it back up (after the test 
> > > training at end of detection).
> > 
> > The link should get retrained as the driver should check the link state on 
> > HPD and retrain if it's not good. At least that's what we do in i915. Of 
> > course if it's not the same display that got plugged back, the retraining 
> > might fail. The new property can help with that case as userspace would 
> > then attempt a full modeset after the failed link training.
> > 
> > > 
> > > >From user mode perspective I think it make sense to keep CRTC running, 
> > > >so vblank is still going so UMD isn't impacted.   As regard to connector 
> > > >and encoder does it

Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure during modeset

2016-11-14 Thread Daniel Vetter 
On Sun, Nov 13, 2016 at 11:43:01PM -0800, Manasi Navare wrote:
> On Fri, Nov 11, 2016 at 07:42:16PM +, Cheng, Tony wrote:
> > In case of link training failure and requiring user space to set a lower 
> > mode, would full mode set address it?  How do we make user mode select 
> > lower resolution?
> > 
> > For example 4k@60Hz monitor, and link training at 4 lane HBR2 fails and 
> > fallback to 4 lanes HBR, 4k@60 is no longer doable.  I would think 
> > preventing user mode from seeing 4K@60Hz from the start is a easier and 
> > more robust solution and requiring user mode to have logic to decide how to 
> > fallback.
> >
> 
> Hi Tony,
> 
> So in case of link training failure, we first call mode_valid that will use 
> the lower link rate/lane count
> values based on the link training fallback to validate the modes and prune 
> the modes that are higher than
> the available link. After this is done, then we send the uevent to userspace 
> indicating that link status is BAD and 
> it will redo a probe and trigger a modeset for next possible lower resolution.

Just in case it's not clear: This entire discussion here is only about the
userspace-visible abi changes. And I think for the worst-case we need
something like this (and Harry at least said on irc that on other os you
do something similar already). It of course does not preclude at all that
you do additional link-training/probe on initial hotplug. That part is
entirely up to drivers (and we might get there on some platforms, but on
others it's a real pain to get a link up unfortunately for
training, since we need to steal a crtc and do a full modeset).
-Daniel

> 
> Manasi
> 
>  
> > -Original Message-
> > From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] 
> > Sent: Friday, November 11, 2016 11:44 AM
> > To: Cheng, Tony <tony.ch...@amd.com>
> > Cc: Deucher, Alexander <alexander.deuc...@amd.com>; 'Jani Nikula' 
> > <jani.nik...@linux.intel.com>; Manasi Navare <manasi.d.nav...@intel.com>; 
> > dri-de...@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Wentland, 
> > Harry <harry.wentl...@amd.com>; Peres, Martin <martin.pe...@intel.com>
> > Subject: Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure during 
> > modeset
> > 
> > On Fri, Nov 11, 2016 at 04:21:58PM +, Cheng, Tony wrote:
> > > For HDMI, you can yank the cable, plug back in, HDMI will light up 
> > > without user mode or kernel mode doing anything.
> > > 
> > > For DP this is not possible, someone will have to retrain the link when 
> > > plugging back in or DP will not light up.  We see that on Ubuntu if 
> > > someone unplug display and plug it back into same connector, we do not 
> > > get KMS so in this case.  It appears there is some optimizations 
> > > somewhere in user mode stack which I am not familiar with yet.  dal added 
> > > workaround to retrain the link to light it back up (after the test 
> > > training at end of detection).
> > 
> > The link should get retrained as the driver should check the link state on 
> > HPD and retrain if it's not good. At least that's what we do in i915. Of 
> > course if it's not the same display that got plugged back, the retraining 
> > might fail. The new property can help with that case as userspace would 
> > then attempt a full modeset after the failed link training.
> > 
> > > 
> > > >From user mode perspective I think it make sense to keep CRTC running, 
> > > >so vblank is still going so UMD isn't impacted.   As regard to connector 
> > > >and encoder does it matter if kernel mode change state without user mode 
> > > >knowing?  It seems to me those are more informational to UMD as UMD 
> > > >doesn't act on them.
> > > 
> > > windows does not know link state and all link management is hidden behind 
> > > kernel driver.   
> > 
> > If the user visible state doesn't change in any way, then the kernel could 
> > try to manage it all internally.
> > 
> > > 
> > > -Original Message-
> > > From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> > > Sent: Friday, November 11, 2016 9:05 AM
> > > To: Cheng, Tony <tony.ch...@amd.com>
> > > Cc: Deucher, Alexander <alexander.deuc...@amd.com>; 'Jani Nikula' 
> > > <jani.nik...@linux.intel.com>; Manasi Navare 
> > > <manasi.d.nav...@intel.com>; dri-de...@lists.freedesktop.org; 
> > > intel-gfx@lists.freedesktop.org; Wentland, Harry 
> > > <harry.wentl...@amd.com>; Peres, Marti

Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure during modeset

2016-11-13 Thread Manasi Navare 
On Fri, Nov 11, 2016 at 07:42:16PM +, Cheng, Tony wrote:
> In case of link training failure and requiring user space to set a lower 
> mode, would full mode set address it?  How do we make user mode select lower 
> resolution?
> 
> For example 4k@60Hz monitor, and link training at 4 lane HBR2 fails and 
> fallback to 4 lanes HBR, 4k@60 is no longer doable.  I would think preventing 
> user mode from seeing 4K@60Hz from the start is a easier and more robust 
> solution and requiring user mode to have logic to decide how to fallback.
>

Hi Tony,

So in case of link training failure, we first call mode_valid that will use the 
lower link rate/lane count
values based on the link training fallback to validate the modes and prune the 
modes that are higher than
the available link. After this is done, then we send the uevent to userspace 
indicating that link status is BAD and 
it will redo a probe and trigger a modeset for next possible lower resolution.

Manasi

 
> -Original Message-
> From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] 
> Sent: Friday, November 11, 2016 11:44 AM
> To: Cheng, Tony <tony.ch...@amd.com>
> Cc: Deucher, Alexander <alexander.deuc...@amd.com>; 'Jani Nikula' 
> <jani.nik...@linux.intel.com>; Manasi Navare <manasi.d.nav...@intel.com>; 
> dri-de...@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Wentland, 
> Harry <harry.wentl...@amd.com>; Peres, Martin <martin.pe...@intel.com>
> Subject: Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure during 
> modeset
> 
> On Fri, Nov 11, 2016 at 04:21:58PM +, Cheng, Tony wrote:
> > For HDMI, you can yank the cable, plug back in, HDMI will light up without 
> > user mode or kernel mode doing anything.
> > 
> > For DP this is not possible, someone will have to retrain the link when 
> > plugging back in or DP will not light up.  We see that on Ubuntu if someone 
> > unplug display and plug it back into same connector, we do not get KMS so 
> > in this case.  It appears there is some optimizations somewhere in user 
> > mode stack which I am not familiar with yet.  dal added workaround to 
> > retrain the link to light it back up (after the test training at end of 
> > detection).
> 
> The link should get retrained as the driver should check the link state on 
> HPD and retrain if it's not good. At least that's what we do in i915. Of 
> course if it's not the same display that got plugged back, the retraining 
> might fail. The new property can help with that case as userspace would then 
> attempt a full modeset after the failed link training.
> 
> > 
> > >From user mode perspective I think it make sense to keep CRTC running, so 
> > >vblank is still going so UMD isn't impacted.   As regard to connector and 
> > >encoder does it matter if kernel mode change state without user mode 
> > >knowing?  It seems to me those are more informational to UMD as UMD 
> > >doesn't act on them.
> > 
> > windows does not know link state and all link management is hidden behind 
> > kernel driver.   
> 
> If the user visible state doesn't change in any way, then the kernel could 
> try to manage it all internally.
> 
> > 
> > -Original Message-
> > From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> > Sent: Friday, November 11, 2016 9:05 AM
> > To: Cheng, Tony <tony.ch...@amd.com>
> > Cc: Deucher, Alexander <alexander.deuc...@amd.com>; 'Jani Nikula' 
> > <jani.nik...@linux.intel.com>; Manasi Navare 
> > <manasi.d.nav...@intel.com>; dri-de...@lists.freedesktop.org; 
> > intel-gfx@lists.freedesktop.org; Wentland, Harry 
> > <harry.wentl...@amd.com>; Peres, Martin <martin.pe...@intel.com>
> > Subject: Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure 
> > during modeset
> > 
> > On Thu, Nov 10, 2016 at 06:51:40PM +, Cheng, Tony wrote:
> > > Amdgpu dal implementation will do a test link training at end of 
> > > detection to verify we can achieve the capability reported in DPCD.  We 
> > > then report mode base on result of test training.
> > > 
> > > AMD hardware (at least the generations supported by amdgpu) is able to 
> > > link train without timing being setup (DP encoder and CRTC is decoupled). 
> > >  Do we have limitation from other vendors where you need timing to be 
> > > there before you can link train?
> > 
> > I can't recall the specifics for all of our supported platforms, but at 
> > least I have the recollection that it would be the case yes.
> > 
> > The other problem wiyh this apporach is that even if you don't

Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure during modeset

2016-11-11 Thread Cheng, Tony 
In case of link training failure and requiring user space to set a lower mode, 
would full mode set address it?  How do we make user mode select lower 
resolution?

For example 4k@60Hz monitor, and link training at 4 lane HBR2 fails and 
fallback to 4 lanes HBR, 4k@60 is no longer doable.  I would think preventing 
user mode from seeing 4K@60Hz from the start is a easier and more robust 
solution and requiring user mode to have logic to decide how to fallback.

-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] 
Sent: Friday, November 11, 2016 11:44 AM
To: Cheng, Tony <tony.ch...@amd.com>
Cc: Deucher, Alexander <alexander.deuc...@amd.com>; 'Jani Nikula' 
<jani.nik...@linux.intel.com>; Manasi Navare <manasi.d.nav...@intel.com>; 
dri-de...@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Wentland, 
Harry <harry.wentl...@amd.com>; Peres, Martin <martin.pe...@intel.com>
Subject: Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure during modeset

On Fri, Nov 11, 2016 at 04:21:58PM +, Cheng, Tony wrote:
> For HDMI, you can yank the cable, plug back in, HDMI will light up without 
> user mode or kernel mode doing anything.
> 
> For DP this is not possible, someone will have to retrain the link when 
> plugging back in or DP will not light up.  We see that on Ubuntu if someone 
> unplug display and plug it back into same connector, we do not get KMS so in 
> this case.  It appears there is some optimizations somewhere in user mode 
> stack which I am not familiar with yet.  dal added workaround to retrain the 
> link to light it back up (after the test training at end of detection).

The link should get retrained as the driver should check the link state on HPD 
and retrain if it's not good. At least that's what we do in i915. Of course if 
it's not the same display that got plugged back, the retraining might fail. The 
new property can help with that case as userspace would then attempt a full 
modeset after the failed link training.

> 
> >From user mode perspective I think it make sense to keep CRTC running, so 
> >vblank is still going so UMD isn't impacted.   As regard to connector and 
> >encoder does it matter if kernel mode change state without user mode 
> >knowing?  It seems to me those are more informational to UMD as UMD doesn't 
> >act on them.
> 
> windows does not know link state and all link management is hidden behind 
> kernel driver.   

If the user visible state doesn't change in any way, then the kernel could try 
to manage it all internally.

> 
> -Original Message-
> From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> Sent: Friday, November 11, 2016 9:05 AM
> To: Cheng, Tony <tony.ch...@amd.com>
> Cc: Deucher, Alexander <alexander.deuc...@amd.com>; 'Jani Nikula' 
> <jani.nik...@linux.intel.com>; Manasi Navare 
> <manasi.d.nav...@intel.com>; dri-de...@lists.freedesktop.org; 
> intel-gfx@lists.freedesktop.org; Wentland, Harry 
> <harry.wentl...@amd.com>; Peres, Martin <martin.pe...@intel.com>
> Subject: Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure 
> during modeset
> 
> On Thu, Nov 10, 2016 at 06:51:40PM +, Cheng, Tony wrote:
> > Amdgpu dal implementation will do a test link training at end of detection 
> > to verify we can achieve the capability reported in DPCD.  We then report 
> > mode base on result of test training.
> > 
> > AMD hardware (at least the generations supported by amdgpu) is able to link 
> > train without timing being setup (DP encoder and CRTC is decoupled).  Do we 
> > have limitation from other vendors where you need timing to be there before 
> > you can link train?
> 
> I can't recall the specifics for all of our supported platforms, but at least 
> I have the recollection that it would be the case yes.
> 
> The other problem wiyh this apporach is that even if you don't need the crtc, 
> you still need the link itself. What happens if the link is still active 
> since userspace just didn't bother to shut it down when the cable was yanked? 
> Can you keep the crtc going but stop it from feeding the link in a way that 
> userspace won't be able to notice? The kms design has always been pretty much 
> that policy is in userspace, and thus the kernel shouldn't shut down crtcs 
> unless explicitly asked to do so.
> 
> > 
> > We can also past DP1.2 link layer compliance with this approach.
> > 
> > -Original Message-
> > From: Deucher, Alexander
> > Sent: Thursday, November 10, 2016 1:43 PM
> > To: 'Jani Nikula' <jani.nik...@linux.intel.com>; Manasi Navare 
> > <manasi.d.nav...@intel.com>; dri-de...@lists.freedesktop.org; 
> > intel-gfx@lists.f

Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure during modeset

2016-11-11 Thread Ville Syrjälä 
On Fri, Nov 11, 2016 at 04:21:58PM +, Cheng, Tony wrote:
> For HDMI, you can yank the cable, plug back in, HDMI will light up without 
> user mode or kernel mode doing anything.
> 
> For DP this is not possible, someone will have to retrain the link when 
> plugging back in or DP will not light up.  We see that on Ubuntu if someone 
> unplug display and plug it back into same connector, we do not get KMS so in 
> this case.  It appears there is some optimizations somewhere in user mode 
> stack which I am not familiar with yet.  dal added workaround to retrain the 
> link to light it back up (after the test training at end of detection).

The link should get retrained as the driver should check the link
state on HPD and retrain if it's not good. At least that's what we do in
i915. Of course if it's not the same display that got plugged back,
the retraining might fail. The new property can help with that
case as userspace would then attempt a full modeset after the failed
link training.

> 
> >From user mode perspective I think it make sense to keep CRTC running, so 
> >vblank is still going so UMD isn't impacted.   As regard to connector and 
> >encoder does it matter if kernel mode change state without user mode 
> >knowing?  It seems to me those are more informational to UMD as UMD doesn't 
> >act on them.
> 
> windows does not know link state and all link management is hidden behind 
> kernel driver.   

If the user visible state doesn't change in any way, then the kernel could
try to manage it all internally.

> 
> -Original Message-
> From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] 
> Sent: Friday, November 11, 2016 9:05 AM
> To: Cheng, Tony <tony.ch...@amd.com>
> Cc: Deucher, Alexander <alexander.deuc...@amd.com>; 'Jani Nikula' 
> <jani.nik...@linux.intel.com>; Manasi Navare <manasi.d.nav...@intel.com>; 
> dri-de...@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Wentland, 
> Harry <harry.wentl...@amd.com>; Peres, Martin <martin.pe...@intel.com>
> Subject: Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure during 
> modeset
> 
> On Thu, Nov 10, 2016 at 06:51:40PM +, Cheng, Tony wrote:
> > Amdgpu dal implementation will do a test link training at end of detection 
> > to verify we can achieve the capability reported in DPCD.  We then report 
> > mode base on result of test training.
> > 
> > AMD hardware (at least the generations supported by amdgpu) is able to link 
> > train without timing being setup (DP encoder and CRTC is decoupled).  Do we 
> > have limitation from other vendors where you need timing to be there before 
> > you can link train?
> 
> I can't recall the specifics for all of our supported platforms, but at least 
> I have the recollection that it would be the case yes.
> 
> The other problem wiyh this apporach is that even if you don't need the crtc, 
> you still need the link itself. What happens if the link is still active 
> since userspace just didn't bother to shut it down when the cable was yanked? 
> Can you keep the crtc going but stop it from feeding the link in a way that 
> userspace won't be able to notice? The kms design has always been pretty much 
> that policy is in userspace, and thus the kernel shouldn't shut down crtcs 
> unless explicitly asked to do so.
> 
> > 
> > We can also past DP1.2 link layer compliance with this approach.
> > 
> > -Original Message-
> > From: Deucher, Alexander
> > Sent: Thursday, November 10, 2016 1:43 PM
> > To: 'Jani Nikula' <jani.nik...@linux.intel.com>; Manasi Navare 
> > <manasi.d.nav...@intel.com>; dri-de...@lists.freedesktop.org; 
> > intel-gfx@lists.freedesktop.org; Wentland, Harry 
> > <harry.wentl...@amd.com>; Cheng, Tony <tony.ch...@amd.com>
> > Cc: Dave Airlie <airl...@gmail.com>; Peres, Martin 
> > <martin.pe...@intel.com>
> > Subject: RE: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure 
> > during modeset
> > 
> > Adding Harry and Tony from our display team to review.
> > 
> > > -----Original Message-
> > > From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
> > > Sent: Thursday, November 10, 2016 1:20 PM
> > > To: Manasi Navare; dri-de...@lists.freedesktop.org; intel- 
> > > g...@lists.freedesktop.org
> > > Cc: Dave Airlie; Peres, Martin; Deucher, Alexander
> > > Subject: Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure 
> > > during modeset
> > > 
> > > On Thu, 10 Nov 2016, Manasi Navare <manasi.d.nav...@intel.com> wrote:
> > > > Link training failure is handled b

Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure during modeset

2016-11-11 Thread Cheng, Tony 
For HDMI, you can yank the cable, plug back in, HDMI will light up without user 
mode or kernel mode doing anything.

For DP this is not possible, someone will have to retrain the link when 
plugging back in or DP will not light up.  We see that on Ubuntu if someone 
unplug display and plug it back into same connector, we do not get KMS so in 
this case.  It appears there is some optimizations somewhere in user mode stack 
which I am not familiar with yet.  dal added workaround to retrain the link to 
light it back up (after the test training at end of detection).

From user mode perspective I think it make sense to keep CRTC running, so 
vblank is still going so UMD isn't impacted.   As regard to connector and 
encoder does it matter if kernel mode change state without user mode knowing?  
It seems to me those are more informational to UMD as UMD doesn't act on them.

windows does not know link state and all link management is hidden behind 
kernel driver.   

-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] 
Sent: Friday, November 11, 2016 9:05 AM
To: Cheng, Tony <tony.ch...@amd.com>
Cc: Deucher, Alexander <alexander.deuc...@amd.com>; 'Jani Nikula' 
<jani.nik...@linux.intel.com>; Manasi Navare <manasi.d.nav...@intel.com>; 
dri-de...@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Wentland, 
Harry <harry.wentl...@amd.com>; Peres, Martin <martin.pe...@intel.com>
Subject: Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure during modeset

On Thu, Nov 10, 2016 at 06:51:40PM +, Cheng, Tony wrote:
> Amdgpu dal implementation will do a test link training at end of detection to 
> verify we can achieve the capability reported in DPCD.  We then report mode 
> base on result of test training.
> 
> AMD hardware (at least the generations supported by amdgpu) is able to link 
> train without timing being setup (DP encoder and CRTC is decoupled).  Do we 
> have limitation from other vendors where you need timing to be there before 
> you can link train?

I can't recall the specifics for all of our supported platforms, but at least I 
have the recollection that it would be the case yes.

The other problem wiyh this apporach is that even if you don't need the crtc, 
you still need the link itself. What happens if the link is still active since 
userspace just didn't bother to shut it down when the cable was yanked? Can you 
keep the crtc going but stop it from feeding the link in a way that userspace 
won't be able to notice? The kms design has always been pretty much that policy 
is in userspace, and thus the kernel shouldn't shut down crtcs unless 
explicitly asked to do so.

> 
> We can also past DP1.2 link layer compliance with this approach.
> 
> -Original Message-
> From: Deucher, Alexander
> Sent: Thursday, November 10, 2016 1:43 PM
> To: 'Jani Nikula' <jani.nik...@linux.intel.com>; Manasi Navare 
> <manasi.d.nav...@intel.com>; dri-de...@lists.freedesktop.org; 
> intel-gfx@lists.freedesktop.org; Wentland, Harry 
> <harry.wentl...@amd.com>; Cheng, Tony <tony.ch...@amd.com>
> Cc: Dave Airlie <airl...@gmail.com>; Peres, Martin 
> <martin.pe...@intel.com>
> Subject: RE: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure 
> during modeset
> 
> Adding Harry and Tony from our display team to review.
> 
> > -Original Message-
> > From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
> > Sent: Thursday, November 10, 2016 1:20 PM
> > To: Manasi Navare; dri-de...@lists.freedesktop.org; intel- 
> > g...@lists.freedesktop.org
> > Cc: Dave Airlie; Peres, Martin; Deucher, Alexander
> > Subject: Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure 
> > during modeset
> > 
> > On Thu, 10 Nov 2016, Manasi Navare <manasi.d.nav...@intel.com> wrote:
> > > Link training failure is handled by lowering the link rate first 
> > > until it reaches the minimum and keeping the lane count maximum 
> > > and then lowering the lane count until it reaches minimim. These 
> > > fallback values are saved and hotplug uevent is sent to the 
> > > userspace after setting the connector link status property to BAD.
> > > Userspace should triiger another modeset on a uevent and if link 
> > > status property is BAD. This will retrain the link at fallback values.
> > > This is repeated until the link is successfully trained.
> > >
> > > This has been validated to pass DP compliance.
> > 
> > This cover letter and the commit messages do a good job of 
> > explaining what the patches do. However, you're lacking the crucial 
> > information of
> > *why* we need userspace cooperation to handle link training failures 
> > on DP mode setting, a

Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure during modeset

2016-11-11 Thread Ville Syrjälä 
On Thu, Nov 10, 2016 at 06:51:40PM +, Cheng, Tony wrote:
> Amdgpu dal implementation will do a test link training at end of detection to 
> verify we can achieve the capability reported in DPCD.  We then report mode 
> base on result of test training.
> 
> AMD hardware (at least the generations supported by amdgpu) is able to link 
> train without timing being setup (DP encoder and CRTC is decoupled).  Do we 
> have limitation from other vendors where you need timing to be there before 
> you can link train?

I can't recall the specifics for all of our supported platforms, but at
least I have the recollection that it would be the case yes.

The other problem wiyh this apporach is that even if you don't need the
crtc, you still need the link itself. What happens if the link is still
active since userspace just didn't bother to shut it down when the cable
was yanked? Can you keep the crtc going but stop it from feeding the
link in a way that userspace won't be able to notice? The kms design has
always been pretty much that policy is in userspace, and thus the kernel
shouldn't shut down crtcs unless explicitly asked to do so.

> 
> We can also past DP1.2 link layer compliance with this approach.
> 
> -Original Message-
> From: Deucher, Alexander 
> Sent: Thursday, November 10, 2016 1:43 PM
> To: 'Jani Nikula' <jani.nik...@linux.intel.com>; Manasi Navare 
> <manasi.d.nav...@intel.com>; dri-de...@lists.freedesktop.org; 
> intel-gfx@lists.freedesktop.org; Wentland, Harry <harry.wentl...@amd.com>; 
> Cheng, Tony <tony.ch...@amd.com>
> Cc: Dave Airlie <airl...@gmail.com>; Peres, Martin <martin.pe...@intel.com>
> Subject: RE: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure during 
> modeset
> 
> Adding Harry and Tony from our display team to review.
> 
> > -Original Message-
> > From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
> > Sent: Thursday, November 10, 2016 1:20 PM
> > To: Manasi Navare; dri-de...@lists.freedesktop.org; intel- 
> > g...@lists.freedesktop.org
> > Cc: Dave Airlie; Peres, Martin; Deucher, Alexander
> > Subject: Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure 
> > during modeset
> > 
> > On Thu, 10 Nov 2016, Manasi Navare <manasi.d.nav...@intel.com> wrote:
> > > Link training failure is handled by lowering the link rate first 
> > > until it reaches the minimum and keeping the lane count maximum and 
> > > then lowering the lane count until it reaches minimim. These 
> > > fallback values are saved and hotplug uevent is sent to the 
> > > userspace after setting the connector link status property to BAD. 
> > > Userspace should triiger another modeset on a uevent and if link 
> > > status property is BAD. This will retrain the link at fallback values.
> > > This is repeated until the link is successfully trained.
> > >
> > > This has been validated to pass DP compliance.
> > 
> > This cover letter and the commit messages do a good job of explaining 
> > what the patches do. However, you're lacking the crucial information 
> > of
> > *why* we need userspace cooperation to handle link training failures 
> > on DP mode setting, and *why* a new connector property is a good 
> > solution for this.
> > 
> > Here goes, including some alternative approaches we've considered (and 
> > even tried):
> > 
> > At the time userspace does setcrtc, we've already promised the mode 
> > would work. The promise is based on the theoretical capabilities of 
> > the link, but it's possible we can't reach this in practice. The DP 
> > spec describes how the link should be reduced, but we can't reduce the 
> > link below the requirements of the mode. Black screen follows.
> > 
> > One idea would be to have setcrtc return a failure. However, it is my 
> > understanding that it already should not fail as the atomic checks 
> > have passed [citation needed]. It would also conflict with the idea of 
> > making setcrtc asynchronous in the future, returning before the actual 
> > mode setting and link training.
> > 
> > Another idea is to train the link "upfront" at hotplug time, before 
> > pruning the mode list, so that we can do the pruning based on 
> > practical not theoretical capabilities. However, the changes for link 
> > training are pretty drastic, all for the sake of error handling and DP 
> > compliance, when the most common happy day scenario is the current 
> > approach of link training at mode setting time, using the optimal 
> > parameters for the mode. It is also not certain all hardware could do 
> > t

Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure during modeset

2016-11-11 Thread Cheng, Tony 
Amdgpu dal implementation will do a test link training at end of detection to 
verify we can achieve the capability reported in DPCD.  We then report mode 
base on result of test training.

AMD hardware (at least the generations supported by amdgpu) is able to link 
train without timing being setup (DP encoder and CRTC is decoupled).  Do we 
have limitation from other vendors where you need timing to be there before you 
can link train?

We can also past DP1.2 link layer compliance with this approach.

-Original Message-
From: Deucher, Alexander 
Sent: Thursday, November 10, 2016 1:43 PM
To: 'Jani Nikula' <jani.nik...@linux.intel.com>; Manasi Navare 
<manasi.d.nav...@intel.com>; dri-de...@lists.freedesktop.org; 
intel-gfx@lists.freedesktop.org; Wentland, Harry <harry.wentl...@amd.com>; 
Cheng, Tony <tony.ch...@amd.com>
Cc: Dave Airlie <airl...@gmail.com>; Peres, Martin <martin.pe...@intel.com>
Subject: RE: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure during modeset

Adding Harry and Tony from our display team to review.

> -Original Message-
> From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
> Sent: Thursday, November 10, 2016 1:20 PM
> To: Manasi Navare; dri-de...@lists.freedesktop.org; intel- 
> g...@lists.freedesktop.org
> Cc: Dave Airlie; Peres, Martin; Deucher, Alexander
> Subject: Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure 
> during modeset
> 
> On Thu, 10 Nov 2016, Manasi Navare <manasi.d.nav...@intel.com> wrote:
> > Link training failure is handled by lowering the link rate first 
> > until it reaches the minimum and keeping the lane count maximum and 
> > then lowering the lane count until it reaches minimim. These 
> > fallback values are saved and hotplug uevent is sent to the 
> > userspace after setting the connector link status property to BAD. 
> > Userspace should triiger another modeset on a uevent and if link 
> > status property is BAD. This will retrain the link at fallback values.
> > This is repeated until the link is successfully trained.
> >
> > This has been validated to pass DP compliance.
> 
> This cover letter and the commit messages do a good job of explaining 
> what the patches do. However, you're lacking the crucial information 
> of
> *why* we need userspace cooperation to handle link training failures 
> on DP mode setting, and *why* a new connector property is a good 
> solution for this.
> 
> Here goes, including some alternative approaches we've considered (and 
> even tried):
> 
> At the time userspace does setcrtc, we've already promised the mode 
> would work. The promise is based on the theoretical capabilities of 
> the link, but it's possible we can't reach this in practice. The DP 
> spec describes how the link should be reduced, but we can't reduce the 
> link below the requirements of the mode. Black screen follows.
> 
> One idea would be to have setcrtc return a failure. However, it is my 
> understanding that it already should not fail as the atomic checks 
> have passed [citation needed]. It would also conflict with the idea of 
> making setcrtc asynchronous in the future, returning before the actual 
> mode setting and link training.
> 
> Another idea is to train the link "upfront" at hotplug time, before 
> pruning the mode list, so that we can do the pruning based on 
> practical not theoretical capabilities. However, the changes for link 
> training are pretty drastic, all for the sake of error handling and DP 
> compliance, when the most common happy day scenario is the current 
> approach of link training at mode setting time, using the optimal 
> parameters for the mode. It is also not certain all hardware could do 
> this without the pipe on; not even all our hardware can do this. Some 
> of this can be solved, but not trivially.
> 
> Both of the above ideas also fail to address link degradation *during* 
> operation.
> 
> So the idea presented in these patches is to do this in a way that a) 
> changes the current happy day scenario as little as possible, to avoid 
> regressions, b) can be implemented the same way by all drm drivers, c) 
> is still opt-in for the drivers and userspace, and opting out doesn't 
> regress the user experience, d) doesn't prevent drivers from 
> implementing better or alternate approaches, possibly without 
> userspace involvement. And, of course, handles all the issues presented.
> 
> The solution is to add a "link status" connector property. In the 
> usual happy day scenario, this is always "good". If something fails 
> during or after a mode set, the kernel driver can set the link status 
> to "bad", prune the mode list based on new information as necessar

Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure during modeset

2016-11-10 Thread Deucher, Alexander 
Adding Harry and Tony from our display team to review.

> -Original Message-
> From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
> Sent: Thursday, November 10, 2016 1:20 PM
> To: Manasi Navare; dri-de...@lists.freedesktop.org; intel-
> g...@lists.freedesktop.org
> Cc: Dave Airlie; Peres, Martin; Deucher, Alexander
> Subject: Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure during
> modeset
> 
> On Thu, 10 Nov 2016, Manasi Navare <manasi.d.nav...@intel.com> wrote:
> > Link training failure is handled by lowering the link rate first
> > until it reaches the minimum and keeping the lane count maximum
> > and then lowering the lane count until it reaches minimim. These
> > fallback values are saved and hotplug uevent is sent to the userspace
> > after setting the connector link status property to BAD. Userspace
> > should triiger another modeset on a uevent and if link status property
> > is BAD. This will retrain the link at fallback values.
> > This is repeated until the link is successfully trained.
> >
> > This has been validated to pass DP compliance.
> 
> This cover letter and the commit messages do a good job of explaining
> what the patches do. However, you're lacking the crucial information of
> *why* we need userspace cooperation to handle link training failures on
> DP mode setting, and *why* a new connector property is a good solution
> for this.
> 
> Here goes, including some alternative approaches we've considered (and
> even tried):
> 
> At the time userspace does setcrtc, we've already promised the mode
> would work. The promise is based on the theoretical capabilities of the
> link, but it's possible we can't reach this in practice. The DP spec
> describes how the link should be reduced, but we can't reduce the link
> below the requirements of the mode. Black screen follows.
> 
> One idea would be to have setcrtc return a failure. However, it is my
> understanding that it already should not fail as the atomic checks have
> passed [citation needed]. It would also conflict with the idea of making
> setcrtc asynchronous in the future, returning before the actual mode
> setting and link training.
> 
> Another idea is to train the link "upfront" at hotplug time, before
> pruning the mode list, so that we can do the pruning based on practical
> not theoretical capabilities. However, the changes for link training are
> pretty drastic, all for the sake of error handling and DP compliance,
> when the most common happy day scenario is the current approach of link
> training at mode setting time, using the optimal parameters for the
> mode. It is also not certain all hardware could do this without the pipe
> on; not even all our hardware can do this. Some of this can be solved,
> but not trivially.
> 
> Both of the above ideas also fail to address link degradation *during*
> operation.
> 
> So the idea presented in these patches is to do this in a way that a)
> changes the current happy day scenario as little as possible, to avoid
> regressions, b) can be implemented the same way by all drm drivers, c)
> is still opt-in for the drivers and userspace, and opting out doesn't
> regress the user experience, d) doesn't prevent drivers from
> implementing better or alternate approaches, possibly without userspace
> involvement. And, of course, handles all the issues presented.
> 
> The solution is to add a "link status" connector property. In the usual
> happy day scenario, this is always "good". If something fails during or
> after a mode set, the kernel driver can set the link status to "bad",
> prune the mode list based on new information as necessary, and send a
> hotplug uevent for userspace to have it re-check the valid modes through
> getconnector, and try again. If the theoretical capabilities of the link
> can't be reached, the mode list is trimmed based on that.
> 
> If the userspace is not aware of the property, the user experience is
> the same as it currently is. If the userspace is aware of the property,
> it has a chance to improve user experience. If a drm driver does not
> modify the property (it stays "good"), the user experience is the same
> as it currently is. A drm driver can also choose to try to handle more
> of the failures in kernel, hardware not limiting, or it can choose to
> involve userspace more. Up to the drivers.
> 
> The reason for adding the property is to handle link training failures,
> but it is not limited to DP or link training. For example, if we
> implement asynchronous setcrtc, we can use this to report any failures
> in that.
> 
> Finally, while DP CTS compliance is advertized (which is great, and
> could be

Re: [Intel-gfx] [PATCH 0/5] Handle Link Training Failure during modeset

2016-11-10 Thread Jani Nikula 
On Thu, 10 Nov 2016, Manasi Navare  wrote:
> Link training failure is handled by lowering the link rate first
> until it reaches the minimum and keeping the lane count maximum
> and then lowering the lane count until it reaches minimim. These
> fallback values are saved and hotplug uevent is sent to the userspace
> after setting the connector link status property to BAD. Userspace
> should triiger another modeset on a uevent and if link status property
> is BAD. This will retrain the link at fallback values.
> This is repeated until the link is successfully trained.
>
> This has been validated to pass DP compliance.

This cover letter and the commit messages do a good job of explaining
what the patches do. However, you're lacking the crucial information of
*why* we need userspace cooperation to handle link training failures on
DP mode setting, and *why* a new connector property is a good solution
for this.

Here goes, including some alternative approaches we've considered (and
even tried):

At the time userspace does setcrtc, we've already promised the mode
would work. The promise is based on the theoretical capabilities of the
link, but it's possible we can't reach this in practice. The DP spec
describes how the link should be reduced, but we can't reduce the link
below the requirements of the mode. Black screen follows.

One idea would be to have setcrtc return a failure. However, it is my
understanding that it already should not fail as the atomic checks have
passed [citation needed]. It would also conflict with the idea of making
setcrtc asynchronous in the future, returning before the actual mode
setting and link training.

Another idea is to train the link "upfront" at hotplug time, before
pruning the mode list, so that we can do the pruning based on practical
not theoretical capabilities. However, the changes for link training are
pretty drastic, all for the sake of error handling and DP compliance,
when the most common happy day scenario is the current approach of link
training at mode setting time, using the optimal parameters for the
mode. It is also not certain all hardware could do this without the pipe
on; not even all our hardware can do this. Some of this can be solved,
but not trivially.

Both of the above ideas also fail to address link degradation *during*
operation.

So the idea presented in these patches is to do this in a way that a)
changes the current happy day scenario as little as possible, to avoid
regressions, b) can be implemented the same way by all drm drivers, c)
is still opt-in for the drivers and userspace, and opting out doesn't
regress the user experience, d) doesn't prevent drivers from
implementing better or alternate approaches, possibly without userspace
involvement. And, of course, handles all the issues presented.

The solution is to add a "link status" connector property. In the usual
happy day scenario, this is always "good". If something fails during or
after a mode set, the kernel driver can set the link status to "bad",
prune the mode list based on new information as necessary, and send a
hotplug uevent for userspace to have it re-check the valid modes through
getconnector, and try again. If the theoretical capabilities of the link
can't be reached, the mode list is trimmed based on that.

If the userspace is not aware of the property, the user experience is
the same as it currently is. If the userspace is aware of the property,
it has a chance to improve user experience. If a drm driver does not
modify the property (it stays "good"), the user experience is the same
as it currently is. A drm driver can also choose to try to handle more
of the failures in kernel, hardware not limiting, or it can choose to
involve userspace more. Up to the drivers.

The reason for adding the property is to handle link training failures,
but it is not limited to DP or link training. For example, if we
implement asynchronous setcrtc, we can use this to report any failures
in that.

Finally, while DP CTS compliance is advertized (which is great, and
could be made to work similarly for all drm drivers), this can be used
for the more important goal of improving user experience on link
training failures, by avoiding black screens.


BR,
Jani.


>
> Manasi Navare (5):
>   drm: Add a new connector property for link status
>   drm/i915: Set link status property for DP connector
>   drm/i915: Update CRTC state if connector link status property changed


This is really drm, not i915.


>   drm/i915: Find fallback link rate/lane count
>   drm/i915: Implement Link Rate fallback on Link training failure
>
>  drivers/gpu/drm/drm_atomic_helper.c   |   7 ++
>  drivers/gpu/drm/drm_connector.c   |  17 
>  drivers/gpu/drm/i915/intel_ddi.c  |  21 +++-
>  drivers/gpu/drm/i915/intel_dp.c   | 138 
> +-
>  drivers/gpu/drm/i915/intel_dp_link_training.c |  12 ++-
>

[Intel-gfx] [PATCH 0/5] Handle Link Training Failure during modeset

2016-11-09 Thread Manasi Navare 
Link training failure is handled by lowering the link rate first
until it reaches the minimum and keeping the lane count maximum
and then lowering the lane count until it reaches minimim. These
fallback values are saved and hotplug uevent is sent to the userspace
after setting the connector link status property to BAD. Userspace
should triiger another modeset on a uevent and if link status property
is BAD. This will retrain the link at fallback values.
This is repeated until the link is successfully trained.

This has been validated to pass DP compliance.

Manasi Navare (5):
  drm: Add a new connector property for link status
  drm/i915: Set link status property for DP connector
  drm/i915: Update CRTC state if connector link status property changed
  drm/i915: Find fallback link rate/lane count
  drm/i915: Implement Link Rate fallback on Link training failure

 drivers/gpu/drm/drm_atomic_helper.c   |   7 ++
 drivers/gpu/drm/drm_connector.c   |  17 
 drivers/gpu/drm/i915/intel_ddi.c  |  21 +++-
 drivers/gpu/drm/i915/intel_dp.c   | 138 +-
 drivers/gpu/drm/i915/intel_dp_link_training.c |  12 ++-
 drivers/gpu/drm/i915/intel_drv.h  |  12 ++-
 include/drm/drm_connector.h   |   7 +-
 include/drm/drm_crtc.h|   5 +
 include/uapi/drm/drm_mode.h   |   4 +
 9 files changed, 214 insertions(+), 9 deletions(-)

-- 
1.9.1

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