| Message ID | 20180924210815.21815-1-radhakrishna.sripada@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v10,1/2] drm: Add connector property to limit max bpc | expand |
On Mon, Sep 24, 2018 at 02:08:14PM -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. > 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 > V4: Split drm and i915 components(Ville) > V5: Make the property per connector(Ville) > V6: Compare the requested bpc to connector bpc(Daniel) > Move the attach_property function to core(Ville) > V7: Fix checkpatch warnings > V8: Simplify the connector check code(Ville) > V9: Const display_info(Ville) > V10: Fix CI issues. > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > 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> > Cc: Sunpeng Li <sunpeng.li@amd.com> > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> Skimming this, I think it looks good now at a high-level. What's missing is now kernel-doc for these new prorties, needs to be added here: https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#standard-connector-properties With that I'm happy with the high-level design: Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> No full review since I didn't look at the igt side for this. Userspace I think is ok, since it's just another connector prop. -Daniel > --- > drivers/gpu/drm/drm_atomic.c | 5 +++++ > drivers/gpu/drm/drm_atomic_helper.c | 4 ++++ > drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++ > drivers/gpu/drm/drm_connector.c | 33 +++++++++++++++++++++++++++++++++ > include/drm/drm_connector.h | 20 ++++++++++++++++++++ > 5 files changed, 66 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 2870ae205237..f328bcca84a8 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -390,6 +390,7 @@ static int drm_atomic_connector_check(struct drm_connector *connector, > { > struct drm_crtc_state *crtc_state; > struct drm_writeback_job *writeback_job = state->writeback_job; > + const struct drm_display_info *info = &connector->display_info; > > if ((connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK) || !writeback_job) > return 0; > @@ -417,6 +418,10 @@ static int drm_atomic_connector_check(struct drm_connector *connector, > return -EINVAL; > } > > + state->max_bpc = info->bpc ? info->bpc : 8; > + if (connector->max_bpc_property) > + state->max_bpc = min(state->max_bpc, state->max_requested_bpc); > + > return 0; > } > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index e49b22381048..75aeca35f6d9 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -639,6 +639,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_requested_bpc != > + new_connector_state->max_requested_bpc) > + new_crtc_state->connectors_changed = true; > } > > if (funcs->atomic_check) > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c > index d5b7f315098c..86ac33922b09 100644 > --- a/drivers/gpu/drm/drm_atomic_uapi.c > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > @@ -740,6 +740,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 == connector->max_bpc_property) { > + state->max_requested_bpc = val; > } else if (connector->funcs->atomic_set_property) { > return connector->funcs->atomic_set_property(connector, > state, property, val); > @@ -804,6 +806,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 == connector->max_bpc_property) { > + *val = state->max_requested_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_connector.c b/drivers/gpu/drm/drm_connector.c > index 1e40e5decbe9..65e22c1b37a5 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -1583,6 +1583,39 @@ void drm_connector_set_link_status_property(struct drm_connector *connector, > EXPORT_SYMBOL(drm_connector_set_link_status_property); > > /** > + * drm_connector_attach_max_bpc_property - attach "max bpc" property > + * @connector: connector to attach max bpc property on. > + * @min: The minimum bit depth supported by the connector. > + * @max: The maximum bit depth supported by the connector. > + * > + * This is used to add support for limiting the bit depth on a connector. > + * > + * Returns: > + * Zero on success, negative errno on failure. > + */ > +int drm_connector_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) { > + prop = drm_property_create_range(dev, 0, "max bpc", min, max); > + if (!prop) > + return -ENOMEM; > + > + connector->max_bpc_property = prop; > + } > + > + drm_object_attach_property(&connector->base, prop, max); > + connector->state->max_requested_bpc = max; > + > + return 0; > +} > +EXPORT_SYMBOL(drm_connector_attach_max_bpc_property); > + > +/** > * drm_connector_init_panel_orientation_property - > * initialize the connecters panel_orientation property > * @connector: connector for which to init the panel-orientation property. > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > index 91a877fa00cb..bfc3e698cda6 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -461,6 +461,18 @@ struct drm_connector_state { > * drm_writeback_signal_completion() > */ > struct drm_writeback_job *writeback_job; > + > + /** > + * @max_requested_bpc: Connector property to limit the maximum bit > + * depth of the pixels. > + */ > + u8 max_requested_bpc; > + > + /** > + * @max_bpc: Connector max_bpc based on the requested max_bpc property > + * and the connector bpc limitations obtained from edid. > + */ > + u8 max_bpc; > }; > > /** > @@ -924,6 +936,12 @@ struct drm_connector { > */ > struct drm_property_blob *path_blob_ptr; > > + /** > + * @max_bpc_property: Default connector property for the max bpc to be > + * driven out of the connector. > + */ > + struct drm_property *max_bpc_property; > + > #define DRM_CONNECTOR_POLL_HPD (1 << 0) > #define DRM_CONNECTOR_POLL_CONNECT (1 << 1) > #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2) > @@ -1201,6 +1219,8 @@ void drm_connector_set_link_status_property(struct drm_connector *connector, > uint64_t link_status); > int drm_connector_init_panel_orientation_property( > struct drm_connector *connector, int width, int height); > +int drm_connector_attach_max_bpc_property(struct drm_connector *connector, > + int min, int max); > > /** > * struct drm_tile_group - Tile group metadata > -- > 2.9.3 >
On Mon, Oct 01, 2018 at 09:23:38AM +0200, Daniel Vetter wrote: > On Mon, Sep 24, 2018 at 02:08:14PM -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. > > 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 > > V4: Split drm and i915 components(Ville) > > V5: Make the property per connector(Ville) > > V6: Compare the requested bpc to connector bpc(Daniel) > > Move the attach_property function to core(Ville) > > V7: Fix checkpatch warnings > > V8: Simplify the connector check code(Ville) > > V9: Const display_info(Ville) > > V10: Fix CI issues. > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > 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> > > Cc: Sunpeng Li <sunpeng.li@amd.com> > > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> > > Skimming this, I think it looks good now at a high-level. > > What's missing is now kernel-doc for these new prorties, needs to be added > here: > > https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#standard-connector-properties > > With that I'm happy with the high-level design: > > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > No full review since I didn't look at the igt side for this. Userspace I > think is ok, since it's just another connector prop. kms_properties would test this newly added property as part of the connector properties. Do you suggest me to add a new subtest to try checking the different values for the newly added property? --Radhakrishna Sripada > -Daniel > > > --- > > drivers/gpu/drm/drm_atomic.c | 5 +++++ > > drivers/gpu/drm/drm_atomic_helper.c | 4 ++++ > > drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++ > > drivers/gpu/drm/drm_connector.c | 33 +++++++++++++++++++++++++++++++++ > > include/drm/drm_connector.h | 20 ++++++++++++++++++++ > > 5 files changed, 66 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > > index 2870ae205237..f328bcca84a8 100644 > > --- a/drivers/gpu/drm/drm_atomic.c > > +++ b/drivers/gpu/drm/drm_atomic.c > > @@ -390,6 +390,7 @@ static int drm_atomic_connector_check(struct drm_connector *connector, > > { > > struct drm_crtc_state *crtc_state; > > struct drm_writeback_job *writeback_job = state->writeback_job; > > + const struct drm_display_info *info = &connector->display_info; > > > > if ((connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK) || !writeback_job) > > return 0; > > @@ -417,6 +418,10 @@ static int drm_atomic_connector_check(struct drm_connector *connector, > > return -EINVAL; > > } > > > > + state->max_bpc = info->bpc ? info->bpc : 8; > > + if (connector->max_bpc_property) > > + state->max_bpc = min(state->max_bpc, state->max_requested_bpc); > > + > > return 0; > > } > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > > index e49b22381048..75aeca35f6d9 100644 > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > @@ -639,6 +639,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_requested_bpc != > > + new_connector_state->max_requested_bpc) > > + new_crtc_state->connectors_changed = true; > > } > > > > if (funcs->atomic_check) > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c > > index d5b7f315098c..86ac33922b09 100644 > > --- a/drivers/gpu/drm/drm_atomic_uapi.c > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > > @@ -740,6 +740,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 == connector->max_bpc_property) { > > + state->max_requested_bpc = val; > > } else if (connector->funcs->atomic_set_property) { > > return connector->funcs->atomic_set_property(connector, > > state, property, val); > > @@ -804,6 +806,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 == connector->max_bpc_property) { > > + *val = state->max_requested_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_connector.c b/drivers/gpu/drm/drm_connector.c > > index 1e40e5decbe9..65e22c1b37a5 100644 > > --- a/drivers/gpu/drm/drm_connector.c > > +++ b/drivers/gpu/drm/drm_connector.c > > @@ -1583,6 +1583,39 @@ void drm_connector_set_link_status_property(struct drm_connector *connector, > > EXPORT_SYMBOL(drm_connector_set_link_status_property); > > > > /** > > + * drm_connector_attach_max_bpc_property - attach "max bpc" property > > + * @connector: connector to attach max bpc property on. > > + * @min: The minimum bit depth supported by the connector. > > + * @max: The maximum bit depth supported by the connector. > > + * > > + * This is used to add support for limiting the bit depth on a connector. > > + * > > + * Returns: > > + * Zero on success, negative errno on failure. > > + */ > > +int drm_connector_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) { > > + prop = drm_property_create_range(dev, 0, "max bpc", min, max); > > + if (!prop) > > + return -ENOMEM; > > + > > + connector->max_bpc_property = prop; > > + } > > + > > + drm_object_attach_property(&connector->base, prop, max); > > + connector->state->max_requested_bpc = max; > > + > > + return 0; > > +} > > +EXPORT_SYMBOL(drm_connector_attach_max_bpc_property); > > + > > +/** > > * drm_connector_init_panel_orientation_property - > > * initialize the connecters panel_orientation property > > * @connector: connector for which to init the panel-orientation property. > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > > index 91a877fa00cb..bfc3e698cda6 100644 > > --- a/include/drm/drm_connector.h > > +++ b/include/drm/drm_connector.h > > @@ -461,6 +461,18 @@ struct drm_connector_state { > > * drm_writeback_signal_completion() > > */ > > struct drm_writeback_job *writeback_job; > > + > > + /** > > + * @max_requested_bpc: Connector property to limit the maximum bit > > + * depth of the pixels. > > + */ > > + u8 max_requested_bpc; > > + > > + /** > > + * @max_bpc: Connector max_bpc based on the requested max_bpc property > > + * and the connector bpc limitations obtained from edid. > > + */ > > + u8 max_bpc; > > }; > > > > /** > > @@ -924,6 +936,12 @@ struct drm_connector { > > */ > > struct drm_property_blob *path_blob_ptr; > > > > + /** > > + * @max_bpc_property: Default connector property for the max bpc to be > > + * driven out of the connector. > > + */ > > + struct drm_property *max_bpc_property; > > + > > #define DRM_CONNECTOR_POLL_HPD (1 << 0) > > #define DRM_CONNECTOR_POLL_CONNECT (1 << 1) > > #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2) > > @@ -1201,6 +1219,8 @@ void drm_connector_set_link_status_property(struct drm_connector *connector, > > uint64_t link_status); > > int drm_connector_init_panel_orientation_property( > > struct drm_connector *connector, int width, int height); > > +int drm_connector_attach_max_bpc_property(struct drm_connector *connector, > > + int min, int max); > > > > /** > > * struct drm_tile_group - Tile group metadata > > -- > > 2.9.3 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Wed, Oct 03, 2018 at 03:24:40PM -0700, Radhakrishna Sripada wrote: > On Mon, Oct 01, 2018 at 09:23:38AM +0200, Daniel Vetter wrote: > > On Mon, Sep 24, 2018 at 02:08:14PM -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. > > > 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 > > > V4: Split drm and i915 components(Ville) > > > V5: Make the property per connector(Ville) > > > V6: Compare the requested bpc to connector bpc(Daniel) > > > Move the attach_property function to core(Ville) > > > V7: Fix checkpatch warnings > > > V8: Simplify the connector check code(Ville) > > > V9: Const display_info(Ville) > > > V10: Fix CI issues. > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > > 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> > > > Cc: Sunpeng Li <sunpeng.li@amd.com> > > > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> > > > > Skimming this, I think it looks good now at a high-level. > > > > What's missing is now kernel-doc for these new prorties, needs to be added > > here: > > > > https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#standard-connector-properties > > > > With that I'm happy with the high-level design: > > > > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > No full review since I didn't look at the igt side for this. Userspace I > > think is ok, since it's just another connector prop. > kms_properties would test this newly added property as part of the connector properties. > Do you suggest me to add a new subtest to try checking the different values for the > newly added property? kms_properties most definitely doesn't do any kind of functional testing here. Which means it's not validated in an automatic way. So yes, we most definitely want a real testcase here. Coming up with a proper validation strategy here is going to be somewhat challenging though. Thanks, Daniel > > --Radhakrishna Sripada > > -Daniel > > > > > --- > > > drivers/gpu/drm/drm_atomic.c | 5 +++++ > > > drivers/gpu/drm/drm_atomic_helper.c | 4 ++++ > > > drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++ > > > drivers/gpu/drm/drm_connector.c | 33 +++++++++++++++++++++++++++++++++ > > > include/drm/drm_connector.h | 20 ++++++++++++++++++++ > > > 5 files changed, 66 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > > > index 2870ae205237..f328bcca84a8 100644 > > > --- a/drivers/gpu/drm/drm_atomic.c > > > +++ b/drivers/gpu/drm/drm_atomic.c > > > @@ -390,6 +390,7 @@ static int drm_atomic_connector_check(struct drm_connector *connector, > > > { > > > struct drm_crtc_state *crtc_state; > > > struct drm_writeback_job *writeback_job = state->writeback_job; > > > + const struct drm_display_info *info = &connector->display_info; > > > > > > if ((connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK) || !writeback_job) > > > return 0; > > > @@ -417,6 +418,10 @@ static int drm_atomic_connector_check(struct drm_connector *connector, > > > return -EINVAL; > > > } > > > > > > + state->max_bpc = info->bpc ? info->bpc : 8; > > > + if (connector->max_bpc_property) > > > + state->max_bpc = min(state->max_bpc, state->max_requested_bpc); > > > + > > > return 0; > > > } > > > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > > > index e49b22381048..75aeca35f6d9 100644 > > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > > @@ -639,6 +639,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_requested_bpc != > > > + new_connector_state->max_requested_bpc) > > > + new_crtc_state->connectors_changed = true; > > > } > > > > > > if (funcs->atomic_check) > > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c > > > index d5b7f315098c..86ac33922b09 100644 > > > --- a/drivers/gpu/drm/drm_atomic_uapi.c > > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > > > @@ -740,6 +740,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 == connector->max_bpc_property) { > > > + state->max_requested_bpc = val; > > > } else if (connector->funcs->atomic_set_property) { > > > return connector->funcs->atomic_set_property(connector, > > > state, property, val); > > > @@ -804,6 +806,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 == connector->max_bpc_property) { > > > + *val = state->max_requested_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_connector.c b/drivers/gpu/drm/drm_connector.c > > > index 1e40e5decbe9..65e22c1b37a5 100644 > > > --- a/drivers/gpu/drm/drm_connector.c > > > +++ b/drivers/gpu/drm/drm_connector.c > > > @@ -1583,6 +1583,39 @@ void drm_connector_set_link_status_property(struct drm_connector *connector, > > > EXPORT_SYMBOL(drm_connector_set_link_status_property); > > > > > > /** > > > + * drm_connector_attach_max_bpc_property - attach "max bpc" property > > > + * @connector: connector to attach max bpc property on. > > > + * @min: The minimum bit depth supported by the connector. > > > + * @max: The maximum bit depth supported by the connector. > > > + * > > > + * This is used to add support for limiting the bit depth on a connector. > > > + * > > > + * Returns: > > > + * Zero on success, negative errno on failure. > > > + */ > > > +int drm_connector_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) { > > > + prop = drm_property_create_range(dev, 0, "max bpc", min, max); > > > + if (!prop) > > > + return -ENOMEM; > > > + > > > + connector->max_bpc_property = prop; > > > + } > > > + > > > + drm_object_attach_property(&connector->base, prop, max); > > > + connector->state->max_requested_bpc = max; > > > + > > > + return 0; > > > +} > > > +EXPORT_SYMBOL(drm_connector_attach_max_bpc_property); > > > + > > > +/** > > > * drm_connector_init_panel_orientation_property - > > > * initialize the connecters panel_orientation property > > > * @connector: connector for which to init the panel-orientation property. > > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > > > index 91a877fa00cb..bfc3e698cda6 100644 > > > --- a/include/drm/drm_connector.h > > > +++ b/include/drm/drm_connector.h > > > @@ -461,6 +461,18 @@ struct drm_connector_state { > > > * drm_writeback_signal_completion() > > > */ > > > struct drm_writeback_job *writeback_job; > > > + > > > + /** > > > + * @max_requested_bpc: Connector property to limit the maximum bit > > > + * depth of the pixels. > > > + */ > > > + u8 max_requested_bpc; > > > + > > > + /** > > > + * @max_bpc: Connector max_bpc based on the requested max_bpc property > > > + * and the connector bpc limitations obtained from edid. > > > + */ > > > + u8 max_bpc; > > > }; > > > > > > /** > > > @@ -924,6 +936,12 @@ struct drm_connector { > > > */ > > > struct drm_property_blob *path_blob_ptr; > > > > > > + /** > > > + * @max_bpc_property: Default connector property for the max bpc to be > > > + * driven out of the connector. > > > + */ > > > + struct drm_property *max_bpc_property; > > > + > > > #define DRM_CONNECTOR_POLL_HPD (1 << 0) > > > #define DRM_CONNECTOR_POLL_CONNECT (1 << 1) > > > #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2) > > > @@ -1201,6 +1219,8 @@ void drm_connector_set_link_status_property(struct drm_connector *connector, > > > uint64_t link_status); > > > int drm_connector_init_panel_orientation_property( > > > struct drm_connector *connector, int width, int height); > > > +int drm_connector_attach_max_bpc_property(struct drm_connector *connector, > > > + int min, int max); > > > > > > /** > > > * struct drm_tile_group - Tile group metadata > > > -- > > > 2.9.3 > > > > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 2870ae205237..f328bcca84a8 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -390,6 +390,7 @@ static int drm_atomic_connector_check(struct drm_connector *connector, { struct drm_crtc_state *crtc_state; struct drm_writeback_job *writeback_job = state->writeback_job; + const struct drm_display_info *info = &connector->display_info; if ((connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK) || !writeback_job) return 0; @@ -417,6 +418,10 @@ static int drm_atomic_connector_check(struct drm_connector *connector, return -EINVAL; } + state->max_bpc = info->bpc ? info->bpc : 8; + if (connector->max_bpc_property) + state->max_bpc = min(state->max_bpc, state->max_requested_bpc); + return 0; } diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index e49b22381048..75aeca35f6d9 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -639,6 +639,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_requested_bpc != + new_connector_state->max_requested_bpc) + new_crtc_state->connectors_changed = true; } if (funcs->atomic_check) diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index d5b7f315098c..86ac33922b09 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -740,6 +740,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 == connector->max_bpc_property) { + state->max_requested_bpc = val; } else if (connector->funcs->atomic_set_property) { return connector->funcs->atomic_set_property(connector, state, property, val); @@ -804,6 +806,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 == connector->max_bpc_property) { + *val = state->max_requested_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_connector.c b/drivers/gpu/drm/drm_connector.c index 1e40e5decbe9..65e22c1b37a5 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1583,6 +1583,39 @@ void drm_connector_set_link_status_property(struct drm_connector *connector, EXPORT_SYMBOL(drm_connector_set_link_status_property); /** + * drm_connector_attach_max_bpc_property - attach "max bpc" property + * @connector: connector to attach max bpc property on. + * @min: The minimum bit depth supported by the connector. + * @max: The maximum bit depth supported by the connector. + * + * This is used to add support for limiting the bit depth on a connector. + * + * Returns: + * Zero on success, negative errno on failure. + */ +int drm_connector_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) { + prop = drm_property_create_range(dev, 0, "max bpc", min, max); + if (!prop) + return -ENOMEM; + + connector->max_bpc_property = prop; + } + + drm_object_attach_property(&connector->base, prop, max); + connector->state->max_requested_bpc = max; + + return 0; +} +EXPORT_SYMBOL(drm_connector_attach_max_bpc_property); + +/** * drm_connector_init_panel_orientation_property - * initialize the connecters panel_orientation property * @connector: connector for which to init the panel-orientation property. diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 91a877fa00cb..bfc3e698cda6 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -461,6 +461,18 @@ struct drm_connector_state { * drm_writeback_signal_completion() */ struct drm_writeback_job *writeback_job; + + /** + * @max_requested_bpc: Connector property to limit the maximum bit + * depth of the pixels. + */ + u8 max_requested_bpc; + + /** + * @max_bpc: Connector max_bpc based on the requested max_bpc property + * and the connector bpc limitations obtained from edid. + */ + u8 max_bpc; }; /** @@ -924,6 +936,12 @@ struct drm_connector { */ struct drm_property_blob *path_blob_ptr; + /** + * @max_bpc_property: Default connector property for the max bpc to be + * driven out of the connector. + */ + struct drm_property *max_bpc_property; + #define DRM_CONNECTOR_POLL_HPD (1 << 0) #define DRM_CONNECTOR_POLL_CONNECT (1 << 1) #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2) @@ -1201,6 +1219,8 @@ void drm_connector_set_link_status_property(struct drm_connector *connector, uint64_t link_status); int drm_connector_init_panel_orientation_property( struct drm_connector *connector, int width, int height); +int drm_connector_attach_max_bpc_property(struct drm_connector *connector, + int min, int max); /** * struct drm_tile_group - Tile group metadata
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. 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 V4: Split drm and i915 components(Ville) V5: Make the property per connector(Ville) V6: Compare the requested bpc to connector bpc(Daniel) Move the attach_property function to core(Ville) V7: Fix checkpatch warnings V8: Simplify the connector check code(Ville) V9: Const display_info(Ville) V10: Fix CI issues. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> 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> Cc: Sunpeng Li <sunpeng.li@amd.com> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> --- drivers/gpu/drm/drm_atomic.c | 5 +++++ drivers/gpu/drm/drm_atomic_helper.c | 4 ++++ drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++ drivers/gpu/drm/drm_connector.c | 33 +++++++++++++++++++++++++++++++++ include/drm/drm_connector.h | 20 ++++++++++++++++++++ 5 files changed, 66 insertions(+)