Re: [PATCH 0/3] drm/panic: Fixes and graphical logo
Hi Stephen and Maxime, On Fri, Jun 14, 2024 at 12:00 AM Stephen Rothwell wrote: > On Thu, 13 Jun 2024 11:48:15 +0200 Geert Uytterhoeven > wrote: > > > > Has the drm-misc git repo moved? > > > > > > It moved to gitlab recently, the new url is > > > g...@gitlab.freedesktop.org:drm/misc/kernel.git > > > > Time to tell Stephen... > > linux-next has been using that URL for some time. OK. Looks like the top commit of last drm-misc PR [1] is part of the drm-misc-next branch. but not of the for-linux-next branch, while Stephen pulls the latter. Is that a drm-misc or a linux-next issue? Thanks! [1] a13aaf157467e694a3824d81304106b58d4c20d6 Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re:Re: [PATCH 13/14] drm/bridge: synopsys: Add DW HDMI QP TX controller driver
Hi Neil, At 2024-06-05 19:48:09, "Neil Armstrong" wrote: >On 05/06/2024 12:11, Cristian Ciocaltea wrote: >> On 6/5/24 12:34 AM, Cristian Ciocaltea wrote: >>> On 6/4/24 11:41 PM, Sam Ravnborg wrote: Hi Cristian. On Tue, Jun 04, 2024 at 10:32:04PM +0300, Cristian Ciocaltea wrote: > Hi Sam, > > On 6/1/24 5:32 PM, Sam Ravnborg wrote: >> Hi Cristian, >> >> a few drive-by comments below. >> >> Sam >> >> >>> + >>> +static const struct drm_connector_funcs dw_hdmi_qp_connector_funcs = { >>> + .fill_modes = drm_helper_probe_single_connector_modes, >>> + .detect = dw_hdmi_connector_detect, >>> + .destroy = drm_connector_cleanup, >>> + .force = dw_hdmi_qp_connector_force, >>> + .reset = drm_atomic_helper_connector_reset, >>> + .atomic_duplicate_state = >>> drm_atomic_helper_connector_duplicate_state, >>> + .atomic_destroy_state = >>> drm_atomic_helper_connector_destroy_state, >>> +}; >>> + >>> +static int dw_hdmi_qp_bridge_attach(struct drm_bridge *bridge, >>> + enum drm_bridge_attach_flags flags) >>> +{ >>> + struct dw_hdmi *hdmi = bridge->driver_private; >>> + >>> + if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) >>> + return drm_bridge_attach(bridge->encoder, >>> hdmi->next_bridge, >>> +bridge, flags); >>> + >>> + return dw_hdmi_connector_create(hdmi, >>> &dw_hdmi_qp_connector_funcs); >>> +} >> >> Are there any users left that requires the display driver to create the >> connector? >> In other words - could this driver fail if DRM_BRIDGE_ATTACH_NO_CONNECTOR >> is not passed and drop dw_hdmi_connector_create()? >> >> I did not try to verify this - just a naive question. > > I've just tested this and it doesn't work - dw_hdmi_connector_create() > is still needed. Hmm, seems the display driver or some other bridge driver fails to support "DRM_BRIDGE_ATTACH_NO_CONNECTOR". what other drivers are involved? >>> >>> Could it be related to the glue driver (updated in the next patch) which >>> is also responsible for setting up the encoder? >>> Note that my comments here should be seen as potential future improvements, and do not block the patch from being used. >>> >>> Thanks for the heads up! Will try to get back to this soon and investigate. >> >> IIUC, modern bridges should not create the connector but rely on display >> drivers to take care of, which in this case is the VOP2 driver. However, >> it also handles some of the older SoCs relying on the non-QP variant of >> DW HDMI IP. Hence the existing dw-hdmi driver would be also impacted in >> order to come up with a proper solution. >> >> A quick check shows there are several users of this IP: >> >> $ git grep -E '= dw_hdmi_(bind|probe)\(' >> drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c:hdmi->dw_hdmi = >> dw_hdmi_probe(pdev, plat_data); >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c: hdmi = dw_hdmi_probe(pdev, >> plat_data); >> drivers/gpu/drm/imx/ipuv3/dw_hdmi-imx.c:hdmi->hdmi = >> dw_hdmi_probe(pdev, match->data); >> drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c: hdmi = dw_hdmi_probe(pdev, >> &ingenic_dw_hdmi_plat_data); >> drivers/gpu/drm/meson/meson_dw_hdmi.c: meson_dw_hdmi->hdmi = >> dw_hdmi_probe(pdev, &meson_dw_hdmi->dw_plat_data); >> drivers/gpu/drm/renesas/rcar-du/rcar_dw_hdmi.c: hdmi = dw_hdmi_probe(pdev, >> &rcar_dw_hdmi_plat_data); >> drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c:hdmi->hdmi = >> dw_hdmi_bind(pdev, encoder, plat_data); >> drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c: hdmi->hdmi = dw_hdmi_bind(pdev, >> encoder, plat_data); >> >> I didn't check which display drivers would be involved, I'd guess there >> are quite a few of them as well. So it seems this ends up being a pretty >> complex task. > >If this would be a brand new driver, then it should only support >DRM_BRIDGE_ATTACH_NO_CONNECTOR, >so you should not create a connector from the driver. > >The fact dw-hdmi accepts an attach without the flag is for legacy purpose >since some DRM drivers haven't switched to DRM_BRIDGE_ATTACH_NO_CONNECTOR yes, >but it's a requirement for new bridges so at some point you'll need to make >sure the rockchip glue and drm driver supports DRM_BRIDGE_ATTACH_NO_CONNECTOR. > >This will greatly simplify the driver! Based on the previous discussion, the DW HDMI QP drivers will be implemented like this: Core bridge library: drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c Rockchip platform specific glue: drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c As a new bridge driver should only support DRM_BRIDGE_ATTACH_NO_CONNECTOR, Is it acceptable if we implement the connector at the rockchip glue dw_hdmi_qp-rockchip.c ? Our current combination is a bit co
[PATCH] video: fbdev: sis: clean up some inconsistent indenting
No functional modification involved. drivers/video/fbdev/sis/sis_main.c:2511 SiS_Sense30x() warn: inconsistent indenting. Reported-by: Abaci Robot Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=9330 Signed-off-by: Jiapeng Chong --- drivers/video/fbdev/sis/sis_main.c | 502 +++-- 1 file changed, 260 insertions(+), 242 deletions(-) diff --git a/drivers/video/fbdev/sis/sis_main.c b/drivers/video/fbdev/sis/sis_main.c index 009bf1d92644..fc5b9a0d78bf 100644 --- a/drivers/video/fbdev/sis/sis_main.c +++ b/drivers/video/fbdev/sis/sis_main.c @@ -2384,266 +2384,284 @@ static int SISDoSense(struct sis_video_info *ivideo, u16 type, u16 test) static void SiS_Sense30x(struct sis_video_info *ivideo) { -u8 backupP4_0d,backupP2_00,backupP2_4d,backupSR_1e,biosflag=0; -u16 svhs=0, svhs_c=0; -u16 cvbs=0, cvbs_c=0; -u16 vga2=0, vga2_c=0; -int myflag, result; -char stdstr[] = "sisfb: Detected"; -char tvstr[] = "TV connected to"; - -if(ivideo->vbflags2 & VB2_301) { - svhs = 0x00b9; cvbs = 0x00b3; vga2 = 0x00d1; - myflag = SiS_GetReg(SISPART4, 0x01); - if(myflag & 0x04) { - svhs = 0x00dd; cvbs = 0x00ee; vga2 = 0x00fd; - } -} else if(ivideo->vbflags2 & (VB2_301B | VB2_302B)) { - svhs = 0x016b; cvbs = 0x0174; vga2 = 0x0190; -} else if(ivideo->vbflags2 & (VB2_301LV | VB2_302LV)) { - svhs = 0x0200; cvbs = 0x0100; -} else if(ivideo->vbflags2 & (VB2_301C | VB2_302ELV | VB2_307T | VB2_307LV)) { - svhs = 0x016b; cvbs = 0x0110; vga2 = 0x0190; -} else - return; - -vga2_c = 0x0e08; svhs_c = 0x0404; cvbs_c = 0x0804; -if(ivideo->vbflags & (VB2_301LV|VB2_302LV|VB2_302ELV|VB2_307LV)) { - svhs_c = 0x0408; cvbs_c = 0x0808; -} - -biosflag = 2; -if(ivideo->haveXGIROM) { - biosflag = ivideo->bios_abase[0x58] & 0x03; -} else if(ivideo->newrom) { - if(ivideo->bios_abase[0x5d] & 0x04) biosflag |= 0x01; -} else if(ivideo->sisvga_engine == SIS_300_VGA) { - if(ivideo->bios_abase) { - biosflag = ivideo->bios_abase[0xfe] & 0x03; - } -} - -if(ivideo->chip == SIS_300) { - myflag = SiS_GetReg(SISSR, 0x3b); - if(!(myflag & 0x01)) vga2 = vga2_c = 0; -} - -if(!(ivideo->vbflags2 & VB2_SISVGA2BRIDGE)) { - vga2 = vga2_c = 0; -} - -backupSR_1e = SiS_GetReg(SISSR, 0x1e); -SiS_SetRegOR(SISSR, 0x1e, 0x20); - -backupP4_0d = SiS_GetReg(SISPART4, 0x0d); -if(ivideo->vbflags2 & VB2_30xC) { - SiS_SetRegANDOR(SISPART4, 0x0d, ~0x07, 0x01); -} else { - SiS_SetRegOR(SISPART4, 0x0d, 0x04); -} -SiS_DDC2Delay(&ivideo->SiS_Pr, 0x2000); - -backupP2_00 = SiS_GetReg(SISPART2, 0x00); -SiS_SetReg(SISPART2, 0x00, ((backupP2_00 | 0x1c) & 0xfc)); - -backupP2_4d = SiS_GetReg(SISPART2, 0x4d); -if(ivideo->vbflags2 & VB2_SISYPBPRBRIDGE) { - SiS_SetReg(SISPART2, 0x4d, (backupP2_4d & ~0x10)); -} - -if(!(ivideo->vbflags2 & VB2_30xCLV)) { - SISDoSense(ivideo, 0, 0); -} - -SiS_SetRegAND(SISCR, 0x32, ~0x14); - -if(vga2_c || vga2) { - if(SISDoSense(ivideo, vga2, vga2_c)) { - if(biosflag & 0x01) { -printk(KERN_INFO "%s %s SCART output\n", stdstr, tvstr); -SiS_SetRegOR(SISCR, 0x32, 0x04); - } else { -printk(KERN_INFO "%s secondary VGA connection\n", stdstr); -SiS_SetRegOR(SISCR, 0x32, 0x10); - } - } -} - -SiS_SetRegAND(SISCR, 0x32, 0x3f); - -if(ivideo->vbflags2 & VB2_30xCLV) { - SiS_SetRegOR(SISPART4, 0x0d, 0x04); -} - -if((ivideo->sisvga_engine == SIS_315_VGA) && (ivideo->vbflags2 & VB2_SISYPBPRBRIDGE)) { - SiS_SetReg(SISPART2, 0x4d, (backupP2_4d | 0x10)); - SiS_DDC2Delay(&ivideo->SiS_Pr, 0x2000); - if((result = SISDoSense(ivideo, svhs, 0x0604))) { - if((result = SISDoSense(ivideo, cvbs, 0x0804))) { -printk(KERN_INFO "%s %s YPbPr component output\n", stdstr, tvstr); -SiS_SetRegOR(SISCR, 0x32, 0x80); - } - } - SiS_SetReg(SISPART2, 0x4d, backupP2_4d); -} - -SiS_SetRegAND(SISCR, 0x32, ~0x03); - -if(!(ivideo->vbflags & TV_YPBPR)) { - if((result = SISDoSense(ivideo, svhs, svhs_c))) { - printk(KERN_INFO "%s %s SVIDEO output\n", stdstr, tvstr); - SiS_SetRegOR(SISCR, 0x32, 0x02); - } - if((biosflag & 0x02) || (!result)) { - if(SISDoSense(ivideo, cvbs, cvbs_c)) { -printk(KERN_INFO "%s %s COMPOSITE output\n", stdstr, tvstr); -SiS_SetRegOR(SISCR, 0x32, 0x01); - } - } -} - -SISDoSense(ivideo, 0, 0); - -SiS_SetReg(SISPART2, 0x00, backupP2_00); -SiS_SetReg(SISPART4, 0x0d, backupP4_0d); -SiS_SetReg(SISSR, 0x1e, backupSR_1e); - -if(ivideo->vbflags2 & VB2_30xCLV) { - biosflag = SiS_GetReg(SISPART2, 0x00); - if(biosflag & 0x20) { - for(myflag = 2; myflag > 0; myfla
Re: [PATCH v4 0/3] drm/panel-edp: remove several legacy compatibles used by the driver
On Fri, 14 Jun 2024 03:02:19 +0300, Dmitry Baryshkov wrote: > There are two ways to describe an eDP panel in device tree. The > recommended way is to add a device on the AUX bus, ideally using the > edp-panel compatible. The legacy way is to define a top-level platform > device for the panel. > > > [ ... ] Acked-by: Maxime Ripard Thanks! Maxime
Re: [PATCH v9 09/21] drm/mediatek: Fix XRGB setting error in OVL
Re: [PATCH v9 17/21] drm/mediatek: Support "Pre-multiplied" blending in Mixer
Re: [PATCH v9 16/21] drm/mediatek: Support "Pre-multiplied" blending in OVL
Re: [PATCH v9 16/21] drm/mediatek: Support "Pre-multiplied" blending in OVL
Re: [PATCH v9 15/21] drm/mediatek: Support "None" blending in Mixer
Re: [PATCH v9 14/21] drm/mediatek: Support "None" blending in OVL
Re: [PATCH v9 13/21] drm/mediatek: Support DRM plane alpha in Mixer
Re: [PATCH v9 12/21] drm/mediatek: Support DRM plane alpha in OVL
[Patch V3] i2c: imx-lpi2c: add eDMA mode support for LPI2C
From: Carlos Song Add eDMA mode support for LPI2C. There are some differences between TX DMA mode and RX DMA mode. LPI2C MTDR register is Controller Transmit Data Register. When lpi2c send data, it is tx cmd register and tx data fifo. When lpi2c receive data, it is just a rx cmd register. LPI2C MRDR register is Controller Receive Data Register, received data are stored in this. MTDR[8:10] is CMD field and MTDR[0:7] is DATA filed. +---+---+ | C M D | D A T A | +---+---+---+---+---+---+---+---+---+---+---+ | 10| 9 | 8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 | +---+---+---+---+---+---+---+---+---+---+---+ MRDR is Controller Receive Data Register. MRDR[0:7] is DATA filed. +---+ | D A T A | +---+---+---+---+---+---+---+---+ | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 | +---+---+---+---+---+---+---+---+ When the LPI2C controller needs to send data, tx cmd and 8-bit data should be written into MTDR: CMD: 000b: Transmit the value in DATA[7:0]. DATA: 8-bit data. If lpi2c controller needs to send N 8-bit data, just write N times (CMD(W) + DATA(u8)) to MTDR. When the LPI2C controller needs to receive data, rx cmd should be written into MTDR, the received data will be stored in the MRDR. MTDR(CMD): 001b: Receive (DATA[7:0] + 1) 8-bit data. MTDR(DATA): byte counter. MRDR(DATA): 8-bit data. So when lpi2c controller needs to receive N 8-bit data, 1. N <= 256: Write 1 time (CMD(R) + BYTE COUNT(N-1)) into MTDR and receive data from MRDR. 2. N > 256: Write N/256 times (CMD(R) + BYTE COUNT(255)) + 1 time (CMD(R) + BYTE COUNT(N%256)) into MTDR and receive data from MRDR. Due to these differences, when lpi2c is in DMA TX mode, only enable TX channel to send data. But when lpi2c is in DMA RX mode, TX and RX channel are both enabled, TX channel is used to send RX cmd and RX channel is used to receive data. Signed-off-by: Carlos Song Reviewed-by: Frank Li --- Change for V3: Optimize DMA timeout calculate function names and variables avoid confusing Change for V2: Optimized eDMA rx cmd buf free function to improve code readability --- drivers/i2c/busses/i2c-imx-lpi2c.c | 531 - 1 file changed, 523 insertions(+), 8 deletions(-) diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c index 0197786892a2..d1ef0e3aa3f5 100644 --- a/drivers/i2c/busses/i2c-imx-lpi2c.c +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c @@ -8,6 +8,8 @@ #include #include #include +#include +#include #include #include #include @@ -29,6 +31,7 @@ #define LPI2C_MCR 0x10/* i2c contrl register */ #define LPI2C_MSR 0x14/* i2c status register */ #define LPI2C_MIER 0x18/* i2c interrupt enable */ +#define LPI2C_MDER 0x1C/* i2c DMA enable */ #define LPI2C_MCFGR0 0x20/* i2c master configuration */ #define LPI2C_MCFGR1 0x24/* i2c master configuration */ #define LPI2C_MCFGR2 0x28/* i2c master configuration */ @@ -70,11 +73,14 @@ #define MCFGR1_AUTOSTOPBIT(8) #define MCFGR1_IGNACK BIT(9) #define MRDR_RXEMPTY BIT(14) +#define MDER_TDDE BIT(0) +#define MDER_RDDE BIT(1) #define I2C_CLK_RATIO 2 #define CHUNK_DATA 256 #define I2C_PM_TIMEOUT 10 /* ms */ +#define I2C_DMA_THRESHOLD 8 /* bytes */ enum lpi2c_imx_mode { STANDARD, /* 100+Kbps */ @@ -108,6 +114,22 @@ struct lpi2c_imx_struct { unsigned intrxfifosize; enum lpi2c_imx_mode mode; struct i2c_bus_recovery_info rinfo; + + boolcan_use_dma; + boolusing_pio_mode; + u8 rx_cmd_buf_len; + u16 *rx_cmd_buf; + u8 *dma_buf; + unsigned intdma_len; + unsigned inttx_burst_num; + unsigned intrx_burst_num; + resource_size_t phy_addr; + dma_addr_t dma_tx_addr; + dma_addr_t dma_addr; + enum dma_data_direction dma_direction; + struct dma_chan *chan_tx; + struct dma_chan *chan_rx; + struct i2c_msg *msg; }; static void lpi2c_imx_intctrl(struct lpi2c_imx_struct *lpi2c_imx, @@ -305,7 +327,7 @@ static int lpi2c_imx_master_disable(struct lpi2c_imx_struct *lpi2c_imx) return 0; } -static int lpi2c_imx_msg_complete(struct lpi2c_imx_struct *lpi2c_imx) +static int lpi2c_imx_pio_msg_complete(struct lpi2c_imx_struct *lpi2c_imx) { unsigned long time_left; @@ -451,6 +473,439 @@ static void lpi2c_imx_read(struct lpi2c_imx_struct *lpi2c_imx, lpi2c_imx_intctrl(lpi2c_imx, MIER_RDIE | MIER_NDIE); } +static bool is_use_dma(struct lpi2c_imx_struct *lpi2c_imx, struct i2c_msg *msg) +{ + if (!lpi2c_imx->can_use_dma) + return false; + + /* +* When the length of data is less than I2C_DMA_THRESHOLD, +* cpu
Re: [PATCH net-next v12 00/13] Device Memory TCP
On Thu, Jun 13, 2024 at 6:35 PM Jakub Kicinski wrote: > > On Thu, 13 Jun 2024 01:35:37 + Mina Almasry wrote: > > v12: > > https://patchwork.kernel.org/project/netdevbpf/list/?series=859747&state=* > > patches 5 and 6 transiently break the build > > ../include/trace/events/page_pool.h:65:23: error: use of undeclared > identifier 'NET_IOV' >65 | __entry->netmem & NET_IOV, __entry->pfn, > __entry->release) > | ^ > ../include/trace/events/page_pool.h:91:23: error: use of undeclared > identifier 'NET_IOV' >91 | __entry->netmem & NET_IOV, __entry->pfn, > __entry->hold) > | ^ > > Looking at NIPA status the builders are 12h behind, so please don't > repost immediately. This series takes a lot of compute cycles to build. > > FWIW there is a docker version of NIPA checks in the nipa repo. > > https://github.com/linux-netdev/nipa/tree/main/docker > > IDK if it still works, but could help avoid mistakes.. My sincere apologies. I have trouble with the patch-by-patch allmodconfig build being very slow on my setup with the headers I'm touching, and I've been running into false positives with the C=1 & W=1 checks. I've been trying to look at the nipa scripts and porting them over to my setup. I'll take a look at the docker image and if not, at least make sure the patch-by-patch allmodconfig with C=1 and W=1 is working. -- Thanks, Mina
[PATCH v2 0/2] CMRR patch fixes
Address following issues regarding CMRR 1. Describe target_rr_divider in struct drm_dp_as_sdp. 2. Use required macro to avoid overflow. -v2: - Remove extra line from commit message. Mitul Golani (2): drm/dp: Describe target_rr_divider in struct drm_dp_as_sdp drm/i915/display: Update calculation to avoid overflow drivers/gpu/drm/i915/display/intel_vrr.c | 9 + include/drm/display/drm_dp_helper.h | 1 + 2 files changed, 6 insertions(+), 4 deletions(-) -- 2.45.2
[PATCH v2 1/2] drm/dp: Describe target_rr_divider in struct drm_dp_as_sdp
Describe newly added parameter target_rr_divider in struct drm_dp_as_sdp. -v2: Remove extra line from commit message.(Lucas) Fixes: a20c6d954d75 ("drm/dp: Add refresh rate divider to struct representing AS SDP") Cc: Mitul Golani Cc: Arun R Murthy Cc: Suraj Kandpal Cc: Ankit Nautiyal Cc: Jani Nikula Cc: Stephen Rothwell Signed-off-by: Mitul Golani Reviewed-by: Ankit Nautiyal --- include/drm/display/drm_dp_helper.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h index ea03e1dd26ba..7f2567fa230d 100644 --- a/include/drm/display/drm_dp_helper.h +++ b/include/drm/display/drm_dp_helper.h @@ -112,6 +112,7 @@ struct drm_dp_vsc_sdp { * @target_rr: Target Refresh * @duration_incr_ms: Successive frame duration increase * @duration_decr_ms: Successive frame duration decrease + * @target_rr_divider: Target refresh rate divider * @mode: Adaptive Sync Operation Mode */ struct drm_dp_as_sdp { -- 2.45.2
[PATCH v2 2/2] drm/i915/display: Update calculation to avoid overflow
Update calculation to avoid overflow. -v2: Remove extra line from commit message.(Lucas) Fixes: 1676ecd303ac ("drm/i915: Compute CMRR and calculate vtotal") Cc: Mitul Golani Cc: Ankit Nautiyal Cc: Suraj Kandpal Cc: Jani Nikula Cc: Stephen Rothwell Signed-off-by: Mitul Golani Reviewed-by: Ankit Nautiyal --- drivers/gpu/drm/i915/display/intel_vrr.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c index eb5b62b54d32..6430da25957d 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.c +++ b/drivers/gpu/drm/i915/display/intel_vrr.c @@ -147,10 +147,11 @@ cmrr_get_vtotal(struct intel_crtc_state *crtc_state, bool video_mode_required) multiplier_n = 1000; } - crtc_state->cmrr.cmrr_n = - desired_refresh_rate * adjusted_mode->crtc_htotal * multiplier_n; - vtotal = (adjusted_mode->crtc_clock * 1000 * multiplier_n) / crtc_state->cmrr.cmrr_n; - adjusted_pixel_rate = adjusted_mode->crtc_clock * 1000 * multiplier_m; + crtc_state->cmrr.cmrr_n = mul_u32_u32(desired_refresh_rate * adjusted_mode->crtc_htotal, + multiplier_n); + vtotal = DIV_ROUND_UP_ULL(mul_u32_u32(adjusted_mode->crtc_clock * 1000, multiplier_n), + crtc_state->cmrr.cmrr_n); + adjusted_pixel_rate = mul_u32_u32(adjusted_mode->crtc_clock * 1000, multiplier_m); crtc_state->cmrr.cmrr_m = do_div(adjusted_pixel_rate, crtc_state->cmrr.cmrr_n); return vtotal; -- 2.45.2
Re: [PATCH v9 11/21] drm/mediatek: Add new color format MACROs in OVL
Re: [PATCH v9 10/21] drm/mediatek: Fix XRGB setting error in Mixer
[PATCH] drm/mediatek: Remove less-than-zero comparison of an unsigned value
From: Hsiao Chien Sung Fix a Coverity error that less-than-zero comparison of an unsigned value is never true. Fixes: 0d9eee9118b7 ("drm/mediatek: Add drm ovl_adaptor sub driver for MT8195") Signed-off-by: Hsiao Chien Sung --- drivers/gpu/drm/mediatek/mtk_ddp_comp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/mediatek/mtk_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_ddp_comp.c index 17b036411292..a66e46d0b45e 100644 --- a/drivers/gpu/drm/mediatek/mtk_ddp_comp.c +++ b/drivers/gpu/drm/mediatek/mtk_ddp_comp.c @@ -593,7 +593,7 @@ int mtk_ddp_comp_init(struct device_node *node, struct mtk_ddp_comp *comp, int ret; #endif - if (comp_id < 0 || comp_id >= DDP_COMPONENT_DRM_ID_MAX) + if (comp_id >= DDP_COMPONENT_DRM_ID_MAX) return -EINVAL; type = mtk_ddp_matches[comp_id].type; -- 2.18.0
[PATCH v9 08/21] drm/mediatek: Support RGBA8888 and RGBX8888 in OVL on MT8195
From: Hsiao Chien Sung Support RGBA and RGBX formats in OVL on MT8195. Reviewed-by: CK Hu Reviewed-by: AngeloGioacchino Del Regno Signed-off-by: Hsiao Chien Sung --- drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c index 878bfb966ed7..946b87ec48ca 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c @@ -114,6 +114,8 @@ static const u32 mt8195_formats[] = { DRM_FORMAT_XBGR, DRM_FORMAT_XBGR2101010, DRM_FORMAT_ABGR2101010, + DRM_FORMAT_RGBX, + DRM_FORMAT_RGBA, DRM_FORMAT_RGBX1010102, DRM_FORMAT_RGBA1010102, DRM_FORMAT_RGB888, -- 2.18.0
[PATCH v9 19/21] drm/mediatek: Support CRC in display driver
From: Hsiao Chien Sung Register CRC related function pointers to support CRC retrieval. Signed-off-by: Hsiao Chien Sung --- drivers/gpu/drm/mediatek/mtk_crtc.c | 279 drivers/gpu/drm/mediatek/mtk_crtc.h | 38 drivers/gpu/drm/mediatek/mtk_ddp_comp.h | 5 + 3 files changed, 322 insertions(+) diff --git a/drivers/gpu/drm/mediatek/mtk_crtc.c b/drivers/gpu/drm/mediatek/mtk_crtc.c index 0df9bf695f65..5dcd8b95b81e 100644 --- a/drivers/gpu/drm/mediatek/mtk_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_crtc.c @@ -18,6 +18,7 @@ #include #include #include +#include #include "mtk_crtc.h" #include "mtk_ddp_comp.h" @@ -74,6 +75,9 @@ struct mtk_crtc { /* lock for display hardware access */ struct mutexhw_lock; boolconfig_updating; + + struct mtk_ddp_comp *crc_provider; + struct drm_vblank_work crc_work; boolsec_on; }; @@ -894,6 +898,88 @@ static void mtk_crtc_update_output(struct drm_crtc *crtc, } } +static void mtk_crtc_crc_work(struct kthread_work *base) +{ + struct drm_vblank_work *work = to_drm_vblank_work(base); + struct mtk_crtc *mtk_crtc = + container_of(work, typeof(*mtk_crtc), crc_work); + struct mtk_ddp_comp *comp = mtk_crtc->crc_provider; + + if (!comp) { + DRM_WARN("%s(crtc-%d): no crc provider\n", +__func__, drm_crtc_index(&mtk_crtc->base)); + return; + } + + if (mtk_crtc->base.crc.opened) { + u64 vblank = drm_crtc_vblank_count(&mtk_crtc->base); + + comp->funcs->crc_read(comp->dev); + + /* could take more than 50ms to finish */ + drm_crtc_add_crc_entry(&mtk_crtc->base, true, vblank, + comp->funcs->crc_entry(comp->dev)); + + drm_vblank_work_schedule(&mtk_crtc->crc_work, vblank + 1, true); + } else { + comp->funcs->crc_stop(comp->dev); + } +} + +static int mtk_crtc_set_crc_source(struct drm_crtc *crtc, const char *src) +{ + struct mtk_crtc *mtk_crtc = to_mtk_crtc(crtc); + struct mtk_ddp_comp *comp = mtk_crtc->crc_provider; + + if (!comp) { + DRM_ERROR("%s(crtc-%d): no crc provider\n", + __func__, drm_crtc_index(crtc)); + return -ENOENT; + } + + if (!src) + return -EINVAL; + + if (strcmp(src, "auto") != 0) { + DRM_ERROR("%s(crtc-%d): unknown source '%s'\n", + __func__, drm_crtc_index(crtc), src); + return -EINVAL; + } + + comp->funcs->crc_start(comp->dev); + + /* +* skip the first crc because the first frame (vblank + 1) is configured +* by mtk_crtc_ddp_hw_init() when atomic enable +*/ + drm_vblank_work_schedule(&mtk_crtc->crc_work, +drm_crtc_vblank_count(crtc) + 2, false); + return 0; +} + +static int mtk_crtc_verify_crc_source(struct drm_crtc *crtc, const char *src, + size_t *cnt) +{ + struct mtk_crtc *mtk_crtc = to_mtk_crtc(crtc); + struct mtk_ddp_comp *comp = mtk_crtc->crc_provider; + + if (!comp) { + DRM_ERROR("%s(crtc-%d): no crc provider\n", + __func__, drm_crtc_index(crtc)); + return -ENOENT; + } + + if (src && strcmp(src, "auto") != 0) { + DRM_ERROR("%s(crtc-%d): unknown source '%s'\n", + __func__, drm_crtc_index(crtc), src); + return -EINVAL; + } + + *cnt = comp->funcs->crc_cnt(comp->dev); + + return 0; +} + int mtk_crtc_plane_check(struct drm_crtc *crtc, struct drm_plane *plane, struct mtk_plane_state *state) { @@ -942,6 +1028,8 @@ static void mtk_crtc_atomic_enable(struct drm_crtc *crtc, drm_crtc_vblank_on(crtc); mtk_crtc->enabled = true; + + drm_vblank_work_init(&mtk_crtc->crc_work, crtc, mtk_crtc_crc_work); } static void mtk_crtc_atomic_disable(struct drm_crtc *crtc, @@ -1035,6 +1123,8 @@ static const struct drm_crtc_funcs mtk_crtc_funcs = { .atomic_destroy_state = mtk_crtc_destroy_state, .enable_vblank = mtk_crtc_enable_vblank, .disable_vblank = mtk_crtc_disable_vblank, + .set_crc_source = mtk_crtc_set_crc_source, + .verify_crc_source = mtk_crtc_verify_crc_source, }; static const struct drm_crtc_helper_funcs mtk_crtc_helper_funcs = { @@ -1228,6 +1318,13 @@ int mtk_crtc_create(struct drm_device *drm_dev, const unsigned int *path, if (comp->funcs->ctm_set) has_ctm = true; + + if (comp->funcs->crc_cnt && + comp-
[PATCH v9 07/21] drm/mediatek: Support more 10bit formats in OVL
From: Hsiao Chien Sung Support more 10bit formats in OVL. Reviewed-by: CK Hu Reviewed-by: AngeloGioacchino Del Regno Signed-off-by: Hsiao Chien Sung --- drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 32 ++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c index 20faac97f910..878bfb966ed7 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c @@ -71,6 +71,22 @@ #defineOVL_CON_VIRT_FLIP BIT(9) #defineOVL_CON_HORZ_FLIP BIT(10) +static inline bool is_10bit_rgb(u32 fmt) +{ + switch (fmt) { + case DRM_FORMAT_XRGB2101010: + case DRM_FORMAT_ARGB2101010: + case DRM_FORMAT_RGBX1010102: + case DRM_FORMAT_RGBA1010102: + case DRM_FORMAT_XBGR2101010: + case DRM_FORMAT_ABGR2101010: + case DRM_FORMAT_BGRX1010102: + case DRM_FORMAT_BGRA1010102: + return true; + } + return false; +} + static const u32 mt8173_formats[] = { DRM_FORMAT_XRGB, DRM_FORMAT_ARGB, @@ -88,12 +104,18 @@ static const u32 mt8173_formats[] = { static const u32 mt8195_formats[] = { DRM_FORMAT_XRGB, DRM_FORMAT_ARGB, + DRM_FORMAT_XRGB2101010, DRM_FORMAT_ARGB2101010, DRM_FORMAT_BGRX, DRM_FORMAT_BGRA, + DRM_FORMAT_BGRX1010102, DRM_FORMAT_BGRA1010102, DRM_FORMAT_ABGR, DRM_FORMAT_XBGR, + DRM_FORMAT_XBGR2101010, + DRM_FORMAT_ABGR2101010, + DRM_FORMAT_RGBX1010102, + DRM_FORMAT_RGBA1010102, DRM_FORMAT_RGB888, DRM_FORMAT_BGR888, DRM_FORMAT_RGB565, @@ -253,9 +275,7 @@ static void mtk_ovl_set_bit_depth(struct device *dev, int idx, u32 format, reg = readl(ovl->regs + DISP_REG_OVL_CLRFMT_EXT); reg &= ~OVL_CON_CLRFMT_BIT_DEPTH_MASK(idx); - if (format == DRM_FORMAT_RGBA1010102 || - format == DRM_FORMAT_BGRA1010102 || - format == DRM_FORMAT_ARGB2101010) + if (is_10bit_rgb(format)) bit_depth = OVL_CON_CLRFMT_10_BIT; reg |= OVL_CON_CLRFMT_BIT_DEPTH(bit_depth, idx); @@ -368,17 +388,23 @@ static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl, unsigned int fmt) return OVL_CON_CLRFMT_RGB888(ovl) | OVL_CON_BYTE_SWAP; case DRM_FORMAT_RGBX: case DRM_FORMAT_RGBA: + case DRM_FORMAT_RGBX1010102: + case DRM_FORMAT_RGBA1010102: return OVL_CON_CLRFMT_ARGB; case DRM_FORMAT_BGRX: case DRM_FORMAT_BGRA: + case DRM_FORMAT_BGRX1010102: case DRM_FORMAT_BGRA1010102: return OVL_CON_CLRFMT_ARGB | OVL_CON_BYTE_SWAP; case DRM_FORMAT_XRGB: case DRM_FORMAT_ARGB: + case DRM_FORMAT_XRGB2101010: case DRM_FORMAT_ARGB2101010: return OVL_CON_CLRFMT_RGBA; case DRM_FORMAT_XBGR: case DRM_FORMAT_ABGR: + case DRM_FORMAT_XBGR2101010: + case DRM_FORMAT_ABGR2101010: return OVL_CON_CLRFMT_RGBA | OVL_CON_BYTE_SWAP; case DRM_FORMAT_UYVY: return OVL_CON_CLRFMT_UYVY | OVL_CON_MTX_YUV_TO_RGB; -- 2.18.0
[PATCH v9 06/21] drm/mediatek: Turn off the layers with zero width or height
From: Hsiao Chien Sung We found that IGT (Intel GPU Tool) will try to commit layers with zero width or height and lead to undefined behaviors in hardware. Disable the layers in such a situation. Fixes: 777b7bc86a0a ("UPSTREAM: drm/mediatek: Add ovl_adaptor support for MT8195") Fixes: fa97fe71f6f9 ("UPSTREAM: drm/mediatek: Add ETHDR support for MT8195") Reviewed-by: CK Hu Reviewed-by: AngeloGioacchino Del Regno Signed-off-by: Hsiao Chien Sung --- drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c | 2 +- drivers/gpu/drm/mediatek/mtk_ethdr.c| 7 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c index ecb8246833a7..39bcc73326f0 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c @@ -158,7 +158,7 @@ void mtk_ovl_adaptor_layer_config(struct device *dev, unsigned int idx, merge = ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_MERGE0 + idx]; ethdr = ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_ETHDR0]; - if (!pending->enable) { + if (!pending->enable || !pending->width || !pending->height) { mtk_merge_stop_cmdq(merge, cmdq_pkt); mtk_mdp_rdma_stop(rdma_l, cmdq_pkt); mtk_mdp_rdma_stop(rdma_r, cmdq_pkt); diff --git a/drivers/gpu/drm/mediatek/mtk_ethdr.c b/drivers/gpu/drm/mediatek/mtk_ethdr.c index 29673611fa75..9796fd1d51f2 100644 --- a/drivers/gpu/drm/mediatek/mtk_ethdr.c +++ b/drivers/gpu/drm/mediatek/mtk_ethdr.c @@ -159,7 +159,12 @@ void mtk_ethdr_layer_config(struct device *dev, unsigned int idx, if (idx >= 4) return; - if (!pending->enable) { + if (!pending->enable || !pending->width || !pending->height) { + /* +* instead of disabling layer with MIX_SRC_CON directly +* set the size to 0 to avoid screen shift due to mixer +* mode switch (hardware behavior) +*/ mtk_ddp_write(cmdq_pkt, 0, &mixer->cmdq_base, mixer->regs, MIX_L_SRC_SIZE(idx)); return; } -- 2.18.0
[PATCH v9 18/21] drm/mediatek: Support alpha blending in display driver
From: Hsiao Chien Sung Support "Pre-multiplied" and "None" blend mode on MediaTek's chips by adding correct blend mode property when the planes init. Before this patch, only the "Coverage" mode (default) is supported. For more information, there are three pixel blend modes in DRM driver: "None", "Pre-multiplied", and "Coverage". To understand the difference between these modes, let's take a look at the following two approaches to do alpha blending: 1. Straight: dst.RGB = src.RGB * src.A + dst.RGB * (1 - src.A) This is straightforward and easy to understand, when the source layer is compositing with the destination layer, it's alpha will affect the result. This is also known as "post-multiplied", or "Coverage" mode. 2. Pre-multiplied: dst.RGB = src.RGB + dst.RGB * (1 - src.A) Since the source RGB have already multiplied its alpha, only destination RGB need to multiply it. This is the "Pre-multiplied" mode in DRM. For the "None" blend mode in DRM, it means the pixel alpha is ignored when compositing the layers, only the constant alpha for the composited layer will take effects. Reviewed-by: CK Hu Reviewed-by: AngeloGioacchino Del Regno Signed-off-by: Hsiao Chien Sung --- drivers/gpu/drm/mediatek/mtk_plane.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/mediatek/mtk_plane.c b/drivers/gpu/drm/mediatek/mtk_plane.c index 713e17473930..9762bba23273 100644 --- a/drivers/gpu/drm/mediatek/mtk_plane.c +++ b/drivers/gpu/drm/mediatek/mtk_plane.c @@ -354,6 +354,17 @@ int mtk_plane_init(struct drm_device *dev, struct drm_plane *plane, DRM_INFO("Create rotation property failed\n"); } + err = drm_plane_create_alpha_property(plane); + if (err) + DRM_ERROR("failed to create property: alpha\n"); + + err = drm_plane_create_blend_mode_property(plane, + BIT(DRM_MODE_BLEND_PREMULTI) | + BIT(DRM_MODE_BLEND_COVERAGE) | + BIT(DRM_MODE_BLEND_PIXEL_NONE)); + if (err) + DRM_ERROR("failed to create property: blend_mode\n"); + drm_plane_helper_add(plane, &mtk_plane_helper_funcs); return 0; -- 2.18.0
[PATCH v9 20/21] drm/mediatek: Support CRC in OVL
From: Hsiao Chien Sung We choose OVL as the CRC generator from other hardware components that are also capable of calculating CRCs, since its frame done event triggers vblanks, it can be used as a signal to know when is safe to retrieve CRC of the frame. Please note that position of the hardware component that is chosen as CRC generator in the display path is significant. For example, while OVL is the first module in VDOSYS0, its CRC won't be affected by the modules after it, which means effects applied by PQ, Gamma, Dither or any other components after OVL won't be calculated in CRC generation. Signed-off-by: Hsiao Chien Sung --- drivers/gpu/drm/mediatek/mtk_ddp_comp.c | 5 + drivers/gpu/drm/mediatek/mtk_disp_drv.h | 5 + drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 209 ++-- 3 files changed, 209 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_ddp_comp.c index dc2b36a8bdd6..c583869b110a 100644 --- a/drivers/gpu/drm/mediatek/mtk_ddp_comp.c +++ b/drivers/gpu/drm/mediatek/mtk_ddp_comp.c @@ -365,6 +365,11 @@ static const struct mtk_ddp_comp_funcs ddp_ovl = { .clk_enable = mtk_ovl_clk_enable, .clk_disable = mtk_ovl_clk_disable, .config = mtk_ovl_config, + .crc_cnt = mtk_ovl_crc_cnt, + .crc_entry = mtk_ovl_crc_entry, + .crc_read = mtk_ovl_crc_read, + .crc_start = mtk_ovl_crc_start, + .crc_stop = mtk_ovl_crc_stop, .start = mtk_ovl_start, .stop = mtk_ovl_stop, .register_vblank_cb = mtk_ovl_register_vblank_cb, diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h b/drivers/gpu/drm/mediatek/mtk_disp_drv.h index 082ac18fe04a..a03d7a10847a 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h @@ -105,6 +105,11 @@ void mtk_ovl_enable_vblank(struct device *dev); void mtk_ovl_disable_vblank(struct device *dev); const u32 *mtk_ovl_get_formats(struct device *dev); size_t mtk_ovl_get_num_formats(struct device *dev); +size_t mtk_ovl_crc_cnt(struct device *dev); +u32 *mtk_ovl_crc_entry(struct device *dev); +void mtk_ovl_crc_read(struct device *dev); +void mtk_ovl_crc_start(struct device *dev); +void mtk_ovl_crc_stop(struct device *dev); void mtk_ovl_adaptor_add_comp(struct device *dev, struct mtk_mutex *mutex); void mtk_ovl_adaptor_remove_comp(struct device *dev, struct mtk_mutex *mutex); diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c index 47d0b039a616..f43b31eec8ad 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c @@ -24,12 +24,20 @@ #define OVL_FME_CPL_INTBIT(1) #define DISP_REG_OVL_INTSTA0x0008 #define DISP_REG_OVL_EN0x000c +#define OVL_EN BIT(0) +#define OVL_OP_8BIT_MODE BIT(4) +#define OVL_HG_FOVL_CK_ON BIT(8) +#define OVL_HF_FOVL_CK_ON BIT(10) +#define DISP_REG_OVL_TRIG 0x0010 +#define OVL_CRC_EN BIT(8) +#define OVL_CRC_CLRBIT(9) #define DISP_REG_OVL_RST 0x0014 #define DISP_REG_OVL_ROI_SIZE 0x0020 #define DISP_REG_OVL_DATAPATH_CON 0x0024 #define OVL_LAYER_SMI_ID_ENBIT(0) #define OVL_BGCLR_SEL_IN BIT(2) #define OVL_LAYER_AFBC_EN(n) BIT(4+n) +#define OVL_OUTPUT_CLAMP BIT(26) #define DISP_REG_OVL_ROI_BGCLR 0x0028 #define DISP_REG_OVL_SRC_CON 0x002c #define DISP_REG_OVL_CON(n)(0x0030 + 0x20 * (n)) @@ -42,7 +50,26 @@ #define DISP_REG_OVL_RDMA_CTRL(n) (0x00c0 + 0x20 * (n)) #define DISP_REG_OVL_RDMA_GMC(n) (0x00c8 + 0x20 * (n)) #define DISP_REG_OVL_ADDR_MT2701 0x0040 +#define DISP_REG_OVL_CRC 0x0270 +#define OVL_CRC_OUT_MASK GENMASK(30, 0) #define DISP_REG_OVL_CLRFMT_EXT0x02D0 +#define DISP_REG_OVL_CLRFMT_EXT1 0x02D8 +#define OVL_CLRFMT_EXT1_CSC_EN(n) (1 << (((n) * 4) + 1)) +#define DISP_REG_OVL_Y2R_PARA_R0(n)(0x0134 + 0x28 * (n)) +#define OVL_Y2R_PARA_C_CF_RMY (GENMASK(14, 0)) +#define DISP_REG_OVL_Y2R_PARA_G0(n)(0x013c + 0x28 * (n)) +#define OVL_Y2R_PARA_C_CF_GMU (GENMASK(30, 16)) +#define DISP_REG_OVL_Y2R_PARA_B1(n)(0x0148 + 0x28 * (n)) +#define OVL_Y2R_PARA_C_CF_BMV (GENMASK(14, 0)) +#define DISP_REG_OVL_Y2R_PARA_YUV_A_0(n) (0x014c + 0x28 * (n)) +#define OVL_Y2R_PARA_C_CF_YA
[PATCH v9 14/21] drm/mediatek: Support "None" blending in OVL
From: Hsiao Chien Sung Support "None" alpha blending mode on MediaTek's chips. Signed-off-by: Hsiao Chien Sung --- drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c index 2316d4a6dca7..6567806cf4e2 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c @@ -431,6 +431,7 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx, unsigned int fmt = pending->format; unsigned int offset = (pending->y << 16) | pending->x; unsigned int src_size = (pending->height << 16) | pending->width; + unsigned int blend_mode = state->base.pixel_blend_mode; unsigned int ignore_pixel_alpha = 0; unsigned int con; bool is_afbc = pending->modifier != DRM_FORMAT_MOD_LINEAR; @@ -460,7 +461,8 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx, * For RGB888 related formats, whether CONST_BLD is enabled or not won't * affect the result. Therefore we use !has_alpha as the condition. */ - if (state->base.fb && !state->base.fb->format->has_alpha) + if ((state->base.fb && !state->base.fb->format->has_alpha) || + blend_mode == DRM_MODE_BLEND_PIXEL_NONE) ignore_pixel_alpha = OVL_CONST_BLEND; if (pending->rotation & DRM_MODE_REFLECT_Y) { -- 2.18.0
[PATCH v9 16/21] drm/mediatek: Support "Pre-multiplied" blending in OVL
From: Hsiao Chien Sung Support "Pre-multiplied" alpha blending mode on in OVL. Before this patch, only the "coverage" mode is supported. Signed-off-by: Hsiao Chien Sung --- drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 42 - 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c index 6567806cf4e2..47d0b039a616 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c @@ -52,8 +52,12 @@ #define GMC_THRESHOLD_HIGH ((1 << GMC_THRESHOLD_BITS) / 4) #define GMC_THRESHOLD_LOW ((1 << GMC_THRESHOLD_BITS) / 8) +#define OVL_CON_CLRFMT_MAN BIT(23) #define OVL_CON_BYTE_SWAP BIT(24) -#define OVL_CON_MTX_YUV_TO_RGB (6 << 16) + +/* OVL_CON_RGB_SWAP works only if OVL_CON_CLRFMT_MAN is enabled */ +#define OVL_CON_RGB_SWAP BIT(25) + #define OVL_CON_CLRFMT_RGB (1 << 12) #define OVL_CON_CLRFMT_ARGB(2 << 12) #define OVL_CON_CLRFMT_RGBA(3 << 12) @@ -61,6 +65,11 @@ #define OVL_CON_CLRFMT_BGRA(OVL_CON_CLRFMT_ARGB | OVL_CON_BYTE_SWAP) #define OVL_CON_CLRFMT_UYVY(4 << 12) #define OVL_CON_CLRFMT_YUYV(5 << 12) +#define OVL_CON_MTX_YUV_TO_RGB (6 << 16) +#define OVL_CON_CLRFMT_PARGB ((3 << 12) | OVL_CON_CLRFMT_MAN) +#define OVL_CON_CLRFMT_PABGR (OVL_CON_CLRFMT_PARGB | OVL_CON_RGB_SWAP) +#define OVL_CON_CLRFMT_PBGRA (OVL_CON_CLRFMT_PARGB | OVL_CON_BYTE_SWAP) +#define OVL_CON_CLRFMT_PRGBA (OVL_CON_CLRFMT_PABGR | OVL_CON_BYTE_SWAP) #define OVL_CON_CLRFMT_RGB565(ovl) ((ovl)->data->fmt_rgb565_is_0 ? \ 0 : OVL_CON_CLRFMT_RGB) #define OVL_CON_CLRFMT_RGB888(ovl) ((ovl)->data->fmt_rgb565_is_0 ? \ @@ -74,6 +83,8 @@ #defineOVL_CON_VIRT_FLIP BIT(9) #defineOVL_CON_HORZ_FLIP BIT(10) +#define OVL_COLOR_ALPHAGENMASK(31, 24) + static inline bool is_10bit_rgb(u32 fmt) { switch (fmt) { @@ -298,7 +309,13 @@ void mtk_ovl_config(struct device *dev, unsigned int w, if (w != 0 && h != 0) mtk_ddp_write_relaxed(cmdq_pkt, h << 16 | w, &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_ROI_SIZE); - mtk_ddp_write_relaxed(cmdq_pkt, 0x0, &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_ROI_BGCLR); + + /* +* The background color must be opaque black (ARGB), +* otherwise the alpha blending will have no effect +*/ + mtk_ddp_write_relaxed(cmdq_pkt, OVL_COLOR_ALPHA, &ovl->cmdq_reg, + ovl->regs, DISP_REG_OVL_ROI_BGCLR); mtk_ddp_write(cmdq_pkt, 0x1, &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_RST); mtk_ddp_write(cmdq_pkt, 0x0, &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_RST); @@ -374,7 +391,8 @@ void mtk_ovl_layer_off(struct device *dev, unsigned int idx, DISP_REG_OVL_RDMA_CTRL(idx)); } -static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl, unsigned int fmt) +static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl, unsigned int fmt, + unsigned int blend_mode) { /* The return value in switch "MEM_MODE_INPUT_FORMAT_XXX" * is defined in mediatek HW data sheet. @@ -395,22 +413,30 @@ static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl, unsigned int fmt) case DRM_FORMAT_RGBA: case DRM_FORMAT_RGBX1010102: case DRM_FORMAT_RGBA1010102: - return OVL_CON_CLRFMT_RGBA; + return blend_mode == DRM_MODE_BLEND_COVERAGE ? + OVL_CON_CLRFMT_RGBA : + OVL_CON_CLRFMT_PRGBA; case DRM_FORMAT_BGRX: case DRM_FORMAT_BGRA: case DRM_FORMAT_BGRX1010102: case DRM_FORMAT_BGRA1010102: - return OVL_CON_CLRFMT_BGRA; + return blend_mode == DRM_MODE_BLEND_COVERAGE ? + OVL_CON_CLRFMT_BGRA : + OVL_CON_CLRFMT_PBGRA; case DRM_FORMAT_XRGB: case DRM_FORMAT_ARGB: case DRM_FORMAT_XRGB2101010: case DRM_FORMAT_ARGB2101010: - return OVL_CON_CLRFMT_ARGB; + return blend_mode == DRM_MODE_BLEND_COVERAGE ? + OVL_CON_CLRFMT_ARGB : + OVL_CON_CLRFMT_PARGB; case DRM_FORMAT_XBGR: case DRM_FORMAT_ABGR: case DRM_FORMAT_XBGR2101010: case DRM_FORMAT_ABGR2101010: - return OVL_CON_CLRFMT_ABGR; + return blend_mode == DRM_MODE_BLEND_COVERAGE ? + OVL_CON_CLRFMT_ABGR : + OVL_CON_CLRFMT_PABGR; case DRM_FORMAT_UYVY: return OVL_CON_CLRFMT_UYVY | OVL_CON_MTX_YUV_TO_RGB; case DRM_FORMAT_YUYV: @@ -450,7 +476,7 @@ void mtk_ovl_layer_config(struct device
[PATCH v9 15/21] drm/mediatek: Support "None" blending in Mixer
From: Hsiao Chien Sung Support "None" alpha blending mode on MediaTek's chips. Signed-off-by: Hsiao Chien Sung --- drivers/gpu/drm/mediatek/mtk_ethdr.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/mediatek/mtk_ethdr.c b/drivers/gpu/drm/mediatek/mtk_ethdr.c index 7eaafd44f320..907c0ed34c64 100644 --- a/drivers/gpu/drm/mediatek/mtk_ethdr.c +++ b/drivers/gpu/drm/mediatek/mtk_ethdr.c @@ -3,6 +3,7 @@ * Copyright (c) 2021 MediaTek Inc. */ +#include #include #include #include @@ -175,7 +176,8 @@ void mtk_ethdr_layer_config(struct device *dev, unsigned int idx, alpha_con |= state->base.alpha & MIXER_ALPHA; } - if (state->base.fb && !state->base.fb->format->has_alpha) { + if ((state->base.fb && !state->base.fb->format->has_alpha) || + state->base.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) { /* * Mixer doesn't support CONST_BLD mode, * use a trick to make the output equivalent -- 2.18.0
[PATCH v9 12/21] drm/mediatek: Support DRM plane alpha in OVL
From: Hsiao Chien Sung Set the plane alpha according to DRM plane property. Signed-off-by: Hsiao Chien Sung --- drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c index 1923bbd96014..2316d4a6dca7 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c @@ -450,8 +450,10 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx, } con = ovl_fmt_convert(ovl, fmt); - if (state->base.fb && state->base.fb->format->has_alpha) - con |= OVL_CON_AEN | OVL_CON_ALPHA; + if (state->base.fb) { + con |= OVL_CON_AEN; + con |= state->base.alpha & OVL_CON_ALPHA; + } /* CONST_BLD must be enabled for XRGB formats although the alpha channel * can be ignored, or OVL will still read the value from memory. -- 2.18.0
[PATCH v9 13/21] drm/mediatek: Support DRM plane alpha in Mixer
From: Hsiao Chien Sung Set the plane alpha according to DRM plane property. Signed-off-by: Hsiao Chien Sung --- drivers/gpu/drm/mediatek/mtk_ethdr.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_ethdr.c b/drivers/gpu/drm/mediatek/mtk_ethdr.c index 902dec03a7dd..7eaafd44f320 100644 --- a/drivers/gpu/drm/mediatek/mtk_ethdr.c +++ b/drivers/gpu/drm/mediatek/mtk_ethdr.c @@ -170,8 +170,10 @@ void mtk_ethdr_layer_config(struct device *dev, unsigned int idx, return; } - if (state->base.fb && state->base.fb->format->has_alpha) - alpha_con = MIXER_ALPHA_AEN | MIXER_ALPHA; + if (state->base.fb) { + alpha_con |= MIXER_ALPHA_AEN; + alpha_con |= state->base.alpha & MIXER_ALPHA; + } if (state->base.fb && !state->base.fb->format->has_alpha) { /* -- 2.18.0
[PATCH v9 05/21] drm/mediatek: Set DRM mode configs accordingly
From: Hsiao Chien Sung Set DRM mode configs limitation according to the hardware capabilities and pass the IGT checks as below: - The test "graphics.IgtKms.kms_plane" requires a frame buffer with width of 4512 pixels (> 4096). - The test "graphics.IgtKms.kms_cursor_crc" checks if the cursor size is defined, and run the test with cursor size from 1x1 to 512x512. Please notice that the test conditions may change as IGT is updated. Reviewed-by: CK Hu Reviewed-by: AngeloGioacchino Del Regno Signed-off-by: Hsiao Chien Sung --- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 22 ++ drivers/gpu/drm/mediatek/mtk_drm_drv.h | 4 2 files changed, 26 insertions(+) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c index 34d1a3aacde5..3737180e1e53 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c @@ -298,6 +298,9 @@ static const struct mtk_mmsys_driver_data mt8188_vdosys0_driver_data = { .conn_routes = mt8188_mtk_ddp_main_routes, .num_conn_routes = ARRAY_SIZE(mt8188_mtk_ddp_main_routes), .mmsys_dev_num = 2, + .max_width = 8191, + .min_width = 1, + .min_height = 1, }; static const struct mtk_mmsys_driver_data mt8192_mmsys_driver_data = { @@ -312,6 +315,9 @@ static const struct mtk_mmsys_driver_data mt8195_vdosys0_driver_data = { .main_path = mt8195_mtk_ddp_main, .main_len = ARRAY_SIZE(mt8195_mtk_ddp_main), .mmsys_dev_num = 2, + .max_width = 8191, + .min_width = 1, + .min_height = 1, }; static const struct mtk_mmsys_driver_data mt8195_vdosys1_driver_data = { @@ -319,6 +325,9 @@ static const struct mtk_mmsys_driver_data mt8195_vdosys1_driver_data = { .ext_len = ARRAY_SIZE(mt8195_mtk_ddp_ext), .mmsys_id = 1, .mmsys_dev_num = 2, + .max_width = 8191, + .min_width = 2, /* 2-pixel align when ethdr is bypassed */ + .min_height = 1, }; static const struct of_device_id mtk_drm_of_ids[] = { @@ -497,6 +506,15 @@ static int mtk_drm_kms_init(struct drm_device *drm) for (j = 0; j < private->data->mmsys_dev_num; j++) { priv_n = private->all_drm_private[j]; + if (priv_n->data->max_width) + drm->mode_config.max_width = priv_n->data->max_width; + + if (priv_n->data->min_width) + drm->mode_config.min_width = priv_n->data->min_width; + + if (priv_n->data->min_height) + drm->mode_config.min_height = priv_n->data->min_height; + if (i == CRTC_MAIN && priv_n->data->main_len) { ret = mtk_crtc_create(drm, priv_n->data->main_path, priv_n->data->main_len, j, @@ -524,6 +542,10 @@ static int mtk_drm_kms_init(struct drm_device *drm) } } + /* IGT will check if the cursor size is configured */ + drm->mode_config.cursor_width = drm->mode_config.max_width; + drm->mode_config.cursor_height = drm->mode_config.max_height; + /* Use OVL device for all DMA memory allocations */ crtc = drm_crtc_from_index(drm, 0); if (crtc) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h index 78d698ede1bf..ce897984de51 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h @@ -46,6 +46,10 @@ struct mtk_mmsys_driver_data { bool shadow_register; unsigned int mmsys_id; unsigned int mmsys_dev_num; + + u16 max_width; + u16 min_width; + u16 min_height; }; struct mtk_drm_private { -- 2.18.0
[PATCH v9 21/21] drm/mediatek: Support CRC in OVL adaptor
From: Hsiao Chien Sung We choose Mixer as CRC generator in OVL adaptor since its frame done event will trigger vblanks, we can know when is safe to retrieve CRC of the frame. In OVL adaptor, there's no image procession after Mixer, unlike the OVL in VDOSYS0, Mixer's CRC will include all the effects that are applied to the frame. Signed-off-by: Hsiao Chien Sung --- drivers/gpu/drm/mediatek/mtk_ddp_comp.c | 5 ++ drivers/gpu/drm/mediatek/mtk_disp_drv.h | 5 ++ .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c | 35 + drivers/gpu/drm/mediatek/mtk_ethdr.c | 72 +++ drivers/gpu/drm/mediatek/mtk_ethdr.h | 7 ++ 5 files changed, 124 insertions(+) diff --git a/drivers/gpu/drm/mediatek/mtk_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_ddp_comp.c index c583869b110a..1207f855bf76 100644 --- a/drivers/gpu/drm/mediatek/mtk_ddp_comp.c +++ b/drivers/gpu/drm/mediatek/mtk_ddp_comp.c @@ -422,6 +422,11 @@ static const struct mtk_ddp_comp_funcs ddp_ovl_adaptor = { .clk_enable = mtk_ovl_adaptor_clk_enable, .clk_disable = mtk_ovl_adaptor_clk_disable, .config = mtk_ovl_adaptor_config, + .crc_cnt = mtk_ovl_adaptor_crc_cnt, + .crc_entry = mtk_ovl_adaptor_crc_entry, + .crc_read = mtk_ovl_adaptor_crc_read, + .crc_start = mtk_ovl_adaptor_crc_start, + .crc_stop = mtk_ovl_adaptor_crc_stop, .start = mtk_ovl_adaptor_start, .stop = mtk_ovl_adaptor_stop, .layer_nr = mtk_ovl_adaptor_layer_nr, diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h b/drivers/gpu/drm/mediatek/mtk_disp_drv.h index a03d7a10847a..0ef32bc3b996 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h @@ -140,6 +140,11 @@ const u32 *mtk_ovl_adaptor_get_formats(struct device *dev); size_t mtk_ovl_adaptor_get_num_formats(struct device *dev); enum drm_mode_status mtk_ovl_adaptor_mode_valid(struct device *dev, const struct drm_display_mode *mode); +size_t mtk_ovl_adaptor_crc_cnt(struct device *dev); +u32 *mtk_ovl_adaptor_crc_entry(struct device *dev); +void mtk_ovl_adaptor_crc_read(struct device *dev); +void mtk_ovl_adaptor_crc_start(struct device *dev); +void mtk_ovl_adaptor_crc_stop(struct device *dev); void mtk_rdma_bypass_shadow(struct device *dev); int mtk_rdma_clk_enable(struct device *dev); diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c index 39bcc73326f0..6c73007518ba 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c @@ -208,6 +208,41 @@ void mtk_ovl_adaptor_layer_config(struct device *dev, unsigned int idx, mtk_ethdr_layer_config(ethdr, idx, state, cmdq_pkt); } +size_t mtk_ovl_adaptor_crc_cnt(struct device *dev) +{ + struct mtk_disp_ovl_adaptor *ovl_adaptor = dev_get_drvdata(dev); + + return mtk_ethdr_crc_cnt(ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_ETHDR0]); +} + +u32 *mtk_ovl_adaptor_crc_entry(struct device *dev) +{ + struct mtk_disp_ovl_adaptor *ovl_adaptor = dev_get_drvdata(dev); + + return mtk_ethdr_crc_entry(ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_ETHDR0]); +} + +void mtk_ovl_adaptor_crc_read(struct device *dev) +{ + struct mtk_disp_ovl_adaptor *ovl_adaptor = dev_get_drvdata(dev); + + mtk_ethdr_crc_read(ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_ETHDR0]); +} + +void mtk_ovl_adaptor_crc_start(struct device *dev) +{ + struct mtk_disp_ovl_adaptor *ovl_adaptor = dev_get_drvdata(dev); + + mtk_ethdr_crc_start(ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_ETHDR0]); +} + +void mtk_ovl_adaptor_crc_stop(struct device *dev) +{ + struct mtk_disp_ovl_adaptor *ovl_adaptor = dev_get_drvdata(dev); + + mtk_ethdr_crc_stop(ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_ETHDR0]); +} + void mtk_ovl_adaptor_config(struct device *dev, unsigned int w, unsigned int h, unsigned int vrefresh, unsigned int bpc, struct cmdq_pkt *cmdq_pkt) diff --git a/drivers/gpu/drm/mediatek/mtk_ethdr.c b/drivers/gpu/drm/mediatek/mtk_ethdr.c index 0aa6b23287e5..84e0756f2e88 100644 --- a/drivers/gpu/drm/mediatek/mtk_ethdr.c +++ b/drivers/gpu/drm/mediatek/mtk_ethdr.c @@ -25,6 +25,9 @@ #define MIX_FME_CPL_INTEN BIT(1) #define MIX_INTSTA 0x8 #define MIX_EN 0xc +#define MIX_TRIG 0x10 +#define MIX_TRIG_CRC_ENBIT(8) +#define MIX_TRIG_CRC_RST BIT(9) #define MIX_RST0x14 #define MIX_ROI_SIZE 0x18 #define MIX_DATAPATH_CON 0x1c @@ -40,6 +43,11 @@ #define PREMULTI_SOURCE(3 << 12) #define MIX_L_SRC_SIZE(n) (0x30 + 0x18 * (n)) #define MIX_L_SRC_OFFSET(n)(0x34 + 0x18 * (n)
[PATCH v9 17/21] drm/mediatek: Support "Pre-multiplied" blending in Mixer
From: Hsiao Chien Sung Support "Pre-multiplied" alpha blending mode in Mixer. Before this patch, only the coverage mode is supported. Signed-off-by: Hsiao Chien Sung --- drivers/gpu/drm/mediatek/mtk_ethdr.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_ethdr.c b/drivers/gpu/drm/mediatek/mtk_ethdr.c index 907c0ed34c64..0aa6b23287e5 100644 --- a/drivers/gpu/drm/mediatek/mtk_ethdr.c +++ b/drivers/gpu/drm/mediatek/mtk_ethdr.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -36,6 +37,7 @@ #define MIX_SRC_L0_EN BIT(0) #define MIX_L_SRC_CON(n) (0x28 + 0x18 * (n)) #define NON_PREMULTI_SOURCE(2 << 12) +#define PREMULTI_SOURCE(3 << 12) #define MIX_L_SRC_SIZE(n) (0x30 + 0x18 * (n)) #define MIX_L_SRC_OFFSET(n)(0x34 + 0x18 * (n)) #define MIX_FUNC_DCM0 0x120 @@ -176,6 +178,11 @@ void mtk_ethdr_layer_config(struct device *dev, unsigned int idx, alpha_con |= state->base.alpha & MIXER_ALPHA; } + if (state->base.pixel_blend_mode != DRM_MODE_BLEND_COVERAGE) + alpha_con |= PREMULTI_SOURCE; + else + alpha_con |= NON_PREMULTI_SOURCE; + if ((state->base.fb && !state->base.fb->format->has_alpha) || state->base.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) { /* @@ -192,8 +199,7 @@ void mtk_ethdr_layer_config(struct device *dev, unsigned int idx, mtk_ddp_write(cmdq_pkt, pending->height << 16 | align_width, &mixer->cmdq_base, mixer->regs, MIX_L_SRC_SIZE(idx)); mtk_ddp_write(cmdq_pkt, offset, &mixer->cmdq_base, mixer->regs, MIX_L_SRC_OFFSET(idx)); - mtk_ddp_write_mask(cmdq_pkt, alpha_con, &mixer->cmdq_base, mixer->regs, MIX_L_SRC_CON(idx), - 0x1ff); + mtk_ddp_write(cmdq_pkt, alpha_con, &mixer->cmdq_base, mixer->regs, MIX_L_SRC_CON(idx)); mtk_ddp_write_mask(cmdq_pkt, BIT(idx), &mixer->cmdq_base, mixer->regs, MIX_SRC_CON, BIT(idx)); } -- 2.18.0
[PATCH v9 00/21] Support IGT in display driver
From: Hsiao Chien Sung This series adds support for running IGT (Intel GPU Tool) tests with MediaTek display driver. The following changes will be applied: 1. Add a new API for creating GCE thread loop to retrieve CRCs from the hardware component 2. Support hardware CRC calculation in both VDOSYS0 and VDOSYS1 3. Support alpha blending in both VDOSYS0 and VDOSYS1 Changes in v9: - Separate the patch into smaller ones Changes in v8: - Start/Stop CRC CMDQ thread on when needed - Squash and rearrange the commits - Add more information to the commit message and comments Changes in v7: - Separate the patch into smaller ones Changes in v6: - Use drm_vblank_work to deffer the CRC work into bottom halves - Separate the patches for "Premultiplied" and "None" alpha blending Changes in v5: - Add more descriptions to the codes - Add DRM mode configs to the driver data - Squash and rearrange the commits Changes in v4: - Separate the patch into smaller ones - Change the title of some patches - Revert the changes that are not related to the series Changes in v3: - Modify the dt-binding document of Mediatek OVL - Set DRM mode configs accroding to the hardware capabilities - Replace cmdq_pkt_jump_absolute() with cmdq_pkt_jump() Changes in v2: - Simplify CMDQ by adding commands that are currently used only - Integrate CRC related codes into new APIs for Mixer and OVL to reuse - Add CPU version CRC retrieval when CMDQ is disabled Hsiao Chien Sung (21): soc: mediatek: Disable 9-bit alpha in ETHDR drm/mediatek: Add OVL compatible name for MT8195 drm/mediatek: Add missing plane settings when async update drm/mediatek: Add DRM_MODE_ROTATE_0 to rotation property drm/mediatek: Set DRM mode configs accordingly drm/mediatek: Turn off the layers with zero width or height drm/mediatek: Support more 10bit formats in OVL drm/mediatek: Support RGBA and RGBX in OVL on MT8195 drm/mediatek: Fix XRGB setting error in OVL drm/mediatek: Fix XRGB setting error in Mixer drm/mediatek: Add new color format MACROs in OVL drm/mediatek: Support DRM plane alpha in OVL drm/mediatek: Support DRM plane alpha in Mixer drm/mediatek: Support "None" blending in OVL drm/mediatek: Support "None" blending in Mixer drm/mediatek: Support "Pre-multiplied" blending in OVL drm/mediatek: Support "Pre-multiplied" blending in Mixer drm/mediatek: Support alpha blending in display driver drm/mediatek: Support CRC in display driver drm/mediatek: Support CRC in OVL drm/mediatek: Support CRC in OVL adaptor drivers/gpu/drm/mediatek/mtk_crtc.c | 279 +++ drivers/gpu/drm/mediatek/mtk_crtc.h | 38 ++ drivers/gpu/drm/mediatek/mtk_ddp_comp.c | 10 + drivers/gpu/drm/mediatek/mtk_ddp_comp.h | 11 +- drivers/gpu/drm/mediatek/mtk_disp_drv.h | 10 + drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 330 +++--- .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c | 37 +- drivers/gpu/drm/mediatek/mtk_drm_drv.c| 24 ++ drivers/gpu/drm/mediatek/mtk_drm_drv.h| 4 + drivers/gpu/drm/mediatek/mtk_ethdr.c | 110 +- drivers/gpu/drm/mediatek/mtk_ethdr.h | 7 + drivers/gpu/drm/mediatek/mtk_plane.c | 15 +- drivers/soc/mediatek/mtk-mmsys.c | 1 + 13 files changed, 826 insertions(+), 50 deletions(-) -- 2.18.0
[PATCH v9 10/21] drm/mediatek: Fix XRGB setting error in Mixer
From: Hsiao Chien Sung Although the alpha channel in XRGB formats can be ignored, ALPHA_CON must be configured accordingly when using XRGB formats or it will still affects CRC generation. Fixes: d886c0009bd0 ("drm/mediatek: Add ETHDR support for MT8195") Signed-off-by: Hsiao Chien Sung --- drivers/gpu/drm/mediatek/mtk_ethdr.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_ethdr.c b/drivers/gpu/drm/mediatek/mtk_ethdr.c index 9796fd1d51f2..902dec03a7dd 100644 --- a/drivers/gpu/drm/mediatek/mtk_ethdr.c +++ b/drivers/gpu/drm/mediatek/mtk_ethdr.c @@ -153,6 +153,7 @@ void mtk_ethdr_layer_config(struct device *dev, unsigned int idx, unsigned int offset = (pending->x & 1) << 31 | pending->y << 16 | pending->x; unsigned int align_width = ALIGN_DOWN(pending->width, 2); unsigned int alpha_con = 0; + bool replace_src_a = false; dev_dbg(dev, "%s+ idx:%d", __func__, idx); @@ -172,8 +173,15 @@ void mtk_ethdr_layer_config(struct device *dev, unsigned int idx, if (state->base.fb && state->base.fb->format->has_alpha) alpha_con = MIXER_ALPHA_AEN | MIXER_ALPHA; - mtk_mmsys_mixer_in_config(priv->mmsys_dev, idx + 1, alpha_con ? false : true, - MIXER_ALPHA, + if (state->base.fb && !state->base.fb->format->has_alpha) { + /* +* Mixer doesn't support CONST_BLD mode, +* use a trick to make the output equivalent +*/ + replace_src_a = true; + } + + mtk_mmsys_mixer_in_config(priv->mmsys_dev, idx + 1, replace_src_a, MIXER_ALPHA, pending->x & 1 ? MIXER_INX_MODE_EVEN_EXTEND : MIXER_INX_MODE_BYPASS, align_width / 2 - 1, cmdq_pkt); -- 2.18.0
[PATCH v9 09/21] drm/mediatek: Fix XRGB setting error in OVL
From: Hsiao Chien Sung CONST_BLD must be enabled for XRGB formats although the alpha channel can be ignored, or OVL will still read the value from memory. This error only affects CRC generation. Fixes: c410fa9b07c3 ("drm/mediatek: Add AFBC support to Mediatek DRM driver") Signed-off-by: Hsiao Chien Sung --- drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c index 946b87ec48ca..fd390fb83d0e 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c @@ -38,6 +38,7 @@ #define DISP_REG_OVL_PITCH_MSB(n) (0x0040 + 0x20 * (n)) #define OVL_PITCH_MSB_2ND_SUBBUF BIT(16) #define DISP_REG_OVL_PITCH(n) (0x0044 + 0x20 * (n)) +#define OVL_CONST_BLENDBIT(28) #define DISP_REG_OVL_RDMA_CTRL(n) (0x00c0 + 0x20 * (n)) #define DISP_REG_OVL_RDMA_GMC(n) (0x00c8 + 0x20 * (n)) #define DISP_REG_OVL_ADDR_MT2701 0x0040 @@ -428,6 +429,7 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx, unsigned int fmt = pending->format; unsigned int offset = (pending->y << 16) | pending->x; unsigned int src_size = (pending->height << 16) | pending->width; + unsigned int ignore_pixel_alpha = 0; unsigned int con; bool is_afbc = pending->modifier != DRM_FORMAT_MOD_LINEAR; union overlay_pitch { @@ -449,6 +451,14 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx, if (state->base.fb && state->base.fb->format->has_alpha) con |= OVL_CON_AEN | OVL_CON_ALPHA; + /* CONST_BLD must be enabled for XRGB formats although the alpha channel +* can be ignored, or OVL will still read the value from memory. +* For RGB888 related formats, whether CONST_BLD is enabled or not won't +* affect the result. Therefore we use !has_alpha as the condition. +*/ + if (state->base.fb && !state->base.fb->format->has_alpha) + ignore_pixel_alpha = OVL_CONST_BLEND; + if (pending->rotation & DRM_MODE_REFLECT_Y) { con |= OVL_CON_VIRT_FLIP; addr += (pending->height - 1) * pending->pitch; @@ -464,8 +474,8 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx, mtk_ddp_write_relaxed(cmdq_pkt, con, &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_CON(idx)); - mtk_ddp_write_relaxed(cmdq_pkt, overlay_pitch.split_pitch.lsb, &ovl->cmdq_reg, ovl->regs, - DISP_REG_OVL_PITCH(idx)); + mtk_ddp_write_relaxed(cmdq_pkt, overlay_pitch.split_pitch.lsb | ignore_pixel_alpha, + &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_PITCH(idx)); mtk_ddp_write_relaxed(cmdq_pkt, src_size, &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_SRC_SIZE(idx)); mtk_ddp_write_relaxed(cmdq_pkt, offset, &ovl->cmdq_reg, ovl->regs, -- 2.18.0
[PATCH v9 11/21] drm/mediatek: Add new color format MACROs in OVL
From: Hsiao Chien Sung Define new color formats to hide the bit operation in the MACROs to make the switch statement more concise. Change the MACROs to align the naming rule in DRM. Signed-off-by: Hsiao Chien Sung --- drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c index fd390fb83d0e..1923bbd96014 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c @@ -55,8 +55,10 @@ #define OVL_CON_BYTE_SWAP BIT(24) #define OVL_CON_MTX_YUV_TO_RGB (6 << 16) #define OVL_CON_CLRFMT_RGB (1 << 12) -#define OVL_CON_CLRFMT_RGBA(2 << 12) -#define OVL_CON_CLRFMT_ARGB(3 << 12) +#define OVL_CON_CLRFMT_ARGB(2 << 12) +#define OVL_CON_CLRFMT_RGBA(3 << 12) +#define OVL_CON_CLRFMT_ABGR(OVL_CON_CLRFMT_RGBA | OVL_CON_BYTE_SWAP) +#define OVL_CON_CLRFMT_BGRA(OVL_CON_CLRFMT_ARGB | OVL_CON_BYTE_SWAP) #define OVL_CON_CLRFMT_UYVY(4 << 12) #define OVL_CON_CLRFMT_YUYV(5 << 12) #define OVL_CON_CLRFMT_RGB565(ovl) ((ovl)->data->fmt_rgb565_is_0 ? \ @@ -393,22 +395,22 @@ static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl, unsigned int fmt) case DRM_FORMAT_RGBA: case DRM_FORMAT_RGBX1010102: case DRM_FORMAT_RGBA1010102: - return OVL_CON_CLRFMT_ARGB; + return OVL_CON_CLRFMT_RGBA; case DRM_FORMAT_BGRX: case DRM_FORMAT_BGRA: case DRM_FORMAT_BGRX1010102: case DRM_FORMAT_BGRA1010102: - return OVL_CON_CLRFMT_ARGB | OVL_CON_BYTE_SWAP; + return OVL_CON_CLRFMT_BGRA; case DRM_FORMAT_XRGB: case DRM_FORMAT_ARGB: case DRM_FORMAT_XRGB2101010: case DRM_FORMAT_ARGB2101010: - return OVL_CON_CLRFMT_RGBA; + return OVL_CON_CLRFMT_ARGB; case DRM_FORMAT_XBGR: case DRM_FORMAT_ABGR: case DRM_FORMAT_XBGR2101010: case DRM_FORMAT_ABGR2101010: - return OVL_CON_CLRFMT_RGBA | OVL_CON_BYTE_SWAP; + return OVL_CON_CLRFMT_ABGR; case DRM_FORMAT_UYVY: return OVL_CON_CLRFMT_UYVY | OVL_CON_MTX_YUV_TO_RGB; case DRM_FORMAT_YUYV: -- 2.18.0
[PATCH v9 04/21] drm/mediatek: Add DRM_MODE_ROTATE_0 to rotation property
From: Hsiao Chien Sung Always add DRM_MODE_ROTATE_0 to rotation property to meet IGT's (Intel GPU Tools) requirement. Reviewed-by: CK Hu Reviewed-by: AngeloGioacchino Del Regno Signed-off-by: Hsiao Chien Sung --- drivers/gpu/drm/mediatek/mtk_ddp_comp.h | 6 +- drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 17 + drivers/gpu/drm/mediatek/mtk_plane.c| 2 +- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_ddp_comp.h index 792fd1b004ee..2d9cdcb7100c 100644 --- a/drivers/gpu/drm/mediatek/mtk_ddp_comp.h +++ b/drivers/gpu/drm/mediatek/mtk_ddp_comp.h @@ -193,7 +193,11 @@ unsigned int mtk_ddp_comp_supported_rotations(struct mtk_ddp_comp *comp) if (comp->funcs && comp->funcs->supported_rotations) return comp->funcs->supported_rotations(comp->dev); - return 0; + /* +* In order to pass IGT tests, DRM_MODE_ROTATE_0 is required when +* rotation is not supported. +*/ + return DRM_MODE_ROTATE_0; } static inline unsigned int mtk_ddp_comp_layer_nr(struct mtk_ddp_comp *comp) diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c index 14c9b30c599b..20faac97f910 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c @@ -296,27 +296,20 @@ int mtk_ovl_layer_check(struct device *dev, unsigned int idx, struct mtk_plane_state *mtk_state) { struct drm_plane_state *state = &mtk_state->base; - unsigned int rotation = 0; - rotation = drm_rotation_simplify(state->rotation, -DRM_MODE_ROTATE_0 | -DRM_MODE_REFLECT_X | -DRM_MODE_REFLECT_Y); - rotation &= ~DRM_MODE_ROTATE_0; - - /* We can only do reflection, not rotation */ - if ((rotation & DRM_MODE_ROTATE_MASK) != 0) + /* check if any unsupported rotation is set */ + if (state->rotation & ~mtk_ovl_supported_rotations(dev)) return -EINVAL; /* * TODO: Rotating/reflecting YUV buffers is not supported at this time. * Only RGB[AX] variants are supported. +* Since DRM_MODE_ROTATE_0 means "no rotation", we should not +* reject layers with this property. */ - if (state->fb->format->is_yuv && rotation != 0) + if (state->fb->format->is_yuv && (state->rotation & ~DRM_MODE_ROTATE_0)) return -EINVAL; - state->rotation = rotation; - return 0; } diff --git a/drivers/gpu/drm/mediatek/mtk_plane.c b/drivers/gpu/drm/mediatek/mtk_plane.c index d20770162736..713e17473930 100644 --- a/drivers/gpu/drm/mediatek/mtk_plane.c +++ b/drivers/gpu/drm/mediatek/mtk_plane.c @@ -346,7 +346,7 @@ int mtk_plane_init(struct drm_device *dev, struct drm_plane *plane, return err; } - if (supported_rotations & ~DRM_MODE_ROTATE_0) { + if (supported_rotations) { err = drm_plane_create_rotation_property(plane, DRM_MODE_ROTATE_0, supported_rotations); -- 2.18.0
[PATCH v9 03/21] drm/mediatek: Add missing plane settings when async update
From: Hsiao Chien Sung Fix an issue that plane coordinate was not saved when calling async update. Fixes: 920fffcc8912 ("drm/mediatek: update cursors by using async atomic update") Reviewed-by: CK Hu Reviewed-by: AngeloGioacchino Del Regno Signed-off-by: Hsiao Chien Sung --- drivers/gpu/drm/mediatek/mtk_plane.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/mediatek/mtk_plane.c b/drivers/gpu/drm/mediatek/mtk_plane.c index 95e1b17091f0..d20770162736 100644 --- a/drivers/gpu/drm/mediatek/mtk_plane.c +++ b/drivers/gpu/drm/mediatek/mtk_plane.c @@ -228,6 +228,8 @@ static void mtk_plane_atomic_async_update(struct drm_plane *plane, plane->state->src_y = new_state->src_y; plane->state->src_h = new_state->src_h; plane->state->src_w = new_state->src_w; + plane->state->dst.x1 = new_state->dst.x1; + plane->state->dst.y1 = new_state->dst.y1; mtk_plane_update_new_state(new_state, new_plane_state); swap(plane->state->fb, new_state->fb); -- 2.18.0
[PATCH v9 01/21] soc: mediatek: Disable 9-bit alpha in ETHDR
From: Hsiao Chien Sung When 9-bit alpha is enabled, its value will be converted from 0-255 to 0-256 (255 = not defined). This is designed for special HDR related calculation, which should be disabled by default, otherwise, alpha blending will not work correctly. Reviewed-by: AngeloGioacchino Del Regno Signed-off-by: Hsiao Chien Sung --- drivers/gpu/drm/mediatek/mtk_ethdr.c | 3 +-- drivers/soc/mediatek/mtk-mmsys.c | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_ethdr.c b/drivers/gpu/drm/mediatek/mtk_ethdr.c index ac4132210585..29673611fa75 100644 --- a/drivers/gpu/drm/mediatek/mtk_ethdr.c +++ b/drivers/gpu/drm/mediatek/mtk_ethdr.c @@ -50,7 +50,6 @@ #define MIXER_INX_MODE_BYPASS 0 #define MIXER_INX_MODE_EVEN_EXTEND 1 -#define DEFAULT_9BIT_ALPHA 0x100 #defineMIXER_ALPHA_AEN BIT(8) #defineMIXER_ALPHA 0xff #define ETHDR_CLK_NUM 13 @@ -169,7 +168,7 @@ void mtk_ethdr_layer_config(struct device *dev, unsigned int idx, alpha_con = MIXER_ALPHA_AEN | MIXER_ALPHA; mtk_mmsys_mixer_in_config(priv->mmsys_dev, idx + 1, alpha_con ? false : true, - DEFAULT_9BIT_ALPHA, + MIXER_ALPHA, pending->x & 1 ? MIXER_INX_MODE_EVEN_EXTEND : MIXER_INX_MODE_BYPASS, align_width / 2 - 1, cmdq_pkt); diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c index afb2c40c85c1..00eff18a3bce 100644 --- a/drivers/soc/mediatek/mtk-mmsys.c +++ b/drivers/soc/mediatek/mtk-mmsys.c @@ -236,6 +236,7 @@ void mtk_mmsys_mixer_in_config(struct device *dev, int idx, bool alpha_sel, u16 mtk_mmsys_update_bits(mmsys, MT8195_VDO1_MIXER_IN1_ALPHA + (idx - 1) * 4, ~0, alpha << 16 | alpha, cmdq_pkt); + mtk_mmsys_update_bits(mmsys, MT8195_VDO1_HDR_TOP_CFG, BIT(15 + idx), 0, cmdq_pkt); mtk_mmsys_update_bits(mmsys, MT8195_VDO1_HDR_TOP_CFG, BIT(19 + idx), alpha_sel << (19 + idx), cmdq_pkt); mtk_mmsys_update_bits(mmsys, MT8195_VDO1_MIXER_IN1_PAD + (idx - 1) * 4, -- 2.18.0
[PATCH v9 02/21] drm/mediatek: Add OVL compatible name for MT8195
From: Hsiao Chien Sung Add OVL compatible name for MT8195. Without this commit, DRM won't work after modifying the device tree. Reviewed-by: CK Hu Reviewed-by: AngeloGioacchino Del Regno Signed-off-by: Hsiao Chien Sung --- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c index 610fe1c0a23d..34d1a3aacde5 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c @@ -763,6 +763,8 @@ static const struct of_device_id mtk_ddp_comp_dt_ids[] = { .data = (void *)MTK_DISP_OVL }, { .compatible = "mediatek,mt8192-disp-ovl", .data = (void *)MTK_DISP_OVL }, + { .compatible = "mediatek,mt8195-disp-ovl", + .data = (void *)MTK_DISP_OVL }, { .compatible = "mediatek,mt8183-disp-ovl-2l", .data = (void *)MTK_DISP_OVL_2L }, { .compatible = "mediatek,mt8192-disp-ovl-2l", -- 2.18.0
[PATCH V2] i2c: imx-lpi2c: add eDMA mode support for LPI2C
From: Carlos Song Add eDMA mode support for LPI2C. There are some differences between TX DMA mode and RX DMA mode. LPI2C MTDR register is Controller Transmit Data Register. When lpi2c send data, it is tx cmd register and tx data fifo. When lpi2c receive data, it is just a rx cmd register. LPI2C MRDR register is Controller Receive Data Register, received data are stored in this. MTDR[8:10] is CMD field and MTDR[0:7] is DATA filed. +---+---+ | C M D | D A T A | +---+---+---+---+---+---+---+---+---+---+---+ | 10| 9 | 8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 | +---+---+---+---+---+---+---+---+---+---+---+ MRDR is Controller Receive Data Register. MRDR[0:7] is DATA filed. +---+ | D A T A | +---+---+---+---+---+---+---+---+ | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 | +---+---+---+---+---+---+---+---+ When the LPI2C controller needs to send data, tx cmd and 8-bit data should be written into MTDR: CMD: 000b: Transmit the value in DATA[7:0]. DATA: 8-bit data. If lpi2c controller needs to send N 8-bit data, just write N times (CMD(W) + DATA(u8)) to MTDR. When the LPI2C controller needs to receive data, rx cmd should be written into MTDR, the received data will be stored in the MRDR. MTDR(CMD): 001b: Receive (DATA[7:0] + 1) 8-bit data. MTDR(DATA): byte counter. MRDR(DATA): 8-bit data. So when lpi2c controller needs to receive N 8-bit data, 1. N <= 256: Write 1 time (CMD(R) + BYTE COUNT(N-1)) into MTDR and receive data from MRDR. 2. N > 256: Write N/256 times (CMD(R) + BYTE COUNT(255)) + 1 time (CMD(R) + BYTE COUNT(N%256)) into MTDR and receive data from MRDR. Due to these differences, when lpi2c is in DMA TX mode, only enable TX channel to send data. But when lpi2c is in DMA RX mode, TX and RX channel are both enabled, TX channel is used to send RX cmd and RX channel is used to receive data. Signed-off-by: Carlos Song Reviewed-by: Frank Li --- Change for V2: Optimized eDMA rx cmd buf free function to improve code readability --- drivers/i2c/busses/i2c-imx-lpi2c.c | 531 - 1 file changed, 523 insertions(+), 8 deletions(-) diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c index 6d72e4e126dd..d73a7d299c03 100644 --- a/drivers/i2c/busses/i2c-imx-lpi2c.c +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c @@ -8,6 +8,8 @@ #include #include #include +#include +#include #include #include #include @@ -29,6 +31,7 @@ #define LPI2C_MCR 0x10/* i2c contrl register */ #define LPI2C_MSR 0x14/* i2c status register */ #define LPI2C_MIER 0x18/* i2c interrupt enable */ +#define LPI2C_MDER 0x1C/* i2c DMA enable */ #define LPI2C_MCFGR0 0x20/* i2c master configuration */ #define LPI2C_MCFGR1 0x24/* i2c master configuration */ #define LPI2C_MCFGR2 0x28/* i2c master configuration */ @@ -70,11 +73,14 @@ #define MCFGR1_AUTOSTOPBIT(8) #define MCFGR1_IGNACK BIT(9) #define MRDR_RXEMPTY BIT(14) +#define MDER_TDDE BIT(0) +#define MDER_RDDE BIT(1) #define I2C_CLK_RATIO 2 #define CHUNK_DATA 256 #define I2C_PM_TIMEOUT 10 /* ms */ +#define I2C_DMA_THRESHOLD 8 /* bytes */ enum lpi2c_imx_mode { STANDARD, /* 100+Kbps */ @@ -107,6 +113,22 @@ struct lpi2c_imx_struct { unsigned intrxfifosize; enum lpi2c_imx_mode mode; struct i2c_bus_recovery_info rinfo; + + boolcan_use_dma; + boolusing_pio_mode; + u8 rx_cmd_buf_len; + u16 *rx_cmd_buf; + u8 *dma_buf; + unsigned intdma_len; + unsigned inttx_burst_num; + unsigned intrx_burst_num; + resource_size_t phy_addr; + dma_addr_t dma_tx_addr; + dma_addr_t dma_addr; + enum dma_data_direction dma_direction; + struct dma_chan *chan_tx; + struct dma_chan *chan_rx; + struct i2c_msg *msg; }; static void lpi2c_imx_intctrl(struct lpi2c_imx_struct *lpi2c_imx, @@ -306,7 +328,7 @@ static int lpi2c_imx_master_disable(struct lpi2c_imx_struct *lpi2c_imx) return 0; } -static int lpi2c_imx_msg_complete(struct lpi2c_imx_struct *lpi2c_imx) +static int lpi2c_imx_pio_msg_complete(struct lpi2c_imx_struct *lpi2c_imx) { unsigned long timeout; @@ -452,6 +474,439 @@ static void lpi2c_imx_read(struct lpi2c_imx_struct *lpi2c_imx, lpi2c_imx_intctrl(lpi2c_imx, MIER_RDIE | MIER_NDIE); } +static bool is_use_dma(struct lpi2c_imx_struct *lpi2c_imx, struct i2c_msg *msg) +{ + if (!lpi2c_imx->can_use_dma) + return false; + + /* +* When the length of data is less than I2C_DMA_THRESHOLD, +* cpu mode is used directly to avoid low performance. +*/ + if (msg->len < I2C_DMA_T
Re: [PATCH v5] drm/ci: add tests on vkms
Hi Maxime, On 13/06/24 22:58, Maxime Ripard wrote: Hi, On Thu, Jun 13, 2024 at 01:56:10PM GMT, Vignesh Raman wrote: On 13/06/24 13:07, Maxime Ripard wrote: Hi, On Tue, Jun 11, 2024 at 02:40:37PM GMT, Vignesh Raman wrote: diff --git a/drivers/gpu/drm/ci/xfails/vkms-none-flakes.txt b/drivers/gpu/drm/ci/xfails/vkms-none-flakes.txt new file mode 100644 index ..56484a30aff5 --- /dev/null +++ b/drivers/gpu/drm/ci/xfails/vkms-none-flakes.txt @@ -0,0 +1,15 @@ +# Board Name: vkms +# Bug Report: https://lore.kernel.org/dri-devel/61ed26af-062c-443c-9df2-d1ee319f3...@collabora.com/T/#u +# Failure Rate: 50 +# IGT Version: 1.28-g0df7b9b97 +# Linux Version: 6.9.0-rc7 +kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic +kms_flip@basic-flip-vs-wf_vblank +kms_flip@flip-vs-expired-vblank-interruptible +kms_flip@flip-vs-wf_vblank-interruptible +kms_flip@plain-flip-fb-recreate-interruptible +kms_flip@plain-flip-ts-check +kms_flip@plain-flip-ts-check-interruptible +kms_flip@flip-vs-absolute-wf_vblank +kms_flip@flip-vs-absolute-wf_vblank-interruptible +kms_flip@flip-vs-blocking-wf-vblank We should have the header for every line here All the flakes in these tests were observed with version 6.9.0-rc7/1.28-g0df7b9b97. So can we group them together? If we update this file for different IGT/kernel version, we can add a separate header for each test. If we don't however, we have no way to tell in a couple months whether those flakes were there before we were adding those metadata, or if the metadata applies to everything, or only a subset. I will send a v6 with this change. Thanks. Regards, Vignesh
Re: [PATCH net-next v12 00/13] Device Memory TCP
On Thu, 13 Jun 2024 01:35:37 + Mina Almasry wrote: > v12: > https://patchwork.kernel.org/project/netdevbpf/list/?series=859747&state=* patches 5 and 6 transiently break the build ../include/trace/events/page_pool.h:65:23: error: use of undeclared identifier 'NET_IOV' 65 | __entry->netmem & NET_IOV, __entry->pfn, __entry->release) | ^ ../include/trace/events/page_pool.h:91:23: error: use of undeclared identifier 'NET_IOV' 91 | __entry->netmem & NET_IOV, __entry->pfn, __entry->hold) | ^ Looking at NIPA status the builders are 12h behind, so please don't repost immediately. This series takes a lot of compute cycles to build. FWIW there is a docker version of NIPA checks in the nipa repo. https://github.com/linux-netdev/nipa/tree/main/docker IDK if it still works, but could help avoid mistakes..
[PATCH v4 3/3] drm/panel-edp: drop several legacy panels
The panel-edp driver supports legacy compatible strings for several eDP panels which were never used in DT files present in Linux tree and most likely have never been used with the upstream kernel. Drop compatibles for these panels in favour of using a generic "edp-panel" device on the AUX bus. Reviewed-by: Douglas Anderson Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/panel/panel-edp.c | 173 ++ 1 file changed, 7 insertions(+), 166 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c index 85edfd3d59f3..3a574a9b46e7 100644 --- a/drivers/gpu/drm/panel/panel-edp.c +++ b/drivers/gpu/drm/panel/panel-edp.c @@ -1045,33 +1045,6 @@ static const struct panel_desc auo_b116xak01 = { }, }; -static const struct drm_display_mode auo_b133han05_mode = { - .clock = 142600, - .hdisplay = 1920, - .hsync_start = 1920 + 58, - .hsync_end = 1920 + 58 + 42, - .htotal = 1920 + 58 + 42 + 60, - .vdisplay = 1080, - .vsync_start = 1080 + 3, - .vsync_end = 1080 + 3 + 5, - .vtotal = 1080 + 3 + 5 + 54, -}; - -static const struct panel_desc auo_b133han05 = { - .modes = &auo_b133han05_mode, - .num_modes = 1, - .bpc = 8, - .size = { - .width = 293, - .height = 165, - }, - .delay = { - .hpd_reliable = 100, - .enable = 20, - .unprepare = 50, - }, -}; - static const struct drm_display_mode auo_b133htn01_mode = { .clock = 150660, .hdisplay = 1920, @@ -1121,33 +1094,6 @@ static const struct panel_desc auo_b133xtn01 = { }, }; -static const struct drm_display_mode auo_b140han06_mode = { - .clock = 141000, - .hdisplay = 1920, - .hsync_start = 1920 + 16, - .hsync_end = 1920 + 16 + 16, - .htotal = 1920 + 16 + 16 + 152, - .vdisplay = 1080, - .vsync_start = 1080 + 3, - .vsync_end = 1080 + 3 + 14, - .vtotal = 1080 + 3 + 14 + 19, -}; - -static const struct panel_desc auo_b140han06 = { - .modes = &auo_b140han06_mode, - .num_modes = 1, - .bpc = 8, - .size = { - .width = 309, - .height = 174, - }, - .delay = { - .hpd_reliable = 100, - .enable = 20, - .unprepare = 50, - }, -}; - static const struct drm_display_mode boe_nv101wxmn51_modes[] = { { .clock = 71900, @@ -1414,33 +1360,6 @@ static const struct panel_desc innolux_p120zdg_bf1 = { }, }; -static const struct drm_display_mode ivo_m133nwf4_r0_mode = { - .clock = 138778, - .hdisplay = 1920, - .hsync_start = 1920 + 24, - .hsync_end = 1920 + 24 + 48, - .htotal = 1920 + 24 + 48 + 88, - .vdisplay = 1080, - .vsync_start = 1080 + 3, - .vsync_end = 1080 + 3 + 12, - .vtotal = 1080 + 3 + 12 + 17, - .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC, -}; - -static const struct panel_desc ivo_m133nwf4_r0 = { - .modes = &ivo_m133nwf4_r0_mode, - .num_modes = 1, - .bpc = 8, - .size = { - .width = 294, - .height = 165, - }, - .delay = { - .hpd_absent = 200, - .unprepare = 500, - }, -}; - static const struct drm_display_mode kingdisplay_kd116n21_30nv_a010_mode = { .clock = 81000, .hdisplay = 1366, @@ -1689,75 +1608,6 @@ static const struct panel_desc sharp_lq123p1jx31 = { }, }; -static const struct drm_display_mode sharp_lq140m1jw46_mode[] = { - { - .clock = 346500, - .hdisplay = 1920, - .hsync_start = 1920 + 48, - .hsync_end = 1920 + 48 + 32, - .htotal = 1920 + 48 + 32 + 80, - .vdisplay = 1080, - .vsync_start = 1080 + 3, - .vsync_end = 1080 + 3 + 5, - .vtotal = 1080 + 3 + 5 + 69, - .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC, - }, { - .clock = 144370, - .hdisplay = 1920, - .hsync_start = 1920 + 48, - .hsync_end = 1920 + 48 + 32, - .htotal = 1920 + 48 + 32 + 80, - .vdisplay = 1080, - .vsync_start = 1080 + 3, - .vsync_end = 1080 + 3 + 5, - .vtotal = 1080 + 3 + 5 + 69, - .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC, - }, -}; - -static const struct panel_desc sharp_lq140m1jw46 = { - .modes = sharp_lq140m1jw46_mode, - .num_modes = ARRAY_SIZE(sharp_lq140m1jw46_mode), - .bpc = 8, - .size = { - .width = 309, - .height = 174, - }, - .delay = { - .hpd_absent = 80, - .enable = 50, - .unprepare = 500, - }, -}; - -static const struct drm_dis
[PATCH v4 1/3] drm/panel-edp: add fat warning against adding new panel compatibles
Add a fat warning against adding new panel compatibles to the panel-edp driver. All new users of the eDP panels are supposed to use the generic "edp-panel" compatible device on the AUX bus. The remaining compatibles are either used by the existing DT or were used previously and are retained for backwards compatibility. Suggested-by: Doug Anderson Reviewed-by: Neil Armstrong Reviewed-by: Douglas Anderson Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/panel/panel-edp.c | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c index 67ab6915d6e4..85edfd3d59f3 100644 --- a/drivers/gpu/drm/panel/panel-edp.c +++ b/drivers/gpu/drm/panel/panel-edp.c @@ -1762,7 +1762,24 @@ static const struct of_device_id platform_of_match[] = { { /* Must be first */ .compatible = "edp-panel", - }, { + }, + /* +* Do not add panels to the list below unless they cannot be handled by +* the generic edp-panel compatible. +* +* The only two valid reasons are: +* - Because of the panel issues (e.g. broken EDID or broken +* identification). +* - Because the eDP drivers didn't wire up the AUX bus properly. +* NOTE that, though this is a marginally valid reason, +* some justification needs to be made for why the platform can't +* wire up the AUX bus properly. +* +* In all other cases the platform should use the aux-bus and declare +* the panel using the 'edp-panel' compatible as a device on the AUX +* bus. +*/ + { .compatible = "auo,b101ean01", .data = &auo_b101ean01, }, { -- 2.39.2
[PATCH v4 2/3] dt-bindings: display: panel-edp-legacy: drop several eDP panels
The panel-edp-legacy.yaml includes legacy bindings for several eDP panels which were never used in DT files present in Linux tree and most likely have never been used with the upstream kernel. Drop compatibles for these panels in favour of using a generic "edp-panel" device on the AUX bus. Reviewed-by: Douglas Anderson Signed-off-by: Dmitry Baryshkov --- .../devicetree/bindings/display/panel/panel-edp-legacy.yaml| 10 -- 1 file changed, 10 deletions(-) diff --git a/Documentation/devicetree/bindings/display/panel/panel-edp-legacy.yaml b/Documentation/devicetree/bindings/display/panel/panel-edp-legacy.yaml index 9e5864de49e7..b308047c1edf 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-edp-legacy.yaml +++ b/Documentation/devicetree/bindings/display/panel/panel-edp-legacy.yaml @@ -31,13 +31,9 @@ properties: # AUO B116XAK01 eDP TFT LCD panel - auo,b116xa01 # AU Optronics Corporation 13.3" FHD (1920x1080) color TFT-LCD panel - - auo,b133han05 -# AU Optronics Corporation 13.3" FHD (1920x1080) color TFT-LCD panel - auo,b133htn01 # AU Optronics Corporation 13.3" WXGA (1366x768) TFT LCD panel - auo,b133xtn01 -# AU Optronics Corporation 14.0" FHD (1920x1080) color TFT-LCD panel - - auo,b140han06 # BOE OPTOELECTRONICS TECHNOLOGY 10.1" WXGA TFT LCD panel - boe,nv101wxmn51 # BOE NV133FHM-N61 13.3" FHD (1920x1080) TFT LCD Panel @@ -56,8 +52,6 @@ properties: - innolux,n125hce-gn1 # Innolux P120ZDG-BF1 12.02 inch eDP 2K display panel - innolux,p120zdg-bf1 -# InfoVision Optoelectronics M133NWF4 R0 13.3" FHD (1920x1080) TFT LCD panel - - ivo,m133nwf4-r0 # King & Display KD116N21-30NV-A010 eDP TFT LCD panel - kingdisplay,kd116n21-30nv-a010 # LG LP079QX1-SP0V 7.9" (1536x2048 pixels) TFT LCD panel @@ -78,10 +72,6 @@ properties: - sharp,ld-d5116z01b # Sharp 12.3" (2400x1600 pixels) TFT LCD panel - sharp,lq123p1jx31 -# Sharp 14" (1920x1080 pixels) TFT LCD panel - - sharp,lq140m1jw46 -# Starry 12.2" (1920x1200 pixels) TFT LCD panel - - starry,kr122ea0sra backlight: true ddc-i2c-bus: true -- 2.39.2
[PATCH v4 0/3] drm/panel-edp: remove several legacy compatibles used by the driver
There are two ways to describe an eDP panel in device tree. The recommended way is to add a device on the AUX bus, ideally using the edp-panel compatible. The legacy way is to define a top-level platform device for the panel. Document that adding support for eDP panels in a legacy way is strongly discouraged (if not forbidden at all). While we are at it, also drop legacy compatible strings and bindings for five panels. These compatible strings were never used by a DT file present in Linux kernel and most likely were never used with the upstream Linux kernel. The following compatibles were never used by the devices supported by the upstream kernel and are a subject to possible removal: - lg,lp097qx1-spa1 - samsung,lsn122dl01-c01 - sharp,ld-d5116z01b I'm considering dropping them later, unless there is a good reason not to do so. To: Douglas Anderson To: Neil Armstrong To: Jessica Zhang To: Sam Ravnborg To: Maarten Lankhorst To: Maxime Ripard To: Thomas Zimmermann To: David Airlie To: Daniel Vetter To: Rob Herring To: Krzysztof Kozlowski To: Conor Dooley Cc: dri-devel@lists.freedesktop.org Cc: linux-ker...@vger.kernel.org Cc: linux-arm-...@vger.kernel.org Cc: linux-rockc...@lists.infradead.org Cc: Jeffrey Hugo Cc: devicet...@vger.kernel.org Signed-off-by: Dmitry Baryshkov Changes in v4: - Rebased on top of drm-misc-next - Added devicetree maintainers to cc list, missed them for v2/v3 - Link to v3: https://lore.kernel.org/r/20240531-edp-panel-drop-v3-0-4c98b2b95...@linaro.org Changes in v3: - Rephrased the warning comment, following some of Doug's suggestions. - Link to v2: https://lore.kernel.org/r/20240529-edp-panel-drop-v2-0-fcfc457fc...@linaro.org Changes in v2: - Actually drop support for five panels acked by Doug Andersson - Link to v1: https://lore.kernel.org/r/20240523-edp-panel-drop-v1-1-045d62511...@linaro.org --- Dmitry Baryshkov (3): drm/panel-edp: add fat warning against adding new panel compatibles dt-bindings: display: panel-edp-legacy: drop several eDP panels drm/panel-edp: drop several legacy panels .../bindings/display/panel/panel-edp-legacy.yaml | 10 -- drivers/gpu/drm/panel/panel-edp.c | 192 +++-- 2 files changed, 25 insertions(+), 177 deletions(-) --- base-commit: 9324410846e13595d453b7f34508b1f6b15fb1a7 change-id: 20240523-edp-panel-drop-00aa239b3c6b Best regards, -- Dmitry Baryshkov
Re: [PATCH v4 4/4] arm64: dts: qcom: add HDMI nodes for msm8998
On 6/13/24 17:15, Marc Gonzalez wrote: From: Arnaud Vrac Port device nodes from vendor code. Signed-off-by: Arnaud Vrac Reviewed-by: Dmitry Baryshkov Signed-off-by: Marc Gonzalez --- [...] + + hdmi: hdmi-tx@c9a { + compatible = "qcom,hdmi-tx-8998"; + reg = <0x0c9a 0x50c>, + <0x0078 0x6220>, + <0x0c9e 0x2c>; + reg-names = "core_physical", + "qfprom_physical", + "hdcp_physical"; The way qfprom is accessed (bypassing nvmem APIs) will need to be reworked.. but since we already have it like that on 8996, I'm fine with batch-reworking these some time in the future.. + + interrupt-parent = <&mdss>; + interrupts = <8>; + + clocks = <&mmcc MDSS_MDP_CLK>, Not sure if the MDP core clock is necessary here. Pretty sure it only powers the display-controller@.. peripheral +<&mmcc MDSS_AHB_CLK>, +<&mmcc MDSS_HDMI_CLK>, +<&mmcc MDSS_HDMI_DP_AHB_CLK>, +<&mmcc MDSS_EXTPCLK_CLK>, +<&mmcc MDSS_AXI_CLK>, +<&mmcc MNOC_AHB_CLK>, This one is an interconnect clock, drop it +<&mmcc MISC_AHB_CLK>; And please confirm whether this one is necessary + clock-names = + "mdp_core", + "iface", + "core", + "alt_iface", + "extp", + "bus", + "mnoc", + "iface_mmss"; + + phys = <&hdmi_phy>; + #sound-dai-cells = <1>; + + pinctrl-names = "default", "sleep"; + pinctrl-0 = <&hdmi_hpd_default +&hdmi_ddc_default +&hdmi_cec_default>; + pinctrl-1 = <&hdmi_hpd_sleep +&hdmi_ddc_default +&hdmi_cec_default>; property property-names please and use <&foo>, <&bar>; for phandle arrays instead of <&foo bar> (this is a really old dt and we still haven't got around to cleaning up old junk for style issues..) + + status = "disabled"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + hdmi_in: endpoint { + remote-endpoint = <&dpu_intf3_out>; + }; + }; + + port@1 { + reg = <1>; + hdmi_out: endpoint { + }; + }; + }; + }; + + hdmi_phy: hdmi-phy@c9a0600 { + compatible = "qcom,hdmi-phy-8998"; + reg = <0x0c9a0600 0x18b>, + <0x0c9a0a00 0x38>, + <0x0c9a0c00 0x38>, + <0x0c9a0e00 0x38>, + <0x0c9a1000 0x38>, + <0x0c9a1200 0x0e8>; + reg-names = "hdmi_pll", + "hdmi_tx_l0", + "hdmi_tx_l1", + "hdmi_tx_l2", + "hdmi_tx_l3", + "hdmi_phy"; + + #clock-cells = <0>; + #phy-cells = <0>; + + clocks = <&mmcc MDSS_AHB_CLK>, +<&gcc GCC_HDMI_CLKREF_CLK>, +<&rpmcc RPM_SMD_XO_CLK_SRC>; +
Re: [PATCH v3 7/9] drm/msm/dpu: drop _dpu_crtc_check_and_setup_lm_bounds from atomic_begin
On 6/13/2024 3:36 PM, Dmitry Baryshkov wrote: The dpu_crtc_atomic_check() already calls the function _dpu_crtc_check_and_setup_lm_bounds(). There is no need to call it again from dpu_crtc_atomic_begin(). Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 -- 1 file changed, 2 deletions(-) Reviewed-by: Abhinav Kumar
Re: [PATCH v3 5/9] drm/msm/dpu: move dpu_format_populate_plane_sizes to atomic_check
On 6/13/2024 3:36 PM, Dmitry Baryshkov wrote: Move a call to dpu_format_populate_plane_sizes() to the atomic_check step, so that any issues with the FB layout can be reported as early as possible. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) Did anything change between v2 to v3 that R-b was dropped? Here it is again, Reviewed-by: Abhinav Kumar
Re: [PATCH v3 9/9] drm/msm/dpu: sync mode_config limits to the FB limits in dpu_plane.c
On 6/13/2024 3:36 PM, Dmitry Baryshkov wrote: Lift mode_config limits set by the DPU driver to the actual FB limits as handled by the dpu_plane.c. Move 2*max_lm_width check where it belongs, to the drm_crtc_helper_funcs::mode_valid() callback. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 15 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 9 ++--- 2 files changed, 17 insertions(+), 7 deletions(-) Did anything change in this patch from v2 that the R-b was dropped? Here it is again, Reviewed-by: Abhinav Kumar
Re: [PATCH v3 2/9] drm/msm/dpu: drop dpu_format_check_modified_format
On 6/13/2024 3:36 PM, Dmitry Baryshkov wrote: The msm_kms_funcs::check_modified_format() callback is not used by the driver. Drop it completely. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 43 - drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h | 16 --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 - drivers/gpu/drm/msm/msm_kms.h | 6 4 files changed, 66 deletions(-) Reviewed-by: Abhinav Kumar
Re: [PATCH v3 1/9] drm/msm/dpu: check for overflow in _dpu_crtc_setup_lm_bounds()
On 6/13/2024 3:36 PM, Dmitry Baryshkov wrote: Make _dpu_crtc_setup_lm_bounds() check that CRTC width is not overflowing LM requirements. Rename the function accordingly. Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support") Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 17 + 1 file changed, 13 insertions(+), 4 deletions(-) Reviewed-by: Abhinav Kumar
Re: Patch "Revert "drm/amdgpu: init iommu after amdkfd device init"" has been added to the 5.15-stable tree
Am 12.06.24 um 14:45 schrieb gre...@linuxfoundation.org: This is a note to let you know that I've just added the patch titled Revert "drm/amdgpu: init iommu after amdkfd device init" to the 5.15-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary The filename of the patch is: revert-drm-amdgpu-init-iommu-after-amdkfd-device-init.patch and it can be found in the queue-5.15 subdirectory. Thank you :) If you, or anyone else, feels it should not be added to the stable tree, please let know about it. From w_ar...@gmx.de Wed Jun 12 14:43:21 2024 From: Armin Wolf Date: Thu, 23 May 2024 19:30:31 +0200 Subject: Revert "drm/amdgpu: init iommu after amdkfd device init" To: alexander.deuc...@amd.com, christian.koe...@amd.com, xinhui@amd.com, gre...@linuxfoundation.org, sas...@kernel.org Cc: sta...@vger.kernel.org, bkau...@gmail.com, yifan1.zh...@amd.com, prike.li...@amd.com, dri-devel@lists.freedesktop.org, amd-...@lists.freedesktop.org Message-ID: <20240523173031.4212-1-w_ar...@gmx.de> From: Armin Wolf This reverts commit 56b522f4668167096a50c39446d6263c96219f5f. A user reported that this commit breaks the integrated gpu of his notebook, causing a black screen. He was able to bisect the problematic commit and verified that by reverting it the notebook works again. He also confirmed that kernel 6.8.1 also works on his device, so the upstream commit itself seems to be ok. An amdgpu developer (Alex Deucher) confirmed that this patch should have never been ported to 5.15 in the first place, so revert this commit from the 5.15 stable series. Reported-by: Barry Kauler Signed-off-by: Armin Wolf Link: https://lore.kernel.org/r/20240523173031.4212-1-w_ar...@gmx.de Signed-off-by: Greg Kroah-Hartman --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |8 1 file changed, 4 insertions(+), 4 deletions(-) --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -2487,6 +2487,10 @@ static int amdgpu_device_ip_init(struct if (r) goto init_failed; + r = amdgpu_amdkfd_resume_iommu(adev); + if (r) + goto init_failed; + r = amdgpu_device_ip_hw_init_phase1(adev); if (r) goto init_failed; @@ -2525,10 +2529,6 @@ static int amdgpu_device_ip_init(struct if (!adev->gmc.xgmi.pending_reset) amdgpu_amdkfd_device_init(adev); - r = amdgpu_amdkfd_resume_iommu(adev); - if (r) - goto init_failed; - amdgpu_fru_get_product_info(adev); init_failed: Patches currently in stable-queue which might be from w_ar...@gmx.de are queue-5.15/revert-drm-amdgpu-init-iommu-after-amdkfd-device-init.patch
[PATCH v3 6/9] drm/msm/dpu: check for the plane pitch overflow
Check that the plane pitch doesn't overflow the maximum pitch size allowed by the hardware. Reviewed-by: Abhinav Kumar Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 2 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 6 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h index 4a910b808687..8998d1862e16 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h @@ -12,6 +12,8 @@ struct dpu_hw_sspp; +#define DPU_SSPP_MAX_PITCH_SIZE0x + /** * Flags */ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 927fde2f1391..b5848fae14ce 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -791,7 +791,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, { struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane); - int ret = 0, min_scale; + int i, ret = 0, min_scale; struct dpu_plane *pdpu = to_dpu_plane(plane); struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base); u64 max_mdp_clk_rate = kms->perf.max_core_clk_rate; @@ -865,6 +865,10 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, return ret; } + for (i = 0; i < pstate->layout.num_planes; i++) + if (pstate->layout.plane_pitch[i] > DPU_SSPP_MAX_PITCH_SIZE) + return -E2BIG; + fmt = msm_framebuffer_format(new_plane_state->fb); max_linewidth = pdpu->catalog->caps->max_linewidth; -- 2.39.2
[PATCH v3 7/9] drm/msm/dpu: drop _dpu_crtc_check_and_setup_lm_bounds from atomic_begin
The dpu_crtc_atomic_check() already calls the function _dpu_crtc_check_and_setup_lm_bounds(). There is no need to call it again from dpu_crtc_atomic_begin(). Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index b0d81e8ea765..5dbf5254d310 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -809,8 +809,6 @@ static void dpu_crtc_atomic_begin(struct drm_crtc *crtc, DRM_DEBUG_ATOMIC("crtc%d\n", crtc->base.id); - _dpu_crtc_check_and_setup_lm_bounds(crtc, crtc->state); - /* encoder will trigger pending mask now */ drm_for_each_encoder_mask(encoder, crtc->dev, crtc->state->encoder_mask) dpu_encoder_trigger_kickoff_pending(encoder); -- 2.39.2
[PATCH v3 9/9] drm/msm/dpu: sync mode_config limits to the FB limits in dpu_plane.c
Lift mode_config limits set by the DPU driver to the actual FB limits as handled by the dpu_plane.c. Move 2*max_lm_width check where it belongs, to the drm_crtc_helper_funcs::mode_valid() callback. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 15 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 9 ++--- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 5dbf5254d310..44531666edf2 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -1236,6 +1236,20 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc, return 0; } +static enum drm_mode_status dpu_crtc_mode_valid(struct drm_crtc *crtc, + const struct drm_display_mode *mode) +{ + struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc); + + /* +* max crtc width is equal to the max mixer width * 2 and max height is +* is 4K +*/ + return drm_mode_validate_size(mode, + 2 * dpu_kms->catalog->caps->max_mixer_width, + 4096); +} + int dpu_crtc_vblank(struct drm_crtc *crtc, bool en) { struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); @@ -1451,6 +1465,7 @@ static const struct drm_crtc_helper_funcs dpu_crtc_helper_funcs = { .atomic_check = dpu_crtc_atomic_check, .atomic_begin = dpu_crtc_atomic_begin, .atomic_flush = dpu_crtc_atomic_flush, + .mode_valid = dpu_crtc_mode_valid, .get_scanout_position = dpu_crtc_get_scanout_position, }; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 0d1dcc94455c..d1b937e127b0 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1147,13 +1147,8 @@ static int dpu_kms_hw_init(struct msm_kms *kms) dev->mode_config.min_width = 0; dev->mode_config.min_height = 0; - /* -* max crtc width is equal to the max mixer width * 2 and max height is -* is 4K -*/ - dev->mode_config.max_width = - dpu_kms->catalog->caps->max_mixer_width * 2; - dev->mode_config.max_height = 4096; + dev->mode_config.max_width = DPU_MAX_IMG_WIDTH; + dev->mode_config.max_height = DPU_MAX_IMG_HEIGHT; dev->max_vblank_count = 0x; /* Disable vblank irqs aggressively for power-saving */ -- 2.39.2
[PATCH v3 3/9] drm/msm/dpu: drop dpu_format_populate_layout from dpu_plane_sspp_atomic_update
The dpu_plane_prepare_fb() already calls dpu_format_populate_layout(). Store the generated layout in the plane state and drop this call from dpu_plane_sspp_update(). Reviewed-by: Abhinav Kumar Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 19 --- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 3 +++ 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 1c3a2657450c..9ee178a09a3b 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -647,7 +647,6 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane, struct drm_framebuffer *fb = new_state->fb; struct dpu_plane *pdpu = to_dpu_plane(plane); struct dpu_plane_state *pstate = to_dpu_plane_state(new_state); - struct dpu_hw_fmt_layout layout; struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base); int ret; @@ -677,7 +676,8 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane, /* validate framebuffer layout before commit */ ret = dpu_format_populate_layout(pstate->aspace, - new_state->fb, &layout); +new_state->fb, +&pstate->layout); if (ret) { DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret); return ret; @@ -1100,17 +1100,6 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane) msm_framebuffer_format(fb); struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg; struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg; - struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base); - struct msm_gem_address_space *aspace = kms->base.aspace; - struct dpu_hw_fmt_layout layout; - bool layout_valid = false; - int ret; - - ret = dpu_format_populate_layout(aspace, fb, &layout); - if (ret) - DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret); - else - layout_valid = true; pstate->pending = true; @@ -1125,12 +1114,12 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane) dpu_plane_sspp_update_pipe(plane, pipe, pipe_cfg, fmt, drm_mode_vrefresh(&crtc->mode), - layout_valid ? &layout : NULL); + &pstate->layout); if (r_pipe->sspp) { dpu_plane_sspp_update_pipe(plane, r_pipe, r_pipe_cfg, fmt, drm_mode_vrefresh(&crtc->mode), - layout_valid ? &layout : NULL); + &pstate->layout); } if (pstate->needs_qos_remap) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h index abd6b21a049b..348b0075d1ce 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h @@ -31,6 +31,7 @@ * @plane_clk: calculated clk per plane * @needs_dirtyfb: whether attached CRTC needs pixel data explicitly flushed * @rotation: simplified drm rotation hint + * @layout: framebuffer memory layout */ struct dpu_plane_state { struct drm_plane_state base; @@ -48,6 +49,8 @@ struct dpu_plane_state { bool needs_dirtyfb; unsigned int rotation; + + struct dpu_hw_fmt_layout layout; }; #define to_dpu_plane_state(x) \ -- 2.39.2
[PATCH v3 5/9] drm/msm/dpu: move dpu_format_populate_plane_sizes to atomic_check
Move a call to dpu_format_populate_plane_sizes() to the atomic_check step, so that any issues with the FB layout can be reported as early as possible. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index a57853ac70b1..927fde2f1391 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -674,12 +674,6 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane, } } - ret = dpu_format_populate_plane_sizes(new_state->fb, &pstate->layout); - if (ret) { - DPU_ERROR_PLANE(pdpu, "failed to get format plane sizes, %d\n", ret); - return ret; - } - /* validate framebuffer layout before commit */ ret = dpu_format_populate_addrs(pstate->aspace, new_state->fb, @@ -865,6 +859,12 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, return -E2BIG; } + ret = dpu_format_populate_plane_sizes(new_plane_state->fb, &pstate->layout); + if (ret) { + DPU_ERROR_PLANE(pdpu, "failed to get format plane sizes, %d\n", ret); + return ret; + } + fmt = msm_framebuffer_format(new_plane_state->fb); max_linewidth = pdpu->catalog->caps->max_linewidth; -- 2.39.2
[PATCH v3 8/9] drm/msm/dpu: merge MAX_IMG_WIDTH/HEIGHT with DPU_MAX_IMG_WIDTH/HEIGHT
dpu_formats.c defines DPU_MAX_IMG_WIDTH and _HEIGHT, while dpu_hw_catalog.h defines just MAX_IMG_WIDTH and _HEIGHT. Merge these constants to remove duplication. Reviewed-by: Abhinav Kumar Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c| 3 --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 4 ++-- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c index c6485cb6f1d2..6d7c4373bf84 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c @@ -13,9 +13,6 @@ #define DPU_UBWC_PLANE_SIZE_ALIGNMENT 4096 -#define DPU_MAX_IMG_WIDTH 0x3FFF -#define DPU_MAX_IMG_HEIGHT 0x3FFF - /* * struct dpu_media_color_map - maps drm format to media format * @format: DRM base pixel format diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h index d1aef778340b..af2ead1c4886 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -21,8 +21,8 @@ #define DPU_HW_BLK_NAME_LEN16 -#define MAX_IMG_WIDTH 0x3fff -#define MAX_IMG_HEIGHT 0x3fff +#define DPU_MAX_IMG_WIDTH 0x3fff +#define DPU_MAX_IMG_HEIGHT 0x3fff #define CRTC_DUAL_MIXERS 2 diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index b5848fae14ce..6000e84598c2 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -852,8 +852,8 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, fb_rect.y2 = new_plane_state->fb->height; /* Ensure fb size is supported */ - if (drm_rect_width(&fb_rect) > MAX_IMG_WIDTH || - drm_rect_height(&fb_rect) > MAX_IMG_HEIGHT) { + if (drm_rect_width(&fb_rect) > DPU_MAX_IMG_WIDTH || + drm_rect_height(&fb_rect) > DPU_MAX_IMG_HEIGHT) { DPU_DEBUG_PLANE(pdpu, "invalid framebuffer " DRM_RECT_FMT "\n", DRM_RECT_ARG(&fb_rect)); return -E2BIG; -- 2.39.2
[PATCH v3 4/9] drm/msm/dpu: split dpu_format_populate_layout
Split dpu_format_populate_layout() into addess-related and pitch/format-related parts. Reviewed-by: Abhinav Kumar Signed-off-by: Dmitry Baryshkov --- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c| 8 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c| 45 -- drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h| 8 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 12 -- 4 files changed, 46 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c index d3ea91c1d7d2..ccf2d030cf20 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c @@ -584,7 +584,13 @@ static void dpu_encoder_phys_wb_prepare_wb_job(struct dpu_encoder_phys *phys_enc return; } - ret = dpu_format_populate_layout(aspace, job->fb, &wb_cfg->dest); + ret = dpu_format_populate_plane_sizes(job->fb, &wb_cfg->dest); + if (ret) { + DPU_DEBUG("failed to populate plane sizes%d\n", ret); + return; + } + + ret = dpu_format_populate_addrs(aspace, job->fb, &wb_cfg->dest); if (ret) { DPU_DEBUG("failed to populate layout %d\n", ret); return; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c index 027eb5ecff08..c6485cb6f1d2 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c @@ -93,7 +93,7 @@ static int _dpu_format_get_media_color_ubwc(const struct msm_format *fmt) return color_fmt; } -static int _dpu_format_get_plane_sizes_ubwc( +static int _dpu_format_populate_plane_sizes_ubwc( const struct msm_format *fmt, const uint32_t width, const uint32_t height, @@ -172,7 +172,7 @@ static int _dpu_format_get_plane_sizes_ubwc( return 0; } -static int _dpu_format_get_plane_sizes_linear( +static int _dpu_format_populate_plane_sizes_linear( const struct msm_format *fmt, const uint32_t width, const uint32_t height, @@ -244,27 +244,38 @@ static int _dpu_format_get_plane_sizes_linear( return 0; } -static int dpu_format_get_plane_sizes( - const struct msm_format *fmt, - const uint32_t w, - const uint32_t h, - struct dpu_hw_fmt_layout *layout, - const uint32_t *pitches) +/* + * dpu_format_populate_addrs - populate non-address part of the layout based on + * fb, and format found in the fb + * @fb:framebuffer pointer + * @layout: format layout structure to populate + * + * Return: error code on failure or 0 if new addresses were populated + */ +int dpu_format_populate_plane_sizes( + struct drm_framebuffer *fb, + struct dpu_hw_fmt_layout *layout) { - if (!layout || !fmt) { + const struct msm_format *fmt; + + if (!layout || !fb) { DRM_ERROR("invalid pointer\n"); return -EINVAL; } - if ((w > DPU_MAX_IMG_WIDTH) || (h > DPU_MAX_IMG_HEIGHT)) { + if (fb->width > DPU_MAX_IMG_WIDTH || + fb->height > DPU_MAX_IMG_HEIGHT) { DRM_ERROR("image dimensions outside max range\n"); return -ERANGE; } + fmt = msm_framebuffer_format(fb); + if (MSM_FORMAT_IS_UBWC(fmt) || MSM_FORMAT_IS_TILE(fmt)) - return _dpu_format_get_plane_sizes_ubwc(fmt, w, h, layout); + return _dpu_format_populate_plane_sizes_ubwc(fmt, fb->width, fb->height, layout); - return _dpu_format_get_plane_sizes_linear(fmt, w, h, layout, pitches); + return _dpu_format_populate_plane_sizes_linear(fmt, fb->width, fb->height, + layout, fb->pitches); } static int _dpu_format_populate_addrs_ubwc( @@ -388,7 +399,7 @@ static int _dpu_format_populate_addrs_linear( return 0; } -int dpu_format_populate_layout( +int dpu_format_populate_addrs( struct msm_gem_address_space *aspace, struct drm_framebuffer *fb, struct dpu_hw_fmt_layout *layout) @@ -406,14 +417,6 @@ int dpu_format_populate_layout( return -ERANGE; } - layout->format = msm_framebuffer_format(fb); - - /* Populate the plane sizes etc via get_format */ - ret = dpu_format_get_plane_sizes(layout->format, fb->width, fb->height, - layout, fb->pitches); - if (ret) - return ret; - /* Populate the addresses given the fb */ if (MSM_FORMAT_IS_UBWC(layout->format) || MSM_FORMAT_IS_TILE(layout->format)) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h b/drivers/gpu/drm/m
[PATCH v3 0/9] drm/msm/dpu: be more friendly to X.org
Unlike other compositors X.org allocates a single framebuffer covering the whole screen space. This is not an issue with the single screens, but with the multi-monitor setup 5120x4096 becomes a limiting factor. Check the hardware-bound limitations and lift the FB max size to 16383x16383. Signed-off-by: Dmitry Baryshkov --- Changes in v3: - Reoder the functions to pull up a fix to the start of the patchset (Abhinav) - Rename the _dpu_crtc_setup_lm_bounds() to _dpu_crtc_check_and_setup_lm_bounds() (Abhinav) - Make dpu_crtc_mode_valid() static. - Link to v2: https://lore.kernel.org/r/20240603-dpu-mode-config-width-v2-0-16af52057...@linaro.org Changes in v2: - Added dpu_crtc_valid() to verify that 2*lm_width limit is enforced (Abhinav) - Link to v1: https://lore.kernel.org/r/20240319-dpu-mode-config-width-v1-0-d0fe6bf81...@linaro.org --- Dmitry Baryshkov (9): drm/msm/dpu: check for overflow in _dpu_crtc_setup_lm_bounds() drm/msm/dpu: drop dpu_format_check_modified_format drm/msm/dpu: drop dpu_format_populate_layout from dpu_plane_sspp_atomic_update drm/msm/dpu: split dpu_format_populate_layout drm/msm/dpu: move dpu_format_populate_plane_sizes to atomic_check drm/msm/dpu: check for the plane pitch overflow drm/msm/dpu: drop _dpu_crtc_check_and_setup_lm_bounds from atomic_begin drm/msm/dpu: merge MAX_IMG_WIDTH/HEIGHT with DPU_MAX_IMG_WIDTH/HEIGHT drm/msm/dpu: sync mode_config limits to the FB limits in dpu_plane.c drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 32 ++-- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c| 8 +- drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c| 91 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h| 24 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h| 2 + drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 10 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 37 + drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 3 + drivers/gpu/drm/msm/msm_kms.h | 6 -- 10 files changed, 91 insertions(+), 126 deletions(-) --- base-commit: 03d44168cbd7fc57d5de56a3730427db758fc7f6 change-id: 20240318-dpu-mode-config-width-626d3c7ad52a Best regards, -- Dmitry Baryshkov
[PATCH v3 2/9] drm/msm/dpu: drop dpu_format_check_modified_format
The msm_kms_funcs::check_modified_format() callback is not used by the driver. Drop it completely. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 43 - drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h | 16 --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 - drivers/gpu/drm/msm/msm_kms.h | 6 4 files changed, 66 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c index 6b1e9a617da3..027eb5ecff08 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c @@ -423,46 +423,3 @@ int dpu_format_populate_layout( return ret; } - -int dpu_format_check_modified_format( - const struct msm_kms *kms, - const struct msm_format *fmt, - const struct drm_mode_fb_cmd2 *cmd, - struct drm_gem_object **bos) -{ - const struct drm_format_info *info; - struct dpu_hw_fmt_layout layout; - uint32_t bos_total_size = 0; - int ret, i; - - if (!fmt || !cmd || !bos) { - DRM_ERROR("invalid arguments\n"); - return -EINVAL; - } - - info = drm_format_info(fmt->pixel_format); - if (!info) - return -EINVAL; - - ret = dpu_format_get_plane_sizes(fmt, cmd->width, cmd->height, - &layout, cmd->pitches); - if (ret) - return ret; - - for (i = 0; i < info->num_planes; i++) { - if (!bos[i]) { - DRM_ERROR("invalid handle for plane %d\n", i); - return -EINVAL; - } - if ((i == 0) || (bos[i] != bos[0])) - bos_total_size += bos[i]->size; - } - - if (bos_total_size < layout.total_size) { - DRM_ERROR("buffers total size too small %u expected %u\n", - bos_total_size, layout.total_size); - return -EINVAL; - } - - return 0; -} diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h index 210d0ed5f0af..ef1239c95058 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h @@ -31,22 +31,6 @@ static inline bool dpu_find_format(u32 format, const u32 *supported_formats, return false; } -/** - * dpu_format_check_modified_format - validate format and buffers for - * dpu non-standard, i.e. modified format - * @kms: kms driver - * @msm_fmt: pointer to the msm_fmt base pointer of an msm_format - * @cmd: fb_cmd2 structure user request - * @bos: gem buffer object list - * - * Return: error code on failure, 0 on success - */ -int dpu_format_check_modified_format( - const struct msm_kms *kms, - const struct msm_format *msm_fmt, - const struct drm_mode_fb_cmd2 *cmd, - struct drm_gem_object **bos); - /** * dpu_format_populate_layout - populate the given format layout based on * mmu, fb, and format found in the fb diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 1955848b1b78..0d1dcc94455c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -981,7 +981,6 @@ static const struct msm_kms_funcs kms_funcs = { .complete_commit = dpu_kms_complete_commit, .enable_vblank = dpu_kms_enable_vblank, .disable_vblank = dpu_kms_disable_vblank, - .check_modified_format = dpu_format_check_modified_format, .destroy = dpu_kms_destroy, .snapshot= dpu_kms_mdp_snapshot, #ifdef CONFIG_DEBUG_FS diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h index 1e0c54de3716..e60162744c66 100644 --- a/drivers/gpu/drm/msm/msm_kms.h +++ b/drivers/gpu/drm/msm/msm_kms.h @@ -92,12 +92,6 @@ struct msm_kms_funcs { * Format handling: */ - /* do format checking on format modified through fb_cmd2 modifiers */ - int (*check_modified_format)(const struct msm_kms *kms, - const struct msm_format *msm_fmt, - const struct drm_mode_fb_cmd2 *cmd, - struct drm_gem_object **bos); - /* misc: */ long (*round_pixclk)(struct msm_kms *kms, unsigned long rate, struct drm_encoder *encoder); -- 2.39.2
[PATCH v3 1/9] drm/msm/dpu: check for overflow in _dpu_crtc_setup_lm_bounds()
Make _dpu_crtc_setup_lm_bounds() check that CRTC width is not overflowing LM requirements. Rename the function accordingly. Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support") Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 17 + 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 9f2164782844..b0d81e8ea765 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -711,12 +711,13 @@ void dpu_crtc_complete_commit(struct drm_crtc *crtc) _dpu_crtc_complete_flip(crtc); } -static void _dpu_crtc_setup_lm_bounds(struct drm_crtc *crtc, +static int _dpu_crtc_check_and_setup_lm_bounds(struct drm_crtc *crtc, struct drm_crtc_state *state) { struct dpu_crtc_state *cstate = to_dpu_crtc_state(state); struct drm_display_mode *adj_mode = &state->adjusted_mode; u32 crtc_split_width = adj_mode->hdisplay / cstate->num_mixers; + struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc); int i; for (i = 0; i < cstate->num_mixers; i++) { @@ -727,7 +728,12 @@ static void _dpu_crtc_setup_lm_bounds(struct drm_crtc *crtc, r->y2 = adj_mode->vdisplay; trace_dpu_crtc_setup_lm_bounds(DRMID(crtc), i, r); + + if (drm_rect_width(r) > dpu_kms->catalog->caps->max_mixer_width) + return -E2BIG; } + + return 0; } static void _dpu_crtc_get_pcc_coeff(struct drm_crtc_state *state, @@ -803,7 +809,7 @@ static void dpu_crtc_atomic_begin(struct drm_crtc *crtc, DRM_DEBUG_ATOMIC("crtc%d\n", crtc->base.id); - _dpu_crtc_setup_lm_bounds(crtc, crtc->state); + _dpu_crtc_check_and_setup_lm_bounds(crtc, crtc->state); /* encoder will trigger pending mask now */ drm_for_each_encoder_mask(encoder, crtc->dev, crtc->state->encoder_mask) @@ -1197,8 +1203,11 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc, if (crtc_state->active_changed) crtc_state->mode_changed = true; - if (cstate->num_mixers) - _dpu_crtc_setup_lm_bounds(crtc, crtc_state); + if (cstate->num_mixers) { + rc = _dpu_crtc_check_and_setup_lm_bounds(crtc, crtc_state); + if (rc) + return rc; + } /* FIXME: move this to dpu_plane_atomic_check? */ drm_atomic_crtc_state_for_each_plane_state(plane, pstate, crtc_state) { -- 2.39.2
[PATCH] drm/i915/gt/uc: Fix typo in comment
Replace "dynmically" with "dynamically". Signed-off-by: Andi Shyti Cc: John Harrison --- drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h index 14797e80bc92..263c9c3f6a03 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h @@ -295,7 +295,7 @@ struct guc_update_scheduling_policy_header { } __packed; /* - * Can't dynmically allocate memory for the scheduling policy KLV because + * Can't dynamically allocate memory for the scheduling policy KLV because * it will be sent from within the reset path. Need a fixed size lump on * the stack instead :(. * -- 2.45.1
[PATCH] drm/i915/gt/uc: Evaluate GuC priority within locks
The ce->guc_state.lock was made to protect guc_prio, which indicates the GuC priority level. But at the begnning of the function we perform some sanity check of guc_prio outside its protected section. Move them within the locked region. Use this occasion to expand the if statement to make it clearer. Signed-off-by: Andi Shyti Cc: Matthew Brost Reviewed-by: Matthew Brost --- Cc: Daniele Ceraolo Spurio Changelog = - Removed the stable tags as this path is not normally under multi task stress. - Refactored the if statements in order to remove unnecessary checks (thanks, Daniele) - Added Matt's r-b. .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 27 +++ 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 0eaa1064242c..9400d0eb682b 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -4267,20 +4267,25 @@ static void guc_bump_inflight_request_prio(struct i915_request *rq, u8 new_guc_prio = map_i915_prio_to_guc_prio(prio); /* Short circuit function */ - if (prio < I915_PRIORITY_NORMAL || - rq->guc_prio == GUC_PRIO_FINI || - (rq->guc_prio != GUC_PRIO_INIT && -!new_guc_prio_higher(rq->guc_prio, new_guc_prio))) + if (prio < I915_PRIORITY_NORMAL) return; spin_lock(&ce->guc_state.lock); - if (rq->guc_prio != GUC_PRIO_FINI) { - if (rq->guc_prio != GUC_PRIO_INIT) - sub_context_inflight_prio(ce, rq->guc_prio); - rq->guc_prio = new_guc_prio; - add_context_inflight_prio(ce, rq->guc_prio); - update_context_prio(ce); - } + + if (rq->guc_prio == GUC_PRIO_FINI) + goto exit; + + if (!new_guc_prio_higher(rq->guc_prio, new_guc_prio)) + goto exit; + + if (rq->guc_prio != GUC_PRIO_INIT) + sub_context_inflight_prio(ce, rq->guc_prio); + + rq->guc_prio = new_guc_prio; + add_context_inflight_prio(ce, rq->guc_prio); + update_context_prio(ce); + +exit: spin_unlock(&ce->guc_state.lock); } -- 2.45.1
[PATCH] drm/i915/gt/uc: Evaluate GuC priority within locks
The ce->guc_state.lock was made to protect guc_prio, which indicates the GuC priority level. But at the begnning of the function we perform some sanity check of guc_prio outside its protected section. Move them within the locked region. Use this occasion to expand the if statement to make it clearer. Signed-off-by: Andi Shyti Cc: Matthew Brost Reviewed-by: Matthew Brost --- Cc: Daniele Ceraolo Spurio Changelog = - Removed the stable tags as this path is not normally under multi task stress. - Refactored the if statements in order to remove unnecessary checks (thanks, Daniele) - Added Matt's r-b. .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 27 +++ 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 0eaa1064242c..9400d0eb682b 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -4267,20 +4267,25 @@ static void guc_bump_inflight_request_prio(struct i915_request *rq, u8 new_guc_prio = map_i915_prio_to_guc_prio(prio); /* Short circuit function */ - if (prio < I915_PRIORITY_NORMAL || - rq->guc_prio == GUC_PRIO_FINI || - (rq->guc_prio != GUC_PRIO_INIT && -!new_guc_prio_higher(rq->guc_prio, new_guc_prio))) + if (prio < I915_PRIORITY_NORMAL) return; spin_lock(&ce->guc_state.lock); - if (rq->guc_prio != GUC_PRIO_FINI) { - if (rq->guc_prio != GUC_PRIO_INIT) - sub_context_inflight_prio(ce, rq->guc_prio); - rq->guc_prio = new_guc_prio; - add_context_inflight_prio(ce, rq->guc_prio); - update_context_prio(ce); - } + + if (rq->guc_prio == GUC_PRIO_FINI) + goto exit; + + if (!new_guc_prio_higher(rq->guc_prio, new_guc_prio)) + goto exit; + + if (rq->guc_prio != GUC_PRIO_INIT) + sub_context_inflight_prio(ce, rq->guc_prio); + + rq->guc_prio = new_guc_prio; + add_context_inflight_prio(ce, rq->guc_prio); + update_context_prio(ce); + +exit: spin_unlock(&ce->guc_state.lock); } -- 2.45.1
Re: [PATCH] fbdev: vesafb: Detect VGA compatibility from screen info's VESA attributes
On 6/13/24 23:50, Javier Martinez Canillas wrote: Helge Deller writes: On 6/13/24 11:02, Thomas Zimmermann wrote: Test the vesa_attributes field in struct screen_info for compatibility with VGA hardware. Vesafb currently tests bit 1 in screen_info's capabilities field, It sets the framebuffer address size and is unrelated to VGA. Section 4.4 of the Vesa VBE 2.0 specifications defines that bit 5 in the mode's attributes field signals VGA compatibility. The mode is compatible with VGA hardware if the bit is clear. In that case, the driver can access VGA state of the VBE's underlying hardware. The vesafb driver uses this feature to program the color LUT in palette modes. Without, colors might be incorrect. The problem got introduced in commit 89ec4c238e7a ("[PATCH] vesafb: Fix incorrect logo colors in x86_64"). It incorrectly stores the mode attributes in the screen_info's capabilities field and updates vesafb accordingly. Later, commit 5e8ddcbe8692 ("Video mode probing support for the new x86 setup code") fixed the screen_info, but did not update vesafb. Color output still tends to work, because bit 1 in capabilities is usually 0. Besides fixing the bug in vesafb, this commit introduces a helper that reads the correct bit from screen_info. Nice catch, Thomas! But do we really need this additional helper? Signed-off-by: Thomas Zimmermann Fixes: 5e8ddcbe8692 ("Video mode probing support for the new x86 setup code") Cc: # v2.6.23+ --- drivers/video/fbdev/vesafb.c | 2 +- include/linux/screen_info.h | 5 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c index 8ab64ae4cad3e..5a161750a3aee 100644 --- a/drivers/video/fbdev/vesafb.c +++ b/drivers/video/fbdev/vesafb.c @@ -271,7 +271,7 @@ static int vesafb_probe(struct platform_device *dev) if (si->orig_video_isVGA != VIDEO_TYPE_VLFB) return -ENODEV; - vga_compat = (si->capabilities & 2) ? 0 : 1; + vga_compat = !__screen_info_vbe_mode_nonvga(si); Instead maybe just this: ? +/* mode is VGA-compatible if BIT 5 is _NOT_ set */ +vga_compat = (si->vesa_attributes & BIT(5)) == 0; I suggest to make patch small, esp. if you ask for backport to v2.6.23+. I prefer the helper. It's a static inline anyways and having it as a function makes it much easier to read / understand. Really? +static inline bool __screen_info_vbe_mode_nonvga(const struct screen_info *si) +{ + return si->vesa_attributes & BIT(5); // VGA if _not_ set +} At least the double negation "!nonvga()" breaks my head and the "//" comment should be converted to /*..*/ IMHO. Helge
Re: [PATCH] drm/amd/display: Increase frame-larger-than warning limit
Hi Palmer (and AMD folks), On Tue, Jun 04, 2024 at 09:04:23AM -0700, Palmer Dabbelt wrote: > On Mon, 03 Jun 2024 15:29:48 PDT (-0700), nat...@kernel.org wrote: > > On Thu, May 30, 2024 at 07:57:42AM -0700, Palmer Dabbelt wrote: > > > From: Palmer Dabbelt > > > > > > I get a handful of build errors along the lines of > > > > > > > > > linux/drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:58:13: > > > error: stack frame size (2352) exceeds limit (2048) in > > > 'DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation' > > > [-Werror,-Wframe-larger-than] > > > static void > > > DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation( > > > ^ > > > > > > linux/drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:1724:6: > > > error: stack frame size (2096) exceeds limit (2048) in > > > 'dml32_ModeSupportAndSystemConfigurationFull' > > > [-Werror,-Wframe-larger-than] > > > void dml32_ModeSupportAndSystemConfigurationFull(struct > > > display_mode_lib *mode_lib) > > > ^ > > > > Judging from the message, this is clang/LLVM? What version? > > Yes, LLVM. Looks like I'm on 16.0.6. Probably time for an update, so I'll > give it a shot. FWIW, I can reproduce this with tip of tree, I was just curious in case that ended up mattering. > > I assume > > this showed up in 6.10-rc1 because of commit 77acc6b55ae4 ("riscv: add > > support for kernel-mode FPU"), which allows this driver to be built for > > RISC-V. > > Seems reasonable. This didn't show up until post-merge, not 100% sure why. > I didn't really dig any farther. Perhaps you fast forwarded your tree to include that commit? > > Is this allmodconfig or some other configuration? > > IIRC both "allmodconfig" and "allyesconfig" show it, but I don't have a > build tree sitting around to be 100% sure. Yeah, allmodconfig triggers it. I was able to come up with a "trivial" reproducer using cvise (attached to this mail if you are curious) that has worse stack usage by a rough factor of 2: $ clang --target=riscv64-linux-gnu -O2 -Wall -Wframe-larger-than=512 -c -o /dev/null display_mode_vba_32.i display_mode_vba_32.i:598:6: warning: stack frame size (1264) exceeds limit (512) in 'DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation' [-Wframe-larger-than] 598 | void DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation() { | ^ 1 warning generated. $ riscv64-linux-gcc -O2 -Wall -Wframe-larger-than=512 -c -o /dev/null display_mode_vba_32.i display_mode_vba_32.i: In function 'DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation': display_mode_vba_32.i:1729:1: warning: the frame size of 528 bytes is larger than 512 bytes [-Wframe-larger-than=] 1729 | } | ^ I have not done too much further investigation but this is almost certainly the same issue that has come up before [1][2] with the AMD display code using functions with a large number of parameters, such that they have to passed on the stack, coupled with inlining (if I remember correctly, LLVM gives more of an inlining discount the less a function is used in a file). While clang does poorly with that code, I am not interested in continuing to fix this code new hardware revision after new hardware revision. We could just avoid this code like we do for arm64 for a similar reason: diff --git a/drivers/gpu/drm/amd/display/Kconfig b/drivers/gpu/drm/amd/display/Kconfig index 5fcd4f778dc3..64df713df878 100644 --- a/drivers/gpu/drm/amd/display/Kconfig +++ b/drivers/gpu/drm/amd/display/Kconfig @@ -8,7 +8,7 @@ config DRM_AMD_DC depends on BROKEN || !CC_IS_CLANG || ARM64 || RISCV || SPARC64 || X86_64 select SND_HDA_COMPONENT if SND_HDA_CORE # !CC_IS_CLANG: https://github.com/ClangBuiltLinux/linux/issues/1752 - select DRM_AMD_DC_FP if ARCH_HAS_KERNEL_FPU_SUPPORT && (!ARM64 || !CC_IS_CLANG) + select DRM_AMD_DC_FP if ARCH_HAS_KERNEL_FPU_SUPPORT && (!(ARM64 || RISCV) || !CC_IS_CLANG) help Choose this option if you want to use the new display engine support for AMDGPU. This adds required support for Vega and [1]: https://lore.kernel.org/20231019205117.GA839902@dev-arch.thelio-3990X/ [2]: https://lore.kernel.org/20220830203409.3491379-1-nat...@kernel.org/ Cheers, Nathan enum { false, true }; enum output_encoder_class { dm_dp2p0 }; enum output_format_class { dm_420 }; enum source_format_class { dm_444_32 }; enum scan_direction_class { dm_vert }; enum dm_swizzle_mode { dm_sw_linear }; enum clock_change_support { dm_std_cvt }; enum odm_combine_mode { dm_odm_combine_mode_2to1dm_odm_combine_mode_4to1 }; enum immediate_flip_requirement { dm_immediate_flip_not_required }; enum unbounded_requesting_policy { dm_unbounded_requesting_disable }; enum dm_rotation_angle { dm_rotation_270m };
Re: [PATCH v2 05/10] mm/util: Fix possible race condition in kstrdup()
On Thu, 13 Jun 2024 at 14:14, Andrew Morton wrote: > > The concept sounds a little strange. If some code takes a copy of a > string while some other code is altering it, yes, the result will be a > mess. This is why get_task_comm() exists, and why it uses locking. The thing is, get_task_comm() is terminally broken. Nobody sane uses it, and sometimes it's literally _because_ it uses locking. Let's look at the numbers: - 39 uses of get_task_comm() - 2 uses of __get_task_comm() because the locking doesn't work - 447 uses of raw "current->comm" - 112 uses of raw 'ta*sk->comm' (and possibly IOW, we need to just accept the fact that nobody actually wants to use "get_task_comm()". It's a broken interface. It's inconvenient, and the locking makes it worse. Now, I'm not convinced that kstrdup() is what anybody should use should, but of the 600 "raw" uses of ->comm, four of them do seem to be kstrdup. Not great, I think they could be removed, but they are examples of people doing this. And I think it *would* be good to have the guarantee that yes, the kstrdup() result is always a proper string, even if it's used for unstable sources. Who knows what other unstable sources exist? I do suspect that most of the raw uses of 'xyz->comm' is for printouts. And I think we would be better with a '%pTSK' vsnprintf() format thing for that. Sadly, I don't think coccinelle can do the kinds of transforms that involve printf format strings. And no, a printk() string still couldn't use the locking version. Linus
Re: [PATCH 4/9] arm64: dts: rockchip: Add nodes for NPU and its MMU to rk3588s
Hi, On Wed, Jun 12, 2024 at 03:52:57PM GMT, Tomeu Vizoso wrote: > See Chapter 36 "RKNN" from the RK3588 TRM (Part 1). > > This is a derivative of NVIDIA's NVDLA, but with its own front-end > processor. > > Mostly taken from downstream. > > Signed-off-by: Tomeu Vizoso > --- Looking at the TRM I noticed, that this register is not mapped: RKNN_global_operation_enable Address: Operational Base + offset (0xF008) -- Sebastian > arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 53 > +++ > 1 file changed, 53 insertions(+) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi > b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi > index 6ac5ac8b48ab..a5d53578c8f6 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi > @@ -2665,6 +2665,59 @@ gpio4: gpio@fec5 { > #interrupt-cells = <2>; > }; > }; > + > + rknn: npu@fdab { > + compatible = "rockchip,rk3588-rknn", "rockchip,rknn"; > + reg = <0x0 0xfdab 0x0 0x9000>, > + <0x0 0xfdac 0x0 0x9000>, > + <0x0 0xfdad 0x0 0x9000>; > + interrupts = , > + , > + ; > + interrupt-names = "npu0_irq", "npu1_irq", "npu2_irq"; > + clocks = <&scmi_clk SCMI_CLK_NPU>, <&cru ACLK_NPU0>, > + <&cru ACLK_NPU1>, <&cru ACLK_NPU2>, > + <&cru HCLK_NPU0>, <&cru HCLK_NPU1>, > + <&cru HCLK_NPU2>, <&cru PCLK_NPU_ROOT>; > + clock-names = "clk_npu", > + "aclk0", "aclk1", "aclk2", > + "hclk0", "hclk1", "hclk2", > + "pclk"; > + assigned-clocks = <&scmi_clk SCMI_CLK_NPU>; > + assigned-clock-rates = <2>; > + resets = <&cru SRST_A_RKNN0>, <&cru SRST_A_RKNN1>, <&cru > SRST_A_RKNN2>, > + <&cru SRST_H_RKNN0>, <&cru SRST_H_RKNN1>, <&cru > SRST_H_RKNN2>; > + reset-names = "srst_a0", "srst_a1", "srst_a2", > + "srst_h0", "srst_h1", "srst_h2"; > + power-domains = <&power RK3588_PD_NPUTOP>, > + <&power RK3588_PD_NPU1>, > + <&power RK3588_PD_NPU2>; > + power-domain-names = "npu0", "npu1", "npu2"; > + iommus = <&rknn_mmu>; > + status = "disabled"; > + }; > + > + rknn_mmu: iommu@fdab9000 { > + compatible = "rockchip,rk3588-iommu"; > + reg = <0x0 0xfdab9000 0x0 0x100>, > + <0x0 0xfdaba000 0x0 0x100>, > + <0x0 0xfdaca000 0x0 0x100>, > + <0x0 0xfdada000 0x0 0x100>; > + interrupts = , > + , > + ; > + interrupt-names = "npu0_mmu", "npu1_mmu", "npu2_mmu"; > + clocks = <&cru ACLK_NPU0>, <&cru ACLK_NPU1>, <&cru ACLK_NPU2>, > + <&cru HCLK_NPU0>, <&cru HCLK_NPU1>, <&cru HCLK_NPU2>; > + clock-names = "aclk0", "aclk1", "aclk2", > + "iface0", "iface1", "iface2"; > + #iommu-cells = <0>; > + power-domains = <&power RK3588_PD_NPUTOP>, > + <&power RK3588_PD_NPU1>, > + <&power RK3588_PD_NPU2>; > + power-domain-names = "npu0", "npu1", "npu2"; > + status = "disabled"; > + }; > }; > > #include "rk3588s-pinctrl.dtsi" > > -- > 2.45.2 > > signature.asc Description: PGP signature
[PATCH v15 8/9] udmabuf: Pin the pages using memfd_pin_folios() API
Using memfd_pin_folios() will ensure that the pages are pinned correctly using FOLL_PIN. And, this also ensures that we don't accidentally break features such as memory hotunplug as it would not allow pinning pages in the movable zone. Using this new API also simplifies the code as we no longer have to deal with extracting individual pages from their mappings or handle shmem and hugetlb cases separately. Cc: David Hildenbrand Cc: Matthew Wilcox Cc: Daniel Vetter Cc: Hugh Dickins Cc: Peter Xu Cc: Jason Gunthorpe Cc: Gerd Hoffmann Cc: Dongwon Kim Cc: Junxiao Chang Acked-by: Dave Airlie Acked-by: Gerd Hoffmann Signed-off-by: Vivek Kasireddy --- drivers/dma-buf/udmabuf.c | 155 -- 1 file changed, 80 insertions(+), 75 deletions(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index e67515808ed3..047c3cd2ceff 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -30,6 +30,12 @@ struct udmabuf { struct sg_table *sg; struct miscdevice *device; pgoff_t *offsets; + struct list_head unpin_list; +}; + +struct udmabuf_folio { + struct folio *folio; + struct list_head list; }; static vm_fault_t udmabuf_vm_fault(struct vm_fault *vmf) @@ -153,17 +159,43 @@ static void unmap_udmabuf(struct dma_buf_attachment *at, return put_sg_table(at->dev, sg, direction); } +static void unpin_all_folios(struct list_head *unpin_list) +{ + struct udmabuf_folio *ubuf_folio; + + while (!list_empty(unpin_list)) { + ubuf_folio = list_first_entry(unpin_list, + struct udmabuf_folio, list); + unpin_folio(ubuf_folio->folio); + + list_del(&ubuf_folio->list); + kfree(ubuf_folio); + } +} + +static int add_to_unpin_list(struct list_head *unpin_list, +struct folio *folio) +{ + struct udmabuf_folio *ubuf_folio; + + ubuf_folio = kzalloc(sizeof(*ubuf_folio), GFP_KERNEL); + if (!ubuf_folio) + return -ENOMEM; + + ubuf_folio->folio = folio; + list_add_tail(&ubuf_folio->list, unpin_list); + return 0; +} + static void release_udmabuf(struct dma_buf *buf) { struct udmabuf *ubuf = buf->priv; struct device *dev = ubuf->device->this_device; - pgoff_t pg; if (ubuf->sg) put_sg_table(dev, ubuf->sg, DMA_BIDIRECTIONAL); - for (pg = 0; pg < ubuf->pagecount; pg++) - folio_put(ubuf->folios[pg]); + unpin_all_folios(&ubuf->unpin_list); kfree(ubuf->offsets); kfree(ubuf->folios); kfree(ubuf); @@ -218,64 +250,6 @@ static const struct dma_buf_ops udmabuf_ops = { #define SEALS_WANTED (F_SEAL_SHRINK) #define SEALS_DENIED (F_SEAL_WRITE) -static int handle_hugetlb_pages(struct udmabuf *ubuf, struct file *memfd, - pgoff_t offset, pgoff_t pgcnt, - pgoff_t *pgbuf) -{ - struct hstate *hpstate = hstate_file(memfd); - pgoff_t mapidx = offset >> huge_page_shift(hpstate); - pgoff_t subpgoff = (offset & ~huge_page_mask(hpstate)) >> PAGE_SHIFT; - pgoff_t maxsubpgs = huge_page_size(hpstate) >> PAGE_SHIFT; - struct folio *folio = NULL; - pgoff_t pgidx; - - mapidx <<= huge_page_order(hpstate); - for (pgidx = 0; pgidx < pgcnt; pgidx++) { - if (!folio) { - folio = __filemap_get_folio(memfd->f_mapping, - mapidx, - FGP_ACCESSED, 0); - if (IS_ERR(folio)) - return PTR_ERR(folio); - } - - folio_get(folio); - ubuf->folios[*pgbuf] = folio; - ubuf->offsets[*pgbuf] = subpgoff << PAGE_SHIFT; - (*pgbuf)++; - if (++subpgoff == maxsubpgs) { - folio_put(folio); - folio = NULL; - subpgoff = 0; - mapidx += pages_per_huge_page(hpstate); - } - } - - if (folio) - folio_put(folio); - - return 0; -} - -static int handle_shmem_pages(struct udmabuf *ubuf, struct file *memfd, - pgoff_t offset, pgoff_t pgcnt, - pgoff_t *pgbuf) -{ - pgoff_t pgidx, pgoff = offset >> PAGE_SHIFT; - struct folio *folio = NULL; - - for (pgidx = 0; pgidx < pgcnt; pgidx++) { - folio = shmem_read_folio(memfd->f_mapping, pgoff + pgidx); - if (IS_ERR(folio)) - return PTR_ERR(folio); - - ubuf->folios[*pgbuf] = folio; - (*pgbuf)++; - } - - return 0; -} - static int check_memfd_seals(struct file *memfd) { int seals; @@ -321,16 +295,1
[PATCH v15 5/9] udmabuf: Use vmf_insert_pfn and VM_PFNMAP for handling mmap
Add VM_PFNMAP to vm_flags in the mmap handler to ensure that the mappings would be managed without using struct page. And, in the vm_fault handler, use vmf_insert_pfn to share the page's pfn to userspace instead of directly sharing the page (via struct page *). Cc: David Hildenbrand Cc: Daniel Vetter Cc: Hugh Dickins Cc: Peter Xu Cc: Jason Gunthorpe Cc: Gerd Hoffmann Cc: Dongwon Kim Cc: Junxiao Chang Suggested-by: David Hildenbrand Acked-by: David Hildenbrand Acked-by: Dave Airlie Acked-by: Gerd Hoffmann Signed-off-by: Vivek Kasireddy --- drivers/dma-buf/udmabuf.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index c40645999648..820c993c8659 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -35,12 +35,13 @@ static vm_fault_t udmabuf_vm_fault(struct vm_fault *vmf) struct vm_area_struct *vma = vmf->vma; struct udmabuf *ubuf = vma->vm_private_data; pgoff_t pgoff = vmf->pgoff; + unsigned long pfn; if (pgoff >= ubuf->pagecount) return VM_FAULT_SIGBUS; - vmf->page = ubuf->pages[pgoff]; - get_page(vmf->page); - return 0; + + pfn = page_to_pfn(ubuf->pages[pgoff]); + return vmf_insert_pfn(vma, vmf->address, pfn); } static const struct vm_operations_struct udmabuf_vm_ops = { @@ -56,6 +57,7 @@ static int mmap_udmabuf(struct dma_buf *buf, struct vm_area_struct *vma) vma->vm_ops = &udmabuf_vm_ops; vma->vm_private_data = ubuf; + vm_flags_set(vma, VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP); return 0; } -- 2.45.1
[PATCH v15 9/9] selftests/udmabuf: Add tests to verify data after page migration
Since the memfd pages associated with a udmabuf may be migrated as part of udmabuf create, we need to verify the data coherency after successful migration. The new tests added in this patch try to do just that using 4k sized pages and also 2 MB sized huge pages for the memfd. Successful completion of the tests would mean that there is no disconnect between the memfd pages and the ones associated with a udmabuf. And, these tests can also be augmented in the future to test newer udmabuf features (such as handling memfd hole punch). The idea for these tests comes from a patch by Mike Kravetz here: https://lists.freedesktop.org/archives/dri-devel/2023-June/410623.html v1->v2: (suggestions from Shuah) - Use ksft_* functions to print and capture results of tests - Use appropriate KSFT_* status codes for exit() - Add Mike Kravetz's suggested-by tag Cc: Shuah Khan Cc: David Hildenbrand Cc: Daniel Vetter Cc: Hugh Dickins Cc: Peter Xu Cc: Jason Gunthorpe Cc: Gerd Hoffmann Cc: Dongwon Kim Cc: Junxiao Chang Cc: linux-kselft...@vger.kernel.org Suggested-by: Mike Kravetz Acked-by: Dave Airlie Acked-by: Gerd Hoffmann Signed-off-by: Vivek Kasireddy --- .../selftests/drivers/dma-buf/udmabuf.c | 214 +++--- 1 file changed, 183 insertions(+), 31 deletions(-) diff --git a/tools/testing/selftests/drivers/dma-buf/udmabuf.c b/tools/testing/selftests/drivers/dma-buf/udmabuf.c index c812080e304e..6062723a172e 100644 --- a/tools/testing/selftests/drivers/dma-buf/udmabuf.c +++ b/tools/testing/selftests/drivers/dma-buf/udmabuf.c @@ -9,52 +9,162 @@ #include #include #include +#include #include #include +#include #include #include +#include "../../kselftest.h" #define TEST_PREFIX"drivers/dma-buf/udmabuf" #define NUM_PAGES 4 +#define NUM_ENTRIES 4 +#define MEMFD_SIZE 1024 /* in pages */ -static int memfd_create(const char *name, unsigned int flags) +static unsigned int page_size; + +static int create_memfd_with_seals(off64_t size, bool hpage) +{ + int memfd, ret; + unsigned int flags = MFD_ALLOW_SEALING; + + if (hpage) + flags |= MFD_HUGETLB; + + memfd = memfd_create("udmabuf-test", flags); + if (memfd < 0) { + ksft_print_msg("%s: [skip,no-memfd]\n", TEST_PREFIX); + exit(KSFT_SKIP); + } + + ret = fcntl(memfd, F_ADD_SEALS, F_SEAL_SHRINK); + if (ret < 0) { + ksft_print_msg("%s: [skip,fcntl-add-seals]\n", TEST_PREFIX); + exit(KSFT_SKIP); + } + + ret = ftruncate(memfd, size); + if (ret == -1) { + ksft_print_msg("%s: [FAIL,memfd-truncate]\n", TEST_PREFIX); + exit(KSFT_FAIL); + } + + return memfd; +} + +static int create_udmabuf_list(int devfd, int memfd, off64_t memfd_size) +{ + struct udmabuf_create_list *list; + int ubuf_fd, i; + + list = malloc(sizeof(struct udmabuf_create_list) + + sizeof(struct udmabuf_create_item) * NUM_ENTRIES); + if (!list) { + ksft_print_msg("%s: [FAIL, udmabuf-malloc]\n", TEST_PREFIX); + exit(KSFT_FAIL); + } + + for (i = 0; i < NUM_ENTRIES; i++) { + list->list[i].memfd = memfd; + list->list[i].offset = i * (memfd_size / NUM_ENTRIES); + list->list[i].size = getpagesize() * NUM_PAGES; + } + + list->count = NUM_ENTRIES; + list->flags = UDMABUF_FLAGS_CLOEXEC; + ubuf_fd = ioctl(devfd, UDMABUF_CREATE_LIST, list); + free(list); + if (ubuf_fd < 0) { + ksft_print_msg("%s: [FAIL, udmabuf-create]\n", TEST_PREFIX); + exit(KSFT_FAIL); + } + + return ubuf_fd; +} + +static void write_to_memfd(void *addr, off64_t size, char chr) +{ + int i; + + for (i = 0; i < size / page_size; i++) { + *((char *)addr + (i * page_size)) = chr; + } +} + +static void *mmap_fd(int fd, off64_t size) { - return syscall(__NR_memfd_create, name, flags); + void *addr; + + addr = mmap(NULL, size, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); + if (addr == MAP_FAILED) { + ksft_print_msg("%s: ubuf_fd mmap fail\n", TEST_PREFIX); + exit(KSFT_FAIL); + } + + return addr; +} + +static int compare_chunks(void *addr1, void *addr2, off64_t memfd_size) +{ + off64_t off; + int i = 0, j, k = 0, ret = 0; + char char1, char2; + + while (i < NUM_ENTRIES) { + off = i * (memfd_size / NUM_ENTRIES); + for (j = 0; j < NUM_PAGES; j++, k++) { + char1 = *((char *)addr1 + off + (j * getpagesize())); + char2 = *((char *)addr2 + (k * getpagesize())); + if (char1 != char2) { + ret = -1; + goto err; + } + } + i++; +
[PATCH v15 4/9] udmabuf: add CONFIG_MMU dependency
From: Arnd Bergmann There is no !CONFIG_MMU version of vmf_insert_pfn(): arm-linux-gnueabi-ld: drivers/dma-buf/udmabuf.o: in function `udmabuf_vm_fault': udmabuf.c:(.text+0xaa): undefined reference to `vmf_insert_pfn' Fixes: d1d00dd1fd2f ("udmabuf: use vmf_insert_pfn and VM_PFNMAP for handling mmap") Signed-off-by: Arnd Bergmann Acked-by: David Hildenbrand Acked-by: Vivek Kasireddy --- drivers/dma-buf/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig index e4dc53a36428..b46eb8a552d7 100644 --- a/drivers/dma-buf/Kconfig +++ b/drivers/dma-buf/Kconfig @@ -35,6 +35,7 @@ config UDMABUF default n depends on DMA_SHARED_BUFFER depends on MEMFD_CREATE || COMPILE_TEST + depends on MMU help A driver to let userspace turn memfd regions into dma-bufs. Qemu can use this to create host dmabufs for guest framebuffers. -- 2.45.1
[PATCH v15 1/9] mm/gup: Introduce unpin_folio/unpin_folios helpers
These helpers are the folio versions of unpin_user_page/unpin_user_pages. They are currently only useful for unpinning folios pinned by memfd_pin_folios() or other associated routines. However, they could find new uses in the future, when more and more folio-only helpers are added to GUP. We should probably sanity check the folio as part of unpin similar to how it is done in unpin_user_page/unpin_user_pages but we cannot cleanly do that at the moment without also checking the subpage. Therefore, sanity checking needs to be added to these routines once we have a way to determine if any given folio is anon-exclusive (via a per folio AnonExclusive flag). Cc: David Hildenbrand Cc: Matthew Wilcox Cc: Christoph Hellwig Cc: Jason Gunthorpe Cc: Peter Xu Suggested-by: David Hildenbrand Reviewed-by: David Hildenbrand Acked-by: Dave Airlie Acked-by: Gerd Hoffmann Signed-off-by: Vivek Kasireddy --- include/linux/mm.h | 2 ++ mm/gup.c | 47 ++ 2 files changed, 49 insertions(+) diff --git a/include/linux/mm.h b/include/linux/mm.h index 25b541974134..0f953405834c 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -1579,11 +1579,13 @@ static inline void put_page(struct page *page) #define GUP_PIN_COUNTING_BIAS (1U << 10) void unpin_user_page(struct page *page); +void unpin_folio(struct folio *folio); void unpin_user_pages_dirty_lock(struct page **pages, unsigned long npages, bool make_dirty); void unpin_user_page_range_dirty_lock(struct page *page, unsigned long npages, bool make_dirty); void unpin_user_pages(struct page **pages, unsigned long npages); +void unpin_folios(struct folio **folios, unsigned long nfolios); static inline bool is_cow_mapping(vm_flags_t flags) { diff --git a/mm/gup.c b/mm/gup.c index 6ff9f95a99a7..d9ea60621628 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -276,6 +276,19 @@ void unpin_user_page(struct page *page) } EXPORT_SYMBOL(unpin_user_page); +/** + * unpin_folio() - release a dma-pinned folio + * @folio: pointer to folio to be released + * + * Folios that were pinned via memfd_pin_folios() or other similar routines + * must be released either using unpin_folio() or unpin_folios(). + */ +void unpin_folio(struct folio *folio) +{ + gup_put_folio(folio, 1, FOLL_PIN); +} +EXPORT_SYMBOL_GPL(unpin_folio); + /** * folio_add_pin - Try to get an additional pin on a pinned folio * @folio: The folio to be pinned @@ -488,6 +501,40 @@ void unpin_user_pages(struct page **pages, unsigned long npages) } EXPORT_SYMBOL(unpin_user_pages); +/** + * unpin_folios() - release an array of gup-pinned folios. + * @folios: array of folios to be marked dirty and released. + * @nfolios: number of folios in the @folios array. + * + * For each folio in the @folios array, release the folio using gup_put_folio. + * + * Please see the unpin_folio() documentation for details. + */ +void unpin_folios(struct folio **folios, unsigned long nfolios) +{ + unsigned long i = 0, j; + + /* +* If this WARN_ON() fires, then the system *might* be leaking folios +* (by leaving them pinned), but probably not. More likely, gup/pup +* returned a hard -ERRNO error to the caller, who erroneously passed +* it here. +*/ + if (WARN_ON(IS_ERR_VALUE(nfolios))) + return; + + while (i < nfolios) { + for (j = i + 1; j < nfolios; j++) + if (folios[i] != folios[j]) + break; + + if (folios[i]) + gup_put_folio(folios[i], j - i, FOLL_PIN); + i = j; + } +} +EXPORT_SYMBOL_GPL(unpin_folios); + /* * Set the MMF_HAS_PINNED if not set yet; after set it'll be there for the mm's * lifecycle. Avoid setting the bit unless necessary, or it might cause write -- 2.45.1
[PATCH v15 6/9] udmabuf: Add back support for mapping hugetlb pages
A user or admin can configure a VMM (Qemu) Guest's memory to be backed by hugetlb pages for various reasons. However, a Guest OS would still allocate (and pin) buffers that are backed by regular 4k sized pages. In order to map these buffers and create dma-bufs for them on the Host, we first need to find the hugetlb pages where the buffer allocations are located and then determine the offsets of individual chunks (within those pages) and use this information to eventually populate a scatterlist. Testcase: default_hugepagesz=2M hugepagesz=2M hugepages=2500 options were passed to the Host kernel and Qemu was launched with these relevant options: qemu-system-x86_64 -m 4096m -device virtio-gpu-pci,max_outputs=1,blob=true,xres=1920,yres=1080 -display gtk,gl=on -object memory-backend-memfd,hugetlb=on,id=mem1,size=4096M -machine memory-backend=mem1 Replacing -display gtk,gl=on with -display gtk,gl=off above would exercise the mmap handler. Cc: David Hildenbrand Cc: Daniel Vetter Cc: Mike Kravetz Cc: Hugh Dickins Cc: Peter Xu Cc: Jason Gunthorpe Cc: Gerd Hoffmann Cc: Dongwon Kim Cc: Junxiao Chang Acked-by: Mike Kravetz (v2) Acked-by: Dave Airlie Acked-by: Gerd Hoffmann Signed-off-by: Vivek Kasireddy --- drivers/dma-buf/udmabuf.c | 122 +++--- 1 file changed, 101 insertions(+), 21 deletions(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 820c993c8659..274defd3fa3e 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -28,6 +29,7 @@ struct udmabuf { struct page **pages; struct sg_table *sg; struct miscdevice *device; + pgoff_t *offsets; }; static vm_fault_t udmabuf_vm_fault(struct vm_fault *vmf) @@ -41,6 +43,8 @@ static vm_fault_t udmabuf_vm_fault(struct vm_fault *vmf) return VM_FAULT_SIGBUS; pfn = page_to_pfn(ubuf->pages[pgoff]); + pfn += ubuf->offsets[pgoff] >> PAGE_SHIFT; + return vmf_insert_pfn(vma, vmf->address, pfn); } @@ -90,23 +94,29 @@ static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf, { struct udmabuf *ubuf = buf->priv; struct sg_table *sg; + struct scatterlist *sgl; + unsigned int i = 0; int ret; sg = kzalloc(sizeof(*sg), GFP_KERNEL); if (!sg) return ERR_PTR(-ENOMEM); - ret = sg_alloc_table_from_pages(sg, ubuf->pages, ubuf->pagecount, - 0, ubuf->pagecount << PAGE_SHIFT, - GFP_KERNEL); + + ret = sg_alloc_table(sg, ubuf->pagecount, GFP_KERNEL); if (ret < 0) - goto err; + goto err_alloc; + + for_each_sg(sg->sgl, sgl, ubuf->pagecount, i) + sg_set_page(sgl, ubuf->pages[i], PAGE_SIZE, ubuf->offsets[i]); + ret = dma_map_sgtable(dev, sg, direction, 0); if (ret < 0) - goto err; + goto err_map; return sg; -err: +err_map: sg_free_table(sg); +err_alloc: kfree(sg); return ERR_PTR(ret); } @@ -143,6 +153,7 @@ static void release_udmabuf(struct dma_buf *buf) for (pg = 0; pg < ubuf->pagecount; pg++) put_page(ubuf->pages[pg]); + kfree(ubuf->offsets); kfree(ubuf->pages); kfree(ubuf); } @@ -196,17 +207,77 @@ static const struct dma_buf_ops udmabuf_ops = { #define SEALS_WANTED (F_SEAL_SHRINK) #define SEALS_DENIED (F_SEAL_WRITE) +static int handle_hugetlb_pages(struct udmabuf *ubuf, struct file *memfd, + pgoff_t offset, pgoff_t pgcnt, + pgoff_t *pgbuf) +{ + struct hstate *hpstate = hstate_file(memfd); + pgoff_t mapidx = offset >> huge_page_shift(hpstate); + pgoff_t subpgoff = (offset & ~huge_page_mask(hpstate)) >> PAGE_SHIFT; + pgoff_t maxsubpgs = huge_page_size(hpstate) >> PAGE_SHIFT; + struct page *hpage = NULL; + struct folio *folio; + pgoff_t pgidx; + + mapidx <<= huge_page_order(hpstate); + for (pgidx = 0; pgidx < pgcnt; pgidx++) { + if (!hpage) { + folio = __filemap_get_folio(memfd->f_mapping, + mapidx, + FGP_ACCESSED, 0); + if (IS_ERR(folio)) + return PTR_ERR(folio); + + hpage = &folio->page; + } + + get_page(hpage); + ubuf->pages[*pgbuf] = hpage; + ubuf->offsets[*pgbuf] = subpgoff << PAGE_SHIFT; + (*pgbuf)++; + if (++subpgoff == maxsubpgs) { + put_page(hpage); + hpage = NULL; + subpgoff = 0; + map
[PATCH v15 7/9] udmabuf: Convert udmabuf driver to use folios
This is mainly a preparatory patch to use memfd_pin_folios() API for pinning folios. Using folios instead of pages makes sense as the udmabuf driver needs to handle both shmem and hugetlb cases. And, using the memfd_pin_folios() API makes this easier as we no longer need to separately handle shmem vs hugetlb cases in the udmabuf driver. Note that, the function vmap_udmabuf() still needs a list of pages; so, we collect all the head pages into a local array in this case. Other changes in this patch include the addition of helpers for checking the memfd seals and exporting dmabuf. Moving code from udmabuf_create() into these helpers improves readability given that udmabuf_create() is a bit long. Cc: David Hildenbrand Cc: Matthew Wilcox Cc: Daniel Vetter Cc: Hugh Dickins Cc: Peter Xu Cc: Jason Gunthorpe Cc: Gerd Hoffmann Cc: Dongwon Kim Cc: Junxiao Chang Acked-by: Dave Airlie Acked-by: Gerd Hoffmann Signed-off-by: Vivek Kasireddy --- drivers/dma-buf/udmabuf.c | 139 +++--- 1 file changed, 83 insertions(+), 56 deletions(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 274defd3fa3e..e67515808ed3 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -26,7 +26,7 @@ MODULE_PARM_DESC(size_limit_mb, "Max size of a dmabuf, in megabytes. Default is struct udmabuf { pgoff_t pagecount; - struct page **pages; + struct folio **folios; struct sg_table *sg; struct miscdevice *device; pgoff_t *offsets; @@ -42,7 +42,7 @@ static vm_fault_t udmabuf_vm_fault(struct vm_fault *vmf) if (pgoff >= ubuf->pagecount) return VM_FAULT_SIGBUS; - pfn = page_to_pfn(ubuf->pages[pgoff]); + pfn = folio_pfn(ubuf->folios[pgoff]); pfn += ubuf->offsets[pgoff] >> PAGE_SHIFT; return vmf_insert_pfn(vma, vmf->address, pfn); @@ -68,11 +68,21 @@ static int mmap_udmabuf(struct dma_buf *buf, struct vm_area_struct *vma) static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map) { struct udmabuf *ubuf = buf->priv; + struct page **pages; void *vaddr; + pgoff_t pg; dma_resv_assert_held(buf->resv); - vaddr = vm_map_ram(ubuf->pages, ubuf->pagecount, -1); + pages = kmalloc_array(ubuf->pagecount, sizeof(*pages), GFP_KERNEL); + if (!pages) + return -ENOMEM; + + for (pg = 0; pg < ubuf->pagecount; pg++) + pages[pg] = &ubuf->folios[pg]->page; + + vaddr = vm_map_ram(pages, ubuf->pagecount, -1); + kfree(pages); if (!vaddr) return -EINVAL; @@ -107,7 +117,8 @@ static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf, goto err_alloc; for_each_sg(sg->sgl, sgl, ubuf->pagecount, i) - sg_set_page(sgl, ubuf->pages[i], PAGE_SIZE, ubuf->offsets[i]); + sg_set_folio(sgl, ubuf->folios[i], PAGE_SIZE, +ubuf->offsets[i]); ret = dma_map_sgtable(dev, sg, direction, 0); if (ret < 0) @@ -152,9 +163,9 @@ static void release_udmabuf(struct dma_buf *buf) put_sg_table(dev, ubuf->sg, DMA_BIDIRECTIONAL); for (pg = 0; pg < ubuf->pagecount; pg++) - put_page(ubuf->pages[pg]); + folio_put(ubuf->folios[pg]); kfree(ubuf->offsets); - kfree(ubuf->pages); + kfree(ubuf->folios); kfree(ubuf); } @@ -215,36 +226,33 @@ static int handle_hugetlb_pages(struct udmabuf *ubuf, struct file *memfd, pgoff_t mapidx = offset >> huge_page_shift(hpstate); pgoff_t subpgoff = (offset & ~huge_page_mask(hpstate)) >> PAGE_SHIFT; pgoff_t maxsubpgs = huge_page_size(hpstate) >> PAGE_SHIFT; - struct page *hpage = NULL; - struct folio *folio; + struct folio *folio = NULL; pgoff_t pgidx; mapidx <<= huge_page_order(hpstate); for (pgidx = 0; pgidx < pgcnt; pgidx++) { - if (!hpage) { + if (!folio) { folio = __filemap_get_folio(memfd->f_mapping, mapidx, FGP_ACCESSED, 0); if (IS_ERR(folio)) return PTR_ERR(folio); - - hpage = &folio->page; } - get_page(hpage); - ubuf->pages[*pgbuf] = hpage; + folio_get(folio); + ubuf->folios[*pgbuf] = folio; ubuf->offsets[*pgbuf] = subpgoff << PAGE_SHIFT; (*pgbuf)++; if (++subpgoff == maxsubpgs) { - put_page(hpage); - hpage = NULL; + folio_put(folio); + folio = NULL; subpgoff = 0; mapidx += pages_per_huge_page(hpstate
[PATCH v15 2/9] mm/gup: Introduce check_and_migrate_movable_folios()
This helper is the folio equivalent of check_and_migrate_movable_pages(). Therefore, all the rules that apply to check_and_migrate_movable_pages() also apply to this one as well. Currently, this helper is only used by memfd_pin_folios(). This patch also includes changes to rename and convert the internal functions collect_longterm_unpinnable_pages() and migrate_longterm_unpinnable_pages() to work on folios. As a result, check_and_migrate_movable_pages() is now a wrapper around check_and_migrate_movable_folios(). Cc: David Hildenbrand Cc: Matthew Wilcox Cc: Christoph Hellwig Cc: Jason Gunthorpe Cc: Peter Xu Suggested-by: David Hildenbrand Acked-by: David Hildenbrand Acked-by: Dave Airlie Acked-by: Gerd Hoffmann Signed-off-by: Vivek Kasireddy --- mm/gup.c | 124 ++- 1 file changed, 77 insertions(+), 47 deletions(-) diff --git a/mm/gup.c b/mm/gup.c index d9ea60621628..a88e19c78730 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -2427,19 +2427,19 @@ struct page *get_dump_page(unsigned long addr) #ifdef CONFIG_MIGRATION /* - * Returns the number of collected pages. Return value is always >= 0. + * Returns the number of collected folios. Return value is always >= 0. */ -static unsigned long collect_longterm_unpinnable_pages( - struct list_head *movable_page_list, - unsigned long nr_pages, - struct page **pages) +static unsigned long collect_longterm_unpinnable_folios( + struct list_head *movable_folio_list, + unsigned long nr_folios, + struct folio **folios) { unsigned long i, collected = 0; struct folio *prev_folio = NULL; bool drain_allow = true; - for (i = 0; i < nr_pages; i++) { - struct folio *folio = page_folio(pages[i]); + for (i = 0; i < nr_folios; i++) { + struct folio *folio = folios[i]; if (folio == prev_folio) continue; @@ -2454,7 +2454,7 @@ static unsigned long collect_longterm_unpinnable_pages( continue; if (folio_test_hugetlb(folio)) { - isolate_hugetlb(folio, movable_page_list); + isolate_hugetlb(folio, movable_folio_list); continue; } @@ -2466,7 +2466,7 @@ static unsigned long collect_longterm_unpinnable_pages( if (!folio_isolate_lru(folio)) continue; - list_add_tail(&folio->lru, movable_page_list); + list_add_tail(&folio->lru, movable_folio_list); node_stat_mod_folio(folio, NR_ISOLATED_ANON + folio_is_file_lru(folio), folio_nr_pages(folio)); @@ -2476,27 +2476,28 @@ static unsigned long collect_longterm_unpinnable_pages( } /* - * Unpins all pages and migrates device coherent pages and movable_page_list. - * Returns -EAGAIN if all pages were successfully migrated or -errno for failure - * (or partial success). + * Unpins all folios and migrates device coherent folios and movable_folio_list. + * Returns -EAGAIN if all folios were successfully migrated or -errno for + * failure (or partial success). */ -static int migrate_longterm_unpinnable_pages( - struct list_head *movable_page_list, - unsigned long nr_pages, - struct page **pages) +static int migrate_longterm_unpinnable_folios( + struct list_head *movable_folio_list, + unsigned long nr_folios, + struct folio **folios) { int ret; unsigned long i; - for (i = 0; i < nr_pages; i++) { - struct folio *folio = page_folio(pages[i]); + for (i = 0; i < nr_folios; i++) { + struct folio *folio = folios[i]; if (folio_is_device_coherent(folio)) { /* -* Migration will fail if the page is pinned, so convert -* the pin on the source page to a normal reference. +* Migration will fail if the folio is pinned, so +* convert the pin on the source folio to a normal +* reference. */ - pages[i] = NULL; + folios[i] = NULL; folio_get(folio); gup_put_folio(folio, 1, FOLL_PIN); @@ -2509,24 +2510,24 @@ static int migrate_longterm_unpinnable_pages( } /* -* We can't migrate pages with une
[PATCH v15 3/9] mm/gup: Introduce memfd_pin_folios() for pinning memfd folios
For drivers that would like to longterm-pin the folios associated with a memfd, the memfd_pin_folios() API provides an option to not only pin the folios via FOLL_PIN but also to check and migrate them if they reside in movable zone or CMA block. This API currently works with memfds but it should work with any files that belong to either shmemfs or hugetlbfs. Files belonging to other filesystems are rejected for now. The folios need to be located first before pinning them via FOLL_PIN. If they are found in the page cache, they can be immediately pinned. Otherwise, they need to be allocated using the filesystem specific APIs and then pinned. Cc: David Hildenbrand Cc: Matthew Wilcox (Oracle) Cc: Christoph Hellwig Cc: Daniel Vetter Cc: Hugh Dickins Cc: Peter Xu Cc: Gerd Hoffmann Cc: Dongwon Kim Cc: Junxiao Chang Suggested-by: Jason Gunthorpe Reviewed-by: Jason Gunthorpe (v2) Reviewed-by: David Hildenbrand (v3) Reviewed-by: Christoph Hellwig (v6) Acked-by: Dave Airlie Acked-by: Gerd Hoffmann Signed-off-by: Vivek Kasireddy --- include/linux/memfd.h | 5 ++ include/linux/mm.h| 3 + mm/gup.c | 136 ++ mm/memfd.c| 35 +++ 4 files changed, 179 insertions(+) diff --git a/include/linux/memfd.h b/include/linux/memfd.h index e7abf6fa4c52..3f2cf339ceaf 100644 --- a/include/linux/memfd.h +++ b/include/linux/memfd.h @@ -6,11 +6,16 @@ #ifdef CONFIG_MEMFD_CREATE extern long memfd_fcntl(struct file *file, unsigned int cmd, unsigned int arg); +struct folio *memfd_alloc_folio(struct file *memfd, pgoff_t idx); #else static inline long memfd_fcntl(struct file *f, unsigned int c, unsigned int a) { return -EINVAL; } +static inline struct folio *memfd_alloc_folio(struct file *memfd, pgoff_t idx) +{ + return ERR_PTR(-EINVAL); +} #endif #endif /* __LINUX_MEMFD_H */ diff --git a/include/linux/mm.h b/include/linux/mm.h index 0f953405834c..42e3752b5eed 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -2495,6 +2495,9 @@ long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages, struct page **pages, unsigned int gup_flags); long pin_user_pages_unlocked(unsigned long start, unsigned long nr_pages, struct page **pages, unsigned int gup_flags); +long memfd_pin_folios(struct file *memfd, loff_t start, loff_t end, + struct folio **folios, unsigned int max_folios, + pgoff_t *offset); int get_user_pages_fast(unsigned long start, int nr_pages, unsigned int gup_flags, struct page **pages); diff --git a/mm/gup.c b/mm/gup.c index a88e19c78730..0278dd94f3e4 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -17,6 +18,7 @@ #include #include #include +#include #include #include @@ -3747,3 +3749,137 @@ long pin_user_pages_unlocked(unsigned long start, unsigned long nr_pages, &locked, gup_flags); } EXPORT_SYMBOL(pin_user_pages_unlocked); + +/** + * memfd_pin_folios() - pin folios associated with a memfd + * @memfd: the memfd whose folios are to be pinned + * @start: the first memfd offset + * @end:the last memfd offset (inclusive) + * @folios: array that receives pointers to the folios pinned + * @max_folios: maximum number of entries in @folios + * @offset: the offset into the first folio + * + * Attempt to pin folios associated with a memfd in the contiguous range + * [start, end]. Given that a memfd is either backed by shmem or hugetlb, + * the folios can either be found in the page cache or need to be allocated + * if necessary. Once the folios are located, they are all pinned via + * FOLL_PIN and @offset is populatedwith the offset into the first folio. + * And, eventually, these pinned folios must be released either using + * unpin_folios() or unpin_folio(). + * + * It must be noted that the folios may be pinned for an indefinite amount + * of time. And, in most cases, the duration of time they may stay pinned + * would be controlled by the userspace. This behavior is effectively the + * same as using FOLL_LONGTERM with other GUP APIs. + * + * Returns number of folios pinned, which could be less than @max_folios + * as it depends on the folio sizes that cover the range [start, end]. + * If no folios were pinned, it returns -errno. + */ +long memfd_pin_folios(struct file *memfd, loff_t start, loff_t end, + struct folio **folios, unsigned int max_folios, + pgoff_t *offset) +{ + unsigned int flags, nr_folios, nr_found; + unsigned int i, pgshift = PAGE_SHIFT; + pgoff_t start_idx, end_idx, next_idx; + struct folio *folio = NULL; + struct folio_batch fbatch; + struct hstate *h; + long ret = -EINVAL; + + if (start < 0 || start > end || !max_folios) +
[PATCH v15 0/9] mm/gup: Introduce memfd_pin_folios() for pinning memfd folios
Currently, some drivers (e.g, Udmabuf) that want to longterm-pin the pages/folios associated with a memfd, do so by simply taking a reference on them. This is not desirable because the pages/folios may reside in Movable zone or CMA block. Therefore, having drivers use memfd_pin_folios() API ensures that the folios are appropriately pinned via FOLL_PIN for longterm DMA. This patchset also introduces a few helpers and converts the Udmabuf driver to use folios and memfd_pin_folios() API to longterm-pin the folios for DMA. Two new Udmabuf selftests are also included to test the driver and the new API. --- Patchset overview: Patch 1-2:GUP helpers to migrate and unpin one or more folios Patch 3: Introduce memfd_pin_folios() API Patch 4-5:Udmabuf driver bug fixes for Qemu + hugetlb=on, blob=true case Patch 6-8:Convert Udmabuf to use memfd_pin_folios() and add selftests This series is tested using the following methods: - Run the subtests added in Patch 8 - Run Qemu (master) with the following options and a few additional patches to Spice: qemu-system-x86_64 -m 4096m -device virtio-gpu-pci,max_outputs=1,blob=true,xres=1920,yres=1080 -spice port=3001,gl=on,disable-ticketing=on,preferred-codec=gstreamer:h264 -object memory-backend-memfd,hugetlb=on,id=mem1,size=4096M -machine memory-backend=mem1 - Run source ./run_vmtests.sh -t gup_test -a to check GUP regressions Changelog: v14 -> v15: - Add an error check start < 0 in memfd_pin_folios() - Return an error in udmabuf driver if memfd_pin_folios() returns 0 These two checks fix the following issue identified by syzbot: https://syzkaller.appspot.com/bug?extid=40c7dad27267f61839d4 - Set memfd = NULL before dmabuf export to ensure that memfd is not closed twice on error. This fixes the following syzbot issue: https://syzkaller.appspot.com/bug?extid=b2cfdac9ae5278d4b621 v13 -> v14: - Drop the redundant comments before check_and_migrate_movable_pages() and refer to check_and_migrate_movable_folios() comments (David) - Use appropriate ksft_* functions for printing and KSFT_* codes for exit() in udmabuf selftest (Shuah) - Add Mike Kravetz's suggested-by tag in udmabuf selftest patch (Shuah) - Collect Ack and Rb tags from David v12 -> v13: (suggestions from David) - Drop the sanity checks in unpin_folio()/unpin_folios() due to unavailability of per folio anon-exclusive flag - Export unpin_folio()/unpin_folios() using EXPORT_SYMBOL_GPL instead of EXPORT_SYMBOL - Have check_and_migrate_movable_pages() just call check_and_migrate_movable_folios() instead of calling other helpers - Slightly improve the comments and commit messages v11 -> v12: - Rebased and tested on mm-unstable v10 -> v11: - Remove the version string from the patch subject (Andrew) - Move the changelog from the patches into the cover letter - Rearrange the patchset to have GUP patches at the beginning v9 -> v10: - Introduce and use unpin_folio(), unpin_folios() and check_and_migrate_movable_folios() helpers - Use a list to track the folios that need to be unpinned in udmabuf v8 -> v9: (suggestions from Matthew) - Drop the extern while declaring memfd_alloc_folio() - Fix memfd_alloc_folio() declaration to have it return struct folio * instead of struct page * when CONFIG_MEMFD_CREATE is not defined - Use folio_pfn() on the folio instead of page_to_pfn() on head page in udmabuf - Don't split the arguments to shmem_read_folio() on multiple lines in udmabuf v7 -> v8: (suggestions from David) - Have caller pass [start, end], max_folios instead of start, nr_pages - Replace offsets array with just offset into the first page - Add comments explaning the need for next_idx - Pin (and return) the folio (via FOLL_PIN) only once v6 -> v7: - Rename this API to memfd_pin_folios() and make it return folios and offsets instead of pages (David) - Don't continue processing the folios in the batch returned by filemap_get_folios_contig() if they do not have correct next_idx - Add the R-b tag from Christoph v5 -> v6: (suggestions from Christoph) - Rename this API to memfd_pin_user_pages() to make it clear that it is intended for memfds - Move the memfd page allocation helper from gup.c to memfd.c - Fix indentation errors in memfd_pin_user_pages() - For contiguous ranges of folios, use a helper such as filemap_get_folios_contig() to lookup the page cache in batches - Split the processing of hugetlb or shmem pages into helpers to simplify the code in udmabuf_create() v4 -> v5: (suggestions from David) - For hugetlb case, ensure that we only obtain head pages from the mapping by using __filemap_get_folio() instead of find_get_page_flags() - Handle -EEXIST when two or more potential users try to simultaneously add a huge page to the mapping by forcing them to retry on failure v3 -> v4: - Remove the local variable "page" and instead use 3 return statements in alloc_file_page() (David) - Add the R-b tag from David v2 -> v3: (suggestions from David)
Re: [PATCH 0/3] drm/panic: Fixes and graphical logo
Hi Geert, On Thu, 13 Jun 2024 11:48:15 +0200 Geert Uytterhoeven wrote: > > > > Has the drm-misc git repo moved? > > > > It moved to gitlab recently, the new url is > > g...@gitlab.freedesktop.org:drm/misc/kernel.git > > Time to tell Stephen... linux-next has been using that URL for some time. -- Cheers, Stephen Rothwell pgpVtS_J_n_oa.pgp Description: OpenPGP digital signature
Re: [PATCH v9 00/13] Make PCI's devres API more consistent
On Thu, Jun 13, 2024 at 01:50:13PM +0200, Philipp Stanner wrote: > Changes in v9: > - Remove forgotten dead code ('enabled' bit in struct pci_dev) in > patch No.8 ("Move pinned status bit...") > - Rework patch No.3: > - Change title from "Reimplement plural devres functions" > to "Add partial-BAR devres support". > - Drop excessive details about the general cleanup from the commit > message. Only motivate why this patch's new infrastructure is > necessary. > - Fix some minor spelling issues (s/pci/PCI ...) > > Changes in v8: > - Rebase the series on the already merged patches which were slightly > modified by Bjorn Helgaas. > - Reword the pci_intx() commit message so it clearly states it's about > reworking pci_intx(). > - Move the removal of find_pci_dr() from patch "Remove legacy > pcim_release()" to patch "Give pci_intx() its own devres callback" > since this later patch already removed all calls to that function. > - In patch "Give pci_intx() its own devres callback": use > pci_is_enabled() (and, thus, the enabled_cnt in struct pci_dev) > instead of a separate enabled field. (Bjorn) > > Changes in v7: > - Split the entire series in smaller, more atomic chunks / patches > (Bjorn) > - Remove functions (such as pcim_iomap_region_range()) that do not yet > have a user (Bjorn) > - Don't export interfaces publicly anymore, except for > pcim_iomap_range(), needed by vboxvideo (Bjorn) > - Mention the actual (vboxvideo) bug in "PCI: Warn users..." commit > (Bjorn) > - Drop docstring warnings on PCI-internal functions (Bjorn) > - Rework docstring warnings > - Fix spelling in a few places. Rewrapp paragraphs (Bjorn) > > Changes in v6: > - Restructure the cleanup in pcim_iomap_regions_request_all() so that > it doesn't trigger a (false positive) test robot warning. No > behavior change intended. (Dan Carpenter) > > Changes in v5: > - Add Hans's Reviewed-by to vboxvideo patch (Hans de Goede) > - Remove stable-kernel from CC in vboxvideo patch (Hans de Goede) > > Changes in v4: > - Rebase against linux-next > > Changes in v3: > - Use the term "PCI devres API" at some forgotten places. > - Fix more grammar errors in patch #3. > - Remove the comment advising to call (the outdated) pcim_intx() in pci.c > - Rename __pcim_request_region_range() flags-field "exclusive" to > "req_flags", since this is what the int actually represents. > - Remove the call to pcim_region_request() from patch #10. (Hans) > > Changes in v2: > - Make commit head lines congruent with PCI's style (Bjorn) > - Add missing error checks for devm_add_action(). (Andy) > - Repair the "Returns: " marks for docu generation (Andy) > - Initialize the addr_devres struct with memset(). (Andy) > - Make pcim_intx() a PCI-internal function so that new drivers won't > be encouraged to use the outdated pci_intx() mechanism. > (Andy / Philipp) > - Fix grammar and spelling (Bjorn) > - Be more precise on why pcim_iomap_table() is problematic (Bjorn) > - Provide the actual structs' and functions' names in the commit > messages (Bjorn) > - Remove redundant variable initializers (Andy) > - Regroup PM bitfield members in struct pci_dev (Andy) > - Make pcim_intx() visible only for the PCI subsystem so that new > drivers won't use this outdated API (Andy, Myself) > - Add a NOTE to pcim_iomap() to warn about this function being the one > exception that does just return NULL. > - Consistently use the term "PCI devres API"; also in Patch #10 (Bjorn) > > > ¡Hola! > > PCI's devres API suffers several weaknesses: > > 1. There are functions prefixed with pcim_. Those are always managed >counterparts to never-managed functions prefixed with pci_ – or so one >would like to think. There are some apparently unmanaged functions >(all region-request / release functions, and pci_intx()) which >suddenly become managed once the user has initialized the device with >pcim_enable_device() instead of pci_enable_device(). This "sometimes >yes, sometimes no" nature of those functions is confusing and >therefore bug-provoking. In fact, it has already caused a bug in DRM. >The last patch in this series fixes that bug. > 2. iomappings: Instead of giving each mapping its own callback, the >existing API uses a statically allocated struct tracking one mapping >per bar. This is not extensible. Especially, you can't create >_ranged_ managed mappings that way, which many drivers want. > 3. Managed request functions only exist as "plural versions" with a >bit-mask as a parameter. That's quite over-engineered considering >that each user only ever mapps one, maybe two bars. > > This series: > - add a set of new "singular" devres functions that use devres the way > its intended, with one callback per resource. > - deprecates the existing iomap-table mechanism. >
Re: [PATCH] fbdev: vesafb: Detect VGA compatibility from screen info's VESA attributes
Helge Deller writes: > On 6/13/24 11:02, Thomas Zimmermann wrote: >> Test the vesa_attributes field in struct screen_info for compatibility >> with VGA hardware. Vesafb currently tests bit 1 in screen_info's >> capabilities field, It sets the framebuffer address size and is >> unrelated to VGA. >> >> Section 4.4 of the Vesa VBE 2.0 specifications defines that bit 5 in >> the mode's attributes field signals VGA compatibility. The mode is >> compatible with VGA hardware if the bit is clear. In that case, the >> driver can access VGA state of the VBE's underlying hardware. The >> vesafb driver uses this feature to program the color LUT in palette >> modes. Without, colors might be incorrect. >> >> The problem got introduced in commit 89ec4c238e7a ("[PATCH] vesafb: Fix >> incorrect logo colors in x86_64"). It incorrectly stores the mode >> attributes in the screen_info's capabilities field and updates vesafb >> accordingly. Later, commit 5e8ddcbe8692 ("Video mode probing support for >> the new x86 setup code") fixed the screen_info, but did not update vesafb. >> Color output still tends to work, because bit 1 in capabilities is >> usually 0. >> >> Besides fixing the bug in vesafb, this commit introduces a helper that >> reads the correct bit from screen_info. > > Nice catch, Thomas! > > But do we really need this additional helper? > > >> >> Signed-off-by: Thomas Zimmermann >> Fixes: 5e8ddcbe8692 ("Video mode probing support for the new x86 setup code") >> Cc: # v2.6.23+ > >> --- >> drivers/video/fbdev/vesafb.c | 2 +- >> include/linux/screen_info.h | 5 + >> 2 files changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c >> index 8ab64ae4cad3e..5a161750a3aee 100644 >> --- a/drivers/video/fbdev/vesafb.c >> +++ b/drivers/video/fbdev/vesafb.c >> @@ -271,7 +271,7 @@ static int vesafb_probe(struct platform_device *dev) >> if (si->orig_video_isVGA != VIDEO_TYPE_VLFB) >> return -ENODEV; >> >> -vga_compat = (si->capabilities & 2) ? 0 : 1; >> +vga_compat = !__screen_info_vbe_mode_nonvga(si); > > Instead maybe just this: ? > + /* mode is VGA-compatible if BIT 5 is _NOT_ set */ > + vga_compat = (si->vesa_attributes & BIT(5)) == 0; > > I suggest to make patch small, esp. if you ask for backport to v2.6.23+. > I prefer the helper. It's a static inline anyways and having it as a function makes it much easier to read / understand. -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH v5] drm/ci: add tests on vkms
Hi, On Thu, Jun 13, 2024 at 01:56:10PM GMT, Vignesh Raman wrote: > On 13/06/24 13:07, Maxime Ripard wrote: > > Hi, > > > > On Tue, Jun 11, 2024 at 02:40:37PM GMT, Vignesh Raman wrote: > > > diff --git a/drivers/gpu/drm/ci/xfails/vkms-none-flakes.txt > > > b/drivers/gpu/drm/ci/xfails/vkms-none-flakes.txt > > > new file mode 100644 > > > index ..56484a30aff5 > > > --- /dev/null > > > +++ b/drivers/gpu/drm/ci/xfails/vkms-none-flakes.txt > > > @@ -0,0 +1,15 @@ > > > +# Board Name: vkms > > > +# Bug Report: > > > https://lore.kernel.org/dri-devel/61ed26af-062c-443c-9df2-d1ee319f3...@collabora.com/T/#u > > > +# Failure Rate: 50 > > > +# IGT Version: 1.28-g0df7b9b97 > > > +# Linux Version: 6.9.0-rc7 > > > +kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic > > > +kms_flip@basic-flip-vs-wf_vblank > > > +kms_flip@flip-vs-expired-vblank-interruptible > > > +kms_flip@flip-vs-wf_vblank-interruptible > > > +kms_flip@plain-flip-fb-recreate-interruptible > > > +kms_flip@plain-flip-ts-check > > > +kms_flip@plain-flip-ts-check-interruptible > > > +kms_flip@flip-vs-absolute-wf_vblank > > > +kms_flip@flip-vs-absolute-wf_vblank-interruptible > > > +kms_flip@flip-vs-blocking-wf-vblank > > > > We should have the header for every line here > > All the flakes in these tests were observed with version > 6.9.0-rc7/1.28-g0df7b9b97. So can we group them together? > > If we update this file for different IGT/kernel version, we can add a > separate header for each test. If we don't however, we have no way to tell in a couple months whether those flakes were there before we were adding those metadata, or if the metadata applies to everything, or only a subset. Maxime signature.asc Description: PGP signature
Re: [PATCH 09/15] net: hbl_en: add habanalabs Ethernet driver
> +static int hbl_en_napi_poll(struct napi_struct *napi, int budget); > +static int hbl_en_port_open(struct hbl_en_port *port); When you do the Intel internal review, i expect this is crop up. No forward declarations please. Put the code in the right order so they are not needed. > +static int hbl_en_get_src_ip(struct hbl_aux_dev *aux_dev, u32 port_idx, u32 > *src_ip) > +{ > + struct hbl_en_port *port = HBL_EN_PORT(aux_dev, port_idx); > + struct net_device *ndev = port->ndev; > + struct in_device *in_dev; > + struct in_ifaddr *ifa; > + int rc = 0; > + > + /* for the case where no src IP is configured */ > + *src_ip = 0; > + > + /* rtnl lock should be acquired in relevant flows before taking > configuration lock */ > + if (!rtnl_is_locked()) { > + netdev_err(port->ndev, "Rtnl lock is not acquired, can't > proceed\n"); > + rc = -EFAULT; > + goto out; > + } You will find all other drivers just do: ASSERT_RTNL(). If your locking is broken, you are probably dead anyway, so you might as well keep going and try to explode in the most interesting way possible. > +static void hbl_en_reset_stats(struct hbl_aux_dev *aux_dev, u32 port_idx) > +{ > + struct hbl_en_port *port = HBL_EN_PORT(aux_dev, port_idx); > + > + port->net_stats.rx_packets = 0; > + port->net_stats.tx_packets = 0; > + port->net_stats.rx_bytes = 0; > + port->net_stats.tx_bytes = 0; > + port->net_stats.tx_errors = 0; > + atomic64_set(&port->net_stats.rx_dropped, 0); > + atomic64_set(&port->net_stats.tx_dropped, 0); Why atomic64_set? Atomics are expensive, so you should not be using them. netdev has other cheaper methods, which other Intel developers should be happy to tell you all about. > +static u32 hbl_en_get_mtu(struct hbl_aux_dev *aux_dev, u32 port_idx) > +{ > + struct hbl_en_port *port = HBL_EN_PORT(aux_dev, port_idx); > + struct net_device *ndev = port->ndev; > + u32 mtu; > + > + if (atomic_cmpxchg(&port->in_reset, 0, 1)) { > + netdev_err(ndev, "port is in reset, can't get MTU\n"); > + return 0; > + } > + > + mtu = ndev->mtu; I think you need a better error message. All this does is access ndev->mtu. What does it matter if the port is in reset? You don't access it. > +static int hbl_en_close(struct net_device *netdev) > +{ > + struct hbl_en_port *port = hbl_netdev_priv(netdev); > + struct hbl_en_device *hdev = port->hdev; > + ktime_t timeout; > + > + /* Looks like the return value of this function is not checked, so we > can't just return > + * EBUSY if the port is under reset. We need to wait until the reset is > finished and then > + * close the port. Otherwise the netdev will set the port as closed > although port_close() > + * wasn't called. Only if we waited long enough and the reset hasn't > finished, we can return > + * an error without actually closing the port as it is a fatal flow > anyway. > + */ > + timeout = ktime_add_ms(ktime_get(), PORT_RESET_TIMEOUT_MSEC); > + while (atomic_cmpxchg(&port->in_reset, 0, 1)) { > + /* If this is called from unregister_netdev() then the port was > already closed and > + * hence we can safely return. > + * We could have just check the port_open boolean, but that > might hide some future > + * bugs. Hence it is better to use a dedicated flag for that. > + */ > + if (READ_ONCE(hdev->in_teardown)) > + return 0; > + > + usleep_range(50, 200); > + if (ktime_compare(ktime_get(), timeout) > 0) { > + netdev_crit(netdev, > + "Timeout while waiting for port to finish > reset, can't close it\n" > + ); > + return -EBUSY; > + } This has the usual bug. Please look at include/linux/iopoll.h. > + timeout = ktime_add_ms(ktime_get(), PORT_RESET_TIMEOUT_MSEC); > + while (atomic_cmpxchg(&port->in_reset, 0, 1)) { > + usleep_range(50, 200); > + if (ktime_compare(ktime_get(), timeout) > 0) { > + netdev_crit(port->ndev, > + "Timeout while waiting for port %d > to finish reset\n", > + port->idx); > + break; > + } > + } and again. Don't roll your own timeout loops like this, use the core version. > +static int hbl_en_change_mtu(struct net_device *netdev, int new_mtu) > +{ > + struct hbl_en_port *port = hbl_netdev_priv(netdev); > + int rc = 0; > + > + if (atomic_cmpxchg(&port->in_reset, 0, 1)) { > + netdev_err(netdev, "port is in reset, can't change MTU\n"); > + return -EBUSY; > + } > + > + i
Re: [PATCH v2 4/8] drm/msm/dpu: pull the is_cmd_mode out of _dpu_encoder_update_vsync_source()
On 2024-06-13 20:05:07, Dmitry Baryshkov wrote: > Setting vsync source makes sense only for DSI CMD panels. Pull the > is_cmd_mode condition out of the function into the calling code, so that > it becomes more explicit. > > Reviewed-by: Abhinav Kumar > Signed-off-by: Dmitry Baryshkov Reviewed-by: Marijn Suijten > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index 4988a1029431..bd37a56b4d03 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -736,8 +736,7 @@ static void _dpu_encoder_update_vsync_source(struct > dpu_encoder_virt *dpu_enc, > return; > } > > - if (hw_mdptop->ops.setup_vsync_source && > - disp_info->is_cmd_mode) { > + if (hw_mdptop->ops.setup_vsync_source) { > for (i = 0; i < dpu_enc->num_phys_encs; i++) > vsync_cfg.ppnumber[i] = dpu_enc->hw_pp[i]->idx; > > @@ -1226,7 +1225,8 @@ static void _dpu_encoder_virt_enable_helper(struct > drm_encoder *drm_enc) > dpu_enc->cur_master->hw_mdptop->ops.intf_audio_select( > dpu_enc->cur_master->hw_mdptop); > > - _dpu_encoder_update_vsync_source(dpu_enc, &dpu_enc->disp_info); > + if (dpu_enc->disp_info.is_cmd_mode) > + _dpu_encoder_update_vsync_source(dpu_enc, &dpu_enc->disp_info); > > if (dpu_enc->disp_info.intf_type == INTF_DSI && > !WARN_ON(dpu_enc->num_phys_encs == 0)) { > > -- > 2.39.2 >
Re: [PATCH 5/9] arm64: dts: rockchip: Enable the NPU on quartzpro64
Hi, On Wed, Jun 12, 2024 at 03:52:58PM GMT, Tomeu Vizoso wrote: > Enable the nodes added in a previous commit to the rk3588s device tree. > > Signed-off-by: Tomeu Vizoso > --- There is a separate regulator for the NPU. For QuartzPro64, which is basically the same as EVB1, it should look like this (obviously the "npu-supply" and "sram-supply" need to become part of the binding): &rknn { npu-supply = <&vdd_npu_s0>; sram-supply = <&vdd_npu_mem_s0>; }; Also the references are supposed to be done alphabetically in the DT file (so &rknn should not be added before &i2c). Greetings, -- Sebastian signature.asc Description: PGP signature
Re: [PATCH 2/9] iommu/rockchip: Attach multiple power domains
Hi, On Thu, Jun 13, 2024 at 11:34:02AM GMT, Tomeu Vizoso wrote: > On Thu, Jun 13, 2024 at 11:24 AM Tomeu Vizoso wrote: > > On Thu, Jun 13, 2024 at 2:05 AM Sebastian Reichel > > wrote: > > > On Wed, Jun 12, 2024 at 03:52:55PM GMT, Tomeu Vizoso wrote: > > > > IOMMUs with multiple base addresses can also have multiple power > > > > domains. > > > > > > > > The base framework only takes care of a single power domain, as some > > > > devices will need for these power domains to be powered on in a specific > > > > order. > > > > > > > > Use a helper function to stablish links in the order in which they are > > > > in the DT. > > > > > > > > This is needed by the IOMMU used by the NPU in the RK3588. > > > > > > > > Signed-off-by: Tomeu Vizoso > > > > --- > > > > > > To me it looks like this is multiple IOMMUs, which should each get > > > their own node. I don't see a good reason for merging these > > > together. > > > > I have made quite a few attempts at splitting the IOMMUs and also the > > cores, but I wasn't able to get things working stably. The TRM is > > really scant about how the 4 IOMMU instances relate to each other, and > > what the fourth one is for. > > > > Given that the vendor driver treats them as a single IOMMU with four > > instances and we don't have any information on them, I resigned myself > > to just have them as a single device. > > > > I would love to be proved wrong though and find a way fo getting > > things stably as different devices so they can be powered on and off > > as needed. We could save quite some code as well. > > FWIW, here a few ways how I tried to structure the DT nodes, none of > these worked reliably: > > https://gitlab.freedesktop.org/tomeu/linux/-/blob/6.10-rocket-multiple-devices-power/arch/arm64/boot/dts/rockchip/rk3588s.dtsi?ref_type=heads#L1163 > https://gitlab.freedesktop.org/tomeu/linux/-/blob/6.10-rocket-schema-subnodes//arch/arm64/boot/dts/rockchip/rk3588s.dtsi?ref_type=heads#L1162 > https://gitlab.freedesktop.org/tomeu/linux/-/blob/6.10-rocket-multiple-devices//arch/arm64/boot/dts/rockchip/rk3588s.dtsi?ref_type=heads#L1163 > https://gitlab.freedesktop.org/tomeu/linux/-/blob/6.10-rocket-multiple-iommus//arch/arm64/boot/dts/rockchip/rk3588s.dtsi?ref_type=heads#L2669 > > I can very well imagine I missed some way of getting this to work, but > for every attempt, the domains, iommus and cores were resumed in > different orders that presumably caused problems during concurrent > execution fo workloads. > > So I fell back to what the vendor driver does, which works reliably > (but all cores have to be powered on at the same time). Mh. The "6.10-rocket-multiple-iommus" branch seems wrong. There is only one iommu node in that. I would have expected a test with rknn { // combined device iommus = <&iommu1>, <&iommu2>, ...; }; Otherwise I think I would go with the schema-subnodes variant. The driver can initially walk through the sub-nodes and collect the resources into the main device, so on the driver side nothing would really change. But that has a couple of advantages: 1. DT and DT binding are easier to read 2. It's similar to e.g. CPU cores each having their own node 3. Easy to extend to more cores in the future 4. The kernel can easily switch to proper per-core device model when the problem has been identified -- Sebastian signature.asc Description: PGP signature
Re: [PATCH v2 3/8] drm/msm/dsi: drop unused GPIOs handling
On 2024-06-13 20:05:06, Dmitry Baryshkov wrote: > Neither disp-enable-gpios nor disp-te-gpios are defined in the schema. > None of the board DT files use those GPIO pins. Drop them from the > driver. What's worse, when people set disp-te-gpios the devm_gpiod_get_optional("disp-te", GPIOD_IN) below resets the typical mdp_vsync function via pinctrl to the IN function, causing vsync signals to be lost and the MDP hardware to fall back to half the requested refresh rate since commit da9e7b7696d8 ("drm/msm/dpu: Correctly configure vsync tearcheck for command mode"). > > Reviewed-by: Abhinav Kumar > Signed-off-by: Dmitry Baryshkov Reviewed-by: Marijn Suijten > --- > drivers/gpu/drm/msm/dsi/dsi_host.c | 37 - > 1 file changed, 37 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c > b/drivers/gpu/drm/msm/dsi/dsi_host.c > index a50f4dda5941..c4d72562c95a 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c > @@ -7,7 +7,6 @@ > #include > #include > #include > -#include > #include > #include > #include > @@ -130,9 +129,6 @@ struct msm_dsi_host { > > unsigned long src_clk_rate; > > - struct gpio_desc *disp_en_gpio; > - struct gpio_desc *te_gpio; > - > const struct msm_dsi_cfg_handler *cfg_hnd; > > struct completion dma_comp; > @@ -1613,28 +1609,6 @@ static irqreturn_t dsi_host_irq(int irq, void *ptr) > return IRQ_HANDLED; > } > > -static int dsi_host_init_panel_gpios(struct msm_dsi_host *msm_host, > - struct device *panel_device) > -{ > - msm_host->disp_en_gpio = devm_gpiod_get_optional(panel_device, > - "disp-enable", > - GPIOD_OUT_LOW); > - if (IS_ERR(msm_host->disp_en_gpio)) { > - DBG("cannot get disp-enable-gpios %ld", > - PTR_ERR(msm_host->disp_en_gpio)); > - return PTR_ERR(msm_host->disp_en_gpio); > - } > - > - msm_host->te_gpio = devm_gpiod_get_optional(panel_device, "disp-te", > - GPIOD_IN); > - if (IS_ERR(msm_host->te_gpio)) { > - DBG("cannot get disp-te-gpios %ld", PTR_ERR(msm_host->te_gpio)); > - return PTR_ERR(msm_host->te_gpio); > - } > - > - return 0; > -} > - > static int dsi_host_attach(struct mipi_dsi_host *host, > struct mipi_dsi_device *dsi) > { > @@ -1651,11 +1625,6 @@ static int dsi_host_attach(struct mipi_dsi_host *host, > if (dsi->dsc) > msm_host->dsc = dsi->dsc; > > - /* Some gpios defined in panel DT need to be controlled by host */ > - ret = dsi_host_init_panel_gpios(msm_host, &dsi->dev); > - if (ret) > - return ret; > - > ret = dsi_dev_attach(msm_host->pdev); > if (ret) > return ret; > @@ -2422,9 +2391,6 @@ int msm_dsi_host_power_on(struct mipi_dsi_host *host, > dsi_sw_reset(msm_host); > dsi_ctrl_enable(msm_host, phy_shared_timings, phy); > > - if (msm_host->disp_en_gpio) > - gpiod_set_value(msm_host->disp_en_gpio, 1); > - > msm_host->power_on = true; > mutex_unlock(&msm_host->dev_mutex); > > @@ -2454,9 +2420,6 @@ int msm_dsi_host_power_off(struct mipi_dsi_host *host) > > dsi_ctrl_disable(msm_host); > > - if (msm_host->disp_en_gpio) > - gpiod_set_value(msm_host->disp_en_gpio, 0); > - > pinctrl_pm_select_sleep_state(&msm_host->pdev->dev); > > cfg_hnd->ops->link_clk_disable(msm_host); > > -- > 2.39.2 >
Re: [PATCH] fbdev: vesafb: Detect VGA compatibility from screen info's VESA attributes
On 6/13/24 11:02, Thomas Zimmermann wrote: Test the vesa_attributes field in struct screen_info for compatibility with VGA hardware. Vesafb currently tests bit 1 in screen_info's capabilities field, It sets the framebuffer address size and is unrelated to VGA. Section 4.4 of the Vesa VBE 2.0 specifications defines that bit 5 in the mode's attributes field signals VGA compatibility. The mode is compatible with VGA hardware if the bit is clear. In that case, the driver can access VGA state of the VBE's underlying hardware. The vesafb driver uses this feature to program the color LUT in palette modes. Without, colors might be incorrect. The problem got introduced in commit 89ec4c238e7a ("[PATCH] vesafb: Fix incorrect logo colors in x86_64"). It incorrectly stores the mode attributes in the screen_info's capabilities field and updates vesafb accordingly. Later, commit 5e8ddcbe8692 ("Video mode probing support for the new x86 setup code") fixed the screen_info, but did not update vesafb. Color output still tends to work, because bit 1 in capabilities is usually 0. Besides fixing the bug in vesafb, this commit introduces a helper that reads the correct bit from screen_info. Nice catch, Thomas! But do we really need this additional helper? Signed-off-by: Thomas Zimmermann Fixes: 5e8ddcbe8692 ("Video mode probing support for the new x86 setup code") Cc: # v2.6.23+ --- drivers/video/fbdev/vesafb.c | 2 +- include/linux/screen_info.h | 5 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c index 8ab64ae4cad3e..5a161750a3aee 100644 --- a/drivers/video/fbdev/vesafb.c +++ b/drivers/video/fbdev/vesafb.c @@ -271,7 +271,7 @@ static int vesafb_probe(struct platform_device *dev) if (si->orig_video_isVGA != VIDEO_TYPE_VLFB) return -ENODEV; - vga_compat = (si->capabilities & 2) ? 0 : 1; + vga_compat = !__screen_info_vbe_mode_nonvga(si); Instead maybe just this: ? + /* mode is VGA-compatible if BIT 5 is _NOT_ set */ + vga_compat = (si->vesa_attributes & BIT(5)) == 0; I suggest to make patch small, esp. if you ask for backport to v2.6.23+. Helge vesafb_fix.smem_start = si->lfb_base; vesafb_defined.bits_per_pixel = si->lfb_depth; if (15 == vesafb_defined.bits_per_pixel) diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h index 75303c126285a..95f2a339de329 100644 --- a/include/linux/screen_info.h +++ b/include/linux/screen_info.h @@ -49,6 +49,11 @@ static inline u64 __screen_info_lfb_size(const struct screen_info *si, unsigned return lfb_size; } +static inline bool __screen_info_vbe_mode_nonvga(const struct screen_info *si) +{ + return si->vesa_attributes & BIT(5); // VGA if _not_ set +} + static inline unsigned int __screen_info_video_type(unsigned int type) { switch (type) {
Re: [PATCH v9 03/13] PCI: Add partial-BAR devres support
On Thu, Jun 13, 2024 at 01:50:16PM +0200, Philipp Stanner wrote: > With the current PCI devres API implementing a managed version of > pci_iomap_range() is impossible. > > Furthermore, the PCI devres API currently is inconsistent and > complicated. This is in large part due to the fact that there are hybrid > functions which are only sometimes managed via devres, and functions > IO-mapping and requesting several BARs at once and returning mappings > through a separately administrated table. > > This table's indexing mechanism does not support partial-BAR mappings. > > Another notable problem is that there are no separate managed > counterparts for region-request functions such as pci_request_region(), > as they exist for other PCI functions (e.g., pci_iomap() <-> > pcim_iomap()). Instead, functions based on __pci_request_region() change > their internal behavior and suddenly become managed functions when > pcim_enable_device() instead of pci_enable_device() is used. The hybrid thing is certainly a problem, but does this patch address it? I don't see that it does (other than adding comments in __pci_request_region() and pci_release_region()), but maybe I missed it. Correct me if I'm wrong, but I don't think this patch makes any user-visible changes. I'm proposing this: PCI: Add managed partial-BAR request and map infrastructure The pcim_iomap_devres table tracks entire-BAR mappings, so we can't use it to build a managed version of pci_iomap_range(), which maps partial BARs. Add struct pcim_addr_devres, which can track request and mapping of both entire BARs and partial BARs. Add the following internal devres functions based on struct pcim_addr_devres: pcim_iomap_region() # request & map entire BAR pcim_iounmap_region() # unmap & release entire BAR pcim_request_region() # request entire BAR pcim_release_region() # release entire BAR pcim_request_all_regions()# request all entire BARs pcim_release_all_regions()# release all entire BARs Rework the following public interfaces using the new infrastructure listed above: pcim_iomap() # map partial BAR pcim_iounmap()# unmap partial BAR pcim_iomap_regions() # request & map specified BARs pcim_iomap_regions_request_all() # request all BARs, map specified BARs pcim_iounmap_regions()# unmap & release specified BARs > This API is hard to understand and potentially bug-provoking. Hence, it > should be made more consistent. > > This patch adds the necessary infrastructure for partial-BAR mappings > managed with devres. That infrastructure also serves as a ground layer > for significantly simplifying the PCI devres API in subsequent patches > which can then cleanly separate managed and unmanaged API. > > When having the long term goal of providing always managed functions > prefixed with "pcim_" and never managed functions prefixed with "pci_" > and, thus, separating managed and unmanaged APIs cleanly, new PCI devres > infrastructure cannot use __pci_request_region() and its wrappers since > those would then again interact with PCI devres and, consequently, > prevent the managed nature from being removed from the pci_* functions > in the first place. Thus, it's necessary to provide an alternative to > __pci_request_region(). > > This patch addresses the following problems of the PCI devres API: > > a) There is no PCI devres infrastructure on which a managed counter > part to pci_iomap_range() could be based on. > > b) The vast majority of the users of plural functions such as > pcim_iomap_regions() only ever sets a single bit in the bit mask, > consequently making them singular functions anyways. > > c) region-request functions being sometimes managed and sometimes not > is bug-provoking. pcim_* functions should always be managed, pci_* > functions never. > > Add a new PCI device resource, pcim_addr_devres, that serves to > encapsulate all device resource types related to region requests and > IO-mappings since those are very frequently created together. > > Add a set of alternatives cleanly separated from the hybrid mechanism in > __pci_request_region() and its respective wrappers: > - __pcim_request_region_range() > - __pcim_release_region_range() > - __pcim_request_region() > - __pcim_release_region() > > Add the following PCI-internal devres functions based on the above: > - pcim_iomap_region() > - pcim_iounmap_region() > - _pcim_request_region() > - pcim_request_region() > - pcim_release_region() > - pcim_request_all_regions() > - pcim_release_all_regions() > > Add new needed helper pcim_remove_bar_from_legacy_table(). > > Rework the following public interfaces using the new infrastructure > listed above: > - pcim_iomap_release() > - pcim_iomap() > - pcim_iounmap() > - pcim_iomap_regions()
[PATCH v2 7/7] drm/panic: Add support for drawing a monochrome graphical logo
Re-use the existing support for boot-up logos to draw a monochrome graphical logo in the DRM panic handler. When no suitable graphical logo is available, the code falls back to the ASCII art penguin logo. Note that all graphical boot-up logos are freed during late kernel initialization, hence a copy must be made for later use. Signed-off-by: Geert Uytterhoeven --- v2: - Rebased, - Inline trivial draw_logo_mono(). --- drivers/gpu/drm/drm_panic.c | 65 + drivers/video/logo/Kconfig | 2 ++ 2 files changed, 60 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c index f7e22b69bb25d3be..af30f243b2802ad7 100644 --- a/drivers/gpu/drm/drm_panic.c +++ b/drivers/gpu/drm/drm_panic.c @@ -7,11 +7,15 @@ */ #include +#include #include #include #include +#include #include +#include #include +#include #include #include @@ -88,6 +92,42 @@ static const struct drm_panic_line logo_ascii[] = { PANIC_LINE(" \\___)=(___/"), }; +#ifdef CONFIG_LOGO +static const struct linux_logo *logo_mono; + +static int drm_panic_setup_logo(void) +{ + const struct linux_logo *logo = fb_find_logo(1); + const unsigned char *logo_data; + struct linux_logo *logo_dup; + + if (!logo || logo->type != LINUX_LOGO_MONO) + return 0; + + /* The logo is __init, so we must make a copy for later use */ + logo_data = kmemdup(logo->data, + size_mul(DIV_ROUND_UP(logo->width, BITS_PER_BYTE), logo->height), + GFP_KERNEL); + if (!logo_data) + return -ENOMEM; + + logo_dup = kmemdup(logo, sizeof(*logo), GFP_KERNEL); + if (!logo_dup) { + kfree(logo_data); + return -ENOMEM; + } + + logo_dup->data = logo_data; + logo_mono = logo_dup; + + return 0; +} + +device_initcall(drm_panic_setup_logo); +#else +#define logo_mono ((const struct linux_logo *)NULL) +#endif + /* * Color conversion */ @@ -452,15 +492,22 @@ static void draw_panic_static_user(struct drm_scanout_buffer *sb) u32 bg_color = convert_from_xrgb(CONFIG_DRM_PANIC_BACKGROUND_COLOR, sb->format->format); const struct font_desc *font = get_default_font(sb->width, sb->height, NULL, NULL); struct drm_rect r_screen, r_logo, r_msg; + unsigned int logo_width, logo_height; if (!font) return; r_screen = DRM_RECT_INIT(0, 0, sb->width, sb->height); - r_logo = DRM_RECT_INIT(0, 0, - get_max_line_len(logo_ascii, logo_ascii_lines) * font->width, - logo_ascii_lines * font->height); + if (logo_mono) { + logo_width = logo_mono->width; + logo_height = logo_mono->height; + } else { + logo_width = get_max_line_len(logo_ascii, logo_ascii_lines) * font->width; + logo_height = logo_ascii_lines * font->height; + } + + r_logo = DRM_RECT_INIT(0, 0, logo_width, logo_height); r_msg = DRM_RECT_INIT(0, 0, min(get_max_line_len(panic_msg, msg_lines) * font->width, sb->width), min(msg_lines * font->height, sb->height)); @@ -471,10 +518,14 @@ static void draw_panic_static_user(struct drm_scanout_buffer *sb) /* Fill with the background color, and draw text on top */ drm_panic_fill(sb, &r_screen, bg_color); - if ((r_msg.x1 >= drm_rect_width(&r_logo) || r_msg.y1 >= drm_rect_height(&r_logo)) && - drm_rect_width(&r_logo) <= sb->width && drm_rect_height(&r_logo) <= sb->height) { - draw_txt_rectangle(sb, font, logo_ascii, logo_ascii_lines, false, &r_logo, - fg_color); + if ((r_msg.x1 >= logo_width || r_msg.y1 >= logo_height) && + logo_width <= sb->width && logo_height <= sb->height) { + if (logo_mono) + drm_panic_blit(sb, &r_logo, logo_mono->data, DIV_ROUND_UP(logo_width, 8), + fg_color); + else + draw_txt_rectangle(sb, font, logo_ascii, logo_ascii_lines, false, &r_logo, + fg_color); } draw_txt_rectangle(sb, font, panic_msg, msg_lines, true, &r_msg, fg_color); } diff --git a/drivers/video/logo/Kconfig b/drivers/video/logo/Kconfig index b7d94d1dd1585a84..ce6bb753522d215d 100644 --- a/drivers/video/logo/Kconfig +++ b/drivers/video/logo/Kconfig @@ -8,6 +8,8 @@ menuconfig LOGO depends on FB_CORE || SGI_NEWPORT_CONSOLE help Enable and select frame buffer bootup logos. + Monochrome logos will also be used by the DRM panic handler, if + enabled. if LOGO -- 2.34.1
[PATCH 08/21] drm/nouveau/nvkm: move vgaarb code from drm
Signed-off-by: Ben Skeggs --- drivers/gpu/drm/nouveau/nouveau_vga.c | 27 .../gpu/drm/nouveau/nvkm/subdev/pci/base.c| 32 +++ 2 files changed, 32 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_vga.c b/drivers/gpu/drm/nouveau/nouveau_vga.c index 83496af605a1..53b332708061 100644 --- a/drivers/gpu/drm/nouveau/nouveau_vga.c +++ b/drivers/gpu/drm/nouveau/nouveau_vga.c @@ -1,5 +1,4 @@ // SPDX-License-Identifier: MIT -#include #include #include @@ -8,28 +7,6 @@ #include "nouveau_acpi.h" #include "nouveau_vga.h" -static unsigned int -nouveau_vga_set_decode(struct pci_dev *pdev, bool state) -{ - struct nouveau_drm *drm = pci_get_drvdata(pdev); - struct nvif_device *device = &drm->device; - - if (device->impl->family == NVIF_DEVICE_CURIE && - device->impl->chipset >= 0x4c) - nvif_wr32(device, 0x088060, state); - else - if (device->impl->chipset >= 0x40) - nvif_wr32(device, 0x088054, state); - else - nvif_wr32(device, 0x001854, state); - - if (state) - return VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM | - VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM; - else - return VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM; -} - static void nouveau_switcheroo_set_state(struct pci_dev *pdev, enum vga_switcheroo_state state) @@ -94,8 +71,6 @@ nouveau_vga_init(struct nouveau_drm *drm) return; pdev = to_pci_dev(dev->dev); - vga_client_register(pdev, nouveau_vga_set_decode); - /* don't register Thunderbolt eGPU with vga_switcheroo */ if (pci_is_thunderbolt_attached(pdev)) return; @@ -118,8 +93,6 @@ nouveau_vga_fini(struct nouveau_drm *drm) return; pdev = to_pci_dev(dev->dev); - vga_client_unregister(pdev); - if (pci_is_thunderbolt_attached(pdev)) return; diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/base.c index 5a0de45d36ce..e4737b89cb63 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/base.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/base.c @@ -27,6 +27,8 @@ #include #include +#include + void nvkm_pci_msi_rearm(struct nvkm_device *device) { @@ -62,6 +64,29 @@ nvkm_pci_mask(struct nvkm_pci *pci, u16 addr, u32 mask, u32 value) return data; } +#include "nouveau_drv.h" + +static unsigned int +nvkm_pci_vga_set_decode(struct pci_dev *pdev, bool state) +{ + struct nvkm_device *device = ((struct nouveau_drm *)pci_get_drvdata(pdev))->nvkm; + + if (device->card_type == NV_40 && + device->chipset >= 0x4c) + nvkm_wr32(device, 0x088060, state); + else + if (device->chipset >= 0x40) + nvkm_wr32(device, 0x088054, state); + else + nvkm_wr32(device, 0x001854, state); + + if (state) + return VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM | + VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM; + else + return VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM; +} + void nvkm_pci_rom_shadow(struct nvkm_pci *pci, bool shadow) { @@ -76,11 +101,16 @@ nvkm_pci_rom_shadow(struct nvkm_pci *pci, bool shadow) static int nvkm_pci_fini(struct nvkm_subdev *subdev, bool suspend) { + struct nvkm_device_pci *pdev = subdev->device->func->pci(subdev->device); struct nvkm_pci *pci = nvkm_pci(subdev); + if (!subdev->use.enabled) + return 0; + if (pci->agp.bridge) nvkm_agp_fini(pci); + vga_client_unregister(pdev->pdev); return 0; } @@ -111,6 +141,7 @@ nvkm_pci_oneinit(struct nvkm_subdev *subdev) static int nvkm_pci_init(struct nvkm_subdev *subdev) { + struct nvkm_device_pci *pdev = subdev->device->func->pci(subdev->device); struct nvkm_pci *pci = nvkm_pci(subdev); int ret; @@ -131,6 +162,7 @@ nvkm_pci_init(struct nvkm_subdev *subdev) if (pci->msi) pci->func->msi_rearm(pci); + vga_client_register(pdev->pdev, nvkm_pci_vga_set_decode); return 0; } -- 2.44.0
Re: [PATCH v2 5/8] drm/msm/dpu: rework vsync_source handling
Maybe retitle this to something that more closely resembles "remove unset is_te_using_watchdog_timer field"? On 2024-06-13 20:05:08, Dmitry Baryshkov wrote: > The struct msm_display_info has is_te_using_watchdog_timer field which > is neither used anywhere nor is flexible enough to specify different Well, it's "used", but not "set" (to anything other than the zero-initialized default). s/used/set? > sources. Replace it with the field specifying the vsync source using > enum dpu_vsync_source. > > Reviewed-by: Abhinav Kumar > Signed-off-by: Dmitry Baryshkov Patch itself is fine, just think the title could be clearer: Reviewed-by: Marijn Suijten > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 5 + > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 5 ++--- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 ++ > 3 files changed, 5 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index bd37a56b4d03..b147f8814a18 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -743,10 +743,7 @@ static void _dpu_encoder_update_vsync_source(struct > dpu_encoder_virt *dpu_enc, > vsync_cfg.pp_count = dpu_enc->num_phys_encs; > vsync_cfg.frame_rate = > drm_mode_vrefresh(&dpu_enc->base.crtc->state->adjusted_mode); > > - if (disp_info->is_te_using_watchdog_timer) > - vsync_cfg.vsync_source = DPU_VSYNC_SOURCE_WD_TIMER_0; > - else > - vsync_cfg.vsync_source = DPU_VSYNC_SOURCE_GPIO_0; > + vsync_cfg.vsync_source = disp_info->vsync_source; > > hw_mdptop->ops.setup_vsync_source(hw_mdptop, &vsync_cfg); > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > index 76be77e30954..cb59bd4436f4 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > @@ -26,15 +26,14 @@ > * @h_tile_instance:Controller instance used per tile. Number of > elements is > * based on num_of_h_tiles > * @is_cmd_mode Boolean to indicate if the CMD mode is requested > - * @is_te_using_watchdog_timer: Boolean to indicate watchdog TE is > - *used instead of panel TE in cmd mode panels > + * @vsync_source:Source of the TE signal for DSI CMD devices > */ > struct msm_display_info { > enum dpu_intf_type intf_type; > uint32_t num_of_h_tiles; > uint32_t h_tile_instance[MAX_H_TILES_PER_DISPLAY]; > bool is_cmd_mode; > - bool is_te_using_watchdog_timer; > + enum dpu_vsync_source vsync_source; > }; > > /** > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > index 1955848b1b78..e9991f3756d4 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > @@ -543,6 +543,8 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev, > > info.is_cmd_mode = msm_dsi_is_cmd_mode(priv->dsi[i]); > > + info.vsync_source = DPU_VSYNC_SOURCE_GPIO_0; > + > encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI, &info); > if (IS_ERR(encoder)) { > DPU_ERROR("encoder init failed for dsi display\n"); > > -- > 2.39.2 >
Re: [PATCH v7 03/13] PCI: Reimplement plural devres functions
On Thu, Jun 13, 2024 at 07:54:38PM +0300, Ilpo Järvinen wrote: > On Wed, 5 Jun 2024, Philipp Stanner wrote: > > > When the original PCI devres API was implemented, priority was given to > > the creation of a set of "plural functions" such as > > pcim_request_regions(). These functions have bit masks as parameters to > > specify which BARs shall get mapped. Most users, however, only use those > > to map 1-3 BARs. > > +static int __pcim_request_region_range(struct pci_dev *pdev, int bar, > > + unsigned long offset, unsigned long maxlen, > > + const char *name, int req_flags) > > +{ > > + resource_size_t start = pci_resource_start(pdev, bar); > > + resource_size_t len = pci_resource_len(pdev, bar); > > + unsigned long dev_flags = pci_resource_flags(pdev, bar); > > + > > + if (start == 0 || len == 0) /* That's an unused BAR. */ > > + return 0; > > + if (len <= offset) > > + return -EINVAL; > > Extra space. Thanks for reviewing this. I dropped the space locally in the v9 series. > > void pcim_iounmap_regions(struct pci_dev *pdev, int mask) > > { > > - void __iomem * const *iomap; > > - int i; > > - > > - iomap = pcim_iomap_table(pdev); > > - if (!iomap) > > - return; > > + short bar; > > The current best practice is to use unsigned for loop vars that will never > be negative. > > I don't entirely follow what is reasoning behind making it short instead > of unsigned int? Existing interfaces like pcim_iomap() take "int bar". I locally changed all the BAR indices to "int". We can make everything unsigned later if worthwhile. > > - for (i = 0; i < PCIM_IOMAP_MAX; i++) { > > - if (!mask_contains_bar(mask, i)) > > + for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) { > > Is this change minimal if it contains variable renames like this? > Was "i" not "bar" even if it was given as a parameter to > mask_contains_bar()? Replaced locally with "i". Bjorn
[RFC] GPU driver with separate "core" and "DRM" modules
NVIDIA has been exploring ways to better support the effort for an upstream kernel mode driver for GPUs that are capable of running GSP-RM firmware, since the introduction[1] to Nova. Use cases have been identified for which separating the core GPU programming out of the full DRM driver stack is a strong requirement from our key customers. An upstreamed NVIDIA GPU driver should be able to support current and emerging customer use cases for vGPU hosts. NVIDIA's vGPU deployments to date do not support compute or graphics functionality within the hypervisor host, and have no dependency on the Linux graphics subsystem, instead implementing the minimal functionality required to run vGPU guest VMs. For security-sensitive environments such as cloud infrastructure, it's important to continue support for running a minimal footprint vGPU host driver in a stripped-down / barebones kernel environment. This can be achieved by supporting both VFIO and DRM drivers as clients of a core driver, without requiring a full-fledged DRM driver (or the DRM subsystem itself) to be built into the host kernel. A core driver would be responsible for booting and communicating with GSP-RM, enumeration of HW configuration, shared/partitioned resource management, exception handling, and event dispatch. The DRM driver would do all the standard things a DRM driver does, and implement GPU memory management (TTM/HMM), KMS, command submission etc, as well as providing UAPI for userspace clients. These features would be implemented using HW resources allocated from a core driver, rather than the DRM driver being directly responsible for HW programming. As Nouveau's KMD is already split (in the logical sense) along similar lines, we're using it here for the purposes of this RFC to demonstrate the feasibility of such an architecture, and open it up for discussion. A link[2] to a tree containing the patches is below. [1] https://lore.kernel.org/all/3ed356488c9b0ca93845501425d427309f4cf616.ca...@redhat.com/ [2] https://gitlab.freedesktop.org/bskeggs/nouveau/-/tree/00.03-module *** BLURB HERE *** Ben Skeggs (2): drm/nouveau/nvkm: export symbols needed by the drm driver drm/nouveau/nvkm: separate out into nvkm.ko drivers/gpu/drm/nouveau/Kbuild | 4 ++-- drivers/gpu/drm/nouveau/include/nvkm/core/module.h | 3 --- drivers/gpu/drm/nouveau/nouveau_drm.c | 10 +- drivers/gpu/drm/nouveau/nvkm/core/driver.c | 1 + drivers/gpu/drm/nouveau/nvkm/core/gpuobj.c | 2 ++ drivers/gpu/drm/nouveau/nvkm/core/mm.c | 4 drivers/gpu/drm/nouveau/nvkm/device/acpi.c | 1 + drivers/gpu/drm/nouveau/nvkm/engine/gr/base.c | 1 + drivers/gpu/drm/nouveau/nvkm/module.c | 8 ++-- drivers/gpu/drm/nouveau/nvkm/subdev/bios/init.c | 1 + drivers/gpu/drm/nouveau/nvkm/subdev/bios/pll.c | 1 + drivers/gpu/drm/nouveau/nvkm/subdev/fb/base.c | 3 +++ drivers/gpu/drm/nouveau/nvkm/subdev/gpio/base.c | 3 +++ drivers/gpu/drm/nouveau/nvkm/subdev/i2c/base.c | 2 ++ drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c | 1 + drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c | 1 + drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c| 1 + drivers/gpu/drm/nouveau/nvkm/subdev/therm/fan.c | 1 + drivers/gpu/drm/nouveau/nvkm/subdev/volt/base.c | 1 + 19 files changed, 33 insertions(+), 16 deletions(-) -- 2.44.0
Re: [PATCH v2 05/10] mm/util: Fix possible race condition in kstrdup()
On Thu, 13 Jun 2024 10:30:39 +0800 Yafang Shao wrote: > In kstrdup(), it is critical to ensure that the dest string is always > NUL-terminated. However, potential race condidtion can occur between a > writer and a reader. > > Consider the following scenario involving task->comm: > > readerwriter > > len = strlen(s) + 1; > strlcpy(tsk->comm, buf, sizeof(tsk->comm)); > memcpy(buf, s, len); > > In this case, there is a race condition between the reader and the > writer. The reader calculate the length of the string `s` based on the > old value of task->comm. However, during the memcpy(), the string `s` > might be updated by the writer to a new value of task->comm. > > If the new task->comm is larger than the old one, the `buf` might not be > NUL-terminated. This can lead to undefined behavior and potential > security vulnerabilities. > > Let's fix it by explicitly adding a NUL-terminator. The concept sounds a little strange. If some code takes a copy of a string while some other code is altering it, yes, the result will be a mess. This is why get_task_comm() exists, and why it uses locking. I get that "your copy is a mess" is less serious than "your string isn't null-terminated" but still. Whichever outcome we get, the calling code is buggy and should be fixed. Are there any other problematic scenarios we're defending against here? > > --- a/mm/util.c > +++ b/mm/util.c > @@ -60,8 +60,10 @@ char *kstrdup(const char *s, gfp_t gfp) > > len = strlen(s) + 1; > buf = kmalloc_track_caller(len, gfp); > - if (buf) > + if (buf) { > memcpy(buf, s, len); > + buf[len - 1] = '\0'; > + } > return buf; > } Now I'll start receiving patches to remove this again. Let's have a code comment please. And kstrdup() is now looking awfully similar to kstrndup(). Perhaps there's a way to reduce duplication?