On 11 October 2013 12:12, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> On Thu, Oct 10, 2013 at 02:19:15PM +0100, Thomas Wood wrote:
>> Parse the 3D_Structure_ALL and 3D_MASK fields of the HDMI Vendor
>> Specific Data Block to expose more stereo 3D modes.
>>
>> Signed-off-by: Thomas Wood <thomas.wood@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/drm_edid.c | 93 ++++++++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 85 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 9e81609..b3949f9 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -2652,6 +2652,50 @@ static int add_hdmi_mode(struct drm_connector *connector, u8 vic)
>> return 1;
>> }
>>
>> +static int add_3d_struct_modes(struct drm_connector *connector, u16 structure,
>> + const u8 *video_db, u8 video_len, u8 video_index)
>> +{
>> + struct drm_device *dev = connector->dev;
>> + struct drm_display_mode *newmode;
>> + int modes = 0;
>> + u8 cea_mode;
>> +
>> + if (video_db == NULL || video_index > video_len)
>> + return 0;
>> +
>> + /* CEA modes are numbered 1..127 */
>> + cea_mode = (video_db[video_index] & 127) - 1;
>> + if (cea_mode >= ARRAY_SIZE(edid_cea_modes))
>> + return 0;
>> +
>> + if (structure & 1) {
>
> Could use (1 << 0) for consistency.
I've updated this for v2.
>
> I'm also wondering if some displays might include some of the mandatory
> modes in 3D_Structure_ALL, and if so should we filter out the
> duplicates?
As Damien mentioned, duplicates are filtered out later on.
>
>> + if ((multi_present == 1 || multi_present == 2) &&
>> + len >= (9 + offset)) {
>
> If multi_present==2 and len is too small for the mask, I think we should
> skip adding the modes since the block is clearly incorrect/corrupted.
>
> So maybe just something like this:
> if ((multi_present == 1 && len < (9 + offset)) ||
> (multi_present == 2 && len < (11 + offset)))
> goto out;
I've added this check to v2 and also made sure multi_present is either 1 or 2
before continuing.
>
> I would also add a similar check for HDMI_3D_LEN since that is
> supposed to include the space required by 3D_Structure_ALL and
> 3D_MASK. Or you could just check HDMI_3D_LEN against 'len' and bail
> out if that doesn't fit. And then you could just check
> 3D_Structure_ALL and 3D_MASK against HDMI_3D_LEN.
>
I've added a check to ensure the value of HDMI_3D_LEN is large enough to
include 3D_Structure_ALL and 3D_MASK, if they are present.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
