Re: [PATCH] drm/amd/pm: Remove unnecessary conversion to bool
[AMD Official Use Only - Internal Distribution Only] if possible, please correct the same issue in navi10_ppt.c Reviewed-by: Kevin Wang Best Regards, Kevin From: Jiapeng Chong Sent: Sunday, February 7, 2021 4:49 PM To: Deucher, Alexander Cc: Koenig, Christian ; airl...@linux.ie ; dan...@ffwll.ch ; amd-...@lists.freedesktop.org ; dri-devel@lists.freedesktop.org ; linux-ker...@vger.kernel.org ; Jiapeng Chong Subject: [PATCH] drm/amd/pm: Remove unnecessary conversion to bool Fix the following coccicheck warning: ./drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c:907:47-52: WARNING: conversion to bool not needed here. Reported-by: Abaci Robot Signed-off-by: Jiapeng Chong --- drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c index d68d3df..b364862 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c @@ -904,7 +904,7 @@ static bool sienna_cichlid_is_support_fine_grained_dpm(struct smu_context *smu, dpm_desc = &pptable->DpmDescriptor[clk_index]; /* 0 - Fine grained DPM, 1 - Discrete DPM */ - return dpm_desc->SnapToDiscrete == 0 ? true : false; + return dpm_desc->SnapToDiscrete != 0; } static int sienna_cichlid_print_clk_levels(struct smu_context *smu, -- 1.8.3.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm/dsi: replace spin_lock_irqsave by spin_lock in hard IRQ
The code has been in a irq-disabled context since it is hard IRQ. There is no necessity to do it again. Signed-off-by: Tian Tao --- drivers/gpu/drm/msm/dsi/dsi_host.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index ab281cb..8ba204d 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -1555,15 +1555,14 @@ static irqreturn_t dsi_host_irq(int irq, void *ptr) { struct msm_dsi_host *msm_host = ptr; u32 isr; - unsigned long flags; if (!msm_host->ctrl_base) return IRQ_HANDLED; - spin_lock_irqsave(&msm_host->intr_lock, flags); + spin_lock(&msm_host->intr_lock); isr = dsi_read(msm_host, REG_DSI_INTR_CTRL); dsi_write(msm_host, REG_DSI_INTR_CTRL, isr); - spin_unlock_irqrestore(&msm_host->intr_lock, flags); + spin_unlock(&msm_host->intr_lock); DBG("isr=0x%x, id=%d", isr, msm_host->id); -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd/pm: Remove unnecessary conversion to bool
Fix the following coccicheck warning: ./drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c:907:47-52: WARNING: conversion to bool not needed here. Reported-by: Abaci Robot Signed-off-by: Jiapeng Chong --- drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c index d68d3df..b364862 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c @@ -904,7 +904,7 @@ static bool sienna_cichlid_is_support_fine_grained_dpm(struct smu_context *smu, dpm_desc = &pptable->DpmDescriptor[clk_index]; /* 0 - Fine grained DPM, 1 - Discrete DPM */ - return dpm_desc->SnapToDiscrete == 0 ? true : false; + return dpm_desc->SnapToDiscrete != 0; } static int sienna_cichlid_print_clk_levels(struct smu_context *smu, -- 1.8.3.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/2] dmabuf: Add dmabuf inode number to /proc/*/fdinfo
On Fri, Feb 05, 2021 at 02:23:20AM +, Kalesh Singh wrote: > +DMA Buffer files > + > + > +:: > + > + pos:0 > + flags: 04002 > + mnt_id: 9 > + dmabuf_inode_no: 63107 inode_nr would be better than inode_no. But even better would be to match stat(2) and just call it 'ino'. > + size: 32768 > + count: 2 > + exp_name: system-heap ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: bridge: convert sysfs sprintf/snprintf family to sysfs_emit
Fix the following coccicheck warning: drivers/gpu/drm/bridge/lontium-lt9611uxc.c:858:8-16: WARNING: use scnprintf or sprintf. Reported-by: Abaci Robot Signed-off-by: Jiapeng Chong --- drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c index fee2795..3cac16d 100644 --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c @@ -855,7 +855,7 @@ static ssize_t lt9611uxc_firmware_show(struct device *dev, struct device_attribu { struct lt9611uxc *lt9611uxc = dev_get_drvdata(dev); - return snprintf(buf, PAGE_SIZE, "%02x\n", lt9611uxc->fw_version); + return sysfs_emit(buf, "%02x\n", lt9611uxc->fw_version); } static DEVICE_ATTR_RW(lt9611uxc_firmware); -- 1.8.3.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] I was wondering why I can't set the resolution to 2560x1080, while in windows 7 I can without a problem. I looked at the radeon driver code and found it doesn't support this resolution. So I m
--- drivers/gpu/drm/radeon/radeon_benchmark.c | 5 ++-- drivers/gpu/drm/radeon/radeon_connectors.c | 30 ++ drivers/gpu/drm/radeon/radeon_drv.c| 5 drivers/gpu/drm/radeon/radeon_encoders.c | 6 +++-- 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_benchmark.c b/drivers/gpu/drm/radeon/radeon_benchmark.c index ac9a5ec481c3..c283b6b15925 100644 --- a/drivers/gpu/drm/radeon/radeon_benchmark.c +++ b/drivers/gpu/drm/radeon/radeon_benchmark.c @@ -30,7 +30,7 @@ #define RADEON_BENCHMARK_COPY_DMA 0 #define RADEON_BENCHMARK_ITERATIONS 1024 -#define RADEON_BENCHMARK_COMMON_MODES_N 17 +#define RADEON_BENCHMARK_COMMON_MODES_N 18 static int radeon_benchmark_do_move(struct radeon_device *rdev, unsigned size, uint64_t saddr, uint64_t daddr, @@ -184,7 +184,8 @@ void radeon_benchmark(struct radeon_device *rdev, int test_number) 1680 * 1050 * 4, 1600 * 1200 * 4, 1920 * 1080 * 4, - 1920 * 1200 * 4 + 1920 * 1200 * 4, + 2560 * 1080 * 4 }; switch (test_number) { diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c index 607ad5620bd9..37927222f5b3 100644 --- a/drivers/gpu/drm/radeon/radeon_connectors.c +++ b/drivers/gpu/drm/radeon/radeon_connectors.c @@ -37,6 +37,8 @@ #include #include +extern int hdmimhz; + static int radeon_dp_handle_hpd(struct drm_connector *connector) { struct radeon_connector *radeon_connector = to_radeon_connector(connector); @@ -503,7 +505,7 @@ static void radeon_add_common_modes(struct drm_encoder *encoder, struct drm_conn struct mode_size { int w; int h; - } common_modes[17] = { + } common_modes[18] = { { 640, 480}, { 720, 480}, { 800, 600}, @@ -520,10 +522,11 @@ static void radeon_add_common_modes(struct drm_encoder *encoder, struct drm_conn {1680, 1050}, {1600, 1200}, {1920, 1080}, - {1920, 1200} + {1920, 1200}, + {2560, 1080} }; - for (i = 0; i < 17; i++) { + for (i = 0; i < 18; i++) { if (radeon_encoder->devices & (ATOM_DEVICE_TV_SUPPORT)) { if (common_modes[i].w > 1024 || common_modes[i].h > 768) @@ -1491,25 +1494,32 @@ static enum drm_mode_status radeon_dvi_mode_valid(struct drm_connector *connecto (mode->clock > 135000)) return MODE_CLOCK_HIGH; - if (radeon_connector->use_digital && (mode->clock > 165000)) { + if (radeon_connector->use_digital && (mode->clock > (hdmimhz * 1000))) { if ((radeon_connector->connector_object_id == CONNECTOR_OBJECT_ID_DUAL_LINK_DVI_I) || (radeon_connector->connector_object_id == CONNECTOR_OBJECT_ID_DUAL_LINK_DVI_D) || - (radeon_connector->connector_object_id == CONNECTOR_OBJECT_ID_HDMI_TYPE_B)) + (radeon_connector->connector_object_id == CONNECTOR_OBJECT_ID_HDMI_TYPE_B)){ + printk("MODE_CLOCK_HIHG0 %d", hdmimhz); return MODE_OK; - else if (ASIC_IS_DCE6(rdev) && drm_detect_hdmi_monitor(radeon_connector_edid(connector))) { + }else if (ASIC_IS_DCE6(rdev) && drm_detect_hdmi_monitor(radeon_connector_edid(connector))) { /* HDMI 1.3+ supports max clock of 340 Mhz */ - if (mode->clock > 34) + if (mode->clock > 34){ + printk("MODE_CLOCK_HIHG1 %d", hdmimhz); return MODE_CLOCK_HIGH; - else + }else{ + printk("MODE_OK1"); return MODE_OK; + } } else { + printk("MODE_CLOCK_HIHG2 %d", hdmimhz); return MODE_CLOCK_HIGH; } } /* check against the max pixel clock */ - if ((mode->clock / 10) > rdev->clock.max_pixel_clock) + if ((mode->clock / 10) > rdev->clock.max_pixel_clock){ + printk("MODE_CLOCK_HIHG3 %d", hdmimhz); return MODE_CLOCK_HIGH; + } return MODE_OK; } @@ -1809,7 +1819,7 @@ static enum drm_mode_status radeon_dp_mode_valid(struct drm_connector *connector if (mode->clock > 34) return MODE_CLOCK_HIGH; } else { - if (mode->clock > 165000) + if (mode->clock > (hdmimhz * 1000)) return MODE_CLOCK_HIGH;
[PATCH 0/3] Add check for max clock rate in mode_valid
Jitao Shi (3): drm/mediatek: mtk_dpi: Add check for max clock rate in mode_valid drm/mediatek: mtk_dpi: Add dpi config for mt8192 dt-bindings: mediatek,dpi: add mt8192 to mediatek,dpi .../display/mediatek/mediatek,dpi.yaml| 1 + drivers/gpu/drm/mediatek/mtk_dpi.c| 27 +++ 2 files changed, 28 insertions(+) -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] dt-bindings: mediatek,dpi: add mt8192 to mediatek,dpi
Add compatible "mediatek,mt8192-dpi" for the mt8192 dpi. Signed-off-by: Jitao Shi --- .../devicetree/bindings/display/mediatek/mediatek,dpi.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml index 6cdb734c91a9..2f566f19e6e0 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml @@ -22,6 +22,7 @@ properties: - mediatek,mt7623-dpi - mediatek,mt8173-dpi - mediatek,mt8183-dpi + - mediatek,mt8192-dpi reg: maxItems: 1 -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] drm/mediatek: mtk_dpi: Add dpi config for mt8192
Signed-off-by: Jitao Shi --- drivers/gpu/drm/mediatek/mtk_dpi.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c index ffa4a0f1989f..b7905f3f4d1b 100644 --- a/drivers/gpu/drm/mediatek/mtk_dpi.c +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c @@ -703,6 +703,13 @@ static const struct mtk_dpi_conf mt8183_conf = { .max_clock_khz = 10, }; +static const struct mtk_dpi_conf mt8192_conf = { + .cal_factor = mt8183_calculate_factor, + .reg_h_fre_con = 0xe0, + .dual_edge = true, + .max_clock_khz = 15, +}; + static int mtk_dpi_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -837,6 +844,9 @@ static const struct of_device_id mtk_dpi_of_ids[] = { { .compatible = "mediatek,mt8183-dpi", .data = &mt8183_conf, }, + { .compatible = "mediatek,mt8192-dpi", + .data = &mt8192_conf, + }, { }, }; -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] drm/mediatek: mtk_dpi: Add check for max clock rate in mode_valid
Add per-platform max clock rate check in mtk_dpi_bridge_mode_valid. Signed-off-by: Jitao Shi --- drivers/gpu/drm/mediatek/mtk_dpi.c | 17 + 1 file changed, 17 insertions(+) diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c index 52f11a63a330..ffa4a0f1989f 100644 --- a/drivers/gpu/drm/mediatek/mtk_dpi.c +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c @@ -118,6 +118,7 @@ struct mtk_dpi_yc_limit { struct mtk_dpi_conf { unsigned int (*cal_factor)(int clock); u32 reg_h_fre_con; + u32 max_clock_khz; bool edge_sel_en; }; @@ -555,9 +556,22 @@ static void mtk_dpi_bridge_enable(struct drm_bridge *bridge) mtk_dpi_set_display_mode(dpi, &dpi->mode); } +static enum drm_mode_status +mtk_dpi_bridge_mode_valid(struct drm_bridge *bridge, + const struct drm_display_mode *mode) +{ + struct mtk_dpi *dpi = bridge_to_dpi(bridge); + + if (dpi->conf->max_clock_khz && mode->clock > dpi->conf->max_clock_khz) + return MODE_CLOCK_HIGH; + + return MODE_OK; +} + static const struct drm_bridge_funcs mtk_dpi_bridge_funcs = { .attach = mtk_dpi_bridge_attach, .mode_set = mtk_dpi_bridge_mode_set, + .mode_valid = mtk_dpi_bridge_mode_valid, .disable = mtk_dpi_bridge_disable, .enable = mtk_dpi_bridge_enable, }; @@ -673,17 +687,20 @@ static unsigned int mt8183_calculate_factor(int clock) static const struct mtk_dpi_conf mt8173_conf = { .cal_factor = mt8173_calculate_factor, .reg_h_fre_con = 0xe0, + .max_clock_khz = 30, }; static const struct mtk_dpi_conf mt2701_conf = { .cal_factor = mt2701_calculate_factor, .reg_h_fre_con = 0xb0, .edge_sel_en = true, + .max_clock_khz = 15, }; static const struct mtk_dpi_conf mt8183_conf = { .cal_factor = mt8183_calculate_factor, .reg_h_fre_con = 0xe0, + .max_clock_khz = 10, }; static int mtk_dpi_probe(struct platform_device *pdev) -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] I was wondering why I can't set the resolution to 2560x1080, while in windows 7 I can without a problem. I looked at the radeon driver code and found it doesn't support this resolution. So
Please keep the commit message short. You probbly want to send this patch to amd-...@lists.freedesktop.org instead of dri-devel. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/lima: add governor data with pre-defined thresholds
Applied to drm-misc-next. Regards, Qiang On Tue, Feb 2, 2021 at 9:04 AM Qiang Yu wrote: > > OK, I see. Patch is also: > Reviewed-by: Qiang Yu > > Regards, > Qiang > > On Mon, Feb 1, 2021 at 5:59 PM Lukasz Luba wrote: > > > > > > > > On 1/30/21 1:57 PM, Qiang Yu wrote: > > > This patch gets minor improvement on glmark2 (160->162). > > > > It has bigger impact when the load is changing and the frequency > > is stuck to min w/o this patch. > > > > > > > > Seems there's no way for user to change this value, do we? > > > Or there's work pending to expose it to sysfs? > > > > True there is no user sysfs. I've proposed a patch to export these via > > sysfs. Chanwoo is going to work on it. When it will land mainline, it's > > probably a few months. So for now, the fix makes sense. > > > > Regards, > > Lukasz > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/lima: Use delayed timer as default in devfreq profile
Applied to drm-misc-next. Regards, Qiang On Thu, Feb 4, 2021 at 10:24 PM Lukasz Luba wrote: > > > > On 2/4/21 1:39 PM, Robin Murphy wrote: > > On 2021-02-03 02:01, Qiang Yu wrote: > >> On Tue, Feb 2, 2021 at 10:02 PM Lukasz Luba wrote: > >>> > >>> > >>> > >>> On 2/2/21 1:01 AM, Qiang Yu wrote: > Hi Lukasz, > > Thanks for the explanation. So the deferred timer option makes a > mistake that > when GPU goes from idle to busy for only one poll periodic, in this > case 50ms, right? > >>> > >>> Not exactly. Driver sets the polling interval to 50ms (in this case) > >>> because it needs ~3-frame average load (in 60fps). I have discovered the > >>> issue quite recently that on systems with 2 CPUs or more, the devfreq > >>> core is not monitoring the devices even for seconds. Therefore, we might > >>> end up with quite big amount of work that GPU is doing, but we don't > >>> know about it. Devfreq core didn't check <- timer didn't fired. Then > >>> suddenly that CPU, which had the deferred timer registered last time, > >>> is waking up and timer triggers to check our device. We get the stats, > >>> but they might be showing load from 1sec not 50ms. We feed them into > >>> governor. Governor sees the new load, but was tested and configured for > >>> 50ms, so it might try to rise the frequency to max. The GPU work might > >>> be already lower and there is no need for such freq. Then the CPU goes > >>> idle again, so no devfreq core check for next e.g. 1sec, but the > >>> frequency stays at max OPP and we burn power. > >>> > >>> So, it's completely unreliable. We might stuck at min frequency and > >>> suffer the frame drops, or sometimes stuck to max freq and burn more > >>> power when there is no such need. > >>> > >>> Similar for thermal governor, which is confused by this old stats and > >>> long period stats, longer than 50ms. > >>> > >>> Stats from last e.g. ~1sec tells you nothing about real recent GPU > >>> workload. > >> Oh, right, I missed this case. > >> > >>> > But delayed timer will wakeup CPU every 50ms even when system is > idle, will this > cause more power consumption for the case like phone suspend? > >>> > >>> No, in case of phone suspend it won't increase the power consumption. > >>> The device won't be woken up, it will stay in suspend. > >> I mean the CPU is waked up frequently by timer when phone suspend, > >> not the whole device (like the display). > >> > >> Seems it's better to have deferred timer when device is suspended for > >> power saving, > >> and delayed timer when device in working state. User knows this and > >> can use sysfs > >> to change it. > > > > Doesn't devfreq_suspend_device() already cancel any timer work either > > way in that case? > > Correct, the governor should pause the monitoring mechanism (and timer). > > Regards, > Lukasz ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[pull] drm/msm: msm-next for 5.12
Hi Dave, This time around: * a6xx speedbin support * a508, a509, a512 support * various a5xx fixes * various dpu fixes * qseed3lite support for sm8250 * dsi fix for msm8994 * mdp5 fix for framerate bug with cmd mode panels * a6xx GMU OOB race fixes that were showing up in CI * various addition and removal of semicolons * gem submit fix for legacy userspace relocs path The following changes since commit 6ee1d745b7c9fd573fba142a2efdad76a9f1cb04: Linux 5.11-rc5 (2021-01-24 16:47:14 -0800) are available in the Git repository at: https://gitlab.freedesktop.org/drm/msm.git tags/drm-msm-next-2021-02-07 for you to fetch changes up to 182b4a2d251305201b6f9cae29067f7112f05835: drm/msm/dp: Add a missing semi-colon (2021-02-07 09:57:04 -0800) Akhil P Oommen (1): drm/msm: Add speed-bin support to a618 gpu AngeloGioacchino Del Regno (16): drm/msm/a5xx: Allow all patchid for A540 chip drm/msm/a5xx: Remove overwriting A5XX_PC_DBG_ECO_CNTL register drm/msm/a5xx: Separate A5XX_PC_DBG_ECO_CNTL write from main branch drm/msm/a5xx: Add support for Adreno 508, 509, 512 GPUs drm/msm/a5xx: Reset VBIF before PC only on A510 and A530 drm/msm/dpu: Fix VBIF_XINL_QOS_LVL_REMAP_000 register offset drm/msm/dpu: Move DPU_SSPP_QOS_8LVL bit to SDM845 and SC7180 masks drm/msm/dpu: Add prog_fetch_lines_worst_case to INTF_BLK macro drm/msm/dpu: Allow specifying features and sblk in DSPP_BLK macro drm/msm/dpu: Disable autorefresh in command mode drm/msm/dpu: Correctly configure vsync tearcheck for command mode drm/msm/dpu: Remove unused call in wait_for_commit_done drm/msm/dsi_pll_10nm: Fix dividing the same numbers twice drm/msm/dsi_pll_10nm: Solve TODO for multiplier frac_bits assignment drm/msm/dsi_pll_10nm: Fix variable usage for pll_lockdet_rate drm/msm/dsi_pll_10nm: Convert pr_err prints to DRM_DEV_ERROR Bernard Zhao (1): drm/msm: remove unneeded variable: "rc" Dmitry Baryshkov (1): drm/msm/dpu1: add support for qseed3lite used on sm8250 Eric Anholt (3): drm/msm: Fix race of GPU init vs timestamp power management. drm/msm: Fix races managing the OOB state for timestamp vs timestamps. drm/msm: Clean up GMU OOB set/clear handling. Iskren Chernev (2): drm/msm: Fix MSM_INFO_GET_IOVA with carveout drm/msm/mdp5: Fix wait-for-commit for cmd panels Jiapeng Zhong (1): drm/msm: remove redundant NULL check Judy Hsiao (1): drm/msm/dp: trigger unplug event in msm_dp_display_disable Konrad Dybcio (5): drm/msm/a5xx: Fix VPC protect value in gpu_write() drm/msm/a5xx: Disable flat shading optimization drm/msm/a5xx: Disable UCHE global filter drm/msm/dsi: Correct io_start for MSM8994 (20nm PHY) drm/msm/disp/mdp5: mdp5_cfg: Fix msm8974v2 max_clk Kuogee Hsieh (2): drm/msm/dp: unplug interrupt missed after irq_hpd handler drm/msm/dp: reset dp controller only at boot up and pm_resume Rob Clark (1): drm/msm: Fix legacy relocs path Sai Prakash Ranjan (2): drm/msm: Add proper checks for GPU LLCC support drm/msm/a6xx: Create an A6XX GPU specific address space Stephen Boyd (2): drm/msm/kms: Make a lock_class_key for each crtc mutex drm/msm/dp: Add a missing semi-colon Xu Wang (2): drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c: Remove unneeded semicolon drm/msm/dp/dp_ctrl: Remove unneeded semicolon drivers/gpu/drm/msm/adreno/a5xx.xml.h | 2 + drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 195 ++--- drivers/gpu/drm/msm/adreno/a5xx_power.c| 4 +- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 105 ++- drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 49 ++ drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 139 ++- drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 2 + drivers/gpu/drm/msm/adreno/adreno_device.c | 54 +- drivers/gpu/drm/msm/adreno/adreno_gpu.c| 23 +-- drivers/gpu/drm/msm/adreno/adreno_gpu.h| 22 ++- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 90 -- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 87 ++--- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c| 26 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h| 14 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c| 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h| 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c| 73 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h| 3 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_vbif.c| 9 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 1 + drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c | 2 +- dri
[PATCH] radeon: added support for 2560x1080 resolution
I was wondering why I can't set the resolution to 2560x1080, while in windows 7 I can without a problem. I looked at the radeon driver code and found it doesn't support this resolution. So I made some changes. I added the hdmi_mhz parameter. In cmdline I set radeon.hdmi_mhz=190 Only tested on the Radeon HD 5830 --- drivers/gpu/drm/radeon/radeon_benchmark.c | 5 +++-- drivers/gpu/drm/radeon/radeon_connectors.c | 25 +- drivers/gpu/drm/radeon/radeon_drv.c| 5 + drivers/gpu/drm/radeon/radeon_encoders.c | 6 -- 4 files changed, 27 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_benchmark.c b/drivers/gpu/drm/radeon/radeon_benchmark.c index ac9a5ec481c3..c283b6b15925 100644 --- a/drivers/gpu/drm/radeon/radeon_benchmark.c +++ b/drivers/gpu/drm/radeon/radeon_benchmark.c @@ -30,7 +30,7 @@ #define RADEON_BENCHMARK_COPY_DMA 0 #define RADEON_BENCHMARK_ITERATIONS 1024 -#define RADEON_BENCHMARK_COMMON_MODES_N 17 +#define RADEON_BENCHMARK_COMMON_MODES_N 18 static int radeon_benchmark_do_move(struct radeon_device *rdev, unsigned size, uint64_t saddr, uint64_t daddr, @@ -184,7 +184,8 @@ void radeon_benchmark(struct radeon_device *rdev, int test_number) 1680 * 1050 * 4, 1600 * 1200 * 4, 1920 * 1080 * 4, - 1920 * 1200 * 4 + 1920 * 1200 * 4, + 2560 * 1080 * 4 }; switch (test_number) { diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c index 607ad5620bd9..182f1e364cbd 100644 --- a/drivers/gpu/drm/radeon/radeon_connectors.c +++ b/drivers/gpu/drm/radeon/radeon_connectors.c @@ -37,6 +37,8 @@ #include #include +extern int hdmimhz; + static int radeon_dp_handle_hpd(struct drm_connector *connector) { struct radeon_connector *radeon_connector = to_radeon_connector(connector); @@ -503,7 +505,7 @@ static void radeon_add_common_modes(struct drm_encoder *encoder, struct drm_conn struct mode_size { int w; int h; - } common_modes[17] = { + } common_modes[] = { { 640, 480}, { 720, 480}, { 800, 600}, @@ -520,10 +522,11 @@ static void radeon_add_common_modes(struct drm_encoder *encoder, struct drm_conn {1680, 1050}, {1600, 1200}, {1920, 1080}, - {1920, 1200} + {1920, 1200}, + {2560, 1080} }; - for (i = 0; i < 17; i++) { + for (i = 0; i < ARRAY_SIZE(common_modes); i++) { if (radeon_encoder->devices & (ATOM_DEVICE_TV_SUPPORT)) { if (common_modes[i].w > 1024 || common_modes[i].h > 768) @@ -1491,25 +1494,27 @@ static enum drm_mode_status radeon_dvi_mode_valid(struct drm_connector *connecto (mode->clock > 135000)) return MODE_CLOCK_HIGH; - if (radeon_connector->use_digital && (mode->clock > 165000)) { + if (radeon_connector->use_digital && (mode->clock > (hdmimhz * 1000))) { if ((radeon_connector->connector_object_id == CONNECTOR_OBJECT_ID_DUAL_LINK_DVI_I) || (radeon_connector->connector_object_id == CONNECTOR_OBJECT_ID_DUAL_LINK_DVI_D) || - (radeon_connector->connector_object_id == CONNECTOR_OBJECT_ID_HDMI_TYPE_B)) + (radeon_connector->connector_object_id == CONNECTOR_OBJECT_ID_HDMI_TYPE_B)){ return MODE_OK; - else if (ASIC_IS_DCE6(rdev) && drm_detect_hdmi_monitor(radeon_connector_edid(connector))) { + }else if (ASIC_IS_DCE6(rdev) && drm_detect_hdmi_monitor(radeon_connector_edid(connector))) { /* HDMI 1.3+ supports max clock of 340 Mhz */ - if (mode->clock > 34) + if (mode->clock > 34){ return MODE_CLOCK_HIGH; - else + }else{ return MODE_OK; + } } else { return MODE_CLOCK_HIGH; } } /* check against the max pixel clock */ - if ((mode->clock / 10) > rdev->clock.max_pixel_clock) + if ((mode->clock / 10) > rdev->clock.max_pixel_clock){ return MODE_CLOCK_HIGH; + } return MODE_OK; } @@ -1809,7 +1814,7 @@ static enum drm_mode_status radeon_dp_mode_valid(struct drm_connector *connector if (mode->clock > 34) return MODE_CLOCK_HIGH; } else { - if (mode->clock > 165000) + if (mode->clock > (hdmimhz * 1000))
[PATCH v15] staging: fbtft: add tearing signal detect
From: Carlis For st7789v IC, when we need continuous full screen refresh, it is best to wait for the tearing effect line signal to arrive to avoid screen tearing. Signed-off-by: Carlis --- v15: change ret value return logic in write_vmem. v14: change to define TE completion and TE irq only in st7789v. v13: change TE completion to par data struct member and add a new function to deal te gpio request, add new write_vmem function. v12: change dev_err to dev_err_probe and add space in comments start, and delete te_mutex, change te wait logic. v11: remove devm_gpio_put and change a dev_err to dev_info. v10: additional notes. v9: change pr_* to dev_*. v8: delete a log line. v7: return error value when request fail. v6: add te gpio request fail deal logic. v5: fix log print. v4: modify some code style and change te irq set function name. v3: modify author and signed-off-by name. v2: add release te gpio after irq request fail. --- drivers/staging/fbtft/fb_st7789v.c | 115 + 1 file changed, 115 insertions(+) diff --git a/drivers/staging/fbtft/fb_st7789v.c b/drivers/staging/fbtft/fb_st7789v.c index 3a280cc..abe9395 100644 --- a/drivers/staging/fbtft/fb_st7789v.c +++ b/drivers/staging/fbtft/fb_st7789v.c @@ -7,9 +7,13 @@ #include #include +#include #include #include +#include +#include #include + #include #include "fbtft.h" @@ -66,6 +70,62 @@ enum st7789v_command { #define MADCTL_MX BIT(6) /* bitmask for column address order */ #define MADCTL_MY BIT(7) /* bitmask for page address order */ +/* 60Hz for 16.6ms, configured as 2*16.6ms */ +#define PANEL_TE_TIMEOUT_MS 33 + +static struct completion panel_te; /* completion for panel TE line */ +static int irq_te; /* Linux IRQ for LCD TE line */ + +static irqreturn_t panel_te_handler(int irq, void *data) +{ + complete(&panel_te); + return IRQ_HANDLED; +} + +/* + * init_tearing_effect_line() - init tearing effect line. + * @par: FBTFT parameter object. + * + * Return: 0 on success, or a negative error code otherwise. + */ +static int init_tearing_effect_line(struct fbtft_par *par) +{ + struct device *dev = par->info->device; + struct gpio_desc *te; + int rc, irq; + + te = gpiod_get_optional(dev, "te", GPIOD_IN); + if (IS_ERR(te)) + return dev_err_probe(dev, PTR_ERR(te), "Failed to request te GPIO\n"); + + /* if te is NULL, indicating no configuration, directly return success */ + if (!te) { + irq_te = 0; + return 0; + } + + irq = gpiod_to_irq(te); + + /* GPIO is locked as an IRQ, we may drop the reference */ + gpiod_put(te); + + if (irq < 0) + return irq; + + irq_te = irq; + init_completion(&panel_te); + + /* The effective state is high and lasts no more than 1000 microseconds */ + rc = devm_request_irq(dev, irq_te, panel_te_handler, + IRQF_TRIGGER_RISING, "TE_GPIO", par); + if (rc) + return dev_err_probe(dev, rc, "TE IRQ request failed.\n"); + + disable_irq_nosync(irq_te); + + return 0; +} + /** * init_display() - initialize the display controller * @@ -82,6 +142,12 @@ enum st7789v_command { */ static int init_display(struct fbtft_par *par) { + int rc; + + rc = init_tearing_effect_line(par); + if (rc) + return rc; + /* turn off sleep mode */ write_reg(par, MIPI_DCS_EXIT_SLEEP_MODE); mdelay(120); @@ -137,6 +203,10 @@ static int init_display(struct fbtft_par *par) */ write_reg(par, PWCTRL1, 0xA4, 0xA1); + /* TE line output is off by default when powering on */ + if (irq_te) + write_reg(par, MIPI_DCS_SET_TEAR_ON, 0x00); + write_reg(par, MIPI_DCS_SET_DISPLAY_ON); if (HSD20_IPS) @@ -145,6 +215,50 @@ static int init_display(struct fbtft_par *par) return 0; } +/* + * write_vmem() - write data to display. + * @par: FBTFT parameter object. + * @offset: offset from screen_buffer. + * @len: the length of data to be writte. + * + * Return: 0 on success, or a negative error code otherwise. + */ +static int write_vmem(struct fbtft_par *par, size_t offset, size_t len) +{ + struct device *dev = par->info->device; + int ret; + + if (irq_te) { + enable_irq(irq_te); + reinit_completion(&panel_te); + ret = wait_for_completion_timeout(&panel_te, + msecs_to_jiffies(PANEL_TE_TIMEOUT_MS)); + if (ret == 0) + dev_err(dev, "wait panel TE timeout\n"); + + disable_irq(irq_te); + } + + switch (par->pdata->display.buswidth) { + case 8: + ret = fbtft_write_vmem16_bus8(par, offset, len); + break; + case 9: + ret = fbtft_write_vmem16_bus9(par, offset,
Re: [Nouveau] [PATCH] drm/nouveau: bail out of nouveau_channel_new if channel init fails
Hi Ben, On Mon, Nov 16, 2020 at 09:04:32AM +1000, Ben Skeggs wrote: > On Mon, 16 Nov 2020 at 05:19, Karol Herbst wrote: > > > > On Sun, Nov 15, 2020 at 6:43 PM Salvatore Bonaccorso > > wrote: > > > > > > Hi, > > > > > > On Fri, Aug 28, 2020 at 11:28:46AM +0200, Frantisek Hrbata wrote: > > > > Unprivileged user can crash kernel by using > > > > DRM_IOCTL_NOUVEAU_CHANNEL_ALLOC > > > > ioctl. This was reported by trinity[1] fuzzer. > > > > > > > > [ 71.073906] nouveau :01:00.0: crashme[1329]: channel failed to > > > > initialise, -17 > > > > [ 71.081730] BUG: kernel NULL pointer dereference, address: > > > > 00a0 > > > > [ 71.088928] #PF: supervisor read access in kernel mode > > > > [ 71.094059] #PF: error_code(0x) - not-present page > > > > [ 71.099189] PGD 119590067 P4D 119590067 PUD 1054f5067 PMD 0 > > > > [ 71.104842] Oops: [#1] SMP NOPTI > > > > [ 71.108498] CPU: 2 PID: 1329 Comm: crashme Not tainted 5.8.0-rc6+ #2 > > > > [ 71.114993] Hardware name: AMD Pike/Pike, BIOS RPK1506A 09/03/2014 > > > > [ 71.121213] RIP: 0010:nouveau_abi16_ioctl_channel_alloc+0x108/0x380 > > > > [nouveau] > > > > [ 71.128339] Code: 48 89 9d f0 00 00 00 41 8b 4c 24 04 41 8b 14 24 45 > > > > 31 c0 4c 8d 4b 10 48 89 ee 4c 89 f7 e8 10 11 00 00 85 c0 75 78 48 8b 43 > > > > 10 <8b> 90 a0 00 00 00 41 89 54 24 08 80 7d 3d 05 0f 86 bb 01 00 00 41 > > > > [ 71.147074] RSP: 0018:b4a1809cfd38 EFLAGS: 00010246 > > > > [ 71.152526] RAX: RBX: 98cedbaa1d20 RCX: > > > > 03bf > > > > [ 71.159651] RDX: 03be RSI: RDI: > > > > 00030160 > > > > [ 71.166774] RBP: 98cee776de00 R08: dc0144198a08 R09: > > > > 98ceeefd4000 > > > > [ 71.173901] R10: 98cee7e81780 R11: 0001 R12: > > > > b4a1809cfe08 > > > > [ 71.181214] R13: 98cee776d000 R14: 98cec519e000 R15: > > > > 98cee776def0 > > > > [ 71.188339] FS: 7fd926250500() GS:98ceeac8() > > > > knlGS: > > > > [ 71.196418] CS: 0010 DS: ES: CR0: 80050033 > > > > [ 71.202155] CR2: 00a0 CR3: 000106622000 CR4: > > > > 000406e0 > > > > [ 71.209297] Call Trace: > > > > [ 71.211777] ? nouveau_abi16_ioctl_getparam+0x1f0/0x1f0 [nouveau] > > > > [ 71.218053] drm_ioctl_kernel+0xac/0xf0 [drm] > > > > [ 71.222421] drm_ioctl+0x211/0x3c0 [drm] > > > > [ 71.226379] ? nouveau_abi16_ioctl_getparam+0x1f0/0x1f0 [nouveau] > > > > [ 71.232500] nouveau_drm_ioctl+0x57/0xb0 [nouveau] > > > > [ 71.237285] ksys_ioctl+0x86/0xc0 > > > > [ 71.240595] __x64_sys_ioctl+0x16/0x20 > > > > [ 71.244340] do_syscall_64+0x4c/0x90 > > > > [ 71.248110] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > > > [ 71.253162] RIP: 0033:0x7fd925d4b88b > > > > [ 71.256731] Code: Bad RIP value. > > > > [ 71.259955] RSP: 002b:7ffc743592d8 EFLAGS: 0206 ORIG_RAX: > > > > 0010 > > > > [ 71.267514] RAX: ffda RBX: RCX: > > > > 7fd925d4b88b > > > > [ 71.274637] RDX: 00601080 RSI: c0586442 RDI: > > > > 0003 > > > > [ 71.281986] RBP: 7ffc74359340 R08: 7fd926016ce0 R09: > > > > 7fd926016ce0 > > > > [ 71.289111] R10: 0003 R11: 0206 R12: > > > > 00400620 > > > > [ 71.296235] R13: 7ffc74359420 R14: R15: > > > > > > > > [ 71.303361] Modules linked in: rfkill sunrpc snd_hda_codec_realtek > > > > snd_hda_codec_generic ledtrig_audio snd_hda_intel snd_intel_dspcfg > > > > snd_hda_codec snd_hda_core edac_mce_amd snd_hwdep kvm_amd snd_seq ccp > > > > snd_seq_device snd_pcm kvm snd_timer snd irqbypass soundcore sp5100_tco > > > > pcspkr crct10dif_pclmul crc32_pclmul ghash_clmulni_intel wmi_bmof > > > > joydev i2c_piix4 fam15h_power k10temp acpi_cpufreq ip_tables xfs > > > > libcrc32c sd_mod t10_pi sg nouveau video mxm_wmi i2c_algo_bit > > > > drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops ttm > > > > broadcom bcm_phy_lib ata_generic ahci drm e1000 crc32c_intel libahci > > > > serio_raw tg3 libata firewire_ohci firewire_core wmi crc_itu_t > > > > dm_mirror dm_region_hash dm_log dm_mod > > > > [ 71.365269] CR2: 00a0 > > > > > > > > simplified reproducer > > > > -8< > > > > /* > > > > * gcc -o crashme crashme.c > > > > * ./crashme /dev/dri/renderD128 > > > > */ > > > > > > > > struct drm_nouveau_channel_alloc { > > > > uint32_t fb_ctxdma_handle; > > > > uint32_t tt_ctxdma_handle; > > > > > > > > int channel; > > > > uint32_t pushbuf_domains; > > > > > > > > /* Notifier memory */ > > > > uint32_t notifier_handle; > > > > > > > > /* DRM-enforced subchannel assignments */ > > > > struct { > > > > uint32_t handle;
Re: [PATCH 0/6] drm: Move vmap out of commit tail for SHMEM-based drivers
Hi Am 05.02.21 um 10:05 schrieb Gerd Hoffmann: Hi, I smoke-tested the code by running fbdev, Xorg and weston with the converted mgag200 driver. Looks sane to me. Survived cirrus smoke test too. Tested-by: Gerd Hoffmann Acked-by: Gerd Hoffmann I had to add one additional patch to v2 of this patchset to make things work with module-only builds. If you have a minute, could you ack this as well? Thanks! https://lore.kernel.org/dri-devel/20210205150628.28072-3-tzimmerm...@suse.de/ Best regards Thomas take care, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer OpenPGP_signature Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/2] pci sysfs file iomem revoke support
Hi Daniel, On Thu, 4 Feb 2021 17:58:29 +0100 Daniel Vetter wrote: > > Hi all, > > This is a revised version of patch 12 from my series to lock down some > follow_pfn vs VM_SPECIAL races: > > https://lore.kernel.org/dri-devel/cakwvodnsrsntgpeuqjyaotsktp2dr9208y66hqg_h1e2lkf...@mail.gmail.com/ > > Stephen reported an issue on HAVE_PCI_LEGACY platforms which this patch > set tries to address. Previous patches are all still in linux-next. > > Stephen, would be awesome if you can give this a spin. OK, I applied the 2 patches on top of next-20210205 and it no longer panics for my simple boot test (PowerPC pseries_le_defconfig under qemu). -- Cheers, Stephen Rothwell pgpo5U5ukj2E4.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 11/14] drm/amdgpu: Guard against write accesses after device removal
On 2/5/21 5:10 PM, Daniel Vetter wrote: On Fri, Feb 5, 2021 at 5:22 PM Andrey Grodzovsky wrote: Daniel, ping. Also, please refer to the other thread with Bjorn from pci-dev on the same topic I added you to. Summarizing my take over there for here plus maybe some more clarification. There's two problems: - You must guarantee that after the ->remove callback of your driver is finished, there's no more mmio or any other hw access. A combination of stopping stuff and drm_dev_enter/exit can help with that. This prevents the use-after-free issue. - For the actual hotunplug time, i.e. anything that can run while your driver is used up to the point where ->remove callback has finished stopp hw access you must guarantee that code doesn't blow up when it gets bogus reads (in the form of 0xff values). drm_dev_enter/exit can't help you with that. Plus you should make sure that we're not spending forever waiting for a big pile of mmio access all to time out because you never bail out - some coarse-grained drm_dev_enter/exit might help here. Plus finally the userspace access problem: You must guarantee that after ->remove has finished that none of the uapi or cross-driver access points (driver ioctl, dma-buf, dma-fence, anything else that hangs around) can reach the data structures/memory mappings/whatever which have been released as part of your ->remove callback. drm_dev_enter/exit is again the tool of choice here. So you have to use drm_dev_enter/exit for some of the problems we face on hotunplug, but it's not the tool that can handle the actual hw hotunplug race conditions for you. Unfortunately the hw hotunplug race condition is an utter pain to test, since essentially you need to validate your driver against spurious 0xff reads at any moment. And I don't even have a clever idea to simulate this, e.g. by forcefully replacing the iobar mapping: What we'd need is a mapping that allows reads (so we can fill a page with 0xff and use that everywhere), but instead of rejecting writes, allows them, but drops them (so that the 0xff stays intact). Maybe we could simulate this with some kernel debug tricks (kinda like mmiotrace) with a read-only mapping and dropping every write every time we fault. Clarification - as far as I know there are no page fault handlers for kernel mappings. And we are talking about kernel mappings here, right ? If there were I could solve all those issues the same as I do for user mappings, by invalidating all existing mappings in the kernel (both kmaps and ioreamps)and insert dummy zero or ~0 filled page instead. Also, I assume forcefully remapping the IO BAR to ~0 filled page would involve ioremap API and it's not something that I think can be easily done according to am answer i got to a related topic a few weeks ago https://www.spinics.net/lists/linux-pci/msg103396.html (that was the only reply i got) But ugh ... Otoh validating an entire driver like amdgpu without such a trick against 0xff reads is practically impossible. So maybe you need to add this as one of the tasks here? Or I could just for validation purposes return ~0 from all reg reads in the code and ignore writes if drm_dev_unplugged, this could already easily validate a big portion of the code flow under such scenario. Andrey -Daniel Andrey On 1/29/21 2:25 PM, Christian König wrote: Am 29.01.21 um 18:35 schrieb Andrey Grodzovsky: On 1/29/21 10:16 AM, Christian König wrote: Am 28.01.21 um 18:23 schrieb Andrey Grodzovsky: On 1/19/21 1:59 PM, Christian König wrote: Am 19.01.21 um 19:22 schrieb Andrey Grodzovsky: On 1/19/21 1:05 PM, Daniel Vetter wrote: [SNIP] So say writing in a loop to some harmless scratch register for many times both for plugged and unplugged case and measure total time delta ? I think we should at least measure the following: 1. Writing X times to a scratch reg without your patch. 2. Writing X times to a scratch reg with your patch. 3. Writing X times to a scratch reg with the hardware physically disconnected. I suggest to repeat that once for Polaris (or older) and once for Vega or Navi. The SRBM on Polaris is meant to introduce some delay in each access, so it might react differently then the newer hardware. Christian. See attached results and the testing code. Ran on Polaris (gfx8) and Vega10(gfx9) In summary, over 1 million WWREG32 in loop with and without this patch you get around 10ms of accumulated overhead ( so 0.1 millisecond penalty for each WWREG32) for using drm_dev_enter check when writing registers. P.S Bullet 3 I cannot test as I need eGPU and currently I don't have one. Well if I'm not completely mistaken that are 100ms of accumulated overhead. So around 100ns per write. And even bigger problem is that this is a ~67% increase. My bad, and 67% from what ? How u calculate ? My bad, (308501-209689)/209689=47% increase. I'm not sure how many write we do during normal operation, but that sounds like a bit much. Ideas? Well, u su
Re: [PATCH v4 11/14] drm/amdgpu: Guard against write accesses after device removal
On Sun, Feb 7, 2021 at 10:28 PM Andrey Grodzovsky wrote: > > > > On 2/5/21 5:10 PM, Daniel Vetter wrote: > > On Fri, Feb 5, 2021 at 5:22 PM Andrey Grodzovsky > > wrote: > >> > >> Daniel, ping. Also, please refer to the other thread with Bjorn from > >> pci-dev > >> on the same topic I added you to. > > > > Summarizing my take over there for here plus maybe some more > > clarification. There's two problems: > > > > - You must guarantee that after the ->remove callback of your driver > > is finished, there's no more mmio or any other hw access. A > > combination of stopping stuff and drm_dev_enter/exit can help with > > that. This prevents the use-after-free issue. > > > > - For the actual hotunplug time, i.e. anything that can run while your > > driver is used up to the point where ->remove callback has finished > > stopp hw access you must guarantee that code doesn't blow up when it > > gets bogus reads (in the form of 0xff values). drm_dev_enter/exit > > can't help you with that. Plus you should make sure that we're not > > spending forever waiting for a big pile of mmio access all to time out > > because you never bail out - some coarse-grained drm_dev_enter/exit > > might help here. > > > > Plus finally the userspace access problem: You must guarantee that > > after ->remove has finished that none of the uapi or cross-driver > > access points (driver ioctl, dma-buf, dma-fence, anything else that > > hangs around) can reach the data structures/memory mappings/whatever > > which have been released as part of your ->remove callback. > > drm_dev_enter/exit is again the tool of choice here. > > > > So you have to use drm_dev_enter/exit for some of the problems we face > > on hotunplug, but it's not the tool that can handle the actual hw > > hotunplug race conditions for you. > > > > Unfortunately the hw hotunplug race condition is an utter pain to > > test, since essentially you need to validate your driver against > > spurious 0xff reads at any moment. And I don't even have a clever idea > > to simulate this, e.g. by forcefully replacing the iobar mapping: What > > we'd need is a mapping that allows reads (so we can fill a page with > > 0xff and use that everywhere), but instead of rejecting writes, allows > > them, but drops them (so that the 0xff stays intact). Maybe we could > > simulate this with some kernel debug tricks (kinda like mmiotrace) > > with a read-only mapping and dropping every write every time we fault. > > Clarification - as far as I know there are no page fault handlers for kernel > mappings. And we are talking about kernel mappings here, right ? If there > were > I could solve all those issues the same as I do for user mappings, by > invalidating all existing mappings in the kernel (both kmaps and ioreamps)and > insert dummy zero or ~0 filled page instead. > Also, I assume forcefully remapping the IO BAR to ~0 filled page would involve > ioremap API and it's not something that I think can be easily done according > to > am answer i got to a related topic a few weeks ago > https://www.spinics.net/lists/linux-pci/msg103396.html (that was the only > reply > i got) mmiotrace can, but only for debug, and only on x86 platforms: https://www.kernel.org/doc/html/latest/trace/mmiotrace.html Should be feasible (but maybe not worth the effort) to extend this to support fake unplug. > > > But ugh ... > > > > Otoh validating an entire driver like amdgpu without such a trick > > against 0xff reads is practically impossible. So maybe you need to add > > this as one of the tasks here? > > Or I could just for validation purposes return ~0 from all reg reads in the > code > and ignore writes if drm_dev_unplugged, this could already easily validate a > big > portion of the code flow under such scenario. Hm yeah if your really wrap them all, that should work too. Since iommappings have __iomem pointer type, as long as amdgpu is sparse warning free, should be doable to guarantee this. -Daniel > Andrey > > > -Daniel > > > >> > >> Andrey > >> > >> On 1/29/21 2:25 PM, Christian König wrote: > >>> Am 29.01.21 um 18:35 schrieb Andrey Grodzovsky: > > On 1/29/21 10:16 AM, Christian König wrote: > > Am 28.01.21 um 18:23 schrieb Andrey Grodzovsky: > >> > >> On 1/19/21 1:59 PM, Christian König wrote: > >>> Am 19.01.21 um 19:22 schrieb Andrey Grodzovsky: > > On 1/19/21 1:05 PM, Daniel Vetter wrote: > > [SNIP] > So say writing in a loop to some harmless scratch register for many > times > both for plugged > and unplugged case and measure total time delta ? > >>> > >>> I think we should at least measure the following: > >>> > >>> 1. Writing X times to a scratch reg without your patch. > >>> 2. Writing X times to a scratch reg with your patch. > >>> 3. Writing X times to a scratch reg with the hardware physically > >>> disconnected. > >>> > >>> I suggest to repeat that once for Polar
[Bug 203905] amdgpu:actual_brightness has unreal/wrong value
https://bugzilla.kernel.org/show_bug.cgi?id=203905 Paulo Nascimento (paulo.ul...@googlemail.com) changed: What|Removed |Added CC||paulo.ul...@googlemail.com --- Comment #21 from Paulo Nascimento (paulo.ul...@googlemail.com) --- I confirm that the bug is not fixed in kenel 5.11-rc3 max brightness: [pn@pn-legion backlight]$ cat /sys/class/backlight/amdgpu_bl1/actual_brightness 311 [pn@pn-legion backlight]$ cat /sys/class/backlight/amdgpu_bl1/brightness 255 [pn@pn-legion backlight]$ cat /sys/class/backlight/amdgpu_bl1/max_brightness 255 min brightness: [pn@pn-legion backlight]$ cat /sys/class/backlight/amdgpu_bl1/actual_brightness 311 [pn@pn-legion backlight]$ cat /sys/class/backlight/amdgpu_bl1/brightness 0 [pn@pn-legion backlight]$ cat /sys/class/backlight/amdgpu_bl1/max_brightness 255 My laptop has an amd ryzen 5 4600h with renoir igpu and nvidia dgpu. Tests are done with renoir igpu: [pn@pn-legion backlight]$ inxi -G Graphics: Device-1: NVIDIA TU116M [GeForce GTX 1660 Ti Mobile] driver: nouveau v: kernel Device-2: Advanced Micro Devices [AMD/ATI] Renoir driver: amdgpu v: kernel Device-3: Chicony Integrated Camera type: USB driver: uvcvideo Display: x11 server: X.Org 1.20.10 driver: loaded: amdgpu,ati,modesetting,nouveau resolution: 1920x1080~120Hz OpenGL: renderer: AMD RENOIR (DRM 3.40.0 5.11.0-1-MANJARO LLVM 11.0.1) v: 4.6 Mesa 20.3.3 -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amdgpu: fix potential integer overflow on shift of a int
From: Colin Ian King The left shift of int 32 bit integer constant 1 is evaluated using 32 bit arithmetic and then assigned to an unsigned 64 bit integer. In the case where *frag is 32 or more this can lead to an oveflow. Avoid this by shifting 1ULL. Addresses-Coverity: ("Unintentional integer overflow") Fixes: dfcd99f6273e ("drm/amdgpu: meld together VM fragment and huge page handling") Signed-off-by: Colin Ian King --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 9d19078246c8..53a925600510 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -1412,7 +1412,7 @@ static void amdgpu_vm_fragment(struct amdgpu_vm_update_params *params, *frag = max_frag; *frag_end = end & ~((1ULL << max_frag) - 1); } else { - *frag_end = start + (1 << *frag); + *frag_end = start + (1ULL << *frag); } } -- 2.29.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/3] Add check for max clock rate in mode_valid
Changes since v1: - fix build err. Jitao Shi (3): drm/mediatek: mtk_dpi: Add check for max clock rate in mode_valid drm/mediatek: mtk_dpi: Add dpi config for mt8192 dt-bindings: mediatek,dpi: add mt8192 to mediatek,dpi .../display/mediatek/mediatek,dpi.yaml| 1 + drivers/gpu/drm/mediatek/mtk_dpi.c| 26 +++ 2 files changed, 27 insertions(+) -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/3] drm/mediatek: mtk_dpi: Add check for max clock rate in mode_valid
Add per-platform max clock rate check in mtk_dpi_bridge_mode_valid. Signed-off-by: Jitao Shi --- drivers/gpu/drm/mediatek/mtk_dpi.c | 17 + 1 file changed, 17 insertions(+) diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c index 52f11a63a330..ffa4a0f1989f 100644 --- a/drivers/gpu/drm/mediatek/mtk_dpi.c +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c @@ -118,6 +118,7 @@ struct mtk_dpi_yc_limit { struct mtk_dpi_conf { unsigned int (*cal_factor)(int clock); u32 reg_h_fre_con; + u32 max_clock_khz; bool edge_sel_en; }; @@ -555,9 +556,22 @@ static void mtk_dpi_bridge_enable(struct drm_bridge *bridge) mtk_dpi_set_display_mode(dpi, &dpi->mode); } +static enum drm_mode_status +mtk_dpi_bridge_mode_valid(struct drm_bridge *bridge, + const struct drm_display_mode *mode) +{ + struct mtk_dpi *dpi = bridge_to_dpi(bridge); + + if (dpi->conf->max_clock_khz && mode->clock > dpi->conf->max_clock_khz) + return MODE_CLOCK_HIGH; + + return MODE_OK; +} + static const struct drm_bridge_funcs mtk_dpi_bridge_funcs = { .attach = mtk_dpi_bridge_attach, .mode_set = mtk_dpi_bridge_mode_set, + .mode_valid = mtk_dpi_bridge_mode_valid, .disable = mtk_dpi_bridge_disable, .enable = mtk_dpi_bridge_enable, }; @@ -673,17 +687,20 @@ static unsigned int mt8183_calculate_factor(int clock) static const struct mtk_dpi_conf mt8173_conf = { .cal_factor = mt8173_calculate_factor, .reg_h_fre_con = 0xe0, + .max_clock_khz = 30, }; static const struct mtk_dpi_conf mt2701_conf = { .cal_factor = mt2701_calculate_factor, .reg_h_fre_con = 0xb0, .edge_sel_en = true, + .max_clock_khz = 15, }; static const struct mtk_dpi_conf mt8183_conf = { .cal_factor = mt8183_calculate_factor, .reg_h_fre_con = 0xe0, + .max_clock_khz = 10, }; static int mtk_dpi_probe(struct platform_device *pdev) -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/3] dt-bindings: mediatek,dpi: add mt8192 to mediatek,dpi
Add compatible "mediatek,mt8192-dpi" for the mt8192 dpi. Signed-off-by: Jitao Shi --- .../devicetree/bindings/display/mediatek/mediatek,dpi.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml index 6cdb734c91a9..2f566f19e6e0 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml @@ -22,6 +22,7 @@ properties: - mediatek,mt7623-dpi - mediatek,mt8173-dpi - mediatek,mt8183-dpi + - mediatek,mt8192-dpi reg: maxItems: 1 -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/3] drm/mediatek: mtk_dpi: Add dpi config for mt8192
Signed-off-by: Jitao Shi --- drivers/gpu/drm/mediatek/mtk_dpi.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c index ffa4a0f1989f..f6f71eb67ff1 100644 --- a/drivers/gpu/drm/mediatek/mtk_dpi.c +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c @@ -703,6 +703,12 @@ static const struct mtk_dpi_conf mt8183_conf = { .max_clock_khz = 10, }; +static const struct mtk_dpi_conf mt8192_conf = { + .cal_factor = mt8183_calculate_factor, + .reg_h_fre_con = 0xe0, + .max_clock_khz = 15, +}; + static int mtk_dpi_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -837,6 +843,9 @@ static const struct of_device_id mtk_dpi_of_ids[] = { { .compatible = "mediatek,mt8183-dpi", .data = &mt8183_conf, }, + { .compatible = "mediatek,mt8192-dpi", + .data = &mt8192_conf, + }, { }, }; -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 0/3] Add check for max clock rate in mode_valid
Changes since v2: - add const struct drm_display_info *info in mtk_dpi_bridge_mode_valid Jitao Shi (3): drm/mediatek: mtk_dpi: Add check for max clock rate in mode_valid drm/mediatek: mtk_dpi: Add dpi config for mt8192 dt-bindings: mediatek,dpi: add mt8192 to mediatek,dpi .../display/mediatek/mediatek,dpi.yaml| 1 + drivers/gpu/drm/mediatek/mtk_dpi.c| 26 +++ 2 files changed, 27 insertions(+) -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/3] drm/mediatek: mtk_dpi: Add check for max clock rate in mode_valid
Add per-platform max clock rate check in mtk_dpi_bridge_mode_valid. Signed-off-by: Jitao Shi --- drivers/gpu/drm/mediatek/mtk_dpi.c | 17 + 1 file changed, 17 insertions(+) diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c index 52f11a63a330..ffa4a0f1989f 100644 --- a/drivers/gpu/drm/mediatek/mtk_dpi.c +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c @@ -118,6 +118,7 @@ struct mtk_dpi_yc_limit { struct mtk_dpi_conf { unsigned int (*cal_factor)(int clock); u32 reg_h_fre_con; + u32 max_clock_khz; bool edge_sel_en; }; @@ -555,9 +556,22 @@ static void mtk_dpi_bridge_enable(struct drm_bridge *bridge) mtk_dpi_set_display_mode(dpi, &dpi->mode); } +static enum drm_mode_status +mtk_dpi_bridge_mode_valid(struct drm_bridge *bridge, + const struct drm_display_mode *mode) +{ + struct mtk_dpi *dpi = bridge_to_dpi(bridge); + + if (dpi->conf->max_clock_khz && mode->clock > dpi->conf->max_clock_khz) + return MODE_CLOCK_HIGH; + + return MODE_OK; +} + static const struct drm_bridge_funcs mtk_dpi_bridge_funcs = { .attach = mtk_dpi_bridge_attach, .mode_set = mtk_dpi_bridge_mode_set, + .mode_valid = mtk_dpi_bridge_mode_valid, .disable = mtk_dpi_bridge_disable, .enable = mtk_dpi_bridge_enable, }; @@ -673,17 +687,20 @@ static unsigned int mt8183_calculate_factor(int clock) static const struct mtk_dpi_conf mt8173_conf = { .cal_factor = mt8173_calculate_factor, .reg_h_fre_con = 0xe0, + .max_clock_khz = 30, }; static const struct mtk_dpi_conf mt2701_conf = { .cal_factor = mt2701_calculate_factor, .reg_h_fre_con = 0xb0, .edge_sel_en = true, + .max_clock_khz = 15, }; static const struct mtk_dpi_conf mt8183_conf = { .cal_factor = mt8183_calculate_factor, .reg_h_fre_con = 0xe0, + .max_clock_khz = 10, }; static int mtk_dpi_probe(struct platform_device *pdev) -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 3/3] dt-bindings: mediatek,dpi: add mt8192 to mediatek,dpi
Add compatible "mediatek,mt8192-dpi" for the mt8192 dpi. Signed-off-by: Jitao Shi --- .../devicetree/bindings/display/mediatek/mediatek,dpi.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml index 6cdb734c91a9..2f566f19e6e0 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml @@ -22,6 +22,7 @@ properties: - mediatek,mt7623-dpi - mediatek,mt8173-dpi - mediatek,mt8183-dpi + - mediatek,mt8192-dpi reg: maxItems: 1 -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 2/3] drm/mediatek: mtk_dpi: Add dpi config for mt8192
Signed-off-by: Jitao Shi --- drivers/gpu/drm/mediatek/mtk_dpi.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c index ffa4a0f1989f..f6f71eb67ff1 100644 --- a/drivers/gpu/drm/mediatek/mtk_dpi.c +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c @@ -703,6 +703,12 @@ static const struct mtk_dpi_conf mt8183_conf = { .max_clock_khz = 10, }; +static const struct mtk_dpi_conf mt8192_conf = { + .cal_factor = mt8183_calculate_factor, + .reg_h_fre_con = 0xe0, + .max_clock_khz = 15, +}; + static int mtk_dpi_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -837,6 +843,9 @@ static const struct of_device_id mtk_dpi_of_ids[] = { { .compatible = "mediatek,mt8183-dpi", .data = &mt8183_conf, }, + { .compatible = "mediatek,mt8192-dpi", + .data = &mt8192_conf, + }, { }, }; -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/tilcdc: replace spin_lock_irqsave by spin_lock in hard IRQ
The code has been in a irq-disabled context since it is hard IRQ. There is no necessity to do it again. Signed-off-by: Tian Tao --- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index 3021370..b3e38e9 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c @@ -904,13 +904,12 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc) tilcdc_clear_irqstatus(dev, stat); if (stat & LCDC_END_OF_FRAME0) { - unsigned long flags; bool skip_event = false; ktime_t now; now = ktime_get(); - spin_lock_irqsave(&tilcdc_crtc->irq_lock, flags); + spin_lock(&tilcdc_crtc->irq_lock); tilcdc_crtc->last_vblank = now; @@ -920,21 +919,21 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc) skip_event = true; } - spin_unlock_irqrestore(&tilcdc_crtc->irq_lock, flags); + spin_unlock(&tilcdc_crtc->irq_lock); drm_crtc_handle_vblank(crtc); if (!skip_event) { struct drm_pending_vblank_event *event; - spin_lock_irqsave(&dev->event_lock, flags); + spin_lock(&dev->event_lock); event = tilcdc_crtc->event; tilcdc_crtc->event = NULL; if (event) drm_crtc_send_vblank_event(crtc, event); - spin_unlock_irqrestore(&dev->event_lock, flags); + spin_unlock(&dev->event_lock); } if (tilcdc_crtc->frame_intact) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 00/14] RFC Support hot device unplug in amdgpu
On 1/20/21 10:59 AM, Daniel Vetter wrote: On Wed, Jan 20, 2021 at 3:20 PM Andrey Grodzovsky wrote: On 1/20/21 4:05 AM, Daniel Vetter wrote: On Tue, Jan 19, 2021 at 01:18:15PM -0500, Andrey Grodzovsky wrote: On 1/19/21 1:08 PM, Daniel Vetter wrote: On Tue, Jan 19, 2021 at 6:31 PM Andrey Grodzovsky wrote: On 1/19/21 9:16 AM, Daniel Vetter wrote: On Mon, Jan 18, 2021 at 04:01:09PM -0500, Andrey Grodzovsky wrote: Until now extracting a card either by physical extraction (e.g. eGPU with thunderbolt connection or by emulation through syfs -> /sys/bus/pci/devices/device_id/remove) would cause random crashes in user apps. The random crashes in apps were mostly due to the app having mapped a device backed BO into its address space was still trying to access the BO while the backing device was gone. To answer this first problem Christian suggested to fix the handling of mapped memory in the clients when the device goes away by forcibly unmap all buffers the user processes has by clearing their respective VMAs mapping the device BOs. Then when the VMAs try to fill in the page tables again we check in the fault handlerif the device is removed and if so, return an error. This will generate a SIGBUS to the application which can then cleanly terminate.This indeed was done but this in turn created a problem of kernel OOPs were the OOPSes were due to the fact that while the app was terminating because of the SIGBUSit would trigger use after free in the driver by calling to accesses device structures that were already released from the pci remove sequence.This was handled by introducing a 'flush' sequence during device removal were we wait for drm file reference to drop to 0 meaning all user clients directly using this device terminated. v2: Based on discussions in the mailing list with Daniel and Pekka [1] and based on the document produced by Pekka from those discussions [2] the whole approach with returning SIGBUS and waiting for all user clients having CPU mapping of device BOs to die was dropped. Instead as per the document suggestion the device structures are kept alive until the last reference to the device is dropped by user client and in the meanwhile all existing and new CPU mappings of the BOs belonging to the device directly or by dma-buf import are rerouted to per user process dummy rw page.Also, I skipped the 'Requirements for KMS UAPI' section of [2] since i am trying to get the minimal set of requirements that still give useful solution to work and this is the'Requirements for Render and Cross-Device UAPI' section and so my test case is removing a secondary device, which is render only and is not involved in KMS. v3: More updates following comments from v2 such as removing loop to find DRM file when rerouting page faults to dummy page,getting rid of unnecessary sysfs handling refactoring and moving prevention of GPU recovery post device unplug from amdgpu to scheduler layer. On top of that added unplug support for the IOMMU enabled system. v4: Drop last sysfs hack and use sysfs default attribute. Guard against write accesses after device removal to avoid modifying released memory. Update dummy pages handling to on demand allocation and release through drm managed framework. Add return value to scheduler job TO handler (by Luben Tuikov) and use this in amdgpu for prevention of GPU recovery post device unplug Also rebase on top of drm-misc-mext instead of amd-staging-drm-next With these patches I am able to gracefully remove the secondary card using sysfs remove hook while glxgears is running off of secondary card (DRI_PRIME=1) without kernel oopses or hangs and keep working with the primary card or soft reset the device without hangs or oopses TODOs for followup work: Convert AMDGPU code to use devm (for hw stuff) and drmm (for sw stuff and allocations) (Daniel) Support plugging the secondary device back after unplug - currently still experiencing HW error on plugging back. Add support for 'Requirements for KMS UAPI' section of [2] - unplugging primary, display connected card. [1] - Discussions during v3 of the patchset https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Famd-gfx%2Fmsg55576.html&data=04%7C01%7CAndrey.Grodzovsky%40amd.com%7Cf3fc3c7b55df40e165f408d8bd5c7364%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637467552072067767%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=fjgP9YubHCrILFxWmpVGSmurTJHkWw%2Bv4okyjSNsPxE%3D&reserved=0 [2] - drm/doc: device hot-unplug for userspace https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Fdri-devel%2Fmsg259755.html&data=04%7C01%7CAndrey.Grodzovsky%40amd.com%7Cf3fc3c7b55df40e165f408d8bd5c7364%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637467552072067767%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=11PYyAhOjLDEiNNho8WaMB%2FL
Re: [PATCH v4 00/14] RFC Support hot device unplug in amdgpu
On Mon, Feb 8, 2021 at 6:59 AM Andrey Grodzovsky wrote: > > > On 1/20/21 10:59 AM, Daniel Vetter wrote: > > On Wed, Jan 20, 2021 at 3:20 PM Andrey Grodzovsky > > wrote: > >> > >> On 1/20/21 4:05 AM, Daniel Vetter wrote: > >>> On Tue, Jan 19, 2021 at 01:18:15PM -0500, Andrey Grodzovsky wrote: > On 1/19/21 1:08 PM, Daniel Vetter wrote: > > On Tue, Jan 19, 2021 at 6:31 PM Andrey Grodzovsky > > wrote: > >> On 1/19/21 9:16 AM, Daniel Vetter wrote: > >>> On Mon, Jan 18, 2021 at 04:01:09PM -0500, Andrey Grodzovsky wrote: > Until now extracting a card either by physical extraction (e.g. eGPU > with > thunderbolt connection or by emulation through syfs -> > /sys/bus/pci/devices/device_id/remove) > would cause random crashes in user apps. The random crashes in apps > were > mostly due to the app having mapped a device backed BO into its > address > space was still trying to access the BO while the backing device was > gone. > To answer this first problem Christian suggested to fix the handling > of mapped > memory in the clients when the device goes away by forcibly unmap > all buffers the > user processes has by clearing their respective VMAs mapping the > device BOs. > Then when the VMAs try to fill in the page tables again we check in > the fault > handlerif the device is removed and if so, return an error. This > will generate a > SIGBUS to the application which can then cleanly terminate.This > indeed was done > but this in turn created a problem of kernel OOPs were the OOPSes > were due to the > fact that while the app was terminating because of the SIGBUSit > would trigger use > after free in the driver by calling to accesses device structures > that were already > released from the pci remove sequence.This was handled by > introducing a 'flush' > sequence during device removal were we wait for drm file reference > to drop to 0 > meaning all user clients directly using this device terminated. > > v2: > Based on discussions in the mailing list with Daniel and Pekka [1] > and based on the document > produced by Pekka from those discussions [2] the whole approach with > returning SIGBUS and > waiting for all user clients having CPU mapping of device BOs to die > was dropped. > Instead as per the document suggestion the device structures are > kept alive until > the last reference to the device is dropped by user client and in > the meanwhile all existing and new CPU mappings of the BOs > belonging to the device directly or by dma-buf import are rerouted > to per user > process dummy rw page.Also, I skipped the 'Requirements for KMS > UAPI' section of [2] > since i am trying to get the minimal set of requirements that still > give useful solution > to work and this is the'Requirements for Render and Cross-Device > UAPI' section and so my > test case is removing a secondary device, which is render only and > is not involved > in KMS. > > v3: > More updates following comments from v2 such as removing loop to > find DRM file when rerouting > page faults to dummy page,getting rid of unnecessary sysfs handling > refactoring and moving > prevention of GPU recovery post device unplug from amdgpu to > scheduler layer. > On top of that added unplug support for the IOMMU enabled system. > > v4: > Drop last sysfs hack and use sysfs default attribute. > Guard against write accesses after device removal to avoid modifying > released memory. > Update dummy pages handling to on demand allocation and release > through drm managed framework. > Add return value to scheduler job TO handler (by Luben Tuikov) and > use this in amdgpu for prevention > of GPU recovery post device unplug > Also rebase on top of drm-misc-mext instead of amd-staging-drm-next > > With these patches I am able to gracefully remove the secondary card > using sysfs remove hook while glxgears > is running off of secondary card (DRI_PRIME=1) without kernel oopses > or hangs and keep working > with the primary card or soft reset the device without hangs or > oopses > > TODOs for followup work: > Convert AMDGPU code to use devm (for hw stuff) and drmm (for sw > stuff and allocations) (Daniel) > Support plugging the secondary de
Re: [RFC v3 2/3] virtio: Introduce Vdmabuf driver
Hi, > > +/* extract pages referenced by sgt */ > > +static struct page **extr_pgs(struct sg_table *sgt, int *nents, int > > *last_len) > > Nack, this doesn't work on dma-buf. And it'll blow up at runtime when you > enable the very recently merged CONFIG_DMABUF_DEBUG (would be good to test > with that, just to make sure). > Aside from this, for virtio/kvm use-cases we've already merged the udmabuf > driver. Does this not work for your usecase? udmabuf can be used on the host side to make a collection of guest pages available as host dmabuf. It's part of the puzzle, but not a complete solution. As I understand it the intended workflow is this: (1) guest gpu driver exports some object as dma-buf (2) dma-buf is imported into this new driver. (3) driver sends the pages to the host. (4) hypervisor uses udmabuf to create a host dma-buf. (5) host dma-buf is passed on. And step (3) is the problematic one as this will not work in case the dma-buf doesn't live in guest ram but in -- for example -- gpu device memory. Reversing the driver roles in the guest (virtio driver allocates pages and exports the dma-buf to the guest gpu driver) should work fine. Which btw is something you can do today with virtio-gpu. Maybe it makes sense to have the option to run virtio-gpu in render-only mode for that use case. take care, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel