Re: [Intel-gfx] [PATCH 3/3] drm/i915: Support DDI lane reversal for DP

2015-08-26 Thread Sivakumar Thulasimani



On 8/26/2015 7:59 PM, Benjamin Tissoires wrote:

On Aug 26 2015 or thereabouts, Sivakumar Thulasimani wrote:


On 8/18/2015 1:36 AM, Benjamin Tissoires wrote:

On Aug 14 2015 or thereabouts, Stéphane Marchesin wrote:

On Wed, Aug 5, 2015 at 12:34 PM, Benjamin Tissoires
 wrote:

On Jul 30 2015 or thereabouts, Sivakumar Thulasimani wrote:

On 7/29/2015 8:52 PM, Benjamin Tissoires wrote:

On Jul 29 2015 or thereabouts, Sivakumar Thulasimani wrote:

why not detect reverse in intel_dp_detect/intel_hpd_pulse ? that way you can
identify both lane count and reversal state without touching anything in the
link training code. i am yet to upstream my changes for CHT that i can share
if required that does the same in intel_dp_detect without touching any line
in link training path.

With my current limited knowledge of the dp hotplug (and i915 driver) I
am not sure we could detect the reversed state without trying to train 1
lane only. I'd be glad to look at your changes and test them on my
system if you think that could help having a cleaner solution.

Cheers,
Benjamin

No, what i recommended was to do link training but in intel_dp_detect. Since
USB Type C cable
also has its own lane count restriction (it can have different lane count
than the one supported
by panel) you might have to figure that out as well. so both reversal and
lane count detection
can be done outside the modeset path and keep the code free of type C
changes outside
detection path.

Please find below the code to do the same. Do not waste time trying to apply
this directly on
nightly since this is based on a local tree and because this is pre- atomic
changes code, so you
might have to modify chv_upfront_link_train to work on top of the latest
nightly code. we
are supposed to upstream this and is in my todo list.


[original patch snipped...]

Hi Sivakumar,

So I managed to manually re-apply your patch on top of
drm-intel-nightly, and tried to port it to make Broadwell working too.
It works OK if the system is already boot without any external DP used.
In this case, the detection works and I can see my external monitor
working properly.

However, if the monitor is cold plugged, the cpu/GPU hangs and I can not
understand why. I think I enabled all that is mentioned in the PRM to be
able to train the DP link, but I am obviously missing something else.
Can you have a look?


Hi Benjamin,

I would recommend against this approach. Some adapters will claim that
they recovered a clock even when it isn't on the lanes you enabled,
which means that the reversal detection doesn't always work. The only
reliable way to do this is to go talk to the Chrome OS EC (you can
find these patches later in the Chrome OS tree). It's not as generic,
but we might be able to abstract that logic, maybe.


Hi Stéphane,

This is a very good news. I was afraid we would not have access to the
hardware controller because the Intel controller hub spec was not
public.

I will try to have a look at it, but the latest chromeos branch (3.18)
seems to differ quite a lot from the upstream one. Anyway, fingers
crossed.

Cheers,
Benjamin

Hi Benjamin/Stéphan,

Hi Sivakumar,


 All Intel platforms (at-least those i inquired about) handle lane
reversal in HW.

That is the theory and what is written in the USB Type C spec IIRC.
Problem is, the Chromebook Pixel 2 external display does not work when a
USB Type-C adapter is in the reversed position (or believe me, I would
definitively not have submitted a patch for the beauty of it).
Everything else works (link training when 4 lanes are activated, or
other communication channels). Only the order of the 4 data lanes
matters in this situation and the USB controller does not reverse them
for us on this laptop.

your statement that link training will pass even on reversed lane seems to
point
to the same fact. in such a scenario why should the encoder/connector care
if the lane is reversed or not ?

Problem is that Stephane said some adapters are lying regarding the
clock recovery. They claim everything is fine while in the end, the
display doesn't show anything because the lanes are reversed. If this is
just a chromebook Pixel 2 issue, that's better then. I won't have to try
to put some generic interface to notify that the display port lanes have
to be reversed.

Cheers,
Benjamin
Thanks for the info :). There is no way in code to confirm if after link 
training display

is working properly (other than short pulse for link related issues)
so this has to be on a case by case basis. i would recommend
if you plan to upstream it to restrict the changes for chromebook Pixel 
2 alone.

--
regards,
Sivakumar



--
regards,
Sivakumar

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Intel-gfx] [PATCH 3/3] drm/i915: Support DDI lane reversal for DP

2015-08-26 Thread Sivakumar Thulasimani



On 8/18/2015 1:36 AM, Benjamin Tissoires wrote:

On Aug 14 2015 or thereabouts, Stéphane Marchesin wrote:

On Wed, Aug 5, 2015 at 12:34 PM, Benjamin Tissoires
 wrote:

On Jul 30 2015 or thereabouts, Sivakumar Thulasimani wrote:


On 7/29/2015 8:52 PM, Benjamin Tissoires wrote:

On Jul 29 2015 or thereabouts, Sivakumar Thulasimani wrote:

why not detect reverse in intel_dp_detect/intel_hpd_pulse ? that way you can
identify both lane count and reversal state without touching anything in the
link training code. i am yet to upstream my changes for CHT that i can share
if required that does the same in intel_dp_detect without touching any line
in link training path.

