From: eric@anholt.net (Eric Anholt)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/2 v2] drm/pl111: Enable device-specific assigned memory
Date: Tue, 17 Apr 2018 11:44:55 -0700 [thread overview]
Message-ID: <87in8pzn88.fsf@anholt.net> (raw)
In-Reply-To: <CACRpkdaT5SPQrdkHKG1tqvs=nqJ6G+BrUQyg0J-_aevMe1vK8g@mail.gmail.com>
Linus Walleij <linus.walleij@linaro.org> writes:
> On Sun, Apr 8, 2018 at 3:08 AM, Eric Anholt <eric@anholt.net> wrote:
>
>>> if (of_property_read_u32(dev->of_node, "max-memory-bandwidth",
>>> &priv->memory_bw)) {
>>> dev_info(dev, "no max memory bandwidth specified, assume unlimited\n");
>>> @@ -275,7 +280,8 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
>>> priv->regs = devm_ioremap_resource(dev, &amba_dev->res);
>>> if (IS_ERR(priv->regs)) {
>>> dev_err(dev, "%s failed mmio\n", __func__);
>>> - return PTR_ERR(priv->regs);
>>> + ret = PTR_ERR(priv->regs);
>>> + goto mem_rel;
>>> }
>>
>> Shouldn't this error path be jumping to dev_unref, as well? We're
>> trying to match drm_dev_alloc(), right?
>
> OK I fixed.
>
>> I'm still a little bothered that we're allowing imports to pl111 of CMA
>> buffers that we can't scan out. Could we just refuse all
>> .gem_prime_import*() on this platform?
>
> I am sorry but I do not understand how CMA, dmabuf and GEM play
> together to decode this and figure out what to do.
>
> Do you mean that if we find device assigned memory, i.e. that there
> is this special memory which is all the controller can scan out,
> I should just implement .gem_prime_impport() instead of the
> currently assigned drm_gem_prime_import() to something just
> returning return ERR_PTR(-EINVAL); so it always fails?
>
> What about .gem_prime_import_sg_table()? Exporting should
> work fine since the memory is always readable by CPU.
Oh, I think you still want drm_gem_prime_import()'s path for "I'm
importing an fd from this same driver to into another process". So,
yeah, have our .import_sg_table hook throw -EINVAL if called on the
device memory platform, and otherwise call down to
drm_gem_cma_prime_import_sg_table().
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180417/ae011011/attachment.sig>
WARNING: multiple messages have this Message-ID (diff)
From: Eric Anholt <eric@anholt.net>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Liviu Dudau <liviu.dudau@arm.com>,
"open list:DRM PANEL DRIVERS" <dri-devel@lists.freedesktop.org>,
Mali DP Maintainers <malidp@foss.arm.com>,
Daniel Vetter <daniel.vetter@intel.com>,
Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 2/2 v2] drm/pl111: Enable device-specific assigned memory
Date: Tue, 17 Apr 2018 11:44:55 -0700 [thread overview]
Message-ID: <87in8pzn88.fsf@anholt.net> (raw)
In-Reply-To: <CACRpkdaT5SPQrdkHKG1tqvs=nqJ6G+BrUQyg0J-_aevMe1vK8g@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 1887 bytes --]
Linus Walleij <linus.walleij@linaro.org> writes:
> On Sun, Apr 8, 2018 at 3:08 AM, Eric Anholt <eric@anholt.net> wrote:
>
>>> if (of_property_read_u32(dev->of_node, "max-memory-bandwidth",
>>> &priv->memory_bw)) {
>>> dev_info(dev, "no max memory bandwidth specified, assume unlimited\n");
>>> @@ -275,7 +280,8 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
>>> priv->regs = devm_ioremap_resource(dev, &amba_dev->res);
>>> if (IS_ERR(priv->regs)) {
>>> dev_err(dev, "%s failed mmio\n", __func__);
>>> - return PTR_ERR(priv->regs);
>>> + ret = PTR_ERR(priv->regs);
>>> + goto mem_rel;
>>> }
>>
>> Shouldn't this error path be jumping to dev_unref, as well? We're
>> trying to match drm_dev_alloc(), right?
>
> OK I fixed.
>
>> I'm still a little bothered that we're allowing imports to pl111 of CMA
>> buffers that we can't scan out. Could we just refuse all
>> .gem_prime_import*() on this platform?
>
> I am sorry but I do not understand how CMA, dmabuf and GEM play
> together to decode this and figure out what to do.
>
> Do you mean that if we find device assigned memory, i.e. that there
> is this special memory which is all the controller can scan out,
> I should just implement .gem_prime_impport() instead of the
> currently assigned drm_gem_prime_import() to something just
> returning return ERR_PTR(-EINVAL); so it always fails?
>
> What about .gem_prime_import_sg_table()? Exporting should
> work fine since the memory is always readable by CPU.
Oh, I think you still want drm_gem_prime_import()'s path for "I'm
importing an fd from this same driver to into another process". So,
yeah, have our .import_sg_table hook throw -EINVAL if called on the
device memory platform, and otherwise call down to
drm_gem_cma_prime_import_sg_table().
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2018-04-17 18:44 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-06 14:19 [PATCH 1/2 v2] drm/pl111: Support the Versatile Express Linus Walleij
2018-04-06 14:19 ` Linus Walleij
2018-04-06 14:19 ` [PATCH 2/2 v2] drm/pl111: Enable device-specific assigned memory Linus Walleij
2018-04-06 14:19 ` Linus Walleij
2018-04-08 1:08 ` Eric Anholt
2018-04-08 1:08 ` Eric Anholt
2018-04-17 12:23 ` Linus Walleij
2018-04-17 12:23 ` Linus Walleij
2018-04-17 18:44 ` Eric Anholt [this message]
2018-04-17 18:44 ` Eric Anholt
2018-04-08 1:16 ` [PATCH 1/2 v2] drm/pl111: Support the Versatile Express Eric Anholt
2018-04-08 1:16 ` Eric Anholt
2018-04-09 6:28 ` Linus Walleij
2018-04-09 6:28 ` Linus Walleij
2018-04-10 9:57 ` Liviu Dudau
2018-04-10 9:57 ` Liviu Dudau
2018-04-17 12:32 ` Linus Walleij
2018-04-17 12:32 ` Linus Walleij
2018-04-17 13:12 ` Robin Murphy
2018-04-17 13:12 ` Robin Murphy
2018-04-17 19:31 ` Linus Walleij
2018-04-17 19:31 ` Linus Walleij
2018-04-18 11:50 ` Robin Murphy
2018-04-18 11:50 ` Robin Murphy
2018-04-18 13:15 ` Liviu Dudau
2018-04-18 13:15 ` Liviu Dudau
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87in8pzn88.fsf@anholt.net \
--to=eric@anholt.net \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.