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

Daniel Vetter
2018-09-05T22:03:02Z

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);

I think this could wold be best put into shared code, computing a max_bpc
that takes connector->display_info and connector_state->max_bpc into
account. So part of the core drm patch. Would need both a ->max_bpc and a
->max_requested_bpc for the property value or something like that.

Besides this small nit I think this looks solid from a high level.
-Daniel

> +
> 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);
>
> 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);
> + 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;
> +}
> --
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org