On 2/27/23 12:12, Jani Nikula wrote:
> 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.
>
This patch itself is okay but without further work can't be used by
drivers since they don't currently have an indication whether a mode
is based on a VII timing.
Harry
>
> 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 */
>>
>
