On Tue, Feb 19, 2019 at 03:09:00PM +0000, Shankar, Uma wrote: > > > >-----Original Message----- > >From: dri-devel [mailto:dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of Ville > >Syrjälä > >Sent: Tuesday, February 19, 2019 1:37 AM > >To: Shankar, Uma <uma.shankar@xxxxxxxxx> > >Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Syrjala, Ville <ville.syrjala@xxxxxxxxx>; Lankhorst, > >Maarten <maarten.lankhorst@xxxxxxxxx>; dri-devel@xxxxxxxxxxxxxxxxxxxxx > >Subject: Re: [v16 3/4] drm: Add colorspace info to AVI Infoframe > > > >On Wed, Feb 13, 2019 at 12:35:10PM +0530, Uma Shankar wrote: > >> This adds colorspace information to HDMI AVI infoframe. > >> A helper function is added to program the same. > >> > >> v2: Moved this to drm core instead of i915 driver. > >> > >> v3: Exported the helper function. > >> > >> v4: Added separate HDMI specific macro as per CTA spec. > >> This is separate from user exposed enum values. This is as per Ville's > >> suggestion. > >> > >> v5: Appended BT709 and SMPTE 170M with YCC information as per Ville's > >> review comment to be clear and not to be confused with RGB. > >> > >> v6: Added bit wise macro for various fields of colorimetry for easier > >> understanding and review as per Ville's comments. Moved the same out > >> of header file to avoid any namespace issues. > >> > >> Signed-off-by: Uma Shankar <uma.shankar@xxxxxxxxx> > >> --- > >> drivers/gpu/drm/drm_edid.c | 66 > >++++++++++++++++++++++++++++++++++++++++++++++ > >> include/drm/drm_edid.h | 6 +++++ > >> 2 files changed, 72 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > >> index 990b190..64816c0 100644 > >> --- a/drivers/gpu/drm/drm_edid.c > >> +++ b/drivers/gpu/drm/drm_edid.c > >> @@ -4924,6 +4924,72 @@ static bool is_hdmi2_sink(struct drm_connector > >> *connector) } > >> EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode); > >> > >> +/* HDMI Colorspace Spec Definitions */ > >> +#define FULL_COLORIMETRY_MASK 0x1FF > >> +#define NORMAL_COLORIMETRY_MASK 0x3 > >> +#define EXTENDED_COLORIMETRY_MASK 0x7 > >> +#define EXTENDED_ACE_COLORIMETRY_MASK 0xF > >> + > >> +#define C(x) ((x) << 0) > >> +#define EC(x) ((x) << 2) > >> +#define ACE(x) ((x) << 5) > >> + > >> +#define HDMI_COLORIMETRY_NO_DATA 0x0 > >> +#define HDMI_COLORIMETRY_SMPTE_170M_YCC (C(1) | EC(0) | > >ACE(0)) > >> +#define HDMI_COLORIMETRY_BT709_YCC (C(2) | EC(0) | ACE(0)) > >> +#define HDMI_COLORIMETRY_XVYCC_601 (C(3) | EC(0) | ACE(0)) > >> +#define HDMI_COLORIMETRY_XVYCC_709 (C(3) | EC(1) | ACE(0)) > >> +#define HDMI_COLORIMETRY_SYCC_601 (C(3) | EC(2) | ACE(0)) > >> +#define HDMI_COLORIMETRY_OPYCC_601 (C(3) | EC(3) | ACE(0)) > >> +#define HDMI_COLORIMETRY_OPRGB (C(3) | EC(4) | > >ACE(0)) > >> +#define HDMI_COLORIMETRY_BT2020_CYCC (C(3) | EC(5) | ACE(0)) > >> +#define HDMI_COLORIMETRY_BT2020_RGB (C(3) | EC(6) | ACE(0)) > >> +#define HDMI_COLORIMETRY_BT2020_YCC (C(3) | EC(6) | ACE(0)) > >> +#define HDMI_COLORIMETRY_DCI_P3_RGB_D65 (C(3) | EC(7) | > >ACE(0)) > >> +#define HDMI_COLORIMETRY_DCI_P3_RGB_THEATER (C(3) | EC(7) | > >ACE(1)) > >> + > >> +static const u32 hdmi_colorimetry_val[] = { > >> + [DRM_MODE_COLORIMETRY_NO_DATA] = > >HDMI_COLORIMETRY_NO_DATA, > >> + [DRM_MODE_COLORIMETRY_SMPTE_170M_YCC] = > >HDMI_COLORIMETRY_SMPTE_170M_YCC, > >> + [DRM_MODE_COLORIMETRY_BT709_YCC] = > >HDMI_COLORIMETRY_BT709_YCC, > >> + [DRM_MODE_COLORIMETRY_XVYCC_601] = > >HDMI_COLORIMETRY_XVYCC_601, > >> + [DRM_MODE_COLORIMETRY_XVYCC_709] = > >HDMI_COLORIMETRY_XVYCC_709, > >> + [DRM_MODE_COLORIMETRY_SYCC_601] = > >HDMI_COLORIMETRY_SYCC_601, > >> + [DRM_MODE_COLORIMETRY_OPYCC_601] = > >HDMI_COLORIMETRY_OPYCC_601, > >> + [DRM_MODE_COLORIMETRY_OPRGB] = HDMI_COLORIMETRY_OPRGB, > >> + [DRM_MODE_COLORIMETRY_BT2020_CYCC] = > >HDMI_COLORIMETRY_BT2020_CYCC, > >> + [DRM_MODE_COLORIMETRY_BT2020_RGB] = > >HDMI_COLORIMETRY_BT2020_RGB, > >> + [DRM_MODE_COLORIMETRY_BT2020_YCC] = > >HDMI_COLORIMETRY_BT2020_YCC, }; > > > >Probably best to > > > >#undef C > >#undef EC > >#undef ACE > > > >here to avoid accidents. > > Sure will add that. So with this fixed and DP stuff dropped, can I mark your RB on all the remaining 2 patches > (DP patch will get dropped). If you confirm will resend with your RB for merge. Yeah, I think it should be good to go in. > > Thanks & Regards, > Uma Shankar > > >With that > >Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > >> + > >> +/** > >> + * drm_hdmi_avi_infoframe_colorspace() - fill the HDMI AVI infoframe > >> + * colorspace information > >> + * @frame: HDMI AVI infoframe > >> + * @conn_state: connector state > >> + */ > >> +void > >> +drm_hdmi_avi_infoframe_colorspace(struct hdmi_avi_infoframe *frame, > >> + const struct drm_connector_state *conn_state) { > >> + u32 colorimetry_val; > >> + u32 colorimetry_index = conn_state->colorspace & > >> +FULL_COLORIMETRY_MASK; > >> + > >> + if (colorimetry_index >= ARRAY_SIZE(hdmi_colorimetry_val)) > >> + colorimetry_val = HDMI_COLORIMETRY_NO_DATA; > >> + else > >> + colorimetry_val = hdmi_colorimetry_val[colorimetry_index]; > >> + > >> + frame->colorimetry = colorimetry_val & NORMAL_COLORIMETRY_MASK; > >> + /* > >> + * ToDo: Extend it for ACE formats as well. Modify the infoframe > >> + * structure and extend it in drivers/video/hdmi > >> + */ > >> + frame->extended_colorimetry = (colorimetry_val >> 2) & > >> + EXTENDED_COLORIMETRY_MASK; > >> +} > >> +EXPORT_SYMBOL(drm_hdmi_avi_infoframe_colorspace); > >> + > >> /** > >> * drm_hdmi_avi_infoframe_quant_range() - fill the HDMI AVI infoframe > >> * quantization range information > >> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index > >> 8dc1a08..9d3b5b9 100644 > >> --- a/include/drm/drm_edid.h > >> +++ b/include/drm/drm_edid.h > >> @@ -331,6 +331,7 @@ struct cea_sad { > >> > >> struct drm_encoder; > >> struct drm_connector; > >> +struct drm_connector_state; > >> struct drm_display_mode; > >> > >> int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads); @@ > >> -358,6 +359,11 @@ int drm_av_sync_delay(struct drm_connector > >> *connector, drm_hdmi_vendor_infoframe_from_display_mode(struct > >hdmi_vendor_infoframe *frame, > >> struct drm_connector *connector, > >> const struct drm_display_mode *mode); > >> + > >> +void > >> +drm_hdmi_avi_infoframe_colorspace(struct hdmi_avi_infoframe *frame, > >> + const struct drm_connector_state *conn_state); > >> + > >> void > >> drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame, > >> struct drm_connector *connector, > >> -- > >> 1.9.1 > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > >-- > >Ville Syrjälä > >Intel > >_______________________________________________ > >dri-devel mailing list > >dri-devel@xxxxxxxxxxxxxxxxxxxxx > >https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
