Re: [PATCH 1/7] gpu: host1x: Resize channel register region on Tegra186 and later

From: Dmitry Osipenko <digetx@gmail.com>
To: Thierry Reding <thierry.reding@gmail.com>,
	Ilia Mirkin <imirkin@alum.mit.edu>
Cc: linux-tegra@vger.kernel.org, jonathanh@nvidia.com,
	dri-devel <dri-devel@lists.freedesktop.org>,
	mperttunen@nvidia.com
Subject: Re: [PATCH 1/7] gpu: host1x: Resize channel register region on Tegra186 and later
Date: Mon, 26 Nov 2018 19:02:46 +0300	[thread overview]
Message-ID: <58731059-f3a5-3bbb-02d9-6a1701a60a54@gmail.com> (raw)
In-Reply-To: <20181126153013.GC19710@ulmo>

On 26.11.2018 18:30, Thierry Reding wrote:
> On Mon, Nov 26, 2018 at 10:11:39AM -0500, Ilia Mirkin wrote:
>> On Fri, Nov 23, 2018 at 7:31 AM Thierry Reding <thierry.reding@gmail.com> wrote:
>>>
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> The register region allocated per channel was decreased from 16384 bytes
>>> to 256 bytes on Tegra186 and later. Resize the region to make sure every
>>> channel (instead of only the first) is properly programmed.
>>>
>>> Suggested-by: Mikko Perttunen <mperttunen@nvidia.com>
>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>> ---
>>>  drivers/gpu/host1x/hw/channel_hw.c | 7 +++++--
>>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/host1x/hw/channel_hw.c b/drivers/gpu/host1x/hw/channel_hw.c
>>> index d188f9068b91..95ea81172a83 100644
>>> --- a/drivers/gpu/host1x/hw/channel_hw.c
>>> +++ b/drivers/gpu/host1x/hw/channel_hw.c
>>> @@ -26,7 +26,6 @@
>>>  #include "../intr.h"
>>>  #include "../job.h"
>>>
>>> -#define HOST1X_CHANNEL_SIZE 16384
>>>  #define TRACE_MAX_LENGTH 128U
>>>
>>>  static void trace_write_gather(struct host1x_cdma *cdma, struct host1x_bo *bo,
>>> @@ -203,7 +202,11 @@ static void enable_gather_filter(struct host1x *host,
>>>  static int host1x_channel_init(struct host1x_channel *ch, struct host1x *dev,
>>>                                unsigned int index)
>>>  {
>>> -       ch->regs = dev->regs + index * HOST1X_CHANNEL_SIZE;
>>> +#if HOST1X_HW < 6
>>> +       ch->regs = dev->regs + index * 0x4000;
>>> +#else
>>> +       ch->regs = dev->regs + index * 0x100;
>>> +#endif
>>
>> Just an observation ... this makes it impossible to build this module
>> for multiple host1x hw revisions in the same kernel. I believe that
>> supporting multiple platforms is frequently desirable, but perhaps
>> there's more going on here (like arm64 vs arm32, etc).
> 
> It actually does work. If you look close enough this file is included
> multiple times by generation specific source files. It's sort of like
> a template if you want.
> 
> It's not a design that's entirely to my liking, but it's not been a
> strong enough itch to scratch for me to have gotten around to changing
> how it works.

I'm currently in process of rewriting host1x driver to make it usable for the new DRM job-submission [U]API. In the process of rewriting I tried to re-design this part of the driver to avoid duplication of the code by unifying all the code using "if (host->soc.hw_ver..", but yesterday returned to the initial template-variant because of the amount of code-churning that the if-else approach introduces, it is especially not nice with the more bitfield differences that newer HW has. Do you have any other suggestions?
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel