On Wed, 2019-09-11 at 21:10 +0300, Ville Syrjälä wrote: > On Wed, Sep 11, 2019 at 10:56:02AM -0700, José Roberto de Souza > wrote: > > This 3 non-atomic drivers all have the same function getting the > > only encoder available in the connector, also atomic drivers have > > this fallback. So moving it a common place and sharing between > > atomic > > and non-atomic drivers. > > > > While at it I also removed the mention of > > drm_atomic_helper_best_encoder() that was renamed in > > commit 297e30b5d9b6 ("drm/atomic-helper: Unexport > > drm_atomic_helper_best_encoder"). > > > > Suggested-by: Ville Syrjälä <ville.syrj...@linux.intel.com> > > Cc: Ville Syrjälä <ville.syrj...@linux.intel.com> > > Cc: Daniel Vetter <daniel.vet...@ffwll.ch> > > Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com> > > Cc: dri-de...@lists.freedesktop.org > > Cc: intel-gfx@lists.freedesktop.org > > Signed-off-by: José Roberto de Souza <jose.so...@intel.com> > > --- > > drivers/gpu/drm/ast/ast_mode.c | 12 ------------ > > drivers/gpu/drm/drm_atomic_helper.c | 15 ++------------- > > drivers/gpu/drm/drm_connector.c | 11 +++++++++++ > > drivers/gpu/drm/drm_crtc_helper.c | 8 +++++++- > > drivers/gpu/drm/drm_crtc_internal.h | 2 ++ > > drivers/gpu/drm/mgag200/mgag200_mode.c | 11 ----------- > > drivers/gpu/drm/udl/udl_connector.c | 8 -------- > > include/drm/drm_modeset_helper_vtables.h | 6 +++--- > > 8 files changed, 25 insertions(+), 48 deletions(-) > > > > diff --git a/drivers/gpu/drm/ast/ast_mode.c > > b/drivers/gpu/drm/ast/ast_mode.c > > index d349c721501c..eef95e1af06b 100644 > > --- a/drivers/gpu/drm/ast/ast_mode.c > > +++ b/drivers/gpu/drm/ast/ast_mode.c > > @@ -687,17 +687,6 @@ static void ast_encoder_destroy(struct > > drm_encoder *encoder) > > kfree(encoder); > > } > > > > - > > -static struct drm_encoder *ast_best_single_encoder(struct > > drm_connector *connector) > > -{ > > - int enc_id = connector->encoder_ids[0]; > > - /* pick the encoder ids */ > > - if (enc_id) > > - return drm_encoder_find(connector->dev, NULL, enc_id); > > - return NULL; > > -} > > - > > - > > static const struct drm_encoder_funcs ast_enc_funcs = { > > .destroy = ast_encoder_destroy, > > }; > > @@ -847,7 +836,6 @@ static void ast_connector_destroy(struct > > drm_connector *connector) > > static const struct drm_connector_helper_funcs > > ast_connector_helper_funcs = { > > .mode_valid = ast_mode_valid, > > .get_modes = ast_get_modes, > > - .best_encoder = ast_best_single_encoder, > > }; > > > > static const struct drm_connector_funcs ast_connector_funcs = { > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > b/drivers/gpu/drm/drm_atomic_helper.c > > index 4706439fb490..9d7e4da6c292 100644 > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > @@ -97,17 +97,6 @@ drm_atomic_helper_plane_changed(struct > > drm_atomic_state *state, > > } > > } > > > > -/* > > - * For connectors that support multiple encoders, either the > > - * .atomic_best_encoder() or .best_encoder() operation must be > > implemented. > > - */ > > -static struct drm_encoder * > > -pick_single_encoder_for_connector(struct drm_connector *connector) > > -{ > > - WARN_ON(connector->encoder_ids[1]); > > - return drm_encoder_find(connector->dev, NULL, connector- > > >encoder_ids[0]); > > -} > > - > > static int handle_conflicting_encoders(struct drm_atomic_state > > *state, > > bool > > disable_conflicting_encoders) > > { > > @@ -135,7 +124,7 @@ static int handle_conflicting_encoders(struct > > drm_atomic_state *state, > > else if (funcs->best_encoder) > > new_encoder = funcs->best_encoder(connector); > > else > > - new_encoder = > > pick_single_encoder_for_connector(connector); > > + new_encoder = > > drm_connector_get_single_encoder(connector); > > > > if (new_encoder) { > > if (encoder_mask & > > drm_encoder_mask(new_encoder)) { > > @@ -359,7 +348,7 @@ update_connector_routing(struct > > drm_atomic_state *state, > > else if (funcs->best_encoder) > > new_encoder = funcs->best_encoder(connector); > > else > > - new_encoder = > > pick_single_encoder_for_connector(connector); > > + new_encoder = > > drm_connector_get_single_encoder(connector); > > > > if (!new_encoder) { > > DRM_DEBUG_ATOMIC("No suitable encoder found for > > [CONNECTOR:%d:%s]\n", > > diff --git a/drivers/gpu/drm/drm_connector.c > > b/drivers/gpu/drm/drm_connector.c > > index 4c766624b20d..3e2a632cf861 100644 > > --- a/drivers/gpu/drm/drm_connector.c > > +++ b/drivers/gpu/drm/drm_connector.c > > @@ -2334,3 +2334,14 @@ struct drm_tile_group > > *drm_mode_create_tile_group(struct drm_device *dev, > > return tg; > > } > > EXPORT_SYMBOL(drm_mode_create_tile_group); > > + > > +/* > > + * For connectors that support multiple encoders, either the > > + * .atomic_best_encoder() or .best_encoder() operation must be > > implemented. > > + */ > > +struct drm_encoder * > > +drm_connector_get_single_encoder(struct drm_connector *connector) > > +{ > > + WARN_ON(connector->encoder_ids[1]); > > + return drm_encoder_find(connector->dev, NULL, connector- > > >encoder_ids[0]); > > +} > > I believe we need EXPORT_SYMBOL.
I don't think we should allow drivers to call this function. > > > diff --git a/drivers/gpu/drm/drm_crtc_helper.c > > b/drivers/gpu/drm/drm_crtc_helper.c > > index a51824a7e7c1..a1f3c388e398 100644 > > --- a/drivers/gpu/drm/drm_crtc_helper.c > > +++ b/drivers/gpu/drm/drm_crtc_helper.c > > @@ -48,6 +48,8 @@ > > #include <drm/drm_print.h> > > #include <drm/drm_vblank.h> > > > > +#include "drm_crtc_internal.h" > > + > > /** > > * DOC: overview > > * > > @@ -625,7 +627,11 @@ int drm_crtc_helper_set_config(struct > > drm_mode_set *set, > > new_encoder = connector->encoder; > > for (ro = 0; ro < set->num_connectors; ro++) { > > if (set->connectors[ro] == connector) { > > - new_encoder = connector_funcs- > > >best_encoder(connector); > > + if (connector_funcs->best_encoder) > > + new_encoder = connector_funcs- > > >best_encoder(connector); > > + else > > + new_encoder = > > drm_connector_get_single_encoder(connector); > > + > > /* if we can't get an encoder for a > > connector > > we are setting now - then fail */ > > if (new_encoder == NULL) > > diff --git a/drivers/gpu/drm/drm_crtc_internal.h > > b/drivers/gpu/drm/drm_crtc_internal.h > > index c7d5e4c21423..80ade1fa66e5 100644 > > --- a/drivers/gpu/drm/drm_crtc_internal.h > > +++ b/drivers/gpu/drm/drm_crtc_internal.h > > @@ -181,6 +181,8 @@ int drm_connector_set_obj_prop(struct > > drm_mode_object *obj, > > int drm_connector_create_standard_properties(struct drm_device > > *dev); > > const char *drm_get_connector_force_name(enum drm_connector_force > > force); > > void drm_connector_free_work_fn(struct work_struct *work); > > +struct drm_encoder * > > +drm_connector_get_single_encoder(struct drm_connector *connector); > > > > /* IOCTL */ > > int drm_connector_property_set_ioctl(struct drm_device *dev, > > diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c > > b/drivers/gpu/drm/mgag200/mgag200_mode.c > > index 5e778b5f1a10..68226556044b 100644 > > --- a/drivers/gpu/drm/mgag200/mgag200_mode.c > > +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c > > @@ -1638,16 +1638,6 @@ static enum drm_mode_status > > mga_vga_mode_valid(struct drm_connector *connector, > > return MODE_OK; > > } > > > > -static struct drm_encoder *mga_connector_best_encoder(struct > > drm_connector > > - *connector) > > -{ > > - int enc_id = connector->encoder_ids[0]; > > - /* pick the encoder ids */ > > - if (enc_id) > > - return drm_encoder_find(connector->dev, NULL, enc_id); > > - return NULL; > > -} > > - > > static void mga_connector_destroy(struct drm_connector *connector) > > { > > struct mga_connector *mga_connector = > > to_mga_connector(connector); > > @@ -1659,7 +1649,6 @@ static void mga_connector_destroy(struct > > drm_connector *connector) > > static const struct drm_connector_helper_funcs > > mga_vga_connector_helper_funcs = { > > .get_modes = mga_vga_get_modes, > > .mode_valid = mga_vga_mode_valid, > > - .best_encoder = mga_connector_best_encoder, > > }; > > > > static const struct drm_connector_funcs mga_vga_connector_funcs = > > { > > diff --git a/drivers/gpu/drm/udl/udl_connector.c > > b/drivers/gpu/drm/udl/udl_connector.c > > index ddb61a60c610..b4ae3e89a7b4 100644 > > --- a/drivers/gpu/drm/udl/udl_connector.c > > +++ b/drivers/gpu/drm/udl/udl_connector.c > > @@ -90,13 +90,6 @@ udl_detect(struct drm_connector *connector, bool > > force) > > return connector_status_connected; > > } > > > > -static struct drm_encoder* > > -udl_best_single_encoder(struct drm_connector *connector) > > -{ > > - int enc_id = connector->encoder_ids[0]; > > - return drm_encoder_find(connector->dev, NULL, enc_id); > > -} > > - > > static int udl_connector_set_property(struct drm_connector > > *connector, > > struct drm_property *property, > > uint64_t val) > > @@ -120,7 +113,6 @@ static void udl_connector_destroy(struct > > drm_connector *connector) > > static const struct drm_connector_helper_funcs > > udl_connector_helper_funcs = { > > .get_modes = udl_get_modes, > > .mode_valid = udl_mode_valid, > > - .best_encoder = udl_best_single_encoder, > > }; > > > > static const struct drm_connector_funcs udl_connector_funcs = { > > diff --git a/include/drm/drm_modeset_helper_vtables.h > > b/include/drm/drm_modeset_helper_vtables.h > > index 6b18c8adfe9d..b55412c6ce3a 100644 > > --- a/include/drm/drm_modeset_helper_vtables.h > > +++ b/include/drm/drm_modeset_helper_vtables.h > > @@ -955,8 +955,8 @@ struct drm_connector_helper_funcs { > > * @atomic_best_encoder. > > * > > * You can leave this function to NULL if the connector is only > > - * attached to a single encoder and you are using the atomic > > helpers. > > - * In this case, the core will call > > drm_atomic_helper_best_encoder() > > + * attached to a single encoder. > > + * In this case, the core will call > > drm_connector_get_single_encoder() > > * for you. > > * > > * RETURNS: > > @@ -977,7 +977,7 @@ struct drm_connector_helper_funcs { > > * > > * This function is used by drm_atomic_helper_check_modeset(). > > * If it is not implemented, the core will fallback to > > @best_encoder > > - * (or drm_atomic_helper_best_encoder() if @best_encoder is > > NULL). > > + * (or drm_connector_get_single_encoder() if @best_encoder is > > NULL). > > * > > * NOTE: > > * > > -- > > 2.23.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx