Re: [PATCH 1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.syrjala@xxxxxxxxxxxxxxx>
> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> > Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> > ---
> >  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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux