On Mon, 27 Feb 2023, Harry Wentland <harry.wentland@xxxxxxx> wrote:
> On 2/26/23 09:10, Yaroslav Bolyukin wrote:
>> As per DisplayID v2.0 Errata E9 spec "DSC pass-through timing support"
>> VESA vendor-specific data block may contain target DSC bits per pixel
>> fields
>>
>
> According to the errata this should only apply to VII timings. The way
> it is currently implemented will make it apply to everything which is
> not what we want.
>
> Can we add this field to drm_mode_info instead of drm_display_info and
> set it inside drm_mode_displayid_detailed when parsing a type_7 timing?
That's actually difficult to do nicely. I think the patch at hand is
fine, and it's fine to add the information to drm_display_info. It's a
dependency to parsing the modes.
How the info will actually be used is a different matter, and obviously
needs to follow the spec. As it is, *this* patch doesn't say anything
about that. But whether it's handled in VII timings parsing or
elsewhere, I still think this part is fine.
BR,
Jani.
>
> Harry
>
>
>> Signed-off-by: Yaroslav Bolyukin <iam@xxxxxxx>
>> ---
>> drivers/gpu/drm/drm_edid.c | 38 +++++++++++++++++++++++++------------
>> include/drm/drm_connector.h | 6 ++++++
>> include/drm/drm_displayid.h | 4 ++++
>> 3 files changed, 36 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 3d0a4da661bc..aa88ac82cbe0 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -6338,7 +6338,7 @@ static void drm_parse_vesa_mso_data(struct drm_connector *connector,
>> if (oui(vesa->oui[0], vesa->oui[1], vesa->oui[2]) != VESA_IEEE_OUI)
>> return;
>>
>> - if (sizeof(*vesa) != sizeof(*block) + block->num_bytes) {
>> + if (block->num_bytes < 5) {
>> drm_dbg_kms(connector->dev,
>> "[CONNECTOR:%d:%s] Unexpected VESA vendor block size\n",
>> connector->base.id, connector->name);
>> @@ -6361,24 +6361,37 @@ static void drm_parse_vesa_mso_data(struct drm_connector *connector,
>> break;
>> }
>>
>> - if (!info->mso_stream_count) {
>> - info->mso_pixel_overlap = 0;
>> - return;
>> - }
>> + info->mso_pixel_overlap = 0;
>> +
>> + if (info->mso_stream_count) {
>> + info->mso_pixel_overlap = FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP, vesa->mso);
>> +
>> + if (info->mso_pixel_overlap > 8) {
>> + drm_dbg_kms(connector->dev,
>> + "[CONNECTOR:%d:%s] Reserved MSO pixel overlap value %u\n",
>> + connector->base.id, connector->name,
>> + info->mso_pixel_overlap);
>> + info->mso_pixel_overlap = 8;
>> + }
>>
>> - info->mso_pixel_overlap = FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP, vesa->mso);
>> - if (info->mso_pixel_overlap > 8) {
>> drm_dbg_kms(connector->dev,
>> - "[CONNECTOR:%d:%s] Reserved MSO pixel overlap value %u\n",
>> + "[CONNECTOR:%d:%s] MSO stream count %u, pixel overlap %u\n",
>> connector->base.id, connector->name,
>> - info->mso_pixel_overlap);
>> - info->mso_pixel_overlap = 8;
>> + info->mso_stream_count, info->mso_pixel_overlap);
>> + }
>> +
>> + if (block->num_bytes < 7) {
>> + /* DSC bpp is optional */
>> + return;
>> }
>>
>> + info->dp_dsc_bpp = FIELD_GET(DISPLAYID_VESA_DSC_BPP_INT, vesa->dsc_bpp_int) * 16
>> + + FIELD_GET(DISPLAYID_VESA_DSC_BPP_FRACT, vesa->dsc_bpp_fract);
>> +
>> drm_dbg_kms(connector->dev,
>> - "[CONNECTOR:%d:%s] MSO stream count %u, pixel overlap %u\n",
>> + "[CONNECTOR:%d:%s] DSC bits per pixel %u\n",
>> connector->base.id, connector->name,
>> - info->mso_stream_count, info->mso_pixel_overlap);
>> + info->dp_dsc_bpp);
>> }
>>
>> static void drm_update_mso(struct drm_connector *connector,
>> @@ -6425,6 +6438,7 @@ static void drm_reset_display_info(struct drm_connector *connector)
>> info->mso_stream_count = 0;
>> info->mso_pixel_overlap = 0;
>> info->max_dsc_bpp = 0;
>> + info->dp_dsc_bpp = 0;
>>
>> kfree(info->vics);
>> info->vics = NULL;
>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>> index 7b5048516185..1d01e0146a7f 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -719,6 +719,12 @@ struct drm_display_info {
>> */
>> u32 max_dsc_bpp;
>>
>> + /**
>> + * @dp_dsc_bpp: DP Display-Stream-Compression (DSC) timing's target
>> + * DST bits per pixel in 6.4 fixed point format. 0 means undefined
>> + */
>> + u16 dp_dsc_bpp;
>> +
>> /**
>> * @vics: Array of vics_len VICs. Internal to EDID parsing.
>> */
>> diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h
>> index 49649eb8447e..0fc3afbd1675 100644
>> --- a/include/drm/drm_displayid.h
>> +++ b/include/drm/drm_displayid.h
>> @@ -131,12 +131,16 @@ struct displayid_detailed_timing_block {
>>
>> #define DISPLAYID_VESA_MSO_OVERLAP GENMASK(3, 0)
>> #define DISPLAYID_VESA_MSO_MODE GENMASK(6, 5)
>> +#define DISPLAYID_VESA_DSC_BPP_INT GENMASK(5, 0)
>> +#define DISPLAYID_VESA_DSC_BPP_FRACT GENMASK(3, 0)
>>
>> struct displayid_vesa_vendor_specific_block {
>> struct displayid_block base;
>> u8 oui[3];
>> u8 data_structure_type;
>> u8 mso;
>> + u8 dsc_bpp_int;
>> + u8 dsc_bpp_fract;
>> } __packed;
>>
>> /* DisplayID iteration */
>
--
Jani Nikula, Intel Open Source Graphics Center
