Re: [PATCH v2 6/6] drm/meson: encoder_cvbs: switch to bridge with ATTACH_NO_CONNECTOR
Hi, On 16/10/2021 00:34, Martin Blumenstingl wrote: > Hi Neil, Hi Sam, > > On Fri, Oct 15, 2021 at 4:11 PM Neil Armstrong > wrote: > [...] >> +static const struct drm_bridge_funcs meson_encoder_cvbs_bridge_funcs = { >> + .attach = meson_encoder_cvbs_attach, >> + .enable = meson_encoder_cvbs_enable, >> + .disable = meson_encoder_cvbs_disable, >> + .mode_valid = meson_encoder_cvbs_mode_valid, >> + .get_modes = meson_encoder_cvbs_get_modes, >> + .atomic_enable = meson_encoder_cvbs_atomic_enable, > I did some testing on boards where u-boot doesn't initialize the video > outputs. > It seems that meson_encoder_cvbs_enable() is never called with this patch. > meson_encoder_cvbs_atomic_enable() is called though. > > From what I can see in drm_bridge.c [0] this is even expected. > Does this mean that we need to move all logic from .enable to > .atomic_enable (and same with .disable -> moving that logic to > .atomic_disable)? > > The same comment applies to the HDMI patch. Good point, I'll fix that for both patches ! Neil > > > Best regards, > Martin > > > [0] > https://elixir.bootlin.com/linux/v5.15-rc5/source/drivers/gpu/drm/drm_bridge.c#L717 >
Re: [PATCH v2 6/6] drm/meson: encoder_cvbs: switch to bridge with ATTACH_NO_CONNECTOR
Hi Neil, Hi Sam, On Fri, Oct 15, 2021 at 4:11 PM Neil Armstrong wrote: [...] > +static const struct drm_bridge_funcs meson_encoder_cvbs_bridge_funcs = { > + .attach = meson_encoder_cvbs_attach, > + .enable = meson_encoder_cvbs_enable, > + .disable = meson_encoder_cvbs_disable, > + .mode_valid = meson_encoder_cvbs_mode_valid, > + .get_modes = meson_encoder_cvbs_get_modes, > + .atomic_enable = meson_encoder_cvbs_atomic_enable, I did some testing on boards where u-boot doesn't initialize the video outputs. It seems that meson_encoder_cvbs_enable() is never called with this patch. meson_encoder_cvbs_atomic_enable() is called though. >From what I can see in drm_bridge.c [0] this is even expected. Does this mean that we need to move all logic from .enable to .atomic_enable (and same with .disable -> moving that logic to .atomic_disable)? The same comment applies to the HDMI patch. Best regards, Martin [0] https://elixir.bootlin.com/linux/v5.15-rc5/source/drivers/gpu/drm/drm_bridge.c#L717
Re: [PATCH v2 6/6] drm/meson: encoder_cvbs: switch to bridge with ATTACH_NO_CONNECTOR
On Fri, Oct 15, 2021 at 4:11 PM Neil Armstrong wrote: > > Drop the local connector and move all callback to bridge funcs in order > to leverage the generic CVBS display connector. > > This will also permit adding custom cvbs connectors for ADC based HPD > detection on some Amlogic SoC based boards. > > Signed-off-by: Neil Armstrong > Acked-by: Sam Ravnborg Acked-by: Martin Blumenstingl