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@linux.intel.com Cc: Harry Wentland harry.wentland@amd.com Cc: Clinton A Taylor clinton.a.taylor@intel.com Cc: Nicholas Kazlauskas nicholas.kazluaskas@amd.com Signed-off-by: Manasi Navare manasi.d.navare@intel.com --- 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); + /* 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__ */
Ville, could you take a look at this patch? I have tested this on the VRR monitor here and it does parse the detailed monitor range correctly to expose the min and max vfreq. Also I got rid of storing the pixel clock in the info->adaptive_sync_limits struct since thats just themax dotclock and not related to adaptive sync limits really.
Harry, could you take a look at this patch as well? This can be used inside amdgpu in several ways, it will automatically populate the drm_display_info during get_edid_modes or this function can be called explicitly in amdgpu_dm_update_freesync_caps() so I would leave the usage of this to you and Nicholas depending on what works better in your driver.
Regards Manasi
On Tue, Jan 07, 2020 at 04:32:08PM -0800, Manasi Navare 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@linux.intel.com Cc: Harry Wentland harry.wentland@amd.com Cc: Clinton A Taylor clinton.a.taylor@intel.com Cc: Nicholas Kazlauskas nicholas.kazluaskas@amd.com Signed-off-by: Manasi Navare manasi.d.navare@intel.com
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);
/* 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__ */
2.19.1
On Tue, Jan 07, 2020 at 04:32:08PM -0800, Manasi Navare 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@linux.intel.com Cc: Harry Wentland harry.wentland@amd.com Cc: Clinton A Taylor clinton.a.taylor@intel.com Cc: Nicholas Kazlauskas nicholas.kazluaskas@amd.com Signed-off-by: Manasi Navare manasi.d.navare@intel.com
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;
Needlessly wide scope for everything above.
- 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;
if (!version_greater(...)) return;
- for (i = 0; i < 4; i++) {
This should probably use for_each_detailed_block()
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);
/* 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__ */
2.19.1
On Thu, Jan 09, 2020 at 03:08:52PM +0200, Ville Syrjälä wrote:
On Tue, Jan 07, 2020 at 04:32:08PM -0800, Manasi Navare 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@linux.intel.com Cc: Harry Wentland harry.wentland@amd.com Cc: Clinton A Taylor clinton.a.taylor@intel.com Cc: Nicholas Kazlauskas nicholas.kazluaskas@amd.com Signed-off-by: Manasi Navare manasi.d.navare@intel.com
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;
Needlessly wide scope for everything above.
Okay will move inside the for_each loop ?
- 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;if (!version_greater(...)) return;
You mean simplify the condition or use define a function version_greater() to check this condition?
- for (i = 0; i < 4; i++) {
This should probably use for_each_detailed_block()
Ok yes will use for_each_detailed_block
Manasi
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);
/* 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__ */
2.19.1
-- Ville Syrjälä Intel
Hi Ville,
So the two major changes you would like to see here are:
use version_greate(edid) function and make use of : drm_for_each_detailed_block() instead of the for loop. But this function does not parse the monitor range yet so you are suggesting modifying that dmr helper function as well?
Manasi
On Fri, Jan 10, 2020 at 03:17:43PM -0800, Manasi Navare wrote:
On Thu, Jan 09, 2020 at 03:08:52PM +0200, Ville Syrjälä wrote:
On Tue, Jan 07, 2020 at 04:32:08PM -0800, Manasi Navare 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@linux.intel.com Cc: Harry Wentland harry.wentland@amd.com Cc: Clinton A Taylor clinton.a.taylor@intel.com Cc: Nicholas Kazlauskas nicholas.kazluaskas@amd.com Signed-off-by: Manasi Navare manasi.d.navare@intel.com
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;
Needlessly wide scope for everything above.
Okay will move inside the for_each loop ?
- 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;if (!version_greater(...)) return;
You mean simplify the condition or use define a function version_greater() to check this condition?
- for (i = 0; i < 4; i++) {
This should probably use for_each_detailed_block()
Ok yes will use for_each_detailed_block
Manasi
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);
/* 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__ */
2.19.1
-- Ville Syrjälä Intel
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, Jan 13, 2020 at 04:39:00PM -0800, Manasi Navare wrote:
Hi Ville,
So the two major changes you would like to see here are:
use version_greate(edid) function and make use of : drm_for_each_detailed_block() instead of the for loop. But this function does not parse the monitor range yet so you are suggesting modifying that dmr helper function as well?
That functions is just the thing that iterates the descriptors, so can't figure out what you're asking.
Manasi
On Fri, Jan 10, 2020 at 03:17:43PM -0800, Manasi Navare wrote:
On Thu, Jan 09, 2020 at 03:08:52PM +0200, Ville Syrjälä wrote:
On Tue, Jan 07, 2020 at 04:32:08PM -0800, Manasi Navare 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@linux.intel.com Cc: Harry Wentland harry.wentland@amd.com Cc: Clinton A Taylor clinton.a.taylor@intel.com Cc: Nicholas Kazlauskas nicholas.kazluaskas@amd.com Signed-off-by: Manasi Navare manasi.d.navare@intel.com
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;
Needlessly wide scope for everything above.
Okay will move inside the for_each loop ?
- 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;if (!version_greater(...)) return;
You mean simplify the condition or use define a function version_greater() to check this condition?
- for (i = 0; i < 4; i++) {
This should probably use for_each_detailed_block()
Ok yes will use for_each_detailed_block
Manasi
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);
/* 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__ */
2.19.1
-- Ville Syrjälä Intel
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, Jan 14, 2020 at 03:07:56PM +0200, Ville Syrjälä wrote:
On Mon, Jan 13, 2020 at 04:39:00PM -0800, Manasi Navare wrote:
Hi Ville,
So the two major changes you would like to see here are:
use version_greate(edid) function and make use of : drm_for_each_detailed_block() instead of the for loop. But this function does not parse the monitor range yet so you are suggesting modifying that dmr helper function as well?
That functions is just the thing that iterates the descriptors, so can't figure out what you're asking.
My question was on your review comment below where you asked me to use for_each_detailed_block(), were you referring to using the existing drm_for_each_detailed_block or just defining a new iterator for looping through 4 detailed blocks to get the monitor range?
Manasi
Manasi
On Fri, Jan 10, 2020 at 03:17:43PM -0800, Manasi Navare wrote:
On Thu, Jan 09, 2020 at 03:08:52PM +0200, Ville Syrjälä wrote:
On Tue, Jan 07, 2020 at 04:32:08PM -0800, Manasi Navare 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@linux.intel.com Cc: Harry Wentland harry.wentland@amd.com Cc: Clinton A Taylor clinton.a.taylor@intel.com Cc: Nicholas Kazlauskas nicholas.kazluaskas@amd.com Signed-off-by: Manasi Navare manasi.d.navare@intel.com
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;
Needlessly wide scope for everything above.
Okay will move inside the for_each loop ?
- 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;if (!version_greater(...)) return;
You mean simplify the condition or use define a function version_greater() to check this condition?
- for (i = 0; i < 4; i++) {
This should probably use for_each_detailed_block()
Ok yes will use for_each_detailed_block
Manasi
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);
/* 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__ */
2.19.1
-- Ville Syrjälä Intel
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- Ville Syrjälä Intel
On Thu, Jan 09, 2020 at 03:08:52PM +0200, Ville Syrjälä wrote:
On Tue, Jan 07, 2020 at 04:32:08PM -0800, Manasi Navare 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@linux.intel.com Cc: Harry Wentland harry.wentland@amd.com Cc: Clinton A Taylor clinton.a.taylor@intel.com Cc: Nicholas Kazlauskas nicholas.kazluaskas@amd.com Signed-off-by: Manasi Navare manasi.d.navare@intel.com
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;
Needlessly wide scope for everything above.
Ok will add this inside the for loop
- 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;if (!version_greater(...)) return;
- for (i = 0; i < 4; i++) {
This should probably use for_each_detailed_block()
Can you elaborate, I dont understand what you mean by using for_each_detailed_block()
Manasi
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);
/* 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__ */
2.19.1
-- Ville Syrjälä Intel
On Fri, Feb 28, 2020 at 01:18:45PM -0800, Manasi Navare wrote:
On Thu, Jan 09, 2020 at 03:08:52PM +0200, Ville Syrjälä wrote:
On Tue, Jan 07, 2020 at 04:32:08PM -0800, Manasi Navare 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@linux.intel.com Cc: Harry Wentland harry.wentland@amd.com Cc: Clinton A Taylor clinton.a.taylor@intel.com Cc: Nicholas Kazlauskas nicholas.kazluaskas@amd.com Signed-off-by: Manasi Navare manasi.d.navare@intel.com
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;
Needlessly wide scope for everything above.
Ok will add this inside the for loop
- 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;if (!version_greater(...)) return;
- for (i = 0; i < 4; i++) {
This should probably use for_each_detailed_block()
Can you elaborate, I dont understand what you mean by using for_each_detailed_block()
Manasi
Does that mean: I need to define drm_edid_get_vrr_range() where i call drm_for_each_detailed_block((u8 * )edid, get_vrr_range, &vrr_min, &vrr_max) but i need to access range = &data->data.range; where data = &timing->data.other_data; inside get_vrr_range() how can I access range?
Manasi
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);
/* 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__ */
2.19.1
-- Ville Syrjälä Intel
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On 2020-02-28 9:38 p.m., Manasi Navare wrote:
On Fri, Feb 28, 2020 at 01:18:45PM -0800, Manasi Navare wrote:
On Thu, Jan 09, 2020 at 03:08:52PM +0200, Ville Syrjälä wrote:
On Tue, Jan 07, 2020 at 04:32:08PM -0800, Manasi Navare 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@linux.intel.com Cc: Harry Wentland harry.wentland@amd.com Cc: Clinton A Taylor clinton.a.taylor@intel.com Cc: Nicholas Kazlauskas nicholas.kazluaskas@amd.com Signed-off-by: Manasi Navare manasi.d.navare@intel.com
Hi Manasi, sorry for the delayed response on this one!
Few comments inline below:
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)
Agree with other feedback here on making this static. We can move over amdgpu to just check the DRM reported caps after.
+{
- 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;
Needlessly wide scope for everything above.
Ok will add this inside the for loop
- 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;
This probably doesn't need to be blocked here, this is just parsing the EDID.
- if (edid->version <= 1 && !(edid->version == 1 && edid->revision > 1))
return;if (!version_greater(...)) return;
- for (i = 0; i < 4; i++) {
This should probably use for_each_detailed_block()
Can you elaborate, I dont understand what you mean by using for_each_detailed_block()
Manasi
Does that mean: I need to define drm_edid_get_vrr_range() where i call drm_for_each_detailed_block((u8 * )edid, get_vrr_range, &vrr_min, &vrr_max) but i need to access range = &data->data.range; where data = &timing->data.other_data; inside get_vrr_range() how can I access range?
Manasi
Wouldn't it look like:
drm_for_each_detailed_block((u8 *)edid, get_adaptive_sync_range, &info->adaptive_sync);
Or something along those lines?
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);
Can drop this EXPORT since this is all in drm_edid.c.
- /* 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);
Can drop this one as well.
Regards, Nicholas Kazlauskas
#endif /* __DRM_EDID_H__ */
2.19.1
-- Ville Syrjälä Intel
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, Mar 02, 2020 at 09:15:27AM -0500, Kazlauskas, Nicholas wrote:
On 2020-02-28 9:38 p.m., Manasi Navare wrote:
On Fri, Feb 28, 2020 at 01:18:45PM -0800, Manasi Navare wrote:
On Thu, Jan 09, 2020 at 03:08:52PM +0200, Ville Syrjälä wrote:
On Tue, Jan 07, 2020 at 04:32:08PM -0800, Manasi Navare 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@linux.intel.com Cc: Harry Wentland harry.wentland@amd.com Cc: Clinton A Taylor clinton.a.taylor@intel.com Cc: Nicholas Kazlauskas nicholas.kazluaskas@amd.com Signed-off-by: Manasi Navare manasi.d.navare@intel.com
Hi Manasi, sorry for the delayed response on this one!
Few comments inline below:
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)Agree with other feedback here on making this static. We can move over amdgpu to just check the DRM reported caps after.
+{
- 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;
Needlessly wide scope for everything above.
Ok will add this inside the for loop
- 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;This probably doesn't need to be blocked here, this is just parsing the EDID.
- if (edid->version <= 1 && !(edid->version == 1 && edid->revision > 1))
return;if (!version_greater(...)) return;
- for (i = 0; i < 4; i++) {
This should probably use for_each_detailed_block()
Can you elaborate, I dont understand what you mean by using for_each_detailed_block()
Manasi
Does that mean: I need to define drm_edid_get_vrr_range() where i call drm_for_each_detailed_block((u8 * )edid, get_vrr_range, &vrr_min, &vrr_max) but i need to access range = &data->data.range; where data = &timing->data.other_data; inside get_vrr_range() how can I access range?
Manasi
Wouldn't it look like:
drm_for_each_detailed_block((u8 *)edid, get_adaptive_sync_range, &info->adaptive_sync);
Yes but am still not sure how to access timing, data and range from the passed argumnet of edid, once that is clear then yes I can just fill info->adaptive sync directly inside the function.
Manasi
Or something along those lines?
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)
/** 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);
Can drop this EXPORT since this is all in drm_edid.c.
/* 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);Can drop this one as well.
Regards, Nicholas Kazlauskas
#endif /* __DRM_EDID_H__ */
2.19.1
-- Ville Syrjälä Intel
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, 07 Jan 2020, Manasi Navare manasi.d.navare@intel.com 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@linux.intel.com Cc: Harry Wentland harry.wentland@amd.com Cc: Clinton A Taylor clinton.a.taylor@intel.com Cc: Nicholas Kazlauskas nicholas.kazluaskas@amd.com Signed-off-by: Manasi Navare manasi.d.navare@intel.com
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?
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__ */
On Thu, Jan 09, 2020 at 05:24:30PM +0200, Jani Nikula wrote:
On Tue, 07 Jan 2020, Manasi Navare manasi.d.navare@intel.com 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@linux.intel.com Cc: Harry Wentland harry.wentland@amd.com Cc: Clinton A Taylor clinton.a.taylor@intel.com Cc: Nicholas Kazlauskas nicholas.kazluaskas@amd.com Signed-off-by: Manasi Navare manasi.d.navare@intel.com
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
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
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@intel.com 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@linux.intel.com Cc: Harry Wentland harry.wentland@amd.com Cc: Clinton A Taylor clinton.a.taylor@intel.com Cc: Nicholas Kazlauskas nicholas.kazluaskas@amd.com Signed-off-by: Manasi Navare manasi.d.navare@intel.com
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.
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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, 14 Jan 2020, Harry Wentland hwentlan@amd.com 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@intel.com wrote:
+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.
Dropping the export here seems to be the right thing to do.
Thanks; this should help unify the behaviour across drivers.
BR, Jani.
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@intel.com 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@linux.intel.com Cc: Harry Wentland harry.wentland@amd.com Cc: Clinton A Taylor clinton.a.taylor@intel.com Cc: Nicholas Kazlauskas nicholas.kazlauskas@amd.com Signed-off-by: Manasi Navare manasi.d.navare@intel.com
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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Harry Wentland -
Jani Nikula -
Kazlauskas, Nicholas -
Manasi Navare -
Ville Syrjälä