[Intel-gfx] [PATCH 04/25] drm/i915: Add basic support for parsing of VBT OEM Custom Block

[Jesse Barnes]

On Thu, 22 Mar 2012 09:11:37 -0700
Ben Widawsky <ben at bwidawsk.net> wrote:

> On Thu, 22 Mar 2012 08:45:32 -0700
> Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> 
> > On Wed, 21 Mar 2012 18:30:11 -0700
> > Ben Widawsky <ben at bwidawsk.net> wrote:
> > 
> > > On Wed, 21 Mar 2012 12:48:25 -0700
> > > Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> > > 
> > > > From: Rohit Jain <rohit.jain at intel.com>
> > > > 
> > > > Added support for parsing the OEM Customizable Modes Block (#20)
> > > > in the VBIOS table.
> > > > 
> > > > Signed-off-by: Rohit Jain <rohit.jain at intel.com>
> > > > Reviewed-by: Shobhit Kumar <shobhit.kumar at intel.com>
> > > > Reviewed-by: Vijay A. Purushothaman
> > > > <vijay.a.purushothaman at intel.com> Acked-by: Jesse Barnes
> > > > <jesse.barnes at intel.com> Reviewed-by: Eugeni Dodonov
> > > > <eugeni.dodonov at intel.com> Signed-off-by: Artem Bityutskiy
> > > > <artem.bityutskiy at linux.intel.com> Signed-off-by: Jesse Barnes
> > > > <jbarnes at virtuousgeek.org> ---
> > > 
> > > I'll be honest, I haven't read through the whole series yet, but I
> > > really don't see the point of this patch, at least in the next few
> > > patches. Could we modify the commit to describe why we need this (or
> > > drop it)?
> > 
> > This one is unrelated to VLV, it's just extra VBT gravy.  Obviously it
> > needs to stuff the mode info somewhere though...
> > 
> 
> THe HW can behave one in two ways, you read the info out and that is
> required to advance the internal state machine to get to the next info
> you care about. Or, you read the info out and nobody knows or cares. I
> truly don't know how this HW works yet, but I'd say if it's the former
> it should be added as a comment to the code. If it's the latter, we
> should drop the patch.

As I said, it doesn't belong in the VLV patchset.  It has nothing to do
with touch hw though; it's just a way for us to get any OEM custom
modes out of the VBT.  For example on a given platform the VBT panel
data may have a certain timing, but an additional timing may also be
preferred, which might be specified in this block.

So we should parse it, and add it to the appropriate connector's mode
list.  This patch only does the parsing, but doesn't add it to the mode
list, so isn't useful by itself.

At any rate, I'll drop it from this series.

-- 
Jesse Barnes, Intel Open Source Technology Center
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20120322/637576d3/attachment.sig>