| Message ID | 20181123123138.20739-1-thierry.reding@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/7] gpu: host1x: Resize channel register region on Tegra186 and later | expand |
Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com> On 23.11.2018 14:31, Thierry Reding 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 > enable_gather_filter(dev, ch); > return 0; > } >
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). Cheers, -ilia
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. Thierry
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?
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 enable_gather_filter(dev, ch); return 0; }