| Message ID | 20180825010217.7612-1-radhakrishna.sripada@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/2] drm: drm/i915: Add connector property to limit max bpc | expand |
On Fri, Aug 24, 2018 at 06:02:16PM -0700, Radhakrishna Sripada wrote: > At times 12bpc HDMI cannot be driven due to faulty cables, dongles > level shifters etc. To workaround them we may need to drive the output > at a lower bpc. Currently the user space does not have a way to limit > the bpc. The default bpc to be programmed is decided by the driver and > is run against connector limitations. > > Creating a new connector property "max bpc" in order to limit the bpc > with which the pixels are scanned out. xrandr can make use of this > connector property to make sure that bpc does not exceed the configured value. > This property can be used by userspace to set the bpc. > > V2: Initialize max_bpc to satisfy kms_properties > V3: Move the property to drm_connector > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Kishore Kadiyala <kishore.kadiyala@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@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/drm_atomic.c | 4 ++++ > drivers/gpu/drm/drm_atomic_helper.c | 4 ++++ > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_hdmi.c | 11 +++++++++++ > drivers/gpu/drm/i915/intel_modes.c | 20 ++++++++++++++++++++ Pls move all the i915 stuff to the second patch. > include/drm/drm_connector.h | 6 ++++++ > include/drm/drm_mode_config.h | 5 +++++ > 7 files changed, 52 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 3eb061e11e2e..461dde0c2c10 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -1416,6 +1416,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, > > return set_out_fence_for_connector(state->state, connector, > fence_ptr); > + } else if (property == config->max_bpc_property) { > + state->max_bpc = val; > } else if (connector->funcs->atomic_set_property) { > return connector->funcs->atomic_set_property(connector, > state, property, val); > @@ -1511,6 +1513,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector, > *val = 0; > } else if (property == config->writeback_out_fence_ptr_property) { > *val = 0; > + } else if (property == config->max_bpc_property) { > + *val = state->max_bpc; > } else if (connector->funcs->atomic_get_property) { > return connector->funcs->atomic_get_property(connector, > state, property, val); > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 38ce9a375ffb..82caac8d1432 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -638,6 +638,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, > if (old_connector_state->link_status != > new_connector_state->link_status) > new_crtc_state->connectors_changed = true; > + > + if (old_connector_state->max_bpc != > + new_connector_state->max_bpc) > + new_crtc_state->connectors_changed = true; > } > > if (funcs->atomic_check) > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 1b78de838c18..209eb1798238 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1862,6 +1862,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 a1799b5c12bb..82739f342246 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -2097,11 +2097,22 @@ 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 (IS_G4X(dev_priv) || IS_VALLEYVIEW(dev_priv) || > + IS_CHERRYVIEW(dev_priv)) { > + intel_attach_max_bpc_property(connector, 8, 10); > + connector->state->max_bpc = 10; These platforms don't support HDMI deep color. The 10bpc stuff in PIPECONF is for DP. Incidentally DP support seems to be missing from this series. > + } else if (INTEL_GEN(dev_priv) >= 5) { > + intel_attach_max_bpc_property(connector, 8, 12); > + connector->state->max_bpc = 12; > + } > } > > /* > diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c > index ca44bf368e24..78f2ce92f194 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_mode_config *config = &dev->mode_config; > + struct drm_property *prop; > + > + prop = config->max_bpc_property; > + if (prop == NULL) { > + prop = drm_property_create_range(dev, 0, "max bpc", min, max); > + if (prop == NULL) > + return; > + > + config->max_bpc_property = prop; > + } > + > + drm_object_attach_property(&connector->base, prop, max); Might make sense to do 'connector->state->max_bpc = max' here. Avoids having to sprinkle it in every driver. > +} > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > index 97ea41dc678f..d59b9e34ae80 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -460,6 +460,12 @@ struct drm_connector_state { > * drm_writeback_signal_completion() > */ > struct drm_writeback_job *writeback_job; > + > + /** > + * @max_bpc: Connector property to limit the maximum bit depth of > + * the pixels being scanned out. The comment is misleading. It makes me think this has something to do with the pixels we scan out from a framebuffer. > + */ > + unsigned int max_bpc; > }; > > /** > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h > index a0b202e1d69a..b9cd7a73b244 100644 > --- a/include/drm/drm_mode_config.h > +++ b/include/drm/drm_mode_config.h > @@ -562,6 +562,11 @@ struct drm_mode_config { > */ > struct drm_property *link_status_property; > /** > + * @max_bpc_property: Default connector property for the max bpc to be > + * driven out of the connector. > + */ > + struct drm_property *max_bpc_property; > + /** > * @plane_type_property: Default plane property to differentiate > * CURSOR, PRIMARY and OVERLAY legacy uses of planes. > */ > -- > 2.9.3
On Mon, Aug 27, 2018 at 04:31:49PM +0300, Ville Syrjälä wrote: > On Fri, Aug 24, 2018 at 06:02:16PM -0700, Radhakrishna Sripada wrote: > > At times 12bpc HDMI cannot be driven due to faulty cables, dongles > > level shifters etc. To workaround them we may need to drive the output > > at a lower bpc. Currently the user space does not have a way to limit > > the bpc. The default bpc to be programmed is decided by the driver and > > is run against connector limitations. > > > > Creating a new connector property "max bpc" in order to limit the bpc > > with which the pixels are scanned out. xrandr can make use of this > > connector property to make sure that bpc does not exceed the configured value. > > This property can be used by userspace to set the bpc. > > > > V2: Initialize max_bpc to satisfy kms_properties > > V3: Move the property to drm_connector > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: Kishore Kadiyala <kishore.kadiyala@intel.com> > > Cc: Rodrigo Vivi <rodrigo.vivi@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/drm_atomic.c | 4 ++++ > > drivers/gpu/drm/drm_atomic_helper.c | 4 ++++ > > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > > drivers/gpu/drm/i915/intel_hdmi.c | 11 +++++++++++ > > drivers/gpu/drm/i915/intel_modes.c | 20 ++++++++++++++++++++ > > Pls move all the i915 stuff to the second patch. Sure will do it in the next rev. > > > include/drm/drm_connector.h | 6 ++++++ > > include/drm/drm_mode_config.h | 5 +++++ > > 7 files changed, 52 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > > index 3eb061e11e2e..461dde0c2c10 100644 > > --- a/drivers/gpu/drm/drm_atomic.c > > +++ b/drivers/gpu/drm/drm_atomic.c > > @@ -1416,6 +1416,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, > > > > return set_out_fence_for_connector(state->state, connector, > > fence_ptr); > > + } else if (property == config->max_bpc_property) { > > + state->max_bpc = val; > > } else if (connector->funcs->atomic_set_property) { > > return connector->funcs->atomic_set_property(connector, > > state, property, val); > > @@ -1511,6 +1513,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector, > > *val = 0; > > } else if (property == config->writeback_out_fence_ptr_property) { > > *val = 0; > > + } else if (property == config->max_bpc_property) { > > + *val = state->max_bpc; > > } else if (connector->funcs->atomic_get_property) { > > return connector->funcs->atomic_get_property(connector, > > state, property, val); > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > > index 38ce9a375ffb..82caac8d1432 100644 > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > @@ -638,6 +638,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, > > if (old_connector_state->link_status != > > new_connector_state->link_status) > > new_crtc_state->connectors_changed = true; > > + > > + if (old_connector_state->max_bpc != > > + new_connector_state->max_bpc) > > + new_crtc_state->connectors_changed = true; > > } > > > > if (funcs->atomic_check) > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index 1b78de838c18..209eb1798238 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -1862,6 +1862,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 a1799b5c12bb..82739f342246 100644 > > --- a/drivers/gpu/drm/i915/intel_hdmi.c > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > > @@ -2097,11 +2097,22 @@ 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 (IS_G4X(dev_priv) || IS_VALLEYVIEW(dev_priv) || > > + IS_CHERRYVIEW(dev_priv)) { > > + intel_attach_max_bpc_property(connector, 8, 10); > > + connector->state->max_bpc = 10; > > These platforms don't support HDMI deep color. The 10bpc stuff in > PIPECONF is for DP. Incidentally DP support seems to be missing from > this series. Limiting this to 8 and removing VLV/CHV to the other case as they support deep color. > > > + } else if (INTEL_GEN(dev_priv) >= 5) { > > + intel_attach_max_bpc_property(connector, 8, 12); > > + connector->state->max_bpc = 12; > > + } > > } > > > > /* > > diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c > > index ca44bf368e24..78f2ce92f194 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_mode_config *config = &dev->mode_config; > > + struct drm_property *prop; > > + > > + prop = config->max_bpc_property; > > + if (prop == NULL) { > > + prop = drm_property_create_range(dev, 0, "max bpc", min, max); > > + if (prop == NULL) > > + return; > > + > > + config->max_bpc_property = prop; > > + } > > + > > + drm_object_attach_property(&connector->base, prop, max); > > Might make sense to do 'connector->state->max_bpc = max' here. > Avoids having to sprinkle it in every driver. Sure will make this change in the next rev. > > > +} > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > > index 97ea41dc678f..d59b9e34ae80 100644 > > --- a/include/drm/drm_connector.h > > +++ b/include/drm/drm_connector.h > > @@ -460,6 +460,12 @@ struct drm_connector_state { > > * drm_writeback_signal_completion() > > */ > > struct drm_writeback_job *writeback_job; > > + > > + /** > > + * @max_bpc: Connector property to limit the maximum bit depth of > > + * the pixels being scanned out. > > The comment is misleading. It makes me think this has something to do > with the pixels we scan out from a framebuffer. Will modify this comment in the next rev. Regards, Radhakrishna(RK) Sripada > > > + */ > > + unsigned int max_bpc; > > }; > > > > /** > > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h > > index a0b202e1d69a..b9cd7a73b244 100644 > > --- a/include/drm/drm_mode_config.h > > +++ b/include/drm/drm_mode_config.h > > @@ -562,6 +562,11 @@ struct drm_mode_config { > > */ > > struct drm_property *link_status_property; > > /** > > + * @max_bpc_property: Default connector property for the max bpc to be > > + * driven out of the connector. > > + */ > > + struct drm_property *max_bpc_property; > > + /** > > * @plane_type_property: Default plane property to differentiate > > * CURSOR, PRIMARY and OVERLAY legacy uses of planes. > > */ > > -- > > 2.9.3 > > -- > Ville Syrjälä > Intel
On Wed, Aug 29, 2018 at 03:57:05PM -0700, Radhakrishna Sripada wrote: > On Mon, Aug 27, 2018 at 04:31:49PM +0300, Ville Syrjälä wrote: > > On Fri, Aug 24, 2018 at 06:02:16PM -0700, Radhakrishna Sripada wrote: > > > At times 12bpc HDMI cannot be driven due to faulty cables, dongles > > > level shifters etc. To workaround them we may need to drive the output > > > at a lower bpc. Currently the user space does not have a way to limit > > > the bpc. The default bpc to be programmed is decided by the driver and > > > is run against connector limitations. > > > > > > Creating a new connector property "max bpc" in order to limit the bpc > > > with which the pixels are scanned out. xrandr can make use of this > > > connector property to make sure that bpc does not exceed the configured value. > > > This property can be used by userspace to set the bpc. > > > > > > V2: Initialize max_bpc to satisfy kms_properties > > > V3: Move the property to drm_connector > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Cc: Kishore Kadiyala <kishore.kadiyala@intel.com> > > > Cc: Rodrigo Vivi <rodrigo.vivi@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/drm_atomic.c | 4 ++++ > > > drivers/gpu/drm/drm_atomic_helper.c | 4 ++++ > > > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > > > drivers/gpu/drm/i915/intel_hdmi.c | 11 +++++++++++ > > > drivers/gpu/drm/i915/intel_modes.c | 20 ++++++++++++++++++++ > > > > Pls move all the i915 stuff to the second patch. > Sure will do it in the next rev. > > > > > > include/drm/drm_connector.h | 6 ++++++ > > > include/drm/drm_mode_config.h | 5 +++++ > > > 7 files changed, 52 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > > > index 3eb061e11e2e..461dde0c2c10 100644 > > > --- a/drivers/gpu/drm/drm_atomic.c > > > +++ b/drivers/gpu/drm/drm_atomic.c > > > @@ -1416,6 +1416,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, > > > > > > return set_out_fence_for_connector(state->state, connector, > > > fence_ptr); > > > + } else if (property == config->max_bpc_property) { > > > + state->max_bpc = val; > > > } else if (connector->funcs->atomic_set_property) { > > > return connector->funcs->atomic_set_property(connector, > > > state, property, val); > > > @@ -1511,6 +1513,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector, > > > *val = 0; > > > } else if (property == config->writeback_out_fence_ptr_property) { > > > *val = 0; > > > + } else if (property == config->max_bpc_property) { > > > + *val = state->max_bpc; > > > } else if (connector->funcs->atomic_get_property) { > > > return connector->funcs->atomic_get_property(connector, > > > state, property, val); > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > > > index 38ce9a375ffb..82caac8d1432 100644 > > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > > @@ -638,6 +638,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, > > > if (old_connector_state->link_status != > > > new_connector_state->link_status) > > > new_crtc_state->connectors_changed = true; > > > + > > > + if (old_connector_state->max_bpc != > > > + new_connector_state->max_bpc) > > > + new_crtc_state->connectors_changed = true; > > > } > > > > > > if (funcs->atomic_check) > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > > index 1b78de838c18..209eb1798238 100644 > > > --- a/drivers/gpu/drm/i915/intel_drv.h > > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > > @@ -1862,6 +1862,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 a1799b5c12bb..82739f342246 100644 > > > --- a/drivers/gpu/drm/i915/intel_hdmi.c > > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > > > @@ -2097,11 +2097,22 @@ 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 (IS_G4X(dev_priv) || IS_VALLEYVIEW(dev_priv) || > > > + IS_CHERRYVIEW(dev_priv)) { > > > + intel_attach_max_bpc_property(connector, 8, 10); > > > + connector->state->max_bpc = 10; > > > > These platforms don't support HDMI deep color. The 10bpc stuff in > > PIPECONF is for DP. Incidentally DP support seems to be missing from > > this series. > Limiting this to 8 and removing VLV/CHV to the other case as they support deep color. They don't. > > > > > > + } else if (INTEL_GEN(dev_priv) >= 5) { if (!HAS_GMCH_DISPLAY()) { ... should be what we want here. > > > + intel_attach_max_bpc_property(connector, 8, 12); > > > + connector->state->max_bpc = 12; > > > + } > > > } > > > > > > /* > > > diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c > > > index ca44bf368e24..78f2ce92f194 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_mode_config *config = &dev->mode_config; > > > + struct drm_property *prop; > > > + > > > + prop = config->max_bpc_property; > > > + if (prop == NULL) { > > > + prop = drm_property_create_range(dev, 0, "max bpc", min, max); > > > + if (prop == NULL) > > > + return; > > > + > > > + config->max_bpc_property = prop; > > > + } > > > + > > > + drm_object_attach_property(&connector->base, prop, max); > > > > Might make sense to do 'connector->state->max_bpc = max' here. > > Avoids having to sprinkle it in every driver. > Sure will make this change in the next rev. > > > > > > +} > > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > > > index 97ea41dc678f..d59b9e34ae80 100644 > > > --- a/include/drm/drm_connector.h > > > +++ b/include/drm/drm_connector.h > > > @@ -460,6 +460,12 @@ struct drm_connector_state { > > > * drm_writeback_signal_completion() > > > */ > > > struct drm_writeback_job *writeback_job; > > > + > > > + /** > > > + * @max_bpc: Connector property to limit the maximum bit depth of > > > + * the pixels being scanned out. > > > > The comment is misleading. It makes me think this has something to do > > with the pixels we scan out from a framebuffer. > Will modify this comment in the next rev. > > Regards, > Radhakrishna(RK) Sripada > > > > > + */ > > > + unsigned int max_bpc; > > > }; > > > > > > /** > > > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h > > > index a0b202e1d69a..b9cd7a73b244 100644 > > > --- a/include/drm/drm_mode_config.h > > > +++ b/include/drm/drm_mode_config.h > > > @@ -562,6 +562,11 @@ struct drm_mode_config { > > > */ > > > struct drm_property *link_status_property; > > > /** > > > + * @max_bpc_property: Default connector property for the max bpc to be > > > + * driven out of the connector. > > > + */ > > > + struct drm_property *max_bpc_property; > > > + /** > > > * @plane_type_property: Default plane property to differentiate > > > * CURSOR, PRIMARY and OVERLAY legacy uses of planes. > > > */ > > > -- > > > 2.9.3 > > > > -- > > Ville Syrjälä > > Intel
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 3eb061e11e2e..461dde0c2c10 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1416,6 +1416,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, return set_out_fence_for_connector(state->state, connector, fence_ptr); + } else if (property == config->max_bpc_property) { + state->max_bpc = val; } else if (connector->funcs->atomic_set_property) { return connector->funcs->atomic_set_property(connector, state, property, val); @@ -1511,6 +1513,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector, *val = 0; } else if (property == config->writeback_out_fence_ptr_property) { *val = 0; + } else if (property == config->max_bpc_property) { + *val = state->max_bpc; } else if (connector->funcs->atomic_get_property) { return connector->funcs->atomic_get_property(connector, state, property, val); diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 38ce9a375ffb..82caac8d1432 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -638,6 +638,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, if (old_connector_state->link_status != new_connector_state->link_status) new_crtc_state->connectors_changed = true; + + if (old_connector_state->max_bpc != + new_connector_state->max_bpc) + new_crtc_state->connectors_changed = true; } if (funcs->atomic_check) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 1b78de838c18..209eb1798238 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1862,6 +1862,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 a1799b5c12bb..82739f342246 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -2097,11 +2097,22 @@ 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 (IS_G4X(dev_priv) || IS_VALLEYVIEW(dev_priv) || + IS_CHERRYVIEW(dev_priv)) { + intel_attach_max_bpc_property(connector, 8, 10); + connector->state->max_bpc = 10; + } else if (INTEL_GEN(dev_priv) >= 5) { + intel_attach_max_bpc_property(connector, 8, 12); + connector->state->max_bpc = 12; + } } /* diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c index ca44bf368e24..78f2ce92f194 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_mode_config *config = &dev->mode_config; + struct drm_property *prop; + + prop = config->max_bpc_property; + if (prop == NULL) { + prop = drm_property_create_range(dev, 0, "max bpc", min, max); + if (prop == NULL) + return; + + config->max_bpc_property = prop; + } + + drm_object_attach_property(&connector->base, prop, max); +} diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 97ea41dc678f..d59b9e34ae80 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -460,6 +460,12 @@ struct drm_connector_state { * drm_writeback_signal_completion() */ struct drm_writeback_job *writeback_job; + + /** + * @max_bpc: Connector property to limit the maximum bit depth of + * the pixels being scanned out. + */ + unsigned int max_bpc; }; /** diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index a0b202e1d69a..b9cd7a73b244 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -562,6 +562,11 @@ struct drm_mode_config { */ struct drm_property *link_status_property; /** + * @max_bpc_property: Default connector property for the max bpc to be + * driven out of the connector. + */ + struct drm_property *max_bpc_property; + /** * @plane_type_property: Default plane property to differentiate * CURSOR, PRIMARY and OVERLAY legacy uses of planes. */
At times 12bpc HDMI cannot be driven due to faulty cables, dongles level shifters etc. To workaround them we may need to drive the output at a lower bpc. Currently the user space does not have a way to limit the bpc. The default bpc to be programmed is decided by the driver and is run against connector limitations. Creating a new connector property "max bpc" in order to limit the bpc with which the pixels are scanned out. xrandr can make use of this connector property to make sure that bpc does not exceed the configured value. This property can be used by userspace to set the bpc. V2: Initialize max_bpc to satisfy kms_properties V3: Move the property to drm_connector Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Kishore Kadiyala <kishore.kadiyala@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@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/drm_atomic.c | 4 ++++ drivers/gpu/drm/drm_atomic_helper.c | 4 ++++ drivers/gpu/drm/i915/intel_drv.h | 2 ++ drivers/gpu/drm/i915/intel_hdmi.c | 11 +++++++++++ drivers/gpu/drm/i915/intel_modes.c | 20 ++++++++++++++++++++ include/drm/drm_connector.h | 6 ++++++ include/drm/drm_mode_config.h | 5 +++++ 7 files changed, 52 insertions(+)