[PATCH v3 1/6] dt-bindings: drm/msm/gpu: Document gpu opp table
Update documentation to list the gpu opp table bindings including the newly added "opp-peak-kBps" needed for GPU-DDR bandwidth scaling. Signed-off-by: Sharat Masetty Acked-by: Rob Herring --- .../devicetree/bindings/display/msm/gpu.txt| 28 ++ 1 file changed, 28 insertions(+) diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt b/Documentation/devicetree/bindings/display/msm/gpu.txt index 70025cb..48bd4ab 100644 --- a/Documentation/devicetree/bindings/display/msm/gpu.txt +++ b/Documentation/devicetree/bindings/display/msm/gpu.txt @@ -79,6 +79,34 @@ Example a6xx (with GMU): interconnects = <_hlos MASTER_GFX3D _hlos SLAVE_EBI1>; + gpu_opp_table: opp-table { + compatible = "operating-points-v2"; + + opp-43000 { + opp-hz = /bits/ 64 <43000>; + opp-level = ; + opp-peak-kBps = <5412000>; + }; + + opp-35500 { + opp-hz = /bits/ 64 <35500>; + opp-level = ; + opp-peak-kBps = <3072000>; + }; + + opp-26700 { + opp-hz = /bits/ 64 <26700>; + opp-level = ; + opp-peak-kBps = <3072000>; + }; + + opp-18000 { + opp-hz = /bits/ 64 <18000>; + opp-level = ; + opp-peak-kBps = <1804000>; + }; + }; + qcom,gmu = <>; zap-shader { -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 3/6] drm: msm: a6xx: use dev_pm_opp_set_bw to scale DDR
This patches replaces the previously used static DDR vote and uses dev_pm_opp_set_bw() to scale GPU->DDR bandwidth along with scaling GPU frequency. Also since the icc path voting is handled completely in the opp driver, remove the icc_path handle and its usage in the drm driver. Signed-off-by: Sharat Masetty --- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 23 --- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 8 drivers/gpu/drm/msm/msm_gpu.h | 2 -- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index 2d8124b..1dd8fc5 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -141,11 +141,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp) gmu->freq = gmu->gpu_freqs[perf_index]; - /* -* Eventually we will want to scale the path vote with the frequency but -* for now leave it at max so that the performance is nominal. -*/ - icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216)); + dev_pm_opp_set_bw(>pdev->dev, opp); } unsigned long a6xx_gmu_get_freq(struct msm_gpu *gpu) @@ -715,6 +711,19 @@ static void a6xx_gmu_set_initial_freq(struct msm_gpu *gpu, struct a6xx_gmu *gmu) dev_pm_opp_put(gpu_opp); } +static void a6xx_gmu_set_initial_bw(struct msm_gpu *gpu, struct a6xx_gmu *gmu) +{ + struct dev_pm_opp *gpu_opp; + unsigned long gpu_freq = gmu->gpu_freqs[gmu->current_perf_index]; + + gpu_opp = dev_pm_opp_find_freq_exact(>pdev->dev, gpu_freq, true); + if (IS_ERR_OR_NULL(gpu_opp)) + return; + + dev_pm_opp_set_bw(>pdev->dev, gpu_opp); + dev_pm_opp_put(gpu_opp); +} + int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) { struct adreno_gpu *adreno_gpu = _gpu->base; @@ -739,7 +748,7 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) } /* Set the bus quota to a reasonable value for boot */ - icc_set_bw(gpu->icc_path, 0, MBps_to_icc(3072)); + a6xx_gmu_set_initial_bw(gpu, gmu); /* Enable the GMU interrupt */ gmu_write(gmu, REG_A6XX_GMU_AO_HOST_INTERRUPT_CLR, ~0); @@ -907,7 +916,7 @@ int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu) a6xx_gmu_shutdown(gmu); /* Remove the bus vote */ - icc_set_bw(gpu->icc_path, 0, 0); + dev_pm_opp_set_bw(>pdev->dev, NULL); /* * Make sure the GX domain is off before turning off the GMU (CX) diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index 2d13694..718c705 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -913,11 +913,6 @@ static int adreno_get_pwrlevels(struct device *dev, DBG("fast_rate=%u, slow_rate=2700", gpu->fast_rate); - /* Check for an interconnect path for the bus */ - gpu->icc_path = of_icc_get(dev, NULL); - if (IS_ERR(gpu->icc_path)) - gpu->icc_path = NULL; - return 0; } @@ -958,13 +953,10 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, void adreno_gpu_cleanup(struct adreno_gpu *adreno_gpu) { - struct msm_gpu *gpu = _gpu->base; unsigned int i; for (i = 0; i < ARRAY_SIZE(adreno_gpu->info->fw); i++) release_firmware(adreno_gpu->fw[i]); - icc_put(gpu->icc_path); - msm_gpu_cleanup(_gpu->base); } diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h index cf0dc6d..c7d74a9 100644 --- a/drivers/gpu/drm/msm/msm_gpu.h +++ b/drivers/gpu/drm/msm/msm_gpu.h @@ -112,8 +112,6 @@ struct msm_gpu { struct clk *ebi1_clk, *core_clk, *rbbmtimer_clk; uint32_t fast_rate; - struct icc_path *icc_path; - /* Hang and Inactivity Detection: */ #define DRM_MSM_INACTIVE_PERIOD 66 /* in ms (roughly four frames) */ -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 4/6] arm64: dts: qcom: SDM845: Enable GPU DDR bw scaling
This patch adds the interconnects property for the gpu node and the opp-peak-kBps property to the opps of the gpu opp table. This should help enable DDR bandwidth scaling dynamically and proportionally to the GPU frequency. Signed-off-by: Sharat Masetty --- arch/arm64/boot/dts/qcom/sdm845.dtsi | 9 + 1 file changed, 9 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index 11fc3f24..6ea6f54 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -3240,6 +3240,8 @@ qcom,gmu = <>; + interconnects = <_noc MASTER_GFX3D _noc SLAVE_EBI1>; + zap_shader: zap-shader { memory-region = <_mem>; }; @@ -3250,36 +3252,43 @@ opp-71000 { opp-hz = /bits/ 64 <71000>; opp-level = ; + opp-peak-kBps = <7216000>; }; opp-67500 { opp-hz = /bits/ 64 <67500>; opp-level = ; + opp-peak-kBps = <7216000>; }; opp-59600 { opp-hz = /bits/ 64 <59600>; opp-level = ; + opp-peak-kBps = <622>; }; opp-52000 { opp-hz = /bits/ 64 <52000>; opp-level = ; + opp-peak-kBps = <622>; }; opp-41400 { opp-hz = /bits/ 64 <41400>; opp-level = ; + opp-peak-kBps = <4068000>; }; opp-34200 { opp-hz = /bits/ 64 <34200>; opp-level = ; + opp-peak-kBps = <2724000>; }; opp-25700 { opp-hz = /bits/ 64 <25700>; opp-level = ; + opp-peak-kBps = <1648000>; }; }; }; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 5/6] arm64: dts: qcom: sc7180: Add interconnects property for GPU
This patch adds the interconnects property to the GPU node. This enables the GPU->DDR path bandwidth voting. Signed-off-by: Sharat Masetty --- arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi index eaede5e..34004ad 100644 --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi @@ -1497,6 +1497,8 @@ operating-points-v2 = <_opp_table>; qcom,gmu = <>; + interconnects = <_noc MASTER_GFX3D _virt SLAVE_EBI1>; + gpu_opp_table: opp-table { compatible = "operating-points-v2"; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 6/6] arm64: dts: qcom: sc7180: Add opp-peak-kBps to GPU opp
Add opp-peak-kBps bindings to the GPU opp table, listing the peak GPU -> DDR bandwidth requirement for each opp level. This will be used to scale the DDR bandwidth along with the GPU frequency dynamically. Signed-off-by: Sharat Masetty Reviewed-by: Matthias Kaehlcke --- arch/arm64/boot/dts/qcom/sc7180.dtsi | 7 +++ 1 file changed, 7 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi index 34004ad..7bef42b 100644 --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi @@ -1505,36 +1505,43 @@ opp-8 { opp-hz = /bits/ 64 <8>; opp-level = ; + opp-peak-kBps = <8532000>; }; opp-65000 { opp-hz = /bits/ 64 <65000>; opp-level = ; + opp-peak-kBps = <7216000>; }; opp-56500 { opp-hz = /bits/ 64 <56500>; opp-level = ; + opp-peak-kBps = <5412000>; }; opp-43000 { opp-hz = /bits/ 64 <43000>; opp-level = ; + opp-peak-kBps = <5412000>; }; opp-35500 { opp-hz = /bits/ 64 <35500>; opp-level = ; + opp-peak-kBps = <3072000>; }; opp-26700 { opp-hz = /bits/ 64 <26700>; opp-level = ; + opp-peak-kBps = <3072000>; }; opp-18000 { opp-hz = /bits/ 64 <18000>; opp-level = ; + opp-peak-kBps = <1804000>; }; }; }; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 2/6] drm: msm: a6xx: send opp instead of a frequency
This patch changes the plumbing to send the devfreq recommended opp rather than the frequency. Also consolidate and rearrange the code in a6xx to set the GPU frequency and the icc vote in preparation for the upcoming changes for GPU->DDR scaling votes. Signed-off-by: Sharat Masetty --- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 62 +++ drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 2 +- drivers/gpu/drm/msm/msm_gpu.c | 3 +- drivers/gpu/drm/msm/msm_gpu.h | 3 +- 4 files changed, 38 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index 748cd37..2d8124b 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -100,17 +100,30 @@ bool a6xx_gmu_gx_is_on(struct a6xx_gmu *gmu) A6XX_GMU_SPTPRAC_PWR_CLK_STATUS_GX_HM_CLK_OFF)); } -static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index) +void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp) { - struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu); - struct adreno_gpu *adreno_gpu = _gpu->base; - struct msm_gpu *gpu = _gpu->base; - int ret; + struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); + struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu); + struct a6xx_gmu *gmu = _gpu->gmu; + u32 perf_index; + unsigned long gpu_freq; + int ret = 0; + + gpu_freq = dev_pm_opp_get_freq(opp); + + if (gpu_freq == gmu->freq) + return; + + for (perf_index = 0; perf_index < gmu->nr_gpu_freqs - 1; perf_index++) + if (gpu_freq == gmu->gpu_freqs[perf_index]) + break; + + gmu->current_perf_index = perf_index; gmu_write(gmu, REG_A6XX_GMU_DCVS_ACK_OPTION, 0); gmu_write(gmu, REG_A6XX_GMU_DCVS_PERF_SETTING, - ((3 & 0xf) << 28) | index); + ((3 & 0xf) << 28) | perf_index); /* * Send an invalid index as a vote for the bus bandwidth and let the @@ -126,7 +139,7 @@ static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index) if (ret) dev_err(gmu->dev, "GMU set GPU frequency error: %d\n", ret); - gmu->freq = gmu->gpu_freqs[index]; + gmu->freq = gmu->gpu_freqs[perf_index]; /* * Eventually we will want to scale the path vote with the frequency but @@ -135,25 +148,6 @@ static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index) icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216)); } -void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq) -{ - struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); - struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu); - struct a6xx_gmu *gmu = _gpu->gmu; - u32 perf_index = 0; - - if (freq == gmu->freq) - return; - - for (perf_index = 0; perf_index < gmu->nr_gpu_freqs - 1; perf_index++) - if (freq == gmu->gpu_freqs[perf_index]) - break; - - gmu->current_perf_index = perf_index; - - __a6xx_gmu_set_freq(gmu, perf_index); -} - unsigned long a6xx_gmu_get_freq(struct msm_gpu *gpu) { struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); @@ -708,6 +702,19 @@ static void a6xx_gmu_force_off(struct a6xx_gmu *gmu) a6xx_gmu_rpmh_off(gmu); } +static void a6xx_gmu_set_initial_freq(struct msm_gpu *gpu, struct a6xx_gmu *gmu) +{ + struct dev_pm_opp *gpu_opp; + unsigned long gpu_freq = gmu->gpu_freqs[gmu->current_perf_index]; + + gpu_opp = dev_pm_opp_find_freq_exact(>pdev->dev, gpu_freq, true); + if (IS_ERR_OR_NULL(gpu_opp)) + return; + + a6xx_gmu_set_freq(gpu, gpu_opp); + dev_pm_opp_put(gpu_opp); +} + int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) { struct adreno_gpu *adreno_gpu = _gpu->base; @@ -759,8 +766,7 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) gmu_write(gmu, REG_A6XX_GMU_GMU2HOST_INTR_MASK, ~A6XX_HFI_IRQ_MASK); enable_irq(gmu->hfi_irq); - /* Set the GPU to the current freq */ - __a6xx_gmu_set_freq(gmu, gmu->current_perf_index); + a6xx_gmu_set_initial_freq(gpu, gmu); /* * "enable" the GX power domain which won't actually do anything but it diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h index 7239b8b..03ba60d 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h @@ -63,7 +63,7 @@ void a6xx_gmu_clear_oob(struct a6xx_gmu *gmu, enum a6xx_gmu_oob_state state); int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node); void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu); -void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq); +void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp); unsigned long a6xx_gmu_get_freq(struct msm_gpu *gpu); void
[PATCH v3 0/6] Add support for GPU DDR BW scaling
This is a respin of [1]. Incorported review feedback and fixed issues observed during testing. Picked up the Georgi's series from opp/linux-next [2], and this series is also dependent on a helper function needed to set and clear ddr bandwidth vote [3]. Patch number 4 in the series adds support for SDM845 as well but its not tested yet(WIP), but the SC7180 patches are well tested now. [1] https://patchwork.freedesktop.org/series/75291/ [2] https://kernel.googlesource.com/pub/scm/linux/kernel/git/vireshk/pm/+log/opp/linux-next/ [3] https://patchwork.kernel.org/patch/11590563/ Sharat Masetty (6): dt-bindings: drm/msm/gpu: Document gpu opp table drm: msm: a6xx: send opp instead of a frequency drm: msm: a6xx: use dev_pm_opp_set_bw to scale DDR arm64: dts: qcom: SDM845: Enable GPU DDR bw scaling arm64: dts: qcom: sc7180: Add interconnects property for GPU arm64: dts: qcom: sc7180: Add opp-peak-kBps to GPU opp .../devicetree/bindings/display/msm/gpu.txt| 28 +++ arch/arm64/boot/dts/qcom/sc7180.dtsi | 9 +++ arch/arm64/boot/dts/qcom/sdm845.dtsi | 9 +++ drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 85 +- drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 2 +- drivers/gpu/drm/msm/adreno/adreno_gpu.c| 8 -- drivers/gpu/drm/msm/msm_gpu.c | 3 +- drivers/gpu/drm/msm/msm_gpu.h | 5 +- 8 files changed, 100 insertions(+), 49 deletions(-) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 206987] [drm] [amdgpu] Whole system crashes when the driver is in mode_support_and_system_configuration
https://bugzilla.kernel.org/show_bug.cgi?id=206987 --- Comment #27 from yao...@protonmail.com --- Created attachment 289535 --> https://bugzilla.kernel.org/attachment.cgi?id=289535=edit systemd journal from crash Update: got a whole system crash again when I was starting up SteamVR. So I guess the issue wasn't resolved for me. It could have reduced the likelihood maybe, or it was luck? Not sure what else to attach here, but I copied journal entries from the time of the crash (which happens at 21:09:31 near the end). Let me know if there's something else I should attach the next time this happens, if more data would be helpful. -- 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
Re: [PATCH 53/59] drm/arc: Move to drm/tiny
Hi Eugeniy, Thanks for testing. I looked at the second one (I hoped it would just magically disappear) and I still don't understand what's going on there. My patch series isn't touching that area at all, so really confused. I squashed in the bugfix from the previous round into the right patches, and pushed a branch with just the arcpgu changes here: https://cgit.freedesktop.org/~danvet/drm/log/?h=for-eugeniy Maybe it's something in my pile of not-so-tested stuff :-) Can you pls test this? And if it still fails, try to bisect where it breaks? Thanks, Daniel On Thu, Jun 4, 2020 at 9:00 PM Eugeniy Paltsev wrote: > > I've tested your change and one issue gone. > > However I still see kernel crash (due to invalid read in kernel mode by 0x0 > address) on weston stop: > --->8--- > Oops > Path: (null) > CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted > 5.7.0-rc6-01594-g4ceda91a4176-dirty #6 > Workqueue: events drm_mode_rmfb_work_fn > Invalid Read @ 0x by insn @ drm_gem_fb_destroy+0x32/0x130 > ECR: 0x00050100 EFA: 0x ERET: 0x813b9a76 > STAT32: 0x80080602 [IE K ] BTA: 0x813b9a72 > BLK: drm_gem_fb_destroy+0xc0/0x130 > SP: 0x9f055ea4 FP: 0x > LPS: 0x813560ec LPE: 0x813560f0 LPC: 0x > r00: 0x r01: 0x9f6a6100 r02: 0x0001 > r03: 0x9fd5dde8 r04: 0x810f5de8 r05: 0x > r06: 0x r07: 0x r08: 0x00e1 > r09: 0x r10: 0x r11: 0x00e1 > r12: 0x813b9b04 > > Stack Trace: > drm_gem_fb_destroy+0x32/0x130 > drm_framebuffer_remove+0x1d2/0x358 > drm_mode_rmfb_work_fn+0x28/0x38 > process_one_work+0x19a/0x358 > worker_thread+0x2c4/0x494 > kthread+0xec/0x100 > ret_from_fork+0x18/0x1c > --->8--- > > > The stack traces may vary but always end in drm_gem_fb_destroy: > --->8--- > Stack Trace: > drm_gem_fb_destroy+0x32/0x130 > drm_mode_rmfb+0x10e/0x148 > drm_ioctl_kernel+0x70/0xa0 > drm_ioctl+0x284/0x410 > ksys_ioctl+0xea/0xa3c > EV_Trap+0xcc/0xd0 > --->8--- > Stack Trace: > drm_gem_fb_destroy+0x32/0x130 > drm_fb_release+0x66/0xb0 > drm_file_free.part.11+0x112/0x1bc > drm_release+0x80/0x120 > __fput+0x98/0x1bc > task_work_run+0x6e/0xa8 > do_exit+0x2b4/0x7fc > do_group_exit+0x2a/0x8c > get_signal+0x9a/0x5f0 > do_signal+0x86/0x23c > resume_user_mode_begin+0x88/0xd0 > --->8--- > > > --- > Eugeniy Paltsev > > > > From: Daniel Vetter > Sent: Thursday, June 4, 2020 14:19 > To: Eugeniy Paltsev > Cc: Intel Graphics Development; DRI Development; Daniel Vetter; Sam Ravnborg; > Alexey Brodkin > Subject: Re: [PATCH 53/59] drm/arc: Move to drm/tiny > > Hi Eugeniy, > > Apologies, somehow I missed your mail. I looked at the code again, and I > think I fumbled something. Does the below diff help to prevent the issues? > > Thanks, Daniel > > > diff --git a/drivers/gpu/drm/tiny/arcpgu.c b/drivers/gpu/drm/tiny/arcpgu.c > index 857812f25bec..33d812a5ad7f 100644 > --- a/drivers/gpu/drm/tiny/arcpgu.c > +++ b/drivers/gpu/drm/tiny/arcpgu.c > @@ -228,6 +228,9 @@ static void arc_pgu_update(struct drm_simple_display_pipe > *pipe, > struct arcpgu_drm_private *arcpgu; > struct drm_gem_cma_object *gem; > > + if (!pipe->plane.state->fb) > + return; > + > arcpgu = pipe_to_arcpgu_priv(pipe); > gem = drm_fb_cma_get_gem_obj(pipe->plane.state->fb, 0); > arc_pgu_write(arcpgu, ARCPGU_REG_BUF0_ADDR, gem->paddr); > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - > https://urldefense.com/v3/__http://blog.ffwll.ch__;!!A4F2R9G_pg!P0EvyJfMuDwqbeZmHZM5S9po30QWr4KgGrggRirNfgo7wrRXfnUO-8iq0AA4fQCW2WGPlDc$ -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[drm-intel:drm-intel-next-queued 1/7] drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:933:21: error: unused function 'unmask_page'
tree: git://anongit.freedesktop.org/drm-intel drm-intel-next-queued head: 84d24cb5247a356a4310a25761f8aa56b8814538 commit: 9e0f9464e2ab36b864359a59b0e9058fdef0ce47 [1/7] drm/i915/gem: Async GPU relocations only config: x86_64-randconfig-a011-20200605 (attached as .config) compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 6dd738e2f0609f7d3313b574a1d471263d2d3ba1) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu git checkout 9e0f9464e2ab36b864359a59b0e9058fdef0ce47 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>, old ones prefixed by <<): >> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:933:21: error: unused >> function 'unmask_page' [-Werror,-Wunused-function] static inline void *unmask_page(unsigned long p) ^ >> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:938:28: error: unused >> function 'unmask_flags' [-Werror,-Wunused-function] static inline unsigned int unmask_flags(unsigned long p) ^ >> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:945:33: error: unused >> function 'cache_to_ggtt' [-Werror,-Wunused-function] static inline struct i915_ggtt *cache_to_ggtt(struct reloc_cache *cache) ^ 3 errors generated. vim +/unmask_page +933 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c 5032d871f7d300 drivers/gpu/drm/i915/i915_gem_execbuffer.c Rafael Barbalho 2013-08-21 932 d50415cc6c8395 drivers/gpu/drm/i915/i915_gem_execbuffer.c Chris Wilson 2016-08-18 @933 static inline void *unmask_page(unsigned long p) d50415cc6c8395 drivers/gpu/drm/i915/i915_gem_execbuffer.c Chris Wilson 2016-08-18 934 { d50415cc6c8395 drivers/gpu/drm/i915/i915_gem_execbuffer.c Chris Wilson 2016-08-18 935 return (void *)(uintptr_t)(p & PAGE_MASK); d50415cc6c8395 drivers/gpu/drm/i915/i915_gem_execbuffer.c Chris Wilson 2016-08-18 936 } d50415cc6c8395 drivers/gpu/drm/i915/i915_gem_execbuffer.c Chris Wilson 2016-08-18 937 d50415cc6c8395 drivers/gpu/drm/i915/i915_gem_execbuffer.c Chris Wilson 2016-08-18 @938 static inline unsigned int unmask_flags(unsigned long p) d50415cc6c8395 drivers/gpu/drm/i915/i915_gem_execbuffer.c Chris Wilson 2016-08-18 939 { d50415cc6c8395 drivers/gpu/drm/i915/i915_gem_execbuffer.c Chris Wilson 2016-08-18 940 return p & ~PAGE_MASK; d50415cc6c8395 drivers/gpu/drm/i915/i915_gem_execbuffer.c Chris Wilson 2016-08-18 941 } d50415cc6c8395 drivers/gpu/drm/i915/i915_gem_execbuffer.c Chris Wilson 2016-08-18 942 d50415cc6c8395 drivers/gpu/drm/i915/i915_gem_execbuffer.c Chris Wilson 2016-08-18 943 #define KMAP 0x4 /* after CLFLUSH_FLAGS */ d50415cc6c8395 drivers/gpu/drm/i915/i915_gem_execbuffer.c Chris Wilson 2016-08-18 944 650bc63568e421 drivers/gpu/drm/i915/i915_gem_execbuffer.c Chris Wilson 2017-06-15 @945 static inline struct i915_ggtt *cache_to_ggtt(struct reloc_cache *cache) 650bc63568e421 drivers/gpu/drm/i915/i915_gem_execbuffer.c Chris Wilson 2017-06-15 946 { 650bc63568e421 drivers/gpu/drm/i915/i915_gem_execbuffer.c Chris Wilson 2017-06-15 947 struct drm_i915_private *i915 = 650bc63568e421 drivers/gpu/drm/i915/i915_gem_execbuffer.c Chris Wilson 2017-06-15 948 container_of(cache, struct i915_execbuffer, reloc_cache)->i915; 650bc63568e421 drivers/gpu/drm/i915/i915_gem_execbuffer.c Chris Wilson 2017-06-15 949 return >ggtt; 650bc63568e421 drivers/gpu/drm/i915/i915_gem_execbuffer.c Chris Wilson 2017-06-15 950 } 650bc63568e421 drivers/gpu/drm/i915/i915_gem_execbuffer.c Chris Wilson 2017-06-15 951 :: The code at line 933 was first introduced by commit :: d50415cc6c8395602052b39a1a39290fba3d313e drm/i915: Refactor execbuffer relocation writing :: TO: Chris Wilson :: CC: Chris Wilson --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 0/8] drm: rcar-du: Add Color Management Module (CMM)
On Fri, Jun 05, 2020 at 03:29:00PM +0200, Jacopo Mondi wrote: > Hello Eugeniu, >sorry for the late reply > > On Wed, May 27, 2020 at 09:15:55AM +0200, Eugeniu Rosca wrote: > > Hi Jacopo, > > > > On Tue, Oct 15, 2019 at 12:46:13PM +0200, Jacopo Mondi wrote: > > 8<--- > > > > > * Testing > > > I have tested by injecting a color inversion LUT table and > > > enabling/disabling it > > > every 50 displayed frames: > > > https://jmondi.org/cgit/kmsxx/log/?h=gamma_lut > > > > Could you kindly share the cross compilation steps for your kmsxx fork? > > I usually build it on the target :) Otherwise, cross-compilation instructions are available in README.md. $ mkdir build $ cd build $ cmake -DCMAKE_TOOLCHAIN_FILE=/output/host/usr/share/buildroot/toolchainfile.cmake .. $ make -j4 I assume you may use yocto instead of buildroot, you would have to locate the toolchainfile.cmake file accordingly. > > Just out of curiosity, have you ever tried to pull the display's HDMI > > cable while reading from CM2_LUT_TBL? > > Ahem, not really :) Did I get you right, you mean disconnecting the > HDMI cable from the board ? > > > At least with the out-of-tree CMM implementation [*], this sends the > > R-Car3 reference targets into an unrecoverable freeze, with no lockup > > reported by the kernel (i.e. looks like an serious HW issue). > > > > > > > > CMM functionalities are retained between suspend/resume cycles (tested > > > with > > > suspend-to-idle) without requiring a re-programming of the LUT tables. > > > > Hmm. Is this backed up by any statement in the HW User's manual? > > This comes in contrast with the original Renesas CMM implementation [**] > > which does make use of suspend (where the freeze actually happens). > > > > Can we infer, based on your statement, that we could also get rid of > > the suspend callback in [**]? > > As Geert (thanks) explained what I've tested with is suspend-to-idle, > which retains the state of the LUT tables (and I assume other > not-yet-implemented CMM features, like CLU). I recall the out-of-tree > driver has suspend/resume routines but I never really tested that. > > > [*] https://github.com/renesas-rcar/du_cmm > > [**] > > https://github.com/renesas-rcar/du_cmm/blob/c393ed49834bdbc/meta-rcar-gen3/recipes-kernel/linux/linux-renesas/0001-drm-rcar-du-Add-DU-CMM-support.patch#L1912 -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/6] drm/ttm: Add unampping of the entire device address space
Am 05.06.20 um 16:29 schrieb Andrey Grodzovsky: On 5/11/20 2:45 AM, Christian König wrote: Am 09.05.20 um 20:51 schrieb Andrey Grodzovsky: Signed-off-by: Andrey Grodzovsky --- drivers/gpu/drm/ttm/ttm_bo.c | 22 +- include/drm/ttm/ttm_bo_driver.h | 2 ++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index c5b516f..eae61cc 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -1750,9 +1750,29 @@ void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo) ttm_bo_unmap_virtual_locked(bo); ttm_mem_io_unlock(man); } +EXPORT_SYMBOL(ttm_bo_unmap_virtual); +void ttm_bo_unmap_virtual_address_space(struct ttm_bo_device *bdev) +{ + struct ttm_mem_type_manager *man; + int i; -EXPORT_SYMBOL(ttm_bo_unmap_virtual); + for (i = 0; i < TTM_NUM_MEM_TYPES; i++) { + man = >man[i]; + if (man->has_type && man->use_type) + ttm_mem_io_lock(man, false); + } You should drop that it will just result in a deadlock warning for Nouveau and has no effect at all. Apart from that looks good to me, Christian. As I am considering to re-include this in V2 of the patchsets, can you clarify please why this will have no effect at all ? The locks are exclusive for Nouveau to allocate/free the io address space. Since we don't do this here we don't need the locks. Christian. Andrey + + unmap_mapping_range(bdev->dev_mapping, 0, 0 , 1); + /*TODO What about ttm_mem_io_free_vm(bo) ? */ + + for (i = TTM_NUM_MEM_TYPES - 1; i >= 0; i--) { + man = >man[i]; + if (man->has_type && man->use_type) + ttm_mem_io_unlock(man); + } +} +EXPORT_SYMBOL(ttm_bo_unmap_virtual_address_space); int ttm_bo_wait(struct ttm_buffer_object *bo, bool interruptible, bool no_wait) diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h index c9e0fd0..3133463 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -600,6 +600,8 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev, */ void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo); +void ttm_bo_unmap_virtual_address_space(struct ttm_bo_device *bdev); + /** * ttm_bo_unmap_virtual * ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm: document how user-space should use link-status
> > > > I'm not 100% sure the paragraph about not resetting link-status or not > > using ALLOW_MODESET is accurate. Just like the previous version, this > > is just an attempt at documenting the current kernel behaviour. > > > > drivers/gpu/drm/drm_connector.c | 20 > > 1 file changed, 20 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_connector.c > > b/drivers/gpu/drm/drm_connector.c > > index f2b20fd66319..1df036b3353b 100644 > > --- a/drivers/gpu/drm/drm_connector.c > > +++ b/drivers/gpu/drm/drm_connector.c > > @@ -994,6 +994,26 @@ static const struct drm_prop_enum_list > > dp_colorspaces[] = { > > * after modeset, the kernel driver may set this to "BAD" and issue a > > * hotplug uevent. Drivers should update this value using > > * drm_connector_set_link_status_property(). > > + * > > + * When user-space receives the hotplug uevent and detects a "BAD" > > + * link-status, the sink doesn't receive pixels anymore (e.g. the > > screen > > + * becomes completely black). The list of available modes may have > > + * changed. User-space is expected to pick a new mode if the current > > one > > + * has disappeared and perform a new modeset with link-status set to > > + * "GOOD" to re-enable the connector. > > + * > > + * If multiple connectors share the same CRTC and one of them gets a > > "BAD" > > + * link-status, the other are unaffected (ie. the sinks still > > continue to > > + * receive pixels). > > + * > > + * When user-space performs an atomic commit on a connector with a > > "BAD" > > + * link-status without resetting the property to "GOOD", the sink may > > + * still not receive pixels. When user-space performs an atomic commit > > + * which resets the link-status property to "GOOD" without the > > + * ALLOW_MODESET flag set, it might fail because a modeset is > > required. > > + * > > + * User-space can only change link-status to "GOOD", changing it to > > "BAD" > > + * is a no-op. > > Above reads well and is accurate I think. I think we should add > another paragraph about the backwards compatibility hack in SETCRTC > ioctl: > > "For backwards compatibility with non-atomic userspace the kernel > tries to automatically set the link-status back to "GOOD" in the > SETCRTC IOCTL. This might fail if the mode is no longer valid, similar > to how it might fail if a different screen has been connected in the > interim." > > With that or similar this has my r-b. Sounds good to me too. A-b stands. Thanks, pq pgphraw5ogSW7.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 004/105] clk: bcm: Add BCM2711 DVP driver
On Wed, May 27, 2020 at 8:49 AM Maxime Ripard wrote: > > The HDMI block has a block that controls clocks and reset signals to the > HDMI0 and HDMI1 controllers. > > Let's expose that through a clock driver implementing a clock and reset > provider. > > Cc: Michael Turquette > Cc: Stephen Boyd > Cc: Rob Herring > Cc: linux-...@vger.kernel.org > Cc: devicet...@vger.kernel.org > Reviewed-by: Stephen Boyd > Signed-off-by: Maxime Ripard > --- > drivers/clk/bcm/Kconfig | 11 +++- > drivers/clk/bcm/Makefile | 1 +- > drivers/clk/bcm/clk-bcm2711-dvp.c | 127 +++- > 3 files changed, 139 insertions(+) > create mode 100644 drivers/clk/bcm/clk-bcm2711-dvp.c > > diff --git a/drivers/clk/bcm/Kconfig b/drivers/clk/bcm/Kconfig > index 8c83977a7dc4..784f12c72365 100644 > --- a/drivers/clk/bcm/Kconfig > +++ b/drivers/clk/bcm/Kconfig > @@ -1,4 +1,15 @@ > # SPDX-License-Identifier: GPL-2.0-only > + > +config CLK_BCM2711_DVP > + tristate "Broadcom BCM2711 DVP support" > + depends on ARCH_BCM2835 ||COMPILE_TEST > + depends on COMMON_CLK > + default ARCH_BCM2835 > + select RESET_SIMPLE > + help > + Enable common clock framework support for the Broadcom BCM2711 > + DVP Controller. > + > config CLK_BCM2835 > bool "Broadcom BCM2835 clock support" > depends on ARCH_BCM2835 || ARCH_BRCMSTB || COMPILE_TEST > diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile > index 0070ddf6cdd2..2c1349062147 100644 > --- a/drivers/clk/bcm/Makefile > +++ b/drivers/clk/bcm/Makefile > @@ -6,6 +6,7 @@ obj-$(CONFIG_CLK_BCM_KONA) += clk-kona-setup.o > obj-$(CONFIG_CLK_BCM_KONA) += clk-bcm281xx.o > obj-$(CONFIG_CLK_BCM_KONA) += clk-bcm21664.o > obj-$(CONFIG_COMMON_CLK_IPROC) += clk-iproc-armpll.o clk-iproc-pll.o > clk-iproc-asiu.o > +obj-$(CONFIG_CLK_BCM2835) += clk-bcm2711-dvp.o > obj-$(CONFIG_CLK_BCM2835) += clk-bcm2835.o > obj-$(CONFIG_CLK_BCM2835) += clk-bcm2835-aux.o > obj-$(CONFIG_CLK_RASPBERRYPI) += clk-raspberrypi.o I do think that single driver is the right model here, but I noticed that you're not using your new CONFIG_ symbol. With that fixed, r-b from me. (though I'd also recommend devm_platform_get_and_ioremap_resource and devm_reset_controller_register()) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/dsi: use stack buffer in mipi_dsi_dcs_write()
On Thu, May 28, 2020 at 05:47:41PM +0100, Emil Velikov wrote: > On Tue, 5 May 2020 at 17:05, Emil Velikov wrote: > > > > Currently the function heap allocates when we have any payload. Where in > > many case the payload is 1 byte - ouch. > > > > From casual observation, vast majority of the payloads are smaller than > > 8 bytes - so use a stack array tx[8] to avoid the senseless kmalloc and > > kfree dance. > > > > Cc: Jani Nikula > > Cc: Thierry Reding > > Signed-off-by: Emil Velikov > > --- > > drivers/gpu/drm/drm_mipi_dsi.c | 20 ++-- > > 1 file changed, 10 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c > > index 55531895dde6..b96d5b4629d7 100644 > > --- a/drivers/gpu/drm/drm_mipi_dsi.c > > +++ b/drivers/gpu/drm/drm_mipi_dsi.c > > @@ -748,26 +748,26 @@ ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device > > *dsi, u8 cmd, > > { > > ssize_t err; > > size_t size; > > + u8 stack_tx[8]; > > u8 *tx; > > > > - if (len > 0) { > > - size = 1 + len; > > - > > + size = 1 + len; > > + if (len > ARRAY_SIZE(stack_tx) - 1) { > > tx = kmalloc(size, GFP_KERNEL); > > if (!tx) > > return -ENOMEM; > > - > > - /* concatenate the DCS command byte and the payload */ > > - tx[0] = cmd; > > - memcpy([1], data, len); > > } else { > > - tx = > > - size = 1; > > + tx = stack_tx; > > } > > > > + /* concatenate the DCS command byte and the payload */ > > + tx[0] = cmd; > > + if (data) > > + memcpy([1], data, len); > > + > > err = mipi_dsi_dcs_write_buffer(dsi, tx, size); > > > > - if (len > 0) > > + if (tx != stack_tx) > > kfree(tx); > > > > return err; > > -- > > Thierry, others - humble ping. > Can you check through the series? I don't see patch 2 of this series anywhere? Did it fall victim to some filter? Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/mipi: use dcs write for mipi_dsi_dcs_set_tear_scanline
On Tue, May 05, 2020 at 05:03:29PM +0100, Emil Velikov wrote: > From: Emil Velikov > > The helper uses the MIPI_DCS_SET_TEAR_SCANLINE, although it's currently > using the generic write. This does not look right. > > Perhaps some platforms don't distinguish between the two writers? I don't think platforms usually care about this level of detail. The Tegra driver for example doesn't really look at the packet type and just packets the data in the standard DSI format and then sends it off on the bus. It does inspect some fields of the packet, but none that are related to the packet type, I think. So it's possible that the panel will accept the packet irrespective of type and handle it correctly. I can imagine that the decoding logic in these panels might be rather primitive, so perhaps it's not very strict as to what exactly the type is as long as it can do something with the data. In any case, it does make sense to send DCS commands using the DCS type, so I'd say let's merge this and see if somebody complains: Reviewed-by: Thierry Reding > Cc: Robert Chiras > Cc: Vinay Simha BN > Cc: Jani Nikula > Cc: Thierry Reding > Fixes: e83950816367 ("drm/dsi: Implement set tear scanline") > Signed-off-by: Emil Velikov > --- > Robert, can you please test this against the only user - the Raydium > RM67191 panel driver that you introduced. > > Thanks > > Vinay, can you confirm if this is a genuine typo or there's something > really subtle happening. > --- > drivers/gpu/drm/drm_mipi_dsi.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c > index b96d5b4629d7..07102d8da58f 100644 > --- a/drivers/gpu/drm/drm_mipi_dsi.c > +++ b/drivers/gpu/drm/drm_mipi_dsi.c > @@ -1082,11 +1082,11 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format); > */ > int mipi_dsi_dcs_set_tear_scanline(struct mipi_dsi_device *dsi, u16 scanline) > { > - u8 payload[3] = { MIPI_DCS_SET_TEAR_SCANLINE, scanline >> 8, > - scanline & 0xff }; > + u8 payload[2] = { scanline >> 8, scanline & 0xff }; > ssize_t err; > > - err = mipi_dsi_generic_write(dsi, payload, sizeof(payload)); > + err = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_TEAR_SCANLINE, payload, > + sizeof(payload)); > if (err < 0) > return err; > > -- > 2.25.1 > signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/dsi: use stack buffer in mipi_dsi_dcs_write()
On Tue, May 05, 2020 at 05:03:27PM +0100, Emil Velikov wrote: > Currently the function heap allocates when we have any payload. Where in > many case the payload is 1 byte - ouch. > > From casual observation, vast majority of the payloads are smaller than > 8 bytes - so use a stack array tx[8] to avoid the senseless kmalloc and > kfree dance. > > Cc: Jani Nikula > Cc: Thierry Reding > Signed-off-by: Emil Velikov > --- > drivers/gpu/drm/drm_mipi_dsi.c | 20 ++-- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c > index 55531895dde6..b96d5b4629d7 100644 > --- a/drivers/gpu/drm/drm_mipi_dsi.c > +++ b/drivers/gpu/drm/drm_mipi_dsi.c > @@ -748,26 +748,26 @@ ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, > u8 cmd, > { > ssize_t err; > size_t size; > + u8 stack_tx[8]; > u8 *tx; > > - if (len > 0) { > - size = 1 + len; > - > + size = 1 + len; > + if (len > ARRAY_SIZE(stack_tx) - 1) { I think it would be clearer to do: if (size > ARRAY_SIZE(stack_tx)) but either way: Reviewed-by: Thierry Reding signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 10/14] drm/mgag200: Move device init and cleanup to mgag200_drv.c
Hi Sam Am 05.06.20 um 16:45 schrieb Sam Ravnborg: > On Fri, Jun 05, 2020 at 03:57:59PM +0200, Thomas Zimmermann wrote: >> Moving the initializer and cleanup functions for device instances >> to mgag200_drv.c prepares for the conversion to managed code. No >> functional changes are made. Remove mgag200_main.c, which is now >> empty. > > The functions are still named xx_load() - which comes from old days > where drm_driver has a load callback. > We can always fight about naming, but I am just left with the impression > that better naming exists today. ... and is available in patch 11. :) The driver will have mgag200_device_create() to allocate and initialize a device. The cleanup is done automatically, so a _destroy() function is not necessary. My first attempt on this series was 8 or 9 patches long. I split up some of them to make the conversion easier to follow and more logical. Occasionally I had to keep obsolete artifacts, such as load and unload, in a patch if it benefits the series as a whole. Best regards Thomas > Just a drive by comment - feel free to ignore. > > Sam > >> >> Signed-off-by: Thomas Zimmermann >> --- >> drivers/gpu/drm/mgag200/Makefile | 3 +- >> drivers/gpu/drm/mgag200/mgag200_drv.c | 68 +++ >> drivers/gpu/drm/mgag200/mgag200_drv.h | 4 -- >> drivers/gpu/drm/mgag200/mgag200_main.c | 77 -- >> 4 files changed, 69 insertions(+), 83 deletions(-) >> delete mode 100644 drivers/gpu/drm/mgag200/mgag200_main.c >> >> diff --git a/drivers/gpu/drm/mgag200/Makefile >> b/drivers/gpu/drm/mgag200/Makefile >> index e6a933874a88c..42fedef538826 100644 >> --- a/drivers/gpu/drm/mgag200/Makefile >> +++ b/drivers/gpu/drm/mgag200/Makefile >> @@ -1,5 +1,4 @@ >> # SPDX-License-Identifier: GPL-2.0-only >> -mgag200-y := mgag200_drv.o mgag200_i2c.o mgag200_main.o mgag200_mm.o \ >> - mgag200_mode.o >> +mgag200-y := mgag200_drv.o mgag200_i2c.o mgag200_mm.o mgag200_mode.o >> >> obj-$(CONFIG_DRM_MGAG200) += mgag200.o >> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c >> b/drivers/gpu/drm/mgag200/mgag200_drv.c >> index ad74e02d8f251..f8bb24199643d 100644 >> --- a/drivers/gpu/drm/mgag200/mgag200_drv.c >> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c >> @@ -39,6 +39,74 @@ static struct drm_driver mgag200_driver = { >> DRM_GEM_SHMEM_DRIVER_OPS, >> }; >> >> +/* >> + * DRM device >> + */ >> + >> +static int mgag200_driver_load(struct drm_device *dev, unsigned long flags) >> +{ >> +struct mga_device *mdev; >> +int ret, option; >> + >> +mdev = devm_kzalloc(dev->dev, sizeof(struct mga_device), GFP_KERNEL); >> +if (mdev == NULL) >> +return -ENOMEM; >> +dev->dev_private = (void *)mdev; >> +mdev->dev = dev; >> + >> +mdev->flags = mgag200_flags_from_driver_data(flags); >> +mdev->type = mgag200_type_from_driver_data(flags); >> + >> +pci_read_config_dword(dev->pdev, PCI_MGA_OPTION, ); >> +mdev->has_sdram = !(option & (1 << 14)); >> + >> +/* BAR 0 is the framebuffer, BAR 1 contains registers */ >> +mdev->rmmio_base = pci_resource_start(dev->pdev, 1); >> +mdev->rmmio_size = pci_resource_len(dev->pdev, 1); >> + >> +if (!devm_request_mem_region(dev->dev, mdev->rmmio_base, >> + mdev->rmmio_size, "mgadrmfb_mmio")) { >> +drm_err(dev, "can't reserve mmio registers\n"); >> +return -ENOMEM; >> +} >> + >> +mdev->rmmio = pcim_iomap(dev->pdev, 1, 0); >> +if (mdev->rmmio == NULL) >> +return -ENOMEM; >> + >> +/* stash G200 SE model number for later use */ >> +if (IS_G200_SE(mdev)) { >> +mdev->unique_rev_id = RREG32(0x1e24); >> +drm_dbg(dev, "G200 SE unique revision id is 0x%x\n", >> +mdev->unique_rev_id); >> +} >> + >> +ret = mgag200_mm_init(mdev); >> +if (ret) >> +goto err_mm; >> + >> +ret = mgag200_modeset_init(mdev); >> +if (ret) { >> +drm_err(dev, "Fatal error during modeset init: %d\n", ret); >> +goto err_mm; >> +} >> + >> +return 0; >> + >> +err_mm: >> +dev->dev_private = NULL; >> +return ret; >> +} >> + >> +static void mgag200_driver_unload(struct drm_device *dev) >> +{ >> +struct mga_device *mdev = to_mga_device(dev); >> + >> +if (mdev == NULL) >> +return; >> +dev->dev_private = NULL; >> +} >> + >> /* >> * PCI driver >> */ >> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h >> b/drivers/gpu/drm/mgag200/mgag200_drv.h >> index 7b6e6827a9a21..b38e5ce4ee20b 100644 >> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h >> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h >> @@ -188,10 +188,6 @@ mgag200_flags_from_driver_data(kernel_ulong_t >> driver_data) >> /* mgag200_mode.c */ >> int mgag200_modeset_init(struct mga_device *mdev); >> >> -/* mgag200_main.c */ >> -int mgag200_driver_load(struct
Re: [PATCH v1 4/6] drm: panel-simple: add Hitachi 3.5" QVGA panel
Hi Doug. On Mon, Jun 01, 2020 at 05:30:53PM -0700, Doug Anderson wrote: > Hi, > > On Mon, Jun 1, 2020 at 1:33 AM Sam Ravnborg wrote: > > > > This panel is used on evaluation boards for Atmel at91sam9261 and > > at91sam6263. > > > > Signed-off-by: Sam Ravnborg > > Cc: Thierry Reding > > Cc: Sam Ravnborg > > --- > > drivers/gpu/drm/panel/panel-simple.c | 29 > > 1 file changed, 29 insertions(+) > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > > b/drivers/gpu/drm/panel/panel-simple.c > > index 8624bb80108c..25c96639631f 100644 > > --- a/drivers/gpu/drm/panel/panel-simple.c > > +++ b/drivers/gpu/drm/panel/panel-simple.c > > @@ -1813,6 +1813,32 @@ static const struct panel_desc hannstar_hsd100pxn1 = > > { > > .connector_type = DRM_MODE_CONNECTOR_LVDS, > > }; > > > > +static const struct drm_display_mode hitachi_tx09d71vm1cca_mode = { > > + .clock = 4965000, > > This is the pixel clock in kHz, right? So it runs at almost 5 Terahertz? > > > > + .hdisplay = 240, > > + .hsync_start = 240 + 0, > > + .hsync_end = 240 + 0 + 5, > > + .htotal = 240 + 0 + 5 + 1, > > + .vdisplay = 320, > > + .vsync_start = 320 + 0, > > + .vsync_end = 320 + 0 + 1, > > + .vtotal = 320 + 0 + 1 + 1, > > Some random datasheet I found has really different numbers. If the > numbers above are what everyone is using then I suppose it's fine, > just curious why the mismatch. The timing comes from: arch/arm/boot/dts/at91sam9263ek.dts - that include display timings for the panel on the evaluation board. I did not verify any datasheet - I just blindly copied what was there. And clock was obviously not properly adjusted to khz. Will fix in v2 - will also try to find a datasheet this time. Thanks for noticing! Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 2/6] drm: panel-simple: add Seiko 70WVW2T 7" simple panel
Hi Doug. On Mon, Jun 01, 2020 at 05:31:06PM -0700, Doug Anderson wrote: > Hi, > > On Mon, Jun 1, 2020 at 1:33 AM Sam Ravnborg wrote: > > > > The Seiko 70WVW2T is a discontinued product, but may be used somewhere. > > Tested on a proprietary product. > > > > Signed-off-by: Sam Ravnborg > > Cc: Søren Andersen > > Cc: Thierry Reding > > Cc: Sam Ravnborg > > --- > > drivers/gpu/drm/panel/panel-simple.c | 28 > > 1 file changed, 28 insertions(+) > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > > b/drivers/gpu/drm/panel/panel-simple.c > > index b067f66cea0e..8624bb80108c 100644 > > --- a/drivers/gpu/drm/panel/panel-simple.c > > +++ b/drivers/gpu/drm/panel/panel-simple.c > > @@ -3194,6 +3194,31 @@ static const struct panel_desc > > shelly_sca07010_bfn_lnn = { > > .bus_format = MEDIA_BUS_FMT_RGB666_1X18, > > }; > > > > +static const struct drm_display_mode sii_70wvw2t_mode = { > > + .clock = 33000, > > + .hdisplay = 800, > > + .hsync_start = 800 + 256, > > + .hsync_end = 800 + 256 + 0, > > + .htotal = 800 + 256 + 0 + 0, > > + .vdisplay = 480, > > + .vsync_start = 480 + 0, > > + .vsync_end = 480 + 0 + 0, > > + .vtotal = 480 + 0 + 0 + 45, > > Important to have a "vrefresh"? > > > > + .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC, > > +}; > > + > > +static const struct panel_desc sii_70wvw2t = { > > + .modes = _70wvw2t_mode, > > + .num_modes = 1, > > Do we want "bpc = 6"? > > > > + .size = { > > + .width = 152, > > + .height = 91, > > + }, > > + .bus_format = MEDIA_BUS_FMT_RGB888_1X24, > > Should this be a 666 format? Random internet-found data sheet says > 262K colors... Thanks for catching this! You are indeed right, this is MEDIA_BUS_FMT_RGB666_1X18 and only bpc = 6. My bad excuse is that other displays for the same HW is RGB888 and bpc = 8. Will fix and repost. Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 07/14] drm/mgag200: Switch to managed MM
On Fri, Jun 05, 2020 at 03:57:56PM +0200, Thomas Zimmermann wrote: > The memory-management code now cleans up automatically as part of > device destruction. > > Signed-off-by: Thomas Zimmermann > --- > drivers/gpu/drm/mgag200/mgag200_drv.h | 1 - > drivers/gpu/drm/mgag200/mgag200_main.c | 5 + > drivers/gpu/drm/mgag200/mgag200_mm.c | 28 ++ > 3 files changed, 16 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h > b/drivers/gpu/drm/mgag200/mgag200_drv.h > index cd786ffe319b8..7b6e6827a9a21 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_drv.h > +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h > @@ -198,6 +198,5 @@ void mgag200_i2c_destroy(struct mga_i2c_chan *i2c); > > /* mgag200_mm.c */ > int mgag200_mm_init(struct mga_device *mdev); > -void mgag200_mm_fini(struct mga_device *mdev); > > #endif /* __MGAG200_DRV_H__ */ > diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c > b/drivers/gpu/drm/mgag200/mgag200_main.c > index e9ad783c2b44d..49bcdfcb40a4e 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_main.c > +++ b/drivers/gpu/drm/mgag200/mgag200_main.c > @@ -57,13 +57,11 @@ int mgag200_driver_load(struct drm_device *dev, unsigned > long flags) > ret = mgag200_modeset_init(mdev); > if (ret) { > drm_err(dev, "Fatal error during modeset init: %d\n", ret); > - goto err_mgag200_mm_fini; > + goto err_mm; > } > > return 0; > > -err_mgag200_mm_fini: > - mgag200_mm_fini(mdev); > err_mm: > dev->dev_private = NULL; > return ret; > @@ -75,6 +73,5 @@ void mgag200_driver_unload(struct drm_device *dev) > > if (mdev == NULL) > return; > - mgag200_mm_fini(mdev); > dev->dev_private = NULL; > } > diff --git a/drivers/gpu/drm/mgag200/mgag200_mm.c > b/drivers/gpu/drm/mgag200/mgag200_mm.c > index f56b0456994f4..1b1e5ec5d1ceb 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_mm.c > +++ b/drivers/gpu/drm/mgag200/mgag200_mm.c > @@ -28,6 +28,8 @@ > > #include > > +#include > + > #include "mgag200_drv.h" > > static size_t mgag200_probe_vram(struct mga_device *mdev, void __iomem *mem, > @@ -73,6 +75,18 @@ static size_t mgag200_probe_vram(struct mga_device *mdev, > void __iomem *mem, > return offset - 65536; > } > > +static void mgag200_mm_release(struct drm_device *dev, void *ptr) > +{ > + struct mga_device *mdev = to_mga_device(dev); > + > + mdev->vram_fb_available = 0; > + iounmap(mdev->vram); > + arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0), > + pci_resource_len(dev->pdev, 0)); > + arch_phys_wc_del(mdev->fb_mtrr); > + mdev->fb_mtrr = 0; > +} > + > int mgag200_mm_init(struct mga_device *mdev) > { > struct drm_device *dev = mdev->dev; > @@ -104,22 +118,10 @@ int mgag200_mm_init(struct mga_device *mdev) > > mdev->vram_fb_available = mdev->mc.vram_size; > > - return 0; > + return drmm_add_action_or_reset(dev, mgag200_mm_release, NULL); > > err_arch_phys_wc_del: > arch_phys_wc_del(mdev->fb_mtrr); > arch_io_free_memtype_wc(start, len); Btw I think devm versions of these two would benefit a bunch of drivers in their cleanup code. devm_ not drmm_ since it's hw resource cleanup. In case you ever run out of ideas :-) Cheeres, Daniel > return ret; > } > - > -void mgag200_mm_fini(struct mga_device *mdev) > -{ > - struct drm_device *dev = mdev->dev; > - > - mdev->vram_fb_available = 0; > - iounmap(mdev->vram); > - arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0), > - pci_resource_len(dev->pdev, 0)); > - arch_phys_wc_del(mdev->fb_mtrr); > - mdev->fb_mtrr = 0; > -} > -- > 2.26.2 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/panel: simple: Add support for KOE TX26D202VM0BWA panel
Hi Emil. > > + .width = 217, > > + .height = 136, > > + }, > > + .delay = { > > + .prepare = 1000, > > + .enable = 1000, > > + .unprepare = 1000, > > + .disable = 1000, > Ouch 1s for each delay is huge. Nevertheless it matches the specs so, > the series is: > Reviewed-by: Emil Velikov > > Sam, Thierry I assume you'll merge the series. Let me know if I should > pick it up. I went ahead and applied both patches to drm-misc-next. They are now pushed out. Sam > > -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/2] drm/panel: simple: Add a KOE WUXGA 10.1" LVDS panel support
On Mon, Jun 01, 2020 at 02:09:35PM +0800, Liu Ying wrote: > This patch set adds a KOE WUXGA 10.1" LVDS panel support. > The panel type is TX26D202VM0BWA. > The panel has dual LVDS channels. > > My panel is manufactured by US Micro Products(USMP). There is a tag at > the back of the panel, which indicates the panel type is 'TX26D202VM0BWA' > and it's made by KOE in Taiwan. > > The panel spec from USMP can be found at: > https://www.usmicroproducts.com/sites/default/files/datasheets/USMP-T101-192120NDU-A0.pdf > > The below panel spec from KOE is basically the same to the one from USMP. > However, the panel type 'TX26D202VM0BAA' is a little bit different. > It looks that the two types of panel are compatible with each other. > http://www.koe.j-display.com/upload/product/TX26D202VM0BAA.pdf > > Patch 1/2 adds compatible for the panel in the panel-simple DT binding doc. > Patch 2/2 adds the panel support in the DRM panel-simple driver. Thanks, both patches applied to drm-misc-next. Sam > > Liu Ying (2): > dt-bindings: panel-simple: Add koe,tx26d202vm0bwa compatible > drm/panel: simple: Add support for KOE TX26D202VM0BWA panel > > .../bindings/display/panel/panel-simple.yaml | 2 ++ > drivers/gpu/drm/panel/panel-simple.c | 34 > ++ > 2 files changed, 36 insertions(+) > > -- > 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 00/43] Convert most CMA-based drivers to GEM object functions
Hi Thomas, On Fri, 5 Jun 2020 at 08:33, Thomas Zimmermann wrote: > > Most of the CMA-based drivers use the default implementation for the > callbacks in struct drm_driver. With this patch, these interfaces are > initialized with a common helper macro and GEM object functions replace > several deprecated interfaces. > > To address Laurent's comment on the amount of changes per patch, I re- > organized the series. > > 1) There are now DRIVER_OPS macros for drivers that require a virtual > address on imported buffers, and macros for drivers that don't. Therefore, > drivers don't switch to drm_gem_cma_prime_import_sg_table_vmap() > implicitly when they begin using the DRIVER_OPS macro. > > 2) I split each driver's patch into two: the first converts the driver to > GEM CMA object functions, the second introduces the DRIVER_OPS macro. > Neither patch should result in a functional change. I kept existing R-b > and A-b tags on both patches. Existing Tested-by tags are only on the > final patch, as that's closest to what has been tested. > > 3) I dropped the conversion to drm_gem_prime_mmap() from the patchset. As > part of this change, the CMA object functions could provide an mmap > function, which is worth it's own series. The patch for aspeed requires > drm_gem_prime_mmap(), so I removed it from the series. > > Patches 1 to 3 update the existing macro and helper to similar naming as with > SHMEM, add a new DRIVER_OPS macro, and add helpers for drivers that override > the default implementation for .dumb_create(). The remaining patches up to > the final one convert the drivers. The kirin driver also changes to the > default dumb_create function. The final patch deletes .gem_print_info from > struct drm_driver. > > I don't have much of the hardware, so it's only compile-tested on aarch64, > arm and x86-64 only. Several patches have Tested-by tags. > For the whole updated series: Reviewed-by: Emil Velikov -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 20/22] drm: mxsfb: Merge mxsfb_set_pixel_fmt() and mxsfb_set_bus_fmt()
Hi Laurent, With the previous disclaimer in mind, the series is: Reviewed-by: Emil Velikov There's a small suggestion inline, for future work. On Sat, 30 May 2020 at 04:11, Laurent Pinchart wrote: > switch (bus_format) { > case MEDIA_BUS_FMT_RGB565_1X16: > - reg |= CTRL_BUS_WIDTH_16; > + ctrl |= CTRL_BUS_WIDTH_16; > break; > case MEDIA_BUS_FMT_RGB666_1X18: > - reg |= CTRL_BUS_WIDTH_18; > + ctrl |= CTRL_BUS_WIDTH_18; > break; > case MEDIA_BUS_FMT_RGB888_1X24: > - reg |= CTRL_BUS_WIDTH_24; > + ctrl |= CTRL_BUS_WIDTH_24; > break; > default: > dev_err(drm->dev, "Unknown media bus format %d\n", > bus_format); > break; Off the top of my head, the default case should be handled in the atomic_check() hook. That is, unless I'm missing something and one can change the bus format in between atomic_check() and atomic_enable(). Which would sound like a very odd thing to do. -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/14] drm/mgag200: Use managed interfaces for auto-cleanup
Hi Thomas. On Fri, Jun 05, 2020 at 03:57:49PM +0200, Thomas Zimmermann wrote: > This patchset cleans up mgag200 device initialization, embeds the > DRM device instance in struct mga_device and finally converts device > initialization to managed interfaces. > > Patches 1 and 2 are actually unrelated. Both remove artifacts that got > lost from earlier patch series. We're fixing this before introducing new > changes. > > Patches 3 to 7 cleanup the memory management code and convert it to > managed. Specifically, all MM code is being moved into a the same file. > That makes it more obvious what's going on and will allow for further > cleanups later on. > > Modesetting is already cleaned up automatically, so there's nothing > to do here. > > With modesetting and MM done, patches 8 to 14 convert the device > initialization to managed interfaces. The allocation is split among > several functions and we move it to the same place in patches 11 and > 12. That is also a good opportunity to embed the DRM device instance > in struct mga_device in patch 13. Patch 14 adds managed release of the > device structure. > > Tested on Matrox G200SE HW. > > Thomas Zimmermann (14): > drm/mgag200: Remove declaration of mgag200_mmap() from header file > drm/mgag200: Remove mgag200_cursor.c > drm/mgag200: Use pcim_enable_device() > drm/mgag200: Rename mgag200_ttm.c to mgag200_mm.c > drm/mgag200: Lookup VRAM PCI BAR start and length only once > drm/mgag200: Merge VRAM setup into MM initialization > drm/mgag200: Switch to managed MM > drm/mgag200: Separate DRM and PCI functionality from each other > drm/mgag200: Prefix global names in mgag200_drv.c with mgag200_ > drm/mgag200: Move device init and cleanup to mgag200_drv.c > drm/mgag200: Separate device initialization into allocation > drm/mgag200: Allocate device structures in mgag200_driver_load() > drm/mgag200: Embed instance of struct drm_device in struct mga_device > drm/mgag200: Use managed device initialization Looked through all patches. A few triggered some small comments. With the comments addressed all patches are: Acked-by: Sam Ravnborg My comments can, if any chenges are required, be addressed when applying. No need for a next round. Sam > > drivers/gpu/drm/mgag200/Makefile | 3 +- > drivers/gpu/drm/mgag200/mgag200_cursor.c | 319 -- > drivers/gpu/drm/mgag200/mgag200_drv.c | 161 ++--- > drivers/gpu/drm/mgag200/mgag200_drv.h | 11 +- > drivers/gpu/drm/mgag200/mgag200_main.c| 155 - > .../mgag200/{mgag200_ttm.c => mgag200_mm.c} | 99 -- > drivers/gpu/drm/mgag200/mgag200_mode.c| 12 +- > 7 files changed, 195 insertions(+), 565 deletions(-) > delete mode 100644 drivers/gpu/drm/mgag200/mgag200_cursor.c > delete mode 100644 drivers/gpu/drm/mgag200/mgag200_main.c > rename drivers/gpu/drm/mgag200/{mgag200_ttm.c => mgag200_mm.c} (51%) > > -- > 2.26.2 > > > Thomas Zimmermann (14): > drm/mgag200: Remove declaration of mgag200_mmap() from header file > drm/mgag200: Remove mgag200_cursor.c > drm/mgag200: Use pcim_enable_device() > drm/mgag200: Rename mgag200_ttm.c to mgag200_mm.c > drm/mgag200: Lookup VRAM PCI BAR start and length only once > drm/mgag200: Merge VRAM setup into MM initialization > drm/mgag200: Switch to managed MM > drm/mgag200: Separate DRM and PCI functionality from each other > drm/mgag200: Prefix global names in mgag200_drv.c with mgag200_ > drm/mgag200: Move device init and cleanup to mgag200_drv.c > drm/mgag200: Separate device initialization into allocation > drm/mgag200: Allocate device structures in mgag200_driver_load() > drm/mgag200: Embed instance of struct drm_device in struct mga_device > drm/mgag200: Use managed device initialization > > drivers/gpu/drm/mgag200/Makefile | 3 +- > drivers/gpu/drm/mgag200/mgag200_cursor.c | 319 -- > drivers/gpu/drm/mgag200/mgag200_drv.c | 161 ++--- > drivers/gpu/drm/mgag200/mgag200_drv.h | 11 +- > drivers/gpu/drm/mgag200/mgag200_main.c| 155 - > .../mgag200/{mgag200_ttm.c => mgag200_mm.c} | 99 -- > drivers/gpu/drm/mgag200/mgag200_mode.c| 12 +- > 7 files changed, 195 insertions(+), 565 deletions(-) > delete mode 100644 drivers/gpu/drm/mgag200/mgag200_cursor.c > delete mode 100644 drivers/gpu/drm/mgag200/mgag200_main.c > rename drivers/gpu/drm/mgag200/{mgag200_ttm.c => mgag200_mm.c} (51%) > > -- > 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 10/14] drm/mgag200: Move device init and cleanup to mgag200_drv.c
On Fri, Jun 05, 2020 at 03:57:59PM +0200, Thomas Zimmermann wrote: > Moving the initializer and cleanup functions for device instances > to mgag200_drv.c prepares for the conversion to managed code. No > functional changes are made. Remove mgag200_main.c, which is now > empty. The functions are still named xx_load() - which comes from old days where drm_driver has a load callback. We can always fight about naming, but I am just left with the impression that better naming exists today. Just a drive by comment - feel free to ignore. Sam > > Signed-off-by: Thomas Zimmermann > --- > drivers/gpu/drm/mgag200/Makefile | 3 +- > drivers/gpu/drm/mgag200/mgag200_drv.c | 68 +++ > drivers/gpu/drm/mgag200/mgag200_drv.h | 4 -- > drivers/gpu/drm/mgag200/mgag200_main.c | 77 -- > 4 files changed, 69 insertions(+), 83 deletions(-) > delete mode 100644 drivers/gpu/drm/mgag200/mgag200_main.c > > diff --git a/drivers/gpu/drm/mgag200/Makefile > b/drivers/gpu/drm/mgag200/Makefile > index e6a933874a88c..42fedef538826 100644 > --- a/drivers/gpu/drm/mgag200/Makefile > +++ b/drivers/gpu/drm/mgag200/Makefile > @@ -1,5 +1,4 @@ > # SPDX-License-Identifier: GPL-2.0-only > -mgag200-y := mgag200_drv.o mgag200_i2c.o mgag200_main.o mgag200_mm.o \ > -mgag200_mode.o > +mgag200-y := mgag200_drv.o mgag200_i2c.o mgag200_mm.o mgag200_mode.o > > obj-$(CONFIG_DRM_MGAG200) += mgag200.o > diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c > b/drivers/gpu/drm/mgag200/mgag200_drv.c > index ad74e02d8f251..f8bb24199643d 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_drv.c > +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c > @@ -39,6 +39,74 @@ static struct drm_driver mgag200_driver = { > DRM_GEM_SHMEM_DRIVER_OPS, > }; > > +/* > + * DRM device > + */ > + > +static int mgag200_driver_load(struct drm_device *dev, unsigned long flags) > +{ > + struct mga_device *mdev; > + int ret, option; > + > + mdev = devm_kzalloc(dev->dev, sizeof(struct mga_device), GFP_KERNEL); > + if (mdev == NULL) > + return -ENOMEM; > + dev->dev_private = (void *)mdev; > + mdev->dev = dev; > + > + mdev->flags = mgag200_flags_from_driver_data(flags); > + mdev->type = mgag200_type_from_driver_data(flags); > + > + pci_read_config_dword(dev->pdev, PCI_MGA_OPTION, ); > + mdev->has_sdram = !(option & (1 << 14)); > + > + /* BAR 0 is the framebuffer, BAR 1 contains registers */ > + mdev->rmmio_base = pci_resource_start(dev->pdev, 1); > + mdev->rmmio_size = pci_resource_len(dev->pdev, 1); > + > + if (!devm_request_mem_region(dev->dev, mdev->rmmio_base, > + mdev->rmmio_size, "mgadrmfb_mmio")) { > + drm_err(dev, "can't reserve mmio registers\n"); > + return -ENOMEM; > + } > + > + mdev->rmmio = pcim_iomap(dev->pdev, 1, 0); > + if (mdev->rmmio == NULL) > + return -ENOMEM; > + > + /* stash G200 SE model number for later use */ > + if (IS_G200_SE(mdev)) { > + mdev->unique_rev_id = RREG32(0x1e24); > + drm_dbg(dev, "G200 SE unique revision id is 0x%x\n", > + mdev->unique_rev_id); > + } > + > + ret = mgag200_mm_init(mdev); > + if (ret) > + goto err_mm; > + > + ret = mgag200_modeset_init(mdev); > + if (ret) { > + drm_err(dev, "Fatal error during modeset init: %d\n", ret); > + goto err_mm; > + } > + > + return 0; > + > +err_mm: > + dev->dev_private = NULL; > + return ret; > +} > + > +static void mgag200_driver_unload(struct drm_device *dev) > +{ > + struct mga_device *mdev = to_mga_device(dev); > + > + if (mdev == NULL) > + return; > + dev->dev_private = NULL; > +} > + > /* > * PCI driver > */ > diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h > b/drivers/gpu/drm/mgag200/mgag200_drv.h > index 7b6e6827a9a21..b38e5ce4ee20b 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_drv.h > +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h > @@ -188,10 +188,6 @@ mgag200_flags_from_driver_data(kernel_ulong_t > driver_data) > /* mgag200_mode.c */ > int mgag200_modeset_init(struct mga_device *mdev); > > - /* mgag200_main.c */ > -int mgag200_driver_load(struct drm_device *dev, unsigned long flags); > -void mgag200_driver_unload(struct drm_device *dev); > - > /* mgag200_i2c.c */ > struct mga_i2c_chan *mgag200_i2c_create(struct drm_device *dev); > void mgag200_i2c_destroy(struct mga_i2c_chan *i2c); > diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c > b/drivers/gpu/drm/mgag200/mgag200_main.c > deleted file mode 100644 > index 49bcdfcb40a4e..0 > --- a/drivers/gpu/drm/mgag200/mgag200_main.c > +++ /dev/null > @@ -1,77 +0,0 @@ > -// SPDX-License-Identifier: GPL-2.0-only > -/* > - * Copyright 2010 Matt Turner. > - * Copyright 2012
Re: [PATCH 09/14] drm/mgag200: Prefix global names in mgag200_drv.c with mgag200_
On Fri, Jun 05, 2020 at 03:57:58PM +0200, Thomas Zimmermann wrote: > The naming of global symbols in mgag200_drv.c is inconsistent. Fix > that by prefixing all names with mgag200_. Hmm, static symbols are hardly global symbols. Patch is fine, but changelog seems a litte off. Sam > > Signed-off-by: Thomas Zimmermann > --- > drivers/gpu/drm/mgag200/mgag200_drv.c | 20 ++-- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c > b/drivers/gpu/drm/mgag200/mgag200_drv.c > index 670e12d57dea8..ad74e02d8f251 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_drv.c > +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c > @@ -27,7 +27,7 @@ module_param_named(modeset, mgag200_modeset, int, 0400); > > DEFINE_DRM_GEM_FOPS(mgag200_driver_fops); > > -static struct drm_driver driver = { > +static struct drm_driver mgag200_driver = { > .driver_features = DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET, > .fops = _driver_fops, > .name = DRIVER_NAME, > @@ -43,7 +43,7 @@ static struct drm_driver driver = { > * PCI driver > */ > > -static const struct pci_device_id pciidlist[] = { > +static const struct pci_device_id mgag200_pciidlist[] = { > { PCI_VENDOR_ID_MATROX, 0x522, PCI_ANY_ID, PCI_ANY_ID, 0, 0, > G200_SE_A | MGAG200_FLAG_HW_BUG_NO_STARTADD}, > { PCI_VENDOR_ID_MATROX, 0x524, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_B > }, > @@ -56,10 +56,10 @@ static const struct pci_device_id pciidlist[] = { > {0,} > }; > > -MODULE_DEVICE_TABLE(pci, pciidlist); > +MODULE_DEVICE_TABLE(pci, mgag200_pciidlist); > > - > -static int mga_pci_probe(struct pci_dev *pdev, const struct pci_device_id > *ent) > +static int > +mgag200_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > { > struct drm_device *dev; > int ret; > @@ -70,7 +70,7 @@ static int mga_pci_probe(struct pci_dev *pdev, const struct > pci_device_id *ent) > if (ret) > return ret; > > - dev = drm_dev_alloc(, >dev); > + dev = drm_dev_alloc(_driver, >dev); > if (IS_ERR(dev)) > return PTR_ERR(dev); > > @@ -96,7 +96,7 @@ static int mga_pci_probe(struct pci_dev *pdev, const struct > pci_device_id *ent) > return ret; > } > > -static void mga_pci_remove(struct pci_dev *pdev) > +static void mgag200_pci_remove(struct pci_dev *pdev) > { > struct drm_device *dev = pci_get_drvdata(pdev); > > @@ -107,9 +107,9 @@ static void mga_pci_remove(struct pci_dev *pdev) > > static struct pci_driver mgag200_pci_driver = { > .name = DRIVER_NAME, > - .id_table = pciidlist, > - .probe = mga_pci_probe, > - .remove = mga_pci_remove, > + .id_table = mgag200_pciidlist, > + .probe = mgag200_pci_probe, > + .remove = mgag200_pci_remove, > }; > > static int __init mgag200_init(void) > -- > 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dma-fence: basic lockdep annotations
On 6/5/20 3:29 PM, Daniel Vetter wrote: Design is similar to the lockdep annotations for workers, but with some twists: - We use a read-lock for the execution/worker/completion side, so that this explicit annotation can be more liberally sprinkled around. With read locks lockdep isn't going to complain if the read-side isn't nested the same way under all circumstances, so ABBA deadlocks are ok. Which they are, since this is an annotation only. - We're using non-recursive lockdep read lock mode, since in recursive read lock mode lockdep does not catch read side hazards. And we _very_ much want read side hazards to be caught. For full details of this limitation see commit e91498589746065e3ae95d9a00b068e525eec34f Author: Peter Zijlstra Date: Wed Aug 23 13:13:11 2017 +0200 locking/lockdep/selftests: Add mixed read-write ABBA tests - To allow nesting of the read-side explicit annotations we explicitly keep track of the nesting. lock_is_held() allows us to do that. - The wait-side annotation is a write lock, and entirely done within dma_fence_wait() for everyone by default. - To be able to freely annotate helper functions I want to make it ok to call dma_fence_begin/end_signalling from soft/hardirq context. First attempt was using the hardirq locking context for the write side in lockdep, but this forces all normal spinlocks nested within dma_fence_begin/end_signalling to be spinlocks. That bollocks. The approach now is to simple check in_atomic(), and for these cases entirely rely on the might_sleep() check in dma_fence_wait(). That will catch any wrong nesting against spinlocks from soft/hardirq contexts. The idea here is that every code path that's critical for eventually signalling a dma_fence should be annotated with dma_fence_begin/end_signalling. The annotation ideally starts right after a dma_fence is published (added to a dma_resv, exposed as a sync_file fd, attached to a drm_syncobj fd, or anything else that makes the dma_fence visible to other kernel threads), up to and including the dma_fence_wait(). Examples are irq handlers, the scheduler rt threads, the tail of execbuf (after the corresponding fences are visible), any workers that end up signalling dma_fences and really anything else. Not annotated should be code paths that only complete fences opportunistically as the gpu progresses, like e.g. shrinker/eviction code. The main class of deadlocks this is supposed to catch are: Thread A: mutex_lock(A); mutex_unlock(A); dma_fence_signal(); Thread B: mutex_lock(A); dma_fence_wait(); mutex_unlock(A); Thread B is blocked on A signalling the fence, but A never gets around to that because it cannot acquire the lock A. Note that dma_fence_wait() is allowed to be nested within dma_fence_begin/end_signalling sections. To allow this to happen the read lock needs to be upgraded to a write lock, which means that any other lock is acquired between the dma_fence_begin_signalling() call and the call to dma_fence_wait(), and still held, this will result in an immediate lockdep complaint. The only other option would be to not annotate such calls, defeating the point. Therefore these annotations cannot be sprinkled over the code entirely mindless to avoid false positives. Originally I hope that the cross-release lockdep extensions would alleviate the need for explicit annotations: https://lwn.net/Articles/709849/ But there's a few reasons why that's not an option: - It's not happening in upstream, since it got reverted due to too many false positives: commit e966eaeeb623f09975ef362c2866fae6f86844f9 Author: Ingo Molnar Date: Tue Dec 12 12:31:16 2017 +0100 locking/lockdep: Remove the cross-release locking checks This code (CONFIG_LOCKDEP_CROSSRELEASE=y and CONFIG_LOCKDEP_COMPLETIONS=y), while it found a number of old bugs initially, was also causing too many false positives that caused people to disable lockdep - which is arguably a worse overall outcome. - cross-release uses the complete() call to annotate the end of critical sections, for dma_fence that would be dma_fence_signal(). But we do not want all dma_fence_signal() calls to be treated as critical, since many are opportunistic cleanup of gpu requests. If these get stuck there's still the main completion interrupt and workers who can unblock everyone. Automatically annotating all dma_fence_signal() calls would hence cause false positives. - cross-release had some educated guesses for when a critical section starts, like fresh syscall or fresh work callback. This would again cause false positives without explicit annotations, since for dma_fence the critical sections only starts when we publish a fence. - Furthermore there can be cases where a thread never does a dma_fence_signal, but is still
Re: [PATCH v3 01/43] drm/cma-helper: Rename symbols from drm_cma_gem_ to drm_gem_cma_
Hi Am 05.06.20 um 16:23 schrieb Sam Ravnborg: > On Fri, Jun 05, 2020 at 04:15:46PM +0200, Thomas Zimmermann wrote: >> Hi >> >> Am 05.06.20 um 10:40 schrieb Laurent Pinchart: >>> Hi Thomas, >>> >>> Thank you for the patch. >>> >>> On Fri, Jun 05, 2020 at 09:32:05AM +0200, Thomas Zimmermann wrote: This fixes the naming of several symbols within CMA helpers. No functional changes are made. Signed-off-by: Thomas Zimmermann >>> >>> Thank you for the patch. >>> >>> Speaking of naming, I wish we could rename drm_gem_cma_* to something >>> else, as those helpers don't use CMA directly (and may not use it at >>> all), but I think that would be too much intrusive changes for too >>> little gain :-( >> >> I agree. Calling them GEM DMA helpers would be more precise. But I don't >> really see an easy solution without either a big patch that touches >> everything, or a lot of small patches with ugly intermediate states. >> >> IMHO the best option would probably be an additional header file >> drm_gem_dma_helper.h that defines a dma name for each cma name. Drivers >> could then be converted individually with a single per-driver patch. > > From todo.rst: > > " > Rename CMA helpers to DMA helpers > - > > CMA (standing for contiguous memory allocator) is really a bit an accident of > what these were used for first, a much better name would be DMA helpers. In > the > text these should even be called coherent DMA memory helpers (so maybe CDM, > but > no one knows what that means) since underneath they just use > dma_alloc_coherent. > > Contact: Laurent Pinchart, Daniel Vetter > > Level: Intermediate (mostly because it is a huge tasks without good partial > milestones, not technically itself that challenging) > " > > The same? Yes, I've been thinking of this. That would be a good task for someone to get familiar with the DRM code. Best regards Thomas > > Sam > >> >> Best regards >> Thomas >> >>> --- drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 2 +- drivers/gpu/drm/drm_gem_cma_helper.c| 10 +- include/drm/drm_gem_cma_helper.h| 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c index 6b27242b9ee3c..5e7ea0459d018 100644 --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c @@ -188,7 +188,7 @@ DEFINE_DRM_GEM_CMA_FOPS(fops); static struct drm_driver aspeed_gfx_driver = { .driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, - .gem_create_object = drm_cma_gem_create_object_default_funcs, + .gem_create_object = drm_gem_cma_create_object_default_funcs, .dumb_create= drm_gem_cma_dumb_create, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c index b3db3ca7bd7a7..842e2fa332354 100644 --- a/drivers/gpu/drm/drm_gem_cma_helper.c +++ b/drivers/gpu/drm/drm_gem_cma_helper.c @@ -572,7 +572,7 @@ void drm_gem_cma_prime_vunmap(struct drm_gem_object *obj, void *vaddr) } EXPORT_SYMBOL_GPL(drm_gem_cma_prime_vunmap); -static const struct drm_gem_object_funcs drm_cma_gem_default_funcs = { +static const struct drm_gem_object_funcs drm_gem_cma_default_funcs = { .free = drm_gem_cma_free_object, .print_info = drm_gem_cma_print_info, .get_sg_table = drm_gem_cma_prime_get_sg_table, @@ -581,7 +581,7 @@ static const struct drm_gem_object_funcs drm_cma_gem_default_funcs = { }; /** - * drm_cma_gem_create_object_default_funcs - Create a CMA GEM object with a + * drm_gem_cma_create_object_default_funcs - Create a CMA GEM object with a * default function table * @dev: DRM device * @size: Size of the object to allocate @@ -593,7 +593,7 @@ static const struct drm_gem_object_funcs drm_cma_gem_default_funcs = { * A pointer to a allocated GEM object or an error pointer on failure. */ struct drm_gem_object * -drm_cma_gem_create_object_default_funcs(struct drm_device *dev, size_t size) +drm_gem_cma_create_object_default_funcs(struct drm_device *dev, size_t size) { struct drm_gem_cma_object *cma_obj; @@ -601,11 +601,11 @@ drm_cma_gem_create_object_default_funcs(struct drm_device *dev, size_t size) if (!cma_obj) return NULL; - cma_obj->base.funcs = _cma_gem_default_funcs; + cma_obj->base.funcs = _gem_cma_default_funcs; return _obj->base; }
Re: [Intel-gfx] [PATCH] drm/i915: Fix comments mentioning typo in IS_ENABLED()
Quoting Kees Cook (2020-06-05 15:19:53) > This has no code changes, but the typo is clearly getting copy/pasted, > so better to avoid this now and fix the typo. IS_ENABLED() takes full > names, and must have the "CONFIG_" prefix. > > Reported-by: Joe Perches > Link: > https://lore.kernel.org/lkml/b08611018fdb6d88757c6008a5c02fa0e07b32fb.ca...@perches.com > Signed-off-by: Kees Cook Reviewed-by: Chris Wilson -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 06/14] drm/mgag200: Merge VRAM setup into MM initialization
Hi Thomas. Some parts I did not understand here. On Fri, Jun 05, 2020 at 03:57:55PM +0200, Thomas Zimmermann wrote: > The VRAM setup in mgag200_drv.c is part of memory management and > should be done in the same place. Merge the code into the memory > management's init function. > > Signed-off-by: Thomas Zimmermann > --- > drivers/gpu/drm/mgag200/mgag200_main.c | 75 -- > drivers/gpu/drm/mgag200/mgag200_mm.c | 52 ++ > 2 files changed, 52 insertions(+), 75 deletions(-) > > diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c > b/drivers/gpu/drm/mgag200/mgag200_main.c > index 3298eff7bd1b4..e9ad783c2b44d 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_main.c > +++ b/drivers/gpu/drm/mgag200/mgag200_main.c > @@ -12,77 +12,6 @@ > > #include "mgag200_drv.h" > > -static int mga_probe_vram(struct mga_device *mdev, void __iomem *mem) > -{ > - int offset; > - int orig; > - int test1, test2; > - int orig1, orig2; > - unsigned int vram_size; > - > - /* Probe */ > - orig = ioread16(mem); > - iowrite16(0, mem); > - > - vram_size = mdev->mc.vram_window; > - > - if ((mdev->type == G200_EW3) && (vram_size >= 0x100)) { > - vram_size = vram_size - 0x40; > - } > - > - for (offset = 0x10; offset < vram_size; offset += 0x4000) { > - orig1 = ioread8(mem + offset); > - orig2 = ioread8(mem + offset + 0x100); > - > - iowrite16(0xaa55, mem + offset); > - iowrite16(0xaa55, mem + offset + 0x100); > - > - test1 = ioread16(mem + offset); > - test2 = ioread16(mem); > - > - iowrite16(orig1, mem + offset); > - iowrite16(orig2, mem + offset + 0x100); > - > - if (test1 != 0xaa55) { > - break; > - } > - > - if (test2) { > - break; > - } > - } > - > - iowrite16(orig, mem); > - return offset - 65536; > -} > - > -/* Map the framebuffer from the card and configure the core */ > -static int mga_vram_init(struct mga_device *mdev) > -{ > - struct drm_device *dev = mdev->dev; > - void __iomem *mem; > - > - /* BAR 0 is VRAM */ > - mdev->mc.vram_base = pci_resource_start(dev->pdev, 0); > - mdev->mc.vram_window = pci_resource_len(dev->pdev, 0); > - > - if (!devm_request_mem_region(dev->dev, mdev->mc.vram_base, > - mdev->mc.vram_window, "mgadrmfb_vram")) { > - DRM_ERROR("can't reserve VRAM\n"); > - return -ENXIO; > - } > - > - mem = pci_iomap(dev->pdev, 0, 0); > - if (!mem) > - return -ENOMEM; > - > - mdev->mc.vram_size = mga_probe_vram(mdev, mem); > - > - pci_iounmap(dev->pdev, mem); mem is the result of pci_iomap() - but I do not see any call to pci_iomap() in the converted code. mdev->vram is used as argument in new code where mem was used in the old code. mdev->vram comes from ioremap(start, len) - will it result in the same? Sam > - > - return 0; > -} > - > int mgag200_driver_load(struct drm_device *dev, unsigned long flags) > { > struct mga_device *mdev; > @@ -121,10 +50,6 @@ int mgag200_driver_load(struct drm_device *dev, unsigned > long flags) > mdev->unique_rev_id); > } > > - ret = mga_vram_init(mdev); > - if (ret) > - return ret; > - > ret = mgag200_mm_init(mdev); > if (ret) > goto err_mm; > diff --git a/drivers/gpu/drm/mgag200/mgag200_mm.c > b/drivers/gpu/drm/mgag200/mgag200_mm.c > index 73e30901e0631..f56b0456994f4 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_mm.c > +++ b/drivers/gpu/drm/mgag200/mgag200_mm.c > @@ -30,6 +30,49 @@ > > #include "mgag200_drv.h" > > +static size_t mgag200_probe_vram(struct mga_device *mdev, void __iomem *mem, > + size_t size) > +{ > + int offset; > + int orig; > + int test1, test2; > + int orig1, orig2; > + size_t vram_size; > + > + /* Probe */ > + orig = ioread16(mem); > + iowrite16(0, mem); > + > + vram_size = size; > + > + if ((mdev->type == G200_EW3) && (vram_size >= 0x100)) > + vram_size = vram_size - 0x40; > + > + for (offset = 0x10; offset < vram_size; offset += 0x4000) { > + orig1 = ioread8(mem + offset); > + orig2 = ioread8(mem + offset + 0x100); > + > + iowrite16(0xaa55, mem + offset); > + iowrite16(0xaa55, mem + offset + 0x100); > + > + test1 = ioread16(mem + offset); > + test2 = ioread16(mem); > + > + iowrite16(orig1, mem + offset); > + iowrite16(orig2, mem + offset + 0x100); > + > + if (test1 != 0xaa55) > + break; > + > + if (test2) > + break; > + } > + > + iowrite16(orig, mem); > + > + return offset -
Re: [PATCH 1/6] drm/ttm: Add unampping of the entire device address space
On 5/11/20 2:45 AM, Christian König wrote: Am 09.05.20 um 20:51 schrieb Andrey Grodzovsky: Signed-off-by: Andrey Grodzovsky --- drivers/gpu/drm/ttm/ttm_bo.c | 22 +- include/drm/ttm/ttm_bo_driver.h | 2 ++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index c5b516f..eae61cc 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -1750,9 +1750,29 @@ void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo) ttm_bo_unmap_virtual_locked(bo); ttm_mem_io_unlock(man); } +EXPORT_SYMBOL(ttm_bo_unmap_virtual); +void ttm_bo_unmap_virtual_address_space(struct ttm_bo_device *bdev) +{ + struct ttm_mem_type_manager *man; + int i; -EXPORT_SYMBOL(ttm_bo_unmap_virtual); + for (i = 0; i < TTM_NUM_MEM_TYPES; i++) { + man = >man[i]; + if (man->has_type && man->use_type) + ttm_mem_io_lock(man, false); + } You should drop that it will just result in a deadlock warning for Nouveau and has no effect at all. Apart from that looks good to me, Christian. As I am considering to re-include this in V2 of the patchsets, can you clarify please why this will have no effect at all ? Andrey + + unmap_mapping_range(bdev->dev_mapping, 0, 0 , 1); + /*TODO What about ttm_mem_io_free_vm(bo) ? */ + + for (i = TTM_NUM_MEM_TYPES - 1; i >= 0; i--) { + man = >man[i]; + if (man->has_type && man->use_type) + ttm_mem_io_unlock(man); + } +} +EXPORT_SYMBOL(ttm_bo_unmap_virtual_address_space); int ttm_bo_wait(struct ttm_buffer_object *bo, bool interruptible, bool no_wait) diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h index c9e0fd0..3133463 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -600,6 +600,8 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev, */ void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo); +void ttm_bo_unmap_virtual_address_space(struct ttm_bo_device *bdev); + /** * ttm_bo_unmap_virtual * ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 01/43] drm/cma-helper: Rename symbols from drm_cma_gem_ to drm_gem_cma_
On Fri, Jun 05, 2020 at 04:15:46PM +0200, Thomas Zimmermann wrote: > Hi > > Am 05.06.20 um 10:40 schrieb Laurent Pinchart: > > Hi Thomas, > > > > Thank you for the patch. > > > > On Fri, Jun 05, 2020 at 09:32:05AM +0200, Thomas Zimmermann wrote: > >> This fixes the naming of several symbols within CMA helpers. No functional > >> changes are made. > >> > >> Signed-off-by: Thomas Zimmermann > > > > Thank you for the patch. > > > > Speaking of naming, I wish we could rename drm_gem_cma_* to something > > else, as those helpers don't use CMA directly (and may not use it at > > all), but I think that would be too much intrusive changes for too > > little gain :-( > > I agree. Calling them GEM DMA helpers would be more precise. But I don't > really see an easy solution without either a big patch that touches > everything, or a lot of small patches with ugly intermediate states. > > IMHO the best option would probably be an additional header file > drm_gem_dma_helper.h that defines a dma name for each cma name. Drivers > could then be converted individually with a single per-driver patch. >From todo.rst: " Rename CMA helpers to DMA helpers - CMA (standing for contiguous memory allocator) is really a bit an accident of what these were used for first, a much better name would be DMA helpers. In the text these should even be called coherent DMA memory helpers (so maybe CDM, but no one knows what that means) since underneath they just use dma_alloc_coherent. Contact: Laurent Pinchart, Daniel Vetter Level: Intermediate (mostly because it is a huge tasks without good partial milestones, not technically itself that challenging) " The same? Sam > > Best regards > Thomas > > > > >> --- > >> drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 2 +- > >> drivers/gpu/drm/drm_gem_cma_helper.c| 10 +- > >> include/drm/drm_gem_cma_helper.h| 4 ++-- > >> 3 files changed, 8 insertions(+), 8 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c > >> b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c > >> index 6b27242b9ee3c..5e7ea0459d018 100644 > >> --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c > >> +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c > >> @@ -188,7 +188,7 @@ DEFINE_DRM_GEM_CMA_FOPS(fops); > >> > >> static struct drm_driver aspeed_gfx_driver = { > >>.driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, > >> - .gem_create_object = drm_cma_gem_create_object_default_funcs, > >> + .gem_create_object = drm_gem_cma_create_object_default_funcs, > >>.dumb_create= drm_gem_cma_dumb_create, > >>.prime_handle_to_fd = drm_gem_prime_handle_to_fd, > >>.prime_fd_to_handle = drm_gem_prime_fd_to_handle, > >> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c > >> b/drivers/gpu/drm/drm_gem_cma_helper.c > >> index b3db3ca7bd7a7..842e2fa332354 100644 > >> --- a/drivers/gpu/drm/drm_gem_cma_helper.c > >> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c > >> @@ -572,7 +572,7 @@ void drm_gem_cma_prime_vunmap(struct drm_gem_object > >> *obj, void *vaddr) > >> } > >> EXPORT_SYMBOL_GPL(drm_gem_cma_prime_vunmap); > >> > >> -static const struct drm_gem_object_funcs drm_cma_gem_default_funcs = { > >> +static const struct drm_gem_object_funcs drm_gem_cma_default_funcs = { > >>.free = drm_gem_cma_free_object, > >>.print_info = drm_gem_cma_print_info, > >>.get_sg_table = drm_gem_cma_prime_get_sg_table, > >> @@ -581,7 +581,7 @@ static const struct drm_gem_object_funcs > >> drm_cma_gem_default_funcs = { > >> }; > >> > >> /** > >> - * drm_cma_gem_create_object_default_funcs - Create a CMA GEM object with > >> a > >> + * drm_gem_cma_create_object_default_funcs - Create a CMA GEM object with > >> a > >> * default function table > >> * @dev: DRM device > >> * @size: Size of the object to allocate > >> @@ -593,7 +593,7 @@ static const struct drm_gem_object_funcs > >> drm_cma_gem_default_funcs = { > >> * A pointer to a allocated GEM object or an error pointer on failure. > >> */ > >> struct drm_gem_object * > >> -drm_cma_gem_create_object_default_funcs(struct drm_device *dev, size_t > >> size) > >> +drm_gem_cma_create_object_default_funcs(struct drm_device *dev, size_t > >> size) > >> { > >>struct drm_gem_cma_object *cma_obj; > >> > >> @@ -601,11 +601,11 @@ drm_cma_gem_create_object_default_funcs(struct > >> drm_device *dev, size_t size) > >>if (!cma_obj) > >>return NULL; > >> > >> - cma_obj->base.funcs = _cma_gem_default_funcs; > >> + cma_obj->base.funcs = _gem_cma_default_funcs; > >> > >>return _obj->base; > >> } > >> -EXPORT_SYMBOL(drm_cma_gem_create_object_default_funcs); > >> +EXPORT_SYMBOL(drm_gem_cma_create_object_default_funcs); > >> > >> /** > >> * drm_gem_cma_prime_import_sg_table_vmap - PRIME import another driver's > >> diff --git a/include/drm/drm_gem_cma_helper.h > >>
[PATCH] drm/i915: Fix comments mentioning typo in IS_ENABLED()
This has no code changes, but the typo is clearly getting copy/pasted, so better to avoid this now and fix the typo. IS_ENABLED() takes full names, and must have the "CONFIG_" prefix. Reported-by: Joe Perches Link: https://lore.kernel.org/lkml/b08611018fdb6d88757c6008a5c02fa0e07b32fb.ca...@perches.com Signed-off-by: Kees Cook --- drivers/dma-buf/selftests.h | 2 +- drivers/gpu/drm/i915/selftests/i915_live_selftests.h | 2 +- drivers/gpu/drm/i915/selftests/i915_mock_selftests.h | 2 +- drivers/gpu/drm/i915/selftests/i915_perf_selftests.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/dma-buf/selftests.h b/drivers/dma-buf/selftests.h index 55918ef9adab..bc8cea67bf1e 100644 --- a/drivers/dma-buf/selftests.h +++ b/drivers/dma-buf/selftests.h @@ -5,7 +5,7 @@ * a module parameter. It must be unique and legal for a C identifier. * * The function should be of type int function(void). It may be conditionally - * compiled using #if IS_ENABLED(DRM_I915_SELFTEST). + * compiled using #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST). * * Tests are executed in order by igt/dmabuf_selftest */ diff --git a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h index 5dd5d81646c4..e42ea9c6703b 100644 --- a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h +++ b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h @@ -11,7 +11,7 @@ * a module parameter. It must be unique and legal for a C identifier. * * The function should be of type int function(void). It may be conditionally - * compiled using #if IS_ENABLED(DRM_I915_SELFTEST). + * compiled using #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST). * * Tests are executed in order by igt/drv_selftest */ diff --git a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h index 6a2be7d0dd95..4be044198af9 100644 --- a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h +++ b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h @@ -11,7 +11,7 @@ * a module parameter. It must be unique and legal for a C identifier. * * The function should be of type int function(void). It may be conditionally - * compiled using #if IS_ENABLED(DRM_I915_SELFTEST). + * compiled using #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST). * * Tests are executed in order by igt/drv_selftest */ diff --git a/drivers/gpu/drm/i915/selftests/i915_perf_selftests.h b/drivers/gpu/drm/i915/selftests/i915_perf_selftests.h index d8da142985eb..c2389f8a257d 100644 --- a/drivers/gpu/drm/i915/selftests/i915_perf_selftests.h +++ b/drivers/gpu/drm/i915/selftests/i915_perf_selftests.h @@ -11,7 +11,7 @@ * a module parameter. It must be unique and legal for a C identifier. * * The function should be of type int function(void). It may be conditionally - * compiled using #if IS_ENABLED(DRM_I915_SELFTEST). + * compiled using #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST). * * Tests are executed in order by igt/i915_selftest */ -- 2.25.1 -- Kees Cook ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 01/43] drm/cma-helper: Rename symbols from drm_cma_gem_ to drm_gem_cma_
Hi Am 05.06.20 um 10:40 schrieb Laurent Pinchart: > Hi Thomas, > > Thank you for the patch. > > On Fri, Jun 05, 2020 at 09:32:05AM +0200, Thomas Zimmermann wrote: >> This fixes the naming of several symbols within CMA helpers. No functional >> changes are made. >> >> Signed-off-by: Thomas Zimmermann > > Thank you for the patch. > > Speaking of naming, I wish we could rename drm_gem_cma_* to something > else, as those helpers don't use CMA directly (and may not use it at > all), but I think that would be too much intrusive changes for too > little gain :-( I agree. Calling them GEM DMA helpers would be more precise. But I don't really see an easy solution without either a big patch that touches everything, or a lot of small patches with ugly intermediate states. IMHO the best option would probably be an additional header file drm_gem_dma_helper.h that defines a dma name for each cma name. Drivers could then be converted individually with a single per-driver patch. Best regards Thomas > >> --- >> drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 2 +- >> drivers/gpu/drm/drm_gem_cma_helper.c| 10 +- >> include/drm/drm_gem_cma_helper.h| 4 ++-- >> 3 files changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c >> b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c >> index 6b27242b9ee3c..5e7ea0459d018 100644 >> --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c >> +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c >> @@ -188,7 +188,7 @@ DEFINE_DRM_GEM_CMA_FOPS(fops); >> >> static struct drm_driver aspeed_gfx_driver = { >> .driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, >> -.gem_create_object = drm_cma_gem_create_object_default_funcs, >> +.gem_create_object = drm_gem_cma_create_object_default_funcs, >> .dumb_create= drm_gem_cma_dumb_create, >> .prime_handle_to_fd = drm_gem_prime_handle_to_fd, >> .prime_fd_to_handle = drm_gem_prime_fd_to_handle, >> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c >> b/drivers/gpu/drm/drm_gem_cma_helper.c >> index b3db3ca7bd7a7..842e2fa332354 100644 >> --- a/drivers/gpu/drm/drm_gem_cma_helper.c >> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c >> @@ -572,7 +572,7 @@ void drm_gem_cma_prime_vunmap(struct drm_gem_object >> *obj, void *vaddr) >> } >> EXPORT_SYMBOL_GPL(drm_gem_cma_prime_vunmap); >> >> -static const struct drm_gem_object_funcs drm_cma_gem_default_funcs = { >> +static const struct drm_gem_object_funcs drm_gem_cma_default_funcs = { >> .free = drm_gem_cma_free_object, >> .print_info = drm_gem_cma_print_info, >> .get_sg_table = drm_gem_cma_prime_get_sg_table, >> @@ -581,7 +581,7 @@ static const struct drm_gem_object_funcs >> drm_cma_gem_default_funcs = { >> }; >> >> /** >> - * drm_cma_gem_create_object_default_funcs - Create a CMA GEM object with a >> + * drm_gem_cma_create_object_default_funcs - Create a CMA GEM object with a >> * default function table >> * @dev: DRM device >> * @size: Size of the object to allocate >> @@ -593,7 +593,7 @@ static const struct drm_gem_object_funcs >> drm_cma_gem_default_funcs = { >> * A pointer to a allocated GEM object or an error pointer on failure. >> */ >> struct drm_gem_object * >> -drm_cma_gem_create_object_default_funcs(struct drm_device *dev, size_t size) >> +drm_gem_cma_create_object_default_funcs(struct drm_device *dev, size_t size) >> { >> struct drm_gem_cma_object *cma_obj; >> >> @@ -601,11 +601,11 @@ drm_cma_gem_create_object_default_funcs(struct >> drm_device *dev, size_t size) >> if (!cma_obj) >> return NULL; >> >> -cma_obj->base.funcs = _cma_gem_default_funcs; >> +cma_obj->base.funcs = _gem_cma_default_funcs; >> >> return _obj->base; >> } >> -EXPORT_SYMBOL(drm_cma_gem_create_object_default_funcs); >> +EXPORT_SYMBOL(drm_gem_cma_create_object_default_funcs); >> >> /** >> * drm_gem_cma_prime_import_sg_table_vmap - PRIME import another driver's >> diff --git a/include/drm/drm_gem_cma_helper.h >> b/include/drm/drm_gem_cma_helper.h >> index 947ac95eb24a9..64b7e9d42129a 100644 >> --- a/include/drm/drm_gem_cma_helper.h >> +++ b/include/drm/drm_gem_cma_helper.h >> @@ -107,7 +107,7 @@ void *drm_gem_cma_prime_vmap(struct drm_gem_object *obj); >> void drm_gem_cma_prime_vunmap(struct drm_gem_object *obj, void *vaddr); >> >> struct drm_gem_object * >> -drm_cma_gem_create_object_default_funcs(struct drm_device *dev, size_t >> size); >> +drm_gem_cma_create_object_default_funcs(struct drm_device *dev, size_t >> size); >> >> /** >> * DRM_GEM_CMA_VMAP_DRIVER_OPS - CMA GEM driver operations ensuring a >> virtual >> @@ -118,7 +118,7 @@ drm_cma_gem_create_object_default_funcs(struct >> drm_device *dev, size_t size); >> * imported buffers. >> */ >> #define DRM_GEM_CMA_VMAP_DRIVER_OPS \ >> -.gem_create_object = drm_cma_gem_create_object_default_funcs, \ >> +
Re: [v2] drm/msm: add shutdown support for display platform_driver
On Tue, 2 Jun 2020 at 17:10, Sai Prakash Ranjan wrote: > > Hi Emil, > > On 2020-06-02 21:09, Emil Velikov wrote: > > On Tue, 2 Jun 2020 at 15:49, Sai Prakash Ranjan > > wrote: > >> > >> Hi Emil, > >> > >> On 2020-06-02 19:43, Emil Velikov wrote: > >> > Hi Krishna, > >> > > >> > On Tue, 2 Jun 2020 at 08:17, Krishna Manikandan > >> > wrote: > >> >> > >> >> Define shutdown callback for display drm driver, > >> >> so as to disable all the CRTCS when shutdown > >> >> notification is received by the driver. > >> >> > >> >> This change will turn off the timing engine so > >> >> that no display transactions are requested > >> >> while mmu translations are getting disabled > >> >> during reboot sequence. > >> >> > >> >> Signed-off-by: Krishna Manikandan > >> >> > >> > AFAICT atomics is setup in msm_drm_ops::bind and shutdown in > >> > msm_drm_ops::unbind. > >> > > >> > Are you saying that unbind never triggers? If so, then we should > >> > really fix that instead, since this patch seems more like a > >> > workaround. > >> > > >> > >> Which path do you suppose that the unbind should be called from, > >> remove > >> callback? Here we are talking about the drivers which are builtin, > >> where > >> remove callbacks are not called from the driver core during > >> reboot/shutdown, > >> instead shutdown callbacks are called which needs to be defined in > >> order > >> to > >> trigger unbind. So AFAICS there is nothing to be fixed. > >> > > Interesting - in drm effectively only drm panels implement .shutdown. > > So my naive assumption was that .remove was used implicitly by core, > > as part of the shutdown process. Yet that's not the case, so it seems > > that many drivers could use some fixes. > > > > Then again, that's an existing problem which is irrelevant for msm. > > -Emil > > To give more context, we are actually targeting the clients/consumers > of SMMU/IOMMU here because we have to make sure that before the supplier > (SMMU) shuts down, its consumers/clients need to be shutdown properly. > Now the ordering of this is taken care in the SMMU driver via > device_link > which makes sure that consumer shutdown callbacks are called first, but > we > need to define shutdown callbacks for all its consumers to make sure we > actually shutdown the clients or else it would invite the crashes during > reboot > which in this case was seen for display. > Thank you very much for the extra details. I think other DRM drivers will be safe as-is. -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/2] drm: vmwgfx: remove drm_driver::master_set() return typ
On Wed, 3 Jun 2020 at 19:37, Roland Scheidegger wrote: > > Looks like a nice cleanup to me. (No idea if at some point there > actually was a reason for a return value...) > It was required up-to quite recently actually: commit 9c84aeba67cce6514374f134c9ce2c5f35ac3831 Author: Thomas Hellstrom AuthorDate: Tue May 28 08:08:55 2019 +0200 Commit: Thomas Hellstrom CommitDate: Thu Aug 15 08:39:27 2019 +0200 drm/vmwgfx: Kill unneeded legacy security features > Reviewed-by: Roland Scheidegger > Thank you. Will push to drm-misc in a moment. -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 22/22] drm: mxsfb: Support the alpha plane
On Tue, 2 Jun 2020 at 23:43, Laurent Pinchart wrote: > > Hi Emil, > > On Sun, May 31, 2020 at 05:54:04PM +0100, Emil Velikov wrote: > > HI Laurent, > > > > From a quick glance the series looks really good and neat. > > Thank you :-) > > > Then again, I don't know much about the hardware to provide meaningful > > review. > > > > A couple of small ideas below. > > > > On Sat, 30 May 2020 at 04:11, Laurent Pinchart wrote: > > > > > +#define LCDC_AS_BUF0x220 > > > +#define LCDC_AS_NEXT_BUF 0x230 > > > > s/LCDC_AS_BUF/LCDC_AS_CUR_BUF/ - to stay consistent with the primary > > plane name scheme. > > The register names come directly from the datasheet (and yes, the > datasheet uses CUR_BUF and NEXT_BUF for the primary plane, and BUF and > NEXT_BUF for the overlay plane :-S). I'd thus rather keep them aligned > with the documentation. > > > Would it make sense to store the above registers in mxsfb_devdata, > > just like we do for the primary planes? > > The reason the register addresses are stored in mxsfb_devdata for the > primary plane is because they differ between hardware revisions (don't > they teach hardware engineers in school these days *not* to move > registers around ? :-)). The overlay plane is only supported in the > latest versions of the IP core, and are always located at the same > address as far as I can tell. I don't think we need an extra level of > indirection. > Right, makes sense. Thanks for the information. -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 0/8] drm: rcar-du: Add Color Management Module (CMM)
Hi Eugeniu On Fri, Jun 05, 2020 at 03:41:24PM +0200, Eugeniu Rosca wrote: > Hi Jacopo, > > On Fri, Jun 05, 2020 at 03:29:00PM +0200, Jacopo Mondi wrote: > > On Wed, May 27, 2020 at 09:15:55AM +0200, Eugeniu Rosca wrote: > > > Could you kindly share the cross compilation steps for your kmsxx fork? > > > > I usually build it on the target :) > > Interesting approach. With ARM getting more and more potent, why not? :) > For 'small' utilities like kmsxx it's doable > > > > > Just out of curiosity, have you ever tried to pull the display's HDMI > > > cable while reading from CM2_LUT_TBL? > > > > Ahem, not really :) Did I get you right, you mean disconnecting the > > HDMI cable from the board ? > > Right. > So, no, I have not tried. Do you see any intersting failure with the mainline version ? > > > > > > At least with the out-of-tree CMM implementation [*], this sends the > > > R-Car3 reference targets into an unrecoverable freeze, with no lockup > > > reported by the kernel (i.e. looks like an serious HW issue). > > > > > > > > > > > CMM functionalities are retained between suspend/resume cycles (tested > > > > with > > > > suspend-to-idle) without requiring a re-programming of the LUT tables. > > > > > > Hmm. Is this backed up by any statement in the HW User's manual? > > > This comes in contrast with the original Renesas CMM implementation [**] > > > which does make use of suspend (where the freeze actually happens). > > > > > > Can we infer, based on your statement, that we could also get rid of > > > the suspend callback in [**]? > > > > As Geert (thanks) explained what I've tested with is suspend-to-idle, > > which retains the state of the LUT tables (and I assume other > > not-yet-implemented CMM features, like CLU). I recall the out-of-tree > > driver has suspend/resume routines but I never really tested that. > > I see. JFYI, there is a flaw in the suspend handling in the out-of-tree > CMM patch [*], which renders the SoC unresponsive on HDMI hotplug. The > fix is currently under review. Hopefully it will make its way to [*] > in the nearest future. Just to keep in mind for the moment when CMM > s2ram will become a mainline feature. Thanks, let's keep this in mind. Next week I'll run a few tests again with s2ram and will get back to you. Thanks j > > > > > > > [*] https://github.com/renesas-rcar/du_cmm > > > [**] > > > https://github.com/renesas-rcar/du_cmm/blob/c393ed49834bdbc/meta-rcar-gen3/recipes-kernel/linux/linux-renesas/0001-drm-rcar-du-Add-DU-CMM-support.patch#L1912 > > -- > Best regards, > Eugeniu Rosca ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 01/14] drm/mgag200: Remove declaration of mgag200_mmap() from header file
Commit 94668ac796a5 ("drm/mgag200: Convert mgag200 driver to VRAM MM") removed the implementation of mgag200_mmap(). Also remove the declaration. Signed-off-by: Thomas Zimmermann Fixes: 94668ac796a5 ("drm/mgag200: Convert mgag200 driver to VRAM MM") Cc: Gerd Hoffmann Cc: Dave Airlie Cc: Krzysztof Kozlowski Cc: Daniel Vetter Cc: Sam Ravnborg Cc: Greg Kroah-Hartman Cc: Thomas Gleixner Cc: "Noralf Trønnes" Cc: Armijn Hemel Cc: Alex Deucher Cc: Emil Velikov Cc: # v5.3+ --- drivers/gpu/drm/mgag200/mgag200_drv.h | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 47df62b1ad290..92b6679029fe5 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -198,6 +198,5 @@ void mgag200_i2c_destroy(struct mga_i2c_chan *i2c); int mgag200_mm_init(struct mga_device *mdev); void mgag200_mm_fini(struct mga_device *mdev); -int mgag200_mmap(struct file *filp, struct vm_area_struct *vma); #endif /* __MGAG200_DRV_H__ */ -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 10/14] drm/mgag200: Move device init and cleanup to mgag200_drv.c
Moving the initializer and cleanup functions for device instances to mgag200_drv.c prepares for the conversion to managed code. No functional changes are made. Remove mgag200_main.c, which is now empty. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/mgag200/Makefile | 3 +- drivers/gpu/drm/mgag200/mgag200_drv.c | 68 +++ drivers/gpu/drm/mgag200/mgag200_drv.h | 4 -- drivers/gpu/drm/mgag200/mgag200_main.c | 77 -- 4 files changed, 69 insertions(+), 83 deletions(-) delete mode 100644 drivers/gpu/drm/mgag200/mgag200_main.c diff --git a/drivers/gpu/drm/mgag200/Makefile b/drivers/gpu/drm/mgag200/Makefile index e6a933874a88c..42fedef538826 100644 --- a/drivers/gpu/drm/mgag200/Makefile +++ b/drivers/gpu/drm/mgag200/Makefile @@ -1,5 +1,4 @@ # SPDX-License-Identifier: GPL-2.0-only -mgag200-y := mgag200_drv.o mgag200_i2c.o mgag200_main.o mgag200_mm.o \ - mgag200_mode.o +mgag200-y := mgag200_drv.o mgag200_i2c.o mgag200_mm.o mgag200_mode.o obj-$(CONFIG_DRM_MGAG200) += mgag200.o diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index ad74e02d8f251..f8bb24199643d 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -39,6 +39,74 @@ static struct drm_driver mgag200_driver = { DRM_GEM_SHMEM_DRIVER_OPS, }; +/* + * DRM device + */ + +static int mgag200_driver_load(struct drm_device *dev, unsigned long flags) +{ + struct mga_device *mdev; + int ret, option; + + mdev = devm_kzalloc(dev->dev, sizeof(struct mga_device), GFP_KERNEL); + if (mdev == NULL) + return -ENOMEM; + dev->dev_private = (void *)mdev; + mdev->dev = dev; + + mdev->flags = mgag200_flags_from_driver_data(flags); + mdev->type = mgag200_type_from_driver_data(flags); + + pci_read_config_dword(dev->pdev, PCI_MGA_OPTION, ); + mdev->has_sdram = !(option & (1 << 14)); + + /* BAR 0 is the framebuffer, BAR 1 contains registers */ + mdev->rmmio_base = pci_resource_start(dev->pdev, 1); + mdev->rmmio_size = pci_resource_len(dev->pdev, 1); + + if (!devm_request_mem_region(dev->dev, mdev->rmmio_base, +mdev->rmmio_size, "mgadrmfb_mmio")) { + drm_err(dev, "can't reserve mmio registers\n"); + return -ENOMEM; + } + + mdev->rmmio = pcim_iomap(dev->pdev, 1, 0); + if (mdev->rmmio == NULL) + return -ENOMEM; + + /* stash G200 SE model number for later use */ + if (IS_G200_SE(mdev)) { + mdev->unique_rev_id = RREG32(0x1e24); + drm_dbg(dev, "G200 SE unique revision id is 0x%x\n", + mdev->unique_rev_id); + } + + ret = mgag200_mm_init(mdev); + if (ret) + goto err_mm; + + ret = mgag200_modeset_init(mdev); + if (ret) { + drm_err(dev, "Fatal error during modeset init: %d\n", ret); + goto err_mm; + } + + return 0; + +err_mm: + dev->dev_private = NULL; + return ret; +} + +static void mgag200_driver_unload(struct drm_device *dev) +{ + struct mga_device *mdev = to_mga_device(dev); + + if (mdev == NULL) + return; + dev->dev_private = NULL; +} + /* * PCI driver */ diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 7b6e6827a9a21..b38e5ce4ee20b 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -188,10 +188,6 @@ mgag200_flags_from_driver_data(kernel_ulong_t driver_data) /* mgag200_mode.c */ int mgag200_modeset_init(struct mga_device *mdev); - /* mgag200_main.c */ -int mgag200_driver_load(struct drm_device *dev, unsigned long flags); -void mgag200_driver_unload(struct drm_device *dev); - /* mgag200_i2c.c */ struct mga_i2c_chan *mgag200_i2c_create(struct drm_device *dev); void mgag200_i2c_destroy(struct mga_i2c_chan *i2c); diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c deleted file mode 100644 index 49bcdfcb40a4e..0 --- a/drivers/gpu/drm/mgag200/mgag200_main.c +++ /dev/null @@ -1,77 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-only -/* - * Copyright 2010 Matt Turner. - * Copyright 2012 Red Hat - * - * Authors: Matthew Garrett - * Matt Turner - * Dave Airlie - */ - -#include - -#include "mgag200_drv.h" - -int mgag200_driver_load(struct drm_device *dev, unsigned long flags) -{ - struct mga_device *mdev; - int ret, option; - - mdev = devm_kzalloc(dev->dev, sizeof(struct mga_device), GFP_KERNEL); - if (mdev == NULL) - return -ENOMEM; - dev->dev_private = (void *)mdev; - mdev->dev = dev; - - mdev->flags =
[PATCH 09/14] drm/mgag200: Prefix global names in mgag200_drv.c with mgag200_
The naming of global symbols in mgag200_drv.c is inconsistent. Fix that by prefixing all names with mgag200_. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/mgag200/mgag200_drv.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index 670e12d57dea8..ad74e02d8f251 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -27,7 +27,7 @@ module_param_named(modeset, mgag200_modeset, int, 0400); DEFINE_DRM_GEM_FOPS(mgag200_driver_fops); -static struct drm_driver driver = { +static struct drm_driver mgag200_driver = { .driver_features = DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET, .fops = _driver_fops, .name = DRIVER_NAME, @@ -43,7 +43,7 @@ static struct drm_driver driver = { * PCI driver */ -static const struct pci_device_id pciidlist[] = { +static const struct pci_device_id mgag200_pciidlist[] = { { PCI_VENDOR_ID_MATROX, 0x522, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_A | MGAG200_FLAG_HW_BUG_NO_STARTADD}, { PCI_VENDOR_ID_MATROX, 0x524, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_B }, @@ -56,10 +56,10 @@ static const struct pci_device_id pciidlist[] = { {0,} }; -MODULE_DEVICE_TABLE(pci, pciidlist); +MODULE_DEVICE_TABLE(pci, mgag200_pciidlist); - -static int mga_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) +static int +mgag200_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) { struct drm_device *dev; int ret; @@ -70,7 +70,7 @@ static int mga_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) return ret; - dev = drm_dev_alloc(, >dev); + dev = drm_dev_alloc(_driver, >dev); if (IS_ERR(dev)) return PTR_ERR(dev); @@ -96,7 +96,7 @@ static int mga_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) return ret; } -static void mga_pci_remove(struct pci_dev *pdev) +static void mgag200_pci_remove(struct pci_dev *pdev) { struct drm_device *dev = pci_get_drvdata(pdev); @@ -107,9 +107,9 @@ static void mga_pci_remove(struct pci_dev *pdev) static struct pci_driver mgag200_pci_driver = { .name = DRIVER_NAME, - .id_table = pciidlist, - .probe = mga_pci_probe, - .remove = mga_pci_remove, + .id_table = mgag200_pciidlist, + .probe = mgag200_pci_probe, + .remove = mgag200_pci_remove, }; static int __init mgag200_init(void) -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 14/14] drm/mgag200: Use managed device initialization
The mgag200 driver now uses managed functions for DRM devices. The individual helpers for modesetting and memory managed are already covered, so only device allocation and initialization is left for conversion. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/mgag200/mgag200_drv.c | 30 +++ 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index 6dfb7c5f79e3c..e19660f4a6371 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -95,27 +95,20 @@ mgag200_device_create(struct pci_dev *pdev, unsigned long flags) struct mga_device *mdev; int ret; - mdev = devm_kzalloc(>dev, sizeof(*mdev), GFP_KERNEL); - if (!mdev) - return ERR_PTR(-ENOMEM); + mdev = devm_drm_dev_alloc(>dev, _driver, + struct mga_device, base); + if (IS_ERR(mdev)) + return mdev; dev = >base; - ret = drm_dev_init(dev, _driver, >dev); - if (ret) - return ERR_PTR(ret); - dev->pdev = pdev; pci_set_drvdata(pdev, dev); ret = mgag200_device_init(mdev, flags); if (ret) - goto err_drm_dev_put; + return ERR_PTR(ret); return mdev; - -err_drm_dev_put: - drm_dev_put(dev); - return ERR_PTR(ret); } /* @@ -151,23 +144,17 @@ mgag200_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) return ret; mdev = mgag200_device_create(pdev, ent->driver_data); - if (IS_ERR(mdev)) { - ret = PTR_ERR(mdev); - goto err_drm_dev_put; - } + if (IS_ERR(mdev)) + return PTR_ERR(mdev); dev = >base; ret = drm_dev_register(dev, ent->driver_data); if (ret) - goto err_drm_dev_put; + return ret; drm_fbdev_generic_setup(dev, 0); return 0; - -err_drm_dev_put: - drm_dev_put(dev); - return ret; } static void mgag200_pci_remove(struct pci_dev *pdev) @@ -175,7 +162,6 @@ static void mgag200_pci_remove(struct pci_dev *pdev) struct drm_device *dev = pci_get_drvdata(pdev); drm_dev_unregister(dev); - drm_dev_put(dev); } static struct pci_driver mgag200_pci_driver = { -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 04/14] drm/mgag200: Rename mgag200_ttm.c to mgag200_mm.c
The mgag200 driver does not use TTM any longer. Rename the related file to mgag200_mm.c (as in 'memory management'). Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/mgag200/Makefile| 4 ++-- drivers/gpu/drm/mgag200/mgag200_drv.h | 1 + drivers/gpu/drm/mgag200/{mgag200_ttm.c => mgag200_mm.c} | 0 3 files changed, 3 insertions(+), 2 deletions(-) rename drivers/gpu/drm/mgag200/{mgag200_ttm.c => mgag200_mm.c} (100%) diff --git a/drivers/gpu/drm/mgag200/Makefile b/drivers/gpu/drm/mgag200/Makefile index 63403133638a3..e6a933874a88c 100644 --- a/drivers/gpu/drm/mgag200/Makefile +++ b/drivers/gpu/drm/mgag200/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0-only -mgag200-y := mgag200_main.o mgag200_mode.o \ - mgag200_drv.o mgag200_i2c.o mgag200_ttm.o +mgag200-y := mgag200_drv.o mgag200_i2c.o mgag200_main.o mgag200_mm.o \ + mgag200_mode.o obj-$(CONFIG_DRM_MGAG200) += mgag200.o diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 92b6679029fe5..cd786ffe319b8 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -196,6 +196,7 @@ void mgag200_driver_unload(struct drm_device *dev); struct mga_i2c_chan *mgag200_i2c_create(struct drm_device *dev); void mgag200_i2c_destroy(struct mga_i2c_chan *i2c); + /* mgag200_mm.c */ int mgag200_mm_init(struct mga_device *mdev); void mgag200_mm_fini(struct mga_device *mdev); diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c b/drivers/gpu/drm/mgag200/mgag200_mm.c similarity index 100% rename from drivers/gpu/drm/mgag200/mgag200_ttm.c rename to drivers/gpu/drm/mgag200/mgag200_mm.c -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 11/14] drm/mgag200: Separate device initialization into allocation
Embedding the DRM device instance in struct mga_device will require changes to device allocation. Moving the device initialization into its own functions gets it out of the way. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/mgag200/mgag200_drv.c | 32 ++- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index f8bb24199643d..926437a27a228 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -43,17 +43,11 @@ static struct drm_driver mgag200_driver = { * DRM device */ -static int mgag200_driver_load(struct drm_device *dev, unsigned long flags) +static int mgag200_device_init(struct mga_device *mdev, unsigned long flags) { - struct mga_device *mdev; + struct drm_device *dev = mdev->dev; int ret, option; - mdev = devm_kzalloc(dev->dev, sizeof(struct mga_device), GFP_KERNEL); - if (mdev == NULL) - return -ENOMEM; - dev->dev_private = (void *)mdev; - mdev->dev = dev; - mdev->flags = mgag200_flags_from_driver_data(flags); mdev->type = mgag200_type_from_driver_data(flags); @@ -83,15 +77,33 @@ static int mgag200_driver_load(struct drm_device *dev, unsigned long flags) ret = mgag200_mm_init(mdev); if (ret) - goto err_mm; + return ret; ret = mgag200_modeset_init(mdev); if (ret) { drm_err(dev, "Fatal error during modeset init: %d\n", ret); - goto err_mm; + return ret; } return 0; +} + +static int mgag200_driver_load(struct drm_device *dev, unsigned long flags) +{ + struct mga_device *mdev; + int ret; + + mdev = devm_kzalloc(dev->dev, sizeof(struct mga_device), GFP_KERNEL); + if (mdev == NULL) + return -ENOMEM; + dev->dev_private = (void *)mdev; + mdev->dev = dev; + + ret = mgag200_device_init(mdev, flags); + if (ret) + goto err_mm; + + return 0; err_mm: dev->dev_private = NULL; -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 08/14] drm/mgag200: Separate DRM and PCI functionality from each other
Moving the DRM driver structures from the middle of the PCI code to the top of the file makes it more readable. Also remove an obsolete comment. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/mgag200/mgag200_drv.c | 42 +-- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index 140ae86082c8e..670e12d57dea8 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -17,17 +17,31 @@ #include "mgag200_drv.h" -/* - * This is the generic driver code. This binds the driver to the drm core, - * which then performs further device association and calls our graphics init - * functions - */ - int mgag200_modeset = -1; MODULE_PARM_DESC(modeset, "Disable/Enable modesetting"); module_param_named(modeset, mgag200_modeset, int, 0400); -static struct drm_driver driver; +/* + * DRM driver + */ + +DEFINE_DRM_GEM_FOPS(mgag200_driver_fops); + +static struct drm_driver driver = { + .driver_features = DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET, + .fops = _driver_fops, + .name = DRIVER_NAME, + .desc = DRIVER_DESC, + .date = DRIVER_DATE, + .major = DRIVER_MAJOR, + .minor = DRIVER_MINOR, + .patchlevel = DRIVER_PATCHLEVEL, + DRM_GEM_SHMEM_DRIVER_OPS, +}; + +/* + * PCI driver + */ static const struct pci_device_id pciidlist[] = { { PCI_VENDOR_ID_MATROX, 0x522, PCI_ANY_ID, PCI_ANY_ID, 0, 0, @@ -91,20 +105,6 @@ static void mga_pci_remove(struct pci_dev *pdev) drm_dev_put(dev); } -DEFINE_DRM_GEM_FOPS(mgag200_driver_fops); - -static struct drm_driver driver = { - .driver_features = DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET, - .fops = _driver_fops, - .name = DRIVER_NAME, - .desc = DRIVER_DESC, - .date = DRIVER_DATE, - .major = DRIVER_MAJOR, - .minor = DRIVER_MINOR, - .patchlevel = DRIVER_PATCHLEVEL, - DRM_GEM_SHMEM_DRIVER_OPS, -}; - static struct pci_driver mgag200_pci_driver = { .name = DRIVER_NAME, .id_table = pciidlist, -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 13/14] drm/mgag200: Embed instance of struct drm_device in struct mga_device
Following current best practice, the instance of struct drm_device is now embedded in struct mga_device. The respective field has been renamed from 'dev' to 'base' to reflect the relationship. Conversion from DRM device is done via upcast. Using dev_private is no longer possible. The patch also open-codes drm_dev_alloc() and DRM device initialization is now performed by a call to drm_device_init(). Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/mgag200/mgag200_drv.c | 47 ++ drivers/gpu/drm/mgag200/mgag200_drv.h | 4 +-- drivers/gpu/drm/mgag200/mgag200_mm.c | 2 +- drivers/gpu/drm/mgag200/mgag200_mode.c | 12 +++ 4 files changed, 27 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index 592e484f87ee7..6dfb7c5f79e3c 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -45,7 +45,7 @@ static struct drm_driver mgag200_driver = { static int mgag200_device_init(struct mga_device *mdev, unsigned long flags) { - struct drm_device *dev = mdev->dev; + struct drm_device *dev = >base; int ret, option; mdev->flags = mgag200_flags_from_driver_data(flags); @@ -89,25 +89,24 @@ static int mgag200_device_init(struct mga_device *mdev, unsigned long flags) } static struct mga_device * -mgag200_driver_load(struct pci_dev *pdev, unsigned long flags) +mgag200_device_create(struct pci_dev *pdev, unsigned long flags) { struct drm_device *dev; struct mga_device *mdev; int ret; - dev = drm_dev_alloc(_driver, >dev); - if (IS_ERR(dev)) - return ERR_CAST(dev); + mdev = devm_kzalloc(>dev, sizeof(*mdev), GFP_KERNEL); + if (!mdev) + return ERR_PTR(-ENOMEM); + dev = >base; + + ret = drm_dev_init(dev, _driver, >dev); + if (ret) + return ERR_PTR(ret); dev->pdev = pdev; pci_set_drvdata(pdev, dev); - mdev = devm_kzalloc(dev->dev, sizeof(struct mga_device), GFP_KERNEL); - if (mdev == NULL) - return ERR_PTR(-ENOMEM); - dev->dev_private = (void *)mdev; - mdev->dev = dev; - ret = mgag200_device_init(mdev, flags); if (ret) goto err_drm_dev_put; @@ -116,19 +115,9 @@ mgag200_driver_load(struct pci_dev *pdev, unsigned long flags) err_drm_dev_put: drm_dev_put(dev); - dev->dev_private = NULL; return ERR_PTR(ret); } -static void mgag200_driver_unload(struct drm_device *dev) -{ - struct mga_device *mdev = to_mga_device(dev); - - if (mdev == NULL) - return; - dev->dev_private = NULL; -} - /* * PCI driver */ @@ -161,21 +150,22 @@ mgag200_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) return ret; - mdev = mgag200_driver_load(pdev, ent->driver_data); - if (IS_ERR(mdev)) - return PTR_ERR(mdev); - dev = mdev->dev; + mdev = mgag200_device_create(pdev, ent->driver_data); + if (IS_ERR(mdev)) { + ret = PTR_ERR(mdev); + goto err_drm_dev_put; + } + dev = >base; ret = drm_dev_register(dev, ent->driver_data); if (ret) - goto err_mgag200_driver_unload; + goto err_drm_dev_put; drm_fbdev_generic_setup(dev, 0); return 0; -err_mgag200_driver_unload: - mgag200_driver_unload(dev); +err_drm_dev_put: drm_dev_put(dev); return ret; } @@ -185,7 +175,6 @@ static void mgag200_pci_remove(struct pci_dev *pdev) struct drm_device *dev = pci_get_drvdata(pdev); drm_dev_unregister(dev); - mgag200_driver_unload(dev); drm_dev_put(dev); } diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index b38e5ce4ee20b..270c2f9a67666 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -142,7 +142,7 @@ enum mga_type { #define IS_G200_SE(mdev) (mdev->type == G200_SE_A || mdev->type == G200_SE_B) struct mga_device { - struct drm_device *dev; + struct drm_device base; unsigned long flags; resource_size_t rmmio_base; @@ -170,7 +170,7 @@ struct mga_device { static inline struct mga_device *to_mga_device(struct drm_device *dev) { - return dev->dev_private; + return container_of(dev, struct mga_device, base); } static inline enum mga_type diff --git a/drivers/gpu/drm/mgag200/mgag200_mm.c b/drivers/gpu/drm/mgag200/mgag200_mm.c index 1b1e5ec5d1ceb..7b69392bcb891 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mm.c +++ b/drivers/gpu/drm/mgag200/mgag200_mm.c @@ -89,7 +89,7 @@ static void mgag200_mm_release(struct drm_device *dev, void *ptr) int mgag200_mm_init(struct mga_device *mdev) { -
[PATCH 02/14] drm/mgag200: Remove mgag200_cursor.c
Support for HW cursors got remove by commit 5a77e2bfdd4f ("drm/mgag200: Remove HW cursor") Apparently the source file was not deleted. Removed it now. Signed-off-by: Thomas Zimmermann Fixes: 5a77e2bfdd4f ("drm/mgag200: Remove HW cursor") Cc: Sam Ravnborg Cc: Emil Velikov Cc: Dave Airlie Cc: "Noralf Trønnes" Cc: Gerd Hoffmann Cc: Daniel Vetter Cc: Greg Kroah-Hartman Cc: Thomas Gleixner Cc: Kate Stewart Cc: Allison Randal Cc: Andrzej Pietrasiewicz Cc: "José Roberto de Souza" --- drivers/gpu/drm/mgag200/mgag200_cursor.c | 319 --- 1 file changed, 319 deletions(-) delete mode 100644 drivers/gpu/drm/mgag200/mgag200_cursor.c diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c deleted file mode 100644 index c6932dc8acf2e..0 --- a/drivers/gpu/drm/mgag200/mgag200_cursor.c +++ /dev/null @@ -1,319 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-only -/* - * Copyright 2013 Matrox Graphics - * - * Author: Christopher Harvey - */ - -#include - -#include "mgag200_drv.h" - -static bool warn_transparent = true; -static bool warn_palette = true; - -static int mgag200_cursor_update(struct mga_device *mdev, void *dst, void *src, -unsigned int width, unsigned int height) -{ - struct drm_device *dev = mdev->dev; - unsigned int i, row, col; - uint32_t colour_set[16]; - uint32_t *next_space = _set[0]; - uint32_t *palette_iter; - uint32_t this_colour; - bool found = false; - int colour_count = 0; - u8 reg_index; - u8 this_row[48]; - - memset(_set[0], 0, sizeof(uint32_t)*16); - /* width*height*4 = 16384 */ - for (i = 0; i < 16384; i += 4) { - this_colour = ioread32(src + i); - /* No transparency */ - if (this_colour>>24 != 0xff && - this_colour>>24 != 0x0) { - if (warn_transparent) { - dev_info(>pdev->dev, "Video card doesn't support cursors with partial transparency.\n"); - dev_info(>pdev->dev, "Not enabling hardware cursor.\n"); - warn_transparent = false; /* Only tell the user once. */ - } - return -EINVAL; - } - /* Don't need to store transparent pixels as colours */ - if (this_colour>>24 == 0x0) - continue; - found = false; - for (palette_iter = _set[0]; palette_iter != next_space; palette_iter++) { - if (*palette_iter == this_colour) { - found = true; - break; - } - } - if (found) - continue; - /* We only support 4bit paletted cursors */ - if (colour_count >= 16) { - if (warn_palette) { - dev_info(>pdev->dev, "Video card only supports cursors with up to 16 colours.\n"); - dev_info(>pdev->dev, "Not enabling hardware cursor.\n"); - warn_palette = false; /* Only tell the user once. */ - } - return -EINVAL; - } - *next_space = this_colour; - next_space++; - colour_count++; - } - - /* Program colours from cursor icon into palette */ - for (i = 0; i < colour_count; i++) { - if (i <= 2) - reg_index = 0x8 + i*0x4; - else - reg_index = 0x60 + i*0x3; - WREG_DAC(reg_index, colour_set[i] & 0xff); - WREG_DAC(reg_index+1, colour_set[i]>>8 & 0xff); - WREG_DAC(reg_index+2, colour_set[i]>>16 & 0xff); - BUG_ON((colour_set[i]>>24 & 0xff) != 0xff); - } - - /* now write colour indices into hardware cursor buffer */ - for (row = 0; row < 64; row++) { - memset(_row[0], 0, 48); - for (col = 0; col < 64; col++) { - this_colour = ioread32(src + 4*(col + 64*row)); - /* write transparent pixels */ - if (this_colour>>24 == 0x0) { - this_row[47 - col/8] |= 0x80>>(col%8); - continue; - } - - /* write colour index here */ - for (i = 0; i < colour_count; i++) { - if (colour_set[i] == this_colour) { - if (col % 2) - this_row[col/2] |= i<<4; - else - this_row[col/2] |= i; -
[PATCH 05/14] drm/mgag200: Lookup VRAM PCI BAR start and length only once
The MM setup code on mgag200 reads PCI BAR 0's start and length several times. Reusing these values makes the code more readable. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/mgag200/mgag200_mm.c | 17 + 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_mm.c b/drivers/gpu/drm/mgag200/mgag200_mm.c index a683642fe4682..73e30901e0631 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mm.c +++ b/drivers/gpu/drm/mgag200/mgag200_mm.c @@ -33,16 +33,18 @@ int mgag200_mm_init(struct mga_device *mdev) { struct drm_device *dev = mdev->dev; + resource_size_t start, len; int ret; - arch_io_reserve_memtype_wc(pci_resource_start(dev->pdev, 0), - pci_resource_len(dev->pdev, 0)); + /* BAR 0 is VRAM */ + start = pci_resource_start(dev->pdev, 0); + len = pci_resource_len(dev->pdev, 0); - mdev->fb_mtrr = arch_phys_wc_add(pci_resource_start(dev->pdev, 0), -pci_resource_len(dev->pdev, 0)); + arch_io_reserve_memtype_wc(start, len); - mdev->vram = ioremap(pci_resource_start(dev->pdev, 0), -pci_resource_len(dev->pdev, 0)); + mdev->fb_mtrr = arch_phys_wc_add(start, len); + + mdev->vram = ioremap(start, len); if (!mdev->vram) { ret = -ENOMEM; goto err_arch_phys_wc_del; @@ -54,8 +56,7 @@ int mgag200_mm_init(struct mga_device *mdev) err_arch_phys_wc_del: arch_phys_wc_del(mdev->fb_mtrr); - arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0), - pci_resource_len(dev->pdev, 0)); + arch_io_free_memtype_wc(start, len); return ret; } -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 03/14] drm/mgag200: Use pcim_enable_device()
Using the managed function simplifies the error handling. After unloading the driver, the PCI device should now get disabled as well. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/mgag200/mgag200_drv.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index 00ddea7d7d270..140ae86082c8e 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -52,15 +52,13 @@ static int mga_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, "mgag200drmfb"); - ret = pci_enable_device(pdev); + ret = pcim_enable_device(pdev); if (ret) return ret; dev = drm_dev_alloc(, >dev); - if (IS_ERR(dev)) { - ret = PTR_ERR(dev); - goto err_pci_disable_device; - } + if (IS_ERR(dev)) + return PTR_ERR(dev); dev->pdev = pdev; pci_set_drvdata(pdev, dev); @@ -81,8 +79,6 @@ static int mga_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) mgag200_driver_unload(dev); err_drm_dev_put: drm_dev_put(dev); -err_pci_disable_device: - pci_disable_device(pdev); return ret; } -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 12/14] drm/mgag200: Allocate device structures in mgag200_driver_load()
Instances of struct drm_device and struct mga_device are now allocated next to each other in mgag200_driver_load(). Yet another preparation before embedding the DRM device instance in struct mga_device. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/mgag200/mgag200_drv.c | 38 +++ 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index 926437a27a228..592e484f87ee7 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -88,26 +88,36 @@ static int mgag200_device_init(struct mga_device *mdev, unsigned long flags) return 0; } -static int mgag200_driver_load(struct drm_device *dev, unsigned long flags) +static struct mga_device * +mgag200_driver_load(struct pci_dev *pdev, unsigned long flags) { + struct drm_device *dev; struct mga_device *mdev; int ret; + dev = drm_dev_alloc(_driver, >dev); + if (IS_ERR(dev)) + return ERR_CAST(dev); + + dev->pdev = pdev; + pci_set_drvdata(pdev, dev); + mdev = devm_kzalloc(dev->dev, sizeof(struct mga_device), GFP_KERNEL); if (mdev == NULL) - return -ENOMEM; + return ERR_PTR(-ENOMEM); dev->dev_private = (void *)mdev; mdev->dev = dev; ret = mgag200_device_init(mdev, flags); if (ret) - goto err_mm; + goto err_drm_dev_put; - return 0; + return mdev; -err_mm: +err_drm_dev_put: + drm_dev_put(dev); dev->dev_private = NULL; - return ret; + return ERR_PTR(ret); } static void mgag200_driver_unload(struct drm_device *dev) @@ -141,6 +151,7 @@ MODULE_DEVICE_TABLE(pci, mgag200_pciidlist); static int mgag200_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) { + struct mga_device *mdev; struct drm_device *dev; int ret; @@ -150,16 +161,10 @@ mgag200_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) return ret; - dev = drm_dev_alloc(_driver, >dev); - if (IS_ERR(dev)) - return PTR_ERR(dev); - - dev->pdev = pdev; - pci_set_drvdata(pdev, dev); - - ret = mgag200_driver_load(dev, ent->driver_data); - if (ret) - goto err_drm_dev_put; + mdev = mgag200_driver_load(pdev, ent->driver_data); + if (IS_ERR(mdev)) + return PTR_ERR(mdev); + dev = mdev->dev; ret = drm_dev_register(dev, ent->driver_data); if (ret) @@ -171,7 +176,6 @@ mgag200_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) err_mgag200_driver_unload: mgag200_driver_unload(dev); -err_drm_dev_put: drm_dev_put(dev); return ret; } -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 07/14] drm/mgag200: Switch to managed MM
The memory-management code now cleans up automatically as part of device destruction. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/mgag200/mgag200_drv.h | 1 - drivers/gpu/drm/mgag200/mgag200_main.c | 5 + drivers/gpu/drm/mgag200/mgag200_mm.c | 28 ++ 3 files changed, 16 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index cd786ffe319b8..7b6e6827a9a21 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -198,6 +198,5 @@ void mgag200_i2c_destroy(struct mga_i2c_chan *i2c); /* mgag200_mm.c */ int mgag200_mm_init(struct mga_device *mdev); -void mgag200_mm_fini(struct mga_device *mdev); #endif /* __MGAG200_DRV_H__ */ diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c index e9ad783c2b44d..49bcdfcb40a4e 100644 --- a/drivers/gpu/drm/mgag200/mgag200_main.c +++ b/drivers/gpu/drm/mgag200/mgag200_main.c @@ -57,13 +57,11 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags) ret = mgag200_modeset_init(mdev); if (ret) { drm_err(dev, "Fatal error during modeset init: %d\n", ret); - goto err_mgag200_mm_fini; + goto err_mm; } return 0; -err_mgag200_mm_fini: - mgag200_mm_fini(mdev); err_mm: dev->dev_private = NULL; return ret; @@ -75,6 +73,5 @@ void mgag200_driver_unload(struct drm_device *dev) if (mdev == NULL) return; - mgag200_mm_fini(mdev); dev->dev_private = NULL; } diff --git a/drivers/gpu/drm/mgag200/mgag200_mm.c b/drivers/gpu/drm/mgag200/mgag200_mm.c index f56b0456994f4..1b1e5ec5d1ceb 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mm.c +++ b/drivers/gpu/drm/mgag200/mgag200_mm.c @@ -28,6 +28,8 @@ #include +#include + #include "mgag200_drv.h" static size_t mgag200_probe_vram(struct mga_device *mdev, void __iomem *mem, @@ -73,6 +75,18 @@ static size_t mgag200_probe_vram(struct mga_device *mdev, void __iomem *mem, return offset - 65536; } +static void mgag200_mm_release(struct drm_device *dev, void *ptr) +{ + struct mga_device *mdev = to_mga_device(dev); + + mdev->vram_fb_available = 0; + iounmap(mdev->vram); + arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0), + pci_resource_len(dev->pdev, 0)); + arch_phys_wc_del(mdev->fb_mtrr); + mdev->fb_mtrr = 0; +} + int mgag200_mm_init(struct mga_device *mdev) { struct drm_device *dev = mdev->dev; @@ -104,22 +118,10 @@ int mgag200_mm_init(struct mga_device *mdev) mdev->vram_fb_available = mdev->mc.vram_size; - return 0; + return drmm_add_action_or_reset(dev, mgag200_mm_release, NULL); err_arch_phys_wc_del: arch_phys_wc_del(mdev->fb_mtrr); arch_io_free_memtype_wc(start, len); return ret; } - -void mgag200_mm_fini(struct mga_device *mdev) -{ - struct drm_device *dev = mdev->dev; - - mdev->vram_fb_available = 0; - iounmap(mdev->vram); - arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0), - pci_resource_len(dev->pdev, 0)); - arch_phys_wc_del(mdev->fb_mtrr); - mdev->fb_mtrr = 0; -} -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 06/14] drm/mgag200: Merge VRAM setup into MM initialization
The VRAM setup in mgag200_drv.c is part of memory management and should be done in the same place. Merge the code into the memory management's init function. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/mgag200/mgag200_main.c | 75 -- drivers/gpu/drm/mgag200/mgag200_mm.c | 52 ++ 2 files changed, 52 insertions(+), 75 deletions(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c index 3298eff7bd1b4..e9ad783c2b44d 100644 --- a/drivers/gpu/drm/mgag200/mgag200_main.c +++ b/drivers/gpu/drm/mgag200/mgag200_main.c @@ -12,77 +12,6 @@ #include "mgag200_drv.h" -static int mga_probe_vram(struct mga_device *mdev, void __iomem *mem) -{ - int offset; - int orig; - int test1, test2; - int orig1, orig2; - unsigned int vram_size; - - /* Probe */ - orig = ioread16(mem); - iowrite16(0, mem); - - vram_size = mdev->mc.vram_window; - - if ((mdev->type == G200_EW3) && (vram_size >= 0x100)) { - vram_size = vram_size - 0x40; - } - - for (offset = 0x10; offset < vram_size; offset += 0x4000) { - orig1 = ioread8(mem + offset); - orig2 = ioread8(mem + offset + 0x100); - - iowrite16(0xaa55, mem + offset); - iowrite16(0xaa55, mem + offset + 0x100); - - test1 = ioread16(mem + offset); - test2 = ioread16(mem); - - iowrite16(orig1, mem + offset); - iowrite16(orig2, mem + offset + 0x100); - - if (test1 != 0xaa55) { - break; - } - - if (test2) { - break; - } - } - - iowrite16(orig, mem); - return offset - 65536; -} - -/* Map the framebuffer from the card and configure the core */ -static int mga_vram_init(struct mga_device *mdev) -{ - struct drm_device *dev = mdev->dev; - void __iomem *mem; - - /* BAR 0 is VRAM */ - mdev->mc.vram_base = pci_resource_start(dev->pdev, 0); - mdev->mc.vram_window = pci_resource_len(dev->pdev, 0); - - if (!devm_request_mem_region(dev->dev, mdev->mc.vram_base, -mdev->mc.vram_window, "mgadrmfb_vram")) { - DRM_ERROR("can't reserve VRAM\n"); - return -ENXIO; - } - - mem = pci_iomap(dev->pdev, 0, 0); - if (!mem) - return -ENOMEM; - - mdev->mc.vram_size = mga_probe_vram(mdev, mem); - - pci_iounmap(dev->pdev, mem); - - return 0; -} - int mgag200_driver_load(struct drm_device *dev, unsigned long flags) { struct mga_device *mdev; @@ -121,10 +50,6 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags) mdev->unique_rev_id); } - ret = mga_vram_init(mdev); - if (ret) - return ret; - ret = mgag200_mm_init(mdev); if (ret) goto err_mm; diff --git a/drivers/gpu/drm/mgag200/mgag200_mm.c b/drivers/gpu/drm/mgag200/mgag200_mm.c index 73e30901e0631..f56b0456994f4 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mm.c +++ b/drivers/gpu/drm/mgag200/mgag200_mm.c @@ -30,6 +30,49 @@ #include "mgag200_drv.h" +static size_t mgag200_probe_vram(struct mga_device *mdev, void __iomem *mem, +size_t size) +{ + int offset; + int orig; + int test1, test2; + int orig1, orig2; + size_t vram_size; + + /* Probe */ + orig = ioread16(mem); + iowrite16(0, mem); + + vram_size = size; + + if ((mdev->type == G200_EW3) && (vram_size >= 0x100)) + vram_size = vram_size - 0x40; + + for (offset = 0x10; offset < vram_size; offset += 0x4000) { + orig1 = ioread8(mem + offset); + orig2 = ioread8(mem + offset + 0x100); + + iowrite16(0xaa55, mem + offset); + iowrite16(0xaa55, mem + offset + 0x100); + + test1 = ioread16(mem + offset); + test2 = ioread16(mem); + + iowrite16(orig1, mem + offset); + iowrite16(orig2, mem + offset + 0x100); + + if (test1 != 0xaa55) + break; + + if (test2) + break; + } + + iowrite16(orig, mem); + + return offset - 65536; +} + int mgag200_mm_init(struct mga_device *mdev) { struct drm_device *dev = mdev->dev; @@ -40,6 +83,11 @@ int mgag200_mm_init(struct mga_device *mdev) start = pci_resource_start(dev->pdev, 0); len = pci_resource_len(dev->pdev, 0); + if (!devm_request_mem_region(dev->dev, start, len, "mgadrmfb_vram")) { + drm_err(dev, "can't reserve VRAM\n"); + return -ENXIO; + } + arch_io_reserve_memtype_wc(start, len); mdev->fb_mtrr =
[PATCH 00/14] drm/mgag200: Use managed interfaces for auto-cleanup
This patchset cleans up mgag200 device initialization, embeds the DRM device instance in struct mga_device and finally converts device initialization to managed interfaces. Patches 1 and 2 are actually unrelated. Both remove artifacts that got lost from earlier patch series. We're fixing this before introducing new changes. Patches 3 to 7 cleanup the memory management code and convert it to managed. Specifically, all MM code is being moved into a the same file. That makes it more obvious what's going on and will allow for further cleanups later on. Modesetting is already cleaned up automatically, so there's nothing to do here. With modesetting and MM done, patches 8 to 14 convert the device initialization to managed interfaces. The allocation is split among several functions and we move it to the same place in patches 11 and 12. That is also a good opportunity to embed the DRM device instance in struct mga_device in patch 13. Patch 14 adds managed release of the device structure. Tested on Matrox G200SE HW. Thomas Zimmermann (14): drm/mgag200: Remove declaration of mgag200_mmap() from header file drm/mgag200: Remove mgag200_cursor.c drm/mgag200: Use pcim_enable_device() drm/mgag200: Rename mgag200_ttm.c to mgag200_mm.c drm/mgag200: Lookup VRAM PCI BAR start and length only once drm/mgag200: Merge VRAM setup into MM initialization drm/mgag200: Switch to managed MM drm/mgag200: Separate DRM and PCI functionality from each other drm/mgag200: Prefix global names in mgag200_drv.c with mgag200_ drm/mgag200: Move device init and cleanup to mgag200_drv.c drm/mgag200: Separate device initialization into allocation drm/mgag200: Allocate device structures in mgag200_driver_load() drm/mgag200: Embed instance of struct drm_device in struct mga_device drm/mgag200: Use managed device initialization drivers/gpu/drm/mgag200/Makefile | 3 +- drivers/gpu/drm/mgag200/mgag200_cursor.c | 319 -- drivers/gpu/drm/mgag200/mgag200_drv.c | 161 ++--- drivers/gpu/drm/mgag200/mgag200_drv.h | 11 +- drivers/gpu/drm/mgag200/mgag200_main.c| 155 - .../mgag200/{mgag200_ttm.c => mgag200_mm.c} | 99 -- drivers/gpu/drm/mgag200/mgag200_mode.c| 12 +- 7 files changed, 195 insertions(+), 565 deletions(-) delete mode 100644 drivers/gpu/drm/mgag200/mgag200_cursor.c delete mode 100644 drivers/gpu/drm/mgag200/mgag200_main.c rename drivers/gpu/drm/mgag200/{mgag200_ttm.c => mgag200_mm.c} (51%) -- 2.26.2 Thomas Zimmermann (14): drm/mgag200: Remove declaration of mgag200_mmap() from header file drm/mgag200: Remove mgag200_cursor.c drm/mgag200: Use pcim_enable_device() drm/mgag200: Rename mgag200_ttm.c to mgag200_mm.c drm/mgag200: Lookup VRAM PCI BAR start and length only once drm/mgag200: Merge VRAM setup into MM initialization drm/mgag200: Switch to managed MM drm/mgag200: Separate DRM and PCI functionality from each other drm/mgag200: Prefix global names in mgag200_drv.c with mgag200_ drm/mgag200: Move device init and cleanup to mgag200_drv.c drm/mgag200: Separate device initialization into allocation drm/mgag200: Allocate device structures in mgag200_driver_load() drm/mgag200: Embed instance of struct drm_device in struct mga_device drm/mgag200: Use managed device initialization drivers/gpu/drm/mgag200/Makefile | 3 +- drivers/gpu/drm/mgag200/mgag200_cursor.c | 319 -- drivers/gpu/drm/mgag200/mgag200_drv.c | 161 ++--- drivers/gpu/drm/mgag200/mgag200_drv.h | 11 +- drivers/gpu/drm/mgag200/mgag200_main.c| 155 - .../mgag200/{mgag200_ttm.c => mgag200_mm.c} | 99 -- drivers/gpu/drm/mgag200/mgag200_mode.c| 12 +- 7 files changed, 195 insertions(+), 565 deletions(-) delete mode 100644 drivers/gpu/drm/mgag200/mgag200_cursor.c delete mode 100644 drivers/gpu/drm/mgag200/mgag200_main.c rename drivers/gpu/drm/mgag200/{mgag200_ttm.c => mgag200_mm.c} (51%) -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 0/8] drm: rcar-du: Add Color Management Module (CMM)
Hello Eugeniu, sorry for the late reply On Wed, May 27, 2020 at 09:15:55AM +0200, Eugeniu Rosca wrote: > Hi Jacopo, > > On Tue, Oct 15, 2019 at 12:46:13PM +0200, Jacopo Mondi wrote: > 8<--- > > > * Testing > > I have tested by injecting a color inversion LUT table and > > enabling/disabling it > > every 50 displayed frames: > > https://jmondi.org/cgit/kmsxx/log/?h=gamma_lut > > Could you kindly share the cross compilation steps for your kmsxx fork? > I usually build it on the target :) > Just out of curiosity, have you ever tried to pull the display's HDMI > cable while reading from CM2_LUT_TBL? Ahem, not really :) Did I get you right, you mean disconnecting the HDMI cable from the board ? > > At least with the out-of-tree CMM implementation [*], this sends the > R-Car3 reference targets into an unrecoverable freeze, with no lockup > reported by the kernel (i.e. looks like an serious HW issue). > > > > > CMM functionalities are retained between suspend/resume cycles (tested with > > suspend-to-idle) without requiring a re-programming of the LUT tables. > > Hmm. Is this backed up by any statement in the HW User's manual? > This comes in contrast with the original Renesas CMM implementation [**] > which does make use of suspend (where the freeze actually happens). > > Can we infer, based on your statement, that we could also get rid of > the suspend callback in [**]? As Geert (thanks) explained what I've tested with is suspend-to-idle, which retains the state of the LUT tables (and I assume other not-yet-implemented CMM features, like CLU). I recall the out-of-tree driver has suspend/resume routines but I never really tested that. Thanks j > > [*] https://github.com/renesas-rcar/du_cmm > [**] > https://github.com/renesas-rcar/du_cmm/blob/c393ed49834bdbc/meta-rcar-gen3/recipes-kernel/linux/linux-renesas/0001-drm-rcar-du-Add-DU-CMM-support.patch#L1912 > > -- > Best regards, > Eugeniu Rosca ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] dma-fence: basic lockdep annotations
Design is similar to the lockdep annotations for workers, but with some twists: - We use a read-lock for the execution/worker/completion side, so that this explicit annotation can be more liberally sprinkled around. With read locks lockdep isn't going to complain if the read-side isn't nested the same way under all circumstances, so ABBA deadlocks are ok. Which they are, since this is an annotation only. - We're using non-recursive lockdep read lock mode, since in recursive read lock mode lockdep does not catch read side hazards. And we _very_ much want read side hazards to be caught. For full details of this limitation see commit e91498589746065e3ae95d9a00b068e525eec34f Author: Peter Zijlstra Date: Wed Aug 23 13:13:11 2017 +0200 locking/lockdep/selftests: Add mixed read-write ABBA tests - To allow nesting of the read-side explicit annotations we explicitly keep track of the nesting. lock_is_held() allows us to do that. - The wait-side annotation is a write lock, and entirely done within dma_fence_wait() for everyone by default. - To be able to freely annotate helper functions I want to make it ok to call dma_fence_begin/end_signalling from soft/hardirq context. First attempt was using the hardirq locking context for the write side in lockdep, but this forces all normal spinlocks nested within dma_fence_begin/end_signalling to be spinlocks. That bollocks. The approach now is to simple check in_atomic(), and for these cases entirely rely on the might_sleep() check in dma_fence_wait(). That will catch any wrong nesting against spinlocks from soft/hardirq contexts. The idea here is that every code path that's critical for eventually signalling a dma_fence should be annotated with dma_fence_begin/end_signalling. The annotation ideally starts right after a dma_fence is published (added to a dma_resv, exposed as a sync_file fd, attached to a drm_syncobj fd, or anything else that makes the dma_fence visible to other kernel threads), up to and including the dma_fence_wait(). Examples are irq handlers, the scheduler rt threads, the tail of execbuf (after the corresponding fences are visible), any workers that end up signalling dma_fences and really anything else. Not annotated should be code paths that only complete fences opportunistically as the gpu progresses, like e.g. shrinker/eviction code. The main class of deadlocks this is supposed to catch are: Thread A: mutex_lock(A); mutex_unlock(A); dma_fence_signal(); Thread B: mutex_lock(A); dma_fence_wait(); mutex_unlock(A); Thread B is blocked on A signalling the fence, but A never gets around to that because it cannot acquire the lock A. Note that dma_fence_wait() is allowed to be nested within dma_fence_begin/end_signalling sections. To allow this to happen the read lock needs to be upgraded to a write lock, which means that any other lock is acquired between the dma_fence_begin_signalling() call and the call to dma_fence_wait(), and still held, this will result in an immediate lockdep complaint. The only other option would be to not annotate such calls, defeating the point. Therefore these annotations cannot be sprinkled over the code entirely mindless to avoid false positives. Originally I hope that the cross-release lockdep extensions would alleviate the need for explicit annotations: https://lwn.net/Articles/709849/ But there's a few reasons why that's not an option: - It's not happening in upstream, since it got reverted due to too many false positives: commit e966eaeeb623f09975ef362c2866fae6f86844f9 Author: Ingo Molnar Date: Tue Dec 12 12:31:16 2017 +0100 locking/lockdep: Remove the cross-release locking checks This code (CONFIG_LOCKDEP_CROSSRELEASE=y and CONFIG_LOCKDEP_COMPLETIONS=y), while it found a number of old bugs initially, was also causing too many false positives that caused people to disable lockdep - which is arguably a worse overall outcome. - cross-release uses the complete() call to annotate the end of critical sections, for dma_fence that would be dma_fence_signal(). But we do not want all dma_fence_signal() calls to be treated as critical, since many are opportunistic cleanup of gpu requests. If these get stuck there's still the main completion interrupt and workers who can unblock everyone. Automatically annotating all dma_fence_signal() calls would hence cause false positives. - cross-release had some educated guesses for when a critical section starts, like fresh syscall or fresh work callback. This would again cause false positives without explicit annotations, since for dma_fence the critical sections only starts when we publish a fence. - Furthermore there can be cases where a thread never does a dma_fence_signal, but is still critical for reaching completion of fences. One example would be a
Re: MIPI DSI, DBI, and tinydrm drivers
Hi Emil, Den 03.06.2020 22.25, skrev Emil Velikov: > Hi Noralf, > > On Wed, 3 Jun 2020 at 13:15, Noralf Trønnes wrote: >> >> Den 28.05.2020 17.27, skrev Emil Velikov: >>> On Sun, 24 May 2020 at 19:35, Daniel Vetter wrote: On Sun, May 24, 2020 at 7:46 PM Noralf Trønnes wrote: > > > > Den 24.05.2020 18.13, skrev Paul Cercueil: >> Hi list, >> >> I'd like to open a discussion about the current support of MIPI DSI and >> DBI panels. >> >> Both are standards from the MIPI alliance, both are communication >> protocols between a LCD controller and a LCD panel, they generally both >> use the same commands (DCS), the main difference is that DSI is serial >> and DBI is generally parallel. >> >> In the kernel right now, DSI is pretty well implemented. All the >> infrastucture to register a DSI host, DSI device etc. is there. DSI >> panels are implemented as regular drm_panel instances, and their drivers >> go through the DSI API to communicate with the panel, which makes them >> independent of the DSI host driver. >> >> DBI, on the other hand, does not have any of this. All (?) DBI panels >> are implemented as tinydrm drivers, which make them impossible to use >> with regular DRM drivers. Writing a standard drm_panel driver is >> impossible, as there is no concept of host and device. All these tinydrm >> drivers register their own DBI host as they all do DBI over SPI. >> >> I think this needs a good cleanup. Given that DSI and DBI are so >> similar, it would probably make sense to fuse DBI support into the >> current DSI code, as trying to update DBI would result in a lot of code >> being duplicated. With the proper host/device registration mechanism >> from DSI code, it would be possible to turn most of the tinydrm drivers >> into regular drm_panel drivers. Do we have drivers with dbi support that actually want to reuse the tinydrm drivers? Good clean is all good, but we need a solid reason for changing stuff. Plus we need to make sure we're not just rediscovering all the old reasons for why we ended up where we are right now in the first place. >> The problem then is that these should still be available as tinydrm >> drivers. If the DSI/DBI panels can somehow register a .update_fb() >> callback, it would make it possible to have a panel-agnostic tinydrm >> driver, which would then probably open a lot of doors, and help a lot to >> clean the mess. >> >> I think I can help with that, I just need some guidance - I am fishing >> in exotic seas here. >> >> Thoughts, comments, are very welcome. > > I did look at this a few months back: > > drm/mipi-dbi: Support panel drivers > https://lists.freedesktop.org/archives/dri-devel/2019-August/228966.html > >>> Coming late to the party - the series looks like a great step forward. >>> > The problem with DBI is that it has reused other busses which means we > don't have DBI drivers, we have SPI drivers instead (6800/8080 is not > avail. as busses in Linux yet). DSI and DPI on the other hand has > dedicated hw controller drivers not shared with other subsystems. > > My initial tinydrm work used drm_panel, but I was not allowed to use it > (at least not the way I had done it). Hm, do we have a summary of all the discussions/reasons from back then? All I remember is that it's all that simple, you've done a lot of work exploring all the options, I'm fairly sure I suggested drm_panel even back then but somehow it didn't really work. Would be good if we make sure we don't at least repeat history too much :-) >>> This pretty much ^^. Does anyone have a link/summary of the concerns? >>> >> >> I found the thread where you Emil suggested I look at drm_panel: >> >> https://lists.freedesktop.org/archives/dri-devel/2015-September/091215.html >> > Guilty as charged ;-) > I guess it turns out that you were right :-) > Guess I should ask some silly questions first: > Was tinydrm modelled as a drm driver itself, because the idea of > drm_panel::update() callback seemed dirty? That's the only concern > raised that I can find on the list... It's effectively in the link you > provided. > > As far as I can tell, first RFC was already using the tiny drm driver model. > https://patchwork.freedesktop.org/patch/77161/ > > Yet again, do we actually need the callback? The mipi-dbi(?) spi > panels in panel/ get away w/o one, while pushing far more pixels onto > the screen (tiny has resolutions up-to 320x480, panel up-to 480x800). > > > That said, I'm a fan of lifting the tiny (panel) drivers into > drm-panel and exposing them via dbi-bus sounds reasonable IMHO. Seems > like Paul has the DT dbi/spi bus questions covered as well. > > Patches illustrating his ideas would be more than welcome. > +1
Re: [PATCH 13/18] drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
On Fri, Jun 5, 2020 at 10:30 AM Pierre-Eric Pelloux-Prayer wrote: > > Hi Daniel, > > On 04/06/2020 10:12, Daniel Vetter wrote: > [...] > > @@ -6910,7 +6910,11 @@ static void amdgpu_dm_commit_planes(struct > > drm_atomic_state *state, > >* explicitly on fences instead > >* and in general should be called for > >* blocking commit to as per framework helpers > > + * > > + * Yes, this deadlocks, since you're calling dma_resv_lock in > > a > > + * path that leads to a dma_fence_signal(). Don't do that. > >*/ > > +#if 0 > > r = amdgpu_bo_reserve(abo, true); > > if (unlikely(r != 0)) > > DRM_ERROR("failed to reserve buffer before flip\n"); > > @@ -6920,6 +6924,12 @@ static void amdgpu_dm_commit_planes(struct > > drm_atomic_state *state, > > tmz_surface = amdgpu_bo_encrypted(abo); > > > > amdgpu_bo_unreserve(abo); > > +#endif > > + /* > > + * this races anyway, so READ_ONCE isn't any better or worse > > + * than the stuff above. Except the stuff above can deadlock. > > + */ > > + tiling_flags = READ_ONCE(abo->tiling_flags); > > With this change "tmz_surface" won't be initialized properly. > Adding the following line should fix it: > > tmz_surface = READ_ONCE(abo->flags) & AMDGPU_GEM_CREATE_ENCRYPTED; So to make this clear, I'm not really proposing to fix up all the drivers in detail. There's a lot more bugs in all the other drivers, I'm pretty sure. The driver fixups really are just quick hacks to illustrate the problem, and at least in some cases, maybe illustrate a possible solution. For the real fixes I think this needs driver teams working on this, and make sure it's all solid. I can help a bit with review (especially for placing the annotations, e.g. the one I put in cs_submit() annotates a bit too much), but that's it. Also I think the patch is from before tmz landed, and I just blindly rebased over it :-) -Daniel > > > Pierre-Eric > > > > > > fill_dc_plane_info_and_addr( > > dm->adev, new_plane_state, tiling_flags, > > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm: cleanup on drm_mode_object_find
Redundant wrapper for drm_mode_object_find is removed. Signed-off-by: Ramalingam C --- drivers/gpu/drm/drm_crtc_internal.h | 6 ++--- drivers/gpu/drm/drm_framebuffer.c | 2 +- drivers/gpu/drm/drm_mode_object.c | 38 +++-- drivers/gpu/drm/drm_property.c | 6 ++--- 4 files changed, 21 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h index da96b2f64d7e..4bfde1367c1a 100644 --- a/drivers/gpu/drm/drm_crtc_internal.h +++ b/drivers/gpu/drm/drm_crtc_internal.h @@ -144,9 +144,9 @@ int drm_mode_object_add(struct drm_device *dev, struct drm_mode_object *obj, uint32_t obj_type); void drm_mode_object_register(struct drm_device *dev, struct drm_mode_object *obj); -struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev, - struct drm_file *file_priv, - uint32_t id, uint32_t type); +struct drm_mode_object *drm_mode_object_find(struct drm_device *dev, +struct drm_file *file_priv, +uint32_t id, uint32_t type); void drm_mode_object_unregister(struct drm_device *dev, struct drm_mode_object *object); int drm_mode_object_get_properties(struct drm_mode_object *obj, bool atomic, diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c index 0375b3d7f8d0..023cea64e87d 100644 --- a/drivers/gpu/drm/drm_framebuffer.c +++ b/drivers/gpu/drm/drm_framebuffer.c @@ -888,7 +888,7 @@ struct drm_framebuffer *drm_framebuffer_lookup(struct drm_device *dev, struct drm_mode_object *obj; struct drm_framebuffer *fb = NULL; - obj = __drm_mode_object_find(dev, file_priv, id, DRM_MODE_OBJECT_FB); + obj = drm_mode_object_find(dev, file_priv, id, DRM_MODE_OBJECT_FB); if (obj) fb = obj_to_fb(obj); return fb; diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c index 901b078abf40..6c5e689d8b60 100644 --- a/drivers/gpu/drm/drm_mode_object.c +++ b/drivers/gpu/drm/drm_mode_object.c @@ -133,9 +133,20 @@ bool drm_mode_object_lease_required(uint32_t type) } } -struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev, - struct drm_file *file_priv, - uint32_t id, uint32_t type) +/** + * drm_mode_object_find - look up a drm object with static lifetime + * @dev: drm device + * @file_priv: drm file + * @id: id of the mode object + * @type: type of the mode object + * + * This function is used to look up a modeset object. It will acquire a + * reference for reference counted objects. This reference must be dropped again + * by callind drm_mode_object_put(). + */ +struct drm_mode_object *drm_mode_object_find(struct drm_device *dev, +struct drm_file *file_priv, +uint32_t id, uint32_t type) { struct drm_mode_object *obj = NULL; @@ -158,27 +169,6 @@ struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev, return obj; } - -/** - * drm_mode_object_find - look up a drm object with static lifetime - * @dev: drm device - * @file_priv: drm file - * @id: id of the mode object - * @type: type of the mode object - * - * This function is used to look up a modeset object. It will acquire a - * reference for reference counted objects. This reference must be dropped again - * by callind drm_mode_object_put(). - */ -struct drm_mode_object *drm_mode_object_find(struct drm_device *dev, - struct drm_file *file_priv, - uint32_t id, uint32_t type) -{ - struct drm_mode_object *obj = NULL; - - obj = __drm_mode_object_find(dev, file_priv, id, type); - return obj; -} EXPORT_SYMBOL(drm_mode_object_find); /** diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c index 6ee04803c362..f1e338f909f1 100644 --- a/drivers/gpu/drm/drm_property.c +++ b/drivers/gpu/drm/drm_property.c @@ -656,7 +656,7 @@ struct drm_property_blob *drm_property_lookup_blob(struct drm_device *dev, struct drm_mode_object *obj; struct drm_property_blob *blob = NULL; - obj = __drm_mode_object_find(dev, NULL, id, DRM_MODE_OBJECT_BLOB); + obj = drm_mode_object_find(dev, NULL, id, DRM_MODE_OBJECT_BLOB); if (obj) blob = obj_to_blob(obj); return blob; @@ -919,8 +919,8 @@ bool drm_property_change_valid_get(struct drm_property *property, if (value == 0) return true; - *ref = __drm_mode_object_find(property->dev, NULL, value, - property->values[0]); +
[PATCH 2/2] drm: Fix kdoc typo for drm_properties
Typo in the KDoc for the DRM_MODE_PROP_SIGNED_RANGE type of drm_properties is fixed. Signed-off-by: Ramalingam C --- include/drm/drm_property.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h index 4a0a80d658c7..5d1874b6ccfc 100644 --- a/include/drm/drm_property.h +++ b/include/drm/drm_property.h @@ -94,7 +94,7 @@ struct drm_property { * drm_property_create_range(). * * DRM_MODE_PROP_SIGNED_RANGE -* Range properties report their minimum and maximum admissible unsigned values. +* Range properties report their minimum and maximum admissible signed values. * The KMS core verifies that values set by application fit in that * range. The range is signed. Range properties are created using * drm_property_create_signed_range(). -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 4/6] drm: Add Generic USB Display driver
Den 02.06.2020 20.38, skrev Peter Stuge: > Alan Stern wrote: A gadget driver can STALL in response to a control-OUT data packet, but only before it has seen the packet. >>> >>> How can it do that for OUT, and IN if possible there too? >> >> In the way described just above: The gadget driver's SETUP handler tells >> the UDC to stall the data packet. >> >>> Is it related to f->setup() returning < 0 ? >> >> Yes; the composite core interprets such a value as meaning to STALL >> endpoint 0. > > Thanks a lot for confirming this. > > >>> The spec also allows NAK, but the gadget code seems to not (need to?) >>> explicitly support that. Can you comment on this as well? >> >> If the gadget driver doesn't submit a usb_request then the UDC will >> reply with NAK. > > And thanks for this as well. > > >>> a status request so I can know the result of the operation the device has >>> performed. > .. >> There are two reasonable approaches you could use. One is to have a >> vendor-specific control request to get the result of the preceding >> operation. > .. >> The other approach is to send the status data over a bulk-IN endpoint. > > I've tried to explain a third approach, which I think fits well because the > status is only a "Ready" flag - ie. a memory barrier or flow control, > to make the host not send data OUT. > > I'm proposing that the gadget should NAK on data OUT when it isn't Ready, and > that the host driver shouldn't query status but simply send data when it has. > > Per Alans description the NAK happens automatically if the gadget driver has > no usb_request pending while it is processing previously received data. > > On the host that NAK makes the host controller retry automatically until > transfer success, timeout or fatal error. > > >> It would have to be formatted in such a way that the host could >> recognize it as a status packet and not some other sort of data packet. > > For host notification of status changes other than Ready I really like > such an IN endpoint, but preferably an interrupt endpoint. > > To avoid the formatting problem each data packet could be one full status > message. That way the host would always receive a known data structure. > Interrupt packets can be max 64 byte. Noralf, do you think that's enough > for everyone in the first version? > I'm going through some treatment that turned out to be worse than expected, so sadly I have to put this project on hold until my body recovers. Noralf. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 09/13] device core: Introduce multiple dma pfn offsets
Hi Andy, On Thu, Jun 4, 2020 at 11:05 AM Andy Shevchenko wrote: > > On Thu, Jun 04, 2020 at 10:35:12AM -0400, Jim Quinlan wrote: > > On Thu, Jun 4, 2020 at 9:53 AM Nicolas Saenz Julienne > > wrote: > > > On Wed, 2020-06-03 at 15:20 -0400, Jim Quinlan wrote: > > ... > > > > > + phys = virt_to_phys(ret); > > > > + pfn = phys >> PAGE_SHIFT; > > > > > > nit: not sure it really pays off to have a pfn variable here. > > Did it for readability; the compiler's optimization should take care > > of any extra variables. But I can switch if you insist. > > One side note: please, try to get familiar with existing helpers in the > kernel. > For example, above line is like > > pfn = PFN_DOWN(phys); I just used the term in the original code; will change to PFN_DOWN(). > > ... > > > > > + if (!WARN_ON(!dev) && dev->dma_pfn_offset_map) > > > > > + *dma_handle -= PFN_PHYS( > > > > + dma_pfn_offset_from_phys_addr(dev, phys)); > > Don't do such indentation, esp. we have now 100! :-) Got it. Thanks, Jim Quinlan > > -- > With Best Regards, > Andy Shevchenko > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 09/13] device core: Introduce multiple dma pfn offsets
Hi Dan, On Thu, Jun 4, 2020 at 7:06 AM Dan Carpenter wrote: > > On Wed, Jun 03, 2020 at 03:20:41PM -0400, Jim Quinlan wrote: > > @@ -786,7 +787,7 @@ static int sun4i_backend_bind(struct device *dev, > > struct device *master, > > const struct sun4i_backend_quirks *quirks; > > struct resource *res; > > void __iomem *regs; > > - int i, ret; > > + int i, ret = 0; > > No need for this. Will fix. > > > > > backend = devm_kzalloc(dev, sizeof(*backend), GFP_KERNEL); > > if (!backend) > > @@ -812,7 +813,9 @@ static int sun4i_backend_bind(struct device *dev, > > struct device *master, > >* on our device since the RAM mapping is at 0 for the DMA > > bus, > >* unlike the CPU. > >*/ > > - drm->dev->dma_pfn_offset = PHYS_PFN_OFFSET; > > + ret = attach_uniform_dma_pfn_offset(dev, PHYS_PFN_OFFSET); > > + if (ret) > > + return ret; > > } > > > > backend->engine.node = dev->of_node; > > diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c > > index 04fbd4bf0ff9..e9cc1c2d47cd 100644 > > --- a/drivers/iommu/io-pgtable-arm.c > > +++ b/drivers/iommu/io-pgtable-arm.c > > @@ -754,7 +754,7 @@ arm_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg) > > if (cfg->oas > ARM_LPAE_MAX_ADDR_BITS) > > return NULL; > > > > - if (!selftest_running && cfg->iommu_dev->dma_pfn_offset) { > > + if (!selftest_running && cfg->iommu_dev->dma_pfn_offset_map) { > > dev_err(cfg->iommu_dev, "Cannot accommodate DMA offset for > > IOMMU page tables\n"); > > return NULL; > > } > > diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c > > b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c > > index eff34ded6305..7212da5e1076 100644 > > --- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c > > +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c > > @@ -7,6 +7,7 @@ > > */ > > > > #include > > +#include > > #include > > #include > > #include > > @@ -183,7 +184,9 @@ static int sun4i_csi_probe(struct platform_device *pdev) > > return ret; > > } else { > > #ifdef PHYS_PFN_OFFSET > > - csi->dev->dma_pfn_offset = PHYS_PFN_OFFSET; > > + ret = attach_uniform_dma_pfn_offset(dev, PHYS_PFN_OFFSET); > > + if (ret) > > + return ret; > > #endif > > } > > > > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c > > b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c > > index 055eb0b8e396..2d66d415b6c3 100644 > > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c > > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c > > @@ -898,7 +898,10 @@ static int sun6i_csi_probe(struct platform_device > > *pdev) > > > > sdev->dev = >dev; > > /* The DMA bus has the memory mapped at 0 */ > > - sdev->dev->dma_pfn_offset = PHYS_OFFSET >> PAGE_SHIFT; > > + ret = attach_uniform_dma_pfn_offset(sdev->dev, > > + PHYS_OFFSET >> PAGE_SHIFT); > > + if (ret) > > + return ret; > > > > ret = sun6i_csi_resource_request(sdev, pdev); > > if (ret) > > diff --git a/drivers/of/address.c b/drivers/of/address.c > > index 96d8cfb14a60..c89333b0a5fb 100644 > > --- a/drivers/of/address.c > > +++ b/drivers/of/address.c > > @@ -918,6 +918,70 @@ void __iomem *of_io_request_and_map(struct device_node > > *np, int index, > > } > > EXPORT_SYMBOL(of_io_request_and_map); > > > > +static int attach_dma_pfn_offset_map(struct device *dev, > > + struct device_node *node, int num_ranges) > > +{ > > + struct of_range_parser parser; > > + struct of_range range; > > + struct dma_pfn_offset_region *r; > > + > > + r = devm_kcalloc(dev, num_ranges + 1, > > + sizeof(struct dma_pfn_offset_region), GFP_KERNEL); > > + if (!r) > > + return -ENOMEM; > > + dev->dma_pfn_offset_map = r; > > + of_dma_range_parser_init(, node); > > + > > + /* > > + * Record all info for DMA ranges array. We could > > + * just use the of_range struct, but if we did that it > > + * would require more calculations for phys_to_dma and > > + * dma_to_phys conversions. > > + */ > > + for_each_of_range(, ) { > > + r->cpu_start = range.cpu_addr; > > + r->cpu_end = r->cpu_start + range.size - 1; > > + r->dma_start = range.bus_addr; > > + r->dma_end = r->dma_start + range.size - 1; > > + r->pfn_offset = PFN_DOWN(range.cpu_addr) > > + - PFN_DOWN(range.bus_addr); > > + r++; > > + } > > + return 0; > > +} > > + > > + > > + > > +/** > > + * attach_dma_pfn_offset - Assign scalar offset for all addresses. > > + * @dev: device pointer; only needed for a corner case. >
Re: [PATCH v3 09/13] device core: Introduce multiple dma pfn offsets
On Thu, Jun 4, 2020 at 9:53 AM Nicolas Saenz Julienne wrote: > > Hi Jim, > > On Wed, 2020-06-03 at 15:20 -0400, Jim Quinlan wrote: > > The new field in struct device 'dma_pfn_offset_map' is used to facilitate > > the use of multiple pfn offsets between cpu addrs and dma addrs. It > > subsumes the role of dev->dma_pfn_offset -- a uniform offset -- and > > designates the single offset a special case. > > > > of_dma_configure() is the typical manner to set pfn offsets but there > > are a number of ad hoc assignments to dev->dma_pfn_offset in the > > kernel code. These cases now invoke the function > > attach_uniform_dma_pfn_offset(dev, pfn_offset). > > > > Signed-off-by: Jim Quinlan > > --- > > arch/arm/include/asm/dma-mapping.h| 9 +- > > arch/arm/mach-keystone/keystone.c | 9 +- > > arch/sh/drivers/pci/pcie-sh7786.c | 3 +- > > arch/sh/kernel/dma-coherent.c | 17 ++-- > > arch/x86/pci/sta2x11-fixup.c | 7 +- > > drivers/acpi/arm64/iort.c | 5 +- > > drivers/gpu/drm/sun4i/sun4i_backend.c | 7 +- > > drivers/iommu/io-pgtable-arm.c| 2 +- > > .../platform/sunxi/sun4i-csi/sun4i_csi.c | 5 +- > > .../platform/sunxi/sun6i-csi/sun6i_csi.c | 5 +- > > drivers/of/address.c | 93 +-- > > drivers/of/device.c | 8 +- > > drivers/remoteproc/remoteproc_core.c | 2 +- > > .../staging/media/sunxi/cedrus/cedrus_hw.c| 7 +- > > drivers/usb/core/message.c| 4 +- > > drivers/usb/core/usb.c| 2 +- > > include/linux/device.h| 4 +- > > include/linux/dma-direct.h| 16 +++- > > include/linux/dma-mapping.h | 45 + > > kernel/dma/coherent.c | 11 ++- > > 20 files changed, 210 insertions(+), 51 deletions(-) > > > > diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma- > > mapping.h > > index bdd80ddbca34..f1e72f99468b 100644 > > --- a/arch/arm/include/asm/dma-mapping.h > > +++ b/arch/arm/include/asm/dma-mapping.h > > @@ -35,8 +35,9 @@ static inline const struct dma_map_ops > > *get_arch_dma_ops(struct bus_type *bus) > > #ifndef __arch_pfn_to_dma > > static inline dma_addr_t pfn_to_dma(struct device *dev, unsigned long pfn) > > { > > - if (dev) > > - pfn -= dev->dma_pfn_offset; > > + if (dev && dev->dma_pfn_offset_map) > > Would it make sense to move the dev->dma_pfn_offset_map check into > dma_pfn_offset_from_phys_addr() and return 0 if not available? Same for the > opposite variant of the function. I think it'd make the code a little simpler > on > some of the use cases, and overall less error prone if anyone starts using the > function elsewhere. Yes it makes sense and I was debating doing it but I just wanted to make it explicit that there was not much cost for this change for the fastpath -- no dma_pfn_offset whatsoever -- as the cost goes from a "pfn += dev->dma_pfn_offset" to a "if (dev->dma_pfn_offset_map)". I will do what you suggest. > > > + pfn -= dma_pfn_offset_from_phys_addr(dev, PFN_PHYS(pfn)); > > + > > return (dma_addr_t)__pfn_to_bus(pfn); > > } > > > > @@ -44,8 +45,8 @@ static inline unsigned long dma_to_pfn(struct device *dev, > > dma_addr_t addr) > > { > > unsigned long pfn = __bus_to_pfn(addr); > > > > - if (dev) > > - pfn += dev->dma_pfn_offset; > > + if (dev && dev->dma_pfn_offset_map) > > + pfn += dma_pfn_offset_from_dma_addr(dev, addr); > > > > return pfn; > > } > > diff --git a/arch/arm/mach-keystone/keystone.c b/arch/arm/mach- > > keystone/keystone.c > > index 638808c4e122..e7d3ee6e9cb5 100644 > > --- a/arch/arm/mach-keystone/keystone.c > > +++ b/arch/arm/mach-keystone/keystone.c > > @@ -8,6 +8,7 @@ > > */ > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -38,9 +39,11 @@ static int keystone_platform_notifier(struct > > notifier_block > > *nb, > > return NOTIFY_BAD; > > > > if (!dev->of_node) { > > - dev->dma_pfn_offset = keystone_dma_pfn_offset; > > - dev_err(dev, "set dma_pfn_offset%08lx\n", > > - dev->dma_pfn_offset); > > + int ret = attach_uniform_dma_pfn_offset > > + (dev, keystone_dma_pfn_offset); > > + > > + dev_err(dev, "set dma_pfn_offset%08lx%s\n", > > + dev->dma_pfn_offset, ret ? " failed" : ""); > > } > > return NOTIFY_OK; > > } > > diff --git a/arch/sh/drivers/pci/pcie-sh7786.c b/arch/sh/drivers/pci/pcie- > > sh7786.c > > index e0b568aaa701..2e832a5c58c1 100644 > > --- a/arch/sh/drivers/pci/pcie-sh7786.c > > +++ b/arch/sh/drivers/pci/pcie-sh7786.c > > @@ -12,6 +12,7 @@ > > #include > > #include > > #include > > +#include > >
Re: [PATCH v3 09/13] device core: Introduce multiple dma pfn offsets
Hi Nicolas, On Thu, Jun 4, 2020 at 12:52 PM Nicolas Saenz Julienne wrote: > > Hi Jim, > > On Thu, 2020-06-04 at 10:35 -0400, Jim Quinlan wrote: > > [...] > > > > > --- a/arch/sh/kernel/dma-coherent.c > > > > +++ b/arch/sh/kernel/dma-coherent.c > > > > @@ -14,6 +14,8 @@ void *arch_dma_alloc(struct device *dev, size_t size, > > > > dma_addr_t *dma_handle, > > > > { > > > > void *ret, *ret_nocache; > > > > int order = get_order(size); > > > > + unsigned long pfn; > > > > + phys_addr_t phys; > > > > > > > > gfp |= __GFP_ZERO; > > > > > > > > @@ -34,11 +36,14 @@ void *arch_dma_alloc(struct device *dev, size_t > > > > size, > > > > dma_addr_t *dma_handle, > > > > return NULL; > > > > } > > > > > > > > - split_page(pfn_to_page(virt_to_phys(ret) >> PAGE_SHIFT), order); > > > > + phys = virt_to_phys(ret); > > > > + pfn = phys >> PAGE_SHIFT; > > > > > > nit: not sure it really pays off to have a pfn variable here. > > Did it for readability; the compiler's optimization should take care > > of any extra variables. But I can switch if you insist. > > No need. > > [...] > > > > > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c > > > > b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c > > > > index 055eb0b8e396..2d66d415b6c3 100644 > > > > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c > > > > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c > > > > @@ -898,7 +898,10 @@ static int sun6i_csi_probe(struct platform_device > > > > *pdev) > > > > > > > > sdev->dev = >dev; > > > > /* The DMA bus has the memory mapped at 0 */ > > > > - sdev->dev->dma_pfn_offset = PHYS_OFFSET >> PAGE_SHIFT; > > > > + ret = attach_uniform_dma_pfn_offset(sdev->dev, > > > > + PHYS_OFFSET >> PAGE_SHIFT); > > > > + if (ret) > > > > + return ret; > > > > > > > > ret = sun6i_csi_resource_request(sdev, pdev); > > > > if (ret) > > > > diff --git a/drivers/of/address.c b/drivers/of/address.c > > > > index 96d8cfb14a60..c89333b0a5fb 100644 > > > > --- a/drivers/of/address.c > > > > +++ b/drivers/of/address.c > > > > @@ -918,6 +918,70 @@ void __iomem *of_io_request_and_map(struct > > > > device_node > > > > *np, int index, > > > > } > > > > EXPORT_SYMBOL(of_io_request_and_map); > > > > > > > > +static int attach_dma_pfn_offset_map(struct device *dev, > > > > + struct device_node *node, int > > > > num_ranges) > > > > > > As with the previous review, please take this comment with a grain of > > > salt. > > > > > > I think there should be a clear split between what belongs to OF and what > > > belongs to the core device infrastructure. > > > > > > OF's job should be to parse DT and provide a list/array of ranges, whereas > > > the > > > core device infrastructure should provide an API to assign a list of > > > ranges/offset to a device. > > > > > > As a concrete example, you're forcing devices like the sta2x11 to build > > > with > > > OF > > > support, which, being an Intel device, it's pretty odd. But I'm also > > > thinking > > > of how will all this fit once an ACPI device wants to use it. > > To fix this I only have to move attach_uniform_dma_pfn_offset() from > > of/address.c to say include/linux/dma-mapping.h. It has no > > dependencies on OF. Do you agree? > > Yes that seems nicer. In case you didn't had it in mind already, I'd change > the > function name to match the naming scheme they use there. > > On the other hand, I'd also move the non OF parts of the non unifom dma_offset > version of the function there. > > > > Expanding on this idea, once you have a split between the OF's and device > > > core > > > roles, it transpires that of_dma_get_range()'s job should only be to > > > provide > > > the ranges in a device understandable structure and of_dma_configre()'s to > > > actually assign the device's parameters. This would obsolete patch #7. > > > > I think you mean patch #8. > > Yes, my bad. > > > I agree with you. The reason I needed a "struct device *" in the call is > > because I wanted to make sure the memory that is alloc'd belongs to the > > device that needs it. If I do a regular kzalloc(), this memory will become > > a leak once someone starts unbinding/binding their device. Also, in all > > uses of of_dma_rtange() -- there is only one -- a dev is required as one > > can't attach an offset map to NULL. > > > > I do see that there are a number of functions in drivers/of/*.c that > > take 'struct device *dev' as an argument so there is precedent for > > something like this. Regardless, I need an owner to the memory I > > alloc(). > > I understand the need for dev to be around, devm_*() is key. But also it's > important to keep the functions on purpose. And if of_dma_get_range() starts > setting ranges it calls, for the very least, for a function rename. Although > I'd rather split the parsing and setting
Re: [PATCH v3 09/13] device core: Introduce multiple dma pfn offsets
On Thu, Jun 4, 2020 at 10:20 AM Dan Carpenter wrote: > > On Thu, Jun 04, 2020 at 09:48:49AM -0400, Jim Quinlan wrote: > > > > + r = devm_kcalloc(dev, 1, sizeof(struct dma_pfn_offset_region), > > > > + GFP_KERNEL); > > > > > > Use:r = devm_kzalloc(dev, sizeof(*r), GFP_KERNEL); > > Will fix. > > > > > > > > > > > > + if (!r) > > > > + return -ENOMEM; > > > > + > > > > + r->uniform_offset = true; > > > > + r->pfn_offset = pfn_offset; > > > > + > > > > + return 0; > > > > +} > > > > > > This function doesn't seem to do anything useful. Is part of it > > > missing? > > No, the uniform pfn offset is a special case. > > Sorry, I wasn't clear. We're talking about different things. The code > does: > > r = devm_kzalloc(dev, sizeof(*r), GFP_KERNEL); > if (!r) > return -ENOMEM; > > r->uniform_offset = true; > r->pfn_offset = pfn_offset; > > return 0; > > The code allocates "r" and then doesn't save it anywhere so there is > no point. You are absolutely right, sorry I missed your point. Will fix. Thanks, Jim Quinlan > > regards, > dan carpenter > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 01/43] drm/cma-helper: Rename symbols from drm_cma_gem_ to drm_gem_cma_
Hi Thomas, Thank you for the patch. On Fri, Jun 05, 2020 at 09:32:05AM +0200, Thomas Zimmermann wrote: > This fixes the naming of several symbols within CMA helpers. No functional > changes are made. > > Signed-off-by: Thomas Zimmermann Thank you for the patch. Speaking of naming, I wish we could rename drm_gem_cma_* to something else, as those helpers don't use CMA directly (and may not use it at all), but I think that would be too much intrusive changes for too little gain :-( > --- > drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 2 +- > drivers/gpu/drm/drm_gem_cma_helper.c| 10 +- > include/drm/drm_gem_cma_helper.h| 4 ++-- > 3 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c > b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c > index 6b27242b9ee3c..5e7ea0459d018 100644 > --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c > +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c > @@ -188,7 +188,7 @@ DEFINE_DRM_GEM_CMA_FOPS(fops); > > static struct drm_driver aspeed_gfx_driver = { > .driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, > - .gem_create_object = drm_cma_gem_create_object_default_funcs, > + .gem_create_object = drm_gem_cma_create_object_default_funcs, > .dumb_create= drm_gem_cma_dumb_create, > .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > .prime_fd_to_handle = drm_gem_prime_fd_to_handle, > diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c > b/drivers/gpu/drm/drm_gem_cma_helper.c > index b3db3ca7bd7a7..842e2fa332354 100644 > --- a/drivers/gpu/drm/drm_gem_cma_helper.c > +++ b/drivers/gpu/drm/drm_gem_cma_helper.c > @@ -572,7 +572,7 @@ void drm_gem_cma_prime_vunmap(struct drm_gem_object *obj, > void *vaddr) > } > EXPORT_SYMBOL_GPL(drm_gem_cma_prime_vunmap); > > -static const struct drm_gem_object_funcs drm_cma_gem_default_funcs = { > +static const struct drm_gem_object_funcs drm_gem_cma_default_funcs = { > .free = drm_gem_cma_free_object, > .print_info = drm_gem_cma_print_info, > .get_sg_table = drm_gem_cma_prime_get_sg_table, > @@ -581,7 +581,7 @@ static const struct drm_gem_object_funcs > drm_cma_gem_default_funcs = { > }; > > /** > - * drm_cma_gem_create_object_default_funcs - Create a CMA GEM object with a > + * drm_gem_cma_create_object_default_funcs - Create a CMA GEM object with a > * default function table > * @dev: DRM device > * @size: Size of the object to allocate > @@ -593,7 +593,7 @@ static const struct drm_gem_object_funcs > drm_cma_gem_default_funcs = { > * A pointer to a allocated GEM object or an error pointer on failure. > */ > struct drm_gem_object * > -drm_cma_gem_create_object_default_funcs(struct drm_device *dev, size_t size) > +drm_gem_cma_create_object_default_funcs(struct drm_device *dev, size_t size) > { > struct drm_gem_cma_object *cma_obj; > > @@ -601,11 +601,11 @@ drm_cma_gem_create_object_default_funcs(struct > drm_device *dev, size_t size) > if (!cma_obj) > return NULL; > > - cma_obj->base.funcs = _cma_gem_default_funcs; > + cma_obj->base.funcs = _gem_cma_default_funcs; > > return _obj->base; > } > -EXPORT_SYMBOL(drm_cma_gem_create_object_default_funcs); > +EXPORT_SYMBOL(drm_gem_cma_create_object_default_funcs); > > /** > * drm_gem_cma_prime_import_sg_table_vmap - PRIME import another driver's > diff --git a/include/drm/drm_gem_cma_helper.h > b/include/drm/drm_gem_cma_helper.h > index 947ac95eb24a9..64b7e9d42129a 100644 > --- a/include/drm/drm_gem_cma_helper.h > +++ b/include/drm/drm_gem_cma_helper.h > @@ -107,7 +107,7 @@ void *drm_gem_cma_prime_vmap(struct drm_gem_object *obj); > void drm_gem_cma_prime_vunmap(struct drm_gem_object *obj, void *vaddr); > > struct drm_gem_object * > -drm_cma_gem_create_object_default_funcs(struct drm_device *dev, size_t size); > +drm_gem_cma_create_object_default_funcs(struct drm_device *dev, size_t size); > > /** > * DRM_GEM_CMA_VMAP_DRIVER_OPS - CMA GEM driver operations ensuring a virtual > @@ -118,7 +118,7 @@ drm_cma_gem_create_object_default_funcs(struct drm_device > *dev, size_t size); > * imported buffers. > */ > #define DRM_GEM_CMA_VMAP_DRIVER_OPS \ > - .gem_create_object = drm_cma_gem_create_object_default_funcs, \ > + .gem_create_object = drm_gem_cma_create_object_default_funcs, \ > .dumb_create= drm_gem_cma_dumb_create, \ > .prime_handle_to_fd = drm_gem_prime_handle_to_fd, \ > .prime_fd_to_handle = drm_gem_prime_fd_to_handle, \ -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/1] drm/mm: add ig_frag selftest
Am 05.06.20 um 10:18 schrieb Nirmoy: On 6/5/20 9:45 AM, Christian König wrote: Am 03.06.20 um 12:32 schrieb Nirmoy Das: This patch introduces fragmentation in the address range and measures time taken by 10k and 20k insertions. ig_frag() will fail if the time taken by 20k insertions takes more than 4 times of 10k insertions as we know that insertions should at most scale quadratically. v2: introduce fragmentation by freeing every other node. only test bottom-up and top-down for now. Signed-off-by: Nirmoy Das --- drivers/gpu/drm/selftests/drm_mm_selftests.h | 1 + drivers/gpu/drm/selftests/test-drm_mm.c | 124 +++ 2 files changed, 125 insertions(+) diff --git a/drivers/gpu/drm/selftests/drm_mm_selftests.h b/drivers/gpu/drm/selftests/drm_mm_selftests.h index 6b943ea1c57d..8c87c964176b 100644 --- a/drivers/gpu/drm/selftests/drm_mm_selftests.h +++ b/drivers/gpu/drm/selftests/drm_mm_selftests.h @@ -14,6 +14,7 @@ selftest(insert, igt_insert) selftest(replace, igt_replace) selftest(insert_range, igt_insert_range) selftest(align, igt_align) +selftest(frag, igt_frag) selftest(align32, igt_align32) selftest(align64, igt_align64) selftest(evict, igt_evict) diff --git a/drivers/gpu/drm/selftests/test-drm_mm.c b/drivers/gpu/drm/selftests/test-drm_mm.c index 9aabe82dcd3a..34231baacd87 100644 --- a/drivers/gpu/drm/selftests/test-drm_mm.c +++ b/drivers/gpu/drm/selftests/test-drm_mm.c @@ -10,6 +10,7 @@ #include #include #include +#include #include @@ -1033,6 +1034,129 @@ static int igt_insert_range(void *ignored) return 0; } +static int prepare_igt_frag(struct drm_mm *mm, + struct drm_mm_node *nodes, + unsigned int num_insert, + const struct insert_mode *mode) +{ + unsigned int size = 4096; + unsigned int i; + u64 ret = -EINVAL; + + for (i = 0; i < num_insert; i++) { + if (!expect_insert(mm, [i], size, 0, i, + mode) != 0) { + pr_err("%s insert failed\n", mode->name); + goto out; + } + } + + /* introduce fragmentation by freeing every other node */ + for (i = 0; i < num_insert; i++) { + if (i % 2 == 0) + drm_mm_remove_node([i]); + } + +out: + return ret; + +} + +static u64 get_insert_time(struct drm_mm *mm, + unsigned int num_insert, + struct drm_mm_node *nodes, + const struct insert_mode *mode) +{ + unsigned int size = 8192; + ktime_t start; + unsigned int i; + u64 ret = -EINVAL; + + start = ktime_get(); + for (i = 0; i < num_insert; i++) { + if (!expect_insert(mm, [i], size, 0, i, mode) != 0) { + pr_err("%s insert failed\n", mode->name); + goto out; + } + } + + ret = ktime_to_ns(ktime_sub(ktime_get(), start)); + +out: + return ret; + +} + +static int igt_frag(void *ignored) +{ + struct drm_mm mm; + const struct insert_mode *mode; + struct drm_mm_node *nodes, *node, *next; + unsigned int insert_size = 1; + unsigned int scale_factor = 4; + int ret = -EINVAL; + + /* We need 4 * insert_size nodes to hold intermediate allocated + * drm_mm nodes. + * 1 times for prepare_igt_frag() + * 1 times for get_insert_time() + * 2 times for get_insert_time() + */ + nodes = vzalloc(array_size(insert_size * 4, sizeof(*nodes))); + if (!nodes) + return -ENOMEM; + + /* For BOTTOMUP and TOPDOWN, we first fragment the + * address space using prepare_igt_frag() and then try to verify + * that that insertions scale quadratically from 10k to 20k insertions + */ + drm_mm_init(, 1, U64_MAX - 2); + for (mode = insert_modes; mode->name; mode++) { + u64 insert_time1, insert_time2; + + if (mode->mode != DRM_MM_INSERT_LOW || + mode->mode != DRM_MM_INSERT_HIGH) + continue; This check here is wrong, that needs to be && instead of || or the test wouldn't execute at all. I didn't bother to check dmesg after adding that "simple" check and the test ran fine. :/ Yeah, after that the test seems to work. But there are is another issues. We only cut of the right or the left branch of the tree and that still makes the implementation rather inefficient. In other words we first go down leftmost or rightmost even if we know that this way is no valuable candidate and then back off again towards the top. Going to look into this, but your patches already improves insertion time by a factor of nearly 30 in a fragmented address space. That is rather nice. Regards, Christian. Sending again. Nirmoy Christian. + + ret = prepare_igt_frag(, nodes, insert_size, mode); + if (!ret) + goto err; + + insert_time1 = get_insert_time(, insert_size, + nodes + insert_size, mode); + if (insert_time1 < 0) + goto err; + + insert_time2 =
Re: [PATCH v3 43/43] drm: Remove struct drm_driver.gem_print_info
Hi Thomas, Thank you for the patch. On Fri, Jun 05, 2020 at 09:32:47AM +0200, Thomas Zimmermann wrote: > The .gem_print_info callback in struct drm_driver is obsolete and has > no users left. Remove it. I like code removal :-) Looking forward to the removal of more GEM-related fields from struct drm_driver. > Signed-off-by: Thomas Zimmermann > Suggested-by: Emil Velikov Reviewed-by: Laurent Pinchart > --- > drivers/gpu/drm/drm_gem.c | 2 -- > include/drm/drm_drv.h | 17 - > 2 files changed, 19 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index efc0367841e2b..08b3fa27ec406 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -1191,8 +1191,6 @@ void drm_gem_print_info(struct drm_printer *p, unsigned > int indent, > > if (obj->funcs && obj->funcs->print_info) > obj->funcs->print_info(p, indent, obj); > - else if (obj->dev->driver->gem_print_info) > - obj->dev->driver->gem_print_info(p, indent, obj); > } > > int drm_gem_pin(struct drm_gem_object *obj) > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h > index bb924cddc09c1..8f110a28b6a23 100644 > --- a/include/drm/drm_drv.h > +++ b/include/drm/drm_drv.h > @@ -353,23 +353,6 @@ struct drm_driver { >*/ > void (*gem_close_object) (struct drm_gem_object *, struct drm_file *); > > - /** > - * @gem_print_info: > - * > - * This callback is deprecated in favour of > - * _gem_object_funcs.print_info. > - * > - * If driver subclasses struct _gem_object, it can implement this > - * optional hook for printing additional driver specific info. > - * > - * drm_printf_indent() should be used in the callback passing it the > - * indent argument. > - * > - * This callback is called from drm_gem_print_info(). > - */ > - void (*gem_print_info)(struct drm_printer *p, unsigned int indent, > -const struct drm_gem_object *obj); > - > /** >* @gem_create_object: constructor for gem objects >* -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 13/18] drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
Hi Daniel, On 04/06/2020 10:12, Daniel Vetter wrote: [...] > @@ -6910,7 +6910,11 @@ static void amdgpu_dm_commit_planes(struct > drm_atomic_state *state, >* explicitly on fences instead >* and in general should be called for >* blocking commit to as per framework helpers > + * > + * Yes, this deadlocks, since you're calling dma_resv_lock in a > + * path that leads to a dma_fence_signal(). Don't do that. >*/ > +#if 0 > r = amdgpu_bo_reserve(abo, true); > if (unlikely(r != 0)) > DRM_ERROR("failed to reserve buffer before flip\n"); > @@ -6920,6 +6924,12 @@ static void amdgpu_dm_commit_planes(struct > drm_atomic_state *state, > tmz_surface = amdgpu_bo_encrypted(abo); > > amdgpu_bo_unreserve(abo); > +#endif > + /* > + * this races anyway, so READ_ONCE isn't any better or worse > + * than the stuff above. Except the stuff above can deadlock. > + */ > + tiling_flags = READ_ONCE(abo->tiling_flags); With this change "tmz_surface" won't be initialized properly. Adding the following line should fix it: tmz_surface = READ_ONCE(abo->flags) & AMDGPU_GEM_CREATE_ENCRYPTED; Pierre-Eric > > fill_dc_plane_info_and_addr( > dm->adev, new_plane_state, tiling_flags, > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 29/43] drm/rcar-du: Use GEM CMA object functions
Hi Thomas, Thank you for the patch. On Fri, Jun 05, 2020 at 09:32:33AM +0200, Thomas Zimmermann wrote: > Create GEM objects with drm_gem_cma_create_object_default_funcs(), which > allocates the object and sets CMA's default object functions. Corresponding > callbacks in struct drm_driver are cleared. No functional changes are made. > > Driver and object-function instances use the same callback functions, with > the exception of vunmap. The implementation of vunmap is empty and left out > in CMA's default object functions. > > v3: > * convert to DRIVER_OPS macro in a separate patch > > Signed-off-by: Thomas Zimmermann > Acked-by: Emil Velikov Reviewed-by: Laurent Pinchart > --- > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 6 +- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > index 3e67cf70f0402..43610d5bf8820 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > @@ -476,14 +476,10 @@ DEFINE_DRM_GEM_CMA_FOPS(rcar_du_fops); > > static struct drm_driver rcar_du_driver = { > .driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, > - .gem_free_object_unlocked = drm_gem_cma_free_object, > - .gem_vm_ops = _gem_cma_vm_ops, > + .gem_create_object = drm_gem_cma_create_object_default_funcs, > .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > .prime_fd_to_handle = drm_gem_prime_fd_to_handle, > - .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, > .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, > - .gem_prime_vmap = drm_gem_cma_prime_vmap, > - .gem_prime_vunmap = drm_gem_cma_prime_vunmap, > .gem_prime_mmap = drm_gem_cma_prime_mmap, > .dumb_create= rcar_du_dumb_create, > .fops = _du_fops, -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 31/43] drm/shmobile: Use GEM CMA object functions
Hi Thomas, Thank you for the patch. On Fri, Jun 05, 2020 at 09:32:35AM +0200, Thomas Zimmermann wrote: > Create GEM objects with drm_gem_cma_create_object_default_funcs(), which > allocates the object and sets CMA's default object functions. Corresponding > callbacks in struct drm_driver are cleared. No functional changes are made. > > Driver and object-function instances use the same callback functions, with > the exception of vunmap. The implementation of vunmap is empty and left out > in CMA's default object functions. > > v3: > * convert to DRIVER_OPS macro in a separate patch > > Signed-off-by: Thomas Zimmermann > Acked-by: Emil Velikov Reviewed-by: Laurent Pinchart > --- > drivers/gpu/drm/shmobile/shmob_drm_drv.c | 6 +- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c > b/drivers/gpu/drm/shmobile/shmob_drm_drv.c > index ae9d6b8d3ca87..e209610d4b8a1 100644 > --- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c > +++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c > @@ -131,14 +131,10 @@ DEFINE_DRM_GEM_CMA_FOPS(shmob_drm_fops); > static struct drm_driver shmob_drm_driver = { > .driver_features= DRIVER_GEM | DRIVER_MODESET, > .irq_handler= shmob_drm_irq, > - .gem_free_object_unlocked = drm_gem_cma_free_object, > - .gem_vm_ops = _gem_cma_vm_ops, > + .gem_create_object = drm_gem_cma_create_object_default_funcs, > .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > .prime_fd_to_handle = drm_gem_prime_fd_to_handle, > - .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, > .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, > - .gem_prime_vmap = drm_gem_cma_prime_vmap, > - .gem_prime_vunmap = drm_gem_cma_prime_vunmap, > .gem_prime_mmap = drm_gem_cma_prime_mmap, > .dumb_create= drm_gem_cma_dumb_create, > .fops = _drm_fops, > -- > 2.26.2 > -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/1] drm/mm: add ig_frag selftest
On 6/5/20 9:45 AM, Christian König wrote: Am 03.06.20 um 12:32 schrieb Nirmoy Das: This patch introduces fragmentation in the address range and measures time taken by 10k and 20k insertions. ig_frag() will fail if the time taken by 20k insertions takes more than 4 times of 10k insertions as we know that insertions should at most scale quadratically. v2: introduce fragmentation by freeing every other node. only test bottom-up and top-down for now. Signed-off-by: Nirmoy Das --- drivers/gpu/drm/selftests/drm_mm_selftests.h | 1 + drivers/gpu/drm/selftests/test-drm_mm.c | 124 +++ 2 files changed, 125 insertions(+) diff --git a/drivers/gpu/drm/selftests/drm_mm_selftests.h b/drivers/gpu/drm/selftests/drm_mm_selftests.h index 6b943ea1c57d..8c87c964176b 100644 --- a/drivers/gpu/drm/selftests/drm_mm_selftests.h +++ b/drivers/gpu/drm/selftests/drm_mm_selftests.h @@ -14,6 +14,7 @@ selftest(insert, igt_insert) selftest(replace, igt_replace) selftest(insert_range, igt_insert_range) selftest(align, igt_align) +selftest(frag, igt_frag) selftest(align32, igt_align32) selftest(align64, igt_align64) selftest(evict, igt_evict) diff --git a/drivers/gpu/drm/selftests/test-drm_mm.c b/drivers/gpu/drm/selftests/test-drm_mm.c index 9aabe82dcd3a..34231baacd87 100644 --- a/drivers/gpu/drm/selftests/test-drm_mm.c +++ b/drivers/gpu/drm/selftests/test-drm_mm.c @@ -10,6 +10,7 @@ #include #include #include +#include #include @@ -1033,6 +1034,129 @@ static int igt_insert_range(void *ignored) return 0; } +static int prepare_igt_frag(struct drm_mm *mm, + struct drm_mm_node *nodes, + unsigned int num_insert, + const struct insert_mode *mode) +{ + unsigned int size = 4096; + unsigned int i; + u64 ret = -EINVAL; + + for (i = 0; i < num_insert; i++) { + if (!expect_insert(mm, [i], size, 0, i, + mode) != 0) { + pr_err("%s insert failed\n", mode->name); + goto out; + } + } + + /* introduce fragmentation by freeing every other node */ + for (i = 0; i < num_insert; i++) { + if (i % 2 == 0) + drm_mm_remove_node([i]); + } + +out: + return ret; + +} + +static u64 get_insert_time(struct drm_mm *mm, + unsigned int num_insert, + struct drm_mm_node *nodes, + const struct insert_mode *mode) +{ + unsigned int size = 8192; + ktime_t start; + unsigned int i; + u64 ret = -EINVAL; + + start = ktime_get(); + for (i = 0; i < num_insert; i++) { + if (!expect_insert(mm, [i], size, 0, i, mode) != 0) { + pr_err("%s insert failed\n", mode->name); + goto out; + } + } + + ret = ktime_to_ns(ktime_sub(ktime_get(), start)); + +out: + return ret; + +} + +static int igt_frag(void *ignored) +{ + struct drm_mm mm; + const struct insert_mode *mode; + struct drm_mm_node *nodes, *node, *next; + unsigned int insert_size = 1; + unsigned int scale_factor = 4; + int ret = -EINVAL; + + /* We need 4 * insert_size nodes to hold intermediate allocated + * drm_mm nodes. + * 1 times for prepare_igt_frag() + * 1 times for get_insert_time() + * 2 times for get_insert_time() + */ + nodes = vzalloc(array_size(insert_size * 4, sizeof(*nodes))); + if (!nodes) + return -ENOMEM; + + /* For BOTTOMUP and TOPDOWN, we first fragment the + * address space using prepare_igt_frag() and then try to verify + * that that insertions scale quadratically from 10k to 20k insertions + */ + drm_mm_init(, 1, U64_MAX - 2); + for (mode = insert_modes; mode->name; mode++) { + u64 insert_time1, insert_time2; + + if (mode->mode != DRM_MM_INSERT_LOW || + mode->mode != DRM_MM_INSERT_HIGH) + continue; This check here is wrong, that needs to be && instead of || or the test wouldn't execute at all. I didn't bother to check dmesg after adding that "simple" check and the test ran fine. :/ Sending again. Nirmoy Christian. + + ret = prepare_igt_frag(, nodes, insert_size, mode); + if (!ret) + goto err; + + insert_time1 = get_insert_time(, insert_size, + nodes + insert_size, mode); + if (insert_time1 < 0) + goto err; + + insert_time2 = get_insert_time(, (insert_size * 2), + nodes + insert_size * 2, mode); + if (insert_time2 < 0) + goto err; + + pr_info("%s fragmented insert of %u and %u insertions took %llu and %llu nsecs\n", + mode->name, insert_size, insert_size * 2, + insert_time1, insert_time2); + + if (insert_time2 > (scale_factor * insert_time1)) { + pr_err("%s fragmented insert took %llu nsecs more\n", + mode->name, + insert_time2 - (scale_factor *
Re: [PATCH v3 03/43] drm/cma-helper: Add DRM_GEM_CMA_DRIVER_OPS to set default GEM CMA functions
Hi Thomas, Thank you for the patch. On Fri, Jun 05, 2020 at 09:32:07AM +0200, Thomas Zimmermann wrote: > The macro to DRM_GEM_CMA_DRIVER_OPS sets GEM callbacks in struct drm_driver > to their defaults for CMA. An variant of the macro is provided for drivers s/An variant/A variant/ > that override the default .dumb_create callback. Adapt drivers to the changes. > > Signed-off-by: Thomas Zimmermann > Reviewed-by: Sam Ravnborg > Reviewed-by: Laurent Pinchart > Acked-by: Emil Velikov > --- > include/drm/drm_gem_cma_helper.h | 46 +--- > 1 file changed, 43 insertions(+), 3 deletions(-) > > diff --git a/include/drm/drm_gem_cma_helper.h > b/include/drm/drm_gem_cma_helper.h > index 2cdf333ae585d..5e1e595c0e72d 100644 > --- a/include/drm/drm_gem_cma_helper.h > +++ b/include/drm/drm_gem_cma_helper.h > @@ -109,6 +109,42 @@ void drm_gem_cma_prime_vunmap(struct drm_gem_object > *obj, void *vaddr); > struct drm_gem_object * > drm_gem_cma_create_object_default_funcs(struct drm_device *dev, size_t size); > > +/** > + * DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE - CMA GEM driver operations > + * @dumb_create_func: callback function for .dumb_create > + * > + * This macro provides a shortcut for setting the default GEM operations in > the > + * _driver structure. > + * > + * This macro is a variant of DRM_GEM_CMA_DRIVER_OPS for drivers that > + * override the default implementation of rm_driver.dumb_create. Use > + * DRM_GEM_CMA_DRIVER_OPS if possible. Drivers that require a virtual address > + * on imported buffers should use > + * DRM_GEM_CMA_DRIVER_OPS_VMAP_WITH_DUMB_CREATE() instead. > + */ > +#define DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE(dumb_create_func) \ > + .gem_create_object = drm_gem_cma_create_object_default_funcs, \ > + .dumb_create= (dumb_create_func), \ Do we need parentheses here ? > + .prime_handle_to_fd = drm_gem_prime_handle_to_fd, \ > + .prime_fd_to_handle = drm_gem_prime_fd_to_handle, \ > + .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, \ > + .gem_prime_mmap = drm_gem_cma_prime_mmap > + > +/** > + * DRM_GEM_CMA_DRIVER_OPS - CMA GEM driver operations > + * > + * This macro provides a shortcut for setting the default GEM operations in > the > + * _driver structure. > + * > + * Drivers that come with their own implementation of > + * drm_driver.dumb_create should use > + * DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE() instead. Use > + * DRM_GEM_CMA_DRIVER_OPS if possible. Drivers that require a virtual address > + * on imported buffers should use DRM_GEM_CMA_DRIVER_OPS_VMAP instead. > + */ > +#define DRM_GEM_CMA_DRIVER_OPS \ > + DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE(drm_gem_cma_dumb_create) > + > /** > * DRM_GEM_CMA_DRIVER_OPS_VMAP_WITH_DUMB_CREATE - CMA GEM driver operations > *ensuring a virtual address > @@ -120,8 +156,10 @@ drm_gem_cma_create_object_default_funcs(struct > drm_device *dev, size_t size); > * imported buffers. > * > * This macro is a variant of DRM_GEM_CMA_DRIVER_OPS_VMAP for drivers that > - * override the default implementation of rm_driver.dumb_create. Use > - * DRM_GEM_CMA_DRIVER_OPS_VMAP if possible. > + * override the default implementation of drm_driver.dumb_create. Use > + * DRM_GEM_CMA_DRIVER_OPS_VMAP if possible. Drivers that do not require a > + * virtual address on imported buffers should use > + * DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE() instead. > */ > #define DRM_GEM_CMA_DRIVER_OPS_VMAP_WITH_DUMB_CREATE(dumb_create_func) \ > .gem_create_object = drm_gem_cma_create_object_default_funcs, \ > @@ -142,7 +180,9 @@ drm_gem_cma_create_object_default_funcs(struct drm_device > *dev, size_t size); > * Drivers that come with their own implementation of > * drm_driver.dumb_create should use > * DRM_GEM_CMA_DRIVER_OPS_VMAP_WITH_DUMB_CREATE() instead. Use > - * DRM_GEM_CMA_DRIVER_OPS_VMAP if possible. > + * DRM_GEM_CMA_DRIVER_OPS_VMAP if possible. Drivers that do not require a > + * virtual address on imported buffers should use DRM_GEM_CMA_DRIVER_OPS > + * instead. > */ > #define DRM_GEM_CMA_DRIVER_OPS_VMAP \ > DRM_GEM_CMA_DRIVER_OPS_VMAP_WITH_DUMB_CREATE(drm_gem_cma_dumb_create) > -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 30/43] drm/rcar-du: Set GEM CMA functions with DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE
Hi Thomas, Thank you for the patch. On Fri, Jun 05, 2020 at 09:32:34AM +0200, Thomas Zimmermann wrote: > DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE sets the functions in > struct drm_driver to their defaults. No functional changes are > made. > > v2: > * update for DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE > > Signed-off-by: Thomas Zimmermann > Tested-by: Kieran Bingham > Acked-by: Emil Velikov Reviewed-by: Laurent Pinchart > --- > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 7 +-- > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > index 43610d5bf8820..f53b0ec710850 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > @@ -476,12 +476,7 @@ DEFINE_DRM_GEM_CMA_FOPS(rcar_du_fops); > > static struct drm_driver rcar_du_driver = { > .driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, > - .gem_create_object = drm_gem_cma_create_object_default_funcs, > - .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > - .prime_fd_to_handle = drm_gem_prime_fd_to_handle, > - .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, > - .gem_prime_mmap = drm_gem_cma_prime_mmap, > - .dumb_create= rcar_du_dumb_create, > + DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE(rcar_du_dumb_create), > .fops = _du_fops, > .name = "rcar-du", > .desc = "Renesas R-Car Display Unit", -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 32/43] drm/shmobile: Set GEM CMA functions with DRM_GEM_CMA_DRIVER_OPS
Hi Thomas, Thank you for the patch. On Fri, Jun 05, 2020 at 09:32:36AM +0200, Thomas Zimmermann wrote: > DRM_GEM_CMA_DRIVER_OPS sets the functions in struct drm_driver > to their defaults. No functional changes are made. > > Signed-off-by: Thomas Zimmermann > Acked-by: Emil Velikov Reviewed-by: Laurent Pinchart > --- > drivers/gpu/drm/shmobile/shmob_drm_drv.c | 7 +-- > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c > b/drivers/gpu/drm/shmobile/shmob_drm_drv.c > index e209610d4b8a1..26a15c214bd3f 100644 > --- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c > +++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c > @@ -131,12 +131,7 @@ DEFINE_DRM_GEM_CMA_FOPS(shmob_drm_fops); > static struct drm_driver shmob_drm_driver = { > .driver_features= DRIVER_GEM | DRIVER_MODESET, > .irq_handler= shmob_drm_irq, > - .gem_create_object = drm_gem_cma_create_object_default_funcs, > - .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > - .prime_fd_to_handle = drm_gem_prime_fd_to_handle, > - .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, > - .gem_prime_mmap = drm_gem_cma_prime_mmap, > - .dumb_create= drm_gem_cma_dumb_create, > + DRM_GEM_CMA_DRIVER_OPS, > .fops = _drm_fops, > .name = "shmob-drm", > .desc = "Renesas SH Mobile DRM", -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Dynamically change enumeration list of DRM enumeration property
On Wed, 03 Jun 2020 20:20:44 + (UTC) Jonas Karlman wrote: > Hi, > > On 2020-06-03 11:12, Pekka Paalanen wrote: > > On Wed, 3 Jun 2020 10:50:28 +0530 > > Yogish Kulkarni wrote: > > ... > >> The primary reason for why content producer masters video/gfx content as > >> limited range is for compatibility with sinks which only support limited > >> range, and not because video decoders are not capable of decoding > >> full-range content. > > > > What I was asking is, even if the video content is limited range, why > > would one not decode it into full-range pixels always and if the sink > > need limited range, then convert again in hardware? When done right, it > > makes no difference in output compared to using limited range > > through-out if both content and sink use limited range. > > For the Allwinner/Amlogic/Rockchip arm devices I mainly play with the > video decoder does not support range conversion (to my knowledge) and will > produce NV12/YU12 framebuffers in the range the video was encoded in. > > These devices typically lack a high-performance GPU/3D-accelerator > and may have limited CSC capabilities in the display controller. > The HDMI block can usually do simple RGB/YUV and full/limited conversions, > but using these conversions typically produce banding effects. > > Being able to passthrough decoded framebuffers in the entire pipeline from > decoder, display controller and hdmi block typically produce best results. This is very helpful. It means I really do need to take range into account in Wayland protocol and make sure it can be communicated. Thanks, pq pgpepNdQ3HDtN.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/1] drm/mm: add ig_frag selftest
Am 03.06.20 um 12:32 schrieb Nirmoy Das: This patch introduces fragmentation in the address range and measures time taken by 10k and 20k insertions. ig_frag() will fail if the time taken by 20k insertions takes more than 4 times of 10k insertions as we know that insertions should at most scale quadratically. v2: introduce fragmentation by freeing every other node. only test bottom-up and top-down for now. Signed-off-by: Nirmoy Das --- drivers/gpu/drm/selftests/drm_mm_selftests.h | 1 + drivers/gpu/drm/selftests/test-drm_mm.c | 124 +++ 2 files changed, 125 insertions(+) diff --git a/drivers/gpu/drm/selftests/drm_mm_selftests.h b/drivers/gpu/drm/selftests/drm_mm_selftests.h index 6b943ea1c57d..8c87c964176b 100644 --- a/drivers/gpu/drm/selftests/drm_mm_selftests.h +++ b/drivers/gpu/drm/selftests/drm_mm_selftests.h @@ -14,6 +14,7 @@ selftest(insert, igt_insert) selftest(replace, igt_replace) selftest(insert_range, igt_insert_range) selftest(align, igt_align) +selftest(frag, igt_frag) selftest(align32, igt_align32) selftest(align64, igt_align64) selftest(evict, igt_evict) diff --git a/drivers/gpu/drm/selftests/test-drm_mm.c b/drivers/gpu/drm/selftests/test-drm_mm.c index 9aabe82dcd3a..34231baacd87 100644 --- a/drivers/gpu/drm/selftests/test-drm_mm.c +++ b/drivers/gpu/drm/selftests/test-drm_mm.c @@ -10,6 +10,7 @@ #include #include #include +#include #include @@ -1033,6 +1034,129 @@ static int igt_insert_range(void *ignored) return 0; } +static int prepare_igt_frag(struct drm_mm *mm, + struct drm_mm_node *nodes, + unsigned int num_insert, + const struct insert_mode *mode) +{ + unsigned int size = 4096; + unsigned int i; + u64 ret = -EINVAL; + + for (i = 0; i < num_insert; i++) { + if (!expect_insert(mm, [i], size, 0, i, + mode) != 0) { + pr_err("%s insert failed\n", mode->name); + goto out; + } + } + + /* introduce fragmentation by freeing every other node */ + for (i = 0; i < num_insert; i++) { + if (i % 2 == 0) + drm_mm_remove_node([i]); + } + +out: + return ret; + +} + +static u64 get_insert_time(struct drm_mm *mm, + unsigned int num_insert, + struct drm_mm_node *nodes, + const struct insert_mode *mode) +{ + unsigned int size = 8192; + ktime_t start; + unsigned int i; + u64 ret = -EINVAL; + + start = ktime_get(); + for (i = 0; i < num_insert; i++) { + if (!expect_insert(mm, [i], size, 0, i, mode) != 0) { + pr_err("%s insert failed\n", mode->name); + goto out; + } + } + + ret = ktime_to_ns(ktime_sub(ktime_get(), start)); + +out: + return ret; + +} + +static int igt_frag(void *ignored) +{ + struct drm_mm mm; + const struct insert_mode *mode; + struct drm_mm_node *nodes, *node, *next; + unsigned int insert_size = 1; + unsigned int scale_factor = 4; + int ret = -EINVAL; + + /* We need 4 * insert_size nodes to hold intermediate allocated +* drm_mm nodes. +* 1 times for prepare_igt_frag() +* 1 times for get_insert_time() +* 2 times for get_insert_time() +*/ + nodes = vzalloc(array_size(insert_size * 4, sizeof(*nodes))); + if (!nodes) + return -ENOMEM; + + /* For BOTTOMUP and TOPDOWN, we first fragment the +* address space using prepare_igt_frag() and then try to verify +* that that insertions scale quadratically from 10k to 20k insertions +*/ + drm_mm_init(, 1, U64_MAX - 2); + for (mode = insert_modes; mode->name; mode++) { + u64 insert_time1, insert_time2; + + if (mode->mode != DRM_MM_INSERT_LOW || + mode->mode != DRM_MM_INSERT_HIGH) + continue; This check here is wrong, that needs to be && instead of || or the test wouldn't execute at all. Christian. + + ret = prepare_igt_frag(, nodes, insert_size, mode); + if (!ret) + goto err; + + insert_time1 = get_insert_time(, insert_size, + nodes + insert_size, mode); + if (insert_time1 < 0) + goto err; + + insert_time2 = get_insert_time(, (insert_size * 2), + nodes + insert_size * 2, mode); + if (insert_time2 < 0) + goto err; + + pr_info("%s fragmented insert of %u and %u insertions took %llu and %llu nsecs\n", + mode->name,
[PATCH v3 30/43] drm/rcar-du: Set GEM CMA functions with DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE
DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE sets the functions in struct drm_driver to their defaults. No functional changes are made. v2: * update for DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE Signed-off-by: Thomas Zimmermann Tested-by: Kieran Bingham Acked-by: Emil Velikov --- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 43610d5bf8820..f53b0ec710850 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c @@ -476,12 +476,7 @@ DEFINE_DRM_GEM_CMA_FOPS(rcar_du_fops); static struct drm_driver rcar_du_driver = { .driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, - .gem_create_object = drm_gem_cma_create_object_default_funcs, - .prime_handle_to_fd = drm_gem_prime_handle_to_fd, - .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, - .gem_prime_mmap = drm_gem_cma_prime_mmap, - .dumb_create= rcar_du_dumb_create, + DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE(rcar_du_dumb_create), .fops = _du_fops, .name = "rcar-du", .desc = "Renesas R-Car Display Unit", -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 27/43] drm/mxsfb: Use GEM CMA object functions
Create GEM objects with drm_gem_cma_create_object_default_funcs(), which allocates the object and sets CMA's default object functions. Corresponding callbacks in struct drm_driver are cleared. No functional changes are made. Driver and object-function instances use the same callback functions, with the exception of vunmap. The implementation of vunmap is empty and left out in CMA's default object functions. v3: * convert to DRIVER_OPS macro in a separate patch Signed-off-by: Thomas Zimmermann Acked-by: Emil Velikov --- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c index 497cf443a9afa..afdf1e0accba6 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c @@ -356,15 +356,11 @@ static struct drm_driver mxsfb_driver = { .irq_handler= mxsfb_irq_handler, .irq_preinstall = mxsfb_irq_preinstall, .irq_uninstall = mxsfb_irq_preinstall, - .gem_free_object_unlocked = drm_gem_cma_free_object, - .gem_vm_ops = _gem_cma_vm_ops, + .gem_create_object = drm_gem_cma_create_object_default_funcs, .dumb_create= drm_gem_cma_dumb_create, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, - .gem_prime_vmap = drm_gem_cma_prime_vmap, - .gem_prime_vunmap = drm_gem_cma_prime_vunmap, .gem_prime_mmap = drm_gem_cma_prime_mmap, .fops = , .name = "mxsfb-drm", -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 21/43] drm/malidp: Use GEM CMA object functions
Create GEM objects with drm_gem_cma_create_object_default_funcs(), which allocates the object and sets CMA's default object functions. Corresponding callbacks in struct drm_driver are cleared. No functional changes are made. Driver and object-function instances use the same callback functions, with the exception of vunmap. The implementation of vunmap is empty and left out in CMA's default object functions. v3: * convert to DRIVER_OPS macro in a separate patch Signed-off-by: Thomas Zimmermann Acked-by: Liviu Dudau Acked-by: Emil Velikov --- drivers/gpu/drm/arm/malidp_drv.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c index c2507b7d8512b..a56a93fc7bc2e 100644 --- a/drivers/gpu/drm/arm/malidp_drv.c +++ b/drivers/gpu/drm/arm/malidp_drv.c @@ -563,15 +563,11 @@ static void malidp_debugfs_init(struct drm_minor *minor) static struct drm_driver malidp_driver = { .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, - .gem_free_object_unlocked = drm_gem_cma_free_object, - .gem_vm_ops = _gem_cma_vm_ops, + .gem_create_object = drm_gem_cma_create_object_default_funcs, .dumb_create = malidp_dumb_create, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, - .gem_prime_vmap = drm_gem_cma_prime_vmap, - .gem_prime_vunmap = drm_gem_cma_prime_vunmap, .gem_prime_mmap = drm_gem_cma_prime_mmap, #ifdef CONFIG_DEBUG_FS .debugfs_init = malidp_debugfs_init, -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 20/43] drm/komeda: Set GEM CMA functions with DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE
DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE sets the functions in struct drm_driver to their defaults. No functional changes are made. v2: * update for DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE Signed-off-by: Thomas Zimmermann Acked-by: Liviu Dudau Acked-by: Emil Velikov --- drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c index af24dca1cab63..1f6682032ca49 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c @@ -61,12 +61,7 @@ static irqreturn_t komeda_kms_irq_handler(int irq, void *data) static struct drm_driver komeda_kms_driver = { .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, .lastclose = drm_fb_helper_lastclose, - .gem_create_object = drm_gem_cma_create_object_default_funcs, - .dumb_create= komeda_gem_cma_dumb_create, - .prime_handle_to_fd = drm_gem_prime_handle_to_fd, - .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, - .gem_prime_mmap = drm_gem_cma_prime_mmap, + DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE(komeda_gem_cma_dumb_create), .fops = _cma_fops, .name = "komeda", .desc = "Arm Komeda Display Processor driver", -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 15/43] drm/imx: Use GEM CMA object functions
Create GEM objects with drm_gem_cma_create_object_default_funcs(), which allocates the object and sets CMA's default object functions. Corresponding callbacks in struct drm_driver are cleared. No functional changes are made. Driver and object-function instances use the same callback functions, with the exception of vunmap. The implementation of vunmap is empty and left out in CMA's default object functions. v3: * convert to DRIVER_OPS macro in a separate patch Signed-off-by: Thomas Zimmermann Acked-by: Emil Velikov --- drivers/gpu/drm/imx/imx-drm-core.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c index 2e38f1a5cf8da..f13e7cd9b7d16 100644 --- a/drivers/gpu/drm/imx/imx-drm-core.c +++ b/drivers/gpu/drm/imx/imx-drm-core.c @@ -146,16 +146,12 @@ static const struct drm_ioctl_desc imx_drm_ioctls[] = { static struct drm_driver imx_drm_driver = { .driver_features= DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC, - .gem_free_object_unlocked = drm_gem_cma_free_object, - .gem_vm_ops = _gem_cma_vm_ops, + .gem_create_object = drm_gem_cma_create_object_default_funcs, .dumb_create= drm_gem_cma_dumb_create, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, - .gem_prime_vmap = drm_gem_cma_prime_vmap, - .gem_prime_vunmap = drm_gem_cma_prime_vunmap, .gem_prime_mmap = drm_gem_cma_prime_mmap, .ioctls = imx_drm_ioctls, .num_ioctls = ARRAY_SIZE(imx_drm_ioctls), -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 12/43] drm/hisilicon/kirin: Set .dumb_create to drm_gem_cma_dumb_create()
The kirin drivers uses drm_gem_cma_dumb_create_internal() for its .dumb_create implementation. The function is meant for internal use only by drivers that require additional buffer setup. Kirin does not do an additional setup, so convert it over to drm_gem_cma_dumb_create(). Signed-off-by: Thomas Zimmermann Tested-by: John Stultz Acked-by: Emil Velikov Cc: Xu YiPing Cc: Rongrong Zou Cc: Xinliang Liu --- drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c index c339e632522a9..18e57e571e054 100644 --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c @@ -923,7 +923,7 @@ static struct drm_driver ade_driver = { .fops = _fops, .gem_free_object_unlocked = drm_gem_cma_free_object, .gem_vm_ops = _gem_cma_vm_ops, - .dumb_create = drm_gem_cma_dumb_create_internal, + .dumb_create = drm_gem_cma_dumb_create, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 17/43] drm/ingenic: Use GEM CMA object functions
Create GEM objects with drm_gem_cma_create_object_default_funcs(), which allocates the object and sets CMA's default object functions. Corresponding callbacks in struct drm_driver are cleared. No functional changes are made. Driver and object-function instances use the same callback functions, with the exception of vunmap. The implementation of vunmap is empty and left out in CMA's default object functions. v3: * convert to DRIVER_OPS macro in a separate patch Signed-off-by: Thomas Zimmermann Reviewed-by: Paul Cercueil Acked-by: Emil Velikov --- drivers/gpu/drm/ingenic/ingenic-drm.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c b/drivers/gpu/drm/ingenic/ingenic-drm.c index 55b49a31729bf..7f02debaf5a0d 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c @@ -520,16 +520,12 @@ static struct drm_driver ingenic_drm_driver_data = { .fops = _drm_fops, + .gem_create_object = drm_gem_cma_create_object_default_funcs, .dumb_create= drm_gem_cma_dumb_create, - .gem_free_object_unlocked = drm_gem_cma_free_object, - .gem_vm_ops = _gem_cma_vm_ops, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, - .gem_prime_vmap = drm_gem_cma_prime_vmap, - .gem_prime_vunmap = drm_gem_cma_prime_vunmap, .gem_prime_mmap = drm_gem_cma_prime_mmap, .irq_handler= ingenic_drm_irq_handler, -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 14/43] drm/hisilicon/kirin: Set GEM CMA functions with DRM_GEM_CMA_DRIVER_OPS
DRM_GEM_CMA_DRIVER_OPS sets the functions in struct drm_driver to their defaults. No functional changes are made. v2: * use DRM_GEM_CMA_DRIVER_OPS Signed-off-by: Thomas Zimmermann Tested-by: John Stultz Acked-by: Emil Velikov Cc: Xu YiPing Cc: Rongrong Zou Cc: Xinliang Liu --- drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c index a550e464153b6..e1108c1735ad0 100644 --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c @@ -921,13 +921,7 @@ DEFINE_DRM_GEM_CMA_FOPS(ade_fops); static struct drm_driver ade_driver = { .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, .fops = _fops, - .gem_create_object = drm_gem_cma_create_object_default_funcs, - .dumb_create = drm_gem_cma_dumb_create, - .prime_handle_to_fd = drm_gem_prime_handle_to_fd, - .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, - .gem_prime_mmap = drm_gem_cma_prime_mmap, - + DRM_GEM_CMA_DRIVER_OPS, .name = "kirin", .desc = "Hisilicon Kirin620 SoC DRM Driver", .date = "20150718", -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 42/43] drm/zte: Set GEM CMA functions with DRM_GEM_CMA_DRIVER_OPS
DRM_GEM_CMA_DRIVER_OPS sets the functions in struct drm_driver to their defaults. No functional changes are made. Signed-off-by: Thomas Zimmermann Acked-by: Emil Velikov --- drivers/gpu/drm/zte/zx_drm_drv.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/gpu/drm/zte/zx_drm_drv.c b/drivers/gpu/drm/zte/zx_drm_drv.c index 305394923e04c..31014a451f8bd 100644 --- a/drivers/gpu/drm/zte/zx_drm_drv.c +++ b/drivers/gpu/drm/zte/zx_drm_drv.c @@ -36,12 +36,7 @@ DEFINE_DRM_GEM_CMA_FOPS(zx_drm_fops); static struct drm_driver zx_drm_driver = { .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, - .gem_create_object = drm_gem_cma_create_object_default_funcs, - .dumb_create = drm_gem_cma_dumb_create, - .prime_handle_to_fd = drm_gem_prime_handle_to_fd, - .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, - .gem_prime_mmap = drm_gem_cma_prime_mmap, + DRM_GEM_CMA_DRIVER_OPS, .fops = _drm_fops, .name = "zx-vou", .desc = "ZTE VOU Controller DRM", -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 25/43] drm/meson: Use GEM CMA object functions
Create GEM objects with drm_gem_cma_create_object_default_funcs(), which allocates the object and sets CMA's default object functions. Corresponding callbacks in struct drm_driver are cleared. No functional changes are made. Driver and object-function instances use the same callback functions, with the exception of vunmap. The implementation of vunmap is empty and left out in CMA's default object functions. v3: * convert to DRIVER_OPS macro in a separate patch Signed-off-by: Thomas Zimmermann Acked-by: Emil Velikov Acked-by: Neil Armstrong --- drivers/gpu/drm/meson/meson_drv.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c index 4c5aafcec7991..036af6e69bb78 100644 --- a/drivers/gpu/drm/meson/meson_drv.c +++ b/drivers/gpu/drm/meson/meson_drv.c @@ -99,16 +99,12 @@ static struct drm_driver meson_driver = { /* PRIME Ops */ .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, - .gem_prime_vmap = drm_gem_cma_prime_vmap, - .gem_prime_vunmap = drm_gem_cma_prime_vunmap, .gem_prime_mmap = drm_gem_cma_prime_mmap, /* GEM Ops */ + .gem_create_object = drm_gem_cma_create_object_default_funcs, .dumb_create= meson_dumb_create, - .gem_free_object_unlocked = drm_gem_cma_free_object, - .gem_vm_ops = _gem_cma_vm_ops, /* Misc */ .fops = , -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 32/43] drm/shmobile: Set GEM CMA functions with DRM_GEM_CMA_DRIVER_OPS
DRM_GEM_CMA_DRIVER_OPS sets the functions in struct drm_driver to their defaults. No functional changes are made. Signed-off-by: Thomas Zimmermann Acked-by: Emil Velikov --- drivers/gpu/drm/shmobile/shmob_drm_drv.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/shmobile/shmob_drm_drv.c index e209610d4b8a1..26a15c214bd3f 100644 --- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c +++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c @@ -131,12 +131,7 @@ DEFINE_DRM_GEM_CMA_FOPS(shmob_drm_fops); static struct drm_driver shmob_drm_driver = { .driver_features= DRIVER_GEM | DRIVER_MODESET, .irq_handler= shmob_drm_irq, - .gem_create_object = drm_gem_cma_create_object_default_funcs, - .prime_handle_to_fd = drm_gem_prime_handle_to_fd, - .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, - .gem_prime_mmap = drm_gem_cma_prime_mmap, - .dumb_create= drm_gem_cma_dumb_create, + DRM_GEM_CMA_DRIVER_OPS, .fops = _drm_fops, .name = "shmob-drm", .desc = "Renesas SH Mobile DRM", -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 33/43] drm/stm: Use GEM CMA object functions
Create GEM objects with drm_gem_cma_create_object_default_funcs(), which allocates the object and sets CMA's default object functions. Corresponding callbacks in struct drm_driver are cleared. No functional changes are made. Driver and object-function instances use the same callback functions, with the exception of vunmap. The implementation of vunmap is empty and left out in CMA's default object functions. v3: * convert to DRIVER_OPS macro in a separate patch Signed-off-by: Thomas Zimmermann Reviewed-by: Philippe Cornu Acked-by: Emil Velikov --- drivers/gpu/drm/stm/drv.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c index 0f85dd86cafa7..441c5b2af9698 100644 --- a/drivers/gpu/drm/stm/drv.c +++ b/drivers/gpu/drm/stm/drv.c @@ -62,15 +62,11 @@ static struct drm_driver drv_driver = { .minor = 0, .patchlevel = 0, .fops = _driver_fops, + .gem_create_object = drm_gem_cma_create_object_default_funcs, .dumb_create = stm_gem_cma_dumb_create, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_free_object_unlocked = drm_gem_cma_free_object, - .gem_vm_ops = _gem_cma_vm_ops, - .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, - .gem_prime_vmap = drm_gem_cma_prime_vmap, - .gem_prime_vunmap = drm_gem_cma_prime_vunmap, .gem_prime_mmap = drm_gem_cma_prime_mmap, }; -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 09/43] drm/atmel-hlcdc: Set GEM CMA functions with DRM_GEM_CMA_DRIVER_OPS
DRM_GEM_CMA_DRIVER_OPS sets the functions in struct drm_driver to their defaults. No functional changes are made. Signed-off-by: Thomas Zimmermann Reviewed-by: Sam Ravnborg Acked-by: Emil Velikov --- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c index e028c58f56c93..871293d1aeeba 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c @@ -821,12 +821,7 @@ static struct drm_driver atmel_hlcdc_dc_driver = { .irq_preinstall = atmel_hlcdc_dc_irq_uninstall, .irq_postinstall = atmel_hlcdc_dc_irq_postinstall, .irq_uninstall = atmel_hlcdc_dc_irq_uninstall, - .gem_create_object = drm_gem_cma_create_object_default_funcs, - .prime_handle_to_fd = drm_gem_prime_handle_to_fd, - .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, - .gem_prime_mmap = drm_gem_cma_prime_mmap, - .dumb_create = drm_gem_cma_dumb_create, + DRM_GEM_CMA_DRIVER_OPS, .fops = , .name = "atmel-hlcdc", .desc = "Atmel HLCD Controller DRM", -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 26/43] drm/meson: Set GEM CMA functions with DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE
DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE sets the functions in struct drm_driver to their defaults. No functional changes are made. v2: * update for DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE Signed-off-by: Thomas Zimmermann Acked-by: Emil Velikov Acked-by: Neil Armstrong --- drivers/gpu/drm/meson/meson_drv.c | 11 ++- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c index 036af6e69bb78..8b9c8dd788c41 100644 --- a/drivers/gpu/drm/meson/meson_drv.c +++ b/drivers/gpu/drm/meson/meson_drv.c @@ -96,15 +96,8 @@ static struct drm_driver meson_driver = { /* IRQ */ .irq_handler= meson_irq, - /* PRIME Ops */ - .prime_handle_to_fd = drm_gem_prime_handle_to_fd, - .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, - .gem_prime_mmap = drm_gem_cma_prime_mmap, - - /* GEM Ops */ - .gem_create_object = drm_gem_cma_create_object_default_funcs, - .dumb_create= meson_dumb_create, + /* CMA Ops */ + DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE(meson_dumb_create), /* Misc */ .fops = , -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 24/43] drm/mcde: Set GEM CMA functions with DRM_GEM_CMA_DRIVER_OPS
DRM_GEM_CMA_DRIVER_OPS sets the functions in struct drm_driver to their defaults. No functional changes are made. Signed-off-by: Thomas Zimmermann Reviewed-by: Linus Walleij Acked-by: Emil Velikov --- drivers/gpu/drm/mcde/mcde_drv.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/gpu/drm/mcde/mcde_drv.c b/drivers/gpu/drm/mcde/mcde_drv.c index 1a715b9e698ad..d300be5ee463d 100644 --- a/drivers/gpu/drm/mcde/mcde_drv.c +++ b/drivers/gpu/drm/mcde/mcde_drv.c @@ -228,13 +228,7 @@ static struct drm_driver mcde_drm_driver = { .major = 1, .minor = 0, .patchlevel = 0, - .gem_create_object = drm_gem_cma_create_object_default_funcs, - .dumb_create = drm_gem_cma_dumb_create, - - .prime_handle_to_fd = drm_gem_prime_handle_to_fd, - .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, - .gem_prime_mmap = drm_gem_cma_prime_mmap, + DRM_GEM_CMA_DRIVER_OPS, }; static int mcde_drm_bind(struct device *dev) -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 40/43] drm/tve200: Set GEM CMA functions with DRM_GEM_CMA_DRIVER_OPS
DRM_GEM_CMA_DRIVER_OPS sets the functions in struct drm_driver to their defaults. No functional changes are made. Signed-off-by: Thomas Zimmermann Reviewed-by: Linus Walleij Acked-by: Emil Velikov --- drivers/gpu/drm/tve200/tve200_drv.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/gpu/drm/tve200/tve200_drv.c b/drivers/gpu/drm/tve200/tve200_drv.c index f10d5bb1323ca..c3aa39bd38ecd 100644 --- a/drivers/gpu/drm/tve200/tve200_drv.c +++ b/drivers/gpu/drm/tve200/tve200_drv.c @@ -147,13 +147,7 @@ static struct drm_driver tve200_drm_driver = { .major = 1, .minor = 0, .patchlevel = 0, - .gem_create_object = drm_gem_cma_create_object_default_funcs, - .dumb_create = drm_gem_cma_dumb_create, - - .prime_handle_to_fd = drm_gem_prime_handle_to_fd, - .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, - .gem_prime_mmap = drm_gem_cma_prime_mmap, + DRM_GEM_CMA_DRIVER_OPS, }; static int tve200_probe(struct platform_device *pdev) -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 23/43] drm/mcde: Use GEM CMA object functions
Create GEM objects with drm_gem_cma_create_object_default_funcs(), which allocates the object and sets CMA's default object functions. Corresponding callbacks in struct drm_driver are cleared. No functional changes are made. Driver and object-function instances use the same callback functions, with the exception of vunmap. The implementation of vunmap is empty and left out in CMA's default object functions. v3: * convert to DRIVER_OPS macro in a separate patch Signed-off-by: Thomas Zimmermann Reviewed-by: Linus Walleij Acked-by: Emil Velikov --- drivers/gpu/drm/mcde/mcde_drv.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/mcde/mcde_drv.c b/drivers/gpu/drm/mcde/mcde_drv.c index 84f3e2dbd77bd..1a715b9e698ad 100644 --- a/drivers/gpu/drm/mcde/mcde_drv.c +++ b/drivers/gpu/drm/mcde/mcde_drv.c @@ -228,16 +228,12 @@ static struct drm_driver mcde_drm_driver = { .major = 1, .minor = 0, .patchlevel = 0, + .gem_create_object = drm_gem_cma_create_object_default_funcs, .dumb_create = drm_gem_cma_dumb_create, - .gem_free_object_unlocked = drm_gem_cma_free_object, - .gem_vm_ops = _gem_cma_vm_ops, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, - .gem_prime_vmap = drm_gem_cma_prime_vmap, - .gem_prime_vunmap = drm_gem_cma_prime_vunmap, .gem_prime_mmap = drm_gem_cma_prime_mmap, }; -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 36/43] drm/sti: Set GEM CMA functions with DRM_GEM_CMA_DRIVER_OPS
DRM_GEM_CMA_DRIVER_OPS sets the functions in struct drm_driver to their defaults. No functional changes are made. Signed-off-by: Thomas Zimmermann Acked-by: Emil Velikov --- drivers/gpu/drm/sti/sti_drv.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c index 1e9dba309f12e..3f54efa360981 100644 --- a/drivers/gpu/drm/sti/sti_drv.c +++ b/drivers/gpu/drm/sti/sti_drv.c @@ -132,14 +132,8 @@ DEFINE_DRM_GEM_CMA_FOPS(sti_driver_fops); static struct drm_driver sti_driver = { .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC, - .gem_create_object = drm_gem_cma_create_object_default_funcs, - .dumb_create = drm_gem_cma_dumb_create, .fops = _driver_fops, - - .prime_handle_to_fd = drm_gem_prime_handle_to_fd, - .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, - .gem_prime_mmap = drm_gem_cma_prime_mmap, + DRM_GEM_CMA_DRIVER_OPS, .debugfs_init = sti_drm_dbg_init, -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 28/43] drm/mxsfb: Set GEM CMA functions with DRM_GEM_CMA_DRIVER_OPS
DRM_GEM_CMA_DRIVER_OPS sets the functions in struct drm_driver to their defaults. No functional changes are made. Signed-off-by: Thomas Zimmermann Acked-by: Emil Velikov --- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c index afdf1e0accba6..47c7dce03da4a 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c @@ -356,12 +356,7 @@ static struct drm_driver mxsfb_driver = { .irq_handler= mxsfb_irq_handler, .irq_preinstall = mxsfb_irq_preinstall, .irq_uninstall = mxsfb_irq_preinstall, - .gem_create_object = drm_gem_cma_create_object_default_funcs, - .dumb_create= drm_gem_cma_dumb_create, - .prime_handle_to_fd = drm_gem_prime_handle_to_fd, - .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, - .gem_prime_mmap = drm_gem_cma_prime_mmap, + DRM_GEM_CMA_DRIVER_OPS, .fops = , .name = "mxsfb-drm", .desc = "MXSFB Controller DRM", -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 41/43] drm/zte: Use GEM CMA object functions
Create GEM objects with drm_gem_cma_create_object_default_funcs(), which allocates the object and sets CMA's default object functions. Corresponding callbacks in struct drm_driver are cleared. No functional changes are made. Driver and object-function instances use the same callback functions, with the exception of vunmap. The implementation of vunmap is empty and left out in CMA's default object functions. v3: * convert to DRIVER_OPS macro in a separate patch Signed-off-by: Thomas Zimmermann Acked-by: Emil Velikov --- drivers/gpu/drm/zte/zx_drm_drv.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/zte/zx_drm_drv.c b/drivers/gpu/drm/zte/zx_drm_drv.c index 1141c1ed1ed04..305394923e04c 100644 --- a/drivers/gpu/drm/zte/zx_drm_drv.c +++ b/drivers/gpu/drm/zte/zx_drm_drv.c @@ -36,15 +36,11 @@ DEFINE_DRM_GEM_CMA_FOPS(zx_drm_fops); static struct drm_driver zx_drm_driver = { .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, - .gem_free_object_unlocked = drm_gem_cma_free_object, - .gem_vm_ops = _gem_cma_vm_ops, + .gem_create_object = drm_gem_cma_create_object_default_funcs, .dumb_create = drm_gem_cma_dumb_create, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, - .gem_prime_vmap = drm_gem_cma_prime_vmap, - .gem_prime_vunmap = drm_gem_cma_prime_vunmap, .gem_prime_mmap = drm_gem_cma_prime_mmap, .fops = _drm_fops, .name = "zx-vou", -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 39/43] drm/tve200: Use GEM CMA object functions
Create GEM objects with drm_gem_cma_create_object_default_funcs(), which allocates the object and sets CMA's default object functions. Corresponding callbacks in struct drm_driver are cleared. No functional changes are made. Driver and object-function instances use the same callback functions, with the exception of vunmap. The implementation of vunmap is empty and left out in CMA's default object functions. v3: * convert to DRIVER_OPS macro in a separate patch Signed-off-by: Thomas Zimmermann Reviewed-by: Linus Walleij Acked-by: Emil Velikov --- drivers/gpu/drm/tve200/tve200_drv.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/tve200/tve200_drv.c b/drivers/gpu/drm/tve200/tve200_drv.c index 00ba9e5ce1307..f10d5bb1323ca 100644 --- a/drivers/gpu/drm/tve200/tve200_drv.c +++ b/drivers/gpu/drm/tve200/tve200_drv.c @@ -147,16 +147,12 @@ static struct drm_driver tve200_drm_driver = { .major = 1, .minor = 0, .patchlevel = 0, + .gem_create_object = drm_gem_cma_create_object_default_funcs, .dumb_create = drm_gem_cma_dumb_create, - .gem_free_object_unlocked = drm_gem_cma_free_object, - .gem_vm_ops = _gem_cma_vm_ops, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, - .gem_prime_vmap = drm_gem_cma_prime_vmap, - .gem_prime_vunmap = drm_gem_cma_prime_vunmap, .gem_prime_mmap = drm_gem_cma_prime_mmap, }; -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel