| Message ID | 1453199833-10401-4-git-send-email-shubhangi.shrivastava@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
The patch summary is now too long. Just use the one from patch 5 ("read
sink_count dpcd always"). You can mention in the commit message that the value
of sink count is now stored in struct intel_dp, but that doesn't need to be in
the summary.
One more comment below.
On Tue, 2016-01-19 at 16:07 +0530, Shubhangi Shrivastava wrote:
> Sink count can change between short pulse hpd hence this patch
> adds a member variable to intel_dp so we can track any changes
> between short pulse interrupts.
>
> 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
>
> v2: Storing value of intel_dp->sink_count that is ready
> for consumption. (Ander)
> Squashing two commits into one. (Ander)
>
> Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@intel.com>
> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 18 +++++++++++-------
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> 2 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index f8d9611..cdf4919 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3855,6 +3855,15 @@ 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;
> +
> + intel_dp->sink_count = DP_GET_SINK_COUNT(intel_dp->sink_count);
> +
> + if (!intel_dp->sink_count)
> + return false;
> +
You convinced me on the other email about this early return. Just add a comment
here with that justification.
With those two things, this is
Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> /* Check if the panel supports PSR */
> memset(intel_dp->psr_dpcd, 0, sizeof(intel_dp->psr_dpcd));
> if (is_edp(intel_dp)) {
> @@ -4372,14 +4381,9 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
> /* If we're HPD-aware, SINK_COUNT changes dynamically */
> if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
> intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) {
> - uint8_t reg;
> -
> - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT,
> - ®, 1) < 0)
> - return connector_status_unknown;
>
> - return DP_GET_SINK_COUNT(reg) ? connector_status_connected
> - :
> connector_status_disconnected;
> + return intel_dp->sink_count ?
> + connector_status_connected : connector_status_disconnected;
> }
>
> /* If no HPD, poke DDC gently */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 059b46e..0879466 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -774,6 +774,7 @@ struct intel_dp {
> uint32_t DP;
> int link_rate;
> uint8_t lane_count;
> + uint8_t sink_count;
> bool has_audio;
> enum hdmi_force_audio force_audio;
> bool limited_color_range;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index f8d9611..cdf4919 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3855,6 +3855,15 @@ 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; + + intel_dp->sink_count = DP_GET_SINK_COUNT(intel_dp->sink_count); + + if (!intel_dp->sink_count) + return false; + /* Check if the panel supports PSR */ memset(intel_dp->psr_dpcd, 0, sizeof(intel_dp->psr_dpcd)); if (is_edp(intel_dp)) { @@ -4372,14 +4381,9 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) /* If we're HPD-aware, SINK_COUNT changes dynamically */ if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) { - uint8_t reg; - - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT, - ®, 1) < 0) - return connector_status_unknown; - return DP_GET_SINK_COUNT(reg) ? connector_status_connected - : connector_status_disconnected; + return intel_dp->sink_count ? + connector_status_connected : connector_status_disconnected; } /* If no HPD, poke DDC gently */ diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 059b46e..0879466 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -774,6 +774,7 @@ struct intel_dp { uint32_t DP; int link_rate; uint8_t lane_count; + uint8_t sink_count; bool has_audio; enum hdmi_force_audio force_audio; bool limited_color_range;
Sink count can change between short pulse hpd hence this patch adds a member variable to intel_dp so we can track any changes between short pulse interrupts. 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 v2: Storing value of intel_dp->sink_count that is ready for consumption. (Ander) Squashing two commits into one. (Ander) Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@intel.com> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 18 +++++++++++------- drivers/gpu/drm/i915/intel_drv.h | 1 + 2 files changed, 12 insertions(+), 7 deletions(-)