On Mon, Mar 13, 2017 at 12:22:53PM +0200, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
> > From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> >
> > Check that the sink really declared 12bpc support before we enable it.
> > This should not actually never happen since it's mandatory for HDMI sinks
> > to support 12bpc if they support any deep color modes. But reality
> > disagrees with the theory and there are actually sinks in the wild that
> > violate the spec.
> >
> > v2: Fix the output_types check
> > Update commit message to state that these things are in fact real
> >
> > Cc: sta...@vger.kernel.org
> > Cc: Nicholas Sielicki <nicholas.sieli...@gmail.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99250
> > Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_hdmi.c | 24 +++++++++++++++++++++---
> > 1 file changed, 21 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> > b/drivers/gpu/drm/i915/intel_hdmi.c
> > index a580de80d2b5..2bf5915284aa 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -1298,16 +1298,34 @@ intel_hdmi_mode_valid(struct drm_connector
> > *connector,
> >
> > static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state) {
> > - struct drm_device *dev = crtc_state->base.crtc->dev;
> > + struct drm_i915_private *dev_priv =
> > + to_i915(crtc_state->base.crtc->dev);
> > + struct drm_atomic_state *state = crtc_state->base.state;
> > + struct drm_connector_state *connector_state;
> > + struct drm_connector *connector;
> > + int i;
> >
> > - if (HAS_GMCH_DISPLAY(to_i915(dev)))
> > + if (HAS_GMCH_DISPLAY(dev_priv))
> > return false;
> >
> > /*
> > * HDMI 12bpc affects the clocks, so it's only possible
> > * when not cloning with other encoder types.
> > */
> > - return crtc_state->output_types == 1 << INTEL_OUTPUT_HDMI;
> > + if (crtc_state->output_types != 1 << INTEL_OUTPUT_HDMI)
> > + return false;
> > +
> This function is called from only one place ( for now), and that already
> has a pipe_config->has_hdmi_sink check.
> Does it makes sense to have only one of the checks ? I can understand
> that this might be to comple
has_hdmi_sink is not the same thing. It just says we're talking to at
least one HDMI sink and thus can send infoframes/audio. output_types on
the other hand lists all the different port types we're cloning with.
So you can do HDMI+VGA for instance and still the HDMI goes to a real
HDMI sink so we'll have has_hdmi_sink==true. But since VGA can't deal
with the 1.5x clock we can't do deep color when cloning.
> > + for_each_connector_in_state(state, connector, connector_state, i) {
> > + const struct drm_display_info *info = &connector->display_info;
> > +
> > + if (connector_state->crtc != crtc_state->base.crtc)
> > + continue;
> > +
> > + if ((info->edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_36) == 0)
> > + return false;
> Do we want to have a info->bpc check too, or they are more or less same ?
That was already checked at the start of the state computation. So
state->pipe_bpp already accounts for that. However as info->bpc is just
the max bpc the sink can do it's not sufficient to guarantee it can doo
all lower bpc values.
> - Shashank
> > + }
> > +
> > + return true;
> > }
> >
> > bool intel_hdmi_compute_config(struct intel_encoder *encoder,
> > --
> > 2.10.2
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
