| Message ID | 20180918181115.3388-2-radhakrishna.sripada@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v8,1/2] drm: Add connector property to limit max bpc | expand |
On Tue, Sep 18, 2018 at 11:11:15AM -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) > V6: Use the core function to attach max_bpc property, remove the redundant > clamping of pipe bpp based on connector info > V7: Fix Checkpatch warnings > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > 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 | 49 ++++++++++++++++++++---------------- > drivers/gpu/drm/i915/intel_dp.c | 5 ++++ > drivers/gpu/drm/i915/intel_hdmi.c | 5 ++++ > 3 files changed, 38 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index eb25037d7b38..75afd53590b1 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -10845,29 +10845,37 @@ static void intel_modeset_update_connector_atomic_state(struct drm_device *dev) > } > > static void > -connected_sink_compute_bpp(struct intel_connector *connector, > - struct intel_crtc_state *pipe_config) > +connected_sink_max_bpp(struct drm_connector_state *conn_state, > + struct intel_crtc_state *pipe_config) > { > - const struct drm_display_info *info = &connector->base.display_info; > - int bpp = pipe_config->pipe_bpp; > - > - DRM_DEBUG_KMS("[CONNECTOR:%d:%s] checking for sink bpp constrains\n", > - connector->base.base.id, > - connector->base.name); > - > - /* Don't use an invalid EDID bpc value */ > - if (info->bpc != 0 && info->bpc * 3 < bpp) { > - DRM_DEBUG_KMS("clamping display bpp (was %d) to EDID reported max of %d\n", > - bpp, info->bpc * 3); > - pipe_config->pipe_bpp = info->bpc * 3; > + if (pipe_config->pipe_bpp < conn_state->max_bpc * 3) { > + conn_state->max_bpc = pipe_config->pipe_bpp / 3; > + return; This back and forth between max_bpc and pipe_bpp is a bit confusing. I'd probably leave max_bpc alone here and just update pipe_bpp as needed. > } > > - /* Clamp bpp to 8 on screens without EDID 1.4 */ > - if (info->bpc == 0 && bpp > 24) { > - DRM_DEBUG_KMS("clamping display bpp (was %d) to default limit of 24\n", > - bpp); > - pipe_config->pipe_bpp = 24; > + switch (conn_state->max_bpc) { This is missing the 6bpc case at least. I suppose the current code isn't particuraly robust against unexpected values coming via info->bpc. The switch statement does seem an improvement in that regard. Though would be nice to compact it a bit using eg. the gcc case range extension. > + 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: With the proposed min() we'd never get bpc > 12 here. > + pipe_config->pipe_bpp = 12 * 3; > + break; > + case 16: > + pipe_config->pipe_bpp = 16 * 3; > + break; > + default: > + break; Maybe just return an error here. I suppose it should never happen unless there's some bogus displays out there that report < 6 bpc. > } > + > + DRM_DEBUG_KMS("Limiting display bpp to %d\n", pipe_config->pipe_bpp); Would be nice to include all the relevant information in this debug print: original pipe_bpp, info->bpc*3, max_requested_bpc. Maybe something like this would work to keep the code easy to read: { bpp = min(pipe_bpp, max_bpc*3); switch (bpp) { ... } if (bpp != pipe_bpp) { DRM_DEBUG_KMS(...); pipe_bpp = bpp; } } > } > > static int > @@ -10898,8 +10906,7 @@ compute_baseline_pipe_bpp(struct intel_crtc *crtc, > if (connector_state->crtc != &crtc->base) > continue; > > - connected_sink_compute_bpp(to_intel_connector(connector), > - pipe_config); > + connected_sink_max_bpp(connector_state, pipe_config); > } > > return bpp; > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 436c22de33b6..aefca1d9e87b 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -5719,6 +5719,11 @@ 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); > + if ((IS_G4X(dev_priv) || IS_VALLEYVIEW(dev_priv) || > + IS_CHERRYVIEW(dev_priv))) Just HAS_GMCH_DISPLAY() will do here. > + drm_connector_attach_max_bpc_property(connector, 8, 10); > + else if (INTEL_GEN(dev_priv) >= 5) > + drm_connector_attach_max_bpc_property(connector, 8, 12); DP does support 6 bpc as well, so we may want to reduce the min to 6 here. > > if (intel_dp_is_edp(intel_dp)) { > u32 allowed_scalers; > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > index a2dab0b6bde6..2b432c7e4f8a 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -2109,11 +2109,16 @@ 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)) > + drm_connector_attach_max_bpc_property(connector, 8, 12); > } > > /* > -- > 2.9.3
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index eb25037d7b38..75afd53590b1 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10845,29 +10845,37 @@ static void intel_modeset_update_connector_atomic_state(struct drm_device *dev) } static void -connected_sink_compute_bpp(struct intel_connector *connector, - struct intel_crtc_state *pipe_config) +connected_sink_max_bpp(struct drm_connector_state *conn_state, + struct intel_crtc_state *pipe_config) { - const struct drm_display_info *info = &connector->base.display_info; - int bpp = pipe_config->pipe_bpp; - - DRM_DEBUG_KMS("[CONNECTOR:%d:%s] checking for sink bpp constrains\n", - connector->base.base.id, - connector->base.name); - - /* Don't use an invalid EDID bpc value */ - if (info->bpc != 0 && info->bpc * 3 < bpp) { - DRM_DEBUG_KMS("clamping display bpp (was %d) to EDID reported max of %d\n", - bpp, info->bpc * 3); - pipe_config->pipe_bpp = info->bpc * 3; + if (pipe_config->pipe_bpp < conn_state->max_bpc * 3) { + conn_state->max_bpc = pipe_config->pipe_bpp / 3; + return; } - /* Clamp bpp to 8 on screens without EDID 1.4 */ - if (info->bpc == 0 && bpp > 24) { - DRM_DEBUG_KMS("clamping display bpp (was %d) to default limit of 24\n", - bpp); - pipe_config->pipe_bpp = 24; + 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 @@ -10898,8 +10906,7 @@ compute_baseline_pipe_bpp(struct intel_crtc *crtc, if (connector_state->crtc != &crtc->base) continue; - connected_sink_compute_bpp(to_intel_connector(connector), - pipe_config); + connected_sink_max_bpp(connector_state, pipe_config); } return bpp; diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 436c22de33b6..aefca1d9e87b 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -5719,6 +5719,11 @@ 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); + if ((IS_G4X(dev_priv) || IS_VALLEYVIEW(dev_priv) || + IS_CHERRYVIEW(dev_priv))) + drm_connector_attach_max_bpc_property(connector, 8, 10); + else if (INTEL_GEN(dev_priv) >= 5) + drm_connector_attach_max_bpc_property(connector, 8, 12); if (intel_dp_is_edp(intel_dp)) { u32 allowed_scalers; diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index a2dab0b6bde6..2b432c7e4f8a 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -2109,11 +2109,16 @@ 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)) + drm_connector_attach_max_bpc_property(connector, 8, 12); } /*
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) V6: Use the core function to attach max_bpc property, remove the redundant clamping of pipe bpp based on connector info V7: Fix Checkpatch warnings Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> 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 | 49 ++++++++++++++++++++---------------- drivers/gpu/drm/i915/intel_dp.c | 5 ++++ drivers/gpu/drm/i915/intel_hdmi.c | 5 ++++ 3 files changed, 38 insertions(+), 21 deletions(-)