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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?



[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux