Re: [PATCH] dma-buf: heaps: Fix potential spectre v1 gadget
Hello Jordy, On Tue, 1 Feb 2022 at 02:09, John Stultz wrote: > > On Sat, Jan 29, 2022 at 7:06 AM Jordy Zomer wrote: > > > > It appears like nr could be a Spectre v1 gadget as it's supplied by a > > user and used as an array index. Prevent the contents > > of kernel memory from being leaked to userspace via speculative > > execution by using array_index_nospec. > > > > Signed-off-by: Jordy Zomer Thanks very much for your patch; I've pushed it to drm-misc-fixes, so we should see it in mainline soon. > > --- > > drivers/dma-buf/dma-heap.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c > > index 56bf5ad01ad5..8f5848aa144f 100644 > > --- a/drivers/dma-buf/dma-heap.c > > +++ b/drivers/dma-buf/dma-heap.c > > @@ -14,6 +14,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -135,6 +136,7 @@ static long dma_heap_ioctl(struct file *file, unsigned > > int ucmd, > > if (nr >= ARRAY_SIZE(dma_heap_ioctl_cmds)) > > return -EINVAL; > > > > + nr = array_index_nospec(nr, ARRAY_SIZE(dma_heap_ioctl_cmds)); > > /* Get the kernel ioctl cmd that matches */ > > kcmd = dma_heap_ioctl_cmds[nr]; > > Thanks for submitting this! It looks sane to me. > > Acked-by: John Stultz > > thanks > -john Best, Sumit.
Re: [PATCH 00/14] Rename dma-buf-map
Hi Am 01.02.22 um 08:46 schrieb Christian König: [..] 2) If renaming, would it still keep the same entry in MAINTAINERS? Thomas suggested drivers core, but this all seem to be used mainly on drm/, with just one exception. I would just add a complete new entry for this and use Thomas as maintainer (with his permission of course) and dri as mailing list. Sure, no problem. Best regards Thomas 3) If renaming, do we have another preferred name? Nope, as Daniel said the name itself is only bikesheed. What is important is that we see this as functionality separated from the inter driver interface. Regards, Christian. thanks Lucas De Marchi But as I said, I don't really have a preference. When crossing subsystems one thing that is hard is that different people have different preferences on these things. At least squashing now is much easier than if I had to split it Try to imagine how much complain I received on going the other way in 25985edcedea6396277003854657b5f3cb31a628 with 2463 files changed, 4252 insertions(+), 4252 deletions(-) Well exactly that is perfectly fine. What you do here is applying your personal hack which is absolutely not welcomed. Regards, Christian. :) Lucas De Marchi Regards, Christian. I built this series, full config with CONFIG_COMPILE_TEST and doing: git rebase -i -x "make -j$(nproc)" I split these patches in a way that wouldn't break the build on purpose. There were a couple that I couldn't build without cross compiling: tegra and rockchip. The others were ok. I'm not really against squashing everything in one to merge, though. It will be hard on the conflicts later, but should get the job done much quicker. Lucas De Marchi Regards, Christian. Am 28.01.22 um 09:36 schrieb Lucas De Marchi: Motivation for this started in https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20220126203702.1784589-1-lucas.demarchi%40intel.com%2F&data=04%7C01%7Cchristian.koenig%40amd.com%7C01142fa3ce484040ade008d9e51aef5d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637792726123940514%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=p8rR28Hn0yMTbwy%2F7bpiGyG9fAu9kG1VUzX2MF44mcs%3D&reserved=0 when trying to extend the dma-buf-map API to cover new use cases: help a single driver with allocations and sharing code paths for IO and system memory. I'm leaving the API additions aside and first renaming the interface as requested. There are already some users in tree outside the context of dma-buf importer/exporter. So before extending the API, let's dissociate it from dma-buf. The iosys-map.h is introduced in the first patch in a way that allows the conversion of each driver to happen separately. After all the conversions are done we can remove the old one, which is the last patch. Another possible way is to squash everything and merge together, but I believe this would make much harder for review. The conversion was done with the following semantic patch: @r1@ @@ - struct dma_buf_map + struct iosys_map @r2@ @@ ( - DMA_BUF_MAP_INIT_VADDR + IOSYS_MAP_INIT_VADDR | - dma_buf_map_set_vaddr + iosys_map_set_vaddr | - dma_buf_map_set_vaddr_iomem + iosys_map_set_vaddr_iomem | - dma_buf_map_is_equal + iosys_map_is_equal | - dma_buf_map_is_null + iosys_map_is_null | - dma_buf_map_is_set + iosys_map_is_set | - dma_buf_map_clear + iosys_map_clear | - dma_buf_map_memcpy_to + iosys_map_memcpy_to | - dma_buf_map_incr + iosys_map_incr ) @@ @@ - #include + #include and then some files had their includes adjusted so we can build everything on each commit in this series. Also some comments were update to remove mentions to dma-buf-map. Simply doing a sed to rename didn't work as dma-buf has some APIs using the dma_buf_map prefix. Once finalized, I think most of this, if not all, could go through the drm-misc-next branch. I split i915, msm, nouveau, and radeon in their own patches in case it's preferred to take those through their own trees. Lucas De Marchi Lucas De Marchi (14): iosys-map: Introduce renamed dma-buf-map misc: fastrpc: Replace dma-buf-map with iosys-map dma-buf: Replace dma-buf-map with iosys-map media: Replace dma-buf-map with iosys-map drm/ttm: Replace dma-buf-map with iosys-map drm: Replace dma-buf-map with iosys-map in drivers drm/i915: Replace dma-buf-map with iosys-map drm/msm: Replace dma-buf-map with iosys-map drm/nouveau: Replace dma-buf-map with iosys-map drm/tegra: Replace dma-buf-map with iosys-map drm/radeon: Replace dma-buf-map with iosys-map drm: Replace dma-buf-map with iosys-map in common code Documentation: Refer to iosys-map instead of dma-buf-map dma-buf-map: Remove API in favor of iosys-map Documentation/driver-api/dma-buf.rst
Re: [Intel-gfx] [PATCH 21/21] fbdev: Make registered_fb[] private to fbmem.c
Hi Daniel, I love your patch! Yet something to improve: [auto build test ERROR on tegra-drm/drm/tegra/for-next] [also build test ERROR on drm/drm-next linus/master v5.17-rc2 next-20220131] [cannot apply to airlied/drm-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/some-fbcon-patches-mostly-locking/20220201-050907 base: git://anongit.freedesktop.org/tegra/linux.git drm/tegra/for-next config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20220201/202202011603.vczwpod7-...@intel.com/config) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 reproduce (this is a W=1 build): # https://github.com/0day-ci/linux/commit/245da5ab93b17c0cf1521713d5bde655a72efb65 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Daniel-Vetter/some-fbcon-patches-mostly-locking/20220201-050907 git checkout 245da5ab93b17c0cf1521713d5bde655a72efb65 # save the config file to linux build tree mkdir build_dir make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): drivers/staging/olpc_dcon/olpc_dcon.c: In function 'dcon_probe': >> drivers/staging/olpc_dcon/olpc_dcon.c:605:6: error: 'num_registered_fb' >> undeclared (first use in this function); did you mean 'WB_registered'? 605 | if (num_registered_fb < 1) { | ^ | WB_registered drivers/staging/olpc_dcon/olpc_dcon.c:605:6: note: each undeclared identifier is reported only once for each function it appears in >> drivers/staging/olpc_dcon/olpc_dcon.c:610:17: error: 'registered_fb' >> undeclared (first use in this function) 610 | dcon->fbinfo = registered_fb[0]; | ^ vim +605 drivers/staging/olpc_dcon/olpc_dcon.c 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 584 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 585 static int dcon_probe(struct i2c_client *client, const struct i2c_device_id *id) 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 586 { 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 587 struct dcon_priv *dcon; 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 588 int rc, i, j; 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 589 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 590 if (!pdata) 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 591 return -ENXIO; 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 592 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 593 dcon = kzalloc(sizeof(*dcon), GFP_KERNEL); 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 594 if (!dcon) 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 595 return -ENOMEM; 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 596 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 597 dcon->client = client; 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 598 init_waitqueue_head(&dcon->waitq); 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 599 INIT_WORK(&dcon->switch_source, dcon_source_switch); 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 600 dcon->reboot_nb.notifier_call = dcon_reboot_notify; 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 601 dcon->reboot_nb.priority = -1; 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 602 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 603 i2c_set_clientdata(client, dcon); 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 604 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 @605 if (num_registered_fb < 1) { 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 606 dev_err(&client->dev, "DCON driver requires a registered fb\n"); 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 607 rc = -EIO; 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 608 goto einit; 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 609 } 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 @610 dcon->fbinfo = registered_fb[0]; 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 611 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 612 rc = dcon_hw_init(dcon, 1); 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 613 if (rc) 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 614 goto einit; 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 615 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 616 /* Add the DCON device */ 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 617 53c43c5ca13328a Greg Kroah-Hartman 2016-04-04 618 dcon_device = platform_device_alloc("dcon", -1); 53c43c5ca13328a Greg Kroah-Hartman 20
Re: [PULL] drm-misc-next
Op 01-02-2022 om 07:38 schreef Dave Airlie: > On Thu, 27 Jan 2022 at 21:57, Maarten Lankhorst > wrote: >> Hi Dave & Daniel, >> >> First pull for v5.18 > I was trying to be all efficient and get this pulled in time for once. > > > However it broke building on my arm test build. > > The new DP helper Kconfig is wrong somewhere. > > I've attached the .config, but it appears I get DRM_DP_HELPER set to M > but ANALOGIX_DP set to Y and it fails to link because the dp helper > should be Y. > > Regards, > Dave. Below should likely fix it? diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig index 6a251e3aa779..f27cfd2a9726 100644 --- a/drivers/gpu/drm/exynos/Kconfig +++ b/drivers/gpu/drm/exynos/Kconfig @@ -66,6 +66,7 @@ config DRM_EXYNOS_DP bool "Exynos specific extensions for Analogix DP driver" depends on DRM_EXYNOS_FIMD || DRM_EXYNOS7_DECON select DRM_ANALOGIX_DP + select DRM_DP_HELPER default DRM_EXYNOS select DRM_PANEL help diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig index d59dca5efb52..fa5cfda4e90e 100644 --- a/drivers/gpu/drm/rockchip/Kconfig +++ b/drivers/gpu/drm/rockchip/Kconfig @@ -8,6 +8,7 @@ config DRM_ROCKCHIP select DRM_PANEL select VIDEOMODE_HELPERS select DRM_ANALOGIX_DP if ROCKCHIP_ANALOGIX_DP + select DRM_DP_HELPER if ROCKCHIP_ANALOGIX_DP select DRM_DW_HDMI if ROCKCHIP_DW_HDMI select DRM_DW_MIPI_DSI if ROCKCHIP_DW_MIPI_DSI select GENERIC_PHY if ROCKCHIP_DW_MIPI_DSI
Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
On Mon, Jan 31, 2022 at 9:39 PM Simon Ser wrote: > On Monday, January 31st, 2022 at 21:36, Simon Ser wrote: > > > This driver only advertises XRGB in ssd1307_formats. It would be nice to > > expose R8 as well so that user-space can directly produce suitable buffers. > > It would also be nice to have some kind of preferred format, so that > > user-space > > knows R8 is preferred over XRGB. > > Hm, since the format used by the hw is actually R1, adding that to > drm_fourcc.h > would be even better. What's the story with the Rn formats? The comments say "n bpp Red", while this is a monochrome (even inverted) display? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
On Tuesday, February 1st, 2022 at 09:26, Geert Uytterhoeven wrote: > What's the story with the Rn formats? > > The comments say "n bpp Red", while this is a monochrome (even > inverted) display? I don't think the color matters that much. "Red" was picked just because it was an arbitrary color, to make the difference with e.g. C8. Or am I mistaken?
Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
Hi Simon, On Tue, Feb 1, 2022 at 9:34 AM Simon Ser wrote: > On Tuesday, February 1st, 2022 at 09:26, Geert Uytterhoeven > wrote: > > What's the story with the Rn formats? > > > > The comments say "n bpp Red", while this is a monochrome (even > > inverted) display? > > I don't think the color matters that much. "Red" was picked just because it > was > an arbitrary color, to make the difference with e.g. C8. Or am I mistaken? I'd expect 8-bit grayscale to be Y8 instead. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
On Tue, Feb 1, 2022 at 9:34 AM Simon Ser wrote: > > On Tuesday, February 1st, 2022 at 09:26, Geert Uytterhoeven > wrote: > > > What's the story with the Rn formats? > > > > The comments say "n bpp Red", while this is a monochrome (even > > inverted) display? > > I don't think the color matters that much. "Red" was picked just because it > was > an arbitrary color, to make the difference with e.g. C8. Or am I mistaken? The red comes from gl, where with shaders it really doesn't matter what meaning you attach to channels, but really just how many you have. So 2-channel formats are called RxGx, 3-channel RxGxBx, 4-channel RxGxBxAx and single-channel Rx. And we use drm_fourcc for interop in general, hence why these exist. We should probably make a comment that this really isn't a red channel when used for display it's a greyscale/intensity format. Aside from that documentation gap I think reusing Rx formats for greyscale/intensity for display makes perfect sense. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
Hi Javier, On Mon, Jan 31, 2022 at 9:12 PM Javier Martinez Canillas wrote: > This patch series adds a DRM driver for the Solomon OLED SSD1305, SSD1306, > SSD1307 and SSD1309 displays. It is a port of the ssd1307fb fbdev driver. Thanks for your series! I'll give it a try on an Adafruit FeatherWing 128x32 OLED, connected to an OrangeCrab ECP5 FPGA board running a 64 MHz VexRiscv RISC-V softcore. > Using the DRM fb emulation, all the tests from Geert Uytterhoeven's fbtest > (https://git.kernel.org/pub/scm/linux/kernel/git/geert/fbtest.git) passes: > > ./fbtest -f /dev/fb1 > Using drawops cfb32 (32 bpp packed pixels) > Available visuals: > Monochrome > Grayscale 256 > Truecolor 8:8:8:0 Oh, fake 32-bpp truecolor ;-) Does it run modetest, too? I'm trying to get modetest working on my atari DRM driver. Comparing to the cirrus driver doesn't help much, as modetest doesn't seem to work with the cirrus driver (modified to not do hardware access, as I don't have cirrus hardware): # modetest -M cirrus -s 31:1024x768-60Hz setting mode 1024x768-60.00Hz on connectors 31, crtc 34 failed to set gamma: Function not implemented Does there exist another simple test program for showing something using the DRM API? Thanks! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH v3 2/3] drm/i915: Fix header test for !CONFIG_X86
Reviewed-by: Siva Mullati On 31/01/22 10:29 pm, Lucas De Marchi wrote: > Architectures others than x86 have a stub implementation calling > WARN_ON_ONCE(). The appropriate headers need to be included, otherwise > the header-test target will fail with: > > HDRTEST drivers/gpu/drm/i915/i915_mm.h > In file included from : > ./drivers/gpu/drm/i915/i915_mm.h: In function ‘remap_io_mapping’: > ./drivers/gpu/drm/i915/i915_mm.h:26:2: error: implicit declaration of > function ‘WARN_ON_ONCE’ [-Werror=implicit-function-declaration] >26 | WARN_ON_ONCE(1); > | ^~~~ > > v2: Do not include since call to pr_err() has been > removed > > Fixes: 67c430bbaae1 ("drm/i915: Skip remap_io_mapping() for non-x86 > platforms") > Cc: Siva Mullati > Signed-off-by: Lucas De Marchi > --- > > v3: No changes from previous version, just submitting to the right > mailing list > > drivers/gpu/drm/i915/i915_mm.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/i915/i915_mm.h b/drivers/gpu/drm/i915/i915_mm.h > index 76f1d53bdf34..3ad22bbe80eb 100644 > --- a/drivers/gpu/drm/i915/i915_mm.h > +++ b/drivers/gpu/drm/i915/i915_mm.h > @@ -6,6 +6,7 @@ > #ifndef __I915_MM_H__ > #define __I915_MM_H__ > > +#include > #include > > struct vm_area_struct;
Re: [PATCH v11] drm/bridge: add it6505 driver
Il 31/01/22 19:36, Robert Foss ha scritto: On Mon, 31 Jan 2022 at 17:55, Hsin-Yi Wang wrote: On Tue, Feb 1, 2022 at 12:37 AM Robert Foss wrote: On Thu, 20 Jan 2022 at 16:25, AngeloGioacchino Del Regno wrote: Il 14/01/22 10:14, allen ha scritto: This adds support for the iTE IT6505. This device can convert DPI signal to DP output. From: Allen Chen Tested-by: Hsin-yi Wang Signed-off-by: Hermes Wu Signed-off-by: Allen Chen --- v10 -> v11 : remove drm_bridge_new_crtc_state --- drivers/gpu/drm/bridge/Kconfig |8 + drivers/gpu/drm/bridge/Makefile |1 + drivers/gpu/drm/bridge/ite-it6505.c | 3352 +++ 3 files changed, 3361 insertions(+) create mode 100644 drivers/gpu/drm/bridge/ite-it6505.c ...snip... +static const struct of_device_id it6505_of_match[] = { + { .compatible = "ite,it6505" }, + { } +}; If you want to have a DT compatible and DT properties, you have to also add dt-bindings (yaml) for this driver, otherwise, any SoC/device DT will fail the dt binding check So, please, add that. Let me second this. A dt-binding is needed for this driver to be complete, it functions as both documentation and a way to test the DTS that use this device, so it is really important. The binding seems to be accepted before the driver: https://elixir.bootlin.com/linux/v5.16.4/source/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml I completely missed that. In that case we're only missing the reviewed-by tag from someone. You have mine... the intention was to give a Reviewed-by, not a Acked-by - I'm sorry for that, I was sending more than one email and the wrong tag slipped through. So, please change my Acked-by tag to Reviewed-by: AngeloGioacchino Del Regno For the driver by itself, though: Acked-by: AngeloGioacchino Del Regno + +static struct i2c_driver it6505_i2c_driver = { + .driver = { + .name = "it6505", + .of_match_table = it6505_of_match, + .pm = &it6505_bridge_pm_ops, + }, + .probe = it6505_i2c_probe, + .remove = it6505_i2c_remove, + .shutdown = it6505_shutdown, + .id_table = it6505_id, +}; + +module_i2c_driver(it6505_i2c_driver); + +MODULE_AUTHOR("Allen Chen "); +MODULE_DESCRIPTION("IT6505 DisplayPort Transmitter driver"); +MODULE_LICENSE("GPL v2");
Re: [PATCH] drm/bridge_connector: enable HPD by default if supported
Hotplug events reported by bridge drivers over drm_bridge_hpd_notify() get ignored unless somebody calls drm_bridge_hpd_enable(). When the connector for the bridge is bridge_connector, such a call is done from drm_bridge_connector_enable_hpd(). However drm_bridge_connector_enable_hpd() is never called on init paths, documentation suggests that it is intended for suspend/resume paths. H... I'm in two minds about this. The problem description is correct, but I wonder if HPD should be enabled unconditionally here, or if this should be left to display drivers to control. drivers/gpu/drm/imx/dcss/dcss-kms.c enables HPD manually at init time, other drivers don't. It feels like this should be under control of the display controller driver, but I can't think of a use case for not enabling HPD at init time. Any second opinion from anyone ? Hi. Can we somehow move forward here?..
[PATCH 1/3] drm/panel: ltk050h3146w: add mipi_dsi_device.mode_flags to of_match_data
From: Quentin Schulz To prepare for a new display to be supported by this driver which has a slightly different set of DSI mode related flags, let's move the currently hardcoded mode flags to the .data field of of_device_id structure. Cc: Quentin Schulz Signed-off-by: Quentin Schulz --- drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c index a5a414920430..67eb474e28c6 100644 --- a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c +++ b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c @@ -25,6 +25,7 @@ struct ltk050h3146w_cmd { struct ltk050h3146w; struct ltk050h3146w_desc { + const unsigned long mode_flags; const struct drm_display_mode *mode; int (*init)(struct ltk050h3146w *ctx); }; @@ -339,6 +340,8 @@ static const struct drm_display_mode ltk050h3146w_mode = { static const struct ltk050h3146w_desc ltk050h3146w_data = { .mode =lanes = 4; dsi->format = MIPI_DSI_FMT_RGB888; - dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST | - MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET; + dsi->mode_flags = ctx->panel_desc->mode_flags; drm_panel_init(&ctx->panel, &dsi->dev,
[PATCH] drm/panel-simple: Add a timing for the Raspberry Pi 7" panel
From: Dave Stevenson The Raspberry Pi 7" 800x480 panel uses a Toshiba TC358762 DSI to DPI bridge chip, so there is a requirement for the timings to be specified for the end panel. Add such a definition. Signed-off-by: Dave Stevenson Signed-off-by: Detlev Casanova --- drivers/gpu/drm/panel/panel-simple.c | 28 1 file changed, 28 insertions(+) diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index 0c8786ebffd1..c4696626b75d 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -3013,6 +3013,31 @@ static const struct panel_desc qishenglong_gopher2b_lcd = { .connector_type = DRM_MODE_CONNECTOR_DPI, }; +static const struct drm_display_mode raspberrypi_7inch_mode = { + .clock = 25979400 / 1000, + .hdisplay = 800, + .hsync_start = 800 + 2, + .hsync_end = 800 + 2 + 2, + .htotal = 800 + 2 + 2 + 46, + .vdisplay = 480, + .vsync_start = 480 + 7, + .vsync_end = 480 + 7 + 2, + .vtotal = 480 + 7 + 2 + 21, + .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC, +}; + +static const struct panel_desc raspberrypi_7inch = { + .modes = &raspberrypi_7inch_mode, + .num_modes = 1, + .bpc = 8, + .size = { + .width = 154, + .height = 86, + }, + .bus_format = MEDIA_BUS_FMT_RGB888_1X24, + .connector_type = DRM_MODE_CONNECTOR_DSI, +}; + static const struct display_timing rocktech_rk070er9427_timing = { .pixelclock = { 2640, 3330, 4680 }, .hactive = { 800, 800, 800 }, @@ -3958,6 +3983,9 @@ static const struct of_device_id platform_of_match[] = { }, { .compatible = "qishenglong,gopher2b-lcd", .data = &qishenglong_gopher2b_lcd, + }, { + .compatible = "raspberrypi,7inch-dsi", + .data = &raspberrypi_7inch, }, { .compatible = "rocktech,rk070er9427", .data = &rocktech_rk070er9427, -- 2.35.0
[PATCH] dt-bindings: ltk050h3146w: replace Heiko Stuebner by myself as maintainer
From: Quentin Schulz Heiko does not work at Theobroma Systems anymore and the boards using those panels are downstream, maintained internally by the company, so let's relieve Heiko of maintainership duties. Cc: Heiko Stuebner Cc: Quentin Schulz Signed-off-by: Quentin Schulz --- .../devicetree/bindings/display/panel/leadtek,ltk050h3146w.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/display/panel/leadtek,ltk050h3146w.yaml b/Documentation/devicetree/bindings/display/panel/leadtek,ltk050h3146w.yaml index 63d2a00348e9..a40ab887ada7 100644 --- a/Documentation/devicetree/bindings/display/panel/leadtek,ltk050h3146w.yaml +++ b/Documentation/devicetree/bindings/display/panel/leadtek,ltk050h3146w.yaml @@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# title: Leadtek LTK050H3146W 5.0in 720x1280 DSI panel maintainers: - - Heiko Stuebner + - Quentin Schulz allOf: - $ref: panel-common.yaml# -- 2.34.1
[PATCH 2/3] drm/panel: ltk050h3146w: add support for Leadtek LTK050H3148W-CTA6 variant
From: Klaus Goger The LTK050H3148W-CTA6 is a 5.0" 720x1280 DSI display, whose driving controller is a Himax HX8394-F, slightly different from LTK050H3146W by its init sequence, mode details and mode flags. Cc: Quentin Schulz Signed-off-by: Klaus Goger Signed-off-by: Quentin Schulz --- .../drm/panel/panel-leadtek-ltk050h3146w.c| 87 +++ 1 file changed, 87 insertions(+) diff --git a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c index 67eb474e28c6..3be815c3be68 100644 --- a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c +++ b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c @@ -253,6 +253,89 @@ struct ltk050h3146w *panel_to_ltk050h3146w(struct drm_panel *panel) return ret; \ } while (0) +static int ltk050h3148w_init_sequence(struct ltk050h3146w *ctx) +{ + struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev); + int ret; + + /* +* Init sequence was supplied by the panel vendor without much +* documentation. +*/ + dsi_dcs_write_seq(dsi, 0xb9, 0xff, 0x83, 0x94); + dsi_dcs_write_seq(dsi, 0xb1, 0x50, 0x15, 0x75, 0x09, 0x32, 0x44, 0x71, + 0x31, 0x55, 0x2f); + dsi_dcs_write_seq(dsi, 0xba, 0x63, 0x03, 0x68, 0x6b, 0xb2, 0xc0); + dsi_dcs_write_seq(dsi, 0xd2, 0x88); + dsi_dcs_write_seq(dsi, 0xb2, 0x00, 0x80, 0x64, 0x10, 0x07); + dsi_dcs_write_seq(dsi, 0xb4, 0x05, 0x70, 0x05, 0x70, 0x01, 0x70, 0x01, + 0x0c, 0x86, 0x75, 0x00, 0x3f, 0x01, 0x74, 0x01, 0x74, + 0x01, 0x74, 0x01, 0x0c, 0x86); + dsi_dcs_write_seq(dsi, 0xd3, 0x00, 0x00, 0x07, 0x07, 0x40, 0x1e, 0x08, + 0x00, 0x32, 0x10, 0x08, 0x00, 0x08, 0x54, 0x15, 0x10, + 0x05, 0x04, 0x02, 0x12, 0x10, 0x05, 0x07, 0x33, 0x34, + 0x0c, 0x0c, 0x37, 0x10, 0x07, 0x17, 0x11, 0x40); + dsi_dcs_write_seq(dsi, 0xd5, 0x19, 0x19, 0x18, 0x18, 0x1b, 0x1b, 0x1a, + 0x1a, 0x04, 0x05, 0x06, 0x07, 0x00, 0x01, 0x02, 0x03, + 0x20, 0x21, 0x18, 0x18, 0x22, 0x23, 0x18, 0x18, 0x18, + 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, + 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, + 0x18); +dsi_dcs_write_seq(dsi, 0xd6, 0x18, 0x18, 0x19, 0x19, 0x1b, 0x1b, 0x1a, + 0x1a, 0x03, 0x02, 0x01, 0x00, 0x07, 0x06, 0x05, 0x04, + 0x23, 0x22, 0x18, 0x18, 0x21, 0x20, 0x18, 0x18, 0x18, + 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, + 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, + 0x18); + dsi_dcs_write_seq(dsi, 0xe0, 0x00, 0x03, 0x09, 0x11, 0x11, 0x14, 0x18, + 0x16, 0x2e, 0x3d, 0x4d, 0x4d, 0x58, 0x6c, 0x72, 0x78, + 0x88, 0x8b, 0x86, 0xa4, 0xb2, 0x58, 0x55, 0x59, 0x5b, + 0x5d, 0x60, 0x64, 0x7f, 0x00, 0x03, 0x09, 0x0f, 0x11, + 0x14, 0x18, 0x16, 0x2e, 0x3d, 0x4d, 0x4d, 0x58, 0x6d, + 0x73, 0x78, 0x88, 0x8b, 0x87, 0xa5, 0xb2, 0x58, 0x55, + 0x58, 0x5b, 0x5d, 0x61, 0x65, 0x7f); + dsi_dcs_write_seq(dsi, 0xcc, 0x0b); + dsi_dcs_write_seq(dsi, 0xc0, 0x1f, 0x31); + dsi_dcs_write_seq(dsi, 0xb6, 0xc4, 0xc4); + dsi_dcs_write_seq(dsi, 0xbd, 0x01); + dsi_dcs_write_seq(dsi, 0xb1, 0x00); + dsi_dcs_write_seq(dsi, 0xbd, 0x00); + dsi_dcs_write_seq(dsi, 0xc6, 0xef); + dsi_dcs_write_seq(dsi, 0xd4, 0x02); + dsi_dcs_write_seq(dsi, 0x11); + dsi_dcs_write_seq(dsi, 0x29); + + ret = mipi_dsi_dcs_set_tear_on(dsi, 1); + if (ret < 0) { + dev_err(ctx->dev, "failed to set tear on: %d\n", ret); + return ret; + } + + msleep(60); + + return 0; +} + +static const struct drm_display_mode ltk050h3148w_mode = { + .hdisplay = 720, + .hsync_start= 720 + 12, + .hsync_end = 720 + 12 + 6, + .htotal = 720 + 12 + 6 + 24, + .vdisplay = 1280, + .vsync_start= 1280 + 9, + .vsync_end = 1280 + 9 + 2, + .vtotal = 1280 + 9 + 2 + 16, + .clock = 59756, + .width_mm = 62, + .height_mm = 110, +}; + +static const struct ltk050h3146w_desc ltk050h3148w_data = { + .mode =dev); @@ -657,6 +740,10 @@ static const struct of_device_id ltk050h3146w_of_match[] = {
[PATCH 3/3] dt-bindings: ltk050h3146w: add compatible for LTK050H3148W-CTA6 variant
From: Quentin Schulz The LTK050H3148W-CTA6 is a 5.0" 720x1280 DSI display, whose driving controller is a Himax HX8394-F, slightly different from LTK050H3146W by its init sequence, mode details and mode flags. Cc: Quentin Schulz Signed-off-by: Quentin Schulz --- .../devicetree/bindings/display/panel/leadtek,ltk050h3146w.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/panel/leadtek,ltk050h3146w.yaml b/Documentation/devicetree/bindings/display/panel/leadtek,ltk050h3146w.yaml index 3715882b63b6..63d2a00348e9 100644 --- a/Documentation/devicetree/bindings/display/panel/leadtek,ltk050h3146w.yaml +++ b/Documentation/devicetree/bindings/display/panel/leadtek,ltk050h3146w.yaml @@ -17,6 +17,7 @@ properties: enum: - leadtek,ltk050h3146w - leadtek,ltk050h3146w-a2 + - leadtek,ltk050h3148w reg: true backlight: true reset-gpios: true -- 2.34.1
Re: [Intel-gfx] linux-next: build failure after merge of the drm-intel-fixes tree
On 31/01/2022 22:27, Stephen Rothwell wrote: Hi all, After merging the drm-intel-fixes tree, today's linux-next build (x86_64 allmodconfig) failed like this: drivers/gpu/drm/i915/i915_vma.c: In function 'i915_vma_bind': drivers/gpu/drm/i915/i915_vma.c:451:25: error: 'ret' undeclared (first use in this function); did you mean 'net'? 451 | ret = i915_gem_object_wait_moving_fence(vma->obj, true); | ^~~ | net Caused by commit 2e872d87cbf2 ("drm/i915: delete shadow "ret" variable") I have reverted that commit for today. Dropping was the right call - I have since removed it from drm-intel-fixes as well. Root cause was a bad Fixes: tag in that patch which caused it to be wrongly cherry-picked and I did not build test before pushing. We can't edit the wrong Fixes: tag now, so for future reference only, 2e872d87cbf2 ("drm/i915: delete shadow "ret" variable") should not be backported to 5.17 by anyone. Wrong tag: Fixes: f6c466b84cfa ("drm/i915: Add support for moving fence waiting") Correct tag should have been: Fixes: 2f6b90da9192 ("drm/i915: Use vma resources for async unbinding") Regards, Tvrtko
Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
On Tuesday, February 1st, 2022 at 09:43, Geert Uytterhoeven wrote: > Does there exist another simple test program for showing something > using the DRM API? If you're fine with going low-level, there's tentative [1] which can apply an arbitrary KMS state. See for instance [2] for basic mode-setting. [1]: https://git.sr.ht/~emersion/tentative [2]: https://git.sr.ht/~emersion/tentative/tree/master/item/examples/modeset
Re: [PATCH 4/4] MAINTAINERS: Add entry for Solomon SSD1307 OLED displays DRM driver
On Mon, Jan 31, 2022 at 09:15:37PM +0100, Javier Martinez Canillas wrote: > To make sure that tools like the get_maintainer.pl script will suggest > to Cc me if patches are posted for this driver. > > Also include the Device Tree binding for the old ssd1307fb fbdev driver > since the new DRM driver was made compatible with the existing binding. Dunno why you have patches 3 and 4 missed references (in terms of email thread). > +DRM DRIVER FOR SOLOMON SSD1307 OLED DISPLAYS > +M: Javier Martinez Canillas > +S: Maintained > +T: git git://anongit.freedesktop.org/drm/drm-misc > +F: Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml > +F: drivers/gpu/drm/tiny/ssd1307.c I think it makes sense to add ssd1307fb as well. At least you may point out people patching old driver about new one until it's gone completely. -- With Best Regards, Andy Shevchenko
Re: [PATCH 3/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
Hi Javier, please see comments and questions below. Am 31.01.22 um 21:29 schrieb Javier Martinez Canillas: Add a DRM driver for SSD1305, SSD1306, SSD1307 and SSD1309 Solomon OLED controllers that can be programmed via an I2C interface. This is a port of the ssd1307fb driver that already supports these devices. A Device Tree binding is not added because the DRM driver is compatible with the existing binding for the ssd1307fb driver. Signed-off-by: Javier Martinez Canillas --- drivers/gpu/drm/tiny/Kconfig | 12 + drivers/gpu/drm/tiny/Makefile | 1 + drivers/gpu/drm/tiny/ssd1307.c | 976 + 3 files changed, 989 insertions(+) create mode 100644 drivers/gpu/drm/tiny/ssd1307.c diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig index 712e0004e96e..25c9c013bcda 100644 --- a/drivers/gpu/drm/tiny/Kconfig +++ b/drivers/gpu/drm/tiny/Kconfig @@ -157,6 +157,18 @@ config TINYDRM_REPAPER If M is selected the module will be called repaper. +config TINYDRM_SSD1307 TINYDRM is so 2010's. Just call it DRM. Some of the drivers use TINY for historical reasons. Simple pipe and mipi helpers originated from here IIRC. And now it's just regular DRM drivers. + tristate "DRM support for Solomon SSD1307 OLED displays" + depends on DRM && I2C + select DRM_KMS_HELPER + select DRM_GEM_SHMEM_HELPER + select BACKLIGHT_CLASS_DEVICE Alphabetical order please. + help + DRM driver for the SSD1305, SSD1306, SSD1307 and SSD1309 Solomon + OLED controllers that can be programmed via an I2C interface. + + If M is selected the module will be called ssd1307. + config TINYDRM_ST7586 tristate "DRM support for Sitronix ST7586 display panels" depends on DRM && SPI diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile index 5d5505d40e7b..38c4ce96 100644 --- a/drivers/gpu/drm/tiny/Makefile +++ b/drivers/gpu/drm/tiny/Makefile @@ -12,5 +12,6 @@ obj-$(CONFIG_TINYDRM_ILI9341) += ili9341.o obj-$(CONFIG_TINYDRM_ILI9486) += ili9486.o obj-$(CONFIG_TINYDRM_MI0283QT)+= mi0283qt.o obj-$(CONFIG_TINYDRM_REPAPER) += repaper.o +obj-$(CONFIG_TINYDRM_SSD1307) += ssd1307.o Maybe call it ssd130x obj-$(CONFIG_TINYDRM_ST7586) += st7586.o obj-$(CONFIG_TINYDRM_ST7735R) += st7735r.o diff --git a/drivers/gpu/drm/tiny/ssd1307.c b/drivers/gpu/drm/tiny/ssd1307.c new file mode 100644 index ..4ea223674587 --- /dev/null +++ b/drivers/gpu/drm/tiny/ssd1307.c @@ -0,0 +1,976 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * DRM driver for Solomon SSD1307 OLED displays + * + * Copyright 2022 Red Hat Inc. + * + * Based on drivers/video/fbdev/ssd1307fb.c + * Copyright 2012 Free Electrons + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define DRIVER_NAME"ssd1307" +#define DRIVER_DESC"DRM driver for Solomon SSD1307 OLED displays" +#define DRIVER_DATE"20220131" +#define DRIVER_MAJOR 1 +#define DRIVER_MINOR 0 + +#define SSD1307_DATA 0x40 +#define SSD1307_COMMAND0x80 + +#define SSD1307_SET_ADDRESS_MODE 0x20 +#define SSD1307_SET_ADDRESS_MODE_HORIZONTAL(0x00) +#define SSD1307_SET_ADDRESS_MODE_VERTICAL (0x01) +#define SSD1307_SET_ADDRESS_MODE_PAGE (0x02) +#define SSD1307_SET_COL_RANGE 0x21 +#define SSD1307_SET_PAGE_RANGE 0x22 +#define SSD1307_CONTRAST 0x81 +#define SSD1307_SET_LOOKUP_TABLE 0x91 +#define SSD1307_CHARGE_PUMP0x8d +#define SSD1307_SEG_REMAP_ON 0xa1 +#define SSD1307_DISPLAY_OFF0xae +#define SSD1307_SET_MULTIPLEX_RATIO0xa8 +#define SSD1307_DISPLAY_ON 0xaf +#define SSD1307_START_PAGE_ADDRESS 0xb0 +#define SSD1307_SET_DISPLAY_OFFSET 0xd3 +#define SSD1307_SET_CLOCK_FREQ 0xd5 +#define SSD1307_SET_AREA_COLOR_MODE0xd8 +#define SSD1307_SET_PRECHARGE_PERIOD 0xd9 +#define SSD1307_SET_COM_PINS_CONFIG0xda +#define SSD1307_SET_VCOMH 0xdb + +#define MAX_CONTRAST 255 + +struct ssd1307_deviceinfo { + u32 default_vcomh; + u32 default_dclk_div; + u32 default_dclk_frq; + int need_pwm; + int need_chargepump; +}; + +struct ssd1307_device { + struct drm_device drm; + struct drm_simple_display_pipe pipe; + struct drm_display_mode mode; + struct drm_connector connector; + struct i2c_client *client; + + const struct ssd1307_deviceinfo *device_info; + + unsigned area_color_enable : 1; + unsigned c
Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
On Mon, Jan 31, 2022 at 09:56:23PM +0100, Sam Ravnborg wrote: > On Mon, Jan 31, 2022 at 09:12:20PM +0100, Javier Martinez Canillas wrote: ... > > Patch #3 adds the driver. The name ssd1307 was used instead of ssd130x > > (which would be more accurate) to avoid confusion for users who want to > > migrate from the existing ssd1307fb fbdev driver. > Looking forward the name ssd130x would make more sense. There is only so > many existing users and a potential of much more new users. > So in my color of the world the naming that benefits the most users > wins. It depends if the binding is going to be preserved. Also this series doesn't answer to the question what to do with the old driver. If you leave it, I would expect the backward compatibility, otherwise the series misses removal of the old driver. -- With Best Regards, Andy Shevchenko
Re: [PATCH 3/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
On Mon, Jan 31, 2022 at 10:30:46PM +0100, Sam Ravnborg wrote: > On Mon, Jan 31, 2022 at 09:29:16PM +0100, Javier Martinez Canillas wrote: ... > > +config TINYDRM_SSD1307 > > + tristate "DRM support for Solomon SSD1307 OLED displays" > Use SSD130X here - so SSD1306 users can find it. It's better to list them all in the "help". How user would grep this? `git grep -n -i ssd1306` ==> no match. > > + depends on DRM && I2C > > + select DRM_KMS_HELPER > > + select DRM_GEM_SHMEM_HELPER > > + select BACKLIGHT_CLASS_DEVICE > > + help > > + DRM driver for the SSD1305, SSD1306, SSD1307 and SSD1309 Solomon > > + OLED controllers that can be programmed via an I2C interface. > > + > > + If M is selected the module will be called ssd1307. -- With Best Regards, Andy Shevchenko
[Bug 215511] Dual monitor with amd 5700 causes system to hang at startup.
https://bugzilla.kernel.org/show_bug.cgi?id=215511 --- Comment #2 from Jose Mestre (pmes...@gmail.com) --- Hello. I've been unable to compile a specific kernel commit (i did not have too much time to find the docs about how to do it) and i don't know C. I've tried releases for archlinux and i can confirm that linux 5.15.12 worked ok, and 5.15.13 was the first release that make kernel crash with the two monitors switched on. If you can point me how to do it i can bisect, compile and try the kernels. Kind regards. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[PATCH v2] drm: document struct drm_mode_fb_cmd2
Follow-up for the DRM_IOCTL_MODE_GETFB2 docs. v2: (Daniel Stone) - Replace fourcc.org with drm_fourcc.h because this is the authoritative source and the website may have mismatches. - Drop assumption that offsets will generally be 0. - Mention that unused entries must be zero'ed out. Signed-off-by: Simon Ser Reviewed-by: Daniel Vetter Cc: Pekka Paalanen Cc: Daniel Stone --- include/uapi/drm/drm_mode.h | 88 + 1 file changed, 60 insertions(+), 28 deletions(-) diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index e1e351682872..a345404dd315 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -663,41 +663,73 @@ struct drm_mode_fb_cmd { #define DRM_MODE_FB_INTERLACED (1<<0) /* for interlaced framebuffers */ #define DRM_MODE_FB_MODIFIERS (1<<1) /* enables ->modifer[] */ +/** + * struct drm_mode_fb_cmd2 - Frame-buffer metadata. + * + * This struct holds frame-buffer metadata. There are two ways to use it: + * + * - User-space can fill this struct and perform a &DRM_IOCTL_MODE_ADDFB2 + * ioctl to register a new frame-buffer. The new frame-buffer object ID will + * be set by the kernel in @fb_id. + * - User-space can set @fb_id and perform a &DRM_IOCTL_MODE_GETFB2 ioctl to + * fetch metadata about an existing frame-buffer. + * + * In case of planar formats, this struct allows up to 4 buffer objects with + * offsets and pitches per plane. The pitch and offset order is dictated by the + * format FourCC as defined by ``drm_fourcc.h``, e.g. NV12 is described as: + * + * YUV 4:2:0 image with a plane of 8 bit Y samples followed by an + * interleaved U/V plane containing 8 bit 2x2 subsampled colour difference + * samples. + * + * So it would consist of a Y plane at ``offsets[0]`` and a UV plane at + * ``offsets[1]``. + * + * To accommodate tiled, compressed, etc formats, a modifier can be specified. + * For more information see the "Format Modifiers" section. Note that even + * though it looks like we have a modifier per-plane, we in fact do not. The + * modifier for each plane must be identical. Thus all combinations of + * different data layouts for multi-plane formats must be enumerated as + * separate modifiers. + * + * All of the entries in @handles, @pitches, @offsets and @modifier must be + * zero when unused. Warning, for @offsets and @modifier zero can't be used to + * figure out whether the entry is used or not since it's a valid value (a zero + * offset is common, and a zero modifier is &DRM_FORMAT_MOD_LINEAR). + */ struct drm_mode_fb_cmd2 { + /** @fb_id: Object ID of the frame-buffer. */ __u32 fb_id; + /** @width: Width of the frame-buffer. */ __u32 width; + /** @height: Height of the frame-buffer. */ __u32 height; - __u32 pixel_format; /* fourcc code from drm_fourcc.h */ - __u32 flags; /* see above flags */ + /** +* @pixel_format: FourCC format code, see ``DRM_FORMAT_*`` constants in +* ``drm_fourcc.h``. +*/ + __u32 pixel_format; + /** +* @flags: Frame-buffer flags (see &DRM_MODE_FB_INTERLACED and +* &DRM_MODE_FB_MODIFIERS). +*/ + __u32 flags; - /* -* In case of planar formats, this ioctl allows up to 4 -* buffer objects with offsets and pitches per plane. -* The pitch and offset order is dictated by the fourcc, -* e.g. NV12 (https://fourcc.org/yuv.php#NV12) is described as: -* -* YUV 4:2:0 image with a plane of 8 bit Y samples -* followed by an interleaved U/V plane containing -* 8 bit 2x2 subsampled colour difference samples. -* -* So it would consist of Y as offsets[0] and UV as -* offsets[1]. Note that offsets[0] will generally -* be 0 (but this is not required). -* -* To accommodate tiled, compressed, etc formats, a -* modifier can be specified. The default value of zero -* indicates "native" format as specified by the fourcc. -* Vendor specific modifier token. Note that even though -* it looks like we have a modifier per-plane, we in fact -* do not. The modifier for each plane must be identical. -* Thus all combinations of different data layouts for -* multi plane formats must be enumerated as separate -* modifiers. + /** +* @handles: GEM buffer handle, one per plane. Set to 0 if the plane is +* unused. */ __u32 handles[4]; - __u32 pitches[4]; /* pitch for each plane */ - __u32 offsets[4]; /* offset of each plane */ - __u64 modifier[4]; /* ie, tiling, compress */ + /** @pitches: Pitch (aka. stride), one per plane. */ + __u32 pitches[4]; + /** @offsets: Offset into the buffer, one per plane. */ + __u32 offsets[4]; + /** +* @modifier: Format modifier, one per plane. See ``DRM_FORMAT
Re: [PATCH 3/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
On Tue, Feb 01, 2022 at 01:14:22AM +0100, Javier Martinez Canillas wrote: > On 1/31/22 22:30, Sam Ravnborg wrote: > > On Mon, Jan 31, 2022 at 09:29:16PM +0100, Javier Martinez Canillas wrote: ... > > The driver uses the pwms property for the backlight. > > It would have been much better to bite the bullet and require the > > backlight to be specified using a backlight node in the DT. > > This is the standard way to do it and then the driver could use the > > existing backlight driver rather than embedding a backlight driver here. > > > > I did consider that. Because that would allow me to use a struct drm_panel > and as you said make the core to manage the backlight. But then decied to > just keep the backward compatibility with the existing binding to make it > easier for users to migrate to the DRM driver. > > I wonder if we could make the driver to support both ? That is, to query > if there's a backlight with drm_panel_of_backlight() and if not found then > registering it's own backlight like the driver is currently doing. If we keep 100% backward compatibility, just drop the old driver. After all module name is not so important as compatibility strings. The problem with no backward compatibility means that removal of old driver makes users unhappy since DT is kinda ABI and we do not break it. -- With Best Regards, Andy Shevchenko
Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
On 2/1/22 09:38, Daniel Vetter wrote: > On Tue, Feb 1, 2022 at 9:34 AM Simon Ser wrote: >> >> On Tuesday, February 1st, 2022 at 09:26, Geert Uytterhoeven >> wrote: >> >>> What's the story with the Rn formats? >>> >>> The comments say "n bpp Red", while this is a monochrome (even >>> inverted) display? >> >> I don't think the color matters that much. "Red" was picked just because it >> was >> an arbitrary color, to make the difference with e.g. C8. Or am I mistaken? > > The red comes from gl, where with shaders it really doesn't matter > what meaning you attach to channels, but really just how many you > have. So 2-channel formats are called RxGx, 3-channel RxGxBx, > 4-channel RxGxBxAx and single-channel Rx. And we use drm_fourcc for > interop in general, hence why these exist. > > We should probably make a comment that this really isn't a red channel > when used for display it's a greyscale/intensity format. Aside from > that documentation gap I think reusing Rx formats for > greyscale/intensity for display makes perfect sense. > -Daniel To sump up the conversation in the #dri-devel channel, these drivers should support the following formats: 1) Dx (Daniel suggested that for darkness, but inverted mono) 2) Rx (single-channel for grayscale) 3) RxGxBxAx (4-channel fake 32-bpp truecolor) The format preference will be in that order, so if user-space is able to use Dx then there won't be a need for any conversion and just the native format will be used. If using Rx then only a Rx -> Dx conversion will happen and the last format will require the less performant RxGxBxAx -> Rx -> Dx path. But we still need RxGxBxAx as a fallback for compatibility with the existing user-space, so all this could be done as a follow-up as an optimization and shouldn't block monochromatic panel drivers IMO. Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH 3/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
On Tue, Feb 01, 2022 at 10:33:31AM +0100, Thomas Zimmermann wrote: > Am 31.01.22 um 21:29 schrieb Javier Martinez Canillas: > > +obj-$(CONFIG_TINYDRM_SSD1307) += ssd1307.o > > Maybe call it ssd130x Either way it's good to list all supported models in "help" of Kconfig. Also here is the question, should we keep backward compatibility or not? If not, we never can remove the old driver (until last user gone). -- With Best Regards, Andy Shevchenko
Re: [PATCH] drm/i915/ttm: Return some errors instead of trying memcpy move
On 01/02/2022 07:03, Thomas Hellström wrote: The i915_ttm_accel_move() function may return error codes that should be propagated further up the stack rather than consumed assuming that the accel move failed and could be replaced with a memcpy move. For -EINTR, -ERESTARTSYS and -EAGAIN, just propagate those codes, rather than retrying with a memcpy move. Fixes: 2b0a750caf33 ("drm/i915/ttm: Failsafe migration blits") Cc: Matthew Auld Signed-off-by: Thomas Hellström Reviewed-by: Matthew Auld
Re: [PATCH 2/4] drm/format-helper: Add drm_fb_gray8_to_mono_reversed()
Hi Am 31.01.22 um 21:12 schrieb Javier Martinez Canillas: Add support to convert 8-bit grayscale to reversed monochrome for drivers that control monochromatic displays, that only have 1 bit per pixel depth. This helper function was based on repaper_gray8_to_mono_reversed() from the drivers/gpu/drm/tiny/repaper.c driver. You could convert repaper to the new helper. Signed-off-by: Javier Martinez Canillas --- drivers/gpu/drm/drm_format_helper.c | 35 + include/drm/drm_format_helper.h | 2 ++ 2 files changed, 37 insertions(+) diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index 0f28dd2bdd72..bf477c136082 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -584,3 +584,38 @@ int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t dst_for return -EINVAL; } EXPORT_SYMBOL(drm_fb_blit_toio); + +/** + * drm_fb_gray8_to_mono_reversed - Convert grayscale to reversed monochrome + * @dst: reversed monochrome destination buffer + * @src: 8-bit grayscale source buffer + * @clip: Clip rectangle area to copy + * + * DRM doesn't have native monochrome or grayscale support. + * Such drivers can announce the commonly supported XR24 format to userspace + * and use drm_fb_xrgb_to_gray8() to convert to grayscale and then this + * helper function to convert to the native format. + */ +void drm_fb_gray8_to_mono_reversed(void *dst, void *src, const struct drm_rect *clip) IMHO it would be better to have a function that converts xrgb to mono and let it handle the intermediate step of gray8. +{ + size_t width = drm_rect_width(clip); + size_t height = drm_rect_width(clip); + + u8 *mono = dst, *gray8 = src; + unsigned int y, xb, i; + + for (y = 0; y < height; y++) + for (xb = 0; xb < width / 8; xb++) { The inner loop should probably go into a separate helper function. See the drm_fb_*_line() helpers in this file At some point, we's want to have a single blit helper that takes source and destination formats/buffers. It would then pick the correct per-line helper for the conversion. So yeah, we'd want something composable. Best regards Thomas + u8 byte = 0x00; + + for (i = 0; i < 8; i++) { + int x = xb * 8 + i; + + byte >>= 1; + if (gray8[y * width + x] >> 7) + byte |= BIT(7); + } + *mono++ = byte; + } +} +EXPORT_SYMBOL(drm_fb_gray8_to_mono_reversed); diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h index b30ed5de0a33..cd4c8b7c78de 100644 --- a/include/drm/drm_format_helper.h +++ b/include/drm/drm_format_helper.h @@ -43,4 +43,6 @@ int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t dst_for const void *vmap, const struct drm_framebuffer *fb, const struct drm_rect *rect); +void drm_fb_gray8_to_mono_reversed(void *dst, void *vaddr, const struct drm_rect *clip); + #endif /* __LINUX_DRM_FORMAT_HELPER_H */ -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
Hi Am 01.02.22 um 09:36 schrieb Geert Uytterhoeven: Hi Simon, On Tue, Feb 1, 2022 at 9:34 AM Simon Ser wrote: On Tuesday, February 1st, 2022 at 09:26, Geert Uytterhoeven wrote: What's the story with the Rn formats? The comments say "n bpp Red", while this is a monochrome (even inverted) display? I don't think the color matters that much. "Red" was picked just because it was an arbitrary color, to make the difference with e.g. C8. Or am I mistaken? I'd expect 8-bit grayscale to be Y8 instead. I like this naming, but DRM_FORMAT_R8 is uapi already. :/ If anything, we could add Yn formats in addition to existing Rn formats. Best regards Thomas 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 -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
On Tuesday, February 1st, 2022 at 11:08, Thomas Zimmermann wrote: > Am 01.02.22 um 09:36 schrieb Geert Uytterhoeven: > > > I'd expect 8-bit grayscale to be Y8 instead. > > I like this naming, but DRM_FORMAT_R8 is uapi already. :/ If anything, > we could add Yn formats in addition to existing Rn formats. Need to be a bit careful, e.g. Y210 exists and isn't a grayscale format. This could be confusing.
Re: [PATCH 03/21] fbcon: Restore fbcon scrolling acceleration
On 1/31/22 22:05, Daniel Vetter wrote: > This functionally undoes 39aead8373b3 ("fbcon: Disable accelerated > scrolling"), but behind the FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION > option. you have two trivial copy-n-paste errors in this patch which still prevent the console acceleration. > diff --git a/drivers/video/fbdev/core/fbcon.c > b/drivers/video/fbdev/core/fbcon.c > index 2ff90061c7f3..39dc18a5de86 100644 > --- a/drivers/video/fbdev/core/fbcon.c > +++ b/drivers/video/fbdev/core/fbcon.c > @@ -1125,13 +1125,15 @@ static void fbcon_init(struct vc_data *vc, int init) > > ops->graphics = 0; > > - /* > - * No more hw acceleration for fbcon. > - * > - * FIXME: Garbage collect all the now dead code after sufficient time > - * has passed. > - */ > +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION should be: #ifdef CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION > + if ((info->flags & FBINFO_HWACCEL_COPYAREA) && > + !(info->flags & FBINFO_HWACCEL_DISABLED)) > + p->scrollmode = SCROLL_MOVE; > + else /* default to something safe */ > + p->scrollmode = SCROLL_REDRAW; > +#else > p->scrollmode = SCROLL_REDRAW; > +#endif > > /* >* ++guenther: console.c:vc_allocate() relies on initializing > @@ -1971,15 +1973,49 @@ static void updatescrollmode(struct fbcon_display *p, > { > struct fbcon_ops *ops = info->fbcon_par; > int fh = vc->vc_font.height; > + int cap = info->flags; > + u16 t = 0; > + int ypan = FBCON_SWAP(ops->rotate, info->fix.ypanstep, > + info->fix.xpanstep); > + int ywrap = FBCON_SWAP(ops->rotate, info->fix.ywrapstep, t); > int yres = FBCON_SWAP(ops->rotate, info->var.yres, info->var.xres); > int vyres = FBCON_SWAP(ops->rotate, info->var.yres_virtual, > info->var.xres_virtual); > + int good_pan = (cap & FBINFO_HWACCEL_YPAN) && > + divides(ypan, vc->vc_font.height) && vyres > yres; > + int good_wrap = (cap & FBINFO_HWACCEL_YWRAP) && > + divides(ywrap, vc->vc_font.height) && > + divides(vc->vc_font.height, vyres) && > + divides(vc->vc_font.height, yres); > + int reading_fast = cap & FBINFO_READS_FAST; > + int fast_copyarea = (cap & FBINFO_HWACCEL_COPYAREA) && > + !(cap & FBINFO_HWACCEL_DISABLED); > + int fast_imageblit = (cap & FBINFO_HWACCEL_IMAGEBLIT) && > + !(cap & FBINFO_HWACCEL_DISABLED); > > p->vrows = vyres/fh; > if (yres > (fh * (vc->vc_rows + 1))) > p->vrows -= (yres - (fh * vc->vc_rows)) / fh; > if ((yres % fh) && (vyres % fh < yres % fh)) > p->vrows--; > + > + if (good_wrap || good_pan) { > + if (reading_fast || fast_copyarea) > + p->scrollmode = good_wrap ? > + SCROLL_WRAP_MOVE : SCROLL_PAN_MOVE; > + else > + p->scrollmode = good_wrap ? SCROLL_REDRAW : > + SCROLL_PAN_REDRAW; > + } else { > + if (reading_fast || (fast_copyarea && !fast_imageblit)) > + p->scrollmode = SCROLL_MOVE; > + else > + p->scrollmode = SCROLL_REDRAW; > + } > + > +#ifndef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION same here... it needs to be: #ifdef CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION > + p->scrollmode = SCROLL_REDRAW; > +#endif > } > > #define PITCH(w) (((w) + 7) >> 3) > still reviewing the other patches... Helge
Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
Hi Am 01.02.22 um 11:11 schrieb Simon Ser: On Tuesday, February 1st, 2022 at 11:08, Thomas Zimmermann wrote: Am 01.02.22 um 09:36 schrieb Geert Uytterhoeven: I'd expect 8-bit grayscale to be Y8 instead. I like this naming, but DRM_FORMAT_R8 is uapi already. :/ If anything, we could add Yn formats in addition to existing Rn formats. Need to be a bit careful, e.g. Y210 exists and isn't a grayscale format. This could be confusing. Well, ok. How about 'I' as in 'intensity'? There aren't too many drivers supporting this yet. So if we want to find a better name, now's the time. Best regards Thomas -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 03/21] fbcon: Restore fbcon scrolling acceleration
On 2/1/22 11:16, Helge Deller wrote: > On 1/31/22 22:05, Daniel Vetter wrote: >> This functionally undoes 39aead8373b3 ("fbcon: Disable accelerated >> scrolling"), but behind the FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION >> option. > > you have two trivial copy-n-paste errors in this patch which still prevent the > console acceleration. > >> diff --git a/drivers/video/fbdev/core/fbcon.c >> b/drivers/video/fbdev/core/fbcon.c >> index 2ff90061c7f3..39dc18a5de86 100644 >> --- a/drivers/video/fbdev/core/fbcon.c >> +++ b/drivers/video/fbdev/core/fbcon.c >> @@ -1125,13 +1125,15 @@ static void fbcon_init(struct vc_data *vc, int init) >> >> ops->graphics = 0; >> >> -/* >> - * No more hw acceleration for fbcon. >> - * >> - * FIXME: Garbage collect all the now dead code after sufficient time >> - * has passed. >> - */ >> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION > > should be: > #ifdef CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION > > >> +if ((info->flags & FBINFO_HWACCEL_COPYAREA) && >> +!(info->flags & FBINFO_HWACCEL_DISABLED)) >> +p->scrollmode = SCROLL_MOVE; >> +else /* default to something safe */ >> +p->scrollmode = SCROLL_REDRAW; >> +#else >> p->scrollmode = SCROLL_REDRAW; >> +#endif >> >> /* >> * ++guenther: console.c:vc_allocate() relies on initializing >> @@ -1971,15 +1973,49 @@ static void updatescrollmode(struct fbcon_display *p, >> { >> struct fbcon_ops *ops = info->fbcon_par; >> int fh = vc->vc_font.height; >> +int cap = info->flags; >> +u16 t = 0; >> +int ypan = FBCON_SWAP(ops->rotate, info->fix.ypanstep, >> + info->fix.xpanstep); >> +int ywrap = FBCON_SWAP(ops->rotate, info->fix.ywrapstep, t); >> int yres = FBCON_SWAP(ops->rotate, info->var.yres, info->var.xres); >> int vyres = FBCON_SWAP(ops->rotate, info->var.yres_virtual, >> info->var.xres_virtual); >> +int good_pan = (cap & FBINFO_HWACCEL_YPAN) && >> +divides(ypan, vc->vc_font.height) && vyres > yres; >> +int good_wrap = (cap & FBINFO_HWACCEL_YWRAP) && >> +divides(ywrap, vc->vc_font.height) && >> +divides(vc->vc_font.height, vyres) && >> +divides(vc->vc_font.height, yres); >> +int reading_fast = cap & FBINFO_READS_FAST; >> +int fast_copyarea = (cap & FBINFO_HWACCEL_COPYAREA) && >> +!(cap & FBINFO_HWACCEL_DISABLED); >> +int fast_imageblit = (cap & FBINFO_HWACCEL_IMAGEBLIT) && >> +!(cap & FBINFO_HWACCEL_DISABLED); >> >> p->vrows = vyres/fh; >> if (yres > (fh * (vc->vc_rows + 1))) >> p->vrows -= (yres - (fh * vc->vc_rows)) / fh; >> if ((yres % fh) && (vyres % fh < yres % fh)) >> p->vrows--; >> + >> +if (good_wrap || good_pan) { >> +if (reading_fast || fast_copyarea) >> +p->scrollmode = good_wrap ? >> +SCROLL_WRAP_MOVE : SCROLL_PAN_MOVE; >> +else >> +p->scrollmode = good_wrap ? SCROLL_REDRAW : >> +SCROLL_PAN_REDRAW; >> +} else { >> +if (reading_fast || (fast_copyarea && !fast_imageblit)) >> +p->scrollmode = SCROLL_MOVE; >> +else >> +p->scrollmode = SCROLL_REDRAW; >> +} >> + >> +#ifndef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION > > same here... it needs to be: > #ifdef CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION actually: #ifndef CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION > > >> +p->scrollmode = SCROLL_REDRAW; >> +#endif >> } >> >> #define PITCH(w) (((w) + 7) >> 3) >> > > still reviewing the other patches... > > Helge >
Re: [PATCH 01/21] MAINTAINERS: Add entry for fbdev core
Am 31.01.22 um 22:05 schrieb Daniel Vetter: Ever since Tomi extracted the core code in 2014 it's been defacto me maintaining this, with help from others from dri-devel and sometimes Linus (but those are mostly merge conflicts): $ git shortlog -ns drivers/video/fbdev/core/ | head -n5 35 Daniel Vetter 23 Linus Torvalds 10 Hans de Goede 9 Dave Airlie 6 Peter Rosin I think ideally we'd also record that the various firmware fb drivers (efifb, vesafb, ...) are also maintained in drm-misc because for the past few years the patches have either been to fix handover issues with drm drivers, or caused handover issues with drm drivers. So any other tree just doesn't make sense. But also, there's plenty of outdated MAINTAINER entries for these with people and git trees that haven't been active in years, so maybe let's just leave them alone. And furthermore distros are now adopting simpledrm as the firmware fb driver, so hopefully the need to care about the fbdev firmware drivers will go down going forward. Note that drm-misc is group maintained, I expect that to continue like we've done before, so no new expectations that patches all go through my hands. That would be silly. This also means I'm happy to put any other volunteer's name in the M: line, but otherwise git log says I'm the one who's stuck with this. Cc: Dave Airlie Cc: Jani Nikula Cc: Linus Torvalds Cc: Linux Fbdev development list Cc: Pavel Machek Cc: Sam Ravnborg Cc: Greg Kroah-Hartman Cc: Javier Martinez Canillas Cc: DRI Development Cc: Linux Kernel Mailing List Cc: Claudio Suarez Cc: Tomi Valkeinen Cc: Geert Uytterhoeven Cc: Thomas Zimmermann Cc: Daniel Vetter Cc: Sven Schnelle Cc: Gerd Hoffmann Signed-off-by: Daniel Vetter Acked-by: Thomas Zimmermann --- MAINTAINERS | 6 ++ 1 file changed, 6 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index ea3e6c914384..49809eaa3096 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7573,6 +7573,12 @@ S: Maintained W:http://floatingpoint.sourceforge.net/emulator/index.html F:arch/x86/math-emu/ +FRAMEBUFFER CORE +M: Daniel Vetter +F: drivers/video/fbdev/core/ +S: Odd Fixes +T: git git://anongit.freedesktop.org/drm/drm-misc + FRAMEBUFFER LAYER M:Helge Deller L:linux-fb...@vger.kernel.org -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH v11] drm/bridge: add it6505 driver
On Fri, 14 Jan 2022 at 10:29, allen wrote: > > This adds support for the iTE IT6505. > This device can convert DPI signal to DP output. > > From: Allen Chen > Tested-by: Hsin-yi Wang > Signed-off-by: Hermes Wu > Signed-off-by: Allen Chen > --- > v10 -> v11 : remove drm_bridge_new_crtc_state > --- > drivers/gpu/drm/bridge/Kconfig |8 + > drivers/gpu/drm/bridge/Makefile |1 + > drivers/gpu/drm/bridge/ite-it6505.c | 3352 +++ > 3 files changed, 3361 insertions(+) > create mode 100644 drivers/gpu/drm/bridge/ite-it6505.c > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > index 61db5a66b4934..f667fdd87a2cb 100644 > --- a/drivers/gpu/drm/bridge/Kconfig > +++ b/drivers/gpu/drm/bridge/Kconfig > @@ -74,6 +74,14 @@ config DRM_DISPLAY_CONNECTOR > on ARM-based platforms. Saying Y here when this driver is not needed > will not cause any issue. > > +config DRM_ITE_IT6505 > +tristate "ITE IT6505 DisplayPort bridge" > +depends on OF > +select DRM_KMS_HELPER > +select EXTCON > +help > + ITE IT6505 DisplayPort bridge chip driver. > + > config DRM_LONTIUM_LT8912B > tristate "Lontium LT8912B DSI/HDMI bridge" > depends on OF > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile > index f2c73683cfcb7..425844c304953 100644 > --- a/drivers/gpu/drm/bridge/Makefile > +++ b/drivers/gpu/drm/bridge/Makefile > @@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_CHIPONE_ICN6211) += chipone-icn6211.o > obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o > obj-$(CONFIG_DRM_CROS_EC_ANX7688) += cros-ec-anx7688.o > obj-$(CONFIG_DRM_DISPLAY_CONNECTOR) += display-connector.o > +obj-$(CONFIG_DRM_ITE_IT6505) += ite-it6505.o > obj-$(CONFIG_DRM_LONTIUM_LT8912B) += lontium-lt8912b.o > obj-$(CONFIG_DRM_LONTIUM_LT9611) += lontium-lt9611.o > obj-$(CONFIG_DRM_LONTIUM_LT9611UXC) += lontium-lt9611uxc.o > diff --git a/drivers/gpu/drm/bridge/ite-it6505.c > b/drivers/gpu/drm/bridge/ite-it6505.c > new file mode 100644 > index 0..f47cf134e7900 > --- /dev/null > +++ b/drivers/gpu/drm/bridge/ite-it6505.c > @@ -0,0 +1,3352 @@ > +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +/* > + * Copyright (c) 2020, The Linux Foundation. All rights reserved. > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#include > +#include > +#include > +#include > +#include This include has changed location, I manually fixed this in this in the patch. > +#include > +#include > +#include > +#include > + > +#include > + > +#define REG_IC_VER 0x04 > + > +#define REG_RESET_CTRL 0x05 > +#define VIDEO_RESET BIT(0) > +#define AUDIO_RESET BIT(1) > +#define ALL_LOGIC_RESET BIT(2) > +#define AUX_RESET BIT(3) > +#define HDCP_RESET BIT(4) > + > +#define INT_STATUS_01 0x06 > +#define INT_MASK_01 0x09 > +#define INT_HPD_CHANGE 0 > +#define INT_RECEIVE_HPD_IRQ 1 > +#define INT_SCDT_CHANGE 2 > +#define INT_HDCP_FAIL 3 > +#define INT_HDCP_DONE 4 > +#define BIT_OFFSET(x) (((x) - INT_STATUS_01) * BITS_PER_BYTE) > +#define BIT_INT_HPD INT_HPD_CHANGE > +#define BIT_INT_HPD_IRQ INT_RECEIVE_HPD_IRQ > +#define BIT_INT_SCDT INT_SCDT_CHANGE > +#define BIT_INT_HDCP_FAIL INT_HDCP_FAIL > +#define BIT_INT_HDCP_DONE INT_HDCP_DONE > + > +#define INT_STATUS_02 0x07 > +#define INT_MASK_02 0x0A > +#define INT_AUX_CMD_FAIL 0 > +#define INT_HDCP_KSV_CHECK 1 > +#define INT_AUDIO_FIFO_ERROR 2 > +#define BIT_INT_AUX_CMD_FAIL (BIT_OFFSET(0x07) + INT_AUX_CMD_FAIL) > +#define BIT_INT_HDCP_KSV_CHECK (BIT_OFFSET(0x07) + INT_HDCP_KSV_CHECK) > +#define BIT_INT_AUDIO_FIFO_ERROR (BIT_OFFSET(0x07) + INT_AUDIO_FIFO_ERROR) > + > +#define INT_STATUS_03 0x08 > +#define INT_MASK_03 0x0B > +#define INT_LINK_TRAIN_FAIL 4 > +#define INT_VID_FIFO_ERROR 5 > +#define INT_IO_LATCH_FIFO_OVERFLOW 7 > +#define BIT_INT_LINK_TRAIN_FAIL (BIT_OFFSET(0x08) + INT_LINK_TRAIN_FAIL) > +#define BIT_INT_VID_FIFO_ERROR (BIT_OFFSET(0x08) + INT_VID_FIFO_ERROR) > +#define BIT_INT_IO_FIFO_OVERFLOW (BIT_OFFSET(0x08) + > INT_IO_LATCH_FIFO_OVERFLOW) > + > +#define REG_SYSTEM_STS 0x0D > +#define INT_STS BIT(0) > +#define HPD_STS BIT(1) > +#define VIDEO_STB BIT(2) > + > +#define REG_LINK_TRAIN_STS 0x0E > +#define LINK_STATE_CR BIT(2) > +#define LINK_STATE_EQ BIT(3) > +#define LINK_STATE_NORP BIT(4) > + > +#define REG_BANK_SEL 0x0F > +#define REG_CLK_CTRL0 0x10 > +#define M_PCLK_DELAY 0x03 > + > +#define REG_AUX_OPT 0x11 > +#define AUX_AUTO_RST BIT(0) > +#define AUX_FIX_FREQ BIT(3) > + > +#define REG_DATA_CTRL0 0x12 > +#define VIDEO_LATCH_EDGE BIT(4) > +#define ENABLE_PCLK_COUNTER BIT(7) > + > +#define REG_PCLK_COUNTER_VALUE 0x13 > + > +#define REG_501_FIFO_CTRL 0x15 > +#define RST_501_FIFO BIT(1) > + > +#define REG_TRAIN_CTRL0 0x16 > +#define FORCE_LBR BIT(0) > +#de
Re: [PATCH 01/21] MAINTAINERS: Add entry for fbdev core
On 1/31/22 22:05, Daniel Vetter wrote: > Ever since Tomi extracted the core code in 2014 it's been defacto me > maintaining this, with help from others from dri-devel and sometimes > Linus (but those are mostly merge conflicts): > > $ git shortlog -ns drivers/video/fbdev/core/ | head -n5 > 35 Daniel Vetter > 23 Linus Torvalds > 10 Hans de Goede > 9 Dave Airlie > 6 Peter Rosin > > I think ideally we'd also record that the various firmware fb drivers > (efifb, vesafb, ...) are also maintained in drm-misc because for the > past few years the patches have either been to fix handover issues > with drm drivers, or caused handover issues with drm drivers. So any > other tree just doesn't make sense. But also, there's plenty of > outdated MAINTAINER entries for these with people and git trees that > haven't been active in years, so maybe let's just leave them alone. > And furthermore distros are now adopting simpledrm as the firmware fb > driver, so hopefully the need to care about the fbdev firmware drivers > will go down going forward. > > Note that drm-misc is group maintained, I expect that to continue like > we've done before, so no new expectations that patches all go through > my hands. That would be silly. This also means I'm happy to put any > other volunteer's name in the M: line, but otherwise git log says I'm > the one who's stuck with this. Yes, agreed. Acked-by: Helge Deller Since the code is used by drm and existing fbdev drivers, please just make sure to not break fbdev... Thanks! Helge > > Cc: Dave Airlie > Cc: Jani Nikula > Cc: Linus Torvalds > Cc: Linux Fbdev development list > Cc: Pavel Machek > Cc: Sam Ravnborg > Cc: Greg Kroah-Hartman > Cc: Javier Martinez Canillas > Cc: DRI Development > Cc: Linux Kernel Mailing List > Cc: Claudio Suarez > Cc: Tomi Valkeinen > Cc: Geert Uytterhoeven > Cc: Thomas Zimmermann > Cc: Daniel Vetter > Cc: Sven Schnelle > Cc: Gerd Hoffmann > Signed-off-by: Daniel Vetter > --- > MAINTAINERS | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index ea3e6c914384..49809eaa3096 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -7573,6 +7573,12 @@ S: Maintained > W: http://floatingpoint.sourceforge.net/emulator/index.html > F: arch/x86/math-emu/ > > +FRAMEBUFFER CORE > +M: Daniel Vetter > +F: drivers/video/fbdev/core/ > +S: Odd Fixes > +T: git git://anongit.freedesktop.org/drm/drm-misc > + > FRAMEBUFFER LAYER > M: Helge Deller > L: linux-fb...@vger.kernel.org >
Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
Hello Geert, On 2/1/22 09:43, Geert Uytterhoeven wrote: > Hi Javier, > > On Mon, Jan 31, 2022 at 9:12 PM Javier Martinez Canillas > wrote: >> This patch series adds a DRM driver for the Solomon OLED SSD1305, SSD1306, >> SSD1307 and SSD1309 displays. It is a port of the ssd1307fb fbdev driver. > > Thanks for your series! > > I'll give it a try on an Adafruit FeatherWing 128x32 OLED, connected > to an OrangeCrab ECP5 FPGA board running a 64 MHz VexRiscv RISC-V > softcore. > Awesome! let me know if you have any issues. I keep an update-to-date version at https://github.com/martinezjavier/linux/tree/ssd1307 >> Using the DRM fb emulation, all the tests from Geert Uytterhoeven's fbtest >> (https://git.kernel.org/pub/scm/linux/kernel/git/geert/fbtest.git) passes: >> >> ./fbtest -f /dev/fb1 >> Using drawops cfb32 (32 bpp packed pixels) >> Available visuals: >> Monochrome >> Grayscale 256 >> Truecolor 8:8:8:0 > > Oh, fake 32-bpp truecolor ;-) > Yes :) that's what the repaper drivers does to have maximum compatibility with existing user-space and I followed the same. > Does it run modetest, too? > It does, yes. And for example `modetest -M ssd1307` will print all the info about encoders, connectors, CRTs, etc. > I'm trying to get modetest working on my atari DRM driver. > Comparing to the cirrus driver doesn't help much, as modetest doesn't > seem to work with the cirrus driver (modified to not do hardware > access, as I don't have cirrus hardware): > > # modetest -M cirrus -s 31:1024x768-60Hz > setting mode 1024x768-60.00Hz on connectors 31, crtc 34 > failed to set gamma: Function not implemented > # modetest -M ssd1307 -c -s 31:128x64-0.12Hz ... setting mode 128x64-0.12Hz on connectors 31, crtc 33 failed to set gamma: Function not implemented this seems to be a bug in modetest. I found a patch posted some time ago but never landed: https://www.spinics.net/lists/dri-devel/msg251356.html > Does there exist another simple test program for showing something > using the DRM API? > I tested with plymouth and gdm that make use of the DRM API, they do start and I see something on the screen but don't really handle that well the fact that's a 128x64 resolution. I didn't test with more DRM programs because was mostly interested in making sure that the fbdev emulation was working correctly. Noticed that Simon shared some simple examples, I'll give them a try. Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH 03/21] fbcon: Restore fbcon scrolling acceleration
On Tue, Feb 1, 2022 at 11:16 AM Helge Deller wrote: > > On 1/31/22 22:05, Daniel Vetter wrote: > > This functionally undoes 39aead8373b3 ("fbcon: Disable accelerated > > scrolling"), but behind the FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION > > option. > > you have two trivial copy-n-paste errors in this patch which still prevent the > console acceleration. Duh :-( But before we dig into details I think the big picture would be better. I honestly don't like the #ifdef pile here that much. I wonder whether your approach, also with GETVX/YRES adjusted somehow, wouldn't look cleaner? Like I said in the cover letter I got mostly distracted with fbcon locking last week, not really with this one here at all, so maybe going with your 4 (or 2 if we squash them like I did here) patches is neater? Cheers, Daniel > > > diff --git a/drivers/video/fbdev/core/fbcon.c > > b/drivers/video/fbdev/core/fbcon.c > > index 2ff90061c7f3..39dc18a5de86 100644 > > --- a/drivers/video/fbdev/core/fbcon.c > > +++ b/drivers/video/fbdev/core/fbcon.c > > @@ -1125,13 +1125,15 @@ static void fbcon_init(struct vc_data *vc, int init) > > > > ops->graphics = 0; > > > > - /* > > - * No more hw acceleration for fbcon. > > - * > > - * FIXME: Garbage collect all the now dead code after sufficient time > > - * has passed. > > - */ > > +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION > > should be: > #ifdef CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION > > > > + if ((info->flags & FBINFO_HWACCEL_COPYAREA) && > > + !(info->flags & FBINFO_HWACCEL_DISABLED)) > > + p->scrollmode = SCROLL_MOVE; > > + else /* default to something safe */ > > + p->scrollmode = SCROLL_REDRAW; > > +#else > > p->scrollmode = SCROLL_REDRAW; > > +#endif > > > > /* > >* ++guenther: console.c:vc_allocate() relies on initializing > > @@ -1971,15 +1973,49 @@ static void updatescrollmode(struct fbcon_display > > *p, > > { > > struct fbcon_ops *ops = info->fbcon_par; > > int fh = vc->vc_font.height; > > + int cap = info->flags; > > + u16 t = 0; > > + int ypan = FBCON_SWAP(ops->rotate, info->fix.ypanstep, > > + info->fix.xpanstep); > > + int ywrap = FBCON_SWAP(ops->rotate, info->fix.ywrapstep, t); > > int yres = FBCON_SWAP(ops->rotate, info->var.yres, info->var.xres); > > int vyres = FBCON_SWAP(ops->rotate, info->var.yres_virtual, > > info->var.xres_virtual); > > + int good_pan = (cap & FBINFO_HWACCEL_YPAN) && > > + divides(ypan, vc->vc_font.height) && vyres > yres; > > + int good_wrap = (cap & FBINFO_HWACCEL_YWRAP) && > > + divides(ywrap, vc->vc_font.height) && > > + divides(vc->vc_font.height, vyres) && > > + divides(vc->vc_font.height, yres); > > + int reading_fast = cap & FBINFO_READS_FAST; > > + int fast_copyarea = (cap & FBINFO_HWACCEL_COPYAREA) && > > + !(cap & FBINFO_HWACCEL_DISABLED); > > + int fast_imageblit = (cap & FBINFO_HWACCEL_IMAGEBLIT) && > > + !(cap & FBINFO_HWACCEL_DISABLED); > > > > p->vrows = vyres/fh; > > if (yres > (fh * (vc->vc_rows + 1))) > > p->vrows -= (yres - (fh * vc->vc_rows)) / fh; > > if ((yres % fh) && (vyres % fh < yres % fh)) > > p->vrows--; > > + > > + if (good_wrap || good_pan) { > > + if (reading_fast || fast_copyarea) > > + p->scrollmode = good_wrap ? > > + SCROLL_WRAP_MOVE : SCROLL_PAN_MOVE; > > + else > > + p->scrollmode = good_wrap ? SCROLL_REDRAW : > > + SCROLL_PAN_REDRAW; > > + } else { > > + if (reading_fast || (fast_copyarea && !fast_imageblit)) > > + p->scrollmode = SCROLL_MOVE; > > + else > > + p->scrollmode = SCROLL_REDRAW; > > + } > > + > > +#ifndef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION > > same here... it needs to be: > #ifdef CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION > > > > + p->scrollmode = SCROLL_REDRAW; > > +#endif > > } > > > > #define PITCH(w) (((w) + 7) >> 3) > > > > still reviewing the other patches... > > Helge -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 09/21] fbcon: Replace FBCON_FLAGS_INIT with a boolean
Am 31.01.22 um 22:05 schrieb Daniel Vetter: It's only one flag and slightly tidier code. Signed-off-by: Daniel Vetter Cc: Daniel Vetter Cc: Tetsuo Handa Cc: Greg Kroah-Hartman Cc: Du Cheng Cc: Thomas Zimmermann Cc: Claudio Suarez Acked-by: Thomas Zimmermann --- drivers/video/fbdev/core/fbcon.c | 11 +-- drivers/video/fbdev/core/fbcon.h | 4 +--- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index affb40658fee..fa30e1909164 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -773,7 +773,7 @@ static void con2fb_init_display(struct vc_data *vc, struct fb_info *info, ops->currcon = fg_console; - if (info->fbops->fb_set_par && !(ops->flags & FBCON_FLAGS_INIT)) { + if (info->fbops->fb_set_par && !ops->initialized) { ret = info->fbops->fb_set_par(info); if (ret) @@ -782,7 +782,7 @@ static void con2fb_init_display(struct vc_data *vc, struct fb_info *info, "error code %d\n", ret); } - ops->flags |= FBCON_FLAGS_INIT; + ops->initialized = true; ops->graphics = 0; fbcon_set_disp(info, &info->var, unit); @@ -1101,8 +1101,7 @@ static void fbcon_init(struct vc_data *vc, int init) * We need to do it in fbcon_init() to prevent screen corruption. */ if (con_is_visible(vc) && vc->vc_mode == KD_TEXT) { - if (info->fbops->fb_set_par && - !(ops->flags & FBCON_FLAGS_INIT)) { + if (info->fbops->fb_set_par && !ops->initialized) { ret = info->fbops->fb_set_par(info); if (ret) @@ -,7 +1110,7 @@ static void fbcon_init(struct vc_data *vc, int init) "error code %d\n", ret); } - ops->flags |= FBCON_FLAGS_INIT; + ops->initialized = true; } ops->graphics = 0; @@ -1186,7 +1185,7 @@ static void fbcon_deinit(struct vc_data *vc) if (con_is_visible(vc)) fbcon_del_cursor_work(info); - ops->flags &= ~FBCON_FLAGS_INIT; + ops->initialized = false; finished: fbcon_free_font(p, free_font); diff --git a/drivers/video/fbdev/core/fbcon.h b/drivers/video/fbdev/core/fbcon.h index dce5ce41093e..b18d0cbf73b6 100644 --- a/drivers/video/fbdev/core/fbcon.h +++ b/drivers/video/fbdev/core/fbcon.h @@ -18,8 +18,6 @@ #include -#define FBCON_FLAGS_INIT 1 - /* *This is the interface between the low-level console driver and the *low-level frame buffer device @@ -77,7 +75,7 @@ struct fbcon_ops { intblank_state; intgraphics; intsave_graphics; /* for debug enter/leave */ - intflags; + bool initialized; This will add 3 bytes of padding. Maybe you can put the bool elsewhere. introtate; intcur_rotate; char *cursor_data; -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 08/20] drm/i915/buddy: adjust res->start
On Wed, 2022-01-26 at 15:21 +, Matthew Auld wrote: > Differentiate between mappable vs non-mappable resources, also if > this > is an actual range allocation ensure we set res->start as the > starting > pfn. Later when we need to do non-mappable -> mappable moves then we > want TTM to see that the current placement is not compatible, which > should result in an actual move, instead of being turned into a noop. > > Signed-off-by: Matthew Auld > Cc: Thomas Hellström Reviewed-by: Thomas Hellström > --- > drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c > b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c > index 6e5842155898..bc725a92fc5c 100644 > --- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c > +++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c > @@ -140,6 +140,13 @@ static int i915_ttm_buddy_man_alloc(struct > ttm_resource_manager *man, > mutex_unlock(&bman->lock); > } > > + if (place->lpfn - place->fpfn == n_pages) > + bman_res->base.start = place->fpfn; > + else if (lpfn <= bman->visible_size) > + bman_res->base.start = 0; > + else > + bman_res->base.start = bman->visible_size; > + > *res = &bman_res->base; > return 0; >
Re: [PATCH 09/20] drm/i915/buddy: tweak 2big check
On Wed, 2022-01-26 at 15:21 +, Matthew Auld wrote: > Otherwise we get -EINVAL, instead of the more useful -E2BIG if the > allocation doesn't fit within the pfn range, like with mappable lmem. > The hugepages selftest, for example, needs this to know if a smaller > size is needed. > > Signed-off-by: Matthew Auld > Cc: Thomas Hellström Reviewed-by: Thomas Hellström > --- > drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c > b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c > index bc725a92fc5c..7c24cc6608e3 100644 > --- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c > +++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c > @@ -81,7 +81,7 @@ static int i915_ttm_buddy_man_alloc(struct > ttm_resource_manager *man, > lpfn = pages; > } > > - if (size > mm->size) { > + if (size > lpfn << PAGE_SHIFT) { > err = -E2BIG; > goto err_free_res; > }
[PATCH v5 00/19] drm/i915/dg2: Enabling 64k page size and flat ccs
This series introduces the enabling patches for new memory compression feature Flat CCS and 64k page support for i915 local memory, along with documentation on the uAPI impact. Included the details of the feature and the implications on the uAPI below. Which is also added into Documentation/gpu/rfc/i915_dg2.rst DG2 64K page size support: = On discrete platforms, starting from DG2, we have to contend with GTT page size restrictions when dealing with I915_MEMORY_CLASS_DEVICE objects. Specifically the hardware only supports 64K or larger GTT page sizes for such memory. The kernel will already ensure that all I915_MEMORY_CLASS_DEVICE memory is allocated using 64K or larger page sizes underneath. Note that the returned size here will always reflect any required rounding up done by the kernel, i.e 4K will now become 64K on devices such as DG2. Special DG2 GTT address alignment requirement: The GTT alignment will also need to be at least 2M for such objects. Note that due to how the hardware implements 64K GTT page support, we have some further complications: 1) The entire PDE (which covers a 2MB virtual address range), must contain only 64K PTEs, i.e mixing 4K and 64K PTEs in the same PDE is forbidden by the hardware. 2) We still need to support 4K PTEs for I915_MEMORY_CLASS_SYSTEM objects. To keep things simple for userland, we mandate that any GTT mappings must be aligned to and rounded up to 2MB. As this only wastes virtual address space and avoids userland having to copy any needlessly complicated PDE sharing scheme (coloring) and only affects DG2, this is deemed to be a good compromise. Flat CCS support for lmem = On Xe-HP and later devices, we use dedicated compression control state (CCS) stored in local memory for each surface, to support the 3D and media compression formats. The memory required for the CCS of the entire local memory is 1/256 of the local memory size. So before the kernel boot, the required memory is reserved for the CCS data and a secure register will be programmed with the CCS base address. Flat CCS data needs to be cleared when a lmem object is allocated. And CCS data can be copied in and out of CCS region through XY_CTRL_SURF_COPY_BLT. CPU can’t access the CCS data directly. When we exaust the lmem, if the object’s placements support smem, then we can directly decompress the compressed lmem object into smem and start using it from smem itself. But when we need to swapout the compressed lmem object into a smem region though objects’ placement doesn’t support smem, then we copy the lmem content as it is into smem region along with ccs data (using XY_CTRL_SURF_COPY_BLT). When the object is referred, lmem content will be swaped in along with restoration of the CCS data (using XY_CTRL_SURF_COPY_BLT) at corresponding location. Flat-CCS Modifiers for different compression formats I915_FORMAT_MOD_4_TILED_DG2_RC_CCS - used to indicate the buffers of Flat CCS render compression formats. Though the general layout is same as I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS, new hashing/compression algorithm is used. Render compression uses 128 byte compression blocks I915_FORMAT_MOD_4_TILED_DG2_MC_CCS -used to indicate the buffers of Flat CCS media compression formats. Though the general layout is same as I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS, new hashing/compression algorithm is used. Media compression uses 256 byte compression blocks. I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC - used to indicate the buffers of Flat CCS clear color render compression formats. Unified compression format for clear color render compression. The genral layout is a tiled layout using 4Kb tiles i.e Tile4 layout. Fast clear color value expected by HW is located in fb at offset 0 of plane#1 v2: Fixed some formatting issues and platform naming issues Added some more documentation on Flat-CCS v3: Plane programming is handled for flat-ccs and clear color Tile4 and flat ccs modifier patches are rebased on table based modifier reference method Three patches are squashed Y tile is pruned for DG2. flat_ccs_cc plane format info is added Added mesa, compute and media ppl for required uAPI ack. v4: Rebasing of the patches v5: KDoc is enhanced for cc modifier. [Nanley & Lionel] inbuild macro usage for functional fix [Bob] Addressed review comments from Matt Platform coverage fix for modifiers [Imre] Abdiel Janulgue (1): drm/i915/lmem: Enable lmem for platforms with Flat CCS Anshuman Gupta (1): drm/i915/dg2: Flat CCS Support Ayaz A Siddiqui (1): drm/i915/gt: Clear compress metadata for Xe_HP platforms CQ Tang (1): drm/i915/xehpsdv: Add has_flat_ccs to device info Matt Roper (1): drm/i915/dg2: Add DG2 unified compression Matthew Auld (6): drm/i915: enforce min GTT alignment for discrete cards drm/i915: support 64K GTT pages for discrete cards drm/i915/gtt: allow overriding the p
[PATCH v5 01/19] drm/i915: add needs_compact_pt flag
Add a new platform flag, needs_compact_pt, to mark the requirement of compact pt layout support for the ppGTT when using 64K GTT pages. With this flag has_64k_pages will only indicate requirement of 64K GTT page sizes or larger for device local memory access. v6: * minor doc formatting Suggested-by: Matthew Auld Signed-off-by: Ramalingam C Signed-off-by: Robert Beckett Reviewed-by: Thomas Hellström --- drivers/gpu/drm/i915/i915_drv.h | 11 --- drivers/gpu/drm/i915/i915_pci.c | 2 ++ drivers/gpu/drm/i915/intel_device_info.h | 1 + 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 00e7594b59c9..4afdfa5fd3b3 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1512,12 +1512,17 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, /* * Set this flag, when platform requires 64K GTT page sizes or larger for - * device local memory access. Also this flag implies that we require or - * at least support the compact PT layout for the ppGTT when using the 64K - * GTT pages. + * device local memory access. */ #define HAS_64K_PAGES(dev_priv) (INTEL_INFO(dev_priv)->has_64k_pages) +/* + * Set this flag when platform doesn't allow both 64k pages and 4k pages in + * the same PT. this flag means we need to support compact PT layout for the + * ppGTT when using the 64K GTT pages. + */ +#define NEEDS_COMPACT_PT(dev_priv) (INTEL_INFO(dev_priv)->needs_compact_pt) + #define HAS_IPC(dev_priv) (INTEL_INFO(dev_priv)->display.has_ipc) #define HAS_REGION(i915, i) (INTEL_INFO(i915)->memory_regions & (i)) diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 2df2db0a5d70..ce6ae6a3cbdf 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -1028,6 +1028,7 @@ static const struct intel_device_info xehpsdv_info = { PLATFORM(INTEL_XEHPSDV), .display = { }, .has_64k_pages = 1, + .needs_compact_pt = 1, .platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VECS1) | BIT(VECS2) | BIT(VECS3) | @@ -1046,6 +1047,7 @@ static const struct intel_device_info dg2_info = { PLATFORM(INTEL_DG2), .has_guc_deprivilege = 1, .has_64k_pages = 1, + .needs_compact_pt = 1, .platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VECS1) | diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index abf1e103c558..d8da40d01dca 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -130,6 +130,7 @@ enum intel_ppgtt_type { /* Keep has_* in alphabetical order */ \ func(has_64bit_reloc); \ func(has_64k_pages); \ + func(needs_compact_pt); \ func(gpu_reset_clobbers_display); \ func(has_reset_engine); \ func(has_global_mocs); \ -- 2.20.1
[PATCH v5 03/19] drm/i915: support 64K GTT pages for discrete cards
From: Matthew Auld discrete cards optimise 64K GTT pages for local-memory, since everything should be allocated at 64K granularity. We say goodbye to sparse entries, and instead get a compact 256B page-table for 64K pages, which should be more cache friendly. 4K pages for local-memory are no longer supported by the HW. v4: don't return uninitialized err in igt_ppgtt_compact Reported-by: kernel test robot Signed-off-by: Matthew Auld Signed-off-by: Stuart Summers Signed-off-by: Ramalingam C Signed-off-by: Robert Beckett Reviewed-by: Thomas Hellström Cc: Joonas Lahtinen Cc: Rodrigo Vivi --- .../gpu/drm/i915/gem/selftests/huge_pages.c | 60 ++ drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 108 +- drivers/gpu/drm/i915/gt/intel_gtt.h | 3 + drivers/gpu/drm/i915/gt/intel_ppgtt.c | 1 + 4 files changed, 169 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c index f36191ebf964..a7d9bdb85d70 100644 --- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c +++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c @@ -1478,6 +1478,65 @@ static int igt_ppgtt_sanity_check(void *arg) return err; } +static int igt_ppgtt_compact(void *arg) +{ + struct drm_i915_private *i915 = arg; + struct drm_i915_gem_object *obj; + int err; + + /* +* Simple test to catch issues with compact 64K pages -- since the pt is +* compacted to 256B that gives us 32 entries per pt, however since the +* backing page for the pt is 4K, any extra entries we might incorrectly +* write out should be ignored by the HW. If ever hit such a case this +* test should catch it since some of our writes would land in scratch. +*/ + + if (!HAS_64K_PAGES(i915)) { + pr_info("device lacks compact 64K page support, skipping\n"); + return 0; + } + + if (!HAS_LMEM(i915)) { + pr_info("device lacks LMEM support, skipping\n"); + return 0; + } + + /* We want the range to cover multiple page-table boundaries. */ + obj = i915_gem_object_create_lmem(i915, SZ_4M, 0); + if (IS_ERR(obj)) + return PTR_ERR(obj); + + err = i915_gem_object_pin_pages_unlocked(obj); + if (err) + goto out_put; + + if (obj->mm.page_sizes.phys < I915_GTT_PAGE_SIZE_64K) { + pr_info("LMEM compact unable to allocate huge-page(s)\n"); + goto out_unpin; + } + + /* +* Disable 2M GTT pages by forcing the page-size to 64K for the GTT +* insertion. +*/ + obj->mm.page_sizes.sg = I915_GTT_PAGE_SIZE_64K; + + err = igt_write_huge(i915, obj); + if (err) + pr_err("LMEM compact write-huge failed\n"); + +out_unpin: + i915_gem_object_unpin_pages(obj); +out_put: + i915_gem_object_put(obj); + + if (err == -ENOMEM) + err = 0; + + return err; +} + static int igt_tmpfs_fallback(void *arg) { struct drm_i915_private *i915 = arg; @@ -1735,6 +1794,7 @@ int i915_gem_huge_page_live_selftests(struct drm_i915_private *i915) SUBTEST(igt_tmpfs_fallback), SUBTEST(igt_ppgtt_smoke_huge), SUBTEST(igt_ppgtt_sanity_check), + SUBTEST(igt_ppgtt_compact), }; if (!HAS_PPGTT(i915)) { diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c index c43e724afa9f..62471730266c 100644 --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c @@ -233,6 +233,8 @@ static u64 __gen8_ppgtt_clear(struct i915_address_space * const vm, start, end, lvl); } else { unsigned int count; + unsigned int pte = gen8_pd_index(start, 0); + unsigned int num_ptes; u64 *vaddr; count = gen8_pt_count(start, end); @@ -242,10 +244,18 @@ static u64 __gen8_ppgtt_clear(struct i915_address_space * const vm, atomic_read(&pt->used)); GEM_BUG_ON(!count || count >= atomic_read(&pt->used)); + num_ptes = count; + if (pt->is_compact) { + GEM_BUG_ON(num_ptes % 16); + GEM_BUG_ON(pte % 16); + num_ptes /= 16; + pte /= 16; + } + vaddr = px_vaddr(pt); - memset64(vaddr + gen8_pd_index(start, 0), + memset64(vaddr + pte, vm->scratch[0]->encode, -count); +num_ptes);
[PATCH v5 02/19] drm/i915: enforce min GTT alignment for discrete cards
From: Matthew Auld For local-memory objects we need to align the GTT addresses to 64K, both for the ppgtt and ggtt. We need to support vm->min_alignment > 4K, depending on the vm itself and the type of object we are inserting. With this in mind update the GTT selftests to take this into account. For compact-pt we further align and pad lmem object GTT addresses to 2MB to ensure PDEs contain consistent page sizes as required by the HW. v3: * use needs_compact_pt flag to discriminate between 64K and 64K with compact-pt * add i915_vm_obj_min_alignment * use i915_vm_obj_min_alignment to round up vma reservation if compact-pt instead of hard coding v5: * fix i915_vm_obj_min_alignment for internal objects which have no memory region v6: * tiled_blits_create correctly pick largest required alignment Signed-off-by: Matthew Auld Signed-off-by: Ramalingam C Signed-off-by: Robert Beckett Reviewed-by: Thomas Hellström Cc: Joonas Lahtinen Cc: Rodrigo Vivi --- .../i915/gem/selftests/i915_gem_client_blt.c | 21 ++-- drivers/gpu/drm/i915/gt/intel_gtt.c | 12 +++ drivers/gpu/drm/i915/gt/intel_gtt.h | 18 drivers/gpu/drm/i915/i915_vma.c | 9 ++ drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 96 --- 5 files changed, 115 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c index c8ff8bf0986d..3675d12a7d9a 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c @@ -39,6 +39,7 @@ struct tiled_blits { struct blit_buffer scratch; struct i915_vma *batch; u64 hole; + u64 align; u32 width; u32 height; }; @@ -410,14 +411,19 @@ tiled_blits_create(struct intel_engine_cs *engine, struct rnd_state *prng) goto err_free; } - hole_size = 2 * PAGE_ALIGN(WIDTH * HEIGHT * 4); + t->align = i915_vm_min_alignment(t->ce->vm, INTEL_MEMORY_LOCAL); + t->align = max(t->align, + i915_vm_min_alignment(t->ce->vm, INTEL_MEMORY_SYSTEM)); + + hole_size = 2 * round_up(WIDTH * HEIGHT * 4, t->align); hole_size *= 2; /* room to maneuver */ - hole_size += 2 * I915_GTT_MIN_ALIGNMENT; + hole_size += 2 * t->align; /* padding on either side */ mutex_lock(&t->ce->vm->mutex); memset(&hole, 0, sizeof(hole)); err = drm_mm_insert_node_in_range(&t->ce->vm->mm, &hole, - hole_size, 0, I915_COLOR_UNEVICTABLE, + hole_size, t->align, + I915_COLOR_UNEVICTABLE, 0, U64_MAX, DRM_MM_INSERT_BEST); if (!err) @@ -428,7 +434,7 @@ tiled_blits_create(struct intel_engine_cs *engine, struct rnd_state *prng) goto err_put; } - t->hole = hole.start + I915_GTT_MIN_ALIGNMENT; + t->hole = hole.start + t->align; pr_info("Using hole at %llx\n", t->hole); err = tiled_blits_create_buffers(t, WIDTH, HEIGHT, prng); @@ -455,7 +461,7 @@ static void tiled_blits_destroy(struct tiled_blits *t) static int tiled_blits_prepare(struct tiled_blits *t, struct rnd_state *prng) { - u64 offset = PAGE_ALIGN(t->width * t->height * 4); + u64 offset = round_up(t->width * t->height * 4, t->align); u32 *map; int err; int i; @@ -486,8 +492,7 @@ static int tiled_blits_prepare(struct tiled_blits *t, static int tiled_blits_bounce(struct tiled_blits *t, struct rnd_state *prng) { - u64 offset = - round_up(t->width * t->height * 4, 2 * I915_GTT_MIN_ALIGNMENT); + u64 offset = round_up(t->width * t->height * 4, 2 * t->align); int err; /* We want to check position invariant tiling across GTT eviction */ @@ -500,7 +505,7 @@ static int tiled_blits_bounce(struct tiled_blits *t, struct rnd_state *prng) /* Reposition so that we overlap the old addresses, and slightly off */ err = tiled_blit(t, -&t->buffers[2], t->hole + I915_GTT_MIN_ALIGNMENT, +&t->buffers[2], t->hole + t->align, &t->buffers[1], t->hole + 3 * offset / 2); if (err) return err; diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c index 46be4197b93f..df23ebdfc994 100644 --- a/drivers/gpu/drm/i915/gt/intel_gtt.c +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c @@ -223,6 +223,18 @@ void i915_address_space_init(struct i915_address_space *vm, int subclass) GEM_BUG_ON(!vm->total); drm_mm_init(&vm->mm, 0, vm->total); + + memset64(vm->min_alignment, I91
[PATCH v5 05/19] drm/i915/gtt: allow overriding the pt alignment
From: Matthew Auld On some platforms we have alignment restrictions when accessing LMEM from the GTT. In the next patch few patches we need to be able to modify the page-tables directly via the GTT itself. Suggested-by: Ramalingam C Signed-off-by: Matthew Auld Cc: Thomas Hellström Cc: Ramalingam C --- drivers/gpu/drm/i915/gt/intel_gtt.h | 10 +- drivers/gpu/drm/i915/gt/intel_ppgtt.c | 16 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h index e6ce0be6d484..2c62411eb52c 100644 --- a/drivers/gpu/drm/i915/gt/intel_gtt.h +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h @@ -200,6 +200,14 @@ void *__px_vaddr(struct drm_i915_gem_object *p); struct i915_vm_pt_stash { /* preallocated chains of page tables/directories */ struct i915_page_table *pt[2]; + /* +* Optionally override the alignment/size of the physical page that +* contains each PT. If not set defaults back to the usual +* I915_GTT_PAGE_SIZE_4K. This does not influence the other paging +* structures. MUST be a power-of-two. ONLY applicable on discrete +* platforms. +*/ + int pt_sz; }; struct i915_vma_ops { @@ -591,7 +599,7 @@ void free_scratch(struct i915_address_space *vm); struct drm_i915_gem_object *alloc_pt_dma(struct i915_address_space *vm, int sz); struct drm_i915_gem_object *alloc_pt_lmem(struct i915_address_space *vm, int sz); -struct i915_page_table *alloc_pt(struct i915_address_space *vm); +struct i915_page_table *alloc_pt(struct i915_address_space *vm, int sz); struct i915_page_directory *alloc_pd(struct i915_address_space *vm); struct i915_page_directory *__alloc_pd(int npde); diff --git a/drivers/gpu/drm/i915/gt/intel_ppgtt.c b/drivers/gpu/drm/i915/gt/intel_ppgtt.c index 043652dc6892..d91e2beb7517 100644 --- a/drivers/gpu/drm/i915/gt/intel_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ppgtt.c @@ -12,7 +12,7 @@ #include "gen6_ppgtt.h" #include "gen8_ppgtt.h" -struct i915_page_table *alloc_pt(struct i915_address_space *vm) +struct i915_page_table *alloc_pt(struct i915_address_space *vm, int sz) { struct i915_page_table *pt; @@ -20,7 +20,7 @@ struct i915_page_table *alloc_pt(struct i915_address_space *vm) if (unlikely(!pt)) return ERR_PTR(-ENOMEM); - pt->base = vm->alloc_pt_dma(vm, I915_GTT_PAGE_SIZE_4K); + pt->base = vm->alloc_pt_dma(vm, sz); if (IS_ERR(pt->base)) { kfree(pt); return ERR_PTR(-ENOMEM); @@ -221,17 +221,25 @@ int i915_vm_alloc_pt_stash(struct i915_address_space *vm, u64 size) { unsigned long count; - int shift, n; + int shift, n, pt_sz; shift = vm->pd_shift; if (!shift) return 0; + pt_sz = stash->pt_sz; + if (!pt_sz) + pt_sz = I915_GTT_PAGE_SIZE_4K; + else + GEM_BUG_ON(!IS_DGFX(vm->i915)); + + GEM_BUG_ON(!is_power_of_2(pt_sz)); + count = pd_count(size, shift); while (count--) { struct i915_page_table *pt; - pt = alloc_pt(vm); + pt = alloc_pt(vm, pt_sz); if (IS_ERR(pt)) { i915_vm_free_pt_stash(vm, stash); return PTR_ERR(pt); -- 2.20.1
[PATCH v5 04/19] drm/i915: add gtt misalignment test
From: Robert Beckett add test to check handling of misaligned offsets and sizes v4: * remove spurious blank lines * explicitly cast intel_region_id to intel_memory_type in misaligned_pin Reported-by: kernel test robot v6: * use NEEDS_COMPACT_PT instead of hard coding for DG2 Signed-off-by: Robert Beckett Reviewed-by: Thomas Hellström --- drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 128 ++ 1 file changed, 128 insertions(+) diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c index b80788a2b7f9..c23b1e5cc436 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c @@ -22,10 +22,12 @@ * */ +#include "gt/intel_gtt.h" #include #include #include "gem/i915_gem_context.h" +#include "gem/i915_gem_region.h" #include "gem/selftests/mock_context.h" #include "gt/intel_context.h" #include "gt/intel_gpu_commands.h" @@ -1067,6 +1069,120 @@ static int shrink_boom(struct i915_address_space *vm, return err; } +static int misaligned_case(struct i915_address_space *vm, struct intel_memory_region *mr, + u64 addr, u64 size, unsigned long flags) +{ + struct drm_i915_gem_object *obj; + struct i915_vma *vma; + int err = 0; + u64 expected_vma_size, expected_node_size; + + obj = i915_gem_object_create_region(mr, size, 0, 0); + if (IS_ERR(obj)) + return PTR_ERR(obj); + + vma = i915_vma_instance(obj, vm, NULL); + if (IS_ERR(vma)) { + err = PTR_ERR(vma); + goto err_put; + } + + err = i915_vma_pin(vma, 0, 0, addr | flags); + if (err) + goto err_put; + i915_vma_unpin(vma); + + if (!drm_mm_node_allocated(&vma->node)) { + err = -EINVAL; + goto err_put; + } + + if (i915_vma_misplaced(vma, 0, 0, addr | flags)) { + err = -EINVAL; + goto err_put; + } + + expected_vma_size = round_up(size, 1 << (ffs(vma->resource->page_sizes_gtt) - 1)); + expected_node_size = expected_vma_size; + + if (NEEDS_COMPACT_PT(vm->i915) && i915_gem_object_is_lmem(obj)) { + /* compact-pt should expand lmem node to 2MB */ + expected_vma_size = round_up(size, I915_GTT_PAGE_SIZE_64K); + expected_node_size = round_up(size, I915_GTT_PAGE_SIZE_2M); + } + + if (vma->size != expected_vma_size || vma->node.size != expected_node_size) { + err = i915_vma_unbind(vma); + err = -EBADSLT; + goto err_put; + } + + err = i915_vma_unbind(vma); + if (err) + goto err_put; + + GEM_BUG_ON(drm_mm_node_allocated(&vma->node)); + +err_put: + i915_gem_object_put(obj); + cleanup_freed_objects(vm->i915); + return err; +} + +static int misaligned_pin(struct i915_address_space *vm, + u64 hole_start, u64 hole_end, + unsigned long end_time) +{ + struct intel_memory_region *mr; + enum intel_region_id id; + unsigned long flags = PIN_OFFSET_FIXED | PIN_USER; + int err = 0; + u64 hole_size = hole_end - hole_start; + + if (i915_is_ggtt(vm)) + flags |= PIN_GLOBAL; + + for_each_memory_region(mr, vm->i915, id) { + u64 min_alignment = i915_vm_min_alignment(vm, (enum intel_memory_type)id); + u64 size = min_alignment; + u64 addr = round_up(hole_start + (hole_size / 2), min_alignment); + + /* we can't test < 4k alignment due to flags being encoded in lower bits */ + if (min_alignment != I915_GTT_PAGE_SIZE_4K) { + err = misaligned_case(vm, mr, addr + (min_alignment / 2), size, flags); + /* misaligned should error with -EINVAL*/ + if (!err) + err = -EBADSLT; + if (err != -EINVAL) + return err; + } + + /* test for vma->size expansion to min page size */ + err = misaligned_case(vm, mr, addr, PAGE_SIZE, flags); + if (min_alignment > hole_size) { + if (!err) + err = -EBADSLT; + else if (err == -ENOSPC) + err = 0; + } + if (err) + return err; + + /* test for intermediate size not expanding vma->size for large alignments */ + err = misaligned_case(vm, mr, addr, size / 2, flags); + if (min_alignment > hole_size) { + if (!err) + err = -EBADSLT; + else if (err == -ENOSPC) +
[PATCH v5 06/19] drm/i915/gtt: add xehpsdv_ppgtt_insert_entry
From: Matthew Auld If this is LMEM then we get a 32 entry PT, with each PTE pointing to some 64K block of memory, otherwise it's just the usual 512 entry PT. This very much assumes the caller knows what they are doing. Signed-off-by: Matthew Auld Cc: Thomas Hellström Cc: Ramalingam C Reviewed-by: Ramalingam C --- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 50 ++-- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c index 62471730266c..f574da00eff1 100644 --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c @@ -715,13 +715,56 @@ static void gen8_ppgtt_insert_entry(struct i915_address_space *vm, gen8_pdp_for_page_index(vm, idx); struct i915_page_directory *pd = i915_pd_entry(pdp, gen8_pd_index(idx, 2)); + struct i915_page_table *pt = i915_pt_entry(pd, gen8_pd_index(idx, 1)); gen8_pte_t *vaddr; - vaddr = px_vaddr(i915_pt_entry(pd, gen8_pd_index(idx, 1))); + GEM_BUG_ON(pt->is_compact); + + vaddr = px_vaddr(pt); vaddr[gen8_pd_index(idx, 0)] = gen8_pte_encode(addr, level, flags); clflush_cache_range(&vaddr[gen8_pd_index(idx, 0)], sizeof(*vaddr)); } +static void __xehpsdv_ppgtt_insert_entry_lm(struct i915_address_space *vm, + dma_addr_t addr, + u64 offset, + enum i915_cache_level level, + u32 flags) +{ + u64 idx = offset >> GEN8_PTE_SHIFT; + struct i915_page_directory * const pdp = + gen8_pdp_for_page_index(vm, idx); + struct i915_page_directory *pd = + i915_pd_entry(pdp, gen8_pd_index(idx, 2)); + struct i915_page_table *pt = i915_pt_entry(pd, gen8_pd_index(idx, 1)); + gen8_pte_t *vaddr; + + GEM_BUG_ON(!IS_ALIGNED(addr, SZ_64K)); + GEM_BUG_ON(!IS_ALIGNED(offset, SZ_64K)); + + if (!pt->is_compact) { + vaddr = px_vaddr(pd); + vaddr[gen8_pd_index(idx, 1)] |= GEN12_PDE_64K; + pt->is_compact = true; + } + + vaddr = px_vaddr(pt); + vaddr[gen8_pd_index(idx, 0) / 16] = gen8_pte_encode(addr, level, flags); +} + +static void xehpsdv_ppgtt_insert_entry(struct i915_address_space *vm, + dma_addr_t addr, + u64 offset, + enum i915_cache_level level, + u32 flags) +{ + if (flags & PTE_LM) + return __xehpsdv_ppgtt_insert_entry_lm(vm, addr, offset, + level, flags); + + return gen8_ppgtt_insert_entry(vm, addr, offset, level, flags); +} + static int gen8_init_scratch(struct i915_address_space *vm) { u32 pte_flags; @@ -921,7 +964,10 @@ struct i915_ppgtt *gen8_ppgtt_create(struct intel_gt *gt, ppgtt->vm.bind_async_flags = I915_VMA_LOCAL_BIND; ppgtt->vm.insert_entries = gen8_ppgtt_insert; - ppgtt->vm.insert_page = gen8_ppgtt_insert_entry; + if (HAS_64K_PAGES(gt->i915)) + ppgtt->vm.insert_page = xehpsdv_ppgtt_insert_entry; + else + ppgtt->vm.insert_page = gen8_ppgtt_insert_entry; ppgtt->vm.allocate_va_range = gen8_ppgtt_alloc; ppgtt->vm.clear_range = gen8_ppgtt_clear; ppgtt->vm.foreach = gen8_ppgtt_foreach; -- 2.20.1
[PATCH v5 07/19] drm/i915/migrate: add acceleration support for DG2
From: Matthew Auld This is all kinds of awkward since we now have to contend with using 64K GTT pages when mapping anything in LMEM(including the page-tables themselves). v2: Rebased [Ram] Signed-off-by: Matthew Auld Cc: Thomas Hellström Cc: Ramalingam C --- drivers/gpu/drm/i915/gt/intel_migrate.c | 179 +++- 1 file changed, 147 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_migrate.c b/drivers/gpu/drm/i915/gt/intel_migrate.c index 18b44af56969..cac791155244 100644 --- a/drivers/gpu/drm/i915/gt/intel_migrate.c +++ b/drivers/gpu/drm/i915/gt/intel_migrate.c @@ -32,6 +32,38 @@ static bool engine_supports_migration(struct intel_engine_cs *engine) return true; } +static void xehpsdv_toggle_pdes(struct i915_address_space *vm, + struct i915_page_table *pt, + void *data) +{ + struct insert_pte_data *d = data; + + /* +* Insert a dummy PTE into every PT that will map to LMEM to ensure +* we have a correctly setup PDE structure for later use. +*/ + vm->insert_page(vm, 0, d->offset, I915_CACHE_NONE, PTE_LM); + GEM_BUG_ON(!pt->is_compact); + d->offset += SZ_2M; +} + +static void xehpsdv_insert_pte(struct i915_address_space *vm, + struct i915_page_table *pt, + void *data) +{ + struct insert_pte_data *d = data; + + /* +* We are playing tricks here, since the actual pt, from the hw +* pov, is only 256bytes with 32 entries, or 4096bytes with 512 +* entries, but we are still guaranteed that the physical +* alignment is 64K underneath for the pt, and we are careful +* not to access the space in the void. +*/ + vm->insert_page(vm, px_dma(pt), d->offset, I915_CACHE_NONE, PTE_LM); + d->offset += SZ_64K; +} + static void insert_pte(struct i915_address_space *vm, struct i915_page_table *pt, void *data) @@ -74,7 +106,12 @@ static struct i915_address_space *migrate_vm(struct intel_gt *gt) * i.e. within the same non-preemptible window so that we do not switch * to another migration context that overwrites the PTE. * -* TODO: Add support for huge LMEM PTEs +* On platforms with HAS_64K_PAGES support we have three windows, and +* dedicate two windows just for mapping lmem pages(smem <-> smem is not +* a thing), since we are forced to use 64K GTT pages underneath which +* requires also modifying the PDE. An alternative might be to instead +* map the PD into the GTT, and then on the fly toggle the 4K/64K mode +* in the PDE from the same batch that also modifies the PTEs. */ vm = i915_ppgtt_create(gt, I915_BO_ALLOC_PM_EARLY); @@ -86,6 +123,9 @@ static struct i915_address_space *migrate_vm(struct intel_gt *gt) goto err_vm; } + if (HAS_64K_PAGES(gt->i915)) + stash.pt_sz = I915_GTT_PAGE_SIZE_64K; + /* * Each engine instance is assigned its own chunk in the VM, so * that we can run multiple instances concurrently @@ -105,14 +145,20 @@ static struct i915_address_space *migrate_vm(struct intel_gt *gt) * We copy in 8MiB chunks. Each PDE covers 2MiB, so we need * 4x2 page directories for source/destination. */ - sz = 2 * CHUNK_SZ; + if (HAS_64K_PAGES(gt->i915)) + sz = 3 * CHUNK_SZ; + else + sz = 2 * CHUNK_SZ; d.offset = base + sz; /* * We need another page directory setup so that we can write * the 8x512 PTE in each chunk. */ - sz += (sz >> 12) * sizeof(u64); + if (HAS_64K_PAGES(gt->i915)) + sz += (sz / SZ_2M) * SZ_64K; + else + sz += (sz >> 12) * sizeof(u64); err = i915_vm_alloc_pt_stash(&vm->vm, &stash, sz); if (err) @@ -133,7 +179,18 @@ static struct i915_address_space *migrate_vm(struct intel_gt *gt) goto err_vm; /* Now allow the GPU to rewrite the PTE via its own ppGTT */ - vm->vm.foreach(&vm->vm, base, d.offset - base, insert_pte, &d); + if (HAS_64K_PAGES(gt->i915)) { + vm->vm.foreach(&vm->vm, base, d.offset - base, + xehpsdv_insert_pte, &d); + d.offset = base + CHUNK_SZ; + vm->vm.foreach(&vm->vm, + d.offset, + 2 * CHUNK_SZ, + xehpsdv_toggle_pdes, &d); + } else { +
[PATCH v5 08/19] drm/i915/uapi: document behaviour for DG2 64K support
From: Matthew Auld On discrete platforms like DG2, we need to support a minimum page size of 64K when dealing with device local-memory. This is quite tricky for various reasons, so try to document the new implicit uapi for this. v3: fix typos and less emphasis v2: Fixed suggestions on formatting [Daniel] Signed-off-by: Matthew Auld Signed-off-by: Ramalingam C Signed-off-by: Robert Beckett Acked-by: Jordan Justen Reviewed-by: Ramalingam C Reviewed-by: Thomas Hellström cc: Simon Ser cc: Pekka Paalanen Cc: Jordan Justen Cc: Kenneth Graunke Cc: mesa-...@lists.freedesktop.org Cc: Tony Ye Cc: Slawomir Milczarek --- include/uapi/drm/i915_drm.h | 44 - 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 5e678917da70..77e5e74c32c1 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -1118,10 +1118,16 @@ struct drm_i915_gem_exec_object2 { /** * When the EXEC_OBJECT_PINNED flag is specified this is populated by * the user with the GTT offset at which this object will be pinned. +* * When the I915_EXEC_NO_RELOC flag is specified this must contain the * presumed_offset of the object. +* * During execbuffer2 the kernel populates it with the value of the * current GTT offset of the object, for future presumed_offset writes. +* +* See struct drm_i915_gem_create_ext for the rules when dealing with +* alignment restrictions with I915_MEMORY_CLASS_DEVICE, on devices with +* minimum page sizes, like DG2. */ __u64 offset; @@ -3145,11 +3151,39 @@ struct drm_i915_gem_create_ext { * * The (page-aligned) allocated size for the object will be returned. * -* Note that for some devices we have might have further minimum -* page-size restrictions(larger than 4K), like for device local-memory. -* However in general the final size here should always reflect any -* rounding up, if for example using the I915_GEM_CREATE_EXT_MEMORY_REGIONS -* extension to place the object in device local-memory. +* +* DG2 64K min page size implications: +* +* On discrete platforms, starting from DG2, we have to contend with GTT +* page size restrictions when dealing with I915_MEMORY_CLASS_DEVICE +* objects. Specifically the hardware only supports 64K or larger GTT +* page sizes for such memory. The kernel will already ensure that all +* I915_MEMORY_CLASS_DEVICE memory is allocated using 64K or larger page +* sizes underneath. +* +* Note that the returned size here will always reflect any required +* rounding up done by the kernel, i.e 4K will now become 64K on devices +* such as DG2. +* +* Special DG2 GTT address alignment requirement: +* +* The GTT alignment will also need to be at least 2M for such objects. +* +* Note that due to how the hardware implements 64K GTT page support, we +* have some further complications: +* +* 1) The entire PDE (which covers a 2MB virtual address range), must +* contain only 64K PTEs, i.e mixing 4K and 64K PTEs in the same +* PDE is forbidden by the hardware. +* +* 2) We still need to support 4K PTEs for I915_MEMORY_CLASS_SYSTEM +* objects. +* +* To keep things simple for userland, we mandate that any GTT mappings +* must be aligned to and rounded up to 2MB. As this only wastes virtual +* address space and avoids userland having to copy any needlessly +* complicated PDE sharing scheme (coloring) and only affects DG2, this +* is deemed to be a good compromise. */ __u64 size; /** -- 2.20.1
[PATCH v5 09/19] Doc/gpu/rfc/i915: i915 DG2 64k pagesize uAPI
Details of the 64k pagesize support added as part of DG2 enabling and its implicit impact on the uAPI. v2: improvised the Flat-CCS documentation [Danvet & CQ] v3: made only for 64k pagesize support Signed-off-by: Ramalingam C cc: Daniel Vetter cc: Matthew Auld cc: Simon Ser cc: Pekka Paalanen Cc: Jordan Justen Cc: Kenneth Graunke Cc: mesa-...@lists.freedesktop.org Cc: Tony Ye Cc: Slawomir Milczarek --- Documentation/gpu/rfc/i915_dg2.rst | 25 + Documentation/gpu/rfc/index.rst| 3 +++ 2 files changed, 28 insertions(+) create mode 100644 Documentation/gpu/rfc/i915_dg2.rst diff --git a/Documentation/gpu/rfc/i915_dg2.rst b/Documentation/gpu/rfc/i915_dg2.rst new file mode 100644 index ..f4eb5a219897 --- /dev/null +++ b/Documentation/gpu/rfc/i915_dg2.rst @@ -0,0 +1,25 @@ + +I915 DG2 RFC Section + + +Upstream plan += +Plan to upstream the DG2 enabling is: + +* Merge basic HW enabling for DG2 (Still without pciid) +* Merge the 64k support for lmem +* Merge the flat CCS enabling patches +* Add the pciid for DG2 and enable the DG2 in CI + + +64K page support for lmem += +On DG2 hw, local-memory supports minimum GTT page size of 64k only. 4k is not +supported anymore. + +DG2 hw doesn't support the 64k (lmem) and 4k (smem) pages in the same ppgtt +Page table. Refer the struct drm_i915_gem_create_ext for the implication of +handling the 64k page size. + +.. kernel-doc:: include/uapi/drm/i915_drm.h +:functions: drm_i915_gem_create_ext diff --git a/Documentation/gpu/rfc/index.rst b/Documentation/gpu/rfc/index.rst index 91e93a705230..afb320ed4028 100644 --- a/Documentation/gpu/rfc/index.rst +++ b/Documentation/gpu/rfc/index.rst @@ -20,6 +20,9 @@ host such documentation: i915_gem_lmem.rst +.. toctree:: +i915_dg2.rst + .. toctree:: i915_scheduler.rst -- 2.20.1
[PATCH v5 10/19] drm/i915/xehpsdv: Add has_flat_ccs to device info
From: CQ Tang Platforms of XeHP and beyond support 3D surface (buffer) compression and various compression formats. This is accomplished by an additional compression control state (CCS) stored for each surface. Gen 12 devices(TGL family and DG1) stores compression states in a separate region of memory. It is managed by user-space and has an associated set of user-space managed page tables used by hardware for address translation. In Xe HP and beyond (XEHPSDV, DG2, etc), there is a new feature introduced i.e Flat CCS. It replaced AUX page tables with a flat indexed region of device memory for storing compression states. Cc: Joonas Lahtinen Cc: Matthew Auld Signed-off-by: CQ Tang Signed-off-by: Ramalingam C Reviewed-by: Matthew Auld --- drivers/gpu/drm/i915/i915_drv.h | 6 ++ drivers/gpu/drm/i915/i915_pci.c | 1 + drivers/gpu/drm/i915/intel_device_info.h | 1 + 3 files changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 4afdfa5fd3b3..384977798c8e 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1528,6 +1528,12 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, #define HAS_REGION(i915, i) (INTEL_INFO(i915)->memory_regions & (i)) #define HAS_LMEM(i915) HAS_REGION(i915, REGION_LMEM) +/* + * Platform has the dedicated compression control state for each lmem surfaces + * stored in lmem to support the 3D and media compression formats. + */ +#define HAS_FLAT_CCS(dev_priv) (INTEL_INFO(dev_priv)->has_flat_ccs) + #define HAS_GT_UC(dev_priv)(INTEL_INFO(dev_priv)->has_gt_uc) #define HAS_POOLED_EU(dev_priv)(INTEL_INFO(dev_priv)->has_pooled_eu) diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index ce6ae6a3cbdf..3976482582b8 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -1003,6 +1003,7 @@ static const struct intel_device_info adl_p_info = { XE_HP_PAGE_SIZES, \ .dma_mask_size = 46, \ .has_64bit_reloc = 1, \ + .has_flat_ccs = 1, \ .has_global_mocs = 1, \ .has_gt_uc = 1, \ .has_llc = 1, \ diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index d8da40d01dca..ef7c7c988b7b 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -133,6 +133,7 @@ enum intel_ppgtt_type { func(needs_compact_pt); \ func(gpu_reset_clobbers_display); \ func(has_reset_engine); \ + func(has_flat_ccs); \ func(has_global_mocs); \ func(has_gt_uc); \ func(has_guc_deprivilege); \ -- 2.20.1
[PATCH v5 11/19] drm/i915/lmem: Enable lmem for platforms with Flat CCS
From: Abdiel Janulgue A portion of device memory is reserved for Flat CCS so usable device memory will be reduced by size of Flat CCS. Size of Flat CCS is specified in “XEHPSDV_FLAT_CCS_BASE_ADDR”. So to get effective device memory we need to subtract total device memory by Flat CCS memory size. v2: Addressed the small bar related issue [Matt] Removed a reduntant check [Matt] Cc: Matthew Auld Signed-off-by: Abdiel Janulgue Signed-off-by: Ramalingam C --- drivers/gpu/drm/i915/gt/intel_gt.c | 19 drivers/gpu/drm/i915/gt/intel_gt.h | 1 + drivers/gpu/drm/i915/gt/intel_region_lmem.c | 24 +++-- drivers/gpu/drm/i915/i915_reg.h | 3 +++ 4 files changed, 45 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index f59933abbb3a..e40d98cb3a2d 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -911,6 +911,25 @@ u32 intel_gt_read_register_fw(struct intel_gt *gt, i915_reg_t reg) return intel_uncore_read_fw(gt->uncore, reg); } +u32 intel_gt_read_register(struct intel_gt *gt, i915_reg_t reg) +{ + int type; + u8 sliceid, subsliceid; + + for (type = 0; type < NUM_STEERING_TYPES; type++) { + if (intel_gt_reg_needs_read_steering(gt, reg, type)) { + intel_gt_get_valid_steering(gt, type, &sliceid, + &subsliceid); + return intel_uncore_read_with_mcr_steering(gt->uncore, + reg, + sliceid, + subsliceid); + } + } + + return intel_uncore_read(gt->uncore, reg); +} + void intel_gt_info_print(const struct intel_gt_info *info, struct drm_printer *p) { diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h index 2dad46c3eff2..0f571c8ee22b 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.h +++ b/drivers/gpu/drm/i915/gt/intel_gt.h @@ -85,6 +85,7 @@ static inline bool intel_gt_needs_read_steering(struct intel_gt *gt, } u32 intel_gt_read_register_fw(struct intel_gt *gt, i915_reg_t reg); +u32 intel_gt_read_register(struct intel_gt *gt, i915_reg_t reg); void intel_gt_info_print(const struct intel_gt_info *info, struct drm_printer *p); diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c b/drivers/gpu/drm/i915/gt/intel_region_lmem.c index 21215a080088..f1d37b46b505 100644 --- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c +++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c @@ -205,8 +205,28 @@ static struct intel_memory_region *setup_lmem(struct intel_gt *gt) if (!IS_DGFX(i915)) return ERR_PTR(-ENODEV); - /* Stolen starts from GSMBASE on DG1 */ - lmem_size = intel_uncore_read64(uncore, GEN12_GSMBASE); + if (HAS_FLAT_CCS(i915)) { + u64 tile_stolen, flat_ccs_base_addr_reg, flat_ccs_base; + + lmem_size = pci_resource_len(pdev, 2); + flat_ccs_base_addr_reg = intel_gt_read_register(gt, XEHPSDV_FLAT_CCS_BASE_ADDR); + flat_ccs_base = (flat_ccs_base_addr_reg >> XEHPSDV_CCS_BASE_SHIFT) * SZ_64K; + + if (GEM_WARN_ON(lmem_size < flat_ccs_base)) + return ERR_PTR(-ENODEV); + + tile_stolen = lmem_size - flat_ccs_base; + + /* If the FLAT_CCS_BASE_ADDR register is not populated, flag an error */ + if (tile_stolen == lmem_size) + DRM_ERROR("CCS_BASE_ADDR register did not have expected value\n"); + + lmem_size -= tile_stolen; + } else { + /* Stolen starts from GSMBASE without CCS */ + lmem_size = intel_uncore_read64(&i915->uncore, GEN12_GSMBASE); + } + io_start = pci_resource_start(pdev, 2); if (GEM_WARN_ON(lmem_size > pci_resource_len(pdev, 2))) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 0f36af8dc3a1..9b5423572fe9 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -11651,6 +11651,9 @@ enum skl_power_gate { #define SGGI_DIS REG_BIT(15) #define SGR_DIS REG_BIT(13) +#define XEHPSDV_FLAT_CCS_BASE_ADDR _MMIO(0x4910) +#define XEHPSDV_CCS_BASE_SHIFT 8 + /* gamt regs */ #define GEN8_L3_LRA_1_GPGPU _MMIO(0x4dd4) #define GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_BDW 0x67F1427F /* max/min for LRA1/2 */ -- 2.20.1
[PATCH v5 12/19] drm/i915/gt: Clear compress metadata for Xe_HP platforms
From: Ayaz A Siddiqui Xe-HP and latest devices support Flat CCS which reserved a portion of the device memory to store compression metadata, during the clearing of device memory buffer object we also need to clear the associated CCS buffer. Flat CCS memory can not be directly accessed by S/W. Address of CCS buffer associated main BO is automatically calculated by device itself. KMD/UMD can only access this buffer indirectly using XY_CTRL_SURF_COPY_BLT cmd via the address of device memory buffer. v2: Fixed issues with platform naming [Lucas] v3: Rebased [Ram] Used the round_up funcs [Bob] Cc: CQ Tang Signed-off-by: Ayaz A Siddiqui Signed-off-by: Ramalingam C --- drivers/gpu/drm/i915/gt/intel_gpu_commands.h | 14 +++ drivers/gpu/drm/i915/gt/intel_migrate.c | 114 ++- 2 files changed, 125 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h index f8253012d166..07bf5a1753bd 100644 --- a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h +++ b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h @@ -203,6 +203,20 @@ #define GFX_OP_DRAWRECT_INFO ((0x3<<29)|(0x1d<<24)|(0x80<<16)|(0x3)) #define GFX_OP_DRAWRECT_INFO_I965 ((0x7900<<16)|0x2) +#define XY_CTRL_SURF_INSTR_SIZE5 +#define MI_FLUSH_DW_SIZE 3 +#define XY_CTRL_SURF_COPY_BLT ((2 << 29) | (0x48 << 22) | 3) +#define SRC_ACCESS_TYPE_SHIFT21 +#define DST_ACCESS_TYPE_SHIFT20 +#define CCS_SIZE_SHIFT 8 +#define XY_CTRL_SURF_MOCS_SHIFT 25 +#define NUM_CCS_BYTES_PER_BLOCK 256 +#define NUM_CCS_BLKS_PER_XFER1024 +#define INDIRECT_ACCESS 0 +#define DIRECT_ACCESS1 +#define MI_FLUSH_LLC BIT(9) +#define MI_FLUSH_CCS BIT(16) + #define COLOR_BLT_CMD (2 << 29 | 0x40 << 22 | (5 - 2)) #define XY_COLOR_BLT_CMD (2 << 29 | 0x50 << 22) #define SRC_COPY_BLT_CMD (2 << 29 | 0x43 << 22) diff --git a/drivers/gpu/drm/i915/gt/intel_migrate.c b/drivers/gpu/drm/i915/gt/intel_migrate.c index cac791155244..3e1cf224cdf0 100644 --- a/drivers/gpu/drm/i915/gt/intel_migrate.c +++ b/drivers/gpu/drm/i915/gt/intel_migrate.c @@ -16,6 +16,8 @@ struct insert_pte_data { }; #define CHUNK_SZ SZ_8M /* ~1ms at 8GiB/s preemption delay */ +#define GET_CCS_SIZE(i915, size) (HAS_FLAT_CCS(i915) ? \ + DIV_ROUND_UP(size, NUM_CCS_BYTES_PER_BLOCK) : 0) static bool engine_supports_migration(struct intel_engine_cs *engine) { @@ -594,19 +596,105 @@ intel_context_migrate_copy(struct intel_context *ce, return err; } +static inline u32 *i915_flush_dw(u32 *cmd, u64 dst, u32 flags) +{ + /* Mask the 3 LSB to use the PPGTT address space */ + *cmd++ = MI_FLUSH_DW | flags; + *cmd++ = lower_32_bits(dst); + *cmd++ = upper_32_bits(dst); + + return cmd; +} + +static u32 calc_ctrl_surf_instr_size(struct drm_i915_private *i915, int size) +{ + u32 num_cmds, num_blks, total_size; + + if (!GET_CCS_SIZE(i915, size)) + return 0; + + /* +* XY_CTRL_SURF_COPY_BLT transfers CCS in 256 byte +* blocks. one XY_CTRL_SURF_COPY_BLT command can +* trnasfer upto 1024 blocks. +*/ + num_blks = GET_CCS_SIZE(i915, size); + num_cmds = (num_blks + (NUM_CCS_BLKS_PER_XFER - 1)) >> 10; + total_size = (XY_CTRL_SURF_INSTR_SIZE) * num_cmds; + + /* +* We need to add a flush before and after +* XY_CTRL_SURF_COPY_BLT +*/ + total_size += 2 * MI_FLUSH_DW_SIZE; + return total_size; +} + +static u32 *_i915_ctrl_surf_copy_blt(u32 *cmd, u64 src_addr, u64 dst_addr, +u8 src_mem_access, u8 dst_mem_access, +int src_mocs, int dst_mocs, +u16 num_ccs_blocks) +{ + int i = num_ccs_blocks; + + /* +* The XY_CTRL_SURF_COPY_BLT instruction is used to copy the CCS +* data in and out of the CCS region. +* +* We can copy at most 1024 blocks of 256 bytes using one +* XY_CTRL_SURF_COPY_BLT instruction. +* +* In case we need to copy more than 1024 blocks, we need to add +* another instruction to the same batch buffer. +* +* 1024 blocks of 256 bytes of CCS represent a total 256KB of CCS. +* +* 256 KB of CCS represents 256 * 256 KB = 64 MB of LMEM. +*/ + do { + /* +* We use logical AND with 1023 since the size field +* takes values which is in the range of 0 - 1023 +*/ + *cmd++ = ((XY_CTRL_SURF_COPY_BLT) | + (src_mem_access << SRC_ACCESS_TYPE_SHIFT) | + (dst_mem_access << DST_ACCESS_TYPE_SHIFT) | +
[PATCH v5 13/19] drm/i915: Introduce new Tile 4 format
From: Stanislav Lisovskiy This tiling layout uses 4KB tiles in a row-major layout. It has the same shape as Tile Y at two granularities: 4KB (128B x 32) and 64B (16B x 4). It only differs from Tile Y at the 256B granularity in between. At this granularity, Tile Y has a shape of 16B x 32 rows, but this tiling has a shape of 64B x 8 rows. Reviewed-by: Imre Deak Acked-by: Nanley Chery Signed-off-by: Stanislav Lisovskiy --- include/uapi/drm/drm_fourcc.h | 11 +++ 1 file changed, 11 insertions(+) diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index fc0c1454d275..b73fe6797fc3 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -572,6 +572,17 @@ extern "C" { */ #define I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC fourcc_mod_code(INTEL, 8) +/* + * Intel Tile 4 layout + * + * This is a tiled layout using 4KB tiles in a row-major layout. It has the same + * shape as Tile Y at two granularities: 4KB (128B x 32) and 64B (16B x 4). It + * only differs from Tile Y at the 256B granularity in between. At this + * granularity, Tile Y has a shape of 16B x 32 rows, but this tiling has a shape + * of 64B x 8 rows. + */ +#define I915_FORMAT_MOD_4_TILED fourcc_mod_code(INTEL, 9) + /* * Tiled, NV12MT, grouped in 64 (pixels) x 32 (lines) -sized macroblocks * -- 2.20.1
Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
On Tue, 1 Feb 2022 10:49:03 +0100 Javier Martinez Canillas wrote: > On 2/1/22 09:38, Daniel Vetter wrote: > > On Tue, Feb 1, 2022 at 9:34 AM Simon Ser wrote: > >> > >> On Tuesday, February 1st, 2022 at 09:26, Geert Uytterhoeven > >> wrote: > >> > >>> What's the story with the Rn formats? > >>> > >>> The comments say "n bpp Red", while this is a monochrome (even > >>> inverted) display? > >> > >> I don't think the color matters that much. "Red" was picked just because > >> it was > >> an arbitrary color, to make the difference with e.g. C8. Or am I mistaken? > >> > > > > The red comes from gl, where with shaders it really doesn't matter > > what meaning you attach to channels, but really just how many you > > have. So 2-channel formats are called RxGx, 3-channel RxGxBx, > > 4-channel RxGxBxAx and single-channel Rx. And we use drm_fourcc for > > interop in general, hence why these exist. > > > > We should probably make a comment that this really isn't a red channel > > when used for display it's a greyscale/intensity format. Aside from > > that documentation gap I think reusing Rx formats for > > greyscale/intensity for display makes perfect sense. > > -Daniel > > To sump up the conversation in the #dri-devel channel, these drivers > should support the following formats: > > 1) Dx (Daniel suggested that for darkness, but inverted mono) Did you consider format C1 instead? To my understanding, the C formats are paletted, which would also fit very nicely semantically. You have an enumerated list of pixel values and each of them produces some arbitrary color on screen. This would fit e.g. blue/white LCD panels nicely. The little problem there is the palette. C8 format is traditionally translated to RGB triplets through GAMMA LUT. Therefore the display itself is still three-channel, it's just the framebuffer format that is single-channel. But now, we are dealing with truly paletted displays. Furthermore, the palette is fixed, ingrained in the panel hardware. So we would probably need a new KMS property for the fixed palette of the panel. What would it be called? Would it be a connector property? The property would be a read-only blob, an array that maps Cx values to "colors". How do we represent "colors"? How do we accommodate C1, C2, C4 and C8 with the same blob? Since the blob is a mapping from color index to "color", and the array in the blob has N entries, we could simply say that Cx integer value is the color index. If the Cx you use does not go up to N, then you miss some colors. If the Cx you use can go higher than N, then Cx values >= N will clamp to N-1, for example. Of course, if your panel palette has only 4 entries, you can expose C1 and C2 and have no reason to expose C4 or C8, avoiding the Cx >= N issue. How do we define the array contents then, the "colors"... plain old RGB triplets do not mean much[1], but that would be better than nothing. I also suppose that people would not be keen on seeing something like CIE 1931 XYZ or Lab values, even though those would probably have the most useful definition. Coming up with those values properly would require a colorimeter. As a compromise, maybe we could use an RGB triplet, and assume sRGB SDR color space and transfer function, just like we do with all displays whether they are that or not. If someone needs to know better, then they can profile the display. sRGB triplets would likely give enough intuition to what color the indices result in, that it could be used in automated color conversions or quantizations from larger color spaces like sRGB with some rough degree of color similarity. It is a lot of hassle, but it would have a clear benefit: userspace would know very well how the display behaves (what colors it shows, roughly), and you could use Cx formats to drive a panel in its "native" format. Possible problems are around interactions with the old GAMMA property, which is traditionally used for the C8 palette. But if you have a fixed-palette panel, then maybe you wouldn't expose GAMMA property on the CRTC at all? I have no idea how this would map to fbdev API though. [1] https://gitlab.freedesktop.org/pq/color-and-hdr/-/blob/main/doc/pixels_color.md Thanks, pq > 2) Rx (single-channel for grayscale) > 3) RxGxBxAx (4-channel fake 32-bpp truecolor) > > The format preference will be in that order, so if user-space is able > to use Dx then there won't be a need for any conversion and just the > native format will be used. > > If using Rx then only a Rx -> Dx conversion will happen and the last > format will require the less performant RxGxBxAx -> Rx -> Dx path. > > But we still need RxGxBxAx as a fallback for compatibility with the > existing user-space, so all this could be done as a follow-up as an > optimization and shouldn't block monochromatic panel drivers IMO. > > Best regards, pgpbE0gGk1VXT.pgp Description: OpenPGP digital signature
[PATCH v5 14/19] drm/i915/dg2: Tile 4 plane format support
From: Stanislav Lisovskiy Tile4 in bspec format is 4K tile organized into 64B subtiles with same basic shape as for legacy TileY which will be supported by Display13. v2: - Moved Tile4 assocating struct for modifier/display to the beginning(Imre Deak) - Removed unneeded case I915_FORMAT_MOD_4_TILED modifier checks(Imre Deak) - Fixed I915_FORMAT_MOD_4_TILED to be 9 instead of 12 (Imre Deak) v3: - Rebased patch on top of new changes related to plane_caps. - Added static assert to check that PLANE_CTL_TILING_YF matches PLANE_CTL_TILING_4(Nanley Chery) - Fixed naming and layout description for Tile 4 in drm uapi header(Nanley Chery) v4: - Extracted drm_fourcc changes to separate patch(Nanley Chery) Reviewed-by: Imre Deak Cc: Matt Roper Cc: Maarten Lankhorst Signed-off-by: Stanislav Lisovskiy Signed-off-by: Matt Roper Signed-off-by: Juha-Pekka Heikkilä --- drivers/gpu/drm/i915/display/intel_display.c | 1 + drivers/gpu/drm/i915/display/intel_fb.c | 15 +++- drivers/gpu/drm/i915/display/intel_fb.h | 1 + drivers/gpu/drm/i915/display/intel_fbc.c | 1 + .../drm/i915/display/intel_plane_initial.c| 1 + .../drm/i915/display/skl_universal_plane.c| 23 --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_pci.c | 1 + drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_device_info.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 1 + 11 files changed, 38 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 75de794185b2..189767cef356 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -7737,6 +7737,7 @@ static int intel_atomic_check_async(struct intel_atomic_state *state, struct int case I915_FORMAT_MOD_X_TILED: case I915_FORMAT_MOD_Y_TILED: case I915_FORMAT_MOD_Yf_TILED: + case I915_FORMAT_MOD_4_TILED: break; default: drm_dbg_kms(&i915->drm, diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c index 23cfe2e5ce2a..94c57facbb46 100644 --- a/drivers/gpu/drm/i915/display/intel_fb.c +++ b/drivers/gpu/drm/i915/display/intel_fb.c @@ -135,11 +135,16 @@ struct intel_modifier_desc { INTEL_PLANE_CAP_CCS_MC) #define INTEL_PLANE_CAP_TILING_MASK(INTEL_PLANE_CAP_TILING_X | \ INTEL_PLANE_CAP_TILING_Y | \ -INTEL_PLANE_CAP_TILING_Yf) +INTEL_PLANE_CAP_TILING_Yf | \ +INTEL_PLANE_CAP_TILING_4) #define INTEL_PLANE_CAP_TILING_NONE0 static const struct intel_modifier_desc intel_modifiers[] = { { + .modifier = I915_FORMAT_MOD_4_TILED, + .display_ver = { 13, 13 }, + .plane_caps = INTEL_PLANE_CAP_TILING_4, + }, { .modifier = I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS, .display_ver = { 12, 13 }, .plane_caps = INTEL_PLANE_CAP_TILING_Y | INTEL_PLANE_CAP_CCS_MC, @@ -545,6 +550,12 @@ intel_tile_width_bytes(const struct drm_framebuffer *fb, int color_plane) return 128; else return 512; + case I915_FORMAT_MOD_4_TILED: + /* +* Each 4K tile consists of 64B(8*8) subtiles, with +* same shape as Y Tile(i.e 4*16B OWords) +*/ + return 128; case I915_FORMAT_MOD_Y_TILED_CCS: if (intel_fb_is_ccs_aux_plane(fb, color_plane)) return 128; @@ -650,6 +661,7 @@ static unsigned int intel_fb_modifier_to_tiling(u64 fb_modifier) return I915_TILING_Y; case INTEL_PLANE_CAP_TILING_X: return I915_TILING_X; + case INTEL_PLANE_CAP_TILING_4: case INTEL_PLANE_CAP_TILING_Yf: case INTEL_PLANE_CAP_TILING_NONE: return I915_TILING_NONE; @@ -737,6 +749,7 @@ unsigned int intel_surf_alignment(const struct drm_framebuffer *fb, case I915_FORMAT_MOD_Y_TILED_CCS: case I915_FORMAT_MOD_Yf_TILED_CCS: case I915_FORMAT_MOD_Y_TILED: + case I915_FORMAT_MOD_4_TILED: case I915_FORMAT_MOD_Yf_TILED: return 1 * 1024 * 1024; default: diff --git a/drivers/gpu/drm/i915/display/intel_fb.h b/drivers/gpu/drm/i915/display/intel_fb.h index ba9df8986c1e..12386f13a4e0 100644 --- a/drivers/gpu/drm/i915/display/intel_fb.h +++ b/drivers/gpu/drm/i915/display/intel_fb.h @@ -27,6 +27,7 @@ struct intel_plane_state; #define INTEL_PLANE_CAP_TILING_X BIT(3) #define
[PATCH v5 15/19] drm/i915/dg2: Add DG2 unified compression
From: Matt Roper DG2 unifies render compression and media compression into a single format for the first time. The programming and buffer layout is supposed to match compression on older gen12 platforms, but the actual compression algorithm is different from any previous platform; as such, we need a new framebuffer modifier to represent buffers in this format, but otherwise we can re-use the existing gen12 compression driver logic. v2: Display version fix [Imre] Signed-off-by: Matt Roper cc: Radhakrishna Sripada Signed-off-by: Mika Kahola (v2) cc: Anshuman Gupta Signed-off-by: Juha-Pekka Heikkilä Signed-off-by: Ramalingam C --- drivers/gpu/drm/i915/display/intel_fb.c | 13 ++ .../drm/i915/display/skl_universal_plane.c| 26 --- include/uapi/drm/drm_fourcc.h | 22 3 files changed, 57 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c index 94c57facbb46..4d4d01963f15 100644 --- a/drivers/gpu/drm/i915/display/intel_fb.c +++ b/drivers/gpu/drm/i915/display/intel_fb.c @@ -141,6 +141,14 @@ struct intel_modifier_desc { static const struct intel_modifier_desc intel_modifiers[] = { { + .modifier = I915_FORMAT_MOD_4_TILED_DG2_MC_CCS, + .display_ver = { 13, 13 }, + .plane_caps = INTEL_PLANE_CAP_TILING_4 | INTEL_PLANE_CAP_CCS_MC, + }, { + .modifier = I915_FORMAT_MOD_4_TILED_DG2_RC_CCS, + .display_ver = { 13, 13 }, + .plane_caps = INTEL_PLANE_CAP_TILING_4 | INTEL_PLANE_CAP_CCS_RC, + }, { .modifier = I915_FORMAT_MOD_4_TILED, .display_ver = { 13, 13 }, .plane_caps = INTEL_PLANE_CAP_TILING_4, @@ -550,6 +558,8 @@ intel_tile_width_bytes(const struct drm_framebuffer *fb, int color_plane) return 128; else return 512; + case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS: + case I915_FORMAT_MOD_4_TILED_DG2_MC_CCS: case I915_FORMAT_MOD_4_TILED: /* * Each 4K tile consists of 64B(8*8) subtiles, with @@ -752,6 +762,9 @@ unsigned int intel_surf_alignment(const struct drm_framebuffer *fb, case I915_FORMAT_MOD_4_TILED: case I915_FORMAT_MOD_Yf_TILED: return 1 * 1024 * 1024; + case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS: + case I915_FORMAT_MOD_4_TILED_DG2_MC_CCS: + return 16 * 1024; default: MISSING_CASE(fb->modifier); return 0; diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c index 5299dfe68802..c38ae0876c15 100644 --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c @@ -764,6 +764,14 @@ static u32 skl_plane_ctl_tiling(u64 fb_modifier) return PLANE_CTL_TILED_Y; case I915_FORMAT_MOD_4_TILED: return PLANE_CTL_TILED_4; + case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS: + return PLANE_CTL_TILED_4 | + PLANE_CTL_RENDER_DECOMPRESSION_ENABLE | + PLANE_CTL_CLEAR_COLOR_DISABLE; + case I915_FORMAT_MOD_4_TILED_DG2_MC_CCS: + return PLANE_CTL_TILED_4 | + PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE | + PLANE_CTL_CLEAR_COLOR_DISABLE; case I915_FORMAT_MOD_Y_TILED_CCS: case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC: return PLANE_CTL_TILED_Y | PLANE_CTL_RENDER_DECOMPRESSION_ENABLE; @@ -2094,6 +2102,10 @@ static bool gen12_plane_has_mc_ccs(struct drm_i915_private *i915, if (IS_ADLP_DISPLAY_STEP(i915, STEP_A0, STEP_B0)) return false; + /* Wa_14013215631 */ + if (IS_DG2_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) + return false; + return plane_id < PLANE_SPRITE4; } @@ -2335,9 +2347,10 @@ skl_get_initial_plane_config(struct intel_crtc *crtc, case PLANE_CTL_TILED_Y: plane_config->tiling = I915_TILING_Y; if (val & PLANE_CTL_RENDER_DECOMPRESSION_ENABLE) - fb->modifier = DISPLAY_VER(dev_priv) >= 12 ? - I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS : - I915_FORMAT_MOD_Y_TILED_CCS; + if (DISPLAY_VER(dev_priv) >= 12) + fb->modifier = I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS; + else + fb->modifier = I915_FORMAT_MOD_Y_TILED_CCS; else if (val & PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE) fb->modifier = I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS; else @@ -2345,7 +2358,12 @@ skl_get_initial_plane_config(struct intel_crtc *crtc, break;
[PATCH v5 16/19] uapi/drm/dg2: Introduce format modifier for DG2 clear color
From: Mika Kahola DG2 clear color render compression uses Tile4 layout. Therefore, we need to define a new format modifier for uAPI to support clear color rendering. v2: Display version is fixed. [Imre] KDoc is enhanced for cc modifier. [Nanley & Lionel] Signed-off-by: Mika Kahola cc: Anshuman Gupta Signed-off-by: Juha-Pekka Heikkilä Signed-off-by: Ramalingam C --- drivers/gpu/drm/i915/display/intel_fb.c| 8 drivers/gpu/drm/i915/display/skl_universal_plane.c | 9 - include/uapi/drm/drm_fourcc.h | 10 ++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c index 4d4d01963f15..3df6ef5ffec5 100644 --- a/drivers/gpu/drm/i915/display/intel_fb.c +++ b/drivers/gpu/drm/i915/display/intel_fb.c @@ -144,6 +144,12 @@ static const struct intel_modifier_desc intel_modifiers[] = { .modifier = I915_FORMAT_MOD_4_TILED_DG2_MC_CCS, .display_ver = { 13, 13 }, .plane_caps = INTEL_PLANE_CAP_TILING_4 | INTEL_PLANE_CAP_CCS_MC, + }, { + .modifier = I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC, + .display_ver = { 13, 13 }, + .plane_caps = INTEL_PLANE_CAP_TILING_4 | INTEL_PLANE_CAP_CCS_RC_CC, + + .ccs.cc_planes = BIT(1), }, { .modifier = I915_FORMAT_MOD_4_TILED_DG2_RC_CCS, .display_ver = { 13, 13 }, @@ -559,6 +565,7 @@ intel_tile_width_bytes(const struct drm_framebuffer *fb, int color_plane) else return 512; case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS: + case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC: case I915_FORMAT_MOD_4_TILED_DG2_MC_CCS: case I915_FORMAT_MOD_4_TILED: /* @@ -763,6 +770,7 @@ unsigned int intel_surf_alignment(const struct drm_framebuffer *fb, case I915_FORMAT_MOD_Yf_TILED: return 1 * 1024 * 1024; case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS: + case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC: case I915_FORMAT_MOD_4_TILED_DG2_MC_CCS: return 16 * 1024; default: diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c index c38ae0876c15..b4dced1907c5 100644 --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c @@ -772,6 +772,8 @@ static u32 skl_plane_ctl_tiling(u64 fb_modifier) return PLANE_CTL_TILED_4 | PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE | PLANE_CTL_CLEAR_COLOR_DISABLE; + case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC: + return PLANE_CTL_TILED_4 | PLANE_CTL_RENDER_DECOMPRESSION_ENABLE; case I915_FORMAT_MOD_Y_TILED_CCS: case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC: return PLANE_CTL_TILED_Y | PLANE_CTL_RENDER_DECOMPRESSION_ENABLE; @@ -2358,10 +2360,15 @@ skl_get_initial_plane_config(struct intel_crtc *crtc, break; case PLANE_CTL_TILED_YF: /* aka PLANE_CTL_TILED_4 on XE_LPD+ */ if (HAS_4TILE(dev_priv)) { - if (val & PLANE_CTL_RENDER_DECOMPRESSION_ENABLE) + u32 rc_mask = PLANE_CTL_RENDER_DECOMPRESSION_ENABLE | + PLANE_CTL_CLEAR_COLOR_DISABLE; + + if ((val & rc_mask) == rc_mask) fb->modifier = I915_FORMAT_MOD_4_TILED_DG2_RC_CCS; else if (val & PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE) fb->modifier = I915_FORMAT_MOD_4_TILED_DG2_MC_CCS; + else if (val & PLANE_CTL_RENDER_DECOMPRESSION_ENABLE) + fb->modifier = I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC; else fb->modifier = I915_FORMAT_MOD_4_TILED; } else { diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index b8fb7b44c03c..697614ea4b84 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -605,6 +605,16 @@ extern "C" { */ #define I915_FORMAT_MOD_4_TILED_DG2_MC_CCS fourcc_mod_code(INTEL, 11) +/* + * Intel color control surfaces (CCS) for DG2 clear color render compression. + * + * DG2 uses a unified compression format for clear color render compression. + * The general layout is a tiled layout using 4Kb tiles i.e. Tile4 layout. + * + * Fast clear color value expected by HW is located in fb at offset 0 of plane#1 + */ +#define I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC fourcc_mod_code(INTEL, 12) + /* * Tiled, NV12MT, grouped in 64 (pixels) x 32 (lines) -sized macroblocks * -- 2.20.1
[PATCH v5 17/19] drm/i915/dg2: Flat CCS Support
From: Anshuman Gupta DG2 onwards discrete gfx has support for new flat CCS mapping, which brings in display feature in to avoid Aux walk for compressed surface. This support build on top of Flat CCS support added in XEHPSDV. FLAT CCS surface base address should be 64k aligned, Compressed displayable surfaces must use tile4 format. HAS: 1407880786 B.Spec : 7655 B.Spec : 53902 Cc: Mika Kahola Signed-off-by: Anshuman Gupta Signed-off-by: Juha-Pekka Heikkilä Signed-off-by: Ramalingam C --- drivers/gpu/drm/i915/display/intel_display.c | 4 ++- drivers/gpu/drm/i915/display/intel_fb.c | 32 +-- .../drm/i915/display/skl_universal_plane.c| 16 ++ 3 files changed, 36 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 189767cef356..2828ae612179 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -8588,7 +8588,9 @@ static void intel_atomic_prepare_plane_clear_colors(struct intel_atomic_state *s /* * The layout of the fast clear color value expected by HW -* (the DRM ABI requiring this value to be located in fb at offset 0 of plane#2): +* (the DRM ABI requiring this value to be located in fb at +* offset 0 of cc plane, plane #2 previous generations or +* plane #1 for flat ccs): * - 4 x 4 bytes per-channel value * (in surface type specific float/int format provided by the fb user) * - 8 bytes native color value used by the display diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c index 3df6ef5ffec5..e94923e9dbb1 100644 --- a/drivers/gpu/drm/i915/display/intel_fb.c +++ b/drivers/gpu/drm/i915/display/intel_fb.c @@ -107,6 +107,21 @@ static const struct drm_format_info gen12_ccs_cc_formats[] = { .hsub = 1, .vsub = 1, .has_alpha = true }, }; +static const struct drm_format_info gen12_flat_ccs_cc_formats[] = { + { .format = DRM_FORMAT_XRGB, .depth = 24, .num_planes = 2, + .char_per_block = { 4, 0 }, .block_w = { 1, 2 }, .block_h = { 1, 1 }, + .hsub = 1, .vsub = 1, }, + { .format = DRM_FORMAT_XBGR, .depth = 24, .num_planes = 2, + .char_per_block = { 4, 0 }, .block_w = { 1, 2 }, .block_h = { 1, 1 }, + .hsub = 1, .vsub = 1, }, + { .format = DRM_FORMAT_ARGB, .depth = 32, .num_planes = 2, + .char_per_block = { 4, 0 }, .block_w = { 1, 2 }, .block_h = { 1, 1 }, + .hsub = 1, .vsub = 1, .has_alpha = true }, + { .format = DRM_FORMAT_ABGR, .depth = 32, .num_planes = 2, + .char_per_block = { 4, 0 }, .block_w = { 1, 2 }, .block_h = { 1, 1 }, + .hsub = 1, .vsub = 1, .has_alpha = true }, +}; + struct intel_modifier_desc { u64 modifier; struct { @@ -150,6 +165,8 @@ static const struct intel_modifier_desc intel_modifiers[] = { .plane_caps = INTEL_PLANE_CAP_TILING_4 | INTEL_PLANE_CAP_CCS_RC_CC, .ccs.cc_planes = BIT(1), + + FORMAT_OVERRIDE(gen12_flat_ccs_cc_formats), }, { .modifier = I915_FORMAT_MOD_4_TILED_DG2_RC_CCS, .display_ver = { 13, 13 }, @@ -399,17 +416,13 @@ bool intel_fb_plane_supports_modifier(struct intel_plane *plane, u64 modifier) static bool format_is_yuv_semiplanar(const struct intel_modifier_desc *md, const struct drm_format_info *info) { - int yuv_planes; - if (!info->is_yuv) return false; - if (plane_caps_contain_any(md->plane_caps, INTEL_PLANE_CAP_CCS_MASK)) - yuv_planes = 4; + if (hweight8(md->ccs.planar_aux_planes) == 2) + return info->num_planes == 4; else - yuv_planes = 2; - - return info->num_planes == yuv_planes; + return info->num_planes == 2; } /** @@ -534,12 +547,13 @@ static unsigned int gen12_ccs_aux_stride(struct intel_framebuffer *fb, int ccs_p int skl_main_to_aux_plane(const struct drm_framebuffer *fb, int main_plane) { + const struct intel_modifier_desc *md = lookup_modifier(fb->modifier); struct drm_i915_private *i915 = to_i915(fb->dev); - if (intel_fb_is_ccs_modifier(fb->modifier)) + if (md->ccs.packed_aux_planes | md->ccs.planar_aux_planes) return main_to_ccs_plane(fb, main_plane); else if (DISPLAY_VER(i915) < 11 && -intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier)) +format_is_yuv_semiplanar(md, fb->format)) return 1; else return 0; diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c index b4dced1907c5..18e50583abaa 100644 --- a/drivers
[PATCH v5 18/19] drm/i915/Flat-CCS: Document on Flat-CCS memory compression
Documents the Flat-CCS feature and kernel handling required along with modifiers used. Signed-off-by: Ramalingam C cc: Simon Ser cc: Pekka Paalanen Cc: Jordan Justen Cc: Kenneth Graunke Cc: mesa-...@lists.freedesktop.org Cc: Tony Ye Cc: Slawomir Milczarek --- drivers/gpu/drm/i915/gt/intel_migrate.c | 47 + 1 file changed, 47 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_migrate.c b/drivers/gpu/drm/i915/gt/intel_migrate.c index 3e1cf224cdf0..5bdab0b3c735 100644 --- a/drivers/gpu/drm/i915/gt/intel_migrate.c +++ b/drivers/gpu/drm/i915/gt/intel_migrate.c @@ -596,6 +596,53 @@ intel_context_migrate_copy(struct intel_context *ce, return err; } +/** + * DOC: Flat-CCS - Memory compression for Local memory + * + * On Xe-HP and later devices, we use dedicated compression control state (CCS) + * stored in local memory for each surface, to support the 3D and media + * compression formats. + * + * The memory required for the CCS of the entire local memory is 1/256 of the + * local memory size. So before the kernel boot, the required memory is reserved + * for the CCS data and a secure register will be programmed with the CCS base + * address. + * + * Flat CCS data needs to be cleared when a lmem object is allocated. + * And CCS data can be copied in and out of CCS region through + * XY_CTRL_SURF_COPY_BLT. CPU can't access the CCS data directly. + * + * When we exaust the lmem, if the object's placements support smem, then we can + * directly decompress the compressed lmem object into smem and start using it + * from smem itself. + * + * But when we need to swapout the compressed lmem object into a smem region + * though objects' placement doesn't support smem, then we copy the lmem content + * as it is into smem region along with ccs data (using XY_CTRL_SURF_COPY_BLT). + * When the object is referred, lmem content will be swaped in along with + * restoration of the CCS data (using XY_CTRL_SURF_COPY_BLT) at corresponding + * location. + * + * + * Flat-CCS Modifiers for different compression formats + * + * + * I915_FORMAT_MOD_F_TILED_DG2_RC_CCS - used to indicate the buffers of Flat CCS + * render compression formats. Though the general layout is same as + * I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS, new hashing/compression algorithm is + * used. Render compression uses 128 byte compression blocks + * + * I915_FORMAT_MOD_F_TILED_DG2_MC_CCS -used to indicate the buffers of Flat CCS + * media compression formats. Though the general layout is same as + * I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS, new hashing/compression algorithm is + * used. Media compression uses 256 byte compression blocks. + * + * I915_FORMAT_MOD_F_TILED_DG2_RC_CCS_CC - used to indicate the buffers of Flat + * CCS clear color render compression formats. Unified compression format for + * clear color render compression. The genral layout is a tiled layout using + * 4Kb tiles i.e Tile4 layout. + */ + static inline u32 *i915_flush_dw(u32 *cmd, u64 dst, u32 flags) { /* Mask the 3 LSB to use the PPGTT address space */ -- 2.20.1
[PATCH v5 19/19] Doc/gpu/rfc/i915: i915 DG2 flat-CCS uAPI
Details of the Flat-CCS getting added as part of DG2 enabling and its implicit impact on the uAPI. Signed-off-by: Ramalingam C cc: Daniel Vetter cc: Matthew Auld cc: Simon Ser cc: Pekka Paalanen Cc: Jordan Justen Cc: Kenneth Graunke Cc: mesa-...@lists.freedesktop.org Cc: Tony Ye Cc: Slawomir Milczarek --- Documentation/gpu/rfc/i915_dg2.rst | 7 +++ 1 file changed, 7 insertions(+) diff --git a/Documentation/gpu/rfc/i915_dg2.rst b/Documentation/gpu/rfc/i915_dg2.rst index f4eb5a219897..9d28b1812bc7 100644 --- a/Documentation/gpu/rfc/i915_dg2.rst +++ b/Documentation/gpu/rfc/i915_dg2.rst @@ -23,3 +23,10 @@ handling the 64k page size. .. kernel-doc:: include/uapi/drm/i915_drm.h :functions: drm_i915_gem_create_ext + + +Flat CCS support for lmem += + +.. kernel-doc:: drivers/gpu/drm/i915/gt/intel_migrate.c +:doc: Flat-CCS - Memory compression for Local memory -- 2.20.1
Re: [PATCH 01/21] MAINTAINERS: Add entry for fbdev core
On Tue, Feb 01, 2022 at 11:19:54AM +0100, Thomas Zimmermann wrote: > > > Am 31.01.22 um 22:05 schrieb Daniel Vetter: > > Ever since Tomi extracted the core code in 2014 it's been defacto me > > maintaining this, with help from others from dri-devel and sometimes > > Linus (but those are mostly merge conflicts): > > > > $ git shortlog -ns drivers/video/fbdev/core/ | head -n5 > > 35 Daniel Vetter > > 23 Linus Torvalds > > 10 Hans de Goede > > 9 Dave Airlie > > 6 Peter Rosin > > > > I think ideally we'd also record that the various firmware fb drivers > > (efifb, vesafb, ...) are also maintained in drm-misc because for the > > past few years the patches have either been to fix handover issues > > with drm drivers, or caused handover issues with drm drivers. So any > > other tree just doesn't make sense. But also, there's plenty of > > outdated MAINTAINER entries for these with people and git trees that > > haven't been active in years, so maybe let's just leave them alone. > > And furthermore distros are now adopting simpledrm as the firmware fb > > driver, so hopefully the need to care about the fbdev firmware drivers > > will go down going forward. > > > > Note that drm-misc is group maintained, I expect that to continue like > > we've done before, so no new expectations that patches all go through > > my hands. That would be silly. This also means I'm happy to put any > > other volunteer's name in the M: line, but otherwise git log says I'm > > the one who's stuck with this. > > > > Cc: Dave Airlie > > Cc: Jani Nikula > > Cc: Linus Torvalds > > Cc: Linux Fbdev development list > > Cc: Pavel Machek > > Cc: Sam Ravnborg > > Cc: Greg Kroah-Hartman > > Cc: Javier Martinez Canillas > > Cc: DRI Development > > Cc: Linux Kernel Mailing List > > Cc: Claudio Suarez > > Cc: Tomi Valkeinen > > Cc: Geert Uytterhoeven > > Cc: Thomas Zimmermann > > Cc: Daniel Vetter > > Cc: Sven Schnelle > > Cc: Gerd Hoffmann > > Signed-off-by: Daniel Vetter > > Acked-by: Thomas Zimmermann Acked-by: Greg Kroah-Hartman
[Bug 215549] My 6900XT can't recover while it's idle (but not asleep), and sometimes doesn't show at boot
https://bugzilla.kernel.org/show_bug.cgi?id=215549 Artem S. Tashkinov (a...@gmx.com) changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |MOVED Regression|No |Yes --- Comment #2 from Artem S. Tashkinov (a...@gmx.com) --- Please refile here: https://gitlab.freedesktop.org/drm/amd -- 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 v5 07/19] drm/i915/migrate: add acceleration support for DG2
On 01/02/2022 10:41, Ramalingam C wrote: From: Matthew Auld This is all kinds of awkward since we now have to contend with using 64K GTT pages when mapping anything in LMEM(including the page-tables themselves). v2: Rebased [Ram] Signed-off-by: Matthew Auld Cc: Thomas Hellström Cc: Ramalingam C This version seems to be missing your review feedback, which I incorporated here[1]. [1] https://patchwork.freedesktop.org/series/97544/ --- drivers/gpu/drm/i915/gt/intel_migrate.c | 179 +++- 1 file changed, 147 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_migrate.c b/drivers/gpu/drm/i915/gt/intel_migrate.c index 18b44af56969..cac791155244 100644 --- a/drivers/gpu/drm/i915/gt/intel_migrate.c +++ b/drivers/gpu/drm/i915/gt/intel_migrate.c @@ -32,6 +32,38 @@ static bool engine_supports_migration(struct intel_engine_cs *engine) return true; } +static void xehpsdv_toggle_pdes(struct i915_address_space *vm, + struct i915_page_table *pt, + void *data) +{ + struct insert_pte_data *d = data; + + /* +* Insert a dummy PTE into every PT that will map to LMEM to ensure +* we have a correctly setup PDE structure for later use. +*/ + vm->insert_page(vm, 0, d->offset, I915_CACHE_NONE, PTE_LM); + GEM_BUG_ON(!pt->is_compact); + d->offset += SZ_2M; +} + +static void xehpsdv_insert_pte(struct i915_address_space *vm, + struct i915_page_table *pt, + void *data) +{ + struct insert_pte_data *d = data; + + /* +* We are playing tricks here, since the actual pt, from the hw +* pov, is only 256bytes with 32 entries, or 4096bytes with 512 +* entries, but we are still guaranteed that the physical +* alignment is 64K underneath for the pt, and we are careful +* not to access the space in the void. +*/ + vm->insert_page(vm, px_dma(pt), d->offset, I915_CACHE_NONE, PTE_LM); + d->offset += SZ_64K; +} + static void insert_pte(struct i915_address_space *vm, struct i915_page_table *pt, void *data) @@ -74,7 +106,12 @@ static struct i915_address_space *migrate_vm(struct intel_gt *gt) * i.e. within the same non-preemptible window so that we do not switch * to another migration context that overwrites the PTE. * -* TODO: Add support for huge LMEM PTEs +* On platforms with HAS_64K_PAGES support we have three windows, and +* dedicate two windows just for mapping lmem pages(smem <-> smem is not +* a thing), since we are forced to use 64K GTT pages underneath which +* requires also modifying the PDE. An alternative might be to instead +* map the PD into the GTT, and then on the fly toggle the 4K/64K mode +* in the PDE from the same batch that also modifies the PTEs. */ vm = i915_ppgtt_create(gt, I915_BO_ALLOC_PM_EARLY); @@ -86,6 +123,9 @@ static struct i915_address_space *migrate_vm(struct intel_gt *gt) goto err_vm; } + if (HAS_64K_PAGES(gt->i915)) + stash.pt_sz = I915_GTT_PAGE_SIZE_64K; + /* * Each engine instance is assigned its own chunk in the VM, so * that we can run multiple instances concurrently @@ -105,14 +145,20 @@ static struct i915_address_space *migrate_vm(struct intel_gt *gt) * We copy in 8MiB chunks. Each PDE covers 2MiB, so we need * 4x2 page directories for source/destination. */ - sz = 2 * CHUNK_SZ; + if (HAS_64K_PAGES(gt->i915)) + sz = 3 * CHUNK_SZ; + else + sz = 2 * CHUNK_SZ; d.offset = base + sz; /* * We need another page directory setup so that we can write * the 8x512 PTE in each chunk. */ - sz += (sz >> 12) * sizeof(u64); + if (HAS_64K_PAGES(gt->i915)) + sz += (sz / SZ_2M) * SZ_64K; + else + sz += (sz >> 12) * sizeof(u64); err = i915_vm_alloc_pt_stash(&vm->vm, &stash, sz); if (err) @@ -133,7 +179,18 @@ static struct i915_address_space *migrate_vm(struct intel_gt *gt) goto err_vm; /* Now allow the GPU to rewrite the PTE via its own ppGTT */ - vm->vm.foreach(&vm->vm, base, d.offset - base, insert_pte, &d); + if (HAS_64K_PAGES(gt->i915)) { + vm->vm.foreach(&vm->vm, base, d.offset - base, + xehpsdv_insert_pte, &d); + d.offset = base + CHUNK_SZ; + vm->vm.foreach(&vm->vm, + d.offset, +
[PATCH v3] fbdev: fbmem: Fix the implicit type casting
In function do_fb_ioctl(), the "arg" is the type of unsigned long, and in "case FBIOBLANK:" this argument is casted into an int before passig to fb_blank(). In fb_blank(), the comparision if (blank > FB_BLANK_POWERDOWN) would be bypass if the original "arg" is a large number, which is possible because it comes from the user input. Fix this by adding the check before the function call. Signed-off-by: Yizhuo Zhai --- drivers/video/fbdev/core/fbmem.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 0fa7ede94fa6..991711bfd647 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -1162,6 +1162,8 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, case FBIOBLANK: console_lock(); lock_fb_info(info); + if (arg > FB_BLANK_POWERDOWN) + arg = FB_BLANK_POWERDOWN; ret = fb_blank(info, arg); /* might again call into fb_blank */ fbcon_fb_blanked(info, arg); -- 2.25.1
[PATCH v3] fbdev: fbmem: Fix the implicit type casting
In function do_fb_ioctl(), the "arg" is the type of unsigned long, and in "case FBIOBLANK:" this argument is casted into an int before passig to fb_blank(). In fb_blank(), the comparision if (blank > FB_BLANK_POWERDOWN) would be bypass if the original "arg" is a large number, which is possible because it comes from the user input. Fix this by adding the check before the function call. Signed-off-by: Yizhuo Zhai --- drivers/video/fbdev/core/fbmem.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 0fa7ede94fa6..991711bfd647 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -1162,6 +1162,8 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, case FBIOBLANK: console_lock(); lock_fb_info(info); + if (arg > FB_BLANK_POWERDOWN) + arg = FB_BLANK_POWERDOWN; ret = fb_blank(info, arg); /* might again call into fb_blank */ fbcon_fb_blanked(info, arg); -- 2.25.1
Re: [PATCH 03/21] fbcon: Restore fbcon scrolling acceleration
On 2/1/22 11:36, Daniel Vetter wrote: > On Tue, Feb 1, 2022 at 11:16 AM Helge Deller wrote: >> >> On 1/31/22 22:05, Daniel Vetter wrote: >>> This functionally undoes 39aead8373b3 ("fbcon: Disable accelerated >>> scrolling"), but behind the FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION >>> option. >> >> you have two trivial copy-n-paste errors in this patch which still prevent >> the >> console acceleration. > > Duh :-( > > But before we dig into details I think the big picture would be > better. I honestly don't like the #ifdef pile here that much. Me neither. The ifdefs give a better separation, but prevents that the compiler checks the various paths when building. > I wonder whether your approach, also with GETVX/YRES adjusted > somehow, wouldn't look cleaner? I think so. You wouldn't even need to touch GETVX/YRES because the compiler will optimize/reduce it from #define GETVYRES(s,i) ({ \ (s == SCROLL_REDRAW || s == SCROLL_MOVE) ? \ (i)->var.yres : (i)->var.yres_virtual; }) to just become: #define GETVYRES(s,i) ((i)->var.yres) > Like I said in the cover letter I got mostly distracted with fbcon > locking last week, not really with this one here at all, so maybe > going with your 4 (or 2 if we squash them like I did here) patches is > neater? The benefit of my patch series was, that it could be easily backported first, and then cleaned up afterwards. Even a small additional backport patch to disable the fbcon acceleration for DRM in the old releases would be easy. But I'm not insisting on backporting the patches, if we find good way forward. So, either with the 4 (or 2) patches would be OK for me (or even your approach). Helge
Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
Hi Pekka, On Tue, Feb 1, 2022 at 11:42 AM Pekka Paalanen wrote: > On Tue, 1 Feb 2022 10:49:03 +0100 > Javier Martinez Canillas wrote: > > On 2/1/22 09:38, Daniel Vetter wrote: > > > On Tue, Feb 1, 2022 at 9:34 AM Simon Ser wrote: > > >> On Tuesday, February 1st, 2022 at 09:26, Geert Uytterhoeven > > >> wrote: > > >>> What's the story with the Rn formats? > > >>> > > >>> The comments say "n bpp Red", while this is a monochrome (even > > >>> inverted) display? > > >> > > >> I don't think the color matters that much. "Red" was picked just because > > >> it was > > >> an arbitrary color, to make the difference with e.g. C8. Or am I > > >> mistaken? > > > > > > The red comes from gl, where with shaders it really doesn't matter > > > what meaning you attach to channels, but really just how many you > > > have. So 2-channel formats are called RxGx, 3-channel RxGxBx, > > > 4-channel RxGxBxAx and single-channel Rx. And we use drm_fourcc for > > > interop in general, hence why these exist. > > > > > > We should probably make a comment that this really isn't a red channel > > > when used for display it's a greyscale/intensity format. Aside from > > > that documentation gap I think reusing Rx formats for > > > greyscale/intensity for display makes perfect sense. > > > -Daniel > > > > To sump up the conversation in the #dri-devel channel, these drivers > > should support the following formats: > > > > 1) Dx (Daniel suggested that for darkness, but inverted mono) > > Did you consider format C1 instead? That would be a 2-color display, which is not necessarily black and white. Cfr. Amiga or Atari bit planes with bpp=1. That's why fbdev has separate visuals for monochrome. > I have no idea how this would map to fbdev API though. #define FB_VISUAL_MONO010 /* Monochr. 1=Black 0=White */ #define FB_VISUAL_MONO101 /* Monochr. 1=White 0=Black */ #define FB_VISUAL_TRUECOLOR 2 /* True color */ The above is RGB (or grayscale, see below). #define FB_VISUAL_PSEUDOCOLOR 3 /* Pseudo color (like atari) */ Palette #define FB_VISUAL_DIRECTCOLOR 4 /* Direct color */ Usually used as RGB with gamma correction, but the actual hardware is more flexible. #define FB_VISUAL_STATIC_PSEUDOCOLOR5 /* Pseudo color readonly */ Fixed palette And: struct fb_var_screeninfo { ... __u32 grayscale;/* 0 = color, 1 = grayscale,*/ Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH] drm/panel: simple: Assign data from panel_dpi_probe() correctly
On 2/1/22 12:01, Christoph Niedermaier wrote: In the function panel_simple_probe() the pointer panel->desc is assigned to the passed pointer desc. If function panel_dpi_probe() is called panel->desc will be updated, but further on only desc will be evaluated. So update the desc pointer to be able to use the data from the function panel_dpi_probe(). Fixes: 4a1d0dbc8332 ("drm/panel: simple: add panel-dpi support") Signed-off-by: Christoph Niedermaier Cc: Marek Vasut Cc: Thierry Reding Cc: Sam Ravnborg Cc: David Airlie Cc: Daniel Vetter To: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/panel/panel-simple.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index 9e46db5e359c..3c08f9827acf 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -588,6 +588,7 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc) err = panel_dpi_probe(dev, panel); if (err) goto free_ddc; + desc = panel->desc; } else { if (!of_get_display_timing(dev->of_node, "panel-timing", &dt)) panel_simple_parse_panel_timing_node(dev, panel, &dt); Reviewed-by: Marek Vasut
Re: [PATCH] drm/imx: parallel-display: Remove bus flags check in imx_pd_bridge_atomic_check()
On 2/1/22 12:03, Christoph Niedermaier wrote: If display timings were read from the devicetree using of_get_display_timing() and pixelclk-active is defined there, the flag DISPLAY_FLAGS_SYNC_POSEDGE/NEGEDGE is automatically generated. Through the function drm_bus_flags_from_videomode() e.g. called in the panel-simple driver this flag got into the bus flags, but then in imx_pd_bridge_atomic_check() the bus flag check failed and will not initialize the display. The original commit fe141cedc433 does not explain why this check was introduced. So remove the bus flags check, because it stops the initialization of the display with valid bus flags. Fixes: fe141cedc433 ("drm/imx: pd: Use bus format/flags provided by the bridge when available") Signed-off-by: Christoph Niedermaier Cc: Marek Vasut Cc: Philipp Zabel Cc: David Airlie Cc: Daniel Vetter Cc: Shawn Guo Cc: Sascha Hauer Cc: Pengutronix Kernel Team Cc: Fabio Estevam Cc: NXP Linux Team Cc: linux-arm-ker...@lists.infradead.org To: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/imx/parallel-display.c | 8 1 file changed, 8 deletions(-) diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c index a8aba0141ce7..06cb1a59b9bc 100644 --- a/drivers/gpu/drm/imx/parallel-display.c +++ b/drivers/gpu/drm/imx/parallel-display.c @@ -217,14 +217,6 @@ static int imx_pd_bridge_atomic_check(struct drm_bridge *bridge, if (!imx_pd_format_supported(bus_fmt)) return -EINVAL; - if (bus_flags & - ~(DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_DE_HIGH | - DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE | - DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)) { - dev_warn(imxpd->dev, "invalid bus_flags (%x)\n", bus_flags); - return -EINVAL; - } +CC Boris
Re: [PATCH 2/4] drm/format-helper: Add drm_fb_gray8_to_mono_reversed()
On Tue, 1 Feb 2022 10:59:43 +0100 Thomas Zimmermann wrote: > Hi > > Am 31.01.22 um 21:12 schrieb Javier Martinez Canillas: > > Add support to convert 8-bit grayscale to reversed monochrome for drivers > > that control monochromatic displays, that only have 1 bit per pixel depth. > > > > This helper function was based on repaper_gray8_to_mono_reversed() from > > the drivers/gpu/drm/tiny/repaper.c driver. > > You could convert repaper to the new helper. > > > > > Signed-off-by: Javier Martinez Canillas > > --- > > > > drivers/gpu/drm/drm_format_helper.c | 35 + > > include/drm/drm_format_helper.h | 2 ++ > > 2 files changed, 37 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_format_helper.c > > b/drivers/gpu/drm/drm_format_helper.c > > index 0f28dd2bdd72..bf477c136082 100644 > > --- a/drivers/gpu/drm/drm_format_helper.c > > +++ b/drivers/gpu/drm/drm_format_helper.c > > @@ -584,3 +584,38 @@ int drm_fb_blit_toio(void __iomem *dst, unsigned int > > dst_pitch, uint32_t dst_for > > return -EINVAL; > > } > > EXPORT_SYMBOL(drm_fb_blit_toio); > > + > > +/** > > + * drm_fb_gray8_to_mono_reversed - Convert grayscale to reversed monochrome > > + * @dst: reversed monochrome destination buffer > > + * @src: 8-bit grayscale source buffer > > + * @clip: Clip rectangle area to copy > > + * > > + * DRM doesn't have native monochrome or grayscale support. > > + * Such drivers can announce the commonly supported XR24 format to > > userspace > > + * and use drm_fb_xrgb_to_gray8() to convert to grayscale and then this > > + * helper function to convert to the native format. > > + */ > > +void drm_fb_gray8_to_mono_reversed(void *dst, void *src, const struct > > drm_rect *clip) > > IMHO it would be better to have a function that converts xrgb to > mono and let it handle the intermediate step of gray8. > > > +{ > > + size_t width = drm_rect_width(clip); > > + size_t height = drm_rect_width(clip); > > + > > + u8 *mono = dst, *gray8 = src; > > + unsigned int y, xb, i; > > + > > + for (y = 0; y < height; y++) > > + for (xb = 0; xb < width / 8; xb++) { > > The inner loop should probably go into a separate helper function. See > the drm_fb_*_line() helpers in this file > > At some point, we's want to have a single blit helper that takes source > and destination formats/buffers. It would then pick the correct per-line > helper for the conversion. So yeah, we'd want something composable. Btw. VKMS is going to do blending, and it needs to support various formats, so the latest patches from Igor probably do something similar. Thanks, pq > > Best regards > Thomas > > > + u8 byte = 0x00; > > + > > + for (i = 0; i < 8; i++) { > > + int x = xb * 8 + i; > > + > > + byte >>= 1; > > + if (gray8[y * width + x] >> 7) > > + byte |= BIT(7); > > + } > > + *mono++ = byte; > > + } > > +} > > +EXPORT_SYMBOL(drm_fb_gray8_to_mono_reversed); pgpGyVptUv0Lw.pgp Description: OpenPGP digital signature
Re: [PULL] drm-misc-next
Hi Am 01.02.22 um 09:17 schrieb Maarten Lankhorst: Op 01-02-2022 om 07:38 schreef Dave Airlie: On Thu, 27 Jan 2022 at 21:57, Maarten Lankhorst wrote: Hi Dave & Daniel, First pull for v5.18 I was trying to be all efficient and get this pulled in time for once. However it broke building on my arm test build. The new DP helper Kconfig is wrong somewhere. I've attached the .config, but it appears I get DRM_DP_HELPER set to M but ANALOGIX_DP set to Y and it fails to link because the dp helper should be Y. Regards, Dave. Below should likely fix it? diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig index 6a251e3aa779..f27cfd2a9726 100644 --- a/drivers/gpu/drm/exynos/Kconfig +++ b/drivers/gpu/drm/exynos/Kconfig @@ -66,6 +66,7 @@ config DRM_EXYNOS_DP bool "Exynos specific extensions for Analogix DP driver" depends on DRM_EXYNOS_FIMD || DRM_EXYNOS7_DECON select DRM_ANALOGIX_DP + select DRM_DP_HELPER default DRM_EXYNOS select DRM_PANEL help diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig index d59dca5efb52..fa5cfda4e90e 100644 --- a/drivers/gpu/drm/rockchip/Kconfig +++ b/drivers/gpu/drm/rockchip/Kconfig @@ -8,6 +8,7 @@ config DRM_ROCKCHIP select DRM_PANEL select VIDEOMODE_HELPERS select DRM_ANALOGIX_DP if ROCKCHIP_ANALOGIX_DP + select DRM_DP_HELPER if ROCKCHIP_ANALOGIX_DP select DRM_DW_HDMI if ROCKCHIP_DW_HDMI select DRM_DW_MIPI_DSI if ROCKCHIP_DW_MIPI_DSI select GENERIC_PHY if ROCKCHIP_DW_MIPI_DSI Thanks a lot. This fixes the build for me. Best regards Thomas -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
[Bug 215558] New: amdgpu driver sometimes crashes when playing games
https://bugzilla.kernel.org/show_bug.cgi?id=215558 Bug ID: 215558 Summary: amdgpu driver sometimes crashes when playing games Product: Drivers Version: 2.5 Kernel Version: 5.15 Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: smoe...@uni-bremen.de Regression: No Created attachment 300371 --> https://bugzilla.kernel.org/attachment.cgi?id=300371&action=edit logs and pictures of screen output Sometimes, when I play games like Counter Strike - Global Offensive (Native, Steam), or League of Legends (via WINE), my system crashes, resulting in distorted colour output, but the mouse cursor still moves, but I cannot interact with any GUI-component or switch to TTY. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 215558] amdgpu driver sometimes crashes when playing games
https://bugzilla.kernel.org/show_bug.cgi?id=215558 smoe...@uni-bremen.de changed: What|Removed |Added Component|Video(DRI - non Intel) |Video(Other) -- 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 4/4] MAINTAINERS: Add entry for Solomon SSD1307 OLED displays DRM driver
Hello Andy, On 2/1/22 10:32, Andy Shevchenko wrote: > On Mon, Jan 31, 2022 at 09:15:37PM +0100, Javier Martinez Canillas wrote: >> To make sure that tools like the get_maintainer.pl script will suggest >> to Cc me if patches are posted for this driver. >> >> Also include the Device Tree binding for the old ssd1307fb fbdev driver >> since the new DRM driver was made compatible with the existing binding. > > Dunno why you have patches 3 and 4 missed references (in terms of email > thread). > Yeah, I use the patman tool [0] to post patches and something went wrong after sending the first patch and I had to manually post the others with git-send-email. I could had used --in-reply-to, but didn't feel like it. [0]: https://gitlab.com/u-boot/u-boot/tree/master/tools/patman >> +DRM DRIVER FOR SOLOMON SSD1307 OLED DISPLAYS >> +M: Javier Martinez Canillas >> +S: Maintained >> +T: git git://anongit.freedesktop.org/drm/drm-misc >> +F: Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml >> +F: drivers/gpu/drm/tiny/ssd1307.c > > I think it makes sense to add ssd1307fb as well. At least you may point out > people patching old driver about new one until it's gone completely. > That's a good idea. I also found some issues in the ssd1307fb driver when doing the port, so I could dig more to fix them and propose myself as a co-maintainer for the fbdev driver. But I'll do that as a separate patch-set. Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
On 2/1/22 10:37, Andy Shevchenko wrote: > On Mon, Jan 31, 2022 at 09:56:23PM +0100, Sam Ravnborg wrote: >> On Mon, Jan 31, 2022 at 09:12:20PM +0100, Javier Martinez Canillas wrote: > > ... > >>> Patch #3 adds the driver. The name ssd1307 was used instead of ssd130x >>> (which would be more accurate) to avoid confusion for users who want to >>> migrate from the existing ssd1307fb fbdev driver. >> Looking forward the name ssd130x would make more sense. There is only so >> many existing users and a potential of much more new users. >> So in my color of the world the naming that benefits the most users >> wins. > > It depends if the binding is going to be preserved. Also this series doesn't > answer to the question what to do with the old driver. > I don't plan to remove the old driver (yet). My goal here is to have an answer for Fedora users that might complain that we disabled all the fbdev drivers. So I wanted to understand the effort involved in porting a fbdev driver to DRM. > If you leave it, I would expect the backward compatibility, otherwise the > series misses removal of the old driver. > I don't see how those two are correlated. You just need different compatible strings to match the new and old drivers. That what was usually done for DRM drivers that were ported. To give an example, the "omapfb" vs "omapdrm". Since the current binding has a compatible "ssd1305fb-i2c", we could make the new one "ssd1305drm-i2c" or better, just "ssd1305-i2c". Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH -next 1/2] video: fbdev: pxa168fb: Remove unnecessary print function dev_err()
On 2/1/22 07:26, Yang Li wrote: > The print function dev_err() is redundant because platform_get_irq() > already prints an error. > > Eliminate the follow coccicheck warning: > ./drivers/video/fbdev/pxa168fb.c:621:2-9: line 621 is redundant because > platform_get_irq() already prints an error > > Reported-by: Abaci Robot > Signed-off-by: Yang Li > --- > drivers/video/fbdev/pxa168fb.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/video/fbdev/pxa168fb.c b/drivers/video/fbdev/pxa168fb.c > index c25739f6934d..d533c0bf7031 100644 > --- a/drivers/video/fbdev/pxa168fb.c > +++ b/drivers/video/fbdev/pxa168fb.c > @@ -618,7 +618,6 @@ static int pxa168fb_probe(struct platform_device *pdev) > > irq = platform_get_irq(pdev, 0); > if (irq < 0) { > - dev_err(&pdev->dev, "no IRQ defined\n"); > return -ENOENT; > } Please drop the surrounding braces then as well in both of your patches, e.g. if (irq < 0) return -ENOENT; Helge
Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
Hi Javier, On Tue, Feb 1, 2022 at 12:31 PM Javier Martinez Canillas wrote: > On 2/1/22 10:37, Andy Shevchenko wrote: > > On Mon, Jan 31, 2022 at 09:56:23PM +0100, Sam Ravnborg wrote: > >> On Mon, Jan 31, 2022 at 09:12:20PM +0100, Javier Martinez Canillas wrote: > > > > ... > > > >>> Patch #3 adds the driver. The name ssd1307 was used instead of ssd130x > >>> (which would be more accurate) to avoid confusion for users who want to > >>> migrate from the existing ssd1307fb fbdev driver. > >> Looking forward the name ssd130x would make more sense. There is only so > >> many existing users and a potential of much more new users. > >> So in my color of the world the naming that benefits the most users > >> wins. > > > > It depends if the binding is going to be preserved. Also this series doesn't > > answer to the question what to do with the old driver. > > > > I don't plan to remove the old driver (yet). My goal here is to have an answer > for Fedora users that might complain that we disabled all the fbdev drivers. > > So I wanted to understand the effort involved in porting a fbdev driver to > DRM. > > > If you leave it, I would expect the backward compatibility, otherwise the > > series misses removal of the old driver. > > > > I don't see how those two are correlated. You just need different compatible > strings to match the new and old drivers. That what was usually done for DRM > drivers that were ported. To give an example, the "omapfb" vs "omapdrm". > > Since the current binding has a compatible "ssd1305fb-i2c", we could make the > new one "ssd1305drm-i2c" or better, just "ssd1305-i2c". DT describes hardware, not software policy. If the hardware is the same, the DT bindings should stay the same. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH 3/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
On 2/1/22 10:41, Andy Shevchenko wrote: > On Mon, Jan 31, 2022 at 10:30:46PM +0100, Sam Ravnborg wrote: >> On Mon, Jan 31, 2022 at 09:29:16PM +0100, Javier Martinez Canillas wrote: > > ... > >>> +config TINYDRM_SSD1307 >>> + tristate "DRM support for Solomon SSD1307 OLED displays" >> Use SSD130X here - so SSD1306 users can find it. > > It's better to list them all in the "help". How user would grep this? > > `git grep -n -i ssd1306` ==> no match. > That's already the case :) $ git grep -n -i ssd1306 drivers/gpu/drm/ drivers/gpu/drm/tiny/Kconfig:167: DRM driver for the SSD1305, SSD1306, SSD1307 and SSD1309 Solomon drivers/gpu/drm/tiny/ssd1307.c:922:static struct ssd1307_deviceinfo ssd1307_ssd1306_deviceinfo = { drivers/gpu/drm/tiny/ssd1307.c:948: .compatible = "solomon,ssd1306fb-i2c", drivers/gpu/drm/tiny/ssd1307.c:949: .data = (void *)&ssd1307_ssd1306_deviceinfo, Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH 3/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
On 2/1/22 10:44, Andy Shevchenko wrote: > On Tue, Feb 01, 2022 at 01:14:22AM +0100, Javier Martinez Canillas wrote: >> On 1/31/22 22:30, Sam Ravnborg wrote: >>> On Mon, Jan 31, 2022 at 09:29:16PM +0100, Javier Martinez Canillas wrote: > > ... > >>> The driver uses the pwms property for the backlight. >>> It would have been much better to bite the bullet and require the >>> backlight to be specified using a backlight node in the DT. >>> This is the standard way to do it and then the driver could use the >>> existing backlight driver rather than embedding a backlight driver here. >>> >> >> I did consider that. Because that would allow me to use a struct drm_panel >> and as you said make the core to manage the backlight. But then decied to >> just keep the backward compatibility with the existing binding to make it >> easier for users to migrate to the DRM driver. >> >> I wonder if we could make the driver to support both ? That is, to query >> if there's a backlight with drm_panel_of_backlight() and if not found then >> registering it's own backlight like the driver is currently doing. > > If we keep 100% backward compatibility, just drop the old driver. > After all module name is not so important as compatibility strings. > As mentioned I don't believe those two things are related. You could make it backward compatible but still keep the old driver around until no one else would care about it. This DRM driver is brand new and there may be bugs, performance issues and whatnot. At least I won't propose to remove the old fbdev driver but would want people using DRM to have a choice. > The problem with no backward compatibility means that removal of old driver > makes users unhappy since DT is kinda ABI and we do not break it. > I think that's the crux of the issue. Do we want people to update their kernel but using their existing Device Tree and be able to switch to the DRM driver ? My take is that we should and that's why I kept the backward compatibility. Maybe we could do that in the meantime and at some point introduce new DT bindings (with a different compatible string) that would use the latest and greatest conventions in DT ? That seems to be a good compromise. Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH 2/4] drm/format-helper: Add drm_fb_gray8_to_mono_reversed()
Hello Thomas, Thanks a lot for your feedback. On 2/1/22 10:59, Thomas Zimmermann wrote: > Hi > > Am 31.01.22 um 21:12 schrieb Javier Martinez Canillas: >> Add support to convert 8-bit grayscale to reversed monochrome for drivers >> that control monochromatic displays, that only have 1 bit per pixel depth. >> >> This helper function was based on repaper_gray8_to_mono_reversed() from >> the drivers/gpu/drm/tiny/repaper.c driver. > > You could convert repaper to the new helper. > Yes, I plan to do it but was out of scope for this patch series. >> >> Signed-off-by: Javier Martinez Canillas >> --- >> >> drivers/gpu/drm/drm_format_helper.c | 35 + >> include/drm/drm_format_helper.h | 2 ++ >> 2 files changed, 37 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_format_helper.c >> b/drivers/gpu/drm/drm_format_helper.c >> index 0f28dd2bdd72..bf477c136082 100644 >> --- a/drivers/gpu/drm/drm_format_helper.c >> +++ b/drivers/gpu/drm/drm_format_helper.c >> @@ -584,3 +584,38 @@ int drm_fb_blit_toio(void __iomem *dst, unsigned int >> dst_pitch, uint32_t dst_for >> return -EINVAL; >> } >> EXPORT_SYMBOL(drm_fb_blit_toio); >> + >> +/** >> + * drm_fb_gray8_to_mono_reversed - Convert grayscale to reversed monochrome >> + * @dst: reversed monochrome destination buffer >> + * @src: 8-bit grayscale source buffer >> + * @clip: Clip rectangle area to copy >> + * >> + * DRM doesn't have native monochrome or grayscale support. >> + * Such drivers can announce the commonly supported XR24 format to userspace >> + * and use drm_fb_xrgb_to_gray8() to convert to grayscale and then this >> + * helper function to convert to the native format. >> + */ >> +void drm_fb_gray8_to_mono_reversed(void *dst, void *src, const struct >> drm_rect *clip) > > IMHO it would be better to have a function that converts xrgb to > mono and let it handle the intermediate step of gray8. > That's a good idea. I'll add that too. >> +{ >> +size_t width = drm_rect_width(clip); >> +size_t height = drm_rect_width(clip); >> + >> +u8 *mono = dst, *gray8 = src; >> +unsigned int y, xb, i; >> + >> +for (y = 0; y < height; y++) >> +for (xb = 0; xb < width / 8; xb++) { > > The inner loop should probably go into a separate helper function. See > the drm_fb_*_line() helpers in this file > > At some point, we's want to have a single blit helper that takes source > and destination formats/buffers. It would then pick the correct per-line > helper for the conversion. So yeah, we'd want something composable. > Agreed. I'll split that into a separate helper function. Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat
[PATCH v2] drm/fb-helper: Mark screen buffers in system memory with FB_VIRTFB
Mark screen buffers in system memory with FB_VIRTFB. Otherwise, fbdev deferred I/O marks mmap'ed areas of system memory with VM_IO. (There's an inverse relation ship between the two flags.) For shadow buffers, also set the FB_READS_FAST hint. v2: * updated commit description (Daniel) * added Fixes tag Signed-off-by: Thomas Zimmermann Fixes: d536540f304c ("drm/fb-helper: Add generic fbdev emulation .fb_probe function") Reviewed-by: Daniel Vetter Cc: dri-devel@lists.freedesktop.org Cc: # v4.19+ --- drivers/gpu/drm/drm_fb_helper.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index ed43b987d306..f15127a32f7a 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -2346,6 +2346,7 @@ static int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper, fbi->fbops = &drm_fbdev_fb_ops; fbi->screen_size = sizes->surface_height * fb->pitches[0]; fbi->fix.smem_len = fbi->screen_size; + fbi->flags = FBINFO_DEFAULT; drm_fb_helper_fill_info(fbi, fb_helper, sizes); @@ -2353,19 +2354,21 @@ static int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper, fbi->screen_buffer = vzalloc(fbi->screen_size); if (!fbi->screen_buffer) return -ENOMEM; + fbi->flags |= FBINFO_VIRTFB | FBINFO_READS_FAST; fbi->fbdefio = &drm_fbdev_defio; - fb_deferred_io_init(fbi); } else { /* buffer is mapped for HW framebuffer */ ret = drm_client_buffer_vmap(fb_helper->buffer, &map); if (ret) return ret; - if (map.is_iomem) + if (map.is_iomem) { fbi->screen_base = map.vaddr_iomem; - else + } else { fbi->screen_buffer = map.vaddr; + fbi->flags |= FBINFO_VIRTFB; + } /* * Shamelessly leak the physical address to user-space. As -- 2.34.1
[PATCH -next 1/2 v2] video: fbdev: pxa168fb: Remove unnecessary print function dev_err()
The print function dev_err() is redundant because platform_get_irq() already prints an error. Eliminate the follow coccicheck warning: ./drivers/video/fbdev/pxa168fb.c:621:2-9: line 621 is redundant because platform_get_irq() already prints an error Reported-by: Abaci Robot Signed-off-by: Yang Li --- -Change in v2: drop the surrounding braces drivers/video/fbdev/pxa168fb.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/video/fbdev/pxa168fb.c b/drivers/video/fbdev/pxa168fb.c index c25739f6934d..e943300d23e8 100644 --- a/drivers/video/fbdev/pxa168fb.c +++ b/drivers/video/fbdev/pxa168fb.c @@ -617,10 +617,8 @@ static int pxa168fb_probe(struct platform_device *pdev) } irq = platform_get_irq(pdev, 0); - if (irq < 0) { - dev_err(&pdev->dev, "no IRQ defined\n"); + if (irq < 0) return -ENOENT; - } info = framebuffer_alloc(sizeof(struct pxa168fb_info), &pdev->dev); if (info == NULL) { -- 2.20.1.7.g153144c
[PATCH -next 2/2 v2] video: fbdev: pxa3xx-gcu: Remove unnecessary print function dev_err()
The print function dev_err() is redundant because platform_get_irq() already prints an error. Eliminate the follow coccicheck warning: ./drivers/video/fbdev/pxa3xx-gcu.c:615:2-9: line 615 is redundant because platform_get_irq() already prints an error Reported-by: Abaci Robot Signed-off-by: Yang Li --- -Change in v2: drop the surrounding braces drivers/video/fbdev/pxa3xx-gcu.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/video/fbdev/pxa3xx-gcu.c b/drivers/video/fbdev/pxa3xx-gcu.c index 9239ecd34169..350b3139c863 100644 --- a/drivers/video/fbdev/pxa3xx-gcu.c +++ b/drivers/video/fbdev/pxa3xx-gcu.c @@ -611,10 +611,8 @@ static int pxa3xx_gcu_probe(struct platform_device *pdev) /* request the IRQ */ irq = platform_get_irq(pdev, 0); - if (irq < 0) { - dev_err(dev, "no IRQ defined: %d\n", irq); + if (irq < 0) return irq; - } ret = devm_request_irq(dev, irq, pxa3xx_gcu_handle_irq, 0, DRV_NAME, priv); -- 2.20.1.7.g153144c
Re: [PATCH 1/4] drm: Add I2C connector type
Den 31.01.2022 21.52, skrev Sam Ravnborg: > On Mon, Jan 31, 2022 at 09:12:21PM +0100, Javier Martinez Canillas wrote: >> There isn't a connector type for display controllers accesed through I2C, >> most drivers use DRM_MODE_CONNECTOR_Unknown or DRM_MODE_CONNECTOR_VIRTUAL. >> >> Add an I2C connector type to match the actual connector. >> >> As Noralf Trønnes mentions in commit fc06bf1d76d6 ("drm: Add SPI connector >> type"), user-space should be able to cope with a connector type that does >> not yet understand. >> It turned out that I wasn't entirely correct here, mpv didn't cope with unknown types. In the PR to add support Emil Velikov wondered if libdrm should handle these connector names: https://github.com/mpv-player/mpv/pull/8989#issuecomment-879187711 >> Tested with `modetest -M ssd1307 -c` and shows the connector as unknown-1. > I had expected unknown-21?? > >> >> Signed-off-by: Javier Martinez Canillas > Reviewed-by: Sam Ravnborg Sam, didn't you and Laurent discuss adding DRM_MODE_CONNECTOR_PANEL for such a use case? If someone adds parallel bus support to the MIPI DBI helper, there will be one more connector type (I wonder what that one will be called). Noralf. >> --- >> >> drivers/gpu/drm/drm_connector.c | 1 + >> include/uapi/drm/drm_mode.h | 1 + >> 2 files changed, 2 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_connector.c >> b/drivers/gpu/drm/drm_connector.c >> index a50c82bc2b2f..975a7525a508 100644 >> --- a/drivers/gpu/drm/drm_connector.c >> +++ b/drivers/gpu/drm/drm_connector.c >> @@ -105,6 +105,7 @@ static struct drm_conn_prop_enum_list >> drm_connector_enum_list[] = { >> { DRM_MODE_CONNECTOR_WRITEBACK, "Writeback" }, >> { DRM_MODE_CONNECTOR_SPI, "SPI" }, >> { DRM_MODE_CONNECTOR_USB, "USB" }, >> +{ DRM_MODE_CONNECTOR_I2C, "I2C" }, >> }; >> >> void drm_connector_ida_init(void) >> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h >> index e1e351682872..d6d6288242db 100644 >> --- a/include/uapi/drm/drm_mode.h >> +++ b/include/uapi/drm/drm_mode.h >> @@ -421,6 +421,7 @@ enum drm_mode_subconnector { >> #define DRM_MODE_CONNECTOR_WRITEBACK18 >> #define DRM_MODE_CONNECTOR_SPI 19 >> #define DRM_MODE_CONNECTOR_USB 20 >> +#define DRM_MODE_CONNECTOR_I2C 21 >> >> /** >> * struct drm_mode_get_connector - Get connector metadata. >> -- >> 2.34.1
Re: [PATCH 3/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
On 2/1/22 10:33, Thomas Zimmermann wrote: > Hi Javier, > > please see comments and questions below. > Thanks again for your comments. [snip] >> >> +config TINYDRM_SSD1307 > > TINYDRM is so 2010's. Just call it DRM. > Haha, Ok. I just followed what other drivers did and thought was a convention. > Some of the drivers use TINY for historical reasons. Simple pipe and > mipi helpers originated from here IIRC. And now it's just regular DRM > drivers. > >> +tristate "DRM support for Solomon SSD1307 OLED displays" >> +depends on DRM && I2C >> +select DRM_KMS_HELPER >> +select DRM_GEM_SHMEM_HELPER >> +select BACKLIGHT_CLASS_DEVICE > > Alphabetical order please. > Ok. [snip] >> obj-$(CONFIG_TINYDRM_REPAPER) += repaper.o >> +obj-$(CONFIG_TINYDRM_SSD1307) += ssd1307.o > > Maybe call it ssd130x > Yes, that's what I had in my RFC posted yesterday but then Andy asked if it could use the old driver name. But now it seems he also agrees with us that ssd130x would be better. I'll change it again. [snip] >> +}; >> + >> +struct ssd1307_array { >> +u8 type; > > 'cmd'? > Sure. >> +u8 data[]; >> +}; > > The name 'array' is misleading. At first I thought that it's a C array > type. But it's actually an element of the adapter's i2c protocol. Is > there a better name that makes this more obvious? ssd130x_i2c_cmd? > Indeed. I took the name from the old driver but I agree with you that's confusing. It's called array because ssd1307_update_display() writes an array of SSD1307_DATA commands to update all the needed screen pages. > If this common enough that it could be part of some kind of i2c helper > library? > That's a good question. There are some other DRM drivers doing I2C read/writes so there may be some room for consolidation in helper functions. But I would do it as a follow-up since it seems out of scope for this series. >> + >> +static inline struct ssd1307_device *drm_to_ssd1307(struct drm_device *drm) >> +{ >> +return container_of(drm, struct ssd1307_device, drm); >> +} >> + >> +static struct ssd1307_array *ssd1307_alloc_array(u32 len, u8 type) >> +{ >> +struct ssd1307_array *array; >> + >> +array = kzalloc(sizeof(struct ssd1307_array) + len, GFP_KERNEL); > > sizeof(*array) is slighly more reliable. But wasn't there even a macro > for such allocations (struct + length)? > Agree on the sizeof(*foo) and will change to use that instead. Thanks, I leared now about the struct_size() and flex_array_size() macros. >> +if (!array) >> +return NULL; > > Personally I prefer pointer-encoded errors instead of NULL. But I don't > thing there's a strict rule about it. > Usually my rule of thumb is whether is necessary to encode different errno codes. But in this particular case the only possible error is in kzalloc() so I thought that is safe to assume in the caller that NULL is -ENOMEM. >> + >> +array->type = type; >> + >> +return array; >> +} >> + >> +static int ssd1307_write_array(struct i2c_client *client, >> + struct ssd1307_array *array, u32 len) >> +{ >> +int ret; >> + >> +len += sizeof(struct ssd1307_array); > > Same thing about sizeof: sizeof(var) is better than sizeof(type); > Agreed. > Since you're using a separate array structure anyway, wouldn't it make > sense to store > Indeed. [snip] >> +{ >> +u8 col_end = col_start + cols - 1; >> +int ret; >> + >> +if (col_start == ssd1307->col_start && col_end == ssd1307->col_end) >> +return 0; >> + >> +ret = ssd1307_write_cmd(ssd1307->client, SSD1307_SET_COL_RANGE); >> +if (ret < 0) >> +return ret; >> + >> +ret = ssd1307_write_cmd(ssd1307->client, col_start); >> +if (ret < 0) >> +return ret; >> + >> +ret = ssd1307_write_cmd(ssd1307->client, col_end); >> +if (ret < 0) >> +return ret; > > Can you write these cmds in one step, such as setting up an array and > sending it with ssd1307_write_array? > I don't think so because the commands are different. But I'll check the ssd1306 datasheet again to confirma that's the case. [snip] >> + >> +buf = kmalloc_array(width, height, GFP_KERNEL); > > There's kcalloc(). > Indeed, forgot about it. Will use it instead... >> +if (!buf) >> +return; >> + >> +/* Clear screen to black if disabled */ >> +memset(buf, 0, width * height); >> + and then could drop this memset() call. [snip] >> + >> +static int ssd1307_fb_blit_rect(struct drm_framebuffer *fb, const struct >> dma_buf_map *map, >> +struct drm_rect *rect) >> +{ >> +struct ssd1307_device *ssd1307 = drm_to_ssd1307(fb->dev); >> +void *vmap = map->vaddr; /* TODO: Use mapping abstraction properly */ >> +int ret = 0; >> +u8 *buf = NULL; >> + >> +buf = kmalloc_array(fb->width, fb->height, GFP_KERNEL); > > Maybe leave a short comment that these array
Re: [PATCH 1/4] drm: Add I2C connector type
Hello Noralf, On 2/1/22 13:58, Noralf Trønnes wrote: > > > Den 31.01.2022 21.52, skrev Sam Ravnborg: >> On Mon, Jan 31, 2022 at 09:12:21PM +0100, Javier Martinez Canillas wrote: >>> There isn't a connector type for display controllers accesed through I2C, >>> most drivers use DRM_MODE_CONNECTOR_Unknown or DRM_MODE_CONNECTOR_VIRTUAL. >>> >>> Add an I2C connector type to match the actual connector. >>> >>> As Noralf Trønnes mentions in commit fc06bf1d76d6 ("drm: Add SPI connector >>> type"), user-space should be able to cope with a connector type that does >>> not yet understand. >>> > > It turned out that I wasn't entirely correct here, mpv didn't cope with > unknown types. In the PR to add support Emil Velikov wondered if libdrm > should handle these connector names: > https://github.com/mpv-player/mpv/pull/8989#issuecomment-879187711 > I see, thanks for the information. What should we do then, just use the type DRM_MODE_CONNECTOR_Unknown then ? Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
Hello Geert, On 2/1/22 12:38, Geert Uytterhoeven wrote: [snip] >> >> Since the current binding has a compatible "ssd1305fb-i2c", we could make the >> new one "ssd1305drm-i2c" or better, just "ssd1305-i2c". > > DT describes hardware, not software policy. > If the hardware is the same, the DT bindings should stay the same. > Yes I know that but the thing is that the current binding don't describe the hardware correctly. For instance, don't use a backlight DT node as a property of the panel and have this "fb" suffix in the compatible strings. Having said that, my opinion is that we should just keep with the existing bindings and make compatible to that even if isn't completely correct. Since that will ease adoption of the new DRM driver and allow users to use it without the need to update their DTBs. Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH 1/4] drm: Add I2C connector type
Den 01.02.2022 14.06, skrev Javier Martinez Canillas: > Hello Noralf, > > On 2/1/22 13:58, Noralf Trønnes wrote: >> >> >> Den 31.01.2022 21.52, skrev Sam Ravnborg: >>> On Mon, Jan 31, 2022 at 09:12:21PM +0100, Javier Martinez Canillas wrote: There isn't a connector type for display controllers accesed through I2C, most drivers use DRM_MODE_CONNECTOR_Unknown or DRM_MODE_CONNECTOR_VIRTUAL. Add an I2C connector type to match the actual connector. As Noralf Trønnes mentions in commit fc06bf1d76d6 ("drm: Add SPI connector type"), user-space should be able to cope with a connector type that does not yet understand. >> >> It turned out that I wasn't entirely correct here, mpv didn't cope with >> unknown types. In the PR to add support Emil Velikov wondered if libdrm >> should handle these connector names: >> https://github.com/mpv-player/mpv/pull/8989#issuecomment-879187711 >> > > I see, thanks for the information. What should we do then, just use the type > DRM_MODE_CONNECTOR_Unknown then ? > Not really, I just wanted to point out that it could be that not all userspace will handle an unknown connector type (I just checked the DE's at the time). I haven't seen any issues after adding the SPI type so it can't be that many apps that has problems. Adding to that a tiny monochrome display is limited in which applications it will encounter I guess :) It was after adding the USB type that I discovered that mpv didn't work. Noralf.
Re: [PATCH 1/4] drm: Add I2C connector type
On Tuesday, February 1st, 2022 at 13:58, Noralf Trønnes wrote: > It turned out that I wasn't entirely correct here, mpv didn't cope with > unknown types. In the PR to add support Emil Velikov wondered if libdrm > should handle these connector names: Opened this MR to try to make things easier for user-space: https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/222
Re: [PATCH 03/21] fbcon: Restore fbcon scrolling acceleration
On Tue, Feb 1, 2022 at 12:01 PM Helge Deller wrote: > On 2/1/22 11:36, Daniel Vetter wrote: > > On Tue, Feb 1, 2022 at 11:16 AM Helge Deller wrote: > >> > >> On 1/31/22 22:05, Daniel Vetter wrote: > >>> This functionally undoes 39aead8373b3 ("fbcon: Disable accelerated > >>> scrolling"), but behind the FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION > >>> option. > >> > >> you have two trivial copy-n-paste errors in this patch which still prevent > >> the > >> console acceleration. > > > > Duh :-( > > > > But before we dig into details I think the big picture would be > > better. I honestly don't like the #ifdef pile here that much. > > Me neither. > The ifdefs give a better separation, but prevents that the compiler > checks the various paths when building. > > > I wonder whether your approach, also with GETVX/YRES adjusted > > somehow, wouldn't look cleaner? > I think so. > You wouldn't even need to touch GETVX/YRES because the compiler > will optimize/reduce it from > > #define GETVYRES(s,i) ({ \ > (s == SCROLL_REDRAW || s == SCROLL_MOVE) ? \ > (i)->var.yres : (i)->var.yres_virtual; }) > > to just become: > > #define GETVYRES(s,i) ((i)->var.yres) Yeah, but you need to roll out your helper to all the callsites. But since you #ifdef out info->scrollmode we should catch them all I guess. > > Like I said in the cover letter I got mostly distracted with fbcon > > locking last week, not really with this one here at all, so maybe > > going with your 4 (or 2 if we squash them like I did here) patches is > > neater? > > The benefit of my patch series was, that it could be easily backported first, > and then cleaned up afterwards. Even a small additional backport patch to > disable > the fbcon acceleration for DRM in the old releases would be easy. > But I'm not insisting on backporting the patches, if we find good way forward. > > So, either with the 4 (or 2) patches would be OK for me (or even your > approach). The idea behind the squash was that it's then impossible to backport without the Kconfig, and so we'll only enable this code when people intentionally want it. Maybe I'm too paranoid? Anyway, you feel like finishing off your approach? Or should I send out v2 of this with the issues fixed you spotted? Like I said either is fine with me. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 1/4] drm: Add I2C connector type
On 2/1/22 14:20, Noralf Trønnes wrote: > > > Den 01.02.2022 14.06, skrev Javier Martinez Canillas: >> Hello Noralf, >> >> On 2/1/22 13:58, Noralf Trønnes wrote: >>> >>> >>> Den 31.01.2022 21.52, skrev Sam Ravnborg: On Mon, Jan 31, 2022 at 09:12:21PM +0100, Javier Martinez Canillas wrote: > There isn't a connector type for display controllers accesed through I2C, > most drivers use DRM_MODE_CONNECTOR_Unknown or DRM_MODE_CONNECTOR_VIRTUAL. > > Add an I2C connector type to match the actual connector. > > As Noralf Trønnes mentions in commit fc06bf1d76d6 ("drm: Add SPI connector > type"), user-space should be able to cope with a connector type that does > not yet understand. > >>> >>> It turned out that I wasn't entirely correct here, mpv didn't cope with >>> unknown types. In the PR to add support Emil Velikov wondered if libdrm >>> should handle these connector names: >>> https://github.com/mpv-player/mpv/pull/8989#issuecomment-879187711 >>> >> >> I see, thanks for the information. What should we do then, just use the type >> DRM_MODE_CONNECTOR_Unknown then ? >> > > Not really, I just wanted to point out that it could be that not all > userspace will handle an unknown connector type (I just checked the DE's > at the time). I haven't seen any issues after adding the SPI type so it > can't be that many apps that has problems. Adding to that a tiny > monochrome display is limited in which applications it will encounter I > guess :) It was after adding the USB type that I discovered that mpv > didn't work. > Anything we do for this rather obscure hardware certainly won't be an issue for most applications :) But I wasn't sure if your previous comment meant that you were nacking $subject. Glad that we can go ahead and describe the correct type then. Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH 01/21] MAINTAINERS: Add entry for fbdev core
On 1/31/22 22:05, Daniel Vetter wrote: > Ever since Tomi extracted the core code in 2014 it's been defacto me > maintaining this, with help from others from dri-devel and sometimes > Linus (but those are mostly merge conflicts): > > $ git shortlog -ns drivers/video/fbdev/core/ | head -n5 > 35 Daniel Vetter > 23 Linus Torvalds > 10 Hans de Goede > 9 Dave Airlie > 6 Peter Rosin > > I think ideally we'd also record that the various firmware fb drivers > (efifb, vesafb, ...) are also maintained in drm-misc because for the > past few years the patches have either been to fix handover issues > with drm drivers, or caused handover issues with drm drivers. So any > other tree just doesn't make sense. But also, there's plenty of > outdated MAINTAINER entries for these with people and git trees that > haven't been active in years, so maybe let's just leave them alone. > And furthermore distros are now adopting simpledrm as the firmware fb > driver, so hopefully the need to care about the fbdev firmware drivers > will go down going forward. > > Note that drm-misc is group maintained, I expect that to continue like > we've done before, so no new expectations that patches all go through > my hands. That would be silly. This also means I'm happy to put any > other volunteer's name in the M: line, but otherwise git log says I'm > the one who's stuck with this. > > Cc: Dave Airlie > Cc: Jani Nikula > Cc: Linus Torvalds > Cc: Linux Fbdev development list > Cc: Pavel Machek > Cc: Sam Ravnborg > Cc: Greg Kroah-Hartman > Cc: Javier Martinez Canillas > Cc: DRI Development > Cc: Linux Kernel Mailing List > Cc: Claudio Suarez > Cc: Tomi Valkeinen > Cc: Geert Uytterhoeven > Cc: Thomas Zimmermann > Cc: Daniel Vetter > Cc: Sven Schnelle > Cc: Gerd Hoffmann > Signed-off-by: Daniel Vetter > --- Reviewed-by: Javier Martinez Canillas Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH 3/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
On Tue, Feb 01, 2022 at 12:45:53PM +0100, Javier Martinez Canillas wrote: > On 2/1/22 10:44, Andy Shevchenko wrote: > > On Tue, Feb 01, 2022 at 01:14:22AM +0100, Javier Martinez Canillas wrote: ... > > The problem with no backward compatibility means that removal of old driver > > makes users unhappy since DT is kinda ABI and we do not break it. > > > > I think that's the crux of the issue. Do we want people to update their > kernel but using their existing Device Tree and be able to switch to the > DRM driver ? > > My take is that we should and that's why I kept the backward compatibility. > > Maybe we could do that in the meantime and at some point introduce new DT > bindings (with a different compatible string) that would use the latest > and greatest conventions in DT ? That seems to be a good compromise. I have over-read in this discussion that current binding is not fully correct from hw perspective. If it's indeed the case (and I believe it's), then probably we should come with brand new driver with ssd130x name and incompatible bindingas (*). Otherwise in this driver we continue to be incorrect in them. *) But even though I think it would be good if you take the old one under your maintainership. -- With Best Regards, Andy Shevchenko
Re: [PATCH 3/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
Hi Javier, On Tue, Feb 1, 2022 at 2:02 PM Javier Martinez Canillas wrote: > On 2/1/22 10:33, Thomas Zimmermann wrote: > >> +{ > >> +u8 col_end = col_start + cols - 1; > >> +int ret; > >> + > >> +if (col_start == ssd1307->col_start && col_end == ssd1307->col_end) > >> +return 0; > >> + > >> +ret = ssd1307_write_cmd(ssd1307->client, SSD1307_SET_COL_RANGE); > >> +if (ret < 0) > >> +return ret; > >> + > >> +ret = ssd1307_write_cmd(ssd1307->client, col_start); > >> +if (ret < 0) > >> +return ret; > >> + > >> +ret = ssd1307_write_cmd(ssd1307->client, col_end); > >> +if (ret < 0) > >> +return ret; > > > > Can you write these cmds in one step, such as setting up an array and > > sending it with ssd1307_write_array? > > I don't think so because the commands are different. But I'll check the > ssd1306 datasheet again to confirma that's the case. IIRC, I tried that while working on the optimizations for ssd1307fb, and it didn't work. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
Hi Javier, On Tue, Feb 1, 2022 at 2:09 PM Javier Martinez Canillas wrote: > On 2/1/22 12:38, Geert Uytterhoeven wrote: > >> Since the current binding has a compatible "ssd1305fb-i2c", we could make > >> the > >> new one "ssd1305drm-i2c" or better, just "ssd1305-i2c". > > > > DT describes hardware, not software policy. > > If the hardware is the same, the DT bindings should stay the same. > > > > Yes I know that but the thing is that the current binding don't describe > the hardware correctly. For instance, don't use a backlight DT node as a > property of the panel and have this "fb" suffix in the compatible strings. > > Having said that, my opinion is that we should just keep with the existing > bindings and make compatible to that even if isn't completely correct. > > Since that will ease adoption of the new DRM driver and allow users to use > it without the need to update their DTBs. To me it looks like the pwms property is not related to the backlight at all, and only needed for some variants? And the actual backlight code seems to be about internal contrast adjustment? So if the pwms usage is OK, what other reasons are there to break DT compatibility? IMHO just the "fb" suffix is not a good reason. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH 1/4] drm: Add I2C connector type
Den 01.02.2022 14.38, skrev Simon Ser: > > On Tuesday, February 1st, 2022 at 13:58, Noralf Trønnes > wrote: > >> It turned out that I wasn't entirely correct here, mpv didn't cope with >> unknown types. In the PR to add support Emil Velikov wondered if libdrm >> should handle these connector names: > > Opened this MR to try to make things easier for user-space: > https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/222 Thanks Simon!