Re: [PATCH] drm/qxl: replace idr_init() by idr_init_base()
On Fri, Nov 06, 2020 at 12:20:16AM +0530, Deepak R Varma wrote:
> idr_init() uses base 0 which is an invalid identifier for this driver.
> The idr_alloc for this driver uses 1 as start value for ID range. The
> new function idr_init_base allows IDR to set the ID lookup from base 1.
> This avoids all lookups that otherwise starts from 0 since 0 is always
> unused / available.
>
> References: commit 6ce711f27500 ("idr: Make 1-based IDRs more efficient")
>
> Signed-off-by: Deepak R Varma
Pushed to drm-misc-next.
thanks,
Gerd
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/virtio: Fix a double free in virtio_gpu_cmd_map()
On Fri, Oct 30, 2020 at 02:48:08PM +0300, Dan Carpenter wrote:
> This is freed both here and in the caller (virtio_gpu_vram_map()) so
> it's a double free. The correct place is only in the caller.
>
> Fixes: 16845c5d5409 ("drm/virtio: implement blob resources: implement vram
> object")
> Signed-off-by: Dan Carpenter
Pushed to drm-misc-next.
thanks,
Gerd
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: don't set page->mapping
Am 05.11.20 um 17:37 schrieb Daniel Vetter: [SNIP] NAK, we use this to determine if a pages belongs to the driver or not in amdgpu for example. Mostly used for debugging, but I would really like to keep that. Can you pls point me at that code? A quick grep hasn't really found much at all. See amdgpu_iomem_read() for an example: Why do you reject this? When IOMMU is disabled or uses an 1 to 1 mapping we would otherwise give the same access as /dev/mem to system memory and that is forbidden. But as I noted this is just for the debugfs file. Ah, there's a config option for that. Plus it's debugfs, anything goes in debugfs, but if you're worried about that hole we should just disable the entire debugfs file for CONFIG_STRICT_DEVMEM. I can perhaps throw that on top, that follow_pfn patch series I'm baking is all about this kind of fun. And exactly that would get a NAK from us. We have specially created that debugfs file as an alternative when CONFIG_STRICT_DEVMEM is set. Uh that doesn't work if you work around core restrictions with your own debugfs paths. That's why we have the restriction to check the mapping of the pages. This way we only expose the memory which was allocated by our driver and don't allow any uncontrolled access to the whole system memory. We have something similar for radeon as well, but there we have a global GART table which we can use for validating stuff. The check doesn't take any locks over the check and copy*user, I don't think it's protecting anything really against somewhat adverse userspace. You mean that it's racing with freeing the pages? Yes that is an obvious problem, but we already had the same issue for radeon as well. On the other hand we could easily prevent that with a lock. I mean fundamentally locking down stuff like STRICT_DEVMEM or all the others makes debugging harder, that's kinda the expected tradeoff. Well we just wanted to have the same debugging functionality we had for radeon. As you said debugfs is a rabbit hole regarding attack vectors anyway. If you are root you can just go to /sys and start reprogramming the hardware to do what you want to do. Regards, Christian. Maybe you can do fun like this in your dkms, but not in upstream. Like if this was specifically created to work around CONFIG_STRICT_DEVMEM (and it sounds like that) then I think this should never have landed in upstream to begin with. I'm also kinda confused that there's distros with CONFIG_STRICT_DEVMEM which allow debugfs. debugfs is a pretty bad root hole all around, or at least that's been the assumption all the time. Yeah, completely agree :) But that's not my problem. I guess I'll do another rfc series and poke a pile of people ... seems to be a habit I'm developing :-) -Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 21/30] usb: host: ehci-tegra: Support OPP and SoC core voltage scaling
05.11.2020 19:07, Alan Stern пишет: > Do you really want to use the same error unwinding for opp_table values > obtained from dev_pm_opp_set_regulators() as from > dev_pm_opp_get_opp_table()? They both are pointing at the same opp_table, which is refcounted. The dev_pm_opp_set_regulators() is dev_pm_opp_get_opp_table() + it sets regulator for the table. https://elixir.bootlin.com/linux/v5.10-rc2/source/drivers/opp/core.c#L1756 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 47/47] PM / devfreq: tegra20: Deprecate in a favor of emc-stat based driver
05.11.2020 05:25, Chanwoo Choi пишет: > Hi Dmitry, > > You need to update the MAINTAINERS file about tegra20-devfreq.c > > 11343 MEMORY FREQUENCY SCALING DRIVERS FOR NVIDIA TEGRA > > 11344 M: Dmitry Osipenko > > 11345 L: linux...@vger.kernel.org > > 11346 L: linux-te...@vger.kernel.org > > 11347 T: git > git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git > 11348 S: Maintained > > 11349 F: drivers/devfreq/tegra20-devfreq.c > > 11350 F: drivers/devfreq/tegra30-devfreq.c > > Except of missing the updating of MAINTAINERS, > Acked-by: Chanwoo Choi Hello Chanwoo, Good catch! Thank you! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: remove pgprot_decrypted() before calls to io_remap_pfn_range()
On Thu, Nov 05, 2020 at 08:17:46PM +0100, Daniel Vetter wrote:
> On Thu, Nov 05, 2020 at 01:00:19PM -0400, Jason Gunthorpe wrote:
> > commit f8f6ae5d077a ("mm: always have io_remap_pfn_range() set
> > pgprot_decrypted()") moves the pgprot_decrypted() into
> > io_remap_pfn_range(). Delete any, now confusing, open coded calls that
> > directly precede io_remap_pfn_range():
> >
> > - drm_io_prot() is only in drm_mmap_locked() to call io_remap_pfn_range()
> >
> > - fb_mmap() immediately calls vm_iomap_memory() which is a convenience
> > wrapper for io_remap_pfn_range()
> >
> > Signed-off-by: Jason Gunthorpe
> > drivers/gpu/drm/drm_vm.c | 3 ---
> > drivers/video/fbdev/core/fbmem.c | 5 -
> > 2 files changed, 8 deletions(-)
> >
> > rc3 will have the dependent patch, this should not be merged to DRM until it
> > has the rc3 commits.
> >
> > There are three other pgprot_decrypted() calls in DRM, I could not figure
> > out
> > what was what there, but other than very special cases I would expect code
> > to
> > use io_remap_pfn_range() instead.
>
> There's 4 now, I think linux-next added one. It's another io_remap_pfn
>
> Of the three you mentioned we have:
> - ttm and i915 use vm_insert_pfn (and ttm also can do also do pud_mkhuge
> entries)
You can't insert IO memory with vmf_insert_pfn_pmd_prot() (it
doesn't set the special flag) so why does it need decrypted?
Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vgm: replace idr_init() by idr_init_base()
On Thu, Nov 05, 2020 at 10:42:15AM +0100, Daniel Vetter wrote:
> On Wed, Nov 04, 2020 at 04:53:38PM +0530, Deepak R Varma wrote:
> > idr_init() uses base 0 which is an invalid identifier. The new function
> > idr_init_base allows IDR to set the ID lookup from base 1. This avoids
> > all lookups that otherwise starts from 0 since 0 is always unused.
> >
> > References: commit 6ce711f27500 ("idr: Make 1-based IDRs more efficient")
> >
> > Signed-off-by: Deepak R Varma
>
> Tiny typo in the commit message summary: s/vgm/vgem/
>
> Also can you pls resbumit this with intel-gfx mailing list on cc (like for
> i915)? There's a CI bot there which runs a few vgem tests, would be good
> to confirm nothing has been broken.
Hi Daniel,
sure. I will correct the summary typo and also feed it to the CI bot.
Also, according to Felix Kuehling's comment on a similar patch for
drm/amdkfd driver, an ID can be 0. The change I am proposing is more
efficient for conditions that do not want to use ID as 0. Otherwise,
id = 0 is an acceptable possibility. So, my statement that "Id 0 is an invalid
identifier" is not true.
Can you please comment if this is accurate and I should reword my log
message as well?
Thank you.
./drv
>
> Otherwise lgtm.
>
> Thanks, Daniel
>
> > ---
> > drivers/gpu/drm/vgem/vgem_fence.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/vgem/vgem_fence.c
> > b/drivers/gpu/drm/vgem/vgem_fence.c
> > index 17f32f550dd9..2902dc6e64fa 100644
> > --- a/drivers/gpu/drm/vgem/vgem_fence.c
> > +++ b/drivers/gpu/drm/vgem/vgem_fence.c
> > @@ -233,7 +233,7 @@ int vgem_fence_signal_ioctl(struct drm_device *dev,
> > int vgem_fence_open(struct vgem_file *vfile)
> > {
> > mutex_init(&vfile->fence_mutex);
> > - idr_init(&vfile->fence_idr);
> > + idr_init_base(&vfile->fence_idr, 1);
> >
> > return 0;
> > }
> > --
> > 2.25.1
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 6/7] drm/vc4: kms: Store the unassigned channel list in the state
If a CRTC is enabled but not active, and that we're then doing a page
flip on another CRTC, drm_atomic_get_crtc_state will bring the first
CRTC state into the global state, and will make us wait for its vblank
as well, even though that might never occur.
Instead of creating the list of the free channels each time atomic_check
is called, and calling drm_atomic_get_crtc_state to retrieve the
allocated channels, let's create a private state object in the main
atomic state, and use it to store the available channels.
Since vc4 has a semaphore (with a value of 1, so a lock) in its commit
implementation to serialize all the commits, even the nonblocking ones, we
are free from the use-after-free race if two subsequent commits are not ran
in their submission order.
Fixes: 87ebcd42fb7b ("drm/vc4: crtc: Assign output to channel automatically")
Reviewed-by: Hoegeun Kwon
Tested-by: Hoegeun Kwon
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_drv.h | 1 +
drivers/gpu/drm/vc4/vc4_kms.c | 124 +++---
2 files changed, 100 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index bdbb9540d47d..014113823647 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -219,6 +219,7 @@ struct vc4_dev {
struct drm_modeset_lock ctm_state_lock;
struct drm_private_obj ctm_manager;
+ struct drm_private_obj hvs_channels;
struct drm_private_obj load_tracker;
/* List of vc4_debugfs_info_entry for adding to debugfs once
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 499c6914fce4..0a231ae500e5 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -37,6 +37,17 @@ static struct vc4_ctm_state *to_vc4_ctm_state(struct
drm_private_state *priv)
return container_of(priv, struct vc4_ctm_state, base);
}
+struct vc4_hvs_state {
+ struct drm_private_state base;
+ unsigned int unassigned_channels;
+};
+
+static struct vc4_hvs_state *
+to_vc4_hvs_state(struct drm_private_state *priv)
+{
+ return container_of(priv, struct vc4_hvs_state, base);
+}
+
struct vc4_load_tracker_state {
struct drm_private_state base;
u64 hvs_load;
@@ -662,6 +673,70 @@ static int vc4_load_tracker_obj_init(struct vc4_dev *vc4)
return drmm_add_action_or_reset(&vc4->base, vc4_load_tracker_obj_fini,
NULL);
}
+static struct drm_private_state *
+vc4_hvs_channels_duplicate_state(struct drm_private_obj *obj)
+{
+ struct vc4_hvs_state *state;
+
+ state = kmemdup(obj->state, sizeof(*state), GFP_KERNEL);
+ if (!state)
+ return NULL;
+
+ __drm_atomic_helper_private_obj_duplicate_state(obj, &state->base);
+
+ return &state->base;
+}
+
+static void vc4_hvs_channels_destroy_state(struct drm_private_obj *obj,
+ struct drm_private_state *state)
+{
+ struct vc4_hvs_state *hvs_state;
+
+ hvs_state = to_vc4_hvs_state(state);
+ kfree(hvs_state);
+}
+
+static const struct drm_private_state_funcs vc4_hvs_state_funcs = {
+ .atomic_duplicate_state = vc4_hvs_channels_duplicate_state,
+ .atomic_destroy_state = vc4_hvs_channels_destroy_state,
+};
+
+static void vc4_hvs_channels_obj_fini(struct drm_device *dev, void *unused)
+{
+ struct vc4_dev *vc4 = to_vc4_dev(dev);
+
+ drm_atomic_private_obj_fini(&vc4->hvs_channels);
+}
+
+static int vc4_hvs_channels_obj_init(struct vc4_dev *vc4)
+{
+ struct vc4_hvs_state *state;
+
+ state = kzalloc(sizeof(*state), GFP_KERNEL);
+ if (!state)
+ return -ENOMEM;
+
+ state->unassigned_channels = GENMASK(HVS_NUM_CHANNELS - 1, 0);
+ drm_atomic_private_obj_init(&vc4->base, &vc4->hvs_channels,
+ &state->base,
+ &vc4_hvs_state_funcs);
+
+ return drmm_add_action_or_reset(&vc4->base, vc4_hvs_channels_obj_fini,
NULL);
+}
+
+static struct vc4_hvs_state *
+vc4_hvs_get_global_state(struct drm_atomic_state *state)
+{
+ struct vc4_dev *vc4 = to_vc4_dev(state->dev);
+ struct drm_private_state *priv_state;
+
+ priv_state = drm_atomic_get_private_obj_state(state,
&vc4->hvs_channels);
+ if (IS_ERR(priv_state))
+ return ERR_CAST(priv_state);
+
+ return to_vc4_hvs_state(priv_state);
+}
+
/*
* The BCM2711 HVS has up to 7 output connected to the pixelvalves and
* the TXP (and therefore all the CRTCs found on that platform).
@@ -678,6 +753,14 @@ static int vc4_load_tracker_obj_init(struct vc4_dev *vc4)
* need to consider all the running CRTCs in the DRM device to assign
* a FIFO, not just the one in the state.
*
+ * - To fix the above, we can't use drm_atomic_get_crtc_state on all
+ * enabled CRTCs to pull their CRTC state into the global state, since
+ * a page flip would start considering their vblank to c
Re: [PATCH v1 00/30] Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs
On Thu, 5 Nov 2020 at 11:40, Viresh Kumar wrote: > > On 05-11-20, 11:34, Ulf Hansson wrote: > > I am not objecting about scaling the voltage through a regulator, > > that's fine to me. However, encoding a power domain as a regulator > > (even if it may seem like a regulator) isn't. Well, unless Mark Brown > > has changed his mind about this. > > > > In this case, it seems like the regulator supply belongs in the > > description of the power domain provider. > > Okay, I wasn't sure if it is a power domain or a regulator here. Btw, > how do we identify if it is a power domain or a regulator ? Good question. It's not a crystal clear line in between them, I think. A power domain to me, means that some part of a silicon (a group of controllers or just a single piece, for example) needs some kind of resource (typically a power rail) to be enabled to be functional, to start with. If there are operating points involved, that's also a clear indication to me, that it's not a regular regulator. Maybe we should try to specify this more exactly in some documentation, somewhere. > > > > In case of Qcom earlier (when we added the performance-state stuff), > > > the eventual hardware was out of kernel's control and we didn't wanted > > > (allowed) to model it as a virtual regulator just to pass the votes to > > > the RPM. And so we did what we did. > > > > > > But if the hardware (where the voltage is required to be changed) is > > > indeed a regulator and is modeled as one, then what Dmitry has done > > > looks okay. i.e. add a supply in the device's node and microvolt > > > property in the DT entries. > > > > I guess I haven't paid enough attention how power domain regulators > > are being described then. I was under the impression that the CPUfreq > > case was a bit specific - and we had legacy bindings to stick with. > > > > Can you point me to some other existing examples of where power domain > > regulators are specified as a regulator in each device's node? > > No, I thought it is a regulator here and not a power domain. Okay, thanks! Kind regards Uffe ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 5/7] drm/vc4: kms: Document the muxing corner cases
We've had a number of muxing corner-cases with specific ways to reproduce
them, so let's document them to make sure they aren't lost and introduce
regressions later on.
Reviewed-by: Hoegeun Kwon
Tested-by: Hoegeun Kwon
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_kms.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index bb2efc5d2d01..499c6914fce4 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -662,6 +662,28 @@ static int vc4_load_tracker_obj_init(struct vc4_dev *vc4)
return drmm_add_action_or_reset(&vc4->base, vc4_load_tracker_obj_fini,
NULL);
}
+/*
+ * The BCM2711 HVS has up to 7 output connected to the pixelvalves and
+ * the TXP (and therefore all the CRTCs found on that platform).
+ *
+ * The naive (and our initial) implementation would just iterate over
+ * all the active CRTCs, try to find a suitable FIFO, and then remove it
+ * from the available FIFOs pool. However, there's a few corner cases
+ * that need to be considered:
+ *
+ * - When running in a dual-display setup (so with two CRTCs involved),
+ * we can update the state of a single CRTC (for example by changing
+ * its mode using xrandr under X11) without affecting the other. In
+ * this case, the other CRTC wouldn't be in the state at all, so we
+ * need to consider all the running CRTCs in the DRM device to assign
+ * a FIFO, not just the one in the state.
+ *
+ * - Since we need the pixelvalve to be disabled and enabled back when
+ * the FIFO is changed, we should keep the FIFO assigned for as long
+ * as the CRTC is enabled, only considering it free again once that
+ * CRTC has been disabled. This can be tested by booting X11 on a
+ * single display, and changing the resolution down and then back up.
+ */
static int vc4_pv_muxing_atomic_check(struct drm_device *dev,
struct drm_atomic_state *state)
{
--
2.28.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/omap: hdmi4: fix reference leak in hdmi_runtime_get
pm_runtime_get_sync will increment pm usage counter even it
failed. Forgetting to pm_runtime_put_noidle will result in
reference leak in hdmi_runtime_get, so we should fix it.
Fixes: ac7674567c620 ("drm: omapdrm: hdmi4: Allocate the omap_hdmi data
structure dynamically")
Signed-off-by: Zhang Qilong
---
drivers/gpu/drm/omapdrm/dss/hdmi4.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
index a14fbf06cb30..33f12c351b08 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
@@ -44,8 +44,10 @@ static int hdmi_runtime_get(struct omap_hdmi *hdmi)
r = pm_runtime_get_sync(&hdmi->pdev->dev);
WARN_ON(r < 0);
- if (r < 0)
+ if (r < 0) {
+ pm_runtime_put_noidle(&hdmi->pdev->dev);
return r;
+ }
return 0;
}
--
2.25.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/stm: Enable RPM during fbdev registration
On 11/5/20 10:39 AM, Daniel Vetter wrote:
On Wed, Nov 04, 2020 at 01:52:00PM +0100, Marek Vasut wrote:
Enable runtime PM before registering the fbdev emulation and disable it
afterward, otherwise register access to the LTDC IP during the fbdev
emulation registration might hang the system.
The problem happens because RPM is activated at the end of ltdc_load(),
but the fbdev emulation registration happens only after that, and ends
up calling ltdc_crtc_mode_set_nofb(), which checks whether RPM is active
and only if it is not active, calls pm_runtime_get_sync() to enable the
clock and so on. If the clock are not enabled, any register access in
ltdc_crtc_mode_set_nofb() could hang the platform completely.
This patch makes sure that ltdc_crtc_mode_set_nofb() is called within
pm_runtime_get_sync(), so with clock enabled.
[...]
This looks like you're papering over a bug in your modeset code. If
userspace later on does a setpar on the fbdev chardev, the exact same
thing could happen. You need to fix your modeset code to avoid this, not
sprinkle temporary rpm_get/put all over some top level entry points,
because you can't even patch those all.
I have a feeling all those pm_runtime_active() checks in the driver
might be the root cause of this ? I wonder why the code doesn't use
pm_runtime_{get,put}_sync() only when accessing registers. Thoughts?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vc4: replace idr_init() by idr_init_base()
idr_init() uses base 0 which is an invalid identifier for this driver.
The idr_alloc for this driver uses VC4_PERFMONID_MIN as start value for
ID range and it is #defined to 1. The new function idr_init_base allows
IDR to set the ID lookup from base 1. This avoids all lookups that
otherwise starts from 0 since 0 is always unused / available.
References: commit 6ce711f27500 ("idr: Make 1-based IDRs more efficient")
Signed-off-by: Deepak R Varma
---
drivers/gpu/drm/vc4/vc4_perfmon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_perfmon.c
b/drivers/gpu/drm/vc4/vc4_perfmon.c
index f4aa75efd16b..7d40f421d922 100644
--- a/drivers/gpu/drm/vc4/vc4_perfmon.c
+++ b/drivers/gpu/drm/vc4/vc4_perfmon.c
@@ -77,7 +77,7 @@ struct vc4_perfmon *vc4_perfmon_find(struct vc4_file
*vc4file, int id)
void vc4_perfmon_open_file(struct vc4_file *vc4file)
{
mutex_init(&vc4file->perfmon.lock);
- idr_init(&vc4file->perfmon.idr);
+ idr_init_base(&vc4file->perfmon.idr, 1);
}
static int vc4_perfmon_idr_del(int id, void *elem, void *data)
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 00/30] Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs
05.11.2020 12:45, Ulf Hansson пишет: ... > I need some more time to review this, but just a quick check found a > few potential issues... Thank you for starting the review! I'm pretty sure it will take a couple revisions until all the questions will be resolved :) > The "core-supply", that you specify as a regulator for each > controller's device node, is not the way we describe power domains. > Instead, it seems like you should register a power-domain provider > (with the help of genpd) and implement the ->set_performance_state() > callback for it. Each device node should then be hooked up to this > power-domain, rather than to a "core-supply". For DT bindings, please > have a look at Documentation/devicetree/bindings/power/power-domain.yaml > and Documentation/devicetree/bindings/power/power_domain.txt. > > In regards to the "sync state" problem (preventing to change > performance states until all consumers have been attached), this can > then be managed by the genpd provider driver instead. I'll need to take a closer look at GENPD, thank you for the suggestion. Sounds like a software GENPD driver which manages clocks and voltages could be a good idea, but it also could be an unnecessary over-engineering. Let's see.. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 00/56] Convert DSI code to use drm_mipi_dsi and drm_panel
Hi Tomi, > Am 05.11.2020 um 13:02 schrieb Tomi Valkeinen : > > Hi, > > This is third version of the series sent by Sebastian in February: > > https://www.spinics.net/lists/linux-omap/msg153465.html > > I took the patches from his git tree, and rebased on 5.10-rc2. There > were some conflicts and compilation errors, and one bug that made dsi to > not work (videomode variable was not initialized to 0). > > I then fixed the few checkpatch and sparse issues. Overall, Sebastian's > patches are pretty much as they were previously. I did drop Laurent's > reviewed-bys, as it's been a long time since the previous series, and > the patches are not identical anyway. > > The topmost 5 patches are new ones, cleanups enabled by the DSI > conversion. They could be handled separately, but it's such a nice > cleanup, and I've been waiting for years to get this done, so here they > are. That said, there are still a _lot_ of cleanups to do. > > Almost all of the patches are omapdrm changes. The two non-omapdrm > changes are: > - After converting panel-dsi-cm to common DRM panel model, it is moved > to drm's panel directory. > - Add MIPI_DSI_MODE_ULPS_IDLE flag > > I have tested these with OMAP4 SDP, AM5 EVM and OMAP4 Panda. SDP has > command mode panel, and I don't have any videomode panels. > > Sebastian, I hope you're ok with all this? I did send you an email, but > didn't get a reply yet, so I thought to just proceed. If you want to > handle this in some other way, or don't want your > authorship/signed-off-by in some of the commits, just tell. That all is great. I was able to apply the patch set cleanly and compile. Next, I migrated my long waiting mipi_dsi/drm_panel driver conversion for the panel of the Pyra handheld (omap 5 based) to compile on 5.10-rc2. And I followed the latest existing panel-orisetech-otm8009a.c which uses a similar video mode controller and mipi-dsi. That one seems to be used by arch/arm/boot/dts/stm32f469-disco.dts. Unfortunately my panel driver is not even loaded by drm/omap so I can't debug. Does this set of drm/omap drivers need a modification of the device tree? If yes, which one? BR and thanks, Nikolaus ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 17/30] mmc: sdhci-tegra: Support OPP and core voltage scaling
On 05-11-20, 17:18, Dmitry Osipenko wrote:
> 05.11.2020 12:58, Viresh Kumar пишет:
> >> +static void sdhci_tegra_deinit_opp_table(void *data)
> >> +{
> >> + struct device *dev = data;
> >> + struct opp_table *opp_table;
> >> +
> >> + opp_table = dev_pm_opp_get_opp_table(dev);
> > So you need to get an OPP table to put one :)
> > You need to save the pointer returned by dev_pm_opp_set_regulators()
> > instead.
>
> This is intentional because why do we need to save the pointer if we're
> not using it and we know that we could get this pointer using OPP API?
Because it is highly inefficient and it doesn't follow the rules set
by the OPP core. Hypothetically speaking, the OPP core is free to
allocate the OPP table structure as much as it wants, and if you don't
use the value returned back to you earlier (think of it as a cookie
assigned to your driver), then it will eventually lead to memory leak.
> This is exactly the same what I did for the CPUFreq driver [1] :)
I will strongly suggest you to save the pointer here and do the same
in the cpufreq driver as well.
> >> +static int devm_sdhci_tegra_init_opp_table(struct device *dev)
> >> +{
> >> + struct opp_table *opp_table;
> >> + const char *rname = "core";
> >> + int err;
> >> +
> >> + /* voltage scaling is optional */
> >> + if (device_property_present(dev, "core-supply"))
> >> + opp_table = dev_pm_opp_set_regulators(dev, &rname, 1);
> >> + else
> >
> >> + opp_table = dev_pm_opp_get_opp_table(dev);
To make it further clear, this will end up allocating an OPP table for
you, which it shouldn't have.
> > Nice. I didn't think that someone will end up abusing this API and so made
> > it
> > available for all, but someone just did that. I will fix that in the OPP
> > core.
To be fair, I allowed the cpufreq-dt driver to abuse it too, which I
am going to fix shortly.
> The dev_pm_opp_put_regulators() handles the case where regulator is
> missing by acting as dev_pm_opp_get_opp_table(), but the
> dev_pm_opp_set_regulators() doesn't do it. Hence I don't think this is
> an abuse, but the OPP API drawback.
I am not sure what you meant here. Normally you are required to call
dev_pm_opp_put_regulators() only if you have called
dev_pm_opp_set_regulators() earlier. And the refcount stays in
balance.
> > Any idea why you are doing what you are doing here ?
>
> Two reasons:
>
> 1. Voltage regulator is optional, but dev_pm_opp_set_regulators()
> doesn't support optional regulators.
>
> 2. We need to balance the opp_table refcount in order to use OPP API
> without polluting code with if(have_regulator), hence the
> dev_pm_opp_get_opp_table() is needed for taking the opp_table reference
> to have the same refcount as in the case of the dev_pm_opp_set_regulators().
I am going to send a patchset shortly after which this call to
dev_pm_opp_get_opp_table() will fail, if it is called before adding
the OPP table.
> I guess we could make dev_pm_opp_set_regulators(dev, count) to accept
> regulators count=0 and then act as dev_pm_opp_get_opp_table(dev), will
> it be acceptable?
Setting regulators for count as 0 doesn't sound good to me.
But, I understand that you don't want to have that if (have_regulator)
check, and it is a fair request. What I will instead do is, allow all
dev_pm_opp_put*() API to start accepting a NULL pointer for the OPP
table and fail silently. And so you won't be required to have this
unwanted check. But you will be required to save the pointer returned
back by dev_pm_opp_set_regulators(), which is the right thing to do
anyways.
--
viresh
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/7] drm/vc4: kms: Switch to drmm_add_action_or_reset
Even though it was pointed in the review by Daniel, and I thought to have
fixed it while applying the patches, but it turns out I forgot to commit
the fixes in the process. Properly fix it this time.
Fixes: dcda7c28bff2 ("drm/vc4: kms: Add functions to create the state objects")
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_kms.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 2b951cae04ad..44db31e16e91 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -113,7 +113,7 @@ static int vc4_ctm_obj_init(struct vc4_dev *vc4)
drm_atomic_private_obj_init(&vc4->base, &vc4->ctm_manager,
&ctm_state->base,
&vc4_ctm_state_funcs);
- return drmm_add_action(&vc4->base, vc4_ctm_obj_fini, NULL);
+ return drmm_add_action_or_reset(&vc4->base, vc4_ctm_obj_fini, NULL);
}
/* Converts a DRM S31.32 value to the HW S0.9 format. */
@@ -657,7 +657,7 @@ static int vc4_load_tracker_obj_init(struct vc4_dev *vc4)
&load_state->base,
&vc4_load_tracker_state_funcs);
- return drmm_add_action(&vc4->base, vc4_load_tracker_obj_fini, NULL);
+ return drmm_add_action_or_reset(&vc4->base, vc4_load_tracker_obj_fini,
NULL);
}
#define NUM_OUTPUTS 6
--
2.28.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: remove pgprot_decrypted() before calls to io_remap_pfn_range()
commit f8f6ae5d077a ("mm: always have io_remap_pfn_range() set
pgprot_decrypted()") moves the pgprot_decrypted() into
io_remap_pfn_range(). Delete any, now confusing, open coded calls that
directly precede io_remap_pfn_range():
- drm_io_prot() is only in drm_mmap_locked() to call io_remap_pfn_range()
- fb_mmap() immediately calls vm_iomap_memory() which is a convenience
wrapper for io_remap_pfn_range()
Signed-off-by: Jason Gunthorpe
---
drivers/gpu/drm/drm_vm.c | 3 ---
drivers/video/fbdev/core/fbmem.c | 5 -
2 files changed, 8 deletions(-)
rc3 will have the dependent patch, this should not be merged to DRM until it
has the rc3 commits.
There are three other pgprot_decrypted() calls in DRM, I could not figure out
what was what there, but other than very special cases I would expect code to
use io_remap_pfn_range() instead.
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index 1a636963378947..6d5a03b3223800 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -70,9 +70,6 @@ static pgprot_t drm_io_prot(struct drm_local_map *map,
{
pgprot_t tmp = vm_get_page_prot(vma->vm_flags);
- /* We don't want graphics memory to be mapped encrypted */
- tmp = pgprot_decrypted(tmp);
-
#if defined(__i386__) || defined(__x86_64__) || defined(__powerpc__) || \
defined(__mips__)
if (map->type == _DRM_REGISTERS && !(map->flags & _DRM_WRITE_COMBINING))
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 8268bbee8cae11..63a27a67a05cfa 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1386,11 +1386,6 @@ fb_mmap(struct file *file, struct vm_area_struct * vma)
mutex_unlock(&info->mm_lock);
vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
- /*
-* The framebuffer needs to be accessed decrypted, be sure
-* SME protection is removed
-*/
- vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
fb_pgprotect(file, vma, start);
return vm_iomap_memory(vma, start, len);
--
2.29.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/2] drm/omap: hdmi: fix reference leak in error handling
pm_runtime_get_sync will increment pm usage counter even it failed. Forgetting to pm_runtime_put_noidle will result in reference leak. This two patches try to fix them in hdmi_runtime_get. Zhang Qilong (2): drm/omap: hdmi4: fix reference leak in hdmi_runtime_get drm/omap: hdmi5: fix reference leak in hdmi_runtime_get drivers/gpu/drm/omapdrm/dss/hdmi4.c | 4 +++- drivers/gpu/drm/omapdrm/dss/hdmi5.c | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) -- 2.25.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 00/30] Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs
On Thu, 5 Nov 2020 at 11:06, Viresh Kumar wrote: > > On 05-11-20, 10:45, Ulf Hansson wrote: > > + Viresh > > Thanks Ulf. I found a bug in OPP core because you cc'd me here :) Happy to help. :-) > > > On Thu, 5 Nov 2020 at 00:44, Dmitry Osipenko wrote: > > I need some more time to review this, but just a quick check found a > > few potential issues... > > > > The "core-supply", that you specify as a regulator for each > > controller's device node, is not the way we describe power domains. > > Maybe I misunderstood your comment here, but there are two ways of > scaling the voltage of a device depending on if it is a regulator (and > can be modeled as one in the kernel) or a power domain. I am not objecting about scaling the voltage through a regulator, that's fine to me. However, encoding a power domain as a regulator (even if it may seem like a regulator) isn't. Well, unless Mark Brown has changed his mind about this. In this case, it seems like the regulator supply belongs in the description of the power domain provider. > > In case of Qcom earlier (when we added the performance-state stuff), > the eventual hardware was out of kernel's control and we didn't wanted > (allowed) to model it as a virtual regulator just to pass the votes to > the RPM. And so we did what we did. > > But if the hardware (where the voltage is required to be changed) is > indeed a regulator and is modeled as one, then what Dmitry has done > looks okay. i.e. add a supply in the device's node and microvolt > property in the DT entries. I guess I haven't paid enough attention how power domain regulators are being described then. I was under the impression that the CPUfreq case was a bit specific - and we had legacy bindings to stick with. Can you point me to some other existing examples of where power domain regulators are specified as a regulator in each device's node? Kind regards Uffe ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/qxl: replace idr_init() by idr_init_base()
idr_init() uses base 0 which is an invalid identifier for this driver.
The idr_alloc for this driver uses 1 as start value for ID range. The
new function idr_init_base allows IDR to set the ID lookup from base 1.
This avoids all lookups that otherwise starts from 0 since 0 is always
unused / available.
References: commit 6ce711f27500 ("idr: Make 1-based IDRs more efficient")
Signed-off-by: Deepak R Varma
---
drivers/gpu/drm/qxl/qxl_kms.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index dc5b3850a4d4..228e2b9198f1 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -231,11 +231,11 @@ int qxl_device_init(struct qxl_device *qdev,
goto cursor_ring_free;
}
- idr_init(&qdev->release_idr);
+ idr_init_base(&qdev->release_idr, 1);
spin_lock_init(&qdev->release_idr_lock);
spin_lock_init(&qdev->release_lock);
- idr_init(&qdev->surf_id_idr);
+ idr_init_base(&qdev->surf_id_idr, 1);
spin_lock_init(&qdev->surf_id_idr_lock);
mutex_init(&qdev->async_io_mutex);
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 17/30] mmc: sdhci-tegra: Support OPP and core voltage scaling
On Thu, Nov 5, 2020 at 5:15 AM Dmitry Osipenko wrote:
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> +static void sdhci_tegra_deinit_opp_table(void *data)
> +{
> + struct device *dev = data;
> + struct opp_table *opp_table;
> +
> + opp_table = dev_pm_opp_get_opp_table(dev);
So you need to get an OPP table to put one :)
You need to save the pointer returned by dev_pm_opp_set_regulators() instead.
> + dev_pm_opp_of_remove_table(dev);
> + dev_pm_opp_put_regulators(opp_table);
> + dev_pm_opp_put_opp_table(opp_table);
> +}
> +
> +static int devm_sdhci_tegra_init_opp_table(struct device *dev)
> +{
> + struct opp_table *opp_table;
> + const char *rname = "core";
> + int err;
> +
> + /* voltage scaling is optional */
> + if (device_property_present(dev, "core-supply"))
> + opp_table = dev_pm_opp_set_regulators(dev, &rname, 1);
> + else
> + opp_table = dev_pm_opp_get_opp_table(dev);
Nice. I didn't think that someone will end up abusing this API and so made it
available for all, but someone just did that. I will fix that in the OPP core.
Any idea why you are doing what you are doing here ?
> +
> + if (IS_ERR(opp_table))
> + return dev_err_probe(dev, PTR_ERR(opp_table),
> + "failed to prepare OPP table\n");
> +
> + /*
> +* OPP table presence is optional and we want the set_rate() of OPP
> +* API to work similarly to clk_set_rate() if table is missing in a
> +* device-tree. The add_table() errors out if OPP is missing in DT.
> +*/
> + if (device_property_present(dev, "operating-points-v2")) {
> + err = dev_pm_opp_of_add_table(dev);
> + if (err) {
> + dev_err(dev, "failed to add OPP table: %d\n", err);
> + goto put_table;
> + }
> + }
> +
> + err = devm_add_action(dev, sdhci_tegra_deinit_opp_table, dev);
> + if (err)
> + goto remove_table;
> +
> + return 0;
> +
> +remove_table:
> + dev_pm_opp_of_remove_table(dev);
> +put_table:
> + dev_pm_opp_put_regulators(opp_table);
> +
> + return err;
> +}
> +
> static int sdhci_tegra_probe(struct platform_device *pdev)
> {
> const struct of_device_id *match;
> @@ -1621,6 +1681,10 @@ static int sdhci_tegra_probe(struct platform_device
> *pdev)
> goto err_power_req;
> }
>
> + rc = devm_sdhci_tegra_init_opp_table(&pdev->dev);
> + if (rc)
> + goto err_parse_dt;
> +
> /*
> * Tegra210 has a separate SDMMC_LEGACY_TM clock used for host
> * timeout clock and SW can choose TMCLK or SDCLK for hardware
> --
> 2.27.0
>
> ___
> devel mailing list
> de...@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 00/30] Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs
+ Viresh On Thu, 5 Nov 2020 at 00:44, Dmitry Osipenko wrote: > > Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs, which reduces > power consumption and heating of the Tegra chips. Tegra SoC has multiple > hardware units which belong to a core power domain of the SoC and share > the core voltage. The voltage must be selected in accordance to a minimum > requirement of every core hardware unit. > > The minimum core voltage requirement depends on: > > 1. Clock enable state of a hardware unit. > 2. Clock frequency. > 3. Unit's internal idling/active state. > > This series is tested on Acer A500 (T20), AC100 (T20), Nexus 7 (T30) and > Ouya (T30) devices. I also added voltage scaling to the Ventana (T20) and > Cardhu (T30) boards which are tested by NVIDIA's CI farm. Tegra30 is now up > to 5C cooler on Nexus 7 and stays cool on Ouya (instead of becoming burning > hot) while system is idling. It should be possible to improve this further > by implementing a more advanced power management features for the kernel > drivers. > > The DVFS support is opt-in for all boards, meaning that older DTBs will > continue to work like they did it before this series. It should be possible > to easily add the core voltage scaling support for Tegra114+ SoCs based on > this grounding work later on, if anyone will want to implement it. > > WARNING(!) This series is made on top of the memory interconnect patches >which are currently under review [1]. The Tegra EMC driver >and devicetree-related patches need to be applied on top of >the ICC series. > > [1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=212196 > > Dmitry Osipenko (30): > dt-bindings: host1x: Document OPP and voltage regulator properties > dt-bindings: mmc: tegra: Document OPP and voltage regulator properties > dt-bindings: pwm: tegra: Document OPP and voltage regulator properties > media: dt: bindings: tegra-vde: Document OPP and voltage regulator > properties > dt-binding: usb: ci-hdrc-usb2: Document OPP and voltage regulator > properties > dt-bindings: usb: tegra-ehci: Document OPP and voltage regulator > properties > soc/tegra: Add sync state API > soc/tegra: regulators: Support Tegra SoC device sync state API > soc/tegra: regulators: Fix lockup when voltage-spread is out of range > regulator: Allow skipping disabled regulators in > regulator_check_consumers() > drm/tegra: dc: Support OPP and SoC core voltage scaling > drm/tegra: gr2d: Correct swapped device-tree compatibles > drm/tegra: gr2d: Support OPP and SoC core voltage scaling > drm/tegra: gr3d: Support OPP and SoC core voltage scaling > drm/tegra: hdmi: Support OPP and SoC core voltage scaling > gpu: host1x: Support OPP and SoC core voltage scaling > mmc: sdhci-tegra: Support OPP and core voltage scaling > pwm: tegra: Support OPP and core voltage scaling > media: staging: tegra-vde: Support OPP and SoC core voltage scaling > usb: chipidea: tegra: Support OPP and SoC core voltage scaling > usb: host: ehci-tegra: Support OPP and SoC core voltage scaling > memory: tegra20-emc: Support Tegra SoC device state syncing > memory: tegra30-emc: Support Tegra SoC device state syncing > ARM: tegra: Add OPP tables for Tegra20 peripheral devices > ARM: tegra: Add OPP tables for Tegra30 peripheral devices > ARM: tegra: ventana: Add voltage supplies to DVFS-capable devices > ARM: tegra: paz00: Add voltage supplies to DVFS-capable devices > ARM: tegra: acer-a500: Add voltage supplies to DVFS-capable devices > ARM: tegra: cardhu-a04: Add voltage supplies to DVFS-capable devices > ARM: tegra: nexus7: Add voltage supplies to DVFS-capable devices > > .../display/tegra/nvidia,tegra20-host1x.txt | 56 +++ > .../bindings/media/nvidia,tegra-vde.txt | 12 + > .../bindings/mmc/nvidia,tegra20-sdhci.txt | 12 + > .../bindings/pwm/nvidia,tegra20-pwm.txt | 13 + > .../devicetree/bindings/usb/ci-hdrc-usb2.txt | 4 + > .../bindings/usb/nvidia,tegra20-ehci.txt | 2 + > .../boot/dts/tegra20-acer-a500-picasso.dts| 30 +- > arch/arm/boot/dts/tegra20-paz00.dts | 40 +- > .../arm/boot/dts/tegra20-peripherals-opp.dtsi | 386 > arch/arm/boot/dts/tegra20-ventana.dts | 65 ++- > arch/arm/boot/dts/tegra20.dtsi| 14 + > .../tegra30-asus-nexus7-grouper-common.dtsi | 23 + > arch/arm/boot/dts/tegra30-cardhu-a04.dts | 44 ++ > .../arm/boot/dts/tegra30-peripherals-opp.dtsi | 415 ++ > arch/arm/boot/dts/tegra30.dtsi| 13 + > drivers/gpu/drm/tegra/Kconfig | 1 + > drivers/gpu/drm/tegra/dc.c| 138 +- > drivers/gpu/drm/tegra/dc.h| 5 + > drivers/gpu/drm/tegra/gr2d.c | 140 +- > drivers/gpu/drm/tegra/gr3d.c | 136 ++ > drivers/gpu/drm/tegra/hdmi.c | 63 ++- > drive
Re: [PATCH v1 17/30] mmc: sdhci-tegra: Support OPP and core voltage scaling
05.11.2020 12:58, Viresh Kumar пишет:
>> +static void sdhci_tegra_deinit_opp_table(void *data)
>> +{
>> + struct device *dev = data;
>> + struct opp_table *opp_table;
>> +
>> + opp_table = dev_pm_opp_get_opp_table(dev);
> So you need to get an OPP table to put one :)
> You need to save the pointer returned by dev_pm_opp_set_regulators() instead.
This is intentional because why do we need to save the pointer if we're
not using it and we know that we could get this pointer using OPP API?
This is exactly the same what I did for the CPUFreq driver [1] :)
[1]
https://elixir.bootlin.com/linux/v5.10-rc2/source/drivers/cpufreq/tegra20-cpufreq.c#L97
>> + dev_pm_opp_of_remove_table(dev);
>> + dev_pm_opp_put_regulators(opp_table);
>> + dev_pm_opp_put_opp_table(opp_table);
>> +}
>> +
>> +static int devm_sdhci_tegra_init_opp_table(struct device *dev)
>> +{
>> + struct opp_table *opp_table;
>> + const char *rname = "core";
>> + int err;
>> +
>> + /* voltage scaling is optional */
>> + if (device_property_present(dev, "core-supply"))
>> + opp_table = dev_pm_opp_set_regulators(dev, &rname, 1);
>> + else
>
>> + opp_table = dev_pm_opp_get_opp_table(dev);
> Nice. I didn't think that someone will end up abusing this API and so made it
> available for all, but someone just did that. I will fix that in the OPP core.
The dev_pm_opp_put_regulators() handles the case where regulator is
missing by acting as dev_pm_opp_get_opp_table(), but the
dev_pm_opp_set_regulators() doesn't do it. Hence I don't think this is
an abuse, but the OPP API drawback.
> Any idea why you are doing what you are doing here ?
Two reasons:
1. Voltage regulator is optional, but dev_pm_opp_set_regulators()
doesn't support optional regulators.
2. We need to balance the opp_table refcount in order to use OPP API
without polluting code with if(have_regulator), hence the
dev_pm_opp_get_opp_table() is needed for taking the opp_table reference
to have the same refcount as in the case of the dev_pm_opp_set_regulators().
I guess we could make dev_pm_opp_set_regulators(dev, count) to accept
regulators count=0 and then act as dev_pm_opp_get_opp_table(dev), will
it be acceptable?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 00/30] Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs
On 05-11-20, 11:34, Ulf Hansson wrote: > I am not objecting about scaling the voltage through a regulator, > that's fine to me. However, encoding a power domain as a regulator > (even if it may seem like a regulator) isn't. Well, unless Mark Brown > has changed his mind about this. > > In this case, it seems like the regulator supply belongs in the > description of the power domain provider. Okay, I wasn't sure if it is a power domain or a regulator here. Btw, how do we identify if it is a power domain or a regulator ? > > In case of Qcom earlier (when we added the performance-state stuff), > > the eventual hardware was out of kernel's control and we didn't wanted > > (allowed) to model it as a virtual regulator just to pass the votes to > > the RPM. And so we did what we did. > > > > But if the hardware (where the voltage is required to be changed) is > > indeed a regulator and is modeled as one, then what Dmitry has done > > looks okay. i.e. add a supply in the device's node and microvolt > > property in the DT entries. > > I guess I haven't paid enough attention how power domain regulators > are being described then. I was under the impression that the CPUfreq > case was a bit specific - and we had legacy bindings to stick with. > > Can you point me to some other existing examples of where power domain > regulators are specified as a regulator in each device's node? No, I thought it is a regulator here and not a power domain. -- viresh ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 4/5] RDMA/mlx5: Support dma-buf based userspace memory region
On Thu, Nov 05, 2020 at 12:36:25AM +, Xiong, Jianxin wrote:
> > From: Jason Gunthorpe
> > Sent: Wednesday, November 04, 2020 4:07 PM
> > To: Xiong, Jianxin
> > Cc: linux-r...@vger.kernel.org; dri-devel@lists.freedesktop.org; Doug
> > Ledford ; Leon Romanovsky
> > ; Sumit Semwal ; Christian Koenig
> > ; Vetter, Daniel
> >
> > Subject: Re: [PATCH v7 4/5] RDMA/mlx5: Support dma-buf based userspace
> > memory region
> >
> > On Wed, Nov 04, 2020 at 02:06:34PM -0800, Jianxin Xiong wrote:
> > > + umem = ib_umem_dmabuf_get(&dev->ib_dev, offset, length, fd,
> > > access_flags,
> > > + &mlx5_ib_dmabuf_attach_ops);
> > > + if (IS_ERR(umem)) {
> > > + mlx5_ib_dbg(dev, "umem get failed (%ld)\n", PTR_ERR(umem));
> > > + return ERR_PTR(PTR_ERR(umem));
> > > + }
> > > +
> > > + mr = alloc_mr_from_cache(pd, umem, virt_addr, access_flags);
> >
> > It is very subtle, but this calls mlx5_umem_find_best_pgsz() which calls
> > ib_umem_find_best_pgsz() which goes over the SGL to determine
> > the page size to use.
> >
>
> When this is called here, the umem sglist is still NULL because
> dma_buf_map_attachment()
> is not called until a page fault occurs. In patch 1/5, the function
> ib_umem_find_best_pgsz()
> has been modified to always return PAGE_SIZE for dma-buf based MR.
Oh.. That isn't a good idea.
ib_umem_find_best_pgsz() must be run on any SGL list to validate it
against the constraints, making it un-runable for the dmabuf case
means we can never support large page size or even validate that the
SGL is properly formed.
So I think this need to change the alloc_mr_from_cache() to early exit
for dma_buf ones
And it still need to call ib_umem_find_best_pgsz() but
just check the page size.
> > Edit the last SGE to have a reduced length
>
> Do you still think modifying the SGL in place needed given the above
> explanation? I do see some benefits of doing so -- hiding the
> discrepancy of sgl and addr/length from the device drivers and avoid
> special handling in the code that use the sgl.
Yes, a umem SGL should always be properly formed or I will have a
meltdown trying to keep all the drivers working :\
Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vgm: replace idr_init() by idr_init_base()
On Thu, Nov 05, 2020 at 12:16:34PM +0100, Daniel Vetter wrote:
> On Thu, Nov 5, 2020 at 11:31 AM Deepak R Varma wrote:
> >
> > On Thu, Nov 05, 2020 at 10:42:15AM +0100, Daniel Vetter wrote:
> > > On Wed, Nov 04, 2020 at 04:53:38PM +0530, Deepak R Varma wrote:
> > > > idr_init() uses base 0 which is an invalid identifier. The new function
> > > > idr_init_base allows IDR to set the ID lookup from base 1. This avoids
> > > > all lookups that otherwise starts from 0 since 0 is always unused.
> > > >
> > > > References: commit 6ce711f27500 ("idr: Make 1-based IDRs more
> > > > efficient")
> > > >
> > > > Signed-off-by: Deepak R Varma
> > >
> > > Tiny typo in the commit message summary: s/vgm/vgem/
> > >
> > > Also can you pls resbumit this with intel-gfx mailing list on cc (like for
> > > i915)? There's a CI bot there which runs a few vgem tests, would be good
> > > to confirm nothing has been broken.
> >
> > Hi Daniel,
> > sure. I will correct the summary typo and also feed it to the CI bot.
> >
> > Also, according to Felix Kuehling's comment on a similar patch for
> > drm/amdkfd driver, an ID can be 0. The change I am proposing is more
> > efficient for conditions that do not want to use ID as 0. Otherwise,
> > id = 0 is an acceptable possibility. So, my statement that "Id 0 is an
> > invalid
> > identifier" is not true.
> >
> > Can you please comment if this is accurate and I should reword my log
> > message as well?
>
> You need to review the vgem code to see whether we're using id 0 as
> invalid identifier or not. That's part of the work that needs to be
> done here. Best would be to explain the evidence you've found in the
> commit message, why id 0 is invalid for this specific code. Since yes
> in general that's not true, it depends how the idr is used.
> -Daniel
>
You are correct. For the vgem driver, id 0 is not used. The patch
should then apply to this driver.
Thank you very much Daniel. I have just sent v2 of the patch with your
suggestions.
./drv
> >
> > Thank you.
> > ./drv
> >
> > >
> > > Otherwise lgtm.
> > >
> > > Thanks, Daniel
> > >
> > > > ---
> > > > drivers/gpu/drm/vgem/vgem_fence.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/vgem/vgem_fence.c
> > > > b/drivers/gpu/drm/vgem/vgem_fence.c
> > > > index 17f32f550dd9..2902dc6e64fa 100644
> > > > --- a/drivers/gpu/drm/vgem/vgem_fence.c
> > > > +++ b/drivers/gpu/drm/vgem/vgem_fence.c
> > > > @@ -233,7 +233,7 @@ int vgem_fence_signal_ioctl(struct drm_device *dev,
> > > > int vgem_fence_open(struct vgem_file *vfile)
> > > > {
> > > > mutex_init(&vfile->fence_mutex);
> > > > - idr_init(&vfile->fence_idr);
> > > > + idr_init_base(&vfile->fence_idr, 1);
> > > >
> > > > return 0;
> > > > }
> > > > --
> > > > 2.25.1
> > > >
> > >
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 00/30] Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs
On 05-11-20, 10:45, Ulf Hansson wrote: > + Viresh Thanks Ulf. I found a bug in OPP core because you cc'd me here :) > On Thu, 5 Nov 2020 at 00:44, Dmitry Osipenko wrote: > I need some more time to review this, but just a quick check found a > few potential issues... > > The "core-supply", that you specify as a regulator for each > controller's device node, is not the way we describe power domains. Maybe I misunderstood your comment here, but there are two ways of scaling the voltage of a device depending on if it is a regulator (and can be modeled as one in the kernel) or a power domain. In case of Qcom earlier (when we added the performance-state stuff), the eventual hardware was out of kernel's control and we didn't wanted (allowed) to model it as a virtual regulator just to pass the votes to the RPM. And so we did what we did. But if the hardware (where the voltage is required to be changed) is indeed a regulator and is modeled as one, then what Dmitry has done looks okay. i.e. add a supply in the device's node and microvolt property in the DT entries. -- viresh ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 07/22] drm/msm: Do rpm get sooner in the submit path
On 05-11-20, 11:24, Rob Clark wrote:
> On Tue, Nov 3, 2020 at 7:04 PM Viresh Kumar wrote:
> >
> > On 03-11-20, 08:50, Rob Clark wrote:
> > > sorry, it didn't apply cleanly (which I guess is due to some other
> > > dependencies that need to be picked back to v5.4 product kernel), and
> > > due to some other things I'm in middle of debugging I didn't have time
> > > yet to switch to v5.10-rc or look at what else needs to
> > > cherry-picked..
> > >
> > > If you could, pushing a branch with this patch somewhere would be a
> > > bit easier to work with (ie. fetch && cherry-pick is easier to deal
> > > with than picking things from list)
> >
> > It has been in linux-next for a few days. Here is the HEAD to pick
> > from. There are few patches there since rc1.
> >
> > commit 203e29749cc0 ("opp: Allocate the OPP table outside of
> > opp_table_lock")
> >
>
> sorry for the delay, with that cherry-picked, I'm getting a whole lot of:
Ahh, sorry about that and thanks for reporting it. Here is the fix:
diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index c718092757d9..6b7f0066942d 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -112,8 +112,6 @@ static struct opp_table *_find_table_of_opp_np(struct
device_node *opp_np)
struct opp_table *opp_table;
struct device_node *opp_table_np;
- lockdep_assert_held(&opp_table_lock);
-
opp_table_np = of_get_parent(opp_np);
if (!opp_table_np)
goto err;
@@ -121,12 +119,15 @@ static struct opp_table *_find_table_of_opp_np(struct
device_node *opp_np)
/* It is safe to put the node now as all we need now is its address */
of_node_put(opp_table_np);
+ mutex_lock(&opp_table_lock);
list_for_each_entry(opp_table, &opp_tables, node) {
if (opp_table_np == opp_table->np) {
_get_opp_table_kref(opp_table);
+ mutex_unlock(&opp_table_lock);
return opp_table;
}
}
+ mutex_unlock(&opp_table_lock);
err:
return ERR_PTR(-ENODEV);
--
viresh
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/vgem: replace idr_init() by idr_init_base()
idr_init() uses base 0 which is an invalid identifier. The new function
idr_init_base allows IDR to set the ID lookup from base 1. This avoids
all lookups that otherwise starts from 0 since 0 is always unused.
References: commit 6ce711f27500 ("idr: Make 1-based IDRs more efficient")
Signed-off-by: Deepak R Varma
---
Changes since v1:
- Changes suggested by Daniel Vetter:
1. Correct typo in patch summary
2. cc intel-gfx to get the patch through CI bot test
drivers/gpu/drm/vgem/vgem_fence.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vgem/vgem_fence.c
b/drivers/gpu/drm/vgem/vgem_fence.c
index 17f32f550dd9..2902dc6e64fa 100644
--- a/drivers/gpu/drm/vgem/vgem_fence.c
+++ b/drivers/gpu/drm/vgem/vgem_fence.c
@@ -233,7 +233,7 @@ int vgem_fence_signal_ioctl(struct drm_device *dev,
int vgem_fence_open(struct vgem_file *vfile)
{
mutex_init(&vfile->fence_mutex);
- idr_init(&vfile->fence_idr);
+ idr_init_base(&vfile->fence_idr, 1);
return 0;
}
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 7/7] drm/vc4: kms: Don't disable the muxing of an active CRTC
Hi!
On Mon, Nov 02, 2020 at 05:47:04PM +0900, Hoegeun Kwon wrote:
> Hi Maxime,
>
> Thanks for V2 patch.
>
>
> On 10/28/20 9:41 PM, Maxime Ripard wrote:
> > The current HVS muxing code will consider the CRTCs in a given state to
> > setup their muxing in the HVS, and disable the other CRTCs muxes.
> >
> > However, it's valid to only update a single CRTC with a state, and in this
> > situation we would mux out a CRTC that was enabled but left untouched by
> > the new state.
> >
> > Fix this by setting a flag on the CRTC state when the muxing has been
> > changed, and only change the muxing configuration when that flag is there.
> >
> > Fixes: 87ebcd42fb7b ("drm/vc4: crtc: Assign output to channel
> > automatically")
> > Signed-off-by: Maxime Ripard
> > ---
> > drivers/gpu/drm/vc4/vc4_drv.h | 1 +-
> > drivers/gpu/drm/vc4/vc4_kms.c | 84 +---
> > 2 files changed, 50 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> > index c6208b040f77..c074c0538e57 100644
> > --- a/drivers/gpu/drm/vc4/vc4_drv.h
> > +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> > @@ -523,6 +523,7 @@ struct vc4_crtc_state {
> > struct drm_mm_node mm;
> > bool feed_txp;
> > bool txp_armed;
> > + bool needs_muxing;
> > unsigned int assigned_channel;
> >
> > struct {
> > diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> > index 2aa726b7422c..409aeb19d210 100644
> > --- a/drivers/gpu/drm/vc4/vc4_kms.c
> > +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> > @@ -224,10 +224,7 @@ static void vc5_hvs_pv_muxing_commit(struct vc4_dev
> > *vc4,
> > {
> > struct drm_crtc_state *crtc_state;
> > struct drm_crtc *crtc;
> > - unsigned char dsp2_mux = 0;
> > - unsigned char dsp3_mux = 3;
> > - unsigned char dsp4_mux = 3;
> > - unsigned char dsp5_mux = 3;
> > + unsigned char mux;
> > unsigned int i;
> > u32 reg;
> >
> > @@ -235,50 +232,59 @@ static void vc5_hvs_pv_muxing_commit(struct vc4_dev
> > *vc4,
> > struct vc4_crtc_state *vc4_state =
> > to_vc4_crtc_state(crtc_state);
> > struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
> >
> > - if (!crtc_state->active)
> > + if (!vc4_state->needs_muxing)
> > continue;
> >
> > switch (vc4_crtc->data->hvs_output) {
> > case 2:
> > - dsp2_mux = (vc4_state->assigned_channel == 2) ? 0 : 1;
> > + mux = (vc4_state->assigned_channel == 2) ? 0 : 1;
> > + reg = HVS_READ(SCALER_DISPECTRL);
> > + HVS_WRITE(SCALER_DISPECTRL,
> > + (reg & ~SCALER_DISPECTRL_DSP2_MUX_MASK) |
> > + VC4_SET_FIELD(mux,
> > SCALER_DISPECTRL_DSP2_MUX));
> > break;
> >
> > case 3:
> > - dsp3_mux = vc4_state->assigned_channel;
> > + if (vc4_state->assigned_channel ==
> > VC4_HVS_CHANNEL_DISABLED)
> > + mux = 3;
> > + else
> > + mux = vc4_state->assigned_channel;
> > +
> > + reg = HVS_READ(SCALER_DISPCTRL);
> > + HVS_WRITE(SCALER_DISPCTRL,
> > + (reg & ~SCALER_DISPCTRL_DSP3_MUX_MASK) |
> > + VC4_SET_FIELD(mux, SCALER_DISPCTRL_DSP3_MUX));
> > break;
> >
> > case 4:
> > - dsp4_mux = vc4_state->assigned_channel;
> > + if (vc4_state->assigned_channel ==
> > VC4_HVS_CHANNEL_DISABLED)
> > + mux = 3;
> > + else
> > + mux = vc4_state->assigned_channel;
> > +
> > + reg = HVS_READ(SCALER_DISPEOLN);
> > + HVS_WRITE(SCALER_DISPEOLN,
> > + (reg & ~SCALER_DISPEOLN_DSP4_MUX_MASK) |
> > + VC4_SET_FIELD(mux, SCALER_DISPEOLN_DSP4_MUX));
> > +
> > break;
> >
> > case 5:
> > - dsp5_mux = vc4_state->assigned_channel;
> > + if (vc4_state->assigned_channel ==
> > VC4_HVS_CHANNEL_DISABLED)
> > + mux = 3;
> > + else
> > + mux = vc4_state->assigned_channel;
> > +
> > + reg = HVS_READ(SCALER_DISPDITHER);
> > + HVS_WRITE(SCALER_DISPDITHER,
> > + (reg & ~SCALER_DISPDITHER_DSP5_MUX_MASK) |
> > + VC4_SET_FIELD(mux,
> > SCALER_DISPDITHER_DSP5_MUX));
> > break;
> >
> > default:
> > break;
> > }
> > }
> > -
> > - reg = HVS_READ(SCALER_DISPECTRL);
> > - HVS_WRITE(SCALER_DISPECTRL,
> > - (reg & ~SCALER_DISPECTRL_DSP2_MUX_MASK) |
>
[PATCH 0/7] opp: Allow dev_pm_opp_put_*() APIs to accept NULL opp_table
Hello, This patchset updates the dev_pm_opp_put_*() helpers to accept a NULL pointer for the OPP table, in order to allow the callers to drop the unnecessary checks they had to carry. All these must get merged upstream through the OPP tree as there is a hard dependency on the first patch here. Thanks. Viresh Kumar (7): opp: Allow dev_pm_opp_put_*() APIs to accept NULL opp_table cpufreq: dt: dev_pm_opp_put_regulators() accepts NULL argument cpufreq: qcom-cpufreq-nvmem: dev_pm_opp_put_*() accepts NULL argument devfreq: exynos: dev_pm_opp_put_*() accepts NULL argument drm/lima: dev_pm_opp_put_*() accepts NULL argument drm/panfrost: dev_pm_opp_put_*() accepts NULL argument media: venus: dev_pm_opp_put_*() accepts NULL argument drivers/cpufreq/cpufreq-dt.c | 6 ++ drivers/cpufreq/qcom-cpufreq-nvmem.c | 15 ++- drivers/devfreq/exynos-bus.c | 12 drivers/gpu/drm/lima/lima_devfreq.c| 13 - drivers/gpu/drm/panfrost/panfrost_devfreq.c| 6 ++ drivers/media/platform/qcom/venus/pm_helpers.c | 3 +-- drivers/opp/core.c | 18 ++ 7 files changed, 37 insertions(+), 36 deletions(-) -- 2.25.0.rc1.19.g042ed3e048af ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 00/56] Convert DSI code to use drm_mipi_dsi and drm_panel
> Am 05.11.2020 um 18:36 schrieb Tomi Valkeinen : > > Hi, > > On 05/11/2020 19:15, H. Nikolaus Schaller wrote: > >> Next, I migrated my long waiting mipi_dsi/drm_panel driver conversion for >> the panel of the Pyra handheld (omap 5 based) to compile on 5.10-rc2. And >> I followed the latest existing panel-orisetech-otm8009a.c which uses a >> similar video mode controller and mipi-dsi. >> >> That one seems to be used by arch/arm/boot/dts/stm32f469-disco.dts. >> >> Unfortunately my panel driver is not even loaded by drm/omap so I can't >> debug. Does this set of drm/omap drivers need a modification of the device >> tree? If yes, which one? > > omapdrm doesn't load the panel drivers. If not even your panel's probe is > called, then it hints at > DT and/or driver's compatible string issue. The panel's probe should get > called even if omapdrm is > not loaded at all. Well, I use the same device tree that loads the old driver... > > Can you push your branch somewhere, so I can have a quick look? Yes, that would be nice! Here: https://git.goldelico.com/?p=letux-kernel.git;a=shortlog;h=refs/heads/letux-pyra-dsi-5.10-rc2 or: https://github.com/goldelico/letux-kernel/tree/letux-pyra-dsi-5.10-rc2 (full config is not included). BR and thanks, Nikolaus ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: Use IS_ERR() instead of IS_ERR_OR_NULL()
dev_pm_opp_find_freq_exact never return null but ERR_PTR(),so we should use IS_ERR() to clarify and avoid compilation warnings. Signed-off-by: Wang Qing --- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index 8c81a89..6247397 100755 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -851,7 +851,7 @@ static void a6xx_gmu_set_initial_freq(struct msm_gpu *gpu, struct a6xx_gmu *gmu) unsigned long gpu_freq = gmu->gpu_freqs[gmu->current_perf_index]; gpu_opp = dev_pm_opp_find_freq_exact(&gpu->pdev->dev, gpu_freq, true); - if (IS_ERR_OR_NULL(gpu_opp)) + if (IS_ERR(gpu_opp)) return; gmu->freq = 0; /* so a6xx_gmu_set_freq() doesn't exit early */ @@ -865,7 +865,7 @@ static void a6xx_gmu_set_initial_bw(struct msm_gpu *gpu, struct a6xx_gmu *gmu) unsigned long gpu_freq = gmu->gpu_freqs[gmu->current_perf_index]; gpu_opp = dev_pm_opp_find_freq_exact(&gpu->pdev->dev, gpu_freq, true); - if (IS_ERR_OR_NULL(gpu_opp)) + if (IS_ERR(gpu_opp)) return; dev_pm_opp_set_bw(&gpu->pdev->dev, gpu_opp); -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/omap: hdmi5: fix reference leak in hdmi_runtime_get
pm_runtime_get_sync will increment pm usage counter even it
failed. Forgetting to pm_runtime_put_noidle will result in
reference leak in hdmi_runtime_get, so we should fix it.
Fixes: c44991ce21bef ("drm: omapdrm: hdmi5: Allocate the omap_hdmi data
structure dynamically")
Signed-off-by: Zhang Qilong
---
drivers/gpu/drm/omapdrm/dss/hdmi5.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5.c
b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
index b738d9750686..26ffbd1bd1cc 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi5.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
@@ -45,8 +45,10 @@ static int hdmi_runtime_get(struct omap_hdmi *hdmi)
r = pm_runtime_get_sync(&hdmi->pdev->dev);
WARN_ON(r < 0);
- if (r < 0)
+ if (r < 0) {
+ pm_runtime_put_noidle(&hdmi->pdev->dev);
return r;
+ }
return 0;
}
--
2.25.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v8 1/5] RDMA/umem: Support importing dma-buf as user memory region
On Thu, Nov 05, 2020 at 02:48:05PM -0800, Jianxin Xiong wrote:
> + /* modify the sgl in-place to match umem address and length */
> +
> + start = ALIGN_DOWN(umem_dmabuf->umem.address, PAGE_SIZE);
> + end = ALIGN(umem_dmabuf->umem.address + umem_dmabuf->umem.length,
> + PAGE_SIZE);
> + cur = 0;
> + nmap = 0;
> + for_each_sgtable_dma_sg(sgt, sg, i) {
> + if (cur >= end)
> + break;
> + if (cur + sg_dma_len(sg) <= start) {
> + cur += sg_dma_len(sg);
> + continue;
> + }
This seems like a strange way to compute interesections
if (cur <= start && start < cur + sg_dma_len(sg))
> + if (cur <= start) {
> + unsigned long offset = start - cur;
> +
> + umem_dmabuf->first_sg = sg;
> + umem_dmabuf->first_sg_offset = offset;
> + sg_dma_address(sg) += offset;
> + sg_dma_len(sg) -= offset;
> + if (&sg_dma_len(sg) != &sg->length)
> + sg->length -= offset;
We don't need to adjust sg->length, only dma_len, so no reason for
this surprising if.
> + cur += offset;
> + }
> + if (cur + sg_dma_len(sg) >= end) {
Same logic here
> + unsigned long trim = cur + sg_dma_len(sg) - end;
> +
> + umem_dmabuf->last_sg = sg;
> + umem_dmabuf->last_sg_trim = trim;
> + sg_dma_len(sg) -= trim;
> + if (&sg_dma_len(sg) != &sg->length)
> + sg->length -= trim;
break, things are done here
> + }
> + cur += sg_dma_len(sg);
> + nmap++;
> + }
> +
> + umem_dmabuf->umem.sg_head.sgl = umem_dmabuf->first_sg;
> + umem_dmabuf->umem.sg_head.nents = nmap;
> + umem_dmabuf->umem.nmap = nmap;
> + umem_dmabuf->sgt = sgt;
> +
> + page_size = ib_umem_find_best_pgsz(&umem_dmabuf->umem, PAGE_SIZE,
> +umem_dmabuf->umem.iova);
> +
> + if (WARN_ON(cur != end || page_size != PAGE_SIZE)) {
Looks like nothing prevents this warn on to tigger
The user could specify a length that is beyond
the dma buf, can the dma buf length be checked during get?
Also page_size can be 0 because iova is not OK. iova should be checked
for alignment during get as well:
iova & (PAGE_SIZE-1) == umem->addr & (PAGE_SIZE-1)
But yes, this is the right idea
Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 1/4] RDMA/umem: Support importing dma-buf as user memory region
On Tue, Nov 03, 2020 at 09:43:17PM +0100, Daniel Vetter wrote: > > > Yeah definitely don't call dma_buf_map_attachment and expect a page back. > > > In practice you'll get a page back fairly often, but I don't think > > > we want to bake that in, maybe we eventually get to non-hacky dma_addr_t > > > only sgl. > > > > > > What I'm wondering is whether dma_buf_attach shouldn't reject such > > > devices directly, instead of each importer having to do that. > > > > Come back here to see if consensus can be reached on who should do the > > check. My > > thinking is that it could be over restrictive for dma_buf_attach to always > > reject > > dma_virt_ops. According to dma-buf documentation the back storage would be > > moved to system area upon mapping unless p2p is requested and can be > > supported > > by the exporter. The sg_list for system memory would have struct page > > present. > > So I'm not clear on what this dma_virt_ops stuff is for, but if it's > an entirely virtual device with cpu access, then you shouldn't do > dma_buf_map_attachment, and then peek at the struct page in the sgl. Yes, so I think the answer is it is rdma device driver error to call these new APIs and touch the struct page side of the SGL. After Christophs series removign dma_virt_ops we could make that more explicit, like was done for the pci p2p case. > As a third option, if it's something about the connectivity between > the importing and exporting device, then this should be checked in the > ->attach callback the exporter can provide, like the p2p check. The Drivers doing p2p are supposed to be calling the p2p distance stuff and p2p dma map stuff which already has these checks. Doing p2p and skipping all that in the dma buf side we already knew was a hacky thing. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 4/7] drm/vc4: kms: Split the HVS muxing check in a separate function
The code that assigns HVS channels during atomic_check is starting to grow
a bit big, let's move it into a separate function.
Reviewed-by: Hoegeun Kwon
Tested-by: Hoegeun Kwon
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_kms.c | 18 +++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index ad69c70f66a2..bb2efc5d2d01 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -662,13 +662,13 @@ static int vc4_load_tracker_obj_init(struct vc4_dev *vc4)
return drmm_add_action_or_reset(&vc4->base, vc4_load_tracker_obj_fini,
NULL);
}
-static int
-vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
+static int vc4_pv_muxing_atomic_check(struct drm_device *dev,
+ struct drm_atomic_state *state)
{
unsigned long unassigned_channels = GENMASK(HVS_NUM_CHANNELS - 1, 0);
struct drm_crtc_state *old_crtc_state, *new_crtc_state;
struct drm_crtc *crtc;
- int i, ret;
+ unsigned int i;
/*
* Since the HVS FIFOs are shared across all the pixelvalves and
@@ -741,6 +741,18 @@ vc4_atomic_check(struct drm_device *dev, struct
drm_atomic_state *state)
}
}
+ return 0;
+}
+
+static int
+vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
+{
+ int ret;
+
+ ret = vc4_pv_muxing_atomic_check(dev, state);
+ if (ret)
+ return ret;
+
ret = vc4_ctm_atomic_check(dev, state);
if (ret < 0)
return ret;
--
2.28.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm: Use state helper instead of CRTC state pointer
Many drivers reference the crtc->pointer in order to get the current CRTC
state in their atomic_begin or atomic_flush hooks, which would be the new
CRTC state in the global atomic state since _swap_state happened when those
hooks are run.
Use the drm_atomic_get_new_crtc_state helper to get that state to make it
more obvious.
This was made using the coccinelle script below:
@ crtc_atomic_func @
identifier helpers;
identifier func;
@@
(
static struct drm_crtc_helper_funcs helpers = {
...,
.atomic_begin = func,
...,
};
|
static struct drm_crtc_helper_funcs helpers = {
...,
.atomic_flush = func,
...,
};
)
@@
identifier crtc_atomic_func.func;
identifier crtc, state;
symbol crtc_state;
expression e;
@@
func(struct drm_crtc *crtc, struct drm_atomic_state *state) {
...
- struct tegra_dc_state *crtc_state = e;
+ struct tegra_dc_state *dc_state = e;
<+...
- crtc_state
+ dc_state
...+>
}
@@
identifier crtc_atomic_func.func;
identifier crtc, state;
symbol crtc_state;
expression e;
@@
func(struct drm_crtc *crtc, struct drm_atomic_state *state) {
...
- struct mtk_crtc_state *crtc_state = e;
+ struct mtk_crtc_state *mtk_crtc_state = e;
<+...
- crtc_state
+ mtk_crtc_state
...+>
}
@ replaces_new_state @
identifier crtc_atomic_func.func;
identifier crtc, state, crtc_state;
@@
func(struct drm_crtc *crtc, struct drm_atomic_state *state) {
...
- struct drm_crtc_state *crtc_state = crtc->state;
+ struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state,
crtc);
...
}
@@
identifier crtc_atomic_func.func;
identifier crtc, state, crtc_state;
@@
func(struct drm_crtc *crtc, struct drm_atomic_state *state) {
struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state,
crtc);
...
- crtc->state
+ crtc_state
...
}
@ adds_new_state @
identifier crtc_atomic_func.func;
identifier crtc, state;
@@
func(struct drm_crtc *crtc, struct drm_atomic_state *state) {
+ struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state,
crtc);
...
- crtc->state
+ crtc_state
...
}
@ include depends on adds_new_state || replaces_new_state @
@@
#include
@ no_include depends on !include && (adds_new_state || replaces_new_state) @
@@
+ #include
#include
Cc: "James (Qian) Wang"
Cc: Liviu Dudau
Cc: Mihail Atanassov
Cc: Brian Starkey
Cc: Russell King
Cc: Paul Cercueil
Cc: Chun-Kuang Hu
Cc: Philipp Zabel
Cc: Sandy Huang
Cc: "Heiko Stübner"
Cc: Thierry Reding
Cc: Gerd Hoffmann
Reviewed-by: Ville Syrjälä
Suggested-by: Ville Syrjälä
Signed-off-by: Maxime Ripard
---
Changes from v1:
- Fixed checkpatch warnings
---
drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 4 +++-
drivers/gpu/drm/armada/armada_crtc.c | 8 ++--
drivers/gpu/drm/ast/ast_mode.c | 4 +++-
drivers/gpu/drm/ingenic/ingenic-drm-drv.c| 7 +--
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 15 +--
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 6 --
drivers/gpu/drm/tegra/dc.c | 8 +---
drivers/gpu/drm/virtio/virtgpu_display.c | 4 +++-
8 files changed, 38 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
index df0b9eeb8933..4b485eb512e2 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
@@ -387,10 +387,12 @@ static void
komeda_crtc_atomic_flush(struct drm_crtc *crtc,
struct drm_atomic_state *state)
{
+ struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state,
+ crtc);
struct drm_crtc_state *old = drm_atomic_get_old_crtc_state(state,
crtc);
/* commit with modeset will be handled in enable/disable */
- if (drm_atomic_crtc_needs_modeset(crtc->state))
+ if (drm_atomic_crtc_needs_modeset(crtc_state))
return;
komeda_crtc_do_flush(crtc, old);
diff --git a/drivers/gpu/drm/armada/armada_crtc.c
b/drivers/gpu/drm/armada/armada_crtc.c
index ca643f4e2064..3ebcf5a52c8b 100644
--- a/drivers/gpu/drm/armada/armada_crtc.c
+++ b/drivers/gpu/drm/armada/armada_crtc.c
@@ -431,11 +431,13 @@ static int armada_drm_crtc_atomic_check(struct drm_crtc
*crtc,
static void armada_drm_crtc_atomic_begin(struct drm_crtc *crtc,
struct drm_atomic_state *state)
{
+ struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state,
+ crtc);
struct armada_crtc *dcrtc = drm_to_armada_crtc(crtc);
DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.id, crtc->name);
- if (crtc->state->color_mgmt_changed)
+
Re: [PATCH v5 05/15] mm/frame-vector: Use FOLL_LONGTERM
On Thu, Nov 05, 2020 at 10:25:24AM +0100, Daniel Vetter wrote: > > /* > > * If we can't determine whether or not a pte is special, then fail > > immediately > > * for ptes. Note, we can still pin HugeTLB and THP as these are guaranteed > > not > > * to be special. > > * > > * For a futex to be placed on a THP tail page, get_futex_key requires a > > * get_user_pages_fast_only implementation that can pin pages. Thus it's > > still > > * useful to have gup_huge_pmd even if we can't operate on ptes. > > */ > > We support hugepage faults in gpu drivers since recently, and I'm not > seeing a pud_mkhugespecial anywhere. So not sure this works, but probably > just me missing something again. It means ioremap can't create an IO page PUD, it has to be broken up. Does ioremap even create anything larger than PTEs? Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 7/7] drm/vc4: kms: Don't disable the muxing of an active CRTC
The current HVS muxing code will consider the CRTCs in a given state to
setup their muxing in the HVS, and disable the other CRTCs muxes.
However, it's valid to only update a single CRTC with a state, and in this
situation we would mux out a CRTC that was enabled but left untouched by
the new state.
Fix this by setting a flag on the CRTC state when the muxing has been
changed, and only change the muxing configuration when that flag is there.
Fixes: 87ebcd42fb7b ("drm/vc4: crtc: Assign output to channel automatically")
Reviewed-by: Hoegeun Kwon
Tested-by: Hoegeun Kwon
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_drv.h | 1 +
drivers/gpu/drm/vc4/vc4_kms.c | 82 ---
2 files changed, 48 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 014113823647..325b53ff11b3 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -524,6 +524,7 @@ struct vc4_crtc_state {
struct drm_mm_node mm;
bool feed_txp;
bool txp_armed;
+ bool needs_muxing;
unsigned int assigned_channel;
struct {
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 0a231ae500e5..7ef164afa9e2 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -226,10 +226,7 @@ static void vc5_hvs_pv_muxing_commit(struct vc4_dev *vc4,
{
struct drm_crtc_state *crtc_state;
struct drm_crtc *crtc;
- unsigned char dsp2_mux = 0;
- unsigned char dsp3_mux = 3;
- unsigned char dsp4_mux = 3;
- unsigned char dsp5_mux = 3;
+ unsigned char mux;
unsigned int i;
u32 reg;
@@ -237,50 +234,59 @@ static void vc5_hvs_pv_muxing_commit(struct vc4_dev *vc4,
struct vc4_crtc_state *vc4_state =
to_vc4_crtc_state(crtc_state);
struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
- if (!crtc_state->active)
+ if (!vc4_state->needs_muxing)
continue;
switch (vc4_crtc->data->hvs_output) {
case 2:
- dsp2_mux = (vc4_state->assigned_channel == 2) ? 0 : 1;
+ mux = (vc4_state->assigned_channel == 2) ? 0 : 1;
+ reg = HVS_READ(SCALER_DISPECTRL);
+ HVS_WRITE(SCALER_DISPECTRL,
+ (reg & ~SCALER_DISPECTRL_DSP2_MUX_MASK) |
+ VC4_SET_FIELD(mux,
SCALER_DISPECTRL_DSP2_MUX));
break;
case 3:
- dsp3_mux = vc4_state->assigned_channel;
+ if (vc4_state->assigned_channel ==
VC4_HVS_CHANNEL_DISABLED)
+ mux = 3;
+ else
+ mux = vc4_state->assigned_channel;
+
+ reg = HVS_READ(SCALER_DISPCTRL);
+ HVS_WRITE(SCALER_DISPCTRL,
+ (reg & ~SCALER_DISPCTRL_DSP3_MUX_MASK) |
+ VC4_SET_FIELD(mux, SCALER_DISPCTRL_DSP3_MUX));
break;
case 4:
- dsp4_mux = vc4_state->assigned_channel;
+ if (vc4_state->assigned_channel ==
VC4_HVS_CHANNEL_DISABLED)
+ mux = 3;
+ else
+ mux = vc4_state->assigned_channel;
+
+ reg = HVS_READ(SCALER_DISPEOLN);
+ HVS_WRITE(SCALER_DISPEOLN,
+ (reg & ~SCALER_DISPEOLN_DSP4_MUX_MASK) |
+ VC4_SET_FIELD(mux, SCALER_DISPEOLN_DSP4_MUX));
+
break;
case 5:
- dsp5_mux = vc4_state->assigned_channel;
+ if (vc4_state->assigned_channel ==
VC4_HVS_CHANNEL_DISABLED)
+ mux = 3;
+ else
+ mux = vc4_state->assigned_channel;
+
+ reg = HVS_READ(SCALER_DISPDITHER);
+ HVS_WRITE(SCALER_DISPDITHER,
+ (reg & ~SCALER_DISPDITHER_DSP5_MUX_MASK) |
+ VC4_SET_FIELD(mux,
SCALER_DISPDITHER_DSP5_MUX));
break;
default:
break;
}
}
-
- reg = HVS_READ(SCALER_DISPECTRL);
- HVS_WRITE(SCALER_DISPECTRL,
- (reg & ~SCALER_DISPECTRL_DSP2_MUX_MASK) |
- VC4_SET_FIELD(dsp2_mux, SCALER_DISPECTRL_DSP2_MUX));
-
- reg = HVS_READ(SCALER_DISPCTRL);
- HVS_WRITE(SCALER_DISPCTRL,
- (reg & ~SCALER_DISPCTRL_DSP3_MUX_MASK) |
- VC4_SET_FIELD(dsp3_mux, SCALER_DISPCTRL_DSP3_MUX));
-
- reg = HVS_READ(SCALER_DISPEOLN);
-
Re: [PATCH v1 00/30] Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs
On 05-11-20, 11:56, Ulf Hansson wrote: > On Thu, 5 Nov 2020 at 11:40, Viresh Kumar wrote: > > Btw, how do we identify if it is a power domain or a regulator ? To be honest, I was a bit afraid and embarrassed to ask this question, and was hoping people to make fun of me in return :) > Good question. It's not a crystal clear line in between them, I think. And I was relieved after reading this :) > A power domain to me, means that some part of a silicon (a group of > controllers or just a single piece, for example) needs some kind of > resource (typically a power rail) to be enabled to be functional, to > start with. Isn't this what a part of regulator does as well ? i.e. enabling/disabling of the regulator or power to a group of controllers. Over that the regulator does voltage/current scaling as well, which normally the power domains don't do (though we did that in performance-state case). > If there are operating points involved, that's also a > clear indication to me, that it's not a regular regulator. Is there any example of that? I hope by OPP you meant both freq and voltage here. I am not sure if I know of a case where a power domain handles both of them. > Maybe we should try to specify this more exactly in some > documentation, somewhere. I think yes, it is very much required. And in absence of that I think, many (or most) of the platforms that also need to scale the voltage would have modeled their hardware as a regulator and not a PM domain. What I always thought was: - Module that can just enable/disable power to a block of SoC is a power domain. - Module that can enable/disable as well as scale voltage is a regulator. And so I thought that this patchset has done the right thing. This changed a bit with the qcom stuff where the IP to be configured was in control of RPM and not Linux and so we couldn't add it as a regulator. If it was controlled by Linux, it would have been a regulator in kernel for sure :) -- viresh ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 21/30] usb: host: ehci-tegra: Support OPP and SoC core voltage scaling
05.11.2020 19:07, Alan Stern пишет: >> +err = devm_tegra_ehci_init_opp_table(&pdev->dev); >> +if (err) >> +return dev_err_probe(&pdev->dev, err, >> + "Failed to initialize OPP\n"); > Why log a second error message? Just return err. Indeed, thanks. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 03/19] gpu: drm: imx: ipuv3-plane: Mark 'crtc_state' as __always_unused
Hello Lee,
On 11/5/20 3:45 PM, Lee Jones wrote:
> In the macro for_each_oldnew_crtc_in_state() 'crtc_state' is provided
> as a container for state->crtcs[i].new_state, but is not utilised in
> this use-case. We cannot simply delete the variable, so here we tell
> the compiler that we're intentionally discarding the read value.
for_each_oldnew_crtc_in_state already (void) casts the drm_crtc and the old
drm_crtc_state to silence unused-but-set-variable warning. Should we maybe
(void) cast the new crtc_state as well?
Cheers
Ahmad
>
> Fixes the following W=1 kernel build warning(s):
>
> drivers/gpu/drm/imx/ipuv3-plane.c: In function ‘ipu_planes_assign_pre’:
> drivers/gpu/drm/imx/ipuv3-plane.c:746:42: warning: variable ‘crtc_state’ set
> but not used [-Wunused-but-set-variable]
>
> Cc: Philipp Zabel
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Shawn Guo
> Cc: Sascha Hauer
> Cc: Pengutronix Kernel Team
> Cc: Fabio Estevam
> Cc: NXP Linux Team
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Lee Jones
> ---
> drivers/gpu/drm/imx/ipuv3-plane.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c
> b/drivers/gpu/drm/imx/ipuv3-plane.c
> index 8a4235d9d9f1e..acc0a3ce4992f 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> @@ -743,7 +743,7 @@ bool ipu_plane_atomic_update_pending(struct drm_plane
> *plane)
> int ipu_planes_assign_pre(struct drm_device *dev,
> struct drm_atomic_state *state)
> {
> - struct drm_crtc_state *old_crtc_state, *crtc_state;
> + struct drm_crtc_state *old_crtc_state, __always_unused *crtc_state;
> struct drm_plane_state *plane_state;
> struct ipu_plane_state *ipu_state;
> struct ipu_plane *ipu_plane;
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: Pass the full state to connectors atomic functions
The current atomic helpers have either their object state being passed as
an argument or the full atomic state.
The former is the pattern that was done at first, before switching to the
latter for new hooks or when it was needed.
Now that the CRTCs have been converted, let's move forward with the
connectors to provide a consistent interface.
The conversion was done using the coccinelle script below, and built tested
on all the drivers.
Cc: Harry Wentland
Cc: Leo Li
Cc: Alex Deucher
Cc: "Christian König"
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Ben Skeggs
Cc: Rodrigo Siqueira
Cc: Melissa Wen
Cc: Haneen Mohammed
Signed-off-by: Maxime Ripard
---
.../drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 5 -
drivers/gpu/drm/drm_atomic_helper.c | 8
drivers/gpu/drm/i915/display/intel_dp_mst.c | 7 +--
drivers/gpu/drm/nouveau/dispnv50/disp.c | 5 -
drivers/gpu/drm/vc4/vc4_txp.c | 4 +++-
drivers/gpu/drm/vkms/vkms_writeback.c | 7 +--
include/drm/drm_modeset_helper_vtables.h| 13 ++---
7 files changed, 31 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index eee19ed5..f346cc74387f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -24,6 +24,7 @@
*/
#include
+#include
#include
#include
#include
@@ -252,8 +253,10 @@ static int dm_dp_mst_get_modes(struct drm_connector
*connector)
static struct drm_encoder *
dm_mst_atomic_best_encoder(struct drm_connector *connector,
- struct drm_connector_state *connector_state)
+ struct drm_atomic_state *state)
{
+ struct drm_connector_state *connector_state =
drm_atomic_get_new_connector_state(state,
+
connector);
struct drm_device *dev = connector->dev;
struct amdgpu_device *adev = drm_to_adev(dev);
struct amdgpu_crtc *acrtc = to_amdgpu_crtc(connector_state->crtc);
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index ddd0e3239150..ba1507036f26 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -122,7 +122,8 @@ static int handle_conflicting_encoders(struct
drm_atomic_state *state,
continue;
if (funcs->atomic_best_encoder)
- new_encoder = funcs->atomic_best_encoder(connector,
new_conn_state);
+ new_encoder = funcs->atomic_best_encoder(connector,
+state);
else if (funcs->best_encoder)
new_encoder = funcs->best_encoder(connector);
else
@@ -345,8 +346,7 @@ update_connector_routing(struct drm_atomic_state *state,
funcs = connector->helper_private;
if (funcs->atomic_best_encoder)
- new_encoder = funcs->atomic_best_encoder(connector,
-new_connector_state);
+ new_encoder = funcs->atomic_best_encoder(connector, state);
else if (funcs->best_encoder)
new_encoder = funcs->best_encoder(connector);
else
@@ -1313,7 +1313,7 @@ static void drm_atomic_helper_commit_writebacks(struct
drm_device *dev,
if (new_conn_state->writeback_job &&
new_conn_state->writeback_job->fb) {
WARN_ON(connector->connector_type !=
DRM_MODE_CONNECTOR_WRITEBACK);
- funcs->atomic_commit(connector, new_conn_state);
+ funcs->atomic_commit(connector, old_state);
}
}
}
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 64d885539e94..b879a0622ada 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -23,6 +23,7 @@
*
*/
+#include
#include
#include
#include
@@ -706,11 +707,13 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector
*connector,
}
static struct drm_encoder *intel_mst_atomic_best_encoder(struct drm_connector
*connector,
-struct
drm_connector_state *state)
+struct
drm_atomic_state *state)
{
+ struct drm_connector_state *connector_state =
drm_atomic_get_new_connector_state(state,
+
connector);
struct intel_connector *intel_connector = to_intel_connector(connector);
struct intel_dp *intel_dp = inte
[PATCH] drm/tegra: replace idr_init() by idr_init_base()
idr_init() uses base 0 which is an invalid identifier for this driver.
The new function idr_init_base allows IDR to set the ID lookup from
base 1. This avoids all lookups that otherwise starts from 0 since
0 is always unused.
References: commit 6ce711f27500 ("idr: Make 1-based IDRs more efficient")
Signed-off-by: Deepak R Varma
---
drivers/gpu/drm/tegra/drm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index ba9d1c3e7cac..e4baf07992a4 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -90,7 +90,7 @@ static int tegra_drm_open(struct drm_device *drm, struct
drm_file *filp)
if (!fpriv)
return -ENOMEM;
- idr_init(&fpriv->contexts);
+ idr_init_base(&fpriv->contexts, 1);
mutex_init(&fpriv->lock);
filp->driver_priv = fpriv;
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amdkfd: replace idr_init() by idr_init_base()
On Wed, Nov 04, 2020 at 03:01:17PM -0500, Felix Kuehling wrote:
> On 2020-11-04 10:13 a.m., Deepak R Varma wrote:
> > idr_init() uses base 0 which is an invalid identifier. The new function
> > idr_init_base allows IDR to set the ID lookup from base 1. This avoids
> > all lookups that otherwise starts from 0 since 0 is always unused.
>
> I disagree. We call idr_alloc with start=0 for both these IDRs. That means 0
> seems to be a valid handle.
Hello Felix,
You are correct. There are calls made to idr_alloc with start range from
0. So, for this driver, id=0 seems a valid use case. The change I
proposed is not relevant for this driver. You may please ignore the
patch.
Thank you,
./drv
>
> Regards,
> Felix
>
>
> >
> > References: commit 6ce711f27500 ("idr: Make 1-based IDRs more efficient")
> >
> > Signed-off-by: Deepak R Varma
> > ---
> > drivers/gpu/drm/amd/amdkfd/kfd_events.c | 2 +-
> > drivers/gpu/drm/amd/amdkfd/kfd_process.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
> > b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
> > index ba2c2ce0c55a..b3339b53c8ad 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
> > @@ -230,7 +230,7 @@ static int create_other_event(struct kfd_process *p,
> > struct kfd_event *ev)
> > void kfd_event_init_process(struct kfd_process *p)
> > {
> > mutex_init(&p->event_mutex);
> > - idr_init(&p->event_idr);
> > + idr_init_base(&p->event_idr, 1);
> > p->signal_page = NULL;
> > p->signal_event_count = 0;
> > }
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> > b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> > index 65803e153a22..022e61babe30 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> > @@ -1289,7 +1289,7 @@ struct kfd_process_device
> > *kfd_create_process_device_data(struct kfd_dev *dev,
> > list_add(&pdd->per_device_list, &p->per_device_data);
> > /* Init idr used for memory handle translation */
> > - idr_init(&pdd->alloc_idr);
> > + idr_init_base(&pdd->alloc_idr, 1);
> > return pdd;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 36/47] memory: tegra20-emc: Add devfreq support
05.11.2020 05:30, Chanwoo Choi пишет:
> On 11/5/20 1:49 AM, Dmitry Osipenko wrote:
>> Add devfreq support to the Tegra20 EMC driver. Memory utilization
>> statistics will be periodically polled from the memory controller and
>> appropriate minimum clock rate will be selected by the devfreq governor.
>>
>> Signed-off-by: Dmitry Osipenko
>> ---
>> drivers/memory/tegra/Kconfig | 2 +
>> drivers/memory/tegra/tegra20-emc.c | 92 ++
>> 2 files changed, 94 insertions(+)
>>
>> diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
>> index ac3dfe155505..76e9a3b10839 100644
>> --- a/drivers/memory/tegra/Kconfig
>> +++ b/drivers/memory/tegra/Kconfig
>> @@ -12,6 +12,8 @@ config TEGRA20_EMC
>> tristate "NVIDIA Tegra20 External Memory Controller driver"
>> default y
>> depends on TEGRA_MC && ARCH_TEGRA_2x_SOC
>> +select DEVFREQ_GOV_SIMPLE_ONDEMAND
>> +select PM_DEVFREQ
>> select PM_OPP
>
> nitpick. If you select PM_DEVFREQ, don't need to select 'PM_OPP'
> bacause PM_DEVFREQ use OPP as mandatory with 'select PM_OPP' in Kconfig.
Ok
...
>> +static int tegra_emc_devfreq_init(struct tegra_emc *emc)
>> +{
>> +int err;
>> +
>> +/*
>> + * PWR_COUNT is 1/2 of PWR_CLOCKS at max, and thus, the up-threshold
>> + * should be less than 50. Secondly, multiple active memory clients
>> + * may cause over 20% of lost clock cycles due to stalls caused by
>> + * competing memory accesses. This means that threshold should be
>> + * set to a less than 30 in order to have a properly working governor.
>> + */
>> +emc->ondemand_data.upthreshold = 20;
>> +
>> +/*
>> + * Reset statistic gathers state, select global bandwidth for the
>> + * statistics collection mode and set clocks counter saturation
>> + * limit to maximum.
>> + */
>> +writel_relaxed(0x, emc->regs + EMC_STAT_CONTROL);
>> +writel_relaxed(0x, emc->regs + EMC_STAT_LLMC_CONTROL);
>> +writel_relaxed(0x, emc->regs + EMC_STAT_PWR_CLOCK_LIMIT);
>> +
>> +emc->devfreq = devfreq_add_device(emc->dev, &tegra_emc_devfreq_profile,
>> + DEVFREQ_GOV_SIMPLE_ONDEMAND,
>> + &emc->ondemand_data);
>
> Do you want to use 'devfreq_add_device' instead of
> 'devm_devfreq_add_device()'? If you have to use 'devfreq_add_device'
> due to some reason, you need to call 'devfreq_remove_device' on exit.
The reason I didn't use the devm here is because the EMC-clk callback
should be unregistered *after* devfreq is removed.
Thinking a bit more about it, I guess the best variant will be to add
devm support to the clk callback registration and then it should be
possible to use devm for the devfreq. I'll try to implement it in v8,
thanks.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 00/30] Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs
On Thu, 5 Nov 2020 at 12:13, Viresh Kumar wrote: > > On 05-11-20, 11:56, Ulf Hansson wrote: > > On Thu, 5 Nov 2020 at 11:40, Viresh Kumar wrote: > > > Btw, how do we identify if it is a power domain or a regulator ? > > To be honest, I was a bit afraid and embarrassed to ask this question, > and was hoping people to make fun of me in return :) > > > Good question. It's not a crystal clear line in between them, I think. > > And I was relieved after reading this :) > > > A power domain to me, means that some part of a silicon (a group of > > controllers or just a single piece, for example) needs some kind of > > resource (typically a power rail) to be enabled to be functional, to > > start with. > > Isn't this what a part of regulator does as well ? i.e. > enabling/disabling of the regulator or power to a group of > controllers. It could, but it shouldn't. > > Over that the regulator does voltage/current scaling as well, which > normally the power domains don't do (though we did that in > performance-state case). > > > If there are operating points involved, that's also a > > clear indication to me, that it's not a regular regulator. > > Is there any example of that? I hope by OPP you meant both freq and > voltage here. I am not sure if I know of a case where a power domain > handles both of them. It may be both voltage and frequency - but in some cases only voltage. >From HW point of view, many ARM legacy platforms have power domains that work like this. As you know, the DVFS case has in many years not been solved in a generic way, but mostly via platform specific hacks. The worst ones are probably those hacking clock drivers (which myself also have contributed to). Have a look at clk_prcmu_opp_prepare(), for example, which is used by the UX500 platform. Another option has been to use the devfreq framework, but it has limitations in regards to this too. That said, I am hoping that people start moving towards the deploying/implementing DVFS through the power-domain approach, together with the OPPs. Maybe there are still some pieces missing from an infrastructure point of view, but that should become more evident as more starts using it. > > > Maybe we should try to specify this more exactly in some > > documentation, somewhere. > > I think yes, it is very much required. And in absence of that I think, > many (or most) of the platforms that also need to scale the voltage > would have modeled their hardware as a regulator and not a PM domain. > > What I always thought was: > > - Module that can just enable/disable power to a block of SoC is a > power domain. > > - Module that can enable/disable as well as scale voltage is a > regulator. > > And so I thought that this patchset has done the right thing. This > changed a bit with the qcom stuff where the IP to be configured was in > control of RPM and not Linux and so we couldn't add it as a regulator. > If it was controlled by Linux, it would have been a regulator in > kernel for sure :) In my view, DT bindings have consistently been pushed back during the year, if they have tried to model power domains as regulator supplies from consumer device nodes. Hence, people have tried other things, as I mentioned above. I definitely agree that we need to update some documentations, explaining things more exactly. Additionally, it seems like a talk at some conferences should make sense, as a way to spread the word. Kind regards Uffe ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 3/7] drm/vc4: kms: Rename NUM_CHANNELS
The NUM_CHANNELS define has a pretty generic name and was right before the
function using it. Let's move to something that makes the hardware-specific
nature more obvious, and to a more appropriate place.
Reviewed-by: Hoegeun Kwon
Tested-by: Hoegeun Kwon
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_kms.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 4b558ccb18fe..ad69c70f66a2 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -24,6 +24,8 @@
#include "vc4_drv.h"
#include "vc4_regs.h"
+#define HVS_NUM_CHANNELS 3
+
struct vc4_ctm_state {
struct drm_private_state base;
struct drm_color_ctm *ctm;
@@ -660,12 +662,10 @@ static int vc4_load_tracker_obj_init(struct vc4_dev *vc4)
return drmm_add_action_or_reset(&vc4->base, vc4_load_tracker_obj_fini,
NULL);
}
-#define NUM_CHANNELS 3
-
static int
vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
{
- unsigned long unassigned_channels = GENMASK(NUM_CHANNELS - 1, 0);
+ unsigned long unassigned_channels = GENMASK(HVS_NUM_CHANNELS - 1, 0);
struct drm_crtc_state *old_crtc_state, *new_crtc_state;
struct drm_crtc *crtc;
int i, ret;
--
2.28.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 00/30] Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs
05.11.2020 04:45, Michał Mirosław пишет: > On Thu, Nov 05, 2020 at 02:43:57AM +0300, Dmitry Osipenko wrote: >> Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs, which reduces >> power consumption and heating of the Tegra chips. Tegra SoC has multiple >> hardware units which belong to a core power domain of the SoC and share >> the core voltage. The voltage must be selected in accordance to a minimum >> requirement of every core hardware unit. > [...] > > Just looked briefly through the series - it looks like there is a lot of > code duplication in *_init_opp_table() functions. Could this be made > more generic / data-driven? Indeed, it should be possible to add a common helper. I had a quick thought about doing it too, but then decided to defer for the starter since there were some differences among the needs of the drivers. I'll take a closer look for the v2, thanks! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 2/7] drm/vc4: kms: Remove useless define
NUM_OUTPUTS isn't used anymore, let's remove it. Reviewed-by: Hoegeun Kwon Tested-by: Hoegeun Kwon Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_kms.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index 44db31e16e91..4b558ccb18fe 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -660,7 +660,6 @@ static int vc4_load_tracker_obj_init(struct vc4_dev *vc4) return drmm_add_action_or_reset(&vc4->base, vc4_load_tracker_obj_fini, NULL); } -#define NUM_OUTPUTS 6 #define NUM_CHANNELS 3 static int -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v8 4/5] RDMA/mlx5: Support dma-buf based userspace memory region
On Thu, Nov 05, 2020 at 02:48:08PM -0800, Jianxin Xiong wrote:
> @@ -966,7 +969,10 @@ static struct mlx5_ib_mr *alloc_mr_from_cache(struct
> ib_pd *pd,
> struct mlx5_ib_mr *mr;
> unsigned int page_size;
>
> - page_size = mlx5_umem_find_best_pgsz(umem, mkc, log_page_size, 0, iova);
> + if (umem->is_dmabuf)
> + page_size = ib_umem_find_best_pgsz(umem, PAGE_SIZE, iova);
You said the sgl is not set here, why doesn't this crash? It is
certainly wrong to call this function without a SGL.
> +/**
> + * mlx5_ib_fence_dmabuf_mr - Stop all access to the dmabuf MR
> + * @mr: to fence
> + *
> + * On return no parallel threads will be touching this MR and no DMA will be
> + * active.
> + */
> +void mlx5_ib_fence_dmabuf_mr(struct mlx5_ib_mr *mr)
> +{
> + struct ib_umem_dmabuf *umem_dmabuf = to_ib_umem_dmabuf(mr->umem);
> +
> + /* Prevent new page faults and prefetch requests from succeeding */
> + xa_erase(&mr->dev->odp_mkeys, mlx5_base_mkey(mr->mmkey.key));
> +
> + /* Wait for all running page-fault handlers to finish. */
> + synchronize_srcu(&mr->dev->odp_srcu);
> +
> + wait_event(mr->q_deferred_work, !atomic_read(&mr->num_deferred_work));
> +
> + dma_resv_lock(umem_dmabuf->attach->dmabuf->resv, NULL);
> + mlx5_mr_cache_invalidate(mr);
> + umem_dmabuf->private = NULL;
> + dma_resv_unlock(umem_dmabuf->attach->dmabuf->resv);
> +
> + if (!mr->cache_ent) {
> + mlx5_core_destroy_mkey(mr->dev->mdev, &mr->mmkey);
> + WARN_ON(mr->descs);
> + }
> +}
I would expect this to call ib_umem_dmabuf_unmap_pages() ?
Who calls it on the dereg path?
This looks quite strange to me, it calls ib_umem_dmabuf_unmap_pages()
only from the invalidate callback?
I feel uneasy how this seems to assume everything works sanely, we can
have parallel page faults so pagefault_dmabuf_mr() can be called
multiple times after an invalidation, and it doesn't protect itself
against calling ib_umem_dmabuf_map_pages() twice.
Perhaps the umem code should keep track of the current map state and
exit if there is already a sgl. NULL or not NULL sgl would do and
seems quite reasonable.
> @@ -810,22 +871,31 @@ static int pagefault_mr(struct mlx5_ib_mr *mr, u64
> io_virt, size_t bcnt,
> u32 *bytes_mapped, u32 flags)
> {
> struct ib_umem_odp *odp = to_ib_umem_odp(mr->umem);
> + struct ib_umem_dmabuf *umem_dmabuf = to_ib_umem_dmabuf(mr->umem);
>
> lockdep_assert_held(&mr->dev->odp_srcu);
> if (unlikely(io_virt < mr->mmkey.iova))
> return -EFAULT;
>
> - if (!odp->is_implicit_odp) {
> + if (is_dmabuf_mr(mr) || !odp->is_implicit_odp) {
> u64 user_va;
> + u64 end;
>
> if (check_add_overflow(io_virt - mr->mmkey.iova,
> -(u64)odp->umem.address, &user_va))
> +(u64)mr->umem->address, &user_va))
> return -EFAULT;
> - if (unlikely(user_va >= ib_umem_end(odp) ||
> - ib_umem_end(odp) - user_va < bcnt))
> + if (is_dmabuf_mr(mr))
> + end = mr->umem->address + mr->umem->length;
> + else
> + end = ib_umem_end(odp);
> + if (unlikely(user_va >= end || end - user_va < bcnt))
> return -EFAULT;
> - return pagefault_real_mr(mr, odp, user_va, bcnt, bytes_mapped,
> - flags);
> + if (is_dmabuf_mr(mr))
> + return pagefault_dmabuf_mr(mr, umem_dmabuf, user_va,
> +bcnt, bytes_mapped, flags);
But this doesn't care about user_va or bcnt it just triggers the whole
thing to be remapped, so why calculate it?
Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 0/7] drm/vc4: Rework the HVS muxing code
Hi, Here's a second attempt at fixing the current issues we have with the muxing code that results in a PV muxing its HVS muxing when only another CRTC is modified by a state, or vblank timeouts when trying to wait for a vblank on a single CRTC while another one is inactive but enabled. Let me know what you think, Maxime Changes from v1: - Dropped the code trying to access all the CRTCs (whether in the state or not) state - Added Hoegeun Kwon's tags - Fixed a build bisection error - Cleaned up the private state using drmm_add_action_or_reset - Rebased on current linux next Maxime Ripard (7): drm/vc4: kms: Switch to drmm_add_action_or_reset drm/vc4: kms: Remove useless define drm/vc4: kms: Rename NUM_CHANNELS drm/vc4: kms: Split the HVS muxing check in a separate function drm/vc4: kms: Document the muxing corner cases drm/vc4: kms: Store the unassigned channel list in the state drm/vc4: kms: Don't disable the muxing of an active CRTC drivers/gpu/drm/vc4/vc4_drv.h | 2 + drivers/gpu/drm/vc4/vc4_kms.c | 247 +- 2 files changed, 185 insertions(+), 64 deletions(-) -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 00/56] Convert DSI code to use drm_mipi_dsi and drm_panel
> Am 05.11.2020 um 19:28 schrieb Tomi Valkeinen : > > On 05/11/2020 20:14, H. Nikolaus Schaller wrote: >> >>> Am 05.11.2020 um 18:36 schrieb Tomi Valkeinen : >>> >>> Hi, >>> >>> On 05/11/2020 19:15, H. Nikolaus Schaller wrote: >>> Next, I migrated my long waiting mipi_dsi/drm_panel driver conversion for the panel of the Pyra handheld (omap 5 based) to compile on 5.10-rc2. And I followed the latest existing panel-orisetech-otm8009a.c which uses a similar video mode controller and mipi-dsi. That one seems to be used by arch/arm/boot/dts/stm32f469-disco.dts. Unfortunately my panel driver is not even loaded by drm/omap so I can't debug. Does this set of drm/omap drivers need a modification of the device tree? If yes, which one? >>> >>> omapdrm doesn't load the panel drivers. If not even your panel's probe is >>> called, then it hints at >>> DT and/or driver's compatible string issue. The panel's probe should get >>> called even if omapdrm is >>> not loaded at all. >> >> Well, I use the same device tree that loads the old driver... > > Yeah, I was mistaken above. With DSI the panel (may be) a child of the DSI > host, so it will affect. > > Can you give pointers to the panel driver source and relevant dts files? BOE > BTL507212-W677L? Yes. It is (now) drivers/gpu/drm/panel/panel-boe-btl507212-w677l.c and arch/arm/boot/dts/omap5-letux-cortex15-common.dtsi (for the basic dsi definitions) arch/arm/boot/dts/pyra-display.dtsi (for the specific display) All this is merged by some arch/arm/boot/dts/omap5-letux-cortex15-v5.3+pyra-v5.2.dts (I know there are quite a lot of variants but it is a modular system potentially able to accomodate a non-omap processor but the same display). BR, Nikolaus ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/vc4: replace idr_init() by idr_init_base()
idr_init() uses base 0 which is an invalid identifier for this driver.
The idr_alloc for this driver uses VC4_PERFMONID_MIN as start value for
ID range and it is #defined to 1. The new function idr_init_base allows
IDR to set the ID lookup from base 1. This avoids all lookups that
otherwise starts from 0 since 0 is always unused / available.
References: commit 6ce711f27500 ("idr: Make 1-based IDRs more efficient")
Signed-off-by: Deepak R Varma
---
Changes since v1:
- Change suggested by Eric Anholt
1. Use VC4_PERFMONID_MIN instead of magic number 1
drivers/gpu/drm/vc4/vc4_perfmon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_perfmon.c
b/drivers/gpu/drm/vc4/vc4_perfmon.c
index f4aa75efd16b..18abc06335c1 100644
--- a/drivers/gpu/drm/vc4/vc4_perfmon.c
+++ b/drivers/gpu/drm/vc4/vc4_perfmon.c
@@ -77,7 +77,7 @@ struct vc4_perfmon *vc4_perfmon_find(struct vc4_file
*vc4file, int id)
void vc4_perfmon_open_file(struct vc4_file *vc4file)
{
mutex_init(&vc4file->perfmon.lock);
- idr_init(&vc4file->perfmon.idr);
+ idr_init_base(&vc4file->perfmon.idr, VC4_PERFMONID_MIN);
}
static int vc4_perfmon_idr_del(int id, void *elem, void *data)
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm: Use state helper instead of CRTC state pointer
On Tue, Nov 03, 2020 at 06:28:24PM +0200, Ville Syrjälä wrote:
> On Tue, Nov 03, 2020 at 05:15:51PM +0100, Maxime Ripard wrote:
> > Hi Ville,
> >
> > On Mon, Nov 02, 2020 at 06:04:06PM +0200, Ville Syrjälä wrote:
> > > On Mon, Nov 02, 2020 at 02:38:33PM +0100, Maxime Ripard wrote:
> > > > Many drivers reference the crtc->pointer in order to get the current
> > > > CRTC
> > > > state in their atomic_begin or atomic_flush hooks, which would be the
> > > > new
> > > > CRTC state in the global atomic state since _swap_state happened when
> > > > those
> > > > hooks are run.
> > > >
> > > > Use the drm_atomic_get_new_crtc_state helper to get that state to make
> > > > it
> > > > more obvious.
> > > >
> > > > This was made using the coccinelle script below:
> > > >
> > > > @ crtc_atomic_func @
> > > > identifier helpers;
> > > > identifier func;
> > > > @@
> > > >
> > > > (
> > > > static struct drm_crtc_helper_funcs helpers = {
> > > > ...,
> > > > .atomic_begin = func,
> > > > ...,
> > > > };
> > > > |
> > > > static struct drm_crtc_helper_funcs helpers = {
> > > > ...,
> > > > .atomic_flush = func,
> > > > ...,
> > > > };
> > > > )
> > > >
> > > > @@
> > > > identifier crtc_atomic_func.func;
> > > > identifier crtc, state;
> > > > symbol crtc_state;
> > > > expression e;
> > > > @@
> > > >
> > > > func(struct drm_crtc *crtc, struct drm_atomic_state *state) {
> > > > ...
> > > > - struct tegra_dc_state *crtc_state = e;
> > > > + struct tegra_dc_state *dc_state = e;
> > > > <+...
> > > > - crtc_state
> > > > + dc_state
> > > > ...+>
> > > > }
> > > >
> > > > @@
> > > > identifier crtc_atomic_func.func;
> > > > identifier crtc, state;
> > > > symbol crtc_state;
> > > > expression e;
> > > > @@
> > > >
> > > > func(struct drm_crtc *crtc, struct drm_atomic_state *state) {
> > > > ...
> > > > - struct mtk_crtc_state *crtc_state = e;
> > > > + struct mtk_crtc_state *mtk_crtc_state = e;
> > > > <+...
> > > > - crtc_state
> > > > + mtk_crtc_state
> > > > ...+>
> > > > }
> > >
> > > These reanames seem a bit out of scpe for this patch. But I guess you
> > > needed then to get the rest of the cocci to work on some drivers?
> >
> > Yeah, those two drivers already had a variable named crtc_state, calling
> > container_of on crtc->state
> >
> > It was cleaner to me to have an intermediate variable storing the result
> > of drm_atomic_get_new_crtc_state, but then the most obvious name was
> > taken so I had to rename those two variables before doing so.
> >
> > > The basic idea looks good:
> > > Reviewed-by: Ville Syrjälä
> > >
> > > But I guess up to the individual driver folks to bikeshed the variable
> > > naming and whatnot.
> > >
> > > One thing I spotted is that a few drivers now gained two aliasing crtc
> > > state pointers in the function: one with the drm type, the other with
> > > a driver specific type. That's something we've outlawed in i915 since
> > > it was making life rather confusing. In i915 we now prefer to use only
> > > the i915 specific types in most places.
> >
> > I didn't spot any of those cases, do you have an example of where it
> > happened?
>
> eg. ast:
> + struct drm_crtc_state *crtc_state =
> drm_atomic_get_new_crtc_state(state,
> +
> crtc);
> struct drm_crtc_state *old_crtc_state =
> drm_atomic_get_old_crtc_state(state,
>
> crtc);
> struct ast_private *ast = to_ast_private(crtc->dev);
>
> - struct ast_crtc_state *ast_crtc_state =
> to_ast_crtc_state(crtc->state);
> + struct ast_crtc_state *ast_crtc_state =
> to_ast_crtc_state(crtc_state);
>
> So here both 'crtc_state' and 'ast_crtc_state' are basically the same
> thing, which can get a bit confusing especially within larger functions
> with lots of variables.
Ah, that kind of aliasing, ok. So it's purely an issue with
ergonomics/convenience?
It seems to be a widespread pattern (I know we're using it in vc4 and
sun4i, and from those reworks I've seen a number of drivers doing so).
I'm not entirely sure how we should fix it through coccinelle too :/
Thanks!
Maxime
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vc4: replace idr_init() by idr_init_base()
On Thu, Nov 05, 2020 at 11:25:11AM -0800, Eric Anholt wrote:
> On Thu, Nov 5, 2020 at 10:25 AM Deepak R Varma wrote:
> >
> > idr_init() uses base 0 which is an invalid identifier for this driver.
> > The idr_alloc for this driver uses VC4_PERFMONID_MIN as start value for
> > ID range and it is #defined to 1. The new function idr_init_base allows
> > IDR to set the ID lookup from base 1. This avoids all lookups that
> > otherwise starts from 0 since 0 is always unused / available.
> >
> > References: commit 6ce711f27500 ("idr: Make 1-based IDRs more efficient")
> >
> > Signed-off-by: Deepak R Varma
> > ---
> > drivers/gpu/drm/vc4/vc4_perfmon.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/vc4/vc4_perfmon.c
> > b/drivers/gpu/drm/vc4/vc4_perfmon.c
> > index f4aa75efd16b..7d40f421d922 100644
> > --- a/drivers/gpu/drm/vc4/vc4_perfmon.c
> > +++ b/drivers/gpu/drm/vc4/vc4_perfmon.c
> > @@ -77,7 +77,7 @@ struct vc4_perfmon *vc4_perfmon_find(struct vc4_file
> > *vc4file, int id)
> > void vc4_perfmon_open_file(struct vc4_file *vc4file)
> > {
> > mutex_init(&vc4file->perfmon.lock);
> > - idr_init(&vc4file->perfmon.idr);
> > + idr_init_base(&vc4file->perfmon.idr, 1);
> > }
>
> Sounds like you should use VC4_PERFMONID_MIN instead of a magic 1 here.
Agreed. I will update and resend v2.
Thank you,
./drv
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: Use IS_ERR() instead of null pointer check
a6xx_gmu_get_mmio() never return null in case of error, but ERR_PTR(), so we should use IS_ERR() instead of null pointer check Signed-off-by: Wang Qing --- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index 491fee4..8c81a89 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -492,7 +492,7 @@ static void a6xx_gmu_rpmh_init(struct a6xx_gmu *gmu) void __iomem *seqptr = a6xx_gmu_get_mmio(pdev, "gmu_pdc_seq"); uint32_t pdc_address_offset; - if (!pdcptr || !seqptr) + if (IS_ERR(pdcptr) || IS_ERR(seqptr)) goto err; if (adreno_is_a618(adreno_gpu) || adreno_is_a640(adreno_gpu)) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/7] drm/lima: dev_pm_opp_put_*() accepts NULL argument
The dev_pm_opp_put_*() APIs now accepts a NULL opp_table pointer and so
there is no need for us to carry the extra check. Drop them.
Signed-off-by: Viresh Kumar
---
drivers/gpu/drm/lima/lima_devfreq.c | 13 -
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/lima/lima_devfreq.c
b/drivers/gpu/drm/lima/lima_devfreq.c
index bbe02817721b..e7b7b8dfd792 100644
--- a/drivers/gpu/drm/lima/lima_devfreq.c
+++ b/drivers/gpu/drm/lima/lima_devfreq.c
@@ -110,15 +110,10 @@ void lima_devfreq_fini(struct lima_device *ldev)
devfreq->opp_of_table_added = false;
}
- if (devfreq->regulators_opp_table) {
- dev_pm_opp_put_regulators(devfreq->regulators_opp_table);
- devfreq->regulators_opp_table = NULL;
- }
-
- if (devfreq->clkname_opp_table) {
- dev_pm_opp_put_clkname(devfreq->clkname_opp_table);
- devfreq->clkname_opp_table = NULL;
- }
+ dev_pm_opp_put_regulators(devfreq->regulators_opp_table);
+ dev_pm_opp_put_clkname(devfreq->clkname_opp_table);
+ devfreq->regulators_opp_table = NULL;
+ devfreq->clkname_opp_table = NULL;
}
int lima_devfreq_init(struct lima_device *ldev)
--
2.25.0.rc1.19.g042ed3e048af
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vmwgfx: replace idr_init() by idr_init_base()
idr_init() uses base 0 which is an invalid identifier for this driver.
The idr_alloc for this driver uses 1 as start value for ID range. The
new function idr_init_base allows IDR to set the ID lookup from base 1.
This avoids all lookups that otherwise starts from 0 since 0 is always
unused / available.
References: commit 6ce711f27500 ("idr: Make 1-based IDRs more efficient")
Signed-off-by: Deepak R Varma
---
drivers/gpu/drm/vmwgfx/ttm_object.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/ttm_object.c
b/drivers/gpu/drm/vmwgfx/ttm_object.c
index 16077785ad47..cbedcb043803 100644
--- a/drivers/gpu/drm/vmwgfx/ttm_object.c
+++ b/drivers/gpu/drm/vmwgfx/ttm_object.c
@@ -540,7 +540,7 @@ ttm_object_device_init(struct ttm_mem_global *mem_glob,
if (ret != 0)
goto out_no_object_hash;
- idr_init(&tdev->idr);
+ idr_init_base(&tdev->idr, 1);
tdev->ops = *ops;
tdev->dmabuf_release = tdev->ops.release;
tdev->ops.release = ttm_prime_dmabuf_release;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 31e3e5c9f362..a24d9363a17e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -675,7 +675,7 @@ static int vmw_driver_load(struct drm_device *dev, unsigned
long chipset)
spin_lock_init(&dev_priv->cursor_lock);
for (i = vmw_res_context; i < vmw_res_max; ++i) {
- idr_init(&dev_priv->res_idr[i]);
+ idr_init_base(&dev_priv->res_idr[i], 1);
INIT_LIST_HEAD(&dev_priv->res_lru[i]);
}
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/7] drm/panfrost: dev_pm_opp_put_*() accepts NULL argument
The dev_pm_opp_put_*() APIs now accepts a NULL opp_table pointer and so
there is no need for us to carry the extra check. Drop them.
Signed-off-by: Viresh Kumar
---
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index 8ab025d0035f..97b5abc7c188 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -170,10 +170,8 @@ void panfrost_devfreq_fini(struct panfrost_device *pfdev)
pfdevfreq->opp_of_table_added = false;
}
- if (pfdevfreq->regulators_opp_table) {
- dev_pm_opp_put_regulators(pfdevfreq->regulators_opp_table);
- pfdevfreq->regulators_opp_table = NULL;
- }
+ dev_pm_opp_put_regulators(pfdevfreq->regulators_opp_table);
+ pfdevfreq->regulators_opp_table = NULL;
}
void panfrost_devfreq_resume(struct panfrost_device *pfdev)
--
2.25.0.rc1.19.g042ed3e048af
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-misc-fixes
Hi Dave, Daniel, Here's this week round of fixes for drm-misc Thanks! Maxime drm-misc-fixes-2020-11-05: Some patches for vc4 to fix some resources cleanup issues, two fixes for panfrost for madvise and the shrinker and a constification of fonts structure The following changes since commit 3cea11cd5e3b00d91caf0b4730194039b45c5891: Linux 5.10-rc2 (2020-11-01 14:43:51 -0800) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2020-11-05 for you to fetch changes up to 9522750c66c689b739e151fcdf895420dc81efc0: Fonts: Replace discarded const qualifier (2020-11-03 10:51:34 +0100) Some patches for vc4 to fix some resources cleanup issues, two fixes for panfrost for madvise and the shrinker and a constification of fonts structure Boris Brezillon (1): drm/panfrost: Fix a deadlock between the shrinker and madvise path Lee Jones (1): Fonts: Replace discarded const qualifier Maxime Ripard (7): drm/vc4: bo: Add a managed action to cleanup the cache drm/vc4: drv: Use managed drm_mode_config_init drm/vc4: gem: Add a managed action to cleanup the job queue drm/vc4: Use the helper to retrieve vc4_dev when needed drm/vc4: Use devm_drm_dev_alloc drm/vc4: kms: Add functions to create the state objects drm/vc4: drv: Remove unused variable Steven Price (1): drm/panfrost: Fix module unload drivers/gpu/drm/panfrost/panfrost_drv.c | 5 +- drivers/gpu/drm/panfrost/panfrost_gem.c | 4 +- drivers/gpu/drm/panfrost/panfrost_gem.h | 2 +- drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c | 14 - drivers/gpu/drm/vc4/vc4_bo.c | 9 +-- drivers/gpu/drm/vc4/vc4_drv.c| 41 +--- drivers/gpu/drm/vc4/vc4_drv.h| 9 ++- drivers/gpu/drm/vc4/vc4_gem.c| 19 +++--- drivers/gpu/drm/vc4/vc4_hvs.c| 4 +- drivers/gpu/drm/vc4/vc4_kms.c| 80 +--- drivers/gpu/drm/vc4/vc4_v3d.c| 12 ++-- lib/fonts/font_10x18.c | 2 +- lib/fonts/font_6x10.c| 2 +- lib/fonts/font_6x11.c| 2 +- lib/fonts/font_6x8.c | 2 +- lib/fonts/font_7x14.c| 2 +- lib/fonts/font_8x16.c| 2 +- lib/fonts/font_8x8.c | 2 +- lib/fonts/font_acorn_8x8.c | 2 +- lib/fonts/font_mini_4x6.c| 2 +- lib/fonts/font_pearl_8x8.c | 2 +- lib/fonts/font_sun12x22.c| 2 +- lib/fonts/font_sun8x16.c | 2 +- lib/fonts/font_ter16x32.c| 2 +- 24 files changed, 128 insertions(+), 97 deletions(-) signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v8 3/5] RDMA/uverbs: Add uverbs command for dma-buf based MR registration
On Thu, Nov 05, 2020 at 02:48:07PM -0800, Jianxin Xiong wrote: > + ret = ib_check_mr_access(access_flags); > + if (ret) > + return ret; This should also reject unsupportable flags like ACCESS_ON_DEMAND and HUGETLB > + > + mr = pd->device->ops.reg_user_mr_dmabuf(pd, offset, length, virt_addr, > + fd, access_flags, > + &attrs->driver_udata); > + if (IS_ERR(mr)) > + return PTR_ERR(mr); > + > + mr->device = pd->device; > + mr->pd = pd; > + mr->type= IB_MR_TYPE_USER; > + mr->uobject = uobj; > + atomic_inc(&pd->usecnt); Fix intending when copying code please > + > + uobj->object = mr; Also bit surprised this works at all, it needs to call uverbs_finalize_uobj_create() Right here. Seems like the core code is missing some check that the API is being used properly. > + > + ret = uverbs_copy_to(attrs, UVERBS_ATTR_REG_DMABUF_MR_RESP_LKEY, > + &mr->lkey, sizeof(mr->lkey)); > + if (ret) > + goto err_dereg; > + > + ret = uverbs_copy_to(attrs, UVERBS_ATTR_REG_DMABUF_MR_RESP_RKEY, > + &mr->rkey, sizeof(mr->rkey)); > + if (ret) > + goto err_dereg; > + > + return 0; > + > +err_dereg: > + ib_dereg_mr_user(mr, uverbs_get_cleared_udata(attrs)); > + > + return ret; > +} > + > DECLARE_UVERBS_NAMED_METHOD( > UVERBS_METHOD_ADVISE_MR, > UVERBS_ATTR_IDR(UVERBS_ATTR_ADVISE_MR_PD_HANDLE, > @@ -253,6 +364,7 @@ static int UVERBS_HANDLER(UVERBS_METHOD_QUERY_MR)( > DECLARE_UVERBS_NAMED_OBJECT( > UVERBS_OBJECT_MR, > UVERBS_TYPE_ALLOC_IDR(uverbs_free_mr), > + &UVERBS_METHOD(UVERBS_METHOD_REG_DMABUF_MR), > &UVERBS_METHOD(UVERBS_METHOD_DM_MR_REG), > &UVERBS_METHOD(UVERBS_METHOD_MR_DESTROY), > &UVERBS_METHOD(UVERBS_METHOD_ADVISE_MR), I trie to keep these sorted, but I see it has become unsorted. You can re-sort it in this patch Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Pass the full state to connectors atomic functions
Hi
Am 05.11.20 um 18:30 schrieb Maxime Ripard:
> The current atomic helpers have either their object state being passed as
> an argument or the full atomic state.
>
> The former is the pattern that was done at first, before switching to the
> latter for new hooks or when it was needed.
>
> Now that the CRTCs have been converted, let's move forward with the
> connectors to provide a consistent interface.
>
> The conversion was done using the coccinelle script below, and built tested
> on all the drivers.
The coccinelle script appears to be missing. Apart from that
Acked-by: Thomas Zimmermann
>
> Cc: Harry Wentland
> Cc: Leo Li
> Cc: Alex Deucher
> Cc: "Christian König"
> Cc: Jani Nikula
> Cc: Joonas Lahtinen
> Cc: Rodrigo Vivi
> Cc: Ben Skeggs
> Cc: Rodrigo Siqueira
> Cc: Melissa Wen
> Cc: Haneen Mohammed
> Signed-off-by: Maxime Ripard
> ---
> .../drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 5 -
> drivers/gpu/drm/drm_atomic_helper.c | 8
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 7 +--
> drivers/gpu/drm/nouveau/dispnv50/disp.c | 5 -
> drivers/gpu/drm/vc4/vc4_txp.c | 4 +++-
> drivers/gpu/drm/vkms/vkms_writeback.c | 7 +--
> include/drm/drm_modeset_helper_vtables.h| 13 ++---
> 7 files changed, 31 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> index eee19ed5..f346cc74387f 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> @@ -24,6 +24,7 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> @@ -252,8 +253,10 @@ static int dm_dp_mst_get_modes(struct drm_connector
> *connector)
>
> static struct drm_encoder *
> dm_mst_atomic_best_encoder(struct drm_connector *connector,
> -struct drm_connector_state *connector_state)
> +struct drm_atomic_state *state)
> {
> + struct drm_connector_state *connector_state =
> drm_atomic_get_new_connector_state(state,
> +
> connector);
> struct drm_device *dev = connector->dev;
> struct amdgpu_device *adev = drm_to_adev(dev);
> struct amdgpu_crtc *acrtc = to_amdgpu_crtc(connector_state->crtc);
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index ddd0e3239150..ba1507036f26 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -122,7 +122,8 @@ static int handle_conflicting_encoders(struct
> drm_atomic_state *state,
> continue;
>
> if (funcs->atomic_best_encoder)
> - new_encoder = funcs->atomic_best_encoder(connector,
> new_conn_state);
> + new_encoder = funcs->atomic_best_encoder(connector,
> + state);
> else if (funcs->best_encoder)
> new_encoder = funcs->best_encoder(connector);
> else
> @@ -345,8 +346,7 @@ update_connector_routing(struct drm_atomic_state *state,
> funcs = connector->helper_private;
>
> if (funcs->atomic_best_encoder)
> - new_encoder = funcs->atomic_best_encoder(connector,
> - new_connector_state);
> + new_encoder = funcs->atomic_best_encoder(connector, state);
> else if (funcs->best_encoder)
> new_encoder = funcs->best_encoder(connector);
> else
> @@ -1313,7 +1313,7 @@ static void drm_atomic_helper_commit_writebacks(struct
> drm_device *dev,
>
> if (new_conn_state->writeback_job &&
> new_conn_state->writeback_job->fb) {
> WARN_ON(connector->connector_type !=
> DRM_MODE_CONNECTOR_WRITEBACK);
> - funcs->atomic_commit(connector, new_conn_state);
> + funcs->atomic_commit(connector, old_state);
> }
> }
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 64d885539e94..b879a0622ada 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -23,6 +23,7 @@
> *
> */
>
> +#include
> #include
> #include
> #include
> @@ -706,11 +707,13 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector
> *connector,
> }
>
> static struct drm_encoder *intel_mst_atomic_best_encoder(struct
> drm_connector *connector,
> - struct
> drm_connector_state *state)
> + struct
> drm_atomic_state *sta
Re: [PATCH 03/19] gpu: drm: imx: ipuv3-plane: Mark 'crtc_state' as __always_unused
On Fri, 06 Nov 2020, Ahmad Fatoum wrote: > On 11/6/20 8:41 AM, Lee Jones wrote: > > On Thu, 05 Nov 2020, Ahmad Fatoum wrote: > > > >> Hello Lee, > >> > >> On 11/5/20 3:45 PM, Lee Jones wrote: > >>> In the macro for_each_oldnew_crtc_in_state() 'crtc_state' is provided > >>> as a container for state->crtcs[i].new_state, but is not utilised in > >>> this use-case. We cannot simply delete the variable, so here we tell > >>> the compiler that we're intentionally discarding the read value. > >> > >> for_each_oldnew_crtc_in_state already (void) casts the drm_crtc and the old > >> drm_crtc_state to silence unused-but-set-variable warning. Should we maybe > >> (void) cast the new crtc_state as well? > > > > From what I saw, it only void casts the ones which aren't assigned. > > How do you mean? I wonder if > > #define for_each_oldnew_crtc_in_state(__state, crtc, old_crtc_state, > new_crtc_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_crtc; \ > (__i)++) \ > for_each_if ((__state)->crtcs[__i].ptr && \ > ((crtc) = (__state)->crtcs[__i].ptr, \ > (void)(crtc) /* Only to avoid > unused-but-set-variable warning */, \ > (old_crtc_state) = > (__state)->crtcs[__i].old_state, \ > (void)(old_crtc_state) /* Only to avoid > unused-but-set-variable warning */, \ > -(new_crtc_state) = > (__state)->crtcs[__i].new_state, 1)) > +(new_crtc_state) = > (__state)->crtcs[__i].new_state, \ > +(void)(new_crtc_state), 1)) > > wouldn't be better. That also works for me. I can fix this up. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH V3] drm/tegra: sor: Don't warn on probe deferral
Deferred probe is an expected return value for tegra_output_probe().
Given that the driver deals with it properly, there's no need to output
a warning that may potentially confuse users.
Signed-off-by: Jon Hunter
---
Changes since V2:
- Removed duplicate errno print
Changes since V1:
- This time, I actually validated it!
drivers/gpu/drm/tegra/sor.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index e88a17c2937f..a5b944dacb35 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -3764,10 +3764,9 @@ static int tegra_sor_probe(struct platform_device *pdev)
return err;
err = tegra_output_probe(&sor->output);
- if (err < 0) {
- dev_err(&pdev->dev, "failed to probe output: %d\n", err);
- return err;
- }
+ if (err < 0)
+ return dev_err_probe(&pdev->dev, err,
+"failed to probe output\n");
if (sor->ops && sor->ops->probe) {
err = sor->ops->probe(sor);
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/36] Rid W=1 issues from TTY
On Wed, Nov 04, 2020 at 07:35:13PM +, Lee Jones wrote: > This set is part of a larger effort attempting to clean-up W=1 > kernel builds, which are currently overwhelmingly riddled with > niggly little warnings. Many of these now applied, please update the series against my tty-testing branch and resend the rest. thanks, greg k-h ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] staging: fbtft: fb_watterott: fix usleep_range is preferred over udelay
On Sun, Nov 01, 2020 at 12:32:44PM +0200, Hassan Shahbazi wrote: > On Sun, Nov 01, 2020 at 07:39:48AM +0100, Greg KH wrote: > > On Sun, Nov 01, 2020 at 02:20:10AM +0200, Hassan Shahbazi wrote: > > > Fix the checkpath.pl issue on fb_watterott.c. write_vmem and > > > write_vmem_8bit functions are within non-atomic context and can > > > safely use usleep_range. > > > see Documentation/timers/timers-howto.txt > > > > > > Signed-off-by: Hassan Shahbazi > > > --- > > > drivers/staging/fbtft/fb_watterott.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/staging/fbtft/fb_watterott.c > > > b/drivers/staging/fbtft/fb_watterott.c > > > index 76b25df376b8..afcc86a17995 100644 > > > --- a/drivers/staging/fbtft/fb_watterott.c > > > +++ b/drivers/staging/fbtft/fb_watterott.c > > > @@ -84,7 +84,7 @@ static int write_vmem(struct fbtft_par *par, size_t > > > offset, size_t len) > > > par->txbuf.buf, 10 + par->info->fix.line_length); > > > if (ret < 0) > > > return ret; > > > - udelay(300); > > > + usleep_range(300, 310); > > > } > > > > > > return 0; > > > @@ -124,7 +124,7 @@ static int write_vmem_8bit(struct fbtft_par *par, > > > size_t offset, size_t len) > > > par->txbuf.buf, 10 + par->info->var.xres); > > > if (ret < 0) > > > return ret; > > > - udelay(700); > > > + usleep_range(700, 710); > > > > How do you know that these ranges are ok? Are you able to test these > > changes with real hardware? > > > > thanks, > > > > greg k-h > > No, I don't have the hardware to test with. I just used the current > value as the minimum and added an epsilon to it for the maximum > param. It's best not to guess about this, sorry, you should have the hardware to test this type of change. thanks, greg k-h ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 05/15] mm/frame-vector: Use FOLL_LONGTERM
On Fri, Nov 6, 2020 at 5:08 AM John Hubbard wrote: > > On 11/5/20 4:49 AM, Jason Gunthorpe wrote: > > On Thu, Nov 05, 2020 at 10:25:24AM +0100, Daniel Vetter wrote: > >>> /* > >>> * If we can't determine whether or not a pte is special, then fail > >>> immediately > >>> * for ptes. Note, we can still pin HugeTLB and THP as these are > >>> guaranteed not > >>> * to be special. > >>> * > >>> * For a futex to be placed on a THP tail page, get_futex_key requires a > >>> * get_user_pages_fast_only implementation that can pin pages. Thus it's > >>> still > >>> * useful to have gup_huge_pmd even if we can't operate on ptes. > >>> */ > >> > >> We support hugepage faults in gpu drivers since recently, and I'm not > >> seeing a pud_mkhugespecial anywhere. So not sure this works, but probably > >> just me missing something again. > > > > It means ioremap can't create an IO page PUD, it has to be broken up. > > > > Does ioremap even create anything larger than PTEs? gpu drivers also tend to use vmf_insert_pfn* directly, so we can do on-demand paging and move buffers around. From what I glanced for lowest level we to the pte_mkspecial correctly (I think I convinced myself that vm_insert_pfn does that), but for pud/pmd levels it seems just yolo. remap_pfn_range seems to indeed split down to pte level always. > From my reading, yes. See ioremap_try_huge_pmd(). The ioremap here shouldn't matter, since this is for kernel-internal mappings. So that's all fine I think. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 05/15] mm/frame-vector: Use FOLL_LONGTERM
On Fri, Nov 6, 2020 at 11:01 AM Daniel Vetter wrote: > > On Fri, Nov 6, 2020 at 5:08 AM John Hubbard wrote: > > > > On 11/5/20 4:49 AM, Jason Gunthorpe wrote: > > > On Thu, Nov 05, 2020 at 10:25:24AM +0100, Daniel Vetter wrote: > > >>> /* > > >>> * If we can't determine whether or not a pte is special, then fail > > >>> immediately > > >>> * for ptes. Note, we can still pin HugeTLB and THP as these are > > >>> guaranteed not > > >>> * to be special. > > >>> * > > >>> * For a futex to be placed on a THP tail page, get_futex_key requires > > >>> a > > >>> * get_user_pages_fast_only implementation that can pin pages. Thus > > >>> it's still > > >>> * useful to have gup_huge_pmd even if we can't operate on ptes. > > >>> */ > > >> > > >> We support hugepage faults in gpu drivers since recently, and I'm not > > >> seeing a pud_mkhugespecial anywhere. So not sure this works, but probably > > >> just me missing something again. > > > > > > It means ioremap can't create an IO page PUD, it has to be broken up. > > > > > > Does ioremap even create anything larger than PTEs? > > gpu drivers also tend to use vmf_insert_pfn* directly, so we can do > on-demand paging and move buffers around. From what I glanced for > lowest level we to the pte_mkspecial correctly (I think I convinced > myself that vm_insert_pfn does that), but for pud/pmd levels it seems > just yolo. So I dug around a bit more and ttm sets PFN_DEV | PFN_MAP to get past the various pft_t_devmap checks (see e.g. vmf_insert_pfn_pmd_prot()). x86-64 has ARCH_HAS_PTE_DEVMAP, and gup.c seems to handle these specially, but frankly I got totally lost in what this does. The comment above the pfn_t_devmap check makes me wonder whether doing this is correct or not. Also adding Thomas Hellstrom, who implemented the huge map support in ttm. -Daniel > remap_pfn_range seems to indeed split down to pte level always. > > > From my reading, yes. See ioremap_try_huge_pmd(). > > The ioremap here shouldn't matter, since this is for kernel-internal > mappings. So that's all fine I think. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] console: Remove dummy con_font_op() callback implementations
On Sat, Oct 31, 2020 at 03:24:41AM -0400, Peilin Ye wrote: > `struct console_font` is a UAPI structure, thus ideally should not be > used for kernel internal abstraction. Remove some dummy .con_font_set, > .con_font_default and .con_font_copy `struct consw` callback > implementations, to make it cleaner. > > Patch "fbcon: Prevent global-out-of-bounds read in fbcon_copy_font()" > depends on this patch, so Cc: stable. > > Cc: sta...@vger.kernel.org > Suggested-by: Daniel Vetter > Signed-off-by: Peilin Ye > --- > Context: > https://lore.kernel.org/lkml/CAKMK7uFY2zv0adjKJ_ORVFT7Zzwn075MaU0rEU7_FuqENLR=u...@mail.gmail.com/ > > drivers/usb/misc/sisusbvga/sisusb_con.c | 21 - > drivers/video/console/dummycon.c| 20 > 2 files changed, 41 deletions(-) Reviewed-by: Greg Kroah-Hartman ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 12/16] drm/i915/hdcp: MST streams support in hdcp port_data
On 2020-11-06 at 12:05:14 +0530, Anshuman Gupta wrote:
> On 2020-11-05 at 22:04:15 +0530, Ramalingam C wrote:
> > On 2020-10-27 at 22:12:04 +0530, Anshuman Gupta wrote:
> > > Add support for multiple mst stream in hdcp port data
> > > which will be used by RepeaterAuthStreamManage msg and
> > > HDCP 2.2 security f/w for m' validation.
> > >
> > > v2:
> > > Init the hdcp port data k for HDMI/DP SST strem.
> > >
> > > v3:
> > > Cosmetic changes. [Uma]
> > >
> > > Cc: Ramalingam C
> > > Signed-off-by: Anshuman Gupta
> > > ---
> > > .../drm/i915/display/intel_display_types.h| 4 +-
> > > drivers/gpu/drm/i915/display/intel_hdcp.c | 103 +++---
> > > 2 files changed, 92 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > index 749c3a7e0b45..24e0067c2e7c 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > @@ -1445,10 +1445,12 @@ struct intel_digital_port {
> > > enum phy_fia tc_phy_fia;
> > > u8 tc_phy_fia_idx;
> > >
> > > - /* protects num_hdcp_streams reference count, port_data */
> > > + /* protects num_hdcp_streams reference count, port_data and port_auth */
> > > struct mutex hdcp_mutex;
> > > /* the number of pipes using HDCP signalling out of this port */
> > > unsigned int num_hdcp_streams;
> > > + /* port HDCP auth status */
> > > + bool port_auth;
> > > /* HDCP port data need to pass to security f/w */
> > > struct hdcp_port_data port_data;
> > since this is no more in hdcp related struct, it will be better to be named
> > as
> > hdcp_auth_status and hdcp_port_data !?
> sure i will do this chnage.
> Thanks,
> Anshuman Gupta.
> >
> > -Ram
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > index a5ec4f72f50f..1df6d4a23476 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > @@ -26,6 +26,64 @@
> > > #define KEY_LOAD_TRIES 5
> > > #define HDCP2_LC_RETRY_CNT 3
> > >
> > > +static int intel_conn_to_vcpi(struct intel_connector *connector)
> > > +{
> > > + /* For HDMI this is forced to be 0x0. For DP SST also this is 0x0. */
> > > + return connector->port ? connector->port->vcpi.vcpi : 0;
> > > +}
> > > +
> > > +static int
> > > +intel_hdcp_required_content_stream(struct intel_digital_port *dig_port)
> > > +{
> > > + struct drm_connector_list_iter conn_iter;
> > > + struct intel_digital_port *conn_dig_port;
> > > + struct intel_connector *connector;
> > > + struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> > > + struct hdcp_port_data *data = &dig_port->port_data;
> > > + bool enforce_type0 = false;
> > > + int k;
> > > +
> > > + if (dig_port->port_auth)
> > > + return 0;
> > > +
> > > + drm_connector_list_iter_begin(&i915->drm, &conn_iter);
> > > + for_each_intel_connector_iter(connector, &conn_iter) {
> > > + if (!intel_encoder_is_mst(intel_attached_encoder(connector)))
> > > + continue;
> > > +
> > > + conn_dig_port = intel_attached_dig_port(connector);
> > > + if (conn_dig_port != dig_port)
> > > + continue;
> > > +
> > > + if (connector->base.status == connector_status_disconnected)
> > > + continue;
> > > +
> > > + if (!enforce_type0 && !intel_hdcp2_capable(connector))
> > > + enforce_type0 = true;
> > > +
> > > + data->streams[data->k].stream_id =
> > > intel_conn_to_vcpi(connector);
> > > + data->k++;
> > > +
> > > + /* if there is only one active stream */
> > > + if (dig_port->dp.active_mst_links <= 1)
> > > + break;
> > > + }
> > > + drm_connector_list_iter_end(&conn_iter);
> > > +
> > > + if (drm_WARN_ON(&i915->drm, data->k > INTEL_NUM_PIPES(i915) || data->k
> > > == 0))
> > > + return -EINVAL;
> > > +
> > > + /*
> > > + * Apply common protection level across all streams in DP MST Topology.
> > > + * Use highest supported content type for all streams in DP MST
> > > Topology.
> > > + */
> > > + for (k = 0; k < data->k; k++)
> > > + data->streams[k].stream_type =
> > > + enforce_type0 ? DRM_MODE_HDCP_CONTENT_TYPE0 :
> > > DRM_MODE_HDCP_CONTENT_TYPE1;
> > > +
> > > + return 0;
> > > +}
This will force userspace to use most highest common possible HDCP type
on all streams. On MST if one sink is HDCP2.2 capable and where others
are not, this will prohibit the owner of the stream corresponding to the
HDCP2.2 capable sink from using the content type1, just bacuse other
sinks are not HDCP2.2 capable. ME FW should change its rules and allow
the different content type on different streams. And also dynamic update
of the stream type support will be ideal.
-Ram
> > > +
> > > st
Re: [PATCH v4 09/16] drm/i915/hdcp: Encapsulate hdcp_port_data to dig_port
On 2020-10-27 at 22:12:01 +0530, Anshuman Gupta wrote:
> hdcp_port_data is specific to a port on which HDCP
> encryption is getting enabled, so encapsulate it to
> intel_digital_port.
> This will be required to enable HDCP 2.2 stream encryption.
>
> Cc: Ramalingam C
> Reviewed-by: Uma Shankar
> Signed-off-by: Anshuman Gupta
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 2 +
> .../drm/i915/display/intel_display_types.h| 5 +-
> drivers/gpu/drm/i915/display/intel_hdcp.c | 56 +++
> 3 files changed, 39 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 779603a38cfc..1bc6cf0b83ec 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4746,6 +4746,8 @@ static void intel_ddi_encoder_destroy(struct
> drm_encoder *encoder)
> intel_dp_encoder_flush_work(encoder);
>
> drm_encoder_cleanup(encoder);
> + if (dig_port)
> + kfree(dig_port->port_data.streams);
> kfree(dig_port);
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 59b8fc21e3e8..749c3a7e0b45 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -402,7 +402,6 @@ struct intel_hdcp {
>* content can flow only through a link protected by HDCP2.2.
>*/
> u8 content_type;
> - struct hdcp_port_data port_data;
>
> bool is_paired;
> bool is_repeater;
> @@ -1446,10 +1445,12 @@ struct intel_digital_port {
> enum phy_fia tc_phy_fia;
> u8 tc_phy_fia_idx;
>
> - /* protects num_hdcp_streams reference count */
> + /* protects num_hdcp_streams reference count, port_data */
> struct mutex hdcp_mutex;
> /* the number of pipes using HDCP signalling out of this port */
> unsigned int num_hdcp_streams;
> + /* HDCP port data need to pass to security f/w */
> + struct hdcp_port_data port_data;
Since this is outside intel_hdcp, better ad prefix of hdcp_
With that addressed
Reviewed-by: Ramalingam C
>
> void (*write_infoframe)(struct intel_encoder *encoder,
> const struct intel_crtc_state *crtc_state,
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index b0f47687bc59..a5ec4f72f50f 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -15,6 +15,7 @@
> #include
> #include
>
> +#include "i915_drv.h"
> #include "i915_reg.h"
> #include "intel_display_power.h"
> #include "intel_display_types.h"
> @@ -1028,7 +1029,8 @@ static int
> hdcp2_prepare_ake_init(struct intel_connector *connector,
> struct hdcp2_ake_init *ake_data)
> {
> - struct hdcp_port_data *data = &connector->hdcp.port_data;
> + struct intel_digital_port *dig_port =
> intel_attached_dig_port(connector);
> + struct hdcp_port_data *data = &dig_port->port_data;
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> struct i915_hdcp_comp_master *comp;
> int ret;
> @@ -1057,7 +1059,8 @@ hdcp2_verify_rx_cert_prepare_km(struct intel_connector
> *connector,
> struct hdcp2_ake_no_stored_km *ek_pub_km,
> size_t *msg_sz)
> {
> - struct hdcp_port_data *data = &connector->hdcp.port_data;
> + struct intel_digital_port *dig_port =
> intel_attached_dig_port(connector);
> + struct hdcp_port_data *data = &dig_port->port_data;
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> struct i915_hdcp_comp_master *comp;
> int ret;
> @@ -1084,7 +1087,8 @@ hdcp2_verify_rx_cert_prepare_km(struct intel_connector
> *connector,
> static int hdcp2_verify_hprime(struct intel_connector *connector,
> struct hdcp2_ake_send_hprime *rx_hprime)
> {
> - struct hdcp_port_data *data = &connector->hdcp.port_data;
> + struct intel_digital_port *dig_port =
> intel_attached_dig_port(connector);
> + struct hdcp_port_data *data = &dig_port->port_data;
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> struct i915_hdcp_comp_master *comp;
> int ret;
> @@ -1109,7 +1113,8 @@ static int
> hdcp2_store_pairing_info(struct intel_connector *connector,
>struct hdcp2_ake_send_pairing_info *pairing_info)
> {
> - struct hdcp_port_data *data = &connector->hdcp.port_data;
> + struct intel_digital_port *dig_port =
> intel_attached_dig_port(connector);
> + struct hdcp_port_data *data = &dig_port->port_data;
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> struct i915_hdcp_comp_master *comp;
> int ret;
> @@ -1135,7 +1140,8 @@ stat
cleanup a fix and add the vma_set_file function
Hi Andrew, can I get you Acked-by to merge this cleanup through the drm-misc-next branch? The affected drivers are mostly from the DRM subsystem. The fix for the other problem you pointed out in mmap_region() has already shown up in that branch. Thanks in advance, Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] mm: mmap: fix fput in error path v2
Patch "495c10cc1c0c CHROMIUM: dma-buf: restore args..."
adds a workaround for a bug in mmap_region.
As the comment states ->mmap() callback can change
vma->vm_file and so we might call fput() on the wrong file.
Revert the workaround and proper fix this in mmap_region.
v2: drop the extra if in dma_buf_mmap as well
Signed-off-by: Christian König
Reviewed-by: Jason Gunthorpe
---
drivers/dma-buf/dma-buf.c | 20 +++-
mm/mmap.c | 2 +-
2 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 0eb80c1ecdab..282bd8b84170 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -1166,9 +1166,6 @@ EXPORT_SYMBOL_GPL(dma_buf_end_cpu_access);
int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma,
unsigned long pgoff)
{
- struct file *oldfile;
- int ret;
-
if (WARN_ON(!dmabuf || !vma))
return -EINVAL;
@@ -1186,22 +1183,11 @@ int dma_buf_mmap(struct dma_buf *dmabuf, struct
vm_area_struct *vma,
return -EINVAL;
/* readjust the vma */
- get_file(dmabuf->file);
- oldfile = vma->vm_file;
- vma->vm_file = dmabuf->file;
+ fput(vma->vm_file);
+ vma->vm_file = get_file(dmabuf->file);
vma->vm_pgoff = pgoff;
- ret = dmabuf->ops->mmap(dmabuf, vma);
- if (ret) {
- /* restore old parameters on failure */
- vma->vm_file = oldfile;
- fput(dmabuf->file);
- } else {
- if (oldfile)
- fput(oldfile);
- }
- return ret;
-
+ return dmabuf->ops->mmap(dmabuf, vma);
}
EXPORT_SYMBOL_GPL(dma_buf_mmap);
diff --git a/mm/mmap.c b/mm/mmap.c
index d91ecb00d38c..30a4e8412a58 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1899,8 +1899,8 @@ unsigned long mmap_region(struct file *file, unsigned
long addr,
return addr;
unmap_and_free_vma:
+ fput(vma->vm_file);
vma->vm_file = NULL;
- fput(file);
/* Undo any partial mapping done by a device driver. */
unmap_region(mm, vma, prev, vma->vm_start, vma->vm_end);
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] mm: introduce vma_set_file function v5
Add the new vma_set_file() function to allow changing
vma->vm_file with the necessary refcount dance.
v2: add more users of this.
v3: add missing EXPORT_SYMBOL, rebase on mmap cleanup,
add comments why we drop the reference on two occasions.
v4: make it clear that changing an anonymous vma is illegal.
v5: move vma_set_file to mm/util.c
Signed-off-by: Christian König
Reviewed-by: Daniel Vetter (v2)
Reviewed-by: Jason Gunthorpe
---
drivers/dma-buf/dma-buf.c | 3 +--
drivers/gpu/drm/etnaviv/etnaviv_gem.c | 4 +---
drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 3 +--
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 5 +++--
drivers/gpu/drm/msm/msm_gem.c | 4 +---
drivers/gpu/drm/omapdrm/omap_gem.c | 3 +--
drivers/gpu/drm/vgem/vgem_drv.c| 3 +--
drivers/staging/android/ashmem.c | 6 +++---
include/linux/mm.h | 2 ++
mm/util.c | 12
10 files changed, 26 insertions(+), 19 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 282bd8b84170..e63684d4cd90 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -1183,8 +1183,7 @@ int dma_buf_mmap(struct dma_buf *dmabuf, struct
vm_area_struct *vma,
return -EINVAL;
/* readjust the vma */
- fput(vma->vm_file);
- vma->vm_file = get_file(dmabuf->file);
+ vma_set_file(vma, dmabuf->file);
vma->vm_pgoff = pgoff;
return dmabuf->ops->mmap(dmabuf, vma);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index 67d9a2b9ea6a..4132acfa11be 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -145,10 +145,8 @@ static int etnaviv_gem_mmap_obj(struct etnaviv_gem_object
*etnaviv_obj,
* address_space (so unmap_mapping_range does what we want,
* in particular in the case of mmap'd dmabufs)
*/
- fput(vma->vm_file);
- get_file(etnaviv_obj->base.filp);
vma->vm_pgoff = 0;
- vma->vm_file = etnaviv_obj->base.filp;
+ vma_set_file(vma, etnaviv_obj->base.filp);
vma->vm_page_prot = vm_page_prot;
}
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
index 0dd477e56573..04e9c04545ad 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
@@ -114,8 +114,7 @@ static int i915_gem_dmabuf_mmap(struct dma_buf *dma_buf,
struct vm_area_struct *
if (ret)
return ret;
- fput(vma->vm_file);
- vma->vm_file = get_file(obj->base.filp);
+ vma_set_file(vma, obj->base.filp);
return 0;
}
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 3d69e51f3e4d..ec28a6cde49b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -893,8 +893,9 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct
*vma)
* requires avoiding extraneous references to their filp, hence why
* we prefer to use an anonymous file for their mmaps.
*/
- fput(vma->vm_file);
- vma->vm_file = anon;
+ vma_set_file(vma, anon);
+ /* Drop the initial creation reference, the vma is now holding one. */
+ fput(anon);
switch (mmo->mmap_type) {
case I915_MMAP_TYPE_WC:
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 2e1bce7c0b19..311721ceee50 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -212,10 +212,8 @@ int msm_gem_mmap_obj(struct drm_gem_object *obj,
* address_space (so unmap_mapping_range does what we want,
* in particular in the case of mmap'd dmabufs)
*/
- fput(vma->vm_file);
- get_file(obj->filp);
vma->vm_pgoff = 0;
- vma->vm_file = obj->filp;
+ vma_set_file(vma, obj->filp);
vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
}
diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c
b/drivers/gpu/drm/omapdrm/omap_gem.c
index d8e09792793a..f063f5a04fb0 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -564,9 +564,8 @@ int omap_gem_mmap_obj(struct drm_gem_object *obj,
* address_space (so unmap_mapping_range does what we want,
* in particular in the case of mmap'd dmabufs)
*/
- fput(vma->vm_file);
vma->vm_pgoff = 0;
- vma->vm_file = get_file(obj->filp);
+ vma_set_file(vma, obj->filp);
vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
}
diff --git a/driver
Re: [PATCH v4 16/16] drm/i915/hdcp: Enable HDCP 2.2 MST support
On 2020-10-27 at 22:12:08 +0530, Anshuman Gupta wrote:
> Enable HDCP 2.2 over DP MST.
> Authenticate and enable port encryption only once for
> an active HDCP 2.2 session, once port is authenticated
> and encrypted enable encryption for each stream that
> requires encryption on this port.
>
> Similarly disable the stream encryption for each encrypted
> stream, once all encrypted stream encryption is disabled,
> disable the port HDCP encryption and deauthenticate the port.
>
> Cc: Ramalingam C
> Reviewed-by: Uma Shankar
> Signed-off-by: Anshuman Gupta
> ---
> drivers/gpu/drm/i915/display/intel_hdcp.c | 46 ++-
> 1 file changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 87f7aaf3a319..71fd01bf63a6 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -1693,6 +1693,32 @@ static int hdcp2_authenticate_sink(struct
> intel_connector *connector)
> return ret;
> }
>
> +static int hdcp2_enable_stream_encryption(struct intel_connector *connector)
> +{
> + struct intel_digital_port *dig_port =
> intel_attached_dig_port(connector);
> + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> + struct intel_hdcp *hdcp = &connector->hdcp;
> + enum transcoder cpu_transcoder = hdcp->cpu_transcoder;
> + enum port port = dig_port->base.port;
> + int ret = 0;
> +
> + if (!(intel_de_read(dev_priv, HDCP2_STATUS(dev_priv, cpu_transcoder,
> port)) &
> + LINK_ENCRYPTION_STATUS)) {
> + drm_err(&dev_priv->drm, "HDCP 2.2 Link is not encrypted\n");
Provide the connector details in err.
> + return -EPERM;
> + }
> +
> + if (hdcp->shim->stream_2_2_encryption) {
> + ret = hdcp->shim->stream_2_2_encryption(dig_port, true);
> + if (ret) {
> + drm_err(&dev_priv->drm, "Failed to enable HDCP 2.2
> stream enc\n");
connector details.
> + return ret;
> + }
> + }
> +
> + return ret;
> +}
> +
> static int hdcp2_enable_encryption(struct intel_connector *connector)
> {
> struct intel_digital_port *dig_port =
> intel_attached_dig_port(connector);
> @@ -1831,7 +1857,7 @@ static int hdcp2_authenticate_and_encrypt(struct
> intel_connector *connector)
> drm_dbg_kms(&i915->drm, "Port deauth failed.\n");
> }
>
> - if (!ret) {
> + if (!ret && !dig_port->port_auth) {
> /*
>* Ensuring the required 200mSec min time interval between
>* Session Key Exchange and encryption.
> @@ -1846,6 +1872,8 @@ static int hdcp2_authenticate_and_encrypt(struct
> intel_connector *connector)
> }
> }
>
> + ret = hdcp2_enable_stream_encryption(connector);
> +
> return ret;
> }
>
> @@ -1891,11 +1919,23 @@ static int _intel_hdcp2_disable(struct
> intel_connector *connector)
> struct intel_digital_port *dig_port =
> intel_attached_dig_port(connector);
> struct drm_i915_private *i915 = to_i915(connector->base.dev);
> struct hdcp_port_data *data = &dig_port->port_data;
> + struct intel_hdcp *hdcp = &connector->hdcp;
> int ret;
>
> drm_dbg_kms(&i915->drm, "[%s:%d] HDCP2.2 is being Disabled\n",
> connector->base.name, connector->base.base.id);
>
> + if (hdcp->shim->stream_2_2_encryption) {
> + ret = hdcp->shim->stream_2_2_encryption(dig_port, false);
> + if (ret) {
> + drm_err(&i915->drm, "Failed to disable HDCP 2.2 stream
> enc\n");
connector details in err.
-Ram
> + return ret;
> + }
> + }
> +
> + if (dig_port->num_hdcp_streams > 0)
> + return ret;
> +
> ret = hdcp2_disable_encryption(connector);
>
> if (hdcp2_deauthenticate_port(connector) < 0)
> @@ -1919,6 +1959,7 @@ static int intel_hdcp2_check_link(struct
> intel_connector *connector)
> int ret = 0;
>
> mutex_lock(&hdcp->mutex);
> + mutex_lock(&dig_port->hdcp_mutex);
> cpu_transcoder = hdcp->cpu_transcoder;
>
> /* hdcp2_check_link is expected only when HDCP2.2 is Enabled */
> @@ -1996,6 +2037,7 @@ static int intel_hdcp2_check_link(struct
> intel_connector *connector)
> }
>
> out:
> + mutex_unlock(&dig_port->hdcp_mutex);
> mutex_unlock(&hdcp->mutex);
> return ret;
> }
> @@ -2177,7 +2219,7 @@ int intel_hdcp_init(struct intel_connector *connector,
> if (!shim)
> return -EINVAL;
>
> - if (is_hdcp2_supported(dev_priv) && !connector->mst_port)
> + if (is_hdcp2_supported(dev_priv))
> intel_hdcp2_init(connector, dig_port, shim);
>
> ret =
> --
> 2.26.2
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
h
Re: [PATCH v4 03/16] drm/i915/hotplug: Handle CP_IRQ for DP-MST
On 2020-10-27 at 22:11:55 +0530, Anshuman Gupta wrote:
> Handle CP_IRQ in DEVICE_SERVICE_IRQ_VECTOR_ESI0
> It requires to call intel_hdcp_handle_cp_irq() in case
> of CP_IRQ is triggered by a sink in DP-MST topology.
>
> Cc: "Ville Syrjälä"
> Cc: Ramalingam C
> Reviewed-by: Uma Shankar
Reviewed-by: Ramalingam C
> Signed-off-by: Anshuman Gupta
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 14 +-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 818daab252f3..21c6c9828cd7 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5657,6 +5657,17 @@ static void intel_dp_handle_test_request(struct
> intel_dp *intel_dp)
> "Could not write test response to sink\n");
> }
>
> +static void
> +intel_dp_mst_hpd_irq(struct intel_dp *intel_dp, u8 *esi, bool *handled)
> +{
> + drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, handled);
> +
> + if (esi[1] & DP_CP_IRQ) {
> + intel_hdcp_handle_cp_irq(intel_dp->attached_connector);
> + *handled = true;
> + }
> +}
> +
> /**
> * intel_dp_check_mst_status - service any pending MST interrupts, check
> link status
> * @intel_dp: Intel DP struct
> @@ -5701,7 +5712,8 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
>
> drm_dbg_kms(&i915->drm, "got esi %3ph\n", esi);
>
> - drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled);
> + intel_dp_mst_hpd_irq(intel_dp, esi, &handled);
> +
> if (!handled)
> break;
>
> --
> 2.26.2
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/gma500: Remove unused function psb_gem_get_aperture()
Apparently, the function was never used at all.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/gma500/gem.c | 6 --
drivers/gpu/drm/gma500/psb_drv.h | 2 --
2 files changed, 8 deletions(-)
diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
index 8f07de83b6fb..db827e591403 100644
--- a/drivers/gpu/drm/gma500/gem.c
+++ b/drivers/gpu/drm/gma500/gem.c
@@ -32,12 +32,6 @@ static void psb_gem_free_object(struct drm_gem_object *obj)
psb_gtt_free_range(obj->dev, gtt);
}
-int psb_gem_get_aperture(struct drm_device *dev, void *data,
- struct drm_file *file)
-{
- return -EINVAL;
-}
-
static const struct vm_operations_struct psb_gem_vm_ops = {
.fault = psb_gem_fault,
.open = drm_gem_vm_open,
diff --git a/drivers/gpu/drm/gma500/psb_drv.h b/drivers/gpu/drm/gma500/psb_drv.h
index c71a5a4e912c..ce6aae4b1bb2 100644
--- a/drivers/gpu/drm/gma500/psb_drv.h
+++ b/drivers/gpu/drm/gma500/psb_drv.h
@@ -735,8 +735,6 @@ extern const struct drm_connector_helper_funcs
extern const struct drm_connector_funcs psb_intel_lvds_connector_funcs;
/* gem.c */
-extern int psb_gem_get_aperture(struct drm_device *dev, void *data,
- struct drm_file *file);
extern int psb_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
struct drm_mode_create_dumb *args);
--
2.29.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/cma-helper: Make default object functions the default
As GEM object functions are now mandatory, DRM drivers based on CMA
helpers either set them in their implementation of gem_create_object,
or use the default via drm_gem_cma_create_object_default_funcs().
Simplify this by setting the default CMA object functions for all
objects that don't have any functions of their own. Follows the pattern
of similar code in SHMEM and VRAM helpers. The function
drm_gem_cma_create_object_default_funcs() is redundant and therefore
being removed.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_gem_cma_helper.c | 47 +++-
drivers/gpu/drm/pl111/pl111_drv.c| 1 -
include/drm/drm_gem_cma_helper.h | 5 ---
3 files changed, 12 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c
b/drivers/gpu/drm/drm_gem_cma_helper.c
index 2165633c9b9e..c3b31b3369c3 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -33,6 +33,14 @@
* display drivers that are unable to map scattered buffers via an IOMMU.
*/
+static const struct drm_gem_object_funcs drm_gem_cma_default_funcs = {
+ .free = drm_gem_cma_free_object,
+ .print_info = drm_gem_cma_print_info,
+ .get_sg_table = drm_gem_cma_prime_get_sg_table,
+ .vmap = drm_gem_cma_prime_vmap,
+ .vm_ops = &drm_gem_cma_vm_ops,
+};
+
/**
* __drm_gem_cma_create - Create a GEM CMA object without allocating memory
* @drm: DRM device
@@ -58,6 +66,10 @@ __drm_gem_cma_create(struct drm_device *drm, size_t size)
gem_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
if (!gem_obj)
return ERR_PTR(-ENOMEM);
+
+ if (!gem_obj->funcs)
+ gem_obj->funcs = &drm_gem_cma_default_funcs;
+
cma_obj = container_of(gem_obj, struct drm_gem_cma_object, base);
ret = drm_gem_object_init(drm, gem_obj, size);
@@ -554,41 +566,6 @@ void drm_gem_cma_prime_vunmap(struct drm_gem_object *obj,
void *vaddr)
}
EXPORT_SYMBOL_GPL(drm_gem_cma_prime_vunmap);
-static const struct drm_gem_object_funcs drm_gem_cma_default_funcs = {
- .free = drm_gem_cma_free_object,
- .print_info = drm_gem_cma_print_info,
- .get_sg_table = drm_gem_cma_prime_get_sg_table,
- .vmap = drm_gem_cma_prime_vmap,
- .vm_ops = &drm_gem_cma_vm_ops,
-};
-
-/**
- * drm_gem_cma_create_object_default_funcs - Create a CMA GEM object with a
- * default function table
- * @dev: DRM device
- * @size: Size of the object to allocate
- *
- * This sets the GEM object functions to the default CMA helper functions.
- * This function can be used as the &drm_driver.gem_create_object callback.
- *
- * Returns:
- * A pointer to a allocated GEM object or an error pointer on failure.
- */
-struct drm_gem_object *
-drm_gem_cma_create_object_default_funcs(struct drm_device *dev, size_t size)
-{
- struct drm_gem_cma_object *cma_obj;
-
- cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
- if (!cma_obj)
- return NULL;
-
- cma_obj->base.funcs = &drm_gem_cma_default_funcs;
-
- return &cma_obj->base;
-}
-EXPORT_SYMBOL(drm_gem_cma_create_object_default_funcs);
-
/**
* drm_gem_cma_prime_import_sg_table_vmap - PRIME import another driver's
* scatter/gather table and get the virtual address of the buffer
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c
b/drivers/gpu/drm/pl111/pl111_drv.c
index ecef8a2383d2..fcb5caea7b47 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -224,7 +224,6 @@ static struct drm_driver pl111_drm_driver = {
.major = 1,
.minor = 0,
.patchlevel = 0,
- .gem_create_object = drm_gem_cma_create_object_default_funcs,
.dumb_create = drm_gem_cma_dumb_create,
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h
index 2bfa2502607a..60aba1e734c0 100644
--- a/include/drm/drm_gem_cma_helper.h
+++ b/include/drm/drm_gem_cma_helper.h
@@ -106,9 +106,6 @@ int drm_gem_cma_prime_mmap(struct drm_gem_object *obj,
void *drm_gem_cma_prime_vmap(struct drm_gem_object *obj);
void drm_gem_cma_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
-struct drm_gem_object *
-drm_gem_cma_create_object_default_funcs(struct drm_device *dev, size_t size);
-
/**
* DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE - CMA GEM driver operations
* @dumb_create_func: callback function for .dumb_create
@@ -123,7 +120,6 @@ drm_gem_cma_create_object_default_funcs(struct drm_device
*dev, size_t size);
* DRM_GEM_CMA_DRIVER_OPS_VMAP_WITH_DUMB_CREATE() instead.
*/
#define DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE(dumb_create_func) \
- .gem_create_object = drm_gem_cma_create_object_default_funcs, \
.dumb_create= (dumb_create_func), \
.prime_handle_to_fd = drm_gem_prime_han
Re: [Nouveau] [PATCH 2/3] drm/nouveau: manage nouveau_drm lifetime with devres
On Fri, Nov 6, 2020 at 3:17 AM Jeremy Cline wrote:
>
> Make use of the devm_drm_dev_alloc() API to bind the lifetime of
> nouveau_drm structure to the drm_device. This is important because a
> reference to nouveau_drm is accessible from drm_device, which is
> provided to a number of DRM layer callbacks that can run after the
> deallocation of nouveau_drm currently occurs.
>
> Signed-off-by: Jeremy Cline
> ---
> drivers/gpu/drm/nouveau/nouveau_drm.c | 44 ---
> drivers/gpu/drm/nouveau/nouveau_drv.h | 10 --
> 2 files changed, 26 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
> b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index bc6f51bf23b7..f750c25e92f9 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -30,9 +30,11 @@
> #include
> #include
>
> +#include
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -532,13 +534,8 @@ nouveau_parent = {
> static int
> nouveau_drm_device_init(struct drm_device *dev)
> {
> - struct nouveau_drm *drm;
> int ret;
> -
> - if (!(drm = kzalloc(sizeof(*drm), GFP_KERNEL)))
> - return -ENOMEM;
> - dev->dev_private = drm;
> - drm->dev = dev;
> + struct nouveau_drm *drm = nouveau_drm(dev);
>
> nvif_parent_ctor(&nouveau_parent, &drm->parent);
> drm->master.base.object.parent = &drm->parent;
> @@ -620,7 +617,6 @@ nouveau_drm_device_init(struct drm_device *dev)
> nouveau_cli_fini(&drm->master);
> fail_alloc:
> nvif_parent_dtor(&drm->parent);
> - kfree(drm);
> return ret;
> }
>
> @@ -654,7 +650,6 @@ nouveau_drm_device_fini(struct drm_device *dev)
> nouveau_cli_fini(&drm->client);
> nouveau_cli_fini(&drm->master);
> nvif_parent_dtor(&drm->parent);
> - kfree(drm);
> }
>
> /*
> @@ -720,6 +715,7 @@ static int nouveau_drm_probe(struct pci_dev *pdev,
> {
> struct nvkm_device *device;
> struct drm_device *drm_dev;
> + struct nouveau_drm *nv_dev;
> int ret;
>
> if (vga_switcheroo_client_probe_defer(pdev))
> @@ -750,15 +746,16 @@ static int nouveau_drm_probe(struct pci_dev *pdev,
> if (nouveau_atomic)
> driver_pci.driver_features |= DRIVER_ATOMIC;
>
> - drm_dev = drm_dev_alloc(&driver_pci, &pdev->dev);
> - if (IS_ERR(drm_dev)) {
> - ret = PTR_ERR(drm_dev);
> + nv_dev = devm_drm_dev_alloc(&pdev->dev, &driver_stub,
> typeof(*nv_dev), drm_dev);
> + if (IS_ERR(nv_dev)) {
> + ret = PTR_ERR(nv_dev);
> goto fail_nvkm;
> }
> + drm_dev = nouveau_to_drm_dev(nv_dev);
>
> ret = pci_enable_device(pdev);
> if (ret)
> - goto fail_drm;
> + goto fail_nvkm;
>
> drm_dev->pdev = pdev;
> pci_set_drvdata(pdev, drm_dev);
> @@ -778,8 +775,6 @@ static int nouveau_drm_probe(struct pci_dev *pdev,
> nouveau_drm_device_fini(drm_dev);
> fail_pci:
> pci_disable_device(pdev);
> -fail_drm:
> - drm_dev_put(drm_dev);
it sounded like that when using devm_drm_dev_alloc we still have an
initial refcount of 1, so at least in this regard nothing changed so I
am wondering why this change is necessary and if the reason is
unrelated it might make sense to move it into its own patch.
> fail_nvkm:
> nvkm_device_del(&device);
> return ret;
> @@ -799,7 +794,6 @@ nouveau_drm_device_remove(struct drm_device *dev)
> device = nvkm_device_find(client->device);
>
> nouveau_drm_device_fini(dev);
> - drm_dev_put(dev);
> nvkm_device_del(&device);
> }
>
> @@ -1285,7 +1279,8 @@ nouveau_platform_device_create(const struct
> nvkm_device_tegra_func *func,
>struct platform_device *pdev,
>struct nvkm_device **pdevice)
> {
> - struct drm_device *drm;
> + struct nouveau_drm *nv_dev;
> + struct drm_device *drm_dev;
> int err;
>
> err = nvkm_device_tegra_new(func, pdev, nouveau_config, nouveau_debug,
> @@ -1293,22 +1288,21 @@ nouveau_platform_device_create(const struct
> nvkm_device_tegra_func *func,
> if (err)
> goto err_free;
>
> - drm = drm_dev_alloc(&driver_platform, &pdev->dev);
> - if (IS_ERR(drm)) {
> - err = PTR_ERR(drm);
> + nv_dev = devm_drm_dev_alloc(&pdev->dev, &driver_platform,
> typeof(*nv_dev), drm_dev);
> + if (IS_ERR(nv_dev)) {
> + err = PTR_ERR(nv_dev);
> goto err_free;
> }
> + drm_dev = nouveau_to_drm_dev(nv_dev);
>
> - err = nouveau_drm_device_init(drm);
> + err = nouveau_drm_device_init(drm_dev);
> if (err)
> - goto err_put;
> + goto err_free;
>
> - platform_set_drvdata(pdev, drm);
> + platform_set_drvdata(pdev, dr
Re: [PATCH] fbdev/sh_mobile: Drop unused include
On Sun, Nov 1, 2020 at 10:36 AM Sam Ravnborg wrote: > On Fri, Oct 30, 2020 at 01:28:50AM +0100, Linus Walleij wrote: > > The driver includes but doesn't use any symbols > > from this file. > > > > Cc: Magnus Damm > > Cc: Geert Uytterhoeven > > Cc: linux-renesas-...@vger.kernel.org > > Signed-off-by: Linus Walleij > > Thanks, I went ahead and applied this to drm-misc-next. > And then I remembered you have commit right too :-( Thanks! It is always better if someone else does it anyways! Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/ttm: fix missing NULL check in the new page pool
The pool parameter can be NULL if we free through the shrinker.
Signed-off-by: Christian König
---
drivers/gpu/drm/ttm/ttm_pool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
index 44ec41aa78d6..1b96780b4989 100644
--- a/drivers/gpu/drm/ttm/ttm_pool.c
+++ b/drivers/gpu/drm/ttm/ttm_pool.c
@@ -135,7 +135,7 @@ static void ttm_pool_free_page(struct ttm_pool *pool, enum
ttm_caching caching,
set_pages_wb(p, 1 << order);
#endif
- if (!pool->use_dma_alloc) {
+ if (!pool || !pool->use_dma_alloc) {
__free_pages(p, order);
return;
}
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 08/19] gpu: drm: omapdrm: dss: dsi: Rework and remove a few unused variables
On Fri, 06 Nov 2020, Tomi Valkeinen wrote: > On 05/11/2020 20:07, Lee Jones wrote: > > On Thu, 05 Nov 2020, Tomi Valkeinen wrote: > > > >> On 05/11/2020 16:45, Lee Jones wrote: > >>> Fixes the following W=1 kernel build warning(s): > >>> > >>> drivers/gpu/drm/omapdrm/dss/dsi.c: In function ‘_dsi_print_reset_status’: > >>> drivers/gpu/drm/omapdrm/dss/dsi.c:1131:6: warning: variable ‘l’ set but > >>> not used [-Wunused-but-set-variable] > >>> drivers/gpu/drm/omapdrm/dss/dsi.c: In function ‘dsi_update’: > >>> drivers/gpu/drm/omapdrm/dss/dsi.c:3943:10: warning: variable ‘dh’ set > >>> but not used [-Wunused-but-set-variable] > >>> drivers/gpu/drm/omapdrm/dss/dsi.c:3943:6: warning: variable ‘dw’ set but > >>> not used [-Wunused-but-set-variable] > >>> > >>> Cc: Tomi Valkeinen > >>> Cc: David Airlie > >>> Cc: Daniel Vetter > >>> Cc: Laurent Pinchart > >>> Cc: dri-devel@lists.freedesktop.org > >>> Signed-off-by: Lee Jones > >>> --- > >>> drivers/gpu/drm/omapdrm/dss/dsi.c | 9 ++--- > >>> 1 file changed, 2 insertions(+), 7 deletions(-) > >> > >> I'd use "drm/omap: dsi: " subject prefix, the current one is fine too: > >> > >> Reviewed-by: Tomi Valkeinen > >> > >> Should I pick this up or do you want to keep the series intact? > > > > If you are in a position to take it, please do so. > > > > I rebase every day, so it will just vanish from my working set. > > I have a 56 patch series on dsi.c that I sent yesterday, and it conflicts > with this one. I'll pick > this one on top of my series. Sounds good, thanks. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH 2/3] drm/nouveau: manage nouveau_drm lifetime with devres
On Fri, Nov 06, 2020 at 02:31:44PM +0100, Karol Herbst wrote:
> On Fri, Nov 6, 2020 at 3:17 AM Jeremy Cline wrote:
> >
> > Make use of the devm_drm_dev_alloc() API to bind the lifetime of
> > nouveau_drm structure to the drm_device. This is important because a
> > reference to nouveau_drm is accessible from drm_device, which is
> > provided to a number of DRM layer callbacks that can run after the
> > deallocation of nouveau_drm currently occurs.
> >
> > Signed-off-by: Jeremy Cline
> > ---
> > drivers/gpu/drm/nouveau/nouveau_drm.c | 44 ---
> > drivers/gpu/drm/nouveau/nouveau_drv.h | 10 --
> > 2 files changed, 26 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
> > b/drivers/gpu/drm/nouveau/nouveau_drm.c
> > index bc6f51bf23b7..f750c25e92f9 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> > @@ -30,9 +30,11 @@
> > #include
> > #include
> >
> > +#include
> > #include
> > #include
> > #include
> > +#include
> >
> > #include
> > #include
> > @@ -532,13 +534,8 @@ nouveau_parent = {
> > static int
> > nouveau_drm_device_init(struct drm_device *dev)
> > {
> > - struct nouveau_drm *drm;
> > int ret;
> > -
> > - if (!(drm = kzalloc(sizeof(*drm), GFP_KERNEL)))
> > - return -ENOMEM;
> > - dev->dev_private = drm;
> > - drm->dev = dev;
> > + struct nouveau_drm *drm = nouveau_drm(dev);
> >
> > nvif_parent_ctor(&nouveau_parent, &drm->parent);
> > drm->master.base.object.parent = &drm->parent;
> > @@ -620,7 +617,6 @@ nouveau_drm_device_init(struct drm_device *dev)
> > nouveau_cli_fini(&drm->master);
> > fail_alloc:
> > nvif_parent_dtor(&drm->parent);
> > - kfree(drm);
> > return ret;
> > }
> >
> > @@ -654,7 +650,6 @@ nouveau_drm_device_fini(struct drm_device *dev)
> > nouveau_cli_fini(&drm->client);
> > nouveau_cli_fini(&drm->master);
> > nvif_parent_dtor(&drm->parent);
> > - kfree(drm);
> > }
> >
> > /*
> > @@ -720,6 +715,7 @@ static int nouveau_drm_probe(struct pci_dev *pdev,
> > {
> > struct nvkm_device *device;
> > struct drm_device *drm_dev;
> > + struct nouveau_drm *nv_dev;
> > int ret;
> >
> > if (vga_switcheroo_client_probe_defer(pdev))
> > @@ -750,15 +746,16 @@ static int nouveau_drm_probe(struct pci_dev *pdev,
> > if (nouveau_atomic)
> > driver_pci.driver_features |= DRIVER_ATOMIC;
> >
> > - drm_dev = drm_dev_alloc(&driver_pci, &pdev->dev);
> > - if (IS_ERR(drm_dev)) {
> > - ret = PTR_ERR(drm_dev);
> > + nv_dev = devm_drm_dev_alloc(&pdev->dev, &driver_stub,
> > typeof(*nv_dev), drm_dev);
> > + if (IS_ERR(nv_dev)) {
> > + ret = PTR_ERR(nv_dev);
> > goto fail_nvkm;
> > }
> > + drm_dev = nouveau_to_drm_dev(nv_dev);
> >
> > ret = pci_enable_device(pdev);
> > if (ret)
> > - goto fail_drm;
> > + goto fail_nvkm;
> >
> > drm_dev->pdev = pdev;
> > pci_set_drvdata(pdev, drm_dev);
> > @@ -778,8 +775,6 @@ static int nouveau_drm_probe(struct pci_dev *pdev,
> > nouveau_drm_device_fini(drm_dev);
> > fail_pci:
> > pci_disable_device(pdev);
> > -fail_drm:
> > - drm_dev_put(drm_dev);
>
> it sounded like that when using devm_drm_dev_alloc we still have an
> initial refcount of 1, so at least in this regard nothing changed so I
> am wondering why this change is necessary and if the reason is
> unrelated it might make sense to move it into its own patch.
>
The way I read the supporting code is that when the allocation occurs,
an action is registered on the parent device to call drm_dev_put(), so
as long as the PCI device is dropped, and as far as I could tell it is
when an error is returned, it should be handled automatically. The same
I *think* goes for the platform device variety with Tegra.
However, this is by far the most likely thing for me to have
misunderstood so I'll look through it a second time and would love to
have a second opinion on it.
> > fail_nvkm:
> > nvkm_device_del(&device);
> > return ret;
> > @@ -799,7 +794,6 @@ nouveau_drm_device_remove(struct drm_device *dev)
> > device = nvkm_device_find(client->device);
> >
> > nouveau_drm_device_fini(dev);
> > - drm_dev_put(dev);
> > nvkm_device_del(&device);
> > }
> >
> > @@ -1285,7 +1279,8 @@ nouveau_platform_device_create(const struct
> > nvkm_device_tegra_func *func,
> >struct platform_device *pdev,
> >struct nvkm_device **pdevice)
> > {
> > - struct drm_device *drm;
> > + struct nouveau_drm *nv_dev;
> > + struct drm_device *drm_dev;
> > int err;
> >
> > err = nvkm_device_tegra_new(func, pdev, nouveau_config,
> > nouv
[PATCH v2 0/2] Default to cachable mappings for GEM SHMEM
Shmem-allocated memory uses the system's default caching flags, usually involving cached mappings. By default, SHMEM GEM helpers map pages using writecombine. Only a few drivers require this setting. Others revert it to default mappings flags. Some could benefit from caching, but don't care. Unify the behaviour by switching the SHMEM GEM code to use cached mappings (i.e., PAGE_KERNEL actually); just like regular shmem memory does. The 3 drivers that require write combining explicitly select it during GEM object creation. The exception is dma-buf imported pages, which are always mapped using writecombine mode. v2: * recreate patches on top of latest SHMEM helpers * update lima, panfrost, v3d (Daniel, Rob) * udl has been updated before separately. Thomas Zimmermann (2): drm/shmem-helper: Use cached mappings by default drm/shmem-helper: Removed drm_gem_shmem_create_object_cached() drivers/gpu/drm/drm_gem_shmem_helper.c | 37 - drivers/gpu/drm/lima/lima_gem.c | 2 +- drivers/gpu/drm/mgag200/mgag200_drv.c | 1 - drivers/gpu/drm/panfrost/panfrost_gem.c | 2 +- drivers/gpu/drm/udl/udl_drv.c | 2 -- drivers/gpu/drm/v3d/v3d_bo.c| 2 +- drivers/gpu/drm/virtio/virtgpu_object.c | 1 - drivers/gpu/drm/vkms/vkms_drv.c | 1 - include/drm/drm_gem_shmem_helper.h | 7 ++--- 9 files changed, 11 insertions(+), 44 deletions(-) -- 2.29.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/2] drm/shmem-helper: Removed drm_gem_shmem_create_object_cached()
Cached page mappings are now the default for SHMEM GEM objects. Remove
the obsolete create function for cached mappings.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 26 --
drivers/gpu/drm/mgag200/mgag200_drv.c | 1 -
drivers/gpu/drm/udl/udl_drv.c | 2 --
drivers/gpu/drm/vkms/vkms_drv.c| 1 -
include/drm/drm_gem_shmem_helper.h | 3 ---
5 files changed, 33 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index ddec0e190f29..5b4240725e64 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -468,32 +468,6 @@ bool drm_gem_shmem_purge(struct drm_gem_object *obj)
}
EXPORT_SYMBOL(drm_gem_shmem_purge);
-/**
- * drm_gem_shmem_create_object_cached - Create a shmem buffer object with
- * cached mappings
- * @dev: DRM device
- * @size: Size of the object to allocate
- *
- * By default, shmem buffer objects use writecombine mappings. This
- * function implements struct drm_driver.gem_create_object for shmem
- * buffer objects with cached mappings.
- *
- * Returns:
- * A struct drm_gem_shmem_object * on success or NULL negative on failure.
- */
-struct drm_gem_object *
-drm_gem_shmem_create_object_cached(struct drm_device *dev, size_t size)
-{
- struct drm_gem_shmem_object *shmem;
-
- shmem = kzalloc(sizeof(*shmem), GFP_KERNEL);
- if (!shmem)
- return NULL;
-
- return &shmem->base;
-}
-EXPORT_SYMBOL(drm_gem_shmem_create_object_cached);
-
/**
* drm_gem_shmem_dumb_create - Create a dumb shmem buffer object
* @file: DRM file structure to create the dumb buffer for
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c
b/drivers/gpu/drm/mgag200/mgag200_drv.c
index 771b26aeee19..aaf50b6a7372 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.c
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
@@ -37,7 +37,6 @@ static struct drm_driver mgag200_driver = {
.major = DRIVER_MAJOR,
.minor = DRIVER_MINOR,
.patchlevel = DRIVER_PATCHLEVEL,
- .gem_create_object = drm_gem_shmem_create_object_cached,
DRM_GEM_SHMEM_DRIVER_OPS,
};
diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
index 96d4317a2c1b..a29147799575 100644
--- a/drivers/gpu/drm/udl/udl_drv.c
+++ b/drivers/gpu/drm/udl/udl_drv.c
@@ -38,8 +38,6 @@ static struct drm_driver driver = {
.driver_features = DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET,
/* GEM hooks */
- .gem_create_object = drm_gem_shmem_create_object_cached,
-
.fops = &udl_driver_fops,
DRM_GEM_SHMEM_DRIVER_OPS,
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 25faba5aac08..b6cd7f2d941c 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -82,7 +82,6 @@ static struct drm_driver vkms_driver = {
.driver_features= DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM,
.release= vkms_release,
.fops = &vkms_driver_fops,
- .gem_create_object = drm_gem_shmem_create_object_cached,
DRM_GEM_SHMEM_DRIVER_OPS,
.name = DRIVER_NAME,
diff --git a/include/drm/drm_gem_shmem_helper.h
b/include/drm/drm_gem_shmem_helper.h
index 268d0284ae02..6bcd2c4d6a51 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -133,9 +133,6 @@ drm_gem_shmem_create_with_handle(struct drm_file *file_priv,
struct drm_device *dev, size_t size,
uint32_t *handle);
-struct drm_gem_object *
-drm_gem_shmem_create_object_cached(struct drm_device *dev, size_t size);
-
int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
struct drm_mode_create_dumb *args);
--
2.29.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/2] drm/shmem-helper: Use cached mappings by default
SHMEM-buffer backing storage is allocated from system memory; which is
typically cachable. The default mode for SHMEM objects is writecombine
though.
Unify SHMEM semantics by defaulting to cached mappings. The exception
is pages imported via dma-buf. DMA memory is usually not cached.
DRM drivers that require write-combined mappings set the map_wc flag
in struct drm_gem_shmem_object to true. This currently affects lima,
panfrost and v3d.
The drivers mgag200, udl, virtio and vkms continue to use default
shmem mappings.
The drivers cirrus and gm12u320 change caching flags. Both used
writecombine and now switch over to shmem defaults. Both drivers use
SHMEM objects as shadow buffers for internal video memory, so cached
mappings will not affect them negatively.
v2:
* recreate patch on top of latest SHMEM helpers
* update lima, panfrost, v3d to select writecombine (Daniel, Rob)
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 11 ++-
drivers/gpu/drm/lima/lima_gem.c | 2 +-
drivers/gpu/drm/panfrost/panfrost_gem.c | 2 +-
drivers/gpu/drm/v3d/v3d_bo.c| 2 +-
drivers/gpu/drm/virtio/virtgpu_object.c | 1 -
include/drm/drm_gem_shmem_helper.h | 4 ++--
6 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 8233bda4692f..ddec0e190f29 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -54,10 +54,12 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t size,
bool private)
if (!obj->funcs)
obj->funcs = &drm_gem_shmem_funcs;
- if (private)
+ if (private) {
drm_gem_private_object_init(dev, obj, size);
- else
+ shmem->map_wc = false; /* dma-buf mappings use always
writecombine */
+ } else {
ret = drm_gem_object_init(dev, obj, size);
+ }
if (ret)
goto err_free;
@@ -278,7 +280,7 @@ static void *drm_gem_shmem_vmap_locked(struct
drm_gem_shmem_object *shmem)
if (ret)
goto err_zero_use;
- if (!shmem->map_cached)
+ if (shmem->map_wc)
prot = pgprot_writecombine(prot);
shmem->vaddr = vmap(shmem->pages, obj->size >> PAGE_SHIFT,
VM_MAP, prot);
@@ -487,7 +489,6 @@ drm_gem_shmem_create_object_cached(struct drm_device *dev,
size_t size)
shmem = kzalloc(sizeof(*shmem), GFP_KERNEL);
if (!shmem)
return NULL;
- shmem->map_cached = true;
return &shmem->base;
}
@@ -616,7 +617,7 @@ int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct
vm_area_struct *vma)
vma->vm_flags |= VM_MIXEDMAP | VM_DONTEXPAND;
vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
- if (!shmem->map_cached)
+ if (shmem->map_wc)
vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
vma->vm_ops = &drm_gem_shmem_vm_ops;
diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c
index 11223fe348df..bbab1413eb0c 100644
--- a/drivers/gpu/drm/lima/lima_gem.c
+++ b/drivers/gpu/drm/lima/lima_gem.c
@@ -225,7 +225,7 @@ struct drm_gem_object *lima_gem_create_object(struct
drm_device *dev, size_t siz
mutex_init(&bo->lock);
INIT_LIST_HEAD(&bo->va);
-
+ bo->base.map_wc = true;
bo->base.base.funcs = &lima_gem_funcs;
return &bo->base.base;
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c
b/drivers/gpu/drm/panfrost/panfrost_gem.c
index fb9f7334ce18..f77b72d995f9 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
@@ -230,7 +230,7 @@ struct drm_gem_object *panfrost_gem_create_object(struct
drm_device *dev, size_t
INIT_LIST_HEAD(&obj->mappings.list);
mutex_init(&obj->mappings.lock);
obj->base.base.funcs = &panfrost_gem_funcs;
- obj->base.map_cached = pfdev->coherent;
+ obj->base.map_wc = !pfdev->coherent;
return &obj->base.base;
}
diff --git a/drivers/gpu/drm/v3d/v3d_bo.c b/drivers/gpu/drm/v3d/v3d_bo.c
index 8b52cb25877c..6a8731ab9d7d 100644
--- a/drivers/gpu/drm/v3d/v3d_bo.c
+++ b/drivers/gpu/drm/v3d/v3d_bo.c
@@ -78,7 +78,7 @@ struct drm_gem_object *v3d_create_object(struct drm_device
*dev, size_t size)
obj = &bo->base.base;
obj->funcs = &v3d_gem_funcs;
-
+ bo->base.map_wc = true;
INIT_LIST_HEAD(&bo->unref_head);
return &bo->base.base;
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c
b/drivers/gpu/drm/virtio/virtgpu_object.c
index 2d3aa7baffe4..47e3b69c3927 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -144,7 +144,6 @@ struct drm_gem_object *virtio_gpu_create_object(struct
drm_device *dev,
dshmem =
Re: [PATCH 0/7] sunxi: Remove the calls to dma_direct_set_offset
Hi, On Fri, Nov 6, 2020 at 11:15 PM Maxime Ripard wrote: > > Hi, > > Here's an attempt to removing the dma_direct_set_offset calls we have in > numerous drivers and move all those quirks into a global notifier as suggested > by Robin. > > Let me know what you think, > Maxime > > Maxime Ripard (7): > drm/sun4i: backend: Fix probe failure with multiple backends > soc: sunxi: Deal with the MBUS DMA offsets in a central place > drm/sun4i: backend: Remove the MBUS quirks > media: sun4i: Remove the MBUS quirks > media: sun6i: Remove the MBUS quirks > media: cedrus: Remove the MBUS quirks > media: sun8i-di: Remove the call to of_dma_configure Whole series looks good to me. Reviewed-by: Chen-Yu Tsai Now the question remaining is how do we merge this series so that the notifier gets merged before all the code dealing with the MBUS quirk gets removed. > drivers/gpu/drm/sun4i/sun4i_backend.c | 13 -- > .../platform/sunxi/sun4i-csi/sun4i_csi.c | 27 > .../platform/sunxi/sun6i-csi/sun6i_csi.c | 17 --- > .../media/platform/sunxi/sun8i-di/sun8i-di.c | 4 - > drivers/soc/sunxi/Kconfig | 8 ++ > drivers/soc/sunxi/Makefile| 1 + > drivers/soc/sunxi/sunxi_mbus.c| 132 ++ > drivers/staging/media/sunxi/cedrus/cedrus.c | 1 - > drivers/staging/media/sunxi/cedrus/cedrus.h | 3 - > .../staging/media/sunxi/cedrus/cedrus_hw.c| 18 --- > 10 files changed, 141 insertions(+), 83 deletions(-) > create mode 100644 drivers/soc/sunxi/sunxi_mbus.c > > -- > 2.28.0 > > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH v8 4/5] RDMA/mlx5: Support dma-buf based userspace memory region
> -Original Message-
> From: Jason Gunthorpe
> Sent: Friday, November 06, 2020 4:49 AM
> To: Xiong, Jianxin
> Cc: linux-r...@vger.kernel.org; dri-devel@lists.freedesktop.org; Doug Ledford
> ; Leon Romanovsky
> ; Sumit Semwal ; Christian Koenig
> ; Vetter, Daniel
>
> Subject: Re: [PATCH v8 4/5] RDMA/mlx5: Support dma-buf based userspace memory
> region
>
> On Fri, Nov 06, 2020 at 01:11:38AM +, Xiong, Jianxin wrote:
> > > On Thu, Nov 05, 2020 at 02:48:08PM -0800, Jianxin Xiong wrote:
> > > > @@ -966,7 +969,10 @@ static struct mlx5_ib_mr
> > > > *alloc_mr_from_cache(struct ib_pd *pd,
> > > > struct mlx5_ib_mr *mr;
> > > > unsigned int page_size;
> > > >
> > > > - page_size = mlx5_umem_find_best_pgsz(umem, mkc, log_page_size,
> > > > 0, iova);
> > > > + if (umem->is_dmabuf)
> > > > + page_size = ib_umem_find_best_pgsz(umem, PAGE_SIZE,
> > > > iova);
> > >
> > > You said the sgl is not set here, why doesn't this crash? It is certainly
> > > wrong to call this function without a SGL.
> >
> > The sgl is NULL, and nmap is 0. The 'for_each_sg' loop is just skipped and
> > won't crash.
>
> Just wire this to 4k it is clearer than calling some no-op pgsz
Ok
>
>
> > > > + if (!mr->cache_ent) {
> > > > + mlx5_core_destroy_mkey(mr->dev->mdev, &mr->mmkey);
> > > > + WARN_ON(mr->descs);
> > > > + }
> > > > +}
> > >
> > > I would expect this to call ib_umem_dmabuf_unmap_pages() ?
> > >
> > > Who calls it on the dereg path?
> > >
> > > This looks quite strange to me, it calls ib_umem_dmabuf_unmap_pages()
> > > only from the invalidate callback?
> >
> > It is also called from ib_umem_dmabuf_release().
>
> Hmm, that is no how the other APIs work, the unmap should be paired with the
> map in the caller, and the sequence for destroy should be
>
> invalidate
> unmap
> destroy_mkey
> release_umem
>
> I have another series coming that makes the other three destroy flows much
> closer to that ideal.
>
Can fix that.
> > > I feel uneasy how this seems to assume everything works sanely, we
> > > can have parallel page faults so pagefault_dmabuf_mr() can be called
> > > multiple times after an invalidation, and it doesn't protect itself
> against calling ib_umem_dmabuf_map_pages() twice.
> > >
> > > Perhaps the umem code should keep track of the current map state and
> > > exit if there is already a sgl. NULL or not NULL sgl would do and seems
> > > quite reasonable.
> >
> > Ib_umem_dmabuf_map() already checks the sgl and will do nothing if it is
> > already set.
>
> How? What I see in patch 1 is an unconditonal call to
> dma_buf_map_attachment() ?
My bad. I misread the lines. It used to be there (in v3) but somehow got lost.
>
> > > > + if (is_dmabuf_mr(mr))
> > > > + return pagefault_dmabuf_mr(mr, umem_dmabuf,
> > > > user_va,
> > > > + bcnt, bytes_mapped,
> > > > flags);
> > >
> > > But this doesn't care about user_va or bcnt it just triggers the whole
> > > thing to be remapped, so why calculate it?
> >
> > The range check is still needed, in order to catch application errors
> > of using incorrect address or count in verbs command. Passing the
> > values further in is to allow pagefault_dmabuf_mr to generate return
> > value and set bytes_mapped in a way consistent with the page fault
> > handler chain.
>
> The HW validates the range. The range check in the ODP case is to protect
> against a HW bug that would cause the kernel to malfunction.
> For dmabuf you don't need to do it
Ok. So the handler can simply return 0 (as the number of pages mapped) and
leave bytes_mapped untouched?
>
> Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 16/19] gpu: drm: panel: panel-ilitek-ili9322: Demote non-conformant kernel-doc header
Hi Lee, > > > > Applied to drm-misc-next. > > Thanks for all these Sam. > > Any idea what happens to the other patches? > > Do they go in via a different Maintainer? I expect the respective drm maintaines to take them. Give them a few days to take action. I look forward for the next set that you said would kill 2000+ warnings. Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/gma500: Remove unused function psb_gem_get_aperture()
On Fri, Nov 06, 2020 at 01:42:24PM +0100, Thomas Zimmermann wrote:
> Apparently, the function was never used at all.
>
> Signed-off-by: Thomas Zimmermann
Reviewed-by: Sam Ravnborg
Indeed unused.
I expect you or Patrik to apply the patch.
Sam
> ---
> drivers/gpu/drm/gma500/gem.c | 6 --
> drivers/gpu/drm/gma500/psb_drv.h | 2 --
> 2 files changed, 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
> index 8f07de83b6fb..db827e591403 100644
> --- a/drivers/gpu/drm/gma500/gem.c
> +++ b/drivers/gpu/drm/gma500/gem.c
> @@ -32,12 +32,6 @@ static void psb_gem_free_object(struct drm_gem_object *obj)
> psb_gtt_free_range(obj->dev, gtt);
> }
>
> -int psb_gem_get_aperture(struct drm_device *dev, void *data,
> - struct drm_file *file)
> -{
> - return -EINVAL;
> -}
> -
> static const struct vm_operations_struct psb_gem_vm_ops = {
> .fault = psb_gem_fault,
> .open = drm_gem_vm_open,
> diff --git a/drivers/gpu/drm/gma500/psb_drv.h
> b/drivers/gpu/drm/gma500/psb_drv.h
> index c71a5a4e912c..ce6aae4b1bb2 100644
> --- a/drivers/gpu/drm/gma500/psb_drv.h
> +++ b/drivers/gpu/drm/gma500/psb_drv.h
> @@ -735,8 +735,6 @@ extern const struct drm_connector_helper_funcs
> extern const struct drm_connector_funcs psb_intel_lvds_connector_funcs;
>
> /* gem.c */
> -extern int psb_gem_get_aperture(struct drm_device *dev, void *data,
> - struct drm_file *file);
> extern int psb_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
> struct drm_mode_create_dumb *args);
>
> --
> 2.29.0
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/cma-helper: Make default object functions the default
Hi Thomas. On Fri, Nov 06, 2020 at 02:16:32PM +0100, Thomas Zimmermann wrote: > As GEM object functions are now mandatory, DRM drivers based on CMA > helpers either set them in their implementation of gem_create_object, > or use the default via drm_gem_cma_create_object_default_funcs(). > > Simplify this by setting the default CMA object functions for all > objects that don't have any functions of their own. Follows the pattern > of similar code in SHMEM and VRAM helpers. The function > drm_gem_cma_create_object_default_funcs() is redundant and therefore > being removed. > > Signed-off-by: Thomas Zimmermann Nice cleanup after all the groundwork you made in order to reach this step. Reviewed-by: Sam Ravnborg Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH v8 3/5] RDMA/uverbs: Add uverbs command for dma-buf based MR registration
> -Original Message- > From: Jason Gunthorpe > Sent: Thursday, November 05, 2020 4:13 PM > To: Xiong, Jianxin > Cc: linux-r...@vger.kernel.org; dri-devel@lists.freedesktop.org; Doug Ledford > ; Leon Romanovsky > ; Sumit Semwal ; Christian Koenig > ; Vetter, Daniel > > Subject: Re: [PATCH v8 3/5] RDMA/uverbs: Add uverbs command for dma-buf based > MR registration > > On Thu, Nov 05, 2020 at 02:48:07PM -0800, Jianxin Xiong wrote: > > > + ret = ib_check_mr_access(access_flags); > > + if (ret) > > + return ret; > > This should also reject unsupportable flags like ACCESS_ON_DEMAND and HUGETLB Will do. > > > + > > + mr = pd->device->ops.reg_user_mr_dmabuf(pd, offset, length, virt_addr, > > + fd, access_flags, > > + &attrs->driver_udata); > > + if (IS_ERR(mr)) > > + return PTR_ERR(mr); > > + > > + mr->device = pd->device; > > + mr->pd = pd; > > + mr->type= IB_MR_TYPE_USER; > > + mr->uobject = uobj; > > + atomic_inc(&pd->usecnt); > > Fix intending when copying code please It could be due to a mix of tab and space. They look aligned in the source file. Will fix. > > > + > > + uobj->object = mr; > > Also bit surprised this works at all, it needs to call > > uverbs_finalize_uobj_create() > > Right here. > Will fix. > Seems like the core code is missing some check that the API is being used > properly. > > > + > > + ret = uverbs_copy_to(attrs, UVERBS_ATTR_REG_DMABUF_MR_RESP_LKEY, > > +&mr->lkey, sizeof(mr->lkey)); > > + if (ret) > > + goto err_dereg; > > + > > + ret = uverbs_copy_to(attrs, UVERBS_ATTR_REG_DMABUF_MR_RESP_RKEY, > > +&mr->rkey, sizeof(mr->rkey)); > > + if (ret) > > + goto err_dereg; > > + > > + return 0; > > + > > +err_dereg: > > + ib_dereg_mr_user(mr, uverbs_get_cleared_udata(attrs)); > > + > > + return ret; > > +} > > + > > DECLARE_UVERBS_NAMED_METHOD( > > UVERBS_METHOD_ADVISE_MR, > > UVERBS_ATTR_IDR(UVERBS_ATTR_ADVISE_MR_PD_HANDLE, > > > @@ -253,6 +364,7 @@ static int UVERBS_HANDLER(UVERBS_METHOD_QUERY_MR)( > > DECLARE_UVERBS_NAMED_OBJECT( > > UVERBS_OBJECT_MR, > > UVERBS_TYPE_ALLOC_IDR(uverbs_free_mr), > > + &UVERBS_METHOD(UVERBS_METHOD_REG_DMABUF_MR), > > &UVERBS_METHOD(UVERBS_METHOD_DM_MR_REG), > > &UVERBS_METHOD(UVERBS_METHOD_MR_DESTROY), > > &UVERBS_METHOD(UVERBS_METHOD_ADVISE_MR), > > I trie to keep these sorted, but I see it has become unsorted. You can > re-sort it in this patch Will do. > > Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH v8 1/5] RDMA/umem: Support importing dma-buf as user memory region
> -Original Message-
> From: Jason Gunthorpe
> Sent: Thursday, November 05, 2020 4:09 PM
> To: Xiong, Jianxin
> Cc: linux-r...@vger.kernel.org; dri-devel@lists.freedesktop.org; Doug Ledford
> ; Leon Romanovsky
> ; Sumit Semwal ; Christian Koenig
> ; Vetter, Daniel
>
> Subject: Re: [PATCH v8 1/5] RDMA/umem: Support importing dma-buf as user
> memory region
>
> On Thu, Nov 05, 2020 at 02:48:05PM -0800, Jianxin Xiong wrote:
> > + /* modify the sgl in-place to match umem address and length */
> > +
> > + start = ALIGN_DOWN(umem_dmabuf->umem.address, PAGE_SIZE);
> > + end = ALIGN(umem_dmabuf->umem.address + umem_dmabuf->umem.length,
> > + PAGE_SIZE);
> > + cur = 0;
> > + nmap = 0;
> > + for_each_sgtable_dma_sg(sgt, sg, i) {
> > + if (cur >= end)
> > + break;
> > + if (cur + sg_dma_len(sg) <= start) {
> > + cur += sg_dma_len(sg);
> > + continue;
> > + }
>
> This seems like a strange way to compute interesections
I can rework that.
>
> if (cur <= start && start < cur + sg_dma_len(sg))
>
> > + if (cur <= start) {
> > + unsigned long offset = start - cur;
> > +
> > + umem_dmabuf->first_sg = sg;
> > + umem_dmabuf->first_sg_offset = offset;
> > + sg_dma_address(sg) += offset;
> > + sg_dma_len(sg) -= offset;
> > + if (&sg_dma_len(sg) != &sg->length)
> > + sg->length -= offset;
>
> We don't need to adjust sg->length, only dma_len, so no reason for this
> surprising if.
>
> > + cur += offset;
> > + }
> > + if (cur + sg_dma_len(sg) >= end) {
>
> Same logic here
>
> > + unsigned long trim = cur + sg_dma_len(sg) - end;
> > +
> > + umem_dmabuf->last_sg = sg;
> > + umem_dmabuf->last_sg_trim = trim;
> > + sg_dma_len(sg) -= trim;
> > + if (&sg_dma_len(sg) != &sg->length)
> > + sg->length -= trim;
>
> break, things are done here
>
> > + }
> > + cur += sg_dma_len(sg);
> > + nmap++;
> > + }
>
> > +
> > + umem_dmabuf->umem.sg_head.sgl = umem_dmabuf->first_sg;
> > + umem_dmabuf->umem.sg_head.nents = nmap;
> > + umem_dmabuf->umem.nmap = nmap;
> > + umem_dmabuf->sgt = sgt;
> > +
> > + page_size = ib_umem_find_best_pgsz(&umem_dmabuf->umem, PAGE_SIZE,
> > + umem_dmabuf->umem.iova);
> > +
> > + if (WARN_ON(cur != end || page_size != PAGE_SIZE)) {
>
> Looks like nothing prevents this warn on to tigger
>
> The user could specify a length that is beyond the dma buf, can the dma buf
> length be checked during get?
In order to check the length, the buffer needs to be mapped. That can be done.
>
> Also page_size can be 0 because iova is not OK. iova should be checked for
> alignment during get as well:
>
> iova & (PAGE_SIZE-1) == umem->addr & (PAGE_SIZE-1)
>
If ib_umem_dmabuf_map_pages is called during get this error is automatically
caught.
> But yes, this is the right idea
>
> Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 16/19] gpu: drm: panel: panel-ilitek-ili9322: Demote non-conformant kernel-doc header
On Fri, 06 Nov 2020, Sam Ravnborg wrote: > Hi Lee, > > > > > > Applied to drm-misc-next. > > > > Thanks for all these Sam. > > > > Any idea what happens to the other patches? > > > > Do they go in via a different Maintainer? > > I expect the respective drm maintaines to take them. > Give them a few days to take action. > > I look forward for the next set that you said would kill 2000+ warnings. Just testing it now. I hope to send it this evening. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/stm: Enable RPM during fbdev registration
Hi Marek,
On 11/5/20 10:45 AM, Marek Vasut wrote:
> On 11/5/20 10:39 AM, Daniel Vetter wrote:
>> On Wed, Nov 04, 2020 at 01:52:00PM +0100, Marek Vasut wrote:
>>> Enable runtime PM before registering the fbdev emulation and disable it
>>> afterward, otherwise register access to the LTDC IP during the fbdev
>>> emulation registration might hang the system.
>>>
>>> The problem happens because RPM is activated at the end of ltdc_load(),
>>> but the fbdev emulation registration happens only after that, and ends
>>> up calling ltdc_crtc_mode_set_nofb(), which checks whether RPM is active
>>> and only if it is not active, calls pm_runtime_get_sync() to enable the
>>> clock and so on. If the clock are not enabled, any register access in
>>> ltdc_crtc_mode_set_nofb() could hang the platform completely.
>>>
>>> This patch makes sure that ltdc_crtc_mode_set_nofb() is called within
>>> pm_runtime_get_sync(), so with clock enabled.
>
> [...]
>
>> This looks like you're papering over a bug in your modeset code. If
>> userspace later on does a setpar on the fbdev chardev, the exact same
>> thing could happen. You need to fix your modeset code to avoid this, not
>> sprinkle temporary rpm_get/put all over some top level entry points,
>> because you can't even patch those all.
>
> I have a feeling all those pm_runtime_active() checks in the driver
> might be the root cause of this ? I wonder why the code doesn't use
> pm_runtime_{get,put}_sync() only when accessing registers. Thoughts?
First line of function ltdc_crtc_mode_set_nofb check the pm_runtime to
avoid to access registers without clock enabled.
static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc)
{
...
if (!pm_runtime_active(ddev->dev)) {
ret = pm_runtime_get_sync(ddev->dev);
I test the fb with framebuffer console, & it works fine on my side.
Do you test fb on a old kernel?
How can I reproduce your issue?
Best regards
Yannick
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH v8 1/5] RDMA/umem: Support importing dma-buf as user memory region
> -Original Message- > From: Jason Gunthorpe > Sent: Friday, November 06, 2020 8:40 AM > To: Xiong, Jianxin > Cc: linux-r...@vger.kernel.org; dri-devel@lists.freedesktop.org; Doug Ledford > ; Leon Romanovsky > ; Sumit Semwal ; Christian Koenig > ; Vetter, Daniel > > Subject: Re: [PATCH v8 1/5] RDMA/umem: Support importing dma-buf as user > memory region > > On Fri, Nov 06, 2020 at 04:34:07PM +, Xiong, Jianxin wrote: > > > > The user could specify a length that is beyond the dma buf, can the > > > dma buf length be checked during get? > > > > In order to check the length, the buffer needs to be mapped. That can be > > done. > > Do DMA bufs even have definitate immutable lengths? Going to be a probelm if > they can shrink Good question. The buffer itself has fixed size. If for whatever reason the mapping is not full it must be temporary. If that does happen ib_umem_dmabuf_map_pages will undo the mapping and return error. It will be retried later via the pagefault handler. > > > > Also page_size can be 0 because iova is not OK. iova should be > > > checked for alignment during get as well: > > > > > > iova & (PAGE_SIZE-1) == umem->addr & (PAGE_SIZE-1) > > > > If ib_umem_dmabuf_map_pages is called during get this error is > > automatically caught. > > The ib_umem_find_best_pgsz() checks this equation, yes. > > So you'd map the sgl before allocating the mkey? This then checks the length > and iova? Yes > > Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 08/19] gpu: drm: omapdrm: dss: dsi: Rework and remove a few unused variables
On 05/11/2020 20:07, Lee Jones wrote: > On Thu, 05 Nov 2020, Tomi Valkeinen wrote: > >> On 05/11/2020 16:45, Lee Jones wrote: >>> Fixes the following W=1 kernel build warning(s): >>> >>> drivers/gpu/drm/omapdrm/dss/dsi.c: In function ‘_dsi_print_reset_status’: >>> drivers/gpu/drm/omapdrm/dss/dsi.c:1131:6: warning: variable ‘l’ set but >>> not used [-Wunused-but-set-variable] >>> drivers/gpu/drm/omapdrm/dss/dsi.c: In function ‘dsi_update’: >>> drivers/gpu/drm/omapdrm/dss/dsi.c:3943:10: warning: variable ‘dh’ set but >>> not used [-Wunused-but-set-variable] >>> drivers/gpu/drm/omapdrm/dss/dsi.c:3943:6: warning: variable ‘dw’ set but >>> not used [-Wunused-but-set-variable] >>> >>> Cc: Tomi Valkeinen >>> Cc: David Airlie >>> Cc: Daniel Vetter >>> Cc: Laurent Pinchart >>> Cc: dri-devel@lists.freedesktop.org >>> Signed-off-by: Lee Jones >>> --- >>> drivers/gpu/drm/omapdrm/dss/dsi.c | 9 ++--- >>> 1 file changed, 2 insertions(+), 7 deletions(-) >> >> I'd use "drm/omap: dsi: " subject prefix, the current one is fine too: >> >> Reviewed-by: Tomi Valkeinen >> >> Should I pick this up or do you want to keep the series intact? > > If you are in a position to take it, please do so. > > I rebase every day, so it will just vanish from my working set. I have a 56 patch series on dsi.c that I sent yesterday, and it conflicts with this one. I'll pick this one on top of my series. Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
