| Message ID | 20211014152606.2289528-8-narmstrong@baylibre.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Neil Armstrong |
| Headers | show |
| Series | drm/meson: rework encoders to pass ATTACH_NO_CONNECTOR | expand |
Hi Neil, with include order fixed and the comment below considered: Acked-by: Sam Ravnborg <sam@ravnborg.org> Sam On Thu, Oct 14, 2021 at 05:26:06PM +0200, Neil Armstrong wrote: > Drop the local connector and move all callback to bridge funcs in order > to leverage the generic CVBS diplay 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 <narmstrong@baylibre.com> > --- > drivers/gpu/drm/meson/meson_encoder_cvbs.c | 178 +++++++++------------ > 1 file changed, 79 insertions(+), 99 deletions(-) > > diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.c b/drivers/gpu/drm/meson/meson_encoder_cvbs.c > index 01024c5f610c..0b974667cf55 100644 > --- a/drivers/gpu/drm/meson/meson_encoder_cvbs.c > +++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.c > @@ -13,6 +13,9 @@ > #include <linux/of_graph.h> > > #include <drm/drm_atomic_helper.h> > +#include <drm/drm_simple_kms_helper.h> > +#include <drm/drm_bridge.h> > +#include <drm/drm_bridge_connector.h> > #include <drm/drm_device.h> > #include <drm/drm_edid.h> > #include <drm/drm_probe_helper.h> alphabetic order. > @@ -30,14 +33,14 @@ > > struct meson_encoder_cvbs { > struct drm_encoder encoder; > - struct drm_connector connector; > + struct drm_bridge bridge; > + struct drm_bridge *next_bridge; > + struct drm_connector *connector; Maybe drop this - see later. > struct meson_drm *priv; > }; > -#define encoder_to_meson_encoder_cvbs(x) \ > - container_of(x, struct meson_encoder_cvbs, encoder) > > -#define connector_to_meson_encoder_cvbs(x) \ > - container_of(x, struct meson_encoder_cvbs, connector) > +#define bridge_to_meson_encoder_cvbs(x) \ > + container_of(x, struct meson_encoder_cvbs, bridge) > > /* Supported Modes */ > > @@ -81,21 +84,18 @@ meson_cvbs_get_mode(const struct drm_display_mode *req_mode) > return NULL; > } > > -/* Connector */ > - > -static void meson_cvbs_connector_destroy(struct drm_connector *connector) > +static int meson_encoder_cvbs_attach(struct drm_bridge *bridge, > + enum drm_bridge_attach_flags flags) > { > - drm_connector_cleanup(connector); > -} > + struct meson_encoder_cvbs *meson_encoder_cvbs = > + bridge_to_meson_encoder_cvbs(bridge); > > -static enum drm_connector_status > -meson_cvbs_connector_detect(struct drm_connector *connector, bool force) > -{ > - /* FIXME: Add load-detect or jack-detect if possible */ > - return connector_status_connected; > + return drm_bridge_attach(bridge->encoder, meson_encoder_cvbs->next_bridge, > + &meson_encoder_cvbs->bridge, flags); > } > > -static int meson_cvbs_connector_get_modes(struct drm_connector *connector) > +static int meson_encoder_cvbs_get_modes(struct drm_bridge *bridge, > + struct drm_connector *connector) > { > struct drm_device *dev = connector->dev; > struct drm_display_mode *mode; > @@ -116,40 +116,18 @@ static int meson_cvbs_connector_get_modes(struct drm_connector *connector) > return i; > } > > -static int meson_cvbs_connector_mode_valid(struct drm_connector *connector, > - struct drm_display_mode *mode) > +static int meson_encoder_cvbs_mode_valid(struct drm_bridge *bridge, > + const struct drm_display_info *display_info, > + const struct drm_display_mode *mode) > { > - /* Validate the modes added in get_modes */ > - return MODE_OK; > -} > - > -static const struct drm_connector_funcs meson_cvbs_connector_funcs = { > - .detect = meson_cvbs_connector_detect, > - .fill_modes = drm_helper_probe_single_connector_modes, > - .destroy = meson_cvbs_connector_destroy, > - .reset = drm_atomic_helper_connector_reset, > - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, > - .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > -}; > - > -static const > -struct drm_connector_helper_funcs meson_cvbs_connector_helper_funcs = { > - .get_modes = meson_cvbs_connector_get_modes, > - .mode_valid = meson_cvbs_connector_mode_valid, > -}; > + if (meson_cvbs_get_mode(mode)) > + return MODE_OK; > > -/* Encoder */ > - > -static void meson_encoder_cvbs_encoder_destroy(struct drm_encoder *encoder) > -{ > - drm_encoder_cleanup(encoder); > + return MODE_BAD; > } > > -static const struct drm_encoder_funcs meson_encoder_cvbs_encoder_funcs = { > - .destroy = meson_encoder_cvbs_encoder_destroy, > -}; > - > -static int meson_encoder_cvbs_encoder_atomic_check(struct drm_encoder *encoder, > +static int meson_encoder_cvbs_atomic_check(struct drm_bridge *bridge, > + struct drm_bridge_state *bridge_state, > struct drm_crtc_state *crtc_state, > struct drm_connector_state *conn_state) > { > @@ -159,10 +137,10 @@ static int meson_encoder_cvbs_encoder_atomic_check(struct drm_encoder *encoder, > return -EINVAL; > } > > -static void meson_encoder_cvbs_encoder_disable(struct drm_encoder *encoder) > +static void meson_encoder_cvbs_disable(struct drm_bridge *bridge) > { > struct meson_encoder_cvbs *meson_encoder_cvbs = > - encoder_to_meson_encoder_cvbs(encoder); > + bridge_to_meson_encoder_cvbs(bridge); > struct meson_drm *priv = meson_encoder_cvbs->priv; > > /* Disable CVBS VDAC */ > @@ -175,10 +153,10 @@ static void meson_encoder_cvbs_encoder_disable(struct drm_encoder *encoder) > } > } > > -static void meson_encoder_cvbs_encoder_enable(struct drm_encoder *encoder) > +static void meson_encoder_cvbs_enable(struct drm_bridge *bridge) > { > struct meson_encoder_cvbs *meson_encoder_cvbs = > - encoder_to_meson_encoder_cvbs(encoder); > + bridge_to_meson_encoder_cvbs(bridge); > struct meson_drm *priv = meson_encoder_cvbs->priv; > > /* VDAC0 source is not from ATV */ > @@ -198,13 +176,13 @@ static void meson_encoder_cvbs_encoder_enable(struct drm_encoder *encoder) > } > } > > -static void meson_encoder_cvbs_encoder_mode_set(struct drm_encoder *encoder, > - struct drm_display_mode *mode, > - struct drm_display_mode *adjusted_mode) > +static void meson_encoder_cvbs_mode_set(struct drm_bridge *bridge, > + const struct drm_display_mode *mode, > + const struct drm_display_mode *adjusted_mode) > { > const struct meson_cvbs_mode *meson_mode = meson_cvbs_get_mode(mode); > struct meson_encoder_cvbs *meson_encoder_cvbs = > - encoder_to_meson_encoder_cvbs(encoder); > + bridge_to_meson_encoder_cvbs(bridge); > struct meson_drm *priv = meson_encoder_cvbs->priv; > > if (meson_mode) { > @@ -218,76 +196,78 @@ static void meson_encoder_cvbs_encoder_mode_set(struct drm_encoder *encoder, > } > } > > -static const struct drm_encoder_helper_funcs > - meson_encoder_cvbs_encoder_helper_funcs = { > - .atomic_check = meson_encoder_cvbs_encoder_atomic_check, > - .disable = meson_encoder_cvbs_encoder_disable, > - .enable = meson_encoder_cvbs_encoder_enable, > - .mode_set = meson_encoder_cvbs_encoder_mode_set, > +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, > + .mode_set = meson_encoder_cvbs_mode_set, > + .get_modes = meson_encoder_cvbs_get_modes, > + .atomic_check = meson_encoder_cvbs_atomic_check, > }; > > -static bool meson_encoder_cvbs_connector_is_available(struct meson_drm *priv) > -{ > - struct device_node *remote; > - > - remote = of_graph_get_remote_node(priv->dev->of_node, 0, 0); > - if (!remote) > - return false; > - > - of_node_put(remote); > - return true; > -} > - > int meson_encoder_cvbs_init(struct meson_drm *priv) > { > struct drm_device *drm = priv->drm; > struct meson_encoder_cvbs *meson_encoder_cvbs; > - struct drm_connector *connector; > - struct drm_encoder *encoder; > + struct device_node *remote; > int ret; > > - if (!meson_encoder_cvbs_connector_is_available(priv)) { > + meson_encoder_cvbs = devm_kzalloc(priv->dev, sizeof(*meson_encoder_cvbs), GFP_KERNEL); > + if (!meson_encoder_cvbs) > + return -ENOMEM; > + > + /* CVBS Connector Bridge */ > + remote = of_graph_get_remote_node(priv->dev->of_node, 0, 0); > + if (!remote) { > dev_info(drm->dev, "CVBS Output connector not available\n"); > return 0; > } > > - meson_encoder_cvbs = devm_kzalloc(priv->dev, sizeof(*meson_encoder_cvbs), > - GFP_KERNEL); > - if (!meson_encoder_cvbs) > - return -ENOMEM; > + meson_encoder_cvbs->next_bridge = of_drm_find_bridge(remote); > + if (!meson_encoder_cvbs->next_bridge) { > + dev_err(priv->dev, "Failed to find CVBS Connector bridge\n"); > + return -EPROBE_DEFER; > + } > > - meson_encoder_cvbs->priv = priv; > - encoder = &meson_encoder_cvbs->encoder; > - connector = &meson_encoder_cvbs->connector; > + /* CVBS Encoder Bridge */ > + meson_encoder_cvbs->bridge.funcs = &meson_encoder_cvbs_bridge_funcs; > + meson_encoder_cvbs->bridge.of_node = priv->dev->of_node; > + meson_encoder_cvbs->bridge.type = DRM_MODE_CONNECTOR_Composite; > + meson_encoder_cvbs->bridge.ops = DRM_BRIDGE_OP_MODES; > + meson_encoder_cvbs->bridge.interlace_allowed = true; > > - /* Connector */ > + drm_bridge_add(&meson_encoder_cvbs->bridge); > > - drm_connector_helper_add(connector, > - &meson_cvbs_connector_helper_funcs); > + meson_encoder_cvbs->priv = priv; > > - ret = drm_connector_init(drm, connector, &meson_cvbs_connector_funcs, > - DRM_MODE_CONNECTOR_Composite); > + /* Encoder */ > + ret = drm_simple_encoder_init(priv->drm, &meson_encoder_cvbs->encoder, > + DRM_MODE_ENCODER_TVDAC); > if (ret) { > - dev_err(priv->dev, "Failed to init CVBS connector\n"); > + dev_err(priv->dev, "Failed to init CVBS encoder: %d\n", ret); > return ret; > } > > - connector->interlace_allowed = 1; > + meson_encoder_cvbs->encoder.possible_crtcs = BIT(0); > > - /* Encoder */ > - > - drm_encoder_helper_add(encoder, &meson_encoder_cvbs_encoder_helper_funcs); > - > - ret = drm_encoder_init(drm, encoder, &meson_encoder_cvbs_encoder_funcs, > - DRM_MODE_ENCODER_TVDAC, "meson_encoder_cvbs"); > + /* Attach CVBS Encoder Bridge to Encoder */ > + ret = drm_bridge_attach(&meson_encoder_cvbs->encoder, &meson_encoder_cvbs->bridge, NULL, > + DRM_BRIDGE_ATTACH_NO_CONNECTOR); > if (ret) { > - dev_err(priv->dev, "Failed to init CVBS encoder\n"); > + dev_err(priv->dev, "Failed to attach bridge: %d\n", ret); > return ret; > } > > - encoder->possible_crtcs = BIT(0); > - > - drm_connector_attach_encoder(connector, encoder); > + /* Initialize & attach Bridge Connector */ > + meson_encoder_cvbs->connector = drm_bridge_connector_init(priv->drm, > + &meson_encoder_cvbs->encoder); I did not see other uses of meson_encoder_cvbs->connector, so if I am right a local variable can be used and the member dropped. > + if (IS_ERR(meson_encoder_cvbs->connector)) { > + dev_err(priv->dev, "Unable to create CVBS bridge connector\n"); > + return PTR_ERR(meson_encoder_cvbs->connector); > + } > + drm_connector_attach_encoder(meson_encoder_cvbs->connector, > + &meson_encoder_cvbs->encoder); > > return 0; > } > -- > 2.25.1
Hi Sam, On 14/10/2021 20:15, Sam Ravnborg wrote: > Hi Neil, > > with include order fixed and the comment below considered: > Acked-by: Sam Ravnborg <sam@ravnborg.org> > > Sam > > > On Thu, Oct 14, 2021 at 05:26:06PM +0200, Neil Armstrong wrote: >> Drop the local connector and move all callback to bridge funcs in order >> to leverage the generic CVBS diplay 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 <narmstrong@baylibre.com> >> --- >> drivers/gpu/drm/meson/meson_encoder_cvbs.c | 178 +++++++++------------ >> 1 file changed, 79 insertions(+), 99 deletions(-) >> >> diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.c b/drivers/gpu/drm/meson/meson_encoder_cvbs.c >> index 01024c5f610c..0b974667cf55 100644 >> --- a/drivers/gpu/drm/meson/meson_encoder_cvbs.c >> +++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.c >> @@ -13,6 +13,9 @@ >> #include <linux/of_graph.h> >> >> #include <drm/drm_atomic_helper.h> >> +#include <drm/drm_simple_kms_helper.h> >> +#include <drm/drm_bridge.h> >> +#include <drm/drm_bridge_connector.h> >> #include <drm/drm_device.h> >> #include <drm/drm_edid.h> >> #include <drm/drm_probe_helper.h> > alphabetic order. > >> @@ -30,14 +33,14 @@ >> >> struct meson_encoder_cvbs { >> struct drm_encoder encoder; >> - struct drm_connector connector; >> + struct drm_bridge bridge; >> + struct drm_bridge *next_bridge; >> + struct drm_connector *connector; > Maybe drop this - see later. > >> struct meson_drm *priv; >> }; >> -#define encoder_to_meson_encoder_cvbs(x) \ >> - container_of(x, struct meson_encoder_cvbs, encoder) >> >> -#define connector_to_meson_encoder_cvbs(x) \ >> - container_of(x, struct meson_encoder_cvbs, connector) >> +#define bridge_to_meson_encoder_cvbs(x) \ >> + container_of(x, struct meson_encoder_cvbs, bridge) >> >> /* Supported Modes */ >> >> @@ -81,21 +84,18 @@ meson_cvbs_get_mode(const struct drm_display_mode *req_mode) >> return NULL; >> } >> >> -/* Connector */ >> - >> -static void meson_cvbs_connector_destroy(struct drm_connector *connector) >> +static int meson_encoder_cvbs_attach(struct drm_bridge *bridge, >> + enum drm_bridge_attach_flags flags) >> { >> - drm_connector_cleanup(connector); >> -} >> + struct meson_encoder_cvbs *meson_encoder_cvbs = >> + bridge_to_meson_encoder_cvbs(bridge); >> >> -static enum drm_connector_status >> -meson_cvbs_connector_detect(struct drm_connector *connector, bool force) >> -{ >> - /* FIXME: Add load-detect or jack-detect if possible */ >> - return connector_status_connected; >> + return drm_bridge_attach(bridge->encoder, meson_encoder_cvbs->next_bridge, >> + &meson_encoder_cvbs->bridge, flags); >> } >> >> -static int meson_cvbs_connector_get_modes(struct drm_connector *connector) >> +static int meson_encoder_cvbs_get_modes(struct drm_bridge *bridge, >> + struct drm_connector *connector) >> { >> struct drm_device *dev = connector->dev; >> struct drm_display_mode *mode; >> @@ -116,40 +116,18 @@ static int meson_cvbs_connector_get_modes(struct drm_connector *connector) >> return i; >> } >> >> -static int meson_cvbs_connector_mode_valid(struct drm_connector *connector, >> - struct drm_display_mode *mode) >> +static int meson_encoder_cvbs_mode_valid(struct drm_bridge *bridge, >> + const struct drm_display_info *display_info, >> + const struct drm_display_mode *mode) >> { >> - /* Validate the modes added in get_modes */ >> - return MODE_OK; >> -} >> - >> -static const struct drm_connector_funcs meson_cvbs_connector_funcs = { >> - .detect = meson_cvbs_connector_detect, >> - .fill_modes = drm_helper_probe_single_connector_modes, >> - .destroy = meson_cvbs_connector_destroy, >> - .reset = drm_atomic_helper_connector_reset, >> - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, >> - .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, >> -}; >> - >> -static const >> -struct drm_connector_helper_funcs meson_cvbs_connector_helper_funcs = { >> - .get_modes = meson_cvbs_connector_get_modes, >> - .mode_valid = meson_cvbs_connector_mode_valid, >> -}; >> + if (meson_cvbs_get_mode(mode)) >> + return MODE_OK; >> >> -/* Encoder */ >> - >> -static void meson_encoder_cvbs_encoder_destroy(struct drm_encoder *encoder) >> -{ >> - drm_encoder_cleanup(encoder); >> + return MODE_BAD; >> } >> >> -static const struct drm_encoder_funcs meson_encoder_cvbs_encoder_funcs = { >> - .destroy = meson_encoder_cvbs_encoder_destroy, >> -}; >> - >> -static int meson_encoder_cvbs_encoder_atomic_check(struct drm_encoder *encoder, >> +static int meson_encoder_cvbs_atomic_check(struct drm_bridge *bridge, >> + struct drm_bridge_state *bridge_state, >> struct drm_crtc_state *crtc_state, >> struct drm_connector_state *conn_state) >> { >> @@ -159,10 +137,10 @@ static int meson_encoder_cvbs_encoder_atomic_check(struct drm_encoder *encoder, >> return -EINVAL; >> } >> >> -static void meson_encoder_cvbs_encoder_disable(struct drm_encoder *encoder) >> +static void meson_encoder_cvbs_disable(struct drm_bridge *bridge) >> { >> struct meson_encoder_cvbs *meson_encoder_cvbs = >> - encoder_to_meson_encoder_cvbs(encoder); >> + bridge_to_meson_encoder_cvbs(bridge); >> struct meson_drm *priv = meson_encoder_cvbs->priv; >> >> /* Disable CVBS VDAC */ >> @@ -175,10 +153,10 @@ static void meson_encoder_cvbs_encoder_disable(struct drm_encoder *encoder) >> } >> } >> >> -static void meson_encoder_cvbs_encoder_enable(struct drm_encoder *encoder) >> +static void meson_encoder_cvbs_enable(struct drm_bridge *bridge) >> { >> struct meson_encoder_cvbs *meson_encoder_cvbs = >> - encoder_to_meson_encoder_cvbs(encoder); >> + bridge_to_meson_encoder_cvbs(bridge); >> struct meson_drm *priv = meson_encoder_cvbs->priv; >> >> /* VDAC0 source is not from ATV */ >> @@ -198,13 +176,13 @@ static void meson_encoder_cvbs_encoder_enable(struct drm_encoder *encoder) >> } >> } >> >> -static void meson_encoder_cvbs_encoder_mode_set(struct drm_encoder *encoder, >> - struct drm_display_mode *mode, >> - struct drm_display_mode *adjusted_mode) >> +static void meson_encoder_cvbs_mode_set(struct drm_bridge *bridge, >> + const struct drm_display_mode *mode, >> + const struct drm_display_mode *adjusted_mode) >> { >> const struct meson_cvbs_mode *meson_mode = meson_cvbs_get_mode(mode); >> struct meson_encoder_cvbs *meson_encoder_cvbs = >> - encoder_to_meson_encoder_cvbs(encoder); >> + bridge_to_meson_encoder_cvbs(bridge); >> struct meson_drm *priv = meson_encoder_cvbs->priv; >> >> if (meson_mode) { >> @@ -218,76 +196,78 @@ static void meson_encoder_cvbs_encoder_mode_set(struct drm_encoder *encoder, >> } >> } >> >> -static const struct drm_encoder_helper_funcs >> - meson_encoder_cvbs_encoder_helper_funcs = { >> - .atomic_check = meson_encoder_cvbs_encoder_atomic_check, >> - .disable = meson_encoder_cvbs_encoder_disable, >> - .enable = meson_encoder_cvbs_encoder_enable, >> - .mode_set = meson_encoder_cvbs_encoder_mode_set, >> +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, >> + .mode_set = meson_encoder_cvbs_mode_set, >> + .get_modes = meson_encoder_cvbs_get_modes, >> + .atomic_check = meson_encoder_cvbs_atomic_check, >> }; >> >> -static bool meson_encoder_cvbs_connector_is_available(struct meson_drm *priv) >> -{ >> - struct device_node *remote; >> - >> - remote = of_graph_get_remote_node(priv->dev->of_node, 0, 0); >> - if (!remote) >> - return false; >> - >> - of_node_put(remote); >> - return true; >> -} >> - >> int meson_encoder_cvbs_init(struct meson_drm *priv) >> { >> struct drm_device *drm = priv->drm; >> struct meson_encoder_cvbs *meson_encoder_cvbs; >> - struct drm_connector *connector; >> - struct drm_encoder *encoder; >> + struct device_node *remote; >> int ret; >> >> - if (!meson_encoder_cvbs_connector_is_available(priv)) { >> + meson_encoder_cvbs = devm_kzalloc(priv->dev, sizeof(*meson_encoder_cvbs), GFP_KERNEL); >> + if (!meson_encoder_cvbs) >> + return -ENOMEM; >> + >> + /* CVBS Connector Bridge */ >> + remote = of_graph_get_remote_node(priv->dev->of_node, 0, 0); >> + if (!remote) { >> dev_info(drm->dev, "CVBS Output connector not available\n"); >> return 0; >> } >> >> - meson_encoder_cvbs = devm_kzalloc(priv->dev, sizeof(*meson_encoder_cvbs), >> - GFP_KERNEL); >> - if (!meson_encoder_cvbs) >> - return -ENOMEM; >> + meson_encoder_cvbs->next_bridge = of_drm_find_bridge(remote); >> + if (!meson_encoder_cvbs->next_bridge) { >> + dev_err(priv->dev, "Failed to find CVBS Connector bridge\n"); >> + return -EPROBE_DEFER; >> + } >> >> - meson_encoder_cvbs->priv = priv; >> - encoder = &meson_encoder_cvbs->encoder; >> - connector = &meson_encoder_cvbs->connector; >> + /* CVBS Encoder Bridge */ >> + meson_encoder_cvbs->bridge.funcs = &meson_encoder_cvbs_bridge_funcs; >> + meson_encoder_cvbs->bridge.of_node = priv->dev->of_node; >> + meson_encoder_cvbs->bridge.type = DRM_MODE_CONNECTOR_Composite; >> + meson_encoder_cvbs->bridge.ops = DRM_BRIDGE_OP_MODES; >> + meson_encoder_cvbs->bridge.interlace_allowed = true; >> >> - /* Connector */ >> + drm_bridge_add(&meson_encoder_cvbs->bridge); >> >> - drm_connector_helper_add(connector, >> - &meson_cvbs_connector_helper_funcs); >> + meson_encoder_cvbs->priv = priv; >> >> - ret = drm_connector_init(drm, connector, &meson_cvbs_connector_funcs, >> - DRM_MODE_CONNECTOR_Composite); >> + /* Encoder */ >> + ret = drm_simple_encoder_init(priv->drm, &meson_encoder_cvbs->encoder, >> + DRM_MODE_ENCODER_TVDAC); >> if (ret) { >> - dev_err(priv->dev, "Failed to init CVBS connector\n"); >> + dev_err(priv->dev, "Failed to init CVBS encoder: %d\n", ret); >> return ret; >> } >> >> - connector->interlace_allowed = 1; >> + meson_encoder_cvbs->encoder.possible_crtcs = BIT(0); >> >> - /* Encoder */ >> - >> - drm_encoder_helper_add(encoder, &meson_encoder_cvbs_encoder_helper_funcs); >> - >> - ret = drm_encoder_init(drm, encoder, &meson_encoder_cvbs_encoder_funcs, >> - DRM_MODE_ENCODER_TVDAC, "meson_encoder_cvbs"); >> + /* Attach CVBS Encoder Bridge to Encoder */ >> + ret = drm_bridge_attach(&meson_encoder_cvbs->encoder, &meson_encoder_cvbs->bridge, NULL, >> + DRM_BRIDGE_ATTACH_NO_CONNECTOR); >> if (ret) { >> - dev_err(priv->dev, "Failed to init CVBS encoder\n"); >> + dev_err(priv->dev, "Failed to attach bridge: %d\n", ret); >> return ret; >> } >> >> - encoder->possible_crtcs = BIT(0); >> - >> - drm_connector_attach_encoder(connector, encoder); >> + /* Initialize & attach Bridge Connector */ >> + meson_encoder_cvbs->connector = drm_bridge_connector_init(priv->drm, >> + &meson_encoder_cvbs->encoder); > I did not see other uses of meson_encoder_cvbs->connector, so if I am > right a local variable can be used and the member dropped. You're right, I'll drop this. Thanks Neil > >> + if (IS_ERR(meson_encoder_cvbs->connector)) { >> + dev_err(priv->dev, "Unable to create CVBS bridge connector\n"); >> + return PTR_ERR(meson_encoder_cvbs->connector); >> + } >> + drm_connector_attach_encoder(meson_encoder_cvbs->connector, >> + &meson_encoder_cvbs->encoder); >> >> return 0; >> } >> -- >> 2.25.1
diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.c b/drivers/gpu/drm/meson/meson_encoder_cvbs.c index 01024c5f610c..0b974667cf55 100644 --- a/drivers/gpu/drm/meson/meson_encoder_cvbs.c +++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.c @@ -13,6 +13,9 @@ #include <linux/of_graph.h> #include <drm/drm_atomic_helper.h> +#include <drm/drm_simple_kms_helper.h> +#include <drm/drm_bridge.h> +#include <drm/drm_bridge_connector.h> #include <drm/drm_device.h> #include <drm/drm_edid.h> #include <drm/drm_probe_helper.h> @@ -30,14 +33,14 @@ struct meson_encoder_cvbs { struct drm_encoder encoder; - struct drm_connector connector; + struct drm_bridge bridge; + struct drm_bridge *next_bridge; + struct drm_connector *connector; struct meson_drm *priv; }; -#define encoder_to_meson_encoder_cvbs(x) \ - container_of(x, struct meson_encoder_cvbs, encoder) -#define connector_to_meson_encoder_cvbs(x) \ - container_of(x, struct meson_encoder_cvbs, connector) +#define bridge_to_meson_encoder_cvbs(x) \ + container_of(x, struct meson_encoder_cvbs, bridge) /* Supported Modes */ @@ -81,21 +84,18 @@ meson_cvbs_get_mode(const struct drm_display_mode *req_mode) return NULL; } -/* Connector */ - -static void meson_cvbs_connector_destroy(struct drm_connector *connector) +static int meson_encoder_cvbs_attach(struct drm_bridge *bridge, + enum drm_bridge_attach_flags flags) { - drm_connector_cleanup(connector); -} + struct meson_encoder_cvbs *meson_encoder_cvbs = + bridge_to_meson_encoder_cvbs(bridge); -static enum drm_connector_status -meson_cvbs_connector_detect(struct drm_connector *connector, bool force) -{ - /* FIXME: Add load-detect or jack-detect if possible */ - return connector_status_connected; + return drm_bridge_attach(bridge->encoder, meson_encoder_cvbs->next_bridge, + &meson_encoder_cvbs->bridge, flags); } -static int meson_cvbs_connector_get_modes(struct drm_connector *connector) +static int meson_encoder_cvbs_get_modes(struct drm_bridge *bridge, + struct drm_connector *connector) { struct drm_device *dev = connector->dev; struct drm_display_mode *mode; @@ -116,40 +116,18 @@ static int meson_cvbs_connector_get_modes(struct drm_connector *connector) return i; } -static int meson_cvbs_connector_mode_valid(struct drm_connector *connector, - struct drm_display_mode *mode) +static int meson_encoder_cvbs_mode_valid(struct drm_bridge *bridge, + const struct drm_display_info *display_info, + const struct drm_display_mode *mode) { - /* Validate the modes added in get_modes */ - return MODE_OK; -} - -static const struct drm_connector_funcs meson_cvbs_connector_funcs = { - .detect = meson_cvbs_connector_detect, - .fill_modes = drm_helper_probe_single_connector_modes, - .destroy = meson_cvbs_connector_destroy, - .reset = drm_atomic_helper_connector_reset, - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, - .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, -}; - -static const -struct drm_connector_helper_funcs meson_cvbs_connector_helper_funcs = { - .get_modes = meson_cvbs_connector_get_modes, - .mode_valid = meson_cvbs_connector_mode_valid, -}; + if (meson_cvbs_get_mode(mode)) + return MODE_OK; -/* Encoder */ - -static void meson_encoder_cvbs_encoder_destroy(struct drm_encoder *encoder) -{ - drm_encoder_cleanup(encoder); + return MODE_BAD; } -static const struct drm_encoder_funcs meson_encoder_cvbs_encoder_funcs = { - .destroy = meson_encoder_cvbs_encoder_destroy, -}; - -static int meson_encoder_cvbs_encoder_atomic_check(struct drm_encoder *encoder, +static int meson_encoder_cvbs_atomic_check(struct drm_bridge *bridge, + struct drm_bridge_state *bridge_state, struct drm_crtc_state *crtc_state, struct drm_connector_state *conn_state) { @@ -159,10 +137,10 @@ static int meson_encoder_cvbs_encoder_atomic_check(struct drm_encoder *encoder, return -EINVAL; } -static void meson_encoder_cvbs_encoder_disable(struct drm_encoder *encoder) +static void meson_encoder_cvbs_disable(struct drm_bridge *bridge) { struct meson_encoder_cvbs *meson_encoder_cvbs = - encoder_to_meson_encoder_cvbs(encoder); + bridge_to_meson_encoder_cvbs(bridge); struct meson_drm *priv = meson_encoder_cvbs->priv; /* Disable CVBS VDAC */ @@ -175,10 +153,10 @@ static void meson_encoder_cvbs_encoder_disable(struct drm_encoder *encoder) } } -static void meson_encoder_cvbs_encoder_enable(struct drm_encoder *encoder) +static void meson_encoder_cvbs_enable(struct drm_bridge *bridge) { struct meson_encoder_cvbs *meson_encoder_cvbs = - encoder_to_meson_encoder_cvbs(encoder); + bridge_to_meson_encoder_cvbs(bridge); struct meson_drm *priv = meson_encoder_cvbs->priv; /* VDAC0 source is not from ATV */ @@ -198,13 +176,13 @@ static void meson_encoder_cvbs_encoder_enable(struct drm_encoder *encoder) } } -static void meson_encoder_cvbs_encoder_mode_set(struct drm_encoder *encoder, - struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode) +static void meson_encoder_cvbs_mode_set(struct drm_bridge *bridge, + const struct drm_display_mode *mode, + const struct drm_display_mode *adjusted_mode) { const struct meson_cvbs_mode *meson_mode = meson_cvbs_get_mode(mode); struct meson_encoder_cvbs *meson_encoder_cvbs = - encoder_to_meson_encoder_cvbs(encoder); + bridge_to_meson_encoder_cvbs(bridge); struct meson_drm *priv = meson_encoder_cvbs->priv; if (meson_mode) { @@ -218,76 +196,78 @@ static void meson_encoder_cvbs_encoder_mode_set(struct drm_encoder *encoder, } } -static const struct drm_encoder_helper_funcs - meson_encoder_cvbs_encoder_helper_funcs = { - .atomic_check = meson_encoder_cvbs_encoder_atomic_check, - .disable = meson_encoder_cvbs_encoder_disable, - .enable = meson_encoder_cvbs_encoder_enable, - .mode_set = meson_encoder_cvbs_encoder_mode_set, +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, + .mode_set = meson_encoder_cvbs_mode_set, + .get_modes = meson_encoder_cvbs_get_modes, + .atomic_check = meson_encoder_cvbs_atomic_check, }; -static bool meson_encoder_cvbs_connector_is_available(struct meson_drm *priv) -{ - struct device_node *remote; - - remote = of_graph_get_remote_node(priv->dev->of_node, 0, 0); - if (!remote) - return false; - - of_node_put(remote); - return true; -} - int meson_encoder_cvbs_init(struct meson_drm *priv) { struct drm_device *drm = priv->drm; struct meson_encoder_cvbs *meson_encoder_cvbs; - struct drm_connector *connector; - struct drm_encoder *encoder; + struct device_node *remote; int ret; - if (!meson_encoder_cvbs_connector_is_available(priv)) { + meson_encoder_cvbs = devm_kzalloc(priv->dev, sizeof(*meson_encoder_cvbs), GFP_KERNEL); + if (!meson_encoder_cvbs) + return -ENOMEM; + + /* CVBS Connector Bridge */ + remote = of_graph_get_remote_node(priv->dev->of_node, 0, 0); + if (!remote) { dev_info(drm->dev, "CVBS Output connector not available\n"); return 0; } - meson_encoder_cvbs = devm_kzalloc(priv->dev, sizeof(*meson_encoder_cvbs), - GFP_KERNEL); - if (!meson_encoder_cvbs) - return -ENOMEM; + meson_encoder_cvbs->next_bridge = of_drm_find_bridge(remote); + if (!meson_encoder_cvbs->next_bridge) { + dev_err(priv->dev, "Failed to find CVBS Connector bridge\n"); + return -EPROBE_DEFER; + } - meson_encoder_cvbs->priv = priv; - encoder = &meson_encoder_cvbs->encoder; - connector = &meson_encoder_cvbs->connector; + /* CVBS Encoder Bridge */ + meson_encoder_cvbs->bridge.funcs = &meson_encoder_cvbs_bridge_funcs; + meson_encoder_cvbs->bridge.of_node = priv->dev->of_node; + meson_encoder_cvbs->bridge.type = DRM_MODE_CONNECTOR_Composite; + meson_encoder_cvbs->bridge.ops = DRM_BRIDGE_OP_MODES; + meson_encoder_cvbs->bridge.interlace_allowed = true; - /* Connector */ + drm_bridge_add(&meson_encoder_cvbs->bridge); - drm_connector_helper_add(connector, - &meson_cvbs_connector_helper_funcs); + meson_encoder_cvbs->priv = priv; - ret = drm_connector_init(drm, connector, &meson_cvbs_connector_funcs, - DRM_MODE_CONNECTOR_Composite); + /* Encoder */ + ret = drm_simple_encoder_init(priv->drm, &meson_encoder_cvbs->encoder, + DRM_MODE_ENCODER_TVDAC); if (ret) { - dev_err(priv->dev, "Failed to init CVBS connector\n"); + dev_err(priv->dev, "Failed to init CVBS encoder: %d\n", ret); return ret; } - connector->interlace_allowed = 1; + meson_encoder_cvbs->encoder.possible_crtcs = BIT(0); - /* Encoder */ - - drm_encoder_helper_add(encoder, &meson_encoder_cvbs_encoder_helper_funcs); - - ret = drm_encoder_init(drm, encoder, &meson_encoder_cvbs_encoder_funcs, - DRM_MODE_ENCODER_TVDAC, "meson_encoder_cvbs"); + /* Attach CVBS Encoder Bridge to Encoder */ + ret = drm_bridge_attach(&meson_encoder_cvbs->encoder, &meson_encoder_cvbs->bridge, NULL, + DRM_BRIDGE_ATTACH_NO_CONNECTOR); if (ret) { - dev_err(priv->dev, "Failed to init CVBS encoder\n"); + dev_err(priv->dev, "Failed to attach bridge: %d\n", ret); return ret; } - encoder->possible_crtcs = BIT(0); - - drm_connector_attach_encoder(connector, encoder); + /* Initialize & attach Bridge Connector */ + meson_encoder_cvbs->connector = drm_bridge_connector_init(priv->drm, + &meson_encoder_cvbs->encoder); + if (IS_ERR(meson_encoder_cvbs->connector)) { + dev_err(priv->dev, "Unable to create CVBS bridge connector\n"); + return PTR_ERR(meson_encoder_cvbs->connector); + } + drm_connector_attach_encoder(meson_encoder_cvbs->connector, + &meson_encoder_cvbs->encoder); return 0; }
Drop the local connector and move all callback to bridge funcs in order to leverage the generic CVBS diplay 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 <narmstrong@baylibre.com> --- drivers/gpu/drm/meson/meson_encoder_cvbs.c | 178 +++++++++------------ 1 file changed, 79 insertions(+), 99 deletions(-)