| Message ID | 20171018222949.11140-1-radhakrishna.sripada@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Oct 18, 2017 at 03:29:48PM -0700, Sripada, Radhakrishna wrote: > Currently the user space does not have a way to force the bpc. The > default bpc to be programmed is decided by the driver and is run > against connector limitations. Creating a new property for the userspace > to recommend a certain color depth while scanning out the pixels. > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: Manasi Navare <manasi.d.navare@intel.com> > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> Why not put this onto the pipe directly? Also, what's the userspace use-case here, we need those patches too. -Daniel > --- > drivers/gpu/drm/i915/i915_drv.h | 7 +++++++ > drivers/gpu/drm/i915/intel_atomic.c | 7 +++++++ > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_hdmi.c | 1 + > drivers/gpu/drm/i915/intel_modes.c | 29 +++++++++++++++++++++++++++++ > 5 files changed, 46 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 2783f07c6fc4..e58856f7b08a 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2521,6 +2521,7 @@ struct drm_i915_private { > > struct drm_property *broadcast_rgb_property; > struct drm_property *force_audio_property; > + struct drm_property *force_bpc_property; > > /* hda/i915 audio component */ > struct i915_audio_component *audio_component; > @@ -2858,6 +2859,12 @@ enum hdmi_force_audio { > HDMI_AUDIO_ON, /* force turn on HDMI audio */ > }; > > +enum connector_force_bpc { > + INTEL_FORCE_BPC_AUTO, > + INTEL_FORCE_BPC_8, > + INTEL_FORCE_BPC_10, > + INTEL_FORCE_BPC_12, > +}; > #define I915_GTT_OFFSET_NONE ((u32)-1) > > /* > diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c > index 36d4e635e4ce..10a74669f49a 100644 > --- a/drivers/gpu/drm/i915/intel_atomic.c > +++ b/drivers/gpu/drm/i915/intel_atomic.c > @@ -58,6 +58,8 @@ int intel_digital_connector_atomic_get_property(struct drm_connector *connector, > *val = intel_conn_state->force_audio; > else if (property == dev_priv->broadcast_rgb_property) > *val = intel_conn_state->broadcast_rgb; > + else if (property == dev_priv->force_bpc_property) > + *val = intel_conn_state->force_bpc; > else { > DRM_DEBUG_ATOMIC("Unknown property %s\n", property->name); > return -EINVAL; > @@ -95,6 +97,11 @@ int intel_digital_connector_atomic_set_property(struct drm_connector *connector, > return 0; > } > > + if (property == dev_priv->force_bpc_property) { > + intel_conn_state->force_bpc = val; > + return 0; > + } > + > DRM_DEBUG_ATOMIC("Unknown property %s\n", property->name); > return -EINVAL; > } > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 77117e188b04..654e76d19514 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -341,6 +341,7 @@ struct intel_digital_connector_state { > > enum hdmi_force_audio force_audio; > int broadcast_rgb; > + enum connector_force_bpc force_bpc; > }; > > #define to_intel_digital_connector_state(x) container_of(x, struct intel_digital_connector_state, base) > @@ -1708,6 +1709,7 @@ 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_force_bpc_property(struct drm_connector *connector); > > > /* intel_overlay.c */ > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > index 50214e342401..a2d6d97cc730 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -1798,6 +1798,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c > intel_attach_force_audio_property(connector); > intel_attach_broadcast_rgb_property(connector); > intel_attach_aspect_ratio_property(connector); > + intel_attach_force_bpc_property(connector); > connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; > } > > diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c > index 28a778b785ac..d7be44a951e8 100644 > --- a/drivers/gpu/drm/i915/intel_modes.c > +++ b/drivers/gpu/drm/i915/intel_modes.c > @@ -151,3 +151,32 @@ intel_attach_aspect_ratio_property(struct drm_connector *connector) > connector->dev->mode_config.aspect_ratio_property, > DRM_MODE_PICTURE_ASPECT_NONE); > } > + > +static const struct drm_prop_enum_list force_bpc_names[] = { > + { INTEL_FORCE_BPC_AUTO, "Automatic" }, > + { INTEL_FORCE_BPC_8, "8 bpc" }, > + { INTEL_FORCE_BPC_10, "10 bpc" }, > + { INTEL_FORCE_BPC_12, "12 bpc" }, > +}; > + > +void > +intel_attach_force_bpc_property(struct drm_connector *connector) > +{ > + struct drm_device *dev = connector->dev; > + struct drm_i915_private *dev_priv = to_i915(dev); > + struct drm_property *prop; > + > + prop = dev_priv->force_bpc_property; > + if (prop == NULL) { > + prop = drm_property_create_enum(dev, DRM_MODE_PROP_ENUM, > + "bpc", > + force_bpc_names, > + ARRAY_SIZE(force_bpc_names)); > + if (prop == NULL) > + return; > + > + dev_priv->force_bpc_property = prop; > + } > + > + drm_object_attach_property(&connector->base, prop, 0); > +} > -- > 2.9.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, 19 Oct 2017, Daniel Vetter <daniel@ffwll.ch> wrote: > On Wed, Oct 18, 2017 at 03:29:48PM -0700, Sripada, Radhakrishna wrote: >> Currently the user space does not have a way to force the bpc. The >> default bpc to be programmed is decided by the driver and is run >> against connector limitations. Creating a new property for the userspace >> to recommend a certain color depth while scanning out the pixels. >> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> >> Cc: Manasi Navare <manasi.d.navare@intel.com> >> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> > > Why not put this onto the pipe directly? Also, what's the userspace > use-case here, we need those patches too. And rationale in the commit message, please. The "why". BR, Jani. > -Daniel > >> --- >> drivers/gpu/drm/i915/i915_drv.h | 7 +++++++ >> drivers/gpu/drm/i915/intel_atomic.c | 7 +++++++ >> drivers/gpu/drm/i915/intel_drv.h | 2 ++ >> drivers/gpu/drm/i915/intel_hdmi.c | 1 + >> drivers/gpu/drm/i915/intel_modes.c | 29 +++++++++++++++++++++++++++++ >> 5 files changed, 46 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index 2783f07c6fc4..e58856f7b08a 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -2521,6 +2521,7 @@ struct drm_i915_private { >> >> struct drm_property *broadcast_rgb_property; >> struct drm_property *force_audio_property; >> + struct drm_property *force_bpc_property; >> >> /* hda/i915 audio component */ >> struct i915_audio_component *audio_component; >> @@ -2858,6 +2859,12 @@ enum hdmi_force_audio { >> HDMI_AUDIO_ON, /* force turn on HDMI audio */ >> }; >> >> +enum connector_force_bpc { >> + INTEL_FORCE_BPC_AUTO, >> + INTEL_FORCE_BPC_8, >> + INTEL_FORCE_BPC_10, >> + INTEL_FORCE_BPC_12, >> +}; >> #define I915_GTT_OFFSET_NONE ((u32)-1) >> >> /* >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c >> index 36d4e635e4ce..10a74669f49a 100644 >> --- a/drivers/gpu/drm/i915/intel_atomic.c >> +++ b/drivers/gpu/drm/i915/intel_atomic.c >> @@ -58,6 +58,8 @@ int intel_digital_connector_atomic_get_property(struct drm_connector *connector, >> *val = intel_conn_state->force_audio; >> else if (property == dev_priv->broadcast_rgb_property) >> *val = intel_conn_state->broadcast_rgb; >> + else if (property == dev_priv->force_bpc_property) >> + *val = intel_conn_state->force_bpc; >> else { >> DRM_DEBUG_ATOMIC("Unknown property %s\n", property->name); >> return -EINVAL; >> @@ -95,6 +97,11 @@ int intel_digital_connector_atomic_set_property(struct drm_connector *connector, >> return 0; >> } >> >> + if (property == dev_priv->force_bpc_property) { >> + intel_conn_state->force_bpc = val; >> + return 0; >> + } >> + >> DRM_DEBUG_ATOMIC("Unknown property %s\n", property->name); >> return -EINVAL; >> } >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >> index 77117e188b04..654e76d19514 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -341,6 +341,7 @@ struct intel_digital_connector_state { >> >> enum hdmi_force_audio force_audio; >> int broadcast_rgb; >> + enum connector_force_bpc force_bpc; >> }; >> >> #define to_intel_digital_connector_state(x) container_of(x, struct intel_digital_connector_state, base) >> @@ -1708,6 +1709,7 @@ 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_force_bpc_property(struct drm_connector *connector); >> >> >> /* intel_overlay.c */ >> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c >> index 50214e342401..a2d6d97cc730 100644 >> --- a/drivers/gpu/drm/i915/intel_hdmi.c >> +++ b/drivers/gpu/drm/i915/intel_hdmi.c >> @@ -1798,6 +1798,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c >> intel_attach_force_audio_property(connector); >> intel_attach_broadcast_rgb_property(connector); >> intel_attach_aspect_ratio_property(connector); >> + intel_attach_force_bpc_property(connector); >> connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; >> } >> >> diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c >> index 28a778b785ac..d7be44a951e8 100644 >> --- a/drivers/gpu/drm/i915/intel_modes.c >> +++ b/drivers/gpu/drm/i915/intel_modes.c >> @@ -151,3 +151,32 @@ intel_attach_aspect_ratio_property(struct drm_connector *connector) >> connector->dev->mode_config.aspect_ratio_property, >> DRM_MODE_PICTURE_ASPECT_NONE); >> } >> + >> +static const struct drm_prop_enum_list force_bpc_names[] = { >> + { INTEL_FORCE_BPC_AUTO, "Automatic" }, >> + { INTEL_FORCE_BPC_8, "8 bpc" }, >> + { INTEL_FORCE_BPC_10, "10 bpc" }, >> + { INTEL_FORCE_BPC_12, "12 bpc" }, >> +}; >> + >> +void >> +intel_attach_force_bpc_property(struct drm_connector *connector) >> +{ >> + struct drm_device *dev = connector->dev; >> + struct drm_i915_private *dev_priv = to_i915(dev); >> + struct drm_property *prop; >> + >> + prop = dev_priv->force_bpc_property; >> + if (prop == NULL) { >> + prop = drm_property_create_enum(dev, DRM_MODE_PROP_ENUM, >> + "bpc", >> + force_bpc_names, >> + ARRAY_SIZE(force_bpc_names)); >> + if (prop == NULL) >> + return; >> + >> + dev_priv->force_bpc_property = prop; >> + } >> + >> + drm_object_attach_property(&connector->base, prop, 0); >> +} >> -- >> 2.9.3 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Oct 19, 2017 at 03:56:37PM +0300, Jani Nikula wrote: > On Thu, 19 Oct 2017, Daniel Vetter <daniel@ffwll.ch> wrote: > > On Wed, Oct 18, 2017 at 03:29:48PM -0700, Sripada, Radhakrishna wrote: > >> Currently the user space does not have a way to force the bpc. The > >> default bpc to be programmed is decided by the driver and is run > >> against connector limitations. Creating a new property for the userspace > >> to recommend a certain color depth while scanning out the pixels. > >> > >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > >> Cc: Manasi Navare <manasi.d.navare@intel.com> > >> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> > > > > Why not put this onto the pipe directly? Also, what's the userspace > > use-case here, we need those patches too. > > And rationale in the commit message, please. The "why". One rationale is cruddy cables/dongles/level shifters/misprogrammed buf translations/etc. which fail when we try to push 12bpc HDMI through them. And to work around those we probably want this to be a connector property so that xrandr will get it automagically. Won't help fbdev of course, but for that we'd either need a modparam, or maybe some generic way to expose properties via sysfs or something. See eg. https://bugs.freedesktop.org/show_bug.cgi?id=91434 for likely 12bpc issues. As far as the implementation goes, I'd probably just make it range property ("max bpc" or something like that) with some sane range of values (8-16?, or maybe we need to go lower?), with the default value being the max. > > BR, > Jani. > > > > > -Daniel > > > >> --- > >> drivers/gpu/drm/i915/i915_drv.h | 7 +++++++ > >> drivers/gpu/drm/i915/intel_atomic.c | 7 +++++++ > >> drivers/gpu/drm/i915/intel_drv.h | 2 ++ > >> drivers/gpu/drm/i915/intel_hdmi.c | 1 + > >> drivers/gpu/drm/i915/intel_modes.c | 29 +++++++++++++++++++++++++++++ > >> 5 files changed, 46 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > >> index 2783f07c6fc4..e58856f7b08a 100644 > >> --- a/drivers/gpu/drm/i915/i915_drv.h > >> +++ b/drivers/gpu/drm/i915/i915_drv.h > >> @@ -2521,6 +2521,7 @@ struct drm_i915_private { > >> > >> struct drm_property *broadcast_rgb_property; > >> struct drm_property *force_audio_property; > >> + struct drm_property *force_bpc_property; > >> > >> /* hda/i915 audio component */ > >> struct i915_audio_component *audio_component; > >> @@ -2858,6 +2859,12 @@ enum hdmi_force_audio { > >> HDMI_AUDIO_ON, /* force turn on HDMI audio */ > >> }; > >> > >> +enum connector_force_bpc { > >> + INTEL_FORCE_BPC_AUTO, > >> + INTEL_FORCE_BPC_8, > >> + INTEL_FORCE_BPC_10, > >> + INTEL_FORCE_BPC_12, > >> +}; > >> #define I915_GTT_OFFSET_NONE ((u32)-1) > >> > >> /* > >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c > >> index 36d4e635e4ce..10a74669f49a 100644 > >> --- a/drivers/gpu/drm/i915/intel_atomic.c > >> +++ b/drivers/gpu/drm/i915/intel_atomic.c > >> @@ -58,6 +58,8 @@ int intel_digital_connector_atomic_get_property(struct drm_connector *connector, > >> *val = intel_conn_state->force_audio; > >> else if (property == dev_priv->broadcast_rgb_property) > >> *val = intel_conn_state->broadcast_rgb; > >> + else if (property == dev_priv->force_bpc_property) > >> + *val = intel_conn_state->force_bpc; > >> else { > >> DRM_DEBUG_ATOMIC("Unknown property %s\n", property->name); > >> return -EINVAL; > >> @@ -95,6 +97,11 @@ int intel_digital_connector_atomic_set_property(struct drm_connector *connector, > >> return 0; > >> } > >> > >> + if (property == dev_priv->force_bpc_property) { > >> + intel_conn_state->force_bpc = val; > >> + return 0; > >> + } > >> + > >> DRM_DEBUG_ATOMIC("Unknown property %s\n", property->name); > >> return -EINVAL; > >> } > >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > >> index 77117e188b04..654e76d19514 100644 > >> --- a/drivers/gpu/drm/i915/intel_drv.h > >> +++ b/drivers/gpu/drm/i915/intel_drv.h > >> @@ -341,6 +341,7 @@ struct intel_digital_connector_state { > >> > >> enum hdmi_force_audio force_audio; > >> int broadcast_rgb; > >> + enum connector_force_bpc force_bpc; > >> }; > >> > >> #define to_intel_digital_connector_state(x) container_of(x, struct intel_digital_connector_state, base) > >> @@ -1708,6 +1709,7 @@ 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_force_bpc_property(struct drm_connector *connector); > >> > >> > >> /* intel_overlay.c */ > >> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > >> index 50214e342401..a2d6d97cc730 100644 > >> --- a/drivers/gpu/drm/i915/intel_hdmi.c > >> +++ b/drivers/gpu/drm/i915/intel_hdmi.c > >> @@ -1798,6 +1798,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c > >> intel_attach_force_audio_property(connector); > >> intel_attach_broadcast_rgb_property(connector); > >> intel_attach_aspect_ratio_property(connector); > >> + intel_attach_force_bpc_property(connector); > >> connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; > >> } > >> > >> diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c > >> index 28a778b785ac..d7be44a951e8 100644 > >> --- a/drivers/gpu/drm/i915/intel_modes.c > >> +++ b/drivers/gpu/drm/i915/intel_modes.c > >> @@ -151,3 +151,32 @@ intel_attach_aspect_ratio_property(struct drm_connector *connector) > >> connector->dev->mode_config.aspect_ratio_property, > >> DRM_MODE_PICTURE_ASPECT_NONE); > >> } > >> + > >> +static const struct drm_prop_enum_list force_bpc_names[] = { > >> + { INTEL_FORCE_BPC_AUTO, "Automatic" }, > >> + { INTEL_FORCE_BPC_8, "8 bpc" }, > >> + { INTEL_FORCE_BPC_10, "10 bpc" }, > >> + { INTEL_FORCE_BPC_12, "12 bpc" }, > >> +}; > >> + > >> +void > >> +intel_attach_force_bpc_property(struct drm_connector *connector) > >> +{ > >> + struct drm_device *dev = connector->dev; > >> + struct drm_i915_private *dev_priv = to_i915(dev); > >> + struct drm_property *prop; > >> + > >> + prop = dev_priv->force_bpc_property; > >> + if (prop == NULL) { > >> + prop = drm_property_create_enum(dev, DRM_MODE_PROP_ENUM, > >> + "bpc", > >> + force_bpc_names, > >> + ARRAY_SIZE(force_bpc_names)); > >> + if (prop == NULL) > >> + return; > >> + > >> + dev_priv->force_bpc_property = prop; > >> + } > >> + > >> + drm_object_attach_property(&connector->base, prop, 0); > >> +} > >> -- > >> 2.9.3 > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Jani Nikula, Intel Open Source Technology Center > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> -----Original Message----- > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > Sent: Thursday, October 19, 2017 7:09 AM > To: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Daniel Vetter <daniel@ffwll.ch>; Sripada, Radhakrishna > <radhakrishna.sripada@intel.com>; intel-gfx@lists.freedesktop.org; Zanoni, > Paulo R <paulo.r.zanoni@intel.com> > Subject: Re: [Intel-gfx] [RFC 1/2] drm/i915: Add connector property to force > bpc > > On Thu, Oct 19, 2017 at 03:56:37PM +0300, Jani Nikula wrote: > > On Thu, 19 Oct 2017, Daniel Vetter <daniel@ffwll.ch> wrote: > > > On Wed, Oct 18, 2017 at 03:29:48PM -0700, Sripada, Radhakrishna wrote: > > >> Currently the user space does not have a way to force the bpc. The > > >> default bpc to be programmed is decided by the driver and is run > > >> against connector limitations. Creating a new property for the > > >> userspace to recommend a certain color depth while scanning out the > pixels. > > >> > > >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > >> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > >> Cc: Manasi Navare <manasi.d.navare@intel.com> > > >> Signed-off-by: Radhakrishna Sripada > > >> <radhakrishna.sripada@intel.com> > > > > > > Why not put this onto the pipe directly? Also, what's the userspace > > > use-case here, we need those patches too. > > > > And rationale in the commit message, please. The "why". > > One rationale is cruddy cables/dongles/level shifters/misprogrammed buf > translations/etc. which fail when we try to push 12bpc HDMI through them. > And to work around those we probably want this to be a connector property > so that xrandr will get it automagically. Won't help fbdev of course, but for > that we'd either need a modparam, or maybe some generic way to expose > properties via sysfs or something. > > See eg. https://bugs.freedesktop.org/show_bug.cgi?id=91434 for likely > 12bpc issues. > > As far as the implementation goes, I'd probably just make it range property > ("max bpc" or something like that) with some sane range of values (8-16?, or > maybe we need to go lower?), with the default value being the max. Thank you for the feedback. I will incorporate the changes in V1 of the patch. Regards, Radhakrishna(RK) > > > > > BR, > > Jani. > > > > > > > > > -Daniel > > > > > >> --- > > >> drivers/gpu/drm/i915/i915_drv.h | 7 +++++++ > > >> drivers/gpu/drm/i915/intel_atomic.c | 7 +++++++ > > >> drivers/gpu/drm/i915/intel_drv.h | 2 ++ > > >> drivers/gpu/drm/i915/intel_hdmi.c | 1 + > > >> drivers/gpu/drm/i915/intel_modes.c | 29 > > >> +++++++++++++++++++++++++++++ > > >> 5 files changed, 46 insertions(+) > > >> > > >> diff --git a/drivers/gpu/drm/i915/i915_drv.h > > >> b/drivers/gpu/drm/i915/i915_drv.h index 2783f07c6fc4..e58856f7b08a > > >> 100644 > > >> --- a/drivers/gpu/drm/i915/i915_drv.h > > >> +++ b/drivers/gpu/drm/i915/i915_drv.h > > >> @@ -2521,6 +2521,7 @@ struct drm_i915_private { > > >> > > >> struct drm_property *broadcast_rgb_property; > > >> struct drm_property *force_audio_property; > > >> + struct drm_property *force_bpc_property; > > >> > > >> /* hda/i915 audio component */ > > >> struct i915_audio_component *audio_component; @@ -2858,6 > +2859,12 > > >> @@ enum hdmi_force_audio { > > >> HDMI_AUDIO_ON, /* force turn on HDMI audio > */ > > >> }; > > >> > > >> +enum connector_force_bpc { > > >> + INTEL_FORCE_BPC_AUTO, > > >> + INTEL_FORCE_BPC_8, > > >> + INTEL_FORCE_BPC_10, > > >> + INTEL_FORCE_BPC_12, > > >> +}; > > >> #define I915_GTT_OFFSET_NONE ((u32)-1) > > >> > > >> /* > > >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c > > >> b/drivers/gpu/drm/i915/intel_atomic.c > > >> index 36d4e635e4ce..10a74669f49a 100644 > > >> --- a/drivers/gpu/drm/i915/intel_atomic.c > > >> +++ b/drivers/gpu/drm/i915/intel_atomic.c > > >> @@ -58,6 +58,8 @@ int > intel_digital_connector_atomic_get_property(struct drm_connector > *connector, > > >> *val = intel_conn_state->force_audio; > > >> else if (property == dev_priv->broadcast_rgb_property) > > >> *val = intel_conn_state->broadcast_rgb; > > >> + else if (property == dev_priv->force_bpc_property) > > >> + *val = intel_conn_state->force_bpc; > > >> else { > > >> DRM_DEBUG_ATOMIC("Unknown property %s\n", > property->name); > > >> return -EINVAL; > > >> @@ -95,6 +97,11 @@ int > intel_digital_connector_atomic_set_property(struct drm_connector > *connector, > > >> return 0; > > >> } > > >> > > >> + if (property == dev_priv->force_bpc_property) { > > >> + intel_conn_state->force_bpc = val; > > >> + return 0; > > >> + } > > >> + > > >> DRM_DEBUG_ATOMIC("Unknown property %s\n", property- > >name); > > >> return -EINVAL; > > >> } > > >> diff --git a/drivers/gpu/drm/i915/intel_drv.h > > >> b/drivers/gpu/drm/i915/intel_drv.h > > >> index 77117e188b04..654e76d19514 100644 > > >> --- a/drivers/gpu/drm/i915/intel_drv.h > > >> +++ b/drivers/gpu/drm/i915/intel_drv.h > > >> @@ -341,6 +341,7 @@ struct intel_digital_connector_state { > > >> > > >> enum hdmi_force_audio force_audio; > > >> int broadcast_rgb; > > >> + enum connector_force_bpc force_bpc; > > >> }; > > >> > > >> #define to_intel_digital_connector_state(x) container_of(x, struct > > >> intel_digital_connector_state, base) @@ -1708,6 +1709,7 @@ 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_force_bpc_property(struct drm_connector > > >> +*connector); > > >> > > >> > > >> /* intel_overlay.c */ > > >> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > > >> b/drivers/gpu/drm/i915/intel_hdmi.c > > >> index 50214e342401..a2d6d97cc730 100644 > > >> --- a/drivers/gpu/drm/i915/intel_hdmi.c > > >> +++ b/drivers/gpu/drm/i915/intel_hdmi.c > > >> @@ -1798,6 +1798,7 @@ intel_hdmi_add_properties(struct intel_hdmi > *intel_hdmi, struct drm_connector *c > > >> intel_attach_force_audio_property(connector); > > >> intel_attach_broadcast_rgb_property(connector); > > >> intel_attach_aspect_ratio_property(connector); > > >> + intel_attach_force_bpc_property(connector); > > >> connector->state->picture_aspect_ratio = > > >> HDMI_PICTURE_ASPECT_NONE; } > > >> > > >> diff --git a/drivers/gpu/drm/i915/intel_modes.c > > >> b/drivers/gpu/drm/i915/intel_modes.c > > >> index 28a778b785ac..d7be44a951e8 100644 > > >> --- a/drivers/gpu/drm/i915/intel_modes.c > > >> +++ b/drivers/gpu/drm/i915/intel_modes.c > > >> @@ -151,3 +151,32 @@ intel_attach_aspect_ratio_property(struct > drm_connector *connector) > > >> connector->dev- > >mode_config.aspect_ratio_property, > > >> DRM_MODE_PICTURE_ASPECT_NONE); > > >> } > > >> + > > >> +static const struct drm_prop_enum_list force_bpc_names[] = { > > >> + { INTEL_FORCE_BPC_AUTO, "Automatic" }, > > >> + { INTEL_FORCE_BPC_8, "8 bpc" }, > > >> + { INTEL_FORCE_BPC_10, "10 bpc" }, > > >> + { INTEL_FORCE_BPC_12, "12 bpc" }, }; > > >> + > > >> +void > > >> +intel_attach_force_bpc_property(struct drm_connector *connector) { > > >> + struct drm_device *dev = connector->dev; > > >> + struct drm_i915_private *dev_priv = to_i915(dev); > > >> + struct drm_property *prop; > > >> + > > >> + prop = dev_priv->force_bpc_property; > > >> + if (prop == NULL) { > > >> + prop = drm_property_create_enum(dev, > DRM_MODE_PROP_ENUM, > > >> + "bpc", > > >> + force_bpc_names, > > >> + ARRAY_SIZE(force_bpc_names)); > > >> + if (prop == NULL) > > >> + return; > > >> + > > >> + dev_priv->force_bpc_property = prop; > > >> + } > > >> + > > >> + drm_object_attach_property(&connector->base, prop, 0); } > > >> -- > > >> 2.9.3 > > >> > > >> _______________________________________________ > > >> Intel-gfx mailing list > > >> Intel-gfx@lists.freedesktop.org > > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Jani Nikula, Intel Open Source Technology Center > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 2783f07c6fc4..e58856f7b08a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2521,6 +2521,7 @@ struct drm_i915_private { struct drm_property *broadcast_rgb_property; struct drm_property *force_audio_property; + struct drm_property *force_bpc_property; /* hda/i915 audio component */ struct i915_audio_component *audio_component; @@ -2858,6 +2859,12 @@ enum hdmi_force_audio { HDMI_AUDIO_ON, /* force turn on HDMI audio */ }; +enum connector_force_bpc { + INTEL_FORCE_BPC_AUTO, + INTEL_FORCE_BPC_8, + INTEL_FORCE_BPC_10, + INTEL_FORCE_BPC_12, +}; #define I915_GTT_OFFSET_NONE ((u32)-1) /* diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 36d4e635e4ce..10a74669f49a 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -58,6 +58,8 @@ int intel_digital_connector_atomic_get_property(struct drm_connector *connector, *val = intel_conn_state->force_audio; else if (property == dev_priv->broadcast_rgb_property) *val = intel_conn_state->broadcast_rgb; + else if (property == dev_priv->force_bpc_property) + *val = intel_conn_state->force_bpc; else { DRM_DEBUG_ATOMIC("Unknown property %s\n", property->name); return -EINVAL; @@ -95,6 +97,11 @@ int intel_digital_connector_atomic_set_property(struct drm_connector *connector, return 0; } + if (property == dev_priv->force_bpc_property) { + intel_conn_state->force_bpc = val; + return 0; + } + DRM_DEBUG_ATOMIC("Unknown property %s\n", property->name); return -EINVAL; } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 77117e188b04..654e76d19514 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -341,6 +341,7 @@ struct intel_digital_connector_state { enum hdmi_force_audio force_audio; int broadcast_rgb; + enum connector_force_bpc force_bpc; }; #define to_intel_digital_connector_state(x) container_of(x, struct intel_digital_connector_state, base) @@ -1708,6 +1709,7 @@ 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_force_bpc_property(struct drm_connector *connector); /* intel_overlay.c */ diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 50214e342401..a2d6d97cc730 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1798,6 +1798,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c intel_attach_force_audio_property(connector); intel_attach_broadcast_rgb_property(connector); intel_attach_aspect_ratio_property(connector); + intel_attach_force_bpc_property(connector); connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; } diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c index 28a778b785ac..d7be44a951e8 100644 --- a/drivers/gpu/drm/i915/intel_modes.c +++ b/drivers/gpu/drm/i915/intel_modes.c @@ -151,3 +151,32 @@ intel_attach_aspect_ratio_property(struct drm_connector *connector) connector->dev->mode_config.aspect_ratio_property, DRM_MODE_PICTURE_ASPECT_NONE); } + +static const struct drm_prop_enum_list force_bpc_names[] = { + { INTEL_FORCE_BPC_AUTO, "Automatic" }, + { INTEL_FORCE_BPC_8, "8 bpc" }, + { INTEL_FORCE_BPC_10, "10 bpc" }, + { INTEL_FORCE_BPC_12, "12 bpc" }, +}; + +void +intel_attach_force_bpc_property(struct drm_connector *connector) +{ + struct drm_device *dev = connector->dev; + struct drm_i915_private *dev_priv = to_i915(dev); + struct drm_property *prop; + + prop = dev_priv->force_bpc_property; + if (prop == NULL) { + prop = drm_property_create_enum(dev, DRM_MODE_PROP_ENUM, + "bpc", + force_bpc_names, + ARRAY_SIZE(force_bpc_names)); + if (prop == NULL) + return; + + dev_priv->force_bpc_property = prop; + } + + drm_object_attach_property(&connector->base, prop, 0); +}
Currently the user space does not have a way to force the bpc. The default bpc to be programmed is decided by the driver and is run against connector limitations. Creating a new property for the userspace to recommend a certain color depth while scanning out the pixels. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Manasi Navare <manasi.d.navare@intel.com> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 7 +++++++ drivers/gpu/drm/i915/intel_atomic.c | 7 +++++++ drivers/gpu/drm/i915/intel_drv.h | 2 ++ drivers/gpu/drm/i915/intel_hdmi.c | 1 + drivers/gpu/drm/i915/intel_modes.c | 29 +++++++++++++++++++++++++++++ 5 files changed, 46 insertions(+)