Re: [PATCH] drm/tegra: fix a possible null pointer dereference
On Fri May 31, 2024 at 3:56 PM CEST, Huai-Yuan Liu wrote:
> In malidp_tegra_crtc_reset, new memory is allocated with kzalloc, but
> no check is performed. Before calling __drm_atomic_helper_crtc_reset,
> mw_state should be checked to prevent possible null pointer dereferene.
Please check that all these variable name references actually are valid.
Also, "dereference".
> Fixes: b7e0b04ae450 ("drm/tegra: Convert to using
> __drm_atomic_helper_crtc_reset() for reset.")
> Signed-off-by: Huai-Yuan Liu
> ---
> drivers/gpu/drm/tegra/dc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index be61c9d1a4f0..7648e129c212 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -1388,7 +1388,8 @@ static void tegra_crtc_reset(struct drm_crtc *crtc)
> if (crtc->state)
> tegra_crtc_atomic_destroy_state(crtc, crtc->state);
>
> - __drm_atomic_helper_crtc_reset(crtc, >base);
> + if (state)
> + __drm_atomic_helper_crtc_reset(crtc, >base);
Looking at how other drivers do this, they call
__drm_atomic_helper_crtc_reset(crtc, NULL);
if they fail to allocate a new state. Any reason why we wouldn't want to
do the same thing here?
Thierry
signature.asc
Description: PGP signature
Re: [PATCH] docs: document python version used for compilation
On Fri May 31, 2024 at 9:33 AM CEST, Geert Uytterhoeven wrote:
> Hi Thierry,
>
> On Thu, May 30, 2024 at 7:07 PM Thierry Reding
> wrote:
> > Alternatively, maybe Kconfig could be taught about build dependencies?
>
> git grep "depends on \$(" -- "*Kconf*"
Duh... of course there's something like this already. =)
Maybe something like the attached patch?
Thierry
From 153eaec61513e14f5a7f8f2a998600d07f17bc84 Mon Sep 17 00:00:00 2001
From: Thierry Reding
Date: Fri, 31 May 2024 10:51:42 +0200
Subject: [PATCH] kbuild: Allow Kconfig dependendencies on Python
Recently drivers have started depending on Python to generate register
definitions during the build process. In order to prevent such drivers
from breaking builds on systems that don't have Python installed, make
them depend on the minimum required Python version that they need via
Kconfig. If Python is not installed on the system, these drivers will
be automatically disabled.
Signed-off-by: Thierry Reding
---
drivers/gpu/drm/msm/Kconfig | 1 +
scripts/min-tool-version.sh | 3 +++
scripts/python-version.sh | 41 +
3 files changed, 45 insertions(+)
create mode 100755 scripts/python-version.sh
diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 1931ecf73e32..5f7f84de55e4 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -11,6 +11,7 @@ config DRM_MSM
depends on QCOM_LLCC || QCOM_LLCC=n
depends on QCOM_COMMAND_DB || QCOM_COMMAND_DB=n
depends on PM
+ depends on $(success,$(srctree)/scripts/python-version.sh)
select IOMMU_IO_PGTABLE
select QCOM_MDT_LOADER if ARCH_QCOM
select REGULATOR
diff --git a/scripts/min-tool-version.sh b/scripts/min-tool-version.sh
index 91c91201212c..447a3ad4c0bf 100755
--- a/scripts/min-tool-version.sh
+++ b/scripts/min-tool-version.sh
@@ -38,6 +38,9 @@ rustc)
bindgen)
echo 0.65.1
;;
+python)
+ echo 3.5.0
+ ;;
*)
echo "$1: unknown tool" >&2
exit 1
diff --git a/scripts/python-version.sh b/scripts/python-version.sh
new file mode 100755
index ..c997d40418dc
--- /dev/null
+++ b/scripts/python-version.sh
@@ -0,0 +1,41 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Print the Python version in a 5 or 6-digit form.
+# Also, perform the minimum version check.
+
+set -e
+
+PYTHON=${PYTHON:-python}
+
+get_canonical_version()
+{
+ IFS=.
+ set -- $1
+
+ # If the 2nd or 3rd field is missing, fill it with a zero.
+ echo $((1 * $1 + 100 * ${2:-0} + ${3:-0}))
+}
+
+output=$(LC_ALL=C "$PYTHON" --version)
+
+# Split the line on spaces.
+IFS=' '
+set -- $output
+name=$1
+version=$2
+
+min_tool_version=$(dirname $0)/min-tool-version.sh
+min_version=$($min_tool_version python)
+
+cversion=$(get_canonical_version $version)
+min_version=$(get_canonical_version $min_version)
+
+if [ "$cversion" -lt "$min_version" ]; then
+ echo >&2 "***"
+ echo >&2 "*** Python is too old."
+ echo >&2 "***"
+ exit 1
+fi
+
+echo $name $version
--
2.44.0
signature.asc
Description: PGP signature
Re: [BUG] Build failure and alleged fix for next-20240523
On Thu May 30, 2024 at 6:55 PM CEST, Paul E. McKenney wrote:
> On Fri, May 24, 2024 at 12:57:58PM -0700, Abhinav Kumar wrote:
> > Hello
> >
> > On 5/24/2024 12:55 PM, Paul E. McKenney wrote:
> > > Hello!
> > >
> > > I get the following allmodconfig build error on x86 in next-20240523:
> > >
> > > Traceback (most recent call last):
> > > File "drivers/gpu/drm/msm/registers/gen_header.py", line 970, in
> > >
> > > main()
> > > File "drivers/gpu/drm/msm/registers/gen_header.py", line 951, in main
> > > parser.add_argument('--validate',
> > > action=argparse.BooleanOptionalAction)
> > > AttributeError: module 'argparse' has no attribute 'BooleanOptionalAction'
> > >
> > > The following patch allows the build to complete successfully:
> > >
> > > https://patchwork.kernel.org/project/dri-devel/patch/20240508091751.336654-1-jonath...@nvidia.com/#25842751
> > >
> > > As to whether this is a proper fix, I must defer to the DRM folks on CC.
> >
> > Thanks for the report.
> >
> > I have raised a merge request for
> > https://patchwork.freedesktop.org/patch/593057/ to make it available for the
> > next fixes release for msm.
>
> Thank you!
>
> This still is not in -next, so I am putting it into -rcu just to silence
> the diagnostic. Or should I push this to mainline via -rcu?
I pushed this to drm-misc-fixes earlier today, so should show up in
linux-next soon and hopefully in v6.10-rc2.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH] docs: document python version used for compilation
On Fri May 10, 2024 at 10:04 PM CEST, Rob Clark wrote: > On Fri, May 10, 2024 at 3:09 AM Jani Nikula wrote: > > > > On Fri, 10 May 2024, Mauro Carvalho Chehab wrote: > > > Em Fri, 10 May 2024 11:08:38 +0300 > > > Jani Nikula escreveu: > > > > > >> On Thu, 09 May 2024, Dmitry Baryshkov > > >> wrote: > > >> > The drm/msm driver had adopted using Python3 script to generate > > >> > register > > >> > header files instead of shipping pre-generated header files. Document > > >> > the minimal Python version supported by the script. > > >> > > > >> > Signed-off-by: Dmitry Baryshkov > > >> > --- > > >> > Documentation/process/changes.rst | 1 + > > >> > 1 file changed, 1 insertion(+) > > >> > > > >> > diff --git a/Documentation/process/changes.rst > > >> > b/Documentation/process/changes.rst > > >> > index 5685d7bfe4d0..8d225a9f65a2 100644 > > >> > --- a/Documentation/process/changes.rst > > >> > +++ b/Documentation/process/changes.rst > > >> > @@ -63,6 +63,7 @@ cpio any cpio > > >> > --version > > >> > GNU tar1.28 tar --version > > >> > gtags (optional) 6.6.5gtags --version > > >> > mkimage (optional) 2017.01 mkimage --version > > >> > +Python (optional) 3.5.xpython3 --version > > >> > > >> Python 3.5 reached end-of-life 3½ years ago [1]. What's the point in > > >> using anything older than the oldest supported version of Python, > > >> i.e. 3.8 at this time? > > > > > > What's the point of breaking compilation with on older distros? > > > The idea of minimal versions here is to specify the absolute minimum > > > version that it is required for the build to happen. If 3.5 is > > > the minimal one, then be it. > > > > AFAICT 3.5 was an arbitrary rather than a deliberate choice. We should > > at least be aware *why* we'd be sticking to old versions. > > > > Minimum versions here also means sticking to features available in said > > versions, for Python just as well as for GCC or any other tool. That's > > not zero cost. > > At this point, the cost to having a lower minimum version is pretty > small, so I'm not worrying too much about it. > > Maybe once kernel developers discover mako, and start generating more > at build time, we'll have to re-evaluate. ;-) You're making an interesting point. Does the build dependency here denote Python (& standard library) or do we assume that if people have Python installed that they can also install arbitrary extra packages? Would a Mako dependency need to be explicitly mentioned here? Thierry signature.asc Description: PGP signature
Re: [PATCH] docs: document python version used for compilation
On Thu May 9, 2024 at 6:48 PM CEST, Jonathan Corbet wrote: > Dmitry Baryshkov writes: > > > The drm/msm driver had adopted using Python3 script to generate register > > header files instead of shipping pre-generated header files. Document > > the minimal Python version supported by the script. > > > > Signed-off-by: Dmitry Baryshkov > > --- > > Documentation/process/changes.rst | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/Documentation/process/changes.rst > > b/Documentation/process/changes.rst > > index 5685d7bfe4d0..8d225a9f65a2 100644 > > --- a/Documentation/process/changes.rst > > +++ b/Documentation/process/changes.rst > > @@ -63,6 +63,7 @@ cpio any cpio --version > > GNU tar1.28 tar --version > > gtags (optional) 6.6.5gtags --version > > mkimage (optional) 2017.01 mkimage --version > > +Python (optional) 3.5.xpython3 --version > > == === > > > > Is it really optional - can you build the driver without it? > > This document needs some help... I'm missing a number of things that are > *not* marked as "optional" (jfsutils, reiserfsprogs, pcmciautils, ppp, > ...) and somehow my system works fine :) It would be nice to document > *why* users might need a specific tool. > > But I guess we aren't going to do that now. I can apply this, but I do > wonder about the "optional" marking. I guess it depends a bit on what exactly "optional" implies. It's optional in the sense that you can easily disable the driver and then build without Python. So does "optional" mean that allmodconfig for all platforms builds without the dependency? Or does it mean some definition of "core" kernel builds for a set of defined platforms? Maybe this really needs to be annotated with the exact Kconfig options that need this. Although that could get out of hands rather quickly. At some point we may have to list a *lot* of these options. Alternatively, maybe Kconfig could be taught about build dependencies? Thierry signature.asc Description: PGP signature
Re: [PATCH] drm/msm: remove python 3.9 dependency for compiling msm
On Wed May 8, 2024 at 1:04 AM CEST, Abhinav Kumar wrote:
> Since commit 5acf49119630 ("drm/msm: import gen_header.py script from Mesa"),
> compilation is broken on machines having python versions older than 3.9
> due to dependency on argparse.BooleanOptionalAction.
>
> Switch to use simple bool for the validate flag to remove the dependency.
>
> Fixes: 5acf49119630 ("drm/msm: import gen_header.py script from Mesa")
> Signed-off-by: Abhinav Kumar
> ---
> drivers/gpu/drm/msm/registers/gen_header.py | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
Irrespective of whether we want to allow Python as a build dependency or
not, it already is since v6.10-rc1, so in the meantime I'm going to
apply this to drm-misc-fixes to unbreak things.
If we decide that we don't want the extra dependency we need revert the
whole generation infrastructure.
Thierry
signature.asc
Description: PGP signature
Re: (subset) [PATCH v7 0/5] Add Tegra Security Engine driver
On Fri Apr 26, 2024 at 5:32 PM CEST, Thierry Reding wrote: > From: Thierry Reding > > > On Wed, 03 Apr 2024 15:30:34 +0530, Akhil R wrote: > > Add support for Tegra Security Engine which can accelerates various > > crypto algorithms. The Engine has two separate instances within for > > AES and HASH algorithms respectively. > > > > The driver registers two crypto engines - one for AES and another for > > HASH algorithms and these operate independently and both uses the host1x > > bus. Additionally, it provides hardware-assisted key protection for up to > > 15 symmetric keys which it can use for the cipher operations. > > > > [...] > > Applied, thanks! > > [4/5] arm64: defconfig: Enable Tegra Security Engine > commit: 4d4d3fe6b3cc2a0b2a334a08bb9c64ba1dcbbea4 For the record, I've also applied patch 5/5 but it didn't apply cleanly and so b4 didn't track it properly. Thanks, Thierry signature.asc Description: PGP signature
Re: (subset) [PATCH v7 0/5] Add Tegra Security Engine driver
From: Thierry Reding On Wed, 03 Apr 2024 15:30:34 +0530, Akhil R wrote: > Add support for Tegra Security Engine which can accelerates various > crypto algorithms. The Engine has two separate instances within for > AES and HASH algorithms respectively. > > The driver registers two crypto engines - one for AES and another for > HASH algorithms and these operate independently and both uses the host1x > bus. Additionally, it provides hardware-assisted key protection for up to > 15 symmetric keys which it can use for the cipher operations. > > [...] Applied, thanks! [4/5] arm64: defconfig: Enable Tegra Security Engine commit: 4d4d3fe6b3cc2a0b2a334a08bb9c64ba1dcbbea4 Best regards, -- Thierry Reding
Re: [PATCH] gpu: host1x: mipi: Benefit from devm_clk_get_prepared()
On Tue Apr 9, 2024 at 6:50 PM CEST, Uwe Kleine-König wrote: > When using devm_clk_get_prepared() instead of devm_clk_get() the clock > is already returned prepared. So probe doesn't need to call > clk_prepare() and at remove time the call to clk_unprepare() can be > dropped. The latter makes the remove callback empty, so it can be > dropped, too. > > Signed-off-by: Uwe Kleine-König > --- > Hello, > > the motivation for this patch is that the driver uses struct > platform_driver::remove() which I plan to change the prototype of. Instead > of converting the driver to the temporal .remove_new() and then back to > the new .remove(), drop the remove callback completely. > > Best regards > Uwe > > drivers/gpu/host1x/mipi.c | 17 + > 1 file changed, 1 insertion(+), 16 deletions(-) Feel free to take this in through whatever tree is most appropriate for this ongoing work: Acked-by: Thierry Reding signature.asc Description: PGP signature
Re: [PATCH 3/4] gpu: host1x: Convert to platform remove callback returning void
On Tue Apr 9, 2024 at 7:02 PM CEST, Uwe Kleine-König wrote: > The .remove() callback for a platform driver returns an int which makes > many driver authors wrongly assume it's possible to do error handling by > returning an error code. However the value returned is ignored (apart > from emitting a warning) and this typically results in resource leaks. > > To improve here there is a quest to make the remove callback return > void. In the first step of this quest all drivers are converted to > .remove_new(), which already returns void. Eventually after all drivers > are converted, .remove_new() will be renamed to .remove(). > > Trivially convert this driver from always returning zero in the remove > callback to the void returning variant. > > Signed-off-by: Uwe Kleine-König > --- > drivers/gpu/host1x/dev.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) Acked-by: Thierry Reding signature.asc Description: PGP signature
Re: [PATCH 0/4] gpu: Convert to platform remove callback returning void
On Fri Apr 19, 2024 at 9:20 AM CEST, Uwe Kleine-König wrote:
> Hello,
>
> On Tue, Apr 09, 2024 at 07:02:47PM +0200, Uwe Kleine-König wrote:
> > with some patches sent earlier[1], this series converts all platform
> > drivers below drivers/gpu to not use struct platform_device::remove()
> > any more.
> >
> > See commit 5c5a7680e67b ("platform: Provide a remove callback that
> > returns no value") for an extended explanation and the eventual goal.
> >
> > All conversations are trivial, because the driver's .remove() callbacks
> > returned zero unconditionally.
> >
> > There are no interdependencies between these patches. This is merge
> > window material.
>
> I wonder how this series will make it in. While I would prefer these
> patches to go in together (that I can consider this thread completed in
> one go), I think with how drm maintenace works, it's best if the patches
> are picked up by their individual maintainers. I guess that's:
>
> - Frank Binns + Matt Coster for imagination
>
> - Chun-Kuang Hu + Philipp Zabel for mediatek
>
> - Thierry Reding + Mikko Perttunen for the host1x driver
>(Note there is another patch for this driver set at
> 20240409165043.105137-2-u.kleine-koe...@pengutronix.de that is
> relevant for the same quest.)
>
> - Philipp Zabel for ipu-v3
>
> I plan to send a patch changing struct platform_driver::remove after the
> end of the merge window leading to 6.10-rc1 for inclusion in next via
> Greg's driver core. So please either care the patches land in 6.10-rc1
> or ack that I include them in the submission to Greg.
I think the latter would make more sense. I'll go ack those patches.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH] gpu: host1x: Do not setup DMA for virtual devices
On Wed Apr 3, 2024 at 12:07 PM CEST, Jon Hunter wrote:
> Hi Thierry,
>
> On 15/03/2024 11:25, Jon Hunter wrote:
> >
> > On 14/03/2024 15:49, Thierry Reding wrote:
> >> From: Thierry Reding
> >>
> >> The host1x devices are virtual compound devices and do not perform DMA
> >> accesses themselves, so they do not need to be set up for DMA.
> >>
> >> Ideally we would also not need to set up DMA masks for the virtual
> >> devices, but we currently still need those for legacy support on old
> >> hardware.
> >>
> >> Signed-off-by: Thierry Reding
> >> ---
> >> drivers/gpu/host1x/bus.c | 8
> >> 1 file changed, 8 deletions(-)
> >>
> >> diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c
> >> index 783975d1384f..7c52757a89db 100644
> >> --- a/drivers/gpu/host1x/bus.c
> >> +++ b/drivers/gpu/host1x/bus.c
> >> @@ -351,11 +351,6 @@ static int host1x_device_uevent(const struct
> >> device *dev,
> >> return 0;
> >> }
> >> -static int host1x_dma_configure(struct device *dev)
> >> -{
> >> - return of_dma_configure(dev, dev->of_node, true);
> >> -}
> >> -
> >> static const struct dev_pm_ops host1x_device_pm_ops = {
> >> .suspend = pm_generic_suspend,
> >> .resume = pm_generic_resume,
> >> @@ -369,7 +364,6 @@ const struct bus_type host1x_bus_type = {
> >> .name = "host1x",
> >> .match = host1x_device_match,
> >> .uevent = host1x_device_uevent,
> >> - .dma_configure = host1x_dma_configure,
> >> .pm = _device_pm_ops,
> >> };
> >> @@ -458,8 +452,6 @@ static int host1x_device_add(struct host1x *host1x,
> >> device->dev.bus = _bus_type;
> >> device->dev.parent = host1x->dev;
> >> - of_dma_configure(>dev, host1x->dev->of_node, true);
> >> -
> >> device->dev.dma_parms = >dma_parms;
> >> dma_set_max_seg_size(>dev, UINT_MAX);
> >
> >
> > Tested-by: Jon Hunter
> > Acked-by: Jon Hunter
>
>
> I don't see this in -next yet?
>
> Ideally, if we don't see any issues with this we should pull this into
> v6.8.y stable branch because I am now seeing the warning there. Should
> we apply a fixes tag to this?
I was finally able to run some finally tests on this and pushed it to
drm-misc-fixes, so it should go into linux-next and then Linus' tree
sometime soon.
I decided against adding a Fixes tag because it's difficult to backport
this all the way to the release which contains the commit that added
the issue. Adding a Fixes tag to the commit that ended up exposing the
issue didn't seem right either, so let's get this into mainline first
and then manually ask stable maintainers to pick this up.
Thierry
signature.asc
Description: PGP signature
[PATCH] gpu: host1x: Do not setup DMA for virtual devices
From: Thierry Reding
The host1x devices are virtual compound devices and do not perform DMA
accesses themselves, so they do not need to be set up for DMA.
Ideally we would also not need to set up DMA masks for the virtual
devices, but we currently still need those for legacy support on old
hardware.
Signed-off-by: Thierry Reding
---
drivers/gpu/host1x/bus.c | 8
1 file changed, 8 deletions(-)
diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c
index 783975d1384f..7c52757a89db 100644
--- a/drivers/gpu/host1x/bus.c
+++ b/drivers/gpu/host1x/bus.c
@@ -351,11 +351,6 @@ static int host1x_device_uevent(const struct device *dev,
return 0;
}
-static int host1x_dma_configure(struct device *dev)
-{
- return of_dma_configure(dev, dev->of_node, true);
-}
-
static const struct dev_pm_ops host1x_device_pm_ops = {
.suspend = pm_generic_suspend,
.resume = pm_generic_resume,
@@ -369,7 +364,6 @@ const struct bus_type host1x_bus_type = {
.name = "host1x",
.match = host1x_device_match,
.uevent = host1x_device_uevent,
- .dma_configure = host1x_dma_configure,
.pm = _device_pm_ops,
};
@@ -458,8 +452,6 @@ static int host1x_device_add(struct host1x *host1x,
device->dev.bus = _bus_type;
device->dev.parent = host1x->dev;
- of_dma_configure(>dev, host1x->dev->of_node, true);
-
device->dev.dma_parms = >dma_parms;
dma_set_max_seg_size(>dev, UINT_MAX);
--
2.44.0
[PATCH] drm: Remove drm_num_crtcs() helper
From: Thierry Reding
The drm_num_crtcs() helper determines the number of CRTCs by iterating
over the list of CRTCs that have been registered with the mode config.
However, we already keep track of that number in the mode config's
num_crtcs field, so we can simply retrieve the value from that and
remove the extra helper function.
Signed-off-by: Thierry Reding
---
drivers/gpu/drm/drm_crtc.c | 15 +--
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 6795624f16e7..82c665d3e74b 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -107,18 +107,6 @@ int drm_crtc_force_disable(struct drm_crtc *crtc)
return drm_mode_set_config_internal();
}
-static unsigned int drm_num_crtcs(struct drm_device *dev)
-{
- unsigned int num = 0;
- struct drm_crtc *tmp;
-
- drm_for_each_crtc(tmp, dev) {
- num++;
- }
-
- return num;
-}
-
int drm_crtc_register_all(struct drm_device *dev)
{
struct drm_crtc *crtc;
@@ -278,8 +266,7 @@ static int __drm_crtc_init_with_planes(struct drm_device
*dev, struct drm_crtc *
if (name) {
crtc->name = kvasprintf(GFP_KERNEL, name, ap);
} else {
- crtc->name = kasprintf(GFP_KERNEL, "crtc-%d",
- drm_num_crtcs(dev));
+ crtc->name = kasprintf(GFP_KERNEL, "crtc-%d", config->num_crtc);
}
if (!crtc->name) {
drm_mode_object_unregister(dev, >base);
--
2.44.0
Re: [PATCH v2] drm/tegra: Remove existing framebuffer only if we support display
On Mon Feb 26, 2024 at 1:08 PM CET, Robert Foss wrote:
> On Mon, Feb 26, 2024 at 12:36 PM Javier Martinez Canillas
> wrote:
> >
> > Thomas Zimmermann writes:
> >
> > > Hi
> > >
> > > Am 23.02.24 um 16:03 schrieb Thierry Reding:
> > >> From: Thierry Reding
> > >>
> > >> Tegra DRM doesn't support display on Tegra234 and later, so make sure
> > >> not to remove any existing framebuffers in that case.
> > >>
> > >> v2: - add comments explaining how this situation can come about
> > >> - clear DRIVER_MODESET and DRIVER_ATOMIC feature bits
> > >>
>
> Fixes: 6848c291a54f ("drm/aperture: Convert drivers to aperture interfaces")
>
> Maybe this is more of a philosophical question, but either the
> introduction of this hardware generation is where this regression was
> introduced or this possibly this commit.
>
> Either way, I'd like to get this into the drm-misc-fixes branch.
That commit looks about right. Technically Tegra234 support was
introduced in Linux 5.10 but the first platform where you we would've
seen this wasn't added until 5.17. The above commit is from 5.14, which
puts it about right in between there, so I think that's fine.
Backporting to anything before 5.14 would need to be manual and isn't
worth it.
Thierry
signature.asc
Description: PGP signature
[PATCH v2] drm/tegra: Remove existing framebuffer only if we support display
From: Thierry Reding
Tegra DRM doesn't support display on Tegra234 and later, so make sure
not to remove any existing framebuffers in that case.
v2: - add comments explaining how this situation can come about
- clear DRIVER_MODESET and DRIVER_ATOMIC feature bits
Signed-off-by: Thierry Reding
---
drivers/gpu/drm/tegra/drm.c | 23 ---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index b1e1a78e30c6..2e1cfe6f6d74 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -1220,9 +1220,26 @@ static int host1x_drm_probe(struct host1x_device *dev)
drm_mode_config_reset(drm);
- err = drm_aperture_remove_framebuffers(_drm_driver);
- if (err < 0)
- goto hub;
+ /*
+* Only take over from a potential firmware framebuffer if any CRTCs
+* have been registered. This must not be a fatal error because there
+* are other accelerators that are exposed via this driver.
+*
+* Another case where this happens is on Tegra234 where the display
+* hardware is no longer part of the host1x complex, so this driver
+* will not expose any modesetting features.
+*/
+ if (drm->mode_config.num_crtc > 0) {
+ err = drm_aperture_remove_framebuffers(_drm_driver);
+ if (err < 0)
+ goto hub;
+ } else {
+ /*
+* Indicate to userspace that this doesn't expose any display
+* capabilities.
+*/
+ drm->driver_features &= ~(DRIVER_MODESET | DRIVER_ATOMIC);
+ }
err = drm_dev_register(drm, 0);
if (err < 0)
--
2.43.2
Re: [PATCH] gpu: host1x: Skip reset assert on Tegra186
On Thu Feb 22, 2024 at 2:05 AM CET, Mikko Perttunen wrote: > From: Mikko Perttunen > > On Tegra186, secure world applications may need to access host1x > during suspend/resume, and rely on the kernel to keep Host1x out > of reset during the suspend cycle. As such, as a quirk, > skip asserting Host1x's reset on Tegra186. > > We don't need to keep the clocks enabled, as BPMP ensures the clock > stays on while Host1x is being used. On newer SoC's, the reset line > is inaccessible, so there is no need for the quirk. > > Signed-off-by: Mikko Perttunen > --- > drivers/gpu/host1x/dev.c | 15 +-- > drivers/gpu/host1x/dev.h | 6 ++ > 2 files changed, 15 insertions(+), 6 deletions(-) Applied to drm-misc-fixes, though I added the Fixes: tag that Jon mentioned in reply to v1 of this as well as his Reviewed-by and Tested-by as well, since this is pretty much the same patch except for the comments. Thanks, Thierry signature.asc Description: PGP signature
Re: [PATCH] gpu: host1x: Skip reset assert on Tegra186
On Mon Feb 19, 2024 at 3:18 AM CET, Mikko Perttunen wrote: > On 2/16/24 19:02, Thierry Reding wrote: > > On Wed Feb 14, 2024 at 12:40 PM CET, Mikko Perttunen wrote: > >> From: Mikko Perttunen > >> > >> On Tegra186, other software components may rely on the kernel to > >> keep Host1x operational even during suspend. As such, as a quirk, > >> skip asserting Host1x's reset on Tegra186. > > > > This all sounds a bit vague. What other software components rely on the > > kernel to keep host1x operational during suspend? And why do they do so? > > Why is this not a problem elsewhere? > > My assumption is that it's due to a secure world application accessing > NVDEC or display engines during suspend or resume. This happening > without kernel knowledge is a bad thing, but it's hard to change at this > point. > > The reset line (CAR vs BPMP vs non-accessible reset line), and the > secure application code programming this stuff is slightly different in > every chip generation, which is where I think the differences happen. *sigh* I guess it is what it is. Please add a bit more background information to the commit message and also a comment for the skip_reset field so that people (including myself) will remember down the road why this exists. Thierry signature.asc Description: PGP signature
Re: [PATCH] phy: constify of_phandle_args in xlate
On Sat Feb 17, 2024 at 10:39 AM CET, Krzysztof Kozlowski wrote: > The xlate callbacks are supposed to translate of_phandle_args to proper > provider without modifying the of_phandle_args. Make the argument > pointer to const for code safety and readability. > > Signed-off-by: Krzysztof Kozlowski > --- > drivers/phy/allwinner/phy-sun4i-usb.c | 2 +- > drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c | 2 +- > drivers/phy/broadcom/phy-bcm-sr-pcie.c | 2 +- > drivers/phy/broadcom/phy-bcm-sr-usb.c | 2 +- > drivers/phy/broadcom/phy-bcm63xx-usbh.c| 2 +- > drivers/phy/broadcom/phy-brcm-usb.c| 2 +- > drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c| 2 +- > drivers/phy/freescale/phy-fsl-lynx-28g.c | 2 +- > drivers/phy/hisilicon/phy-histb-combphy.c | 2 +- > drivers/phy/intel/phy-intel-lgm-combo.c| 2 +- > drivers/phy/lantiq/phy-lantiq-vrx200-pcie.c| 2 +- > drivers/phy/marvell/phy-armada375-usb2.c | 2 +- > drivers/phy/marvell/phy-armada38x-comphy.c | 2 +- > drivers/phy/marvell/phy-berlin-sata.c | 2 +- > drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 2 +- > drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 2 +- > drivers/phy/mediatek/phy-mtk-mipi-csi-0-5.c| 2 +- > drivers/phy/mediatek/phy-mtk-tphy.c| 2 +- > drivers/phy/mediatek/phy-mtk-xsphy.c | 2 +- > drivers/phy/microchip/lan966x_serdes.c | 2 +- > drivers/phy/microchip/sparx5_serdes.c | 2 +- > drivers/phy/mscc/phy-ocelot-serdes.c | 2 +- > drivers/phy/phy-core.c | 8 > drivers/phy/phy-xgene.c| 2 +- > drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 2 +- > drivers/phy/ralink/phy-mt7621-pci.c| 2 +- > drivers/phy/renesas/phy-rcar-gen2.c| 2 +- > drivers/phy/renesas/phy-rcar-gen3-usb2.c | 2 +- > drivers/phy/renesas/r8a779f0-ether-serdes.c| 2 +- > drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 2 +- > drivers/phy/rockchip/phy-rockchip-pcie.c | 2 +- > drivers/phy/samsung/phy-exynos-mipi-video.c| 2 +- > drivers/phy/samsung/phy-exynos5-usbdrd.c | 2 +- > drivers/phy/samsung/phy-samsung-usb2.c | 2 +- > drivers/phy/socionext/phy-uniphier-usb2.c | 2 +- > drivers/phy/st/phy-miphy28lp.c | 2 +- > drivers/phy/st/phy-spear1310-miphy.c | 2 +- > drivers/phy/st/phy-spear1340-miphy.c | 2 +- > drivers/phy/st/phy-stm32-usbphyc.c | 2 +- > drivers/phy/tegra/xusb.c | 2 +- > drivers/phy/ti/phy-am654-serdes.c | 2 +- > drivers/phy/ti/phy-da8xx-usb.c | 2 +- > drivers/phy/ti/phy-gmii-sel.c | 2 +- > drivers/phy/xilinx/phy-zynqmp.c| 2 +- > drivers/pinctrl/tegra/pinctrl-tegra-xusb.c | 2 +- > include/linux/phy/phy.h| 14 +++--- > 46 files changed, 55 insertions(+), 55 deletions(-) Makes sense: Acked-by: Thierry Reding signature.asc Description: PGP signature
Re: [PATCH][next] gpu: host1x: remove redundant assignment to variable space
On Thu Feb 15, 2024 at 11:43 PM CET, Colin Ian King wrote: > The variable space is being initialized with a value that is never read, > it is being re-assigned later on. The initialization is redundant and > can be removed. Also merge two declaration lines together. > > Cleans up clang scan build warning: > drivers/gpu/host1x/cdma.c:628:15: warning: Value stored to 'space' > during its initialization is never read [deadcode.DeadStores] > > Signed-off-by: Colin Ian King > --- > drivers/gpu/host1x/cdma.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) Applied to drm-misc-next, thanks. Thierry signature.asc Description: PGP signature
Re: [PATCH] gpu: host1x: Skip reset assert on Tegra186
On Wed Feb 14, 2024 at 12:40 PM CET, Mikko Perttunen wrote: > From: Mikko Perttunen > > On Tegra186, other software components may rely on the kernel to > keep Host1x operational even during suspend. As such, as a quirk, > skip asserting Host1x's reset on Tegra186. This all sounds a bit vague. What other software components rely on the kernel to keep host1x operational during suspend? And why do they do so? Why is this not a problem elsewhere? Thierry signature.asc Description: PGP signature
Re: [PATCH] gpu: host1x: bus: make host1x_bus_type const
On Tue Feb 13, 2024 at 3:44 PM CET, Ricardo B. Marliere wrote:
> Since commit d492cc2573a0 ("driver core: device.h: make struct
> bus_type a const *"), the driver core can properly handle constant
> struct bus_type, move the host1x_bus_type variable to be a constant
> structure as well, placing it into read-only memory which can not be
> modified at runtime.
>
> Cc: Greg Kroah-Hartman
> Suggested-by: Greg Kroah-Hartman
> Signed-off-by: Ricardo B. Marliere
> ---
> drivers/gpu/host1x/bus.c | 2 +-
> drivers/gpu/host1x/bus.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
Applied to drm-misc-next, thanks.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH 0/6] drm/tegra: Fix some error handling paths
On Sat, Sep 02, 2023 at 05:22:07PM +0200, Christophe JAILLET wrote: > Most of the patches are retated to tegra_output_probe() and missing > tegra_output_remove(). Others are things spotted while writting the serie. > > > Patches 1, 3, 4 are verbose, but some functions called in the probe can > return -EPROBE_DEFER, so it is nice to correctly release resources. > > Maybe moving the tegra_output_probe() call would minimize the changes, but I'm > always reluctant to move code, because of possible side-effects. > > > Christophe JAILLET (6): > drm/tegra: dsi: Fix some error handling paths in tegra_dsi_probe() > drm/tegra: dsi: Fix missing pm_runtime_disable() in the error handling > path of tegra_dsi_probe() > drm/tegra: dsi: Fix some error handling paths in tegra_hdmi_probe() > drm/tegra: rgb: Fix some error handling paths in tegra_dc_rgb_probe() > drm/tegra: rgb: Fix missing clk_put() in the error handling paths of > tegra_dc_rgb_probe() > drm/tegra: output: Fix missing i2c_put_adapter() in the error handling > paths of tegra_output_probe() > > drivers/gpu/drm/tegra/dsi.c| 55 ++ > drivers/gpu/drm/tegra/hdmi.c | 20 - > drivers/gpu/drm/tegra/output.c | 16 +++--- > drivers/gpu/drm/tegra/rgb.c| 18 +++ > 4 files changed, 74 insertions(+), 35 deletions(-) Sorry, this fell through the cracks. Applied now, thanks. Thierry signature.asc Description: PGP signature
Re: [PATCH] drm/tegra: dpaux: Fix PM disable depth imbalance in tegra_dpaux_probe
On Wed, Oct 04, 2023 at 10:10:55PM +0800, Zhang Shurong wrote:
> The pm_runtime_enable function increases the power disable depth,
> which means that we must perform a matching decrement on the error
> handling path to maintain balance within the given context.
> Additionally, we need to address the same issue for pm_runtime_get_sync.
> We fix this by invoking pm_runtime_disable and pm_runtime_put_sync
> when error returns.
>
> Fixes: 82b81b3ec1a7 ("drm/tegra: dpaux: Implement runtime PM")
> Signed-off-by: Zhang Shurong
> ---
> drivers/gpu/drm/tegra/dpaux.c | 14 ++
> 1 file changed, 10 insertions(+), 4 deletions(-)
Applied, thanks.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH v2] drm/tegra: include drm/drm_edid.h only where needed
On Wed, Dec 13, 2023 at 12:19:51PM +0200, Jani Nikula wrote: > Reduce the need for rebuilds when drm_edid.h is modified by including it > only where needed. > > v2: Fix build (kernel test robot ) > > Signed-off-by: Jani Nikula > --- > drivers/gpu/drm/tegra/drm.h| 2 +- > drivers/gpu/drm/tegra/output.c | 1 + > drivers/gpu/drm/tegra/sor.c| 1 + > 3 files changed, 3 insertions(+), 1 deletion(-) Applied, thanks. Thierry signature.asc Description: PGP signature
Re: [PATCH] drm/tegra: dsi: Add missing check for of_find_device_by_node
On Tue, Oct 24, 2023 at 08:07:38AM +, Chen Ni wrote:
> Add check for the return value of of_find_device_by_node() and return
> the error if it fails in order to avoid NULL pointer dereference.
>
> Fixes: e94236cde4d5 ("drm/tegra: dsi: Add ganged mode support")
> Signed-off-by: Chen Ni
> ---
> drivers/gpu/drm/tegra/dsi.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Applied, thanks.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH] fbdev/simplefb: change loglevel when the power domains cannot be parsed
On Tue, Dec 12, 2023 at 02:57:54PM -0500, Brian Masney wrote:
> When the power domains cannot be parsed, the message is incorrectly
> logged as an info message. Let's change this to an error since an error
> is returned.
>
> Fixes: 92a511a568e4 ("fbdev/simplefb: Add support for generic power-domains")
> Signed-off-by: Brian Masney
> ---
> drivers/video/fbdev/simplefb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Thierry Reding
signature.asc
Description: PGP signature
Re: [PATCH v4 000/115] pwm: Fix lifetime issues for pwm_chips
On Fri, Dec 08, 2023 at 07:50:33PM +0100, Uwe Kleine-König wrote: > Hello Thierry, > > On Fri, Dec 08, 2023 at 04:41:39PM +0100, Thierry Reding wrote: > > On Wed, Dec 06, 2023 at 12:43:14PM +0100, Uwe Kleine-König wrote: > > > This series is based on Thierry's for-next. > > > > > > It starts with some cleanups that were all sent out separately already: > > > > > > - "pwm: Reduce number of pointer dereferences in pwm_device_request()" > > > > > > https://lore.kernel.org/linux-pwm/20231130072105.966848-1-u.kleine-koe...@pengutronix.de > > > - "pwm: crc: Use consistent variable naming for driver data" > > > > > > https://lore.kernel.org/linux-pwm/20231130074133.969806-1-u.kleine-koe...@pengutronix.de > > > - Two leds/qcom-lpg patches > > > > > > https://lore.kernel.org/linux-leds/20231126095230.683204-1-u.kleine-koe...@pengutronix.de > > >Lee already claimed to have taken the series already, but it's not yet > > > in > > >next. > > > - "leds: qcom-lpg: Introduce a wrapper for getting driver data from a > > > pwm chip" > > > > > > https://lore.kernel.org/linux-leds/20231124215208.616551-3-u.kleine-koe...@pengutronix.de > > > > > > In the following patches I changed: > > > > > > - "pwm: cros-ec: Change prototype of helper to prepare further changes" + > > >This was simplified in response to feedback by Tzung-Bi Shih > > > - Make pwmchip_priv() static (and don't export it), let drivers use a new > > >pwmchip_get_drvdata() instead. > > > - For drm/ti-sn65dsi86.c and leds/qcom-lpg make use of > > >pwmchip_set_drvdata() which makes the conversion to > > >devm_pwmchip_alloc() much prettier. > > > - Some cleanups here and there > > > - Add review tags received in v3 > > >I kept all tags even though the pwmchip_alloc() patches changed > > >slightly. Most of the time that's only > > >s/pwmchip_priv/pwmchip_get_drvdata/ though. Still, if you object, > > >just tell me. (This affects Paul Cercueil on patch #68, Conor Dooley > > >on patch #76 and Greg for patch #109.) > > > > > > I kept the pwmchip_alloc() + pwmchip_register() approach despite Bart > > > not liking it. To balance that out I don't like Bart's alternative > > > approach. There are no technically relevant differences between the two > > > approaches and no benchmarks that show either of the two to be better > > > than the other. Conceptually the design ideas around pwmchip_alloc() + > > > pwmchip_register() are used in several other subsystems, so it's a > > > proven way to do things. > > > > [Trimming the recipients, keeping only Bart and the mailing lists.] > > > > I do think there are technically relevant differences. For one, the > > better we isolate the internal data structure, the easier this becomes > > to manage. I'm attaching a patch that I've prototyped which should > > basically get us to somewhere around patch 110 of this series but with > > something like 1/8th of the changes. It doesn't need every driver to > > change and (mostly) decouples driver code from the core code. > > You don't need to touch all drivers because you didn't change struct > pwm_chip::dev yet. (If you really want, you don't need to change that, > but then you have some duplication as chip->dev holds the same value as > priv->dev.parent in the end.) I don't think that's a problem. These are for two logically separate things, after all. Duplication can also sometimes be useful to simplify things. There are plently of cases where we use local variables for the same reason. > > Now, I know that you think this is all bad because it's not a single > > allocation, but I much prefer the end result because it's got the driver > > and internals much more cleanly separated. Going forward I think it > > would be easier to apply all the ref-counting on top of that because we > > only need to keep the PWM framework-internal data structure alive after > > a PWM chip has gone away. > > Nearly all drivers need driver private data. Isn't it a nice service by > the pwm core to make handling this private data easier for the lowlevel > drivers? That's only true if you think pwmchip_alloc() makes it easier. I happen to think that it doesn't. On the contrary, I think having drivers pass in a PWM chip descriptor that can be embedded in driver-private data is much easier. > > From 72ea79887d96850f9ccc832ce52b907ca276c940
Re: [PATCH v4 000/115] pwm: Fix lifetime issues for pwm_chips
On Wed, Dec 06, 2023 at 12:43:14PM +0100, Uwe Kleine-König wrote: > Hello, > > This series is based on Thierry's for-next. > > It starts with some cleanups that were all sent out separately already: > > - "pwm: Reduce number of pointer dereferences in pwm_device_request()" > > https://lore.kernel.org/linux-pwm/20231130072105.966848-1-u.kleine-koe...@pengutronix.de > - "pwm: crc: Use consistent variable naming for driver data" > > https://lore.kernel.org/linux-pwm/20231130074133.969806-1-u.kleine-koe...@pengutronix.de > - Two leds/qcom-lpg patches > > https://lore.kernel.org/linux-leds/20231126095230.683204-1-u.kleine-koe...@pengutronix.de >Lee already claimed to have taken the series already, but it's not yet in >next. > - "leds: qcom-lpg: Introduce a wrapper for getting driver data from a pwm > chip" > > https://lore.kernel.org/linux-leds/20231124215208.616551-3-u.kleine-koe...@pengutronix.de > > In the following patches I changed: > > - "pwm: cros-ec: Change prototype of helper to prepare further changes" + >This was simplified in response to feedback by Tzung-Bi Shih > - Make pwmchip_priv() static (and don't export it), let drivers use a new >pwmchip_get_drvdata() instead. > - For drm/ti-sn65dsi86.c and leds/qcom-lpg make use of >pwmchip_set_drvdata() which makes the conversion to >devm_pwmchip_alloc() much prettier. > - Some cleanups here and there > - Add review tags received in v3 >I kept all tags even though the pwmchip_alloc() patches changed >slightly. Most of the time that's only >s/pwmchip_priv/pwmchip_get_drvdata/ though. Still, if you object, >just tell me. (This affects Paul Cercueil on patch #68, Conor Dooley >on patch #76 and Greg for patch #109.) > > I kept the pwmchip_alloc() + pwmchip_register() approach despite Bart > not liking it. To balance that out I don't like Bart's alternative > approach. There are no technically relevant differences between the two > approaches and no benchmarks that show either of the two to be better > than the other. Conceptually the design ideas around pwmchip_alloc() + > pwmchip_register() are used in several other subsystems, so it's a > proven way to do things. [Trimming the recipients, keeping only Bart and the mailing lists.] I do think there are technically relevant differences. For one, the better we isolate the internal data structure, the easier this becomes to manage. I'm attaching a patch that I've prototyped which should basically get us to somewhere around patch 110 of this series but with something like 1/8th of the changes. It doesn't need every driver to change and (mostly) decouples driver code from the core code. Now, I know that you think this is all bad because it's not a single allocation, but I much prefer the end result because it's got the driver and internals much more cleanly separated. Going forward I think it would be easier to apply all the ref-counting on top of that because we only need to keep the PWM framework-internal data structure alive after a PWM chip has gone away. Thierry From 72ea79887d96850f9ccc832ce52b907ca276c940 Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Tue, 28 Nov 2023 15:42:39 +0100 Subject: [PATCH] pwm: Isolate internal data into a separate structure In order to prepare for proper reference counting of PWM chips and PWM devices, move the internal data from the public PWM chip to a private PWM chip structure. This ensures that the data that the subsystem core may need to reference later on can stick around beyond the lifetime of the driver-private data. Signed-off-by: Thierry Reding --- drivers/pwm/core.c| 185 +- drivers/pwm/internal.h| 38 +++ drivers/pwm/pwm-atmel-hlcdc.c | 8 +- drivers/pwm/pwm-fsl-ftm.c | 6 +- drivers/pwm/pwm-lpc18xx-sct.c | 4 +- drivers/pwm/pwm-lpss.c| 14 +-- drivers/pwm/pwm-pca9685.c | 6 +- drivers/pwm/pwm-samsung.c | 6 +- drivers/pwm/pwm-sifive.c | 4 +- drivers/pwm/pwm-stm32-lp.c| 6 +- drivers/pwm/pwm-stm32.c | 6 +- drivers/pwm/pwm-tiecap.c | 6 +- drivers/pwm/pwm-tiehrpwm.c| 6 +- drivers/pwm/sysfs.c | 48 - include/linux/pwm.h | 26 + 15 files changed, 228 insertions(+), 141 deletions(-) create mode 100644 drivers/pwm/internal.h diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index f60b715abe62..54d57dec6dce 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -24,17 +24,19 @@ #define CREATE_TRACE_POINTS #include +#include "internal.h" + static DEFINE_MUTEX(pwm_lookup_lock); static LIST_HEAD(pwm_lookup_list); -/* protects access to pwmchip_idr */ +/* protects access to pwm_chips */ static DEFINE_MUTEX(pwm_lock); -static
[PATCH] drm/nouveau: Fixup gk20a instobj hierarchy
From: Thierry Reding
Commit 12c9b05da918 ("drm/nouveau/imem: support allocations not
preserved across suspend") uses container_of() to cast from struct
nvkm_memory to struct nvkm_instobj, assuming that all instance objects
are derived from struct nvkm_instobj. For the gk20a family that's not
the case and they are derived from struct nvkm_memory instead. This
causes some subtle data corruption (nvkm_instobj.preserve ends up
mapping to gk20a_instobj.vaddr) that causes a NULL pointer dereference
in gk20a_instobj_acquire_iommu() (and possibly elsewhere) and also
prevents suspend/resume from working.
Fix this by making struct gk20a_instobj derive from struct nvkm_instobj
instead.
Fixes: 12c9b05da918 ("drm/nouveau/imem: support allocations not preserved
across suspend")
Reported-by: Jonathan Hunter
Signed-off-by: Thierry Reding
---
Note that this was probably subtly wrong before the above-mentioned
commit already, but I don't think we've seen any reports that would
indicate any actual failures related to this before. So I think it's
good enough to apply this fix for v6.7. The next closest thing would
be commit d8e83994aaf6 ("drm/nouveau/imem: improve management of
instance memory"), but that's 8 years old (Linux v4.3)...
---
.../drm/nouveau/nvkm/subdev/instmem/gk20a.c| 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c
index 1b811d6972a1..201022ae9214 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c
@@ -49,14 +49,14 @@
#include
struct gk20a_instobj {
- struct nvkm_memory memory;
+ struct nvkm_instobj base;
struct nvkm_mm_node *mn;
struct gk20a_instmem *imem;
/* CPU mapping */
u32 *vaddr;
};
-#define gk20a_instobj(p) container_of((p), struct gk20a_instobj, memory)
+#define gk20a_instobj(p) container_of((p), struct gk20a_instobj, base.memory)
/*
* Used for objects allocated using the DMA API
@@ -148,7 +148,7 @@ gk20a_instobj_iommu_recycle_vaddr(struct
gk20a_instobj_iommu *obj)
list_del(>vaddr_node);
vunmap(obj->base.vaddr);
obj->base.vaddr = NULL;
- imem->vaddr_use -= nvkm_memory_size(>base.memory);
+ imem->vaddr_use -= nvkm_memory_size(>base.base.memory);
nvkm_debug(>base.subdev, "vaddr used: %x/%x\n", imem->vaddr_use,
imem->vaddr_max);
}
@@ -283,7 +283,7 @@ gk20a_instobj_map(struct nvkm_memory *memory, u64 offset,
struct nvkm_vmm *vmm,
{
struct gk20a_instobj *node = gk20a_instobj(memory);
struct nvkm_vmm_map map = {
- .memory = >memory,
+ .memory = >base.memory,
.offset = offset,
.mem = node->mn,
};
@@ -391,8 +391,8 @@ gk20a_instobj_ctor_dma(struct gk20a_instmem *imem, u32
npages, u32 align,
return -ENOMEM;
*_node = >base;
- nvkm_memory_ctor(_instobj_func_dma, >base.memory);
- node->base.memory.ptrs = _instobj_ptrs;
+ nvkm_memory_ctor(_instobj_func_dma, >base.base.memory);
+ node->base.base.memory.ptrs = _instobj_ptrs;
node->base.vaddr = dma_alloc_attrs(dev, npages << PAGE_SHIFT,
>handle, GFP_KERNEL,
@@ -438,8 +438,8 @@ gk20a_instobj_ctor_iommu(struct gk20a_instmem *imem, u32
npages, u32 align,
*_node = >base;
node->dma_addrs = (void *)(node->pages + npages);
- nvkm_memory_ctor(_instobj_func_iommu, >base.memory);
- node->base.memory.ptrs = _instobj_ptrs;
+ nvkm_memory_ctor(_instobj_func_iommu, >base.base.memory);
+ node->base.base.memory.ptrs = _instobj_ptrs;
/* Allocate backing memory */
for (i = 0; i < npages; i++) {
@@ -533,7 +533,7 @@ gk20a_instobj_new(struct nvkm_instmem *base, u32 size, u32
align, bool zero,
else
ret = gk20a_instobj_ctor_dma(imem, size >> PAGE_SHIFT,
align, );
- *pmemory = node ? >memory : NULL;
+ *pmemory = node ? >base.memory : NULL;
if (ret)
return ret;
--
2.43.0
Re: (subset) [PATCH 00/17] dt-bindings: samsung: add specific compatibles for existing SoC
On Tue, Nov 28, 2023 at 09:58:41PM +0100, Uwe Kleine-König wrote: > On Tue, Nov 28, 2023 at 06:49:23PM +0100, Thierry Reding wrote: > > > > On Wed, 08 Nov 2023 11:43:26 +0100, Krzysztof Kozlowski wrote: > > > Merging > > > === > > > I propose to take entire patchset through my tree (Samsung SoC), because: > ^^^ > > > > 1. Next cycle two new SoCs will be coming (Google GS101 and > > > ExynosAutov920), so > > >they will touch the same lines in some of the DT bindings (not all, > > > though). > > >It is reasonable for me to take the bindings for the new SoCs, to have > > > clean > > >`make dtbs_check` on the new DTS. > > > 2. Having it together helps me to have clean `make dtbs_check` within my > > > tree > > >on the existing DTS. > > > 3. No drivers are affected by this change. > > > 4. I plan to do the same for Tesla FSD and Exynos ARM32 SoCs, thus expect > > >follow up patchsets. > > > > > > [...] > > > > Applied, thanks! > > > > [12/17] dt-bindings: pwm: samsung: add specific compatibles for existing SoC > > commit: 5d67b8f81b9d598599366214e3b2eb5f84003c9f > > You didn't honor (or even comment) Krzysztof's proposal to take the > whole patchset via his tree (marked above). Was there some off-list > agreement? I had read all that and then looking at patchwork saw that you had marked all other patches in the series as "handled-elsewhere" and only this one was left as "new", so I assumed that, well, everything else was handled elsewhere and I was supposed to pick this one up... I'll drop this one. Thierry signature.asc Description: PGP signature
Re: [PATCH 08/10] iommu/tegra: Use tegra_dev_iommu_get_stream_id() in the remaining places
On Wed, Nov 29, 2023 at 03:26:03PM -0400, Jason Gunthorpe wrote:
> On Wed, Nov 29, 2023 at 05:23:13PM +0100, Thierry Reding wrote:
> > > diff --git a/drivers/memory/tegra/tegra186.c
> > > b/drivers/memory/tegra/tegra186.c
> > > index 533f85a4b2bdb7..3e4fbe94dd666e 100644
> > > --- a/drivers/memory/tegra/tegra186.c
> > > +++ b/drivers/memory/tegra/tegra186.c
> > > @@ -111,21 +111,21 @@ static void tegra186_mc_client_sid_override(struct
> > > tegra_mc *mc,
> > > static int tegra186_mc_probe_device(struct tegra_mc *mc, struct device
> > > *dev)
> > > {
> > > #if IS_ENABLED(CONFIG_IOMMU_API)
> > > - struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> > > struct of_phandle_args args;
> > > unsigned int i, index = 0;
> > > + u32 sid;
> > >
> > > + WARN_ON(!tegra_dev_iommu_get_stream_id(dev, ));
> >
> > I know the code previously didn't check for any errors, but we may want
> > to do so now. If tegra_dev_iommu_get_stream_id() ever fails we may end
> > up writing some undefined value into the override register.
>
> My assumption was it never fails otherwise this probably already
> doesn't work?
I guess the point I was trying to make is that previously we would not
have written anything to the stream ID register and so ignoring the
error here might end up writing to a register that previously we would
not have written to. Looking at the current code more closely I see now
that the reason why we wouldn't have written to the register is because
we would've crashed before.
So I think this okay.
>
> > I'm also unsure if WARN_ON() is appropriate here. I vaguely recall that
> > ->probe_device() was called for all devices on the bus and not all of
> > them may have been associated with the IOMMU. Not all of them may in
> > fact access memory in the first place.
>
> So you are thinkin that of_parse_phandle_with_args() is a NOP
> sometimes so it will tolerate the failure?
>
> Seems like the best thing to do is just continue to ignore it then?
Yeah, exactly. It would've just skipped over everything, basically.
> > Perhaps I'm misremembering and the IOMMU core now takes care of only
> > calling this when fwspec is indeed valid?
>
> Can't advise, I have no idea what tegra_mc_ops is for :)
In a nutshell, it's a hook that allows us to configure the memory
controller when a device is attached to the IOMMU. The memory controller
contains a set of registers that specify which memory client uses which
stream ID by default. For some devices this can be overridden (which is
where tegra_dev_iommu_get_stream_id() comes into play in those drivers)
and for other devices we can't override, which is when the memory
controller defaults come into play.
Anyway, I took a closer look at this and ran some tests. Turns out that
tegra186_mc_probe_device() really only gets called for devices that have
their fwspec properly initialized anyway, so I don't think there's
anything special we need to do here.
Strictly from a static analysis point of view I suppose we could now
have a situation that sid is uninitialized when the call to
tegra_dev_iommu_get_stream_id() fails and so using it in the loop is not
correct, theoretically, but I think that's just not a case that we'll
ever hit in practice.
So either way is fine with me. I have a slight preference for just
returning 0 in case tegra_dev_iommu_get_stream_id() fails, because it's
simple to do and avoids any of these (theoretical) ambiguities. So
whichever way you decide:
Reviewed-by: Thierry Reding
signature.asc
Description: PGP signature
Re: [PATCH 08/10] iommu/tegra: Use tegra_dev_iommu_get_stream_id() in the remaining places
On Tue, Nov 28, 2023 at 08:48:04PM -0400, Jason Gunthorpe wrote:
> This API was defined to formalize the access to internal iommu details on
> some Tegra SOCs, but a few callers got missed. Add them.
>
> The helper already masks by 0x so remove this code from the callers.
>
> Suggested-by: Thierry Reding
> Signed-off-by: Jason Gunthorpe
> ---
> drivers/dma/tegra186-gpc-dma.c | 8 +++-
> drivers/gpu/drm/nouveau/nvkm/subdev/ltc/gp10b.c | 7 ++-
> drivers/memory/tegra/tegra186.c | 12 ++--
> 3 files changed, 11 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/dma/tegra186-gpc-dma.c b/drivers/dma/tegra186-gpc-dma.c
> index fa4d4142a68a21..88547a23825b18 100644
> --- a/drivers/dma/tegra186-gpc-dma.c
> +++ b/drivers/dma/tegra186-gpc-dma.c
> @@ -1348,8 +1348,8 @@ static int tegra_dma_program_sid(struct
> tegra_dma_channel *tdc, int stream_id)
> static int tegra_dma_probe(struct platform_device *pdev)
> {
> const struct tegra_dma_chip_data *cdata = NULL;
> - struct iommu_fwspec *iommu_spec;
> - unsigned int stream_id, i;
> + unsigned int i;
> + u32 stream_id;
> struct tegra_dma *tdma;
> int ret;
>
> @@ -1378,12 +1378,10 @@ static int tegra_dma_probe(struct platform_device
> *pdev)
>
> tdma->dma_dev.dev = >dev;
>
> - iommu_spec = dev_iommu_fwspec_get(>dev);
> - if (!iommu_spec) {
> + if (!tegra_dev_iommu_get_stream_id(>dev, _id)) {
> dev_err(>dev, "Missing iommu stream-id\n");
> return -EINVAL;
> }
> - stream_id = iommu_spec->ids[0] & 0x;
>
> ret = device_property_read_u32(>dev, "dma-channel-mask",
> >chan_mask);
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/ltc/gp10b.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/ltc/gp10b.c
> index e7e8fdf3adab7a..b40fd1dbb21617 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/ltc/gp10b.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/ltc/gp10b.c
> @@ -28,16 +28,13 @@ static void
> gp10b_ltc_init(struct nvkm_ltc *ltc)
> {
> struct nvkm_device *device = ltc->subdev.device;
> - struct iommu_fwspec *spec;
> + u32 sid;
>
> nvkm_wr32(device, 0x17e27c, ltc->ltc_nr);
> nvkm_wr32(device, 0x17e000, ltc->ltc_nr);
> nvkm_wr32(device, 0x100800, ltc->ltc_nr);
>
> - spec = dev_iommu_fwspec_get(device->dev);
> - if (spec) {
> - u32 sid = spec->ids[0] & 0x;
> -
> + if (tegra_dev_iommu_get_stream_id(device->dev, )) {
> /* stream ID */
> nvkm_wr32(device, 0x16, sid << 2);
We could probably also remove the comment now since the function and
variable names make it obvious what's being written here.
> }
> diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
> index 533f85a4b2bdb7..3e4fbe94dd666e 100644
> --- a/drivers/memory/tegra/tegra186.c
> +++ b/drivers/memory/tegra/tegra186.c
> @@ -111,21 +111,21 @@ static void tegra186_mc_client_sid_override(struct
> tegra_mc *mc,
> static int tegra186_mc_probe_device(struct tegra_mc *mc, struct device *dev)
> {
> #if IS_ENABLED(CONFIG_IOMMU_API)
> - struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> struct of_phandle_args args;
> unsigned int i, index = 0;
> + u32 sid;
>
> + WARN_ON(!tegra_dev_iommu_get_stream_id(dev, ));
I know the code previously didn't check for any errors, but we may want
to do so now. If tegra_dev_iommu_get_stream_id() ever fails we may end
up writing some undefined value into the override register.
I'm also unsure if WARN_ON() is appropriate here. I vaguely recall that
->probe_device() was called for all devices on the bus and not all of
them may have been associated with the IOMMU. Not all of them may in
fact access memory in the first place.
Perhaps I'm misremembering and the IOMMU core now takes care of only
calling this when fwspec is indeed valid?
Thierry
signature.asc
Description: PGP signature
Re: (subset) [PATCH 00/17] dt-bindings: samsung: add specific compatibles for existing SoC
On Wed, 08 Nov 2023 11:43:26 +0100, Krzysztof Kozlowski wrote: > Merging > === > I propose to take entire patchset through my tree (Samsung SoC), because: > 1. Next cycle two new SoCs will be coming (Google GS101 and ExynosAutov920), > so >they will touch the same lines in some of the DT bindings (not all, > though). >It is reasonable for me to take the bindings for the new SoCs, to have > clean >`make dtbs_check` on the new DTS. > 2. Having it together helps me to have clean `make dtbs_check` within my tree >on the existing DTS. > 3. No drivers are affected by this change. > 4. I plan to do the same for Tesla FSD and Exynos ARM32 SoCs, thus expect >follow up patchsets. > > [...] Applied, thanks! [12/17] dt-bindings: pwm: samsung: add specific compatibles for existing SoC commit: 5d67b8f81b9d598599366214e3b2eb5f84003c9f Best regards, -- Thierry Reding
Re: [PATCH v5 1/4] pwm: rename pwm_apply_state() to pwm_apply_cansleep()
On Sat, Nov 18, 2023 at 04:16:17PM +, Sean Young wrote: > In order to introduce a pwm api which can be used from atomic context, > we will need two functions for applying pwm changes: > > int pwm_apply_cansleep(struct pwm *, struct pwm_state *); > int pwm_apply_atomic(struct pwm *, struct pwm_state *); > > This commit just deals with renaming pwm_apply_state(), a following > commit will introduce the pwm_apply_atomic() function. Sorry, I still don't agree with that _cansleep suffix. I think it's the wrong terminology. Just because something can sleep doesn't mean that it ever will. "Might sleep" is much more accurate because it says exactly what might happen and indicates what we're guarding against. Thierry signature.asc Description: PGP signature
[PATCH v2 2/2] fbdev/simplefb: Add support for generic power-domains
From: Thierry Reding
The simple-framebuffer device tree bindings document the power-domains
property, so make sure that simplefb supports it. This ensures that the
power domains remain enabled as long as simplefb is active.
v2: - remove unnecessary call to simplefb_detach_genpds() since that's
already done automatically by devres
- fix crash if power-domains property is missing in DT
Signed-off-by: Thierry Reding
---
drivers/video/fbdev/simplefb.c | 93 ++
1 file changed, 93 insertions(+)
diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
index 18025f34fde7..fe682af63827 100644
--- a/drivers/video/fbdev/simplefb.c
+++ b/drivers/video/fbdev/simplefb.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include
static const struct fb_fix_screeninfo simplefb_fix = {
@@ -78,6 +79,11 @@ struct simplefb_par {
unsigned int clk_count;
struct clk **clks;
#endif
+#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
+ unsigned int num_genpds;
+ struct device **genpds;
+ struct device_link **genpd_links;
+#endif
#if defined CONFIG_OF && defined CONFIG_REGULATOR
bool regulators_enabled;
u32 regulator_count;
@@ -432,6 +438,89 @@ static void simplefb_regulators_enable(struct simplefb_par
*par,
static void simplefb_regulators_destroy(struct simplefb_par *par) { }
#endif
+#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
+static void simplefb_detach_genpds(void *res)
+{
+ struct simplefb_par *par = res;
+ unsigned int i = par->num_genpds;
+
+ if (par->num_genpds <= 1)
+ return;
+
+ while (i--) {
+ if (par->genpd_links[i])
+ device_link_del(par->genpd_links[i]);
+
+ if (!IS_ERR_OR_NULL(par->genpds[i]))
+ dev_pm_domain_detach(par->genpds[i], true);
+ }
+}
+
+static int simplefb_attach_genpds(struct simplefb_par *par,
+ struct platform_device *pdev)
+{
+ struct device *dev = >dev;
+ unsigned int i;
+ int err;
+
+ err = of_count_phandle_with_args(dev->of_node, "power-domains",
+"#power-domain-cells");
+ if (err < 0) {
+ dev_info(dev, "failed to parse power-domains: %d\n", err);
+ return err;
+ }
+
+ par->num_genpds = err;
+
+ /*
+* Single power-domain devices are handled by the driver core, so
+* nothing to do here.
+*/
+ if (par->num_genpds <= 1)
+ return 0;
+
+ par->genpds = devm_kcalloc(dev, par->num_genpds, sizeof(*par->genpds),
+ GFP_KERNEL);
+ if (!par->genpds)
+ return -ENOMEM;
+
+ par->genpd_links = devm_kcalloc(dev, par->num_genpds,
+ sizeof(*par->genpd_links),
+ GFP_KERNEL);
+ if (!par->genpd_links)
+ return -ENOMEM;
+
+ for (i = 0; i < par->num_genpds; i++) {
+ par->genpds[i] = dev_pm_domain_attach_by_id(dev, i);
+ if (IS_ERR(par->genpds[i])) {
+ err = PTR_ERR(par->genpds[i]);
+ if (err == -EPROBE_DEFER) {
+ simplefb_detach_genpds(par);
+ return err;
+ }
+
+ dev_warn(dev, "failed to attach domain %u: %d\n", i,
err);
+ continue;
+ }
+
+ par->genpd_links[i] = device_link_add(dev, par->genpds[i],
+ DL_FLAG_STATELESS |
+ DL_FLAG_PM_RUNTIME |
+ DL_FLAG_RPM_ACTIVE);
+ if (!par->genpd_links[i])
+ dev_warn(dev, "failed to link power-domain %u\n", i);
+ }
+
+ return devm_add_action_or_reset(dev, simplefb_detach_genpds, par);
+}
+#else
+static int simplefb_attach_genpds(struct simplefb_par *par,
+ struct platform_device *pdev)
+{
+ return 0;
+}
+#endif
+
static int simplefb_probe(struct platform_device *pdev)
{
int ret;
@@ -518,6 +607,10 @@ static int simplefb_probe(struct platform_device *pdev)
if (ret < 0)
goto error_clocks;
+ ret = simplefb_attach_genpds(par, pdev);
+ if (ret < 0)
+ goto error_regulators;
+
simplefb_clocks_enable(par, pdev);
simplefb_regulators_enable(par, pdev);
--
2.42.0
[PATCH v2 1/2] fbdev/simplefb: Support memory-region property
From: Thierry Reding
The simple-framebuffer bindings specify that the "memory-region"
property can be used as an alternative to the "reg" property to define
the framebuffer memory used by the display hardware. Implement support
for this in the simplefb driver.
Reviewed-by: Hans de Goede
Signed-off-by: Thierry Reding
---
drivers/video/fbdev/simplefb.c | 35 +-
1 file changed, 30 insertions(+), 5 deletions(-)
diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
index 62f99f6fccd3..18025f34fde7 100644
--- a/drivers/video/fbdev/simplefb.c
+++ b/drivers/video/fbdev/simplefb.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -121,12 +122,13 @@ struct simplefb_params {
u32 height;
u32 stride;
struct simplefb_format *format;
+ struct resource memory;
};
static int simplefb_parse_dt(struct platform_device *pdev,
struct simplefb_params *params)
{
- struct device_node *np = pdev->dev.of_node;
+ struct device_node *np = pdev->dev.of_node, *mem;
int ret;
const char *format;
int i;
@@ -166,6 +168,23 @@ static int simplefb_parse_dt(struct platform_device *pdev,
return -EINVAL;
}
+ mem = of_parse_phandle(np, "memory-region", 0);
+ if (mem) {
+ ret = of_address_to_resource(mem, 0, >memory);
+ if (ret < 0) {
+ dev_err(>dev, "failed to parse memory-region\n");
+ of_node_put(mem);
+ return ret;
+ }
+
+ if (of_property_present(np, "reg"))
+ dev_warn(>dev, "preferring \"memory-region\" over
\"reg\" property\n");
+
+ of_node_put(mem);
+ } else {
+ memset(>memory, 0, sizeof(params->memory));
+ }
+
return 0;
}
@@ -193,6 +212,8 @@ static int simplefb_parse_pd(struct platform_device *pdev,
return -EINVAL;
}
+ memset(>memory, 0, sizeof(params->memory));
+
return 0;
}
@@ -431,10 +452,14 @@ static int simplefb_probe(struct platform_device *pdev)
if (ret)
return ret;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- dev_err(>dev, "No memory resource\n");
- return -EINVAL;
+ if (params.memory.start == 0 && params.memory.end == 0) {
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(>dev, "No memory resource\n");
+ return -EINVAL;
+ }
+ } else {
+ res =
}
mem = request_mem_region(res->start, resource_size(res), "simplefb");
--
2.42.0
[PATCH v2 0/2] fbdev/simplefb: Add missing simple-framebuffer features
From: Thierry Reding Hi, This contains two patches that bring simplefb up to feature parity with simpledrm. The patches add support for the "memory-region" property that provides an alternative to the "reg" property to describe the memory used for the framebuffer and allow attaching the simple-framebuffer device to one or more generic power domains to make sure they aren't turned off during the boot process and take down the display configuration. Changes in v2: - remove unnecessary call to simplefb_detach_genpds() since that's already done automatically by devres - fix crash if power-domains property is missing in DT Thanks, Thierry Thierry Reding (2): fbdev/simplefb: Support memory-region property fbdev/simplefb: Add support for generic power-domains drivers/video/fbdev/simplefb.c | 128 +++-- 1 file changed, 123 insertions(+), 5 deletions(-) -- 2.42.0
Re: [PATCH 2/2] fbdev/simplefb: Add support for generic power-domains
On Thu, Oct 26, 2023 at 02:50:27PM +0200, Hans de Goede wrote:
> Hi,
>
> Thank you for your patches.
>
> On 10/11/23 16:38, Thierry Reding wrote:
> > From: Thierry Reding
> >
> > The simple-framebuffer device tree bindings document the power-domains
> > property, so make sure that simplefb supports it. This ensures that the
> > power domains remain enabled as long as simplefb is active.
> >
> > Signed-off-by: Thierry Reding
> > ---
> > drivers/video/fbdev/simplefb.c | 93 +-
> > 1 file changed, 91 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
> > index 18025f34fde7..e69fb0ad2d54 100644
> > --- a/drivers/video/fbdev/simplefb.c
> > +++ b/drivers/video/fbdev/simplefb.c
> > @@ -25,6 +25,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> >
> > static const struct fb_fix_screeninfo simplefb_fix = {
> > @@ -78,6 +79,11 @@ struct simplefb_par {
> > unsigned int clk_count;
> > struct clk **clks;
> > #endif
> > +#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
> > + unsigned int num_genpds;
> > + struct device **genpds;
> > + struct device_link **genpd_links;
> > +#endif
> > #if defined CONFIG_OF && defined CONFIG_REGULATOR
> > bool regulators_enabled;
> > u32 regulator_count;
> > @@ -432,6 +438,83 @@ static void simplefb_regulators_enable(struct
> > simplefb_par *par,
> > static void simplefb_regulators_destroy(struct simplefb_par *par) { }
> > #endif
> >
> > +#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
> > +static void simplefb_detach_genpds(void *res)
> > +{
> > + struct simplefb_par *par = res;
> > + unsigned int i = par->num_genpds;
> > +
> > + if (par->num_genpds <= 1)
> > + return;
> > +
> > + while (i--) {
> > + if (par->genpd_links[i])
> > + device_link_del(par->genpd_links[i]);
> > +
> > + if (!IS_ERR_OR_NULL(par->genpds[i]))
> > + dev_pm_domain_detach(par->genpds[i], true);
> > + }
>
> Using this i-- construct means that genpd at index 0 will
> not be cleaned up.
This is actually a common variant to clean up in reverse order. You'll
find this used a lot in core code and so on. It has the advantage that
you can use it to unwind (not the case here) because i will already be
set to the right value, typically. It's also nice because it works for
unsigned integers.
Note that this uses the postfix decrement, so evaluation happens before
the decrement and therefore the last iteration of the loop will run with
i == 0. For unsigned integers this also means that after the loop the
variable will actually have wrapped around, but that's usually not a
problem since it isn't used after this point anymore.
> > static int simplefb_probe(struct platform_device *pdev)
> > {
> > int ret;
> > @@ -518,6 +601,10 @@ static int simplefb_probe(struct platform_device *pdev)
> > if (ret < 0)
> > goto error_clocks;
> >
> > + ret = simplefb_attach_genpd(par, pdev);
> > + if (ret < 0)
> > + goto error_regulators;
> > +
> > simplefb_clocks_enable(par, pdev);
> > simplefb_regulators_enable(par, pdev);
> >
> > @@ -534,18 +621,20 @@ static int simplefb_probe(struct platform_device
> > *pdev)
> > ret = devm_aperture_acquire_for_platform_device(pdev, par->base,
> > par->size);
> > if (ret) {
> > dev_err(>dev, "Unable to acquire aperture: %d\n", ret);
> > - goto error_regulators;
> > + goto error_genpds;
>
> This is not necessary since simplefb_attach_genpd() ends with:
>
> devm_add_action_or_reset(dev, simplefb_detach_genpds, par)
>
> Which causes simplefb_detach_genpds() to run when probe() fails.
Yes, you're right. I've removed all these explicit cleanup paths since
they are not needed.
>
> > }
> > ret = register_framebuffer(info);
> > if (ret < 0) {
> > dev_err(>dev, "Unable to register simplefb: %d\n", ret);
> > - goto error_regulators;
> > + goto error_genpds;
> > }
> >
> > dev_info(>dev, "fb%d: simplefb registered!\n", info->node);
> >
> > return 0;
> >
> > +error_genpds:
> > + simplefb_detach_genpds(par);
>
> As the kernel test robot (LKP) already pointed out this is causing
> compile errors when #if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
> evaluates as false.
>
> Since there is no simplefb_detach_genpds() stub in the #else, but as
> mentioned above this is not necessary so just remove it.
Yep, done.
Thanks,
Thierry
signature.asc
Description: PGP signature
Re: [PATCH 1/2] gpu: host1x: Correct allocated size for contexts
On Fri, Sep 01, 2023 at 02:59:09PM +0300, Mikko Perttunen wrote:
> From: Johnny Liu
>
> Original implementation over allocates the memory size for the
> contexts list. The size of memory for the contexts list is based
> on the number of iommu groups specified in the device tree.
>
> Fixes: 8aa5bcb61612 ("gpu: host1x: Add context device management code")
> Signed-off-by: Johnny Liu
> Signed-off-by: Mikko Perttunen
> ---
> drivers/gpu/host1x/context.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Both patches applied, thanks.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH] gpu: host1x: Syncpoint interrupt sharding
On Fri, Sep 01, 2023 at 02:40:07PM +0300, Mikko Perttunen wrote: > From: Mikko Perttunen > > Support sharded syncpoint interrupts on Tegra234+. This feature > allows specifying one of eight interrupt lines for each syncpoint > to lower processing latency of syncpoint threshold > interrupts. > > Signed-off-by: Mikko Perttunen > --- > drivers/gpu/host1x/dev.c| 28 +--- > drivers/gpu/host1x/dev.h| 3 ++- > drivers/gpu/host1x/hw/intr_hw.c | 46 - > 3 files changed, 60 insertions(+), 17 deletions(-) Applied, thanks. Thierry signature.asc Description: PGP signature
Re: [PATCH 1/3] gpu: host1x: Add locking in channel allocation
On Fri, Sep 01, 2023 at 02:15:07PM +0300, Mikko Perttunen wrote: > From: Mikko Perttunen > > Add locking around channel allocation to avoid race conditions. > > Signed-off-by: Mikko Perttunen > --- > drivers/gpu/host1x/channel.c | 7 +++ > drivers/gpu/host1x/channel.h | 3 +++ > 2 files changed, 10 insertions(+) All three patches applied, thanks. Thierry signature.asc Description: PGP signature
[PATCH 1/2] fbdev/simplefb: Support memory-region property
From: Thierry Reding
The simple-framebuffer bindings specify that the "memory-region"
property can be used as an alternative to the "reg" property to define
the framebuffer memory used by the display hardware. Implement support
for this in the simplefb driver.
Signed-off-by: Thierry Reding
---
drivers/video/fbdev/simplefb.c | 35 +-
1 file changed, 30 insertions(+), 5 deletions(-)
diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
index 62f99f6fccd3..18025f34fde7 100644
--- a/drivers/video/fbdev/simplefb.c
+++ b/drivers/video/fbdev/simplefb.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -121,12 +122,13 @@ struct simplefb_params {
u32 height;
u32 stride;
struct simplefb_format *format;
+ struct resource memory;
};
static int simplefb_parse_dt(struct platform_device *pdev,
struct simplefb_params *params)
{
- struct device_node *np = pdev->dev.of_node;
+ struct device_node *np = pdev->dev.of_node, *mem;
int ret;
const char *format;
int i;
@@ -166,6 +168,23 @@ static int simplefb_parse_dt(struct platform_device *pdev,
return -EINVAL;
}
+ mem = of_parse_phandle(np, "memory-region", 0);
+ if (mem) {
+ ret = of_address_to_resource(mem, 0, >memory);
+ if (ret < 0) {
+ dev_err(>dev, "failed to parse memory-region\n");
+ of_node_put(mem);
+ return ret;
+ }
+
+ if (of_property_present(np, "reg"))
+ dev_warn(>dev, "preferring \"memory-region\" over
\"reg\" property\n");
+
+ of_node_put(mem);
+ } else {
+ memset(>memory, 0, sizeof(params->memory));
+ }
+
return 0;
}
@@ -193,6 +212,8 @@ static int simplefb_parse_pd(struct platform_device *pdev,
return -EINVAL;
}
+ memset(>memory, 0, sizeof(params->memory));
+
return 0;
}
@@ -431,10 +452,14 @@ static int simplefb_probe(struct platform_device *pdev)
if (ret)
return ret;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- dev_err(>dev, "No memory resource\n");
- return -EINVAL;
+ if (params.memory.start == 0 && params.memory.end == 0) {
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(>dev, "No memory resource\n");
+ return -EINVAL;
+ }
+ } else {
+ res =
}
mem = request_mem_region(res->start, resource_size(res), "simplefb");
--
2.42.0
[PATCH 2/2] fbdev/simplefb: Add support for generic power-domains
From: Thierry Reding
The simple-framebuffer device tree bindings document the power-domains
property, so make sure that simplefb supports it. This ensures that the
power domains remain enabled as long as simplefb is active.
Signed-off-by: Thierry Reding
---
drivers/video/fbdev/simplefb.c | 93 +-
1 file changed, 91 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
index 18025f34fde7..e69fb0ad2d54 100644
--- a/drivers/video/fbdev/simplefb.c
+++ b/drivers/video/fbdev/simplefb.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include
static const struct fb_fix_screeninfo simplefb_fix = {
@@ -78,6 +79,11 @@ struct simplefb_par {
unsigned int clk_count;
struct clk **clks;
#endif
+#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
+ unsigned int num_genpds;
+ struct device **genpds;
+ struct device_link **genpd_links;
+#endif
#if defined CONFIG_OF && defined CONFIG_REGULATOR
bool regulators_enabled;
u32 regulator_count;
@@ -432,6 +438,83 @@ static void simplefb_regulators_enable(struct simplefb_par
*par,
static void simplefb_regulators_destroy(struct simplefb_par *par) { }
#endif
+#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
+static void simplefb_detach_genpds(void *res)
+{
+ struct simplefb_par *par = res;
+ unsigned int i = par->num_genpds;
+
+ if (par->num_genpds <= 1)
+ return;
+
+ while (i--) {
+ if (par->genpd_links[i])
+ device_link_del(par->genpd_links[i]);
+
+ if (!IS_ERR_OR_NULL(par->genpds[i]))
+ dev_pm_domain_detach(par->genpds[i], true);
+ }
+}
+
+static int simplefb_attach_genpd(struct simplefb_par *par,
+struct platform_device *pdev)
+{
+ struct device *dev = >dev;
+ unsigned int i;
+ int err;
+
+ par->num_genpds = of_count_phandle_with_args(dev->of_node,
+"power-domains",
+"#power-domain-cells");
+ /*
+* Single power-domain devices are handled by the driver core, so
+* nothing to do here.
+*/
+ if (par->num_genpds <= 1)
+ return 0;
+
+ par->genpds = devm_kcalloc(dev, par->num_genpds, sizeof(*par->genpds),
+ GFP_KERNEL);
+ if (!par->genpds)
+ return -ENOMEM;
+
+ par->genpd_links = devm_kcalloc(dev, par->num_genpds,
+ sizeof(*par->genpd_links),
+ GFP_KERNEL);
+ if (!par->genpd_links)
+ return -ENOMEM;
+
+ for (i = 0; i < par->num_genpds; i++) {
+ par->genpds[i] = dev_pm_domain_attach_by_id(dev, i);
+ if (IS_ERR(par->genpds[i])) {
+ err = PTR_ERR(par->genpds[i]);
+ if (err == -EPROBE_DEFER) {
+ simplefb_detach_genpds(par);
+ return err;
+ }
+
+ dev_warn(dev, "failed to attach domain %u: %d\n", i,
err);
+ continue;
+ }
+
+ par->genpd_links[i] = device_link_add(dev, par->genpds[i],
+ DL_FLAG_STATELESS |
+ DL_FLAG_PM_RUNTIME |
+ DL_FLAG_RPM_ACTIVE);
+ if (!par->genpd_links[i])
+ dev_warn(dev, "failed to link power-domain %u\n", i);
+ }
+
+ return devm_add_action_or_reset(dev, simplefb_detach_genpds, par);
+}
+#else
+static int simplefb_attach_genpd(struct simplefb_par *par,
+struct platform_device *pdev)
+{
+ return 0;
+}
+#endif
+
static int simplefb_probe(struct platform_device *pdev)
{
int ret;
@@ -518,6 +601,10 @@ static int simplefb_probe(struct platform_device *pdev)
if (ret < 0)
goto error_clocks;
+ ret = simplefb_attach_genpd(par, pdev);
+ if (ret < 0)
+ goto error_regulators;
+
simplefb_clocks_enable(par, pdev);
simplefb_regulators_enable(par, pdev);
@@ -534,18 +621,20 @@ static int simplefb_probe(struct platform_device *pdev)
ret = devm_aperture_acquire_for_platform_device(pdev, par->base,
par->size);
if (ret) {
dev_err(>dev, "Unable to acquire aperture: %d\n", ret);
- goto error_regulators;
+ goto error_genpds;
}
ret = register_f
[PATCH 0/2] fbdev/simplefb: Add missing simple-framebuffer features
From: Thierry Reding Hi, This contains two patches that bring simplefb up to feature parity with simpledrm. The patches add support for the "memory-region" property that provides an alternative to the "reg" property to describe the memory used for the framebuffer and allow attaching the simple-framebuffer device to one or more generic power domains to make sure they aren't turned off during the boot process and take down the display configuration. Thanks, Thierry Thierry Reding (2): fbdev/simplefb: Support memory-region property fbdev/simplefb: Add support for generic power-domains drivers/video/fbdev/simplefb.c | 128 +++-- 1 file changed, 121 insertions(+), 7 deletions(-) -- 2.42.0
[PATCH] drm/simpledrm: Fix power domain device link validity check
From: Thierry Reding
We need to check if a link is non-NULL before trying to delete it.
Signed-off-by: Thierry Reding
---
drivers/gpu/drm/tiny/simpledrm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 9c597461d1e2..8bdaf66044fc 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -506,7 +506,7 @@ static void simpledrm_device_detach_genpd(void *res)
return;
for (i = sdev->pwr_dom_count - 1; i >= 0; i--) {
- if (!sdev->pwr_dom_links[i])
+ if (sdev->pwr_dom_links[i])
device_link_del(sdev->pwr_dom_links[i]);
if (!IS_ERR_OR_NULL(sdev->pwr_dom_devs[i]))
dev_pm_domain_detach(sdev->pwr_dom_devs[i], true);
--
2.42.0
Re: (subset) [PATCH v3 0/5] Support bridge/connector by Tegra HDMI
From: Thierry Reding On Mon, 07 Aug 2023 17:35:10 +0300, Svyatoslav Ryhel wrote: > This patch adds support for the bridge/connector attached to the > HDMI output, allowing to model the hardware properly. It keeps > backwards compatibility with existing bindings and is required > by devices which have a simple or MHL bridge connected to HDMI > output like ASUS P1801-T or LG P880/P895 or HTC One X. > > Tested on ASUS Transformers which have no dedicated bridge but > have type d HDMI connector directly available. Tests went smoothly. > > [...] Applied, thanks! [1/5] ARM: dts: tegra: Drop unit-address from parallel RGB output port (no commit info) Best regards, -- Thierry Reding
Re: [PATCH -next] drm/tegra: Remove two unused function declarations
On Wed, Aug 09, 2023 at 11:02:26AM +0800, Yue Haibing wrote:
> Commit 776dc3840367 ("drm/tegra: Move subdevice infrastructure to host1x")
> removed the implementation but not the declaration.
>
> Signed-off-by: Yue Haibing
> ---
> drivers/gpu/drm/tegra/drm.h | 3 ---
> 1 file changed, 3 deletions(-)
Applied, thanks.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH 1/2] drm/tegra: Return an error code if fails
On Tue, Oct 10, 2023 at 03:22:56PM +0200, Thierry Reding wrote:
> On Mon, Jun 26, 2023 at 10:33:30PM +0800, Sui Jingfeng wrote:
> > Return -ENOMEM if tegra_bo_mmap() fails.
> >
> > Signed-off-by: Sui Jingfeng
> > ---
> > drivers/gpu/drm/tegra/gem.c | 2 ++
> > 1 file changed, 2 insertions(+)
>
> Sorry, this fell through the cracks. I think it'd be better if
> tegra_bo_mmap() were to be improved to always return either an ERR_PTR()
> encoded error code or a valid pointer. Throwing NULL into the mix isn't
> useful because it typically means something like -ENOMEM anyway. Error
> codes are more explicit, so since we're already using them for some
> cases, might as well return them for all.
>
> Actually, looks like tegra_bo_mmap() never actually returns an ERR_PTR()
> encoded error code. It's either obj->vaddr, the return value of vmap()
> (which is either NULL or the address of the mapping), or the address
> obtained from dma_buf_vmap_unlocked() (i.e. map.vaddr) or NULL on
> failure. So I think it would equally make sense to keep your patch and
> to remove the IS_ERR() check below it.
>
> I would slightly prefer the first option, but either is fine.
How about the attached patch?
Thierry
From b34a09efcf7b1d2c25d3baf8c6d91c5ca09b4e0f Mon Sep 17 00:00:00 2001
From: Thierry Reding
Date: Tue, 10 Oct 2023 17:26:14 +0200
Subject: [PATCH] drm/tegra: gem: Do not return NULL in tegra_bo_mmap()
It's confusing for a function to return NULL and ERR_PTR()-encoded error
codes on failure. Make sure we only ever return the latter since that's
what callers already expect.
Signed-off-by: Thierry Reding
---
drivers/gpu/drm/tegra/gem.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index 11296de59c5a..679460e05c05 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -178,6 +178,7 @@ static void *tegra_bo_mmap(struct host1x_bo *bo)
{
struct tegra_bo *obj = host1x_to_tegra_bo(bo);
struct iosys_map map;
+ void *vaddr;
int ret;
if (obj->vaddr)
@@ -185,10 +186,18 @@ static void *tegra_bo_mmap(struct host1x_bo *bo)
if (obj->gem.import_attach) {
ret = dma_buf_vmap_unlocked(obj->gem.import_attach->dmabuf,
);
- return ret ? NULL : map.vaddr;
+ if (ret < 0)
+ return ERR_PTR(ret);
+
+ return map.vaddr;
}
- return vmap(obj->pages, obj->num_pages, VM_MAP,
pgprot_writecombine(PAGE_KERNEL));
+ vaddr = vmap(obj->pages, obj->num_pages, VM_MAP,
+pgprot_writecombine(PAGE_KERNEL));
+ if (!vaddr)
+ return -ENOMEM;
+
+ return vaddr;
}
static void tegra_bo_munmap(struct host1x_bo *bo, void *addr)
--
2.42.0
signature.asc
Description: PGP signature
Re: [PATCH 2/2] drm/tegra: Remove surplus else after return
On Mon, Jun 26, 2023 at 10:33:31PM +0800, Sui Jingfeng wrote: > else is not generally useful after return > > Signed-off-by: Sui Jingfeng > --- > drivers/gpu/drm/tegra/gem.c | 19 ++- > 1 file changed, 10 insertions(+), 9 deletions(-) Applied, thanks. Thierry signature.asc Description: PGP signature
Re: [PATCH 1/2] drm/tegra: Return an error code if fails
On Mon, Jun 26, 2023 at 10:33:30PM +0800, Sui Jingfeng wrote: > Return -ENOMEM if tegra_bo_mmap() fails. > > Signed-off-by: Sui Jingfeng > --- > drivers/gpu/drm/tegra/gem.c | 2 ++ > 1 file changed, 2 insertions(+) Sorry, this fell through the cracks. I think it'd be better if tegra_bo_mmap() were to be improved to always return either an ERR_PTR() encoded error code or a valid pointer. Throwing NULL into the mix isn't useful because it typically means something like -ENOMEM anyway. Error codes are more explicit, so since we're already using them for some cases, might as well return them for all. Actually, looks like tegra_bo_mmap() never actually returns an ERR_PTR() encoded error code. It's either obj->vaddr, the return value of vmap() (which is either NULL or the address of the mapping), or the address obtained from dma_buf_vmap_unlocked() (i.e. map.vaddr) or NULL on failure. So I think it would equally make sense to keep your patch and to remove the IS_ERR() check below it. I would slightly prefer the first option, but either is fine. Thierry > diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c > index dea38892d6e6..0ce22935fbd3 100644 > --- a/drivers/gpu/drm/tegra/gem.c > +++ b/drivers/gpu/drm/tegra/gem.c > @@ -710,6 +710,8 @@ static int tegra_gem_prime_vmap(struct dma_buf *buf, > struct iosys_map *map) > void *vaddr; > > vaddr = tegra_bo_mmap(>base); > + if (!vaddr) > + return -ENOMEM; > if (IS_ERR(vaddr)) > return PTR_ERR(vaddr); > > -- > 2.25.1 > signature.asc Description: PGP signature
Re: [PATCH] drm/tegra: Remove existing framebuffer only if we support display
On Thu, Aug 31, 2023 at 10:04:29AM +0200, Daniel Vetter wrote:
> On Thu, 31 Aug 2023 at 08:33, Mikko Perttunen wrote:
> >
> > On 8/30/23 13:19, Thomas Zimmermann wrote:
> > > Hi
> > >
> > > Am 25.08.23 um 15:22 schrieb Thierry Reding:
> > >> From: Thierry Reding
> > >>
> > >> Tegra DRM doesn't support display on Tegra234 and later, so make sure
> > >> not to remove any existing framebuffers in that case.
> > >>
> > >> Signed-off-by: Thierry Reding
> > >> ---
> > >> drivers/gpu/drm/tegra/drm.c | 8 +---
> > >> 1 file changed, 5 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> > >> index b1e1a78e30c6..7a38dadbc264 100644
> > >> --- a/drivers/gpu/drm/tegra/drm.c
> > >> +++ b/drivers/gpu/drm/tegra/drm.c
> > >> @@ -1220,9 +1220,11 @@ static int host1x_drm_probe(struct
> > >> host1x_device *dev)
> > >> drm_mode_config_reset(drm);
> > >> -err = drm_aperture_remove_framebuffers(_drm_driver);
> > >> -if (err < 0)
> > >> -goto hub;
> > >> +if (drm->mode_config.num_crtc > 0) {
> > >
> > > If you don't support the hardware, wouldn't it be better to return
> > > -ENODEV if !num_crtc?
> >
> > While display is not supported through TegraDRM on Tegra234+, certain
> > multimedia accelerators are supported, so we need to finish probe for those.
>
> Ideally you also register the tegra driver without DRIVER_MODESET |
> DRIVER_ATOMIC in that case, to avoid unecessary userspace confusion.
> Most userspace can cope with a display driver without any crtc, but I
> think xorg-modesettting actually falls over. Or at least I've seen
> some hacks that the agx people added to make sure X doesn't
> accidentally open the wrong driver.
That's a good point. However I recall from earlier attempts at doing
something like this in Nouveau (although this is now very long ago) that
it's not very easy. The problem, as I recall, is that the driver is a
singleton, so we would essentially be supporting either modesetting or
not, for any device in the system.
Now, it's unlikely that we'd have a mix of one Tegra DRM driver with
display support and another without, but it's something that I recall
back at the time with Nouveau was problematic because you could have the
Tegra integrated graphics (without display support) and a PCI-connected
discrete GPU (with display support) within the same system.
I need to look into it a bit more to see if I can come up with something
good to account for this.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH] drm/tegra: Remove existing framebuffer only if we support display
On Wed, Aug 30, 2023 at 08:13:04AM +0200, Javier Martinez Canillas wrote:
> Thierry Reding writes:
>
> Hello Thierry,
>
> > From: Thierry Reding
> >
> > Tegra DRM doesn't support display on Tegra234 and later, so make sure
> > not to remove any existing framebuffers in that case.
> >
>
> I see, this makes sense to me.
>
> Acked-by: Javier Martinez Canillas
>
> A couple of comments below though:
>
> > Signed-off-by: Thierry Reding
> > ---
> > drivers/gpu/drm/tegra/drm.c | 8 +---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> > index b1e1a78e30c6..7a38dadbc264 100644
> > --- a/drivers/gpu/drm/tegra/drm.c
> > +++ b/drivers/gpu/drm/tegra/drm.c
> > @@ -1220,9 +1220,11 @@ static int host1x_drm_probe(struct host1x_device
> > *dev)
> >
> > drm_mode_config_reset(drm);
> >
> > - err = drm_aperture_remove_framebuffers(_drm_driver);
> > - if (err < 0)
> > - goto hub;
> > + if (drm->mode_config.num_crtc > 0) {
>
> Maybe you can add a comment here explaining why the check is needed?
Sure, will do.
> I also wonder if is worth to move the drm_num_crtcs() function from
> drivers/gpu/drm/drm_crtc.c to include/drm/drm_crtc.h and use that helper
> instead?
I've been looking at this, there's a few things that come to mind. It
seems like we have a couple of different ways to get the number of CRTCs
for a device. We have struct drm_device's num_crtcs, which is set during
drm_vblank_init(), then we have struct drm_mode_config's num_crtc, which
is incremented every time a new CRTC is added (and decremented when a
CRTC is removed), and finally we've got the drm_num_crtcs() which
"computes" the number of CRTCs registered by iterating over all CRTCs
that have been registered.
Are there any cases where these three can yield different values? Would
it not make sense to consolidate these into a single variable?
Thierry
signature.asc
Description: PGP signature
[PATCH] drm/tegra: Remove existing framebuffer only if we support display
From: Thierry Reding
Tegra DRM doesn't support display on Tegra234 and later, so make sure
not to remove any existing framebuffers in that case.
Signed-off-by: Thierry Reding
---
drivers/gpu/drm/tegra/drm.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index b1e1a78e30c6..7a38dadbc264 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -1220,9 +1220,11 @@ static int host1x_drm_probe(struct host1x_device *dev)
drm_mode_config_reset(drm);
- err = drm_aperture_remove_framebuffers(_drm_driver);
- if (err < 0)
- goto hub;
+ if (drm->mode_config.num_crtc > 0) {
+ err = drm_aperture_remove_framebuffers(_drm_driver);
+ if (err < 0)
+ goto hub;
+ }
err = drm_dev_register(drm, 0);
if (err < 0)
--
2.41.0
Re: [PATCH 15/20] drm/tegra/hub: Increase buffer size to ensure all possible values can be stored
On Thu, Aug 24, 2023 at 01:01:24PM +0100, Lee Jones wrote: > On Thu, 24 Aug 2023, Jani Nikula wrote: > > > On Thu, 24 Aug 2023, Thierry Reding wrote: > > > On Thu, Aug 24, 2023 at 08:37:00AM +0100, Lee Jones wrote: > > >> When converting from int to string, we must allow for up to 10-chars > > >> (2147483647). > > >> > > >> Fixes the following W=1 kernel build warning(s): > > >> > > >> drivers/gpu/drm/tegra/hub.c: In function ‘tegra_display_hub_probe’: > > >> drivers/gpu/drm/tegra/hub.c:1106:47: warning: ‘%u’ directive output may > > >> be truncated writing between 1 and 10 bytes into a region of size 4 > > >> [-Wformat-truncation=] > > >> drivers/gpu/drm/tegra/hub.c:1106:42: note: directive argument in the > > >> range [0, 4294967294] > > >> drivers/gpu/drm/tegra/hub.c:1106:17: note: ‘snprintf’ output between 6 > > >> and 15 bytes into a destination of size 8 > > > > > > I wish there was (perhaps there is?) a better way to annotate that i > > > will always be within a given range. In practice this is always going to > > > be smaller than 10 and even in future hardware I wouldn't expect this to > > > ever exceed anything like 32 or 64, so 8 characters is already generous. > > > > I assume you could do > > > > snprintf(id, sizeof(id), "wgrp%u", (unsigned char)i); > > > > but it's perhaps less obvious than just increasing the size of the > > buffer. > > I had the very same thought process. It's not a big deal, this happens all on the stack, so adding a couple of bytes isn't going to hurt very much. Still with all the tooling that we have it'd be nice if something could be added. Perhaps an alternative would be to reject values for num_wgrp that are bigger than 32. With that the compiler might have enough information not to warn about this. Anyway, no need to spend any more time on this, I'm fine with the patch as-is. Thierry signature.asc Description: PGP signature
Re: [PATCH 15/20] drm/tegra/hub: Increase buffer size to ensure all possible values can be stored
On Thu, Aug 24, 2023 at 08:37:00AM +0100, Lee Jones wrote:
> When converting from int to string, we must allow for up to 10-chars
> (2147483647).
>
> Fixes the following W=1 kernel build warning(s):
>
> drivers/gpu/drm/tegra/hub.c: In function ‘tegra_display_hub_probe’:
> drivers/gpu/drm/tegra/hub.c:1106:47: warning: ‘%u’ directive output may be
> truncated writing between 1 and 10 bytes into a region of size 4
> [-Wformat-truncation=]
> drivers/gpu/drm/tegra/hub.c:1106:42: note: directive argument in the range
> [0, 4294967294]
> drivers/gpu/drm/tegra/hub.c:1106:17: note: ‘snprintf’ output between 6 and
> 15 bytes into a destination of size 8
I wish there was (perhaps there is?) a better way to annotate that i
will always be within a given range. In practice this is always going to
be smaller than 10 and even in future hardware I wouldn't expect this to
ever exceed anything like 32 or 64, so 8 characters is already generous.
Thierry
>
> Signed-off-by: Lee Jones
> ---
> Cc: Thierry Reding
> Cc: Mikko Perttunen
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Jonathan Hunter
> Cc: Philipp Zabel
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-te...@vger.kernel.org
> ---
> drivers/gpu/drm/tegra/hub.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c
> index 1af5f8318d914..f21e57e8599ee 100644
> --- a/drivers/gpu/drm/tegra/hub.c
> +++ b/drivers/gpu/drm/tegra/hub.c
> @@ -1101,7 +1101,7 @@ static int tegra_display_hub_probe(struct
> platform_device *pdev)
>
> for (i = 0; i < hub->soc->num_wgrps; i++) {
> struct tegra_windowgroup *wgrp = >wgrps[i];
> - char id[8];
> + char id[16];
>
> snprintf(id, sizeof(id), "wgrp%u", i);
> mutex_init(>lock);
> --
> 2.42.0.rc1.204.g551eb34607-goog
>
signature.asc
Description: PGP signature
Re: [PATCH v3 0/5] Add JDI LPM102A188A display panel support
On Mon, Aug 07, 2023 at 02:33:00PM +0100, Diogo Ivo wrote: > Hello, > > These patches add support for the JDI LPM102A188A display panel, > found in the Google Pixel C. > > Patch 1 adds the DT bindings for the panel. > > Patch 2 adds the panel driver, which is based on the downstream > kernel driver published by Google and developed by Sean Paul. > > Patches 3-5 add DT nodes for the regulator, backlight controller and > display panel. > > The first version of this patch series can be found at: > https://lore.kernel.org/all/20220929170502.1034040-1-diogo@tecnico.ulisboa.pt/ > > The first submission of v2 can be found at: > https://lore.kernel.org/all/20221025153746.101278-1-diogo@tecnico.ulisboa.pt/ > > Changes in v2: > - Patch 1: remove touchscreen reset gpio property > - Patch 2: clear register based on its value rather than a DT property > - Patch 3: tune backlight delay values > - Patch 4: add generic node names, remove underscores > > Changes in v3: > - Patch 1: add Reviewed-by > - Patch 2: fix error handling, remove enabled/prepared booleans, add >dc/dc setting > - Patches 3-5: Split previous patch 3 into three different patches, >each adding a separate node > - removed previous patch 2 pertaining to Tegra DSI reset as it was upstreamed > > Diogo Ivo (5): > dt-bindings: display: Add bindings for JDI LPM102A188A > drm/panel: Add driver for JDI LPM102A188A > arm64: dts: smaug: Add DSI/CSI regulator > arm64: dts: smaug: Add backlight node > arm64: dts: smaug: Add display panel node I've picked up patches 3-5 into the Tegra tree and I assume the other two will go in through drm-misc? Thierry signature.asc Description: PGP signature
Re: (subset) [PATCH v3 0/5] Add JDI LPM102A188A display panel support
From: Thierry Reding On Mon, 07 Aug 2023 14:33:00 +0100, Diogo Ivo wrote: > These patches add support for the JDI LPM102A188A display panel, > found in the Google Pixel C. > > Patch 1 adds the DT bindings for the panel. > > Patch 2 adds the panel driver, which is based on the downstream > kernel driver published by Google and developed by Sean Paul. > > [...] Applied, thanks! [3/5] arm64: dts: smaug: Add DSI/CSI regulator (no commit info) [4/5] arm64: dts: smaug: Add backlight node (no commit info) [5/5] arm64: dts: smaug: Add display panel node (no commit info) Best regards, -- Thierry Reding
Re: [PATCH v2 0/2] Support bridge/connector by Tegra HDMI
On Thu, Jul 27, 2023 at 07:24:56PM +0300, Svyatoslav Ryhel wrote: > > > 27 липня 2023 р. 18:09:22 GMT+03:00, Thierry Reding > написав(-ла): > >On Sun, Jun 18, 2023 at 11:50:44AM +0300, Svyatoslav Ryhel wrote: > >> This patch adds support for the bridge/connector attached to the > >> HDMI output, allowing to model the hardware properly. It keeps > >> backwards compatibility with existing bindings and is required > >> by devices which have a simple or MHL bridge connected to HDMI > >> output like ASUS P1801-T or LG P880/P895 or HTC One X. > >> > >> Tested on ASUS Transformers which have no dedicated bridge but > >> have type d HDMI connector directly available. Tests went smoothly. > > > >If I understand correctly, we still need the drm/tegra patch to be > >applied before the DT change, otherwise the driver won't know what to do > >about the connector, right? > > > >That shouldn't be big problem, but it means that the patches need to be > >staged in correctly to avoid breaking things. > > Patchset contains drm/tegra patch I understand, but my point is that if we apply the DT patch before the driver patch, then the display won't be correctly initialized because the old driver code only looks within the HDMI node for the additional properties. Only after the drm/tegra patch is applied will the move in DT be recognized by the driver. So for now I've picked up the drm/tegra patch and then I'll apply the DT change later on. Thierry signature.asc Description: PGP signature
Re: [PATCH v2 2/2] ARM: tegra: transformers: add connector node
On Thu, Jul 27, 2023 at 07:26:28PM +0300, Svyatoslav Ryhel wrote:
>
>
> 27 липня 2023 р. 18:11:15 GMT+03:00, Thierry Reding
> написав(-ла):
> >On Sun, Jun 18, 2023 at 11:50:46AM +0300, Svyatoslav Ryhel wrote:
> >> All ASUS Transformers have micro-HDMI connector directly available.
> >> After Tegra HDMI got bridge/connector support, we should use connector
> >> framework for proper HW description.
> >>
> >> Tested-by: Andreas Westman Dorcsak # ASUS TF T30
> >> Tested-by: Robert Eckelmann # ASUS TF101 T20
> >> Tested-by: Svyatoslav Ryhel # ASUS TF201 T30
> >> Signed-off-by: Svyatoslav Ryhel
> >> ---
> >> arch/arm/boot/dts/tegra20-asus-tf101.dts | 22 ---
> >> .../dts/tegra30-asus-transformer-common.dtsi | 21 --
> >> 2 files changed, 38 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/arch/arm/boot/dts/tegra20-asus-tf101.dts
> >> b/arch/arm/boot/dts/tegra20-asus-tf101.dts
> >> index c2a9c3fb5b33..97350f566539 100644
> >> --- a/arch/arm/boot/dts/tegra20-asus-tf101.dts
> >> +++ b/arch/arm/boot/dts/tegra20-asus-tf101.dts
> >> @@ -82,9 +82,11 @@ hdmi@5428 {
> >>pll-supply = <_pll_reg>;
> >>hdmi-supply = <_hdmi_en>;
> >>
> >> - nvidia,ddc-i2c-bus = <_ddc>;
> >> - nvidia,hpd-gpio = < TEGRA_GPIO(N, 7)
> >> - GPIO_ACTIVE_HIGH>;
> >> + port@0 {
> >> + hdmi_out: endpoint {
> >> + remote-endpoint = <_in>;
> >> + };
> >> + };
> >
> >Does this need a bindings change? nvidia,tegra20-hdmi currently doesn't
> >support OF graphs, so this would probably fail to validate if we merge
> >it without a corresponding DT bindings update.
>
> drm/tegra patch is backwards compatible and connector node is optional.
We still need to document the connector node, otherwise the DT
validation will complain about port@0 being used here, won't it?
Thierry
signature.asc
Description: PGP signature
Re: (subset) [PATCH 1/3] dt-bindings: display: panel: Move HannStar HSD101PWW2 to LVDS
From: Thierry Reding On Wed, 26 Jul 2023 20:48:55 +0200, Thierry Reding wrote: > From: Thierry Reding > > The HannStar HSD101PWW2 is an LVDS panel, so move it to the correct > bindings file. > > Applied, thanks! [3/3] ARM: tegra: Use Hannstar HSD101PWW2 on Pegatron Chagall commit: b28d3af99ac4885f136f6330fec6499b15ad5b25 Best regards, -- Thierry Reding
Re: (subset) [PATCH 1/3] dt-bindings: display: panel: Move Chunghwa CLAA070WP03XG to LVDS
From: Thierry Reding On Wed, 26 Jul 2023 20:50:08 +0200, Thierry Reding wrote: > From: Thierry Reding > > The Chunghwa CLAA070WP03XG is an LVDS panel, so move it to the correct > bindings file. > > Applied, thanks! [3/3] ARM: tegra: Provide specific compatible string for Nexus 7 panel commit: c9a706ab227ef59cc49923358513251ca4965563 Best regards, -- Thierry Reding
Re: [PATCH v2 2/2] ARM: tegra: transformers: add connector node
On Sun, Jun 18, 2023 at 11:50:46AM +0300, Svyatoslav Ryhel wrote:
> All ASUS Transformers have micro-HDMI connector directly available.
> After Tegra HDMI got bridge/connector support, we should use connector
> framework for proper HW description.
>
> Tested-by: Andreas Westman Dorcsak # ASUS TF T30
> Tested-by: Robert Eckelmann # ASUS TF101 T20
> Tested-by: Svyatoslav Ryhel # ASUS TF201 T30
> Signed-off-by: Svyatoslav Ryhel
> ---
> arch/arm/boot/dts/tegra20-asus-tf101.dts | 22 ---
> .../dts/tegra30-asus-transformer-common.dtsi | 21 --
> 2 files changed, 38 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/boot/dts/tegra20-asus-tf101.dts
> b/arch/arm/boot/dts/tegra20-asus-tf101.dts
> index c2a9c3fb5b33..97350f566539 100644
> --- a/arch/arm/boot/dts/tegra20-asus-tf101.dts
> +++ b/arch/arm/boot/dts/tegra20-asus-tf101.dts
> @@ -82,9 +82,11 @@ hdmi@5428 {
> pll-supply = <_pll_reg>;
> hdmi-supply = <_hdmi_en>;
>
> - nvidia,ddc-i2c-bus = <_ddc>;
> - nvidia,hpd-gpio = < TEGRA_GPIO(N, 7)
> - GPIO_ACTIVE_HIGH>;
> + port@0 {
> + hdmi_out: endpoint {
> + remote-endpoint = <_in>;
> + };
> + };
Does this need a bindings change? nvidia,tegra20-hdmi currently doesn't
support OF graphs, so this would probably fail to validate if we merge
it without a corresponding DT bindings update.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH v2 0/2] Support bridge/connector by Tegra HDMI
On Sun, Jun 18, 2023 at 11:50:44AM +0300, Svyatoslav Ryhel wrote: > This patch adds support for the bridge/connector attached to the > HDMI output, allowing to model the hardware properly. It keeps > backwards compatibility with existing bindings and is required > by devices which have a simple or MHL bridge connected to HDMI > output like ASUS P1801-T or LG P880/P895 or HTC One X. > > Tested on ASUS Transformers which have no dedicated bridge but > have type d HDMI connector directly available. Tests went smoothly. If I understand correctly, we still need the drm/tegra patch to be applied before the DT change, otherwise the driver won't know what to do about the connector, right? That shouldn't be big problem, but it means that the patches need to be staged in correctly to avoid breaking things. Thierry signature.asc Description: PGP signature
[PATCH 3/3] ARM: tegra: Provide specific compatible string for Nexus 7 panel
From: Thierry Reding
panel-lvds alone is not a valid compatible string and we always need a
specific compatible string as well. Nexus 7 can come with one of (at
least) two panels, so pick one of them as the specific compatible
string.
Signed-off-by: Thierry Reding
---
.../nvidia/tegra30-asus-nexus7-grouper-common.dtsi | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/arch/arm/boot/dts/nvidia/tegra30-asus-nexus7-grouper-common.dtsi
b/arch/arm/boot/dts/nvidia/tegra30-asus-nexus7-grouper-common.dtsi
index 4fa6b20c4fdb..a9342e04b14b 100644
--- a/arch/arm/boot/dts/nvidia/tegra30-asus-nexus7-grouper-common.dtsi
+++ b/arch/arm/boot/dts/nvidia/tegra30-asus-nexus7-grouper-common.dtsi
@@ -1092,15 +1092,11 @@ cpu3: cpu@3 {
display-panel {
/*
-* Nexus 7 supports two compatible panel models:
-*
-* 1. hydis,hv070wx2-1e0
-* 2. chunghwa,claa070wp03xg
-*
-* We want to use timing which is optimized for Nexus 7,
-* hence we need to customize the timing.
+* Some device variants come with a Hydis HV070WX2-1E0, but
+* since they are all largely compatible, we'll go with the
+* Chunghwa one here.
*/
- compatible = "panel-lvds";
+ compatible = "chunghwa,claa070wp03xg", "panel-lvds";
width-mm = <94>;
height-mm = <150>;
--
2.41.0
[PATCH 1/3] dt-bindings: display: panel: Move Chunghwa CLAA070WP03XG to LVDS
From: Thierry Reding The Chunghwa CLAA070WP03XG is an LVDS panel, so move it to the correct bindings file. Signed-off-by: Thierry Reding --- Documentation/devicetree/bindings/display/panel/panel-lvds.yaml | 2 ++ .../devicetree/bindings/display/panel/panel-simple.yaml | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml b/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml index 344e5df40c2f..dbbf32a8be87 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml @@ -40,6 +40,8 @@ properties: items: - enum: - auo,b101ew05 + # Chunghwa Picture Tubes Ltd. 7" WXGA (800x1280) TFT LCD LVDS panel + - chunghwa,claa070wp03xg # HannStar Display Corp. HSD101PWW2 10.1" WXGA (1280x800) LVDS panel - hannstar,hsd101pww2 - tbs,a711-panel diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml index f4d9da4afefd..67959290b212 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml @@ -103,8 +103,6 @@ properties: - cdtech,s070wv95-ct16 # Chefree CH101OLHLWH-002 10.1" (1280x800) color TFT LCD panel - chefree,ch101olhlwh-002 -# Chunghwa Picture Tubes Ltd. 7" WXGA TFT LCD panel - - chunghwa,claa070wp03xg # Chunghwa Picture Tubes Ltd. 10.1" WXGA TFT LCD panel - chunghwa,claa101wa01a # Chunghwa Picture Tubes Ltd. 10.1" WXGA TFT LCD panel -- 2.41.0
[PATCH 2/3] dt-bindings: display: panel: Document Hydis HV070WX2-1E0
From: Thierry Reding The Hydis HV070WX2-1E0 is a 7" WXGA (800x1280) TFT LCD LVDS panel that is one of the variants used on Google Nexus 7. Signed-off-by: Thierry Reding --- Documentation/devicetree/bindings/display/panel/panel-lvds.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml b/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml index dbbf32a8be87..9f1016551e0b 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml @@ -44,6 +44,8 @@ properties: - chunghwa,claa070wp03xg # HannStar Display Corp. HSD101PWW2 10.1" WXGA (1280x800) LVDS panel - hannstar,hsd101pww2 + # Hydis Technologies 7" WXGA (800x1280) TFT LCD LVDS panel + - hydis,hv070wx2-1e0 - tbs,a711-panel - const: panel-lvds -- 2.41.0
[PATCH 3/3] ARM: tegra: Use Hannstar HSD101PWW2 on Pegatron Chagall
From: Thierry Reding
The LVDS bindings require a specific compatible string in addition to
the generic "panel-lvds". Add the HannStar HSD101PWW2 which is used on
a similar device (ASUS TF201) and seems to work fine with slightly
modified timings in DT.
Suggested-by: Svyatoslav Ryhel
Signed-off-by: Thierry Reding
---
arch/arm/boot/dts/nvidia/tegra30-pegatron-chagall.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/nvidia/tegra30-pegatron-chagall.dts
b/arch/arm/boot/dts/nvidia/tegra30-pegatron-chagall.dts
index c81d5875c31c..4012f9c799a8 100644
--- a/arch/arm/boot/dts/nvidia/tegra30-pegatron-chagall.dts
+++ b/arch/arm/boot/dts/nvidia/tegra30-pegatron-chagall.dts
@@ -2628,7 +2628,7 @@ cpu3: cpu@3 {
};
display-panel {
- compatible = "panel-lvds";
+ compatible = "hannstar,hsd101pww2", "panel-lvds";
width-mm = <217>;
height-mm = <136>;
--
2.41.0
[PATCH 2/3] drm/panel: Relax porches for HannStar HSD101PWW2
From: Thierry Reding
The porch maximum values for the HannStar HSD101PWW2 are unusually
small. Make them a bit larger to allow a more flexibility when
overriding the timings in device tree. Unfortunately the datasheet
doesn't list porch limits in detail, so this is a bit of guesswork.
Signed-off-by: Thierry Reding
---
drivers/gpu/drm/panel/panel-simple.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index 4badda6570d5..4bab181e9d4b 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2104,13 +2104,13 @@ static const struct panel_desc hannstar_hsd100pxn1 = {
static const struct display_timing hannstar_hsd101pww2_timing = {
.pixelclock = { 6430, 7110, 8200 },
.hactive = { 1280, 1280, 1280 },
- .hfront_porch = { 1, 1, 10 },
- .hback_porch = { 1, 1, 10 },
- .hsync_len = { 58, 158, 661 },
+ .hfront_porch = { 1, 1, 64 },
+ .hback_porch = { 1, 1, 64 },
+ .hsync_len = { 58, 158, 553 },
.vactive = { 800, 800, 800 },
- .vfront_porch = { 1, 1, 10 },
- .vback_porch = { 1, 1, 10 },
- .vsync_len = { 1, 21, 203 },
+ .vfront_porch = { 1, 1, 32 },
+ .vback_porch = { 1, 1, 32 },
+ .vsync_len = { 1, 21, 159 },
.flags = DISPLAY_FLAGS_DE_HIGH,
};
--
2.41.0
[PATCH 1/3] dt-bindings: display: panel: Move HannStar HSD101PWW2 to LVDS
From: Thierry Reding The HannStar HSD101PWW2 is an LVDS panel, so move it to the correct bindings file. Signed-off-by: Thierry Reding --- Documentation/devicetree/bindings/display/panel/panel-lvds.yaml | 2 ++ .../devicetree/bindings/display/panel/panel-simple.yaml | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml b/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml index 929fe046d1e7..344e5df40c2f 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml @@ -40,6 +40,8 @@ properties: items: - enum: - auo,b101ew05 + # HannStar Display Corp. HSD101PWW2 10.1" WXGA (1280x800) LVDS panel + - hannstar,hsd101pww2 - tbs,a711-panel - const: panel-lvds diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml index df1cec8fd21b..f4d9da4afefd 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml @@ -168,8 +168,6 @@ properties: - hannstar,hsd070pww1 # HannStar Display Corp. HSD100PXN1 10.1" XGA LVDS panel - hannstar,hsd100pxn1 -# HannStar Display Corp. HSD101PWW2 10.1" WXGA (1280x800) LVDS panel - - hannstar,hsd101pww2 # Hitachi Ltd. Corporation 9" WVGA (800x480) TFT LCD panel - hit,tx23d38vm0caa # InfoVision Optoelectronics M133NWF4 R0 13.3" FHD (1920x1080) TFT LCD panel -- 2.41.0
Re: (subset) [PATCH v2 0/4] video: backlight: lp855x: modernize bindings
From: Thierry Reding On Fri, 19 May 2023 20:07:24 +0200, Artur Weber wrote: > Convert TI LP855X backlight controller bindings from TXT to YAML and, > while we're at it, rework some of the code related to PWM handling. > Also correct existing DTS files to avoid introducing new dtb_check > errors. > > Signed-off-by: Artur Weber > > [...] Applied, thanks! [4/4] arm64: dts: adapt to LP855X bindings changes commit: faae0778fa10fa4e8909fe9164f06acab170f1e9 Best regards, -- Thierry Reding
Re: [PATCH v3 2/2] gpu: host1x: Stop open-coding of_device_uevent()
On Thu, Jun 22, 2023 at 11:32:14PM +0200, Miquel Raynal wrote: > There is apparently no reasons to open-code of_device_uevent() besides: > - The helper receives a struct device while we want to use the of_node > member of the struct device *parent*. > - of_device_uevent() could not be called by modules because of a missing > EXPORT_SYMBOL*(). > > In practice, the former point is not very constraining, just calling > of_device_uevent(dev->parent, ...) would have made the trick. Yeah, looks like that's correct. I guess I always thought this information would get added to the sysfs node of the struct device * that was passed in, while it actually gets passed to the environment created for the struct device passed into the caller. In other words what we pass to of_device_uevent() here is only ever used as a source of information, so passing dev->parent works. I've also verified this on Tegra30 Beaver just to make sure and it looks like the generated uevent file is identical before and after this patch. Applied to drm-misc, thanks. Thierry signature.asc Description: PGP signature
Re: [PATCH] gpu/host1x: Explicitly include correct DT includes
On Fri, Jul 14, 2023 at 11:45:49AM -0600, Rob Herring wrote: > The DT of_device.h and of_platform.h date back to the separate > of_platform_bus_type before it as merged into the regular platform bus. > As part of that merge prepping Arm DT support 13 years ago, they > "temporarily" include each other. They also include platform_device.h > and of.h. As a result, there's a pretty much random mix of those include > files used throughout the tree. In order to detangle these headers and > replace the implicit includes with struct declarations, users need to > explicitly include the correct includes. > > Signed-off-by: Rob Herring > --- > drivers/gpu/host1x/context.c | 2 +- > drivers/gpu/host1x/dev.c | 3 ++- > 2 files changed, 3 insertions(+), 2 deletions(-) Applied to drm-misc, thanks. Thierry signature.asc Description: PGP signature
Re: [PATCH] drm: Explicitly include correct DT includes
On Fri, Jul 14, 2023 at 11:45:34AM -0600, Rob Herring wrote: > The DT of_device.h and of_platform.h date back to the separate > of_platform_bus_type before it as merged into the regular platform bus. > As part of that merge prepping Arm DT support 13 years ago, they > "temporarily" include each other. They also include platform_device.h > and of.h. As a result, there's a pretty much random mix of those include > files used throughout the tree. In order to detangle these headers and > replace the implicit includes with struct declarations, users need to > explicitly include the correct includes. > > Signed-off-by: Rob Herring [Trimming the recipient list so that Google will let me send this] Test builds were fine, so I've pushed this to drm-misc now. Thierry signature.asc Description: PGP signature
Re: [Freedreno] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
On Thu, Jul 13, 2023 at 05:39:36PM +0200, Uwe Kleine-König wrote: > On Thu, Jul 13, 2023 at 10:41:45AM -0400, Sean Paul wrote: > > On Thu, Jul 13, 2023 at 9:04 AM Uwe Kleine-König > > > But even with the one-patch-per-rename approach I'd consider the > > > renaming a net win, because ease of understanding code has a big value. > > > It's value is not so easy measurable as "conflicts when backporting", > > > but it also matters in say two years from now, while backporting > > > shouldn't be an issue then any more. > > > > You've rightly identified the conjecture in your statement. I've been > > on both sides of the argument, having written/maintained drm code > > upstream and cherry-picked changes to a downstream kernel. Perhaps > > it's because drm's definition of dev is ingrained in my muscle memory, > > or maybe it's because I don't do a lot of upstream development these > > days, but I just have a hard time seeing the benefit here. > > I see that my change doesn't immediately benefit those who are already > at home in drivers/gpu/drm. So it's quite understandable that someone in > a senior role (long time contributor or maintainer) doesn't see a big > upside. > > In contrast I think my change (with its indisputable cost) lowers the > bar for new contributors to drm. IMHO that's something a maintainer has > to have on their radar, too, and that's easily undervalued in my > experience. To be honest, DRM/KMS is quite a complex subsystem, so whether dev is struct device or struct drm_device doesn't really do anything to move the bar in either direction. Thierry signature.asc Description: PGP signature
Re: [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
Shortening the absurdly long recipient list, Google won't let me send this otherwise. On Wed, Jul 12, 2023 at 05:34:28PM +0300, Jani Nikula wrote: > On Wed, 12 Jul 2023, Uwe Kleine-König wrote: > > Hello, > > > > while I debugged an issue in the imx-lcdc driver I was constantly > > irritated about struct drm_device pointer variables being named "dev" > > because with that name I usually expect a struct device pointer. > > > > I think there is a big benefit when these are all renamed to "drm_dev". > > I have no strong preference here though, so "drmdev" or "drm" are fine > > for me, too. Let the bikesheding begin! > > > > Some statistics: > > > > $ git grep -ohE 'struct drm_device *\* *[^ (),;]*' v6.5-rc1 | sort | uniq > > -c | sort -n > > 1 struct drm_device *adev_to_drm > > 1 struct drm_device *drm_ > > 1 struct drm_device *drm_dev > > 1 struct drm_device*drm_dev > > 1 struct drm_device *pdev > > 1 struct drm_device *rdev > > 1 struct drm_device *vdev > > 2 struct drm_device *dcss_drv_dev_to_drm > > 2 struct drm_device **ddev > > 2 struct drm_device *drm_dev_alloc > > 2 struct drm_device *mock > > 2 struct drm_device *p_ddev > > 5 struct drm_device *device > > 9 struct drm_device * dev > > 25 struct drm_device *d > > 95 struct drm_device * > > 216 struct drm_device *ddev > > 234 struct drm_device *drm_dev > > 611 struct drm_device *drm > >4190 struct drm_device *dev > > > > This series starts with renaming struct drm_crtc::dev to drm_dev. If > > it's not only me and others like the result of this effort it should be > > followed up by adapting the other structs and the individual usages in > > the different drivers. > > I think this is an unnecessary change. In drm, a dev is usually a drm > device, i.e. struct drm_device *. As shown by the numbers above. I fully agree with this. I can understand that this is potentially confusing to people that work across subsystems and aren't used to seeing this. But to most DRM/KMS developers this is quite familiar and since they spend much more time on this than the occasional cross-kernel patch, it outweighs the minor nuisance that this may be for everyone else. Since Uwe has already shown that "dev" is the consensus, I think if we really must have this kind of patch the logical choice would be to change any struct drm_device * to be named "dev" rather than trying to establish a new consensus. > If folks insist on following through with this anyway, I'm firmly in the > camp the name should be "drm" and nothing else. Same here. If people can't live with "dev", then simply "drm" seems like the next best choice. It's short and to the point. A _dev suffix is redundant because any other DRM structure variables will typically be named something else anyway (i.e. crtc, connector, encoder, plane, ... or a corresponding abbreviated form). Thierry signature.asc Description: PGP signature
Re: [PATCH 12/18] drm/tegra: dpaux: Convert to devm_platform_ioremap_resource()
On Fri, Jul 07, 2023 at 03:20:28PM +0800, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li
> ---
> drivers/gpu/drm/tegra/dpaux.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
> index 4d2677dcd831..f120897ce4b3 100644
> --- a/drivers/gpu/drm/tegra/dpaux.c
> +++ b/drivers/gpu/drm/tegra/dpaux.c
> @@ -447,7 +447,6 @@ static const struct pinmux_ops tegra_dpaux_pinmux_ops = {
> static int tegra_dpaux_probe(struct platform_device *pdev)
> {
> struct tegra_dpaux *dpaux;
> - struct resource *regs;
> u32 value;
> int err;
>
> @@ -461,14 +460,13 @@ static int tegra_dpaux_probe(struct platform_device
> *pdev)
> INIT_LIST_HEAD(>list);
> dpaux->dev = >dev;
>
> - regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - dpaux->regs = devm_ioremap_resource(>dev, regs);
> + dpaux->regs = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(dpaux->regs))
> return PTR_ERR(dpaux->regs);
>
> dpaux->irq = platform_get_irq(pdev, 0);
> if (dpaux->irq < 0)
> - return -ENXIO;
> + return dpaux->irq;
This change has nothing to do with what the commit message says. It
looks correct, but it should be a separate patch.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH v3 05/12] drm/tegra: Store pointer to vmap'ed framebuffer in screen_buffer
On Fri, Jul 07, 2023 at 10:31:56AM +0200, Thomas Zimmermann wrote: > Tegra uses DMA-able memory, which has to be acessed with CPU ops > for system-memory. Store the framebuffer's vmap address in struct > fb_info.screen_buffer. The currently used field 'screen_base' is > for I/O memory. > > Suggested-by: Thierry Reding > Signed-off-by: Thomas Zimmermann > Cc: Thierry Reding > Cc: Mikko Perttunen > --- > drivers/gpu/drm/tegra/fbdev.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Acked-by: Thierry Reding signature.asc Description: PGP signature
Re: [PATCH v3 04/12] drm/tegra: Set fbdev FBINFO_VIRTFB flag
On Fri, Jul 07, 2023 at 10:31:55AM +0200, Thomas Zimmermann wrote: > Mark the framebuffer with FBINFO_VIRTFB. The framebuffer range is > in DMA-able memory and should be accessed with the CPU's regular > memory ops. > > v2: > * drop FBINFO_DEFAULT > > Signed-off-by: Thomas Zimmermann > Reviewed-by: Javier Martinez Canillas > Acked-by: Maxime Ripard > Cc: Thierry Reding > Cc: Mikko Perttunen > --- > drivers/gpu/drm/tegra/fbdev.c | 1 + > 1 file changed, 1 insertion(+) Acked-by: Thierry Reding signature.asc Description: PGP signature
Re: [PATCH v3 03/12] drm/tegra: Use fbdev DMA helpers
On Fri, Jul 07, 2023 at 10:31:54AM +0200, Thomas Zimmermann wrote: > Use fbdev's DMA helpers for fbdev emulation. They are equivalent to the > previously used system-memory helpers, so no functional changes here. > > Signed-off-by: Thomas Zimmermann > Reviewed-by: Javier Martinez Canillas > Acked-by: Maxime Ripard > Cc: Thierry Reding > Cc: Mikko Perttunen > --- > drivers/gpu/drm/tegra/Kconfig | 2 +- > drivers/gpu/drm/tegra/fbdev.c | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) Acked-by: Thierry Reding signature.asc Description: PGP signature
Re: [PATCH v2 04/11] drm/tegra: Set fbdev FBINFO_VIRTFB flag
On Thu, Jul 06, 2023 at 02:46:42PM +0200, Thomas Zimmermann wrote: > Mark the framebuffer with FBINFO_VIRTFB. The framebuffer range is > in DMA-able memory and should be accessed with the CPU's regular > memory ops. > > v2: > * drop FBINFO_DEFAULT > > Signed-off-by: Thomas Zimmermann > Reviewed-by: Javier Martinez Canillas > Acked-by: Maxime Ripard > Cc: Thierry Reding > Cc: Mikko Perttunen > --- > drivers/gpu/drm/tegra/fbdev.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/tegra/fbdev.c b/drivers/gpu/drm/tegra/fbdev.c > index 82577b7c88da..d8460c5dc91e 100644 > --- a/drivers/gpu/drm/tegra/fbdev.c > +++ b/drivers/gpu/drm/tegra/fbdev.c > @@ -132,6 +132,7 @@ static int tegra_fbdev_probe(struct drm_fb_helper *helper, > } > } > > + info->flags |= FBINFO_VIRTFB; > info->screen_base = (void __iomem *)bo->vaddr + offset; As part of this, do we also need to set info->screen_buffer instead of info->screen_base? The drm_fbdev_dma_helper functions do that. Thierry > info->screen_size = size; > info->fix.smem_start = (unsigned long)(bo->iova + offset); > -- > 2.41.0 > signature.asc Description: PGP signature
Re: [PATCH v2 0/2] Enable GPU on Smaug
From: Thierry Reding On Tue, 16 May 2023 09:28:27 +0100, Diogo Ivo wrote: > This patch series enables the use of the GM20B GPU in the > Google Pixel C. > > Patch 1 adds the needed regulator DT node for the GPU. > > Patch 2 enables the GPU in the DT. > > [...] Applied, thanks! [1/2] arm64: dts: tegra: smaug: add GPU power rail regulator (no commit info) [2/2] arm64: dts: tegra: smaug: add GPU node (no commit info) Best regards, -- Thierry Reding
Re: [PATCH v5 09/13] drm/tegra: Use regular fbdev I/O helpers
On Tue, May 30, 2023 at 05:12:24PM +0200, Thomas Zimmermann wrote: > Use the regular fbdev helpers for framebuffer I/O instead of DRM's > helpers. Tegra does not use damage handling, so DRM's fbdev helpers > are mere wrappers around the fbdev code. > > By using fbdev helpers directly within each DRM fbdev emulation, > we can eventually remove DRM's wrapper functions entirely. > > v4: > * use initializer macros for struct fb_ops > v2: > * use FB_SYS_HELPERS option > > Signed-off-by: Thomas Zimmermann > Acked-by: Sam Ravnborg > Cc: Thierry Reding > Cc: Mikko Perttunen > Cc: Jonathan Hunter > --- > drivers/gpu/drm/tegra/Kconfig | 1 + > drivers/gpu/drm/tegra/fbdev.c | 8 +++- > 2 files changed, 4 insertions(+), 5 deletions(-) Acked-by: Thierry Reding signature.asc Description: PGP signature
Re: [GIT PULL] drm/tegra: Changes for v6.4-rc1
On Thu, Apr 06, 2023 at 03:55:10PM +0200, Daniel Vetter wrote: > On Thu, Apr 06, 2023 at 04:18:46PM +0300, Mikko Perttunen wrote: > > On 4/6/23 16:09, Daniel Vetter wrote: > > > On Thu, Apr 06, 2023 at 02:14:04PM +0200, Thierry Reding wrote: > > > > Hi Dave, Daniel, > > > > > > > > The following changes since commit > > > > e8d018dd0257f744ca50a729e3d042cf2ec9da65: > > > > > > > >Linux 6.3-rc3 (2023-03-19 13:27:55 -0700) > > > > > > > > are available in the Git repository at: > > > > > > > >https://gitlab.freedesktop.org/drm/tegra.git > > > > tags/drm/tegra/for-6.4-rc1 > > > > > > > > for you to fetch changes up to 2429b3c529da29d4277d519bd66d034842dcd70c: > > > > > > > >drm/tegra: Avoid potential 32-bit integer overflow (2023-04-06 > > > > 14:02:33 +0200) > > > > > > > > Thanks, > > > > Thierry > > > > > > > > > > > > drm/tegra: Changes for v6.4-rc1 > > > > > > > > The majority of this is minor cleanups and fixes. Other than those, this > > > > contains Uwe's conversion to the new driver remove callback and Thomas' > > > > fbdev DRM client conversion. The driver can now also be built on other > > > > architectures to easy compile coverage. > > > > > > Neat cleanup on top might be too look at the generic fbdev stuff, just as > > > an idea. > > > > > > > Finally, this adds Mikko as a second maintainer for the driver. As a > > > > next step we also want Tegra DRM to move into drm-misc to streamline the > > > > maintenance process. > > > > > > Amusingly the one patch that dim flagged as lacking a 2nd set of eyes (no > > > a-b/rb or committer!=author) is the MAINTAINERS patch, would have been > > > good to record Mikko's ack for getting volunteered :-) > > > > Haha, admittedly I was a bit surprised to see myself being added to > > MAINTAINERS so quickly after talking about it with Thierry; but yes, I > > submit myself to the duty :) > > Thanks for confirming! :-) I guess I tried to rush it so that Mikko wouldn't get cold feet. =P Thierry signature.asc Description: PGP signature
[GIT PULL] drm/tegra: Changes for v6.4-rc1
Hi Dave, Daniel, The following changes since commit e8d018dd0257f744ca50a729e3d042cf2ec9da65: Linux 6.3-rc3 (2023-03-19 13:27:55 -0700) are available in the Git repository at: https://gitlab.freedesktop.org/drm/tegra.git tags/drm/tegra/for-6.4-rc1 for you to fetch changes up to 2429b3c529da29d4277d519bd66d034842dcd70c: drm/tegra: Avoid potential 32-bit integer overflow (2023-04-06 14:02:33 +0200) Thanks, Thierry drm/tegra: Changes for v6.4-rc1 The majority of this is minor cleanups and fixes. Other than those, this contains Uwe's conversion to the new driver remove callback and Thomas' fbdev DRM client conversion. The driver can now also be built on other architectures to easy compile coverage. Finally, this adds Mikko as a second maintainer for the driver. As a next step we also want Tegra DRM to move into drm-misc to streamline the maintenance process. Cai Huoqing (3): drm/tegra: sor: Make use of the helper function dev_err_probe() drm/tegra: dsi: Make use of the helper function dev_err_probe() drm/tegra: plane: Improve use of dev_err_probe() Christian König (2): drm/tegra: Allow compile test on !ARM v2 drm/tegra: Fix another missing include Deepak R Varma (1): drm/tegra: sor: Remove redundant error logging Diogo Ivo (1): drm/tegra: dsi: Clear enable register if powered by bootloader Lee Jones (1): drm/tegra: dc: Remove set but unused variable 'state' Mikko Perttunen (1): gpu: host1x: Don't rely on dma_fence_wait_timeout return value Nur Hussein (1): drm/tegra: Avoid potential 32-bit integer overflow Thierry Reding (1): MAINTAINERS: Add Mikko as backup maintainer for Tegra DRM Thomas Zimmermann (7): drm/tegra: Include drm/tegra: Include drm/tegra: Removed fb from struct tegra_fbdev drm/tegra: Remove struct tegra_fbdev drm/tegra: Hide fbdev support behind config option drm/tegra: Initialize fbdev DRM client drm/tegra: Implement fbdev emulation as in-kernel client Uwe Kleine-König (12): gpu: host1x: Make host1x_client_unregister() return void drm/tegra: rgb: Make tegra_dc_rgb_remove() return void drm/tegra: dc: Convert to platform remove callback returning void drm/tegra: dpaux: Convert to platform remove callback returning void drm/tegra: dsi: Convert to platform remove callback returning void drm/tegra: gr2d: Convert to platform remove callback returning void drm/tegra: gr3d: Convert to platform remove callback returning void drm/tegra: hdmi: Convert to platform remove callback returning void drm/tegra: hub: Convert to platform remove callback returning void drm/tegra: nvdec: Convert to platform remove callback returning void drm/tegra: sor: Convert to platform remove callback returning void drm/tegra: vic: Convert to platform remove callback returning void Yang Yingliang (2): gpu: host1x: Fix potential double free if IOMMU is disabled gpu: host1x: Fix memory leak of device names Ye Xingchen (1): gpu: host1x: mipi: Use devm_platform_get_and_ioremap_resource() MAINTAINERS | 1 + drivers/gpu/drm/tegra/Kconfig | 2 +- drivers/gpu/drm/tegra/Makefile | 2 + drivers/gpu/drm/tegra/dc.c | 22 +-- drivers/gpu/drm/tegra/dc.h | 2 +- drivers/gpu/drm/tegra/dpaux.c | 6 +- drivers/gpu/drm/tegra/drm.c | 23 +-- drivers/gpu/drm/tegra/drm.h | 27 ++-- drivers/gpu/drm/tegra/dsi.c | 51 --- drivers/gpu/drm/tegra/fb.c | 242 +--- drivers/gpu/drm/tegra/fbdev.c | 241 +++ drivers/gpu/drm/tegra/gem.c | 1 + drivers/gpu/drm/tegra/gr2d.c| 14 +- drivers/gpu/drm/tegra/gr3d.c| 14 +- drivers/gpu/drm/tegra/hdmi.c| 14 +- drivers/gpu/drm/tegra/hub.c | 13 +- drivers/gpu/drm/tegra/nvdec.c | 14 +- drivers/gpu/drm/tegra/output.c | 3 + drivers/gpu/drm/tegra/plane.c | 16 +-- drivers/gpu/drm/tegra/rgb.c | 7 +- drivers/gpu/drm/tegra/sor.c | 44 ++ drivers/gpu/drm/tegra/vic.c | 14 +- drivers/gpu/host1x/Kconfig | 2 +- drivers/gpu/host1x/bus.c| 6 +- drivers/gpu/host1x/context.c| 24 ++-- drivers/gpu/host1x/mipi.c | 4 +- drivers/gpu/host1x/syncpt.c | 8 +- drivers/staging/media/tegra-video/csi.c | 8 +- drivers/staging/media/tegra-video/vi.c | 8 +- include/linux/host1x.h | 2 +- 30 files changed, 370 insertions(+), 465 deletions(-) create mode 100644 drivers/gpu/drm/tegra/fbdev.c
Re: [PATCH] drm/tegra : Avoid potential integer overflow of 32 bit int
From: Thierry Reding On Thu, 6 Apr 2023 04:25:59 +0800, Nur Hussein wrote: > In tegra_sor_compute_config(), the 32-bit value mode->clock > is multiplied by 1000, and assigned to the u64 variable pclk. > We can avoid a potential 32-bit integer overflow by casting > mode->clock to u64 before we do the arithmetic and assignment. > > Applied, thanks! [1/1] drm/tegra : Avoid potential integer overflow of 32 bit int (no commit info) Best regards, -- Thierry Reding
Re: [PATCH 0/7] drm/tegra: Convert fbdev to DRM client
From: Thierry Reding On Thu, 30 Mar 2023 10:36:00 +0200, Thomas Zimmermann wrote: > Convert tegra's fbdev code to struct drm_client. Replaces the current > ad-hoc integration. The conversion includes a number of cleanups. As > with most other drivers' fbdev emulation, fbdev in tegra is now just > another DRM client that runs after the DRM device has been registered. > > Once all drivers' fbdev emulation has been converted to struct drm_client, > we can attempt to add additional in-kernel clients. A DRM-based dmesg > log or a bootsplash are commonly mentioned. DRM can then switch easily > among the existing clients if/when required. > > [...] Applied, thanks! [1/7] drm/tegra: Include commit: 162b2ae95e0887ea75883bc419d55dd714b8fbf5 [2/7] drm/tegra: Include commit: 0e4ec6d97a2c6e96a5ec8d0edc00aa658238ed3f [3/7] drm/tegra: Removed fb from struct tegra_fbdev commit: 5705d5b6a21e75c095df29deec8a13aa6b59f83c [4/7] drm/tegra: Remove struct tegra_fbdev commit: fc5646b848222601d8be78b66b6498130437abe1 [5/7] drm/tegra: Hide fbdev support behind config option commit: 63ab4848d1d2eda1658ae82a3cb6eb7e03d28cec [6/7] drm/tegra: Initialize fbdev DRM client commit: d9d1e306e70db905f29d05952c1499fd3c6ef6ef [7/7] drm/tegra: Implement fbdev emulation as in-kernel client commit: 8e5113c627334ed32748d95ababd548171d2333d Best regards, -- Thierry Reding
Re: [PATCH 0/7] drm/tegra: Convert fbdev to DRM client
On Thu, Mar 30, 2023 at 10:36:00AM +0200, Thomas Zimmermann wrote: > Convert tegra's fbdev code to struct drm_client. Replaces the current > ad-hoc integration. The conversion includes a number of cleanups. As > with most other drivers' fbdev emulation, fbdev in tegra is now just > another DRM client that runs after the DRM device has been registered. > > Once all drivers' fbdev emulation has been converted to struct drm_client, > we can attempt to add additional in-kernel clients. A DRM-based dmesg > log or a bootsplash are commonly mentioned. DRM can then switch easily > among the existing clients if/when required. > > I did the conversion from similar experience with other drivers. But I > don't have the hardware to test this. Any testing is welcome. > > Thomas Zimmermann (7): > drm/tegra: Include > drm/tegra: Include > drm/tegra: Removed fb from struct tegra_fbdev > drm/tegra: Remove struct tegra_fbdev > drm/tegra: Hide fbdev support behind config option > drm/tegra: Initialize fbdev DRM client > drm/tegra: Implement fbdev emulation as in-kernel client > > drivers/gpu/drm/tegra/Makefile | 2 + > drivers/gpu/drm/tegra/drm.c| 23 +--- > drivers/gpu/drm/tegra/drm.h| 27 ++-- > drivers/gpu/drm/tegra/fb.c | 242 + > drivers/gpu/drm/tegra/fbdev.c | 240 > drivers/gpu/drm/tegra/output.c | 3 + > drivers/gpu/drm/tegra/rgb.c| 1 + > 7 files changed, 265 insertions(+), 273 deletions(-) > create mode 100644 drivers/gpu/drm/tegra/fbdev.c Seems to be working just fine. Applied, thanks. Thierry signature.asc Description: PGP signature
Re: [RESEND PATCH v4 03/21] staging: media: tegra-video: fix .vidioc_enum_fmt_vid_cap to return all formats
On Wed, Apr 05, 2023 at 10:50:37AM +0200, Hans Verkuil wrote: [...] > Note that this driver will stay in staging since it still fails when I try to > capture from two sensors at the same time: syncpoint errors start appearing > in that case. I think there are locking issues. I think I have someone to take > a look at that, but first I want your series to get merged. Mikko (added) is familiar with syncpoints, so he may be able to help with. Can you provide steps to reproduce these issues? That may make it easier for us to help figure this out. Unfortunately I don't have any device with an actual sensor on it, so I can only test with the test pattern generator, but syncpoint errors sound like they would happen with either setup. Thierry signature.asc Description: PGP signature
Re: [PATCH 3/8] drm/aperture: Remove primary argument
On Tue, Apr 04, 2023 at 10:18:37PM +0200, Daniel Vetter wrote: > Only really pci devices have a business setting this - it's for > figuring out whether the legacy vga stuff should be nuked too. And > with the preceeding two patches those are all using the pci version of > this. > > Which means for all other callers primary == false and we can remove > it now. > > v2: > - Reorder to avoid compile fail (Thomas) > - Include gma500, which retained it's called to the non-pci version. > > Signed-off-by: Daniel Vetter > Cc: Thomas Zimmermann > Cc: Javier Martinez Canillas > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Deepak Rawat > Cc: Neil Armstrong > Cc: Kevin Hilman > Cc: Jerome Brunet > Cc: Martin Blumenstingl > Cc: Thierry Reding > Cc: Jonathan Hunter > Cc: Emma Anholt > Cc: Helge Deller > Cc: David Airlie > Cc: Daniel Vetter > Cc: linux-hyp...@vger.kernel.org > Cc: linux-amlo...@lists.infradead.org > Cc: linux-arm-ker...@lists.infradead.org > Cc: linux-te...@vger.kernel.org > Cc: linux-fb...@vger.kernel.org > --- > drivers/gpu/drm/arm/hdlcd_drv.c | 2 +- > drivers/gpu/drm/armada/armada_drv.c | 2 +- > drivers/gpu/drm/drm_aperture.c | 11 +++ > drivers/gpu/drm/gma500/psb_drv.c| 2 +- > drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 1 - > drivers/gpu/drm/meson/meson_drv.c | 2 +- > drivers/gpu/drm/msm/msm_fbdev.c | 2 +- > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 2 +- > drivers/gpu/drm/stm/drv.c | 2 +- > drivers/gpu/drm/sun4i/sun4i_drv.c | 2 +- > drivers/gpu/drm/tegra/drm.c | 2 +- > drivers/gpu/drm/vc4/vc4_drv.c | 2 +- > include/drm/drm_aperture.h | 7 +++ > 13 files changed, 16 insertions(+), 23 deletions(-) Acked-by: Thierry Reding signature.asc Description: PGP signature
Re: [PATCH] dt-bindings: arm: nvidia: Drop unneeded quotes
From: Thierry Reding On Fri, 31 Mar 2023 13:21:59 -0500, Rob Herring wrote: > Cleanup bindings dropping unneeded quotes. Once all these are fixed, > checking for this can be enabled in yamllint. > > Applied, thanks! [1/1] dt-bindings: arm: nvidia: Drop unneeded quotes commit: c94673e80377d67ba36ee4059d7814b2ab98fa71 Best regards, -- Thierry Reding
Re: [PATCH] gpu: host1x: Don't rely on dma_fence_wait_timeout return value
From: Thierry Reding On Wed, 1 Mar 2023 15:51:06 +0200, Mikko Perttunen wrote: > From: Mikko Perttunen > > dma_fence_wait_timeout (along with a host of other jiffies-based > timeouting functions) returns zero both in case of timeout and when > the wait completes during the last jiffy before timeout. As such, > we can't rely on it to distinguish between success and timeout. > > [...] Applied, thanks! [1/1] gpu: host1x: Don't rely on dma_fence_wait_timeout return value commit: c1aaee94380874fd40f7bb8417c597aba3f72c75 Best regards, -- Thierry Reding
Re: (subset) [PATCH v2 RESEND 2/4] drm/tegra: dsi: Clear enable register if powered by bootloader
From: Thierry Reding On Mon, 28 Nov 2022 16:28:49 +, Diogo Ivo wrote: > In cases where the DSI module is left on by the bootloader > some panels may fail to initialize if the enable register is not cleared > before the panel's initialization sequence is sent, so clear it if that > is the case. > > Applied, thanks! [2/4] drm/tegra: dsi: Clear enable register if powered by bootloader commit: 588ed52d31ab37c5ac86816911f6428d2de265a6 Best regards, -- Thierry Reding
Re: [PATCH v2] drm/tegra: sor: Make use of the helper function dev_err_probe()
From: Thierry Reding On Fri, 17 Sep 2021 10:07:41 +0800, Cai Huoqing wrote: > When possible use dev_err_probe help to properly deal with the > PROBE_DEFER error, the benefit is that DEFER issue will be logged > in the devices_deferred debugfs file. > And using dev_err_probe() can reduce code size, the error value > gets printed. > > > [...] Applied, thanks! [1/1] drm/tegra: sor: Make use of the helper function dev_err_probe() commit: a4c56f2f8ce0e242eb51e5309a743361e2348a64 Best regards, -- Thierry Reding
Re: [PATCH 00/12] drm/tegra: Convert to platform remove callback returning void
From: Thierry Reding On Wed, 22 Mar 2023 18:02:11 +0100, Uwe Kleine-König wrote: > this series adapts the platform drivers below drivers/gpu/drm/tegra to > use the .remove_new() callback. Compared to the traditional .remove() > callback .remove_new() returns no value. This is a good thing because > the driver core doesn't (and cannot) cope for errors during remove. The > only effect of a non-zero return value in .remove() is that the driver > core emits a warning. The device is removed anyhow and an early return > from .remove() usually yields a resource leak. > > [...] Applied, thanks! [01/12] gpu: host1x: Make host1x_client_unregister() return void commit: 1d83d1a2df0bfb6bd79400746c289e2c4edc5909 [02/12] drm/tegra: rgb: Make tegra_dc_rgb_remove() return void commit: bbf9c91c6efaabbd4be0b894d9b753a69a5e02b6 [03/12] drm/tegra: dc: Convert to platform remove callback returning void commit: 902a0ab485b6f2b37450635b82b91cd17e8aa608 [04/12] drm/tegra: dpaux: Convert to platform remove callback returning void commit: 1255aa402c5c8e07ef6d453474ef1bd25651b420 [05/12] drm/tegra: dsi: Convert to platform remove callback returning void commit: 6e470293da9d83569373f83655fdd851bd4dd1d2 [06/12] drm/tegra: gr2d: Convert to platform remove callback returning void commit: f7140bd214d5f55dbd2096673290c8bc2bb6121c [07/12] drm/tegra: gr3d: Convert to platform remove callback returning void commit: d2c29d8095c82eae11af29f8857e854f40186f59 [08/12] drm/tegra: hdmi: Convert to platform remove callback returning void commit: a3365945203bc6c75f8323ce7df38f1a91600ce7 [09/12] drm/tegra: hub: Convert to platform remove callback returning void commit: a407ae48ac9f72f719c6598fe61d03e6b8687349 [10/12] drm/tegra: nvdec: Convert to platform remove callback returning void commit: 222ace4a40bf5b2beafe7e4a226fab673360d689 [11/12] drm/tegra: sor: Convert to platform remove callback returning void commit: b674031a7bbda964741e0fa961cca8ca6b5faae2 [12/12] drm/tegra: vic: Convert to platform remove callback returning void commit: ba0fe014a9ebd5e578d52b2f6521591d409b8f61 Best regards, -- Thierry Reding
Re: (subset) [PATCH 09/37] drm/tegra/dc: Remove set but unused variable 'state'
From: Thierry Reding On Fri, 17 Mar 2023 08:16:50 +, Lee Jones wrote: > Fixes the following W=1 kernel build warning(s): > > drivers/gpu/drm/tegra/dc.c: In function > ‘tegra_crtc_calculate_memory_bandwidth’: > drivers/gpu/drm/tegra/dc.c:2384:38: warning: variable ‘old_state’ set but > not used [-Wunused-but-set-variable] > > Applied, thanks! [09/37] drm/tegra/dc: Remove set but unused variable 'state' commit: 42d364ad88ee81356f0417170bafbdc894594914 Best regards, -- Thierry Reding
Re: [PATCH] drm/tegra: allow compile test on !ARM v2
From: Thierry Reding On Wed, 22 Mar 2023 11:39:15 +0100, Christian König wrote: > This compile tests on x86 just perfectly fine. > > v2: fix missing include complained by kernel test robot > > Applied, thanks! [1/1] drm/tegra: allow compile test on !ARM v2 commit: 224718f4e59442e205524c7307815f09148f051b Best regards, -- Thierry Reding
Re: [PATCH] drm/tegra: plane: Improve use of dev_err_probe()
From: Thierry Reding On Thu, 16 Sep 2021 15:37:21 +0800, Cai Huoqing wrote: > Return dev_err_probe() directly, because the return value of > dev_err_probe() is the appropriate error code, and it can > reduce code size, simplify the code. > > Applied, thanks! [1/1] drm/tegra: plane: Improve use of dev_err_probe() commit: d94e2de7bdb19ae76d9deb541999cff171e14931 Best regards, -- Thierry Reding
Re: [PATCH] drm/tegra: dsi: Make use of the helper function dev_err_probe()
From: Thierry Reding On Thu, 16 Sep 2021 18:56:40 +0800, Cai Huoqing wrote: > When possible use dev_err_probe help to properly deal with the > PROBE_DEFER error, the benefit is that DEFER issue will be logged > in the devices_deferred debugfs file. > And using dev_err_probe() can reduce code size, the error value > gets printed. > > > [...] Applied, thanks! [1/1] drm/tegra: dsi: Make use of the helper function dev_err_probe() commit: 6fae3bc81e679961cfd198622f2aed2d720ee631 Best regards, -- Thierry Reding
[PATCH] MAINTAINERS: Add Mikko as backup maintainer for Tegra DRM
From: Thierry Reding Mikko has been involved as the primary author of the host1x driver and has volunteered to help out with maintenance. Signed-off-by: Thierry Reding --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 7bfa5228d1ea..0a517baef16e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7047,6 +7047,7 @@ F:drivers/phy/mediatek/phy-mtk-mipi* DRM DRIVERS FOR NVIDIA TEGRA M: Thierry Reding +M: Mikko Perttunen L: dri-devel@lists.freedesktop.org L: linux-te...@vger.kernel.org S: Supported -- 2.40.0
