Re: [PATCH v2 1/9] media: v4l2-subdev: Add a pad variant of .query_dv_timings()
On 28/02/2024 16:34, Paweł Anikiel wrote: > On Wed, Feb 28, 2024 at 12:25 PM Hans Verkuil > wrote: >> >> Hi Paweł, >> >> On 21/02/2024 17:02, Paweł Anikiel wrote: >>> Currently, .query_dv_timings() is defined as a video callback without >>> a pad argument. This is a problem if the subdevice can have different >>> dv timings for each pad (e.g. a DisplayPort receiver with multiple >>> virtual channels). >>> >>> To solve this, add a pad variant of this callback which includes >>> the pad number as an argument. >> >> So now we have two query_dv_timings ops: one for video ops, and one >> for pad ops. That's not very maintainable. I would suggest switching >> all current users of the video op over to the pad op. > > I agree it would be better if there was only one. However, I have some > concerns: > 1. Isn't there a problem with backwards compatibility? For example, an > out-of-tree driver is likely to use this callback, which would break. > I'm asking because I'm not familiar with how such API changes are > handled. It's out of tree, so they will just have to adapt. That's how life is if you are not part of the mainline kernel. > 2. If I do switch all current users to the pad op, I can't test those > changes. Isn't that a problem? I can test one or two drivers, but in general I don't expect this to be a problem. > 3. Would I need to get ACK from all the driver maintainers? CC the patches to the maintainers. Generally you will get back Acks from some but not all maintainers, but that's OK. For changes affecting multiple drivers you never reach 100% on that. I can review the remainder. The DV Timings API is my expert area, so that shouldn't be a problem. A quick grep gives me these subdev drivers that implement it: adv748x, adv7604, adv7842, tc358743, tda1997x, tvp7002, gs1662. And these bridge drivers that call the subdevs: cobalt, rcar-vin, vpif_capture. The bridge drivers can use the following pad when calling query_dv_timings: cobalt: ADV76XX_PAD_HDMI_PORT_A rcar_vin: vin->parallel.sink_pad vpif_capture: 0 The converted subdev drivers should check if the pad is an input pad. Ideally it should check if the pad is equal to the current input pad since most devices can only query the timings for the currently selected input pad. But some older drivers predate the pad concept and they ignore the pad value. Regards, Hans
[PATCH] drm/msm/a7xx: allow writing to CP_BV counter selection registers
From: Zan Dobersek
In addition to the CP_PERFCTR_CP_SEL register range, allow writes to the
CP_BV_PERFCTR_CP_SEL registers in the 0x8e0-0x8e6 range for profiling
purposes of tools like fdperf and perfetto.
Signed-off-by: Zan Dobersek
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index c9c55e2ea584..09c554f2fcf6 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1175,8 +1175,9 @@ static const u32 a730_protect[] = {
A6XX_PROTECT_NORDWR(0x00699, 0x01e9),
A6XX_PROTECT_NORDWR(0x008a0, 0x0008),
A6XX_PROTECT_NORDWR(0x008ab, 0x0024),
- /* 0x008d0-0x008dd are unprotected on purpose for tools like perfetto */
- A6XX_PROTECT_RDONLY(0x008de, 0x0154),
+ /* 0x008d0-0x008dd and 0x008e0-0x008e6 are unprotected on purpose for
tools like perfetto */
+ A6XX_PROTECT_NORDWR(0x008de, 0x0001),
+ A6XX_PROTECT_RDONLY(0x008e7, 0x014b),
A6XX_PROTECT_NORDWR(0x00900, 0x004d),
A6XX_PROTECT_NORDWR(0x0098d, 0x00b2),
A6XX_PROTECT_NORDWR(0x00a41, 0x01be),
--
2.43.0
Re: [PATCH 4/4] drm/atomic-helper: Add select_output_bus_format callback
Hi,
On Wed, Feb 28, 2024 at 10:00:19PM +, Klymenko, Anatoliy wrote:
> > > diff --git a/include/drm/drm_modeset_helper_vtables.h
> > > b/include/drm/drm_modeset_helper_vtables.h
> > > index 881b03e4dc28..7c21ae1fe3ad 100644
> > > --- a/include/drm/drm_modeset_helper_vtables.h
> > > +++ b/include/drm/drm_modeset_helper_vtables.h
> > > @@ -489,6 +489,37 @@ struct drm_crtc_helper_funcs {
> > >bool in_vblank_irq, int *vpos, int *hpos,
> > >ktime_t *stime, ktime_t *etime,
> > >const struct drm_display_mode *mode);
> > > +
> > > + /**
> > > + * @select_output_bus_format
> > > + *
> > > + * Called by the first connected DRM bridge to negotiate input media
> > > + * bus format. CRTC is expected to pick preferable media formats from
> > > + * the list supported by the DRM bridge chain.
> >
> > There's nothing restricting it to bridges here. Please rephrase this to
> > remove the
> > bridge mention. The user is typically going to be the encoder, and bridges
> > are just
> > an automagic implementation of an encoder.
> >
>
> OK. I'll fix than in the next version.
>
> > And generally speaking, I'd really like to have an implementation available
> > before
> > merging this.
> >
>
> Well, 2 instances of this callback implementations exist as drafts, as
> this is the new API. A little bit of a chicken and egg problem. I'll
> try to groom at least one of them into upstreamable shape and attach
> it to the patch set.
That's totally what I meant :)
I basically don't want to have an interface that isn't used. If you
provide an implementation in the same series, it's totally reasonable
Maxime
signature.asc
Description: PGP signature
Re: [PATCH 2/3] dt-bindings: display: mediatek: gamma: Add support for MT8188
On 29/02/2024 03:35, Jason-JH.Lin wrote: > The gamma LUT setting of MT8188 and MT8195 are the same, so we create > a one of items for MT8188 to reuse the driver data settings of MT8195. > > Signed-off-by: Jason-JH.Lin > --- > .../devicetree/bindings/display/mediatek/mediatek,gamma.yaml | 4 > 1 file changed, 4 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml > b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml > index 3e6cb8f48bcc..90c454eea06f 100644 > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml > @@ -29,6 +29,10 @@ properties: >- enum: >- mediatek,mt6795-disp-gamma >- const: mediatek,mt8173-disp-gamma > + - items: > + - enum: > + - mediatek,mt8188-disp-gamma > + - const: mediatek,mt8195-disp-gamma >- items: >- enum: >- mediatek,mt8186-disp-gamma Please keep this ordered by fallback compatible, so your list with 8195 fallback should go below the list here. Best regards, Krzysztof
Re: [PATCH 1/3] dt-bindings: display: mediatek: gamma: Change MT8195 to single enum group
On 29/02/2024 03:35, Jason-JH.Lin wrote: > Since MT8195 gamma has multiple bank for 12 bits LUT and it is > different from any other SoC LUT setting. > > So we move MT8195 compatible from the one of items to the > single enum group. > > Signed-off-by: Jason-JH.Lin > --- Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v3 00/10] Make PCI's devres API more consistent
@Bjorn: Hey Bjorn, are we good with this series? Any more wishes or suggestions? P. On Tue, 2024-02-06 at 14:39 +0100, Philipp Stanner wrote: > Changes in v3: > - Use the term "PCI devres API" in some forgotten places. > - Fix more grammar errors in patch #3. > - Remove the comment advising to call (the outdated) pcim_intx() in > pci.c > - Rename __pcim_request_region_range() flags-field "exclusive" to > "req_flags", since this is what the int actually represents. > - Remove the call to pcim_region_request() from patch #10. (Hans) > > Changes in v2: > - Make commit head lines congruent with PCI's style (Bjorn) > - Add missing error checks for devm_add_action(). (Andy) > - Repair the "Returns: " marks for docu generation (Andy) > - Initialize the addr_devres struct with memset(). (Andy) > - Make pcim_intx() a PCI-internal function so that new drivers > won't > be encouraged to use the outdated pci_intx() mechanism. > (Andy / Philipp) > - Fix grammar and spelling (Bjorn) > - Be more precise on why pcim_iomap_table() is problematic (Bjorn) > - Provide the actual structs' and functions' names in the commit > messages (Bjorn) > - Remove redundant variable initializers (Andy) > - Regroup PM bitfield members in struct pci_dev (Andy) > - Make pcim_intx() visible only for the PCI subsystem so that > new > drivers won't use this outdated API (Andy, Myself) > - Add a NOTE to pcim_iomap() to warn about this function being > the onee > xception that does just return NULL. > - Consistently use the term "PCI devres API"; also in Patch #10 > (Bjorn) > > > ¡Hola! > > PCI's devres API suffers several weaknesses: > > 1. There are functions prefixed with pcim_. Those are always managed > counterparts to never-managed functions prefixed with pci_ – or so > one > would like to think. There are some apparently unmanaged functions > (all region-request / release functions, and pci_intx()) which > suddenly become managed once the user has initialized the device > with > pcim_enable_device() instead of pci_enable_device(). This > "sometimes > yes, sometimes no" nature of those functions is confusing and > therefore bug-provoking. In fact, it has already caused a bug in > DRM. > The last patch in this series fixes that bug. > 2. iomappings: Instead of giving each mapping its own callback, the > existing API uses a statically allocated struct tracking one > mapping > per bar. This is not extensible. Especially, you can't create > _ranged_ managed mappings that way, which many drivers want. > 3. Managed request functions only exist as "plural versions" with a > bit-mask as a parameter. That's quite over-engineered considering > that each user only ever mapps one, maybe two bars. > > This series: > - add a set of new "singular" devres functions that use devres the > way > its intended, with one callback per resource. > - deprecates the existing iomap-table mechanism. > - deprecates the hybrid nature of pci_ functions. > - preserves backwards compatibility so that drivers using the > existing > API won't notice any changes. > - adds documentation, especially some warning users about the > complicated nature of PCI's devres. > > > Note that this series is based on my "unify pci_iounmap"-series from > a > few weeks ago. [1] > > I tested this on a x86 VM with a simple pci test-device with two > regions. Operates and reserves resources as intended on my system. > Kasan and kmemleak didn't find any problems. > > I believe this series cleans the API up as much as possible without > having to port all existing drivers to the new API. Especially, I > think > that this implementation is easy to extend if the need for new > managed > functions arises :) > > Greetings, > P. > > Philipp Stanner (10): > PCI: Add new set of devres functions > PCI: Deprecate iomap-table functions > PCI: Warn users about complicated devres nature > PCI: Make devres region requests consistent > PCI: Move dev-enabled status bit to struct pci_dev > PCI: Move pinned status bit to struct pci_dev > PCI: Give pcim_set_mwi() its own devres callback > PCI: Give pci(m)_intx its own devres callback > PCI: Remove legacy pcim_release() > drm/vboxvideo: fix mapping leaks > > drivers/gpu/drm/vboxvideo/vbox_main.c | 20 +- > drivers/pci/devres.c | 1015 +-- > -- > drivers/pci/iomap.c | 18 + > drivers/pci/pci.c | 123 ++- > drivers/pci/pci.h | 25 +- > include/linux/pci.h | 18 +- > 6 files changed, 1004 insertions(+), 215 deletions(-) >
RE: [PATCH 1/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing
> -Original Message- > From: Helen Koike > > Hey all, > > You can check the validation of this patchset on: > https://gitlab.collabora.com/koike/linux/-/pipelines/87035 > > I would appreciate your feedback on this work, what do you think? > > If you would rate from 0 to 5, where: > > [ ] 0. I don't think this is useful at all, and I doubt it will ever be. It > doesn't seem worthwhile. > [ ] 1. I don't find it useful in its current form. > [ ] 2. It might be useful to others, but not for me. > [ ] 3. It has potential, but it's not yet something I can incorporate into my > workflow. > [ ] 4. This is useful, but it needs some adjustments before I can include it > in my workflow. > [ ] 5. This is really useful! I'm eager to start using it right away. Why > didn't you send this earlier? :) > > Which rating would you select? For me, this is a "5". I don't currently use gitlab, but I might install it just to test this out. It looks like there are a large number of YAML files which define the integration between the test scripts and gitlab. Also, there are a number of shell scripts to perform some of the setup and tests. Do you have any idea how difficult it would be to use the shell scripts outside of the CI system (e.g. manually)? That is, are there dependencies between the CI system and the shell scripts? I think the convention, of putting this kind of stuff under a "ci" directory, makes sense. And it appears that the sub-dir structure allows for other CI systems to add their own config and files also. I have a few minor comments below. > > --- > .gitlab-ci.yml| 2 + > MAINTAINERS | 8 ++ > ci/gitlab-ci/bootstrap-gitlab-runner.sh | 55 + > ci/gitlab-ci/ci-scripts/build-docs.sh | 35 ++ > ci/gitlab-ci/ci-scripts/build-kernel.sh | 35 ++ > ci/gitlab-ci/ci-scripts/ici-functions.sh | 104 ++ > ci/gitlab-ci/ci-scripts/install-smatch.sh | 13 +++ > .../ci-scripts/parse_commit_message.sh| 27 + > ci/gitlab-ci/ci-scripts/run-checkpatch.sh | 20 > ci/gitlab-ci/ci-scripts/run-smatch.sh | 45 > ci/gitlab-ci/docker-compose.yaml | 18 +++ > ci/gitlab-ci/linux.code-workspace | 11 ++ > ci/gitlab-ci/yml/build.yml| 43 > ci/gitlab-ci/yml/cache.yml| 26 + > ci/gitlab-ci/yml/container.yml| 36 ++ > ci/gitlab-ci/yml/gitlab-ci.yml| 71 > ci/gitlab-ci/yml/kernel-combinations.yml | 18 +++ > ci/gitlab-ci/yml/scenarios.yml| 12 ++ > ci/gitlab-ci/yml/scenarios/file-systems.yml | 21 > ci/gitlab-ci/yml/scenarios/media.yml | 21 > ci/gitlab-ci/yml/scenarios/network.yml| 21 > ci/gitlab-ci/yml/static-checks.yml| 21 > 22 files changed, 663 insertions(+) > create mode 100644 .gitlab-ci.yml > create mode 100755 ci/gitlab-ci/bootstrap-gitlab-runner.sh > create mode 100755 ci/gitlab-ci/ci-scripts/build-docs.sh > create mode 100755 ci/gitlab-ci/ci-scripts/build-kernel.sh > create mode 100644 ci/gitlab-ci/ci-scripts/ici-functions.sh > create mode 100755 ci/gitlab-ci/ci-scripts/install-smatch.sh > create mode 100755 ci/gitlab-ci/ci-scripts/parse_commit_message.sh > create mode 100755 ci/gitlab-ci/ci-scripts/run-checkpatch.sh > create mode 100755 ci/gitlab-ci/ci-scripts/run-smatch.sh > create mode 100644 ci/gitlab-ci/docker-compose.yaml > create mode 100644 ci/gitlab-ci/linux.code-workspace > create mode 100644 ci/gitlab-ci/yml/build.yml > create mode 100644 ci/gitlab-ci/yml/cache.yml > create mode 100644 ci/gitlab-ci/yml/container.yml > create mode 100644 ci/gitlab-ci/yml/gitlab-ci.yml > create mode 100644 ci/gitlab-ci/yml/kernel-combinations.yml > create mode 100644 ci/gitlab-ci/yml/scenarios.yml > create mode 100644 ci/gitlab-ci/yml/scenarios/file-systems.yml > create mode 100644 ci/gitlab-ci/yml/scenarios/media.yml > create mode 100644 ci/gitlab-ci/yml/scenarios/network.yml > create mode 100644 ci/gitlab-ci/yml/static-checks.yml > > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml > new file mode 100644 > index 0..fac523bb86866 > --- /dev/null > +++ b/.gitlab-ci.yml > @@ -0,0 +1,2 @@ > +include: > + - ci/gitlab-ci/yml/gitlab-ci.yml > diff --git a/MAINTAINERS b/MAINTAINERS > index 716b2e22598c8..aa0f65791c2ee 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4998,6 +4998,14 @@ T: git git://linuxtv.org/media_tree.git > F: Documentation/devicetree/bindings/media/i2c/chrontel,ch7322.yaml > F: drivers/media/cec/i2c/ch7322.c > > +CI AUTOMATED TESTING > +M: Helen Koike > +L: linux-ker...@vger.kernel.org > +S: Maintained > +T: git https://gitlab.collabora.com/koike/linux.git > +F: .gitlab-ci.yml > +F: ci/ > + > CIRRUS LOGIC AUDIO CODEC DRIVERS > M: James Schu
[PATCH] drm/i915: Remove unneeded double drm_rect_visible call in check_overlay_dst
check_overlay_dst for clipped is called 2 times: in drm_rect_intersect
and than directly. Change second call for check of drm_rect_intersect
result to save some time (in locked code section).
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Fixes: 8d8b2dd3995f ("drm/i915: Make the PIPESRC rect relative to the
entire bigjoiner area")
Signed-off-by: Nikita Kiryushin
---
drivers/gpu/drm/i915/display/intel_overlay.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c
b/drivers/gpu/drm/i915/display/intel_overlay.c
index 2b1392d5a902..1cda1c163a92 100644
--- a/drivers/gpu/drm/i915/display/intel_overlay.c
+++ b/drivers/gpu/drm/i915/display/intel_overlay.c
@@ -972,9 +972,8 @@ static int check_overlay_dst(struct intel_overlay
*overlay,
rec->dst_width, rec->dst_height);
clipped = req;
- drm_rect_intersect(&clipped, &crtc_state->pipe_src);
- if (!drm_rect_visible(&clipped) ||
+ if (!drm_rect_intersect(&clipped, &crtc_state->pipe_src) ||
!drm_rect_equals(&clipped, &req))
return -EINVAL;
-- 2.34.1
[PATCH v6 1/9] usb: misc: onboard_hub: use pointer consistently in the probe function
Commit 14485de431b0 ("usb: Use device_get_match_data()") overlooked the
already existing pointer to pdev->dev 'dev'.
Use the existing pointer 'dev' in device_get_match_data() to keep code
consistency.
Signed-off-by: Javier Carrasco
---
drivers/usb/misc/onboard_usb_hub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/misc/onboard_usb_hub.c
b/drivers/usb/misc/onboard_usb_hub.c
index 0dd2b032c90b..81c001fd38c1 100644
--- a/drivers/usb/misc/onboard_usb_hub.c
+++ b/drivers/usb/misc/onboard_usb_hub.c
@@ -260,7 +260,7 @@ static int onboard_hub_probe(struct platform_device *pdev)
if (!hub)
return -ENOMEM;
- hub->pdata = device_get_match_data(&pdev->dev);
+ hub->pdata = device_get_match_data(dev);
if (!hub->pdata)
return -EINVAL;
--
2.40.1
[PATCH v6 2/9] usb: misc: onboard_hub: use device supply names
The current implementation uses generic names for the power supplies,
which conflicts with proper name definitions in the device bindings.
Add a per-device property to include real supply names and keep generic
names for existing devices to keep backward compatibility.
Signed-off-by: Javier Carrasco
---
drivers/usb/misc/onboard_usb_hub.c | 49 --
drivers/usb/misc/onboard_usb_hub.h | 12 ++
2 files changed, 38 insertions(+), 23 deletions(-)
diff --git a/drivers/usb/misc/onboard_usb_hub.c
b/drivers/usb/misc/onboard_usb_hub.c
index 81c001fd38c1..1f3e0094957d 100644
--- a/drivers/usb/misc/onboard_usb_hub.c
+++ b/drivers/usb/misc/onboard_usb_hub.c
@@ -29,17 +29,6 @@
#include "onboard_usb_hub.h"
-/*
- * Use generic names, as the actual names might differ between hubs. If a new
- * hub requires more than the currently supported supplies, add a new one here.
- */
-static const char * const supply_names[] = {
- "vdd",
- "vdd2",
-};
-
-#define MAX_SUPPLIES ARRAY_SIZE(supply_names)
-
static void onboard_hub_attach_usb_driver(struct work_struct *work);
static struct usb_device_driver onboard_hub_usbdev_driver;
@@ -65,6 +54,29 @@ struct onboard_hub {
struct clk *clk;
};
+static int onboard_hub_get_regulators(struct onboard_hub *hub)
+{
+ const char * const *supply_names = hub->pdata->supply_names;
+ unsigned int num_supplies = hub->pdata->num_supplies;
+ struct device *dev = hub->dev;
+ unsigned int i;
+ int err;
+
+ if (num_supplies > MAX_SUPPLIES)
+ return dev_err_probe(dev, -EINVAL, "max %d supplies
supported!\n",
+MAX_SUPPLIES);
+
+ for (i = 0; i < num_supplies; i++)
+ hub->supplies[i].supply = supply_names[i];
+
+ err = devm_regulator_bulk_get(dev, num_supplies, hub->supplies);
+ if (err)
+ dev_err(dev, "Failed to get regulator supplies: %pe\n",
+ ERR_PTR(err));
+
+ return err;
+}
+
static int onboard_hub_power_on(struct onboard_hub *hub)
{
int err;
@@ -253,7 +265,6 @@ static int onboard_hub_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct onboard_hub *hub;
- unsigned int i;
int err;
hub = devm_kzalloc(dev, sizeof(*hub), GFP_KERNEL);
@@ -264,18 +275,11 @@ static int onboard_hub_probe(struct platform_device *pdev)
if (!hub->pdata)
return -EINVAL;
- if (hub->pdata->num_supplies > MAX_SUPPLIES)
- return dev_err_probe(dev, -EINVAL, "max %zu supplies
supported!\n",
-MAX_SUPPLIES);
-
- for (i = 0; i < hub->pdata->num_supplies; i++)
- hub->supplies[i].supply = supply_names[i];
+ hub->dev = dev;
- err = devm_regulator_bulk_get(dev, hub->pdata->num_supplies,
hub->supplies);
- if (err) {
- dev_err(dev, "Failed to get regulator supplies: %pe\n",
ERR_PTR(err));
+ err = onboard_hub_get_regulators(hub);
+ if (err)
return err;
- }
hub->clk = devm_clk_get_optional(dev, NULL);
if (IS_ERR(hub->clk))
@@ -286,7 +290,6 @@ static int onboard_hub_probe(struct platform_device *pdev)
if (IS_ERR(hub->reset_gpio))
return dev_err_probe(dev, PTR_ERR(hub->reset_gpio), "failed to
get reset GPIO\n");
- hub->dev = dev;
mutex_init(&hub->lock);
INIT_LIST_HEAD(&hub->udev_list);
diff --git a/drivers/usb/misc/onboard_usb_hub.h
b/drivers/usb/misc/onboard_usb_hub.h
index f360d5cf8d8a..0ee515e7feae 100644
--- a/drivers/usb/misc/onboard_usb_hub.h
+++ b/drivers/usb/misc/onboard_usb_hub.h
@@ -6,54 +6,66 @@
#ifndef _USB_MISC_ONBOARD_USB_HUB_H
#define _USB_MISC_ONBOARD_USB_HUB_H
+#define MAX_SUPPLIES 2
+
struct onboard_hub_pdata {
unsigned long reset_us; /* reset pulse width in us */
unsigned int num_supplies; /* number of supplies */
+ const char * const supply_names[MAX_SUPPLIES];
};
static const struct onboard_hub_pdata microchip_usb424_data = {
.reset_us = 1,
.num_supplies = 1,
+ .supply_names = { "vdd" },
};
static const struct onboard_hub_pdata microchip_usb5744_data = {
.reset_us = 0,
.num_supplies = 2,
+ .supply_names = { "vdd", "vdd2" },
};
static const struct onboard_hub_pdata realtek_rts5411_data = {
.reset_us = 0,
.num_supplies = 1,
+ .supply_names = { "vdd" },
};
static const struct onboard_hub_pdata ti_tusb8041_data = {
.reset_us = 3000,
.num_supplies = 1,
+ .supply_names = { "vdd" },
};
static const struct onboard_hub_pdata cypress_hx3_data = {
.reset_us = 1,
.num_supplies = 2,
+ .supply_names = { "vdd", "vdd2" },
};
static const struct onboard_hub_pdata cypress_hx2vl_data = {
.reset_us = 1,
.num_su
[PATCH v6 0/9] usb: misc: onboard_hub: add support for XMOS XVF3500
This series adds support for the XMOS XVF3500 VocalFusion Voice
Processor[1], a low-latency, 32-bit multicore controller for voice
processing.
The XVF3500 requires a specific power sequence, which consists of
enabling the regulators that control the 3V3 and 1V0 device supplies,
and a reset de-assertion after a delay of at least 100ns. Once in normal
operation, the XVF3500 registers itself as a regular USB device and no
device-specific management is required.
The power management provided by onboard_usb_hub is not specific for hubs
and any other USB device with the same power sequence could profit from
that driver, provided that the device does not have any specific
requirements beyond the power management. To account for non-hub devices,
the driver has been renamed and an extra flag has been added to identify
hubs and provide their specific functionality.
Support for device-specific power suply names has also been added, keeping
generic names for already supported devices to keep backwards
compatibility.
The references to onboard_usb_hub in the core and config files have been
updated as well.
The diff is way much bulkier than the actual code addition because of the
file renaming, so in order to ease reviews and catch hub-specific code
that might still affect non-hub devices, the complete renaming was moved
to a single commit.
This series has been tested with a Rockchip-based SoC and an XMOS
XVF3500-FB167-C.
[1] https://www.xmos.com/xvf3500/
To: Liam Girdwood
To: Mark Brown
To: Rob Herring
To: Krzysztof Kozlowski
To: Conor Dooley
To: Matthias Kaehlcke
To: Greg Kroah-Hartman
To: Helen Koike
To: Maarten Lankhorst
To: Maxime Ripard
To: Thomas Zimmermann
To: David Airlie
To: Daniel Vetter
To: Catalin Marinas
To: Will Deacon
To: Russell King
Cc: linux-so...@vger.kernel.org
Cc: devicet...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-arm-ker...@lists.infradead.org
Signed-off-by: Javier Carrasco
Changes in v6:
- onboard_usb_hub.c: use dev pointer in probe consistently (new patch).
- onboard_usb_hub.c: rename get_regulator_bulk function to
get_regulators and only pass onboard_hub (hub in probe) as argument.
- onboard_usb_hub.c: drop file after renaming.
- onboard_usb_dev.c: improve device descriptions in usb_device_id table.
- onboard_usb_dev.c: keep non-hub devices powered on in suspend.
- General: update commit messages (use usb_hub_dev after renaming).
- Link to v5:
https://lore.kernel.org/r/20240228-onboard_xvf3500-v5-0-76b805fd3...@wolfvision.net
Changes in v5:
- onboard_usb_dev: move device suppy names handling to [1/8].
- onboard_usb_dev.c: make always_powered_in_suspend not visible for
non-hub devices.
- onboard_usb_dev.c: move is_hub check in suspend() to functio entry.
- onboard_usb_dev_pdevs.c: comment rephrasing to account for
hub-specific attribute.
- Link to v4:
https://lore.kernel.org/r/20240220-onboard_xvf3500-v4-0-dc1617cc5...@wolfvision.net
Changes in v4:
- General: use device supply names and generics as fallback.
- onbord_usb_dev.c: fix suspend callback for non-hub devices.
- onboard_usb_dev.c: fix typos.
- Link to v3:
https://lore.kernel.org/r/20240206-onboard_xvf3500-v3-0-f85b04116...@wolfvision.net
Changes in v3:
- onboard_usb_hub: rename to onboard_usb_dev to include non-hub devices.
- onboard_hub_dev: add flag to identify hubs and provide their extra
functionality.
- dt-bindings: add reference to usb-device.yaml and usb node in the
example.
- dt-bindings: generic node name.
- Link to v2:
https://lore.kernel.org/r/20240130-onboard_xvf3500-v1-0-51b539840...@wolfvision.net
Changes in v2:
- general: add support in onboard_usb_hub instead of using a dedicated
driver.
- dt-bindings: use generic usb-device compatible ("usbVID,PID").
- Link to v1:
https://lore.kernel.org/all/20240115-feature-xvf3500_driver-v1-0-ed9cfb48b...@wolfvision.net/
---
Javier Carrasco (9):
usb: misc: onboard_hub: use pointer consistently in the probe function
usb: misc: onboard_hub: use device supply names
usb: misc: onboard_hub: rename to onboard_dev
drm: ci: arm64.config: update ONBOARD_USB_HUB to ONBOARD_USB_DEV
arm64: defconfig: update ONBOARD_USB_HUB to ONBOARD_USB_DEV
ARM: multi_v7_defconfig: update ONBOARD_USB_HUB to ONBOAD_USB_DEV
usb: misc: onboard_dev: add support for non-hub devices
ASoC: dt-bindings: xmos,xvf3500: add XMOS XVF3500 voice processor
usb: misc: onboard_dev: add support for XMOS XVF3500
...-usb-hub => sysfs-bus-platform-onboard-usb-dev} | 3 +-
.../devicetree/bindings/sound/xmos,xvf3500.yaml| 63 +++
MAINTAINERS| 4 +-
arch/arm/configs/multi_v7_defconfig| 2 +-
arch/arm64/configs/defconfig | 2 +-
drivers/gpu/drm/ci/arm64.config| 4 +-
drivers/usb/core/Makefile | 4 +-
drivers/usb/c
[PATCH v6 3/9] usb: misc: onboard_hub: rename to onboard_dev
This patch prepares onboad_hub to support non-hub devices by renaming
the driver files and their content, the headers and their references.
The comments and descriptions have been slightly modified to keep
coherence and account for the specific cases that only affect onboard
hubs (e.g. peer-hub).
The "hub" variables in functions where "dev" (and similar names) variables
already exist have been renamed to onboard_dev for clarity, which adds a
few lines in cases where more than 80 characters are used.
No new functionality has been added.
Signed-off-by: Javier Carrasco
---
...-usb-hub => sysfs-bus-platform-onboard-usb-dev} | 3 +-
MAINTAINERS| 4 +-
drivers/usb/core/Makefile | 4 +-
drivers/usb/core/hub.c | 8 +-
drivers/usb/core/hub.h | 2 +-
drivers/usb/misc/Kconfig | 16 +-
drivers/usb/misc/Makefile | 2 +-
drivers/usb/misc/onboard_usb_dev.c | 519 +
.../misc/{onboard_usb_hub.h => onboard_usb_dev.h} | 28 +-
...ard_usb_hub_pdevs.c => onboard_usb_dev_pdevs.c} | 47 +-
drivers/usb/misc/onboard_usb_hub.c | 504
include/linux/usb/onboard_dev.h| 18 +
include/linux/usb/onboard_hub.h| 18 -
13 files changed, 595 insertions(+), 578 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-bus-platform-onboard-usb-hub
b/Documentation/ABI/testing/sysfs-bus-platform-onboard-usb-dev
similarity index 74%
rename from Documentation/ABI/testing/sysfs-bus-platform-onboard-usb-hub
rename to Documentation/ABI/testing/sysfs-bus-platform-onboard-usb-dev
index 42deb0552065..b06a48c3c85a 100644
--- a/Documentation/ABI/testing/sysfs-bus-platform-onboard-usb-hub
+++ b/Documentation/ABI/testing/sysfs-bus-platform-onboard-usb-dev
@@ -5,4 +5,5 @@ Contact:Matthias Kaehlcke
linux-...@vger.kernel.org
Description:
(RW) Controls whether the USB hub remains always powered
- during system suspend or not.
\ No newline at end of file
+ during system suspend or not. This attribute is not
+ available for non-hub devices.
diff --git a/MAINTAINERS b/MAINTAINERS
index 8999497011a2..7ad556ecca40 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16297,8 +16297,8 @@ ONBOARD USB HUB DRIVER
M: Matthias Kaehlcke
L: linux-...@vger.kernel.org
S: Maintained
-F: Documentation/ABI/testing/sysfs-bus-platform-onboard-usb-hub
-F: drivers/usb/misc/onboard_usb_hub.c
+F: Documentation/ABI/testing/sysfs-bus-platform-onboard-usb-dev
+F: drivers/usb/misc/onboard_usb_dev.c
ONENAND FLASH DRIVER
M: Kyungmin Park
diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
index 7d338e9c0657..ac006abd13b3 100644
--- a/drivers/usb/core/Makefile
+++ b/drivers/usb/core/Makefile
@@ -12,8 +12,8 @@ usbcore-$(CONFIG_OF) += of.o
usbcore-$(CONFIG_USB_PCI) += hcd-pci.o
usbcore-$(CONFIG_ACPI) += usb-acpi.o
-ifdef CONFIG_USB_ONBOARD_HUB
-usbcore-y += ../misc/onboard_usb_hub_pdevs.o
+ifdef CONFIG_USB_ONBOARD_DEV
+usbcore-y += ../misc/onboard_usb_dev_pdevs.o
endif
obj-$(CONFIG_USB) += usbcore.o
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index ffd7c99e24a3..f85b3e928a35 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -23,7 +23,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
@@ -1776,7 +1776,7 @@ static void hub_disconnect(struct usb_interface *intf)
if (hub->quirk_disable_autosuspend)
usb_autopm_put_interface(intf);
- onboard_hub_destroy_pdevs(&hub->onboard_hub_devs);
+ onboard_dev_destroy_pdevs(&hub->onboard_devs);
kref_put(&hub->kref, hub_release);
}
@@ -1895,7 +1895,7 @@ static int hub_probe(struct usb_interface *intf, const
struct usb_device_id *id)
INIT_DELAYED_WORK(&hub->leds, led_work);
INIT_DELAYED_WORK(&hub->init_work, NULL);
INIT_WORK(&hub->events, hub_event);
- INIT_LIST_HEAD(&hub->onboard_hub_devs);
+ INIT_LIST_HEAD(&hub->onboard_devs);
spin_lock_init(&hub->irq_urb_lock);
timer_setup(&hub->irq_urb_retry, hub_retry_irq_urb, 0);
usb_get_intf(intf);
@@ -1925,7 +1925,7 @@ static int hub_probe(struct usb_interface *intf, const
struct usb_device_id *id)
}
if (hub_configure(hub, &desc->endpoint[0].desc) >= 0) {
- onboard_hub_create_pdevs(hdev, &hub->onboard_hub_devs);
+ onboard_dev_create_pdevs(hdev, &hub->onboard_devs);
return 0;
}
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 43ce21c96a51..3820703b11d8 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -74,7
[PATCH v6 6/9] ARM: multi_v7_defconfig: update ONBOARD_USB_HUB to ONBOAD_USB_DEV
The onboard_usb_hub driver has been updated to support non-hub devices, which has led to some renaming. Update to the new name accordingly. Update to the new name (ONBOARD_USB_DEV) accordingly. Signed-off-by: Javier Carrasco --- arch/arm/configs/multi_v7_defconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig index ecb3e286107a..6a6ebec173dc 100644 --- a/arch/arm/configs/multi_v7_defconfig +++ b/arch/arm/configs/multi_v7_defconfig @@ -876,7 +876,7 @@ CONFIG_USB_CHIPIDEA_UDC=y CONFIG_USB_CHIPIDEA_HOST=y CONFIG_USB_ISP1760=y CONFIG_USB_HSIC_USB3503=y -CONFIG_USB_ONBOARD_HUB=m +CONFIG_USB_ONBOARD_DEV=m CONFIG_AB8500_USB=y CONFIG_KEYSTONE_USB_PHY=m CONFIG_NOP_USB_XCEIV=y -- 2.40.1
[PATCH v6 8/9] ASoC: dt-bindings: xmos,xvf3500: add XMOS XVF3500 voice processor
The XMOS XVF3500 VocalFusion Voice Processor[1] is a low-latency, 32-bit
multicore controller for voice processing.
Add new bindings to define the device properties.
[1] https://www.xmos.com/xvf3500/
Reviewed-by: Krzysztof Kozlowski
Acked-by: Mark Brown
Signed-off-by: Javier Carrasco
---
.../devicetree/bindings/sound/xmos,xvf3500.yaml| 63 ++
1 file changed, 63 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/xmos,xvf3500.yaml
b/Documentation/devicetree/bindings/sound/xmos,xvf3500.yaml
new file mode 100644
index ..fb77a61f1350
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/xmos,xvf3500.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/xmos,xvf3500.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: XMOS XVF3500 VocalFusion Voice Processor
+
+maintainers:
+ - Javier Carrasco
+
+description:
+ The XMOS XVF3500 VocalFusion Voice Processor is a low-latency, 32-bit
+ multicore controller for voice processing.
+ https://www.xmos.com/xvf3500/
+
+allOf:
+ - $ref: /schemas/usb/usb-device.yaml#
+
+properties:
+ compatible:
+const: usb20b1,0013
+
+ reg: true
+
+ reset-gpios:
+maxItems: 1
+
+ vdd-supply:
+description:
+ Regulator for the 1V0 supply.
+
+ vddio-supply:
+description:
+ Regulator for the 3V3 supply.
+
+required:
+ - compatible
+ - reg
+ - reset-gpios
+ - vdd-supply
+ - vddio-supply
+
+additionalProperties: false
+
+examples:
+ - |
+#include
+
+usb {
+#address-cells = <1>;
+#size-cells = <0>;
+
+voice_processor: voice-processor@1 {
+compatible = "usb20b1,0013";
+reg = <1>;
+reset-gpios = <&gpio 5 GPIO_ACTIVE_LOW>;
+vdd-supply = <&vcc1v0>;
+vddio-supply = <&vcc3v3>;
+};
+};
+
+...
--
2.40.1
[PATCH v6 9/9] usb: misc: onboard_dev: add support for XMOS XVF3500
The XMOS XVF3500 VocalFusion Voice Processor[1] is a low-latency, 32-bit
multicore controller for voice processing.
This device requires a specific power sequence, which consists of
enabling the regulators that control the 3V3 and 1V0 device supplies,
and a reset de-assertion after a delay of at least 100ns.
Once in normal operation, the XVF3500 registers itself as a USB device,
and it does not require any device-specific operations in the driver.
[1] https://www.xmos.com/xvf3500/
Signed-off-by: Javier Carrasco
---
drivers/usb/misc/onboard_usb_dev.c | 2 ++
drivers/usb/misc/onboard_usb_dev.h | 8
2 files changed, 10 insertions(+)
diff --git a/drivers/usb/misc/onboard_usb_dev.c
b/drivers/usb/misc/onboard_usb_dev.c
index f1b174503c44..ee702448e446 100644
--- a/drivers/usb/misc/onboard_usb_dev.c
+++ b/drivers/usb/misc/onboard_usb_dev.c
@@ -407,6 +407,7 @@ static struct platform_driver onboard_dev_driver = {
#define VENDOR_ID_REALTEK 0x0bda
#define VENDOR_ID_TI 0x0451
#define VENDOR_ID_VIA 0x2109
+#define VENDOR_ID_XMOS 0x20B1
/*
* Returns the onboard_dev platform device that is associated with the USB
@@ -499,6 +500,7 @@ static const struct usb_device_id onboard_dev_id_table[] = {
{ USB_DEVICE(VENDOR_ID_TI, 0x8142) }, /* TI USB8041 2.0 HUB */
{ USB_DEVICE(VENDOR_ID_VIA, 0x0817) }, /* VIA VL817 3.1 HUB */
{ USB_DEVICE(VENDOR_ID_VIA, 0x2817) }, /* VIA VL817 2.0 HUB */
+ { USB_DEVICE(VENDOR_ID_XMOS, 0x0013) }, /* XMOS XVF3500 Voice Processor
*/
{}
};
MODULE_DEVICE_TABLE(usb, onboard_dev_id_table);
diff --git a/drivers/usb/misc/onboard_usb_dev.h
b/drivers/usb/misc/onboard_usb_dev.h
index 58cf8c81b2cf..51c4c074d158 100644
--- a/drivers/usb/misc/onboard_usb_dev.h
+++ b/drivers/usb/misc/onboard_usb_dev.h
@@ -78,6 +78,13 @@ static const struct onboard_dev_pdata vialab_vl817_data = {
.is_hub = true,
};
+static const struct onboard_dev_pdata xmos_xvf3500_data = {
+ .reset_us = 1,
+ .num_supplies = 2,
+ .supply_names = { "vdd", "vddio" },
+ .is_hub = false,
+};
+
static const struct of_device_id onboard_dev_match[] = {
{ .compatible = "usb424,2412", .data = µchip_usb424_data, },
{ .compatible = "usb424,2514", .data = µchip_usb424_data, },
@@ -99,6 +106,7 @@ static const struct of_device_id onboard_dev_match[] = {
{ .compatible = "usbbda,5414", .data = &realtek_rts5411_data, },
{ .compatible = "usb2109,817", .data = &vialab_vl817_data, },
{ .compatible = "usb2109,2817", .data = &vialab_vl817_data, },
+ { .compatible = "usb20b1,0013", .data = &xmos_xvf3500_data, },
{}
};
--
2.40.1
[PATCH v6 7/9] usb: misc: onboard_dev: add support for non-hub devices
Most of the functionality this driver provides can be used by non-hub
devices as well.
To account for the hub-specific code, add a flag to the device data
structure and check its value for hub-specific code.
The 'always_powered_in_supend' attribute is only available for hub
devices, keeping the driver's default behavior for non-hub devices (keep
on in suspend).
Signed-off-by: Javier Carrasco
---
drivers/usb/misc/onboard_usb_dev.c | 25 -
drivers/usb/misc/onboard_usb_dev.h | 10 ++
2 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/misc/onboard_usb_dev.c
b/drivers/usb/misc/onboard_usb_dev.c
index 4ae580445408..f1b174503c44 100644
--- a/drivers/usb/misc/onboard_usb_dev.c
+++ b/drivers/usb/misc/onboard_usb_dev.c
@@ -261,7 +261,27 @@ static struct attribute *onboard_dev_attrs[] = {
&dev_attr_always_powered_in_suspend.attr,
NULL,
};
-ATTRIBUTE_GROUPS(onboard_dev);
+
+static umode_t onboard_dev_attrs_are_visible(struct kobject *kobj,
+struct attribute *attr,
+int n)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct onboard_dev *onboard_dev = dev_get_drvdata(dev);
+
+ if (attr == &dev_attr_always_powered_in_suspend.attr &&
+ !onboard_dev->pdata->is_hub)
+ return 0;
+
+ return attr->mode;
+}
+
+static const struct attribute_group onboard_dev_group = {
+ .is_visible = onboard_dev_attrs_are_visible,
+ .attrs = onboard_dev_attrs,
+};
+__ATTRIBUTE_GROUPS(onboard_dev);
+
static void onboard_dev_attach_usb_driver(struct work_struct *work)
{
@@ -286,6 +306,9 @@ static int onboard_dev_probe(struct platform_device *pdev)
if (!onboard_dev->pdata)
return -EINVAL;
+ if (!onboard_dev->pdata->is_hub)
+ onboard_dev->always_powered_in_suspend = true;
+
onboard_dev->dev = dev;
err = onboard_dev_get_regulators(onboard_dev);
diff --git a/drivers/usb/misc/onboard_usb_dev.h
b/drivers/usb/misc/onboard_usb_dev.h
index 4da9f3b7f9e9..58cf8c81b2cf 100644
--- a/drivers/usb/misc/onboard_usb_dev.h
+++ b/drivers/usb/misc/onboard_usb_dev.h
@@ -12,60 +12,70 @@ struct onboard_dev_pdata {
unsigned long reset_us; /* reset pulse width in us */
unsigned int num_supplies; /* number of supplies */
const char * const supply_names[MAX_SUPPLIES];
+ bool is_hub;/* true if the device is a HUB */
};
static const struct onboard_dev_pdata microchip_usb424_data = {
.reset_us = 1,
.num_supplies = 1,
.supply_names = { "vdd" },
+ .is_hub = true,
};
static const struct onboard_dev_pdata microchip_usb5744_data = {
.reset_us = 0,
.num_supplies = 2,
.supply_names = { "vdd", "vdd2" },
+ .is_hub = true,
};
static const struct onboard_dev_pdata realtek_rts5411_data = {
.reset_us = 0,
.num_supplies = 1,
.supply_names = { "vdd" },
+ .is_hub = true,
};
static const struct onboard_dev_pdata ti_tusb8041_data = {
.reset_us = 3000,
.num_supplies = 1,
.supply_names = { "vdd" },
+ .is_hub = true,
};
static const struct onboard_dev_pdata cypress_hx3_data = {
.reset_us = 1,
.num_supplies = 2,
.supply_names = { "vdd", "vdd2" },
+ .is_hub = true,
};
static const struct onboard_dev_pdata cypress_hx2vl_data = {
.reset_us = 1,
.num_supplies = 1,
.supply_names = { "vdd" },
+ .is_hub = true,
};
static const struct onboard_dev_pdata genesys_gl850g_data = {
.reset_us = 3,
.num_supplies = 1,
.supply_names = { "vdd" },
+ .is_hub = true,
};
static const struct onboard_dev_pdata genesys_gl852g_data = {
.reset_us = 50,
.num_supplies = 1,
.supply_names = { "vdd" },
+ .is_hub = true,
};
static const struct onboard_dev_pdata vialab_vl817_data = {
.reset_us = 10,
.num_supplies = 1,
.supply_names = { "vdd" },
+ .is_hub = true,
};
static const struct of_device_id onboard_dev_match[] = {
--
2.40.1
[PATCH v6 4/9] drm: ci: arm64.config: update ONBOARD_USB_HUB to ONBOARD_USB_DEV
The onboard_usb_hub driver has been updated to support non-hub devices, which has led to some renaming. Update to the new name (ONBOARD_USB_DEV) accordingly. Acked-by: Helen Koike Signed-off-by: Javier Carrasco --- drivers/gpu/drm/ci/arm64.config | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/ci/arm64.config b/drivers/gpu/drm/ci/arm64.config index 8dbce9919a57..4140303d6260 100644 --- a/drivers/gpu/drm/ci/arm64.config +++ b/drivers/gpu/drm/ci/arm64.config @@ -87,7 +87,7 @@ CONFIG_DRM_PARADE_PS8640=y CONFIG_DRM_LONTIUM_LT9611UXC=y CONFIG_PHY_QCOM_USB_HS=y CONFIG_QCOM_GPI_DMA=y -CONFIG_USB_ONBOARD_HUB=y +CONFIG_USB_ONBOARD_DEV=y CONFIG_NVMEM_QCOM_QFPROM=y CONFIG_PHY_QCOM_USB_SNPS_FEMTO_V2=y @@ -97,7 +97,7 @@ CONFIG_USB_RTL8152=y # db820c ethernet CONFIG_ATL1C=y # Chromebooks ethernet -CONFIG_USB_ONBOARD_HUB=y +CONFIG_USB_ONBOARD_DEV=y # 888 HDK ethernet CONFIG_USB_LAN78XX=y -- 2.40.1
[PATCH v6 5/9] arm64: defconfig: update ONBOARD_USB_HUB to ONBOARD_USB_DEV
The onboard_usb_hub driver has been updated to support non-hub devices, which has led to some renaming. Update to the new name (ONBOARD_USB_DEV) accordingly. Signed-off-by: Javier Carrasco --- arch/arm64/configs/defconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index e6cf3e5d63c3..3c6196b6c984 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -1047,7 +1047,7 @@ CONFIG_USB_SERIAL_FTDI_SIO=m CONFIG_USB_SERIAL_OPTION=m CONFIG_USB_QCOM_EUD=m CONFIG_USB_HSIC_USB3503=y -CONFIG_USB_ONBOARD_HUB=m +CONFIG_USB_ONBOARD_DEV=m CONFIG_NOP_USB_XCEIV=y CONFIG_USB_GADGET=y CONFIG_USB_RENESAS_USBHS_UDC=m -- 2.40.1
Re: [PATCH 0/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing
On Thu, Feb 29, 2024 at 01:07:25AM +0200, Laurent Pinchart wrote: > > Chat Discussions > > > > > > For those interested in further discussions: > > > > **Join Our Slack Channel:** > > We have a Slack channel, #gitlab-ci, on the KernelCI Slack instance > > https://kernelci.slack.com/ . > > Feel free to join and contribute to the conversation. The KernelCI team has > > weekly calls where we also discuss the GitLab-CI pipeline. > > Could we communicate using free software please ? Furthermore, it's not > possible to create an account on that slack instance unless you have an > e-mail address affiliated with a small number of companies > (https://kernelci.slack.com/signup#/domain-signup). That's a big no-go > for me. Yeah, and that list looks super restrictive and arbitrary. Like, microsoft is there but kernel.org isn't? I'm sure there's a reason, but we should at the very least open it to everyone. Maxime signature.asc Description: PGP signature
Re: [PATCH 1/3] dt-bindings: display: mediatek: gamma: Change MT8195 to single enum group
Il 29/02/24 03:35, Jason-JH.Lin ha scritto:
Since MT8195 gamma has multiple bank for 12 bits LUT and it is
different from any other SoC LUT setting.
So we move MT8195 compatible from the one of items to the
single enum group.
Signed-off-by: Jason-JH.Lin
---
.../devicetree/bindings/display/mediatek/mediatek,gamma.yaml| 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
index c6641acd75d6..3e6cb8f48bcc 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
@@ -24,6 +24,7 @@ properties:
- enum:
- mediatek,mt8173-disp-gamma
- mediatek,mt8183-disp-gamma
+ - mediatek,mt8195-disp-gamma
- items:
- enum:
- mediatek,mt6795-disp-gamma
@@ -33,7 +34,6 @@ properties:
- mediatek,mt8186-disp-gamma
- mediatek,mt8188-disp-gamma
- mediatek,mt8192-disp-gamma
- - mediatek,mt8195-disp-gamma
While I agree on allowing mt8195-disp-gamma to have its own separated compatible
as the IP is actually different from the one in MT8183, you can't do it like
that,
or dtbs_check will fail validation for the mt8195 devicetree.
That one declares...
gamma0: gamma@1c006000 {
compatible = "mediatek,mt8195-disp-gamma", "mediatek,mt8183-disp-gamma";
...Please always run dtbs_check when performing bindings modifications.
Cheers,
Angelo
- const: mediatek,mt8183-disp-gamma
reg:
Re: [PATCH 2/3] dt-bindings: display: mediatek: gamma: Add support for MT8188
Il 29/02/24 03:35, Jason-JH.Lin ha scritto: The gamma LUT setting of MT8188 and MT8195 are the same, so we create a one of items for MT8188 to reuse the driver data settings of MT8195. Signed-off-by: Jason-JH.Lin Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH 3/3] drm/mediatek: Add gamma support for MT8195
Il 29/02/24 03:35, Jason-JH.Lin ha scritto: Since MT8195 compatible is in the single enum group, we have to add its compatible into mediatek-drm component binding table to ensure that it can be bound as a ddp_comp. Reviewed-by: AngeloGioacchino Del Regno
[PULL] drm-misc-next
Hi Dave, Sima, here's the additional PR for drm-misc-next. Best regards Thomas drm-misc-next-2024-02-29: drm-misc-next for v6.9: UAPI Changes: Cross-subsystem Changes: backlight: - corgi: include backlight header fbdev: - Cleanup includes in public header file - fbtft: Include backlight header Core Changes: edid: - Remove built-in EDID data dp: - Avoid AUX transfers on powered-down displays - Add VSC SDP helpers modesetting: - Add sanity checks for polling - Cleanups scheduler: - Cleanups tests: - Add helpers for mode-setting tests Driver Changes: i915: - Use shared VSC SDP helper mgag200: - Work around PCI write bursts mxsfb: - Use managed mode config nouveau: - Include backlight header where necessary qiac: - Cleanups sun4: - HDMI: updates to atomic mode setting tegra: - Fix GEM refounting in error paths tidss: - Fix multi display - Fix initial Z position v3d: - Support display MMU page size The following changes since commit f112b68f273fb0121cb64e0c3ac06adcb91e32b8: Merge v6.8-rc6 into drm-next (2024-02-26 11:41:07 +0100) are available in the Git repository at: https://anongit.freedesktop.org/git/drm/drm-misc tags/drm-misc-next-2024-02-29 for you to fetch changes up to 8df1ddb5bf11ab820ad991e164dab82c0960add9: drm/dp: Don't attempt AUX transfers when eDP panels are not powered (2024-02-28 12:43:36 -0800) drm-misc-next for v6.9: UAPI Changes: Cross-subsystem Changes: backlight: - corgi: include backlight header fbdev: - Cleanup includes in public header file - fbtft: Include backlight header Core Changes: edid: - Remove built-in EDID data dp: - Avoid AUX transfers on powered-down displays - Add VSC SDP helpers modesetting: - Add sanity checks for polling - Cleanups scheduler: - Cleanups tests: - Add helpers for mode-setting tests Driver Changes: i915: - Use shared VSC SDP helper mgag200: - Work around PCI write bursts mxsfb: - Use managed mode config nouveau: - Include backlight header where necessary qiac: - Cleanups sun4: - HDMI: updates to atomic mode setting tegra: - Fix GEM refounting in error paths tidss: - Fix multi display - Fix initial Z position v3d: - Support display MMU page size Abhinav Kumar (2): drm/dp: move intel_dp_vsc_sdp_pack() to generic helper drm/dp: drop the size parameter from drm_dp_vsc_sdp_pack() Douglas Anderson (1): drm/dp: Don't attempt AUX transfers when eDP panels are not powered Fedor Pchelkin (1): drm/tegra: put drm_gem_object ref on error in tegra_fb_create Jeff Johnson (1): accel/qaic: Constify aic100_channels Jocelyn Falempe (1): drm/mgag200: Add a workaround for low-latency Kunwu Chan (1): drm/scheduler: Simplify the allocation of slab caches in drm_sched_fence_slab_init Marek Vasut (2): drm/mxsfb: Switch to drmm_mode_config_init drm: lcdif: Switch to drmm_mode_config_init Maxime Ripard (10): drm/sun4i: hdmi: Convert encoder to atomic drm/sun4i: hdmi: Move mode_set into enable drm/sun4i: hdmi: Switch to container_of_const drm/sun4i: hdmi: Consolidate atomic_check and mode_valid drm/edid/firmware: Remove built-in EDIDs drm/tests: helpers: Include missing drm_drv header drm/tests: helpers: Add atomic helpers drm/tests: Add helper to create mock plane drm/tests: Add helper to create mock crtc drm/tests: connector: Add tests for drmm_connector_init Maíra Canal (1): drm/v3d: Enable V3D to use different PAGE_SIZE Paloma Arellano (1): drm/dp: add an API to indicate if sink supports VSC SDP Rodrigo Vivi (1): drm/i915: convert remaining intel_dp_vsc_sdp_pack Shradha Gupta (2): drm: Check output polling initialized before disabling drm: Check polling initialized before enabling in drm_helper_probe_single_connector_modes Thierry Reding (1): drm: Remove drm_num_crtcs() helper Thomas Zimmermann (10): Merge drm/drm-next into drm-misc-next backlight/corgi-lcd: Include drm/nouveau: Include staging/fbtft: Include fbdev: Do not include in header fbdev: Do not include in header fbdev: Do not include in header fbdev: Do not include in header fbdev: Clean up forward declarations in header file fbdev: Clean up include statements in header file Tomi Valkeinen (2): drm/tidss: Fix initial plane zpos values drm/tidss: Fix sync-lost issue with two displays Documentation/admin-guide/edid.rst | 35 +-- Documentation/admin-guide/kernel-parameters.txt | 14 +- drivers/accel/qaic/mhi_controller.c | 2 +- drivers/gpu/drm/display/drm_dp_helper.c | 132 +++ drivers/gpu/drm/drm_crtc.c | 15 +- drivers/gpu/drm/drm_edid_load.c | 162 ++ drivers/gpu/drm/drm_modeset_helper.c
Re: [PATCH v2 3/9] drm/vkms: write/update the documentation for pixel conversion and pixel write functions
On Tue, 27 Feb 2024 16:02:10 +0100
Louis Chauvet wrote:
> [...]
>
> > > diff --git a/drivers/gpu/drm/vkms/vkms_formats.c
> > > b/drivers/gpu/drm/vkms/vkms_formats.c
> > > index 172830a3936a..cb7a49b7c8e7 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_formats.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_formats.c
> > > @@ -9,6 +9,17 @@
> > >
> > > #include "vkms_formats.h"
> > >
> > > +/**
> > > + * packed_pixels_offset() - Get the offset of the block containing the
> > > pixel at coordinates x/y
> > > + * in the first plane
> > > + *
> > > + * @frame_info: Buffer metadata
> > > + * @x: The x coordinate of the wanted pixel in the buffer
> > > + * @y: The y coordinate of the wanted pixel in the buffer
> > > + *
> > > + * The caller must be aware that this offset is not always a pointer to
> > > a pixel. If individual
> > > + * pixel values are needed, they have to be extracted from the resulting
> > > block.
> >
> > Just wondering how the caller will be able to extract the right pixel
> > from the block without re-using the knowledge already used in this
> > function. I'd also expect the function to round down x,y to be
> > divisible by block dimensions, but that's not visible in this email.
> > Then the caller needs the remainder from the round-down, too?
>
> You are right, the current implementation is only working when block_h ==
> block_w == 1. I think I wrote the documentation for PATCHv2 5/9, but when
> backporting this comment for PATCHv2 3/9 I forgot to update it.
> The new comment will be:
>
> * pixels_offset() - Get the offset of a given pixel data at coordinate
> * x/y in the first plane
>[...]
> * The caller must ensure that the framebuffer associated with this
> * request uses a pixel format where block_h == block_w == 1.
> * If this requirement is not fulfilled, the resulting offset can be
> * completly wrong.
Hi Louis,
if there is no plan for how non-1x1 blocks would work yet, then I think
the above wording is fine. In my mind, the below wording would
encourage callers to seek out and try arbitrary tricks to make things
work for non-1x1 without rewriting the function to actually work.
I believe something would need to change in the function signature to
make it properly usable for non-1x1 blocks, but I too cannot suggest
anything off-hand.
>
> And yes, even after PATCHv2 5/9 it is not clear what is the offset. Is
> this better to replace the last sentence? (I will do the same update for
> the last sentence of packed_pixels_addr)
>
>[...]
> * The returned offset correspond to the offset of the block containing the
> pixel at coordinates
> * x/y.
> * The caller must use this offset with care, as for formats with block_h !=
> 1 or block_w != 1
> * the requested pixel value may have to be extracted from the block, even if
> they are
> * individually adressable.
>
> > > + */
> > > static size_t pixel_offset(const struct vkms_frame_info *frame_info, int
> > > x, int y)
> > > {
> > > struct drm_framebuffer *fb = frame_info->fb;
> > > @@ -17,12 +28,13 @@ static size_t pixel_offset(const struct
> > > vkms_frame_info *frame_info, int x, int
> > > + (x * fb->format->cpp[0]);
> > > }
> > >
>
> [...]
>
> > > +/**
> > > + * Retrieve the correct read_pixel function for a specific format.
> > > + * The returned pointer is NULL for unsupported pixel formats. The
> > > caller must ensure that the
> > > + * pointer is valid before using it in a vkms_plane_state.
> > > + *
> > > + * @format: 4cc of the format
> >
> > Since there are many different 4cc style pixel format definition tables
> > in existence with conflicting definitions, it would not hurt to be more
> > specific that this is about DRM_FORMAT_* or drm_fourcc.h.
>
> Is this better?
>
>@format: DRM_FORMAT_* value for which to obtain a conversion function (see
> [drm_fourcc.h])
Much better!
Thanks,
pq
> > > + */
> > > void *get_pixel_conversion_function(u32 format)
> > > {
> > > switch (format) {
> > > @@ -247,6 +280,13 @@ void *get_pixel_conversion_function(u32 format)
> > > }
> > > }
> > >
> > > +/**
> > > + * Retrieve the correct write_pixel function for a specific format.
> > > + * The returned pointer is NULL for unsupported pixel formats. The
> > > caller must ensure that the
> > > + * pointer is valid before using it in a vkms_writeback_job.
> > > + *
> > > + * @format: 4cc of the format
> >
> > This too.
>
> Ack, I will use the same as above
>
> > > + */
> > > void *get_pixel_write_function(u32 format)
> > > {
> > > switch (format) {
> > >
> >
> > I couldn't check if the docs are correct since the patch context is not
> > wide enough, but they all sound plausible to me.
>
> I checked again, I don't see other errors than your first comment.
>
> >
> > Thanks,
> > pq
>
> Kind regards,
> Louis Chauvet
>
> --
> Louis Chauvet, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
pgptwfT2EOPTS.p
Re: (subset) [PATCH 0/3] drm/panel: Pixel 3a Panel
Hi, On Thu, 08 Feb 2024 19:16:41 -0500, Richard Acayan wrote: > This adds support for the AMS559NK06 panel with the S6E3FA7 display > controller and enables the display subsystem on the Pixel 3a. > > Richard Acayan (3): > dt-bindings: display: panel-simple-dsi: add s6e3fa7 ams559nk06 compat > drm/panel: add samsung s6e3fa7 panel driver > arm64: dts: qcom: sdm670-google-sargo: add panel > > [...] Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next) [1/3] dt-bindings: display: panel-simple-dsi: add s6e3fa7 ams559nk06 compat https://cgit.freedesktop.org/drm/drm-misc/commit/?id=2689b33b88641a3b9a8cc411a0c8094cbed7e871 [2/3] drm/panel: add samsung s6e3fa7 panel driver https://cgit.freedesktop.org/drm/drm-misc/commit/?id=bf0390e2c95bf630b22dddc7cde5f83762b658e5 -- Neil
[PULL] drm-misc-next-fixes
Hi Dave, Sima, here's the release cycle's first PR for drm-misc-next-fixes. Best regards Thomas drm-misc-next-fixes-2024-02-29: Short summary of fixes pull: i915: - Fix NULL-pointer deref imx: - dcss: Fix resource-size calculation firmware: - sysfb: Fix returned error code The following changes since commit f112b68f273fb0121cb64e0c3ac06adcb91e32b8: Merge v6.8-rc6 into drm-next (2024-02-26 11:41:07 +0100) are available in the Git repository at: https://anongit.freedesktop.org/git/drm/drm-misc tags/drm-misc-next-fixes-2024-02-29 for you to fetch changes up to 9cb3542aeeac31b3dd6b5a7d58b9b7d6fe9fd2bc: drm/imx/dcss: fix resource size calculation (2024-02-28 09:16:59 +) Short summary of fixes pull: i915: - Fix NULL-pointer deref imx: - dcss: Fix resource-size calculation firmware: - sysfb: Fix returned error code Dan Carpenter (2): firmware/sysfb: fix an error code in sysfb_init() drm/imx/dcss: fix resource size calculation Thomas Zimmermann (1): Merge drm/drm-next into drm-misc-next-fixes Tvrtko Ursulin (1): drm/i915: Fix possible null pointer dereference after drm_dbg_printer conversion drivers/firmware/sysfb.c | 4 +++- drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c | 4 ++-- drivers/gpu/drm/imx/dcss/dcss-dev.c | 4 +--- 3 files changed, 6 insertions(+), 6 deletions(-) -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)
Re: [PATCH 1/3] dt-bindings: Add Crystal Clear Technology vendor prefix
Hi Jérémie, On 23/02/2024 19:22, Conor Dooley wrote: On Fri, Feb 23, 2024 at 02:45:15PM +0100, Jérémie Dautheribes wrote: Update Documentation/devicetree/bindings/vendor-prefixes.yaml to include "cct" as a vendor prefix for "Crystal Clear Technology". CCT is the vendor of the CMT430B19N00 TFT-LCD panel. Acked-by: Conor Dooley And add a Link: http://www.cct.com.my/ as that actually explains why "cct" is the right choice. Can you send a v2 with this change ? Thanks, Neil Cheers, Conor. Signed-off-by: Jérémie Dautheribes --- Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml index fef2e12b504e..96e47742e250 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml @@ -248,6 +248,8 @@ patternProperties: description: Catalyst Semiconductor, Inc. "^cavium,.*": description: Cavium, Inc. + "^cct,.*": +description: Crystal Clear Technology Sdn. Bhd. "^cdns,.*": description: Cadence Design Systems Inc. "^cdtech,.*": -- 2.34.1
Re: [drm-drm-misc:drm-misc-next v2] dt-bindings: nt35510: document 'port' property
Hi On 2/25/24 10:01, Dario Binacchi wrote: Hi, On Wed, Feb 14, 2024 at 10:47 AM Alexandre TORGUE wrote: Hi Heiko On 1/31/24 16:53, Conor Dooley wrote: On Wed, Jan 31, 2024 at 10:28:44AM +0100, Dario Binacchi wrote: Allow 'port' property (coming from panel-common.yaml) to be used in DTS: st/stm32f769-disco-mb1166-reva09.dtb: panel@0: 'port' does not match any of the regexes: 'pinctrl-[0-9]+' Signed-off-by: Dario Binacchi Cc: Alexandre Torgue Acked-by: Conor Dooley --- Changes in v2: - Rework the patch to drop errors found by command 'make DT_CHECKER_FLAGS=-m dt_binding_check'. .../devicetree/bindings/display/panel/novatek,nt35510.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/panel/novatek,nt35510.yaml b/Documentation/devicetree/bindings/display/panel/novatek,nt35510.yaml index a4afaff483b7..91921f4b0e5f 100644 --- a/Documentation/devicetree/bindings/display/panel/novatek,nt35510.yaml +++ b/Documentation/devicetree/bindings/display/panel/novatek,nt35510.yaml @@ -31,6 +31,7 @@ properties: vddi-supply: description: regulator that supplies the vddi voltage backlight: true + port: true required: - compatible -- 2.43.0 Do you plan to take this patch in drm-misc next branch ? As I have a dependency with it to merge a DT patch I can take in my tree (stm32-next) if you prefer. Let me know. Cheers Alex It's been some weeks, so a gentle ping seems in order :) Applied on stm32-next. Thanks Alex Thanks and regards, Dario
Re: [PATCH 1/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing
Hi Helen, Thanks for working on this On Wed, Feb 28, 2024 at 07:55:25PM -0300, Helen Koike wrote: > This patch introduces a `.gitlab-ci` file along with a `ci/` folder, > defininga basic test pipeline triggered by code pushes to a GitLab-CI > instance. This initial version includes static checks (checkpatch and > smatch for now) and build tests across various architectures and > configurations. It leverages an integrated cache for efficient build > times and introduces a flexible 'scenarios' mechanism for > subsystem-specific extensions. > > [ci: add prerequisites to run check-patch on MRs] > Co-developed-by: Tales Aparecida > Signed-off-by: Tales Aparecida > Signed-off-by: Helen Koike > > --- > > Hey all, > > You can check the validation of this patchset on: > https://gitlab.collabora.com/koike/linux/-/pipelines/87035 > > I would appreciate your feedback on this work, what do you think? > > If you would rate from 0 to 5, where: > > [ ] 0. I don't think this is useful at all, and I doubt it will ever be. It > doesn't seem worthwhile. > [ ] 1. I don't find it useful in its current form. > [ ] 2. It might be useful to others, but not for me. > [ ] 3. It has potential, but it's not yet something I can incorporate into my > workflow. > [ ] 4. This is useful, but it needs some adjustments before I can include it > in my workflow. > [ ] 5. This is really useful! I'm eager to start using it right away. Why > didn't you send this earlier? :) > > Which rating would you select? 4.5 :) One thing I'm wondering here is how we're going to cope with the different requirements each user / framework has. Like, Linus probably want to have a different set of CI before merging a PR than (say) linux-next does, or stable, or before doing an actual release. Similarly, DRM probably has a different set of requirements than drm-misc, drm-amd or nouveau. I don't see how the current architecture could accomodate for that. I know that Gitlab allows to store issues template in a separate repo, maybe we could ask them to provide a feature where the actions would be separate from the main repo? That way, any gitlab project could provide its own set of tests, without conflicting with each others (and we could still share them if we wanted to) I know some of use had good relationship with Gitlab, so maybe it would be worth asking? Maxime signature.asc Description: PGP signature
Re: [PATCH] drm: xlnx: dp: Reset DisplayPort IP
Tomi, could you push this through drm-misc ?
On Wed, Feb 28, 2024 at 06:22:25PM +0200, Laurent Pinchart wrote:
> Hello Rohit,
>
> Thank you for the patch.
>
> On Fri, Feb 16, 2024 at 04:40:43AM -0800, Rohit Visavalia wrote:
> > Assert DisplayPort reset signal before deasserting,
> > it is to clear out any registers programmed before booting kernel.
> >
> > Signed-off-by: Rohit Visavalia
> > ---
> > drivers/gpu/drm/xlnx/zynqmp_dp.c | 4
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> > b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> > index 1846c4971fd8..5a40aa1d4283 100644
> > --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> > +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> > @@ -1714,6 +1714,10 @@ int zynqmp_dp_probe(struct zynqmp_dpsub *dpsub)
> > goto err_free;
> > }
> >
> > + ret = zynqmp_dp_reset(dp, true);
> > + if (ret < 0)
> > + return ret;
> > +
>
> This looks fine to me, so
>
> Reviewed-by: Laurent Pinchart
>
> However, looking at zynqmp_dp_reset(), we have
>
> static int zynqmp_dp_reset(struct zynqmp_dp *dp, bool assert)
> {
> unsigned long timeout;
>
> if (assert)
> reset_control_assert(dp->reset);
> else
> reset_control_deassert(dp->reset);
>
> /* Wait for the (de)assert to complete. */
> timeout = jiffies + msecs_to_jiffies(RST_TIMEOUT_MS);
> while (!time_after_eq(jiffies, timeout)) {
> bool status = !!reset_control_status(dp->reset);
>
> if (assert == status)
> return 0;
>
> cpu_relax();
> }
>
> dev_err(dp->dev, "reset %s timeout\n", assert ? "assert" : "deassert");
> return -ETIMEDOUT;
> }
>
> That doesn't seem quite right. Aren't reset_control_assert() and
> reset_control_deassert() supposed to be synchronous ? If so there should
> be no need to wait, and if there's a need to wait, it could be a bug in
> the reset controller driver. This should be fixed, and then the code in
> probe could be replaced with a call to reset_control_reset().
>
> > ret = zynqmp_dp_reset(dp, false);
> > if (ret < 0)
> > goto err_free;
--
Regards,
Laurent Pinchart
Re: [PATCH v2 4/9] drm/vkms: Add typedef and documentation for pixel_read and pixel_write functions
On Tue, 27 Feb 2024 16:02:13 +0100
Louis Chauvet wrote:
> Le 26/02/24 - 13:36, Pekka Paalanen a écrit :
> > On Fri, 23 Feb 2024 12:37:24 +0100
> > Louis Chauvet wrote:
> >
> > > Introduce two typedefs: pixel_read_t and pixel_write_t. It allows the
> > > compiler to check if the passed functions take the correct arguments.
> > > Such typedefs will help ensuring consistency across the code base in
> > > case of update of these prototypes.
> > >
> > > Introduce a check around the get_pixel_*_functions to avoid using a
> > > nullptr as a function.
> > >
> > > Document for those typedefs.
> > >
> > > Signed-off-by: Louis Chauvet
> > > ---
> > > drivers/gpu/drm/vkms/vkms_drv.h | 23 +--
> > > drivers/gpu/drm/vkms/vkms_formats.c | 8
> > > drivers/gpu/drm/vkms/vkms_formats.h | 4 ++--
> > > drivers/gpu/drm/vkms/vkms_plane.c | 9 -
> > > drivers/gpu/drm/vkms/vkms_writeback.c | 9 -
> > > 5 files changed, 43 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h
> > > b/drivers/gpu/drm/vkms/vkms_drv.h
> > > index 18086423a3a7..886c885c8cf5 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> > > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> > > @@ -53,12 +53,31 @@ struct line_buffer {
> > > struct pixel_argb_u16 *pixels;
> > > };
> > >
> > > +/**
> > > + * typedef pixel_write_t - These functions are used to read a pixel from
> > > a
> > > + * `struct pixel_argb_u16*`, convert it in a specific format and write
> > > it in the @dst_pixels
> > > + * buffer.
> > > + *
> > > + * @dst_pixel: destination address to write the pixel
> > > + * @in_pixel: pixel to write
> > > + */
> > > +typedef void (*pixel_write_t)(u8 *dst_pixels, struct pixel_argb_u16
> > > *in_pixel);
> >
> > There are some inconsistencies in pixel_write_t and pixel_read_t. At
> > this point of the series they still operate on a single pixel, but you
> > use dst_pixels and src_pixels, plural. Yet the documentation correctly
> > talks about processing a single pixel.
>
> I will fix this for v4.
>
> > I would also expect the source to be always const, but that's a whole
> > another patch to change.
>
> The v4 will contains a new patch "drm/vkms: Use const pointer for
> pixel_read and pixel_write functions"
Sounds good!
>
> [...]
>
> > > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c
> > > b/drivers/gpu/drm/vkms/vkms_plane.c
> > > index d5203f531d96..f68b1b03d632 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_plane.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> > > @@ -106,6 +106,13 @@ static void vkms_plane_atomic_update(struct
> > > drm_plane *plane,
> > > return;
> > >
> > > fmt = fb->format->format;
> > > + pixel_read_t pixel_read = get_pixel_read_function(fmt);
> > > +
> > > + if (!pixel_read) {
> > > + DRM_WARN("Pixel format is not supported by VKMS planes. State
> > > is inchanged\n");
> >
> > DRM_WARN() is the kernel equivalent to userspace assert(), right?
>
> For the DRM_WARN it is just a standard prinkt(KERN_WARN, ...) (hidden
> behind drm internal macros).
My concern here is that does hitting this cause additional breakage of
the UAPI contract? For example, the UAPI contract expects that the old
FB is unreffed and the new FB is reffed by the plane in question. If
this early return causes that FB swap to be skipped, it could cause
secondary unexpected failures or misbehaviour for userspace later. That
could mislead debugging to think that there is a userspace bug.
Even if you cannot actually read FB due to an internal bug, it would be
good to still apply all the state changes that the UAPI contract
mandates.
Unless, this is a kernel bug kind of thing which explodes very
verbosely, but DRM_WARN is not that.
> > In that failing the check means an internal invariant was violated,
> > which means a code bug in kernel?
> >
> > Maybe this could be more specific about what invariant was violated?
> > E.g. atomic check should have rejected this attempt already.
>
> I'm not an expert (yet) in DRM, so please correct me:
> When atomic_update is called, the new state is always validated by
> atomic_check before? There is no way to pass something not validated by
> atomic_check to atomic_update? If this is the case, yes, it should be an
> ERROR and not a WARN as an invalid format passed the atomic_check
> verification.
I only know about the UAPI, I'm not familiar with kernel internals.
We see that atomic_update returns void, so I think it simply cannot
return errors. To my understanding, atomic_check needs to ensure that
atomic_update cannot fail. There is even UAPI to exercise atomic_check
alone: the atomic commit TEST_ONLY flag. Userspace trusts that flag, and
will not expect an identical atomic commit to fail without TEST_ONLY
when it succeeded with TEST_ONLY.
> If so, is this better?
>
> if (!pixel_read) {
> /*
>* This is a bug as the vkms_plane_ato
Re: [PATCH v5 11/14] drm/panthor: Add the driver frontend block
On Sun, 18 Feb 2024 22:41:25 +0100
Boris Brezillon wrote:
> +static int panthor_ioctl_bo_create(struct drm_device *ddev, void *data,
> +struct drm_file *file)
> +{
> + struct panthor_file *pfile = file->driver_priv;
> + struct drm_panthor_bo_create *args = data;
> + struct panthor_vm *vm = NULL;
> + int cookie, ret;
> +
> + if (!drm_dev_enter(ddev, &cookie))
> + return -ENODEV;
> +
> + if (!args->size || args->pad ||
> + (args->flags & ~PANTHOR_BO_FLAGS)) {
> + ret = -EINVAL;
> + goto out_dev_exit;
> + }
> +
> + if (args->exclusive_vm_id) {
> + vm = panthor_vm_pool_get_vm(pfile->vms, args->exclusive_vm_id);
> + if (!vm) {
> + ret = -EINVAL;
> + goto out_dev_exit;
> + }
> + }
> +
> + ret = panthor_gem_create_with_handle(file, ddev, vm, args->size,
> + args->flags, &args->handle);
Despite what's stated in the uAPI doc, we never update args->size to
make it page-aligned. We need to change
panthor_gem_create_with_handle()'s prototype to take the size as an 'u64
*' so we can reflect the page-size alignment done by the BO allocation
logic.
Will send a v6 with this fix and the other 2 fixes for the bugs I
reported previously.
Re: [PATCH 2/3] dt-bindings: display: mediatek: gamma: Add support for MT8188
Re: [PATCH] drm/bridge: Select DRM_KMS_HELPER for DRM_PANEL_BRIDGE
On Wed Jan 17, 2024 at 9:59 AM CET, Luca Weiss wrote:
> On Mon Jan 15, 2024 at 9:43 AM CET, Neil Armstrong wrote:
> > Hi Luca,
> >
> > On 11/01/2024 13:38, Luca Weiss wrote:
> > > Since the kconfig symbol of DRM_PANEL_BRIDGE is only adding
> > > bridge/panel.o to drm_kms_helper object, we need to select
> > > DRM_KMS_HELPER to make sure the file is actually getting built.
> > >
> > > Otherwise with certain defconfigs e.g. devm_drm_of_get_bridge will not
> > > be properly available:
> > >
> > >aarch64-linux-gnu-ld: drivers/phy/qualcomm/phy-qcom-qmp-combo.o: in
> > > function `qmp_combo_bridge_attach':
> > >drivers/phy/qualcomm/phy-qcom-qmp-combo.c:3204:(.text+0x8f4):
> > > undefined reference to `devm_drm_of_get_bridge'
> > >
> > > Signed-off-by: Luca Weiss
> > > ---
> > > I can see "depends on DRM_KMS_HELPER" was removed with commit
> > > 3c3384050d68 ("drm: Don't make DRM_PANEL_BRIDGE dependent on
> > > DRM_KMS_HELPERS")
> > >
> > > I'm not too familiar with Kconfig but it feels more correct if
> > > PHY_QCOM_QMP_COMBO selects DRM_PANEL_BRIDGE that that's enough; and it
> > > doesn't also has to explicitly select DRM_KMS_HELPER because of how the
> > > objects are built in the Makefile.
> > >
> > > Alternatively solution to this patch could be adjusting this line in
> > > include/drm/drm_bridge.h:
> > >
> > >-#if defined(CONFIG_OF) && defined(CONFIG_DRM_PANEL_BRIDGE)
> > >+#if defined(CONFIG_OF) && defined(CONFIG_DRM_PANEL_BRIDGE) &&
> > > defined(CONFIG_DRM_KMS_HELPER)
> > > struct drm_bridge *devm_drm_of_get_bridge(struct device *dev, struct
> > > device_node *node,
> > > u32 port, u32 endpoint);
> > >
> > > .. and then selecting DRM_KMS_HELPER for PHY_QCOM_QMP_COMBO.
> > >
> > > But I think the solution in this patch is better. Let me know what you
> > > think.
> >
> > I think this is no more the case after on linux-next:
> > 35921910bbd0 phy: qcom: qmp-combo: switch to DRM_AUX_BRIDGE
> >
> > But could you still check ?
>
> On next-20240117 the error happens in the aux-bridge file instead then.
>
> aarch64-linux-gnu-ld: drivers/gpu/drm/bridge/aux-bridge.o: in function
> `drm_aux_bridge_probe':
> drivers/gpu/drm/bridge/aux-bridge.c:115:(.text+0xe0): undefined reference to
> `devm_drm_of_get_bridge'
>
> I'm attaching the defconfig with which I can reproduce this but it's
> really just DRM_KMS_HELPER=n and PHY_QCOM_QMP_COMBO=y I believe.
Hi Neil,
Ping on this patch
Regards
Luca
>
> Regards
> Luca
>
>
> >
> > Neil
> >
> > > ---
> > > drivers/gpu/drm/bridge/Kconfig | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/Kconfig
> > > b/drivers/gpu/drm/bridge/Kconfig
> > > index ac9ec5073619..ae782b427829 100644
> > > --- a/drivers/gpu/drm/bridge/Kconfig
> > > +++ b/drivers/gpu/drm/bridge/Kconfig
> > > @@ -8,6 +8,7 @@ config DRM_BRIDGE
> > > config DRM_PANEL_BRIDGE
> > > def_bool y
> > > depends on DRM_BRIDGE
> > > + select DRM_KMS_HELPER
> > > select DRM_PANEL
> > > help
> > > DRM bridge wrapper of DRM panels
> > >
> > > ---
> > > base-commit: b9c3a1fa6fb324e691a03cf124b79f4842e65d76
> > > change-id: 20240111-drm-panel-bridge-fixup-5c2977fb969f
> > >
> > > Best regards,
Re: [PATCH 0/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing
On Thu, Feb 29, 2024 at 11:26:51AM +0200, Nikolai Kondrashov wrote: > On 2/29/24 01:07, Laurent Pinchart wrote: > > On Wed, Feb 28, 2024 at 07:55:24PM -0300, Helen Koike wrote: > >> **Join Our Slack Channel:** > >> We have a Slack channel, #gitlab-ci, on the KernelCI Slack instance > >> https://kernelci.slack.com/ . > >> Feel free to join and contribute to the conversation. The KernelCI team has > >> weekly calls where we also discuss the GitLab-CI pipeline. > > > > Could we communicate using free software please ? Furthermore, it's not > > possible to create an account on that slack instance unless you have an > > e-mail address affiliated with a small number of companies > > (https://kernelci.slack.com/signup#/domain-signup). That's a big no-go > > for me. > > Yes, it's not ideal that we use closed-source software for communication, but > FWIW I'd be happy to invite you there. Perhaps if you try logging in e.g. > with > a Google account, I'd be able to see and approve your attempt too. I don't use Google accounts to authenticate to third-party services, sorry. And in any case, that won't make slack free software. > Otherwise, our video meetings are generally open to everyone and run in Jitsi. -- Regards, Laurent Pinchart
Re: Making drm_gpuvm work across gpu devices
Am 28.02.24 um 20:51 schrieb Zeng, Oak: The mail wasn’t indent/preface correctly. Manually format it. *From:*Christian König *Sent:* Tuesday, February 27, 2024 1:54 AM *To:* Zeng, Oak ; Danilo Krummrich ; Dave Airlie ; Daniel Vetter ; Felix Kuehling ; jgli...@redhat.com *Cc:* Welty, Brian ; dri-devel@lists.freedesktop.org; intel...@lists.freedesktop.org; Bommu, Krishnaiah ; Ghimiray, Himal Prasad ; thomas.hellst...@linux.intel.com; Vishwanathapura, Niranjana ; Brost, Matthew ; Gupta, saurabhg *Subject:* Re: Making drm_gpuvm work across gpu devices Hi Oak, Am 23.02.24 um 21:12 schrieb Zeng, Oak: Hi Christian, I go back this old email to ask a question. sorry totally missed that one. Quote from your email: “Those ranges can then be used to implement the SVM feature required for higher level APIs and not something you need at the UAPI or even inside the low level kernel memory management.” “SVM is a high level concept of OpenCL, Cuda, ROCm etc.. This should not have any influence on the design of the kernel UAPI.” There are two category of SVM: 1.driver svm allocator: this is implemented in user space, i.g., cudaMallocManaged (cuda) or zeMemAllocShared (L0) or clSVMAlloc(openCL). Intel already have gem_create/vm_bind in xekmd and our umd implemented clSVMAlloc and zeMemAllocShared on top of gem_create/vm_bind. Range A..B of the process address space is mapped into a range C..D of the GPU address space, exactly as you said. 2.system svm allocator: This doesn’t introduce extra driver API for memory allocation. Any valid CPU virtual address can be used directly transparently in a GPU program without any extra driver API call. Quote from kernel Documentation/vm/hmm.hst: “Any application memory region (private anonymous, shared memory, or regular file backed memory) can be used by a device transparently” and “to share the address space by duplicating the CPU page table in the device page table so the same address points to the same physical memory for any valid main memory address in the process address space”. In system svm allocator, we don’t need that A..B C..D mapping. It looks like you were talking of 1). Were you? No, even when you fully mirror the whole address space from a process into the GPU you still need to enable this somehow with an IOCTL. And while enabling this you absolutely should specify to which part of the address space this mirroring applies and where it maps to. */[Zeng, Oak] /* Lets say we have a hardware platform where both CPU and GPU support 57bit(use it for example. The statement apply to any address range) virtual address range, how do you decide “which part of the address space this mirroring applies”? You have to mirror the whole address space [0~2^57-1], do you? As you designed it, the gigantic vm_bind/mirroring happens at the process initialization time, and at that time, you don’t know which part of the address space will be used for gpu program. Remember for system allocator, *any* valid CPU address can be used for GPU program. If you add an offset to [0~2^57-1], you get an address out of 57bit address range. Is this a valid concern? Well you can perfectly mirror on demand. You just need something similar to userfaultfd() for the GPU. This way you don't need to mirror the full address space, but can rather work with large chunks created on demand, let's say 1GiB or something like that. The virtual address space is basically just a hardware functionality to route memory accesses. While the mirroring approach is a very common use case for data-centers and high performance computing there are quite a number of different use cases which makes use of virtual address space in a non "standard" fashion. The native context approach for VMs is just one example, databases and emulators are another one. I see the system svm allocator as just a special case of the driver allocator where not fully backed buffer objects are allocated, but rather sparse one which are filled and migrated on demand. */[Zeng, Oak] /* Above statement is true to me. We don’t have BO for system svm allocator. It is a sparse one as we can sparsely map vma to GPU. Our migration policy decide which pages/how much of the vma is migrated/mapped to GPU page table. *//* The difference b/t your mind and mine is, you want a gigantic vma (created during the gigantic vm_bind) to be sparsely populated to gpu. While I thought vma (xe_vma in xekmd codes) is a place to save memory attributes (such as caching, user preferred placement etc). All those memory attributes are range based, i.e., user can specify range1 is cached while range2 is uncached. So I don’t see how you can manage it with the gigantic vma. Do you split your gigantic vma later to save range based memory attributes? Yes, exactly that. I mean the splitting and eventua
Re: [PATCH 1/1] drm/bridge: ti-sn65dsi83: Fix enable error path
Hello Alexander,
On Wed, 28 Feb 2024 09:15:46 +0100
Alexander Stein wrote:
[...]
> Oh I mistook this DSI-LVDS bridge with the DSI-DP bridge on a different
> board, my bad. I hope I can provide some insights. My platform is
> imx8mm-tqma8mqml-mba8mx-lvds-tm070jvhg33.dtb.
> I can easily cause a PLL lock failure by reducing the delay for the
> enable-gpios 'gpio_delays'. This will result in a PLL lock faiure.
> On my platform the vcc-supply counters do look sane:
> > /sys/kernel/debug/regulator/SN65DSI83_1V8/open_count:1
> > /sys/kernel/debug/regulator/SN65DSI83_1V8/use_count:0
Interesting. Thanks for taking time to report your initial issue!
> Once I remove the ti_sn65dsi83 module, the open_count decrements to 0 as
> well. Looks sane to me.
>
> If I revert commit c81cd8f7c774 ("Revert "drm/bridge: ti-sn65dsi83:
> Fix enable error path""), vcc-supply counters are:
> > /sys/kernel/debug/regulator/SN65DSI83_1V8/open_count:1
> > /sys/kernel/debug/regulator/SN65DSI83_1V8/use_count:1
>
> So in my case the use_count does not decrease! If I remove the module
> ti_sn65dsi83, I get the WARN_ON (enable_count is still non-zero):
> > WARNING: CPU: 2 PID: 402 at drivers/regulator/core.c:2398
> > _regulator_put+0x15c/0x164
>
> This is on 6.8.0-rc6-next-20240228 with the following diff applied:
> --->8---
> diff --git a/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> b/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> index 427467df42bf..8461e1fd396f 100644
> --- a/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> +++ b/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> @@ -285,7 +285,7 @@ &i2c3 {
> dsi_lvds_bridge: bridge@2d {
> compatible = "ti,sn65dsi84";
> reg = <0x2d>;
> - enable-gpios = <&gpio_delays 0 13 0>;
> + enable-gpios = <&gpio_delays 0 0 0>;
> vcc-supply = <®_sn65dsi83_1v8>;
> status = "disabled";
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> index 4814b7b6d1fd..57a7ed13f996 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> @@ -478,7 +478,6 @@ static void sn65dsi83_atomic_pre_enable(struct drm_bridge
> *bridge,
> dev_err(ctx->dev, "failed to lock PLL, ret=%i\n", ret);
> /* On failure, disable PLL again and exit. */
> regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
> - regulator_disable(ctx->vcc);
> return;
> }
> --->8---
>
> So my patch indeed did fix an actual problem. On the other hand it seems
> sn65dsi83_atomic_disable is not called in my case for some reason.
So you remove the module and atomic_disable is not called, after
having called atomic_pre_enable?
I'm very possibly missing something, but this looks like a bug in the
DRM bridge code at first sight.
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH] drm: xlnx: dp: Reset DisplayPort IP
On 29/02/2024 11:05, Laurent Pinchart wrote: Tomi, could you push this through drm-misc ? On Wed, Feb 28, 2024 at 06:22:25PM +0200, Laurent Pinchart wrote: Hello Rohit, Thank you for the patch. On Fri, Feb 16, 2024 at 04:40:43AM -0800, Rohit Visavalia wrote: Assert DisplayPort reset signal before deasserting, it is to clear out any registers programmed before booting kernel. Signed-off-by: Rohit Visavalia --- drivers/gpu/drm/xlnx/zynqmp_dp.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c index 1846c4971fd8..5a40aa1d4283 100644 --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c @@ -1714,6 +1714,10 @@ int zynqmp_dp_probe(struct zynqmp_dpsub *dpsub) goto err_free; } + ret = zynqmp_dp_reset(dp, true); + if (ret < 0) + return ret; + This looks fine to me, so Reviewed-by: Laurent Pinchart Thanks, tested and pushed to drm-misc-next. Tomi
RE: [PATCH v12 0/8] Enable Adaptive Sync SDP Support for DP
On Thu, 29 Feb 2024, "Golani, Mitulkumar Ajitkumar" wrote: >> Subject: Re: [PATCH v12 0/8] Enable Adaptive Sync SDP Support for DP >> >> On Wed, 28 Feb 2024, Mitul Golani >> wrote: >> > -v12: >> > - Update cover letter >> >> Did you just send the entire series again within 30 minutes just to update >> the >> cover letter? You could've just replied to the cover letter... > > Sorry, I should have used git send-email --in-reply-to, will take care from > next time onwards. I mean you can just reply to the cover letter and say what you missed, you don't have to send the whole thing again. It's no different from any email. BR, Jani. -- Jani Nikula, Intel
[PATCH] drm/tests/buddy: fix print format
This will report a build warning once we have: 806cb2270237 ("kunit:
Annotate _MSG assertion variants with gnu printf specifiers").
Reported-by: Stephen Rothwell
Fixes: c70703320e55 ("drm/tests/drm_buddy: add alloc_range_bias test")
Signed-off-by: Matthew Auld
Cc: Arunpravin Paneer Selvam
Cc: Christian König
---
drivers/gpu/drm/tests/drm_buddy_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c
b/drivers/gpu/drm/tests/drm_buddy_test.c
index be2d9d7764be..484360c7e1f6 100644
--- a/drivers/gpu/drm/tests/drm_buddy_test.c
+++ b/drivers/gpu/drm/tests/drm_buddy_test.c
@@ -189,7 +189,7 @@ static void drm_test_buddy_alloc_range_bias(struct kunit
*test)
&allocated,
DRM_BUDDY_RANGE_ALLOCATION),
"buddy_alloc failed with bias(%x-%x),
size=%u, ps=%u\n",
- bias_start, bias_end, size);
+ bias_start, bias_end, size, ps);
bias_rem -= size;
/*
--
2.43.2
Re: [PATCH 1/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing
Hi! On Thu, Feb 29, 2024 at 11:23:22AM +0200, Nikolai Kondrashov wrote: > Hi everyone, > > On 2/29/24 11:02, Maxime Ripard wrote: > > On Wed, Feb 28, 2024 at 07:55:25PM -0300, Helen Koike wrote: > > > Which rating would you select? > > > > 4.5 :) > > > > One thing I'm wondering here is how we're going to cope with the > > different requirements each user / framework has. > > > > Like, Linus probably want to have a different set of CI before merging a > > PR than (say) linux-next does, or stable, or before doing an actual > > release. > > > > Similarly, DRM probably has a different set of requirements than > > drm-misc, drm-amd or nouveau. > > > > I don't see how the current architecture could accomodate for that. I > > know that Gitlab allows to store issues template in a separate repo, > > maybe we could ask them to provide a feature where the actions would be > > separate from the main repo? That way, any gitlab project could provide > > its own set of tests, without conflicting with each others (and we could > > still share them if we wanted to) > > > > I know some of use had good relationship with Gitlab, so maybe it would > > be worth asking? > > GitLab already supports getting the CI YAML from other repos. You can change > that in the repo settings. I'm interested but couldn't find it in the doc, do you have a link to the right section? > However, I think a better approach would be *not* to add the .gitlab-ci.yaml > file in the root of the source tree, but instead change the very same repo > setting to point to a particular entry YAML, *inside* the repo (somewhere > under "ci" directory) instead. > > This way all the different subtrees can have completely different setup, but > some could still use Helen's work and employ the "scenarios" she > implemented. I'm worried that this kind of setup will just create duplicated YAML that will be developped in complete silos and will become difficult to maintain. But that's definitely an opinion :) Maxime signature.asc Description: PGP signature
Re: [PATCH v12 1/8] drm/dp: Add support to indicate if sink supports AS SDP
On 2/28/2024 8:08 PM, Mitul Golani wrote:
Add an API that indicates support for Adaptive Sync SDP in
the sink, which can be utilized by the rest of the DP programming.
--v1:
- Format commit message properly.
Signed-off-by: Mitul Golani
LGTM.
Reviewed-by: Ankit Nautiyal
---
drivers/gpu/drm/display/drm_dp_helper.c | 25 +
include/drm/display/drm_dp_helper.h | 1 +
2 files changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
b/drivers/gpu/drm/display/drm_dp_helper.c
index 9ac52cf5d4d8..f94c04db7187 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -2913,6 +2913,31 @@ void drm_dp_vsc_sdp_log(struct drm_printer *p, const
struct drm_dp_vsc_sdp *vsc)
}
EXPORT_SYMBOL(drm_dp_vsc_sdp_log);
+/**
+ * drm_dp_as_sdp_supported() - check if adaptive sync sdp is supported
+ * @aux: DisplayPort AUX channel
+ * @dpcd: DisplayPort configuration data
+ *
+ * Returns true if adaptive sync sdp is supported, else returns false
+ */
+bool drm_dp_as_sdp_supported(struct drm_dp_aux *aux, const u8
dpcd[DP_RECEIVER_CAP_SIZE])
+{
+ u8 rx_feature;
+
+ if (dpcd[DP_DPCD_REV] < DP_DPCD_REV_13)
+ return false;
+
+ if (drm_dp_dpcd_readb(aux, DP_DPRX_FEATURE_ENUMERATION_LIST_CONT_1,
+ &rx_feature) != 1) {
+ drm_dbg_dp(aux->drm_dev,
+ "Failed to read
DP_DPRX_FEATURE_ENUMERATION_LIST_CONT_1\n");
+ return false;
+ }
+
+ return (rx_feature & DP_ADAPTIVE_SYNC_SDP_SUPPORTED);
+}
+EXPORT_SYMBOL(drm_dp_as_sdp_supported);
+
/**
* drm_dp_vsc_sdp_supported() - check if vsc sdp is supported
* @aux: DisplayPort AUX channel
diff --git a/include/drm/display/drm_dp_helper.h
b/include/drm/display/drm_dp_helper.h
index 0c1a4021e098..7c1aa3a703c8 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -101,6 +101,7 @@ struct drm_dp_vsc_sdp {
void drm_dp_vsc_sdp_log(struct drm_printer *p, const struct drm_dp_vsc_sdp
*vsc);
bool drm_dp_vsc_sdp_supported(struct drm_dp_aux *aux, const u8 dpcd[DP_RECEIVER_CAP_SIZE]);
+bool drm_dp_as_sdp_supported(struct drm_dp_aux *aux, const u8
dpcd[DP_RECEIVER_CAP_SIZE]);
int drm_dp_psr_setup_time(const u8 psr_cap[EDP_PSR_RECEIVER_CAP_SIZE]);
Re: [PATCH 1/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing
On Thu, Feb 29, 2024 at 10:56:58AM +0100, Maxime Ripard wrote: > Hi! > > On Thu, Feb 29, 2024 at 11:23:22AM +0200, Nikolai Kondrashov wrote: > > Hi everyone, > > > > On 2/29/24 11:02, Maxime Ripard wrote: > > > On Wed, Feb 28, 2024 at 07:55:25PM -0300, Helen Koike wrote: > > > > Which rating would you select? > > > > > > 4.5 :) > > > > > > One thing I'm wondering here is how we're going to cope with the > > > different requirements each user / framework has. > > > > > > Like, Linus probably want to have a different set of CI before merging a > > > PR than (say) linux-next does, or stable, or before doing an actual > > > release. > > > > > > Similarly, DRM probably has a different set of requirements than > > > drm-misc, drm-amd or nouveau. > > > > > > I don't see how the current architecture could accomodate for that. I > > > know that Gitlab allows to store issues template in a separate repo, > > > maybe we could ask them to provide a feature where the actions would be > > > separate from the main repo? That way, any gitlab project could provide > > > its own set of tests, without conflicting with each others (and we could > > > still share them if we wanted to) > > > > > > I know some of use had good relationship with Gitlab, so maybe it would > > > be worth asking? > > > > GitLab already supports getting the CI YAML from other repos. You can change > > that in the repo settings. > > I'm interested but couldn't find it in the doc, do you have a link to > the right section? e.g. https://gitlab.freedesktop.org/camera/libcamera/-/settings/ci_cd Expand "General pipelines", the setting is "CI/CD configuration file". You can specify the path to a file in the local repository, or in a separate repository. See https://gitlab.freedesktop.org/help/ci/pipelines/settings#specify-a-custom-cicd-configuration-file for the syntax. > > However, I think a better approach would be *not* to add the .gitlab-ci.yaml > > file in the root of the source tree, but instead change the very same repo > > setting to point to a particular entry YAML, *inside* the repo (somewhere > > under "ci" directory) instead. > > > > This way all the different subtrees can have completely different setup, but > > some could still use Helen's work and employ the "scenarios" she > > implemented. > > I'm worried that this kind of setup will just create duplicated YAML > that will be developped in complete silos and will become difficult to > maintain. But that's definitely an opinion :) -- Regards, Laurent Pinchart
Re: [PATCH v12 2/8] drm: Add Adaptive Sync SDP logging
On 2/28/2024 8:08 PM, Mitul Golani wrote:
Add structure representing Adaptive Sync Secondary Data Packet (AS SDP).
Also, add Adaptive Sync SDP logging in drm_dp_helper.c to facilitate
debugging.
--v2:
- Update logging. [Jani, Ankit]
- Use 'as_sdp' instead of 'async' [Ankit]
- Correct define placeholders to where they are actually used. [Jani]
- Update members in 'as_sdp' structure to make it uniform. [Jani]
--v3:
- Added changes to dri-devel mailing list. No code changes.
--v4:
- Instead of directly using operation mode, use an enum to accommodate
all operation modes (Ankit).
--v5:
Nit-pick changes to commit message.
Signed-off-by: Mitul Golani
---
drivers/gpu/drm/display/drm_dp_helper.c | 12 +++
.../drm/i915/display/intel_crtc_state_dump.c | 12 +++
.../drm/i915/display/intel_display_types.h| 1 +
include/drm/display/drm_dp.h | 9 +
include/drm/display/drm_dp_helper.h | 33 +++
5 files changed, 67 insertions(+)
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
b/drivers/gpu/drm/display/drm_dp_helper.c
index f94c04db7187..b1459ac92aea 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -2913,6 +2913,18 @@ void drm_dp_vsc_sdp_log(struct drm_printer *p, const
struct drm_dp_vsc_sdp *vsc)
}
EXPORT_SYMBOL(drm_dp_vsc_sdp_log);
+void drm_dp_as_sdp_log(struct drm_printer *p, const struct drm_dp_as_sdp *as_sdp)
+{
+ drm_printf(p, "DP SDP: AS_SDP, revision %u, length %u\n",
+ as_sdp->revision, as_sdp->length);
+ drm_printf(p, "vtotal: %d\n", as_sdp->vtotal);
+ drm_printf(p, "target_rr: %d\n", as_sdp->target_rr);
+ drm_printf(p, "duration_incr_ms: %d\n", as_sdp->duration_incr_ms);
+ drm_printf(p, "duration_decr_ms: %d\n", as_sdp->duration_decr_ms);
+ drm_printf(p, "operation_mode: %d\n", as_sdp->mode);
+}
+EXPORT_SYMBOL(drm_dp_as_sdp_log);
+
/**
* drm_dp_as_sdp_supported() - check if adaptive sync sdp is supported
* @aux: DisplayPort AUX channel
diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
index 4bcf446c75f4..26d77c2934e8 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
@@ -60,6 +60,15 @@ intel_dump_dp_vsc_sdp(struct drm_i915_private *i915,
drm_dp_vsc_sdp_log(&p, vsc);
}
+static void
+intel_dump_dp_as_sdp(struct drm_i915_private *i915,
+const struct drm_dp_as_sdp *as_sdp)
+{
+ struct drm_printer p = drm_dbg_printer(&i915->drm, DRM_UT_KMS,
"AS_SDP");
+
+ drm_dp_as_sdp_log(&p, as_sdp);
+}
+
static void
intel_dump_buffer(struct drm_i915_private *i915,
const char *prefix, const u8 *buf, size_t len)
@@ -299,6 +308,9 @@ void intel_crtc_state_dump(const struct intel_crtc_state
*pipe_config,
if (pipe_config->infoframes.enable &
intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GAMUT_METADATA))
intel_dump_infoframe(i915, &pipe_config->infoframes.drm);
+ if (pipe_config->infoframes.enable &
+ intel_hdmi_infoframe_enable(DP_SDP_ADAPTIVE_SYNC))
+ intel_dump_dp_as_sdp(i915, &pipe_config->infoframes.as_sdp);
if (pipe_config->infoframes.enable &
intel_hdmi_infoframe_enable(DP_SDP_VSC))
intel_dump_dp_vsc_sdp(i915, &pipe_config->infoframes.vsc);
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 8ce986fadd9a..1256730ea276 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1335,6 +1335,7 @@ struct intel_crtc_state {
union hdmi_infoframe hdmi;
union hdmi_infoframe drm;
struct drm_dp_vsc_sdp vsc;
+ struct drm_dp_as_sdp as_sdp;
} infoframes;
u8 eld[MAX_ELD_BYTES];
diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
index 281afff6ee4e..0601b95d53db 100644
--- a/include/drm/display/drm_dp.h
+++ b/include/drm/display/drm_dp.h
@@ -1578,10 +1578,12 @@ enum drm_dp_phy {
#define DP_SDP_AUDIO_COPYMANAGEMENT 0x05 /* DP 1.2 */
#define DP_SDP_ISRC 0x06 /* DP 1.2 */
#define DP_SDP_VSC0x07 /* DP 1.2 */
+#define DP_SDP_ADAPTIVE_SYNC 0x22 /* DP 1.4 */
#define DP_SDP_CAMERA_GENERIC(i) (0x08 + (i)) /* 0-7, DP 1.3 */
#define DP_SDP_PPS0x10 /* DP 1.4 */
#define DP_SDP_VSC_EXT_VESA 0x20 /* DP 1.4 */
#define DP_SDP_VSC_EXT_CEA0x21 /* DP 1.4 */
+
/* 0x80+ CEA-861 infoframe types */
#define DP_SDP_AUDIO_INFOFRAME_HB2 0x1b
@@ -1737,4 +1739,11 @@ enum dp_content_type {
DP_CONTENT_TYPE_GAME = 0x04,
};
+enum operation_mode {
+ DP_AS_SDP_AVT_DYNAMI
Re: [PATCH v2 5/9] drm/vkms: Re-introduce line-per-line composition algorithm
On Tue, 27 Feb 2024 16:02:09 +0100
Louis Chauvet wrote:
> [...]
>
> > > -static void pre_mul_alpha_blend(struct vkms_frame_info *frame_info,
> > > - struct line_buffer *stage_buffer,
> > > - struct line_buffer *output_buffer)
> > > +static void pre_mul_alpha_blend(
> > > + struct line_buffer *stage_buffer,
> > > + struct line_buffer *output_buffer,
> > > + int x_start,
> > > + int pixel_count)
> > > {
> > > - int x_dst = frame_info->dst.x1;
> > > - struct pixel_argb_u16 *out = output_buffer->pixels + x_dst;
> > > - struct pixel_argb_u16 *in = stage_buffer->pixels;
> > > - int x_limit = min_t(size_t, drm_rect_width(&frame_info->dst),
> > > - stage_buffer->n_pixels);
> > > -
> > > - for (int x = 0; x < x_limit; x++) {
> > > - out[x].a = (u16)0x;
> > > - out[x].r = pre_mul_blend_channel(in[x].r, out[x].r, in[x].a);
> > > - out[x].g = pre_mul_blend_channel(in[x].g, out[x].g, in[x].a);
> > > - out[x].b = pre_mul_blend_channel(in[x].b, out[x].b, in[x].a);
> > > + struct pixel_argb_u16 *out = &output_buffer->pixels[x_start];
> > > + struct pixel_argb_u16 *in = &stage_buffer->pixels[x_start];
> >
> > Input buffers and pointers should be const.
>
> They will be const in v4.
>
> > > +
> > > + for (int i = 0; i < pixel_count; i++) {
> > > + out[i].a = (u16)0x;
> > > + out[i].r = pre_mul_blend_channel(in[i].r, out[i].r, in[i].a);
> > > + out[i].g = pre_mul_blend_channel(in[i].g, out[i].g, in[i].a);
> > > + out[i].b = pre_mul_blend_channel(in[i].b, out[i].b, in[i].a);
> > > }
> > > }
> >
> > Somehow the hunk above does not feel like it is part of "re-introduce
> > line-per-line composition algorithm". This function was already running
> > line-by-line. Would it be easy enough to collect this and directly
> > related changes into a separate patch?
>
> It is not directly related to the reintroduction of line-by-line
> algorithm, but in the simplification and maintenability effort, I
> changed a bit the function to avoid having multiple place computing the
> x_start/pixel_count values. I don't see an interrest to extract it, it
> will be just a translation of the few lines into the calling place.
It does make review more difficult, because it makes the patch bigger
and is not explained in the commit message. It is a surprise to a
reviewer, who then needs to think what this means and does it belong
here.
If you explain it in the commit message and note it in the commit
summary line, I think it would become fairly obvious that this patch is
doing two things rather than one.
Therefore, *if* it is easy to extract as a separate patch, then it
would be nice to do so. However, if doing so would require you to write
a bunch of temporary code that the next patch would just rewrite again,
then doing so would be counter-productive.
Patch split is about finding a good trade-off to make things easy for
reviewers:
- Smaller patches are better as long as they are self-standing and
understandable in isolation, and of course do not regress anything.
- Rewriting the same thing multiple times in the same series is extra
work for a reviewer and therefore best avoided.
- The simpler the semantic change, the bigger a patch can be and still
be easy to review.
And all the patch writing rules specific to the kernel project that I
don't know about.
> [...]
>
> > > +/**
> > > + * direction_for_rotation() - Helper to get the correct reading
> > > direction for a specific rotation
> > > + *
> > > + * @rotation: rotation to analyze
> >
> > This is KMS plane rotation property, right?
> >
> > So the KMS plane has been rotated by this, and what we want to find is
> > the read direction on the attached FB so that reading returns pixels in
> > the CRTC line/scanout order, right?
> >
> > Maybe extend the doc to explain that.
>
> Is it better?
>
> * direction_for_rotation() - Get the correct reading direction for a given
> rotation
> *
> * This function will use the @rotation parameter to compute the correct
> reading direction to read
> * a line from the source buffer.
> * For example, if the buffer is reflected on X axis, the pixel must be read
> from right to left.
> * @rotation: Rotation to analyze. It correspond the the field
> @frame_info.rotation.
I think it is important to define what determines the correct result.
In this case, we want the reading to produce pixels in the CRTC scanout
line order, I believe. If you don't say "CRTC", the reader does not
know what "the correct reading direction" should match to.
> > > + */
> > > +enum pixel_read_direction direction_for_rotation(unsigned int rotation)
> > > +{
> > > + if (rotation & DRM_MODE_ROTATE_0) {
> > > + if (rotation & DRM_MODE_REFLECT_X)
> > > + return READ_LEFT;
> > > + else
> > > + return READ_RIGHT;
> > > + } else if (rotation & DRM_MODE_ROTATE_9
Re: [PATCH v2 8/9] media: dt-bindings: Add Intel Displayport RX IP
On Wed, Feb 28, 2024 at 7:10 PM Rob Herring wrote: > > On Wed, Feb 28, 2024 at 02:09:33PM +0100, Paweł Anikiel wrote: > > On Wed, Feb 28, 2024 at 1:18 PM Krzysztof Kozlowski > > wrote: > > > > > > On 28/02/2024 12:05, Paweł Anikiel wrote: > > > > On Tue, Feb 27, 2024 at 3:29 PM Rob Herring wrote: > > > >> > > > >> On Mon, Feb 26, 2024 at 11:59:42AM +0100, Paweł Anikiel wrote: > > > >>> On Mon, Feb 26, 2024 at 10:13 AM Krzysztof Kozlowski > > > >>> wrote: > > > > > > On 21/02/2024 17:02, Paweł Anikiel wrote: > > > > The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA > > > > IP > > > > Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video > > > > capture and Multi-Stream Transport. The user guide can be found > > > > here: > > > > > > > > https://www.intel.com/programmable/technical-pdfs/683273.pdf > > > > > > > > Signed-off-by: Paweł Anikiel > > > > --- > > > > .../devicetree/bindings/media/intel,dprx.yaml | 160 > > > > ++ > > > > 1 file changed, 160 insertions(+) > > > > create mode 100644 > > > > Documentation/devicetree/bindings/media/intel,dprx.yaml > > > > > > > > diff --git > > > > a/Documentation/devicetree/bindings/media/intel,dprx.yaml > > > > b/Documentation/devicetree/bindings/media/intel,dprx.yaml > > > > new file mode 100644 > > > > index ..31025f2d5dcd > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/media/intel,dprx.yaml > > > > @@ -0,0 +1,160 @@ > > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > > > +%YAML 1.2 > > > > +--- > > > > +$id: http://devicetree.org/schemas/media/intel,dprx.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: Intel DisplayPort RX IP > > > > + > > > > +maintainers: > > > > + - Paweł Anikiel > > > > + > > > > +description: | > > > > + The Intel Displayport RX IP is a part of the DisplayPort Intel > > > > FPGA IP > > > > + Core. It implements a DisplayPort 1.4 receiver capable of HBR3 > > > > video > > > > + capture and Multi-Stream Transport. > > > > + > > > > + The IP features a large number of configuration parameters, > > > > found at: > > > > + > > > > https://www.intel.com/content/www/us/en/docs/programmable/683273/23-3-20-0-1/sink-parameters.html > > > > + > > > > + The following parameters have to be enabled: > > > > +- Support DisplayPort sink > > > > +- Enable GPU control > > > > + The following parameters' values have to be set in the > > > > devicetree: > > > > +- RX maximum link rate > > > > +- Maximum lane count > > > > +- Support MST > > > > +- Max stream count (only if Support MST is enabled) > > > > + > > > > +properties: > > > > + compatible: > > > > +const: intel,dprx-20.0.1 > > > > + > > > > + reg: > > > > +maxItems: 1 > > > > + > > > > + interrupts: > > > > +maxItems: 1 > > > > + > > > > + intel,max-link-rate: > > > > +$ref: /schemas/types.yaml#/definitions/uint32 > > > > +description: Max link rate configuration parameter > > > > > > Please do not duplicate property name in description. It's useless. > > > Instead explain what is this responsible for. > > > > > > Why max-link-rate would differ for the same dprx-20.0.1? And why > > > standard properties cannot be used? > > > > > > Same for all questions below. > > > >>> > > > >>> These four properties are the IP configuration parameters mentioned in > > > >>> the device description. When generating the IP core you can set these > > > >>> parameters, which could make them differ for the same dprx-20.0.1. > > > >>> They are documented in the user guide, for which I also put a link in > > > >>> the description. Is that enough? Or should I also document these > > > >>> parameters here? > > > >> > > > >> Use the standard properties: link-frequencies and data-lanes. Those go > > > >> under the port(s) because they are inheritly per logical link. > > > > > > > > The DP receiver has one input interface (a deserialized DP stream), > > > > and up to four output interfaces (the decoded video streams). The "max > > > > link rate" and "max lane count" parameters only describe the input > > > > interface to the receiver. However, the port(s) I am using here are > > > > for the output streams. They are not affected by those parameters, so > > > > I don't think these properties should go under the output port(s). > > > > > > > > The receiver doesn't have an input port in the DT, because there isn't > > > > any controllable entity on the other side - the deserializer doesn't > > > > have any software interface. Since these standard properties > > > > (link-frequencies and data-
Re: [PATCH v12 3/8] drm/i915/dp: Add Read/Write support for Adaptive Sync SDP
On 2/28/2024 8:08 PM, Mitul Golani wrote:
Add the necessary structures and functions to handle reading and
unpacking Adaptive Sync Secondary Data Packets. Also add support
to write and pack AS SDP.
--v2:
- Correct use of REG_BIT and REG_GENMASK. [Jani]
- Use as_sdp instead of async. [Jani]
- Remove unrelated comments and changes. [Jani]
- Correct code indent. [Jani]
--v3:
- Update definition names for AS SDP which are starting from
HSW, as these defines are applicable for ADLP+.(Ankit)
--v4:
- Remove as_sdp_mode from crtc_state.
- Drop metadata keyword.
- For consistency, update ADL_ prefix or post fix as required.
Signed-off-by: Mitul Golani
---
drivers/gpu/drm/i915/display/intel_dp.c | 88 ++-
drivers/gpu/drm/i915/display/intel_hdmi.c | 12 +++-
drivers/gpu/drm/i915/i915_reg.h | 8 +++
include/drm/display/drm_dp_helper.h | 1 +
4 files changed, 106 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index e13121dc3a03..e5377cdc71c6 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4089,6 +4089,32 @@ intel_dp_needs_vsc_sdp(const struct intel_crtc_state
*crtc_state,
return false;
}
+static ssize_t intel_dp_as_sdp_pack(const struct drm_dp_as_sdp *as_sdp,
+ struct dp_sdp *sdp, size_t size)
+{
+ size_t length = sizeof(struct dp_sdp);
+
+ if (size < length)
+ return -ENOSPC;
+
+ memset(sdp, 0, size);
+
+ /* Prepare AS (Adaptive Sync) SDP Header */
+ sdp->sdp_header.HB0 = 0;
+ sdp->sdp_header.HB1 = as_sdp->sdp_type;
+ sdp->sdp_header.HB2 = 0x02;
+ sdp->sdp_header.HB3 = as_sdp->length;
+
+ /* Fill AS (Adaptive Sync) SDP Payload */
+ sdp->db[0] = as_sdp->mode;
+ sdp->db[1] = as_sdp->vtotal & 0xFF;
+ sdp->db[2] = (as_sdp->vtotal >> 8) & 0xFF;
+ sdp->db[3] = as_sdp->target_rr;
+ sdp->db[4] = (as_sdp->target_rr >> 8) & 0x3;
+
+ return length;
+}
+
static ssize_t
intel_dp_hdr_metadata_infoframe_sdp_pack(struct drm_i915_private *i915,
const struct hdmi_drm_infoframe
*drm_infoframe,
@@ -4188,6 +4214,10 @@ static void intel_write_dp_sdp(struct intel_encoder
*encoder,
&crtc_state->infoframes.drm.drm,
&sdp,
sizeof(sdp));
break;
+ case DP_SDP_ADAPTIVE_SYNC:
+ len = intel_dp_as_sdp_pack(&crtc_state->infoframes.as_sdp, &sdp,
+ sizeof(sdp));
+ break;
default:
MISSING_CASE(type);
return;
@@ -4208,7 +4238,8 @@ void intel_dp_set_infoframes(struct intel_encoder
*encoder,
i915_reg_t reg = HSW_TVIDEO_DIP_CTL(crtc_state->cpu_transcoder);
u32 dip_enable = VIDEO_DIP_ENABLE_AVI_HSW | VIDEO_DIP_ENABLE_GCP_HSW |
VIDEO_DIP_ENABLE_VS_HSW | VIDEO_DIP_ENABLE_GMP_HSW |
-VIDEO_DIP_ENABLE_SPD_HSW | VIDEO_DIP_ENABLE_DRM_GLK;
+VIDEO_DIP_ENABLE_SPD_HSW | VIDEO_DIP_ENABLE_DRM_GLK |
+VIDEO_DIP_ENABLE_AS_ADL;
u32 val = intel_de_read(dev_priv, reg) & ~dip_enable;
/* TODO: Sanitize DSC enabling wrt. intel_dsc_dp_pps_write(). */
@@ -4230,6 +4261,36 @@ void intel_dp_set_infoframes(struct intel_encoder
*encoder,
intel_write_dp_sdp(encoder, crtc_state,
HDMI_PACKET_TYPE_GAMUT_METADATA);
}
+static
+int intel_dp_as_sdp_unpack(struct drm_dp_as_sdp *as_sdp,
+ const void *buffer, size_t size)
+{
+ const struct dp_sdp *sdp = buffer;
+
+ if (size < sizeof(struct dp_sdp))
+ return -EINVAL;
+
+ memset(as_sdp, 0, sizeof(*as_sdp));
+
+ if (sdp->sdp_header.HB0 != 0)
+ return -EINVAL;
+
+ if (sdp->sdp_header.HB1 != DP_SDP_ADAPTIVE_SYNC)
+ return -EINVAL;
+
+ if (sdp->sdp_header.HB2 != 0x02)
+ return -EINVAL;
+
+ if ((sdp->sdp_header.HB3 & 0x3F) != 9)
+ return -EINVAL;
+
+ as_sdp->mode = sdp->db[0] & AS_SDP_OP_MODE;
+ as_sdp->vtotal = ((u64)sdp->db[2] << 32) | (u64)sdp->db[1];
+ as_sdp->target_rr = (u64)sdp->db[3] | ((u64)sdp->db[4] & 0x3);
+
+ return 0;
+}
+
static int intel_dp_vsc_sdp_unpack(struct drm_dp_vsc_sdp *vsc,
const void *buffer, size_t size)
{
@@ -4300,6 +4361,27 @@ static int intel_dp_vsc_sdp_unpack(struct drm_dp_vsc_sdp
*vsc,
return 0;
}
+static int
+intel_read_dp_infoframe_as_sdp(struct intel_encoder *encoder,
+ struct intel_crtc_state *crtc_state,
+ struct drm_dp_as_sdp *as_sdp)
+{
+ struct i
Re: [PATCH] drm/tests/buddy: fix print format
Reviewed-by: Arunpravin Paneer Selvam
On 2/29/2024 3:22 PM, Matthew Auld wrote:
This will report a build warning once we have: 806cb2270237 ("kunit:
Annotate _MSG assertion variants with gnu printf specifiers").
Reported-by: Stephen Rothwell
Fixes: c70703320e55 ("drm/tests/drm_buddy: add alloc_range_bias test")
Signed-off-by: Matthew Auld
Cc: Arunpravin Paneer Selvam
Cc: Christian König
---
drivers/gpu/drm/tests/drm_buddy_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c
b/drivers/gpu/drm/tests/drm_buddy_test.c
index be2d9d7764be..484360c7e1f6 100644
--- a/drivers/gpu/drm/tests/drm_buddy_test.c
+++ b/drivers/gpu/drm/tests/drm_buddy_test.c
@@ -189,7 +189,7 @@ static void drm_test_buddy_alloc_range_bias(struct kunit
*test)
&allocated,
DRM_BUDDY_RANGE_ALLOCATION),
"buddy_alloc failed with bias(%x-%x),
size=%u, ps=%u\n",
- bias_start, bias_end, size);
+ bias_start, bias_end, size, ps);
bias_rem -= size;
/*
Re: [PATCH v12 4/8] drm/i915/display/dp: Add wrapper function to check AS SDP
On 2/28/2024 8:08 PM, Mitul Golani wrote:
Add a wrapper function to check if both the source and
sink support Adaptive Sync SDP.
Signed-off-by: Mitul Golani
Just use drm/i915/dp in subject line
Otherwise LGTM.
Reviewed-by: Ankit Nautiyal
---
drivers/gpu/drm/i915/display/intel_display_device.h | 1 +
drivers/gpu/drm/i915/display/intel_dp.c | 8
drivers/gpu/drm/i915/display/intel_dp.h | 1 +
3 files changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h
b/drivers/gpu/drm/i915/display/intel_display_device.h
index fe4268813786..6399fbc6c738 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.h
+++ b/drivers/gpu/drm/i915/display/intel_display_device.h
@@ -68,6 +68,7 @@ struct drm_printer;
#define HAS_TRANSCODER(i915, trans)
((DISPLAY_RUNTIME_INFO(i915)->cpu_transcoder_mask & \
BIT(trans)) != 0)
#define HAS_VRR(i915) (DISPLAY_VER(i915) >= 11)
+#define HAS_AS_SDP(i915) (DISPLAY_VER(i915) >= 13)
#define INTEL_NUM_PIPES(i915)
(hweight8(DISPLAY_RUNTIME_INFO(i915)->pipe_mask))
#define I915_HAS_HOTPLUG(i915)
(DISPLAY_INFO(i915)->has_hotplug)
#define OVERLAY_NEEDS_PHYSICAL(i915)
(DISPLAY_INFO(i915)->overlay_needs_physical)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index e5377cdc71c6..7eb83924f3fe 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -120,6 +120,14 @@ bool intel_dp_is_edp(struct intel_dp *intel_dp)
return dig_port->base.type == INTEL_OUTPUT_EDP;
}
+bool intel_dp_as_sdp_supported(struct intel_dp *intel_dp)
+{
+ struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+
+ return HAS_AS_SDP(i915) &&
+ drm_dp_as_sdp_supported(&intel_dp->aux, intel_dp->dpcd);
+}
+
static void intel_dp_unset_edid(struct intel_dp *intel_dp);
/* Is link rate UHBR and thus 128b/132b? */
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
b/drivers/gpu/drm/i915/display/intel_dp.h
index 530cc97bc42f..cc5e069091ff 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -80,6 +80,7 @@ void intel_dp_audio_compute_config(struct intel_encoder
*encoder,
struct drm_connector_state *conn_state);
bool intel_dp_has_hdmi_sink(struct intel_dp *intel_dp);
bool intel_dp_is_edp(struct intel_dp *intel_dp);
+bool intel_dp_as_sdp_supported(struct intel_dp *intel_dp);
bool intel_dp_is_uhbr(const struct intel_crtc_state *crtc_state);
int intel_dp_link_symbol_size(int rate);
int intel_dp_link_symbol_clock(int rate);
Re: [RFC] drm/fourcc: Add RPI modifiers
On Tue, Feb 27, 2024 at 03:10:06PM +0200, Laurent Pinchart wrote: > On Mon, Feb 26, 2024 at 05:24:41PM +0100, Jacopo Mondi wrote: > > On Mon, Feb 26, 2024 at 04:46:24PM +0100, Daniel Vetter wrote: > > > On Mon, 26 Feb 2024 at 16:39, Jacopo Mondi wrote: > > > > > > > > Add modifiers for the Raspberry Pi PiSP compressed formats. > > > > > > > > The compressed formats are documented at: > > > > Documentation/userspace-api/media/v4l/pixfmt-pisp-comp-rggb.rst > > > > > > > > and in the PiSP datasheet: > > > > https://datasheets.raspberrypi.com/camera/raspberry-pi-image-signal-processor-specification.pdf > > > > > > > > Signed-off-by: Jacopo Mondi > > > > --- > > > > > > > > Background: > > > > --- > > > > > > > > The Raspberry Pi PiSP camera subsystem is on its way to upstream > > > > through the > > > > Video4Linux2 subsystem: > > > > https://patchwork.linuxtv.org/project/linux-media/list/?series=12310 > > > > > > > > The PiSP camera system is composed by a "Front End" and a "Back End". > > > > The FrontEnd part is a MIPI CSI-2 receiver that store frames to memory > > > > and > > > > produce statistics, and the BackEnd is a memory-to-memory ISP that > > > > converts > > > > images in a format usable by application. > > > > > > > > The "FrontEnd" is capable of encoding RAW Bayer images as received by > > > > the > > > > image sensor in a 'compressed' format defined by Raspberry Pi and fully > > > > documented in the PiSP manual: > > > > https://datasheets.raspberrypi.com/camera/raspberry-pi-image-signal-processor-specification.pdf > > > > > > > > The compression scheme is documented in the in-review patch series for > > > > the BE > > > > support at: > > > > https://patchwork.linuxtv.org/project/linux-media/patch/20240223163012.300763-7-jacopo.mo...@ideasonboard.com/ > > > > > > > > The "BackEnd" is capable of consuming images in the compressed format > > > > and > > > > optionally user application might want to inspect those images for > > > > debugging > > > > purposes. > > > > > > > > Why a DRM modifier > > > > -- > > > > > > > > The PiSP support is entirely implemented in libcamera, with the support > > > > of an > > > > hw-specific library called 'libpisp'. > > > > > > > > libcamera uses the fourcc codes defined by DRM to define its formats: > > > > https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/formats.yaml > > > > > > > > And to define a new libcamera format for the Raspberry Pi compressed > > > > ones we > > > > need to associate the above proposed modifiers with a RAW Bayer format > > > > identifier. > > > > > > > > In example: > > > > > > > > - RGGB16_PISP_COMP1: > > > > fourcc: DRM_FORMAT_SRGGB16 > > > > mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > - GRBG16_PISP_COMP1: > > > > fourcc: DRM_FORMAT_SGRBG16 > > > > mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > - GBRG16_PISP_COMP1: > > > > fourcc: DRM_FORMAT_SGBRG16 > > > > mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > - BGGR16_PISP_COMP1: > > > > fourcc: DRM_FORMAT_SBGGR16 > > > > mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > - MONO_PISP_COMP1: > > > > fourcc: DRM_FORMAT_R16 > > > > mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > > > > > See > > > > https://patchwork.libcamera.org/patch/19503/ > > > > > > > > Would if be acceptable for DRM to include the above proposed modifiers > > > > for the > > > > purpose of defining the above presented libcamera formats ? There will > > > > be no > > > > graphic format associated with these modifiers as their purpose it not > > > > displaying images but rather exchange them between the components of the > > > > camera subsystem (and possibly be inspected by specialized test > > > > applications). > > > > > > Yeah I think libcamera using drm-fourcc formats and modifiers is > > > absolutely ok, and has my ack in principle. And for these users we're > > > ok with merging modifiers that the kernel doesn't use. > > > > Great! > > > > > I think it would be really good to formalize this by adding libcamera > > > to the officially listed users in the "Open Source User Waiver" > > > section in the drm_fourcc.h docs: > > > > > > https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#open-source-user-waiver > > > > > > You might want to convert that into a list, it could get a bit > > > confusing. Then we can get that patch properly acked (by kernel and > > > libcamera folks) to record the community consensus. > > > > I see your point, but I wonder if libcamera actually need to be part > > of this list; we want in-kernel upstream driver and open-source > > For the kernel side we'll use V4L2, so the DRM modifier won't have an > in-kernel user. Also if you e.g. use a gpu or accel thing for post-processing, which would be a drm driver, then you still don't have an in-kernel user for these because the userspace driver handles that all directly and just sends command buffers to the kernel. So I think adding li
Re: [PATCH v12 2/8] drm: Add Adaptive Sync SDP logging
On Wed, 28 Feb 2024, Mitul Golani wrote:
> Add structure representing Adaptive Sync Secondary Data Packet (AS SDP).
> Also, add Adaptive Sync SDP logging in drm_dp_helper.c to facilitate
> debugging.
To be honest, the division of patches is a bit weird. There's no reason
to change i915 here, is there?
> --v2:
> - Update logging. [Jani, Ankit]
> - Use 'as_sdp' instead of 'async' [Ankit]
> - Correct define placeholders to where they are actually used. [Jani]
> - Update members in 'as_sdp' structure to make it uniform. [Jani]
>
> --v3:
> - Added changes to dri-devel mailing list. No code changes.
>
> --v4:
> - Instead of directly using operation mode, use an enum to accommodate
> all operation modes (Ankit).
>
> --v5:
> Nit-pick changes to commit message.
>
> Signed-off-by: Mitul Golani
> ---
> drivers/gpu/drm/display/drm_dp_helper.c | 12 +++
> .../drm/i915/display/intel_crtc_state_dump.c | 12 +++
> .../drm/i915/display/intel_display_types.h| 1 +
> include/drm/display/drm_dp.h | 9 +
> include/drm/display/drm_dp_helper.h | 33 +++
> 5 files changed, 67 insertions(+)
>
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> b/drivers/gpu/drm/display/drm_dp_helper.c
> index f94c04db7187..b1459ac92aea 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -2913,6 +2913,18 @@ void drm_dp_vsc_sdp_log(struct drm_printer *p, const
> struct drm_dp_vsc_sdp *vsc)
> }
> EXPORT_SYMBOL(drm_dp_vsc_sdp_log);
>
> +void drm_dp_as_sdp_log(struct drm_printer *p, const struct drm_dp_as_sdp
> *as_sdp)
> +{
> + drm_printf(p, "DP SDP: AS_SDP, revision %u, length %u\n",
> +as_sdp->revision, as_sdp->length);
> + drm_printf(p, "vtotal: %d\n", as_sdp->vtotal);
> + drm_printf(p, "target_rr: %d\n", as_sdp->target_rr);
> + drm_printf(p, "duration_incr_ms: %d\n", as_sdp->duration_incr_ms);
> + drm_printf(p, "duration_decr_ms: %d\n", as_sdp->duration_decr_ms);
> + drm_printf(p, "operation_mode: %d\n", as_sdp->mode);
> +}
> +EXPORT_SYMBOL(drm_dp_as_sdp_log);
> +
> /**
> * drm_dp_as_sdp_supported() - check if adaptive sync sdp is supported
> * @aux: DisplayPort AUX channel
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> index 4bcf446c75f4..26d77c2934e8 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> @@ -60,6 +60,15 @@ intel_dump_dp_vsc_sdp(struct drm_i915_private *i915,
> drm_dp_vsc_sdp_log(&p, vsc);
> }
>
> +static void
> +intel_dump_dp_as_sdp(struct drm_i915_private *i915,
> + const struct drm_dp_as_sdp *as_sdp)
> +{
> + struct drm_printer p = drm_dbg_printer(&i915->drm, DRM_UT_KMS,
> "AS_SDP");
> +
> + drm_dp_as_sdp_log(&p, as_sdp);
> +}
> +
> static void
> intel_dump_buffer(struct drm_i915_private *i915,
> const char *prefix, const u8 *buf, size_t len)
> @@ -299,6 +308,9 @@ void intel_crtc_state_dump(const struct intel_crtc_state
> *pipe_config,
> if (pipe_config->infoframes.enable &
> intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GAMUT_METADATA))
> intel_dump_infoframe(i915, &pipe_config->infoframes.drm);
> + if (pipe_config->infoframes.enable &
> + intel_hdmi_infoframe_enable(DP_SDP_ADAPTIVE_SYNC))
> + intel_dump_dp_as_sdp(i915, &pipe_config->infoframes.as_sdp);
> if (pipe_config->infoframes.enable &
> intel_hdmi_infoframe_enable(DP_SDP_VSC))
> intel_dump_dp_vsc_sdp(i915, &pipe_config->infoframes.vsc);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 8ce986fadd9a..1256730ea276 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1335,6 +1335,7 @@ struct intel_crtc_state {
> union hdmi_infoframe hdmi;
> union hdmi_infoframe drm;
> struct drm_dp_vsc_sdp vsc;
> + struct drm_dp_as_sdp as_sdp;
> } infoframes;
>
> u8 eld[MAX_ELD_BYTES];
> diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
> index 281afff6ee4e..0601b95d53db 100644
> --- a/include/drm/display/drm_dp.h
> +++ b/include/drm/display/drm_dp.h
> @@ -1578,10 +1578,12 @@ enum drm_dp_phy {
> #define DP_SDP_AUDIO_COPYMANAGEMENT 0x05 /* DP 1.2 */
> #define DP_SDP_ISRC 0x06 /* DP 1.2 */
> #define DP_SDP_VSC 0x07 /* DP 1.2 */
> +#define DP_SDP_ADAPTIVE_SYNC 0x22 /* DP 1.4 */
> #define DP_SDP_CAMERA_GENERIC(i) (0x08 + (i)) /* 0-7, DP 1.3 */
> #define DP_SDP_PPS 0x10 /* DP 1.4 */
> #define DP_SDP_VSC_EXT_VESA 0x20 /* DP 1.4 */
> #define DP_SDP_VSC_EXT_CEA 0x21 /* DP
Re: [RFC] drm/fourcc: Add RPI modifiers
On Wed, Feb 28, 2024 at 01:13:45PM +0200, Laurent Pinchart wrote: > On Wed, Feb 28, 2024 at 11:41:57AM +0100, Jacopo Mondi wrote: > > On Tue, Feb 27, 2024 at 03:08:27PM +0200, Laurent Pinchart wrote: > > > On Mon, Feb 26, 2024 at 04:46:24PM +0100, Daniel Vetter wrote: > > > > On Mon, 26 Feb 2024 at 16:39, Jacopo Mondi wrote: > > > > > > > > > > Add modifiers for the Raspberry Pi PiSP compressed formats. > > > > > > > > > > The compressed formats are documented at: > > > > > Documentation/userspace-api/media/v4l/pixfmt-pisp-comp-rggb.rst > > > > > > > > > > and in the PiSP datasheet: > > > > > https://datasheets.raspberrypi.com/camera/raspberry-pi-image-signal-processor-specification.pdf > > > > > > > > > > Signed-off-by: Jacopo Mondi > > > > > --- > > > > > > > > > > Background: > > > > > --- > > > > > > > > > > The Raspberry Pi PiSP camera subsystem is on its way to upstream > > > > > through the > > > > > Video4Linux2 subsystem: > > > > > https://patchwork.linuxtv.org/project/linux-media/list/?series=12310 > > > > > > > > > > The PiSP camera system is composed by a "Front End" and a "Back End". > > > > > The FrontEnd part is a MIPI CSI-2 receiver that store frames to > > > > > memory and > > > > > produce statistics, and the BackEnd is a memory-to-memory ISP that > > > > > converts > > > > > images in a format usable by application. > > > > > > > > > > The "FrontEnd" is capable of encoding RAW Bayer images as received by > > > > > the > > > > > image sensor in a 'compressed' format defined by Raspberry Pi and > > > > > fully > > > > > documented in the PiSP manual: > > > > > https://datasheets.raspberrypi.com/camera/raspberry-pi-image-signal-processor-specification.pdf > > > > > > > > > > The compression scheme is documented in the in-review patch series > > > > > for the BE > > > > > support at: > > > > > https://patchwork.linuxtv.org/project/linux-media/patch/20240223163012.300763-7-jacopo.mo...@ideasonboard.com/ > > > > > > > > > > The "BackEnd" is capable of consuming images in the compressed format > > > > > and > > > > > optionally user application might want to inspect those images for > > > > > debugging > > > > > purposes. > > > > > > > > > > Why a DRM modifier > > > > > -- > > > > > > > > > > The PiSP support is entirely implemented in libcamera, with the > > > > > support of an > > > > > hw-specific library called 'libpisp'. > > > > > > > > > > libcamera uses the fourcc codes defined by DRM to define its formats: > > > > > https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/formats.yaml > > > > > > > > > > And to define a new libcamera format for the Raspberry Pi compressed > > > > > ones we > > > > > need to associate the above proposed modifiers with a RAW Bayer format > > > > > identifier. > > > > > > > > > > In example: > > > > > > > > > > - RGGB16_PISP_COMP1: > > > > > fourcc: DRM_FORMAT_SRGGB16 > > > > > > An "interesting" issue here is that these formats currently live in > > > libcamera only, we haven't merged them in DRM "yet". This may be a > > > prerequisite ? > > > > > > > Ah right! I didn't notice! > > > > I think there are two issues at play here, one to be clarified by the > > DRM maintainers, the other more technically involved with the > > definition of the Bayer formats themselves. > > > > - Does DRM want RAW Bayer formats to be listed here, as these are not > > typically 'graphic' formats. What's the DRM maintainers opinion here ? > > To give some context, the "historical mistake" I keep referring to > regarding V4L2 is the decision to combine the bit depth of raw formats > with the colour filter array (a.k.a. Bayer) pattern into a fourcc. I > think we should have defined raw pixel formats that only encode a bit > depth, and conveyed the CFA pattern out-of-band. This is especially the > case for media bus codes (formats on the buses between hardware > devices). The reasoning here is that most devices only care about the > bit depth and not the CFA pattern. Adding a new CFA pattern (for > instance for RGB+Ir) for a camera sensor would currently require adding > a new media bus code (easy), using it in the sensor driver (obvious), > and patching *every* CSI-2 receiver driver to pass it through. Userspace > would similarly need to grow support for the new format, even for > userspace code that only cares about capturing the raw data without > processing the colour components. Having raw pixel formats that don't > convey the CFA pattern would simplify all this (and it's something I'm > considering adding to the media bus codes). > part is lots of churn So both drm fourcc and modifer formats are meant to be entirely opaque and complete. Which means if you take them from one driver (whether userspace or kernel driver shouldn't matter) and pass it to another, together with the height, width (in pixels) and the pitch (in linearized bytes, people got confused and wanted to do tile row pitch here and create and absolut
Re: [RFC PATCH v4 00/42] Color Pipeline API w/ VKMS
On Mon, Feb 26, 2024 at 04:10:14PM -0500, Harry Wentland wrote: > This is an RFC set for a color pipeline API, along with a sample > implementation in VKMS. All the key API bits are here. VKMS now > supports two named transfer function colorops and two matrix > colorops. We have IGT tests that check all four of these colorops > with a pixel-by-pixel comparison that checks that these colorops > do what we expect them to do with a +/- 1 8 bpc code point margin. So vkms is definitely great to make sure the igts are generic enough and somewhat useful, but ... does steam run on vkms too? I think that would be a really good test to show that the api we have here is actually useful for compositors in a cross-driver way, and not just a neat idea that doesn't survive harsh reality. And yes I realize that's probably going to be a bunch of work, but I feel like the color pipeline discussion has dragged around enough in hypotheticals and concerns that I think it would really help a lot. Thoughts? -Sima > > The big new change with v4 is the addition of an amdgpu color > pipeline, for all AMD GPUs with DCN 3 and newer. Amdgpu now support > the following: > > 1. 1D Curve EOTF > 2. 3x4 CTM > 3. Multiplier > 4. 1D Curve Inverse EOTF > 5. 1D LUT > 6. 1D Curve EOTF > 7. 1D LUT > > The supported curves for the 1D Curve type are: > - sRGB EOTF and its inverse > - PQ EOTF, scaled to [0.0, 125.0] and its inverse > - BT.2020/BT.709 OETF and its inverse > > Note that the 1st and 5th colorops take the EOTF or Inverse > OETF while the 3rd colorop takes the Inverse EOTF or OETF. > > We are working on two more ops for amdgpu, the HDR multiplier > and the 3DLUT, which will give us this: > > 1. 1D Curve EOTF > 2. 3x4 CTM > 3. HDR Multiplier > 4. 1D Curve Inverse EOTF > 5. 1D LUT > 6. 3D LUT > 7. 1D Curve EOTF > 8. 1D LUT > > This, essentially mirrors the color pipeline used by gamescope > and presented by Melissa Wen, with the exception of the DEGAM > LUT, which is not currently used. See > [1] > https://indico.freedesktop.org/event/4/contributions/186/attachments/138/218/xdc2023-TheRainbowTreasureMap-MelissaWen.pdf > > After this we'd like to also add the following ops: > - Scaler (Informational only) > - Color Encoding, to replace drm_plane's COLOR_ENCODING > - Color Range, to replace drm_plane's COLOR_RANGE > > This patchset is grouped as follows: > - Patches 1-3: couple general patches/fixes > - Patches 4-7: introduce kunit to VKMS > - Patch 7: description of motivation and details behind the > Color Pipeline API. If you're reading nothing else > but are interested in the topic I highly recommend > you take a look at this. > - Patches 7-27: DRM core and VKMS changes for color pipeline API > - Patches 28-40: DRM core and amdgpu changes for color pipeline API > > VKMS patches could still be improved in a few ways, though the > payoff might be limited and I would rather focus on other work > at the moment. The most obvious thing to improve would be to > eliminate the hard-coded LUTs for identity, and sRGB, and replace > them with fixed-point math instead. > > There are plenty of things that I would like to see here but > haven't had a chance to look at. These will (hopefully) be > addressed in future iterations, either in VKMS or amdgpu: > - Clear documentation for each drm_colorop_type > - Add custom LUT colorops to VKMS > - Add pre-blending 3DLUT > - How to support HW which can't bypass entire pipeline? > - Add ability to create colorops that don't have BYPASS > - Can we do a LOAD / COMMIT model for LUTs (and other properties)? > - read-only scaling colorop which defines scaling taps and position > - read-only color format colorop to define supported color formats >for a pipeline > - named matrices, for things like converting YUV to RGB > > IGT tests can be found at > https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/merge_requests/1 > > IGT patches are also being sent to the igt-dev mailing list. > > If you prefer a gitlab MR for review you can find it at > https://gitlab.freedesktop.org/hwentland/linux/-/merge_requests/5 > > v4: > - Add amdgpu color pipeline (WIP) > - Don't block setting of deprecated properties, instead pass client cap >to atomic check so drivers can ignore these props > - Drop IOCTL definitions (Pekka) > - Use enum property for colorop TYPE (Pekka) > - A few cleanups to the docs (Pekka) > - Rework the TYPE enum to name relation to avoid code duplication (Pekka) > - Add missing function declarations (Chaitanya Kumar Borah) > - Allow setting of NEXT property to NULL in _set_ function (Chaitanya Kumar > Borah) > - Add helper for creation of pipeline drm_plane property (Pekka) > - Always create Bypass pipeline (Pekka) > - A bunch of changes to VKMS kunit tests (Pekka) > - Fix index in CTM doc (Pekka) > > v3: > - Abandon IOCTLs and discover colorops as clients iterate the pipeline > - Remove need for libdrm > - Add co
Re: [PATCH RFC 0/4] Support for Simulated Panels
On Wed, Feb 28, 2024 at 01:49:34PM -0800, Jessica Zhang wrote: > > > On 2/2/2024 2:15 AM, Maxime Ripard wrote: > > On Tue, Jan 30, 2024 at 09:53:13AM +0100, Daniel Vetter wrote: > > > > > > > > Wouldn't it be simpler if we had a vkms-like panel that we > > > > > > > > could either > > > > > > > > configure from DT or from debugfs that would just be registered > > > > > > > > the > > > > > > > > usual way and would be the only panel we register? > > > > > > > > > > > > > > > > > > > No, we need to have validate actual hardware pipeline with the > > > > > > simulated > > > > > > panel. With vkms, actual display pipeline will not be validated. > > > > > > With > > > > > > incorrect display pipeline misconfigurations arising from different > > > > > > panel > > > > > > combinations, this can easily be caught with any existing IGT CRC > > > > > > testing. > > > > > > In addition, all performance related bugs can also be easily caught > > > > > > by > > > > > > simulating high resolution displays. > > > > > > > > > > That's not what I meant. What I meant was that something like a > > > > > user-configurable, generic, panel driver would be a good idea. Just > > > > > like > > > > > vkms (with the debugfs patches) is for a full blown KMS device. > > > > > > > > > > > > > Let me respond for both this question and the one below from you/Jani. > > > > > > > > Certainly having user-configurable information is a goal here. The > > > > end-goal > > > > is to make everything there in the existing panels such as below like I > > > > wrote: > > > > > > > > 1) Display resolution with timings (drm_display_mode) > > > > 2) Compression/non-compression > > > > 3) Command mode/Video mode > > > > 4) MIPI mode flags > > > > 5) DCS commands for panel enable/disable and other panel sequences > > > > 6) Power-up/Power-down sequence for the panel > > > > > > > > But, we also have to see what all is feasible today from the DRM fwk > > > > standpoint. There are some limitations about what is boot-time > > > > configurable > > > > using bootparams and what is runtime configurable (across a modeset) > > > > using > > > > debugfs. > > > > > > > > 1) Today, everything part of struct mipi_dsi_device needs to be > > > > available at > > > > boot time from what I can see as we need that while calling > > > > mipi_dsi_attach(). So for that we went with boot-params. > > > > > > > > 2) For the list of modes, we can move this to a debugfs like > > > > "populate_modes" which the client using a sim panel can call before > > > > picking > > > > a mode and triggering a commit. > > > > > > > > But we need to have some default mode and configuration. > > > > > > Uh, at the risk of sounding a bit like I'm just chasing the latest > > > buzzwords, but this sounds like something that's screaming for ebpf. > > > > I make a half-joke to Jani on IRC about it, but I was also being > > half-serious. If the goal we want to have is to fully emulate any panel > > variation, ebpf really looks like the best and most flexible way > > forward. > > Hi Maxime and Daniel, > > For our current sim panel requirements, we can go with implementing the > configfs first then add ebpf if requirements get more complex. Agreed, this is definitely the pragmatic approach to get this going. -Sima -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 1/1] drm/bridge: ti-sn65dsi83: Fix enable error path
On 29.02.24 10:47, Luca Ceresoli wrote:
> Hello Alexander,
>
> On Wed, 28 Feb 2024 09:15:46 +0100
> Alexander Stein wrote:
>
>
> [...]
>
>> Oh I mistook this DSI-LVDS bridge with the DSI-DP bridge on a different
>> board, my bad. I hope I can provide some insights. My platform is
>> imx8mm-tqma8mqml-mba8mx-lvds-tm070jvhg33.dtb.
>> I can easily cause a PLL lock failure by reducing the delay for the
>> enable-gpios 'gpio_delays'. This will result in a PLL lock faiure.
>> On my platform the vcc-supply counters do look sane:
>>> /sys/kernel/debug/regulator/SN65DSI83_1V8/open_count:1
>>> /sys/kernel/debug/regulator/SN65DSI83_1V8/use_count:0
>
> Interesting. Thanks for taking time to report your initial issue!
>
>> Once I remove the ti_sn65dsi83 module, the open_count decrements to 0 as
>> well. Looks sane to me.
>>
>> If I revert commit c81cd8f7c774 ("Revert "drm/bridge: ti-sn65dsi83:
>> Fix enable error path""), vcc-supply counters are:
>>> /sys/kernel/debug/regulator/SN65DSI83_1V8/open_count:1
>>> /sys/kernel/debug/regulator/SN65DSI83_1V8/use_count:1
>>
>> So in my case the use_count does not decrease! If I remove the module
>> ti_sn65dsi83, I get the WARN_ON (enable_count is still non-zero):
>>> WARNING: CPU: 2 PID: 402 at drivers/regulator/core.c:2398
>>> _regulator_put+0x15c/0x164
>>
>> This is on 6.8.0-rc6-next-20240228 with the following diff applied:
>> --->8---
>> diff --git a/arch/arm64/boot/dts/freescale/mba8mx.dtsi
>> b/arch/arm64/boot/dts/freescale/mba8mx.dtsi
>> index 427467df42bf..8461e1fd396f 100644
>> --- a/arch/arm64/boot/dts/freescale/mba8mx.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/mba8mx.dtsi
>> @@ -285,7 +285,7 @@ &i2c3 {
>> dsi_lvds_bridge: bridge@2d {
>> compatible = "ti,sn65dsi84";
>> reg = <0x2d>;
>> - enable-gpios = <&gpio_delays 0 13 0>;
>> + enable-gpios = <&gpio_delays 0 0 0>;
>> vcc-supply = <®_sn65dsi83_1v8>;
>> status = "disabled";
>>
>> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
>> b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
>> index 4814b7b6d1fd..57a7ed13f996 100644
>> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
>> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
>> @@ -478,7 +478,6 @@ static void sn65dsi83_atomic_pre_enable(struct
>> drm_bridge *bridge,
>> dev_err(ctx->dev, "failed to lock PLL, ret=%i\n", ret);
>> /* On failure, disable PLL again and exit. */
>> regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
>> - regulator_disable(ctx->vcc);
>> return;
>> }
>> --->8---
>>
>> So my patch indeed did fix an actual problem. On the other hand it seems
>> sn65dsi83_atomic_disable is not called in my case for some reason.
>
> So you remove the module and atomic_disable is not called, after
> having called atomic_pre_enable?
>
> I'm very possibly missing something, but this looks like a bug in the
> DRM bridge code at first sight.
I'm just guessing, but could it be that this patch [1] would fix it?
It looks like nobody cared to pick this up, although there are several
reports for defects caused by [2] and this patch is supposed to fix them.
[1] https://patchwork.freedesktop.org/patch/529288/
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=4fb912e5e19075874379cfcf074d90bd51ebf8ea
Re: [PATCH v3 1/3] bits: introduce fixed-type genmasks
On Wed, Feb 28, 2024 at 05:39:21PM -0600, Lucas De Marchi wrote: > On Thu, Feb 22, 2024 at 06:49:59AM -0800, Yury Norov wrote: > > On Wed, Feb 21, 2024 at 03:59:06PM -0600, Lucas De Marchi wrote: > > > On Wed, Feb 21, 2024 at 11:04:22PM +0200, Andy Shevchenko wrote: > > > > On Wed, Feb 21, 2024 at 10:30:02PM +0200, Dmitry Baryshkov wrote: > > > > > On Thu, 8 Feb 2024 at 09:45, Lucas De Marchi > > > > > wrote: ... > I build-tested this in x86-64, x86-32 and arm64. I didn't like much the > need to fork the __GENMASK() implementation on the 2 sides of the ifdef > since I think the GENMASK_INPUT_CHECK() should be the one covering the > input checks. However to make it common we'd need to solve 2 problems: > the casts and the sizeof. The sizeof can be passed as arg to > __GENMASK(), however the casts I think would need a __CAST_U8(x) > or the like and sprinkle it everywhere, which would hurt readability. > Not pretty. Or go back to the original submission and make it less > horrible :-/ I'm wondering if we can use _Generic() approach here. ... > > #define GENMASK_INPUT_CHECK(h, l) 0 > > +#define __GENMASK(t, h, l) \ > > + ((~0 - (1 << (l)) + 1) & (~0 >> (BITS_PER_LONG - 1 - (h > > humn... this builds, but does it work if GENMASK_ULL() is used in > assembly? That BITS_PER_LONG does not match the type width. UL()/ULL() macros are not just for fun. -- With Best Regards, Andy Shevchenko
Re: [RFC] drm/fourcc: Add RPI modifiers
On Thu, Feb 29, 2024 at 11:39:24AM +0100, Daniel Vetter wrote: > On Wed, Feb 28, 2024 at 01:13:45PM +0200, Laurent Pinchart wrote: > > On Wed, Feb 28, 2024 at 11:41:57AM +0100, Jacopo Mondi wrote: > > > On Tue, Feb 27, 2024 at 03:08:27PM +0200, Laurent Pinchart wrote: > > > > On Mon, Feb 26, 2024 at 04:46:24PM +0100, Daniel Vetter wrote: > > > > > On Mon, 26 Feb 2024 at 16:39, Jacopo Mondi wrote: > > > > > > > > > > > > Add modifiers for the Raspberry Pi PiSP compressed formats. > > > > > > > > > > > > The compressed formats are documented at: > > > > > > Documentation/userspace-api/media/v4l/pixfmt-pisp-comp-rggb.rst > > > > > > > > > > > > and in the PiSP datasheet: > > > > > > https://datasheets.raspberrypi.com/camera/raspberry-pi-image-signal-processor-specification.pdf > > > > > > > > > > > > Signed-off-by: Jacopo Mondi > > > > > > --- > > > > > > > > > > > > Background: > > > > > > --- > > > > > > > > > > > > The Raspberry Pi PiSP camera subsystem is on its way to upstream > > > > > > through the > > > > > > Video4Linux2 subsystem: > > > > > > https://patchwork.linuxtv.org/project/linux-media/list/?series=12310 > > > > > > > > > > > > The PiSP camera system is composed by a "Front End" and a "Back > > > > > > End". > > > > > > The FrontEnd part is a MIPI CSI-2 receiver that store frames to > > > > > > memory and > > > > > > produce statistics, and the BackEnd is a memory-to-memory ISP that > > > > > > converts > > > > > > images in a format usable by application. > > > > > > > > > > > > The "FrontEnd" is capable of encoding RAW Bayer images as received > > > > > > by the > > > > > > image sensor in a 'compressed' format defined by Raspberry Pi and > > > > > > fully > > > > > > documented in the PiSP manual: > > > > > > https://datasheets.raspberrypi.com/camera/raspberry-pi-image-signal-processor-specification.pdf > > > > > > > > > > > > The compression scheme is documented in the in-review patch series > > > > > > for the BE > > > > > > support at: > > > > > > https://patchwork.linuxtv.org/project/linux-media/patch/20240223163012.300763-7-jacopo.mo...@ideasonboard.com/ > > > > > > > > > > > > The "BackEnd" is capable of consuming images in the compressed > > > > > > format and > > > > > > optionally user application might want to inspect those images for > > > > > > debugging > > > > > > purposes. > > > > > > > > > > > > Why a DRM modifier > > > > > > -- > > > > > > > > > > > > The PiSP support is entirely implemented in libcamera, with the > > > > > > support of an > > > > > > hw-specific library called 'libpisp'. > > > > > > > > > > > > libcamera uses the fourcc codes defined by DRM to define its > > > > > > formats: > > > > > > https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/formats.yaml > > > > > > > > > > > > And to define a new libcamera format for the Raspberry Pi > > > > > > compressed ones we > > > > > > need to associate the above proposed modifiers with a RAW Bayer > > > > > > format > > > > > > identifier. > > > > > > > > > > > > In example: > > > > > > > > > > > > - RGGB16_PISP_COMP1: > > > > > > fourcc: DRM_FORMAT_SRGGB16 > > > > > > > > An "interesting" issue here is that these formats currently live in > > > > libcamera only, we haven't merged them in DRM "yet". This may be a > > > > prerequisite ? > > > > > > > > > > Ah right! I didn't notice! > > > > > > I think there are two issues at play here, one to be clarified by the > > > DRM maintainers, the other more technically involved with the > > > definition of the Bayer formats themselves. > > > > > > - Does DRM want RAW Bayer formats to be listed here, as these are not > > > typically 'graphic' formats. What's the DRM maintainers opinion here ? > > > > To give some context, the "historical mistake" I keep referring to > > regarding V4L2 is the decision to combine the bit depth of raw formats > > with the colour filter array (a.k.a. Bayer) pattern into a fourcc. I > > think we should have defined raw pixel formats that only encode a bit > > depth, and conveyed the CFA pattern out-of-band. This is especially the > > case for media bus codes (formats on the buses between hardware > > devices). The reasoning here is that most devices only care about the > > bit depth and not the CFA pattern. Adding a new CFA pattern (for > > instance for RGB+Ir) for a camera sensor would currently require adding > > a new media bus code (easy), using it in the sensor driver (obvious), > > and patching *every* CSI-2 receiver driver to pass it through. Userspace > > would similarly need to grow support for the new format, even for > > userspace code that only cares about capturing the raw data without > > processing the colour components. Having raw pixel formats that don't > > convey the CFA pattern would simplify all this (and it's something I'm > > considering adding to the media bus codes). > > part is lots of churn > > So both drm fourcc and modifer formats are meant to be enti
[PATCH 1/2] drm/buddy: stop using PAGE_SIZE
The drm_buddy minimum page-size requirements should be distinct from the
CPU PAGE_SIZE. Only restriction is that the minimum page-size is at
least 4K.
Signed-off-by: Matthew Auld
Cc: Arunpravin Paneer Selvam
Cc: Christian König
Cc: Arnd Bergmann
---
drivers/gpu/drm/drm_buddy.c | 2 +-
include/drm/drm_buddy.h | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index 5ebdd6f8f36e..f999568d69c1 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -102,7 +102,7 @@ int drm_buddy_init(struct drm_buddy *mm, u64 size, u64
chunk_size)
if (size < chunk_size)
return -EINVAL;
- if (chunk_size < PAGE_SIZE)
+ if (chunk_size < SZ_4K)
return -EINVAL;
if (!is_power_of_2(chunk_size))
diff --git a/include/drm/drm_buddy.h b/include/drm/drm_buddy.h
index a5b39fc01003..19ed661a32f3 100644
--- a/include/drm/drm_buddy.h
+++ b/include/drm/drm_buddy.h
@@ -53,8 +53,8 @@ struct drm_buddy_block {
struct list_head tmp_link;
};
-/* Order-zero must be at least PAGE_SIZE */
-#define DRM_BUDDY_MAX_ORDER (63 - PAGE_SHIFT)
+/* Order-zero must be at least SZ_4K */
+#define DRM_BUDDY_MAX_ORDER (63 - 12)
/*
* Binary Buddy System.
@@ -82,7 +82,7 @@ struct drm_buddy {
unsigned int n_roots;
unsigned int max_order;
- /* Must be at least PAGE_SIZE */
+ /* Must be at least SZ_4K */
u64 chunk_size;
u64 size;
u64 avail;
--
2.43.2
[PATCH 2/2] drm/tests/buddy: stop using PAGE_SIZE
Gives the wrong impression that min page-size has to be tied to the CPU
PAGE_SIZE.
Signed-off-by: Matthew Auld
Cc: Arunpravin Paneer Selvam
Cc: Christian König
Cc: Arnd Bergmann
---
drivers/gpu/drm/tests/drm_buddy_test.c | 42 +-
1 file changed, 21 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c
b/drivers/gpu/drm/tests/drm_buddy_test.c
index be2d9d7764be..8528f39a84e6 100644
--- a/drivers/gpu/drm/tests/drm_buddy_test.c
+++ b/drivers/gpu/drm/tests/drm_buddy_test.c
@@ -329,8 +329,8 @@ static void drm_test_buddy_alloc_pathological(struct kunit
*test)
* Eventually we will have a fully 50% fragmented mm.
*/
- mm_size = PAGE_SIZE << max_order;
- KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_init(&mm, mm_size, PAGE_SIZE),
+ mm_size = SZ_4K << max_order;
+ KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_init(&mm, mm_size, SZ_4K),
"buddy_init failed\n");
KUNIT_EXPECT_EQ(test, mm.max_order, max_order);
@@ -344,7 +344,7 @@ static void drm_test_buddy_alloc_pathological(struct kunit
*test)
}
for (order = top; order--;) {
- size = get_size(order, PAGE_SIZE);
+ size = get_size(order, mm.chunk_size);
KUNIT_ASSERT_FALSE_MSG(test,
drm_buddy_alloc_blocks(&mm, start,
mm_size, size, size,
&tmp, flags),
@@ -358,7 +358,7 @@ static void drm_test_buddy_alloc_pathological(struct kunit
*test)
}
/* There should be one final page for this sub-allocation */
- size = get_size(0, PAGE_SIZE);
+ size = get_size(0, mm.chunk_size);
KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, start,
mm_size,
size, size,
&tmp, flags),
"buddy_alloc hit
-ENOMEM for hole\n");
@@ -368,7 +368,7 @@ static void drm_test_buddy_alloc_pathological(struct kunit
*test)
list_move_tail(&block->link, &holes);
- size = get_size(top, PAGE_SIZE);
+ size = get_size(top, mm.chunk_size);
KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(&mm, start,
mm_size,
size, size,
&tmp, flags),
"buddy_alloc
unexpectedly succeeded at top-order %d/%d, it should be full!",
@@ -379,7 +379,7 @@ static void drm_test_buddy_alloc_pathological(struct kunit
*test)
/* Nothing larger than blocks of chunk_size now available */
for (order = 1; order <= max_order; order++) {
- size = get_size(order, PAGE_SIZE);
+ size = get_size(order, mm.chunk_size);
KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(&mm, start,
mm_size,
size, size,
&tmp, flags),
"buddy_alloc
unexpectedly succeeded at order %d, it should be full!",
@@ -408,14 +408,14 @@ static void drm_test_buddy_alloc_pessimistic(struct kunit
*test)
* page left.
*/
- mm_size = PAGE_SIZE << max_order;
- KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_init(&mm, mm_size, PAGE_SIZE),
+ mm_size = SZ_4K << max_order;
+ KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_init(&mm, mm_size, SZ_4K),
"buddy_init failed\n");
KUNIT_EXPECT_EQ(test, mm.max_order, max_order);
for (order = 0; order < max_order; order++) {
- size = get_size(order, PAGE_SIZE);
+ size = get_size(order, mm.chunk_size);
KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, start,
mm_size,
size, size,
&tmp, flags),
"buddy_alloc hit
-ENOMEM with order=%d\n",
@@ -428,7 +428,7 @@ static void drm_test_buddy_alloc_pessimistic(struct kunit
*test)
}
/* And now the last remaining block available */
- size = get_size(0, PAGE_SIZE);
+ size = get_size(0, mm.chunk_size);
KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, start, mm_size,
size, size, &tmp,
flags),
"buddy_alloc hit -ENOMEM on
final alloc\n");
@@ -440,7 +440,7 @@ static void drm_test_buddy_alloc_pessimistic(struct kunit
*test)
/* Should be completely full! */
for (order = max_order; order--;) {
- size = get_size(order, PAG
Re: UAPI Re: [PATCH 1/3] drm: Add DRM_MODE_TV_MODE_MONOCHROME
On Wed, Feb 28, 2024 at 04:22:56PM +, Simon Ser wrote: > On Wednesday, February 28th, 2024 at 17:14, Maxime Ripard > wrote: > > > > I don't know what the rules were 8 years ago, but the current uAPI rules > > > are what they are, and a new enum entry is new uAPI. > > > > TBF, and even if the wayland compositors support is missing, this > > property is perfectly usable as it is with upstream, open-source code, > > through either the command-line or X.org, and it's documented. > > > > So it's fine by me from a UAPI requirement side. > > That is not a valid way to pass the uAPI requirements IMHO. Yes, one > can program any KMS property via modetest or xrandr. Does that mean that > none of the new uAPI need a "real" implementation anymore? Does that mean > that the massive patch adding a color pipeline uAPI doesn't need > user-space anymore? xrandr only supports properties on the connector, so it's right out for the color pipeline. Also "we use xrandr for color properties" very much doesn't pass the bs filter of "is it a toy". My take would be that this escape hatch is also not valid for all connector property, stuff that is clearly meant to be configured automatically by the compositors cannot use the "we use xrandr" excuse, because users can't type fast enough and hit precisely enough to update a property in lockstep with the compositor's redraw loop :-) > The only thing I'm saying is that this breaks the usual DRM requirements. > If, as a maintainer, you're fine with breaking the rules and have a good > motivation to do so, that's fine by me. Rules are meant to be broken from > time to time depending on the situation. But please don't pretend that > modetest/xrandr is valid user-space to pass the rules. I think it bends it pretty badly, because people running native Xorg are slowly going away, and the modetest hack does not clear the bar for "is it a joke/test/demo hack" for me. I think some weston (or whatever compositor you like) config file support to set a bunch of "really only way to configure is by hand" output properties would clear the bar here for me. Because that is a feature I already mentioned that xrandr _does_ have, and which modetest hackery very much does not. -Sima -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v12 5/8] drm/i915/display: Compute AS SDP parameters.
On 2/28/2024 8:08 PM, Mitul Golani wrote:
Add necessary function definitions to compute AS SDP data.
The new intel_dp_compute_as_sdp function computes AS SDP
values based on the display configuration, ensuring proper
handling of Variable Refresh Rate (VRR).
--v2:
- Added DP_SDP_ADAPTIVE_SYNC to infoframe_type_to_idx(). [Ankit]
- Separated patch for intel_read/write_dp_sdp. [Ankit]
- _HSW_VIDEO_DIP_ASYNC_DATA_A should be from ADL onward. [Ankit]
- Fixed indentation issues. [Ankit]
--v3:
- Added VIDEO_DIP_ENABLE_AS_HSW flag to intel_dp_set_infoframes.
--v4:
- Added HAS_VRR check before writing AS SDP.
--v5:
Added missed HAS_VRR check before reading AS SDP.
--v6:
- Used Adaptive Sync sink status as a check for read/write SDP. (Ankit)
--v7:
- Remove as_sdp_enable from crtc_state.
- Add a comment mentioning current support of
DP_AS_SDP_AVT_FIXED_VTOTAL.
- Add state checker for AS_SDP infoframe enable.
Signed-off-by: Mitul Golani
---
drivers/gpu/drm/i915/display/intel_dp.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 7eb83924f3fe..1cd3cc0d0c0b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2626,6 +2626,30 @@ static void intel_dp_compute_vsc_colorimetry(const
struct intel_crtc_state *crtc
vsc->content_type = DP_CONTENT_TYPE_NOT_DEFINED;
}
+static void intel_dp_compute_as_sdp(struct intel_dp *intel_dp,
+ struct intel_crtc_state *crtc_state,
+ const struct drm_connector_state
*conn_state)
Drop conn_state, it is not used in the function.
Also remove fullstop in subject line.
With the above addressed, this is:
Reviewed-by: Ankit Nautiyal
+{
+ struct drm_dp_as_sdp *as_sdp = &crtc_state->infoframes.as_sdp;
+ struct intel_connector *connector = intel_dp->attached_connector;
+ const struct drm_display_mode *adjusted_mode =
+ &crtc_state->hw.adjusted_mode;
+ int vrefresh = drm_mode_vrefresh(adjusted_mode);
+
+ if (!intel_vrr_is_in_range(connector, vrefresh) ||
+ !intel_dp_as_sdp_supported(intel_dp))
+ return;
+
+ /* Currently only DP_AS_SDP_AVT_FIXED_VTOTAL mode supported */
+ as_sdp->sdp_type = DP_SDP_ADAPTIVE_SYNC;
+ as_sdp->length = 0x9;
+ as_sdp->mode = DP_AS_SDP_AVT_FIXED_VTOTAL;
+ as_sdp->vtotal = adjusted_mode->vtotal;
+ as_sdp->target_rr = 0;
+ as_sdp->duration_incr_ms = 0;
+ as_sdp->duration_incr_ms = 0;
+}
+
static void intel_dp_compute_vsc_sdp(struct intel_dp *intel_dp,
struct intel_crtc_state *crtc_state,
const struct drm_connector_state
*conn_state)
@@ -2951,6 +2975,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
g4x_dp_set_clock(encoder, pipe_config);
intel_vrr_compute_config(pipe_config, conn_state);
+ intel_dp_compute_as_sdp(intel_dp, pipe_config, conn_state);
intel_psr_compute_config(intel_dp, pipe_config, conn_state);
intel_dp_drrs_compute_config(connector, pipe_config, link_bpp_x16);
intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state);
Re: [PATCH 1/2] drm/buddy: stop using PAGE_SIZE
Reviewed-by: Arunpravin Paneer Selvam
On 2/29/2024 4:21 PM, Matthew Auld wrote:
The drm_buddy minimum page-size requirements should be distinct from the
CPU PAGE_SIZE. Only restriction is that the minimum page-size is at
least 4K.
Signed-off-by: Matthew Auld
Cc: Arunpravin Paneer Selvam
Cc: Christian König
Cc: Arnd Bergmann
---
drivers/gpu/drm/drm_buddy.c | 2 +-
include/drm/drm_buddy.h | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index 5ebdd6f8f36e..f999568d69c1 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -102,7 +102,7 @@ int drm_buddy_init(struct drm_buddy *mm, u64 size, u64
chunk_size)
if (size < chunk_size)
return -EINVAL;
- if (chunk_size < PAGE_SIZE)
+ if (chunk_size < SZ_4K)
return -EINVAL;
if (!is_power_of_2(chunk_size))
diff --git a/include/drm/drm_buddy.h b/include/drm/drm_buddy.h
index a5b39fc01003..19ed661a32f3 100644
--- a/include/drm/drm_buddy.h
+++ b/include/drm/drm_buddy.h
@@ -53,8 +53,8 @@ struct drm_buddy_block {
struct list_head tmp_link;
};
-/* Order-zero must be at least PAGE_SIZE */
-#define DRM_BUDDY_MAX_ORDER (63 - PAGE_SHIFT)
+/* Order-zero must be at least SZ_4K */
+#define DRM_BUDDY_MAX_ORDER (63 - 12)
/*
* Binary Buddy System.
@@ -82,7 +82,7 @@ struct drm_buddy {
unsigned int n_roots;
unsigned int max_order;
- /* Must be at least PAGE_SIZE */
+ /* Must be at least SZ_4K */
u64 chunk_size;
u64 size;
u64 avail;
Re: [PATCH 0/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing
On Thu, Feb 29, 2024 at 09:39:01AM +, Sakari Ailus wrote: > On Thu, Feb 29, 2024 at 01:07:25AM +0200, Laurent Pinchart wrote: > > > We have a Slack channel, #gitlab-ci, on the KernelCI Slack instance > > > https://kernelci.slack.com/ . > > > Feel free to join and contribute to the conversation. The KernelCI team > > > has > > > weekly calls where we also discuss the GitLab-CI pipeline. > > Could we communicate using free software please ? Furthermore, it's not > > possible to create an account on that slack instance unless you have an > > e-mail address affiliated with a small number of companies > > (https://kernelci.slack.com/signup#/domain-signup). That's a big no-go > > for me. > I agree. There is no shortage of good alternatives either: we have IRC, > Matrix and XMPP for instance. Just pick one of them. And indeed KernelCI does actually already have #kernelci on libera.chat. signature.asc Description: PGP signature
Re: [PATCH 2/2] drm/tests/buddy: stop using PAGE_SIZE
Reviewed-by: Arunpravin Paneer Selvam
On 2/29/2024 4:21 PM, Matthew Auld wrote:
Gives the wrong impression that min page-size has to be tied to the CPU
PAGE_SIZE.
Signed-off-by: Matthew Auld
Cc: Arunpravin Paneer Selvam
Cc: Christian König
Cc: Arnd Bergmann
---
drivers/gpu/drm/tests/drm_buddy_test.c | 42 +-
1 file changed, 21 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c
b/drivers/gpu/drm/tests/drm_buddy_test.c
index be2d9d7764be..8528f39a84e6 100644
--- a/drivers/gpu/drm/tests/drm_buddy_test.c
+++ b/drivers/gpu/drm/tests/drm_buddy_test.c
@@ -329,8 +329,8 @@ static void drm_test_buddy_alloc_pathological(struct kunit
*test)
* Eventually we will have a fully 50% fragmented mm.
*/
- mm_size = PAGE_SIZE << max_order;
- KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_init(&mm, mm_size, PAGE_SIZE),
+ mm_size = SZ_4K << max_order;
+ KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_init(&mm, mm_size, SZ_4K),
"buddy_init failed\n");
KUNIT_EXPECT_EQ(test, mm.max_order, max_order);
@@ -344,7 +344,7 @@ static void drm_test_buddy_alloc_pathological(struct kunit
*test)
}
for (order = top; order--;) {
- size = get_size(order, PAGE_SIZE);
+ size = get_size(order, mm.chunk_size);
KUNIT_ASSERT_FALSE_MSG(test,
drm_buddy_alloc_blocks(&mm, start,
mm_size, size, size,
&tmp, flags),
@@ -358,7 +358,7 @@ static void drm_test_buddy_alloc_pathological(struct kunit
*test)
}
/* There should be one final page for this sub-allocation */
- size = get_size(0, PAGE_SIZE);
+ size = get_size(0, mm.chunk_size);
KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, start,
mm_size,
size, size,
&tmp, flags),
"buddy_alloc hit -ENOMEM
for hole\n");
@@ -368,7 +368,7 @@ static void drm_test_buddy_alloc_pathological(struct kunit
*test)
list_move_tail(&block->link, &holes);
- size = get_size(top, PAGE_SIZE);
+ size = get_size(top, mm.chunk_size);
KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(&mm, start,
mm_size,
size, size,
&tmp, flags),
"buddy_alloc unexpectedly
succeeded at top-order %d/%d, it should be full!",
@@ -379,7 +379,7 @@ static void drm_test_buddy_alloc_pathological(struct kunit
*test)
/* Nothing larger than blocks of chunk_size now available */
for (order = 1; order <= max_order; order++) {
- size = get_size(order, PAGE_SIZE);
+ size = get_size(order, mm.chunk_size);
KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(&mm, start,
mm_size,
size, size,
&tmp, flags),
"buddy_alloc unexpectedly
succeeded at order %d, it should be full!",
@@ -408,14 +408,14 @@ static void drm_test_buddy_alloc_pessimistic(struct kunit
*test)
* page left.
*/
- mm_size = PAGE_SIZE << max_order;
- KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_init(&mm, mm_size, PAGE_SIZE),
+ mm_size = SZ_4K << max_order;
+ KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_init(&mm, mm_size, SZ_4K),
"buddy_init failed\n");
KUNIT_EXPECT_EQ(test, mm.max_order, max_order);
for (order = 0; order < max_order; order++) {
- size = get_size(order, PAGE_SIZE);
+ size = get_size(order, mm.chunk_size);
KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, start,
mm_size,
size, size,
&tmp, flags),
"buddy_alloc hit -ENOMEM
with order=%d\n",
@@ -428,7 +428,7 @@ static void drm_test_buddy_alloc_pessimistic(struct kunit
*test)
}
/* And now the last remaining block available */
- size = get_size(0, PAGE_SIZE);
+ size = get_size(0, mm.chunk_size);
KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, start, mm_size,
size, size, &tmp,
flags),
"buddy_alloc hit -ENOMEM on final
alloc\n");
@@ -440,7 +440,7 @@ static void drm_test_buddy_alloc_pessimistic(struct kunit
*test)
/* Should be completely full! */
for (order = max_order; order--;) {
- size = get_si
Re: [PATCH 1/1] drm/bridge: ti-sn65dsi83: Fix enable error path
Hi Luca,
Am Donnerstag, 29. Februar 2024, 10:47:23 CET schrieb Luca Ceresoli:
> Hello Alexander,
>
> On Wed, 28 Feb 2024 09:15:46 +0100
> Alexander Stein wrote:
>
>
> [...]
>
> > Oh I mistook this DSI-LVDS bridge with the DSI-DP bridge on a different
> > board, my bad. I hope I can provide some insights. My platform is
> > imx8mm-tqma8mqml-mba8mx-lvds-tm070jvhg33.dtb.
> > I can easily cause a PLL lock failure by reducing the delay for the
> > enable-gpios 'gpio_delays'. This will result in a PLL lock faiure.
> > On my platform the vcc-supply counters do look sane:
> > > /sys/kernel/debug/regulator/SN65DSI83_1V8/open_count:1
> > > /sys/kernel/debug/regulator/SN65DSI83_1V8/use_count:0
>
> Interesting. Thanks for taking time to report your initial issue!
>
> > Once I remove the ti_sn65dsi83 module, the open_count decrements to 0 as
> > well. Looks sane to me.
> >
> > If I revert commit c81cd8f7c774 ("Revert "drm/bridge: ti-sn65dsi83:
> > Fix enable error path""), vcc-supply counters are:
> > > /sys/kernel/debug/regulator/SN65DSI83_1V8/open_count:1
> > > /sys/kernel/debug/regulator/SN65DSI83_1V8/use_count:1
> >
> > So in my case the use_count does not decrease! If I remove the module
> > ti_sn65dsi83, I get the WARN_ON (enable_count is still non-zero):
> > > WARNING: CPU: 2 PID: 402 at drivers/regulator/core.c:2398
> > > _regulator_put+0x15c/0x164
> >
> > This is on 6.8.0-rc6-next-20240228 with the following diff applied:
> > --->8---
> > diff --git a/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> > b/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> > index 427467df42bf..8461e1fd396f 100644
> > --- a/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> > @@ -285,7 +285,7 @@ &i2c3 {
> > dsi_lvds_bridge: bridge@2d {
> > compatible = "ti,sn65dsi84";
> > reg = <0x2d>;
> > - enable-gpios = <&gpio_delays 0 13 0>;
> > + enable-gpios = <&gpio_delays 0 0 0>;
> > vcc-supply = <®_sn65dsi83_1v8>;
> > status = "disabled";
> >
> > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > index 4814b7b6d1fd..57a7ed13f996 100644
> > --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > @@ -478,7 +478,6 @@ static void sn65dsi83_atomic_pre_enable(struct
> > drm_bridge *bridge,
> > dev_err(ctx->dev, "failed to lock PLL, ret=%i\n", ret);
> > /* On failure, disable PLL again and exit. */
> > regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
> > - regulator_disable(ctx->vcc);
> > return;
> > }
> > --->8---
> >
> > So my patch indeed did fix an actual problem. On the other hand it seems
> > sn65dsi83_atomic_disable is not called in my case for some reason.
>
> So you remove the module and atomic_disable is not called, after
> having called atomic_pre_enable?
Yes, that's the case.
> > I'm very possibly missing something, but this looks like a bug in the
> > DRM bridge code at first sight.
Yes, that seemed fishy for me as well. I should not be able to remove the
bridge driver while the pipeline is running. Weston is still running, but
nevertheless I can remove the bridge driver.
Best regards,
Alexander
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/
Re: [PATCH v8 5/8] drm/simpledrm: Add drm_panic support
On Tue, Feb 27, 2024 at 11:04:16AM +0100, Jocelyn Falempe wrote:
> Add support for the drm_panic module, which displays a user-friendly
> message to the screen when a kernel panic occurs.
>
> v8:
> * Replace get_scanout_buffer() with drm_panic_set_buffer()
>(Thomas Zimmermann)
>
> Signed-off-by: Jocelyn Falempe
> ---
> drivers/gpu/drm/tiny/simpledrm.c | 17 +
> 1 file changed, 17 insertions(+)
>
> diff --git a/drivers/gpu/drm/tiny/simpledrm.c
> b/drivers/gpu/drm/tiny/simpledrm.c
> index 7ce1c4617675..a2190995354a 100644
> --- a/drivers/gpu/drm/tiny/simpledrm.c
> +++ b/drivers/gpu/drm/tiny/simpledrm.c
> @@ -25,6 +25,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #define DRIVER_NAME "simpledrm"
> @@ -735,6 +736,20 @@ static const struct drm_connector_funcs
> simpledrm_connector_funcs = {
> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> };
>
> +static void simpledrm_init_panic_buffer(struct drm_plane *plane)
> +{
> + struct simpledrm_device *sdev = simpledrm_device_of_dev(plane->dev);
> + struct drm_framebuffer fb;
> +
> + /* Fake framebuffer struct for drm_panic_set_buffer */
> + fb.width = sdev->mode.hdisplay;
> + fb.height = sdev->mode.vdisplay;
> + fb.format = sdev->format;
> + fb.pitches[0] = sdev->pitch;
> +
> + drm_panic_set_buffer(plane->panic_scanout, &fb, &sdev->screen_base);
> +}
> +
> static const struct drm_mode_config_funcs simpledrm_mode_config_funcs = {
> .fb_create = drm_gem_fb_create_with_dirty,
> .atomic_check = drm_atomic_helper_check,
> @@ -945,6 +960,8 @@ static struct simpledrm_device
> *simpledrm_device_create(struct drm_driver *drv,
> return ERR_PTR(ret);
> drm_plane_helper_add(primary_plane,
> &simpledrm_primary_plane_helper_funcs);
> drm_plane_enable_fb_damage_clips(primary_plane);
> + drm_panic_register(primary_plane);
Just a quick comment on this:
This does not work, the driver is not ready to handle panic calls at this
stage. Instead we need to automatically register all planes that support
panic handling in drm_dev_register(), and we need to remove them all again
in drm_dev_unregister(). Outside of these functions it is not safe to call
into driver code.
At that point it might be simpler to only register one panic notifier per
drm_device, and push the loop into the panic handler again.
Cheers, Sima
> + simpledrm_init_panic_buffer(primary_plane);
>
> /* CRTC */
>
> --
> 2.43.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH 0/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing
On Thu, Feb 29, 2024 at 01:10:16PM +0200, Nikolai Kondrashov wrote: > On 2/29/24 11:34 AM, Laurent Pinchart wrote: > > On Thu, Feb 29, 2024 at 11:26:51AM +0200, Nikolai Kondrashov wrote: > >> On 2/29/24 01:07, Laurent Pinchart wrote: > >>> On Wed, Feb 28, 2024 at 07:55:24PM -0300, Helen Koike wrote: > **Join Our Slack Channel:** > We have a Slack channel, #gitlab-ci, on the KernelCI Slack instance > https://kernelci.slack.com/ . > Feel free to join and contribute to the conversation. The KernelCI team > has > weekly calls where we also discuss the GitLab-CI pipeline. > >>> > >>> Could we communicate using free software please ? Furthermore, it's not > >>> possible to create an account on that slack instance unless you have an > >>> e-mail address affiliated with a small number of companies > >>> (https://kernelci.slack.com/signup#/domain-signup). That's a big no-go > >>> for me. > >> > >> Yes, it's not ideal that we use closed-source software for communication, > >> but > >> FWIW I'd be happy to invite you there. Perhaps if you try logging in e.g. > >> with > >> a Google account, I'd be able to see and approve your attempt too. > > > > I don't use Google accounts to authenticate to third-party services, > > sorry. And in any case, that won't make slack free software. > > Of course. You're also welcome to join the #kernelci channel on libera.chat. Isn't that a bit pointless if it's no the main IM channel ? > It's much quieter there, though. -- Regards, Laurent Pinchart
Re: [PATCH v8 3/8] drm/panic: Add debugfs entry to test without triggering panic.
On Tue, Feb 27, 2024 at 11:04:14AM +0100, Jocelyn Falempe wrote:
> Add a debugfs file, so you can test drm_panic without freezing
> your machine. This is unsafe, and should be enabled only for
> developer or tester.
>
> to display the drm_panic screen, just run:
> echo 1 > /sys/kernel/debug/drm_panic/trigger
>
> Signed-off-by: Jocelyn Falempe
> ---
> drivers/gpu/drm/Kconfig | 9 +++
> drivers/gpu/drm/drm_panic.c | 47 +
> 2 files changed, 56 insertions(+)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index c17d8a8f6877..8dcea29f595c 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -125,6 +125,15 @@ config DRM_PANIC_BACKGROUND_COLOR
> depends on DRM_PANIC
> default 0x00
>
> +config DRM_PANIC_DEBUG
> + bool "Add a debug fs entry to trigger drm_panic"
> + depends on DRM_PANIC && DEBUG_FS
> + help
> + Add drm_panic/trigger in the kernel debugfs, to force the panic
> + handler to write the panic message to the scanout buffer. This is
> + unsafe and should not be enabled on a production build.
> + If in doubt, say "N".
> +
> config DRM_DEBUG_DP_MST_TOPOLOGY_REFS
> bool "Enable refcount backtrace history in the DP MST helpers"
> depends on STACKTRACE_SUPPORT
> diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
> index c9f386476ef9..c5d3f725c5f5 100644
> --- a/drivers/gpu/drm/drm_panic.c
> +++ b/drivers/gpu/drm/drm_panic.c
> @@ -398,3 +398,50 @@ void drm_panic_unregister(struct drm_plane *plane)
> }
> EXPORT_SYMBOL(drm_panic_unregister);
>
> +
> +/*
> + * DEBUG, This is currently unsafe.
> + * Also it will call all panic_notifier, since there is no way to filter and
> + * only call the drm_panic notifier.
> + */
> +#ifdef CONFIG_DRM_PANIC_DEBUG
> +#include
> +
> +static struct dentry *debug_dir;
> +static struct dentry *debug_trigger;
> +
> +static ssize_t dbgfs_trigger_write(struct file *file, const char __user
> *user_buf,
> +size_t count, loff_t *ppos)
> +{
> + bool run;
> +
> + if (kstrtobool_from_user(user_buf, count, &run) == 0 && run)
> + atomic_notifier_call_chain(&panic_notifier_list, 0, "Test drm
> panic from debugfs");
Since this is just the general panic notifier it feels very misplaced in
the drm subsystem. I think moving that code into the core panic code makes
a lot more sense, then we'd also have all the right people on Cc: to
figure out how we can best recreate the correct calling context (like nmi
context or whatever) for best case simulation of panic code. John Ogness
definitely needs to see this and ack, wherever we put it.
-Sima
> + return count;
> +}
> +
> +static const struct file_operations dbg_drm_panic_ops = {
> + .owner = THIS_MODULE,
> + .write = dbgfs_trigger_write,
> +};
> +
> +static int __init debugfs_start(void)
> +{
> + debug_dir = debugfs_create_dir("drm_panic", NULL);
> +
> + if (IS_ERR(debug_dir))
> + return PTR_ERR(debug_dir);
> + debug_trigger = debugfs_create_file("trigger", 0200, debug_dir,
> + NULL, &dbg_drm_panic_ops);
> + return 0;
> +}
> +
> +static void __exit debugfs_end(void)
> +{
> + debugfs_remove_recursive(debug_dir);
> +}
> +
> +module_init(debugfs_start);
> +module_exit(debugfs_end);
> +
> +#endif
> --
> 2.43.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH v12 2/8] drm: Add Adaptive Sync SDP logging
On Thu, 29 Feb 2024, "Nautiyal, Ankit K" wrote:
> On 2/28/2024 8:08 PM, Mitul Golani wrote:
>> +enum operation_mode {
>> +DP_AS_SDP_AVT_DYNAMIC_VTOTAL = 0x00,
>> +DP_AS_SDP_AVT_FIXED_VTOTAL = 0x01,
>> +DP_AS_SDP_FAVT_TRR_NOT_REACHED = 0x02,
>> +DP_AS_SDP_FAVT_TRR_REACHED = 0x03
>> +};
>
> We can drop the initialization here.
For stuff that needs to match the spec it's common to include the
initializations instead of relying on the auto enumeration.
BR,
Jani.
--
Jani Nikula, Intel
Re: [PATCH v2 1/9] media: v4l2-subdev: Add a pad variant of .query_dv_timings()
On Thu, Feb 29, 2024 at 9:02 AM Hans Verkuil wrote: > > On 28/02/2024 16:34, Paweł Anikiel wrote: > > On Wed, Feb 28, 2024 at 12:25 PM Hans Verkuil > > wrote: > >> > >> Hi Paweł, > >> > >> On 21/02/2024 17:02, Paweł Anikiel wrote: > >>> Currently, .query_dv_timings() is defined as a video callback without > >>> a pad argument. This is a problem if the subdevice can have different > >>> dv timings for each pad (e.g. a DisplayPort receiver with multiple > >>> virtual channels). > >>> > >>> To solve this, add a pad variant of this callback which includes > >>> the pad number as an argument. > >> > >> So now we have two query_dv_timings ops: one for video ops, and one > >> for pad ops. That's not very maintainable. I would suggest switching > >> all current users of the video op over to the pad op. > > > > I agree it would be better if there was only one. However, I have some > > concerns: > > 1. Isn't there a problem with backwards compatibility? For example, an > > out-of-tree driver is likely to use this callback, which would break. > > I'm asking because I'm not familiar with how such API changes are > > handled. > > It's out of tree, so they will just have to adapt. That's how life is if > you are not part of the mainline kernel. > > > 2. If I do switch all current users to the pad op, I can't test those > > changes. Isn't that a problem? > > I can test one or two drivers, but in general I don't expect this to be > a problem. > > > 3. Would I need to get ACK from all the driver maintainers? > > CC the patches to the maintainers. Generally you will get back Acks from > some but not all maintainers, but that's OK. For changes affecting multiple > drivers you never reach 100% on that. I can review the remainder. The DV > Timings API is my expert area, so that shouldn't be a problem. > > A quick grep gives me these subdev drivers that implement it: > > adv748x, adv7604, adv7842, tc358743, tda1997x, tvp7002, gs1662. > > And these bridge drivers that call the subdevs: > > cobalt, rcar-vin, vpif_capture. > > The bridge drivers can use the following pad when calling query_dv_timings: > > cobalt: ADV76XX_PAD_HDMI_PORT_A > rcar_vin: vin->parallel.sink_pad > vpif_capture: 0 > > The converted subdev drivers should check if the pad is an input pad. > Ideally it should check if the pad is equal to the current input pad > since most devices can only query the timings for the currently selected > input pad. But some older drivers predate the pad concept and they > ignore the pad value. Thank you for the helpful info. I will switch all these drivers to the pad op, then. Would you like me to prepare a separate patchset, or should I include the changes in this one?
Re: [PATCH v2 1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first
Hi,
On 28.03.23 19:07, Jagan Teki wrote:
> For a given bridge pipeline if any bridge sets pre_enable_prev_first
> flag then the pre_enable for the previous bridge will be called before
> pre_enable of this bridge and opposite is done for post_disable.
>
> These are the potential bridge flags to alter bridge init order in order
> to satisfy the MIPI DSI host and downstream panel or bridge to function.
> However the existing pre_enable_prev_first logic with associated bridge
> ordering has broken for both pre_enable and post_disable calls.
>
> [pre_enable]
>
> The altered bridge ordering has failed if two consecutive bridges on a
> given pipeline enables the pre_enable_prev_first flag.
>
> Example:
> - Panel
> - Bridge 1
> - Bridge 2 pre_enable_prev_first
> - Bridge 3
> - Bridge 4 pre_enable_prev_first
> - Bridge 5 pre_enable_prev_first
> - Bridge 6
> - Encoder
>
> In this example, Bridge 4 and Bridge 5 have pre_enable_prev_first.
>
> The logic looks for a bridge which enabled pre_enable_prev_first flag
> on each iteration and assigned the previou bridge to limit pointer
> if the bridge doesn't enable pre_enable_prev_first flags.
>
> If control found Bridge 2 is pre_enable_prev_first then the iteration
> looks for Bridge 3 and found it is not pre_enable_prev_first and assigns
> it's previous Bridge 4 to limit pointer and calls pre_enable of Bridge 3
> and Bridge 2 and assign iter pointer with limit which is Bridge 4.
>
> Here is the actual problem, for the next iteration control look for
> Bridge 5 instead of Bridge 4 has iter pointer in previous iteration
> moved to Bridge 4 so this iteration skips the Bridge 4. The iteration
> found Bridge 6 doesn't pre_enable_prev_first flags so the limit assigned
> to Encoder. From next iteration Encoder skips as it is the last bridge
> for reverse order pipeline.
>
> So, the resulting pre_enable bridge order would be,
> - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5.
>
> This patch fixes this by assigning limit to next pointer instead of
> previous bridge since the iteration always looks for bridge that does
> NOT request prev so assigning next makes sure the last bridge on a
> given iteration what exactly the limit bridge is.
>
> So, the resulting pre_enable bridge order with fix would be,
> - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5, Bridge 4,
> Encoder.
>
> [post_disable]
>
> The altered bridge ordering has failed if two consecutive bridges on a
> given pipeline enables the pre_enable_prev_first flag.
>
> Example:
> - Panel
> - Bridge 1
> - Bridge 2 pre_enable_prev_first
> - Bridge 3
> - Bridge 4 pre_enable_prev_first
> - Bridge 5 pre_enable_prev_first
> - Bridge 6
> - Encoder
>
> In this example Bridge 5 and Bridge 4 have pre_enable_prev_first.
>
> The logic looks for a bridge which enabled pre_enable_prev_first flags
> on each iteration and assigned the previou bridge to next and next to
> limit pointer if the bridge does enable pre_enable_prev_first flag.
>
> If control starts from Bridge 6 then it found next Bridge 5 is
> pre_enable_prev_first and immediately the next assigned to previous
> Bridge 6 and limit assignments to next Bridge 6 and call post_enable
> of Bridge 6 even though the next consecutive Bridge 5 is enabled with
> pre_enable_prev_first. This clearly misses the logic to find the state
> of next conducive bridge as everytime the next and limit assigns
> previous bridge if given bridge enabled pre_enable_prev_first.
>
> So, the resulting post_disable bridge order would be,
> - Encoder, Bridge 6, Bridge 5, Bridge 4, Bridge 3, Bridge 2, Bridge 1,
> Panel.
>
> This patch fixes this by assigning next with previou bridge only if the
> bridge doesn't enable pre_enable_prev_first flag and the next further
> assign it to limit. This way we can find the bridge that NOT requested
> prev to disable last.
>
> So, the resulting pre_enable bridge order with fix would be,
> - Encoder, Bridge 4, Bridge 5, Bridge 6, Bridge 2, Bridge 3, Bridge 1,
> Panel.
>
> Validated the bridge init ordering by incorporating dummy bridges in
> the sun6i-mipi-dsi pipeline
>
> Fixes: 4fb912e5e190 ("drm/bridge: Introduce pre_enable_prev_first to
> alter bridge init order")
> Signed-off-by: Jagan Teki
This patch is now almost 1 year old and it has been tested and reviewed
and there have been multiple pings.
Is there anything missing? Why is it not applied yet?
Andrzej, Neil, Robert: As DRM bridge maintainers, can you take care of this?
Thanks
Frieder
Re: [PATCH 0/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing
On Thu, Feb 29, 2024 at 01:19:19PM +0200, Laurent Pinchart wrote: > On Thu, Feb 29, 2024 at 01:10:16PM +0200, Nikolai Kondrashov wrote: > > Of course. You're also welcome to join the #kernelci channel on libera.chat. > Isn't that a bit pointless if it's no the main IM channel ? It *was* the original channel and still gets some usage (mostly started by me admittedly since I've never joined slack for a bunch of reasons that make it hassle), IIRC the Slack was started because there were some interns who had trouble figuring out IRC and intermittent connectivity but people seem to have migrated. signature.asc Description: PGP signature
[drm-misc:drm-misc-next 4/5] drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c:191:17: error: implicit declaration of function 'drm_atomic_get_new_connector_state'; did you mean 'drm_atomic_helper_connector_rese
tree: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
head: be318d01a90366029e181068e8857c6252e1fadc
commit: 358e76fd613a762bdba18b6b9fb0469a481de3a3 [4/5] drm/sun4i: hdmi:
Consolidate atomic_check and mode_valid
config: xtensa-randconfig-002-20240229
(https://download.01.org/0day-ci/archive/20240229/202402291942.zvb1vx4y-...@intel.com/config)
compiler: xtensa-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20240229/202402291942.zvb1vx4y-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202402291942.zvb1vx4y-...@intel.com/
All error/warnings (new ones prefixed by >>):
drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c: In function
'sun4i_hdmi_connector_atomic_check':
>> drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c:191:17: error: implicit declaration
>> of function 'drm_atomic_get_new_connector_state'; did you mean
>> 'drm_atomic_helper_connector_reset'? [-Werror=implicit-function-declaration]
191 | drm_atomic_get_new_connector_state(state, connector);
| ^~
| drm_atomic_helper_connector_reset
>> drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c:191:17: warning: initialization of
>> 'struct drm_connector_state *' from 'int' makes pointer from integer without
>> a cast [-Wint-conversion]
cc1: some warnings being treated as errors
vim +191 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
186
187 static int sun4i_hdmi_connector_atomic_check(struct drm_connector
*connector,
188 struct drm_atomic_state
*state)
189 {
190 struct drm_connector_state *conn_state =
> 191 drm_atomic_get_new_connector_state(state, connector);
192 struct drm_crtc *crtc = conn_state->crtc;
193 struct drm_crtc_state *crtc_state = crtc->state;
194 struct drm_display_mode *mode = &crtc_state->adjusted_mode;
195 enum drm_mode_status status;
196
197 status = sun4i_hdmi_connector_clock_valid(connector, mode,
198mode->clock * 1000);
199 if (status != MODE_OK)
200 return -EINVAL;
201
202 return 0;
203 }
204
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
[PATCH] fbdev/mb862xxfb: Fix defined but not used error
socrates_gc_mode is defined at the top-level but then only used inside
an #ifdef CONFIG_FB_MB862XX_LIME, leading to an error with some configs:
drivers/video/fbdev/mb862xx/mb862xxfbdrv.c:36:31: error: ‘socrates_gc_mode’
defined but not used
36 | static struct mb862xx_gc_mode socrates_gc_mode = {
Fix it by moving socrates_gc_mode inside that ifdef, immediately prior
to the only function where it's used.
Signed-off-by: Michael Ellerman
---
drivers/video/fbdev/mb862xx/mb862xxfbdrv.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
index 7c402e9fd7a9..baec312d7b33 100644
--- a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
+++ b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
@@ -32,15 +32,6 @@
#define CARMINE_MEM_SIZE 0x800
#define DRV_NAME "mb862xxfb"
-#if defined(CONFIG_SOCRATES)
-static struct mb862xx_gc_mode socrates_gc_mode = {
- /* Mode for Prime View PM070WL4 TFT LCD Panel */
- { "800x480", 45, 800, 480, 4, 86, 42, 33, 10, 128, 2, 0, 0, 0 },
- /* 16 bits/pixel, 16MB, 133MHz, SDRAM memory mode value */
- 16, 0x100, GC_CCF_COT_133, 0x4157ba63
-};
-#endif
-
/* Helpers */
static inline int h_total(struct fb_var_screeninfo *var)
{
@@ -666,6 +657,15 @@ static int mb862xx_gdc_init(struct mb862xxfb_par *par)
return 0;
}
+#if defined(CONFIG_SOCRATES)
+static struct mb862xx_gc_mode socrates_gc_mode = {
+ /* Mode for Prime View PM070WL4 TFT LCD Panel */
+ { "800x480", 45, 800, 480, 4, 86, 42, 33, 10, 128, 2, 0, 0, 0 },
+ /* 16 bits/pixel, 16MB, 133MHz, SDRAM memory mode value */
+ 16, 0x100, GC_CCF_COT_133, 0x4157ba63
+};
+#endif
+
static int of_platform_mb862xx_probe(struct platform_device *ofdev)
{
struct device_node *np = ofdev->dev.of_node;
--
2.43.2
Re: [PATCH v12 6/8] drm/i915/display: Add state checker for Adaptive Sync SDP
On 2/28/2024 8:08 PM, Mitul Golani wrote:
Enable infoframe and add state checker for Adaptive Sync
SDP enablement.
Signed-off-by: Mitul Golani
---
drivers/gpu/drm/i915/display/intel_display.c | 46
drivers/gpu/drm/i915/display/intel_dp.c | 2 +
2 files changed, 48 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 00ac65a14029..be0a5fae4e58 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4781,6 +4781,17 @@ intel_compare_dp_vsc_sdp(const struct drm_dp_vsc_sdp *a,
a->content_type == b->content_type;
}
+static bool
+intel_compare_dp_as_sdp(const struct drm_dp_as_sdp *a,
+ const struct drm_dp_as_sdp *b)
+{
+ return a->vtotal == b->vtotal &&
+ a->target_rr == b->target_rr &&
+ a->duration_incr_ms == b->duration_incr_ms &&
+ a->duration_decr_ms == b->duration_decr_ms &&
+ a->mode == b->mode;
+}
+
static bool
intel_compare_buffer(const u8 *a, const u8 *b, size_t len)
{
@@ -4836,6 +4847,30 @@ pipe_config_dp_vsc_sdp_mismatch(struct drm_i915_private
*i915,
drm_dp_vsc_sdp_log(&p, b);
}
+static void
+pipe_config_dp_as_sdp_mismatch(struct drm_i915_private *i915,
+ bool fastset, const char *name,
+ const struct drm_dp_as_sdp *a,
+ const struct drm_dp_as_sdp *b)
+{
+ struct drm_printer p;
+
+ if (fastset) {
+ p = drm_dbg_printer(&i915->drm, DRM_UT_KMS, NULL);
+
+ drm_printf(&p, "fastset requirement not met in %s dp sdp\n",
name);
+ } else {
+ p = drm_err_printer(&i915->drm, NULL);
+
+ drm_printf(&p, "mismatch in %s dp sdp\n", name);
+ }
+
+ drm_printf(&p, "expected:\n");
+ drm_dp_as_sdp_log(&p, a);
+ drm_printf(&p, "found:\n");
+ drm_dp_as_sdp_log(&p, b);
+}
+
/* Returns the length up to and including the last differing byte */
static size_t
memcmp_diff_len(const u8 *a, const u8 *b, size_t len)
@@ -5089,6 +5124,16 @@ intel_pipe_config_compare(const struct intel_crtc_state
*current_config,
} \
} while (0)
+#define PIPE_CONF_CHECK_DP_AS_SDP(name) do { \
+ if (!intel_compare_dp_as_sdp(¤t_config->infoframes.name, \
+ &pipe_config->infoframes.name)) { \
+ pipe_config_dp_as_sdp_mismatch(dev_priv, fastset,
__stringify(name), \
+
¤t_config->infoframes.name, \
+ &pipe_config->infoframes.name);
\
+ ret = false; \
+ } \
+} while (0)
+
#define PIPE_CONF_CHECK_BUFFER(name, len) do { \
BUILD_BUG_ON(sizeof(current_config->name) != (len)); \
BUILD_BUG_ON(sizeof(pipe_config->name) != (len)); \
@@ -5270,6 +5315,7 @@ intel_pipe_config_compare(const struct intel_crtc_state
*current_config,
PIPE_CONF_CHECK_INFOFRAME(hdmi);
PIPE_CONF_CHECK_INFOFRAME(drm);
PIPE_CONF_CHECK_DP_VSC_SDP(vsc);
+ PIPE_CONF_CHECK_DP_AS_SDP(as_sdp);
PIPE_CONF_CHECK_X(sync_mode_slaves_mask);
PIPE_CONF_CHECK_I(master_transcoder);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 1cd3cc0d0c0b..2ec1f923a5a0 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2648,6 +2648,8 @@ static void intel_dp_compute_as_sdp(struct intel_dp
*intel_dp,
as_sdp->target_rr = 0;
as_sdp->duration_incr_ms = 0;
as_sdp->duration_incr_ms = 0;
+
+ crtc_state->infoframes.enable |=
intel_hdmi_infoframe_enable(DP_SDP_ADAPTIVE_SYNC);
This change does not seem to belong to this patch.
Regards,
Ankit
}
static void intel_dp_compute_vsc_sdp(struct intel_dp *intel_dp,
[PATCH][next] staging: fbtft: remove unused variable 'count'
The variable count is being initialized and incremented but it is never
actually referenced in any other way. The variable is redundant and can
be removed.
Cleans up clang scan build warning:
drivers/staging/fbtft/fbtft-core.c:330:6: warning: variable 'count' set
but not used [-Wunused-but-set-variable]
Signed-off-by: Colin Ian King
---
drivers/staging/fbtft/fbtft-core.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c
b/drivers/staging/fbtft/fbtft-core.c
index 68add4d598ae..38845f23023f 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -327,7 +327,6 @@ static void fbtft_deferred_io(struct fb_info *info, struct
list_head *pagereflis
unsigned int dirty_lines_start, dirty_lines_end;
struct fb_deferred_io_pageref *pageref;
unsigned int y_low = 0, y_high = 0;
- int count = 0;
spin_lock(&par->dirty_lock);
dirty_lines_start = par->dirty_lines_start;
@@ -339,7 +338,6 @@ static void fbtft_deferred_io(struct fb_info *info, struct
list_head *pagereflis
/* Mark display lines as dirty */
list_for_each_entry(pageref, pagereflist, list) {
- count++;
y_low = pageref->offset / info->fix.line_length;
y_high = (pageref->offset + PAGE_SIZE - 1) /
info->fix.line_length;
dev_dbg(info->device,
--
2.39.2
Re: [PATCH v2 1/9] media: v4l2-subdev: Add a pad variant of .query_dv_timings()
On 2/29/24 12:33, Paweł Anikiel wrote: > On Thu, Feb 29, 2024 at 9:02 AM Hans Verkuil wrote: >> >> On 28/02/2024 16:34, Paweł Anikiel wrote: >>> On Wed, Feb 28, 2024 at 12:25 PM Hans Verkuil >>> wrote: Hi Paweł, On 21/02/2024 17:02, Paweł Anikiel wrote: > Currently, .query_dv_timings() is defined as a video callback without > a pad argument. This is a problem if the subdevice can have different > dv timings for each pad (e.g. a DisplayPort receiver with multiple > virtual channels). > > To solve this, add a pad variant of this callback which includes > the pad number as an argument. So now we have two query_dv_timings ops: one for video ops, and one for pad ops. That's not very maintainable. I would suggest switching all current users of the video op over to the pad op. >>> >>> I agree it would be better if there was only one. However, I have some >>> concerns: >>> 1. Isn't there a problem with backwards compatibility? For example, an >>> out-of-tree driver is likely to use this callback, which would break. >>> I'm asking because I'm not familiar with how such API changes are >>> handled. >> >> It's out of tree, so they will just have to adapt. That's how life is if >> you are not part of the mainline kernel. >> >>> 2. If I do switch all current users to the pad op, I can't test those >>> changes. Isn't that a problem? >> >> I can test one or two drivers, but in general I don't expect this to be >> a problem. >> >>> 3. Would I need to get ACK from all the driver maintainers? >> >> CC the patches to the maintainers. Generally you will get back Acks from >> some but not all maintainers, but that's OK. For changes affecting multiple >> drivers you never reach 100% on that. I can review the remainder. The DV >> Timings API is my expert area, so that shouldn't be a problem. >> >> A quick grep gives me these subdev drivers that implement it: >> >> adv748x, adv7604, adv7842, tc358743, tda1997x, tvp7002, gs1662. >> >> And these bridge drivers that call the subdevs: >> >> cobalt, rcar-vin, vpif_capture. >> >> The bridge drivers can use the following pad when calling query_dv_timings: >> >> cobalt: ADV76XX_PAD_HDMI_PORT_A >> rcar_vin: vin->parallel.sink_pad >> vpif_capture: 0 >> >> The converted subdev drivers should check if the pad is an input pad. >> Ideally it should check if the pad is equal to the current input pad >> since most devices can only query the timings for the currently selected >> input pad. But some older drivers predate the pad concept and they >> ignore the pad value. > > Thank you for the helpful info. I will switch all these drivers to the > pad op, then. Would you like me to prepare a separate patchset, or > should I include the changes in this one? I think I prefer a separate patchset for this. Regards, Hans
Re: [PATCH v2 6/9] drm/vkms: Add YUV support
On Wed, 28 Feb 2024 22:52:09 -0300
Arthur Grillo wrote:
> On 27/02/24 17:01, Arthur Grillo wrote:
> >
> >
> > On 27/02/24 12:02, Louis Chauvet wrote:
> >> Hi Pekka,
> >>
> >> For all the comment related to the conversion part, maybe Arthur have an
> >> opinion on it, I took his patch as a "black box" (I did not want to
> >> break (and debug) it).
> >>
> >> Le 26/02/24 - 14:19, Pekka Paalanen a écrit :
> >>> On Fri, 23 Feb 2024 12:37:26 +0100
> >>> Louis Chauvet wrote:
> >>>
> From: Arthur Grillo
>
> Add support to the YUV formats bellow:
>
> - NV12
> - NV16
> - NV24
> - NV21
> - NV61
> - NV42
> - YUV420
> - YUV422
> - YUV444
> - YVU420
> - YVU422
> - YVU444
>
> The conversion matrices of each encoding and range were obtained by
> rounding the values of the original conversion matrices multiplied by
> 2^8. This is done to avoid the use of fixed point operations.
>
> Signed-off-by: Arthur Grillo
> [Louis Chauvet: Adapted Arthur's work and implemented the read_line_t
> callbacks for yuv formats]
> Signed-off-by: Louis Chauvet
> ---
> drivers/gpu/drm/vkms/vkms_composer.c | 2 +-
> drivers/gpu/drm/vkms/vkms_drv.h | 6 +-
> drivers/gpu/drm/vkms/vkms_formats.c | 289
> +--
> drivers/gpu/drm/vkms/vkms_formats.h | 4 +
> drivers/gpu/drm/vkms/vkms_plane.c| 14 +-
> 5 files changed, 295 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
> b/drivers/gpu/drm/vkms/vkms_composer.c
> index e555bf9c1aee..54fc5161d565 100644
> --- a/drivers/gpu/drm/vkms/vkms_composer.c
> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> @@ -312,7 +312,7 @@ static void blend(struct vkms_writeback_job *wb,
> * buffer [1]
> */
> current_plane->pixel_read_line(
> -current_plane->frame_info,
> +current_plane,
> x_start,
> y_start,
> direction,
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h
> b/drivers/gpu/drm/vkms/vkms_drv.h
> index ccc5be009f15..a4f6456cb971 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -75,6 +75,8 @@ enum pixel_read_direction {
> READ_RIGHT
> };
>
> +struct vkms_plane_state;
> +
> /**
> <<< HEAD
> * typedef pixel_read_line_t - These functions are used to read a pixel
> line in the source frame,
> @@ -87,8 +89,8 @@ enum pixel_read_direction {
> * @out_pixel: Pointer where to write the pixel value. Pixels will be
> written between x_start and
> * x_end.
> */
> -typedef void (*pixel_read_line_t)(struct vkms_frame_info *frame_info,
> int x_start, int y_start, enum
> -pixel_read_direction direction, int count, struct
> pixel_argb_u16 out_pixel[]);
> +typedef void (*pixel_read_line_t)(struct vkms_plane_state *frame_info,
> int x_start, int y_start,
> +enum pixel_read_direction direction, int count, struct
> pixel_argb_u16 out_pixel[]);
> >>>
> >>> This is the second or third time in this one series changing this type.
> >>> Could you not do the change once, in its own patch if possible?
> >>
> >> Sorry, this is not a change here, but a wrong formatting (missed when
> >> rebasing).
> >>
> >> Do you think that it make sense to re-order my patches and put this
> >> typedef at the end? This way it is never updated.
I'm not sure, I haven't checked how it would change your patches. The
intermediate changes might get a lot uglier?
Just try to fold changes so that you don't need to change something
twice over the series unless there is a good reason to. "How hard would
it be to review this?" is my measure stick.
> >>
>
> /**
> * vkms_plane_state - Driver specific plane state
> diff --git a/drivers/gpu/drm/vkms/vkms_formats.c
> b/drivers/gpu/drm/vkms/vkms_formats.c
> index 46daea6d3ee9..515c80866a58 100644
> --- a/drivers/gpu/drm/vkms/vkms_formats.c
> +++ b/drivers/gpu/drm/vkms/vkms_formats.c
> @@ -33,7 +33,8 @@ static size_t packed_pixels_offset(const struct
> vkms_frame_info *frame_info, int
> */
> return fb->offsets[plane_index] +
> (y / drm_format_info_block_width(format, plane_index)) *
> fb->pitches[plane_index] +
> - (x / drm_format_info_block_height(format, plane_index))
> * format->char_per_block[plane_index];
> + (x / drm_format_info_block_height(format, plane_index))
Re: [PATCH v12 2/8] drm: Add Adaptive Sync SDP logging
On 2/29/2024 4:53 PM, Jani Nikula wrote:
On Thu, 29 Feb 2024, "Nautiyal, Ankit K" wrote:
On 2/28/2024 8:08 PM, Mitul Golani wrote:
+enum operation_mode {
+ DP_AS_SDP_AVT_DYNAMIC_VTOTAL = 0x00,
+ DP_AS_SDP_AVT_FIXED_VTOTAL = 0x01,
+ DP_AS_SDP_FAVT_TRR_NOT_REACHED = 0x02,
+ DP_AS_SDP_FAVT_TRR_REACHED = 0x03
+};
We can drop the initialization here.
For stuff that needs to match the spec it's common to include the
initializations instead of relying on the auto enumeration.
Ah alright, got it.
Regards,
Ankit
BR,
Jani.
Re: [PATCH] MAINTAINERS: Update email address for Tvrtko Ursulin
Quoting Tvrtko Ursulin (2024-02-28 16:22:40) > From: Tvrtko Ursulin > > I will lose access to my @.*intel.com e-mail addresses soon so let me > adjust the maintainers entry and update the mailmap too. > > While at it consolidate a few other of my old emails to point to the > main one. > > Signed-off-by: Tvrtko Ursulin > Cc: Daniel Vetter > Cc: Dave Airlie > Cc: Jani Nikula > Cc: Joonas Lahtinen > Cc: Rodrigo Vivi Acked-by: Joonas Lahtinen Regards, Joonas
Re: [PATCH 0/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing
On Thu, Feb 29, 2024 at 12:53:38PM +0100, Guillaume Tucker wrote: > On 29/02/2024 12:41, Mark Brown wrote: > > On Thu, Feb 29, 2024 at 01:19:19PM +0200, Laurent Pinchart wrote: > >> On Thu, Feb 29, 2024 at 01:10:16PM +0200, Nikolai Kondrashov wrote: > > > >>> Of course. You're also welcome to join the #kernelci channel on > >>> libera.chat. > > > >> Isn't that a bit pointless if it's no the main IM channel ? > > > > It *was* the original channel and still gets some usage (mostly started > > by me admittedly since I've never joined slack for a bunch of reasons > > that make it hassle), IIRC the Slack was started because there were some > > interns who had trouble figuring out IRC and intermittent connectivity > > but people seem to have migrated. > > In fact it was initially created for the members of the Linux > Foundation project only, which is why registration is moderated > for emails that don't have a domain linked to a member (BTW not > any Google account will just work e.g. @gmail.com is moderated, > only @google.com for Google employees isn't). > > And yes IRC is the "least common denominator" chat platform. > Maybe having a bridge between the main Slack channel and IRC > would help. If the gitlab CI pipeline proposal wants to be considered for inclusion in the kernel, I think it needs to switch to a free software solution for its *main* communication channels. -- Regards, Laurent Pinchart
Re: [PATCH v2 7/9] drm/vkms: Add range and encoding properties to pixel_read function
On Tue, 27 Feb 2024 16:02:10 +0100 Louis Chauvet wrote: > (same as for PATCHv2 6/9, I took the patch from Arthur with no > modifications) > > Le 26/02/24 - 14:23, Pekka Paalanen a écrit : > > On Fri, 23 Feb 2024 12:37:27 +0100 > > Louis Chauvet wrote: > > > > > From: Arthur Grillo > > > > > > Create range and encoding properties. This should be noop, as none of > > > the conversion functions need those properties. > > > > None of the conversion function needs this? How can one say so? > > The previous patch is making use of them already, AFAICT? > > It's my fault, I mixed the commits (in Arthur's series, "Add range..." was > before "Add YUV support"), but for me it makes no sens to have the color > property without the support in the driver. Ah, so if there was no YUV support, these properties would never affect anything. Ok, I see where that is coming from. > > Maybe it's better just to merge "Add range..." with "Add YUV support"? > > > How is this a noop? Is it not exposing new UAPI from VKMS? > > It's not a no-op from userspace, but from the driver side, yes. If it all is already hooked up and handled in the driver, then say just that? "Now that the driver internally handles these quantization ranges and YUV encoding matrices, expose the UAPI for setting them." And fix the commit summary line too, nothing "pixel_read" here. Thanks, pq > > Kind regards, > Louis Chauvet > > > Thanks, > > pq > > > > > > > > Signed-off-by: Arthur Grillo > > > [Louis Chauvet: retained only relevant parts] > > > Signed-off-by: Louis Chauvet > > > --- > > > drivers/gpu/drm/vkms/vkms_plane.c | 9 + > > > 1 file changed, 9 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c > > > b/drivers/gpu/drm/vkms/vkms_plane.c > > > index 427ca67c60ce..95dfde297377 100644 > > > --- a/drivers/gpu/drm/vkms/vkms_plane.c > > > +++ b/drivers/gpu/drm/vkms/vkms_plane.c > > > @@ -228,5 +228,14 @@ struct vkms_plane *vkms_plane_init(struct > > > vkms_device *vkmsdev, > > > drm_plane_create_rotation_property(&plane->base, DRM_MODE_ROTATE_0, > > > DRM_MODE_ROTATE_MASK | > > > DRM_MODE_REFLECT_MASK); > > > > > > + drm_plane_create_color_properties(&plane->base, > > > + BIT(DRM_COLOR_YCBCR_BT601) | > > > + BIT(DRM_COLOR_YCBCR_BT709) | > > > + BIT(DRM_COLOR_YCBCR_BT2020), > > > + BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) | > > > + BIT(DRM_COLOR_YCBCR_FULL_RANGE), > > > + DRM_COLOR_YCBCR_BT601, > > > + DRM_COLOR_YCBCR_FULL_RANGE); > > > + > > > return plane; > > > } > > > > > > > > pgpBVFZw36xFu.pgp Description: OpenPGP digital signature
Re: [PATCH 0/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing
On Thu, Feb 29, 2024 at 02:20:41PM +0200, Laurent Pinchart wrote: > On Thu, Feb 29, 2024 at 12:53:38PM +0100, Guillaume Tucker wrote: > > On 29/02/2024 12:41, Mark Brown wrote: > > > On Thu, Feb 29, 2024 at 01:19:19PM +0200, Laurent Pinchart wrote: > > >> On Thu, Feb 29, 2024 at 01:10:16PM +0200, Nikolai Kondrashov wrote: > > > > > >>> Of course. You're also welcome to join the #kernelci channel on > > >>> libera.chat. > > > > > >> Isn't that a bit pointless if it's no the main IM channel ? > > > > > > It *was* the original channel and still gets some usage (mostly started > > > by me admittedly since I've never joined slack for a bunch of reasons > > > that make it hassle), IIRC the Slack was started because there were some > > > interns who had trouble figuring out IRC and intermittent connectivity > > > but people seem to have migrated. > > > > In fact it was initially created for the members of the Linux > > Foundation project only, which is why registration is moderated > > for emails that don't have a domain linked to a member (BTW not > > any Google account will just work e.g. @gmail.com is moderated, > > only @google.com for Google employees isn't). > > > > And yes IRC is the "least common denominator" chat platform. > > Maybe having a bridge between the main Slack channel and IRC > > would help. > > If the gitlab CI pipeline proposal wants to be considered for inclusion > in the kernel, I think it needs to switch to a free software solution > for its *main* communication channels. And to clarify, I didn't meant the kernel CI project, but only the gitlab CI pipeline for the Linux kernel project. I don't know how tightly integrated the two projects are though. -- Regards, Laurent Pinchart
Re: [PATCH] drm/i915: Remove unneeded double drm_rect_visible call in check_overlay_dst
On Wed, Feb 28, 2024 at 09:32:47PM +0300, Nikita Kiryushin wrote:
>
> check_overlay_dst for clipped is called 2 times: in drm_rect_intersect
> and than directly. Change second call for check of drm_rect_intersect
> result to save some time (in locked code section).
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: 8d8b2dd3995f ("drm/i915: Make the PIPESRC rect relative to the
> entire bigjoiner area")
> Signed-off-by: Nikita Kiryushin
> ---
> drivers/gpu/drm/i915/display/intel_overlay.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c
> b/drivers/gpu/drm/i915/display/intel_overlay.c
> index 2b1392d5a902..1cda1c163a92 100644
> --- a/drivers/gpu/drm/i915/display/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/display/intel_overlay.c
> @@ -972,9 +972,8 @@ static int check_overlay_dst(struct intel_overlay
> *overlay,
> rec->dst_width, rec->dst_height);
> clipped = req;
> - drm_rect_intersect(&clipped, &crtc_state->pipe_src);
> - if (!drm_rect_visible(&clipped) ||
> + if (!drm_rect_intersect(&clipped, &crtc_state->pipe_src) ||
I prefer the current way where we have no side effects in
the if statement.
> !drm_rect_equals(&clipped, &req))
> return -EINVAL;
> -- 2.34.1
--
Ville Syrjälä
Intel
Re: [PATCH v3 3/9] drm/vkms: write/update the documentation for pixel conversion and pixel write functions
On Tue, 27 Feb 2024 15:47:08 -0300
Arthur Grillo wrote:
> On 27/02/24 12:02, Louis Chauvet wrote:
> > Le 26/02/24 - 10:07, Arthur Grillo a écrit :
> >>
> >>
> >> On 26/02/24 05:46, Louis Chauvet wrote:
> >>> Add some documentation on pixel conversion functions.
> >>> Update of outdated comments for pixel_write functions.
> >>>
> >>> Signed-off-by: Louis Chauvet
> >>> ---
> >>> drivers/gpu/drm/vkms/vkms_composer.c | 4 +++
> >>> drivers/gpu/drm/vkms/vkms_drv.h | 13
> >>> drivers/gpu/drm/vkms/vkms_formats.c | 58
> >>> ++--
> >>> 3 files changed, 66 insertions(+), 9 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
> >>> b/drivers/gpu/drm/vkms/vkms_composer.c
> >>> index c6d9b4a65809..5b341222d239 100644
> >>> --- a/drivers/gpu/drm/vkms/vkms_composer.c
> >>> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> >>> @@ -189,6 +189,10 @@ static void blend(struct vkms_writeback_job *wb,
> >>>
> >>> size_t crtc_y_limit = crtc_state->base.crtc->mode.vdisplay;
> >>>
> >>> + /*
> >>> + * The planes are composed line-by-line. It is a necessary complexity
> >>> to avoid poor
> >>> + * blending performance.
> >>
> >> At this moment in the series, you have not yet reintroduced the
> >> line-by-line algorithm yet. Maybe it's better to add this comment when
> >> you do.
> >
> > Is it better with this:
> >
> > /*
> > * The planes are composed line-by-line to avoid heavy memory usage. It
> > is a necessary
> > * complexity to avoid poor blending performance.
> > *
> > * The function vkms_compose_row is used to read a line,
> > pixel-by-pixel, into the staging
> > * buffer.
> > */
> >
> >> Also, I think it's good to give more context, like:
> >> "The planes are composed line-by-line, instead of pixel-by-pixel"
> >
> > And after PATCHv3 5/9:
> >
> > /*
> > * The planes are composed line-by-line to avoid heavy memory usage. It
> > is a necessary
> > * complexity to avoid poor blending performance.
> > *
> > * The function pixel_read_line callback is used to read a line, using
> > an efficient
> > * algorithm for a specific format, into the staging buffer.
> > */
> >
Hi,
there are a few reasons for the line-by-line algorithm, and the
optimizations at large:
VKMS uses temporary stage and output buffers so that blending functions
can operate on just one high-precision pixel format, struct
pixel_argb_u16. We can make pixel-format-specific read and write
functions completely orthogonal from the blending operations and FB
format combinations. This avoids a combinatorial explosion of needed
functions for { input pixel formats × blending operations × output pixel
formats }.
We can use a temporary stage and output buffer whose size is one line
and not whole FB or CRTC framebuffer. This is the memory savings.
Using a temporary output buffer also avoids repeated
read-decode-blend-encode-write cycles into the final destination
buffer, as we don't need to decode/encode the pixel format.
Finally, doing elementary operations (read, blend, write) line-by-line
is much more efficient than pixel-by-pixel, because it allows making
the inner-most loop very tight. It avoids repeatedly computing a result
that does not change, like which function to call for a specific pixel
format or blending equation.
Thanks,
pq
pgpEfUKR6v06m.pgp
Description: OpenPGP digital signature
Re: UAPI Re: [PATCH 1/3] drm: Add DRM_MODE_TV_MODE_MONOCHROME
Hi Simon, On Wed, Feb 28, 2024 at 04:22:56PM +, Simon Ser wrote: > On Wednesday, February 28th, 2024 at 17:14, Maxime Ripard > wrote: > > > > I don't know what the rules were 8 years ago, but the current uAPI rules > > > are what they are, and a new enum entry is new uAPI. > > > > TBF, and even if the wayland compositors support is missing, this > > property is perfectly usable as it is with upstream, open-source code, > > through either the command-line or X.org, and it's documented. > > > > So it's fine by me from a UAPI requirement side. > > That is not a valid way to pass the uAPI requirements IMHO. Yes, one > can program any KMS property via modetest or xrandr. Does that mean that > none of the new uAPI need a "real" implementation anymore? Does that mean > that the massive patch adding a color pipeline uAPI doesn't need > user-space anymore? I guess it's also a matter what the API is exactly? Like, xrandr or the kernel command line allows to use that particular API fully. Can you fully exert the color pipeline uAPI with xrandr? And at the time we submitted it, even with our best intent, we couldn't totally clear the userspace requirement because the PR would have been rejected because nobody wanted to deal with analog TV. And that's fair, any upstream project is free to do as it wants and analog TV is certainly not the state of the art anymore. But we had some variation of that property used in many drivers (i915, nouveau, vc4, sun4i and amlogic from the top of my head), all drivers-specific, and having that kind of support was also one of the blockers to move the few remaining fbdev drivers to KMS. It seems a bit unfair to put that requirement now that maybe some compositors could be interested. > The only thing I'm saying is that this breaks the usual DRM requirements. > If, as a maintainer, you're fine with breaking the rules and have a good > motivation to do so, that's fine by me. Rules are meant to be broken from > time to time depending on the situation. But please don't pretend that > modetest/xrandr is valid user-space to pass the rules. Ack. And indeed, modetest surely was a bad example. Maxime signature.asc Description: PGP signature
[PULL] drm-xe-fixes
Dave, Sima The xe fixes for -rc7. It's mostly uapi sanitizing and future-proofing, and a couple of driver fixes. drm-xe-fixes-2024-02-29: UAPI Changes: - A couple of tracepoint updates from Priyanka and Lucas. - Make sure BINDs are completed before accepting UNBINDs on LR vms. - Don't arbitrarily restrict max number of batched binds. - Add uapi for dumpable bos (agreed on IRC). - Remove unused uapi flags and a leftover comment. Driver Changes: - A couple of fixes related to the execlist backend. - A 32-bit fix. /Thomas The following changes since commit 6650d23f3e20ca00482a71a4ef900f0ea776fb15: drm/xe: Fix modpost warning on xe_mocs kunit module (2024-02-21 11:06:52 +0100) are available in the Git repository at: https://gitlab.freedesktop.org/drm/xe/kernel.git tags/drm-xe-fixes-2024-02-29 for you to fetch changes up to 8188cae3cc3d8018ec97ca9ab8caa3acc69a056d: drm/xe/xe_trace: Add move_lacks_source detail to xe_bo_move trace (2024-02-29 12:32:15 +0100) UAPI Changes: - A couple of tracepoint updates from Priyanka and Lucas. - Make sure BINDs are completed before accepting UNBINDs on LR vms. - Don't arbitrarily restrict max number of batched binds. - Add uapi for dumpable bos (agreed on IRC). - Remove unused uapi flags and a leftover comment. Driver Changes: - A couple of fixes related to the execlist backend. - A 32-bit fix. Arnd Bergmann (1): drm/xe/mmio: fix build warning for BAR resize on 32-bit Francois Dugast (1): drm/xe/uapi: Remove unused flags José Roberto de Souza (1): drm/xe/uapi: Remove DRM_XE_VM_BIND_FLAG_ASYNC comment left over Lucas De Marchi (1): drm/xe: Use pointers in trace events Maarten Lankhorst (1): drm/xe: Add uapi for dumpable bos Matthew Brost (3): drm/xe: Fix execlist splat drm/xe: Don't support execlists in xe_gt_tlb_invalidation layer drm/xe: Use vmalloc for array of bind allocation in bind IOCTL Mika Kuoppala (2): drm/xe: Expose user fence from xe_sync_entry drm/xe: Deny unbinds if uapi ufence pending Paulo Zanoni (1): drm/xe: get rid of MAX_BINDS Priyanka Dandamudi (2): drm/xe/xe_bo_move: Enhance xe_bo_move trace drm/xe/xe_trace: Add move_lacks_source detail to xe_bo_move trace drivers/gpu/drm/xe/xe_bo.c | 11 +++- drivers/gpu/drm/xe/xe_bo.h | 1 + drivers/gpu/drm/xe/xe_drm_client.c | 12 +--- drivers/gpu/drm/xe/xe_exec_queue.c | 88 + drivers/gpu/drm/xe/xe_exec_queue_types.h| 10 drivers/gpu/drm/xe/xe_execlist.c| 2 +- drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c | 12 drivers/gpu/drm/xe/xe_lrc.c | 10 +--- drivers/gpu/drm/xe/xe_mmio.c| 2 +- drivers/gpu/drm/xe/xe_sync.c| 58 +++ drivers/gpu/drm/xe/xe_sync.h| 4 ++ drivers/gpu/drm/xe/xe_sync_types.h | 2 +- drivers/gpu/drm/xe/xe_trace.h | 59 +-- drivers/gpu/drm/xe/xe_vm.c | 80 ++ drivers/gpu/drm/xe/xe_vm_types.h| 11 ++-- include/uapi/drm/xe_drm.h | 21 +-- 16 files changed, 187 insertions(+), 196 deletions(-)
Re: [PATCH 1/2] drm/buddy: stop using PAGE_SIZE
Am 29.02.24 um 11:51 schrieb Matthew Auld:
The drm_buddy minimum page-size requirements should be distinct from the
CPU PAGE_SIZE. Only restriction is that the minimum page-size is at
least 4K.
Signed-off-by: Matthew Auld
Cc: Arunpravin Paneer Selvam
Cc: Christian König
Cc: Arnd Bergmann
Acked-by: Christian König for the series.
---
drivers/gpu/drm/drm_buddy.c | 2 +-
include/drm/drm_buddy.h | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index 5ebdd6f8f36e..f999568d69c1 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -102,7 +102,7 @@ int drm_buddy_init(struct drm_buddy *mm, u64 size, u64
chunk_size)
if (size < chunk_size)
return -EINVAL;
- if (chunk_size < PAGE_SIZE)
+ if (chunk_size < SZ_4K)
return -EINVAL;
if (!is_power_of_2(chunk_size))
diff --git a/include/drm/drm_buddy.h b/include/drm/drm_buddy.h
index a5b39fc01003..19ed661a32f3 100644
--- a/include/drm/drm_buddy.h
+++ b/include/drm/drm_buddy.h
@@ -53,8 +53,8 @@ struct drm_buddy_block {
struct list_head tmp_link;
};
-/* Order-zero must be at least PAGE_SIZE */
-#define DRM_BUDDY_MAX_ORDER (63 - PAGE_SHIFT)
+/* Order-zero must be at least SZ_4K */
+#define DRM_BUDDY_MAX_ORDER (63 - 12)
/*
* Binary Buddy System.
@@ -82,7 +82,7 @@ struct drm_buddy {
unsigned int n_roots;
unsigned int max_order;
- /* Must be at least PAGE_SIZE */
+ /* Must be at least SZ_4K */
u64 chunk_size;
u64 size;
u64 avail;
Re: [PATCH 1/2] drm/buddy: stop using PAGE_SIZE
On Thu, Feb 29, 2024, at 11:51, Matthew Auld wrote: > The drm_buddy minimum page-size requirements should be distinct from the > CPU PAGE_SIZE. Only restriction is that the minimum page-size is at > least 4K. > > Signed-off-by: Matthew Auld > Cc: Arunpravin Paneer Selvam > Cc: Christian König > Cc: Arnd Bergmann Acked-by: Arnd Bergmann
[PULL] drm-misc-fixes
Hi, Here's this week drm-misc fixes PR. There's two commits for files unders drivers/soc/qcom that don't have a maintainer Acked-by. Bjorn's Acked-by was provided on IRC, and Konrad provided it by mail after the facts so we're covered. Maxime drm-misc-fixes-2024-02-29: A reset fix for host1x, a resource leak fix and a probe fix for aux-hpd, a use-after-free fix and a boot fix for a pmic_glink qcom driver in drivers/soc, a fix for the simpledrm/tegra transition, a kunit fix for the TTM tests, a font handling fix for fbcon, two allocation fixes and a kunit test to cover them for drm/buddy The following changes since commit 72fa02fdf83306c52bc1eede28359e3fa32a151a: nouveau: add an ioctl to report vram usage (2024-02-23 10:20:07 +1000) are available in the Git repository at: https://anongit.freedesktop.org/git/drm/drm-misc tags/drm-misc-fixes-2024-02-29 for you to fetch changes up to c70703320e557ff30847915e6a7631a9abdda16b: drm/tests/drm_buddy: add alloc_range_bias test (2024-02-28 08:03:29 +0100) A reset fix for host1x, a resource leak fix and a probe fix for aux-hpd, a use-after-free fix and a boot fix for a pmic_glink qcom driver in drivers/soc, a fix for the simpledrm/tegra transition, a kunit fix for the TTM tests, a font handling fix for fbcon, two allocation fixes and a kunit test to cover them for drm/buddy Christian König (1): drm/ttm/tests: depend on UML || COMPILE_TEST Jiri Slaby (SUSE) (1): fbcon: always restore the old font data in fbcon_do_set_font() Johan Hovold (3): drm/bridge: aux-hpd: fix OF node leaks drm/bridge: aux-hpd: separate allocation and registration soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free Matthew Auld (3): drm/buddy: fix range bias drm/buddy: check range allocation matches alignment drm/tests/drm_buddy: add alloc_range_bias test Maxime Ripard (1): Merge drm/drm-fixes into drm-misc-fixes Mikko Perttunen (1): gpu: host1x: Skip reset assert on Tegra186 Rob Clark (1): soc: qcom: pmic_glink: Fix boot when QRTR=m Thierry Reding (1): drm/tegra: Remove existing framebuffer only if we support display drivers/gpu/drm/Kconfig | 5 +- drivers/gpu/drm/bridge/aux-hpd-bridge.c | 70 +++--- drivers/gpu/drm/drm_buddy.c | 16 ++- drivers/gpu/drm/tegra/drm.c | 23 +++- drivers/gpu/drm/tests/drm_buddy_test.c | 218 drivers/gpu/host1x/dev.c| 15 ++- drivers/gpu/host1x/dev.h| 6 + drivers/soc/qcom/pmic_glink.c | 21 +-- drivers/soc/qcom/pmic_glink_altmode.c | 16 ++- drivers/video/fbdev/core/fbcon.c| 8 +- include/drm/bridge/aux-bridge.h | 15 +++ 11 files changed, 368 insertions(+), 45 deletions(-) signature.asc Description: PGP signature
[PATCH 2/2] drm/amdgpu: use GTT only as fallback for VRAM|GTT
Try to fill up VRAM as well by setting the busy flag on GTT allocations. This fixes the issue that when VRAM was evacuated for suspend it's never filled up again unless the application is restarted. Signed-off-by: Christian König Reviewed-by: Zack Rusin --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 010b0cb7693c..8bc79924d171 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -173,6 +173,12 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain) abo->flags & AMDGPU_GEM_CREATE_PREEMPTIBLE ? AMDGPU_PL_PREEMPT : TTM_PL_TT; places[c].flags = 0; + /* +* When GTT is just an alternative to VRAM make sure that we +* only use it as fallback and still try to fill up VRAM first. +*/ + if (domain & abo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM) + places[c].flags |= TTM_PL_FLAG_FALLBACK; c++; } -- 2.34.1
[PATCH 1/2] drm/ttm: improve idle/busy handling v5
Previously we would never try to move a BO into the preferred placements
when it ever landed in a busy placement since those were considered
compatible.
Rework the whole handling and finally unify the idle and busy handling.
ttm_bo_validate() is now responsible to try idle placement first and then
use the busy placement if that didn't worked.
Drawback is that we now always try the idle placement first for each
validation which might cause some additional CPU overhead on overcommit.
v2: fix kerneldoc warning and coding style
v3: take care of XE as well
v4: keep the ttm_bo_mem_space functionality as it is for now, only add
new handling for ttm_bo_validate as suggested by Thomas
v5: fix bug pointed out by Matthew
Signed-off-by: Christian König
Reviewed-by: Zack Rusin v3
---
drivers/gpu/drm/ttm/ttm_bo.c | 231 +
drivers/gpu/drm/ttm/ttm_resource.c | 16 +-
include/drm/ttm/ttm_resource.h | 3 +-
3 files changed, 121 insertions(+), 129 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 96a724e8f3ff..e059b1e1b13b 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -724,64 +724,36 @@ static int ttm_bo_add_move_fence(struct ttm_buffer_object
*bo,
return ret;
}
-/*
- * Repeatedly evict memory from the LRU for @mem_type until we create enough
- * space, or we've evicted everything and there isn't enough space.
- */
-static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo,
- const struct ttm_place *place,
- struct ttm_resource **mem,
- struct ttm_operation_ctx *ctx)
-{
- struct ttm_device *bdev = bo->bdev;
- struct ttm_resource_manager *man;
- struct ww_acquire_ctx *ticket;
- int ret;
-
- man = ttm_manager_type(bdev, place->mem_type);
- ticket = dma_resv_locking_ctx(bo->base.resv);
- do {
- ret = ttm_resource_alloc(bo, place, mem);
- if (likely(!ret))
- break;
- if (unlikely(ret != -ENOSPC))
- return ret;
- ret = ttm_mem_evict_first(bdev, man, place, ctx,
- ticket);
- if (unlikely(ret != 0))
- return ret;
- } while (1);
-
- return ttm_bo_add_move_fence(bo, man, *mem, ctx->no_wait_gpu);
-}
-
/**
- * ttm_bo_mem_space
+ * ttm_bo_alloc_resource - Allocate backing store for a BO
*
- * @bo: Pointer to a struct ttm_buffer_object. the data of which
- * we want to allocate space for.
- * @placement: Proposed new placement for the buffer object.
- * @mem: A struct ttm_resource.
+ * @bo: Pointer to a struct ttm_buffer_object of which we want a resource for
+ * @placement: Proposed new placement for the buffer object
* @ctx: if and how to sleep, lock buffers and alloc memory
+ * @force_space: If we should evict buffers to force space
+ * @res: The resulting struct ttm_resource.
*
- * Allocate memory space for the buffer object pointed to by @bo, using
- * the placement flags in @placement, potentially evicting other idle buffer
objects.
- * This function may sleep while waiting for space to become available.
+ * Allocates a resource for the buffer object pointed to by @bo, using the
+ * placement flags in @placement, potentially evicting other buffer objects
when
+ * @force_space is true.
+ * This function may sleep while waiting for resources to become available.
* Returns:
- * -EBUSY: No space available (only if no_wait == 1).
+ * -EBUSY: No space available (only if no_wait == true).
* -ENOSPC: Could not allocate space for the buffer object, either due to
* fragmentation or concurrent allocators.
* -ERESTARTSYS: An interruptible sleep was interrupted by a signal.
*/
-int ttm_bo_mem_space(struct ttm_buffer_object *bo,
- struct ttm_placement *placement,
- struct ttm_resource **mem,
- struct ttm_operation_ctx *ctx)
+static int ttm_bo_alloc_resource(struct ttm_buffer_object *bo,
+struct ttm_placement *placement,
+struct ttm_operation_ctx *ctx,
+bool force_space,
+struct ttm_resource **res)
{
struct ttm_device *bdev = bo->bdev;
- bool type_found = false;
+ struct ww_acquire_ctx *ticket;
int i, ret;
+ ticket = dma_resv_locking_ctx(bo->base.resv);
ret = dma_resv_reserve_fences(bo->base.resv, 1);
if (unlikely(ret))
return ret;
@@ -790,98 +762,73 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
const struct ttm_place *place = &placement->placement[i];
struct ttm_resource_manager *man;
- if (place->flags & TTM_PL_FLAG_FALLBACK)
-
Re: [PULL] drm-misc-fixes
On 29/02/2024 13:37, Maxime Ripard wrote: Hi, Here's this week drm-misc fixes PR. There's two commits for files unders drivers/soc/qcom that don't have a maintainer Acked-by. Bjorn's Acked-by was provided on IRC, and Konrad provided it by mail after the facts so we're covered. Maxime drm-misc-fixes-2024-02-29: A reset fix for host1x, a resource leak fix and a probe fix for aux-hpd, a use-after-free fix and a boot fix for a pmic_glink qcom driver in drivers/soc, a fix for the simpledrm/tegra transition, a kunit fix for the TTM tests, a font handling fix for fbcon, two allocation fixes and a kunit test to cover them for drm/buddy The following changes since commit 72fa02fdf83306c52bc1eede28359e3fa32a151a: nouveau: add an ioctl to report vram usage (2024-02-23 10:20:07 +1000) are available in the Git repository at: https://anongit.freedesktop.org/git/drm/drm-misc tags/drm-misc-fixes-2024-02-29 for you to fetch changes up to c70703320e557ff30847915e6a7631a9abdda16b: drm/tests/drm_buddy: add alloc_range_bias test (2024-02-28 08:03:29 +0100) A reset fix for host1x, a resource leak fix and a probe fix for aux-hpd, a use-after-free fix and a boot fix for a pmic_glink qcom driver in drivers/soc, a fix for the simpledrm/tegra transition, a kunit fix for the TTM tests, a font handling fix for fbcon, two allocation fixes and a kunit test to cover them for drm/buddy Christian König (1): drm/ttm/tests: depend on UML || COMPILE_TEST Jiri Slaby (SUSE) (1): fbcon: always restore the old font data in fbcon_do_set_font() Johan Hovold (3): drm/bridge: aux-hpd: fix OF node leaks drm/bridge: aux-hpd: separate allocation and registration soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free Matthew Auld (3): drm/buddy: fix range bias drm/buddy: check range allocation matches alignment drm/tests/drm_buddy: add alloc_range_bias test Note that there is a build fix needed for this one: https://patchwork.freedesktop.org/patch/580568/?series=130552&rev=1 Maxime Ripard (1): Merge drm/drm-fixes into drm-misc-fixes Mikko Perttunen (1): gpu: host1x: Skip reset assert on Tegra186 Rob Clark (1): soc: qcom: pmic_glink: Fix boot when QRTR=m Thierry Reding (1): drm/tegra: Remove existing framebuffer only if we support display drivers/gpu/drm/Kconfig | 5 +- drivers/gpu/drm/bridge/aux-hpd-bridge.c | 70 +++--- drivers/gpu/drm/drm_buddy.c | 16 ++- drivers/gpu/drm/tegra/drm.c | 23 +++- drivers/gpu/drm/tests/drm_buddy_test.c | 218 drivers/gpu/host1x/dev.c| 15 ++- drivers/gpu/host1x/dev.h| 6 + drivers/soc/qcom/pmic_glink.c | 21 +-- drivers/soc/qcom/pmic_glink_altmode.c | 16 ++- drivers/video/fbdev/core/fbcon.c| 8 +- include/drm/bridge/aux-bridge.h | 15 +++ 11 files changed, 368 insertions(+), 45 deletions(-)
Re: [PATCH v8 3/8] drm/panic: Add debugfs entry to test without triggering panic.
On 29/02/2024 12:21, Daniel Vetter wrote:
On Tue, Feb 27, 2024 at 11:04:14AM +0100, Jocelyn Falempe wrote:
Add a debugfs file, so you can test drm_panic without freezing
your machine. This is unsafe, and should be enabled only for
developer or tester.
to display the drm_panic screen, just run:
echo 1 > /sys/kernel/debug/drm_panic/trigger
Signed-off-by: Jocelyn Falempe
---
drivers/gpu/drm/Kconfig | 9 +++
drivers/gpu/drm/drm_panic.c | 47 +
2 files changed, 56 insertions(+)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index c17d8a8f6877..8dcea29f595c 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -125,6 +125,15 @@ config DRM_PANIC_BACKGROUND_COLOR
depends on DRM_PANIC
default 0x00
+config DRM_PANIC_DEBUG
+ bool "Add a debug fs entry to trigger drm_panic"
+ depends on DRM_PANIC && DEBUG_FS
+ help
+ Add drm_panic/trigger in the kernel debugfs, to force the panic
+ handler to write the panic message to the scanout buffer. This is
+ unsafe and should not be enabled on a production build.
+ If in doubt, say "N".
+
config DRM_DEBUG_DP_MST_TOPOLOGY_REFS
bool "Enable refcount backtrace history in the DP MST helpers"
depends on STACKTRACE_SUPPORT
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index c9f386476ef9..c5d3f725c5f5 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -398,3 +398,50 @@ void drm_panic_unregister(struct drm_plane *plane)
}
EXPORT_SYMBOL(drm_panic_unregister);
+
+/*
+ * DEBUG, This is currently unsafe.
+ * Also it will call all panic_notifier, since there is no way to filter and
+ * only call the drm_panic notifier.
+ */
+#ifdef CONFIG_DRM_PANIC_DEBUG
+#include
+
+static struct dentry *debug_dir;
+static struct dentry *debug_trigger;
+
+static ssize_t dbgfs_trigger_write(struct file *file, const char __user
*user_buf,
+ size_t count, loff_t *ppos)
+{
+ bool run;
+
+ if (kstrtobool_from_user(user_buf, count, &run) == 0 && run)
+ atomic_notifier_call_chain(&panic_notifier_list, 0, "Test drm panic
from debugfs");
Since this is just the general panic notifier it feels very misplaced in
the drm subsystem. I think moving that code into the core panic code makes
a lot more sense, then we'd also have all the right people on Cc: to
figure out how we can best recreate the correct calling context (like nmi
context or whatever) for best case simulation of panic code. John Ogness
definitely needs to see this and ack, wherever we put it.
I'm not sure it makes sense to test all panic notifiers at once.
So maybe I can write an atomic_notifier_call_chain_with_filter(), and
filter on the callback address, so it will only call the drm_panic
handlers ?
--
Jocelyn
-Sima
+ return count;
+}
+
+static const struct file_operations dbg_drm_panic_ops = {
+ .owner = THIS_MODULE,
+ .write = dbgfs_trigger_write,
+};
+
+static int __init debugfs_start(void)
+{
+ debug_dir = debugfs_create_dir("drm_panic", NULL);
+
+ if (IS_ERR(debug_dir))
+ return PTR_ERR(debug_dir);
+ debug_trigger = debugfs_create_file("trigger", 0200, debug_dir,
+ NULL, &dbg_drm_panic_ops);
+ return 0;
+}
+
+static void __exit debugfs_end(void)
+{
+ debugfs_remove_recursive(debug_dir);
+}
+
+module_init(debugfs_start);
+module_exit(debugfs_end);
+
+#endif
--
2.43.0
Re: [PATCH v8 5/8] drm/simpledrm: Add drm_panic support
On 29/02/2024 12:17, Daniel Vetter wrote:
On Tue, Feb 27, 2024 at 11:04:16AM +0100, Jocelyn Falempe wrote:
Add support for the drm_panic module, which displays a user-friendly
message to the screen when a kernel panic occurs.
v8:
* Replace get_scanout_buffer() with drm_panic_set_buffer()
(Thomas Zimmermann)
Signed-off-by: Jocelyn Falempe
---
drivers/gpu/drm/tiny/simpledrm.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 7ce1c4617675..a2190995354a 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include
#define DRIVER_NAME "simpledrm"
@@ -735,6 +736,20 @@ static const struct drm_connector_funcs
simpledrm_connector_funcs = {
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
};
+static void simpledrm_init_panic_buffer(struct drm_plane *plane)
+{
+ struct simpledrm_device *sdev = simpledrm_device_of_dev(plane->dev);
+ struct drm_framebuffer fb;
+
+ /* Fake framebuffer struct for drm_panic_set_buffer */
+ fb.width = sdev->mode.hdisplay;
+ fb.height = sdev->mode.vdisplay;
+ fb.format = sdev->format;
+ fb.pitches[0] = sdev->pitch;
+
+ drm_panic_set_buffer(plane->panic_scanout, &fb, &sdev->screen_base);
+}
+
static const struct drm_mode_config_funcs simpledrm_mode_config_funcs = {
.fb_create = drm_gem_fb_create_with_dirty,
.atomic_check = drm_atomic_helper_check,
@@ -945,6 +960,8 @@ static struct simpledrm_device
*simpledrm_device_create(struct drm_driver *drv,
return ERR_PTR(ret);
drm_plane_helper_add(primary_plane,
&simpledrm_primary_plane_helper_funcs);
drm_plane_enable_fb_damage_clips(primary_plane);
+ drm_panic_register(primary_plane);
Just a quick comment on this:
This does not work, the driver is not ready to handle panic calls at this
stage. Instead we need to automatically register all planes that support
panic handling in drm_dev_register(), and we need to remove them all again
in drm_dev_unregister(). Outside of these functions it is not safe to call
into driver code.
If you register the primary plane and didn't call drm_panic_set_buffer()
yet, the panic handler will not do anything, so it should be safe.
But if we revert to using the get_scanout_buffer(), this makes sense.
At that point it might be simpler to only register one panic notifier per
drm_device, and push the loop into the panic handler again.
Cheers, Sima
+ simpledrm_init_panic_buffer(primary_plane);
/* CRTC */
--
2.43.0
Re: [PATCH] MAINTAINERS: Update email address for Tvrtko Ursulin
On Wed, Feb 28, 2024 at 02:22:40PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > I will lose access to my @.*intel.com e-mail addresses soon so let me > adjust the maintainers entry and update the mailmap too. > > While at it consolidate a few other of my old emails to point to the > main one. > > Signed-off-by: Tvrtko Ursulin > Cc: Daniel Vetter > Cc: Dave Airlie > Cc: Jani Nikula > Cc: Joonas Lahtinen > Cc: Rodrigo Vivi Acked-by: Rodrigo Vivi > --- > .mailmap| 5 + > MAINTAINERS | 2 +- > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/.mailmap b/.mailmap > index b99a238ee3bd..d67e351bce8e 100644 > --- a/.mailmap > +++ b/.mailmap > @@ -608,6 +608,11 @@ TripleX Chung > TripleX Chung > Tsuneo Yoshioka > Tudor Ambarus > +Tvrtko Ursulin > +Tvrtko Ursulin > +Tvrtko Ursulin > +Tvrtko Ursulin > +Tvrtko Ursulin > Tycho Andersen > Tzung-Bi Shih > Uwe Kleine-König > diff --git a/MAINTAINERS b/MAINTAINERS > index 19f6f8014f94..b940bfe2a692 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -10734,7 +10734,7 @@ INTEL DRM I915 DRIVER (Meteor Lake, DG2 and older > excluding Poulsbo, Moorestown > M: Jani Nikula > M: Joonas Lahtinen > M: Rodrigo Vivi > -M: Tvrtko Ursulin > +M: Tvrtko Ursulin > L: intel-...@lists.freedesktop.org > S: Supported > W: https://drm.pages.freedesktop.org/intel-docs/ > -- > 2.40.1 >
Re: [PATCH 1/3] dt-bindings: display: mediatek: gamma: Change MT8195 to single enum group
Re: [PATCH] drm/i915/display: Save a few bytes of memory in intel_backlight_device_register()
On Fri, 23 Feb 2024, Christophe JAILLET wrote:
> 'name' may still be "intel_backlight" when backlight_device_register() is
> called.
> In such a case, using kstrdup_const() saves a memory duplication when
> dev_set_name() is called in backlight_device_register().
>
> Use kfree_const() accordingly.
>
> Signed-off-by: Christophe JAILLET
Thanks, pushed to drm-intel-next.
BR,
Jani.
> ---
> Compile tested only
> ---
> drivers/gpu/drm/i915/display/intel_backlight.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c
> b/drivers/gpu/drm/i915/display/intel_backlight.c
> index 1946d7fb3c2e..9e4a9d4f1585 100644
> --- a/drivers/gpu/drm/i915/display/intel_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_backlight.c
> @@ -949,7 +949,7 @@ int intel_backlight_device_register(struct
> intel_connector *connector)
> else
> props.power = FB_BLANK_POWERDOWN;
>
> - name = kstrdup("intel_backlight", GFP_KERNEL);
> + name = kstrdup_const("intel_backlight", GFP_KERNEL);
> if (!name)
> return -ENOMEM;
>
> @@ -963,7 +963,7 @@ int intel_backlight_device_register(struct
> intel_connector *connector)
>* compatibility. Use unique names for subsequent backlight
> devices as a
>* fallback when the default name already exists.
>*/
> - kfree(name);
> + kfree_const(name);
> name = kasprintf(GFP_KERNEL, "card%d-%s-backlight",
>i915->drm.primary->index,
> connector->base.name);
> if (!name)
> @@ -987,7 +987,7 @@ int intel_backlight_device_register(struct
> intel_connector *connector)
> connector->base.base.id, connector->base.name, name);
>
> out:
> - kfree(name);
> + kfree_const(name);
>
> return ret;
> }
--
Jani Nikula, Intel
[PATCH v2 2/3] dt-bindings: display: mediatek: gamma: Add support for MT8188
The gamma LUT setting of MT8188 and MT8195 are the same, so we create a one of items for MT8188 to reuse the driver data settings of MT8195. Signed-off-by: Jason-JH.Lin Reviewed-by: AngeloGioacchino Del Regno --- .../devicetree/bindings/display/mediatek/mediatek,gamma.yaml | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml index ef1f575757f6..b8b8e83ebc3f 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml @@ -36,6 +36,10 @@ properties: - mediatek,mt8192-disp-gamma - mediatek,mt8195-disp-gamma - const: mediatek,mt8183-disp-gamma + - items: + - enum: + - mediatek,mt8188-disp-gamma + - const: mediatek,mt8195-disp-gamma reg: maxItems: 1 -- 2.18.0
[PATCH v2 0/3] Add GAMMA 12-bit LUT support for MT8188
From: Jason-jh Lin Since MT8195 supports GAMMA 12-bit LUT after the landing of [1] series, we can now add support for MT8188. [1] MediaTek DDP GAMMA - 12-bit LUT support - https://patchwork.kernel.org/project/linux-mediatek/list/?series=792516 Change in v2: 1. Keep MT8195 compatible in the group of MT8183. 2. Move MT8195 compatible group to the end of items list. Jason-JH.Lin (3): dt-bindings: display: mediatek: gamma: Change MT8195 to single enum group dt-bindings: display: mediatek: gamma: Add support for MT8188 drm/mediatek: Add gamma support for MT8195 .../devicetree/bindings/display/mediatek/mediatek,gamma.yaml | 5 + drivers/gpu/drm/mediatek/mtk_drm_drv.c | 2 ++ 2 files changed, 7 insertions(+) -- 2.18.0
