Re: [PATCH 2/2] drm/i915/dp: check eDP display control capability registers

From: Thierry Reding <thierry.reding@gmail.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: Alex Deucher <alexdeucher@gmail.com>,
	Jani Nikula <jani.nikula@intel.com>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	Maling list - DRI developers <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH 2/2] drm/i915/dp: check eDP display control capability registers
Date: Tue, 19 Nov 2013 09:37:04 +0100	[thread overview]
Message-ID: <20131119083703.GB31504@ulmo.nvidia.com> (raw)
In-Reply-To: <CAKMK7uFMwBwj=9-LoMRDHL08c-ceY=0p70=1z5tfbaM9Meaf-A@mail.gmail.com>

[-- Attachment #1.1: Type: text/plain, Size: 1964 bytes --]

On Mon, Nov 18, 2013 at 05:38:18PM +0100, Daniel Vetter wrote:
> On Mon, Nov 18, 2013 at 5:31 PM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> >> Note that there's already a bit of abstraction for i2c over dp aux, but
> >> imo that's at the wrong level. At least reading through i915, gma500 and
> >> radeon code there's a lot more we could share with just a dp aux helper
> >> library (which then implements useful stuff on top of it).
> >
> > I have some difficulty envisioning how the helper code can work without
> > some sort of driver-specific ops implementation. Currently the helpers
> > only use a snapshot of the DPCD to extract information. Eventually we'll
> > be bound to modify the DPCD, so some method of writing it back (or a
> > subset of it) will be needed. Otherwise the scope of the helper library
> > will be somewhat limited.
> >
> > Once we have the callbacks, the current helpers could be reworked to not
> > use a buffer, but rather an "AUX channel object" and access the
> > registers directly. If there are concerns about performance, it could
> > possibly be implemented as a sort of cache, too. That would make it fast
> > to query the status. I don't think it'll be worth the added complexity,
> > though.
> 
> Oh, my idea is that the dp aux driver callback would at the level of
> the intel_dp_aux_ch function in i915/intel_dp.c (gma500 and radeon
> have something very similar). That alone would allow us to share a
> considerable amount of code. Should have been a bit clearer, I've
> discussed this in a bit more detail with Alex many moons ago ...

Yeah, that's similar to what I had in mind. I think we may need
something slightly more complex, though. We want to support both AUX as
well as I2C over AUX transactions, so we'll probably need to add a mode
argument. I was thinking about adding a dp_aux_msg structure in order to
keep the argument list to a reasonable length.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx