| Message ID | 1379353735-4472-6-git-send-email-damien.lespiau@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@intel.com> > --- > 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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, Sep 16, 2013 at 09:29:31PM +0300, Ville Syrjälä wrote: > > 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? You're not missing anything. It's indeed fine to add the mandatory modes before the 4k ones because the mandatory modes are either 1080p, 1080i or 720p. As for alternate clocks, I opted to re-use the current code with one tiny adjustement, so the alternate clocks logic stays in one place.
On Mon, Sep 16, 2013 at 09:29:31PM +0300, Ville Syrjälä wrote: > > +struct stereo_mandatory_mode { > > + int width, height, freq; [..] > > + unsigned int interlace_flag, layouts; > > What's the benefit of separating the two? Just that we can easily cycle through the layout flags and add a mode per flag without having to clear the interlace bit. Trade 16 bytes against one instruction or so, can do I guess.
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; + unsigned int interlace_flag, layouts; +}; + +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) +{ + 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; + 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); + 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); + + 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; }
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@intel.com> --- drivers/gpu/drm/drm_edid.c | 108 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 101 insertions(+), 7 deletions(-)