diff mbox series

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

Message ID 20181031004517.17250-2-anusha.srivatsa@intel.com (mailing list archive)
State New, archived
Headers show
Series Forward Error Correction | expand

Commit Message

Srivatsa, Anusha Oct. 31, 2018, 12:45 a.m. UTC 
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(+)

Comments

Navare, Manasi Nov. 1, 2018, 10:31 p.m. UTC | #1 
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.
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
>
Srivatsa, Anusha Nov. 1, 2018, 11:02 p.m. UTC | #2 
>-----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?

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
>>
Navare, Manasi Nov. 1, 2018, 11:15 p.m. UTC | #3 
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
> >>
diff mbox series

Patch

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");
+		}
 	}
 }
 
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;