>-----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. 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
