On 07/01/18 22:14, Jyri Sarha wrote:
>>> +static u64 dispc_api_read_irqstatus(u64 clearmask)
>>> +{
>>> + u32 hw_clearmask = dispc_api_to_hw_irq(clearmask);
>>> + u32 hw_status = dispc_read_irqstatus();
>>> +
>>> + dispc_clear_irqstatus(hw_clearmask & hw_status);
>>> +
>>> + return dispc_hw_to_api_irq(hw_status);
>>> +}
>>
>> I think we always want to read the whole irqstatus, and clear it. You
>> can do that with the function above, but I'm not sure if clearmask
>> offers us anything we ever need to use.
>>
>
> But the semantically correct way is to clear only the interrupts we
> handle. In theory there could be some other entity following some other
> interrupts and clearing the interrupts we are not handling could ruin
If that would be the case, then we need to ensure that the irqenable and
irqstatus are handled race free, and we need to make sure those entities
never touch the same bits, even if they'd use proper locking.
Yes, it's possible, but we don't do it and I hope we never do.
> that. But in practice at the moment everything would work fine without
> the clearmask too. Is that reason enough to remove it?
Well, I would ask if there's enough reason to add it? What's the
scenario you see that it would be used (not theoretical, but real one)?
Or is there any other downside to just clearing all irqstatus bits?
My main worry with this is that we somehow (well, bug) would end up
having a dispc irq enabled in irqenable, and we would not have that bit
in clearmask. The end result would be an endless irq flood.
If in the future there's a use case that needs this, it would be trivial
to add it at that point of time. So my guideline would be to keep things
as simple as possible, and only add unused features if there's a
realistic near-future use case for it which you know you'll be implementing.
Tomi
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
