On Mon, Sep 16, 2013 at 06:48:48PM +0100, Damien Lespiau wrote: > For now, let's just look at the 3D_present flag of the CEA HDMI vendor > block to detect if the sink supports a small list of then mandatory 3D > formats. > > See the HDMI 1.4a 3D extraction for detail: > http://www.hdmi.org/manufacturer/specification.aspx > > Signed-off-by: Damien Lespiau <damien.lespiau@xxxxxxxxx> > --- > drivers/gpu/drm/drm_edid.c | 108 ++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 101 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index a207cc3..78009d1 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -2550,13 +2550,93 @@ do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len) > return modes; > } > > +struct stereo_mandatory_mode { > + int width, height, freq; Can we call it vrefresh like in drm_display_mode? > + unsigned int interlace_flag, layouts; What's the benefit of separating the two? > +}; > + > +static const struct stereo_mandatory_mode stereo_mandatory_modes[] = { > + { 1920, 1080, 24, 0, > + DRM_MODE_FLAG_3D_TOP_AND_BOTTOM | DRM_MODE_FLAG_3D_FRAME_PACKING }, > + { 1920, 1080, 50, DRM_MODE_FLAG_INTERLACE, > + DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF }, > + { 1920, 1080, 60, DRM_MODE_FLAG_INTERLACE, > + DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF }, > + { 1280, 720, 50, 0, > + DRM_MODE_FLAG_3D_TOP_AND_BOTTOM | DRM_MODE_FLAG_3D_FRAME_PACKING }, > + { 1280, 720, 60, 0, > + DRM_MODE_FLAG_3D_TOP_AND_BOTTOM | DRM_MODE_FLAG_3D_FRAME_PACKING } > +}; > + > +static bool > +stereo_match_mandatory(const struct drm_display_mode *mode, > + const struct stereo_mandatory_mode *stereo_mode) > +{ > + unsigned int interlaced = mode->flags & DRM_MODE_FLAG_INTERLACE; > + > + return mode->hdisplay == stereo_mode->width && > + mode->vdisplay == stereo_mode->height && > + interlaced == stereo_mode->interlace_flag && > + drm_mode_vrefresh(mode) == stereo_mode->freq; > +} > + > +static const struct stereo_mandatory_mode * > +hdmi_find_stereo_mandatory_mode(struct drm_display_mode *mode) ^ Can be const. > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(stereo_mandatory_modes); i++) > + if (stereo_match_mandatory(mode, &stereo_mandatory_modes[i])) > + return &stereo_mandatory_modes[i]; > + > + return NULL; > +} > + > +static int add_hdmi_mandatory_stereo_modes(struct drm_connector *connector) > +{ > + struct drm_device *dev = connector->dev; > + struct drm_display_mode *mode, *new_mode; 'mode' can be const, 'new_mode' could be moved into tighter scope. > + struct list_head stereo_modes; > + int modes = 0; > + > + INIT_LIST_HEAD(&stereo_modes); > + > + list_for_each_entry(mode, &connector->probed_modes, head) { > + const struct stereo_mandatory_mode *mandatory; > + u32 stereo_layouts, layout; > + > + mandatory = hdmi_find_stereo_mandatory_mode(mode); > + if (!mandatory) > + continue; > + > + stereo_layouts = mandatory->layouts; > + do { ^^^^^^^^^^^^^^^^ > + layout = 1 << (ffs(stereo_layouts) - 1); ^^^^^^^^^^^^^^^^^^^^^^^^ -ENOTABS > + stereo_layouts &= ~layout; > + > + new_mode = drm_mode_duplicate(dev, mode); > + if (!new_mode) > + continue; > + > + new_mode->flags |= layout; > + list_add_tail(&new_mode->head, &stereo_modes); > + modes++; > + } while (stereo_layouts); > + } > + > + list_splice_tail(&stereo_modes, &connector->probed_modes); > + > + return modes; > +} > + > /* > * do_hdmi_vsdb_modes - Parse the HDMI Vendor Specific data block > * @connector: connector corresponding to the HDMI sink > * @db: start of the CEA vendor specific block > * @len: length of the CEA block payload, ie. one can access up to db[len] > * > - * Parses the HDMI VSDB looking for modes to add to @connector. > + * Parses the HDMI VSDB looking for modes to add to @connector. This function > + * also adds the stereo 3d modes when applicable. > */ > static int > do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len) > @@ -2582,10 +2662,15 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len) > > /* the declared length is not long enough for the 2 first bytes > * of additional video format capabilities */ > - offset += 2; > - if (len < (8 + offset)) > + if (len < (8 + offset + 2)) > goto out; > > + /* 3D_Present */ > + offset++; > + if (db[8 + offset] & (1 << 7)) > + modes += add_hdmi_mandatory_stereo_modes(connector); Hmm. I was thinking this is a bit soon since we haven't added the 4k modes, nor the alternate clock modes yet. But I guess the 4k modes aren't relevant here, and the alternate modes vs. 3D modes steps can be done in either order. Or did I miss something here? > + > + offset++; > vic_len = db[8 + offset] >> 5; > > for (i = 0; i < vic_len && len >= (9 + offset + i); i++) { > @@ -2665,8 +2750,8 @@ static int > add_cea_modes(struct drm_connector *connector, struct edid *edid) > { > const u8 *cea = drm_find_cea_extension(edid); > - const u8 *db; > - u8 dbl; > + const u8 *db, *hdmi = NULL; > + u8 dbl, hdmi_len; > int modes = 0; > > if (cea && cea_revision(cea) >= 3) { > @@ -2681,11 +2766,20 @@ add_cea_modes(struct drm_connector *connector, struct edid *edid) > > if (cea_db_tag(db) == VIDEO_BLOCK) > modes += do_cea_modes(connector, db + 1, dbl); > - else if (cea_db_is_hdmi_vsdb(db)) > - modes += do_hdmi_vsdb_modes(connector, db, dbl); > + else if (cea_db_is_hdmi_vsdb(db)) { > + hdmi = db; > + hdmi_len = dbl; > + } > } > } > > + /* > + * We parse the HDMI VSDB after having added the cea modes as we will > + * be patching their flags when the sink supports stereo 3D. > + */ > + if (hdmi) > + modes += do_hdmi_vsdb_modes(connector, hdmi, hdmi_len); > + > return modes; > } > > -- > 1.8.3.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel