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@xxxxxxxxx> wrote:
>>>
>>> From: Thierry Reding <treding@xxxxxxxxxx>
>>>
>>> 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@xxxxxxxxxx>
>>> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
>>> ---
>>> 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?
