Re: [PATCH v22 08/14] media: qcom: camss: csid: Add support for CSID 1080

[Hangxiang Ma]

On 10/26/2026 6:49 PM, Loic Poulain wrote:
> On Sun, Oct 25, 2026 at 4:17 AM Hangxiang Ma
> <hangxiang.ma@xxxxxxxxxxxxxxxx> wrote:
>>
>> Add support for the CSID 1080 hardware found on the Kaanapali SoC.
>>
[...]>> +static void csid_configure_stream(struct csid_device *csid, u8
enable)
>> +{
>> +       u8 i, k;
>> +
>> +       __csid_configure_rx(csid, &csid->phy);
>> +
>> +       for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS_1080; i++) {
>> +               if (csid->phy.en_vc & BIT(i)) {
>> +                       __csid_configure_rdi_stream(csid, enable, i, 0);
>> +                       __csid_configure_rx_vc(csid, 0);
>> +
>> +                       for (k = 0; k < CAMSS_INIT_BUF_COUNT; k++)
>> +                               __csid_addr_update(csid, i);
>
> Do we really need to trigger the same AUP several times (one per
> buffer in the FIFO)? could we have a small comment on why?
> Isn't this already done from the vfe_enable_output helper?
>
Yes, this is required. In v2 we hit an AUP-related issue where one
buffer was missed and capture got stuck on the last frame of the stream.
Vijay explained the details in this thread:

<https://lore.kernel.org/all/662a21a3-de8b-406f-a15d-b8a572aa79ab@xxxxxxxxxxxxxxxx/>

In short, since the split design was introduced on the 980 platform, the
CSID has to be configured after the VFE. Otherwise, rup_done or buf_done
IRQs can be missed. Also, each buffer queued in the FIFO needs its own
AUP. If only one AUP is triggered, the last buffer is never processed,
and the pipeline stalls waiting for a buf_done that never arrives.

That is the underlying story of the 04/14 patch and explains why it is
introduced to handle the platform hardware differences. Thanks.

<https://lore.kernel.org/all/20261024-kaanapali-camss-v22-4-7da3ea49c8a1@xxxxxxxxxxxxxxxx/>

[...]
>> diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
>> index 2f7e593defed..603621c16315 100644
>> --- a/drivers/media/platform/qcom/camss/camss-csid.h
>> +++ b/drivers/media/platform/qcom/camss/camss-csid.h
>> @@ -162,7 +162,13 @@ struct csid_device {
>>          void __iomem *base;
>>          u32 irq;
>>          char irq_name[30];
>> -       u32 reg_update;
>> +       union {
>> +               u32 reg_update;
>> +               struct {
>> +                       u32 rup_update;
>> +                       u32 aup_update;
>
> I would say keep it simple and just add addr_update field after reg_update.
>
>
Ack. Thanks.

---
Best Regards,
Hangxiang