Re: [v4 1/7] i915/dp/fec: Cache the FEC_CAPABLE DPCD register

[Manasi Navare <manasi.d.navare@intel.com>]

On Thu, Nov 01, 2018 at 04:02:48PM -0700, Srivatsa, Anusha wrote:
> 
> 
> >-----Original Message-----
> >From: Navare, Manasi D
> >Sent: Thursday, November 1, 2018 3:31 PM
> >To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> >Cc: intel-gfx@lists.freedesktop.org; Jani Nikula <jani.nikula@linux.intel.com>;
> >Ville Syrjala <ville.syrjala@linux.intel.com>
> >Subject: Re: [v4 1/7] i915/dp/fec: Cache the FEC_CAPABLE DPCD register
> >
> >On Tue, Oct 30, 2018 at 05:45:11PM -0700, Anusha Srivatsa wrote:
> >> Similar to DSC DPCD registers, let us cache FEC_CAPABLE register to
> >> avoid using stale values. With this we can avoid aux reads everytime
> >> and instead read the cached values.
> >>
> >> v2: Avoid using memset and array for a single field. (Manasi,Jani)
> >>
> >> v3:
> >>
> >> Suggested-by: Jani Nikula <jani.nikula@linux.intel.com>
> >> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> >> Cc: Manasi Navare <manasi.d.navare@intel.com>
> >> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_dp.c  | 9 +++++++++
> >> drivers/gpu/drm/i915/intel_drv.h | 1 +
> >>  2 files changed, 10 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> >> b/drivers/gpu/drm/i915/intel_dp.c index 5a638503e36a..8ae7cf3d4ee1
> >> 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -4201,6 +4201,9 @@ static void intel_dp_get_dsc_sink_cap(struct intel_dp
> >*intel_dp)
> >>  	 */
> >>  	memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd));
> >>
> >> +	/* Clear fec_capable to avoid using stale values */
> >> +	intel_dp->fec_capable = 0;
> >> +
> >>  	/* Cache the DSC DPCD if eDP or DP rev >= 1.4 */
> >>  	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x14 ||
> >>  	    intel_dp->edp_dpcd[0] >= DP_EDP_14) { @@ -4213,6 +4216,12 @@
> >> static void intel_dp_get_dsc_sink_cap(struct intel_dp *intel_dp)
> >>  		DRM_DEBUG_KMS("DSC DPCD: %*ph\n",
> >>  			      (int)sizeof(intel_dp->dsc_dpcd),
> >>  			      intel_dp->dsc_dpcd);
> >> +		/* FEC is supported only on DP 1.4 */
> >> +		if (!intel_dp_is_edp(intel_dp)) {
> >> +			if (drm_dp_dpcd_readb(&intel_dp->aux,
> >DP_FEC_CAPABILITY,
> >> +					      &intel_dp->fec_capable) < 0)
> >> +				DRM_ERROR("Failed to read FEC DPCD
> >register\n");
> >
> >Would be nice to print the value of FEC_CAPABLE in th above DRM_DEBUG_KMS
> >right after DSC DPCD.
> 
> So unlike DSC DPCD, the FEC_CAPABLE is just one byte. Would we gain much by printing the value?
>

I think it will still be useful since its the contents of DPCD regsiter and will be printed where
we print all DPCDs, DSC DPCD and then this.
Else for debugging, people will need to add a debug print after the macro and rebuild the kernel.

Manasi
 
> Anusha 
> >With that change:
> >
> >Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
> >
> >You can make this change, add my r-b , rebase and submit 1st and 2nd patch with
> >CI prefix.
> >
> >Manasi
> >
> >> +		}
> >>  	}
> >>  }
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> >> b/drivers/gpu/drm/i915/intel_drv.h
> >> index 16bbc3768e02..9a94c6544bf5 100644
> >> --- a/drivers/gpu/drm/i915/intel_drv.h
> >> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >> @@ -1119,6 +1119,7 @@ struct intel_dp {
> >>  	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
> >>  	uint8_t edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
> >>  	u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE];
> >> +	u8 fec_capable;
> >>  	/* source rates */
> >>  	int num_source_rates;
> >>  	const int *source_rates;
> >> --
> >> 2.17.1
> >>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx