Re: [PATCH v2 03/13] gpu: host1x: Support 40-bit addressing

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3992 bytes --]

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@nvidia.com>
> > 
> > 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@nvidia.com>
> > ---
> >   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 #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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