> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> Sent: Thursday, October 19, 2017 7:09 AM
> To: Jani Nikula <jani.nik...@linux.intel.com>
> Cc: Daniel Vetter <dan...@ffwll.ch>; Sripada, Radhakrishna
> <radhakrishna.srip...@intel.com>; intel-gfx@lists.freedesktop.org; Zanoni,
> Paulo R <paulo.r.zan...@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 <dan...@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.syrj...@linux.intel.com>
> > >> Cc: Paulo Zanoni <paulo.r.zan...@intel.com>
> > >> Cc: Manasi Navare <manasi.d.nav...@intel.com>
> > >> Signed-off-by: Radhakrishna Sripada
> > >> <radhakrishna.srip...@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
