On Monday 18 January 2016 06:30 PM, Ander Conselvan De Oliveira wrote:
On Mon, 2016-01-18 at 18:14 +0530, Shubhangi Shrivastava wrote:
On Thursday 14 January 2016 06:34 PM, Ander Conselvan De Oliveira wrote:
On Tue, 2016-01-05 at 18:20 +0530, Shubhangi Shrivastava wrote:
This patch reads sink_count dpcd always and removes its
read operation based on values in downstream port dpcd.
SINK_COUNT dpcd is not dependent on DOWNSTREAM_PORT_PRESENT dpcd.
SINK_COUNT denotes if a display is attached, while
DOWNSTREAM_PORT_PRESET indicates how many ports are available
in the dongle where display can be attached. so it is possible
for sink count to change irrespective of value in downstream
port dpcd.
Here is a table of possible values and scenarios
sink_count downstream_port
present
0 0 no display is attached
0 1 dongle is connected without display
1 0 display connected directly
1 1 display connected through dongle
Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@xxxxxxxxx>
Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@xxxxxxxxx>
Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@xxxxxxxxx>
---
drivers/gpu/drm/i915/intel_dp.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
index c2e8516..0d58bfd 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3865,6 +3865,13 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
if (intel_dp->dpcd[DP_DPCD_REV] == 0)
return false; /* DPCD not present */
+ if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT,
+ &intel_dp->sink_count, 1) < 0)
+ return false;
+
+ if (!DP_GET_SINK_COUNT(intel_dp->sink_count))
+ return false;
+
My understanding is that this function should only read the DPCD data while
detection based on that data is done in intel_dp_detect_dpcd(). With the
return
on sink_count == 0 here, we skip the end of the function, which updates the
cached downstream port information. Is there a reason why we need this early
return here?
Also, I think this could be squashed with the previous patch.
Ander
As described in the commit message, if sink_count is 0, then there is no
display present. So, irrespective of value of downstream port, we should
terminate the function and thus, an early return is present here.
You wrote that "SINK_COUNT dpcd is not dependent on DOWNSTREAM_PORT_PRESENT
dpcd". Now, the get_dpcd() function is called from different places with the
purpose of retrieving information stored in dpcd. By adding the early return,
the downstream port information, which you claimed is independent from sink
count, is not updated.
The way I see it, you should terminate detection when sink count is 0, not the
reading of DPCD. That way the logical split between intel_dp_get_dpcd() and
intel_dp_detect_dpcd() is maintained. The former reads DPCD and the latter
reasons about it.
Ander
Yes, that's how it is.. But, SINK_COUNT == 0 and DOWNSTREAM_PORT_PRESENT
== 1 implies that a dongle is present but no display. Unless we require
to know if a dongle is present or not, we don't need to update
downstream port information. So, an early return here saves time from
performing other operations which are not required.
/* Check if the panel supports PSR */
memset(intel_dp->psr_dpcd, 0, sizeof(intel_dp->psr_dpcd));
if (is_edp(intel_dp)) {
@@ -4386,10 +4393,6 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) {
- if (intel_dp_dpcd_read_wake(&intel_dp->aux,
DP_SINK_COUNT,
- &intel_dp->sink_count, 1) <
0)
- return connector_status_unknown;
-
return DP_GET_SINK_COUNT(intel_dp->sink_count) ?
connector_status_connected :
connector_status_disconnected;
}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
