Understanding correct ISR implementation

The hardware manual for the device will details the peripheral registers, and any bits that need to be cleared during interrupt handling.
Are you writing your own interrupt handler, or using a callback that called from an existing interrupt handler in the SSP?

Hi Jeremy,

yes I'm talking about ISRs, not callbacks from Synergy.

I'm using a timer and the DMAC transfer driver callback, which I think is an IRQ as well, by looking at r_dmac.c, R_DMAC_Open():

    if (NULL != p_cfg->p_callback)
    {
        ssp_vector_info_t * p_vector_info;
        fmi_event_info_t event_info = {(IRQn_Type) 0U};
        err = g_fmi_on_fmi.eventInfoGet(&feature, SSP_SIGNAL_DMAC_INT, &event_info);
        p_ctrl->irq = event_info.irq;
        DMAC_ERROR_RETURN(SSP_INVALID_VECTOR != p_ctrl->irq, SSP_ERR_IRQ_BSP_DISABLED);
        R_SSP_VectorInfoGet(p_ctrl->irq, &p_vector_info);
        NVIC_SetPriority(p_ctrl->irq, p_cfg->irq_ipl);
        *(p_vector_info->pp_ctrl) = p_ctrl;
    }

 

For the timer I don't set any register flags in the Interrupt handler. For the DMAC, my code currently looks like this:

 

void dmac_adc_measurement_callback(transfer_callback_args_t * p_args)
{
    SF_CONTEXT_SAVE;
    SSP_PARAMETER_NOT_USED(p_args);

    g_gpt0.p_api->stop(g_gpt0.p_ctrl);

    /* Count blocks and store adc_buffer.buffer index of the next block */
    adc_device.interrupt_count = (adc_device.interrupt_count + 1) % DMAC_NUM_BLOCKS;
    adc_device.current_block_index = adc_device.interrupt_count * DMAC_NUM_TRANSFERS;

    /* Check if measurement is completed (= buffer is filled once since trigger_index)*/
    if(adc_device.state == ADCDevice::State::MEASURING
        && adc_device.current_block_index == adc_device.trigger_index)
        adc_device.set_state(ADCDevice::State::MEASUREMENT_END);
    else
    {
        /* Set the number of transfers for the next block */
        R_DMAC0->DMCRB = DMAC_NUM_TRANSFERS;

        /* If we start again at the beginning */
        if(adc_device.interrupt_count == 0)
        {
            /* Reset the DMAC for the next capture */
            /* Reset the source, destination address */
            g_transfer0.p_cfg->p_info->p_dest = static_cast<void*>(adc_device.buffer.data());

            /* Reset the DMAC.
             * Source address will be the same.
             * New Destination address.
             * Refresh the block count
             */
            g_transfer0.p_api->reset(g_transfer0.p_ctrl,
                                               g_transfer0.p_cfg->p_info->p_src,
                                               g_transfer0.p_cfg->p_info->p_dest,
                                               DMAC_NUM_TRANSFERS );
        }
        g_transfer0.p_api->enable(g_transfer0.p_ctrl);

        g_gpt0.p_api->start(g_gpt0.p_ctrl);
    }

    /* Calibrate the sensor channels while idle */
    //double FM_idle_time = static_cast<double>(tx_time_get() - FM::instance().idle_since) / static_cast<double>(TX_TIMER_TICKS_PER_SECOND);
    if(adc_device.state == ADCDevice::State::IDLE && (tx_time_get() - FM::instance().idle_since) > DMAC_NUM_TRANSFERS * ADC_SPEED * TX_TIMER_TICKS_PER_SECOND)
        calibration_step();

    SF_CONTEXT_RESTORE;
}

The code is used to implement a multi-DMAC block ring buffer with a pre/post-triggering system for short time measurements, somewhat similar to an oscilloscope. The code is (mostly) working as expected. so I believe that I don't need to clear any additional registers.

However, I sometimes see an erroneous data point at the block boundary, which I assume comes from timing problems in this fucntion. I've written some different variants which will be tested shortly:

1. Not stopping the timer
2. Resetting the timer after stopping
3. Increasing interrupt priority
4. Clearing pending IRQ (Shouldn't do anything here as this IRQ triggers once every 5 ms)

I hope that one of these will help with this problem, otherwise I'll have to do some signal processing to filter out invalid data points.

Chris,

I did look at that and there doesn't seem to be an easy way.

However, my thought is this. Prior to starting the ADC that the DMA's will be all configured, ready to take the ADC interrupt event and transfer data accordingly.
Once configured, to enable a DMA is one line of code, setting the DTE bit in the corresponding DMCNT register.
i.e. R_DMAC0->DMCNT = 0x01; or R_DMAC1->DMCNT = 0x01;
If this is the first line of code in the DMAC ISR then I think you will have enough time.
You say that the ADC (& DMAC) is being triggered every 2us. With the S5D5 ICLK running at 120MHz, that gives you 240 CPU clocks to take the DMAC ISR, and execute the code R_DMAC0->DMCNT = 0x01;
If may be worth while considering using a custom DMAC ISR as the SSP ISR handlers can be rather "code heavy" and could slow the response time.
If you think that you system may not be able to repsond in time (the CPU is doing other system critical tasks that can't be interrupted) then the DMAC ISR can trigger the DTC. The DTC could be set up to write the value 0x01 to the corresponding DMCNT register.
Let me know your thoughts and if required I can put together a proof on concept?

Regards,
Richard

If may be worth while considering using a custom DMAC ISR as the SSP ISR handlers can be rather "code heavy" and could slow the response time.

Do you mean replacing g_transfer0.p_api->enable() with a custom implementation?

We cannot (or probably shouldn't) start it directly at the ISR start since we may have to stop the measurement at the post triggering time. It may be possible to use a slightly larger buffer to keep it running before doing the check at the cost of some additional complexity.

I think there isn't too much time critical other processes going on. We have some ethernet communication and a much slower ADC measurement on ADC1. There's also some controlling done in the ISR I showed above, but that can happen after reconfiguration. There are however some IRQ based triggers for the measurement with higher or equal priority as the DMAC ISR.

You can do that. Everything that eliminates code execution time.
But I was actually thinking of the DMA ISR. From the code you have posted it looks as if you are using the DMA callback function. " void dmac_adc_measurement_callback(transfer_callback_args_t * p_args)"
This callback is called as part of the DMA ISR.
Whilst its functionality is absolutely fine, it may be possible to save a few CPU clocks by writing your own.

Ah yes you're right, I didn't look too far into the DMA transfer driver code. That function could be simplified a bit I think. It's probably easiest to exchange it during linking with -Wl,--wrap=dmac_int_isr or to define SSP_SUPPRESS_ISR_g_transfer0 and set SSP_VECTOR_DEFINE_CHAN(dmac_int_isr, DMAC, INT, 0); somewhere else.

I've started some optimizations for the ISR:

void dmac_int_isr (void)
{
    /* Stop the timer that triggers the ADC */
    g_gpt0.p_api->stop(g_gpt0.p_ctrl);

    /* Count blocks and store adc_buffer.buffer index of the next block */
    adc_device.interrupt_count = (adc_device.interrupt_count + 1) % DMAC_NUM_BLOCKS;

    /* Check if measurement is completed (= buffer is filled once since trigger_index)*/
    if(adc_device.state != ADCDevice::State::MEASURING
        || adc_device.interrupt_count != adc_device.trigger_interrupt_count)
    {
        /* Set the number of transfers for the next block */
        R_DMAC0->DMCRB = DMAC_NUM_TRANSFERS;

        /* If we start again at the beginning */
        if(adc_device.interrupt_count == 0)
        {
            /* Reset the DMAC for the next capture */
            /* Reset the source, destination address */
            g_transfer0.p_cfg->p_info->p_dest = static_cast<void*>(adc_device.buffer.data());

            /* Reset the DMAC.
             * Source address will be the same.
             * New Destination address.
             * Refresh the block count
             */
            g_transfer0.p_api->reset(g_transfer0.p_ctrl,
                                               g_transfer0.p_cfg->p_info->p_src,
                                               g_transfer0.p_cfg->p_info->p_dest,
                                               DMAC_NUM_TRANSFERS );
        }

        /* Transfer is disabled during the interrupt if an interrupt is requested after each block. If not all transfers
         * are complete, reenable transfer here. */
        auto p_ctrl = static_cast<dmac_instance_ctrl_t*>(g_transfer0.p_ctrl);
        static_cast<R_DMAC0_Type*>(p_ctrl->p_reg)->DMCNT_b.DTE = true;
        extern R_ICU_Type * gp_icu_regs;
        gp_icu_regs->DELSRn[p_ctrl->channel].DELSRn_b.DELS = p_ctrl->trigger;

        /* Start the ADC timer again to resume measurement */
        g_gpt0.p_api->start(g_gpt0.p_ctrl);
    }
    else
        adc_device.set_state(ADCDevice::State::MEASUREMENT_END);
    adc_device.current_block_index = adc_device.interrupt_count * DMAC_NUM_TRANSFERS;

    /* Calibrate the sensor channels while idle */
    //double FM_idle_time = static_cast<double>(tx_time_get() - FM::instance().idle_since) / static_cast<double>(TX_TIMER_TICKS_PER_SECOND);
    if(adc_device.state == ADCDevice::State::IDLE && (tx_time_get() - FM::instance().idle_since) > DMAC_NUM_TRANSFERS * ADC_SPEED * TX_TIMER_TICKS_PER_SECOND)
        calibration_step();

    /** Clear pending IRQ to make sure it doesn't fire again after exiting */
    R_BSP_IrqStatusClear(R_SSP_CurrentIrqGet());
} /* End of function dmac_int_isr */

It might be possible to optimize away the stopping of the ADC timer (if it's fast enough) and the check for ADCDevice::State::MEASURING. I'm not sure if the gp_icu_regs->DELSRn[p_ctrl->channel].DELSRn_b.DELS = p_ctrl->trigger; line is needed.

In the next step I'll try to use a second DMA channel. If that is not enough I might add one more block to the rin buffer to directly resume the measurement without checking for the stop condition. If that is still not sufficient the DTC approach might be needed. Do you think this is a valid approach?

Chris,
I have just uploaded a project to the Media Gallery (S5D5_TB_ADC_DUAL_DMAC.zip) which I hope will give you some pointers.

The example that uses 2 DMA channels to read the ADC and write data into an incrementing buffer
DMA0 writes to the buffer 10000 results and then DMA1 takes over
When DMA1 has written 10000 results, DMA0 takes over.

When the buffer is full, the buffer size is 100000 elements in size, the buffer wraps around

The DMAs are configured for block mode, reading 4 ADC channels, and the block transfer number is 2500
The DMA ISRs enable the next DMA channel.
I hope this provides glitch free ADC readings.

Regards,
Richard

Hi Richard,

thanks for the example! I actually went ahead and wrote an implementation myself in parallel. I will check yours out and compare it against mine. I'll report back with test results tomorrow.

One question: Is it necessary to move the resetting of the DMAC channel that recently finished into the main loop / a thread or can it remain in the ISR after starting the new one?

I more or less have the same implementation, except that I haven't migrated to my own ISR yet and still rely on the callback, in which I also setup the DMAC channel that will be needed next time. I've also used pointers to the transfer structures and registers which I swap on each invocation of the callback.

Best Regards,
Chris

Chris,
One final question. What size is the circular buffer that you are using?
If you can use a circular buffer of 2^n (n=1->27) then a solution using DMA Extended repeat mode could be used.
(The sizes are detailed in Table 17.2 of the Hardware Users Manual.)

This extended repeat area solution uses minimal interrupts, just an occasional interrupt to rewrite the DMCRB so that it never gets to 0 and stops the DMA.
The reconfiguration of the DMA destination address is performed automatically. The only limitation is that you buffer would have to be of a specific size.

I can provide further info tomorrow if this is of interest?
Regards,
Richard

I think this might be an option. Right now I'm measuring 4 channels, 12 bit, 500 kHz for 1 s, so 4*2*500000=4000000 bytes. A buffer of 2^22=4194304 might thus be a good fit. I'll check out this implementation first though, maybe it's already sufficient.

Hi Richard,
could you explain this line:
R_DMAC0->DMCRA = 0x00040004;

In my understanding only the block number needs to be reset. In R_DMAC_Reset there is
if ((TRANSFER_MODE_NORMAL != HW_DMAC_ModeGet(p_dmac_regs)))
{
HW_DMAC_BlockNumberSet(p_dmac_regs, num_transfers);
}
else
{
HW_DMAC_TransferNumberSet(p_dmac_regs, num_transfers);
}

so here only HW_DMAC_BlockNumberSet(p_dmac_regs, num_transfers); would be called.

I assume you are referring to this part of the code:

/* Reset the DMAC for the next capture
* Not using the api g_adc_transfer1.p_api->blockReset as that automatically enables the DMA
* This is not required as the other DMAC interrupt will do this
* Therefore, reinitialise manually
*/
R_DMAC0->DMDAR = (uint32_t)&adc0_result_buffer[ dmac0_buffer_full_counter * 10000L ];
R_DMAC0->DMCRA = 0x00040004;
R_DMAC0->DMCRB = 2500;


If so, the reason is a bit of lazy cut and pasting from a previous project!

You are correct. Once the DMAC has been set up for block mode transfer the DMCRA.H is constant (block size) and DMCRA.L (block size counter) is decremented and automatically initialised when it equals 0, so it's only the DMCRB (the DMA Block Transfer Count Register) that needs reinitialising.
You can remove it.

I had some issues getting my version to run but I solved them and the first results look promising.
I'm using one callback for both DMACs and so I use the enable/disable functions as they control the interrupt as well.
We will do more extensive tests on Monday.

Keep me posted. If your implementation has issues then I have another version, that uses a timer to count the number of DMA transfers and via a timer ISR, keeps the DMCRB from reaching 0. Therefore, the DMA can run continuously.
This may want to look at this too?

That also sounds like an interesting approach, I didn't know that it was possible to set the registers while running. Is it possible to guarantee a deterministic stopping behaviour with that method? I need to be able to save the whole ring buffer.

Chris,
I have uploaded my project (S7G2_DK_ADC_DMAC_CONTINUOUS.zip) to the media gallery.
The project captures ADC data to the SDRAM buffer in 1second packets ( 2000000 ADC data values )

The mechanism is as such:
(Project written for the S7G2-DK board, as this is the only board I have with SDRAM fitted)

S1 (user push button) generates IRQ11. IRQ11 ISR (callback) starts GPT2 running

GPT2 starts GPT1 and 1 second later, GPT2 stops GPT1. GPT2 will also stop itself.

When running, GPT1 triggers the ADC every 2us

ADC performs a conversion on AN000, AN001, AN002 & AN003

ADC conversion end event triggers DMAC0

DMAC0 performs a block transfer of AN000, AN001, AN002 & AN003 to SDRAM. DMAC0 block transfer counter (DMCRB) is decremented by 1. If DMCRB = 0, the DMAC will stop and will not perform further transfers until it is re-enabled.
The starting count value of DMCRB = 65535;

ADC conversion end event also increments the counter of GPT0.

GPT0 is configured to overflow at 65530 and generate an interrupt on overflow.
At overflow, the DMRCB = 5, very close to reaching 0 and stopping the DMAC.

The GPT0 overflow ISR resets the DMRCB back to 65535.
Therefore, the DMAC never stops as DMRCB never reaches 0.

This mechanism allows for 2000000 ADC samples to be captured by the DMA continuously.


To answer your question
"I didn't know that it was possible to set the registers while running. Is it possible to guarantee a deterministic stopping behaviour with that method? I need to be able to save the whole ring buffer."

The DMA has the ACT status flag, which is set when a transfer is tacking place, and clear when not.
The User's Manual has the restriction on register access:

Do not write to the following registers of DMACm while the ACT flag in DMSTS of the associated channel is set to 1 (DMAC active state) or the DTE bit in DMCNT of the associated channel is set to 1 (DMA transfer enabled):
DMSAR
DMDAR
DMCRA
DMCRB
DMTMD
DMINT
DMAMD
DMOFR.

Therefore, in the GPT0 ISR that rewrites the DMCRB, the mechanism is:

while(R_DMAC0->DMSTS_b.ACT == 1); /* Wait for the DMA to be idle */

R_DMAC0->DMCNT_b.DTE = 0; /* Disable DMAC */
R_DMAC0->DMCRB = DMAC_DMCRB_FREE_RUN_COUNT; /* Rewrite the DMCRB */
R_DMAC0->DMCNT_b.DTE = 1; /* Enable DMAC */


As mentioned, the project uses a GPT timer to generate an accurate 1 second window. Via the Compare Match A & B events, and the ELC, GPT1 (and the ADC / DMAC ) can be controlled in an accurate manor.
To ensure that I'm not getting any ADC error readings, I do some simple data processing to ensure that there are no ADC results above or below a threshold. My tests indicate that this is the case.

This approach may be of interest.
I hope that with this project and with the previous ones that we have discussed you are able to get your application working as required.
Any issues, let me know
Regards,
Richard

Chris,
I have uploaded my project (S7G2_DK_ADC_DMAC_CONTINUOUS.zip) to the media gallery.
The project captures ADC data to the SDRAM buffer in 1second packets ( 2000000 ADC data values )

The mechanism is as such:
(Project written for the S7G2-DK board, as this is the only board I have with SDRAM fitted)

S1 (user push button) generates IRQ11. IRQ11 ISR (callback) starts GPT2 running

GPT2 starts GPT1 and 1 second later, GPT2 stops GPT1. GPT2 will also stop itself.

When running, GPT1 triggers the ADC every 2us

ADC performs a conversion on AN000, AN001, AN002 & AN003

ADC conversion end event triggers DMAC0

DMAC0 performs a block transfer of AN000, AN001, AN002 & AN003 to SDRAM. DMAC0 block transfer counter (DMCRB) is decremented by 1. If DMCRB = 0, the DMAC will stop and will not perform further transfers until it is re-enabled.
The starting count value of DMCRB = 65535;

ADC conversion end event also increments the counter of GPT0.

GPT0 is configured to overflow at 65530 and generate an interrupt on overflow.
At overflow, the DMRCB = 5, very close to reaching 0 and stopping the DMAC.

The GPT0 overflow ISR resets the DMRCB back to 65535.
Therefore, the DMAC never stops as DMRCB never reaches 0.

This mechanism allows for 2000000 ADC samples to be captured by the DMA continuously.


To answer your question
"I didn't know that it was possible to set the registers while running. Is it possible to guarantee a deterministic stopping behaviour with that method? I need to be able to save the whole ring buffer."

The DMA has the ACT status flag, which is set when a transfer is tacking place, and clear when not.
The User's Manual has the restriction on register access:

Do not write to the following registers of DMACm while the ACT flag in DMSTS of the associated channel is set to 1 (DMAC active state) or the DTE bit in DMCNT of the associated channel is set to 1 (DMA transfer enabled):
DMSAR
DMDAR
DMCRA
DMCRB
DMTMD
DMINT
DMAMD
DMOFR.

Therefore, in the GPT0 ISR that rewrites the DMCRB, the mechanism is:

while(R_DMAC0->DMSTS_b.ACT == 1); /* Wait for the DMA to be idle */

R_DMAC0->DMCNT_b.DTE = 0; /* Disable DMAC */
R_DMAC0->DMCRB = DMAC_DMCRB_FREE_RUN_COUNT; /* Rewrite the DMCRB */
R_DMAC0->DMCNT_b.DTE = 1; /* Enable DMAC */


As mentioned, the project uses a GPT timer to generate an accurate 1 second window. Via the Compare Match A & B events, and the ELC, GPT1 (and the ADC / DMAC ) can be controlled in an accurate manor.
To ensure that I'm not getting any ADC error readings, I do some simple data processing to ensure that there are no ADC results above or below a threshold. My tests indicate that this is the case.

This approach may be of interest.
I hope that with this project and with the previous ones that we have discussed you are able to get your application working as required.
Any issues, let me know
Regards,
Richard

Hi Richard,

we've tested approximately 100 measurements and didn't see this issue anymore so I think the dual DMA channel solution works for us.

Thanks a lot for your support!
Best Regards,
Chris