Re: [PATCH v2 00/10] Treewide GPL SPDX conversion and cleanup (in response to Didi's GPL full name fixes)
On 5/12/23 18:23, Greg Kroah-Hartman wrote: > I'm glad to take these types of changes through the SPDX tree, but > please break them up into smaller changes that show the justification > for each type of change in each subsystem, so that we can evaluate them > on an individual basis. As you did here, you are lumping things > together only by the existance of the file in the tree, not by the > logical type of change happening, which isn't ok. > > Also, you can send them as subsystem-specific series, so as to not have > to cross-post all of the changes all over the place. I doubt the drm > developers care about ethernet driver license issues :) > OK, thanks! -- An old man doll... just what I always wanted! - Clara
Re: [PATCH v2 09/10] udf: Replace license notice with SPDX identifier
On 5/12/23 20:21, Richard Fontana wrote: > On Fri, May 12, 2023 at 6:07 AM Bagas Sanjaya wrote: > >> diff --git a/fs/udf/ecma_167.h b/fs/udf/ecma_167.h >> index de17a97e866742..b2b5bca45758df 100644 >> --- a/fs/udf/ecma_167.h >> +++ b/fs/udf/ecma_167.h >> @@ -1,3 +1,4 @@ >> +/* SPDX-License-Identifier: BSD-2-Clause OR GPL-1.0+ */ >> /* >> * ecma_167.h >> * >> @@ -8,29 +9,6 @@ >> * Copyright (c) 2017-2019 Pali Rohár >> * All rights reserved. >> * >> - * Redistribution and use in source and binary forms, with or without >> - * modification, are permitted provided that the following conditions >> - * are met: >> - * 1. Redistributions of source code must retain the above copyright >> - *notice, this list of conditions, and the following disclaimer, >> - *without modification. >> - * 2. The name of the author may not be used to endorse or promote products >> - *derived from this software without specific prior written permission. >> - * > > This is not BSD-2-Clause. Ignoring the interior statement about the > GPL, I think the closest SPDX identifier might be > https://spdx.org/licenses/BSD-Source-Code.html > but it doesn't quite match. > BSD-2-Clause but the second clause is the third one on BSD-3-Clause. Weird... >> diff --git a/fs/udf/udftime.c b/fs/udf/udftime.c >> index fce4ad976c8c29..d0fce5348fd3f3 100644 >> --- a/fs/udf/udftime.c >> +++ b/fs/udf/udftime.c >> @@ -1,21 +1,4 @@ >> -/* Copyright (C) 1993, 1994, 1995, 1996, 1997 Free Software Foundation, Inc. >> - This file is part of the GNU C Library. >> - Contributed by Paul Eggert (egg...@twinsun.com). >> - >> - The GNU C Library is free software; you can redistribute it and/or >> - modify it under the terms of the GNU Library General Public License as >> - published by the Free Software Foundation; either version 2 of the >> - License, or (at your option) any later version. >> - >> - The GNU C Library is distributed in the hope that it will be useful, >> - but WITHOUT ANY WARRANTY; without even the implied warranty of >> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> - Library General Public License for more details. >> - >> - You should have received a copy of the GNU Library General Public >> - License along with the GNU C Library; see the file COPYING.LIB. If not, >> - write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330, >> - Boston, MA 02111-1307, USA. */ >> +// SPDX-License-Identifier: GPL-2.0-only > > Shouldn't this be > // SPDX-License-Identifier: LGPL-2.0-or-later ? > (or are you implicitly using the obscure LGPLv2.x section ... 3 mechanism?) > That's right. I missed the exact notice above when I submitted this series. Thanks. -- An old man doll... just what I always wanted! - Clara
Re: [PATCH v2 09/10] udf: Replace license notice with SPDX identifier
On Friday 12 May 2023 17:06:20 Bagas Sanjaya wrote: > diff --git a/fs/udf/udftime.c b/fs/udf/udftime.c > index fce4ad976c8c29..d0fce5348fd3f3 100644 > --- a/fs/udf/udftime.c > +++ b/fs/udf/udftime.c > @@ -1,21 +1,4 @@ > -/* Copyright (C) 1993, 1994, 1995, 1996, 1997 Free Software Foundation, Inc. > - This file is part of the GNU C Library. > - Contributed by Paul Eggert (egg...@twinsun.com). > - > - The GNU C Library is free software; you can redistribute it and/or > - modify it under the terms of the GNU Library General Public License as > - published by the Free Software Foundation; either version 2 of the > - License, or (at your option) any later version. > - > - The GNU C Library is distributed in the hope that it will be useful, > - but WITHOUT ANY WARRANTY; without even the implied warranty of > - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > - Library General Public License for more details. > - > - You should have received a copy of the GNU Library General Public > - License along with the GNU C Library; see the file COPYING.LIB. If not, > - write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330, > - Boston, MA 02111-1307, USA. */ > +// SPDX-License-Identifier: GPL-2.0-only > > /* > * dgb 10/02/98: ripped this from glibc source to help convert timestamps Please, dot not do this. It is really rude to people who worked on it in past (even if they do not care about this particular file anymore) as technically they still have ownership of this code / file. And such change never remove their ownership or copyright in most countries.
Re: [PATCH v2 09/10] udf: Replace license notice with SPDX identifier
On Sat, May 13, 2023 at 11:48:51AM +0200, Pali Rohár wrote: > On Friday 12 May 2023 17:06:20 Bagas Sanjaya wrote: > > diff --git a/fs/udf/udftime.c b/fs/udf/udftime.c > > index fce4ad976c8c29..d0fce5348fd3f3 100644 > > --- a/fs/udf/udftime.c > > +++ b/fs/udf/udftime.c > > @@ -1,21 +1,4 @@ > > -/* Copyright (C) 1993, 1994, 1995, 1996, 1997 Free Software Foundation, > > Inc. > > - This file is part of the GNU C Library. > > - Contributed by Paul Eggert (egg...@twinsun.com). > > - > > - The GNU C Library is free software; you can redistribute it and/or > > - modify it under the terms of the GNU Library General Public License as > > - published by the Free Software Foundation; either version 2 of the > > - License, or (at your option) any later version. > > - > > - The GNU C Library is distributed in the hope that it will be useful, > > - but WITHOUT ANY WARRANTY; without even the implied warranty of > > - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > - Library General Public License for more details. > > - > > - You should have received a copy of the GNU Library General Public > > - License along with the GNU C Library; see the file COPYING.LIB. If not, > > - write to the Free Software Foundation, Inc., 59 Temple Place - Suite > > 330, > > - Boston, MA 02111-1307, USA. */ > > +// SPDX-License-Identifier: GPL-2.0-only > > > > /* > > * dgb 10/02/98: ripped this from glibc source to help convert timestamps > > Please, dot not do this. It is really rude to people who worked on it in > past (even if they do not care about this particular file anymore) as > technically they still have ownership of this code / file. And such > change never remove their ownership or copyright in most countries. > Yes, copyright lines should never be removed. Bagas, I recommend taking the Linux Foundation "copyright and licensing" online course before you respin this series. It's free online and might help you in understanding some of the issues involved here and prevent you from making these types of mistakes in the future. thanks, greg k-h
Re: [PATCH v2 08/10] drivers: watchdog: Replace GPL license notice with SPDX identifier
On 5/12/23 19:46, Richard Fontana wrote: > On Fri, May 12, 2023 at 6:07 AM Bagas Sanjaya wrote: > > >> diff --git a/drivers/watchdog/sb_wdog.c b/drivers/watchdog/sb_wdog.c >> index 504be461f992a9..822bf8905bf3ce 100644 >> --- a/drivers/watchdog/sb_wdog.c >> +++ b/drivers/watchdog/sb_wdog.c >> @@ -1,3 +1,4 @@ >> +// SPDX-License-Identifier: GPL-1.0+ >> /* >> * Watchdog driver for SiByte SB1 SoCs >> * >> @@ -38,10 +39,6 @@ >> * (c) Copyright 1996 Alan Cox , >> * All Rights Reserved. >> * >> - * This program is free software; you can redistribute it and/or >> - * modify it under the terms of the GNU General Public License >> - * version 1 or 2 as published by the Free Software Foundation. > > Shouldn't this be > // SPDX-License-Identifier: GPL-1.0 OR GPL-2.0 > (or in current SPDX notation GPL-1.0-only OR GPL-2.0-only) ? > Nope, as it will fail spdxcheck.py. Also, SPDX specification [1] doesn't have negation operator (NOT), thus the licensing requirement on the above notice can't be expressed reliably in SPDX here. [1]: https://spdx.github.io/spdx-spec/v2.3/SPDX-license-expressions/ -- An old man doll... just what I always wanted! - Clara
Re: drm/ssd130x: Fix include guard name
Reviewed-by: Sui Jingfeng On 2023/5/12 20:02, Javier Martinez Canillas wrote: This is a leftover from an early iteration of the driver when it was still named ssd1307 instead of ssd130x. Change it for consistency with the rest. Signed-off-by: Javier Martinez Canillas Reviewed-by: Sam Ravnborg --- drivers/gpu/drm/solomon/ssd130x.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/solomon/ssd130x.h b/drivers/gpu/drm/solomon/ssd130x.h index 03038c1b6476..db03ee5db392 100644 --- a/drivers/gpu/drm/solomon/ssd130x.h +++ b/drivers/gpu/drm/solomon/ssd130x.h @@ -10,8 +10,8 @@ * Copyright 2012 Free Electrons */ -#ifndef __SSD1307X_H__ -#define __SSD1307X_H__ +#ifndef __SSD130X_H__ +#define __SSD130X_H__ #include #include @@ -94,4 +94,4 @@ struct ssd130x_device *ssd130x_probe(struct device *dev, struct regmap *regmap); void ssd130x_remove(struct ssd130x_device *ssd130x); void ssd130x_shutdown(struct ssd130x_device *ssd130x); -#endif /* __SSD1307X_H__ */ +#endif /* __SSD130X_H__ */
Re: drm/drm_plane.h: fix grammar of the comment
Hi, thanks a lot
On 2023/4/9 21:15, Sui Jingfeng wrote:
From: Sui Jingfeng
Signed-off-by: Sui Jingfeng
---
include/drm/drm_plane.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 51291983ea44..79d62856defb 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -56,7 +56,7 @@ struct drm_plane_state {
/**
* @crtc:
*
-* Currently bound CRTC, NULL if disabled. Do not this write directly,
+* Currently bound CRTC, NULL if disabled. Do not write this directly,
* use drm_atomic_set_crtc_for_plane()
*/
struct drm_crtc *crtc;
Re: [PATCH v2 08/10] drivers: watchdog: Replace GPL license notice with SPDX identifier
On Sat, May 13, 2023 at 6:53 AM Bagas Sanjaya wrote: > > On 5/12/23 19:46, Richard Fontana wrote: > > On Fri, May 12, 2023 at 6:07 AM Bagas Sanjaya wrote: > > > > > >> diff --git a/drivers/watchdog/sb_wdog.c b/drivers/watchdog/sb_wdog.c > >> index 504be461f992a9..822bf8905bf3ce 100644 > >> --- a/drivers/watchdog/sb_wdog.c > >> +++ b/drivers/watchdog/sb_wdog.c > >> @@ -1,3 +1,4 @@ > >> +// SPDX-License-Identifier: GPL-1.0+ > >> /* > >> * Watchdog driver for SiByte SB1 SoCs > >> * > >> @@ -38,10 +39,6 @@ > >> * (c) Copyright 1996 Alan Cox , > >> * All Rights Reserved. > >> * > >> - * This program is free software; you can redistribute it and/or > >> - * modify it under the terms of the GNU General Public License > >> - * version 1 or 2 as published by the Free Software Foundation. > > > > Shouldn't this be > > // SPDX-License-Identifier: GPL-1.0 OR GPL-2.0 > > (or in current SPDX notation GPL-1.0-only OR GPL-2.0-only) ? > > > > Nope, as it will fail spdxcheck.py. Also, SPDX specification [1] > doesn't have negation operator (NOT), thus the licensing requirement > on the above notice can't be expressed reliably in SPDX here. > > [1]: https://spdx.github.io/spdx-spec/v2.3/SPDX-license-expressions/ The GPL identifiers in recent versions of SPDX include an `-only` and an `-or-later` variant. So I don't see why you can't represent it as `GPL-1.0-only OR GPL-2.0-only`. From what I understand the kernel requires/prefers use of the earlier approach to GPL identifiers (which was better in my opinion) under which `GPL-1.0 OR GPL-2.0` would at least be semantically similar. I don't see why you need a negation operator in this case. You have other patches where you used the `-only` identifiers. Richard
[PATCH v3 1/5] i2c: Enhance i2c_new_ancillary_device API
Renesas PMIC RAA215300 exposes two separate i2c devices, one for the main
device and another for rtc device.
Enhance i2c_new_ancillary_device() to instantiate a real device.
(eg: Instantiate rtc device from PMIC driver)
Signed-off-by: Biju Das
---
v3:
* New patch
Ref:
https://patchwork.kernel.org/project/linux-renesas-soc/patch/20230505172530.357455-5-biju.das...@bp.renesas.com/
---
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 6 ++--
drivers/i2c/i2c-core-base.c | 38
drivers/media/i2c/adv748x/adv748x-core.c | 2 +-
drivers/media/i2c/adv7604.c | 3 +-
include/linux/i2c.h | 3 +-
5 files changed, 39 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index ddceafa7b637..86306b010a0a 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -1072,7 +1072,7 @@ static int adv7511_init_cec_regmap(struct adv7511 *adv)
int ret;
adv->i2c_cec = i2c_new_ancillary_device(adv->i2c_main, "cec",
- ADV7511_CEC_I2C_ADDR_DEFAULT);
+ ADV7511_CEC_I2C_ADDR_DEFAULT, NULL);
if (IS_ERR(adv->i2c_cec))
return PTR_ERR(adv->i2c_cec);
@@ -1261,7 +1261,7 @@ static int adv7511_probe(struct i2c_client *i2c)
adv7511_packet_disable(adv7511, 0x);
adv7511->i2c_edid = i2c_new_ancillary_device(i2c, "edid",
- ADV7511_EDID_I2C_ADDR_DEFAULT);
+ ADV7511_EDID_I2C_ADDR_DEFAULT, NULL);
if (IS_ERR(adv7511->i2c_edid)) {
ret = PTR_ERR(adv7511->i2c_edid);
goto uninit_regulators;
@@ -1271,7 +1271,7 @@ static int adv7511_probe(struct i2c_client *i2c)
adv7511->i2c_edid->addr << 1);
adv7511->i2c_packet = i2c_new_ancillary_device(i2c, "packet",
- ADV7511_PACKET_I2C_ADDR_DEFAULT);
+ ADV7511_PACKET_I2C_ADDR_DEFAULT, NULL);
if (IS_ERR(adv7511->i2c_packet)) {
ret = PTR_ERR(adv7511->i2c_packet);
goto err_i2c_unregister_edid;
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index ae3af738b03f..4f0964326968 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -1122,15 +1122,17 @@ EXPORT_SYMBOL_GPL(devm_i2c_new_dummy_device);
* @client: Handle to the primary client
* @name: Handle to specify which secondary address to get
* @default_addr: Used as a fallback if no secondary address was specified
+ * @aux_device_name: Ancillary device name
* Context: can sleep
*
* I2C clients can be composed of multiple I2C slaves bound together in a
single
* component. The I2C client driver then binds to the master I2C slave and
needs
- * to create I2C dummy clients to communicate with all the other slaves.
+ * to create I2C ancillary clients to communicate with all the other slaves.
*
- * This function creates and returns an I2C dummy client whose I2C address is
- * retrieved from the platform firmware based on the given slave name. If no
- * address is specified by the firmware default_addr is used.
+ * This function creates and returns an I2C ancillary client whose I2C address
+ * is retrieved from the platform firmware based on the given slave name. If no
+ * address is specified by the firmware default_addr is used. If no aux_device_
+ * name is specified by the firmware, it will create an I2C dummy client.
*
* On DT-based platforms the address is retrieved from the "reg" property entry
* cell whose "reg-names" value matches the slave name.
@@ -1139,10 +1141,12 @@ EXPORT_SYMBOL_GPL(devm_i2c_new_dummy_device);
* i2c_unregister_device(); or an ERR_PTR to describe the error.
*/
struct i2c_client *i2c_new_ancillary_device(struct i2c_client *client,
- const char *name,
- u16 default_addr)
+ const char *name,
+ u16 default_addr,
+ const char *aux_device_name)
{
struct device_node *np = client->dev.of_node;
+ struct i2c_client *i2c_aux_client;
u32 addr = default_addr;
int i;
@@ -1153,7 +1157,27 @@ struct i2c_client *i2c_new_ancillary_device(struct
i2c_client *client,
}
dev_dbg(&client->adapter->dev, "Address for %s : 0x%x\n", name, addr);
- return i2c_new_dummy_device(client->adapter, addr);
+
+ if (aux_device_name) {
+ struct i2c_board_info info;
+ size_t aux_device_name_len = strlen(aux_device_name);
+
+ if (aux_device_name_len > I2C_NAME_SIZE - 1) {
+
Re: [PATCH v4 01/13] dt-bindings: clk: g12a-clkc: export VCLK2_SEL and add CTS_ENCL clock ids
On 12/05/2023 15:11, Neil Armstrong wrote: > Expose VCLK2_SEL clock id and add new ids for the CTS_ENCL and CTS_ENCL_SEL > clocks on G12A compatible SoCs. > > Signed-off-by: Neil Armstrong > --- > drivers/clk/meson/g12a.h | 1 - > include/dt-bindings/clock/g12a-clkc.h | 3 +++ > 2 files changed, 3 insertions(+), 1 deletion(-) Bindings must be a separate patch from the driver changes. If this causes bisectability issues, this means entire solution breaks ABI and is not appropriate anyway... Best regards, Krzysztof
Re: [PATCH v4 04/13] dt-bindings: display: add Amlogic MIPI DSI Host Controller bindings
On 12/05/2023 15:11, Neil Armstrong wrote:
> The Amlogic G12A, G12B & SM1 SoCs embeds a Synopsys DW-MIPI-DSI transceiver
> (ver 1.21a),
> with a custom glue managing the IP resets, clock and data input similar to
> the DW-HDMI Glue
> on the same Amlogic SoCs.
Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v5.18-rc4/source/Documentation/process/submitting-patches.rst#L586
Subject: drop second/last, redundant "bindings". The "dt-bindings"
prefix is already stating that these are bindings.
>
> Signed-off-by: Neil Armstrong
> Signed-off-by: Neil Armstrong
> ---
> .../display/amlogic,meson-g12a-dw-mipi-dsi.yaml| 117
> +
> 1 file changed, 117 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/display/amlogic,meson-g12a-dw-mipi-dsi.yaml
>
> b/Documentation/devicetree/bindings/display/amlogic,meson-g12a-dw-mipi-dsi.yaml
> new file mode 100644
> index ..8169c7e93ff5
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/display/amlogic,meson-g12a-dw-mipi-dsi.yaml
> @@ -0,0 +1,117 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2020 BayLibre, SAS
> +%YAML 1.2
> +---
> +$id:
> http://devicetree.org/schemas/display/amlogic,meson-g12a-dw-mipi-dsi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Amlogic specific extensions to the Synopsys Designware MIPI DSI Host
> Controller
> +
> +maintainers:
> + - Neil Armstrong
> +
> +description: |
> + The Amlogic Meson Synopsys Designware Integration is composed of
> + - A Synopsys DesignWare MIPI DSI Host Controller IP
> + - A TOP control block controlling the Clocks & Resets of the IP
> +
> +allOf:
> + - $ref: dsi-controller.yaml#
> +
> +properties:
> + compatible:
> +enum:
> + - amlogic,meson-g12a-dw-mipi-dsi
> +
> + reg:
> +maxItems: 1
> +
> + clocks:
> +minItems: 3
Missing maxItems
> +
> + clock-names:
> +minItems: 3
> +items:
> + - const: pclk
> + - const: bit_clk
> + - const: px_clk
> + - const: meas_clk
Drop _clk suffixes. pclk can stay, it's a bit odd but recently Rob
clarified that suffix with underscore should not be there.
> +
> + resets:
> +minItems: 1
maxItems instead
> +
> + reset-names:
> +items:
> + - const: top
> +
> + phys:
> +minItems: 1
Ditto
> +
> + phy-names:
> +items:
> + - const: dphy
> +
> + ports:
> +$ref: /schemas/graph.yaml#/properties/ports
> +
> +properties:
> + port@0:
> +$ref: /schemas/graph.yaml#/properties/port
> +description: Input node to receive pixel data.
> +
> + port@1:
> +$ref: /schemas/graph.yaml#/properties/port
> +description: DSI output node to panel.
> +
> +required:
> + - port@0
> + - port@1
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - resets
> + - reset-names
> + - phys
> + - phy-names
> + - ports
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> +dsi@7000 {
> + compatible = "amlogic,meson-g12a-dw-mipi-dsi";
> + reg = <0x6000 0x400>;
Your reg does not match unit address. The dt_binding_check should
actually complain about it.
Best regards,
Krzysztof
Re: [PATCH v8 2/8] drm/msm/dpu: add DPU_PINGPONG_DSC feature bit for DPU < 7.0.0
On 2023-05-12 11:00:17, Kuogee Hsieh wrote:
>
> DPU < 7.0.0 requires the PINGPONG block to be involved during
> DSC setting up. Since DPU >= 7.0.0, enabling and starting the DSC
> encoder engine was moved to INTF with the help of the flush mechanism.
> Add a DPU_PINGPONG_DSC feature bit to restrict the availability of
> dpu_hw_pp_setup_dsc() and dpu_hw_pp_dsc_{enable,disable}() on the
> PINGPONG block to DPU < 7.0.0 hardware, as the registers are not
> available [in the PINGPONG block] on DPU 7.0.0 and higher anymore.
Fwiw I added the brackets in the suggestion as an "up to you to include
this or not". Drop the brackets if you think this should be part of the
sentence.
> Add DPU_PINGPONG_DSC to PINGPONG_SDM845_MASK, PINGPONG_SDM845_TE2_MASK
> and PINGPONG_SM8150_MASK which is used for all DPU < 7.0 chipsets.
>
> changes in v6:
> -- split patches and rearrange to keep catalog related files at this patch
>
> changes in v7:
> -- rewording commit text as suggested at review comments
>
> Signed-off-by: Kuogee Hsieh
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 6 +++---
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 +++-
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> index 82b58c6..78e4bf6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> @@ -76,13 +76,13 @@
> (BIT(DPU_DIM_LAYER) | BIT(DPU_MIXER_COMBINED_ALPHA))
>
> #define PINGPONG_SDM845_MASK \
> - (BIT(DPU_PINGPONG_DITHER) | BIT(DPU_PINGPONG_TE))
> + (BIT(DPU_PINGPONG_DITHER) | BIT(DPU_PINGPONG_TE) |
> BIT(DPU_PINGPONG_DSC))
>
> #define PINGPONG_SDM845_TE2_MASK \
> - (PINGPONG_SDM845_MASK | BIT(DPU_PINGPONG_TE2))
> + (PINGPONG_SDM845_MASK | BIT(DPU_PINGPONG_TE2) | BIT(DPU_PINGPONG_DSC))
Don't add it here, this is already in PINGPONG_SDM845_MASK.
>
> #define PINGPONG_SM8150_MASK \
> - (BIT(DPU_PINGPONG_DITHER))
> + (BIT(DPU_PINGPONG_DITHER) | BIT(DPU_PINGPONG_DSC))
>
> #define CTL_SC7280_MASK \
> (BIT(DPU_CTL_ACTIVE_CFG) | \
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> index 6ee48f0..dc0a4da 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> @@ -144,7 +144,8 @@ enum {
> * @DPU_PINGPONG_TE2Additional tear check block for split pipes
> * @DPU_PINGPONG_SPLIT PP block supports split fifo
> * @DPU_PINGPONG_SLAVE PP block is a suitable slave for split fifo
> - * @DPU_PINGPONG_DITHER,Dither blocks
> + * @DPU_PINGPONG_DITHER Dither blocks
> + * @DPU_PINGPONG_DSCPP ops functions required for DSC
Following the other documentation wording:
PP block supports DSC
Or:
PP block has DSC enable/disable registers
- Marijn
> * @DPU_PINGPONG_MAX
> */
> enum {
> @@ -153,6 +154,7 @@ enum {
> DPU_PINGPONG_SPLIT,
> DPU_PINGPONG_SLAVE,
> DPU_PINGPONG_DITHER,
> + DPU_PINGPONG_DSC,
> DPU_PINGPONG_MAX
> };
>
> --
> 2.7.4
>
[PATCH] drm: bridge: dw-mipi-dsi: Drop panel_bridge post_disable call
This panel_bridge post_disable callback is called from the bridge chain now, so drop the explicit call here. This fixes call imbalance, where this driver does not call ->pre_enable, but does call ->post_disable . In case either of the two callbacks implemented in one of the panel or bridge drivers contains any operation with refcounted object, like regulator, this would make kernel complain about the imbalance. This can be triggered e.g. with ST7701 driver, which operates on regulators in its prepare/unprepare callbacks, which are called from pre_enable/post_disable callbacks respectively. The former is called once, the later twice, during entry to suspend. Drop the post_disable call to fix the imbalance. Signed-off-by: Marek Vasut --- Cc: Andrzej Hajda Cc: Antonio Borneo Cc: Daniel Vetter Cc: David Airlie Cc: Jernej Skrabec Cc: Jonas Karlman Cc: Laurent Pinchart Cc: Marek Vasut Cc: Neil Armstrong Cc: Philipp Zabel Cc: Philippe Cornu Cc: Robert Foss Cc: Vincent Abriou Cc: Yannick Fertre Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 9 - 1 file changed, 9 deletions(-) diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c index b2efecf7d1603..63ac972547361 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c @@ -859,15 +859,6 @@ static void dw_mipi_dsi_bridge_post_atomic_disable(struct drm_bridge *bridge, */ dw_mipi_dsi_set_mode(dsi, 0); - /* -* TODO Only way found to call panel-bridge post_disable & -* panel unprepare before the dsi "final" disable... -* This needs to be fixed in the drm_bridge framework and the API -* needs to be updated to manage our own call chains... -*/ - if (dsi->panel_bridge->funcs->post_disable) - dsi->panel_bridge->funcs->post_disable(dsi->panel_bridge); - if (phy_ops->power_off) phy_ops->power_off(dsi->plat_data->priv_data); -- 2.39.2
Re: [PATCH v10 1/8] drm/display/dsc: Add flatness and initial scale value calculations
On 2023-05-12 14:32:11, Jessica Zhang wrote:
>
> Add helpers to calculate det_thresh_flatness and initial_scale_value as
> these calculations are defined within the DSC spec.
>
> Reviewed-by: Marijn Suijten
> Signed-off-by: Jessica Zhang
> ---
> include/drm/display/drm_dsc_helper.h | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/include/drm/display/drm_dsc_helper.h
> b/include/drm/display/drm_dsc_helper.h
> index 0bb0c3afd740..060b22ec02eb 100644
> --- a/include/drm/display/drm_dsc_helper.h
> +++ b/include/drm/display/drm_dsc_helper.h
> @@ -25,5 +25,15 @@ void drm_dsc_set_rc_buf_thresh(struct drm_dsc_config
> *vdsc_cfg);
> int drm_dsc_setup_rc_params(struct drm_dsc_config *vdsc_cfg, enum
> drm_dsc_params_kind kind);
> int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg);
>
> +static inline int drm_dsc_initial_scale_value(const struct drm_dsc_config
> *dsc)
Should this truncate and return u8?
> +{
> + return 8 * dsc->rc_model_size / (dsc->rc_model_size -
> dsc->initial_offset);
> +}
> +
> +static inline int drm_dsc_flatness_det_thresh(const struct drm_dsc_config
> *dsc)
Should this return u32?
- Marijn
> +{
> + return 2 << (dsc->bits_per_component - 8);
> +}
> +
> #endif /* _DRM_DSC_HELPER_H_ */
>
>
> --
> 2.40.1
>
Re: [PATCH v5 2/8] drm/i915/dsc: move rc_buf_thresh values to common helper
On 2023-05-04 18:35:05, Dmitry Baryshkov wrote:
>
> The rc_buf_thresh values are common to all DSC implementations. Move
> them to the common helper together with the code to propagage them to
Propagate*
> the drm_dsc_config.
>
> Reviewed-by: Jani Nikula
> Signed-off-by: Dmitry Baryshkov
After right-shifting these values by 6 they are indeed, as promised,
identical to the values used in MSM.
Reviewed-by: Marijn Suijten
If a tested-by is relevant in addition to r-b, let me know. This works
(no regressions) on quite a few MSM devices on my end.
(same question for the other patches)
- Marijn
> ---
> drivers/gpu/drm/display/drm_dsc_helper.c | 35 +++
> drivers/gpu/drm/i915/display/intel_vdsc.c | 24 +---
> include/drm/display/drm_dsc_helper.h | 1 +
> 3 files changed, 37 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c
> b/drivers/gpu/drm/display/drm_dsc_helper.c
> index c869c6e51e2b..be91abe2cfb2 100644
> --- a/drivers/gpu/drm/display/drm_dsc_helper.c
> +++ b/drivers/gpu/drm/display/drm_dsc_helper.c
> @@ -270,6 +270,41 @@ void drm_dsc_pps_payload_pack(struct
> drm_dsc_picture_parameter_set *pps_payload,
> }
> EXPORT_SYMBOL(drm_dsc_pps_payload_pack);
>
> +/* From DSC_v1.11 spec, rc_parameter_Set syntax element typically constant */
> +static const u16 drm_dsc_rc_buf_thresh[] = {
> + 896, 1792, 2688, 3584, 4480, 5376, 6272, 6720, 7168, 7616,
> + 7744, 7872, 8000, 8064
> +};
> +
> +/**
> + * drm_dsc_set_rc_buf_thresh() - Set thresholds for the RC model
> + * in accordance with the DSC 1.2 specification.
> + *
> + * @vdsc_cfg: DSC Configuration data partially filled by driver
> + */
> +void drm_dsc_set_rc_buf_thresh(struct drm_dsc_config *vdsc_cfg)
> +{
> + int i;
> +
> + BUILD_BUG_ON(ARRAY_SIZE(drm_dsc_rc_buf_thresh) !=
> + DSC_NUM_BUF_RANGES - 1);
> + BUILD_BUG_ON(ARRAY_SIZE(drm_dsc_rc_buf_thresh) !=
> + ARRAY_SIZE(vdsc_cfg->rc_buf_thresh));
> +
> + for (i = 0; i < ARRAY_SIZE(drm_dsc_rc_buf_thresh); i++)
> + vdsc_cfg->rc_buf_thresh[i] = drm_dsc_rc_buf_thresh[i] >> 6;
> +
> + /*
> + * For 6bpp, RC Buffer threshold 12 and 13 need a different value
> + * as per C Model
> + */
> + if (vdsc_cfg->bits_per_pixel == 6 << 4) {
> + vdsc_cfg->rc_buf_thresh[12] = 7936 >> 6;
> + vdsc_cfg->rc_buf_thresh[13] = 8000 >> 6;
> + }
> +}
> +EXPORT_SYMBOL(drm_dsc_set_rc_buf_thresh);
> +
> /**
> * drm_dsc_compute_rc_parameters() - Write rate control
> * parameters to the dsc configuration defined in
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index 7003ae9f683a..2fd08375bbe3 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -37,12 +37,6 @@ enum COLUMN_INDEX_BPC {
> MAX_COLUMN_INDEX
> };
>
> -/* From DSC_v1.11 spec, rc_parameter_Set syntax element typically constant */
> -static const u16 rc_buf_thresh[] = {
> - 896, 1792, 2688, 3584, 4480, 5376, 6272, 6720, 7168, 7616,
> - 7744, 7872, 8000, 8064
> -};
> -
> struct rc_parameters {
> u16 initial_xmit_delay;
> u8 first_line_bpg_offset;
> @@ -543,23 +537,7 @@ int intel_dsc_compute_params(struct intel_crtc_state
> *pipe_config)
>
> vdsc_cfg->bits_per_component = pipe_config->pipe_bpp / 3;
>
> - for (i = 0; i < DSC_NUM_BUF_RANGES - 1; i++) {
> - /*
> - * six 0s are appended to the lsb of each threshold value
> - * internally in h/w.
> - * Only 8 bits are allowed for programming RcBufThreshold
> - */
> - vdsc_cfg->rc_buf_thresh[i] = rc_buf_thresh[i] >> 6;
> - }
> -
> - /*
> - * For 6bpp, RC Buffer threshold 12 and 13 need a different value
> - * as per C Model
> - */
> - if (compressed_bpp == 6) {
> - vdsc_cfg->rc_buf_thresh[12] = 0x7C;
> - vdsc_cfg->rc_buf_thresh[13] = 0x7D;
> - }
> + drm_dsc_set_rc_buf_thresh(vdsc_cfg);
>
> /*
>* From XE_LPD onwards we supports compression bpps in steps of 1
> diff --git a/include/drm/display/drm_dsc_helper.h
> b/include/drm/display/drm_dsc_helper.h
> index 8b41edbbabab..706ba1d34742 100644
> --- a/include/drm/display/drm_dsc_helper.h
> +++ b/include/drm/display/drm_dsc_helper.h
> @@ -14,6 +14,7 @@ void drm_dsc_dp_pps_header_init(struct dp_sdp_header
> *pps_header);
> int drm_dsc_dp_rc_buffer_size(u8 rc_buffer_block_size, u8 rc_buffer_size);
> void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_sdp,
> const struct drm_dsc_config *dsc_cfg);
> +void drm_dsc_set_rc_buf_thresh(struct drm_dsc_config *vdsc_cfg);
> int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg);
>
> #endif /* _DRM_DSC_HELPER_H_ */
> --
> 2.39.2
>
Re: [PATCH v10 3/8] drm/msm/dsi: use DRM DSC helpers for DSC setup
On 2023-05-12 14:32:13, Jessica Zhang wrote:
>
> From: Dmitry Baryshkov
>
> Use new DRM DSC helpers to setup DSI DSC configuration. The
> initial_scale_value needs to be adjusted according to the standard, but
> this is a separate change.
>
> Signed-off-by: Dmitry Baryshkov
> Reviewed-by: Abhinav Kumar
> Signed-off-by: Jessica Zhang
All the parameters check out.
Reviewed-by: Marijn Suijten
(And as asked elsewhere: is it valuable to have t-b on top of this r-b,
for all the devices/boards/SoCs/panels I have these patches working
on?)
> ---
> drivers/gpu/drm/msm/dsi/dsi_host.c | 61
> +-
> 1 file changed, 8 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 961689a255c4..74d38f90398a 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -1731,28 +1731,9 @@ static int dsi_host_parse_lane_data(struct
> msm_dsi_host *msm_host,
> return -EINVAL;
> }
>
> -static u32 dsi_dsc_rc_buf_thresh[DSC_NUM_BUF_RANGES - 1] = {
> - 0x0e, 0x1c, 0x2a, 0x38, 0x46, 0x54, 0x62,
> - 0x69, 0x70, 0x77, 0x79, 0x7b, 0x7d, 0x7e
> -};
> -
> -/* only 8bpc, 8bpp added */
> -static char min_qp[DSC_NUM_BUF_RANGES] = {
> - 0, 0, 1, 1, 3, 3, 3, 3, 3, 3, 5, 5, 5, 7, 13
> -};
> -
> -static char max_qp[DSC_NUM_BUF_RANGES] = {
> - 4, 4, 5, 6, 7, 7, 7, 8, 9, 10, 11, 12, 13, 13, 15
> -};
> -
> -static char bpg_offset[DSC_NUM_BUF_RANGES] = {
> - 2, 0, 0, -2, -4, -6, -8, -8, -8, -10, -10, -12, -12, -12, -12
> -};
> -
> static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct
> drm_dsc_config *dsc)
> {
> - int i;
> - u16 bpp = dsc->bits_per_pixel >> 4;
> + int ret;
>
> if (dsc->bits_per_pixel & 0xf) {
> DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support
> fractional bits_per_pixel\n");
> @@ -1764,49 +1745,23 @@ static int dsi_populate_dsc_params(struct
> msm_dsi_host *msm_host, struct drm_dsc
> return -EOPNOTSUPP;
> }
>
> - dsc->rc_model_size = 8192;
> - dsc->first_line_bpg_offset = 12;
> - dsc->rc_edge_factor = 6;
> - dsc->rc_tgt_offset_high = 3;
> - dsc->rc_tgt_offset_low = 3;
> dsc->simple_422 = 0;
> dsc->convert_rgb = 1;
> dsc->vbr_enable = 0;
>
> - /* handle only bpp = bpc = 8 */
> - for (i = 0; i < DSC_NUM_BUF_RANGES - 1 ; i++)
> - dsc->rc_buf_thresh[i] = dsi_dsc_rc_buf_thresh[i];
> + drm_dsc_set_const_params(dsc);
> + drm_dsc_set_rc_buf_thresh(dsc);
>
> - for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
> - dsc->rc_range_params[i].range_min_qp = min_qp[i];
> - dsc->rc_range_params[i].range_max_qp = max_qp[i];
> - /*
> - * Range BPG Offset contains two's-complement signed values
> that fill
> - * 8 bits, yet the registers and DCS PPS field are only 6 bits
> wide.
> - */
I wish drm_dsc_setup_rc_params() used this comment :)
> - dsc->rc_range_params[i].range_bpg_offset = bpg_offset[i] &
> DSC_RANGE_BPG_OFFSET_MASK;
> + /* handle only bpp = bpc = 8, pre-SCR panels */
> + ret = drm_dsc_setup_rc_params(dsc, DRM_DSC_1_1_PRE_SCR);
> + if (ret) {
> + DRM_DEV_ERROR(&msm_host->pdev->dev, "could not find DSC RC
> parameters\n");
> + return ret;
> }
>
> - dsc->initial_offset = 6144; /* Not bpp 12 */
> - if (bpp != 8)
> - dsc->initial_offset = 2048; /* bpp = 12 */
> -
> - if (dsc->bits_per_component <= 10)
> - dsc->mux_word_size = DSC_MUX_WORD_SIZE_8_10_BPC;
> - else
> - dsc->mux_word_size = DSC_MUX_WORD_SIZE_12_BPC;
> -
> - dsc->initial_xmit_delay = 512;
> dsc->initial_scale_value = 32;
> - dsc->first_line_bpg_offset = 12;
> dsc->line_buf_depth = dsc->bits_per_component + 1;
>
> - /* bpc 8 */
> - dsc->flatness_min_qp = 3;
> - dsc->flatness_max_qp = 12;
> - dsc->rc_quant_incr_limit0 = 11;
> - dsc->rc_quant_incr_limit1 = 11;
> -
> return drm_dsc_compute_rc_parameters(dsc);
> }
>
>
> --
> 2.40.1
>
[GIT PULL] fbdev fixes and updates for v6.4-rc2
The following changes since commit ac9a78681b921877518763ba0e89202254349d1b: Linux 6.4-rc1 (2023-05-07 13:34:35 -0700) are available in the Git repository at: http://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git tags/fbdev-for-6.4-rc2 for you to fetch changes up to 0bdf1ad8d10bd4e50a8b1a2c53d15984165f7fea: fbdev: stifb: Fix info entry in sti_struct on error path (2023-05-12 11:50:33 +0200) fbdev fixes and updates for kernel 6.4-rc2: - use after free fix in imsttfb (Zheng Wang) - fix error handling in arcfb (Zongjie Li) - lots of whitespace cleanups (Thomas Zimmermann) - add 1920x1080 modedb entry (me) Helge Deller (2): fbdev: modedb: Add 1920x1080 at 60 Hz video mode fbdev: stifb: Fix info entry in sti_struct on error path Thomas Zimmermann (15): fbdev: 68328fb: Remove trailing whitespaces fbdev: atmel_lcdfb: Remove trailing whitespaces fbdev: cg14: Remove trailing whitespaces fbdev: controlfb: Remove trailing whitespaces fbdev: g364fb: Remove trailing whitespaces fbdev: hgafb: Remove trailing whitespaces fbdev: hpfb: Remove trailing whitespaces fbdev: macfb: Remove trailing whitespaces fbdev: maxinefb: Remove trailing whitespaces fbdev: p9100: Remove trailing whitespaces fbdev: platinumfb: Remove trailing whitespaces fbdev: sa1100fb: Remove trailing whitespaces fbdev: stifb: Remove trailing whitespaces fbdev: valkyriefb: Remove trailing whitespaces fbdev: vfb: Remove trailing whitespaces Zheng Wang (1): fbdev: imsttfb: Fix use after free bug in imsttfb_probe Zongjie Li (1): fbdev: arcfb: Fix error handling in arcfb_probe() drivers/video/fbdev/68328fb.c | 12 +-- drivers/video/fbdev/arcfb.c | 15 ++-- drivers/video/fbdev/atmel_lcdfb.c | 2 +- drivers/video/fbdev/cg14.c| 2 +- drivers/video/fbdev/controlfb.c | 34 - drivers/video/fbdev/core/modedb.c | 5 ++ drivers/video/fbdev/g364fb.c | 6 +- drivers/video/fbdev/hgafb.c | 36 - drivers/video/fbdev/hpfb.c| 8 +- drivers/video/fbdev/imsttfb.c | 15 ++-- drivers/video/fbdev/macfb.c | 10 +-- drivers/video/fbdev/maxinefb.c| 2 +- drivers/video/fbdev/p9100.c | 4 +- drivers/video/fbdev/platinumfb.c | 30 drivers/video/fbdev/sa1100fb.c| 32 drivers/video/fbdev/stifb.c | 157 +++--- drivers/video/fbdev/valkyriefb.c | 14 ++-- drivers/video/fbdev/vfb.c | 10 +-- 18 files changed, 202 insertions(+), 192 deletions(-)
Re: [GIT PULL] fbdev fixes and updates for v6.4-rc2
The pull request you sent on Sun, 14 May 2023 01:32:11 +0200: > http://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git > tags/fbdev-for-6.4-rc2 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/adfbf653a3ba6bb8bbb84ed90bf4f1533db545d3 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [PATCH v2 08/10] drivers: watchdog: Replace GPL license notice with SPDX identifier
On Sat, May 13, 2023 at 09:43:39AM -0400, Richard Fontana wrote: > On Sat, May 13, 2023 at 6:53 AM Bagas Sanjaya wrote: > > > > On 5/12/23 19:46, Richard Fontana wrote: > > > On Fri, May 12, 2023 at 6:07 AM Bagas Sanjaya > > > wrote: > > > > > > > > >> diff --git a/drivers/watchdog/sb_wdog.c b/drivers/watchdog/sb_wdog.c > > >> index 504be461f992a9..822bf8905bf3ce 100644 > > >> --- a/drivers/watchdog/sb_wdog.c > > >> +++ b/drivers/watchdog/sb_wdog.c > > >> @@ -1,3 +1,4 @@ > > >> +// SPDX-License-Identifier: GPL-1.0+ > > >> /* > > >> * Watchdog driver for SiByte SB1 SoCs > > >> * > > >> @@ -38,10 +39,6 @@ > > >> * (c) Copyright 1996 Alan Cox , > > >> * All Rights Reserved. > > >> * > > >> - * This program is free software; you can redistribute it and/or > > >> - * modify it under the terms of the GNU General Public License > > >> - * version 1 or 2 as published by the Free Software Foundation. > > > > > > Shouldn't this be > > > // SPDX-License-Identifier: GPL-1.0 OR GPL-2.0 > > > (or in current SPDX notation GPL-1.0-only OR GPL-2.0-only) ? > > > > > > > Nope, as it will fail spdxcheck.py. Also, SPDX specification [1] > > doesn't have negation operator (NOT), thus the licensing requirement > > on the above notice can't be expressed reliably in SPDX here. > > > > [1]: https://spdx.github.io/spdx-spec/v2.3/SPDX-license-expressions/ > > The GPL identifiers in recent versions of SPDX include an `-only` and > an `-or-later` variant. But Linux does not use the newer versions of SPDX given that we started the conversion before the "-only" variant came out. Let's stick with the original one please before worrying about converting to a newer version of SPDX and mixing things up. thanks, greg k-h
