[PATCH 00/24 v2] Documentation: correct lots of spelling errors (series 1)
Correct many spelling errors in Documentation/ as reported by codespell. Maintainers of specific kernel subsystems are only Cc-ed on their respective patches, not the entire series. These patches are based on linux-next-20230209. [PATCH 01/24] Documentation: arm: correct spelling [PATCH 02/24] Documentation: block: correct spelling [PATCH 03/24] Documentation: core-api: correct spelling [PATCH 04/24] Documentation: fault-injection: correct spelling [PATCH 05/24] Documentation: fb: correct spelling [PATCH 06/24] Documentation: features: correct spelling [PATCH 07/24] Documentation: input: correct spelling [PATCH 08/24] Documentation: isdn: correct spelling [PATCH 09/24] Documentation: livepatch: correct spelling [PATCH 10/24] Documentation: locking: correct spelling [PATCH 11/24] Documentation: mm: correct spelling [PATCH 12/24] Documentation: openrisc: correct spelling [PATCH 13/24] Documentation: PCI: correct spelling [PATCH 14/24] Documentation: powerpc: correct spelling [PATCH 15/24] Documentation: s390: correct spelling [PATCH 16/24] Documentation: scheduler: correct spelling [PATCH 17/24] Documentation: security: correct spelling [PATCH 18/24] Documentation: timers: correct spelling [PATCH 19/24] Documentation: tools/rtla: correct spelling [PATCH 20/24] Documentation: trace/rv: correct spelling [PATCH 21/24] Documentation: trace: correct spelling [PATCH 22/24] Documentation: w1: correct spelling [PATCH 23/24] Documentation: x86: correct spelling [PATCH 24/24] Documentation: xtensa: correct spelling diffstat: Documentation/PCI/endpoint/pci-vntb-howto.rst|2 +- Documentation/PCI/msi-howto.rst |2 +- Documentation/arm/arm.rst|2 +- Documentation/arm/ixp4xx.rst |4 ++-- Documentation/arm/keystone/knav-qmss.rst |2 +- Documentation/arm/stm32/stm32-dma-mdma-chaining.rst |6 +++--- Documentation/arm/sunxi/clocks.rst |2 +- Documentation/arm/swp_emulation.rst |2 +- Documentation/arm/tcm.rst|2 +- Documentation/arm/vlocks.rst |2 +- Documentation/block/data-integrity.rst |2 +- Documentation/core-api/packing.rst |2 +- Documentation/core-api/padata.rst|2 +- Documentation/fault-injection/fault-injection.rst|2 +- Documentation/fb/sm712fb.rst |2 +- Documentation/fb/sstfb.rst |2 +- Documentation/features/core/thread-info-in-task/arch-support.txt |2 +- Documentation/input/devices/iforce-protocol.rst |2 +- Documentation/input/multi-touch-protocol.rst |2 +- Documentation/isdn/interface_capi.rst|2 +- Documentation/isdn/m_isdn.rst|2 +- Documentation/livepatch/reliable-stacktrace.rst |2 +- Documentation/locking/lockdep-design.rst |4 ++-- Documentation/locking/locktorture.rst|2 +- Documentation/locking/locktypes.rst |2 +- Documentation/locking/preempt-locking.rst|2 +- Documentation/mm/hmm.rst |4 ++-- Documentation/mm/hwpoison.rst|2 +- Documentation/openrisc/openrisc_port.rst |4 ++-- Documentation/power/suspend-and-interrupts.rst |2 +- Documentation/powerpc/kasan.txt |2 +- Documentation/powerpc/papr_hcalls.rst|2 +- Documentation/powerpc/qe_firmware.rst|4 ++-- Documentation/powerpc/vas-api.rst|4 ++-- Documentation/s390/pci.rst |4 ++-- Documentation/s390/vfio-ccw.rst |2 +- Documentation/scheduler/sched-bwc.rst|2 +- Documentation/scheduler/sched-energy.rst |4 ++-- Documentation/security/digsig.rst|4 ++-- Documentation/security/keys/core.rst |2 +- Documentation/security/secrets/coco.rst |2 +- Documentation/timers/hrtimers.rst|2 +- Documentation/tools/rtla/rtla-timerlat-top.rst |2 +- Documentation/trace/coresight/coresight-etm4x-reference.rst |2 +- Documentation/trace/events.rst
[PATCH 05/24] Documentation: fb: correct spelling
Correct spelling problems for Documentation/fb/ as reported by codespell. Signed-off-by: Randy Dunlap Cc: Helge Deller Cc: linux-fb...@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: Jonathan Corbet Cc: linux-...@vger.kernel.org --- Documentation/fb/sm712fb.rst |2 +- Documentation/fb/sstfb.rst |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff -- a/Documentation/fb/sm712fb.rst b/Documentation/fb/sm712fb.rst --- a/Documentation/fb/sm712fb.rst +++ b/Documentation/fb/sm712fb.rst @@ -31,5 +31,5 @@ Missing Features (alias TODO list) - * 2D acceleratrion + * 2D acceleration * dual-head support diff -- a/Documentation/fb/sstfb.rst b/Documentation/fb/sstfb.rst --- a/Documentation/fb/sstfb.rst +++ b/Documentation/fb/sstfb.rst @@ -73,7 +73,7 @@ Module insertion the device will be /dev/fb0. You can check this by doing a cat /proc/fb. You can find a copy of con2fb in tools/ directory. if you don't have another fb device, this step is superfluous, - as the console subsystem automagicaly binds ttys to the fb. + as the console subsystem automagically binds ttys to the fb. #. switch to the virtual console you just mapped. "tadaaa" ... Module removal
[PATCH] drm/amdgpu/display: remove duplicate include header in link_dpms.c
From: Ye Xingchen link_hwss.h is included more than once. Signed-off-by: Ye Xingchen --- drivers/gpu/drm/amd/display/dc/link/link_dpms.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/link/link_dpms.c b/drivers/gpu/drm/amd/display/dc/link/link_dpms.c index 9cdfa7f7dc77..0c26b3589608 100644 --- a/drivers/gpu/drm/amd/display/dc/link/link_dpms.c +++ b/drivers/gpu/drm/amd/display/dc/link/link_dpms.c @@ -51,7 +51,6 @@ #include "link_enc_cfg.h" #include "resource.h" #include "dsc.h" -#include "link_hwss.h" #include "dccg.h" #include "clk_mgr.h" #include "atomfirmware.h" -- 2.25.1
Re: [PATCH 1/5] drm/sched: Create wrapper to add a syncobj dependency to job
Am 08.02.23 um 23:20 schrieb Luben Tuikov: On 2023-02-08 14:54, Christian König wrote: Am 08.02.23 um 20:48 schrieb Maíra Canal: In order to add a syncobj's fence as a dependency to a job, it is necessary to call drm_syncobj_find_fence() to find the fence and then add the dependency with drm_sched_job_add_dependency(). So, wrap these steps in one single function, drm_sched_job_add_syncobj_dependency(). Signed-off-by: Maíra Canal Just one nit pick below, with that fixed Reviewed-by: Christian König I'm pretty sure we have the exact same function now in amdgpu cause I cleaned that up just a few weeks ago to look the same as the other drivers. Would be nice to have that new function applied there as well. Hi Christian, Is that R-B for the series or just this patch? Just this patch, haven't looked at the driver implementations in detail. Regards, Christian.
[pull] amdgpu drm-fixes-6.2
Hi Dave, Daniel, Fixes for 6.2. The following changes since commit 04119ab1a49fc41cb70f0472be5455af268fa260: nvidiafb: detect the hardware support before removing console. (2023-02-07 08:42:29 +1000) are available in the Git repository at: https://gitlab.freedesktop.org/agd5f/linux.git tags/amd-drm-fixes-6.2-2023-02-08 for you to fetch changes up to c6ac406cd8ff610a2d5da298b1d3071acfcde7f0: drm/amdgpu/smu: skip pptable init under sriov (2023-02-08 22:33:37 -0500) amd-drm-fixes-6.2-2023-02-08: amdgpu: - Flickering fixes for DCN 2.1, 3.1.2/3 - Re-enable S/G display on DCN 3.1.4 - Properly fix S/G display with AGP aperture enabled - Fix cursor offset with 180 rotation - SMU13 fixes - Use TGID for GPUVM traces - Fix oops on in fence error path - Don't run IB tests on hw rings when sw rings are in use Alex Deucher (4): drm/amd/display: disable S/G display on DCN 2.1.0 drm/amd/display: disable S/G display on DCN 3.1.2/3 drm/amd/display: properly handling AGP aperture in vm setup Revert "drm/amd/display: disable S/G display on DCN 3.1.4" Evan Quan (3): drm/amd/pm: add SMU 13.0.7 missing GetPptLimit message mapping drm/amd/pm: bump SMU 13.0.0 driver_if header version drm/amd/pm: bump SMU 13.0.7 driver_if header version Friedrich Vock (1): drm/amdgpu: Use the TGID for trace_amdgpu_vm_update_ptes Guilherme G. Piccoli (1): drm/amdgpu/fence: Fix oops due to non-matching drm_sched init/fini Jane Jian (1): drm/amdgpu/smu: skip pptable init under sriov JesseZhang (1): amd/amdgpu: remove test ib on hw ring Kenneth Feng (1): drm/amd/amdgpu: enable athub cg 11.0.3 Kent Russell (1): drm/amdgpu: Add unique_id support for GC 11.0.1/2 Melissa Wen (1): drm/amd/display: fix cursor offset on rotation 180 drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 8 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 2 +- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 1 - drivers/gpu/drm/amd/amdgpu/soc21.c | 4 +- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 46 ++ .../drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 2 +- drivers/gpu/drm/amd/pm/amdgpu_pm.c | 2 + .../pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_0.h | 5 ++- .../pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_7.h | 29 +++--- drivers/gpu/drm/amd/pm/swsmu/inc/smu_v13_0.h | 4 +- .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 6 +++ .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 1 + 13 files changed, 71 insertions(+), 41 deletions(-)
Re: [PATCH v11 1/9] device property: Add remote endpoint to devcon matcher
Hi Sakari, Thanks for the review. On Mon, Feb 6, 2023 at 5:11 AM Sakari Ailus wrote: > > Hi Pin-yen, > > On Sat, Feb 04, 2023 at 09:30:32PM +0800, Pin-yen Lin wrote: > > From: Prashant Malani > > > > When searching the device graph for device matches, check the > > remote-endpoint itself for a match. > > > > Some drivers register devices for individual endpoints. This allows > > the matcher code to evaluate those for a match too, instead > > of only looking at the remote parent devices. This is required when a > > device supports two mode switches in its endpoints, so we can't simply > > register the mode switch with the parent node. > > > > Signed-off-by: Prashant Malani > > Signed-off-by: Pin-yen Lin > > Reviewed-by: Chen-Yu Tsai > > Tested-by: Chen-Yu Tsai > > Thanks for the update. > > I intended to give my Reviewed-by: but there's something still needs to be > addressed. See below. > > > > > --- > > > > Changes in v11: > > - Added missing fwnode_handle_put in drivers/base/property.c > > > > Changes in v10: > > - Collected Reviewed-by and Tested-by tags > > > > Changes in v6: > > - New in v6 > > > > drivers/base/property.c | 16 > > 1 file changed, 16 insertions(+) > > > > diff --git a/drivers/base/property.c b/drivers/base/property.c > > index 2a5a37fcd998..e6f915b72eb7 100644 > > --- a/drivers/base/property.c > > +++ b/drivers/base/property.c > > @@ -1223,6 +1223,22 @@ static unsigned int > > fwnode_graph_devcon_matches(struct fwnode_handle *fwnode, > > break; > > } > > > > + /* > > + * Some drivers may register devices for endpoints. Check > > + * the remote-endpoints for matches in addition to the remote > > + * port parent. > > + */ > > + node = fwnode_graph_get_remote_endpoint(ep); > > Here fwnode_graph_get_remote_endpoint() returns an endpoint... > > > + if (fwnode_device_is_available(node)) { > > and you're calling fwnode_device_is_available() on the endpoint node, which > always returns true. > > Shouldn't you call this on the device node instead? What about match() > below? Yes we should have checked the availability on the device node itself instead of the endpoint node. But regarding the match() call, we need to call it with the endpoint node because that's where we put the "mode-switch" properties and register the mode switches on. We can't use the device node because we want to register two mode switches for the same device node. Regards, Pin-yen > > > + ret = match(node, con_id, data); > > + if (ret) { > > + if (matches) > > + matches[count] = ret; > > + count++; > > + } > > + } > > + fwnode_handle_put(node); > > + > > node = fwnode_graph_get_remote_port_parent(ep); > > if (!fwnode_device_is_available(node)) { > > fwnode_handle_put(node); > > -- > Kind regards, > > Sakari Ailus
Re: [PATCH v3 0/4] Reserve DSPPs based on user request
Hi, On Wed, Feb 8, 2023 at 8:16 PM Kalyan Thota wrote: > > Hi Doug, > > Have you picked the core change to program dspp's (below) ? the current > series will go on top of it. > https://patchwork.kernel.org/project/linux-arm-msm/patch/1671542719-12655-1-git-send-email-quic_kaly...@quicinc.com/ I didn't pick v11 of it like you link, but I did pick v12 of the same patch. In my response I said that I picked 5 patches, this series plus [1] where [1] is: [1] https://lore.kernel.org/all/1674814487-2112-1-git-send-email-quic_kaly...@quicinc.com/ > Thanks, > Kalyan > > >-Original Message- > >From: Doug Anderson > >Sent: Wednesday, February 8, 2023 10:44 PM > >To: Kalyan Thota (QUIC) > >Cc: dri-devel@lists.freedesktop.org; linux-arm-...@vger.kernel.org; > >freedr...@lists.freedesktop.org; devicet...@vger.kernel.org; linux- > >ker...@vger.kernel.org; robdcl...@chromium.org; swb...@chromium.org; > >Vinod Polimera (QUIC) ; > >dmitry.barysh...@linaro.org; Abhinav Kumar (QUIC) > >; marijn.suij...@somainline.org > >Subject: Re: [PATCH v3 0/4] Reserve DSPPs based on user request > > > >WARNING: This email originated from outside of Qualcomm. Please be wary of > >any links or attachments, and do not enable macros. > > > >Hi, > > > >On Wed, Feb 8, 2023 at 5:42 AM Kalyan Thota > >wrote: > >> > >> This series will enable color features on sc7280 target which has > >> primary panel as eDP > >> > >> The series removes DSPP allocation based on encoder type and allows > >> the DSPP reservation based on user request via CTM. > >> > >> The series will release/reserve the dpu resources when ever there is a > >> topology change to suit the new requirements. > >> > >> Kalyan Thota (4): > >> drm/msm/dpu: clear DSPP reservations in rm release > >> drm/msm/dpu: add DSPPs into reservation upon a CTM request > >> drm/msm/dpu: avoid unnecessary check in DPU reservations > >> drm/msm/dpu: reserve the resources on topology change > >> > >> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 2 + > >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 58 -- > >--- > >> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 2 + > >> 3 files changed, 37 insertions(+), 25 deletions(-) > > > >I tried out your changes, but unfortunately it seems like there's something > >wrong. > >:( I did this: > > > >1. Picked your 5 patches to the chromeos-5.15 tree (this series plus [1]) > > > >2. Put them on herobrine villager. > > > >3. Booted up with no external display plugged in. > > > >4. Tried to enable night light in the ChromeOS UI. > > > >5. Night light didn't turn on for the internal display. > > > > > >I also tried applying them to the top of msm-next (had to resolve some small > >conflicts). Same thing, night light didn't work. > > > > > >I thought maybe this was because the Chrome browser hasn't been updated to > >properly use atomic_check for testing for night light, so I hacked my > >herobrine > >device tree to not mark "mdss_dp" as "okay". Now there's _only_ an eDP > >display. > >Same thing, night light didn't work. > > > > > >I could only get night light to work for the internal display if I plugged > >and > >unplugged an external display in. > > > > > >Is the above the behavior that's expected right now? > > > > > >[1] https://lore.kernel.org/all/1674814487-2112-1-git-send-email- > >quic_kaly...@quicinc.com/
Re: [PATCH] firmware: qcom_scm: Move qcom_scm.h to include/linux/firmware/qcom/
On Fri, 3 Feb 2023 13:09:52 -0800, Elliot Berman wrote: > Move include/linux/qcom_scm.h to include/linux/firmware/qcom/qcom_scm.h. > This removes 1 of a few remaining Qualcomm-specific headers into a more > approciate subdirectory under include/. > > Applied, thanks! [1/1] firmware: qcom_scm: Move qcom_scm.h to include/linux/firmware/qcom/ commit: 3bf90eca76c98c55c975fa817799789b9176f9f3 Best regards, -- Bjorn Andersson
Re: (subset) [PATCH v2 0/8] arm64: dts: qcom: sm8350: enable GPU on the HDK board
On Mon, 6 Feb 2023 16:56:59 +0200, Dmitry Baryshkov wrote: > Add A660 device to the Qualcomm SM8350 platform and enable it for the > sm8350-hdk board. Unfortunately while adding the GPU & related devices I > noticed that DT nodes on SM8350 are greatly out of the preagreed order, > so patches 4-6 reorder DT nodes to follow the agreement. > > Changes since v1: > - Fixed the subject and commit message for patch 1 > - Fixed GMU's clocks to follow the vendor kernel > - Marked Adreno SMMU as dma-coherent > - Dropped comments targeting sm8350 v1, we do not support that chip > revision. > > [...] Applied, thanks! [2/8] dt-bindings: power: qcom,rpmpd: add RPMH_REGULATOR_LEVEL_LOW_SVS_L1 commit: bdd133c2eeffad142e7c8a48ab7e86e1ca37e67d Best regards, -- Bjorn Andersson
Re: (subset) [PATCH 0/6] drm/msm/hdmi: integrate msm8960 HDMI PHY with DT clocks infrastructure
On Thu, 19 Jan 2023 15:22:13 +0200, Dmitry Baryshkov wrote: > Make msm8960's HDMI PHY accept clocks from DT and also register it as a > DT clock provider. > > Dmitry Baryshkov (6): > dt-bindings: phy: qcom,hdmi-phy-other: use pxo clock > dt-bindings: phy: qcom,hdmi-phy-other: mark it as clock provider > drm/msm/hdmi: switch hdmi_pll_8960 to use parent_data > drm/msm/hdmi: make hdmi_phy_8960 OF clk provider > ARM: dts: qcom: apq8064: add #clock-cells to the HDMI PHY node > ARM: dts: qcom: apq8064: use hdmi_phy for the MMCC's hdmipll clock > > [...] Applied, thanks! [5/6] ARM: dts: qcom: apq8064: add #clock-cells to the HDMI PHY node commit: c9f678afe0bbdfb3748c1db5ac94d1c02a6eb64b [6/6] ARM: dts: qcom: apq8064: use hdmi_phy for the MMCC's hdmipll clock commit: f1a359db6d9d198b84877e2340dd7c37809a4882 Best regards, -- Bjorn Andersson
RE: [PATCH v3 0/4] Reserve DSPPs based on user request
Hi Doug, Have you picked the core change to program dspp's (below) ? the current series will go on top of it. https://patchwork.kernel.org/project/linux-arm-msm/patch/1671542719-12655-1-git-send-email-quic_kaly...@quicinc.com/ Thanks, Kalyan >-Original Message- >From: Doug Anderson >Sent: Wednesday, February 8, 2023 10:44 PM >To: Kalyan Thota (QUIC) >Cc: dri-devel@lists.freedesktop.org; linux-arm-...@vger.kernel.org; >freedr...@lists.freedesktop.org; devicet...@vger.kernel.org; linux- >ker...@vger.kernel.org; robdcl...@chromium.org; swb...@chromium.org; >Vinod Polimera (QUIC) ; >dmitry.barysh...@linaro.org; Abhinav Kumar (QUIC) >; marijn.suij...@somainline.org >Subject: Re: [PATCH v3 0/4] Reserve DSPPs based on user request > >WARNING: This email originated from outside of Qualcomm. Please be wary of >any links or attachments, and do not enable macros. > >Hi, > >On Wed, Feb 8, 2023 at 5:42 AM Kalyan Thota >wrote: >> >> This series will enable color features on sc7280 target which has >> primary panel as eDP >> >> The series removes DSPP allocation based on encoder type and allows >> the DSPP reservation based on user request via CTM. >> >> The series will release/reserve the dpu resources when ever there is a >> topology change to suit the new requirements. >> >> Kalyan Thota (4): >> drm/msm/dpu: clear DSPP reservations in rm release >> drm/msm/dpu: add DSPPs into reservation upon a CTM request >> drm/msm/dpu: avoid unnecessary check in DPU reservations >> drm/msm/dpu: reserve the resources on topology change >> >> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 2 + >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 58 -- >--- >> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 2 + >> 3 files changed, 37 insertions(+), 25 deletions(-) > >I tried out your changes, but unfortunately it seems like there's something >wrong. >:( I did this: > >1. Picked your 5 patches to the chromeos-5.15 tree (this series plus [1]) > >2. Put them on herobrine villager. > >3. Booted up with no external display plugged in. > >4. Tried to enable night light in the ChromeOS UI. > >5. Night light didn't turn on for the internal display. > > >I also tried applying them to the top of msm-next (had to resolve some small >conflicts). Same thing, night light didn't work. > > >I thought maybe this was because the Chrome browser hasn't been updated to >properly use atomic_check for testing for night light, so I hacked my herobrine >device tree to not mark "mdss_dp" as "okay". Now there's _only_ an eDP display. >Same thing, night light didn't work. > > >I could only get night light to work for the internal display if I plugged and >unplugged an external display in. > > >Is the above the behavior that's expected right now? > > >[1] https://lore.kernel.org/all/1674814487-2112-1-git-send-email- >quic_kaly...@quicinc.com/
Re: [PATCH v11 7/9] dt-bindings: display: bridge: it6505: Add mode-switch support
Hi Rob, Thanks for the review. On Wed, Feb 8, 2023 at 4:52 AM Rob Herring wrote: > > On Sat, Feb 04, 2023 at 09:30:38PM +0800, Pin-yen Lin wrote: > > ITE IT6505 can be used in systems to switch the DP traffic between > > two downstreams, which can be USB Type-C DisplayPort alternate mode > > lane or regular DisplayPort output ports. > > > > Update the binding to accommodate this usage by introducing a > > data-lanes and a mode-switch property on endpoints. > > > > Signed-off-by: Pin-yen Lin > > > > --- > > > > Changes in v11: > > - Updated the description of the endpoints in the bindings > > - Referenced video-interfaces.yaml instead for the endpoints binding > > - Removed duplicated definitions from inherited schema > > > > Changes in v9: > > - Fixed subject prefix again > > - Changed the naming of the example node for it6505 > > > > Changes in v8: > > - Updated bindings for data-lanes property > > - Fixed subject prefix > > > > Changes in v7: > > - Fixed issues reported by dt_binding_check. > > - Updated the schema and the example dts for data-lanes. > > - Changed to generic naming for the example dts node. > > > > Changes in v6: > > - Remove switches node and use endpoints and data-lanes property to > > describe the connections. > > > > .../bindings/display/bridge/ite,it6505.yaml | 101 +++--- > > 1 file changed, 88 insertions(+), 13 deletions(-) > > > > diff --git > > a/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml > > b/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml > > index b16a9d9127dd..8ae9c5cba22c 100644 > > --- a/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml > > +++ b/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml > > @@ -75,22 +75,49 @@ properties: > >port@1: > > $ref: /schemas/graph.yaml#/$defs/port-base > > unevaluatedProperties: false > > -description: Video port for DP output > > +description: > > + Video port for DP output. Each endpoint connects to a video > > output > > + downstream, and the "data-lanes" property is used to describe > > the pin > > + connections. 0, 1, 2, 3 in "data-lanes" maps to TX0, TX1, TX2, > > TX3, > > + respectively. > > > > -properties: > > - endpoint: > > -$ref: /schemas/graph.yaml#/$defs/endpoint-base > > + > > +patternProperties: > > + "^endpoint@[01]$": > > +$ref: /schemas/media/video-interfaces.yaml# > > unevaluatedProperties: false > > > > properties: > > + reg: true > > + > > + remote-endpoint: true > > + > >data-lanes: > > -minItems: 1 > > -uniqueItems: true > > -items: > > - - enum: [ 0, 1 ] > > - - const: 1 > > - - const: 2 > > - - const: 3 > > +oneOf: > > + - items: > > + - enum: [0, 1, 2, 3] > > + > > + - items: > > + - const: 0 > > + - const: 1 > > + > > + - items: > > + - const: 2 > > + - const: 3 > > + > > + - items: > > + - const: 0 > > + - const: 1 > > + - const: 2 > > + - const: 3 > > + > > + mode-switch: > > +type: boolean > > +description: Register this node as a Type-C mode switch or > > not. > > Existing users put this property in the device's node, not the endpoint. > That seems more like a property of the device, than the DP link. In our use case, we want to register two mode switches for the same device. That's why we put the "mode-switch" property in the endpoints instead of the device node. > > You are using fwnode_typec_mux_get(), right? Yes. This is called by cros_ec_typec.c[1] in our use case. [1]: https://elixir.bootlin.com/linux/latest/source/drivers/platform/chrome/cros_ec_typec.c#L148 Regards, Pin-yen > > Rob
Re: [PATCH v2 4/8] arm64: dts: qcom: sm8350: reorder device nodes
On Mon, Feb 06, 2023 at 04:57:03PM +0200, Dmitry Baryshkov wrote: > Start ordering DT nodes according to agreed order. Move apps SMMU, GIC, > timer, apps RSC, cpufreq ADSP and cDSP nodes to the end to the proper > position at the end of /soc/. > I think "according to agreed order" means "sorted by address", but it would be nice to have that expressed in the message. If nothing else for others to know what such agreed order might be. Unfortunately this doesn't apply to my tree, and it's not clear where it failed. Could you please rebase this? Thanks, Bjorn > Signed-off-by: Dmitry Baryshkov > --- > arch/arm64/boot/dts/qcom/sm8350.dtsi | 1228 +- > 1 file changed, 614 insertions(+), 614 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi > b/arch/arm64/boot/dts/qcom/sm8350.dtsi > index 0de42a333d32..061aa3fec1c4 100644 > --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi > @@ -1423,111 +1423,6 @@ spi13: spi@a94000 { > }; > }; > > - apps_smmu: iommu@1500 { > - compatible = "qcom,sm8350-smmu-500", "arm,mmu-500"; > - reg = <0 0x1500 0 0x10>; > - #iommu-cells = <2>; > - #global-interrupts = <2>; > - interrupts =, > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > - , > -
Re: remove arch/sh
On 2/8/23 06:13, John Paul Adrian Glaubitz wrote: > Hi Randy! > > On Tue, 2023-02-07 at 17:31 -0800, Randy Dunlap wrote: >> >> On 2/7/23 01:06, John Paul Adrian Glaubitz wrote: >> > Hello Christoph! >> > >> > On Fri, 2023-02-03 at 08:14 +0100, Christoph Hellwig wrote: >> > > On Mon, Jan 16, 2023 at 09:52:10AM +0100, John Paul Adrian Glaubitz >> > > wrote: >> > > > We have had a discussion between multiple people invested in the >> > > > SuperH port and >> > > > I have decided to volunteer as a co-maintainer of the port to support >> > > > Rich Felker >> > > > when he isn't available. >> > > >> > > So, this still isn't reflected in MAINTAINERS in linux-next. When >> > > do you plan to take over? What platforms will remain supported and >> > > what can we start dropping due to being unused and unmaintained? >> > >> > I'm getting everything ready now with Geert's help and I have a probably >> > dumb >> > question regarding the MAINTAINERS file change: Shall I just add myself as >> > an >> > additional maintainer first or shall I also drop Yoshinori Sato? >> > >> > Also, is it desirable to add a "T:" entry for the kernel tree? >> >> Yes, definitely. > > Geert has suggested to wait with adding a tree source to the entry until I > get my > own kernel.org account. I have enough GPG signatures from multiple kernel > developers > on my GPG key, so I think it shouldn't be too difficult to qualify for an > account. So you're not planning to use https://lk.j-core.org/J-Core-Developers/sh-linux but push to kernel.org and ask Linus to pull from there? > Adrian Rob
Re: remove arch/sh
On 2/3/23 09:57, Randy Dunlap wrote: > Hi-- > > On 2/3/23 02:33, Geert Uytterhoeven wrote: >> Hi Adrian, >> >> On Fri, Feb 3, 2023 at 11:29 AM John Paul Adrian Glaubitz >> wrote: >>> On Fri, 2023-02-03 at 09:30 +0100, Christoph Hellwig wrote: On Fri, Feb 03, 2023 at 09:24:46AM +0100, John Paul Adrian Glaubitz wrote: > Since this is my very first time stepping up as a kernel maintainer, I > was hoping > to get some pointers on what to do to make this happen. > > So far, we have set up a new kernel tree and I have set up a local > development and > test environment for SH kernels using my SH7785LCR board as the target > platform. > > Do I just need to send a patch asking to change the corresponding entry > in the > MAINTAINERS file? I'm not sure a there is a document, but: - add the MAINTAINERS change to your tree - ask Stephen to get your tree included in linux-next then eventually send a pull request to Linus with all of that. Make sure it's been in linux-next for a while. >>> >>> OK, thanks for the pointers! Will try to get this done by next week. >>> >>> We're still discussing among SuperH developer community whether there will >>> be a second >>> maintainer, so please bear with us a few more days. I will collect patches >>> in the >>> meantime. >> >> Thanks a lot! >> >> If you need any help with process, setup, ... don't hesitate to ask >> (on e.g. #renesas-soc on Libera). > > While Adrian and Geert are reading this, I have a question: > > Is this "sh64" still accurate and applicable? I hadn't noticed it was there... Randy Dunlap added that in 2018 (commit 09b1565324cba). I wonder why? > from Documentation/kbuild/kbuild.rst: There isn't an active 64 bit superh architecture for the moment: sh5 was a prototype that never shipped in volume, and support was removed in commit 37744feebc08. From the j-core side j64 hasn't shipped yet either (still planned last I heard, but j-core went downmarket first instead due to customer demand, and multi-issue is on the roadmap before 64 bit address space). The general trend in linux kernel architectures has been to merge 32 and 64 bit anyway, and just have the .config set CONFIG_64BIT to distinguish: arch/x86 was created by merging arch/i386 and arch/x86_64 in 2007, arch/powerpc merged the 32 and 64 bit directories in 2005, arch/s390 and s390x are in the same dir, arch/mips... (For some reason arm and arm64 are still split, but that might be fallout from Arm Ltd trying to distinguish aarrcchh6644 from "arm" for some reason? Dunno.) I wonder why is this going the other way? I thought $ARCH mostly just specified the subdirectory under arch/ with a few historical aliases in the top level Makefile: # Additional ARCH settings for x86 ifeq ($(ARCH),i386) SRCARCH := x86 endif ifeq ($(ARCH),x86_64) SRCARCH := x86 endif # Additional ARCH settings for sparc ifeq ($(ARCH),sparc32) SRCARCH := sparc endif ifeq ($(ARCH),sparc64) SRCARCH := sparc endif # Additional ARCH settings for parisc ifeq ($(ARCH),parisc64) SRCARCH := parisc endif But you could always just specify the correct ARCH directory directly and it would work. (Always did when I tried it, although I haven't built sparc in years because there's no musl-libc support, and never built parisc64 because I couldn't get it to work with uClibc even before musl. I _am_ still building both 32 bit and 64 bit x86 with ARCH=x86 both times...) > But some architectures such as x86 and sparc have aliases. > > - x86: i386 for 32 bit, x86_64 for 64 bit > - sh: sh for 32 bit, sh64 for 64 bit <<< > - sparc: sparc32 for 32 bit, sparc64 for 64 bit Randy also added the sparc alias in commit 5ba800962a80. That at least exists in the top level Makefile. Did he mean parisc64 and typoed sh64? Because that's the only other alias in the top level Makefile... In any case, these are historical aliases for old builds, which can probably get yanked because it should be a trivial fix to use the right ARCH= value for modern builds? (I'd think?) You'd even be able to build a 64 bit version of ARCH=i386 just fine if it wasn't for the ONE place in arch/x86/Kconfig that actually checks: config 64BIT bool "64-bit kernel" if "$(ARCH)" = "x86" default "$(ARCH)" != "i386" Same for arch/sparc/Kconfig: config 64BIT bool "64-bit kernel" if "$(ARCH)" = "sparc" default "$(ARCH)" = "sparc64" Nothing else anywhere seems to care... > Thanks. Rob
Re: [PATCH 1/9] dt-bindings: gpu: mali-bifrost: Don't allow sram-supply by default
On Wed, Feb 8, 2023 at 6:37 PM AngeloGioacchino Del Regno wrote: > > The sram-supply is MediaTek-specific, it is and will ever be used > only for the mediatek,mt8183-mali compatible due to the addition of > the mediatek-regulator-coupler driver: change the binding to add > this supply when mediatek,mt8183-mali is present as a compatible > instead of disabling it when not present. > > This is done in preparation for adding new bindings for other > MediaTek SoCs, such as MT8192 and others. > > Signed-off-by: AngeloGioacchino Del Regno > > --- > Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > index 78964c140b46..69212f3b1328 100644 > --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > @@ -57,8 +57,6 @@ properties: > >mali-supply: true > > - sram-supply: true > - Have you tried actually validating the device trees against this? Based on my previous tests this gives out errors. The reason is that each conditional is a separate sub-schema, and the validator is run against each schema and sub-schema separately, instead of collapsing matching schemas and sub-schemas together and validating once. So we'll get a validation error on sram-supply not being a valid property when validating current mt8183 against the base schema. We have a similar issue with power-domain-names, for which I'll send a patch to fix. See the following for the fix: http://git.kernel.org/wens/c/d1adb38ab2ad0442755607c2bcc726cc17cce2c7 and the following for what I did for MT8192 on top of the previous patch: http://git.kernel.org/wens/c/049bd164884398d7e5f72c710da6aaa9a95bc10a Regards ChenYu >operating-points-v2: true > >power-domains: > @@ -157,6 +155,7 @@ allOf: > - const: core0 > - const: core1 > - const: core2 > +sram-supply: true > >required: > - sram-supply > @@ -166,7 +165,6 @@ allOf: >properties: > power-domains: >maxItems: 1 > -sram-supply: false >- if: >properties: > compatible: > -- > 2.39.1 >
Re: [PATCH v3 27/27] drm/msm/dpu: add support for wide planes
On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote: Typically SSPP can support rectangle with width up to 2560. However it's Not always 2560. Depends on the chipset. possible to use multirect feature and split source to use the SSPP to output two consecutive rectangles. This commit brings in this capability to support wider screen resolutions. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 6 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 116 +++--- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 4 + 3 files changed, 114 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 0ca3bc38ff7e..867832a752b2 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -485,6 +485,12 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, fetch_active, >pipe); + _dpu_crtc_blend_setup_pipe(crtc, plane, + mixer, cstate->num_mixers, + stage_cfg, pstate->stage, 1, + fetch_active, + >r_pipe); + /* blend config update */ for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++) { _dpu_crtc_setup_blend_cfg(mixer + lm_idx, pstate, format); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index e2e85688ed3c..401ead64c6bd 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -365,6 +365,9 @@ static void _dpu_plane_set_qos_ctrl(struct drm_plane *plane, struct dpu_plane *pdpu = to_dpu_plane(plane); struct dpu_hw_pipe_qos_cfg pipe_qos_cfg; + if (!pipe->sspp) + return; + memset(_qos_cfg, 0, sizeof(pipe_qos_cfg)); if (flags & DPU_PLANE_QOS_VBLANK_CTRL) { @@ -647,6 +650,9 @@ static int _dpu_plane_color_fill_pipe(struct dpu_plane_state *pstate, { struct dpu_hw_sspp_cfg pipe_cfg; + if (!pipe->sspp) + return 0; instead of checking if sspp was present, is it not better for the caller to check if the rpipe is valid before calling this? + /* update sspp */ if (!pipe->sspp->ops.setup_solidfill) return 0; @@ -701,6 +707,8 @@ static void _dpu_plane_color_fill(struct dpu_plane *pdpu, /* update sspp */ _dpu_plane_color_fill_pipe(pstate, >pipe, >pipe_cfg, fill_color, fmt); + + _dpu_plane_color_fill_pipe(pstate, >r_pipe, >r_pipe_cfg, fill_color, fmt); } So cant we do if (pstate->r_pipe.sspp) _dpu_plane_color_fill_pipe(pstate, >r_pipe, >r_pipe_cfg, fill_color, fmt); It just seems better to me as the caller would already know if the sspp was assigned. int dpu_plane_validate_multirect_v2(struct dpu_multirect_plane_states *plane) @@ -911,6 +919,9 @@ static int dpu_plane_atomic_check_pipe(struct dpu_plane *pdpu, { uint32_t min_src_size; + if (!pipe->sspp) + return 0; + min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1; if (DPU_FORMAT_IS_YUV(fmt) && @@ -957,9 +968,12 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, int ret = 0, min_scale; struct dpu_plane *pdpu = to_dpu_plane(plane); struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state); + struct dpu_sw_pipe *pipe = >pipe; + struct dpu_sw_pipe *r_pipe = >r_pipe; const struct drm_crtc_state *crtc_state = NULL; const struct dpu_format *fmt; struct dpu_hw_sspp_cfg *pipe_cfg = >pipe_cfg; + struct dpu_hw_sspp_cfg *r_pipe_cfg = >r_pipe_cfg; struct drm_rect fb_rect = { 0 }; uint32_t max_linewidth; unsigned int rotation; @@ -983,8 +997,11 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, if (!new_plane_state->visible) return 0; - pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO; - pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE; + pipe->multirect_index = DPU_SSPP_RECT_SOLO; + pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; + r_pipe->multirect_index = DPU_SSPP_RECT_SOLO; + r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; + r_pipe->sspp = NULL; pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos; if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) { @@ -1016,16 +1033,53 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, max_linewidth = pdpu->catalog->caps->max_linewidth; - /* check decimated source width */ if (drm_rect_width(_cfg->src_rect) > max_linewidth) { - DPU_DEBUG_PLANE(pdpu, "invalid src "
Re: [PATCH v3 25/27] drm/msm/dpu: rework static color fill code
On 09/02/2023 00:34, Abhinav Kumar wrote: On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote: Rework static color fill code to separate the pipe / pipe_cfg handling. This is a preparation for the r_pipe support. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 70 +-- 1 file changed, 41 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 05047192cb37..e2e85688ed3c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -639,20 +639,54 @@ static void _dpu_plane_setup_scaler(struct dpu_sw_pipe *pipe, fmt); } +static int _dpu_plane_color_fill_pipe(struct dpu_plane_state *pstate, + struct dpu_sw_pipe *pipe, + struct dpu_hw_sspp_cfg *old_pipe_cfg, Why is this called old_pipe_cfg instead of just pipe_cfg? Ack. Probably got that wrong during mass-renaming and then missed to fix it. + u32 fill_color, + const struct dpu_format *fmt) +{ + struct dpu_hw_sspp_cfg pipe_cfg; + + /* update sspp */ + if (!pipe->sspp->ops.setup_solidfill) + return 0; You can just return from here and make this function void? Of course. + + pipe->sspp->ops.setup_solidfill(pipe, fill_color); + + /* override scaler/decimation if solid fill */ + pipe_cfg.dst_rect = old_pipe_cfg->dst_rect; + + pipe_cfg.src_rect.x1 = 0; + pipe_cfg.src_rect.y1 = 0; + pipe_cfg.src_rect.x2 = + drm_rect_width(_cfg.dst_rect); + pipe_cfg.src_rect.y2 = + drm_rect_height(_cfg.dst_rect); + + if (pipe->sspp->ops.setup_format) + pipe->sspp->ops.setup_format(pipe, fmt, DPU_SSPP_SOLID_FILL); + + if (pipe->sspp->ops.setup_rects) + pipe->sspp->ops.setup_rects(pipe, _cfg); + + _dpu_plane_setup_scaler(pipe, fmt, true, _cfg, pstate->rotation); + + return 0; +} + /** * _dpu_plane_color_fill - enables color fill on plane * @pdpu: Pointer to DPU plane object * @color: RGB fill color value, [23..16] Blue, [15..8] Green, [7..0] Red * @alpha: 8-bit fill alpha value, 255 selects 100% alpha - * Returns: 0 on success */ -static int _dpu_plane_color_fill(struct dpu_plane *pdpu, +static void _dpu_plane_color_fill(struct dpu_plane *pdpu, uint32_t color, uint32_t alpha) { const struct dpu_format *fmt; const struct drm_plane *plane = >base; struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state); - struct dpu_hw_sspp_cfg pipe_cfg; + u32 fill_color = (color & 0xFF) | ((alpha & 0xFF) << 24); DPU_DEBUG_PLANE(pdpu, "\n"); @@ -661,34 +695,12 @@ static int _dpu_plane_color_fill(struct dpu_plane *pdpu, * h/w only supports RGB variants */ fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR); + /* should not happen ever */ + if (!fmt) + return; /* update sspp */ - if (fmt && pstate->pipe.sspp->ops.setup_solidfill) { - pstate->pipe.sspp->ops.setup_solidfill(>pipe, - (color & 0xFF) | ((alpha & 0xFF) << 24)); - - /* override scaler/decimation if solid fill */ - pipe_cfg.dst_rect = pstate->base.dst; - - pipe_cfg.src_rect.x1 = 0; - pipe_cfg.src_rect.y1 = 0; - pipe_cfg.src_rect.x2 = - drm_rect_width(_cfg.dst_rect); - pipe_cfg.src_rect.y2 = - drm_rect_height(_cfg.dst_rect); - - if (pstate->pipe.sspp->ops.setup_format) - pstate->pipe.sspp->ops.setup_format(>pipe, - fmt, DPU_SSPP_SOLID_FILL); - - if (pstate->pipe.sspp->ops.setup_rects) - pstate->pipe.sspp->ops.setup_rects(>pipe, - _cfg); - - _dpu_plane_setup_scaler(>pipe, fmt, true, _cfg, pstate->rotation); - } - - return 0; + _dpu_plane_color_fill_pipe(pstate, >pipe, >pipe_cfg, fill_color, fmt); } int dpu_plane_validate_multirect_v2(struct dpu_multirect_plane_states *plane) -- With best wishes Dmitry
Re: [PATCH v3 22/27] drm/msm/dpu: rework dpu_plane_sspp_atomic_update()
On 07/02/2023 02:22, Abhinav Kumar wrote: On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote: Split pipe-dependent code from dpu_plane_sspp_atomic_update() into the sspp-dependent? No, this is really pipe-dependent. It takes dpu_sw_pipe and dpu_sw_pipe_cfg arguments. separate function dpu_plane_sspp_update_pipe(). This is one of preparational steps to add r_pipe support. Signed-off-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v3 22/27] drm/msm/dpu: rework dpu_plane_sspp_atomic_update()
On 07/02/2023 02:22, Abhinav Kumar wrote: On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote: Split pipe-dependent code from dpu_plane_sspp_atomic_update() into the sspp-dependent? separate function dpu_plane_sspp_update_pipe(). This is one of preparational steps to add r_pipe support. Signed-off-by: Dmitry Baryshkov Just a couple of minor comments below but otherwise this split up lgtm --- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 113 -- 1 file changed, 63 insertions(+), 50 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 0986e740b978..f94e132733f3 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -404,12 +404,13 @@ static void _dpu_plane_set_qos_ctrl(struct drm_plane *plane, * _dpu_plane_set_ot_limit - set OT limit for the given plane * @plane: Pointer to drm plane * @pipe: Pointer to software pipe - * @crtc: Pointer to drm crtc * @pipe_cfg: Pointer to pipe configuration + * @frame_rate: CRTC's frame rate Can you please check the spacing here. There seems to be an extra tab before the CRTC's frame rate */ static void _dpu_plane_set_ot_limit(struct drm_plane *plane, struct dpu_sw_pipe *pipe, - struct drm_crtc *crtc, struct dpu_hw_sspp_cfg *pipe_cfg) + struct dpu_hw_sspp_cfg *pipe_cfg, + int frame_rate) { struct dpu_plane *pdpu = to_dpu_plane(plane); struct dpu_vbif_set_ot_params ot_params; @@ -421,7 +422,7 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane, ot_params.width = drm_rect_width(_cfg->src_rect); ot_params.height = drm_rect_height(_cfg->src_rect); ot_params.is_wfd = !pdpu->is_rt_pipe; - ot_params.frame_rate = drm_mode_vrefresh(>mode); + ot_params.frame_rate = frame_rate; ot_params.vbif_idx = VBIF_RT; ot_params.clk_ctrl = pipe->sspp->cap->clk_ctrl; ot_params.rd = true; @@ -457,26 +458,6 @@ static void _dpu_plane_set_qos_remap(struct drm_plane *plane, dpu_vbif_set_qos_remap(dpu_kms, _params); } -static void _dpu_plane_set_scanout(struct drm_plane *plane, - struct dpu_plane_state *pstate, - struct drm_framebuffer *fb) -{ - struct dpu_plane *pdpu = to_dpu_plane(plane); - struct dpu_kms *kms = _dpu_plane_get_kms(>base); - struct msm_gem_address_space *aspace = kms->base.aspace; - struct dpu_hw_fmt_layout layout; - int ret; - - ret = dpu_format_populate_layout(aspace, fb, ); - if (ret) - DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret); - else if (pstate->pipe.sspp->ops.setup_sourceaddress) { - trace_dpu_plane_set_scanout(>pipe, - ); - pstate->pipe.sspp->ops.setup_sourceaddress(>pipe, ); - } -} - static void _dpu_plane_setup_scaler3(struct dpu_hw_sspp *pipe_hw, uint32_t src_w, uint32_t src_h, uint32_t dst_w, uint32_t dst_h, struct dpu_hw_scaler3_cfg *scale_cfg, @@ -1102,35 +1083,25 @@ void dpu_plane_set_error(struct drm_plane *plane, bool error) pdpu->is_error = error; } -static void dpu_plane_sspp_atomic_update(struct drm_plane *plane) +static void dpu_plane_sspp_update_pipe(struct drm_plane *plane, + struct dpu_sw_pipe *pipe, + struct dpu_hw_sspp_cfg *pipe_cfg, You can call this parameter sspp_cfg instead of pipe_cfg? I think, I'll add a commit renaming dpu_hw_sspp_cfg to dpu_sw_pipe_cfg, it would be simmetrical to dpu_sw_pipe then. -- With best wishes Dmitry
Re: [PATCH v3 18/27] drm/msm/dpu: populate SmartDMA features in hw catalog
On 05/02/2023 02:36, Abhinav Kumar wrote: On 2/4/2023 4:29 PM, Dmitry Baryshkov wrote: On Sun, 5 Feb 2023 at 01:20, Abhinav Kumar wrote: On 2/4/2023 1:08 PM, Dmitry Baryshkov wrote: On 04/02/2023 20:35, Abhinav Kumar wrote: On 2/4/2023 2:43 AM, Dmitry Baryshkov wrote: On 04/02/2023 07:10, Abhinav Kumar wrote: On 2/3/2023 8:10 PM, Dmitry Baryshkov wrote: On 04/02/2023 04:43, Abhinav Kumar wrote: On 2/3/2023 6:29 PM, Dmitry Baryshkov wrote: On 04/02/2023 01:35, Abhinav Kumar wrote: On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote: Downstream driver uses dpu->caps->smart_dma_rev to update sspp->cap->features with the bit corresponding to the supported SmartDMA version. Upstream driver does not do this, resulting in SSPP subdriver not enbaling setup_multirect callback. Add corresponding SmartDMA SSPP feature bits to dpu hw catalog. While reviewing this patch, I had a first hand experience of how we are reusing SSPP bitmasks for so many chipsets but I think overall you got them right here :) Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c index cf053e8f081e..fc818b0273e7 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -21,13 +21,16 @@ (VIG_MASK | BIT(DPU_SSPP_SCALER_QSEED3)) #define VIG_SDM845_MASK \ - (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3)) + (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3) |\ + BIT(DPU_SSPP_SMART_DMA_V2)) #define VIG_SC7180_MASK \ - (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED4)) + (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED4) |\ + BIT(DPU_SSPP_SMART_DMA_V2)) #define VIG_SM8250_MASK \ - (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3LITE)) + (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3LITE) |\ + BIT(DPU_SSPP_SMART_DMA_V2)) #define VIG_QCM2290_MASK (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL)) @@ -42,6 +45,7 @@ #define DMA_SDM845_MASK \ (BIT(DPU_SSPP_SRC) | BIT(DPU_SSPP_QOS) | BIT(DPU_SSPP_QOS_8LVL) |\ BIT(DPU_SSPP_TS_PREFILL) | BIT(DPU_SSPP_TS_PREFILL_REC1) |\ + BIT(DPU_SSPP_SMART_DMA_V2) |\ BIT(DPU_SSPP_CDP) | BIT(DPU_SSPP_EXCL_RECT)) #define DMA_CURSOR_SDM845_MASK \ VIG_SDM845_MASK and DMA_SDM845_MASK are used for many other chipsets like 8250, 8450, 8550. At the moment, for visual validation of this series, I only have sc7180/sc7280. We are leaving the rest for CI. Was that an intentional approach? If so, we will need tested-by tags from folks having 8350/8450/8550/sc8280x,qcm2290? I am only owning the visual validation on sc7280 atm. I'm not quite sure what is your intent here. Are there any SoCs after 845 that do not have SmartDMA 2.5? Or do you propose to enable SmartDMA only for the chipsets that we can visually test? That sounds strange. Yes I was thinking to enable smartDMA at the moment on chipsets which we can validate visually that display comes up. But I am not sure if thats entirely practical. But the intent was I just want to make sure basic display does come up with smartDMA enabled if we are enabling it for all chipsets. I don't think it is practical or logical. We don't require validating other changes on all possible chipsets, so what is so different with this one? Thats because with smartDMA if the programming of stages goes wrong we could potentially just see a blank screen. Its not about other changes, this change in particular controls enabling a feature. But thats just my thought. I am not going to request to ensure this or block this for this. You can still have my Reviewed-by: Abhinav Kumar But think of the validations that have to be done before we merge it. The usual way: verify as much as feasible and let anybody else complain during the development cycle. Well, our perspective is to enable the feature on devices on which you are able to test and not enable then wait for others to complain. This would not be really practical. There are plenty of people who can test things on obscure platforms, but unfortunately far less amount of people who tightly follow the development and can track which new feature applies to a particular platform. I hope to be able to fix that slightly with the hw catalog rework. However enabling features on other platforms definitely requires more knowledge than simply testing the kernel. I did not say test all devices. My point was to enable smartDMA on devices which we are able to test. There are other examples of this, like inline rotation, writeback etc. which are at the moment enabled only on devices which QC or others have tested on. But at the time it was added, inline rotation 2.0 could only be supported on
Re: [PATCH v3 26/27] drm/msm/dpu: split pipe handling from _dpu_crtc_blend_setup_mixer
On 09/02/2023 01:44, Abhinav Kumar wrote: On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote: Rework _dpu_crtc_blend_setup_mixer() to split away pipe handling to a separate functon. This is a preparation for the r_pipe support. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 86 --- drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 10 ++- 2 files changed, 63 insertions(+), 33 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 73e1a8c69ef0..0ca3bc38ff7e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -400,6 +400,47 @@ static void _dpu_crtc_program_lm_output_roi(struct drm_crtc *crtc) } } +static void _dpu_crtc_blend_setup_pipe(struct drm_crtc *crtc, + struct drm_plane *plane, + struct dpu_crtc_mixer *mixer, + u32 num_mixers, + struct dpu_hw_stage_cfg *stage_cfg, + enum dpu_stage stage, + unsigned int stage_idx, + unsigned long *fetch_active, + struct dpu_sw_pipe *pipe + ) +{ + uint32_t lm_idx; + enum dpu_sspp sspp_idx; + struct drm_plane_state *state; + + if (!pipe->sspp) + return; + + sspp_idx = pipe->sspp->idx; + + state = plane->state; + + DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n", + crtc->base.id, + stage, + plane->base.id, + sspp_idx - SSPP_NONE, + state->fb ? state->fb->base.id : -1); + + set_bit(sspp_idx, fetch_active); + + stage_cfg->stage[stage][stage_idx] = sspp_idx; + stage_cfg->multirect_index[stage][stage_idx] = + pipe->multirect_index; + + /* blend config update */ + for (lm_idx = 0; lm_idx < num_mixers; lm_idx++) + mixer[lm_idx].lm_ctl->ops.update_pending_flush_sspp(mixer[lm_idx].lm_ctl, + sspp_idx); If you just pass the format to this function you can move rest of the for loop also to this function. Also, you will be able to add the trace_dpu_crtc_setup_mixer() with complete information. trace_dpu_crtc_setup_mixer is currently missing te stage_idx which is important to debug blend issues. Ack, I'll add it back, thanks for the note. +} + static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, struct dpu_crtc *dpu_crtc, struct dpu_crtc_mixer *mixer, struct dpu_hw_stage_cfg *stage_cfg) @@ -412,15 +453,12 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, struct dpu_format *format; struct dpu_hw_ctl *ctl = mixer->lm_ctl; - uint32_t stage_idx, lm_idx; - int zpos_cnt[DPU_STAGE_MAX + 1] = { 0 }; + uint32_t lm_idx; bool bg_alpha_enable = false; DECLARE_BITMAP(fetch_active, SSPP_MAX); memset(fetch_active, 0, sizeof(fetch_active)); drm_atomic_crtc_for_each_plane(plane, crtc) { - enum dpu_sspp sspp_idx; - state = plane->state; if (!state) continue; @@ -431,39 +469,25 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, pstate = to_dpu_plane_state(state); fb = state->fb; - sspp_idx = pstate->pipe.sspp->idx; - set_bit(sspp_idx, fetch_active); - - DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n", - crtc->base.id, - pstate->stage, - plane->base.id, - sspp_idx - SSPP_VIG0, - state->fb ? state->fb->base.id : -1); - format = to_dpu_format(msm_framebuffer_format(pstate->base.fb)); if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable) bg_alpha_enable = true; - stage_idx = zpos_cnt[pstate->stage]++; - stage_cfg->stage[pstate->stage][stage_idx] = - sspp_idx; - stage_cfg->multirect_index[pstate->stage][stage_idx] = - pstate->pipe.multirect_index; - trace_dpu_crtc_setup_mixer(DRMID(crtc), DRMID(plane), - state, pstate, stage_idx, + state, pstate, format->base.pixel_format, fb ? fb->modifier : 0); + _dpu_crtc_blend_setup_pipe(crtc, plane, + mixer, cstate->num_mixers, + stage_cfg, pstate->stage, 0, + fetch_active, + >pipe); + /* blend config update */ for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++) { - _dpu_crtc_setup_blend_cfg(mixer + lm_idx, - pstate, format); - - mixer[lm_idx].lm_ctl->ops.update_pending_flush_sspp(mixer[lm_idx].lm_ctl, - sspp_idx); +
Re: [PATCH v3 26/27] drm/msm/dpu: split pipe handling from _dpu_crtc_blend_setup_mixer
On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote: Rework _dpu_crtc_blend_setup_mixer() to split away pipe handling to a separate functon. This is a preparation for the r_pipe support. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 86 --- drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 10 ++- 2 files changed, 63 insertions(+), 33 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 73e1a8c69ef0..0ca3bc38ff7e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -400,6 +400,47 @@ static void _dpu_crtc_program_lm_output_roi(struct drm_crtc *crtc) } } +static void _dpu_crtc_blend_setup_pipe(struct drm_crtc *crtc, + struct drm_plane *plane, + struct dpu_crtc_mixer *mixer, + u32 num_mixers, + struct dpu_hw_stage_cfg *stage_cfg, + enum dpu_stage stage, + unsigned int stage_idx, + unsigned long *fetch_active, + struct dpu_sw_pipe *pipe + ) +{ + uint32_t lm_idx; + enum dpu_sspp sspp_idx; + struct drm_plane_state *state; + + if (!pipe->sspp) + return; + + sspp_idx = pipe->sspp->idx; + + state = plane->state; + + DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n", +crtc->base.id, +stage, +plane->base.id, +sspp_idx - SSPP_NONE, +state->fb ? state->fb->base.id : -1); + + set_bit(sspp_idx, fetch_active); + + stage_cfg->stage[stage][stage_idx] = sspp_idx; + stage_cfg->multirect_index[stage][stage_idx] = + pipe->multirect_index; + + /* blend config update */ + for (lm_idx = 0; lm_idx < num_mixers; lm_idx++) + mixer[lm_idx].lm_ctl->ops.update_pending_flush_sspp(mixer[lm_idx].lm_ctl, + sspp_idx); If you just pass the format to this function you can move rest of the for loop also to this function. Also, you will be able to add the trace_dpu_crtc_setup_mixer() with complete information. trace_dpu_crtc_setup_mixer is currently missing te stage_idx which is important to debug blend issues. +} + static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, struct dpu_crtc *dpu_crtc, struct dpu_crtc_mixer *mixer, struct dpu_hw_stage_cfg *stage_cfg) @@ -412,15 +453,12 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, struct dpu_format *format; struct dpu_hw_ctl *ctl = mixer->lm_ctl; - uint32_t stage_idx, lm_idx; - int zpos_cnt[DPU_STAGE_MAX + 1] = { 0 }; + uint32_t lm_idx; bool bg_alpha_enable = false; DECLARE_BITMAP(fetch_active, SSPP_MAX); memset(fetch_active, 0, sizeof(fetch_active)); drm_atomic_crtc_for_each_plane(plane, crtc) { - enum dpu_sspp sspp_idx; - state = plane->state; if (!state) continue; @@ -431,39 +469,25 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, pstate = to_dpu_plane_state(state); fb = state->fb; - sspp_idx = pstate->pipe.sspp->idx; - set_bit(sspp_idx, fetch_active); - - DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n", - crtc->base.id, - pstate->stage, - plane->base.id, - sspp_idx - SSPP_VIG0, - state->fb ? state->fb->base.id : -1); - format = to_dpu_format(msm_framebuffer_format(pstate->base.fb)); if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable) bg_alpha_enable = true; - stage_idx = zpos_cnt[pstate->stage]++; - stage_cfg->stage[pstate->stage][stage_idx] = - sspp_idx; - stage_cfg->multirect_index[pstate->stage][stage_idx] = - pstate->pipe.multirect_index; - trace_dpu_crtc_setup_mixer(DRMID(crtc), DRMID(plane), - state, pstate, stage_idx, + state, pstate, format->base.pixel_format, fb ? fb->modifier : 0); + _dpu_crtc_blend_setup_pipe(crtc, plane, + mixer,
Re: [RFC PATCH] drm/msm/dpu: Move TE setup to prepare_for_kickoff()
On 09/02/2023 01:24, Jessica Zhang wrote: On 2/8/2023 2:18 PM, Dmitry Baryshkov wrote: On 08/02/2023 23:37, Jessica Zhang wrote: Currently, DPU will enable TE during prepare_commit(). However, this will cause issues when trying to read/write to register in get_autorefresh_config(), because the core clock rates aren't set at that time. This used to work because phys_enc->hw_pp is only initialized in mode set [1], so the first prepare_commit() will return before any register read/write as hw_pp would be NULL. However, when we try to implement support for INTF TE, we will run into the clock issue described above as hw_intf will *not* be NULL on the first prepare_commit(). This is because the initialization of dpu_enc->hw_intf has been moved to dpu_encoder_setup() [2]. To avoid this issue, let's enable TE during prepare_for_kickoff() instead as the core clock rates are guaranteed to be set then. Depends on: "Implement tearcheck support on INTF block" [3] [1] https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L1109 [2] https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2339 [3] https://patchwork.freedesktop.org/series/112332/ Signed-off-by: Jessica Zhang --- .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 78 ++- 1 file changed, 43 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c index 279a0b7015ce..746250bce3d1 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c @@ -587,39 +587,6 @@ static void dpu_encoder_phys_cmd_destroy(struct dpu_encoder_phys *phys_enc) kfree(cmd_enc); } -static void dpu_encoder_phys_cmd_prepare_for_kickoff( - struct dpu_encoder_phys *phys_enc) -{ - struct dpu_encoder_phys_cmd *cmd_enc = - to_dpu_encoder_phys_cmd(phys_enc); - int ret; - - if (!phys_enc->hw_pp) { - DPU_ERROR("invalid encoder\n"); - return; - } - DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n", DRMID(phys_enc->parent), - phys_enc->hw_pp->idx - PINGPONG_0, - atomic_read(_enc->pending_kickoff_cnt)); - - /* - * Mark kickoff request as outstanding. If there are more than one, - * outstanding, then we have to wait for the previous one to complete - */ - ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc); - if (ret) { - /* force pending_kickoff_cnt 0 to discard failed kickoff */ - atomic_set(_enc->pending_kickoff_cnt, 0); - DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n", - DRMID(phys_enc->parent), ret, - phys_enc->hw_pp->idx - PINGPONG_0); - } - - DPU_DEBUG_CMDENC(cmd_enc, "pp:%d pending_cnt %d\n", - phys_enc->hw_pp->idx - PINGPONG_0, - atomic_read(_enc->pending_kickoff_cnt)); -} - static bool dpu_encoder_phys_cmd_is_ongoing_pptx( struct dpu_encoder_phys *phys_enc) { @@ -645,8 +612,7 @@ static bool dpu_encoder_phys_cmd_is_ongoing_pptx( return false; } -static void dpu_encoder_phys_cmd_prepare_commit( - struct dpu_encoder_phys *phys_enc) +static void dpu_encoder_phys_cmd_enable_te(struct dpu_encoder_phys *phys_enc) { struct dpu_encoder_phys_cmd *cmd_enc = to_dpu_encoder_phys_cmd(phys_enc); @@ -704,6 +670,48 @@ static void dpu_encoder_phys_cmd_prepare_commit( "disabled autorefresh\n"); } +static void dpu_encoder_phys_cmd_prepare_for_kickoff( + struct dpu_encoder_phys *phys_enc) Could you please move the function back to the place, so that we can see the actual difference? Hi Dmitry, This function was moved because prepare_commit() and is_ongoing_pptx() (which is called in prepare_commit()) were originally defined later in the file. +{ + struct dpu_encoder_phys_cmd *cmd_enc = + to_dpu_encoder_phys_cmd(phys_enc); + int ret; + + if (!phys_enc->hw_pp) { + DPU_ERROR("invalid encoder\n"); + return; + } + + + DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n", DRMID(phys_enc->parent), + phys_enc->hw_pp->idx - PINGPONG_0, + atomic_read(_enc->pending_kickoff_cnt)); + + /* + * Mark kickoff request as outstanding. If there are more than one, + * outstanding, then we have to wait for the previous one to complete + */ + ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc); + if (ret) { + /* force pending_kickoff_cnt 0 to discard failed kickoff */ + atomic_set(_enc->pending_kickoff_cnt, 0); + DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n", + DRMID(phys_enc->parent), ret, + phys_enc->hw_pp->idx - PINGPONG_0); + } + + dpu_encoder_phys_cmd_enable_te(phys_enc); + + DPU_DEBUG_CMDENC(cmd_enc, "pp:%d pending_cnt %d\n", +
Re: [RFC PATCH] drm/msm/dpu: Move TE setup to prepare_for_kickoff()
On 2/8/2023 2:18 PM, Dmitry Baryshkov wrote: On 08/02/2023 23:37, Jessica Zhang wrote: Currently, DPU will enable TE during prepare_commit(). However, this will cause issues when trying to read/write to register in get_autorefresh_config(), because the core clock rates aren't set at that time. This used to work because phys_enc->hw_pp is only initialized in mode set [1], so the first prepare_commit() will return before any register read/write as hw_pp would be NULL. However, when we try to implement support for INTF TE, we will run into the clock issue described above as hw_intf will *not* be NULL on the first prepare_commit(). This is because the initialization of dpu_enc->hw_intf has been moved to dpu_encoder_setup() [2]. To avoid this issue, let's enable TE during prepare_for_kickoff() instead as the core clock rates are guaranteed to be set then. Depends on: "Implement tearcheck support on INTF block" [3] [1] https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L1109 [2] https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2339 [3] https://patchwork.freedesktop.org/series/112332/ Signed-off-by: Jessica Zhang --- .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 78 ++- 1 file changed, 43 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c index 279a0b7015ce..746250bce3d1 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c @@ -587,39 +587,6 @@ static void dpu_encoder_phys_cmd_destroy(struct dpu_encoder_phys *phys_enc) kfree(cmd_enc); } -static void dpu_encoder_phys_cmd_prepare_for_kickoff( - struct dpu_encoder_phys *phys_enc) -{ - struct dpu_encoder_phys_cmd *cmd_enc = - to_dpu_encoder_phys_cmd(phys_enc); - int ret; - - if (!phys_enc->hw_pp) { - DPU_ERROR("invalid encoder\n"); - return; - } - DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n", DRMID(phys_enc->parent), - phys_enc->hw_pp->idx - PINGPONG_0, - atomic_read(_enc->pending_kickoff_cnt)); - - /* - * Mark kickoff request as outstanding. If there are more than one, - * outstanding, then we have to wait for the previous one to complete - */ - ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc); - if (ret) { - /* force pending_kickoff_cnt 0 to discard failed kickoff */ - atomic_set(_enc->pending_kickoff_cnt, 0); - DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n", - DRMID(phys_enc->parent), ret, - phys_enc->hw_pp->idx - PINGPONG_0); - } - - DPU_DEBUG_CMDENC(cmd_enc, "pp:%d pending_cnt %d\n", - phys_enc->hw_pp->idx - PINGPONG_0, - atomic_read(_enc->pending_kickoff_cnt)); -} - static bool dpu_encoder_phys_cmd_is_ongoing_pptx( struct dpu_encoder_phys *phys_enc) { @@ -645,8 +612,7 @@ static bool dpu_encoder_phys_cmd_is_ongoing_pptx( return false; } -static void dpu_encoder_phys_cmd_prepare_commit( - struct dpu_encoder_phys *phys_enc) +static void dpu_encoder_phys_cmd_enable_te(struct dpu_encoder_phys *phys_enc) { struct dpu_encoder_phys_cmd *cmd_enc = to_dpu_encoder_phys_cmd(phys_enc); @@ -704,6 +670,48 @@ static void dpu_encoder_phys_cmd_prepare_commit( "disabled autorefresh\n"); } +static void dpu_encoder_phys_cmd_prepare_for_kickoff( + struct dpu_encoder_phys *phys_enc) Could you please move the function back to the place, so that we can see the actual difference? Hi Dmitry, This function was moved because prepare_commit() and is_ongoing_pptx() (which is called in prepare_commit()) were originally defined later in the file. +{ + struct dpu_encoder_phys_cmd *cmd_enc = + to_dpu_encoder_phys_cmd(phys_enc); + int ret; + + if (!phys_enc->hw_pp) { + DPU_ERROR("invalid encoder\n"); + return; + } + + + DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n", DRMID(phys_enc->parent), + phys_enc->hw_pp->idx - PINGPONG_0, + atomic_read(_enc->pending_kickoff_cnt)); + + /* + * Mark kickoff request as outstanding. If there are more than one, + * outstanding, then we have to wait for the previous one to complete + */ + ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc); + if (ret) { + /* force pending_kickoff_cnt 0 to discard failed kickoff */ + atomic_set(_enc->pending_kickoff_cnt, 0); + DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n", + DRMID(phys_enc->parent), ret, + phys_enc->hw_pp->idx - PINGPONG_0); + } + + dpu_encoder_phys_cmd_enable_te(phys_enc); + + DPU_DEBUG_CMDENC(cmd_enc, "pp:%d pending_cnt %d\n", + phys_enc->hw_pp->idx - PINGPONG_0, +
Re: [PATCH v3 17/27] drm/msm/dpu: rewrite plane's QoS-related functions to take dpu_sw_pipe and dpu_format
On 2/3/2023 3:20 PM, Dmitry Baryshkov wrote: On 04/02/2023 01:07, Abhinav Kumar wrote: On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote: Rewrite dpu_plane's QoS related functions to take struct dpu_sw_pipe and struct dpu_format as arguments rather than fetching them from the pstate or drm_framebuffer. Signed-off-by: Dmitry Baryshkov Nothing wrong with the change as such but why is this needed? I looked through tne next patches in the series briefly and unless I am missing something, I am not able to see how this rewrite is helping or needed for the remaining patches. Having a separate pipe argument eases adding support for r_pipe. After all these changes only upper level functions access pstate->pipe. Then it becomes natural to do: dpu_plane_do_something(plane->pipe); if (plane->r_pipe) dpu_plane_do_something(plane->r_pipe); Understood, Reviewed-by: Abhinav Kumar
Re: [PATCH] drm/vmwgfx: Stop accessing buffer objects which failed init
On 2/8/23 10:00, Zack Rusin wrote: > From: Zack Rusin > > ttm_bo_init_reserved on failure puts the buffer object back which > causes it to be deleted, but kfree was still being called on the same > buffer in vmw_bo_create leading to a double free. > > After the double free the vmw_gem_object_create_with_handle was > setting the gem function objects before checking the return status > of vmw_bo_create leading to null pointer access. > > Fix the entire path by relaying on ttm_bo_init_reserved to delete the > buffer objects on failure and making sure the return status is checked > before setting the gem function objects on the buffer object. > > Signed-off-by: Zack Rusin > Fixes: 8afa13a0583f ("drm/vmwgfx: Implement DRIVER_GEM") > Cc: # v5.17+ > --- > drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 4 +++- > drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 4 ++-- > 2 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > index 63486802c8fd..43ffa5c7acbd 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > @@ -431,13 +431,15 @@ int vmw_bo_create(struct vmw_private *vmw, > return -ENOMEM; > } > > + /* > + * vmw_bo_init will delete the *p_bo object if it fails > + */ > ret = vmw_bo_init(vmw, *p_bo, params, vmw_bo_free); > if (unlikely(ret != 0)) > goto out_error; > > return ret; > out_error: > - kfree(*p_bo); > *p_bo = NULL; > return ret; > } > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c > index f042e22b8b59..51bd1f8c5cc4 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c > @@ -127,11 +127,11 @@ int vmw_gem_object_create_with_handle(struct > vmw_private *dev_priv, > }; > > ret = vmw_bo_create(dev_priv, , p_vbo); > - > - (*p_vbo)->tbo.base.funcs = _gem_object_funcs; > if (ret != 0) > goto out_no_bo; > > + (*p_vbo)->tbo.base.funcs = _gem_object_funcs; > + > ret = drm_gem_handle_create(filp, &(*p_vbo)->tbo.base, handle); > /* drop reference from allocate - handle holds it now */ > drm_gem_object_put(&(*p_vbo)->tbo.base); LGTM! Reviewed-by: Maaz Mombasawala -- Maaz Mombasawala (VMware)
Re: [PATCH] drm/vmwgfx: Do not drop the reference to the handle too soon
On 2/8/23 13:53, Zack Rusin wrote: > From: Zack Rusin > > It is possible for userspace to predict the next buffer handle and > to destroy the buffer while it's still used by the kernel. Delay > dropping the internal reference on the buffers until kernel is done > with them. > > Signed-off-by: Zack Rusin > Fixes: 8afa13a0583f ("drm/vmwgfx: Implement DRIVER_GEM") > Cc: # v5.17+ > --- > drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 3 ++- > drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 4 ++-- > drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 1 - > 3 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > index 43ffa5c7acbd..65bd88c8fef9 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > @@ -708,7 +708,8 @@ int vmw_dumb_create(struct drm_file *file_priv, > ret = vmw_gem_object_create_with_handle(dev_priv, file_priv, > args->size, >handle, > ); > - > + /* drop reference from allocate - handle holds it now */ > + drm_gem_object_put(>tbo.base); > return ret; > } > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c > index 51bd1f8c5cc4..d6baf73a6458 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c > @@ -133,8 +133,6 @@ int vmw_gem_object_create_with_handle(struct vmw_private > *dev_priv, > (*p_vbo)->tbo.base.funcs = _gem_object_funcs; > > ret = drm_gem_handle_create(filp, &(*p_vbo)->tbo.base, handle); > - /* drop reference from allocate - handle holds it now */ > - drm_gem_object_put(&(*p_vbo)->tbo.base); > out_no_bo: > return ret; > } > @@ -161,6 +159,8 @@ int vmw_gem_object_create_ioctl(struct drm_device *dev, > void *data, > rep->map_handle = drm_vma_node_offset_addr(>tbo.base.vma_node); > rep->cur_gmr_id = handle; > rep->cur_gmr_offset = 0; > + /* drop reference from allocate - handle holds it now */ > + drm_gem_object_put(>tbo.base); > out_no_bo: > return ret; > } > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c > index 9d4ae9623a00..d18fec953fa7 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c > @@ -867,7 +867,6 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void > *data, > goto out_unlock; > } > vmw_bo_reference(res->guest_memory_bo); > - drm_gem_object_get(>guest_memory_bo->tbo.base); > } > > tmp = vmw_resource_reference(>res); LGTM! Reviewed-by: Maaz Mombasawala -- Maaz Mombasawala (VMware)
Re: [PATCH 1/5] drm/sched: Create wrapper to add a syncobj dependency to job
On 2023-02-08 14:48, Maíra Canal wrote: > In order to add a syncobj's fence as a dependency to a job, it is > necessary to call drm_syncobj_find_fence() to find the fence and then > add the dependency with drm_sched_job_add_dependency(). So, wrap these > steps in one single function, drm_sched_job_add_syncobj_dependency(). Yes, that's a good change--thanks! Patch is: Reviewed-by: Luben Tuikov -- Regards, Luben
Re: [PATCH v3 25/27] drm/msm/dpu: rework static color fill code
On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote: Rework static color fill code to separate the pipe / pipe_cfg handling. This is a preparation for the r_pipe support. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 70 +-- 1 file changed, 41 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 05047192cb37..e2e85688ed3c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -639,20 +639,54 @@ static void _dpu_plane_setup_scaler(struct dpu_sw_pipe *pipe, fmt); } +static int _dpu_plane_color_fill_pipe(struct dpu_plane_state *pstate, + struct dpu_sw_pipe *pipe, + struct dpu_hw_sspp_cfg *old_pipe_cfg, Why is this called old_pipe_cfg instead of just pipe_cfg? + u32 fill_color, + const struct dpu_format *fmt) +{ + struct dpu_hw_sspp_cfg pipe_cfg; + + /* update sspp */ + if (!pipe->sspp->ops.setup_solidfill) + return 0; You can just return from here and make this function void? + + pipe->sspp->ops.setup_solidfill(pipe, fill_color); + + /* override scaler/decimation if solid fill */ + pipe_cfg.dst_rect = old_pipe_cfg->dst_rect; + + pipe_cfg.src_rect.x1 = 0; + pipe_cfg.src_rect.y1 = 0; + pipe_cfg.src_rect.x2 = + drm_rect_width(_cfg.dst_rect); + pipe_cfg.src_rect.y2 = + drm_rect_height(_cfg.dst_rect); + + if (pipe->sspp->ops.setup_format) + pipe->sspp->ops.setup_format(pipe, fmt, DPU_SSPP_SOLID_FILL); + + if (pipe->sspp->ops.setup_rects) + pipe->sspp->ops.setup_rects(pipe, _cfg); + + _dpu_plane_setup_scaler(pipe, fmt, true, _cfg, pstate->rotation); + + return 0; +} + /** * _dpu_plane_color_fill - enables color fill on plane * @pdpu: Pointer to DPU plane object * @color: RGB fill color value, [23..16] Blue, [15..8] Green, [7..0] Red * @alpha: 8-bit fill alpha value, 255 selects 100% alpha - * Returns: 0 on success */ -static int _dpu_plane_color_fill(struct dpu_plane *pdpu, +static void _dpu_plane_color_fill(struct dpu_plane *pdpu, uint32_t color, uint32_t alpha) { const struct dpu_format *fmt; const struct drm_plane *plane = >base; struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state); - struct dpu_hw_sspp_cfg pipe_cfg; + u32 fill_color = (color & 0xFF) | ((alpha & 0xFF) << 24); DPU_DEBUG_PLANE(pdpu, "\n"); @@ -661,34 +695,12 @@ static int _dpu_plane_color_fill(struct dpu_plane *pdpu, * h/w only supports RGB variants */ fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR); + /* should not happen ever */ + if (!fmt) + return; /* update sspp */ - if (fmt && pstate->pipe.sspp->ops.setup_solidfill) { - pstate->pipe.sspp->ops.setup_solidfill(>pipe, - (color & 0xFF) | ((alpha & 0xFF) << 24)); - - /* override scaler/decimation if solid fill */ - pipe_cfg.dst_rect = pstate->base.dst; - - pipe_cfg.src_rect.x1 = 0; - pipe_cfg.src_rect.y1 = 0; - pipe_cfg.src_rect.x2 = - drm_rect_width(_cfg.dst_rect); - pipe_cfg.src_rect.y2 = - drm_rect_height(_cfg.dst_rect); - - if (pstate->pipe.sspp->ops.setup_format) - pstate->pipe.sspp->ops.setup_format(>pipe, - fmt, DPU_SSPP_SOLID_FILL); - - if (pstate->pipe.sspp->ops.setup_rects) - pstate->pipe.sspp->ops.setup_rects(>pipe, - _cfg); - - _dpu_plane_setup_scaler(>pipe, fmt, true, _cfg, pstate->rotation); - } - - return 0; + _dpu_plane_color_fill_pipe(pstate, >pipe, >pipe_cfg, fill_color, fmt); } int dpu_plane_validate_multirect_v2(struct dpu_multirect_plane_states *plane)
Re: [PATCH 1/5] drm/sched: Create wrapper to add a syncobj dependency to job
On 2023-02-08 14:54, Christian König wrote: > Am 08.02.23 um 20:48 schrieb Maíra Canal: >> In order to add a syncobj's fence as a dependency to a job, it is >> necessary to call drm_syncobj_find_fence() to find the fence and then >> add the dependency with drm_sched_job_add_dependency(). So, wrap these >> steps in one single function, drm_sched_job_add_syncobj_dependency(). >> >> Signed-off-by: Maíra Canal > Just one nit pick below, with that fixed Reviewed-by: Christian König > > > I'm pretty sure we have the exact same function now in amdgpu cause I > cleaned that up just a few weeks ago to look the same as the other drivers. > > Would be nice to have that new function applied there as well. Hi Christian, Is that R-B for the series or just this patch? -- Regards, Luben
Re: [RFC PATCH] drm/msm/dpu: Move TE setup to prepare_for_kickoff()
On 08/02/2023 23:37, Jessica Zhang wrote: Currently, DPU will enable TE during prepare_commit(). However, this will cause issues when trying to read/write to register in get_autorefresh_config(), because the core clock rates aren't set at that time. This used to work because phys_enc->hw_pp is only initialized in mode set [1], so the first prepare_commit() will return before any register read/write as hw_pp would be NULL. However, when we try to implement support for INTF TE, we will run into the clock issue described above as hw_intf will *not* be NULL on the first prepare_commit(). This is because the initialization of dpu_enc->hw_intf has been moved to dpu_encoder_setup() [2]. To avoid this issue, let's enable TE during prepare_for_kickoff() instead as the core clock rates are guaranteed to be set then. Depends on: "Implement tearcheck support on INTF block" [3] [1] https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L1109 [2] https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2339 [3] https://patchwork.freedesktop.org/series/112332/ Signed-off-by: Jessica Zhang --- .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 78 ++- 1 file changed, 43 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c index 279a0b7015ce..746250bce3d1 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c @@ -587,39 +587,6 @@ static void dpu_encoder_phys_cmd_destroy(struct dpu_encoder_phys *phys_enc) kfree(cmd_enc); } -static void dpu_encoder_phys_cmd_prepare_for_kickoff( - struct dpu_encoder_phys *phys_enc) -{ - struct dpu_encoder_phys_cmd *cmd_enc = - to_dpu_encoder_phys_cmd(phys_enc); - int ret; - - if (!phys_enc->hw_pp) { - DPU_ERROR("invalid encoder\n"); - return; - } - DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n", DRMID(phys_enc->parent), - phys_enc->hw_pp->idx - PINGPONG_0, - atomic_read(_enc->pending_kickoff_cnt)); - - /* -* Mark kickoff request as outstanding. If there are more than one, -* outstanding, then we have to wait for the previous one to complete -*/ - ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc); - if (ret) { - /* force pending_kickoff_cnt 0 to discard failed kickoff */ - atomic_set(_enc->pending_kickoff_cnt, 0); - DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n", - DRMID(phys_enc->parent), ret, - phys_enc->hw_pp->idx - PINGPONG_0); - } - - DPU_DEBUG_CMDENC(cmd_enc, "pp:%d pending_cnt %d\n", - phys_enc->hw_pp->idx - PINGPONG_0, - atomic_read(_enc->pending_kickoff_cnt)); -} - static bool dpu_encoder_phys_cmd_is_ongoing_pptx( struct dpu_encoder_phys *phys_enc) { @@ -645,8 +612,7 @@ static bool dpu_encoder_phys_cmd_is_ongoing_pptx( return false; } -static void dpu_encoder_phys_cmd_prepare_commit( - struct dpu_encoder_phys *phys_enc) +static void dpu_encoder_phys_cmd_enable_te(struct dpu_encoder_phys *phys_enc) { struct dpu_encoder_phys_cmd *cmd_enc = to_dpu_encoder_phys_cmd(phys_enc); @@ -704,6 +670,48 @@ static void dpu_encoder_phys_cmd_prepare_commit( "disabled autorefresh\n"); } +static void dpu_encoder_phys_cmd_prepare_for_kickoff( + struct dpu_encoder_phys *phys_enc) Could you please move the function back to the place, so that we can see the actual difference? +{ + struct dpu_encoder_phys_cmd *cmd_enc = + to_dpu_encoder_phys_cmd(phys_enc); + int ret; + + if (!phys_enc->hw_pp) { + DPU_ERROR("invalid encoder\n"); + return; + } + + + DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n", DRMID(phys_enc->parent), + phys_enc->hw_pp->idx - PINGPONG_0, + atomic_read(_enc->pending_kickoff_cnt)); + + /* +* Mark kickoff request as outstanding. If there are more than one, +* outstanding, then we have to wait for the previous one to complete +*/ + ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc); + if (ret) { + /* force pending_kickoff_cnt 0 to discard failed kickoff */ + atomic_set(_enc->pending_kickoff_cnt, 0); + DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n", + DRMID(phys_enc->parent), ret, + phys_enc->hw_pp->idx - PINGPONG_0); + } + + dpu_encoder_phys_cmd_enable_te(phys_enc); + +
Re: [PATCH v2 0/8] QAIC accel driver
On 2/6/2023 8:41 AM, Jeffrey Hugo wrote: Regarding the open userspace (see the documentation patch), the UMD and compiler are a week or so away from being posted in the indicated repos. Just need to polish some documentation. An update to this, the compiler is now live on github at the link specified in the documentation patch. -Jeff
Re: [PATCH v12 13/14] drm/msm/disp/dpu: add PSR support for eDP interface in dpu driver
On 07/02/2023 17:25, Dmitry Baryshkov wrote: On 07/02/2023 16:26, Vinod Polimera wrote: -Original Message- From: Dmitry Baryshkov Sent: Tuesday, January 31, 2023 6:29 PM To: Vinod Polimera (QUIC) ; dri- de...@lists.freedesktop.org; linux-arm-...@vger.kernel.org; freedr...@lists.freedesktop.org; devicet...@vger.kernel.org Cc: linux-ker...@vger.kernel.org; robdcl...@gmail.com; diand...@chromium.org; swb...@chromium.org; Kalyan Thota (QUIC) ; Kuogee Hsieh (QUIC) ; Vishnuvardhan Prodduturi (QUIC) ; Bjorn Andersson (QUIC) ; Abhinav Kumar (QUIC) ; Sankeerth Billakanti (QUIC) Subject: Re: [PATCH v12 13/14] drm/msm/disp/dpu: add PSR support for eDP interface in dpu driver On 30/01/2023 17:11, Vinod Polimera wrote: Enable PSR on eDP interface using drm self-refresh librabry. This patch uses a trigger from self-refresh library to enter/exit into PSR, when there are no updates from framework. Signed-off-by: Kalyan Thota Signed-off-by: Vinod Polimera Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 13 - drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 14 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 +- 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index f29a339..60e5984 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -21,6 +21,7 @@ #include #include #include +#include #include "dpu_kms.h" #include "dpu_hw_lm.h" @@ -1021,6 +1022,9 @@ static void dpu_crtc_disable(struct drm_crtc *crtc, DRM_DEBUG_KMS("crtc%d\n", crtc->base.id); + if (old_crtc_state->self_refresh_active) + return; + I have been looking at the crtc_needs_disable(). It explicitly mentions that 'We also need to run through the crtc_funcs->disable() function [..] if it's transitioning to self refresh mode...'. Don't we need to perform some cleanup here (like disabling the vblank irq handling, freeing the bandwidth, etc)? When self refresh active is enabled, then we will clean up irq handling and bandwidth etc. The above case is to handle display off commit triggered when we are in psr as all the resources are already cleaned up . we just need to do an early return. /* Disable/save vblank irq handling */ drm_crtc_vblank_off(crtc); @@ -1577,7 +1581,7 @@ struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane, { struct drm_crtc *crtc = NULL; struct dpu_crtc *dpu_crtc = NULL; - int i; + int i, ret; dpu_crtc = kzalloc(sizeof(*dpu_crtc), GFP_KERNEL); if (!dpu_crtc) @@ -1614,6 +1618,13 @@ struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane, /* initialize event handling */ spin_lock_init(_crtc->event_lock); + ret = drm_self_refresh_helper_init(crtc); + if (ret) { + DPU_ERROR("Failed to initialize %s with self-refresh helpers %d\n", + crtc->name, ret); + return ERR_PTR(ret); + } + DRM_DEBUG_KMS("%s: successfully initialized crtc\n", dpu_crtc- name); return crtc; } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 01b7509..450abb1 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -12,6 +12,7 @@ #include #include +#include #include #include #include @@ -1212,11 +1213,24 @@ static void dpu_encoder_virt_atomic_disable(struct drm_encoder *drm_enc, struct drm_atomic_state *state) { struct dpu_encoder_virt *dpu_enc = NULL; + struct drm_crtc *crtc; + struct drm_crtc_state *old_state = NULL; int i = 0; dpu_enc = to_dpu_encoder_virt(drm_enc); DPU_DEBUG_ENC(dpu_enc, "\n"); + crtc = drm_atomic_get_old_crtc_for_encoder(state, drm_enc); + if (crtc) + old_state = drm_atomic_get_old_crtc_state(state, crtc); + + /* + * The encoder is already disabled if self refresh mode was set earlier, + * in the old_state for the corresponding crtc. + */ + if (old_state && old_state->self_refresh_active) + return; + Again the same question here, doesn't crtc_needs_disable() take care of this clause? I might be missing something in the PSR state transitions. Could you please add some explanation here? Same usecase as above, applies to encoder disable also when triggered via disable commit When driver is in psr state. Ack, thank you for the explanations. I'd like to take another glance later today, but generally it look good to me. After another glance it still looks good to me. Please send the last iteration of the series: - moving all core patches to the first place, as it was asked previously. This will help
[PATCH] drm/vmwgfx: Do not drop the reference to the handle too soon
From: Zack Rusin It is possible for userspace to predict the next buffer handle and to destroy the buffer while it's still used by the kernel. Delay dropping the internal reference on the buffers until kernel is done with them. Signed-off-by: Zack Rusin Fixes: 8afa13a0583f ("drm/vmwgfx: Implement DRIVER_GEM") Cc: # v5.17+ --- drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 3 ++- drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 4 ++-- drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 1 - 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c index 43ffa5c7acbd..65bd88c8fef9 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c @@ -708,7 +708,8 @@ int vmw_dumb_create(struct drm_file *file_priv, ret = vmw_gem_object_create_with_handle(dev_priv, file_priv, args->size, >handle, ); - + /* drop reference from allocate - handle holds it now */ + drm_gem_object_put(>tbo.base); return ret; } diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c index 51bd1f8c5cc4..d6baf73a6458 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c @@ -133,8 +133,6 @@ int vmw_gem_object_create_with_handle(struct vmw_private *dev_priv, (*p_vbo)->tbo.base.funcs = _gem_object_funcs; ret = drm_gem_handle_create(filp, &(*p_vbo)->tbo.base, handle); - /* drop reference from allocate - handle holds it now */ - drm_gem_object_put(&(*p_vbo)->tbo.base); out_no_bo: return ret; } @@ -161,6 +159,8 @@ int vmw_gem_object_create_ioctl(struct drm_device *dev, void *data, rep->map_handle = drm_vma_node_offset_addr(>tbo.base.vma_node); rep->cur_gmr_id = handle; rep->cur_gmr_offset = 0; + /* drop reference from allocate - handle holds it now */ + drm_gem_object_put(>tbo.base); out_no_bo: return ret; } diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c index 9d4ae9623a00..d18fec953fa7 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c @@ -867,7 +867,6 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void *data, goto out_unlock; } vmw_bo_reference(res->guest_memory_bo); - drm_gem_object_get(>guest_memory_bo->tbo.base); } tmp = vmw_resource_reference(>res); -- 2.38.1
[RFC PATCH] drm/msm/dpu: Move TE setup to prepare_for_kickoff()
Currently, DPU will enable TE during prepare_commit(). However, this will cause issues when trying to read/write to register in get_autorefresh_config(), because the core clock rates aren't set at that time. This used to work because phys_enc->hw_pp is only initialized in mode set [1], so the first prepare_commit() will return before any register read/write as hw_pp would be NULL. However, when we try to implement support for INTF TE, we will run into the clock issue described above as hw_intf will *not* be NULL on the first prepare_commit(). This is because the initialization of dpu_enc->hw_intf has been moved to dpu_encoder_setup() [2]. To avoid this issue, let's enable TE during prepare_for_kickoff() instead as the core clock rates are guaranteed to be set then. Depends on: "Implement tearcheck support on INTF block" [3] [1] https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L1109 [2] https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2339 [3] https://patchwork.freedesktop.org/series/112332/ Signed-off-by: Jessica Zhang --- .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 78 ++- 1 file changed, 43 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c index 279a0b7015ce..746250bce3d1 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c @@ -587,39 +587,6 @@ static void dpu_encoder_phys_cmd_destroy(struct dpu_encoder_phys *phys_enc) kfree(cmd_enc); } -static void dpu_encoder_phys_cmd_prepare_for_kickoff( - struct dpu_encoder_phys *phys_enc) -{ - struct dpu_encoder_phys_cmd *cmd_enc = - to_dpu_encoder_phys_cmd(phys_enc); - int ret; - - if (!phys_enc->hw_pp) { - DPU_ERROR("invalid encoder\n"); - return; - } - DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n", DRMID(phys_enc->parent), - phys_enc->hw_pp->idx - PINGPONG_0, - atomic_read(_enc->pending_kickoff_cnt)); - - /* -* Mark kickoff request as outstanding. If there are more than one, -* outstanding, then we have to wait for the previous one to complete -*/ - ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc); - if (ret) { - /* force pending_kickoff_cnt 0 to discard failed kickoff */ - atomic_set(_enc->pending_kickoff_cnt, 0); - DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n", - DRMID(phys_enc->parent), ret, - phys_enc->hw_pp->idx - PINGPONG_0); - } - - DPU_DEBUG_CMDENC(cmd_enc, "pp:%d pending_cnt %d\n", - phys_enc->hw_pp->idx - PINGPONG_0, - atomic_read(_enc->pending_kickoff_cnt)); -} - static bool dpu_encoder_phys_cmd_is_ongoing_pptx( struct dpu_encoder_phys *phys_enc) { @@ -645,8 +612,7 @@ static bool dpu_encoder_phys_cmd_is_ongoing_pptx( return false; } -static void dpu_encoder_phys_cmd_prepare_commit( - struct dpu_encoder_phys *phys_enc) +static void dpu_encoder_phys_cmd_enable_te(struct dpu_encoder_phys *phys_enc) { struct dpu_encoder_phys_cmd *cmd_enc = to_dpu_encoder_phys_cmd(phys_enc); @@ -704,6 +670,48 @@ static void dpu_encoder_phys_cmd_prepare_commit( "disabled autorefresh\n"); } +static void dpu_encoder_phys_cmd_prepare_for_kickoff( + struct dpu_encoder_phys *phys_enc) +{ + struct dpu_encoder_phys_cmd *cmd_enc = + to_dpu_encoder_phys_cmd(phys_enc); + int ret; + + if (!phys_enc->hw_pp) { + DPU_ERROR("invalid encoder\n"); + return; + } + + + DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n", DRMID(phys_enc->parent), + phys_enc->hw_pp->idx - PINGPONG_0, + atomic_read(_enc->pending_kickoff_cnt)); + + /* +* Mark kickoff request as outstanding. If there are more than one, +* outstanding, then we have to wait for the previous one to complete +*/ + ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc); + if (ret) { + /* force pending_kickoff_cnt 0 to discard failed kickoff */ + atomic_set(_enc->pending_kickoff_cnt, 0); + DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n", + DRMID(phys_enc->parent), ret, + phys_enc->hw_pp->idx - PINGPONG_0); + } + + dpu_encoder_phys_cmd_enable_te(phys_enc); + + DPU_DEBUG_CMDENC(cmd_enc, "pp:%d pending_cnt %d\n", + phys_enc->hw_pp->idx - PINGPONG_0, +
Re: [Intel-gfx] [PATCH 1/2] drm: Introduce plane SIZE_HINTS property
On Wed, Feb 08, 2023 at 03:03:49PM +0200, Ville Syrjälä wrote: > On Wed, Feb 08, 2023 at 02:13:12PM +0200, Pekka Paalanen wrote: > > On Wed, 8 Feb 2023 06:09:10 +0200 > > Ville Syrjala wrote: > > > > > From: Ville Syrjälä > > > > > > Add a new immutable plane property by which a plane can advertise > > > a handful of recommended plane sizes. This would be mostly exposed > > > by cursor planes as a slightly more capable replacement for > > > the DRM_CAP_CURSOR_WIDTH/HEIGHT caps, which can only declare > > > a one size fits all limit for the whole device. > > > > > > Currently eg. amdgpu/i915/nouveau just advertize the max cursor > > > size via the cursor size caps. But always using the max sized > > > cursor can waste a surprising amount of power, so a better > > > stragey is desirable. > > > > > > Most other drivers don't specify any cursor size at all, in > > > which case the ioctl code just claims that 64x64 is a great > > > choice. Whether that is actually true is debatable. > > > > > > A poll of various compositor developers informs us that > > > blindly probing with setcursor/atomic ioctl to determine > > > suitable cursor sizes is not acceptable, thus the > > > introduction of the new property to supplant the cursor > > > size caps. The compositor will now be free to select a > > > more optimal cursor size from the short list of options. > > > > > > Note that the reported sizes (either via the property or the > > > caps) make no claims about things such as plane scaling. So > > > these things should only really be consulted for simple > > > "cursor like" use cases. > > > > > > Cc: Simon Ser > > > Cc: Jonas Ådahl > > > Cc: Daniel Stone > > > Cc: Pekka Paalanen > > > Signed-off-by: Ville Syrjälä > > > --- > > > drivers/gpu/drm/drm_mode_config.c | 7 +++ > > > drivers/gpu/drm/drm_plane.c | 33 +++ > > > include/drm/drm_mode_config.h | 5 + > > > include/drm/drm_plane.h | 4 > > > include/uapi/drm/drm_mode.h | 5 + > > > 5 files changed, 54 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_mode_config.c > > > b/drivers/gpu/drm/drm_mode_config.c > > > index 87eb591fe9b5..21860f94a18c 100644 > > > --- a/drivers/gpu/drm/drm_mode_config.c > > > +++ b/drivers/gpu/drm/drm_mode_config.c > > > @@ -374,6 +374,13 @@ static int > > > drm_mode_create_standard_properties(struct drm_device *dev) > > > return -ENOMEM; > > > dev->mode_config.modifiers_property = prop; > > > > > > + prop = drm_property_create(dev, > > > +DRM_MODE_PROP_IMMUTABLE | DRM_MODE_PROP_BLOB, > > > +"SIZE_HINTS", 0); > > > + if (!prop) > > > + return -ENOMEM; > > > + dev->mode_config.size_hints_property = prop; > > > + > > > return 0; > > > } > > > > > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > > > index 24e7998d1731..d0a277f4be1f 100644 > > > --- a/drivers/gpu/drm/drm_plane.c > > > +++ b/drivers/gpu/drm/drm_plane.c > > > @@ -1582,3 +1582,36 @@ int > > > drm_plane_create_scaling_filter_property(struct drm_plane *plane, > > > return 0; > > > } > > > EXPORT_SYMBOL(drm_plane_create_scaling_filter_property); > > > + > > > +/** > > > + * drm_plane_add_size_hint_property - create a size hint property > > > + * > > > + * @plane: drm plane > > > + * @hints: size hints > > > + * @num_hints: number of size hints > > > + * > > > + * Create a size hints property for the plane. > > > + * > > > + * RETURNS: > > > + * Zero for success or -errno > > > + */ > > > +int drm_plane_add_size_hints_property(struct drm_plane *plane, > > > + const struct drm_plane_size_hint *hints, > > > + int num_hints) > > > +{ > > > + struct drm_device *dev = plane->dev; > > > + struct drm_mode_config *config = >mode_config; > > > + struct drm_property_blob *blob; > > > + > > > + blob = drm_property_create_blob(dev, > > > + array_size(sizeof(hints[0]), num_hints), > > > + hints); > > > + if (IS_ERR(blob)) > > > + return PTR_ERR(blob); > > > + > > > + drm_object_attach_property(>base, config->size_hints_property, > > > +blob->base.id); > > > + > > > + return 0; > > > +} > > > +EXPORT_SYMBOL(drm_plane_add_size_hints_property); > > > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h > > > index e5b053001d22..ed9f6938dca1 100644 > > > --- a/include/drm/drm_mode_config.h > > > +++ b/include/drm/drm_mode_config.h > > > @@ -949,6 +949,11 @@ struct drm_mode_config { > > >*/ > > > struct drm_property *modifiers_property; > > > > > > + /** > > > + * @size_hints_property: Plane SIZE_HINTS property. > > > + */ > > > + struct drm_property *size_hints_property; > > > + > > > /* cursor size */ > > > uint32_t cursor_width, cursor_height; > > > > > > diff --git
[PATCH v2 1/2] drm: Introduce plane SIZE_HINTS property
From: Ville Syrjälä Add a new immutable plane property by which a plane can advertise a handful of recommended plane sizes. This would be mostly exposed by cursor planes as a slightly more capable replacement for the DRM_CAP_CURSOR_WIDTH/HEIGHT caps, which can only declare a one size fits all limit for the whole device. Currently eg. amdgpu/i915/nouveau just advertize the max cursor size via the cursor size caps. But always using the max sized cursor can waste a surprising amount of power, so a better stragey is desirable. Most other drivers don't specify any cursor size at all, in which case the ioctl code just claims that 64x64 is a great choice. Whether that is actually true is debatable. A poll of various compositor developers informs us that blindly probing with setcursor/atomic ioctl to determine suitable cursor sizes is not acceptable, thus the introduction of the new property to supplant the cursor size caps. The compositor will now be free to select a more optimal cursor size from the short list of options. Note that the reported sizes (either via the property or the caps) make no claims about things such as plane scaling. So these things should only really be consulted for simple "cursor like" use cases. v2: Try to add some docs Cc: Simon Ser Cc: Jonas Ådahl Cc: Daniel Stone Cc: Pekka Paalanen Acked-by: Harry Wentland Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_mode_config.c | 7 + drivers/gpu/drm/drm_plane.c | 48 +++ include/drm/drm_mode_config.h | 5 include/drm/drm_plane.h | 4 +++ include/uapi/drm/drm_mode.h | 11 +++ 5 files changed, 75 insertions(+) diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index 87eb591fe9b5..21860f94a18c 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -374,6 +374,13 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.modifiers_property = prop; + prop = drm_property_create(dev, + DRM_MODE_PROP_IMMUTABLE | DRM_MODE_PROP_BLOB, + "SIZE_HINTS", 0); + if (!prop) + return -ENOMEM; + dev->mode_config.size_hints_property = prop; + return 0; } diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index 24e7998d1731..ae51b1f83755 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -140,6 +140,21 @@ * DRM_FORMAT_MOD_LINEAR. Before linux kernel release v5.1 there have been * various bugs in this area with inconsistencies between the capability * flag and per-plane properties. + * + * SIZE_HINTS: + * Blob property which contains the set of recommended plane size + * which can used for simple "cursor like" use cases (eg. no scaling). + * Using these hints frees userspace from extensive probing of + * supported plane sizes through atomic/setcursor ioctls. + * + * For optimal usage userspace should pick the smallest size + * that satisfies its own requirements. + * + * The blob contains an array of struct drm_plane_size_hint. + * + * Drivers should only attach this property to planes that + * support a very limited set of sizes (eg. cursor planes + * on typical hardware). */ static unsigned int drm_num_planes(struct drm_device *dev) @@ -1582,3 +1597,36 @@ int drm_plane_create_scaling_filter_property(struct drm_plane *plane, return 0; } EXPORT_SYMBOL(drm_plane_create_scaling_filter_property); + +/** + * drm_plane_add_size_hint_property - create a size hint property + * + * @plane: drm plane + * @hints: size hints + * @num_hints: number of size hints + * + * Create a size hints property for the plane. + * + * RETURNS: + * Zero for success or -errno + */ +int drm_plane_add_size_hints_property(struct drm_plane *plane, + const struct drm_plane_size_hint *hints, + int num_hints) +{ + struct drm_device *dev = plane->dev; + struct drm_mode_config *config = >mode_config; + struct drm_property_blob *blob; + + blob = drm_property_create_blob(dev, + array_size(sizeof(hints[0]), num_hints), + hints); + if (IS_ERR(blob)) + return PTR_ERR(blob); + + drm_object_attach_property(>base, config->size_hints_property, + blob->base.id); + + return 0; +} +EXPORT_SYMBOL(drm_plane_add_size_hints_property); diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index e5b053001d22..5bc8aed9b445 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -949,6 +949,11 @@ struct drm_mode_config { */ struct drm_property *modifiers_property; +
Re: [PATCH] drm/amd/display: don't call dc_interrupt_set() for disabled crtcs
On 2/8/23 15:01, Hamza Mahfooz wrote: > As made mention of in commit 4ea7fc09539b ("drm/amd/display: Do not > program interrupt status on disabled crtc"), we shouldn't program > disabled crtcs. So, filter out disabled crtcs in dm_set_vupdate_irq() > and dm_set_vblank(). > > Fixes: 589d2739332d ("drm/amd/display: Use crtc enable/disable_vblank hooks") > Fixes: d2574c33bb71 ("drm/amd/display: In VRR mode, do DRM core vblank > handling at end of vblank. (v2)") > Signed-off-by: Hamza Mahfooz Reviewed-by: Harry Wentland Harry > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c > index 1e39d0939700..dc4f37240beb 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c > @@ -77,6 +77,9 @@ int dm_set_vupdate_irq(struct drm_crtc *crtc, bool enable) > struct amdgpu_device *adev = drm_to_adev(crtc->dev); > int rc; > > + if (acrtc->otg_inst == -1) > + return 0; > + > irq_source = IRQ_TYPE_VUPDATE + acrtc->otg_inst; > > rc = dc_interrupt_set(adev->dm.dc, irq_source, enable) ? 0 : -EBUSY; > @@ -151,6 +154,9 @@ static inline int dm_set_vblank(struct drm_crtc *crtc, > bool enable) > struct vblank_control_work *work; > int rc = 0; > > + if (acrtc->otg_inst == -1) > + goto skip; > + > if (enable) { > /* vblank irq on -> Only need vupdate irq in vrr mode */ > if (amdgpu_dm_vrr_active(acrtc_state)) > @@ -168,6 +174,7 @@ static inline int dm_set_vblank(struct drm_crtc *crtc, > bool enable) > if (!dc_interrupt_set(adev->dm.dc, irq_source, enable)) > return -EBUSY; > > +skip: > if (amdgpu_in_reset(adev)) > return 0; >
Re: [PATCH 4/5] drm/panfrost: Use drm_sched_job_add_syncobj_dependency()
R-b, thanks On Wed, Feb 08, 2023 at 04:48:16PM -0300, Ma??ra Canal wrote: > As panfrost_copy_in_sync() performs the same steps as > drm_sched_job_add_syncobj_dependency(), replace the open-coded > implementation in Panfrost in order to simply, using the DRM function. > > Signed-off-by: Ma??ra Canal > --- > drivers/gpu/drm/panfrost/panfrost_drv.c | 11 ++- > 1 file changed, 2 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c > b/drivers/gpu/drm/panfrost/panfrost_drv.c > index abb0dadd8f63..f49096f53141 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -220,15 +220,8 @@ panfrost_copy_in_sync(struct drm_device *dev, > } > > for (i = 0; i < in_fence_count; i++) { > - struct dma_fence *fence; > - > - ret = drm_syncobj_find_fence(file_priv, handles[i], 0, 0, > - ); > - if (ret) > - goto fail; > - > - ret = drm_sched_job_add_dependency(>base, fence); > - > + ret = drm_sched_job_add_syncobj_dependency(>base, > file_priv, > +handles[i], 0); > if (ret) > goto fail; > } > -- > 2.39.1 >
Re: [PATCH] Revert "drm/i915/hwmon: Enable PL1 power limit"
On Wed, Feb 08, 2023 at 11:03:12AM -0800, Ashutosh Dixit wrote: > This reverts commit 0349c41b05968befaffa5fbb7e73d0ee6004f610. > > 0349c41b0596 ("drm/i915/hwmon: Enable PL1 power limit") is incorrect and > caused a major regression on ATSM. The change enabled the PL1 power limit > but FW sets the default value of the PL1 limit to 0 which implies HW now > works at minimum power and therefore the lowest effective frequency. This > means all workloads now run slower resulting in even GuC FW load operations > timing out, rendering ATSM unusable. > > A different solution to the original issue of the PL1 limit being disabled > on ATSM is needed but till that is developed, revert 0349c41b0596. > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8062 pushed to drm-intel-next and removed from drm-intel-fixes. Thanks for the quick reaction. > Signed-off-by: Ashutosh Dixit > --- > drivers/gpu/drm/i915/i915_hwmon.c | 5 - > 1 file changed, 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_hwmon.c > b/drivers/gpu/drm/i915/i915_hwmon.c > index 4683a5b96eff1..1225bc432f0d5 100644 > --- a/drivers/gpu/drm/i915/i915_hwmon.c > +++ b/drivers/gpu/drm/i915/i915_hwmon.c > @@ -687,11 +687,6 @@ hwm_get_preregistration_info(struct drm_i915_private > *i915) > for_each_gt(gt, i915, i) > hwm_energy(>ddat_gt[i], ); > } > - > - /* Enable PL1 power limit */ > - if (i915_mmio_reg_valid(hwmon->rg.pkg_rapl_limit)) > - hwm_locked_with_pm_intel_uncore_rmw(ddat, > hwmon->rg.pkg_rapl_limit, > - PKG_PWR_LIM_1_EN, > PKG_PWR_LIM_1_EN); > } > > void i915_hwmon_register(struct drm_i915_private *i915) > -- > 2.38.0 >
[PATCH] drm/amd/display: don't call dc_interrupt_set() for disabled crtcs
As made mention of in commit 4ea7fc09539b ("drm/amd/display: Do not program interrupt status on disabled crtc"), we shouldn't program disabled crtcs. So, filter out disabled crtcs in dm_set_vupdate_irq() and dm_set_vblank(). Fixes: 589d2739332d ("drm/amd/display: Use crtc enable/disable_vblank hooks") Fixes: d2574c33bb71 ("drm/amd/display: In VRR mode, do DRM core vblank handling at end of vblank. (v2)") Signed-off-by: Hamza Mahfooz --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c index 1e39d0939700..dc4f37240beb 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c @@ -77,6 +77,9 @@ int dm_set_vupdate_irq(struct drm_crtc *crtc, bool enable) struct amdgpu_device *adev = drm_to_adev(crtc->dev); int rc; + if (acrtc->otg_inst == -1) + return 0; + irq_source = IRQ_TYPE_VUPDATE + acrtc->otg_inst; rc = dc_interrupt_set(adev->dm.dc, irq_source, enable) ? 0 : -EBUSY; @@ -151,6 +154,9 @@ static inline int dm_set_vblank(struct drm_crtc *crtc, bool enable) struct vblank_control_work *work; int rc = 0; + if (acrtc->otg_inst == -1) + goto skip; + if (enable) { /* vblank irq on -> Only need vupdate irq in vrr mode */ if (amdgpu_dm_vrr_active(acrtc_state)) @@ -168,6 +174,7 @@ static inline int dm_set_vblank(struct drm_crtc *crtc, bool enable) if (!dc_interrupt_set(adev->dm.dc, irq_source, enable)) return -EBUSY; +skip: if (amdgpu_in_reset(adev)) return 0; -- 2.39.1
Re: [PATCH 1/5] drm/sched: Create wrapper to add a syncobj dependency to job
Am 08.02.23 um 20:48 schrieb Maíra Canal: In order to add a syncobj's fence as a dependency to a job, it is necessary to call drm_syncobj_find_fence() to find the fence and then add the dependency with drm_sched_job_add_dependency(). So, wrap these steps in one single function, drm_sched_job_add_syncobj_dependency(). Signed-off-by: Maíra Canal Just one nit pick below, with that fixed Reviewed-by: Christian König I'm pretty sure we have the exact same function now in amdgpu cause I cleaned that up just a few weeks ago to look the same as the other drivers. Would be nice to have that new function applied there as well. Thanks, Christian. --- drivers/gpu/drm/scheduler/sched_main.c | 29 ++ include/drm/gpu_scheduler.h| 6 ++ 2 files changed, 35 insertions(+) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 0e4378420271..d5331b1877a3 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -53,6 +53,7 @@ #include #include +#include #include #include @@ -718,6 +719,34 @@ int drm_sched_job_add_dependency(struct drm_sched_job *job, } EXPORT_SYMBOL(drm_sched_job_add_dependency); +/** + * drm_sched_job_add_syncobj_dependency - adds a syncobj's fence as a job dependency + * @job: scheduler job to add the dependencies to + * @file_private: drm file private pointer + * @handle: syncobj handle to lookup + * @point: timeline point + * + * This adds the fence matching the given syncobj to @job. + * + * Returns: + * 0 on success, or an error on failing to expand the array. + */ +int drm_sched_job_add_syncobj_dependency(struct drm_sched_job *job, +struct drm_file *file, +u32 handle, +u32 point) +{ + struct dma_fence *fence; + int ret = 0; Please don't initialize any local return variables if it isn't necessary. This just suppresses uninitialized variables from the compiler which quite often have helped finding more wider bugs. Regards, Christian. + + ret = drm_syncobj_find_fence(file, handle, point, 0, ); + if (ret) + return ret; + + return drm_sched_job_add_dependency(job, fence); +} +EXPORT_SYMBOL(drm_sched_job_add_syncobj_dependency); + /** * drm_sched_job_add_resv_dependencies - add all fences from the resv to the job * @job: scheduler job to add the dependencies to diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 9935d1e2ff69..4cc54f8b57b4 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -48,6 +48,8 @@ struct drm_gem_object; struct drm_gpu_scheduler; struct drm_sched_rq; +struct drm_file; + /* These are often used as an (initial) index * to an array, and as such should start at 0. */ @@ -515,6 +517,10 @@ int drm_sched_job_init(struct drm_sched_job *job, void drm_sched_job_arm(struct drm_sched_job *job); int drm_sched_job_add_dependency(struct drm_sched_job *job, struct dma_fence *fence); +int drm_sched_job_add_syncobj_dependency(struct drm_sched_job *job, +struct drm_file *file, +u32 handle, +u32 point); int drm_sched_job_add_resv_dependencies(struct drm_sched_job *job, struct dma_resv *resv, enum dma_resv_usage usage);
[PATCH 5/5] drm/v3d: Use drm_sched_job_add_syncobj_dependency()
As v3d_job_add_deps() performs the same steps as drm_sched_job_add_syncobj_dependency(), replace the open-coded implementation in v3d in order to simply, using the DRM function. Signed-off-by: Maíra Canal --- drivers/gpu/drm/v3d/v3d_gem.c | 9 + 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c index 5da1806f3969..f149526ec971 100644 --- a/drivers/gpu/drm/v3d/v3d_gem.c +++ b/drivers/gpu/drm/v3d/v3d_gem.c @@ -400,14 +400,7 @@ static int v3d_job_add_deps(struct drm_file *file_priv, struct v3d_job *job, u32 in_sync, u32 point) { - struct dma_fence *in_fence = NULL; - int ret; - - ret = drm_syncobj_find_fence(file_priv, in_sync, point, 0, _fence); - if (ret == -EINVAL) - return ret; - - return drm_sched_job_add_dependency(>base, in_fence); + return drm_sched_job_add_syncobj_dependency(>base, file_priv, in_sync, point); } static int -- 2.39.1
[PATCH 4/5] drm/panfrost: Use drm_sched_job_add_syncobj_dependency()
As panfrost_copy_in_sync() performs the same steps as drm_sched_job_add_syncobj_dependency(), replace the open-coded implementation in Panfrost in order to simply, using the DRM function. Signed-off-by: Maíra Canal --- drivers/gpu/drm/panfrost/panfrost_drv.c | 11 ++- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index abb0dadd8f63..f49096f53141 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -220,15 +220,8 @@ panfrost_copy_in_sync(struct drm_device *dev, } for (i = 0; i < in_fence_count; i++) { - struct dma_fence *fence; - - ret = drm_syncobj_find_fence(file_priv, handles[i], 0, 0, -); - if (ret) - goto fail; - - ret = drm_sched_job_add_dependency(>base, fence); - + ret = drm_sched_job_add_syncobj_dependency(>base, file_priv, + handles[i], 0); if (ret) goto fail; } -- 2.39.1
[PATCH 3/5] drm/msm: Use drm_sched_job_add_syncobj_dependency()
As msm_parse_deps() performs the same steps as drm_sched_job_add_syncobj_dependency(), replace the open-coded implementation in msm in order to simply, using the DRM function. Signed-off-by: Maíra Canal --- drivers/gpu/drm/msm/msm_gem_submit.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c index 73a2ca122c57..622f1e27fcca 100644 --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -570,12 +570,8 @@ static struct drm_syncobj **msm_parse_deps(struct msm_gem_submit *submit, break; } - ret = drm_syncobj_find_fence(file, syncobj_desc.handle, -syncobj_desc.point, 0, ); - if (ret) - break; - - ret = drm_sched_job_add_dependency(>base, fence); + ret = drm_sched_job_add_syncobj_dependency(>base, file, + syncobj_desc.handle, syncobj_desc.point); if (ret) break; -- 2.39.1
[PATCH 2/5] drm/lima: Use drm_sched_job_add_syncobj_dependency()
As lima_gem_add_deps() performs the same steps as drm_sched_job_add_syncobj_dependency(), replace the open-coded implementation in Lima in order to simply, using the DRM function. Signed-off-by: Maíra Canal --- drivers/gpu/drm/lima/lima_gem.c | 12 ++-- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c index 0f1ca0b0db49..10252dc11a22 100644 --- a/drivers/gpu/drm/lima/lima_gem.c +++ b/drivers/gpu/drm/lima/lima_gem.c @@ -277,21 +277,13 @@ static int lima_gem_add_deps(struct drm_file *file, struct lima_submit *submit) int i, err; for (i = 0; i < ARRAY_SIZE(submit->in_sync); i++) { - struct dma_fence *fence = NULL; - if (!submit->in_sync[i]) continue; - err = drm_syncobj_find_fence(file, submit->in_sync[i], -0, 0, ); + err = drm_sched_job_add_syncobj_dependency(>task->base, file, + submit->in_sync[i], 0); if (err) return err; - - err = drm_sched_job_add_dependency(>task->base, fence); - if (err) { - dma_fence_put(fence); - return err; - } } return 0; -- 2.39.1
[PATCH 1/5] drm/sched: Create wrapper to add a syncobj dependency to job
In order to add a syncobj's fence as a dependency to a job, it is necessary to call drm_syncobj_find_fence() to find the fence and then add the dependency with drm_sched_job_add_dependency(). So, wrap these steps in one single function, drm_sched_job_add_syncobj_dependency(). Signed-off-by: Maíra Canal --- drivers/gpu/drm/scheduler/sched_main.c | 29 ++ include/drm/gpu_scheduler.h| 6 ++ 2 files changed, 35 insertions(+) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 0e4378420271..d5331b1877a3 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -53,6 +53,7 @@ #include #include +#include #include #include @@ -718,6 +719,34 @@ int drm_sched_job_add_dependency(struct drm_sched_job *job, } EXPORT_SYMBOL(drm_sched_job_add_dependency); +/** + * drm_sched_job_add_syncobj_dependency - adds a syncobj's fence as a job dependency + * @job: scheduler job to add the dependencies to + * @file_private: drm file private pointer + * @handle: syncobj handle to lookup + * @point: timeline point + * + * This adds the fence matching the given syncobj to @job. + * + * Returns: + * 0 on success, or an error on failing to expand the array. + */ +int drm_sched_job_add_syncobj_dependency(struct drm_sched_job *job, +struct drm_file *file, +u32 handle, +u32 point) +{ + struct dma_fence *fence; + int ret = 0; + + ret = drm_syncobj_find_fence(file, handle, point, 0, ); + if (ret) + return ret; + + return drm_sched_job_add_dependency(job, fence); +} +EXPORT_SYMBOL(drm_sched_job_add_syncobj_dependency); + /** * drm_sched_job_add_resv_dependencies - add all fences from the resv to the job * @job: scheduler job to add the dependencies to diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 9935d1e2ff69..4cc54f8b57b4 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -48,6 +48,8 @@ struct drm_gem_object; struct drm_gpu_scheduler; struct drm_sched_rq; +struct drm_file; + /* These are often used as an (initial) index * to an array, and as such should start at 0. */ @@ -515,6 +517,10 @@ int drm_sched_job_init(struct drm_sched_job *job, void drm_sched_job_arm(struct drm_sched_job *job); int drm_sched_job_add_dependency(struct drm_sched_job *job, struct dma_fence *fence); +int drm_sched_job_add_syncobj_dependency(struct drm_sched_job *job, +struct drm_file *file, +u32 handle, +u32 point); int drm_sched_job_add_resv_dependencies(struct drm_sched_job *job, struct dma_resv *resv, enum dma_resv_usage usage); -- 2.39.1
[PATCH 0/5] drm/sched: Create wrapper to add a syncobj dependency to job
Some drivers perform the same operation to add a syncobj's fence to the sched as a dependency: first, call drm_syncobj_find_fence() to find the fence and then, call drm_sched_job_add_dependency(). Therefore, create a wrapper to encapsulate those steps in one single function. The first patch creates the wrapper for the operation and the following patches make the drivers use the new function drm_sched_job_add_syncobj_dependency(). Best Regards, - Maíra Canal Maíra Canal (5): drm/sched: Create wrapper to add a syncobj dependency to job drm/lima: Use drm_sched_job_add_syncobj_dependency() drm/msm: Use drm_sched_job_add_syncobj_dependency() drm/panfrost: Use drm_sched_job_add_syncobj_dependency() drm/v3d: Use drm_sched_job_add_syncobj_dependency() drivers/gpu/drm/lima/lima_gem.c | 12 ++ drivers/gpu/drm/msm/msm_gem_submit.c| 8 ++- drivers/gpu/drm/panfrost/panfrost_drv.c | 11 ++ drivers/gpu/drm/scheduler/sched_main.c | 29 + drivers/gpu/drm/v3d/v3d_gem.c | 9 +--- include/drm/gpu_scheduler.h | 6 + 6 files changed, 42 insertions(+), 33 deletions(-) -- 2.39.1
Re: [PATCH v3 5/6] drm/debugfs: Make the show callback pass the pointer to the right object
On 2/8/23 15:17, Daniel Vetter wrote: On Wed, Feb 08, 2023 at 07:12:22PM +0100, Daniel Vetter wrote: On Tue, Jan 31, 2023 at 04:58:25PM -0300, Maíra Canal wrote: Currently, the drivers need to access the struct drm_debugfs_entry to get the proper device on the show callback. There is no need for such thing, as you can wrap the show callback in order to provide to the driver the proper parameters: the struct seq_file, the struct drm_device and the driver-specific data stored in the struct drm_debugfs_info. Therefore, make the show callback pass the pointer to the right object in the parameters, which makes the API more type-safe. Signed-off-by: Maíra Canal --- drivers/gpu/drm/arm/hdlcd_drv.c | 8 ++-- drivers/gpu/drm/drm_atomic.c | 4 +--- drivers/gpu/drm/drm_client.c | 5 ++--- drivers/gpu/drm/drm_debugfs.c | 25 - drivers/gpu/drm/drm_framebuffer.c | 4 +--- drivers/gpu/drm/drm_gem_vram_helper.c | 5 ++--- drivers/gpu/drm/gud/gud_drv.c | 5 ++--- drivers/gpu/drm/v3d/v3d_debugfs.c | 16 drivers/gpu/drm/vc4/vc4_bo.c | 4 +--- drivers/gpu/drm/vc4/vc4_debugfs.c | 6 ++ drivers/gpu/drm/vc4/vc4_hdmi.c| 6 ++ drivers/gpu/drm/vc4/vc4_hvs.c | 8 ++-- drivers/gpu/drm/vc4/vc4_v3d.c | 4 +--- drivers/gpu/drm/vkms/vkms_drv.c | 4 +--- include/drm/drm_debugfs.h | 14 -- 15 files changed, 43 insertions(+), 75 deletions(-) [...] diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h index 423aa3de506a..0fb7ad5f6893 100644 --- a/include/drm/drm_debugfs.h +++ b/include/drm/drm_debugfs.h @@ -36,6 +36,9 @@ #include #include #include + +struct drm_device; + /** * struct drm_info_list - debugfs info list entry * @@ -108,11 +111,10 @@ struct drm_debugfs_info { /** * @show: * -* Show callback. _file->private will be set to the -* drm_debugfs_entry corresponding to the instance of this info -* on a given drm_device. +* Show callback. This callback will be casted in order to provide +* the _file, the DRM object and the data stored in this struct. */ - int (*show)(struct seq_file*, void*); + void *show; The problem here is that with this we loose type-checking, and so all the users of drm_debugfs_add_file() have been missed in the conversion. That's not very good :-/ Correction, you didn't miss that, but it's the risk that could happen because the driver doesn't check things. I think the only way to sort this out is if we duplicate the driver-facing functions/structs (maybe we don't need the add_files() functions in all cases?), and only use the type-unsafe void* internally. Since I didnt' spell it out: If you only keep the change to add the drm_device *dev pointer in this patch, but keep the full type everywhere, then struct drm_debugfs_entry becomes an implementation detail and you can move it into drm_debugfs.c. Once you have that you can throw out the driver api facing struct drm_debugfs_info and just put all the required pointers and things directly in there as void *, and it should work out with reasonable amounts of code sharing, while the driver api all stays type safe. So basically, the idea is to make this patchset using int (*show)(struct seq_file*, struct drm_device*, void*)? I'm not sure how could we could reuse the struct drm_debugfs_info if we don't use void * in the show callback, as add_files needs the struct. The type-checking is handle by the add_file and the show wrapper. Otherwise, we would have to duplicate the drm_debugfs_info and add_file for each object. Or do you have any other idea on how to implement it? Anyway, I'll send a new version with int (*show)(struct seq_file*, struct drm_device*, void*) and moving the struct drm_debugfs_entry to drm_debugfs.c and we can keep working on the API. Thanks for all the feedback! Best Regards, - Maíra Canal -Daniel -Daniel /** @driver_features: Required driver features for this entry. */ u32 driver_features; @@ -146,7 +148,7 @@ int drm_debugfs_remove_files(const struct drm_info_list *files, int count, struct drm_minor *minor); void drm_debugfs_add_file(struct drm_device *dev, const char *name, - int (*show)(struct seq_file*, void*), void *data); + int (*show)(struct seq_file*, struct drm_device*, void*), void *data); void drm_debugfs_add_files(struct drm_device *dev, const struct drm_debugfs_info *files, int count); @@ -163,7 +165,7 @@ static inline int drm_debugfs_remove_files(const struct drm_info_list *files, } static inline void drm_debugfs_add_file(struct drm_device *dev, const char *name, - int (*show)(struct seq_file*, void*), +
Re: drm: panel-orientation-quirks: Add quirk for Lenovo IdeaPad Duet 3 10IGL5
I've resolved this by adding a matching quirk in drivers/firmware/efi/sysfb_efi.c - see below. Are you the right people to be notifying about this? --- diff --git a/kernel/6.2-rc6 original/sysfb_efi.c b/kernel/6.2-rc6 changes/sysfb_efi.c index 7882d4b..f06fdac 100755 --- a/kernel/6.2-rc6 original/sysfb_efi.c +++ b/kernel/6.2-rc6 changes/sysfb_efi.c @@ -264,6 +264,14 @@ static const struct dmi_system_id efifb_dmi_swap_width_height[] __initconst = { "Lenovo ideapad D330-10IGM"), }, }, + { + /* Lenovo IdeaPad Duet 3 10IGL5 with 1200x1920 portrait screen */ + .matches = { + DMI_EXACT_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_EXACT_MATCH(DMI_PRODUCT_VERSION, + "IdeaPad Duet 3 10IGL5"), + }, + }, {}, }; --- Thanks, Darrell On Tue, 7 Feb 2023 at 15:51, Darrell Kavanagh wrote: > > Further information: > > With the above fix, the very early boot console messages are not > rotated. adding "fbcon=rotate:1" to the kernel command line corrects > this. But these early boot console messages are still garbled - it > looks like the display driver in use at the time cannot write to the > screen fast enough - lines are half-formed before scrolling. > > Note that this corrects itself and later boot messages are legible > before the plymouth splash (if in use). I can't see anything that > looks like useful information re the fb in journalctl immediately > preceding the first legible output seen during boot. > > I've played around with console, earlycon and fbcon parms to no useful > effect. Any ideas? > > Darrell > > On Fri, 3 Feb 2023 at 18:32, Darrell Kavanagh > wrote: > > > > Hi, > > > > This is another Lenovo with detachable keyboard and 1200x1920 screen > > mounted sideways. > > > > The following has been tested with 6.2.0-rc6. > > > > Thanks, > > Darrell > > > > index 3659f04..590bb7b 100644 > > --- a/kernel/drm_panel_orientation > > _quirks.c > > +++ b/kernel/linux-6.2-rc6/drivers/gpu/drm/drm_panel_orientation_quirks.c > > @@ -304,6 +304,12 @@ static const struct dmi_system_id orientation_data[] = > > { > > DMI_EXACT_MATCH(DMI_PRODUCT_VERSION, "Lenovo ideapad > > D330-10IGM"), > > }, > > .driver_data = (void *)_rightside_up, > > + }, {/* Lenovo IdeaPad Duet 3 10IGL5 */ > > + .matches = { > > + DMI_EXACT_MATCH(DMI_SYS_VENDOR, "LENOVO"), > > + DMI_EXACT_MATCH(DMI_PRODUCT_VERSION, "IdeaPad Duet 3 > > 10IGL5"), > > + }, > > + .driver_data = (void *)_rightside_up, > > }, {/* Lenovo Ideapad D330-10IGL (HD) */ > > .matches = { > > DMI_EXACT_MATCH(DMI_SYS_VENDOR, "LENOVO"),
[PATCH] Revert "drm/i915/hwmon: Enable PL1 power limit"
This reverts commit 0349c41b05968befaffa5fbb7e73d0ee6004f610. 0349c41b0596 ("drm/i915/hwmon: Enable PL1 power limit") is incorrect and caused a major regression on ATSM. The change enabled the PL1 power limit but FW sets the default value of the PL1 limit to 0 which implies HW now works at minimum power and therefore the lowest effective frequency. This means all workloads now run slower resulting in even GuC FW load operations timing out, rendering ATSM unusable. A different solution to the original issue of the PL1 limit being disabled on ATSM is needed but till that is developed, revert 0349c41b0596. Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8062 Signed-off-by: Ashutosh Dixit --- drivers/gpu/drm/i915/i915_hwmon.c | 5 - 1 file changed, 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c index 4683a5b96eff1..1225bc432f0d5 100644 --- a/drivers/gpu/drm/i915/i915_hwmon.c +++ b/drivers/gpu/drm/i915/i915_hwmon.c @@ -687,11 +687,6 @@ hwm_get_preregistration_info(struct drm_i915_private *i915) for_each_gt(gt, i915, i) hwm_energy(>ddat_gt[i], ); } - - /* Enable PL1 power limit */ - if (i915_mmio_reg_valid(hwmon->rg.pkg_rapl_limit)) - hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit, - PKG_PWR_LIM_1_EN, PKG_PWR_LIM_1_EN); } void i915_hwmon_register(struct drm_i915_private *i915) -- 2.38.0
Re: [PATCH v3 1/6] drm/debugfs: Introduce wrapper for debugfs list
On Wed, Feb 08, 2023 at 03:39:13PM -0300, Maíra Canal wrote: > On 2/8/23 15:06, Daniel Vetter wrote: > > On Tue, Jan 31, 2023 at 04:58:21PM -0300, Maíra Canal wrote: > > > Introduce a struct wrapper for all the debugfs-related stuff: the list > > > of debugfs files and the mutex that protects it. This will make it > > > easier to initialize all the debugfs list in a DRM object and will > > > create a good abstraction for a possible implementation of the debugfs > > > infrastructure for KMS objects. > > > > > > Signed-off-by: Maíra Canal > > > --- > > > drivers/gpu/drm/drm_debugfs.c | 18 ++ > > > drivers/gpu/drm/drm_internal.h | 12 > > > include/drm/drm_debugfs.h | 16 > > > 3 files changed, 46 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c > > > index 4f643a490dc3..8658d3929ea5 100644 > > > --- a/drivers/gpu/drm/drm_debugfs.c > > > +++ b/drivers/gpu/drm/drm_debugfs.c > > > @@ -218,6 +218,24 @@ void drm_debugfs_create_files(const struct > > > drm_info_list *files, int count, > > > } > > > EXPORT_SYMBOL(drm_debugfs_create_files); > > > +struct drm_debugfs_files *drm_debugfs_files_init(void) > > > +{ > > > + struct drm_debugfs_files *debugfs_files; > > > + > > > + debugfs_files = kzalloc(sizeof(*debugfs_files), GFP_KERNEL); > > > + > > > + INIT_LIST_HEAD(_files->list); > > > + mutex_init(_files->mutex); > > > + > > > + return debugfs_files; > > > +} > > > + > > > +void drm_debugfs_files_destroy(struct drm_debugfs_files *debugfs_files) > > > +{ > > > + mutex_destroy(_files->mutex); > > > + kfree(debugfs_files); > > > +} > > > + > > > int drm_debugfs_init(struct drm_minor *minor, int minor_id, > > >struct dentry *root) > > > { > > > diff --git a/drivers/gpu/drm/drm_internal.h > > > b/drivers/gpu/drm/drm_internal.h > > > index ed2103ee272c..f1c8766ed828 100644 > > > --- a/drivers/gpu/drm/drm_internal.h > > > +++ b/drivers/gpu/drm/drm_internal.h > > > @@ -23,6 +23,7 @@ > > > #include > > > +#include > > > #include > > > #include > > > @@ -183,6 +184,8 @@ int drm_gem_dumb_destroy(struct drm_file *file, > > > struct drm_device *dev, > > > /* drm_debugfs.c drm_debugfs_crc.c */ > > > #if defined(CONFIG_DEBUG_FS) > > > +struct drm_debugfs_files *drm_debugfs_files_init(void); > > > +void drm_debugfs_files_destroy(struct drm_debugfs_files *debugfs_files); > > > int drm_debugfs_init(struct drm_minor *minor, int minor_id, > > >struct dentry *root); > > > void drm_debugfs_cleanup(struct drm_minor *minor); > > > @@ -193,6 +196,15 @@ void drm_debugfs_crtc_add(struct drm_crtc *crtc); > > > void drm_debugfs_crtc_remove(struct drm_crtc *crtc); > > > void drm_debugfs_crtc_crc_add(struct drm_crtc *crtc); > > > #else > > > +static inline struct drm_debugfs_files *drm_debugfs_files_init(void) > > > +{ > > > + return NULL; > > > +} > > > + > > > +static inline void drm_debugfs_files_destroy(struct drm_debugfs_files > > > *debugfs_files) > > > +{ > > > +} > > > + > > > static inline int drm_debugfs_init(struct drm_minor *minor, int > > > minor_id, > > > struct dentry *root) > > > { > > > diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h > > > index 7616f457ce70..423aa3de506a 100644 > > > --- a/include/drm/drm_debugfs.h > > > +++ b/include/drm/drm_debugfs.h > > > @@ -32,6 +32,8 @@ > > > #ifndef _DRM_DEBUGFS_H_ > > > #define _DRM_DEBUGFS_H_ > > > +#include > > > +#include > > > #include > > > #include > > > /** > > > @@ -79,6 +81,20 @@ struct drm_info_node { > > > struct dentry *dent; > > > }; > > > +/** > > > + * struct drm_debugfs_files - Encapsulates the debugfs list and its mutex > > > + * > > > + * This structure represents the debugfs list of files and is > > > encapsulated > > > + * with a mutex to protect the access of the list. > > > + */ > > > +struct drm_debugfs_files { > > > + /** @list: List of debugfs files to be created by the DRM object. */ > > > + struct list_head list; > > > + > > > + /** @mutex: Protects access. */ > > > + struct mutex mutex; > > > > I'm not seeing any use for the mutex here? Also unless you also plan to > > put like the debugfs directory pointers in this struct, I'm not sure we > > need this abstraction since it's purely internal to debugfs code (so also > > should really be in the headers where drivers could perhaps come up with > > funny ideas). > > Isn't this mutex needed to guarantee race-conditions safety when adding new > files to the list, as drm_debugfs_add_file() is called by the drivers? [1] > > [1] > https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/drm_debugfs.c#n343 Hm I looked at the wrong place and only spotted the locking for the old drm_minor debugfs stuff ... it does look a bit funny if we only do this in add_file(), also usually drivers really don't have anything multi-threaded in
Re: [PATCH v3 1/6] drm/debugfs: Introduce wrapper for debugfs list
On 2/8/23 15:06, Daniel Vetter wrote: On Tue, Jan 31, 2023 at 04:58:21PM -0300, Maíra Canal wrote: Introduce a struct wrapper for all the debugfs-related stuff: the list of debugfs files and the mutex that protects it. This will make it easier to initialize all the debugfs list in a DRM object and will create a good abstraction for a possible implementation of the debugfs infrastructure for KMS objects. Signed-off-by: Maíra Canal --- drivers/gpu/drm/drm_debugfs.c | 18 ++ drivers/gpu/drm/drm_internal.h | 12 include/drm/drm_debugfs.h | 16 3 files changed, 46 insertions(+) diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c index 4f643a490dc3..8658d3929ea5 100644 --- a/drivers/gpu/drm/drm_debugfs.c +++ b/drivers/gpu/drm/drm_debugfs.c @@ -218,6 +218,24 @@ void drm_debugfs_create_files(const struct drm_info_list *files, int count, } EXPORT_SYMBOL(drm_debugfs_create_files); +struct drm_debugfs_files *drm_debugfs_files_init(void) +{ + struct drm_debugfs_files *debugfs_files; + + debugfs_files = kzalloc(sizeof(*debugfs_files), GFP_KERNEL); + + INIT_LIST_HEAD(_files->list); + mutex_init(_files->mutex); + + return debugfs_files; +} + +void drm_debugfs_files_destroy(struct drm_debugfs_files *debugfs_files) +{ + mutex_destroy(_files->mutex); + kfree(debugfs_files); +} + int drm_debugfs_init(struct drm_minor *minor, int minor_id, struct dentry *root) { diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index ed2103ee272c..f1c8766ed828 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -23,6 +23,7 @@ #include +#include #include #include @@ -183,6 +184,8 @@ int drm_gem_dumb_destroy(struct drm_file *file, struct drm_device *dev, /* drm_debugfs.c drm_debugfs_crc.c */ #if defined(CONFIG_DEBUG_FS) +struct drm_debugfs_files *drm_debugfs_files_init(void); +void drm_debugfs_files_destroy(struct drm_debugfs_files *debugfs_files); int drm_debugfs_init(struct drm_minor *minor, int minor_id, struct dentry *root); void drm_debugfs_cleanup(struct drm_minor *minor); @@ -193,6 +196,15 @@ void drm_debugfs_crtc_add(struct drm_crtc *crtc); void drm_debugfs_crtc_remove(struct drm_crtc *crtc); void drm_debugfs_crtc_crc_add(struct drm_crtc *crtc); #else +static inline struct drm_debugfs_files *drm_debugfs_files_init(void) +{ + return NULL; +} + +static inline void drm_debugfs_files_destroy(struct drm_debugfs_files *debugfs_files) +{ +} + static inline int drm_debugfs_init(struct drm_minor *minor, int minor_id, struct dentry *root) { diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h index 7616f457ce70..423aa3de506a 100644 --- a/include/drm/drm_debugfs.h +++ b/include/drm/drm_debugfs.h @@ -32,6 +32,8 @@ #ifndef _DRM_DEBUGFS_H_ #define _DRM_DEBUGFS_H_ +#include +#include #include #include /** @@ -79,6 +81,20 @@ struct drm_info_node { struct dentry *dent; }; +/** + * struct drm_debugfs_files - Encapsulates the debugfs list and its mutex + * + * This structure represents the debugfs list of files and is encapsulated + * with a mutex to protect the access of the list. + */ +struct drm_debugfs_files { + /** @list: List of debugfs files to be created by the DRM object. */ + struct list_head list; + + /** @mutex: Protects access. */ + struct mutex mutex; I'm not seeing any use for the mutex here? Also unless you also plan to put like the debugfs directory pointers in this struct, I'm not sure we need this abstraction since it's purely internal to debugfs code (so also should really be in the headers where drivers could perhaps come up with funny ideas). Isn't this mutex needed to guarantee race-conditions safety when adding new files to the list, as drm_debugfs_add_file() is called by the drivers? [1] [1] https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/drm_debugfs.c#n343 Best Regards, - Maíra Canal -Daniel +}; + /** * struct drm_debugfs_info - debugfs info list entry * -- 2.39.1
[GIT PULL] etnaviv-next for 6.3
Hi Dave, hi Daniel, please pull the following etnaviv changes for the next merge window. This time we've added support for reporting of GPU load via the common fdinfo format, as already supported by multiple other drivers. Improved diagnostic messages for MMU faults. And finally added experimental support for driving the VeriSilicon NPU cores, which are very close relatives to the GPU designs, so close in fact that they can run the same compute instruction set, but with a big NN-fabric/matrix/tensor execution array glued to the side. Regards, Lucas The following changes since commit 1b929c02afd37871d5afb9d498426f83432e71c2: Linux 6.2-rc1 (2022-12-25 13:41:39 -0800) are available in the Git repository at: https://git.pengutronix.de/git/lst/linux etnaviv/next for you to fetch changes up to 4c22c61e429f004d84eba72d7195bccef33ea0ec: drm/etnaviv: show number of NN cores in GPU debugfs info (2023-02-07 20:49:55 +0100) Christian Gmeiner (1): drm/etnaviv: print MMU exception cause Lucas Stach (7): drm/etnaviv: update hardware headers from rnndb drm/etnaviv: split fence lock drm/etnaviv: convert user fence tracking to XArray drm/scheduler: track GPU active time per entity drm/etnaviv: allocate unique ID per drm_file drm/etnaviv: export client GPU usage statistics via fdinfo drm/etnaviv: show number of NN cores in GPU debugfs info Paul Cercueil (1): drm/etnaviv: Remove #ifdef guards for PM related functions Tomeu Vizoso (3): drm/etnaviv: Add nn_core_count to chip feature struct drm/etnaviv: Warn when probing on NPUs drm/etnaviv: add HWDB entry for VIPNano-QI.7120.0055 drivers/gpu/drm/etnaviv/etnaviv_drv.c| 54 - drivers/gpu/drm/etnaviv/etnaviv_drv.h| 5 +++ drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 9 ++--- drivers/gpu/drm/etnaviv/etnaviv_gpu.c| 66 +-- drivers/gpu/drm/etnaviv/etnaviv_gpu.h| 8 +++-- drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 36 +++ drivers/gpu/drm/etnaviv/etnaviv_sched.c | 18 +- drivers/gpu/drm/etnaviv/state_hi.xml.h | 86 +- drivers/gpu/drm/scheduler/sched_main.c | 6 include/drm/gpu_scheduler.h | 7 10 files changed, 238 insertions(+), 57 deletions(-)
Re: DRM accel and debugfs/sysfs
On 2/8/23 15:13, Oded Gabbay wrote: On Wed, Feb 8, 2023 at 8:07 PM Daniel Vetter wrote: On Tue, Feb 07, 2023 at 01:17:47PM -0300, Maíra Canal wrote: On 2/7/23 12:43, Jeffrey Hugo wrote: On 2/7/2023 4:31 AM, Maíra Canal wrote: Hi Stanislaw, On 2/1/23 12:20, Stanislaw Gruszka wrote: Hi I was about to send debugfs support for ivpu and noticed that there are current changes that deprecate drm_devel->debugfs_init callback. Further I looked at this commit [1], that stated we should not use drm_minor for debugfs and sysfs. What is quite contrary to what drm accel framework did in the first place. So my question is how we should use debugfs/sysfs in accel? Use it with old fashioned minor-centric way or change the framework somehow ? As we are trying to replace drm_debugfs_create_files() [1], it would be nice to see the accel debugfs support use the new debugfs API. This would mean using the debugfs_list from the drm_device, deprecating the debugfs_init callback, and adding the a similar code snippet to accel_debugfs_init: list_for_each_entry_safe(entry, tmp, >debugfs_list, list) { debugfs_create_file(entry->file.name, 0444, minor->debugfs_root, entry, _debugfs_entry_fops); list_del(>list); Maybe Daniel has some more thoughts on this matter, but I guess it would be better to drop the use of the old-fashioned minor-centric implementation in accel. It was a simple case of two things landing in parallel and not being synchronized. Would be good if accel could be adapted to use the new debugfs infra, now that both accel and the new debugfs stuff have landed. -Daniel Yes, definitely. Does anyone volunteer to send a patch to align ? I can send a patch this week align it. Best Regards, - Maíra Canal If not, we will do it internally and send a patch. Oded [1] https://cgit.freedesktop.org/drm/drm-misc/tree/Documentation/gpu/todo.rst#n511 Best Regards, - Maíra Canal Thank you for the details Maira. It helps to explain what the todo is suggesting. Is there an example of a driver/drm_device that uses debugfs_list which you can easily point to? The implementation of this device-centered infrastructure is linked in [1] and an example of the conversion of debugfs APIs is linked in [2], and other drivers such as v3d, vkms, vc4 and gud use this new API as well. [1] https://cgit.freedesktop.org/drm/drm-misc/commit/?id=1c9cacbea880513a896aee65a5c58007bcb55653 [2] https://cgit.freedesktop.org/drm/drm-misc/commit/?id=2e3ab8a6994f265bbd4dbd00448b84548f18464c Best Regards, - Maíra Canal -Jeff [1] https://cgit.freedesktop.org/drm/drm-misc/commit/?id=99845faae7099cd704ebf67514c1157c26960a26 Regards Stanislaw -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v3 1/6] drm/debugfs: Introduce wrapper for debugfs list
On Wed, Feb 08, 2023 at 07:06:19PM +0100, Daniel Vetter wrote: > On Tue, Jan 31, 2023 at 04:58:21PM -0300, Maíra Canal wrote: > > Introduce a struct wrapper for all the debugfs-related stuff: the list > > of debugfs files and the mutex that protects it. This will make it > > easier to initialize all the debugfs list in a DRM object and will > > create a good abstraction for a possible implementation of the debugfs > > infrastructure for KMS objects. > > > > Signed-off-by: Maíra Canal > > --- > > drivers/gpu/drm/drm_debugfs.c | 18 ++ > > drivers/gpu/drm/drm_internal.h | 12 > > include/drm/drm_debugfs.h | 16 > > 3 files changed, 46 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c > > index 4f643a490dc3..8658d3929ea5 100644 > > --- a/drivers/gpu/drm/drm_debugfs.c > > +++ b/drivers/gpu/drm/drm_debugfs.c > > @@ -218,6 +218,24 @@ void drm_debugfs_create_files(const struct > > drm_info_list *files, int count, > > } > > EXPORT_SYMBOL(drm_debugfs_create_files); > > > > +struct drm_debugfs_files *drm_debugfs_files_init(void) > > +{ > > + struct drm_debugfs_files *debugfs_files; > > + > > + debugfs_files = kzalloc(sizeof(*debugfs_files), GFP_KERNEL); > > + > > + INIT_LIST_HEAD(_files->list); > > + mutex_init(_files->mutex); > > + > > + return debugfs_files; > > +} > > + > > +void drm_debugfs_files_destroy(struct drm_debugfs_files *debugfs_files) > > +{ > > + mutex_destroy(_files->mutex); > > + kfree(debugfs_files); > > +} > > + > > int drm_debugfs_init(struct drm_minor *minor, int minor_id, > > struct dentry *root) > > { > > diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h > > index ed2103ee272c..f1c8766ed828 100644 > > --- a/drivers/gpu/drm/drm_internal.h > > +++ b/drivers/gpu/drm/drm_internal.h > > @@ -23,6 +23,7 @@ > > > > #include > > > > +#include > > #include > > #include > > > > @@ -183,6 +184,8 @@ int drm_gem_dumb_destroy(struct drm_file *file, struct > > drm_device *dev, > > > > /* drm_debugfs.c drm_debugfs_crc.c */ > > #if defined(CONFIG_DEBUG_FS) > > +struct drm_debugfs_files *drm_debugfs_files_init(void); > > +void drm_debugfs_files_destroy(struct drm_debugfs_files *debugfs_files); > > int drm_debugfs_init(struct drm_minor *minor, int minor_id, > > struct dentry *root); > > void drm_debugfs_cleanup(struct drm_minor *minor); > > @@ -193,6 +196,15 @@ void drm_debugfs_crtc_add(struct drm_crtc *crtc); > > void drm_debugfs_crtc_remove(struct drm_crtc *crtc); > > void drm_debugfs_crtc_crc_add(struct drm_crtc *crtc); > > #else > > +static inline struct drm_debugfs_files *drm_debugfs_files_init(void) > > +{ > > + return NULL; > > +} > > + > > +static inline void drm_debugfs_files_destroy(struct drm_debugfs_files > > *debugfs_files) > > +{ > > +} > > + > > static inline int drm_debugfs_init(struct drm_minor *minor, int minor_id, > >struct dentry *root) > > { > > diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h > > index 7616f457ce70..423aa3de506a 100644 > > --- a/include/drm/drm_debugfs.h > > +++ b/include/drm/drm_debugfs.h > > @@ -32,6 +32,8 @@ > > #ifndef _DRM_DEBUGFS_H_ > > #define _DRM_DEBUGFS_H_ > > > > +#include > > +#include > > #include > > #include > > /** > > @@ -79,6 +81,20 @@ struct drm_info_node { > > struct dentry *dent; > > }; > > > > +/** > > + * struct drm_debugfs_files - Encapsulates the debugfs list and its mutex > > + * > > + * This structure represents the debugfs list of files and is encapsulated > > + * with a mutex to protect the access of the list. > > + */ > > +struct drm_debugfs_files { > > + /** @list: List of debugfs files to be created by the DRM object. */ > > + struct list_head list; > > + > > + /** @mutex: Protects access. */ > > + struct mutex mutex; > > I'm not seeing any use for the mutex here? Also unless you also plan to > put like the debugfs directory pointers in this struct, I'm not sure we > need this abstraction since it's purely internal to debugfs code (so also > should really be in the headers where drivers could perhaps come up with > funny ideas). To clarify, I think any struct or code which is potentially type unsafe, like this one here or the drm_debugfs_entry one, should be moved into drm_debugfs.c. That way drivers do not ever see the potentially dangerous pieces, and only have a type-safe interface for everything. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v3 5/6] drm/debugfs: Make the show callback pass the pointer to the right object
On Wed, Feb 08, 2023 at 07:12:22PM +0100, Daniel Vetter wrote: > On Tue, Jan 31, 2023 at 04:58:25PM -0300, Maíra Canal wrote: > > Currently, the drivers need to access the struct drm_debugfs_entry to > > get the proper device on the show callback. There is no need for such > > thing, as you can wrap the show callback in order to provide to the > > driver the proper parameters: the struct seq_file, the struct drm_device > > and the driver-specific data stored in the struct drm_debugfs_info. > > > > Therefore, make the show callback pass the pointer to the right object > > in the parameters, which makes the API more type-safe. > > > > Signed-off-by: Maíra Canal > > --- > > drivers/gpu/drm/arm/hdlcd_drv.c | 8 ++-- > > drivers/gpu/drm/drm_atomic.c | 4 +--- > > drivers/gpu/drm/drm_client.c | 5 ++--- > > drivers/gpu/drm/drm_debugfs.c | 25 - > > drivers/gpu/drm/drm_framebuffer.c | 4 +--- > > drivers/gpu/drm/drm_gem_vram_helper.c | 5 ++--- > > drivers/gpu/drm/gud/gud_drv.c | 5 ++--- > > drivers/gpu/drm/v3d/v3d_debugfs.c | 16 > > drivers/gpu/drm/vc4/vc4_bo.c | 4 +--- > > drivers/gpu/drm/vc4/vc4_debugfs.c | 6 ++ > > drivers/gpu/drm/vc4/vc4_hdmi.c| 6 ++ > > drivers/gpu/drm/vc4/vc4_hvs.c | 8 ++-- > > drivers/gpu/drm/vc4/vc4_v3d.c | 4 +--- > > drivers/gpu/drm/vkms/vkms_drv.c | 4 +--- > > include/drm/drm_debugfs.h | 14 -- > > 15 files changed, 43 insertions(+), 75 deletions(-) > > > > diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c > > b/drivers/gpu/drm/arm/hdlcd_drv.c > > index e3507dd6f82a..b70bc7b11764 100644 > > --- a/drivers/gpu/drm/arm/hdlcd_drv.c > > +++ b/drivers/gpu/drm/arm/hdlcd_drv.c > > @@ -193,10 +193,8 @@ static int hdlcd_setup_mode_config(struct drm_device > > *drm) > > } > > > > #ifdef CONFIG_DEBUG_FS > > -static int hdlcd_show_underrun_count(struct seq_file *m, void *arg) > > +static int hdlcd_show_underrun_count(struct seq_file *m, struct drm_device > > *drm, void *arg) > > { > > - struct drm_debugfs_entry *entry = m->private; > > - struct drm_device *drm = entry->dev; > > struct hdlcd_drm_private *hdlcd = drm_to_hdlcd_priv(drm); > > > > seq_printf(m, "underrun : %d\n", > > atomic_read(>buffer_underrun_count)); > > @@ -206,10 +204,8 @@ static int hdlcd_show_underrun_count(struct seq_file > > *m, void *arg) > > return 0; > > } > > > > -static int hdlcd_show_pxlclock(struct seq_file *m, void *arg) > > +static int hdlcd_show_pxlclock(struct seq_file *m, struct drm_device *drm, > > void *arg) > > { > > - struct drm_debugfs_entry *entry = m->private; > > - struct drm_device *drm = entry->dev; > > struct hdlcd_drm_private *hdlcd = drm_to_hdlcd_priv(drm); > > unsigned long clkrate = clk_get_rate(hdlcd->clk); > > unsigned long mode_clock = hdlcd->crtc.mode.crtc_clock * 1000; > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > > index 5457c02ca1ab..38f140481fcc 100644 > > --- a/drivers/gpu/drm/drm_atomic.c > > +++ b/drivers/gpu/drm/drm_atomic.c > > @@ -1754,10 +1754,8 @@ void drm_state_dump(struct drm_device *dev, struct > > drm_printer *p) > > EXPORT_SYMBOL(drm_state_dump); > > > > #ifdef CONFIG_DEBUG_FS > > -static int drm_state_info(struct seq_file *m, void *data) > > +static int drm_state_info(struct seq_file *m, struct drm_device *dev, void > > *data) > > { > > - struct drm_debugfs_entry *entry = m->private; > > - struct drm_device *dev = entry->dev; > > struct drm_printer p = drm_seq_file_printer(m); > > > > __drm_state_dump(dev, , true); > > diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c > > index 009e7b10455c..ec2e6bc3515d 100644 > > --- a/drivers/gpu/drm/drm_client.c > > +++ b/drivers/gpu/drm/drm_client.c > > @@ -488,10 +488,9 @@ int drm_client_framebuffer_flush(struct > > drm_client_buffer *buffer, struct drm_re > > EXPORT_SYMBOL(drm_client_framebuffer_flush); > > > > #ifdef CONFIG_DEBUG_FS > > -static int drm_client_debugfs_internal_clients(struct seq_file *m, void > > *data) > > +static int drm_client_debugfs_internal_clients(struct seq_file *m, struct > > drm_device *dev, > > + void *data) > > { > > - struct drm_debugfs_entry *entry = m->private; > > - struct drm_device *dev = entry->dev; > > struct drm_printer p = drm_seq_file_printer(m); > > struct drm_client_dev *client; > > > > diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c > > index b4d2e7dd87f5..21f01c7d0ab1 100644 > > --- a/drivers/gpu/drm/drm_debugfs.c > > +++ b/drivers/gpu/drm/drm_debugfs.c > > @@ -49,10 +49,8 @@ > > * Initialization, etc. > > **/ > > > > -static int drm_name_info(struct seq_file *m, void *data) > > +static int drm_name_info(struct seq_file *m, struct drm_device
Re: DRM accel and debugfs/sysfs
On Wed, Feb 8, 2023 at 8:07 PM Daniel Vetter wrote: > > On Tue, Feb 07, 2023 at 01:17:47PM -0300, Maíra Canal wrote: > > On 2/7/23 12:43, Jeffrey Hugo wrote: > > > On 2/7/2023 4:31 AM, Maíra Canal wrote: > > > > Hi Stanislaw, > > > > > > > > On 2/1/23 12:20, Stanislaw Gruszka wrote: > > > > > Hi > > > > > > > > > > I was about to send debugfs support for ivpu and noticed that there > > > > > are current changes that deprecate drm_devel->debugfs_init callback. > > > > > > > > > > Further I looked at this commit [1], that stated we should not > > > > > use drm_minor for debugfs and sysfs. What is quite contrary to > > > > > what drm accel framework did in the first place. > > > > > > > > > > So my question is how we should use debugfs/sysfs in accel? > > > > > Use it with old fashioned minor-centric way or change > > > > > the framework somehow ? > > > > > > > > As we are trying to replace drm_debugfs_create_files() [1], it would > > > > be nice to see the accel debugfs support use the new debugfs API. This > > > > would mean using the debugfs_list from the drm_device, deprecating > > > > the debugfs_init callback, and adding the a similar code snippet to > > > > accel_debugfs_init: > > > > > > > > list_for_each_entry_safe(entry, tmp, >debugfs_list, list) { > > > > debugfs_create_file(entry->file.name, 0444, > > > > minor->debugfs_root, entry, _debugfs_entry_fops); > > > > list_del(>list); > > > > > > > > Maybe Daniel has some more thoughts on this matter, but I guess it > > > > would be better to drop the use of the old-fashioned minor-centric > > > > implementation in accel. > > It was a simple case of two things landing in parallel and not being > synchronized. Would be good if accel could be adapted to use the new > debugfs infra, now that both accel and the new debugfs stuff have landed. > -Daniel Yes, definitely. Does anyone volunteer to send a patch to align ? If not, we will do it internally and send a patch. Oded > > > > > > > > > [1] > > > > https://cgit.freedesktop.org/drm/drm-misc/tree/Documentation/gpu/todo.rst#n511 > > > > > > > > Best Regards, > > > > - Maíra Canal > > > > > > Thank you for the details Maira. It helps to explain what the todo is > > > suggesting. Is there an example of a driver/drm_device that uses > > > debugfs_list which you can easily point to? > > > > The implementation of this device-centered infrastructure is linked in [1] > > and an example of the conversion of debugfs APIs is linked in [2], and other > > drivers such as v3d, vkms, vc4 and gud use this new API as well. > > > > [1] > > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=1c9cacbea880513a896aee65a5c58007bcb55653 > > [2] > > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=2e3ab8a6994f265bbd4dbd00448b84548f18464c > > > > Best Regards, > > - Maíra Canal > > > > > > > > -Jeff > > > > > > > > > > > > > > > > > [1] > > > > > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=99845faae7099cd704ebf67514c1157c26960a26 > > > > > > > > > > Regards > > > > > Stanislaw > > > > > > > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
Re: [PATCH v3 5/6] drm/debugfs: Make the show callback pass the pointer to the right object
On Tue, Jan 31, 2023 at 04:58:25PM -0300, Maíra Canal wrote: > Currently, the drivers need to access the struct drm_debugfs_entry to > get the proper device on the show callback. There is no need for such > thing, as you can wrap the show callback in order to provide to the > driver the proper parameters: the struct seq_file, the struct drm_device > and the driver-specific data stored in the struct drm_debugfs_info. > > Therefore, make the show callback pass the pointer to the right object > in the parameters, which makes the API more type-safe. > > Signed-off-by: Maíra Canal > --- > drivers/gpu/drm/arm/hdlcd_drv.c | 8 ++-- > drivers/gpu/drm/drm_atomic.c | 4 +--- > drivers/gpu/drm/drm_client.c | 5 ++--- > drivers/gpu/drm/drm_debugfs.c | 25 - > drivers/gpu/drm/drm_framebuffer.c | 4 +--- > drivers/gpu/drm/drm_gem_vram_helper.c | 5 ++--- > drivers/gpu/drm/gud/gud_drv.c | 5 ++--- > drivers/gpu/drm/v3d/v3d_debugfs.c | 16 > drivers/gpu/drm/vc4/vc4_bo.c | 4 +--- > drivers/gpu/drm/vc4/vc4_debugfs.c | 6 ++ > drivers/gpu/drm/vc4/vc4_hdmi.c| 6 ++ > drivers/gpu/drm/vc4/vc4_hvs.c | 8 ++-- > drivers/gpu/drm/vc4/vc4_v3d.c | 4 +--- > drivers/gpu/drm/vkms/vkms_drv.c | 4 +--- > include/drm/drm_debugfs.h | 14 -- > 15 files changed, 43 insertions(+), 75 deletions(-) > > diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c > index e3507dd6f82a..b70bc7b11764 100644 > --- a/drivers/gpu/drm/arm/hdlcd_drv.c > +++ b/drivers/gpu/drm/arm/hdlcd_drv.c > @@ -193,10 +193,8 @@ static int hdlcd_setup_mode_config(struct drm_device > *drm) > } > > #ifdef CONFIG_DEBUG_FS > -static int hdlcd_show_underrun_count(struct seq_file *m, void *arg) > +static int hdlcd_show_underrun_count(struct seq_file *m, struct drm_device > *drm, void *arg) > { > - struct drm_debugfs_entry *entry = m->private; > - struct drm_device *drm = entry->dev; > struct hdlcd_drm_private *hdlcd = drm_to_hdlcd_priv(drm); > > seq_printf(m, "underrun : %d\n", > atomic_read(>buffer_underrun_count)); > @@ -206,10 +204,8 @@ static int hdlcd_show_underrun_count(struct seq_file *m, > void *arg) > return 0; > } > > -static int hdlcd_show_pxlclock(struct seq_file *m, void *arg) > +static int hdlcd_show_pxlclock(struct seq_file *m, struct drm_device *drm, > void *arg) > { > - struct drm_debugfs_entry *entry = m->private; > - struct drm_device *drm = entry->dev; > struct hdlcd_drm_private *hdlcd = drm_to_hdlcd_priv(drm); > unsigned long clkrate = clk_get_rate(hdlcd->clk); > unsigned long mode_clock = hdlcd->crtc.mode.crtc_clock * 1000; > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 5457c02ca1ab..38f140481fcc 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -1754,10 +1754,8 @@ void drm_state_dump(struct drm_device *dev, struct > drm_printer *p) > EXPORT_SYMBOL(drm_state_dump); > > #ifdef CONFIG_DEBUG_FS > -static int drm_state_info(struct seq_file *m, void *data) > +static int drm_state_info(struct seq_file *m, struct drm_device *dev, void > *data) > { > - struct drm_debugfs_entry *entry = m->private; > - struct drm_device *dev = entry->dev; > struct drm_printer p = drm_seq_file_printer(m); > > __drm_state_dump(dev, , true); > diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c > index 009e7b10455c..ec2e6bc3515d 100644 > --- a/drivers/gpu/drm/drm_client.c > +++ b/drivers/gpu/drm/drm_client.c > @@ -488,10 +488,9 @@ int drm_client_framebuffer_flush(struct > drm_client_buffer *buffer, struct drm_re > EXPORT_SYMBOL(drm_client_framebuffer_flush); > > #ifdef CONFIG_DEBUG_FS > -static int drm_client_debugfs_internal_clients(struct seq_file *m, void > *data) > +static int drm_client_debugfs_internal_clients(struct seq_file *m, struct > drm_device *dev, > +void *data) > { > - struct drm_debugfs_entry *entry = m->private; > - struct drm_device *dev = entry->dev; > struct drm_printer p = drm_seq_file_printer(m); > struct drm_client_dev *client; > > diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c > index b4d2e7dd87f5..21f01c7d0ab1 100644 > --- a/drivers/gpu/drm/drm_debugfs.c > +++ b/drivers/gpu/drm/drm_debugfs.c > @@ -49,10 +49,8 @@ > * Initialization, etc. > **/ > > -static int drm_name_info(struct seq_file *m, void *data) > +static int drm_name_info(struct seq_file *m, struct drm_device *dev, void > *data) > { > - struct drm_debugfs_entry *entry = m->private; > - struct drm_device *dev = entry->dev; > struct drm_master *master; > > mutex_lock(>master_mutex); > @@ -70,10 +68,8 @@ static int
Re: DRM accel and debugfs/sysfs
On Tue, Feb 07, 2023 at 01:17:47PM -0300, Maíra Canal wrote: > On 2/7/23 12:43, Jeffrey Hugo wrote: > > On 2/7/2023 4:31 AM, Maíra Canal wrote: > > > Hi Stanislaw, > > > > > > On 2/1/23 12:20, Stanislaw Gruszka wrote: > > > > Hi > > > > > > > > I was about to send debugfs support for ivpu and noticed that there > > > > are current changes that deprecate drm_devel->debugfs_init callback. > > > > > > > > Further I looked at this commit [1], that stated we should not > > > > use drm_minor for debugfs and sysfs. What is quite contrary to > > > > what drm accel framework did in the first place. > > > > > > > > So my question is how we should use debugfs/sysfs in accel? > > > > Use it with old fashioned minor-centric way or change > > > > the framework somehow ? > > > > > > As we are trying to replace drm_debugfs_create_files() [1], it would > > > be nice to see the accel debugfs support use the new debugfs API. This > > > would mean using the debugfs_list from the drm_device, deprecating > > > the debugfs_init callback, and adding the a similar code snippet to > > > accel_debugfs_init: > > > > > > list_for_each_entry_safe(entry, tmp, >debugfs_list, list) { > > > debugfs_create_file(entry->file.name, 0444, > > > minor->debugfs_root, entry, _debugfs_entry_fops); > > > list_del(>list); > > > > > > Maybe Daniel has some more thoughts on this matter, but I guess it > > > would be better to drop the use of the old-fashioned minor-centric > > > implementation in accel. It was a simple case of two things landing in parallel and not being synchronized. Would be good if accel could be adapted to use the new debugfs infra, now that both accel and the new debugfs stuff have landed. -Daniel > > > > > > [1] > > > https://cgit.freedesktop.org/drm/drm-misc/tree/Documentation/gpu/todo.rst#n511 > > > > > > Best Regards, > > > - Maíra Canal > > > > Thank you for the details Maira. It helps to explain what the todo is > > suggesting. Is there an example of a driver/drm_device that uses > > debugfs_list which you can easily point to? > > The implementation of this device-centered infrastructure is linked in [1] > and an example of the conversion of debugfs APIs is linked in [2], and other > drivers such as v3d, vkms, vc4 and gud use this new API as well. > > [1] > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=1c9cacbea880513a896aee65a5c58007bcb55653 > [2] > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=2e3ab8a6994f265bbd4dbd00448b84548f18464c > > Best Regards, > - Maíra Canal > > > > > -Jeff > > > > > > > > > > > > > [1] > > > > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=99845faae7099cd704ebf67514c1157c26960a26 > > > > > > > > Regards > > > > Stanislaw > > > > > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v3 1/6] drm/debugfs: Introduce wrapper for debugfs list
On Tue, Jan 31, 2023 at 04:58:21PM -0300, Maíra Canal wrote: > Introduce a struct wrapper for all the debugfs-related stuff: the list > of debugfs files and the mutex that protects it. This will make it > easier to initialize all the debugfs list in a DRM object and will > create a good abstraction for a possible implementation of the debugfs > infrastructure for KMS objects. > > Signed-off-by: Maíra Canal > --- > drivers/gpu/drm/drm_debugfs.c | 18 ++ > drivers/gpu/drm/drm_internal.h | 12 > include/drm/drm_debugfs.h | 16 > 3 files changed, 46 insertions(+) > > diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c > index 4f643a490dc3..8658d3929ea5 100644 > --- a/drivers/gpu/drm/drm_debugfs.c > +++ b/drivers/gpu/drm/drm_debugfs.c > @@ -218,6 +218,24 @@ void drm_debugfs_create_files(const struct drm_info_list > *files, int count, > } > EXPORT_SYMBOL(drm_debugfs_create_files); > > +struct drm_debugfs_files *drm_debugfs_files_init(void) > +{ > + struct drm_debugfs_files *debugfs_files; > + > + debugfs_files = kzalloc(sizeof(*debugfs_files), GFP_KERNEL); > + > + INIT_LIST_HEAD(_files->list); > + mutex_init(_files->mutex); > + > + return debugfs_files; > +} > + > +void drm_debugfs_files_destroy(struct drm_debugfs_files *debugfs_files) > +{ > + mutex_destroy(_files->mutex); > + kfree(debugfs_files); > +} > + > int drm_debugfs_init(struct drm_minor *minor, int minor_id, >struct dentry *root) > { > diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h > index ed2103ee272c..f1c8766ed828 100644 > --- a/drivers/gpu/drm/drm_internal.h > +++ b/drivers/gpu/drm/drm_internal.h > @@ -23,6 +23,7 @@ > > #include > > +#include > #include > #include > > @@ -183,6 +184,8 @@ int drm_gem_dumb_destroy(struct drm_file *file, struct > drm_device *dev, > > /* drm_debugfs.c drm_debugfs_crc.c */ > #if defined(CONFIG_DEBUG_FS) > +struct drm_debugfs_files *drm_debugfs_files_init(void); > +void drm_debugfs_files_destroy(struct drm_debugfs_files *debugfs_files); > int drm_debugfs_init(struct drm_minor *minor, int minor_id, >struct dentry *root); > void drm_debugfs_cleanup(struct drm_minor *minor); > @@ -193,6 +196,15 @@ void drm_debugfs_crtc_add(struct drm_crtc *crtc); > void drm_debugfs_crtc_remove(struct drm_crtc *crtc); > void drm_debugfs_crtc_crc_add(struct drm_crtc *crtc); > #else > +static inline struct drm_debugfs_files *drm_debugfs_files_init(void) > +{ > + return NULL; > +} > + > +static inline void drm_debugfs_files_destroy(struct drm_debugfs_files > *debugfs_files) > +{ > +} > + > static inline int drm_debugfs_init(struct drm_minor *minor, int minor_id, > struct dentry *root) > { > diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h > index 7616f457ce70..423aa3de506a 100644 > --- a/include/drm/drm_debugfs.h > +++ b/include/drm/drm_debugfs.h > @@ -32,6 +32,8 @@ > #ifndef _DRM_DEBUGFS_H_ > #define _DRM_DEBUGFS_H_ > > +#include > +#include > #include > #include > /** > @@ -79,6 +81,20 @@ struct drm_info_node { > struct dentry *dent; > }; > > +/** > + * struct drm_debugfs_files - Encapsulates the debugfs list and its mutex > + * > + * This structure represents the debugfs list of files and is encapsulated > + * with a mutex to protect the access of the list. > + */ > +struct drm_debugfs_files { > + /** @list: List of debugfs files to be created by the DRM object. */ > + struct list_head list; > + > + /** @mutex: Protects access. */ > + struct mutex mutex; I'm not seeing any use for the mutex here? Also unless you also plan to put like the debugfs directory pointers in this struct, I'm not sure we need this abstraction since it's purely internal to debugfs code (so also should really be in the headers where drivers could perhaps come up with funny ideas). -Daniel > +}; > + > /** > * struct drm_debugfs_info - debugfs info list entry > * > -- > 2.39.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH] drm/vmwgfx: Stop accessing buffer objects which failed init
From: Zack Rusin ttm_bo_init_reserved on failure puts the buffer object back which causes it to be deleted, but kfree was still being called on the same buffer in vmw_bo_create leading to a double free. After the double free the vmw_gem_object_create_with_handle was setting the gem function objects before checking the return status of vmw_bo_create leading to null pointer access. Fix the entire path by relaying on ttm_bo_init_reserved to delete the buffer objects on failure and making sure the return status is checked before setting the gem function objects on the buffer object. Signed-off-by: Zack Rusin Fixes: 8afa13a0583f ("drm/vmwgfx: Implement DRIVER_GEM") Cc: # v5.17+ --- drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 4 +++- drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c index 63486802c8fd..43ffa5c7acbd 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c @@ -431,13 +431,15 @@ int vmw_bo_create(struct vmw_private *vmw, return -ENOMEM; } + /* +* vmw_bo_init will delete the *p_bo object if it fails +*/ ret = vmw_bo_init(vmw, *p_bo, params, vmw_bo_free); if (unlikely(ret != 0)) goto out_error; return ret; out_error: - kfree(*p_bo); *p_bo = NULL; return ret; } diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c index f042e22b8b59..51bd1f8c5cc4 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c @@ -127,11 +127,11 @@ int vmw_gem_object_create_with_handle(struct vmw_private *dev_priv, }; ret = vmw_bo_create(dev_priv, , p_vbo); - - (*p_vbo)->tbo.base.funcs = _gem_object_funcs; if (ret != 0) goto out_no_bo; + (*p_vbo)->tbo.base.funcs = _gem_object_funcs; + ret = drm_gem_handle_create(filp, &(*p_vbo)->tbo.base, handle); /* drop reference from allocate - handle holds it now */ drm_gem_object_put(&(*p_vbo)->tbo.base); -- 2.38.1
Re: [PATCH v3 0/4] Reserve DSPPs based on user request
Hi, On Wed, Feb 8, 2023 at 5:42 AM Kalyan Thota wrote: > > This series will enable color features on sc7280 target which has > primary panel as eDP > > The series removes DSPP allocation based on encoder type and allows > the DSPP reservation based on user request via CTM. > > The series will release/reserve the dpu resources when ever there is > a topology change to suit the new requirements. > > Kalyan Thota (4): > drm/msm/dpu: clear DSPP reservations in rm release > drm/msm/dpu: add DSPPs into reservation upon a CTM request > drm/msm/dpu: avoid unnecessary check in DPU reservations > drm/msm/dpu: reserve the resources on topology change > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 2 + > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 58 > - > drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 2 + > 3 files changed, 37 insertions(+), 25 deletions(-) I tried out your changes, but unfortunately it seems like there's something wrong. :( I did this: 1. Picked your 5 patches to the chromeos-5.15 tree (this series plus [1]) 2. Put them on herobrine villager. 3. Booted up with no external display plugged in. 4. Tried to enable night light in the ChromeOS UI. 5. Night light didn't turn on for the internal display. I also tried applying them to the top of msm-next (had to resolve some small conflicts). Same thing, night light didn't work. I thought maybe this was because the Chrome browser hasn't been updated to properly use atomic_check for testing for night light, so I hacked my herobrine device tree to not mark "mdss_dp" as "okay". Now there's _only_ an eDP display. Same thing, night light didn't work. I could only get night light to work for the internal display if I plugged and unplugged an external display in. Is the above the behavior that's expected right now? [1] https://lore.kernel.org/all/1674814487-2112-1-git-send-email-quic_kaly...@quicinc.com/
Re: [PATCH 1/2] drm: Introduce plane SIZE_HINTS property
On 2/7/23 23:09, Ville Syrjala wrote: From: Ville Syrjälä Add a new immutable plane property by which a plane can advertise a handful of recommended plane sizes. This would be mostly exposed by cursor planes as a slightly more capable replacement for the DRM_CAP_CURSOR_WIDTH/HEIGHT caps, which can only declare a one size fits all limit for the whole device. Currently eg. amdgpu/i915/nouveau just advertize the max cursor size via the cursor size caps. But always using the max sized cursor can waste a surprising amount of power, so a better stragey is desirable. Most other drivers don't specify any cursor size at all, in which case the ioctl code just claims that 64x64 is a great choice. Whether that is actually true is debatable. A poll of various compositor developers informs us that blindly probing with setcursor/atomic ioctl to determine suitable cursor sizes is not acceptable, thus the introduction of the new property to supplant the cursor size caps. The compositor will now be free to select a more optimal cursor size from the short list of options. Note that the reported sizes (either via the property or the caps) make no claims about things such as plane scaling. So these things should only really be consulted for simple "cursor like" use cases. Cc: Simon Ser Cc: Jonas Ådahl Cc: Daniel Stone Cc: Pekka Paalanen Signed-off-by: Ville Syrjälä This would be great to have. amdgpu could take advantage of it as well. I didn't have a thorough look at the details of this implementation but like what it does, so this is Acked-by: Harry Wentland Harry --- drivers/gpu/drm/drm_mode_config.c | 7 +++ drivers/gpu/drm/drm_plane.c | 33 +++ include/drm/drm_mode_config.h | 5 + include/drm/drm_plane.h | 4 include/uapi/drm/drm_mode.h | 5 + 5 files changed, 54 insertions(+) diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index 87eb591fe9b5..21860f94a18c 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -374,6 +374,13 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.modifiers_property = prop; + prop = drm_property_create(dev, + DRM_MODE_PROP_IMMUTABLE | DRM_MODE_PROP_BLOB, + "SIZE_HINTS", 0); + if (!prop) + return -ENOMEM; + dev->mode_config.size_hints_property = prop; + return 0; } diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index 24e7998d1731..d0a277f4be1f 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -1582,3 +1582,36 @@ int drm_plane_create_scaling_filter_property(struct drm_plane *plane, return 0; } EXPORT_SYMBOL(drm_plane_create_scaling_filter_property); + +/** + * drm_plane_add_size_hint_property - create a size hint property + * + * @plane: drm plane + * @hints: size hints + * @num_hints: number of size hints + * + * Create a size hints property for the plane. + * + * RETURNS: + * Zero for success or -errno + */ +int drm_plane_add_size_hints_property(struct drm_plane *plane, + const struct drm_plane_size_hint *hints, + int num_hints) +{ + struct drm_device *dev = plane->dev; + struct drm_mode_config *config = >mode_config; + struct drm_property_blob *blob; + + blob = drm_property_create_blob(dev, + array_size(sizeof(hints[0]), num_hints), + hints); + if (IS_ERR(blob)) + return PTR_ERR(blob); + + drm_object_attach_property(>base, config->size_hints_property, + blob->base.id); + + return 0; +} +EXPORT_SYMBOL(drm_plane_add_size_hints_property); diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index e5b053001d22..ed9f6938dca1 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -949,6 +949,11 @@ struct drm_mode_config { */ struct drm_property *modifiers_property; + /** +* @size_hints_property: Plane SIZE_HINTS property. +*/ + struct drm_property *size_hints_property; + /* cursor size */ uint32_t cursor_width, cursor_height; diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index 51291983ea44..1997d7d64b69 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -32,6 +32,7 @@ #include struct drm_crtc; +struct drm_plane_size_hint; struct drm_printer; struct drm_modeset_acquire_ctx; @@ -945,5 +946,8 @@ drm_plane_get_damage_clips(const struct drm_plane_state *state); int drm_plane_create_scaling_filter_property(struct drm_plane *plane,
Re: [PATCH v2 8/8] arm64: dts: qcom: sm8350-hdk: enable GPU
On 6.02.2023 15:57, Dmitry Baryshkov wrote: > Enable the GPU on the SM8350-HDK device. The ZAP shader is required for > the GPU to function properly. > > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Konrad Dybcio Konrad > arch/arm64/boot/dts/qcom/sm8350-hdk.dts | 8 > 1 file changed, 8 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > index df841230d1b7..5e744423a673 100644 > --- a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > +++ b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > @@ -284,6 +284,14 @@ _dma1 { > status = "okay"; > }; > > + { > + status = "okay"; > + > + zap-shader { > + firmware-name = "qcom/sm8350/a660_zap.mbn"; > + }; > +}; > + > { > clock-frequency = <40>; > status = "okay";
[PATCH] habanalabs: change unused extern decl of hdev to forward decl of hl_device
Building with clang W=2 has several similar warnings drivers/accel/habanalabs/common/decoder.c:46:51: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] static void dec_error_intr_work(struct hl_device *hdev, u32 base_addr, u32 core_id) ^ drivers/accel/habanalabs/common/security.h:13:26: note: previous declaration is here extern struct hl_device *hdev; ^ There is no global definition of hdev, so the extern is not needed. Searched with grep -r '^struct' . | grep hl_dev Change to an forward decl to resolve these issues drivers/accel/habanalabs/common/mmu/../security.h:133:40: error: ‘struct hl_device’ declared inside parameter list will not be visible outside of this definition or declaration [-Werror] 133 | bool (*skip_block_hook)(struct hl_device *hdev, |^ Signed-off-by: Tom Rix --- drivers/accel/habanalabs/common/security.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/accel/habanalabs/common/security.h b/drivers/accel/habanalabs/common/security.h index 234b4a6ed8bc..d7a3b3e82ea4 100644 --- a/drivers/accel/habanalabs/common/security.h +++ b/drivers/accel/habanalabs/common/security.h @@ -10,7 +10,7 @@ #include -extern struct hl_device *hdev; +struct hl_device; /* special blocks */ #define HL_MAX_NUM_OF_GLBL_ERR_CAUSE 10 -- 2.26.3
Re: [PATCH 0/9] Panfrost: Improve and add MediaTek SoCs support
On 08/02/2023 10:37, AngeloGioacchino Del Regno wrote: > This series adds support for new MediaTek SoCs (MT8186/MT8192/MT8195) > and improves MT8183 support: since the mtk-regulator-coupler driver > was picked, it is now useless for Panfrost to look for, and manage, > two regulators (GPU Vcore and GPU SRAM) on MediaTek; Yay! ;) I never did like Panfrost dealing with two regulators. > The aforementioned driver will take care of keeping the voltage > relation (/constraints) of the two regulators on its own when a > voltage change request is sent to the Vcore, solving the old time > issue with not working DVFS on Panfrost+MediaTek (due to devfreq > supporting only single regulator). > > In the specific case of MT8183, in order to not break the ABI, it > was necessary to add a new compatible for enabling DVFS. It's a shame to need a new compatible, but it seems sensible to me. For the panfrost changes: Reviewed-by: Steven Price I'll let others comment on the DT binding changes. Thanks, Steve > Alyssa Rosenzweig (3): > drm/panfrost: Increase MAX_PM_DOMAINS to 5 > drm/panfrost: Add the MT8192 GPU ID > drm/panfrost: Add mediatek,mt8192-mali compatible > > AngeloGioacchino Del Regno (6): > dt-bindings: gpu: mali-bifrost: Don't allow sram-supply by default > dt-bindings: gpu: mali-bifrost: Allow up to 5 power domains for MT8192 > dt-bindings: gpu: mali-bifrost: Add compatible for MT8195 SoC > dt-bindings: gpu: mali-bifrost: Add new MT8183 compatible > dt-bindings: gpu: mali-bifrost: Add a compatible for MediaTek MT8186 > drm/panfrost: Add new compatible for Mali on the MT8183 SoC > > .../bindings/gpu/arm,mali-bifrost.yaml| 68 +-- > drivers/gpu/drm/panfrost/panfrost_device.h| 2 +- > drivers/gpu/drm/panfrost/panfrost_drv.c | 28 > drivers/gpu/drm/panfrost/panfrost_gpu.c | 8 +++ > 4 files changed, 99 insertions(+), 7 deletions(-) >
[PATCH 3/4] drm/vmwgfx: handle NULL bo->resource in move callback
The ttm bo now initially has NULL bo->resource, and leaves the driver the handle that. However it looks like we forgot to handle that for vmwgfx. It looks like this will just null-ptr-deref in vmw_move(), if bo->resource is NULL. Fix this by calling move_null() if the new resource is TTM_PL_SYSTEM, otherwise do the multi-hop sequence to ensure can safely call into ttm_bo_move_memcpy(), since it might also need to clear the memory. This should give the same behaviour as before. Fixes: 180253782038 ("drm/ttm: stop allocating dummy resources during BO creation") Signed-off-by: Matthew Auld Cc: Christian König --- drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c index 856a352a72a6..c598c5a9fe2c 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c @@ -596,10 +596,23 @@ static int vmw_move(struct ttm_buffer_object *bo, struct ttm_resource *new_mem, struct ttm_place *hop) { - struct ttm_resource_manager *old_man = ttm_manager_type(bo->bdev, bo->resource->mem_type); + struct ttm_resource_manager *old_man; struct ttm_resource_manager *new_man = ttm_manager_type(bo->bdev, new_mem->mem_type); int ret; + if (!bo->resource) { + if (new_mem->mem_type != TTM_PL_SYSTEM) { + hop->mem_type = TTM_PL_SYSTEM; + hop->flags = TTM_PL_FLAG_TEMPORARY; + return -EMULTIHOP; + } + + ttm_bo_move_null(bo, new_mem); + return 0; + } + + old_man = ttm_manager_type(bo->bdev, bo->resource->mem_type); + if (new_man->use_tt && !vmw_memtype_is_system(new_mem->mem_type)) { ret = vmw_ttm_bind(bo->bdev, bo->ttm, new_mem); if (ret) -- 2.39.1
[PATCH 4/4] drm/radeon: handle NULL bo->resource in move callback
The ttm bo now initially has NULL bo->resource, and leaves the driver the handle that. However it looks like we forgot to handle that for radeon. It looks like this will just null-ptr-deref in radeon_bo_move(), if bo->resource is NULL. Fix this by calling move_null(). Fixes: 180253782038 ("drm/ttm: stop allocating dummy resources during BO creation") Signed-off-by: Matthew Auld Cc: Christian König --- drivers/gpu/drm/radeon/radeon_ttm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index 67075c85f847..2220cdf6a3f6 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -213,7 +213,8 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, rbo = container_of(bo, struct radeon_bo, tbo); rdev = radeon_get_rdev(bo->bdev); - if (old_mem->mem_type == TTM_PL_SYSTEM && bo->ttm == NULL) { + if (!old_mem || (old_mem->mem_type == TTM_PL_SYSTEM && +bo->ttm == NULL)) { ttm_bo_move_null(bo, new_mem); goto out; } -- 2.39.1
[PATCH 2/4] drm/qxl: handle NULL bo->resource in move callback
The ttm bo now initially has NULL bo->resource, and leaves the driver the handle that. However it looks like we forgot to handle that for qxl. It looks like this will just null-ptr-deref in qxl_bo_move(), if bo->resource is NULL. Fix this by calling move_null() if the new resource is TTM_PL_SYSTEM, otherwise do the multi-hop sequence to ensure can safely call into ttm_bo_move_memcpy(), since it might also need to clear the memory. This should give the same behaviour as before. Fixes: 180253782038 ("drm/ttm: stop allocating dummy resources during BO creation") Signed-off-by: Matthew Auld Cc: Christian König --- drivers/gpu/drm/qxl/qxl_ttm.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c index a92a5b0d4c25..1a82629bce3f 100644 --- a/drivers/gpu/drm/qxl/qxl_ttm.c +++ b/drivers/gpu/drm/qxl/qxl_ttm.c @@ -143,6 +143,17 @@ static int qxl_bo_move(struct ttm_buffer_object *bo, bool evict, struct ttm_resource *old_mem = bo->resource; int ret; + if (!old_mem) { + if (new_mem->mem_type != TTM_PL_SYSTEM) { + hop->mem_type = TTM_PL_SYSTEM; + hop->flags = TTM_PL_FLAG_TEMPORARY; + return -EMULTIHOP; + } + + ttm_bo_move_null(bo, new_mem); + return 0; + } + qxl_bo_move_notify(bo, new_mem); ret = ttm_bo_wait_ctx(bo, ctx); -- 2.39.1
[PATCH 1/4] drm/gem-vram: handle NULL bo->resource in move callback
The ttm BO now initially has NULL bo->resource, and leaves the driver the handle that. However it looks like we forgot to handle that for ttm_bo_move_memcpy() users, like with vram-gem, since it just silently returns zero. This seems to then trigger warnings like: WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/drm_gem_vram_helper.c:255 drm_gem_vram_offset (??:?) Fix this by calling move_null() if the new resource is TTM_PL_SYSTEM, otherwise do the multi-hop sequence to ensure can safely call into ttm_bo_move_memcpy(), since it might also need to clear the memory. This should give the same behaviour as before. While we are here let's also treat calling ttm_bo_move_memcpy() with NULL bo->resource as programmer error, where expectation is that upper layers should now handle it. Fixes: 180253782038 ("drm/ttm: stop allocating dummy resources during BO creation") Signed-off-by: Matthew Auld Cc: Christian König --- drivers/gpu/drm/drm_gem_vram_helper.c | 11 +++ drivers/gpu/drm/ttm/ttm_bo_util.c | 4 ++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index d40b3edb52d0..0bea3df2a16d 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -916,6 +916,17 @@ static int bo_driver_move(struct ttm_buffer_object *bo, { struct drm_gem_vram_object *gbo; + if (!bo->resource) { + if (new_mem->mem_type != TTM_PL_SYSTEM) { + hop->mem_type = TTM_PL_SYSTEM; + hop->flags = TTM_PL_FLAG_TEMPORARY; + return -EMULTIHOP; + } + + ttm_bo_move_null(bo, new_mem); + return 0; + } + gbo = drm_gem_vram_of_bo(bo); return drm_gem_vram_bo_driver_move(gbo, evict, ctx, new_mem); diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index d9d2b0903b22..fd9fd3d15101 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -157,8 +157,8 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo, bool clear; int ret = 0; - if (!src_mem) - return 0; + if (WARN_ON(!src_mem)) + return -EINVAL; src_man = ttm_manager_type(bdev, src_mem->mem_type); if (ttm && ((ttm->page_flags & TTM_TT_FLAG_SWAPPED) || -- 2.39.1
Re: [PATCH 3/3] drm/connector: Deprecate split for BT.2020 in drm_colorspace enum
On Wed, Feb 08, 2023 at 11:18:42AM +0200, Pekka Paalanen wrote: > On Fri, 3 Feb 2023 16:02:51 +0200 > Ville Syrjälä wrote: > > > On Fri, Feb 03, 2023 at 02:52:50PM +0100, Sebastian Wick wrote: > > > On Fri, Feb 3, 2023 at 2:35 PM Ville Syrjälä > > > wrote: > > > > > > > > On Fri, Feb 03, 2023 at 01:59:07PM +0100, Sebastian Wick wrote: > > > > > On Fri, Feb 3, 2023 at 11:40 AM Ville Syrjälä > > > > > wrote: > > > > > > > > > > > > On Fri, Feb 03, 2023 at 02:07:44AM +, Joshua Ashton wrote: > > > > > > > Userspace has no way of controlling or knowing the pixel encoding > > > > > > > currently, so there is no way for it to ever get the right values > > > > > > > here. > > > > > > > > > > > > That applies to a lot of the other values as well (they are > > > > > > explicitly RGB or YCC). The idea was that this property sets the > > > > > > infoframe/MSA/SDP value exactly, and other properties should be > > > > > > added to for use userspace to control the pixel encoding/colorspace > > > > > > conversion(if desired, or userspace just makes sure to > > > > > > directly feed in correct kind of data). > > > > > > > > > > I'm all for getting userspace control over pixel encoding but even > > > > > then the kernel always knows which pixel encoding is selected and > > > > > which InfoFrame has to be sent. Is there a reason why userspace would > > > > > want to control the variant explicitly to the wrong value? > > > > > > > > What do you mean wrong value? Userspace sets it based on what > > > > kind of data it has generated (or asked the display hardware > > > > to generate if/when we get explicit control over that part). > > > > > > Wrong in the sense of sending the YCC variant when the pixel encoding > > > is RGB for example. > > > > > > Maybe I'm missing something here but my assumption is that the kernel > > > always has to know the pixel encoding anyway. The color pipeline also > > > assumes that the pixel values are RGB. User space might be able to > > > generate YCC content but for subsampling etc the pixel encoding still > > > has to be explicitly set. > > > > The kernel doesn't really know much atm. In theory you can just > > configure the thing to do a straight passthough and put anything you > > want into your pixels. > > But it's impossible to use a YCbCr framebuffer and have that *not* > converted to RGB for the KMS color pipeline even if userspace wanted it > to be strictly pass-through, only to be converted again to YCbCr for > the cable, is it not? > > Even more so with 4:2:0. > > How could it be possible to stop the driver from doing those two > YUV-to-RGB and RGB-to-YCbCr conversions at the beginning and at the end > of the KMS color pipeline? You can stop the conversion at the start of the pipeline by using a "RGB" framebuffer. At the end of the pipe it's not possible with the current props. -- Ville Syrjälä Intel
Re: [PATCH v3 4/4] drm/msm/dpu: reserve the resources on topology change
On 08/02/2023 15:42, Kalyan Thota wrote: Some features like CTM can be enabled dynamically. Release and reserve the DPU resources whenever a topology change occurs such that required hw blocks are allocated appropriately. Signed-off-by: Kalyan Thota --- Changes in v1: - Avoid mode_set call directly (Dmitry) Changes in v2: - Minor nits (Dmitry) Changes in v3: - avoid unnecessary modeset check call (Dmitry) --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 2 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 24 +++- 2 files changed, 21 insertions(+), 5 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
[bug report] drm/i915/mtl: Add hardware-level lock for steering
Hello Matt Roper, The patch 3100240bf846: "drm/i915/mtl: Add hardware-level lock for steering" from Nov 28, 2022, leads to the following Smatch static checker warning: drivers/gpu/drm/i915/gt/intel_gt_mcr.c:379 intel_gt_mcr_lock() warn: sleeping in atomic context CALL TREE: intel_engine_reset() <- disables preempt intel_gt_handle_error() <- disables preempt live_hold_reset() <- disables preempt reset_virtual_engine() <- disables preempt -> __intel_engine_reset_bh() -> intel_engine_resume() -> intel_engine_apply_workarounds() -> wa_list_apply() -> intel_gt_mcr_lock() drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c:769 intel_guc_ct_send() warn: sleeping in atomic context CALL TREE: guc_submission_tasklet() <- disables preempt -> guc_dequeue_one_context() guc_submit_request() <- disables preempt -> guc_bypass_tasklet_submit() -> guc_add_request() -> __guc_add_request() <- disables preempt -> intel_guc_send_nb() -> intel_guc_ct_send() drivers/gpu/drm/i915/gt/intel_gt_mcr.c 366 void intel_gt_mcr_lock(struct intel_gt *gt, unsigned long *flags) 367 { 368 unsigned long __flags; 369 int err = 0; 370 371 lockdep_assert_not_held(>uncore->lock); 372 373 /* 374 * Starting with MTL, we need to coordinate not only with other 375 * driver threads, but also with hardware/firmware agents. A dedicated 376 * locking register is used. 377 */ 378 if (GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 70)) --> 379 err = wait_for(intel_uncore_read_fw(gt->uncore, 380 MTL_STEER_SEMAPHORE) == 0x1, 100); The wait_for() macro sleeps. But we can't sleep if preempt is disabled. The code in the caller looks likes: drivers/gpu/drm/i915/gt/selftest_execlists.c 626 err = engine_lock_reset_tasklet(engine); ^^ local_bh_disable() bumps the preempt_count. 627 if (err) 628 goto out; 629 630 engine->sched_engine->tasklet.callback(>sched_engine->tasklet); 631 GEM_BUG_ON(execlists_active(>execlists) != rq); 632 633 i915_request_get(rq); 634 execlists_hold(engine, rq); 635 GEM_BUG_ON(!i915_request_on_hold(rq)); 636 637 __intel_engine_reset_bh(engine, NULL); ^^ Sleeping. 638 GEM_BUG_ON(rq->fence.error != -EIO); 639 640 engine_unlock_reset_tasklet(engine); preempt enabled again. 641 642 /* Check that we do not resubmit the held request */ regards, dan carpenter
Re: remove arch/sh
> Yes, that's the plan. We're collecting the various patches people have sent > in for arch/sh, review and test them and apply them. > > My test board is running the latest kernel now, so I can test new patches, > too. I am just witnessing this development, but I want to say thanks for your effort and congrats on your progress. Looks like you do the right things correctly, cool! Kudos also to Geert and others for their assistance. signature.asc Description: PGP signature
[PATCH v3 4/4] drm/msm/dpu: reserve the resources on topology change
Some features like CTM can be enabled dynamically. Release and reserve the DPU resources whenever a topology change occurs such that required hw blocks are allocated appropriately. Signed-off-by: Kalyan Thota --- Changes in v1: - Avoid mode_set call directly (Dmitry) Changes in v2: - Minor nits (Dmitry) Changes in v3: - avoid unnecessary modeset check call (Dmitry) --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 2 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 24 +++- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h index 539b68b..85bd5645 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h @@ -204,6 +204,7 @@ struct dpu_crtc { * @hw_ctls : List of active ctl paths * @crc_source: CRC source * @crc_frame_skip_count: Number of frames skipped before getting CRC + * @ctm_enabled : Cached color management enablement state */ struct dpu_crtc_state { struct drm_crtc_state base; @@ -225,6 +226,7 @@ struct dpu_crtc_state { enum dpu_crtc_crc_source crc_source; int crc_frame_skip_count; + bool ctm_enabled; }; #define to_dpu_crtc_state(x) \ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 3920efd..038e077 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -217,6 +217,19 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = { 15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10 }; +static bool _dpu_enc_is_dspp_changed(struct drm_crtc_state *crtc_state, + struct msm_display_topology topology) +{ + struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc_state); + + if ((cstate->ctm_enabled && !topology.num_dspp) || + (!cstate->ctm_enabled && topology.num_dspp)) { + crtc_state->mode_changed = true; + return true; + } + + return false; +} bool dpu_encoder_is_widebus_enabled(const struct drm_encoder *drm_enc) { @@ -642,14 +655,15 @@ static int dpu_encoder_virt_atomic_check( topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state); + _dpu_enc_is_dspp_changed(crtc_state, topology); + /* * Release and Allocate resources on every modeset -* Dont allocate when active is false. */ if (drm_atomic_crtc_needs_modeset(crtc_state)) { dpu_rm_release(global_state, drm_enc); - if (!crtc_state->active_changed || crtc_state->active) + if (crtc_state->enable) ret = dpu_rm_reserve(_kms->rm, global_state, drm_enc, crtc_state, topology); } @@ -1022,7 +1036,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_ENC]; struct dpu_hw_blk *hw_dspp[MAX_CHANNELS_PER_ENC] = { NULL }; struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC]; - int num_lm, num_ctl, num_pp, num_dsc; + int num_lm, num_ctl, num_pp, num_dsc, num_dspp; unsigned int dsc_mask = 0; int i; @@ -1053,7 +1067,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, drm_enc->base.id, DPU_HW_BLK_CTL, hw_ctl, ARRAY_SIZE(hw_ctl)); num_lm = dpu_rm_get_assigned_resources(_kms->rm, global_state, drm_enc->base.id, DPU_HW_BLK_LM, hw_lm, ARRAY_SIZE(hw_lm)); - dpu_rm_get_assigned_resources(_kms->rm, global_state, + num_dspp = dpu_rm_get_assigned_resources(_kms->rm, global_state, drm_enc->base.id, DPU_HW_BLK_DSPP, hw_dspp, ARRAY_SIZE(hw_dspp)); @@ -1084,7 +1098,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, } cstate->num_mixers = num_lm; - + cstate->ctm_enabled = !!num_dspp; dpu_enc->connector = conn_state->connector; for (i = 0; i < dpu_enc->num_phys_encs; i++) { -- 2.7.4
[PATCH v3 2/4] drm/msm/dpu: add DSPPs into reservation upon a CTM request
Add DSPP blocks into the topology for reservation, if there is a CTM request for that composition. Signed-off-by: Kalyan Thota Reviewed-by: Dmitry Baryshkov --- Changes in v1: - Minor nits (Dmitry) Changes in v2: - Populate DSPPs into the reservation only if CTM is requested (Dmitry) --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 15 ++- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 9c6817b..46d2a5c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -545,7 +545,8 @@ bool dpu_encoder_use_dsc_merge(struct drm_encoder *drm_enc) static struct msm_display_topology dpu_encoder_get_topology( struct dpu_encoder_virt *dpu_enc, struct dpu_kms *dpu_kms, - struct drm_display_mode *mode) + struct drm_display_mode *mode, + struct drm_crtc_state *crtc_state) { struct msm_display_topology topology = {0}; int i, intf_count = 0; @@ -563,8 +564,7 @@ static struct msm_display_topology dpu_encoder_get_topology( * 1 LM, 1 INTF * 2 LM, 1 INTF (stream merge to support high resolution interfaces) * -* Adding color blocks only to primary interface if available in -* sufficient number +* Add dspps to the reservation requirements if ctm is requested */ if (intf_count == 2) topology.num_lm = 2; @@ -573,11 +573,8 @@ static struct msm_display_topology dpu_encoder_get_topology( else topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1; - if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI) { - if (dpu_kms->catalog->dspp && - (dpu_kms->catalog->dspp_count >= topology.num_lm)) - topology.num_dspp = topology.num_lm; - } + if (crtc_state->ctm) + topology.num_dspp = topology.num_lm; topology.num_enc = 0; topology.num_intf = intf_count; @@ -643,7 +640,7 @@ static int dpu_encoder_virt_atomic_check( } } - topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode); + topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state); /* Reserve dynamic resources now. */ if (!ret) { -- 2.7.4
[PATCH v3 1/4] drm/msm/dpu: clear DSPP reservations in rm release
Clear DSPP reservations from the global state during rm release Fixes: e47616df008b ("drm/msm/dpu: add support for color processing blocks in dpu driver") Signed-off-by: Kalyan Thota Reviewed-by: Dmitry Baryshkov Reviewed-by: Marijn Suijten --- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c index 73b3442..718ea0a 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c @@ -572,6 +572,8 @@ void dpu_rm_release(struct dpu_global_state *global_state, ARRAY_SIZE(global_state->ctl_to_enc_id), enc->base.id); _dpu_rm_clear_mapping(global_state->dsc_to_enc_id, ARRAY_SIZE(global_state->dsc_to_enc_id), enc->base.id); + _dpu_rm_clear_mapping(global_state->dspp_to_enc_id, + ARRAY_SIZE(global_state->dspp_to_enc_id), enc->base.id); } int dpu_rm_reserve( -- 2.7.4
[PATCH v3 3/4] drm/msm/dpu: avoid unnecessary check in DPU reservations
Return immediately on failure, this will make dpu reservations part look cleaner. Signed-off-by: Kalyan Thota Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 23 ++- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 46d2a5c..3920efd 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -636,25 +636,22 @@ static int dpu_encoder_virt_atomic_check( if (ret) { DPU_ERROR_ENC(dpu_enc, "mode unsupported, phys idx %d\n", i); - break; + return ret; } } topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state); - /* Reserve dynamic resources now. */ - if (!ret) { - /* -* Release and Allocate resources on every modeset -* Dont allocate when active is false. -*/ - if (drm_atomic_crtc_needs_modeset(crtc_state)) { - dpu_rm_release(global_state, drm_enc); + /* +* Release and Allocate resources on every modeset +* Dont allocate when active is false. +*/ + if (drm_atomic_crtc_needs_modeset(crtc_state)) { + dpu_rm_release(global_state, drm_enc); - if (!crtc_state->active_changed || crtc_state->active) - ret = dpu_rm_reserve(_kms->rm, global_state, - drm_enc, crtc_state, topology); - } + if (!crtc_state->active_changed || crtc_state->active) + ret = dpu_rm_reserve(_kms->rm, global_state, + drm_enc, crtc_state, topology); } trace_dpu_enc_atomic_check_flags(DRMID(drm_enc), adj_mode->flags); -- 2.7.4
[PATCH v3 0/4] Reserve DSPPs based on user request
This series will enable color features on sc7280 target which has primary panel as eDP The series removes DSPP allocation based on encoder type and allows the DSPP reservation based on user request via CTM. The series will release/reserve the dpu resources when ever there is a topology change to suit the new requirements. Kalyan Thota (4): drm/msm/dpu: clear DSPP reservations in rm release drm/msm/dpu: add DSPPs into reservation upon a CTM request drm/msm/dpu: avoid unnecessary check in DPU reservations drm/msm/dpu: reserve the resources on topology change drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 2 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 58 - drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 2 + 3 files changed, 37 insertions(+), 25 deletions(-) -- 2.7.4
Re: [PATCH 1/2] drm: Introduce plane SIZE_HINTS property
On Wed, Feb 08, 2023 at 02:13:12PM +0200, Pekka Paalanen wrote: > On Wed, 8 Feb 2023 06:09:10 +0200 > Ville Syrjala wrote: > > > From: Ville Syrjälä > > > > Add a new immutable plane property by which a plane can advertise > > a handful of recommended plane sizes. This would be mostly exposed > > by cursor planes as a slightly more capable replacement for > > the DRM_CAP_CURSOR_WIDTH/HEIGHT caps, which can only declare > > a one size fits all limit for the whole device. > > > > Currently eg. amdgpu/i915/nouveau just advertize the max cursor > > size via the cursor size caps. But always using the max sized > > cursor can waste a surprising amount of power, so a better > > stragey is desirable. > > > > Most other drivers don't specify any cursor size at all, in > > which case the ioctl code just claims that 64x64 is a great > > choice. Whether that is actually true is debatable. > > > > A poll of various compositor developers informs us that > > blindly probing with setcursor/atomic ioctl to determine > > suitable cursor sizes is not acceptable, thus the > > introduction of the new property to supplant the cursor > > size caps. The compositor will now be free to select a > > more optimal cursor size from the short list of options. > > > > Note that the reported sizes (either via the property or the > > caps) make no claims about things such as plane scaling. So > > these things should only really be consulted for simple > > "cursor like" use cases. > > > > Cc: Simon Ser > > Cc: Jonas Ådahl > > Cc: Daniel Stone > > Cc: Pekka Paalanen > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/drm_mode_config.c | 7 +++ > > drivers/gpu/drm/drm_plane.c | 33 +++ > > include/drm/drm_mode_config.h | 5 + > > include/drm/drm_plane.h | 4 > > include/uapi/drm/drm_mode.h | 5 + > > 5 files changed, 54 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_mode_config.c > > b/drivers/gpu/drm/drm_mode_config.c > > index 87eb591fe9b5..21860f94a18c 100644 > > --- a/drivers/gpu/drm/drm_mode_config.c > > +++ b/drivers/gpu/drm/drm_mode_config.c > > @@ -374,6 +374,13 @@ static int drm_mode_create_standard_properties(struct > > drm_device *dev) > > return -ENOMEM; > > dev->mode_config.modifiers_property = prop; > > > > + prop = drm_property_create(dev, > > + DRM_MODE_PROP_IMMUTABLE | DRM_MODE_PROP_BLOB, > > + "SIZE_HINTS", 0); > > + if (!prop) > > + return -ENOMEM; > > + dev->mode_config.size_hints_property = prop; > > + > > return 0; > > } > > > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > > index 24e7998d1731..d0a277f4be1f 100644 > > --- a/drivers/gpu/drm/drm_plane.c > > +++ b/drivers/gpu/drm/drm_plane.c > > @@ -1582,3 +1582,36 @@ int drm_plane_create_scaling_filter_property(struct > > drm_plane *plane, > > return 0; > > } > > EXPORT_SYMBOL(drm_plane_create_scaling_filter_property); > > + > > +/** > > + * drm_plane_add_size_hint_property - create a size hint property > > + * > > + * @plane: drm plane > > + * @hints: size hints > > + * @num_hints: number of size hints > > + * > > + * Create a size hints property for the plane. > > + * > > + * RETURNS: > > + * Zero for success or -errno > > + */ > > +int drm_plane_add_size_hints_property(struct drm_plane *plane, > > + const struct drm_plane_size_hint *hints, > > + int num_hints) > > +{ > > + struct drm_device *dev = plane->dev; > > + struct drm_mode_config *config = >mode_config; > > + struct drm_property_blob *blob; > > + > > + blob = drm_property_create_blob(dev, > > + array_size(sizeof(hints[0]), num_hints), > > + hints); > > + if (IS_ERR(blob)) > > + return PTR_ERR(blob); > > + > > + drm_object_attach_property(>base, config->size_hints_property, > > + blob->base.id); > > + > > + return 0; > > +} > > +EXPORT_SYMBOL(drm_plane_add_size_hints_property); > > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h > > index e5b053001d22..ed9f6938dca1 100644 > > --- a/include/drm/drm_mode_config.h > > +++ b/include/drm/drm_mode_config.h > > @@ -949,6 +949,11 @@ struct drm_mode_config { > > */ > > struct drm_property *modifiers_property; > > > > + /** > > +* @size_hints_property: Plane SIZE_HINTS property. > > +*/ > > + struct drm_property *size_hints_property; > > + > > /* cursor size */ > > uint32_t cursor_width, cursor_height; > > > > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h > > index 51291983ea44..1997d7d64b69 100644 > > --- a/include/drm/drm_plane.h > > +++ b/include/drm/drm_plane.h > > @@ -32,6 +32,7 @@ > > #include > > > > struct drm_crtc; > > +struct drm_plane_size_hint; > > struct
Re: [drm-misc:drm-misc-next] [drm/ttm] 1802537820: WARNING:at_drivers/gpu/drm/drm_gem_vram_helper.c:#drm_gem_vram_offset
On Wed, 8 Feb 2023 at 12:41, Christian König wrote: > > Am 08.02.23 um 10:38 schrieb Matthew Auld: > > On Wed, 8 Feb 2023 at 08:32, Christian König > > wrote: > >> Hey guys, > >> > >> I'm pretty sure this is a bug in bochs which happens to surface because > >> of a recent TTM change, we have seen similar problems in the past with > >> this driver. > >> > >> What happens is that userspace tries to bind a BO to a CRTC before the > >> BO has even a backing store. > >> > >> Any idea how to fix this? I can just remove the warning, but that's not > >> really a good solution. > > IIUC this driver is just using ttm_bo_move_memcpy() underneath for its > > bo_move callback, which looks to be doing: > > > > if (!bo->resource) > > return 0; > > > > Which doesn't make any sense to me.There should at least be a > > move_null(), and maybe also a multi-hop to handle clearing. Otherwise > > bo->resource is likely always NULL (and we hit the above warning), > > even after the dummy move. What do you think? > > Oh, good point. That should indeed be move_null(). > > Do you want to write a patch or should I take care of this? I can try to type that. > > Thanks for pointing that out, > Christian. > > > > >> Regards, > >> Christian. > >> > >> Am 08.02.23 um 05:32 schrieb kernel test robot: > >>> Greeting, > >>> > >>> FYI, we noticed > >>> WARNING:at_drivers/gpu/drm/drm_gem_vram_helper.c:#drm_gem_vram_offset due > >>> to commit (built with gcc-11): > >>> > >>> commit: 1802537820389183dfcd814e0f6a60d1496a75ef ("drm/ttm: stop > >>> allocating dummy resources during BO creation") > >>> git://anongit.freedesktop.org/drm/drm-misc drm-misc-next > >>> > >>> in testcase: boot > >>> > >>> on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 > >>> -m 16G > >>> > >>> caused below changes (please refer to attached dmesg/kmsg for entire > >>> log/backtrace): > >>> > >>> > >>> If you fix the issue, kindly add following tag > >>> | Reported-by: kernel test robot > >>> | Link: > >>> https://lore.kernel.org/oe-lkp/202302081038.984b8c1-oliver.s...@intel.com > >>> > >>> > >>> [ 25.994992][T1] [ cut here ] > >>> [ 25.995050][ T1] WARNING: CPU: 0 PID: 1 at > >>> drivers/gpu/drm/drm_gem_vram_helper.c:255 drm_gem_vram_offset (??:?) > >>> [ 25.995080][T1] Modules linked in: > >>> [ 25.995100][T1] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G > >>> T 6.2.0-rc6-01191-g180253782038 #1 > >>> a8db67375c3ac749313dafaec43f39836e38fae9 > >>> [ 25.995117][T1] Hardware name: QEMU Standard PC (i440FX + PIIX, > >>> 1996), BIOS 1.16.0-debian-1.16.0-5 04/01/2014 > >>> [ 25.995128][ T1] RIP: 0010:drm_gem_vram_offset (??:?) > >>> [ 25.995144][ T1] Code: 02 00 00 b8 ff ff 37 00 48 89 fa 48 c1 e0 2a 48 > >>> c1 ea 03 80 3c 02 00 74 05 e8 7f 1f eb fe 48 8b 9b 20 02 00 00 48 85 db > >>> 75 06 <0f> 0b 31 c0 eb 4b 48 8d 7b 10 b8 ff ff 37 00 48 89 fa 48 c1 e0 2a > >>> All code > >>> > >>> 0:02 00 add(%rax),%al > >>> 2:00 b8 ff ff 37 00 add%bh,0x37(%rax) > >>> 8:48 89 famov%rdi,%rdx > >>> b:48 c1 e0 2a shl$0x2a,%rax > >>> f:48 c1 ea 03 shr$0x3,%rdx > >>> 13:80 3c 02 00 cmpb $0x0,(%rdx,%rax,1) > >>> 17:74 05 je 0x1e > >>> 19:e8 7f 1f eb fe callq 0xfeeb1f9d > >>> 1e:48 8b 9b 20 02 00 00mov0x220(%rbx),%rbx > >>> 25:48 85 dbtest %rbx,%rbx > >>> 28:75 06 jne0x30 > >>> 2a:* 0f 0b ud2 <-- trapping > >>> instruction > >>> 2c:31 c0 xor%eax,%eax > >>> 2e:eb 4b jmp0x7b > >>> 30:48 8d 7b 10 lea0x10(%rbx),%rdi > >>> 34:b8 ff ff 37 00 mov$0x37,%eax > >>> 39:48 89 famov%rdi,%rdx > >>> 3c:48 c1 e0 2a shl$0x2a,%rax > >>> > >>> Code starting with the faulting instruction > >>> === > >>> 0:0f 0b ud2 > >>> 2:31 c0 xor%eax,%eax > >>> 4:eb 4b jmp0x51 > >>> 6:48 8d 7b 10 lea0x10(%rbx),%rdi > >>> a:b8 ff ff 37 00 mov$0x37,%eax > >>> f:48 89 famov%rdi,%rdx > >>> 12:48 c1 e0 2a shl$0x2a,%rax > >>> [ 25.995156][T1] RSP: :c901f028 EFLAGS: 00210246 > >>> [ 25.995174][T1] RAX: dc00 RBX: RCX: > >>> > >>> [ 25.995186][T1] RDX: 111026dee544 RSI: 8881372d4b10 RDI: > >>> 888136f72a20 > >>> [ 25.995196][T1]
Re: [Intel-gfx] [PATCH 4/4] drm/i915/selftest: Use forcewake to sanity check engine wa lists
On Tue, Feb 07, 2023 at 07:37:58PM -0300, Gustavo Sousa wrote: > On Wed, Feb 01, 2023 at 02:28:31PM -0800, Matt Roper wrote: > > Although register information in the bspec includes a field that is > > supposed to reflect a register's reset characteristics (i.e., whether a > > register maintains its value through engine resets), it's been > > discovered that this information is incorrect for some register ranges > > (i.e., registers that are not affected by engine resets are tagged in a > > way that indicates they would be). > > > > We can sanity check workaround registers placed on the RCS/CCS engine > > workaround lists (including those placed there via the > > general_render_compute_wa_init() function) by comparing against the > > forcewake table. As far as we know, there's never a case where a > > register that lives outside the RENDER powerwell will be reset by an > > RCS/CCS engine reset. > > > > Signed-off-by: Matt Roper > > --- > > .../gpu/drm/i915/gt/selftest_workarounds.c| 52 +++ > > 1 file changed, 52 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c > > b/drivers/gpu/drm/i915/gt/selftest_workarounds.c > > index 14a8b25b6204..1bc8febc5c1d 100644 > > --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c > > +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c > > @@ -1362,12 +1362,64 @@ live_engine_reset_workarounds(void *arg) > > return ret; > > } > > > > +/* > > + * The bspec's documentation for register reset behavior can be unreliable > > for > > + * some MMIO ranges. But in general we do not expect registers outside the > > + * RENDER forcewake domain to be reset by RCS/CCS engine resets. If we > > find > > + * workaround registers on an RCS or CCS engine's list, it likely indicates > > I think "workaround registers" is too general and makes the sentence a bit > confusing. I believe you mean those registers mentioned in the previous > sentence, right? Maybe s/workaround registers/said registers/? > > > + * the register is misdocumented in the bspec and the workaround > > implementation > > + * should be moved to the GT workaround list instead. > > + */ > > +static int > > +live_check_engine_workarounds_fw(void *arg) > > +{ > > + struct intel_gt *gt = arg; > > + struct intel_engine_cs *engine; > > + struct wa_lists *lists; > > + enum intel_engine_id id; > > + int ret = 0; > > + > > + lists = kzalloc(sizeof(*lists), GFP_KERNEL); > > + if (!lists) > > + return -ENOMEM; > > + > > + reference_lists_init(gt, lists); > > + > > + for_each_engine(engine, gt, id) { > > + struct i915_wa_list *wal = >engine[id].wa_list; > > + struct i915_wa *wa; > > + int i; > > + > > + if (engine->class != RENDER_CLASS && > > + engine->class != COMPUTE_CLASS) > > + continue; > > + > > + for (i = 0, wa = wal->list; i < wal->count; i++, wa++) { > > + enum forcewake_domains fw; > > + > > + fw = intel_uncore_forcewake_for_reg(gt->uncore, wa->reg, > > + FW_REG_READ | > > FW_REG_WRITE); > > + if ((fw & FORCEWAKE_RENDER) == 0) { > > + pr_err("%s: Register %#x not in RENDER > > forcewake domain!\n", > > + engine->name, > > i915_mmio_reg_offset(wa->reg)); > > I think it is safer to use the correct member (wa->reg vs wa->mcr_reg) > according > to the value of wa->is_mcr. Coincidently the reg member for both types have > the > same offset in the struct, but I do not think we should rely on that. > > One issue is that, unlike i915_mmio_reg_offset(), > intel_uncore_forcewake_for_reg() is not implemented with generics and expects > i915_reg_t. A workaround here would be "converting" the wa->mcr_reg (when > wa->is_mcr holds) an i915_reg_t by copying the correct fields. While this is > trivial since both types have only one field, I think the proper way > (future-proof) of doing that is by having a dedicated function/macro to do the > transformation. Ah, we already have that: mcr_reg_cast() :-) So my suggestion is: i915_reg_t reg = wa->is_mcr ? mcr_reg_cast(wa->mcr_reg) : wa->reg; Ans use reg as argument for both intel_uncore_forcewake_for_reg() and i915_mmio_reg_offset(). > > Thinking about an alternative approach, do you think we could say that > i915_mcr_reg_t will always have the same fields as i915_reg_t? In that case, > we > could tweak the type definition (or at least formalize via code documentation) > to reflect that and then it would be okay to always use wa->reg here, as > i915_mcr_reg_t would be thought as a subclass of i915_reg_t. > > > + ret = -EINVAL; > > + } > > + } > > + } > > + > > + reference_lists_fini(gt, lists); > > + kfree(lists); > > + > > + return ret; > > +} > > + > > int
Re: [drm-misc:drm-misc-next] [drm/ttm] 1802537820: WARNING:at_drivers/gpu/drm/drm_gem_vram_helper.c:#drm_gem_vram_offset
Am 08.02.23 um 10:38 schrieb Matthew Auld: On Wed, 8 Feb 2023 at 08:32, Christian König wrote: Hey guys, I'm pretty sure this is a bug in bochs which happens to surface because of a recent TTM change, we have seen similar problems in the past with this driver. What happens is that userspace tries to bind a BO to a CRTC before the BO has even a backing store. Any idea how to fix this? I can just remove the warning, but that's not really a good solution. IIUC this driver is just using ttm_bo_move_memcpy() underneath for its bo_move callback, which looks to be doing: if (!bo->resource) return 0; Which doesn't make any sense to me.There should at least be a move_null(), and maybe also a multi-hop to handle clearing. Otherwise bo->resource is likely always NULL (and we hit the above warning), even after the dummy move. What do you think? Oh, good point. That should indeed be move_null(). Do you want to write a patch or should I take care of this? Thanks for pointing that out, Christian. Regards, Christian. Am 08.02.23 um 05:32 schrieb kernel test robot: Greeting, FYI, we noticed WARNING:at_drivers/gpu/drm/drm_gem_vram_helper.c:#drm_gem_vram_offset due to commit (built with gcc-11): commit: 1802537820389183dfcd814e0f6a60d1496a75ef ("drm/ttm: stop allocating dummy resources during BO creation") git://anongit.freedesktop.org/drm/drm-misc drm-misc-next in testcase: boot on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace): If you fix the issue, kindly add following tag | Reported-by: kernel test robot | Link: https://lore.kernel.org/oe-lkp/202302081038.984b8c1-oliver.s...@intel.com [ 25.994992][T1] [ cut here ] [ 25.995050][ T1] WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/drm_gem_vram_helper.c:255 drm_gem_vram_offset (??:?) [ 25.995080][T1] Modules linked in: [ 25.995100][T1] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G T 6.2.0-rc6-01191-g180253782038 #1 a8db67375c3ac749313dafaec43f39836e38fae9 [ 25.995117][T1] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-debian-1.16.0-5 04/01/2014 [ 25.995128][ T1] RIP: 0010:drm_gem_vram_offset (??:?) [ 25.995144][ T1] Code: 02 00 00 b8 ff ff 37 00 48 89 fa 48 c1 e0 2a 48 c1 ea 03 80 3c 02 00 74 05 e8 7f 1f eb fe 48 8b 9b 20 02 00 00 48 85 db 75 06 <0f> 0b 31 c0 eb 4b 48 8d 7b 10 b8 ff ff 37 00 48 89 fa 48 c1 e0 2a All code 0:02 00 add(%rax),%al 2:00 b8 ff ff 37 00 add%bh,0x37(%rax) 8:48 89 famov%rdi,%rdx b:48 c1 e0 2a shl$0x2a,%rax f:48 c1 ea 03 shr$0x3,%rdx 13:80 3c 02 00 cmpb $0x0,(%rdx,%rax,1) 17:74 05 je 0x1e 19:e8 7f 1f eb fe callq 0xfeeb1f9d 1e:48 8b 9b 20 02 00 00mov0x220(%rbx),%rbx 25:48 85 dbtest %rbx,%rbx 28:75 06 jne0x30 2a:* 0f 0b ud2 <-- trapping instruction 2c:31 c0 xor%eax,%eax 2e:eb 4b jmp0x7b 30:48 8d 7b 10 lea0x10(%rbx),%rdi 34:b8 ff ff 37 00 mov$0x37,%eax 39:48 89 famov%rdi,%rdx 3c:48 c1 e0 2a shl$0x2a,%rax Code starting with the faulting instruction === 0:0f 0b ud2 2:31 c0 xor%eax,%eax 4:eb 4b jmp0x51 6:48 8d 7b 10 lea0x10(%rbx),%rdi a:b8 ff ff 37 00 mov$0x37,%eax f:48 89 famov%rdi,%rdx 12:48 c1 e0 2a shl$0x2a,%rax [ 25.995156][T1] RSP: :c901f028 EFLAGS: 00210246 [ 25.995174][T1] RAX: dc00 RBX: RCX: [ 25.995186][T1] RDX: 111026dee544 RSI: 8881372d4b10 RDI: 888136f72a20 [ 25.995196][T1] RBP: c901f030 R08: R09: [ 25.995206][T1] R10: R11: R12: 8881372d4b00 [ 25.995215][T1] R13: 888136e9ee00 R14: 888136f4a060 R15: 0500 [ 25.995225][T1] FS: () GS:8883aee0() knlGS: [ 25.995236][T1] CS: 0010 DS: ES: CR0: 80050033 [ 25.995247][T1] CR2: f7fa1cd4 CR3: 06015000 CR4: 000406b0 [ 25.995262][T1] Call Trace: [ 25.995271][T1] [ 25.995287][ T1] bochs_plane_update (bochs.c:?) [ 25.995308][ T1] ?
Re: remove arch/sh
Hi Huacei! On Wed, 2023-02-08 at 20:24 +0800, Huacai Chen wrote: > Emm, maybe this patch has its chance to be merged now. :) > > https://lore.kernel.org/linux-sh/caahv-h6siotvkzpks4aabejgzcqtwp3tiha0+0hgz1+mu3x...@mail.gmail.com/T/#u Yes, that's the plan. We're collecting the various patches people have sent in for arch/sh, review and test them and apply them. My test board is running the latest kernel now, so I can test new patches, too. Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer `. `' Physicist `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Re: remove arch/sh
Emm, maybe this patch has its chance to be merged now. :) https://lore.kernel.org/linux-sh/caahv-h6siotvkzpks4aabejgzcqtwp3tiha0+0hgz1+mu3x...@mail.gmail.com/T/#u Huacai On Wed, Feb 8, 2023 at 8:14 PM John Paul Adrian Glaubitz wrote: > > Hi Randy! > > On Tue, 2023-02-07 at 17:31 -0800, Randy Dunlap wrote: > > > > On 2/7/23 01:06, John Paul Adrian Glaubitz wrote: > > > Hello Christoph! > > > > > > On Fri, 2023-02-03 at 08:14 +0100, Christoph Hellwig wrote: > > > > On Mon, Jan 16, 2023 at 09:52:10AM +0100, John Paul Adrian Glaubitz > > > > wrote: > > > > > We have had a discussion between multiple people invested in the > > > > > SuperH port and > > > > > I have decided to volunteer as a co-maintainer of the port to support > > > > > Rich Felker > > > > > when he isn't available. > > > > > > > > So, this still isn't reflected in MAINTAINERS in linux-next. When > > > > do you plan to take over? What platforms will remain supported and > > > > what can we start dropping due to being unused and unmaintained? > > > > > > I'm getting everything ready now with Geert's help and I have a probably > > > dumb > > > question regarding the MAINTAINERS file change: Shall I just add myself > > > as an > > > additional maintainer first or shall I also drop Yoshinori Sato? > > > > > > Also, is it desirable to add a "T:" entry for the kernel tree? > > > > Yes, definitely. > > Geert has suggested to wait with adding a tree source to the entry until I > get my > own kernel.org account. I have enough GPG signatures from multiple kernel > developers > on my GPG key, so I think it shouldn't be too difficult to qualify for an > account. > > Adrian > > -- > .''`. John Paul Adrian Glaubitz > : :' : Debian Developer > `. `' Physicist > `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Re: [PATCH 3/6] drm/ttm: Change the meaning of resource->start from pfn to bytes
That finally starts to look sane. I'm going to make a few more adjustments and then send this out. Christian. Am 08.02.23 um 10:01 schrieb Somalapuram Amaranath: Change resource->start from pfn to bytes to allow allocating objects smaller than a page. Change all DRM drivers using ttm_resource start and size pfn to bytes. Change amdgpu_res_first() cur->start, cur->size from pfn to bytes. Replacing ttm_resource resource->start field with cursor.start. Change amdgpu_gtt_mgr_new() allocation from pfn to bytes. Signed-off-by: Somalapuram Amaranath --- drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 13 +++-- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 4 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h | 8 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 10 +++--- .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 6 +- drivers/gpu/drm/drm_gem_vram_helper.c | 2 +- drivers/gpu/drm/nouveau/nouveau_bo.c| 13 ++--- drivers/gpu/drm/nouveau/nouveau_bo0039.c| 4 ++-- drivers/gpu/drm/nouveau/nouveau_mem.c | 10 +- drivers/gpu/drm/nouveau/nouveau_ttm.c | 2 +- drivers/gpu/drm/nouveau/nv17_fence.c| 2 +- drivers/gpu/drm/nouveau/nv50_fence.c| 2 +- drivers/gpu/drm/qxl/qxl_drv.h | 2 +- drivers/gpu/drm/qxl/qxl_object.c| 2 +- drivers/gpu/drm/qxl/qxl_ttm.c | 5 ++--- drivers/gpu/drm/radeon/radeon_object.c | 6 +++--- drivers/gpu/drm/radeon/radeon_object.h | 2 +- drivers/gpu/drm/radeon/radeon_ttm.c | 13 ++--- drivers/gpu/drm/radeon/radeon_vm.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 4 ++-- drivers/gpu/drm/vmwgfx/vmwgfx_cmd.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 3 +-- 23 files changed, 63 insertions(+), 56 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c index 44367f03316f..a1fbfc5984d8 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c @@ -116,7 +116,6 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man, struct ttm_resource **res) { struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man); - uint32_t num_pages = PFN_UP(tbo->base.size); struct ttm_range_mgr_node *node; int r; @@ -134,8 +133,10 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man, if (place->lpfn) { spin_lock(>lock); r = drm_mm_insert_node_in_range(>mm, >mm_nodes[0], - num_pages, tbo->page_alignment, - 0, place->fpfn, place->lpfn, + tbo->base.size, + tbo->page_alignment << PAGE_SHIFT, 0, + place->fpfn << PAGE_SHIFT, + place->lpfn << PAGE_SHIFT, DRM_MM_INSERT_BEST); spin_unlock(>lock); if (unlikely(r)) @@ -144,7 +145,7 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man, node->base.start = node->mm_nodes[0].start; } else { node->mm_nodes[0].start = 0; - node->mm_nodes[0].size = PFN_UP(node->base.size); + node->mm_nodes[0].size = node->base.size; node->base.start = AMDGPU_BO_INVALID_OFFSET; } @@ -285,8 +286,8 @@ int amdgpu_gtt_mgr_init(struct amdgpu_device *adev, uint64_t gtt_size) ttm_resource_manager_init(man, >mman.bdev, gtt_size); - start = AMDGPU_GTT_MAX_TRANSFER_SIZE * AMDGPU_GTT_NUM_TRANSFER_WINDOWS; - size = (adev->gmc.gart_size >> PAGE_SHIFT) - start; + start = (AMDGPU_GTT_MAX_TRANSFER_SIZE * AMDGPU_GTT_NUM_TRANSFER_WINDOWS) << PAGE_SHIFT; + size = adev->gmc.gart_size - start; drm_mm_init(>mm, start, size); spin_lock_init(>lock); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index d835ee2131d2..f5d5eee09cea 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -1488,9 +1488,11 @@ u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo) u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo) { struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev); + struct amdgpu_res_cursor cursor; uint64_t offset; - offset = (bo->tbo.resource->start << PAGE_SHIFT) + + amdgpu_res_first(bo->tbo.resource, 0,
Re: remove arch/sh
Hi Randy! On Tue, 2023-02-07 at 17:31 -0800, Randy Dunlap wrote: > > On 2/7/23 01:06, John Paul Adrian Glaubitz wrote: > > Hello Christoph! > > > > On Fri, 2023-02-03 at 08:14 +0100, Christoph Hellwig wrote: > > > On Mon, Jan 16, 2023 at 09:52:10AM +0100, John Paul Adrian Glaubitz wrote: > > > > We have had a discussion between multiple people invested in the SuperH > > > > port and > > > > I have decided to volunteer as a co-maintainer of the port to support > > > > Rich Felker > > > > when he isn't available. > > > > > > So, this still isn't reflected in MAINTAINERS in linux-next. When > > > do you plan to take over? What platforms will remain supported and > > > what can we start dropping due to being unused and unmaintained? > > > > I'm getting everything ready now with Geert's help and I have a probably > > dumb > > question regarding the MAINTAINERS file change: Shall I just add myself as > > an > > additional maintainer first or shall I also drop Yoshinori Sato? > > > > Also, is it desirable to add a "T:" entry for the kernel tree? > > Yes, definitely. Geert has suggested to wait with adding a tree source to the entry until I get my own kernel.org account. I have enough GPG signatures from multiple kernel developers on my GPG key, so I think it shouldn't be too difficult to qualify for an account. Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer `. `' Physicist `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Re: [PATCH 1/2] drm: Introduce plane SIZE_HINTS property
On Wed, 8 Feb 2023 06:09:10 +0200 Ville Syrjala wrote: > From: Ville Syrjälä > > Add a new immutable plane property by which a plane can advertise > a handful of recommended plane sizes. This would be mostly exposed > by cursor planes as a slightly more capable replacement for > the DRM_CAP_CURSOR_WIDTH/HEIGHT caps, which can only declare > a one size fits all limit for the whole device. > > Currently eg. amdgpu/i915/nouveau just advertize the max cursor > size via the cursor size caps. But always using the max sized > cursor can waste a surprising amount of power, so a better > stragey is desirable. > > Most other drivers don't specify any cursor size at all, in > which case the ioctl code just claims that 64x64 is a great > choice. Whether that is actually true is debatable. > > A poll of various compositor developers informs us that > blindly probing with setcursor/atomic ioctl to determine > suitable cursor sizes is not acceptable, thus the > introduction of the new property to supplant the cursor > size caps. The compositor will now be free to select a > more optimal cursor size from the short list of options. > > Note that the reported sizes (either via the property or the > caps) make no claims about things such as plane scaling. So > these things should only really be consulted for simple > "cursor like" use cases. > > Cc: Simon Ser > Cc: Jonas Ådahl > Cc: Daniel Stone > Cc: Pekka Paalanen > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/drm_mode_config.c | 7 +++ > drivers/gpu/drm/drm_plane.c | 33 +++ > include/drm/drm_mode_config.h | 5 + > include/drm/drm_plane.h | 4 > include/uapi/drm/drm_mode.h | 5 + > 5 files changed, 54 insertions(+) > > diff --git a/drivers/gpu/drm/drm_mode_config.c > b/drivers/gpu/drm/drm_mode_config.c > index 87eb591fe9b5..21860f94a18c 100644 > --- a/drivers/gpu/drm/drm_mode_config.c > +++ b/drivers/gpu/drm/drm_mode_config.c > @@ -374,6 +374,13 @@ static int drm_mode_create_standard_properties(struct > drm_device *dev) > return -ENOMEM; > dev->mode_config.modifiers_property = prop; > > + prop = drm_property_create(dev, > +DRM_MODE_PROP_IMMUTABLE | DRM_MODE_PROP_BLOB, > +"SIZE_HINTS", 0); > + if (!prop) > + return -ENOMEM; > + dev->mode_config.size_hints_property = prop; > + > return 0; > } > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > index 24e7998d1731..d0a277f4be1f 100644 > --- a/drivers/gpu/drm/drm_plane.c > +++ b/drivers/gpu/drm/drm_plane.c > @@ -1582,3 +1582,36 @@ int drm_plane_create_scaling_filter_property(struct > drm_plane *plane, > return 0; > } > EXPORT_SYMBOL(drm_plane_create_scaling_filter_property); > + > +/** > + * drm_plane_add_size_hint_property - create a size hint property > + * > + * @plane: drm plane > + * @hints: size hints > + * @num_hints: number of size hints > + * > + * Create a size hints property for the plane. > + * > + * RETURNS: > + * Zero for success or -errno > + */ > +int drm_plane_add_size_hints_property(struct drm_plane *plane, > + const struct drm_plane_size_hint *hints, > + int num_hints) > +{ > + struct drm_device *dev = plane->dev; > + struct drm_mode_config *config = >mode_config; > + struct drm_property_blob *blob; > + > + blob = drm_property_create_blob(dev, > + array_size(sizeof(hints[0]), num_hints), > + hints); > + if (IS_ERR(blob)) > + return PTR_ERR(blob); > + > + drm_object_attach_property(>base, config->size_hints_property, > +blob->base.id); > + > + return 0; > +} > +EXPORT_SYMBOL(drm_plane_add_size_hints_property); > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h > index e5b053001d22..ed9f6938dca1 100644 > --- a/include/drm/drm_mode_config.h > +++ b/include/drm/drm_mode_config.h > @@ -949,6 +949,11 @@ struct drm_mode_config { >*/ > struct drm_property *modifiers_property; > > + /** > + * @size_hints_property: Plane SIZE_HINTS property. > + */ > + struct drm_property *size_hints_property; > + > /* cursor size */ > uint32_t cursor_width, cursor_height; > > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h > index 51291983ea44..1997d7d64b69 100644 > --- a/include/drm/drm_plane.h > +++ b/include/drm/drm_plane.h > @@ -32,6 +32,7 @@ > #include > > struct drm_crtc; > +struct drm_plane_size_hint; > struct drm_printer; > struct drm_modeset_acquire_ctx; > > @@ -945,5 +946,8 @@ drm_plane_get_damage_clips(const struct drm_plane_state > *state); > > int drm_plane_create_scaling_filter_property(struct drm_plane *plane, >
Re: [PATCH 2/6] drm/amdgpu: Remove TTM resource->start visible VRAM condition
Am 08.02.23 um 10:01 schrieb Somalapuram Amaranath: Use amdgpu_bo_in_cpu_visible_vram() instead. Signed-off-by: Somalapuram Amaranath --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 981010de0a28..d835ee2131d2 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -600,7 +600,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev, if (!amdgpu_gmc_vram_full_visible(>gmc) && bo->tbo.resource->mem_type == TTM_PL_VRAM && - bo->tbo.resource->start < adev->gmc.visible_vram_size >> PAGE_SHIFT) + amdgpu_bo_in_cpu_visible_vram(bo)) amdgpu_cs_report_moved_bytes(adev, ctx.bytes_moved, ctx.bytes_moved); else @@ -1346,7 +1346,6 @@ vm_fault_t amdgpu_bo_fault_reserve_notify(struct ttm_buffer_object *bo) struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev); struct ttm_operation_ctx ctx = { false, false }; struct amdgpu_bo *abo = ttm_to_amdgpu_bo(bo); - unsigned long offset; int r; /* Remember that this BO was accessed by the CPU */ @@ -1355,8 +1354,7 @@ vm_fault_t amdgpu_bo_fault_reserve_notify(struct ttm_buffer_object *bo) if (bo->resource->mem_type != TTM_PL_VRAM) return 0; - offset = bo->resource->start << PAGE_SHIFT; - if ((offset + bo->base.size) <= adev->gmc.visible_vram_size) + if (amdgpu_bo_in_cpu_visible_vram(abo)) return 0; /* Can't move a pinned BO to visible VRAM */ @@ -1378,10 +1376,9 @@ vm_fault_t amdgpu_bo_fault_reserve_notify(struct ttm_buffer_object *bo) else if (unlikely(r)) return VM_FAULT_SIGBUS; - offset = bo->resource->start << PAGE_SHIFT; /* this should never happen */ if (bo->resource->mem_type == TTM_PL_VRAM && - (offset + bo->base.size) > adev->gmc.visible_vram_size) + amdgpu_bo_in_cpu_visible_vram(abo)) This check needs to be inversed. E.g. we return the error if the BO is not in visible VRAM. Apart from that the patch looks good to me. Regards, Christian. return VM_FAULT_SIGBUS; ttm_bo_move_to_lru_tail_unlocked(bo);
Re: [PATCH] drm: Rename headers to match DP2.1 spec
On Mon, Feb 06, 2023 at 02:37:08PM -0500, jdhillon wrote: > This patch changes the headers defined in drm_dp.h to match > the DP 2.1 spec. > > Signed-off-by: Jasdeep Dhillon > --- > drivers/gpu/drm/tegra/dp.c | 2 +- > include/drm/display/drm_dp.h | 13 +++-- > 2 files changed, 8 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/tegra/dp.c b/drivers/gpu/drm/tegra/dp.c > index 08fbd8f151a1..f33e468ece0a 100644 > --- a/drivers/gpu/drm/tegra/dp.c > +++ b/drivers/gpu/drm/tegra/dp.c > @@ -499,7 +499,7 @@ static int drm_dp_link_apply_training(struct drm_dp_link > *link) > for (i = 0; i < lanes; i++) > values[i / 2] |= DP_LANE_POST_CURSOR(i, pc[i]); > > - err = drm_dp_dpcd_write(aux, DP_TRAINING_LANE0_1_SET2, values, > + err = drm_dp_dpcd_write(aux, DP_LINK_SQUARE_PATTERN, values, > DIV_ROUND_UP(lanes, 2)); > if (err < 0) { > DRM_ERROR("failed to set post-cursor: %d\n", err); > diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h > index ed10e6b6f99d..2093c1f8d8e0 100644 > --- a/include/drm/display/drm_dp.h > +++ b/include/drm/display/drm_dp.h > @@ -641,12 +641,11 @@ > # define DP_LINK_QUAL_PATTERN_CUSTOM0x40 > # define DP_LINK_QUAL_PATTERN_SQUARE0x48 > > -#define DP_TRAINING_LANE0_1_SET2 0x10f > -#define DP_TRAINING_LANE2_3_SET2 0x110 > -# define DP_LANE02_POST_CURSOR2_SET_MASK(3 << 0) > -# define DP_LANE02_MAX_POST_CURSOR2_REACHED (1 << 2) > -# define DP_LANE13_POST_CURSOR2_SET_MASK(3 << 4) > -# define DP_LANE13_MAX_POST_CURSOR2_REACHED (1 << 6) > +#define DP_LINK_SQUARE_PATTERN 0x10f I think it'd be better to add new definitions for these rather than replace the existing ones. DP v1.2 calls these TRAINING_LANE0_1_SET2 and TRAINING_LANE2_3_SET2, respectively, so within the context of DP v1.2 hardware the new names don't make any sense. While at it, maybe add a comment to the new definitions that the old ones were deprecated in DP v1.3 and have been repurposed in v2.0 and v2.1, respectively. Thierry signature.asc Description: PGP signature
Re: [PATCH] media: Fix building pdfdocs
Em Wed, 8 Feb 2023 11:11:34 +0200 Laurent Pinchart escreveu: > Hi Tomi, > > Thank you for the patch. > > On Wed, Feb 08, 2023 at 10:29:16AM +0200, Tomi Valkeinen wrote: > > Commit 8d0e3fc61abd ("media: Add 2-10-10-10 RGB formats") added > > documatation for a few new RGB formats. For some reason these break the > > s/documatation/documentation/ > > > pdfdocs build, even if the same style seems to work elsewhere in the > > file. > > > > Remove the trailing empty dash lines, which seems to fix the issue. > > > > Fixes: 8d0e3fc61abd ("media: Add 2-10-10-10 RGB formats") > > Reported-by: Akira Yokosawa > > Signed-off-by: Tomi Valkeinen > > Reviewed-by: Laurent Pinchart > > > --- > > > > Note: the offending patch was merged via drm tree, so we may want to > > apply the fix to the drm tree also. > > Sounds good to me. Mauro, could you ack this patch ? Acked-by: Mauro Carvalho Chehab > > > Documentation/userspace-api/media/v4l/pixfmt-rgb.rst | 3 --- > > 1 file changed, 3 deletions(-) > > > > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-rgb.rst > > b/Documentation/userspace-api/media/v4l/pixfmt-rgb.rst > > index d330aeb4d3eb..ea545ed1aeaa 100644 > > --- a/Documentation/userspace-api/media/v4l/pixfmt-rgb.rst > > +++ b/Documentation/userspace-api/media/v4l/pixfmt-rgb.rst > > @@ -868,7 +868,6 @@ number of bits for each component. > >- r\ :sub:`4` > >- r\ :sub:`3` > >- r\ :sub:`2` > > - - > > * .. _V4L2-PIX-FMT-RGBA1010102: > > > >- ``V4L2_PIX_FMT_RGBA1010102`` > > @@ -909,7 +908,6 @@ number of bits for each component. > >- r\ :sub:`4` > >- r\ :sub:`3` > >- r\ :sub:`2` > > - - > > * .. _V4L2-PIX-FMT-ARGB2101010: > > > >- ``V4L2_PIX_FMT_ARGB2101010`` > > @@ -950,7 +948,6 @@ number of bits for each component. > >- r\ :sub:`6` > >- r\ :sub:`5` > >- r\ :sub:`4` > > - - > > > > .. raw:: latex > > > Thanks, Mauro
[PATCH 9/9] drm/panfrost: Add new compatible for Mali on the MT8183 SoC
The "mediatek,mt8183-mali" compatible uses platform data that calls for getting (and managing) two regulators ("mali" and "sram") but devfreq does not support this usecase, resulting in DVFS not working. Since a lot of MediaTek SoCs need to set the voltages for the GPU SRAM regulator in a specific relation to the GPU VCORE regulator, a MediaTek SoC specific driver was introduced to automatically satisfy, through coupling, these constraints: this means that there is at all no need to manage both regulators in panfrost but to otherwise just manage the main "mali" (-> gpu vcore) regulator instead. Keeping in mind that we cannot break the ABI, the most sensible route (avoiding hacks and uselessly overcomplicated code) to get a MT8183 node with one power supply was to add a new "mediatek,mt8183b-mali" compatible, which effectively deprecates the former. Signed-off-by: AngeloGioacchino Del Regno --- drivers/gpu/drm/panfrost/panfrost_drv.c | 17 + 1 file changed, 17 insertions(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index 5d25e77e1037..14cdeaeeb5c4 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -654,6 +654,14 @@ static const struct panfrost_compatible amlogic_data = { .vendor_quirk = panfrost_gpu_amlogic_quirk, }; +/* + * The old data with two power supplies for MT8183 is here only to + * keep retro-compatibility with older devicetrees, as DVFS will + * not work with this one. + * + * On new devicetrees please use the _b variant with a single and + * coupled regulators instead. + */ static const char * const mediatek_mt8183_supplies[] = { "mali", "sram", NULL }; static const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1", "core2" }; static const struct panfrost_compatible mediatek_mt8183_data = { @@ -663,6 +671,14 @@ static const struct panfrost_compatible mediatek_mt8183_data = { .pm_domain_names = mediatek_mt8183_pm_domains, }; +static const char * const mediatek_mt8183_b_supplies[] = { "mali", NULL }; +static const struct panfrost_compatible mediatek_mt8183_b_data = { + .num_supplies = ARRAY_SIZE(mediatek_mt8183_b_supplies) - 1, + .supply_names = mediatek_mt8183_b_supplies, + .num_pm_domains = ARRAY_SIZE(mediatek_mt8183_pm_domains), + .pm_domain_names = mediatek_mt8183_pm_domains, +}; + static const char * const mediatek_mt8192_supplies[] = { "mali", NULL }; static const char * const mediatek_mt8192_pm_domains[] = { "core0", "core1", "core2", "core3", "core4" }; @@ -691,6 +707,7 @@ static const struct of_device_id dt_match[] = { { .compatible = "arm,mali-bifrost", .data = _data, }, { .compatible = "arm,mali-valhall-jm", .data = _data, }, { .compatible = "mediatek,mt8183-mali", .data = _mt8183_data }, + { .compatible = "mediatek,mt8183b-mali", .data = _mt8183_b_data }, { .compatible = "mediatek,mt8192-mali", .data = _mt8192_data }, {} }; -- 2.39.1
[PATCH 8/9] drm/panfrost: Add mediatek,mt8192-mali compatible
From: Alyssa Rosenzweig Required for Mali-G57 on the Mediatek MT8192 and MT8195, which uses even more power domains than the MT8183 before it. Signed-off-by: Alyssa Rosenzweig [Angelo: Removed unneeded "sram" supply, added mt8195 to commit description] Co-developed-by: AngeloGioacchino Del Regno Signed-off-by: AngeloGioacchino Del Regno --- drivers/gpu/drm/panfrost/panfrost_drv.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index abb0dadd8f63..5d25e77e1037 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -663,6 +663,16 @@ static const struct panfrost_compatible mediatek_mt8183_data = { .pm_domain_names = mediatek_mt8183_pm_domains, }; +static const char * const mediatek_mt8192_supplies[] = { "mali", NULL }; +static const char * const mediatek_mt8192_pm_domains[] = { "core0", "core1", "core2", + "core3", "core4" }; +static const struct panfrost_compatible mediatek_mt8192_data = { + .num_supplies = ARRAY_SIZE(mediatek_mt8192_supplies) - 1, + .supply_names = mediatek_mt8192_supplies, + .num_pm_domains = ARRAY_SIZE(mediatek_mt8192_pm_domains), + .pm_domain_names = mediatek_mt8192_pm_domains, +}; + static const struct of_device_id dt_match[] = { /* Set first to probe before the generic compatibles */ { .compatible = "amlogic,meson-gxm-mali", @@ -681,6 +691,7 @@ static const struct of_device_id dt_match[] = { { .compatible = "arm,mali-bifrost", .data = _data, }, { .compatible = "arm,mali-valhall-jm", .data = _data, }, { .compatible = "mediatek,mt8183-mali", .data = _mt8183_data }, + { .compatible = "mediatek,mt8192-mali", .data = _mt8192_data }, {} }; MODULE_DEVICE_TABLE(of, dt_match); -- 2.39.1
[PATCH 3/9] dt-bindings: gpu: mali-bifrost: Add compatible for MT8195 SoC
The MediaTek MT8195 SoC has a Mali G57 MC5 (Valhall-JM) and has the same number of power domains and requirements as MT8192 in terms of bindings. Signed-off-by: AngeloGioacchino Del Regno --- Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml | 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index e7aba66ddb8f..6bd0a5b3c5b7 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -25,6 +25,11 @@ properties: - rockchip,px30-mali - rockchip,rk3568-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable + - items: + - enum: + - mediatek,mt8195-mali + - const: mediatek,mt8192-mali + - const: arm,mali-valhall-jm # Mali Valhall GPU model/revision is fully discoverable - items: - enum: - mediatek,mt8192-mali -- 2.39.1
[PATCH 7/9] drm/panfrost: Add the MT8192 GPU ID
From: Alyssa Rosenzweig MediaTek MT8192 has a Mali-G57 with a special GPU ID. Add its GPU ID, but treat it as otherwise identical to a standard Mali-G57. We do _not_ fix up the GPU ID here -- userspace needs to be aware of the special GPU ID, in case we find functional differences between MediaTek's implementation and the standard Mali-G57 down the line. Signed-off-by: Alyssa Rosenzweig Signed-off-by: AngeloGioacchino Del Regno --- drivers/gpu/drm/panfrost/panfrost_gpu.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c index 6452e4e900dd..d28b99732dde 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c @@ -204,6 +204,14 @@ static const struct panfrost_model gpu_models[] = { GPU_MODEL(g57, 0x9001, GPU_REV(g57, 0, 0)), + + /* MediaTek MT8192 has a Mali-G57 with a different GPU ID from the +* standard. Arm's driver does not appear to handle this model. +* ChromeOS has a hack downstream for it. Treat it as equivalent to +* standard Mali-G57 for now. +*/ + GPU_MODEL(g57, 0x9003, + GPU_REV(g57, 0, 0)), }; static void panfrost_gpu_init_features(struct panfrost_device *pfdev) -- 2.39.1
[PATCH 4/9] dt-bindings: gpu: mali-bifrost: Add new MT8183 compatible
Since new platform data was required in Panfrost for getting GPU DVFS finally working on MediaTek SoCs, add a new "mediatek,mt8183b-mali" compatible. Signed-off-by: AngeloGioacchino Del Regno --- .../bindings/gpu/arm,mali-bifrost.yaml| 20 +++ 1 file changed, 20 insertions(+) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 6bd0a5b3c5b7..8c57b89ee866 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -19,6 +19,7 @@ properties: - enum: - amlogic,meson-g12a-mali - mediatek,mt8183-mali + - mediatek,mt8183b-mali - realtek,rtd1619-mali - renesas,r9a07g044-mali - renesas,r9a07g054-mali @@ -153,6 +154,7 @@ allOf: contains: enum: - mediatek,mt8183-mali +- mediatek,mt8183b-mali - mediatek,mt8192-mali then: properties: @@ -178,6 +180,24 @@ allOf: - sram-supply - power-domains - power-domain-names + - if: + properties: +compatible: + contains: +const: mediatek,mt8183b-mali +then: + properties: +power-domains: + minItems: 3 +power-domain-names: + items: +- const: core0 +- const: core1 +- const: core2 + + required: +- power-domains +- power-domain-names - if: properties: compatible: -- 2.39.1
[PATCH 6/9] drm/panfrost: Increase MAX_PM_DOMAINS to 5
From: Alyssa Rosenzweig Increase the MAX_PM_DOMAINS constant from 3 to 5, to support the extra power domains required by the Mali-G57 on the MT8192. Signed-off-by: Alyssa Rosenzweig Signed-off-by: AngeloGioacchino Del Regno --- drivers/gpu/drm/panfrost/panfrost_device.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h index d9ba68cffb77..b0126b9fbadc 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.h +++ b/drivers/gpu/drm/panfrost/panfrost_device.h @@ -23,7 +23,7 @@ struct panfrost_job; struct panfrost_perfcnt; #define NUM_JOB_SLOTS 3 -#define MAX_PM_DOMAINS 3 +#define MAX_PM_DOMAINS 5 struct panfrost_features { u16 id; -- 2.39.1
[PATCH 5/9] dt-bindings: gpu: mali-bifrost: Add a compatible for MediaTek MT8186
Get GPU support on MT8186 by adding its compatible. Signed-off-by: AngeloGioacchino Del Regno --- Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml | 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 8c57b89ee866..85111559dfe0 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -15,6 +15,11 @@ properties: compatible: oneOf: + - items: + - enum: + - mediatek,mt8186-mali + - const: mediatek,mt8183b-mali + - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable - items: - enum: - amlogic,meson-g12a-mali -- 2.39.1