[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200812092217.GC6057@pendragon.ideasonboard.com>
Date: Wed, 12 Aug 2020 12:22:17 +0300
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Algea Cao <algea.cao@...k-chips.com>
Cc: a.hajda@...sung.com, kuankuan.y@...il.com, hjc@...k-chips.com,
tzimmermann@...e.de, dri-devel@...ts.freedesktop.org,
sam@...nborg.org, airlied@...ux.ie, heiko@...ech.de,
jernej.skrabec@...l.net, laurent.pinchart+renesas@...asonboard.com,
jonas@...boo.se, mripard@...nel.org, darekm@...gle.com,
linux-rockchip@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, cychiang@...omium.org,
linux-kernel@...r.kernel.org, narmstrong@...libre.com,
jbrunet@...libre.com, maarten.lankhorst@...ux.intel.com,
daniel@...ll.ch
Subject: Re: [PATCH 2/6] drm: bridge: dw-hdmi: Implement connector
atomic_begin/atomic_flush
Hi Algea,
Thank you for the patch.
On Wed, Aug 12, 2020 at 04:34:33PM +0800, Algea Cao wrote:
> Introduce dw_hdmi_connector_atomic_begin() and
> dw_hdmi_connector_atomic_flush() to implement connector
> atomic_begin/atomic_flush. When enc_out_bus_format or
> enc_in_bus_format changed, dw_hdmi_setup is called.
>
> To avoid screen flash when updating bus format, it's need
> to send AVMUTE flag to make screen black, and clear flag
> after bus format updated.
>
> Signed-off-by: Algea Cao <algea.cao@...k-chips.com>
> ---
>
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 65 +++++++++++++++++++++++
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 4 ++
> 2 files changed, 69 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 6148a022569a..a1a81fc768c2 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -108,6 +108,8 @@ struct hdmi_vmode {
> };
>
> struct hdmi_data_info {
> + unsigned int prev_enc_in_bus_format;
> + unsigned int prev_enc_out_bus_format;
> unsigned int enc_in_bus_format;
> unsigned int enc_out_bus_format;
> unsigned int enc_in_encoding;
> @@ -116,6 +118,7 @@ struct hdmi_data_info {
> unsigned int hdcp_enable;
> struct hdmi_vmode video_mode;
> bool rgb_limited_range;
> + bool update;
> };
>
> struct dw_hdmi_i2c {
> @@ -2401,6 +2404,60 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector)
> return ret;
> }
>
> +static void
> +dw_hdmi_connector_atomic_begin(struct drm_connector *connector,
> + struct drm_connector_state *conn_state)
> +{
> + struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
> + connector);
> + unsigned int enc_in_bus_fmt = hdmi->hdmi_data.enc_in_bus_format;
> + unsigned int enc_out_bus_fmt = hdmi->hdmi_data.enc_out_bus_format;
> + unsigned int prev_enc_in_bus_fmt =
> + hdmi->hdmi_data.prev_enc_in_bus_format;
> + unsigned int prev_enc_out_bus_fmt =
> + hdmi->hdmi_data.prev_enc_out_bus_format;
> +
> + if (!conn_state->crtc)
> + return;
> +
> + if (!hdmi->hdmi_data.video_mode.mpixelclock)
> + return;
> +
> + if (enc_in_bus_fmt != prev_enc_in_bus_fmt ||
> + enc_out_bus_fmt != prev_enc_out_bus_fmt) {
> + hdmi->hdmi_data.update = true;
> + hdmi_writeb(hdmi, HDMI_FC_GCP_SET_AVMUTE, HDMI_FC_GCP);
> + /* Add delay to make av mute work on sink*/
> + msleep(50);
> + } else {
> + hdmi->hdmi_data.update = false;
> + }
> +}
> +
> +static void
> +dw_hdmi_connector_atomic_flush(struct drm_connector *connector,
> + struct drm_connector_state *conn_state)
> +{
> + struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
> + connector);
> +
> + if (!conn_state->crtc)
> + return;
> +
> + DRM_DEBUG("%s\n", __func__);
> +
> + if (hdmi->hdmi_data.update) {
> + dw_hdmi_setup(hdmi, hdmi->curr_conn, &hdmi->previous_mode);
> + /*
> + * Before clear AVMUTE, delay is needed to
> + * prevent display flash.
> + */
> + msleep(50);
> + hdmi_writeb(hdmi, HDMI_FC_GCP_CLEAR_AVMUTE, HDMI_FC_GCP);
> + hdmi->hdmi_data.update = false;
> + }
> +}
> +
> static bool hdr_metadata_equal(const struct drm_connector_state *old_state,
> const struct drm_connector_state *new_state)
> {
> @@ -2465,6 +2522,8 @@ static const struct drm_connector_funcs dw_hdmi_connector_funcs = {
> static const struct drm_connector_helper_funcs dw_hdmi_connector_helper_funcs = {
> .get_modes = dw_hdmi_connector_get_modes,
> .atomic_check = dw_hdmi_connector_atomic_check,
> + .atomic_begin = dw_hdmi_connector_atomic_begin,
> + .atomic_flush = dw_hdmi_connector_atomic_flush,
> };
>
> static int dw_hdmi_connector_create(struct dw_hdmi *hdmi)
> @@ -2778,6 +2837,12 @@ static int dw_hdmi_bridge_atomic_check(struct drm_bridge *bridge,
> {
> struct dw_hdmi *hdmi = bridge->driver_private;
>
> + hdmi->hdmi_data.prev_enc_out_bus_format =
> + hdmi->hdmi_data.enc_out_bus_format;
> +
> + hdmi->hdmi_data.prev_enc_in_bus_format =
> + hdmi->hdmi_data.enc_in_bus_format;
> +
> hdmi->hdmi_data.enc_out_bus_format =
> bridge_state->output_bus_cfg.format;
>
.atomic_check() isn't allowed to change the device state, neither the
hardware state, nor the software state stored in struct dw_hdmi. You
essentially need to treat the drm_bridge and dw_hdmi as const in the
.atomic_check() operation.
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> index 1999db05bc3b..05182418efbb 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> @@ -842,6 +842,10 @@ enum {
> HDMI_FC_AVICONF3_QUANT_RANGE_LIMITED = 0x00,
> HDMI_FC_AVICONF3_QUANT_RANGE_FULL = 0x04,
>
> +/* HDMI_FC_GCP */
> + HDMI_FC_GCP_SET_AVMUTE = 0x2,
> + HDMI_FC_GCP_CLEAR_AVMUTE = 0x1,
> +
> /* FC_DBGFORCE field values */
> HDMI_FC_DBGFORCE_FORCEAUDIO = 0x10,
> HDMI_FC_DBGFORCE_FORCEVIDEO = 0x1,
--
Regards,
Laurent Pinchart
Powered by blists - more mailing lists