On Fri, Jan 25, 2019 at 11:13:41AM +0200, Mikko Perttunen wrote:
> On 24.1.2019 20.02, Thierry Reding wrote:
> > From: Thierry Reding <treding@xxxxxxxxxx>
> >
> > Tegra186 and later support 40 bits of address space. Additional
> > registers need to be programmed to store the full 40 bits of push
> > buffer addresses.
> >
> > Since command stream gathers can also reside in buffers in a 40-bit
> > address space, a new variant of the GATHER opcode is also introduced.
> > It takes two parameters: the first parameter contains the lower 32
> > bits of the address and the second parameter contains bits 32 to 39.
> >
> > Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> > ---
> > drivers/gpu/host1x/hw/cdma_hw.c | 13 ++++++++---
> > drivers/gpu/host1x/hw/channel_hw.c | 28 +++++++++++++++++++----
> > drivers/gpu/host1x/hw/host1x06_hardware.h | 5 ++++
> > drivers/gpu/host1x/hw/host1x07_hardware.h | 5 ++++
> > 4 files changed, 44 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/host1x/hw/cdma_hw.c b/drivers/gpu/host1x/hw/cdma_hw.c
> > index ce320534cbed..6d2b7af2af89 100644
> > --- a/drivers/gpu/host1x/hw/cdma_hw.c
> > +++ b/drivers/gpu/host1x/hw/cdma_hw.c
> > @@ -68,20 +68,27 @@ static void cdma_timeout_cpu_incr(struct host1x_cdma *cdma, u32 getptr,
> > static void cdma_start(struct host1x_cdma *cdma)
> > {
> > struct host1x_channel *ch = cdma_to_channel(cdma);
> > + u64 start, end;
> > if (cdma->running)
> > return;
> > cdma->last_pos = cdma->push_buffer.pos;
> > + start = cdma->push_buffer.dma;
> > + end = cdma->push_buffer.dma + cdma->push_buffer.size + 4;
> > host1x_ch_writel(ch, HOST1X_CHANNEL_DMACTRL_DMASTOP,
> > HOST1X_CHANNEL_DMACTRL);
> > /* set base, put and end pointer */
> > - host1x_ch_writel(ch, cdma->push_buffer.dma, HOST1X_CHANNEL_DMASTART);
> > + host1x_ch_writel(ch, lower_32_bits(start), HOST1X_CHANNEL_DMASTART);
> > host1x_ch_writel(ch, cdma->push_buffer.pos, HOST1X_CHANNEL_DMAPUT);
> > - host1x_ch_writel(ch, cdma->push_buffer.dma + cdma->push_buffer.size + 4,
> > - HOST1X_CHANNEL_DMAEND);
> > + host1x_ch_writel(ch, lower_32_bits(end), HOST1X_CHANNEL_DMAEND);
> > +
> > +#if HOST1X_HW >= 6
> > + host1x_ch_writel(ch, upper_32_bits(start), HOST1X_CHANNEL_DMASTART_HI);
> > + host1x_ch_writel(ch, upper_32_bits(end), HOST1X_CHANNEL_DMAEND_HI);
> > +#endif
> > /* reset GET */
> > host1x_ch_writel(ch, HOST1X_CHANNEL_DMACTRL_DMASTOP |
> > diff --git a/drivers/gpu/host1x/hw/channel_hw.c b/drivers/gpu/host1x/hw/channel_hw.c
> > index ff137fe72d34..78fb49539e8c 100644
> > --- a/drivers/gpu/host1x/hw/channel_hw.c
> > +++ b/drivers/gpu/host1x/hw/channel_hw.c
> > @@ -64,11 +64,31 @@ static void submit_gathers(struct host1x_job *job)
> > for (i = 0; i < job->num_gathers; i++) {
> > struct host1x_job_gather *g = &job->gathers[i];
> > - u32 op1 = host1x_opcode_gather(g->words);
> > - u32 op2 = g->base + g->offset;
> > + dma_addr_t addr = g->base + g->offset;
> > + u32 op2, op3;
> > - trace_write_gather(cdma, g->bo, g->offset, op1 & 0xffff);
> > - host1x_cdma_push(cdma, op1, op2);
> > + op2 = lower_32_bits(addr);
> > + op3 = upper_32_bits(addr);
> > +
> > + trace_write_gather(cdma, g->bo, g->offset, g->words);
> > +
> > + if (op3 != 0) {
> > +#if HOST1X_HW >= 6
> > + u32 op1 = host1x_opcode_gather_wide(g->words);
> > + u32 op4 = HOST1X_OPCODE_NOP;
> > +
> > + host1x_cdma_push(cdma, op1, op2);
> > + host1x_cdma_push(cdma, op3, op4);
>
> This will break if the first push goes as the last slot of the ringbuffer
> and the second push goes as the first slot of the ringbuffer.
>
> Otherwise looks good to me.
Why would that break? Isn't the purpose of a ringbuffer to behave as if
it was infinitely sequential? If this really is a problem, how do we fix
it? Would we have to stash NOPs into the pushbuffer until we wrap
around?
Thierry
Attachment:
signature.asc
Description: PGP signature
