[PATCH v3 1/3] drm: Create new structure for HDMI info

[PATCH v3 1/3] drm: Create new structure for HDMI info

[Shashank Sharma]

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,
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 2/3] drm: parse hf-vsdb

[Shashank Sharma]

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
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 3/3] drm: clean cached display info

[Shashank Sharma]

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;
 	}
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 1/3] drm: Create new structure for HDMI info

[Jose Abreu]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 2/3] drm: parse hf-vsdb

[Jose Abreu]

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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 3/3] drm: clean cached display info

[Jose Abreu]

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;
>  	}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3 1/3] drm: Create new structure for HDMI info

[Ville Syrjälä]

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.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 1/3] drm: Create new structure for HDMI info

[Sharma, Shashank]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3 3/3] drm: clean cached display info

[Daniel Vetter]

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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 3/3] drm: clean cached display info

[Sharma, Shashank]

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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 3/3] drm: clean cached display info

[Jose Abreu]

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 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

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_mailman_listinfo_intel-2Dgfx&d=DgIC-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=1G5dnBp7Y6VEifpEnDT2wKFoDRBXnxGXAnA-4883H74&s=y1M2ce128zpR_lBDPSgS_JGm-HoPIJjneK2s3tkrvyo&e=
>>
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 3/3] drm: clean cached display info

[Sharma, Shashank]

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 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
> 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_mailman_listinfo_intel-2Dgfx&d=DgIC-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=1G5dnBp7Y6VEifpEnDT2wKFoDRBXnxGXAnA-4883H74&s=y1M2ce128zpR_lBDPSgS_JGm-HoPIJjneK2s3tkrvyo&e=

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx