From: Robin Murphy <robin.murphy@arm.com>
To: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
devicetree@vger.kernel.org, Yong Deng <yong.deng@magewell.com>,
Arnd Bergmann <arnd@arndb.de>,
Daniel Vetter <daniel.vetter@ffwll.ch>,
dri-devel@lists.freedesktop.org,
Dave Martin <dave.martin@arm.com>,
Paul Kocialkowski <paul.kocialkowski@bootlin.com>,
Chen-Yu Tsai <wens@csie.org>, Rob Herring <robh+dt@kernel.org>,
Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
Frank Rowand <frowand.list@gmail.com>,
Georgi Djakov <georgi.djakov@linaro.org>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v3 5/7] drm/sun4i: Rely on dma interconnect for our RAM offset
Date: Tue, 5 Mar 2019 16:11:58 +0000 [thread overview]
Message-ID: <7f089cc2-8d39-80e8-ee81-67372de618b8@arm.com> (raw)
In-Reply-To: <20190219105551.hyq3nefz6iueyjt3@flea>
On 19/02/2019 10:55, Maxime Ripard wrote:
> Hi Robin,
>
> On Wed, Feb 13, 2019 at 04:40:11PM +0000, Robin Murphy wrote:
>> On 13/02/2019 15:41, Maxime Ripard wrote:
>>> Hi Robin,
>>>
>>> Thanks for your feedback!
>>>
>>> On Tue, Feb 12, 2019 at 06:46:40PM +0000, Robin Murphy wrote:
>>>> On 11/02/2019 15:02, Maxime Ripard wrote:
>>>>> Now that we can express our DMA topology, rely on those property instead of
>>>>> hardcoding an offset from the dma_addr_t which wasn't really great.
>>>>>
>>>>> We still need to add some code to deal with the old DT that would lack that
>>>>> property, but we move the offset to the DRM device dma_pfn_offset to be
>>>>> able to rely on just the dma_addr_t associated to the GEM object.
>>>>>
>>>>> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
>>>>> ---
>>>>> drivers/gpu/drm/sun4i/sun4i_backend.c | 28 +++++++++++++++++++++-------
>>>>> 1 file changed, 21 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
>>>>> index 9e9255ee59cd..1846a1b30fea 100644
>>>>> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
>>>>> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
>>>>> @@ -383,13 +383,6 @@ int sun4i_backend_update_layer_buffer(struct sun4i_backend *backend,
>>>>> paddr = drm_fb_cma_get_gem_addr(fb, state, 0);
>>>>> DRM_DEBUG_DRIVER("Setting buffer address to %pad\n", &paddr);
>>>>> - /*
>>>>> - * backend DMA accesses DRAM directly, bypassing the system
>>>>> - * bus. As such, the address range is different and the buffer
>>>>> - * address needs to be corrected.
>>>>> - */
>>>>> - paddr -= PHYS_OFFSET;
>>>>> -
>>>>> if (fb->format->is_yuv)
>>>>> return sun4i_backend_update_yuv_buffer(backend, fb, paddr);
>>>>> @@ -835,6 +828,27 @@ static int sun4i_backend_bind(struct device *dev, struct device *master,
>>>>> dev_set_drvdata(dev, backend);
>>>>> spin_lock_init(&backend->frontend_lock);
>>>>> + if (of_find_property(dev->of_node, "interconnects", NULL)) {
>>>>> + /*
>>>>> + * This assume we have the same DMA constraints for all our the
>>>>> + * devices in our pipeline (all the backends, but also the
>>>>> + * frontends). This sounds bad, but it has always been the case
>>>>> + * for us, and DRM doesn't do per-device allocation either, so
>>>>> + * we would need to fix DRM first...
>>>>> + */
>>>>> + ret = of_dma_configure(drm->dev, dev->of_node, true);
>>>>
>>>> It would be even nicer if we could ensure that drm->dev originates from a DT
>>>> node which has the appropriate interconnects property itself, such that we
>>>> can assume it's already configured correctly.
>>>
>>> The thing is drm->dev comes from a node in the DT that is a virtual
>>> node, and therefore doesn't have any resources attached, so I'm not
>>> sure we have any other way, unfortunately.
>>
>> Right, I appreciate that it may not be feasible to swizzle drm->dev for one
>> of the 'real' component devices, but what I was also thinking was that since
>> the virtual device node effectively represents the aggregation of the other
>> component devices, we could just say that it also has to have its own link
>> to the MBUS interconnect (with the ID of the pipeline entrypoint it's
>> associated with, I guess). That ought to be enough to get dma_configure() to
>> do the job, and in fairness is no *less* accurate a description of the
>> hardware, even if might look a little funky to some.
>
> That virtual device however can work with up to 4 devices that can
> perform DMA operations, all of them going through a different port of
> the MBUS controller that can account for DMA usage on a per-master
> basis.
>
> Eventually, we should support that feature as well, but the point is
> that from a DT point of view, there's not a single point of DMA access
> for that particular device but more likely 2-4 points, each with a
> different argument to the phandle.
>
> We could of course have a hack and use only one of them, but that
> would be less accurate than what we currently have. Plus, this is
> really due to a restriction on the DRM side, that could change in the
> future (even though it's unlikely). Fixing that restriction would make
> that property completely useless, confusing and wrong from an hardware
> PoV.
Thanks for the clarification. Much as I'd like to keep
of_dma_configure() out of drivers as much as possible, I agree that it's
not sufficient justification for (ab)using DT to sidestep a
Linux-specific issue which might eventually get properly fixed anyway.
I'm sure the last thing sunxi needs is any more awkward DT ABI concerns :)
Robin.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-03-05 16:12 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-11 15:02 [PATCH v3 0/7] sunxi: Add DT representation for the MBUS controller Maxime Ripard
2019-02-11 15:02 ` [PATCH v3 1/7] dt-bindings: interconnect: Add a dma interconnect name Maxime Ripard
2019-03-01 17:48 ` Georgi Djakov
2019-03-05 15:53 ` Maxime Ripard
2019-03-05 16:14 ` Robin Murphy
2019-03-07 15:15 ` Georgi Djakov
2019-03-07 15:47 ` Maxime Ripard
2019-03-07 16:09 ` Chen-Yu Tsai
2019-03-11 10:11 ` Maxime Ripard
2019-03-11 14:11 ` Chen-Yu Tsai
2019-03-13 11:48 ` Georgi Djakov
2019-02-11 15:02 ` [PATCH v3 2/7] dt-bindings: bus: Add binding for the Allwinner MBUS controller Maxime Ripard
2019-02-12 18:53 ` Robin Murphy
2019-02-11 15:02 ` [PATCH v3 3/7] of: address: Add parent pointer to the __of_translate_address args Maxime Ripard
2019-02-12 18:02 ` Robin Murphy
2019-02-11 15:02 ` [PATCH v3 4/7] of: address: Add support for the parent DMA bus Maxime Ripard
2019-02-12 18:15 ` Robin Murphy
2019-02-11 15:02 ` [PATCH v3 5/7] drm/sun4i: Rely on dma interconnect for our RAM offset Maxime Ripard
2019-02-12 18:46 ` Robin Murphy
2019-02-13 15:41 ` Maxime Ripard
2019-02-13 16:40 ` Robin Murphy
2019-02-19 10:55 ` Maxime Ripard
2019-03-05 16:11 ` Robin Murphy [this message]
2019-02-11 15:02 ` [PATCH v3 6/7] clk: sunxi-ng: sun5i: Export the MBUS clock Maxime Ripard
2019-02-11 15:02 ` [PATCH v3 7/7] ARM: dts: sun5i: Add the MBUS controller Maxime Ripard
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=7f089cc2-8d39-80e8-ee81-67372de618b8@arm.com \
--to=robin.murphy@arm.com \
--cc=arnd@arndb.de \
--cc=daniel.vetter@ffwll.ch \
--cc=dave.martin@arm.com \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=frowand.list@gmail.com \
--cc=georgi.djakov@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=mark.rutland@arm.com \
--cc=maxime.ripard@bootlin.com \
--cc=paul.kocialkowski@bootlin.com \
--cc=robh+dt@kernel.org \
--cc=thomas.petazzoni@bootlin.com \
--cc=wens@csie.org \
--cc=yong.deng@magewell.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).