Re: [PATCH v2 3/4] drm/omap: Make fixed resolution panels work

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 1945 bytes --]

On 2013-03-19 08:45, Archit Taneja wrote:

> I was trying to come up with a macro which could add default ops to the
> omap_dss_driver. It isn't straight forward as I thought, because you
> need to choose either the default op, or the panel driver's op if it
> exists. For example, I can't create a macro which adds an op for
> get_timings() to all panel drivers, the panel drivers which already this
> op defined will have 2 instances of get_timings() in the omap_dss_driver
> struct.

Yep, I noticed the same a few days ago.

> I have been looking around in the kernel to see how undeclared ops in a
> struct are generally dealt with. Till now, I've noticed that the code
> which uses this ops takes the responsibility to check whether they are
> NULL or not.
> 
> The easiest way would be to have these default funcs inlined in a
> header, and every panel driver's omap_dss_driver struct fills in the
> default op. This way we can make the driver ops const. Do you have any
> idea of a macro which could do the same as above, and leads to less
> addition of code?

Why would they need to be inlined?

Another option would be to create global funcs that are used to call the
ops. So instead of:

dssdev->dssdrv->foo(dssdev)

the user would call this function:

int dss_foo(struct omap_dss_device *dssdev)
{
	if (dssdev->dssdrv->foo == NULL)
		return 0; /* or error, depending on case */
	return dssdev->dssdrv->foo(dssdev);
}

But that'd require adding a bunch of functions, and changing all the
callers.

I think the safest way to fix this for now is to add the checks into
omapdrm as you do in your original patch. If we go for some other route,
I fear that omapfb/omap_vout could be affected, as it could presume that
an op being NULL or non-NULL means something. If we change the ops to be
always non-NULL, we should go over the uses of those ops to verify they
work correctly.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]