diff mbox series

[v2,4/6] drm/edid: parse the DisplayID v2.0 VESA vendor block for MSO

Message ID 73ca2887e7b37880690f5c9ba4594c9cd1170669.1630419362.git.jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/displayid: VESA vendor block and drm/i915 MSO use of it | expand

Commit Message

Jani Nikula Aug. 31, 2021, 2:17 p.m. UTC
The VESA Organization Vendor-Specific Data Block, defined in VESA
DisplayID Standard v2.0, specifies the eDP Multi-SST Operation (MSO)
stream count and segment pixel overlap.

DisplayID v1.3 has Appendix B: DisplayID as an EDID Extension,
describing how DisplayID sections may be embedded in EDID extension
blocks. DisplayID v2.0 does not have such a section, perhaps implying
that DisplayID v2.0 data should not be included in EDID extensions, but
rather in a "pure" DisplayID structure at its own DDC address pair
A4h/A5h, as described in VESA E-DDC Standard v1.3 chapter 3.

However, in practice, displays out in the field have embedded DisplayID
v2.0 data blocks in EDID extensions, including, in particular, some eDP
MSO displays, where a pure DisplayID structure is not available at all.

Parse the MSO data from the DisplayID data block. Do it as part of
drm_add_display_info(), extending it to parse also DisplayID data to
avoid requiring extra calls to update the information.

v2: Check for VESA OUI (Ville)

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_edid.c  | 72 +++++++++++++++++++++++++++++++++++++
 include/drm/drm_connector.h | 12 +++++++
 include/drm/drm_displayid.h | 13 +++++++
 3 files changed, 97 insertions(+)

Comments

Shankar, Uma Sept. 13, 2021, 6:23 a.m. UTC | #1
> -----Original Message-----
> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of Jani Nikula
> Sent: Tuesday, August 31, 2021 7:48 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; ville.syrjala@linux.intel.com; Nikula, Jani
> <jani.nikula@intel.com>
> Subject: [PATCH v2 4/6] drm/edid: parse the DisplayID v2.0 VESA vendor block for
> MSO
> 
> The VESA Organization Vendor-Specific Data Block, defined in VESA DisplayID
> Standard v2.0, specifies the eDP Multi-SST Operation (MSO) stream count and
> segment pixel overlap.
> 
> DisplayID v1.3 has Appendix B: DisplayID as an EDID Extension, describing how
> DisplayID sections may be embedded in EDID extension blocks. DisplayID v2.0 does
> not have such a section, perhaps implying that DisplayID v2.0 data should not be
> included in EDID extensions, but rather in a "pure" DisplayID structure at its own DDC
> address pair A4h/A5h, as described in VESA E-DDC Standard v1.3 chapter 3.
> 
> However, in practice, displays out in the field have embedded DisplayID
> v2.0 data blocks in EDID extensions, including, in particular, some eDP MSO displays,
> where a pure DisplayID structure is not available at all.
> 
> Parse the MSO data from the DisplayID data block. Do it as part of
> drm_add_display_info(), extending it to parse also DisplayID data to avoid requiring
> extra calls to update the information.
> 
> v2: Check for VESA OUI (Ville)
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c  | 72 +++++++++++++++++++++++++++++++++++++
>  include/drm/drm_connector.h | 12 +++++++  include/drm/drm_displayid.h | 13
> +++++++
>  3 files changed, 97 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index
> 92974b1478bc..c45c225267ca 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -28,6 +28,7 @@
>   * DEALINGS IN THE SOFTWARE.
>   */
> 
> +#include <linux/bitfield.h>
>  #include <linux/hdmi.h>
>  #include <linux/i2c.h>
>  #include <linux/kernel.h>
> @@ -5145,6 +5146,71 @@ void drm_get_monitor_range(struct drm_connector
> *connector,
>  		      info->monitor_range.max_vfreq);  }
> 
> +static void drm_parse_vesa_mso_data(struct drm_connector *connector,
> +				    const struct displayid_block *block) {
> +	struct displayid_vesa_vendor_specific_block *vesa =
> +		(struct displayid_vesa_vendor_specific_block *)block;
> +	struct drm_display_info *info = &connector->display_info;
> +
> +	if (block->num_bytes < 3) {
> +		drm_dbg_kms(connector->dev, "Unexpected vendor block size
> %u\n",
> +			    block->num_bytes);
> +		return;
> +	}
> +
> +	if (oui(vesa->oui[0], vesa->oui[1], vesa->oui[2]) != VESA_IEEE_OUI)
> +		return;
> +
> +	if (sizeof(*vesa) != sizeof(*block) + block->num_bytes) {
> +		drm_dbg_kms(connector->dev, "Unexpected VESA vendor block
> size\n");
> +		return;
> +	}
> +
> +	switch (FIELD_GET(DISPLAYID_VESA_MSO_MODE, vesa->mso)) {
> +	default:
> +		drm_dbg_kms(connector->dev, "Reserved MSO mode value\n");
> +		fallthrough;
> +	case 0:
> +		info->mso_stream_count = 0;
> +		break;
> +	case 1:
> +		info->mso_stream_count = 2; /* 2 or 4 links */
> +		break;
> +	case 2:
> +		info->mso_stream_count = 4; /* 4 links */
> +		break;
> +	}
> +
> +	if (!info->mso_stream_count) {
> +		info->mso_pixel_overlap = 0;
> +		return;
> +	}
> +
> +	info->mso_pixel_overlap = FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP,
> vesa->mso);
> +	if (info->mso_pixel_overlap > 8) {
> +		drm_dbg_kms(connector->dev, "Reserved MSO pixel overlap value
> %u\n",
> +			    info->mso_pixel_overlap);
> +		info->mso_pixel_overlap = 8;

Going beyond 8 is not right from a vendor perspective as it goes into reserved region.
Should we not just set to 0 or how we decide that we fixed overlap at 8. It seems an
undefined operation and it may vary from sink to sink.

Regards,
Uma Shankar

> +	}
> +
> +	drm_dbg_kms(connector->dev, "MSO stream count %u, pixel overlap %u\n",
> +		    info->mso_stream_count, info->mso_pixel_overlap); }
> +
> +static void drm_update_mso(struct drm_connector *connector, const
> +struct edid *edid) {
> +	const struct displayid_block *block;
> +	struct displayid_iter iter;
> +
> +	displayid_iter_edid_begin(edid, &iter);
> +	displayid_iter_for_each(block, &iter) {
> +		if (block->tag == DATA_BLOCK_2_VENDOR_SPECIFIC)
> +			drm_parse_vesa_mso_data(connector, block);
> +	}
> +	displayid_iter_end(&iter);
> +}
> +
>  /* 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
>   */
> @@ -5168,6 +5234,9 @@ drm_reset_display_info(struct drm_connector
> *connector)
> 
>  	info->non_desktop = 0;
>  	memset(&info->monitor_range, 0, sizeof(info->monitor_range));
> +
> +	info->mso_stream_count = 0;
> +	info->mso_pixel_overlap = 0;
>  }
> 
>  u32 drm_add_display_info(struct drm_connector *connector, const struct edid
> *edid) @@ -5246,6 +5315,9 @@ u32 drm_add_display_info(struct drm_connector
> *connector, const struct edid *edi
>  		info->color_formats |= DRM_COLOR_FORMAT_YCRCB444;
>  	if (edid->features & DRM_EDID_FEATURE_RGB_YCRCB422)
>  		info->color_formats |= DRM_COLOR_FORMAT_YCRCB422;
> +
> +	drm_update_mso(connector, edid);
> +
>  	return quirks;
>  }
> 
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index
> 79fa34e5ccdb..379746d3266f 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -590,6 +590,18 @@ struct drm_display_info {
>  	 * @monitor_range: Frequency range supported by monitor range descriptor
>  	 */
>  	struct drm_monitor_range_info monitor_range;
> +
> +	/**
> +	 * @mso_stream_count: eDP Multi-SST Operation (MSO) stream count from
> +	 * the DisplayID VESA vendor block. 0 for conventional Single-Stream
> +	 * Transport (SST), or 2 or 4 MSO streams.
> +	 */
> +	u8 mso_stream_count;
> +
> +	/**
> +	 * @mso_pixel_overlap: eDP MSO segment pixel overlap, 0-8 pixels.
> +	 */
> +	u8 mso_pixel_overlap;
>  };
> 
>  int drm_display_info_set_bus_formats(struct drm_display_info *info, diff --git
> a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h index
> 79771091771a..7ffbd9f7bfc7 100644
> --- a/include/drm/drm_displayid.h
> +++ b/include/drm/drm_displayid.h
> @@ -23,9 +23,12 @@
>  #define DRM_DISPLAYID_H
> 
>  #include <linux/types.h>
> +#include <linux/bits.h>
> 
>  struct edid;
> 
> +#define VESA_IEEE_OUI				0x3a0292
> +
>  /* DisplayID Structure versions */
>  #define DISPLAY_ID_STRUCTURE_VER_12		0x12
>  #define DISPLAY_ID_STRUCTURE_VER_20		0x20
> @@ -126,6 +129,16 @@ struct displayid_detailed_timing_block {
>  	struct displayid_detailed_timings_1 timings[];  };
> 
> +#define DISPLAYID_VESA_MSO_OVERLAP	GENMASK(3, 0)
> +#define DISPLAYID_VESA_MSO_MODE		GENMASK(6, 5)
> +
> +struct displayid_vesa_vendor_specific_block {
> +	struct displayid_block base;
> +	u8 oui[3];
> +	u8 data_structure_type;
> +	u8 mso;
> +} __packed;
> +
>  /* DisplayID iteration */
>  struct displayid_iter {
>  	const struct edid *edid;
> --
> 2.30.2
Jani Nikula Sept. 13, 2021, 9:30 a.m. UTC | #2
On Mon, 13 Sep 2021, "Shankar, Uma" <uma.shankar@intel.com> wrote:
>> -----Original Message-----
>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of Jani Nikula
>> Sent: Tuesday, August 31, 2021 7:48 PM
>> To: intel-gfx@lists.freedesktop.org
>> Cc: dri-devel@lists.freedesktop.org; ville.syrjala@linux.intel.com; Nikula, Jani
>> <jani.nikula@intel.com>
>> Subject: [PATCH v2 4/6] drm/edid: parse the DisplayID v2.0 VESA vendor block for
>> MSO
>> 
>> The VESA Organization Vendor-Specific Data Block, defined in VESA DisplayID
>> Standard v2.0, specifies the eDP Multi-SST Operation (MSO) stream count and
>> segment pixel overlap.
>> 
>> DisplayID v1.3 has Appendix B: DisplayID as an EDID Extension, describing how
>> DisplayID sections may be embedded in EDID extension blocks. DisplayID v2.0 does
>> not have such a section, perhaps implying that DisplayID v2.0 data should not be
>> included in EDID extensions, but rather in a "pure" DisplayID structure at its own DDC
>> address pair A4h/A5h, as described in VESA E-DDC Standard v1.3 chapter 3.
>> 
>> However, in practice, displays out in the field have embedded DisplayID
>> v2.0 data blocks in EDID extensions, including, in particular, some eDP MSO displays,
>> where a pure DisplayID structure is not available at all.
>> 
>> Parse the MSO data from the DisplayID data block. Do it as part of
>> drm_add_display_info(), extending it to parse also DisplayID data to avoid requiring
>> extra calls to update the information.
>> 
>> v2: Check for VESA OUI (Ville)
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/drm_edid.c  | 72 +++++++++++++++++++++++++++++++++++++
>>  include/drm/drm_connector.h | 12 +++++++  include/drm/drm_displayid.h | 13
>> +++++++
>>  3 files changed, 97 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index
>> 92974b1478bc..c45c225267ca 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -28,6 +28,7 @@
>>   * DEALINGS IN THE SOFTWARE.
>>   */
>> 
>> +#include <linux/bitfield.h>
>>  #include <linux/hdmi.h>
>>  #include <linux/i2c.h>
>>  #include <linux/kernel.h>
>> @@ -5145,6 +5146,71 @@ void drm_get_monitor_range(struct drm_connector
>> *connector,
>>  		      info->monitor_range.max_vfreq);  }
>> 
>> +static void drm_parse_vesa_mso_data(struct drm_connector *connector,
>> +				    const struct displayid_block *block) {
>> +	struct displayid_vesa_vendor_specific_block *vesa =
>> +		(struct displayid_vesa_vendor_specific_block *)block;
>> +	struct drm_display_info *info = &connector->display_info;
>> +
>> +	if (block->num_bytes < 3) {
>> +		drm_dbg_kms(connector->dev, "Unexpected vendor block size
>> %u\n",
>> +			    block->num_bytes);
>> +		return;
>> +	}
>> +
>> +	if (oui(vesa->oui[0], vesa->oui[1], vesa->oui[2]) != VESA_IEEE_OUI)
>> +		return;
>> +
>> +	if (sizeof(*vesa) != sizeof(*block) + block->num_bytes) {
>> +		drm_dbg_kms(connector->dev, "Unexpected VESA vendor block
>> size\n");
>> +		return;
>> +	}
>> +
>> +	switch (FIELD_GET(DISPLAYID_VESA_MSO_MODE, vesa->mso)) {
>> +	default:
>> +		drm_dbg_kms(connector->dev, "Reserved MSO mode value\n");
>> +		fallthrough;
>> +	case 0:
>> +		info->mso_stream_count = 0;
>> +		break;
>> +	case 1:
>> +		info->mso_stream_count = 2; /* 2 or 4 links */
>> +		break;
>> +	case 2:
>> +		info->mso_stream_count = 4; /* 4 links */
>> +		break;
>> +	}
>> +
>> +	if (!info->mso_stream_count) {
>> +		info->mso_pixel_overlap = 0;
>> +		return;
>> +	}
>> +
>> +	info->mso_pixel_overlap = FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP,
>> vesa->mso);
>> +	if (info->mso_pixel_overlap > 8) {
>> +		drm_dbg_kms(connector->dev, "Reserved MSO pixel overlap value
>> %u\n",
>> +			    info->mso_pixel_overlap);
>> +		info->mso_pixel_overlap = 8;
>
> Going beyond 8 is not right from a vendor perspective as it goes into reserved region.
> Should we not just set to 0 or how we decide that we fixed overlap at 8. It seems an
> undefined operation and it may vary from sink to sink.

I don't know if there's a right choice here. I don't mind setting it to
0 if you prefer that.

BR,
Jani.


>
> Regards,
> Uma Shankar
>
>> +	}
>> +
>> +	drm_dbg_kms(connector->dev, "MSO stream count %u, pixel overlap %u\n",
>> +		    info->mso_stream_count, info->mso_pixel_overlap); }
>> +
>> +static void drm_update_mso(struct drm_connector *connector, const
>> +struct edid *edid) {
>> +	const struct displayid_block *block;
>> +	struct displayid_iter iter;
>> +
>> +	displayid_iter_edid_begin(edid, &iter);
>> +	displayid_iter_for_each(block, &iter) {
>> +		if (block->tag == DATA_BLOCK_2_VENDOR_SPECIFIC)
>> +			drm_parse_vesa_mso_data(connector, block);
>> +	}
>> +	displayid_iter_end(&iter);
>> +}
>> +
>>  /* 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
>>   */
>> @@ -5168,6 +5234,9 @@ drm_reset_display_info(struct drm_connector
>> *connector)
>> 
>>  	info->non_desktop = 0;
>>  	memset(&info->monitor_range, 0, sizeof(info->monitor_range));
>> +
>> +	info->mso_stream_count = 0;
>> +	info->mso_pixel_overlap = 0;
>>  }
>> 
>>  u32 drm_add_display_info(struct drm_connector *connector, const struct edid
>> *edid) @@ -5246,6 +5315,9 @@ u32 drm_add_display_info(struct drm_connector
>> *connector, const struct edid *edi
>>  		info->color_formats |= DRM_COLOR_FORMAT_YCRCB444;
>>  	if (edid->features & DRM_EDID_FEATURE_RGB_YCRCB422)
>>  		info->color_formats |= DRM_COLOR_FORMAT_YCRCB422;
>> +
>> +	drm_update_mso(connector, edid);
>> +
>>  	return quirks;
>>  }
>> 
>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index
>> 79fa34e5ccdb..379746d3266f 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -590,6 +590,18 @@ struct drm_display_info {
>>  	 * @monitor_range: Frequency range supported by monitor range descriptor
>>  	 */
>>  	struct drm_monitor_range_info monitor_range;
>> +
>> +	/**
>> +	 * @mso_stream_count: eDP Multi-SST Operation (MSO) stream count from
>> +	 * the DisplayID VESA vendor block. 0 for conventional Single-Stream
>> +	 * Transport (SST), or 2 or 4 MSO streams.
>> +	 */
>> +	u8 mso_stream_count;
>> +
>> +	/**
>> +	 * @mso_pixel_overlap: eDP MSO segment pixel overlap, 0-8 pixels.
>> +	 */
>> +	u8 mso_pixel_overlap;
>>  };
>> 
>>  int drm_display_info_set_bus_formats(struct drm_display_info *info, diff --git
>> a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h index
>> 79771091771a..7ffbd9f7bfc7 100644
>> --- a/include/drm/drm_displayid.h
>> +++ b/include/drm/drm_displayid.h
>> @@ -23,9 +23,12 @@
>>  #define DRM_DISPLAYID_H
>> 
>>  #include <linux/types.h>
>> +#include <linux/bits.h>
>> 
>>  struct edid;
>> 
>> +#define VESA_IEEE_OUI				0x3a0292
>> +
>>  /* DisplayID Structure versions */
>>  #define DISPLAY_ID_STRUCTURE_VER_12		0x12
>>  #define DISPLAY_ID_STRUCTURE_VER_20		0x20
>> @@ -126,6 +129,16 @@ struct displayid_detailed_timing_block {
>>  	struct displayid_detailed_timings_1 timings[];  };
>> 
>> +#define DISPLAYID_VESA_MSO_OVERLAP	GENMASK(3, 0)
>> +#define DISPLAYID_VESA_MSO_MODE		GENMASK(6, 5)
>> +
>> +struct displayid_vesa_vendor_specific_block {
>> +	struct displayid_block base;
>> +	u8 oui[3];
>> +	u8 data_structure_type;
>> +	u8 mso;
>> +} __packed;
>> +
>>  /* DisplayID iteration */
>>  struct displayid_iter {
>>  	const struct edid *edid;
>> --
>> 2.30.2
>
Shankar, Uma Sept. 13, 2021, 11:21 a.m. UTC | #3
> -----Original Message-----
> From: Nikula, Jani <jani.nikula@intel.com>
> Sent: Monday, September 13, 2021 3:00 PM
> To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; ville.syrjala@linux.intel.com
> Subject: RE: [PATCH v2 4/6] drm/edid: parse the DisplayID v2.0 VESA vendor block
> for MSO
> 
> On Mon, 13 Sep 2021, "Shankar, Uma" <uma.shankar@intel.com> wrote:
> >> -----Original Message-----
> >> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf
> >> Of Jani Nikula
> >> Sent: Tuesday, August 31, 2021 7:48 PM
> >> To: intel-gfx@lists.freedesktop.org
> >> Cc: dri-devel@lists.freedesktop.org; ville.syrjala@linux.intel.com;
> >> Nikula, Jani <jani.nikula@intel.com>
> >> Subject: [PATCH v2 4/6] drm/edid: parse the DisplayID v2.0 VESA
> >> vendor block for MSO
> >>
> >> The VESA Organization Vendor-Specific Data Block, defined in VESA
> >> DisplayID Standard v2.0, specifies the eDP Multi-SST Operation (MSO)
> >> stream count and segment pixel overlap.
> >>
> >> DisplayID v1.3 has Appendix B: DisplayID as an EDID Extension,
> >> describing how DisplayID sections may be embedded in EDID extension
> >> blocks. DisplayID v2.0 does not have such a section, perhaps implying
> >> that DisplayID v2.0 data should not be included in EDID extensions,
> >> but rather in a "pure" DisplayID structure at its own DDC address pair A4h/A5h, as
> described in VESA E-DDC Standard v1.3 chapter 3.
> >>
> >> However, in practice, displays out in the field have embedded
> >> DisplayID
> >> v2.0 data blocks in EDID extensions, including, in particular, some
> >> eDP MSO displays, where a pure DisplayID structure is not available at all.
> >>
> >> Parse the MSO data from the DisplayID data block. Do it as part of
> >> drm_add_display_info(), extending it to parse also DisplayID data to
> >> avoid requiring extra calls to update the information.
> >>
> >> v2: Check for VESA OUI (Ville)
> >>
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >>  drivers/gpu/drm/drm_edid.c  | 72
> >> +++++++++++++++++++++++++++++++++++++
> >>  include/drm/drm_connector.h | 12 +++++++
> >> include/drm/drm_displayid.h | 13
> >> +++++++
> >>  3 files changed, 97 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> >> index 92974b1478bc..c45c225267ca 100644
> >> --- a/drivers/gpu/drm/drm_edid.c
> >> +++ b/drivers/gpu/drm/drm_edid.c
> >> @@ -28,6 +28,7 @@
> >>   * DEALINGS IN THE SOFTWARE.
> >>   */
> >>
> >> +#include <linux/bitfield.h>
> >>  #include <linux/hdmi.h>
> >>  #include <linux/i2c.h>
> >>  #include <linux/kernel.h>
> >> @@ -5145,6 +5146,71 @@ void drm_get_monitor_range(struct
> >> drm_connector *connector,
> >>  		      info->monitor_range.max_vfreq);  }
> >>
> >> +static void drm_parse_vesa_mso_data(struct drm_connector *connector,
> >> +				    const struct displayid_block *block) {
> >> +	struct displayid_vesa_vendor_specific_block *vesa =
> >> +		(struct displayid_vesa_vendor_specific_block *)block;
> >> +	struct drm_display_info *info = &connector->display_info;
> >> +
> >> +	if (block->num_bytes < 3) {
> >> +		drm_dbg_kms(connector->dev, "Unexpected vendor block size
> >> %u\n",
> >> +			    block->num_bytes);
> >> +		return;
> >> +	}
> >> +
> >> +	if (oui(vesa->oui[0], vesa->oui[1], vesa->oui[2]) != VESA_IEEE_OUI)
> >> +		return;
> >> +
> >> +	if (sizeof(*vesa) != sizeof(*block) + block->num_bytes) {
> >> +		drm_dbg_kms(connector->dev, "Unexpected VESA vendor block
> >> size\n");
> >> +		return;
> >> +	}
> >> +
> >> +	switch (FIELD_GET(DISPLAYID_VESA_MSO_MODE, vesa->mso)) {
> >> +	default:
> >> +		drm_dbg_kms(connector->dev, "Reserved MSO mode value\n");
> >> +		fallthrough;
> >> +	case 0:
> >> +		info->mso_stream_count = 0;
> >> +		break;
> >> +	case 1:
> >> +		info->mso_stream_count = 2; /* 2 or 4 links */
> >> +		break;
> >> +	case 2:
> >> +		info->mso_stream_count = 4; /* 4 links */
> >> +		break;
> >> +	}
> >> +
> >> +	if (!info->mso_stream_count) {
> >> +		info->mso_pixel_overlap = 0;
> >> +		return;
> >> +	}
> >> +
> >> +	info->mso_pixel_overlap = FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP,
> >> vesa->mso);
> >> +	if (info->mso_pixel_overlap > 8) {
> >> +		drm_dbg_kms(connector->dev, "Reserved MSO pixel overlap value
> >> %u\n",
> >> +			    info->mso_pixel_overlap);
> >> +		info->mso_pixel_overlap = 8;
> >
> > Going beyond 8 is not right from a vendor perspective as it goes into reserved
> region.
> > Should we not just set to 0 or how we decide that we fixed overlap at
> > 8. It seems an undefined operation and it may vary from sink to sink.
> 
> I don't know if there's a right choice here. I don't mind setting it to
> 0 if you prefer that.

I feel it will be wrong on a vendor's behalf if he programs anything above 8. But not sure what
should driver do in such case, it's an undefined behavior. If he intentionally programs it as say 20,
fixing at 8 will still lead to some artifacts. 0 only helps in case where this field is corrupted and has
garbage data.

No strong objection, I will leave to your discretion Jani.

With this, all else looks good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

Regards,
Uma Shankar

> BR,
> Jani.
> 
> 
> >
> > Regards,
> > Uma Shankar
> >
> >> +	}
> >> +
> >> +	drm_dbg_kms(connector->dev, "MSO stream count %u, pixel overlap %u\n",
> >> +		    info->mso_stream_count, info->mso_pixel_overlap); }
> >> +
> >> +static void drm_update_mso(struct drm_connector *connector, const
> >> +struct edid *edid) {
> >> +	const struct displayid_block *block;
> >> +	struct displayid_iter iter;
> >> +
> >> +	displayid_iter_edid_begin(edid, &iter);
> >> +	displayid_iter_for_each(block, &iter) {
> >> +		if (block->tag == DATA_BLOCK_2_VENDOR_SPECIFIC)
> >> +			drm_parse_vesa_mso_data(connector, block);
> >> +	}
> >> +	displayid_iter_end(&iter);
> >> +}
> >> +
> >>  /* 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
> >>   */
> >> @@ -5168,6 +5234,9 @@ drm_reset_display_info(struct drm_connector
> >> *connector)
> >>
> >>  	info->non_desktop = 0;
> >>  	memset(&info->monitor_range, 0, sizeof(info->monitor_range));
> >> +
> >> +	info->mso_stream_count = 0;
> >> +	info->mso_pixel_overlap = 0;
> >>  }
> >>
> >>  u32 drm_add_display_info(struct drm_connector *connector, const
> >> struct edid
> >> *edid) @@ -5246,6 +5315,9 @@ u32 drm_add_display_info(struct
> >> drm_connector *connector, const struct edid *edi
> >>  		info->color_formats |= DRM_COLOR_FORMAT_YCRCB444;
> >>  	if (edid->features & DRM_EDID_FEATURE_RGB_YCRCB422)
> >>  		info->color_formats |= DRM_COLOR_FORMAT_YCRCB422;
> >> +
> >> +	drm_update_mso(connector, edid);
> >> +
> >>  	return quirks;
> >>  }
> >>
> >> diff --git a/include/drm/drm_connector.h
> >> b/include/drm/drm_connector.h index 79fa34e5ccdb..379746d3266f 100644
> >> --- a/include/drm/drm_connector.h
> >> +++ b/include/drm/drm_connector.h
> >> @@ -590,6 +590,18 @@ struct drm_display_info {
> >>  	 * @monitor_range: Frequency range supported by monitor range descriptor
> >>  	 */
> >>  	struct drm_monitor_range_info monitor_range;
> >> +
> >> +	/**
> >> +	 * @mso_stream_count: eDP Multi-SST Operation (MSO) stream count from
> >> +	 * the DisplayID VESA vendor block. 0 for conventional Single-Stream
> >> +	 * Transport (SST), or 2 or 4 MSO streams.
> >> +	 */
> >> +	u8 mso_stream_count;
> >> +
> >> +	/**
> >> +	 * @mso_pixel_overlap: eDP MSO segment pixel overlap, 0-8 pixels.
> >> +	 */
> >> +	u8 mso_pixel_overlap;
> >>  };
> >>
> >>  int drm_display_info_set_bus_formats(struct drm_display_info *info,
> >> diff --git a/include/drm/drm_displayid.h
> >> b/include/drm/drm_displayid.h index
> >> 79771091771a..7ffbd9f7bfc7 100644
> >> --- a/include/drm/drm_displayid.h
> >> +++ b/include/drm/drm_displayid.h
> >> @@ -23,9 +23,12 @@
> >>  #define DRM_DISPLAYID_H
> >>
> >>  #include <linux/types.h>
> >> +#include <linux/bits.h>
> >>
> >>  struct edid;
> >>
> >> +#define VESA_IEEE_OUI				0x3a0292
> >> +
> >>  /* DisplayID Structure versions */
> >>  #define DISPLAY_ID_STRUCTURE_VER_12		0x12
> >>  #define DISPLAY_ID_STRUCTURE_VER_20		0x20
> >> @@ -126,6 +129,16 @@ struct displayid_detailed_timing_block {
> >>  	struct displayid_detailed_timings_1 timings[];  };
> >>
> >> +#define DISPLAYID_VESA_MSO_OVERLAP	GENMASK(3, 0)
> >> +#define DISPLAYID_VESA_MSO_MODE		GENMASK(6, 5)
> >> +
> >> +struct displayid_vesa_vendor_specific_block {
> >> +	struct displayid_block base;
> >> +	u8 oui[3];
> >> +	u8 data_structure_type;
> >> +	u8 mso;
> >> +} __packed;
> >> +
> >>  /* DisplayID iteration */
> >>  struct displayid_iter {
> >>  	const struct edid *edid;
> >> --
> >> 2.30.2
> >
> 
> --
> Jani Nikula, Intel Open Source Graphics Center
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 92974b1478bc..c45c225267ca 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -28,6 +28,7 @@ 
  * DEALINGS IN THE SOFTWARE.
  */
 
+#include <linux/bitfield.h>
 #include <linux/hdmi.h>
 #include <linux/i2c.h>
 #include <linux/kernel.h>
@@ -5145,6 +5146,71 @@  void drm_get_monitor_range(struct drm_connector *connector,
 		      info->monitor_range.max_vfreq);
 }
 
+static void drm_parse_vesa_mso_data(struct drm_connector *connector,
+				    const struct displayid_block *block)
+{
+	struct displayid_vesa_vendor_specific_block *vesa =
+		(struct displayid_vesa_vendor_specific_block *)block;
+	struct drm_display_info *info = &connector->display_info;
+
+	if (block->num_bytes < 3) {
+		drm_dbg_kms(connector->dev, "Unexpected vendor block size %u\n",
+			    block->num_bytes);
+		return;
+	}
+
+	if (oui(vesa->oui[0], vesa->oui[1], vesa->oui[2]) != VESA_IEEE_OUI)
+		return;
+
+	if (sizeof(*vesa) != sizeof(*block) + block->num_bytes) {
+		drm_dbg_kms(connector->dev, "Unexpected VESA vendor block size\n");
+		return;
+	}
+
+	switch (FIELD_GET(DISPLAYID_VESA_MSO_MODE, vesa->mso)) {
+	default:
+		drm_dbg_kms(connector->dev, "Reserved MSO mode value\n");
+		fallthrough;
+	case 0:
+		info->mso_stream_count = 0;
+		break;
+	case 1:
+		info->mso_stream_count = 2; /* 2 or 4 links */
+		break;
+	case 2:
+		info->mso_stream_count = 4; /* 4 links */
+		break;
+	}
+
+	if (!info->mso_stream_count) {
+		info->mso_pixel_overlap = 0;
+		return;
+	}
+
+	info->mso_pixel_overlap = FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP, vesa->mso);
+	if (info->mso_pixel_overlap > 8) {
+		drm_dbg_kms(connector->dev, "Reserved MSO pixel overlap value %u\n",
+			    info->mso_pixel_overlap);
+		info->mso_pixel_overlap = 8;
+	}
+
+	drm_dbg_kms(connector->dev, "MSO stream count %u, pixel overlap %u\n",
+		    info->mso_stream_count, info->mso_pixel_overlap);
+}
+
+static void drm_update_mso(struct drm_connector *connector, const struct edid *edid)
+{
+	const struct displayid_block *block;
+	struct displayid_iter iter;
+
+	displayid_iter_edid_begin(edid, &iter);
+	displayid_iter_for_each(block, &iter) {
+		if (block->tag == DATA_BLOCK_2_VENDOR_SPECIFIC)
+			drm_parse_vesa_mso_data(connector, block);
+	}
+	displayid_iter_end(&iter);
+}
+
 /* 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
  */
@@ -5168,6 +5234,9 @@  drm_reset_display_info(struct drm_connector *connector)
 
 	info->non_desktop = 0;
 	memset(&info->monitor_range, 0, sizeof(info->monitor_range));
+
+	info->mso_stream_count = 0;
+	info->mso_pixel_overlap = 0;
 }
 
 u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edid)
@@ -5246,6 +5315,9 @@  u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi
 		info->color_formats |= DRM_COLOR_FORMAT_YCRCB444;
 	if (edid->features & DRM_EDID_FEATURE_RGB_YCRCB422)
 		info->color_formats |= DRM_COLOR_FORMAT_YCRCB422;
+
+	drm_update_mso(connector, edid);
+
 	return quirks;
 }
 
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 79fa34e5ccdb..379746d3266f 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -590,6 +590,18 @@  struct drm_display_info {
 	 * @monitor_range: Frequency range supported by monitor range descriptor
 	 */
 	struct drm_monitor_range_info monitor_range;
+
+	/**
+	 * @mso_stream_count: eDP Multi-SST Operation (MSO) stream count from
+	 * the DisplayID VESA vendor block. 0 for conventional Single-Stream
+	 * Transport (SST), or 2 or 4 MSO streams.
+	 */
+	u8 mso_stream_count;
+
+	/**
+	 * @mso_pixel_overlap: eDP MSO segment pixel overlap, 0-8 pixels.
+	 */
+	u8 mso_pixel_overlap;
 };
 
 int drm_display_info_set_bus_formats(struct drm_display_info *info,
diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h
index 79771091771a..7ffbd9f7bfc7 100644
--- a/include/drm/drm_displayid.h
+++ b/include/drm/drm_displayid.h
@@ -23,9 +23,12 @@ 
 #define DRM_DISPLAYID_H
 
 #include <linux/types.h>
+#include <linux/bits.h>
 
 struct edid;
 
+#define VESA_IEEE_OUI				0x3a0292
+
 /* DisplayID Structure versions */
 #define DISPLAY_ID_STRUCTURE_VER_12		0x12
 #define DISPLAY_ID_STRUCTURE_VER_20		0x20
@@ -126,6 +129,16 @@  struct displayid_detailed_timing_block {
 	struct displayid_detailed_timings_1 timings[];
 };
 
+#define DISPLAYID_VESA_MSO_OVERLAP	GENMASK(3, 0)
+#define DISPLAYID_VESA_MSO_MODE		GENMASK(6, 5)
+
+struct displayid_vesa_vendor_specific_block {
+	struct displayid_block base;
+	u8 oui[3];
+	u8 data_structure_type;
+	u8 mso;
+} __packed;
+
 /* DisplayID iteration */
 struct displayid_iter {
 	const struct edid *edid;