Re: [PATCH v2] drm/dp: Add function to parse EDID descriptors for adaptive sync limits

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jan 14, 2020 at 08:31:22AM -0500, Harry Wentland wrote:
> Fixing Nick's email.
> 
> On 2020-01-10 5:43 p.m., Manasi Navare wrote:
> > On Thu, Jan 09, 2020 at 05:24:30PM +0200, Jani Nikula wrote:
> >> On Tue, 07 Jan 2020, Manasi Navare <manasi.d.navare@xxxxxxxxx> wrote:
> >>> Adaptive Sync is a VESA feature so add a DRM core helper to parse
> >>> the EDID's detailed descritors to obtain the adaptive sync monitor range.
> >>> Store this info as part fo drm_display_info so it can be used
> >>> across all drivers.
> >>> This part of the code is stripped out of amdgpu's function
> >>> amdgpu_dm_update_freesync_caps() to make it generic and be used
> >>> across all DRM drivers
> >>>
> >>> v2:
> >>> * Change vmin and vmax to use u8 (Ville)
> >>> * Dont store pixel clock since that is just a max dotclock
> >>> and not related to VRR mode (Manasi)
> >>>
> >>> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >>> Cc: Harry Wentland <harry.wentland@xxxxxxx>
> >>> Cc: Clinton A Taylor <clinton.a.taylor@xxxxxxxxx>
> >>> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@xxxxxxx>
> >>> Signed-off-by: Manasi Navare <manasi.d.navare@xxxxxxxxx>
> >>> ---
> >>>  drivers/gpu/drm/drm_edid.c  | 51 +++++++++++++++++++++++++++++++++++++
> >>>  include/drm/drm_connector.h | 22 ++++++++++++++++
> >>>  include/drm/drm_edid.h      |  2 ++
> >>>  3 files changed, 75 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> >>> index 99769d6c9f84..52781a0e708b 100644
> >>> --- a/drivers/gpu/drm/drm_edid.c
> >>> +++ b/drivers/gpu/drm/drm_edid.c
> >>> @@ -4880,6 +4880,54 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
> >>>  	}
> >>>  }
> >>>  
> >>> +void drm_get_adaptive_sync_limits(struct drm_connector *connector,
> >>> +				  const struct edid *edid)
> >>> +{
> >>> +	struct drm_display_info *info = &connector->display_info;
> >>> +	const struct detailed_timing *timing;
> >>> +	const struct detailed_non_pixel *data;
> >>> +	const struct detailed_data_monitor_range *range;
> >>> +	int i;
> >>> +
> >>> +	/*
> >>> +	 * Restrict Adaptive Sync only for dp and edp
> >>> +	 */
> >>> +	if (connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort &&
> >>> +	    connector->connector_type != DRM_MODE_CONNECTOR_eDP)
> >>> +		return;
> >>> +
> >>> +	if (edid->version <= 1 && !(edid->version == 1 && edid->revision > 1))
> >>> +		return;
> >>> +
> >>> +	for (i = 0; i < 4; i++) {
> >>> +		timing  = &edid->detailed_timings[i];
> >>> +		data    = &timing->data.other_data;
> >>> +		range   = &data->data.range;
> >>> +		/*
> >>> +		 * Check if monitor has continuous frequency mode
> >>> +		 */
> >>> +		if (data->type != EDID_DETAIL_MONITOR_RANGE)
> >>> +			continue;
> >>> +		/*
> >>> +		 * Check for flag range limits only. If flag == 1 then
> >>> +		 * no additional timing information provided.
> >>> +		 * Default GTF, GTF Secondary curve and CVT are not
> >>> +		 * supported
> >>> +		 */
> >>> +		if (range->flags != 1)
> >>> +			continue;
> >>> +
> >>> +		info->adaptive_sync.min_vfreq = range->min_vfreq;
> >>> +		info->adaptive_sync.max_vfreq = range->max_vfreq;
> >>> +
> >>> +		DRM_DEBUG_KMS("Adaptive Sync refresh rate range is %d Hz - %d Hz\n",
> >>> +			      info->adaptive_sync.min_vfreq,
> >>> +			      info->adaptive_sync.max_vfreq);
> >>> +		break;
> >>> +	}
> >>> +}
> >>> +EXPORT_SYMBOL(drm_get_adaptive_sync_limits);
> >>
> >> Why the export? Rather, why is this not static?
> >>
> > 
> > I could make it static but since the way AMDGPU code is written right now they
> > would be calling this function explicitly to populate the vmin and vmax in their local
> > structs but I can make it static and with some minor refactoring they should be able to
> > use thsi infor directly from drm_display_info
> > 
> 
> Calling this through drm_add_edid_modes should be sufficient for us.
> We'll just need to move amdgpu_dm_update_freesync_caps to our get_modes
> function.

Thanks Harry, Would you or Nick make these changes as follow up changes after this patch lands?
Since it might be easier for you to figure out where to accomodate the changes in amdgpu.

Manasi


> 
> Dropping the export here seems to be the right thing to do.
> 
> Harry
> 
> > Manasi
> >  
> >> BR,
> >> Jani.
> >>
> >>> +
> >>>  /* A connector has no EDID information, so we've got no EDID to compute quirks from. Reset
> >>>   * all of the values which would have been set from EDID
> >>>   */
> >>> @@ -4901,6 +4949,7 @@ drm_reset_display_info(struct drm_connector *connector)
> >>>  	memset(&info->hdmi, 0, sizeof(info->hdmi));
> >>>  
> >>>  	info->non_desktop = 0;
> >>> +	memset(&info->adaptive_sync, 0, sizeof(info->adaptive_sync));
> >>>  }
> >>>  
> >>>  u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edid)
> >>> @@ -4916,6 +4965,8 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi
> >>>  
> >>>  	info->non_desktop = !!(quirks & EDID_QUIRK_NON_DESKTOP);
> >>>  
> >>> +	drm_get_adaptive_sync_limits(connector, edid);
> >>> +
> >>>  	DRM_DEBUG_KMS("non_desktop set to %d\n", info->non_desktop);
> >>>  
> >>>  	if (edid->revision < 3)
> >>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> >>> index 221910948b37..77df404a2e01 100644
> >>> --- a/include/drm/drm_connector.h
> >>> +++ b/include/drm/drm_connector.h
> >>> @@ -254,6 +254,23 @@ enum drm_panel_orientation {
> >>>  	DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
> >>>  };
> >>>  
> >>> +/**
> >>> + * struct drm_adaptive_sync_info - Panel's Adaptive Sync capabilities for
> >>> + * &drm_display_info
> >>> + *
> >>> + * This struct is used to store a Panel's Adaptive Sync capabilities
> >>> + * as parsed from EDID's detailed monitor range descriptor block.
> >>> + *
> >>> + * @min_vfreq: This is the min supported refresh rate in Hz from
> >>> + *             EDID's detailed monitor range.
> >>> + * @max_vfreq: This is the max supported refresh rate in Hz from
> >>> + *             EDID's detailed monitor range
> >>> + */
> >>> +struct drm_adaptive_sync_info {
> >>> +	u8 min_vfreq;
> >>> +	u8 max_vfreq;
> >>> +};
> >>> +
> >>>  /*
> >>>   * This is a consolidated colorimetry list supported by HDMI and
> >>>   * DP protocol standard. The respective connectors will register
> >>> @@ -465,6 +482,11 @@ struct drm_display_info {
> >>>  	 * @non_desktop: Non desktop display (HMD).
> >>>  	 */
> >>>  	bool non_desktop;
> >>> +
> >>> +	/**
> >>> +	 * @adaptive_sync: Adaptive Sync capabilities of the DP/eDP sink
> >>> +	 */
> >>> +	struct drm_adaptive_sync_info adaptive_sync;
> >>>  };
> >>>  
> >>>  int drm_display_info_set_bus_formats(struct drm_display_info *info,
> >>> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> >>> index f0b03d401c27..b9a230aa3e69 100644
> >>> --- a/include/drm/drm_edid.h
> >>> +++ b/include/drm/drm_edid.h
> >>> @@ -503,4 +503,6 @@ void drm_edid_get_monitor_name(struct edid *edid, char *name,
> >>>  struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev,
> >>>  					   int hsize, int vsize, int fresh,
> >>>  					   bool rb);
> >>> +void drm_get_adaptive_sync_limits(struct drm_connector *connector,
> >>> +				  const struct edid *edid);
> >>>  #endif /* __DRM_EDID_H__ */
> >>
> >> -- 
> >> Jani Nikula, Intel Open Source Graphics Center
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux