[PATCH 3/4] drm/omapdrm: Work-a-round for errata i734 (LCD1 Gamma) in DSS dispc

[Jyri Sarha]

On 05/20/16 16:21, Tomi Valkeinen wrote:
>> > +
>> > +static int dispc_errata_i734_wa_init(void)
>> > +{
>> > +	size_t buff_size = i734.ovli.width * i734.ovli.height *
> "buf_size" would match the other names =).
> 
> Usually I try to do only simple initializations when declaring the
> locals. This is a bit on the complex side, and also there's a check
> below that can make all this init calculation not needed.
> 

Compiler optimizations should delay the initialization to happen only if
it is needed.

I usually try to keep the functions as compact as possible so that they
fit on the screen at once. But I think it is better to move the size
also inside the struct i734_buf so it we only need to calculate the size
once and functions become even smaller :).

...
>> > +
>> > +static void dispc_errata_i734_wa(void)
>> > +{
>> > +	u32 vsync_irq = dispc_mgr_get_vsync_irq(OMAP_DSS_CHANNEL_LCD);
>> > +	u32 framedone_irq = dispc_mgr_get_framedone_irq(OMAP_DSS_CHANNEL_LCD);
>> > +	u32 gatestate = REG_GET(DISPC_CONTROL, 8, 4);
> Here too, too complex initializations. Especially doing register
> reads/writes should be something done in the "real" code. The bits you
> read might not even be valid on some platforms which don't have the i734.
> 

You are right about the register read. Compiler should not optimize the
register access. But - but with your permission - I'll keep the IRQ mask
initializations as they are quite constant in nature anyway.

BR,
Jyri



-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160523/6a674655/attachment.sig>