From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 08/12] drm: Set the relevant infoframe field when scanning out a 3D mode Date: Mon, 16 Sep 2013 21:59:19 +0300 Message-ID: <20130916185919.GN4531@intel.com> References: <1379353735-4472-1-git-send-email-damien.lespiau@intel.com> <1379353735-4472-9-git-send-email-damien.lespiau@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <1379353735-4472-9-git-send-email-damien.lespiau@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Damien Lespiau Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Sep 16, 2013 at 06:48:51PM +0100, Damien Lespiau wrote: > When scanning out a 3D mode on HDMI, we need to send an HDMI infoframe > with the corresponding layout to the sink. > = > Signed-off-by: Damien Lespiau > --- > drivers/gpu/drm/drm_edid.c | 25 +++++++++++++++++++++++-- > 1 file changed, 23 insertions(+), 2 deletions(-) > = > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index e016a5d..9a36b6e 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -3364,6 +3364,18 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hd= mi_avi_infoframe *frame, > } > EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode); > = > +static enum hdmi_3d_structure > +s3d_structure_from_display_mode(const struct drm_display_mode *mode) > +{ > + u32 s3d_mode =3D (mode->flags & DRM_MODE_FLAG_3D_MASK) >> 14; > + int set =3D ffs(s3d_mode) - 1; > + > + if (set =3D=3D 7) > + return HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF; This function feels a bit too subtle. I would perhaps go w/ just a switch statement. Or maybe leave a hole for 7 in the DRM_MODE_FLAG_3D flags. And if we run out of bits before 3d_structure=3D7 gets defined we just repurpose the bit for something else. But maybe that's equally subtle. Hmm. The spec is quite poor too. In one place it says the quincunx modes are valid for 3d_structure=3D8 (and 15 is reserved), but in another place it says 3d_structure=3D15 is when the quincunx stuff applies. But I guss we can just keep ignoring the 3d_structure > 8 case for now. > + > + return set; > +} > + > /** > * drm_hdmi_vendor_infoframe_from_display_mode() - fill an HDMI infofram= e with > * data from a DRM display mode > @@ -3381,20 +3393,29 @@ drm_hdmi_vendor_infoframe_from_display_mode(struc= t hdmi_vendor_infoframe *frame, > const struct drm_display_mode *mode) > { > int err; > + u32 s3d_flags; > u8 vic; > = > if (!frame || !mode) > return -EINVAL; > = > vic =3D drm_match_hdmi_mode(mode); > - if (!vic) > + s3d_flags =3D mode->flags & DRM_MODE_FLAG_3D_MASK; > + > + if (!vic && !s3d_flags) > + return -EINVAL; > + > + if (vic && s3d_flags) > return -EINVAL; Could be just '!vic =3D=3D !s3d_flags' or w/ !! on both sides if we want to stick to positive thinking. But maybe it's me who's getting into the subtle territory here :) Other than the bikesheds it looks OK to me: Reviewed-by: Ville Syrj=E4l=E4 > = > err =3D hdmi_vendor_infoframe_init(frame); > if (err < 0) > return err; > = > - frame->vic =3D vic; > + if (vic) > + frame->vic =3D vic; > + else > + frame->s3d_struct =3D s3d_structure_from_display_mode(mode); > = > return 0; > } > -- = > 1.8.3.1 > = > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- = Ville Syrj=E4l=E4 Intel OTC