Re: [PATCH v18 0/2] Add memory bandwidth management to NVIDIA Tegra DRM driver
07.06.2021 01:40, Dmitry Osipenko пишет: > 01.06.2021 07:21, Dmitry Osipenko пишет: >> This series adds memory bandwidth management to the NVIDIA Tegra DRM driver, >> which is done using interconnect framework. It fixes display corruption that >> happens due to insufficient memory bandwidth. >> >> Changelog: >> >> v18: - Moved total peak bandwidth from CRTC state to plane state and removed >>dummy plane bandwidth state initialization from T186+ plane hub. This >>was suggested by Thierry Reding to v17. >> >> - I haven't done anything about the cursor's plane bandwidth which >>doesn't contribute to overlapping bandwidths for a small sized >>window because it works okay as-is. > > Thierry, will you take these patches for 5.14? > The display controller does _NOT_WORK_ properly without bandwidth management. Can we get this patch into 5.14? What is the problem?
Re:Re: [PATCH 0/4] delete useless function return values & remove meaningless if(r) check code
From: "Christian König" Date: 2021-06-21 00:59:27 To: Bernard Zhao ,Alex Deucher ,David Airlie ,Daniel Vetter ,amd-...@lists.freedesktop.org,dri-devel@lists.freedesktop.org,linux-ker...@vger.kernel.org Subject: Re: [PATCH 0/4] delete useless function return values & remove meaningless if(r) check code>Am 19.06.21 um 08:43 schrieb Bernard Zhao: >> Function radeon_fence_driver_init always returns success, >> the function type maybe coule be changed to void. >> This patch series will first delete the check of the return >> value of the function call radeon_fence_driver_init, then, >> optimise the function declaration and function to void type. > >Please merge all of this into a single patch, I don't see any point >separating this. > >Christian. Hi Sure, i will resubmit one new ptach, thanks! BR//Bernard >> >> Signed-off-by: Bernard Zhao >> >> Bernard Zhao (4): >>drm/radeon: remove meaningless if(r) check code >>drm/radeon: remove meaningless if(r) check code >>drm/radeon: remove meaningless if(r) check code >>drm/radeon: delete useless return values >> >> drivers/gpu/drm/radeon/cik.c | 4 +--- >> drivers/gpu/drm/radeon/evergreen.c| 4 +--- >> drivers/gpu/drm/radeon/ni.c | 4 +--- >> drivers/gpu/drm/radeon/r100.c | 4 +--- >> drivers/gpu/drm/radeon/r300.c | 4 +--- >> drivers/gpu/drm/radeon/r420.c | 5 + >> drivers/gpu/drm/radeon/r520.c | 4 +--- >> drivers/gpu/drm/radeon/r600.c | 4 +--- >> drivers/gpu/drm/radeon/radeon.h | 2 +- >> drivers/gpu/drm/radeon/radeon_fence.c | 5 + >> drivers/gpu/drm/radeon/rs400.c| 4 +--- >> drivers/gpu/drm/radeon/rs600.c| 4 +--- >> drivers/gpu/drm/radeon/rs690.c| 4 +--- >> drivers/gpu/drm/radeon/rv515.c| 4 +--- >> drivers/gpu/drm/radeon/rv770.c| 4 +--- >> drivers/gpu/drm/radeon/si.c | 4 +--- >> 16 files changed, 16 insertions(+), 48 deletions(-) >> >
[PATCH] drm: mxsfb: Clear FIFO_CLEAR bit
Make sure the FIFO_CLEAR bit is latched in when configuring the
controller, so that the FIFO is really cleared. And then clear
the FIFO_CLEAR bit, since it is not self-clearing.
Fixes: 45d59d704080 ("drm: Add new driver for MXSFB controller")
Signed-off-by: Marek Vasut
Cc: Daniel Abrecht
Cc: Emil Velikov
Cc: Laurent Pinchart
Cc: Lucas Stach
Cc: Stefan Agner
---
drivers/gpu/drm/mxsfb/mxsfb_kms.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
index 98d8ba0bae84..22cb749fc9bc 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
@@ -241,6 +241,9 @@ static void mxsfb_crtc_mode_set_nofb(struct
mxsfb_drm_private *mxsfb,
/* Clear the FIFOs */
writel(CTRL1_FIFO_CLEAR, mxsfb->base + LCDC_CTRL1 + REG_SET);
+ readl(mxsfb->base + LCDC_CTRL1);
+ writel(CTRL1_FIFO_CLEAR, mxsfb->base + LCDC_CTRL1 + REG_CLR);
+ readl(mxsfb->base + LCDC_CTRL1);
if (mxsfb->devdata->has_overlay)
writel(0, mxsfb->base + LCDC_AS_CTRL);
--
2.30.2
[PATCH] drm: mxsfb: Use bus_format from the nearest bridge if present
In case there is a bridge connected to the LCDIF, use bus_format
from the bridge, otherwise behave as before and use bus_format
from the connector. This way, even if there are multiple bridges
in the display pipeline, the LCDIF will use the correct format.
Signed-off-by: Marek Vasut
Cc: Daniel Abrecht
Cc: Emil Velikov
Cc: Laurent Pinchart
Cc: Lucas Stach
Cc: Stefan Agner
---
drivers/gpu/drm/mxsfb/mxsfb_kms.c | 33 +++
1 file changed, 25 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
index 5bcc06c1ac0b..98d8ba0bae84 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
@@ -47,16 +47,13 @@ static u32 set_hsync_pulse_width(struct mxsfb_drm_private
*mxsfb, u32 val)
* Setup the MXSFB registers for decoding the pixels out of the framebuffer and
* outputting them on the bus.
*/
-static void mxsfb_set_formats(struct mxsfb_drm_private *mxsfb)
+static void mxsfb_set_formats(struct mxsfb_drm_private *mxsfb,
+ const u32 bus_format)
{
struct drm_device *drm = mxsfb->drm;
const u32 format = mxsfb->crtc.primary->state->fb->format->format;
- u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
u32 ctrl, ctrl1;
- if (mxsfb->connector->display_info.num_bus_formats)
- bus_format = mxsfb->connector->display_info.bus_formats[0];
-
DRM_DEV_DEBUG_DRIVER(drm->dev, "Using bus_format: 0x%08X\n",
bus_format);
@@ -222,7 +219,8 @@ static dma_addr_t mxsfb_get_fb_paddr(struct drm_plane
*plane)
return gem->paddr;
}
-static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb)
+static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb,
+const u32 bus_format)
{
struct drm_device *drm = mxsfb->crtc.dev;
struct drm_display_mode *m = >crtc.state->adjusted_mode;
@@ -247,7 +245,7 @@ static void mxsfb_crtc_mode_set_nofb(struct
mxsfb_drm_private *mxsfb)
if (mxsfb->devdata->has_overlay)
writel(0, mxsfb->base + LCDC_AS_CTRL);
- mxsfb_set_formats(mxsfb);
+ mxsfb_set_formats(mxsfb, bus_format);
clk_set_rate(mxsfb->clk, m->crtc_clock * 1000);
@@ -345,7 +343,9 @@ static void mxsfb_crtc_atomic_enable(struct drm_crtc *crtc,
struct drm_atomic_state *state)
{
struct mxsfb_drm_private *mxsfb = to_mxsfb_drm_private(crtc->dev);
+ struct drm_bridge_state *bridge_state;
struct drm_device *drm = mxsfb->drm;
+ u32 bus_format = 0;
dma_addr_t paddr;
pm_runtime_get_sync(drm->dev);
@@ -353,7 +353,24 @@ static void mxsfb_crtc_atomic_enable(struct drm_crtc *crtc,
drm_crtc_vblank_on(crtc);
- mxsfb_crtc_mode_set_nofb(mxsfb);
+ /* If there is a bridge attached to the LCDIF, use its bus format */
+ if (mxsfb->bridge && state) {
+ bridge_state =
+ drm_atomic_get_new_bridge_state(state,
+ mxsfb->bridge);
+ if (bridge_state)
+ bus_format = bridge_state->input_bus_cfg.format;
+ }
+
+ /* If there is no bridge, use bus format from connector */
+ if (!bus_format && mxsfb->connector->display_info.num_bus_formats)
+ bus_format = mxsfb->connector->display_info.bus_formats[0];
+
+ /* If all else fails, default to RGB888_1X24 */
+ if (!bus_format)
+ bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+
+ mxsfb_crtc_mode_set_nofb(mxsfb, bus_format);
/* Write cur_buf as well to avoid an initial corrupt frame */
paddr = mxsfb_get_fb_paddr(crtc->primary);
--
2.30.2
Re: [PATCH] drm/vc4: dsi: Only register our component once a DSI device is attached
Hi Dave,
On Sun, Jun 20, 2021 at 09:42:27PM +0300, Laurent Pinchart wrote:
> On Sun, Jun 20, 2021 at 03:29:03PM +0100, Dave Stevenson wrote:
> > On Sun, 20 Jun 2021 at 04:26, Laurent Pinchart wrote:
> > >
> > > Hi Maxime,
> > >
> > > I'm testing this, and I'm afraid it causes an issue with all the
> > > I2C-controlled bridges. I'm focussing on the newly merged ti-sn65dsi83
> > > driver at the moment, but other are affected the same way.
> > >
> > > With this patch, the DSI component is only added when the DSI device is
> > > attached to the host with mipi_dsi_attach(). In the ti-sn65dsi83 driver,
> > > this happens in the bridge attach callback, which is called when the
> > > bridge is attached by a call to drm_bridge_attach() in vc4_dsi_bind().
> > > This creates a circular dependency, and the DRM/KMS device is never
> > > created.
> > >
> > > How should this be solved ? Dave, I think you have shown an interest in
> > > the sn65dsi83 recently, any help would be appreciated. On a side note,
> > > I've tested the ti-sn65dsi83 driver on a v5.10 RPi kernel, without much
> > > success (on top of commit e1499baa0b0c I get a very weird frame rate -
> > > 147 fps of 99 fps instead of 60 fps - and nothing on the screen, and on
> > > top of the latest v5.10 RPi branch, I get lock-related warnings at every
> > > page flip), which is why I tried v5.12 and noticed this patch. Is it
> > > worth trying to bring up the display on the v5.10 RPi kernel in parallel
> > > to fixing the issue introduced in this patch, or is DSI known to be
> > > broken there ?
> >
> > I've been looking at SN65DSI83/4, but as I don't have any hardware
> > I've largely been suggesting things to try to those on the forums who
> > do [1].
> >
> > My branch at https://github.com/6by9/linux/tree/rpi-5.10.y-sn65dsi8x-marek
> > is the latest one I've worked on. It's rpi-5.10.y with Marek's driver
> > cherry-picked, and an overlay and simple-panel definition by others.
> > It also has a rework for vc4_dsi to use pm_runtime, instead of
> > breaking up the DSI bridge chain (which is flawed as it never calls
> > the bridge mode_set or mode_valid functions which sn65dsi83 relies
> > on).
> >
> > I ran it on Friday in the lab and encountered an issue with vc4_dsi
> > should vc4_dsi_encoder_mode_fixup wish for a divider of 7 (required
> > for this 800x1280 panel over 4 lanes) where it resulted in an invalid
> > mode configuration. That resulted in patch [2] which then gave me
> > sensible numbers.
> >
> > That branch with dtoverlay=vc4-kms-v3d and
> > dtoverlay=vc4-kms-dsi-ti-sn65dsi83 created all the expected devices,
> > and everything came up normally.
> > It was a busy day, but I think I even stuck a scope on the clock lanes
> > at that point and confirmed that they were at the link frequency
> > expected.
>
> Thanks, I'll test your branch and will report the results.
I had to apply the following diff to work around a crash:
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 55b6c53207f5..647426aa793a 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -525,6 +525,9 @@ static bool sn65dsi83_mode_fixup(struct drm_bridge *bridge,
/* The DSI format is always RGB888_1X24 */
list_for_each_entry(connector, >mode_config.connector_list, head)
{
+ if (!connector->display_info.bus_formats)
+ continue;
+
switch (connector->display_info.bus_formats[0]) {
case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
ctx->lvds_format_24bpp = false;
connector->display_info.bus_formats is NULL for the HDMI connectors, as
I have nothing connected to them, as well as for the writeback
connector.
Then, when running kmstest --flip, I get one warning per frame:
[ 29.762089] [drm:vc4_dsi_runtime_resume] *ERROR* vc4_dsi_runtime_resume:
[ 29.763200] [drm:vc4_dsi_runtime_resume] *ERROR* vc4_dsi_runtime_resume: All
good
[ 29.793861] [ cut here ]
[ 29.798572] WARNING: CPU: 2 PID: 249 at
drivers/gpu/drm/drm_modeset_lock.c:246 drm_modeset_lock+0xd0/0x100
[ 29.808365] Modules linked in: ipv6 bcm2835_codec(C) bcm2835_unicam
bcm2835_v4l2(C) bcm2835_isp(C) bcm2835_mmal_vchiq(C) v4l2_mem2mem
v4l2_dv_timings imx296 rtc_ds1307 videobuf2_vmallom
[ 29.855284] CPU: 2 PID: 249 Comm: kworker/u8:10 Tainted: G C
5.10.44-v8+ #23
[ 29.863756] Hardware name: Raspberry Pi Compute Module 4 Rev 1.0 (DT)
[ 29.870297] Workqueue: events_unbound commit_work
[ 29.875077] pstate: 8005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
[ 29.881172] pc : drm_modeset_lock+0xd0/0x100
[ 29.885506] lr : drm_atomic_get_new_or_current_crtc_state+0x6c/0x110
[ 29.891950] sp : ffc011fcbcb0
[ 29.895308] x29: ffc011fcbcb0 x28: ff80403fe780
[ 29.900705] x27: ff80415a2000 x26: ffc0106f
[ 29.906100] x25: x24: ff80420d3c80
[
[PATCH] drm: mxsfb: Disable overlay plane support for i.MX8MM/i.MX8MN
The iMX8MM and iMX8MN do not support the overlay plane, so they
are MXSFB V4. Add the compatible strings to reflect this. Note
that iMX8MQ does support the overlay plane, so it is MXSFB V6.
Signed-off-by: Marek Vasut
Cc: Daniel Abrecht
Cc: Emil Velikov
Cc: Laurent Pinchart
Cc: Lucas Stach
Cc: Stefan Agner
---
drivers/gpu/drm/mxsfb/mxsfb_drv.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index c277d3f61a5e..0ae4a3553304 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -308,6 +308,8 @@ static const struct of_device_id mxsfb_dt_ids[] = {
{ .compatible = "fsl,imx23-lcdif", .data = _devdata[MXSFB_V3], },
{ .compatible = "fsl,imx28-lcdif", .data = _devdata[MXSFB_V4], },
{ .compatible = "fsl,imx6sx-lcdif", .data = _devdata[MXSFB_V6], },
+ { .compatible = "fsl,imx8mm-lcdif", .data = _devdata[MXSFB_V4], },
+ { .compatible = "fsl,imx8mn-lcdif", .data = _devdata[MXSFB_V4], },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, mxsfb_dt_ids);
--
2.30.2
[PATCH] drm: mxsfb: Increase number of outstanding requests on V4 and newer HW
In case the DRAM is under high load, the MXSFB FIFO might underflow
and that causes visible artifacts. This could be triggered on i.MX8MM
using e.g. "$ memtester 128M" on a device with 1920x1080 panel. The
first "Stuck Address" test of the memtester will completely corrupt
the image on the panel and leave the MXSFB FIFO in odd state.
To avoid this underflow, increase number of outstanding requests to
DRAM from 2 to 16, which is the maximum. This mitigates the issue
and it can no longer be triggered.
Fixes: 45d59d704080 ("drm: Add new driver for MXSFB controller")
Signed-off-by: Marek Vasut
Cc: Daniel Abrecht
Cc: Emil Velikov
Cc: Laurent Pinchart
Cc: Lucas Stach
Cc: Stefan Agner
---
drivers/gpu/drm/mxsfb/mxsfb_drv.c | 3 +++
drivers/gpu/drm/mxsfb/mxsfb_drv.h | 1 +
drivers/gpu/drm/mxsfb/mxsfb_kms.c | 8
drivers/gpu/drm/mxsfb/mxsfb_regs.h | 8
4 files changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 6da93551e2e5..c277d3f61a5e 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -51,6 +51,7 @@ static const struct mxsfb_devdata mxsfb_devdata[] = {
.hs_wdth_mask = 0xff,
.hs_wdth_shift = 24,
.has_overlay= false,
+ .has_ctrl2 = false,
},
[MXSFB_V4] = {
.transfer_count = LCDC_V4_TRANSFER_COUNT,
@@ -59,6 +60,7 @@ static const struct mxsfb_devdata mxsfb_devdata[] = {
.hs_wdth_mask = 0x3fff,
.hs_wdth_shift = 18,
.has_overlay= false,
+ .has_ctrl2 = true,
},
[MXSFB_V6] = {
.transfer_count = LCDC_V4_TRANSFER_COUNT,
@@ -67,6 +69,7 @@ static const struct mxsfb_devdata mxsfb_devdata[] = {
.hs_wdth_mask = 0x3fff,
.hs_wdth_shift = 18,
.has_overlay= true,
+ .has_ctrl2 = true,
},
};
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h
b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
index 399d23e91ed1..7c720e226fdf 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.h
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
@@ -22,6 +22,7 @@ struct mxsfb_devdata {
unsigned inths_wdth_mask;
unsigned inths_wdth_shift;
boolhas_overlay;
+ boolhas_ctrl2;
};
struct mxsfb_drm_private {
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
index 01e0f525360f..5bcc06c1ac0b 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
@@ -107,6 +107,14 @@ static void mxsfb_enable_controller(struct
mxsfb_drm_private *mxsfb)
clk_prepare_enable(mxsfb->clk_disp_axi);
clk_prepare_enable(mxsfb->clk);
+ /* Increase number of outstanding requests on all supported IPs */
+ if (mxsfb->devdata->has_ctrl2) {
+ reg = readl(mxsfb->base + LCDC_V4_CTRL2);
+ reg &= ~CTRL2_SET_OUTSTANDING_REQS_MASK;
+ reg |= CTRL2_SET_OUTSTANDING_REQS_16;
+ writel(reg, mxsfb->base + LCDC_V4_CTRL2);
+ }
+
/* If it was disabled, re-enable the mode again */
writel(CTRL_DOTCLK_MODE, mxsfb->base + LCDC_CTRL + REG_SET);
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_regs.h
b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
index df90e960f495..694fea13e893 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_regs.h
+++ b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
@@ -15,6 +15,7 @@
#define LCDC_CTRL 0x00
#define LCDC_CTRL1 0x10
#define LCDC_V3_TRANSFER_COUNT 0x20
+#define LCDC_V4_CTRL2 0x20
#define LCDC_V4_TRANSFER_COUNT 0x30
#define LCDC_V4_CUR_BUF0x40
#define LCDC_V4_NEXT_BUF 0x50
@@ -61,6 +62,13 @@
#define CTRL1_CUR_FRAME_DONE_IRQ_ENBIT(13)
#define CTRL1_CUR_FRAME_DONE_IRQ BIT(9)
+#define CTRL2_SET_OUTSTANDING_REQS_1 0
+#define CTRL2_SET_OUTSTANDING_REQS_2 (0x1 << 21)
+#define CTRL2_SET_OUTSTANDING_REQS_4 (0x2 << 21)
+#define CTRL2_SET_OUTSTANDING_REQS_8 (0x3 << 21)
+#define CTRL2_SET_OUTSTANDING_REQS_16 (0x4 << 21)
+#define CTRL2_SET_OUTSTANDING_REQS_MASK(0x7 << 21)
+
#define TRANSFER_COUNT_SET_VCOUNT(x) (((x) & 0x) << 16)
#define TRANSFER_COUNT_GET_VCOUNT(x) (((x) >> 16) & 0x)
#define TRANSFER_COUNT_SET_HCOUNT(x) ((x) & 0x)
--
2.30.2
[PATCH] drm: mxsfb: Enable recovery on underflow
There is some sort of corner case behavior of the controller,
which could rarely be triggered at least on i.MX6SX connected
to 800x480 DPI panel and i.MX8MM connected to DPI->DSI->LVDS
bridged 1920x1080 panel (and likely on other setups too), where
the image on the panel shifts to the right and wraps around.
This happens either when the controller is enabled on boot or
even later during run time. The condition does not correct
itself automatically, i.e. the display image remains shifted.
It seems this problem is known and is due to sporadic underflows
of the LCDIF FIFO. While the LCDIF IP does have underflow/overflow
IRQs, neither of the IRQs trigger and neither IRQ status bit is
asserted when this condition occurs.
All known revisions of the LCDIF IP have CTRL1 RECOVER_ON_UNDERFLOW
bit, which is described in the reference manual since i.MX23 as
"
Set this bit to enable the LCDIF block to recover in the next
field/frame if there was an underflow in the current field/frame.
"
Enable this bit to mitigate the sporadic underflows.
Fixes: 45d59d704080 ("drm: Add new driver for MXSFB controller")
Signed-off-by: Marek Vasut
Cc: Daniel Abrecht
Cc: Emil Velikov
Cc: Laurent Pinchart
Cc: Lucas Stach
Cc: Stefan Agner
---
drivers/gpu/drm/mxsfb/mxsfb_kms.c | 29 +
drivers/gpu/drm/mxsfb/mxsfb_regs.h | 1 +
2 files changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
index 300e7bab0f43..01e0f525360f 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
@@ -115,6 +115,35 @@ static void mxsfb_enable_controller(struct
mxsfb_drm_private *mxsfb)
reg |= VDCTRL4_SYNC_SIGNALS_ON;
writel(reg, mxsfb->base + LCDC_VDCTRL4);
+ /*
+* Enable recovery on underflow.
+*
+* There is some sort of corner case behavior of the controller,
+* which could rarely be triggered at least on i.MX6SX connected
+* to 800x480 DPI panel and i.MX8MM connected to DPI->DSI->LVDS
+* bridged 1920x1080 panel (and likely on other setups too), where
+* the image on the panel shifts to the right and wraps around.
+* This happens either when the controller is enabled on boot or
+* even later during run time. The condition does not correct
+* itself automatically, i.e. the display image remains shifted.
+*
+* It seems this problem is known and is due to sporadic underflows
+* of the LCDIF FIFO. While the LCDIF IP does have underflow/overflow
+* IRQs, neither of the IRQs trigger and neither IRQ status bit is
+* asserted when this condition occurs.
+*
+* All known revisions of the LCDIF IP have CTRL1 RECOVER_ON_UNDERFLOW
+* bit, which is described in the reference manual since i.MX23 as
+* "
+* Set this bit to enable the LCDIF block to recover in the next
+* field/frame if there was an underflow in the current field/frame.
+* "
+* Enable this bit to mitigate the sporadic underflows.
+*/
+ reg = readl(mxsfb->base + LCDC_CTRL1);
+ reg |= CTRL1_RECOVER_ON_UNDERFLOW;
+ writel(reg, mxsfb->base + LCDC_CTRL1);
+
writel(CTRL_RUN, mxsfb->base + LCDC_CTRL + REG_SET);
}
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_regs.h
b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
index 55d28a27f912..df90e960f495 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_regs.h
+++ b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
@@ -54,6 +54,7 @@
#define CTRL_DF24 BIT(1)
#define CTRL_RUN BIT(0)
+#define CTRL1_RECOVER_ON_UNDERFLOW BIT(24)
#define CTRL1_FIFO_CLEAR BIT(21)
#define CTRL1_SET_BYTE_PACKAGING(x)(((x) & 0xf) << 16)
#define CTRL1_GET_BYTE_PACKAGING(x)(((x) >> 16) & 0xf)
--
2.30.2
[PATCH] drm/bridge: ti-sn65dsi83: Replace connector format patching with atomic_get_input_bus_fmts
Patching the connector format is causing various problematic
side effects. Implement .atomic_get_input_bus_fmts callback
instead, which sets up the input (DSI-end) format, and that
format can then be used in pipeline format negotiation between
the DSI-end of this bridge and the other component closer to
the scanout engine.
Signed-off-by: Marek Vasut
Cc: Adam Ford
Cc: Douglas Anderson
Cc: Frieder Schrempf
Cc: Jagan Teki
Cc: Laurent Pinchart
Cc: Linus Walleij
Cc: Loic Poulain
Cc: Philippe Schenker
Cc: Robert Foss
Cc: Sam Ravnborg
Cc: Stephen Boyd
Cc: Valentin Raevsky
Cc: dri-devel@lists.freedesktop.org
---
drivers/gpu/drm/bridge/ti-sn65dsi83.c | 35 ---
1 file changed, 31 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 750f2172ef08..32bda20f5dda 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -517,7 +517,6 @@ static bool sn65dsi83_mode_fixup(struct drm_bridge *bridge,
struct drm_display_mode *adj)
{
struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
- u32 input_bus_format = MEDIA_BUS_FMT_RGB888_1X24;
struct drm_encoder *encoder = bridge->encoder;
struct drm_device *ddev = encoder->dev;
struct drm_connector *connector;
@@ -550,14 +549,37 @@ static bool sn65dsi83_mode_fixup(struct drm_bridge
*bridge,
connector->display_info.bus_formats[0]);
break;
}
-
- drm_display_info_set_bus_formats(>display_info,
-_bus_format, 1);
}
return true;
}
+#define MAX_INPUT_SEL_FORMATS 1
+
+static u32 *
+sn65dsi83_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;
+
+ *num_input_fmts = 0;
+
+ input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
+GFP_KERNEL);
+ if (!input_fmts)
+ return NULL;
+
+ /* This is the DSI-end bus format */
+ input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
+ *num_input_fmts = 1;
+
+ return input_fmts;
+}
+
static const struct drm_bridge_funcs sn65dsi83_funcs = {
.attach = sn65dsi83_attach,
.pre_enable = sn65dsi83_pre_enable,
@@ -567,6 +589,11 @@ static const struct drm_bridge_funcs sn65dsi83_funcs = {
.mode_valid = sn65dsi83_mode_valid,
.mode_set = sn65dsi83_mode_set,
.mode_fixup = sn65dsi83_mode_fixup,
+
+ .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+ .atomic_reset = drm_atomic_helper_bridge_reset,
+ .atomic_get_input_bus_fmts = sn65dsi83_atomic_get_input_bus_fmts,
};
static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model
model)
--
2.30.2
[Bug 213391] AMDGPU retries page fault with some specific processes amdgpu and sometimes followed [gfxhub0] retry page fault until *ERROR* ring gfx timeout, but soft recovered
https://bugzilla.kernel.org/show_bug.cgi?id=213391 --- Comment #22 from dimit...@gmail.com --- Updated initrd also to 20210315, ran under 5.12.11-200.fc33 for a day or so without issues, now under 5.12.12-200.fc33, we'll see how it goes. For reference what's the best way to check the active/loaded firmware? I don't see anything obvious on dmesg or lspci -vv. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[PATCH v2 1/2] backlight: lm3630a: fix return code of .update_status() callback
According to .update_status() is supposed to
return 0 on success and a negative error code otherwise. Adapt
lm3630a_bank_a_update_status() to actually do it.
While touching that also add the error code to the failure message.
Signed-off-by: Uwe Kleine-König
---
drivers/video/backlight/lm3630a_bl.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/video/backlight/lm3630a_bl.c
b/drivers/video/backlight/lm3630a_bl.c
index e88a2b0e5904..16a2658a72e1 100644
--- a/drivers/video/backlight/lm3630a_bl.c
+++ b/drivers/video/backlight/lm3630a_bl.c
@@ -190,7 +190,7 @@ static int lm3630a_bank_a_update_status(struct
backlight_device *bl)
if ((pwm_ctrl & LM3630A_PWM_BANK_A) != 0) {
lm3630a_pwm_ctrl(pchip, bl->props.brightness,
bl->props.max_brightness);
- return bl->props.brightness;
+ return 0;
}
/* disable sleep */
@@ -210,8 +210,8 @@ static int lm3630a_bank_a_update_status(struct
backlight_device *bl)
return 0;
out_i2c_err:
- dev_err(pchip->dev, "i2c failed to access\n");
- return bl->props.brightness;
+ dev_err(pchip->dev, "i2c failed to access (%pe)\n", ERR_PTR(ret));
+ return ret;
}
static int lm3630a_bank_a_get_brightness(struct backlight_device *bl)
base-commit: 6efb943b8616ec53a5e444193dccf1af9ad627b5
--
2.30.2
[PATCH v2 2/2] backlight: lm3630a: convert to atomic PWM API and check for errors
The practical upside here is that this only needs a single API call to
program the hardware which (depending on the underlaying hardware) can
be more effective and prevents glitches.
Up to now the return value of the pwm functions was ignored. Fix this
and propagate the error to the caller.
Signed-off-by: Uwe Kleine-König
---
drivers/video/backlight/lm3630a_bl.c | 34 +---
1 file changed, 16 insertions(+), 18 deletions(-)
diff --git a/drivers/video/backlight/lm3630a_bl.c
b/drivers/video/backlight/lm3630a_bl.c
index 16a2658a72e1..99eb8277149b 100644
--- a/drivers/video/backlight/lm3630a_bl.c
+++ b/drivers/video/backlight/lm3630a_bl.c
@@ -52,6 +52,7 @@ struct lm3630a_chip {
struct gpio_desc *enable_gpio;
struct regmap *regmap;
struct pwm_device *pwmd;
+ struct pwm_state pwmd_state;
};
/* i2c access */
@@ -167,16 +168,19 @@ static int lm3630a_intr_config(struct lm3630a_chip *pchip)
return rval;
}
-static void lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max)
+static int lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max)
{
- unsigned int period = pchip->pdata->pwm_period;
- unsigned int duty = br * period / br_max;
+ int err;
- pwm_config(pchip->pwmd, duty, period);
- if (duty)
- pwm_enable(pchip->pwmd);
- else
- pwm_disable(pchip->pwmd);
+ pchip->pwmd_state.period = pchip->pdata->pwm_period;
+
+ err = pwm_set_relative_duty_cycle(>pwmd_state, br, br_max);
+ if (err)
+ return err;
+
+ pchip->pwmd_state.enabled = pchip->pwmd_state.duty_cycle ? true : false;
+
+ return pwm_apply_state(pchip->pwmd, >pwmd_state);
}
/* update and get brightness */
@@ -187,11 +191,9 @@ static int lm3630a_bank_a_update_status(struct
backlight_device *bl)
enum lm3630a_pwm_ctrl pwm_ctrl = pchip->pdata->pwm_ctrl;
/* pwm control */
- if ((pwm_ctrl & LM3630A_PWM_BANK_A) != 0) {
- lm3630a_pwm_ctrl(pchip, bl->props.brightness,
-bl->props.max_brightness);
- return 0;
- }
+ if ((pwm_ctrl & LM3630A_PWM_BANK_A) != 0)
+ return lm3630a_pwm_ctrl(pchip, bl->props.brightness,
+ bl->props.max_brightness);
/* disable sleep */
ret = lm3630a_update(pchip, REG_CTRL, 0x80, 0x00);
@@ -563,11 +565,7 @@ static int lm3630a_probe(struct i2c_client *client,
return PTR_ERR(pchip->pwmd);
}
- /*
-* FIXME: pwm_apply_args() should be removed when switching to
-* the atomic PWM API.
-*/
- pwm_apply_args(pchip->pwmd);
+ pwm_init_state(pchip->pwmd, >pwmd_state);
}
/* interrupt enable : irq 0 is not allowed */
--
2.30.2
Re: [PATCH] drm/vc4: dsi: Only register our component once a DSI device is attached
Hi Dave, On Sun, Jun 20, 2021 at 03:29:03PM +0100, Dave Stevenson wrote: > On Sun, 20 Jun 2021 at 04:26, Laurent Pinchart wrote: > > > > Hi Maxime, > > > > I'm testing this, and I'm afraid it causes an issue with all the > > I2C-controlled bridges. I'm focussing on the newly merged ti-sn65dsi83 > > driver at the moment, but other are affected the same way. > > > > With this patch, the DSI component is only added when the DSI device is > > attached to the host with mipi_dsi_attach(). In the ti-sn65dsi83 driver, > > this happens in the bridge attach callback, which is called when the > > bridge is attached by a call to drm_bridge_attach() in vc4_dsi_bind(). > > This creates a circular dependency, and the DRM/KMS device is never > > created. > > > > How should this be solved ? Dave, I think you have shown an interest in > > the sn65dsi83 recently, any help would be appreciated. On a side note, > > I've tested the ti-sn65dsi83 driver on a v5.10 RPi kernel, without much > > success (on top of commit e1499baa0b0c I get a very weird frame rate - > > 147 fps of 99 fps instead of 60 fps - and nothing on the screen, and on > > top of the latest v5.10 RPi branch, I get lock-related warnings at every > > page flip), which is why I tried v5.12 and noticed this patch. Is it > > worth trying to bring up the display on the v5.10 RPi kernel in parallel > > to fixing the issue introduced in this patch, or is DSI known to be > > broken there ? > > I've been looking at SN65DSI83/4, but as I don't have any hardware > I've largely been suggesting things to try to those on the forums who > do [1]. > > My branch at https://github.com/6by9/linux/tree/rpi-5.10.y-sn65dsi8x-marek > is the latest one I've worked on. It's rpi-5.10.y with Marek's driver > cherry-picked, and an overlay and simple-panel definition by others. > It also has a rework for vc4_dsi to use pm_runtime, instead of > breaking up the DSI bridge chain (which is flawed as it never calls > the bridge mode_set or mode_valid functions which sn65dsi83 relies > on). > > I ran it on Friday in the lab and encountered an issue with vc4_dsi > should vc4_dsi_encoder_mode_fixup wish for a divider of 7 (required > for this 800x1280 panel over 4 lanes) where it resulted in an invalid > mode configuration. That resulted in patch [2] which then gave me > sensible numbers. > > That branch with dtoverlay=vc4-kms-v3d and > dtoverlay=vc4-kms-dsi-ti-sn65dsi83 created all the expected devices, > and everything came up normally. > It was a busy day, but I think I even stuck a scope on the clock lanes > at that point and confirmed that they were at the link frequency > expected. Thanks, I'll test your branch and will report the results. > Coming back to this patch though, it isn't in 5.10 so I'm not seeing > the issues. As to the exact ordering of attaches, I can't claim > sufficient knowledge on that front. > I can try a cherry-pick of this patch to see what goes on, but it > won't be for a day or two. Let's see if Maxime has an opinion :-) > [1] Largely https://www.raspberrypi.org/forums/viewtopic.php?f=44=305690, > but ignore about the first 5 pages of the thread as different driver > versions were floating about. Most stuff after that is based on > Marek's driver. > [2] > https://github.com/6by9/linux/commit/c3c774136a1e946109048711d16974be8d520aaa > > > On Tue, Jul 07, 2020 at 12:19:12PM +0200, Maxime Ripard wrote: > > > If the DSI driver is the last to probe, component_add will try to run all > > > the bind callbacks straight away and return the error code. > > > > > > However, since we depend on a power domain, we're pretty much guaranteed > > > to > > > be in that case on the BCM2711, and are just lucky on the previous SoCs > > > since the v3d also depends on that power domain and is further in the > > > probe > > > order. > > > > > > In that case, the DSI host will not stick around in the system: the DSI > > > bind callback will be executed, will not find any DSI device attached and > > > will return EPROBE_DEFER, and we will then remove the DSI host and ask to > > > be probed later on. > > > > > > But since that host doesn't stick around, DSI devices like the RaspberryPi > > > touchscreen whose probe is not linked to the DSI host (unlike the usual > > > DSI > > > devices that will be probed through the call to mipi_dsi_host_register) > > > cannot attach to the DSI host, and we thus end up in a situation where the > > > DSI host cannot probe because the panel hasn't probed yet, and the panel > > > cannot probe because the DSI host hasn't yet. > > > > > > In order to break this cycle, let's wait until there's a DSI device that > > > attaches to the DSI host to register the component and allow to progress > > > further. > > > > > > Suggested-by: Andrzej Hajda > > > Signed-off-by: Maxime Ripard > > > --- > > > drivers/gpu/drm/vc4/vc4_dsi.c | 25 - > > > 1 file changed, 8 insertions(+), 17 deletions(-) > > > > > > diff --git
Re: [PATCH 0/4] delete useless function return values & remove meaningless if(r) check code
Am 19.06.21 um 08:43 schrieb Bernard Zhao: Function radeon_fence_driver_init always returns success, the function type maybe coule be changed to void. This patch series will first delete the check of the return value of the function call radeon_fence_driver_init, then, optimise the function declaration and function to void type. Please merge all of this into a single patch, I don't see any point separating this. Christian. Signed-off-by: Bernard Zhao Bernard Zhao (4): drm/radeon: remove meaningless if(r) check code drm/radeon: remove meaningless if(r) check code drm/radeon: remove meaningless if(r) check code drm/radeon: delete useless return values drivers/gpu/drm/radeon/cik.c | 4 +--- drivers/gpu/drm/radeon/evergreen.c| 4 +--- drivers/gpu/drm/radeon/ni.c | 4 +--- drivers/gpu/drm/radeon/r100.c | 4 +--- drivers/gpu/drm/radeon/r300.c | 4 +--- drivers/gpu/drm/radeon/r420.c | 5 + drivers/gpu/drm/radeon/r520.c | 4 +--- drivers/gpu/drm/radeon/r600.c | 4 +--- drivers/gpu/drm/radeon/radeon.h | 2 +- drivers/gpu/drm/radeon/radeon_fence.c | 5 + drivers/gpu/drm/radeon/rs400.c| 4 +--- drivers/gpu/drm/radeon/rs600.c| 4 +--- drivers/gpu/drm/radeon/rs690.c| 4 +--- drivers/gpu/drm/radeon/rv515.c| 4 +--- drivers/gpu/drm/radeon/rv770.c| 4 +--- drivers/gpu/drm/radeon/si.c | 4 +--- 16 files changed, 16 insertions(+), 48 deletions(-)
Re: [PATCH] drm/vc4: dsi: Only register our component once a DSI device is attached
Hi Laurent On Sun, 20 Jun 2021 at 04:26, Laurent Pinchart wrote: > > Hi Maxime, > > I'm testing this, and I'm afraid it causes an issue with all the > I2C-controlled bridges. I'm focussing on the newly merged ti-sn65dsi83 > driver at the moment, but other are affected the same way. > > With this patch, the DSI component is only added when the DSI device is > attached to the host with mipi_dsi_attach(). In the ti-sn65dsi83 driver, > this happens in the bridge attach callback, which is called when the > bridge is attached by a call to drm_bridge_attach() in vc4_dsi_bind(). > This creates a circular dependency, and the DRM/KMS device is never > created. > > How should this be solved ? Dave, I think you have shown an interest in > the sn65dsi83 recently, any help would be appreciated. On a side note, > I've tested the ti-sn65dsi83 driver on a v5.10 RPi kernel, without much > success (on top of commit e1499baa0b0c I get a very weird frame rate - > 147 fps of 99 fps instead of 60 fps - and nothing on the screen, and on > top of the latest v5.10 RPi branch, I get lock-related warnings at every > page flip), which is why I tried v5.12 and noticed this patch. Is it > worth trying to bring up the display on the v5.10 RPi kernel in parallel > to fixing the issue introduced in this patch, or is DSI known to be > broken there ? I've been looking at SN65DSI83/4, but as I don't have any hardware I've largely been suggesting things to try to those on the forums who do [1]. My branch at https://github.com/6by9/linux/tree/rpi-5.10.y-sn65dsi8x-marek is the latest one I've worked on. It's rpi-5.10.y with Marek's driver cherry-picked, and an overlay and simple-panel definition by others. It also has a rework for vc4_dsi to use pm_runtime, instead of breaking up the DSI bridge chain (which is flawed as it never calls the bridge mode_set or mode_valid functions which sn65dsi83 relies on). I ran it on Friday in the lab and encountered an issue with vc4_dsi should vc4_dsi_encoder_mode_fixup wish for a divider of 7 (required for this 800x1280 panel over 4 lanes) where it resulted in an invalid mode configuration. That resulted in patch [2] which then gave me sensible numbers. That branch with dtoverlay=vc4-kms-v3d and dtoverlay=vc4-kms-dsi-ti-sn65dsi83 created all the expected devices, and everything came up normally. It was a busy day, but I think I even stuck a scope on the clock lanes at that point and confirmed that they were at the link frequency expected. Coming back to this patch though, it isn't in 5.10 so I'm not seeing the issues. As to the exact ordering of attaches, I can't claim sufficient knowledge on that front. I can try a cherry-pick of this patch to see what goes on, but it won't be for a day or two. Hope that helps Dave [1] Largely https://www.raspberrypi.org/forums/viewtopic.php?f=44=305690, but ignore about the first 5 pages of the thread as different driver versions were floating about. Most stuff after that is based on Marek's driver. [2] https://github.com/6by9/linux/commit/c3c774136a1e946109048711d16974be8d520aaa > On Tue, Jul 07, 2020 at 12:19:12PM +0200, Maxime Ripard wrote: > > If the DSI driver is the last to probe, component_add will try to run all > > the bind callbacks straight away and return the error code. > > > > However, since we depend on a power domain, we're pretty much guaranteed to > > be in that case on the BCM2711, and are just lucky on the previous SoCs > > since the v3d also depends on that power domain and is further in the probe > > order. > > > > In that case, the DSI host will not stick around in the system: the DSI > > bind callback will be executed, will not find any DSI device attached and > > will return EPROBE_DEFER, and we will then remove the DSI host and ask to > > be probed later on. > > > > But since that host doesn't stick around, DSI devices like the RaspberryPi > > touchscreen whose probe is not linked to the DSI host (unlike the usual DSI > > devices that will be probed through the call to mipi_dsi_host_register) > > cannot attach to the DSI host, and we thus end up in a situation where the > > DSI host cannot probe because the panel hasn't probed yet, and the panel > > cannot probe because the DSI host hasn't yet. > > > > In order to break this cycle, let's wait until there's a DSI device that > > attaches to the DSI host to register the component and allow to progress > > further. > > > > Suggested-by: Andrzej Hajda > > Signed-off-by: Maxime Ripard > > --- > > drivers/gpu/drm/vc4/vc4_dsi.c | 25 - > > 1 file changed, 8 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c > > index eaf276978ee7..19aab4e7e209 100644 > > --- a/drivers/gpu/drm/vc4/vc4_dsi.c > > +++ b/drivers/gpu/drm/vc4/vc4_dsi.c > > @@ -1246,10 +1246,12 @@ static ssize_t vc4_dsi_host_transfer(struct > > mipi_dsi_host *host, > > return ret; > > } > > > > +static const struct component_ops
Re: [PATCH v3 0/8] Support DEVICE_GENERIC memory in migrate_vma_*
On Thu, Jun 17, 2021 at 10:16:57AM -0500, Alex Sierra wrote: > v1: > AMD is building a system architecture for the Frontier supercomputer with a > coherent interconnect between CPUs and GPUs. This hardware architecture allows > the CPUs to coherently access GPU device memory. We have hardware in our labs > and we are working with our partner HPE on the BIOS, firmware and software > for delivery to the DOE. > > The system BIOS advertises the GPU device memory (aka VRAM) as SPM > (special purpose memory) in the UEFI system address map. The amdgpu driver > looks > it up with lookup_resource and registers it with devmap as > MEMORY_DEVICE_GENERIC > using devm_memremap_pages. > > Now we're trying to migrate data to and from that memory using the > migrate_vma_* > helpers so we can support page-based migration in our unified memory > allocations, > while also supporting CPU access to those pages. > > This patch series makes a few changes to make MEMORY_DEVICE_GENERIC pages > behave > correctly in the migrate_vma_* helpers. We are looking for feedback about this > approach. If we're close, what's needed to make our patches acceptable > upstream? > If we're not close, any suggestions how else to achieve what we are trying to > do > (i.e. page migration and coherent CPU access to VRAM)? Is there a way we can test the codepaths touched by this patchset? It doesn't have to be via a complete qemu simulation of the GPU device memory, but some way of creating MEMORY_DEVICE_GENERIC subject to migrate_vma_* helpers so we can test for regressions moving forward. Thanks, - Ted
[Bug 213391] AMDGPU retries page fault with some specific processes amdgpu and sometimes followed [gfxhub0] retry page fault until *ERROR* ring gfx timeout, but soft recovered
https://bugzilla.kernel.org/show_bug.cgi?id=213391 --- Comment #21 from Dominic Letz (dominic.l...@berlin.de) --- So I'm running since 16th on 20210315 and it has been stable so far vs. multiple freezes a day before. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH AUTOSEL 5.4 07/15] drm/sun4i: dw-hdmi: Make HDMI PHY into a platform device
On Tue, Jun 15, 2021 at 09:26:16AM -0700, Saravana Kannan wrote: On Tue, Jun 15, 2021 at 8:50 AM Sasha Levin wrote: From: Saravana Kannan [ Upstream commit 9bf3797796f570b34438235a6a537df85832bdad ] On sunxi boards that use HDMI output, HDMI device probe keeps being avoided indefinitely with these repeated messages in dmesg: platform 1ee.hdmi: probe deferral - supplier 1ef.hdmi-phy not ready There's a fwnode_link being created with fw_devlink=on between hdmi and hdmi-phy nodes, because both nodes have 'compatible' property set. Fw_devlink code assumes that nodes that have compatible property set will also have a device associated with them by some driver eventually. This is not the case with the current sun8i-hdmi driver. fw_devlink isn't present in 5.4 or earlier. So technically this patch isn't needed. I'll drop it from <=5.4, thanks! -- Thanks, Sasha
[PATCH v6, 2/2] soc: mediatek: mmsys: Add mt8192 mmsys routing table
mt8192 has different routing registers than mt8183
Signed-off-by: Yongqiang Niu
---
drivers/soc/mediatek/mt8192-mmsys.h | 68 +
drivers/soc/mediatek/mtk-mmsys.c| 11 ++
2 files changed, 79 insertions(+)
create mode 100644 drivers/soc/mediatek/mt8192-mmsys.h
diff --git a/drivers/soc/mediatek/mt8192-mmsys.h
b/drivers/soc/mediatek/mt8192-mmsys.h
new file mode 100644
index 000..3179029
--- /dev/null
+++ b/drivers/soc/mediatek/mt8192-mmsys.h
@@ -0,0 +1,68 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __SOC_MEDIATEK_MT8192_MMSYS_H
+#define __SOC_MEDIATEK_MT8192_MMSYS_H
+
+#define MT8192_MMSYS_OVL_MOUT_EN 0xf04
+#define MT8192_DISP_OVL1_2L_MOUT_EN0xf08
+#define MT8192_DISP_OVL0_2L_MOUT_EN0xf18
+#define MT8192_DISP_OVL0_MOUT_EN 0xf1c
+#define MT8192_DISP_RDMA0_SEL_IN 0xf2c
+#define MT8192_DISP_RDMA0_SOUT_SEL 0xf30
+#define MT8192_DISP_CCORR0_SOUT_SEL0xf34
+#define MT8192_DISP_AAL0_SEL_IN0xf38
+#define MT8192_DISP_DITHER0_MOUT_EN0xf3c
+#define MT8192_DISP_DSI0_SEL_IN0xf40
+#define MT8192_DISP_OVL2_2L_MOUT_EN0xf4c
+
+#define MT8192_DISP_OVL0_GO_BLEND BIT(0)
+#define MT8192_DITHER0_MOUT_IN_DSI0BIT(0)
+#define MT8192_OVL0_MOUT_EN_DISP_RDMA0 BIT(0)
+#define MT8192_OVL2_2L_MOUT_EN_RDMA4 BIT(0)
+#define MT8192_DISP_OVL0_GO_BG BIT(1)
+#define MT8192_DISP_OVL0_2L_GO_BLEND BIT(2)
+#define MT8192_DISP_OVL0_2L_GO_BG BIT(3)
+#define MT8192_OVL1_2L_MOUT_EN_RDMA1 BIT(4)
+#define MT8192_OVL0_MOUT_EN_OVL0_2LBIT(4)
+#define MT8192_RDMA0_SEL_IN_OVL0_2L0x3
+#define MT8192_RDMA0_SOUT_COLOR0 0x1
+#define MT8192_CCORR0_SOUT_AAL00x1
+#define MT8192_AAL0_SEL_IN_CCORR0 0x1
+#define MT8192_DSI0_SEL_IN_DITHER0 0x1
+
+static const struct mtk_mmsys_routes mmsys_mt8192_routing_table[] = {
+ {
+ DDP_COMPONENT_OVL_2L0, DDP_COMPONENT_RDMA0,
+ MT8192_DISP_OVL0_2L_MOUT_EN, MT8192_OVL0_MOUT_EN_DISP_RDMA0,
+ }, {
+ DDP_COMPONENT_OVL_2L2, DDP_COMPONENT_RDMA4,
+ MT8192_DISP_OVL2_2L_MOUT_EN, MT8192_OVL2_2L_MOUT_EN_RDMA4
+ }, {
+ DDP_COMPONENT_DITHER, DDP_COMPONENT_DSI0,
+ MT8192_DISP_DITHER0_MOUT_EN, MT8192_DITHER0_MOUT_IN_DSI0
+ }, {
+ DDP_COMPONENT_OVL_2L0, DDP_COMPONENT_RDMA0,
+ MT8192_DISP_RDMA0_SEL_IN, MT8192_RDMA0_SEL_IN_OVL0_2L
+ }, {
+ DDP_COMPONENT_CCORR, DDP_COMPONENT_AAL0,
+ MT8192_DISP_AAL0_SEL_IN, MT8192_AAL0_SEL_IN_CCORR0
+ }, {
+ DDP_COMPONENT_DITHER, DDP_COMPONENT_DSI0,
+ MT8192_DISP_DSI0_SEL_IN, MT8192_DSI0_SEL_IN_DITHER0
+ }, {
+ DDP_COMPONENT_RDMA0, DDP_COMPONENT_COLOR0,
+ MT8192_DISP_RDMA0_SOUT_SEL, MT8192_RDMA0_SOUT_COLOR0
+ }, {
+ DDP_COMPONENT_CCORR, DDP_COMPONENT_AAL0,
+ MT8192_DISP_CCORR0_SOUT_SEL, MT8192_CCORR0_SOUT_AAL0
+ }, {
+ DDP_COMPONENT_OVL0, DDP_COMPONENT_OVL_2L0,
+ MT8192_MMSYS_OVL_MOUT_EN, MT8192_DISP_OVL0_GO_BG,
+ }, {
+ DDP_COMPONENT_OVL_2L0, DDP_COMPONENT_RDMA0,
+ MT8192_MMSYS_OVL_MOUT_EN, MT8192_DISP_OVL0_2L_GO_BLEND,
+ }
+};
+
+#endif /* __SOC_MEDIATEK_MT8192_MMSYS_H */
+
diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
index 080660e..de7b122 100644
--- a/drivers/soc/mediatek/mtk-mmsys.c
+++ b/drivers/soc/mediatek/mtk-mmsys.c
@@ -13,6 +13,7 @@
#include "mtk-mmsys.h"
#include "mt8167-mmsys.h"
#include "mt8183-mmsys.h"
+#include "mt8192-mmsys.h"
static const struct mtk_mmsys_driver_data mt2701_mmsys_driver_data = {
.clk_driver = "clk-mt2701-mm",
@@ -52,6 +53,12 @@
.num_routes = ARRAY_SIZE(mmsys_mt8183_routing_table),
};
+static const struct mtk_mmsys_driver_data mt8192_mmsys_driver_data = {
+ .clk_driver = "clk-mt8192-mm",
+ .routes = mmsys_mt8192_routing_table,
+ .num_routes = ARRAY_SIZE(mmsys_mt8192_routing_table),
+};
+
struct mtk_mmsys {
void __iomem *regs;
const struct mtk_mmsys_driver_data *data;
@@ -157,6 +164,10 @@ static int mtk_mmsys_probe(struct platform_device *pdev)
.compatible = "mediatek,mt8183-mmsys",
.data = _mmsys_driver_data,
},
+ {
+ .compatible = "mediatek,mt8192-mmsys",
+ .data = _mmsys_driver_data,
+ },
{ }
};
--
1.8.1.1.dirty
[PATCH v6, 1/2] soc: mediatek: mmsys: add comp OVL_2L2/POSTMASK/RDMA4
This patch add some more ddp component
OVL_2L2 is ovl which include 2 layers overlay
POSTMASK control round corner for display frame
RDMA4 read dma buffer
Change-Id: I464ea2dce6a312de8fad2cdbd94a4c71ab45af8f
Signed-off-by: Yongqiang Niu
Reviewed-by: Chun-Kuang Hu
Reviewed-by: Enric Balletbo i Serra
Signed-off-by: Yongqiang Niu
---
include/linux/soc/mediatek/mtk-mmsys.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/linux/soc/mediatek/mtk-mmsys.h
b/include/linux/soc/mediatek/mtk-mmsys.h
index 2228bf6..4bba275 100644
--- a/include/linux/soc/mediatek/mtk-mmsys.h
+++ b/include/linux/soc/mediatek/mtk-mmsys.h
@@ -29,13 +29,16 @@ enum mtk_ddp_comp_id {
DDP_COMPONENT_OVL0,
DDP_COMPONENT_OVL_2L0,
DDP_COMPONENT_OVL_2L1,
+ DDP_COMPONENT_OVL_2L2,
DDP_COMPONENT_OVL1,
+ DDP_COMPONENT_POSTMASK0,
DDP_COMPONENT_PWM0,
DDP_COMPONENT_PWM1,
DDP_COMPONENT_PWM2,
DDP_COMPONENT_RDMA0,
DDP_COMPONENT_RDMA1,
DDP_COMPONENT_RDMA2,
+ DDP_COMPONENT_RDMA4,
DDP_COMPONENT_UFOE,
DDP_COMPONENT_WDMA0,
DDP_COMPONENT_WDMA1,
--
1.8.1.1.dirty
[PATCH v6, 0/2] soc: mediatek: mmsys: add mt8192 mmsys support
base 5.13-rc1 Change since v5: - squash ddp component into one patch - add 8192 mmsys compatible data Yongqiang Niu (2): soc: mediatek: mmsys: add comp OVL_2L2/POSTMASK/RDMA4 soc: mediatek: mmsys: Add mt8192 mmsys routing table drivers/soc/mediatek/mt8192-mmsys.h| 68 ++ drivers/soc/mediatek/mtk-mmsys.c | 11 ++ include/linux/soc/mediatek/mtk-mmsys.h | 3 ++ 3 files changed, 82 insertions(+) create mode 100644 drivers/soc/mediatek/mt8192-mmsys.h -- 1.8.1.1.dirty
[PATCH v3 2/2] drm: protect drm_master pointers in drm_lease.c
Currently, direct copies of drm_file->master pointers should be
protected by drm_device.master_mutex when being dereferenced. This is
because drm_file->master is not invariant for the lifetime of
drm_file. If drm_file is not the creator of master, then
drm_file->is_master is false, and a call to drm_setmaster_ioctl will
invoke drm_new_set_master, which then allocates a new master for
drm_file and puts the old master.
Thus, without holding drm_device.master_mutex, the old value of
drm_file->master could be freed while it is being used by another
concurrent process.
In drm_lease.c, there are multiple instances where drm_file->master is
accessed and dereferenced while drm_device.master_mutex is not
held. This makes drm_lease.c vulnerable to use-after-free bugs.
We address this issue as follows:
1. Clarify in the kerneldoc that drm_file->master is protected by
drm_device.master_mutex.
2. Add a new drm_file_get_master() function that calls drm_master_get
on drm_file->master while holding on to drm_device.master_mutex. Since
drm_master_get increments the reference count of master, this
prevents master from being freed until we unreference it with
drm_master_put.
3. In each case where drm_file->master is directly accessed and
eventually dereferenced in drm_lease.c, we wrap the access in a call
to the new drm_file_get_master function, then unreference the master
pointer once we are done using it.
Reported-by: Daniel Vetter
Signed-off-by: Desmond Cheong Zhi Xi
---
drivers/gpu/drm/drm_auth.c | 22 ++
drivers/gpu/drm/drm_lease.c | 57 ++---
include/drm/drm_auth.h | 1 +
include/drm/drm_file.h | 15 --
4 files changed, 75 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 86d4b72e95cb..0c64a77c67a6 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -384,6 +384,28 @@ struct drm_master *drm_master_get(struct drm_master
*master)
}
EXPORT_SYMBOL(drm_master_get);
+/**
+ * drm_file_get_master - reference @file_priv->master
+ * @file_priv: DRM file private
+ *
+ * Increments the reference count of @file_priv->master and returns
+ * @file_priv->master.
+ *
+ * Master pointers returned from this function should be unreferenced using
+ * drm_master_put().
+ */
+struct drm_master *drm_file_get_master(struct drm_file *file_priv)
+{
+ struct drm_master *master;
+
+ mutex_lock(_priv->master->dev->master_mutex);
+ master = drm_master_get(file_priv->master);
+ mutex_unlock(_priv->master->dev->master_mutex);
+
+ return master;
+}
+EXPORT_SYMBOL(drm_file_get_master);
+
static void drm_master_destroy(struct kref *kref)
{
struct drm_master *master = container_of(kref, struct drm_master,
refcount);
diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
index da4f085fc09e..65eab82f8acc 100644
--- a/drivers/gpu/drm/drm_lease.c
+++ b/drivers/gpu/drm/drm_lease.c
@@ -107,10 +107,17 @@ static bool _drm_has_leased(struct drm_master *master,
int id)
*/
bool _drm_lease_held(struct drm_file *file_priv, int id)
{
+ bool ret;
+ struct drm_master *master;
+
if (!file_priv || !file_priv->master)
return true;
- return _drm_lease_held_master(file_priv->master, id);
+ master = drm_file_get_master(file_priv);
+ ret = _drm_lease_held_master(master, id);
+ drm_master_put();
+
+ return ret;
}
/**
@@ -132,10 +139,11 @@ bool drm_lease_held(struct drm_file *file_priv, int id)
if (!file_priv || !file_priv->master || !file_priv->master->lessor)
return true;
- master = file_priv->master;
+ master = drm_file_get_master(file_priv);
mutex_lock(>dev->mode_config.idr_mutex);
ret = _drm_lease_held_master(master, id);
mutex_unlock(>dev->mode_config.idr_mutex);
+ drm_master_put();
return ret;
}
@@ -158,7 +166,7 @@ uint32_t drm_lease_filter_crtcs(struct drm_file *file_priv,
uint32_t crtcs_in)
if (!file_priv || !file_priv->master || !file_priv->master->lessor)
return crtcs_in;
- master = file_priv->master;
+ master = drm_file_get_master(file_priv);
dev = master->dev;
count_in = count_out = 0;
@@ -177,6 +185,7 @@ uint32_t drm_lease_filter_crtcs(struct drm_file *file_priv,
uint32_t crtcs_in)
count_in++;
}
mutex_unlock(>dev->mode_config.idr_mutex);
+ drm_master_put();
return crtcs_out;
}
@@ -490,7 +499,7 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
size_t object_count;
int ret = 0;
struct idr leases;
- struct drm_master *lessor = lessor_priv->master;
+ struct drm_master *lessor;
struct drm_master *lessee = NULL;
struct file *lessee_file = NULL;
struct file *lessor_file = lessor_priv->filp;
@@ -502,12 +511,6 @@ int
[PATCH v3 1/2] drm: add a locked version of drm_is_current_master
While checking the master status of the DRM file in
drm_is_current_master(), the device's master mutex should be
held. Without the mutex, the pointer fpriv->master may be freed
concurrently by another process calling drm_setmaster_ioctl(). This
could lead to use-after-free errors when the pointer is subsequently
dereferenced in drm_lease_owner().
The callers of drm_is_current_master() from drm_auth.c hold the
device's master mutex, but external callers do not. Hence, we implement
drm_is_current_master_locked() to be used within drm_auth.c, and
modify drm_is_current_master() to grab the device's master mutex
before checking the master status.
Reported-by: Daniel Vetter
Signed-off-by: Desmond Cheong Zhi Xi
Reviewed-by: Emil Velikov
---
drivers/gpu/drm/drm_auth.c | 51 --
1 file changed, 32 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 232abbba3686..86d4b72e95cb 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -61,6 +61,35 @@
* trusted clients.
*/
+static bool drm_is_current_master_locked(struct drm_file *fpriv)
+{
+ lockdep_assert_held_once(>master->dev->master_mutex);
+
+ return fpriv->is_master && drm_lease_owner(fpriv->master) ==
fpriv->minor->dev->master;
+}
+
+/**
+ * drm_is_current_master - checks whether @priv is the current master
+ * @fpriv: DRM file private
+ *
+ * Checks whether @fpriv is current master on its device. This decides whether
a
+ * client is allowed to run DRM_MASTER IOCTLs.
+ *
+ * Most of the modern IOCTL which require DRM_MASTER are for kernel modesetting
+ * - the current master is assumed to own the non-shareable display hardware.
+ */
+bool drm_is_current_master(struct drm_file *fpriv)
+{
+ bool ret;
+
+ mutex_lock(>master->dev->master_mutex);
+ ret = drm_is_current_master_locked(fpriv);
+ mutex_unlock(>master->dev->master_mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL(drm_is_current_master);
+
int drm_getmagic(struct drm_device *dev, void *data, struct drm_file
*file_priv)
{
struct drm_auth *auth = data;
@@ -223,7 +252,7 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data,
if (ret)
goto out_unlock;
- if (drm_is_current_master(file_priv))
+ if (drm_is_current_master_locked(file_priv))
goto out_unlock;
if (dev->master) {
@@ -272,7 +301,7 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data,
if (ret)
goto out_unlock;
- if (!drm_is_current_master(file_priv)) {
+ if (!drm_is_current_master_locked(file_priv)) {
ret = -EINVAL;
goto out_unlock;
}
@@ -321,7 +350,7 @@ void drm_master_release(struct drm_file *file_priv)
if (file_priv->magic)
idr_remove(_priv->master->magic_map, file_priv->magic);
- if (!drm_is_current_master(file_priv))
+ if (!drm_is_current_master_locked(file_priv))
goto out;
drm_legacy_lock_master_cleanup(dev, master);
@@ -342,22 +371,6 @@ void drm_master_release(struct drm_file *file_priv)
mutex_unlock(>master_mutex);
}
-/**
- * drm_is_current_master - checks whether @priv is the current master
- * @fpriv: DRM file private
- *
- * Checks whether @fpriv is current master on its device. This decides whether
a
- * client is allowed to run DRM_MASTER IOCTLs.
- *
- * Most of the modern IOCTL which require DRM_MASTER are for kernel modesetting
- * - the current master is assumed to own the non-shareable display hardware.
- */
-bool drm_is_current_master(struct drm_file *fpriv)
-{
- return fpriv->is_master && drm_lease_owner(fpriv->master) ==
fpriv->minor->dev->master;
-}
-EXPORT_SYMBOL(drm_is_current_master);
-
/**
* drm_master_get - reference a master pointer
* @master: drm_master
--
2.25.1
[PATCH v3 0/2] drm: address potential UAF bugs with drm_master ptrs
This patch series addresses potential use-after-free errors when dereferencing pointers to struct drm_master. These were identified after one such bug was caught by Syzbot in drm_getunique(): https://syzkaller.appspot.com/bug?id=148d2f1dfac64af52ffd27b661981a540724f803 The series is broken up into two patches: 1. Implement a locked version of drm_is_current_master() function that's used within drm_auth.c. 2. Identify areas in drm_lease.c where pointers to struct drm_master are dereferenced, and ensure that the master pointers are not freed during use. Changes in v2 -> v3: - Patch 1: Move the definition of drm_is_current_master and the _locked version higher up in drm_auth.c to avoid needing a forward declaration of drm_is_current_master_locked. As suggested by Daniel Vetter. - Patch 2: Instead of leaking drm_device.master_mutex into drm_lease.c to protect drm_master pointers, add a new drm_file_get_master() function that returns drm_file->master while increasing its reference count, to prevent drm_file->master from being freed. As suggested by Daniel Vetter. Changes in v1 -> v2: - Patch 2: Move the lock and assignment before the DRM_DEBUG_LEASE in drm_mode_get_lease_ioctl, as suggested by Emil Velikov. Desmond Cheong Zhi Xi (2): drm: add a locked version of drm_is_current_master drm: protect drm_master pointers in drm_lease.c drivers/gpu/drm/drm_auth.c | 73 +++-- drivers/gpu/drm/drm_lease.c | 57 - include/drm/drm_auth.h | 1 + include/drm/drm_file.h | 15 ++-- 4 files changed, 107 insertions(+), 39 deletions(-) -- 2.25.1
Re: [v7 5/5] drm/panel-simple: Add Samsung ATNA33XC20
Hi Rajeev
On Sat, Jun 19, 2021 at 04:10:30PM +0530, Rajeev Nandan wrote:
> Add Samsung 13.3" FHD eDP AMOLED panel.
>
> Signed-off-by: Rajeev Nandan
> Reviewed-by: Douglas Anderson
> ---
>
> Changes in v4:
> - New
>
> Changes in v5:
> - Remove "uses_dpcd_backlight" property, not required now. (Douglas)
>
> Changes in v7:
> - Update disable_to_power_off and power_to_enable delays. (Douglas)
>
> drivers/gpu/drm/panel/panel-simple.c | 33 +
> 1 file changed, 33 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c
> index 86e5a45..4adc44a 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -3562,6 +3562,36 @@ static const struct panel_desc rocktech_rk101ii01d_ct
> = {
> .connector_type = DRM_MODE_CONNECTOR_LVDS,
> };
>
> +static const struct drm_display_mode samsung_atna33xc20_mode = {
> + .clock = 138770,
> + .hdisplay = 1920,
> + .hsync_start = 1920 + 48,
> + .hsync_end = 1920 + 48 + 32,
> + .htotal = 1920 + 48 + 32 + 80,
> + .vdisplay = 1080,
> + .vsync_start = 1080 + 8,
> + .vsync_end = 1080 + 8 + 8,
> + .vtotal = 1080 + 8 + 8 + 16,
> + .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC,
> +};
> +
> +static const struct panel_desc samsung_atna33xc20 = {
> + .modes = _atna33xc20_mode,
> + .num_modes = 1,
> + .bpc = 10,
> + .size = {
> + .width = 294,
> + .height = 165,
> + },
> + .delay = {
> + .disable_to_power_off = 200,
> + .power_to_enable = 400,
> + .hpd_absent_delay = 200,
> + .unprepare = 500,
> + },
> + .connector_type = DRM_MODE_CONNECTOR_eDP,
> +};
bus_format is missing. There should be a warning about this when you
probe the display.
The bpc of 10 in unusual, the current code warns if bpc is neither 6 nor
8. If 10 is correct then update the code to accept bpc=10.
Sam
> +
> static const struct drm_display_mode samsung_lsn122dl01_c01_mode = {
> .clock = 271560,
> .hdisplay = 2560,
> @@ -4563,6 +4593,9 @@ static const struct of_device_id platform_of_match[] = {
> .compatible = "rocktech,rk101ii01d-ct",
> .data = _rk101ii01d_ct,
> }, {
> + .compatible = "samsung,atna33xc20",
> + .data = _atna33xc20,
> + }, {
> .compatible = "samsung,lsn122dl01-c01",
> .data = _lsn122dl01_c01,
> }, {
> --
> 2.7.4
Re: [v7 1/5] drm/panel: add basic DP AUX backlight support
Hi Rajeev
On Sat, Jun 19, 2021 at 04:10:26PM +0530, Rajeev Nandan wrote:
> Some panels support backlight control over DP AUX channel using
> VESA's standard backlight control interface.
> Using new DRM eDP backlight helpers, add support to create and
> register a backlight for those panels in drm_panel to simplify
> the panel drivers.
>
> The panel driver with access to "struct drm_dp_aux" can create and
> register a backlight device using following code snippet in its
> probe() function:
>
> err = drm_panel_dp_aux_backlight(panel, aux);
> if (err)
> return err;
IT very good to have this supported by drm_panel, so we avoid
bolierplate in various drivers.
>
> Then drm_panel will handle backlight_(enable|disable) calls
> similar to the case when drm_panel_of_backlight() is used.
>
> Currently, we are not supporting one feature where the source
> device can combine the backlight brightness levels set through
> DP AUX and the BL_PWM_DIM eDP connector pin. Since it's not
> required for the basic backlight controls, it can be added later.
>
> Signed-off-by: Rajeev Nandan
> Reviewed-by: Douglas Anderson
> Reviewed-by: Lyude Paul
> ---
>
> (no changes since v6)
>
> Changes in v5:
> - New
>
> Changes in v6:
> - Fixed ordering of memory allocation (Douglas)
> - Updated word wrapping in a comment (Douglas)
>
> drivers/gpu/drm/drm_panel.c | 108
>
> include/drm/drm_panel.h | 15 --
> 2 files changed, 119 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index f634371..9e65342 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -26,12 +26,20 @@
> #include
>
> #include
> +#include
> #include
> #include
>
> static DEFINE_MUTEX(panel_lock);
> static LIST_HEAD(panel_list);
>
> +struct dp_aux_backlight {
> + struct backlight_device *base;
> + struct drm_dp_aux *aux;
> + struct drm_edp_backlight_info info;
> + bool enabled;
> +};
> +
> /**
> * DOC: drm panel
> *
> @@ -342,6 +350,106 @@ int drm_panel_of_backlight(struct drm_panel *panel)
> return 0;
> }
> EXPORT_SYMBOL(drm_panel_of_backlight);
> +
> +static int dp_aux_backlight_update_status(struct backlight_device *bd)
> +{
> + struct dp_aux_backlight *bl = bl_get_data(bd);
> + u16 brightness = backlight_get_brightness(bd);
backlight_get_brightness() returns an int, so using u16 seems wrong.
But then drm_edp_backlight_enable() uses u16 for level - so I guess it
is OK.
We use unsigned long, int, u16 for brightness. Looks like something one
could look at one day, but today is not that day.
> + int ret = 0;
> +
> + if (brightness > 0) {
Use backlight_is_blank(bd) here, as this is really what you test for.
I cannot see why you need the extra check on ->enabled?
Would it be sufficient to check backlight_is_blank() only?
> + if (!bl->enabled) {
> + drm_edp_backlight_enable(bl->aux, >info,
> brightness);
> + bl->enabled = true;
> + return 0;
> + }
> + ret = drm_edp_backlight_set_level(bl->aux, >info,
> brightness);
> + } else {
> + if (bl->enabled) {
> + drm_edp_backlight_disable(bl->aux, >info);
> + bl->enabled = false;
> + }
> + }
> +
> + return ret;
> +}
Sam
