On 02/03/2020 10:18, Laurent Pinchart wrote:
Hi Neil and Jonas,
(CC'ing Daniel for a framework question)
Thank you for the patch.
On Fri, Feb 21, 2020 at 09:50:18AM +0100, Neil Armstrong wrote:
On 17/02/2020 07:38, Jernej Škrabec wrote:
Dne četrtek, 06. februar 2020 ob 20:18:25 CET je Neil Armstrong napisal(a):
From: Jonas Karlman jonas@kwiboo.se
Add the max_bpc property to the dw-hdmi connector to prepare support for 10, 12 & 16bit output support.
Signed-off-by: Jonas Karlman jonas@kwiboo.se Signed-off-by: Neil Armstrong narmstrong@baylibre.com
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index 9e0927d22db6..051001f77dd4 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -2406,6 +2406,10 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge) DRM_MODE_CONNECTOR_HDMIA, hdmi->ddc);
- drm_atomic_helper_connector_reset(connector);
Why is this reset needed?
I assume it's to allocate a new connector state to attach a the bpc propery.
But indeed, this helper is never used here, but only as callback to the drm_connector_funcs->reset.
But, amdgpu calls : /* * Some of the properties below require access to state, like bpc. * Allocate some default initial connector state with our reset helper. */ if (aconnector->base.funcs->reset) aconnector->base.funcs->reset(&aconnector->base);
which is the same.
A comment would be useful:
/* * drm_connector_attach_max_bpc_property() requires the * connector to have a state. */ drm_atomic_helper_connector_reset(connector);
drm_connector_attach_max_bpc_property(connector, 8, 16);
Done
I don't like this much though, it feels like the initial reset performed by drm_mode_config_reset() should set default values for all state members that are related to properties. Daniel, what's the rationale behind the current implementation ?
This is a DRM core issue that shouldn't block this patch though, so
I'll investigate why, but I haven't found out how the intel driver got the connector state initialized since they don't use the atomic helpers....
Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
- drm_connector_attach_max_bpc_property(connector, 8, 16);
- 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);