Re: [PATCH v5 2/2] drm/i915: Allow "max bpc" property to limit pipe_bpp

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

On Wed, Sep 05, 2018 at 01:12:00PM -0700, Radhakrishna Sripada wrote:
> Use the newly added "max bpc" connector property to limit pipe bpp.
> 
> V3: Use drm_connector_state to access the "max bpc" property
> V4: Initialize the drm property, add suuport to DP(Ville)
> V5: Use the property in the connector and fix CI failure(Ville)
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Kishore Kadiyala <kishore.kadiyala@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 31 +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dp.c      |  1 +
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  drivers/gpu/drm/i915/intel_hdmi.c    |  7 +++++++
>  drivers/gpu/drm/i915/intel_modes.c   | 20 ++++++++++++++++++++
>  5 files changed, 61 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1bd14c61dab5..a890aade094c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10787,6 +10787,34 @@ connected_sink_compute_bpp(struct intel_connector *connector,
>  	}
>  }
>  
> +static void
> +connected_sink_max_bpp(struct drm_connector_state *conn_state,
> +			     struct intel_crtc_state *pipe_config)
> +{
> +	switch (conn_state->max_bpc) {
> +	case 8:
> +	case 9:
> +		pipe_config->pipe_bpp = 8*3;
> +		break;
> +	case 10:
> +	case 11:
> +		pipe_config->pipe_bpp = 10*3;
> +		break;
> +	case 12:
> +	case 13:
> +	case 14:
> +	case 15:
> +		pipe_config->pipe_bpp = 12*3;
> +		break;
> +	case 16:
> +		pipe_config->pipe_bpp = 16*3;
> +		break;
> +	default:
> +		break;
> +	}
> +	DRM_DEBUG_KMS("Limiting display bpp to %d\n", pipe_config->pipe_bpp);
> +}
> +
>  static int
>  compute_baseline_pipe_bpp(struct intel_crtc *crtc,
>  			  struct intel_crtc_state *pipe_config)
> @@ -10815,6 +10843,9 @@ compute_baseline_pipe_bpp(struct intel_crtc *crtc,
>  		if (connector_state->crtc != &crtc->base)
>  			continue;
>  
> +		if (connector_state->max_bpc)
> +			connected_sink_max_bpp(connector_state, pipe_config);
> +
>  		connected_sink_compute_bpp(to_intel_connector(connector),
>  					   pipe_config);
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 436c22de33b6..3955745a4d9f 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5719,6 +5719,7 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
>  		intel_attach_force_audio_property(connector);
>  
>  	intel_attach_broadcast_rgb_property(connector);
> +	intel_attach_max_bpc_property(connector, 8, 16);

IIRC gmch platforms can't do more than 10bpc, and the rest
are limited to 12bpc.

>  
>  	if (intel_dp_is_edp(intel_dp)) {
>  		u32 allowed_scalers;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f5731215210a..b3c703dacc92 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1869,6 +1869,8 @@ int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
>  void intel_attach_force_audio_property(struct drm_connector *connector);
>  void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
>  void intel_attach_aspect_ratio_property(struct drm_connector *connector);
> +void intel_attach_max_bpc_property(struct drm_connector *connector, int min, int
> +				   max);
>  
>  
>  /* intel_overlay.c */
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index a2dab0b6bde6..e649bbf07642 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -2109,11 +2109,18 @@ static const struct drm_encoder_funcs intel_hdmi_enc_funcs = {
>  static void
>  intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *connector)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> +
>  	intel_attach_force_audio_property(connector);
>  	intel_attach_broadcast_rgb_property(connector);
>  	intel_attach_aspect_ratio_property(connector);
>  	drm_connector_attach_content_type_property(connector);
>  	connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> +
> +	if (HAS_GMCH_DISPLAY(dev_priv))
> +		intel_attach_max_bpc_property(connector, 8, 8);

Not sure exposing the prop makes much sense when you can't modify it.

> +	else
> +		intel_attach_max_bpc_property(connector, 8, 12);
>  }
>  
>  /*
> diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
> index ca44bf368e24..12f1238bad8a 100644
> --- a/drivers/gpu/drm/i915/intel_modes.c
> +++ b/drivers/gpu/drm/i915/intel_modes.c
> @@ -133,3 +133,23 @@ intel_attach_aspect_ratio_property(struct drm_connector *connector)
>  			connector->dev->mode_config.aspect_ratio_property,
>  			DRM_MODE_PICTURE_ASPECT_NONE);
>  }
> +
> +void
> +intel_attach_max_bpc_property(struct drm_connector *connector, int min, int
> +			       max)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_property *prop;
> +
> +	prop = connector->max_bpc_property;
> +	if (prop == NULL) {
> +		prop = drm_property_create_range(dev, 0, "max bpc", min, max);
> +		if (prop == NULL)
> +			return;
> +
> +		connector->max_bpc_property = prop;
> +	}
> +
> +	drm_object_attach_property(&connector->base, prop, max);
> +	connector->state->max_bpc = max;
> +}

There's nothing i915 specific in this function, so might as well move it
to the core.

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel