Re: [Intel-gfx] [PATCH 3/3] drm/i915/dp: intel_dp connector hook for VRR support

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 17 Mar 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
> This defines the get_vrr_support hook for intel DP connector
> VRR support is set to true based on the DPCD ignore MSA and
> EDID monitor range

Yeah... but what do you use it for?

>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Nicholas Kazlauskas <Nicholas.Kazlauskas@amd.com>
> Cc: Aditya Swarup <aditya.swarup@intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  .../drm/i915/display/intel_display_types.h    |  3 +++
>  drivers/gpu/drm/i915/display/intel_dp.c       | 19 +++++++++++++++++++
>  2 files changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 5e00e611f077..cd37ee6db1ff 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1353,6 +1353,9 @@ struct intel_dp {
>  
>  	/* Display stream compression testing */
>  	bool force_dsc_en;
> +
> +	/* DP Variable refresh rate/ Adaptive sync support */
> +	bool vrr_capable;

Only set, never read.

>  };
>  
>  enum lspcon_vendor {
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 0a417cd2af2b..ccf5d868b5c1 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5860,6 +5860,24 @@ static int intel_dp_get_modes(struct drm_connector *connector)
>  	return 0;
>  }
>  
> +static void intel_dp_get_vrr_support(struct drm_connector *connector)
> +{
> +	struct intel_dp *intel_dp = intel_attached_dp(to_intel_connector(connector));
> +	const struct drm_display_info *info = &connector->display_info;
> +	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> +
> +	/*
> +	 * DP Sink is capable of Variable refresh video timings if
> +	 * Ignore MSA bit is set in DPCD.
> +	 * EDID monitor range also should be atleast 10 for reasonable
> +	 * Adaptive sync/ VRR end user experience.
> +	 */
> +	if (INTEL_GEN(dev_priv) >= 12 &&
> +	    drm_dp_sink_is_capable_without_timing_msa(intel_dp->dpcd) &&
> +	    info->monitor_range.max_vfreq - info->monitor_range.min_vfreq > 10)
> +		intel_dp->vrr_capable = true;

So for now this is just a cached value for i915 use. I don't know what
you'll need it for, but you also don't explain why it needs to be
*cached* instead of having a helper to tell you based on the above
data. You only ever set ->vrr_capable to true, but you never reset it
back to false e.g. when the display is changed on the connector.

Furthermore, because of the placing of the hook call in the previous
patch, this will only use whatever details ->get_modes gives you, not
the fallback data.

BR,
Jani.


> +}
> +
>  static int
>  intel_dp_connector_register(struct drm_connector *connector)
>  {
> @@ -6756,6 +6774,7 @@ static const struct drm_connector_helper_funcs intel_dp_connector_helper_funcs =
>  	.get_modes = intel_dp_get_modes,
>  	.mode_valid = intel_dp_mode_valid,
>  	.atomic_check = intel_dp_connector_atomic_check,
> +	.get_adaptive_sync_support = intel_dp_get_vrr_support,
>  };
>  
>  static const struct drm_encoder_funcs intel_dp_enc_funcs = {

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx