Re: [RFC/RFT PATCH 4/4] drm/bridge: dw-hdmi: Take input format from plat_data

From: Laurent Pinchart
Date: Tue Jan 17 2017 - 09:49:58 EST


Hi Neil,

Thank you for the patch.

On Tuesday 17 Jan 2017 13:31:34 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 <narmstrong@xxxxxxxxxxxx>
> ---
> drivers/gpu/drm/bridge/dw-hdmi.c | 7 +++++--
> include/drm/bridge/dw_hdmi.h | 9 +++++++++
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c
> b/drivers/gpu/drm/bridge/dw-hdmi.c index 8a6a183..fa4147c 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -1420,8 +1420,11 @@ 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 = RGB;

This should ideally be queried dynamically. I believe we need to extend the
DRM bridge API for this purpose. I might be willing to accept a pdata-based
solution in the meantime though.

> hdmi->hdmi_data.enc_out_format = RGB;
>
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index d6a0ab3..4f426c3 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_INPUT_FMT_RGB = 0,
> + DW_HDMI_INPUT_FMT_YCBCR444,
> + DW_HDMI_INPUT_FMT_YCBCR422_16BITS,
> + DW_HDMI_INPUT_FMT_YCBCR422_8BITS,
> + DW_HDMI_INPUT_FMT_XVYCC444,
> +};

How about giving the enum a name and dropping the

#define RGB 0
#define YCBCR444 1
#define YCBCR422_16BITS 2
#define YCBCR422_8BITS 3
#define XVYCC444 4

macros from the driver ? Even better, how about using a media bus format code
from include/uapi/linux/media-bus-format.h ? We would need an additional
colorspace field to express the difference between the YCBCR and XVYCC
formats, as both of them are YUV 4:4:4 and map to the same bus format.

> enum dw_hdmi_phy_type {
> DW_HDMI_PHY_DWC_HDMI_TX_PHY = 0x00,
> DW_HDMI_PHY_DWC_MHL_PHY_HEAC = 0xb2,
> @@ -68,6 +76,7 @@ struct dw_hdmi_plat_data {
> const struct dw_hdmi_plat_data *data);
> bool (*hdmi_read_hpd)(struct dw_hdmi *hdmi,
> const struct dw_hdmi_plat_data *data);
> + int input_fmt;
> };
>
> int dw_hdmi_probe(struct platform_device *pdev,

--
Regards,

Laurent Pinchart