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 <jo...@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 <jo...@kwiboo.se>
> >> Signed-off-by: Neil Armstrong <narmstr...@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);
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
Reviewed-by: Laurent Pinchart <laurent.pinch...@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);
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
