Re: [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too v2

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Thu, 20 Nov 2014 16:49:42 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Tue, Nov 18, 2014 at 09:45:52AM -0800, Jesse Barnes wrote:
> > Just like we do in the HDMI code, set the infoframe flag if we detect
> > that infoframes are enabled.
> > 
> > v2: check for actual infoframe status as in hdmi code (Daniel)
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 07c5625..24110c9 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2075,6 +2075,14 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >  		break;
> >  	}
> >  
> > +	if (encoder->type == INTEL_OUTPUT_HDMI) {
> 
> Hm I dind't look too closely apparently at this. You again rely upon sw
> state here, just encoder->type this time around. Which means you can't
> upcast the intel_hdmi struct, and you also can't really rely upon the
> encoder->crtc link (that's all just about to get reconstructed). Imo the
> code should have stayed in the TRANS_DDI_MODE_SELECT_HDMI case.

Hm, we look at the encoder->type later and check for eDP, so I figured
it must be valid at this point.  It's easy enough to move though if
not...

> The later depency upon encoder->crtc is an issue for everything !g4x, but
> on hsw there's the additional issue that you have to look at the cpu
> transcoder and I guess that part blows up.

I'll check out Paulo's trace; haven't looked yet.

> g4x infoframe readout is probably broken too because it doesn't check that
> the port selected is the one actually queried for.

Yeah that looks like a separate bug from the infoframe_enabled
additions.

> 
> Overall I think we need to:
> - Inline the g4x readout into the hdmi get_config function and check the
>   port.
> - Inline the ibx/cpt readout code into the relevant get_pipe_config
>   functions (well pch config) since that state is per-pipe. We should
>   probably double-check the port, too.
> - Same inline for vlv and hsw, with the addition that we need to make sure
>   on hsw to not try to read this for the edp transcoder.

I'll check, we may not need to inline since we should be able to get
the port info easily enough.

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx