This patch creates a new structure drm_hdmi_info (inspired from drm_display_info). Driver will parse HDMI sink's advance capabilities from HF-VSDB and populate this structure. This structure will be kept and used as a sub-class within drm_display_info.
We are adding parsing of HF-VSDB In the next patch.
V3: Address review comments from Jose - Modify the usage of the structure drm_display_info in other drivers apart from I915
Cc: Thierry Reding treding@nvidia.com Cc: Daniel Vetter daniel.vetter@intel.com Cc: Jose Abreu joabreu@synopsys.com Suggested-by: Thierry Reding thierry.reding@gmail.com Signed-off-by: Shashank Sharma shashank.sharma@intel.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 2 +- drivers/gpu/drm/drm_edid.c | 6 +- drivers/gpu/drm/radeon/radeon_connectors.c | 2 +- include/drm/drm_connector.h | 79 ++++++++++++++++++++++++-- 4 files changed, 79 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c index 8d1cf2d..270ab5d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c @@ -180,7 +180,7 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector *connector)
/* Check if bpc is within clock limit. Try to degrade gracefully otherwise */ if ((bpc == 12) && (mode_clock * 3/2 > max_tmds_clock)) { - if ((connector->display_info.edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_30) && + if ((connector->display_info.hdmi_info.edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_30) && (mode_clock * 5/4 <= max_tmds_clock)) bpc = 10; else diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 67d6a73..b552197 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3782,21 +3782,21 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
if (hdmi[6] & DRM_EDID_HDMI_DC_30) { dc_bpc = 10; - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_30; + info->hdmi_info.edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_30; DRM_DEBUG("%s: HDMI sink does deep color 30.\n", connector->name); }
if (hdmi[6] & DRM_EDID_HDMI_DC_36) { dc_bpc = 12; - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_36; + info->hdmi_info.edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_36; DRM_DEBUG("%s: HDMI sink does deep color 36.\n", connector->name); }
if (hdmi[6] & DRM_EDID_HDMI_DC_48) { dc_bpc = 16; - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_48; + info->hdmi_info.edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_48; DRM_DEBUG("%s: HDMI sink does deep color 48.\n", connector->name); } diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c index 27affbd..9edd13b 100644 --- a/drivers/gpu/drm/radeon/radeon_connectors.c +++ b/drivers/gpu/drm/radeon/radeon_connectors.c @@ -210,7 +210,7 @@ int radeon_get_monitor_bpc(struct drm_connector *connector)
/* Check if bpc is within clock limit. Try to degrade gracefully otherwise */ if ((bpc == 12) && (mode_clock * 3/2 > max_tmds_clock)) { - if ((connector->display_info.edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_30) && + if ((connector->display_info.hdmi_info.edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_30) && (mode_clock * 5/4 <= max_tmds_clock)) bpc = 10; else diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 6e352a0..fba2b88 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -90,6 +90,76 @@ enum subpixel_order { };
/** + * struct drm_hdmi_info - runtime data specific to a connected hdmi sink + * + * Describes a given hdmi display (e.g. CRT or flat panel) and its capabilities. + * Mostly refects the advanced features added in HDMI 2.0 specs and the deep + * color support. This is a sub-segment of struct drm_display_info and should be + * used within. + * + * For sinks which provide an EDID this can be filled out by calling + * drm_add_edid_modes(). + */ + +struct drm_hdmi_info { + + /** + * @edid_hdmi_dc_modes: Mask of supported hdmi deep color modes. Even + * more stuff redundant with @bus_formats. + */ + u8 edid_hdmi_dc_modes; + + /** + * @edid_yuv420_dc_modes: bpc for deep color yuv420 encoding. + * various sinks can support 10/12/16 bit per channel deep + * color encoding. edid_yuv420_dc_modes = 0 means sink doesn't + * support deep color yuv420 encoding. + */ + u8 edid_yuv420_dc_modes; + + +#define DRM_HFVSDB_SCDC_SUPPORT (1<<7) +#define DRM_HFVSDB_SCDC_RR_CAP (1<<6) +#define DRM_HFVSDB_SCRAMBLING (1<<3) +#define DRM_HFVSDB_INDEPENDENT_VIEW (1<<2) +#define DRM_HFVSDB_DUAL_VIEW (1<<1) +#define DRM_HFVSDB_3D_OSD (1<<0) + + /** + * @scdc_supported: Sink supports SCDC functionality. + */ + bool scdc_supported; + + /** + * @scdc_rr_cap: Sink has SCDC read request capability. + */ + bool scdc_rr_cap; + + /** + * @scrambling: Sync supports scrambling for <=340 Mcsc TMDS + * char rates. Above 340 Mcsc rates, scrambling is always reqd. + */ + bool scrambling; + + /** + * @independent_view_3d: Sink supports 3d independent view signaling + * in HF-VSIF. + */ + bool independent_view_3d; + + /** + * @dual_view_3d: Sink supports 3d dual view signaling in HF-VSIF. + */ + bool dual_view_3d; + + /** + * @osd_disparity_3d: Sink supports 3d osd disparity indication + * in HF-VSIF. + */ + bool osd_disparity_3d; +}; + +/** * struct drm_display_info - runtime data about the connected sink * * Describes a given display (e.g. CRT or flat panel) and its limitations. For @@ -179,15 +249,14 @@ struct drm_display_info { bool dvi_dual;
/** - * @edid_hdmi_dc_modes: Mask of supported hdmi deep color modes. Even - * more stuff redundant with @bus_formats. + * @cea_rev: CEA revision of the HDMI sink. */ - u8 edid_hdmi_dc_modes; + u8 cea_rev;
/** - * @cea_rev: CEA revision of the HDMI sink. + * @ drm_hdmi_info: Capabilities of connected HDMI display */ - u8 cea_rev; + struct drm_hdmi_info hdmi_info; };
int drm_display_info_set_bus_formats(struct drm_display_info *info,
HDMI 2.0 / CEA-861-F specs define a new CEA extension data block, called hdmi-forum vendor specific data block (HF-VSDB). This block contains information about sink's support for HDMI 2.0 compliant features. These features are: - Deep color YUV 420 support and BPC - 3D flags for - OSD Displarity - Dual view signaling - independent view signaling - SCDC support - Max TMDS char rate - Scrambling support
This patch adds a parser function for this block, and add flags to indicate support for new features, in drm_display_info structure
V2: - Addressed review comments from Thierry - remove len > 31 check - remove version check - fix duplicate values for macros of 36 and 30-bit depths - Added a sub-class for HDMI related information within drm_display_info (Thierry, Daniel) and populated it with HF-VSDB specific info.
V3: - Addressed review comments from Jose - check if SCDC supported while checking for scrambling - Fix the bit nos for YUV420 deep color macros - write HDMI_IEEE_OUI_HFVSDB value in lower case
Cc: Thierry Reding treding@nvidia.com Cc: Daniel Vetter daniel.vetter@intel.com Cc: Jose Abreu joabreu@synopsys.com Signed-off-by: Shashank Sharma shashank.sharma@intel.com --- drivers/gpu/drm/drm_edid.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++ include/drm/drm_edid.h | 5 ++++ include/linux/hdmi.h | 1 + 3 files changed, 76 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index b552197..f235576 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3224,6 +3224,23 @@ static int add_3d_struct_modes(struct drm_connector *connector, u16 structure, return 0; }
+static bool cea_db_is_hf_vsdb(const u8 *db) +{ + u8 len; + int hfvsdb_id; + + if (cea_db_tag(db) != VENDOR_BLOCK) + return false; + + len = cea_db_payload_len(db); + if (len < 7) + return false; + + hfvsdb_id = db[1] | (db[2] << 8) | (db[3] << 16); + + return hfvsdb_id == HDMI_IEEE_OUI_HFVSDB; +} + static bool cea_db_is_hdmi_vsdb(const u8 *db) { int hdmi_id; @@ -3768,6 +3785,57 @@ bool drm_rgb_quant_range_selectable(struct edid *edid) } EXPORT_SYMBOL(drm_rgb_quant_range_selectable);
+static void drm_parse_yuv420_deep_color_info(struct drm_connector *connector, + const u8 *db) +{ + struct drm_hdmi_info *info = &connector->display_info.hdmi_info; + + if (db[7] & DRM_EDID_YUV420_DC_48) + info->edid_yuv420_dc_modes |= DRM_EDID_YUV420_DC_48; + if (db[7] & DRM_EDID_YUV420_DC_36) + info->edid_yuv420_dc_modes |= DRM_EDID_YUV420_DC_36; + if (db[7] & DRM_EDID_YUV420_DC_30) + info->edid_yuv420_dc_modes |= DRM_EDID_YUV420_DC_30; + + if (!info->edid_yuv420_dc_modes) { + DRM_DEBUG("%s: No YUV 420 deep color support in sink.\n", + connector->name); + return; + } +} + +static void +drm_parse_hf_vsdb(struct drm_connector *connector, const u8 *db) +{ + struct drm_display_info *info = &connector->display_info; + struct drm_hdmi_info *hdmi_info = &info->hdmi_info; + + if (db[5]) { + /* + * If the sink supplies max tmds char rate in db, + * the actual max tmds rate = db[5] * 5Mhz. + */ + info->max_tmds_clock = db[5] * 5000; + DRM_DEBUG_KMS("HF-VSDB: max TMDS clock %d kHz\n", + info->max_tmds_clock); + } + + if (db[6] & DRM_HFVSDB_SCDC_SUPPORT) + hdmi_info->scdc_supported = true; + if (db[6] & DRM_HFVSDB_SCDC_RR_CAP) + hdmi_info->scdc_rr_cap = true; + if ((db[6] & DRM_HFVSDB_SCRAMBLING) && hdmi_info->scdc_supported) + hdmi_info->scrambling = true; + if (db[6] & DRM_HFVSDB_INDEPENDENT_VIEW) + hdmi_info->independent_view_3d = true; + if (db[6] & DRM_HFVSDB_DUAL_VIEW) + hdmi_info->dual_view_3d = true; + if (db[6] & DRM_HFVSDB_3D_OSD) + hdmi_info->osd_disparity_3d = true; + + drm_parse_yuv420_deep_color_info(connector, db); +} + static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, const u8 *hdmi) { @@ -3882,6 +3950,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
if (cea_db_is_hdmi_vsdb(db)) drm_parse_hdmi_vsdb_video(connector, db); + if (cea_db_is_hf_vsdb(db)) + drm_parse_hf_vsdb(connector, db); } }
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 38eabf6..a6453f7 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -212,6 +212,11 @@ struct detailed_timing { #define DRM_EDID_HDMI_DC_30 (1 << 4) #define DRM_EDID_HDMI_DC_Y444 (1 << 3)
+/* YUV 420 deep color modes */ +#define DRM_EDID_YUV420_DC_48 (1 << 2) +#define DRM_EDID_YUV420_DC_36 (1 << 1) +#define DRM_EDID_YUV420_DC_30 (1 << 0) + /* ELD Header Block */ #define DRM_ELD_HEADER_BLOCK_SIZE 4
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h index edbb4fc..3dd4e9a 100644 --- a/include/linux/hdmi.h +++ b/include/linux/hdmi.h @@ -35,6 +35,7 @@ enum hdmi_infoframe_type { };
#define HDMI_IEEE_OUI 0x000c03 +#define HDMI_IEEE_OUI_HFVSDB 0xc45dd8 #define HDMI_INFOFRAME_HEADER_SIZE 4 #define HDMI_AVI_INFOFRAME_SIZE 13 #define HDMI_SPD_INFOFRAME_SIZE 25
Hi Shashank,
On 21-12-2016 15:29, Shashank Sharma wrote:
HDMI 2.0 / CEA-861-F specs define a new CEA extension data block, called hdmi-forum vendor specific data block (HF-VSDB). This block contains information about sink's support for HDMI 2.0 compliant features. These features are: - Deep color YUV 420 support and BPC - 3D flags for - OSD Displarity - Dual view signaling - independent view signaling - SCDC support - Max TMDS char rate - Scrambling support
This patch adds a parser function for this block, and add flags to indicate support for new features, in drm_display_info structure
V2:
- Addressed review comments from Thierry
- remove len > 31 check
- remove version check
- fix duplicate values for macros of 36 and 30-bit depths
- Added a sub-class for HDMI related information within drm_display_info (Thierry, Daniel) and populated it with HF-VSDB specific info.
V3:
- Addressed review comments from Jose
- check if SCDC supported while checking for scrambling
- Fix the bit nos for YUV420 deep color macros
- write HDMI_IEEE_OUI_HFVSDB value in lower case
Cc: Thierry Reding treding@nvidia.com Cc: Daniel Vetter daniel.vetter@intel.com Cc: Jose Abreu joabreu@synopsys.com Signed-off-by: Shashank Sharma shashank.sharma@intel.com
Looks good by me. I don't have the setup to test it right now, but:
Reviewed-by: Jose Abreu joabreu@synopsys.com
Best regards, Jose Miguel Abreu
drivers/gpu/drm/drm_edid.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++ include/drm/drm_edid.h | 5 ++++ include/linux/hdmi.h | 1 + 3 files changed, 76 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index b552197..f235576 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3224,6 +3224,23 @@ static int add_3d_struct_modes(struct drm_connector *connector, u16 structure, return 0; }
+static bool cea_db_is_hf_vsdb(const u8 *db) +{
- u8 len;
- int hfvsdb_id;
- if (cea_db_tag(db) != VENDOR_BLOCK)
return false;- len = cea_db_payload_len(db);
- if (len < 7)
return false;- hfvsdb_id = db[1] | (db[2] << 8) | (db[3] << 16);
- return hfvsdb_id == HDMI_IEEE_OUI_HFVSDB;
+}
static bool cea_db_is_hdmi_vsdb(const u8 *db) { int hdmi_id; @@ -3768,6 +3785,57 @@ bool drm_rgb_quant_range_selectable(struct edid *edid) } EXPORT_SYMBOL(drm_rgb_quant_range_selectable);
+static void drm_parse_yuv420_deep_color_info(struct drm_connector *connector,
const u8 *db)+{
- struct drm_hdmi_info *info = &connector->display_info.hdmi_info;
- if (db[7] & DRM_EDID_YUV420_DC_48)
info->edid_yuv420_dc_modes |= DRM_EDID_YUV420_DC_48;- if (db[7] & DRM_EDID_YUV420_DC_36)
info->edid_yuv420_dc_modes |= DRM_EDID_YUV420_DC_36;- if (db[7] & DRM_EDID_YUV420_DC_30)
info->edid_yuv420_dc_modes |= DRM_EDID_YUV420_DC_30;- if (!info->edid_yuv420_dc_modes) {
DRM_DEBUG("%s: No YUV 420 deep color support in sink.\n",connector->name);return;- }
+}
+static void +drm_parse_hf_vsdb(struct drm_connector *connector, const u8 *db) +{
- struct drm_display_info *info = &connector->display_info;
- struct drm_hdmi_info *hdmi_info = &info->hdmi_info;
- if (db[5]) {
/** If the sink supplies max tmds char rate in db,* the actual max tmds rate = db[5] * 5Mhz.*/info->max_tmds_clock = db[5] * 5000;DRM_DEBUG_KMS("HF-VSDB: max TMDS clock %d kHz\n",info->max_tmds_clock);- }
- if (db[6] & DRM_HFVSDB_SCDC_SUPPORT)
hdmi_info->scdc_supported = true;- if (db[6] & DRM_HFVSDB_SCDC_RR_CAP)
hdmi_info->scdc_rr_cap = true;- if ((db[6] & DRM_HFVSDB_SCRAMBLING) && hdmi_info->scdc_supported)
hdmi_info->scrambling = true;- if (db[6] & DRM_HFVSDB_INDEPENDENT_VIEW)
hdmi_info->independent_view_3d = true;- if (db[6] & DRM_HFVSDB_DUAL_VIEW)
hdmi_info->dual_view_3d = true;- if (db[6] & DRM_HFVSDB_3D_OSD)
hdmi_info->osd_disparity_3d = true;- drm_parse_yuv420_deep_color_info(connector, db);
+}
static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, const u8 *hdmi) { @@ -3882,6 +3950,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
if (cea_db_is_hdmi_vsdb(db)) drm_parse_hdmi_vsdb_video(connector, db);
if (cea_db_is_hf_vsdb(db))drm_parse_hf_vsdb(connector, db);}
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 38eabf6..a6453f7 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -212,6 +212,11 @@ struct detailed_timing { #define DRM_EDID_HDMI_DC_30 (1 << 4) #define DRM_EDID_HDMI_DC_Y444 (1 << 3)
+/* YUV 420 deep color modes */ +#define DRM_EDID_YUV420_DC_48 (1 << 2) +#define DRM_EDID_YUV420_DC_36 (1 << 1) +#define DRM_EDID_YUV420_DC_30 (1 << 0)
/* ELD Header Block */ #define DRM_ELD_HEADER_BLOCK_SIZE 4
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h index edbb4fc..3dd4e9a 100644 --- a/include/linux/hdmi.h +++ b/include/linux/hdmi.h @@ -35,6 +35,7 @@ enum hdmi_infoframe_type { };
#define HDMI_IEEE_OUI 0x000c03 +#define HDMI_IEEE_OUI_HFVSDB 0xc45dd8 #define HDMI_INFOFRAME_HEADER_SIZE 4 #define HDMI_AVI_INFOFRAME_SIZE 13 #define HDMI_SPD_INFOFRAME_SIZE 25
This patch adds a small helper function, which clears the cached information about a hot-pluggable display, from connector. On event This will run on event of a hot-unplug, keeping the connector's display info up-to-date, avoiding any errors due to invalid cached data.
Cc: Jose Abreu joabreu@synopsys.com
Suggested-by: Jose Abreu joabreu@synopsys.com Signed-off-by: Shashank Sharma shashank.sharma@intel.com --- drivers/gpu/drm/drm_probe_helper.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 7cff91e..9e97b45 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -164,6 +164,18 @@ void drm_kms_helper_poll_enable_locked(struct drm_device *dev) }
/** + * drm_helper_clear_display_info - clean cached display information for + * hot pluggable displays, on event of hot-unplug + * @connector: connector under event + */ +void drm_helper_clear_display_info(struct drm_connector *connector) +{ + struct drm_display_info *info = &connector->display_info; + + memset(info, 0, sizeof(*info)); +} + +/** * drm_helper_probe_single_connector_modes - get complete set of display modes * @connector: connector to probe * @maxX: max width for modes @@ -288,6 +300,14 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, DRM_DEBUG_KMS("[CONNECTOR:%d:%s] disconnected\n", connector->base.id, connector->name); drm_mode_connector_update_edid_property(connector, NULL); + + /* + * Connector status change to disconnected, time to clean + * cached display information + */ + if (connector->status == connector_status_disconnected) + drm_helper_clear_display_info(connector); + verbose_prune = false; goto prune; }
Hi Shashank,
On 21-12-2016 15:29, Shashank Sharma wrote:
This patch adds a small helper function, which clears the cached information about a hot-pluggable display, from connector. On event This will run on event of a hot-unplug, keeping the connector's display info up-to-date, avoiding any errors due to invalid cached data.
Cc: Jose Abreu joabreu@synopsys.com
Suggested-by: Jose Abreu joabreu@synopsys.com Signed-off-by: Shashank Sharma shashank.sharma@intel.com
drivers/gpu/drm/drm_probe_helper.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 7cff91e..9e97b45 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -164,6 +164,18 @@ void drm_kms_helper_poll_enable_locked(struct drm_device *dev) }
/**
- drm_helper_clear_display_info - clean cached display information for
- hot pluggable displays, on event of hot-unplug
- @connector: connector under event
- */
+void drm_helper_clear_display_info(struct drm_connector *connector) +{
- struct drm_display_info *info = &connector->display_info;
- memset(info, 0, sizeof(*info));
+}
+/**
- drm_helper_probe_single_connector_modes - get complete set of display modes
- @connector: connector to probe
- @maxX: max width for modes
@@ -288,6 +300,14 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, DRM_DEBUG_KMS("[CONNECTOR:%d:%s] disconnected\n", connector->base.id, connector->name); drm_mode_connector_update_edid_property(connector, NULL);
/** Connector status change to disconnected, time to clean* cached display information*/if (connector->status == connector_status_disconnected)drm_helper_clear_display_info(connector);
I don't know if this is the right place to do this because it is a helper and I don't know if it is used by all the drivers. We may need something more general that is always called when probing modes, or force drivers that don't use the helper to use the drm_helper_clear_display_info function. As I told you before, I'm new to dri-devel so we need more comments.
Best regards, Jose Miguel Abreu
verbose_prune = false; goto prune;}
On Thu, Dec 22, 2016 at 10:21:25AM +0000, Jose Abreu wrote:
Hi Shashank,
On 21-12-2016 15:29, Shashank Sharma wrote:
This patch adds a small helper function, which clears the cached information about a hot-pluggable display, from connector. On event This will run on event of a hot-unplug, keeping the connector's display info up-to-date, avoiding any errors due to invalid cached data.
Cc: Jose Abreu joabreu@synopsys.com
Suggested-by: Jose Abreu joabreu@synopsys.com Signed-off-by: Shashank Sharma shashank.sharma@intel.com
drivers/gpu/drm/drm_probe_helper.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 7cff91e..9e97b45 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -164,6 +164,18 @@ void drm_kms_helper_poll_enable_locked(struct drm_device *dev) }
/**
- drm_helper_clear_display_info - clean cached display information for
- hot pluggable displays, on event of hot-unplug
- @connector: connector under event
- */
+void drm_helper_clear_display_info(struct drm_connector *connector) +{
- struct drm_display_info *info = &connector->display_info;
- memset(info, 0, sizeof(*info));
+}
+/**
- drm_helper_probe_single_connector_modes - get complete set of display modes
- @connector: connector to probe
- @maxX: max width for modes
@@ -288,6 +300,14 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, DRM_DEBUG_KMS("[CONNECTOR:%d:%s] disconnected\n", connector->base.id, connector->name); drm_mode_connector_update_edid_property(connector, NULL);
/** Connector status change to disconnected, time to clean* cached display information*/if (connector->status == connector_status_disconnected)drm_helper_clear_display_info(connector);I don't know if this is the right place to do this because it is a helper and I don't know if it is used by all the drivers. We may need something more general that is always called when probing modes, or force drivers that don't use the helper to use the drm_helper_clear_display_info function. As I told you before, I'm new to dri-devel so we need more comments.
Seems reasonable to me, since afaik all drivers do use the probe helpers. -Daniel
Best regards, Jose Miguel Abreu
verbose_prune = false; goto prune;}
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Regards
Shashank
On 12/27/2016 3:07 PM, Daniel Vetter wrote:
On Thu, Dec 22, 2016 at 10:21:25AM +0000, Jose Abreu wrote:
Hi Shashank,
On 21-12-2016 15:29, Shashank Sharma wrote:
This patch adds a small helper function, which clears the cached information about a hot-pluggable display, from connector. On event This will run on event of a hot-unplug, keeping the connector's display info up-to-date, avoiding any errors due to invalid cached data.
Cc: Jose Abreu joabreu@synopsys.com
Suggested-by: Jose Abreu joabreu@synopsys.com Signed-off-by: Shashank Sharma shashank.sharma@intel.com
drivers/gpu/drm/drm_probe_helper.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 7cff91e..9e97b45 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -164,6 +164,18 @@ void drm_kms_helper_poll_enable_locked(struct drm_device *dev) }
/**
- drm_helper_clear_display_info - clean cached display information for
- hot pluggable displays, on event of hot-unplug
- @connector: connector under event
- */
+void drm_helper_clear_display_info(struct drm_connector *connector) +{
- struct drm_display_info *info = &connector->display_info;
- memset(info, 0, sizeof(*info));
+}
+/**
- drm_helper_probe_single_connector_modes - get complete set of display modes
- @connector: connector to probe
- @maxX: max width for modes
@@ -288,6 +300,14 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, DRM_DEBUG_KMS("[CONNECTOR:%d:%s] disconnected\n", connector->base.id, connector->name); drm_mode_connector_update_edid_property(connector, NULL);
/** Connector status change to disconnected, time to clean* cached display information*/if (connector->status == connector_status_disconnected)drm_helper_clear_display_info(connector);I don't know if this is the right place to do this because it is a helper and I don't know if it is used by all the drivers. We may need something more general that is always called when probing modes, or force drivers that don't use the helper to use the drm_helper_clear_display_info function. As I told you before, I'm new to dri-devel so we need more comments.
Seems reasonable to me, since afaik all drivers do use the probe helpers. -Daniel
This was my understanding too. Jose, you think there would be any drivers who dont use this probe ? - Shashank
Best regards, Jose Miguel Abreu
verbose_prune = false; goto prune;}
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi Shashank,
On 29-12-2016 05:53, Sharma, Shashank wrote:
Regards
Shashank
On 12/27/2016 3:07 PM, Daniel Vetter wrote:
On Thu, Dec 22, 2016 at 10:21:25AM +0000, Jose Abreu wrote:
Hi Shashank,
On 21-12-2016 15:29, Shashank Sharma wrote:
This patch adds a small helper function, which clears the cached information about a hot-pluggable display, from connector. On event This will run on event of a hot-unplug, keeping the connector's display info up-to-date, avoiding any errors due to invalid cached data.
Cc: Jose Abreu joabreu@synopsys.com
Suggested-by: Jose Abreu joabreu@synopsys.com Signed-off-by: Shashank Sharma shashank.sharma@intel.com
drivers/gpu/drm/drm_probe_helper.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 7cff91e..9e97b45 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -164,6 +164,18 @@ void drm_kms_helper_poll_enable_locked(struct drm_device *dev) } /**
- drm_helper_clear_display_info - clean cached display
information for
- hot pluggable displays, on event of hot-unplug
- @connector: connector under event
- */
+void drm_helper_clear_display_info(struct drm_connector *connector) +{
- struct drm_display_info *info = &connector->display_info;
- memset(info, 0, sizeof(*info));
+}
+/**
- drm_helper_probe_single_connector_modes - get complete
set of display modes
- @connector: connector to probe
- @maxX: max width for modes
@@ -288,6 +300,14 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, DRM_DEBUG_KMS("[CONNECTOR:%d:%s] disconnected\n", connector->base.id, connector->name); drm_mode_connector_update_edid_property(connector, NULL);
/** Connector status change to disconnected, time toclean
* cached display information*/if (connector->status ==connector_status_disconnected)
drm_helper_clear_display_info(connector);I don't know if this is the right place to do this because it is a helper and I don't know if it is used by all the drivers. We may need something more general that is always called when probing modes, or force drivers that don't use the helper to use the drm_helper_clear_display_info function. As I told you before, I'm new to dri-devel so we need more comments.
Seems reasonable to me, since afaik all drivers do use the probe helpers. -Daniel
This was my understanding too. Jose, you think there would be any drivers who dont use this probe ?
- Shashank
I found only one driver that don't use this helper: vmwgfx. But, this driver does not seem to use EDID fields, it has a list of preferred video modes and manually adds these modes.
So, I think it is safe to add this in the helper as long as future drivers that use EDID use this helper also. Maybe a small comment about this should be added in the helper declaration.
Best regards, Jose Miguel Abreu
Best regards, Jose Miguel Abreu
verbose_prune = false; goto prune; }
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_m...
Regards
Shashank
On 12/29/2016 3:35 PM, Jose Abreu wrote:
Hi Shashank,
On 29-12-2016 05:53, Sharma, Shashank wrote:
Regards
Shashank
On 12/27/2016 3:07 PM, Daniel Vetter wrote:
On Thu, Dec 22, 2016 at 10:21:25AM +0000, Jose Abreu wrote:
Hi Shashank,
On 21-12-2016 15:29, Shashank Sharma wrote:
This patch adds a small helper function, which clears the cached information about a hot-pluggable display, from connector. On event This will run on event of a hot-unplug, keeping the connector's display info up-to-date, avoiding any errors due to invalid cached data.
Cc: Jose Abreu joabreu@synopsys.com
Suggested-by: Jose Abreu joabreu@synopsys.com Signed-off-by: Shashank Sharma shashank.sharma@intel.com
drivers/gpu/drm/drm_probe_helper.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 7cff91e..9e97b45 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -164,6 +164,18 @@ void drm_kms_helper_poll_enable_locked(struct drm_device *dev) } /**
- drm_helper_clear_display_info - clean cached display
information for
- hot pluggable displays, on event of hot-unplug
- @connector: connector under event
- */
+void drm_helper_clear_display_info(struct drm_connector *connector) +{
- struct drm_display_info *info = &connector->display_info;
- memset(info, 0, sizeof(*info));
+}
+/** * drm_helper_probe_single_connector_modes - get complete set of display modes * @connector: connector to probe * @maxX: max width for modes @@ -288,6 +300,14 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, DRM_DEBUG_KMS("[CONNECTOR:%d:%s] disconnected\n", connector->base.id, connector->name); drm_mode_connector_update_edid_property(connector, NULL);
/** Connector status change to disconnected, time toclean
* cached display information*/if (connector->status ==connector_status_disconnected)
drm_helper_clear_display_info(connector);I don't know if this is the right place to do this because it is a helper and I don't know if it is used by all the drivers. We may need something more general that is always called when probing modes, or force drivers that don't use the helper to use the drm_helper_clear_display_info function. As I told you before, I'm new to dri-devel so we need more comments.
Seems reasonable to me, since afaik all drivers do use the probe helpers. -Daniel
This was my understanding too. Jose, you think there would be any drivers who dont use this probe ?
- Shashank
I found only one driver that don't use this helper: vmwgfx. But, this driver does not seem to use EDID fields, it has a list of preferred video modes and manually adds these modes.
So, I think it is safe to add this in the helper as long as future drivers that use EDID use this helper also. Maybe a small comment about this should be added in the helper declaration.
Best regards, Jose Miguel Abreu
Sure, I will add a comment and publish a new patchset. Shashank
Best regards, Jose Miguel Abreu
verbose_prune = false; goto prune; }
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_m...
Hi Shashank,
On 21-12-2016 15:29, Shashank Sharma wrote:
[snip]
- /**
* @edid_yuv420_dc_modes: bpc for deep color yuv420 encoding.* various sinks can support 10/12/16 bit per channel deep* color encoding. edid_yuv420_dc_modes = 0 means sink doesn't* support deep color yuv420 encoding.*/- u8 edid_yuv420_dc_modes;
+#define DRM_HFVSDB_SCDC_SUPPORT (1<<7) +#define DRM_HFVSDB_SCDC_RR_CAP (1<<6) +#define DRM_HFVSDB_SCRAMBLING (1<<3) +#define DRM_HFVSDB_INDEPENDENT_VIEW (1<<2) +#define DRM_HFVSDB_DUAL_VIEW (1<<1) +#define DRM_HFVSDB_3D_OSD (1<<0)
- /**
* @scdc_supported: Sink supports SCDC functionality.*/- bool scdc_supported;
- /**
* @scdc_rr_cap: Sink has SCDC read request capability.*/- bool scdc_rr_cap;
- /**
* @scrambling: Sync supports scrambling for <=340 Mcsc TMDS* char rates. Above 340 Mcsc rates, scrambling is always reqd.*/- bool scrambling;
- /**
* @independent_view_3d: Sink supports 3d independent view signaling* in HF-VSIF.*/- bool independent_view_3d;
- /**
* @dual_view_3d: Sink supports 3d dual view signaling in HF-VSIF.*/- bool dual_view_3d;
- /**
* @osd_disparity_3d: Sink supports 3d osd disparity indication* in HF-VSIF.*/- bool osd_disparity_3d;
+};
[snip]
I thought we agreed in only adding these fields (edid_yuv420_dc_modes, scdc_supported, scdc_rr_cap, scrambling, independent_view_3d, dual_view_3d, osd_disparity_3d) in patch 2. I think it makes sense because you are only using them after that patch. Though, I would like to hear more comments about this as I am quite new to dri-devel.
Best regards, Jose Miguel Abreu
On Thu, Dec 22, 2016 at 10:02:26AM +0000, Jose Abreu wrote:
Hi Shashank,
On 21-12-2016 15:29, Shashank Sharma wrote:
[snip]
- /**
* @edid_yuv420_dc_modes: bpc for deep color yuv420 encoding.* various sinks can support 10/12/16 bit per channel deep* color encoding. edid_yuv420_dc_modes = 0 means sink doesn't* support deep color yuv420 encoding.*/- u8 edid_yuv420_dc_modes;
+#define DRM_HFVSDB_SCDC_SUPPORT (1<<7) +#define DRM_HFVSDB_SCDC_RR_CAP (1<<6) +#define DRM_HFVSDB_SCRAMBLING (1<<3) +#define DRM_HFVSDB_INDEPENDENT_VIEW (1<<2) +#define DRM_HFVSDB_DUAL_VIEW (1<<1) +#define DRM_HFVSDB_3D_OSD (1<<0)
- /**
* @scdc_supported: Sink supports SCDC functionality.*/- bool scdc_supported;
- /**
* @scdc_rr_cap: Sink has SCDC read request capability.*/- bool scdc_rr_cap;
- /**
* @scrambling: Sync supports scrambling for <=340 Mcsc TMDS* char rates. Above 340 Mcsc rates, scrambling is always reqd.*/- bool scrambling;
- /**
* @independent_view_3d: Sink supports 3d independent view signaling* in HF-VSIF.*/- bool independent_view_3d;
- /**
* @dual_view_3d: Sink supports 3d dual view signaling in HF-VSIF.*/- bool dual_view_3d;
- /**
* @osd_disparity_3d: Sink supports 3d osd disparity indication* in HF-VSIF.*/- bool osd_disparity_3d;
+};
[snip]
I thought we agreed in only adding these fields (edid_yuv420_dc_modes, scdc_supported, scdc_rr_cap, scrambling, independent_view_3d, dual_view_3d, osd_disparity_3d) in patch 2. I think it makes sense because you are only using them after that patch. Though, I would like to hear more comments about this as I am quite new to dri-devel.
Generally you shouldn't add stuff you don't use. In this seires nothing actually uses any of of this stuff, so I don't think we should add any of it.
The only piece of information actually used is the max TMDS clock, so parsing that does make sense. But I think that might be buggy as the patch will go ahead and parse both the old and new HDMI data blocks. IIRC the spec did have some words about which one should be used in which case. I don't think I spotted anything matching that in these patches.
Regards
Shashank
On 12/22/2016 5:26 PM, Ville Syrjälä wrote:
On Thu, Dec 22, 2016 at 10:02:26AM +0000, Jose Abreu wrote:
Hi Shashank,
On 21-12-2016 15:29, Shashank Sharma wrote:
[snip]
- /**
* @edid_yuv420_dc_modes: bpc for deep color yuv420 encoding.* various sinks can support 10/12/16 bit per channel deep* color encoding. edid_yuv420_dc_modes = 0 means sink doesn't* support deep color yuv420 encoding.*/- u8 edid_yuv420_dc_modes;
+#define DRM_HFVSDB_SCDC_SUPPORT (1<<7) +#define DRM_HFVSDB_SCDC_RR_CAP (1<<6) +#define DRM_HFVSDB_SCRAMBLING (1<<3) +#define DRM_HFVSDB_INDEPENDENT_VIEW (1<<2) +#define DRM_HFVSDB_DUAL_VIEW (1<<1) +#define DRM_HFVSDB_3D_OSD (1<<0)
- /**
* @scdc_supported: Sink supports SCDC functionality.*/- bool scdc_supported;
- /**
* @scdc_rr_cap: Sink has SCDC read request capability.*/- bool scdc_rr_cap;
- /**
* @scrambling: Sync supports scrambling for <=340 Mcsc TMDS* char rates. Above 340 Mcsc rates, scrambling is always reqd.*/- bool scrambling;
- /**
* @independent_view_3d: Sink supports 3d independent view signaling* in HF-VSIF.*/- bool independent_view_3d;
- /**
* @dual_view_3d: Sink supports 3d dual view signaling in HF-VSIF.*/- bool dual_view_3d;
- /**
* @osd_disparity_3d: Sink supports 3d osd disparity indication* in HF-VSIF.*/- bool osd_disparity_3d;
+};
[snip]
I thought we agreed in only adding these fields (edid_yuv420_dc_modes, scdc_supported, scdc_rr_cap, scrambling, independent_view_3d, dual_view_3d, osd_disparity_3d) in patch 2. I think it makes sense because you are only using them after that patch. Though, I would like to hear more comments about this as I am quite new to dri-devel.
Generally you shouldn't add stuff you don't use. In this seires nothing actually uses any of of this stuff, so I don't think we should add any of it.
The only piece of information actually used is the max TMDS clock, so parsing that does make sense. But I think that might be buggy as the patch will go ahead and parse both the old and new HDMI data blocks. IIRC the spec did have some words about which one should be used in which case. I don't think I spotted anything matching that in these patches.
If I understood the spec right, H14B VSDB block should contain the max_tmds_clock value to be reflected for tmds_clock_rates <340Mhz, But if the sink supports tmds_rates above 340Mhz, it should set this field in hf-vsdb. Also, this field allows sinks to support higher color depths to lower resolutions, than it can support to higher resolutions.
In this case, if this byte is hf-vsdb is set, max tmds clock should be picked from this block.
Regards Shashank
Regards Shashank
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Jose Abreu -
Sharma, Shashank -
Shashank Sharma
-
Ville Syrjälä