Re: [PATCH v4 6/9] drm/i915: Convert intel_dp properties to atomic.

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Apr 12, 2017 at 12:50:04PM +0200, Maarten Lankhorst wrote:
> intel_dp supports 3 properties, scaling mode, broadcast rgb and
> force_audio. intel_digital_connector handles the plumbing,
> so we only have to hook this up in compute_config and init.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 136 +++++++--------------------------------
>  drivers/gpu/drm/i915/intel_drv.h |   3 -
>  2 files changed, 24 insertions(+), 115 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b1a0cb3c79d4..f976d10b4f0a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1630,6 +1630,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	enum port port = dp_to_dig_port(intel_dp)->port;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
>  	struct intel_connector *intel_connector = intel_dp->attached_connector;
> +	struct intel_digital_connector_state *intel_conn_state =
> +		to_intel_digital_connector_state(conn_state);
>  	int lane_count, clock;
>  	int min_lane_count = 1;
>  	int max_lane_count = intel_dp_max_lane_count(intel_dp);
> @@ -1655,10 +1657,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	pipe_config->has_drrs = false;
>  	if (port == PORT_A)
>  		pipe_config->has_audio = false;
> -	else if (intel_dp->force_audio == HDMI_AUDIO_AUTO)
> +	else if (intel_conn_state->force_audio == HDMI_AUDIO_AUTO)
>  		pipe_config->has_audio = intel_dp->has_audio;
>  	else
> -		pipe_config->has_audio = intel_dp->force_audio == HDMI_AUDIO_ON;
> +		pipe_config->has_audio = intel_conn_state->force_audio == HDMI_AUDIO_ON;
>  
>  	if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
>  		intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
> @@ -1673,10 +1675,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  
>  		if (HAS_GMCH_DISPLAY(dev_priv))
>  			intel_gmch_panel_fitting(intel_crtc, pipe_config,
> -						 intel_connector->panel.fitting_mode);
> +						 conn_state->scaling_mode);
>  		else
>  			intel_pch_panel_fitting(intel_crtc, pipe_config,
> -						intel_connector->panel.fitting_mode);
> +						conn_state->scaling_mode);
>  	}
>  
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
> @@ -1745,7 +1747,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	return false;
>  
>  found:
> -	if (intel_dp->color_range_auto) {
> +	if (intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_AUTO) {
>  		/*
>  		 * See:
>  		 * CEA-861-E - 5.1 Default Encoding Parameters
> @@ -1757,7 +1759,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  			HDMI_QUANTIZATION_RANGE_LIMITED;
>  	} else {
>  		pipe_config->limited_color_range =
> -			intel_dp->limited_color_range;
> +			intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_LIMITED;
>  	}
>  
>  	pipe_config->lane_count = lane_count;
> @@ -4781,104 +4783,6 @@ static int intel_dp_get_modes(struct drm_connector *connector)
>  }
>  
>  static int
> -intel_dp_set_property(struct drm_connector *connector,
> -		      struct drm_property *property,
> -		      uint64_t val)
> -{
> -	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> -	struct intel_connector *intel_connector = to_intel_connector(connector);
> -	struct intel_encoder *intel_encoder = intel_attached_encoder(connector);
> -	struct intel_dp *intel_dp = enc_to_intel_dp(&intel_encoder->base);
> -	int ret;
> -
> -	ret = drm_object_property_set_value(&connector->base, property, val);
> -	if (ret)
> -		return ret;
> -
> -	if (property == dev_priv->force_audio_property) {
> -		int i = val;
> -		bool has_audio, old_has_audio;
> -		int old_force_audio = intel_dp->force_audio;
> -
> -		if (i == intel_dp->force_audio)
> -			return 0;
> -
> -		if (old_force_audio == HDMI_AUDIO_AUTO)
> -			old_has_audio = intel_dp->has_audio;
> -		else
> -			old_has_audio = old_force_audio;
> -
> -		intel_dp->force_audio = i;
> -
> -		if (i == HDMI_AUDIO_AUTO)
> -			has_audio = intel_dp->has_audio;
> -		else
> -			has_audio = (i == HDMI_AUDIO_ON);
> -
> -		if (has_audio == old_has_audio)
> -			return 0;
> -
> -		goto done;
> -	}
> -
> -	if (property == dev_priv->broadcast_rgb_property) {
> -		bool old_auto = intel_dp->color_range_auto;
> -		bool old_range = intel_dp->limited_color_range;
> -
> -		switch (val) {
> -		case INTEL_BROADCAST_RGB_AUTO:
> -			intel_dp->color_range_auto = true;
> -			break;
> -		case INTEL_BROADCAST_RGB_FULL:
> -			intel_dp->color_range_auto = false;
> -			intel_dp->limited_color_range = false;
> -			break;
> -		case INTEL_BROADCAST_RGB_LIMITED:
> -			intel_dp->color_range_auto = false;
> -			intel_dp->limited_color_range = true;
> -			break;
> -		default:
> -			return -EINVAL;
> -		}
> -
> -		if (old_auto == intel_dp->color_range_auto &&
> -		    old_range == intel_dp->limited_color_range)
> -			return 0;
> -
> -		goto done;
> -	}
> -
> -	if (is_edp(intel_dp) &&
> -	    property == connector->dev->mode_config.scaling_mode_property) {
> -		if (val == DRM_MODE_SCALE_NONE) {
> -			DRM_DEBUG_KMS("no scaling not supported\n");
> -			return -EINVAL;
> -		}
> -		if (HAS_GMCH_DISPLAY(dev_priv) &&
> -		    val == DRM_MODE_SCALE_CENTER) {
> -			DRM_DEBUG_KMS("centering not supported\n");
> -			return -EINVAL;
> -		}
> -
> -		if (intel_connector->panel.fitting_mode == val) {
> -			/* the eDP scaling property is not changed */
> -			return 0;
> -		}
> -		intel_connector->panel.fitting_mode = val;
> -
> -		goto done;
> -	}
> -
> -	return -EINVAL;
> -
> -done:
> -	if (intel_encoder->base.crtc)
> -		intel_crtc_restore_mode(intel_encoder->base.crtc);
> -
> -	return 0;
> -}
> -
> -static int
>  intel_dp_connector_register(struct drm_connector *connector)
>  {
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
> @@ -5036,19 +4940,21 @@ static const struct drm_connector_funcs intel_dp_connector_funcs = {
>  	.dpms = drm_atomic_helper_connector_dpms,
>  	.force = intel_dp_force,
>  	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.set_property = intel_dp_set_property,
> -	.atomic_get_property = intel_connector_atomic_get_property,
> +	.set_property = drm_atomic_helper_connector_set_property,
> +	.atomic_get_property = intel_digital_connector_atomic_get_property,
> +	.atomic_set_property = intel_digital_connector_atomic_set_property,
>  	.late_register = intel_dp_connector_register,
>  	.early_unregister = intel_dp_connector_unregister,
>  	.destroy = intel_dp_connector_destroy,
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> -	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> +	.atomic_duplicate_state = intel_digital_connector_duplicate_state,
>  };
>  
>  static const struct drm_connector_helper_funcs intel_dp_connector_helper_funcs = {
>  	.detect_ctx = intel_dp_detect,
>  	.get_modes = intel_dp_get_modes,
>  	.mode_valid = intel_dp_mode_valid,
> +	.atomic_check = intel_digital_connector_atomic_check,
>  };
>  
>  static const struct drm_encoder_funcs intel_dp_enc_funcs = {
> @@ -5142,11 +5048,8 @@ bool intel_dp_is_edp(struct drm_i915_private *dev_priv, enum port port)
>  static void
>  intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connector)
>  {
> -	struct intel_connector *intel_connector = to_intel_connector(connector);
> -
>  	intel_attach_force_audio_property(connector);
>  	intel_attach_broadcast_rgb_property(connector);
> -	intel_dp->color_range_auto = true;
>  
>  	if (is_edp(intel_dp)) {
>  		drm_mode_create_scaling_mode_property(connector->dev);
> @@ -5154,7 +5057,8 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
>  			&connector->base,
>  			connector->dev->mode_config.scaling_mode_property,
>  			DRM_MODE_SCALE_ASPECT);
> -		intel_connector->panel.fitting_mode = DRM_MODE_SCALE_ASPECT;
> +
> +		connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT;
>  	}
>  }
>  
> @@ -5797,6 +5701,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  	struct drm_display_mode *scan;
>  	struct edid *edid;
>  	enum pipe pipe = INVALID_PIPE;
> +	unsigned allowed_fittings;
>  
>  	if (!is_edp(intel_dp))
>  		return true;
> @@ -5890,7 +5795,14 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  			      pipe_name(pipe));
>  	}
>  
> -	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode, 0);
> +	allowed_fittings = 0;
> +	if (!HAS_GMCH_DISPLAY(dev_priv))
> +		allowed_fittings |= BIT(DRM_MODE_SCALE_CENTER);
> +	allowed_fittings |= BIT(DRM_MODE_SCALE_ASPECT);
> +	allowed_fittings |= BIT(DRM_MODE_SCALE_FULLSCREEN);
> +
> +	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode,
> +			 allowed_fittings);
>  	intel_connector->panel.backlight.power = intel_edp_backlight_power;
>  	intel_panel_setup_backlight(connector, pipe);
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4df9e4531edc..9f0f80203472 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -261,7 +261,6 @@ struct intel_encoder {
>  struct intel_panel {
>  	struct drm_display_mode *fixed_mode;
>  	struct drm_display_mode *downclock_mode;
> -	int fitting_mode;
>  	unsigned allowed_fitting_mode_mask;
>  
>  	/* backlight */
> @@ -952,9 +951,7 @@ struct intel_dp {
>  	bool detect_done;
>  	bool channel_eq_status;
>  	bool reset_link_params;
> -	enum hdmi_force_audio force_audio;
>  	bool limited_color_range;

Can't we remove limited_color_range now too? Otherwise lgtm.
-Daniel


> -	bool color_range_auto;
>  	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
>  	uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
>  	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
> -- 
> 2.7.4
> 
> _______________________________________________
> 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