Re: [PATCH 3/3] drm/i915: Make infoframe trnsmission more reliable on g4x

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Feb 10, 2014 at 02:40:07PM +0200, Ville Syrjälä wrote:
> On Mon, Feb 10, 2014 at 12:16:14PM +0000, Damien Lespiau wrote:
> > On Thu, Jan 23, 2014 at 11:15:35PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > On g4x there's just one video DIP, but there can be two HDMI/DVI
> > > ports. Currently even a DVI monitor on another port can clobber
> > > the infoframes meant for a real HDMI monitor on the other port.
> > > 
> > > Make sure we only ever send DIPs to one port. The first port with a
> > > HDMI sink to get there gets to own the DIP until such time that the
> > > port is completely turned off (ie. not just DPMS off). If there are
> > > two HDMI sinks attached, the one to arrive second doesn't get any
> > > infoframes. And if there's a DVI sink on the other port, it will
> > > no longer disable DIP transmission for the other port.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
> > 
> > I guess follow-up patches could turn off Video DIPs on other platforms,
> > if I read the code correctly we're always leaving Video DIPs on when
> > disabling the pipe.
> 
> Yeah. Actually I realized afterwards that that's also a problem if we
> switch from say HDMI->DP on the same transcoder. We might end up
> sending infoframes to the DP sink that we constructed for the HDMI sink.
> Or if the video DIP has port select bits (not all platforms do), then
> I think we might even end up sending infoframes from some transcoder to
> a port that isn't connected to said transcoder. I have no idea if the
> hardware can even do that, but it would seem safer not to program it
> that way in the first place.
> 
> But the .off hook isn't sufficient to fix that since we could switch the
> output without calling the .off hook. I think the real solution would to
> always disable infoframes in hdmi disable or post_disable, and always
> re-enabling them in enable or pre_enable.
> 
> That would require changing the new g4x logic you just reviewed since
> we couldn't track the owner of the video DIP by the register contents
> anymore. So I think we'd need to add a piece of software state for that
> purpose instead.
> 
> If anyone's looking for a relatively straightforward small project,
> this would seem to fit the bill.

Or plan B:
1. Add infoframe tracking to the pipe_config.
2. Use the new ->new_config pointers to arbitrate on g4x/vlv and clear the
pipe_config->has_infoframes bit if more than one pipe needs. Yeah that
means if you disable the 1st pipe the 2nd won't automatically get
infoframes, but a) meh b) we can fix that once we have atomic modeset and
recompute the config always. So a FIXME comment should be good enough.
3. Lock it down with infoframes hw readout support. Make that also check
that the infoframes are correct (i.e. our own), which gives us
infoframe-correct fastboot support.

Now the ->off hook in this series moves into another direction entirely,
so I'm not sure whether I should merge it. Also we don't seem to have bug
reports about this really, probably due to the lack of state checking ;-)
Suggestions highly welcome.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch