Re: [PATCH resend 1/2] drm/connector: Split out orientation quirk detection (v2)

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

 



On Mon, Dec 16, 2019 at 12:51:57PM +0100, Hans de Goede wrote:
> From: Derek Basehore <dbasehore@xxxxxxxxxxxx>
> 
> Not every platform needs quirk detection for panel orientation, so
> split the drm_connector_init_panel_orientation_property into two
> functions. One for platforms without the need for quirks, and the
> other for platforms that need quirks.
> 
> Hans de Goede (changes in v2):
> 
> Rename the function from drm_connector_init_panel_orientation_property
> to drm_connector_set_panel_orientation[_with_quirk] and pass in the
> panel-orientation to set.
> 
> Beside the rename, also make the function set the passed in value
> only once, if the value was set before (to a value other then
> DRM_MODE_PANEL_ORIENTATION_UNKNOWN) make any further set calls a no-op.
> 
> This change is preparation for allowing the user to override the
> panel-orientation for any connector from the kernel commandline.
> When the panel-orientation is overridden this way, then we must ignore
> the panel-orientation detection done by the driver.
> 
> Signed-off-by: Derek Basehore <dbasehore@xxxxxxxxxxxx>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
>  drivers/gpu/drm/drm_connector.c         | 74 ++++++++++++++++++-------
>  drivers/gpu/drm/i915/display/icl_dsi.c  |  5 +-
>  drivers/gpu/drm/i915/display/intel_dp.c |  9 ++-
>  drivers/gpu/drm/i915/display/vlv_dsi.c  |  5 +-
>  include/drm/drm_connector.h             |  9 ++-
>  5 files changed, 71 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 0965632008a9..f4fa5c59717d 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1139,7 +1139,8 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
>   *	coordinates, so if userspace rotates the picture to adjust for
>   *	the orientation it must also apply the same transformation to the
>   *	touchscreen input coordinates. This property is initialized by calling
> - *	drm_connector_init_panel_orientation_property().
> + *	drm_connector_set_panel_orientation() or
> + *	drm_connector_set_panel_orientation_with_quirk()

do we have a better name than quirks for these dsi modes?

