Re: [PATCH 4/4] drm/i915/dp: read DPCD PSR capability only on eDP

Daniel Vetter
2013-09-23T07:29:33Z

On Fri, Sep 20, 2013 at 03:31:42PM -0700, Todd Previte wrote:
> On 09/20/2013 01:57 PM, Paulo Zanoni wrote:
> >2013/9/20 Todd Previte <tprevite@gmail.com>:
> >>On 09/20/2013 06:42 AM, Jani Nikula wrote:
> >>>Reduce AUX transactions for non-eDP.
> >>>
> >>>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >>>---
> >>> drivers/gpu/drm/i915/intel_dp.c | 13 ++++++++-----
> >>> 1 file changed, 8 insertions(+), 5 deletions(-)
> >>>
> >>>diff --git a/drivers/gpu/drm/i915/intel_dp.c
> >>>b/drivers/gpu/drm/i915/intel_dp.c
> >>>index dd780bf..f2e16a1 100644
> >>>--- a/drivers/gpu/drm/i915/intel_dp.c
> >>>+++ b/drivers/gpu/drm/i915/intel_dp.c
> >>>@@ -2691,11 +2691,14 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
> >>> /* Check if the panel supports PSR */
> >>> memset(intel_dp->psr_dpcd, 0, sizeof(intel_dp->psr_dpcd));
> >>>- intel_dp_aux_native_read_retry(intel_dp, DP_PSR_SUPPORT,
> >>>- intel_dp->psr_dpcd,
> >>>- sizeof(intel_dp->psr_dpcd));
> >>>- if (is_edp_psr(intel_dp))
> >>>- DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
> >>>+ if (is_edp(intel_dp)) {
> >>>+ intel_dp_aux_native_read_retry(intel_dp, DP_PSR_SUPPORT,
> >>>+ intel_dp->psr_dpcd,
> >>>+
> >>>sizeof(intel_dp->psr_dpcd));
> >>>+ if (is_edp_psr(intel_dp))
> >>>+ DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
> >>>+ }
> >>>+
> >>> if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> >>> DP_DWN_STRM_PORT_PRESENT))
> >>> return true; /* native DP sink */
> >>
> >>Couldn't you just use is_edp_psr() at the top and not need the second
> >>is_edp() inside the if() statement? As in:
> >No, because is_edp_psr() only makes sense after we do the DPCD read
> >that checks the PSR bit.
>
> Yep, I see that now. And thus...
>
> [Reviewed-by]: Todd Previte <tprevite@gmail.com>

Queued for -next, thanks for the patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch