Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v2 1/4] drm/bridge: dw-hdmi: Add Dynamic Range and
 Mastering InfoFrame support
To:     Jonas Karlman <jonas@kwiboo.se>,
        Neil Armstrong <narmstrong@baylibre.com>
Cc:     Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
        Jernej Skrabec <jernej.skrabec@siol.net>,
        Kevin Hilman <khilman@baylibre.com>,
        Sandy Huang <hjc@rock-chips.com>,
        Heiko Stubner <heiko@sntech.de>,
        Maxime Ripard <mripard@kernel.org>,
        Chen-Yu Tsai <wens@csie.org>,
        "zhengyang@rock-chips.com" <zhengyang@rock-chips.com>,
        "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
From:   Andrzej Hajda <a.hajda@samsung.com>
Message-ID: <ced39de1-fc69-4e9b-550f-f3fe7d73a434@samsung.com>
Date:   Thu, 10 Oct 2019 12:12:47 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
        Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <HE1PR06MB4011D7B916CBF8B740ACC45FAC9B0@HE1PR06MB4011.eurprd06.prod.outlook.com>
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <HE1PR06MB401113BF395C06E96503344FAC9B0@HE1PR06MB4011.eurprd06.prod.outlook.com>
        <CGME20191007192154epcas4p32fbfbce564847de5dac47a5e99972644@epcas4p3.samsung.com>
        <HE1PR06MB4011D7B916CBF8B740ACC45FAC9B0@HE1PR06MB4011.eurprd06.prod.outlook.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 07.10.2019 21:21, Jonas Karlman wrote:
> Add support for configuring Dynamic Range and Mastering InfoFrame from
> the hdr_output_metadata connector property.
>
> This patch adds a use_drm_infoframe flag to dw_hdmi_plat_data that platform
> drivers use to signal when Dynamic Range and Mastering infoframes is supported.
> This flag is needed because Amlogic GXBB and GXL report same DW-HDMI version,
> and only GXL support DRM InfoFrame.
>
> These changes were based on work done by Zheng Yang <zhengyang@rock-chips.com>
> to support DRM InfoFrame on the Rockchip 4.4 BSP kernel at [1] and [2]
>
> [1] https://github.com/rockchip-linux/kernel/tree/develop-4.4
> [2] https://github.com/rockchip-linux/kernel/commit/d1943fde81ff41d7cca87f4a42f03992e90bddd5
>
> Cc: Zheng Yang <zhengyang@rock-chips.com>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>


Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>


Neil, since you want to take care of patch3/4, will you take care of the
whole patchset?


 --
Regards
Andrzej


> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 81 +++++++++++++++++++++++
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 37 +++++++++++
>  include/drm/bridge/dw_hdmi.h              |  1 +
>  3 files changed, 119 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index a15fbf71b9d7..fdc29869d75a 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -25,6 +25,7 @@
>  #include <uapi/linux/videodev2.h>
>  
>  #include <drm/bridge/dw_hdmi.h>
> +#include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_bridge.h>
>  #include <drm/drm_edid.h>
> @@ -1743,6 +1744,41 @@ static void hdmi_config_vendor_specific_infoframe(struct dw_hdmi *hdmi,
>  			HDMI_FC_DATAUTO0_VSD_MASK);
>  }
>  
> +static void hdmi_config_drm_infoframe(struct dw_hdmi *hdmi)
> +{
> +	const struct drm_connector_state *conn_state = hdmi->connector.state;
> +	struct hdmi_drm_infoframe frame;
> +	u8 buffer[30];
> +	ssize_t err;
> +	int i;
> +
> +	if (!hdmi->plat_data->use_drm_infoframe)
> +		return;
> +
> +	hdmi_modb(hdmi, HDMI_FC_PACKET_TX_EN_DRM_DISABLE,
> +		  HDMI_FC_PACKET_TX_EN_DRM_MASK, HDMI_FC_PACKET_TX_EN);
> +
> +	err = drm_hdmi_infoframe_set_hdr_metadata(&frame, conn_state);
> +	if (err < 0)
> +		return;
> +
> +	err = hdmi_drm_infoframe_pack(&frame, buffer, sizeof(buffer));
> +	if (err < 0) {
> +		dev_err(hdmi->dev, "Failed to pack drm infoframe: %zd\n", err);
> +		return;
> +	}
> +
> +	hdmi_writeb(hdmi, frame.version, HDMI_FC_DRM_HB0);
> +	hdmi_writeb(hdmi, frame.length, HDMI_FC_DRM_HB1);
> +
> +	for (i = 0; i < frame.length; i++)
> +		hdmi_writeb(hdmi, buffer[4 + i], HDMI_FC_DRM_PB0 + i);
> +
> +	hdmi_writeb(hdmi, 1, HDMI_FC_DRM_UP);
> +	hdmi_modb(hdmi, HDMI_FC_PACKET_TX_EN_DRM_ENABLE,
> +		  HDMI_FC_PACKET_TX_EN_DRM_MASK, HDMI_FC_PACKET_TX_EN);
> +}
> +
>  static void hdmi_av_composer(struct dw_hdmi *hdmi,
>  			     const struct drm_display_mode *mode)
>  {
> @@ -2064,6 +2100,7 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
>  		/* HDMI Initialization Step F - Configure AVI InfoFrame */
>  		hdmi_config_AVI(hdmi, mode);
>  		hdmi_config_vendor_specific_infoframe(hdmi, mode);
> +		hdmi_config_drm_infoframe(hdmi);
>  	} else {
>  		dev_dbg(hdmi->dev, "%s DVI mode\n", __func__);
>  	}
> @@ -2230,6 +2267,45 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector)
>  	return ret;
>  }
>  
> +static bool hdr_metadata_equal(const struct drm_connector_state *old_state,
> +			       const struct drm_connector_state *new_state)
> +{
> +	struct drm_property_blob *old_blob = old_state->hdr_output_metadata;
> +	struct drm_property_blob *new_blob = new_state->hdr_output_metadata;
> +
> +	if (!old_blob || !new_blob)
> +		return old_blob == new_blob;
> +
> +	if (old_blob->length != new_blob->length)
> +		return false;
> +
> +	return !memcmp(old_blob->data, new_blob->data, old_blob->length);
> +}
> +
> +static int dw_hdmi_connector_atomic_check(struct drm_connector *connector,
> +					  struct drm_atomic_state *state)
> +{
> +	struct drm_connector_state *old_state =
> +		drm_atomic_get_old_connector_state(state, connector);
> +	struct drm_connector_state *new_state =
> +		drm_atomic_get_new_connector_state(state, connector);
> +	struct drm_crtc *crtc = new_state->crtc;
> +	struct drm_crtc_state *crtc_state;
> +
> +	if (!crtc)
> +		return 0;
> +
> +	if (!hdr_metadata_equal(old_state, new_state)) {
> +		crtc_state = drm_atomic_get_crtc_state(state, crtc);
> +		if (IS_ERR(crtc_state))
> +			return PTR_ERR(crtc_state);
> +
> +		crtc_state->mode_changed = true;
> +	}
> +
> +	return 0;
> +}
> +
>  static void dw_hdmi_connector_force(struct drm_connector *connector)
>  {
>  	struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
> @@ -2254,6 +2330,7 @@ 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,
>  };
>  
>  static int dw_hdmi_bridge_attach(struct drm_bridge *bridge)
> @@ -2274,6 +2351,10 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge)
>  				    DRM_MODE_CONNECTOR_HDMIA,
>  				    hdmi->ddc);
>  
> +	if (hdmi->version >= 0x200a && hdmi->plat_data->use_drm_infoframe)
> +		drm_object_attach_property(&connector->base,
> +			connector->dev->mode_config.hdr_output_metadata_property, 0);
> +
>  	drm_connector_attach_encoder(connector, encoder);
>  
>  	cec_fill_conn_info_from_drm(&conn_info, connector);
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> index fcff5059db24..1999db05bc3b 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> @@ -254,6 +254,7 @@
>  #define HDMI_FC_POL2                            0x10DB
>  #define HDMI_FC_PRCONF                          0x10E0
>  #define HDMI_FC_SCRAMBLER_CTRL                  0x10E1
> +#define HDMI_FC_PACKET_TX_EN                    0x10E3
>  
>  #define HDMI_FC_GMD_STAT                        0x1100
>  #define HDMI_FC_GMD_EN                          0x1101
> @@ -289,6 +290,37 @@
>  #define HDMI_FC_GMD_PB26                        0x111F
>  #define HDMI_FC_GMD_PB27                        0x1120
>  
> +#define HDMI_FC_DRM_UP                          0x1167
> +#define HDMI_FC_DRM_HB0                         0x1168
> +#define HDMI_FC_DRM_HB1                         0x1169
> +#define HDMI_FC_DRM_PB0                         0x116A
> +#define HDMI_FC_DRM_PB1                         0x116B
> +#define HDMI_FC_DRM_PB2                         0x116C
> +#define HDMI_FC_DRM_PB3                         0x116D
> +#define HDMI_FC_DRM_PB4                         0x116E
> +#define HDMI_FC_DRM_PB5                         0x116F
> +#define HDMI_FC_DRM_PB6                         0x1170
> +#define HDMI_FC_DRM_PB7                         0x1171
> +#define HDMI_FC_DRM_PB8                         0x1172
> +#define HDMI_FC_DRM_PB9                         0x1173
> +#define HDMI_FC_DRM_PB10                        0x1174
> +#define HDMI_FC_DRM_PB11                        0x1175
> +#define HDMI_FC_DRM_PB12                        0x1176
> +#define HDMI_FC_DRM_PB13                        0x1177
> +#define HDMI_FC_DRM_PB14                        0x1178
> +#define HDMI_FC_DRM_PB15                        0x1179
> +#define HDMI_FC_DRM_PB16                        0x117A
> +#define HDMI_FC_DRM_PB17                        0x117B
> +#define HDMI_FC_DRM_PB18                        0x117C
> +#define HDMI_FC_DRM_PB19                        0x117D
> +#define HDMI_FC_DRM_PB20                        0x117E
> +#define HDMI_FC_DRM_PB21                        0x117F
> +#define HDMI_FC_DRM_PB22                        0x1180
> +#define HDMI_FC_DRM_PB23                        0x1181
> +#define HDMI_FC_DRM_PB24                        0x1182
> +#define HDMI_FC_DRM_PB25                        0x1183
> +#define HDMI_FC_DRM_PB26                        0x1184
> +
>  #define HDMI_FC_DBGFORCE                        0x1200
>  #define HDMI_FC_DBGAUD0CH0                      0x1201
>  #define HDMI_FC_DBGAUD1CH0                      0x1202
> @@ -744,6 +776,11 @@ enum {
>  	HDMI_FC_PRCONF_OUTPUT_PR_FACTOR_MASK = 0x0F,
>  	HDMI_FC_PRCONF_OUTPUT_PR_FACTOR_OFFSET = 0,
>  
> +/* FC_PACKET_TX_EN field values */
> +	HDMI_FC_PACKET_TX_EN_DRM_MASK = 0x80,
> +	HDMI_FC_PACKET_TX_EN_DRM_ENABLE = 0x80,
> +	HDMI_FC_PACKET_TX_EN_DRM_DISABLE = 0x00,
> +
>  /* FC_AVICONF0-FC_AVICONF3 field values */
>  	HDMI_FC_AVICONF0_PIX_FMT_MASK = 0x03,
>  	HDMI_FC_AVICONF0_PIX_FMT_RGB = 0x00,
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index 4b3e863c4f8a..fbf3812c4326 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -126,6 +126,7 @@ struct dw_hdmi_plat_data {
>  					   const struct drm_display_mode *mode);
>  	unsigned long input_bus_format;
>  	unsigned long input_bus_encoding;
> +	bool use_drm_infoframe;
>  
>  	/* Vendor PHY support */
>  	const struct dw_hdmi_phy_ops *phy_ops;