From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Neil Armstrong <narmstrong@baylibre.com>
Cc: dri-devel@lists.freedesktop.org,
laurent.pinchart+renesas@ideasonboard.com,
Jose.Abreu@synopsys.com, kieran.bingham@ideasonboard.com,
linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] drm: bridge: dw-hdmi: Take input format from plat_data
Date: Thu, 02 Mar 2017 17:45:09 +0200 [thread overview]
Message-ID: <2768953.c6tz7qsbuQ@avalon> (raw)
In-Reply-To: <1488468572-31971-2-git-send-email-narmstrong@baylibre.com>
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 <narmstrong@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
next prev parent reply other threads:[~2017-03-02 22:19 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-02 15:29 [PATCH v2 0/2] drm: bridge: dw-hdmi: Add support for Custom PHYs Neil Armstrong
2017-03-02 15:29 ` [PATCH v2 1/2] drm: bridge: dw-hdmi: Take input format from plat_data Neil Armstrong
2017-03-02 15:45 ` Laurent Pinchart [this message]
2017-03-03 11:31 ` Neil Armstrong
2017-03-03 16:39 ` Jose Abreu
2017-03-03 16:42 ` Neil Armstrong
2017-03-03 17:22 ` Jose Abreu
2017-03-06 10:41 ` Neil Armstrong
2017-03-06 12:17 ` Jose Abreu
2017-03-06 12:39 ` Neil Armstrong
2017-03-02 15:29 ` [PATCH v2 2/2] drm: bridge: Move HPD handling to PHY operations Neil Armstrong
2017-03-02 16:18 ` Laurent Pinchart
2017-03-03 9:07 ` Neil Armstrong
2017-03-03 10:05 ` Jose Abreu
2017-03-03 12:16 ` Laurent Pinchart
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2768953.c6tz7qsbuQ@avalon \
--to=laurent.pinchart@ideasonboard.com \
--cc=Jose.Abreu@synopsys.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=kieran.bingham@ideasonboard.com \
--cc=laurent.pinchart+renesas@ideasonboard.com \
--cc=linux-amlogic@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=narmstrong@baylibre.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).