Hi Neil,
Thank you for the patch.
On Thursday 02 Mar 2017 16:29:31 Neil Armstrong wrote:
> Some display pipelines can only provide non-RBG input pixels to the HDMI TX
> Controller, this patch takes the pixel format from the plat_data if
> provided.
>
> Signed-off-by: Neil Armstrong <narmstr...@baylibre.com>
> ---
> drivers/gpu/drm/bridge/dw-hdmi.c | 59 ++++++++++++++++++-------------------
> include/drm/bridge/dw_hdmi.h | 9 ++++++
> 2 files changed, 39 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c
> b/drivers/gpu/drm/bridge/dw-hdmi.c index 026a0dc..653ecd7 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -35,12 +35,6 @@
>
> #define HDMI_EDID_LEN 512
>
> -#define RGB 0
> -#define YCBCR444 1
> -#define YCBCR422_16BITS 2
> -#define YCBCR422_8BITS 3
> -#define XVYCC444 4
> -
> enum hdmi_datamap {
> RGB444_8B = 0x01,
> RGB444_10B = 0x03,
> @@ -94,8 +88,8 @@ struct hdmi_vmode {
> };
>
> struct hdmi_data_info {
> - unsigned int enc_in_format;
> - unsigned int enc_out_format;
> + enum dw_hdmi_color_enc_format enc_in_format;
> + enum dw_hdmi_color_enc_format enc_out_format;
> unsigned int enc_color_depth;
> unsigned int colorimetry;
> unsigned int pix_repet_factor;
> @@ -569,7 +563,7 @@ static void hdmi_video_sample(struct dw_hdmi *hdmi)
> int color_format = 0;
> u8 val;
>
> - if (hdmi->hdmi_data.enc_in_format == RGB) {
> + if (hdmi->hdmi_data.enc_in_format == DW_HDMI_ENC_FMT_RGB) {
> if (hdmi->hdmi_data.enc_color_depth == 8)
> color_format = 0x01;
> else if (hdmi->hdmi_data.enc_color_depth == 10)
> @@ -580,7 +574,7 @@ static void hdmi_video_sample(struct dw_hdmi *hdmi)
> color_format = 0x07;
> else
> return;
> - } else if (hdmi->hdmi_data.enc_in_format == YCBCR444) {
> + } else if (hdmi->hdmi_data.enc_in_format == DW_HDMI_ENC_FMT_YCBCR444)
{
> if (hdmi->hdmi_data.enc_color_depth == 8)
> color_format = 0x09;
> else if (hdmi->hdmi_data.enc_color_depth == 10)
> @@ -591,7 +585,8 @@ static void hdmi_video_sample(struct dw_hdmi *hdmi)
> color_format = 0x0F;
> else
> return;
> - } else if (hdmi->hdmi_data.enc_in_format == YCBCR422_8BITS) {
> + } else if (hdmi->hdmi_data.enc_in_format ==
> + DW_HDMI_ENC_FMT_YCBCR422_8BITS) {
> if (hdmi->hdmi_data.enc_color_depth == 8)
> color_format = 0x16;
> else if (hdmi->hdmi_data.enc_color_depth == 10)
> @@ -627,20 +622,20 @@ static int is_color_space_conversion(struct dw_hdmi
> *hdmi)
>
> static int is_color_space_decimation(struct dw_hdmi *hdmi)
> {
> - if (hdmi->hdmi_data.enc_out_format != YCBCR422_8BITS)
> + if (hdmi->hdmi_data.enc_out_format != DW_HDMI_ENC_FMT_YCBCR422_8BITS)
> return 0;
> - if (hdmi->hdmi_data.enc_in_format == RGB ||
> - hdmi->hdmi_data.enc_in_format == YCBCR444)
> + if (hdmi->hdmi_data.enc_in_format == DW_HDMI_ENC_FMT_RGB ||
> + hdmi->hdmi_data.enc_in_format == DW_HDMI_ENC_FMT_YCBCR444)
> return 1;
> return 0;
> }
>
> static int is_color_space_interpolation(struct dw_hdmi *hdmi)
> {
> - if (hdmi->hdmi_data.enc_in_format != YCBCR422_8BITS)
> + if (hdmi->hdmi_data.enc_in_format != DW_HDMI_ENC_FMT_YCBCR422_8BITS)
> return 0;
> - if (hdmi->hdmi_data.enc_out_format == RGB ||
> - hdmi->hdmi_data.enc_out_format == YCBCR444)
> + if (hdmi->hdmi_data.enc_out_format == DW_HDMI_ENC_FMT_RGB ||
> + hdmi->hdmi_data.enc_out_format == DW_HDMI_ENC_FMT_YCBCR444)
> return 1;
> return 0;
> }
> @@ -652,13 +647,14 @@ static void dw_hdmi_update_csc_coeffs(struct dw_hdmi
> *hdmi) u32 csc_scale = 1;
>
> if (is_color_space_conversion(hdmi)) {
> - if (hdmi->hdmi_data.enc_out_format == RGB) {
> + if (hdmi->hdmi_data.enc_out_format == DW_HDMI_ENC_FMT_RGB) {
> if (hdmi->hdmi_data.colorimetry ==
> HDMI_COLORIMETRY_ITU_601)
> csc_coeff = &csc_coeff_rgb_out_eitu601;
> else
> csc_coeff = &csc_coeff_rgb_out_eitu709;
> - } else if (hdmi->hdmi_data.enc_in_format == RGB) {
> + } else if (hdmi->hdmi_data.enc_in_format ==
> + DW_HDMI_ENC_FMT_RGB) {
> if (hdmi->hdmi_data.colorimetry ==
> HDMI_COLORIMETRY_ITU_601)
> csc_coeff = &csc_coeff_rgb_in_eitu601;
> @@ -730,8 +726,8 @@ static void hdmi_video_packetize(struct dw_hdmi *hdmi)
> struct hdmi_data_info *hdmi_data = &hdmi->hdmi_data;
> u8 val, vp_conf;
>
> - if (hdmi_data->enc_out_format == RGB ||
> - hdmi_data->enc_out_format == YCBCR444) {
> + if (hdmi_data->enc_out_format == DW_HDMI_ENC_FMT_RGB ||
> + hdmi_data->enc_out_format == DW_HDMI_ENC_FMT_YCBCR444) {
> if (!hdmi_data->enc_color_depth) {
> output_select = HDMI_VP_CONF_OUTPUT_SELECTOR_BYPASS;
> } else if (hdmi_data->enc_color_depth == 8) {
> @@ -746,7 +742,8 @@ static void hdmi_video_packetize(struct dw_hdmi *hdmi)
> } else {
> return;
> }
> - } else if (hdmi_data->enc_out_format == YCBCR422_8BITS) {
> + } else if (hdmi_data->enc_out_format ==
> + DW_HDMI_ENC_FMT_YCBCR422_8BITS) {
> if (!hdmi_data->enc_color_depth ||
> hdmi_data->enc_color_depth == 8)
> remap_size = HDMI_VP_REMAP_YCC422_16bit;
> @@ -1138,15 +1135,16 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi,
> struct drm_display_mode *mode) /* Initialise info frame from DRM mode */
> drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
>
> - if (hdmi->hdmi_data.enc_out_format == YCBCR444)
> + if (hdmi->hdmi_data.enc_out_format == DW_HDMI_ENC_FMT_YCBCR444)
> frame.colorspace = HDMI_COLORSPACE_YUV444;
> - else if (hdmi->hdmi_data.enc_out_format == YCBCR422_8BITS)
> + else if (hdmi->hdmi_data.enc_out_format ==
> + DW_HDMI_ENC_FMT_YCBCR422_8BITS)
> frame.colorspace = HDMI_COLORSPACE_YUV422;
> else
> frame.colorspace = HDMI_COLORSPACE_RGB;
>
> /* Set up colorimetry */
> - if (hdmi->hdmi_data.enc_out_format == XVYCC444) {
> + if (hdmi->hdmi_data.enc_out_format == DW_HDMI_ENC_FMT_XVYCC444) {
> frame.colorimetry = HDMI_COLORIMETRY_EXTENDED;
> if (hdmi->hdmi_data.colorimetry == HDMI_COLORIMETRY_ITU_601)
> frame.extended_colorimetry =
> @@ -1154,7 +1152,7 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi,
> struct drm_display_mode *mode) else /*hdmi->hdmi_data.colorimetry ==
> HDMI_COLORIMETRY_ITU_709*/ frame.extended_colorimetry =
> HDMI_EXTENDED_COLORIMETRY_XV_YCC_709;
> - } else if (hdmi->hdmi_data.enc_out_format != RGB) {
> + } else if (hdmi->hdmi_data.enc_out_format != DW_HDMI_ENC_FMT_RGB) {
> frame.colorimetry = hdmi->hdmi_data.colorimetry;
> frame.extended_colorimetry =
HDMI_EXTENDED_COLORIMETRY_XV_YCC_601;
> } else { /* Carries no data */
> @@ -1443,10 +1441,13 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi,
> struct drm_display_mode *mode)
> hdmi->hdmi_data.video_mode.mpixelrepetitionoutput = 0;
> hdmi->hdmi_data.video_mode.mpixelrepetitioninput = 0;
>
> - /* TODO: Get input format from IPU (via FB driver interface) */
> - hdmi->hdmi_data.enc_in_format = RGB;
> + /* Get input format from plat data or fallback to RGB */
> + if (hdmi->plat_data->input_fmt >= 0)
> + hdmi->hdmi_data.enc_in_format = hdmi->plat_data->input_fmt;
> + else
> + hdmi->hdmi_data.enc_in_format = DW_HDMI_ENC_FMT_RGB;
>
> - hdmi->hdmi_data.enc_out_format = RGB;
> + hdmi->hdmi_data.enc_out_format = DW_HDMI_ENC_FMT_RGB;
>
> hdmi->hdmi_data.enc_color_depth = 8;
> hdmi->hdmi_data.pix_repet_factor = 0;
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index bcceee8..8c0cc13 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -21,6 +21,14 @@ enum {
> DW_HDMI_RES_MAX,
> };
>
> +enum dw_hdmi_color_enc_format {
> + DW_HDMI_ENC_FMT_RGB = 0,
> + DW_HDMI_ENC_FMT_YCBCR444,
> + DW_HDMI_ENC_FMT_YCBCR422_16BITS,
> + DW_HDMI_ENC_FMT_YCBCR422_8BITS,
> + DW_HDMI_ENC_FMT_XVYCC444,
> +};
> +
> enum dw_hdmi_phy_type {
> DW_HDMI_PHY_DWC_HDMI_TX_PHY = 0x00,
> DW_HDMI_PHY_DWC_MHL_PHY_HEAC = 0xb2,
> @@ -62,6 +70,7 @@ struct dw_hdmi_plat_data {
> struct regmap *regm;
> enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
> struct drm_display_mode *mode);
> + enum dw_hdmi_color_enc_format input_fmt;
As reported before, I think we should use a combination of media bus format
and color space information instead of a custom enum. Ideally this should also
be queried at runtime, but that could be done in a second step.
>
> /* Vendor PHY support */
> const struct dw_hdmi_phy_ops *phy_ops;
--
Regards,
Laurent Pinchart
