On Mon, Mar 13, 2017 at 01:09:10PM +0200, Sharma, Shashank wrote: > Regards > > Shashank > > > On 3/13/2017 12:53 PM, Ville Syrjälä wrote: > > On Mon, Mar 13, 2017 at 12:22:53PM +0200, Sharma, Shashank wrote: > >> Regards > >> > >> Shashank > >> > >>> From: Ville Syrjälä <[email protected]> > >>> > >>> 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: [email protected] > >>> Cc: Nicholas Sielicki <[email protected]> > >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99250 > >>> Signed-off-by: Ville Syrjälä <[email protected]> > >>> --- > >>> 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. > Makes sense :-). > >>> + 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. > I guess just in this scenario, the max bpc value(12) and the value of > our interest is same, so it was making > sense, but I agree, would not be true for all cases like max 16bpc. > > Please feel free to use: > Reviewed-by: Shashank Sharma <[email protected]>
Thanks. Patch pushed to dinq. > > - Shashank > > > >> - Shashank > >>> + } > >>> + > >>> + return true; > >>> } > >>> > >>> bool intel_hdmi_compute_config(struct intel_encoder *encoder, > >>> -- > >>> 2.10.2 > >>> > >>> _______________________________________________ > >>> dri-devel mailing list > >>> [email protected] > >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
