RE: [EXT] Re: [v2 06/10] drm: bridge: cadence: Add MHDP HDMI driver for i.MX8MQ

[Sandor Yu]

Hi Alexander,

> -Original Message-
> From: Alexander Stein 
> Sent: 2022年11月10日 23:44
> To: Sandor Yu 
> Cc: jo...@kwiboo.se; dri-devel@lists.freedesktop.org;
> devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> linux-ker...@vger.kernel.org; linux-...@lists.infradead.org;
> andrzej.ha...@intel.com; neil.armstr...@linaro.org; robert.f...@linaro.org;
> laurent.pinch...@ideasonboard.com; jernej.skra...@gmail.com;
> kis...@ti.com; vk...@kernel.org; Oliver Brown ;
> krzysztof.kozlowski...@linaro.org; s...@ravnborg.org;
> tzimmerm...@suse.de; s.ha...@pengutronix.de; javierm@redhat. com
> ; penguin-ker...@i-love.sakura.ne.jp;
> robh...@kernel.org; dl-linux-imx ;
> ker...@pengutronix.de; shawn...@kernel.org; p.ya...@ti.com;
> max...@cerno.tech
> Subject: RE: [EXT] Re: [v2 06/10] drm: bridge: cadence: Add MHDP HDMI
> driver for i.MX8MQ
> 
> Caution: EXT Email
> 
> Hi Sandor,
> 
> Am Mittwoch, 9. November 2022, 14:26:14 CET schrieb Sandor Yu:
> > Thanks for your comments.
> >
> >
> > > -Original Message-
> > > From: Alexander Stein 
> > > Sent: 2022年11月8日 21:17
> > > To: jo...@kwiboo.se; Sandor Yu 
> > > Cc: dri-devel@lists.freedesktop.org; devicet...@vger.kernel.org;
> > > linux-arm-ker...@lists.infradead.org; linux-ker...@vger.kernel.org;
> > > linux-...@lists.infradead.org; andrzej.ha...@intel.com;
> > > neil.armstr...@linaro.org; robert.f...@linaro.org;
> > > laurent.pinch...@ideasonboard.com; jernej.skra...@gmail.com;
> > > kis...@ti.com; vk...@kernel.org; Oliver Brown
> > > ; krzysztof.kozlowski...@linaro.org;
> > > s...@ravnborg.org; jani.nik...@intel.com;
>  tzimmerm...@suse.de; s.ha...@pengutronix.de;
> > > javi...@redhat.com;
> > > penguin-ker...@i-love.sakura.ne.jp; robh...@kernel.org; dl-linux-imx
> > > ; ker...@pengutronix.de; Sandor Yu
> > > ; shawn...@kernel.org; p.ya...@ti.com;
> > > max...@cerno.tech
> > > Subject: [EXT] Re: [v2 06/10] drm: bridge: cadence: Add MHDP HDMI
> > > driver for i.MX8MQ
> > >
> > > Caution: EXT Email
> > >
> > > Hello,
> > >
> > > thanks for working on this and the updated version.
> > >
> > > Am Freitag, 4. November 2022, 07:44:56 CET schrieb Sandor Yu:
> > >
> > > > Add a new DRM HDMI bridge driver for Candence MHDP used in
> i.MX8MQ
> > > > SOC. MHDP IP could support HDMI or DisplayPort standards according
> > > > embedded Firmware running in the uCPU.
> > > >
> > > >
> > > >
> > > > For iMX8MQ SOC, the HDMI FW was loaded and activated by SOC ROM
> > >
> > > code.
> > >
> > > > Bootload binary included HDMI FW was required for the driver.
> > > >
> > > >
> > > >
> > > > Signed-off-by: Sandor Yu 
> > > > ---
> > > >
> > > >  drivers/gpu/drm/bridge/cadence/Kconfig|   12 +
> > > >  .../gpu/drm/bridge/cadence/cdns-hdmi-core.c   | 1038
> > >
> > > +
> > >
> > > >  2 files changed, 1050 insertions(+)  create mode 100644
> > > > drivers/gpu/drm/bridge/cadence/cdns-hdmi-core.c
> > > >
> > > >
> > > >
> > > > diff --git a/drivers/gpu/drm/bridge/cadence/Kconfig
> > > > b/drivers/gpu/drm/bridge/cadence/Kconfig index
> > > > e79ae1af3765..377452d09992
> > > > 100644
> > > > --- a/drivers/gpu/drm/bridge/cadence/Kconfig
> > > > +++ b/drivers/gpu/drm/bridge/cadence/Kconfig
> 
> [snip]
> 
> > > > +static int cdns_hdmi_get_edid_block(void *data, u8 *edid,
> > > > +   u32 block, size_t length) {
> > > > + struct cdns_mhdp_device *mhdp = data;
> > > > + u8 msg[2], reg[5], i;
> > > > + int ret;
> > > > +
> > > > + mutex_lock(>mbox_mutex);
> > > > +
> > > > + for (i = 0; i < 4; i++) {
> > >
> > >
> > > What is i? It is not used inside the loop.
> >
> > EDID data read by HDMI firmware are not guarantee 100% successful.
> > Base on experiments, try 4 times if EDID read failed.
> 
> Mh, 4 times sounds a bit too arbitrary to me. How about using a timeout in ms,
> like 50ms, for retrying to read the EDID?
> 
For EDID read failed case, the mailbox read will timeout, additional timeout in 
the loop is not necessary.
> [snip]
> 
> > > > +static int cdns_mhdp_imx_probe(struct platform_device *pdev) {
> > > > + struct device *dev = >dev;
> > > > + struct cdns_mhdp_device *mhdp;
> > > > + struct platform_device_info pdevinfo;
> > > > + struct resource *res;
> > > > + u32 reg;
> > > > + int ret;
> > > > +
> > > > + mhdp = devm_kzalloc(dev, sizeof(*mhdp), GFP_KERNEL);
> > > > + if (!mhdp)
> > > > + return -ENOMEM;
> > > > +
> > > > + mutex_init(>lock);
> > > > + mutex_init(>mbox_mutex);
> > > > +
> > > > + INIT_DELAYED_WORK(>hotplug_work,
> hotplug_work_func);
> > > > +
> > > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > > + if (!res)
> > > > + return -ENODEV;
> > > > + mhdp->regs = devm_ioremap(dev, res->start,
> resource_size(res));
> > > > + if (IS_ERR(mhdp->regs))
> > > > + return PTR_ERR(mhdp->regs);
> > >
> > >
> > > Please use devm_platform_get_and_ioremap_resource.
> >
> > Both HDMI PHY 

Re: [PATCH v8 09/14] drm: bridge: samsung-dsim: Add atomic_get_input_bus_fmts

[Jagan Teki]

On Sun, Nov 13, 2022 at 5:51 AM Marek Vasut  wrote:
>
> On 11/10/22 19:38, Jagan Teki wrote:
> > Finding the right input bus format throughout the pipeline is hard
> > so add atomic_get_input_bus_fmts callback and initialize with the
> > proper input format from list of supported output formats.
> >
> > This format can be used in pipeline for negotiating bus format between
> > the DSI-end of this bridge and the other component closer to pipeline
> > components.
> >
> > List of Pixel formats are taken from,
> > AN13573 i.MX 8/RT MIPI DSI/CSI-2, Rev. 0, 21 March 2022
> > 3.7.4 Pixel formats
> > Table 14. DSI pixel packing formats
> >
> > v8:
> > * added pixel formats supported by NXP AN13573 i.MX 8/RT MIPI DSI/CSI-2
> >
> > v7, v6, v5, v4:
> > * none
> >
> > v3:
> > * include media-bus-format.h
> >
> > v2:
> > * none
> >
> > v1:
> > * new patch
> >
> > Signed-off-by: Jagan Teki 
> > ---
> >   drivers/gpu/drm/bridge/samsung-dsim.c | 53 +++
> >   1 file changed, 53 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
> > b/drivers/gpu/drm/bridge/samsung-dsim.c
> > index 0fe153b29e4f..33e5ae9c865f 100644
> > --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> > @@ -15,6 +15,7 @@
> >   #include 
> >   #include 
> >   #include 
> > +#include 
> >   #include 
> >   #include 
> >
> > @@ -1321,6 +1322,57 @@ static void samsung_dsim_atomic_post_disable(struct 
> > drm_bridge *bridge,
> >   pm_runtime_put_sync(dsi->dev);
> >   }
> >
> > +/*
> > + * This pixel output formats list referenced from,
> > + * AN13573 i.MX 8/RT MIPI DSI/CSI-2, Rev. 0, 21 March 2022
> > + * 3.7.4 Pixel formats
> > + * Table 14. DSI pixel packing formats
> > + */
> > +static const u32 samsung_dsim_pixel_output_fmts[] = {
>
> You can also add :
>
> MEDIA_BUS_FMT_YUYV10_1X20
>
> MEDIA_BUS_FMT_YUYV12_1X24

Are these for the below formats?

"Loosely Packed Pixel Stream, 20-bit YCbCr, 4:2:2
 Packed Pixel Stream, 24-bit YCbCr, 4:2:2"
>
> > + MEDIA_BUS_FMT_UYVY8_1X16,
> > + MEDIA_BUS_FMT_RGB101010_1X30,
> > + MEDIA_BUS_FMT_RGB121212_1X36,
> > + MEDIA_BUS_FMT_RGB565_1X16,
> > + MEDIA_BUS_FMT_RGB666_1X18,
> > + MEDIA_BUS_FMT_RGB888_1X24,
> > +};
> > +
> > +static bool samsung_dsim_pixel_output_fmt_supported(u32 fmt)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(samsung_dsim_pixel_output_fmts); i++) {
> > + if (samsung_dsim_pixel_output_fmts[i] == fmt)
> > + return true;
> > + }
> > +
> > + return false;
> > +}
> > +
> > +static u32 *
> > +samsung_dsim_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> > +struct drm_bridge_state *bridge_state,
> > +struct drm_crtc_state *crtc_state,
> > +struct drm_connector_state *conn_state,
> > +u32 output_fmt,
> > +unsigned int *num_input_fmts)
> > +{
> > + u32 *input_fmts;
> > +
> > + if (!samsung_dsim_pixel_output_fmt_supported(output_fmt))
> > + return NULL;
> > +
> > + *num_input_fmts = 1;
>
> Shouldn't this be 6 ?
>
> > + input_fmts = kmalloc(sizeof(*input_fmts), GFP_KERNEL);
> > + if (!input_fmts)
> > + return NULL;
> > +
> > + input_fmts[0] = output_fmt;
>
> Shouldn't this be a list of all 6 supported pixel formats ?

Negotiation would settle to return one input_fmt from the list of
supporting output_fmts. so the num_input_fmts would be 1 rather than
the number of fmts in the supporting list. This is what I understood
from the atomic_get_input_bus_fmts API. let me know if I miss
something here.

Jagan.

Re: [PATCH] fbdev: smscufx: fix error handling code in ufx_usb_probe

[Helge Deller]

On 11/11/22 06:49, Dongliang Mu wrote:

The current error handling code in ufx_usb_probe have many unmatching
issues, e.g., missing ufx_free_usb_list, destroy_modedb label should
only include framebuffer_release, fb_dealloc_cmap only matches
fb_alloc_cmap.

My local syzkaller reports a memory leak bug:

memory leak in ufx_usb_probe

BUG: memory leak
unreferenced object 0x88802f879580 (size 128):
   comm "kworker/0:7", pid 17416, jiffies 4295067474 (age 46.710s)
   hex dump (first 32 bytes):
 80 21 7c 2e 80 88 ff ff 18 d0 d0 0c 80 88 ff ff  .!|.
 00 d0 d0 0c 80 88 ff ff e0 ff ff ff 0f 00 00 00  
   backtrace:
 [] kmalloc_trace+0x20/0x90 mm/slab_common.c:1045
 [] kmalloc include/linux/slab.h:553 [inline]
 [] kzalloc include/linux/slab.h:689 [inline]
 [] ufx_alloc_urb_list drivers/video/fbdev/smscufx.c:1873 
[inline]
 [] ufx_usb_probe+0x11c/0x15a0 
drivers/video/fbdev/smscufx.c:1655
 [] usb_probe_interface+0x177/0x370 
drivers/usb/core/driver.c:396
 [] call_driver_probe drivers/base/dd.c:560 [inline]
 [] really_probe+0x12d/0x390 drivers/base/dd.c:639
 [] __driver_probe_device+0xbf/0x140 drivers/base/dd.c:778
 [] driver_probe_device+0x2a/0x120 drivers/base/dd.c:808
 [] __device_attach_driver+0xf7/0x150 
drivers/base/dd.c:936
 [] bus_for_each_drv+0xb7/0x100 drivers/base/bus.c:427
 [] __device_attach+0x105/0x2d0 drivers/base/dd.c:1008
 [] bus_probe_device+0xc6/0xe0 drivers/base/bus.c:487
 [] device_add+0x642/0xdc0 drivers/base/core.c:3517
 [] usb_set_configuration+0x8ef/0xb80 
drivers/usb/core/message.c:2170
 [] usb_generic_driver_probe+0x8c/0xc0 
drivers/usb/core/generic.c:238
 [] usb_probe_device+0x5c/0x140 
drivers/usb/core/driver.c:293
 [] call_driver_probe drivers/base/dd.c:560 [inline]
 [] really_probe+0x12d/0x390 drivers/base/dd.c:639
 [] __driver_probe_device+0xbf/0x140 drivers/base/dd.c:778

Fix this bug by rewriting the error handling code in ufx_usb_probe.

Reported-by: syzkaller 
Tested-by: Dongliang Mu 
Signed-off-by: Dongliang Mu 


applied.
Thanks!
Helge


---
  drivers/video/fbdev/smscufx.c | 46 +++
  1 file changed, 31 insertions(+), 15 deletions(-)

diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c
index 9343b7a4ac89..2ad6e98ce10d 100644
--- a/drivers/video/fbdev/smscufx.c
+++ b/drivers/video/fbdev/smscufx.c
@@ -1622,7 +1622,7 @@ static int ufx_usb_probe(struct usb_interface *interface,
struct usb_device *usbdev;
struct ufx_data *dev;
struct fb_info *info;
-   int retval;
+   int retval = -ENOMEM;
u32 id_rev, fpga_rev;

/* usb initialization */
@@ -1654,15 +1654,17 @@ static int ufx_usb_probe(struct usb_interface 
*interface,

if (!ufx_alloc_urb_list(dev, WRITES_IN_FLIGHT, MAX_TRANSFER)) {
dev_err(dev->gdev, "ufx_alloc_urb_list failed\n");
-   goto e_nomem;
+   goto put_ref;
}

/* We don't register a new USB class. Our client interface is fbdev */

/* allocates framebuffer driver structure, not framebuffer memory */
info = framebuffer_alloc(0, >dev);
-   if (!info)
-   goto e_nomem;
+   if (!info) {
+   dev_err(dev->gdev, "framebuffer_alloc failed\n");
+   goto free_urb_list;
+   }

dev->info = info;
info->par = dev;
@@ -1705,22 +1707,34 @@ static int ufx_usb_probe(struct usb_interface 
*interface,
check_warn_goto_error(retval, "unable to find common mode for display and 
adapter");

retval = ufx_reg_set_bits(dev, 0x4000, 0x0001);
-   check_warn_goto_error(retval, "error %d enabling graphics engine", 
retval);
+   if (retval < 0) {
+   dev_err(dev->gdev, "error %d enabling graphics engine", retval);
+   goto setup_modes;
+   }

/* ready to begin using device */
atomic_set(>usb_active, 1);

dev_dbg(dev->gdev, "checking var");
retval = ufx_ops_check_var(>var, info);
-   check_warn_goto_error(retval, "error %d ufx_ops_check_var", retval);
+   if (retval < 0) {
+   dev_err(dev->gdev, "error %d ufx_ops_check_var", retval);
+   goto reset_active;
+   }

dev_dbg(dev->gdev, "setting par");
retval = ufx_ops_set_par(info);
-   check_warn_goto_error(retval, "error %d ufx_ops_set_par", retval);
+   if (retval < 0) {
+   dev_err(dev->gdev, "error %d ufx_ops_set_par", retval);
+   goto reset_active;
+   }

dev_dbg(dev->gdev, "registering framebuffer");
retval = register_framebuffer(info);
-   check_warn_goto_error(retval, "error %d register_framebuffer", retval);
+   if (retval < 0) {
+   dev_err(dev->gdev, "error %d register_framebuffer", retval);
+   goto reset_active;
+   }

dev_info(dev->gdev, "SMSC UDX 

Re: [PATCH] video: fbdev: via: Fix error in via_core_init()

[Helge Deller]

On 11/14/22 02:08, Shang XiaoJing wrote:

via_core_init() won't exit the driver when pci_register_driver() failed.
Exit the viafb-i2c and the viafb-gpio in failed path to prevent error.

VIA Graphics Integration Chipset framebuffer 2.4 initializing
Error: Driver 'viafb-i2c' is already registered, aborting...
Error: Driver 'viafb-gpio' is already registered, aborting...

Fixes: 7582eb9be85f ("viafb: Turn GPIO and i2c into proper platform devices")
Signed-off-by: Shang XiaoJing 


applied.
Thanks!
Helge


---
  drivers/video/fbdev/via/via-core.c | 10 +-
  1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/via/via-core.c 
b/drivers/video/fbdev/via/via-core.c
index 2ee8fcae08df..b2e3b5df38cd 100644
--- a/drivers/video/fbdev/via/via-core.c
+++ b/drivers/video/fbdev/via/via-core.c
@@ -730,7 +730,15 @@ static int __init via_core_init(void)
return ret;
viafb_i2c_init();
viafb_gpio_init();
-   return pci_register_driver(_driver);
+   ret = pci_register_driver(_driver);
+   if (ret) {
+   viafb_gpio_exit();
+   viafb_i2c_exit();
+   viafb_exit();
+   return ret;
+   }
+
+   return 0;
  }

  static void __exit via_core_exit(void)

Re: [PATCH v2] udmabuf: add vmap method to udmabuf_ops

[Lukasz Wiecaszek]

On Sun, Nov 13, 2022 at 07:35:20PM +0300, Dmitry Osipenko wrote:
> On 11/13/22 18:05, Lukasz Wiecaszek wrote:
> > +static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map)
> > +{
> > +   struct udmabuf *ubuf = buf->priv;
> > +
> > +   if (!ubuf->vaddr) {
> > +   ubuf->vaddr = vm_map_ram(ubuf->pages, ubuf->pagecount, -1);
> > +   if (!ubuf->vaddr)
> > +   return -EINVAL;
> > +   }
> 
> Create a new mapping on each vmap_udmabuf() and add the corresponding
> vunmap.
> 
> Otherwise persistent vmapping shall be released together with udmabuf.
> It doesn't look that persistent vmapping is needed for udmabufs.
> 
> -- 
> Best regards,
> Dmitry

Right. Thanks for review and remarks. Adding vunmap sounds reasonable to
me. Will add it somehow this week.

Regards,
Lukasz

Re: [PATCH v8 01/14] drm: exynos: dsi: Fix MIPI_DSI*_NO_* mode flags

[Marek Vasut]

On 11/14/22 02:11, Nicolas Boichat wrote:

On Sun, Nov 13, 2022 at 8:29 AM Marek Vasut  wrote:


On 11/11/22 13:12, Nicolas Boichat wrote:

[...]


BTW, are you sure DSIM_HSE_MODE is correct now?


Yes, we have tested in imx8m platforms as well. Sébastien Szymanski
initially observed this issue on the imx8m platform.


I'll repeat, are you sure about HSE specifically? You invert the
polarity for HBP, HFP, and HSA, which makes sense given your patch
02/14.

I'm concerned about HSE. Is the bit really a disable bit?
MIPI_DSI_MODE_VIDEO_HSE is supposed to be an enable flag, so you
should not do `reg |= DSIM_HSE_DISABLE;`, probably.


I suspect the HSE bit is a misnomer, but its handling in the driver is
correct.

i.MX 8M Plus Applications Processor Reference Manual, Rev. 1, 06/2021
Page 5436

23 HseDisableMode

In Vsync pulse and Vporch area, MIPI DSI master transfers only Hsync
start packet to MIPI DSI slave at MIPI DSI spec 1.1r02. This bit
transfers Hsync end packet in Vsync pulse and Vporch area (optional).

0 = Disables transfer
1 = Enables transfer

In command mode, this bit is ignored.


Okay. I'd suggest adding a comment in the code, it'd be so tempting to
attempt to "fix" this as the if/or pattern looks different from the
others.

But it's up to you all.


I agree. Clearly the discrepancy is confusing and leads to mistakes.

Re: [syzbot] possible deadlock in vfs_fileattr_set

[syzbot]

syzbot has bisected this issue to:

commit 6dd6b7643e723b4779e59c8ad97bd5db6ff3bb12
Author: Thomas Zimmermann 
Date:   Mon Jan 18 13:14:19 2021 +

drm/vmwgfx: Remove reference to struct drm_device.pdev

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=1418e6a588
start commit:   f8f60f322f06 Add linux-next specific files for 2022
git tree:   linux-next
final oops: https://syzkaller.appspot.com/x/report.txt?x=1618e6a588
console output: https://syzkaller.appspot.com/x/log.txt?x=1218e6a588
kernel config:  https://syzkaller.appspot.com/x/.config?x=85ba52c07cd97289
dashboard link: https://syzkaller.appspot.com/bug?extid=abe01a74653f00aabe3e
syz repro:  https://syzkaller.appspot.com/x/repro.syz?x=138b76ae88
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=16ab1bfe88

Reported-by: syzbot+abe01a74653f00aab...@syzkaller.appspotmail.com
Fixes: 6dd6b7643e72 ("drm/vmwgfx: Remove reference to struct drm_device.pdev")

For information about bisection process see: https://goo.gl/tpsmEJ#bisection

Re: [PATCH v8 01/14] drm: exynos: dsi: Fix MIPI_DSI*_NO_* mode flags

[Nicolas Boichat]

On Sun, Nov 13, 2022 at 8:29 AM Marek Vasut  wrote:
>
> On 11/11/22 13:12, Nicolas Boichat wrote:
>
> [...]
>
> >>> BTW, are you sure DSIM_HSE_MODE is correct now?
> >>
> >> Yes, we have tested in imx8m platforms as well. Sébastien Szymanski
> >> initially observed this issue on the imx8m platform.
> >
> > I'll repeat, are you sure about HSE specifically? You invert the
> > polarity for HBP, HFP, and HSA, which makes sense given your patch
> > 02/14.
> >
> > I'm concerned about HSE. Is the bit really a disable bit?
> > MIPI_DSI_MODE_VIDEO_HSE is supposed to be an enable flag, so you
> > should not do `reg |= DSIM_HSE_DISABLE;`, probably.
>
> I suspect the HSE bit is a misnomer, but its handling in the driver is
> correct.
>
> i.MX 8M Plus Applications Processor Reference Manual, Rev. 1, 06/2021
> Page 5436
>
> 23 HseDisableMode
>
> In Vsync pulse and Vporch area, MIPI DSI master transfers only Hsync
> start packet to MIPI DSI slave at MIPI DSI spec 1.1r02. This bit
> transfers Hsync end packet in Vsync pulse and Vporch area (optional).
>
> 0 = Disables transfer
> 1 = Enables transfer
>
> In command mode, this bit is ignored.

Okay. I'd suggest adding a comment in the code, it'd be so tempting to
attempt to "fix" this as the if/or pattern looks different from the
others.

But it's up to you all.

Thanks,

Re: [PATCH 1/2] drm/vc4: hdmi: Enforce the minimum rate at runtime_resume

[Stefan Wahren]

Am 11.11.22 um 22:08 schrieb Stefan Wahren:

Hi Maxime,

Am 29.09.22 um 11:21 schrieb Maxime Ripard:

This is a revert of commit fd5894fa2413 ("drm/vc4: hdmi: Remove clock
rate initialization"), with the code slightly moved around.

It turns out that we can't downright remove that code from the driver,
since the Pi0-3 and Pi4 are in different cases, and it only works for
the Pi4.

Indeed, the commit mentioned above was relying on the RaspberryPi
firmware clocks driver to initialize the rate if it wasn't done by the
firmware. However, the Pi0-3 are using the clk-bcm2835 clock driver that
wasn't doing this initialization. We therefore end up with the clock not
being assigned a rate, and the CPU stalling when trying to access a
register.

We can't move that initialization in the clk-bcm2835 driver, since the
HSM clock we depend on is actually part of the HDMI power domain, so any
rate setup is only valid when the power domain is enabled. Thus, we
reinstated the minimum rate setup at runtime_suspend, which should
address both issues.

Link: 
https://lore.kernel.org/dri-devel/20220922145448.w3xfywkn5ecak...@pengutronix.de/

Fixes: fd5894fa2413 ("drm/vc4: hdmi: Remove clock rate initialization")
Reported-by: Marc Kleine-Budde 
Signed-off-by: Maxime Ripard 
---
  drivers/gpu/drm/vc4/vc4_hdmi.c | 9 +
  1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c 
b/drivers/gpu/drm/vc4/vc4_hdmi.c

index 199bc398817f..2e28fe16ed5e 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -2891,6 +2891,15 @@ static int vc4_hdmi_runtime_resume(struct 
device *dev)

  u32 __maybe_unused value;
  int ret;
  +    /*
+ * The HSM clock is in the HDMI power domain, so we need to set
+ * its frequency while the power domain is active so that it
+ * keeps its rate.
+ */
+    ret = clk_set_min_rate(vc4_hdmi->hsm_clock, HSM_MIN_CLOCK_FREQ);
+    if (ret)
+    return ret;
+


unfortunately this breaks X on Raspberry Pi 4 in Linux 6.0.5 
(multi_v7_defconfig + LPAE). Today i saw this report [1] and bisected 
the issue down to this patch. Shame on me that i only tested this 
patch with Rpi 3B+ :-(
Looks like "drm/vc4: hdmi: Fix HSM clock too low on Pi4" addresses this 
issue ...


Best regards

[1] - https://bugzilla.suse.com/show_bug.cgi?id=1205259


  ret = clk_prepare_enable(vc4_hdmi->hsm_clock);
  if (ret)
  return ret;

Re: [PATCH 06/26] drm: sun4i: Use the dev_pm_ops provided by modeset helper

[Samuel Holland]

On 11/7/22 11:50, Paul Cercueil wrote:
> Use the drm_mode_config_pm_ops structure exported by
> drm_modeset_helper.c, which provides the exact same PM callbacks.
> 
> Signed-off-by: Paul Cercueil 
> ---
> Cc: Maxime Ripard 
> Cc: Chen-Yu Tsai 
> Cc: Jernej Skrabec 
> Cc: Samuel Holland 
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-su...@lists.linux.dev
> ---
>  drivers/gpu/drm/sun4i/sun4i_drv.c | 24 ++--
>  1 file changed, 2 insertions(+), 22 deletions(-)

Reviewed-by: Samuel Holland 

Re: [PATCH] drm/mediatek: Fix return type of mtk_hdmi_bridge_mode_valid()

[Chun-Kuang Hu]

 Hi, Nathan:

Nathan Chancellor  於 2022年11月2日 週三 晚上11:47寫道：
>
> With clang's kernel control flow integrity (kCFI, CONFIG_CFI_CLANG),
> indirect call targets are validated against the expected function
> pointer prototype to make sure the call target is valid to help mitigate
> ROP attacks. If they are not identical, there is a failure at run time,
> which manifests as either a kernel panic or thread getting killed. A
> proposed warning in clang aims to catch these at compile time, which
> reveals:
>
>   drivers/gpu/drm/mediatek/mtk_hdmi.c:1407:16: error: incompatible function 
> pointer types initializing 'enum drm_mode_status (*)(struct drm_bridge *, 
> const struct drm_display_info *, const struct drm_display_mode *)' with an 
> expression of type 'int (struct drm_bridge *, const struct drm_display_info 
> *, const struct drm_display_mode *)' 
> [-Werror,-Wincompatible-function-pointer-types-strict]
>   .mode_valid = mtk_hdmi_bridge_mode_valid,
> ^~
>   1 error generated.
>
> ->mode_valid() in 'struct drm_bridge_funcs' expects a return type of
> 'enum drm_mode_status', not 'int'. Adjust the return type of
> mtk_hdmi_bridge_mode_valid() to match the prototype's to resolve the
> warning and CFI failure.

Applied to mediatek-drm-next [1], thanks.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next

Chun-Kuang.

>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1750
> Reported-by: Sami Tolvanen 
> Signed-off-by: Nathan Chancellor 
> ---
>  drivers/gpu/drm/mediatek/mtk_hdmi.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c 
> b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> index 4c80b6896dc3..6e8f99554f54 100644
> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> @@ -1202,9 +1202,10 @@ static enum drm_connector_status 
> mtk_hdmi_detect(struct mtk_hdmi *hdmi)
> return mtk_hdmi_update_plugged_status(hdmi);
>  }
>
> -static int mtk_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
> - const struct drm_display_info *info,
> - const struct drm_display_mode *mode)
> +static enum drm_mode_status
> +mtk_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
> +  const struct drm_display_info *info,
> +  const struct drm_display_mode *mode)
>  {
> struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge);
> struct drm_bridge *next_bridge;
>
> base-commit: 9abf2313adc1ca1b6180c508c25f22f9395cc780
> --
> 2.38.1
>

Re: [PATCH v2] drm/mediatek: make eDP panel as the first connected connector

[Chun-Kuang Hu]

Hi Gil:

Gil Dekel  於 2022年11月11日 週五 凌晨2:49寫道：
>
> [Why]
> Some userspaces assume that the first connected connector is the "main"
> display, which supposed to display, for example, the login screen.
> For laptops, this should be the internal connector.
>
> [How]
> This patch calls drm_helper_move_panel_connectors_to_head() right before
> crtc creation to ensure internal connectors are at the top of the
> connector list.
>
> Tested by ensuring the internal panels are at the top of the connector
> list via modetest -c.
>
> This patch does to mediatek what the following patch
> https://www.spinics.net/lists/stable/msg590605.html
> did for qualcomm.

Applied to mediatek-drm-next [1], thanks.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next

Regards,
Chun-Kuang.

>
> Signed-off-by: Gil Dekel 
> Tested-by: Gil Dekel 
> ---
> v2: Fix copy-paste errors in commit message so it's relevant for this
> patch and the mediatek driver.
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c 
> b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 91f58db5915f..76daaf6a0945 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -386,6 +386,12 @@ static int mtk_drm_kms_init(struct drm_device *drm)
> if (ret)
> goto put_mutex_dev;
>
> +   /*
> +* Ensure internal panels are at the top of the connector list before
> +* crtc creation.
> +*/
> +   drm_helper_move_panel_connectors_to_head(drm);
> +
> /*
>  * We currently support two fixed data streams, each optional,
>  * and each statically assigned to a crtc:
> --
> Gil Dekel, Software Engineer, Google / ChromeOS Display and Graphics

[PATCH v1] drm/gem-vram: Fix deadlock in drm_gem_vram_vmap()

[Dmitry Osipenko]

Recently DRM framebuffer core and all drivers were moved to unlocked
vmapping functions that take the reservation lock. The drm_gem_vram_vmap()
was missed out by accident and now deadlocks drivers that use drm_gem_vram
helpers when framebuffer is updated, like Bochs driver. Remove the locking
from drm_gem_vram_v[un]map() functions to fix the deadlock.

Reported-by: Dmitry Vyukov 
Fixes: 79e2cf2e7a19 ("drm/gem: Take reservation lock for vmap/vunmap 
operations")
Signed-off-by: Dmitry Osipenko 
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 18 +++---
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c 
b/drivers/gpu/drm/drm_gem_vram_helper.c
index 125160b534be..b6c7e3803bb3 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -433,25 +433,19 @@ int drm_gem_vram_vmap(struct drm_gem_vram_object *gbo, 
struct iosys_map *map)
 {
int ret;
 
-   ret = ttm_bo_reserve(>bo, true, false, NULL);
-   if (ret)
-   return ret;
+   dma_resv_assert_held(gbo->bo.base.resv);
 
ret = drm_gem_vram_pin_locked(gbo, 0);
if (ret)
-   goto err_ttm_bo_unreserve;
+   return ret;
ret = drm_gem_vram_kmap_locked(gbo, map);
if (ret)
goto err_drm_gem_vram_unpin_locked;
 
-   ttm_bo_unreserve(>bo);
-
return 0;
 
 err_drm_gem_vram_unpin_locked:
drm_gem_vram_unpin_locked(gbo);
-err_ttm_bo_unreserve:
-   ttm_bo_unreserve(>bo);
return ret;
 }
 EXPORT_SYMBOL(drm_gem_vram_vmap);
@@ -467,16 +461,10 @@ EXPORT_SYMBOL(drm_gem_vram_vmap);
 void drm_gem_vram_vunmap(struct drm_gem_vram_object *gbo,
 struct iosys_map *map)
 {
-   int ret;
-
-   ret = ttm_bo_reserve(>bo, false, false, NULL);
-   if (WARN_ONCE(ret, "ttm_bo_reserve_failed(): ret=%d\n", ret))
-   return;
+   dma_resv_assert_held(gbo->bo.base.resv);
 
drm_gem_vram_kunmap_locked(gbo, map);
drm_gem_vram_unpin_locked(gbo);
-
-   ttm_bo_unreserve(>bo);
 }
 EXPORT_SYMBOL(drm_gem_vram_vunmap);
 
-- 
2.37.3

Re: [PATCH v4] drm: mediatek: Modify dpi power on/off sequence.

[Chun-Kuang Hu]

Hi, Xinlei:

 於 2022年11月9日 週三 下午6:01寫道：
>
> From: Xinlei Lee 
>
> Modify dpi power on/off sequence so that the first gpio operation will take 
> effect.

Applied to mediatek-drm-next [1], thanks.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next

Regards,
Chun-Kuang.

>
> Fixes: 6bd4763fd532 ("drm/mediatek: set dpi pin mode to gpio low to avoid 
> leakage current")
> Signed-off-by: Xinlei Lee 
> ---
> change note:
> v3: Moved pull-down pin control after mtk_dpi_power_off.
>
> v2: Remove the empty line between Fixes: and S-o-b.
>
> v1: Rebase on linus/master v6.1-rc1. Change nothing.
>
> Because dpi power_on/off is protected by dpi->refcount, the first time
> it cannot be powered on and off successfully, it will cause leakage.
> ---
> ---
>  drivers/gpu/drm/mediatek/mtk_dpi.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c 
> b/drivers/gpu/drm/mediatek/mtk_dpi.c
> index 508a6d994e83..1f5d39a4077c 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> @@ -461,9 +461,6 @@ static void mtk_dpi_power_off(struct mtk_dpi *dpi)
> if (--dpi->refcount != 0)
> return;
>
> -   if (dpi->pinctrl && dpi->pins_gpio)
> -   pinctrl_select_state(dpi->pinctrl, dpi->pins_gpio);
> -
> mtk_dpi_disable(dpi);
> clk_disable_unprepare(dpi->pixel_clk);
> clk_disable_unprepare(dpi->engine_clk);
> @@ -488,9 +485,6 @@ static int mtk_dpi_power_on(struct mtk_dpi *dpi)
> goto err_pixel;
> }
>
> -   if (dpi->pinctrl && dpi->pins_dpi)
> -   pinctrl_select_state(dpi->pinctrl, dpi->pins_dpi);
> -
> return 0;
>
>  err_pixel:
> @@ -721,12 +715,18 @@ static void mtk_dpi_bridge_disable(struct drm_bridge 
> *bridge)
> struct mtk_dpi *dpi = bridge_to_dpi(bridge);
>
> mtk_dpi_power_off(dpi);
> +
> +   if (dpi->pinctrl && dpi->pins_gpio)
> +   pinctrl_select_state(dpi->pinctrl, dpi->pins_gpio);
>  }
>
>  static void mtk_dpi_bridge_enable(struct drm_bridge *bridge)
>  {
> struct mtk_dpi *dpi = bridge_to_dpi(bridge);
>
> +   if (dpi->pinctrl && dpi->pins_dpi)
> +   pinctrl_select_state(dpi->pinctrl, dpi->pins_dpi);
> +
> mtk_dpi_power_on(dpi);
> mtk_dpi_set_display_mode(dpi, >mode);
> mtk_dpi_enable(dpi);
> --
> 2.18.0
>

linux-next: manual merge of the drm-intel tree with Linus' tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the drm-intel tree got a conflict in:

  drivers/gpu/drm/i915/display/intel_backlight.c

between commit:

  b1d36e73cc1c ("drm/i915: Don't register backlight when another backlight 
should be used (v2)")

from Linus' tree and commit:

  801543b2593b ("drm/i915: stop including i915_irq.h from i915_trace.h")

from the drm-intel tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/gpu/drm/i915/display/intel_backlight.c
index beba39a38c87,0438071f58cf..
--- a/drivers/gpu/drm/i915/display/intel_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_backlight.c
@@@ -8,8 -8,7 +8,9 @@@
  #include 
  #include 
  
 +#include 
 +
+ #include "i915_reg.h"
  #include "intel_backlight.h"
  #include "intel_backlight_regs.h"
  #include "intel_connector.h"


pgpiaQQ8lEeNw.pgp
Description: OpenPGP digital signature

Re: dealock in drm_fb_helper_damage_work

[Dmitry Osipenko]

On 11/13/22 23:48, Dmitry Vyukov wrote:
>> Hi,
>>
>> I am getting the following deadlock on reservation_ww_class_mutex
>> while trying to boot next-2022 kernel:
> The code is recently added by this commit:
> 
> commit 79e2cf2e7a193473dfb0da3b9b869682b43dc60f
> Author: Dmitry Osipenko 
> Date:   Mon Oct 17 20:22:11 2022 +0300
> drm/gem: Take reservation lock for vmap/vunmap operations

Thanks for the report. I reproduced this problem using bochs driver,
will send the fix ASAP.

-- 
Best regards,
Dmitry

Re: dealock in drm_fb_helper_damage_work

[Dmitry Vyukov]

On Sun, 13 Nov 2022 at 21:42, Dmitry Vyukov  wrote:
>
> Hi,
>
> I am getting the following deadlock on reservation_ww_class_mutex
> while trying to boot next-2022 kernel:

The code is recently added by this commit:

commit 79e2cf2e7a193473dfb0da3b9b869682b43dc60f
Author: Dmitry Osipenko 
Date:   Mon Oct 17 20:22:11 2022 +0300
drm/gem: Take reservation lock for vmap/vunmap operations

> 
> WARNING: possible recursive locking detected
> 6.1.0-rc4-next-2022 #193 Not tainted
> 
> kworker/4:1/81 is trying to acquire lock:
> 88812ebe89a8 (reservation_ww_class_mutex){+.+.}-{3:3}, at:
> dma_resv_lock_interruptible include/linux/dma-resv.h:372 [inline]
> 88812ebe89a8 (reservation_ww_class_mutex){+.+.}-{3:3}, at:
> ttm_bo_reserve include/drm/ttm/ttm_bo_driver.h:121 [inline]
> 88812ebe89a8 (reservation_ww_class_mutex){+.+.}-{3:3}, at:
> drm_gem_vram_vmap+0xa4/0x590 drivers/gpu/drm/drm_gem_vram_helper.c:436
>
> but task is already holding lock:
> 88812ebe89a8 (reservation_ww_class_mutex){+.+.}-{3:3}, at:
> dma_resv_lock include/linux/dma-resv.h:345 [inline]
> 88812ebe89a8 (reservation_ww_class_mutex){+.+.}-{3:3}, at:
> drm_gem_vmap_unlocked+0x3f/0xa0 drivers/gpu/drm/drm_gem.c:1195
>
> other info that might help us debug this:
>  Possible unsafe locking scenario:
>
>CPU0
>
>   lock(reservation_ww_class_mutex);
>   lock(reservation_ww_class_mutex);
>
>  *** DEADLOCK ***
>
>  May be due to missing lock nesting notation
>
> 4 locks held by kworker/4:1/81:
>  #0: 888100078d38 ((wq_completion)events){+.+.}-{0:0}, at:
> arch_atomic64_set arch/x86/include/asm/atomic64_64.h:34 [inline]
>  #0: 888100078d38 ((wq_completion)events){+.+.}-{0:0}, at:
> arch_atomic_long_set include/linux/atomic/atomic-long.h:41 [inline]
>  #0: 888100078d38 ((wq_completion)events){+.+.}-{0:0}, at:
> atomic_long_set include/linux/atomic/atomic-instrumented.h:1280
> [inline]
>  #0: 888100078d38 ((wq_completion)events){+.+.}-{0:0}, at:
> set_work_data kernel/workqueue.c:636 [inline]
>  #0: 888100078d38 ((wq_completion)events){+.+.}-{0:0}, at:
> set_work_pool_and_clear_pending kernel/workqueue.c:663 [inline]
>  #0: 888100078d38 ((wq_completion)events){+.+.}-{0:0}, at:
> process_one_work+0x8e4/0x1720 kernel/workqueue.c:2260
>  #1: c9000694fda0
> ((work_completion)(>damage_work)){+.+.}-{0:0}, at:
> process_one_work+0x918/0x1720 kernel/workqueue.c:2264
>  #2: 88812ebe8278 (>lock){+.+.}-{3:3}, at:
> drm_fbdev_damage_blit drivers/gpu/drm/drm_fbdev_generic.c:312 [inline]
>  #2: 88812ebe8278 (>lock){+.+.}-{3:3}, at:
> drm_fbdev_fb_dirty+0x30e/0xcd0 drivers/gpu/drm/drm_fbdev_generic.c:342
>  #3: 88812ebe89a8 (reservation_ww_class_mutex){+.+.}-{3:3}, at:
> dma_resv_lock include/linux/dma-resv.h:345 [inline]
>  #3: 88812ebe89a8 (reservation_ww_class_mutex){+.+.}-{3:3}, at:
> drm_gem_vmap_unlocked+0x3f/0xa0 drivers/gpu/drm/drm_gem.c:1195
>
> stack backtrace:
> CPU: 4 PID: 81 Comm: kworker/4:1 Not tainted 6.1.0-rc4-next-2022 #193
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
> rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
> Workqueue: events drm_fb_helper_damage_work
> Call Trace:
>  
>  __dump_stack lib/dump_stack.c:88 [inline]
>  dump_stack_lvl+0x100/0x178 lib/dump_stack.c:106
>  print_deadlock_bug kernel/locking/lockdep.c:2990 [inline]
>  check_deadlock kernel/locking/lockdep.c:3033 [inline]
>  validate_chain kernel/locking/lockdep.c:3818 [inline]
>  __lock_acquire.cold+0x119/0x3b9 kernel/locking/lockdep.c:5055
>  lock_acquire kernel/locking/lockdep.c:5668 [inline]
>  lock_acquire+0x1e0/0x610 kernel/locking/lockdep.c:5633
>  __mutex_lock_common kernel/locking/mutex.c:603 [inline]
>  __ww_mutex_lock.constprop.0+0x1ba/0x2ee0 kernel/locking/mutex.c:754
>  ww_mutex_lock_interruptible+0x37/0x140 kernel/locking/mutex.c:886
>  dma_resv_lock_interruptible include/linux/dma-resv.h:372 [inline]
>  ttm_bo_reserve include/drm/ttm/ttm_bo_driver.h:121 [inline]
>  drm_gem_vram_vmap+0xa4/0x590 drivers/gpu/drm/drm_gem_vram_helper.c:436
>  drm_gem_vmap+0xc5/0x1b0 drivers/gpu/drm/drm_gem.c:1166
>  drm_gem_vmap_unlocked+0x4a/0xa0 drivers/gpu/drm/drm_gem.c:1196
>  drm_client_buffer_vmap+0x45/0xd0 drivers/gpu/drm/drm_client.c:326
>  drm_fbdev_damage_blit drivers/gpu/drm/drm_fbdev_generic.c:314 [inline]
>  drm_fbdev_fb_dirty+0x31e/0xcd0 drivers/gpu/drm/drm_fbdev_generic.c:342
>  drm_fb_helper_damage_work+0x27a/0x5d0 drivers/gpu/drm/drm_fb_helper.c:388
>  process_one_work+0xa33/0x1720 kernel/workqueue.c:2289
>  worker_thread+0x67d/0x10e0 kernel/workqueue.c:2436
>  kthread+0x2e4/0x3a0 kernel/kthread.c:376
>  ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:308
>  
>
> The config is:
> https://gist.githubusercontent.com/dvyukov/2897b21db809075a22db0370c495ed2d/raw/9b2535b2ba77bb57e4f1ba2b909ad4075b6e2c6a/gistfile1.txt
>
> Qemu command line:
> qemu-system-x86_64 -enable-kvm 

dealock in drm_fb_helper_damage_work

[Dmitry Vyukov]

Hi,

I am getting the following deadlock on reservation_ww_class_mutex
while trying to boot next-2022 kernel:


WARNING: possible recursive locking detected
6.1.0-rc4-next-2022 #193 Not tainted

kworker/4:1/81 is trying to acquire lock:
88812ebe89a8 (reservation_ww_class_mutex){+.+.}-{3:3}, at:
dma_resv_lock_interruptible include/linux/dma-resv.h:372 [inline]
88812ebe89a8 (reservation_ww_class_mutex){+.+.}-{3:3}, at:
ttm_bo_reserve include/drm/ttm/ttm_bo_driver.h:121 [inline]
88812ebe89a8 (reservation_ww_class_mutex){+.+.}-{3:3}, at:
drm_gem_vram_vmap+0xa4/0x590 drivers/gpu/drm/drm_gem_vram_helper.c:436

but task is already holding lock:
88812ebe89a8 (reservation_ww_class_mutex){+.+.}-{3:3}, at:
dma_resv_lock include/linux/dma-resv.h:345 [inline]
88812ebe89a8 (reservation_ww_class_mutex){+.+.}-{3:3}, at:
drm_gem_vmap_unlocked+0x3f/0xa0 drivers/gpu/drm/drm_gem.c:1195

other info that might help us debug this:
 Possible unsafe locking scenario:

   CPU0
   
  lock(reservation_ww_class_mutex);
  lock(reservation_ww_class_mutex);

 *** DEADLOCK ***

 May be due to missing lock nesting notation

4 locks held by kworker/4:1/81:
 #0: 888100078d38 ((wq_completion)events){+.+.}-{0:0}, at:
arch_atomic64_set arch/x86/include/asm/atomic64_64.h:34 [inline]
 #0: 888100078d38 ((wq_completion)events){+.+.}-{0:0}, at:
arch_atomic_long_set include/linux/atomic/atomic-long.h:41 [inline]
 #0: 888100078d38 ((wq_completion)events){+.+.}-{0:0}, at:
atomic_long_set include/linux/atomic/atomic-instrumented.h:1280
[inline]
 #0: 888100078d38 ((wq_completion)events){+.+.}-{0:0}, at:
set_work_data kernel/workqueue.c:636 [inline]
 #0: 888100078d38 ((wq_completion)events){+.+.}-{0:0}, at:
set_work_pool_and_clear_pending kernel/workqueue.c:663 [inline]
 #0: 888100078d38 ((wq_completion)events){+.+.}-{0:0}, at:
process_one_work+0x8e4/0x1720 kernel/workqueue.c:2260
 #1: c9000694fda0
((work_completion)(>damage_work)){+.+.}-{0:0}, at:
process_one_work+0x918/0x1720 kernel/workqueue.c:2264
 #2: 88812ebe8278 (>lock){+.+.}-{3:3}, at:
drm_fbdev_damage_blit drivers/gpu/drm/drm_fbdev_generic.c:312 [inline]
 #2: 88812ebe8278 (>lock){+.+.}-{3:3}, at:
drm_fbdev_fb_dirty+0x30e/0xcd0 drivers/gpu/drm/drm_fbdev_generic.c:342
 #3: 88812ebe89a8 (reservation_ww_class_mutex){+.+.}-{3:3}, at:
dma_resv_lock include/linux/dma-resv.h:345 [inline]
 #3: 88812ebe89a8 (reservation_ww_class_mutex){+.+.}-{3:3}, at:
drm_gem_vmap_unlocked+0x3f/0xa0 drivers/gpu/drm/drm_gem.c:1195

stack backtrace:
CPU: 4 PID: 81 Comm: kworker/4:1 Not tainted 6.1.0-rc4-next-2022 #193
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
Workqueue: events drm_fb_helper_damage_work
Call Trace:
 
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0x100/0x178 lib/dump_stack.c:106
 print_deadlock_bug kernel/locking/lockdep.c:2990 [inline]
 check_deadlock kernel/locking/lockdep.c:3033 [inline]
 validate_chain kernel/locking/lockdep.c:3818 [inline]
 __lock_acquire.cold+0x119/0x3b9 kernel/locking/lockdep.c:5055
 lock_acquire kernel/locking/lockdep.c:5668 [inline]
 lock_acquire+0x1e0/0x610 kernel/locking/lockdep.c:5633
 __mutex_lock_common kernel/locking/mutex.c:603 [inline]
 __ww_mutex_lock.constprop.0+0x1ba/0x2ee0 kernel/locking/mutex.c:754
 ww_mutex_lock_interruptible+0x37/0x140 kernel/locking/mutex.c:886
 dma_resv_lock_interruptible include/linux/dma-resv.h:372 [inline]
 ttm_bo_reserve include/drm/ttm/ttm_bo_driver.h:121 [inline]
 drm_gem_vram_vmap+0xa4/0x590 drivers/gpu/drm/drm_gem_vram_helper.c:436
 drm_gem_vmap+0xc5/0x1b0 drivers/gpu/drm/drm_gem.c:1166
 drm_gem_vmap_unlocked+0x4a/0xa0 drivers/gpu/drm/drm_gem.c:1196
 drm_client_buffer_vmap+0x45/0xd0 drivers/gpu/drm/drm_client.c:326
 drm_fbdev_damage_blit drivers/gpu/drm/drm_fbdev_generic.c:314 [inline]
 drm_fbdev_fb_dirty+0x31e/0xcd0 drivers/gpu/drm/drm_fbdev_generic.c:342
 drm_fb_helper_damage_work+0x27a/0x5d0 drivers/gpu/drm/drm_fb_helper.c:388
 process_one_work+0xa33/0x1720 kernel/workqueue.c:2289
 worker_thread+0x67d/0x10e0 kernel/workqueue.c:2436
 kthread+0x2e4/0x3a0 kernel/kthread.c:376
 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:308
 

The config is:
https://gist.githubusercontent.com/dvyukov/2897b21db809075a22db0370c495ed2d/raw/9b2535b2ba77bb57e4f1ba2b909ad4075b6e2c6a/gistfile1.txt

Qemu command line:
qemu-system-x86_64 -enable-kvm -machine q35,nvdimm -cpu
max,migratable=off -smp 18 \
-m 72G -hda buildroot-amd64-2021.08 -kernel arch/x86/boot/bzImage -nographic \
-net user,host=10.0.2.10,hostfwd=tcp::10022-:22 -net nic,model=virtio-net-pci \
-append "console=ttyS0 root=/dev/sda1 earlyprintk=serial rodata=n \
oops=panic panic_on_warn=1 panic=86400 coredump_filter=0x"

Re: [PATCH] drm/edid/firmware: stop using throwaway platform device

[Matthieu CHARETTE]

Hi,

I've tested the patch and I can confirm that it fixed the issue.
Tested on Fedora 36 with kernel 6.0.8.

Thanks,
Matthieu

On Tue, Nov 8 2022 at 04:40:52 PM +0100, Matthieu CHARETTE 
 wrote:
I didn't test the patch yet. I will do. But even without testing I 
can tell you that it will work (It will not crash).
Currently when the crash occurs, all screens remain black after 
resume. I'm not able to login with ssh neither. And logs end before 
the suspend. So the crash seems to be some kind of kernel panic.


Matthieu

On Tue, Nov 8 2022 at 01:27:33 PM +0200, Jani Nikula 
 wrote:
On Sun, 06 Nov 2022, Matthieu CHARETTE  
wrote:

 Hi,

 Can you tell me what are we waiting for? Maybe I can help.


Have you tried the patch? Is it an improvement over the status quo?

The "crash" is still ambiguous to me. Do you observe it with the 
patch?

Do you have logs? Etc.

BR,
Jani.




 Thanks.

 Matthieu

 On Wed, Oct 12 2022 at 07:16:29 PM +0200, Matthieu CHARETTE
  wrote:

 By crash, I mean that an error is returned here:
 
https://kernel.googlesource.com/pub/scm/linux/kernel/git/torvalds/linux.git/+/refs/heads/master/drivers/gpu/drm/drm_edid_load.c#195
 I don't really know what happens next, but on my machine the 
built-in

 screen and the external remains dark. Also the kernel seems to
 freeze. I suspect a kernel panic, but I'm not sure. Anyway, the 
error

 is definitely not well handled, and a fix would be great.
 Also, request_firmware() will crash if called for the first time 
on
 the resume path because the file system isn't reachable on the 
resume
 process. And no cache is available for this firmware. So I guess 
that

 in this case, request_firmware() returns an error.
 Suspend-plug-resume case is not my priority nether as long as it
 doesn't make the system crash (Which is currently the case).

 On Wed, Oct 12 2022 at 11:25:59 AM +0300, Jani Nikula
  wrote:
 On Tue, 11 Oct 2022, Matthieu CHARETTE 


 wrote:
  Currently the EDID is requested during the resume. But since 
it's
  requested too early, this means before the filesystem is 
mounted,

 the
  firmware request fails. This make the DRM driver crash when
 resuming.
  This kind of issue should be prevented by the firmware caching
 process
  which cache every firmware requested for the next resume. But
 since we
  are using a temporary device, the firmware isn't cached on 
suspend

  since the device doesn't work anymore.
  When using a non temporary device to get the EDID, the firmware
 will
  be cached on suspend for the next resume. So requesting the
 firmware
  during resume will succeed.
  But if the firmware has never been requested since the boot, 
this

  means that the monitor isn't plugged since the boot. The kernel
 will
  not be caching the EDID. So if we plug the monitor while the
 machine
  is suspended. The resume will fail to load the firmware. And 
the

 DRM
  driver will crash.
  So basically, your fix should solve the issue except for the 
case

  where the monitor hasn't been plugged since boot and is plugged
 while
  the machine is suspended.
  I hope I was clear. Tell me if I wasn't. I'm not really good at
 explaining.


 That was a pretty good explanation. The only thing I'm missing is
 what
 the failure mode is exactly when you claim the driver will 
crash. Why
 would request_firmware() "crash" if called for the first time on 
the

 resume path?

 I'm not sure I care much about not being able to load the 
firmware

 EDID
 in the suspend-plug-resume case (as this can be remedied with a
 subsequent modeset), but obviously any errors need to be handled
 gracefully, without crashing.

 BR,
 Jani.


 --
 Jani Nikula, Intel Open Source Graphics Center








--
Jani Nikula, Intel Open Source Graphics Center

Re: [PATCH] drm/amd/display: add parameter backlight_min

[Filip Moc]

On Tue, Nov 01, 2022 at 12:06:55PM -0400, Alex Deucher wrote:
> On Tue, Nov 1, 2022 at 11:42 AM Filip Moc  wrote:
> >
> > Hello Alex,
> >
> > thank you for your response.
> >
> > Yes, I have HP ENVY x360 Convertible 13-ay1xxx, and backlight_min=2
> > seems to work the best in my case.
> >
> > I added a dmi based quirk table. So far with support only for display 0
> > to make it simple. Support for more displays in quirk table can be added
> > later if ever needed.
> >
> > According to [1] HP ENVY x360 Convertible 13-ag0xxx also needs a quirk
> > so I'm going to use this to cover both:
> > DMI_EXACT_MATCH(DMI_SYS_VENDOR, "HP")
> > DMI_MATCH(DMI_PRODUCT_NAME, "HP ENVY x360 Convertible 13-")
> 
> You might want to add an additional check for the panel name/vendor
> from the EDID as well in case HP uses multiple panels from different
> vendors on that system.

Hello,

thank you for this feedback.

I agree it would be nice to have this additional check but I'm not sure
if there is an easy way to do this.

I don't think we can add this to the EDID quirk table in drm_edid.c as
we also need to store the value for backlight min_input_signal.
There might also be different laptop manufacturers using the same panel
which only one of them needs a quirk or both of them might need a quirk
but with different value. Or there also might be more devices with the
same DMI but different panels where each panel needs a quirk with
different value.

So it seems if we want to cover all possible situations we need a nested
quirk tables for this. One main table for DMI matches where each record
would contain another table for EDID matches.

Then there's also a question of how to obtain the EDID.
The most simple and straightforward approach seems to be getting the
EDID vendor/product identification from dc_edid_caps. But this seems to
be going completely against the goal 10. in display/TODO.

So another approach I'm considering is to use drm_edid_get_panel_id but
for that we would need a pointer to i2c_adapter for the corresponding
backlight_link. Which I think is currently only available via
amdgpu_dm_connector which from dc_link is only accessible via void
*priv. Which seems like something that shouldn't be touched here.

So overall I don't know what would be the best way to implement this.
It also seems too complex I'm not even sure if it's worth the trouble
and maybe just hoping there won't be any two devices with the same DMI
but different EDID which only one of them would need a quirk while the
other would break with it might seem more reasonable.

Or maybe I'm missing something?
Please let me know what you think.

Filip

> 
> Alex
> 
> >
> > So far it seems to be working fine.
> > I'll send this in v2 once I do some final tweaks and do more tests.
> >
> > [1] https://bugzilla.kernel.org/show_bug.cgi?id=203439#c5
> >
> > Filip
> >
> >
> > V Mon, Oct 31, 2022 at 11:36:09AM -0400, Alex Deucher napsal(a):
> > > On Sun, Oct 30, 2022 at 5:26 AM Filip Moc  wrote:
> > > >
> > > > There are some devices on which amdgpu won't allow user to set 
> > > > brightness
> > > > to sufficiently low values even though the hardware would support it 
> > > > just
> > > > fine.
> > > >
> > > > This usually happens in two cases when either configuration of 
> > > > brightness
> > > > levels via ACPI/ATIF is not available and amdgpu falls back to defaults
> > > > (currently 12 for minimum level) which may be too high for some devices 
> > > > or
> > > > even the configuration via ATIF is available but the minimum brightness
> > > > level provided by the manufacturer is set to unreasonably high value.
> > > >
> > > > In either case user can use this new module parameter to adjust the
> > > > minimum allowed backlight brightness level.
> > > >
> > > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=203439
> > > > Signed-off-by: Filip Moc 
> > >
> > > Does your system require an override and if so, what is it?  It would
> > > be good to add a quirk for your system as well so that other users of
> > > the same system wouldn't need to manually figure out an apply the
> > > settings.
> > >
> > > Alex
> > >
> > >
> > > > ---
> > > >  drivers/gpu/drm/amd/amdgpu/amdgpu.h   |  3 +++
> > > >  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   | 15 +++
> > > >  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 +++
> > > >  3 files changed, 33 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > > > index 0e6ddf05c23c..c5445402c49d 100644
> > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > > > @@ -200,6 +200,9 @@ extern uint amdgpu_dc_debug_mask;
> > > >  extern uint amdgpu_dc_visual_confirm;
> > > >  extern uint amdgpu_dm_abm_level;
> > > >  extern int amdgpu_backlight;
> > > > +#ifdef CONFIG_DRM_AMD_DC
> > > > +extern int amdgpu_backlight_override_min[];
> > > > +#endif
> > > >  extern struct 

Re: [PATCH] drm/amd/display: don't enable DRM CRTC degamma property for DCE

[Rodrigo Siqueira]

On 11/3/22 14:45, Melissa Wen wrote:

DM maps DRM CRTC degamma to DPP (pre-blending) degamma block, but DCE doesn't
support programmable degamma curve anywhere. Currently, a custom degamma is
accepted by DM but just ignored by DCE driver and degamma correction isn't
actually applied. There is no way to map custom degamma in DCE, therefore, DRM
CRTC degamma property shouldn't be enabled for DCE drivers.

Signed-off-by: Melissa Wen 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
index 9ac2805c5d63..b3eadfc61555 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
@@ -415,7 +415,7 @@ int amdgpu_dm_crtc_init(struct amdgpu_display_manager *dm,
  {
struct amdgpu_crtc *acrtc = NULL;
struct drm_plane *cursor_plane;
-
+   bool is_dcn;
int res = -ENOMEM;
  
  	cursor_plane = kzalloc(sizeof(*cursor_plane), GFP_KERNEL);

@@ -453,8 +453,14 @@ int amdgpu_dm_crtc_init(struct amdgpu_display_manager *dm,
acrtc->otg_inst = -1;
  
  	dm->adev->mode_info.crtcs[crtc_index] = acrtc;

-   drm_crtc_enable_color_mgmt(>base, MAX_COLOR_LUT_ENTRIES,
+
+   /* Don't enable DRM CRTC degamma property for DCE since it doesn't
+* support programmable degamma anywhere.
+*/
+   is_dcn = dm->adev->dm.dc->caps.color.dpp.dcn_arch;
+   drm_crtc_enable_color_mgmt(>base, is_dcn ? MAX_COLOR_LUT_ENTRIES 
: 0,
   true, MAX_COLOR_LUT_ENTRIES);
+
drm_mode_crtc_set_gamma_size(>base, 
MAX_COLOR_LEGACY_LUT_ENTRIES);
  
  	return 0;


Hi,

I tested it in a DCE device and the patch lgtm.

Reviewed-by: Rodrigo Siqueira 

and merged it into amd-staging-drm-next.

Thanks
Siqueira

Re: [PATCH v2] udmabuf: add vmap method to udmabuf_ops

[Dmitry Osipenko]

On 11/13/22 18:05, Lukasz Wiecaszek wrote:
> +static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map)
> +{
> + struct udmabuf *ubuf = buf->priv;
> +
> + if (!ubuf->vaddr) {
> + ubuf->vaddr = vm_map_ram(ubuf->pages, ubuf->pagecount, -1);
> + if (!ubuf->vaddr)
> + return -EINVAL;
> + }

Create a new mapping on each vmap_udmabuf() and add the corresponding
vunmap.

Otherwise persistent vmapping shall be released together with udmabuf.
It doesn't look that persistent vmapping is needed for udmabufs.

-- 
Best regards,
Dmitry

Re: [PATCH v2] udmabuf: add vmap method to udmabuf_ops

[Dmitry Osipenko]

On 11/13/22 18:05, Lukasz Wiecaszek wrote:
> The reason behind that patch is associated with videobuf2 subsystem
> (or more genrally with v4l2 framework) and user created
> dma buffers (udmabuf). In some circumstances
> when dealing with V4L2_MEMORY_DMABUF buffers videobuf2 subsystem
> wants to use dma_buf_vmap() method on the attached dma buffer.
> As udmabuf does not have .vmap operation implemented,
> such dma_buf_vmap() natually fails.
> 
> videobuf2_common: [cap-3473b2f1] __vb2_queue_alloc: allocated 3 
> buffers, 1 plane(s) each
> videobuf2_common: [cap-3473b2f1] __prepare_dmabuf: buffer for plane 0 
> changed
> videobuf2_common: [cap-3473b2f1] __prepare_dmabuf: failed to map 
> dmabuf for plane 0
> videobuf2_common: [cap-3473b2f1] __buf_prepare: buffer preparation 
> failed: -14
> 
> The patch itself seems to be strighforward.
> It adds implementation of .vmap method to 'struct dma_buf_ops udmabuf_ops'.
> .vmap method itself uses vm_map_ram() to map pages linearly
> into the kernel virtual address space (only if such mapping
> hasn't been created yet).
> 
> Signed-off-by: Lukasz Wiecaszek 
> Reported-by: kernel test robot 
> ---
> v1: 
> https://lore.kernel.org/linux-media/202211120352.g7wpasop-...@intel.com/T/#t
> 
> v1 -> v2: Patch prepared and tested against 6.1.0-rc2+
> 
>  drivers/dma-buf/udmabuf.c | 19 +++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
> index 2bcdb935a3ac..2ca0e3639360 100644
> --- a/drivers/dma-buf/udmabuf.c
> +++ b/drivers/dma-buf/udmabuf.c
> @@ -12,6 +12,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  
>  static int list_limit = 1024;
>  module_param(list_limit, int, 0644);
> @@ -26,6 +28,7 @@ struct udmabuf {
>   struct page **pages;
>   struct sg_table *sg;
>   struct miscdevice *device;
> + void *vaddr;
>  };
>  
>  static vm_fault_t udmabuf_vm_fault(struct vm_fault *vmf)
> @@ -57,6 +60,21 @@ static int mmap_udmabuf(struct dma_buf *buf, struct 
> vm_area_struct *vma)
>   return 0;
>  }
>  
> +static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map)
> +{
> + struct udmabuf *ubuf = buf->priv;
> +
> + if (!ubuf->vaddr) {
> + ubuf->vaddr = vm_map_ram(ubuf->pages, ubuf->pagecount, -1);
> + if (!ubuf->vaddr)
> + return -EINVAL;
> + }
> +
> + iosys_map_set_vaddr(map, ubuf->vaddr);
> +
> + return 0;
> +}
> +
>  static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf,
>enum dma_data_direction direction)
>  {
> @@ -159,6 +177,7 @@ static const struct dma_buf_ops udmabuf_ops = {
>   .unmap_dma_buf = unmap_udmabuf,
>   .release   = release_udmabuf,
>   .mmap  = mmap_udmabuf,
> + .vmap  = vmap_udmabuf,
>   .begin_cpu_access  = begin_cpu_udmabuf,
>   .end_cpu_access= end_cpu_udmabuf,
>  };

Where is vunmap?

-- 
Best regards,
Dmitry

[PATCH v2] udmabuf: add vmap method to udmabuf_ops

[Lukasz Wiecaszek]

The reason behind that patch is associated with videobuf2 subsystem
(or more genrally with v4l2 framework) and user created
dma buffers (udmabuf). In some circumstances
when dealing with V4L2_MEMORY_DMABUF buffers videobuf2 subsystem
wants to use dma_buf_vmap() method on the attached dma buffer.
As udmabuf does not have .vmap operation implemented,
such dma_buf_vmap() natually fails.

videobuf2_common: [cap-3473b2f1] __vb2_queue_alloc: allocated 3 
buffers, 1 plane(s) each
videobuf2_common: [cap-3473b2f1] __prepare_dmabuf: buffer for plane 0 
changed
videobuf2_common: [cap-3473b2f1] __prepare_dmabuf: failed to map dmabuf 
for plane 0
videobuf2_common: [cap-3473b2f1] __buf_prepare: buffer preparation 
failed: -14

The patch itself seems to be strighforward.
It adds implementation of .vmap method to 'struct dma_buf_ops udmabuf_ops'.
.vmap method itself uses vm_map_ram() to map pages linearly
into the kernel virtual address space (only if such mapping
hasn't been created yet).

Signed-off-by: Lukasz Wiecaszek 
Reported-by: kernel test robot 
---
v1: https://lore.kernel.org/linux-media/202211120352.g7wpasop-...@intel.com/T/#t

v1 -> v2: Patch prepared and tested against 6.1.0-rc2+

 drivers/dma-buf/udmabuf.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index 2bcdb935a3ac..2ca0e3639360 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -12,6 +12,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 static int list_limit = 1024;
 module_param(list_limit, int, 0644);
@@ -26,6 +28,7 @@ struct udmabuf {
struct page **pages;
struct sg_table *sg;
struct miscdevice *device;
+   void *vaddr;
 };
 
 static vm_fault_t udmabuf_vm_fault(struct vm_fault *vmf)
@@ -57,6 +60,21 @@ static int mmap_udmabuf(struct dma_buf *buf, struct 
vm_area_struct *vma)
return 0;
 }
 
+static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map)
+{
+   struct udmabuf *ubuf = buf->priv;
+
+   if (!ubuf->vaddr) {
+   ubuf->vaddr = vm_map_ram(ubuf->pages, ubuf->pagecount, -1);
+   if (!ubuf->vaddr)
+   return -EINVAL;
+   }
+
+   iosys_map_set_vaddr(map, ubuf->vaddr);
+
+   return 0;
+}
+
 static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf,
 enum dma_data_direction direction)
 {
@@ -159,6 +177,7 @@ static const struct dma_buf_ops udmabuf_ops = {
.unmap_dma_buf = unmap_udmabuf,
.release   = release_udmabuf,
.mmap  = mmap_udmabuf,
+   .vmap  = vmap_udmabuf,
.begin_cpu_access  = begin_cpu_udmabuf,
.end_cpu_access= end_cpu_udmabuf,
 };
-- 
2.25.1

Re: [RFC PATCH v3 0/3] new subsystem for compute accelerator devices

[Oded Gabbay]

On Sat, Nov 12, 2022 at 12:04 AM Christopher Friedt
 wrote:
>
> Hi Oded,
>
> On Sun, Nov 6, 2022 at 4:03 PM Oded Gabbay  wrote:
> > The patches are in the following repo:
> > https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/accel.git/log/?h=accel_v3
> >
> > As in v2, The HEAD of that branch is a commit adding a dummy driver that
> > registers an accel device using the new framework. This can be served
> > as a simple reference. I have checked inserting and removing the dummy 
> > driver,
> > and opening and closing /dev/accel/accel0 and nothing got broken :)
> >
> > v1 cover letter:
> > https://lkml.org/lkml/2022/10/22/544
> >
> > v2 cover letter:
> > https://lore.kernel.org/lkml/20221102203405.1797491-1-ogab...@kernel.org/T/
>
> I was in the room at Plumbers when a lot of this was discussed (in
> 2022 and also 2019), but I haven't really had an opportunity to
> provide feedback until now. In general, I think it's great and thanks
> for pushing it forward and getting feedback.
>
> The v1 cover letter mentioned RAS (reliability, availability,
> serviceability) and Dave also mentioned it here [1]. There was a
> suggestion to use Netlink. It's an area that I'm fairly interested in
> because I do a lot of development on the firmware side (and
> specifically, with Zephyr).
>
> Personally, I think Netlink could be one option for serializing and
> deserializing RAS information but it would be helpful for that
> interface to be somewhat flexible, like a void * and length, and to
> provide userspace the capability of querying which RAS formats are
> supported.
>
> For example, AntMicro used OpenAMP + rpmsg in their NVMe accelerator,
> and gave a talk on it at ZDS and Plumbers this year [2][3].
>
> In Zephyr, the LGPL license for Netlink might be a non-starter
> (although I'm no lawyer). However, Zephyr does already support
> OpenAMP, protobufs, json, and will soon support Thrift.
>
> Some companies might prefer to use Netlink. Others might prefer to use
> ASN.1. Some companies might prefer to use key-value pairs and limit
> the parameters and messages to uint32s. Some might handle all of the
> RAS details in-kernel, while others might want the kernel to act more
> like a transport to firmware.
>
> Companies already producing accelerators may have a particular
> preference for serialization / deserialization in their own
> datacenters.
>
> With that, it would be helpful to be able to query RAS capabilities via ioctl.
>
> #define ACCEL_CAP_RAS_KEY_VAL_32 BIT(0)
> #define ACCEL_CAP_RAS_NETLINK BIT(1)
> #define ACCEL_CAP_RAS_JSON BIT(2)
> #define ACCEL_CAP_RAS_PROTOBUF BIT(3)
> #define ACCEL_CAP_RAS_GRPC BIT(4)
> #define ACCEL_CAP_RAS_THRIFT BIT(5)
> #define ACCEL_CAP_RAS_JSON BIT(6)
> #define ACCEL_CAP_RAS_ASN1 BIT(7)
>
> or something along those lines. Anyway, just putting the idea out there.
>
> I'm sure there are a lot of opinions on this topic and that there are
> a lot of implications of using this or that serialization format.
> Obviously there can be security implications as well.
>
> Apologies if I've already missed some of this discussion.
>
> Cheers,
>
> C
>
> [1] 
> https://airlied.blogspot.com/2022/09/accelerators-bof-outcomes-summary.html
> [2] 
> https://zephyr2022.sched.com/event/10CFD/open-source-nvme-ai-accelerator-platform-with-zephyr-karol-gugala-antmicro
> [3] https://lpc.events/event/16/contributions/1245/

Hi Christopher,
Thanks for all this information.
At this stage, I'm mainly trying to gather information on RAS current
status in the OCP (Open Compute Project) and Linux kernel, so your
email was on point :)
It seems to me that this topic is broader than just accelerators or
GPUs, because there are other device types that are implementing some
kind of RAS (e.g. NIC).
My gut feeling is that the end solution would be some kind of generic
kernel driver/framework that will expose RAS to userspace for any
device type, but it's too early to tell.
I'll update once I have the full picture.

Thanks,
Oded

Re: [PATCH v2.5] drm/msm/dsi: switch to DRM_PANEL_BRIDGE

[Dmitry Baryshkov]

Hi Caleb,

On Fri, 11 Nov 2022 at 18:30, Caleb Connolly  wrote:
>
> Hi,
>
> This patch has caused a regression on 6.1-rc for some devices that use
> DSI panels. The new behaviour results in the DSI controller being
> switched off before the panel unprepare hook is called. As a result,
> panel drivers which call mipi_dsi_dcs_write() or similar in
> unprepare() fail.

Thanks for the notice. Can you move your command stream to
panel_disable() hook? (even if it's just as a temporary workaround)

>From what I see from other panels, some of them call
mipi_dsi_dcs_set_display_off() in the unprepare() hook, while others
do it in disable().

Yes, this is (again) the DSI host vs device order here. Short story:
the DRM has a notion of 'the display pipe (i.e. clocks and timing
signals) feeding the bridge being running'. That's the difference
between enable/pre_enable and disable/post_disable. For the DSI we
have a third state, when the DSI clock and ln0 allow transferring
commands to the panel, but the image is not enabled.

There was a somewhat promising patchset at [1], but it seems it went
out of the radar. I can try working on an alternative (explicit)
approach if I have time.

