On Thu, Aug 11, 2016 at 04:49:28PM +0200, Hans Verkuil wrote:

On 08/11/16 16:16, Russell King - ARM Linux wrote:

...

On Thu, Aug 11, 2016 at 12:49:21PM +0200, Hans Verkuil wrote:

...

I thought this was the raw EDID data. But if you pass a struct edid around, then why not make this const struct edid *edid? I fail to see why you would want to use a void pointer here.

struct edid is a DRM thing - it's not generic. Using struct edid here would force everyone to use the DRM structure, whereas other subsystems use, eg, unsigned char.

So how the edid pointer should be interpreted depends on which subsystem sends it? That doesn't sound right. It makes it really hard for the drivers receiving the notifications.

No, that's wrong... and I really don't see why you're trying to make a meal of this.

In both cases, it's the EDID as received from the device. One subsystem happens to represent it internally as a "struct edid" in its drivers, another subsystem uses "unsigned char []".

In order to _cleanly_ accept both styles, without having to resort to driver authors needing to litter their code with lots of idiotic casts, I'm accepting a void * pointer, so that we can accept _both_ a struct edid and an unsigned char array.

Both are compatible with each other. Both have the same bytes at the same offset. There is no difference between them other than the C types that are used by each subsystem.

But I understand that currently the main purpose of the hdmi_event_new_edid is to get the physical address from the EDID for CEC? Or do you have other uses as well?

That is one use, but if an audio driver wants to parse the raw EDID instead of the ELD, it can.

What I want to avoid is having the video driver parse lots of different bits from the EDID and then send lots of events "just in case" something wants some of the parsed information - it's much more efficient to give drivers the raw information and allow them to parse what they need from the EDID. I'm not convinced that parsing out the physical address, and then passing that from the video side is a good idea.

The reason I made an exception for the ELD is because (a) it's already passed between video and audio drivers, (b) the code we have present to convert an EDID to ELD is in DRM, and requires DRM data structures which won't be available in an audio driver.

If you want a mechanism to pass just the physical address, we could add it, and require CEC drivers to make use of both the EDID or physical address events. That would mean situations where the physical address is known to be fixed before hand can be easily catered for.