All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ser, Simon" <simon.ser@intel.com>
To: "Hiler, Arkadiusz" <arkadiusz.hiler@intel.com>,
	"jani.nikula@linux.intel.com" <jani.nikula@linux.intel.com>
Cc: "igt-dev@lists.freedesktop.org" <igt-dev@lists.freedesktop.org>
Subject: Re: [igt-dev] [PATCH i-g-t v2] lib/igt_edid: new library for generating EDIDs
Date: Tue, 16 Apr 2019 12:11:07 +0000	[thread overview]
Message-ID: <fa08ed6d06600b63d2fd79b1c8b6de247f9991db.camel@intel.com> (raw)
In-Reply-To: <87k1fuw6pj.fsf@intel.com>

On Tue, 2019-04-16 at 14:05 +0300, Jani Nikula wrote:
> > > Do you already have some thoughts on how we want to solve handling the
> > > extensions blocks?
> > 
> > I've already started to work on this. I think callers should allocate a
> > large enough buffer, e.g.:
> > 
> >   char buf[sizeof(struct edid) + 2*sizeof(struct edid_ext)];
> > 
> > (Extension blocks have a fixed size)
> > 
> > And then should map a struct edid to it:
> > 
> >   struct edid *edid = (struct edid *) buf;
> > 
> > The annoying thing is that the number of extensions at the end of the
> > EDID block isn't fixed. We could add a new field at the end of struct
> > edid:
> > 
> >   struct edid {
> >     …
> >     struct edid_ext extensions[];
> >   };
> > 
> > So that callers can do this:
> > 
> >   struct edid_ext *ext1 = edid->extensions[0];
> >   struct edid_ext *ext2 = edid->extensions[1];
> > 
> > Another annoying thing is that extensions can themselves contain
> > multiple sub-blocks of different sizes… For now I've opted for a char
> > buffer that can be casted into sub-blocks. This isn't the best solution
> > ever, but I haven't found a better design yet. Let's discuss about this
> > in the next patch. :)
> 
> While it's easy to write something that's better than what we currently
> have, as you see this gets complicated more quickly than you'd like,
> especially working in C. We're not the only ones needing EDID
> generation, so I'd suggest looking at what others are doing before
> embarking on writing a full blown generator. See e.g. [1].
> 
> [1] http://mid.mail-archive.com/20181121133624.i4vxgt7atnv3ads6@sirius.home.kraxel.org

Thanks for the link! I can't find other EDID generators written in C,
it seems most people focus on parsing (and most people = a few…).

Here is the code for reference: [1].

It's really too simple for us. It takes the output identifier, the DPI
and the resolution and generates an EDID from that. It also operates on
a raw `char *` buffer which makes getting the indexes right tricky. All
in all, not a lot we can learn from.

FWIW, I've been thnking about a possible "libedid" for a while. Many
projects need to parse/format EDIDs and it's a little unfortunate that
we need to duplicate code (especially when parsing also involves
applying quirks). See [2] for a discussion about this.

[1]: https://github.com/qemu/qemu/blob/master/hw/display/edid-generate.c
[2]: https://marc.info/?t=154555671400001&r=1&w=2
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

  reply	other threads:[~2019-04-16 12:11 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-15 12:38 [igt-dev] [PATCH i-g-t] lib/igt_edid: new library for generating EDIDs Simon Ser
2019-04-15 12:51 ` Petri Latvala
2019-04-15 13:32 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
2019-04-15 14:20 ` [igt-dev] [PATCH i-g-t v2] " Simon Ser
2019-04-16  8:11   ` Arkadiusz Hiler
2019-04-16  8:46     ` Ser, Simon
2019-04-16 11:05       ` Jani Nikula
2019-04-16 12:11         ` Ser, Simon [this message]
2019-04-16 13:23           ` Jani Nikula
2019-04-15 14:58 ` [igt-dev] ✓ Fi.CI.BAT: success for lib/igt_edid: new library for generating EDIDs (rev2) Patchwork
2019-04-15 15:09 ` [igt-dev] ✓ Fi.CI.IGT: success for lib/igt_edid: new library for generating EDIDs Patchwork
2019-04-15 16:46 ` [igt-dev] ✓ Fi.CI.IGT: success for lib/igt_edid: new library for generating EDIDs (rev2) Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fa08ed6d06600b63d2fd79b1c8b6de247f9991db.camel@intel.com \
    --to=simon.ser@intel.com \
    --cc=arkadiusz.hiler@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.