With respect to your panel. Let me quote the docs: 'Before stopping
video transmission from the display controller it can be necessary to
turn off the panel to avoid visual glitches. This is done in the
.disable() function. Analogously to .enable() this typically involves
turning off the backlight and waiting for some time to make sure no
image is visible on the panel. It is then safe for the display
controller to cease transmission of video data.'

So, if we stop the call chain after switching the DSI host off but
before calling the panel's unprepare() hook, will we see any
artifacts/image leftover/etc. on the panel? Generally I have the
feeling that all panels should call mipi_dsi_dcs_set_display_off() in
the .disable() hook, not in the .unprepare() one.

[1] 
https://lore.kernel.org/dri-devel/cover.1646406653.git.dave.steven...@raspberrypi.com/

>
> I've noticed it specifically on the OnePlus 6 (with upstream Samsung
> s0fef00 panel driver) and the SHIFT6mq with an out of tree driver.
>
> On 12/07/2022 14:22, Dmitry Baryshkov wrote:
> > Currently the DSI driver has two separate paths: one if the next device
> > in a chain is a bridge and another one if the panel is connected
> > directly to the DSI host. Simplify the code path by using panel-bridge
> > driver (already selected in Kconfig) and dropping support for
> > handling the panel directly.
> >
> > Signed-off-by: Dmitry Baryshkov 
> > ---
> >
> > I'm not sending this as a separate patchset (I'd like to sort out mdp5
> > first), but more of a preview of changes related to
> > msm_dsi_manager_ext_bridge_init().
> >
> > ---
> >   drivers/gpu/drm/msm/dsi/dsi.c |  35 +---
> >   drivers/gpu/drm/msm/dsi/dsi.h |  16 +-
> >   drivers/gpu/drm/msm/dsi/dsi_host.c|  25 ---
> >   drivers/gpu/drm/msm/dsi/dsi_manager.c | 283 +++---
> >   4 files changed, 36 insertions(+), 323 deletions(-)

[skipped the patch itself]

-- 
With best wishes
Dmitry

[PATCH 1/1] drm/shmem: Dual licence the files as GPL-2 and MIT

[Robert Swindells]

Contributors to these files are:

Noralf Trønnes 
Liu Zixian 
Dave Airlie 
Thomas Zimmermann 
Lucas De Marchi 
Gerd Hoffmann 
Rob Herring 
Jakub Kicinski 
Marcel Ziswiler 
Stephen Rothwell 
Daniel Vetter 
Cai Huoqing 
Neil Roberts 
Marek Szyprowski 
Emil Velikov 
Sam Ravnborg 
Boris Brezillon 
Dan Carpenter 

Signed-off-by: Robert Swindells 
---
 drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +-
 include/drm/drm_gem_shmem_helper.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c 
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 35138f8a375c..f1a68a71f876 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -1,4 +1,4 @@
-// SPDX-License-Identifier: GPL-2.0
+// SPDX-License-Identifier: GPL-2.0 or MIT
 /*
  * Copyright 2018 Noralf Trønnes
  */
diff --git a/include/drm/drm_gem_shmem_helper.h 
b/include/drm/drm_gem_shmem_helper.h
index a2201b2488c5..56ac32947d1c 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -1,4 +1,4 @@
-/* SPDX-License-Identifier: GPL-2.0 */
+/* SPDX-License-Identifier: GPL-2.0 or MIT */
 
 #ifndef __DRM_GEM_SHMEM_HELPER_H__
 #define __DRM_GEM_SHMEM_HELPER_H__
-- 
2.38.0

[PATCH 0/1] drm/shmem: Dual licence the files as GPL-2 and MIT

[Robert Swindells]

There was some earlier discussion on dual licencing as GPL-2 and MIT
some files that were previously just GPL-2:



Would it be possible to dual licence two more of the files?

drivers/gpu/drm/drm_gem_shmem_helper.c
include/drm/drm_gem_shmem_helper.h

They are used by the lima driver, which is dual licenced.

Robert Swindells
r...@netbsd.org

Robert Swindells (1):
  drm/shmem: Dual licence the files as GPL-2 and MIT

 drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +-
 include/drm/drm_gem_shmem_helper.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.38.0

Re: [PATCH] udmabuf: add vmap method to udmabuf_ops

[Lukasz Wiecaszek]

On Fri, Nov 11, 2022 at 03:31:15PM +0300, Dmitry Osipenko wrote:
> On 11/11/22 15:05, Christian König wrote:
> > Adding Dmitry as well.
> > 
> > Am 11.11.22 um 12:45 schrieb Lukasz Wiecaszek:
> >> The reason behind that patch is associated with videobuf2 subsystem
> >> (or more genrally with v4l2 framework) and user created
> >> dma buffers (udmabuf). In some circumstances
> >> when dealing with V4L2_MEMORY_DMABUF buffers videobuf2 subsystem
> >> wants to use dma_buf_vmap() method on the attached dma buffer.
> >> As udmabuf does not have .vmap operation implemented,
> >> such dma_buf_vmap() natually fails.
> >>
> >> videobuf2_common: [cap-3473b2f1] __vb2_queue_alloc: allocated
> >> 3 buffers, 1 plane(s) each
> >> videobuf2_common: [cap-3473b2f1] __prepare_dmabuf: buffer for
> >> plane 0 changed
> >> videobuf2_common: [cap-3473b2f1] __prepare_dmabuf: failed to
> >> map dmabuf for plane 0
> >> videobuf2_common: [cap-3473b2f1] __buf_prepare: buffer
> >> preparation failed: -14
> >>
> >> The patch itself seems to be strighforward.
> >> It adds implementation of .vmap method to 'struct dma_buf_ops
> >> udmabuf_ops'.
> >> .vmap method itself uses vm_map_ram() to map pages linearly
> >> into the kernel virtual address space (only if such mapping
> >> hasn't been created yet).
> > 
> > Of hand that sounds sane to me.
> > 
> > You should probably mention somewhere in a code comment that the cached
> > vaddr is protected by the reservation lock being taken. That's not
> > necessary obvious to everybody.
> > 
> > Apart from that looks good to me.
> 
> Adding a comment won't hurt.
> 
> We have the dma_resv_assert_held() in dma_buf_vmap() that will help
> spotting a missing lock at runtime by developers. While the
> dmbuf_ops->vmap() shouldn't be ever used directly by importers.
> 
> -- 
> Best regards,
> Dmitry
>

Give me some time guys. I need to prepare patch agains 6.1. And this is
my first time, so now it hurts. 

Lukasz

Re: [PATCH V2 0/3] DSI host and peripheral initialisation ordering

[Dmitry Baryshkov]

Hi Dave,

On 19/07/2022 16:45, Dave Stevenson wrote:

Hi Sam

On Mon, 18 Jul 2022 at 21:52, Sam Ravnborg  wrote:


Hi Dave,

a long overdue reply on this series.

On Fri, Mar 04, 2022 at 03:17:55PM +, Dave Stevenson wrote:

Hi All

Changes from v1:
- New patch to refactor drm_bridge_chain_post_disable and 
drm_bridge_chain_pre_enable
   to reuse drm_atomic_bridge_chain_post_disable / 
drm_atomic_bridge_chain_pre_enable
   but with a NULL state.
- New patch that adds a pre_enable_upstream_first to drm_panel.
- changed from an OPS flag to a bool "pre_enable_upstream_first" in drm_bridge.
- Followed Andrzej's suggestion of using continue in the main loop to avoid
   needing 2 additional loops (one forward to find the last bridge wanting
   upstream first, and the second backwards again).
- Actioned Laurent's review comments on docs patch.

Original cover letter:

Hopefully I've cc'ed all those that have bashed this problem around previously,
or are otherwise linked to DRM bridges.

There have been numerous discussions around how DSI support is currently broken
as it doesn't support initialising the PHY to LP-11 and potentially the clock
lane to HS prior to configuring the DSI peripheral. There is no op where the
interface is initialised but HS video isn't also being sent.
Currently you have:
- peripheral pre_enable (host not initialised yet)
- host pre_enable
- encoder enable
- host enable
- peripheral enable (video already running)

vc4 and exynos currently implement the DSI host as an encoder, and split the
bridge_chain. This fails if you want to switch to being a bridge and/or use
atomic calls as the state of all the elements split off are not added by
drm_atomic_add_encoder_bridges.


A typically chain looks like this:

CRTC => Encoder => Bridge A => Bridge B

We have in DRM bridges established what is the "next" bridge - indicated
with the direction of the arrows in the drawing.

This set of patches introduces the concept of "upstream" bridges.

pre_enable_prev_bridge_first would be easier to understand as it uses
the current terminology.
I get that "upstream" is used in the DSI specification - but we are
dealing with bridges that happens to support DSI and more, and mixing
the two terminologies is not good.

Note: Upstream is also used in a bridge doc section - here it should
   most likely be updated too.


Sure, I have no issues with switching to prev/next from upstream/downstream.
To the outsider it can be confusing - in pre_enable and disable, the
next bridge to be called is the previous one. At least it is
documented.


The current approach set a flag that magically makes the core do something
else. Have you considered a much more explicit approach?

A few helpers like:

 drm_bridge_pre_enable_prev_bridge()
 drm_bridge_enable_prev_bridge()
 drm_bridge_disable_prev_bridge()
 drm_bridge_post_disable_prev_bridge()


No point in drm_bridge_enable_prev_bridge() and
drm_bridge_post_disable_prev_bridge() as the call order down the chain
will mean that they have already been called.
drm_bridge_enable_next_bridge() and
drm_bridge_post_disable_next_bridge() possibly.


And then update the core so the relevant function is only called once
for a bridge.
Then the need for DSI lanes in LP-11 can be archived by a call to

 drm_bridge_pre_enable_prev_bridge()


Unfortunately it gets ugly with post_disable.
The DSI host controller post_disable will have been called before the
DSI peripheral's post_disable, and there are conditions where the
peripheral needs to send DSI commands in post_disable (eg
panel-asus-z00t-tm5p5-n35596 [1]). Changing all DSI hosts to call
drm_bridge_post_disable_next_bridge feels like the wrong thing to do.
There are currently hacks in dw-mipi-dsi that do call the next
panel/bridge post_disable [2] and it would be nice to get rid of them.
Currently the calls aren't tracked for state, so you end up with
post_disable being called twice, and panels having to track state (eg
jdi_lt070me05 [3]).

[1] tm5p5_nt35596_unprepare() calls tm5p5_nt35596_off()
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c#L107
[2] 
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c#L889
[3] 
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c#L44


This is more explicit than a flag that triggers some magic behaviour.
It may even see uses we have not realised yet.


Personally it feels like more boilerplate in almost all DSI drivers,
and generally I see a push to remove boilerplate.


It is late here - so maybe the above is not a good idea tomorrow - but
right now I like the simplicity of it.

Other than the above I read that a mipi_dsi_host_init() is planned,
which is also explicit and simple - good.


It's been raised, but the justification for most use cases hasn't been
made. The Exynos conversion looks to be doing the wrong thing 

[PATCH] drm/radeon: fix potential racing issue due to mmap_lock

[Dawei Li]

Both find_vma() and get_user_pages() need explicit protection of
mmap lock, fix them by mmap_lock and get_user_pages_fast().

Fixes: ddd00e33e17a ("drm/radeon: add userptr flag to limit it to anonymous 
memory v2")
Fixes: f72a113a71ab ("drm/radeon: add userptr support v8")
Signed-off-by: Dawei Li 
---
 drivers/gpu/drm/radeon/radeon_ttm.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
b/drivers/gpu/drm/radeon/radeon_ttm.c
index d33fec488713..741ea64b9402 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -351,7 +351,10 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_device 
*bdev, struct ttm_tt *ttm
   to prevent problems with writeback */
unsigned long end = gtt->userptr + (u64)ttm->num_pages * 
PAGE_SIZE;
struct vm_area_struct *vma;
+
+   mmap_read_lock(gtt->usermm);
vma = find_vma(gtt->usermm, gtt->userptr);
+   mmap_read_unlock(gtt->usermm);
if (!vma || vma->vm_file || vma->vm_end < end)
return -EPERM;
}
@@ -361,8 +364,7 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_device 
*bdev, struct ttm_tt *ttm
uint64_t userptr = gtt->userptr + pinned * PAGE_SIZE;
struct page **pages = ttm->pages + pinned;
 
-   r = get_user_pages(userptr, num_pages, write ? FOLL_WRITE : 0,
-  pages, NULL);
+   r = get_user_pages_fast(userptr, num_pages, write ? FOLL_WRITE 
: 0, pages);
if (r < 0)
goto release_pages;
 
-- 
2.25.1

Re: [PATCH v2.5] drm/msm/dsi: switch to DRM_PANEL_BRIDGE #forregzbot

[Thorsten Leemhuis]

[Note: this mail is primarily send for documentation purposes and/or for
regzbot, my Linux kernel regression tracking bot. That's why I removed
most or all folks from the list of recipients, but left any that looked
like a mailing lists. These mails usually contain '#forregzbot' in the
subject, to make them easy to spot and filter out.]

[TLDR: I'm adding this regression report to the list of tracked
regressions; all text from me you find below is based on a few templates
paragraphs you might have encountered already already in similar form.]

Hi, this is your Linux kernel regression tracker.

On 11.11.22 16:30, Caleb Connolly wrote:
> 
> This patch has caused a regression on 6.1-rc for some devices that use
> DSI panels. The new behaviour results in the DSI controller being
> switched off before the panel unprepare hook is called. As a result,
> panel drivers which call mipi_dsi_dcs_write() or similar in unprepare()
> fail.
> 
> I've noticed it specifically on the OnePlus 6 (with upstream Samsung
> s0fef00 panel driver) and the SHIFT6mq with an out of tree driver.

Thanks for the report. To be sure below issue doesn't fall through the
cracks unnoticed, I'm adding it to regzbot, my Linux kernel regression
tracking bot:

#regzbot ^introduced 007ac0262b0d
#regzbot title drm: msm: DSI controller being switched off before the
panel unprepare hook is called
#regzbot ignore-activity

This isn't a regression? This issue or a fix for it are already
discussed somewhere else? It was fixed already? You want to clarify when
the regression started to happen? Or point out I got the title or
something else totally wrong? Then just reply -- ideally with also
telling regzbot about it, as explained here:
https://linux-regtracking.leemhuis.info/tracked-regression/

Reminder for developers: When fixing the issue, add 'Link:' tags
pointing to the report (the mail this one replies to), as explained for
in the Linux kernel's documentation; above webpage explains why this is
important for tracked regressions.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I deal with a lot of
reports and sometimes miss something important when writing mails like
this. If that's the case here, don't hesitate to tell me in a public
reply, it's in everyone's interest to set the public record straight.

> On 12/07/2022 14:22, Dmitry Baryshkov wrote:
>> Currently the DSI driver has two separate paths: one if the next device
>> in a chain is a bridge and another one if the panel is connected
>> directly to the DSI host. Simplify the code path by using panel-bridge
>> driver (already selected in Kconfig) and dropping support for
>> handling the panel directly.
>>
>> Signed-off-by: Dmitry Baryshkov 
>> ---
>>
>> I'm not sending this as a separate patchset (I'd like to sort out mdp5
>> first), but more of a preview of changes related to
>> msm_dsi_manager_ext_bridge_init().
>>
>> ---
>>   drivers/gpu/drm/msm/dsi/dsi.c |  35 +---
>>   drivers/gpu/drm/msm/dsi/dsi.h |  16 +-
>>   drivers/gpu/drm/msm/dsi/dsi_host.c    |  25 ---
>>   drivers/gpu/drm/msm/dsi/dsi_manager.c | 283 +++---
>>   4 files changed, 36 insertions(+), 323 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dsi/dsi.c
>> b/drivers/gpu/drm/msm/dsi/dsi.c
>> index 1625328fa430..4edb9167e600 100644
>> --- a/drivers/gpu/drm/msm/dsi/dsi.c
>> +++ b/drivers/gpu/drm/msm/dsi/dsi.c
>> @@ -6,14 +6,6 @@
>>   #include "dsi.h"
>>   #include "dsi_cfg.h"
>>
>> -struct drm_encoder *msm_dsi_get_encoder(struct msm_dsi *msm_dsi)
>> -{
>> -    if (!msm_dsi || !msm_dsi_device_connected(msm_dsi))
>> -    return NULL;
>> -
>> -    return msm_dsi->encoder;
>> -}
>> -
>>   bool msm_dsi_is_cmd_mode(struct msm_dsi *msm_dsi)
>>   {
>>   unsigned long host_flags =
>> msm_dsi_host_get_mode_flags(msm_dsi->host);
>> @@ -220,7 +212,6 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi,
>> struct drm_device *dev,
>>    struct drm_encoder *encoder)
>>   {
>>   struct msm_drm_private *priv;
>> -    struct drm_bridge *ext_bridge;
>>   int ret;
>>
>>   if (WARN_ON(!encoder) || WARN_ON(!msm_dsi) || WARN_ON(!dev))
>> @@ -254,26 +245,10 @@ int msm_dsi_modeset_init(struct msm_dsi
>> *msm_dsi, struct drm_device *dev,
>>   goto fail;
>>   }
>>
>> -    /*
>> - * check if the dsi encoder output is connected to a panel or an
>> - * external bridge. We create a connector only if we're connected
>> to a
>> - * drm_panel device. When we're connected to an external bridge, we
>> - * assume that the drm_bridge driver will create the connector
>> itself.
>> - */
>> -    ext_bridge = msm_dsi_host_get_bridge(msm_dsi->host);
>> -
>> -    if (ext_bridge)
>> -    msm_dsi->connector =
>> -    msm_dsi_manager_ext_bridge_init(msm_dsi->id);
>> -    else
>> -    msm_dsi->connector =
>> -    msm_dsi_manager_connector_init(msm_dsi->id);
>> -
>> -    if