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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux