From: Neil Armstrong <narmstrong@baylibre.com>
To: "H. Nikolaus Schaller" <hns@goldelico.com>,
Andrzej Hajda <andrzej.hajda@intel.com>,
Robert Foss <robert.foss@linaro.org>,
Paul Boddie <paul@boddie.org.uk>,
Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
Jernej Skrabec <jernej.skrabec@gmail.com>,
David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
Paul Cercueil <paul@crapouillou.net>,
Maxime Ripard <maxime@cerno.tech>,
Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Cc: letux-kernel@openphoenux.org, linux-mips@vger.kernel.org,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
Jonas Karlman <jonas@kwiboo.se>
Subject: Re: [PATCH v17 4/6] drm/bridge: dw-hdmi: handle unusable or non-configured CSC module
Date: Thu, 7 Apr 2022 10:28:05 +0200 [thread overview]
Message-ID: <0a8e4e32-cc86-e901-364b-d1e6e2bd37b5@baylibre.com> (raw)
In-Reply-To: <8de76ca2b478016f4dbed84e37db231e7810e56c.1649262368.git.hns@goldelico.com>
Hi,
On 06/04/2022 18:26, H. Nikolaus Schaller wrote:
> From: Neil Armstrong <narmstrong@baylibre.com>
>
> The dw-hdmi integrates an optional Color Space Conversion feature used
> to handle color-space conversions.
>
> On some platforms, the CSC isn't built-in or non-functional.
>
> This adds the necessary code to disable the CSC functionality
> and limit the bus format negotiation to force using the same
> input bus format as the output bus format.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 100 +++++++++++++++-------
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 1 +
> include/drm/bridge/dw_hdmi.h | 1 +
> 3 files changed, 71 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index f50af40e10340..b5a665c5e406e 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -158,6 +158,8 @@ struct dw_hdmi {
> struct hdmi_data_info hdmi_data;
> const struct dw_hdmi_plat_data *plat_data;
>
> + bool csc_available; /* indicates if the CSC engine is usable */
> +
> int vic;
>
> u8 edid[HDMI_EDID_LEN];
> @@ -1009,9 +1011,10 @@ static int is_color_space_interpolation(struct dw_hdmi *hdmi)
>
> static bool is_csc_needed(struct dw_hdmi *hdmi)
> {
> - return is_color_space_conversion(hdmi) ||
> - is_color_space_decimation(hdmi) ||
> - is_color_space_interpolation(hdmi);
> + return hdmi->csc_available &&
> + (is_color_space_conversion(hdmi) ||
> + is_color_space_decimation(hdmi) ||
> + is_color_space_interpolation(hdmi));
> }
>
> static void dw_hdmi_update_csc_coeffs(struct dw_hdmi *hdmi)
> @@ -1064,6 +1067,9 @@ static void hdmi_video_csc(struct dw_hdmi *hdmi)
> int interpolation = HDMI_CSC_CFG_INTMODE_DISABLE;
> int decimation = 0;
>
> + if (!hdmi->csc_available)
> + return;
> +
> /* YCC422 interpolation to 444 mode */
> if (is_color_space_interpolation(hdmi))
> interpolation = HDMI_CSC_CFG_INTMODE_CHROMA_INT_FORMULA1;
> @@ -2665,6 +2671,7 @@ static u32 *dw_hdmi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> u32 output_fmt,
> unsigned int *num_input_fmts)
> {
> + struct dw_hdmi *hdmi = bridge->driver_private;
> u32 *input_fmts;
> unsigned int i = 0;
>
> @@ -2683,62 +2690,81 @@ static u32 *dw_hdmi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> /* 8bit */
> case MEDIA_BUS_FMT_RGB888_1X24:
> input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
> - input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
> - input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
> + if (hdmi->csc_available) {
> + input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
> + input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
> + }
> break;
> case MEDIA_BUS_FMT_YUV8_1X24:
> input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
> - input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
> - input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
> + if (hdmi->csc_available) {
> + input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
> + input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
> + }
> break;
> case MEDIA_BUS_FMT_UYVY8_1X16:
> input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
> - input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
> - input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
> + if (hdmi->csc_available) {
> + input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
> + input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
> + }
> break;
>
> /* 10bit */
> case MEDIA_BUS_FMT_RGB101010_1X30:
> input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
> - input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
> - input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
> + if (hdmi->csc_available) {
> + input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
> + input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
> + }
> break;
> case MEDIA_BUS_FMT_YUV10_1X30:
> input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
> - input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
> - input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
> + if (hdmi->csc_available) {
> + input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
> + input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
> + }
> break;
> case MEDIA_BUS_FMT_UYVY10_1X20:
> input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
> - input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
> - input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
> + if (hdmi->csc_available) {
> + input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
> + input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
> + }
> break;
>
> /* 12bit */
> case MEDIA_BUS_FMT_RGB121212_1X36:
> input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
> - input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
> - input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
> + if (hdmi->csc_available) {
> + input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
> + input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
> + }
> break;
> case MEDIA_BUS_FMT_YUV12_1X36:
> input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
> - input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
> - input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
> + if (hdmi->csc_available) {
> + input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
> + input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
> + }
> break;
> case MEDIA_BUS_FMT_UYVY12_1X24:
> input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
> - input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
> - input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
> + if (hdmi->csc_available) {
> + input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
> + input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
> + }
> break;
>
> /* 16bit */
> case MEDIA_BUS_FMT_RGB161616_1X48:
> input_fmts[i++] = MEDIA_BUS_FMT_RGB161616_1X48;
> - input_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
> + if (hdmi->csc_available)
> + input_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
> break;
> case MEDIA_BUS_FMT_YUV16_1X48:
> - input_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
> - input_fmts[i++] = MEDIA_BUS_FMT_RGB161616_1X48;
> + if (hdmi->csc_available)
> + input_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
> break;
>
> /*YUV 4:2:0 */
> @@ -2767,15 +2793,24 @@ static int dw_hdmi_bridge_atomic_check(struct drm_bridge *bridge,
> {
> struct dw_hdmi *hdmi = bridge->driver_private;
>
> - hdmi->hdmi_data.enc_out_bus_format =
> - bridge_state->output_bus_cfg.format;
> + if (!hdmi->csc_available &&
> + bridge_state->output_bus_cfg.format != bridge_state->input_bus_cfg.format) {
> + dev_warn(hdmi->dev, "different input format 0x%04x & output format 0x%04x while CSC isn't usable, fallback to safe format\n",
> + bridge_state->input_bus_cfg.format,
> + bridge_state->output_bus_cfg.format);
> + hdmi->hdmi_data.enc_out_bus_format = MEDIA_BUS_FMT_FIXED;
> + hdmi->hdmi_data.enc_in_bus_format = MEDIA_BUS_FMT_FIXED;
> + } else {
> + hdmi->hdmi_data.enc_out_bus_format =
> + bridge_state->output_bus_cfg.format;
>
> - hdmi->hdmi_data.enc_in_bus_format =
> - bridge_state->input_bus_cfg.format;
> + hdmi->hdmi_data.enc_in_bus_format =
> + bridge_state->input_bus_cfg.format;
>
> - dev_dbg(hdmi->dev, "input format 0x%04x, output format 0x%04x\n",
> - bridge_state->input_bus_cfg.format,
> - bridge_state->output_bus_cfg.format);
> + dev_dbg(hdmi->dev, "input format 0x%04x, output format 0x%04x\n",
> + bridge_state->input_bus_cfg.format,
> + bridge_state->output_bus_cfg.format);
> + }
>
> return 0;
> }
> @@ -3481,6 +3516,9 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
> hdmi->cec = platform_device_register_full(&pdevinfo);
> }
>
> + /* Get CSC useability from config0 register and permit override for platforms */
> + hdmi->csc_available = !plat_data->disable_csc || (config0 & HDMI_CONFIG0_CSC);
> +
> drm_bridge_add(&hdmi->bridge);
>
> return hdmi;
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> index 1999db05bc3b2..279722e4d1898 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> @@ -541,6 +541,7 @@ enum {
>
> /* CONFIG0_ID field values */
> HDMI_CONFIG0_I2S = 0x10,
> + HDMI_CONFIG0_CSC = 0x04,
> HDMI_CONFIG0_CEC = 0x02,
>
> /* CONFIG1_ID field values */
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index 2a1f85f9a8a3f..b2f689cbe864c 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -157,6 +157,7 @@ struct dw_hdmi_plat_data {
> unsigned long mpixelclock);
>
> unsigned int disable_cec : 1;
> + unsigned int disable_csc : 1;
> };
>
> struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
Is this really still needed now you filter correctly the possible
modes in patch 1 ?
Neil
next prev parent reply other threads:[~2022-04-07 8:28 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-06 16:26 [PATCH v17 0/6] MIPS: JZ4780 and CI20 HDMI H. Nikolaus Schaller
2022-04-06 16:26 ` [PATCH v17 1/6] drm/ingenic: Implement proper .atomic_get_input_bus_fmts H. Nikolaus Schaller
2022-04-06 16:26 ` [PATCH v17 2/6] drm/ingenic: Add dw-hdmi driver specialization for jz4780 H. Nikolaus Schaller
2022-04-06 16:26 ` [PATCH v17 3/6] drm/bridge: dw-hdmi: filter out YUV output formats when DVI H. Nikolaus Schaller
2022-04-06 16:26 ` [PATCH v17 4/6] drm/bridge: dw-hdmi: handle unusable or non-configured CSC module H. Nikolaus Schaller
2022-04-07 8:28 ` Neil Armstrong [this message]
2022-04-07 11:15 ` H. Nikolaus Schaller
2022-04-06 16:26 ` [PATCH v17 5/6] drm/bridge: dw-hdmi: add YCBCR formats only if CSC is available H. Nikolaus Schaller
2022-04-07 8:29 ` Neil Armstrong
2022-04-07 11:15 ` H. Nikolaus Schaller
2022-04-06 16:26 ` [PATCH v17 6/6] drm/bridge: display-connector: add ddc-en gpio support H. Nikolaus Schaller
2022-04-07 7:30 ` Laurent Pinchart
2022-04-07 11:15 ` H. Nikolaus Schaller
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=0a8e4e32-cc86-e901-364b-d1e6e2bd37b5@baylibre.com \
--to=narmstrong@baylibre.com \
--cc=Laurent.pinchart@ideasonboard.com \
--cc=airlied@linux.ie \
--cc=andrzej.hajda@intel.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=hns@goldelico.com \
--cc=jernej.skrabec@gmail.com \
--cc=jonas@kwiboo.se \
--cc=kieran.bingham+renesas@ideasonboard.com \
--cc=letux-kernel@openphoenux.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@vger.kernel.org \
--cc=maxime@cerno.tech \
--cc=paul@boddie.org.uk \
--cc=paul@crapouillou.net \
--cc=robert.foss@linaro.org \
/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).