On Thu, Dec 25, 2014 at 10:42:58AM +0800, Vince Hsu wrote:
>
> On 12/24/2014 09:23 PM, Lucas Stach wrote:
> >Am Dienstag, den 23.12.2014, 18:39 +0800 schrieb Vince Hsu:
> >>This patch adds some missing pieces of the rail gaing/ungating sequence that
> >>can improve the stability in theory.
> >>
> >>Signed-off-by: Vince Hsu <vinceh@xxxxxxxxxx>
> >>---
> >> drm/nouveau_platform.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> >> drm/nouveau_platform.h | 3 +++
> >> 2 files changed, 45 insertions(+)
> >>
> >>diff --git a/drm/nouveau_platform.c b/drm/nouveau_platform.c
> >>index 68788b17a45c..527fe2358fc9 100644
> >>--- a/drm/nouveau_platform.c
> >>+++ b/drm/nouveau_platform.c
> >>@@ -25,9 +25,11 @@
> >> #include <linux/module.h>
> >> #include <linux/platform_device.h>
> >> #include <linux/of.h>
> >>+#include <linux/of_platform.h>
> >> #include <linux/reset.h>
> >> #include <linux/regulator/consumer.h>
> >> #include <soc/tegra/fuse.h>
> >>+#include <soc/tegra/mc.h>
> >> #include <soc/tegra/pmc.h>
> >> #include "nouveau_drm.h"
> >>@@ -61,6 +63,9 @@ static int nouveau_platform_power_up(struct nouveau_platform_gpu *gpu)
> >> reset_control_deassert(gpu->rst);
> >> udelay(10);
> >>+ tegra_mc_flush(gpu->mc, gpu->swgroup, false);
> >>+ udelay(10);
> >>+
> >> return 0;
> >> err_clamp:
> >>@@ -77,6 +82,14 @@ static int nouveau_platform_power_down(struct nouveau_platform_gpu *gpu)
> >> {
> >> int err;
> >>+ tegra_mc_flush(gpu->mc, gpu->swgroup, true);
> >>+ udelay(10);
> >>+
> >>+ err = tegra_powergate_gpu_set_clamping(true);
> >>+ if (err)
> >>+ return err;
> >>+ udelay(10);
> >>+
> >> reset_control_assert(gpu->rst);
> >> udelay(10);
> >>@@ -91,6 +104,31 @@ static int nouveau_platform_power_down(struct nouveau_platform_gpu *gpu)
> >> return 0;
> >> }
> >>+static int nouveau_platform_get_mc(struct device *dev,
> >>+ struct tegra_mc **mc, unsigned int *swgroup)
> >Uhm, no. If this is needed this has to be a Tegra MC function and not
> >burried into nouveau code. You are using knowledge about the internal
> >workings of the MC driver here.
> >
> >Also this should probably only take the Dt node pointer as argument and
> >return a something like a tegra_mc_client struct that contains both the
> >MC device pointer and the swgroup so you can pass that to
> >tegra_mc_flush().
> Good idea. I will have something as below in V2 if there is no other
> comments for this.
>
> tegra_mc_client *tegra_mc_find_client(struct device_node *node)
> {
> ...
> ret = of_parse_phandle_with_args(node, "nvidia,memory-client", ...)
> ...
> }
>
> There were some discussion about this few weeks ago. I'm not sure whether we
> have some conclusion/implementation though. Thierry?
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/308703.html
I don't think client is a good fit here. Flushing is done per SWGROUP
(on all clients of the SWGROUP). So I think we'll want something like:
gpu@0,57000000 {
...
nvidia,swgroup = <&mc TEGRA_SWGROUP_GPU>;
...
};
In the DT and return a struct tegra_mc_swgroup along the lines of:
struct tegra_mc_client {
unsigned int id;
unsigned int swgroup;
struct list_head list;
};
struct tegra_mc_swgroup {
struct list_head clients;
unsigned int id;
};
Where tegra_mc_swgroup.clients is a list of struct tegra_mc_client
structures, each representing a memory client pertaining to the
SWGROUP.
We probably don't want to expose these structures publicly, an opaque
type should be enough. Then you can use functions like:
struct tegra_mc_swgroup *tegra_mc_find_swgroup(struct device_node *node);
At some point we may even need something like:
struct tegra_mc_client *tegra_mc_find_client(struct device_node *node,
const char *name);
And DT content like this:
gpu@0,57000000 {
...
nvidia,memory-clients = <&mc 0x58>, <&mc 0x59>;
nvidia,memory-client-names = "read", "write";
...
};
This could be useful for latency allowance programming, but we can cross
that bridge when we come to it.
Thierry
Attachment:
pgpNiL4IMZVEv.pgp
Description: PGP signature
