On 05/22/2015 01:25 PM, Thierry Reding wrote:
* PGP Signed by an unknown key
On Thu, May 21, 2015 at 05:40:31PM +0300, Mikko Perttunen wrote:
On 05/21/2015 04:20 PM, Arto Merilainen wrote:
[...]
+static int vic_is_addr_reg(struct device *dev, u32 class, u32 offset, u32 val)
+{
+ struct vic *vic = dev_get_drvdata(dev);
+
+ /* handle host class */
+ if (class == HOST1X_CLASS_HOST1X) {
+ if (offset == 0x2b)
+ return true;
+ return false;
"return (offset == 0x2b);" perhaps?
I think this should really be extracted into a separate helper. If we
ever need to take into account additional offsets we would otherwise
have to extend every driver rather than just the helper.
I agree, that would be better.
Also I think the 0x2b should be replaced by some symbolic name.
According to the TRM 0x2b is the host1x class method named
NV_CLASS_HOST_INDCTRL_0. Oddly enough that doesn't seem to be an address
register. Instead the address seems to be in the INDOFF2 and INDOFF
methods (0x2c and 0x2d). I also can't tell from the TRM what exactly
these are supposed to do.
Arto, can you clarify?
This looks like an unfortunate mistake that got reproduced from gr2d and
gr3d.
The INDCTRL method is used for indirect register accessing and it allows
Host1x to read registers of an engine - or write data directly to
memory. It allow implementing context switch for the clients whose state
should be not change between jobs from the same application.
+ if (IS_ERR(vic->rst)) {
+ dev_err(&pdev->dev, "cannot get reset\n");
+ return PTR_ERR(vic->rst);
+ }
+
+ platform_set_drvdata(pdev, vic);
+
+ INIT_LIST_HEAD(&vic->client.base.list);
+ vic->client.base.ops = &vic_client_ops;
+ vic->client.base.dev = dev;
+ vic->client.base.class = vic_config->class_id;
+ vic->client.base.syncpts = syncpts;
+ vic->client.base.num_syncpts = 1;
+ vic->dev = dev;
+ vic->config = vic_config;
+
+ INIT_LIST_HEAD(&vic->client.list);
+ vic->client.ops = &vic_ops;
+
+ err = tegra_powergate_sequence_power_up(vic->config->powergate_id,
+ vic->clk, vic->rst);
+ if (err) {
+ dev_err(dev, "cannot turn on the device\n");
+ return err;
+ }
+
+ err = host1x_client_register(&vic->client.base);
+ if (err < 0) {
You used 'if (err) {' previously, so maybe also here.
For consistency with other Tegra DRM code these checks should use (at
least where possible) the (err < 0) notation.
Will fix.
- Arto
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