With my current limited knowledge of the dp hotplug (and i915 driver) I
am not sure we could detect the reversed state without trying to train 1
lane only. I'd be glad to look at your changes and test them on my
system if you think that could help having a cleaner solution.

Cheers,
Benjamin

No, what i recommended was to do link training but in intel_dp_detect. Since
USB Type C cable
also has its own lane count restriction (it can have different lane count
than the one supported
by panel) you might have to figure that out as well. so both reversal and
lane count detection
can be done outside the modeset path and keep the code free of type C
changes outside
detection path.

Please find below the code to do the same. Do not waste time trying to apply
this directly on
nightly since this is based on a local tree and because this is pre- atomic
changes code, so you
might have to modify chv_upfront_link_train to work on top of the latest
nightly code. we
are supposed to upstream this and is in my todo list.


[original patch snipped...]

Hi Sivakumar,

So I managed to manually re-apply your patch on top of
drm-intel-nightly, and tried to port it to make Broadwell working too.
It works OK if the system is already boot without any external DP used.
In this case, the detection works and I can see my external monitor
working properly.

However, if the monitor is cold plugged, the cpu/GPU hangs and I can not
understand why. I think I enabled all that is mentioned in the PRM to be
able to train the DP link, but I am obviously missing something else.
Can you have a look?


Hi Benjamin,

I would recommend against this approach. Some adapters will claim that
they recovered a clock even when it isn't on the lanes you enabled,
which means that the reversal detection doesn't always work. The only
reliable way to do this is to go talk to the Chrome OS EC (you can
find these patches later in the Chrome OS tree). It's not as generic,
but we might be able to abstract that logic, maybe.


Hi Stéphane,

This is a very good news. I was afraid we would not have access to the
hardware controller because the Intel controller hub spec was not
public.

I will try to have a look at it, but the latest chromeos branch (3.18)
seems to differ quite a lot from the upstream one. Anyway, fingers
crossed.

Cheers,
Benjamin

Hi Benjamin/Stéphan,
All Intel platforms (at-least those i inquired about) handle lane 
reversal in HW.
your statement that link training will pass even on reversed lane seems 
to point

to the same fact. in such a scenario why should the encoder/connector care
if the lane is reversed or not ?

--
regards,
Sivakumar

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Intel-gfx] [PATCH 3/3] drm/i915: Support DDI lane reversal for DP

2015-08-26 Thread Sivakumar Thulasimani



On 8/18/2015 1:36 AM, Benjamin Tissoires wrote:

On Aug 14 2015 or thereabouts, Stéphane Marchesin wrote:

On Wed, Aug 5, 2015 at 12:34 PM, Benjamin Tissoires
benjamin.tissoi...@redhat.com wrote:

On Jul 30 2015 or thereabouts, Sivakumar Thulasimani wrote:


On 7/29/2015 8:52 PM, Benjamin Tissoires wrote:

On Jul 29 2015 or thereabouts, Sivakumar Thulasimani wrote:

why not detect reverse in intel_dp_detect/intel_hpd_pulse ? that way you can
identify both lane count and reversal state without touching anything in the
link training code. i am yet to upstream my changes for CHT that i can share
if required that does the same in intel_dp_detect without touching any line
in link training path.

With my current limited knowledge of the dp hotplug (and i915 driver) I
am not sure we could detect the reversed state without trying to train 1
lane only. I'd be glad to look at your changes and test them on my
system if you think that could help having a cleaner solution.

Cheers,
Benjamin

No, what i recommended was to do link training but in intel_dp_detect. Since
USB Type C cable
also has its own lane count restriction (it can have different lane count
than the one supported
by panel) you might have to figure that out as well. so both reversal and
lane count detection
can be done outside the modeset path and keep the code free of type C
changes outside
detection path.

Please find below the code to do the same. Do not waste time trying to apply
this directly on
nightly since this is based on a local tree and because this is pre- atomic
changes code, so you
might have to modify chv_upfront_link_train to work on top of the latest
nightly code. we
are supposed to upstream this and is in my todo list.


[original patch snipped...]

Hi Sivakumar,

So I managed to manually re-apply your patch on top of
drm-intel-nightly, and tried to port it to make Broadwell working too.
It works OK if the system is already boot without any external DP used.
In this case, the detection works and I can see my external monitor
working properly.

However, if the monitor is cold plugged, the cpu/GPU hangs and I can not
understand why. I think I enabled all that is mentioned in the PRM to be
able to train the DP link, but I am obviously missing something else.
Can you have a look?


Hi Benjamin,

I would recommend against this approach. Some adapters will claim that
they recovered a clock even when it isn't on the lanes you enabled,
which means that the reversal detection doesn't always work. The only
reliable way to do this is to go talk to the Chrome OS EC (you can
find these patches later in the Chrome OS tree). It's not as generic,
but we might be able to abstract that logic, maybe.


Hi Stéphane,

This is a very good news. I was afraid we would not have access to the
hardware controller because the Intel controller hub spec was not
public.

I will try to have a look at it, but the latest chromeos branch (3.18)
seems to differ quite a lot from the upstream one. Anyway, fingers
crossed.

Cheers,
Benjamin

Hi Benjamin/Stéphan,
All Intel platforms (at-least those i inquired about) handle lane 
reversal in HW.
your statement that link training will pass even on reversed lane seems 
to point

to the same fact. in such a scenario why should the encoder/connector care
if the lane is reversed or not ?

--
regards,
Sivakumar

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Intel-gfx] [PATCH 3/3] drm/i915: Support DDI lane reversal for DP

2015-08-26 Thread Sivakumar Thulasimani



On 8/26/2015 7:59 PM, Benjamin Tissoires wrote:

On Aug 26 2015 or thereabouts, Sivakumar Thulasimani wrote:


On 8/18/2015 1:36 AM, Benjamin Tissoires wrote:

On Aug 14 2015 or thereabouts, Stéphane Marchesin wrote:

On Wed, Aug 5, 2015 at 12:34 PM, Benjamin Tissoires
benjamin.tissoi...@redhat.com wrote:

On Jul 30 2015 or thereabouts, Sivakumar Thulasimani wrote:

On 7/29/2015 8:52 PM, Benjamin Tissoires wrote:

On Jul 29 2015 or thereabouts, Sivakumar Thulasimani wrote:

why not detect reverse in intel_dp_detect/intel_hpd_pulse ? that way you can
identify both lane count and reversal state without touching anything in the
link training code. i am yet to upstream my changes for CHT that i can share
if required that does the same in intel_dp_detect without touching any line
in link training path.

With my current limited knowledge of the dp hotplug (and i915 driver) I
am not sure we could detect the reversed state without trying to train 1
lane only. I'd be glad to look at your changes and test them on my
system if you think that could help having a cleaner solution.

Cheers,
Benjamin

No, what i recommended was to do link training but in intel_dp_detect. Since
USB Type C cable
also has its own lane count restriction (it can have different lane count
than the one supported
by panel) you might have to figure that out as well. so both reversal and
lane count detection
can be done outside the modeset path and keep the code free of type C
changes outside
detection path.

Please find below the code to do the same. Do not waste time trying to apply
this directly on
nightly since this is based on a local tree and because this is pre- atomic
changes code, so you
might have to modify chv_upfront_link_train to work on top of the latest
nightly code. we
are supposed to upstream this and is in my todo list.


[original patch snipped...]

Hi Sivakumar,

So I managed to manually re-apply your patch on top of
drm-intel-nightly, and tried to port it to make Broadwell working too.
It works OK if the system is already boot without any external DP used.
In this case, the detection works and I can see my external monitor
working properly.

However, if the monitor is cold plugged, the cpu/GPU hangs and I can not
understand why. I think I enabled all that is mentioned in the PRM to be
able to train the DP link, but I am obviously missing something else.
Can you have a look?


Hi Benjamin,

I would recommend against this approach. Some adapters will claim that
they recovered a clock even when it isn't on the lanes you enabled,
which means that the reversal detection doesn't always work. The only
reliable way to do this is to go talk to the Chrome OS EC (you can
find these patches later in the Chrome OS tree). It's not as generic,
but we might be able to abstract that logic, maybe.


Hi Stéphane,

This is a very good news. I was afraid we would not have access to the
hardware controller because the Intel controller hub spec was not
public.

I will try to have a look at it, but the latest chromeos branch (3.18)
seems to differ quite a lot from the upstream one. Anyway, fingers
crossed.

Cheers,
Benjamin

Hi Benjamin/Stéphan,

Hi Sivakumar,


 All Intel platforms (at-least those i inquired about) handle lane
reversal in HW.

That is the theory and what is written in the USB Type C spec IIRC.
Problem is, the Chromebook Pixel 2 external display does not work when a
USB Type-C adapter is in the reversed position (or believe me, I would
definitively not have submitted a patch for the beauty of it).
Everything else works (link training when 4 lanes are activated, or
other communication channels). Only the order of the 4 data lanes
matters in this situation and the USB controller does not reverse them
for us on this laptop.

your statement that link training will pass even on reversed lane seems to
point
to the same fact. in such a scenario why should the encoder/connector care
if the lane is reversed or not ?

Problem is that Stephane said some adapters are lying regarding the
clock recovery. They claim everything is fine while in the end, the
display doesn't show anything because the lanes are reversed. If this is
just a chromebook Pixel 2 issue, that's better then. I won't have to try
to put some generic interface to notify that the display port lanes have
to be reversed.

Cheers,
Benjamin
Thanks for the info :). There is no way in code to confirm if after link 
training display

is working properly (other than short pulse for link related issues)
so this has to be on a case by case basis. i would recommend
if you plan to upstream it to restrict the changes for chromebook Pixel 
2 alone.

--
regards,
Sivakumar



--
regards,
Sivakumar

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Intel-gfx] [PATCH 3/3] drm/i915: Support DDI lane reversal for DP

2015-08-06 Thread Sivakumar Thulasimani



On 8/6/2015 1:04 AM, Benjamin Tissoires wrote:

On Jul 30 2015 or thereabouts, Sivakumar Thulasimani wrote:


On 7/29/2015 8:52 PM, Benjamin Tissoires wrote:

On Jul 29 2015 or thereabouts, Sivakumar Thulasimani wrote:

why not detect reverse in intel_dp_detect/intel_hpd_pulse ? that way you can
identify both lane count and reversal state without touching anything in the
link training code. i am yet to upstream my changes for CHT that i can share
if required that does the same in intel_dp_detect without touching any line
in link training path.

With my current limited knowledge of the dp hotplug (and i915 driver) I
am not sure we could detect the reversed state without trying to train 1
lane only. I'd be glad to look at your changes and test them on my
system if you think that could help having a cleaner solution.

Cheers,
Benjamin

No, what i recommended was to do link training but in intel_dp_detect. Since
USB Type C cable
also has its own lane count restriction (it can have different lane count
than the one supported
by panel) you might have to figure that out as well. so both reversal and
lane count detection
can be done outside the modeset path and keep the code free of type C
changes outside
detection path.

Please find below the code to do the same. Do not waste time trying to apply
this directly on
nightly since this is based on a local tree and because this is pre- atomic
changes code, so you
might have to modify chv_upfront_link_train to work on top of the latest
nightly code. we
are supposed to upstream this and is in my todo list.


[original patch snipped...]

Hi Sivakumar,

So I managed to manually re-apply your patch on top of
drm-intel-nightly, and tried to port it to make Broadwell working too.
It works OK if the system is already boot without any external DP used.
In this case, the detection works and I can see my external monitor
working properly.

However, if the monitor is cold plugged, the cpu/GPU hangs and I can not
understand why. I think I enabled all that is mentioned in the PRM to be
able to train the DP link, but I am obviously missing something else.
Can you have a look?

My patch is now:




@@ -4495,7 +4499,9 @@ intel_dp_detect(struct drm_connector *connector, bool 
force)
struct intel_dp *intel_dp = intel_attached_dp(connector);
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct intel_encoder *intel_encoder = _dig_port->base;
+   struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
struct drm_device *dev = connector->dev;
+   struct intel_crtc *intel_crtc;
enum drm_connector_status status;
enum intel_display_power_domain power_domain;
bool ret;
@@ -4540,6 +4546,18 @@ intel_dp_detect(struct drm_connector *connector, bool 
force)
  
  	if (intel_encoder->type != INTEL_OUTPUT_EDP)

intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
+
+   if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT &&
+   (IS_BROADWELL(dev) || IS_CHERRYVIEW(dev))) {
+   /* Do not handle connected boot scenario where panel is enabled
+* by GOP/VBIOS.
+*/
+   if ((connector->status != connector_status_connected) &&
+   !(intel_encoder->connectors_active &&
+ crtc && crtc->enabled))
+   intel_upfront_link_train(dev, intel_dp, NULL);
+   }
+
here you are calling intel_upfront_link_train only if display is plugged 
in and there is no crtc associated with it.
hence your code is working on hotplug. modify the above to consider 
scenario when crtc is set and enabled
which happens when you plug in dp and boot the system. a good pointer is 
the original code :)

+if (intel_encoder->connectors_active &&
+crtc && crtc->enabled) {
+intel_crtc = to_intel_crtc(crtc);
+DRM_DEBUG_KMS("Disabling crtc %c for upfront link training\n",
+pipe_name(intel_crtc->pipe));
+intel_crtc_control(crtc, false);
+}

But again, all these are experimental :), any point you touch crtc it is 
not in line with the new "atomic"
thinking and will not be allowed upstream till it is fixed as per the 
new way :).




