Re: [PATCH v5 9/9] drm: Introduce documentation for hotspot properties
On Thursday, July 20th, 2023 at 07:03, Zack Rusin wrote: > I'll give this series a few more hours on the list and if no one objects I'll > push > it to drm-misc later today. Thanks! Sorry, but this doesn't seem to be enough to satisfy the DRM merge requirements. This introduces a new uAPI but is missing user-space patches and IGT. See [1] and [2]. [1]: https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#requirements [2]: https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements
Re: [PATCH v2 1/2] drm: bridge: samsung-dsim: Fix i.MX8M enable flow to meet spec
On 19.07.23 18:34, Tim Harvey wrote:
> On Wed, Jul 19, 2023 at 12:05 AM Frieder Schrempf
> wrote:
>>
>> Hi Tim,
>>
>> On 19.07.23 01:03, Tim Harvey wrote:
>>> On Thu, Jul 13, 2023 at 3:01 AM Frieder Schrempf
>>> wrote:
Hi Tim,
On 13.07.23 09:18, Frieder Schrempf wrote:
> Hi Tim,
>
> On 13.07.23 00:34, Tim Harvey wrote:
>> On Wed, May 3, 2023 at 9:33 AM Frieder Schrempf wrote:
>>>
>>> From: Frieder Schrempf
>>>
>>> According to the documentation [1] the proper enable flow is:
>>>
>>> 1. Enable DSI link and keep data lanes in LP-11 (stop state)
>>> 2. Disable stop state to bring data lanes into HS mode
>>>
>>> Currently we do this all at once within enable(), which doesn't
>>> allow to meet the requirements of some downstream bridges.
>>>
>>> To fix this we now enable the DSI in pre_enable() and force it
>>> into stop state using the FORCE_STOP_STATE bit in the ESCMODE
>>> register until enable() is called where we reset the bit.
>>>
>>> We currently do this only for i.MX8M as Exynos uses a different
>>> init flow where samsung_dsim_init() is called from
>>> samsung_dsim_host_transfer().
>>>
>>> [1]
>>> https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation
>>>
>>> Signed-off-by: Frieder Schrempf
>>> ---
>>> Changes for v2:
>>> * Drop RFC
>>> ---
>>> drivers/gpu/drm/bridge/samsung-dsim.c | 25 +++--
>>> 1 file changed, 23 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
>>> b/drivers/gpu/drm/bridge/samsung-dsim.c
>>> index e0a402a85787..9775779721d9 100644
>>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
>>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
>>> @@ -859,6 +859,10 @@ static int samsung_dsim_init_link(struct
>>> samsung_dsim *dsi)
>>> reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
>>> reg &= ~DSIM_STOP_STATE_CNT_MASK;
>>> reg |=
>>> DSIM_STOP_STATE_CNT(driver_data->reg_values[STOP_STATE_CNT]);
>>> +
>>> + if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
>>> + reg |= DSIM_FORCE_STOP_STATE;
>>> +
>>> samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
>>>
>>> reg = DSIM_BTA_TIMEOUT(0xff) | DSIM_LPDR_TIMEOUT(0x);
>>> @@ -1340,6 +1344,9 @@ static void samsung_dsim_atomic_pre_enable(struct
>>> drm_bridge *bridge,
>>> ret = samsung_dsim_init(dsi);
>>> if (ret)
>>> return;
>>> +
>>> + samsung_dsim_set_display_mode(dsi);
>>> + samsung_dsim_set_display_enable(dsi, true);
>>> }
>>> }
>>>
>>> @@ -1347,9 +1354,16 @@ static void samsung_dsim_atomic_enable(struct
>>> drm_bridge *bridge,
>>>struct drm_bridge_state
>>> *old_bridge_state)
>>> {
>>> struct samsung_dsim *dsi = bridge_to_dsi(bridge);
>>> + u32 reg;
>>>
>>> - samsung_dsim_set_display_mode(dsi);
>>> - samsung_dsim_set_display_enable(dsi, true);
>>> + if (samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
>>> + samsung_dsim_set_display_mode(dsi);
>>> + samsung_dsim_set_display_enable(dsi, true);
>>> + } else {
>>> + reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
>>> + reg &= ~DSIM_FORCE_STOP_STATE;
>>> + samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
>>> + }
>>>
>>> dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
>>> }
>>> @@ -1358,10 +1372,17 @@ static void samsung_dsim_atomic_disable(struct
>>> drm_bridge *bridge,
>>> struct drm_bridge_state
>>> *old_bridge_state)
>>> {
>>> struct samsung_dsim *dsi = bridge_to_dsi(bridge);
>>> + u32 reg;
>>>
>>> if (!(dsi->state & DSIM_STATE_ENABLED))
>>> return;
>>>
>>> + if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
>>> + reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
>>> + reg |= DSIM_FORCE_STOP_STATE;
>>> + samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
>>> + }
>>> +
>>> dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
>>> }
>>>
>>> --
>>> 2.40.0
>>>
>>
>> Hi Frieder,
>>
>> I found this patch to break mipi-dsi display on my board which has:
>> - FocalTech FT5406 10pt touch controller (with no interrupt)
>> - Powertip PH800480T013-IDF02 compatible panel
>> - Toshiba TC358762 compatible DSI to DBI bridge
>> - ATTINY based regulator used for backlight controller and panel enable
>
[drm-misc:for-linux-next 2/2] drivers/gpu/drm/drm_debugfs.c:212:28: warning: cast from pointer to integer of different size
tree: git://anongit.freedesktop.org/drm/drm-misc for-linux-next
head: 4f66feeab173bd73e71028b8c2e1dcea07e32dd5
commit: 4f66feeab173bd73e71028b8c2e1dcea07e32dd5 [2/2] drm: debugfs: provide
infrastructure to dump a DRM GPU VA space
config: m68k-allyesconfig
(https://download.01.org/0day-ci/archive/20230720/202307201445.dc5d5leo-...@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.3.0
reproduce:
(https://download.01.org/0day-ci/archive/20230720/202307201445.dc5d5leo-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202307201445.dc5d5leo-...@intel.com/
All warnings (new ones prefixed by >>):
drivers/gpu/drm/drm_debugfs.c: In function 'drm_debugfs_gpuva_info':
>> drivers/gpu/drm/drm_debugfs.c:212:28: warning: cast from pointer to integer
>> of different size [-Wpointer-to-int-cast]
212 |(u64)va->gem.obj, va->gem.offset);
|^
vim +212 drivers/gpu/drm/drm_debugfs.c
178
179 /**
180 * drm_debugfs_gpuva_info - dump the given DRM GPU VA space
181 * @m: pointer to the &seq_file to write
182 * @mgr: the &drm_gpuva_manager representing the GPU VA space
183 *
184 * Dumps the GPU VA mappings of a given DRM GPU VA manager.
185 *
186 * For each DRM GPU VA space drivers should call this function from
their
187 * &drm_info_list's show callback.
188 *
189 * Returns: 0 on success, -ENODEV if the &mgr is not initialized
190 */
191 int drm_debugfs_gpuva_info(struct seq_file *m,
192 struct drm_gpuva_manager *mgr)
193 {
194 struct drm_gpuva *va, *kva = &mgr->kernel_alloc_node;
195
196 if (!mgr->name)
197 return -ENODEV;
198
199 seq_printf(m, "DRM GPU VA space (%s) [0x%016llx;0x%016llx]\n",
200 mgr->name, mgr->mm_start, mgr->mm_start +
mgr->mm_range);
201 seq_printf(m, "Kernel reserved node [0x%016llx;0x%016llx]\n",
202 kva->va.addr, kva->va.addr + kva->va.range);
203 seq_puts(m, "\n");
204 seq_puts(m, " VAs | start | range |
end| object | object offset\n");
205 seq_puts(m,
"-\n");
206 drm_gpuva_for_each_va(va, mgr) {
207 if (unlikely(va == kva))
208 continue;
209
210 seq_printf(m, " | 0x%016llx | 0x%016llx | 0x%016llx
| 0x%016llx | 0x%016llx\n",
211 va->va.addr, va->va.range, va->va.addr +
va->va.range,
> 212 (u64)va->gem.obj, va->gem.offset);
213 }
214
215 return 0;
216 }
217 EXPORT_SYMBOL(drm_debugfs_gpuva_info);
218
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
[drm-misc:for-linux-next 1/2] drivers/gpu/drm/drm_gpuva_mgr.c:622:48: warning: format specifies type 'unsigned long' but the argument has type 'unsigned int'
tree: git://anongit.freedesktop.org/drm/drm-misc for-linux-next
head: 4f66feeab173bd73e71028b8c2e1dcea07e32dd5
commit: e6303f323b1ad9c02ae813fc3dedeaa9dadfd3b0 [1/2] drm: manager to keep
track of GPUs VA mappings
config: hexagon-randconfig-r004-20230720
(https://download.01.org/0day-ci/archive/20230720/202307201438.qq76qj5x-...@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git
4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce:
(https://download.01.org/0day-ci/archive/20230720/202307201438.qq76qj5x-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202307201438.qq76qj5x-...@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/drm_gpuva_mgr.c:622:48: warning: format specifies type
>> 'unsigned long' but the argument has type 'unsigned int' [-Wformat]
622 | "GPUVA address limited to %lu bytes.\n",
sizeof(end));
| ~~~
^~~
| %u
include/asm-generic/bug.h:133:29: note: expanded from macro 'WARN'
133 | __WARN_printf(TAINT_WARN, format);
\
| ^~
include/asm-generic/bug.h:97:48: note: expanded from macro '__WARN_printf'
97 | warn_slowpath_fmt(__FILE__, __LINE__, taint, arg);
\
| ^~~
drivers/gpu/drm/drm_gpuva_mgr.c:1079:32: warning: variable 'prev' set but
not used [-Wunused-but-set-variable]
1079 | struct drm_gpuva *va, *next, *prev = NULL;
| ^
2 warnings generated.
vim +622 drivers/gpu/drm/drm_gpuva_mgr.c
615
616 static bool
617 drm_gpuva_check_overflow(u64 addr, u64 range)
618 {
619 u64 end;
620
621 return WARN(check_add_overflow(addr, range, &end),
> 622 "GPUVA address limited to %lu bytes.\n",
> sizeof(end));
623 }
624
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v2] fb: Explicitly include correct DT includes
On 7/18/23 16:32, Rob Herring wrote: The DT of_device.h and of_platform.h date back to the separate of_platform_bus_type before it as merged into the regular platform bus. As part of that merge prepping Arm DT support 13 years ago, they "temporarily" include each other. They also include platform_device.h and of.h. As a result, there's a pretty much random mix of those include files used throughout the tree. In order to detangle these headers and replace the implicit includes with struct declarations, users need to explicitly include the correct includes. Reviewed-by: Thomas Zimmermann Signed-off-by: Rob Herring applied to fbdev git tree. Thanks! Helge --- v2: - Drop whitespace changes in sbuslib.c --- drivers/video/fbdev/bw2.c| 3 ++- drivers/video/fbdev/cg14.c | 3 ++- drivers/video/fbdev/cg3.c| 3 ++- drivers/video/fbdev/cg6.c| 3 ++- drivers/video/fbdev/ffb.c| 3 ++- drivers/video/fbdev/grvga.c | 3 +-- drivers/video/fbdev/leo.c| 3 ++- drivers/video/fbdev/mb862xx/mb862xxfb_accel.c| 4 +--- drivers/video/fbdev/mb862xx/mb862xxfbdrv.c | 6 +++--- drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c | 2 +- drivers/video/fbdev/p9100.c | 3 ++- drivers/video/fbdev/platinumfb.c | 4 ++-- drivers/video/fbdev/sbuslib.c| 2 +- drivers/video/fbdev/sunxvr1000.c | 3 ++- drivers/video/fbdev/sunxvr2500.c | 2 +- drivers/video/fbdev/sunxvr500.c | 2 +- drivers/video/fbdev/tcx.c| 3 ++- drivers/video/fbdev/xilinxfb.c | 5 ++--- 18 files changed, 31 insertions(+), 26 deletions(-) diff --git a/drivers/video/fbdev/bw2.c b/drivers/video/fbdev/bw2.c index 025d663dc6fd..39f438de0d6b 100644 --- a/drivers/video/fbdev/bw2.c +++ b/drivers/video/fbdev/bw2.c @@ -17,7 +17,8 @@ #include #include #include -#include +#include +#include #include #include diff --git a/drivers/video/fbdev/cg14.c b/drivers/video/fbdev/cg14.c index 832a82f45c80..90fdc9d9bf5a 100644 --- a/drivers/video/fbdev/cg14.c +++ b/drivers/video/fbdev/cg14.c @@ -17,7 +17,8 @@ #include #include #include -#include +#include +#include #include #include diff --git a/drivers/video/fbdev/cg3.c b/drivers/video/fbdev/cg3.c index 6335cd364c74..98c60f72046a 100644 --- a/drivers/video/fbdev/cg3.c +++ b/drivers/video/fbdev/cg3.c @@ -17,7 +17,8 @@ #include #include #include -#include +#include +#include #include #include diff --git a/drivers/video/fbdev/cg6.c b/drivers/video/fbdev/cg6.c index 6884572efea1..6427b85f1a94 100644 --- a/drivers/video/fbdev/cg6.c +++ b/drivers/video/fbdev/cg6.c @@ -17,7 +17,8 @@ #include #include #include -#include +#include +#include #include #include diff --git a/drivers/video/fbdev/ffb.c b/drivers/video/fbdev/ffb.c index c6d3111dcbb0..c473841eb6ff 100644 --- a/drivers/video/fbdev/ffb.c +++ b/drivers/video/fbdev/ffb.c @@ -16,7 +16,8 @@ #include #include #include -#include +#include +#include #include #include diff --git a/drivers/video/fbdev/grvga.c b/drivers/video/fbdev/grvga.c index 9aa15be29ea9..d4a9a58b3691 100644 --- a/drivers/video/fbdev/grvga.c +++ b/drivers/video/fbdev/grvga.c @@ -12,8 +12,7 @@ #include #include -#include -#include +#include #include #include #include diff --git a/drivers/video/fbdev/leo.c b/drivers/video/fbdev/leo.c index 3ffc0a725f89..89ca48235dbe 100644 --- a/drivers/video/fbdev/leo.c +++ b/drivers/video/fbdev/leo.c @@ -16,8 +16,9 @@ #include #include #include -#include #include +#include +#include #include diff --git a/drivers/video/fbdev/mb862xx/mb862xxfb_accel.c b/drivers/video/fbdev/mb862xx/mb862xxfb_accel.c index 61aed7fc0b8d..c35a7479fbf2 100644 --- a/drivers/video/fbdev/mb862xx/mb862xxfb_accel.c +++ b/drivers/video/fbdev/mb862xx/mb862xxfb_accel.c @@ -15,9 +15,7 @@ #include #include #include -#if defined(CONFIG_OF) -#include -#endif + #include "mb862xxfb.h" #include "mb862xx_reg.h" #include "mb862xxfb_accel.h" diff --git a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c index b5c8fcab9940..9dc347d163cf 100644 --- a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c +++ b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c @@ -18,11 +18,11 @@ #include #include #include -#if defined(CONFIG_OF) +#include #include #include -#include -#endif +#include + #include "mb862xxfb.h" #include "mb862xx_reg.h" diff --git a/drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c b/drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c index ba94a0a
Re: [PATCH 1/7] vgacon: switch vgacon_scrolldelta() and vgacon_restore_screen()
On 7/12/23 10:59, Jiri Slaby (SUSE) wrote:
Switch vgacon_scrolldelta() and vgacon_restore_screen() positions, so
that the former is not needed to be forward-declared.
Signed-off-by: Jiri Slaby (SUSE)
Cc: Helge Deller
Cc: linux-fb...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Thanks Jiri !
I've applied this series to the fbdev git tree.
Helge
---
drivers/video/console/vgacon.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index e25ba523892e..fbed2862c317 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -142,12 +142,6 @@ static inline void vga_set_mem_top(struct vc_data *c)
write_vga(12, (c->vc_visible_origin - vga_vram_base) / 2);
}
-static void vgacon_restore_screen(struct vc_data *c)
-{
- if (c->vc_origin != c->vc_visible_origin)
- vgacon_scrolldelta(c, 0);
-}
-
static void vgacon_scrolldelta(struct vc_data *c, int lines)
{
vc_scrolldelta_helper(c, lines, vga_rolled_over, (void *)vga_vram_base,
@@ -155,6 +149,12 @@ static void vgacon_scrolldelta(struct vc_data *c, int
lines)
vga_set_mem_top(c);
}
+static void vgacon_restore_screen(struct vc_data *c)
+{
+ if (c->vc_origin != c->vc_visible_origin)
+ vgacon_scrolldelta(c, 0);
+}
+
static const char *vgacon_startup(void)
{
const char *display_desc = NULL;
[PATCH] backlight: gpio_backlight: Drop output gpio direction check for initial power state
Bootloader may leave gpio direction as input and gpio value as logical low. It hints that initial backlight power state should be FB_BLANK_POWERDOWN since the gpio value is literally logical low. So, let's drop output gpio direction check and only check gpio value to set the initial power state. Signed-off-by: Liu Ying --- drivers/video/backlight/gpio_backlight.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index d3bea42407f1..d28c30b2a35d 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -87,8 +87,7 @@ static int gpio_backlight_probe(struct platform_device *pdev) /* Not booted with device tree or no phandle link to the node */ bl->props.power = def_value ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN; - else if (gpiod_get_direction(gbl->gpiod) == 0 && -gpiod_get_value_cansleep(gbl->gpiod) == 0) + else if (gpiod_get_value_cansleep(gbl->gpiod) == 0) bl->props.power = FB_BLANK_POWERDOWN; else bl->props.power = FB_BLANK_UNBLANK; -- 2.37.1
Re: [PATCH][next][V2] video: fbdev: kyro: make some const read-only arrays static and reduce type size
On 7/12/23 18:11, Colin Ian King wrote:
Don't populate the const read-only arrays on the stack but instead
make them static const. Use smaller types to use less storage for
the arrays. Also makes the object code a little smaller.
Signed-off-by: Colin Ian King
applied.
Thanks!
Helge
---
V2: Use smaller int types, kudos to Helge Deller for suggesting this
---
drivers/video/fbdev/kyro/STG4000InitDevice.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/video/fbdev/kyro/STG4000InitDevice.c
b/drivers/video/fbdev/kyro/STG4000InitDevice.c
index edfa0a04854d..79886a246638 100644
--- a/drivers/video/fbdev/kyro/STG4000InitDevice.c
+++ b/drivers/video/fbdev/kyro/STG4000InitDevice.c
@@ -83,11 +83,11 @@ volatile u32 i,count=0; \
static u32 InitSDRAMRegisters(volatile STG4000REG __iomem *pSTGReg,
u32 dwSubSysID, u32 dwRevID)
{
- u32 adwSDRAMArgCfg0[] = { 0xa0, 0x80, 0xa0, 0xa0, 0xa0 };
- u32 adwSDRAMCfg1[] = { 0x8732, 0x8732, 0xa732, 0xa732, 0x8732 };
- u32 adwSDRAMCfg2[] = { 0x87d2, 0x87d2, 0xa7d2, 0x87d2, 0xa7d2 };
- u32 adwSDRAMRsh[] = { 36, 39, 40 };
- u32 adwChipSpeed[] = { 110, 120, 125 };
+ static const u8 adwSDRAMArgCfg0[] = { 0xa0, 0x80, 0xa0, 0xa0, 0xa0 };
+ static const u16 adwSDRAMCfg1[] = { 0x8732, 0x8732, 0xa732, 0xa732,
0x8732 };
+ static const u16 adwSDRAMCfg2[] = { 0x87d2, 0x87d2, 0xa7d2, 0x87d2,
0xa7d2 };
+ static const u8 adwSDRAMRsh[] = { 36, 39, 40 };
+ static const u8 adwChipSpeed[] = { 110, 120, 125 };
u32 dwMemTypeIdx;
u32 dwChipSpeedIdx;
Re: [PATCH v5 9/9] drm: Introduce documentation for hotspot properties
On Wed, 2023-07-19 at 11:15 +0300, Pekka Paalanen wrote: > On Tue, 18 Jul 2023 21:42:18 -0400 > Zack Rusin wrote: > > > From: Michael Banack > > > > To clarify the intent and reasoning behind the hotspot properties > > introduce userspace documentation that goes over cursor handling > > in para-virtualized environments. > > > > The documentation is generic enough to not special case for any > > specific hypervisor and should apply equally to all. > > > > Signed-off-by: Zack Rusin > > --- > > Documentation/gpu/drm-kms.rst | 6 > > drivers/gpu/drm/drm_plane.c | 58 ++- > > 2 files changed, 63 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst > > index c92d425cb2dd..7159b3e90a8a 100644 > > --- a/Documentation/gpu/drm-kms.rst > > +++ b/Documentation/gpu/drm-kms.rst > > @@ -577,6 +577,12 @@ Variable Refresh Properties > > .. kernel-doc:: drivers/gpu/drm/drm_connector.c > > :doc: Variable refresh properties > > > > +Cursor Hotspot Properties > > +--- > > + > > +.. kernel-doc:: drivers/gpu/drm/drm_plane.c > > + :doc: hotspot properties > > + > > Existing KMS Properties > > --- > > > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > > index 1dc00ad4c33c..f3f2eae83cca 100644 > > --- a/drivers/gpu/drm/drm_plane.c > > +++ b/drivers/gpu/drm/drm_plane.c > > @@ -230,6 +230,61 @@ static int create_in_format_blob(struct drm_device > > *dev, > > struct drm_plane *plane > > return 0; > > } > > > > +/** > > + * DOC: hotspot properties > > + * > > + * HOTSPOT_X: property to set mouse hotspot x offset. > > + * HOTSPOT_Y: property to set mouse hotspot y offset. > > + * > > + * When the plane is being used as a cursor image to display a mouse > > pointer, > > + * the "hotspot" is the offset within the cursor image where mouse events > > + * are expected to go. > > + * > > + * Positive values move the hotspot from the top-left corner of the cursor > > + * plane towards the right and bottom. > > + * > > + * Most display drivers do not need this information because the > > + * hotspot is not actually connected to anything visible on screen. > > + * However, this is necessary for display drivers like the para-virtualized > > + * drivers (eg qxl, vbox, virtio, vmwgfx), that are attached to a user > > console > > + * with a mouse pointer. Since these consoles are often being remoted > > over a > > + * network, they would otherwise have to wait to display the pointer > > movement > > to > > + * the user until a full network round-trip has occurred. New mouse events > > have > > + * to be sent from the user's console, over the network to the virtual > > input > > + * devices, forwarded to the desktop for processing, and then the cursor > > plane's > > + * position can be updated and sent back to the user's console over the > > network. > > + * Instead, with the hotspot information, the console can anticipate the > > new > > + * location, and draw the mouse cursor there before the confirmation comes > > in. > > + * To do that correctly, the user's console must be able predict how the > > + * desktop will process mouse events, which normally requires the desktop's > > + * mouse topology information, ie where each CRTC sits in the mouse > > coordinate > > + * space. This is typically sent to the para-virtualized drivers using > > some > > + * driver-specific method, and the driver then forwards it to the console > > by > > + * way of the virtual display device or hypervisor. > > + * > > + * The assumption is generally made that there is only one cursor plane > > being > > + * used this way at a time, and that the desktop is feeding all mouse > > devices > > + * into the same global pointer. Para-virtualized drivers that require > > this > > + * should only be exposing a single cursor plane, or find some other way > > + * to coordinate with a userspace desktop that supports multiple pointers. > > + * If the hotspot properties are set, the cursor plane is therefore > > assumed to > > be > > + * used only for displaying a mouse cursor image, and the position of the > > combined > > + * cursor plane + offset can therefore be used for coordinating with input > > from > > a > > + * mouse device. > > + * > > + * The cursor will then be drawn either at the location of the plane in the > > CRTC > > + * console, or as a free-floating cursor plane on the user's console > > + * corresponding to their desktop mouse position. > > + * > > + * DRM clients which would like to work correctly on drivers which expose > > + * hotspot properties should advertise DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT. > > + * Setting this property on drivers which do not special case > > + * cursor planes will return EOPNOTSUPP, which can be used by userspace to > > + * gauge requirements of the hardware/drivers they're running on. > > Advertising > > + * DRM_
[PATCH] vt: remove spaces after '*'
remove redundant spaces to clear checkpatch errors.
ERROR: "foo * bar" should be "foo *bar"
Signed-off-by: Ran Sun
---
include/linux/kbd_kern.h | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/linux/kbd_kern.h b/include/linux/kbd_kern.h
index c40811d79769..6cb4ab685a84 100644
--- a/include/linux/kbd_kern.h
+++ b/include/linux/kbd_kern.h
@@ -69,52 +69,52 @@ extern void (*kbd_ledfunc)(unsigned int led);
extern int set_console(int nr);
extern void schedule_console_callback(void);
-static inline int vc_kbd_mode(struct kbd_struct * kbd, int flag)
+static inline int vc_kbd_mode(struct kbd_struct *kbd, int flag)
{
return ((kbd->modeflags >> flag) & 1);
}
-static inline int vc_kbd_led(struct kbd_struct * kbd, int flag)
+static inline int vc_kbd_led(struct kbd_struct *kbd, int flag)
{
return ((kbd->ledflagstate >> flag) & 1);
}
-static inline void set_vc_kbd_mode(struct kbd_struct * kbd, int flag)
+static inline void set_vc_kbd_mode(struct kbd_struct *kbd, int flag)
{
kbd->modeflags |= 1 << flag;
}
-static inline void set_vc_kbd_led(struct kbd_struct * kbd, int flag)
+static inline void set_vc_kbd_led(struct kbd_struct *kbd, int flag)
{
kbd->ledflagstate |= 1 << flag;
}
-static inline void clr_vc_kbd_mode(struct kbd_struct * kbd, int flag)
+static inline void clr_vc_kbd_mode(struct kbd_struct *kbd, int flag)
{
kbd->modeflags &= ~(1 << flag);
}
-static inline void clr_vc_kbd_led(struct kbd_struct * kbd, int flag)
+static inline void clr_vc_kbd_led(struct kbd_struct *kbd, int flag)
{
kbd->ledflagstate &= ~(1 << flag);
}
-static inline void chg_vc_kbd_lock(struct kbd_struct * kbd, int flag)
+static inline void chg_vc_kbd_lock(struct kbd_struct *kbd, int flag)
{
kbd->lockstate ^= 1 << flag;
}
-static inline void chg_vc_kbd_slock(struct kbd_struct * kbd, int flag)
+static inline void chg_vc_kbd_slock(struct kbd_struct *kbd, int flag)
{
kbd->slockstate ^= 1 << flag;
}
-static inline void chg_vc_kbd_mode(struct kbd_struct * kbd, int flag)
+static inline void chg_vc_kbd_mode(struct kbd_struct *kbd, int flag)
{
kbd->modeflags ^= 1 << flag;
}
-static inline void chg_vc_kbd_led(struct kbd_struct * kbd, int flag)
+static inline void chg_vc_kbd_led(struct kbd_struct *kbd, int flag)
{
kbd->ledflagstate ^= 1 << flag;
}
[PATCH v2 2/2] drm: Replace drm_framebuffer plane size functions with its equivalents
The functions drm_framebuffer_plane_{width,height} and
fb_plane_{width,height} do exactly the same job of its
equivalents drm_format_info_plane_{width,height} from drm_fourcc.
The only reason to have these functions on drm_framebuffer
would be if they would added a abstraction layer to call it just
passing a drm_framebuffer pointer and the desired plane index,
which is not the case, where these functions actually implements
just part of it. In the actual implementation, every call to both
drm_framebuffer_plane_{width,height} and fb_plane_{width,height} should
pass some drm_framebuffer attribute, which is the same as calling the
drm_format_info_plane_{width,height} functions.
The drm_format_info_pane_{width,height} functions are much more
consistent in both its implementation and its location on code. The
kind of calculation that they do is intrinsically derivated from the
drm_format_info struct and has not to do with drm_framebuffer, except
by the potential motivation described above, which is still not a good
justification to have drm_framebuffer functions to calculate it.
So, replace each drm_framebuffer_plane_{width,height} and
fb_plane_{width,height} call to drm_format_info_plane_{width,height}
and remove them.
Signed-off-by: Carlos Eduardo Gallo Filho
---
drivers/gpu/drm/drm_framebuffer.c | 64 ++---
drivers/gpu/drm/i915/display/intel_fb.c | 2 +-
include/drm/drm_framebuffer.h | 5 --
3 files changed, 5 insertions(+), 66 deletions(-)
diff --git a/drivers/gpu/drm/drm_framebuffer.c
b/drivers/gpu/drm/drm_framebuffer.c
index aff3746dedfb..efed4cd7965e 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -151,24 +151,6 @@ int drm_mode_addfb_ioctl(struct drm_device *dev,
return drm_mode_addfb(dev, data, file_priv);
}
-static int fb_plane_width(int width,
- const struct drm_format_info *format, int plane)
-{
- if (plane == 0)
- return width;
-
- return DIV_ROUND_UP(width, format->hsub);
-}
-
-static int fb_plane_height(int height,
- const struct drm_format_info *format, int plane)
-{
- if (plane == 0)
- return height;
-
- return DIV_ROUND_UP(height, format->vsub);
-}
-
static int framebuffer_check(struct drm_device *dev,
const struct drm_mode_fb_cmd2 *r)
{
@@ -196,8 +178,8 @@ static int framebuffer_check(struct drm_device *dev,
info = drm_get_format_info(dev, r);
for (i = 0; i < info->num_planes; i++) {
- unsigned int width = fb_plane_width(r->width, info, i);
- unsigned int height = fb_plane_height(r->height, info, i);
+ unsigned int width = drm_format_info_plane_width(info,
r->width, i);
+ unsigned int height = drm_format_info_plane_height(info,
r->height, i);
unsigned int block_size = info->char_per_block[i];
u64 min_pitch = drm_format_info_min_pitch(info, i, width);
@@ -1136,44 +1118,6 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb)
}
EXPORT_SYMBOL(drm_framebuffer_remove);
-/**
- * drm_framebuffer_plane_width - width of the plane given the first plane
- * @width: width of the first plane
- * @fb: the framebuffer
- * @plane: plane index
- *
- * Returns:
- * The width of @plane, given that the width of the first plane is @width.
- */
-int drm_framebuffer_plane_width(int width,
- const struct drm_framebuffer *fb, int plane)
-{
- if (plane >= fb->format->num_planes)
- return 0;
-
- return fb_plane_width(width, fb->format, plane);
-}
-EXPORT_SYMBOL(drm_framebuffer_plane_width);
-
-/**
- * drm_framebuffer_plane_height - height of the plane given the first plane
- * @height: height of the first plane
- * @fb: the framebuffer
- * @plane: plane index
- *
- * Returns:
- * The height of @plane, given that the height of the first plane is @height.
- */
-int drm_framebuffer_plane_height(int height,
-const struct drm_framebuffer *fb, int plane)
-{
- if (plane >= fb->format->num_planes)
- return 0;
-
- return fb_plane_height(height, fb->format, plane);
-}
-EXPORT_SYMBOL(drm_framebuffer_plane_height);
-
void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent,
const struct drm_framebuffer *fb)
{
@@ -1189,8 +1133,8 @@ void drm_framebuffer_print_info(struct drm_printer *p,
unsigned int indent,
for (i = 0; i < fb->format->num_planes; i++) {
drm_printf_indent(p, indent + 1, "size[%u]=%dx%d\n", i,
- drm_framebuffer_plane_width(fb->width, fb, i),
- drm_framebuffer_plane_height(fb->height, fb,
i));
+ drm_format_info_plane_width(fb->format,
fb->width, i),
+ drm_forma
[PATCH v2 1/2] drm: Remove plane hsub/vsub alignment requirement for core helpers
The drm_format_info_plane_{height,width} functions was implemented using
regular division for the plane size calculation, which cause issues [1][2]
when used on contexts where the dimensions are misaligned with relation
to the subsampling factors. So, replace the regular division by the
DIV_ROUND_UP macro.
This allows these functions to be used in more drivers, making further
work to bring more core presence on them possible.
[1]
http://patchwork.freedesktop.org/patch/msgid/20170321181218.10042-3-ville.syrj...@linux.intel.com
[2]
https://patchwork.freedesktop.org/patch/msgid/20211026225105.2783797-2-imre.d...@intel.com
Signed-off-by: Carlos Eduardo Gallo Filho
---
include/drm/drm_fourcc.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index 532ae78ca747..ccf91daa4307 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -22,6 +22,7 @@
#ifndef __DRM_FOURCC_H__
#define __DRM_FOURCC_H__
+#include
#include
#include
@@ -279,7 +280,7 @@ int drm_format_info_plane_width(const struct
drm_format_info *info, int width,
if (plane == 0)
return width;
- return width / info->hsub;
+ return DIV_ROUND_UP(width, info->hsub);
}
/**
@@ -301,7 +302,7 @@ int drm_format_info_plane_height(const struct
drm_format_info *info, int height,
if (plane == 0)
return height;
- return height / info->vsub;
+ return DIV_ROUND_UP(height, info->vsub);
}
const struct drm_format_info *__drm_format_info(u32 format);
--
2.41.0
[PATCH v2 0/2] drm: Refactor plane size calculation by core helper functions
There's duplicated functions on drm that do the same job of calculating the size of planes from a drm_format_info and the size of its first plane. So this patchset throw away the more specific version intended to be used from a given framebuffer and make the generic version way more portable against the drivers. Thanks, Carlos --- v1 -> v2: https://lore.kernel.org/dri-devel/20230627182239.15676-1-gcar...@disroot.org/ - New patch "[PATCH v2 1/2] drm: Remove plane hsub/vsub alignment requirement for core helpers". --- Carlos Eduardo Gallo Filho (2): drm: Remove plane hsub/vsub alignment requirement for core helpers drm: Replace drm_framebuffer plane size functions with its equivalents drivers/gpu/drm/drm_framebuffer.c | 64 ++--- drivers/gpu/drm/i915/display/intel_fb.c | 2 +- include/drm/drm_fourcc.h| 5 +- include/drm/drm_framebuffer.h | 5 -- 4 files changed, 8 insertions(+), 68 deletions(-) -- 2.41.0
Re: [PATCH v6 05/11] drm/mediatek: dp: Move AUX_P0 setting to mtk_dp_initialize_aux_settings()
Re: [PATCH -next] drm/amdgpu: Fix one kernel-doc comment
On 7/19/23 18:05, Yang Li wrote: > Use colon to separate parameter name from their specific meaning. > silence the warning: > > drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c:793: warning: Function parameter or > member 'adev' not described in 'amdgpu_vm_pte_update_noretry_flags' > > Signed-off-by: Yang Li Reviewed-by: Randy Dunlap Thanks. > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c > index 83e1923f6775..96d601e209b8 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c > @@ -783,7 +783,7 @@ int amdgpu_vm_pde_update(struct amdgpu_vm_update_params > *params, > /** > * amdgpu_vm_pte_update_noretry_flags - Update PTE no-retry flags > * > - * @adev - amdgpu_device pointer > + * @adev: amdgpu_device pointer > * @flags: pointer to PTE flags > * > * Update PTE no-retry flags when TF is enabled. -- ~Randy
[PATCH -next] drm/amdgpu: Fix one kernel-doc comment
Use colon to separate parameter name from their specific meaning. silence the warning: drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c:793: warning: Function parameter or member 'adev' not described in 'amdgpu_vm_pte_update_noretry_flags' Signed-off-by: Yang Li --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c index 83e1923f6775..96d601e209b8 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c @@ -783,7 +783,7 @@ int amdgpu_vm_pde_update(struct amdgpu_vm_update_params *params, /** * amdgpu_vm_pte_update_noretry_flags - Update PTE no-retry flags * - * @adev - amdgpu_device pointer + * @adev: amdgpu_device pointer * @flags: pointer to PTE flags * * Update PTE no-retry flags when TF is enabled. -- 2.20.1.7.g153144c
Re: [PATCH v3] drm/i915: Refactor PAT/object cache handling
On Wed, Jul 19, 2023 at 05:07:15PM -0700, Yang, Fei wrote:
> [snip]
> >> @@ -27,15 +28,8 @@ static bool gpu_write_needs_clflush(struct
> >> drm_i915_gem_object *obj)
> >
> > The code change here looks accurate, but while we're here, I have a side
> > question about this function in general...it was originally introduced
> > in commit 48004881f693 ("drm/i915: Mark CPU cache as dirty when used for
> > rendering") which states that GPU rendering ends up in the CPU cache
> > (and thus needs a clflush later to make sure it lands in memory). That
> > makes sense to me for LLC platforms, but is it really true for non-LLC
> > snooping platforms (like MTL) as the commit states?
>
> For non-LLC platforms objects can be set to 1-way coherent which means
> GPU rendering ending up in CPU cache as well, so for non-LLC platform
> the logic here should be checking 1-way coherent flag.
That's the part that I'm questioning (and not just for MTL, but for all
of our other non-LLC platforms too). Just because there's coherency
doesn't mean that device writes landed in the CPU cache. Coherency is
also achieved if device writes invalidate the contents of the CPU cache.
I thought our non-LLC snooping platforms were coherent due to
write-invalidate rather than write-update, but I can't find it
specifically documented anywhere at the moment. If write-invalidate was
used, then there shouldn't be a need for a later clflush either.
>
> > My understanding
> > was that snooping platforms just invalidated the CPU cache to prevent
> > future CPU reads from seeing stale data but didn't actually stick any
> > new data in there? Am I off track or is the original logic of this
> > function not quite right?
> >
> > Anyway, even if the logic of this function is wrong, it's a mistake that
> > would only hurt performance
>
> Yes, this logic will introduce performance impact because it's missing the
> checking for obj->pat_set_by_user. For objects with pat_set_by_user==true,
> even if the object is snooping or 1-way coherent, we don't want to enforce
> a clflush here since the coherency is supposed to be handled by user space.
>
> > (flushing more often than we truly need to)
> > rather than functionality, so not something we really need to dig into
> > right now as part of this patch.
> >
> >> if (IS_DGFX(i915))
> >> return false;
> >>
> >> -/*
> >> - * For objects created by userspace through GEM_CREATE with pat_index
> >> - * set by set_pat extension, i915_gem_object_has_cache_level() will
> >> - * always return true, because the coherency of such object is managed
> >> - * by userspace. Othereise the call here would fall back to checking
> >> - * whether the object is un-cached or write-through.
> >> - */
> >> -return !(i915_gem_object_has_cache_level(obj, I915_CACHE_NONE) ||
> >> - i915_gem_object_has_cache_level(obj, I915_CACHE_WT));
> >> +return i915_gem_object_has_cache_mode(obj, I915_CACHE_MODE_UC) != 1 &&
> >> + i915_gem_object_has_cache_mode(obj, I915_CACHE_MODE_WT) != 1;
> >> }
>
> [snip]
> >> @@ -640,15 +640,9 @@ static inline int use_cpu_reloc(const struct
> >> reloc_cache *cache,
> >> if (DBG_FORCE_RELOC == FORCE_GTT_RELOC)
> >> return false;
> >>
> >> -/*
> >> - * For objects created by userspace through GEM_CREATE with pat_index
> >> - * set by set_pat extension, i915_gem_object_has_cache_level() always
> >> - * return true, otherwise the call would fall back to checking whether
> >> - * the object is un-cached.
> >> - */
> >> return (cache->has_llc ||
> >> obj->cache_dirty ||
> >> -!i915_gem_object_has_cache_level(obj, I915_CACHE_NONE));
> >> +i915_gem_object_has_cache_mode(obj, I915_CACHE_MODE_UC) != 1);
> >
> > Platforms with relocations and platforms with user-specified PAT have no
> > overlap, right? So a -1 return should be impossible here and this is
> > one case where we could just treat the return value as a boolean, right?
>
> My understanding is that the condition here means to say that, if GPU
> access is uncached, don't use CPU reloc because the CPU cache might
> contain stale data. This condition is sufficient for snooping platforms.
> But from MTL onward, the condition show be whether the GPU access is
> coherent with CPU. So, we should be checking 1-way coherent flag instead
> of UC mode, because even if the GPU access is WB, it's still non-coherent,
> thus CPU cache could be out-dated.
My point is that this is relocation code --- it should be impossible to
get here on MTL and beyond, right? So user-provided PAT isn't a
consideration.
>
> [snip]
> >> @@ -208,12 +230,6 @@ bool i915_gem_object_can_bypass_llc(struct
> >> drm_i915_gem_object *obj)
> >> if (!(obj->flags & I915_BO_ALLOC_USER))
> >> return false;
> >>
> >> -/*
> >> - * Always flush cache for UMD objects at creation time.
> >> - */
> >> -if (ob
[PATCH drm-misc-next v8 12/12] drm/nouveau: debugfs: implement DRM GPU VA debugfs
Provide the driver indirection iterating over all DRM GPU VA spaces to
enable the common 'gpuvas' debugfs file for dumping DRM GPU VA spaces.
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/nouveau/nouveau_debugfs.c | 39 +++
1 file changed, 39 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/nouveau_debugfs.c
b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
index 99d022a91afc..053f703f2f68 100644
--- a/drivers/gpu/drm/nouveau/nouveau_debugfs.c
+++ b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
@@ -203,6 +203,44 @@ nouveau_debugfs_pstate_open(struct inode *inode, struct
file *file)
return single_open(file, nouveau_debugfs_pstate_get, inode->i_private);
}
+static void
+nouveau_debugfs_gpuva_regions(struct seq_file *m, struct nouveau_uvmm *uvmm)
+{
+ MA_STATE(mas, &uvmm->region_mt, 0, 0);
+ struct nouveau_uvma_region *reg;
+
+ seq_puts (m, " VA regions | start | range |
end\n");
+ seq_puts (m,
"\n");
+ mas_for_each(&mas, reg, ULONG_MAX)
+ seq_printf(m, " | 0x%016llx | 0x%016llx |
0x%016llx\n",
+ reg->va.addr, reg->va.range, reg->va.addr +
reg->va.range);
+}
+
+static int
+nouveau_debugfs_gpuva(struct seq_file *m, void *data)
+{
+ struct drm_info_node *node = (struct drm_info_node *) m->private;
+ struct nouveau_drm *drm = nouveau_drm(node->minor->dev);
+ struct nouveau_cli *cli;
+
+ mutex_lock(&drm->clients_lock);
+ list_for_each_entry(cli, &drm->clients, head) {
+ struct nouveau_uvmm *uvmm = nouveau_cli_uvmm(cli);
+
+ if (!uvmm)
+ continue;
+
+ nouveau_uvmm_lock(uvmm);
+ drm_debugfs_gpuva_info(m, &uvmm->umgr);
+ seq_puts(m, "\n");
+ nouveau_debugfs_gpuva_regions(m, uvmm);
+ nouveau_uvmm_unlock(uvmm);
+ }
+ mutex_unlock(&drm->clients_lock);
+
+ return 0;
+}
+
static const struct file_operations nouveau_pstate_fops = {
.owner = THIS_MODULE,
.open = nouveau_debugfs_pstate_open,
@@ -214,6 +252,7 @@ static const struct file_operations nouveau_pstate_fops = {
static struct drm_info_list nouveau_debugfs_list[] = {
{ "vbios.rom", nouveau_debugfs_vbios_image, 0, NULL },
{ "strap_peek", nouveau_debugfs_strap_peek, 0, NULL },
+ DRM_DEBUGFS_GPUVA_INFO(nouveau_debugfs_gpuva, NULL),
};
#define NOUVEAU_DEBUGFS_ENTRIES ARRAY_SIZE(nouveau_debugfs_list)
--
2.41.0
[PATCH drm-misc-next v8 11/12] drm/nouveau: implement new VM_BIND uAPI
This commit provides the implementation for the new uapi motivated by the
Vulkan API. It allows user mode drivers (UMDs) to:
1) Initialize a GPU virtual address (VA) space via the new
DRM_IOCTL_NOUVEAU_VM_INIT ioctl for UMDs to specify the portion of VA
space managed by the kernel and userspace, respectively.
2) Allocate and free a VA space region as well as bind and unbind memory
to the GPUs VA space via the new DRM_IOCTL_NOUVEAU_VM_BIND ioctl.
UMDs can request the named operations to be processed either
synchronously or asynchronously. It supports DRM syncobjs
(incl. timelines) as synchronization mechanism. The management of the
GPU VA mappings is implemented with the DRM GPU VA manager.
3) Execute push buffers with the new DRM_IOCTL_NOUVEAU_EXEC ioctl. The
execution happens asynchronously. It supports DRM syncobj (incl.
timelines) as synchronization mechanism. DRM GEM object locking is
handled with drm_exec.
Both, DRM_IOCTL_NOUVEAU_VM_BIND and DRM_IOCTL_NOUVEAU_EXEC, use the DRM
GPU scheduler for the asynchronous paths.
Signed-off-by: Danilo Krummrich
---
Documentation/gpu/driver-uapi.rst |3 +
drivers/gpu/drm/nouveau/Kbuild |3 +
drivers/gpu/drm/nouveau/Kconfig |2 +
drivers/gpu/drm/nouveau/nouveau_abi16.c | 24 +
drivers/gpu/drm/nouveau/nouveau_abi16.h |1 +
drivers/gpu/drm/nouveau/nouveau_bo.c| 156 +-
drivers/gpu/drm/nouveau/nouveau_bo.h|2 +-
drivers/gpu/drm/nouveau/nouveau_drm.c | 27 +-
drivers/gpu/drm/nouveau/nouveau_drv.h | 59 +-
drivers/gpu/drm/nouveau/nouveau_exec.c | 414 +
drivers/gpu/drm/nouveau/nouveau_exec.h | 54 +
drivers/gpu/drm/nouveau/nouveau_gem.c | 25 +-
drivers/gpu/drm/nouveau/nouveau_mem.h |5 +
drivers/gpu/drm/nouveau/nouveau_prime.c |2 +-
drivers/gpu/drm/nouveau/nouveau_sched.c | 462 ++
drivers/gpu/drm/nouveau/nouveau_sched.h | 123 ++
drivers/gpu/drm/nouveau/nouveau_uvmm.c | 1970 +++
drivers/gpu/drm/nouveau/nouveau_uvmm.h | 107 ++
18 files changed, 3372 insertions(+), 67 deletions(-)
create mode 100644 drivers/gpu/drm/nouveau/nouveau_exec.c
create mode 100644 drivers/gpu/drm/nouveau/nouveau_exec.h
create mode 100644 drivers/gpu/drm/nouveau/nouveau_sched.c
create mode 100644 drivers/gpu/drm/nouveau/nouveau_sched.h
create mode 100644 drivers/gpu/drm/nouveau/nouveau_uvmm.c
create mode 100644 drivers/gpu/drm/nouveau/nouveau_uvmm.h
diff --git a/Documentation/gpu/driver-uapi.rst
b/Documentation/gpu/driver-uapi.rst
index 9c7ca6e33a68..c08bcbb95fb3 100644
--- a/Documentation/gpu/driver-uapi.rst
+++ b/Documentation/gpu/driver-uapi.rst
@@ -13,4 +13,7 @@ drm/nouveau uAPI
VM_BIND / EXEC uAPI
---
+.. kernel-doc:: drivers/gpu/drm/nouveau/nouveau_exec.c
+:doc: Overview
+
.. kernel-doc:: include/uapi/drm/nouveau_drm.h
diff --git a/drivers/gpu/drm/nouveau/Kbuild b/drivers/gpu/drm/nouveau/Kbuild
index 5e5617006da5..cf6b3a80c0c8 100644
--- a/drivers/gpu/drm/nouveau/Kbuild
+++ b/drivers/gpu/drm/nouveau/Kbuild
@@ -47,6 +47,9 @@ nouveau-y += nouveau_prime.o
nouveau-y += nouveau_sgdma.o
nouveau-y += nouveau_ttm.o
nouveau-y += nouveau_vmm.o
+nouveau-y += nouveau_exec.o
+nouveau-y += nouveau_sched.o
+nouveau-y += nouveau_uvmm.o
# DRM - modesetting
nouveau-$(CONFIG_DRM_NOUVEAU_BACKLIGHT) += nouveau_backlight.o
diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
index a70bd65e1400..c52e8096cca4 100644
--- a/drivers/gpu/drm/nouveau/Kconfig
+++ b/drivers/gpu/drm/nouveau/Kconfig
@@ -10,6 +10,8 @@ config DRM_NOUVEAU
select DRM_KMS_HELPER
select DRM_TTM
select DRM_TTM_HELPER
+ select DRM_EXEC
+ select DRM_SCHED
select I2C
select I2C_ALGOBIT
select BACKLIGHT_CLASS_DEVICE if DRM_NOUVEAU_BACKLIGHT
diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c
b/drivers/gpu/drm/nouveau/nouveau_abi16.c
index 82dab51d8aeb..a112f28681d3 100644
--- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
+++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
@@ -35,6 +35,7 @@
#include "nouveau_chan.h"
#include "nouveau_abi16.h"
#include "nouveau_vmm.h"
+#include "nouveau_sched.h"
static struct nouveau_abi16 *
nouveau_abi16(struct drm_file *file_priv)
@@ -125,6 +126,17 @@ nouveau_abi16_chan_fini(struct nouveau_abi16 *abi16,
{
struct nouveau_abi16_ntfy *ntfy, *temp;
+ /* When a client exits without waiting for it's queued up jobs to
+* finish it might happen that we fault the channel. This is due to
+* drm_file_free() calling drm_gem_release() before the postclose()
+* callback. Hence, we can't tear down this scheduler entity before
+* uvmm mappings are unmapped. Currently, we can't detect this case.
+*
+* However, this should be rare and harmless, since the channel isn't
+* needed anymore.
+*/
+ nouveau_sched_entity_fini(&chan->sched_entity);
+
/* wait for all activity
[PATCH drm-misc-next v8 09/12] drm/nouveau: chan: provide nouveau_channel_kill()
The new VM_BIND UAPI implementation introduced in subsequent commits
will allow asynchronous jobs processing push buffers and emitting fences.
If a job times out, we need a way to recover from this situation. For
now, simply kill the channel to unblock all hung up jobs and signal
userspace that the device is dead on the next EXEC or VM_BIND ioctl.
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/nouveau/nouveau_chan.c | 14 +++---
drivers/gpu/drm/nouveau/nouveau_chan.h | 1 +
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c
b/drivers/gpu/drm/nouveau/nouveau_chan.c
index f47c0363683c..a975f8b0e0e5 100644
--- a/drivers/gpu/drm/nouveau/nouveau_chan.c
+++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
@@ -40,6 +40,14 @@ MODULE_PARM_DESC(vram_pushbuf, "Create DMA push buffers in
VRAM");
int nouveau_vram_pushbuf;
module_param_named(vram_pushbuf, nouveau_vram_pushbuf, int, 0400);
+void
+nouveau_channel_kill(struct nouveau_channel *chan)
+{
+ atomic_set(&chan->killed, 1);
+ if (chan->fence)
+ nouveau_fence_context_kill(chan->fence, -ENODEV);
+}
+
static int
nouveau_channel_killed(struct nvif_event *event, void *repv, u32 repc)
{
@@ -47,9 +55,9 @@ nouveau_channel_killed(struct nvif_event *event, void *repv,
u32 repc)
struct nouveau_cli *cli = (void *)chan->user.client;
NV_PRINTK(warn, cli, "channel %d killed!\n", chan->chid);
- atomic_set(&chan->killed, 1);
- if (chan->fence)
- nouveau_fence_context_kill(chan->fence, -ENODEV);
+
+ if (unlikely(!atomic_read(&chan->killed)))
+ nouveau_channel_kill(chan);
return NVIF_EVENT_DROP;
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.h
b/drivers/gpu/drm/nouveau/nouveau_chan.h
index e06a8ffed31a..e483f4a254da 100644
--- a/drivers/gpu/drm/nouveau/nouveau_chan.h
+++ b/drivers/gpu/drm/nouveau/nouveau_chan.h
@@ -65,6 +65,7 @@ int nouveau_channel_new(struct nouveau_drm *, struct
nvif_device *, bool priv,
u32 vram, u32 gart, struct nouveau_channel **);
void nouveau_channel_del(struct nouveau_channel **);
int nouveau_channel_idle(struct nouveau_channel *);
+void nouveau_channel_kill(struct nouveau_channel *);
extern int nouveau_vram_pushbuf;
--
2.41.0
[PATCH drm-misc-next v8 10/12] drm/nouveau: nvkm/vmm: implement raw ops to manage uvmm
The new VM_BIND UAPI uses the DRM GPU VA manager to manage the VA space.
Hence, we a need a way to manipulate the MMUs page tables without going
through the internal range allocator implemented by nvkm/vmm.
This patch adds a raw interface for nvkm/vmm to pass the resposibility
for managing the address space and the corresponding map/unmap/sparse
operations to the upper layers.
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/nouveau/include/nvif/if000c.h | 26 ++-
drivers/gpu/drm/nouveau/include/nvif/vmm.h| 19 +-
.../gpu/drm/nouveau/include/nvkm/subdev/mmu.h | 20 +-
drivers/gpu/drm/nouveau/nouveau_svm.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_vmm.c | 4 +-
drivers/gpu/drm/nouveau/nvif/vmm.c| 100 +++-
.../gpu/drm/nouveau/nvkm/subdev/mmu/uvmm.c| 213 --
drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c | 197
drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.h | 25 ++
.../drm/nouveau/nvkm/subdev/mmu/vmmgf100.c| 16 +-
.../drm/nouveau/nvkm/subdev/mmu/vmmgp100.c| 16 +-
.../gpu/drm/nouveau/nvkm/subdev/mmu/vmmnv50.c | 27 ++-
12 files changed, 566 insertions(+), 99 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/include/nvif/if000c.h
b/drivers/gpu/drm/nouveau/include/nvif/if000c.h
index 9c7ff56831c5..a5a182b3c28d 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/if000c.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/if000c.h
@@ -3,7 +3,10 @@
struct nvif_vmm_v0 {
__u8 version;
__u8 page_nr;
- __u8 managed;
+#define NVIF_VMM_V0_TYPE_UNMANAGED 0x00
+#define NVIF_VMM_V0_TYPE_MANAGED 0x01
+#define NVIF_VMM_V0_TYPE_RAW 0x02
+ __u8 type;
__u8 pad03[5];
__u64 addr;
__u64 size;
@@ -17,6 +20,7 @@ struct nvif_vmm_v0 {
#define NVIF_VMM_V0_UNMAP 0x04
#define NVIF_VMM_V0_PFNMAP 0x05
#define NVIF_VMM_V0_PFNCLR 0x06
+#define NVIF_VMM_V0_RAW0x07
#define NVIF_VMM_V0_MTHD(i) ((i) +
0x80)
struct nvif_vmm_page_v0 {
@@ -66,6 +70,26 @@ struct nvif_vmm_unmap_v0 {
__u64 addr;
};
+struct nvif_vmm_raw_v0 {
+ __u8 version;
+#define NVIF_VMM_RAW_V0_GET0x0
+#define NVIF_VMM_RAW_V0_PUT0x1
+#define NVIF_VMM_RAW_V0_MAP0x2
+#define NVIF_VMM_RAW_V0_UNMAP 0x3
+#define NVIF_VMM_RAW_V0_SPARSE 0x4
+ __u8 op;
+ __u8 sparse;
+ __u8 ref;
+ __u8 shift;
+ __u32 argc;
+ __u8 pad01[7];
+ __u64 addr;
+ __u64 size;
+ __u64 offset;
+ __u64 memory;
+ __u64 argv;
+};
+
struct nvif_vmm_pfnmap_v0 {
__u8 version;
__u8 page;
diff --git a/drivers/gpu/drm/nouveau/include/nvif/vmm.h
b/drivers/gpu/drm/nouveau/include/nvif/vmm.h
index a2ee92201ace..0ecedd0ee0a5 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/vmm.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/vmm.h
@@ -4,6 +4,12 @@
struct nvif_mem;
struct nvif_mmu;
+enum nvif_vmm_type {
+ UNMANAGED,
+ MANAGED,
+ RAW,
+};
+
enum nvif_vmm_get {
ADDR,
PTES,
@@ -30,8 +36,9 @@ struct nvif_vmm {
int page_nr;
};
-int nvif_vmm_ctor(struct nvif_mmu *, const char *name, s32 oclass, bool
managed,
- u64 addr, u64 size, void *argv, u32 argc, struct nvif_vmm *);
+int nvif_vmm_ctor(struct nvif_mmu *, const char *name, s32 oclass,
+ enum nvif_vmm_type, u64 addr, u64 size, void *argv, u32 argc,
+ struct nvif_vmm *);
void nvif_vmm_dtor(struct nvif_vmm *);
int nvif_vmm_get(struct nvif_vmm *, enum nvif_vmm_get, bool sparse,
u8 page, u8 align, u64 size, struct nvif_vma *);
@@ -39,4 +46,12 @@ void nvif_vmm_put(struct nvif_vmm *, struct nvif_vma *);
int nvif_vmm_map(struct nvif_vmm *, u64 addr, u64 size, void *argv, u32 argc,
struct nvif_mem *, u64 offset);
int nvif_vmm_unmap(struct nvif_vmm *, u64);
+
+int nvif_vmm_raw_get(struct nvif_vmm *vmm, u64 addr, u64 size, u8 shift);
+int nvif_vmm_raw_put(struct nvif_vmm *vmm, u64 addr, u64 size, u8 shift);
+int nvif_vmm_raw_map(struct nvif_vmm *vmm, u64 addr, u64 size, u8 shift,
+void *argv, u32 argc, struct nvif_mem *mem, u64 offset);
+int nvif_vmm_raw_unmap(struct nvif_vmm *vmm, u64 addr, u64 size,
+ u8 shift, bool sparse);
+int nvif_vmm_raw_sparse(struct nvif_vmm *vmm, u64 addr, u64 size, bool ref);
#endif
diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/mmu.h
b/drivers/gpu/drm/nouveau/include/nvkm/subdev/mmu.h
index 70e7887ef4b4..2fd2f2433fc7 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/mmu.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/mmu.h
@@ -17,6 +17,7 @@ s
[PATCH drm-misc-next v8 08/12] drm/nouveau: fence: fail to emit when fence context is killed
The new VM_BIND UAPI implementation introduced in subsequent commits
will allow asynchronous jobs processing push buffers and emitting
fences.
If a fence context is killed, e.g. due to a channel fault, jobs which
are already queued for execution might still emit new fences. In such a
case a job would hang forever.
To fix that, fail to emit a new fence on a killed fence context with
-ENODEV to unblock the job.
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/nouveau/nouveau_fence.c | 7 +++
drivers/gpu/drm/nouveau/nouveau_fence.h | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c
b/drivers/gpu/drm/nouveau/nouveau_fence.c
index e946408f945b..77c739a55b19 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fence.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
@@ -96,6 +96,7 @@ nouveau_fence_context_kill(struct nouveau_fence_chan *fctx,
int error)
if (nouveau_fence_signal(fence))
nvif_event_block(&fctx->event);
}
+ fctx->killed = 1;
spin_unlock_irqrestore(&fctx->lock, flags);
}
@@ -229,6 +230,12 @@ nouveau_fence_emit(struct nouveau_fence *fence, struct
nouveau_channel *chan)
dma_fence_get(&fence->base);
spin_lock_irq(&fctx->lock);
+ if (unlikely(fctx->killed)) {
+ spin_unlock_irq(&fctx->lock);
+ dma_fence_put(&fence->base);
+ return -ENODEV;
+ }
+
if (nouveau_fence_update(chan, fctx))
nvif_event_block(&fctx->event);
diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.h
b/drivers/gpu/drm/nouveau/nouveau_fence.h
index 7c73c7c9834a..2c72d96ef17d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fence.h
+++ b/drivers/gpu/drm/nouveau/nouveau_fence.h
@@ -44,7 +44,7 @@ struct nouveau_fence_chan {
char name[32];
struct nvif_event event;
- int notify_ref, dead;
+ int notify_ref, dead, killed;
};
struct nouveau_fence_priv {
--
2.41.0
[PATCH drm-misc-next v8 06/12] drm/nouveau: move usercopy helpers to nouveau_drv.h
Move the usercopy helpers to a common driver header file to make it
usable for the new API added in subsequent commits.
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/nouveau/nouveau_drv.h | 26 ++
drivers/gpu/drm/nouveau/nouveau_gem.c | 26 --
2 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h
b/drivers/gpu/drm/nouveau/nouveau_drv.h
index 81350e685b50..20a7f31b9082 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -130,6 +130,32 @@ nouveau_cli(struct drm_file *fpriv)
return fpriv ? fpriv->driver_priv : NULL;
}
+static inline void
+u_free(void *addr)
+{
+ kvfree(addr);
+}
+
+static inline void *
+u_memcpya(uint64_t user, unsigned nmemb, unsigned size)
+{
+ void *mem;
+ void __user *userptr = (void __force __user *)(uintptr_t)user;
+
+ size *= nmemb;
+
+ mem = kvmalloc(size, GFP_KERNEL);
+ if (!mem)
+ return ERR_PTR(-ENOMEM);
+
+ if (copy_from_user(mem, userptr, size)) {
+ u_free(mem);
+ return ERR_PTR(-EFAULT);
+ }
+
+ return mem;
+}
+
#include
#include
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c
b/drivers/gpu/drm/nouveau/nouveau_gem.c
index 45ca4eb98f54..a48f42aaeab9 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -613,32 +613,6 @@ nouveau_gem_pushbuf_validate(struct nouveau_channel *chan,
return 0;
}
-static inline void
-u_free(void *addr)
-{
- kvfree(addr);
-}
-
-static inline void *
-u_memcpya(uint64_t user, unsigned nmemb, unsigned size)
-{
- void *mem;
- void __user *userptr = (void __force __user *)(uintptr_t)user;
-
- size *= nmemb;
-
- mem = kvmalloc(size, GFP_KERNEL);
- if (!mem)
- return ERR_PTR(-ENOMEM);
-
- if (copy_from_user(mem, userptr, size)) {
- u_free(mem);
- return ERR_PTR(-EFAULT);
- }
-
- return mem;
-}
-
static int
nouveau_gem_pushbuf_reloc_apply(struct nouveau_cli *cli,
struct drm_nouveau_gem_pushbuf *req,
--
2.41.0
[PATCH drm-misc-next v8 07/12] drm/nouveau: fence: separate fence alloc and emit
The new (VM_BIND) UAPI exports DMA fences through DRM syncobjs. Hence,
in order to emit fences within DMA fence signalling critical sections
(e.g. as typically done in the DRM GPU schedulers run_job() callback) we
need to separate fence allocation and fence emitting.
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/nouveau/dispnv04/crtc.c | 9 -
drivers/gpu/drm/nouveau/nouveau_bo.c| 52 +++--
drivers/gpu/drm/nouveau/nouveau_chan.c | 6 ++-
drivers/gpu/drm/nouveau/nouveau_dmem.c | 9 +++--
drivers/gpu/drm/nouveau/nouveau_fence.c | 16 +++-
drivers/gpu/drm/nouveau/nouveau_fence.h | 3 +-
drivers/gpu/drm/nouveau/nouveau_gem.c | 5 ++-
7 files changed, 59 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index a6f2e681bde9..a34924523133 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -1122,11 +1122,18 @@ nv04_page_flip_emit(struct nouveau_channel *chan,
PUSH_NVSQ(push, NV_SW, NV_SW_PAGE_FLIP, 0x);
PUSH_KICK(push);
- ret = nouveau_fence_new(chan, false, pfence);
+ ret = nouveau_fence_new(pfence);
if (ret)
goto fail;
+ ret = nouveau_fence_emit(*pfence, chan);
+ if (ret)
+ goto fail_fence_unref;
+
return 0;
+
+fail_fence_unref:
+ nouveau_fence_unref(pfence);
fail:
spin_lock_irqsave(&dev->event_lock, flags);
list_del(&s->head);
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 057bc995f19b..e9cbbf594e6f 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -820,29 +820,39 @@ nouveau_bo_move_m2mf(struct ttm_buffer_object *bo, int
evict,
mutex_lock(&cli->mutex);
else
mutex_lock_nested(&cli->mutex, SINGLE_DEPTH_NESTING);
+
ret = nouveau_fence_sync(nouveau_bo(bo), chan, true,
ctx->interruptible);
- if (ret == 0) {
- ret = drm->ttm.move(chan, bo, bo->resource, new_reg);
- if (ret == 0) {
- ret = nouveau_fence_new(chan, false, &fence);
- if (ret == 0) {
- /* TODO: figure out a better solution here
-*
-* wait on the fence here explicitly as going
through
-* ttm_bo_move_accel_cleanup somehow doesn't
seem to do it.
-*
-* Without this the operation can timeout and
we'll fallback to a
-* software copy, which might take several
minutes to finish.
-*/
- nouveau_fence_wait(fence, false, false);
- ret = ttm_bo_move_accel_cleanup(bo,
- &fence->base,
- evict, false,
- new_reg);
- nouveau_fence_unref(&fence);
- }
- }
+ if (ret)
+ goto out_unlock;
+
+ ret = drm->ttm.move(chan, bo, bo->resource, new_reg);
+ if (ret)
+ goto out_unlock;
+
+ ret = nouveau_fence_new(&fence);
+ if (ret)
+ goto out_unlock;
+
+ ret = nouveau_fence_emit(fence, chan);
+ if (ret) {
+ nouveau_fence_unref(&fence);
+ goto out_unlock;
}
+
+ /* TODO: figure out a better solution here
+*
+* wait on the fence here explicitly as going through
+* ttm_bo_move_accel_cleanup somehow doesn't seem to do it.
+*
+* Without this the operation can timeout and we'll fallback to a
+* software copy, which might take several minutes to finish.
+*/
+ nouveau_fence_wait(fence, false, false);
+ ret = ttm_bo_move_accel_cleanup(bo, &fence->base, evict, false,
+ new_reg);
+ nouveau_fence_unref(&fence);
+
+out_unlock:
mutex_unlock(&cli->mutex);
return ret;
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c
b/drivers/gpu/drm/nouveau/nouveau_chan.c
index 1068abe41024..f47c0363683c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_chan.c
+++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
@@ -62,9 +62,11 @@ nouveau_channel_idle(struct nouveau_channel *chan)
struct nouveau_fence *fence = NULL;
int ret;
- ret = nouveau_fence_new(chan, false, &fence);
+ ret = nouveau_fence_new(&fence);
if (!ret) {
- ret = nouveau_fence_wait(fence, false, false);
+ ret = nouveau_fence_
[PATCH drm-misc-next v8 05/12] drm/nouveau: bo: initialize GEM GPU VA interface
Initialize the GEM's DRM GPU VA manager interface in preparation for the (u)vmm implementation, provided by subsequent commits, to make use of it. Signed-off-by: Danilo Krummrich --- drivers/gpu/drm/nouveau/nouveau_bo.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 7724fe63067d..057bc995f19b 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -215,11 +215,14 @@ nouveau_bo_alloc(struct nouveau_cli *cli, u64 *size, int *align, u32 domain, nvbo = kzalloc(sizeof(struct nouveau_bo), GFP_KERNEL); if (!nvbo) return ERR_PTR(-ENOMEM); + INIT_LIST_HEAD(&nvbo->head); INIT_LIST_HEAD(&nvbo->entry); INIT_LIST_HEAD(&nvbo->vma_list); nvbo->bo.bdev = &drm->ttm.bdev; + drm_gem_gpuva_init(&nvbo->bo.base); + /* This is confusing, and doesn't actually mean we want an uncached * mapping, but is what NOUVEAU_GEM_DOMAIN_COHERENT gets translated * into in nouveau_gem_new(). -- 2.41.0
[PATCH drm-misc-next v8 04/12] drm/nouveau: get vmm via nouveau_cli_vmm()
Provide a getter function for the client's current vmm context. Since
we'll add a new (u)vmm context for UMD bindings in subsequent commits,
this will keep the code clean.
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/nouveau/nouveau_bo.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_chan.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_drv.h | 9 +
drivers/gpu/drm/nouveau/nouveau_gem.c | 6 +++---
4 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index c2ec91cc845d..7724fe63067d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -204,7 +204,7 @@ nouveau_bo_alloc(struct nouveau_cli *cli, u64 *size, int
*align, u32 domain,
struct nouveau_drm *drm = cli->drm;
struct nouveau_bo *nvbo;
struct nvif_mmu *mmu = &cli->mmu;
- struct nvif_vmm *vmm = cli->svm.cli ? &cli->svm.vmm : &cli->vmm.vmm;
+ struct nvif_vmm *vmm = &nouveau_cli_vmm(cli)->vmm;
int i, pi = -1;
if (!*size) {
diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c
b/drivers/gpu/drm/nouveau/nouveau_chan.c
index e648ecd0c1a0..1068abe41024 100644
--- a/drivers/gpu/drm/nouveau/nouveau_chan.c
+++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
@@ -148,7 +148,7 @@ nouveau_channel_prep(struct nouveau_drm *drm, struct
nvif_device *device,
chan->device = device;
chan->drm = drm;
- chan->vmm = cli->svm.cli ? &cli->svm : &cli->vmm;
+ chan->vmm = nouveau_cli_vmm(cli);
atomic_set(&chan->killed, 0);
/* allocate memory for dma push buffer */
diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h
b/drivers/gpu/drm/nouveau/nouveau_drv.h
index b5de312a523f..81350e685b50 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -112,6 +112,15 @@ struct nouveau_cli_work {
struct dma_fence_cb cb;
};
+static inline struct nouveau_vmm *
+nouveau_cli_vmm(struct nouveau_cli *cli)
+{
+ if (cli->svm.cli)
+ return &cli->svm;
+
+ return &cli->vmm;
+}
+
void nouveau_cli_work_queue(struct nouveau_cli *, struct dma_fence *,
struct nouveau_cli_work *);
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c
b/drivers/gpu/drm/nouveau/nouveau_gem.c
index ab9062e50977..45ca4eb98f54 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -103,7 +103,7 @@ nouveau_gem_object_open(struct drm_gem_object *gem, struct
drm_file *file_priv)
struct nouveau_bo *nvbo = nouveau_gem_object(gem);
struct nouveau_drm *drm = nouveau_bdev(nvbo->bo.bdev);
struct device *dev = drm->dev->dev;
- struct nouveau_vmm *vmm = cli->svm.cli ? &cli->svm : &cli->vmm;
+ struct nouveau_vmm *vmm = nouveau_cli_vmm(cli);
struct nouveau_vma *vma;
int ret;
@@ -180,7 +180,7 @@ nouveau_gem_object_close(struct drm_gem_object *gem, struct
drm_file *file_priv)
struct nouveau_bo *nvbo = nouveau_gem_object(gem);
struct nouveau_drm *drm = nouveau_bdev(nvbo->bo.bdev);
struct device *dev = drm->dev->dev;
- struct nouveau_vmm *vmm = cli->svm.cli ? &cli->svm : & cli->vmm;
+ struct nouveau_vmm *vmm = nouveau_cli_vmm(cli);
struct nouveau_vma *vma;
int ret;
@@ -269,7 +269,7 @@ nouveau_gem_info(struct drm_file *file_priv, struct
drm_gem_object *gem,
{
struct nouveau_cli *cli = nouveau_cli(file_priv);
struct nouveau_bo *nvbo = nouveau_gem_object(gem);
- struct nouveau_vmm *vmm = cli->svm.cli ? &cli->svm : &cli->vmm;
+ struct nouveau_vmm *vmm = nouveau_cli_vmm(cli);
struct nouveau_vma *vma;
if (is_power_of_2(nvbo->valid_domains))
--
2.41.0
[PATCH drm-misc-next v8 02/12] drm: debugfs: provide infrastructure to dump a DRM GPU VA space
This commit adds a function to dump a DRM GPU VA space and a macro for
drivers to register the struct drm_info_list 'gpuvas' entry.
Most likely, most drivers might maintain one DRM GPU VA space per struct
drm_file, but there might also be drivers not having a fixed relation
between DRM GPU VA spaces and a DRM core infrastructure, hence we need the
indirection via the driver iterating it's maintained DRM GPU VA spaces.
Reviewed-by: Boris Brezillon
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/drm_debugfs.c | 40 +++
include/drm/drm_debugfs.h | 25 ++
2 files changed, 65 insertions(+)
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 4855230ba2c6..c90dbcffa0dc 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -39,6 +39,7 @@
#include
#include
#include
+#include
#include "drm_crtc_internal.h"
#include "drm_internal.h"
@@ -175,6 +176,45 @@ static const struct file_operations drm_debugfs_fops = {
.release = single_release,
};
+/**
+ * drm_debugfs_gpuva_info - dump the given DRM GPU VA space
+ * @m: pointer to the &seq_file to write
+ * @mgr: the &drm_gpuva_manager representing the GPU VA space
+ *
+ * Dumps the GPU VA mappings of a given DRM GPU VA manager.
+ *
+ * For each DRM GPU VA space drivers should call this function from their
+ * &drm_info_list's show callback.
+ *
+ * Returns: 0 on success, -ENODEV if the &mgr is not initialized
+ */
+int drm_debugfs_gpuva_info(struct seq_file *m,
+ struct drm_gpuva_manager *mgr)
+{
+ struct drm_gpuva *va, *kva = &mgr->kernel_alloc_node;
+
+ if (!mgr->name)
+ return -ENODEV;
+
+ seq_printf(m, "DRM GPU VA space (%s) [0x%016llx;0x%016llx]\n",
+ mgr->name, mgr->mm_start, mgr->mm_start + mgr->mm_range);
+ seq_printf(m, "Kernel reserved node [0x%016llx;0x%016llx]\n",
+ kva->va.addr, kva->va.addr + kva->va.range);
+ seq_puts(m, "\n");
+ seq_puts(m, " VAs | start | range | end
| object | object offset\n");
+ seq_puts(m,
"-\n");
+ drm_gpuva_for_each_va(va, mgr) {
+ if (unlikely(va == kva))
+ continue;
+
+ seq_printf(m, " | 0x%016llx | 0x%016llx | 0x%016llx |
0x%016llx | 0x%016llx\n",
+ va->va.addr, va->va.range, va->va.addr +
va->va.range,
+ (u64)va->gem.obj, va->gem.offset);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_debugfs_gpuva_info);
/**
* drm_debugfs_create_files - Initialize a given set of debugfs files for DRM
diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h
index 7616f457ce70..cb2c1956a214 100644
--- a/include/drm/drm_debugfs.h
+++ b/include/drm/drm_debugfs.h
@@ -34,6 +34,22 @@
#include
#include
+
+#include
+
+/**
+ * DRM_DEBUGFS_GPUVA_INFO - &drm_info_list entry to dump a GPU VA space
+ * @show: the &drm_info_list's show callback
+ * @data: driver private data
+ *
+ * Drivers should use this macro to define a &drm_info_list entry to provide a
+ * debugfs file for dumping the GPU VA space regions and mappings.
+ *
+ * For each DRM GPU VA space drivers should call drm_debugfs_gpuva_info() from
+ * their @show callback.
+ */
+#define DRM_DEBUGFS_GPUVA_INFO(show, data) {"gpuvas", show, DRIVER_GEM_GPUVA,
data}
+
/**
* struct drm_info_list - debugfs info list entry
*
@@ -134,6 +150,9 @@ void drm_debugfs_add_file(struct drm_device *dev, const
char *name,
void drm_debugfs_add_files(struct drm_device *dev,
const struct drm_debugfs_info *files, int count);
+
+int drm_debugfs_gpuva_info(struct seq_file *m,
+ struct drm_gpuva_manager *mgr);
#else
static inline void drm_debugfs_create_files(const struct drm_info_list *files,
int count, struct dentry *root,
@@ -155,6 +174,12 @@ static inline void drm_debugfs_add_files(struct drm_device
*dev,
const struct drm_debugfs_info *files,
int count)
{}
+
+static inline int drm_debugfs_gpuva_info(struct seq_file *m,
+struct drm_gpuva_manager *mgr)
+{
+ return 0;
+}
#endif
#endif /* _DRM_DEBUGFS_H_ */
--
2.41.0
[PATCH drm-misc-next v8 03/12] drm/nouveau: new VM_BIND uapi interfaces
This commit provides the interfaces for the new UAPI motivated by the
Vulkan API. It allows user mode drivers (UMDs) to:
1) Initialize a GPU virtual address (VA) space via the new
DRM_IOCTL_NOUVEAU_VM_INIT ioctl. UMDs can provide a kernel reserved
VA area.
2) Bind and unbind GPU VA space mappings via the new
DRM_IOCTL_NOUVEAU_VM_BIND ioctl.
3) Execute push buffers with the new DRM_IOCTL_NOUVEAU_EXEC ioctl.
Both, DRM_IOCTL_NOUVEAU_VM_BIND and DRM_IOCTL_NOUVEAU_EXEC support
asynchronous processing with DRM syncobjs as synchronization mechanism.
The default DRM_IOCTL_NOUVEAU_VM_BIND is synchronous processing,
DRM_IOCTL_NOUVEAU_EXEC supports asynchronous processing only.
Co-authored-by: Dave Airlie
Signed-off-by: Danilo Krummrich
---
Documentation/gpu/driver-uapi.rst | 8 ++
include/uapi/drm/nouveau_drm.h| 209 ++
2 files changed, 217 insertions(+)
diff --git a/Documentation/gpu/driver-uapi.rst
b/Documentation/gpu/driver-uapi.rst
index 4411e6919a3d..9c7ca6e33a68 100644
--- a/Documentation/gpu/driver-uapi.rst
+++ b/Documentation/gpu/driver-uapi.rst
@@ -6,3 +6,11 @@ drm/i915 uAPI
=
.. kernel-doc:: include/uapi/drm/i915_drm.h
+
+drm/nouveau uAPI
+
+
+VM_BIND / EXEC uAPI
+---
+
+.. kernel-doc:: include/uapi/drm/nouveau_drm.h
diff --git a/include/uapi/drm/nouveau_drm.h b/include/uapi/drm/nouveau_drm.h
index 853a327433d3..4d3a70529637 100644
--- a/include/uapi/drm/nouveau_drm.h
+++ b/include/uapi/drm/nouveau_drm.h
@@ -126,6 +126,209 @@ struct drm_nouveau_gem_cpu_fini {
__u32 handle;
};
+/**
+ * struct drm_nouveau_sync - sync object
+ *
+ * This structure serves as synchronization mechanism for (potentially)
+ * asynchronous operations such as EXEC or VM_BIND.
+ */
+struct drm_nouveau_sync {
+ /**
+* @flags: the flags for a sync object
+*
+* The first 8 bits are used to determine the type of the sync object.
+*/
+ __u32 flags;
+#define DRM_NOUVEAU_SYNC_SYNCOBJ 0x0
+#define DRM_NOUVEAU_SYNC_TIMELINE_SYNCOBJ 0x1
+#define DRM_NOUVEAU_SYNC_TYPE_MASK 0xf
+ /**
+* @handle: the handle of the sync object
+*/
+ __u32 handle;
+ /**
+* @timeline_value:
+*
+* The timeline point of the sync object in case the syncobj is of
+* type DRM_NOUVEAU_SYNC_TIMELINE_SYNCOBJ.
+*/
+ __u64 timeline_value;
+};
+
+/**
+ * struct drm_nouveau_vm_init - GPU VA space init structure
+ *
+ * Used to initialize the GPU's VA space for a user client, telling the kernel
+ * which portion of the VA space is managed by the UMD and kernel respectively.
+ */
+struct drm_nouveau_vm_init {
+ /**
+* @unmanaged_addr: start address of the kernel managed VA space region
+*/
+ __u64 unmanaged_addr;
+ /**
+* @unmanaged_size: size of the kernel managed VA space region in bytes
+*/
+ __u64 unmanaged_size;
+};
+
+/**
+ * struct drm_nouveau_vm_bind_op - VM_BIND operation
+ *
+ * This structure represents a single VM_BIND operation. UMDs should pass
+ * an array of this structure via struct drm_nouveau_vm_bind's &op_ptr field.
+ */
+struct drm_nouveau_vm_bind_op {
+ /**
+* @op: the operation type
+*/
+ __u32 op;
+/**
+ * @DRM_NOUVEAU_VM_BIND_OP_MAP:
+ *
+ * Map a GEM object to the GPU's VA space. Optionally, the
+ * &DRM_NOUVEAU_VM_BIND_SPARSE flag can be passed to instruct the kernel to
+ * create sparse mappings for the given range.
+ */
+#define DRM_NOUVEAU_VM_BIND_OP_MAP 0x0
+/**
+ * @DRM_NOUVEAU_VM_BIND_OP_UNMAP:
+ *
+ * Unmap an existing mapping in the GPU's VA space. If the region the mapping
+ * is located in is a sparse region, new sparse mappings are created where the
+ * unmapped (memory backed) mapping was mapped previously. To remove a sparse
+ * region the &DRM_NOUVEAU_VM_BIND_SPARSE must be set.
+ */
+#define DRM_NOUVEAU_VM_BIND_OP_UNMAP 0x1
+ /**
+* @flags: the flags for a &drm_nouveau_vm_bind_op
+*/
+ __u32 flags;
+/**
+ * @DRM_NOUVEAU_VM_BIND_SPARSE:
+ *
+ * Indicates that an allocated VA space region should be sparse.
+ */
+#define DRM_NOUVEAU_VM_BIND_SPARSE (1 << 8)
+ /**
+* @handle: the handle of the DRM GEM object to map
+*/
+ __u32 handle;
+ /**
+* @pad: 32 bit padding, should be 0
+*/
+ __u32 pad;
+ /**
+* @addr:
+*
+* the address the VA space region or (memory backed) mapping should be
mapped to
+*/
+ __u64 addr;
+ /**
+* @bo_offset: the offset within the BO backing the mapping
+*/
+ __u64 bo_offset;
+ /**
+* @range: the size of the requested mapping in bytes
+*/
+ __u64 range;
+};
+
+/**
+ * struct drm_nouveau_vm_bind - structure for DRM_IOCTL_NOUVEAU_VM_BIND
+ */
+struct drm_nouveau_vm_bind {
+ /**
+* @op_count: the number of &drm_nou
[PATCH drm-misc-next v8 01/12] drm: manager to keep track of GPUs VA mappings
Add infrastructure to keep track of GPU virtual address (VA) mappings with a decicated VA space manager implementation. New UAPIs, motivated by Vulkan sparse memory bindings graphics drivers start implementing, allow userspace applications to request multiple and arbitrary GPU VA mappings of buffer objects. The DRM GPU VA manager is intended to serve the following purposes in this context. 1) Provide infrastructure to track GPU VA allocations and mappings, making using an interval tree (RB-tree). 2) Generically connect GPU VA mappings to their backing buffers, in particular DRM GEM objects. 3) Provide a common implementation to perform more complex mapping operations on the GPU VA space. In particular splitting and merging of GPU VA mappings, e.g. for intersecting mapping requests or partial unmap requests. Acked-by: Thomas Hellström Acked-by: Matthew Brost Reviewed-by: Boris Brezillon Tested-by: Matthew Brost Tested-by: Donald Robson Suggested-by: Dave Airlie Signed-off-by: Danilo Krummrich --- Documentation/gpu/drm-mm.rst| 36 + drivers/gpu/drm/Makefile|1 + drivers/gpu/drm/drm_gem.c |3 + drivers/gpu/drm/drm_gpuva_mgr.c | 1728 +++ include/drm/drm_drv.h |6 + include/drm/drm_gem.h | 79 ++ include/drm/drm_gpuva_mgr.h | 706 + 7 files changed, 2559 insertions(+) create mode 100644 drivers/gpu/drm/drm_gpuva_mgr.c create mode 100644 include/drm/drm_gpuva_mgr.h diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst index a52e6f4117d6..3d5dc9dc1bfe 100644 --- a/Documentation/gpu/drm-mm.rst +++ b/Documentation/gpu/drm-mm.rst @@ -466,6 +466,42 @@ DRM MM Range Allocator Function References .. kernel-doc:: drivers/gpu/drm/drm_mm.c :export: +DRM GPU VA Manager +== + +Overview + + +.. kernel-doc:: drivers/gpu/drm/drm_gpuva_mgr.c + :doc: Overview + +Split and Merge +--- + +.. kernel-doc:: drivers/gpu/drm/drm_gpuva_mgr.c + :doc: Split and Merge + +Locking +--- + +.. kernel-doc:: drivers/gpu/drm/drm_gpuva_mgr.c + :doc: Locking + +Examples + + +.. kernel-doc:: drivers/gpu/drm/drm_gpuva_mgr.c + :doc: Examples + +DRM GPU VA Manager Function References +-- + +.. kernel-doc:: include/drm/drm_gpuva_mgr.h + :internal: + +.. kernel-doc:: drivers/gpu/drm/drm_gpuva_mgr.c + :export: + DRM Buddy Allocator === diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 021b3f0ac152..215e78e79125 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -45,6 +45,7 @@ drm-y := \ drm_vblank.o \ drm_vblank_work.o \ drm_vma_manager.o \ + drm_gpuva_mgr.o \ drm_writeback.o drm-$(CONFIG_DRM_LEGACY) += \ drm_agpsupport.o \ diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index c18686f434d4..dfe76c70bcb3 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -164,6 +164,9 @@ void drm_gem_private_object_init(struct drm_device *dev, if (!obj->resv) obj->resv = &obj->_resv; + if (drm_core_check_feature(dev, DRIVER_GEM_GPUVA)) + drm_gem_gpuva_init(obj); + drm_vma_node_reset(&obj->vma_node); INIT_LIST_HEAD(&obj->lru_node); } diff --git a/drivers/gpu/drm/drm_gpuva_mgr.c b/drivers/gpu/drm/drm_gpuva_mgr.c new file mode 100644 index ..dee2235530d6 --- /dev/null +++ b/drivers/gpu/drm/drm_gpuva_mgr.c @@ -0,0 +1,1728 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2022 Red Hat. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + * + * Authors: + * Danilo Krummrich + * + */ + +#include + +#include +#include + +/** + * DOC: Overview + * + * The DRM GPU VA Manager, represented by struct drm_gpuva_manager keeps track + * of a GPU's virtual address (VA) s
[PATCH drm-misc-next v8 00/12] DRM GPUVA Manager & Nouveau VM_BIND UAPI
MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch series provides a new UAPI for the Nouveau driver in order to support Vulkan features, such as sparse bindings and sparse residency. Furthermore, with the DRM GPUVA manager it provides a new DRM core feature to keep track of GPU virtual address (VA) mappings in a more generic way. The DRM GPUVA manager is indented to help drivers implement userspace-manageable GPU VA spaces in reference to the Vulkan API. In order to achieve this goal it serves the following purposes in this context. 1) Provide infrastructure to track GPU VA allocations and mappings, using an interval tree (RB-tree). 2) Generically connect GPU VA mappings to their backing buffers, in particular DRM GEM objects. 3) Provide a common implementation to perform more complex mapping operations on the GPU VA space. In particular splitting and merging of GPU VA mappings, e.g. for intersecting mapping requests or partial unmap requests. The new VM_BIND Nouveau UAPI build on top of the DRM GPUVA manager, itself providing the following new interfaces. 1) Initialize a GPU VA space via the new DRM_IOCTL_NOUVEAU_VM_INIT ioctl for UMDs to specify the portion of VA space managed by the kernel and userspace, respectively. 2) Allocate and free a VA space region as well as bind and unbind memory to the GPUs VA space via the new DRM_IOCTL_NOUVEAU_VM_BIND ioctl. 3) Execute push buffers with the new DRM_IOCTL_NOUVEAU_EXEC ioctl. Both, DRM_IOCTL_NOUVEAU_VM_BIND and DRM_IOCTL_NOUVEAU_EXEC, make use of the DRM scheduler to queue jobs and support asynchronous processing with DRM syncobjs as synchronization mechanism. By default DRM_IOCTL_NOUVEAU_VM_BIND does synchronous processing, DRM_IOCTL_NOUVEAU_EXEC supports asynchronous processing only. The new VM_BIND UAPI for Nouveau makes also use of drm_exec (execution context for GEM buffers) by Christian König. Since the patch implementing drm_exec was not yet merged into drm-next it is part of this series, as well as a small fix for this patch, which was found while testing this series. This patch series is also available at [1]. There is a Mesa NVK merge request by Dave Airlie [2] implementing the corresponding userspace parts for this series. The Vulkan CTS test suite passes the sparse binding and sparse residency test cases for the new UAPI together with Dave's Mesa work. There are also some test cases in the igt-gpu-tools project [3] for the new UAPI and hence the DRM GPU VA manager. However, most of them are testing the DRM GPU VA manager's logic through Nouveau's new UAPI and should be considered just as helper for implementation. However, I absolutely intend to change those test cases to proper kunit test cases for the DRM GPUVA manager, once and if we agree on it's usefulness and design. [1] https://gitlab.freedesktop.org/nouvelles/kernel/-/tree/new-uapi-drm-next / https://gitlab.freedesktop.org/nouvelles/kernel/-/merge_requests/1 [2] https://gitlab.freedesktop.org/nouveau/mesa/-/merge_requests/150/ [3] https://gitlab.freedesktop.org/dakr/igt-gpu-tools/-/tree/wip_nouveau_vm_bind Changes in V2: == Nouveau: - Reworked the Nouveau VM_BIND UAPI to avoid memory allocations in fence signalling critical sections. Updates to the VA space are split up in three separate stages, where only the 2. stage executes in a fence signalling critical section: 1. update the VA space, allocate new structures and page tables 2. (un-)map the requested memory bindings 3. free structures and page tables - Separated generic job scheduler code from specific job implementations. - Separated the EXEC and VM_BIND implementation of the UAPI. - Reworked the locking parts of the nvkm/vmm RAW interface, such that (un-)map operations can be executed in fence signalling critical sections. GPUVA Manager: - made drm_gpuva_regions optional for users of the GPUVA manager - allow NULL GEMs for drm_gpuva entries - swichted from drm_mm to maple_tree for track drm_gpuva / drm_gpuva_region entries - provide callbacks for users to allocate custom drm_gpuva_op structures to allow inheritance - added user bits to drm_gpuva_flags - added a prefetch operation type in order to support generating prefetch operations in the same way other operations generated - hand the responsibility for mutual exclusion for a GEM's drm_gpuva list to the user; simplified corresponding (un-)link functions Maple Tree: - I added two maple tree patches to the series, one to support custom tree walk macros and one to hand the locking responsibility to the user of the GPUVA manager without pre-defined lockdep checks. Changes in V3: == Nouveau: - Reworked the Nouveau VM_BIND UAPI to do the job cleanup (including pa
RE: [PATCH v3] drm/i915: Refactor PAT/object cache handling
[snip]
>> @@ -27,15 +28,8 @@ static bool gpu_write_needs_clflush(struct
>> drm_i915_gem_object *obj)
>
> The code change here looks accurate, but while we're here, I have a side
> question about this function in general...it was originally introduced
> in commit 48004881f693 ("drm/i915: Mark CPU cache as dirty when used for
> rendering") which states that GPU rendering ends up in the CPU cache
> (and thus needs a clflush later to make sure it lands in memory). That
> makes sense to me for LLC platforms, but is it really true for non-LLC
> snooping platforms (like MTL) as the commit states?
For non-LLC platforms objects can be set to 1-way coherent which means
GPU rendering ending up in CPU cache as well, so for non-LLC platform
the logic here should be checking 1-way coherent flag.
> My understanding
> was that snooping platforms just invalidated the CPU cache to prevent
> future CPU reads from seeing stale data but didn't actually stick any
> new data in there? Am I off track or is the original logic of this
> function not quite right?
>
> Anyway, even if the logic of this function is wrong, it's a mistake that
> would only hurt performance
Yes, this logic will introduce performance impact because it's missing the
checking for obj->pat_set_by_user. For objects with pat_set_by_user==true,
even if the object is snooping or 1-way coherent, we don't want to enforce
a clflush here since the coherency is supposed to be handled by user space.
> (flushing more often than we truly need to)
> rather than functionality, so not something we really need to dig into
> right now as part of this patch.
>
>> if (IS_DGFX(i915))
>> return false;
>>
>> -/*
>> - * For objects created by userspace through GEM_CREATE with pat_index
>> - * set by set_pat extension, i915_gem_object_has_cache_level() will
>> - * always return true, because the coherency of such object is managed
>> - * by userspace. Othereise the call here would fall back to checking
>> - * whether the object is un-cached or write-through.
>> - */
>> -return !(i915_gem_object_has_cache_level(obj, I915_CACHE_NONE) ||
>> - i915_gem_object_has_cache_level(obj, I915_CACHE_WT));
>> +return i915_gem_object_has_cache_mode(obj, I915_CACHE_MODE_UC) != 1 &&
>> + i915_gem_object_has_cache_mode(obj, I915_CACHE_MODE_WT) != 1;
>> }
[snip]
>> @@ -640,15 +640,9 @@ static inline int use_cpu_reloc(const struct
>> reloc_cache *cache,
>> if (DBG_FORCE_RELOC == FORCE_GTT_RELOC)
>> return false;
>>
>> -/*
>> - * For objects created by userspace through GEM_CREATE with pat_index
>> - * set by set_pat extension, i915_gem_object_has_cache_level() always
>> - * return true, otherwise the call would fall back to checking whether
>> - * the object is un-cached.
>> - */
>> return (cache->has_llc ||
>> obj->cache_dirty ||
>> -!i915_gem_object_has_cache_level(obj, I915_CACHE_NONE));
>> +i915_gem_object_has_cache_mode(obj, I915_CACHE_MODE_UC) != 1);
>
> Platforms with relocations and platforms with user-specified PAT have no
> overlap, right? So a -1 return should be impossible here and this is
> one case where we could just treat the return value as a boolean, right?
My understanding is that the condition here means to say that, if GPU
access is uncached, don't use CPU reloc because the CPU cache might
contain stale data. This condition is sufficient for snooping platforms.
But from MTL onward, the condition show be whether the GPU access is
coherent with CPU. So, we should be checking 1-way coherent flag instead
of UC mode, because even if the GPU access is WB, it's still non-coherent,
thus CPU cache could be out-dated.
[snip]
>> @@ -208,12 +230,6 @@ bool i915_gem_object_can_bypass_llc(struct
>> drm_i915_gem_object *obj)
>> if (!(obj->flags & I915_BO_ALLOC_USER))
>> return false;
>>
>> -/*
>> - * Always flush cache for UMD objects at creation time.
>> - */
>> -if (obj->pat_set_by_user)
>> -return true;
>> -
I'm still worried that the removal of these lines would cause the
MESA failure seen before. I know you are checking pat index below, but
that is only about GPU access. It doesn't tell you how CPU is going to
access the memory. If user space is setting an uncached PAT, then use
copy engine to zero out the memory, but on the CPU side the mapping is
cacheable, you could still seeing garbage data.
I agree the lines don't belong here because it doesn't have anything
to do with LLC, but they need to be moved to the right location instead
of being removed.
>> /*
>> * EHL and JSL add the 'Bypass LLC' MOCS entry, which should make it
>> * possible for userspace to bypass the GTT caching bits set by the
>> @@ -226,7 +242,21 @@ bool i915_gem_object_can_bypass_llc(struct
>> drm_i915_gem_object *obj)
>> * it, but since i915 takes the stance of always zeroing me
Re: [RFC PATCH 00/10] Device Memory TCP
On Wed, Jul 19, 2023 at 10:57:11AM -0700, Stephen Hemminger wrote: > Naive idea. > Would it be possible for process to use mmap() on the GPU memory and then > do zero copy TCP receive some how? Or is this what is being proposed. It could be possible, but currently there is no API to recover the underlying dmabuf from the VMA backing the mmap. Also you can't just take arbitary struct pages from any old VMA and make them "netmem" Jason
Re: [PATCH v3 4/9] PCI/VGA: Improve the default VGA device selection
On 2023/7/20 03:32, Bjorn Helgaas wrote: but I think it's just confusing to mention this in the commit log, so I would just remove it. Ok, will be done at the next version.
Re: [PATCH v3 4/9] PCI/VGA: Improve the default VGA device selection
On 2023/7/20 06:32, suijingfeng wrote: it will be works no matter CONFIG_DRM_AST=m or CONFIG_DRM_AST=y It will be works regardless of CONFIG_DRM_AST=m or CONFIG_DRM_AST=y. When vgaarb call to the device driver, device driver already loaded successfully. and the PCI(e) device emulation already finished. So the last change the vgaarb gave us to override is actually happen very late. But it will be happen as long as the device driver get loaded successfully.
Re: [PATCH v3 4/9] PCI/VGA: Improve the default VGA device selection
Hi, On 2023/7/20 03:32, Bjorn Helgaas wrote: [+cc linux-pci (please cc in the future since the bulk of this patch is in drivers/pci/)] On Wed, Jul 12, 2023 at 12:43:05AM +0800, Sui Jingfeng wrote: From: Sui Jingfeng Currently, the strategy of selecting the default boot on a multiple video card coexistence system is not perfect. Potential problems are: 1) This function is a no-op on non-x86 architectures. Which function in particular is a no-op for non-x86? I refer to the vga_is_firmware_default() function, I will improve the commit message at the next version. (To make it more human readable). Thanks you point it out. 2) It does not take the PCI Bar may get relocated into consideration. 3) It is not effective for the PCI device without a dedicated VRAM Bar. 4) It is device-agnostic, thus it has to waste the effort to iterate all of the PCI Bar to find the VRAM aperture. 5) It has invented lots of methods to determine which one is the default boot device, but this is still a policy because it doesn't give the user a choice to override. I don't think we need a list of *potential* problems. We need an example of the specific problem this will solve, i.e., what currently does not work? 1) The selection of primary GPU on Non-x86 platform. (Arm64, risc-v, powerpc etc) Mostly server platforms have equipped with aspeed bmc, and such hardware platforms have a lot PCIe slot. So I think, aspeed bmc V.S (P.K) radeon(or amdgpu) is very common. 2) The ability to pass the control back to the end user. Convert the *device driven* to the "driver driven" or "human driven". Currently, it is the machine making the decision. Emm, I probably will be able to give some examples at the next version. The drm/ast and maybe drm/loongson patches are the only ones that use the new callback, so I assume there are real problems with those drivers. CONFIG_DRM_AST is a tristate. We're talking about identifying the boot-time console device. So if CONFIG_DRM_AST=m, I guess we don't get the benefit of the new callback unless the module gets loaded? Since, this patch set is mostly for the user of X server. It is actually okey if CONFIG_DRM_AST=m. (it will be works no matter CONFIG_DRM_AST=m or CONFIG_DRM_AST=y) As the device and the driver bound at a latter time. So we are lucky, we need this behavior to implement the override.
Re: [PATCH v3] drm/i915: Refactor PAT/object cache handling
On Wed, Jul 19, 2023 at 01:37:30PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> Commit 9275277d5324 ("drm/i915: use pat_index instead of cache_level") has
> introduced PAT indices to i915 internal APIs, partially replacing the
> usage of driver internal cache_level, but has also added a few
> questionable design decisions which this patch tries to improve upon.
>
> Principal change is to invert the per platform cache level to PAT index
> table which was added by the referenced commit, and by doing so enable
> i915 to understand the cache mode between PAT indices, changing them from
> opaque to transparent.
>
> Once we have the inverted table we are able to remove the hidden false
> "return true" from i915_gem_object_has_cache_level.
>
> Other changes/fixes/improvements we are able to do:
>
> 1)
> Replace the enum i915_cache_level with i915_cache_t, composed of a more
> detailed representation of each cache mode (base mode plus flags).
>
> For instance this way we are able to express the difference between WB and
> 1-way coherent WB on Meteorlake. Which in turn enables us to map the i915
> "cached" mode to the correct Meteorlake PAT index.
>
> 2)
> We can cache PAT indices of the caching modes used by the driver itself in
> struct drm_i915_private, which eliminates the runtime calls to
> i915_gem_get_pat_index from both high- and low-level i915 components.
>
> 3)
> We can also cache the caching modes used by the driver for coherent
> access and for display buffers.
>
> 4)
> Remove the incorrect references to enum i915_cache_level from low level
> PTE encode vfuncs, since those are actually given PAT indices by their
> callers.
>
> 5)
> Because i915 now understands PAT indices, we can remove the overly
> aggressive flushing triggered from i915_gem_object_can_bypass_llc() and
> limit it to non-coherent write-back mode only.
>
> 6)
> Finally we are able to replace the platform dependent cache mode to string
> code in debugfs and elsewhere by the single implementation based on
> i915_cache_t.
>
> v2:
> * Fix PAT-to-cache-mode table for PVC. (Fei)
> * Cache display caching mode too. (Fei)
> * Improve and document criteria in i915_gem_object_can_bypass_llc() (Matt)
>
> v3:
> * Checkpath issues.
> * Cache mode flags check fixed.
>
> Signed-off-by: Tvrtko Ursulin
> Fixes: 9275277d5324 ("drm/i915: use pat_index instead of cache_level")
> Cc: Chris Wilson
> Cc: Fei Yang
> Cc: Andi Shyti
> Cc: Matt Roper
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> .../drm/i915/display/intel_plane_initial.c| 3 +-
> drivers/gpu/drm/i915/gem/i915_gem_domain.c| 56 ---
> drivers/gpu/drm/i915/gem/i915_gem_domain.h| 5 +-
> .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 13 +-
> drivers/gpu/drm/i915/gem/i915_gem_internal.c | 4 +-
> drivers/gpu/drm/i915/gem/i915_gem_mman.c | 12 +-
> drivers/gpu/drm/i915/gem/i915_gem_object.c| 152 +++---
> drivers/gpu/drm/i915/gem/i915_gem_object.h| 11 +-
> .../gpu/drm/i915/gem/i915_gem_object_types.h | 116 +
> drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 8 +-
> drivers/gpu/drm/i915/gem/i915_gem_stolen.c| 11 +-
> drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 44 ++---
> drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 2 +-
> .../drm/i915/gem/selftests/huge_gem_object.c | 4 +-
> .../gpu/drm/i915/gem/selftests/huge_pages.c | 6 +-
> drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 4 +-
> drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 19 +--
> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +-
> drivers/gpu/drm/i915/gt/intel_ggtt.c | 33 ++--
> drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c | 4 +-
> drivers/gpu/drm/i915/gt/intel_gtt.c | 2 +-
> drivers/gpu/drm/i915/gt/intel_gtt.h | 3 +-
> drivers/gpu/drm/i915/gt/intel_migrate.c | 11 +-
> drivers/gpu/drm/i915/gt/intel_ppgtt.c | 6 +-
> .../gpu/drm/i915/gt/intel_ring_submission.c | 4 +-
> drivers/gpu/drm/i915/gt/intel_timeline.c | 2 +-
> drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 2 +-
> drivers/gpu/drm/i915/gt/selftest_migrate.c| 9 +-
> drivers/gpu/drm/i915/gt/selftest_reset.c | 14 +-
> drivers/gpu/drm/i915/gt/selftest_tlb.c| 5 +-
> .../gpu/drm/i915/gt/selftest_workarounds.c| 2 +-
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 8 +-
> drivers/gpu/drm/i915/i915_cache.c | 91 +++
> drivers/gpu/drm/i915/i915_cache.h | 60 +++
> drivers/gpu/drm/i915/i915_debugfs.c | 53 +-
> drivers/gpu/drm/i915/i915_driver.c| 5 +
> drivers/gpu/drm/i915/i915_drv.h | 5 +
> drivers/gpu/drm/i915/i915_gem.c | 21 +--
> drivers/gpu/drm/i915/i915_gpu_error.c | 7 +-
> drivers/gpu/drm/i915/i915_pci.c | 82 +-
> drivers/gpu/drm/i915/i915_perf.c | 2 +-
> drivers/gpu/drm/i915
Re: [PATCH v3 06/15] dt-bindings: display/msm: sc7180-dpu: Describe SM6125
On Thu, 20 Jul 2023 at 01:09, Marijn Suijten wrote: > > On 2023-07-19 01:06:03, Dmitry Baryshkov wrote: > > On 19/07/2023 00:24, Marijn Suijten wrote: > > > SM6125 is identical to SM6375 except that while downstream also defines > > > a throttle clock, its presence results in timeouts whereas SM6375 > > > requires it to not observe any timeouts. This is represented by > > > reducing the clock array length to 6 so that it cannot be passed. Note > > > that any SoC other than SM6375 (currently SC7180 and SM6350) are > > > unconstrained and could either pass or leave out this "throttle" clock. > > > > Could you please describe, what kind of timeouts do you observe? Is this > > the DSI underruns issue? > > Ping-pong timeouts and low(er) framerate. However, they were previosuly > not happening on a random boot out of tens... and now I can no longer > reproduce the timeout on 4 consecutive boots after adding the throttle > clock. Could it perhaps be the power domains and opps that we added in > v2 and v3? Quite unlikely, but who knows. My main question is whether we should continue skipping the throttle clocks or if it should be enabled now. > > We previously discussed in DMs that the rate was bouncing between 25MHz > and 403MHz without the clock specified, and with it it it got set at 385 > or 403MHz. Now, a month or so later, repeatedly running this command > shows 25MHz when the panel is not being refreshed, and between 337 and > 403MHz on modetest -r -v: > > sony-pdx201 ~ $ sudo ./debugcc -p sm6125 gcc_disp_throttle_core_clk > gcc_disp_throttle_core_clk: 337.848277MHz (337848277Hz) > > Either all these boots are flukes, or it is really fixed and this patch > should be revised... > > > If so, it might be fixed by the MDSS > > interconnect fix ([1]). > > > > [1] https://patchwork.freedesktop.org/series/116576/ > > Might have an effect but I don't have any interconnects defined in this > SoC DT yet. > > - Marijn > > > > Reviewed-by: Rob Herring > > > Signed-off-by: Marijn Suijten > > > --- > > > .../devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml | 14 > > > ++ > > > 1 file changed, 14 insertions(+) > > > > > > diff --git > > > a/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml > > > b/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml > > > index 630b11480496..37f66940c5e3 100644 > > > --- a/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml > > > +++ b/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml > > > @@ -15,6 +15,7 @@ properties: > > > compatible: > > > enum: > > > - qcom,sc7180-dpu > > > + - qcom,sm6125-dpu > > > - qcom,sm6350-dpu > > > - qcom,sm6375-dpu > > > > > > @@ -73,6 +74,19 @@ allOf: > > > clock-names: > > > minItems: 7 > > > > > > + - if: > > > + properties: > > > +compatible: > > > + const: qcom,sm6125-dpu > > > + > > > +then: > > > + properties: > > > +clocks: > > > + maxItems: 6 > > > + > > > +clock-names: > > > + maxItems: 6 > > > + > > > examples: > > > - | > > > #include > > > > > > > -- > > With best wishes > > Dmitry > > -- With best wishes Dmitry
Re: [PATCH v3 06/15] dt-bindings: display/msm: sc7180-dpu: Describe SM6125
On 2023-07-19 01:06:03, Dmitry Baryshkov wrote: > On 19/07/2023 00:24, Marijn Suijten wrote: > > SM6125 is identical to SM6375 except that while downstream also defines > > a throttle clock, its presence results in timeouts whereas SM6375 > > requires it to not observe any timeouts. This is represented by > > reducing the clock array length to 6 so that it cannot be passed. Note > > that any SoC other than SM6375 (currently SC7180 and SM6350) are > > unconstrained and could either pass or leave out this "throttle" clock. > > Could you please describe, what kind of timeouts do you observe? Is this > the DSI underruns issue? Ping-pong timeouts and low(er) framerate. However, they were previosuly not happening on a random boot out of tens... and now I can no longer reproduce the timeout on 4 consecutive boots after adding the throttle clock. Could it perhaps be the power domains and opps that we added in v2 and v3? We previously discussed in DMs that the rate was bouncing between 25MHz and 403MHz without the clock specified, and with it it it got set at 385 or 403MHz. Now, a month or so later, repeatedly running this command shows 25MHz when the panel is not being refreshed, and between 337 and 403MHz on modetest -r -v: sony-pdx201 ~ $ sudo ./debugcc -p sm6125 gcc_disp_throttle_core_clk gcc_disp_throttle_core_clk: 337.848277MHz (337848277Hz) Either all these boots are flukes, or it is really fixed and this patch should be revised... > If so, it might be fixed by the MDSS > interconnect fix ([1]). > > [1] https://patchwork.freedesktop.org/series/116576/ Might have an effect but I don't have any interconnects defined in this SoC DT yet. - Marijn > > Reviewed-by: Rob Herring > > Signed-off-by: Marijn Suijten > > --- > > .../devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml | 14 > > ++ > > 1 file changed, 14 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml > > b/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml > > index 630b11480496..37f66940c5e3 100644 > > --- a/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml > > +++ b/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml > > @@ -15,6 +15,7 @@ properties: > > compatible: > > enum: > > - qcom,sc7180-dpu > > + - qcom,sm6125-dpu > > - qcom,sm6350-dpu > > - qcom,sm6375-dpu > > > > @@ -73,6 +74,19 @@ allOf: > > clock-names: > > minItems: 7 > > > > + - if: > > + properties: > > +compatible: > > + const: qcom,sm6125-dpu > > + > > +then: > > + properties: > > +clocks: > > + maxItems: 6 > > + > > +clock-names: > > + maxItems: 6 > > + > > examples: > > - | > > #include > > > > -- > With best wishes > Dmitry >
Re: [PATCH v2 02/41] dt-bindings: display: Add Renesas SH-Mobile LCDC bindings
On Tue, 18 Jul 2023 18:54:07 +0200, Geert Uytterhoeven wrote: > Add device tree bindings for the LCD Controller (LCDC) found in Renesas > SuperH SH-Mobile and ARM SH/R-Mobile SOCs. > > Based on a plain text prototype by Laurent Pinchart. > > Signed-off-by: Geert Uytterhoeven > --- > Cc: Rob Herring > Cc: Krzysztof Kozlowski > Cc: Conor Dooley > Cc: devicet...@vger.kernel.org > > v2: > - Add myself as co-maintainer, > - Make fck clock required, > - Drop ports description referring to obsolete graph.txt, > - Condition ports to compatible strings, > - Drop label and status from example. > > Changes compared to Laurent's original: > - Convert to json-schema, > - Rename compatible values from "renesas,lcdc-" to > "renesas,-lcdc", > - Add power-domains property, > - Add MIPI-DSI port on SH-Mobile AG5, > - Update example to reflect reality, > - Add to MAINTAINERS. > --- > .../display/renesas,shmobile-lcdc.yaml| 130 ++ > MAINTAINERS | 1 + > 2 files changed, 131 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/renesas,shmobile-lcdc.yaml > Reviewed-by: Rob Herring
Re: [Intel-gfx] [PATCH v3 3/9] PCI/VGA: Switch to aperture_contain_firmware_fb_nonreloc()
Hi,
On 2023/7/20 04:43, Bjorn Helgaas wrote:
[+cc linux-pci; I don't apply or ack PCI patches unless they appear there]
On Wed, Jul 12, 2023 at 12:43:04AM +0800, Sui Jingfeng wrote:
From: Sui Jingfeng
The observation behind this is that we should avoid accessing the global
screen_info directly. Call the aperture_contain_firmware_fb_nonreloc()
function to implement the detection of whether an aperture contains the
firmware FB.
Because it's better to access the global screen_info from
aperture_contain_firmware_fb_nonreloc()? The reasoning here is not
super clear to me.
Yes, honestly the benefits of this patch is not obvious.
But I do have some (may not practical) ideas in my mind when I create
this patch.
See my explanation at the end.
This patch helps to decouple the determination from the implementation.
Or, in other words, we intend to make the determination opaque to the
caller. The determination may choose to be arch-dependent or
arch-independent. But vgaarb, as a consumer of the determination,
shouldn't care how the does determination is implemented.
"how the determination ..." (drop the "does")
Ok, will be fixed at the next version.
Are you saying that aperture_contain_firmware_fb_nonreloc() might be
arch-dependent? Are there multiple callers? Or does this just move
code from one place to a more appropriate place?
1) To form a unify approach, and drop the screen_info.h header.
There are similar cleanup patch at patchwork.
screen_info.h is definitely arch-dependent, while vgaarb is just
device-dependent.
I think, they do have subtle difference.
2) Convert the *device driven* to the "driver driven".
Move it from vgaarb.c to video/apperture allow code sharing.
While this function are not going to be shared in vgaarb.
Previous it is the device make the decision,
after applied this patch it allow driver make the decision.
They do have subtle difference.
Emm, I will try to give some examples at the next version.
3) I was imagine to drag platform display controllers in (get platform
devices involved in the arbitration).
As Alex seem hint to implement something platform-independent.
The aperture_contain_firmware_fb_nonreloc() actually is possible be shared.
The aperture of platform device will be not moved.
So it seems that platform device driver could call this function to do
something else.
Signed-off-by: Sui Jingfeng
---
drivers/pci/vgaarb.c | 19 ---
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index bf96e085751d..953daf731b2c 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -14,6 +14,7 @@
#define vgaarb_info(dev, fmt, arg...) dev_info(dev, "vgaarb: " fmt, ##arg)
#define vgaarb_err(dev, fmt, arg...) dev_err(dev, "vgaarb: " fmt, ##arg)
+#include
#include
#include
#include
@@ -26,7 +27,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -558,20 +558,11 @@ void vga_put(struct pci_dev *pdev, unsigned int rsrc)
}
EXPORT_SYMBOL(vga_put);
+/* Select the device owning the boot framebuffer if there is one */
static bool vga_is_firmware_default(struct pci_dev *pdev)
{
#if defined(CONFIG_X86) || defined(CONFIG_IA64)
- u64 base = screen_info.lfb_base;
- u64 size = screen_info.lfb_size;
struct resource *r;
- u64 limit;
-
- /* Select the device owning the boot framebuffer if there is one */
-
- if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE)
- base |= (u64)screen_info.ext_lfb_base << 32;
-
- limit = base + size;
/* Does firmware framebuffer belong to us? */
pci_dev_for_each_resource(pdev, r) {
@@ -581,10 +572,8 @@ static bool vga_is_firmware_default(struct pci_dev *pdev)
if (!r->start || !r->end)
continue;
- if (base < r->start || limit >= r->end)
- continue;
-
- return true;
+ if (aperture_contain_firmware_fb_nonreloc(r->start, r->end))
+ return true;
}
#endif
return false;
--
2.25.1
Re: [PATCH v3 02/15] arm64: dts: qcom: sm6125: Sort spmi_bus node numerically by reg
On 2023-07-19 01:02:56, Dmitry Baryshkov wrote:
> On 19/07/2023 00:24, Marijn Suijten wrote:
> > This node has always resided in the wrong spot, making it somewhat
> > harder to contribute new node entries while maintaining proper sorting
> > around it. Move the node up to sit after hsusb_phy1 where it maintains
> > proper numerical sorting on the (first of its many) reg address
> > property.
> >
> > Fixes: cff4bbaf2a2d ("arm64: dts: qcom: Add support for SM6125")
> > Reviewed-by: Konrad Dybcio
> > Signed-off-by: Marijn Suijten
> > ---
> > arch/arm64/boot/dts/qcom/sm6125.dtsi | 38
> > ++--
> > 1 file changed, 19 insertions(+), 19 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sm6125.dtsi
> > b/arch/arm64/boot/dts/qcom/sm6125.dtsi
> > index 6937c7ebdb81..cfd0901d4555 100644
> > --- a/arch/arm64/boot/dts/qcom/sm6125.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sm6125.dtsi
> > @@ -684,6 +684,24 @@ hsusb_phy1: phy@1613000 {
> > status = "disabled";
> > };
> >
> > + spmi_bus: spmi@1c4 {
> > + compatible = "qcom,spmi-pmic-arb";
> > + reg = <0x01c4 0x1100>,
> > + <0x01e0 0x200>,
> > + <0x03e0 0x10>,
> > + <0x03f0 0xa>,
> > + <0x01c0a000 0x26000>;
> > + reg-names = "core", "chnls", "obsrvr", "intr", "cnfg";
> > + interrupt-names = "periph_irq";
> > + interrupts = ;
> > + qcom,ee = <0>;
> > + qcom,channel = <0>;
> > + #address-cells = <2>;
> > + #size-cells = <0>;
> > + interrupt-controller;
> > + #interrupt-cells = <4>;
> > + };
> > +
> > rpm_msg_ram: sram@45f {
> > compatible = "qcom,rpm-msg-ram";
> > reg = <0x045f 0x7000>;
> > @@ -1189,27 +1207,9 @@ sram@469 {
> > reg = <0x0469 0x1>;
> > };
> >
> > - spmi_bus: spmi@1c4 {
> > - compatible = "qcom,spmi-pmic-arb";
> > - reg = <0x01c4 0x1100>,
> > - <0x01e0 0x200>,
> > - <0x03e0 0x10>,
> > - <0x03f0 0xa>,
> > - <0x01c0a000 0x26000>;
> > - reg-names = "core", "chnls", "obsrvr", "intr", "cnfg";
> > - interrupt-names = "periph_irq";
> > - interrupts = ;
> > - qcom,ee = <0>;
> > - qcom,channel = <0>;
> > - #address-cells = <2>;
> > - #size-cells = <0>;
> > - interrupt-controller;
> > - #interrupt-cells = <4>;
> > - };
> > -
> > apps_smmu: iommu@c60 {
> > compatible = "qcom,sm6125-smmu-500", "qcom,smmu-500",
> > "arm,mmu-500";
> > - reg = <0xc60 0x8>;
> > + reg = <0x0c60 0x8>;
>
> Irrelevant, please split.
This was already here in v1, and it is what likely contributed to the
sorting mismatch in the first place.
But will split it and send a v4 for just this...
- Marijn
Re: [PATCH v2 10/15] dt-bindings: msm: dsi-phy-14nm: Document SM6125 variant
On 2023-07-19 01:01:54, Dmitry Baryshkov wrote: > On 19/07/2023 00:00, Marijn Suijten wrote: > > On 2023-06-29 13:54:13, Dmitry Baryshkov wrote: > >> On 27/06/2023 23:14, Marijn Suijten wrote: > >>> Document availability of the 14nm DSI PHY on SM6125. Note that this > >>> compatible uses the SoC-suffix variant, intead of postfixing an > >>> arbitrary number without the sm/sdm portion. The PHY is not powered by > >>> a vcca regulator like on most SoCs, but by the MX power domain that is > >>> provided via the power-domains property and a single corresponding > >>> required-opps. > >>> > >>> Acked-by: Krzysztof Kozlowski > >>> Signed-off-by: Marijn Suijten > >>> --- > >>>.../devicetree/bindings/display/msm/dsi-phy-14nm.yaml | 11 > >>> +++ > >>>1 file changed, 11 insertions(+) > >>> > >>> diff --git > >>> a/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml > >>> b/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml > >>> index a43e11d3b00d..183a26f8a6dc 100644 > >>> --- a/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml > >>> +++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml > >>> @@ -19,6 +19,7 @@ properties: > >>> - qcom,dsi-phy-14nm-2290 > >>> - qcom,dsi-phy-14nm-660 > >>> - qcom,dsi-phy-14nm-8953 > >>> + - qcom,sm6125-dsi-phy-14nm > >>> > >>> reg: > >>>items: > >>> @@ -35,6 +36,16 @@ properties: > >>> vcca-supply: > >>>description: Phandle to vcca regulator device node. > >>> > >>> + power-domains: > >>> +description: > >>> + A phandle and PM domain specifier for an optional power domain. > >>> +maxItems: 1 > >>> + > >>> + required-opps: > >>> +description: > >>> + A phandle to an OPP node describing an optional performance point. > >> > >> I'd rephrase this to be something more exact, like 'desribing power > >> domain's performance point'. > > > > Sure. I'll leave out the word "optional", that becomes obvious from > > maxItems:1 without minItems, together with referencing a PM which itself > > is already optional. > > no, default minItems is equal to maxItems. It is not listing this > property under the required what makes it optional. I thought it was both. Magic. - Marijn
Re: [PATCH 2/6] PCI/VGA: Deal with PCI VGA compatible devices only
Hi, On 2023/7/20 05:13, Sui Jingfeng wrote: Otherwise there 30+ noisy(useless) events got snooped. See below: ``` [ 0.246077] pci :01:00.0: vgaarb: setting as boot VGA device [ 0.246077] pci :01:00.0: vgaarb: bridge control possible [ 0.246077] pci :01:00.0: vgaarb: VGA device added: decodes=io+mem,owns=io+mem,locks=none [ 0.246077] vgaarb: loaded [ 0.294169] skl_uncore :00:00.0: vgaarb: pci_notify: action=3 [ 0.294182] skl_uncore :00:00.0: vgaarb: pci_notify: action=4 [ 0.301297] pcieport :00:01.0: vgaarb: pci_notify: action=3 [ 0.301482] pcieport :00:01.0: vgaarb: pci_notify: action=4 [ 0.301488] pcieport :00:1c.0: vgaarb: pci_notify: action=3 [ 0.301705] pcieport :00:1c.0: vgaarb: pci_notify: action=4 [ 1.806445] xhci_hcd :00:14.0: vgaarb: pci_notify: action=3 [ 1.810976] ahci :00:17.0: vgaarb: pci_notify: action=3 [ 1.824383] xhci_hcd :00:14.0: vgaarb: pci_notify: action=4 [ 1.857470] ahci :00:17.0: vgaarb: pci_notify: action=4 [ 4.692700] intel_pch_thermal :00:14.2: vgaarb: pci_notify: action=3 [ 4.693110] intel_pch_thermal :00:14.2: vgaarb: pci_notify: action=4 [ 4.746712] i801_smbus :00:1f.4: vgaarb: pci_notify: action=3 [ 4.747212] pci :00:1f.1: vgaarb: pci_notify: action=0 [ 4.747227] pci :00:1f.1: vgaarb: pci_notify: action=1 [ 4.747250] pci :00:1f.1: vgaarb: pci_notify: action=2 [ 4.749098] i801_smbus :00:1f.4: vgaarb: pci_notify: action=4 [ 4.799217] mei_me :00:16.0: vgaarb: pci_notify: action=3 [ 4.802503] mei_me :00:16.0: vgaarb: pci_notify: action=4 [ 4.874880] intel-lpss :00:15.0: vgaarb: pci_notify: action=3 [ 4.881227] intel-lpss :00:15.0: vgaarb: pci_notify: action=4 [ 4.881240] intel-lpss :00:15.1: vgaarb: pci_notify: action=3 [ 4.887578] intel-lpss :00:15.1: vgaarb: pci_notify: action=4 [ 4.985796] r8169 :02:00.0: vgaarb: pci_notify: action=3 [ 4.991862] r8169 :02:00.0: vgaarb: pci_notify: action=4 [ 5.404835] snd_hda_intel :00:1f.3: vgaarb: pci_notify: action=3 [ 5.405175] snd_hda_intel :00:1f.3: vgaarb: pci_notify: action=4 [ 5.405401] snd_hda_intel :01:00.1: vgaarb: pci_notify: action=3 [ 5.405973] snd_hda_intel :01:00.1: vgaarb: pci_notify: action=4 [ 10.793665] i915 :00:02.0: vgaarb: pci_notify: action=3 [ 11.201384] i915 :00:02.0: vgaarb: pci_notify: action=4 [ 16.135842] amdgpu :01:00.0: vgaarb: pci_notify: action=3 [ 16.140458] amdgpu :01:00.0: vgaarb: deactivate vga console [ 16.638564] amdgpu :01:00.0: vgaarb: pci_notify: action=4 ``` After apply my patch, this events are still will notify me, it is just that if we found it is irrelevant, we will return immediately. No further process is needed.
[Bug 217664] Laptop doesnt wake up from suspend mode.
https://bugzilla.kernel.org/show_bug.cgi?id=217664 --- Comment #6 from popus_czy_to_ty (pentelja...@o2.pl) --- tested debian 12 - dead tested mx linux with card choose during instalation, chosen amdgpu - dead -- 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 v3 1/9] video/aperture: Add a helper to detect if an aperture contains firmware FB
Hi, On 2023/7/20 04:43, Bjorn Helgaas wrote: On Wed, Jul 12, 2023 at 12:43:02AM +0800, Sui Jingfeng wrote: From: Sui Jingfeng This patch adds the aperture_contain_firmware_fb() function to do the determination. Unfortunately, due to the fact that the apertures list will be freed dynamically, the location and size information of the firmware FB will be lost after dedicated drivers call aperture_remove_conflicting_devices(), aperture_remove_conflicting_pci_devices() or aperture_remove_all_conflicting_devices() functions We solve this problem by introducing two static variables that record the firmware framebuffer's start addrness and end addrness. It assumes that the system has only one active firmware framebuffer driver at a time. We don't use the global structure screen_info here, because PCI resources may get reallocated (the VRAM BAR could be moved) during the kernel boot stage. s/addrness/address/ (twice) Will be fixed at the next version, thanks.
Re: [PATCH 2/6] PCI/VGA: Deal with PCI VGA compatible devices only
Hi,
On 2023/7/20 02:26, Bjorn Helgaas wrote:
On Tue, Jul 11, 2023 at 09:43:50PM +0800, Sui Jingfeng wrote:
From: Sui Jingfeng
Currently, vgaarb only cares about PCI VGA-compatible class devices.
While vga_arbiter_del_pci_device() gets called unbalanced when some PCI
device is about to be removed. This happens even during the boot process.
The previous code calls vga_arbiter_add_pci_device() for every device
(every device present at boot and also every hot-added device). It
only allocates a vga_device if pdev->class is 0x0300XX.
It calls vga_arbiter_del_pci_device() for every device removal. It
does nothing unless it finds a vga_device.
This seems symmetric and reasonable to me. Did you observe a problem
with it?
Not big deal, but the vgaarb does do some useless work there.
Right, it calls vga_arbiter_del_pci_device() for every device removal.
And it can not finds a vga_device at the most time. (Because on normal
case, a user only have one or two GPU device in the system.)
But even it can not finds a vga_device, vga_arbiter_del_pci_device()
still brings
additional(and it is unnecessary) overheads.
For an example, on my i3-8100 (the motherboard model is H110 D4L) machine,
The PCI device(:00:1f.1) will trigger the call to
vga_arbiter_del_pci_device().
Even though it can not finds a vga_device,
vga_arbiter_del_pci_device() is *NOT* a no-op still.
```
static bool vga_arbiter_del_pci_device(struct pci_dev *pdev)
{
struct vga_device *vgadev;
unsigned long flags;
bool ret = true;
spin_lock_irqsave(&vga_lock, flags);
vgadev = vgadev_find(pdev);
if (vgadev == NULL) {
ret = false;
goto bail;
}
// omit ...
bail:
spin_unlock_irqrestore(&vga_lock, flags);
kfree(vgadev);
return ret;
}
```
1) It call spin_lock_irqsave() and spin_unlock_irqrestore() pair for
complete irrelevant PCI devices
2) It try to find a vgadev with pdev pointer, which have to search the
whole list (All nodes in the list got accessed), because it can not find.
3) It call kfree() to free NULL pointer, it's just that kfree() will
just return if you pass a NULL, so no bug happen.
It is not efficient.
While the major contribution of my patch is to filter irrelevant PCI device.
Otherwise there 30+ noisy(useless) events got snooped. See below:
```
[ 0.246077] pci :01:00.0: vgaarb: setting as boot VGA device
[ 0.246077] pci :01:00.0: vgaarb: bridge control possible
[ 0.246077] pci :01:00.0: vgaarb: VGA device added:
decodes=io+mem,owns=io+mem,locks=none
[ 0.246077] vgaarb: loaded
[ 0.294169] skl_uncore :00:00.0: vgaarb: pci_notify: action=3
[ 0.294182] skl_uncore :00:00.0: vgaarb: pci_notify: action=4
[ 0.301297] pcieport :00:01.0: vgaarb: pci_notify: action=3
[ 0.301482] pcieport :00:01.0: vgaarb: pci_notify: action=4
[ 0.301488] pcieport :00:1c.0: vgaarb: pci_notify: action=3
[ 0.301705] pcieport :00:1c.0: vgaarb: pci_notify: action=4
[ 1.806445] xhci_hcd :00:14.0: vgaarb: pci_notify: action=3
[ 1.810976] ahci :00:17.0: vgaarb: pci_notify: action=3
[ 1.824383] xhci_hcd :00:14.0: vgaarb: pci_notify: action=4
[ 1.857470] ahci :00:17.0: vgaarb: pci_notify: action=4
[ 4.692700] intel_pch_thermal :00:14.2: vgaarb: pci_notify: action=3
[ 4.693110] intel_pch_thermal :00:14.2: vgaarb: pci_notify: action=4
[ 4.746712] i801_smbus :00:1f.4: vgaarb: pci_notify: action=3
[ 4.747212] pci :00:1f.1: vgaarb: pci_notify: action=0
[ 4.747227] pci :00:1f.1: vgaarb: pci_notify: action=1
[ 4.747250] pci :00:1f.1: vgaarb: pci_notify: action=2
[ 4.749098] i801_smbus :00:1f.4: vgaarb: pci_notify: action=4
[ 4.799217] mei_me :00:16.0: vgaarb: pci_notify: action=3
[ 4.802503] mei_me :00:16.0: vgaarb: pci_notify: action=4
[ 4.874880] intel-lpss :00:15.0: vgaarb: pci_notify: action=3
[ 4.881227] intel-lpss :00:15.0: vgaarb: pci_notify: action=4
[ 4.881240] intel-lpss :00:15.1: vgaarb: pci_notify: action=3
[ 4.887578] intel-lpss :00:15.1: vgaarb: pci_notify: action=4
[ 4.985796] r8169 :02:00.0: vgaarb: pci_notify: action=3
[ 4.991862] r8169 :02:00.0: vgaarb: pci_notify: action=4
[ 5.404835] snd_hda_intel :00:1f.3: vgaarb: pci_notify: action=3
[ 5.405175] snd_hda_intel :00:1f.3: vgaarb: pci_notify: action=4
[ 5.405401] snd_hda_intel :01:00.1: vgaarb: pci_notify: action=3
[ 5.405973] snd_hda_intel :01:00.1: vgaarb: pci_notify: action=4
[ 10.793665] i915 :00:02.0: vgaarb: pci_notify: action=3
[ 11.201384] i915 :00:02.0: vgaarb: pci_notify: action=4
[ 16.135842] amdgpu :01:00.0: vgaarb: pci_notify: action=3
[ 16.140458] amdgpu :01:00.0: vgaarb: deactivate vga console
[ 16.638564] amdgpu :01:00.0: vgaarb: pci_notify: action=4
```
Re: [PATCH] drm/pl111: Fix missing unwind goto in pl111_amba_probe()
Harshit Mogalapalli writes:
> Smatch reports:
> drivers/gpu/drm/pl111/pl111_drv.c:300
> pl111_amba_probe() warn: missing unwind goto?
>
> When devm_request_irq() returns non-zero value, we need to drop the
> reference for drm device and also release reserved memory which is
> done in "dev_put" label. So instead of directly returning, goto dev_put
> to fix this bug.
>
> Fixes: bed41005e617 ("drm/pl111: Initial drm/kms driver for pl111")
> Signed-off-by: Harshit Mogalapalli
> ---
> This is based on static analysis, Only Compile tested.
> ---
> drivers/gpu/drm/pl111/pl111_drv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
The patch looks correct to me.
Reviewed-by: Javier Martinez Canillas
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
Re: [PATCH] drm/shmem-helper: Remove duplicate include
Jiapeng Chong writes: > ./drivers/gpu/drm/drm_gem_shmem_helper.c: linux/module.h is included more > than once. > > Reported-by: Abaci Robot > Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=4567 > Signed-off-by: Jiapeng Chong > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH v3 1/9] video/aperture: Add a helper to detect if an aperture contains firmware FB
On Wed, Jul 12, 2023 at 12:43:02AM +0800, Sui Jingfeng wrote: > From: Sui Jingfeng > > This patch adds the aperture_contain_firmware_fb() function to do the > determination. Unfortunately, due to the fact that the apertures list > will be freed dynamically, the location and size information of the > firmware FB will be lost after dedicated drivers call > aperture_remove_conflicting_devices(), > aperture_remove_conflicting_pci_devices() or > aperture_remove_all_conflicting_devices() functions > We solve this problem by introducing two static variables that record the > firmware framebuffer's start addrness and end addrness. It assumes that the > system has only one active firmware framebuffer driver at a time. We don't > use the global structure screen_info here, because PCI resources may get > reallocated (the VRAM BAR could be moved) during the kernel boot stage. s/addrness/address/ (twice)
Re: [Intel-gfx] [PATCH v3 3/9] PCI/VGA: Switch to aperture_contain_firmware_fb_nonreloc()
[+cc linux-pci; I don't apply or ack PCI patches unless they appear there]
On Wed, Jul 12, 2023 at 12:43:04AM +0800, Sui Jingfeng wrote:
> From: Sui Jingfeng
>
> The observation behind this is that we should avoid accessing the global
> screen_info directly. Call the aperture_contain_firmware_fb_nonreloc()
> function to implement the detection of whether an aperture contains the
> firmware FB.
Because it's better to access the global screen_info from
aperture_contain_firmware_fb_nonreloc()? The reasoning here is not
super clear to me.
> This patch helps to decouple the determination from the implementation.
> Or, in other words, we intend to make the determination opaque to the
> caller. The determination may choose to be arch-dependent or
> arch-independent. But vgaarb, as a consumer of the determination,
> shouldn't care how the does determination is implemented.
"how the determination ..." (drop the "does")
Are you saying that aperture_contain_firmware_fb_nonreloc() might be
arch-dependent? Are there multiple callers? Or does this just move
code from one place to a more appropriate place?
> Signed-off-by: Sui Jingfeng
> ---
> drivers/pci/vgaarb.c | 19 ---
> 1 file changed, 4 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
> index bf96e085751d..953daf731b2c 100644
> --- a/drivers/pci/vgaarb.c
> +++ b/drivers/pci/vgaarb.c
> @@ -14,6 +14,7 @@
> #define vgaarb_info(dev, fmt, arg...)dev_info(dev, "vgaarb: " fmt,
> ##arg)
> #define vgaarb_err(dev, fmt, arg...) dev_err(dev, "vgaarb: " fmt, ##arg)
>
> +#include
> #include
> #include
> #include
> @@ -26,7 +27,6 @@
> #include
> #include
> #include
> -#include
> #include
> #include
> #include
> @@ -558,20 +558,11 @@ void vga_put(struct pci_dev *pdev, unsigned int rsrc)
> }
> EXPORT_SYMBOL(vga_put);
>
> +/* Select the device owning the boot framebuffer if there is one */
> static bool vga_is_firmware_default(struct pci_dev *pdev)
> {
> #if defined(CONFIG_X86) || defined(CONFIG_IA64)
> - u64 base = screen_info.lfb_base;
> - u64 size = screen_info.lfb_size;
> struct resource *r;
> - u64 limit;
> -
> - /* Select the device owning the boot framebuffer if there is one */
> -
> - if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE)
> - base |= (u64)screen_info.ext_lfb_base << 32;
> -
> - limit = base + size;
>
> /* Does firmware framebuffer belong to us? */
> pci_dev_for_each_resource(pdev, r) {
> @@ -581,10 +572,8 @@ static bool vga_is_firmware_default(struct pci_dev *pdev)
> if (!r->start || !r->end)
> continue;
>
> - if (base < r->start || limit >= r->end)
> - continue;
> -
> - return true;
> + if (aperture_contain_firmware_fb_nonreloc(r->start, r->end))
> + return true;
> }
> #endif
> return false;
> --
> 2.25.1
>
Re: [PATCH 1/2] drm/mipi-dbi: Lock SPI bus before setting D/C GPIO
Hi Otto,
kernel test robot noticed the following build errors:
[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on drm/drm-next drm-exynos/exynos-drm-next
drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip
linus/master v6.5-rc2 next-20230719]
[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#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/Otto-Pfl-ger/drm-mipi-dbi-Lock-SPI-bus-before-setting-D-C-GPIO/20230719-180941
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:
https://lore.kernel.org/r/20230719095343.88359-2-otto.pflueger%40abscue.de
patch subject: [PATCH 1/2] drm/mipi-dbi: Lock SPI bus before setting D/C GPIO
config: riscv-randconfig-r042-20230720
(https://download.01.org/0day-ci/archive/20230720/202307200456.jgqv0osc-...@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git
4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce:
(https://download.01.org/0day-ci/archive/20230720/202307200456.jgqv0osc-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202307200456.jgqv0osc-...@intel.com/
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/tiny/ili9225.c:321:54: error: too few arguments to function
>> call, expected 6, have 5
321 | ret = mipi_dbi_spi_transfer(spi, speed_hz, 8, cmd, 1);
| ~ ^
include/drm/drm_mipi_dbi.h:187:5: note: 'mipi_dbi_spi_transfer' declared here
187 | int mipi_dbi_spi_transfer(struct spi_device *spi, u32 speed_hz,
| ^
drivers/gpu/drm/tiny/ili9225.c:331:59: error: too few arguments to function
call, expected 6, have 5
331 | return mipi_dbi_spi_transfer(spi, speed_hz, bpw, par, num);
|~ ^
include/drm/drm_mipi_dbi.h:187:5: note: 'mipi_dbi_spi_transfer' declared here
187 | int mipi_dbi_spi_transfer(struct spi_device *spi, u32 speed_hz,
| ^
2 errors generated.
vim +321 drivers/gpu/drm/tiny/ili9225.c
b57e8b7661e046 drivers/gpu/drm/tinydrm/ili9225.c David Lechner 2017-11-19 310
36b5057216236a drivers/gpu/drm/tinydrm/ili9225.c Noralf Trønnes 2019-07-22 311
static int ili9225_dbi_command(struct mipi_dbi *dbi, u8 *cmd, u8 *par,
b57e8b7661e046 drivers/gpu/drm/tinydrm/ili9225.c David Lechner 2017-11-19 312
size_t num)
b57e8b7661e046 drivers/gpu/drm/tinydrm/ili9225.c David Lechner 2017-11-19 313
{
36b5057216236a drivers/gpu/drm/tinydrm/ili9225.c Noralf Trønnes 2019-07-22 314
struct spi_device *spi = dbi->spi;
b57e8b7661e046 drivers/gpu/drm/tinydrm/ili9225.c David Lechner 2017-11-19 315
unsigned int bpw = 8;
b57e8b7661e046 drivers/gpu/drm/tinydrm/ili9225.c David Lechner 2017-11-19 316
u32 speed_hz;
b57e8b7661e046 drivers/gpu/drm/tinydrm/ili9225.c David Lechner 2017-11-19 317
int ret;
b57e8b7661e046 drivers/gpu/drm/tinydrm/ili9225.c David Lechner 2017-11-19 318
36b5057216236a drivers/gpu/drm/tinydrm/ili9225.c Noralf Trønnes 2019-07-22 319
gpiod_set_value_cansleep(dbi->dc, 0);
b57e8b7661e046 drivers/gpu/drm/tinydrm/ili9225.c David Lechner 2017-11-19 320
speed_hz = mipi_dbi_spi_cmd_max_speed(spi, 1);
d23d4d4dac0119 drivers/gpu/drm/tinydrm/ili9225.c Noralf Trønnes 2019-07-19 @321
ret = mipi_dbi_spi_transfer(spi, speed_hz, 8, cmd, 1);
b57e8b7661e046 drivers/gpu/drm/tinydrm/ili9225.c David Lechner 2017-11-19 322
if (ret || !num)
b57e8b7661e046 drivers/gpu/drm/tinydrm/ili9225.c David Lechner 2017-11-19 323
return ret;
b57e8b7661e046 drivers/gpu/drm/tinydrm/ili9225.c David Lechner 2017-11-19 324
36b5057216236a drivers/gpu/drm/tinydrm/ili9225.c Noralf Trønnes 2019-07-22 325
if (*cmd == ILI9225_WRITE_DATA_TO_GRAM && !dbi->swap_bytes)
b57e8b7661e046 drivers/gpu/drm/tinydrm/ili9225.c David Lechner 2017-11-19 326
bpw = 16;
b57e8b7661e046 drivers/gpu/drm/tinydrm/ili9225.c David Lechner 2017-11-19 327
36b5057216236a drivers/gpu/drm/tinydrm/ili9225.c Noralf Trønnes 2019-07-22 328
gpiod_set_value_cansleep(dbi->dc, 1);
b57e8b7661e046 drivers/gpu/drm/tinydrm/ili9225.c David Lechner 2017-11-19 329
speed_hz = mipi_dbi_spi_cmd_max_speed(spi, num);
b57e8b7661e046 drivers/gpu/drm/tinydrm/ili9225.c David Lechner 2017-11-19 330
d23d4d4dac0119 drivers/gpu/drm/tinydrm/ili9225.c Noralf Trønnes 2019-07-19 331
return mipi_dbi_spi_transfer(s
Re: [RFC PATCH 00/10] Device Memory TCP
On Wed, 19 Jul 2023 08:10:58 -0700 Mina Almasry wrote: > From Jakub and David's comments it sounds (if I understood correctly), > you'd like to tie the dma-buf bind/unbind functions to the lifetime of > a netlink socket, rather than a struct file like I was thinking. That > does sound cleaner, but I'm not sure how. Can you link me to any > existing code examples? Or rough pointers to any existing code? I don't have a strong preference whether the lifetime is bound to the socket or not. My main point was that if we're binding lifetimes to processes, it should be done via netlink sockets, not special- -purpose FDs. Inevitably more commands and info will be needed and we'll start reinventing the uAPI wheel which is Netlink. Currently adding state to netlink sockets is a bit raw. You can create an Xarray which stores the per socket state using socket's portid (genl_info->snd_portid) and use netlink_register_notifier() to get notifications when sockets are closed.
Re: [RFC v5 00/17] DRM cgroup controller with scheduling control and memory stats
On Wed, Jul 12, 2023 at 4:47 AM Tvrtko Ursulin wrote: > > drm.memory.stat > A nested file containing cumulative memory statistics for the whole > sub-hierarchy, broken down into separate GPUs and separate memory > regions supported by the latter. > > For example:: > > $ cat drm.memory.stat > card0 region=system total=12898304 shared=0 active=0 > resident=12111872 purgeable=167936 > card0 region=stolen-system total=0 shared=0 active=0 resident=0 > purgeable=0 > > Card designation corresponds to the DRM device names and multiple line > entries can be present per card. > > Memory region names should be expected to be driver specific with the > exception of 'system' which is standardised and applicable for GPUs > which can operate on system memory buffers. > > Sub-keys 'resident' and 'purgeable' are optional. > > Per category region usage is reported in bytes. > > * Feedback from people interested in drm.active_us and drm.memory.stat is >required to understand the use cases and their usefulness (of the fields). > >Memory stats are something which was easy to add to my series, since I was >already working on the fdinfo memory stats patches, but the question is how >useful it is. > Hi Tvrtko, I think this style of driver-defined categories for reporting of memory could potentially allow us to eliminate the GPU memory tracking tracepoint used on Android (gpu_mem_total). This would involve reading drm.memory.stat at the root cgroup (I see it's currently disabled on the root), which means traversing the whole cgroup tree under the cgroup lock to generate the values on-demand. This would be done rarely, but I still wonder what the cost of that would turn out to be. The drm_memory_stats categories in the output don't seem like a big value-add for this use-case, but no real objection to them being there. I know it's called the DRM cgroup controller, but it'd be nice if there were a way to make the mem tracking part work for any driver that wishes to participate as many of our devices don't use a DRM driver. But making that work doesn't look like it would fit very cleanly into this controller, so I'll just shut up now. Thanks! -T.J.
Re: [PATCH 2/6] PCI/VGA: Deal with PCI VGA compatible devices only
On 2023/7/20 03:58, Sui Jingfeng wrote: On the other hand, even though the lest significant 8 but if pdev->class is really matter. If the low eight bits of pdev->class is really matters, maybe we should wait the potential problems became severe. Currently, it is not obvious.
Re: [PATCH v5 1/2] dt-bindings: display/msm: mdss-common: add memory-region property
On Thu, 13 Jul 2023 22:22:37 +0530, Amit Pundir wrote: > Add and document the reserved memory region property in the > mdss-common schema. > > For now (sdm845-db845c), it points to a framebuffer memory > region reserved by the bootloader for splash screen. > > Signed-off-by: Amit Pundir > --- > v5: Moving the dt-binding to mdss-common schema with > updated commit message and property description. > > v4: Adding this new dt-binding patch, in qcom,sdm845-mdss > schema, in the v4 of the follow-up patch for > sdm845-db845c. > > https://lore.kernel.org/lkml/20230712130215.666924-2-amit.pun...@linaro.org/ > > .../devicetree/bindings/display/msm/mdss-common.yaml | 5 + > 1 file changed, 5 insertions(+) > Reviewed-by: Rob Herring
Re: [PATCH 11/11] drm/vc4: tests: pv-muxing: Document test scenario
Maxime Ripard writes: > We've had a couple of tests that weren't really obvious, nor did they > document what they were supposed to test. Document that to make it > hopefully more obvious. > > Signed-off-by: Maxime Ripard > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH 2/6] PCI/VGA: Deal with PCI VGA compatible devices only
On 2023/7/20 03:58, Sui Jingfeng wrote: My explanation about the minor tweak being made before this version and previous version is that I want to keep my patch *less distraction*. The minor tweak being made between this version and previous version is to keep my patch *less distraction*.
Re: [PATCH 10/11] drm/vc4: tests: Switch to atomic state allocation helper
Maxime Ripard writes: > Now that we have a helper that takes care of an atomic state allocation > and cleanup, we can migrate to it to simplify our tests. > > Signed-off-by: Maxime Ripard > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH 2/6] PCI/VGA: Deal with PCI VGA compatible devices only
On 2023/7/20 03:58, Sui Jingfeng wrote: What this version adds here is *same* before this patch set is applied. The filter method is *same* , in the cases of before this patch is applied and after this patch is applied.
Re: [PATCH 09/11] drm/vc4: tests: pv-muxing: Switch to managed locking init
Maxime Ripard writes: > The new helper to init the locking context allows to remove some > boilerplate. > > Signed-off-by: Maxime Ripard > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH 08/11] drm/vc4: tests: mock: Use a kunit action to unregister DRM device
Maxime Ripard writes: > The *_mock_device functions allocate a DRM device that needs to be > released using drm_dev_unregister. > > Now that we have a kunit release action API, we can switch to it and > don't require any kind of garbage collection from the caller. > > Signed-off-by: Maxime Ripard > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH 07/11] drm/vc4: tests: pv-muxing: Remove call to drm_kunit_helper_free_device()
Maxime Ripard writes: > Calling drm_kunit_helper_free_device() to clean up the resources > allocated by drm_kunit_helper_alloc_device() is now optional and not > needed in most cases. > > Remove it. > > Signed-off-by: Maxime Ripard > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH 06/11] drm/tests: helpers: Create an helper to allocate an atomic state
Maxime Ripard writes: > As we gain more tests, boilerplate to allocate an atomic state and free > it starts to be there more and more as well. > > In order to reduce the allocation boilerplate, we can create an helper > to create that atomic state, and call an action when the test is done. > This will also clean up the exit path. > > Signed-off-by: Maxime Ripard > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH 2/6] PCI/VGA: Deal with PCI VGA compatible devices only
Hi,
On 2023/7/20 02:26, Bjorn Helgaas wrote:
On Tue, Jul 11, 2023 at 09:43:50PM +0800, Sui Jingfeng wrote:
[...]
Reviewed-by: Mario Limonciello
Signed-off-by: Sui Jingfeng
I do not see Mario's Reviewed-by on the list. I do see Mario's
Reviewed-by [2] for a previous version, but that version added this in
pci_notify():
+ if (pdev->class != PCI_CLASS_DISPLAY_VGA << 8)
+ return 0;
while this version adds:
+ if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
+ return 0;
It's OK to carry a review to future versions if there are
insignificant changes, but this is a functional change that seems
significant to me. The first matches only 0x03, while the second
discards the low eight bits so it matches 0x0300XX.
Yes, you are right.
But I suddenly realized that this may deserve another patch, desperate
trivial.
What this version adds here is *same* before this patch set is applied.
My explanation about the minor tweak being made before this version and
previous version
is that I want to keep my patch *less distraction*.
The major functional gains(benefit) is that we filter non VGA compatible
devices out.
As a start point, I should keep one patch do one thing (do one thing and
do it well).
On the other hand, even though the lest significant 8 but if pdev->class
is really matter.
I think I still need to wait the things(a bug emerged, for example)
became clear.
Instead of cleanup all potential problems with obvious motivation.
I think Mario will accept my explanation.
[1] https://lore.kernel.org/r/20230718231400.GA496927@bhelgaas
[2] https://lore.kernel.org/all/5b6fdf65-b354-94a9-f883-be820157e...@amd.com/
---
drivers/pci/vgaarb.c | 25 +
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index c1bc6c983932..021116ed61cb 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -754,10 +754,6 @@ static bool vga_arbiter_add_pci_device(struct pci_dev
*pdev)
struct pci_dev *bridge;
u16 cmd;
- /* Only deal with VGA class devices */
- if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
- return false;
-
/* Allocate structure */
vgadev = kzalloc(sizeof(struct vga_device), GFP_KERNEL);
if (vgadev == NULL) {
@@ -1502,6 +1498,10 @@ static int pci_notify(struct notifier_block *nb,
unsigned long action,
vgaarb_dbg(dev, "%s\n", __func__);
+ /* Deal with VGA compatible devices only */
+ if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
+ return 0;
+
/* For now we're only intereted in devices added and removed. I didn't
* test this thing here, so someone needs to double check for the
* cases of hotplugable vga cards. */
@@ -1534,8 +1534,8 @@ static struct miscdevice vga_arb_device = {
static int __init vga_arb_device_init(void)
{
+ struct pci_dev *pdev = NULL;
int rc;
- struct pci_dev *pdev;
rc = misc_register(&vga_arb_device);
if (rc < 0)
@@ -1543,13 +1543,14 @@ static int __init vga_arb_device_init(void)
bus_register_notifier(&pci_bus_type, &pci_notifier);
- /* We add all PCI devices satisfying VGA class in the arbiter by
-* default */
- pdev = NULL;
- while ((pdev =
- pci_get_subsys(PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
- PCI_ANY_ID, pdev)) != NULL)
- vga_arbiter_add_pci_device(pdev);
+ /*
+* We add all PCI VGA compatible devices in the arbiter by default
+*/
+ do {
+ pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, pdev);
+ if (pdev)
+ vga_arbiter_add_pci_device(pdev);
+ } while (pdev);
pr_info("loaded\n");
return rc;
--
2.25.1
Re: [Intel-xe] [PATCH v5] Documentation/gpu: Add a VM_BIND async draft document
On Sat, 2023-07-15 at 17:45 +0200, Thomas Hellström wrote: > Add a motivation for and description of asynchronous VM_BIND operation I think I may have missed some other documentation, which would explain some of my questions below, so please be patient with my misunderstandings. But here's a review from the POV of a UMD person. > > v2: > - Fix typos (Nirmoy Das) > - Improve the description of a memory fence (Oak Zeng) > - Add a reference to the document in the Xe RFC. > - Add pointers to sample uAPI suggestions > v3: > - Address review comments (Danilo Krummrich) > - Formatting fixes > v4: > - Address typos (Francois Dugast) > - Explain why in-fences are not allowed for VM_BIND operations for long- > running workloads (Matthew Brost) > v5: > - More typo- and style fixing > - Further clarify the implications of disallowing in-fences for VM_BIND > operations for long-running workloads (Matthew Brost) > > Signed-off-by: Thomas Hellström > Acked-by: Nirmoy Das > --- > Documentation/gpu/drm-vm-bind-async.rst | 171 > Documentation/gpu/rfc/xe.rst| 4 +- > 2 files changed, 173 insertions(+), 2 deletions(-) > create mode 100644 Documentation/gpu/drm-vm-bind-async.rst > > diff --git a/Documentation/gpu/drm-vm-bind-async.rst > b/Documentation/gpu/drm-vm-bind-async.rst > new file mode 100644 > index ..d2b02a38198a > --- /dev/null > +++ b/Documentation/gpu/drm-vm-bind-async.rst > @@ -0,0 +1,171 @@ > + > +Asynchronous VM_BIND > + > + > +Nomenclature: > += > + > +* ``VRAM``: On-device memory. Sometimes referred to as device local memory. > + > +* ``gpu_vm``: A GPU address space. Typically per process, but can be shared > by > + multiple processes. > + > +* ``VM_BIND``: An operation or a list of operations to modify a gpu_vm using > + an IOCTL. The operations include mapping and unmapping system- or > + VRAM memory. > + > +* ``syncobj``: A container that abstracts synchronization objects. The > + synchronization objects can be either generic, like dma-fences or > + driver specific. A syncobj typically indicates the type of the > + underlying synchronization object. > + > +* ``in-syncobj``: Argument to a VM_BIND IOCTL, the VM_BIND operation waits > + for these before starting. > + > +* ``out-syncobj``: Argument to a VM_BIND_IOCTL, the VM_BIND operation > + signals these when the bind operation is complete. > + > +* ``memory fence``: A synchronization object, different from a dma-fence. Since you've mentioned it twice in this document already, for completeness would you mind also giving a definition for dma-fence in what it relates/contrasts to the rest of the text? > + A memory fence uses the value of a specified memory location to determine > + signaled status. A memory fence can be awaited and signaled by both > + the GPU and CPU. Memory fences are sometimes referred to as > + user-fences, userspace-fences or gpu futexes and do not necessarily obey > + the dma-fence rule of signaling within a "reasonable amount of time". > + The kernel should thus avoid waiting for memory fences with locks held. > + > +* ``long-running workload``: A workload that may take more than the > + current stipulated dma-fence maximum signal delay to complete and Where is this delay defined? Is this the same as the gpuhang timer? > + which therefore needs to set the gpu_vm or the GPU execution context in > + a certain mode that disallows completion dma-fences. > + > +* ``exec function``: An exec function is a function that revalidates all > + affected gpu_vmas, submits a GPU command batch and registers the > + dma_fence representing the GPU command's activity with all affected > + dma_resvs. For completeness, although not covered by this document, > + it's worth mentioning that an exec function may also be the > + revalidation worker that is used by some drivers in compute / > + long-running mode. > + > +* ``bind context``: A context identifier used for the VM_BIND > + operation. VM_BIND operations that use the same bind context can be > + assumed, where it matters, to complete in order of submission. No such > + assumptions can be made for VM_BIND operations using separate bind > contexts. > + > +* ``UMD``: User-mode driver. > + > +* ``KMD``: Kernel-mode driver. > + > + > +Synchronous / Asynchronous VM_BIND operation > + > + > +Synchronous VM_BIND > +___ > +With Synchronous VM_BIND, the VM_BIND operations all complete before the > +IOCTL returns. A synchronous VM_BIND takes neither in-fences nor > +out-fences. Synchronous VM_BIND may block and wait for GPU operations; > +for example swap-in or clearing, or even previous binds. > + > +Asynchronous VM_BIND > + > +Asynchronous VM_BIND accepts both in-syncobjs and out-syncobjs. While the > +IOCTL may return immediately, the VM_BIND operations wait for the in-syncobjs > +
Re: [PATCH v3 0/9] PCI/VGA: Improve the default VGA device selection
[+cc linux-pci] On Wed, Jul 12, 2023 at 12:43:01AM +0800, Sui Jingfeng wrote: > From: Sui Jingfeng > > Currently, the default VGA device selection is not perfect. Potential > problems are: > > 1) This function is a no-op on non-x86 architectures. > 2) It does not take the PCI Bar may get relocated into consideration. > 3) It is not effective for the PCI device without a dedicated VRAM Bar. > 4) It is device-agnostic, thus it has to waste the effort to iterate all >of the PCI Bar to find the VRAM aperture. > 5) It has invented lots of methods to determine which one is the default >boot device on a multiple video card coexistence system. But this is >still a policy because it doesn't give the user a choice to override. > > With the observation that device drivers or video aperture helpers may > have better knowledge about which PCI bar contains the firmware FB, > > This patch tries to solve the above problems by introducing a function > callback to the vga_client_register() function interface. DRM device > drivers for the PCI device need to register the is_boot_device() function > callback during the driver loading time. Once the driver binds the device > successfully, VRAARB will call back to the driver. This gives the device > drivers a chance to provide accurate boot device identification. Which in > turn unlock the abitration service to non-x86 architectures. A device > driver can also pass a NULL pointer to keep the original behavior. I skimmed all these patches, but the only one that seems to mention an actual problem being solved, i.e., something that doesn't work for an end user, is "drm/ast: Register as a vga client ..." Maybe "drm/loongson: Add an implement for ..." also solves a problem, but it lacks a commit log, so I don't know what the problem is. In the future, can you please cc: linux-pci with the entire series? In this posting, only the patches that touched drivers/pci/vgaarb.c went to linux-pci, but those aren't enough to make sense of the series as a whole. > video/aperture: Add a helper to detect if an aperture contains > firmware FB > video/aperture: Add a helper for determining if an unmoved aperture > contain FB > PCI/VGA: Switch to aperture_contain_firmware_fb_nonreloc() Since this subject includes the function name (which is nice!), it would also be helpful if the "Add a helper ..." subject included the same function name. > PCI/VGA: Improve the default VGA device selection If you can make this subject any more specific, that would be useful. There's more to say about that patch, so I'll respond there. > drm/amdgpu: Implement the is_primary_gpu callback of > vga_client_register() > drm/radeon: Add an implement for the is_primary_gpu function callback > drm/i915: Add an implement for the is_primary_gpu hook > drm/ast: Register as a vga client to vgaarb by calling > vga_client_register() > drm/loongson: Add an implement for the is_primary_gpu function > callback There's unnecessary variation in the subject lines (and the commit logs) of these patches. If they all do the same thing but in different drivers, it's useful if the patches all *look* the same. You might be able to write these subjects as "Implement .is_primary_gpu() callback" for brevity. Bjorn
Re: [PATCH v3 4/9] PCI/VGA: Improve the default VGA device selection
[+cc linux-pci (please cc in the future since the bulk of this patch
is in drivers/pci/)]
On Wed, Jul 12, 2023 at 12:43:05AM +0800, Sui Jingfeng wrote:
> From: Sui Jingfeng
>
> Currently, the strategy of selecting the default boot on a multiple video
> card coexistence system is not perfect. Potential problems are:
>
> 1) This function is a no-op on non-x86 architectures.
Which function in particular is a no-op for non-x86?
> 2) It does not take the PCI Bar may get relocated into consideration.
> 3) It is not effective for the PCI device without a dedicated VRAM Bar.
> 4) It is device-agnostic, thus it has to waste the effort to iterate all
>of the PCI Bar to find the VRAM aperture.
> 5) It has invented lots of methods to determine which one is the default
>boot device, but this is still a policy because it doesn't give the
>user a choice to override.
I don't think we need a list of *potential* problems. We need an
example of the specific problem this will solve, i.e., what currently
does not work?
The drm/ast and maybe drm/loongson patches are the only ones that use
the new callback, so I assume there are real problems with those
drivers.
CONFIG_DRM_AST is a tristate. We're talking about identifying the
boot-time console device. So if CONFIG_DRM_AST=m, I guess we don't
get the benefit of the new callback unless the module gets loaded?
> Also honor the comment: "Clients have *TWO* callback mechanisms they
> can use"
This refers to the existing vga_client_register() function comment:
* vga_client_register - register or unregister a VGA arbitration client
* @pdev: pci device of the VGA client
* @set_decode: vga decode change callback
*
* Clients have two callback mechanisms they can use.
*
* @set_decode callback: If a client can disable its GPU VGA resource, it
* will get a callback from this to set the encode/decode state.
and the fact that struct vga_device currently only contains *one*
callback function pointer:
unsigned int (*set_decode)(struct pci_dev *pdev, bool decode);
Adding the .is_primary_gpu() callback does mean there will now be two
callbacks, as the comment says, but I think it's just confusing to
mention this in the commit log, so I would just remove it.
> @@ -1509,13 +1543,24 @@ static int pci_notify(struct notifier_block *nb,
> unsigned long action,
>* cases of hotplugable vga cards.
>*/
>
> - if (action == BUS_NOTIFY_ADD_DEVICE)
> + switch (action) {
> + case BUS_NOTIFY_ADD_DEVICE:
> notify = vga_arbiter_add_pci_device(pdev);
> - else if (action == BUS_NOTIFY_DEL_DEVICE)
> + if (notify)
> + vga_arbiter_notify_clients();
> + break;
> + case BUS_NOTIFY_DEL_DEVICE:
> notify = vga_arbiter_del_pci_device(pdev);
> + if (notify)
> + vga_arbiter_notify_clients();
> + break;
> + case BUS_NOTIFY_BOUND_DRIVER:
> + vga_arbiter_do_arbitration(pdev);
> + break;
> + default:
> + break;
> + }
Changing from if/else to switch makes the patch bigger than necessary
for no real benefit and obscures what is really changing.
Bjorn
Re: [05/11] drm/tests: helpers: Create an helper to allocate a locking ctx
suijingfeng writes:
> Hi,
>
> On 2023/7/10 15:47, Maxime Ripard wrote:
>> As we get more and more tests, the locking context initialisation
[...]
>> +/**
>> + * drm_kunit_helper_context_alloc - Allocates an acquire context
>> + * @test: The test context object
>> + *
>> + * Allocates and initializes a modeset acquire context.
>> + *
>> + * The context is tied to the kunit test context, so we must not call
>> + * drm_modeset_acquire_fini() on it, it will be done so automatically.
>> + *
>> + * Returns:
>> + * An ERR_PTR on error, a pointer to the newly allocated context otherwise
>> + */
>> +struct drm_modeset_acquire_ctx *
>> +drm_kunit_helper_acquire_ctx_alloc(struct kunit *test)
>> +{
>> +struct drm_modeset_acquire_ctx *ctx;
>> +int ret;
>> +
>> +ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
>
> Because kunit_kzalloc() is also managed,
>
> Is there any possibility that kfree(ctx) get called before
> action_drm_release_context(ctx) ?
>
> Currently, I can't find where the order is guaranteed.
>
It isn't documented indeed in Documentation/dev-tools/kunit/usage.rst but
the kunit_add_action() kernel-doc says:
"All functions registered with kunit_add_action() will execute in the
opposite order to that they were registered in".
And now that kunit_kzalloc() and friends are also implemented using the
cleanup actions, it will be part of that execution chain.
Probably the kunit docs can make this more clear.
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
Re: [PATCH 2/3] dt-bindings: display: panel: Add panels based on ILITEK ILI9806E
On 19/07/2023 17:21, Luca Ceresoli wrote: > Add bindings for LCD panels based on the ILITEK ILI9806E RGB controller > connected over SPI and the "ShenZhen New Display Co NDS040480800-V3" > 480x800 panel based on it. > diff --git a/MAINTAINERS b/MAINTAINERS > index aee340630eca..3c38699ee821 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6515,6 +6515,12 @@ T: git git://anongit.freedesktop.org/drm/drm-misc > F: Documentation/devicetree/bindings/display/ilitek,ili9486.yaml > F: drivers/gpu/drm/tiny/ili9486.c > > +DRM DRIVER FOR ILITEK ILI9806E PANELS > +M: Luca Ceresoli > +S: Maintained > +T: git git://anongit.freedesktop.org/drm/drm-misc Nope, same for recent one-driver-subsystem. It's like a second try... You do not have git tree for one driver. The git tree is for subsystem, not driver. Best regards, Krzysztof
Re: [05/11] drm/tests: helpers: Create an helper to allocate a locking ctx
suijingfeng writes:
> Hi,
>
> On 2023/7/10 15:47, Maxime Ripard wrote:
[...]
>> +
>> +/**
>> + * drm_kunit_helper_context_alloc - Allocates an acquire context
>> + * @test: The test context object
>> + *
>> + * Allocates and initializes a modeset acquire context.
>> + *
>> + * The context is tied to the kunit test context, so we must not call
>> + * drm_modeset_acquire_fini() on it, it will be done so automatically.
>> + *
>> + * Returns:
>> + * An ERR_PTR on error, a pointer to the newly allocated context otherwise
>> + */
>> +struct drm_modeset_acquire_ctx *
>> +drm_kunit_helper_acquire_ctx_alloc(struct kunit *test)
>> +{
>> +struct drm_modeset_acquire_ctx *ctx;
>> +int ret;
>> +
>> +ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
>> +KUNIT_ASSERT_NOT_NULL(test, ctx);
>> +
>> +drm_modeset_acquire_init(ctx, 0);
>> +
>> +ret = kunit_add_action_or_reset(test,
>> +action_drm_release_context,
>> +ctx);
>> +if (ret)
>> +return ERR_PTR(ret);
>> +
>> +return ctx;
>> +}
>> +EXPORT_SYMBOL_GPL(drm_kunit_helper_acquire_ctx_alloc);
>> +
>
> I think all of the patch inside this series are quite well.
>
> Personally, I can't find problems in it.
>
>
> But I still want to ask a question:
>
> Should the managed functions you introduced be prefixed with drmm_
> (instead of drm_) ?
>
That's a good question. But personally I think that the drmm_ prefix
should be reserved for drm_device managed resources and helpers.
> As mindless programmer may still want to call drm_modeset_acquire_fini()
> on the pointer returned by
>
> drm_kunit_helper_acquire_ctx_alloc()?
>
The function kernel-doc already mentions that there's no need to do that
and that will be done automatically by kunit. So shouldn't be different of
other functions helper where the programmer didn't read the documentation.
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
Re: [PATCH 05/11] drm/tests: helpers: Create an helper to allocate a locking ctx
Maxime Ripard writes: > As we get more and more tests, the locking context initialisation > creates more and more boilerplate, both at creation and destruction. > > Let's create a helper that will allocate, initialise a context, and > register kunit actions to clean up once the test is done. > > Signed-off-by: Maxime Ripard > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH 2/3] dt-bindings: display: panel: Add panels based on ILITEK ILI9806E
On Wed, Jul 19, 2023 at 05:21:46PM +0200, Luca Ceresoli wrote:
> Add bindings for LCD panels based on the ILITEK ILI9806E RGB controller
> connected over SPI and the "ShenZhen New Display Co NDS040480800-V3"
> 480x800 panel based on it.
>
> Signed-off-by: Luca Ceresoli
> ---
> .../display/panel/ilitek,ili9806e.yaml| 69 +++
> MAINTAINERS | 6 ++
> 2 files changed, 75 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
>
> diff --git
> a/Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
> b/Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
> new file mode 100644
> index ..42abc6923065
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/ilitek,ili9806e.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Ilitek ILI9806E display panels
> +
> +maintainers:
> + - Luca Ceresoli
> +
> +description:
> + This binding is for display panels using an Ilitek ILI9806E controller in
> + SPI mode.
> +
> +allOf:
> + - $ref: panel-common.yaml#
A SPI device should reference spi-peripheral-props.yaml as well.
> +
> +properties:
> + compatible:
> +items:
> + - enum:
> + # ShenZhen New Display Co 3.97" 480x800 RGB a-SI TFT LCD
> + - newdisplay,nds040480800-v3
> + - const: ilitek,ili9806e
> +
> + reg: true
maxItems: 1
> + spi-max-frequency: true
> + reset-gpios: true
> + backlight: true
> + port: true
Drop all these and ...
> +
> +required:
> + - compatible
> + - reg
> + - port
> +
> +additionalProperties: false
... use "unevaluatedProperties" instead.
> +
> +examples:
> + - |
> +#include
> +
> +backlight: backlight {
> +compatible = "gpio-backlight";
> +gpios = <&gpio 22 GPIO_ACTIVE_HIGH>;
> +};
The exact backlight is outside the scope of this binding and should be
dropped from the example.
> +spi {
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +display@0 {
> +compatible = "newdisplay,nds040480800-v3", "ilitek,ili9806e";
> +reg = <0>;
> +spi-max-frequency = <100>;
> +pinctrl-names = "default";
> +pinctrl-0 = <&pinctrl_lcdgpios>;
> +reset-gpios = <&gpio 26 GPIO_ACTIVE_LOW>;
> +backlight = <&backlight>;
> +
> +port {
> +ili9806e_in: endpoint {
> +remote-endpoint = <&lcdif_out>;
> +};
> +};
> +};
> +};
> +
> +...
Re: [PATCH 3/3] drm/scheduler: Clean up jobs when the scheduler is torn down.
On 2023-07-19 14:16, Konstantin Ryabitsev wrote: > July 18, 2023 at 1:14 AM, "Luben Tuikov" wrote: Not sure about other drivers--they can speak for themselves and the CC list should include them--please use "dim add-missing-cc" and make sure that the Git commit description contains the Cc tags--then git send-email will populate the SMTP CC. Feel free to add more Cc tags on top of that. >>> >>> I use `b4 prep -c` which I think does the same thing? I just ran it >>> again and it only added 'linaro-mm-...@lists.linaro.org', not sure why >>> that one wasn't there. Am I missing anything else? >> >> Not sure about "b4 prep -c"--using "git send-email" instead, but what is >> important is to add the Cc: tags as part of the commit message. A "git log" >> of >> drm-misc-next shows the proper format. Then maintainers add Link: >> tag to the correct email thread, which is usually completely automated >> by "dim" or by "git am", or both. > > It's useful to note here that this is not standard practice across the > entirety of the Linux tree. In general, Cc: trailers are added to individual > commits when get_maintainer.pl wouldn't otherwise include someone in the > recipient list. The "dim" tool mentioned here is specific to the DRM > subsystem (the "d" stands for "DRM"). Since both tools work on git series, > you can use it alongside b4. > In DRM we use "dim"--it's just how we do things and everyone complies with this. "dim" also includes the Link: tag (which "git am" can also be made add), and this adds certain amount of accountability, which is a good thing. This is why I suggested that a subsequent version of these patches, include the Cc: tags, which would normally come from "dim add-missing-cc", which uses "scripts/get_maintainer.pl". DRM maintainers regularly use `git rebase --exec "dim add-missing-cc" ...'. > DRM folks, if get_maintainer.pl isn't finding someone who should be included > on a series of patches, should the MAINTAINERS file be updated to make it > easier to submit valid patches without needing to know of "dim"? "scripts/get_maintainer.pl" does consult the MAINTAINERS file. There's been no immediate need to update the MAINTAINERS file. Sometimes a single function or a single line in a function (as in some kind of complex calculation), might be coming from someone who doesn't normally commit to the subsystem. This is where "git blame" and "git log" are helpful to inspect and add a Cc: tag with that email to the commit message, and this of course depends on the nature of the incoming patch. -- Regards, Luben
Re: [PATCH v6 3/3] drm/virtio: Support sync objects
27.06.2023 15:01, Geert Uytterhoeven пишет:
> Hi Dmitry,
>
> On Mon, Jun 26, 2023 at 6:11 PM Dmitry Osipenko
> wrote:
>> On 6/25/23 18:36, Geert Uytterhoeven wrote:
>>> On Sun, Jun 25, 2023 at 2:41 PM Dmitry Osipenko
>>> wrote:
On 6/25/23 11:47, Geert Uytterhoeven wrote:
> On Sun, Apr 16, 2023 at 1:55 PM Dmitry Osipenko
> wrote:
>> Add sync object DRM UAPI support to VirtIO-GPU driver. Sync objects
>> support is needed by native context VirtIO-GPU Mesa drivers, it also will
>> be used by Venus and Virgl contexts.
>>
>> Reviewed-by; Emil Velikov
>> Signed-off-by: Dmitry Osipenko
>
> Thanks for your patch!
>
>> --- a/drivers/gpu/drm/virtio/virtgpu_submit.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_submit.c
>
>> +static int
>> +virtio_gpu_parse_deps(struct virtio_gpu_submit *submit)
>> +{
>> + struct drm_virtgpu_execbuffer *exbuf = submit->exbuf;
>> + struct drm_virtgpu_execbuffer_syncobj syncobj_desc;
>> + size_t syncobj_stride = exbuf->syncobj_stride;
>> + u32 num_in_syncobjs = exbuf->num_in_syncobjs;
>> + struct drm_syncobj **syncobjs;
>> + int ret = 0, i;
>> +
>> + if (!num_in_syncobjs)
>> + return 0;
>> +
>> + /*
>> +* kvalloc at first tries to allocate memory using kmalloc and
>> +* falls back to vmalloc only on failure. It also uses GFP_NOWARN
>
> GFP_NOWARN does not exist.
https://elixir.bootlin.com/linux/v6.4-rc7/source/include/linux/gfp_types.h#L38
>>>
>>> That line defines "__GFP_NOWARN", not "GFP_NOWARN".
>>> C is case- and underscore-sensitive. as is "git grep -w" ;-)
>>
>> The removal of underscores was done intentionally for improving
>> readability of the comment
>
> Please don't do that, as IMHO it actually hampers readability:
> 1. For some xxx, both GFP_xxx and __GFP_xxx are defined,
> so it does matter which one you are referring to,
> 2. After dropping the underscores, "git grep -w" can no longer find
> the definition, nor its users.
>
> Thanks!
Alright, I'll change it
--
Best regards,
Dmitry
Re: [PATCH v6 0/3] Add sync object UAPI support to VirtIO-GPU driver
27.06.2023 20:16, Rob Clark пишет: ... >> Now these are just suggestions, and while I think they are good, you can >> safely ignore them. >> >> But there's also the DRM requirements, which state "userspace side must be >> fully reviewed and tested to the standards of that user-space project.". So >> I think to meet the minimum requirements, I think we should at-least have >> one of the following (not all, just one) reviewed: >> >> 1) venus using the new uapi >> 2) gfxstream vk using the new uapi >> 3) amdgpu nctx out of "draft" mode and using the new uapi. >> 4) virtio-intel using new uapi >> 5) turnip using your new uapi > > forgot to mention this earlier, but > https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/23533 > > Dmitry, you can also add, if you haven't already: > > Tested-by: Rob Clark Gurchetan, Turnip Mesa virtio support is ready to be merged upstream, it's using this new syncobj UAPI. Could you please give yours r-b if you don't have objections? -- Best regards, Dmitry
Re: [PATCH] drm/panel: r66451: select CONFIG_DRM_DISPLAY_DP_HELPER
On 7/19/2023 6:09 AM, Arnd Bergmann wrote:
From: Arnd Bergmann
The newly added driver only builds when DRM_DISPLAY_DP_HELPER is enabled:
x86_64-linux-ld: drivers/gpu/drm/panel/panel-visionox-r66451.o: in function
`visionox_r66451_enable':
panel-visionox-r66451.c:(.text+0x105): undefined reference to
`drm_dsc_pps_payload_pack'
Select both CONFIG_DRM_DISPLAY_DP_HELPER and CONFIG_DRM_DISPLAY_HELPER to
ensure the helper function is always available.
Hi Arnd,
Thanks for catching this -- hadn't seen this issue due to DRM_MSM
selecting both configs already.
Reviewed-by: Jessica Zhang
Thanks,
Jessica Zhang
Fixes: a6dfab2738fc2 ("drm/panel: Add driver for Visionox r66451 panel")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/panel/Kconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 1a0fd0754692e..e8c9f4613a4b4 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -798,6 +798,8 @@ config DRM_PANEL_VISIONOX_R66451
depends on OF
depends on DRM_MIPI_DSI
depends on BACKLIGHT_CLASS_DEVICE
+ select DRM_DISPLAY_DP_HELPER
+ select DRM_DISPLAY_HELPER
help
Say Y here if you want to enable support for Visionox
R66451 1080x2340 AMOLED DSI panel.
--
2.39.2
Re: [PATCH 04/11] drm/tests: probe-helper: Remove call to drm_kunit_helper_free_device()
Maxime Ripard writes: > Calling drm_kunit_helper_free_device() to clean up the resources > allocated by drm_kunit_helper_alloc_device() is now optional and not > needed in most cases. > > Remove it. > > Signed-off-by: Maxime Ripard > --- I wonder if makes sense to just squash 2-3 and this one as a single patch. Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH 03/11] drm/tests: modes: Remove call to drm_kunit_helper_free_device()
Maxime Ripard writes: > Calling drm_kunit_helper_free_device() to clean up the resources > allocated by drm_kunit_helper_alloc_device() is now optional and not > needed in most cases. > > Remove it. > > Signed-off-by: Maxime Ripard > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH 02/11] drm/tests: client-modeset: Remove call to drm_kunit_helper_free_device()
Maxime Ripard writes: > Calling drm_kunit_helper_free_device() to clean up the resources > allocated by drm_kunit_helper_alloc_device() is now optional and not > needed in most cases. > > Remove it. > > Signed-off-by: Maxime Ripard > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH 01/11] drm/tests: helpers: Switch to kunit actions
Maxime Ripard writes:
Hello Maxime,
The patch looks good to me. I've two questions below though.
Reviewed-by: Javier Martinez Canillas
> Signed-off-by: Maxime Ripard
> ---
> drivers/gpu/drm/tests/drm_kunit_helpers.c | 32
> +++
> 1 file changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.c
> b/drivers/gpu/drm/tests/drm_kunit_helpers.c
> index 4df47071dc88..38211fea9ae6 100644
> --- a/drivers/gpu/drm/tests/drm_kunit_helpers.c
> +++ b/drivers/gpu/drm/tests/drm_kunit_helpers.c
> @@ -35,8 +35,8 @@ static struct platform_driver fake_platform_driver = {
> * able to leverage the usual infrastructure and most notably the
> * device-managed resources just like a "real" device.
> *
> - * Callers need to make sure drm_kunit_helper_free_device() on the
> - * device when done.
> + * Resources will be cleaned up automatically, but the removal can be
> + * forced using @drm_kunit_helper_free_device.
> *
> * Returns:
> * A pointer to the new device, or an ERR_PTR() otherwise.
> @@ -49,12 +49,31 @@ struct device *drm_kunit_helper_alloc_device(struct kunit
> *test)
> ret = platform_driver_register(&fake_platform_driver);
> KUNIT_ASSERT_EQ(test, ret, 0);
>
> + ret = kunit_add_action_or_reset(test,
> + (kunit_action_t
> *)platform_driver_unregister,
> + &fake_platform_driver);
> + KUNIT_ASSERT_EQ(test, ret, 0);
> +
> pdev = platform_device_alloc(KUNIT_DEVICE_NAME, PLATFORM_DEVID_NONE);
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
>
> + ret = kunit_add_action_or_reset(test,
> + (kunit_action_t *)platform_device_put,
> + pdev);
> + KUNIT_ASSERT_EQ(test, ret, 0);
> +
> ret = platform_device_add(pdev);
> KUNIT_ASSERT_EQ(test, ret, 0);
>
> + kunit_remove_action(test,
> + (kunit_action_t *)platform_device_put,
> + pdev);
> +
I understand that this action removal is because platform_device_put() is
not needed anymore after the platform_device_unregister() remove action is
registered since that already takes care of the platform_device_put().
But maybe add a comment to make more clear for someone who is not familiar
with these details of the platform core ?
> EXPORT_SYMBOL_GPL(drm_kunit_helper_alloc_device);
> @@ -70,8 +89,13 @@ void drm_kunit_helper_free_device(struct kunit *test,
> struct device *dev)
> {
> struct platform_device *pdev = to_platform_device(dev);
>
> - platform_device_unregister(pdev);
> - platform_driver_unregister(&fake_platform_driver);
> + kunit_release_action(test,
> + (kunit_action_t *)platform_device_unregister,
> + pdev);
> +
> + kunit_release_action(test,
> + (kunit_action_t *)platform_driver_unregister,
> + &fake_platform_driver);
> }
> EXPORT_SYMBOL_GPL(drm_kunit_helper_free_device);
>
I thought the point of using the kunit cleanup actions was that you could
just make the kunit framework handle the release of resources and not do
it manually?
Can you just remove this function helper or is still needed in some cases?
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
Re: [PATCH 2/6] PCI/VGA: Deal with PCI VGA compatible devices only
On Tue, Jul 11, 2023 at 09:43:50PM +0800, Sui Jingfeng wrote:
> From: Sui Jingfeng
>
> Currently, vgaarb only cares about PCI VGA-compatible class devices.
>
> While vga_arbiter_del_pci_device() gets called unbalanced when some PCI
> device is about to be removed. This happens even during the boot process.
The previous code calls vga_arbiter_add_pci_device() for every device
(every device present at boot and also every hot-added device). It
only allocates a vga_device if pdev->class is 0x0300XX.
It calls vga_arbiter_del_pci_device() for every device removal. It
does nothing unless it finds a vga_device.
This seems symmetric and reasonable to me. Did you observe a problem
with it?
> Another reason is that the vga_arb_device_init() function is not efficient.
> Since we only care about VGA-compatible devices (pdev->class == 0x03),
> We could filter the unqualified devices out in the vga_arb_device_init()
> function. While the current implementation is to search all PCI devices
> in a system, this is not necessary.
Optimization is fine, but the most important thing here is to be clear
about what functional change this patch makes. As I mentioned at [1],
if this patch affects the class codes accepted, please make that clear
here.
> Reviewed-by: Mario Limonciello
> Signed-off-by: Sui Jingfeng
I do not see Mario's Reviewed-by on the list. I do see Mario's
Reviewed-by [2] for a previous version, but that version added this in
pci_notify():
+ if (pdev->class != PCI_CLASS_DISPLAY_VGA << 8)
+ return 0;
while this version adds:
+ if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
+ return 0;
It's OK to carry a review to future versions if there are
insignificant changes, but this is a functional change that seems
significant to me. The first matches only 0x03, while the second
discards the low eight bits so it matches 0x0300XX.
[1] https://lore.kernel.org/r/20230718231400.GA496927@bhelgaas
[2] https://lore.kernel.org/all/5b6fdf65-b354-94a9-f883-be820157e...@amd.com/
> ---
> drivers/pci/vgaarb.c | 25 +
> 1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
> index c1bc6c983932..021116ed61cb 100644
> --- a/drivers/pci/vgaarb.c
> +++ b/drivers/pci/vgaarb.c
> @@ -754,10 +754,6 @@ static bool vga_arbiter_add_pci_device(struct pci_dev
> *pdev)
> struct pci_dev *bridge;
> u16 cmd;
>
> - /* Only deal with VGA class devices */
> - if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
> - return false;
> -
> /* Allocate structure */
> vgadev = kzalloc(sizeof(struct vga_device), GFP_KERNEL);
> if (vgadev == NULL) {
> @@ -1502,6 +1498,10 @@ static int pci_notify(struct notifier_block *nb,
> unsigned long action,
>
> vgaarb_dbg(dev, "%s\n", __func__);
>
> + /* Deal with VGA compatible devices only */
> + if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
> + return 0;
> +
> /* For now we're only intereted in devices added and removed. I didn't
>* test this thing here, so someone needs to double check for the
>* cases of hotplugable vga cards. */
> @@ -1534,8 +1534,8 @@ static struct miscdevice vga_arb_device = {
>
> static int __init vga_arb_device_init(void)
> {
> + struct pci_dev *pdev = NULL;
> int rc;
> - struct pci_dev *pdev;
>
> rc = misc_register(&vga_arb_device);
> if (rc < 0)
> @@ -1543,13 +1543,14 @@ static int __init vga_arb_device_init(void)
>
> bus_register_notifier(&pci_bus_type, &pci_notifier);
>
> - /* We add all PCI devices satisfying VGA class in the arbiter by
> - * default */
> - pdev = NULL;
> - while ((pdev =
> - pci_get_subsys(PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
> -PCI_ANY_ID, pdev)) != NULL)
> - vga_arbiter_add_pci_device(pdev);
> + /*
> + * We add all PCI VGA compatible devices in the arbiter by default
> + */
> + do {
> + pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, pdev);
> + if (pdev)
> + vga_arbiter_add_pci_device(pdev);
> + } while (pdev);
>
> pr_info("loaded\n");
> return rc;
> --
> 2.25.1
>
Re: [PATCH 2/4] PCI/VGA: Deal only with PCI VGA class devices
On Tue, Jul 18, 2023 at 06:14:00PM -0500, Bjorn Helgaas wrote: > On Fri, Jun 30, 2023 at 06:17:29PM +0800, Sui Jingfeng wrote: > > From: Sui Jingfeng > > > > VGAARB should only care about PCI VGA class devices (pdev->class == 0x0300) > > since only those devices might have VGA routed to them. > > This is not actually a question of whether VGA addresses (mem > 0xa-0xb and io 0x3b0-0x3bb, 0x3c0-0x3df) might be *routed* to > the device because that routing is controlled by the bridge VGA Enable > bit, not by a device Class Code. > > I think the important question here is what devices will *respond* to > those VGA addresses. The VGA arbiter works by managing bridge VGA > Enable bits, so if we know a device doesn't respond to the VGA > addresses, there's no point in adding a vga_device for it. Sorry, I see that I replied to an old version of this patch. I'll go look at this series instead: https://lore.kernel.org/r/20230711134354.755966-1-sui.jingf...@linux.dev Bjorn
Re: [RFC PATCH 00/10] Device Memory TCP
On Wed, 19 Jul 2023 08:10:58 -0700 Mina Almasry wrote: > On Tue, Jul 18, 2023 at 3:45 PM Jakub Kicinski wrote: > > > > On Tue, 18 Jul 2023 16:35:17 -0600 David Ahern wrote: > > > I do not see how 1 RSS context (or more specifically a h/w Rx queue) can > > > be used properly with memory from different processes (or dma-buf > > > references). > > Right, my experience with dma-bufs from GPUs are that they're > allocated from the userspace and owned by the process that allocated > the backing GPU memory and generated the dma-buf from it. I.e., we're > limited to 1 dma-buf per RX queue. If we enable binding multiple > dma-bufs to the same RX queue, we have a problem, because AFAIU the > NIC can't decide which dma-buf to put the packet into (it hasn't > parsed the packet's destination yet). > > > > When the process dies, that memory needs to be flushed from > > > the H/W queues. Queues with interlaced submissions make that more > > > complicated. > > > > When the process dies, do we really want to flush the memory from the > hardware queues? The drivers I looked at don't seem to have a function > to flush the rx queues alone, they usually do an entire driver reset > to achieve that. Not sure if that's just convenience or there is some > technical limitation there. Do we really want to trigger a driver > reset at the event a userspace process crashes? Naive idea. Would it be possible for process to use mmap() on the GPU memory and then do zero copy TCP receive some how? Or is this what is being proposed.
Re: [PATCH v6 0/4] Allow disabling all native fbdev drivers and only keeping DRM emulation
Helge Deller writes: Hello Helge, > Hi Javier, > > On 7/19/23 10:15, Javier Martinez Canillas wrote: >> This patch series splits the fbdev core support in two different Kconfig >> symbols: FB and FB_CORE. The motivation for this is to allow CONFIG_FB to >> be disabled, while still having the the core fbdev support needed for the > > One "the" too much. > (correcting just because this is a cover letter) > >> CONFIG_DRM_FBDEV_EMULATION to be enabled. The motivation is automatically >> disabling all fbdev drivers instead of having to be disabled individually. >> >> The reason for doing this is that now with simpledrm, there's no need for >> the legacy fbdev (e.g: efifb or vesafb) drivers anymore and many distros >> now disable them. > But it would simplify the config a lot fo have a single >> Kconfig symbol to disable all fbdev drivers. > > I suggest to rephrase this, e.g.: > The reason for doing this is that with simpledrm, mainstream Linux > distributions > like Fedora, SUSE or Ubuntu can then more easily enable the DRM drivers only > by switching off legacy fbdev drivers with one Kconfig option (e.g. for efifb > or > vesafb). > > (note: there are quite many other distributions, e.g. debian and the > derivates, > which still need the fbdev drivers for the various other architectures) > Right, I meant that there is no need anymore for the remaining fbdev drivers that many distributions used to enable only to have fallback fbcon/VT (like vesafb, efifb, simplefb, etc). But I'm OK with your rephrasing and agree that's more accurate to cover all the distros. >> I've built tested with possible combinations of CONFIG_FB, CONFIG_FB_CORE, >> CONFIG_DRM_FBDEV_EMULATION and CONFIG_FB_DEVICE symbols set to 'y' or 'n'. >> >> Patch #1 moves the auxdisplay drivers to "Graphics support" Kconfig menu, >> patch #2 moves the core fbdev Kconfig symbols to a separate Kconfig file, >> patch #3 does the FB symbol split and introduces the FB_CORE symbol and >> finally patch #4 makes the DRM symbol to select FB_CORE if the DRM fbdev >> emualtion support was enabled. >> >> Since this series touches three subsystems (auxdisplay, fbdev and DRM), >> I would like to merge it through DRM with the acks of these maintainers. > > Sure. Ack from me. > > The patch series look good otherwise. You may add: > Acked-by: Helge Deller > Thanks! -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH v6 0/4] Allow disabling all native fbdev drivers and only keeping DRM emulation
Hi Javier,
On 7/19/23 10:15, Javier Martinez Canillas wrote:
This patch series splits the fbdev core support in two different Kconfig
symbols: FB and FB_CORE. The motivation for this is to allow CONFIG_FB to
be disabled, while still having the the core fbdev support needed for the
One "the" too much.
(correcting just because this is a cover letter)
CONFIG_DRM_FBDEV_EMULATION to be enabled. The motivation is automatically
disabling all fbdev drivers instead of having to be disabled individually.
The reason for doing this is that now with simpledrm, there's no need for
the legacy fbdev (e.g: efifb or vesafb) drivers anymore and many distros
now disable them. > But it would simplify the config a lot fo have a single
Kconfig symbol to disable all fbdev drivers.
I suggest to rephrase this, e.g.:
The reason for doing this is that with simpledrm, mainstream Linux distributions
like Fedora, SUSE or Ubuntu can then more easily enable the DRM drivers only
by switching off legacy fbdev drivers with one Kconfig option (e.g. for efifb or
vesafb).
(note: there are quite many other distributions, e.g. debian and the derivates,
which still need the fbdev drivers for the various other architectures)
I've built tested with possible combinations of CONFIG_FB, CONFIG_FB_CORE,
CONFIG_DRM_FBDEV_EMULATION and CONFIG_FB_DEVICE symbols set to 'y' or 'n'.
Patch #1 moves the auxdisplay drivers to "Graphics support" Kconfig menu,
patch #2 moves the core fbdev Kconfig symbols to a separate Kconfig file,
patch #3 does the FB symbol split and introduces the FB_CORE symbol and
finally patch #4 makes the DRM symbol to select FB_CORE if the DRM fbdev
emualtion support was enabled.
Since this series touches three subsystems (auxdisplay, fbdev and DRM),
I would like to merge it through DRM with the acks of these maintainers.
Sure. Ack from me.
The patch series look good otherwise. You may add:
Acked-by: Helge Deller
This is a v6 of the patch-set that addresses issues pointed out by Arnd
Bergmann in the previous v5:
https://lists.freedesktop.org/archives/dri-devel/2023-July/413943.html
Changes in v6:
- Don't move FB_{HECUBA,SVGALIB,MACMODES} to config/Kcore (Arnd Bergmann).
- Fix link error when CONFIG_FB_CORE=y and CONFIG_FB=m (Arnd Bergmann).
Changes in v5:
- Take the auxdisplay/Kconfig source out of "if HAS_IOMEM" (Geert Uytterhoeven).
- Fix ifdef guard check in drivers/video/backlight/backlight.c (Arnd Bergmann).
Changes in v4:
- Fix menuconfig hierarchy that was broken in v3 (Arnd Bergmann).
Changes in v3:
- Really make a hidden symbol by removing the prompt (Arnd Bergmann).
- Change FB_CORE to config instead of menuconfig (Arnd Bergmann).
- Keep "depends on FB" for FIRMWARE_EDID (Arnd Bergmann).
- Compile out fb_backlight.o and fbmon.o that are only needed for FB
(Arnd Bergmann).
- Make FB_DEVICE to depend on FB_CORE instead of selecting it.
- Make the DRM symbol to select FB_CORE if DRM_FBDEV_EMULATION is
enabled (Arnd Bergmann).
- Also make DRM select FB_SYS_HELPERS_DEFERRED if DRM_FBDEV_EMULATION
- Make DRM_FBDEV_EMULATION to depend on DRM instead of DRM_KMS_HELPER.
Changes in v2:
- Keep "depends on FB" for FB_DDC, FB_HECUBA, FB_SVGALIB, FB_MACMODES,
FB_BACKLIGHT, FB_MODE_HELPERS and FB_TILEBLITTING (Arnd Bergmann).
- Don't change the fb.o object name (Arnd Bergmann).
- Make FB_CORE a non-visible Kconfig symbol instead (Thomas Zimmermann).
- Make CONFIG_DRM_FBDEV_EMULATION to select FB_CORE (Thomas Zimmermann).
Javier Martinez Canillas (4):
video: Add auxiliary display drivers to Graphics support menu
fbdev: Move core fbdev symbols to a separate Kconfig file
fbdev: Split frame buffer support in FB and FB_CORE symbols
drm: Make FB_CORE to be selected if DRM fbdev emulation is enabled
arch/x86/Makefile | 2 +-
arch/x86/video/Makefile | 2 +-
drivers/Kconfig | 2 -
drivers/gpu/drm/Kconfig | 7 +-
drivers/video/Kconfig | 2 +
drivers/video/backlight/backlight.c | 6 +-
drivers/video/console/Kconfig | 2 +-
drivers/video/fbdev/Kconfig | 197 ++--
drivers/video/fbdev/core/Kconfig| 190 +++
drivers/video/fbdev/core/Makefile | 10 +-
10 files changed, 218 insertions(+), 202 deletions(-)
create mode 100644 drivers/video/fbdev/core/Kconfig
Re: [PATCH v2 1/2] dt-bindings: add bindings for pcd8544 displays
On 19/07/2023 17:44, Viktar Simanenka wrote: > Signed-off-by: Viktar Simanenka You still miss commit msg. > > V2: deleted oneOf property from compatible > changed prefix from 'philips' to existing vendor prefix 'nxp' > placed `reg = <0>` right after `compatible` That's not where the changelog is put. Please read again the patch, I gave you link to. > > About parameters exposed for controller: > inverted - I had an unbranded display that after reset had all pixels black, > while register was in 'normal operation'. The display I have now is opposite: > all white after reset in normal mode. This parameter should help user keep > same settings in KMS(?) for diverse displays; OK, it was enough to respond to my comment. > voltage-op & temperature-coeff - adjusts contrast for display. voltage-op > cannot be expressed in real units, because resulting equation looks like > V = a + Vop * b, where a and b varies with ambient temperature. Vop is a > coefficient. same story for temperature-coeff - it shifts resulting voltage > curve depending on display usage invironment; > bias - relates to waveforms for LCD segments. default is 4 and should be > changed only if you use external oscillator for display. Most of this could be in property description, not here. Best regards, Krzysztof
[PATCH] drm/syncobj: add DRM_IOCTL_SYNCOBJ_IMPORT/EXPORT_SYNC_FILE
These new ioctls perform a task similar to
DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD/FD_TO_HANDLE with the
IMPORT/EXPORT_SYNC_FILE flag set, except that they allow specifying the
timeline point to import or export the fence to or from on a timeline
syncobj.
This eliminates the need to use a temporary binary syncobj along with
DRM_IOCTL_SYNCOBJ_TRANSFER to achieve such a thing, which is the
technique userspace has had to employ up to this point. While that does
work, it is rather awkward from the programmer's perspective. Since DRM
syncobjs have been proposed as the basis for display server explicit
synchronization protocols, e.g. [1] and [2], providing a more
streamlined interface now seems worthwhile.
[1] https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/90
[2] https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/967
Accompanying userspace patches...
IGT:
https://gitlab.freedesktop.org/ekurzinger/igt-gpu-tools/-/commit/241e7f379aeaa9b22a32277e77ad4011c8717a57
libdrm:
https://gitlab.freedesktop.org/ekurzinger/drm/-/commit/b3961a592fc6f8b05f7e3a12413fb58eca2dbfa2
Signed-off-by: Erik Kurzinger
---
drivers/gpu/drm/drm_internal.h | 4 +++
drivers/gpu/drm/drm_ioctl.c| 4 +++
drivers/gpu/drm/drm_syncobj.c | 60 ++
include/uapi/drm/drm.h | 9 +
4 files changed, 71 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index d7e023bbb0d5..64a28ed26a16 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -253,6 +253,10 @@ int drm_syncobj_timeline_signal_ioctl(struct drm_device
*dev, void *data,
struct drm_file *file_private);
int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
+int drm_syncobj_import_sync_file_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_private);
+int drm_syncobj_export_sync_file_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_private);
/* drm_framebuffer.c */
void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 7c9d66ee917d..0344e8e447bc 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -710,6 +710,10 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_QUERY, drm_syncobj_query_ioctl,
DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_IMPORT_SYNC_FILE,
drm_syncobj_import_sync_file_ioctl,
+ DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_EXPORT_SYNC_FILE,
drm_syncobj_export_sync_file_ioctl,
+ DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl,
0),
DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE,
drm_crtc_queue_sequence_ioctl, 0),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl,
DRM_MASTER),
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 0c2be8360525..bf0c1eae353a 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -181,6 +181,13 @@
* Note that if you want to transfer a struct &dma_fence_chain from a given
* point on a timeline syncobj from/into a binary syncobj, you can use the
* point 0 to mean take/replace the fence in the syncobj.
+ *
+ * &DRM_IOCTL_SYNCOBJ_IMPORT_SYNC_FILE and &DRM_IOCTL_SYNCOBJ_EXPORT_SYNC_FILE
+ * let the client import or export the struct &dma_fence_chain of a syncobj
+ * at a particular timeline point from or to a sync file.
+ * These behave similarly to &DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE
+ * and &DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE described above, except
+ * that they accommodate timeline syncobjs in addition to binary syncobjs.
*/
#include
@@ -682,10 +689,11 @@ static int drm_syncobj_fd_to_handle(struct drm_file
*file_private,
}
static int drm_syncobj_import_sync_file_fence(struct drm_file *file_private,
- int fd, int handle)
+ int fd, u64 point, int handle)
{
struct dma_fence *fence = sync_file_get_fence(fd);
struct drm_syncobj *syncobj;
+ int ret = 0;
if (!fence)
return -EINVAL;
@@ -696,14 +704,23 @@ static int drm_syncobj_import_sync_file_fence(struct
drm_file *file_private,
return -ENOENT;
}
- drm_syncobj_replace_fence(syncobj, fence);
+ if (point == 0) {
+ drm_syncobj_replace_fence(syncobj, fence);
+ } else {
+ struct dma_fence_chain *chain = dma_fence_chain_alloc();
+ if (chain) {
Re: [PATCH 2/3] dt-bindings: display: panel: Add panels based on ILITEK ILI9806E
Hey Luca,
On Wed, Jul 19, 2023 at 05:21:46PM +0200, Luca Ceresoli wrote:
> Add bindings for LCD panels based on the ILITEK ILI9806E RGB controller
> connected over SPI and the "ShenZhen New Display Co NDS040480800-V3"
> 480x800 panel based on it.
>
> Signed-off-by: Luca Ceresoli
> ---
> .../display/panel/ilitek,ili9806e.yaml| 69 +++
> MAINTAINERS | 6 ++
> 2 files changed, 75 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
>
> diff --git
> a/Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
> b/Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
> new file mode 100644
> index ..42abc6923065
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/ilitek,ili9806e.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Ilitek ILI9806E display panels
> +
> +maintainers:
> + - Luca Ceresoli
> +
> +description:
> + This binding is for display panels using an Ilitek ILI9806E controller in
> + SPI mode.
I figure you explicitly mention SPI mode here because it also supports
D{P,S}I?
> +
> +allOf:
> + - $ref: panel-common.yaml#
> +
> +properties:
> + compatible:
> +items:
> + - enum:
> + # ShenZhen New Display Co 3.97" 480x800 RGB a-SI TFT LCD
> + - newdisplay,nds040480800-v3
> + - const: ilitek,ili9806e
> +
> + reg: true
> + spi-max-frequency: true
> + reset-gpios: true
> + backlight: true
> + port: true
> +
> +required:
> + - compatible
> + - reg
> + - port
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +#include
> +
> +backlight: backlight {
> +compatible = "gpio-backlight";
> +gpios = <&gpio 22 GPIO_ACTIVE_HIGH>;
> +};
> +spi {
Just a nit, a blank line between properties please. Clearly no
respinning needed for that...
Otherwise,
Reviewed-by: Conor Dooley
Thanks,
Conor.
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +display@0 {
> +compatible = "newdisplay,nds040480800-v3", "ilitek,ili9806e";
> +reg = <0>;
> +spi-max-frequency = <100>;
> +pinctrl-names = "default";
> +pinctrl-0 = <&pinctrl_lcdgpios>;
> +reset-gpios = <&gpio 26 GPIO_ACTIVE_LOW>;
> +backlight = <&backlight>;
> +
> +port {
> +ili9806e_in: endpoint {
> +remote-endpoint = <&lcdif_out>;
> +};
> +};
> +};
> +};
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index aee340630eca..3c38699ee821 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6515,6 +6515,12 @@ T: git git://anongit.freedesktop.org/drm/drm-misc
> F: Documentation/devicetree/bindings/display/ilitek,ili9486.yaml
> F: drivers/gpu/drm/tiny/ili9486.c
>
> +DRM DRIVER FOR ILITEK ILI9806E PANELS
> +M: Luca Ceresoli
> +S: Maintained
> +T: git git://anongit.freedesktop.org/drm/drm-misc
> +F: Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
> +
> DRM DRIVER FOR JADARD JD9365DA-H3 MIPI-DSI LCD PANELS
> M: Jagan Teki
> S: Maintained
> --
> 2.34.1
>
signature.asc
Description: PGP signature
Re: [PATCH v2 1/2] drm: bridge: samsung-dsim: Fix i.MX8M enable flow to meet spec
On Wed, Jul 19, 2023 at 12:05 AM Frieder Schrempf
wrote:
>
> Hi Tim,
>
> On 19.07.23 01:03, Tim Harvey wrote:
> > On Thu, Jul 13, 2023 at 3:01 AM Frieder Schrempf
> > wrote:
> >>
> >> Hi Tim,
> >>
> >> On 13.07.23 09:18, Frieder Schrempf wrote:
> >>> Hi Tim,
> >>>
> >>> On 13.07.23 00:34, Tim Harvey wrote:
> On Wed, May 3, 2023 at 9:33 AM Frieder Schrempf wrote:
> >
> > From: Frieder Schrempf
> >
> > According to the documentation [1] the proper enable flow is:
> >
> > 1. Enable DSI link and keep data lanes in LP-11 (stop state)
> > 2. Disable stop state to bring data lanes into HS mode
> >
> > Currently we do this all at once within enable(), which doesn't
> > allow to meet the requirements of some downstream bridges.
> >
> > To fix this we now enable the DSI in pre_enable() and force it
> > into stop state using the FORCE_STOP_STATE bit in the ESCMODE
> > register until enable() is called where we reset the bit.
> >
> > We currently do this only for i.MX8M as Exynos uses a different
> > init flow where samsung_dsim_init() is called from
> > samsung_dsim_host_transfer().
> >
> > [1]
> > https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation
> >
> > Signed-off-by: Frieder Schrempf
> > ---
> > Changes for v2:
> > * Drop RFC
> > ---
> > drivers/gpu/drm/bridge/samsung-dsim.c | 25 +++--
> > 1 file changed, 23 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
> > b/drivers/gpu/drm/bridge/samsung-dsim.c
> > index e0a402a85787..9775779721d9 100644
> > --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> > @@ -859,6 +859,10 @@ static int samsung_dsim_init_link(struct
> > samsung_dsim *dsi)
> > reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> > reg &= ~DSIM_STOP_STATE_CNT_MASK;
> > reg |=
> > DSIM_STOP_STATE_CNT(driver_data->reg_values[STOP_STATE_CNT]);
> > +
> > + if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
> > + reg |= DSIM_FORCE_STOP_STATE;
> > +
> > samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> >
> > reg = DSIM_BTA_TIMEOUT(0xff) | DSIM_LPDR_TIMEOUT(0x);
> > @@ -1340,6 +1344,9 @@ static void samsung_dsim_atomic_pre_enable(struct
> > drm_bridge *bridge,
> > ret = samsung_dsim_init(dsi);
> > if (ret)
> > return;
> > +
> > + samsung_dsim_set_display_mode(dsi);
> > + samsung_dsim_set_display_enable(dsi, true);
> > }
> > }
> >
> > @@ -1347,9 +1354,16 @@ static void samsung_dsim_atomic_enable(struct
> > drm_bridge *bridge,
> >struct drm_bridge_state
> > *old_bridge_state)
> > {
> > struct samsung_dsim *dsi = bridge_to_dsi(bridge);
> > + u32 reg;
> >
> > - samsung_dsim_set_display_mode(dsi);
> > - samsung_dsim_set_display_enable(dsi, true);
> > + if (samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
> > + samsung_dsim_set_display_mode(dsi);
> > + samsung_dsim_set_display_enable(dsi, true);
> > + } else {
> > + reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> > + reg &= ~DSIM_FORCE_STOP_STATE;
> > + samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> > + }
> >
> > dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
> > }
> > @@ -1358,10 +1372,17 @@ static void samsung_dsim_atomic_disable(struct
> > drm_bridge *bridge,
> > struct drm_bridge_state
> > *old_bridge_state)
> > {
> > struct samsung_dsim *dsi = bridge_to_dsi(bridge);
> > + u32 reg;
> >
> > if (!(dsi->state & DSIM_STATE_ENABLED))
> > return;
> >
> > + if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
> > + reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> > + reg |= DSIM_FORCE_STOP_STATE;
> > + samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> > + }
> > +
> > dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
> > }
> >
> > --
> > 2.40.0
> >
>
> Hi Frieder,
>
> I found this patch to break mipi-dsi display on my board which has:
> - FocalTech FT5406 10pt touch controller (with no interrupt)
> - Powertip PH800480T013-IDF02 compatible panel
> - Toshiba TC358762 compatible DSI to DBI bridge
> - ATTINY based regulator used for backlight controller and panel enable
>
> I enable this via a dt overlay in a
Re: [PATCH v2 2/4] drm/tests: Add test for drm_framebuffer_check_src_coords()
Hi Maxime, thanks for the reviews!
On 7/19/23 04:49, Maxime Ripard wrote:
Hi,
On Tue, Jul 18, 2023 at 03:17:24PM -0300, Carlos Eduardo Gallo Filho wrote:
Add a parametrized test for the drm_framebuffer_check_src_coords function.
Signed-off-by: Carlos Eduardo Gallo Filho
---
drivers/gpu/drm/tests/drm_framebuffer_test.c | 126 +++
1 file changed, 126 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_framebuffer_test.c
b/drivers/gpu/drm/tests/drm_framebuffer_test.c
index f759d9f3b76e..ee92120cd8e9 100644
--- a/drivers/gpu/drm/tests/drm_framebuffer_test.c
+++ b/drivers/gpu/drm/tests/drm_framebuffer_test.c
@@ -9,6 +9,7 @@
#include
#include
+#include
#include
#include
@@ -366,7 +367,132 @@ static void drm_framebuffer_test_to_desc(const struct drm_framebuffer_test *t, c
KUNIT_ARRAY_PARAM(drm_framebuffer_create, drm_framebuffer_create_cases,
drm_framebuffer_test_to_desc);
+/* Parameters for testing drm_framebuffer_check_src_coords function */
+struct check_src_coords_case {
+ const char *name; /* Description of the parameter case */
+ const int expect; /* Expected returned value by the function */
+
+ /* All function args */
+ const uint32_t src_x;
+ const uint32_t src_y;
+ const uint32_t src_w;
+ const uint32_t src_h;
+ const struct drm_framebuffer fb;
+};
+
+static const struct check_src_coords_case check_src_coords_cases[] = {
+ /* Regular case where the source just fit in the framebuffer */
+ { .name = "source inside framebuffer with normal sizes and coordinates",
+ .expect = 0,
+ .src_x = 500 << 16, .src_y = 700 << 16,
+ .src_w = 100 << 16, .src_h = 100 << 16,
I don't think we need to duplicate the << 16 everywhere, this can be
added by the test function.
Maxime
I thought about it, but there's some cases where we have some values
composed
by both a shifted part and a regular one, like ".src_x = (600 << 16) +
1". If we
left to shift everything on the test function we won't be able to have
this kind
of values, which would compromise the test. Or if we just put off the
regular part,
we will deal with a test that won't cover the out-of-bound cases at the
subpixel
level.
Of course this could be implemented by adding some new members to the
case struct,
being each src_{x,y,w,h} composed by two, where one is always shifted,
though I
guess it would be way worse than having the shifts everywhere on the
cases array.
I'll be happy to know about if you have some suggestion of how it can
implemented
without throwing away the non-shifted part!
Thanks,
Carlos
Re: [PATCH v5] Documentation/gpu: Add a VM_BIND async draft document
On 7/15/23 17:45, Thomas Hellström wrote: Add a motivation for and description of asynchronous VM_BIND operation v2: - Fix typos (Nirmoy Das) - Improve the description of a memory fence (Oak Zeng) - Add a reference to the document in the Xe RFC. - Add pointers to sample uAPI suggestions v3: - Address review comments (Danilo Krummrich) - Formatting fixes v4: - Address typos (Francois Dugast) - Explain why in-fences are not allowed for VM_BIND operations for long- running workloads (Matthew Brost) v5: - More typo- and style fixing - Further clarify the implications of disallowing in-fences for VM_BIND operations for long-running workloads (Matthew Brost) Signed-off-by: Thomas Hellström Acked-by: Nirmoy Das --- Documentation/gpu/drm-vm-bind-async.rst | 171 Documentation/gpu/rfc/xe.rst| 4 +- 2 files changed, 173 insertions(+), 2 deletions(-) create mode 100644 Documentation/gpu/drm-vm-bind-async.rst diff --git a/Documentation/gpu/drm-vm-bind-async.rst b/Documentation/gpu/drm-vm-bind-async.rst new file mode 100644 index ..d2b02a38198a --- /dev/null +++ b/Documentation/gpu/drm-vm-bind-async.rst @@ -0,0 +1,171 @@ + +Asynchronous VM_BIND + + +Nomenclature: += + +* ``VRAM``: On-device memory. Sometimes referred to as device local memory. + +* ``gpu_vm``: A GPU address space. Typically per process, but can be shared by + multiple processes. Again, pretty obvious, but I suggest to be explicit "GPU virtual address space". Also, you might want to remove "draft" from the patch subject. Otherwise: Reviewed-by: Danilo Krummrich + +* ``VM_BIND``: An operation or a list of operations to modify a gpu_vm using + an IOCTL. The operations include mapping and unmapping system- or + VRAM memory. + +* ``syncobj``: A container that abstracts synchronization objects. The + synchronization objects can be either generic, like dma-fences or + driver specific. A syncobj typically indicates the type of the + underlying synchronization object. + +* ``in-syncobj``: Argument to a VM_BIND IOCTL, the VM_BIND operation waits + for these before starting. + +* ``out-syncobj``: Argument to a VM_BIND_IOCTL, the VM_BIND operation + signals these when the bind operation is complete. + +* ``memory fence``: A synchronization object, different from a dma-fence. + A memory fence uses the value of a specified memory location to determine + signaled status. A memory fence can be awaited and signaled by both + the GPU and CPU. Memory fences are sometimes referred to as + user-fences, userspace-fences or gpu futexes and do not necessarily obey + the dma-fence rule of signaling within a "reasonable amount of time". + The kernel should thus avoid waiting for memory fences with locks held. + +* ``long-running workload``: A workload that may take more than the + current stipulated dma-fence maximum signal delay to complete and + which therefore needs to set the gpu_vm or the GPU execution context in + a certain mode that disallows completion dma-fences. + +* ``exec function``: An exec function is a function that revalidates all + affected gpu_vmas, submits a GPU command batch and registers the + dma_fence representing the GPU command's activity with all affected + dma_resvs. For completeness, although not covered by this document, + it's worth mentioning that an exec function may also be the + revalidation worker that is used by some drivers in compute / + long-running mode. + +* ``bind context``: A context identifier used for the VM_BIND + operation. VM_BIND operations that use the same bind context can be + assumed, where it matters, to complete in order of submission. No such + assumptions can be made for VM_BIND operations using separate bind contexts. + +* ``UMD``: User-mode driver. + +* ``KMD``: Kernel-mode driver. + + +Synchronous / Asynchronous VM_BIND operation + + +Synchronous VM_BIND +___ +With Synchronous VM_BIND, the VM_BIND operations all complete before the +IOCTL returns. A synchronous VM_BIND takes neither in-fences nor +out-fences. Synchronous VM_BIND may block and wait for GPU operations; +for example swap-in or clearing, or even previous binds. + +Asynchronous VM_BIND + +Asynchronous VM_BIND accepts both in-syncobjs and out-syncobjs. While the +IOCTL may return immediately, the VM_BIND operations wait for the in-syncobjs +before modifying the GPU page-tables, and signal the out-syncobjs when +the modification is done in the sense that the next exec function that +awaits for the out-syncobjs will see the change. Errors are reported +synchronously assuming that the asynchronous part of the job never errors. +In low-memory situations the implementation may block, performing the +VM_BIND synchronously, because there might not be enough memory +immediately available for preparing the asynchronous operation. + +
[PATCH v2 1/2] dt-bindings: add bindings for pcd8544 displays
Signed-off-by: Viktar Simanenka
V2: deleted oneOf property from compatible
changed prefix from 'philips' to existing vendor prefix 'nxp'
placed `reg = <0>` right after `compatible`
About parameters exposed for controller:
inverted - I had an unbranded display that after reset had all pixels black,
while register was in 'normal operation'. The display I have now is opposite:
all white after reset in normal mode. This parameter should help user keep
same settings in KMS(?) for diverse displays;
voltage-op & temperature-coeff - adjusts contrast for display. voltage-op
cannot be expressed in real units, because resulting equation looks like
V = a + Vop * b, where a and b varies with ambient temperature. Vop is a
coefficient. same story for temperature-coeff - it shifts resulting voltage
curve depending on display usage invironment;
bias - relates to waveforms for LCD segments. default is 4 and should be
changed only if you use external oscillator for display.
v1 link:
https://lore.kernel.org/linux-devicetree/20230719092903.316452-1-viteo...@gmail.com/
---
.../bindings/display/nxp,pcd8544.yaml | 94 +++
1 file changed, 94 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/nxp,pcd8544.yaml
diff --git a/Documentation/devicetree/bindings/display/nxp,pcd8544.yaml
b/Documentation/devicetree/bindings/display/nxp,pcd8544.yaml
new file mode 100644
index ..52e40fd0eacb
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/nxp,pcd8544.yaml
@@ -0,0 +1,94 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/nxp,pcd8544.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Philips Semiconductors PCD8544 LCD Display Controller
+
+maintainers:
+ - Viktar Simanenka
+
+description: |
+ Philips Semiconductors PCD8544 LCD Display Controller with SPI control bus.
+ Designed to drive a graphic display of 48 rows and 84 columns,
+ such as Nokia 5110/3310 LCDs.
+
+allOf:
+ - $ref: panel/panel-common.yaml#
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+ compatible:
+enum:
+ - nxp,pcd8544
+
+ dc-gpios:
+maxItems: 1
+description: Data/Command selection pin (D/CX)
+
+ reset-gpios:
+maxItems: 1
+description: Display Reset pin (RST)
+
+ nxp,inverted:
+type: boolean
+description: Display color inversion
+
+ nxp,voltage-op:
+$ref: /schemas/types.yaml#/definitions/uint32
+minimum: 0
+maximum: 127
+description: |
+ Liquid crystall voltage operation coefficient. Determines the LCD
+ controlling voltage on the display segments. Should be adjusted
+ depending on the ambient temperature.
+
+ nxp,temperature-coeff:
+$ref: /schemas/types.yaml#/definitions/uint32
+minimum: 0
+maximum: 3
+description: |
+ Display temperature compensation coefficient. Increases LCD
+ controlling voltage at lower temperatures to maintain optimum
+ contrast.
+
+ nxp,bias:
+$ref: /schemas/types.yaml#/definitions/uint32
+minimum: 0
+maximum: 7
+description: Display bias system coefficient.
+
+required:
+ - compatible
+ - reg
+ - dc-gpios
+ - reset-gpios
+
+unevaluatedProperties: false
+
+examples:
+ - |
+#include
+
+spi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+display@0 {
+compatible = "nxp,pcd8544";
+reg = <0>;
+spi-max-frequency = <800>;
+
+dc-gpios = <&pio 0 3 GPIO_ACTIVE_HIGH>; /* DC=PA3 */
+reset-gpios = <&pio 0 1 GPIO_ACTIVE_HIGH>; /* RESET=PA1 */
+backlight = <&backlight>;
+
+nxp,inverted;
+nxp,voltage-op = <0>;
+nxp,bias = <4>;
+nxp,temperature-coeff = <0>;
+};
+};
+
+...
--
2.34.1
[PATCH v2 2/2] drm/tiny: add display driver for philips pcd8544 display controller
Support for common monochrome LCD displays based on PCD8544 (such as
Nokia 5110/3310 LCD) SPI controlled displays.
Signed-off-by: Viktar Simanenka
V2: checked and fixed with sparse and smatch
changed param prefixes
v1 link:
https://lore.kernel.org/linux-devicetree/20230718080727.323426-1-viteo...@gmail.com/
---
drivers/gpu/drm/tiny/Kconfig | 11 +
drivers/gpu/drm/tiny/Makefile | 1 +
drivers/gpu/drm/tiny/pcd8544.c | 506 +
3 files changed, 518 insertions(+)
create mode 100644 drivers/gpu/drm/tiny/pcd8544.c
diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
index f6889f649bc1..10caa0818253 100644
--- a/drivers/gpu/drm/tiny/Kconfig
+++ b/drivers/gpu/drm/tiny/Kconfig
@@ -172,6 +172,17 @@ config TINYDRM_MI0283QT
DRM driver for the Multi-Inno MI0283QT display panel
If M is selected the module will be called mi0283qt.
+config TINYDRM_PCD8544
+ tristate "DRM support for PCD8544 displays"
+ depends on DRM && SPI
+ select DRM_KMS_HELPER
+ select DRM_GEM_DMA_HELPER
+ select BACKLIGHT_CLASS_DEVICE
+ help
+ DRM driver for PCD8544 (Nokia 5110/3310) 84x48 LCD displays.
+
+ If M is selected the module will be called pcd8544.
+
config TINYDRM_REPAPER
tristate "DRM support for Pervasive Displays RePaper panels (V231)"
depends on DRM && SPI
diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
index 76dde89a044b..75bc112a02f9 100644
--- a/drivers/gpu/drm/tiny/Makefile
+++ b/drivers/gpu/drm/tiny/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_TINYDRM_ILI9225) += ili9225.o
obj-$(CONFIG_TINYDRM_ILI9341) += ili9341.o
obj-$(CONFIG_TINYDRM_ILI9486) += ili9486.o
obj-$(CONFIG_TINYDRM_MI0283QT) += mi0283qt.o
+obj-$(CONFIG_TINYDRM_PCD8544) += pcd8544.o
obj-$(CONFIG_TINYDRM_REPAPER) += repaper.o
obj-$(CONFIG_TINYDRM_ST7586) += st7586.o
obj-$(CONFIG_TINYDRM_ST7735R) += st7735r.o
diff --git a/drivers/gpu/drm/tiny/pcd8544.c b/drivers/gpu/drm/tiny/pcd8544.c
new file mode 100644
index ..73958b302a36
--- /dev/null
+++ b/drivers/gpu/drm/tiny/pcd8544.c
@@ -0,0 +1,506 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * DRM driver for Philips PCD8544 LCD controller/driver.
+ * Compatible with Nokia 5110/3310 84x48 LCD displays.
+ *
+ * Copyright 2023 Viktar Simanenka
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/*
+ * The display is monochrome, every bit in buffer is a pixel.
+ * Display RAM divided into 6 banks along y-axis, each bank 84 bytes along
x-axis.
+ * Driver uses horizontal addressing.
+ */
+
+#define PCD8544_FUNCTIONSET 0x20
+#define PCD8544_DISPLAYCONTROL 0x08
+#define PCD8544_SETYADDR0x40
+#define PCD8544_SETXADDR0x80
+#define PCD8544_SETBIAS 0x10
+#define PCD8544_SETTEMPCOEF 0x04
+#define PCD8544_SETVOP 0x80
+
+#define PCD8544_EXTENDED_INSTRUCTION0x01
+#define PCD8544_VERTICAL_ADDRESSING 0x02
+#define PCD8544_DISPLAYNORMAL 0x04
+#define PCD8544_DISPLAYINVERTED 0x05
+
+struct pcd8544_device {
+ struct drm_device drm;
+ struct drm_simple_display_pipe pipe;
+ struct drm_connector connector;
+ struct drm_display_mode mode;
+ struct spi_device *spi;
+
+ u32 width;
+ u32 height;
+ u8 *tx_buf; // Buffer used for transfer
+ size_t tx_buflen;
+
+ struct backlight_device *backlight;
+ struct gpio_desc *reset;
+ struct gpio_desc *dc;
+
+ u32 inverted;
+ u32 temperature_coeff;
+ u32 bias;
+ u32 voltage_op;
+};
+
+MODULE_PARM_DESC(inverted, "Invert display colors: 1 - enable, 0 - disable");
+MODULE_PARM_DESC(voltage_op, "Vop[6:0] LCD voltage operation coefficient:
0-127 (default: 0)");
+MODULE_PARM_DESC(temperature_coeff, "TC[1:0] Temperature compensation
coefficient: 0-3 (default: 0)");
+MODULE_PARM_DESC(bias, "BS[2:0] Bias system coefficient: 0-7 (default: 4)");
+
+#define drm_to_dev(__dev) container_of(__dev, struct pcd8544_device, drm)
+
+static int pcd8544_spi_transfer(struct spi_device *spi, const void *buf,
size_t len)
+{
+ size_t max_chunk = spi_max_transfer_size(spi);
+ struct spi_transfer tr = {
+ .bits_per_word = 8,
+ .speed_hz = 0,
+ };
+ struct spi_message m;
+ size_t chunk;
+ int ret;
+
+ max_chunk = ALIGN_DOWN(max_chunk, 2);
+
+ spi_message_init_with_transfers(&m, &tr, 1);
+
+ while (len) {
+ chunk = min(len, max_chunk);
+
+ tr.tx_buf = buf;
+ tr.len = chunk;
+ buf += chunk;
+ len -= chunk;
+
+ ret = spi_sync(spi, &m);
+ if (ret)
+
Re: [PATCH] drm: mediatek: mtk_dsi: Fix NO_EOT_PACKET settings/handling
Hi, Jitao:
Do you have any comment?
If you have no comment, I would apply this patch.
Regards,
Chun-Kuang.
AngeloGioacchino Del Regno 於
2023年5月23日 週二 下午6:42寫道：
>
> Due to the initial confusion about MIPI_DSI_MODE_EOT_PACKET, properly
> renamed to MIPI_DSI_MODE_NO_EOT_PACKET, reflecting its actual meaning,
> both the DSI_TXRX_CON register setting for bit (HSTX_)DIS_EOT and the
> later calculation for horizontal sync-active (HSA), back (HBP) and
> front (HFP) porches got incorrect due to the logic being inverted.
>
> This means that a number of settings were wrong because:
> - DSI_TXRX_CON register setting: bit (HSTX_)DIS_EOT should be
>set in order to disable the End of Transmission packet;
> - Horizontal Sync and Back/Front porches: The delta used to
>calculate all of HSA, HBP and HFP should account for the
>additional EOT packet.
>
> Before this change...
> - Bit (HSTX_)DIS_EOT was being set when EOT packet was enabled;
> - For HSA/HBP/HFP delta... all three were wrong, as words were
>added when EOT disabled, instead of when EOT packet enabled!
>
> Invert the logic around flag MIPI_DSI_MODE_NO_EOT_PACKET in the
> MediaTek DSI driver to fix the aforementioned issues.
>
> Fixes: 8b2b99fd7931 ("drm/mediatek: dsi: Fine tune the line time caused by
> EOTp")
> Fixes: 2d52bfba09d1 ("drm/mediatek: add non-continuous clock mode and EOT
> packet control")
> Signed-off-by: AngeloGioacchino Del Regno
>
> ---
> drivers/gpu/drm/mediatek/mtk_dsi.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
> b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 7d5250351193..b0ab38e59db9 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -407,7 +407,7 @@ static void mtk_dsi_rxtx_control(struct mtk_dsi *dsi)
> if (dsi->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS)
> tmp_reg |= HSTX_CKLP_EN;
>
> - if (!(dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET))
> + if (dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET)
> tmp_reg |= DIS_EOT;
>
> writel(tmp_reg, dsi->regs + DSI_TXRX_CTRL);
> @@ -484,7 +484,7 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
> timing->da_hs_zero + timing->da_hs_exit + 3;
>
> delta = dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST ? 18 : 12;
> - delta += dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET ? 2 : 0;
> + delta += dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET ? 0 : 2;
>
> horizontal_frontporch_byte = vm->hfront_porch * dsi_tmp_buf_bpp;
> horizontal_front_back_byte = horizontal_frontporch_byte +
> horizontal_backporch_byte;
> --
> 2.40.1
>
[PATCH v3 3/4] drm: Remove references to removed transitional helpers
The transitional helpers were removed a long time ago, but some
references stuck. Remove them.
Fixes: 21ebe615c16994f3 ("drm: Remove transitional helpers")
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Laurent Pinchart
---
v3:
- Add Reviewed-by,
v2:
- Drop "first part" in drivers/gpu/drm/drm_plane_helper.c.
---
drivers/gpu/drm/drm_plane_helper.c | 12 +-
include/drm/drm_crtc.h | 5 ---
include/drm/drm_modeset_helper_vtables.h | 48 +++-
3 files changed, 23 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/drm_plane_helper.c
b/drivers/gpu/drm/drm_plane_helper.c
index c91e454eba097942..5e95089676ff81ed 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -40,8 +40,8 @@
/**
* DOC: overview
*
- * This helper library has two parts. The first part has support to implement
- * primary plane support on top of the normal CRTC configuration interface.
+ * This helper library contains helpers to implement primary plane support on
+ * top of the normal CRTC configuration interface.
* Since the legacy &drm_mode_config_funcs.set_config interface ties the
primary
* plane together with the CRTC state this does not allow userspace to disable
* the primary plane itself. The default primary plane only expose XRBG and
@@ -51,14 +51,6 @@
* planes, and newly merged drivers must not rely upon these transitional
* helpers.
*
- * The second part also implements transitional helpers which allow drivers to
- * gradually switch to the atomic helper infrastructure for plane updates. Once
- * that switch is complete drivers shouldn't use these any longer, instead
using
- * the proper legacy implementations for update and disable plane hooks
provided
- * by the atomic helpers.
- *
- * Again drivers are strongly urged to switch to the new interfaces.
- *
* The plane helpers share the function table structures with other helpers,
* specifically also the atomic helpers. See &struct drm_plane_helper_funcs for
* the details.
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 8e1cbc75143ef216..8b48a1974da3143c 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -77,11 +77,6 @@ struct drm_plane_helper_funcs;
* intended to indicate whether a full modeset is needed, rather than strictly
* describing what has changed in a commit. See also:
* drm_atomic_crtc_needs_modeset()
- *
- * WARNING: Transitional helpers (like drm_helper_crtc_mode_set() or
- * drm_helper_crtc_mode_set_base()) do not maintain many of the derived control
- * state like @plane_mask so drivers not converted over to atomic helpers
should
- * not rely on these being accurate!
*/
struct drm_crtc_state {
/** @crtc: backpointer to the CRTC */
diff --git a/include/drm/drm_modeset_helper_vtables.h
b/include/drm/drm_modeset_helper_vtables.h
index 965faf082a6d1acb..e3c3ac615909474b 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -59,8 +59,8 @@ enum mode_set_atomic {
/**
* struct drm_crtc_helper_funcs - helper operations for CRTCs
*
- * These hooks are used by the legacy CRTC helpers, the transitional plane
- * helpers and the new atomic modesetting helpers.
+ * These hooks are used by the legacy CRTC helpers and the new atomic
+ * modesetting helpers.
*/
struct drm_crtc_helper_funcs {
/**
@@ -216,9 +216,7 @@ struct drm_crtc_helper_funcs {
*
* This callback is used to update the display mode of a CRTC without
* changing anything of the primary plane configuration. This fits the
-* requirement of atomic and hence is used by the atomic helpers. It is
-* also used by the transitional plane helpers to implement a
-* @mode_set hook in drm_helper_crtc_mode_set().
+* requirement of atomic and hence is used by the atomic helpers.
*
* Note that the display pipe is completely off when this function is
* called. Atomic drivers which need hardware to be running before they
@@ -333,8 +331,8 @@ struct drm_crtc_helper_funcs {
* all updated. Again the recommendation is to just call check helpers
* until a maximal configuration is reached.
*
-* This callback is used by the atomic modeset helpers and by the
-* transitional plane helpers, but it is optional.
+* This callback is used by the atomic modeset helpers, but it is
+* optional.
*
* NOTE:
*
@@ -373,8 +371,8 @@ struct drm_crtc_helper_funcs {
* has picked. See drm_atomic_helper_commit_planes() for a discussion of
* the tradeoffs and variants of plane commit helpers.
*
-* This callback is used by the atomic modeset helpers and by the
-* transitional plane helpers, but it is optional.
+* This callback is used by the atomic modeset helpers, but it is
+* optional.
[PATCH v3 1/4] drm/todo: Add atomic modesetting references
The section about converting existing KMS drivers to atomic modesetting mentions the existence of a conversion guide, but does not reference it. While the guide is old and rusty, it still contains useful information, so add a link to it. Also link to the LWN.net articles that give an overview about the atomic mode setting design. While at it, remove the reference to unconverted virtual HW drivers, as they've been converted. Signed-off-by: Geert Uytterhoeven Reviewed-by: Javier Martinez Canillas Reviewed-by: Laurent Pinchart --- v3: - No changes, v2: - Add Reviewed-by, - Drop double space after full stop, - Use footnotes for references, - Remore reference to unconverted virtual HW drivers. --- Documentation/gpu/todo.rst | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index b05b32c12975559b..b93059e384128904 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -49,14 +49,18 @@ converted over. Modern compositors like Wayland or Surfaceflinger on Android really want an atomic modeset interface, so this is all about the bright future. -There is a conversion guide for atomic and all you need is a GPU for a -non-converted driver (again virtual HW drivers for KVM are still all -suitable). +There is a conversion guide for atomic [1]_ and all you need is a GPU for a +non-converted driver. The "Atomic mode setting design overview" series [2]_ +[3]_ at LWN.net can also be helpful. As part of this drivers also need to convert to universal plane (which means exposing primary & cursor as proper plane objects). But that's much easier to do by directly using the new atomic helper driver callbacks. + .. [1] https://blog.ffwll.ch/2014/11/atomic-modeset-support-for-kms-drivers.html + .. [2] https://lwn.net/Articles/653071/ + .. [3] https://lwn.net/Articles/653466/ + Contact: Daniel Vetter, respective driver maintainers Level: Advanced -- 2.34.1
[PATCH v3 0/4] drm: Atomic modesetting doc and comment improvements
Hi all,
This patch series contains various improvements to the documentation and
comments related to atomic modesetting. Hopefully, it will ease the job
of DRM novice who want to tackle the daunting task of converting a
legacy DRM driver to atomic modesetting.
Changes compared to v2[1]:
- Make main text read correctly when ignoring the footnotes,
- Add Reviewed-by.
Changes compared to v1[2]:
- Add Reviewed-by,
- Drop double space after full stop,
- Use footnotes for references,
- Remore reference to unconverted virtual HW drivers,
- New patch [2/4],
- Drop "first part" in drivers/gpu/drm/drm_plane_helper.c.
Thanks for applying!
[1] https://lore.kernel.org/r/cover.1686318012.git.geert+rene...@glider.be
[2] https://lore.kernel.org/r/cover.1685696114.git.geert+rene...@glider.be
Geert Uytterhoeven (4):
drm/todo: Add atomic modesetting references
drm/todo: Convert list of fbconv links to footnotes
drm: Remove references to removed transitional helpers
drm: Fix references to drm_plane_helper_check_state()
Documentation/gpu/todo.rst| 22 +
drivers/gpu/drm/drm_plane_helper.c| 12 +
.../gpu/drm/renesas/rcar-du/rcar_du_plane.c | 3 +-
drivers/gpu/drm/tidss/tidss_plane.c | 3 +-
include/drm/drm_crtc.h| 5 --
include/drm/drm_modeset_helper_vtables.h | 48 ---
6 files changed, 40 insertions(+), 53 deletions(-)
--
2.34.1
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
[PATCH v3 2/4] drm/todo: Convert list of fbconv links to footnotes
Convert the references to fbconv links to footnotes, so they can be navigated. Signed-off-by: Geert Uytterhoeven --- v3: - Make main text read correctly when ignoring the footnotes, v2: - New. --- Documentation/gpu/todo.rst | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index b93059e384128904..f2a4f6f90c54eefe 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -756,16 +756,16 @@ existing hardware. The new driver's call-back functions are filled from existing fbdev code. More complex fbdev drivers can be refactored step-by-step into a DRM -driver with the help of the DRM fbconv helpers. [1] These helpers provide +driver with the help of the DRM fbconv helpers [4]_. These helpers provide the transition layer between the DRM core infrastructure and the fbdev driver interface. Create a new DRM driver on top of the fbconv helpers, copy over the fbdev driver, and hook it up to the DRM code. Examples for -several fbdev drivers are available at [1] and a tutorial of this process -available at [2]. The result is a primitive DRM driver that can run X11 -and Weston. +several fbdev drivers are available in Thomas Zimmermann's fbconv tree +[4]_, as well as a tutorial of this process [5]_. The result is a primitive +DRM driver that can run X11 and Weston. - - [1] https://gitlab.freedesktop.org/tzimmermann/linux/tree/fbconv - - [2] https://gitlab.freedesktop.org/tzimmermann/linux/blob/fbconv/drivers/gpu/drm/drm_fbconv_helper.c + .. [4] https://gitlab.freedesktop.org/tzimmermann/linux/tree/fbconv + .. [5] https://gitlab.freedesktop.org/tzimmermann/linux/blob/fbconv/drivers/gpu/drm/drm_fbconv_helper.c Contact: Thomas Zimmermann -- 2.34.1
[PATCH v3 4/4] drm: Fix references to drm_plane_helper_check_state()
As of commit a01cb8ba3f628293 ("drm: Move drm_plane_helper_check_state()
into drm_atomic_helper.c"), drm_plane_helper_check_state() no longer
exists, but is part of drm_atomic_helper_check_plane_state().
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Laurent Pinchart
---
v3:
- No changes,
v2:
- Add Reviewed-by.
---
drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.c | 3 ++-
drivers/gpu/drm/tidss/tidss_plane.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.c
b/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.c
index d759e019218181ce..e445fac8e0b46c21 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.c
@@ -600,7 +600,8 @@ int __rcar_du_plane_atomic_check(struct drm_plane *plane,
if (!state->crtc) {
/*
* The visible field is not reset by the DRM core but only
-* updated by drm_plane_helper_check_state(), set it manually.
+* updated by drm_atomic_helper_check_plane_state(), set it
+* manually.
*/
state->visible = false;
*format = NULL;
diff --git a/drivers/gpu/drm/tidss/tidss_plane.c
b/drivers/gpu/drm/tidss/tidss_plane.c
index 6bdd6e4a955ab3cc..e1c0ef0c3894c855 100644
--- a/drivers/gpu/drm/tidss/tidss_plane.c
+++ b/drivers/gpu/drm/tidss/tidss_plane.c
@@ -38,7 +38,8 @@ static int tidss_plane_atomic_check(struct drm_plane *plane,
if (!new_plane_state->crtc) {
/*
* The visible field is not reset by the DRM core but only
-* updated by drm_plane_helper_check_state(), set it manually.
+* updated by drm_atomic_helper_check_plane_state(), set it
+* manually.
*/
new_plane_state->visible = false;
return 0;
--
2.34.1
[PATCH 3/3] DRM: panel: add Ilitek ILI9806E driver
Add a driver for the ILITEK ILI9806E 480x864 RGB LCD controller connected over SPI, and implement the ShenZhen New Display Co NDS040480800-V3 480x800 panel. Signed-off-by: Luca Ceresoli --- MAINTAINERS | 1 + drivers/gpu/drm/panel/Kconfig | 13 + drivers/gpu/drm/panel/Makefile| 1 + drivers/gpu/drm/panel/panel-ilitek-ili9806e.c | 384 ++ 4 files changed, 399 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9806e.c diff --git a/MAINTAINERS b/MAINTAINERS index 3c38699ee821..4d657a049acc 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6520,6 +6520,7 @@ M:Luca Ceresoli S: Maintained T: git git://anongit.freedesktop.org/drm/drm-misc F: Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml +F: drivers/gpu/drm/panel/panel-ilitek-ili9806e.c DRM DRIVER FOR JADARD JD9365DA-H3 MIPI-DSI LCD PANELS M: Jagan Teki diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 203c0ef0bbfd..e3e89d86668a 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -194,6 +194,19 @@ config DRM_PANEL_ILITEK_ILI9341 QVGA (240x320) RGB panels. support serial & parallel rgb interface. +config DRM_PANEL_ILITEK_ILI9806E + tristate "Ilitek ILI9806E panel" + depends on OF + depends on BACKLIGHT_CLASS_DEVICE + select VIDEOMODE_HELPERS + select DRM_MIPI_DBI + help + Say Y here if you want to enable support for LCD panels connected + over SPI and based on the Ilitek ILI9806E controller. + + The ILI9806E is an LCD controller capable of driving 18-bit a-Si + TFT LCDs up to a resolution of 480x800. + config DRM_PANEL_ILITEK_ILI9881C tristate "Ilitek ILI9881C-based panels" depends on OF diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index 30cf553c8d1d..f465140ae7df 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -17,6 +17,7 @@ obj-$(CONFIG_DRM_PANEL_FEIYANG_FY07024DI26A30D) += panel-feiyang-fy07024di26a30d obj-$(CONFIG_DRM_PANEL_HIMAX_HX8394) += panel-himax-hx8394.o obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9341) += panel-ilitek-ili9341.o +obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9806E) += panel-ilitek-ili9806e.o obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o obj-$(CONFIG_DRM_PANEL_INNOLUX_EJ030NA) += panel-innolux-ej030na.o obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c b/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c new file mode 100644 index ..57c12bff70f8 --- /dev/null +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c @@ -0,0 +1,384 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Driver for the Ilitek ILI9806E a-Si TFT LCD controller. + * + * Copyright (c) 2023 Delcon SRL + * Luca Ceresoli + */ + +#include +#include +#include + +#include +#include +#include +#include +#include + +#include + +#define ILI9806E_BUS_FORMATMEDIA_BUS_FMT_RGB666_1X18 + +// Page 1 registers +#define ILI9806E_P1_IFMODE10x08// Interface Mode Control 1 +#define IFMODE1_SEPT_SDIO BIT(3) // 1 = two data pins +#define IFMODE1_SDO_STATUS BIT(4) // 0 = SDO has output enable +#define ILI9806E_P1_DISCTRL1 0x20// Display Function Control 1 +#define ILI9806E_P1_DISCTRL2 0x21// Display Function Control 2 +#define DISCTRL2_EPL BIT(0) // DE polarity (1 = active high) +#define DISCTRL2_DPL BIT(1) // PCLK polarity (1 = fetch on falling edge) +#define DISCTRL2_HSPL BIT(2) // HS polarity (1 = active high) +#define DISCTRL2_VSPL BIT(3) // VS polarity (1 = active high) +#define ILI9806E_P1_RESCTRL0x30// Resolution Control +#define RESCTRL_480x8640x0 +#define RESCTRL_480x8540x1 +#define RESCTRL_480x8000x2 +#define RESCTRL_480x6400x3 +#define RESCTRL_480x7200x4 +#define ILI9806E_P1_INVTR 0x31// Display Inversion Control +#define INVTR_NLA_COLUMN 0x0 +#define INVTR_NLA_1DOT 0x1 +#define INVTR_NLA_2DOT 0x2 +#define INVTR_NLA_3DOT 0x3 +#define INVTR_NLA_4DOT 0x4 +#define ILI9806E_P1_PWCTRL10x40// Power Control 1 +#define ILI9806E_P1_PWCTRL20x41// Power Control 2 +#define ILI9806E_P1_PWCTRL30x42// Power Control 3 +#define ILI9806E_P1_PWCTRL40x43// Power Control 4 +#define ILI9806E_P1_PWCTRL50x44// Power Control 5 +#define ILI9806E_P1_PWCTRL60x45// Power Control 6 +#define ILI9806E_P1_PWCTRL70x46// Power Control 7 +#define ILI9806E_P1_PWCTRL80x47// Power Control 8 +#define ILI9806E_P1_PWCTR
