> -----Original Message----- > From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx] > Sent: Thursday, October 19, 2017 7:09 AM > To: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> > Cc: Daniel Vetter <daniel@xxxxxxxx>; Sripada, Radhakrishna > <radhakrishna.sripada@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Zanoni, > Paulo R <paulo.r.zanoni@xxxxxxxxx> > Subject: Re: [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@xxxxxxxx> 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@xxxxxxxxxxxxxxx> > > >> Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > > >> Cc: Manasi Navare <manasi.d.navare@xxxxxxxxx> > > >> Signed-off-by: Radhakrishna Sripada > > >> <radhakrishna.sripada@xxxxxxxxx> > > > > > > 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@xxxxxxxxxxxxxxxxxxxxx > > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Jani Nikula, Intel Open Source Technology Center > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