status = connector_status_connected;
  
  	/* Try to read the source of the interrupt */

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 320c9e6..fcc95d5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1163,6 +1163,13 @@ bool intel_dp_init_connector(struct intel_digital_port 
*intel_dig_port,
  void intel_dp_start_link_train(struct intel_dp *intel_dp);
  void intel_dp_complete_link_train(struct intel_dp *intel_dp);
  void intel_dp_stop_link_train(struct intel_dp *intel_dp);
+bool intel_upfront_link_train(struct drm_de

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Support DDI lane reversal for DP

2015-08-06 Thread Sivakumar Thulasimani



On 8/6/2015 1:04 AM, Benjamin Tissoires wrote:

On Jul 30 2015 or thereabouts, Sivakumar Thulasimani wrote:


On 7/29/2015 8:52 PM, Benjamin Tissoires wrote:

On Jul 29 2015 or thereabouts, Sivakumar Thulasimani wrote:

why not detect reverse in intel_dp_detect/intel_hpd_pulse ? that way you can
identify both lane count and reversal state without touching anything in the
link training code. i am yet to upstream my changes for CHT that i can share
if required that does the same in intel_dp_detect without touching any line
in link training path.

With my current limited knowledge of the dp hotplug (and i915 driver) I
am not sure we could detect the reversed state without trying to train 1
lane only. I'd be glad to look at your changes and test them on my
system if you think that could help having a cleaner solution.

Cheers,
Benjamin

No, what i recommended was to do link training but in intel_dp_detect. Since
USB Type C cable
also has its own lane count restriction (it can have different lane count
than the one supported
by panel) you might have to figure that out as well. so both reversal and
lane count detection
can be done outside the modeset path and keep the code free of type C
changes outside
detection path.

Please find below the code to do the same. Do not waste time trying to apply
this directly on
nightly since this is based on a local tree and because this is pre- atomic
changes code, so you
might have to modify chv_upfront_link_train to work on top of the latest
nightly code. we
are supposed to upstream this and is in my todo list.


[original patch snipped...]

Hi Sivakumar,

So I managed to manually re-apply your patch on top of
drm-intel-nightly, and tried to port it to make Broadwell working too.
It works OK if the system is already boot without any external DP used.
In this case, the detection works and I can see my external monitor
working properly.

However, if the monitor is cold plugged, the cpu/GPU hangs and I can not
understand why. I think I enabled all that is mentioned in the PRM to be
able to train the DP link, but I am obviously missing something else.
Can you have a look?

My patch is now:

/snipping some more


@@ -4495,7 +4499,9 @@ intel_dp_detect(struct drm_connector *connector, bool 
force)
struct intel_dp *intel_dp = intel_attached_dp(connector);
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct intel_encoder *intel_encoder = intel_dig_port-base;
+   struct drm_crtc *crtc = intel_dig_port-base.base.crtc;
struct drm_device *dev = connector-dev;
+   struct intel_crtc *intel_crtc;
enum drm_connector_status status;
enum intel_display_power_domain power_domain;
bool ret;
@@ -4540,6 +4546,18 @@ intel_dp_detect(struct drm_connector *connector, bool 
force)
  
  	if (intel_encoder-type != INTEL_OUTPUT_EDP)

intel_encoder-type = INTEL_OUTPUT_DISPLAYPORT;
+
+   if (intel_encoder-type == INTEL_OUTPUT_DISPLAYPORT 
+   (IS_BROADWELL(dev) || IS_CHERRYVIEW(dev))) {
+   /* Do not handle connected boot scenario where panel is enabled
+* by GOP/VBIOS.
+*/
+   if ((connector-status != connector_status_connected) 
+   !(intel_encoder-connectors_active 
+ crtc  crtc-enabled))
+   intel_upfront_link_train(dev, intel_dp, NULL);
+   }
+
here you are calling intel_upfront_link_train only if display is plugged 
in and there is no crtc associated with it.
hence your code is working on hotplug. modify the above to consider 
scenario when crtc is set and enabled
which happens when you plug in dp and boot the system. a good pointer is 
the original code :)

+if (intel_encoder-connectors_active 
+crtc  crtc-enabled) {
+intel_crtc = to_intel_crtc(crtc);
+DRM_DEBUG_KMS(Disabling crtc %c for upfront link training\n,
+pipe_name(intel_crtc-pipe));
+intel_crtc_control(crtc, false);
+}

But again, all these are experimental :), any point you touch crtc it is 
not in line with the new atomic
thinking and will not be allowed upstream till it is fixed as per the 
new way :).




status = connector_status_connected;
  
  	/* Try to read the source of the interrupt */

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 320c9e6..fcc95d5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1163,6 +1163,13 @@ bool intel_dp_init_connector(struct intel_digital_port 
*intel_dig_port,
  void intel_dp_start_link_train(struct intel_dp *intel_dp);
  void intel_dp_complete_link_train(struct intel_dp *intel_dp);
  void intel_dp_stop_link_train(struct intel_dp *intel_dp);
+bool intel_upfront_link_train(struct drm_device *dev,
+ struct intel_dp *intel_dp,
+ struct

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Support DDI lane reversal for DP

2015-07-29 Thread Sivakumar Thulasimani



On 7/29/2015 8:52 PM, Benjamin Tissoires wrote:

On Jul 29 2015 or thereabouts, Sivakumar Thulasimani wrote:

why not detect reverse in intel_dp_detect/intel_hpd_pulse ? that way you can
identify both lane count and reversal state without touching anything in the
link training code. i am yet to upstream my changes for CHT that i can share
if required that does the same in intel_dp_detect without touching any line
in link training path.

With my current limited knowledge of the dp hotplug (and i915 driver) I
am not sure we could detect the reversed state without trying to train 1
lane only. I'd be glad to look at your changes and test them on my
system if you think that could help having a cleaner solution.

Cheers,
Benjamin
No, what i recommended was to do link training but in intel_dp_detect. 
Since USB Type C cable
also has its own lane count restriction (it can have different lane 
count than the one supported
by panel) you might have to figure that out as well. so both reversal 
and lane count detection
can be done outside the modeset path and keep the code free of type C 
changes outside

detection path.

Please find below the code to do the same. Do not waste time trying to 
apply this directly on
nightly since this is based on a local tree and because this is pre- 
atomic changes code, so you
might have to modify chv_upfront_link_train to work on top of the latest 
nightly code. we

are supposed to upstream this and is in my todo list.

---

Author: Durgadoss R 
Date:   Fri May 22 14:30:07 2015 +0530

   drm/i915: Enable Upfront link training for type-C DP support

To support USB type C alternate DP mode, the display driver needs 
to know the
number of lanes required by DP panel as well as number of lanes 
that can be
supported by the type-C cable. Sometimes, the type-C cable may 
limit the
bandwidth even if Panel can support more lanes. To address these 
scenarios,
the display driver will start link training with max lanes, and if 
the link

training fails, the driver then falls back to x2 lanes.

* Since link training is done before modeset, planes are not 
enabled. Only

  encoder and the its associated PLLs are enabled.
* Once link training is done, the encoder and its PLLs are 
disabled; so that

  the subsequent modeset is not aware of these changes.
* As of now, this is tested only on CHV.

Signed-off-by: Durgadoss R 

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c

index 0c8ae2a..c72dcaa 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14793,3 +14793,121 @@ intel_display_print_error_state(struct 
drm_i915_error_state_buf *m,

 err_printf(m, "  VSYNC: %08x\n", error->transcoder[i].vsync);
 }
 }
+
+bool chv_upfront_link_train(struct drm_device *dev,
+struct intel_dp *intel_dp, struct intel_crtc *crtc)
+{
+struct drm_i915_private *dev_priv = dev->dev_private;
+struct intel_connector *connector = intel_dp->attached_connector;
+struct intel_encoder *encoder = connector->encoder;
+bool found = false;
+bool valid_crtc = false;
+
+if (!connector || !encoder) {
+DRM_DEBUG_KMS("dp connector/encoder is NULL\n");
+return false;
+}
+
+/* If we already have a crtc, start link training directly */
+if (crtc) {
+valid_crtc = true;
+goto start_link_train;
+}
+
+/* Find an unused crtc and use it for link training */
+for_each_intel_crtc(dev, crtc) {
+if (intel_crtc_active(>base))
+continue;
+
+connector->new_encoder = encoder;
+encoder->new_crtc = crtc;
+encoder->base.crtc = >base;
+
+/* Make sure the new crtc will work with the encoder */
+if (drm_encoder_crtc_ok(>base,
+ >base)) {
+found = true;
+break;
+}
+}
+
+if (!found) {
+DRM_ERROR("Could not find crtc for upfront link training\n");
+return false;
+}
+
+start_link_train:
+
+DRM_DEBUG_KMS("upfront link training on pipe:%c\n",
+pipe_name(crtc->pipe));
+found = false;
+
+/* Initialize with Max Link rate & lane count supported by panel */
+intel_dp->link_bw =  intel_dp->dpcd[DP_MAX_LINK_RATE];
+intel_dp->lane_count = intel_dp->dpcd[DP_MAX_LANE_COUNT] &
+DP_MAX_LANE_COUNT_MASK;
+
+do {
+/* Find port clock from link_bw */
+crtc->config.port_clock =
+drm_dp_bw_code_to_link_rate(intel_dp->link_bw);
+
+/* Enable PLL followed by port */
+intel_dp_set_clock(encoder, >config, intel_dp->link_bw);
+chv_update_pll(crtc);
+encoder->pre

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Support DDI lane reversal for DP

2015-07-29 Thread Sivakumar Thulasimani
why not detect reverse in intel_dp_detect/intel_hpd_pulse ? that way you 
can identify both lane count and reversal state without touching 
anything in the link training code. i am yet to upstream my changes for 
CHT that i can share if required that does the same in intel_dp_detect 
without touching any line in link training path.


On 7/28/2015 9:33 PM, Benjamin Tissoires wrote:

The DP outputs connected through a USB Type-C port can have inverted
lanes. To detect that case, we implement autodetection by training only
the first lane if it doesn't work, we assume that we need to invert
the lanes.

Tested on a Chromebook Pixel 2015 (samus) with a USB Type-C to HDMI
adapter and a Dell 4K and some various regular monitors.

Based on 2 patches from the ChromeOS tree by:
Stéphane Marchesin 
Todd Broch 

Signed-off-by: Benjamin Tissoires 
---
  drivers/gpu/drm/i915/intel_ddi.c | 13 +
  drivers/gpu/drm/i915/intel_dp.c  | 36 
  drivers/gpu/drm/i915/intel_drv.h |  1 +
  3 files changed, 50 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 9a40bfb..0b0c1ec 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2249,6 +2249,7 @@ static void intel_ddi_pre_enable(struct intel_encoder 
*intel_encoder)
enum port port = intel_ddi_get_encoder_port(intel_encoder);
int type = intel_encoder->type;
int hdmi_level;
+   bool reversed = false;
  
  	if (type == INTEL_OUTPUT_EDP) {

struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
@@ -2295,8 +2296,20 @@ static void intel_ddi_pre_enable(struct intel_encoder 
*intel_encoder)
if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
  
+		if (IS_BROADWELL(dev) && type == INTEL_OUTPUT_DISPLAYPORT) {

+   intel_ddi_init_dp_buf_reg(intel_encoder);
+   reversed = intel_dp_is_reversed(intel_dp);
+   }
+
intel_ddi_init_dp_buf_reg(intel_encoder);
  
+		if (IS_BROADWELL(dev)) {

+   if (reversed)
+   intel_dp->DP |= DDI_BUF_PORT_REVERSAL;
+   else
+   intel_dp->DP &= ~DDI_BUF_PORT_REVERSAL;
+   }
+
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
intel_dp_start_link_train(intel_dp);
intel_dp_complete_link_train(intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b740987..18280cc 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3820,6 +3820,42 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
intel_dp->DP = DP;
  }
  
+bool intel_dp_is_reversed(struct intel_dp *intel_dp)

+{
+   struct drm_encoder *encoder = _to_dig_port(intel_dp)->base.base;
+   struct drm_device *dev = encoder->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   uint32_t DP = intel_dp->DP;
+
+   /*
+* Train with 1 lane. There is no guarantee that the monitor supports
+* 2 or 4 lanes, and we wouldn't see any asymetricity with 4 lanes.
+*/
+   const uint8_t lane_count = 1;
+   bool reversed;
+
+   if (!HAS_DDI(dev))
+   return false;
+
+   DP &= ~(DDI_BUF_PORT_REVERSAL | DDI_PORT_WIDTH(4));
+   DP |= DDI_PORT_WIDTH(lane_count);
+
+   I915_WRITE(intel_dp->output_reg, DP);
+   POSTING_READ(intel_dp->output_reg);
+   udelay(600);
+
+   if (!_intel_dp_start_link_train(intel_dp, lane_count, , true))
+   return true;
+
+   reversed = !_intel_dp_complete_link_train(intel_dp, lane_count, , 
true);
+
+   /* clear training, we had only one lane */
+   intel_dp->train_set_valid = false;
+
+   return reversed;
+
+}
+
  void intel_dp_stop_link_train(struct intel_dp *intel_dp)
  {
intel_dp_set_link_train(intel_dp, _dp->DP,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 320c9e6..cba00c6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1169,6 +1169,7 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc);
  bool intel_dp_compute_config(struct intel_encoder *encoder,
 struct intel_crtc_state *pipe_config);
  bool intel_dp_is_edp(struct drm_device *dev, enum port port);
+bool intel_dp_is_reversed(struct intel_dp *intel_dp);
  enum irqreturn intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port,
  bool long_hpd);
  void intel_edp_backlight_on(struct intel_dp *intel_dp);


--
regards,
Sivakumar

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Support DDI lane reversal for DP

2015-07-29 Thread Sivakumar Thulasimani



On 7/29/2015 8:52 PM, Benjamin Tissoires wrote:

On Jul 29 2015 or thereabouts, Sivakumar Thulasimani wrote:

why not detect reverse in intel_dp_detect/intel_hpd_pulse ? that way you can
identify both lane count and reversal state without touching anything in the
link training code. i am yet to upstream my changes for CHT that i can share
if required that does the same in intel_dp_detect without touching any line
in link training path.

With my current limited knowledge of the dp hotplug (and i915 driver) I
am not sure we could detect the reversed state without trying to train 1
lane only. I'd be glad to look at your changes and test them on my
system if you think that could help having a cleaner solution.

Cheers,
Benjamin
No, what i recommended was to do link training but in intel_dp_detect. 
Since USB Type C cable
also has its own lane count restriction (it can have different lane 
count than the one supported
by panel) you might have to figure that out as well. so both reversal 
and lane count detection
can be done outside the modeset path and keep the code free of type C 
changes outside

detection path.

Please find below the code to do the same. Do not waste time trying to 
apply this directly on
nightly since this is based on a local tree and because this is pre- 
atomic changes code, so you
might have to modify chv_upfront_link_train to work on top of the latest 
nightly code. we

are supposed to upstream this and is in my todo list.

---

Author: Durgadoss R durgados...@intel.com
Date:   Fri May 22 14:30:07 2015 +0530

   drm/i915: Enable Upfront link training for type-C DP support

To support USB type C alternate DP mode, the display driver needs 
to know the
number of lanes required by DP panel as well as number of lanes 
that can be
supported by the type-C cable. Sometimes, the type-C cable may 
limit the
bandwidth even if Panel can support more lanes. To address these 
scenarios,
the display driver will start link training with max lanes, and if 
the link

training fails, the driver then falls back to x2 lanes.

* Since link training is done before modeset, planes are not 
enabled. Only

  encoder and the its associated PLLs are enabled.
* Once link training is done, the encoder and its PLLs are 
disabled; so that

  the subsequent modeset is not aware of these changes.
* As of now, this is tested only on CHV.

Signed-off-by: Durgadoss R durgados...@intel.com

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c

index 0c8ae2a..c72dcaa 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14793,3 +14793,121 @@ intel_display_print_error_state(struct 
drm_i915_error_state_buf *m,

 err_printf(m,   VSYNC: %08x\n, error-transcoder[i].vsync);
 }
 }
+
+bool chv_upfront_link_train(struct drm_device *dev,
+struct intel_dp *intel_dp, struct intel_crtc *crtc)
+{
+struct drm_i915_private *dev_priv = dev-dev_private;
+struct intel_connector *connector = intel_dp-attached_connector;
+struct intel_encoder *encoder = connector-encoder;
+bool found = false;
+bool valid_crtc = false;
+
+if (!connector || !encoder) {
+DRM_DEBUG_KMS(dp connector/encoder is NULL\n);
+return false;
+}
+
+/* If we already have a crtc, start link training directly */
+if (crtc) {
+valid_crtc = true;
+goto start_link_train;
+}
+
+/* Find an unused crtc and use it for link training */
+for_each_intel_crtc(dev, crtc) {
+if (intel_crtc_active(crtc-base))
+continue;
+
+connector-new_encoder = encoder;
+encoder-new_crtc = crtc;
+encoder-base.crtc = crtc-base;
+
+/* Make sure the new crtc will work with the encoder */
+if (drm_encoder_crtc_ok(encoder-base,
+ crtc-base)) {
+found = true;
+break;
+}
+}
+
+if (!found) {
+DRM_ERROR(Could not find crtc for upfront link training\n);
+return false;
+}
+
+start_link_train:
+
+DRM_DEBUG_KMS(upfront link training on pipe:%c\n,
+pipe_name(crtc-pipe));
+found = false;
+
+/* Initialize with Max Link rate  lane count supported by panel */
+intel_dp-link_bw =  intel_dp-dpcd[DP_MAX_LINK_RATE];
+intel_dp-lane_count = intel_dp-dpcd[DP_MAX_LANE_COUNT] 
+DP_MAX_LANE_COUNT_MASK;
+
+do {
+/* Find port clock from link_bw */
+crtc-config.port_clock =
+drm_dp_bw_code_to_link_rate(intel_dp-link_bw);
+
+/* Enable PLL followed by port */
+intel_dp_set_clock(encoder, crtc-config, intel_dp-link_bw);
+chv_update_pll(crtc);
+encoder-pre_pll_enable(encoder);
+chv_enable_pll(crtc);
+encoder

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Support DDI lane reversal for DP

2015-07-29 Thread Sivakumar Thulasimani
why not detect reverse in intel_dp_detect/intel_hpd_pulse ? that way you 
can identify both lane count and reversal state without touching 
anything in the link training code. i am yet to upstream my changes for 
CHT that i can share if required that does the same in intel_dp_detect 
without touching any line in link training path.


On 7/28/2015 9:33 PM, Benjamin Tissoires wrote:

The DP outputs connected through a USB Type-C port can have inverted
lanes. To detect that case, we implement autodetection by training only
the first lane if it doesn't work, we assume that we need to invert
the lanes.

Tested on a Chromebook Pixel 2015 (samus) with a USB Type-C to HDMI
adapter and a Dell 4K and some various regular monitors.

Based on 2 patches from the ChromeOS tree by:
Stéphane Marchesin marc...@chromium.org
Todd Broch tbr...@chromium.org

Signed-off-by: Benjamin Tissoires benjamin.tissoi...@redhat.com
---
  drivers/gpu/drm/i915/intel_ddi.c | 13 +
  drivers/gpu/drm/i915/intel_dp.c  | 36 
  drivers/gpu/drm/i915/intel_drv.h |  1 +
  3 files changed, 50 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 9a40bfb..0b0c1ec 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2249,6 +2249,7 @@ static void intel_ddi_pre_enable(struct intel_encoder 
*intel_encoder)
enum port port = intel_ddi_get_encoder_port(intel_encoder);
int type = intel_encoder-type;
int hdmi_level;
+   bool reversed = false;
  
  	if (type == INTEL_OUTPUT_EDP) {

struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
@@ -2295,8 +2296,20 @@ static void intel_ddi_pre_enable(struct intel_encoder 
*intel_encoder)
if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
  
+		if (IS_BROADWELL(dev)  type == INTEL_OUTPUT_DISPLAYPORT) {

+   intel_ddi_init_dp_buf_reg(intel_encoder);
+   reversed = intel_dp_is_reversed(intel_dp);
+   }
+
intel_ddi_init_dp_buf_reg(intel_encoder);
  
+		if (IS_BROADWELL(dev)) {

+   if (reversed)
+   intel_dp-DP |= DDI_BUF_PORT_REVERSAL;
+   else
+   intel_dp-DP = ~DDI_BUF_PORT_REVERSAL;
+   }
+
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
intel_dp_start_link_train(intel_dp);
intel_dp_complete_link_train(intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b740987..18280cc 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3820,6 +3820,42 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
intel_dp-DP = DP;
  }
  
+bool intel_dp_is_reversed(struct intel_dp *intel_dp)

+{
+   struct drm_encoder *encoder = dp_to_dig_port(intel_dp)-base.base;
+   struct drm_device *dev = encoder-dev;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   uint32_t DP = intel_dp-DP;
+
+   /*
+* Train with 1 lane. There is no guarantee that the monitor supports
+* 2 or 4 lanes, and we wouldn't see any asymetricity with 4 lanes.
+*/
+   const uint8_t lane_count = 1;
+   bool reversed;
+
+   if (!HAS_DDI(dev))
+   return false;
+
+   DP = ~(DDI_BUF_PORT_REVERSAL | DDI_PORT_WIDTH(4));
+   DP |= DDI_PORT_WIDTH(lane_count);
+
+   I915_WRITE(intel_dp-output_reg, DP);
+   POSTING_READ(intel_dp-output_reg);
+   udelay(600);
+
+   if (!_intel_dp_start_link_train(intel_dp, lane_count, DP, true))
+   return true;
+
+   reversed = !_intel_dp_complete_link_train(intel_dp, lane_count, DP, 
true);
+
+   /* clear training, we had only one lane */
+   intel_dp-train_set_valid = false;
+
+   return reversed;
+
+}
+
  void intel_dp_stop_link_train(struct intel_dp *intel_dp)
  {
intel_dp_set_link_train(intel_dp, intel_dp-DP,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 320c9e6..cba00c6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1169,6 +1169,7 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc);
  bool intel_dp_compute_config(struct intel_encoder *encoder,
 struct intel_crtc_state *pipe_config);
  bool intel_dp_is_edp(struct drm_device *dev, enum port port);
+bool intel_dp_is_reversed(struct intel_dp *intel_dp);
  enum irqreturn intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port,
  bool long_hpd);
  void intel_edp_backlight_on(struct intel_dp *intel_dp);


--
regards,
Sivakumar

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to