Re: [PATCH 2/4] drm/i915: Adding Panel Filter function for DP

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Aug 10, 2015 at 03:26:09PM +0800, Xiong Zhang wrote:
> Only internal eDP, LVDS, DVI screen could set scalling mode, some
> customers need to set scalling mode for external DP, HDMI, VGA
> screen. Let's fulfill this.
> 
> bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90989
> 
> Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 63 ++++++++++++++++++++++++++++-------------
>  1 file changed, 44 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index f1b9f93..2da334b 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -207,7 +207,13 @@ intel_dp_mode_valid(struct drm_connector *connector,
>  	int target_clock = mode->clock;
>  	int max_rate, mode_rate, max_lanes, max_link_clock;
>  
> -	if (is_edp(intel_dp) && fixed_mode) {
> +	if (mode->clock < 10000)
> +		return MODE_CLOCK_LOW;
> +
> +	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> +		return MODE_H_ILLEGAL;
> +
> +	if (!intel_panel_scale_none(&intel_connector->panel)) {
>  		if (mode->hdisplay > fixed_mode->hdisplay)
>  			return MODE_PANEL;
>  
> @@ -226,12 +232,6 @@ intel_dp_mode_valid(struct drm_connector *connector,
>  	if (mode_rate > max_rate)
>  		return MODE_CLOCK_HIGH;
>  
> -	if (mode->clock < 10000)
> -		return MODE_CLOCK_LOW;
> -
> -	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> -		return MODE_H_ILLEGAL;
> -
>  	return MODE_OK;
>  }
>  
> @@ -1378,7 +1378,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	pipe_config->has_drrs = false;
>  	pipe_config->has_audio = intel_dp->has_audio && port != PORT_A;
>  
> -	if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
> +	if (!intel_panel_scale_none(&intel_connector->panel)) {
>  		intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
>  				       adjusted_mode);
>  
> @@ -4592,6 +4592,23 @@ static int intel_dp_get_modes(struct drm_connector *connector)
>  	edid = intel_connector->detect_edid;
>  	if (edid) {
>  		int ret = intel_connector_update_modes(connector, edid);
> +
> +		if (ret && intel_connector->panel.fixed_mode == NULL) {
> +			/* init fixed mode as preferred mode for DP */
> +			struct drm_display_mode *fixed_mode = NULL;
> +			struct drm_display_mode *scan;
> +
> +			list_for_each_entry(scan, &connector->probed_modes, head) {
> +				if (scan->type & DRM_MODE_TYPE_PREFERRED)
> +					fixed_mode = drm_mode_duplicate(connector->dev,
> +									scan);
> +			}
> +
> +			if (fixed_mode)
> +				intel_panel_init(&intel_connector->panel,
> +						 fixed_mode, NULL);
> +		}

How are we supposed to get rid of a stale fixed_mode when some other
display gets plugged in?

Also what would happen if the preferred mode can't be supported due to
some source limitation?

In general I'm not entirely happy with having this kind of policy in
the kernel. I'd much prefer if we could get crtc size and border properties
done so that userspace could set up the scaling any which way it chooses.

> +
>  		if (ret)
>  			return ret;
>  	}
> @@ -4688,15 +4705,14 @@ intel_dp_set_property(struct drm_connector *connector,
>  		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");
> +	if (property == connector->dev->mode_config.scaling_mode_property) {
> +		if (is_edp(intel_dp) && val == DRM_MODE_SCALE_NONE) {
> +			DRM_DEBUG_KMS("eDP: no scaling not supported\n");
>  			return -EINVAL;
>  		}
>  
>  		if (intel_connector->panel.fitting_mode == val) {
> -			/* the eDP scaling property is not changed */
> +			/* the connector scaling property is not changed */
>  			return 0;
>  		}
>  		intel_connector->panel.fitting_mode = val;
> @@ -4989,13 +5005,22 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
>  	intel_attach_broadcast_rgb_property(connector);
>  	intel_dp->color_range_auto = true;
>  
> -	if (is_edp(intel_dp)) {
> +	/* Each pipe has panel filter since Ironlake. */
> +	if (INTEL_INFO(connector->dev)->gen >= 5) {
>  		drm_mode_create_scaling_mode_property(connector->dev);
> -		drm_object_attach_property(
> -			&connector->base,
> -			connector->dev->mode_config.scaling_mode_property,
> -			DRM_MODE_SCALE_ASPECT);
> -		intel_connector->panel.fitting_mode = DRM_MODE_SCALE_ASPECT;
> +		if (is_edp(intel_dp)) {
> +			drm_object_attach_property(
> +				&connector->base,
> +				connector->dev->mode_config.scaling_mode_property,
> +				DRM_MODE_SCALE_ASPECT);
> +			intel_connector->panel.fitting_mode = DRM_MODE_SCALE_ASPECT;
> +		} else {
> +			drm_object_attach_property(
> +				&connector->base,
> +				connector->dev->mode_config.scaling_mode_property,
> +				DRM_MODE_SCALE_NONE);
> +			intel_connector->panel.fitting_mode = DRM_MODE_SCALE_NONE;
> +		}
>  	}
>  }
>  
> -- 
> 1.8.2.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx