On 26.05.2019 23:19, Jonas Karlman wrote:
Add support for configuring Dynamic Range and Mastering InfoFrame from the hdr_output_metadata connector property.
This patch adds a 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/d1943fde81ff41d7cca87f4a42f0...
Cc: Zheng Yang zhengyang@rock-chips.com Signed-off-by: Jonas Karlman jonas@kwiboo.se
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 109 ++++++++++++++++++++++ drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 37 ++++++++ include/drm/bridge/dw_hdmi.h | 1 + 3 files changed, 147 insertions(+)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index 284ce59be8f8..801bbbd732fd 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -24,6 +24,7 @@
#include <drm/drm_of.h> #include <drm/drmP.h> +#include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_edid.h> #include <drm/drm_encoder_slave.h> @@ -1592,6 +1593,78 @@ static void hdmi_config_vendor_specific_infoframe(struct dw_hdmi *hdmi, HDMI_FC_DATAUTO0_VSD_MASK); }
+#define HDR_LSB(n) ((n) & 0xff) +#define HDR_MSB(n) (((n) & 0xff00) >> 8)
+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;
- int ret;
- if (hdmi->version < 0x200a || !hdmi->plat_data->drm_infoframe)
Why do we need this double check? Here and in dw_hdmi_bridge_attach.
Wouldn't be enough to test 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);- ret = drm_hdmi_infoframe_set_hdr_metadata(&frame, conn_state);
- if (ret < 0)
return;- ret = hdmi_drm_infoframe_check(&frame);
- if (WARN_ON(ret))
return;- hdmi_writeb(hdmi, frame.version, HDMI_FC_DRM_HB0);
- hdmi_writeb(hdmi, frame.length, HDMI_FC_DRM_HB1);
- hdmi_writeb(hdmi, frame.eotf, HDMI_FC_DRM_PB0);
- hdmi_writeb(hdmi, frame.metadata_type, HDMI_FC_DRM_PB1);
- hdmi_writeb(hdmi, HDR_LSB(frame.display_primaries[0].x),
HDMI_FC_DRM_PB2);- hdmi_writeb(hdmi, HDR_MSB(frame.display_primaries[0].x),
HDMI_FC_DRM_PB3);
Adding two-byte version hdmi_writew would allow to remove multiple lines here, but since register names and their relative location reflects structure of raw frame you should be able to replace this long/boring/error-prone sequence of writes with something like:
len = hdmi_infoframe_pack(frame, buf, len);
if (len < 0)
...
for (i = 0; i < len; ++i)
hdmi_writeb(hdmi, buf[i], HDMI_FC_DRM_HB0 + i);
... or something similar.
- hdmi_writeb(hdmi, HDR_LSB(frame.display_primaries[0].y),
HDMI_FC_DRM_PB4);- hdmi_writeb(hdmi, HDR_MSB(frame.display_primaries[0].y),
HDMI_FC_DRM_PB5);- hdmi_writeb(hdmi, HDR_LSB(frame.display_primaries[1].x),
HDMI_FC_DRM_PB6);- hdmi_writeb(hdmi, HDR_MSB(frame.display_primaries[1].x),
HDMI_FC_DRM_PB7);- hdmi_writeb(hdmi, HDR_LSB(frame.display_primaries[1].y),
HDMI_FC_DRM_PB8);- hdmi_writeb(hdmi, HDR_MSB(frame.display_primaries[1].y),
HDMI_FC_DRM_PB9);- hdmi_writeb(hdmi, HDR_LSB(frame.display_primaries[2].x),
HDMI_FC_DRM_PB10);- hdmi_writeb(hdmi, HDR_MSB(frame.display_primaries[2].x),
HDMI_FC_DRM_PB11);- hdmi_writeb(hdmi, HDR_LSB(frame.display_primaries[2].y),
HDMI_FC_DRM_PB12);- hdmi_writeb(hdmi, HDR_MSB(frame.display_primaries[2].y),
HDMI_FC_DRM_PB13);- hdmi_writeb(hdmi, HDR_LSB(frame.white_point.x), HDMI_FC_DRM_PB14);
- hdmi_writeb(hdmi, HDR_MSB(frame.white_point.x), HDMI_FC_DRM_PB15);
- hdmi_writeb(hdmi, HDR_LSB(frame.white_point.y), HDMI_FC_DRM_PB16);
- hdmi_writeb(hdmi, HDR_MSB(frame.white_point.y), HDMI_FC_DRM_PB17);
- hdmi_writeb(hdmi, HDR_LSB(frame.max_display_mastering_luminance),
HDMI_FC_DRM_PB18);- hdmi_writeb(hdmi, HDR_MSB(frame.max_display_mastering_luminance),
HDMI_FC_DRM_PB19);- hdmi_writeb(hdmi, HDR_LSB(frame.min_display_mastering_luminance),
HDMI_FC_DRM_PB20);- hdmi_writeb(hdmi, HDR_MSB(frame.min_display_mastering_luminance),
HDMI_FC_DRM_PB21);- hdmi_writeb(hdmi, HDR_LSB(frame.max_cll), HDMI_FC_DRM_PB22);
- hdmi_writeb(hdmi, HDR_MSB(frame.max_cll), HDMI_FC_DRM_PB23);
- hdmi_writeb(hdmi, HDR_LSB(frame.max_fall), HDMI_FC_DRM_PB24);
- hdmi_writeb(hdmi, HDR_MSB(frame.max_fall), HDMI_FC_DRM_PB25);
- 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) { @@ -1963,6 +2036,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);@@ -2139,6 +2213,36 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector) return ret; }
+static bool blob_equal(const struct drm_property_blob *a,
const struct drm_property_blob *b)
Quite generic function, but since there are no other users(?) it can stay here.
+{
- if (a && b)
return a->length == b->length &&!memcmp(a->data, b->data, a->length);- return !a == !b;
Ugly code (but it is probably matter of taste), maybe sth like this:
if (!a || !b)
return a != b;
if (a->length != b->length)
return false;
return !memcmp(a->data, b->data, a->length);
+}
+static int dw_hdmi_connector_atomic_check(struct drm_connector *conn,
- struct drm_connector_state *new_state)
+{
- struct drm_connector_state *old_state =
drm_atomic_get_old_connector_state(new_state->state, conn);- struct drm_crtc_state *crtc_state;
- if (!new_state->crtc)
return 0;- crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
new_state->crtc);- if (!blob_equal(new_state->hdr_output_metadata,
old_state->hdr_output_metadata))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, @@ -2163,6 +2267,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) @@ -2179,6 +2284,10 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge) drm_connector_init(bridge->dev, connector, &dw_hdmi_connector_funcs, DRM_MODE_CONNECTOR_HDMIA);
if (hdmi->version >= 0x200a && hdmi->plat_data->drm_infoframe)
drm_object_attach_property(&connector->base,connector->dev->mode_config.hdr_output_metadata_property, 0);drm_connector_attach_encoder(connector, encoder);
return 0;
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h index 3f3c616eba97..d4efbec14f68 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h @@ -256,6 +256,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 @@ -291,6 +292,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 @@ -746,6 +778,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 0f0e82638fbe..04d1ec60f218 100644 --- a/include/drm/bridge/dw_hdmi.h +++ b/include/drm/bridge/dw_hdmi.h @@ -131,6 +131,7 @@ struct dw_hdmi_plat_data { unsigned long input_bus_format; unsigned long input_bus_encoding; bool ycbcr_420_allowed;
- bool drm_infoframe;
maybe sth less confusing, for example: use_drm_infoframe?
Regards
Andrzej
/* Vendor PHY support */ const struct dw_hdmi_phy_ops *phy_ops;