Re: [PATCH v5 1/2] drm/bridge: sil_sii8620: make remote control optional.
Hi Andrzej, On Mon, Apr 15, 2019 at 10:58:09AM +0200, Andrzej Hajda wrote: > On 15.04.2019 10:12, Ronald Tschalär wrote: > > commit d6abe6df706c (drm/bridge: sil_sii8620: do not have a dependency > > of RC_CORE) changed the driver to select both RC_CORE and INPUT. > > However, this causes problems with other drivers, in particular an input > > driver that depends on MFD_INTEL_LPSS_PCI (to be added in a separate > > commit): > > > > drivers/clk/Kconfig:9:error: recursive dependency detected! > > drivers/clk/Kconfig:9:symbol COMMON_CLK is selected by > > MFD_INTEL_LPSS > > drivers/mfd/Kconfig:566: symbol MFD_INTEL_LPSS is selected by > > MFD_INTEL_LPSS_PCI > > drivers/mfd/Kconfig:580: symbol MFD_INTEL_LPSS_PCI is implied by > > KEYBOARD_APPLESPI > > drivers/input/keyboard/Kconfig:73:symbol KEYBOARD_APPLESPI depends on > > INPUT > > drivers/input/Kconfig:8: symbol INPUT is selected by DRM_SIL_SII8620 > > drivers/gpu/drm/bridge/Kconfig:83:symbol DRM_SIL_SII8620 depends on > > DRM_BRIDGE > > drivers/gpu/drm/bridge/Kconfig:1: symbol DRM_BRIDGE is selected by > > DRM_PL111 > > drivers/gpu/drm/pl111/Kconfig:1: symbol DRM_PL111 depends on > > COMMON_CLK > > > > According to the docs and general consensus, select should only be used > > for non user-visible symbols, but both RC_CORE and INPUT are > > user-visible. Furthermore almost all other references to INPUT > > throughout the kernel config are depends, not selects. For this reason > > the first part of this change reverts commit d6abe6df706c. > > > > In order to address the original reason for commit d6abe6df706c, namely > > that not all boards use the remote controller functionality and hence > > should not need have to deal with RC_CORE, the second part of this > > change now makes the remote control support in the driver optional and > > contingent on RC_CORE being defined. And with this the hard dependency > > on INPUT also goes away as that is only needed if RC_CORE is defined > > (which in turn already depends on INPUT). > > > > CC: Inki Dae > > CC: Andrzej Hajda > > CC: Laurent Pinchart > > CC: Dmitry Torokhov > > Signed-off-by: Ronald Tschalär > > > Reviewed-by: Andrzej Hajda Thanks for your reviews! > If there are no objections I will take it to drm-misc tomorrow. This brings us back to the discussion started in response to the first version of my patch (see https://lore.kernel.org/lkml/20190124082423.23139-1-ron...@innovation.ch/T/#m24f45fecd987a787a9554c8088f463fd10de2b00). To recap: the problem is that the applespi patch depends on this patch here, as make-config will break as described above otherwise. So if this patch is submitted through drm-misc, then it's unclear to me how to ensure that the two patches make it upstream in proper order, unless the applespi patch is also upstreamed through drm-misc, or the Kconfig for applespi is (temporarily) modified to not trigger the config error and another patch is later submitted to fix the Kconfig again (which seems somewhat ugly to me). Assuming that consensus is to merge both patches through one tree, then it would seem that because this patch here is relatively small that maybe it could be merged through the input tree along with the applespi patch? Cheers, Ronald ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/2] drm/ttm: Reset ttm_mem_global when initialized
Certain attributes such as num_zones and kobj in the
ttm_mem_global structure were never reset after calling
ttm_mem_global_release(). Consequently, when multiple GPU drivers
are loaded, and the first one fails to load its firmware, the second
driver will attempt to load its own firmware. Initializing the
second driver invokes ttm_mem_global_init where ttm_mem_global.num_zones
is eventually incremented beyond TTM_MEM_MAX_ZONES.
ttm_mem_global.num_zones is then used to dereference a ttm_mem_zone beyond
the amount of ttm_mem_zones allocated, resulting in a crash.
Zero initialize the ttm_mem_global structure such that its state will
be pristine and thus, ready to be re-initialized.
Signed-off-by: Brian Yip
---
drivers/gpu/drm/ttm/ttm_memory.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
index 699fed9e08ee..73b461d5a4c0 100644
--- a/drivers/gpu/drm/ttm/ttm_memory.c
+++ b/drivers/gpu/drm/ttm/ttm_memory.c
@@ -419,6 +419,8 @@ int ttm_mem_global_init(struct ttm_mem_global *glob)
int i;
struct ttm_mem_zone *zone;
+ memset(glob, 0, sizeof(struct ttm_mem_global));
+
spin_lock_init(&glob->lock);
glob->swap_queue = create_singlethread_workqueue("ttm_swap");
INIT_WORK(&glob->work, ttm_shrink_work);
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] video: fbdev: atmel_lcdfb: add COMPILE_TEST support
On 12/04/2019 13:29:11+0200, Bartlomiej Zolnierkiewicz wrote: > Add COMPILE_TEST support to atmel_lcdfb driver for better compile > testing coverage. > > Signed-off-by: Bartlomiej Zolnierkiewicz Acked-by: Alexandre Belloni > --- > drivers/video/fbdev/Kconfig |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > Index: b/drivers/video/fbdev/Kconfig > === > --- a/drivers/video/fbdev/Kconfig 2019-04-12 13:12:24.0 +0200 > +++ b/drivers/video/fbdev/Kconfig 2019-04-12 13:15:03.986235226 +0200 > @@ -859,7 +859,7 @@ config FB_S1D13XXX > > config FB_ATMEL > tristate "AT91 LCD Controller support" > - depends on FB && OF && HAVE_FB_ATMEL > + depends on FB && OF && (HAVE_FB_ATMEL || COMPILE_TEST) > select FB_BACKLIGHT > select FB_CFB_FILLRECT > select FB_CFB_COPYAREA -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] include: linux: Regularise the use of FIELD_SIZEOF macro
Hi, On Mon, Apr 15, 2019 at 11:13 AM Alexei Starovoitov wrote: > > On Sun, Apr 14, 2019 at 2:15 AM Shyam Saini > wrote: > > > > Currently, there are 3 different macros, namely sizeof_field, SIZEOF_FIELD > > and FIELD_SIZEOF which are used to calculate the size of a member of > > structure, so to bring uniformity in entire kernel source tree lets use > > FIELD_SIZEOF and replace all occurrences of other two macros with this. > > > > For this purpose, redefine FIELD_SIZEOF in include/linux/stddef.h and > > tools/testing/selftests/bpf/bpf_util.h and remove its defination from > > include/linux/kernel.h > > > > Signed-off-by: Shyam Saini > > --- > > arch/arm64/include/asm/processor.h | 10 +- > > arch/mips/cavium-octeon/executive/cvmx-bootmem.c | 2 +- > > drivers/gpu/drm/i915/gvt/scheduler.c | 2 +- > > drivers/net/ethernet/mellanox/mlxsw/spectrum_fid.c | 4 ++-- > > fs/befs/linuxvfs.c | 2 +- > > fs/ext2/super.c| 2 +- > > fs/ext4/super.c| 2 +- > > fs/freevxfs/vxfs_super.c | 2 +- > > fs/orangefs/super.c| 2 +- > > fs/ufs/super.c | 2 +- > > include/linux/kernel.h | 9 - > > include/linux/slab.h | 2 +- > > include/linux/stddef.h | 11 ++- > > kernel/fork.c | 2 +- > > kernel/utsname.c | 2 +- > > net/caif/caif_socket.c | 2 +- > > net/core/skbuff.c | 2 +- > > net/ipv4/raw.c | 2 +- > > net/ipv6/raw.c | 2 +- > > net/sctp/socket.c | 4 ++-- > > tools/testing/selftests/bpf/bpf_util.h | 11 ++- > > tools/ directory is for user space pieces that don't include kernel's include. > I bet your pathes break the user space builds. I think it shouldn't because I haven't included any kernel header in tool/ files, instead I have introduced definition of macro in tool/ , so this patch doesn't create any dependency on kernel headers. Thanks a lot, Shyam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
On Sat 2019-04-13 09:31:27, Alastair D'Silva wrote: > > -Original Message- > > From: Petr Mladek > > Sent: Saturday, 13 April 2019 12:12 AM > > To: Alastair D'Silva > > Cc: alast...@d-silva.org; Jani Nikula ; > Joonas > > Lahtinen ; Rodrigo Vivi > > ; David Airlie ; Daniel Vetter > > ; Karsten Keil ; Jassi Brar > > ; Tom Lendacky ; > > David S. Miller ; Jose Abreu > > ; Kalle Valo ; > > Stanislaw Gruszka ; Benson Leung > > ; Enric Balletbo i Serra > > ; James E.J. Bottomley > > ; Martin K. Petersen ; > > Greg Kroah-Hartman ; Alexander Viro > > ; Sergey Senozhatsky > > ; Steven Rostedt ; > > Andrew Morton ; intel- > > g...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux- > > ker...@vger.kernel.org; net...@vger.kernel.org; > > ath...@lists.infradead.org; linux-wirel...@vger.kernel.org; linux- > > s...@vger.kernel.org; linux-fb...@vger.kernel.org; > > de...@driverdev.osuosl.org; linux-fsde...@vger.kernel.org > > Subject: Re: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in > > hex_dump_to_buffer with flags > > > > On Wed 2019-04-10 13:17:19, Alastair D'Silva wrote: > > > From: Alastair D'Silva > > > > > > In order to support additional features in hex_dump_to_buffer, replace > > > the ascii bool parameter with flags. > > > > > > Signed-off-by: Alastair D'Silva > > > --- > > > drivers/gpu/drm/i915/intel_engine_cs.c| 2 +- > > > drivers/isdn/hardware/mISDN/mISDNisar.c | 6 -- > > > drivers/mailbox/mailbox-test.c| 2 +- > > > drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 2 +- > > > drivers/net/ethernet/synopsys/dwc-xlgmac-common.c | 2 +- > > > drivers/net/wireless/ath/ath10k/debug.c | 3 ++- > > > drivers/net/wireless/intel/iwlegacy/3945-mac.c| 2 +- > > > drivers/platform/chrome/wilco_ec/debugfs.c| 3 ++- > > > drivers/scsi/scsi_logging.c | 8 +++- > > > drivers/staging/fbtft/fbtft-core.c| 2 +- > > > fs/seq_file.c | 3 ++- > > > include/linux/printk.h| 2 +- > > > lib/hexdump.c | 15 --- > > > lib/test_hexdump.c| 5 +++-- > > > 14 files changed, 31 insertions(+), 26 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c > > > b/drivers/gpu/drm/i915/intel_engine_cs.c > > > index 49fa43ff02ba..fb133e729f9a 100644 > > > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > > > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > > > @@ -1318,7 +1318,7 @@ static void hexdump(struct drm_printer *m, const > > void *buf, size_t len) > > > WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - > > pos, > > > rowsize, sizeof(u32), > > > line, sizeof(line), > > > - false) >= sizeof(line)); > > > + 0) >= sizeof(line)); > > > > It might be more clear when we define: > > > > #define HEXDUMP_BINARY 0 > > This feels unnecessary, and weird. Omitting the flag won't disable the hex > output (as expected), and if you don't want hex output why call hexdump in > the first place? Why do we have HEXDUMP_ASCII then? Why is the above funtion not using HEXDUMP_ASCII? Who would call it with (HEXDUMP_ASCII | HEXDUMP_BINARY)? Best Regards, Petr ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
> -Original Message- > From: Petr Mladek > Sent: Monday, 15 April 2019 7:24 PM > To: Alastair D'Silva > Cc: 'Alastair D'Silva' ; 'Jani Nikula' > ; 'Joonas Lahtinen' > ; 'Rodrigo Vivi' ; > 'David Airlie' ; 'Daniel Vetter' ; 'Karsten > Keil' ; 'Jassi Brar' ; 'Tom > Lendacky' ; 'David S. Miller' > ; 'Jose Abreu' ; 'Kalle > Valo' ; 'Stanislaw Gruszka' ; > 'Benson Leung' ; 'Enric Balletbo i Serra' > ; 'James E.J. Bottomley' > ; 'Martin K. Petersen' ; > 'Greg Kroah-Hartman' ; 'Alexander Viro' > ; 'Sergey Senozhatsky' > ; 'Steven Rostedt' > ; 'Andrew Morton' ; > intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux- > ker...@vger.kernel.org; net...@vger.kernel.org; > ath...@lists.infradead.org; linux-wirel...@vger.kernel.org; linux- > s...@vger.kernel.org; linux-fb...@vger.kernel.org; > de...@driverdev.osuosl.org; linux-fsde...@vger.kernel.org > Subject: Re: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in > hex_dump_to_buffer with flags > > On Sat 2019-04-13 09:31:27, Alastair D'Silva wrote: > > > -Original Message- > > > From: Petr Mladek > > > Sent: Saturday, 13 April 2019 12:12 AM > > > To: Alastair D'Silva > > > Cc: alast...@d-silva.org; Jani Nikula ; > > Joonas > > > Lahtinen ; Rodrigo Vivi > > > ; David Airlie ; Daniel > > > Vetter ; Karsten Keil ; Jassi > > > Brar ; Tom Lendacky > > > ; David S. Miller > ; > > > Jose Abreu ; Kalle Valo > > > ; Stanislaw Gruszka ; > > > Benson Leung ; Enric Balletbo i Serra > > > ; James E.J. Bottomley > > > ; Martin K. Petersen > > > ; Greg Kroah-Hartman > > > ; Alexander Viro > > > ; Sergey Senozhatsky > > > ; Steven Rostedt > > > ; Andrew Morton foundation.org>; > > > intel- g...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; > > > linux- ker...@vger.kernel.org; net...@vger.kernel.org; > > > ath...@lists.infradead.org; linux-wirel...@vger.kernel.org; linux- > > > s...@vger.kernel.org; linux-fb...@vger.kernel.org; > > > de...@driverdev.osuosl.org; linux-fsde...@vger.kernel.org > > > Subject: Re: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in > > > hex_dump_to_buffer with flags > > > > > > On Wed 2019-04-10 13:17:19, Alastair D'Silva wrote: > > > > From: Alastair D'Silva > > > > > > > > In order to support additional features in hex_dump_to_buffer, > > > > replace the ascii bool parameter with flags. > > > > > > > > Signed-off-by: Alastair D'Silva > > > > --- > > > > drivers/gpu/drm/i915/intel_engine_cs.c| 2 +- > > > > drivers/isdn/hardware/mISDN/mISDNisar.c | 6 -- > > > > drivers/mailbox/mailbox-test.c| 2 +- > > > > drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 2 +- > > > > drivers/net/ethernet/synopsys/dwc-xlgmac-common.c | 2 +- > > > > drivers/net/wireless/ath/ath10k/debug.c | 3 ++- > > > > drivers/net/wireless/intel/iwlegacy/3945-mac.c| 2 +- > > > > drivers/platform/chrome/wilco_ec/debugfs.c| 3 ++- > > > > drivers/scsi/scsi_logging.c | 8 +++- > > > > drivers/staging/fbtft/fbtft-core.c| 2 +- > > > > fs/seq_file.c | 3 ++- > > > > include/linux/printk.h| 2 +- > > > > lib/hexdump.c | 15 --- > > > > lib/test_hexdump.c| 5 +++-- > > > > 14 files changed, 31 insertions(+), 26 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c > > > > b/drivers/gpu/drm/i915/intel_engine_cs.c > > > > index 49fa43ff02ba..fb133e729f9a 100644 > > > > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > > > > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > > > > @@ -1318,7 +1318,7 @@ static void hexdump(struct drm_printer *m, > > > > const > > > void *buf, size_t len) > > > > WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - > > > pos, > > > > rowsize, sizeof(u32), > > > > line, sizeof(line), > > > > - false) >= sizeof(line)); > > > > + 0) >= sizeof(line)); > > > > > > It might be more clear when we define: > > > > > > #define HEXDUMP_BINARY 0 > > > > This feels unnecessary, and weird. Omitting the flag won't disable the > > hex output (as expected), and if you don't want hex output why call > > hexdump in the first place? > > Why do we have HEXDUMP_ASCII then? > Why is the above funtion not using HEXDUMP_ASCII? > Who would call it with (HEXDUMP_ASCII | HEXDUMP_BINARY)? The ASCII flag controls the optional ASCII output, and replaces the boolean that existed prior. A user would enable it in the same situations where they currently pass true for the ascii parameter. In the above example the author only wants the hex output, while in other situations, both hex & ASCII output is desirable. If you just want ASCII output, the caller should just
[PATCH v3 3/3] backlight: lm3630a: add firmware node support
Add fwnode support to the lm3630a driver and allow configuring
the initial and maximum LED brightness on both LED banks independently.
The two outputs can be controlled by bank A and B independently or
bank A can control both outputs.
If the platform data was not configured, then the driver defaults to
enabling both banks. This patch changes the default value to disable
both banks before parsing the firmware node so that just a single bank
can be enabled if desired. There are no in-tree users of this driver.
Driver was tested on a LG Nexus 5 (hammerhead) phone.
Signed-off-by: Brian Masney
---
Changes since v2
- Separated out control banks and outputs via the reg and led-sources
properties.
- Use fwnode instead of of API
- Disable both banks by default before calling lm3630a_parse_node()
- Add lots of error handling
- Check for duplicate source / bank definitions
- Remove extra ;
- Make probe() method fail if fwnode parsing fails.
drivers/video/backlight/lm3630a_bl.c | 128 ++-
1 file changed, 126 insertions(+), 2 deletions(-)
diff --git a/drivers/video/backlight/lm3630a_bl.c
b/drivers/video/backlight/lm3630a_bl.c
index ef2553f452ca..15922da9c05a 100644
--- a/drivers/video/backlight/lm3630a_bl.c
+++ b/drivers/video/backlight/lm3630a_bl.c
@@ -364,6 +364,116 @@ static const struct regmap_config lm3630a_regmap = {
.max_register = REG_MAX,
};
+/**
+ * lm3630a_parse_led_sources - Parse the optional led-sources fwnode property.
+ * @node: firmware node
+ * @default_bitmask: bitmask to return if the led-sources property was not
+ * specified
+ *
+ * Parses the optional led-sources firmware node and returns a bitmask that
+ * contains the outputs that are associated with the control bank. If the
+ * property is missing, then the value of of @default_bitmask will be returned.
+ */
+static int lm3630a_parse_led_sources(struct fwnode_handle *node,
+int default_bitmask)
+{
+ int ret, num_sources, i;
+ u32 sources[2];
+
+ num_sources = fwnode_property_read_u32_array(node, "led-sources", NULL,
+0);
+ if (num_sources < 0)
+ return default_bitmask;
+ else if (num_sources > ARRAY_SIZE(sources))
+ return -EINVAL;
+
+ ret = fwnode_property_read_u32_array(node, "led-sources", sources,
+num_sources);
+ if (ret)
+ return ret;
+
+ ret = 0;
+ for (i = 0; i < num_sources; i++) {
+ if (sources[i] < 0 || sources[i] > 1)
+ return -EINVAL;
+
+ ret |= BIT(sources[i]);
+ }
+
+ return ret;
+}
+
+static int lm3630a_parse_node(struct lm3630a_chip *pchip,
+ struct lm3630a_platform_data *pdata)
+{
+ int seen = 0, led_sources, ret;
+ struct fwnode_handle *node;
+ u32 bank, val;
+ bool linear;
+
+ device_for_each_child_node(pchip->dev, node) {
+ ret = fwnode_property_read_u32(node, "reg", &bank);
+ if (ret < 0)
+ return ret;
+
+ if (bank < 0 || bank > 1)
+ return -EINVAL;
+
+ if (seen & BIT(bank))
+ return -EINVAL;
+ seen |= BIT(bank);
+
+ led_sources = lm3630a_parse_led_sources(node, BIT(bank));
+ if (led_sources < 0)
+ return led_sources;
+
+ linear = fwnode_property_read_bool(node,
+ "ti,linear-mapping-mode");
+ if (bank == 0) {
+ if (!(led_sources & BIT(0)))
+ return -EINVAL;
+
+ pdata->leda_ctrl = linear ?
+ LM3630A_LEDA_ENABLE_LINEAR :
+ LM3630A_LEDA_ENABLE;
+
+ if (led_sources & BIT(1)) {
+ if (seen & BIT(1))
+ return -EINVAL;
+ seen |= BIT(1);
+
+ pdata->ledb_ctrl = LM3630A_LEDB_ON_A;
+ }
+ } else {
+ if (led_sources & BIT(0) || !(led_sources & BIT(1)))
+ return -EINVAL;
+
+ pdata->ledb_ctrl = linear ?
+ LM3630A_LEDB_ENABLE_LINEAR :
+ LM3630A_LEDB_ENABLE;
+ }
+
+ ret = fwnode_property_read_u32(node, "default-brightness",
+ &val);
+ if (!ret) {
+ if (bank == 0)
+ pdata->leda_init_brt = val;
+ else
+ pdata->ledb_init_brt = val;
+ }
RE: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
From: Alastair D'Silva > Sent: 15 April 2019 11:07 ... > In the above example the author only wants the hex output, while in other > situations, both hex & ASCII output is desirable. If you just want ASCII > output, the caller should just use a printk or one of it's wrappers. Hexdump will 'sanitise' the ASCII. Although I think you'd want a 'no hex' flag to suppress the hex. Probably more useful flags are ones to suppress the address column. I've also used flags to enable (or disable) suppression of multiple lines of zeros of constant bytes. In that case you may want hexdump to return the flags for the next call when a large buffer is being dumped in fragments. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/4] lib/hexdump.c: Optionally suppress lines of filler bytes
On Sat 2019-04-13 09:28:03, Alastair D'Silva wrote:
> > -Original Message-
> > From: Petr Mladek
> > Sent: Saturday, 13 April 2019 12:04 AM
> > To: Alastair D'Silva
> > Cc: alast...@d-silva.org; Jani Nikula ;
> Joonas
> > Lahtinen ; Rodrigo Vivi
> > ; David Airlie ; Daniel Vetter
> > ; Karsten Keil ; Jassi Brar
> > ; Tom Lendacky ;
> > David S. Miller ; Jose Abreu
> > ; Kalle Valo ;
> > Stanislaw Gruszka ; Benson Leung
> > ; Enric Balletbo i Serra
> > ; James E.J. Bottomley
> > ; Martin K. Petersen ;
> > Greg Kroah-Hartman ; Alexander Viro
> > ; Sergey Senozhatsky
> > ; Steven Rostedt ;
> > Andrew Morton ; intel-
> > g...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-
> > ker...@vger.kernel.org; net...@vger.kernel.org;
> > ath...@lists.infradead.org; linux-wirel...@vger.kernel.org; linux-
> > s...@vger.kernel.org; linux-fb...@vger.kernel.org;
> > de...@driverdev.osuosl.org; linux-fsde...@vger.kernel.org
> > Subject: Re: [PATCH 2/4] lib/hexdump.c: Optionally suppress lines of
> filler
> > bytes
> >
> > On Wed 2019-04-10 13:17:18, Alastair D'Silva wrote:
> > > From: Alastair D'Silva
> > >
> > > Some buffers may only be partially filled with useful data, while the
> > > rest is padded (typically with 0x00 or 0xff).
> > >
> > > This patch introduces flags which allow lines of padding bytes to be
> > > suppressed, making the output easier to interpret:
> > > HEXDUMP_SUPPRESS_0X00, HEXDUMP_SUPPRESS_0XFF
> > >
> > > The first and last lines are not suppressed by default, so the
> > > function always outputs something. This behaviour can be further
> > > controlled with the HEXDUMP_SUPPRESS_FIRST &
> > HEXDUMP_SUPPRESS_LAST flags.
> > >
> > > An inline wrapper function is provided for backwards compatibility
> > > with existing code, which maintains the original behaviour.
> > >
> >
> > > diff --git a/lib/hexdump.c b/lib/hexdump.c index
> > > b8a164814744..2f3bafb55a44 100644
> > > --- a/lib/hexdump.c
> > > +++ b/lib/hexdump.c
> > > +void print_hex_dump_ext(const char *level, const char *prefix_str,
> > > + int prefix_type, int rowsize, int groupsize,
> > > + const void *buf, size_t len, u64 flags)
> > > {
> > > const u8 *ptr = buf;
> > > - int i, linelen, remaining = len;
> > > + int i, remaining = len;
> > > unsigned char linebuf[64 * 3 + 2 + 64 + 1];
> > > + bool first_line = true;
> > >
> > > if (rowsize != 16 && rowsize != 32 && rowsize != 64)
> > > rowsize = 16;
> > >
> > > for (i = 0; i < len; i += rowsize) {
> > > - linelen = min(remaining, rowsize);
> > > + bool skip = false;
> > > + int linelen = min(remaining, rowsize);
> > > +
> > > remaining -= rowsize;
> > >
> > > + if (flags & HEXDUMP_SUPPRESS_0X00)
> > > + skip = buf_is_all(ptr + i, linelen, 0x00);
> > > +
> > > + if (!skip && (flags & HEXDUMP_SUPPRESS_0XFF))
> > > + skip = buf_is_all(ptr + i, linelen, 0xff);
> > > +
> > > + if (first_line && !(flags & HEXDUMP_SUPPRESS_FIRST))
> > > + skip = false;
> > > +
> > > + if (remaining <= 0 && !(flags & HEXDUMP_SUPPRESS_LAST))
> > > + skip = false;
> > > +
> > > + if (skip)
> > > + continue;
> >
> > IMHO, quietly skipping lines could cause a lot of confusion, espcially
> when the address is not printed.
> >
> It's up to the caller to decide how they want it displayed.
I wonder who would want to quietly skip some data values.
Are you using it yourself? Could you please provide an
example?
I do not see why we would need to complicate the API and code
by this.
The behavior proposed by Tvrtko Ursulin makes much more
sense. I mean
https://lkml.kernel.org/r/929244ed-cc7f-b0f3-b5ac-50e798e83...@linux.intel.com
> > I wonder how it would look like when we print something like:
> >
> > --- skipped XX lines full of 0x00 ---
>
> This could be added as a later enhancement, with a new flag (eg.
> HEXDUMP_SUPPRESS_VERBOSE).
Who will add this later? Frankly, this looks like a half baked
feature that it good enough for you. If you want it upstream,
it must behave reasonably and it must be worth it.
Best Regards,
Petr
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 2/2] Input: add Apple SPI keyboard and trackpad driver.
The keyboard and trackpad on recent MacBook's (since 8,1) and
MacBookPro's (13,* and 14,*) are attached to an SPI controller instead
of USB, as previously. The higher level protocol is not publicly
documented and hence has been reverse engineered. As a consequence there
are still a number of unknown fields and commands. However, the known
parts have been working well and received extensive testing and use.
In order for this driver to work, the proper SPI drivers need to be
loaded too; for MB8,1 these are spi_pxa2xx_platform and spi_pxa2xx_pci;
for all others they are spi_pxa2xx_platform and intel_lpss_pci. For this
reason enabling this driver in the config implies enabling the above
drivers.
CC: Federico Lorenzi
CC: Lukas Wunner
CC: Andy Shevchenko
Link: https://bugzilla.kernel.org/show_bug.cgi?id=99891
Link: https://bugzilla.kernel.org/show_bug.cgi?id=108331
Signed-off-by: Ronald Tschalär
---
drivers/input/keyboard/Kconfig | 15 +
drivers/input/keyboard/Makefile |1 +
drivers/input/keyboard/applespi.c | 1985 +++
drivers/input/keyboard/applespi.h | 29 +
drivers/input/keyboard/applespi_trace.h | 94 ++
5 files changed, 2124 insertions(+)
create mode 100644 drivers/input/keyboard/applespi.c
create mode 100644 drivers/input/keyboard/applespi.h
create mode 100644 drivers/input/keyboard/applespi_trace.h
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index a878351f1643..d0a9e7fa2508 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -70,6 +70,21 @@ config KEYBOARD_AMIGA
config ATARI_KBD_CORE
bool
+config KEYBOARD_APPLESPI
+ tristate "Apple SPI keyboard and trackpad"
+ depends on ACPI && EFI
+ depends on SPI
+ depends on X86 || COMPILE_TEST
+ imply SPI_PXA2XX
+ imply SPI_PXA2XX_PCI
+ imply MFD_INTEL_LPSS_PCI
+ help
+ Say Y here if you are running Linux on any Apple MacBook8,1 or later,
+ or any MacBookPro13,* or MacBookPro14,*.
+
+ To compile this driver as a module, choose M here: the
+ module will be called applespi.
+
config KEYBOARD_ATARI
tristate "Atari keyboard"
depends on ATARI
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 182e92985dbf..9283fee2505a 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_KEYBOARD_ADP5520)+=
adp5520-keys.o
obj-$(CONFIG_KEYBOARD_ADP5588) += adp5588-keys.o
obj-$(CONFIG_KEYBOARD_ADP5589) += adp5589-keys.o
obj-$(CONFIG_KEYBOARD_AMIGA) += amikbd.o
+obj-$(CONFIG_KEYBOARD_APPLESPI)+= applespi.o
obj-$(CONFIG_KEYBOARD_ATARI) += atakbd.o
obj-$(CONFIG_KEYBOARD_ATKBD) += atkbd.o
obj-$(CONFIG_KEYBOARD_BCM) += bcm-keypad.o
diff --git a/drivers/input/keyboard/applespi.c
b/drivers/input/keyboard/applespi.c
new file mode 100644
index ..5c3d7687e346
--- /dev/null
+++ b/drivers/input/keyboard/applespi.c
@@ -0,0 +1,1985 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * MacBook (Pro) SPI keyboard and touchpad driver
+ *
+ * Copyright (c) 2015-2018 Federico Lorenzi
+ * Copyright (c) 2017-2018 Ronald Tschalär
+ */
+
+/*
+ * The keyboard and touchpad controller on the MacBookAir6, MacBookPro12,
+ * MacBook8 and newer can be driven either by USB or SPI. However the USB
+ * pins are only connected on the MacBookAir6 and 7 and the MacBookPro12.
+ * All others need this driver. The interface is selected using ACPI methods:
+ *
+ * * UIEN ("USB Interface Enable"): If invoked with argument 1, disables SPI
+ * and enables USB. If invoked with argument 0, disables USB.
+ * * UIST ("USB Interface Status"): Returns 1 if USB is enabled, 0 otherwise.
+ * * SIEN ("SPI Interface Enable"): If invoked with argument 1, disables USB
+ * and enables SPI. If invoked with argument 0, disables SPI.
+ * * SIST ("SPI Interface Status"): Returns 1 if SPI is enabled, 0 otherwise.
+ * * ISOL: Resets the four GPIO pins used for SPI. Intended to be invoked with
+ * argument 1, then once more with argument 0.
+ *
+ * UIEN and UIST are only provided on models where the USB pins are connected.
+ *
+ * SPI-based Protocol
+ * --
+ *
+ * The device and driver exchange messages (struct message); each message is
+ * encapsulated in one or more packets (struct spi_packet). There are two types
+ * of exchanges: reads, and writes. A read is signaled by a GPE, upon which one
+ * message can be read from the device. A write exchange consists of writing a
+ * command message, immediately reading a short status packet, and then, upon
+ * receiving a GPE, reading the response message. Write exchanges cannot be
+ * interleaved, i.e. a new write exchange must not be started till the previous
+ * write exchange is complete. Whether a received message is part of a read or
+ * write ex
Re: [PATCH 1/4] lib/hexdump.c: Allow 64 bytes per line
On Sat 2019-04-13 09:22:05, Alastair D'Silva wrote: > > -Original Message- > > From: Petr Mladek > > Sent: Friday, 12 April 2019 11:48 PM > > To: Alastair D'Silva > > Cc: alast...@d-silva.org; Jani Nikula ; > Joonas > > Lahtinen ; Rodrigo Vivi > > ; David Airlie ; Daniel Vetter > > ; Karsten Keil ; Jassi Brar > > ; Tom Lendacky ; > > David S. Miller ; Jose Abreu > > ; Kalle Valo ; > > Stanislaw Gruszka ; Benson Leung > > ; Enric Balletbo i Serra > > ; James E.J. Bottomley > > ; Martin K. Petersen ; > > Greg Kroah-Hartman ; Alexander Viro > > ; Sergey Senozhatsky > > ; Steven Rostedt ; > > Andrew Morton ; intel- > > g...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux- > > ker...@vger.kernel.org; net...@vger.kernel.org; > > ath...@lists.infradead.org; linux-wirel...@vger.kernel.org; linux- > > s...@vger.kernel.org; linux-fb...@vger.kernel.org; > > de...@driverdev.osuosl.org; linux-fsde...@vger.kernel.org > > Subject: Re: [PATCH 1/4] lib/hexdump.c: Allow 64 bytes per line > > > > On Wed 2019-04-10 13:17:17, Alastair D'Silva wrote: > > > From: Alastair D'Silva > > > > > > With modern high resolution screens, we can display more data, which > > > makes life a bit easier when debugging. > > > > I have quite some doubts about this feature. > > > > We are talking about more than 256 characters per-line. I wonder if such a > > long line is really easier to read for a human. > > It's basically 2 separate panes of information side by side, the hexdump and > the ASCII version. > > I'm using this myself when dealing with the pmem labels, and it works quite > nicely. I am sure that it works for you. But I do not believe that it would be useful in general. > > I am not expert but there is a reason why the standard is 80 characters > per- > > line. I guess that anything above 100 characters is questionable. > > https://en.wikipedia.org/wiki/Line_length > > somehow confirms that. > > > > Second, if we take 8 pixels per-character. Then we need > > 2048 to show the 256 characters. It is more than HD. > > IMHO, there is still huge number of people that even do not have HD > display, > > especially on a notebook. > > The intent is to make debugging easier when dealing with large chunks of > binary data. I don't expect end users to see this output. How is it supposed to be used then? Only by your temporary patches? Best Regards, Petr ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 2/2] Input: add Apple SPI keyboard and trackpad driver.
On Mon, Apr 15, 2019 at 01:13:00AM -0700, Ronald Tschalär wrote:
> The keyboard and trackpad on recent MacBook's (since 8,1) and
> MacBookPro's (13,* and 14,*) are attached to an SPI controller instead
> of USB, as previously. The higher level protocol is not publicly
> documented and hence has been reverse engineered. As a consequence there
> are still a number of unknown fields and commands. However, the known
> parts have been working well and received extensive testing and use.
>
> In order for this driver to work, the proper SPI drivers need to be
> loaded too; for MB8,1 these are spi_pxa2xx_platform and spi_pxa2xx_pci;
> for all others they are spi_pxa2xx_platform and intel_lpss_pci. For this
> reason enabling this driver in the config implies enabling the above
> drivers.
Thank you for an update.
I suddenly realized couple of places where something maybe optimized.
Nevertheless, FWIW,
Reviewed-by: Andy Shevchenko
>
> CC: Federico Lorenzi
> CC: Lukas Wunner
> CC: Andy Shevchenko
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=99891
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=108331
> Signed-off-by: Ronald Tschalär
> ---
> drivers/input/keyboard/Kconfig | 15 +
> drivers/input/keyboard/Makefile |1 +
> drivers/input/keyboard/applespi.c | 1985 +++
> drivers/input/keyboard/applespi.h | 29 +
> drivers/input/keyboard/applespi_trace.h | 94 ++
> 5 files changed, 2124 insertions(+)
> create mode 100644 drivers/input/keyboard/applespi.c
> create mode 100644 drivers/input/keyboard/applespi.h
> create mode 100644 drivers/input/keyboard/applespi_trace.h
>
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index a878351f1643..d0a9e7fa2508 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -70,6 +70,21 @@ config KEYBOARD_AMIGA
> config ATARI_KBD_CORE
> bool
>
> +config KEYBOARD_APPLESPI
> + tristate "Apple SPI keyboard and trackpad"
> + depends on ACPI && EFI
> + depends on SPI
> + depends on X86 || COMPILE_TEST
> + imply SPI_PXA2XX
> + imply SPI_PXA2XX_PCI
> + imply MFD_INTEL_LPSS_PCI
> + help
> + Say Y here if you are running Linux on any Apple MacBook8,1 or later,
> + or any MacBookPro13,* or MacBookPro14,*.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called applespi.
> +
> config KEYBOARD_ATARI
> tristate "Atari keyboard"
> depends on ATARI
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index 182e92985dbf..9283fee2505a 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_KEYBOARD_ADP5520) +=
> adp5520-keys.o
> obj-$(CONFIG_KEYBOARD_ADP5588) += adp5588-keys.o
> obj-$(CONFIG_KEYBOARD_ADP5589) += adp5589-keys.o
> obj-$(CONFIG_KEYBOARD_AMIGA) += amikbd.o
> +obj-$(CONFIG_KEYBOARD_APPLESPI) += applespi.o
> obj-$(CONFIG_KEYBOARD_ATARI) += atakbd.o
> obj-$(CONFIG_KEYBOARD_ATKBD) += atkbd.o
> obj-$(CONFIG_KEYBOARD_BCM) += bcm-keypad.o
> diff --git a/drivers/input/keyboard/applespi.c
> b/drivers/input/keyboard/applespi.c
> new file mode 100644
> index ..5c3d7687e346
> --- /dev/null
> +++ b/drivers/input/keyboard/applespi.c
> @@ -0,0 +1,1985 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * MacBook (Pro) SPI keyboard and touchpad driver
> + *
> + * Copyright (c) 2015-2018 Federico Lorenzi
> + * Copyright (c) 2017-2018 Ronald Tschalär
> + */
> +
> +/*
> + * The keyboard and touchpad controller on the MacBookAir6, MacBookPro12,
> + * MacBook8 and newer can be driven either by USB or SPI. However the USB
> + * pins are only connected on the MacBookAir6 and 7 and the MacBookPro12.
> + * All others need this driver. The interface is selected using ACPI methods:
> + *
> + * * UIEN ("USB Interface Enable"): If invoked with argument 1, disables SPI
> + * and enables USB. If invoked with argument 0, disables USB.
> + * * UIST ("USB Interface Status"): Returns 1 if USB is enabled, 0 otherwise.
> + * * SIEN ("SPI Interface Enable"): If invoked with argument 1, disables USB
> + * and enables SPI. If invoked with argument 0, disables SPI.
> + * * SIST ("SPI Interface Status"): Returns 1 if SPI is enabled, 0 otherwise.
> + * * ISOL: Resets the four GPIO pins used for SPI. Intended to be invoked
> with
> + * argument 1, then once more with argument 0.
> + *
> + * UIEN and UIST are only provided on models where the USB pins are
> connected.
> + *
> + * SPI-based Protocol
> + * --
> + *
> + * The device and driver exchange messages (struct message); each message is
> + * encapsulated in one or more packets (struct spi_packet). There are two
> types
> + * of exchanges: reads, and writes. A read is signaled by a GPE, upon which
> one
[PATCH v2 2/2] drm/ttm: Reset ttm_bo_glob when initialized
Always initialize ttm_bo_glob from a pristine state when its use_count
is 0. Persist use_count so that ttm_bo_glob can later be released.
Signed-off-by: Brian Yip
---
drivers/gpu/drm/ttm/ttm_bo.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 988416fb8a0b..d95762a90654 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1543,12 +1543,16 @@ static int ttm_bo_global_init(void)
{
struct ttm_bo_global *glob = &ttm_bo_glob;
int ret = 0;
- unsigned i;
+ unsigned i, uc;
mutex_lock(&ttm_global_mutex);
if (++glob->use_count > 1)
goto out;
+ uc = glob->use_count;
+ memset(glob, 0, sizeof(struct ttm_bo_global));
+ glob->use_count = uc;
+
ret = ttm_mem_global_init(&ttm_mem_glob);
if (ret)
goto out;
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 1/4] lib/hexdump.c: Allow 64 bytes per line
> > > On Wed 2019-04-10 13:17:17, Alastair D'Silva wrote: > > > > From: Alastair D'Silva > > > > > > > > With modern high resolution screens, we can display more data, > > > > which makes life a bit easier when debugging. > > > > > > I have quite some doubts about this feature. > > > > > > We are talking about more than 256 characters per-line. I wonder if > > > such a long line is really easier to read for a human. > > > > It's basically 2 separate panes of information side by side, the > > hexdump and the ASCII version. > > > > I'm using this myself when dealing with the pmem labels, and it works > > quite nicely. > > I am sure that it works for you. But I do not believe that it would be useful in > general. I do, and I believe the choice of the output length should be in the hands of the caller. On further thought, it would make more sense to remove the hardcoded list of sizes and just enforce a power of 2. The function shouldn't dictate what the caller can and can't do beyond the technical limits of it's implementation. Other print/debug functions don't restrict the output size, and I can't see a good justification why hexdump should either. > > > I am not expert but there is a reason why the standard is 80 > > > characters > > per- > > > line. I guess that anything above 100 characters is questionable. > > > https://en.wikipedia.org/wiki/Line_length > > > somehow confirms that. > > > > > > Second, if we take 8 pixels per-character. Then we need > > > 2048 to show the 256 characters. It is more than HD. > > > IMHO, there is still huge number of people that even do not have HD > > display, > > > especially on a notebook. > > > > The intent is to make debugging easier when dealing with large chunks > > of binary data. I don't expect end users to see this output. > > How is it supposed to be used then? Only by your temporary patches? Let me rephrase that, I don't expect end users to *use* this data. Current usage of the hexdump functions are predominantly centred around logging and debugging, and clearly targeted at someone intimately familiar with the relevant subsystem. I expect future use would be similar. Debugging may be as part of active development, or from a log supplied from an end user. In either case, it should be up to the author (as a representative for the consumers of the data) to decide how it should be formatted. -- Alastair D'Silva mob: 0423 762 819 skype: alastair_dsilva msn: alast...@d-silva.org blog: http://alastair.d-silva.orgTwitter: @EvilDeece ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 2/4] lib/hexdump.c: Optionally suppress lines of filler bytes
> > > On Wed 2019-04-10 13:17:18, Alastair D'Silva wrote:
> > > > From: Alastair D'Silva
> > > >
> > > > Some buffers may only be partially filled with useful data, while
> > > > the rest is padded (typically with 0x00 or 0xff).
> > > >
> > > > This patch introduces flags which allow lines of padding bytes to
> > > > be suppressed, making the output easier to interpret:
> > > > HEXDUMP_SUPPRESS_0X00, HEXDUMP_SUPPRESS_0XFF
> > > >
> > > > The first and last lines are not suppressed by default, so the
> > > > function always outputs something. This behaviour can be further
> > > > controlled with the HEXDUMP_SUPPRESS_FIRST &
> > > HEXDUMP_SUPPRESS_LAST flags.
> > > >
> > > > An inline wrapper function is provided for backwards compatibility
> > > > with existing code, which maintains the original behaviour.
> > > >
> > >
> > > > diff --git a/lib/hexdump.c b/lib/hexdump.c index
> > > > b8a164814744..2f3bafb55a44 100644
> > > > --- a/lib/hexdump.c
> > > > +++ b/lib/hexdump.c
> > > > +void print_hex_dump_ext(const char *level, const char *prefix_str,
> > > > + int prefix_type, int rowsize, int groupsize,
> > > > + const void *buf, size_t len, u64 flags)
> > > > {
> > > > const u8 *ptr = buf;
> > > > - int i, linelen, remaining = len;
> > > > + int i, remaining = len;
> > > > unsigned char linebuf[64 * 3 + 2 + 64 + 1];
> > > > + bool first_line = true;
> > > >
> > > > if (rowsize != 16 && rowsize != 32 && rowsize != 64)
> > > > rowsize = 16;
> > > >
> > > > for (i = 0; i < len; i += rowsize) {
> > > > - linelen = min(remaining, rowsize);
> > > > + bool skip = false;
> > > > + int linelen = min(remaining, rowsize);
> > > > +
> > > > remaining -= rowsize;
> > > >
> > > > + if (flags & HEXDUMP_SUPPRESS_0X00)
> > > > + skip = buf_is_all(ptr + i, linelen, 0x00);
> > > > +
> > > > + if (!skip && (flags & HEXDUMP_SUPPRESS_0XFF))
> > > > + skip = buf_is_all(ptr + i, linelen, 0xff);
> > > > +
> > > > + if (first_line && !(flags & HEXDUMP_SUPPRESS_FIRST))
> > > > + skip = false;
> > > > +
> > > > + if (remaining <= 0 && !(flags &
HEXDUMP_SUPPRESS_LAST))
> > > > + skip = false;
> > > > +
> > > > + if (skip)
> > > > + continue;
> > >
> > > IMHO, quietly skipping lines could cause a lot of confusion,
> > > espcially
> > when the address is not printed.
> > >
> > It's up to the caller to decide how they want it displayed.
>
> I wonder who would want to quietly skip some data values.
> Are you using it yourself? Could you please provide an example?
Yes, but I don't have the content with me at the moment, so I can't share
it. I'm dumping persistent memory labels, which are 64kB long, but only the
first few hundred bytes are populated.
> I do not see why we would need to complicate the API and code by this.
>
> The behavior proposed by Tvrtko Ursulin makes much more sense. I mean
> https://lkml.kernel.org/r/929244ed-cc7f-b0f3-b5ac-
> 50e798e83...@linux.intel.com
I agree that is better, I'll add that to V2.
--
Alastair D'Silva mob: 0423 762 819
skype: alastair_dsilva msn: alast...@d-silva.org
blog: http://alastair.d-silva.orgTwitter: @EvilDeece
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 2/3] dt-bindings: backlight: add lm3630a bindings
Add new backlight bindings for the TI LM3630A dual-string white LED.
Signed-off-by: Brian Masney
---
Rob: Since the common bindings aren't converted to the new JSON schema
yet, I'm not sure how to do led-sources here. I would expect that we'd
have the uint32-array on the common binding once it exists. I had to
add it here to keep 'make dt_binding_check' happy. I left the
description off though for that property since that'll come from common
once its converted.
Changes since v2:
- Update description of max-brightness
- Add description for reg
- Correct typo: s/tranisiton/transition
- Remove label from bindings since this isn't on backlight_properties
- add reg to control banks
- add additionalProperties
.../leds/backlight/lm3630a-backlight.yaml | 124 ++
1 file changed, 124 insertions(+)
create mode 100644
Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
diff --git
a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
new file mode 100644
index ..cccd43c02732
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
@@ -0,0 +1,124 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/backlight/lm3630a-backlight.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI LM3630A High-Efficiency Dual-String White LED
+
+maintainers:
+ - Lee Jones
+ - Daniel Thompson
+ - Jingoo Han
+
+description: |
+ The LM3630A is a current-mode boost converter which supplies the power and
+ controls the current in up to two strings of 10 LEDs per string.
+ https://www.ti.com/product/LM3630A
+
+properties:
+ compatible:
+const: ti,lm3630a
+
+ reg:
+description: The I2C address of the device
+maxItems: 1
+
+ ti,linear-mapping-mode:
+description: |
+ Enable linear mapping mode. If disabled, then it will use exponential
+ mapping mode in which the ramp up/down appears to have a more uniform
+ transition to the human eye.
+type: boolean
+
+required:
+ - compatible
+ - reg
+
+patternProperties:
+ "^led@[01]$":
+type: object
+description: |
+ Properties for a string of connected LEDs.
+
+properties:
+ reg:
+description: Control Bank
+maxItems: 1
+minimum: 0
+maximum: 1
+
+ led-sources:
+allOf:
+ - $ref: /schemas/types.yaml#/definitions/uint32-array
+ - minItems: 1
+maxItems: 2
+items:
+ minimum: 0
+ maximum: 1
+
+ default-brightness:
+description: Default brightness level on boot.
+minimum: 0
+maximum: 255
+
+ max-brightness:
+description: Maximum brightness that is allowed during runtime.
+minimum: 0
+maximum: 255
+
+required:
+ - reg
+
+additionalProperties: false
+
+additionalProperties: false
+
+examples:
+ - |
+i2c {
+#address-cells = <1>;
+#size-cells = <0>;
+
+lm3630a_bl@38 {
+compatible = "ti,lm3630a";
+status = "ok";
+reg = <0x38>;
+
+#address-cells = <1>;
+#size-cells = <0>;
+
+led@0 {
+reg = <0>;
+led-sources = <0 1>;
+default-brightness = <200>;
+max-brightness = <255>;
+};
+};
+};
+ - |
+i2c {
+#address-cells = <1>;
+#size-cells = <0>;
+
+lm3630a_bl@38 {
+compatible = "ti,lm3630a";
+status = "ok";
+reg = <0x38>;
+
+#address-cells = <1>;
+#size-cells = <0>;
+
+led@0 {
+reg = <0>;
+default-brightness = <150>;
+ti,linear-mapping-mode;
+};
+
+led@1 {
+reg = <1>;
+default-brightness = <225>;
+ti,linear-mapping-mode;
+};
+};
+};
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/3] drm/panfrost: Add initial panfrost driver
On 15/04/2019 10:18, Daniel Vetter wrote: > On Fri, Apr 05, 2019 at 05:42:33PM +0100, Steven Price wrote: >> On 05/04/2019 17:16, Alyssa Rosenzweig wrote: >>> acronym once ever and have it as a "??"), I'm not sure how to respond to >>> that... We don't know how to allocate memory for the GPU-internal data >>> structures (the tiler heap, for instance, but also a few others I've >>> just named "misc_0" and "scratchpad" -- guessing one of those is for >>> "TLS"). With kbase, I took the worst-case strategy of allocating >>> gigantic chunks on startup with tiny commit counts and GROW_ON_GPF set. >>> With the new driver, well, our memory consumption is scary since >>> implementing GROW_ON_GPF in an upstream-friendly way is a bit more work >>> and isn't expected to hit the 5.2 window. >> >> Yes GROW_ON_GPF is pretty much required for the tiler heap - it's not >> (reasonably) possible to determine how big it should be. The Arm user >> space driver does the same approach (tiny commit count, but allow it to >> grow). > > Jumping in here with a drive through comment ... > > Growing gem bo and dma-buf is going to be endless amounts of fun, since we > hard-coded that their size is invariant. > > I think the only reasonable way to implement this is if you allocate a > really huge bo, map it, but only put the pages in on faulting. Or when > really evil userspace tries to export it. Actually changing the underlying > buffer size is not going to work I think. Yes the idea is that you allocate a large amount of virtual address space, but only put a few physical pages in. If the GPU needs more you fault them in as necessary. The "buffer size" (i.e. virtual address region) never changes size. > Note: I didn't read kbase, so might be totally wrong in how GROW_ON_GPF > works. For kbase we simply don't support exporting this type of memory, and are fairly restrictive about mapping it into user space (user space shouldn't normally need to read it). Since Panfrost is using GEM BO it will have to deal with malicious user space. But it would be sufficient to simply fully back the region in that case. Recent version of kbase also support what is termed JIT (Just In Time memory allocation). Basically this involves the kernel driver allocating/freeing memory regions just before the job is loaded onto the GPU. These regions might also be GROW_ON_GPF. The intention is that when there isn't memory pressure these regions can be kept between jobs, but under memory pressure they can be discarded and recreated if they turn out to be needed again. Given the differences between the Panfrost and the proprietary user space I'm not sure exactly what form this will need to be for Panfrost, but Vulkan makes memory management "more interesting"! Allocating upfront for the worst case can become prohibitively expensive. Steve ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
> -Original Message- > From: David Laight > Sent: Monday, 15 April 2019 9:04 PM > To: 'Alastair D'Silva' ; 'Petr Mladek' > > Cc: 'Alastair D'Silva' ; 'Jani Nikula' > ; 'Joonas Lahtinen' > ; 'Rodrigo Vivi' ; > 'David Airlie' ; 'Daniel Vetter' ; 'Karsten > Keil' ; 'Jassi Brar' ; 'Tom > Lendacky' ; 'David S. Miller' > ; 'Jose Abreu' ; 'Kalle > Valo' ; 'Stanislaw Gruszka' ; > 'Benson Leung' ; 'Enric Balletbo i Serra' > ; 'James E.J. Bottomley' > ; 'Martin K. Petersen' ; > 'Greg Kroah-Hartman' ; 'Alexander Viro' > ; 'Sergey Senozhatsky' > ; 'Steven Rostedt' > ; 'Andrew Morton' ; > intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux- > ker...@vger.kernel.org; net...@vger.kernel.org; > ath...@lists.infradead.org; linux-wirel...@vger.kernel.org; linux- > s...@vger.kernel.org; linux-fb...@vger.kernel.org; > de...@driverdev.osuosl.org; linux-fsde...@vger.kernel.org > Subject: RE: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in > hex_dump_to_buffer with flags > > From: Alastair D'Silva > > Sent: 15 April 2019 11:45 > ... > > > Although I think you'd want a 'no hex' flag to suppress the hex. > > > > > > Probably more useful flags are ones to suppress the address column. > > > > This is already supported by the prefix_type parameter - are you > > proposing that we eliminate the parameter & combine it with flags? > > I was looking at the flags on one of my hexdump() functions... > > > > I've also used flags to enable (or disable) suppression of multiple > > > lines of zeros of constant bytes. > > > In that case you may want hexdump to return the flags for the next > > > call when a large buffer is being dumped in fragments. > > > > I'm afraid I don't quite follow here, hex_dump_to_buffer doesn't alter > > the flags, so the caller already knows it. > > If you are suppressing lines of zeros and dumping a buffer in several blocks > then subsequent calls need to know that the last line of the previous call was > suppressed zeros - and carry on with the same suppressed block. Why wouldn't you do this with a single call to print_hex_dump? (that is where the repeated lines are suppressed) That will already take chunks of the buffer until the whole thing is output, in what situation do you see a caller chunking the access themselves? -- Alastair D'Silva mob: 0423 762 819 skype: alastair_dsilva msn: alast...@d-silva.org blog: http://alastair.d-silva.orgTwitter: @EvilDeece ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 0/3] backlight: lm3630a: bug fix and fwnode support
Here is a patch series that fixes a single bug and adds fwnode support to the lm3630a driver. This work was tested on a LG Nexus 5 (hammerhead) phone. My status page at https://masneyb.github.io/nexus-5-upstream/ describes what is working so far with the upstream kernel. See the individual patches for the changelog. Brian Masney (3): backlight: lm3630a: return 0 on success in update_status functions dt-bindings: backlight: add lm3630a bindings backlight: lm3630a: add firmware node support .../leds/backlight/lm3630a-backlight.yaml | 124 drivers/video/backlight/lm3630a_bl.c | 132 +- 2 files changed, 252 insertions(+), 4 deletions(-) create mode 100644 Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 1/4] lib/hexdump.c: Allow 64 bytes per line
> From: Alastair D'Silva > > Sent: 15 April 2019 11:29 > ... > > I do, and I believe the choice of the output length should be in the > > hands of the caller. > > > > On further thought, it would make more sense to remove the hardcoded > > list of sizes and just enforce a power of 2. The function shouldn't > > dictate what the caller can and can't do beyond the technical limits of it's > implementation. > > Why powers of two? > You may want the length to match sizeof (struct foo). > You might even want the address increment to be larger that the number of > lines dumped. Good point, the base requirement is that it should be a multiple of groupsize. -- Alastair D'Silva mob: 0423 762 819 skype: alastair_dsilva msn: alast...@d-silva.org blog: http://alastair.d-silva.orgTwitter: @EvilDeece ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/3] dt-bindings: backlight: add lm3630a bindings
On Mon, Apr 15, 2019 at 07:10:04AM -0500, Dan Murphy wrote: > I noticed we are missing "label". It is defined as optional and it is hard > coded in the driver > but wondering if there is a need to add it. OK, I'll make it optional and have it fall back to the hardcoded values if it is missing. Thanks for the quick feedback on this patch and the other one. Brian ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 3/3] backlight: lm3630a: add firmware node support
On 4/15/19 2:29 AM, Brian Masney wrote:
> Add fwnode support to the lm3630a driver and allow configuring
> the initial and maximum LED brightness on both LED banks independently.
> The two outputs can be controlled by bank A and B independently or
> bank A can control both outputs.
>
> If the platform data was not configured, then the driver defaults to
> enabling both banks. This patch changes the default value to disable
> both banks before parsing the firmware node so that just a single bank
> can be enabled if desired. There are no in-tree users of this driver.
>
> Driver was tested on a LG Nexus 5 (hammerhead) phone.
>
> Signed-off-by: Brian Masney
> ---
> Changes since v2
> - Separated out control banks and outputs via the reg and led-sources
> properties.
> - Use fwnode instead of of API
> - Disable both banks by default before calling lm3630a_parse_node()
> - Add lots of error handling
> - Check for duplicate source / bank definitions
> - Remove extra ;
> - Make probe() method fail if fwnode parsing fails.
>
> drivers/video/backlight/lm3630a_bl.c | 128 ++-
> 1 file changed, 126 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/backlight/lm3630a_bl.c
> b/drivers/video/backlight/lm3630a_bl.c
> index ef2553f452ca..15922da9c05a 100644
> --- a/drivers/video/backlight/lm3630a_bl.c
> +++ b/drivers/video/backlight/lm3630a_bl.c
> @@ -364,6 +364,116 @@ static const struct regmap_config lm3630a_regmap = {
> .max_register = REG_MAX,
> };
>
> +/**
> + * lm3630a_parse_led_sources - Parse the optional led-sources fwnode
> property.
> + * @node: firmware node
> + * @default_bitmask: bitmask to return if the led-sources property was not
> + * specified
> + *
> + * Parses the optional led-sources firmware node and returns a bitmask that
> + * contains the outputs that are associated with the control bank. If the
> + * property is missing, then the value of of @default_bitmask will be
> returned.
> + */
> +static int lm3630a_parse_led_sources(struct fwnode_handle *node,
> + int default_bitmask)
> +{
> + int ret, num_sources, i;
> + u32 sources[2];
> +
> + num_sources = fwnode_property_read_u32_array(node, "led-sources", NULL,
> + 0);
> + if (num_sources < 0)
> + return default_bitmask;
> + else if (num_sources > ARRAY_SIZE(sources))
> + return -EINVAL;
> +
> + ret = fwnode_property_read_u32_array(node, "led-sources", sources,
> + num_sources);
> + if (ret)
> + return ret;
> +
> + ret = 0;
> + for (i = 0; i < num_sources; i++) {
> + if (sources[i] < 0 || sources[i] > 1)
> + return -EINVAL;
> +
> + ret |= BIT(sources[i]);
> + }
> +
> + return ret;
> +}
> +
> +static int lm3630a_parse_node(struct lm3630a_chip *pchip,
> + struct lm3630a_platform_data *pdata)
> +{
> + int seen = 0, led_sources, ret;
> + struct fwnode_handle *node;
> + u32 bank, val;
> + bool linear;
> +
> + device_for_each_child_node(pchip->dev, node) {
> + ret = fwnode_property_read_u32(node, "reg", &bank);
> + if (ret < 0)
> + return ret;
> +
> + if (bank < 0 || bank > 1)
> + return -EINVAL;
> +
> + if (seen & BIT(bank))
> + return -EINVAL;
Need line break for clarity.
> + seen |= BIT(bank);
> +
> + led_sources = lm3630a_parse_led_sources(node, BIT(bank));
> + if (led_sources < 0)
> + return led_sources;
> +
> + linear = fwnode_property_read_bool(node,
> +"ti,linear-mapping-mode");
> + if (bank == 0) {
> + if (!(led_sources & BIT(0)))
> + return -EINVAL;
> +
> + pdata->leda_ctrl = linear ?
> + LM3630A_LEDA_ENABLE_LINEAR :
> + LM3630A_LEDA_ENABLE;
> +
> + if (led_sources & BIT(1)) {
> + if (seen & BIT(1))
> + return -EINVAL;
Need line break for clarity.
> + seen |= BIT(1);
> +
> + pdata->ledb_ctrl = LM3630A_LEDB_ON_A;
> + }
> + } else {
> + if (led_sources & BIT(0) || !(led_sources & BIT(1)))
> + return -EINVAL;
> +
> + pdata->ledb_ctrl = linear ?
> + LM3630A_LEDB_ENABLE_LINEAR :
> + LM3630A_LEDB_ENABLE;
> + }
> +
> + ret = fwnode_property_read_u32(node, "default-brightness",
> +
RE: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
> -Original Message- > From: David Laight > Sent: Monday, 15 April 2019 8:21 PM > To: 'Alastair D'Silva' ; 'Petr Mladek' > > Cc: 'Alastair D'Silva' ; 'Jani Nikula' > ; 'Joonas Lahtinen' > ; 'Rodrigo Vivi' ; > 'David Airlie' ; 'Daniel Vetter' ; 'Karsten > Keil' ; 'Jassi Brar' ; 'Tom > Lendacky' ; 'David S. Miller' > ; 'Jose Abreu' ; 'Kalle > Valo' ; 'Stanislaw Gruszka' ; > 'Benson Leung' ; 'Enric Balletbo i Serra' > ; 'James E.J. Bottomley' > ; 'Martin K. Petersen' ; > 'Greg Kroah-Hartman' ; 'Alexander Viro' > ; 'Sergey Senozhatsky' > ; 'Steven Rostedt' > ; 'Andrew Morton' ; > intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux- > ker...@vger.kernel.org; net...@vger.kernel.org; > ath...@lists.infradead.org; linux-wirel...@vger.kernel.org; linux- > s...@vger.kernel.org; linux-fb...@vger.kernel.org; > de...@driverdev.osuosl.org; linux-fsde...@vger.kernel.org > Subject: RE: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in > hex_dump_to_buffer with flags > > From: Alastair D'Silva > > Sent: 15 April 2019 11:07 > ... > > In the above example the author only wants the hex output, while in > > other situations, both hex & ASCII output is desirable. If you just > > want ASCII output, the caller should just use a printk or one of it's > wrappers. > > Hexdump will 'sanitise' the ASCII. > This is functionality that doesn't exist in the current hexdump implementation (you always get the hex version). I think a better option would be to factor out the sanitisation and expose that as a separate function. > Although I think you'd want a 'no hex' flag to suppress the hex. > > Probably more useful flags are ones to suppress the address column. This is already supported by the prefix_type parameter - are you proposing that we eliminate the parameter & combine it with flags? > I've also used flags to enable (or disable) suppression of multiple lines of > zeros of constant bytes. > In that case you may want hexdump to return the flags for the next call when > a large buffer is being dumped in fragments. I'm afraid I don't quite follow here, hex_dump_to_buffer doesn't alter the flags, so the caller already knows it. -- Alastair D'Silva mob: 0423 762 819 skype: alastair_dsilva msn: alast...@d-silva.org blog: http://alastair.d-silva.orgTwitter: @EvilDeece ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/3] dt-bindings: backlight: add lm3630a bindings
Brian
On 4/15/19 2:29 AM, Brian Masney wrote:
> Add new backlight bindings for the TI LM3630A dual-string white LED.
>
> Signed-off-by: Brian Masney
> ---
> Rob: Since the common bindings aren't converted to the new JSON schema
> yet, I'm not sure how to do led-sources here. I would expect that we'd
> have the uint32-array on the common binding once it exists. I had to
> add it here to keep 'make dt_binding_check' happy. I left the
> description off though for that property since that'll come from common
> once its converted.
>
> Changes since v2:
> - Update description of max-brightness
> - Add description for reg
> - Correct typo: s/tranisiton/transition
> - Remove label from bindings since this isn't on backlight_properties
> - add reg to control banks
> - add additionalProperties
>
> .../leds/backlight/lm3630a-backlight.yaml | 124 ++
> 1 file changed, 124 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
>
> diff --git
> a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
> b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
> new file mode 100644
> index ..cccd43c02732
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
> @@ -0,0 +1,124 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/backlight/lm3630a-backlight.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI LM3630A High-Efficiency Dual-String White LED
> +
> +maintainers:
> + - Lee Jones
> + - Daniel Thompson
> + - Jingoo Han
> +
> +description: |
> + The LM3630A is a current-mode boost converter which supplies the power and
> + controls the current in up to two strings of 10 LEDs per string.
> + https://www.ti.com/product/LM3630A
> +
> +properties:
> + compatible:
> +const: ti,lm3630a
> +
> + reg:
> +description: The I2C address of the device
> +maxItems: 1
> +
> + ti,linear-mapping-mode:
> +description: |
> + Enable linear mapping mode. If disabled, then it will use exponential
> + mapping mode in which the ramp up/down appears to have a more uniform
> + transition to the human eye.
> +type: boolean
> +
> +required:
> + - compatible
> + - reg
> +
> +patternProperties:
> + "^led@[01]$":
> +type: object
> +description: |
> + Properties for a string of connected LEDs.
> +
> +properties:
> + reg:
> +description: Control Bank
> +maxItems: 1
> +minimum: 0
> +maximum: 1
> +
> + led-sources:
> +allOf:
> + - $ref: /schemas/types.yaml#/definitions/uint32-array
> + - minItems: 1
> +maxItems: 2
> +items:
> + minimum: 0
> + maximum: 1
> +
> + default-brightness:
> +description: Default brightness level on boot.
> +minimum: 0
> +maximum: 255
> +
> + max-brightness:
> +description: Maximum brightness that is allowed during runtime.
> +minimum: 0
> +maximum: 255
> +
> +required:
> + - reg
> +
> +additionalProperties: false
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +i2c {
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +lm3630a_bl@38 {
> +compatible = "ti,lm3630a";
> +status = "ok";
> +reg = <0x38>;
> +
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +led@0 {
> +reg = <0>;
> +led-sources = <0 1>;
> +default-brightness = <200>;
> +max-brightness = <255>;
> +};
> +};
> +};
> + - |
I noticed we are missing "label". It is defined as optional and it is hard
coded in the driver
but wondering if there is a need to add it.
Dan
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] include: linux: Regularise the use of FIELD_SIZEOF macro
> On Apr 14, 2019, at 3:14 AM, Shyam Saini
> wrote:
>
> Currently, there are 3 different macros, namely sizeof_field, SIZEOF_FIELD
> and FIELD_SIZEOF which are used to calculate the size of a member of
> structure, so to bring uniformity in entire kernel source tree lets use
> FIELD_SIZEOF and replace all occurrences of other two macros with this.
>
> For this purpose, redefine FIELD_SIZEOF in include/linux/stddef.h and
> tools/testing/selftests/bpf/bpf_util.h and remove its defination from
> include/linux/kernel.h
> --- a/include/linux/stddef.h
> +++ b/include/linux/stddef.h
> @@ -20,6 +20,15 @@ enum {
> #endif
>
> /**
> + * FIELD_SIZEOF - get the size of a struct's field
> + * @t: the target struct
> + * @f: the target struct's field
> + * Return: the size of @f in the struct definition without having a
> + * declared instance of @t.
> + */
> +#define FIELD_SIZEOF(t, f) (sizeof(((t *)0)->f))
> +
> +/**
> * sizeof_field(TYPE, MEMBER)
> *
> * @TYPE: The structure containing the field of interest
> @@ -34,6 +43,6 @@ enum {
> * @MEMBER: The member within the structure to get the end offset of
> */
> #define offsetofend(TYPE, MEMBER) \
> - (offsetof(TYPE, MEMBER) + sizeof_field(TYPE, MEMBER))
> + (offsetof(TYPE, MEMBER) + FIELD_SIZEOF(TYPE, MEMBER))
If you're doing this, why are you leaving the definition of sizeof_field() in
stddef.h untouched?
Given the way this has worked historically, if you are leaving it in place for
source compatibility reasons, shouldn't it be redefined in terms of
FIELD_SIZEOF(), e.g.:
#define sizeof_field(TYPE, MEMBER) FIELD_SIZEOF(TYPE, MEMBER)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/panel: simple: Add Evervision VGG804821 pannel support
Hi Thierry, gentle ping. On 19-01-14 11:28, Marco Felsch wrote: > Hi Sam, > > On 19-01-04 17:40, Sam Ravnborg wrote: > > Hi Marco. > > > > In $subject pannel => panel > > Thanks for covering that, Thierry can you fix this inline? Is that doable for you? Regards, Marco > Regards, > Marco > > > > > Other than that, bonus points for specifying > > all of .flags, .bus_format and .bus_flags > > > > Sam > > > > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 1/4] lib/hexdump.c: Allow 64 bytes per line
From: Alastair D'Silva > Sent: 15 April 2019 11:29 ... > I do, and I believe the choice of the output length should be in the hands > of the caller. > > On further thought, it would make more sense to remove the hardcoded list of > sizes and just enforce a power of 2. The function shouldn't dictate what the > caller can and can't do beyond the technical limits of it's implementation. Why powers of two? You may want the length to match sizeof (struct foo). You might even want the address increment to be larger that the number of lines dumped. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
From: Alastair D'Silva > Sent: 15 April 2019 11:45 ... > > Although I think you'd want a 'no hex' flag to suppress the hex. > > > > Probably more useful flags are ones to suppress the address column. > > This is already supported by the prefix_type parameter - are you proposing > that we eliminate the > parameter & combine it with flags? I was looking at the flags on one of my hexdump() functions... > > I've also used flags to enable (or disable) suppression of multiple lines of > > zeros of constant bytes. > > In that case you may want hexdump to return the flags for the next call when > > a large buffer is being dumped in fragments. > > I'm afraid I don't quite follow here, hex_dump_to_buffer doesn't alter the > flags, > so the caller already knows it. If you are suppressing lines of zeros and dumping a buffer in several blocks then subsequent calls need to know that the last line of the previous call was suppressed zeros - and carry on with the same suppressed block. I've not looked to see if it does support suppressing lines of zeros/0xff. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/panel: simple: Add Evervision VGG804821 pannel support
Hi Sam, thanks for your fast response =) On 19-04-15 18:10, Sam Ravnborg wrote: > Hi Marco > > On Mon, Apr 15, 2019 at 05:46:48PM +0200, Marco Felsch wrote: > > Hi Thierry, > > > > gentle ping. > > > > On 19-01-14 11:28, Marco Felsch wrote: > > > Hi Sam, > > > > > > On 19-01-04 17:40, Sam Ravnborg wrote: > > > > Hi Marco. > > > > > > > > In $subject pannel => panel > > > > > > Thanks for covering that, Thierry can you fix this inline? > > > > Is that doable for you? > > If you resend I will apply. I deleted the original patch-set. > This will allow me to try my first commit to drm-misc :-) Okay, let me resend it as v2 with your integrated comment. Regards, Marco > > Sam > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 1/2] drm/bridge: sil_sii8620: make remote control optional.
commit d6abe6df706c (drm/bridge: sil_sii8620: do not have a dependency
of RC_CORE) changed the driver to select both RC_CORE and INPUT.
However, this causes problems with other drivers, in particular an input
driver that depends on MFD_INTEL_LPSS_PCI (to be added in a separate
commit):
drivers/clk/Kconfig:9:error: recursive dependency detected!
drivers/clk/Kconfig:9:symbol COMMON_CLK is selected by MFD_INTEL_LPSS
drivers/mfd/Kconfig:566: symbol MFD_INTEL_LPSS is selected by
MFD_INTEL_LPSS_PCI
drivers/mfd/Kconfig:580: symbol MFD_INTEL_LPSS_PCI is implied by
KEYBOARD_APPLESPI
drivers/input/keyboard/Kconfig:73:symbol KEYBOARD_APPLESPI depends on
INPUT
drivers/input/Kconfig:8: symbol INPUT is selected by DRM_SIL_SII8620
drivers/gpu/drm/bridge/Kconfig:83:symbol DRM_SIL_SII8620 depends on
DRM_BRIDGE
drivers/gpu/drm/bridge/Kconfig:1: symbol DRM_BRIDGE is selected by
DRM_PL111
drivers/gpu/drm/pl111/Kconfig:1: symbol DRM_PL111 depends on COMMON_CLK
According to the docs and general consensus, select should only be used
for non user-visible symbols, but both RC_CORE and INPUT are
user-visible. Furthermore almost all other references to INPUT
throughout the kernel config are depends, not selects. For this reason
the first part of this change reverts commit d6abe6df706c.
In order to address the original reason for commit d6abe6df706c, namely
that not all boards use the remote controller functionality and hence
should not need have to deal with RC_CORE, the second part of this
change now makes the remote control support in the driver optional and
contingent on RC_CORE being defined. And with this the hard dependency
on INPUT also goes away as that is only needed if RC_CORE is defined
(which in turn already depends on INPUT).
CC: Inki Dae
CC: Andrzej Hajda
CC: Laurent Pinchart
CC: Dmitry Torokhov
Signed-off-by: Ronald Tschalär
---
drivers/gpu/drm/bridge/Kconfig | 3 +--
drivers/gpu/drm/bridge/sil-sii8620.c | 10 +++---
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 2fee47b0d50b..9cf07105b73a 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -85,8 +85,7 @@ config DRM_SIL_SII8620
depends on OF
select DRM_KMS_HELPER
imply EXTCON
- select INPUT
- select RC_CORE
+ imply RC_CORE
help
Silicon Image SII8620 HDMI/MHL bridge chip driver.
diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c
b/drivers/gpu/drm/bridge/sil-sii8620.c
index a6e8f4591e63..cff3131aae6c 100644
--- a/drivers/gpu/drm/bridge/sil-sii8620.c
+++ b/drivers/gpu/drm/bridge/sil-sii8620.c
@@ -1763,10 +1763,8 @@ static bool sii8620_rcp_consume(struct sii8620 *ctx, u8
scancode)
scancode &= MHL_RCP_KEY_ID_MASK;
- if (!ctx->rc_dev) {
- dev_dbg(ctx->dev, "RCP input device not initialized\n");
+ if (!IS_ENABLED(CONFIG_RC_CORE) || !ctx->rc_dev)
return false;
- }
if (pressed)
rc_keydown(ctx->rc_dev, RC_PROTO_CEC, scancode, 0);
@@ -2103,6 +2101,9 @@ static void sii8620_init_rcp_input_dev(struct sii8620
*ctx)
struct rc_dev *rc_dev;
int ret;
+ if (!IS_ENABLED(CONFIG_RC_CORE))
+ return;
+
rc_dev = rc_allocate_device(RC_DRIVER_SCANCODE);
if (!rc_dev) {
dev_err(ctx->dev, "Failed to allocate RC device\n");
@@ -2217,6 +2218,9 @@ static void sii8620_detach(struct drm_bridge *bridge)
{
struct sii8620 *ctx = bridge_to_sii8620(bridge);
+ if (!IS_ENABLED(CONFIG_RC_CORE))
+ return;
+
rc_unregister_device(ctx->rc_dev);
}
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 2/2] Input: add Apple SPI keyboard and trackpad driver.
Hi Andy,
On Mon, Apr 15, 2019 at 12:03:46PM +0300, Andy Shevchenko wrote:
> On Mon, Apr 15, 2019 at 01:13:00AM -0700, Ronald Tschalär wrote:
> > The keyboard and trackpad on recent MacBook's (since 8,1) and
> > MacBookPro's (13,* and 14,*) are attached to an SPI controller instead
> > of USB, as previously. The higher level protocol is not publicly
> > documented and hence has been reverse engineered. As a consequence there
> > are still a number of unknown fields and commands. However, the known
> > parts have been working well and received extensive testing and use.
> >
> > In order for this driver to work, the proper SPI drivers need to be
> > loaded too; for MB8,1 these are spi_pxa2xx_platform and spi_pxa2xx_pci;
> > for all others they are spi_pxa2xx_platform and intel_lpss_pci. For this
> > reason enabling this driver in the config implies enabling the above
> > drivers.
>
> Thank you for an update.
> I suddenly realized couple of places where something maybe optimized.
>
> Nevertheless, FWIW,
> Reviewed-by: Andy Shevchenko
Many thanks for all your reviews!
[snip]
> > +static void
> > +applespi_remap_fn_key(struct keyboard_protocol *keyboard_protocol)
> > +{
> > + unsigned char tmp;
>
> > + u8 bit = BIT(fnremap - 1);
>
> The above is UB and I'm sorry I didn't find this earlier.
>
> So, something like this would work
>
> u8 bit = BIT((fnremap - 1) & 0x07);
fnremap is already constrained by the following:
> > +
> > + if (!fnremap || fnremap > ARRAY_SIZE(applespi_controlcodes) ||
> > + !applespi_controlcodes[fnremap - 1])
> > + return;
and the array-size of applespi_controlcodes is constrained to the
number of bits in u8 according to this assertion
> > + compiletime_assert(ARRAY_SIZE(applespi_controlcodes) ==
> > + sizeof_field(struct keyboard_protocol, modifiers) *
> > 8,
> > + "applespi_controlcodes has wrong number of entries");
So I don't see that the masking buys anything new.
[snip]
> > + /* check for rollover overflow, which is signalled by all keys == 1 */
> > + for (i = 0; i < MAX_ROLLOVER; i++) {
> > + if (keyboard_protocol->keys_pressed[i] != 1)
> > + break;
> > + }
> > +
> > + if (i == MAX_ROLLOVER) /* all keys were 1 */
> > + return;
>
> Since keys_pressed is an array of byte values, it may be replaced with
> memchr_inv().
>
> > +
> > + /* remap fn key if desired */
> > + applespi_remap_fn_key(keyboard_protocol);
> > +
> > + /* check released keys */
> > + for (i = 0; i < MAX_ROLLOVER; i++) {
> > + for (j = 0; j < MAX_ROLLOVER; j++) {
> > + if (applespi->last_keys_pressed[i] ==
> > + keyboard_protocol->keys_pressed[j])
> > + break;
> > + }
> > +
> > + if (j < MAX_ROLLOVER) /* key is still pressed */
> > + continue;
>
> And memchr() here.
Ah, yes, excellent suggestion. Thanks.
Cheers,
Ronald
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/2] drm/ttm: Cleanup state in global ttm structures
State in global ttm structures was not properly reset on cleanup. In the scenario where multiple drm drivers are loaded and the first one fails to initialize, there is an attempt to reset the global state in the ttm structures. However, there were some attributes which were not properly reset during this process. This resulted in a crash and false positive kobj initialization warnings during subsequent initialization of these global structures. Fix the crash and false positive kobj initialization warnings. --- Changes in v2: State in global ttm structures is reset by means of zeroing memory in global structure initialization functions rather than checking reference counters in their corresponding release functions. Brian Yip (2): drm/ttm: Reset ttm_mem_global when initialized drm/ttm: Reset ttm_bo_glob when initialized drivers/gpu/drm/ttm/ttm_bo.c | 6 +- drivers/gpu/drm/ttm/ttm_memory.c | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/3] backlight: lm3630a: return 0 on success in update_status functions
lm3630a_bank_a_update_status() and lm3630a_bank_b_update_status()
both return the brightness value if the brightness was successfully
updated. Writing to these attributes via sysfs would cause a 'Bad
address' error to be returned. These functions should return 0 on
success, so let's change it to correct that error.
Signed-off-by: Brian Masney
Fixes: 28e64a68a2ef ("backlight: lm3630: apply chip revision")
Acked-by: Pavel Machek
---
Changes since v2:
- None
drivers/video/backlight/lm3630a_bl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/backlight/lm3630a_bl.c
b/drivers/video/backlight/lm3630a_bl.c
index 2030a6b77a09..ef2553f452ca 100644
--- a/drivers/video/backlight/lm3630a_bl.c
+++ b/drivers/video/backlight/lm3630a_bl.c
@@ -201,7 +201,7 @@ static int lm3630a_bank_a_update_status(struct
backlight_device *bl)
LM3630A_LEDA_ENABLE, LM3630A_LEDA_ENABLE);
if (ret < 0)
goto out_i2c_err;
- return bl->props.brightness;
+ return 0;
out_i2c_err:
dev_err(pchip->dev, "i2c failed to access\n");
@@ -278,7 +278,7 @@ static int lm3630a_bank_b_update_status(struct
backlight_device *bl)
LM3630A_LEDB_ENABLE, LM3630A_LEDB_ENABLE);
if (ret < 0)
goto out_i2c_err;
- return bl->props.brightness;
+ return 0;
out_i2c_err:
dev_err(pchip->dev, "i2c failed to access REG_CTRL\n");
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 0/2] Add Apple SPI keyboard and trackpad driver
This changeset adds a driver for the SPI keyboard and trackpad on recent MacBook's and MacBook Pro's. The driver has seen a fair amount of use over the last 2 years (basically anybody running linux on these machines), with only relatively small changes in the last year or so. For those interested, the driver development has been hosted at https://github.com/cb22/macbook12-spi-driver/ (as well as my clone at https://github.com/roadrunner2/macbook12-spi-driver/). The first patch fixes a problem during config. The second patch contains the new applespi driver. Changes in v5: Applied all feedback from review by Andy Shevchenko (applespi) and Andrzej Hajda (sil-sii8620), including: - (sil_sii8620) use 'if (IS_DEFINED(...))' instead of conditional compilation to handle RC_CORE being undefined - (applespi) a number of cleanups of leftovers from changes to earlier versions - (applespi) replace some bool flags with loop-end detection - (applespi) improve error reporting in a couple places The full set of changes to applespi can be viewed at https://github.com/roadrunner2/macbook12-spi-driver/ as individual commits daf60f8..36afd70 in the upstreaming-review branch. Ronald Tschalär (2): drm/bridge: sil_sii8620: make remote control optional. Input: add Apple SPI keyboard and trackpad driver. drivers/gpu/drm/bridge/Kconfig |3 +- drivers/gpu/drm/bridge/sil-sii8620.c| 10 +- drivers/input/keyboard/Kconfig | 15 + drivers/input/keyboard/Makefile |1 + drivers/input/keyboard/applespi.c | 1985 +++ drivers/input/keyboard/applespi.h | 29 + drivers/input/keyboard/applespi_trace.h | 94 ++ 7 files changed, 2132 insertions(+), 5 deletions(-) create mode 100644 drivers/input/keyboard/applespi.c create mode 100644 drivers/input/keyboard/applespi.h create mode 100644 drivers/input/keyboard/applespi_trace.h -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: add DRM_PRIME_CAP_LOCAL
On Wed, Apr 10, 2019 at 09:47:12AM +0200, Gerd Hoffmann wrote:
> Some drivers (for example qxl) support neither import nor export of
> dma-bufs. But you can still use dma-bufs to pass buffer references
> from one process to another; drm_gem_prime_import() will figure the
> dma-buf came from the same driver and just takes a reference in that
> case instead of doing a full export/import.
>
> Right now there is no way for userspace to figure it can do this. Add
> the new prime capability DRM_PRIME_CAP_LOCAL to indicate that. Set the
> bit for every driver which has the DRIVER_PRIME feature bit set.
It can just try? Any driver that supports dma-buf (even if only for
self-import) should set DRIVER_PRIME. It's perfectly fine to reject any
other sharing.
-Daniel
>
> Cc: Dave Airlie
> Signed-off-by: Gerd Hoffmann
> ---
> include/uapi/drm/drm.h | 1 +
> drivers/gpu/drm/drm_ioctl.c | 2 ++
> 2 files changed, 3 insertions(+)
>
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 236b01a1fabf..b6157d48dc1a 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -632,6 +632,7 @@ struct drm_gem_open {
> #define DRM_CAP_PRIME0x5
> #define DRM_PRIME_CAP_IMPORT0x1
> #define DRM_PRIME_CAP_EXPORT0x2
> +#define DRM_PRIME_CAP_LOCAL 0x4
> #define DRM_CAP_TIMESTAMP_MONOTONIC 0x6
> #define DRM_CAP_ASYNC_PAGE_FLIP 0x7
> /*
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index d337f161909c..00599758783b 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -241,6 +241,8 @@ static int drm_getcap(struct drm_device *dev, void *data,
> struct drm_file *file_
> case DRM_CAP_PRIME:
> req->value |= dev->driver->prime_fd_to_handle ?
> DRM_PRIME_CAP_IMPORT : 0;
> req->value |= dev->driver->prime_handle_to_fd ?
> DRM_PRIME_CAP_EXPORT : 0;
> + req->value |= (dev->driver->driver_features & DRIVER_PRIME)
> + ? DRM_PRIME_CAP_LOCAL : 0;
> return 0;
> case DRM_CAP_SYNCOBJ:
> req->value = drm_core_check_feature(dev, DRIVER_SYNCOBJ);
> --
> 2.18.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: fix drm_fb_xrgb8888_to_rgb888_dstclip()
On Thu, Apr 11, 2019 at 06:49:32AM +0200, Gerd Hoffmann wrote:
> Oops, the __iomem annotation was added to the header file only.
> Add it to the implementation (and documentation) too.
>
> Fixes: 5c5373b51bec ("drm: switch drm_fb_xrgb_to_rgb888_dstclip to accept
> __iomem dst")
> Signed-off-by: Gerd Hoffmann
> ---
> drivers/gpu/drm/drm_format_helper.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_format_helper.c
> b/drivers/gpu/drm/drm_format_helper.c
> index 319de608a088..da388012df2a 100644
> --- a/drivers/gpu/drm/drm_format_helper.c
> +++ b/drivers/gpu/drm/drm_format_helper.c
> @@ -230,7 +230,7 @@ static void drm_fb_xrgb_to_rgb888_line(u8 *dbuf, u32
> *sbuf,
>
> /**
> * drm_fb_xrgb_to_rgb888_dstclip - Convert XRGB to RGB888 clip buffer
> - * @dst: RGB565 destination buffer
> + * @dst: RGB565 destination buffer (iomem)
> * @dst_pitch: destination buffer pitch
> * @vaddr: XRGB source buffer
> * @fb: DRM framebuffer
> @@ -241,9 +241,9 @@ static void drm_fb_xrgb_to_rgb888_line(u8 *dbuf, u32
> *sbuf,
> * support XRGB.
> *
> * This function applies clipping on dst, i.e. the destination is a
> - * full framebuffer but only the clip rect content is copied over.
> + * full (iomem) framebuffer but only the clip rect content is copied over.
> */
All this nice kerneldoc isn't pulled into the docs in
Documentation/gpu/drm-kms-helper.rst anywhere. Can you pls fix that too?
Thanks, Daniel
> -void drm_fb_xrgb_to_rgb888_dstclip(void *dst, unsigned int dst_pitch,
> +void drm_fb_xrgb_to_rgb888_dstclip(void __iomem *dst, unsigned int
> dst_pitch,
> void *vaddr, struct drm_framebuffer *fb,
> struct drm_rect *clip)
> {
> --
> 2.18.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [v3 1/7] drm: Add gamma mode property
On Fri, Apr 12, 2019 at 03:50:57PM +0530, Uma Shankar wrote:
> From: Ville Syrjälä
>
> Add a gamma mode property to enable various kind of
> gamma modes supported by platforms like: Interpolated, Split,
> Multi Segmented etc. Userspace can get this property and
> should be able to get the platform capabilties wrt various
> gamma modes possible and the possible ranges.
>
> It can select one of the modes exposed as blob_id as an
> enum and set the respective mode.
>
> It can then create the LUT and send it to driver using
> already available GAMMA_LUT property as blob.
>
> v2: Addressed Sam Ravnborg's review comments. Implemented
> gamma mode with just one property and renamed the current
> one to GAMMA_MODE property as recommended by Ville.
>
> Signed-off-by: Ville Syrjälä
> Signed-off-by: Uma Shankar
Please also extend the CTM property docs, see
https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#color-management-properties
And especially how GAMMA_MODE interacts with everything else we have
already. I think the current comments don't really explain well how this
is supposed to be used.
Also, since this is quite a complicated data structure, can't we do at
least some basic validation in the core code?
-Daniel
> ---
> drivers/gpu/drm/drm_atomic_uapi.c | 5 +++
> drivers/gpu/drm/drm_color_mgmt.c | 77
> +++
> include/drm/drm_color_mgmt.h | 8
> include/drm/drm_crtc.h| 7
> include/drm/drm_mode_config.h | 6 +++
> include/uapi/drm/drm_mode.h | 38 +++
> 6 files changed, 141 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index ea797d4..d85e0c9 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -459,6 +459,9 @@ static int drm_atomic_crtc_set_property(struct drm_crtc
> *crtc,
> &replaced);
> state->color_mgmt_changed |= replaced;
> return ret;
> + } else if (property == config->gamma_mode_property) {
> + state->gamma_mode = val;
> + state->color_mgmt_changed |= replaced;
> } else if (property == config->prop_out_fence_ptr) {
> s32 __user *fence_ptr = u64_to_user_ptr(val);
>
> @@ -495,6 +498,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc
> *crtc,
> *val = (state->mode_blob) ? state->mode_blob->base.id : 0;
> else if (property == config->prop_vrr_enabled)
> *val = state->vrr_enabled;
> + else if (property == config->gamma_mode_property)
> + *val = state->gamma_mode;
> else if (property == config->degamma_lut_property)
> *val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
> else if (property == config->ctm_property)
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c
> b/drivers/gpu/drm/drm_color_mgmt.c
> index d5d34d0..4d6792d 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -176,6 +176,83 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> }
> EXPORT_SYMBOL(drm_crtc_enable_color_mgmt);
>
> +void drm_crtc_attach_gamma_mode_property(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct drm_mode_config *config = &dev->mode_config;
> +
> + if (!config->gamma_mode_property)
> + return;
> +
> + drm_object_attach_property(&crtc->base,
> +config->gamma_mode_property, 0);
> +}
> +EXPORT_SYMBOL(drm_crtc_attach_gamma_mode_property);
> +
> +int drm_color_create_gamma_mode_property(struct drm_device *dev,
> + int num_values)
> +{
> + struct drm_mode_config *config = &dev->mode_config;
> + struct drm_property *prop;
> +
> + prop = drm_property_create(dev,
> +DRM_MODE_PROP_ENUM,
> +"GAMMA_MODE", num_values);
> + if (!prop)
> + return -ENOMEM;
> +
> + config->gamma_mode_property = prop;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_color_create_gamma_mode_property);
> +
> +int drm_color_add_gamma_mode_range(struct drm_device *dev,
> +const char *name,
> +const struct drm_color_lut_range *ranges,
> +size_t length)
> +{
> + struct drm_mode_config *config = &dev->mode_config;
> + struct drm_property_blob *blob;
> + struct drm_property *prop;
> + int num_ranges = length / sizeof(ranges[0]);
> + int i, ret, num_types_0;
> +
> + if (WARN_ON(length == 0 || length % sizeof(ranges[0]) != 0))
> + return -EINVAL;
> +
> + num_types_0 = hweight8(ranges[0].flags & (DRM_MODE_LUT_GAMMA |
> + DRM_MODE_LUT_DEGAMMA));
> + if (num_types_0 == 0)
> + return -EI
Re: [v2,1/2] drm: Add writeback_w,h properties
On Mon, Apr 15, 2019 at 10:20:45AM +0100, Liviu Dudau wrote:
> On Mon, Apr 15, 2019 at 08:59:30AM +0100, james qian wang (Arm Technology
> China) wrote:
> > On Fri, Apr 12, 2019 at 02:08:28PM +, Ben Davis wrote:
> > > Add new properties to specify width and height for writeback.
> > >
> > > Signed-off-by: Ben Davis
> > > ---
> > > drivers/gpu/drm/drm_atomic_uapi.c | 8
> > > drivers/gpu/drm/drm_writeback.c | 28
> > > include/drm/drm_connector.h | 4
> > > include/drm/drm_mode_config.h | 10 ++
> > > 4 files changed, 50 insertions(+)
> > >
> > > --
> > > 2.7.4
> > >
> > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> > > b/drivers/gpu/drm/drm_atomic_uapi.c
> > > index d520a04..1f68dce 100644
> > > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > > @@ -765,6 +765,10 @@ static int drm_atomic_connector_set_property(struct
> > > drm_connector *connector,
> > > return -EINVAL;
> > > }
> > > state->content_protection = val;
> > > + } else if (property == config->prop_writeback_w) {
> > > + state->writeback_w = val;
> > > + } else if (property == config->prop_writeback_h) {
> > > + state->writeback_h = val;
> > > } else if (property == config->writeback_fb_id_property) {
> > > struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, NULL,
> > > val);
> > > int ret = drm_atomic_set_writeback_fb_for_connector(state, fb);
> > > @@ -837,6 +841,10 @@ drm_atomic_connector_get_property(struct
> > > drm_connector *connector,
> > > *val = state->scaling_mode;
> > > } else if (property == connector->content_protection_property) {
> > > *val = state->content_protection;
> > > + } else if (property == config->prop_writeback_w) {
> > > + *val = state->writeback_w;
> > > + } else if (property == config->prop_writeback_h) {
> > > + *val = state->writeback_h;
> > > } else if (property == config->writeback_fb_id_property) {
> > > /* Writeback framebuffer is one-shot, write and forget */
> > > *val = 0;
> > > diff --git a/drivers/gpu/drm/drm_writeback.c
> > > b/drivers/gpu/drm/drm_writeback.c
> > > index c20e6fe..3d0453e 100644
> > > --- a/drivers/gpu/drm/drm_writeback.c
> > > +++ b/drivers/gpu/drm/drm_writeback.c
> > > @@ -74,6 +74,12 @@
> > > * applications making use of writeback connectors *always*
> > > retrieve an
> > > * out-fence for the commit and use it appropriately.
> > > * From userspace, this property will always read as zero.
> > > + *
> > > + * "WRITEBACK_W":
> > > + * The width of the writeback buffer to write back. 0 acts as
> > > default.
> > > + *
> > > + * "WRITEBACK_H":
> > > + * The height of the writeback buffer to write back. 0 acts as
> > > default.
> > > */
> > >
> > > #define fence_to_wb_connector(x) container_of(x->lock, \
> > > @@ -141,6 +147,22 @@ static int create_writeback_properties(struct
> > > drm_device *dev)
> > > dev->mode_config.writeback_out_fence_ptr_property = prop;
> > > }
> > >
> > > + if (!dev->mode_config.prop_writeback_w) {
> > > + prop = drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC,
> > > + "WRITEBACK_W", 1, UINT_MAX);
> > > + if (!prop)
> > > + return -ENOMEM;
> > > + dev->mode_config.prop_writeback_w = prop;
> > > + }
> > > +
> > > + if (!dev->mode_config.prop_writeback_h) {
> > > + prop = drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC,
> > > + "WRITEBACK_H", 1, UINT_MAX);
> > > + if (!prop)
> > > + return -ENOMEM;
> > > + dev->mode_config.prop_writeback_h = prop;
> > > + }
> > > +
> > > return 0;
> > > }
> > >
> > > @@ -225,6 +247,12 @@ int drm_writeback_connector_init(struct drm_device
> > > *dev,
> > > drm_object_attach_property(&connector->base,
> > > config->writeback_pixel_formats_property,
> > > blob->base.id);
> > > +
> > > + drm_object_attach_property(&connector->base,
> > > +config->prop_writeback_w, 0);
> > > + drm_object_attach_property(&connector->base,
> > > +config->prop_writeback_h, 0);
> >
> > Hi Ben:
> > Do we real need these two individual properties for specifing the writeback
> > w/h, can we use fb->w/h ?
> > And since these two properties are added as common and standard properties,
> > it influnce all existing write-back implementation which all assumed
> > writeback size as fb->w/h.
>
> The idea of having these additional properties was to maintain backwards
> compatibility (of some sort). If you don't set writeback_w/h then the
> assumption is that they are the same as fb->w/h, but I can see how it's
> going to be confusing to have fb->w/h differen
Re: [PATCH v1] drm: Do not allow to set NOFB for active primary plane
On Mon, Apr 15, 2019 at 11:27:25AM +, Lisovskiy, Stanislav wrote:
> On Mon, 2019-04-15 at 12:40 +0200, Maarten Lankhorst wrote:
> > Op 15-04-2019 om 10:00 schreef Stanislav Lisovskiy:
> > > There was an issue reported from end users, confirmed
> > > also locally that when user executes xrandr --output
> > > --rotate left/right, the other eDP screen gets blank after
> > > rotation.
> > >
> > > Investigation showed that reason was that primary plane
> > > was for some reason disabled, while cursor plane was still enabled.
> > > After some effort it turns out that userspace might wrongly
> > > assign NOFB to active primary plane for some reason.
> > >
> > > Then this gets detected in drm_atomic_helper_check_plane_state,
> > > called from ->plane_check and plane gets deactivated, leaving
> > > the screen blank and cursor visible. This can be cured by reboot
> > > or xrandr off/on sequence for that crtc.
> > >
> > > This patch is proposal to fix the issue by forbiding fb removal
> > > from active primary plane. If one needs to remove fb plane must be
> > > disabled first.
> > >
> > > Not sure if this is correct, however it fixes the issue at least.
> > >
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110375
> > > Signed-off-by: Stanislav Lisovskiy
> > > ---
> > > drivers/gpu/drm/drm_atomic_uapi.c | 9 +
> > > 1 file changed, 9 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> > > b/drivers/gpu/drm/drm_atomic_uapi.c
> > > index ea797d4c82ee..e2f078b302f3 100644
> > > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > > @@ -229,6 +229,15 @@ drm_atomic_set_fb_for_plane(struct
> > > drm_plane_state *plane_state,
> > > {
> > > struct drm_plane *plane = plane_state->plane;
> > >
> > > + if (!fb) {
> > > + if (plane->state->visible && plane->type ==
> > > DRM_PLANE_TYPE_PRIMARY) {
> > > + DRM_DEBUG_ATOMIC("Not allowed to set [NOFB] for
> > > active"
> > > + " primary [PLANE:%d:%s] -
> > > disable first",
> > > + plane->base.id, plane->name);
> > > + return;
> > > + }
> > > + }
> > > +
> > > if (fb)
> > > DRM_DEBUG_ATOMIC("Set [FB:%d] for [PLANE:%d:%s] state
> > > %p\n",
> > >fb->base.id, plane->base.id, plane-
> > > >name,
> >
> > If userspace asks to disable the primary plane, but not the crtc then
> > that is not an atomic bug. The bug is in the userspace requesting
> > this (Xorg).
> >
> > If you want to include such a check, the correct place is at the end
> > of drm_atomic_check_only(), after ->atomic_check(), return -EINVAL
> > there.
> >
> > But such a patch will not be accepted. It's perfectly acceptable to
> > disable the primary plane for whatever reason, and some of our
> > internal tests do so.
>
> Well, yeah that's why I was not quite sure, if this is correct.
> However to me the problem seems to be that userspace is disabling
> primary plane without realizing it. For example on my skl I see
> that drm_atomic_helper_check_plane_state complains that primary
> plane has no FB and then disables it.
Yeah we need to figure out where this is coming from. Is this with sna or
-modesetting or something else?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 17/25] drm/mediatek: add background color input select function for ovl/ovl_2l
Hi, Yongqiang:
On Wed, 2019-03-27 at 14:19 +0800, yongqiang@mediatek.com wrote:
> From: Yongqiang Niu
>
> This patch add background color input select function for ovl/ovl_2l
>
> ovl include 4 DRAM layer and 1 background color layer
> ovl_2l include 4 DRAM layer and 1 background color layer
> DRAM layer frame buffer data from render hardware, GPU for example.
> backgournd color layer is embed in ovl/ovl_2l, we can only set
> it color, but not support DRAM frame buffer.
>
> for ovl0->ovl0_2l direct link usecase,
> we need set ovl0_2l background color intput select from ovl0
> if render send DRAM buffer layer number <=4, all these layer read
> by ovl.
> layer0 is at the bottom of all layers.
> layer3 is at the top of all layers.
> if render send DRAM buffer layer numbfer >=4 && <=6
> ovl0 read layer0~3
> ovl0_2l read layer4~5
> layer5 is at the top ot all these layers.
>
> the decision of how to setting ovl0/ovl0_2l read these layer data
> is controlled in mtk crtc, which will be another patch
>
> Signed-off-by: Yongqiang Niu
> ---
> drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 24
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index a0ab760..c226284 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -27,6 +27,8 @@
> #define DISP_REG_OVL_EN 0x000c
> #define DISP_REG_OVL_RST 0x0014
> #define DISP_REG_OVL_ROI_SIZE0x0020
> +#define DISP_REG_OVL_DATAPATH_CON0x0024
> +#define OVL_BGCLR_SEL_INBIT(2)
> #define DISP_REG_OVL_ROI_BGCLR 0x0028
> #define DISP_REG_OVL_SRC_CON 0x002c
> #define DISP_REG_OVL_CON(n) (0x0030 + 0x20 * (n))
> @@ -245,6 +247,26 @@ static void mtk_ovl_layer_config(struct mtk_ddp_comp
> *comp, unsigned int idx,
> mtk_ovl_layer_on(comp, idx);
> }
>
> +static void mtk_ovl_bgclr_in_on(struct mtk_ddp_comp *comp,
> + enum mtk_ddp_comp_id prev)
> +{
> + int is_ovl = 0;
> +
> +if (prev == DDP_COMPONENT_OVL0 ||
> +prev == DDP_COMPONENT_OVL0_2L ||
> +prev == DDP_COMPONENT_OVL1_2L)
> + is_ovl = 1;
I think this logic should be moved to mtk crtc and OVL need not to care
about which component is in front of it.
Regards,
CK
> +
> + mtk_ddp_write_mask((is_ovl << 2), comp,
> +DISP_REG_OVL_DATAPATH_CON, OVL_BGCLR_SEL_IN);
> +}
> +
> +static void mtk_ovl_bgclr_in_off(struct mtk_ddp_comp *comp)
> +{
> +mtk_ddp_write_mask(0, comp,
> + DISP_REG_OVL_DATAPATH_CON, OVL_BGCLR_SEL_IN);
> +}
> +
> static const struct mtk_ddp_comp_funcs mtk_disp_ovl_funcs = {
> .config = mtk_ovl_config,
> .start = mtk_ovl_start,
> @@ -255,6 +277,8 @@ static void mtk_ovl_layer_config(struct mtk_ddp_comp
> *comp, unsigned int idx,
> .layer_on = mtk_ovl_layer_on,
> .layer_off = mtk_ovl_layer_off,
> .layer_config = mtk_ovl_layer_config,
> + .bgclr_in_on = mtk_ovl_bgclr_in_on,
> + .bgclr_in_off = mtk_ovl_bgclr_in_off,
> };
>
> static int mtk_disp_ovl_bind(struct device *dev, struct device *master,
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Expose "FB_DAMAGE_CLIPS" property to atomic aware user-space only
On Mon, Apr 15, 2019 at 05:28:05PM +, Deepak Singh Rawat wrote:
> Plane property "FB_DAMAGE_CLIPS" can only be used by atomic aware
> user-space, so no point exposing it otherwise.
>
> Signed-off-by: Deepak Rawat
> Fixes: d3b21767821e ("drm: Add a new plane property to send damage during
> plane update")
Makes sense, probably good if we add Cc: sta...@vger.kernel.org too
Reviewed-by: Daniel Vetter
btw, want drm-misc commit rights to push this? Assuming you'd then push
vmware drm patches in general ofc. See
https://drm.pages.freedesktop.org/maintainer-tools/getting-started.html
Cheers, Daniel
> ---
> drivers/gpu/drm/drm_mode_config.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mode_config.c
> b/drivers/gpu/drm/drm_mode_config.c
> index 4a1c2023ccf0..1a346ae1599d 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -297,8 +297,9 @@ static int drm_mode_create_standard_properties(struct
> drm_device *dev)
> return -ENOMEM;
> dev->mode_config.prop_crtc_id = prop;
>
> - prop = drm_property_create(dev, DRM_MODE_PROP_BLOB, "FB_DAMAGE_CLIPS",
> -0);
> + prop = drm_property_create(dev,
> + DRM_MODE_PROP_ATOMIC | DRM_MODE_PROP_BLOB,
> + "FB_DAMAGE_CLIPS", 0);
> if (!prop)
> return -ENOMEM;
> dev->mode_config.prop_fb_damage_clips = prop;
> --
> 2.21.0
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: turn syncobj timeline support off by default
On Mon, Apr 15, 2019 at 08:48:25PM +0100, Lionel Landwerlin wrote: > Unfortunately userspace users of this API cannot be publicly disclosed > yet so disable this stuff by default until all is revealed. > > Signed-off-by: Lionel Landwerlin > --- > drivers/gpu/drm/Kconfig | 10 ++ > drivers/gpu/drm/drm_syncobj.c | 12 > 2 files changed, 22 insertions(+) > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 5e1bc630b885..5b002793f0e4 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -423,3 +423,13 @@ config DRM_PANEL_ORIENTATION_QUIRKS > config DRM_LIB_RANDOM > bool > default n > + > +config DRM_SYNCOBJ_TIMELINE > + bool "Enable syncobj timeline support" > + depends on DRM > + default n > + help > + Choose this option if you want to enable preliminary support > + for timeline syncobjs. > + > + If in doubt, say "N". > diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c > index ea22b79a7170..dd1bdd287225 100644 > --- a/drivers/gpu/drm/drm_syncobj.c > +++ b/drivers/gpu/drm/drm_syncobj.c > @@ -754,6 +754,9 @@ drm_syncobj_transfer_ioctl(struct drm_device *dev, void > *data, > struct drm_syncobj_transfer *args = data; > int ret; > > + if (!IS_ENABLED(CONFIG_DRM_SYNCOBJ_TIMELINE)) > + return -ENODEV; I think the latest rage for this stuff is a module_option_unsafe. Much easier for igts to set by default (we want to keep CI'ing this ofc). And _unsafe taints the kernel when you set it, so about equally good. -Daniel > + > if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ)) > return -ENODEV; > > @@ -1105,6 +1108,9 @@ drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, > void *data, > struct drm_syncobj **syncobjs; > int ret = 0; > > + if (!IS_ENABLED(CONFIG_DRM_SYNCOBJ_TIMELINE)) > + return -ENODEV; > + > if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ)) > return -ENODEV; > > @@ -1209,6 +1215,9 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device > *dev, void *data, > uint32_t i, j; > int ret; > > + if (!IS_ENABLED(CONFIG_DRM_SYNCOBJ_TIMELINE)) > + return -ENODEV; > + > if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ)) > return -EOPNOTSUPP; > > @@ -1280,6 +1289,9 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, > void *data, > uint32_t i; > int ret; > > + if (!IS_ENABLED(CONFIG_DRM_SYNCOBJ_TIMELINE)) > + return -ENODEV; > + > if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ)) > return -ENODEV; > > -- > 2.21.0.392.gf8f6787159e > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/3] drm/panfrost: Add initial panfrost driver
On Mon, Apr 15, 2019 at 10:30:14AM +0100, Steven Price wrote: > On 15/04/2019 10:18, Daniel Vetter wrote: > > On Fri, Apr 05, 2019 at 05:42:33PM +0100, Steven Price wrote: > >> On 05/04/2019 17:16, Alyssa Rosenzweig wrote: > >>> acronym once ever and have it as a "??"), I'm not sure how to respond to > >>> that... We don't know how to allocate memory for the GPU-internal data > >>> structures (the tiler heap, for instance, but also a few others I've > >>> just named "misc_0" and "scratchpad" -- guessing one of those is for > >>> "TLS"). With kbase, I took the worst-case strategy of allocating > >>> gigantic chunks on startup with tiny commit counts and GROW_ON_GPF set. > >>> With the new driver, well, our memory consumption is scary since > >>> implementing GROW_ON_GPF in an upstream-friendly way is a bit more work > >>> and isn't expected to hit the 5.2 window. > >> > >> Yes GROW_ON_GPF is pretty much required for the tiler heap - it's not > >> (reasonably) possible to determine how big it should be. The Arm user > >> space driver does the same approach (tiny commit count, but allow it to > >> grow). > > > > Jumping in here with a drive through comment ... > > > > Growing gem bo and dma-buf is going to be endless amounts of fun, since we > > hard-coded that their size is invariant. > > > > I think the only reasonable way to implement this is if you allocate a > > really huge bo, map it, but only put the pages in on faulting. Or when > > really evil userspace tries to export it. Actually changing the underlying > > buffer size is not going to work I think. > > Yes the idea is that you allocate a large amount of virtual address > space, but only put a few physical pages in. If the GPU needs more you > fault them in as necessary. The "buffer size" (i.e. virtual address > region) never changes size. > > > Note: I didn't read kbase, so might be totally wrong in how GROW_ON_GPF > > works. > > For kbase we simply don't support exporting this type of memory, and are > fairly restrictive about mapping it into user space (user space > shouldn't normally need to read it). You can still disallow sharing with any other driver (in the dma-buf attach callback), and then enforce whatever mapping restrictions you want on the panfrost.ko ioctl interface. That should be roughly equivalent to the restrictions kbase imposes. > > Since Panfrost is using GEM BO it will have to deal with malicious user > space. But it would be sufficient to simply fully back the region in > that case. > > Recent version of kbase also support what is termed JIT (Just In Time > memory allocation). Basically this involves the kernel driver > allocating/freeing memory regions just before the job is loaded onto the > GPU. These regions might also be GROW_ON_GPF. The intention is that when > there isn't memory pressure these regions can be kept between jobs, but > under memory pressure they can be discarded and recreated if they turn > out to be needed again. > > Given the differences between the Panfrost and the proprietary user > space I'm not sure exactly what form this will need to be for Panfrost, > but Vulkan makes memory management "more interesting"! Allocating > upfront for the worst case can become prohibitively expensive. The usual way to do that is with a WONTNEED/WILLNEED madvise ioctl on the gem bo. I guess that could also be set at create time to essentially only require the bo to exist during an execbuf call. Should fit pretty well into what other drivers are doing with gem shmem already I think. ofc needs a shrinker to be able to reap these bo. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC] MAINTAINERS: Add Sam as reviewer for drm/panel
From: Thierry Reding Sam has been helping out a lot with reviewing DRM panel patches. Add him as reviewer to help him do this important work. Signed-off-by: Thierry Reding --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index c227d2818c98..3d199592fd2c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5340,6 +5340,7 @@ T:git git://anongit.freedesktop.org/drm/drm-misc DRM PANEL DRIVERS M: Thierry Reding +R: Sam Ravnborg L: dri-devel@lists.freedesktop.org T: git git://anongit.freedesktop.org/drm/drm-misc S: Maintained -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 02/12] drm/fb-helper: Avoid race with DRM userspace
On Sun, Apr 07, 2019 at 06:52:33PM +0200, Noralf Trønnes wrote:
> drm_fb_helper_is_bound() is used to check if DRM userspace is in control.
> This is done by looking at the fb on the primary plane. By the time
> fb-helper gets around to committing, it's possible that the facts have
> changed.
>
> Avoid this race by holding the drm_device->master_mutex lock while
> committing. When DRM userspace does its first open, it will now wait
> until fb-helper is done. The helper will stay away if there's a master.
>
> Locking rule: Always take the fb-helper lock first.
>
> v2:
> - Remove drm_fb_helper_is_bound() (Daniel Vetter)
> - No need to check fb_helper->dev->master in
> drm_fb_helper_single_fb_probe(), restore_fbdev_mode() has the check.
>
> Suggested-by: Daniel Vetter
> Signed-off-by: Noralf Trønnes
> ---
> drivers/gpu/drm/drm_auth.c | 20
> drivers/gpu/drm/drm_fb_helper.c | 90 -
> drivers/gpu/drm/drm_internal.h | 2 +
> 3 files changed, 67 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
> index 1669c42c40ed..db199807b7dc 100644
> --- a/drivers/gpu/drm/drm_auth.c
> +++ b/drivers/gpu/drm/drm_auth.c
> @@ -368,3 +368,23 @@ void drm_master_put(struct drm_master **master)
> *master = NULL;
> }
> EXPORT_SYMBOL(drm_master_put);
> +
> +/* Used by drm_client and drm_fb_helper */
> +bool drm_master_internal_acquire(struct drm_device *dev)
> +{
> + mutex_lock(&dev->master_mutex);
> + if (dev->master) {
> + mutex_unlock(&dev->master_mutex);
> + return false;
> + }
> +
> + return true;
> +}
> +EXPORT_SYMBOL(drm_master_internal_acquire);
> +
> +/* Used by drm_client and drm_fb_helper */
> +void drm_master_internal_release(struct drm_device *dev)
> +{
> + mutex_unlock(&dev->master_mutex);
> +}
> +EXPORT_SYMBOL(drm_master_internal_release);
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 84791dd4a90d..a6be09ae899b 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -44,6 +44,7 @@
>
> #include "drm_crtc_internal.h"
> #include "drm_crtc_helper_internal.h"
> +#include "drm_internal.h"
>
> static bool drm_fbdev_emulation = true;
> module_param_named(fbdev_emulation, drm_fbdev_emulation, bool, 0600);
> @@ -509,7 +510,7 @@ static int restore_fbdev_mode_legacy(struct drm_fb_helper
> *fb_helper)
> return ret;
> }
>
> -static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
> +static int restore_fbdev_mode_force(struct drm_fb_helper *fb_helper)
Bikeshed: usually the function variant that's run with locks already taken
is called _locked or has a __ prefix. _force feels a bit misplaced.
> {
> struct drm_device *dev = fb_helper->dev;
>
> @@ -519,6 +520,21 @@ static int restore_fbdev_mode(struct drm_fb_helper
> *fb_helper)
> return restore_fbdev_mode_legacy(fb_helper);
> }
>
> +static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
> +{
> + struct drm_device *dev = fb_helper->dev;
> + int ret;
> +
> + if (!drm_master_internal_acquire(dev))
> + return -EBUSY;
> +
> + ret = restore_fbdev_mode_force(fb_helper);
> +
> + drm_master_internal_release(dev);
> +
> + return ret;
> +}
> +
> /**
> * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
> * @fb_helper: driver-allocated fbdev helper, can be NULL
> @@ -556,34 +572,6 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct
> drm_fb_helper *fb_helper)
> }
> EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked);
>
> -static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper)
> -{
> - struct drm_device *dev = fb_helper->dev;
> - struct drm_crtc *crtc;
> - int bound = 0, crtcs_bound = 0;
> -
> - /*
> - * Sometimes user space wants everything disabled, so don't steal the
> - * display if there's a master.
> - */
> - if (READ_ONCE(dev->master))
> - return false;
> -
> - drm_for_each_crtc(crtc, dev) {
> - drm_modeset_lock(&crtc->mutex, NULL);
> - if (crtc->primary->fb)
> - crtcs_bound++;
> - if (crtc->primary->fb == fb_helper->fb)
> - bound++;
> - drm_modeset_unlock(&crtc->mutex);
> - }
> -
> - if (bound < crtcs_bound)
> - return false;
> -
> - return true;
> -}
> -
> #ifdef CONFIG_MAGIC_SYSRQ
> /*
> * restore fbcon display for all kms driver's using this helper, used for
> sysrq
> @@ -604,7 +592,7 @@ static bool drm_fb_helper_force_kernel_mode(void)
> continue;
>
> mutex_lock(&helper->lock);
> - ret = restore_fbdev_mode(helper);
> + ret = restore_fbdev_mode_force(helper);
I'd leave this as-is, because:
a) I'm too lazy to review the locking of our open/close calls to convince
myself that la
Re: [PATCH v2 18/25] drm/mediatek: add RDMA fifo size error handle
Hi, Yongqiang:
On Wed, 2019-03-27 at 14:19 +0800, yongqiang@mediatek.com wrote:
> From: Yongqiang Niu
>
> This patch add RDMA fifo size error handle
> rdma fifo size will not always bigger than the calculated threshold
> if that case happened, we need set fifo size as the threshold
>
> Signed-off-by: Yongqiang Niu
> ---
> drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> index b0a5cff..ead38ba 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> @@ -137,11 +137,14 @@ static void mtk_rdma_config(struct mtk_ddp_comp *comp,
> unsigned int width,
> {
> unsigned int threshold;
> unsigned int reg;
> + unsigned int rdma_fifo_size;
> struct mtk_disp_rdma *rdma = comp_to_rdma(comp);
>
> rdma_update_bits(comp, DISP_REG_RDMA_SIZE_CON_0, 0xfff, width);
> rdma_update_bits(comp, DISP_REG_RDMA_SIZE_CON_1, 0xf, height);
>
> + rdma_fifo_size = RDMA_FIFO_SIZE(rdma);
> +
> /*
>* Enable FIFO underflow since DSI and DPI can't be blocked.
>* Keep the FIFO pseudo size reset default of 8 KiB. Set the
> @@ -149,8 +152,12 @@ static void mtk_rdma_config(struct mtk_ddp_comp *comp,
> unsigned int width,
>* account for blanking, and with a pixel depth of 4 bytes:
>*/
> threshold = width * height * vrefresh * 4 * 7 / 100;
> +
> + if (threshold > rdma_fifo_size)
> + threshold = rdma_fifo_size;
I think this is a work around not a correct solution. Why MT8173 has no
this problem but MT8183 has? Is the formula of threshold different in
MT8173 and MT8183?
Regards,
CK
> +
> reg = RDMA_FIFO_UNDERFLOW_EN |
> - RDMA_FIFO_PSEUDO_SIZE(RDMA_FIFO_SIZE(rdma)) |
> + RDMA_FIFO_PSEUDO_SIZE(rdma_fifo_size) |
> RDMA_OUTPUT_VALID_FIFO_THRESHOLD(threshold);
> writel(reg, comp->regs + DISP_REG_RDMA_FIFO_CON);
> }
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] MAINTAINERS: Add Sam as reviewer for drm/panel
On Tue, Apr 16, 2019 at 1:29 PM Thierry Reding wrote: > > From: Thierry Reding > > Sam has been helping out a lot with reviewing DRM panel patches. Add him > as reviewer to help him do this important work. > > Signed-off-by: Thierry Reding > --- > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index c227d2818c98..3d199592fd2c 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -5340,6 +5340,7 @@ T:git git://anongit.freedesktop.org/drm/drm-misc > > DRM PANEL DRIVERS > M: Thierry Reding > +R: Sam Ravnborg Acked-by: Jagan Teki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 20/25] drm/mediatek: add ovl0/ovl0_2l usecase
Hi, Yongqiang:
On Wed, 2019-03-27 at 14:19 +0800, yongqiang@mediatek.com wrote:
> From: Yongqiang Niu
>
> This patch add ovl0/ovl0_2l usecase
> in ovl->ovl0_2l direct link usecase:
> 1. the crtc support layer number will 4+2
> 2. ovl0_2l background color input select ovl0 when crtc init
> and disable it when crtc finish
> 3. config ovl0_2l layer, if crtc config layer number is
> bigger than ovl0 support layers(max is 4)
>
> Signed-off-by: Yongqiang Niu
> ---
> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 38
> +++--
> 1 file changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 11e3404..0f97ee3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -283,6 +283,13 @@ static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc
> *mtk_crtc)
>
> for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
> struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[i];
> + enum mtk_ddp_comp_id prev;
> +
> + if (i > 0)
> + prev = mtk_crtc->ddp_comp[i - 1]->id;
> + else
> + prev = DDP_COMPONENT_ID_MAX;
> + mtk_ddp_comp_bgclr_in_on(comp, prev);
>
> mtk_ddp_comp_config(comp, width, height, vrefresh, bpc);
> mtk_ddp_comp_start(comp);
> @@ -292,9 +299,18 @@ static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc
> *mtk_crtc)
> for (i = 0; i < mtk_crtc->layer_nr; i++) {
> struct drm_plane *plane = &mtk_crtc->planes[i];
> struct mtk_plane_state *plane_state;
> + struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0];
> + unsigned int comp_layer_nr = mtk_ddp_comp_layer_nr(comp);
> + unsigned int local_layer;
>
> plane_state = to_mtk_plane_state(plane->state);
> - mtk_ddp_comp_layer_config(mtk_crtc->ddp_comp[0], i,
> +
> + if (i >= comp_layer_nr) {
> + comp = mtk_crtc->ddp_comp[1];
> + local_layer = i - comp_layer_nr;
> +} else
> + local_layer = i;
> + mtk_ddp_comp_layer_config(comp , local_layer,
> plane_state);
> }
>
> @@ -320,6 +336,7 @@ static void mtk_crtc_ddp_hw_fini(struct mtk_drm_crtc
> *mtk_crtc)
> mtk_crtc->ddp_comp[i]->id);
> mtk_disp_mutex_disable(mtk_crtc->mutex);
> for (i = 0; i < mtk_crtc->ddp_comp_nr - 1; i++) {
> + mtk_ddp_comp_bgclr_in_off(mtk_crtc->ddp_comp[i]);
> mtk_ddp_remove_comp_from_path(mtk_crtc->config_regs,
> mtk_crtc->mmsys_reg_data,
> mtk_crtc->ddp_comp[i]->id,
> @@ -340,6 +357,8 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
> struct mtk_crtc_state *state = to_mtk_crtc_state(mtk_crtc->base.state);
> struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0];
> unsigned int i;
> + unsigned int comp_layer_nr = mtk_ddp_comp_layer_nr(comp);
> + unsigned int local_layer;
>
> /*
>* TODO: instead of updating the registers here, we should prepare
> @@ -362,7 +381,14 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
> plane_state = to_mtk_plane_state(plane->state);
>
> if (plane_state->pending.config) {
> - mtk_ddp_comp_layer_config(comp, i, plane_state);
> + if (i >= comp_layer_nr) {
> + comp = mtk_crtc->ddp_comp[1];
> + local_layer = i - comp_layer_nr;
> +} else
> + local_layer = i;
> +
> + mtk_ddp_comp_layer_config(comp, local_layer,
> + plane_state);
> plane_state->pending.config = false;
> }
> }
> @@ -604,6 +630,14 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev,
> }
>
> mtk_crtc->layer_nr = mtk_ddp_comp_layer_nr(mtk_crtc->ddp_comp[0]);
> +if (mtk_crtc->ddp_comp_nr > 1) {
> + struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[1];
> + enum mtk_ddp_comp_type comp_type;
> +
> + comp_type = mtk_ddp_comp_get_type(comp->id);
> + if (comp_type == MTK_DISP_OVL || comp_type == MTK_DISP_OVL_2L)
I think you need not to invent the function mtk_ddp_comp_get_type() to
get the type. Only the component implementing bgclr_in_on() has the
ability to the extend the ovl layer, so you could judge bgclr_in_on()
exist or not to extend the ovl layer.
Regards,
CK
> + mtk_crtc->layer_nr += mtk_ddp_comp_lay
Re: [PATCH v2 22/25] drm/mediatek: adjust ddp clock control flow
Hi, Yongqiang:
On Wed, 2019-03-27 at 14:19 +0800, yongqiang@mediatek.com wrote:
> From: Yongqiang Niu
>
> display hardware clock will not unprepare when
> crtc is disable, until crtc is destroyed.
> with this patch, hard clock will disable and unprepare
> at the same time.
Reviewed-by: CK Hu
>
> Signed-off-by: Yongqiang Niu
> ---
> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 26 ++
> 1 file changed, 6 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 0f97ee3..606c6e2 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -195,7 +195,7 @@ static int mtk_crtc_ddp_clk_enable(struct mtk_drm_crtc
> *mtk_crtc)
>
> DRM_DEBUG_DRIVER("%s\n", __func__);
> for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
> - ret = clk_enable(mtk_crtc->ddp_comp[i]->clk);
> + ret = clk_prepare_enable(mtk_crtc->ddp_comp[i]->clk);
> if (ret) {
> DRM_ERROR("Failed to enable clock %d: %d\n", i, ret);
> goto err;
> @@ -205,7 +205,7 @@ static int mtk_crtc_ddp_clk_enable(struct mtk_drm_crtc
> *mtk_crtc)
> return 0;
> err:
> while (--i >= 0)
> - clk_disable(mtk_crtc->ddp_comp[i]->clk);
> + clk_disable_unprepare(mtk_crtc->ddp_comp[i]->clk);
> return ret;
> }
>
> @@ -215,7 +215,7 @@ static void mtk_crtc_ddp_clk_disable(struct mtk_drm_crtc
> *mtk_crtc)
>
> DRM_DEBUG_DRIVER("%s\n", __func__);
> for (i = 0; i < mtk_crtc->ddp_comp_nr; i++)
> - clk_disable(mtk_crtc->ddp_comp[i]->clk);
> + clk_disable_unprepare(mtk_crtc->ddp_comp[i]->clk);
> }
>
> static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc)
> @@ -615,15 +615,7 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev,
> if (!comp) {
> dev_err(dev, "Component %pOF not initialized\n", node);
> ret = -ENODEV;
> - goto unprepare;
> - }
> -
> - ret = clk_prepare(comp->clk);
> - if (ret) {
> - dev_err(dev,
> - "Failed to prepare clock for component %pOF:
> %d\n",
> - node, ret);
> - goto unprepare;
> + return ret;
> }
>
> mtk_crtc->ddp_comp[i] = comp;
> @@ -649,23 +641,17 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev,
> ret = mtk_plane_init(drm_dev, &mtk_crtc->planes[zpos],
>BIT(pipe), type);
> if (ret)
> - goto unprepare;
> + return ret;
> }
>
> ret = mtk_drm_crtc_init(drm_dev, mtk_crtc, &mtk_crtc->planes[0],
> mtk_crtc->layer_nr > 1 ? &mtk_crtc->planes[1] :
> NULL, pipe);
> if (ret < 0)
> - goto unprepare;
> + return ret;
> drm_mode_crtc_set_gamma_size(&mtk_crtc->base, MTK_LUT_SIZE);
> drm_crtc_enable_color_mgmt(&mtk_crtc->base, 0, false, MTK_LUT_SIZE);
> priv->num_pipes++;
>
> return 0;
> -
> -unprepare:
> - while (--i >= 0)
> - clk_unprepare(mtk_crtc->ddp_comp[i]->clk);
> -
> - return ret;
> }
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 23/25] drm/mediatek: add vmap support for mediatek drm
Hi, Yongqiang:
On Wed, 2019-03-27 at 14:19 +0800, yongqiang@mediatek.com wrote:
> From: Yongqiang Niu
>
> This patch add add vmap support for mediatek drm
I think the upstreamed patch 'drm/mediatek: Implement gem prime
vmap/vunmap function' [1] has the same function of this patch. You could
cherry-pick that patch to your local and need not to upstream this
patch.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/mediatek?h=v5.1-rc5&id=3df64d7b0a4f70f1797f23cfd4cca5c4d48131fe
Regards,
CK
>
> Signed-off-by: Yongqiang Niu
> ---
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 +
> drivers/gpu/drm/mediatek/mtk_drm_gem.c | 7 +++
> drivers/gpu/drm/mediatek/mtk_drm_gem.h | 1 +
> 3 files changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 84c6626..942789d 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -370,6 +370,7 @@ static void mtk_drm_kms_deinit(struct drm_device *drm)
> .gem_prime_get_sg_table = mtk_gem_prime_get_sg_table,
> .gem_prime_import_sg_table = mtk_gem_prime_import_sg_table,
> .gem_prime_mmap = mtk_drm_gem_mmap_buf,
> + .gem_prime_vmap = mtk_drm_gem_vmap_buf,
> .fops = &mtk_drm_fops,
>
> .name = DRIVER_NAME,
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> index 259b7b0..c230237 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> @@ -165,6 +165,13 @@ int mtk_drm_gem_mmap_buf(struct drm_gem_object *obj,
> struct vm_area_struct *vma)
> return mtk_drm_gem_object_mmap(obj, vma);
> }
>
> +void *mtk_drm_gem_vmap_buf(struct drm_gem_object *obj)
> +{
> + struct mtk_drm_gem_obj *mtk_gem = to_mtk_gem_obj(obj);
> +
> + return mtk_gem->cookie;
> +}
> +
> int mtk_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> {
> struct drm_gem_object *obj;
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_gem.h
> b/drivers/gpu/drm/mediatek/mtk_drm_gem.h
> index 534639b..3c48af9 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.h
> @@ -49,6 +49,7 @@ int mtk_drm_gem_dumb_create(struct drm_file *file_priv,
> struct drm_device *dev,
> int mtk_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
> int mtk_drm_gem_mmap_buf(struct drm_gem_object *obj,
>struct vm_area_struct *vma);
> +void *mtk_drm_gem_vmap_buf(struct drm_gem_object *obj);
> struct sg_table *mtk_gem_prime_get_sg_table(struct drm_gem_object *obj);
> struct drm_gem_object *mtk_gem_prime_import_sg_table(struct drm_device *dev,
> struct dma_buf_attachment *attach, struct sg_table *sg);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 06/12] drm/fb-helper: Remove drm_fb_helper_crtc
On Sun, Apr 07, 2019 at 06:52:37PM +0200, Noralf Trønnes wrote:
> It now only contains the modeset so use that directly instead and attach
> a modeset array to drm_client_dev. drm_fb_helper will use this array.
> Code will later be moved to drm_client, so add code there in a new file
> drm_client_modeset.c with MIT license to match drm_fb_helper.c.
>
> The modeset connector array size is hardcoded for the cloned case to avoid
> having to pass in a value from the driver. A value of 8 is chosen to err
> on the safe side. This means that the max connector argument for
> drm_fb_helper_init() and drm_fb_helper_fbdev_setup() isn't used anymore,
> a todo entry for this is added.
>
> In pan_display_atomic() restore_fbdev_mode_force() is used instead of
> restore_fbdev_mode_atomic() because that one will later become internal
> to drm_client_modeset.
>
> Locking order:
> 1. drm_fb_helper->lock
> 2. drm_master_internal_acquire
> 3. drm_client_dev->modeset_mutex
>
> v2:
> - Add modesets array to drm_client (Daniel Vetter)
> - Use a new file for the modeset code (Daniel Vetter)
> - File has to be MIT licensed (Emmanuel Vadot)
> - Add copyrights from drm_fb_helper.c
>
> Signed-off-by: Noralf Trønnes
> ---
> Documentation/gpu/todo.rst | 7 +
> drivers/gpu/drm/Makefile | 2 +-
> drivers/gpu/drm/drm_client.c | 10 +-
> drivers/gpu/drm/drm_client_modeset.c | 102 +
> drivers/gpu/drm/drm_fb_helper.c | 301 +++
> include/drm/drm_client.h | 28 +++
> include/drm/drm_fb_helper.h | 8 -
> 7 files changed, 272 insertions(+), 186 deletions(-)
> create mode 100644 drivers/gpu/drm/drm_client_modeset.c
>
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 1528ad2d598b..8fa08b5feab7 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -300,6 +300,13 @@ it to use drm_mode_hsync() instead.
>
> Contact: Sean Paul
>
> +drm_fb_helper cleanup tasks
> +---
> +
> +- The max connector argument for drm_fb_helper_init() and
> + drm_fb_helper_fbdev_setup() isn't used anymore and can be removed.
> +
> +
> Core refactorings
> =
>
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 6c7e8d162b4e..08c77c10ccbb 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -19,7 +19,7 @@ drm-y := drm_auth.o drm_bufs.o drm_cache.o \
> drm_plane.o drm_color_mgmt.o drm_print.o \
> drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \
> drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o \
> - drm_atomic_uapi.o
> + drm_client_modeset.o drm_atomic_uapi.o
>
> drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
> drm-$(CONFIG_DRM_VM) += drm_vm.o
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index 9b2bd28dde0a..3ad5b57c1419 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -91,14 +91,20 @@ int drm_client_init(struct drm_device *dev, struct
> drm_client_dev *client,
> client->name = name;
> client->funcs = funcs;
>
> - ret = drm_client_open(client);
> + ret = drm_client_modeset_create(client);
> if (ret)
> goto err_put_module;
>
> + ret = drm_client_open(client);
> + if (ret)
> + goto err_free;
> +
> drm_dev_get(dev);
>
> return 0;
>
> +err_free:
> + drm_client_modeset_free(client);
> err_put_module:
> if (funcs)
> module_put(funcs->owner);
> @@ -147,6 +153,8 @@ void drm_client_release(struct drm_client_dev *client)
>
> DRM_DEV_DEBUG_KMS(dev->dev, "%s\n", client->name);
>
> + drm_client_modeset_release(client);
> + drm_client_modeset_free(client);
> drm_client_close(client);
> drm_dev_put(dev);
> if (client->funcs)
> diff --git a/drivers/gpu/drm/drm_client_modeset.c
> b/drivers/gpu/drm/drm_client_modeset.c
> new file mode 100644
> index ..bb34222c9db8
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -0,0 +1,102 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright 2018 Noralf Trønnes
> + * Copyright (c) 2006-2009 Red Hat Inc.
> + * Copyright (c) 2006-2008 Intel Corporation
> + * Jesse Barnes
> + * Copyright (c) 2007 Dave Airlie
> + */
> +
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +
> +/* Used by drm_client and drm_fb_helper */
> +int drm_client_modeset_create(struct drm_client_dev *client)
> +{
> + struct drm_device *dev = client->dev;
> + unsigned int num_crtc = dev->mode_config.num_crtc;
> + unsigned int max_connector_count = 1;
> + struct drm_mode_set *modeset;
> + struct drm_crtc *crtc;
> + unsigned int i = 0;
> +
> + /* Add terminating zero entry to enable index less iteration */
> + client->modesets = kcalloc(num_cr
Re: [PATCH v2 24/25] drm/mediatek: respect page offset for PRIME mmap calls
Hi, Yongqiang: On Wed, 2019-03-27 at 14:19 +0800, yongqiang@mediatek.com wrote: > From: Yongqiang Niu > > Respect page offset for PRIME mmap calls Reviewed-by: CK Hu > > Signed-off-by: Yongqiang Niu > --- > drivers/gpu/drm/mediatek/mtk_drm_gem.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_gem.c > b/drivers/gpu/drm/mediatek/mtk_drm_gem.c > index c230237..524e494 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c > @@ -144,7 +144,6 @@ static int mtk_drm_gem_object_mmap(struct drm_gem_object > *obj, >* VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap(). >*/ > vma->vm_flags &= ~VM_PFNMAP; > - vma->vm_pgoff = 0; > > ret = dma_mmap_attrs(priv->dma_dev, vma, mtk_gem->cookie, >mtk_gem->dma_addr, obj->size, mtk_gem->dma_attrs); > @@ -183,6 +182,12 @@ int mtk_drm_gem_mmap(struct file *filp, struct > vm_area_struct *vma) > > obj = vma->vm_private_data; > > + /* > + * Set vm_pgoff (used as a fake buffer offset by DRM) to 0 and map the > + * whole buffer from the start. > + */ > + vma->vm_pgoff = 0; > + > return mtk_drm_gem_object_mmap(obj, vma); > } > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 08/12] drm/fb-helper: Move out commit code
On Sun, Apr 07, 2019 at 06:52:39PM +0200, Noralf Trønnes wrote:
> Move the modeset commit code to drm_client_modeset.
> No changes except exporting API.
>
> v2: Move to drm_client_modeset.c instead of drm_client.c
>
> Signed-off-by: Noralf Trønnes
> ---
> drivers/gpu/drm/drm_client_modeset.c | 287 +++
Please add a new chapter in Documentation/gpu to pull in all the nice new
kerneldoc you've typed.
> drivers/gpu/drm/drm_fb_helper.c | 282 --
> include/drm/drm_client.h | 4 +
> 3 files changed, 291 insertions(+), 282 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client_modeset.c
> b/drivers/gpu/drm/drm_client_modeset.c
> index bb34222c9db8..94b52f97c06b 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -11,9 +11,14 @@
> #include
> #include
>
> +#include
> #include
> #include
> #include
> +#include
> +
> +#include "drm_crtc_internal.h"
> +#include "drm_internal.h"
>
> /* Used by drm_client and drm_fb_helper */
> int drm_client_modeset_create(struct drm_client_dev *client)
> @@ -100,3 +105,285 @@ struct drm_mode_set *drm_client_find_modeset(struct
> drm_client_dev *client, stru
> }
> /* TODO: Remove export when modeset code has been moved over */
> EXPORT_SYMBOL(drm_client_find_modeset);
> +
> +/**
> + * drm_client_panel_rotation() - Check panel orientation
> + * @modeset: DRM modeset
> + * @rotation: Returned rotation value
> + *
> + * This function checks if the primary plane in @modeset can hw rotate to
> match
> + * the panel orientation on its connector.
> + *
> + * Note: Currently only 0 and 180 degrees are supported.
> + *
> + * Return:
> + * True if the plane can do the rotation, false otherwise.
> + */
> +bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int
> *rotation)
> +{
> + struct drm_connector *connector = modeset->connectors[0];
> + struct drm_plane *plane = modeset->crtc->primary;
> + u64 valid_mask = 0;
> + unsigned int i;
> +
> + if (!modeset->num_connectors)
> + return false;
> +
> + switch (connector->display_info.panel_orientation) {
> + case DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP:
> + *rotation = DRM_MODE_ROTATE_180;
> + break;
> + case DRM_MODE_PANEL_ORIENTATION_LEFT_UP:
> + *rotation = DRM_MODE_ROTATE_90;
> + break;
> + case DRM_MODE_PANEL_ORIENTATION_RIGHT_UP:
> + *rotation = DRM_MODE_ROTATE_270;
> + break;
> + default:
> + *rotation = DRM_MODE_ROTATE_0;
> + }
> +
> + /*
> + * TODO: support 90 / 270 degree hardware rotation,
> + * depending on the hardware this may require the framebuffer
> + * to be in a specific tiling format.
> + */
> + if (*rotation != DRM_MODE_ROTATE_180 || !plane->rotation_property)
> + return false;
> +
> + for (i = 0; i < plane->rotation_property->num_values; i++)
> + valid_mask |= (1ULL << plane->rotation_property->values[i]);
> +
> + if (!(*rotation & valid_mask))
> + return false;
> +
> + return true;
> +}
> +
> +static int drm_client_modeset_commit_atomic(struct drm_client_dev *client,
> bool active)
> +{
> + struct drm_device *dev = client->dev;
> + struct drm_plane_state *plane_state;
> + struct drm_plane *plane;
> + struct drm_atomic_state *state;
> + struct drm_modeset_acquire_ctx ctx;
> + struct drm_mode_set *mode_set;
> + int ret;
> +
> + drm_modeset_acquire_init(&ctx, 0);
> +
> + state = drm_atomic_state_alloc(dev);
> + if (!state) {
> + ret = -ENOMEM;
> + goto out_ctx;
> + }
> +
> + state->acquire_ctx = &ctx;
> +retry:
> + drm_for_each_plane(plane, dev) {
> + plane_state = drm_atomic_get_plane_state(state, plane);
> + if (IS_ERR(plane_state)) {
> + ret = PTR_ERR(plane_state);
> + goto out_state;
> + }
> +
> + plane_state->rotation = DRM_MODE_ROTATE_0;
> +
> + /* disable non-primary: */
> + if (plane->type == DRM_PLANE_TYPE_PRIMARY)
> + continue;
> +
> + ret = __drm_atomic_helper_disable_plane(plane, plane_state);
> + if (ret != 0)
> + goto out_state;
> + }
> +
> + drm_client_for_each_modeset(mode_set, client) {
> + struct drm_plane *primary = mode_set->crtc->primary;
> + unsigned int rotation;
> +
> + if (drm_client_panel_rotation(mode_set, &rotation)) {
> + /* Cannot fail as we've already gotten the plane state
> above */
> + plane_state = drm_atomic_get_new_plane_state(state,
> primary);
> + plane_state->rotation = rotation;
> + }
> +
> + ret = __drm_atomic_helper_set_config(mode_set, st
Re: [PATCH v2 00/12] drm/fb-helper: Move modesetting code to drm_client
On Sun, Apr 07, 2019 at 06:52:31PM +0200, Noralf Trønnes wrote:
> This moves the modesetting code from drm_fb_helper to drm_client so it
> can be shared by all internal clients.
>
> The main change this time is to attach the modeset array to
> drm_client_dev and honour the drm_fb_helper MIT license. I've dropped
> the display abstraction.
>
> Noralf.
>
> Cc: Emmanuel Vadot
>
> Noralf Trønnes (12):
> drm/atomic: Move __drm_atomic_helper_disable_plane/set_config()
> drm/fb-helper: Avoid race with DRM userspace
> drm/i915/fbdev: Move intel_fb_initial_config() to fbdev helper
> drm/fb-helper: No need to cache rotation and sw_rotations
> drm/fb-helper: Remove drm_fb_helper_crtc->{x,y,desired_mode}
> drm/fb-helper: Remove drm_fb_helper_crtc
> drm/fb-helper: Prepare to move out commit code
> drm/fb-helper: Move out commit code
> drm/fb-helper: Remove drm_fb_helper_connector
> drm/fb-helper: Prepare to move out modeset config code
> drm/fb-helper: Move out modeset config code
> drm/client: Hack: Add bootsplash example
I like.
Reviewed some of the prep patches, plus some more suggestions for
drm_client_modeset api polishing ideas.
Maxime is working on some other fbdev helper features and your two patch
series will conflict badly I think. Probably best if you coordinate and
cross-review for final details and best coordination for merging into
drm-misc-next.
I think for the bootsplash good option would be to add it as a todo item,
with a link to patch of your latest proof of concept.
Cheers, Daniel
>
> Documentation/gpu/todo.rst | 10 +
> drivers/gpu/drm/Kconfig |5 +
> drivers/gpu/drm/Makefile |3 +-
> drivers/gpu/drm/drm_atomic.c | 168
> drivers/gpu/drm/drm_atomic_helper.c | 164
> drivers/gpu/drm/drm_auth.c | 20 +
> drivers/gpu/drm/drm_bootsplash.c | 359
> drivers/gpu/drm/drm_client.c | 17 +-
> drivers/gpu/drm/drm_client_modeset.c | 1086 +++
> drivers/gpu/drm/drm_crtc_internal.h |5 +
> drivers/gpu/drm/drm_drv.c|4 +
> drivers/gpu/drm/drm_fb_helper.c | 1195 +++---
> drivers/gpu/drm/drm_internal.h |2 +
> drivers/gpu/drm/i915/intel_fbdev.c | 218 -
> include/drm/drm_atomic_helper.h |4 -
> include/drm/drm_client.h | 48 ++
> include/drm/drm_fb_helper.h | 125 +--
> 17 files changed, 1859 insertions(+), 1574 deletions(-)
> create mode 100644 drivers/gpu/drm/drm_bootsplash.c
> create mode 100644 drivers/gpu/drm/drm_client_modeset.c
>
> --
> 2.20.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 00/12] drm/fb-helper: Move modesetting code to drm_client
On Tue, Apr 16, 2019 at 10:41:06AM +0200, Daniel Vetter wrote:
> On Sun, Apr 07, 2019 at 06:52:31PM +0200, Noralf Trønnes wrote:
> > This moves the modesetting code from drm_fb_helper to drm_client so it
> > can be shared by all internal clients.
> >
> > The main change this time is to attach the modeset array to
> > drm_client_dev and honour the drm_fb_helper MIT license. I've dropped
> > the display abstraction.
> >
> > Noralf.
> >
> > Cc: Emmanuel Vadot
> >
> > Noralf Trønnes (12):
> > drm/atomic: Move __drm_atomic_helper_disable_plane/set_config()
> > drm/fb-helper: Avoid race with DRM userspace
> > drm/i915/fbdev: Move intel_fb_initial_config() to fbdev helper
> > drm/fb-helper: No need to cache rotation and sw_rotations
> > drm/fb-helper: Remove drm_fb_helper_crtc->{x,y,desired_mode}
> > drm/fb-helper: Remove drm_fb_helper_crtc
> > drm/fb-helper: Prepare to move out commit code
> > drm/fb-helper: Move out commit code
> > drm/fb-helper: Remove drm_fb_helper_connector
> > drm/fb-helper: Prepare to move out modeset config code
> > drm/fb-helper: Move out modeset config code
> > drm/client: Hack: Add bootsplash example
>
> I like.
>
> Reviewed some of the prep patches, plus some more suggestions for
> drm_client_modeset api polishing ideas.
>
> Maxime is working on some other fbdev helper features and your two patch
> series will conflict badly I think. Probably best if you coordinate and
> cross-review for final details and best coordination for merging into
> drm-misc-next.
https://marc.info/?l=linux-arm-kernel&m=155498898611173&w=2
is what I meant.
-Daniel
>
> I think for the bootsplash good option would be to add it as a todo item,
> with a link to patch of your latest proof of concept.
>
> Cheers, Daniel
>
> >
> > Documentation/gpu/todo.rst | 10 +
> > drivers/gpu/drm/Kconfig |5 +
> > drivers/gpu/drm/Makefile |3 +-
> > drivers/gpu/drm/drm_atomic.c | 168
> > drivers/gpu/drm/drm_atomic_helper.c | 164
> > drivers/gpu/drm/drm_auth.c | 20 +
> > drivers/gpu/drm/drm_bootsplash.c | 359
> > drivers/gpu/drm/drm_client.c | 17 +-
> > drivers/gpu/drm/drm_client_modeset.c | 1086 +++
> > drivers/gpu/drm/drm_crtc_internal.h |5 +
> > drivers/gpu/drm/drm_drv.c|4 +
> > drivers/gpu/drm/drm_fb_helper.c | 1195 +++---
> > drivers/gpu/drm/drm_internal.h |2 +
> > drivers/gpu/drm/i915/intel_fbdev.c | 218 -
> > include/drm/drm_atomic_helper.h |4 -
> > include/drm/drm_client.h | 48 ++
> > include/drm/drm_fb_helper.h | 125 +--
> > 17 files changed, 1859 insertions(+), 1574 deletions(-)
> > create mode 100644 drivers/gpu/drm/drm_bootsplash.c
> > create mode 100644 drivers/gpu/drm/drm_client_modeset.c
> >
> > --
> > 2.20.1
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [v3 6/7] drm: Add Client Cap for advance gamma mode
mån 2019-04-15 klockan 15:43 +0300 skrev Ville Syrjälä: > On Mon, Apr 15, 2019 at 10:57:52AM +, Lankhorst, Maarten wrote: > > fre 2019-04-12 klockan 15:51 +0530 skrev Uma Shankar: > > > Introduced a client cap for advance cap mode > > > capability. Userspace should set this to get > > > to be able to use the new gamma_mode property. > > > > > > If this is not set, driver will work in legacy > > > mode. > > > > > > Suggested-by: Ville Syrjälä > > > Signed-off-by: Uma Shankar > > > > Nack, this doesn't seem like a sensible idea. We already guard it > > behind the gamma mode property. Userspace shouldn't set the gamma > > mode > > to a value it doesn't understand. > > Old userspace wouldn't know what a gamma_mode prop is. While a client > cap isn't an entirely satisfactory solution I can't think of a better > way at the moment. > > Well, maybe the old "hey kernel, please reset all my props to some > sane default" idea could be another way to deal with this sort of > stuff. Yes, but this approach wouldn't work, lot of other properties that can cause problems when not reset, like plane alpha and blend mode. I don't see why gamma is special in that case. If userspace did set it to some special value, then presumably it knows how to reset it too. It would be different if the split gamma mode was the default. Then I would understand this, but right now this would even break s/r. ~Maarten smime.p7s Description: S/MIME cryptographic signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] arm64: dts: mt8183: add pwm0 node
Add pwm0 node to the mt8183
Signed-off-by: Jitao Shi
---
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 11 +++
1 file changed, 11 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 80929a0e5a6f..2830008c4921 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -323,6 +323,17 @@
status = "disabled";
};
+ pwm0: pwm@1100e000 {
+ compatible = "mediatek,mt8183-disp-pwm";
+ reg = <0 0x1100e000 0 0x1000>;
+ interrupts = ;
+ power-domains = <&scpsys MT8183_POWER_DOMAIN_DISP>;
+ #pwm-cells = <2>;
+ clocks = <&topckgen CLK_TOP_MUX_DISP_PWM>,
+ <&infracfg CLK_INFRA_DISP_PWM>;
+ clock-names = "main", "mm";
+ };
+
audiosys: syscon@1122 {
compatible = "mediatek,mt8183-audiosys", "syscon";
reg = <0 0x1122 0 0x1000>;
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] arm64: dts: mt8183: add dsi node
Add dsi and mipitx nodes to the mt8183
Signed-off-by: Jitao Shi
---
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 25
1 file changed, 25 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index b36e37fcdfe3..80929a0e5a6f 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -353,6 +353,16 @@
status = "disabled";
};
+ mipi_tx0: mipi-dphy@11e5 {
+ compatible = "mediatek,mt8183-mipi-tx";
+ reg = <0 0x11e5 0 0x1000>;
+ clocks = <&apmixedsys CLK_APMIXED_MIPID0_26M>;
+ clock-names = "ref_clk";
+ #clock-cells = <0>;
+ #phy-cells = <0>;
+ clock-output-names = "mipi_tx0_pll";
+ };
+
mfgcfg: syscon@1300 {
compatible = "mediatek,mt8183-mfgcfg", "syscon";
reg = <0 0x1300 0 0x1000>;
@@ -365,6 +375,21 @@
#clock-cells = <1>;
};
+ dsi0: dsi@14014000 {
+ compatible = "mediatek,mt8173-dsi",
+ "mediatek,mt8183-dsi";
+ reg = <0 0x14014000 0 0x1000>;
+ interrupts = ;
+ power-domains = <&scpsys MT8183_POWER_DOMAIN_DISP>;
+ mediatek,syscon-dsi = <&mmsys 0x140>;
+ clocks = <&mmsys CLK_MM_DSI0_MM>,
+ <&mmsys CLK_MM_DSI0_IF>,
+ <&mipi_tx0>;
+ clock-names = "engine", "digital", "hs";
+ phys = <&mipi_tx0>;
+ phy-names = "dphy";
+ };
+
smi_common: smi@14019000 {
compatible = "mediatek,mt8183-smi-common", "syscon";
reg = <0 0x14019000 0 0x1000>;
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/2] add dsi and pwm0 nodes to mt8183 device tree
This patch is based on v5.1-rc1 and these patches: http://lists.infradead.org/pipermail/linux-mediatek/2019-March/017963.html https://patchwork.kernel.org/patch/10856987/ https://patchwork.kernel.org/cover/10879001/ https://patchwork.kernel.org/cover/10846677/ https://patchwork.kernel.org/patch/10893519/ Jitao Shi (2): arm64: dts: mt8183: add dsi node arm64: dts: mt8183: add pwm0 node arch/arm64/boot/dts/mediatek/mt8183.dtsi | 36 1 file changed, 36 insertions(+) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 05/13] drivers: create binary sysfs for class
On Mon, Apr 15, 2019 at 09:14:12PM +0200, Daniel Vetter wrote: > On Mon, Apr 15, 2019 at 8:01 PM Greg Kroah-Hartman > wrote: > > On Mon, Apr 15, 2019 at 10:44:12PM +0530, Ramalingam C wrote: > > > On 2019-04-15 at 16:47:16 +0200, Greg Kroah-Hartman wrote: > > > > On Mon, Apr 15, 2019 at 06:11:13PM +0530, Ramalingam C wrote: > > > > > On 2019-04-05 at 14:32:00 +0200, Greg Kroah-Hartman wrote: > > > > > > On Fri, Apr 05, 2019 at 04:06:22PM +0530, Ramalingam C wrote: > > > > > > > On 2019-04-05 at 11:23:00 +0200, Greg Kroah-Hartman wrote: > > > > > > > > On Fri, Apr 05, 2019 at 02:12:54PM +0530, Ramalingam C wrote: > > > > > > > > > Functions to create and remove the binary sysfs for class are > > > > > > > > > added. > > > > > > > > > > > > > > > > > > These are getting introduced as DRM wants to create the > > > > > > > > > common binary > > > > > > > > > sysfs across the drm subsystem to handle hdcp srm. > > > > > > > > > > > > > > > > Why do you need individual files? That's almost always a sign > > > > > > > > that you > > > > > > > > are going to race with userspace in a bad way. Why not just > > > > > > > > use an > > > > > > > > attribute group which provides automatic support for this? > > > > > > > Greg, > > > > > > > > > > > > > > Reason behind this move is to have a common srm entry path across > > > > > > > all drm > > > > > > > drivers. And the data fed into this is binary blob. So I am > > > > > > > creating a > > > > > > > binary sysfs "hdcp_srm" at /sys/class/drm/ > > > > > > > > > > > > Ah, you want to have a file in your class directory, not your class > > > > > > device directory. > > > > > > > > > > > > No, please do not do that. There's a reason I got rid of those same > > > > > > types of apis in the past. > > > > > > > > > > > > And "binary blobs" are horrid anyway, they are only to be used as a > > > > > > pass-through to the device itself, from the kernel, no touching the > > > > > > data > > > > > > at all. If you really need/want this, then put it in the device's > > > > > > directory as that is where the data is going to, not the kernel > > > > > > "class" > > > > > > code as it sure as heck better not be doing anything with it. > > > > > Greg, > > > > > > > > > > But here the parsing of the received binary blob is done outside the > > > > > drm > > > > > device/cards. This will be generic code for all drm cardx(drivers). > > > > > And > > > > > this will provide the service helper functions to the drm drivers for > > > > > HDCP SRM checking. > > > > > > > > Again, the kernel is NOT to be parsing any binary data that comes > > > > through a sysfs file. If you need such a crazy thing, do it through > > > > your normal drm ioctl. > > > > > > > > > So we prefer to have the binary sysfs at /sys/class/drm/ than inside > > > > > the > > > > > cardx folders, so that it will be generic. Could you please suggest a > > > > > way to achieve that? > > > > > > > > What is a "cardx" driver? > > > Meant card0 card1 etc.. Drm drivers. > > > > Why can you not do it in a device-specific > > > > location? Are suddenly _ALL_ DRM drivers going to need this > > > > information? What is the use case? Who is going to be providing this > > > > blob and where is it going? What in the kernel uses it? What on the > > > > hardware uses it? What is it actually? > > > > > > This is for HDCP usecase. List of compromised receivers' ID called system > > > renewability Message (SRM) will be available at userspace which needs to > > > be passed to be kernel. And this data can be parsed at kernel used > > > across all DRM drivers to implement the HDCP authentication. Hence we > > > want generic code to parse the SRM data and provide a helper function to > > > all DRM drivers to validate their HDCP sink's ID. > > > > > > To achieve this we want to keep the sysfs and parsing logic outside the > > > drm drivers at the class level. > > > > > > At present I915 will be using these implementation. in future other DRM > > > drivers can use the same. > > > > Again, binary sysfs files are not for any data that the kernel has to > > parse at all. So do not use sysfs for this. > > So what's the recommend thing then? > - requrest_firmware blob could work, aside that the blob might change > (the point of this to allow updates). For some hw you can even stuff > the SRM into some eprom iirc, so not entirely misfit. Plus kernel > parses it, so not sure request_firmware is really the right thing. I think it is, as this really looks like "firmware" to me as you are wanting to do something with the buffer and pass it on. > - configfs? I never used that one, but the transactional config stuff > this provides feels like silly amounts of overkill Maybe, but why do you want to stuf binary data in your kernel in the first place? It is configuration, right, why not have it in a format that you can easily handle it in? Or is that the binary format you have? > - procfs :-) Hah, no. > - ioctl it definitely isn
[PATCH] drm: add drm_format_helper.c to kerneldoc
Also drop the dstclip parameter sphinx has warned about (leftolver from an earlier patch version). Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/drm_format_helper.c | 1 - Documentation/gpu/drm-kms-helpers.rst | 6 ++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index da388012df2a..a18da35145b7 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -235,7 +235,6 @@ static void drm_fb_xrgb_to_rgb888_line(u8 *dbuf, u32 *sbuf, * @vaddr: XRGB source buffer * @fb: DRM framebuffer * @clip: Clip rectangle area to copy - * @dstclip: Clip destination too. * * Drivers can use this function for RGB888 devices that don't natively * support XRGB. diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst index 58b375e47615..14102ae035dc 100644 --- a/Documentation/gpu/drm-kms-helpers.rst +++ b/Documentation/gpu/drm-kms-helpers.rst @@ -107,6 +107,12 @@ fbdev Helper Functions Reference .. kernel-doc:: drivers/gpu/drm/drm_fb_helper.c :export: +format Helper Functions Reference += + +.. kernel-doc:: drivers/gpu/drm/drm_format_helper.c + :export: + Framebuffer CMA Helper Functions Reference == -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/6] Fix crash after reloading a driver using ttm
On Tue, Apr 16, 2019 at 8:38 AM Christian König wrote: > > Am 16.04.19 um 02:35 schrieb Karol Herbst: > > Kobjects are supposed to be dynamically allocated, but with recent changes > > this rule was violated. Reverting those commits fixes crashes when a drm > > driver using TTM gets loaded again. > > > > The object in question is "ttm_mem_glob" declared inside > > "include/drm/ttm/ttm_memory.h" and instatiated inside > > "drivers/gpu/drm/ttm/ttm_memory.c". > > > > from "Documentation/kobject.txt": > > "Because kobjects are dynamic, they must not be declared statically or on > > the stack, but instead, always allocated dynamically. Future versions of > > the kernel will contain a run-time check for kobjects that are created > > statically and will warn the developer of this improper usage." > > > > Unloading ttm before reloading the driver workarounds that crash, because > > the memory backing the kobject member "kobj" is cleaned up. The kobject_del > > and kobject_put function never free or clean up the kobject object leaving > > it in an undefined state. > > > > I reverted a few more commits to make it less painful for me to rever this > > rather big change. > > Well, NAK. By reverting those change you also re-introduced the problems > we originally fixed with those patches. > > Please work on a proper fix instead, > Christian. And which problem was that besides duplicated code (or maybe even a bit of memory consumption if multiple ttm driver were used)? If I had to choose between duplicated code and a crash, I choose the former. Maybe I missed the real reason why those changes are made, but the commit messages don't really seem to tell me. > > > > > dmesg output: > > [54758.418036] kobject (687a067d): tried to init an initialized > > object, something is seriously wrong. > > [54758.418040] CPU: 6 PID: 26746 Comm: insmod Tainted: G U OE > > 5.0.6-200.fc29.x86_64 #1 > > [54758.418041] Hardware name: Dell Inc. XPS 15 9560/05FFDN, BIOS 1.12.1 > > 10/02/2018 > > [54758.418041] Call Trace: > > [54758.418049] dump_stack+0x5c/0x80 > > [54758.418054] kobject_init.cold.9+0x31/0x3f > > [54758.418057] kobject_init_and_add+0x35/0xa0 > > [54758.418063] ttm_mem_global_init+0x8f/0x2b0 [ttm] > > [54758.418067] ? __debugfs_create_file+0xe1/0x110 > > [54758.418071] ttm_bo_device_init+0x198/0x2a0 [ttm] > > [54758.418144] nouveau_ttm_init+0xbf/0x340 [nouveau] > > [54758.418206] nouveau_drm_device_init+0x125/0x7d0 [nouveau] > > [54758.418210] ? pci_bus_read_config_word+0x49/0x70 > > [54758.418266] nouveau_drm_probe+0x26f/0x2c0 [nouveau] > > [54758.418270] local_pci_probe+0x41/0x90 > > [54758.418272] pci_device_probe+0x118/0x1a0 > > [54758.418275] really_probe+0xf8/0x3b0 > > [54758.418277] driver_probe_device+0xb3/0xf0 > > [54758.418278] __driver_attach+0xdd/0x110 > > [54758.418280] ? driver_probe_device+0xf0/0xf0 > > [54758.418282] bus_for_each_dev+0x77/0xc0 > > [54758.418285] ? klist_add_tail+0x3b/0x60 > > [54758.418287] bus_add_driver+0x152/0x230 > > [54758.418288] ? 0xc1027000 > > [54758.418290] driver_register+0x6b/0xb0 > > [54758.418291] ? 0xc1027000 > > [54758.418294] do_one_initcall+0x46/0x1c3 > > [54758.418296] ? _cond_resched+0x15/0x30 > > [54758.418299] ? kmem_cache_alloc_trace+0x154/0x1d0 > > [54758.418302] do_init_module+0x5a/0x210 > > [54758.418304] load_module+0x2096/0x22d0 > > [54758.418308] ? ima_post_read_file+0xf4/0x100 > > [54758.418310] ? __do_sys_finit_module+0xa8/0x110 > > [54758.418312] __do_sys_finit_module+0xa8/0x110 > > [54758.418315] do_syscall_64+0x5b/0x160 > > [54758.418317] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > [54758.418319] RIP: 0033:0x7fc0b38b6edd > > [54758.418321] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 > > 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 > > <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 7b 7f 0c 00 f7 d8 64 89 01 48 > > [54758.418322] RSP: 002b:7ffc7f1620d8 EFLAGS: 0246 ORIG_RAX: > > 0139 > > [54758.418323] RAX: ffda RBX: 5629a3996ac0 RCX: > > 7fc0b38b6edd > > [54758.418324] RDX: RSI: 5629a3996260 RDI: > > 0003 > > [54758.418325] RBP: 5629a3996260 R08: R09: > > > > [54758.418326] R10: 0003 R11: 0246 R12: > > > > [54758.418326] R13: 5629a3996a80 R14: R15: > > 5629a3996260 > > [54758.418346] BUG: unable to handle kernel paging request at > > 0004ee194700 > > [54758.418348] #PF error: [WRITE] > > [54758.418349] PGD 0 P4D 0 > > [54758.418352] Oops: 0002 [#1] SMP PTI > > [54758.418354] CPU: 6 PID: 26746 Comm: insmod Tainted: G U OE > > 5.0.6-200.fc29.x86_64 #1 > > [54758.418355] Hardware name: Dell Inc. XPS 15 9560/05FFDN, BIOS 1.12.1 > > 10/02/2018 > > [54758.418360] RIP: 0010:ttm_mem_global_init+0x1fe/0x2b0 [ttm] > > [54758.418361] Code: 00 00 00 48 89 5d 40 48
Re: [PATCH v2 01/12] drm/atomic: Move __drm_atomic_helper_disable_plane/set_config()
On Sun, Apr 07, 2019 at 06:52:32PM +0200, Noralf Trønnes wrote: > Prepare for moving drm_fb_helper modesetting code to drm_client. > drm_client will be linked to drm.ko, so move > __drm_atomic_helper_disable_plane() and __drm_atomic_helper_set_config() > out of drm_kms_helper.ko. > > While at it, fix two checkpatch complaints: > - WARNING: Block comments use a trailing */ on a separate line > - CHECK: Alignment should match open parenthesis > > Signed-off-by: Noralf Trønnes > Reviewed-by: Daniel Vetter Reviewed-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/6] Fix crash after reloading a driver using ttm
Am 16.04.19 um 11:10 schrieb Karol Herbst: > On Tue, Apr 16, 2019 at 8:38 AM Christian König > wrote: >> Am 16.04.19 um 02:35 schrieb Karol Herbst: >>> Kobjects are supposed to be dynamically allocated, but with recent changes >>> this rule was violated. Reverting those commits fixes crashes when a drm >>> driver using TTM gets loaded again. >>> >>> The object in question is "ttm_mem_glob" declared inside >>> "include/drm/ttm/ttm_memory.h" and instatiated inside >>> "drivers/gpu/drm/ttm/ttm_memory.c". >>> >>> from "Documentation/kobject.txt": >>> "Because kobjects are dynamic, they must not be declared statically or on >>> the stack, but instead, always allocated dynamically. Future versions of >>> the kernel will contain a run-time check for kobjects that are created >>> statically and will warn the developer of this improper usage." >>> >>> Unloading ttm before reloading the driver workarounds that crash, because >>> the memory backing the kobject member "kobj" is cleaned up. The kobject_del >>> and kobject_put function never free or clean up the kobject object leaving >>> it in an undefined state. >>> >>> I reverted a few more commits to make it less painful for me to rever this >>> rather big change. >> Well, NAK. By reverting those change you also re-introduced the problems >> we originally fixed with those patches. >> >> Please work on a proper fix instead, >> Christian. > And which problem was that besides duplicated code (or maybe even a > bit of memory consumption if multiple ttm driver were used)? If I had > to choose between duplicated code and a crash, I choose the former. > > Maybe I missed the real reason why those changes are made, but the > commit messages don't really seem to tell me. The old implementation crashed because different drivers tried to allocate the same kobj. Crashing in one way is not better than crashing in another way. Christian. > >>> dmesg output: >>> [54758.418036] kobject (687a067d): tried to init an initialized >>> object, something is seriously wrong. >>> [54758.418040] CPU: 6 PID: 26746 Comm: insmod Tainted: G U OE >>> 5.0.6-200.fc29.x86_64 #1 >>> [54758.418041] Hardware name: Dell Inc. XPS 15 9560/05FFDN, BIOS 1.12.1 >>> 10/02/2018 >>> [54758.418041] Call Trace: >>> [54758.418049] dump_stack+0x5c/0x80 >>> [54758.418054] kobject_init.cold.9+0x31/0x3f >>> [54758.418057] kobject_init_and_add+0x35/0xa0 >>> [54758.418063] ttm_mem_global_init+0x8f/0x2b0 [ttm] >>> [54758.418067] ? __debugfs_create_file+0xe1/0x110 >>> [54758.418071] ttm_bo_device_init+0x198/0x2a0 [ttm] >>> [54758.418144] nouveau_ttm_init+0xbf/0x340 [nouveau] >>> [54758.418206] nouveau_drm_device_init+0x125/0x7d0 [nouveau] >>> [54758.418210] ? pci_bus_read_config_word+0x49/0x70 >>> [54758.418266] nouveau_drm_probe+0x26f/0x2c0 [nouveau] >>> [54758.418270] local_pci_probe+0x41/0x90 >>> [54758.418272] pci_device_probe+0x118/0x1a0 >>> [54758.418275] really_probe+0xf8/0x3b0 >>> [54758.418277] driver_probe_device+0xb3/0xf0 >>> [54758.418278] __driver_attach+0xdd/0x110 >>> [54758.418280] ? driver_probe_device+0xf0/0xf0 >>> [54758.418282] bus_for_each_dev+0x77/0xc0 >>> [54758.418285] ? klist_add_tail+0x3b/0x60 >>> [54758.418287] bus_add_driver+0x152/0x230 >>> [54758.418288] ? 0xc1027000 >>> [54758.418290] driver_register+0x6b/0xb0 >>> [54758.418291] ? 0xc1027000 >>> [54758.418294] do_one_initcall+0x46/0x1c3 >>> [54758.418296] ? _cond_resched+0x15/0x30 >>> [54758.418299] ? kmem_cache_alloc_trace+0x154/0x1d0 >>> [54758.418302] do_init_module+0x5a/0x210 >>> [54758.418304] load_module+0x2096/0x22d0 >>> [54758.418308] ? ima_post_read_file+0xf4/0x100 >>> [54758.418310] ? __do_sys_finit_module+0xa8/0x110 >>> [54758.418312] __do_sys_finit_module+0xa8/0x110 >>> [54758.418315] do_syscall_64+0x5b/0x160 >>> [54758.418317] entry_SYSCALL_64_after_hwframe+0x44/0xa9 >>> [54758.418319] RIP: 0033:0x7fc0b38b6edd >>> [54758.418321] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 >>> 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 >>> <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 7b 7f 0c 00 f7 d8 64 89 01 48 >>> [54758.418322] RSP: 002b:7ffc7f1620d8 EFLAGS: 0246 ORIG_RAX: >>> 0139 >>> [54758.418323] RAX: ffda RBX: 5629a3996ac0 RCX: >>> 7fc0b38b6edd >>> [54758.418324] RDX: RSI: 5629a3996260 RDI: >>> 0003 >>> [54758.418325] RBP: 5629a3996260 R08: R09: >>> >>> [54758.418326] R10: 0003 R11: 0246 R12: >>> >>> [54758.418326] R13: 5629a3996a80 R14: R15: >>> 5629a3996260 >>> [54758.418346] BUG: unable to handle kernel paging request at >>> 0004ee194700 >>> [54758.418348] #PF error: [WRITE] >>> [54758.418349] PGD 0 P4D 0 >>> [54758.418352] Oops: 0002 [#1] SMP PTI >>> [54758.418354] CPU: 6 PID: 26746 Comm: insmod Tainted: G U OE >>> 5.0.6-20
Re: [v2,1/2] drm: Add writeback_w,h properties
On Tue, Apr 16, 2019 at 09:34:20AM +0200, Daniel Vetter wrote:
> On Mon, Apr 15, 2019 at 10:20:45AM +0100, Liviu Dudau wrote:
> > On Mon, Apr 15, 2019 at 08:59:30AM +0100, james qian wang (Arm Technology
> > China) wrote:
> > > On Fri, Apr 12, 2019 at 02:08:28PM +, Ben Davis wrote:
> > > > Add new properties to specify width and height for writeback.
> > > >
> > > > Signed-off-by: Ben Davis
> > > > ---
> > > > drivers/gpu/drm/drm_atomic_uapi.c | 8
> > > > drivers/gpu/drm/drm_writeback.c | 28
> > > > include/drm/drm_connector.h | 4
> > > > include/drm/drm_mode_config.h | 10 ++
> > > > 4 files changed, 50 insertions(+)
> > > >
> > > > --
> > > > 2.7.4
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> > > > b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > index d520a04..1f68dce 100644
> > > > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > @@ -765,6 +765,10 @@ static int
> > > > drm_atomic_connector_set_property(struct drm_connector *connector,
> > > > return -EINVAL;
> > > > }
> > > > state->content_protection = val;
> > > > + } else if (property == config->prop_writeback_w) {
> > > > + state->writeback_w = val;
> > > > + } else if (property == config->prop_writeback_h) {
> > > > + state->writeback_h = val;
> > > > } else if (property == config->writeback_fb_id_property) {
> > > > struct drm_framebuffer *fb =
> > > > drm_framebuffer_lookup(dev, NULL, val);
> > > > int ret =
> > > > drm_atomic_set_writeback_fb_for_connector(state, fb);
> > > > @@ -837,6 +841,10 @@ drm_atomic_connector_get_property(struct
> > > > drm_connector *connector,
> > > > *val = state->scaling_mode;
> > > > } else if (property == connector->content_protection_property) {
> > > > *val = state->content_protection;
> > > > + } else if (property == config->prop_writeback_w) {
> > > > + *val = state->writeback_w;
> > > > + } else if (property == config->prop_writeback_h) {
> > > > + *val = state->writeback_h;
> > > > } else if (property == config->writeback_fb_id_property) {
> > > > /* Writeback framebuffer is one-shot, write and forget
> > > > */
> > > > *val = 0;
> > > > diff --git a/drivers/gpu/drm/drm_writeback.c
> > > > b/drivers/gpu/drm/drm_writeback.c
> > > > index c20e6fe..3d0453e 100644
> > > > --- a/drivers/gpu/drm/drm_writeback.c
> > > > +++ b/drivers/gpu/drm/drm_writeback.c
> > > > @@ -74,6 +74,12 @@
> > > > * applications making use of writeback connectors *always*
> > > > retrieve an
> > > > * out-fence for the commit and use it appropriately.
> > > > * From userspace, this property will always read as zero.
> > > > + *
> > > > + * "WRITEBACK_W":
> > > > + * The width of the writeback buffer to write back. 0 acts as
> > > > default.
> > > > + *
> > > > + * "WRITEBACK_H":
> > > > + * The height of the writeback buffer to write back. 0 acts as
> > > > default.
> > > > */
> > > >
> > > > #define fence_to_wb_connector(x) container_of(x->lock, \
> > > > @@ -141,6 +147,22 @@ static int create_writeback_properties(struct
> > > > drm_device *dev)
> > > > dev->mode_config.writeback_out_fence_ptr_property =
> > > > prop;
> > > > }
> > > >
> > > > + if (!dev->mode_config.prop_writeback_w) {
> > > > + prop = drm_property_create_range(dev,
> > > > DRM_MODE_PROP_ATOMIC,
> > > > +"WRITEBACK_W", 1,
> > > > UINT_MAX);
> > > > + if (!prop)
> > > > + return -ENOMEM;
> > > > + dev->mode_config.prop_writeback_w = prop;
> > > > + }
> > > > +
> > > > + if (!dev->mode_config.prop_writeback_h) {
> > > > + prop = drm_property_create_range(dev,
> > > > DRM_MODE_PROP_ATOMIC,
> > > > +"WRITEBACK_H", 1,
> > > > UINT_MAX);
> > > > + if (!prop)
> > > > + return -ENOMEM;
> > > > + dev->mode_config.prop_writeback_h = prop;
> > > > + }
> > > > +
> > > > return 0;
> > > > }
> > > >
> > > > @@ -225,6 +247,12 @@ int drm_writeback_connector_init(struct drm_device
> > > > *dev,
> > > > drm_object_attach_property(&connector->base,
> > > >
> > > > config->writeback_pixel_formats_property,
> > > >blob->base.id);
> > > > +
> > > > + drm_object_attach_property(&connector->base,
> > > > + config->prop_writeback_w, 0);
> > > > + drm_object_attach_property(&connector->base,
> > > > + config->prop_writeba
Re: [PATCH v4 05/13] drivers: create binary sysfs for class
On Tue, Apr 16, 2019 at 11:04 AM Greg Kroah-Hartman wrote: > On Mon, Apr 15, 2019 at 09:14:12PM +0200, Daniel Vetter wrote: > > On Mon, Apr 15, 2019 at 8:01 PM Greg Kroah-Hartman > > wrote: > > > On Mon, Apr 15, 2019 at 10:44:12PM +0530, Ramalingam C wrote: > > > > On 2019-04-15 at 16:47:16 +0200, Greg Kroah-Hartman wrote: > > > > > On Mon, Apr 15, 2019 at 06:11:13PM +0530, Ramalingam C wrote: > > > > > > On 2019-04-05 at 14:32:00 +0200, Greg Kroah-Hartman wrote: > > > > > > > On Fri, Apr 05, 2019 at 04:06:22PM +0530, Ramalingam C wrote: > > > > > > > > On 2019-04-05 at 11:23:00 +0200, Greg Kroah-Hartman wrote: > > > > > > > > > On Fri, Apr 05, 2019 at 02:12:54PM +0530, Ramalingam C wrote: > > > > > > > > > > Functions to create and remove the binary sysfs for class > > > > > > > > > > are added. > > > > > > > > > > > > > > > > > > > > These are getting introduced as DRM wants to create the > > > > > > > > > > common binary > > > > > > > > > > sysfs across the drm subsystem to handle hdcp srm. > > > > > > > > > > > > > > > > > > Why do you need individual files? That's almost always a > > > > > > > > > sign that you > > > > > > > > > are going to race with userspace in a bad way. Why not just > > > > > > > > > use an > > > > > > > > > attribute group which provides automatic support for this? > > > > > > > > Greg, > > > > > > > > > > > > > > > > Reason behind this move is to have a common srm entry path > > > > > > > > across all drm > > > > > > > > drivers. And the data fed into this is binary blob. So I am > > > > > > > > creating a > > > > > > > > binary sysfs "hdcp_srm" at /sys/class/drm/ > > > > > > > > > > > > > > Ah, you want to have a file in your class directory, not your > > > > > > > class > > > > > > > device directory. > > > > > > > > > > > > > > No, please do not do that. There's a reason I got rid of those > > > > > > > same > > > > > > > types of apis in the past. > > > > > > > > > > > > > > And "binary blobs" are horrid anyway, they are only to be used as > > > > > > > a > > > > > > > pass-through to the device itself, from the kernel, no touching > > > > > > > the data > > > > > > > at all. If you really need/want this, then put it in the device's > > > > > > > directory as that is where the data is going to, not the kernel > > > > > > > "class" > > > > > > > code as it sure as heck better not be doing anything with it. > > > > > > Greg, > > > > > > > > > > > > But here the parsing of the received binary blob is done outside > > > > > > the drm > > > > > > device/cards. This will be generic code for all drm cardx(drivers). > > > > > > And > > > > > > this will provide the service helper functions to the drm drivers > > > > > > for HDCP SRM checking. > > > > > > > > > > Again, the kernel is NOT to be parsing any binary data that comes > > > > > through a sysfs file. If you need such a crazy thing, do it through > > > > > your normal drm ioctl. > > > > > > > > > > > So we prefer to have the binary sysfs at /sys/class/drm/ than > > > > > > inside the > > > > > > cardx folders, so that it will be generic. Could you please suggest > > > > > > a way to achieve that? > > > > > > > > > > What is a "cardx" driver? > > > > Meant card0 card1 etc.. Drm drivers. > > > > > Why can you not do it in a device-specific > > > > > location? Are suddenly _ALL_ DRM drivers going to need this > > > > > information? What is the use case? Who is going to be providing this > > > > > blob and where is it going? What in the kernel uses it? What on the > > > > > hardware uses it? What is it actually? > > > > > > > > This is for HDCP usecase. List of compromised receivers' ID called > > > > system > > > > renewability Message (SRM) will be available at userspace which needs to > > > > be passed to be kernel. And this data can be parsed at kernel used > > > > across all DRM drivers to implement the HDCP authentication. Hence we > > > > want generic code to parse the SRM data and provide a helper function to > > > > all DRM drivers to validate their HDCP sink's ID. > > > > > > > > To achieve this we want to keep the sysfs and parsing logic outside the > > > > drm drivers at the class level. > > > > > > > > At present I915 will be using these implementation. in future other DRM > > > > drivers can use the same. > > > > > > Again, binary sysfs files are not for any data that the kernel has to > > > parse at all. So do not use sysfs for this. > > > > So what's the recommend thing then? > > - requrest_firmware blob could work, aside that the blob might change > > (the point of this to allow updates). For some hw you can even stuff > > the SRM into some eprom iirc, so not entirely misfit. Plus kernel > > parses it, so not sure request_firmware is really the right thing. > > I think it is, as this really looks like "firmware" to me as you are > wanting to do something with the buffer and pass it on. > > > - configfs? I never used that one, but the transactional config stuff > > this provides fe
Re: [PATCH v2 02/12] drm/fb-helper: Avoid race with DRM userspace
On Sun, Apr 07, 2019 at 06:52:33PM +0200, Noralf Trønnes wrote: > drm_fb_helper_is_bound() is used to check if DRM userspace is in control. > This is done by looking at the fb on the primary plane. By the time > fb-helper gets around to committing, it's possible that the facts have > changed. > > Avoid this race by holding the drm_device->master_mutex lock while > committing. When DRM userspace does its first open, it will now wait > until fb-helper is done. The helper will stay away if there's a master. > > Locking rule: Always take the fb-helper lock first. > > v2: > - Remove drm_fb_helper_is_bound() (Daniel Vetter) > - No need to check fb_helper->dev->master in > drm_fb_helper_single_fb_probe(), restore_fbdev_mode() has the check. > > Suggested-by: Daniel Vetter > Signed-off-by: Noralf Trønnes With the changes asked by Daniel, Reviewed-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 04/12] drm/fb-helper: No need to cache rotation and sw_rotations
On Sun, Apr 07, 2019 at 06:52:35PM +0200, Noralf Trønnes wrote: > Getting rotation info is cheap so we can do it on demand. > > This is done in preparation for the removal of struct drm_fb_helper_crtc. > > Cc: Hans de Goede > Signed-off-by: Noralf Trønnes > Acked-by: Daniel Vetter Reviewed-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 05/12] drm/fb-helper: Remove drm_fb_helper_crtc->{x, y, desired_mode}
On Sun, Apr 07, 2019 at 06:52:36PM +0200, Noralf Trønnes wrote: > The values are already present in the modeset. > > This is done in preparation for the removal of struct drm_fb_helper_crtc. > > Signed-off-by: Noralf Trønnes > Reviewed-by: Daniel Vetter Reviewed-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 09/12] drm/fb-helper: Remove drm_fb_helper_connector
Hi, On Sun, Apr 07, 2019 at 06:52:40PM +0200, Noralf Trønnes wrote: > All drivers add all their connectors so there's no need to keep around an > array of available connectors. > > Rename functions which signature is changed since they will be moved to > drm_client in a later patch. > > Signed-off-by: Noralf Trønnes The patch itself looks fine in itself, but I was planning on using connector_info to store the connector properties set on the kernel command line as part of video= Where should we put them now? Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 11/12] drm/fb-helper: Move out modeset config code
On Sun, Apr 07, 2019 at 06:52:42PM +0200, Noralf Trønnes wrote: > No functional changes, just moving code as-is and fixing includes. > > Signed-off-by: Noralf Trønnes Reviewed-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 10/12] drm/fb-helper: Prepare to move out modeset config code
On Sun, Apr 07, 2019 at 06:52:41PM +0200, Noralf Trønnes wrote: > This prepares the modeset code so it can be moved out as-is in the next > patch. > > Signed-off-by: Noralf Trønnes Reviewed-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 1/5] drm/scheduler: rework job destruction
Am 15.04.19 um 23:17 schrieb Eric Anholt: Andrey Grodzovsky writes: From: Christian König We now destroy finished jobs from the worker thread to make sure that we never destroy a job currently in timeout processing. By this we avoid holding lock around ring mirror list in drm_sched_stop which should solve a deadlock reported by a user. v2: Remove unused variable. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109692 Signed-off-by: Christian König Signed-off-by: Andrey Grodzovsky --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 17 ++-- drivers/gpu/drm/etnaviv/etnaviv_dump.c | 4 - drivers/gpu/drm/etnaviv/etnaviv_sched.c| 9 +- drivers/gpu/drm/scheduler/sched_main.c | 138 + drivers/gpu/drm/v3d/v3d_sched.c| 9 +- Missing corresponding panfrost and lima updates. You should probably pull in drm-misc for hacking on the scheduler. diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c index ce7c737b..8efb091 100644 --- a/drivers/gpu/drm/v3d/v3d_sched.c +++ b/drivers/gpu/drm/v3d/v3d_sched.c @@ -232,11 +232,18 @@ v3d_gpu_reset_for_timeout(struct v3d_dev *v3d, struct drm_sched_job *sched_job) /* block scheduler */ for (q = 0; q < V3D_MAX_QUEUES; q++) - drm_sched_stop(&v3d->queue[q].sched); + drm_sched_stop(&v3d->queue[q].sched, sched_job); if(sched_job) drm_sched_increase_karma(sched_job); + /* +* Guilty job did complete and hence needs to be manually removed +* See drm_sched_stop doc. +*/ + if (list_empty(&sched_job->node)) + sched_job->sched->ops->free_job(sched_job); If the if (sched_job) is necessary up above, then this should clearly be under it. But, can we please have a core scheduler thing we call here instead of drivers all replicating it? Yeah that's also something I noted before. Essential problem is that we remove finished jobs from the mirror list and so need to destruct them because we otherwise leak them. Alternative approach here would be to keep the jobs on the ring mirror list, but not submit them again. Regards, Christian. + /* get the GPU back into the init state */ v3d_reset(v3d); ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: turn syncobj timeline support off by default
On 15/04/2019 21:23, Dave Airlie wrote: On Tue, 16 Apr 2019 at 06:05, Lionel Landwerlin wrote: On 15/04/2019 20:52, Dave Airlie wrote: On Tue, 16 Apr 2019 at 05:48, Lionel Landwerlin wrote: Unfortunately userspace users of this API cannot be publicly disclosed yet so disable this stuff by default until all is revealed. This begs the question how userspace is meant to know we support these? Is there a CAP for it? if not why not? Dave. This comes with a submission path, so in i915 I've added a CAP. AMD seems to have a versioned interface. I think we would do like we did for syncobjs, you have a generic cap that the driver controls. The versioned interface won't be useful if we decide to backport a fix for this. Dave. Okay, but then it's not a global setting anymore :) Which is what was suggested on IRC. I'm fine with it regardless :) -Lionel -Lionel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v2,1/2] drm: Add writeback_w,h properties
On Tue, Apr 16, 2019 at 11:17 AM Liviu Dudau wrote:
>
> On Tue, Apr 16, 2019 at 09:34:20AM +0200, Daniel Vetter wrote:
> > On Mon, Apr 15, 2019 at 10:20:45AM +0100, Liviu Dudau wrote:
> > > On Mon, Apr 15, 2019 at 08:59:30AM +0100, james qian wang (Arm Technology
> > > China) wrote:
> > > > On Fri, Apr 12, 2019 at 02:08:28PM +, Ben Davis wrote:
> > > > > Add new properties to specify width and height for writeback.
> > > > >
> > > > > Signed-off-by: Ben Davis
> > > > > ---
> > > > > drivers/gpu/drm/drm_atomic_uapi.c | 8
> > > > > drivers/gpu/drm/drm_writeback.c | 28
> > > > > include/drm/drm_connector.h | 4
> > > > > include/drm/drm_mode_config.h | 10 ++
> > > > > 4 files changed, 50 insertions(+)
> > > > >
> > > > > --
> > > > > 2.7.4
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > index d520a04..1f68dce 100644
> > > > > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > @@ -765,6 +765,10 @@ static int
> > > > > drm_atomic_connector_set_property(struct drm_connector *connector,
> > > > > return -EINVAL;
> > > > > }
> > > > > state->content_protection = val;
> > > > > + } else if (property == config->prop_writeback_w) {
> > > > > + state->writeback_w = val;
> > > > > + } else if (property == config->prop_writeback_h) {
> > > > > + state->writeback_h = val;
> > > > > } else if (property == config->writeback_fb_id_property) {
> > > > > struct drm_framebuffer *fb =
> > > > > drm_framebuffer_lookup(dev, NULL, val);
> > > > > int ret =
> > > > > drm_atomic_set_writeback_fb_for_connector(state, fb);
> > > > > @@ -837,6 +841,10 @@ drm_atomic_connector_get_property(struct
> > > > > drm_connector *connector,
> > > > > *val = state->scaling_mode;
> > > > > } else if (property ==
> > > > > connector->content_protection_property) {
> > > > > *val = state->content_protection;
> > > > > + } else if (property == config->prop_writeback_w) {
> > > > > + *val = state->writeback_w;
> > > > > + } else if (property == config->prop_writeback_h) {
> > > > > + *val = state->writeback_h;
> > > > > } else if (property == config->writeback_fb_id_property) {
> > > > > /* Writeback framebuffer is one-shot, write and
> > > > > forget */
> > > > > *val = 0;
> > > > > diff --git a/drivers/gpu/drm/drm_writeback.c
> > > > > b/drivers/gpu/drm/drm_writeback.c
> > > > > index c20e6fe..3d0453e 100644
> > > > > --- a/drivers/gpu/drm/drm_writeback.c
> > > > > +++ b/drivers/gpu/drm/drm_writeback.c
> > > > > @@ -74,6 +74,12 @@
> > > > > * applications making use of writeback connectors *always*
> > > > > retrieve an
> > > > > * out-fence for the commit and use it appropriately.
> > > > > * From userspace, this property will always read as zero.
> > > > > + *
> > > > > + * "WRITEBACK_W":
> > > > > + * The width of the writeback buffer to write back. 0 acts as
> > > > > default.
> > > > > + *
> > > > > + * "WRITEBACK_H":
> > > > > + * The height of the writeback buffer to write back. 0 acts as
> > > > > default.
> > > > > */
> > > > >
> > > > > #define fence_to_wb_connector(x) container_of(x->lock, \
> > > > > @@ -141,6 +147,22 @@ static int create_writeback_properties(struct
> > > > > drm_device *dev)
> > > > > dev->mode_config.writeback_out_fence_ptr_property =
> > > > > prop;
> > > > > }
> > > > >
> > > > > + if (!dev->mode_config.prop_writeback_w) {
> > > > > + prop = drm_property_create_range(dev,
> > > > > DRM_MODE_PROP_ATOMIC,
> > > > > +"WRITEBACK_W", 1,
> > > > > UINT_MAX);
> > > > > + if (!prop)
> > > > > + return -ENOMEM;
> > > > > + dev->mode_config.prop_writeback_w = prop;
> > > > > + }
> > > > > +
> > > > > + if (!dev->mode_config.prop_writeback_h) {
> > > > > + prop = drm_property_create_range(dev,
> > > > > DRM_MODE_PROP_ATOMIC,
> > > > > +"WRITEBACK_H", 1,
> > > > > UINT_MAX);
> > > > > + if (!prop)
> > > > > + return -ENOMEM;
> > > > > + dev->mode_config.prop_writeback_h = prop;
> > > > > + }
> > > > > +
> > > > > return 0;
> > > > > }
> > > > >
> > > > > @@ -225,6 +247,12 @@ int drm_writeback_connector_init(struct
> > > > > drm_device *dev,
> > > > > drm_object_attach_property(&connector->base,
> > > > >
> > > > > config->writeback_pixel_formats_property,
> > > > >blob->base.id);
Re: [PATCH 00/15] Share TTM code among framebuffer drivers
Am 15.04.19 um 21:17 schrieb Daniel Vetter: > On Mon, Apr 15, 2019 at 6:21 PM Thomas Zimmermann wrote: >> Hi >> >> Am 15.04.19 um 17:54 schrieb Daniel Vetter: >>> On Tue, Apr 09, 2019 at 09:50:40AM +0200, Thomas Zimmermann wrote: Hi Am 09.04.19 um 09:12 schrieb kra...@redhat.com: [SNIP] > I'd expect the same applies to the vbox driver. > > Dunno about the other drm drivers and the fbdev drivers you plan to > migrate over. The AST HW can support up to 512 MiB, but 32-64 MiB seems more realistic for a server. It's similar with mgag200 HW. The old fbdev-supported device are all somewhere in the range between cirrus and bochs. Some drivers would probably benefit from the cirrus approach, some could use VRAM directly. >>> I think for dumb scanout with vram all we need is: >>> - pin framebuffers, which potentially moves the underlying bo into vram >>> - unpin framebuffers (which is just accounting, we don't want to move the >>>bo on every flip!) >>> - if a pin doesn't find enough space, move one of the unpinned bo still >>>resident in vram out >> For dumb buffers, I'd expect userspace to have a working set of only a >> front and back buffer (plus maybe a third one). This working set has to >> reside in VRAM for performance reasons; non-WS BOs from other userspace >> programs don't have to be. >> >> So we could simplify even more: if there's not enough free space in >> vram, remove all unpinned BO's. This would avoid the need to implement >> an LRU algorithm or another eviction strategy. Userspace with a WS >> larger than the absolute VRAM would see degraded performance but >> otherwise still work. > You still need a list of unpinned bo, and the lru scan algorithm is > just a few lines of code more than unpinning everything. Plus it'd be > a neat example of the drm_mm scan logic. Given that some folks might > think that not having lru evict si a problem and get them to type > their own, I'd just add it. But up to you. Plus with ttm you get it no > matter what. Well how about making an drm_lru component which just does the following (and nothing else, please :): 1. Keep a list of objects and a spinlock protecting the list. 2. Offer helpers for adding/deleting/moving stuff from the list. 3. Offer a functionality to do the necessary dance of picking the first entry where we can trylock it's reservation object. 4. Offer bulk move functionality similar to what TTM does at the moment (can be implemented later on). Regards, Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: turn syncobj timeline support off by default
On Tue, Apr 16, 2019 at 11:51 AM Lionel Landwerlin wrote: > > On 15/04/2019 21:23, Dave Airlie wrote: > > On Tue, 16 Apr 2019 at 06:05, Lionel Landwerlin > > wrote: > >> On 15/04/2019 20:52, Dave Airlie wrote: > >>> On Tue, 16 Apr 2019 at 05:48, Lionel Landwerlin > >>> wrote: > Unfortunately userspace users of this API cannot be publicly disclosed > yet so disable this stuff by default until all is revealed. > >>> This begs the question how userspace is meant to know we support these? > >>> > >>> Is there a CAP for it? if not why not? > >>> > >>> Dave. > >>> > >> This comes with a submission path, so in i915 I've added a CAP. > >> > >> AMD seems to have a versioned interface. > > I think we would do like we did for syncobjs, you have a generic cap > > that the driver controls. > > > > The versioned interface won't be useful if we decide to backport a fix for > > this. I guess amdgpu could then backport the version bump. Also, this is why I don't like versioned uapi really. Either way we also need to hide the amdgpu version bump until we've finalized the uapi and khr pushed out the spec. > Okay, but then it's not a global setting anymore :) > > Which is what was suggested on IRC. > > > I'm fine with it regardless :) I think global is good enough. If either i915 or amdgpu screwed up the uapi enough that we can't backport the enabling patch, then maybe not a good idea to do for either. Ofc this assumes the uapi as merged is forward compatible already, i.e. new userspace has a way to figure out whether timeline sync are support or not for a given driver. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/ttm: fix re-init of global structures
When a driver unloads without unloading TTM we don't correctly
clear the global structures leading to errors on re-init.
Next step should probably be to remove the global structures and
kobjs all together, but this is tricky since we need to maintain
backward compatibility.
Signed-off-by: Christian König
---
drivers/gpu/drm/ttm/ttm_bo.c | 10 +-
drivers/gpu/drm/ttm/ttm_memory.c | 5 +++--
include/drm/ttm/ttm_bo_driver.h | 1 -
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 41d07faa2eae..2845fceb2fbd 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -49,9 +49,8 @@ static void ttm_bo_global_kobj_release(struct kobject *kobj);
* ttm_global_mutex - protecting the global BO state
*/
DEFINE_MUTEX(ttm_global_mutex);
-struct ttm_bo_global ttm_bo_glob = {
- .use_count = 0
-};
+unsigned ttm_bo_glob_use_count;
+struct ttm_bo_global ttm_bo_glob;
static struct attribute ttm_bo_count = {
.name = "bo_count",
@@ -1531,12 +1530,13 @@ static void ttm_bo_global_release(void)
struct ttm_bo_global *glob = &ttm_bo_glob;
mutex_lock(&ttm_global_mutex);
- if (--glob->use_count > 0)
+ if (--ttm_bo_glob_use_count > 0)
goto out;
kobject_del(&glob->kobj);
kobject_put(&glob->kobj);
ttm_mem_global_release(&ttm_mem_glob);
+ memset(glob, 0, sizeof(*glob));
out:
mutex_unlock(&ttm_global_mutex);
}
@@ -1548,7 +1548,7 @@ static int ttm_bo_global_init(void)
unsigned i;
mutex_lock(&ttm_global_mutex);
- if (++glob->use_count > 1)
+ if (++ttm_bo_glob_use_count > 1)
goto out;
ret = ttm_mem_global_init(&ttm_mem_glob);
diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
index 699fed9e08ee..8617958b7ae6 100644
--- a/drivers/gpu/drm/ttm/ttm_memory.c
+++ b/drivers/gpu/drm/ttm/ttm_memory.c
@@ -461,8 +461,8 @@ int ttm_mem_global_init(struct ttm_mem_global *glob)
void ttm_mem_global_release(struct ttm_mem_global *glob)
{
- unsigned int i;
struct ttm_mem_zone *zone;
+ unsigned int i;
/* let the page allocator first stop the shrink work. */
ttm_page_alloc_fini();
@@ -475,9 +475,10 @@ void ttm_mem_global_release(struct ttm_mem_global *glob)
zone = glob->zones[i];
kobject_del(&zone->kobj);
kobject_put(&zone->kobj);
- }
+ }
kobject_del(&glob->kobj);
kobject_put(&glob->kobj);
+ memset(glob, 0, sizeof(*glob));
}
static void ttm_check_swapping(struct ttm_mem_global *glob)
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 3db265bf2d4e..c008346c2401 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -420,7 +420,6 @@ extern struct ttm_bo_global {
/**
* Protected by ttm_global_mutex.
*/
- unsigned int use_count;
struct list_head device_list;
/**
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/ttm: remove pointers to globals
As the name says global memory and bo accounting is global. So it doesn't
make to much sense having pointers to global structures all around the code.
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
drivers/gpu/drm/qxl/qxl_release.c| 4 +--
drivers/gpu/drm/qxl/qxl_ttm.c| 2 +-
drivers/gpu/drm/ttm/ttm_agp_backend.c| 2 +-
drivers/gpu/drm/ttm/ttm_bo.c | 38 +++-
drivers/gpu/drm/ttm/ttm_bo_util.c| 2 +-
drivers/gpu/drm/ttm/ttm_bo_vm.c | 4 +--
drivers/gpu/drm/ttm/ttm_execbuf_util.c | 20 -
drivers/gpu/drm/ttm/ttm_memory.c | 2 +-
drivers/gpu/drm/ttm/ttm_page_alloc.c | 4 +--
drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 4 +--
include/drm/ttm/ttm_bo_driver.h | 6 ++--
include/drm/ttm/ttm_memory.h | 1 -
14 files changed, 37 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
index 6d11e1721147..03ec278b2080 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
@@ -68,7 +68,7 @@
*/
static int amdgpu_gart_dummy_page_init(struct amdgpu_device *adev)
{
- struct page *dummy_page = adev->mman.bdev.glob->dummy_read_page;
+ struct page *dummy_page = ttm_bo_glob.dummy_read_page;
if (adev->dummy_page_addr)
return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 56d838fe6633..a376adb426e8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -600,7 +600,7 @@ void amdgpu_vm_del_from_lru_notify(struct ttm_buffer_object
*bo)
void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
struct amdgpu_vm *vm)
{
- struct ttm_bo_global *glob = adev->mman.bdev.glob;
+ struct ttm_bo_global *glob = &ttm_bo_glob;
struct amdgpu_vm_bo_base *bo_base;
if (vm->bulk_moveable) {
diff --git a/drivers/gpu/drm/qxl/qxl_release.c
b/drivers/gpu/drm/qxl/qxl_release.c
index 30f85f0130cb..2b6b0258a459 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -424,8 +424,8 @@ void qxl_release_unmap(struct qxl_device *qdev,
void qxl_release_fence_buffer_objects(struct qxl_release *release)
{
+ struct ttm_bo_global *glob = &ttm_bo_glob;
struct ttm_buffer_object *bo;
- struct ttm_bo_global *glob;
struct ttm_bo_device *bdev;
struct ttm_validate_buffer *entry;
struct qxl_device *qdev;
@@ -447,8 +447,6 @@ void qxl_release_fence_buffer_objects(struct qxl_release
*release)
release->id | 0xf000, release->base.seqno);
trace_dma_fence_emit(&release->base);
- glob = bdev->glob;
-
spin_lock(&glob->lru_lock);
list_for_each_entry(entry, &release->bos, head) {
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 3bc6d53deb57..6225dbdcedc5 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -360,8 +360,8 @@ static int qxl_mm_dump_table(struct seq_file *m, void *data)
struct drm_mm *mm = (struct drm_mm *)node->info_ent->data;
struct drm_device *dev = node->minor->dev;
struct qxl_device *rdev = dev->dev_private;
- struct ttm_bo_global *glob = rdev->mman.bdev.glob;
struct drm_printer p = drm_seq_file_printer(m);
+ struct ttm_bo_global *glob = &ttm_bo_glob;
spin_lock(&glob->lru_lock);
drm_mm_print(mm, &p);
diff --git a/drivers/gpu/drm/ttm/ttm_agp_backend.c
b/drivers/gpu/drm/ttm/ttm_agp_backend.c
index ea4d59eb8966..6050dc846894 100644
--- a/drivers/gpu/drm/ttm/ttm_agp_backend.c
+++ b/drivers/gpu/drm/ttm/ttm_agp_backend.c
@@ -51,7 +51,7 @@ struct ttm_agp_backend {
static int ttm_agp_bind(struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem)
{
struct ttm_agp_backend *agp_be = container_of(ttm, struct
ttm_agp_backend, ttm);
- struct page *dummy_read_page = ttm->bdev->glob->dummy_read_page;
+ struct page *dummy_read_page = ttm_bo_glob.dummy_read_page;
struct drm_mm_node *node = bo_mem->mm_node;
struct agp_memory *mem;
int ret, cached = (bo_mem->placement & TTM_PL_FLAG_CACHED);
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 2845fceb2fbd..e2f9abe3920d 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -51,6 +51,7 @@ static void ttm_bo_global_kobj_release(struct kobject *kobj);
DEFINE_MUTEX(ttm_global_mutex);
unsigned ttm_bo_glob_use_count;
struct ttm_bo_global ttm_bo_glob;
+EXPORT_SYMBOL(ttm_bo_glob);
static struct attribute ttm_bo_count = {
.name = "bo_count",
@@ -148,7 +149,6 @@ static void ttm_bo_release_list(struct kref *list_kref)
{
struct ttm_buffer_
Re: [PATCH 0/6] Fix crash after reloading a driver using ttm
On Tue, Apr 16, 2019 at 11:12 AM Koenig, Christian wrote: > > Am 16.04.19 um 11:10 schrieb Karol Herbst: > > On Tue, Apr 16, 2019 at 8:38 AM Christian König > > wrote: > >> Am 16.04.19 um 02:35 schrieb Karol Herbst: > >>> Kobjects are supposed to be dynamically allocated, but with recent changes > >>> this rule was violated. Reverting those commits fixes crashes when a drm > >>> driver using TTM gets loaded again. > >>> > >>> The object in question is "ttm_mem_glob" declared inside > >>> "include/drm/ttm/ttm_memory.h" and instatiated inside > >>> "drivers/gpu/drm/ttm/ttm_memory.c". > >>> > >>> from "Documentation/kobject.txt": > >>> "Because kobjects are dynamic, they must not be declared statically or on > >>> the stack, but instead, always allocated dynamically. Future versions of > >>> the kernel will contain a run-time check for kobjects that are created > >>> statically and will warn the developer of this improper usage." > >>> > >>> Unloading ttm before reloading the driver workarounds that crash, because > >>> the memory backing the kobject member "kobj" is cleaned up. The > >>> kobject_del > >>> and kobject_put function never free or clean up the kobject object leaving > >>> it in an undefined state. > >>> > >>> I reverted a few more commits to make it less painful for me to rever this > >>> rather big change. > >> Well, NAK. By reverting those change you also re-introduced the problems > >> we originally fixed with those patches. > >> > >> Please work on a proper fix instead, > >> Christian. > > And which problem was that besides duplicated code (or maybe even a > > bit of memory consumption if multiple ttm driver were used)? If I had > > to choose between duplicated code and a crash, I choose the former. > > > > Maybe I missed the real reason why those changes are made, but the > > commit messages don't really seem to tell me. > > The old implementation crashed because different drivers tried to > allocate the same kobj. > > Crashing in one way is not better than crashing in another way. > > Christian. > how can that old crash be triggered? By loading two ttm based drivers at the same time or by other means? > > > >>> dmesg output: > >>> [54758.418036] kobject (687a067d): tried to init an initialized > >>> object, something is seriously wrong. > >>> [54758.418040] CPU: 6 PID: 26746 Comm: insmod Tainted: G U OE > >>> 5.0.6-200.fc29.x86_64 #1 > >>> [54758.418041] Hardware name: Dell Inc. XPS 15 9560/05FFDN, BIOS 1.12.1 > >>> 10/02/2018 > >>> [54758.418041] Call Trace: > >>> [54758.418049] dump_stack+0x5c/0x80 > >>> [54758.418054] kobject_init.cold.9+0x31/0x3f > >>> [54758.418057] kobject_init_and_add+0x35/0xa0 > >>> [54758.418063] ttm_mem_global_init+0x8f/0x2b0 [ttm] > >>> [54758.418067] ? __debugfs_create_file+0xe1/0x110 > >>> [54758.418071] ttm_bo_device_init+0x198/0x2a0 [ttm] > >>> [54758.418144] nouveau_ttm_init+0xbf/0x340 [nouveau] > >>> [54758.418206] nouveau_drm_device_init+0x125/0x7d0 [nouveau] > >>> [54758.418210] ? pci_bus_read_config_word+0x49/0x70 > >>> [54758.418266] nouveau_drm_probe+0x26f/0x2c0 [nouveau] > >>> [54758.418270] local_pci_probe+0x41/0x90 > >>> [54758.418272] pci_device_probe+0x118/0x1a0 > >>> [54758.418275] really_probe+0xf8/0x3b0 > >>> [54758.418277] driver_probe_device+0xb3/0xf0 > >>> [54758.418278] __driver_attach+0xdd/0x110 > >>> [54758.418280] ? driver_probe_device+0xf0/0xf0 > >>> [54758.418282] bus_for_each_dev+0x77/0xc0 > >>> [54758.418285] ? klist_add_tail+0x3b/0x60 > >>> [54758.418287] bus_add_driver+0x152/0x230 > >>> [54758.418288] ? 0xc1027000 > >>> [54758.418290] driver_register+0x6b/0xb0 > >>> [54758.418291] ? 0xc1027000 > >>> [54758.418294] do_one_initcall+0x46/0x1c3 > >>> [54758.418296] ? _cond_resched+0x15/0x30 > >>> [54758.418299] ? kmem_cache_alloc_trace+0x154/0x1d0 > >>> [54758.418302] do_init_module+0x5a/0x210 > >>> [54758.418304] load_module+0x2096/0x22d0 > >>> [54758.418308] ? ima_post_read_file+0xf4/0x100 > >>> [54758.418310] ? __do_sys_finit_module+0xa8/0x110 > >>> [54758.418312] __do_sys_finit_module+0xa8/0x110 > >>> [54758.418315] do_syscall_64+0x5b/0x160 > >>> [54758.418317] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > >>> [54758.418319] RIP: 0033:0x7fc0b38b6edd > >>> [54758.418321] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa > >>> 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f > >>> 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 7b 7f 0c 00 f7 d8 64 89 01 48 > >>> [54758.418322] RSP: 002b:7ffc7f1620d8 EFLAGS: 0246 ORIG_RAX: > >>> 0139 > >>> [54758.418323] RAX: ffda RBX: 5629a3996ac0 RCX: > >>> 7fc0b38b6edd > >>> [54758.418324] RDX: RSI: 5629a3996260 RDI: > >>> 0003 > >>> [54758.418325] RBP: 5629a3996260 R08: R09: > >>> > >>> [54758.418326] R10: 0003 R11: 0246 R12: > >>> > >>> [54758
Re: [PATCH 00/15] Share TTM code among framebuffer drivers
On Tue, Apr 16, 2019 at 12:05 PM Koenig, Christian wrote: > > Am 15.04.19 um 21:17 schrieb Daniel Vetter: > > On Mon, Apr 15, 2019 at 6:21 PM Thomas Zimmermann > > wrote: > >> Hi > >> > >> Am 15.04.19 um 17:54 schrieb Daniel Vetter: > >>> On Tue, Apr 09, 2019 at 09:50:40AM +0200, Thomas Zimmermann wrote: > Hi > > Am 09.04.19 um 09:12 schrieb kra...@redhat.com: > [SNIP] > > I'd expect the same applies to the vbox driver. > > > > Dunno about the other drm drivers and the fbdev drivers you plan to > > migrate over. > The AST HW can support up to 512 MiB, but 32-64 MiB seems more realistic > for a server. It's similar with mgag200 HW. The old fbdev-supported > device are all somewhere in the range between cirrus and bochs. Some > drivers would probably benefit from the cirrus approach, some could use > VRAM directly. > >>> I think for dumb scanout with vram all we need is: > >>> - pin framebuffers, which potentially moves the underlying bo into vram > >>> - unpin framebuffers (which is just accounting, we don't want to move the > >>>bo on every flip!) > >>> - if a pin doesn't find enough space, move one of the unpinned bo still > >>>resident in vram out > >> For dumb buffers, I'd expect userspace to have a working set of only a > >> front and back buffer (plus maybe a third one). This working set has to > >> reside in VRAM for performance reasons; non-WS BOs from other userspace > >> programs don't have to be. > >> > >> So we could simplify even more: if there's not enough free space in > >> vram, remove all unpinned BO's. This would avoid the need to implement > >> an LRU algorithm or another eviction strategy. Userspace with a WS > >> larger than the absolute VRAM would see degraded performance but > >> otherwise still work. > > You still need a list of unpinned bo, and the lru scan algorithm is > > just a few lines of code more than unpinning everything. Plus it'd be > > a neat example of the drm_mm scan logic. Given that some folks might > > think that not having lru evict si a problem and get them to type > > their own, I'd just add it. But up to you. Plus with ttm you get it no > > matter what. > > Well how about making an drm_lru component which just does the following > (and nothing else, please :): > > 1. Keep a list of objects and a spinlock protecting the list. > > 2. Offer helpers for adding/deleting/moving stuff from the list. > > 3. Offer a functionality to do the necessary dance of picking the first > entry where we can trylock it's reservation object. > > 4. Offer bulk move functionality similar to what TTM does at the moment > (can be implemented later on). At a basic level, this is list_head of drm_gem_object. Not sure that's all that useful (outside of the fairly simplistic vram helpers we're discussing here). Reasons for that is that there's a lot of trickery in selecting which is the best object to pick in any given case (e.g. do you want to use drm_mm scanning, or is there a slab of objects you prefer to throw out because that avoids. Given that I'm not sure implementing the entire scanning/drm_lru logic is beneficial. The magic trylock+kref_get_unless_zero otoh could be worth implementing as a helper, together with a note about how to build your own custom lru algorithm. Same for some bulk/nonblocking movement helpers maybe. Both not really needed for the dumb scanout vram helpers we're discussing here. -Daniel -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/6] Fix crash after reloading a driver using ttm
Am 16.04.19 um 12:54 schrieb Karol Herbst: > On Tue, Apr 16, 2019 at 11:12 AM Koenig, Christian > wrote: >> Am 16.04.19 um 11:10 schrieb Karol Herbst: >>> On Tue, Apr 16, 2019 at 8:38 AM Christian König >>> wrote: Am 16.04.19 um 02:35 schrieb Karol Herbst: > Kobjects are supposed to be dynamically allocated, but with recent changes > this rule was violated. Reverting those commits fixes crashes when a drm > driver using TTM gets loaded again. > > The object in question is "ttm_mem_glob" declared inside > "include/drm/ttm/ttm_memory.h" and instatiated inside > "drivers/gpu/drm/ttm/ttm_memory.c". > > from "Documentation/kobject.txt": > "Because kobjects are dynamic, they must not be declared statically or on > the stack, but instead, always allocated dynamically. Future versions of > the kernel will contain a run-time check for kobjects that are created > statically and will warn the developer of this improper usage." > > Unloading ttm before reloading the driver workarounds that crash, because > the memory backing the kobject member "kobj" is cleaned up. The > kobject_del > and kobject_put function never free or clean up the kobject object leaving > it in an undefined state. > > I reverted a few more commits to make it less painful for me to rever this > rather big change. Well, NAK. By reverting those change you also re-introduced the problems we originally fixed with those patches. Please work on a proper fix instead, Christian. >>> And which problem was that besides duplicated code (or maybe even a >>> bit of memory consumption if multiple ttm driver were used)? If I had >>> to choose between duplicated code and a crash, I choose the former. >>> >>> Maybe I missed the real reason why those changes are made, but the >>> commit messages don't really seem to tell me. >> The old implementation crashed because different drivers tried to >> allocate the same kobj. >> >> Crashing in one way is not better than crashing in another way. >> >> Christian. >> > how can that old crash be triggered? By loading two ttm based drivers > at the same time or by other means? Yes, exactly. Even worse it could be triggered by one driver instantiating multiple times at the same time, e.g two AMD GPUs in the same system. On the other hand I completely agree that using kobj static is completely nuts. The problem is that using a kobj was the wrong approach in the first place. In other words when you have something which controls a global behavior of a module, what do you use? Right, a module parameter. Point is that we can't get away from those kobj without breaking the uapi, so that is something which can't be done easily. Regards, Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/15] Share TTM code among framebuffer drivers
Am 16.04.19 um 13:03 schrieb Daniel Vetter: > On Tue, Apr 16, 2019 at 12:05 PM Koenig, Christian > wrote: >> Am 15.04.19 um 21:17 schrieb Daniel Vetter: >>> On Mon, Apr 15, 2019 at 6:21 PM Thomas Zimmermann >>> wrote: Hi Am 15.04.19 um 17:54 schrieb Daniel Vetter: > On Tue, Apr 09, 2019 at 09:50:40AM +0200, Thomas Zimmermann wrote: >> Hi >> >> Am 09.04.19 um 09:12 schrieb kra...@redhat.com: >> [SNIP] >>> I'd expect the same applies to the vbox driver. >>> >>> Dunno about the other drm drivers and the fbdev drivers you plan to >>> migrate over. >> The AST HW can support up to 512 MiB, but 32-64 MiB seems more realistic >> for a server. It's similar with mgag200 HW. The old fbdev-supported >> device are all somewhere in the range between cirrus and bochs. Some >> drivers would probably benefit from the cirrus approach, some could use >> VRAM directly. > I think for dumb scanout with vram all we need is: > - pin framebuffers, which potentially moves the underlying bo into vram > - unpin framebuffers (which is just accounting, we don't want to move the > bo on every flip!) > - if a pin doesn't find enough space, move one of the unpinned bo still > resident in vram out For dumb buffers, I'd expect userspace to have a working set of only a front and back buffer (plus maybe a third one). This working set has to reside in VRAM for performance reasons; non-WS BOs from other userspace programs don't have to be. So we could simplify even more: if there's not enough free space in vram, remove all unpinned BO's. This would avoid the need to implement an LRU algorithm or another eviction strategy. Userspace with a WS larger than the absolute VRAM would see degraded performance but otherwise still work. >>> You still need a list of unpinned bo, and the lru scan algorithm is >>> just a few lines of code more than unpinning everything. Plus it'd be >>> a neat example of the drm_mm scan logic. Given that some folks might >>> think that not having lru evict si a problem and get them to type >>> their own, I'd just add it. But up to you. Plus with ttm you get it no >>> matter what. >> Well how about making an drm_lru component which just does the following >> (and nothing else, please :): >> >> 1. Keep a list of objects and a spinlock protecting the list. >> >> 2. Offer helpers for adding/deleting/moving stuff from the list. >> >> 3. Offer a functionality to do the necessary dance of picking the first >> entry where we can trylock it's reservation object. >> >> 4. Offer bulk move functionality similar to what TTM does at the moment >> (can be implemented later on). > At a basic level, this is list_head of drm_gem_object. Not sure that's > all that useful (outside of the fairly simplistic vram helpers we're > discussing here). Reasons for that is that there's a lot of trickery > in selecting which is the best object to pick in any given case (e.g. > do you want to use drm_mm scanning, or is there a slab of objects you > prefer to throw out because that avoids. Given that I'm not sure > implementing the entire scanning/drm_lru logic is beneficial. > > The magic trylock+kref_get_unless_zero otoh could be worth > implementing as a helper, together with a note about how to build your > own custom lru algorithm. Same for some bulk/nonblocking movement > helpers maybe. Both not really needed for the dumb scanout vram > helpers we're discussing here. Yeah, exactly that's what I wanted to get towards as well. This magic trylock+kref_get is what needs to be handled correctly by all drivers which implement an LRU. LRU bulk move is something which is tricky to get right as well, but so far only amdgpu uses it so it only make sense to share when somebody else wants the same approach. Christian. > -Daniel > > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/6] Fix crash after reloading a driver using ttm
On Tue, Apr 16, 2019 at 1:07 PM Koenig, Christian wrote: > > Am 16.04.19 um 12:54 schrieb Karol Herbst: > > On Tue, Apr 16, 2019 at 11:12 AM Koenig, Christian > > wrote: > >> Am 16.04.19 um 11:10 schrieb Karol Herbst: > >>> On Tue, Apr 16, 2019 at 8:38 AM Christian König > >>> wrote: > Am 16.04.19 um 02:35 schrieb Karol Herbst: > > Kobjects are supposed to be dynamically allocated, but with recent > > changes > > this rule was violated. Reverting those commits fixes crashes when a drm > > driver using TTM gets loaded again. > > > > The object in question is "ttm_mem_glob" declared inside > > "include/drm/ttm/ttm_memory.h" and instatiated inside > > "drivers/gpu/drm/ttm/ttm_memory.c". > > > > from "Documentation/kobject.txt": > > "Because kobjects are dynamic, they must not be declared statically or > > on > > the stack, but instead, always allocated dynamically. Future versions > > of > > the kernel will contain a run-time check for kobjects that are created > > statically and will warn the developer of this improper usage." > > > > Unloading ttm before reloading the driver workarounds that crash, > > because > > the memory backing the kobject member "kobj" is cleaned up. The > > kobject_del > > and kobject_put function never free or clean up the kobject object > > leaving > > it in an undefined state. > > > > I reverted a few more commits to make it less painful for me to rever > > this > > rather big change. > Well, NAK. By reverting those change you also re-introduced the problems > we originally fixed with those patches. > > Please work on a proper fix instead, > Christian. > >>> And which problem was that besides duplicated code (or maybe even a > >>> bit of memory consumption if multiple ttm driver were used)? If I had > >>> to choose between duplicated code and a crash, I choose the former. > >>> > >>> Maybe I missed the real reason why those changes are made, but the > >>> commit messages don't really seem to tell me. > >> The old implementation crashed because different drivers tried to > >> allocate the same kobj. > >> > >> Crashing in one way is not better than crashing in another way. > >> > >> Christian. > >> > > how can that old crash be triggered? By loading two ttm based drivers > > at the same time or by other means? > > Yes, exactly. Even worse it could be triggered by one driver > instantiating multiple times at the same time, e.g two AMD GPUs in the > same system. > > On the other hand I completely agree that using kobj static is > completely nuts. The problem is that using a kobj was the wrong approach > in the first place. > > In other words when you have something which controls a global behavior > of a module, what do you use? Right, a module parameter. > > Point is that we can't get away from those kobj without breaking the > uapi, so that is something which can't be done easily. > > Regards, > Christian. okay, thanks for the explanation! Currently I will be testing your first patch you sent out on top of 5.0.7, so that we could propose that for stable. The second didn't apply, but it didn't look relevant for the actual fix. Will reply to the patch with my results then. Thanks ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next] drm/meson: Make some functions static
Le 13/04/2019 16:14, Yue Haibing a écrit :
> From: YueHaibing
>
> Fix sparse warnings:
>
> drivers/gpu/drm/meson/meson_viu.c:93:6: warning: symbol
> 'meson_viu_set_g12a_osd1_matrix' was not declared. Should it be static?
> drivers/gpu/drm/meson/meson_viu.c:121:6: warning: symbol
> 'meson_viu_set_osd_matrix' was not declared. Should it be static?
> drivers/gpu/drm/meson/meson_viu.c:190:6: warning: symbol
> 'meson_viu_set_osd_lut' was not declared. Should it be static?
>
> Signed-off-by: YueHaibing
> ---
> drivers/gpu/drm/meson/meson_viu.c | 16
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/meson/meson_viu.c
> b/drivers/gpu/drm/meson/meson_viu.c
> index 0169c98..b590723 100644
> --- a/drivers/gpu/drm/meson/meson_viu.c
> +++ b/drivers/gpu/drm/meson/meson_viu.c
> @@ -90,8 +90,8 @@ static int eotf_bypass_coeff[EOTF_COEFF_SIZE] = {
> EOTF_COEFF_RIGHTSHIFT /* right shift */
> };
>
> -void meson_viu_set_g12a_osd1_matrix(struct meson_drm *priv, int *m,
> -bool csc_on)
> +static void meson_viu_set_g12a_osd1_matrix(struct meson_drm *priv,
> +int *m, bool csc_on)
> {
> /* VPP WRAP OSD1 matrix */
> writel(((m[0] & 0xfff) << 16) | (m[1] & 0xfff),
> @@ -118,8 +118,8 @@ void meson_viu_set_g12a_osd1_matrix(struct meson_drm
> *priv, int *m,
> priv->io_base + _REG(VPP_WRAP_OSD1_MATRIX_EN_CTRL));
> }
>
> -void meson_viu_set_osd_matrix(struct meson_drm *priv,
> - enum viu_matrix_sel_e m_select,
> +static void meson_viu_set_osd_matrix(struct meson_drm *priv,
> + enum viu_matrix_sel_e m_select,
> int *m, bool csc_on)
> {
> if (m_select == VIU_MATRIX_OSD) {
> @@ -187,10 +187,10 @@ void meson_viu_set_osd_matrix(struct meson_drm *priv,
> #define OSD_EOTF_LUT_SIZE 33
> #define OSD_OETF_LUT_SIZE 41
>
> -void meson_viu_set_osd_lut(struct meson_drm *priv, enum viu_lut_sel_e
> lut_sel,
> -unsigned int *r_map, unsigned int *g_map,
> -unsigned int *b_map,
> -bool csc_on)
> +static void
> +meson_viu_set_osd_lut(struct meson_drm *priv, enum viu_lut_sel_e lut_sel,
> + unsigned int *r_map, unsigned int *g_map,
> + unsigned int *b_map, bool csc_on)
> {
> unsigned int addr_port;
> unsigned int data_port;
>
Acked-by: Neil Armstrong
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/6] Fix crash after reloading a driver using ttm
Am 16.04.19 um 13:20 schrieb Karol Herbst: On Tue, Apr 16, 2019 at 1:07 PM Koenig, Christian wrote: Am 16.04.19 um 12:54 schrieb Karol Herbst: On Tue, Apr 16, 2019 at 11:12 AM Koenig, Christian wrote: Am 16.04.19 um 11:10 schrieb Karol Herbst: On Tue, Apr 16, 2019 at 8:38 AM Christian König wrote: Am 16.04.19 um 02:35 schrieb Karol Herbst: Kobjects are supposed to be dynamically allocated, but with recent changes this rule was violated. Reverting those commits fixes crashes when a drm driver using TTM gets loaded again. The object in question is "ttm_mem_glob" declared inside "include/drm/ttm/ttm_memory.h" and instatiated inside "drivers/gpu/drm/ttm/ttm_memory.c". from "Documentation/kobject.txt": "Because kobjects are dynamic, they must not be declared statically or on the stack, but instead, always allocated dynamically. Future versions of the kernel will contain a run-time check for kobjects that are created statically and will warn the developer of this improper usage." Unloading ttm before reloading the driver workarounds that crash, because the memory backing the kobject member "kobj" is cleaned up. The kobject_del and kobject_put function never free or clean up the kobject object leaving it in an undefined state. I reverted a few more commits to make it less painful for me to rever this rather big change. Well, NAK. By reverting those change you also re-introduced the problems we originally fixed with those patches. Please work on a proper fix instead, Christian. And which problem was that besides duplicated code (or maybe even a bit of memory consumption if multiple ttm driver were used)? If I had to choose between duplicated code and a crash, I choose the former. Maybe I missed the real reason why those changes are made, but the commit messages don't really seem to tell me. The old implementation crashed because different drivers tried to allocate the same kobj. Crashing in one way is not better than crashing in another way. Christian. how can that old crash be triggered? By loading two ttm based drivers at the same time or by other means? Yes, exactly. Even worse it could be triggered by one driver instantiating multiple times at the same time, e.g two AMD GPUs in the same system. On the other hand I completely agree that using kobj static is completely nuts. The problem is that using a kobj was the wrong approach in the first place. In other words when you have something which controls a global behavior of a module, what do you use? Right, a module parameter. Point is that we can't get away from those kobj without breaking the uapi, so that is something which can't be done easily. Regards, Christian. okay, thanks for the explanation! Currently I will be testing your first patch you sent out on top of 5.0.7, so that we could propose that for stable. The second didn't apply, but it didn't look relevant for the actual fix. Will reply to the patch with my results then. The second is just a further clean, please ignore that one for backporting. Let me know if the first one works on 5.0.7, if yes I'm going to add a CC stable tag before pushing. Thanks, Christian. Thanks ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/ttm: fix re-init of global structures
On Tue, Apr 16, 2019 at 12:53 PM Christian König
wrote:
>
> When a driver unloads without unloading TTM we don't correctly
> clear the global structures leading to errors on re-init.
>
> Next step should probably be to remove the global structures and
> kobjs all together, but this is tricky since we need to maintain
> backward compatibility.
>
> Signed-off-by: Christian König
> ---
> drivers/gpu/drm/ttm/ttm_bo.c | 10 +-
> drivers/gpu/drm/ttm/ttm_memory.c | 5 +++--
> include/drm/ttm/ttm_bo_driver.h | 1 -
> 3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 41d07faa2eae..2845fceb2fbd 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -49,9 +49,8 @@ static void ttm_bo_global_kobj_release(struct kobject
> *kobj);
> * ttm_global_mutex - protecting the global BO state
> */
> DEFINE_MUTEX(ttm_global_mutex);
> -struct ttm_bo_global ttm_bo_glob = {
> - .use_count = 0
> -};
> +unsigned ttm_bo_glob_use_count;
> +struct ttm_bo_global ttm_bo_glob;
>
> static struct attribute ttm_bo_count = {
> .name = "bo_count",
> @@ -1531,12 +1530,13 @@ static void ttm_bo_global_release(void)
> struct ttm_bo_global *glob = &ttm_bo_glob;
>
> mutex_lock(&ttm_global_mutex);
> - if (--glob->use_count > 0)
> + if (--ttm_bo_glob_use_count > 0)
> goto out;
>
> kobject_del(&glob->kobj);
> kobject_put(&glob->kobj);
> ttm_mem_global_release(&ttm_mem_glob);
> + memset(glob, 0, sizeof(*glob));
> out:
> mutex_unlock(&ttm_global_mutex);
> }
> @@ -1548,7 +1548,7 @@ static int ttm_bo_global_init(void)
> unsigned i;
>
> mutex_lock(&ttm_global_mutex);
> - if (++glob->use_count > 1)
> + if (++ttm_bo_glob_use_count > 1)
> goto out;
>
> ret = ttm_mem_global_init(&ttm_mem_glob);
> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c
> b/drivers/gpu/drm/ttm/ttm_memory.c
> index 699fed9e08ee..8617958b7ae6 100644
> --- a/drivers/gpu/drm/ttm/ttm_memory.c
> +++ b/drivers/gpu/drm/ttm/ttm_memory.c
> @@ -461,8 +461,8 @@ int ttm_mem_global_init(struct ttm_mem_global *glob)
>
> void ttm_mem_global_release(struct ttm_mem_global *glob)
> {
> - unsigned int i;
> struct ttm_mem_zone *zone;
> + unsigned int i;
comment below
>
> /* let the page allocator first stop the shrink work. */
> ttm_page_alloc_fini();
> @@ -475,9 +475,10 @@ void ttm_mem_global_release(struct ttm_mem_global *glob)
> zone = glob->zones[i];
> kobject_del(&zone->kobj);
> kobject_put(&zone->kobj);
> - }
> + }
comment below
> kobject_del(&glob->kobj);
> kobject_put(&glob->kobj);
> + memset(glob, 0, sizeof(*glob));
> }
>
> static void ttm_check_swapping(struct ttm_mem_global *glob)
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 3db265bf2d4e..c008346c2401 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -420,7 +420,6 @@ extern struct ttm_bo_global {
> /**
> * Protected by ttm_global_mutex.
> */
> - unsigned int use_count;
> struct list_head device_list;
>
> /**
> --
> 2.17.1
>
tested on 5.0.7 and it fixes the crash I've encountered, so:
Tested-by: Karol Herbst
Personally I think we should split formatting and other unrelated
changes out of a patch for stable, but those are still fine changes,
anyway:
Reviewed-by: Karol Herbst
Thanks!
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 18/25] drm/mediatek: add RDMA fifo size error handle
On Tue, 2019-04-16 at 16:37 +0800, Yongqiang Niu wrote:
> On Tue, 2019-04-16 at 16:00 +0800, CK Hu wrote:
> > Hi, Yongqiang:
> >
> > On Wed, 2019-03-27 at 14:19 +0800, yongqiang@mediatek.com wrote:
> > > From: Yongqiang Niu
> > >
> > > This patch add RDMA fifo size error handle
> > > rdma fifo size will not always bigger than the calculated threshold
> > > if that case happened, we need set fifo size as the threshold
> > >
> > > Signed-off-by: Yongqiang Niu
> > > ---
> > > drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 9 -
> > > 1 file changed, 8 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> > > b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> > > index b0a5cff..ead38ba 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> > > @@ -137,11 +137,14 @@ static void mtk_rdma_config(struct mtk_ddp_comp
> > > *comp, unsigned int width,
> > > {
> > > unsigned int threshold;
> > > unsigned int reg;
> > > + unsigned int rdma_fifo_size;
> > > struct mtk_disp_rdma *rdma = comp_to_rdma(comp);
> > >
> > > rdma_update_bits(comp, DISP_REG_RDMA_SIZE_CON_0, 0xfff, width);
> > > rdma_update_bits(comp, DISP_REG_RDMA_SIZE_CON_1, 0xf, height);
> > >
> > > + rdma_fifo_size = RDMA_FIFO_SIZE(rdma);
> > > +
> > > /*
> > >* Enable FIFO underflow since DSI and DPI can't be blocked.
> > >* Keep the FIFO pseudo size reset default of 8 KiB. Set the
> > > @@ -149,8 +152,12 @@ static void mtk_rdma_config(struct mtk_ddp_comp
> > > *comp, unsigned int width,
> > >* account for blanking, and with a pixel depth of 4 bytes:
> > >*/
> > > threshold = width * height * vrefresh * 4 * 7 / 100;
> > > +
> > > + if (threshold > rdma_fifo_size)
> > > + threshold = rdma_fifo_size;
> >
> > I think this is a work around not a correct solution. Why MT8173 has no
> > this problem but MT8183 has? Is the formula of threshold different in
> > MT8173 and MT8183?
> >
> > Regards,
> > CK
> >
>
> fifo size of RDMA0 and RDMA1 in MT8173 are same, which is SZ_8K.
> this formula calculate result will not overflow if the screen size is
> not big enough.
>
> but fifo size of RDMA1 in MT8183 only SZ_2K, if RDMA1 display with
> solution 1080p60hz, this formula calculate result 3483 will overflow
> SZ_2K.
RDMA1 with SZ_2K can support up to 1080p60hz, even the formula shows
overflow, is it correct?
The formula shows all resolution something more than 1280x1080@60hz will
overflow SZ_2K, and in this patch all set to maximum value.
The patch should implement different FIFO size depends on RDMA0
(SZ_5K)and RDMA1 (SZ_2K), and use the value from the formula.
Regards,
yt.shen
>
> > > +
> > > reg = RDMA_FIFO_UNDERFLOW_EN |
> > > - RDMA_FIFO_PSEUDO_SIZE(RDMA_FIFO_SIZE(rdma)) |
> > > + RDMA_FIFO_PSEUDO_SIZE(rdma_fifo_size) |
> > > RDMA_OUTPUT_VALID_FIFO_THRESHOLD(threshold);
> > > writel(reg, comp->regs + DISP_REG_RDMA_FIFO_CON);
> > > }
> >
> >
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/gma500/cdv: Check vbt config bits when detecting lvds panels
Some machines have an lvds child device in vbt even though a panel is
not attached. To make detection more reliable we now also check the lvds
config bits available in the vbt.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1665766
Signed-off-by: Patrik Jakobsson
Cc: Hans de Goede
Cc: Ville Syrjälä
---
drivers/gpu/drm/gma500/cdv_intel_lvds.c | 3 +++
drivers/gpu/drm/gma500/intel_bios.c | 3 +++
drivers/gpu/drm/gma500/psb_drv.h| 1 +
3 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index de9531caaca0..9c8446184b17 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -594,6 +594,9 @@ void cdv_intel_lvds_init(struct drm_device *dev,
int pipe;
u8 pin;
+ if (!dev_priv->lvds_enabled_in_vbt)
+ return;
+
pin = GMBUS_PORT_PANEL;
if (!lvds_is_present_in_vbt(dev, &pin)) {
DRM_DEBUG_KMS("LVDS is not present in VBT\n");
diff --git a/drivers/gpu/drm/gma500/intel_bios.c
b/drivers/gpu/drm/gma500/intel_bios.c
index 63bde4e86c6a..e019ea271ffc 100644
--- a/drivers/gpu/drm/gma500/intel_bios.c
+++ b/drivers/gpu/drm/gma500/intel_bios.c
@@ -436,6 +436,9 @@ parse_driver_features(struct drm_psb_private *dev_priv,
if (driver->lvds_config == BDB_DRIVER_FEATURE_EDP)
dev_priv->edp.support = 1;
+ dev_priv->lvds_enabled_in_vbt = driver->lvds_config != 0;
+ DRM_DEBUG_KMS("LVDS VBT config bits: 0x%x\n", driver->lvds_config);
+
/* This bit means to use 96Mhz for DPLL_A or not */
if (driver->primary_lfp_id)
dev_priv->dplla_96mhz = true;
diff --git a/drivers/gpu/drm/gma500/psb_drv.h b/drivers/gpu/drm/gma500/psb_drv.h
index 941b238bdcc9..bc608ddc3bd1 100644
--- a/drivers/gpu/drm/gma500/psb_drv.h
+++ b/drivers/gpu/drm/gma500/psb_drv.h
@@ -537,6 +537,7 @@ struct drm_psb_private {
int lvds_ssc_freq;
bool is_lvds_on;
bool is_mipi_on;
+ bool lvds_enabled_in_vbt;
u32 mipi_ctrl_display;
unsigned int core_freq;
--
2.16.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amdgpu: fix spelling mistake "gateing" -> "gating"
From: Colin Ian King
There is a spelling mistake in a DRM_INFO message. Fix it.
Signed-off-by: Colin Ian King
---
drivers/gpu/drm/amd/amdgpu/vce_v2_0.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
index bed78a778e3f..40363ca6c5f1 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
@@ -283,7 +283,7 @@ static int vce_v2_0_stop(struct amdgpu_device *adev)
}
if (vce_v2_0_wait_for_idle(adev)) {
- DRM_INFO("VCE is busy, Can't set clock gateing");
+ DRM_INFO("VCE is busy, Can't set clock gating");
return 0;
}
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/6] Fix crash after reloading a driver using ttm
On Tue, Apr 16, 2019 at 11:06:54AM +, Koenig, Christian wrote: > Am 16.04.19 um 12:54 schrieb Karol Herbst: > > On Tue, Apr 16, 2019 at 11:12 AM Koenig, Christian > > wrote: > >> Am 16.04.19 um 11:10 schrieb Karol Herbst: > >>> On Tue, Apr 16, 2019 at 8:38 AM Christian König > >>> wrote: > Am 16.04.19 um 02:35 schrieb Karol Herbst: > > Kobjects are supposed to be dynamically allocated, but with recent > > changes > > this rule was violated. Reverting those commits fixes crashes when a drm > > driver using TTM gets loaded again. > > > > The object in question is "ttm_mem_glob" declared inside > > "include/drm/ttm/ttm_memory.h" and instatiated inside > > "drivers/gpu/drm/ttm/ttm_memory.c". > > > > from "Documentation/kobject.txt": > > "Because kobjects are dynamic, they must not be declared statically or > > on > > the stack, but instead, always allocated dynamically. Future versions > > of > > the kernel will contain a run-time check for kobjects that are created > > statically and will warn the developer of this improper usage." > > > > Unloading ttm before reloading the driver workarounds that crash, > > because > > the memory backing the kobject member "kobj" is cleaned up. The > > kobject_del > > and kobject_put function never free or clean up the kobject object > > leaving > > it in an undefined state. > > > > I reverted a few more commits to make it less painful for me to rever > > this > > rather big change. > Well, NAK. By reverting those change you also re-introduced the problems > we originally fixed with those patches. > > Please work on a proper fix instead, > Christian. > >>> And which problem was that besides duplicated code (or maybe even a > >>> bit of memory consumption if multiple ttm driver were used)? If I had > >>> to choose between duplicated code and a crash, I choose the former. > >>> > >>> Maybe I missed the real reason why those changes are made, but the > >>> commit messages don't really seem to tell me. > >> The old implementation crashed because different drivers tried to > >> allocate the same kobj. > >> > >> Crashing in one way is not better than crashing in another way. > >> > >> Christian. > >> > > how can that old crash be triggered? By loading two ttm based drivers > > at the same time or by other means? > > Yes, exactly. Even worse it could be triggered by one driver > instantiating multiple times at the same time, e.g two AMD GPUs in the > same system. > > On the other hand I completely agree that using kobj static is > completely nuts. The problem is that using a kobj was the wrong approach > in the first place. > > In other words when you have something which controls a global behavior > of a module, what do you use? Right, a module parameter. > > Point is that we can't get away from those kobj without breaking the > uapi, so that is something which can't be done easily. Randome idea: Push the kobj setup into drm.ko (and shrugg it off as another lesson in how maybe uapi shouldn't have been designed, but hey not our worst mistake by far). I think that'd be totally ok. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: add drm_format_helper.c to kerneldoc
On Tue, Apr 16, 2019 at 11:05:33AM +0200, Gerd Hoffmann wrote: > Also drop the dstclip parameter sphinx has warned about (leftolver from > an earlier patch version). > > Signed-off-by: Gerd Hoffmann Assuming it all builds cleanly and all the links work and no warnings from sphinx: Acked-by: Daniel Vetter > --- > drivers/gpu/drm/drm_format_helper.c | 1 - > Documentation/gpu/drm-kms-helpers.rst | 6 ++ > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_format_helper.c > b/drivers/gpu/drm/drm_format_helper.c > index da388012df2a..a18da35145b7 100644 > --- a/drivers/gpu/drm/drm_format_helper.c > +++ b/drivers/gpu/drm/drm_format_helper.c > @@ -235,7 +235,6 @@ static void drm_fb_xrgb_to_rgb888_line(u8 *dbuf, u32 > *sbuf, > * @vaddr: XRGB source buffer > * @fb: DRM framebuffer > * @clip: Clip rectangle area to copy > - * @dstclip: Clip destination too. > * > * Drivers can use this function for RGB888 devices that don't natively > * support XRGB. > diff --git a/Documentation/gpu/drm-kms-helpers.rst > b/Documentation/gpu/drm-kms-helpers.rst > index 58b375e47615..14102ae035dc 100644 > --- a/Documentation/gpu/drm-kms-helpers.rst > +++ b/Documentation/gpu/drm-kms-helpers.rst > @@ -107,6 +107,12 @@ fbdev Helper Functions Reference > .. kernel-doc:: drivers/gpu/drm/drm_fb_helper.c > :export: > > +format Helper Functions Reference > += > + > +.. kernel-doc:: drivers/gpu/drm/drm_format_helper.c > + :export: > + > Framebuffer CMA Helper Functions Reference > == > > -- > 2.18.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/6] Fix crash after reloading a driver using ttm
Am 16.04.19 um 14:18 schrieb Daniel Vetter: > On Tue, Apr 16, 2019 at 11:06:54AM +, Koenig, Christian wrote: >> Am 16.04.19 um 12:54 schrieb Karol Herbst: >>> On Tue, Apr 16, 2019 at 11:12 AM Koenig, Christian >>> wrote: Am 16.04.19 um 11:10 schrieb Karol Herbst: > On Tue, Apr 16, 2019 at 8:38 AM Christian König > wrote: >> Am 16.04.19 um 02:35 schrieb Karol Herbst: >>> Kobjects are supposed to be dynamically allocated, but with recent >>> changes >>> this rule was violated. Reverting those commits fixes crashes when a drm >>> driver using TTM gets loaded again. >>> >>> The object in question is "ttm_mem_glob" declared inside >>> "include/drm/ttm/ttm_memory.h" and instatiated inside >>> "drivers/gpu/drm/ttm/ttm_memory.c". >>> >>> from "Documentation/kobject.txt": >>> "Because kobjects are dynamic, they must not be declared statically or >>> on >>> the stack, but instead, always allocated dynamically. Future versions >>> of >>> the kernel will contain a run-time check for kobjects that are created >>> statically and will warn the developer of this improper usage." >>> >>> Unloading ttm before reloading the driver workarounds that crash, >>> because >>> the memory backing the kobject member "kobj" is cleaned up. The >>> kobject_del >>> and kobject_put function never free or clean up the kobject object >>> leaving >>> it in an undefined state. >>> >>> I reverted a few more commits to make it less painful for me to rever >>> this >>> rather big change. >> Well, NAK. By reverting those change you also re-introduced the problems >> we originally fixed with those patches. >> >> Please work on a proper fix instead, >> Christian. > And which problem was that besides duplicated code (or maybe even a > bit of memory consumption if multiple ttm driver were used)? If I had > to choose between duplicated code and a crash, I choose the former. > > Maybe I missed the real reason why those changes are made, but the > commit messages don't really seem to tell me. The old implementation crashed because different drivers tried to allocate the same kobj. Crashing in one way is not better than crashing in another way. Christian. >>> how can that old crash be triggered? By loading two ttm based drivers >>> at the same time or by other means? >> Yes, exactly. Even worse it could be triggered by one driver >> instantiating multiple times at the same time, e.g two AMD GPUs in the >> same system. >> >> On the other hand I completely agree that using kobj static is >> completely nuts. The problem is that using a kobj was the wrong approach >> in the first place. >> >> In other words when you have something which controls a global behavior >> of a module, what do you use? Right, a module parameter. >> >> Point is that we can't get away from those kobj without breaking the >> uapi, so that is something which can't be done easily. > Randome idea: Push the kobj setup into drm.ko (and shrugg it off as > another lesson in how maybe uapi shouldn't have been designed, but hey not > our worst mistake by far). I think that'd be totally ok. Yeah, thought about that as well. But I would rather re-design this from the scratch instead of just moving it over. And yes I agree with a bit of luck that UAPI is actually not used at all, so we could remove it sooner or later. Regards, Christian. > -Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm: introduce a capability flag for syncobj timeline support
Unfortunately userspace users of this API cannot be publicly disclosed
yet.
This commit effectively disables timeline syncobj ioctls for all
drivers. Each driver wishing to support this feature will need to
expose DRIVER_SYNCOBJ_TIMELINE.
Signed-off-by: Lionel Landwerlin
Cc: Dave Airlie
Cc: Daniel Vetter
Cc: Christian König
Cc: Chunming Zhou
---
drivers/gpu/drm/drm_syncobj.c | 10 +-
include/drm/drm_drv.h | 7 +++
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index fb65f13d25cf..72a38ff6e3e4 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -755,7 +755,7 @@ drm_syncobj_transfer_ioctl(struct drm_device *dev, void
*data,
struct drm_syncobj_transfer *args = data;
int ret;
- if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+ if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
return -EOPNOTSUPP;
if (args->pad)
@@ -1106,7 +1106,7 @@ drm_syncobj_timeline_wait_ioctl(struct drm_device *dev,
void *data,
struct drm_syncobj **syncobjs;
int ret = 0;
- if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+ if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
return -EOPNOTSUPP;
if (args->flags & ~(DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL |
@@ -1210,7 +1210,7 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device *dev,
void *data,
uint32_t i, j;
int ret;
- if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+ if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
return -EOPNOTSUPP;
if (args->pad != 0)
@@ -1281,8 +1281,8 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void
*data,
uint32_t i;
int ret;
- if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
- return -ENODEV;
+ if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
+ return -EOPNOTSUPP;
if (args->pad != 0)
return -EINVAL;
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 5cc7f728ec73..68ca736c548d 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -91,6 +91,13 @@ enum drm_driver_feature {
* submission.
*/
DRIVER_SYNCOBJ = BIT(5),
+ /**
+* @DRIVER_SYNCOBJ_TIMELINE:
+*
+* Driver supports the timeline flavor of &drm_syncobj for explicit
+* synchronization of command submission.
+*/
+ DRIVER_SYNCOBJ_TIMELINE = BIT(6),
/* IMPORTANT: Below are all the legacy flags, add new ones above. */
--
2.21.0.392.gf8f6787159e
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm: report consistent errors when checking syncobj capibility
We've been somewhat inconsistent when adding the new ioctl and
returned ENODEV instead of EOPNOTSUPPORTED upon failing the syncobj
capibility.
Signed-off-by: Lionel Landwerlin
Fixes: ea569910cbab98 ("drm/syncobj: add transition iotcls between binary and
timeline v2")
Fixes: 01d6c357837918 ("drm/syncobj: add support for timeline point wait v8")
Cc: Dave Airlie
Cc: Christian König
Cc: Chunming Zhou
---
drivers/gpu/drm/drm_syncobj.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index c534c5d46f1e..fb65f13d25cf 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -756,7 +756,7 @@ drm_syncobj_transfer_ioctl(struct drm_device *dev, void
*data,
int ret;
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
- return -ENODEV;
+ return -EOPNOTSUPP;
if (args->pad)
return -EINVAL;
@@ -1107,7 +1107,7 @@ drm_syncobj_timeline_wait_ioctl(struct drm_device *dev,
void *data,
int ret = 0;
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
- return -ENODEV;
+ return -EOPNOTSUPP;
if (args->flags & ~(DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL |
DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |
--
2.21.0.392.gf8f6787159e
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: report consistent errors when checking syncobj capibility
Am 16.04.19 um 14:30 schrieb Lionel Landwerlin:
We've been somewhat inconsistent when adding the new ioctl and
returned ENODEV instead of EOPNOTSUPPORTED upon failing the syncobj
capibility.
Signed-off-by: Lionel Landwerlin
Fixes: ea569910cbab98 ("drm/syncobj: add transition iotcls between binary and
timeline v2")
Fixes: 01d6c357837918 ("drm/syncobj: add support for timeline point wait v8")
Cc: Dave Airlie
Cc: Christian König
Cc: Chunming Zhou
Reviewed-by: Christian König for the series.
How about also adding a DRM_CAP_TIMELINE_SYNCOBJ as Daniel suggested so
that userspace can note that as well?
Thanks,
Christian.
---
drivers/gpu/drm/drm_syncobj.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index c534c5d46f1e..fb65f13d25cf 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -756,7 +756,7 @@ drm_syncobj_transfer_ioctl(struct drm_device *dev, void
*data,
int ret;
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
- return -ENODEV;
+ return -EOPNOTSUPP;
if (args->pad)
return -EINVAL;
@@ -1107,7 +1107,7 @@ drm_syncobj_timeline_wait_ioctl(struct drm_device *dev,
void *data,
int ret = 0;
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
- return -ENODEV;
+ return -EOPNOTSUPP;
if (args->flags & ~(DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL |
DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: report consistent errors when checking syncobj capibility
On Tue, Apr 16, 2019 at 02:40:37PM +0200, Christian König wrote:
> Am 16.04.19 um 14:30 schrieb Lionel Landwerlin:
> > We've been somewhat inconsistent when adding the new ioctl and
> > returned ENODEV instead of EOPNOTSUPPORTED upon failing the syncobj
> > capibility.
> >
> > Signed-off-by: Lionel Landwerlin
> > Fixes: ea569910cbab98 ("drm/syncobj: add transition iotcls between binary
> > and timeline v2")
> > Fixes: 01d6c357837918 ("drm/syncobj: add support for timeline point wait
> > v8")
> > Cc: Dave Airlie
> > Cc: Christian König
> > Cc: Chunming Zhou
>
> Reviewed-by: Christian König for the series.
>
> How about also adding a DRM_CAP_TIMELINE_SYNCOBJ as Daniel suggested so that
> userspace can note that as well?
Attempting one of the ioctls and getting a EOPNOTSUPP should be good
enough. In case that "Daniel" meant me ...
-Daniel
>
> Thanks,
> Christian.
>
> > ---
> > drivers/gpu/drm/drm_syncobj.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> > index c534c5d46f1e..fb65f13d25cf 100644
> > --- a/drivers/gpu/drm/drm_syncobj.c
> > +++ b/drivers/gpu/drm/drm_syncobj.c
> > @@ -756,7 +756,7 @@ drm_syncobj_transfer_ioctl(struct drm_device *dev, void
> > *data,
> > int ret;
> > if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
> > - return -ENODEV;
> > + return -EOPNOTSUPP;
> > if (args->pad)
> > return -EINVAL;
> > @@ -1107,7 +1107,7 @@ drm_syncobj_timeline_wait_ioctl(struct drm_device
> > *dev, void *data,
> > int ret = 0;
> > if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
> > - return -ENODEV;
> > + return -EOPNOTSUPP;
> > if (args->flags & ~(DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL |
> > DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v2,1/2] drm: Add writeback_w,h properties
On Tue, Apr 16, 2019 at 11:55:34AM +0200, Daniel Vetter wrote:
> On Tue, Apr 16, 2019 at 11:17 AM Liviu Dudau wrote:
> >
> > On Tue, Apr 16, 2019 at 09:34:20AM +0200, Daniel Vetter wrote:
> > > On Mon, Apr 15, 2019 at 10:20:45AM +0100, Liviu Dudau wrote:
> > > > On Mon, Apr 15, 2019 at 08:59:30AM +0100, james qian wang (Arm
> > > > Technology China) wrote:
> > > > > On Fri, Apr 12, 2019 at 02:08:28PM +, Ben Davis wrote:
> > > > > > Add new properties to specify width and height for writeback.
> > > > > >
> > > > > > Signed-off-by: Ben Davis
> > > > > > ---
> > > > > > drivers/gpu/drm/drm_atomic_uapi.c | 8
> > > > > > drivers/gpu/drm/drm_writeback.c | 28
> > > > > > include/drm/drm_connector.h | 4
> > > > > > include/drm/drm_mode_config.h | 10 ++
> > > > > > 4 files changed, 50 insertions(+)
> > > > > >
> > > > > > --
> > > > > > 2.7.4
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > > b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > > index d520a04..1f68dce 100644
> > > > > > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > > @@ -765,6 +765,10 @@ static int
> > > > > > drm_atomic_connector_set_property(struct drm_connector *connector,
> > > > > > return -EINVAL;
> > > > > > }
> > > > > > state->content_protection = val;
> > > > > > + } else if (property == config->prop_writeback_w) {
> > > > > > + state->writeback_w = val;
> > > > > > + } else if (property == config->prop_writeback_h) {
> > > > > > + state->writeback_h = val;
> > > > > > } else if (property == config->writeback_fb_id_property) {
> > > > > > struct drm_framebuffer *fb =
> > > > > > drm_framebuffer_lookup(dev, NULL, val);
> > > > > > int ret =
> > > > > > drm_atomic_set_writeback_fb_for_connector(state, fb);
> > > > > > @@ -837,6 +841,10 @@ drm_atomic_connector_get_property(struct
> > > > > > drm_connector *connector,
> > > > > > *val = state->scaling_mode;
> > > > > > } else if (property ==
> > > > > > connector->content_protection_property) {
> > > > > > *val = state->content_protection;
> > > > > > + } else if (property == config->prop_writeback_w) {
> > > > > > + *val = state->writeback_w;
> > > > > > + } else if (property == config->prop_writeback_h) {
> > > > > > + *val = state->writeback_h;
> > > > > > } else if (property == config->writeback_fb_id_property) {
> > > > > > /* Writeback framebuffer is one-shot, write and
> > > > > > forget */
> > > > > > *val = 0;
> > > > > > diff --git a/drivers/gpu/drm/drm_writeback.c
> > > > > > b/drivers/gpu/drm/drm_writeback.c
> > > > > > index c20e6fe..3d0453e 100644
> > > > > > --- a/drivers/gpu/drm/drm_writeback.c
> > > > > > +++ b/drivers/gpu/drm/drm_writeback.c
> > > > > > @@ -74,6 +74,12 @@
> > > > > > * applications making use of writeback connectors *always*
> > > > > > retrieve an
> > > > > > * out-fence for the commit and use it appropriately.
> > > > > > * From userspace, this property will always read as zero.
> > > > > > + *
> > > > > > + * "WRITEBACK_W":
> > > > > > + * The width of the writeback buffer to write back. 0 acts as
> > > > > > default.
> > > > > > + *
> > > > > > + * "WRITEBACK_H":
> > > > > > + * The height of the writeback buffer to write back. 0 acts as
> > > > > > default.
> > > > > > */
> > > > > >
> > > > > > #define fence_to_wb_connector(x) container_of(x->lock, \
> > > > > > @@ -141,6 +147,22 @@ static int create_writeback_properties(struct
> > > > > > drm_device *dev)
> > > > > > dev->mode_config.writeback_out_fence_ptr_property =
> > > > > > prop;
> > > > > > }
> > > > > >
> > > > > > + if (!dev->mode_config.prop_writeback_w) {
> > > > > > + prop = drm_property_create_range(dev,
> > > > > > DRM_MODE_PROP_ATOMIC,
> > > > > > +"WRITEBACK_W", 1,
> > > > > > UINT_MAX);
> > > > > > + if (!prop)
> > > > > > + return -ENOMEM;
> > > > > > + dev->mode_config.prop_writeback_w = prop;
> > > > > > + }
> > > > > > +
> > > > > > + if (!dev->mode_config.prop_writeback_h) {
> > > > > > + prop = drm_property_create_range(dev,
> > > > > > DRM_MODE_PROP_ATOMIC,
> > > > > > +"WRITEBACK_H", 1,
> > > > > > UINT_MAX);
> > > > > > + if (!prop)
> > > > > > + return -ENOMEM;
> > > > > > + dev->mode_config.prop_writeback_h = prop;
> > > > > > + }
> > > > > > +
> > > > > > return 0;
> > > > > > }
> > > > > >
> > > > > > @@ -225,6 +247,12 @@ int drm_writeback_c
Re: [PATCH 1/2] drm: report consistent errors when checking syncobj capibility
On 16/04/2019 13:40, Christian König wrote:
Am 16.04.19 um 14:30 schrieb Lionel Landwerlin:
We've been somewhat inconsistent when adding the new ioctl and
returned ENODEV instead of EOPNOTSUPPORTED upon failing the syncobj
capibility.
Signed-off-by: Lionel Landwerlin
Fixes: ea569910cbab98 ("drm/syncobj: add transition iotcls between
binary and timeline v2")
Fixes: 01d6c357837918 ("drm/syncobj: add support for timeline point
wait v8")
Cc: Dave Airlie
Cc: Christian König
Cc: Chunming Zhou
Reviewed-by: Christian König for the series.
How about also adding a DRM_CAP_TIMELINE_SYNCOBJ as Daniel suggested
so that userspace can note that as well?
Thanks,
Christian.
Thanks Christian, I forgot about that...
---
drivers/gpu/drm/drm_syncobj.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c
b/drivers/gpu/drm/drm_syncobj.c
index c534c5d46f1e..fb65f13d25cf 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -756,7 +756,7 @@ drm_syncobj_transfer_ioctl(struct drm_device
*dev, void *data,
int ret;
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
- return -ENODEV;
+ return -EOPNOTSUPP;
if (args->pad)
return -EINVAL;
@@ -1107,7 +1107,7 @@ drm_syncobj_timeline_wait_ioctl(struct
drm_device *dev, void *data,
int ret = 0;
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
- return -ENODEV;
+ return -EOPNOTSUPP;
if (args->flags & ~(DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL |
DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: report consistent errors when checking syncobj capibility
Am 16.04.19 um 14:43 schrieb Daniel Vetter:
> On Tue, Apr 16, 2019 at 02:40:37PM +0200, Christian König wrote:
>> Am 16.04.19 um 14:30 schrieb Lionel Landwerlin:
>>> We've been somewhat inconsistent when adding the new ioctl and
>>> returned ENODEV instead of EOPNOTSUPPORTED upon failing the syncobj
>>> capibility.
>>>
>>> Signed-off-by: Lionel Landwerlin
>>> Fixes: ea569910cbab98 ("drm/syncobj: add transition iotcls between binary
>>> and timeline v2")
>>> Fixes: 01d6c357837918 ("drm/syncobj: add support for timeline point wait
>>> v8")
>>> Cc: Dave Airlie
>>> Cc: Christian König
>>> Cc: Chunming Zhou
>> Reviewed-by: Christian König for the series.
>>
>> How about also adding a DRM_CAP_TIMELINE_SYNCOBJ as Daniel suggested so that
>> userspace can note that as well?
> Attempting one of the ioctls and getting a EOPNOTSUPP should be good
> enough. In case that "Daniel" meant me ...
Oh, sorry my fault. It was actually Dave who suggested that...
Christian.
> -Daniel
>
>> Thanks,
>> Christian.
>>
>>> ---
>>>drivers/gpu/drm/drm_syncobj.c | 4 ++--
>>>1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
>>> index c534c5d46f1e..fb65f13d25cf 100644
>>> --- a/drivers/gpu/drm/drm_syncobj.c
>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>> @@ -756,7 +756,7 @@ drm_syncobj_transfer_ioctl(struct drm_device *dev, void
>>> *data,
>>> int ret;
>>> if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
>>> - return -ENODEV;
>>> + return -EOPNOTSUPP;
>>> if (args->pad)
>>> return -EINVAL;
>>> @@ -1107,7 +1107,7 @@ drm_syncobj_timeline_wait_ioctl(struct drm_device
>>> *dev, void *data,
>>> int ret = 0;
>>> if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
>>> - return -ENODEV;
>>> + return -EOPNOTSUPP;
>>> if (args->flags & ~(DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL |
>>> DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
