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@xxxxxxxxxxxxxx> > 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@xxxxxxxxxxxxxx> > Signed-off-by: Jonas Karlman <jonas@xxxxxxxxx> > --- > 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); > } else { > dev_dbg(hdmi->dev, "%s DVI mode\n", __func__); > } > @@ -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; _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
