On Mon, May 01, 2017 at 03:37:57PM +0200, Maarten Lankhorst wrote:
> Some atomic properties are common between the various kinds of
> connectors, for example a lot of them use panel fitting mode.
> It makes sense to put a lot of it in a common place, so each
> connector can use it while they're being converted.
> 
> Implement the properties required for the connectors:
> - scaling mode property
> - force audio property
> - broadcast rgb
> - aspect ratio
> 
> While at it, make clear that intel_digital_connector_atomic_get_property
> is a hack that has to be removed when all connector properties
> are converted to atomic.
> 
> Changes since v1:
> - Scaling mode and aspect ratio are partly handled in core now.
> Changes since v2:
> - Split out the scaling mode / aspect ratio changes to a preparation
>   patch.
> - Use mode_changed for panel fitter, changes to this property
>   are checked by fastset.
> - Allowed_scaling_modes is removed, handled through core now.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>

Patches 3-5: Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>

Pls ping Dave for an Ack for getting the first 2 landed through drm-intel
(they'll still show up in the first pull request right after -rc1, so all
good imo).

Cheers, Daniel

> ---
>  drivers/gpu/drm/i915/intel_atomic.c  | 131 
> +++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_display.c |  14 +++-
>  drivers/gpu/drm/i915/intel_drv.h     |  23 ++++++
>  3 files changed, 159 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
> b/drivers/gpu/drm/i915/intel_atomic.c
> index 50fb1f76cc5f..182909f266f5 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -36,7 +36,7 @@
>  #include "intel_drv.h"
>  
>  /**
> - * intel_connector_atomic_get_property - fetch connector property value
> + * intel_connector_atomic_get_property - fetch legacy connector property 
> value
>   * @connector: connector to fetch property for
>   * @state: state containing the property value
>   * @property: property to look up
> @@ -45,12 +45,14 @@
>   * The DRM core does not store shadow copies of properties for
>   * atomic-capable drivers.  This entrypoint is used to fetch
>   * the current value of a driver-specific connector property.
> + *
> + * This is a intermediary solution until all connectors are
> + * converted to support full atomic properties.
>   */
> -int
> -intel_connector_atomic_get_property(struct drm_connector *connector,
> -                                 const struct drm_connector_state *state,
> -                                 struct drm_property *property,
> -                                 uint64_t *val)
> +int intel_connector_atomic_get_property(struct drm_connector *connector,
> +                                     const struct drm_connector_state *state,
> +                                     struct drm_property *property,
> +                                     uint64_t *val)
>  {
>       int i;
>  
> @@ -73,7 +75,122 @@ intel_connector_atomic_get_property(struct drm_connector 
> *connector,
>       return -EINVAL;
>  }
>  
> -/*
> +/**
> + * intel_digital_connector_atomic_get_property - hook for 
> connector->atomic_get_property.
> + * @connector: Connector to get the property for.
> + * @state: Connector state to retrieve the property from.
> + * @property: Property to retrieve.
> + * @val: Return value for the property.
> + *
> + * Returns the atomic property value for a digital connector.
> + */
> +int intel_digital_connector_atomic_get_property(struct drm_connector 
> *connector,
> +                                             const struct 
> drm_connector_state *state,
> +                                             struct drm_property *property,
> +                                             uint64_t *val)
> +{
> +     struct drm_device *dev = connector->dev;
> +     struct drm_i915_private *dev_priv = to_i915(dev);
> +     struct intel_digital_connector_state *intel_conn_state =
> +             to_intel_digital_connector_state(state);
> +
> +     if (property == dev_priv->force_audio_property)
> +             *val = intel_conn_state->force_audio;
> +     else if (property == dev_priv->broadcast_rgb_property)
> +             *val = intel_conn_state->broadcast_rgb;
> +     else {
> +             DRM_DEBUG_ATOMIC("Unknown property %s\n", property->name);
> +             return -EINVAL;
> +     }
> +
> +     return 0;
> +}
> +
> +/**
> + * intel_digital_connector_atomic_set_property - hook for 
> connector->atomic_set_property.
> + * @connector: Connector to set the property for.
> + * @state: Connector state to set the property on.
> + * @property: Property to set.
> + * @val: New value for the property.
> + *
> + * Sets the atomic property value for a digital connector.
> + */
> +int intel_digital_connector_atomic_set_property(struct drm_connector 
> *connector,
> +                                             struct drm_connector_state 
> *state,
> +                                             struct drm_property *property,
> +                                             uint64_t val)
> +{
> +     struct drm_device *dev = connector->dev;
> +     struct drm_i915_private *dev_priv = to_i915(dev);
> +     struct intel_digital_connector_state *intel_conn_state =
> +             to_intel_digital_connector_state(state);
> +
> +     if (property == dev_priv->force_audio_property) {
> +             intel_conn_state->force_audio = val;
> +             return 0;
> +     }
> +
> +     if (property == dev_priv->broadcast_rgb_property) {
> +             intel_conn_state->broadcast_rgb = val;
> +             return 0;
> +     }
> +
> +     DRM_DEBUG_ATOMIC("Unknown property %s\n", property->name);
> +     return -EINVAL;
> +}
> +
> +int intel_digital_connector_atomic_check(struct drm_connector *conn,
> +                                      struct drm_connector_state *new_state)
> +{
> +     struct intel_digital_connector_state *new_conn_state =
> +             to_intel_digital_connector_state(new_state);
> +     struct drm_connector_state *old_state =
> +             drm_atomic_get_old_connector_state(new_state->state, conn);
> +     struct intel_digital_connector_state *old_conn_state =
> +             to_intel_digital_connector_state(old_state);
> +     struct drm_crtc_state *crtc_state;
> +
> +     if (!new_state->crtc)
> +             return 0;
> +
> +     crtc_state = drm_atomic_get_new_crtc_state(new_state->state, 
> new_state->crtc);
> +
> +     /*
> +      * These properties are handled by fastset, and might not end
> +      * up in a modeset.
> +      */
> +     if (new_conn_state->force_audio != old_conn_state->force_audio ||
> +         new_conn_state->broadcast_rgb != old_conn_state->broadcast_rgb ||
> +         new_conn_state->base.picture_aspect_ratio != 
> old_conn_state->base.picture_aspect_ratio ||
> +         new_conn_state->base.scaling_mode != 
> old_conn_state->base.scaling_mode)
> +             crtc_state->mode_changed = true;
> +
> +     return 0;
> +}
> +
> +/**
> + * intel_digital_connector_duplicate_state - duplicate connector state
> + * @connector: digital connector
> + *
> + * Allocates and returns a copy of the connector state (both common and
> + * digital connector specific) for the specified connector.
> + *
> + * Returns: The newly allocated connector state, or NULL on failure.
> + */
> +struct drm_connector_state *
> +intel_digital_connector_duplicate_state(struct drm_connector *connector)
> +{
> +     struct intel_digital_connector_state *state;
> +
> +     state = kmemdup(connector->state, sizeof(*state), GFP_KERNEL);
> +     if (!state)
> +             return NULL;
> +
> +     __drm_atomic_helper_connector_duplicate_state(connector, &state->base);
> +     return &state->base;
> +}
> +
> +/**
>   * intel_crtc_duplicate_state - duplicate crtc state
>   * @crtc: drm crtc
>   *
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 85b9e2f521a0..108122daa807 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5956,11 +5956,21 @@ static void intel_connector_verify_state(struct 
> intel_connector *connector)
>  
>  int intel_connector_init(struct intel_connector *connector)
>  {
> -     drm_atomic_helper_connector_reset(&connector->base);
> +     struct intel_digital_connector_state *conn_state;
>  
> -     if (!connector->base.state)
> +     /*
> +      * Allocate enough memory to hold intel_digital_connector_state,
> +      * This might be a few bytes too many, but for connectors that don't
> +      * need it we'll free the state and allocate a smaller one on the first
> +      * succesful commit anyway.
> +      */
> +     conn_state = kzalloc(sizeof(*conn_state), GFP_KERNEL);
> +     if (!conn_state)
>               return -ENOMEM;
>  
> +     __drm_atomic_helper_connector_reset(&connector->base,
> +                                         &conn_state->base);
> +
>       return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index 2b243444cf8b..4e63f397c4a2 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -330,6 +330,15 @@ struct intel_connector {
>       struct work_struct modeset_retry_work;
>  };
>  
> +struct intel_digital_connector_state {
> +     struct drm_connector_state base;
> +
> +     enum hdmi_force_audio force_audio;
> +     int broadcast_rgb;
> +};
> +
> +#define to_intel_digital_connector_state(x) container_of(x, struct 
> intel_digital_connector_state, base)
> +
>  struct dpll {
>       /* given values */
>       int n;
> @@ -1885,6 +1894,20 @@ int intel_connector_atomic_get_property(struct 
> drm_connector *connector,
>                                       const struct drm_connector_state *state,
>                                       struct drm_property *property,
>                                       uint64_t *val);
> +
> +int intel_digital_connector_atomic_get_property(struct drm_connector 
> *connector,
> +                                             const struct 
> drm_connector_state *state,
> +                                             struct drm_property *property,
> +                                             uint64_t *val);
> +int intel_digital_connector_atomic_set_property(struct drm_connector 
> *connector,
> +                                             struct drm_connector_state 
> *state,
> +                                             struct drm_property *property,
> +                                             uint64_t val);
> +int intel_digital_connector_atomic_check(struct drm_connector *conn,
> +                                      struct drm_connector_state *new_state);
> +struct drm_connector_state *
> +intel_digital_connector_duplicate_state(struct drm_connector *connector);
> +
>  struct drm_crtc_state *intel_crtc_duplicate_state(struct drm_crtc *crtc);
>  void intel_crtc_destroy_state(struct drm_crtc *crtc,
>                              struct drm_crtc_state *state);
> -- 
> 2.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to