Re: [PATCH 16/18] drm/i915/sdvo: Read out HDMI infoframes

From: Daniel Vetter <daniel@ffwll.ch>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 16/18] drm/i915/sdvo: Read out HDMI infoframes
Date: Mon, 1 Oct 2018 18:48:10 +0200	[thread overview]
Message-ID: <20181001164810.GD11082@phenom.ffwll.local> (raw)
In-Reply-To: <20181001133550.GF9144@intel.com>

On Mon, Oct 01, 2018 at 04:35:50PM +0300, Ville Syrjälä wrote:
> On Mon, Oct 01, 2018 at 08:59:41AM +0200, Daniel Vetter wrote:
> > On Mon, Sep 24, 2018 at 08:13:08PM +0300, Ville Syrjälä wrote:
> > > On Mon, Sep 24, 2018 at 06:10:14PM +0200, Daniel Vetter wrote:
> > > > On Thu, Sep 20, 2018 at 09:51:43PM +0300, Ville Syrjala wrote:
> > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > 
> > > > > Read the HDMI infoframes from the hbuf and unpack them into
> > > > > the crtc state.
> > > > > 
> > > > > Well, actually just AVI infoframe for now but let's write the
> > > > > infoframe readout code in a more generic fashion in case we
> > > > > expand this later.
> > > > > 
> > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Hm, caring about sdvo seems a bit overkill. And afaik we don't have any
> > > > sdvo (much less hdmi) in CI. I'm leaning towards just adding a
> > > > PIPE_CONFIG_QUIRK_INFOFRAMES for sdvo, and short-circuiting the checks if
> > > > that's set. Except if you can somehow convince CI folks to add an sdvo
> > > > hdmi card to CI :-)
> > > 
> > > Unfortunately I only have one SDVO HDMI device and it has the chip
> > > straight on the motherboard. I can't give mine up for ci :) I guess
> > > we could try to find another one of those as that model doesn't
> > > even seem super rare. Just the annoying usual problem of getting
> > > one from somewhere approved.
> > > 
> > > I think having to maintain a quirk is ~500% more annoying than
> > > adding the readout code.
> > 
> > My experience disagrees, a bunch of the (early?) sdvo chips don't bother
> > to implement all the get stuff. I had a pile of these (but some of them
> > died, and plugging them all into machines is a pain), and just because it
> > works on 1 chip doesn't mean it's actually all that useful. That's why I
> > suggested we do the same thing as with the other stuff we read out from
> > the sdvo chip (as opposed to things we can read out from
> > crtc/sdvo-host-side registers).
> 
> We do read out the hdmi encoding and pixel multipler from
> the sdvo chip already. If the chips don't implement the hdmi
> stuff we treat them as dvi. I have some (supposedly) hdmi
> add2 cards like that. So I don't think those pose any kind
> of real problem.

Hm ok, but you get to keep the pieces if there's an sdvo regression report
:-) I.e. in that case I'd vote for a revert + quirk flag, and call it a
day. But I guess for now we could assume that hdmi sdvo cards are less
garbage, and implement the spec better.

On both this and the previous patch:

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

(Since I'm too lazy to dig out the hdmi sdvo spec and check all the
details, imo not quite worth it).

It would be good to capture the gist of this discussion here in the commit
message, for future reference. Maybe stuff it into the readout patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx