[PATCH V2 1/3] gpu: host1x: Reduce host1x job allocation size

[PATCH V2 1/3] gpu: host1x: Reduce host1x job allocation size

[David Ung]

There is 2 set of num_relocs * sizeof(*) array at the end of host1x job.
Only the 1st set is really used and with job->gather_addr_phys pointing
somewhere within the 1st set of reloc physical addresses.
This patch removes the 2nd set of unused addresses.

Signed-off-by: David Ung <davidu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/gpu/host1x/job.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c
index 63bd63f..9c11265 100644
--- a/drivers/gpu/host1x/job.c
+++ b/drivers/gpu/host1x/job.c
@@ -46,8 +46,9 @@ struct host1x_job *host1x_job_alloc(struct host1x_channel *ch,
 		(u64)num_unpins * sizeof(struct host1x_job_unpin_data) +
 		(u64)num_waitchks * sizeof(struct host1x_waitchk) +
 		(u64)num_cmdbufs * sizeof(struct host1x_job_gather) +
-		(u64)num_unpins * sizeof(dma_addr_t) +
-		(u64)num_unpins * sizeof(u32 *);
+		(u64)num_relocs * sizeof(dma_addr_t) +
+		(u64)num_cmdbufs * sizeof(dma_addr_t);
+
 	if (total > ULONG_MAX)
 		return NULL;
 
-- 
1.8.1.5

[PATCH V2 2/3] gpu: host1x: Remove addr_phys member from host1x_job

[David Ung]

We are using addr_phys and gather_addr_phys/reloc_addr_phys interchangeably
which points to the same set of memory. Using only gather/reloc_addr_phys
make the code clearer and less error prone.

Signed-off-by: David Ung <davidu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/gpu/host1x/job.c | 11 +++++------
 include/linux/host1x.h   |  1 -
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c
index 9c11265..7ecbefd 100644
--- a/drivers/gpu/host1x/job.c
+++ b/drivers/gpu/host1x/job.c
@@ -69,10 +69,9 @@ struct host1x_job *host1x_job_alloc(struct host1x_channel *ch,
 	mem += num_waitchks * sizeof(struct host1x_waitchk);
 	job->gathers = num_cmdbufs ? mem : NULL;
 	mem += num_cmdbufs * sizeof(struct host1x_job_gather);
-	job->addr_phys = num_unpins ? mem : NULL;
-
-	job->reloc_addr_phys = job->addr_phys;
-	job->gather_addr_phys = &job->addr_phys[num_relocs];
+	job->reloc_addr_phys = num_unpins ? mem : NULL;
+	mem += num_relocs * sizeof(dma_addr_t);
+	job->gather_addr_phys = num_unpins ? mem : NULL;
 
 	return job;
 }
@@ -194,7 +193,7 @@ static unsigned int pin_job(struct host1x_job *job)
 		if (!phys_addr)
 			goto unpin;
 
-		job->addr_phys[job->num_unpins] = phys_addr;
+		job->reloc_addr_phys[i] = phys_addr;
 		job->unpins[job->num_unpins].bo = reloc->target.bo;
 		job->unpins[job->num_unpins].sgt = sgt;
 		job->num_unpins++;
@@ -213,7 +212,7 @@ static unsigned int pin_job(struct host1x_job *job)
 		if (!phys_addr)
 			goto unpin;
 
-		job->addr_phys[job->num_unpins] = phys_addr;
+		job->gather_addr_phys[i] = phys_addr;
 		job->unpins[job->num_unpins].bo = g->bo;
 		job->unpins[job->num_unpins].sgt = sgt;
 		job->num_unpins++;
diff --git a/include/linux/host1x.h b/include/linux/host1x.h
index bb9840f..a722c4b 100644
--- a/include/linux/host1x.h
+++ b/include/linux/host1x.h
@@ -202,7 +202,6 @@ struct host1x_job {
 	struct host1x_job_unpin_data *unpins;
 	unsigned int num_unpins;
 
-	dma_addr_t *addr_phys;
 	dma_addr_t *gather_addr_phys;
 	dma_addr_t *reloc_addr_phys;
 
-- 
1.8.1.5

[PATCH V2 3/3] gpu: host1x: Record the physical address for gathers

[David Ung]

The gather's base physical address is recorded during patching of gathers,
but only the 1st one within the same buffer object has it set, subsequent
gathers in the same buffer are being skipped.
This patch records the phys addr during pin_job for all gathers.

Signed-off-by: David Ung <davidu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/gpu/host1x/job.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c
index 7ecbefd..6b374bc 100644
--- a/drivers/gpu/host1x/job.c
+++ b/drivers/gpu/host1x/job.c
@@ -202,17 +202,16 @@ static unsigned int pin_job(struct host1x_job *job)
 	for (i = 0; i < job->num_gathers; i++) {
 		struct host1x_job_gather *g = &job->gathers[i];
 		struct sg_table *sgt;
-		dma_addr_t phys_addr;
 
 		g->bo = host1x_bo_get(g->bo);
 		if (!g->bo)
 			goto unpin;
 
-		phys_addr = host1x_bo_pin(g->bo, &sgt);
-		if (!phys_addr)
+		g->base = host1x_bo_pin(g->bo, &sgt);
+		if (!g->base)
 			goto unpin;
 
-		job->gather_addr_phys[i] = phys_addr;
+		job->gather_addr_phys[i] = g->base;
 		job->unpins[job->num_unpins].bo = g->bo;
 		job->unpins[job->num_unpins].sgt = sgt;
 		job->num_unpins++;
@@ -536,8 +535,6 @@ int host1x_job_pin(struct host1x_job *job, struct device *dev)
 		if (g->handled)
 			continue;
 
-		g->base = job->gather_addr_phys[i];
-
 		for (j = i + 1; j < job->num_gathers; j++)
 			if (job->gathers[j].bo == g->bo)
 				job->gathers[j].handled = true;
-- 
1.8.1.5

RE: [PATCH V2 1/3] gpu: host1x: Reduce host1x job allocation size

[David Ung]

ping?

________________________________________
From: David Ung <davidu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Sent: Thursday, March 5, 2015 5:51 PM
To: airlied-cv59FeDIM0c@public.gmane.org
Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org; Arto Merilainen; Terje Bergstrom; gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; David Ung
Subject: [PATCH V2 1/3] gpu: host1x: Reduce host1x job allocation size

There is 2 set of num_relocs * sizeof(*) array at the end of host1x job.
Only the 1st set is really used and with job->gather_addr_phys pointing
somewhere within the 1st set of reloc physical addresses.
This patch removes the 2nd set of unused addresses.

Signed-off-by: David Ung <davidu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/gpu/host1x/job.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c
index 63bd63f..9c11265 100644
--- a/drivers/gpu/host1x/job.c
+++ b/drivers/gpu/host1x/job.c
@@ -46,8 +46,9 @@ struct host1x_job *host1x_job_alloc(struct host1x_channel *ch,
                (u64)num_unpins * sizeof(struct host1x_job_unpin_data) +
                (u64)num_waitchks * sizeof(struct host1x_waitchk) +
                (u64)num_cmdbufs * sizeof(struct host1x_job_gather) +
-               (u64)num_unpins * sizeof(dma_addr_t) +
-               (u64)num_unpins * sizeof(u32 *);
+               (u64)num_relocs * sizeof(dma_addr_t) +
+               (u64)num_cmdbufs * sizeof(dma_addr_t);
+
        if (total > ULONG_MAX)
                return NULL;

--
1.8.1.5

Re: [PATCH V2 1/3] gpu: host1x: Reduce host1x job allocation size

[Alexandre Courbot]

The series looks ok to me, the only problem I see is that we are
lacking means to thoroughly test host1x patches across all the
supported Tegra chips.

Could you detail what kind of testing this patch series has undergone,
and which chips/boards have been tested?

On Wed, Mar 18, 2015 at 9:53 AM, David Ung <davidu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> ping?
>
> ________________________________________
> From: David Ung <davidu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> Sent: Thursday, March 5, 2015 5:51 PM
> To: airlied-cv59FeDIM0c@public.gmane.org
> Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org; Arto Merilainen; Terje Bergstrom; gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; David Ung
> Subject: [PATCH V2 1/3] gpu: host1x: Reduce host1x job allocation size
>
> There is 2 set of num_relocs * sizeof(*) array at the end of host1x job.
> Only the 1st set is really used and with job->gather_addr_phys pointing
> somewhere within the 1st set of reloc physical addresses.
> This patch removes the 2nd set of unused addresses.
>
> Signed-off-by: David Ung <davidu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/gpu/host1x/job.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c
> index 63bd63f..9c11265 100644
> --- a/drivers/gpu/host1x/job.c
> +++ b/drivers/gpu/host1x/job.c
> @@ -46,8 +46,9 @@ struct host1x_job *host1x_job_alloc(struct host1x_channel *ch,
>                 (u64)num_unpins * sizeof(struct host1x_job_unpin_data) +
>                 (u64)num_waitchks * sizeof(struct host1x_waitchk) +
>                 (u64)num_cmdbufs * sizeof(struct host1x_job_gather) +
> -               (u64)num_unpins * sizeof(dma_addr_t) +
> -               (u64)num_unpins * sizeof(u32 *);
> +               (u64)num_relocs * sizeof(dma_addr_t) +
> +               (u64)num_cmdbufs * sizeof(dma_addr_t);
> +
>         if (total > ULONG_MAX)
>                 return NULL;
>
> --
> 1.8.1.5
>

RE: [PATCH V2 1/3] gpu: host1x: Reduce host1x job allocation size

[David Ung]

> -----Original Message-----
> From: Alexandre Courbot [mailto:gnurou@gmail.com]
> Sent: Monday, March 23, 2015 12:09 AM
> To: David Ung
> Cc: airlied@linux.ie; linux-tegra@vger.kernel.org;
> thierry.reding@gmail.com; swarren@wwwdotorg.org; Arto Merilainen;
> Terje Bergstrom
> Subject: Re: [PATCH V2 1/3] gpu: host1x: Reduce host1x job allocation size
> 
> The series looks ok to me, the only problem I see is that we are lacking
> means to thoroughly test host1x patches across all the supported Tegra
> chips.
> 
> Could you detail what kind of testing this patch series has undergone, and
> which chips/boards have been tested?

Tested the host1x channel tests for T132 on A44.
Also ran Paul's EIMT set up which only tests booting kernel to console for T20/T30/T114/T124 and QEMU

Re: [PATCH V2 1/3] gpu: host1x: Reduce host1x job allocation size

[Alexandre Courbot]

On Tue, Mar 24, 2015 at 4:39 AM, David Ung <davidu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>
>> -----Original Message-----
>> From: Alexandre Courbot [mailto:gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
>> Sent: Monday, March 23, 2015 12:09 AM
>> To: David Ung
>> Cc: airlied-cv59FeDIM0c@public.gmane.org; linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
>> thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org; Arto Merilainen;
>> Terje Bergstrom
>> Subject: Re: [PATCH V2 1/3] gpu: host1x: Reduce host1x job allocation size
>>
>> The series looks ok to me, the only problem I see is that we are lacking
>> means to thoroughly test host1x patches across all the supported Tegra
>> chips.
>>
>> Could you detail what kind of testing this patch series has undergone, and
>> which chips/boards have been tested?
>
> Tested the host1x channel tests for T132 on A44.
> Also ran Paul's EIMT set up which only tests booting kernel to console for T20/T30/T114/T124 and QEMU

Thierry, what do you think?

I know we are lacking proper testing infrastructure for host1x, but
these patches really looks harmless to me.

RE: [PATCH V2 1/3] gpu: host1x: Reduce host1x job allocation size

[David Ung]

> -----Original Message-----
> From: Alexandre Courbot [mailto:gnurou@gmail.com]
> Sent: Monday, March 30, 2015 11:30 PM
> To: David Ung; thierry.reding@gmail.com
> Cc: airlied@linux.ie; linux-tegra@vger.kernel.org;
> swarren@wwwdotorg.org; Arto Merilainen; Terje Bergstrom
> Subject: Re: [PATCH V2 1/3] gpu: host1x: Reduce host1x job allocation size
> 
> On Tue, Mar 24, 2015 at 4:39 AM, David Ung <davidu@nvidia.com> wrote:
> >
> >> -----Original Message-----
> >> From: Alexandre Courbot [mailto:gnurou@gmail.com]
> >> Sent: Monday, March 23, 2015 12:09 AM
> >> To: David Ung
> >> Cc: airlied@linux.ie; linux-tegra@vger.kernel.org;
> >> thierry.reding@gmail.com; swarren@wwwdotorg.org; Arto Merilainen;
> >> Terje Bergstrom
> >> Subject: Re: [PATCH V2 1/3] gpu: host1x: Reduce host1x job allocation
> >> size
> >>
> >> The series looks ok to me, the only problem I see is that we are
> >> lacking means to thoroughly test host1x patches across all the
> >> supported Tegra chips.
> >>
> >> Could you detail what kind of testing this patch series has
> >> undergone, and which chips/boards have been tested?
> >
> > Tested the host1x channel tests for T132 on A44.
> > Also ran Paul's EIMT set up which only tests booting kernel to console
> > for T20/T30/T114/T124 and QEMU
> 
> Thierry, what do you think?
> 
> I know we are lacking proper testing infrastructure for host1x, but these
> patches really looks harmless to me.


Thank you Alex for the review.
It would be good to have closure to this patchset. 

Thierry ping?

Re: [PATCH V2 1/3] gpu: host1x: Reduce host1x job allocation size

[Alexandre Courbot]

Forgot this:

Reviewed-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

On Fri, Apr 3, 2015 at 8:13 AM, David Ung <davidu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>> -----Original Message-----
>> From: Alexandre Courbot [mailto:gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
>> Sent: Monday, March 30, 2015 11:30 PM
>> To: David Ung; thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
>> Cc: airlied-cv59FeDIM0c@public.gmane.org; linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
>> swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org; Arto Merilainen; Terje Bergstrom
>> Subject: Re: [PATCH V2 1/3] gpu: host1x: Reduce host1x job allocation size
>>
>> On Tue, Mar 24, 2015 at 4:39 AM, David Ung <davidu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>> >
>> >> -----Original Message-----
>> >> From: Alexandre Courbot [mailto:gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
>> >> Sent: Monday, March 23, 2015 12:09 AM
>> >> To: David Ung
>> >> Cc: airlied-cv59FeDIM0c@public.gmane.org; linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
>> >> thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org; Arto Merilainen;
>> >> Terje Bergstrom
>> >> Subject: Re: [PATCH V2 1/3] gpu: host1x: Reduce host1x job allocation
>> >> size
>> >>
>> >> The series looks ok to me, the only problem I see is that we are
>> >> lacking means to thoroughly test host1x patches across all the
>> >> supported Tegra chips.
>> >>
>> >> Could you detail what kind of testing this patch series has
>> >> undergone, and which chips/boards have been tested?
>> >
>> > Tested the host1x channel tests for T132 on A44.
>> > Also ran Paul's EIMT set up which only tests booting kernel to console
>> > for T20/T30/T114/T124 and QEMU
>>
>> Thierry, what do you think?
>>
>> I know we are lacking proper testing infrastructure for host1x, but these
>> patches really looks harmless to me.
>
>
> Thank you Alex for the review.
> It would be good to have closure to this patchset.
>
> Thierry ping?
>

Re: [PATCH V2 1/3] gpu: host1x: Reduce host1x job allocation size

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2102 bytes --]

On Thu, Apr 02, 2015 at 11:13:38PM +0000, David Ung wrote:
> > -----Original Message-----
> > From: Alexandre Courbot [mailto:gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> > Sent: Monday, March 30, 2015 11:30 PM
> > To: David Ung; thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
> > Cc: airlied-cv59FeDIM0c@public.gmane.org; linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> > swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org; Arto Merilainen; Terje Bergstrom
> > Subject: Re: [PATCH V2 1/3] gpu: host1x: Reduce host1x job allocation size
> > 
> > On Tue, Mar 24, 2015 at 4:39 AM, David Ung <davidu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> > >
> > >> -----Original Message-----
> > >> From: Alexandre Courbot [mailto:gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> > >> Sent: Monday, March 23, 2015 12:09 AM
> > >> To: David Ung
> > >> Cc: airlied-cv59FeDIM0c@public.gmane.org; linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> > >> thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org; Arto Merilainen;
> > >> Terje Bergstrom
> > >> Subject: Re: [PATCH V2 1/3] gpu: host1x: Reduce host1x job allocation
> > >> size
> > >>
> > >> The series looks ok to me, the only problem I see is that we are
> > >> lacking means to thoroughly test host1x patches across all the
> > >> supported Tegra chips.
> > >>
> > >> Could you detail what kind of testing this patch series has
> > >> undergone, and which chips/boards have been tested?
> > >
> > > Tested the host1x channel tests for T132 on A44.
> > > Also ran Paul's EIMT set up which only tests booting kernel to console
> > > for T20/T30/T114/T124 and QEMU
> > 
> > Thierry, what do you think?
> > 
> > I know we are lacking proper testing infrastructure for host1x, but these
> > patches really looks harmless to me.
> 
> 
> Thank you Alex for the review.
> It would be good to have closure to this patchset. 

We'll get closure on this once the testing infrastructure appears. I
won't apply these patches before then.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]