>   *
>   * scaling mode:
>   *	This property defines how a non-native mode is upscaled to the native
> @@ -2046,38 +2047,41 @@ void drm_connector_set_vrr_capable_property(
>  EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
>  
>  /**
> - * drm_connector_init_panel_orientation_property -
> - *	initialize the connecters panel_orientation property
> - * @connector: connector for which to init the panel-orientation property.
> - * @width: width in pixels of the panel, used for panel quirk detection
> - * @height: height in pixels of the panel, used for panel quirk detection
> + * drm_connector_set_panel_orientation - sets the connecter's panel_orientation
> + * @connector: connector for which to set the panel-orientation property.
> + * @panel_orientation: drm_panel_orientation value to set
> + *
> + * This function sets the connector's panel_orientation and attaches
> + * a "panel orientation" property to the connector.
>   *
> - * This function should only be called for built-in panels, after setting
> - * connector->display_info.panel_orientation first (if known).
> + * Calling this function on a connector where the panel_orientation has
> + * already been set is a no-op (e.g. the orientation has been overridden with
> + * a kernel commandline option).
>   *
> - * This function will check for platform specific (e.g. DMI based) quirks
> - * overriding display_info.panel_orientation first, then if panel_orientation
> - * is not DRM_MODE_PANEL_ORIENTATION_UNKNOWN it will attach the
> - * "panel orientation" property to the connector.
> + * It is allowed to call this function with a panel_orientation of
> + * DRM_MODE_PANEL_ORIENTATION_UNKNOWN, in which case it is a no-op.
>   *
>   * Returns:
>   * Zero on success, negative errno on failure.
>   */
> -int drm_connector_init_panel_orientation_property(
> -	struct drm_connector *connector, int width, int height)
> +int drm_connector_set_panel_orientation(
> +	struct drm_connector *connector,
> +	enum drm_panel_orientation panel_orientation)
>  {
>  	struct drm_device *dev = connector->dev;
>  	struct drm_display_info *info = &connector->display_info;
>  	struct drm_property *prop;
> -	int orientation_quirk;
>  
> -	orientation_quirk = drm_get_panel_orientation_quirk(width, height);
> -	if (orientation_quirk != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> -		info->panel_orientation = orientation_quirk;
> +	/* Already set? */
> +	if (info->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> +		return 0;

What happens on the scenario of ICL DSI here?
In case we had something badly set we just respect the bad choices?
Any way to at least have some kind of warn when we tried the dsi mode but
it had already been set?

>  
> -	if (info->panel_orientation == DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> +	/* Don't attach the property if the orientation is unknown */
> +	if (panel_orientation == DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
>  		return 0;
>  
> +	info->panel_orientation = panel_orientation;
> +
>  	prop = dev->mode_config.panel_orientation_property;
>  	if (!prop) {
>  		prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
> @@ -2094,7 +2098,37 @@ int drm_connector_init_panel_orientation_property(
>  				   info->panel_orientation);
>  	return 0;
>  }
> -EXPORT_SYMBOL(drm_connector_init_panel_orientation_property);
> +EXPORT_SYMBOL(drm_connector_set_panel_orientation);
> +
> +/**
> + * drm_connector_set_panel_orientation_with_quirk -
> + *	set the connecter's panel_orientation after checking for quirks
> + * @connector: connector for which to init the panel-orientation property.
> + * @panel_orientation: drm_panel_orientation value to set
> + * @width: width in pixels of the panel, used for panel quirk detection
> + * @height: height in pixels of the panel, used for panel quirk detection
> + *
> + * Like drm_connector_set_panel_orientation(), but with a check for platform
> + * specific (e.g. DMI based) quirks overriding the passed in panel_orientation.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_connector_set_panel_orientation_with_quirk(
> +	struct drm_connector *connector,
> +	enum drm_panel_orientation panel_orientation,
> +	int width, int height)
> +{
> +	int orientation_quirk;
> +
> +	orientation_quirk = drm_get_panel_orientation_quirk(width, height);
> +	if (orientation_quirk != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> +		panel_orientation = orientation_quirk;
> +
> +	return drm_connector_set_panel_orientation(connector,
> +						   panel_orientation);
> +}
> +EXPORT_SYMBOL(drm_connector_set_panel_orientation_with_quirk);
>  
>  int drm_connector_set_obj_prop(struct drm_mode_object *obj,
>  				    struct drm_property *property,
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 6e398c33a524..8cd51cf67d02 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -1536,9 +1536,8 @@ static void icl_dsi_add_properties(struct intel_connector *connector)
>  
>  	connector->base.state->scaling_mode = DRM_MODE_SCALE_ASPECT;
>  
> -	connector->base.display_info.panel_orientation =
> -			intel_dsi_get_panel_orientation(connector);
> -	drm_connector_init_panel_orientation_property(&connector->base,
> +	drm_connector_set_panel_orientation_with_quirk(&connector->base,
> +				intel_dsi_get_panel_orientation(connector),
>  				connector->panel.fixed_mode->hdisplay,
>  				connector->panel.fixed_mode->vdisplay);
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index aa515261cb9f..9f4425f8d0ac 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -7340,9 +7340,12 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  	intel_connector->panel.backlight.power = intel_edp_backlight_power;
>  	intel_panel_setup_backlight(connector, pipe);
>  
> -	if (fixed_mode)
> -		drm_connector_init_panel_orientation_property(
> -			connector, fixed_mode->hdisplay, fixed_mode->vdisplay);
> +	if (fixed_mode) {
> +		/* We do not know the orientation, but their might be a quirk */
> +		drm_connector_set_panel_orientation_with_quirk(connector,
> +				DRM_MODE_PANEL_ORIENTATION_UNKNOWN,
> +				fixed_mode->hdisplay, fixed_mode->vdisplay);
> +	}
>  
>  	return true;
>  
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
> index 50064cde0724..a0de8e70e426 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> @@ -1632,10 +1632,9 @@ static void vlv_dsi_add_properties(struct intel_connector *connector)
>  
>  		connector->base.state->scaling_mode = DRM_MODE_SCALE_ASPECT;
>  
> -		connector->base.display_info.panel_orientation =
> -			vlv_dsi_get_panel_orientation(connector);
> -		drm_connector_init_panel_orientation_property(
> +		drm_connector_set_panel_orientation_with_quirk(
>  				&connector->base,
> +				vlv_dsi_get_panel_orientation(connector),
>  				connector->panel.fixed_mode->hdisplay,
>  				connector->panel.fixed_mode->vdisplay);
>  	}
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 221910948b37..2113500b4075 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1552,8 +1552,13 @@ void drm_connector_set_link_status_property(struct drm_connector *connector,
>  					    uint64_t link_status);
>  void drm_connector_set_vrr_capable_property(
>  		struct drm_connector *connector, bool capable);
> -int drm_connector_init_panel_orientation_property(
> -	struct drm_connector *connector, int width, int height);
> +int drm_connector_set_panel_orientation(
> +	struct drm_connector *connector,
> +	enum drm_panel_orientation panel_orientation);
> +int drm_connector_set_panel_orientation_with_quirk(
> +	struct drm_connector *connector,
> +	enum drm_panel_orientation panel_orientation,
> +	int width, int height);
>  int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
>  					  int min, int max);
>  
> -- 
> 2.23.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux