[PULL] drm-intel-next-fixes
Hi Dave & Daniel, Here's a batch of -next-fixes from drm-intel-next/drm-intel-gt-next. On GT side just a fix to relax GGTT alignment down 64K from 2M. Addition of missing "name" attribute for GVT mdev device. On display side async flip fixes and a static checker fix. CI results had some display errors on TGL, the display has been rebooted to fix those so should cause no worries. Regards, Joonas *** drm-intel-next-fixes-2022-03-10: - Reduce overzealous alignment constraints for GGTT - Add missing mdev attribute "name" for GVT - Async flip fixes (Ville) - Static checker fix (Ville) The following changes since commit 6de7e4f02640fba2ffa6ac04e2be13785d614175: Merge tag 'drm-msm-next-2022-03-01' of https://gitlab.freedesktop.org/drm/msm into drm-next (2022-03-04 14:39:00 +1000) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-next-fixes-2022-03-10 for you to fetch changes up to 5e7f44b5c2c035fe2e5458193c2bbee56db6a090: drm/i915/gtt: reduce overzealous alignment constraints for GGTT (2022-03-09 08:34:55 +0200) - Reduce overzealous alignment constraints for GGTT - Add missing mdev attribute "name" for GVT - Async flip fixes (Ville) - Static checker fix (Ville) Joonas Lahtinen (1): Merge tag 'gvt-next-2022-03-07' of https://github.com/intel/gvt-linux into drm-intel-next-fixes Matthew Auld (1): drm/i915/gtt: reduce overzealous alignment constraints for GGTT Ville Syrjälä (4): drm/i915: Avoid negative shift due to bigjoiner_pipes==0 drm/i915: Don't skip ddb allocation if data_rate==0 drm/i915: Check async flip capability early on drm/i915: Fix the async flip wm0/ddb optimization Zhi Wang (1): drm/i915/gvt: add the missing mdev attribute "name" drivers/gpu/drm/i915/display/intel_atomic.c| 1 + drivers/gpu/drm/i915/display/intel_atomic_plane.c | 7 +- drivers/gpu/drm/i915/display/intel_crtc.c | 4 +- drivers/gpu/drm/i915/display/intel_display.c | 122 + drivers/gpu/drm/i915/display/intel_display_types.h | 6 +- drivers/gpu/drm/i915/gt/intel_gtt.c| 3 +- drivers/gpu/drm/i915/gvt/kvmgt.c | 15 +++ drivers/gpu/drm/i915/intel_pm.c| 30 ++--- 8 files changed, 136 insertions(+), 52 deletions(-)
[PATCH] drm/i915/uapi: Add DRM_I915_QUERY_GEOMETRY_SUBSLICES
Newer platforms have DSS that aren't necessarily available for both
geometry and compute, two queries will need to exist. This introduces
the first, when passing a valid engine class and engine instance in the
flags returns a topology describing geometry.
v2: fix white space errors
Cc: Ashutosh Dixit
Cc: Matt Roper
Cc: Joonas Lahtinen
UMD (mesa): https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/14143
Signed-off-by: Matt Atwood
---
drivers/gpu/drm/i915/i915_query.c | 68 ++-
include/uapi/drm/i915_drm.h | 24 +++
2 files changed, 65 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_query.c
b/drivers/gpu/drm/i915/i915_query.c
index 2dfbc22857a3..e4f35da28642 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -9,6 +9,7 @@
#include "i915_drv.h"
#include "i915_perf.h"
#include "i915_query.h"
+#include "gt/intel_engine_user.h"
#include
static int copy_query_item(void *query_hdr, size_t query_sz,
@@ -28,36 +29,30 @@ static int copy_query_item(void *query_hdr, size_t query_sz,
return 0;
}
-static int query_topology_info(struct drm_i915_private *dev_priv,
- struct drm_i915_query_item *query_item)
+static int fill_topology_info(const struct sseu_dev_info *sseu,
+ struct drm_i915_query_item *query_item,
+ const u8 *subslice_mask)
{
- const struct sseu_dev_info *sseu = _gt(dev_priv)->info.sseu;
struct drm_i915_query_topology_info topo;
u32 slice_length, subslice_length, eu_length, total_length;
int ret;
- if (query_item->flags != 0)
- return -EINVAL;
+ BUILD_BUG_ON(sizeof(u8) != sizeof(sseu->slice_mask));
if (sseu->max_slices == 0)
return -ENODEV;
- BUILD_BUG_ON(sizeof(u8) != sizeof(sseu->slice_mask));
-
slice_length = sizeof(sseu->slice_mask);
subslice_length = sseu->max_slices * sseu->ss_stride;
eu_length = sseu->max_slices * sseu->max_subslices * sseu->eu_stride;
total_length = sizeof(topo) + slice_length + subslice_length +
eu_length;
- ret = copy_query_item(, sizeof(topo), total_length,
- query_item);
+ ret = copy_query_item(, sizeof(topo), total_length, query_item);
+
if (ret != 0)
return ret;
- if (topo.flags != 0)
- return -EINVAL;
-
memset(, 0, sizeof(topo));
topo.max_slices = sseu->max_slices;
topo.max_subslices = sseu->max_subslices;
@@ -69,27 +64,61 @@ static int query_topology_info(struct drm_i915_private
*dev_priv,
topo.eu_stride = sseu->eu_stride;
if (copy_to_user(u64_to_user_ptr(query_item->data_ptr),
- , sizeof(topo)))
+, sizeof(topo)))
return -EFAULT;
if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + sizeof(topo)),
- >slice_mask, slice_length))
+>slice_mask, slice_length))
return -EFAULT;
if (copy_to_user(u64_to_user_ptr(query_item->data_ptr +
- sizeof(topo) + slice_length),
- sseu->subslice_mask, subslice_length))
+sizeof(topo) + slice_length),
+subslice_mask, subslice_length))
return -EFAULT;
if (copy_to_user(u64_to_user_ptr(query_item->data_ptr +
- sizeof(topo) +
- slice_length + subslice_length),
- sseu->eu_mask, eu_length))
+sizeof(topo) +
+slice_length + subslice_length),
+sseu->eu_mask, eu_length))
return -EFAULT;
return total_length;
}
+static int query_topology_info(struct drm_i915_private *dev_priv,
+ struct drm_i915_query_item *query_item)
+{
+ const struct sseu_dev_info *sseu = _gt(dev_priv)->info.sseu;
+
+ if (query_item->flags != 0)
+ return -EINVAL;
+
+ return fill_topology_info(sseu, query_item, sseu->subslice_mask);
+}
+
+static int query_geometry_subslices(struct drm_i915_private *i915,
+ struct drm_i915_query_item *query_item)
+{
+ const struct sseu_dev_info *sseu;
+ struct intel_engine_cs *engine;
+ u8 engine_class, engine_instance;
+
+ if (GRAPHICS_VER_FULL(i915) < IP_VER(12, 50))
+ return -ENODEV;
+
+ engine_class = query_item->flags & 0xFF;
+ engine_instance = (query_item->flags >> 8) & 0xFF;
+
+ engine = intel_engine_lookup_user(i915, engine_class, engine_instance);
+
+ if (!engine)
+
Re: [PATCH v9 00/11] vgaarb: Rework default VGA device selection
I have no objection :) Huacai On Thu, Mar 10, 2022 at 12:29 AM Bjorn Helgaas wrote: > > On Fri, Feb 25, 2022 at 04:15:23PM -0600, Bjorn Helgaas wrote: > > On Thu, Feb 24, 2022 at 04:47:42PM -0600, Bjorn Helgaas wrote: > > > From: Bjorn Helgaas > > > > > > Current default VGA device selection fails in some cases because part of > > > it > > > is done in the vga_arb_device_init() subsys_initcall, and some arches > > > enumerate PCI devices in pcibios_init(), which runs *after* that. > > > > > > The big change from the v8 posting is that this moves vgaarb.c from > > > drivers/gpu/vga to drivers/pci because it really has nothing to do with > > > GPUs or DRM. > > > > I provisionally applied this to pci/vga and put it into -next just > > to get a little runtime on it. > > > > But I'd prefer not to unilaterally yank this out of drivers/gpu > > without a consensus from the GPU folks that this is the right thing to > > do. > > > > Any thoughts? If it seems OK to you, I think patch 1/11 (the move > > itself) is all you would need to look at, although of course I would > > still be grateful for any review and feedback on the rest. > > > > After it's in drivers/pci, all the blame for any issues would come my > > way. > > Ping? This has been in -next since the Feb 28 tree, and I plan to > ask Linus to include it during the v5.18 merge window (which will > probably open Mar 13) unless somebody objects. > > Bjorn
Re: [PATCH v2, 0/4] Cooperate with DSI RX devices to modify dsi funcs and delay mipi high to cooperate with panel sequence
On Thu, Mar 10, 2022 at 11:33 AM xinlei.lee wrote: > > On Tue, 2022-03-08 at 11:00 +0100, Benjamin Gaignard wrote: > > Le 08/03/2022 à 10:12, Hsin-Yi Wang a écrit : > > > On Fri, Mar 4, 2022 at 7:25 PM Benjamin Gaignard > > > wrote: > > > > > > > > Le 04/03/2022 à 11:15, xinlei@mediatek.com a écrit : > > > > > From: Xinlei Lee > > > > > > > > > > In upstream-v5.8, dsi_enable will operate panel_enable, but > > > > > this > > > > > modification has been moved in v5.9. In order to ensure the > > > > > timing of > > > > > dsi_power_on/off and the timing of pulling up/down the MIPI > > > > > signal, > > > > > the modification of v5.9 is synchronized in this series of > > > > > patches. > > > > > > > > Hello, > > > > > > > > I'm trying to debug an issue on mt8183 kukui krane sku176 device. > > > > The problem is that boe-tv101wum-nl6 panel isn't working. > > > > At boot time I can see these logs: > > > > panel-boe-tv101wum-nl6 14014000.dsi.0: failed to write command 1 > > > > panel-boe-tv101wum-nl6 14014000.dsi.0: failed to init panel: -62 > > > > and a DSI interrupt time out. > > > > > > > > Since I believe the problem is link to DSI/panel enabling > > > > sequence > > > > I have try this series but that doesn't solve the issue. > > > > I notice that when going out of deep sleep mode panel is > > > > functional. > > > > > > > > May you have any idea to debug/solve this problem ? > > > > > > > > > > Hi Benjamin, > > > > > > I think this might not be related to this series. Which kernel are > > > you using? > > > I tried the krane sku176 with linux-next 5.17-rc6 > > > (519dd6c19986696f59847ff8bf930436ccffd9a1 (tag: next-20220307, > > > linux-next/master) with or without this series, both can get the > > > display on. > > > > > > dsi related message: > > > [0.206330] mediatek-drm mediatek-drm.1.auto: Adding component > > > match for /soc/dsi@14014000 > > > [4.567577] panel-boe-tv101wum-nl6 14014000.dsi.0: supply pp3300 > > > not found, using dummy regulator > > > [4.567732] panel-boe-tv101wum-nl6 14014000.dsi.0: GPIO lookup > > > for > > > consumer enable > > > [4.567738] panel-boe-tv101wum-nl6 14014000.dsi.0: using device > > > tree for GPIO lookup > > > [4.567757] of_get_named_gpiod_flags: parsed 'enable-gpios' > > > property of node '/soc/dsi@14014000/panel@0[0]' - status (0) > > > [4.585884] panel-boe-tv101wum-nl6 14014000.dsi.0: supply pp3300 > > > not found, using dummy regulator > > > [4.586037] panel-boe-tv101wum-nl6 14014000.dsi.0: GPIO lookup > > > for > > > consumer enable > > > [4.586042] panel-boe-tv101wum-nl6 14014000.dsi.0: using device > > > tree for GPIO lookup > > > [4.586059] of_get_named_gpiod_flags: parsed 'enable-gpios' > > > property of node '/soc/dsi@14014000/panel@0[0]' - status (0) > > > [4.587430] mediatek-drm mediatek-drm.1.auto: bound 14014000.dsi > > > (ops 0xffd369a752b8) > > > > > > > > > Maybe some config is not enabled? > > > > I using 5.17.0-rc1-next-20220127 kernel tag. > > The configs look similar. > > > > I have the follow log at boot time: > > > > [1.533384] phy phy-11e5.dsi-phy.2: Looking up phy-supply from > > device tree > > [1.533402] phy phy-11e5.dsi-phy.2: Looking up phy-supply > > property in node /soc/dsi-phy@11e5 failed > > [3.173068] mediatek-drm mediatek-drm.1.auto: Adding component > > match for /soc/dsi@14014000 > > [4.671806] panel-boe-tv101wum-nl6 14014000.dsi.0: Looking up > > avdd-supply from device tree > > [4.680348] panel-boe-tv101wum-nl6 14014000.dsi.0: Looking up > > avee-supply from device tree > > [4.688784] panel-boe-tv101wum-nl6 14014000.dsi.0: Looking up > > pp3300-supply from device tree > > [4.697816] panel-boe-tv101wum-nl6 14014000.dsi.0: Looking up > > pp1800-supply from device tree > > [4.842346] panel-boe-tv101wum-nl6 14014000.dsi.0: Looking up > > avdd-supply from device tree > > [4.854573] panel-boe-tv101wum-nl6 14014000.dsi.0: Looking up > > avee-supply from device tree > > [4.862976] panel-boe-tv101wum-nl6 14014000.dsi.0: Looking up > > pp3300-supply from device tree > > [4.871568] panel-boe-tv101wum-nl6 14014000.dsi.0: Looking up > > pp1800-supply from device tree > > [4.964021] mediatek-drm mediatek-drm.1.auto: bound 14014000.dsi > > (ops mtk_dsi_component_ops) > > ... > > [ 38.273437] [drm] Wait DSI IRQ(0x0002) Timeout > > [ 38.281584] panel-boe-tv101wum-nl6 14014000.dsi.0: failed to write > > command 1 > > [ 38.288651] panel-boe-tv101wum-nl6 14014000.dsi.0: failed to init > > panel: -62 > > ... > > [ 70.113674] mediatek-drm mediatek-drm.1.auto: [drm] *ERROR* > > flip_done timed out > > [ 70.121054] mediatek-drm mediatek-drm.1.auto: [drm] *ERROR* > > [CRTC:45:crtc-0] commit wait timed out > > [ 70.130037] [drm:mtk_drm_crtc_atomic_begin] *ERROR* new event > > while there is still a pending event > > [ 70.241222] [ cut here ] > > [ 70.245898] [CRTC:45:crtc-0] vblank wait timed out > >
Re: Report 2 in ext4 and journal based on v5.17-rc1
On Sun, Mar 06, 2022 at 09:19:10AM -0500, Theodore Ts'o wrote: > On Sun, Mar 06, 2022 at 07:51:42PM +0900, Byungchul Park wrote: > > > > > > Users of DEPT must not have to understand how DEPT works in order to > > > > Users must not have to understand how Dept works for sure, and haters > > must not blame things based on what they guess wrong. > > For the record, I don't hate DEPT. I *fear* that DEPT will result in > my getting spammed with a huge number of false posiives once automated > testing systems like Syzkaller, zero-day test robot, etcs., get a hold > of it once it gets merged and start generating hundreds of automated > reports. Agree. Dept should not be a part of *automated testing system* until it finally works as much as Lockdep in terms of false positives. However, it's impossible to achieve it by doing it out of the tree. Besides automated testing system, Dept works great in the middle of developing something that is so complicated in terms of synchronization. They don't have to worry about real reports anymore, that should be reported, from getting prevented by a false positve. I will explicitely describe EXPERIMENTAL and "Dept might false-alarm" in Kconfig until it's considered a few-false-alarming tool. > > Sure, it should be done manually. I should do it on my own when that > > kind of issue arises. > > The question here is how often will it need to be done, and how easy I guess it's gonna rarely happens. I want to see too. > will it be to "do it manually"? Suppose we mark all of the DEPT false Very easy. Equal to or easier than the way we do for lockdep. But the interest would be wait/event objects rather than locks. > positives before it gets merged? How easy will it be able to suppress > future false positives in the future, as the kernel evolves? Same as - or even better than - what we do for Lockdep. And we'd better consider those activies as a code-documentation. Not only making things just work but organizing code and documenting in code, are also very meaningful. > Perhaps one method is to haved a way to take a particular wait queue, > or call to schedule(), or at the level of an entire kernel source > file, and opt it out from DEPT analysis? That way, if DEPT gets > merged, and a maintainer starts getting spammed by bogus (or Once Dept gets stable - hoplefully now that Dept is working very conservatively, there might not be as many false positives as you're concerning. The situation is in control. > That way we don't necessarily need to have a debate over how close to > zero percent false positives is necessary before DEPT can get merged. Non-sense. I would agree with you if it was so when Lockdep was merged. But I'll try to achieve almost zero false positives, again, it's impossible to do it out of tree. > And we avoid needing to force maintainers to prove that a DEPT report So... It'd be okay if Dept goes not as a part of automated testing system. Right? > is a false positive, which is from my experience hard to do, since > they get accused of being DEPT haters and not understanding DEPT. Honestly, it's not a problem of that they don't understand other domians than what they are familiar with, but another issue. I won't mention it. And it sounds like you'd do nothing unless it turns out to be problematic 100%. It's definitely the *easiest* way to maintain something because it's the same as not checking its correctness at all. Even if it's so hard to do, checking if the code is safe for real repeatedly, is what it surely should be done. Again, I understand it would be freaking hard. But it doesn't mean we should avoid it. Here, there seems to be two points you'd like to say: 1. Fundamental question. Does Dept track wait and event correctly? 2. Even if so, can Dept consider all the subtle things in the kernel? For 1, I'm willing to response to whatever it is. And not only me but we can make it perfectly work if the concept and direction is *right*. For 2, I need to ask things and try my best to fix those if it exists. Again. Dept won't be a part of *automated testing system* until it finally works as much as Lockdep in terms of false positives. Hopefully you are okay with it. --- Byungchul
Re: DSI Bridge switching
On Wed, Mar 9, 2022 at 1:11 PM Jagan Teki wrote: > > or a Hi All, > > On Thu, Oct 14, 2021 at 6:45 PM Jagan Teki wrote: > > > > Hi Laurent, > > > > On Fri, Oct 8, 2021 at 7:07 PM Laurent Pinchart > > wrote: > > > > > > Hello, > > > > > > On Fri, Oct 08, 2021 at 03:27:43PM +0200, Andrzej Hajda wrote: > > > > Hi, > > > > > > > > Removed my invalid email (I will update files next week). > > > > > > > > On 08.10.2021 13:14, Jagan Teki wrote: > > > > > Hi, > > > > > > > > > > I think this seems to be a known use case for industrial these days > > > > > with i.mx8m. > > > > > > > > > > The host DSI would configure with two bridges one for DSI to LVDS > > > > > (SN65DSI83) and another for DSI to HDMI Out (ADV7535). Technically we > > > > > can use only one bridge at a time as host DSI support single out port. > > > > > So we can have two separate device tree files for LVDS and HDMI and > > > > > load them static. > > > > > > > > > > But, one of the use cases is to support both of them in single dts, > > > > > and > > > > > - Turn On LVDS (default) > > > > > - Turn Off LVDS then Turn On HDMI when cable plug-in > > > > > > > > Are you sure it will work from hardware PoV? Do you have some demuxer? > > > > isolation of pins? > > > > > > It may be in the category of "you shouldn't do this, but it actually > > > works". I've seen the same being done with two CSI-2 camera sensors > > > connected to the same receiver, with one of them being held in reset at > > > all times. > > > > Yes. Here the design has 2 MIPI D-PHY switches. Each switch take 2 > > input data lanes and 1 clock lane from SoC and produces 4 data lanes > > and 2 clock lanes and from switch output 2 lanes and 1 clock are > > inputting to HDMI bridge and other 2 lanes and 1 clock is inputting to > > LVDS. So 1st pair of 1st switch and 1st pair of 2nd switch goes to > > HDMI and 2nd pair of 1st switch and 2nd pair of 2nd switch does to > > LVDS. > > > > However, routing of these lanes are controlled by SEL, OE GPIO pins. > > So at a time we can access only single bridge. > > > > > > > > > > The HDMI event can be detected via some HDMI-INT GPIO on-board design. > > > > > > > > > > The possible solution, I'm thinking of adding LVDS on port 1, HDMI on > > > > > port 2 in the DSI host node, and trying to attach the respective > > > > > bridge based on HDMI-INT like repeating the bridge attachment cycle > > > > > based on the HDMI-INT. > > > > > > > > I think more appropriate would be to share the same port, but provide > > > > two endpoints inside this port - we have two hardware sharing the same > > > > physical port. > > > > > > That sounds like the correct DT description to me. > > > > > > > > Can it be possible to do bridge attachment at runtime? something like > > > > > a bridge hotplug event? or any other possible solutions? > > > > > > > > > > Any suggestions? > > > > > > > > Practically it is possible, see exynos_dsi + panels, or exynos_dsi + > > > > some toshiba bridge - panel and bridge are dynamically 'plugged' and > > > > 'unplugged' from exynos_drm, but they do not use bridge chain for this > > > > and some other reasons. (un|re|)plugging should be performed of course > > > > when pipeline is off (connector disconnected). I am not sure about > > > > bridges added to bridge chain - you need to inspect all opses to ensure > > > > it can be done safely. > > > > > > > > And the main issue: Daniel does not like it :) > > > > > > Neither do I :-) Could it be handled with two DRM connectors that are > > > mutually exclusive ? > > > > How about adding lvds-connector, hdmi-connector on the pipeline and > > select them based on the switch SEL GPIO? does it make sense to do > > this implementation via display-connector.c > > I have somehow managed to make runtime switching possible between LVDS > and HDMI with the help of DRM bridges. > > | => ADV7535=> > HDMI-A Connector > DSI Host => display-switch => | > |=> SN65DSI83 => > Panel-Simple > > display-switch here is a bridge driver that can switch or attach the > downstream bridge flow based on HDMI HPD here. One potential problem > is that when we switch from LVDS to HDMI Out the HDMI Out is displayed > with the resolution that LVDS has and it is unable to display higher > HDMI resolutions even though it supports it. Does anyone aware of > changing the resolution at runtime, please let me know? > > Technically, the display-switch hardware does available in various forms > 1. MIPI Switch PI3WVR626 > 2. Conventional Mux Switch > 3. Converter bridge DSI to LVDS/HDMI (from Lontium). > > Overall I believe this can be a potential possible feature and good to > support on Mainline as the hardware is intended to design for it. > > Any thoughts on this please let me know? I wonder if it would be possible to trigger a hot plug event similar to what is done when an HDMI cable is inserted/disconnected. If one
[PATCH] drm/i915/uapi: Add DRM_I915_QUERY_GEOMETRY_SUBSLICES
Newer platforms have DSS that aren't necessarily available for both
geometry and compute, two queries will need to exist. This introduces
the first, when passing a valid engine class and engine instance in the
flags returns a topology describing geometry.
Cc: Ashutosh Dixit
Cc: Matt Roper
UMD (mesa): https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/14143
Signed-off-by: Matt Atwood
---
drivers/gpu/drm/i915/i915_query.c | 68 ++-
include/uapi/drm/i915_drm.h | 24 +++
2 files changed, 65 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_query.c
b/drivers/gpu/drm/i915/i915_query.c
index 2dfbc22857a3..0cc2670ae09c 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -9,6 +9,7 @@
#include "i915_drv.h"
#include "i915_perf.h"
#include "i915_query.h"
+#include "gt/intel_engine_user.h"
#include
static int copy_query_item(void *query_hdr, size_t query_sz,
@@ -28,36 +29,30 @@ static int copy_query_item(void *query_hdr, size_t query_sz,
return 0;
}
-static int query_topology_info(struct drm_i915_private *dev_priv,
- struct drm_i915_query_item *query_item)
+static int fill_topology_info(const struct sseu_dev_info *sseu,
+ struct drm_i915_query_item *query_item,
+ const u8 *subslice_mask)
{
- const struct sseu_dev_info *sseu = _gt(dev_priv)->info.sseu;
struct drm_i915_query_topology_info topo;
u32 slice_length, subslice_length, eu_length, total_length;
int ret;
- if (query_item->flags != 0)
- return -EINVAL;
+ BUILD_BUG_ON(sizeof(u8) != sizeof(sseu->slice_mask));
if (sseu->max_slices == 0)
return -ENODEV;
- BUILD_BUG_ON(sizeof(u8) != sizeof(sseu->slice_mask));
-
slice_length = sizeof(sseu->slice_mask);
subslice_length = sseu->max_slices * sseu->ss_stride;
eu_length = sseu->max_slices * sseu->max_subslices * sseu->eu_stride;
total_length = sizeof(topo) + slice_length + subslice_length +
eu_length;
- ret = copy_query_item(, sizeof(topo), total_length,
- query_item);
+ ret = copy_query_item(, sizeof(topo), total_length, query_item);
+
if (ret != 0)
return ret;
- if (topo.flags != 0)
- return -EINVAL;
-
memset(, 0, sizeof(topo));
topo.max_slices = sseu->max_slices;
topo.max_subslices = sseu->max_subslices;
@@ -69,27 +64,61 @@ static int query_topology_info(struct drm_i915_private
*dev_priv,
topo.eu_stride = sseu->eu_stride;
if (copy_to_user(u64_to_user_ptr(query_item->data_ptr),
- , sizeof(topo)))
+, sizeof(topo)))
return -EFAULT;
if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + sizeof(topo)),
- >slice_mask, slice_length))
+>slice_mask, slice_length))
return -EFAULT;
if (copy_to_user(u64_to_user_ptr(query_item->data_ptr +
- sizeof(topo) + slice_length),
- sseu->subslice_mask, subslice_length))
+sizeof(topo) + slice_length),
+subslice_mask, subslice_length))
return -EFAULT;
if (copy_to_user(u64_to_user_ptr(query_item->data_ptr +
- sizeof(topo) +
- slice_length + subslice_length),
- sseu->eu_mask, eu_length))
+sizeof(topo) +
+slice_length + subslice_length),
+sseu->eu_mask, eu_length))
return -EFAULT;
return total_length;
}
+static int query_topology_info(struct drm_i915_private *dev_priv,
+ struct drm_i915_query_item *query_item)
+{
+ const struct sseu_dev_info *sseu = _gt(dev_priv )->info.sseu;
+
+ if (query_item->flags != 0)
+ return -EINVAL;
+
+ return fill_topology_info(sseu, query_item, sseu->subslice_mask);
+}
+
+static int query_geometry_subslices(struct drm_i915_private *i915,
+ struct drm_i915_query_item *query_item)
+{
+ const struct sseu_dev_info *sseu;
+ struct intel_engine_cs *engine;
+ u8 engine_class, engine_instance;
+
+ if (GRAPHICS_VER_FULL(i915) < IP_VER(12, 50))
+ return -ENODEV;
+
+ engine_class = query_item->flags & 0xFF;
+ engine_instance = (query_item->flags >>8) & 0xFF;
+
+ engine = intel_engine_lookup_user(i915, engine_class, engine_instance);
+
+ if(!engine)
+ return -EINVAL;
+
+ sseu =
BUG: KASAN: use-after-free in drm_atomic_helper_wait_for_vblanks()
Hi, I was playing/testing SuperTuxKart using VirtIO-GPU driver and spotted a UAF bug in drm_atomic_helper_wait_for_vblanks(). SuperTuxKart can use DRM directly, i.e. you can run game in VT without Xorg or Wayland, this is where bugs happens. SuperTuxKart uses a non-blocking atomic page flips and UAF happens when a new atomic state is committed while there is a previous page flip still in-fly. What happens is that the new and old atomic states refer to the same CRTC state somehow. Once the older atomic state is destroyed, the CRTC state is freed and the newer atomic state continues to use the freed CRTC state. The bug is easily reproducible (at least by me) by playing SuperTuxKart for a minute. It presents on latest -next and 5.17-rc7, I haven't checked older kernel versions. I'm not an expert of the non-blocking code paths in DRM, so asking for suggestions about where the root of the problem could be. Here is the KASAN report: == BUG: KASAN: use-after-free in drm_atomic_helper_wait_for_vblanks.part.0+0x10b/0x4b0 Read of size 1 at addr 888110354809 by task kworker/u8:5/97 CPU: 1 PID: 97 Comm: kworker/u8:5 Not tainted 5.17.0-rc7-next-20220309+ #158 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014 Workqueue: events_unbound commit_work Call Trace: dump_stack_lvl+0x49/0x5e print_report.cold+0x9c/0x562 ? drm_atomic_helper_wait_for_vblanks.part.0+0x10b/0x4b0 kasan_report+0xb9/0xf0 ? drm_atomic_helper_wait_for_vblanks.part.0+0x10b/0x4b0 __asan_load1+0x4d/0x50 drm_atomic_helper_wait_for_vblanks.part.0+0x10b/0x4b0 ? page_flip_common+0x150/0x150 ? complete_all+0x41/0x50 ? drm_atomic_helper_commit_hw_done+0x1a2/0x220 drm_atomic_helper_commit_tail+0x8c/0xa0 commit_tail+0x15c/0x1d0 commit_work+0x12/0x20 process_one_work+0x50e/0x9d0 ? pwq_dec_nr_in_flight+0x120/0x120 ? do_raw_spin_lock+0x10a/0x190 worker_thread+0x2ba/0x630 ? process_one_work+0x9d0/0x9d0 kthread+0x15d/0x190 ? kthread_complete_and_exit+0x30/0x30 ret_from_fork+0x1f/0x30 Allocated by task 325: kasan_save_stack+0x26/0x50 __kasan_kmalloc+0x88/0xa0 kmem_cache_alloc_trace+0x1fa/0x380 drm_atomic_helper_crtc_duplicate_state+0x4a/0x80 drm_atomic_get_crtc_state+0xbf/0x1d0 page_flip_common+0x46/0x150 drm_atomic_helper_page_flip+0x7a/0xe0 drm_mode_page_flip_ioctl+0x9c6/0xa20 drm_ioctl_kernel+0x145/0x220 drm_ioctl+0x34e/0x5f0 __x64_sys_ioctl+0xbe/0xf0 do_syscall_64+0x35/0x80 entry_SYSCALL_64_after_hwframe+0x44/0xae Freed by task 230: kasan_save_stack+0x26/0x50 kasan_set_track+0x25/0x30 kasan_set_free_info+0x24/0x40 __kasan_slab_free+0x100/0x140 kfree+0xaf/0x310 drm_atomic_helper_crtc_destroy_state+0x1e/0x30 drm_atomic_state_default_clear+0x20e/0x5d0 __drm_atomic_state_free+0xbf/0x130 commit_tail+0x166/0x1d0 commit_work+0x12/0x20 process_one_work+0x50e/0x9d0 worker_thread+0x2ba/0x630 kthread+0x15d/0x190 ret_from_fork+0x1f/0x30 The buggy address belongs to the object at 888110354800 which belongs to the cache kmalloc-512 of size 512 The buggy address is located 9 bytes inside of 512-byte region [888110354800, 888110354a00) The buggy address belongs to the physical page: page:10a164bd refcount:1 mapcount:0 mapping: index:0x0 pfn:0x110354 head:10a164bd order:2 compound_mapcount:0 compound_pincount:0 flags: 0x80010200(slab|head|zone=2) raw: 80010200 dead0001 888100042c80 raw: 00100010 0001 page dumped because: kasan: bad access detected Memory state around the buggy address: 888110354700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc 888110354780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc >888110354800: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ^ 888110354880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb 888110354900: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb == [ cut here ]
[pull] amdgpu, amdkfd drm-next-5.18
Hi Dave, Daniel, Same PR from last week with fixed Fixes tag, clang warning fix, and a CS rework regression fix. The following changes since commit 38a15ad9488e21cad8f42d3befca20f91e5b2874: Merge tag 'amd-drm-next-5.18-2022-02-25' of https://gitlab.freedesktop.org/agd5f/linux into drm-next (2022-03-01 16:19:02 +1000) are available in the Git repository at: https://gitlab.freedesktop.org/agd5f/linux.git tags/amd-drm-next-5.18-2022-03-09 for you to fetch changes up to 96a2f0f2c8006d338a9647e068a15c6eb299f864: drm/amdgpu: fix a wrong ib reference (2022-03-09 17:28:37 -0500) amd-drm-next-5.18-2022-03-09: amdgpu: - Misc code cleanups - Misc display fixes - PSR display fixes - More RAS cleanup - Hotplug fix - Bump minor version for hotplug tests - SR-IOV fixes - GC 10.3.7 updates - Remove some firmwares which are no longer used - Mode2 reset refactor - Aldebaran fixes - Add VCN fwlog feature for VCN debugging - CS code cleanup - Fix clang warning - Fix CS clean up rebase breakage amdkfd: - SVM fixes - SMI event fixes and cleanups - vmid_pasid mapping fix for gfx10.3 Alex Deucher (4): drm/amdgpu: Use IP versions in convert_tiling_flags_to_modifier() drm/amdgpu: remove unused gpu_info firmwares drm/amdgpu/gfx10: drop unused cyan skillfish firmware drm/amdgpu/sdma5: drop unused cyan skillfish firmware Andrey Grodzovsky (2): drm/amdgpu: Fix sigsev when accessing MMIO on hot unplug. drm/amdgpu: Bump minor version for hot plug tests enabling. Anthony Koo (1): drm/amd/display: [FW Promotion] Release 0.0.106.0 Aric Cyr (1): drm/amd/display: 3.2.175 Charlene Liu (1): drm/amd/display: add verify_link_cap back for hdmi Chris Park (1): drm/amd/display: Reset VIC if HDMI_VIC is present Christian König (5): drm/amdgpu: install ctx entities with cmpxchg drm/amdgpu: header cleanup drm/amdgpu: use job and ib structures directly in CS parsers drm/amdgpu: properly embed the IBs into the job drm/amdgpu: initialize the vmid_wait with the stub fence Danijel Slivka (1): drm/amd/pm: new v3 SmuMetrics data structure for Sienna Cichlid David Yu (1): drm/amdgpu: Add DFC CAP support for aldebaran Dillon Varone (2): drm/amd/display: Add frame alternate 3D & restrict HW packed on dongles drm/amd/display: Modify plane removal sequence to avoid hangs. George Shen (1): drm/amd/display: Refactor fixed VS w/a for PHY tests Hansen Dsouza (1): drm/amd/display: Remove invalid RDPCS Programming in DAL Harish Kasiviswanathan (1): drm/amdgpu: Set correct DMA mask for aldebaran Jingwen Chen (1): drm/amd/amdgpu: set disabled vcn to no_schduler Lang Yu (1): drm/amdgpu: fix a wrong ib reference Lijo Lazar (1): drm/amdgpu: Refactor mode2 reset logic for v13.0.2 Luben Tuikov (1): drm/amd/display: Don't fill up the logs Meng Tang (1): gpu/amd: vega10_hwmgr: fix inappropriate private variable name Michael Strauss (1): drm/amd/display: Pass HostVM enable flag into DCN3.1 DML Nicholas Kazlauskas (1): drm/amd/display: Make functional resource functions non-static Philip Yang (4): Revert "drm/amdkfd: process_info lock not needed for svm" drm/amdkfd: Correct SMI event read size drm/amdkfd: Add SMI add event helper drm/amdkfd: Add format attribute to kfd_smi_event_add Prike Liang (4): drm/amdgpu: enable gfx clock gating control for GC 10.3.7 drm/amdgpu/nv: enable clock gating for GC 10.3.7 subblock drm/amdgpu: enable gfx power gating for GC 10.3.7 drm/amdgpu: enable gfxoff routine for GC 10.3.7 Qiang Yu (1): drm/amdgpu: fix suspend/resume hang regression Robin Chen (1): drm/amd/display: Pass deep sleep disabled allow info to dmub fw Ruijing Dong (2): drm/amdgpu/vcn: Update fw shared data structure drm/amdgpu/vcn: Add vcn firmware log Shah Dharati (1): drm/amd/display: Adding a dc_debug option and dmub setting to use PHY FSM for PSR Tom Rix (1): drm/amdgpu: Fix realloc of ptr Weiguo Li (1): drm/amdgpu: remove redundant null check Wesley Chalmers (1): drm/amd/display: Program OPP before ODM Yifan Zha (1): drm/amdgpu: Move CAP firmware loading to the beginning of PSP firmware list Yifan Zhang (5): drm/amdgpu: move amdgpu_gmc_noretry_set after ip_versions populated drm/amdgpu: convert code name to ip version for noretry set drm/amdkfd: judge get_atc_vmid_pasid_mapping_info before call drm/amdkfd: implement get_atc_vmid_pasid_mapping_info for gfx10.3 drm/amdkfd: bail out early if no get_atc_vmid_pasid_mapping_info jinzh (1): drm/amd/display: refine the EDID override yipechai (12): drm/amdgpu: Modify .ras_fini function pointer parameter drm/amdgpu: Optimize xxx_ras_fini
Re: [PATCH v1 0/5] Add memory shrinker to VirtIO-GPU DRM driver
On 3/10/22 00:51, Rob Clark wrote: > On Wed, Mar 9, 2022 at 12:06 PM Dmitry Osipenko > wrote: >> >> On 3/9/22 03:56, Rob Clark wrote: If we really can't track madvise state in the guest for dealing with host memory pressure, I think the better option is to introduce MADV:WILLNEED_REPLACE, ie. something to tell the host kernel that the buffer is needed but the previous contents are not (as long as the GPU VA remains the same). With this the host could allocate new pages if needed, and the guest would not need to wait for a reply from host. >>> If variant with the memory ballooning will work, then it will be >>> possible to track the state within guest-only. Let's consider the >>> simplest variant for now. >>> >>> I'll try to implement the balloon driver support in the v2 and will get >>> back to you. >>> >> >> I looked at the generic balloon driver and looks like this not what we >> want because: >> >> 1. Memory ballooning is primarily about handling memory overcommit >> situations. I.e. when there are multiple VMs consuming more memory than >> available in the system. Ballooning allows host to ask guest to give >> unused pages back to host and host could give pages to other VMs. >> >> 2. Memory ballooning operates with guest memory pages only. I.e. each >> ballooned page is reported to/from host in a form of page's DMA address. >> >> 3. There is no direct connection between host's OOM events and the >> balloon manager. I guess host could watch system's memory pressure and >> inflate VMs' balloons on low memory, releasing the guest's memory to the >> system, but apparently this use-case not supported by anyone today, at >> least I don't see Qemu supporting it. >> > > hmm, on CrOS I do see balloon getting used to balance host vs guest > memory.. but admittedly I've not yet looked closely at how that works, > and it does seem like we have some things that are not yet upstream > all over the place (not to mention crosvm vs qemu) That's interesting, I missed that CrOS uses a customized ballooning. >> So the virtio-balloon driver isn't very useful for us as-is. >> >> One possible solution could be to create something like a new >> virtio-shrinker device or add shrinker functionality to the virtio-gpu >> device, allowing host to ask guests to drop shared caches. Host then >> should become a PSI handler. I think this should be doable in a case of >> crosvm. In a case of GNU world, it could take a lot of effort to get >> everything to upstreamable state, at first there is a need to >> demonstrate real problem being solved by this solution. > > I guess with 4GB chromebooks running one or more VMs in addition to > lots of browser tabs in the host, it shouldn't be too hard to > demonstrate a problem ;-) But then guest also should have a significant amount of BOs in cache to purge, which potentially could be solved using a timer approach. > (but also, however we end up solving that, certainly shouldn't block > this series) Sure, there is no problem with extending shrinker functionality with more features later on, so the host's shrinker isn't a blocker. >> The other minor issue is that only integrated GPUs may use system's >> memory and even then they could use a dedicated memory carveout, i.e. >> releasing VRAM BOs may not help with host's OOM. In case of virgl >> context we have no clue about where buffers are physically located. On >> the other hand, in the worst case dropping host caches just won't help >> with OOM. > > Userspace should know whether the BO has CPU storage, so I don't think > this should be a problem virtio_gpu needs to worry about > >> It's now unclear how we should proceed with the host-side shrinker >> support. Thoughts? >> >> We may start easy and instead of thinking about host-side shrinker, we >> could make VirtIO-GPU driver to expire cached BOs after a certain >> timeout. Mesa already uses timeout-based BO caching, but it doesn't have >> an alarm timer and simply checks expiration when BO is allocated. Should >> be too much trouble to handle timers within Mesa since it's executed in >> application context, easier to do it in kernel, like VC4 driver does it >> for example. This is not good as a proper memory shrinker, but could be >> good enough in practice. > > I think that, given virgl uses host storage, guest shrinker should be > still useful.. so I think continue with this series. Guest shrinker indeed will be useful for virgl today. I was already questioning why virgl needs both host and guest storages, maybe it will move to a host-only storage approach in the future. I think the variant with the timer expiration actually could be interesting to try because it should allow host to purge its VM BOs by itself, preventing host OOMs. > For host shrinker, I'll have to look more at when crosvm triggers > balloon inflation. I could still go the MADV:WILLNEED_REPLACE > approach instead, which does have the advantage of host kernel not > relying on
Re: [PATCH v1 0/5] Add memory shrinker to VirtIO-GPU DRM driver
On 3/9/22 22:28, Thomas Zimmermann wrote: > Hi > > Am 09.03.22 um 12:55 schrieb Dmitry Osipenko: >> Hello, >> >> On 3/9/22 11:59, Thomas Zimmermann wrote: >>> Hi >>> >>> Am 08.03.22 um 14:17 schrieb Dmitry Osipenko: Hello, This patchset introduces memory shrinker for the VirtIO-GPU DRM driver. During OOM, the shrinker will release BOs that are marked as "not needed" by userspace using the new madvise IOCTL. The userspace in this case is the Mesa VirGL driver, it will mark the cached BOs as "not needed", allowing kernel driver to release memory of the cached shmem BOs on lowmem situations, preventing OOM kills. >>> >>> Virtio-gpu is build on top of GEM shmem helpers. I have a prototype >>> patchset that adds a shrinker to these helpers. If you want to go >>> further, you could implement something like that instead. Panfrost and >>> lima also have their own shrinker and could certainly be converted to >>> the gem-shmem shrinker. >> >> I had a thought that it could be possible to unify shrinkers into a >> common DRM framework. Could you please give me a link to yours prototype >> patchset? > > I uploaded the patches to > > > https://gitlab.freedesktop.org/tzimmermann/linux/-/commits/gem-shmem-cached-mappings > > > it's incomplete and un-debugged, but it shows what needs to be done. It > has the infrastructure, but lacks the changes to the GEM shmem code. > > The reason for this work is to keep GEM shmem pages mapped and allocated > even while the BO is neither mapped nor pinned. As it is now, GEM SHMEM > creates and releases pages on each pin and unpin, and maps and unmaps > memory ranges on each vmap and vunmap. It's all wasteful. Only the > first pin and vmap calls should establish pages and mappings and only > the purge and free functions should release them. Hm, aren't maps and pins already refcounted? > The patchset adds new helpers for BO purging to struct > drm_gem_object_funcs. With this, I think it might be possible to have > one global DRM shrinker and let it handle all BOs; independent of each > BO's memory manager. Thank you, I'll give it a try.
Re: [pull] amdgpu, amdkfd drm-next-5.18
On Thu, 10 Mar 2022 at 08:16, Alex Deucher wrote:
>
> On Wed, Mar 9, 2022 at 5:12 PM Dave Airlie wrote:
> >
> > On Tue, 8 Mar 2022 at 06:08, Alex Deucher wrote:
> > >
> > > Hi Dave, Daniel,
> > >
> > > Same PR as last week, just fixed up a bad Fixes tag.
> > >
> > > The following changes since commit
> > > 38a15ad9488e21cad8f42d3befca20f91e5b2874:
> > >
> > > Merge tag 'amd-drm-next-5.18-2022-02-25' of
> > > https://gitlab.freedesktop.org/agd5f/linux into drm-next (2022-03-01
> > > 16:19:02 +1000)
> > >
> > > are available in the Git repository at:
> > >
> > > https://gitlab.freedesktop.org/agd5f/linux.git
> > > tags/amd-drm-next-5.18-2022-03-07
> > >
> > > for you to fetch changes up to 53b97af4a44abd21344cc9f13986ba53051287bb:
> > >
> > > drm/amdkfd: Add format attribute to kfd_smi_event_add (2022-03-07
> > > 14:59:59 -0500)
> >
> > clang says no.
> >
> > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:508:6:
> > error: variable 'vmid' is used uninitialized whenever 'if' condition
> > is false [-Werror,-Wsometimes-uninitialized]
> > if (dev->kfd2kgd->get_atc_vmid_pasid_mapping_info) {
> > ^
> > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:521:6:
> > note: uninitialized use occurs here
> > if (vmid > last_vmid_to_scan) {
> > ^~~~
> > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:508:2:
> > note: remove the 'if' if its condition is always true
> > if (dev->kfd2kgd->get_atc_vmid_pasid_mapping_info) {
> > ^~~
> > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:490:19:
> > note: initialize the variable 'vmid' to silence this warning
> > unsigned int vmid;
> > ^
> > = 0
> >
>
> Already fixed in:
> https://gitlab.freedesktop.org/agd5f/linux/-/commit/455331caeea5058d6df20f31a414b68ca502ec25
> was going to send that out with additional fixes this week, but I can
> just spin a new PR if you'd prefer.
A respin would be great,
Thanks,
Dave.
Re: [pull] amdgpu, amdkfd drm-next-5.18
On Wed, Mar 9, 2022 at 5:12 PM Dave Airlie wrote:
>
> On Tue, 8 Mar 2022 at 06:08, Alex Deucher wrote:
> >
> > Hi Dave, Daniel,
> >
> > Same PR as last week, just fixed up a bad Fixes tag.
> >
> > The following changes since commit 38a15ad9488e21cad8f42d3befca20f91e5b2874:
> >
> > Merge tag 'amd-drm-next-5.18-2022-02-25' of
> > https://gitlab.freedesktop.org/agd5f/linux into drm-next (2022-03-01
> > 16:19:02 +1000)
> >
> > are available in the Git repository at:
> >
> > https://gitlab.freedesktop.org/agd5f/linux.git
> > tags/amd-drm-next-5.18-2022-03-07
> >
> > for you to fetch changes up to 53b97af4a44abd21344cc9f13986ba53051287bb:
> >
> > drm/amdkfd: Add format attribute to kfd_smi_event_add (2022-03-07
> > 14:59:59 -0500)
>
> clang says no.
>
> /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:508:6:
> error: variable 'vmid' is used uninitialized whenever 'if' condition
> is false [-Werror,-Wsometimes-uninitialized]
> if (dev->kfd2kgd->get_atc_vmid_pasid_mapping_info) {
> ^
> /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:521:6:
> note: uninitialized use occurs here
> if (vmid > last_vmid_to_scan) {
> ^~~~
> /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:508:2:
> note: remove the 'if' if its condition is always true
> if (dev->kfd2kgd->get_atc_vmid_pasid_mapping_info) {
> ^~~
> /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:490:19:
> note: initialize the variable 'vmid' to silence this warning
> unsigned int vmid;
> ^
> = 0
>
Already fixed in:
https://gitlab.freedesktop.org/agd5f/linux/-/commit/455331caeea5058d6df20f31a414b68ca502ec25
was going to send that out with additional fixes this week, but I can
just spin a new PR if you'd prefer.
Alex
> Dave.
> >
> >
> > amd-drm-next-5.18-2022-03-07:
> >
> > amdgpu:
> > - Misc code cleanups
> > - Misc display fixes
> > - PSR display fixes
> > - More RAS cleanup
> > - Hotplug fix
> > - Bump minor version for hotplug tests
> > - SR-IOV fixes
> > - GC 10.3.7 updates
> > - Remove some firmwares which are no longer used
> > - Mode2 reset refactor
> > - Aldebaran fixes
> > - Add VCN fwlog feature for VCN debugging
> > - CS code cleanup
> >
> > amdkfd:
> > - SVM fixes
> > - SMI event fixes and cleanups
> > - vmid_pasid mapping fix for gfx10.3
> >
> >
> > Alex Deucher (4):
> > drm/amdgpu: Use IP versions in convert_tiling_flags_to_modifier()
> > drm/amdgpu: remove unused gpu_info firmwares
> > drm/amdgpu/gfx10: drop unused cyan skillfish firmware
> > drm/amdgpu/sdma5: drop unused cyan skillfish firmware
> >
> > Andrey Grodzovsky (2):
> > drm/amdgpu: Fix sigsev when accessing MMIO on hot unplug.
> > drm/amdgpu: Bump minor version for hot plug tests enabling.
> >
> > Anthony Koo (1):
> > drm/amd/display: [FW Promotion] Release 0.0.106.0
> >
> > Aric Cyr (1):
> > drm/amd/display: 3.2.175
> >
> > Charlene Liu (1):
> > drm/amd/display: add verify_link_cap back for hdmi
> >
> > Chris Park (1):
> > drm/amd/display: Reset VIC if HDMI_VIC is present
> >
> > Christian König (5):
> > drm/amdgpu: install ctx entities with cmpxchg
> > drm/amdgpu: header cleanup
> > drm/amdgpu: use job and ib structures directly in CS parsers
> > drm/amdgpu: properly embed the IBs into the job
> > drm/amdgpu: initialize the vmid_wait with the stub fence
> >
> > Danijel Slivka (1):
> > drm/amd/pm: new v3 SmuMetrics data structure for Sienna Cichlid
> >
> > David Yu (1):
> > drm/amdgpu: Add DFC CAP support for aldebaran
> >
> > Dillon Varone (2):
> > drm/amd/display: Add frame alternate 3D & restrict HW packed on
> > dongles
> > drm/amd/display: Modify plane removal sequence to avoid hangs.
> >
> > George Shen (1):
> > drm/amd/display: Refactor fixed VS w/a for PHY tests
> >
> > Hansen Dsouza (1):
> > drm/amd/display: Remove invalid RDPCS Programming in DAL
> >
> > Harish Kasiviswanathan (1):
> > drm/amdgpu: Set correct DMA mask for aldebaran
> >
> > Jingwen Chen (1):
> > drm/amd/amdgpu: set disabled vcn to no_schduler
> >
> > Lijo Lazar (1):
> > drm/amdgpu: Refactor mode2 reset logic for v13.0.2
> >
> > Luben Tuikov (1):
> > drm/amd/display: Don't fill up the logs
> >
> > Meng Tang (1):
> > gpu/amd: vega10_hwmgr: fix inappropriate private variable name
> >
> > Michael Strauss (1):
> > drm/amd/display: Pass HostVM enable flag into DCN3.1 DML
> >
> > Nicholas Kazlauskas (1):
> >
Re: [pull] amdgpu, amdkfd drm-next-5.18
On Tue, 8 Mar 2022 at 06:08, Alex Deucher wrote:
>
> Hi Dave, Daniel,
>
> Same PR as last week, just fixed up a bad Fixes tag.
>
> The following changes since commit 38a15ad9488e21cad8f42d3befca20f91e5b2874:
>
> Merge tag 'amd-drm-next-5.18-2022-02-25' of
> https://gitlab.freedesktop.org/agd5f/linux into drm-next (2022-03-01 16:19:02
> +1000)
>
> are available in the Git repository at:
>
> https://gitlab.freedesktop.org/agd5f/linux.git
> tags/amd-drm-next-5.18-2022-03-07
>
> for you to fetch changes up to 53b97af4a44abd21344cc9f13986ba53051287bb:
>
> drm/amdkfd: Add format attribute to kfd_smi_event_add (2022-03-07 14:59:59
> -0500)
clang says no.
/home/airlied/devel/kernel/dim/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:508:6:
error: variable 'vmid' is used uninitialized whenever 'if' condition
is false [-Werror,-Wsometimes-uninitialized]
if (dev->kfd2kgd->get_atc_vmid_pasid_mapping_info) {
^
/home/airlied/devel/kernel/dim/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:521:6:
note: uninitialized use occurs here
if (vmid > last_vmid_to_scan) {
^~~~
/home/airlied/devel/kernel/dim/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:508:2:
note: remove the 'if' if its condition is always true
if (dev->kfd2kgd->get_atc_vmid_pasid_mapping_info) {
^~~
/home/airlied/devel/kernel/dim/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:490:19:
note: initialize the variable 'vmid' to silence this warning
unsigned int vmid;
^
= 0
Dave.
>
>
> amd-drm-next-5.18-2022-03-07:
>
> amdgpu:
> - Misc code cleanups
> - Misc display fixes
> - PSR display fixes
> - More RAS cleanup
> - Hotplug fix
> - Bump minor version for hotplug tests
> - SR-IOV fixes
> - GC 10.3.7 updates
> - Remove some firmwares which are no longer used
> - Mode2 reset refactor
> - Aldebaran fixes
> - Add VCN fwlog feature for VCN debugging
> - CS code cleanup
>
> amdkfd:
> - SVM fixes
> - SMI event fixes and cleanups
> - vmid_pasid mapping fix for gfx10.3
>
>
> Alex Deucher (4):
> drm/amdgpu: Use IP versions in convert_tiling_flags_to_modifier()
> drm/amdgpu: remove unused gpu_info firmwares
> drm/amdgpu/gfx10: drop unused cyan skillfish firmware
> drm/amdgpu/sdma5: drop unused cyan skillfish firmware
>
> Andrey Grodzovsky (2):
> drm/amdgpu: Fix sigsev when accessing MMIO on hot unplug.
> drm/amdgpu: Bump minor version for hot plug tests enabling.
>
> Anthony Koo (1):
> drm/amd/display: [FW Promotion] Release 0.0.106.0
>
> Aric Cyr (1):
> drm/amd/display: 3.2.175
>
> Charlene Liu (1):
> drm/amd/display: add verify_link_cap back for hdmi
>
> Chris Park (1):
> drm/amd/display: Reset VIC if HDMI_VIC is present
>
> Christian König (5):
> drm/amdgpu: install ctx entities with cmpxchg
> drm/amdgpu: header cleanup
> drm/amdgpu: use job and ib structures directly in CS parsers
> drm/amdgpu: properly embed the IBs into the job
> drm/amdgpu: initialize the vmid_wait with the stub fence
>
> Danijel Slivka (1):
> drm/amd/pm: new v3 SmuMetrics data structure for Sienna Cichlid
>
> David Yu (1):
> drm/amdgpu: Add DFC CAP support for aldebaran
>
> Dillon Varone (2):
> drm/amd/display: Add frame alternate 3D & restrict HW packed on dongles
> drm/amd/display: Modify plane removal sequence to avoid hangs.
>
> George Shen (1):
> drm/amd/display: Refactor fixed VS w/a for PHY tests
>
> Hansen Dsouza (1):
> drm/amd/display: Remove invalid RDPCS Programming in DAL
>
> Harish Kasiviswanathan (1):
> drm/amdgpu: Set correct DMA mask for aldebaran
>
> Jingwen Chen (1):
> drm/amd/amdgpu: set disabled vcn to no_schduler
>
> Lijo Lazar (1):
> drm/amdgpu: Refactor mode2 reset logic for v13.0.2
>
> Luben Tuikov (1):
> drm/amd/display: Don't fill up the logs
>
> Meng Tang (1):
> gpu/amd: vega10_hwmgr: fix inappropriate private variable name
>
> Michael Strauss (1):
> drm/amd/display: Pass HostVM enable flag into DCN3.1 DML
>
> Nicholas Kazlauskas (1):
> drm/amd/display: Make functional resource functions non-static
>
> Philip Yang (4):
> Revert "drm/amdkfd: process_info lock not needed for svm"
> drm/amdkfd: Correct SMI event read size
> drm/amdkfd: Add SMI add event helper
> drm/amdkfd: Add format attribute to kfd_smi_event_add
>
> Prike Liang (4):
> drm/amdgpu: enable gfx clock gating control for GC 10.3.7
> drm/amdgpu/nv: enable clock gating for GC 10.3.7 subblock
> drm/amdgpu: enable gfx power gating for GC 10.3.7
> drm/amdgpu: enable gfxoff routine
Re: [PATCH v1 0/5] Add memory shrinker to VirtIO-GPU DRM driver
On Wed, Mar 9, 2022 at 12:06 PM Dmitry Osipenko wrote: > > On 3/9/22 03:56, Rob Clark wrote: > >> If we really can't track madvise state in the guest for dealing with > >> host memory pressure, I think the better option is to introduce > >> MADV:WILLNEED_REPLACE, ie. something to tell the host kernel that the > >> buffer is needed but the previous contents are not (as long as the GPU > >> VA remains the same). With this the host could allocate new pages if > >> needed, and the guest would not need to wait for a reply from host. > > If variant with the memory ballooning will work, then it will be > > possible to track the state within guest-only. Let's consider the > > simplest variant for now. > > > > I'll try to implement the balloon driver support in the v2 and will get > > back to you. > > > > I looked at the generic balloon driver and looks like this not what we > want because: > > 1. Memory ballooning is primarily about handling memory overcommit > situations. I.e. when there are multiple VMs consuming more memory than > available in the system. Ballooning allows host to ask guest to give > unused pages back to host and host could give pages to other VMs. > > 2. Memory ballooning operates with guest memory pages only. I.e. each > ballooned page is reported to/from host in a form of page's DMA address. > > 3. There is no direct connection between host's OOM events and the > balloon manager. I guess host could watch system's memory pressure and > inflate VMs' balloons on low memory, releasing the guest's memory to the > system, but apparently this use-case not supported by anyone today, at > least I don't see Qemu supporting it. > hmm, on CrOS I do see balloon getting used to balance host vs guest memory.. but admittedly I've not yet looked closely at how that works, and it does seem like we have some things that are not yet upstream all over the place (not to mention crosvm vs qemu) > > So the virtio-balloon driver isn't very useful for us as-is. > > One possible solution could be to create something like a new > virtio-shrinker device or add shrinker functionality to the virtio-gpu > device, allowing host to ask guests to drop shared caches. Host then > should become a PSI handler. I think this should be doable in a case of > crosvm. In a case of GNU world, it could take a lot of effort to get > everything to upstreamable state, at first there is a need to > demonstrate real problem being solved by this solution. I guess with 4GB chromebooks running one or more VMs in addition to lots of browser tabs in the host, it shouldn't be too hard to demonstrate a problem ;-) (but also, however we end up solving that, certainly shouldn't block this series) > The other minor issue is that only integrated GPUs may use system's > memory and even then they could use a dedicated memory carveout, i.e. > releasing VRAM BOs may not help with host's OOM. In case of virgl > context we have no clue about where buffers are physically located. On > the other hand, in the worst case dropping host caches just won't help > with OOM. Userspace should know whether the BO has CPU storage, so I don't think this should be a problem virtio_gpu needs to worry about > It's now unclear how we should proceed with the host-side shrinker > support. Thoughts? > > We may start easy and instead of thinking about host-side shrinker, we > could make VirtIO-GPU driver to expire cached BOs after a certain > timeout. Mesa already uses timeout-based BO caching, but it doesn't have > an alarm timer and simply checks expiration when BO is allocated. Should > be too much trouble to handle timers within Mesa since it's executed in > application context, easier to do it in kernel, like VC4 driver does it > for example. This is not good as a proper memory shrinker, but could be > good enough in practice. I think that, given virgl uses host storage, guest shrinker should be still useful.. so I think continue with this series. For host shrinker, I'll have to look more at when crosvm triggers balloon inflation. I could still go the MADV:WILLNEED_REPLACE approach instead, which does have the advantage of host kernel not relying on host userspace or vm having a chance to run in order to release pages. The downside (perhaps?) is it would be more specific to virtgpu-native-context and less so to virgl or venus (but I guess there doesn't currently exist a way for madvise to be useful for vk drivers). BR, -R
Re: [PATCH] fixup! drm/amdkfd: CRIU export dmabuf handles for GTT BOs
No problem. squashed in:
https://gitlab.freedesktop.org/agd5f/linux/-/commit/74041e46982cd627e7b52f9c3ed37d23a4973b5f
Alex
Alex
On Wed, Mar 9, 2022 at 4:23 PM Felix Kuehling wrote:
>
> On 2022-03-09 16:20, David Yat Sin wrote:
> > Signed-off-by: David Yat Sin
>
> Please add the commit description back. And let's wait for Alex to
> confirm that the fixup-method is OK. With that fixed, the patch is
>
> Reviewed-by: Felix Kuehling
>
>
> > ---
> > drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 6 ++
> > include/uapi/linux/kfd_ioctl.h | 2 ++
> > 2 files changed, 8 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > index e1e2362841f8..607f65ab39ac 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > @@ -1767,7 +1767,10 @@ static int criu_checkpoint_bos(struct kfd_process *p,
> > _bucket->dmabuf_fd);
> > if (ret)
> > goto exit;
> > + } else {
> > + bo_bucket->dmabuf_fd = KFD_INVALID_FD;
> > }
> > +
> > if (bo_bucket->alloc_flags &
> > KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL)
> > bo_bucket->offset = KFD_MMAP_TYPE_DOORBELL |
> > KFD_MMAP_GPU_ID(pdd->dev->id);
> > @@ -2219,7 +,10 @@ static int criu_restore_bo(struct kfd_process *p,
> > _bucket->dmabuf_fd);
> > if (ret)
> > return ret;
> > + } else {
> > + bo_bucket->dmabuf_fd = KFD_INVALID_FD;
> > }
> > +
> > return 0;
> > }
> >
> > diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
> > index eb9ff85f8556..42975e940758 100644
> > --- a/include/uapi/linux/kfd_ioctl.h
> > +++ b/include/uapi/linux/kfd_ioctl.h
> > @@ -196,6 +196,8 @@ struct kfd_ioctl_dbg_wave_control_args {
> > __u32 buf_size_in_bytes;/*including gpu_id and buf_size */
> > };
> >
> > +#define KFD_INVALID_FD 0x
> > +
> > /* Matching HSA_EVENTTYPE */
> > #define KFD_IOC_EVENT_SIGNAL0
> > #define KFD_IOC_EVENT_NODECHANGE1
Re: [RFC v3 8/8] selftests: Add binder cgroup gpu memory transfer test
On 3/9/22 9:52 AM, T.J. Mercier wrote:
This test verifies that the cgroup GPU memory charge is transferred
correctly when a dmabuf is passed between processes in two different
cgroups and the sender specifies BINDER_BUFFER_FLAG_SENDER_NO_NEED in the
binder transaction data containing the dmabuf file descriptor.
Signed-off-by: T.J. Mercier
---
.../selftests/drivers/android/binder/Makefile | 8 +
.../drivers/android/binder/binder_util.c | 254 +
.../drivers/android/binder/binder_util.h | 32 ++
.../selftests/drivers/android/binder/config | 4 +
.../binder/test_dmabuf_cgroup_transfer.c | 480 ++
5 files changed, 778 insertions(+)
create mode 100644 tools/testing/selftests/drivers/android/binder/Makefile
create mode 100644
tools/testing/selftests/drivers/android/binder/binder_util.c
create mode 100644
tools/testing/selftests/drivers/android/binder/binder_util.h
create mode 100644 tools/testing/selftests/drivers/android/binder/config
create mode 100644
tools/testing/selftests/drivers/android/binder/test_dmabuf_cgroup_transfer.c
diff --git a/tools/testing/selftests/drivers/android/binder/Makefile
b/tools/testing/selftests/drivers/android/binder/Makefile
new file mode 100644
index ..726439d10675
--- /dev/null
+++ b/tools/testing/selftests/drivers/android/binder/Makefile
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0
+CFLAGS += -Wall
+
Does this test inteded to be built on all architectures? Is arch
check necessary here?
Also does this test require root previleges - I see mount and
unmount operations in the test. If so add root check and skip
if non-root user runs the test.
+TEST_GEN_PROGS = test_dmabuf_cgroup_transfer
+
+include ../../../lib.mk
+
+$(OUTPUT)/test_dmabuf_cgroup_transfer: ../../../cgroup/cgroup_util.c
binder_util.c
diff --git a/tools/testing/selftests/drivers/android/binder/binder_util.c
b/tools/testing/selftests/drivers/android/binder/binder_util.c
new file mode 100644
index ..c9dcf5b9d42b
--- /dev/null
+++ b/tools/testing/selftests/drivers/android/binder/binder_util.c
@@ -0,0 +1,254 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include "binder_util.h"
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+static const size_t BINDER_MMAP_SIZE = 64 * 1024;
+
+static void binderfs_unmount(const char *mountpoint)
+{
+ if (umount2(mountpoint, MNT_DETACH))
+ fprintf(stderr, "Failed to unmount binderfs at %s: %s\n",
+ mountpoint, strerror(errno));
+ else
+ fprintf(stderr, "Binderfs unmounted: %s\n", mountpoint);
+
+ if (rmdir(mountpoint))
+ fprintf(stderr, "Failed to remove binderfs mount %s: %s\n",
+ mountpoint, strerror(errno));
+ else
+ fprintf(stderr, "Binderfs mountpoint destroyed: %s\n",
mountpoint);
Does umount require root previleges? Same commment as above about
non-root user running test.
+}
+
+struct binderfs_ctx create_binderfs(const char *name)
+{
+ int fd, ret, saved_errno;
+ struct binderfs_device device = { 0 };
+ struct binderfs_ctx ctx = { 0 };
+
+ /*
+* P_tmpdir is set to "/tmp/" on Android platforms where Binder is most
+* commonly used, but this path does not actually exist on Android. We
+* will first try using "/data/local/tmp" and fallback to P_tmpdir if
+* that fails for non-Android platforms.
+*/
+ static const char tmpdir[] = "/data/local/tmp";
+ static const size_t MAX_TMPDIR_SIZE =
+ sizeof(tmpdir) > sizeof(P_tmpdir) ?
+ sizeof(tmpdir) : sizeof(P_tmpdir);
+ static const char template[] = "/binderfs_XX";
+
+ char *mkdtemp_result;
+ char binderfs_mntpt[MAX_TMPDIR_SIZE + sizeof(template)];
+ char device_path[MAX_TMPDIR_SIZE + sizeof(template) +
BINDERFS_MAX_NAME];
+
+ snprintf(binderfs_mntpt, sizeof(binderfs_mntpt), "%s%s", tmpdir,
template);
+
+ mkdtemp_result = mkdtemp(binderfs_mntpt);
+ if (mkdtemp_result == NULL) {
+ fprintf(stderr, "Failed to create binderfs mountpoint at %s:
%s.\n",
+ binderfs_mntpt, strerror(errno));
+ fprintf(stderr, "Trying fallback mountpoint...\n");
+ snprintf(binderfs_mntpt, sizeof(binderfs_mntpt), "%s%s",
P_tmpdir, template);
+ if (mkdtemp(binderfs_mntpt) == NULL) {
+ fprintf(stderr, "Failed to create binderfs mountpoint at %s:
%s\n",
+ binderfs_mntpt, strerror(errno));
+ return ctx;
+ }
+ }
+ fprintf(stderr, "Binderfs mountpoint created at %s\n", binderfs_mntpt);
Does mount require root previleges? Same commment as above about
non-root user running test.
+
+ if (mount(NULL, binderfs_mntpt, "binder", 0,
Re: [PATCH] drm/amdkfd: Set handle to invalid for non GTT/VRAM BOs
On Wed, Mar 9, 2022 at 4:21 PM Alex Deucher wrote:
>
> On Wed, Mar 9, 2022 at 4:10 PM Felix Kuehling wrote:
> >
> > On 2022-03-09 12:41, David Yat Sin wrote:
> > > Set dmabuf handle to invalid for BOs that cannot be accessed using SDMA
> > > during checkpoint/restore.
> > >
> > > Signed-off-by: David Yat Sin
> > > ---
> > > drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 8 ++--
> > > include/uapi/linux/kfd_ioctl.h | 2 ++
> > > 2 files changed, 8 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > > b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > > index e1e2362841f8..1ffa976ad318 100644
> > > --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > > @@ -1767,7 +1767,9 @@ static int criu_checkpoint_bos(struct kfd_process
> > > *p,
> > > _bucket->dmabuf_fd);
> > > if (ret)
> > > goto exit;
> > > - }
> > > + } else
> > > + bo_bucket->dmabuf_fd = KFD_INVALID_FD;
> >
> > Minor nit-pick: It would be better to use {} around the else-branch for
> > consistency with the if-branch. Same below.
> >
> > Ideally, this should have been part of the patch that bumped the KFD
> > version to 1.8. Alex, is there a way to squash this when you send this
> > in a pull-request for drm-next? Maybe if we create the commit with "git
> > commit --fixup" you can let auto-squash handle it.
> >
>
> When did that patch land? If I haven't included it in a PR yet, I can
> squash this in.
Ah, I see it. It was from yesterday, so no problem.
Alex
>
> Alex
>
>
> > Other than that, the patch looks good to me.
> >
> > Regards,
> >Felix
> >
> >
> > > +
> > > if (bo_bucket->alloc_flags &
> > > KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL)
> > > bo_bucket->offset = KFD_MMAP_TYPE_DOORBELL |
> > > KFD_MMAP_GPU_ID(pdd->dev->id);
> > > @@ -2219,7 +2221,9 @@ static int criu_restore_bo(struct kfd_process *p,
> > > _bucket->dmabuf_fd);
> > > if (ret)
> > > return ret;
> > > - }
> > > + } else
> > > + bo_bucket->dmabuf_fd = KFD_INVALID_FD;
> > > +
> > > return 0;
> > > }
> > >
> > > diff --git a/include/uapi/linux/kfd_ioctl.h
> > > b/include/uapi/linux/kfd_ioctl.h
> > > index eb9ff85f8556..42975e940758 100644
> > > --- a/include/uapi/linux/kfd_ioctl.h
> > > +++ b/include/uapi/linux/kfd_ioctl.h
> > > @@ -196,6 +196,8 @@ struct kfd_ioctl_dbg_wave_control_args {
> > > __u32 buf_size_in_bytes;/*including gpu_id and buf_size */
> > > };
> > >
> > > +#define KFD_INVALID_FD 0x
> > > +
> > > /* Matching HSA_EVENTTYPE */
> > > #define KFD_IOC_EVENT_SIGNAL0
> > > #define KFD_IOC_EVENT_NODECHANGE1
Re: [PATCH] fixup! drm/amdkfd: CRIU export dmabuf handles for GTT BOs
On 2022-03-09 16:20, David Yat Sin wrote:
Signed-off-by: David Yat Sin
Please add the commit description back. And let's wait for Alex to
confirm that the fixup-method is OK. With that fixed, the patch is
Reviewed-by: Felix Kuehling
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 6 ++
include/uapi/linux/kfd_ioctl.h | 2 ++
2 files changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index e1e2362841f8..607f65ab39ac 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1767,7 +1767,10 @@ static int criu_checkpoint_bos(struct kfd_process *p,
_bucket->dmabuf_fd);
if (ret)
goto exit;
+ } else {
+ bo_bucket->dmabuf_fd = KFD_INVALID_FD;
}
+
if (bo_bucket->alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL)
bo_bucket->offset = KFD_MMAP_TYPE_DOORBELL |
KFD_MMAP_GPU_ID(pdd->dev->id);
@@ -2219,7 +,10 @@ static int criu_restore_bo(struct kfd_process *p,
_bucket->dmabuf_fd);
if (ret)
return ret;
+ } else {
+ bo_bucket->dmabuf_fd = KFD_INVALID_FD;
}
+
return 0;
}
diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
index eb9ff85f8556..42975e940758 100644
--- a/include/uapi/linux/kfd_ioctl.h
+++ b/include/uapi/linux/kfd_ioctl.h
@@ -196,6 +196,8 @@ struct kfd_ioctl_dbg_wave_control_args {
__u32 buf_size_in_bytes;/*including gpu_id and buf_size */
};
+#define KFD_INVALID_FD 0x
+
/* Matching HSA_EVENTTYPE */
#define KFD_IOC_EVENT_SIGNAL 0
#define KFD_IOC_EVENT_NODECHANGE 1
Re: [PATCH] drm/amdkfd: Set handle to invalid for non GTT/VRAM BOs
On Wed, Mar 9, 2022 at 4:10 PM Felix Kuehling wrote:
>
> On 2022-03-09 12:41, David Yat Sin wrote:
> > Set dmabuf handle to invalid for BOs that cannot be accessed using SDMA
> > during checkpoint/restore.
> >
> > Signed-off-by: David Yat Sin
> > ---
> > drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 8 ++--
> > include/uapi/linux/kfd_ioctl.h | 2 ++
> > 2 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > index e1e2362841f8..1ffa976ad318 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > @@ -1767,7 +1767,9 @@ static int criu_checkpoint_bos(struct kfd_process *p,
> > _bucket->dmabuf_fd);
> > if (ret)
> > goto exit;
> > - }
> > + } else
> > + bo_bucket->dmabuf_fd = KFD_INVALID_FD;
>
> Minor nit-pick: It would be better to use {} around the else-branch for
> consistency with the if-branch. Same below.
>
> Ideally, this should have been part of the patch that bumped the KFD
> version to 1.8. Alex, is there a way to squash this when you send this
> in a pull-request for drm-next? Maybe if we create the commit with "git
> commit --fixup" you can let auto-squash handle it.
>
When did that patch land? If I haven't included it in a PR yet, I can
squash this in.
Alex
> Other than that, the patch looks good to me.
>
> Regards,
>Felix
>
>
> > +
> > if (bo_bucket->alloc_flags &
> > KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL)
> > bo_bucket->offset = KFD_MMAP_TYPE_DOORBELL |
> > KFD_MMAP_GPU_ID(pdd->dev->id);
> > @@ -2219,7 +2221,9 @@ static int criu_restore_bo(struct kfd_process *p,
> > _bucket->dmabuf_fd);
> > if (ret)
> > return ret;
> > - }
> > + } else
> > + bo_bucket->dmabuf_fd = KFD_INVALID_FD;
> > +
> > return 0;
> > }
> >
> > diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
> > index eb9ff85f8556..42975e940758 100644
> > --- a/include/uapi/linux/kfd_ioctl.h
> > +++ b/include/uapi/linux/kfd_ioctl.h
> > @@ -196,6 +196,8 @@ struct kfd_ioctl_dbg_wave_control_args {
> > __u32 buf_size_in_bytes;/*including gpu_id and buf_size */
> > };
> >
> > +#define KFD_INVALID_FD 0x
> > +
> > /* Matching HSA_EVENTTYPE */
> > #define KFD_IOC_EVENT_SIGNAL0
> > #define KFD_IOC_EVENT_NODECHANGE1
[PATCH] fixup! drm/amdkfd: CRIU export dmabuf handles for GTT BOs
Signed-off-by: David Yat Sin
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 6 ++
include/uapi/linux/kfd_ioctl.h | 2 ++
2 files changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index e1e2362841f8..607f65ab39ac 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1767,7 +1767,10 @@ static int criu_checkpoint_bos(struct kfd_process *p,
_bucket->dmabuf_fd);
if (ret)
goto exit;
+ } else {
+ bo_bucket->dmabuf_fd = KFD_INVALID_FD;
}
+
if (bo_bucket->alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL)
bo_bucket->offset = KFD_MMAP_TYPE_DOORBELL |
KFD_MMAP_GPU_ID(pdd->dev->id);
@@ -2219,7 +,10 @@ static int criu_restore_bo(struct kfd_process *p,
_bucket->dmabuf_fd);
if (ret)
return ret;
+ } else {
+ bo_bucket->dmabuf_fd = KFD_INVALID_FD;
}
+
return 0;
}
diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
index eb9ff85f8556..42975e940758 100644
--- a/include/uapi/linux/kfd_ioctl.h
+++ b/include/uapi/linux/kfd_ioctl.h
@@ -196,6 +196,8 @@ struct kfd_ioctl_dbg_wave_control_args {
__u32 buf_size_in_bytes;/*including gpu_id and buf_size */
};
+#define KFD_INVALID_FD 0x
+
/* Matching HSA_EVENTTYPE */
#define KFD_IOC_EVENT_SIGNAL 0
#define KFD_IOC_EVENT_NODECHANGE 1
--
2.35.1
Re: [Intel-gfx] [PATCH v3 4/4] drm/i915: Improve long running OCL w/a for GuC submission
On 3/8/2022 01:41, Tvrtko Ursulin wrote: On 03/03/2022 22:37, john.c.harri...@intel.com wrote: From: John Harrison A workaround was added to the driver to allow OpenCL workloads to run 'forever' by disabling pre-emption on the RCS engine for Gen12. It is not totally unbound as the heartbeat will kick in eventually and cause a reset of the hung engine. However, this does not work well in GuC submission mode. In GuC mode, the pre-emption timeout is how GuC detects hung contexts and triggers a per engine reset. Thus, disabling the timeout means also losing all per engine reset ability. A full GT reset will still occur when the heartbeat finally expires, but that is a much more destructive and undesirable mechanism. The purpose of the workaround is actually to give OpenCL tasks longer to reach a pre-emption point after a pre-emption request has been issued. This is necessary because Gen12 does not support mid-thread pre-emption and OpenCL can have long running threads. So, rather than disabling the timeout completely, just set it to a 'long' value. v2: Review feedback from Tvrtko - must hard code the 'long' value instead of determining it algorithmically. So make it an extra CONFIG definition. Also, remove the execlist centric comment from the existing pre-emption timeout CONFIG option given that it applies to more than just execlists. Signed-off-by: John Harrison Reviewed-by: Daniele Ceraolo Spurio (v1) Acked-by: Michal Mrozek --- drivers/gpu/drm/i915/Kconfig.profile | 26 +++ drivers/gpu/drm/i915/gt/intel_engine_cs.c | 9 ++-- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/Kconfig.profile b/drivers/gpu/drm/i915/Kconfig.profile index 39328567c200..7cc38d25ee5c 100644 --- a/drivers/gpu/drm/i915/Kconfig.profile +++ b/drivers/gpu/drm/i915/Kconfig.profile @@ -57,10 +57,28 @@ config DRM_I915_PREEMPT_TIMEOUT default 640 # milliseconds help How long to wait (in milliseconds) for a preemption event to occur - when submitting a new context via execlists. If the current context - does not hit an arbitration point and yield to HW before the timer - expires, the HW will be reset to allow the more important context - to execute. + when submitting a new context. If the current context does not hit + an arbitration point and yield to HW before the timer expires, the + HW will be reset to allow the more important context to execute. + + This is adjustable via + /sys/class/drm/card?/engine/*/preempt_timeout_ms + + May be 0 to disable the timeout. + + The compiled in default may get overridden at driver probe time on + certain platforms and certain engines which will be reflected in the + sysfs control. + +config DRM_I915_PREEMPT_TIMEOUT_COMPUTE + int "Preempt timeout for compute engines (ms, jiffy granularity)" + default 7500 # milliseconds + help + How long to wait (in milliseconds) for a preemption event to occur + when submitting a new context to a compute capable engine. If the + current context does not hit an arbitration point and yield to HW + before the timer expires, the HW will be reset to allow the more + important context to execute. This is adjustable via /sys/class/drm/card?/engine/*/preempt_timeout_ms diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index 4185c7338581..cc0954ad836a 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -438,9 +438,14 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id, engine->props.timeslice_duration_ms = CONFIG_DRM_I915_TIMESLICE_DURATION; - /* Override to uninterruptible for OpenCL workloads. */ + /* + * Mid-thread pre-emption is not available in Gen12. Unfortunately, + * some OpenCL workloads run quite long threads. That means they get + * reset due to not pre-empting in a timely manner. So, bump the + * pre-emption timeout value to be much higher for compute engines. + */ if (GRAPHICS_VER(i915) == 12 && (engine->flags & I915_ENGINE_HAS_RCS_REG_STATE)) - engine->props.preempt_timeout_ms = 0; + engine->props.preempt_timeout_ms = CONFIG_DRM_I915_PREEMPT_TIMEOUT_COMPUTE; I wouldn't go as far as adding a config option since as it is it only applies to Gen12 but Kconfig text says nothing about that. And I am not saying you should add a Gen12 specific config option, that would be weird. So IMO just drop it. You were the one arguing that the driver was illegally overriding the user's explicitly chosen settings, including the compile time config options. Just having a hardcoded magic number in the driver is the absolute worst kind of override there is. And technically, the config option is not Gen12 specific. It is actually compute
Re: [Intel-gfx] [PATCH v3 1/4] drm/i915/guc: Limit scheduling properties to avoid overflow
On 3/8/2022 01:43, Tvrtko Ursulin wrote:
On 03/03/2022 22:37, john.c.harri...@intel.com wrote:
From: John Harrison
GuC converts the pre-emption timeout and timeslice quantum values into
clock ticks internally. That significantly reduces the point of 32bit
overflow. On current platforms, worst case scenario is approximately
110 seconds. Rather than allowing the user to set higher values and
then get confused by early timeouts, add limits when setting these
values.
v2: Add helper functins for clamping (review feedback from Tvrtko).
Signed-off-by: John Harrison
Reviewed-by: Daniele Ceraolo Spurio
(v1)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index b3a429a92c0d..8208164c25e7 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -2218,13 +2218,24 @@ static inline u32
get_children_join_value(struct intel_context *ce,
static void guc_context_policy_init(struct intel_engine_cs *engine,
struct guc_lrc_desc *desc)
{
+ struct drm_device *drm = >i915->drm;
+
desc->policy_flags = 0;
if (engine->flags & I915_ENGINE_WANT_FORCED_PREEMPTION)
desc->policy_flags |=
CONTEXT_POLICY_FLAG_PREEMPT_TO_IDLE;
/* NB: For both of these, zero means disabled. */
+ if (overflows_type(engine->props.timeslice_duration_ms * 1000,
+ desc->execution_quantum))
+ drm_warn_once(drm, "GuC interface cannot support %lums
timeslice!\n",
+ engine->props.timeslice_duration_ms);
desc->execution_quantum = engine->props.timeslice_duration_ms
* 1000;
+
+ if (overflows_type(engine->props.preempt_timeout_ms * 1000,
+ desc->preemption_timeout))
+ drm_warn_once(drm, "GuC interface cannot support %lums
preemption timeout!\n",
+ engine->props.preempt_timeout_ms);
desc->preemption_timeout = engine->props.preempt_timeout_ms *
1000;
}
As previously explained, this is wrong. If the check must be present
then it should be a BUG_ON as it is indicative of an internal driver
failure. There is already a top level helper function for ensuring all
range checks are done and the value is valid. If that is broken then
that is a bug and should have been caught in pre-merge testing or code
review. It is not possible for a bad value to get beyond that helper
function. That is the whole point of the helper. We do not double bag
every other value check in the driver. Once you have passed input
validation, the values are assumed to be correct. Otherwise we would
have every other line of code be a value check! And if somehow a bad
value did make it through, simply printing a once shot warning is
pointless. You are still going to get undefined behaviour potentially
leading to a totally broken system. E.g. your very big timeout has
overflowed and become extremely small, thus no batch buffer can ever
complete because they all get reset before they have even finished the
context switch in. That is a fundamentally broken system.
John.
With that:
Reviewed-by: Tvrtko Ursulin
Regards,
Tvrtko
---
drivers/gpu/drm/i915/gt/intel_engine.h | 6 ++
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 69 +
drivers/gpu/drm/i915/gt/sysfs_engines.c | 25 +---
drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 9 +++
4 files changed, 99 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h
b/drivers/gpu/drm/i915/gt/intel_engine.h
index 1c0ab05c3c40..d7044c4e526e 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -351,4 +351,10 @@ intel_engine_get_hung_context(struct
intel_engine_cs *engine)
return engine->hung_ce;
}
+u64 intel_clamp_heartbeat_interval_ms(struct intel_engine_cs
*engine, u64 value);
+u64 intel_clamp_max_busywait_duration_ns(struct intel_engine_cs
*engine, u64 value);
+u64 intel_clamp_preempt_timeout_ms(struct intel_engine_cs *engine,
u64 value);
+u64 intel_clamp_stop_timeout_ms(struct intel_engine_cs *engine, u64
value);
+u64 intel_clamp_timeslice_duration_ms(struct intel_engine_cs
*engine, u64 value);
+
#endif /* _INTEL_RINGBUFFER_H_ */
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 7447411a5b26..22e70e4e007c 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -442,6 +442,26 @@ static int intel_engine_setup(struct intel_gt
*gt, enum intel_engine_id id,
engine->flags |= I915_ENGINE_HAS_EU_PRIORITY;
}
+ /* Cap properties according to any system limits */
+#define CLAMP_PROP(field) \
+ do { \
+ u64 clamp = intel_clamp_##field(engine, engine->props.field); \
+ if (clamp != engine->props.field) { \
+
Re: [PATCH] drm/amdkfd: Set handle to invalid for non GTT/VRAM BOs
On 2022-03-09 12:41, David Yat Sin wrote:
Set dmabuf handle to invalid for BOs that cannot be accessed using SDMA
during checkpoint/restore.
Signed-off-by: David Yat Sin
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 8 ++--
include/uapi/linux/kfd_ioctl.h | 2 ++
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index e1e2362841f8..1ffa976ad318 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1767,7 +1767,9 @@ static int criu_checkpoint_bos(struct kfd_process *p,
_bucket->dmabuf_fd);
if (ret)
goto exit;
- }
+ } else
+ bo_bucket->dmabuf_fd = KFD_INVALID_FD;
Minor nit-pick: It would be better to use {} around the else-branch for
consistency with the if-branch. Same below.
Ideally, this should have been part of the patch that bumped the KFD
version to 1.8. Alex, is there a way to squash this when you send this
in a pull-request for drm-next? Maybe if we create the commit with "git
commit --fixup" you can let auto-squash handle it.
Other than that, the patch looks good to me.
Regards,
Felix
+
if (bo_bucket->alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL)
bo_bucket->offset = KFD_MMAP_TYPE_DOORBELL |
KFD_MMAP_GPU_ID(pdd->dev->id);
@@ -2219,7 +2221,9 @@ static int criu_restore_bo(struct kfd_process *p,
_bucket->dmabuf_fd);
if (ret)
return ret;
- }
+ } else
+ bo_bucket->dmabuf_fd = KFD_INVALID_FD;
+
return 0;
}
diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
index eb9ff85f8556..42975e940758 100644
--- a/include/uapi/linux/kfd_ioctl.h
+++ b/include/uapi/linux/kfd_ioctl.h
@@ -196,6 +196,8 @@ struct kfd_ioctl_dbg_wave_control_args {
__u32 buf_size_in_bytes;/*including gpu_id and buf_size */
};
+#define KFD_INVALID_FD 0x
+
/* Matching HSA_EVENTTYPE */
#define KFD_IOC_EVENT_SIGNAL 0
#define KFD_IOC_EVENT_NODECHANGE 1
Re: [PATCH v2 3/4] drm/bridge: ti-sn65dsi86: Support DisplayPort (non-eDP) mode
Hi, On Wed, Mar 9, 2022 at 9:05 AM Kieran Bingham wrote: > > >> I think it was done for DRM purpose, to prevent signals meant for a > >> panel to be connected to a device that could capture video from a DP > >> source. > > Is this better: > > /* > * Only eDP pannels which support Alternate Scrambler Seed Reset > (ASSR) s/pannels/panels > * are supported by the SN65DSI86. For DisplayPort, disable scrambling > * mode. > */ > > I don't know if it answers your 'why' ... and I don't think adding > "We think it is for DRM protection" > > really suits the comment. Yeah, that looks pretty good. ...or even: eDP panels use an Alternate Scrambler Seed compared to displays hooked up via a full DisplayPort connector. SN65DSI86 only supports the alternate scrambler seed, not the normal one, so the only way we can support full DisplayPort displays is by fully turning off the scrambler. -Doug
Re: [PATCH v2] drm/amdgpu: Add support for drm_privacy_screen
Hi Sean,
I love your patch! Perhaps something to improve:
[auto build test WARNING on drm/drm-next]
[also build test WARNING on drm-intel/for-linux-next drm-tip/drm-tip
drm-exynos/exynos-drm-next next-20220309]
[cannot apply to tegra-drm/drm/tegra/for-next airlied/drm-next v5.17-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Sean-Paul/drm-amdgpu-Add-support-for-drm_privacy_screen/20220309-230712
base: git://anongit.freedesktop.org/drm/drm drm-next
config: arc-randconfig-r043-20220309
(https://download.01.org/0day-ci/archive/20220310/202203100441.aipzrkhu-...@intel.com/config)
compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/0day-ci/linux/commit/6fedd7f8ea75c68634df4fa3cbacb0ee5f2fc984
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Sean-Paul/drm-amdgpu-Add-support-for-drm_privacy_screen/20220309-230712
git checkout 6fedd7f8ea75c68634df4fa3cbacb0ee5f2fc984
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross
O=build_dir ARCH=arc SHELL=/bin/bash drivers/gpu/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
In file included from
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.c:44:
drivers/gpu/drm/amd/amdgpu/../display/dmub/inc/dmub_cmd.h: In function
'dmub_rb_flush_pending':
drivers/gpu/drm/amd/amdgpu/../display/dmub/inc/dmub_cmd.h:2961:26: warning:
variable 'temp' set but not used [-Wunused-but-set-variable]
2961 | uint64_t temp;
| ^~~~
In file included from include/linux/device.h:15,
from include/linux/acpi.h:15,
from include/linux/i2c.h:13,
from include/drm/drm_crtc.h:28,
from include/drm/drm_atomic.h:31,
from
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.c:26:
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.c: In
function 'amdgpu_dm_initialize_dp_connector':
>> drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgpu.h:41:22: warning: format '%d'
>> expects argument of type 'int', but argument 3 has type 'long int'
>> [-Wformat=]
41 | #define dev_fmt(fmt) "amdgpu: " fmt
| ^~
include/linux/dev_printk.h:110:30: note: in definition of macro
'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__);
\
| ^~~
include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt),
##__VA_ARGS__)
|^~~
include/drm/drm_print.h:425:9: note: in expansion of macro 'dev_err'
425 | dev_##level##type((drm)->dev, "[drm] " fmt, ##__VA_ARGS__)
| ^~~~
include/drm/drm_print.h:438:9: note: in expansion of macro '__drm_printk'
438 | __drm_printk((drm), err,, "*ERROR* " fmt, ##__VA_ARGS__)
| ^~~~
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.c:533:25:
note: in expansion of macro 'drm_err'
533 | drm_err(dev, "Error getting privacy screen,
ret=%d\n",
| ^~~
In file included from
drivers/gpu/drm/amd/amdgpu/../display/dc/inc/dc_link_ddc.h:29,
from
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.c:39:
At top level:
drivers/gpu/drm/amd/amdgpu/../display/include/ddc_service_types.h:128:22:
warning: 'SYNAPTICS_DEVICE_ID' defined but not used [-Wunused-const-variable=]
128 | static const uint8_t SYNAPTICS_DEVICE_ID[] = "SYNA";
| ^~~
drivers/gpu/drm/amd/amdgpu/../display/include/ddc_service_types.h:125:22:
warning: 'DP_SINK_DEVICE_STR_ID_2' defined but not used
[-Wunused-const-variable=]
125 | static const uint8_t DP_SINK_DEVICE_STR_ID_2[] = {7, 1, 8, 7, 5, 0};
| ^~~
drivers/gpu/drm/amd/amdgpu/../display/include/ddc_service_types.h:124:22:
warning: 'DP_SINK_DEVICE_STR_ID_1' defined but not used
[-Wunused-const-variable=]
124 | static const uint8_t DP_SINK_DEVICE_STR_ID_1[] = {7, 1, 8
Re: [PATCH v6 3/6] drm: Add driver for Solomon SSD130x OLED displays
Hello Geert,
On 3/9/22 21:04, Geert Uytterhoeven wrote:
[snip]
>> +
>> + /* Last page may be partial */
>> + if (8 * (i + 1) > ssd130x->height)
>> + m = ssd130x->height % 8;
>> + for (j = x; j < x + width; j++) {
>> + u8 data = 0;
>> +
>> + for (k = 0; k < m; k++) {
>> + u8 byte = buf[(8 * i + k) * line_length + j
>> / 8];
>
> As buf does not point to (0, 0), the above is not correct if rect.x1 !=
> 0 or rect.y1 != 0. After fixing that, writing more than one text line
> to the console works, but I still see an issue with updates where the
> rectangle size and/or position are not aligned to 8 pixels horizontally.
> Will do more investigation, and send fixes...
>
Right, I believe this is a consequence of introducing shadow buffers at
some point and not adjusting the logic in this function.
Thanks a lot for tracking down the issues and working on fixes for them!
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [PATCH v1 0/5] Add memory shrinker to VirtIO-GPU DRM driver
On 3/9/22 03:56, Rob Clark wrote: >> If we really can't track madvise state in the guest for dealing with >> host memory pressure, I think the better option is to introduce >> MADV:WILLNEED_REPLACE, ie. something to tell the host kernel that the >> buffer is needed but the previous contents are not (as long as the GPU >> VA remains the same). With this the host could allocate new pages if >> needed, and the guest would not need to wait for a reply from host. > If variant with the memory ballooning will work, then it will be > possible to track the state within guest-only. Let's consider the > simplest variant for now. > > I'll try to implement the balloon driver support in the v2 and will get > back to you. > I looked at the generic balloon driver and looks like this not what we want because: 1. Memory ballooning is primarily about handling memory overcommit situations. I.e. when there are multiple VMs consuming more memory than available in the system. Ballooning allows host to ask guest to give unused pages back to host and host could give pages to other VMs. 2. Memory ballooning operates with guest memory pages only. I.e. each ballooned page is reported to/from host in a form of page's DMA address. 3. There is no direct connection between host's OOM events and the balloon manager. I guess host could watch system's memory pressure and inflate VMs' balloons on low memory, releasing the guest's memory to the system, but apparently this use-case not supported by anyone today, at least I don't see Qemu supporting it. So the virtio-balloon driver isn't very useful for us as-is. One possible solution could be to create something like a new virtio-shrinker device or add shrinker functionality to the virtio-gpu device, allowing host to ask guests to drop shared caches. Host then should become a PSI handler. I think this should be doable in a case of crosvm. In a case of GNU world, it could take a lot of effort to get everything to upstreamable state, at first there is a need to demonstrate real problem being solved by this solution. The other minor issue is that only integrated GPUs may use system's memory and even then they could use a dedicated memory carveout, i.e. releasing VRAM BOs may not help with host's OOM. In case of virgl context we have no clue about where buffers are physically located. On the other hand, in the worst case dropping host caches just won't help with OOM. It's now unclear how we should proceed with the host-side shrinker support. Thoughts? We may start easy and instead of thinking about host-side shrinker, we could make VirtIO-GPU driver to expire cached BOs after a certain timeout. Mesa already uses timeout-based BO caching, but it doesn't have an alarm timer and simply checks expiration when BO is allocated. Should be too much trouble to handle timers within Mesa since it's executed in application context, easier to do it in kernel, like VC4 driver does it for example. This is not good as a proper memory shrinker, but could be good enough in practice.
Re: [PATCH v6 3/6] drm: Add driver for Solomon SSD130x OLED displays
Hi Javier,
On Mon, Feb 14, 2022 at 2:37 PM Javier Martinez Canillas
wrote:
> This adds a DRM driver for SSD1305, SSD1306, SSD1307 and SSD1309 Solomon
> OLED display controllers.
>
> It's only the core part of the driver and a bus specific driver is needed
> for each transport interface supported by the display controllers.
>
> Signed-off-by: Javier Martinez Canillas
> --- /dev/null
> +++ b/drivers/gpu/drm/solomon/ssd130x.c
> +static int ssd130x_update_rect(struct ssd130x_device *ssd130x, u8 *buf,
> + struct drm_rect *rect)
> +{
> + unsigned int x = rect->x1;
> + unsigned int y = rect->y1;
> + unsigned int width = drm_rect_width(rect);
> + unsigned int height = drm_rect_height(rect);
> + unsigned int line_length = DIV_ROUND_UP(width, 8);
> + unsigned int pages = DIV_ROUND_UP(y % 8 + height, 8);
> + u32 array_idx = 0;
> + int ret, i, j, k;
> + u8 *data_array = NULL;
> +
> + data_array = kcalloc(width, pages, GFP_KERNEL);
> + if (!data_array)
> + return -ENOMEM;
> +
> + /*
> +* The screen is divided in pages, each having a height of 8
> +* pixels, and the width of the screen. When sending a byte of
> +* data to the controller, it gives the 8 bits for the current
> +* column. I.e, the first byte are the 8 bits of the first
> +* column, then the 8 bits for the second column, etc.
> +*
> +*
> +* Representation of the screen, assuming it is 5 bits
> +* wide. Each letter-number combination is a bit that controls
> +* one pixel.
> +*
> +* A0 A1 A2 A3 A4
> +* B0 B1 B2 B3 B4
> +* C0 C1 C2 C3 C4
> +* D0 D1 D2 D3 D4
> +* E0 E1 E2 E3 E4
> +* F0 F1 F2 F3 F4
> +* G0 G1 G2 G3 G4
> +* H0 H1 H2 H3 H4
> +*
> +* If you want to update this screen, you need to send 5 bytes:
> +* (1) A0 B0 C0 D0 E0 F0 G0 H0
> +* (2) A1 B1 C1 D1 E1 F1 G1 H1
> +* (3) A2 B2 C2 D2 E2 F2 G2 H2
> +* (4) A3 B3 C3 D3 E3 F3 G3 H3
> +* (5) A4 B4 C4 D4 E4 F4 G4 H4
> +*/
> +
> + ret = ssd130x_set_col_range(ssd130x, ssd130x->col_offset + x, width);
> + if (ret < 0)
> + goto out_free;
> +
> + ret = ssd130x_set_page_range(ssd130x, ssd130x->page_offset + y / 8,
> pages);
> + if (ret < 0)
> + goto out_free;
> +
> + for (i = y / 8; i < y / 8 + pages; i++) {
> + int m = 8;
> +
> + /* Last page may be partial */
> + if (8 * (i + 1) > ssd130x->height)
> + m = ssd130x->height % 8;
> + for (j = x; j < x + width; j++) {
> + u8 data = 0;
> +
> + for (k = 0; k < m; k++) {
> + u8 byte = buf[(8 * i + k) * line_length + j /
> 8];
As buf does not point to (0, 0), the above is not correct if rect.x1 !=
0 or rect.y1 != 0. After fixing that, writing more than one text line
to the console works, but I still see an issue with updates where the
rectangle size and/or position are not aligned to 8 pixels horizontally.
Will do more investigation, and send fixes...
> + u8 bit = (byte >> (j % 8)) & 1;
> +
> + data |= bit << k;
> + }
> + data_array[array_idx++] = data;
> + }
> + }
> +
> + ret = ssd130x_write_data(ssd130x, data_array, width * pages);
> +
> +out_free:
> + kfree(data_array);
> + return ret;
> +}
> +
> +static void ssd130x_clear_screen(struct ssd130x_device *ssd130x)
> +{
> + u8 *buf = NULL;
> + struct drm_rect fullscreen = {
> + .x1 = 0,
> + .x2 = ssd130x->width,
> + .y1 = 0,
> + .y2 = ssd130x->height,
> + };
> +
> + buf = kcalloc(ssd130x->width, ssd130x->height, GFP_KERNEL);
This buffer is larger than needed. Will send a fix.
> + if (!buf)
> + return;
> +
> + ssd130x_update_rect(ssd130x, buf, );
> +
> + kfree(buf);
> +}
> +
> +static int ssd130x_fb_blit_rect(struct drm_framebuffer *fb, const struct
> dma_buf_map *map,
> + struct drm_rect *rect)
> +{
> + struct ssd130x_device *ssd130x = drm_to_ssd130x(fb->dev);
> + void *vmap = map->vaddr; /* TODO: Use mapping abstraction properly */
> + int ret = 0;
> + u8 *buf = NULL;
> +
> + buf = kcalloc(fb->width, fb->height, GFP_KERNEL);
This buffer is much larger than needed. Will send a fix.
> + if (!buf)
> + return -ENOMEM;
> +
> + drm_fb_xrgb_to_mono_reversed(buf, 0, vmap, fb, rect);
> +
> + ssd130x_update_rect(ssd130x, buf, rect);
> +
> + kfree(buf);
> +
> + return ret;
> +}
Gr{oetje,eeting}s,
Re: [PATCH v1 0/5] Add memory shrinker to VirtIO-GPU DRM driver
Hi Am 09.03.22 um 12:55 schrieb Dmitry Osipenko: Hello, On 3/9/22 11:59, Thomas Zimmermann wrote: Hi Am 08.03.22 um 14:17 schrieb Dmitry Osipenko: Hello, This patchset introduces memory shrinker for the VirtIO-GPU DRM driver. During OOM, the shrinker will release BOs that are marked as "not needed" by userspace using the new madvise IOCTL. The userspace in this case is the Mesa VirGL driver, it will mark the cached BOs as "not needed", allowing kernel driver to release memory of the cached shmem BOs on lowmem situations, preventing OOM kills. Virtio-gpu is build on top of GEM shmem helpers. I have a prototype patchset that adds a shrinker to these helpers. If you want to go further, you could implement something like that instead. Panfrost and lima also have their own shrinker and could certainly be converted to the gem-shmem shrinker. I had a thought that it could be possible to unify shrinkers into a common DRM framework. Could you please give me a link to yours prototype patchset? I uploaded the patches to https://gitlab.freedesktop.org/tzimmermann/linux/-/commits/gem-shmem-cached-mappings it's incomplete and un-debugged, but it shows what needs to be done. It has the infrastructure, but lacks the changes to the GEM shmem code. The reason for this work is to keep GEM shmem pages mapped and allocated even while the BO is neither mapped nor pinned. As it is now, GEM SHMEM creates and releases pages on each pin and unpin, and maps and unmaps memory ranges on each vmap and vunmap. It's all wasteful. Only the first pin and vmap calls should establish pages and mappings and only the purge and free functions should release them. The patchset adds new helpers for BO purging to struct drm_gem_object_funcs. With this, I think it might be possible to have one global DRM shrinker and let it handle all BOs; independent of each BO's memory manager. Best regards Thomas -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH v5 5/5] drm/msm/disp/dpu1: set mdp clk to the maximum frequency in opp table during probe
Hi, On Tue, Mar 8, 2022 at 8:55 AM Vinod Polimera wrote: > > use max clock during probe/bind sequence from the opp table. > The clock will be scaled down when framework sends an update. > > Signed-off-by: Vinod Polimera > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 3 +++ > 1 file changed, 3 insertions(+) In addition to Dmitry's requests, can you also make this patch #1 in the series since the DTS stuff really ought to come _after_ this one. ...and actually, thinking about it further: 1. If we land this fix, we actually don't _need_ the dts patches, right? Sure, the clock rate will get assigned before probe but then we'll change it right away in probe, right? 2. If we land the dts patches _before_ the driver patch then it will be a regression, right? 3. The dts patches and driver patch will probably land through separate trees. The driver patch will go through the MSM DRM tree and the device tree patches through the Qualcomm armsoc tree, right? Assuming that the above is right, we should: 1. Put the driver patch first. 2. Remove the "Fixes" tag on the dts patches. I guess in theory we could have a FIxes tag on this patch? 3. Note in the dts patches commit messages that they depend on the driver patch. 4. Delay the dts patches until the driver change has made it to mainline. Does that sound reasonable?
Re: DSI Bridge switching
or a Hi All, On Thu, Oct 14, 2021 at 6:45 PM Jagan Teki wrote: > > Hi Laurent, > > On Fri, Oct 8, 2021 at 7:07 PM Laurent Pinchart > wrote: > > > > Hello, > > > > On Fri, Oct 08, 2021 at 03:27:43PM +0200, Andrzej Hajda wrote: > > > Hi, > > > > > > Removed my invalid email (I will update files next week). > > > > > > On 08.10.2021 13:14, Jagan Teki wrote: > > > > Hi, > > > > > > > > I think this seems to be a known use case for industrial these days > > > > with i.mx8m. > > > > > > > > The host DSI would configure with two bridges one for DSI to LVDS > > > > (SN65DSI83) and another for DSI to HDMI Out (ADV7535). Technically we > > > > can use only one bridge at a time as host DSI support single out port. > > > > So we can have two separate device tree files for LVDS and HDMI and > > > > load them static. > > > > > > > > But, one of the use cases is to support both of them in single dts, and > > > > - Turn On LVDS (default) > > > > - Turn Off LVDS then Turn On HDMI when cable plug-in > > > > > > Are you sure it will work from hardware PoV? Do you have some demuxer? > > > isolation of pins? > > > > It may be in the category of "you shouldn't do this, but it actually > > works". I've seen the same being done with two CSI-2 camera sensors > > connected to the same receiver, with one of them being held in reset at > > all times. > > Yes. Here the design has 2 MIPI D-PHY switches. Each switch take 2 > input data lanes and 1 clock lane from SoC and produces 4 data lanes > and 2 clock lanes and from switch output 2 lanes and 1 clock are > inputting to HDMI bridge and other 2 lanes and 1 clock is inputting to > LVDS. So 1st pair of 1st switch and 1st pair of 2nd switch goes to > HDMI and 2nd pair of 1st switch and 2nd pair of 2nd switch does to > LVDS. > > However, routing of these lanes are controlled by SEL, OE GPIO pins. > So at a time we can access only single bridge. > > > > > > > The HDMI event can be detected via some HDMI-INT GPIO on-board design. > > > > > > > > The possible solution, I'm thinking of adding LVDS on port 1, HDMI on > > > > port 2 in the DSI host node, and trying to attach the respective > > > > bridge based on HDMI-INT like repeating the bridge attachment cycle > > > > based on the HDMI-INT. > > > > > > I think more appropriate would be to share the same port, but provide > > > two endpoints inside this port - we have two hardware sharing the same > > > physical port. > > > > That sounds like the correct DT description to me. > > > > > > Can it be possible to do bridge attachment at runtime? something like > > > > a bridge hotplug event? or any other possible solutions? > > > > > > > > Any suggestions? > > > > > > Practically it is possible, see exynos_dsi + panels, or exynos_dsi + > > > some toshiba bridge - panel and bridge are dynamically 'plugged' and > > > 'unplugged' from exynos_drm, but they do not use bridge chain for this > > > and some other reasons. (un|re|)plugging should be performed of course > > > when pipeline is off (connector disconnected). I am not sure about > > > bridges added to bridge chain - you need to inspect all opses to ensure > > > it can be done safely. > > > > > > And the main issue: Daniel does not like it :) > > > > Neither do I :-) Could it be handled with two DRM connectors that are > > mutually exclusive ? > > How about adding lvds-connector, hdmi-connector on the pipeline and > select them based on the switch SEL GPIO? does it make sense to do > this implementation via display-connector.c I have somehow managed to make runtime switching possible between LVDS and HDMI with the help of DRM bridges. | => ADV7535=> HDMI-A Connector DSI Host => display-switch => | |=> SN65DSI83 => Panel-Simple display-switch here is a bridge driver that can switch or attach the downstream bridge flow based on HDMI HPD here. One potential problem is that when we switch from LVDS to HDMI Out the HDMI Out is displayed with the resolution that LVDS has and it is unable to display higher HDMI resolutions even though it supports it. Does anyone aware of changing the resolution at runtime, please let me know? Technically, the display-switch hardware does available in various forms 1. MIPI Switch PI3WVR626 2. Conventional Mux Switch 3. Converter bridge DSI to LVDS/HDMI (from Lontium). Overall I believe this can be a potential possible feature and good to support on Mainline as the hardware is intended to design for it. Any thoughts on this please let me know? Thanks, Jagan.
Re: [PATCH v2 1/2] drm: Add GPU reset sysfs event
On Tue, Mar 8, 2022 at 11:40 PM Shashank Sharma wrote: > > From: Shashank Sharma > > This patch adds a new sysfs event, which will indicate > the userland about a GPU reset, and can also provide > some information like: > - process ID of the process involved with the GPU reset > - process name of the involved process > - the GPU status info (using flags) > > This patch also introduces the first flag of the flags > bitmap, which can be appended as and when required. Why invent something new, rather than using the already existing devcoredump? I don't think we need (or should encourage/allow) something drm specific when there is already an existing solution used by both drm and non-drm drivers. Userspace should not have to learn to support yet another mechanism to do the same thing. BR, -R
Re: [PATCH v2] drm/amdgpu: Add support for drm_privacy_screen
On Wed, Mar 9, 2022 at 12:54 PM Rajat Jain wrote:
>
> On Wed, Mar 9, 2022 at 7:06 AM Sean Paul wrote:
> >
> > From: Sean Paul
> >
> > This patch adds the necessary hooks to make amdgpu aware of privacy
> > screens. On devices with privacy screen drivers (such as thinkpad-acpi),
> > the amdgpu driver will defer probe until it's ready and then sync the sw
> > and hw state on each commit the connector is involved and enabled.
> >
> > Changes in v2:
> > -Tweaked the drm_privacy_screen_get() error check to avoid logging
> > errors when privacy screen is absent (Hans)
> >
> > Signed-off-by: Sean Paul
> > Link: https://patchwork.freedesktop.org/patch/477640/ #v1
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 +
> > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c| 3 +++
> > .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 16 +++-
> > 3 files changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > index 2ab675123ae3..e2cfae56c020 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > @@ -26,6 +26,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include "amdgpu_drv.h"
> > @@ -1988,6 +1989,7 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
> > {
> > struct drm_device *ddev;
> > struct amdgpu_device *adev;
> > + struct drm_privacy_screen *privacy_screen;
> > unsigned long flags = ent->driver_data;
> > int ret, retry = 0, i;
> > bool supports_atomic = false;
> > @@ -2063,6 +2065,13 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
> > size = pci_resource_len(pdev, 0);
> > is_fw_fb = amdgpu_is_fw_framebuffer(base, size);
> >
> > + /* If the LCD panel has a privacy screen, defer probe until its
> > ready */
> > + privacy_screen = drm_privacy_screen_get(>dev, NULL);
> > + if (IS_ERR(privacy_screen) && PTR_ERR(privacy_screen) ==
> > -EPROBE_DEFER)
> > + return -EPROBE_DEFER;
> > +
> > + drm_privacy_screen_put(privacy_screen);
> > +
> > /* Get rid of things like offb */
> > ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev,
> > _kms_driver);
> > if (ret)
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index e1d3db3fe8de..9e2bb6523add 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -9781,6 +9781,9 @@ static void amdgpu_dm_atomic_commit_tail(struct
> > drm_atomic_state *state)
> > if (acrtc) {
> > new_crtc_state =
> > drm_atomic_get_new_crtc_state(state, >base);
> > old_crtc_state =
> > drm_atomic_get_old_crtc_state(state, >base);
> > +
> > + /* Sync the privacy screen state between hw and sw
> > */
> > + drm_connector_update_privacy_screen(new_con_state);
> > }
> >
> > /* Skip any modesets/resets */
> > 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 740435ae3997..594a8002975a 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
> > @@ -27,6 +27,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include "dm_services.h"
> > #include "amdgpu.h"
> > #include "amdgpu_dm.h"
> > @@ -506,6 +507,7 @@ void amdgpu_dm_initialize_dp_connector(struct
> > amdgpu_display_manager *dm,
> >struct amdgpu_dm_connector
> > *aconnector,
> >int link_index)
> > {
> > + struct drm_device *dev = dm->ddev;
> > struct dc_link_settings max_link_enc_cap = {0};
> >
> > aconnector->dm_dp_aux.aux.name =
> > @@ -519,8 +521,20 @@ void amdgpu_dm_initialize_dp_connector(struct
> > amdgpu_display_manager *dm,
> > drm_dp_cec_register_connector(>dm_dp_aux.aux,
> > >base);
> >
> > - if (aconnector->base.connector_type == DRM_MODE_CONNECTOR_eDP)
> > + if (aconnector->base.connector_type == DRM_MODE_CONNECTOR_eDP) {
> > + struct drm_privacy_screen *privacy_screen)
> > +
> > + /* Reference given up in drm_connector_cleanup() */
> > + privacy_screen = drm_privacy_screen_get(dev->dev, NULL);
>
> Can we try to be more specific when looking up for privacy screen, e.g.:
>
> privacy_screen = drm_privacy_screen_get(dev->dev, "eDP-1");
> (and then also making the corresponding change in arch_init_data[] in
> drm_privacy_screen_x86.c"
>
> My comment applies to
[PATCH] drm/amdkfd: Set handle to invalid for non GTT/VRAM BOs
Set dmabuf handle to invalid for BOs that cannot be accessed using SDMA
during checkpoint/restore.
Signed-off-by: David Yat Sin
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 8 ++--
include/uapi/linux/kfd_ioctl.h | 2 ++
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index e1e2362841f8..1ffa976ad318 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1767,7 +1767,9 @@ static int criu_checkpoint_bos(struct kfd_process *p,
_bucket->dmabuf_fd);
if (ret)
goto exit;
- }
+ } else
+ bo_bucket->dmabuf_fd = KFD_INVALID_FD;
+
if (bo_bucket->alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL)
bo_bucket->offset = KFD_MMAP_TYPE_DOORBELL |
KFD_MMAP_GPU_ID(pdd->dev->id);
@@ -2219,7 +2221,9 @@ static int criu_restore_bo(struct kfd_process *p,
_bucket->dmabuf_fd);
if (ret)
return ret;
- }
+ } else
+ bo_bucket->dmabuf_fd = KFD_INVALID_FD;
+
return 0;
}
diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
index eb9ff85f8556..42975e940758 100644
--- a/include/uapi/linux/kfd_ioctl.h
+++ b/include/uapi/linux/kfd_ioctl.h
@@ -196,6 +196,8 @@ struct kfd_ioctl_dbg_wave_control_args {
__u32 buf_size_in_bytes;/*including gpu_id and buf_size */
};
+#define KFD_INVALID_FD 0x
+
/* Matching HSA_EVENTTYPE */
#define KFD_IOC_EVENT_SIGNAL 0
#define KFD_IOC_EVENT_NODECHANGE 1
--
2.35.1
Re: [PATCH v2 3/4] drm/bridge: ti-sn65dsi86: Support DisplayPort (non-eDP) mode
Quoting Doug Anderson (2022-03-07 23:22:00) > Hi, > > On Mon, Mar 7, 2022 at 10:00 AM Kieran Bingham > wrote: > > > > From: Laurent Pinchart > > > > Despite the SN65DSI86 being an eDP bridge, on some systems its output is > > routed to a DisplayPort connector. Enable DisplayPort mode when the next > > component in the display pipeline is detected as a DisplayPort > > connector, and disable eDP features in that case. > > > > Signed-off-by: Laurent Pinchart > > Reworked to set bridge type based on the next bridge/connector. > > Signed-off-by: Kieran Bingham > > -- > > Changes since v1/RFC: > > - Rebased on top of "drm/bridge: ti-sn65dsi86: switch to > >devm_drm_of_get_bridge" > > - eDP/DP mode determined from the next bridge connector type. > > > > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 16 +++- > > 1 file changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > index 29f5f7123ed9..461f963faa0b 100644 > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > @@ -60,6 +60,7 @@ > > #define SN_LN_ASSIGN_REG 0x59 > > #define LN_ASSIGN_WIDTH 2 > > #define SN_ENH_FRAME_REG 0x5A > > +#define ASSR_CONTROL BIT(0) > > #define VSTREAM_ENABLEBIT(3) > > #define LN_POLRS_OFFSET 4 > > #define LN_POLRS_MASK 0xf0 > > @@ -91,6 +92,8 @@ > > #define SN_DATARATE_CONFIG_REG 0x94 > > #define DP_DATARATE_MASK GENMASK(7, 5) > > #define DP_DATARATE(x)((x) << 5) > > +#define SN_TRAINING_SETTING_REG0x95 > > +#define SCRAMBLE_DISABLE BIT(4) > > #define SN_ML_TX_MODE_REG 0x96 > > #define ML_TX_MAIN_LINK_OFF 0 > > #define ML_TX_NORMAL_MODE BIT(0) > > @@ -1005,6 +1008,11 @@ static int ti_sn_link_training(struct ti_sn65dsi86 > > *pdata, int dp_rate_idx, > > regmap_update_bits(pdata->regmap, SN_DATARATE_CONFIG_REG, > >DP_DATARATE_MASK, DP_DATARATE(dp_rate_idx)); > > > > + /* For DisplayPort, use the standard DP scrambler seed. */ > > + if (pdata->bridge.type == DRM_MODE_CONNECTOR_DisplayPort) > > + regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG, > > + ASSR_CONTROL, 0); > > I thought it was agreed that this hunk wasn't doing anything and > should be removed? I did some research previously [1] and the manual > said that this bit is "read only" unless "TEST2" is pulled high. In > the previous discussion Laurent said that it wasn't. I also pointed > out that this conflicts with the bit of code in ti_sn_bridge_enable() > which tells the sink to enable the alternate scrambler. Sorry - I had misremembered indeed, and looking back I confirmed that this hunk was not required. I had incorrectly remembered that the second part was required (below) and assumed that had meant both were. Of course if we're disabling the scrambling mode, then it likely doesn't matter what the seed is!. Looking at the datasheet though, register 0x5a, clearly says the default is 01 (ASSR) which we're not using. So the datasheet implies we want the DP scrambler seed (if it were enabled?) Reading the 0x5a register here shows we have 0x05. Indeed, re-reading after attempting to write '0' to it still shows 0x05. So it's read-only and not relevant regardless. I've removed this hunk now. > > /* enable DP PLL */ > > regmap_write(pdata->regmap, SN_PLL_ENABLE_REG, 1); > > > > @@ -1016,6 +1024,11 @@ static int ti_sn_link_training(struct ti_sn65dsi86 > > *pdata, int dp_rate_idx, > > goto exit; > > } > > > > + /* For DisplayPort, disable scrambling mode. */ > > + if (pdata->bridge.type == DRM_MODE_CONNECTOR_DisplayPort) > > + regmap_update_bits(pdata->regmap, SN_TRAINING_SETTING_REG, > > + SCRAMBLE_DISABLE, SCRAMBLE_DISABLE); > > In my previous review I asked for some comments to include the "why" > we disabling scrambling mode for DP. Can you please add that? > > I also presume that for DP it's probably a good idea to avoid the code > in ti_sn_bridge_enable() that tells the sink to use the alternate > scrambler. looking at it yes. I've added a check to avoid that in my DP connector case, and there doesn't seem to be any effect on the output. But I'll add it to the patch for the next version. >> I think it was done for DRM purpose, to prevent signals meant for a >> panel to be connected to a device that could capture video from a DP >> source. Is this better: /* * Only eDP pannels which support Alternate Scrambler Seed Reset (ASSR) *
Re: [PATCH v1 6/9] drm/bridge: ti-sn65dsi86: Add NO_CONNECTOR support
Hi Sam,
Quoting Sam Ravnborg (2022-02-06 15:44:02)
> From: Rob Clark
>
> Slightly awkward to fish out the display_info when we aren't creating
> own connector. But I don't see an obvious better way.
>
> v3:
> - Rebased and dropped the ti_sn_bridge_get_bpp() patch
>as this was solved in a different way (Sam)
>
> v2:
> - Remove error return with NO_CONNECTOR flag (Rob)
>
> Signed-off-by: Rob Clark
> Signed-off-by: Sam Ravnborg
> Cc: Rob Clark
> Cc: Douglas Anderson
> Cc: Andrzej Hajda
> Cc: Neil Armstrong
> Cc: Robert Foss
> Cc: Laurent Pinchart
> Cc: Jonas Karlman
> Cc: Jernej Skrabec
> ---
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 20 +---
> 1 file changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index dc6ec40bc1ef..a9041dfd2ae5 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -746,11 +746,6 @@ static int ti_sn_bridge_attach(struct drm_bridge *bridge,
> struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge);
> int ret;
>
> - if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
> - DRM_ERROR("Fix bridge driver to make connector optional!");
> - return -EINVAL;
> - }
> -
> pdata->aux.drm_dev = bridge->dev;
> ret = drm_dp_aux_register(>aux);
> if (ret < 0) {
> @@ -758,12 +753,14 @@ static int ti_sn_bridge_attach(struct drm_bridge
> *bridge,
> return ret;
> }
>
> - ret = ti_sn_bridge_connector_init(pdata);
> - if (ret < 0)
> - goto err_conn_init;
> + if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
> + ret = ti_sn_bridge_connector_init(pdata);
> + if (ret < 0)
> + goto err_conn_init;
>
> - /* We never want the next bridge to *also* create a connector: */
> - flags |= DRM_BRIDGE_ATTACH_NO_CONNECTOR;
> + /* We never want the next bridge to *also* create a
> connector: */
> + flags |= DRM_BRIDGE_ATTACH_NO_CONNECTOR;
> + }
>
> /* Attach the next bridge */
> ret = drm_bridge_attach(bridge->encoder, pdata->next_bridge,
> @@ -774,7 +771,8 @@ static int ti_sn_bridge_attach(struct drm_bridge *bridge,
> return 0;
>
> err_dsi_host:
> - drm_connector_cleanup(>connector);
> + if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR))
I think this is unreachable / always false due to the
flags |= DRM_BRIDGE_ATTACH_NO_CONNECTOR.
I've solved this locally by doing:
- /* Attach the next bridge */
+ /*
+* Attach the next bridge We never want the next bridge to *also* create
+* a connector:
+*/
ret = drm_bridge_attach(bridge->encoder, pdata->next_bridge,
- >bridge, flags);
+ >bridge,
+ flags | DRM_BRIDGE_ATTACH_NO_CONNECTOR);
if (ret < 0)
goto err_initted_aux;
+ if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
+ return 0;
+
pdata->connector = drm_bridge_connector_init(pdata->bridge.dev,
pdata->bridge.encoder);
if (IS_ERR(pdata->connector)) {
ret = PTR_ERR(pdata->connector);
goto err_initted_aux;
}
drm_connector_attach_encoder(pdata->connector, pdata->bridge.encoder);
return 0;
Which also fixes the support for
flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR
--
Regards
Kieran
> + drm_connector_cleanup(>connector);
> err_conn_init:
> drm_dp_aux_unregister(>aux);
> return ret;
> --
> 2.32.0
>
Re: [PATCH v9 00/11] vgaarb: Rework default VGA device selection
On Fri, Feb 25, 2022 at 04:15:23PM -0600, Bjorn Helgaas wrote: > On Thu, Feb 24, 2022 at 04:47:42PM -0600, Bjorn Helgaas wrote: > > From: Bjorn Helgaas > > > > Current default VGA device selection fails in some cases because part of it > > is done in the vga_arb_device_init() subsys_initcall, and some arches > > enumerate PCI devices in pcibios_init(), which runs *after* that. > > > > The big change from the v8 posting is that this moves vgaarb.c from > > drivers/gpu/vga to drivers/pci because it really has nothing to do with > > GPUs or DRM. > > I provisionally applied this to pci/vga and put it into -next just > to get a little runtime on it. > > But I'd prefer not to unilaterally yank this out of drivers/gpu > without a consensus from the GPU folks that this is the right thing to > do. > > Any thoughts? If it seems OK to you, I think patch 1/11 (the move > itself) is all you would need to look at, although of course I would > still be grateful for any review and feedback on the rest. > > After it's in drivers/pci, all the blame for any issues would come my > way. Ping? This has been in -next since the Feb 28 tree, and I plan to ask Linus to include it during the v5.18 merge window (which will probably open Mar 13) unless somebody objects. Bjorn
[PATCH AUTOSEL 5.4 14/19] drm/vrr: Set VRR capable prop only if it is attached to connector
From: Manasi Navare
[ Upstream commit 62929726ef0ec72cbbe9440c5d125d4278b99894 ]
VRR capable property is not attached by default to the connector
It is attached only if VRR is supported.
So if the driver tries to call drm core set prop function without
it being attached that causes NULL dereference.
Cc: Jani Nikula
Cc: Ville Syrjälä
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Manasi Navare
Reviewed-by: Ville Syrjälä
Link:
https://patchwork.freedesktop.org/patch/msgid/20220225013055.9282-1-manasi.d.nav...@intel.com
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/drm_connector.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 2337b3827e6a..11a81e8ba963 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1984,6 +1984,9 @@ EXPORT_SYMBOL(drm_connector_attach_max_bpc_property);
void drm_connector_set_vrr_capable_property(
struct drm_connector *connector, bool capable)
{
+ if (!connector->vrr_capable_property)
+ return;
+
drm_object_property_set_value(>base,
connector->vrr_capable_property,
capable);
--
2.34.1
[PATCH AUTOSEL 5.10 15/20] drm/vrr: Set VRR capable prop only if it is attached to connector
From: Manasi Navare
[ Upstream commit 62929726ef0ec72cbbe9440c5d125d4278b99894 ]
VRR capable property is not attached by default to the connector
It is attached only if VRR is supported.
So if the driver tries to call drm core set prop function without
it being attached that causes NULL dereference.
Cc: Jani Nikula
Cc: Ville Syrjälä
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Manasi Navare
Reviewed-by: Ville Syrjälä
Link:
https://patchwork.freedesktop.org/patch/msgid/20220225013055.9282-1-manasi.d.nav...@intel.com
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/drm_connector.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 717c4e7271b0..5163433ac561 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2155,6 +2155,9 @@ EXPORT_SYMBOL(drm_connector_attach_max_bpc_property);
void drm_connector_set_vrr_capable_property(
struct drm_connector *connector, bool capable)
{
+ if (!connector->vrr_capable_property)
+ return;
+
drm_object_property_set_value(>base,
connector->vrr_capable_property,
capable);
--
2.34.1
[PATCH AUTOSEL 5.15 19/24] drm/vrr: Set VRR capable prop only if it is attached to connector
From: Manasi Navare
[ Upstream commit 62929726ef0ec72cbbe9440c5d125d4278b99894 ]
VRR capable property is not attached by default to the connector
It is attached only if VRR is supported.
So if the driver tries to call drm core set prop function without
it being attached that causes NULL dereference.
Cc: Jani Nikula
Cc: Ville Syrjälä
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Manasi Navare
Reviewed-by: Ville Syrjälä
Link:
https://patchwork.freedesktop.org/patch/msgid/20220225013055.9282-1-manasi.d.nav...@intel.com
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/drm_connector.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 2ba257b1ae20..e9b7926d9b66 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2233,6 +2233,9 @@ EXPORT_SYMBOL(drm_connector_atomic_hdr_metadata_equal);
void drm_connector_set_vrr_capable_property(
struct drm_connector *connector, bool capable)
{
+ if (!connector->vrr_capable_property)
+ return;
+
drm_object_property_set_value(>base,
connector->vrr_capable_property,
capable);
--
2.34.1
[PATCH AUTOSEL 5.16 22/27] drm/vrr: Set VRR capable prop only if it is attached to connector
From: Manasi Navare
[ Upstream commit 62929726ef0ec72cbbe9440c5d125d4278b99894 ]
VRR capable property is not attached by default to the connector
It is attached only if VRR is supported.
So if the driver tries to call drm core set prop function without
it being attached that causes NULL dereference.
Cc: Jani Nikula
Cc: Ville Syrjälä
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Manasi Navare
Reviewed-by: Ville Syrjälä
Link:
https://patchwork.freedesktop.org/patch/msgid/20220225013055.9282-1-manasi.d.nav...@intel.com
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/drm_connector.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 52e20c68813b..6ae26e7d3dec 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2275,6 +2275,9 @@ EXPORT_SYMBOL(drm_connector_atomic_hdr_metadata_equal);
void drm_connector_set_vrr_capable_property(
struct drm_connector *connector, bool capable)
{
+ if (!connector->vrr_capable_property)
+ return;
+
drm_object_property_set_value(>base,
connector->vrr_capable_property,
capable);
--
2.34.1
Re: [PATCH v4 2/2] drm/bridge: analogix_dp: Enable autosuspend
Hi, On Thu, Mar 3, 2022 at 3:02 PM Doug Anderson wrote: > > Hi, > > On Tue, Mar 1, 2022 at 6:11 PM Brian Norris wrote: > > > > DP AUX transactions can consist of many short operations. There's no > > need to power things up/down in short intervals. > > > > I pick an arbitrary 100ms; for the systems I'm testing (Rockchip > > RK3399), runtime-PM transitions only take a few microseconds. > > > > Signed-off-by: Brian Norris > > --- > > > > Changes in v4: > > - call pm_runtime_mark_last_busy() and > >pm_runtime_dont_use_autosuspend() > > - drop excess pm references around drm_get_edid(), now that we grab and > >hold in the dp-aux helper > > > > Changes in v3: > > - New in v3 > > > > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 9 ++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > This looks great to me now, thanks. > > Reviewed-by: Douglas Anderson > > Though I'm not a massive expert on the Analogix DP driver, I'm pretty > confident about the DP AUX stuff that Brian is touching. I just > checked and I see that this driver isn't changing lots and the last > change landed in drm-misc, which means that I can commit this. Thus, > unless someone else shouts, I'll plan to wait until next week and > commit these two patches to drm-misc. > > The first of the two patches is a "Fix" but since it's been broken > since 2016 I'll assume that nobody is chomping at the bit for these to > get into stable and that it would be easier to land both in > "drm-misc-next". Please yell if someone disagrees. Pushed both to drm-misc-next: f28dd5075675 drm/bridge: analogix_dp: Enable autosuspend 8fb6c44fe846 drm/bridge: analogix_dp: Grab runtime PM reference for DP-AUX -Doug
Re: [RFC v2 1/2] drm/doc/rfc: VM_BIND feature design document
On Mon, Mar 7, 2022 at 3:30 PM Niranjana Vishwanathapura wrote: > > VM_BIND design document with description of intended use cases. > > Signed-off-by: Niranjana Vishwanathapura > --- > Documentation/gpu/rfc/i915_vm_bind.rst | 210 + > Documentation/gpu/rfc/index.rst| 4 + > 2 files changed, 214 insertions(+) > create mode 100644 Documentation/gpu/rfc/i915_vm_bind.rst > > diff --git a/Documentation/gpu/rfc/i915_vm_bind.rst > b/Documentation/gpu/rfc/i915_vm_bind.rst > new file mode 100644 > index ..cdc6bb25b942 > --- /dev/null > +++ b/Documentation/gpu/rfc/i915_vm_bind.rst > @@ -0,0 +1,210 @@ > +== > +I915 VM_BIND feature design and use cases > +== > + > +VM_BIND feature > + > +DRM_I915_GEM_VM_BIND/UNBIND ioctls allows UMD to bind/unbind GEM buffer > +objects (BOs) or sections of a BOs at specified GPU virtual addresses on > +a specified address space (VM). > + > +These mappings (also referred to as persistent mappings) will be persistent > +across multiple GPU submissions (execbuff) issued by the UMD, without user > +having to provide a list of all required mappings during each submission > +(as required by older execbuff mode). > + > +VM_BIND ioctl deferes binding the mappings until next execbuff submission > +where it will be required, or immediately if I915_GEM_VM_BIND_IMMEDIATE > +flag is set (useful if mapping is required for an active context). > + > +VM_BIND feature is advertised to user via I915_PARAM_HAS_VM_BIND. > +User has to opt-in for VM_BIND mode of binding for an address space (VM) > +during VM creation time via I915_VM_CREATE_FLAGS_USE_VM_BIND extension. > +A VM in VM_BIND mode will not support older execbuff mode of binding. > + > +UMDs can still send BOs of these persistent mappings in execlist of execbuff > +for specifying BO dependencies (implicit fencing) and to use BO as a batch, > +but those BOs should be mapped ahead via vm_bind ioctl. > + > +VM_BIND features include, > +- Multiple Virtual Address (VA) mappings can map to the same physical pages > + of an object (aliasing). > +- VA mapping can map to a partial section of the BO (partial binding). > +- Support capture of persistent mappings in the dump upon GPU error. > +- TLB is flushed upon unbind completion. Batching of TLB flushes in some > + usecases will be helpful. > +- Asynchronous vm_bind and vm_unbind support. > +- VM_BIND uses user/memory fence mechanism for signaling bind completion > + and for signaling batch completion in long running contexts (explained > + below). > + > +VM_PRIVATE objects > +-- > +By default, BOs can be mapped on multiple VMs and can also be dma-buf > +exported. Hence these BOs are referred to as Shared BOs. > +During each execbuff submission, the request fence must be added to the > +dma-resv fence list of all shared BOs mapped on the VM. > + > +VM_BIND feature introduces an optimization where user can create BO which > +is private to a specified VM via I915_GEM_CREATE_EXT_VM_PRIVATE flag during > +BO creation. Unlike Shared BOs, these VM private BOs can only be mapped on > +the VM they are private to and can't be dma-buf exported. > +All private BOs of a VM share the dma-resv object. Hence during each execbuff > +submission, they need only one dma-resv fence list updated. Thus the fast > +path (where required mappings are already bound) submission latency is O(1) > +w.r.t the number of VM private BOs. > + > +VM_BIND locking hirarchy > +- > +VM_BIND locking order is as below. > + > +1) A vm_bind mutex will protect vm_bind lists. This lock is taken in vm_bind/ > + vm_unbind ioctl calls, in the execbuff path and while releasing the > mapping. > + > + In future, when GPU page faults are supported, we can potentially use a > + rwsem instead, so that multiple pagefault handlers can take the read side > + lock to lookup the mapping and hence can run in parallel. > + > +2) The BO's dma-resv lock will protect i915_vma state and needs to be held > + while binding a vma and while updating dma-resv fence list of a BO. > + The private BOs of a VM will all share a dma-resv object. > + > + This lock is held in vm_bind call for immediate binding, during vm_unbind > + call for unbinding and during execbuff path for binding the mapping and > + updating the dma-resv fence list of the BO. > + > +3) Spinlock/s to protect some of the VM's lists. > + > +We will also need support for bluk LRU movement of persistent mapping to > +avoid additional latencies in execbuff path. > + > +GPU page faults > + > +Both older execbuff mode and the newer VM_BIND mode of binding will require > +using dma-fence to ensure residency. > +In future when GPU page faults are supported, no dma-fence usage is required > +as residency is purely managed by installing and removing/invalidating ptes. > + > + > +User/Memory
Re: [PATCH 1/3] drm/bridge: Add MAINTAINERS entry for DRM drivers for bridge chip bindings
Hi, On Tue, Mar 8, 2022 at 11:07 AM Douglas Anderson wrote: > > The bindings for bridge chips should also get the same maintainers > entry so the right people get notified about bindings changes. > > Signed-off-by: Douglas Anderson > --- > > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) There didn't seem to be a reason to wait, so I pushed all 3 to drm-misc-next w/ collected tags 46db48f25ed1 drm/bridge: Add myself as a reviewer for the Parade PS8640 bridge chip 59c217b3dde5 drm/bridge: Add myself as a reviewer for the TI SN65DSI86 bridge chip 73a46da4fa7c drm/bridge: Add MAINTAINERS entry for DRM drivers for bridge chip bindings -Doug
Re: [PATCH v2] drm/amdgpu: Add support for drm_privacy_screen
Hi,
On 3/9/22 16:06, Sean Paul wrote:
> From: Sean Paul
>
> This patch adds the necessary hooks to make amdgpu aware of privacy
> screens. On devices with privacy screen drivers (such as thinkpad-acpi),
> the amdgpu driver will defer probe until it's ready and then sync the sw
> and hw state on each commit the connector is involved and enabled.
>
> Changes in v2:
> -Tweaked the drm_privacy_screen_get() error check to avoid logging
> errors when privacy screen is absent (Hans)
>
> Signed-off-by: Sean Paul
> Link: https://patchwork.freedesktop.org/patch/477640/ #v1
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede
Regards,
Hans
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 +
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c| 3 +++
> .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 16 +++-
> 3 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 2ab675123ae3..e2cfae56c020 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -26,6 +26,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include "amdgpu_drv.h"
> @@ -1988,6 +1989,7 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
> {
> struct drm_device *ddev;
> struct amdgpu_device *adev;
> + struct drm_privacy_screen *privacy_screen;
> unsigned long flags = ent->driver_data;
> int ret, retry = 0, i;
> bool supports_atomic = false;
> @@ -2063,6 +2065,13 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
> size = pci_resource_len(pdev, 0);
> is_fw_fb = amdgpu_is_fw_framebuffer(base, size);
>
> + /* If the LCD panel has a privacy screen, defer probe until its ready */
> + privacy_screen = drm_privacy_screen_get(>dev, NULL);
> + if (IS_ERR(privacy_screen) && PTR_ERR(privacy_screen) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> +
> + drm_privacy_screen_put(privacy_screen);
> +
> /* Get rid of things like offb */
> ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev,
> _kms_driver);
> if (ret)
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index e1d3db3fe8de..9e2bb6523add 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -9781,6 +9781,9 @@ static void amdgpu_dm_atomic_commit_tail(struct
> drm_atomic_state *state)
> if (acrtc) {
> new_crtc_state = drm_atomic_get_new_crtc_state(state,
> >base);
> old_crtc_state = drm_atomic_get_old_crtc_state(state,
> >base);
> +
> + /* Sync the privacy screen state between hw and sw */
> + drm_connector_update_privacy_screen(new_con_state);
> }
>
> /* Skip any modesets/resets */
> 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 740435ae3997..594a8002975a 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
> @@ -27,6 +27,7 @@
> #include
> #include
> #include
> +#include
> #include "dm_services.h"
> #include "amdgpu.h"
> #include "amdgpu_dm.h"
> @@ -506,6 +507,7 @@ void amdgpu_dm_initialize_dp_connector(struct
> amdgpu_display_manager *dm,
> struct amdgpu_dm_connector *aconnector,
> int link_index)
> {
> + struct drm_device *dev = dm->ddev;
> struct dc_link_settings max_link_enc_cap = {0};
>
> aconnector->dm_dp_aux.aux.name =
> @@ -519,8 +521,20 @@ void amdgpu_dm_initialize_dp_connector(struct
> amdgpu_display_manager *dm,
> drm_dp_cec_register_connector(>dm_dp_aux.aux,
> >base);
>
> - if (aconnector->base.connector_type == DRM_MODE_CONNECTOR_eDP)
> + if (aconnector->base.connector_type == DRM_MODE_CONNECTOR_eDP) {
> + struct drm_privacy_screen *privacy_screen;
> +
> + /* Reference given up in drm_connector_cleanup() */
> + privacy_screen = drm_privacy_screen_get(dev->dev, NULL);
> + if (!IS_ERR(privacy_screen)) {
> +
> drm_connector_attach_privacy_screen_provider(>base,
> +
> privacy_screen);
> + } else if (PTR_ERR(privacy_screen) != -ENODEV) {
> + drm_err(dev, "Error getting privacy screen, ret=%d\n",
> + PTR_ERR(privacy_screen));
> + }
> return;
> + }
>
> dc_link_dp_get_max_link_enc_cap(aconnector->dc_link,
[PATCH 2/2] drm/bridge: adv7533: Add option to disable lane switching
On Renesas RZ/{G2L,V2L} platforms changing the lanes from 4 to 3 at
lower frequencies causes display instability. On such platforms, it
is better to avoid switching lanes from 4 to 3 as it needs different
set of PLL parameter constraints to make the display stable with 3
lanes.
This patch adds an option to disable lane switching if
'adi,disable-lanes-override' property is present in DT.
Signed-off-by: Biju Das
Reviewed-by: Lad Prabhakar
---
drivers/gpu/drm/bridge/adv7511/adv7511.h | 1 +
drivers/gpu/drm/bridge/adv7511/adv7533.c | 5 -
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
b/drivers/gpu/drm/bridge/adv7511/adv7511.h
index 592ecfcf00ca..7505601f10fb 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -368,6 +368,7 @@ struct adv7511 {
struct mipi_dsi_device *dsi;
u8 num_dsi_lanes;
bool use_timing_gen;
+ bool override_lanes;
enum adv7511_type type;
struct platform_device *audio_pdev;
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c
b/drivers/gpu/drm/bridge/adv7511/adv7533.c
index eb7579dec40a..7f6a8e95d70e 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
@@ -108,7 +108,7 @@ void adv7533_mode_set(struct adv7511 *adv, const struct
drm_display_mode *mode)
if (adv->num_dsi_lanes != 4)
return;
- if (mode->clock > 8)
+ if (!adv->override_lanes || mode->clock > 8)
lanes = 4;
else
lanes = 3;
@@ -195,6 +195,9 @@ int adv7533_parse_dt(struct device_node *np, struct adv7511
*adv)
adv->use_timing_gen = !of_property_read_bool(np,
"adi,disable-timing-generator");
+ adv->override_lanes = !of_property_read_bool(np,
+ "adi,disable-lanes-override");
+
/* TODO: Check if these need to be parsed by DT or not */
adv->rgb = true;
adv->embedded_sync = false;
--
2.17.1
[PATCH 1/2] dt-bindings: drm: bridge: adi, adv7533: Document adi, disable-lanes-override property
On Renesas RZ/{G2L,V2L} platforms changing the lanes from 4 to 3 at
lower frequencies causes display instability. On such platforms, it
is better to avoid switching lanes from 4 to 3 as it needs different
set of PLL parameter constraints to make the display stable with 3
lanes.
This patch introduces 'adi,disable-lanes-override' property to disable
lane switching at lower frequencies.
Signed-off-by: Biju Das
Reviewed-by: Lad Prabhakar
---
.../devicetree/bindings/display/bridge/adi,adv7533.yaml | 5 +
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
b/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
index f36209137c8a..2dc378039d21 100644
--- a/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
@@ -84,6 +84,11 @@ properties:
timings for HDMI output.
type: boolean
+ adi,disable-lanes-override:
+description:
+ Disables the overriding lanes at lower frequencies.
+type: boolean
+
adi,dsi-lanes:
description: Number of DSI data lanes connected to the DSI host.
$ref: /schemas/types.yaml#/definitions/uint32
--
2.17.1
[PATCH v2] drm/amdgpu: Add support for drm_privacy_screen
From: Sean Paul
This patch adds the necessary hooks to make amdgpu aware of privacy
screens. On devices with privacy screen drivers (such as thinkpad-acpi),
the amdgpu driver will defer probe until it's ready and then sync the sw
and hw state on each commit the connector is involved and enabled.
Changes in v2:
-Tweaked the drm_privacy_screen_get() error check to avoid logging
errors when privacy screen is absent (Hans)
Signed-off-by: Sean Paul
Link: https://patchwork.freedesktop.org/patch/477640/ #v1
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 +
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c| 3 +++
.../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 16 +++-
3 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 2ab675123ae3..e2cfae56c020 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -26,6 +26,7 @@
#include
#include
#include
+#include
#include
#include
#include "amdgpu_drv.h"
@@ -1988,6 +1989,7 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
{
struct drm_device *ddev;
struct amdgpu_device *adev;
+ struct drm_privacy_screen *privacy_screen;
unsigned long flags = ent->driver_data;
int ret, retry = 0, i;
bool supports_atomic = false;
@@ -2063,6 +2065,13 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
size = pci_resource_len(pdev, 0);
is_fw_fb = amdgpu_is_fw_framebuffer(base, size);
+ /* If the LCD panel has a privacy screen, defer probe until its ready */
+ privacy_screen = drm_privacy_screen_get(>dev, NULL);
+ if (IS_ERR(privacy_screen) && PTR_ERR(privacy_screen) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+
+ drm_privacy_screen_put(privacy_screen);
+
/* Get rid of things like offb */
ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev,
_kms_driver);
if (ret)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index e1d3db3fe8de..9e2bb6523add 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -9781,6 +9781,9 @@ static void amdgpu_dm_atomic_commit_tail(struct
drm_atomic_state *state)
if (acrtc) {
new_crtc_state = drm_atomic_get_new_crtc_state(state,
>base);
old_crtc_state = drm_atomic_get_old_crtc_state(state,
>base);
+
+ /* Sync the privacy screen state between hw and sw */
+ drm_connector_update_privacy_screen(new_con_state);
}
/* Skip any modesets/resets */
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 740435ae3997..594a8002975a 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
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include "dm_services.h"
#include "amdgpu.h"
#include "amdgpu_dm.h"
@@ -506,6 +507,7 @@ void amdgpu_dm_initialize_dp_connector(struct
amdgpu_display_manager *dm,
struct amdgpu_dm_connector *aconnector,
int link_index)
{
+ struct drm_device *dev = dm->ddev;
struct dc_link_settings max_link_enc_cap = {0};
aconnector->dm_dp_aux.aux.name =
@@ -519,8 +521,20 @@ void amdgpu_dm_initialize_dp_connector(struct
amdgpu_display_manager *dm,
drm_dp_cec_register_connector(>dm_dp_aux.aux,
>base);
- if (aconnector->base.connector_type == DRM_MODE_CONNECTOR_eDP)
+ if (aconnector->base.connector_type == DRM_MODE_CONNECTOR_eDP) {
+ struct drm_privacy_screen *privacy_screen;
+
+ /* Reference given up in drm_connector_cleanup() */
+ privacy_screen = drm_privacy_screen_get(dev->dev, NULL);
+ if (!IS_ERR(privacy_screen)) {
+
drm_connector_attach_privacy_screen_provider(>base,
+
privacy_screen);
+ } else if (PTR_ERR(privacy_screen) != -ENODEV) {
+ drm_err(dev, "Error getting privacy screen, ret=%d\n",
+ PTR_ERR(privacy_screen));
+ }
return;
+ }
dc_link_dp_get_max_link_enc_cap(aconnector->dc_link, _link_enc_cap);
aconnector->mst_mgr.cbs = _mst_cbs;
--
Sean Paul, Software Engineer, Google / Chromium OS
Re: [PATCH] drm/bridge: anx7625: switch to devm_drm_of_get_bridge
On Mon, 21 Feb 2022 at 09:55, Maxime Ripard wrote: > > On Mon, Feb 21, 2022 at 08:28:35AM +0100, José Expósito wrote: > > The function "drm_of_find_panel_or_bridge" has been deprecated in > > favor of "devm_drm_of_get_bridge". > > > > Switch to the new function and reduce boilerplate. > > > > Signed-off-by: José Expósito > > Reviewed-by: Maxime Ripard Applied to drm-misc-next
[Bug 205089] amdgpu : drm:amdgpu_cs_ioctl : Failed to initialize parser -125
https://bugzilla.kernel.org/show_bug.cgi?id=205089 --- Comment #33 from Alex Deucher (alexdeuc...@gmail.com) --- Please try newer or older mesa drivers if you can repro this with a particular game like dota2. The kernel driver is just the messenger. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH v2 4/4] drm/bridge: ti-sn65dsi86: Support hotplug detection
Quoting Doug Anderson (2022-03-07 23:22:17)
> Hi,
>
> On Mon, Mar 7, 2022 at 10:00 AM Kieran Bingham
> wrote:
> >
> > @@ -1264,6 +1321,14 @@ static int ti_sn_bridge_probe(struct
> > auxiliary_device *adev,
> > return PTR_ERR(pdata->next_bridge);
> > }
> >
> > + pdata->no_hpd = of_property_read_bool(np, "no-hpd");
> > + if (pdata->next_bridge->type != DRM_MODE_CONNECTOR_DisplayPort &&
> > + !pdata->no_hpd) {
> > + dev_warn(pdata->dev,
> > +"HPD support requires a DisplayPort connector\n");
>
> Maybe "HPD support only implemented for full DP connectors".
>
> Plausibly someone could come up with a reason to hook HPD up for eDP
> one day, but so far we haven't seen it.
>
Ok, updated.
>
> > @@ -1272,7 +1337,8 @@ static int ti_sn_bridge_probe(struct auxiliary_device
> > *adev,
> >
> > pdata->bridge.funcs = _sn_bridge_funcs;
> > pdata->bridge.of_node = np;
> > - pdata->bridge.ops = DRM_BRIDGE_OP_EDID;
> > + pdata->bridge.ops = (pdata->no_hpd ? 0 : DRM_BRIDGE_OP_DETECT |
> > DRM_BRIDGE_OP_HPD)
> > + | DRM_BRIDGE_OP_EDID;
>
> Seems like "OP_HPD" ought to be dependent on whether the IRQ was
> provided, right? AKA you could have "detect" because HPD is plumbed
> through to the bridge but not "HPD" if the IRQ from the bridge isn't
> hooked up to the main processor...
Yes, I think that's right. If there's no IRQ, then OP_HPD shouldn't be
set, and it will fall back to polling.
I'll fix that up.
> > @@ -1840,6 +1906,34 @@ static int ti_sn65dsi86_parse_regulators(struct
> > ti_sn65dsi86 *pdata)
> >pdata->supplies);
> > }
> >
> > +static irqreturn_t ti_sn65dsi86_irq_handler(int irq, void *arg)
> > +{
> > + struct ti_sn65dsi86 *pdata = arg;
> > + int ret;
> > + int hpd;
>
> `hpd` should be an `unsigned int` to match the prototype of regmap-read.
Agreed, and updated.
> > @@ -1881,6 +1975,16 @@ static int ti_sn65dsi86_probe(struct i2c_client
> > *client,
> > return dev_err_probe(dev, PTR_ERR(pdata->refclk),
> > "failed to get reference clock\n");
> >
> > + if (client->irq > 0) {
> > + ret = devm_request_threaded_irq(dev, client->irq, NULL,
> > + ti_sn65dsi86_irq_handler,
> > + IRQF_ONESHOT,
> > "sn65dsi86-irq",
> > + pdata);
> > + if (ret)
> > + return dev_err_probe(dev, ret,
> > +"Failed to register DP
> > interrupt\n");
> > + }
>
> Is this the right place to request the IRQ, or should it be in
> ti_sn_bridge_probe(). As soon as you request it the interrupt can go
> off, but you're relying on a bunch of bridge stuff to have been
> initted, right?
Indeed, it will be relying upon the bridge to have been set up.
You're right I believe, ti_sn_bridge_probe() sounds reasonable. And only
after that should we enable the interrupts.
Fixing ... (But getting stuck/blocked by the connector changes, so ..
I'll keep plowing through).
> > @@ -1888,6 +1992,9 @@ static int ti_sn65dsi86_probe(struct i2c_client
> > *client,
> > pm_runtime_set_autosuspend_delay(pdata->dev, 500);
> > pm_runtime_use_autosuspend(pdata->dev);
> >
> > + /* Enable interrupt handling */
> > + regmap_write(pdata->regmap, SN_IRQ_EN_REG, IRQ_EN);
>
> Shouldn't we only enable interrupt handling if client->irq > 0? AKA
> combine this with the "if" statement?
Agreed.
> -Doug
RE: [PATCH v2] drm/amdkfd: CRIU export dmabuf handles for GTT BOs
This is the link to the user mode change:
https://github.com/checkpoint-restore/criu/pull/1709
Regards,
David
> -Original Message-
> From: Kuehling, Felix
> Sent: Tuesday, March 8, 2022 4:20 PM
> To: Yat Sin, David ; amd-...@lists.freedesktop.org;
> dri-devel@lists.freedesktop.org
> Subject: Re: [PATCH v2] drm/amdkfd: CRIU export dmabuf handles for GTT
> BOs
>
>
> Am 2022-03-08 um 16:08 schrieb David Yat Sin:
> > Export dmabuf handles for GTT BOs so that their contents can be
> > accessed using SDMA during checkpoint/restore.
> >
> > Signed-off-by: David Yat Sin
>
> Looks good to me. Please also post a link to the user mode change for this.
>
> Note that the user mode code has not been merged upstream yet. I think this
> should be the final cleanup before the user mode CRIU plugin can be merged
> with the updated KFD version dependency.
>
> Thanks,
> Felix
>
>
> > ---
> > drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 12
> > include/uapi/linux/kfd_ioctl.h | 3 ++-
> > 2 files changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > index 2c7d76e67ddb..e1e2362841f8 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > @@ -1759,7 +1759,8 @@ static int criu_checkpoint_bos(struct
> kfd_process *p,
> > goto exit;
> > }
> > }
> > - if (bo_bucket->alloc_flags &
> KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
> > + if (bo_bucket->alloc_flags
> > + & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
> > +KFD_IOC_ALLOC_MEM_FLAGS_GTT)) {
> > ret = criu_get_prime_handle(_bo-
> >tbo.base,
> > bo_bucket->alloc_flags &
> >
> KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE ? DRM_RDWR : 0, @@ -
> 1812,7
> > +1813,8 @@ static int criu_checkpoint_bos(struct kfd_process *p,
> >
> > exit:
> > while (ret && bo_index--) {
> > - if (bo_buckets[bo_index].alloc_flags &
> KFD_IOC_ALLOC_MEM_FLAGS_VRAM)
> > + if (bo_buckets[bo_index].alloc_flags
> > + & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
> KFD_IOC_ALLOC_MEM_FLAGS_GTT))
> > close_fd(bo_buckets[bo_index].dmabuf_fd);
> > }
> >
> > @@ -2211,7 +2213,8 @@ static int criu_restore_bo(struct kfd_process
> > *p,
> >
> > pr_debug("map memory was successful for the BO\n");
> > /* create the dmabuf object and export the bo */
> > - if (bo_bucket->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
> > + if (bo_bucket->alloc_flags
> > + & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
> KFD_IOC_ALLOC_MEM_FLAGS_GTT))
> > +{
> > ret = criu_get_prime_handle(_mem->bo->tbo.base,
> DRM_RDWR,
> > _bucket->dmabuf_fd);
> > if (ret)
> > @@ -2281,7 +2284,8 @@ static int criu_restore_bos(struct kfd_process
> > *p,
> >
> > exit:
> > while (ret && i--) {
> > - if (bo_buckets[i].alloc_flags &
> KFD_IOC_ALLOC_MEM_FLAGS_VRAM)
> > + if (bo_buckets[i].alloc_flags
> > + & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
> KFD_IOC_ALLOC_MEM_FLAGS_GTT))
> > close_fd(bo_buckets[i].dmabuf_fd);
> > }
> > kvfree(bo_buckets);
> > diff --git a/include/uapi/linux/kfd_ioctl.h
> > b/include/uapi/linux/kfd_ioctl.h index b40687bf1014..eb9ff85f8556
> > 100644
> > --- a/include/uapi/linux/kfd_ioctl.h
> > +++ b/include/uapi/linux/kfd_ioctl.h
> > @@ -33,9 +33,10 @@
> >* - 1.5 - Add SVM API
> >* - 1.6 - Query clear flags in SVM get_attr API
> >* - 1.7 - Checkpoint Restore (CRIU) API
> > + * - 1.8 - CRIU - Support for SDMA transfers with GTT BOs
> >*/
> > #define KFD_IOCTL_MAJOR_VERSION 1
> > -#define KFD_IOCTL_MINOR_VERSION 7
> > +#define KFD_IOCTL_MINOR_VERSION 8
> >
> > struct kfd_ioctl_get_version_args {
> > __u32 major_version;/* from KFD */
Re: [PATCH 06/11] drm/amdgpu: remove GTT accounting v2
Hi
This patch seems to be causing me problems
https://gitlab.freedesktop.org/drm/amd/-/issues/1927
There are 3 issues I'm experiencing, two kernel bugs and a mesa bug
Cheers
Mike
On Mon, 14 Feb 2022 at 09:34, Christian König
wrote:
>
> This is provided by TTM now.
>
> Also switch man->size to bytes instead of pages and fix the double
> printing of size and usage in debugfs.
>
> v2: fix size checking as well
>
> Signed-off-by: Christian König
> Tested-by: Bas Nieuwenhuizen
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 49 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 ++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 -
> 4 files changed, 16 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> index e0c7fbe01d93..3bcd27ae379d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> @@ -60,7 +60,7 @@ static ssize_t amdgpu_mem_info_gtt_total_show(struct device
> *dev,
> struct ttm_resource_manager *man;
>
> man = ttm_manager_type(>mman.bdev, TTM_PL_TT);
> - return sysfs_emit(buf, "%llu\n", man->size * PAGE_SIZE);
> + return sysfs_emit(buf, "%llu\n", man->size);
> }
>
> /**
> @@ -77,8 +77,9 @@ static ssize_t amdgpu_mem_info_gtt_used_show(struct device
> *dev,
> {
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
> + struct ttm_resource_manager *man = >mman.gtt_mgr.manager;
>
> - return sysfs_emit(buf, "%llu\n",
> amdgpu_gtt_mgr_usage(>mman.gtt_mgr));
> + return sysfs_emit(buf, "%llu\n", ttm_resource_manager_usage(man));
> }
>
> static DEVICE_ATTR(mem_info_gtt_total, S_IRUGO,
> @@ -130,20 +131,17 @@ static int amdgpu_gtt_mgr_new(struct
> ttm_resource_manager *man,
> struct amdgpu_gtt_node *node;
> int r;
>
> - if (!(place->flags & TTM_PL_FLAG_TEMPORARY) &&
> - atomic64_add_return(num_pages, >used) > man->size) {
> - atomic64_sub(num_pages, >used);
> - return -ENOSPC;
> - }
> -
> node = kzalloc(struct_size(node, base.mm_nodes, 1), GFP_KERNEL);
> - if (!node) {
> - r = -ENOMEM;
> - goto err_out;
> - }
> + if (!node)
> + return -ENOMEM;
>
> node->tbo = tbo;
> ttm_resource_init(tbo, place, >base.base);
> + if (!(place->flags & TTM_PL_FLAG_TEMPORARY) &&
> + ttm_resource_manager_usage(man) > man->size) {
> + r = -ENOSPC;
> + goto err_free;
> + }
>
> if (place->lpfn) {
> spin_lock(>lock);
> @@ -169,11 +167,6 @@ static int amdgpu_gtt_mgr_new(struct
> ttm_resource_manager *man,
> err_free:
> ttm_resource_fini(man, >base.base);
> kfree(node);
> -
> -err_out:
> - if (!(place->flags & TTM_PL_FLAG_TEMPORARY))
> - atomic64_sub(num_pages, >used);
> -
> return r;
> }
>
> @@ -196,25 +189,10 @@ static void amdgpu_gtt_mgr_del(struct
> ttm_resource_manager *man,
> drm_mm_remove_node(>base.mm_nodes[0]);
> spin_unlock(>lock);
>
> - if (!(res->placement & TTM_PL_FLAG_TEMPORARY))
> - atomic64_sub(res->num_pages, >used);
> -
> ttm_resource_fini(man, res);
> kfree(node);
> }
>
> -/**
> - * amdgpu_gtt_mgr_usage - return usage of GTT domain
> - *
> - * @mgr: amdgpu_gtt_mgr pointer
> - *
> - * Return how many bytes are used in the GTT domain
> - */
> -uint64_t amdgpu_gtt_mgr_usage(struct amdgpu_gtt_mgr *mgr)
> -{
> - return atomic64_read(>used) * PAGE_SIZE;
> -}
> -
> /**
> * amdgpu_gtt_mgr_recover - re-init gart
> *
> @@ -260,9 +238,6 @@ static void amdgpu_gtt_mgr_debug(struct
> ttm_resource_manager *man,
> spin_lock(>lock);
> drm_mm_print(>mm, printer);
> spin_unlock(>lock);
> -
> - drm_printf(printer, "man size:%llu pages, gtt used:%llu pages\n",
> - man->size, atomic64_read(>used));
> }
>
> static const struct ttm_resource_manager_func amdgpu_gtt_mgr_func = {
> @@ -288,14 +263,12 @@ int amdgpu_gtt_mgr_init(struct amdgpu_device *adev,
> uint64_t gtt_size)
> man->use_tt = true;
> man->func = _gtt_mgr_func;
>
> - ttm_resource_manager_init(man, >mman.bdev,
> - gtt_size >> PAGE_SHIFT);
> + ttm_resource_manager_init(man, >mman.bdev, gtt_size);
>
> start = AMDGPU_GTT_MAX_TRANSFER_SIZE *
> AMDGPU_GTT_NUM_TRANSFER_WINDOWS;
> size = (adev->gmc.gart_size >> PAGE_SHIFT) - start;
> drm_mm_init(>mm, start, size);
> spin_lock_init(>lock);
> - atomic64_set(>used, 0);
>
> ttm_set_driver_manager(>mman.bdev, TTM_PL_TT, >manager);
> ttm_resource_manager_set_used(man, true);
> diff --git
RE: [PATCH v2] drm/amdkfd: CRIU export dmabuf handles for GTT BOs
[AMD Official Use Only]
Please ignore the previous email, that was sent in error. This one is with the
minor version bump so this looks good.
Reviewed-by : Rajneesh Bhardwaj
-Original Message-
From: amd-gfx On Behalf Of David Yat Sin
Sent: Tuesday, March 8, 2022 4:08 PM
To: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org
Cc: Kuehling, Felix ; Yat Sin, David
Subject: [PATCH v2] drm/amdkfd: CRIU export dmabuf handles for GTT BOs
Export dmabuf handles for GTT BOs so that their contents can be accessed using
SDMA during checkpoint/restore.
Signed-off-by: David Yat Sin
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 12
include/uapi/linux/kfd_ioctl.h | 3 ++-
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 2c7d76e67ddb..e1e2362841f8 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1759,7 +1759,8 @@ static int criu_checkpoint_bos(struct kfd_process *p,
goto exit;
}
}
- if (bo_bucket->alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
+ if (bo_bucket->alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
KFD_IOC_ALLOC_MEM_FLAGS_GTT))
+{
ret =
criu_get_prime_handle(_bo->tbo.base,
bo_bucket->alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE ? DRM_RDWR : 0, @@ -1812,7 +1813,8 @@ static
int criu_checkpoint_bos(struct kfd_process *p,
exit:
while (ret && bo_index--) {
- if (bo_buckets[bo_index].alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_VRAM)
+ if (bo_buckets[bo_index].alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
KFD_IOC_ALLOC_MEM_FLAGS_GTT))
close_fd(bo_buckets[bo_index].dmabuf_fd);
}
@@ -2211,7 +2213,8 @@ static int criu_restore_bo(struct kfd_process *p,
pr_debug("map memory was successful for the BO\n");
/* create the dmabuf object and export the bo */
- if (bo_bucket->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
+ if (bo_bucket->alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM | KFD_IOC_ALLOC_MEM_FLAGS_GTT)) {
ret = criu_get_prime_handle(_mem->bo->tbo.base, DRM_RDWR,
_bucket->dmabuf_fd);
if (ret)
@@ -2281,7 +2284,8 @@ static int criu_restore_bos(struct kfd_process *p,
exit:
while (ret && i--) {
- if (bo_buckets[i].alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM)
+ if (bo_buckets[i].alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
KFD_IOC_ALLOC_MEM_FLAGS_GTT))
close_fd(bo_buckets[i].dmabuf_fd);
}
kvfree(bo_buckets);
diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
index b40687bf1014..eb9ff85f8556 100644
--- a/include/uapi/linux/kfd_ioctl.h
+++ b/include/uapi/linux/kfd_ioctl.h
@@ -33,9 +33,10 @@
* - 1.5 - Add SVM API
* - 1.6 - Query clear flags in SVM get_attr API
* - 1.7 - Checkpoint Restore (CRIU) API
+ * - 1.8 - CRIU - Support for SDMA transfers with GTT BOs
*/
#define KFD_IOCTL_MAJOR_VERSION 1
-#define KFD_IOCTL_MINOR_VERSION 7
+#define KFD_IOCTL_MINOR_VERSION 8
struct kfd_ioctl_get_version_args {
__u32 major_version;/* from KFD */
--
2.17.1
Re: [PATCH v6 0/6] drm: exynos: dsi: Convert drm bridge
Hi Frieder, On Wed, Mar 9, 2022 at 6:54 PM Frieder Schrempf wrote: > > Hi Jagan, > > Am 03.03.22 um 17:36 schrieb Jagan Teki: > > Updated series about drm bridge conversion of exynos dsi. > > > > Previous version can be accessible, here [1]. > > > > Patch 1: tc358764 panel_bridge API > > > > Patch 2: connector reset > > > > Patch 3: bridge attach in MIC > > > > Patch 4: panel_bridge API > > > > Patch 5: bridge conversion > > > > Patch 6: atomic functions > > > > [1] https://patchwork.amarulasolutions.com/cover/1839 > > > > Any inputs? > > Thanks for your efforts. I didn't follow the whole history, but I'm > looking forward and hope to see upstream support for the i.MX8MM DSIM in > the not too distant future. > > Can you give me a short update about the state of this patchset? Are > there still any major obstacles? > > I can't help with testing on Exynos, but if you have the matching > follow-up patches for i.MX8MM support somewhere around I could do some > tests with those on i.MX8MM. Unfortunately, it is getting slow due to existing exynos dsi drivers. Idea is to push exynos and then move the bridge as per Mailing-list discussion. I have initial series to support i.MX8MM on linux-next [1] which is working on my setup. However I'm waiting for this series to move further to send those on the mailing list. Indeed I'm solely relaying on Marek testing to move further as I too don't have Exynos hardware to validate. [1] https://github.com/openedev/kernel/tree/imx8mm-dsi Thanks, Jagan.
RE: [PATCH] drm/amdkfd: CRIU export dmabuf handles for GTT BOs
[AMD Official Use Only]
Reviewed-by: Rajneesh Bhardwaj
-Original Message-
From: amd-gfx On Behalf Of David Yat Sin
Sent: Tuesday, March 8, 2022 2:12 PM
To: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org
Cc: Kuehling, Felix ; Yat Sin, David
Subject: [PATCH] drm/amdkfd: CRIU export dmabuf handles for GTT BOs
Export dmabuf handles for GTT BOs so that their contents can be accessed using
SDMA during checkpoint/restore.
Signed-off-by: David Yat Sin
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 2c7d76e67ddb..e1e2362841f8 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1759,7 +1759,8 @@ static int criu_checkpoint_bos(struct kfd_process *p,
goto exit;
}
}
- if (bo_bucket->alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
+ if (bo_bucket->alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
KFD_IOC_ALLOC_MEM_FLAGS_GTT))
+{
ret =
criu_get_prime_handle(_bo->tbo.base,
bo_bucket->alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE ? DRM_RDWR : 0, @@ -1812,7 +1813,8 @@ static
int criu_checkpoint_bos(struct kfd_process *p,
exit:
while (ret && bo_index--) {
- if (bo_buckets[bo_index].alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_VRAM)
+ if (bo_buckets[bo_index].alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
KFD_IOC_ALLOC_MEM_FLAGS_GTT))
close_fd(bo_buckets[bo_index].dmabuf_fd);
}
@@ -2211,7 +2213,8 @@ static int criu_restore_bo(struct kfd_process *p,
pr_debug("map memory was successful for the BO\n");
/* create the dmabuf object and export the bo */
- if (bo_bucket->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
+ if (bo_bucket->alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM | KFD_IOC_ALLOC_MEM_FLAGS_GTT)) {
ret = criu_get_prime_handle(_mem->bo->tbo.base, DRM_RDWR,
_bucket->dmabuf_fd);
if (ret)
@@ -2281,7 +2284,8 @@ static int criu_restore_bos(struct kfd_process *p,
exit:
while (ret && i--) {
- if (bo_buckets[i].alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM)
+ if (bo_buckets[i].alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
KFD_IOC_ALLOC_MEM_FLAGS_GTT))
close_fd(bo_buckets[i].dmabuf_fd);
}
kvfree(bo_buckets);
--
2.17.1
Re: [PATCH 1/2] drm: ssd130x: Fix COM scan direction register mask
On 3/9/22 13:56, Geert Uytterhoeven wrote:
> On Wed, Mar 9, 2022 at 2:57 AM Chen-Yu Tsai wrote:
>> From: Chen-Yu Tsai
>>
>> The SSD130x's command to toggle COM scan direction uses bit 3 and only
>> bit 3 to set the direction of the scanout. The driver has an incorrect
>> GENMASK(3, 2), causing the setting to be set on bit 2, rendering it
>> ineffective.
>>
>> Fix the mask to only bit 3, so that the requested setting is applied
>> correctly.
>>
>> Fixes: a61732e80867 ("drm: Add driver for Solomon SSD130x OLED displays")
>> Signed-off-by: Chen-Yu Tsai
>
> Thanks, this fixes the vertically-mirrored display on my Adafruit
> FeatherWing 128x32 OLED.
> Tested-by: Geert Uytterhoeven
> Reviewed-by: Geert Uytterhoeven
>
Thanks for the testing and review. I've pushed both patches to drm-misc-next.
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [PATCH] drm/bridge: ti-sn65dsi86: switch to devm_drm_of_get_bridge
On Fri, 4 Mar 2022 at 13:12, Kieran Bingham
wrote:
>
> Hi José
>
> Quoting Kieran Bingham (2022-03-03 21:59:26)
> > Quoting José Expósito (2022-03-03 18:37:20)
> > > On Mon, Feb 28, 2022 at 11:24:36PM +, Kieran Bingham wrote:
> > > > Hi José
> > > >
> > > > Quoting José Expósito (2022-02-28 18:39:54)
> > > > > The function "drm_of_find_panel_or_bridge" has been deprecated in
> > > > > favor of "devm_drm_of_get_bridge".
> > > > >
> > > > > Switch to the new function and reduce boilerplate.
> > > > >
> > > > > Signed-off-by: José Expósito
> > > > > ---
> > > > > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 8 +---
> > > > > 1 file changed, 1 insertion(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > > > b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > > > index dab8f76618f3..fb8e16ed7e90 100644
> > > > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > > > @@ -1232,15 +1232,9 @@ static int ti_sn_bridge_probe(struct
> > > > > auxiliary_device *adev,
> > > > > {
> > > > > struct ti_sn65dsi86 *pdata =
> > > > > dev_get_drvdata(adev->dev.parent);
> > > > > struct device_node *np = pdata->dev->of_node;
> > > > > - struct drm_panel *panel;
> > > > > int ret;
> > > > >
> > > > > - ret = drm_of_find_panel_or_bridge(np, 1, 0, , NULL);
> > > > > - if (ret)
> > > > > - return dev_err_probe(>dev, ret,
> > > > > -"could not find any panel
> > > > > node\n");
> > > > > -
> > > > > - pdata->next_bridge = devm_drm_panel_bridge_add(pdata->dev,
> > > > > panel);
> > > > > + pdata->next_bridge = devm_drm_of_get_bridge(pdata->dev, np,
> > > > > 1, 0);
> > > >
> > > > Yikes, I was about to rely on this panel variable to determine if the
> > > > device is a panel or a display port connector. (Well, I am relying on
> > > > it, and patches are hoping to be reposted this week).
> > > >
> > > > Is there expected to be another way to identify if the next connection
> > > > is a panel or a bridge?
> > > >
> > > > Regards
> > >
> > > Hi Kieran,
> > >
> > > I'm getting started in the DRM subsystem. I couldn't tell if there is a
> > > good way to access the panel pointer... I didn't manage to find it, but
> > > hopefully someone with more experience can point us to a solution.
> > >
> > > Since you mentioned display port, I'm not sure if in your case checking
> > > "pdata->next_bridge->type" could be good enough.
>
> Actually, it is. And I think this is actually cleaner (both here, and in
> the series I'm working on).
>
> > > Anyway, if this patch causes you problems, please go ahead and ignore it.
> > > I'm sure the series you are working on are more important than removing
> > > a deprecated function :)
> >
> > If it's deprecated, I don't want to block it's removal. Hopefully I can
> > resume my work on this tomorrow so I can check to see what I can parse.
> > Thanks for the lead on the bridge type, I'm sure I've seen that around
> > too so hopefully that's enough. If it is, I'll rebase my work on top of
> > your patch and retest.
>
> So - my series is now working with a bit of adaptation to run on top of
> your patch, and I think the overall result is better. So:
>
> Reviewed-by: Kieran Bingham
>
Applied to drm-misc-next.
Re: [PATCH] drm/bridge: anx7625: check the return on anx7625_aux_trans
On Wed, 9 Mar 2022 at 02:45, Tom Rix wrote:
>
>
> On 3/8/22 2:57 PM, Nick Desaulniers wrote:
> > On Thu, Mar 3, 2022 at 12:19 PM wrote:
> >> From: Tom Rix
> >>
> >> Clang static analysis reports this issue
> >> anx7625.c:876:13: warning: The left operand of '&' is
> >>a garbage value
> >>if (!(bcap & 0xOA01)) {
> >> ^
> >>
> >> bcap is only set by a successful call to
> >> anx7625_aux_trans(). So check.
> >>
> >> Fixes: cd1637c7e480 ("drm/bridge: anx7625: add HDCP support")
> > Is this the correct Fixes tag?
> yes
> > I think it should be
> >
> > Fixes: adca62ec370c ("drm/bridge: anx7625: Support reading edid
> > through aux channel")
>
> This one changes the name of the function
>
> - anx7625_aux_dpcd_trans(ctx, DP_AUX_NATIVE_READ, 0x68028, 1, );
> + anx7625_aux_trans(ctx, DP_AUX_NATIVE_READ, 0x68028, 1, );
>
> A return check from the earlier commit, when this block of code came
> into existence, is when it was first needed.
>
> Tom
>
> >
> > instead.
> >
> >> Signed-off-by: Tom Rix
> >> ---
> >> drivers/gpu/drm/bridge/analogix/anx7625.c | 5 -
> >> 1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
> >> b/drivers/gpu/drm/bridge/analogix/anx7625.c
> >> index 633618bafd75d..f02ac079ed2ec 100644
> >> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> >> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> >> @@ -872,7 +872,10 @@ static int anx7625_hdcp_enable(struct anx7625_data
> >> *ctx)
> >> }
> >>
> >> /* Read downstream capability */
> >> - anx7625_aux_trans(ctx, DP_AUX_NATIVE_READ, 0x68028, 1, );
> >> + ret = anx7625_aux_trans(ctx, DP_AUX_NATIVE_READ, 0x68028, 1,
> >> );
> >> + if (ret < 0)
> >> + return ret;
> >> +
> >> if (!(bcap & 0x01)) {
> >> pr_warn("downstream not support HDCP 1.4, cap(%x).\n",
> >> bcap);
> >> return 0;
> >> --
> >> 2.26.3
> >>
> >
>
Reviewed-by: Robert Foss
Applied to drm-misc-next.
Re: [PATCH v6 3/6] drm: Add driver for Solomon SSD130x OLED displays
Hello Geert, On 3/9/22 13:56, Geert Uytterhoeven wrote: > Hi Javier, > > On Wed, Mar 9, 2022 at 1:14 PM Javier Martinez Canillas > wrote: >> On 3/8/22 17:30, Geert Uytterhoeven wrote: >>> Unfortunately a regression was introduced since your v3: printed >>> text is mirrored upside-down. I.e. "E" is rendered correctly, but "L" >>> turns into "Γ" (Greek Gamma). >>> I suspect something went wrong with the display initialization >>> sequence. >>> >> >> Could you please try Chen-Yu's fix for the COM scan direction mask ? >> >> https://lists.freedesktop.org/archives/dri-devel/2022-March/345915.html >> >> I made a mistake when converting to use the GENMASK() and FIELD_PREP() >> macros in v4 as suggested by Andy. The SSD130X_SET_COM_SCAN_DIR_MASK >> wasn't correct which would explain the output to be vertically flipped. > > Thanks, confirmed fixed. > Great, thanks a lot for testing and the confirmation! -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH v2 RFC 10/10] drm/fourcc: Add DRM_FORMAT_D[1248]
On 3/7/22 21:52, Geert Uytterhoeven wrote: > As Rn is light-on-dark, and Cn can be any colors, there are currently > no fourcc codes to describe dark-on-light displays. > > Introduce fourcc codes for a single-channel dark-on-light frame buffer > format with two, four, sixteen, or 256 darkness levels. > > As the number of bits per pixel may be less than eight, some of these > formats rely on proper block handling for the calculation of bits per > pixel and pitch. > > Signed-off-by: Geert Uytterhoeven > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH v2 RFC 09/10] drm/fourcc: Add DRM_FORMAT_R[124]
On 3/7/22 21:52, Geert Uytterhoeven wrote: > Introduce fourcc codes for single-channel light-on-dark frame buffer > formats with two, four, and sixteen intensity levels. > > As the number of bits per pixel is less than eight, these rely on proper > block handling for the calculation of bits per pixel and pitch. > > Signed-off-by: Geert Uytterhoeven > --- IIRC the agreement was that it was worth to have both Cx and Rx fourcc formats separately, so this patch makes sense to me. Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH v2 RFC 08/10] drm/fourcc: Document that single-channel "red" can be any color
On 3/7/22 21:52, Geert Uytterhoeven wrote: > Traditionally, the first channel has been called the "red" channel, but > the fourcc values for single-channel "red" formats can also be used for > other light-on-dark displays, like grayscale. > > Signed-off-by: Geert Uytterhoeven > --- Yes, I learned that "Red" actually meant just a color channel that may not be red in one of the thread about fourcc formats. So I agree that would be good to have a comment about this. Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH v6 0/6] drm: exynos: dsi: Convert drm bridge
Hi Jagan, Am 03.03.22 um 17:36 schrieb Jagan Teki: > Updated series about drm bridge conversion of exynos dsi. > > Previous version can be accessible, here [1]. > > Patch 1: tc358764 panel_bridge API > > Patch 2: connector reset > > Patch 3: bridge attach in MIC > > Patch 4: panel_bridge API > > Patch 5: bridge conversion > > Patch 6: atomic functions > > [1] https://patchwork.amarulasolutions.com/cover/1839 > > Any inputs? Thanks for your efforts. I didn't follow the whole history, but I'm looking forward and hope to see upstream support for the i.MX8MM DSIM in the not too distant future. Can you give me a short update about the state of this patchset? Are there still any major obstacles? I can't help with testing on Exynos, but if you have the matching follow-up patches for i.MX8MM support somewhere around I could do some tests with those on i.MX8MM. Thanks Frieder
Re: [PATCH 3/3] drm/bridge: Add myself as a reviewer for the Parade PS8640 bridge chip
On Tue, 8 Mar 2022 at 20:07, Douglas Anderson wrote: > > Though the parade bridge chip is a little bit of a black box, I'm at > least interested in hearing about changes to the driver since this > bridge chip is used on some Chromebooks that I'm involved with. > > Signed-off-by: Douglas Anderson > --- > > MAINTAINERS | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 7d25d0b4dccc..db7fe53643c2 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6171,6 +6171,11 @@ S: Maintained > F: > Documentation/devicetree/bindings/display/panel/olimex,lcd-olinuxino.yaml > F: drivers/gpu/drm/panel/panel-olimex-lcd-olinuxino.c > > +DRM DRIVER FOR PARADE PS8640 BRIDGE CHIP > +R: Douglas Anderson > +F: Documentation/devicetree/bindings/display/bridge/ps8640.yaml > +F: drivers/gpu/drm/bridge/parade-ps8640.c > + > DRM DRIVER FOR PERVASIVE DISPLAYS REPAPER PANELS > M: Noralf Trønnes > S: Maintained > -- > 2.35.1.616.g0bdcbb4464-goog > Reviewed-by: Robert Foss
Re: [PATCH 2/3] drm/bridge: Add myself as a reviewer for the TI SN65DSI86 bridge chip
On Tue, 8 Mar 2022 at 20:07, Douglas Anderson wrote: > > I've spent quite a bit of time poking at this driver and it's used on > several Chromebooks I'm involved with. I'd like to get notified about > patches. Add myself as a reviewer. It's expected that changes will > still be landed through drm-misc as they always have been. > > Signed-off-by: Douglas Anderson > --- > > MAINTAINERS | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index a73179d55d00..7d25d0b4dccc 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6284,6 +6284,11 @@ DRM DRIVER FOR TDFX VIDEO CARDS > S: Orphan / Obsolete > F: drivers/gpu/drm/tdfx/ > > +DRM DRIVER FOR TI SN65DSI86 BRIDGE CHIP > +R: Douglas Anderson > +F: Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml > +F: drivers/gpu/drm/bridge/ti-sn65dsi86.c > + > DRM DRIVER FOR TPO TPG110 PANELS > M: Linus Walleij > S: Maintained > -- > 2.35.1.616.g0bdcbb4464-goog > Reviewed-by: Robert Foss
Re: [RESEND V11 0/3] force hsa hbp hfp packets multiple of lanenum to avoid screen shift
On Wed, 9 Mar 2022 at 08:36, Rex-BC Chen wrote: > > Resend v11: > - Resend this series for reviewing. > - Rebase to 5.17-rc7. > > Changes since v10: > - Rebase to 5.17-rc3. > - Add more maintainers. > > Changes since v9: > - Change description of "MIPI_DSI_HS_PKT_END_ALIGNED". > - Use mode_flags directly instead of another variable on patch [2/3]. > - Add explanation of implementation in mtk_dsi.c on commit message of [2/3]. > > Changes since v8: > - Use mode_flags to control this limitation instead of > "hs_packet_end_aligned". > - Add new bit definition "MIPI_DSI_HS_PKT_END_ALIGNED" for mode_flags. > > Changes since v7: > - Rebase to kernel 5.16 > - Add tags of reviewed-by and acked-by. > - Add detailed commit message for flag "hs_packet_end_aligned" in DSI common > driver. > > Changes since v6: > - Add "bool hs_packet_end_aligned" in "struct mipi_dsi_device" to control > the dsi aligned. > - Config the "hs_packet_end_aligned" in ANX7725 .attach(). > > Changes since v5: > - Search the anx7625 compatible as flag to control dsi output aligned. > > Changes since v4: > - Move "dt-bindings: drm/bridge: anx7625: add force_dsi_end_without_null" > before >"drm/mediatek: force hsa hbp hfp packets multiple of lanenum to avoid". > - Retitle "dt-bindings: drm/bridge: anx7625: add force_dsi_end_without_null". > > Rex-BC Chen (3): > drm/dsi: transfer DSI HS packets ending at the same time > drm/mediatek: implement the DSI HS packets aligned > drm/bridge: anx7625: config hs packets end aligned to avoid screen > shift > > drivers/gpu/drm/bridge/analogix/anx7625.c | 3 ++- > drivers/gpu/drm/mediatek/mtk_dsi.c| 12 > include/drm/drm_mipi_dsi.h| 2 ++ > 3 files changed, 16 insertions(+), 1 deletion(-) > > -- > 2.18.0 > Applied to drm-misc-next
Re: [PATCH v2 RFC 07/10] drm/gem-fb-helper: Use actual bpp for size calculations
On 3/7/22 21:52, Geert Uytterhoeven wrote: > The AFBC helpers derive the number of bits per pixel from the deprecated > drm_format_info.cpp[] field, which does not take into account block > sizes. > > Fix this by using the actual number of bits per pixel instead. > > Signed-off-by: Geert Uytterhoeven > --- > RFC, as this code path was untested. > Looks good to me but haven't tested either. Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH v2 06/10] drm/fb-helper: Add support for DRM_FORMAT_C[124]
Hi Javier,
On Wed, Mar 9, 2022 at 2:10 PM Javier Martinez Canillas
wrote:
> On 3/7/22 21:52, Geert Uytterhoeven wrote:
> > Add support for color-indexed frame buffer formats with two, four, and
> > sixteen colors to the DRM framebuffer helper functions:
> > 1. Add support for 1, 2, and 4 bits per pixel to the damage helper,
> > 2. For color-indexed modes, the length of the color bitfields must be
> > set to the color depth, else the logo code may pick a logo with too
> > many colors. Drop the incorrect DAC width comment, which
> > originates from the i915 driver.
> > 3. Accept C[124] modes when validating or filling in struct
> > fb_var_screeninfo, and use the correct number of bits per pixel.
> > 4. Set the visual to FB_VISUAL_PSEUDOCOLOR for all color-indexed
> > modes.
> >
> > Signed-off-by: Geert Uytterhoeven
>
> [snip]
>
> > static void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch,
> > -uint32_t depth)
> > +bool is_color_indexed)
> > {
> > info->fix.type = FB_TYPE_PACKED_PIXELS;
> > - info->fix.visual = depth == 8 ? FB_VISUAL_PSEUDOCOLOR :
> > - FB_VISUAL_TRUECOLOR;
> > + info->fix.visual = is_color_indexed ? FB_VISUAL_PSEUDOCOLOR
> > + : info->fix.visual;
>
> Shouldn't this be the following instead ?
>
> + info->fix.visual = is_color_indexed ? FB_VISUAL_PSEUDOCOLOR
> + : FB_VISUAL_TRUECOLOR;
Indeed. Will fix in v3.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 1/3] drm/bridge: Add MAINTAINERS entry for DRM drivers for bridge chip bindings
On Wed, 9 Mar 2022 at 09:04, Neil Armstrong wrote: > > On 08/03/2022 20:06, Douglas Anderson wrote: > > The bindings for bridge chips should also get the same maintainers > > entry so the right people get notified about bindings changes. > > > > Signed-off-by: Douglas Anderson > > --- > > > > MAINTAINERS | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 0216d2ffe728..a73179d55d00 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -6403,6 +6403,7 @@ R: Jonas Karlman > > R: Jernej Skrabec > > S: Maintained > > T: git git://anongit.freedesktop.org/drm/drm-misc > > +F: Documentation/devicetree/bindings/display/bridge/ > > F: drivers/gpu/drm/bridge/ > > > > DRM DRIVERS FOR EXYNOS > > Acked-by: Neil Armstrong Acked-by: Robert Foss
Re: [PATCH v2 06/10] drm/fb-helper: Add support for DRM_FORMAT_C[124]
On 3/7/22 21:52, Geert Uytterhoeven wrote:
> Add support for color-indexed frame buffer formats with two, four, and
> sixteen colors to the DRM framebuffer helper functions:
> 1. Add support for 1, 2, and 4 bits per pixel to the damage helper,
> 2. For color-indexed modes, the length of the color bitfields must be
> set to the color depth, else the logo code may pick a logo with too
> many colors. Drop the incorrect DAC width comment, which
> originates from the i915 driver.
> 3. Accept C[124] modes when validating or filling in struct
> fb_var_screeninfo, and use the correct number of bits per pixel.
> 4. Set the visual to FB_VISUAL_PSEUDOCOLOR for all color-indexed
> modes.
>
> Signed-off-by: Geert Uytterhoeven
[snip]
> static void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch,
> -uint32_t depth)
> +bool is_color_indexed)
> {
> info->fix.type = FB_TYPE_PACKED_PIXELS;
> - info->fix.visual = depth == 8 ? FB_VISUAL_PSEUDOCOLOR :
> - FB_VISUAL_TRUECOLOR;
> + info->fix.visual = is_color_indexed ? FB_VISUAL_PSEUDOCOLOR
> + : info->fix.visual;
Shouldn't this be the following instead ?
+ info->fix.visual = is_color_indexed ? FB_VISUAL_PSEUDOCOLOR
+ : FB_VISUAL_TRUECOLOR;
Other than that the patch looks good to me.
Reviewed-by: Javier Martinez Canillas
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [PATCH v6 3/6] drm: Add driver for Solomon SSD130x OLED displays
Hi Javier,
On Wed, Mar 9, 2022 at 1:14 PM Javier Martinez Canillas
wrote:
> On 3/8/22 17:30, Geert Uytterhoeven wrote:
> > Unfortunately a regression was introduced since your v3: printed
> > text is mirrored upside-down. I.e. "E" is rendered correctly, but "L"
> > turns into "Γ" (Greek Gamma).
> > I suspect something went wrong with the display initialization
> > sequence.
> >
>
> Could you please try Chen-Yu's fix for the COM scan direction mask ?
>
> https://lists.freedesktop.org/archives/dri-devel/2022-March/345915.html
>
> I made a mistake when converting to use the GENMASK() and FIELD_PREP()
> macros in v4 as suggested by Andy. The SSD130X_SET_COM_SCAN_DIR_MASK
> wasn't correct which would explain the output to be vertically flipped.
Thanks, confirmed fixed.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v2 05/10] drm/fourcc: Add DRM_FORMAT_C[124]
On 3/7/22 21:52, Geert Uytterhoeven wrote: > Introduce fourcc codes for color-indexed frame buffer formats with two, > four, and sixteen colors, and provide a mapping from bit per pixel and > depth to fourcc codes. > > As the number of bits per pixel is less than eight, these rely on proper > block handling for the calculation of bits per pixel and pitch. > > Signed-off-by: Geert Uytterhoeven > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH 1/2] drm: ssd130x: Fix COM scan direction register mask
On Wed, Mar 9, 2022 at 2:57 AM Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai
>
> The SSD130x's command to toggle COM scan direction uses bit 3 and only
> bit 3 to set the direction of the scanout. The driver has an incorrect
> GENMASK(3, 2), causing the setting to be set on bit 2, rendering it
> ineffective.
>
> Fix the mask to only bit 3, so that the requested setting is applied
> correctly.
>
> Fixes: a61732e80867 ("drm: Add driver for Solomon SSD130x OLED displays")
> Signed-off-by: Chen-Yu Tsai
Thanks, this fixes the vertically-mirrored display on my Adafruit
FeatherWing 128x32 OLED.
Tested-by: Geert Uytterhoeven
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v2 04/10] drm/framebuffer: Use actual bpp for DRM_IOCTL_MODE_GETFB
On 3/7/22 21:52, Geert Uytterhoeven wrote: > When userspace queries the properties of a frame buffer, the number of > bits per pixel is derived from the deprecated drm_format_info.cpp[] > field, which does not take into account block sizes. > > Fix this by using the actual number of bits per pixel instead. > > Signed-off-by: Geert Uytterhoeven > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH v2 03/10] drm/client: Use actual bpp when allocating frame buffers
On 3/7/22 21:52, Geert Uytterhoeven wrote: > When allocating a frame buffer, the number of bits per pixel needed is > derived from the deprecated drm_format_info.cpp[] field. While this > works for formats using less than 8 bits per pixel, it does lead to a > large overallocation. > > Reduce memory consumption by using the actual number of bits per pixel > instead. > > Signed-off-by: Geert Uytterhoeven > Acked-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH v2 02/10] drm/fourcc: Add drm_format_info.is_color_indexed flag
On 3/7/22 21:52, Geert Uytterhoeven wrote: > Add a flag to struct drm_format_info to indicate if a format is > color-indexed, similar to the existing .is_yuv flag. > > This way generic code and drivers can just check this flag, instead of > checking against a list of fourcc formats. > > Signed-off-by: Geert Uytterhoeven > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH v2 01/10] drm/fourcc: Add drm_format_info_bpp() helper
Hello Geert, On 3/7/22 21:52, Geert Uytterhoeven wrote: > Add a helper to retrieve the actual number of bits per pixel for a > plane, taking into account the number of characters and pixels per > block for tiled formats. > > Signed-off-by: Geert Uytterhoeven > --- Patch looks good to me. Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH v2 11/12] drm/gma500: Move GTT enable and disable code into helpers
On Tue, Mar 8, 2022 at 8:52 PM Thomas Zimmermann wrote:
>
> Move the code for enabling and disabling the GTT into helpers and call
> the functions in psb_gtt_init(), psb_gtt_fini() and psb_gtt_resume().
> Removes code duplication.
That makes it much more readable. Thanks.
Acked-by: Patrik Jakobsson
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/gma500/gtt.c | 81
> 1 file changed, 46 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
> index b03feec64f01..83d9a9f7c73c 100644
> --- a/drivers/gpu/drm/gma500/gtt.c
> +++ b/drivers/gpu/drm/gma500/gtt.c
> @@ -125,17 +125,44 @@ void psb_gtt_remove_pages(struct drm_psb_private *pdev,
> const struct resource *r
> mutex_unlock(>gtt_mutex);
> }
>
> -void psb_gtt_fini(struct drm_device *dev)
> +static int psb_gtt_enable(struct drm_psb_private *dev_priv)
> {
> - struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> + struct drm_device *dev = _priv->dev;
> struct pci_dev *pdev = to_pci_dev(dev->dev);
> + int ret;
>
> - iounmap(dev_priv->gtt_map);
> + ret = pci_read_config_word(pdev, PSB_GMCH_CTRL, _priv->gmch_ctrl);
> + if (ret)
> + return pcibios_err_to_errno(ret);
> + ret = pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl
> | _PSB_GMCH_ENABLED);
> + if (ret)
> + return pcibios_err_to_errno(ret);
> +
> + dev_priv->pge_ctl = PSB_RVDC32(PSB_PGETBL_CTL);
> + PSB_WVDC32(dev_priv->pge_ctl | _PSB_PGETBL_ENABLED, PSB_PGETBL_CTL);
> +
> + (void)PSB_RVDC32(PSB_PGETBL_CTL);
> +
> + return 0;
> +}
> +
> +static void psb_gtt_disable(struct drm_psb_private *dev_priv)
> +{
> + struct drm_device *dev = _priv->dev;
> + struct pci_dev *pdev = to_pci_dev(dev->dev);
>
> pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
> PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
> +
> (void)PSB_RVDC32(PSB_PGETBL_CTL);
> +}
>
> +void psb_gtt_fini(struct drm_device *dev)
> +{
> + struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> +
> + iounmap(dev_priv->gtt_map);
> + psb_gtt_disable(dev_priv);
> mutex_destroy(_priv->gtt_mutex);
> }
>
> @@ -159,22 +186,15 @@ int psb_gtt_init(struct drm_device *dev)
> {
> struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> struct pci_dev *pdev = to_pci_dev(dev->dev);
> + struct psb_gtt *pg = _priv->gtt;
> unsigned gtt_pages;
> - struct psb_gtt *pg;
> - int ret = 0;
> + int ret;
>
> mutex_init(_priv->gtt_mutex);
>
> - pg = _priv->gtt;
> -
> - /* Enable the GTT */
> - pci_read_config_word(pdev, PSB_GMCH_CTRL, _priv->gmch_ctrl);
> - pci_write_config_word(pdev, PSB_GMCH_CTRL,
> - dev_priv->gmch_ctrl | _PSB_GMCH_ENABLED);
> -
> - dev_priv->pge_ctl = PSB_RVDC32(PSB_PGETBL_CTL);
> - PSB_WVDC32(dev_priv->pge_ctl | _PSB_PGETBL_ENABLED, PSB_PGETBL_CTL);
> - (void) PSB_RVDC32(PSB_PGETBL_CTL);
> + ret = psb_gtt_enable(dev_priv);
> + if (ret)
> + goto err_mutex_destroy;
>
> /* The root resource we allocate address space from */
> pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
> @@ -227,17 +247,16 @@ int psb_gtt_init(struct drm_device *dev)
> if (!dev_priv->gtt_map) {
> dev_err(dev->dev, "Failure to map gtt.\n");
> ret = -ENOMEM;
> - goto err_gtt_disable;
> + goto err_psb_gtt_disable;
> }
>
> psb_gtt_clear(dev_priv);
>
> return 0;
>
> -err_gtt_disable:
> - pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
> - PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
> - (void)PSB_RVDC32(PSB_PGETBL_CTL);
> +err_psb_gtt_disable:
> + psb_gtt_disable(dev_priv);
> +err_mutex_destroy:
> mutex_destroy(_priv->gtt_mutex);
> return ret;
> }
> @@ -246,20 +265,14 @@ int psb_gtt_resume(struct drm_device *dev)
> {
> struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> struct pci_dev *pdev = to_pci_dev(dev->dev);
> + struct psb_gtt *pg = _priv->gtt;
> unsigned int gtt_pages;
> - struct psb_gtt *pg;
> int ret;
>
> - pg = _priv->gtt;
> -
> /* Enable the GTT */
> - pci_read_config_word(pdev, PSB_GMCH_CTRL, _priv->gmch_ctrl);
> - pci_write_config_word(pdev, PSB_GMCH_CTRL,
> - dev_priv->gmch_ctrl | _PSB_GMCH_ENABLED);
> -
> - dev_priv->pge_ctl = PSB_RVDC32(PSB_PGETBL_CTL);
> - PSB_WVDC32(dev_priv->pge_ctl | _PSB_PGETBL_ENABLED, PSB_PGETBL_CTL);
> - (void) PSB_RVDC32(PSB_PGETBL_CTL);
> + ret = psb_gtt_enable(dev_priv);
> + if (ret)
> + return ret;
>
> /* The root resource we allocate address
Re: [PATCH v2 12/12] drm/gma500: Move GTT memory-range setup into helper
On Tue, Mar 8, 2022 at 8:52 PM Thomas Zimmermann wrote:
>
> Move the setup code for GTT/GATT memory ranges into a new helper and
> call the function from psb_gtt_init() and psb_gtt_resume(). Removes
> code duplication.
LGTM
Acked-by: Patrik Jakobsson
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/gma500/gtt.c | 153 +++
> 1 file changed, 64 insertions(+), 89 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
> index 83d9a9f7c73c..379bc218aa6b 100644
> --- a/drivers/gpu/drm/gma500/gtt.c
> +++ b/drivers/gpu/drm/gma500/gtt.c
> @@ -182,68 +182,91 @@ static void psb_gtt_clear(struct drm_psb_private *pdev)
> (void)ioread32(pdev->gtt_map + i - 1);
> }
>
> -int psb_gtt_init(struct drm_device *dev)
> +static void psb_gtt_init_ranges(struct drm_psb_private *dev_priv)
> {
> - struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> + struct drm_device *dev = _priv->dev;
> struct pci_dev *pdev = to_pci_dev(dev->dev);
> struct psb_gtt *pg = _priv->gtt;
> - unsigned gtt_pages;
> - int ret;
> -
> - mutex_init(_priv->gtt_mutex);
> -
> - ret = psb_gtt_enable(dev_priv);
> - if (ret)
> - goto err_mutex_destroy;
> + resource_size_t gtt_phys_start, mmu_gatt_start, gtt_start, gtt_pages,
> + gatt_start, gatt_pages;
> + struct resource *gtt_mem;
>
> /* The root resource we allocate address space from */
> - pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
> + gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
>
> /*
> -* The video mmu has a hw bug when accessing 0x0D000.
> -* Make gatt start at 0x0e000,. This doesn't actually
> -* matter for us but may do if the video acceleration ever
> -* gets opened up.
> +* The video MMU has a HW bug when accessing 0x0d000. Make
> +* GATT start at 0x0e000. This doesn't actually matter for
> +* us now, but maybe will if the video acceleration ever gets
> +* opened up.
> */
> - pg->mmu_gatt_start = 0xE000;
> + mmu_gatt_start = 0xe000;
> +
> + gtt_start = pci_resource_start(pdev, PSB_GTT_RESOURCE);
> + gtt_pages = pci_resource_len(pdev, PSB_GTT_RESOURCE) >> PAGE_SHIFT;
>
> - pg->gtt_start = pci_resource_start(pdev, PSB_GTT_RESOURCE);
> - gtt_pages = pci_resource_len(pdev, PSB_GTT_RESOURCE)
> - >> PAGE_SHIFT;
> /* CDV doesn't report this. In which case the system has 64 gtt pages
> */
> - if (pg->gtt_start == 0 || gtt_pages == 0) {
> + if (!gtt_start || !gtt_pages) {
> dev_dbg(dev->dev, "GTT PCI BAR not initialized.\n");
> gtt_pages = 64;
> - pg->gtt_start = dev_priv->pge_ctl;
> + gtt_start = dev_priv->pge_ctl;
> }
>
> - pg->gatt_start = pci_resource_start(pdev, PSB_GATT_RESOURCE);
> - pg->gatt_pages = pci_resource_len(pdev, PSB_GATT_RESOURCE)
> - >> PAGE_SHIFT;
> - dev_priv->gtt_mem = >resource[PSB_GATT_RESOURCE];
> + gatt_start = pci_resource_start(pdev, PSB_GATT_RESOURCE);
> + gatt_pages = pci_resource_len(pdev, PSB_GATT_RESOURCE) >> PAGE_SHIFT;
>
> - if (pg->gatt_pages == 0 || pg->gatt_start == 0) {
> + if (!gatt_pages || !gatt_start) {
> static struct resource fudge; /* Preferably peppermint */
> - /* This can occur on CDV systems. Fudge it in this case.
> - We really don't care what imaginary space is being
> allocated
> - at this point */
> +
> + /*
> +* This can occur on CDV systems. Fudge it in this case. We
> +* really don't care what imaginary space is being allocated
> +* at this point.
> +*/
> dev_dbg(dev->dev, "GATT PCI BAR not initialized.\n");
> - pg->gatt_start = 0x4000;
> - pg->gatt_pages = (128 * 1024 * 1024) >> PAGE_SHIFT;
> - /* This is a little confusing but in fact the GTT is providing
> - a view from the GPU into memory and not vice versa. As such
> - this is really allocating space that is not the same as the
> - CPU address space on CDV */
> + gatt_start = 0x4000;
> + gatt_pages = (128 * 1024 * 1024) >> PAGE_SHIFT;
> +
> + /*
> +* This is a little confusing but in fact the GTT is providing
> +* a view from the GPU into memory and not vice versa. As such
> +* this is really allocating space that is not the same as the
> +* CPU address space on CDV.
> +*/
>
Re: [PATCH v6 3/6] drm: Add driver for Solomon SSD130x OLED displays
On 3/8/22 17:30, Geert Uytterhoeven wrote:
> Hi Javier,
>
> On Mon, Feb 14, 2022 at 2:37 PM Javier Martinez Canillas
> wrote:
>> This adds a DRM driver for SSD1305, SSD1306, SSD1307 and SSD1309 Solomon
>> OLED display controllers.
>>
>> It's only the core part of the driver and a bus specific driver is needed
>> for each transport interface supported by the display controllers.
>>
>> Signed-off-by: Javier Martinez Canillas
>
> Thanks for your patch, which is now commit a61732e808672cfa ("drm:
> Add driver for Solomon SSD130x OLED displays") in drm/drm-next
>
> Sorry for the delay, but finally I gave it a try on my Adafruit
> FeatherWing 128x32 OLED.
> Some of the weird issues (cursor disappears after printing some text,
> more text also doesn't appear until I clear the display) are still there.
I see. Thought that I tested using it as a console and it did work
correctly for me. I'll do more tests again.
> Unfortunately a regression was introduced since your v3: printed
> text is mirrored upside-down. I.e. "E" is rendered correctly, but "L"
> turns into "Γ" (Greek Gamma).
> I suspect something went wrong with the display initialization
> sequence.
>
Could you please try Chen-Yu's fix for the COM scan direction mask ?
https://lists.freedesktop.org/archives/dri-devel/2022-March/345915.html
I made a mistake when converting to use the GENMASK() and FIELD_PREP()
macros in v4 as suggested by Andy. The SSD130X_SET_COM_SCAN_DIR_MASK
wasn't correct which would explain the output to be vertically flipped.
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [PATCH v7 11/24] dt-bindings: display: rockchip: dw-hdmi: Add additional clock
On 2022-02-25 07:51, Sascha Hauer wrote:
The rk3568 HDMI has an additional clock that needs to be enabled for the
HDMI controller to work. The purpose of that clock is not clear. It is
named "hclk" in the downstream driver, so use the same name.
Further to the clarification of the underlying purpose on the other
thread, I suggest we call the new clock "niu" and describe it as
something like "Additional clock required to ungate the bus interface on
certain SoCs".
Cheers,
Robin.
Signed-off-by: Sascha Hauer
Acked-by: Rob Herring
---
Notes:
Changes since v4:
- Add Robs Ack
.../bindings/display/rockchip/rockchip,dw-hdmi.yaml| 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git
a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
index 38ebb69830287..67a76f51637a7 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
@@ -44,12 +44,13 @@ properties:
items:
- {}
- {}
- # The next three clocks are all optional, but shall be specified in this
+ # The next four clocks are all optional, but shall be specified in this
# order when present.
- description: The HDMI CEC controller main clock
- description: Power for GRF IO
- description: External clock for some HDMI PHY (old clock name,
deprecated)
- description: External clock for some HDMI PHY (new name)
+ - description: hclk
clock-names:
minItems: 2
@@ -61,13 +62,17 @@ properties:
- grf
- vpll
- ref
+ - hclk
- enum:
- grf
- vpll
- ref
+ - hclk
- enum:
- vpll
- ref
+ - hclk
+ - const: hclk
ddc-i2c-bus:
$ref: /schemas/types.yaml#/definitions/phandle
Re: [PATCH v7 10/24] drm/rockchip: dw_hdmi: Add support for hclk
On 2022-03-09 08:37, elaine.zhang wrote: hi, 在 2022/3/9 下午4:18, Sascha Hauer 写道: Hi Elaine, On Wed, Mar 09, 2022 at 09:41:39AM +0800, zhangq...@rock-chips.com wrote: hi，all： Let me explain the clock dependency： From the clock tree, pclk_vo0 and hclk_vo0 are completely independent clocks with different parent clocks and different clock frequencies。 But the niu path is : pclk_vo is dependent on hclk_vo, and the pclk_vo niu goes through hclk_vo niu. Thanks, this is the information we are looking for. What is "NIU" btw? I think this is even documented in the Reference Manual. With the right pointer I just found: The NIU (native interface unit) You can see 2.8.6(NIU Clock gating reliance) in TRM. A part of niu clocks have a dependence on another niu clock in order to sharing the internal bus. When these clocks are in use, another niu clock must be opened, and cannot be gated. These clocks and the special clock on which they are relied are as following: Clocks which have dependency The clock which can not be gated - ... pclk_vo_niu, hclk_vo_s_niu hclk_vo_niu ... Yeah, the dependency is this. It may be not very detailed, I don't have permission to publish detailed NIU designs. The clock tree and NIU bus paths are designed independently So there are three solutions to this problem: 1. DTS adds a reference to Hclk while referencing Pclk. 2, The dependent clock is always on, such as HCLK_VO0, but this is not friendly for the system power. 3. Create a non-clock-tree reference. Clk-link, for example, we have an implementation in our internal branch, but Upstream is not sure how to push it. I thought about something similar. That would help us here and on i.MX we have a similar situation: We have one bit that switches multiple clocks. That as well cannot be designed properly in the clock framework currently, but could be modelled with a concept of linked clocks. Doing this sounds like quite a bit of work and discussion though, I don't really like having this as a dependency to mainline the VOP2 driver. I vote for 1. in that case, we could still ignore the hclk in dts later when we have linked clocks. OK so just to clarify, the issue is not just that the upstream clock driver doesn't currently model the NIU clocks, but that even if it did, it would still need to implement some new multi-parent clock type to manage the internal dependency? That's fair enough - I do think that improving the clock driver would be the best long-term goal, but for the scope of this series, having an interim workaround does seem more reasonable now that we understand *why* we need it. Thanks, Robin.
Re: [PATCH v6 2/6] drm/format-helper: Add drm_fb_xrgb8888_to_mono_reversed()
Hello Geert,
Thanks a lot for your feedback.
On 3/8/22 17:13, Geert Uytterhoeven wrote:
[snip]
>> +
>> +static void drm_fb_gray8_to_mono_reversed_line(u8 *dst, const u8 *src,
>> unsigned int pixels,
>
> "pixels" is not the number of pixels to process, but the number of
> bytes in the monochrome destination buffer.
>
Right, that parameter name is misleading / incorrect indeed. Probably
should be changed by dst_pitch or dst_stride.
>> + unsigned int start_offset,
>> unsigned int end_len)
>> +{
>> + unsigned int xb, i;
>> +
>> + for (xb = 0; xb < pixels; xb++) {
>> + unsigned int start = 0, end = 8;
>> + u8 byte = 0x00;
>> +
>> + if (xb == 0 && start_offset)
>> + start = start_offset;
>> +
>> + if (xb == pixels - 1 && end_len)
>> + end = end_len;
>> +
>> + for (i = start; i < end; i++) {
>> + unsigned int x = xb * 8 + i;
>> +
>> + byte >>= 1;
>> + if (src[x] >> 7)
>> + byte |= BIT(7);
>> + }
>> + *dst++ = byte;
>> + }
>
> The above is IMHO very hard to read.
> I think it can be made simpler by passing the total number of pixels
> to process instead of "pixels" (which is bytes) and "end_len".
>
Agreed that's hard to read. I think is better if you propose a patch
with your idea to make it simpler.
[snip]
>> +void drm_fb_xrgb_to_mono_reversed(void *dst, unsigned int dst_pitch,
>> const void *vaddr,
>> + const struct drm_framebuffer *fb,
>> const struct drm_rect *clip)
>> +{
>> + unsigned int linepixels = drm_rect_width(clip);
>> + unsigned int lines = clip->y2 - clip->y1;
>
> drm_rect_height(clip)?
>
Yes, unsure why didn't use it since used drm_rect_width() for the width.
>> + unsigned int cpp = fb->format->cpp[0];
>> + unsigned int len_src32 = linepixels * cpp;
>> + struct drm_device *dev = fb->dev;
>> + unsigned int start_offset, end_len;
>> + unsigned int y;
>> + u8 *mono = dst, *gray8;
>> + u32 *src32;
>> +
>> + if (drm_WARN_ON(dev, fb->format->format != DRM_FORMAT_XRGB))
>> + return;
>> +
>> + /*
>> +* The reversed mono destination buffer contains 1 bit per pixel
>> +* and destination scanlines have to be in multiple of 8 pixels.
>> +*/
>> + if (!dst_pitch)
>> + dst_pitch = DIV_ROUND_UP(linepixels, 8);
>
> This is not correct when clip->x1 is not a multiple of 8 pixels.
> Should be:
>
> DIV_ROUND_UP(linepixels + clip->x1 % 8, 8);
>
Agreed.
>> +
>> + drm_WARN_ONCE(dev, dst_pitch % 8 != 0, "dst_pitch is not a multiple
>> of 8\n");
>
> This triggers for me: dst_pitch = 1.
> Which is perfectly fine, when flashing an 8-pixel wide cursor ;-)
>
Indeed. I think we should just drop that warn.
Do you want me to post patches for all these or would you do it
when simplifying the drm_fb_gray8_to_mono_reversed_line() logic ?
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [PATCH v1 5/5] drm/virtio: Add memory shrinker
On 3/9/22 04:12, Rob Clark wrote:
>> +static unsigned long
>> +virtio_gpu_gem_shrinker_count_objects(struct shrinker *shrinker,
>> + struct shrink_control *sc)
>> +{
>> + struct drm_gem_shmem_object *shmem;
>> + struct virtio_gpu_device *vgdev;
>> + unsigned long count = 0;
>> + bool empty = true;
>> +
>> + vgdev = container_of(shrinker, struct virtio_gpu_device,
>> +vgshrinker.shrinker);
>> +
>> + if (!mutex_trylock(>mm_lock))
>> + return 0;
> One bit of advice from previously dealing with shrinker and heavy
> memory pressure situations (turns out 4GB chromebooks can be pretty
> much under *constant* memory pressure):
>
> You *really* want to make shrinker->count_objects lockless.. and
> minimize the lock contention on shrinker->scan_objects (ie. The
> problem is you can end up with shrinking going on on all CPU cores in
> parallel, you want to not funnel that thru one lock as much as
> possible.
>
> See in particular:
>
> 25ed38b3ed26 ("drm/msm: Drop mm_lock in scan loop")
> cc8a4d5a1bd8 ("drm/msm: Avoid mutex in shrinker_count()")
Thank you, I'll take that into account for v2.
Re: [PATCH v1 0/5] Add memory shrinker to VirtIO-GPU DRM driver
Hello, On 3/9/22 11:59, Thomas Zimmermann wrote: > Hi > > Am 08.03.22 um 14:17 schrieb Dmitry Osipenko: >> Hello, >> >> This patchset introduces memory shrinker for the VirtIO-GPU DRM driver. >> During OOM, the shrinker will release BOs that are marked as "not needed" >> by userspace using the new madvise IOCTL. The userspace in this case is >> the Mesa VirGL driver, it will mark the cached BOs as "not needed", >> allowing kernel driver to release memory of the cached shmem BOs on >> lowmem >> situations, preventing OOM kills. > > Virtio-gpu is build on top of GEM shmem helpers. I have a prototype > patchset that adds a shrinker to these helpers. If you want to go > further, you could implement something like that instead. Panfrost and > lima also have their own shrinker and could certainly be converted to > the gem-shmem shrinker. I had a thought that it could be possible to unify shrinkers into a common DRM framework. Could you please give me a link to yours prototype patchset?
Re: [PATCH v1 06/10] drm/msm/a6xx: Propagate OOB set error
Hi Akhil,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm/drm-next]
[also build test WARNING on drm-intel/for-linux-next drm-tip/drm-tip
drm-exynos/exynos-drm-next v5.17-rc7 next-20220308]
[cannot apply to tegra-drm/drm/tegra/for-next airlied/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Akhil-P-Oommen/Support-for-GMU-coredump-and-some-related-improvements/20220303-013028
base: git://anongit.freedesktop.org/drm/drm drm-next
config: s390-randconfig-m031-20220307
(https://download.01.org/0day-ci/archive/20220309/202203091923.2rd2ech3-...@intel.com/config)
compiler: s390-linux-gcc (GCC) 11.2.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
smatch warnings:
drivers/gpu/drm/msm/adreno/a6xx_gpu.c:894 hw_init() warn: inconsistent indenting
vim +894 drivers/gpu/drm/msm/adreno/a6xx_gpu.c
874
875 #define A6XX_INT_MASK (A6XX_RBBM_INT_0_MASK_CP_AHB_ERROR | \
876A6XX_RBBM_INT_0_MASK_RBBM_ATB_ASYNCFIFO_OVERFLOW | \
877A6XX_RBBM_INT_0_MASK_CP_HW_ERROR | \
878A6XX_RBBM_INT_0_MASK_CP_IB2 | \
879A6XX_RBBM_INT_0_MASK_CP_IB1 | \
880A6XX_RBBM_INT_0_MASK_CP_RB | \
881A6XX_RBBM_INT_0_MASK_CP_CACHE_FLUSH_TS | \
882A6XX_RBBM_INT_0_MASK_RBBM_ATB_BUS_OVERFLOW | \
883A6XX_RBBM_INT_0_MASK_RBBM_HANG_DETECT | \
884A6XX_RBBM_INT_0_MASK_UCHE_OOB_ACCESS | \
885A6XX_RBBM_INT_0_MASK_UCHE_TRAP_INTR)
886
887 static int hw_init(struct msm_gpu *gpu)
888 {
889 struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
890 struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
891 int ret;
892
893 /* Make sure the GMU keeps the GPU on while we set it up */
> 894 ret = a6xx_gmu_set_oob(_gpu->gmu, GMU_OOB_GPU_SET);
895
896 gpu_write(gpu, REG_A6XX_RBBM_SECVID_TSB_CNTL, 0);
897
898 /*
899 * Disable the trusted memory range - we don't actually
supported secure
900 * memory rendering at this point in time and we don't want to
block off
901 * part of the virtual memory space.
902 */
903 gpu_write64(gpu, REG_A6XX_RBBM_SECVID_TSB_TRUSTED_BASE_LO,
904 REG_A6XX_RBBM_SECVID_TSB_TRUSTED_BASE_HI, 0x);
905 gpu_write(gpu, REG_A6XX_RBBM_SECVID_TSB_TRUSTED_SIZE,
0x);
906
907 /* Turn on 64 bit addressing for all blocks */
908 gpu_write(gpu, REG_A6XX_CP_ADDR_MODE_CNTL, 0x1);
909 gpu_write(gpu, REG_A6XX_VSC_ADDR_MODE_CNTL, 0x1);
910 gpu_write(gpu, REG_A6XX_GRAS_ADDR_MODE_CNTL, 0x1);
911 gpu_write(gpu, REG_A6XX_RB_ADDR_MODE_CNTL, 0x1);
912 gpu_write(gpu, REG_A6XX_PC_ADDR_MODE_CNTL, 0x1);
913 gpu_write(gpu, REG_A6XX_HLSQ_ADDR_MODE_CNTL, 0x1);
914 gpu_write(gpu, REG_A6XX_VFD_ADDR_MODE_CNTL, 0x1);
915 gpu_write(gpu, REG_A6XX_VPC_ADDR_MODE_CNTL, 0x1);
916 gpu_write(gpu, REG_A6XX_UCHE_ADDR_MODE_CNTL, 0x1);
917 gpu_write(gpu, REG_A6XX_SP_ADDR_MODE_CNTL, 0x1);
918 gpu_write(gpu, REG_A6XX_TPL1_ADDR_MODE_CNTL, 0x1);
919 gpu_write(gpu, REG_A6XX_RBBM_SECVID_TSB_ADDR_MODE_CNTL, 0x1);
920
921 /* enable hardware clockgating */
922 a6xx_set_hwcg(gpu, true);
923
924 /* VBIF/GBIF start*/
925 if (adreno_is_a640_family(adreno_gpu) ||
926 adreno_is_a650_family(adreno_gpu)) {
927 gpu_write(gpu, REG_A6XX_GBIF_QSB_SIDE0, 0x00071620);
928 gpu_write(gpu, REG_A6XX_GBIF_QSB_SIDE1, 0x00071620);
929 gpu_write(gpu, REG_A6XX_GBIF_QSB_SIDE2, 0x00071620);
930 gpu_write(gpu, REG_A6XX_GBIF_QSB_SIDE3, 0x00071620);
931 gpu_write(gpu, REG_A6XX_GBIF_QSB_SIDE3, 0x00071620);
932 gpu_write(gpu, REG_A6XX_RBBM_GBIF_CLIENT_QOS_CNTL, 0x3);
933 } else {
934 gpu_write(gpu, REG_A6XX_RBBM_VBIF_CLIENT_QOS_CNTL, 0x3);
935 }
936
937 if (adreno_is_a630(adreno_gpu))
938 gpu_write(gpu, REG_A6XX_VBIF_GATE_OFF_WRREQ_EN,
0x0009);
939
940 /* Make all blocks contribute to the GPU BUSY perf counter */
941 gpu_write(gpu, REG_A6XX_RBBM_PERFCTR_GPU_BUSY_MASKED,
0x);
942
943 /* Disable L2 bypass in the UCHE */
944 gpu_write(gpu, REG_A6XX_UCHE_WRITE_RANGE_MAX_LO, 0xffc0);
945 gpu_write(gpu, REG_A6XX_UCHE_WRITE_RANGE_MAX_HI, 0x0001);
Re: [PATCH 9/9] drm/virtio: Implement dumb_create_fbdev with GEM SHMEM helpers
On 3/9/22 09:52, Thomas Zimmermann wrote:
[snip]
>>> +struct drm_gem_object *virtio_gpu_create_object_fbdev(struct drm_device
>>> *dev,
>>> + size_t size)
>>> +{
>>> + return ERR_PTR(-ENOSYS);
>>> +}
>>
>> As mentioned, I believe this should be ERR_PTR(-ENOTSUPP) instead.
>
> I've been wondering about this as well. I finally went with the rules at
> [1]. All the variants of ENOTOP/ENOTSUPP seem to be for specific use
> cases, such as a certain feature is not implemented be a specific
> interface (e.g., sockets for EOPNOTSUPP). ENOSYS is the only general
> error that indicates that an entire interface is missing. Even though
> checkpatch.pl warns that it's only for system calls.
>
> Best regards
> Thomas
>
> [1] https://www.cs.helsinki.fi/linux/linux-kernel/2002-30/1135.html
>
Thanks for the link. It would be good to have an authoritative guideline
about this in the kernel documentation (and make checkpatch.pl aware).
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [PATCH 8/9] drm/gem-shmem: Implement fbdev dumb buffer and mmap helpers
On 3/9/22 09:47, Thomas Zimmermann wrote:
[snip]
>>>
>>> +static const struct drm_gem_object_funcs drm_gem_shmem_funcs_fbdev = {
>>> + .free = drm_gem_shmem_object_free,
>>> + .print_info = drm_gem_shmem_object_print_info,
>>> + .pin = drm_gem_shmem_object_pin,
>>> + .unpin = drm_gem_shmem_object_unpin,
>>> + .get_sg_table = drm_gem_shmem_object_get_sg_table,
>>> + .vmap = drm_gem_shmem_object_vmap,
>>> + .vunmap = drm_gem_shmem_object_vunmap,
>>> + .mmap = drm_gem_shmem_object_mmap_fbdev,
>>> + .vm_ops = _gem_shmem_vm_ops_fbdev,
>>
>> The drm_gem_shmem_funcs_fbdev is the same than drm_gem_shmem_funcs but
>> .mmap and .vm_ops callbacks. Maybe adding a macro to declare these two
>> struct drm_gem_object_funcs could make easier to maintain it long term ?
>
> I see you point, but I'm undecided about this. Such macros also add some
> amount of obfuscation. I'm not sure if it's worth it for an internal
> constant. And since the fbdev buffer is never exported, we could remove
> some of the callbacks. AFAICT we never call pin, unpin or get_sg_table.
>
Yeah, that's true too. It was just a suggestion, I'm OK with patch as is.
> Best regards
> Thomas
>
>>
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [PATCH 3/9] fbdev: Track deferred-I/O pages in pageref struct
Hello Thomas, On 3/9/22 09:36, Thomas Zimmermann wrote: [snip] > > I thought about using pageref->offset in fbdev drivers as well. It would > be more correct, but didn't want to add unnecessary churn. Especially > since I cannot test most of the fbdev drivers. If you think it's worth > it, I'd change the drivers as well. > Thanks for the explanation. No, I don't think is worth it or at least as as part of this series. > Best regards > Thomas > -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH v2 1/2] drm: Add GPU reset sysfs event
On 3/9/2022 8:47 AM, Simon Ser wrote: Hi, Maybe it would be a good idea to state the intended use-case in the commit message? It was added in the second patch, but yeah, it makes more sense to add a cover-letter probably. And explain why the current (driver-specific IIRC) APIs aren't enough? As you mentioned, we also started with a driver specific API, and then realized that most of the drivers can benefit from this infrastructure, as most of them would be more or less interested in the similar information. Since this introduces new uAPI, can you point to a user-space patch which uses the new uAPI? See this link for more info on DRM user-space requirements: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdri.freedesktop.org%2Fdocs%2Fdrm%2Fgpu%2Fdrm-uapi.html%23open-source-userspace-requirementsdata=04%7C01%7Cshashank.sharma%40amd.com%7C85e8e40c84524f3272e908da01a11b1c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637824088716047386%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=%2BMTzBIfl3FGHdAv%2BSx%2F83a86x8eksbOhdQVqdrJ2Rq0%3Dreserved=0 The userspace work is in progress, this was to get a general feedback from the community and the interest. Please use drm_dbg_* and drm_warn_* helpers instead of DRM_DEBUG and DRM_WARN. This allows identifying on which device the uevent happens. Well, I don't want to break the consistency across the file, as other functions seems to be using DRM_DEBUG family of prints. On Tuesday, March 8th, 2022 at 19:04, Shashank Sharma wrote: +void drm_sysfs_reset_event(struct drm_device *dev, struct drm_reset_event *reset_info) reset_info can be const. Sure. - Shashank
Re: [v3,4/5] fbdev: Improve performance of cfb_imageblit()
Hi Marek,
On Wed, Mar 9, 2022 at 10:22 AM Marek Szyprowski
wrote:
> On 09.03.2022 09:22, Thomas Zimmermann wrote:
> > Am 08.03.22 um 23:52 schrieb Marek Szyprowski:
> >> On 23.02.2022 20:38, Thomas Zimmermann wrote:
> >>> Improve the performance of cfb_imageblit() by manually unrolling
> >>> the inner blitting loop and moving some invariants out. The compiler
> >>> failed to do this automatically. This change keeps cfb_imageblit()
> >>> in sync with sys_imagebit().
> >>>
> >>> A microbenchmark measures the average number of CPU cycles
> >>> for cfb_imageblit() after a stabilizing period of a few minutes
> >>> (i7-4790, FullHD, simpledrm, kernel with debugging).
> >>>
> >>> cfb_imageblit(), new: 15724 cycles
> >>> cfb_imageblit(): old: 30566 cycles
> >>>
> >>> In the optimized case, cfb_imageblit() is now ~2x faster than before.
> >>>
> >>> v3:
> >>> * fix commit description (Pekka)
> >>>
> >>> Signed-off-by: Thomas Zimmermann
> >>> Acked-by: Sam Ravnborg
> >>> Reviewed-by: Javier Martinez Canillas
> >> This patch landed recently in linux next-20220308 as commit 0d03011894d2
> >> ("fbdev: Improve performance of cfb_imageblit()"). Sadly it causes a
> >> freeze after DRM and emulated fbdev initialization on various Samsung
> >> Exynos ARM 32bit based boards. This happens when kernel is compiled from
> >> exynos_defconfig. Surprisingly when kernel is compiled from
> >> multi_v7_defconfig all those boards boot fine, so this is a matter of
> >> one of the debugging options enabled in the exynos_defconfig. I will try
> >> to analyze this further and share the results. Reverting $subject on top
> >> of next-20220308 fixes the boot issue.
> >
> > Thanks for reporting. I don't have the hardware to reproduce it and
> > there's no obvious difference to the original version. It's supposed
> > to be the same algorithm with a different implementation. Unless you
> > can figure out the issue, we can also revert the patch easily.
>
> I've played a bit with .config options and found that the issue is
> caused by the compiled-in fonts used for the framebuffer. For some
> reasons (so far unknown to me), exynos_defconfig has the following odd
> setup:
>
> CONFIG_FONT_SUPPORT=y
> CONFIG_FONTS=y
> # CONFIG_FONT_8x8 is not set
> # CONFIG_FONT_8x16 is not set
> # CONFIG_FONT_6x11 is not set
> CONFIG_FONT_7x14=y
> # CONFIG_FONT_PEARL_8x8 is not set
> # CONFIG_FONT_ACORN_8x8 is not set
> # CONFIG_FONT_MINI_4x6 is not set
> # CONFIG_FONT_6x10 is not set
> # CONFIG_FONT_10x18 is not set
> # CONFIG_FONT_SUN8x16 is not set
> # CONFIG_FONT_SUN12x22 is not set
> # CONFIG_FONT_TER16x32 is not set
> # CONFIG_FONT_6x8 is not set
>
> Such setup causes a freeze during framebuffer initialization (or just
> after it got registered). I've reproduced this even on Raspberry Pi 3B
> with multi_v7_defconfig and changed fonts configuration (this also
> required to disable vivid driver, which forces 8x16 font), where I got
> the following panic:
>
> simple-framebuffer 3eace000.framebuffer: framebuffer at 0x3eace000,
> 0x12c000 bytes
> simple-framebuffer 3eace000.framebuffer: format=a8r8g8b8,
> mode=640x480x32, linelength=2560
> 8<--- cut here ---
> Unable to handle kernel paging request at virtual address f0aac000
So support for images with offsets or widths that are not a multiple
of 8 got broken in cfb_imageblit(). Oops...
BTW, the various drawing routines used to set a bitmask indicating
which alignments were supported (see blit_x), but most of them no
longer do, presumably because all alignments are now supported
(since ca. 20 years?).
So you can (temporarily) work around this by filling in blit_x,
preventing the use of the 7x14 font.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 0/6] drm/tiny/st7735r: Match up with staging/fbtft driver
Den 24.11.2021 16.07, skrev Noralf Trønnes: > Hi, > > This patchset adds a missing piece for decommissioning the > staging/fbtft/fb_st7735r.c driver namely a way to configure the > controller from Device Tree. > > All fbtft drivers have builtin support for one display panel and all > other panels using that controller are configured using the Device Tree > 'init' property. This property is supported by all fbtft drivers and > provides a generic way to set register values or issue commands > (depending on the type of controller). > > It is common for these types of displays to have a datasheet listing the > necessary controller settings/commands or some example code doing the > same. > > This is how the panel directly supported by the fb_st7735r staging > driver is described using Device Tree with that driver: > > width = <160>; > height = <128>; > > init = <0x101 > 0x296 > 0x111 > 0x2ff > 0x1B1 0x01 0x2C 0x2D > 0x1B4 0x07 > 0x1C0 0xA2 0x02 0x84 > 0x1C1 0xC5 > 0x1C2 0x0A 0x00 > 0x1C5 0x0E > 0x13a 0x55 > 0x136 0x60 > 0x1E0 0x0F 0x1A 0x0F 0x18 0x2F 0x28 0x20 0x22 > 0x1F 0x1B 0x23 0x37 0x00 0x07 0x02 0x10 > 0x1E1 0x0F 0x1B 0x0F 0x17 0x33 0x2C 0x29 0x2E > 0x30 0x30 0x39 0x3F 0x00 0x07 0x03 0x10 > 0x129 > 0x264>; > > > This is how the same panel is described using the st7735r drm driver and > this patchset: > > width = <160>; > height = <128>; > > frmctr1 = [ 01 2C 2D ]; > invctr = [ 07 ]; > pwctr1 = [ A2 02 84 ]; > pwctr2 = [ C5 ]; > pwctr3 = [ 0A 00 ]; > vmctr1 = [ 0E ]; > madctl = [ 60 ]; > gamctrp1 = [ 0F 1A 0F 18 2F 28 20 22 1F 1B 23 37 00 07 02 10 ]; > gamctrn1 = [ 0F 1B 0F 17 33 2C 29 2E 30 30 39 3F 00 07 03 10 ]; > > > Back when the fbtft drivers were added to staging I asked on the DT > mailinglist if it was OK to use the 'init' property but it was turned > down. In this patchset I'm trying the same approach used by the > solomon,ssd1307fb.yaml binding in describing the attached panel. That > binding prefixes the properties with the vendor name, not sure why that > is and I think it looks strange so I try without it. > > Noralf. > > > Noralf Trønnes (6): > dt-bindings: display: sitronix,st7735r: Fix backlight in example > dt-bindings: display: sitronix,st7735r: Make reset-gpios optional > dt-bindings: display: sitronix,st7735r: Remove spi-max-frequency limit Patches 1-3 applied, thanks for reviewing. The change to the driver has been replaced by a new generic driver panel-mipi-dbi. Noralf. > dt-bindings: display: sitronix,st7735r: Add initialization properties > drm/mipi-dbi: Add device property functions > drm: tiny: st7735r: Support DT initialization of controller > > .../bindings/display/sitronix,st7735r.yaml| 122 ++- > drivers/gpu/drm/drm_mipi_dbi.c| 139 ++ > drivers/gpu/drm/tiny/st7735r.c| 87 +-- > include/drm/drm_mipi_dbi.h| 3 + > 4 files changed, 334 insertions(+), 17 deletions(-) >
Re: [PATCH v2 1/2] drm: Add GPU reset sysfs event
On 09/03/2022 11:24, Christian König wrote: > Am 09.03.22 um 11:10 schrieb Simon Ser: >> On Wednesday, March 9th, 2022 at 10:56, Pierre-Eric Pelloux-Prayer >> wrote: >> >>> Would it be possible to include the app parameters as well? >> Can all processes read sysfs events? > > No, but application parameters are usually not secret. > >> There might be security implications here. The app parameters might >> contain sensitive information, like passwords or tokens. > > It's a well known security vulnerably to provide password etc.. as > application parameter since everybody can read them through procfs. > >> Can't the uevent receiver query the kernel to get that info from the >> PID? > > I'm leaning also towards just providing the PID and not the application name. > The information should be as short as possible. > Indeed you're both right. The PID + reading procfs should be enough. Thanks, Pierre-Eric
Re: [PATCH v2 1/2] drm: Add GPU reset sysfs event
On Wednesday, March 9th, 2022 at 11:24, Christian König wrote: > Am 09.03.22 um 11:10 schrieb Simon Ser: > > On Wednesday, March 9th, 2022 at 10:56, Pierre-Eric Pelloux-Prayer > > wrote: > > > >> Would it be possible to include the app parameters as well? > > Can all processes read sysfs events? > > No, but application parameters are usually not secret. It's a bad practice, yes. But people still do it. > > There might be security implications here. The app parameters might > > contain sensitive information, like passwords or tokens. > > It's a well known security vulnerably to provide password etc.. as > application parameter since everybody can read them through procfs. I was thinking mostly about Flatpak apps here. Flatpak apps are running in a process namespace so they can't query the CLI parameters of PIDs outside of the namespace. But they might still receive sysfs uevents.
Re: [PATCH v2 1/2] drm: Add GPU reset sysfs event
Am 09.03.22 um 11:10 schrieb Simon Ser: On Wednesday, March 9th, 2022 at 10:56, Pierre-Eric Pelloux-Prayer wrote: Would it be possible to include the app parameters as well? Can all processes read sysfs events? No, but application parameters are usually not secret. There might be security implications here. The app parameters might contain sensitive information, like passwords or tokens. It's a well known security vulnerably to provide password etc.. as application parameter since everybody can read them through procfs. Can't the uevent receiver query the kernel to get that info from the PID? I'm leaning also towards just providing the PID and not the application name. The information should be as short as possible. Regards, Christian.
Re: [PATCH v2 1/2] drm: Add GPU reset sysfs event
On Wednesday, March 9th, 2022 at 10:56, Pierre-Eric Pelloux-Prayer wrote: > Would it be possible to include the app parameters as well? Can all processes read sysfs events? There might be security implications here. The app parameters might contain sensitive information, like passwords or tokens. Can't the uevent receiver query the kernel to get that info from the PID?
Re: [PATCH v2 1/2] drm: Add GPU reset sysfs event
Hi Shashank,
On 08/03/2022 19:04, Shashank Sharma wrote:
> From: Shashank Sharma
>
> This patch adds a new sysfs event, which will indicate
> the userland about a GPU reset, and can also provide
> some information like:
> - process ID of the process involved with the GPU reset
> - process name of the involved process
Would it be possible to include the app parameters as well?
piglit has a shader_runner test application that can cause hangs,
and it's run like this:
shader_runner 1.shader_test
Knowing that shader_runner caused the hang is not really useful
without the "1.shader_test" part.
Thanks,
Pierre-Eric
> - the GPU status info (using flags)
>
> This patch also introduces the first flag of the flags
> bitmap, which can be appended as and when required.
>
> V2: Addressed review comments from Christian and Amar
>- move the reset information structure to DRM layer
>- drop _ctx from struct name
>- make pid 32 bit(than 64)
>- set flag when VRAM invalid (than valid)
>- add process name as well (Amar)
>
> Cc: Alexandar Deucher
> Cc: Christian Koenig
> Cc: Amaranath Somalapuram
> Signed-off-by: Shashank Sharma
> ---
> drivers/gpu/drm/drm_sysfs.c | 31 +++
> include/drm/drm_sysfs.h | 10 ++
> 2 files changed, 41 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index 430e00b16eec..840994810910 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -409,6 +409,37 @@ void drm_sysfs_hotplug_event(struct drm_device *dev)
> }
> EXPORT_SYMBOL(drm_sysfs_hotplug_event);
>
> +/**
> + * drm_sysfs_reset_event - generate a DRM uevent to indicate GPU reset
> + * @dev: DRM device
> + * @reset_info: The contextual information about the reset (like PID, flags)
> + *
> + * Send a uevent for the DRM device specified by @dev. This informs
> + * user that a GPU reset has occurred, so that an interested client
> + * can take any recovery or profiling measure.
> + */
> +void drm_sysfs_reset_event(struct drm_device *dev, struct drm_reset_event
> *reset_info)
> +{
> + unsigned char pid_str[13];
> + unsigned char flags_str[15];
> + unsigned char pname_str[TASK_COMM_LEN + 6];
> + unsigned char reset_str[] = "RESET=1";
> + char *envp[] = { reset_str, pid_str, pname_str, flags_str, NULL };
> +
> + if (!reset_info) {
> + DRM_WARN("No reset info, not sending the event\n");
> + return;
> + }
> +
> + DRM_DEBUG("generating reset event\n");
> +
> + snprintf(pid_str, ARRAY_SIZE(pid_str), "PID=%u", reset_info->pid);
> + snprintf(pname_str, ARRAY_SIZE(pname_str), "NAME=%s",
> reset_info->pname);
> + snprintf(flags_str, ARRAY_SIZE(flags_str), "FLAGS=%u",
> reset_info->flags);
> + kobject_uevent_env(>primary->kdev->kobj, KOBJ_CHANGE, envp);
> +}
> +EXPORT_SYMBOL(drm_sysfs_reset_event);
> +
> /**
> * drm_sysfs_connector_hotplug_event - generate a DRM uevent for any
> connector
> * change
> diff --git a/include/drm/drm_sysfs.h b/include/drm/drm_sysfs.h
> index 6273cac44e47..5ba11c760619 100644
> --- a/include/drm/drm_sysfs.h
> +++ b/include/drm/drm_sysfs.h
> @@ -1,16 +1,26 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> #ifndef _DRM_SYSFS_H_
> #define _DRM_SYSFS_H_
> +#include
> +
> +#define DRM_GPU_RESET_FLAG_VRAM_INVALID (1 << 0)
>
> struct drm_device;
> struct device;
> struct drm_connector;
> struct drm_property;
>
> +struct drm_reset_event {
> + uint32_t pid;
> + uint32_t flags;
> + char pname[TASK_COMM_LEN];
> +};
> +
> int drm_class_device_register(struct device *dev);
> void drm_class_device_unregister(struct device *dev);
>
> void drm_sysfs_hotplug_event(struct drm_device *dev);
> +void drm_sysfs_reset_event(struct drm_device *dev, struct drm_reset_event
> *reset_info);
> void drm_sysfs_connector_hotplug_event(struct drm_connector *connector);
> void drm_sysfs_connector_status_event(struct drm_connector *connector,
> struct drm_property *property);
>
Re: [PATCH] drm/i915/dpll: make read-only array div1_vals static const
On Mon, 07 Mar 2022, Colin Ian King wrote:
> Don't populate the read-only array div1_vals on the stack but
> instead make it static const. Also makes the object code a little
> smaller.
Thanks, but this was just fixed in commit fe70b262e781 ("drm/i915: Move
a bunch of stuff into rodata from the stack").
BR,
Jani.
>
> Signed-off-by: Colin Ian King
> ---
> drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index 569903d47aea..17668b58b30c 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -2759,7 +2759,7 @@ static bool icl_mg_pll_find_divisors(int clock_khz,
> bool is_dp, bool use_ssc,
>bool is_dkl)
> {
> u32 dco_min_freq, dco_max_freq;
> - int div1_vals[] = {7, 5, 3, 2};
> + static const int div1_vals[] = {7, 5, 3, 2};
> unsigned int i;
> int div2;
--
Jani Nikula, Intel Open Source Graphics Center
