Re: [PATCH v1] Added max_bpp property to limit maximum bpp even if HDMI TV advertises higher limit.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2018-08-08 at 16:41 -0700, Rodrigo Vivi wrote:
> On Thu, Jul 26, 2018 at 04:02:53PM +0300, StanLis wrote:
> > From: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>
> > 
> > This was inspired, by a bugs like this:
> > 
> > Bugzilla: https://bugs.freedesktop.org/93361
> > 
> > In short, when TV advertises 12bpc, the refresh
> > rate might get inaccurate causing some playback
> > issues.
> > 
> > The temporary solution was to hack the EDID,
> > so that it doesn't advertise deep color, so this
> > new property makes it unnecessary.
> > 
> > Long term solution would be to change clock frequency,
> > however this affects all the clock tree and might not
> > be doable, so this is the currently proposed fix.
> 
> It seems that the fix for this should be on the refresh rate side,
> not providing an option to the user to workaround and hide the issue.

Correct. I mentioned this in the commit message. However,
after discussion with Ville we decided that this is way to go 
at least for now, as refresh rate side fix would require changing
the whole implementation of finding best pll and clock divisor
algorithm, which will also affect all of the other connected outputs.
Currently as I understood it's not even clear, if this is doable at
all. I can of course ask again, if we still stick to this option or
should I start looking at implementing a real fix.

> 
> > 
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h     |  1 +
> >  drivers/gpu/drm/i915/intel_atomic.c |  8 ++++++++
> >  drivers/gpu/drm/i915/intel_drv.h    |  4 ++++
> >  drivers/gpu/drm/i915/intel_hdmi.c   | 13 ++++++++-----
> >  drivers/gpu/drm/i915/intel_modes.c  | 20 ++++++++++++++++++++
> >  5 files changed, 41 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index eeb002a47032..823eccd29f70 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1817,6 +1817,7 @@ struct drm_i915_private {
> >  	struct intel_fbdev *fbdev;
> >  	struct work_struct fbdev_suspend_work;
> >  
> > +	struct drm_property *max_bpp_property;
> >  	struct drm_property *broadcast_rgb_property;
> >  	struct drm_property *force_audio_property;
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> > b/drivers/gpu/drm/i915/intel_atomic.c
> > index b04952bacf77..ac890fda8029 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->max_bpp_property)
> > +		*val = intel_conn_state->max_bpp;
> >  	else {
> >  		DRM_DEBUG_ATOMIC("Unknown property
> > [PROP:%d:%s]\n",
> >  				 property->base.id, property-
> > >name);
> > @@ -96,6 +98,11 @@ int
> > intel_digital_connector_atomic_set_property(struct drm_connector
> > *connector,
> >  		return 0;
> >  	}
> >  
> > +	if (property == dev_priv->max_bpp_property) {
> > +		intel_conn_state->max_bpp = val;
> > +		return 0;
> > +	}
> > +
> >  	DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
> >  			 property->base.id, property->name);
> >  	return -EINVAL;
> > @@ -126,6 +133,7 @@ int intel_digital_connector_atomic_check(struct
> > drm_connector *conn,
> >  	if (new_conn_state->force_audio != old_conn_state-
> > >force_audio ||
> >  	    new_conn_state->broadcast_rgb != old_conn_state-
> > >broadcast_rgb ||
> >  	    new_conn_state->base.picture_aspect_ratio !=
> > old_conn_state->base.picture_aspect_ratio ||
> > +	    new_conn_state->max_bpp != old_conn_state->max_bpp ||
> >  	    new_conn_state->base.content_type != old_conn_state-
> > >base.content_type ||
> >  	    new_conn_state->base.scaling_mode != old_conn_state-
> > >base.scaling_mode)
> >  		crtc_state->mode_changed = true;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 61e715ddd0d5..38d1f430b44f 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -422,6 +422,7 @@ struct intel_digital_connector_state {
> >  
> >  	enum hdmi_force_audio force_audio;
> >  	int broadcast_rgb;
> > +	int max_bpp;
> >  };
> >  
> >  #define to_intel_digital_connector_state(x) container_of(x, struct
> > intel_digital_connector_state, base)
> > @@ -1837,9 +1838,12 @@ bool intel_is_dual_link_lvds(struct
> > drm_device *dev);
> >  int intel_connector_update_modes(struct drm_connector *connector,
> >  				 struct edid *edid);
> >  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_max_bpp_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 8363fbd18ee8..323b96f5eac7 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -1748,15 +1748,16 @@ bool intel_hdmi_compute_config(struct
> > intel_encoder *encoder,
> >  	 */
> >  	if (hdmi_deep_color_possible(pipe_config, 12) &&
> >  	    hdmi_port_clock_valid(intel_hdmi, clock_12bpc,
> > -				  true, force_dvi) == MODE_OK) {
> > +				  true, force_dvi) == MODE_OK &&
> > +		intel_conn_state->max_bpp >= 12 * 3) {
> >  		DRM_DEBUG_KMS("picking bpc to 12 for HDMI
> > output\n");
> > -		desired_bpp = 12*3;
> > -
> > +		desired_bpp = 12 * 3;
> >  		/* Need to adjust the port link by 1.5x for 12bpc.
> > */
> >  		pipe_config->port_clock = clock_12bpc;
> >  	} else if (hdmi_deep_color_possible(pipe_config, 10) &&
> >  		   hdmi_port_clock_valid(intel_hdmi, clock_10bpc,
> > -					 true, force_dvi) ==
> > MODE_OK) {
> > +					 true, force_dvi) ==
> > MODE_OK &&
> > +		   intel_conn_state->max_bpp >= 10 * 3) {
> >  		DRM_DEBUG_KMS("picking bpc to 10 for HDMI
> > output\n");
> >  		desired_bpp = 10 * 3;
> >  
> > @@ -1764,7 +1765,7 @@ bool intel_hdmi_compute_config(struct
> > intel_encoder *encoder,
> >  		pipe_config->port_clock = clock_10bpc;
> >  	} else {
> >  		DRM_DEBUG_KMS("picking bpc to 8 for HDMI
> > output\n");
> > -		desired_bpp = 8*3;
> > +		desired_bpp = 8 * 3;
> >  
> >  		pipe_config->port_clock = clock_8bpc;
> >  	}
> > @@ -2096,8 +2097,10 @@ 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_max_bpp_property(connector);
> >  	drm_connector_attach_content_type_property(connector);
> >  	connector->state->picture_aspect_ratio =
> > HDMI_PICTURE_ASPECT_NONE;
> > +	to_intel_digital_connector_state(connector->state)-
> > >max_bpp = 8*3;
> >  }
> >  
> >  /*
> > diff --git a/drivers/gpu/drm/i915/intel_modes.c
> > b/drivers/gpu/drm/i915/intel_modes.c
> > index b39846613e3c..6f2ff2714c68 100644
> > --- a/drivers/gpu/drm/i915/intel_modes.c
> > +++ b/drivers/gpu/drm/i915/intel_modes.c
> > @@ -97,6 +97,26 @@ intel_attach_force_audio_property(struct
> > drm_connector *connector)
> >  	drm_object_attach_property(&connector->base, prop, 0);
> >  }
> >  
> > +void
> > +intel_attach_force_max_bpp_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->max_bpp_property;
> > +	if (prop == NULL) {
> > +		prop = drm_property_create_range(dev, 0,
> > +			"Max bpp", 0, 12*3);
> > +		if (prop == NULL)
> > +			return;
> > +
> > +		dev_priv->max_bpp_property = prop;
> > +	}
> > +
> > +	drm_object_attach_property(&connector->base, prop, 8*3);
> > +}
> > +
> >  static const struct drm_prop_enum_list broadcast_rgb_names[] = {
> >  	{ INTEL_BROADCAST_RGB_AUTO, "Automatic" },
> >  	{ INTEL_BROADCAST_RGB_FULL, "Full" },
> > -- 
> > 2.17.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- 
Best Regards,

Lisovskiy Stanislav
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux