Re: [PATCH 13/15] drm/amdgpu/display: split dp connector registration (v3)
On Fri, Feb 07, 2020 at 04:17:13PM -0500, Alex Deucher wrote: > Split into init and register functions to avoid a segfault > in some configs when the load/unload callbacks are removed. > > v2: > - add back accidently dropped has_aux setting > - set dev in late_register > > v3: > - fix dp cec ordering Why did you move this back out of the late_register callback when going from v2->v3? In i915 we register the cec stuff from ->late_register, like anything else userspace visible. Maybe follow-up patch (the idea behind removing the ->load callback is to close all the driver load races, instead of only open("/dev/dri/0"), which is protected by drm_global_mutex). On this: Reviewed-by: Daniel Vetter Cheers, Daniel > > Signed-off-by: Alex Deucher > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 16 > drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 10 ++ > .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 7 ++- > 3 files changed, 24 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > index ec1501e3a63a..f355d9a752d2 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > @@ -1461,6 +1461,20 @@ static enum drm_mode_status > amdgpu_connector_dp_mode_valid(struct drm_connector > return MODE_OK; > } > > +static int > +amdgpu_connector_late_register(struct drm_connector *connector) > +{ > + struct amdgpu_connector *amdgpu_connector = > to_amdgpu_connector(connector); > + int r = 0; > + > + if (amdgpu_connector->ddc_bus->has_aux) { > + amdgpu_connector->ddc_bus->aux.dev = > amdgpu_connector->base.kdev; > + r = drm_dp_aux_register(_connector->ddc_bus->aux); > + } > + > + return r; > +} > + > static const struct drm_connector_helper_funcs > amdgpu_connector_dp_helper_funcs = { > .get_modes = amdgpu_connector_dp_get_modes, > .mode_valid = amdgpu_connector_dp_mode_valid, > @@ -1475,6 +1489,7 @@ static const struct drm_connector_funcs > amdgpu_connector_dp_funcs = { > .early_unregister = amdgpu_connector_unregister, > .destroy = amdgpu_connector_destroy, > .force = amdgpu_connector_dvi_force, > + .late_register = amdgpu_connector_late_register, > }; > > static const struct drm_connector_funcs amdgpu_connector_edp_funcs = { > @@ -1485,6 +1500,7 @@ static const struct drm_connector_funcs > amdgpu_connector_edp_funcs = { > .early_unregister = amdgpu_connector_unregister, > .destroy = amdgpu_connector_destroy, > .force = amdgpu_connector_dvi_force, > + .late_register = amdgpu_connector_late_register, > }; > > void > diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c > b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c > index ea702a64f807..9b74cfdba7b8 100644 > --- a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c > +++ b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c > @@ -186,16 +186,10 @@ amdgpu_atombios_dp_aux_transfer(struct drm_dp_aux *aux, > struct drm_dp_aux_msg *m > > void amdgpu_atombios_dp_aux_init(struct amdgpu_connector *amdgpu_connector) > { > - int ret; > - > amdgpu_connector->ddc_bus->rec.hpd = amdgpu_connector->hpd.hpd; > - amdgpu_connector->ddc_bus->aux.dev = amdgpu_connector->base.kdev; > amdgpu_connector->ddc_bus->aux.transfer = > amdgpu_atombios_dp_aux_transfer; > - ret = drm_dp_aux_register(_connector->ddc_bus->aux); > - if (!ret) > - amdgpu_connector->ddc_bus->has_aux = true; > - > - WARN(ret, "drm_dp_aux_register_i2c_bus() failed with error %d\n", ret); > + drm_dp_aux_init(_connector->ddc_bus->aux); > + amdgpu_connector->ddc_bus->has_aux = true; > } > > /* general DP utility functions */ > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > index 3959c942c88b..d5b9e72f2649 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > @@ -155,6 +155,11 @@ amdgpu_dm_mst_connector_late_register(struct > drm_connector *connector) > struct amdgpu_dm_connector *amdgpu_dm_connector = > to_amdgpu_dm_connector(connector); > struct drm_dp_mst_port *port = amdgpu_dm_connector->port; > + int r; > + > + r = drm_dp_aux_register(_dm_connector->dm_dp_aux.aux); > + if (r) > + return r; > > #if defined(CONFIG_DEBUG_FS) > connector_debugfs_init(amdgpu_dm_connector); > @@ -484,7 +489,7 @@ void amdgpu_dm_initialize_dp_connector(struct > amdgpu_display_manager *dm, > aconnector->dm_dp_aux.aux.transfer = dm_dp_aux_transfer; > aconnector->dm_dp_aux.ddc_service = aconnector->dc_link->ddc; > > - drm_dp_aux_register(>dm_dp_aux.aux); > + drm_dp_aux_init(>dm_dp_aux.aux); >
Re: [PATCH] drm/mediatek: fix race condition for HDMI jack status reporting
Hi, Tzung-Bi: On Thu, 2020-02-13 at 15:59 +0800, Tzung-Bi Shih wrote: > hdmi_conn_detect and mtk_hdmi_audio_hook_plugged_cb would be called > by different threads. > > Imaging the following calling sequence: >Thread AThread B > > mtk_hdmi_audio_hook_plugged_cb() > mtk_cec_hpd_high() -> disconnected > hdmi_conn_detect() > mtk_cec_hpd_high() -> connected > plugged_cb(connected) > plugged_cb(disconnected) > > The latest disconnected is false reported. Makes mtk_cec_hpd_high > and plugged_cb atomic to fix. > > plugged_cb and codec_dev are also in danger of race condition. Instead > of using mutex to protect them: > - Checks NULLs first. > - Uses WRITE_ONCE() to prevent store tearing (i.e. write to plugged_cb > after codec_dev). > - Uses codec_dev as a signal to report HDMI jack status. > > Fixes: 5d3c64477392 ("drm/mediatek: support HDMI jack status reporting") > > Signed-off-by: Tzung-Bi Shih > --- > Previous discussion: https://patchwork.kernel.org/patch/11367625/ > Previous attempt: https://patchwork.kernel.org/patch/11378413/ > > drivers/gpu/drm/mediatek/mtk_hdmi.c | 18 ++ > 1 file changed, 14 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c > b/drivers/gpu/drm/mediatek/mtk_hdmi.c > index 03aeb73005ef..b1e5d0c538fa 100644 > --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c > +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c > @@ -12,6 +12,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -171,6 +172,7 @@ struct mtk_hdmi { > bool enabled; > hdmi_codec_plugged_cb plugged_cb; > struct device *codec_dev; > + struct mutex update_plugged_status_lock; > }; > > static inline struct mtk_hdmi *hdmi_ctx_from_bridge(struct drm_bridge *b) > @@ -1199,10 +1201,13 @@ static void mtk_hdmi_clk_disable_audio(struct > mtk_hdmi *hdmi) > static enum drm_connector_status > mtk_hdmi_update_plugged_status(struct mtk_hdmi *hdmi) > { > - bool connected = mtk_cec_hpd_high(hdmi->cec_dev); > + bool connected; > > - if (hdmi->plugged_cb && hdmi->codec_dev) > + mutex_lock(>update_plugged_status_lock); > + connected = mtk_cec_hpd_high(hdmi->cec_dev); > + if (hdmi->codec_dev) > hdmi->plugged_cb(hdmi->codec_dev, connected); > + mutex_unlock(>update_plugged_status_lock); > > return connected ? > connector_status_connected : connector_status_disconnected; > @@ -1669,8 +1674,12 @@ static int mtk_hdmi_audio_hook_plugged_cb(struct > device *dev, void *data, > { > struct mtk_hdmi *hdmi = data; > > - hdmi->plugged_cb = fn; > - hdmi->codec_dev = codec_dev; > + if (!fn || !codec_dev) I think sound driver could be removed for some reason, and fn should be set to NULL before sound driver removed. In this case, codec_dev != NULL and fn == NULL. Regards, CK > + return -EINVAL; > + > + /* Use WRITE_ONCE() to prevent store tearing. */ > + WRITE_ONCE(hdmi->plugged_cb, fn); > + WRITE_ONCE(hdmi->codec_dev, codec_dev); > mtk_hdmi_update_plugged_status(hdmi); > > return 0; > @@ -1729,6 +1738,7 @@ static int mtk_drm_hdmi_probe(struct platform_device > *pdev) > return ret; > } > > + mutex_init(>update_plugged_status_lock); > platform_set_drvdata(pdev, hdmi); > > ret = mtk_hdmi_output_init(hdmi); ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 01/13] dt-bindings: arm: move mmsys description to display
Hi, Matthias: On Thu, 2020-02-13 at 21:19 +0100, matthias@kernel.org wrote: > From: Matthias Brugger > > The mmsys block provides registers and clocks for the display > subsystem. The binding description should therefore live together with > the rest of the display descriptions. Move it to display/mediatek. > Yes, for the upstreamed driver, only display (DRM) use mmsys clock. For some MDP patches [1] in progress, MDP also use mmsys clock. So we just consider what's upstreamed now? [1] https://patchwork.kernel.org/patch/11140747/ Regards, CK > Signed-off-by: Matthias Brugger > > --- > > Changes in v7: > - move the binding description > > Changes in v6: None > Changes in v5: None > Changes in v4: None > Changes in v3: None > Changes in v2: None > > .../bindings/{arm => display}/mediatek/mediatek,mmsys.txt | 0 > 1 file changed, 0 insertions(+), 0 deletions(-) > rename Documentation/devicetree/bindings/{arm => > display}/mediatek/mediatek,mmsys.txt (100%) > > diff --git > a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt > b/Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt > similarity index 100% > rename from Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt > rename to > Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/mediatek: make sure previous message done or be aborted before send
Hi, Bibby: On Fri, 2020-02-14 at 12:49 +0800, Bibby Hsieh wrote: > Mediatek CMDQ driver removed atomic parameter and implementation > related to atomic. DRM driver need to make sure previous message > done or be aborted before we send next message. > > If previous message is still waiting for event, it means the > setting hasn't been updated into display hardware register, > we can abort the message and send next message to update the > newest setting into display hardware. > If previous message already started, we have to wait it until > transmission has been completed. > > So we flush mbox client before we send new message to controller > driver. > Reviewed-by: CK Hu > This patch depends on ptach: > [0/3] Remove atomic_exec > https://patchwork.kernel.org/cover/11381677/ > > Signed-off-by: Bibby Hsieh > --- > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > index 3c53ea22208c..e35b66c5ba0f 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > @@ -491,6 +491,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc > *mtk_crtc) > } > #if IS_ENABLED(CONFIG_MTK_CMDQ) > if (mtk_crtc->cmdq_client) { > + mbox_flush(mtk_crtc->cmdq_client->chan, 2000); > cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE); > cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event); > cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event); ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/i915: Cast remain to unsigned long in eb_relocate_vma
On Thu, 13 Feb 2020, Nathan Chancellor wrote: > A recent commit in clang added -Wtautological-compare to -Wall, which is > enabled for i915 after -Wtautological-compare is disabled for the rest > of the kernel so we see the following warning on x86_64: > > ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1433:22: warning: > result of comparison of constant 576460752303423487 with expression of > type 'unsigned int' is always false > [-Wtautological-constant-out-of-range-compare] > if (unlikely(remain > N_RELOC(ULONG_MAX))) > ^ > ../include/linux/compiler.h:78:42: note: expanded from macro 'unlikely' > # define unlikely(x)__builtin_expect(!!(x), 0) > ^ > 1 warning generated. > > It is not wrong in the case where ULONG_MAX > UINT_MAX but it does not > account for the case where this file is built for 32-bit x86, where > ULONG_MAX == UINT_MAX and this check is still relevant. > > Cast remain to unsigned long, which keeps the generated code the same > (verified with clang-11 on x86_64 and GCC 9.2.0 on x86 and x86_64) and > the warning is silenced so we can catch more potential issues in the > future. > > Link: https://github.com/ClangBuiltLinux/linux/issues/778 > Suggested-by: Michel Dänzer > Signed-off-by: Nathan Chancellor Works for me as a workaround, Reviewed-by: Jani Nikula > --- > > Round 3 :) > > Previous threads/patches: > > https://lore.kernel.org/lkml/20191123195321.41305-1-natechancel...@gmail.com/ > https://lore.kernel.org/lkml/20200211050808.29463-1-natechancel...@gmail.com/ > > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > index 60c984e10c4a..47f4d8ab281e 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > @@ -1430,7 +1430,7 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, > struct i915_vma *vma) > > urelocs = u64_to_user_ptr(entry->relocs_ptr); > remain = entry->relocation_count; > - if (unlikely(remain > N_RELOC(ULONG_MAX))) > + if (unlikely((unsigned long)remain > N_RELOC(ULONG_MAX))) > return -EINVAL; > > /* -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 3/3] drm/dp_mst: Remove single tx msg restriction.
[AMD Public Use] Hi Paul, Thanks for the mail! I tried to solve this problem by having restriction on sending one msg at a time due to hub/dock compatibility problems. >From my experience, some branch devices don't handle well on interleaved >replies (Dock from HP I think) As the result of that, correct me if I'm wrong, I remember most gpu vendors just send one down request at a time now in windows environment. I would suggest the original solution :) Thanks! > -Original Message- > From: Sean Paul > Sent: Friday, February 14, 2020 5:15 AM > To: dri-devel@lists.freedesktop.org > Cc: ly...@redhat.com; Lin, Wayne ; Sean Paul > ; Maarten Lankhorst > ; Maxime Ripard ; > David Airlie > Subject: [PATCH 3/3] drm/dp_mst: Remove single tx msg restriction. > > From: Sean Paul > > Now that we can support multiple simultaneous replies, remove the > restrictions placed on sending new tx msgs. > > This patch essentially just reverts commit > 5a64967a2f3b ("drm/dp_mst: Have DP_Tx send one msg at a time") now > that the problem is solved in a different way. > > Cc: Wayne Lin > Signed-off-by: Sean Paul > --- > drivers/gpu/drm/drm_dp_mst_topology.c | 14 ++ > include/drm/drm_dp_mst_helper.h | 6 -- > 2 files changed, 2 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > b/drivers/gpu/drm/drm_dp_mst_topology.c > index 7e6a82efdfc02..cbf0bb0ddeb84 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -1203,8 +1203,6 @@ static int drm_dp_mst_wait_tx_reply(struct > drm_dp_mst_branch *mstb, > txmsg->state == DRM_DP_SIDEBAND_TX_SENT) { > mstb->tx_slots[txmsg->seqno] = NULL; > } > - mgr->is_waiting_for_dwn_reply = false; > - > } > out: > if (unlikely(ret == -EIO) && drm_debug_enabled(DRM_UT_DP)) { @@ > -1214,7 +1212,6 @@ static int drm_dp_mst_wait_tx_reply(struct > drm_dp_mst_branch *mstb, > } > mutex_unlock(>qlock); > > - drm_dp_mst_kick_tx(mgr); > return ret; > } > > @@ -2797,11 +2794,9 @@ static void process_single_down_tx_qlock(struct > drm_dp_mst_topology_mgr *mgr) > ret = process_single_tx_qlock(mgr, txmsg, false); > if (ret == 1) { > /* txmsg is sent it should be in the slots now */ > - mgr->is_waiting_for_dwn_reply = true; > list_del(>next); > } else if (ret) { > DRM_DEBUG_KMS("failed to send msg in q %d\n", ret); > - mgr->is_waiting_for_dwn_reply = false; > list_del(>next); > if (txmsg->seqno != -1) > txmsg->dst->tx_slots[txmsg->seqno] = NULL; @@ -2841,8 > +2836,7 @@ static void drm_dp_queue_down_tx(struct > drm_dp_mst_topology_mgr *mgr, > drm_dp_mst_dump_sideband_msg_tx(, txmsg); > } > > - if (list_is_singular(>tx_msg_downq) && > - !mgr->is_waiting_for_dwn_reply) > + if (list_is_singular(>tx_msg_downq)) > process_single_down_tx_qlock(mgr); > mutex_unlock(>qlock); > } > @@ -3822,7 +3816,6 @@ static int drm_dp_mst_handle_down_rep(struct > drm_dp_mst_topology_mgr *mgr) > mutex_lock(>qlock); > txmsg->state = DRM_DP_SIDEBAND_TX_RX; > mstb->tx_slots[seqno] = NULL; > - mgr->is_waiting_for_dwn_reply = false; > mutex_unlock(>qlock); > > wake_up_all(>tx_waitq); > @@ -3830,9 +3823,6 @@ static int drm_dp_mst_handle_down_rep(struct > drm_dp_mst_topology_mgr *mgr) > return 0; > > out_clear_reply: > - mutex_lock(>qlock); > - mgr->is_waiting_for_dwn_reply = false; > - mutex_unlock(>qlock); > if (msg) > memset(msg, 0, sizeof(struct drm_dp_sideband_msg_rx)); > out: > @@ -4670,7 +4660,7 @@ static void drm_dp_tx_work(struct work_struct > *work) > struct drm_dp_mst_topology_mgr *mgr = container_of(work, struct > drm_dp_mst_topology_mgr, tx_work); > > mutex_lock(>qlock); > - if (!list_empty(>tx_msg_downq) > && !mgr->is_waiting_for_dwn_reply) > + if (!list_empty(>tx_msg_downq)) > process_single_down_tx_qlock(mgr); > mutex_unlock(>qlock); > } > diff --git a/include/drm/drm_dp_mst_helper.h > b/include/drm/drm_dp_mst_helper.h index 7d0341f94ce1b..fcc30e64c8e7e > 100644 > --- a/include/drm/drm_dp_mst_helper.h > +++ b/include/drm/drm_dp_mst_helper.h > @@ -619,12 +619,6 @@ struct drm_dp_mst_topology_mgr { >* _dp_sideband_msg_tx.state once they are queued >*/ > struct mutex qlock; > - > - /** > - * @is_waiting_for_dwn_reply: indicate whether is waiting for down > reply > - */ > - bool is_waiting_for_dwn_reply; > - > /** >* @tx_msg_downq: List of pending down replies. >*/ > -- > Sean Paul, Software Engineer, Google / Chromium OS -- Wayne Lin ___ dri-devel mailing list dri-devel@lists.freedesktop.org
[PATCH 2/3] drm/mediatek: make sure previous message done or be aborted before send
Mediatek CMDQ driver removed atomic parameter and implementation related to atomic. DRM driver need to make sure previous message done or be aborted before we send next message. If previous message is still waiting for event, it means the setting hasn't been updated into display hardware register, we can abort the message and send next message to update the newest setting into display hardware. If previous message already started, we have to wait it until transmission has been completed. So we flush mbox client before we send new message to controller driver. This patch depends on ptach: [0/3] Remove atomic_exec https://patchwork.kernel.org/cover/11381677/ Signed-off-by: Bibby Hsieh --- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index 3c53ea22208c..e35b66c5ba0f 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -491,6 +491,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc) } #if IS_ENABLED(CONFIG_MTK_CMDQ) if (mtk_crtc->cmdq_client) { + mbox_flush(mtk_crtc->cmdq_client->chan, 2000); cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE); cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event); cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event); -- 2.18.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] arm64: dts: mt8183: Add gce setting in display node
In order to use GCE function, we need add some information into display node (mboxes, mediatek,gce-client-reg, mediatek,gce-events). Signed-off-by: Bibby Hsieh Signed-off-by: Yongqiang Niu --- arch/arm64/boot/dts/mediatek/mt8183.dtsi | 16 1 file changed, 16 insertions(+) diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi index be4428c92f35..8b522b039a37 100644 --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi @@ -9,6 +9,7 @@ #include #include #include +#include #include "mt8183-pinfunc.h" / { @@ -664,6 +665,9 @@ reg = <0 0x1400 0 0x1000>; power-domains = < MT8183_POWER_DOMAIN_DISP>; #clock-cells = <1>; + mboxes = < 0 CMDQ_THR_PRIO_HIGHEST>, +< 1 CMDQ_THR_PRIO_HIGHEST>; + mediatek,gce-client-reg = < SUBSYS_1400 0 0x1000>; }; ovl0: ovl@14008000 { @@ -672,6 +676,7 @@ interrupts = ; power-domains = < MT8183_POWER_DOMAIN_DISP>; clocks = < CLK_MM_DISP_OVL0>; + mediatek,gce-client-reg = < SUBSYS_1400 0x8000 0x1000>; }; ovl_2l0: ovl@14009000 { @@ -680,6 +685,7 @@ interrupts = ; power-domains = < MT8183_POWER_DOMAIN_DISP>; clocks = < CLK_MM_DISP_OVL0_2L>; + mediatek,gce-client-reg = < SUBSYS_1400 0x9000 0x1000>; }; ovl_2l1: ovl@1400a000 { @@ -688,6 +694,7 @@ interrupts = ; power-domains = < MT8183_POWER_DOMAIN_DISP>; clocks = < CLK_MM_DISP_OVL1_2L>; + mediatek,gce-client-reg = < SUBSYS_1400 0xa000 0x1000>; }; rdma0: rdma@1400b000 { @@ -697,6 +704,7 @@ power-domains = < MT8183_POWER_DOMAIN_DISP>; clocks = < CLK_MM_DISP_RDMA0>; mediatek,rdma_fifo_size = <5120>; + mediatek,gce-client-reg = < SUBSYS_1400 0xb000 0x1000>; }; rdma1: rdma@1400c000 { @@ -706,6 +714,7 @@ power-domains = < MT8183_POWER_DOMAIN_DISP>; clocks = < CLK_MM_DISP_RDMA1>; mediatek,rdma_fifo_size = <2048>; + mediatek,gce-client-reg = < SUBSYS_1400 0xc000 0x1000>; }; color0: color@1400e000 { @@ -715,6 +724,7 @@ interrupts = ; power-domains = < MT8183_POWER_DOMAIN_DISP>; clocks = < CLK_MM_DISP_COLOR0>; + mediatek,gce-client-reg = < SUBSYS_1400 0xe000 0x1000>; }; ccorr0: ccorr@1400f000 { @@ -723,6 +733,7 @@ interrupts = ; power-domains = < MT8183_POWER_DOMAIN_DISP>; clocks = < CLK_MM_DISP_CCORR0>; + mediatek,gce-client-reg = < SUBSYS_1400 0xf000 0x1000>; }; aal0: aal@1401 { @@ -732,6 +743,7 @@ interrupts = ; power-domains = < MT8183_POWER_DOMAIN_DISP>; clocks = < CLK_MM_DISP_AAL0>; + mediatek,gce-client-reg = < SUBSYS_1401 0 0x1000>; }; gamma0: gamma@14011000 { @@ -741,6 +753,7 @@ interrupts = ; power-domains = < MT8183_POWER_DOMAIN_DISP>; clocks = < CLK_MM_DISP_GAMMA0>; + mediatek,gce-client-reg = < SUBSYS_1401 0x1000 0x1000>; }; dither0: dither@14012000 { @@ -749,6 +762,7 @@ interrupts = ; power-domains = < MT8183_POWER_DOMAIN_DISP>; clocks = < CLK_MM_DISP_DITHER0>; + mediatek,gce-client-reg = < SUBSYS_1401 0x2000 0x1000>; }; mutex: mutex@14016000 { @@ -756,6 +770,8 @@ reg = <0 0x14016000 0 0x1000>; interrupts = ; power-domains = < MT8183_POWER_DOMAIN_DISP>; + mediatek,gce-events = , + ; }; smi_common: smi@14019000 { -- 2.18.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] drm/mediatek: move gce event property to mutex device node
According mtk hardware design, stream_done0 and stream_done1 are generated by mutex, so we move gce event property to mutex device mode. Signed-off-by: Bibby Hsieh --- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index e35b66c5ba0f..e1cc7703a312 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -820,7 +820,7 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev, drm_crtc_index(_crtc->base)); mtk_crtc->cmdq_client = NULL; } - ret = of_property_read_u32_index(dev->of_node, "mediatek,gce-events", + ret = of_property_read_u32_index(priv->mutex_node, "mediatek,gce-events", drm_crtc_index(_crtc->base), _crtc->cmdq_event); if (ret) -- 2.18.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[git pull] drm fixes for 5.6-rc2
Hey Linus, This week's fixes ready for rc2. The core has a build fix for edid code on certain compilers/arches/, one MST fix and one vgem fix. Regular amdgpu fixes, and a couple of small driver fixes. The i915 fixes are bit larger than normal for this stage, but they were having CI issues last week, and they hadn't sent any fixes last week due to this. Regards, Dave. drm-fixes-2020-02-14: drm fixes for 5.6-rc2 core: - edid build fix mst: - fix NULL ptr deref vgem: - fix close after free msm: - better dma-api usage sun4i: - disable allow_fb_modifiers amdgpu: - Additional OD fixes for navi - Misc display fixes - VCN 2.5 DPG fix - Prevent build errors on PowerPC on some configs - GDS EDC fix i915: - dsi/acpi fixes - gvt locking and allocation fixes - gem/gt fixes - bios timing parameters fix The following changes since commit bb6d3fb354c5ee8d6bde2d576eb7220ea09862b9: Linux 5.6-rc1 (2020-02-09 16:08:48 -0800) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2020-02-14 for you to fetch changes up to 6f4134b30b6ee33e2fd4d602099e6c5e60d0351a: Merge tag 'drm-intel-next-fixes-2020-02-13' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes (2020-02-14 13:04:46 +1000) drm fixes for 5.6-rc2 core: - edid build fix mst: - fix NULL ptr deref vgem: - fix close after free msm: - better dma-api usage sun4i: - disable allow_fb_modifiers amdgpu: - Additional OD fixes for navi - Misc display fixes - VCN 2.5 DPG fix - Prevent build errors on PowerPC on some configs - GDS EDC fix i915: - dsi/acpi fixes - gvt locking and allocation fixes - gem/gt fixes - bios timing parameters fix Alex Deucher (2): drm/amdgpu: update smu_v11_0_pptable.h drm/amdgpu:/navi10: use the ODCAP enum to index the caps array Aric Cyr (1): drm/amd/display: Check engine is not NULL before acquiring Boris Brezillon (1): drm/panfrost: Make sure the shrinker does not reclaim referenced BOs Chris Wilson (19): drm/i915/pmu: Correct the rc6 offset upon enabling drm/i915/gem: Take local vma references for the parser drm/i915/selftests: Add a mock i915_vma to the mock_ring drm/i915/gt: Use the BIT when checking the flags, not the index drm/i915/execlists: Leave resetting ring to intel_ring drm/i915/gem: Store mmap_offsets in an rbtree rather than a plain list drm/i915: Don't show the blank process name for internal/simulated errors drm/i915/gem: Detect overflow in calculating dumb buffer size drm/i915: Check activity on i915_vma after confirming pin_count==0 drm/i915: Stub out i915_gpu_coredump_put drm/i915: Tighten atomicity of i915_active_acquire vs i915_active_release drm/i915/gt: Acquire ce->active before ce->pin_count/ce->pin_mutex drm/i915/gem: Tighten checks and acquiring the mmap object drm/i915: Keep track of request among the scheduling lists drm/i915/gt: Allow temporary suspension of inflight requests drm/i915/execlists: Offline error capture drm/i915/execlists: Take a reference while capturing the guilty request drm/i915/execlists: Reclaim the hanging virtual request drm/i915: Mark the removal of the i915_request from the sched.link Daniel Kolesa (1): amdgpu: Prevent build errors regarding soft/hard-float FP ABI tags Daniel Vetter (1): drm/vgem: Close use-after-free race in vgem_gem_create Dave Airlie (4): Merge tag 'drm-misc-fixes-2020-02-07' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes Merge tag 'drm-misc-next-fixes-2020-02-07' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes Merge tag 'amd-drm-fixes-5.6-2020-02-12' of git://people.freedesktop.org/~agd5f/linux into drm-fixes Merge tag 'drm-intel-next-fixes-2020-02-13' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes Guchun Chen (2): drm/amdgpu: limit GDS clearing workaround in cold boot sequence drm/amdgpu: correct comment to clear up the confusion Igor Druzhinin (2): drm/i915/gvt: fix high-order allocation failure on late load drm/i915/gvt: more locking for ppgtt mm LRU list Isabel Zhang (1): drm/amd/display: Add initialitions for PLL2 clock source James Zhu (2): drm/amdgpu/vcn2.5: fix DPG mode power off issue on instance 1 drm/amdgpu/vcn2.5: fix warning Jani Nikula (1): Merge tag 'gvt-fixes-2020-02-12' of https://github.com/intel/gvt-linux into drm-intel-next-fixes Jernej Skrabec (1): Revert "drm/sun4i: drv: Allow framebuffer modifiers in mode config" Jonathan Kim (1): drm/amdgpu: fix amdgpu pmu to use hwc->config instead of hwc->conf José Roberto de Souza (2): drm/mst: Fix possible NULL pointer dereference in drm_dp_mst_process_up_req() drm/i915: Fix preallocated barrier list append Mauro
Re: [PATCH v7 11/13] clk: mediatek: mt8183: switch mmsys to platform device probing
Hi, Matthias: On Thu, 2020-02-13 at 21:19 +0100, matthias@kernel.org wrote: > From: Matthias Brugger > > Switch probing for the MMSYS to support invocation to a > plain paltform device. The driver will be probed by the DRM subsystem. > > Signed-off-by: Matthias Brugger > > --- > > Changes in v7: > - free clk_data->clks as well > - get rid of private data structure > > Changes in v6: None > Changes in v5: None > Changes in v4: None > Changes in v3: None > Changes in v2: None > > drivers/clk/mediatek/clk-mt8183-mm.c | 30 ++-- > 1 file changed, 20 insertions(+), 10 deletions(-) > > diff --git a/drivers/clk/mediatek/clk-mt8183-mm.c > b/drivers/clk/mediatek/clk-mt8183-mm.c > index 720c696b506d..7576cd231be3 100644 > --- a/drivers/clk/mediatek/clk-mt8183-mm.c > +++ b/drivers/clk/mediatek/clk-mt8183-mm.c > @@ -3,8 +3,10 @@ > // Copyright (c) 2018 MediaTek Inc. > // Author: Weiyi Lu > > +#include > #include > #include > +#include > > #include "clk-mtk.h" > #include "clk-gate.h" > @@ -85,27 +87,35 @@ static const struct mtk_gate mm_clks[] = { > static int clk_mt8183_mm_probe(struct platform_device *pdev) > { > struct clk_onecell_data *clk_data; > - struct device_node *node = pdev->dev.of_node; > + struct device_node *node = pdev->dev.parent->of_node; > + > + clk_data = devm_kzalloc(>dev, sizeof(*clk_data), GFP_KERNEL); I think this is redundant. > + if (!clk_data) > + return -ENOMEM; > > clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK); > + platform_set_drvdata(pdev, clk_data); > > - mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks), > - clk_data); > + mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks), clk_data); > > return of_clk_add_provider(node, of_clk_src_onecell_get, clk_data); > } > > -static const struct of_device_id of_match_clk_mt8183_mm[] = { > - { .compatible = "mediatek,mt8183-mmsys", }, > - {} > -}; > +static int clk_mt8183_mm_remove(struct platform_device *pdev) > +{ > + struct clk_onecell_data *clk_data = platform_get_drvdata(pdev); > + > + kfree(clk_data->clks); > + kfree(clk_data); These two statement looks like a reverse of mtk_alloc_clk_data() and exist in many files. It is worth to have a function (maybe mtk_free_clk_data()) to do this. In addition, should we undo what is done in clk_mt8183_mm_probe() such as mtk_clk_register_gates() and of_clk_add_provider()? Regards, CK > + > + return 0; > +} > > static struct platform_driver clk_mt8183_mm_drv = { > .probe = clk_mt8183_mm_probe, > + .remove = clk_mt8183_mm_remove, > .driver = { > .name = "clk-mt8183-mm", > - .of_match_table = of_match_clk_mt8183_mm, > }, > }; > - > -builtin_platform_driver(clk_mt8183_mm_drv); > +module_platform_driver(clk_mt8183_mm_drv); ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Re: [DPU PATCH v3 3/5] drm/msm/dp: add displayPort driver support
Hello Rob, We removed the bridge object for DisplayPort due to review comments in patch set 1. Added more details below. Original Message Subject: Re: [DPU PATCH v3 3/5] drm/msm/dp: add displayPort driver support Date: 2019-12-02 08:48 From: Rob Clark To: Chandan Uddaraju Cc: "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , linux-arm-msm , Abhinav Kumar , Sean Paul , dri-devel , "Kristian H. Kristensen" , freedreno On Mon, Dec 2, 2019 at 5:48 AM Chandan Uddaraju wrote: Add the needed displayPort files to enable DP driver on msm target. "dp_display" module is the main module that calls into other sub-modules. "dp_drm" file represents the interface between DRM framework and DP driver. changes in v2: -- Update copyright markings on all relevant files. -- Change pr_err() to DRM_ERROR() -- Use APIs directly instead of function pointers. -- Use drm_display_mode structure to store link parameters in the driver. -- Use macros for register definitions instead of hardcoded values. -- Replace writel_relaxed/readl_relaxed with writel/readl and remove memory barriers. -- Remove unnecessary NULL checks. -- Use drm helper functions for dpcd read/write. -- Use DRM_DEBUG_DP for debug msgs. changes in V3: -- Removed changes in dpu_io_util.[ch] -- Added locking around "is_connected" flag and removed atomic_set() -- Removed the argument validation checks in all the static functions except initialization functions and few API calls across msm/dp files -- Removed hardcoded values for register reads/writes -- Removed vreg related generic structures. -- Added return values where ever necessary. -- Updated dp_ctrl_on function. -- Calling the ctrl specific catalog functions directly instead of function pointers. -- Added seperate change that adds standard value in drm_dp_helper file. -- Added separate change in this list that is used to initialize displayport in DPU driver. -- Added change to use drm_dp_get_adjust_request_voltage() function. Signed-off-by: Chandan Uddaraju --- [snip] diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index f96e142..29ac7d3 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -967,6 +967,9 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc, trace_dpu_enc_mode_set(DRMID(drm_enc)); + if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS && priv->dp) + msm_dp_display_mode_set(priv->dp, drm_enc, mode, adj_mode); + for better or for worse, DSI and HDMI backends create an internal drm_bridge object to avoid all of these shunts over from the encoder. We might be still the only driver to do this, but IMHO it is better than making each encoder know about each backend, so we might as well stick with that. (same goes for the other 'if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS)'s) We had the below comments from Sean Paul to remove the bridge object in patch set 1 of this change. ** ** +static const struct drm_bridge_funcs dp_bridge_ops = { + .mode_fixup = dp_bridge_mode_fixup, + .pre_enable = dp_bridge_pre_enable, + .enable = dp_bridge_enable, + .disable = dp_bridge_disable, + .post_disable = dp_bridge_post_disable, + .mode_set = dp_bridge_mode_set, +}; I'm not convinced you need any of this. The only advantage a bridge gets you is to be involved in modeset. However the only thing you do in modeset is save the mode (which is only used in enable). So why not just use the mode from the crtc's state when encoder->enable is called? That allows you to delete all of the bridge stuff here. + +int dp_connector_post_init(struct drm_connector *connector, void *display) +{ + struct msm_dp *dp_display = display; + + if (!dp_display) + return -EINVAL; *** thanks Chandan BR, -R list_for_each_entry(conn_iter, connector_list, head) if (conn_iter->encoder == drm_enc) conn = conn_iter; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH] drm/mm: Break long searches in fragmented address spaces
Quoting Andi Shyti (2020-02-11 22:56:44) > Hi Chris, > > On Fri, Feb 07, 2020 at 03:17:20PM +, Chris Wilson wrote: > > We try hard to select a suitable hole in the drm_mm first time. But if > > that is unsuccessful, we then have to look at neighbouring nodes, and > > this requires traversing the rbtree. Walking the rbtree can be slow > > (much slower than a linear list for deep trees), and if the drm_mm has > > been purposefully fragmented our search can be trapped for a long, long > > time. For non-preemptible kernels, we need to break up long CPU bound > > sections by manually checking for cond_resched(); similarly we should > > checking for "fatal signals" you mean? We need to call schedule() ourselves for non-voluntary CONFIG_PREEMPT kernels, and we need to escape from the kernel back to userspace to handle a pending signal in this process. Other applications, sysadmins tend to notice and complain about driver hogging a CPU not letting anything else run; typically the user notices that their application doesn't close on ^C. (That is schedule() delays of a few ms will be noticed, but it takes a couple of seconds before a user will become aware of a severe problem with a stuck signal. :) Now since responding to a signal itself may be expensive (we have to restart the syscall and re-process all the state from before this point), we make the trade-off here of only responding to a fatal-signal (process exit), which should allow for faster system recovery under pathological conditions. -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] ARM: dts: am437x-gp/epos-evm: drop unused panel timings
Hi, On Tue, Feb 11, 2020 at 01:10:07PM +0200, Laurent Pinchart wrote: > On Tue, Feb 11, 2020 at 01:08:12PM +0200, Tomi Valkeinen wrote: > > On 11/02/2020 13:07, Laurent Pinchart wrote: > > > > >> Hopefully soon (in five years? =) we can say that omapdrm supports all > > >> the boards, and we can deprecate omapfb. > > > > > > I'd love to send a patch to remove omapfb, but I'll let you do the > > > honours :-) > > > > Not before we add DSI support to omapdrm... > > Details, details ;-) > > Seriously speaking, Sebastian's patches are on my todo list. The patches need to be rebased. My hack for supporting panel un/rebind stole connector and drm_dev pointer from drm_panel, which is no longer possible. I hoped this could be cleaned up once your and my omapdrm patches landed... I do have a compile-tested-only WIP branch here, but I doubt I got this working on the first try: https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-n900.git/log/?h=omapdrm-dsi-drm-panel-5.6-rc1-wip -- Sebastian signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH] nouveau: no need to check return value of debugfs_create functions
On Fri, Feb 14, 2020 at 12:30:48AM +0100, Daniel Vetter wrote: > On Thu, Feb 13, 2020 at 11:39 PM Greg Kroah-Hartman > wrote: > > > > On Thu, Feb 13, 2020 at 02:30:09PM -0800, John Hubbard wrote: > > > On 2/9/20 2:55 AM, Greg Kroah-Hartman wrote: > > > > When calling debugfs functions, there is no need to ever check the > > > > return value. The function can work or not, but the code logic should > > > > never do something different based on this. > > > > > > > > > > Should we follow that line of reasoning further, and simply return void > > > from the debugfs functions--rather than playing whack-a-mole with this > > > indefinitely? > > > > That is what we (well I) have been doing. Look at all of the changes > > that have happened to include/linux/debugfs.h over the past few > > releases. I'm slowly winnowing down the api to make it impossible to > > get wrong for this type of thing, and am almost there. > > > > DRM is the big fish left to tackle, I have submitted some patches in the > > past, but lots more cleanup needs to be done to get them into mergable > > shape. I just need to find the time... > > Just to avoid duplication, Wambui (cc'ed) just started working on > this. Expect a lot more void return values and a pile of deleted code > rsn. Nice! It's not duplication if I haven't started on it :) greg k-h ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH] nouveau: no need to check return value of debugfs_create functions
On Thu, Feb 13, 2020 at 11:39 PM Greg Kroah-Hartman wrote: > > On Thu, Feb 13, 2020 at 02:30:09PM -0800, John Hubbard wrote: > > On 2/9/20 2:55 AM, Greg Kroah-Hartman wrote: > > > When calling debugfs functions, there is no need to ever check the > > > return value. The function can work or not, but the code logic should > > > never do something different based on this. > > > > > > > Should we follow that line of reasoning further, and simply return void > > from the debugfs functions--rather than playing whack-a-mole with this > > indefinitely? > > That is what we (well I) have been doing. Look at all of the changes > that have happened to include/linux/debugfs.h over the past few > releases. I'm slowly winnowing down the api to make it impossible to > get wrong for this type of thing, and am almost there. > > DRM is the big fish left to tackle, I have submitted some patches in the > past, but lots more cleanup needs to be done to get them into mergable > shape. I just need to find the time... Just to avoid duplication, Wambui (cc'ed) just started working on this. Expect a lot more void return values and a pile of deleted code rsn. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH] nouveau: no need to check return value of debugfs_create functions
On 2/13/20 2:39 PM, Greg Kroah-Hartman wrote: > On Thu, Feb 13, 2020 at 02:30:09PM -0800, John Hubbard wrote: >> On 2/9/20 2:55 AM, Greg Kroah-Hartman wrote: >>> When calling debugfs functions, there is no need to ever check the >>> return value. The function can work or not, but the code logic should >>> never do something different based on this. >>> >> >> Should we follow that line of reasoning further, and simply return void >> from the debugfs functions--rather than playing whack-a-mole with this >> indefinitely? > > That is what we (well I) have been doing. Look at all of the changes > that have happened to include/linux/debugfs.h over the past few > releases. I'm slowly winnowing down the api to make it impossible to > get wrong for this type of thing, and am almost there. > Oops, I see now that you have already been very busy with this. :) Looking good... thanks, -- John Hubbard NVIDIA > DRM is the big fish left to tackle, I have submitted some patches in the > past, but lots more cleanup needs to be done to get them into mergable > shape. I just need to find the time... >> > thanks, > > greg k-h > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/5] *** Delay context create cmd ***
Let's delay enqueuing the context creation command until the first 3D ioctl, as we add more context types. This series is based on kraxel's "[PATCH v3 0/4] drm/virtio: rework batching" patches. Gurchetan Singh (5): drm/virtio: use consistent names for drm_files drm/virtio: factor out context create hyercall drm/virtio: track whether or not a context has been initiated drm/virtio: enqueue virtio_gpu_create_context after the first 3D ioctl drm/virtio: add virtio_gpu_context_type drivers/gpu/drm/virtio/virtgpu_drv.h | 1 + drivers/gpu/drm/virtio/virtgpu_ioctl.c | 55 +- drivers/gpu/drm/virtio/virtgpu_kms.c | 25 +++- 3 files changed, 52 insertions(+), 29 deletions(-) -- 2.25.0.265.gbab2e86ba0-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/5] drm/virtio: add virtio_gpu_context_type
We'll have to do something like this eventually, and this conveys we want a Virgl context by default. Signed-off-by: Gurchetan Singh --- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 23 +-- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index d2f17778bdc4..f3be29d3f103 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -33,8 +33,14 @@ #include "virtgpu_drv.h" +/* TODO: add more context types */ +enum virtio_gpu_context_type { + virtio_gpu_virgl_context, +}; + static void virtio_gpu_create_context(struct drm_device *dev, - struct drm_file *file) + struct drm_file *file, + enum virtio_gpu_context_type type) { struct virtio_gpu_device *vgdev = dev->dev_private; struct virtio_gpu_fpriv *vfpriv = file->driver_priv; @@ -47,6 +53,11 @@ static void virtio_gpu_create_context(struct drm_device *dev, if (!atomic_add_unless(>context_initiated, 1, 1)) return; + if (type != virtio_gpu_virgl_context) { + DRM_ERROR("Unsupported context type: %u\n", type); + return; + } + get_task_comm(dbgname, current); virtio_gpu_cmd_context_create(vgdev, vfpriv->ctx_id, strlen(dbgname), dbgname); @@ -93,7 +104,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, exbuf->fence_fd = -1; - virtio_gpu_create_context(dev, file); + virtio_gpu_create_context(dev, file, virtio_gpu_virgl_context); if (exbuf->flags & VIRTGPU_EXECBUF_FENCE_FD_IN) { struct dma_fence *in_fence; @@ -244,7 +255,7 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data, return -EINVAL; } - virtio_gpu_create_context(dev, file); + virtio_gpu_create_context(dev, file, virtio_gpu_virgl_context); params.format = rc->format; params.width = rc->width; params.height = rc->height; @@ -318,7 +329,7 @@ static int virtio_gpu_transfer_from_host_ioctl(struct drm_device *dev, if (vgdev->has_virgl_3d == false) return -ENOSYS; - virtio_gpu_create_context(dev, file); + virtio_gpu_create_context(dev, file, virtio_gpu_virgl_context); objs = virtio_gpu_array_from_handles(file, >bo_handle, 1); if (objs == NULL) return -ENOENT; @@ -366,7 +377,7 @@ static int virtio_gpu_transfer_to_host_ioctl(struct drm_device *dev, void *data, args->box.w, args->box.h, args->box.x, args->box.y, objs, NULL); } else { - virtio_gpu_create_context(dev, file); + virtio_gpu_create_context(dev, file, virtio_gpu_virgl_context); ret = virtio_gpu_array_lock_resv(objs); if (ret != 0) goto err_put_free; @@ -467,7 +478,7 @@ static int virtio_gpu_get_caps_ioctl(struct drm_device *dev, spin_unlock(>display_info_lock); /* not in cache - need to talk to hw */ - virtio_gpu_create_context(dev, file); + virtio_gpu_create_context(dev, file, virtio_gpu_virgl_context); virtio_gpu_cmd_get_capset(vgdev, found_valid, args->cap_set_ver, _ent); -- 2.25.0.265.gbab2e86ba0-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/5] drm/virtio: enqueue virtio_gpu_create_context after the first 3D ioctl
For old userspace, initialization will still be implicit. For backwards compatibility, enqueue virtio_gpu_cmd_context_create after the first 3D ioctl. v2: staticify virtio_gpu_create_context Signed-off-by: Gurchetan Singh --- drivers/gpu/drm/virtio/virtgpu_drv.h | 2 -- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 9 +++-- drivers/gpu/drm/virtio/virtgpu_kms.c | 1 - 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index ca505984f8ab..4de0741da454 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -215,8 +215,6 @@ struct virtio_gpu_fpriv { /* virtio_ioctl.c */ #define DRM_VIRTIO_NUM_IOCTLS 10 extern struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS]; -void virtio_gpu_create_context(struct drm_device *dev, - struct drm_file *file); /* virtio_kms.c */ int virtio_gpu_init(struct drm_device *dev); diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index a98884462944..d2f17778bdc4 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -33,8 +33,8 @@ #include "virtgpu_drv.h" -void virtio_gpu_create_context(struct drm_device *dev, - struct drm_file *file) +static void virtio_gpu_create_context(struct drm_device *dev, + struct drm_file *file) { struct virtio_gpu_device *vgdev = dev->dev_private; struct virtio_gpu_fpriv *vfpriv = file->driver_priv; @@ -93,6 +93,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, exbuf->fence_fd = -1; + virtio_gpu_create_context(dev, file); if (exbuf->flags & VIRTGPU_EXECBUF_FENCE_FD_IN) { struct dma_fence *in_fence; @@ -243,6 +244,7 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data, return -EINVAL; } + virtio_gpu_create_context(dev, file); params.format = rc->format; params.width = rc->width; params.height = rc->height; @@ -316,6 +318,7 @@ static int virtio_gpu_transfer_from_host_ioctl(struct drm_device *dev, if (vgdev->has_virgl_3d == false) return -ENOSYS; + virtio_gpu_create_context(dev, file); objs = virtio_gpu_array_from_handles(file, >bo_handle, 1); if (objs == NULL) return -ENOENT; @@ -363,6 +366,7 @@ static int virtio_gpu_transfer_to_host_ioctl(struct drm_device *dev, void *data, args->box.w, args->box.h, args->box.x, args->box.y, objs, NULL); } else { + virtio_gpu_create_context(dev, file); ret = virtio_gpu_array_lock_resv(objs); if (ret != 0) goto err_put_free; @@ -463,6 +467,7 @@ static int virtio_gpu_get_caps_ioctl(struct drm_device *dev, spin_unlock(>display_info_lock); /* not in cache - need to talk to hw */ + virtio_gpu_create_context(dev, file); virtio_gpu_cmd_get_capset(vgdev, found_valid, args->cap_set_ver, _ent); diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c index 25248bad3fc4..f84f2cb7546a 100644 --- a/drivers/gpu/drm/virtio/virtgpu_kms.c +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c @@ -265,7 +265,6 @@ int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file) vfpriv->ctx_id = handle + 1; atomic_set(>context_initiated, 0); file->driver_priv = vfpriv; - virtio_gpu_create_context(dev, file); return 0; } -- 2.25.0.265.gbab2e86ba0-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/5] drm/virtio: track whether or not a context has been initiated
Use an atomic variable to track whether a context has been initiated. v2: Fix possible race (@olv) Signed-off-by: Gurchetan Singh --- drivers/gpu/drm/virtio/virtgpu_drv.h | 1 + drivers/gpu/drm/virtio/virtgpu_ioctl.c | 3 +++ drivers/gpu/drm/virtio/virtgpu_kms.c | 1 + 3 files changed, 5 insertions(+) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 72c1d9b59dfe..ca505984f8ab 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -209,6 +209,7 @@ struct virtio_gpu_device { struct virtio_gpu_fpriv { uint32_t ctx_id; + atomic_t context_initiated; }; /* virtio_ioctl.c */ diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 896c3f419a6d..a98884462944 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -44,6 +44,9 @@ void virtio_gpu_create_context(struct drm_device *dev, if (!vgdev->has_virgl_3d) return; + if (!atomic_add_unless(>context_initiated, 1, 1)) + return; + get_task_comm(dbgname, current); virtio_gpu_cmd_context_create(vgdev, vfpriv->ctx_id, strlen(dbgname), dbgname); diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c index 282558576527..25248bad3fc4 100644 --- a/drivers/gpu/drm/virtio/virtgpu_kms.c +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c @@ -263,6 +263,7 @@ int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file) } vfpriv->ctx_id = handle + 1; + atomic_set(>context_initiated, 0); file->driver_priv = vfpriv; virtio_gpu_create_context(dev, file); return 0; -- 2.25.0.265.gbab2e86ba0-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/5] drm/virtio: factor out context create hyercall
We currently create an OpenGL context when opening the DRM fd if 3D is available. We may need other context types (VK,..) in the future, and the plan is to have explicit initialization for that. For explicit initialization to work, we need to factor out virtio_gpu_create_context from driver initialization. v2: Move context handle initialization too (@olv) Signed-off-by: Gurchetan Singh --- drivers/gpu/drm/virtio/virtgpu_drv.h | 2 ++ drivers/gpu/drm/virtio/virtgpu_ioctl.c | 16 drivers/gpu/drm/virtio/virtgpu_kms.c | 25 ++--- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 2f6c4ccbfd14..72c1d9b59dfe 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -214,6 +214,8 @@ struct virtio_gpu_fpriv { /* virtio_ioctl.c */ #define DRM_VIRTIO_NUM_IOCTLS 10 extern struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS]; +void virtio_gpu_create_context(struct drm_device *dev, + struct drm_file *file); /* virtio_kms.c */ int virtio_gpu_init(struct drm_device *dev); diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index b0edf1ca095f..896c3f419a6d 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -33,6 +33,22 @@ #include "virtgpu_drv.h" +void virtio_gpu_create_context(struct drm_device *dev, + struct drm_file *file) +{ + struct virtio_gpu_device *vgdev = dev->dev_private; + struct virtio_gpu_fpriv *vfpriv = file->driver_priv; + char dbgname[TASK_COMM_LEN]; + + /* can't create contexts without 3d renderer */ + if (!vgdev->has_virgl_3d) + return; + + get_task_comm(dbgname, current); + virtio_gpu_cmd_context_create(vgdev, vfpriv->ctx_id, + strlen(dbgname), dbgname); +} + static int virtio_gpu_map_ioctl(struct drm_device *dev, void *data, struct drm_file *file) { diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c index 8fd7acef960f..282558576527 100644 --- a/drivers/gpu/drm/virtio/virtgpu_kms.c +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c @@ -52,18 +52,6 @@ static void virtio_gpu_config_changed_work_func(struct work_struct *work) events_clear, _clear); } -static int virtio_gpu_context_create(struct virtio_gpu_device *vgdev, - uint32_t nlen, const char *name) -{ - int handle = ida_alloc(>ctx_id_ida, GFP_KERNEL); - - if (handle < 0) - return handle; - handle += 1; - virtio_gpu_cmd_context_create(vgdev, handle, nlen, name); - return handle; -} - static void virtio_gpu_context_destroy(struct virtio_gpu_device *vgdev, uint32_t ctx_id) { @@ -257,8 +245,7 @@ int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file) { struct virtio_gpu_device *vgdev = dev->dev_private; struct virtio_gpu_fpriv *vfpriv; - int id; - char dbgname[TASK_COMM_LEN]; + int handle; /* can't create contexts without 3d renderer */ if (!vgdev->has_virgl_3d) @@ -269,15 +256,15 @@ int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file) if (!vfpriv) return -ENOMEM; - get_task_comm(dbgname, current); - id = virtio_gpu_context_create(vgdev, strlen(dbgname), dbgname); - if (id < 0) { + handle = ida_alloc(>ctx_id_ida, GFP_KERNEL); + if (handle < 0) { kfree(vfpriv); - return id; + return handle; } - vfpriv->ctx_id = id; + vfpriv->ctx_id = handle + 1; file->driver_priv = vfpriv; + virtio_gpu_create_context(dev, file); return 0; } -- 2.25.0.265.gbab2e86ba0-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/5] drm/virtio: use consistent names for drm_files
Minor cleanup, change: - file_priv--> file, - drm_file --> file. Signed-off-by: Gurchetan Singh --- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 467649733d24..b0edf1ca095f 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -34,12 +34,12 @@ #include "virtgpu_drv.h" static int virtio_gpu_map_ioctl(struct drm_device *dev, void *data, - struct drm_file *file_priv) + struct drm_file *file) { struct virtio_gpu_device *vgdev = dev->dev_private; struct drm_virtgpu_map *virtio_gpu_map = data; - return virtio_gpu_mode_dumb_mmap(file_priv, vgdev->ddev, + return virtio_gpu_mode_dumb_mmap(file, vgdev->ddev, virtio_gpu_map->handle, _gpu_map->offset); } @@ -51,11 +51,11 @@ static int virtio_gpu_map_ioctl(struct drm_device *dev, void *data, * VIRTIO_GPUReleaseInfo struct (first XXX bytes) */ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, -struct drm_file *drm_file) +struct drm_file *file) { struct drm_virtgpu_execbuffer *exbuf = data; struct virtio_gpu_device *vgdev = dev->dev_private; - struct virtio_gpu_fpriv *vfpriv = drm_file->driver_priv; + struct virtio_gpu_fpriv *vfpriv = file->driver_priv; struct virtio_gpu_fence *out_fence; int ret; uint32_t *bo_handles = NULL; @@ -116,7 +116,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, goto out_unused_fd; } - buflist = virtio_gpu_array_from_handles(drm_file, bo_handles, + buflist = virtio_gpu_array_from_handles(file, bo_handles, exbuf->num_bo_handles); if (!buflist) { ret = -ENOENT; @@ -177,7 +177,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, } static int virtio_gpu_getparam_ioctl(struct drm_device *dev, void *data, -struct drm_file *file_priv) +struct drm_file *file) { struct virtio_gpu_device *vgdev = dev->dev_private; struct drm_virtgpu_getparam *param = data; @@ -200,7 +200,7 @@ static int virtio_gpu_getparam_ioctl(struct drm_device *dev, void *data, } static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data, - struct drm_file *file_priv) + struct drm_file *file) { struct virtio_gpu_device *vgdev = dev->dev_private; struct drm_virtgpu_resource_create *rc = data; @@ -251,7 +251,7 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data, return ret; obj = >base.base; - ret = drm_gem_handle_create(file_priv, obj, ); + ret = drm_gem_handle_create(file, obj, ); if (ret) { drm_gem_object_release(obj); return ret; @@ -264,13 +264,13 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data, } static int virtio_gpu_resource_info_ioctl(struct drm_device *dev, void *data, - struct drm_file *file_priv) + struct drm_file *file) { struct drm_virtgpu_resource_info *ri = data; struct drm_gem_object *gobj = NULL; struct virtio_gpu_object *qobj = NULL; - gobj = drm_gem_object_lookup(file_priv, ri->bo_handle); + gobj = drm_gem_object_lookup(file, ri->bo_handle); if (gobj == NULL) return -ENOENT; -- 2.25.0.265.gbab2e86ba0-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH] nouveau: no need to check return value of debugfs_create functions
On Thu, Feb 13, 2020 at 02:30:09PM -0800, John Hubbard wrote: > On 2/9/20 2:55 AM, Greg Kroah-Hartman wrote: > > When calling debugfs functions, there is no need to ever check the > > return value. The function can work or not, but the code logic should > > never do something different based on this. > > > > Should we follow that line of reasoning further, and simply return void > from the debugfs functions--rather than playing whack-a-mole with this > indefinitely? That is what we (well I) have been doing. Look at all of the changes that have happened to include/linux/debugfs.h over the past few releases. I'm slowly winnowing down the api to make it impossible to get wrong for this type of thing, and am almost there. DRM is the big fish left to tackle, I have submitted some patches in the past, but lots more cleanup needs to be done to get them into mergable shape. I just need to find the time... thanks, greg k-h ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH] nouveau: no need to check return value of debugfs_create functions
On 2/9/20 2:55 AM, Greg Kroah-Hartman wrote: > When calling debugfs functions, there is no need to ever check the > return value. The function can work or not, but the code logic should > never do something different based on this. > Should we follow that line of reasoning further, and simply return void from the debugfs functions--rather than playing whack-a-mole with this indefinitely? thanks, -- John Hubbard NVIDIA > Cc: Ben Skeggs > Cc: David Airlie > Cc: Daniel Vetter > Cc: dri-devel@lists.freedesktop.org > Cc: nouv...@lists.freedesktop.org > Signed-off-by: Greg Kroah-Hartman > --- > drivers/gpu/drm/nouveau/nouveau_debugfs.c | 12 > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_debugfs.c > b/drivers/gpu/drm/nouveau/nouveau_debugfs.c > index 080e964d49aa..d1c82fc45a68 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_debugfs.c > +++ b/drivers/gpu/drm/nouveau/nouveau_debugfs.c > @@ -224,14 +224,10 @@ nouveau_drm_debugfs_init(struct drm_minor *minor) > struct dentry *dentry; > int i; > > - for (i = 0; i < ARRAY_SIZE(nouveau_debugfs_files); i++) { > - dentry = debugfs_create_file(nouveau_debugfs_files[i].name, > - S_IRUGO | S_IWUSR, > - minor->debugfs_root, minor->dev, > - nouveau_debugfs_files[i].fops); > - if (!dentry) > - return -ENOMEM; > - } > + for (i = 0; i < ARRAY_SIZE(nouveau_debugfs_files); i++) > + debugfs_create_file(nouveau_debugfs_files[i].name, > + S_IRUGO | S_IWUSR, minor->debugfs_root, > + minor->dev, nouveau_debugfs_files[i].fops); > > drm_debugfs_create_files(nouveau_debugfs_list, >NOUVEAU_DEBUGFS_ENTRIES, > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 0/3] KVM: x86: honor guest memory type
On Thu, Feb 13, 2020 at 1:41 PM Paolo Bonzini wrote: > > On 13/02/20 22:30, Chia-I Wu wrote: > > Hi, > > > > Host GPU drivers like to give userspace WC mapping. When the userspace > > makes > > the mapping available to a guest, it also tells the guest to create a WC > > mapping. However, even when the guest kernel picks the correct memory type, > > it gets ignored because of VMX_EPT_IPAT_BIT on Intel. > > > > This series adds a new flag to KVM_SET_USER_MEMORY_REGION, which tells the > > host kernel to honor the guest memory type for the memslot. An alternative > > fix is for KVM to unconditionally honor the guest memory type (unless it is > > MMIO, to avoid MCEs on Intel). I believe the alternative fix is how things > > are on ARM, and probably also how things are on AMD. > > > > I am new to KVM and HW virtualization technologies. This series is meant as > > an RFC. > > > > When we tried to do this in the past, we got machine checks everywhere > unfortunately due to the same address being mapped with different memory > types. Unfortunately I cannot find the entry anymore in bugzilla, but > this was not fixed as far as I know. Yeah, I did a bit of history digging here https://gitlab.freedesktop.org/virgl/virglrenderer/issues/151#note_372594 The bug you mentioned was probably this one https://bugzilla.kernel.org/show_bug.cgi?id=104091 which was caused by https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fd717f11015f673487ffc826e59b2bad69d20fe5 >From what I can tell, the commit allowed the guests to create cached mappings to MMIO regions and caused MCEs. That is different than what I need, which is to allow guests to create uncached mappings to system ram (i.e., !kvm_is_mmio_pfn) when the host userspace also has uncached mappings. But it is true that this still allows the userspace & guest kernel to create conflicting memory types. Implementation-wise, the current implementation uses kvm_arch_register_noncoherent_dma. It essentially treats a memslot with the new flag as a non-coherent vfio device as far as mmu is concerned. > > Paolo > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare
On Thu, 13 Feb 2020, Nathan Chancellor wrote: > On Thu, Feb 13, 2020 at 04:37:15PM +0200, Jani Nikula wrote: >> On Wed, 12 Feb 2020, Michel Dänzer wrote: >> > On 2020-02-12 6:07 p.m., Nathan Chancellor wrote: >> >> On Wed, Feb 12, 2020 at 09:52:52AM +0100, Michel Dänzer wrote: >> >>> On 2020-02-11 9:39 p.m., Nathan Chancellor wrote: >> On Tue, Feb 11, 2020 at 10:41:48AM +0100, Michel Dänzer wrote: >> > On 2020-02-11 7:13 a.m., Nathan Chancellor wrote: >> >> A recent commit in clang added -Wtautological-compare to -Wall, which >> >> is >> >> enabled for i915 so we see the following warning: >> >> >> >> ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1485:22: warning: >> >> result of comparison of constant 576460752303423487 with expression of >> >> type 'unsigned int' is always false >> >> [-Wtautological-constant-out-of-range-compare] >> >> if (unlikely(remain > N_RELOC(ULONG_MAX))) >> >> ^ >> >> >> >> This warning only happens on x86_64 but that check is relevant for >> >> 32-bit x86 so we cannot remove it. >> > >> > That's suprising. AFAICT N_RELOC(ULONG_MAX) works out to the same value >> > in both cases, and remain is a 32-bit value in both cases. How can it >> > be >> > larger than N_RELOC(ULONG_MAX) on 32-bit (but not on 64-bit)? >> > >> >> Hi Michel, >> >> Can't this condition be true when UINT_MAX == ULONG_MAX? >> >>> >> >>> Oh, right, I think I was wrongly thinking long had 64 bits even on >> >>> 32-bit. >> >>> >> >>> >> >>> Anyway, this suggests a possible better solution: >> >>> >> >>> #if UINT_MAX == ULONG_MAX >> >>> if (unlikely(remain > N_RELOC(ULONG_MAX))) >> >>> return -EINVAL; >> >>> #endif >> >>> >> >>> >> >>> Or if that can't be used for some reason, something like >> >>> >> >>> if (unlikely((unsigned long)remain > N_RELOC(ULONG_MAX))) >> >>> return -EINVAL; >> >>> >> >>> should silence the warning. >> >> >> >> I do like this one better than the former. >> > >> > FWIW, one downside of this one compared to all alternatives (presumably) >> > is that it might end up generating actual code even on 64-bit, which >> > always ends up skipping the return. >> >> I like this better than the UINT_MAX == ULONG_MAX comparison because >> that creates a dependency on the type of remain. >> >> Then again, a sufficiently clever compiler could see through the cast, >> and flag the warning anyway... > > Would you prefer a patch that adds that cast rather than silencing the > warning outright? It does appear to work for clang. I'd take the cast. If that fails for whatever reason, per-file CFLAGS_gem/i915_gem_execbuffer.o = $(call cc-disable-warning, tautological-constant-out-of-range-compare) over subdir-ccflags-y would be preferrable I think. BR, Jani. > > Cheers, > Nathan -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] ARM: dts: am437x-gp/epos-evm: drop unused panel timings
Hi, On Tue, Feb 11, 2020 at 07:22:14PM +0200, Tomi Valkeinen wrote: > On 11/02/2020 18:27, Tony Lindgren wrote: > > > We are still missing DSI command mode support, and moving it > > > to the common DRM model. > > > > Nope, DSI command mode support has been working just fine for > > a while now :) And Sebastian has a WIP git tree of the common DRM > > Indeed... It had been going on for so long that now my mind is > stuck at dsi-command-mode-not-yet-in =). Welcome in the future :) > > model changes for it. I don't think we have devices with DSI > > command mode working for omapfb but not for omapdrm? > > Yes, I think that is true. Note, that OMAP3 quirk is missing (IDK if its supported in omapfb, haven't used it for ages). I planned to have a look at OMAP3 once the patchset moving omapdrm DSI to common DRM is merged, which needs a non-trivial rebase. > > What got missed for v5.6-rc1 is the LCD backlight patch though, > > I think the only issue there is default-brightness vs more common > > default-brightness-value usage if you have any input to that. > > At least for some boards a power supply is needed, and I think > there was no conclusion on who should enable that. It didn't seem > to fit in anywhere... > > But need to check on the latest status. I wasn't following that > work closely, as JJ was working on it. FWIW omapdrm's DSI driver is ready for that and omapfb is not :P -- Sebastian signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 0/3] KVM: x86: honor guest memory type
On 13/02/20 22:30, Chia-I Wu wrote: > Hi, > > Host GPU drivers like to give userspace WC mapping. When the userspace makes > the mapping available to a guest, it also tells the guest to create a WC > mapping. However, even when the guest kernel picks the correct memory type, > it gets ignored because of VMX_EPT_IPAT_BIT on Intel. > > This series adds a new flag to KVM_SET_USER_MEMORY_REGION, which tells the > host kernel to honor the guest memory type for the memslot. An alternative > fix is for KVM to unconditionally honor the guest memory type (unless it is > MMIO, to avoid MCEs on Intel). I believe the alternative fix is how things > are on ARM, and probably also how things are on AMD. > > I am new to KVM and HW virtualization technologies. This series is meant as > an RFC. > When we tried to do this in the past, we got machine checks everywhere unfortunately due to the same address being mapped with different memory types. Unfortunately I cannot find the entry anymore in bugzilla, but this was not fixed as far as I know. Paolo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 1/3] KVM: vmx: rewrite the comment in vmx_get_mt_mask
Better reflect the structure of the code and metion why we could not always honor the guest. Signed-off-by: Chia-I Wu Cc: Gurchetan Singh Cc: Gerd Hoffmann --- arch/x86/kvm/vmx/vmx.c | 27 +-- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 3be25ecae145..266ef87042da 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -6854,17 +6854,24 @@ static u64 vmx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio) u8 cache; u64 ipat = 0; - /* For VT-d and EPT combination -* 1. MMIO: always map as UC -* 2. EPT with VT-d: -* a. VT-d without snooping control feature: can't guarantee the -* result, try to trust guest. -* b. VT-d with snooping control feature: snooping control feature of -* VT-d engine can guarantee the cache correctness. Just set it -* to WB to keep consistent with host. So the same as item 3. -* 3. EPT without VT-d: always map as WB and set IPAT=1 to keep -*consistent with host MTRR + /* We wanted to honor guest CD/MTRR/PAT, but doing so could result in +* memory aliases with conflicting memory types and sometimes MCEs. +* We have to be careful as to what are honored and when. +* +* For MMIO, guest CD/MTRR are ignored. The EPT memory type is set to +* UC. The effective memory type is UC or WC depending on guest PAT. +* This was historically the source of MCEs and we want to be +* conservative. +* +* When there is no need to deal with noncoherent DMA (e.g., no VT-d +* or VT-d has snoop control), guest CD/MTRR/PAT are all ignored. The +* EPT memory type is set to WB. The effective memory type is forced +* WB. +* +* Otherwise, we trust guest. Guest CD/MTRR/PAT are all honored. The +* EPT memory type is used to emulate guest CD/MTRR. */ + if (is_mmio) { cache = MTRR_TYPE_UNCACHABLE; goto exit; -- 2.25.0.265.gbab2e86ba0-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 3/3] RFC: KVM: x86: support KVM_CAP_DMA_MEM
When a memslot has KVM_MEM_DMA set, we want VMX_EPT_IPAT_BIT cleared for the memslot. Before that is possible, simply call kvm_arch_register_noncoherent_dma for the memslot. SVM does not have the ignore-pat bit. Guest PAT is always honored. Signed-off-by: Chia-I Wu Cc: Gurchetan Singh Cc: Gerd Hoffmann --- arch/x86/include/uapi/asm/kvm.h | 1 + arch/x86/kvm/x86.c | 6 ++ 2 files changed, 7 insertions(+) diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h index 503d3f42da16..578b686e3880 100644 --- a/arch/x86/include/uapi/asm/kvm.h +++ b/arch/x86/include/uapi/asm/kvm.h @@ -48,6 +48,7 @@ #define __KVM_HAVE_XSAVE #define __KVM_HAVE_XCRS #define __KVM_HAVE_READONLY_MEM +#define __KVM_HAVE_DMA_MEM /* Architectural interrupt line count. */ #define KVM_NR_INTERRUPTS 256 diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index fb5d64ebc35d..c89a4647fef6 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3331,6 +3331,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) case KVM_CAP_GET_TSC_KHZ: case KVM_CAP_KVMCLOCK_CTRL: case KVM_CAP_READONLY_MEM: + case KVM_CAP_DMA_MEM: case KVM_CAP_HYPERV_TIME: case KVM_CAP_IOAPIC_POLARITY_IGNORED: case KVM_CAP_TSC_DEADLINE_TIMER: @@ -10045,6 +10046,11 @@ void kvm_arch_commit_memory_region(struct kvm *kvm, */ if (change != KVM_MR_DELETE) kvm_mmu_slot_apply_flags(kvm, (struct kvm_memory_slot *) new); + + if (change == KVM_MR_CREATE && new->flags & KVM_MEM_DMA) + kvm_arch_register_noncoherent_dma(kvm); + else if (change == KVM_MR_DELETE && old->flags & KVM_MEM_DMA) + kvm_arch_unregister_noncoherent_dma(kvm); } void kvm_arch_flush_shadow_all(struct kvm *kvm) -- 2.25.0.265.gbab2e86ba0-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 2/3] RFC: KVM: add KVM_MEM_DMA
When the flag is set, it means the the userspace wants to do DMA with the memory and the guest will use an appropriate memory type to access the memory. The kernel should be prepared to honor the guest's memory type. Signed-off-by: Chia-I Wu Cc: Gurchetan Singh Cc: Gerd Hoffmann --- Documentation/virt/kvm/api.rst | 17 +++-- include/uapi/linux/kvm.h | 2 ++ virt/kvm/kvm_main.c| 6 +- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst index 97a72a53fa4b..e6a27e6e45c2 100644 --- a/Documentation/virt/kvm/api.rst +++ b/Documentation/virt/kvm/api.rst @@ -1237,6 +1237,7 @@ yet and must be cleared on entry. /* for kvm_memory_region::flags */ #define KVM_MEM_LOG_DIRTY_PAGES (1UL << 0) #define KVM_MEM_READONLY (1UL << 1) + #define KVM_MEM_DMA (1UL << 2) This ioctl allows the user to create, modify or delete a guest physical memory slot. Bits 0-15 of "slot" specify the slot id and this value @@ -1264,12 +1265,16 @@ It is recommended that the lower 21 bits of guest_phys_addr and userspace_addr be identical. This allows large pages in the guest to be backed by large pages in the host. -The flags field supports two flags: KVM_MEM_LOG_DIRTY_PAGES and -KVM_MEM_READONLY. The former can be set to instruct KVM to keep track of -writes to memory within the slot. See KVM_GET_DIRTY_LOG ioctl to know how to -use it. The latter can be set, if KVM_CAP_READONLY_MEM capability allows it, -to make a new slot read-only. In this case, writes to this memory will be -posted to userspace as KVM_EXIT_MMIO exits. +The flags field supports these flags: KVM_MEM_LOG_DIRTY_PAGES, +KVM_MEM_READONLY, and KVM_MEM_DMA. KVM_MEM_LOG_DIRTY_PAGES can be set to +instruct KVM to keep track of writes to memory within the slot. See +KVM_GET_DIRTY_LOG ioctl to know how to use it. KVM_MEM_READONLY can be set, +if KVM_CAP_READONLY_MEM capability allows it, to make a new slot read-only. +In this case, writes to this memory will be posted to userspace as +KVM_EXIT_MMIO exits. KVM_MEM_DMA can be set, if KVM_CAP_DMA_MEM capability +allows it, to make a new slot support DMA. It is the userspace's +responsibility to make sure userspace_addr points at a DMA-able memory and the +guest's responsibility to map guest_phys_addr with the proper memory type. When the KVM_CAP_SYNC_MMU capability is available, changes in the backing of the memory region are automatically reflected into the guest. For example, an diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index 4b95f9a31a2f..578292e4b072 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -109,6 +109,7 @@ struct kvm_userspace_memory_region { */ #define KVM_MEM_LOG_DIRTY_PAGES(1UL << 0) #define KVM_MEM_READONLY (1UL << 1) +#define KVM_MEM_DMA(1UL << 2) /* for KVM_IRQ_LINE */ struct kvm_irq_level { @@ -1010,6 +1011,7 @@ struct kvm_ppc_resize_hpt { #define KVM_CAP_ARM_NISV_TO_USER 177 #define KVM_CAP_ARM_INJECT_EXT_DABT 178 #define KVM_CAP_S390_VCPU_RESETS 179 +#define KVM_CAP_DMA_MEM 180 #ifdef KVM_CAP_IRQ_ROUTING diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 70f03ce0e5c1..a4b6c782a168 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -940,6 +940,9 @@ static int check_memory_region_flags(const struct kvm_userspace_memory_region *m #ifdef __KVM_HAVE_READONLY_MEM valid_flags |= KVM_MEM_READONLY; #endif +#ifdef __KVM_HAVE_DMA_MEM + valid_flags |= KVM_MEM_DMA; +#endif if (mem->flags & ~valid_flags) return -EINVAL; @@ -1047,7 +1050,8 @@ int __kvm_set_memory_region(struct kvm *kvm, else { /* Modify an existing slot. */ if ((mem->userspace_addr != old.userspace_addr) || (npages != old.npages) || - ((new.flags ^ old.flags) & KVM_MEM_READONLY)) + ((new.flags ^ old.flags) & +(KVM_MEM_READONLY | KVM_MEM_DMA))) goto out; if (base_gfn != old.base_gfn) -- 2.25.0.265.gbab2e86ba0-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 0/3] KVM: x86: honor guest memory type
Hi, Host GPU drivers like to give userspace WC mapping. When the userspace makes the mapping available to a guest, it also tells the guest to create a WC mapping. However, even when the guest kernel picks the correct memory type, it gets ignored because of VMX_EPT_IPAT_BIT on Intel. This series adds a new flag to KVM_SET_USER_MEMORY_REGION, which tells the host kernel to honor the guest memory type for the memslot. An alternative fix is for KVM to unconditionally honor the guest memory type (unless it is MMIO, to avoid MCEs on Intel). I believe the alternative fix is how things are on ARM, and probably also how things are on AMD. I am new to KVM and HW virtualization technologies. This series is meant as an RFC. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/msm/dpu: fix BGR565 vs RGB565 confusion
On Thu, Feb 13, 2020 at 3:03 PM Rob Clark wrote: > > From: Rob Clark > > The component order between the two was swapped, resulting in incorrect > color when games with 565 visual hit the overlay path instead of GPU > composition. > > Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support") > Signed-off-by: Rob Clark Reviewed-by: Sean Paul > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c > index 24ab6249083a..6f420cc73dbd 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c > @@ -255,13 +255,13 @@ static const struct dpu_format dpu_format_map[] = { > > INTERLEAVED_RGB_FMT(RGB565, > 0, COLOR_5BIT, COLOR_6BIT, COLOR_5BIT, > - C2_R_Cr, C0_G_Y, C1_B_Cb, 0, 3, > + C1_B_Cb, C0_G_Y, C2_R_Cr, 0, 3, > false, 2, 0, > DPU_FETCH_LINEAR, 1), > > INTERLEAVED_RGB_FMT(BGR565, > 0, COLOR_5BIT, COLOR_6BIT, COLOR_5BIT, > - C1_B_Cb, C0_G_Y, C2_R_Cr, 0, 3, > + C2_R_Cr, C0_G_Y, C1_B_Cb, 0, 3, > false, 2, 0, > DPU_FETCH_LINEAR, 1), > > -- > 2.24.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] drm/mst: Support simultaneous down replies
From: Sean Paul Currently we have one down reply message servicing the mst manager, so we need to serialize all tx msgs to ensure we only have one message in flight at a time. For obvious reasons this is suboptimal (but less suboptimal than the free-for-all we had before serialization). This patch removes the single down_rep_recv message from manager and adds 2 replies in the branch structure. The 2 replies mirrors the tx_slots which we use to rate-limit outgoing messages and correspond to seqno in the packet headers. Cc: Wayne Lin Signed-off-by: Sean Paul --- drivers/gpu/drm/drm_dp_mst_topology.c | 80 --- include/drm/drm_dp_mst_helper.h | 59 ++-- 2 files changed, 78 insertions(+), 61 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index e8bb39bb17a0f..7e6a82efdfc02 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -3690,7 +3690,8 @@ int drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr, } EXPORT_SYMBOL(drm_dp_mst_topology_mgr_resume); -static bool drm_dp_get_one_sb_msg(struct drm_dp_mst_topology_mgr *mgr, bool up) +static bool drm_dp_get_one_sb_msg(struct drm_dp_mst_topology_mgr *mgr, bool up, + struct drm_dp_mst_branch **mstb, int *seqno) { int len; u8 replyblock[32]; @@ -3702,7 +3703,8 @@ static bool drm_dp_get_one_sb_msg(struct drm_dp_mst_topology_mgr *mgr, bool up) int basereg = up ? DP_SIDEBAND_MSG_UP_REQ_BASE : DP_SIDEBAND_MSG_DOWN_REP_BASE; - msg = up ? >up_req_recv : >down_rep_recv; + *mstb = NULL; + *seqno = -1; len = min(mgr->max_dpcd_transaction_bytes, 16); ret = drm_dp_dpcd_read(mgr->aux, basereg, replyblock, len); @@ -3719,6 +3721,21 @@ static bool drm_dp_get_one_sb_msg(struct drm_dp_mst_topology_mgr *mgr, bool up) return false; } + *seqno = hdr.seqno; + + if (up) { + msg = >up_req_recv; + } else { + /* Caller is responsible for giving back this reference */ + *mstb = drm_dp_get_mst_branch_device(mgr, hdr.lct, hdr.rad); + if (!*mstb) { + DRM_DEBUG_KMS("Got MST reply from unknown device %d\n", + hdr.lct); + return false; + } + msg = &(*mstb)->down_rep_recv[hdr.seqno]; + } + if (!drm_dp_sideband_msg_set_header(msg, , hdrlen)) { DRM_DEBUG_KMS("sideband msg set header failed %d\n", replyblock[0]); @@ -3759,53 +3776,52 @@ static bool drm_dp_get_one_sb_msg(struct drm_dp_mst_topology_mgr *mgr, bool up) static int drm_dp_mst_handle_down_rep(struct drm_dp_mst_topology_mgr *mgr) { struct drm_dp_sideband_msg_tx *txmsg; - struct drm_dp_mst_branch *mstb; - struct drm_dp_sideband_msg_hdr *hdr = >down_rep_recv.initial_hdr; - int slot = -1; + struct drm_dp_mst_branch *mstb = NULL; + struct drm_dp_sideband_msg_rx *msg = NULL; + int seqno = -1; - if (!drm_dp_get_one_sb_msg(mgr, false)) - goto clear_down_rep_recv; + if (!drm_dp_get_one_sb_msg(mgr, false, , )) + goto out_clear_reply; - if (!mgr->down_rep_recv.have_eomt) - return 0; + msg = >down_rep_recv[seqno]; - mstb = drm_dp_get_mst_branch_device(mgr, hdr->lct, hdr->rad); - if (!mstb) { - DRM_DEBUG_KMS("Got MST reply from unknown device %d\n", - hdr->lct); - goto clear_down_rep_recv; - } + /* Multi-packet message transmission, don't clear the reply */ + if (!msg->have_eomt) + goto out; /* find the message */ - slot = hdr->seqno; mutex_lock(>qlock); - txmsg = mstb->tx_slots[slot]; + txmsg = mstb->tx_slots[seqno]; /* remove from slots */ mutex_unlock(>qlock); if (!txmsg) { + struct drm_dp_sideband_msg_hdr *hdr; + hdr = >initial_hdr; DRM_DEBUG_KMS("Got MST reply with no msg %p %d %d %02x %02x\n", mstb, hdr->seqno, hdr->lct, hdr->rad[0], - mgr->down_rep_recv.msg[0]); - goto no_msg; + msg->msg[0]); + goto out_clear_reply; } - drm_dp_sideband_parse_reply(>down_rep_recv, >reply); + drm_dp_sideband_parse_reply(msg, >reply); - if (txmsg->reply.reply_type == DP_SIDEBAND_REPLY_NAK) + if (txmsg->reply.reply_type == DP_SIDEBAND_REPLY_NAK) { DRM_DEBUG_KMS("Got NAK reply: req 0x%02x (%s), reason 0x%02x (%s), nak data 0x%02x\n", txmsg->reply.req_type,
[PATCH 3/3] drm/dp_mst: Remove single tx msg restriction.
From: Sean Paul Now that we can support multiple simultaneous replies, remove the restrictions placed on sending new tx msgs. This patch essentially just reverts commit 5a64967a2f3b ("drm/dp_mst: Have DP_Tx send one msg at a time") now that the problem is solved in a different way. Cc: Wayne Lin Signed-off-by: Sean Paul --- drivers/gpu/drm/drm_dp_mst_topology.c | 14 ++ include/drm/drm_dp_mst_helper.h | 6 -- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 7e6a82efdfc02..cbf0bb0ddeb84 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -1203,8 +1203,6 @@ static int drm_dp_mst_wait_tx_reply(struct drm_dp_mst_branch *mstb, txmsg->state == DRM_DP_SIDEBAND_TX_SENT) { mstb->tx_slots[txmsg->seqno] = NULL; } - mgr->is_waiting_for_dwn_reply = false; - } out: if (unlikely(ret == -EIO) && drm_debug_enabled(DRM_UT_DP)) { @@ -1214,7 +1212,6 @@ static int drm_dp_mst_wait_tx_reply(struct drm_dp_mst_branch *mstb, } mutex_unlock(>qlock); - drm_dp_mst_kick_tx(mgr); return ret; } @@ -2797,11 +2794,9 @@ static void process_single_down_tx_qlock(struct drm_dp_mst_topology_mgr *mgr) ret = process_single_tx_qlock(mgr, txmsg, false); if (ret == 1) { /* txmsg is sent it should be in the slots now */ - mgr->is_waiting_for_dwn_reply = true; list_del(>next); } else if (ret) { DRM_DEBUG_KMS("failed to send msg in q %d\n", ret); - mgr->is_waiting_for_dwn_reply = false; list_del(>next); if (txmsg->seqno != -1) txmsg->dst->tx_slots[txmsg->seqno] = NULL; @@ -2841,8 +2836,7 @@ static void drm_dp_queue_down_tx(struct drm_dp_mst_topology_mgr *mgr, drm_dp_mst_dump_sideband_msg_tx(, txmsg); } - if (list_is_singular(>tx_msg_downq) && - !mgr->is_waiting_for_dwn_reply) + if (list_is_singular(>tx_msg_downq)) process_single_down_tx_qlock(mgr); mutex_unlock(>qlock); } @@ -3822,7 +3816,6 @@ static int drm_dp_mst_handle_down_rep(struct drm_dp_mst_topology_mgr *mgr) mutex_lock(>qlock); txmsg->state = DRM_DP_SIDEBAND_TX_RX; mstb->tx_slots[seqno] = NULL; - mgr->is_waiting_for_dwn_reply = false; mutex_unlock(>qlock); wake_up_all(>tx_waitq); @@ -3830,9 +3823,6 @@ static int drm_dp_mst_handle_down_rep(struct drm_dp_mst_topology_mgr *mgr) return 0; out_clear_reply: - mutex_lock(>qlock); - mgr->is_waiting_for_dwn_reply = false; - mutex_unlock(>qlock); if (msg) memset(msg, 0, sizeof(struct drm_dp_sideband_msg_rx)); out: @@ -4670,7 +4660,7 @@ static void drm_dp_tx_work(struct work_struct *work) struct drm_dp_mst_topology_mgr *mgr = container_of(work, struct drm_dp_mst_topology_mgr, tx_work); mutex_lock(>qlock); - if (!list_empty(>tx_msg_downq) && !mgr->is_waiting_for_dwn_reply) + if (!list_empty(>tx_msg_downq)) process_single_down_tx_qlock(mgr); mutex_unlock(>qlock); } diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index 7d0341f94ce1b..fcc30e64c8e7e 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -619,12 +619,6 @@ struct drm_dp_mst_topology_mgr { * _dp_sideband_msg_tx.state once they are queued */ struct mutex qlock; - - /** -* @is_waiting_for_dwn_reply: indicate whether is waiting for down reply -*/ - bool is_waiting_for_dwn_reply; - /** * @tx_msg_downq: List of pending down replies. */ -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/3] drm/mst: Add support for simultaneous down replies
From: Sean Paul Hey all, Earlier this week on my 5.5 kernel (I can't seem to get a 5.6 kernel to behave on any of my devices), I ran into the multi-reply problem that Wayne fixed in January. Without realizing there was already a fix upstream, I went about solving it in different way. It wasn't until rebasing the patches on 5.6 for the list that I realized there was already a solution. At any rate, I think this way of handling things might be a bit more performant. I'm not super happy with the cleanliness of the code, I think this series should make things easier to read, but I don't think I achieved that. So suggestions are welcome on how to break this apart. Thanks, Sean Sean Paul (3): drm/mst: Separate sideband packet header parsing from message building drm/mst: Support simultaneous down replies drm/dp_mst: Remove single tx msg restriction. drivers/gpu/drm/drm_dp_mst_topology.c | 196 ++ include/drm/drm_dp_mst_helper.h | 65 - 2 files changed, 137 insertions(+), 124 deletions(-) -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] drm/mst: Separate sideband packet header parsing from message building
From: Sean Paul In preparation of per-branch device down message handling, separate header parsing from message building. This will allow us to peek at figure out which branch device the message is from before starting to parse the message contents. Signed-off-by: Sean Paul --- drivers/gpu/drm/drm_dp_mst_topology.c | 102 ++ 1 file changed, 57 insertions(+), 45 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index a811247cecfef..e8bb39bb17a0f 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -687,51 +687,45 @@ static void drm_dp_encode_sideband_reply(struct drm_dp_sideband_msg_reply_body * raw->cur_len = idx; } -/* this adds a chunk of msg to the builder to get the final msg */ -static bool drm_dp_sideband_msg_build(struct drm_dp_sideband_msg_rx *msg, - u8 *replybuf, u8 replybuflen, bool hdr) +static int drm_dp_sideband_msg_set_header(struct drm_dp_sideband_msg_rx *msg, + struct drm_dp_sideband_msg_hdr *hdr, + u8 hdrlen) { - int ret; - u8 crc4; + /* +* ignore out-of-order messages or messages that are part of a +* failed transaction +*/ + if (!hdr->somt && !msg->have_somt) + return false; - if (hdr) { - u8 hdrlen; - struct drm_dp_sideband_msg_hdr recv_hdr; - ret = drm_dp_decode_sideband_msg_hdr(_hdr, replybuf, replybuflen, ); - if (ret == false) { - print_hex_dump(KERN_DEBUG, "failed hdr", DUMP_PREFIX_NONE, 16, 1, replybuf, replybuflen, false); - return false; - } + /* get length contained in this portion */ + msg->curchunk_idx = 0; + msg->curchunk_len = hdr->msg_len; + msg->curchunk_hdrlen = hdrlen; - /* -* ignore out-of-order messages or messages that are part of a -* failed transaction -*/ - if (!recv_hdr.somt && !msg->have_somt) - return false; + /* we have already gotten an somt - don't bother parsing */ + if (hdr->somt && msg->have_somt) + return false; - /* get length contained in this portion */ - msg->curchunk_len = recv_hdr.msg_len; - msg->curchunk_hdrlen = hdrlen; + if (hdr->somt) { + memcpy(>initial_hdr, hdr, + sizeof(struct drm_dp_sideband_msg_hdr)); + msg->have_somt = true; + } + if (hdr->eomt) + msg->have_eomt = true; - /* we have already gotten an somt - don't bother parsing */ - if (recv_hdr.somt && msg->have_somt) - return false; + return true; +} - if (recv_hdr.somt) { - memcpy(>initial_hdr, _hdr, sizeof(struct drm_dp_sideband_msg_hdr)); - msg->have_somt = true; - } - if (recv_hdr.eomt) - msg->have_eomt = true; +/* this adds a chunk of msg to the builder to get the final msg */ +static bool drm_dp_sideband_msg_build(struct drm_dp_sideband_msg_rx *msg, + u8 *replybuf, u8 replybuflen) +{ + u8 crc4; - /* copy the bytes for the remainder of this header chunk */ - msg->curchunk_idx = min(msg->curchunk_len, (u8)(replybuflen - hdrlen)); - memcpy(>chunk[0], replybuf + hdrlen, msg->curchunk_idx); - } else { - memcpy(>chunk[msg->curchunk_idx], replybuf, replybuflen); - msg->curchunk_idx += replybuflen; - } + memcpy(>chunk[msg->curchunk_idx], replybuf, replybuflen); + msg->curchunk_idx += replybuflen; if (msg->curchunk_idx >= msg->curchunk_len) { /* do CRC */ @@ -3702,25 +3696,43 @@ static bool drm_dp_get_one_sb_msg(struct drm_dp_mst_topology_mgr *mgr, bool up) u8 replyblock[32]; int replylen, curreply; int ret; + u8 hdrlen; + struct drm_dp_sideband_msg_hdr hdr; struct drm_dp_sideband_msg_rx *msg; - int basereg = up ? DP_SIDEBAND_MSG_UP_REQ_BASE : DP_SIDEBAND_MSG_DOWN_REP_BASE; + int basereg = up ? DP_SIDEBAND_MSG_UP_REQ_BASE : + DP_SIDEBAND_MSG_DOWN_REP_BASE; + msg = up ? >up_req_recv : >down_rep_recv; len = min(mgr->max_dpcd_transaction_bytes, 16); - ret = drm_dp_dpcd_read(mgr->aux, basereg, - replyblock, len); + ret = drm_dp_dpcd_read(mgr->aux, basereg, replyblock, len); if (ret != len) { DRM_DEBUG_KMS("failed to read DPCD down rep %d %d\n", len, ret); return false; } -
[PATCH v7 13/13] drm/mediatek: Add support for mmsys through a pdev
From: Matthias Brugger The MMSYS subsystem includes clocks and drm components. This patch adds an initailization path through a platform device for the clock part, so that both drivers get probed from the same device tree compatible. Signed-off-by: Matthias Brugger Reviewed-by: Enric Balletbo i Serra --- Changes in v7: - Add Rv-by from Enric Changes in v6: - re-arrange the patch order - generate platform_device for mmsys clock driver inside the DRM driver - fix DTS binding accordingly - switch all mmsys clock driver to platform probing - fix mt8173 platform driver remove function - fix probe defer path in HDMI driver - fix probe defer path in mtk_mdp_comp - fix identation of error messages Changes in v5: - fix missing regmap accessors in drm diver (patch 1) - omit probe deffered warning on all drivers (patch 5) - update drm and clk bindings (patch 6 and 7) - put mmsys clock part in dts child node of mmsys. Only done for HW where no dts backport compatible breakage is expected (either DRM driver not yet implemented or no HW available to the public) (patch 9 to 12) Changes in v4: - use platform device to probe clock driver - add Acked-by CK Hu for the probe deferred patch Changes in v3: - fix kconfig typo (shame on me) - delete __initconst from mm_clocks as converted to a platform driver Changes in v2: - add binding documentation - ddp: use regmap_update_bits - ddp: ignore EPROBE_DEFER on clock probing - mfd: delete mmsys_private - add Reviewed-by and Acked-by tags drivers/gpu/drm/mediatek/mtk_drm_drv.c | 24 drivers/gpu/drm/mediatek/mtk_drm_drv.h | 2 ++ 2 files changed, 26 insertions(+) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c index b68837ea02b3..68605dedf997 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c @@ -125,6 +125,7 @@ static const struct mtk_mmsys_driver_data mt2701_mmsys_driver_data = { .ext_path = mt2701_mtk_ddp_ext, .ext_len = ARRAY_SIZE(mt2701_mtk_ddp_ext), .shadow_register = true, + .clk_drv_name = "clk-mt2701-mm", }; static const struct mtk_mmsys_driver_data mt2712_mmsys_driver_data = { @@ -134,6 +135,7 @@ static const struct mtk_mmsys_driver_data mt2712_mmsys_driver_data = { .ext_len = ARRAY_SIZE(mt2712_mtk_ddp_ext), .third_path = mt2712_mtk_ddp_third, .third_len = ARRAY_SIZE(mt2712_mtk_ddp_third), + .clk_drv_name = "clk-mt2712-mm", }; static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = { @@ -141,6 +143,7 @@ static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = { .main_len = ARRAY_SIZE(mt8173_mtk_ddp_main), .ext_path = mt8173_mtk_ddp_ext, .ext_len = ARRAY_SIZE(mt8173_mtk_ddp_ext), + .clk_drv_name = "clk-mt8173-mm", }; static int mtk_drm_kms_init(struct drm_device *drm) @@ -437,6 +440,24 @@ static int mtk_drm_probe(struct platform_device *pdev) private->data = of_device_get_match_data(dev); + /* +* MMSYS includes apart from components management a block providing +* clocks for the subsystem. We probe this clock driver via a platform +* device. +*/ + if (private->data->clk_drv_name) { + private->clk_dev = platform_device_register_data(dev, + private->data->clk_drv_name, -1, + NULL, 0); + + if (IS_ERR(private->clk_dev)) { + dev_err(dev, "failed to register %s platform device\n", + private->data->clk_drv_name); + + return PTR_ERR(private->clk_dev); + } + } + private->config_regs = syscon_node_to_regmap(dev->of_node); if (IS_ERR(private->config_regs)) return PTR_ERR(private->config_regs); @@ -544,6 +565,9 @@ static int mtk_drm_remove(struct platform_device *pdev) for (i = 0; i < DDP_COMPONENT_ID_MAX; i++) of_node_put(private->comp_node[i]); + if (private->clk_dev) + platform_device_unregister(private->clk_dev); + return 0; } diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h index 03201080688d..15652264c233 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h @@ -29,11 +29,13 @@ struct mtk_mmsys_driver_data { unsigned int third_len; bool shadow_register; + const char *clk_drv_name; }; struct mtk_drm_private { struct drm_device *drm; struct device *dma_dev; + struct platform_device *clk_dev; unsigned int num_pipes; -- 2.24.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 06/13] media: mtk-mdp: Check return value of of_clk_get
From: Matthias Brugger Check the return value of of_clk_get and print an error message if not EPROBE_DEFER. Signed-off-by: Matthias Brugger --- Changes in v7: - fix check of return value of of_clk_get - fix identation Changes in v6: None Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: None drivers/media/platform/mtk-mdp/mtk_mdp_comp.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c index 0c4788af78dd..58abfbdfb82d 100644 --- a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c @@ -110,6 +110,12 @@ int mtk_mdp_comp_init(struct device *dev, struct device_node *node, for (i = 0; i < ARRAY_SIZE(comp->clk); i++) { comp->clk[i] = of_clk_get(node, i); + if (IS_ERR(comp->clk[i])) { + if (PTR_ERR(comp->clk[i]) != -EPROBE_DEFER) + dev_err(dev, "Failed to get clock\n"); + + return PTR_ERR(comp->clk[i]); + } /* Only RDMA needs two clocks */ if (comp->type != MTK_MDP_RDMA) -- 2.24.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 07/13] clk: mediatek: mt2701: switch mmsys to platform device probing
From: Matthias Brugger Switch probing for the MMSYS to support invocation to a plain paltform device. The driver will be probed by the DRM subsystem. Signed-off-by: Matthias Brugger --- Changes in v7: - free clk_data->clks as well - get rid of private data structure Changes in v6: None Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: None drivers/clk/mediatek/clk-mt2701-mm.c | 34 +++- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/drivers/clk/mediatek/clk-mt2701-mm.c b/drivers/clk/mediatek/clk-mt2701-mm.c index 054b597d4a73..eab7dd4735ad 100644 --- a/drivers/clk/mediatek/clk-mt2701-mm.c +++ b/drivers/clk/mediatek/clk-mt2701-mm.c @@ -4,8 +4,10 @@ * Author: Shunli Wang */ +#include #include #include +#include #include "clk-mtk.h" #include "clk-gate.h" @@ -79,21 +81,21 @@ static const struct mtk_gate mm_clks[] = { GATE_DISP1(CLK_MM_TVE_FMM, "mm_tve_fmm", "mm_sel", 14), }; -static const struct of_device_id of_match_clk_mt2701_mm[] = { - { .compatible = "mediatek,mt2701-mmsys", }, - {} -}; - static int clk_mt2701_mm_probe(struct platform_device *pdev) { - struct clk_onecell_data *clk_data; int r; - struct device_node *node = pdev->dev.of_node; + struct device_node *node = pdev->dev.parent->of_node; + struct clk_onecell_data *clk_data; + + clk_data = devm_kzalloc(>dev, sizeof(*clk_data), GFP_KERNEL); + if (!clk_data) + return -ENOMEM; clk_data = mtk_alloc_clk_data(CLK_MM_NR); - mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks), - clk_data); + platform_set_drvdata(pdev, clk_data); + + mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks), clk_data); r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data); if (r) @@ -104,12 +106,22 @@ static int clk_mt2701_mm_probe(struct platform_device *pdev) return r; } +static int clk_mt2701_mm_remove(struct platform_device *pdev) +{ + struct clk_onecell_data *clk_data = platform_get_drvdata(pdev); + + kfree(clk_data->clks); + kfree(clk_data); + + return 0; +} + static struct platform_driver clk_mt2701_mm_drv = { .probe = clk_mt2701_mm_probe, + .remove = clk_mt2701_mm_remove, .driver = { .name = "clk-mt2701-mm", - .of_match_table = of_match_clk_mt2701_mm, }, }; -builtin_platform_driver(clk_mt2701_mm_drv); +module_platform_driver(clk_mt2701_mm_drv); -- 2.24.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 08/13] clk: mediatek: mt2712e: switch to platform device probing
From: Matthias Brugger Switch probing for the MMSYS to support invocation to a plain paltform device. The driver will be probed by the DRM subsystem. Signed-off-by: Matthias Brugger --- Changes in v7: - free clk_data->clks as well - get rid of private data structure Changes in v6: None Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: None drivers/clk/mediatek/clk-mt2712-mm.c | 32 ++-- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/drivers/clk/mediatek/clk-mt2712-mm.c b/drivers/clk/mediatek/clk-mt2712-mm.c index 1c5948be35f3..2ab86262dc17 100644 --- a/drivers/clk/mediatek/clk-mt2712-mm.c +++ b/drivers/clk/mediatek/clk-mt2712-mm.c @@ -4,8 +4,10 @@ * Author: Weiyi Lu */ +#include #include #include +#include #include "clk-mtk.h" #include "clk-gate.h" @@ -128,14 +130,18 @@ static const struct mtk_gate mm_clks[] = { static int clk_mt2712_mm_probe(struct platform_device *pdev) { - struct clk_onecell_data *clk_data; int r; - struct device_node *node = pdev->dev.of_node; + struct device_node *node = pdev->dev.parent->of_node; + struct clk_onecell_data *clk_data; + + clk_data = devm_kzalloc(>dev, sizeof(*clk_data), GFP_KERNEL); + if (!clk_data) + return -ENOMEM; clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK); + platform_set_drvdata(pdev, clk_data); - mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks), - clk_data); + mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks), clk_data); r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data); @@ -146,17 +152,21 @@ static int clk_mt2712_mm_probe(struct platform_device *pdev) return r; } -static const struct of_device_id of_match_clk_mt2712_mm[] = { - { .compatible = "mediatek,mt2712-mmsys", }, - {} -}; +static int clk_mt2712_mm_remove(struct platform_device *pdev) +{ + struct clk_onecell_data *clk_data = platform_get_drvdata(pdev); + + kfree(clk_data->clks); + kfree(clk_data); + + return 0; +} static struct platform_driver clk_mt2712_mm_drv = { .probe = clk_mt2712_mm_probe, + .remove = clk_mt2712_mm_remove, .driver = { .name = "clk-mt2712-mm", - .of_match_table = of_match_clk_mt2712_mm, }, }; - -builtin_platform_driver(clk_mt2712_mm_drv); +module_platform_driver(clk_mt2712_mm_drv); -- 2.24.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 11/13] clk: mediatek: mt8183: switch mmsys to platform device probing
From: Matthias Brugger Switch probing for the MMSYS to support invocation to a plain paltform device. The driver will be probed by the DRM subsystem. Signed-off-by: Matthias Brugger --- Changes in v7: - free clk_data->clks as well - get rid of private data structure Changes in v6: None Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: None drivers/clk/mediatek/clk-mt8183-mm.c | 30 ++-- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/drivers/clk/mediatek/clk-mt8183-mm.c b/drivers/clk/mediatek/clk-mt8183-mm.c index 720c696b506d..7576cd231be3 100644 --- a/drivers/clk/mediatek/clk-mt8183-mm.c +++ b/drivers/clk/mediatek/clk-mt8183-mm.c @@ -3,8 +3,10 @@ // Copyright (c) 2018 MediaTek Inc. // Author: Weiyi Lu +#include #include #include +#include #include "clk-mtk.h" #include "clk-gate.h" @@ -85,27 +87,35 @@ static const struct mtk_gate mm_clks[] = { static int clk_mt8183_mm_probe(struct platform_device *pdev) { struct clk_onecell_data *clk_data; - struct device_node *node = pdev->dev.of_node; + struct device_node *node = pdev->dev.parent->of_node; + + clk_data = devm_kzalloc(>dev, sizeof(*clk_data), GFP_KERNEL); + if (!clk_data) + return -ENOMEM; clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK); + platform_set_drvdata(pdev, clk_data); - mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks), - clk_data); + mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks), clk_data); return of_clk_add_provider(node, of_clk_src_onecell_get, clk_data); } -static const struct of_device_id of_match_clk_mt8183_mm[] = { - { .compatible = "mediatek,mt8183-mmsys", }, - {} -}; +static int clk_mt8183_mm_remove(struct platform_device *pdev) +{ + struct clk_onecell_data *clk_data = platform_get_drvdata(pdev); + + kfree(clk_data->clks); + kfree(clk_data); + + return 0; +} static struct platform_driver clk_mt8183_mm_drv = { .probe = clk_mt8183_mm_probe, + .remove = clk_mt8183_mm_remove, .driver = { .name = "clk-mt8183-mm", - .of_match_table = of_match_clk_mt8183_mm, }, }; - -builtin_platform_driver(clk_mt8183_mm_drv); +module_platform_driver(clk_mt8183_mm_drv); -- 2.24.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 05/13] drm: mediatek: Omit warning on probe defers
From: Matthias Brugger It can happen that the mmsys clock drivers aren't probed before the platform driver gets invoked. The platform driver used to print a warning that the driver failed to get the clocks. Omit this error on the defered probe path. Signed-off-by: Matthias Brugger Reviewed-by: CK Hu --- Changes in v7: - add Rv-by from CK Changes in v6: None Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: None drivers/gpu/drm/mediatek/mtk_disp_color.c | 5 - drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 5 - drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 5 - drivers/gpu/drm/mediatek/mtk_dpi.c| 12 +--- drivers/gpu/drm/mediatek/mtk_drm_ddp.c| 3 ++- drivers/gpu/drm/mediatek/mtk_dsi.c| 8 ++-- drivers/gpu/drm/mediatek/mtk_hdmi.c | 4 +++- 7 files changed, 32 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_disp_color.c b/drivers/gpu/drm/mediatek/mtk_disp_color.c index 6fb0d6983a4a..3ae9c810845b 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_color.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_color.c @@ -119,7 +119,10 @@ static int mtk_disp_color_probe(struct platform_device *pdev) ret = mtk_ddp_comp_init(dev, dev->of_node, >ddp_comp, comp_id, _disp_color_funcs); if (ret) { - dev_err(dev, "Failed to initialize component: %d\n", ret); + if (ret != -EPROBE_DEFER) + dev_err(dev, "Failed to initialize component: %d\n", + ret); + return ret; } diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c index 891d80c73e04..28651bc579bc 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c @@ -386,7 +386,10 @@ static int mtk_disp_ovl_probe(struct platform_device *pdev) ret = mtk_ddp_comp_init(dev, dev->of_node, >ddp_comp, comp_id, _disp_ovl_funcs); if (ret) { - dev_err(dev, "Failed to initialize component: %d\n", ret); + if (ret != -EPROBE_DEFER) + dev_err(dev, "Failed to initialize component: %d\n", + ret); + return ret; } diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c index 0cb848d64206..e04319fedf46 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c @@ -294,7 +294,10 @@ static int mtk_disp_rdma_probe(struct platform_device *pdev) ret = mtk_ddp_comp_init(dev, dev->of_node, >ddp_comp, comp_id, _disp_rdma_funcs); if (ret) { - dev_err(dev, "Failed to initialize component: %d\n", ret); + if (ret != -EPROBE_DEFER) + dev_err(dev, "Failed to initialize component: %d\n", + ret); + return ret; } diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c index 01fa8b8d763d..1b219edef541 100644 --- a/drivers/gpu/drm/mediatek/mtk_dpi.c +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c @@ -701,21 +701,27 @@ static int mtk_dpi_probe(struct platform_device *pdev) dpi->engine_clk = devm_clk_get(dev, "engine"); if (IS_ERR(dpi->engine_clk)) { ret = PTR_ERR(dpi->engine_clk); - dev_err(dev, "Failed to get engine clock: %d\n", ret); + if (ret != -EPROBE_DEFER) + dev_err(dev, "Failed to get engine clock: %d\n", ret); + return ret; } dpi->pixel_clk = devm_clk_get(dev, "pixel"); if (IS_ERR(dpi->pixel_clk)) { ret = PTR_ERR(dpi->pixel_clk); - dev_err(dev, "Failed to get pixel clock: %d\n", ret); + if (ret != -EPROBE_DEFER) + dev_err(dev, "Failed to get pixel clock: %d\n", ret); + return ret; } dpi->tvd_clk = devm_clk_get(dev, "pll"); if (IS_ERR(dpi->tvd_clk)) { ret = PTR_ERR(dpi->tvd_clk); - dev_err(dev, "Failed to get tvdpll clock: %d\n", ret); + if (ret != -EPROBE_DEFER) + dev_err(dev, "Failed to get tvdpll clock: %d\n", ret); + return ret; } diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c index 302753744cc6..39700b9428b9 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c @@ -620,7 +620,8 @@ static int mtk_ddp_probe(struct platform_device *pdev) if (!ddp->data->no_clk) { ddp->clk = devm_clk_get(dev, NULL); if (IS_ERR(ddp->clk)) { - dev_err(dev, "Failed to get clock\n"); + if
[PATCH v7 12/13] clk: mediatek: mt8173: switch mmsys to platform device probing
From: Matthias Brugger Switch probing for the MMSYS to support invocation to a plain paltform device. The driver will be probed by the DRM subsystem. Signed-off-by: Matthias Brugger --- Changes in v7: - add blank line after declaration - free clk_data->clks as well - get rid of private data structure Changes in v6: None Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: None drivers/clk/mediatek/clk-mt8173.c | 45 ++- 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c index 537a7f49b0f7..0608d9fffef7 100644 --- a/drivers/clk/mediatek/clk-mt8173.c +++ b/drivers/clk/mediatek/clk-mt8173.c @@ -5,8 +5,11 @@ */ #include +#include #include #include +#include +#include #include "clk-mtk.h" #include "clk-gate.h" @@ -783,7 +786,7 @@ static const struct mtk_gate_regs mm1_cg_regs __initconst = { .ops = _clk_gate_ops_setclr,\ } -static const struct mtk_gate mm_clks[] __initconst = { +static const struct mtk_gate mm_clks[] = { /* MM0 */ GATE_MM0(CLK_MM_SMI_COMMON, "mm_smi_common", "mm_sel", 0), GATE_MM0(CLK_MM_SMI_LARB0, "mm_smi_larb0", "mm_sel", 1), @@ -1144,22 +1147,52 @@ static void __init mtk_imgsys_init(struct device_node *node) } CLK_OF_DECLARE(mtk_imgsys, "mediatek,mt8173-imgsys", mtk_imgsys_init); -static void __init mtk_mmsys_init(struct device_node *node) +static int mtk_mmsys_probe(struct platform_device *pdev) { - struct clk_onecell_data *clk_data; int r; + struct device_node *node; + struct clk_onecell_data *clk_data; + + node = pdev->dev.parent->of_node; + + clk_data = devm_kzalloc(>dev, sizeof(*clk_data), GFP_KERNEL); + if (!clk_data) + return -ENOMEM; clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK); - mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks), - clk_data); + platform_set_drvdata(pdev, clk_data); + + mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks), clk_data); r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data); if (r) pr_err("%s(): could not register clock provider: %d\n", __func__, r); + + return r; +} + +static int mtk_mmsys_remove(struct platform_device *pdev) +{ + struct clk_onecell_data *clk_data; + + clk_data = platform_get_drvdata(pdev); + + kfree(clk_data->clks); + kfree(clk_data); + + return 0; } -CLK_OF_DECLARE(mtk_mmsys, "mediatek,mt8173-mmsys", mtk_mmsys_init); + +static struct platform_driver clk_mt8173_mm_drv = { + .probe = mtk_mmsys_probe, + .remove = mtk_mmsys_remove, + .driver = { + .name = "clk-mt8173-mm", + }, +}; +module_platform_driver(clk_mt8173_mm_drv); static void __init mtk_vdecsys_init(struct device_node *node) { -- 2.24.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 03/13] dt-bindings: mediatek: Add compatible for mt7623
From: Matthias Brugger MediaTek mt7623 uses the mt2701 bindings as fallback. Document this in the binding description. Signed-off-by: Matthias Brugger Acked-by: Rob Herring --- Changes in v7: - fix typo in commit message - add Rob's ack Changes in v6: None Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: None .../devicetree/bindings/display/mediatek/mediatek,disp.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt index 8e453026ef78..456e502f538c 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt @@ -46,6 +46,8 @@ Required properties (all function blocks): "mediatek,-disp-od" - overdrive "mediatek,-mmsys", "syscon" - provide clocks and components management the supported chips are mt2701, mt2712 and mt8173. + For mt7623, compatible must be: +"mediatek,mt7623-" , "mediatek,mt2701-" - reg: Physical base address and length of the function block register space - interrupts: The interrupt signal from the function block (required, except for -- 2.24.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 09/13] clk: mediatek: mt6779: switch mmsys to platform device probing
From: Matthias Brugger Switch probing for the MMSYS to support invocation to a plain paltform device. The driver will be probed by the DRM subsystem. Signed-off-by: Matthias Brugger --- Changes in v7: - free clk_data->clks as well - get rid of private data structure Changes in v6: None Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: None drivers/clk/mediatek/clk-mt6779-mm.c | 32 ++-- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/drivers/clk/mediatek/clk-mt6779-mm.c b/drivers/clk/mediatek/clk-mt6779-mm.c index fb5fbb8e3e41..70a1f3b413ba 100644 --- a/drivers/clk/mediatek/clk-mt6779-mm.c +++ b/drivers/clk/mediatek/clk-mt6779-mm.c @@ -4,9 +4,11 @@ * Author: Wendell Lin */ +#include #include #include #include +#include #include "clk-mtk.h" #include "clk-gate.h" @@ -84,30 +86,38 @@ static const struct mtk_gate mm_clks[] = { GATE_MM1(CLK_MM_DISP_OVL_FBDC, "mm_disp_ovl_fbdc", "mm_sel", 16), }; -static const struct of_device_id of_match_clk_mt6779_mm[] = { - { .compatible = "mediatek,mt6779-mmsys", }, - {} -}; - static int clk_mt6779_mm_probe(struct platform_device *pdev) { struct clk_onecell_data *clk_data; - struct device_node *node = pdev->dev.of_node; + struct device_node *node = pdev->dev.parent->of_node; + + clk_data = devm_kzalloc(>dev, sizeof(*clk_data), GFP_KERNEL); + if (!clk_data) + return -ENOMEM; clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK); + platform_set_drvdata(pdev, clk_data); - mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks), - clk_data); + mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks), clk_data); return of_clk_add_provider(node, of_clk_src_onecell_get, clk_data); } +static int clk_mt6779_mm_remove(struct platform_device *pdev) +{ + struct clk_onecell_data *clk_data = platform_get_drvdata(pdev); + + kfree(clk_data->clks); + kfree(clk_data); + + return 0; +} + static struct platform_driver clk_mt6779_mm_drv = { .probe = clk_mt6779_mm_probe, + .remove = clk_mt6779_mm_remove, .driver = { .name = "clk-mt6779-mm", - .of_match_table = of_match_clk_mt6779_mm, }, }; - -builtin_platform_driver(clk_mt6779_mm_drv); +module_platform_driver(clk_mt6779_mm_drv); -- 2.24.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 10/13] clk: mediatek: mt6797: switch to platform device probing
From: Matthias Brugger Switch probing for the MMSYS to support invocation to a plain paltform device. The driver will be probed by the DRM subsystem. Signed-off-by: Matthias Brugger --- Changes in v7: - free clk_data->clks as well - get rid of private data structure Changes in v6: None Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: None drivers/clk/mediatek/clk-mt6797-mm.c | 34 +++- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/drivers/clk/mediatek/clk-mt6797-mm.c b/drivers/clk/mediatek/clk-mt6797-mm.c index 8f05653b387d..6a3c54b6b793 100644 --- a/drivers/clk/mediatek/clk-mt6797-mm.c +++ b/drivers/clk/mediatek/clk-mt6797-mm.c @@ -4,8 +4,10 @@ * Author: Kevin Chen */ +#include #include #include +#include #include #include "clk-mtk.h" @@ -92,23 +94,24 @@ static const struct mtk_gate mm_clks[] = { "clk26m", 3), }; -static const struct of_device_id of_match_clk_mt6797_mm[] = { - { .compatible = "mediatek,mt6797-mmsys", }, - {} -}; - static int clk_mt6797_mm_probe(struct platform_device *pdev) { struct clk_onecell_data *clk_data; int r; - struct device_node *node = pdev->dev.of_node; + struct device *parent = pdev->dev.parent; + + clk_data = devm_kzalloc(>dev, sizeof(*clk_data), GFP_KERNEL); + if (!clk_data) + return -ENOMEM; clk_data = mtk_alloc_clk_data(CLK_MM_NR); + platform_set_drvdata(pdev, clk_data); - mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks), + mtk_clk_register_gates(parent->of_node, mm_clks, ARRAY_SIZE(mm_clks), clk_data); - r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data); + r = of_clk_add_provider(parent->of_node, of_clk_src_onecell_get, + clk_data); if (r) dev_err(>dev, "could not register clock provider: %s: %d\n", @@ -117,12 +120,21 @@ static int clk_mt6797_mm_probe(struct platform_device *pdev) return r; } +static int clk_mt6797_mm_remove(struct platform_device *pdev) +{ + struct clk_onecell_data *clk_data = platform_get_drvdata(pdev); + + kfree(clk_data->clks); + kfree(clk_data); + + return 0; +} + static struct platform_driver clk_mt6797_mm_drv = { .probe = clk_mt6797_mm_probe, + .remove = clk_mt6797_mm_remove, .driver = { .name = "clk-mt6797-mm", - .of_match_table = of_match_clk_mt6797_mm, }, }; - -builtin_platform_driver(clk_mt6797_mm_drv); +module_platform_driver(clk_mt6797_mm_drv); -- 2.24.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 04/13] drm/mediatek: Use regmap for register access
From: Matthias Brugger The mmsys memory space is shared between the drm and the clk driver. Use regmap to access it. Signed-off-by: Matthias Brugger Reviewed-by: Philipp Zabel Reviewed-by: CK Hu --- Changes in v7: - add R-by from CK Changes in v6: None Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: None drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 4 +- drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 50 +++-- drivers/gpu/drm/mediatek/mtk_drm_ddp.h | 4 +- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 13 ++- drivers/gpu/drm/mediatek/mtk_drm_drv.h | 2 +- 5 files changed, 30 insertions(+), 43 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index 0dfcd1787e65..ea003a225604 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -28,7 +28,7 @@ * @enabled: records whether crtc_enable succeeded * @planes: array of 4 drm_plane structures, one for each overlay plane * @pending_planes: whether any plane has pending changes to be applied - * @config_regs: memory mapped mmsys configuration register space + * @config_regs: regmap mapped mmsys configuration register space * @mutex: handle to one of the ten disp_mutex streams * @ddp_comp_nr: number of components in ddp_comp * @ddp_comp: array of pointers the mtk_ddp_comp structures used by this crtc @@ -50,7 +50,7 @@ struct mtk_drm_crtc { u32 cmdq_event; #endif - void __iomem*config_regs; + struct regmap *config_regs; struct mtk_disp_mutex *mutex; unsigned intddp_comp_nr; struct mtk_ddp_comp **ddp_comp; diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c index 13035c906035..302753744cc6 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c @@ -383,61 +383,53 @@ static unsigned int mtk_ddp_sel_in(enum mtk_ddp_comp_id cur, return value; } -static void mtk_ddp_sout_sel(void __iomem *config_regs, +static void mtk_ddp_sout_sel(struct regmap *config_regs, enum mtk_ddp_comp_id cur, enum mtk_ddp_comp_id next) { if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) { - writel_relaxed(BLS_TO_DSI_RDMA1_TO_DPI1, - config_regs + DISP_REG_CONFIG_OUT_SEL); + regmap_write(config_regs, DISP_REG_CONFIG_OUT_SEL, + BLS_TO_DSI_RDMA1_TO_DPI1); } else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DPI0) { - writel_relaxed(BLS_TO_DPI_RDMA1_TO_DSI, - config_regs + DISP_REG_CONFIG_OUT_SEL); - writel_relaxed(DSI_SEL_IN_RDMA, - config_regs + DISP_REG_CONFIG_DSI_SEL); - writel_relaxed(DPI_SEL_IN_BLS, - config_regs + DISP_REG_CONFIG_DPI_SEL); + regmap_write(config_regs, DISP_REG_CONFIG_OUT_SEL, + BLS_TO_DPI_RDMA1_TO_DSI); + regmap_write(config_regs, DISP_REG_CONFIG_DSI_SEL, + DSI_SEL_IN_RDMA); + regmap_write(config_regs, DISP_REG_CONFIG_DPI_SEL, + DPI_SEL_IN_BLS); } } -void mtk_ddp_add_comp_to_path(void __iomem *config_regs, +void mtk_ddp_add_comp_to_path(struct regmap *config_regs, enum mtk_ddp_comp_id cur, enum mtk_ddp_comp_id next) { - unsigned int addr, value, reg; + unsigned int addr, value; value = mtk_ddp_mout_en(cur, next, ); - if (value) { - reg = readl_relaxed(config_regs + addr) | value; - writel_relaxed(reg, config_regs + addr); - } + if (value) + regmap_update_bits(config_regs, addr, value, value); mtk_ddp_sout_sel(config_regs, cur, next); value = mtk_ddp_sel_in(cur, next, ); - if (value) { - reg = readl_relaxed(config_regs + addr) | value; - writel_relaxed(reg, config_regs + addr); - } + if (value) + regmap_update_bits(config_regs, addr, value, value); } -void mtk_ddp_remove_comp_from_path(void __iomem *config_regs, +void mtk_ddp_remove_comp_from_path(struct regmap *config_regs, enum mtk_ddp_comp_id cur, enum mtk_ddp_comp_id next) { - unsigned int addr, value, reg; + unsigned int addr, value; value = mtk_ddp_mout_en(cur, next, ); - if (value) { - reg = readl_relaxed(config_regs + addr) & ~value; - writel_relaxed(reg, config_regs + addr); - } + if (value) +
[PATCH v7 00/13] arm/arm64: mediatek: Fix mmsys device probing
From: Matthias Brugger This is version seven of the series. The biggest change is, that I added a first patch that actually moves the mmsys binding from arm/mediatek to display/mediatek, as in effect the mmsys is part of the display subsystem. Since version five, the clock probing is implemented through a platform driver. The corresponding platform device get's created in the DRM driver. I converted all the clock drivers to platform drivers and tested the approach on the Acer Chromebook R13 (mt8173 based). Apart from that I reordered the patches so that the DT bindings update are the first patches. MMSYS in Mediatek SoCs has some registers to control clock gates (which is used in the clk driver) and some registers to set the routing and enable the differnet blocks of the display subsystem. Up to now both drivers, clock and drm are probed with the same device tree compatible. But only the first driver get probed, which in effect breaks graphics on mt8173 and mt2701. This patch uses a platform device registration in the DRM driver, which will trigger the probe of the corresponding clock driver. It was tested on the Acer R13 Chromebook. Changes in v7: - move the binding description - add hint to the mmsys binding document - make mmsys description generic - fix typo in commit message - fix check of return value of of_clk_get - free clk_data->clks as well - get rid of private data structure Changes in v6: - re-arrange the patch order - generate platform_device for mmsys clock driver inside the DRM driver - fix DTS binding accordingly - switch all mmsys clock driver to platform probing - fix mt8173 platform driver remove function - fix probe defer path in HDMI driver - fix probe defer path in mtk_mdp_comp - fix identation of error messages Changes in v5: - fix missing regmap accessors in drm diver (patch 1) - omit probe deffered warning on all drivers (patch 5) - update drm and clk bindings (patch 6 and 7) - put mmsys clock part in dts child node of mmsys. Only done for HW where no dts backport compatible breakage is expected (either DRM driver not yet implemented or no HW available to the public) (patch 9 to 12) Changes in v4: - use platform device to probe clock driver - add Acked-by CK Hu for the probe deferred patch Changes in v3: - fix kconfig typo (shame on me) - delete __initconst from mm_clocks as converted to a platform driver Changes in v2: - add binding documentation - ddp: use regmap_update_bits - ddp: ignore EPROBE_DEFER on clock probing - mfd: delete mmsys_private - add Reviewed-by and Acked-by tags Matthias Brugger (13): dt-bindings: arm: move mmsys description to display dt-bindings: display: mediatek: Add mmsys binding description dt-bindings: mediatek: Add compatible for mt7623 drm/mediatek: Use regmap for register access drm: mediatek: Omit warning on probe defers media: mtk-mdp: Check return value of of_clk_get clk: mediatek: mt2701: switch mmsys to platform device probing clk: mediatek: mt2712e: switch to platform device probing clk: mediatek: mt6779: switch mmsys to platform device probing clk: mediatek: mt6797: switch to platform device probing clk: mediatek: mt8183: switch mmsys to platform device probing clk: mediatek: mt8173: switch mmsys to platform device probing drm/mediatek: Add support for mmsys through a pdev .../display/mediatek/mediatek,disp.txt| 5 ++ .../mediatek/mediatek,mmsys.txt | 9 +--- drivers/clk/mediatek/clk-mt2701-mm.c | 34 drivers/clk/mediatek/clk-mt2712-mm.c | 32 +++ drivers/clk/mediatek/clk-mt6779-mm.c | 32 +++ drivers/clk/mediatek/clk-mt6797-mm.c | 34 drivers/clk/mediatek/clk-mt8173.c | 45 +--- drivers/clk/mediatek/clk-mt8183-mm.c | 30 +++ drivers/gpu/drm/mediatek/mtk_disp_color.c | 5 +- drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 5 +- drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 5 +- drivers/gpu/drm/mediatek/mtk_dpi.c| 12 +++-- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 4 +- drivers/gpu/drm/mediatek/mtk_drm_ddp.c| 53 --- drivers/gpu/drm/mediatek/mtk_drm_ddp.h| 4 +- drivers/gpu/drm/mediatek/mtk_drm_drv.c| 35 +--- drivers/gpu/drm/mediatek/mtk_drm_drv.h| 4 +- drivers/gpu/drm/mediatek/mtk_dsi.c| 8 ++- drivers/gpu/drm/mediatek/mtk_hdmi.c | 4 +- drivers/media/platform/mtk-mdp/mtk_mdp_comp.c | 6 +++ 20 files changed, 246 insertions(+), 120 deletions(-) rename Documentation/devicetree/bindings/{arm => display}/mediatek/mediatek,mmsys.txt (61%) -- 2.24.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 01/13] dt-bindings: arm: move mmsys description to display
From: Matthias Brugger The mmsys block provides registers and clocks for the display subsystem. The binding description should therefore live together with the rest of the display descriptions. Move it to display/mediatek. Signed-off-by: Matthias Brugger --- Changes in v7: - move the binding description Changes in v6: None Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: None .../bindings/{arm => display}/mediatek/mediatek,mmsys.txt | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename Documentation/devicetree/bindings/{arm => display}/mediatek/mediatek,mmsys.txt (100%) diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt b/Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt similarity index 100% rename from Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt rename to Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt -- 2.24.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 02/13] dt-bindings: display: mediatek: Add mmsys binding description
From: Matthias Brugger The MediaTek DRM has a block called mmsys, which sets the routing and enables the different blocks. This patch adds one line for the mmsys bindings description and changes the mmsys description to use the generic form of referring to a specific Soc. Signed-off-by: Matthias Brugger --- Changes in v7: - add hint to the mmsys binding document - make mmsys description generic - fix typo in commit message Changes in v6: None Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: None .../bindings/display/mediatek/mediatek,disp.txt | 3 +++ .../bindings/display/mediatek/mediatek,mmsys.txt | 9 + 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt index b91e709db7a4..8e453026ef78 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt @@ -24,6 +24,7 @@ connected to. For a description of the display interface sink function blocks, see Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.txt and Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt. +Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt. Required properties (all function blocks): - compatible: "mediatek,-disp-", one of @@ -43,7 +44,9 @@ Required properties (all function blocks): "mediatek,-dpi" - DPI controller, see mediatek,dpi.txt "mediatek,-disp-mutex"- display mutex "mediatek,-disp-od" - overdrive + "mediatek,-mmsys", "syscon" - provide clocks and components management the supported chips are mt2701, mt2712 and mt8173. + - reg: Physical base address and length of the function block register space - interrupts: The interrupt signal from the function block (required, except for merge and split function blocks). diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt b/Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt index 301eefbe1618..7bbadee820e3 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt @@ -5,14 +5,7 @@ The Mediatek mmsys controller provides various clocks to the system. Required Properties: -- compatible: Should be one of: - - "mediatek,mt2701-mmsys", "syscon" - - "mediatek,mt2712-mmsys", "syscon" - - "mediatek,mt6779-mmsys", "syscon" - - "mediatek,mt6797-mmsys", "syscon" - - "mediatek,mt7623-mmsys", "mediatek,mt2701-mmsys", "syscon" - - "mediatek,mt8173-mmsys", "syscon" - - "mediatek,mt8183-mmsys", "syscon" +- compatible: "mediatek,-mmsys" - #clock-cells: Must be 1 The mmsys controller uses the common clk binding from -- 2.24.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm/dpu: fix BGR565 vs RGB565 confusion
From: Rob Clark The component order between the two was swapped, resulting in incorrect color when games with 565 visual hit the overlay path instead of GPU composition. Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support") Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c index 24ab6249083a..6f420cc73dbd 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c @@ -255,13 +255,13 @@ static const struct dpu_format dpu_format_map[] = { INTERLEAVED_RGB_FMT(RGB565, 0, COLOR_5BIT, COLOR_6BIT, COLOR_5BIT, - C2_R_Cr, C0_G_Y, C1_B_Cb, 0, 3, + C1_B_Cb, C0_G_Y, C2_R_Cr, 0, 3, false, 2, 0, DPU_FETCH_LINEAR, 1), INTERLEAVED_RGB_FMT(BGR565, 0, COLOR_5BIT, COLOR_6BIT, COLOR_5BIT, - C1_B_Cb, C0_G_Y, C2_R_Cr, 0, 3, + C2_R_Cr, C0_G_Y, C1_B_Cb, 0, 3, false, 2, 0, DPU_FETCH_LINEAR, 1), -- 2.24.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4] drm/tidss: dispc: Rewrite naive plane positioning code
The old implementation of placing planes on the CRTC while configuring the planes was naive and relied on the order in which the planes were configured, enabled, and disabled. The situation where a plane's zpos was changed on the fly was completely broken. The usual symptoms of this problem was scrambled display and a flood of sync lost errors, when a plane was active in two layers at the same time, or a missing plane, in case when a layer was accidentally disabled. The rewrite takes a more straight forward approach when HW is concerned. The plane positioning registers are in the CRTC (or actually OVR) register space and it is more natural to configure them in a one go when configuring the CRTC. To do this we need make sure we have all the planes on the updated CRTCs in the new atomic state. The untouched planes on CRTCs that need plane position update are added to the atomic state in tidss_atomic_check(). Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tidss/tidss_crtc.c | 55 + drivers/gpu/drm/tidss/tidss_crtc.h | 2 ++ drivers/gpu/drm/tidss/tidss_dispc.c | 55 +++-- drivers/gpu/drm/tidss/tidss_dispc.h | 5 +++ drivers/gpu/drm/tidss/tidss_kms.c | 49 - 5 files changed, 130 insertions(+), 36 deletions(-) diff --git a/drivers/gpu/drm/tidss/tidss_crtc.c b/drivers/gpu/drm/tidss/tidss_crtc.c index 032c31ee2820..631ec61b086a 100644 --- a/drivers/gpu/drm/tidss/tidss_crtc.c +++ b/drivers/gpu/drm/tidss/tidss_crtc.c @@ -17,6 +17,7 @@ #include "tidss_dispc.h" #include "tidss_drv.h" #include "tidss_irq.h" +#include "tidss_plane.h" /* Page flip and frame done IRQs */ @@ -111,6 +112,54 @@ static int tidss_crtc_atomic_check(struct drm_crtc *crtc, return dispc_vp_bus_check(dispc, hw_videoport, state); } +/* + * This needs all affected planes to be present in the atomic + * state. The untouched planes are added to the state in + * tidss_atomic_check(). + */ +static void tidss_crtc_position_planes(struct tidss_device *tidss, + struct drm_crtc *crtc, + struct drm_crtc_state *old_state, + bool newmodeset) +{ + struct drm_atomic_state *ostate = old_state->state; + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); + struct drm_crtc_state *cstate = crtc->state; + int zpos; + + if (!newmodeset && !cstate->zpos_changed && + !to_tidss_crtc_state(cstate)->plane_pos_changed) + return; + + for (zpos = 0; zpos < tidss->feat->num_planes; zpos++) { + struct drm_plane_state *pstate; + struct drm_plane *plane; + bool zpos_taken = false; + int i; + + for_each_new_plane_in_state(ostate, plane, pstate, i) { + if (pstate->crtc != crtc || !pstate->visible) + continue; + + if (pstate->normalized_zpos == zpos) { + zpos_taken = true; + break; + } + } + + if (zpos_taken) { + struct tidss_plane *tplane = to_tidss_plane(plane); + + dispc_ovr_set_plane(tidss->dispc, tplane->hw_plane_id, + tcrtc->hw_videoport, + pstate->crtc_x, pstate->crtc_y, + zpos); + } + dispc_ovr_enable_layer(tidss->dispc, tcrtc->hw_videoport, zpos, + zpos_taken); + } +} + static void tidss_crtc_atomic_flush(struct drm_crtc *crtc, struct drm_crtc_state *old_crtc_state) { @@ -146,6 +195,9 @@ static void tidss_crtc_atomic_flush(struct drm_crtc *crtc, /* Write vp properties to HW if needed. */ dispc_vp_setup(tidss->dispc, tcrtc->hw_videoport, crtc->state, false); + /* Update plane positions if needed. */ + tidss_crtc_position_planes(tidss, crtc, old_crtc_state, false); + WARN_ON(drm_crtc_vblank_get(crtc) != 0); spin_lock_irqsave(>event_lock, flags); @@ -183,6 +235,7 @@ static void tidss_crtc_atomic_enable(struct drm_crtc *crtc, return; dispc_vp_setup(tidss->dispc, tcrtc->hw_videoport, crtc->state, true); + tidss_crtc_position_planes(tidss, crtc, old_state, true); /* Turn vertical blanking interrupt reporting on. */ drm_crtc_vblank_on(crtc); @@ -318,6 +371,8 @@ static struct drm_crtc_state *tidss_crtc_duplicate_state(struct drm_crtc *crtc) __drm_atomic_helper_crtc_duplicate_state(crtc, >base); + state->plane_pos_changed = false; + state->bus_format = current_state->bus_format; state->bus_flags = current_state->bus_flags; diff --git
Re: [resend PATCH v6 12/12] drm/mediatek: Add support for mmsys through a pdev
On 09/12/2019 06:34, CK Hu wrote: > Hi, Matthias: > > On Sat, 2019-12-07 at 23:47 +0100, matthias@kernel.org wrote: >> From: Matthias Brugger >> >> The MMSYS subsystem includes clocks and drm components. >> This patch adds an initailization path through a platform device >> for the clock part, so that both drivers get probed from the same >> device tree compatible. > > You've switched mt6779 and mt6797 clock driver to platform device > probing, but you does not probe then in drm driver, so your design is to > let mmsys clock not work if drm driver is not enabled? It's fine for me > because it seems that drm driver is the only user of mmsys clock now. > Exactly. The mmsys clocks are only needed for the drm driver. Therefor we only probe them when probing the drm driver. Regards, Matthias > Regards, > CK > >> >> Signed-off-by: Matthias Brugger >> --- >> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 24 >> drivers/gpu/drm/mediatek/mtk_drm_drv.h | 2 ++ >> 2 files changed, 26 insertions(+) >> >> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c >> b/drivers/gpu/drm/mediatek/mtk_drm_drv.c >> index 210455e9f46c..5ada74d8d0c9 100644 >> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c >> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c >> @@ -186,6 +186,7 @@ static const struct mtk_mmsys_driver_data >> mt2701_mmsys_driver_data = { >> .ext_path = mt2701_mtk_ddp_ext, >> .ext_len = ARRAY_SIZE(mt2701_mtk_ddp_ext), >> .shadow_register = true, >> +.clk_drv_name = "clk-mt2701-mm", >> }; >> >> static const struct mtk_mmsys_driver_data mt2712_mmsys_driver_data = { >> @@ -195,6 +196,7 @@ static const struct mtk_mmsys_driver_data >> mt2712_mmsys_driver_data = { >> .ext_len = ARRAY_SIZE(mt2712_mtk_ddp_ext), >> .third_path = mt2712_mtk_ddp_third, >> .third_len = ARRAY_SIZE(mt2712_mtk_ddp_third), >> +.clk_drv_name = "clk-mt2712-mm", >> }; >> >> static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = { >> @@ -202,6 +204,7 @@ static const struct mtk_mmsys_driver_data >> mt8173_mmsys_driver_data = { >> .main_len = ARRAY_SIZE(mt8173_mtk_ddp_main), >> .ext_path = mt8173_mtk_ddp_ext, >> .ext_len = ARRAY_SIZE(mt8173_mtk_ddp_ext), >> +.clk_drv_name = "clk-mt8173-mm", >> }; >> >> static int mtk_drm_kms_init(struct drm_device *drm) >> @@ -499,6 +502,24 @@ static int mtk_drm_probe(struct platform_device *pdev) >> INIT_WORK(>commit.work, mtk_atomic_work); >> private->data = of_device_get_match_data(dev); >> >> +/* >> + * MMSYS includes apart from components management a block providing >> + * clocks for the subsystem. We probe this clock driver via a platform >> + * device. >> + */ >> +if (private->data->clk_drv_name) { >> +private->clk_dev = platform_device_register_data(dev, >> +private->data->clk_drv_name, -1, >> +NULL, 0); >> + >> +if (IS_ERR(private->clk_dev)) { >> +dev_err(dev, "failed to register %s platform device\n", >> +private->data->clk_drv_name); >> + >> +return PTR_ERR(private->clk_dev); >> +} >> +} >> + >> private->config_regs = syscon_node_to_regmap(dev->of_node); >> if (IS_ERR(private->config_regs)) >> return PTR_ERR(private->config_regs); >> @@ -605,6 +626,9 @@ static int mtk_drm_remove(struct platform_device *pdev) >> for (i = 0; i < DDP_COMPONENT_ID_MAX; i++) >> of_node_put(private->comp_node[i]); >> >> +if (private->clk_dev) >> +platform_device_unregister(private->clk_dev); >> + >> return 0; >> } >> >> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h >> b/drivers/gpu/drm/mediatek/mtk_drm_drv.h >> index 63a121577dcb..8fe9136adc38 100644 >> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h >> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h >> @@ -29,11 +29,13 @@ struct mtk_mmsys_driver_data { >> unsigned int third_len; >> >> bool shadow_register; >> +const char *clk_drv_name; >> }; >> >> struct mtk_drm_private { >> struct drm_device *drm; >> struct device *dma_dev; >> +struct platform_device *clk_dev; >> >> unsigned int num_pipes; >> > > ___ > Linux-mediatek mailing list > linux-media...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/tidss: dispc: Rewrite naive plane positioning code
On 13/02/2020 13:52, Jyri Sarha wrote: > On 13/02/2020 12:49, Tomi Valkeinen wrote: >> On 13/02/2020 12:44, Jyri Sarha wrote: >> >>> + /* >>> + * If a plane on a CRTC changes add all active planes on that >>> + * CRTC to the atomic state. This is needed for updating the >>> + * plane positions in tidss_crtc_position_planes() which is >>> + * called from crtc_atomic_enable() and crtc_atomic_flush(). >>> + * The update is needed for x,y-position changes too, so >>> + * zpos_changed condition is not enough and we need this if >>> + * planes_changed is true too. >>> + */ >>> + for_each_new_crtc_in_state(state, crtc, cstate, i) { >>> + if (cstate->zpos_changed || cstate->planes_changed) { >>> + ret = drm_atomic_add_affected_planes(state, crtc); >>> + if (ret) >>> + return ret; >>> + } >>> + } >> >> I think 99.99...% of the commits are such where only planes' fb address >> is changed. I think "planes_changed" is true for these. I wonder if it >> would be a sensible optimization to skip this for those 99.99...% cases. >> Can we detect that easily? >> > > Sure by looping all planes in the state through with > for_each_oldnew_plane_in_state() and checking if crtc_x or crtc_y has > changed. But then again writing the positions of max 4 planes is really > not that heavy operation. There is more calculation to do and more > registers to write when updating the fp, so I do not think avoiding the > OVR update justifies the extra complexity. > Well, I implemented this anyway just for the fun of it. After all the added complexity is not that much, since we already have an extended CRTC state. Judge your self, I'll send v4 shortly. Best regards, Jyri >> Well, we haven't optimized for those 99.99...% cases anywhere else >> either, so it's possible it doesn't matter. >> > > > > > -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [resend PATCH v6 10/12] clk: mediatek: mt8183: switch mmsys to platform device probing
On 09/12/2019 09:51, CK Hu wrote: > Hi, Matthias: > > On Sat, 2019-12-07 at 23:47 +0100, matthias@kernel.org wrote: >> From: Matthias Brugger >> >> Switch probing for the MMSYS to support invocation to a >> plain paltform device. The driver will be probed by the DRM subsystem. >> >> Singed-off-by: Matthias Brugger >> --- >> drivers/clk/mediatek/clk-mt8183-mm.c | 39 +++- >> 1 file changed, 27 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/clk/mediatek/clk-mt8183-mm.c >> b/drivers/clk/mediatek/clk-mt8183-mm.c >> index 720c696b506d..e6dcad18d81a 100644 >> --- a/drivers/clk/mediatek/clk-mt8183-mm.c >> +++ b/drivers/clk/mediatek/clk-mt8183-mm.c >> @@ -3,14 +3,20 @@ >> // Copyright (c) 2018 MediaTek Inc. >> // Author: Weiyi Lu >> >> +#include >> #include >> #include >> +#include >> >> #include "clk-mtk.h" >> #include "clk-gate.h" >> >> #include >> >> +struct clk_mt8183_mm_priv { >> +struct clk_onecell_data *clk_data; >> +}; >> + >> static const struct mtk_gate_regs mm0_cg_regs = { >> .set_ofs = 0x104, >> .clr_ofs = 0x108, >> @@ -84,28 +90,37 @@ static const struct mtk_gate mm_clks[] = { >> >> static int clk_mt8183_mm_probe(struct platform_device *pdev) >> { >> -struct clk_onecell_data *clk_data; >> -struct device_node *node = pdev->dev.of_node; >> +struct clk_mt8183_mm_priv *private; >> +struct device_node *node = pdev->dev.parent->of_node; >> + >> +private = devm_kzalloc(>dev, sizeof(*private), GFP_KERNEL); >> +if (!private) >> +return -ENOMEM; >> >> -clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK); >> +private->clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK); >> +platform_set_drvdata(pdev, private); > > There is a more simple modification that you need not to define struct > clk_mt8183_mm_priv, > > clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK); > platform_set_drvdata(pdev, clk_data); > Yes you are right, I'll fix that for all the drivers. Thanks, Matthias > Regards, > CK > >> >> mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks), >> -clk_data); >> +private->clk_data); >> >> -return of_clk_add_provider(node, of_clk_src_onecell_get, clk_data); >> +return of_clk_add_provider(node, of_clk_src_onecell_get, >> +private->clk_data); >> } >> >> -static const struct of_device_id of_match_clk_mt8183_mm[] = { >> -{ .compatible = "mediatek,mt8183-mmsys", }, >> -{} >> -}; >> +static int clk_mt8183_mm_remove(struct platform_device *pdev) >> +{ >> +struct clk_mt8183_mm_priv *private = platform_get_drvdata(pdev); >> + >> +kfree(private->clk_data); >> + >> +return 0; >> +} >> >> static struct platform_driver clk_mt8183_mm_drv = { >> .probe = clk_mt8183_mm_probe, >> +.remove = clk_mt8183_mm_remove, >> .driver = { >> .name = "clk-mt8183-mm", >> -.of_match_table = of_match_clk_mt8183_mm, >> }, >> }; >> - >> -builtin_platform_driver(clk_mt8183_mm_drv); >> +module_platform_driver(clk_mt8183_mm_drv); > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [resend PATCH v6 06/12] clk: mediatek: mt2701: switch mmsys to platform device probing
On 09/12/2019 10:58, Enric Balletbo i Serra wrote: > Hi Matthias, > > On 7/12/19 23:47, matthias@kernel.org wrote: >> From: Matthias Brugger >> >> Switch probing for the MMSYS to support invocation to a plain >> paltform device. The driver will be probed by the DRM subsystem. >> >> Signed-off-by: Matthias Brugger >> --- >> drivers/clk/mediatek/clk-mt2701-mm.c | 41 >> 1 file changed, 29 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/clk/mediatek/clk-mt2701-mm.c >> b/drivers/clk/mediatek/clk-mt2701-mm.c >> index 054b597d4a73..4a9433c2b2b8 100644 >> --- a/drivers/clk/mediatek/clk-mt2701-mm.c >> +++ b/drivers/clk/mediatek/clk-mt2701-mm.c >> @@ -4,14 +4,20 @@ >> * Author: Shunli Wang >> */ >> >> +#include >> #include >> #include >> +#include >> >> #include "clk-mtk.h" >> #include "clk-gate.h" >> >> #include >> >> +struct clk_mt2701_mm_priv { >> +struct clk_onecell_data *clk_data; >> +}; >> + >> static const struct mtk_gate_regs disp0_cg_regs = { >> .set_ofs = 0x0104, >> .clr_ofs = 0x0108, >> @@ -79,23 +85,25 @@ static const struct mtk_gate mm_clks[] = { >> GATE_DISP1(CLK_MM_TVE_FMM, "mm_tve_fmm", "mm_sel", 14), >> }; >> >> -static const struct of_device_id of_match_clk_mt2701_mm[] = { >> -{ .compatible = "mediatek,mt2701-mmsys", }, >> -{} >> -}; >> - >> static int clk_mt2701_mm_probe(struct platform_device *pdev) >> { >> -struct clk_onecell_data *clk_data; >> int r; >> -struct device_node *node = pdev->dev.of_node; >> +struct device_node *node = pdev->dev.parent->of_node; >> +struct clk_mt2701_mm_priv *private; >> + >> +private = devm_kzalloc(>dev, sizeof(*private), GFP_KERNEL); >> +if (!private) >> +return -ENOMEM; >> >> -clk_data = mtk_alloc_clk_data(CLK_MM_NR); >> +private->clk_data = mtk_alloc_clk_data(CLK_MM_NR); >> + >> +platform_set_drvdata(pdev, private); >> >> mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks), >> -clk_data); >> +private->clk_data); >> >> -r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data); >> +r = of_clk_add_provider(node, of_clk_src_onecell_get, >> +private->clk_data); >> if (r) >> dev_err(>dev, >> "could not register clock provider: %s: %d\n", >> @@ -104,12 +112,21 @@ static int clk_mt2701_mm_probe(struct platform_device >> *pdev) >> return r; >> } >> >> +static int clk_mt2701_mm_remove(struct platform_device *pdev) >> +{ >> +struct clk_mt2701_mm_priv *private = platform_get_drvdata(pdev); >> + > > I think that private->clk_data->clks is also kallocated and need to be freed? > > But I think that the best approach now is to switch to use devm allocations in > clk-mt2701-mm.c and this remove function will not be needed. > This is an API change which will affect over 50 drivers. For now I'll fix it by adding a kfree here. I will send a follow-up patch to fix it in the API. Good catch! Matthias >> +kfree(private->clk_data); >> + >> +return 0; >> +} >> + >> static struct platform_driver clk_mt2701_mm_drv = { >> .probe = clk_mt2701_mm_probe, >> +.remove = clk_mt2701_mm_remove, >> .driver = { >> .name = "clk-mt2701-mm", >> -.of_match_table = of_match_clk_mt2701_mm, >> }, >> }; >> >> -builtin_platform_driver(clk_mt2701_mm_drv); >> +module_platform_driver(clk_mt2701_mm_drv); >> ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [resend PATCH v6 04/12] drm: mediatek: Omit warning on probe defers
On 09/12/2019 10:39, Enric Balletbo i Serra wrote: > Hi Matthias, > > On 7/12/19 23:47, matthias@kernel.org wrote: >> From: Matthias Brugger >> >> It can happen that the mmsys clock drivers aren't probed before the >> platform driver gets invoked. The platform driver used to print a warning >> that the driver failed to get the clocks. Omit this error on >> the defered probe path. >> >> Signed-off-by: Matthias Brugger >> --- >> drivers/gpu/drm/mediatek/mtk_disp_color.c | 5 - >> drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 5 - >> drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 5 - >> drivers/gpu/drm/mediatek/mtk_dpi.c| 12 +--- >> drivers/gpu/drm/mediatek/mtk_drm_ddp.c| 4 +++- >> drivers/gpu/drm/mediatek/mtk_dsi.c| 8 ++-- >> drivers/gpu/drm/mediatek/mtk_hdmi.c | 4 +++- >> 7 files changed, 33 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_color.c >> b/drivers/gpu/drm/mediatek/mtk_disp_color.c >> index 59de2a46aa49..8f0fc96ef7bc 100644 >> --- a/drivers/gpu/drm/mediatek/mtk_disp_color.c >> +++ b/drivers/gpu/drm/mediatek/mtk_disp_color.c >> @@ -118,7 +118,10 @@ static int mtk_disp_color_probe(struct platform_device >> *pdev) >> ret = mtk_ddp_comp_init(dev, dev->of_node, >ddp_comp, comp_id, >> _disp_color_funcs); >> if (ret) { >> -dev_err(dev, "Failed to initialize component: %d\n", ret); >> +if (ret != -EPROBE_DEFER) >> +dev_err(dev, "Failed to initialize component: %d\n", >> +ret); >> + >> return ret; >> } >> >> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c >> b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c >> index 21851756c579..7487b0182c05 100644 >> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c >> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c >> @@ -285,7 +285,10 @@ static int mtk_disp_ovl_probe(struct platform_device >> *pdev) >> ret = mtk_ddp_comp_init(dev, dev->of_node, >ddp_comp, comp_id, >> _disp_ovl_funcs); >> if (ret) { >> -dev_err(dev, "Failed to initialize component: %d\n", ret); >> +if (ret != -EPROBE_DEFER) >> +dev_err(dev, "Failed to initialize component: %d\n", >> +ret); >> + >> return ret; >> } >> >> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c >> b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c >> index 405afef31407..835ea8f8dab9 100644 >> --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c >> +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c >> @@ -287,7 +287,10 @@ static int mtk_disp_rdma_probe(struct platform_device >> *pdev) >> ret = mtk_ddp_comp_init(dev, dev->of_node, >ddp_comp, comp_id, >> _disp_rdma_funcs); >> if (ret) { >> -dev_err(dev, "Failed to initialize component: %d\n", ret); >> +if (ret != -EPROBE_DEFER) >> +dev_err(dev, "Failed to initialize component: %d\n", >> +ret); >> + >> return ret; >> } >> >> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c >> b/drivers/gpu/drm/mediatek/mtk_dpi.c >> index be6d95c5ff25..9ed32470ad02 100644 >> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c >> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c >> @@ -700,21 +700,27 @@ static int mtk_dpi_probe(struct platform_device *pdev) >> dpi->engine_clk = devm_clk_get(dev, "engine"); >> if (IS_ERR(dpi->engine_clk)) { >> ret = PTR_ERR(dpi->engine_clk); >> -dev_err(dev, "Failed to get engine clock: %d\n", ret); >> +if (ret != -EPROBE_DEFER) >> +dev_err(dev, "Failed to get engine clock: %d\n", ret); > > This is only to print an error and I think that devm_clk_get will print a > warning if the clk is not found. I guess that you can just remove the dev_err > print logic, here and below. > I didn't follow all the paths, but at least devres_alloc in devm_clk_get isn't able to alloc the memory, it will silently return -ENOMEM. So I think it is OK to print an error message here. Regards, Matthias > In case there is an optional clock you could use devm_clk_get_optional, not > sure > if there is any, though. > >> + >> return ret; >> } >> >> dpi->pixel_clk = devm_clk_get(dev, "pixel"); >> if (IS_ERR(dpi->pixel_clk)) { >> ret = PTR_ERR(dpi->pixel_clk); >> -dev_err(dev, "Failed to get pixel clock: %d\n", ret); >> +if (ret != -EPROBE_DEFER) >> +dev_err(dev, "Failed to get pixel clock: %d\n", ret); >> + > > ditto > >> return ret; >> } >> >> dpi->tvd_clk = devm_clk_get(dev, "pll"); >> if (IS_ERR(dpi->tvd_clk)) { >> ret = PTR_ERR(dpi->tvd_clk); >> -dev_err(dev, "Failed to get tvdpll clock: %d\n", ret); >> +if (ret !=
Re: [PATCH v3 3/4] drm/virtio: batch resource creation
On Thu, Feb 13, 2020 at 5:22 AM Gerd Hoffmann wrote: > > Move virtio_gpu_notify() to higher-level functions for > virtio_gpu_cmd_create_resource(), virtio_gpu_cmd_resource_create_3d() > and virtio_gpu_cmd_resource_attach_backing(). > > virtio_gpu_object_create() will batch commands and notify only once when > creating a resource. > > Signed-off-by: Gerd Hoffmann > --- > drivers/gpu/drm/virtio/virtgpu_object.c | 1 + > drivers/gpu/drm/virtio/virtgpu_vq.c | 3 --- > 2 files changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c > b/drivers/gpu/drm/virtio/virtgpu_object.c > index 8870ee23ff2b..65d6834d3c74 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_object.c > +++ b/drivers/gpu/drm/virtio/virtgpu_object.c > @@ -224,6 +224,7 @@ int virtio_gpu_object_create(struct virtio_gpu_device > *vgdev, > return ret; The virtqueue might become full without ever being notified on errors. We should notify on errors, or better yet, virtio_gpu_queue_ctrl_sgs should notify before waiting. > } > > + virtio_gpu_notify(vgdev); > *bo_ptr = bo; > return 0; > > diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c > b/drivers/gpu/drm/virtio/virtgpu_vq.c > index 9d4ca0fafa5f..778b7acf2f7f 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_vq.c > +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c > @@ -514,7 +514,6 @@ void virtio_gpu_cmd_create_resource(struct > virtio_gpu_device *vgdev, > cmd_p->height = cpu_to_le32(params->height); > > virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, fence); > - virtio_gpu_notify(vgdev); > bo->created = true; > } > > @@ -643,7 +642,6 @@ virtio_gpu_cmd_resource_attach_backing(struct > virtio_gpu_device *vgdev, > vbuf->data_size = sizeof(*ents) * nents; > > virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, fence); > - virtio_gpu_notify(vgdev); > } > > static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device > *vgdev, > @@ -1010,7 +1008,6 @@ virtio_gpu_cmd_resource_create_3d(struct > virtio_gpu_device *vgdev, > cmd_p->flags = cpu_to_le32(params->flags); > > virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, fence); > - virtio_gpu_notify(vgdev); > > bo->created = true; > } > -- > 2.18.2 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 0/4] drm/virtio: rework batching
Series is Reviewed-by: Chia-I Wu After the series, virtio_gpu_cmd_* may or may not call virtio_gpu_notify. It is error-prone and should be fixed, such that virtio_gpu_cmd_* never notifies, or such that different naming conventions are used for functions that notify and for those don't. On Thu, Feb 13, 2020 at 5:22 AM Gerd Hoffmann wrote: > > v4: > - split into multiple patches. > > Gerd Hoffmann (4): > drm/virtio: rework notification for better batching > drm/virtio: batch plane updates (pageflip) > drm/virtio: batch resource creation > drm/virtio: batch display query. > > drivers/gpu/drm/virtio/virtgpu_drv.h | 6 ++-- > drivers/gpu/drm/virtio/virtgpu_display.c | 2 ++ > drivers/gpu/drm/virtio/virtgpu_ioctl.c | 1 + > drivers/gpu/drm/virtio/virtgpu_kms.c | 2 ++ > drivers/gpu/drm/virtio/virtgpu_object.c | 1 + > drivers/gpu/drm/virtio/virtgpu_plane.c | 7 ++-- > drivers/gpu/drm/virtio/virtgpu_vq.c | 41 +--- > 7 files changed, 33 insertions(+), 27 deletions(-) > > -- > 2.18.2 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [resend PATCH v6 01/12] dt-bindings: display: mediatek: Add mmsys binding description
On 09/12/2019 06:12, CK Hu wrote: > Hi, Matthias: > > On Sat, 2019-12-07 at 23:47 +0100, matthias@kernel.org wrote: >> From: Matthias Brugger >> >> The MediaTek DRM has a block called mmsys, which sets >> the routing and enalbes the different blocks. >> This patch adds one line for the mmsys bindings description. >> >> Signed-off-by: Matthias Brugger >> --- >> .../display/mediatek/mediatek,disp.txt| 28 ++- >> 1 file changed, 15 insertions(+), 13 deletions(-) >> >> diff --git >> a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt >> b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt >> index 8469de510001..c71c8a4b73ff 100644 >> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt >> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt >> @@ -27,20 +27,22 @@ >> Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt. >> >> Required properties (all function blocks): >> - compatible: "mediatek,-disp-", one of >> -"mediatek,-disp-ovl" - overlay (4 layers, blending, csc) >> -"mediatek,-disp-rdma" - read DMA / line buffer >> -"mediatek,-disp-wdma" - write DMA >> -"mediatek,-disp-color" - color processor >> -"mediatek,-disp-aal" - adaptive ambient light controller >> -"mediatek,-disp-gamma" - gamma correction >> -"mediatek,-disp-merge" - merge streams from two RDMA sources >> -"mediatek,-disp-split" - split stream to two encoders >> -"mediatek,-disp-ufoe" - data compression engine >> -"mediatek,-dsi"- DSI controller, see mediatek,dsi.txt >> -"mediatek,-dpi"- DPI controller, see mediatek,dpi.txt >> -"mediatek,-disp-mutex" - display mutex >> -"mediatek,-disp-od"- overdrive >> +"mediatek,-disp-ovl" - overlay (4 layers, blending, >> csc) > > This patch conflicts with 5.5-rc, please resend this patch base on > 5.5-rc1. > >> +"mediatek,-disp-rdma" - read DMA / line buffer >> +"mediatek,-disp-wdma" - write DMA >> +"mediatek,-disp-color"- color processor >> +"mediatek,-disp-aal" - adaptive ambient light >> controller >> +"mediatek,-disp-gamma"- gamma correction >> +"mediatek,-disp-merge"- merge streams from two RDMA >> sources >> +"mediatek,-disp-split"- split stream to two encoders >> +"mediatek,-disp-ufoe" - data compression engine >> +"mediatek,-dsi" - DSI controller, see >> mediatek,dsi.txt >> +"mediatek,-dpi" - DPI controller, see >> mediatek,dpi.txt >> +"mediatek,-disp-mutex"- display mutex >> +"mediatek,-disp-od" - overdrive >> +"mediatek,-mmsys", "syscon" - provide clocks and components >> management >>the supported chips are mt2701, mt2712 and mt8173. > > The original binding document for mmsys is in [1], I think we should not > define it in duplicate. Maybe you could remove the original document. > > [1] > https://www.kernel.org/doc/Documentation/devicetree/bindings/arm/mediatek/mediatek%2Cmmsys.txt I think we should keep it, as it describes some requirements that are otherwise lost. I'll adapt the mmsys description and add a hint to it, like we do for dsi and dpi. Regards, Matthias > > Regards, > CK > >> + >> - reg: Physical base address and length of the function block register space >> - interrupts: The interrupt signal from the function block (required, >> except for >>merge and split function blocks). > > ___ > Linux-mediatek mailing list > linux-media...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 14/15] drm/amdgpu/ring: move debugfs init into core amdgpu debugfs
On Thu, Feb 13, 2020 at 12:28 PM Christian König wrote: > > Am 13.02.20 um 15:32 schrieb Alex Deucher: > > On Thu, Feb 13, 2020 at 5:17 AM Christian König > > wrote: > >> Am 13.02.20 um 10:54 schrieb Daniel Vetter: > >>> On Fri, Feb 07, 2020 at 02:50:57PM -0500, Alex Deucher wrote: > In order to remove the load and unload drm callbacks, > we need to reorder the init sequence to move all the drm > debugfs file handling. Do this for rings. > > Acked-by: Christian König > Signed-off-by: Alex Deucher > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 23 - > drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c| 15 +++--- > drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h| 4 > 3 files changed, 29 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > index df3919ef886b..7379910790c9 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > @@ -1218,7 +1218,7 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_ib_preempt, NULL, > > int amdgpu_debugfs_init(struct amdgpu_device *adev) > { > -int r; > +int r, i; > > adev->debugfs_preempt = > debugfs_create_file("amdgpu_preempt_ib", 0600, > @@ -1268,12 +1268,33 @@ int amdgpu_debugfs_init(struct amdgpu_device > *adev) > } > #endif > > +for (i = 0; i < AMDGPU_MAX_RINGS; ++i) { > +struct amdgpu_ring *ring = adev->rings[i]; > + > +if (!ring) > +continue; > + > +if (amdgpu_debugfs_ring_init(adev, ring)) { > +DRM_ERROR("Failed to register debugfs file for > rings !\n"); > +} > +} > + > return amdgpu_debugfs_add_files(adev, amdgpu_debugfs_list, > ARRAY_SIZE(amdgpu_debugfs_list)); > } > > void amdgpu_debugfs_fini(struct amdgpu_device *adev) > >>> btw debugfs_fini shouldn't be needed anymore, Greg KH removed this all. > >>> drm core removes all debugfs files recusrively for you, there should be 0 > >>> need for debugfs cleanup. > >> Oh, yes please. Removing that was on my TODO list for an eternity as well. > >> > >>> Also at least my tree doesn't even have this, where does this apply to? > >> I would guess amd-staging-drm-next, but it might be that Alex is waiting > >> for the next rebase to land. > >> > > Patches are against my drm-next branch which is based on fdo drm-next. > > There are a number of files which the driver creates directly rather > > than through drm. > > The last time I locked it I was about to completely nuke creating > anything through DRM and just create it directly. > > As Daniel wrote we also don't have to remove anything explicitly, that > is done implicitly when the whole directory is removed. I dunno. Most of this stuff has been there for years. Maybe someone wants to take a look if it can be further cleaned up. It builds fine against current kernels. Alex > > Christian. > > > > > Alex > > > >> Christian. > >> > >>> -Daniel > >>> > { > +int i; > + > +for (i = 0; i < AMDGPU_MAX_RINGS; ++i) { > +struct amdgpu_ring *ring = adev->rings[i]; > + > +if (!ring) > +continue; > + > +amdgpu_debugfs_ring_fini(ring); > +} > amdgpu_ttm_debugfs_fini(adev); > debugfs_remove(adev->debugfs_preempt); > } > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c > index e5c83e164d82..539be138260e 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c > @@ -48,9 +48,6 @@ > * wptr. The GPU then starts fetching commands and executes > * them until the pointers are equal again. > */ > -static int amdgpu_debugfs_ring_init(struct amdgpu_device *adev, > -struct amdgpu_ring *ring); > -static void amdgpu_debugfs_ring_fini(struct amdgpu_ring *ring); > > /** > * amdgpu_ring_alloc - allocate space on the ring buffer > @@ -334,10 +331,6 @@ int amdgpu_ring_init(struct amdgpu_device *adev, > struct amdgpu_ring *ring, > for (i = 0; i < DRM_SCHED_PRIORITY_MAX; ++i) > atomic_set(>num_jobs[i], 0); > > -if (amdgpu_debugfs_ring_init(adev, ring)) { > -DRM_ERROR("Failed to register debugfs file for rings !\n"); > -} > - > return 0; > } > > @@ -367,8 +360,6 @@ void amdgpu_ring_fini(struct amdgpu_ring *ring) >
Re: [PATCH 14/15] drm/amdgpu/ring: move debugfs init into core amdgpu debugfs
Am 13.02.20 um 15:32 schrieb Alex Deucher: On Thu, Feb 13, 2020 at 5:17 AM Christian König wrote: Am 13.02.20 um 10:54 schrieb Daniel Vetter: On Fri, Feb 07, 2020 at 02:50:57PM -0500, Alex Deucher wrote: In order to remove the load and unload drm callbacks, we need to reorder the init sequence to move all the drm debugfs file handling. Do this for rings. Acked-by: Christian König Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 23 - drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c| 15 +++--- drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h| 4 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c index df3919ef886b..7379910790c9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c @@ -1218,7 +1218,7 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_ib_preempt, NULL, int amdgpu_debugfs_init(struct amdgpu_device *adev) { -int r; +int r, i; adev->debugfs_preempt = debugfs_create_file("amdgpu_preempt_ib", 0600, @@ -1268,12 +1268,33 @@ int amdgpu_debugfs_init(struct amdgpu_device *adev) } #endif +for (i = 0; i < AMDGPU_MAX_RINGS; ++i) { +struct amdgpu_ring *ring = adev->rings[i]; + +if (!ring) +continue; + +if (amdgpu_debugfs_ring_init(adev, ring)) { +DRM_ERROR("Failed to register debugfs file for rings !\n"); +} +} + return amdgpu_debugfs_add_files(adev, amdgpu_debugfs_list, ARRAY_SIZE(amdgpu_debugfs_list)); } void amdgpu_debugfs_fini(struct amdgpu_device *adev) btw debugfs_fini shouldn't be needed anymore, Greg KH removed this all. drm core removes all debugfs files recusrively for you, there should be 0 need for debugfs cleanup. Oh, yes please. Removing that was on my TODO list for an eternity as well. Also at least my tree doesn't even have this, where does this apply to? I would guess amd-staging-drm-next, but it might be that Alex is waiting for the next rebase to land. Patches are against my drm-next branch which is based on fdo drm-next. There are a number of files which the driver creates directly rather than through drm. The last time I locked it I was about to completely nuke creating anything through DRM and just create it directly. As Daniel wrote we also don't have to remove anything explicitly, that is done implicitly when the whole directory is removed. Christian. Alex Christian. -Daniel { +int i; + +for (i = 0; i < AMDGPU_MAX_RINGS; ++i) { +struct amdgpu_ring *ring = adev->rings[i]; + +if (!ring) +continue; + +amdgpu_debugfs_ring_fini(ring); +} amdgpu_ttm_debugfs_fini(adev); debugfs_remove(adev->debugfs_preempt); } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c index e5c83e164d82..539be138260e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c @@ -48,9 +48,6 @@ * wptr. The GPU then starts fetching commands and executes * them until the pointers are equal again. */ -static int amdgpu_debugfs_ring_init(struct amdgpu_device *adev, -struct amdgpu_ring *ring); -static void amdgpu_debugfs_ring_fini(struct amdgpu_ring *ring); /** * amdgpu_ring_alloc - allocate space on the ring buffer @@ -334,10 +331,6 @@ int amdgpu_ring_init(struct amdgpu_device *adev, struct amdgpu_ring *ring, for (i = 0; i < DRM_SCHED_PRIORITY_MAX; ++i) atomic_set(>num_jobs[i], 0); -if (amdgpu_debugfs_ring_init(adev, ring)) { -DRM_ERROR("Failed to register debugfs file for rings !\n"); -} - return 0; } @@ -367,8 +360,6 @@ void amdgpu_ring_fini(struct amdgpu_ring *ring) >gpu_addr, (void **)>ring); -amdgpu_debugfs_ring_fini(ring); - dma_fence_put(ring->vmid_wait); ring->vmid_wait = NULL; ring->me = 0; @@ -485,8 +476,8 @@ static const struct file_operations amdgpu_debugfs_ring_fops = { #endif -static int amdgpu_debugfs_ring_init(struct amdgpu_device *adev, -struct amdgpu_ring *ring) +int amdgpu_debugfs_ring_init(struct amdgpu_device *adev, + struct amdgpu_ring *ring) { #if defined(CONFIG_DEBUG_FS) struct drm_minor *minor = adev->ddev->primary; @@ -507,7 +498,7 @@ static int amdgpu_debugfs_ring_init(struct amdgpu_device *adev, return 0; } -static void amdgpu_debugfs_ring_fini(struct amdgpu_ring *ring) +void amdgpu_debugfs_ring_fini(struct amdgpu_ring *ring) { #if defined(CONFIG_DEBUG_FS) debugfs_remove(ring->ent); diff --git
Re: [RFC PATCH 5/6] drm/qxl: don't use ttm bo->offset
Am 13.02.20 um 15:30 schrieb Gerd Hoffmann: @@ -311,10 +311,8 @@ qxl_bo_physical_address(struct qxl_device *qdev, struct qxl_bo *bo, (bo->tbo.mem.mem_type == TTM_PL_VRAM) ? >main_slot : >surfaces_slot; - WARN_ON_ONCE((bo->tbo.offset & slot->gpu_offset) != slot->gpu_offset); - - /* TODO - need to hold one of the locks to read tbo.offset */ - return slot->high_bits | (bo->tbo.offset - slot->gpu_offset + offset); + return slot->high_bits | ((bo->tbo.mem.start << PAGE_SHIFT) + + slot->gpu_offset + offset); } --verbose please. The warning and the TODO should probably stay, don't they? I don't get the logic behind this change. We ran into the problem that the whole offset handling in TTM is actually completely hardware specific. E.g. Some need pfn, some need byte offsets, some need 32bit, some 64bit and some don't have a consistent offset at all (e.g. amdgpu for exmaple). So we try to move that back into the drivers to concentrate on memory management in TTM. The other chunks look sane, calculating slot->gpu_offset in setup_slot() certainly makes sense. Thanks for the review, Christian. cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 13/15] drm/amdgpu/display: split dp connector registration (v3)
Anyone want to take a shot at this one? Alex On Fri, Feb 7, 2020 at 4:17 PM Alex Deucher wrote: > > Split into init and register functions to avoid a segfault > in some configs when the load/unload callbacks are removed. > > v2: > - add back accidently dropped has_aux setting > - set dev in late_register > > v3: > - fix dp cec ordering > > Signed-off-by: Alex Deucher > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 16 > drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 10 ++ > .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 7 ++- > 3 files changed, 24 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > index ec1501e3a63a..f355d9a752d2 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > @@ -1461,6 +1461,20 @@ static enum drm_mode_status > amdgpu_connector_dp_mode_valid(struct drm_connector > return MODE_OK; > } > > +static int > +amdgpu_connector_late_register(struct drm_connector *connector) > +{ > + struct amdgpu_connector *amdgpu_connector = > to_amdgpu_connector(connector); > + int r = 0; > + > + if (amdgpu_connector->ddc_bus->has_aux) { > + amdgpu_connector->ddc_bus->aux.dev = > amdgpu_connector->base.kdev; > + r = drm_dp_aux_register(_connector->ddc_bus->aux); > + } > + > + return r; > +} > + > static const struct drm_connector_helper_funcs > amdgpu_connector_dp_helper_funcs = { > .get_modes = amdgpu_connector_dp_get_modes, > .mode_valid = amdgpu_connector_dp_mode_valid, > @@ -1475,6 +1489,7 @@ static const struct drm_connector_funcs > amdgpu_connector_dp_funcs = { > .early_unregister = amdgpu_connector_unregister, > .destroy = amdgpu_connector_destroy, > .force = amdgpu_connector_dvi_force, > + .late_register = amdgpu_connector_late_register, > }; > > static const struct drm_connector_funcs amdgpu_connector_edp_funcs = { > @@ -1485,6 +1500,7 @@ static const struct drm_connector_funcs > amdgpu_connector_edp_funcs = { > .early_unregister = amdgpu_connector_unregister, > .destroy = amdgpu_connector_destroy, > .force = amdgpu_connector_dvi_force, > + .late_register = amdgpu_connector_late_register, > }; > > void > diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c > b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c > index ea702a64f807..9b74cfdba7b8 100644 > --- a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c > +++ b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c > @@ -186,16 +186,10 @@ amdgpu_atombios_dp_aux_transfer(struct drm_dp_aux *aux, > struct drm_dp_aux_msg *m > > void amdgpu_atombios_dp_aux_init(struct amdgpu_connector *amdgpu_connector) > { > - int ret; > - > amdgpu_connector->ddc_bus->rec.hpd = amdgpu_connector->hpd.hpd; > - amdgpu_connector->ddc_bus->aux.dev = amdgpu_connector->base.kdev; > amdgpu_connector->ddc_bus->aux.transfer = > amdgpu_atombios_dp_aux_transfer; > - ret = drm_dp_aux_register(_connector->ddc_bus->aux); > - if (!ret) > - amdgpu_connector->ddc_bus->has_aux = true; > - > - WARN(ret, "drm_dp_aux_register_i2c_bus() failed with error %d\n", > ret); > + drm_dp_aux_init(_connector->ddc_bus->aux); > + amdgpu_connector->ddc_bus->has_aux = true; > } > > /* general DP utility functions */ > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > index 3959c942c88b..d5b9e72f2649 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > @@ -155,6 +155,11 @@ amdgpu_dm_mst_connector_late_register(struct > drm_connector *connector) > struct amdgpu_dm_connector *amdgpu_dm_connector = > to_amdgpu_dm_connector(connector); > struct drm_dp_mst_port *port = amdgpu_dm_connector->port; > + int r; > + > + r = drm_dp_aux_register(_dm_connector->dm_dp_aux.aux); > + if (r) > + return r; > > #if defined(CONFIG_DEBUG_FS) > connector_debugfs_init(amdgpu_dm_connector); > @@ -484,7 +489,7 @@ void amdgpu_dm_initialize_dp_connector(struct > amdgpu_display_manager *dm, > aconnector->dm_dp_aux.aux.transfer = dm_dp_aux_transfer; > aconnector->dm_dp_aux.ddc_service = aconnector->dc_link->ddc; > > - drm_dp_aux_register(>dm_dp_aux.aux); > + drm_dp_aux_init(>dm_dp_aux.aux); > drm_dp_cec_register_connector(>dm_dp_aux.aux, > >base); > > -- > 2.24.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/atmel-hlcdc: set layer REP bit to enable replication logic
On Fri, 12 Jul 2019 18:21:17 +0200 Sam Ravnborg wrote: > Hi Joshua. > > On Tue, Jul 09, 2019 at 04:24:49PM +, nicolas.fe...@microchip.com wrote: > > On 09/07/2019 at 17:35, Joshua Henderson wrote: > > > This bit enables replication logic to expand an RGB color less than 24 > > > bits, to 24 bits, which is used internally for all formats. Otherwise, > > > the least significant bits are always set to zero and the color may not > > > be what is expected. > > > > > > Signed-off-by: Joshua Henderson > > > > Acked-by: Nicolas Ferre > > > > Here is patchwork entry: > > https://patchwork.kernel.org/patch/11037167/ > > > > Thanks, best regards, > >Nicolas > > > > > --- > > > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c > > > b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c > > > index eb7c4cf..6f6cf61 100644 > > > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c > > > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c > > > @@ -389,7 +389,7 @@ atmel_hlcdc_plane_update_general_settings(struct > > > atmel_hlcdc_plane *plane, > > > atmel_hlcdc_layer_write_cfg(>layer, > > > ATMEL_HLCDC_LAYER_DMA_CFG, > > > cfg); > > > > > > - cfg = ATMEL_HLCDC_LAYER_DMA; > > > + cfg = ATMEL_HLCDC_LAYER_DMA | ATMEL_HLCDC_LAYER_REP; > > > > > > if (plane->base.type != DRM_PLANE_TYPE_PRIMARY) { > > > cfg |= ATMEL_HLCDC_LAYER_OVR | > > > ATMEL_HLCDC_LAYER_ITER2BL | > > Thanks - this gave me an opportunity to read a bit more in the datasheet > of the controller. > Applied to drm-misc-next with Ack from Nicolas. I was about to add my R-b and ask you to apply the patch :-). I'm glad you didn't wait for my feedback to apply the fix, that means I'll be able to remove my name from the Atmel HLCDC entry soon ;-). > > Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 206519] [amdgpu] kernel NULL pointer dereference on shutdown
https://bugzilla.kernel.org/show_bug.cgi?id=206519 Alex Deucher (alexdeuc...@gmail.com) changed: What|Removed |Added CC||alexdeuc...@gmail.com --- Comment #2 from Alex Deucher (alexdeuc...@gmail.com) --- Can you bisect? -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 206519] [amdgpu] kernel NULL pointer dereference on shutdown
https://bugzilla.kernel.org/show_bug.cgi?id=206519 --- Comment #1 from Shlomo (shl...@fastmail.com) --- Created attachment 287355 --> https://bugzilla.kernel.org/attachment.cgi?id=287355=edit dmesg after boot -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 206519] New: [amdgpu] kernel NULL pointer dereference on shutdown
https://bugzilla.kernel.org/show_bug.cgi?id=206519 Bug ID: 206519 Summary: [amdgpu] kernel NULL pointer dereference on shutdown Product: Drivers Version: 2.5 Kernel Version: 5.5.1.arch1-1, 5.5.3-arch1-1 Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: shl...@fastmail.com Regression: No Created attachment 287353 --> https://bugzilla.kernel.org/attachment.cgi?id=287353=edit shutdown screen photo When I try to power off my machine, it shows the usual shutdown messages and the screens turn off, but the machine is still powered on. The virtual console shows a kernel NULL pointer dereference at address 0. I run Arch Linux. The bug occurs even if I never run X. I can turn on the machine and immediately try to shut it down, and the same bug still occurs. This bug occurred since I upgraded linux 5.4.15.arch1-1 to 5.5.1.arch1-1. I now run linux 5.5.3.arch1-1 and the bug still exists. My graphics card is Gigabyte Radeon RX VEGA 56 GAMING OC 8G, connected to six monitors. A photo of the screen at shutdown is attached. I think these are the relevant lines for this bug: BUG: kernel NULL pointer dereference, address: 0 [...] RIP: 0010:queue_work_on+0x17/0x40 Code: fd ff ff 44 89 e0 5d 41 5c c3 [...] Call Trace: handle_hpd_rx_irq+0x26e/0x320 [amdgpu] ? _raw_spin_unlock_irq+0x1d/0x30 dm_irq_work_func+0x49/0x60 [amdgpu] process_one_work+0x1e1/0x3d0 [...] -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/4] drm/radeon: align short build log
This beautifies the build log. [Before] HOSTCC drivers/gpu/drm/radeon/mkregtable MKREGTABLE drivers/gpu/drm/radeon/r100_reg_safe.h MKREGTABLE drivers/gpu/drm/radeon/rn50_reg_safe.h CC [M] drivers/gpu/drm/radeon/r100.o MKREGTABLE drivers/gpu/drm/radeon/r300_reg_safe.h CC [M] drivers/gpu/drm/radeon/r300.o [After] HOSTCC drivers/gpu/drm/radeon/mkregtable MKREG drivers/gpu/drm/radeon/r100_reg_safe.h MKREG drivers/gpu/drm/radeon/rn50_reg_safe.h CC [M] drivers/gpu/drm/radeon/r100.o MKREG drivers/gpu/drm/radeon/r300_reg_safe.h CC [M] drivers/gpu/drm/radeon/r300.o Signed-off-by: Masahiro Yamada --- drivers/gpu/drm/radeon/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/radeon/Makefile b/drivers/gpu/drm/radeon/Makefile index 480a8d4a3c82..11c97edde54d 100644 --- a/drivers/gpu/drm/radeon/Makefile +++ b/drivers/gpu/drm/radeon/Makefile @@ -6,7 +6,7 @@ hostprogs := mkregtable targets := rn50_reg_safe.h r100_reg_safe.h r200_reg_safe.h rv515_reg_safe.h r300_reg_safe.h r420_reg_safe.h rs600_reg_safe.h r600_reg_safe.h evergreen_reg_safe.h cayman_reg_safe.h -quiet_cmd_mkregtable = MKREGTABLE $@ +quiet_cmd_mkregtable = MKREG $@ cmd_mkregtable = $(obj)/mkregtable $< > $@ $(obj)/%_reg_safe.h: $(src)/reg_srcs/% $(obj)/mkregtable FORCE -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/4] drm/radeon: use pattern rule to avoid code duplication in Makefile
This Makefile repeats similar build rules. Use a pattern rule. Signed-off-by: Masahiro Yamada --- drivers/gpu/drm/radeon/Makefile | 29 + 1 file changed, 1 insertion(+), 28 deletions(-) diff --git a/drivers/gpu/drm/radeon/Makefile b/drivers/gpu/drm/radeon/Makefile index fda115cefe4d..480a8d4a3c82 100644 --- a/drivers/gpu/drm/radeon/Makefile +++ b/drivers/gpu/drm/radeon/Makefile @@ -9,34 +9,7 @@ targets := rn50_reg_safe.h r100_reg_safe.h r200_reg_safe.h rv515_reg_safe.h r300 quiet_cmd_mkregtable = MKREGTABLE $@ cmd_mkregtable = $(obj)/mkregtable $< > $@ -$(obj)/rn50_reg_safe.h: $(src)/reg_srcs/rn50 $(obj)/mkregtable FORCE - $(call if_changed,mkregtable) - -$(obj)/r100_reg_safe.h: $(src)/reg_srcs/r100 $(obj)/mkregtable FORCE - $(call if_changed,mkregtable) - -$(obj)/r200_reg_safe.h: $(src)/reg_srcs/r200 $(obj)/mkregtable FORCE - $(call if_changed,mkregtable) - -$(obj)/rv515_reg_safe.h: $(src)/reg_srcs/rv515 $(obj)/mkregtable FORCE - $(call if_changed,mkregtable) - -$(obj)/r300_reg_safe.h: $(src)/reg_srcs/r300 $(obj)/mkregtable FORCE - $(call if_changed,mkregtable) - -$(obj)/r420_reg_safe.h: $(src)/reg_srcs/r420 $(obj)/mkregtable FORCE - $(call if_changed,mkregtable) - -$(obj)/rs600_reg_safe.h: $(src)/reg_srcs/rs600 $(obj)/mkregtable FORCE - $(call if_changed,mkregtable) - -$(obj)/r600_reg_safe.h: $(src)/reg_srcs/r600 $(obj)/mkregtable FORCE - $(call if_changed,mkregtable) - -$(obj)/evergreen_reg_safe.h: $(src)/reg_srcs/evergreen $(obj)/mkregtable FORCE - $(call if_changed,mkregtable) - -$(obj)/cayman_reg_safe.h: $(src)/reg_srcs/cayman $(obj)/mkregtable FORCE +$(obj)/%_reg_safe.h: $(src)/reg_srcs/% $(obj)/mkregtable FORCE $(call if_changed,mkregtable) $(obj)/r100.o: $(obj)/r100_reg_safe.h $(obj)/rn50_reg_safe.h -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/4] drm/radeon: fix build rules of *_reg_safe.h
if_changed must have FORCE as a prerequisite, and the targets must be added to 'targets'. Signed-off-by: Masahiro Yamada --- drivers/gpu/drm/radeon/Makefile | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/radeon/Makefile b/drivers/gpu/drm/radeon/Makefile index 9d5d3dc1011f..fda115cefe4d 100644 --- a/drivers/gpu/drm/radeon/Makefile +++ b/drivers/gpu/drm/radeon/Makefile @@ -4,39 +4,39 @@ # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher. hostprogs := mkregtable -clean-files := rn50_reg_safe.h r100_reg_safe.h r200_reg_safe.h rv515_reg_safe.h r300_reg_safe.h r420_reg_safe.h rs600_reg_safe.h r600_reg_safe.h evergreen_reg_safe.h cayman_reg_safe.h +targets := rn50_reg_safe.h r100_reg_safe.h r200_reg_safe.h rv515_reg_safe.h r300_reg_safe.h r420_reg_safe.h rs600_reg_safe.h r600_reg_safe.h evergreen_reg_safe.h cayman_reg_safe.h quiet_cmd_mkregtable = MKREGTABLE $@ cmd_mkregtable = $(obj)/mkregtable $< > $@ -$(obj)/rn50_reg_safe.h: $(src)/reg_srcs/rn50 $(obj)/mkregtable +$(obj)/rn50_reg_safe.h: $(src)/reg_srcs/rn50 $(obj)/mkregtable FORCE $(call if_changed,mkregtable) -$(obj)/r100_reg_safe.h: $(src)/reg_srcs/r100 $(obj)/mkregtable +$(obj)/r100_reg_safe.h: $(src)/reg_srcs/r100 $(obj)/mkregtable FORCE $(call if_changed,mkregtable) -$(obj)/r200_reg_safe.h: $(src)/reg_srcs/r200 $(obj)/mkregtable +$(obj)/r200_reg_safe.h: $(src)/reg_srcs/r200 $(obj)/mkregtable FORCE $(call if_changed,mkregtable) -$(obj)/rv515_reg_safe.h: $(src)/reg_srcs/rv515 $(obj)/mkregtable +$(obj)/rv515_reg_safe.h: $(src)/reg_srcs/rv515 $(obj)/mkregtable FORCE $(call if_changed,mkregtable) -$(obj)/r300_reg_safe.h: $(src)/reg_srcs/r300 $(obj)/mkregtable +$(obj)/r300_reg_safe.h: $(src)/reg_srcs/r300 $(obj)/mkregtable FORCE $(call if_changed,mkregtable) -$(obj)/r420_reg_safe.h: $(src)/reg_srcs/r420 $(obj)/mkregtable +$(obj)/r420_reg_safe.h: $(src)/reg_srcs/r420 $(obj)/mkregtable FORCE $(call if_changed,mkregtable) -$(obj)/rs600_reg_safe.h: $(src)/reg_srcs/rs600 $(obj)/mkregtable +$(obj)/rs600_reg_safe.h: $(src)/reg_srcs/rs600 $(obj)/mkregtable FORCE $(call if_changed,mkregtable) -$(obj)/r600_reg_safe.h: $(src)/reg_srcs/r600 $(obj)/mkregtable +$(obj)/r600_reg_safe.h: $(src)/reg_srcs/r600 $(obj)/mkregtable FORCE $(call if_changed,mkregtable) -$(obj)/evergreen_reg_safe.h: $(src)/reg_srcs/evergreen $(obj)/mkregtable +$(obj)/evergreen_reg_safe.h: $(src)/reg_srcs/evergreen $(obj)/mkregtable FORCE $(call if_changed,mkregtable) -$(obj)/cayman_reg_safe.h: $(src)/reg_srcs/cayman $(obj)/mkregtable +$(obj)/cayman_reg_safe.h: $(src)/reg_srcs/cayman $(obj)/mkregtable FORCE $(call if_changed,mkregtable) $(obj)/r100.o: $(obj)/r100_reg_safe.h $(obj)/rn50_reg_safe.h -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 5/6] drm/qxl: don't use ttm bo->offset
On 2/13/20 3:30 PM, Gerd Hoffmann wrote: @@ -311,10 +311,8 @@ qxl_bo_physical_address(struct qxl_device *qdev, struct qxl_bo *bo, (bo->tbo.mem.mem_type == TTM_PL_VRAM) ? >main_slot : >surfaces_slot; - WARN_ON_ONCE((bo->tbo.offset & slot->gpu_offset) != slot->gpu_offset); - - /* TODO - need to hold one of the locks to read tbo.offset */ - return slot->high_bits | (bo->tbo.offset - slot->gpu_offset + offset); + return slot->high_bits | ((bo->tbo.mem.start << PAGE_SHIFT) + + slot->gpu_offset + offset); } --verbose please. I don't get the logic behind this change. Hi Gerd, I borrowed the logic for removed ttm part diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 229205e499db..2ccfebc3c9a2 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -382,12 +381,6 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, bo->evicted = false; } - if (bo->mem.mm_node) - bo->offset = (bo->mem.start << PAGE_SHIFT) + - bdev->man[bo->mem.mem_type].gpu_offset; - else - bo->offset = 0; - My assumption is (bo->tbo.offset - slot->gpu_offset + offset) == (bo->tbo.mem.start << PAGE_SHIFT) + bdev->man[bo->mem.mem_type].gpu_offset - slot->gpu_offset + offset) -> == (bo->tbo.mem.start << PAGE_SHIFT) + offset and we loose slot->gpu_offset so I thought it should be ((bo->tbo.mem.start << PAGE_SHIFT) + slot->gpu_offset + offset); Can you please suggest me how to calculate the offset correctly here. Regards, Nirmo The other chunks look sane, calculating slot->gpu_offset in setup_slot() certainly makes sense. cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/pl111: Support Integrator IM-PD1 module
On Thu, Feb 13, 2020 at 1:48 PM Linus Walleij wrote: > The last in-kernel user of the old framebuffer driver is the > IM-PD1 module for the Integrator/AP. Let's implement support for > this remaining user so we can migrate the last user over to > DRM and delete the old FB driver. > > On the Integrator/AP the IM-PD1 system controller will exist > alongside the common Integrator system controller so make > sure to do a special lookup for the IM-PD1 syscon and make it > take precedence if found. > > Tested on the Integrator/AP with the IM-PD1 mounted. > > Signed-off-by: Linus Walleij Looking around in the arm/mach-integrator code this seems to match roughly :-) I noticed there's also two more outputs for two panels, but not here. Do we not care about these anymore? Anyway, lgtm. Acked-by: Daniel Vetter > --- > drivers/gpu/drm/pl111/pl111_versatile.c | 73 + > 1 file changed, 73 insertions(+) > > diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c > b/drivers/gpu/drm/pl111/pl111_versatile.c > index 09aeaffb7660..4f325c410b5d 100644 > --- a/drivers/gpu/drm/pl111/pl111_versatile.c > +++ b/drivers/gpu/drm/pl111/pl111_versatile.c > @@ -19,6 +19,7 @@ static struct regmap *versatile_syscon_map; > * We detect the different syscon types from the compatible strings. > */ > enum versatile_clcd { > + INTEGRATOR_IMPD1, > INTEGRATOR_CLCD_CM, > VERSATILE_CLCD, > REALVIEW_CLCD_EB, > @@ -65,6 +66,14 @@ static const struct of_device_id versatile_clcd_of_match[] > = { > {}, > }; > > +static const struct of_device_id impd1_clcd_of_match[] = { > + { > + .compatible = "arm,im-pd1-syscon", > + .data = (void *)INTEGRATOR_IMPD1, > + }, > + {}, > +}; > + > /* > * Core module CLCD control on the Integrator/CP, bits > * 8 thru 19 of the CM_CONTROL register controls a bunch > @@ -125,6 +134,36 @@ static void pl111_integrator_enable(struct drm_device > *drm, u32 format) >val); > } > > +#define IMPD1_CTRL_OFFSET 0x18 > +#define IMPD1_CTRL_DISP_LCD(0 << 0) > +#define IMPD1_CTRL_DISP_VGA(1 << 0) > +#define IMPD1_CTRL_DISP_LCD1 (2 << 0) > +#define IMPD1_CTRL_DISP_ENABLE (1 << 2) > +#define IMPD1_CTRL_DISP_MASK (7 << 0) > + > +static void pl111_impd1_enable(struct drm_device *drm, u32 format) > +{ > + u32 val; > + > + dev_info(drm->dev, "enable IM-PD1 CLCD connectors\n"); > + val = IMPD1_CTRL_DISP_VGA | IMPD1_CTRL_DISP_ENABLE; > + > + regmap_update_bits(versatile_syscon_map, > + IMPD1_CTRL_OFFSET, > + IMPD1_CTRL_DISP_MASK, > + val); > +} > + > +static void pl111_impd1_disable(struct drm_device *drm) > +{ > + dev_info(drm->dev, "disable IM-PD1 CLCD connectors\n"); > + > + regmap_update_bits(versatile_syscon_map, > + IMPD1_CTRL_OFFSET, > + IMPD1_CTRL_DISP_MASK, > + 0); > +} > + > /* > * This configuration register in the Versatile and RealView > * family is uniformly present but appears more and more > @@ -270,6 +309,20 @@ static const struct pl111_variant_data pl110_integrator > = { > .fb_bpp = 16, > }; > > +/* > + * The IM-PD1 variant is a PL110 with a bunch of broken, or not > + * yet implemented features > + */ > +static const struct pl111_variant_data pl110_impd1 = { > + .name = "PL110 IM-PD1", > + .is_pl110 = true, > + .broken_clockdivider = true, > + .broken_vblank = true, > + .formats = pl110_integrator_pixel_formats, > + .nformats = ARRAY_SIZE(pl110_integrator_pixel_formats), > + .fb_bpp = 16, > +}; > + > /* > * This is the in-between PL110 variant found in the ARM Versatile, > * supporting RGB565/BGR565 > @@ -322,8 +375,21 @@ int pl111_versatile_init(struct device *dev, struct > pl111_drm_dev_private *priv) > /* Non-ARM reference designs, just bail out */ > return 0; > } > + > versatile_clcd_type = (enum versatile_clcd)clcd_id->data; > > + /* > +* On the Integrator, check if we should use the IM-PD1 instead, > +* if we find it, it will take precedence. This is on the > Integrator/AP > +* which only has this option for PL110 graphics. > +*/ > +if (versatile_clcd_type == INTEGRATOR_CLCD_CM) { > + np = of_find_matching_node_and_match(NULL, > impd1_clcd_of_match, > +_id); > + if (np) > + versatile_clcd_type = (enum > versatile_clcd)clcd_id->data; > + } > + > /* Versatile Express special handling */ > if (versatile_clcd_type == VEXPRESS_CLCD_V2M) { > struct platform_device *pdev; > @@ -367,6 +433,13 @@ int pl111_versatile_init(struct device *dev, struct > pl111_drm_dev_private *priv) >
Re: [Intel-gfx] [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare
On Wed, 12 Feb 2020, Michel Dänzer wrote: > On 2020-02-12 6:07 p.m., Nathan Chancellor wrote: >> On Wed, Feb 12, 2020 at 09:52:52AM +0100, Michel Dänzer wrote: >>> On 2020-02-11 9:39 p.m., Nathan Chancellor wrote: On Tue, Feb 11, 2020 at 10:41:48AM +0100, Michel Dänzer wrote: > On 2020-02-11 7:13 a.m., Nathan Chancellor wrote: >> A recent commit in clang added -Wtautological-compare to -Wall, which is >> enabled for i915 so we see the following warning: >> >> ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1485:22: warning: >> result of comparison of constant 576460752303423487 with expression of >> type 'unsigned int' is always false >> [-Wtautological-constant-out-of-range-compare] >> if (unlikely(remain > N_RELOC(ULONG_MAX))) >> ^ >> >> This warning only happens on x86_64 but that check is relevant for >> 32-bit x86 so we cannot remove it. > > That's suprising. AFAICT N_RELOC(ULONG_MAX) works out to the same value > in both cases, and remain is a 32-bit value in both cases. How can it be > larger than N_RELOC(ULONG_MAX) on 32-bit (but not on 64-bit)? > Hi Michel, Can't this condition be true when UINT_MAX == ULONG_MAX? >>> >>> Oh, right, I think I was wrongly thinking long had 64 bits even on 32-bit. >>> >>> >>> Anyway, this suggests a possible better solution: >>> >>> #if UINT_MAX == ULONG_MAX >>> if (unlikely(remain > N_RELOC(ULONG_MAX))) >>> return -EINVAL; >>> #endif >>> >>> >>> Or if that can't be used for some reason, something like >>> >>> if (unlikely((unsigned long)remain > N_RELOC(ULONG_MAX))) >>> return -EINVAL; >>> >>> should silence the warning. >> >> I do like this one better than the former. > > FWIW, one downside of this one compared to all alternatives (presumably) > is that it might end up generating actual code even on 64-bit, which > always ends up skipping the return. I like this better than the UINT_MAX == ULONG_MAX comparison because that creates a dependency on the type of remain. Then again, a sufficiently clever compiler could see through the cast, and flag the warning anyway... BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 0/6] do not store GPU address in TTM
On Thu, Feb 13, 2020 at 01:01:57PM +0100, Nirmoy Das wrote: > With this patch series I am trying to remove GPU address dependency in > TTM and moving GPU address calculation to individual drm drivers. > This is required[1] to continue the work started by Brian Welty to create > struct drm_mem_region which can be leverage by DRM cgroup controller to > manage memory > limits. > > > I have only manage to test amdgpu driver as I only have GPU for that. > I might be doing something really stupid while calculeting gpu offset for > some of the drivers so please be patient and let me know how can I improve > that. > > [1] > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.mail-archive.com%2Fdri-devel%40lists.freedesktop.org%2Fmsg272238.htmldata=02%7C01%7Cray.huang%40amd.com%7Cad8c8464b13e4764556008d7b07c3344%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637171919805856109sdata=zlA%2FHePGKcILKg7Ezc9CGc%2FWXJkRa5xmrBznvJcAomk%3Dreserved=0 Looks good for me as well for amd part. Acked-by: Huang Rui > > Nirmoy Das (6): > drm/amdgpu: move ttm bo->offset to amdgpu_bo > drm/radeon: don't use ttm bo->offset > drm/vmwgfx: don't use ttm bo->offset > drm/nouveau: don't use ttm bo->offset > drm/qxl: don't use ttm bo->offset > drm/ttm: do not keep GPU dependent addresses > > drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 22 ++-- > drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 + > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 29 - > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 + > drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 4 +-- > drivers/gpu/drm/nouveau/dispnv04/crtc.c | 6 ++--- > drivers/gpu/drm/nouveau/dispnv04/disp.c | 2 +- > drivers/gpu/drm/nouveau/dispnv04/overlay.c | 6 ++--- > drivers/gpu/drm/nouveau/dispnv50/base507c.c | 2 +- > drivers/gpu/drm/nouveau/dispnv50/core507d.c | 2 +- > drivers/gpu/drm/nouveau/dispnv50/ovly507e.c | 2 +- > drivers/gpu/drm/nouveau/dispnv50/wndw.c | 2 +- > drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c | 2 +- > drivers/gpu/drm/nouveau/nouveau_abi16.c | 8 +++--- > drivers/gpu/drm/nouveau/nouveau_bo.c| 1 + > drivers/gpu/drm/nouveau/nouveau_bo.h| 3 +++ > drivers/gpu/drm/nouveau/nouveau_chan.c | 2 +- > drivers/gpu/drm/nouveau/nouveau_fbcon.c | 2 +- > drivers/gpu/drm/nouveau/nouveau_gem.c | 10 +++ > drivers/gpu/drm/qxl/qxl_drv.h | 6 ++--- > drivers/gpu/drm/qxl/qxl_kms.c | 3 +++ > drivers/gpu/drm/qxl/qxl_object.h| 5 > drivers/gpu/drm/qxl/qxl_ttm.c | 9 --- > drivers/gpu/drm/radeon/radeon.h | 1 + > drivers/gpu/drm/radeon/radeon_object.h | 16 +++- > drivers/gpu/drm/radeon/radeon_ttm.c | 4 +-- > drivers/gpu/drm/ttm/ttm_bo.c| 7 - > drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 4 +-- > drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 2 +- > drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c| 2 +- > drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 -- > include/drm/ttm/ttm_bo_api.h| 2 -- > include/drm/ttm/ttm_bo_driver.h | 1 - > 33 files changed, 99 insertions(+), 72 deletions(-) > > -- > 2.25.0 > > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7Cray.huang%40amd.com%7Cad8c8464b13e4764556008d7b07c3344%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637171919805856109sdata=lnJkwlCEbUmtsBhBY94rB3hRgaYg4ENQ0DNTXxxwPL4%3Dreserved=0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 14/15] drm/amdgpu/ring: move debugfs init into core amdgpu debugfs
On Thu, Feb 13, 2020 at 5:17 AM Christian König wrote: > > Am 13.02.20 um 10:54 schrieb Daniel Vetter: > > On Fri, Feb 07, 2020 at 02:50:57PM -0500, Alex Deucher wrote: > >> In order to remove the load and unload drm callbacks, > >> we need to reorder the init sequence to move all the drm > >> debugfs file handling. Do this for rings. > >> > >> Acked-by: Christian König > >> Signed-off-by: Alex Deucher > >> --- > >> drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 23 - > >> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c| 15 +++--- > >> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h| 4 > >> 3 files changed, 29 insertions(+), 13 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > >> index df3919ef886b..7379910790c9 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > >> @@ -1218,7 +1218,7 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_ib_preempt, NULL, > >> > >> int amdgpu_debugfs_init(struct amdgpu_device *adev) > >> { > >> -int r; > >> +int r, i; > >> > >> adev->debugfs_preempt = > >> debugfs_create_file("amdgpu_preempt_ib", 0600, > >> @@ -1268,12 +1268,33 @@ int amdgpu_debugfs_init(struct amdgpu_device *adev) > >> } > >> #endif > >> > >> +for (i = 0; i < AMDGPU_MAX_RINGS; ++i) { > >> +struct amdgpu_ring *ring = adev->rings[i]; > >> + > >> +if (!ring) > >> +continue; > >> + > >> +if (amdgpu_debugfs_ring_init(adev, ring)) { > >> +DRM_ERROR("Failed to register debugfs file for rings > >> !\n"); > >> +} > >> +} > >> + > >> return amdgpu_debugfs_add_files(adev, amdgpu_debugfs_list, > >> ARRAY_SIZE(amdgpu_debugfs_list)); > >> } > >> > >> void amdgpu_debugfs_fini(struct amdgpu_device *adev) > > btw debugfs_fini shouldn't be needed anymore, Greg KH removed this all. > > drm core removes all debugfs files recusrively for you, there should be 0 > > need for debugfs cleanup. > > Oh, yes please. Removing that was on my TODO list for an eternity as well. > > > > > Also at least my tree doesn't even have this, where does this apply to? > > I would guess amd-staging-drm-next, but it might be that Alex is waiting > for the next rebase to land. > Patches are against my drm-next branch which is based on fdo drm-next. There are a number of files which the driver creates directly rather than through drm. Alex > Christian. > > > -Daniel > > > >> { > >> +int i; > >> + > >> +for (i = 0; i < AMDGPU_MAX_RINGS; ++i) { > >> +struct amdgpu_ring *ring = adev->rings[i]; > >> + > >> +if (!ring) > >> +continue; > >> + > >> +amdgpu_debugfs_ring_fini(ring); > >> +} > >> amdgpu_ttm_debugfs_fini(adev); > >> debugfs_remove(adev->debugfs_preempt); > >> } > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c > >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c > >> index e5c83e164d82..539be138260e 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c > >> @@ -48,9 +48,6 @@ > >>* wptr. The GPU then starts fetching commands and executes > >>* them until the pointers are equal again. > >>*/ > >> -static int amdgpu_debugfs_ring_init(struct amdgpu_device *adev, > >> -struct amdgpu_ring *ring); > >> -static void amdgpu_debugfs_ring_fini(struct amdgpu_ring *ring); > >> > >> /** > >>* amdgpu_ring_alloc - allocate space on the ring buffer > >> @@ -334,10 +331,6 @@ int amdgpu_ring_init(struct amdgpu_device *adev, > >> struct amdgpu_ring *ring, > >> for (i = 0; i < DRM_SCHED_PRIORITY_MAX; ++i) > >> atomic_set(>num_jobs[i], 0); > >> > >> -if (amdgpu_debugfs_ring_init(adev, ring)) { > >> -DRM_ERROR("Failed to register debugfs file for rings !\n"); > >> -} > >> - > >> return 0; > >> } > >> > >> @@ -367,8 +360,6 @@ void amdgpu_ring_fini(struct amdgpu_ring *ring) > >>>gpu_addr, > >>(void **)>ring); > >> > >> -amdgpu_debugfs_ring_fini(ring); > >> - > >> dma_fence_put(ring->vmid_wait); > >> ring->vmid_wait = NULL; > >> ring->me = 0; > >> @@ -485,8 +476,8 @@ static const struct file_operations > >> amdgpu_debugfs_ring_fops = { > >> > >> #endif > >> > >> -static int amdgpu_debugfs_ring_init(struct amdgpu_device *adev, > >> -struct amdgpu_ring *ring) > >> +int amdgpu_debugfs_ring_init(struct amdgpu_device *adev, > >> + struct amdgpu_ring *ring) > >> { > >> #if defined(CONFIG_DEBUG_FS) > >> struct drm_minor *minor = adev->ddev->primary; > >> @@ -507,7 +498,7 @@ static int amdgpu_debugfs_ring_init(struct > >> amdgpu_device *adev, > >>
Re: [RFC PATCH 5/6] drm/qxl: don't use ttm bo->offset
> @@ -311,10 +311,8 @@ qxl_bo_physical_address(struct qxl_device *qdev, struct > qxl_bo *bo, > (bo->tbo.mem.mem_type == TTM_PL_VRAM) > ? >main_slot : >surfaces_slot; > > - WARN_ON_ONCE((bo->tbo.offset & slot->gpu_offset) != slot->gpu_offset); > - > - /* TODO - need to hold one of the locks to read tbo.offset */ > - return slot->high_bits | (bo->tbo.offset - slot->gpu_offset + offset); > + return slot->high_bits | ((bo->tbo.mem.start << PAGE_SHIFT) + > + slot->gpu_offset + offset); > } --verbose please. I don't get the logic behind this change. The other chunks look sane, calculating slot->gpu_offset in setup_slot() certainly makes sense. cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/mediatek: add fb swap in async_update
Hi, Missatge de CK Hu del dia dj., 13 de febr. 2020 a les 5:06: > > Hi, Bibby: > > On Thu, 2020-02-13 at 09:23 +0800, Bibby Hsieh wrote: > > Besides x, y position, width and height, > > fb also need updating in async update. > > > > Reviewed-by: CK Hu > > > Fixes: 920fffcc8912 ("drm/mediatek: update cursors by using async atomic > > update") > > > > Signed-off-by: Bibby Hsieh > > --- This patch actually fixes two issues as explained in [1], I send the patch without seeing that another one was already sent. Both do the same thing. So, Tested-by: Enric Balletbo i Serra [1] https://lkml.org/lkml/2020/2/13/286 > > drivers/gpu/drm/mediatek/mtk_drm_plane.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c > > b/drivers/gpu/drm/mediatek/mtk_drm_plane.c > > index d32b494ff1de..e084c36fdd8a 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c > > @@ -122,6 +122,7 @@ static void mtk_plane_atomic_async_update(struct > > drm_plane *plane, > > plane->state->src_y = new_state->src_y; > > plane->state->src_h = new_state->src_h; > > plane->state->src_w = new_state->src_w; > > + swap(plane->state->fb, new_state->fb); > > state->pending.async_dirty = true; > > > > mtk_drm_crtc_async_update(new_state->crtc, plane, new_state); > > -- > CK Hu > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 2/6] drm/radeon: don't use ttm bo->offset
On 2/13/20 2:00 PM, Christian König wrote: Cool, than that is a lot easier to implement than I thought it would be. I assume you don't have Radeon hardware lying around to test this? Yes this would be nice I don't have a Radeon hw with me. If you can you give me a branch on the AMD (or public) servers I can give it at least a quick round of testing. Thanks, Nirmoy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 2/6] drm/radeon: don't use ttm bo->offset
On 2/13/20 2:00 PM, Christian König wrote: Am 13.02.20 um 13:31 schrieb Nirmoy: On 2/13/20 1:18 PM, Christian König wrote: Looks like most of the patch is missing? We should have quite a number of uses of the BO offset in radeon or do all of those go through radeon_bo_gpu_offset? Compilation worked so I think all those(bo->offset) accesses went through radeon_bo_gpu_offset. Cool, than that is a lot easier to implement than I thought it would be. I assume you don't have Radeon hardware lying around to test this? If you can you give me a branch on the AMD (or public) servers I can give it at least a quick round of testing. huh it seems I have to rebase it a bit for drm/drm-next as it is based on our internal branch :/ You can access the branch at http://gitlab1.amd.com/nirmodas/linux/tree/ttm_clean.1 I will rebase this for drm-next in next version of the patch series. Christian. If yes then the change is much smaller than I thought i needs to be. Christian. Regards, Nirmoy ___ amd-gfx mailing list amd-...@lists.freedesktop.org https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7CNirmoy.Das%40amd.com%7C60aa17c05a2f474663f008d7b084b550%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637171956342728389sdata=I2n0yMXK5CtrG5tY9Q47igxJv9Onb%2FA7PBeLwrpPb54%3Dreserved=0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 3/4] drm/virtio: batch resource creation
Move virtio_gpu_notify() to higher-level functions for virtio_gpu_cmd_create_resource(), virtio_gpu_cmd_resource_create_3d() and virtio_gpu_cmd_resource_attach_backing(). virtio_gpu_object_create() will batch commands and notify only once when creating a resource. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_object.c | 1 + drivers/gpu/drm/virtio/virtgpu_vq.c | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c index 8870ee23ff2b..65d6834d3c74 100644 --- a/drivers/gpu/drm/virtio/virtgpu_object.c +++ b/drivers/gpu/drm/virtio/virtgpu_object.c @@ -224,6 +224,7 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, return ret; } + virtio_gpu_notify(vgdev); *bo_ptr = bo; return 0; diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index 9d4ca0fafa5f..778b7acf2f7f 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -514,7 +514,6 @@ void virtio_gpu_cmd_create_resource(struct virtio_gpu_device *vgdev, cmd_p->height = cpu_to_le32(params->height); virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, fence); - virtio_gpu_notify(vgdev); bo->created = true; } @@ -643,7 +642,6 @@ virtio_gpu_cmd_resource_attach_backing(struct virtio_gpu_device *vgdev, vbuf->data_size = sizeof(*ents) * nents; virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, fence); - virtio_gpu_notify(vgdev); } static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev, @@ -1010,7 +1008,6 @@ virtio_gpu_cmd_resource_create_3d(struct virtio_gpu_device *vgdev, cmd_p->flags = cpu_to_le32(params->flags); virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, fence); - virtio_gpu_notify(vgdev); bo->created = true; } -- 2.18.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 2/4] drm/virtio: batch plane updates (pageflip)
Move virtio_gpu_notify() to higher-level functions for virtio_gpu_cmd_resource_flush(), virtio_gpu_cmd_set_scanout() and virtio_gpu_cmd_transfer_to_host_{2d,3d}(). virtio_gpu_primary_plane_update() will notify only once for a series of commands (restores plane update command batching). Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_display.c | 2 ++ drivers/gpu/drm/virtio/virtgpu_ioctl.c | 1 + drivers/gpu/drm/virtio/virtgpu_plane.c | 3 +++ drivers/gpu/drm/virtio/virtgpu_vq.c | 4 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c index af953db4a0c9..2b7e6ae65546 100644 --- a/drivers/gpu/drm/virtio/virtgpu_display.c +++ b/drivers/gpu/drm/virtio/virtgpu_display.c @@ -90,6 +90,7 @@ static void virtio_gpu_crtc_mode_set_nofb(struct drm_crtc *crtc) virtio_gpu_cmd_set_scanout(vgdev, output->index, 0, crtc->mode.hdisplay, crtc->mode.vdisplay, 0, 0); + virtio_gpu_notify(vgdev); } static void virtio_gpu_crtc_atomic_enable(struct drm_crtc *crtc, @@ -108,6 +109,7 @@ static void virtio_gpu_crtc_atomic_disable(struct drm_crtc *crtc, struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc); virtio_gpu_cmd_set_scanout(vgdev, output->index, 0, 0, 0, 0, 0); + virtio_gpu_notify(vgdev); output->enabled = false; } diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 0477d1250f2d..467649733d24 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -359,6 +359,7 @@ static int virtio_gpu_transfer_to_host_ioctl(struct drm_device *dev, void *data, args->level, >box, objs, fence); dma_fence_put(>f); } + virtio_gpu_notify(vgdev); return 0; err_unlock: diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c index 08b2e4127eb3..52d24179bcec 100644 --- a/drivers/gpu/drm/virtio/virtgpu_plane.c +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c @@ -148,6 +148,7 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane, plane->state->src_w >> 16, plane->state->src_h >> 16, 0, 0); + virtio_gpu_notify(vgdev); return; } @@ -184,6 +185,7 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane, rect.y1, rect.x2 - rect.x1, rect.y2 - rect.y1); + virtio_gpu_notify(vgdev); } static int virtio_gpu_cursor_prepare_fb(struct drm_plane *plane, @@ -262,6 +264,7 @@ static void virtio_gpu_cursor_plane_update(struct drm_plane *plane, plane->state->crtc_w, plane->state->crtc_h, 0, 0, objs, vgfb->fence); + virtio_gpu_notify(vgdev); dma_fence_wait(>fence->f, true); dma_fence_put(>fence->f); vgfb->fence = NULL; diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index 812212975440..9d4ca0fafa5f 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -567,7 +567,6 @@ void virtio_gpu_cmd_set_scanout(struct virtio_gpu_device *vgdev, cmd_p->r.y = cpu_to_le32(y); virtio_gpu_queue_ctrl_buffer(vgdev, vbuf); - virtio_gpu_notify(vgdev); } void virtio_gpu_cmd_resource_flush(struct virtio_gpu_device *vgdev, @@ -589,7 +588,6 @@ void virtio_gpu_cmd_resource_flush(struct virtio_gpu_device *vgdev, cmd_p->r.y = cpu_to_le32(y); virtio_gpu_queue_ctrl_buffer(vgdev, vbuf); - virtio_gpu_notify(vgdev); } void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev, @@ -622,7 +620,6 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev, cmd_p->r.y = cpu_to_le32(y); virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, fence); - virtio_gpu_notify(vgdev); } static void @@ -1048,7 +1045,6 @@ void virtio_gpu_cmd_transfer_to_host_3d(struct virtio_gpu_device *vgdev, cmd_p->level = cpu_to_le32(level); virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, fence); - virtio_gpu_notify(vgdev); } void virtio_gpu_cmd_transfer_from_host_3d(struct virtio_gpu_device *vgdev, -- 2.18.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 4/4] drm/virtio: batch display query.
Move virtio_gpu_notify() to higher-level functions for virtio_gpu_cmd_get_display_info() and virtio_gpu_cmd_get_edids(). virtio_gpu_config_changed_work_func() and virtio_gpu_init() will batch commands and notify only once per update Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_kms.c | 2 ++ drivers/gpu/drm/virtio/virtgpu_vq.c | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c index 4009c2f97d08..8fd7acef960f 100644 --- a/drivers/gpu/drm/virtio/virtgpu_kms.c +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c @@ -44,6 +44,7 @@ static void virtio_gpu_config_changed_work_func(struct work_struct *work) if (vgdev->has_edid) virtio_gpu_cmd_get_edids(vgdev); virtio_gpu_cmd_get_display_info(vgdev); + virtio_gpu_notify(vgdev); drm_helper_hpd_irq_event(vgdev->ddev); events_clear |= VIRTIO_GPU_EVENT_DISPLAY; } @@ -205,6 +206,7 @@ int virtio_gpu_init(struct drm_device *dev) if (vgdev->has_edid) virtio_gpu_cmd_get_edids(vgdev); virtio_gpu_cmd_get_display_info(vgdev); + virtio_gpu_notify(vgdev); wait_event_timeout(vgdev->resp_wq, !vgdev->display_info_pending, 5 * HZ); return 0; diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index 778b7acf2f7f..f5740d7b67be 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -774,7 +774,6 @@ int virtio_gpu_cmd_get_display_info(struct virtio_gpu_device *vgdev) vgdev->display_info_pending = true; cmd_p->type = cpu_to_le32(VIRTIO_GPU_CMD_GET_DISPLAY_INFO); virtio_gpu_queue_ctrl_buffer(vgdev, vbuf); - virtio_gpu_notify(vgdev); return 0; } @@ -902,7 +901,6 @@ int virtio_gpu_cmd_get_edids(struct virtio_gpu_device *vgdev) cmd_p->hdr.type = cpu_to_le32(VIRTIO_GPU_CMD_GET_EDID); cmd_p->scanout = cpu_to_le32(scanout); virtio_gpu_queue_ctrl_buffer(vgdev, vbuf); - virtio_gpu_notify(vgdev); } return 0; -- 2.18.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 0/4] drm/virtio: rework batching
v4: - split into multiple patches. Gerd Hoffmann (4): drm/virtio: rework notification for better batching drm/virtio: batch plane updates (pageflip) drm/virtio: batch resource creation drm/virtio: batch display query. drivers/gpu/drm/virtio/virtgpu_drv.h | 6 ++-- drivers/gpu/drm/virtio/virtgpu_display.c | 2 ++ drivers/gpu/drm/virtio/virtgpu_ioctl.c | 1 + drivers/gpu/drm/virtio/virtgpu_kms.c | 2 ++ drivers/gpu/drm/virtio/virtgpu_object.c | 1 + drivers/gpu/drm/virtio/virtgpu_plane.c | 7 ++-- drivers/gpu/drm/virtio/virtgpu_vq.c | 41 +--- 7 files changed, 33 insertions(+), 27 deletions(-) -- 2.18.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/4] drm/virtio: rework notification for better batching
Drop the virtio_gpu_{disable,enable}_notify(). Add a new virtio_gpu_notify() call instead, which must be called whenever the driver wants make sure the host is notified needed. Drop automatic notification from command submission. Add virtio_gpu_notify() calls after each command query instead. This allows more fine-grained control over host notification and can move around the notify calls in subsequent patches to batch command submissions. With this in place it is also possible to make notification optional for userspace ioctls. Page flip batching goes away (temporarely). v3: - move batching to separate patches. v2: - rebase to latest drm-misc-next. - use "if (!atomic_read())". - add review & test tags. Signed-off-by: Gerd Hoffmann Reviewed-by: Gurchetan Singh Tested-by: Gurchetan Singh --- drivers/gpu/drm/virtio/virtgpu_drv.h | 6 ++-- drivers/gpu/drm/virtio/virtgpu_plane.c | 4 --- drivers/gpu/drm/virtio/virtgpu_vq.c| 50 -- 3 files changed, 33 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index af9403e1cf78..2f6c4ccbfd14 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -179,8 +179,7 @@ struct virtio_gpu_device { struct virtio_gpu_queue cursorq; struct kmem_cache *vbufs; - bool disable_notify; - bool pending_notify; + atomic_t pending_commands; struct ida resource_ida; @@ -335,8 +334,7 @@ void virtio_gpu_dequeue_ctrl_func(struct work_struct *work); void virtio_gpu_dequeue_cursor_func(struct work_struct *work); void virtio_gpu_dequeue_fence_func(struct work_struct *work); -void virtio_gpu_disable_notify(struct virtio_gpu_device *vgdev); -void virtio_gpu_enable_notify(struct virtio_gpu_device *vgdev); +void virtio_gpu_notify(struct virtio_gpu_device *vgdev); /* virtio_gpu_display.c */ void virtio_gpu_modeset_init(struct virtio_gpu_device *vgdev); diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c index d1c3f5fbfee4..08b2e4127eb3 100644 --- a/drivers/gpu/drm/virtio/virtgpu_plane.c +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c @@ -154,8 +154,6 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane, if (!drm_atomic_helper_damage_merged(old_state, plane->state, )) return; - virtio_gpu_disable_notify(vgdev); - bo = gem_to_virtio_gpu_obj(plane->state->fb->obj[0]); if (bo->dumb) virtio_gpu_update_dumb_bo(vgdev, plane->state, ); @@ -186,8 +184,6 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane, rect.y1, rect.x2 - rect.x1, rect.y2 - rect.y1); - - virtio_gpu_enable_notify(vgdev); } static int virtio_gpu_cursor_prepare_fb(struct drm_plane *plane, diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index cfe9c54f87a3..812212975440 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -329,7 +329,6 @@ static void virtio_gpu_queue_ctrl_sgs(struct virtio_gpu_device *vgdev, int incnt) { struct virtqueue *vq = vgdev->ctrlq.vq; - bool notify = false; int ret, idx; if (!drm_dev_enter(vgdev->ddev, )) { @@ -368,16 +367,10 @@ static void virtio_gpu_queue_ctrl_sgs(struct virtio_gpu_device *vgdev, trace_virtio_gpu_cmd_queue(vq, virtio_gpu_vbuf_ctrl_hdr(vbuf)); - notify = virtqueue_kick_prepare(vq); + atomic_inc(>pending_commands); spin_unlock(>ctrlq.qlock); - if (notify) { - if (vgdev->disable_notify) - vgdev->pending_notify = true; - else - virtqueue_notify(vq); - } drm_dev_exit(idx); } @@ -434,19 +427,20 @@ static void virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device *vgdev, } } -void virtio_gpu_disable_notify(struct virtio_gpu_device *vgdev) +void virtio_gpu_notify(struct virtio_gpu_device *vgdev) { - vgdev->disable_notify = true; -} + bool notify; -void virtio_gpu_enable_notify(struct virtio_gpu_device *vgdev) -{ - vgdev->disable_notify = false; - - if (!vgdev->pending_notify) + if (!atomic_read(>pending_commands)) return; - vgdev->pending_notify = false; - virtqueue_notify(vgdev->ctrlq.vq); + + spin_lock(>ctrlq.qlock); + atomic_set(>pending_commands, 0); + notify = virtqueue_kick_prepare(vgdev->ctrlq.vq); + spin_unlock(>ctrlq.qlock); + + if (notify) + virtqueue_notify(vgdev->ctrlq.vq); } static void virtio_gpu_queue_ctrl_buffer(struct virtio_gpu_device *vgdev, @@ -520,6 +514,7 @@ void virtio_gpu_cmd_create_resource(struct
Re: [RFC PATCH 2/6] drm/radeon: don't use ttm bo->offset
Am 13.02.20 um 13:31 schrieb Nirmoy: On 2/13/20 1:18 PM, Christian König wrote: Looks like most of the patch is missing? We should have quite a number of uses of the BO offset in radeon or do all of those go through radeon_bo_gpu_offset? Compilation worked so I think all those(bo->offset) accesses went through radeon_bo_gpu_offset. Cool, than that is a lot easier to implement than I thought it would be. I assume you don't have Radeon hardware lying around to test this? If you can you give me a branch on the AMD (or public) servers I can give it at least a quick round of testing. Christian. If yes then the change is much smaller than I thought i needs to be. Christian. Regards, Nirmoy ___ amd-gfx mailing list amd-...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH 1/2] drm/nouveau: move io_reserve_lru handling into the driver v2
Hi Ben, sorry for the delayed response. Haven been rather busy recently. Am 28.01.20 um 06:49 schrieb Ben Skeggs: On Sat, 25 Jan 2020 at 00:30, Christian König wrote: From: Christian König While working on TTM cleanups I've found that the io_reserve_lru used by Nouveau is actually not working at all. In general we should remove driver specific handling from the memory management, so this patch moves the io_reserve_lru handling into Nouveau instead. The patch should be functional correct, but is only compile tested! NACK on this as it currently stands. It not only causes invalid io accesses somehow on module load, but while attempting to track down why, I realised there's a more severe issue. This removes the distinction between kmap() and mapping into userspace, the former of which should not be placed onto the LRU as an eviction candidate. Yeah, I already feared that this won't work of hand. It's just a bit hard to write stuff without being able to test it. We *do* require the LRU, so it's not something that can just be dropped completely. There's a user report where they're getting a SIGBUS due to the bug you noticed causing it to not work right now. Well that you require an LRU is obvious. But since this is completely Nouveau specificthat handling doesn't needs to be and actually should not be in TTM. And as Daniel pointed out the current handling in the combination of TTM and Nouveau is fundamentally broken. Userspace can either always force other applications into a SIGBUS or the page faulting into a live lock. Both of that is rather bad. To outline how this could work correctly: 1. You split up your PCIe BAR into pages/segments of fixed size. I'm not sure what the NVidia hardware can handle, but I would use something like 2MB segments for that to match the x86 huge page size. 2. Whenever you get a page fault or a kmap request for a buffer you walk your LRU and grab a free segment. 3. kmap requests obviously are not put back on the LRU (or are skipped while grabbed). Segments for page fault requests are put on the LRU immediately again. The idea here is that you have at least enough segments (in this example 256 MB BAR / 2 MB segment size = 128 segments) so that you always have more segments than page faults can happen at the same time. I'm completely fine with keeping the code around for now, but as outlined above the whole handling is rather broken. Regards, Christian. Ben. v2: don't call ttm_bo_unmap_virtual in nouveau_ttm_io_mem_reserve Signed-off-by: Christian König Acked-by: Daniel Vetter --- drivers/gpu/drm/nouveau/nouveau_bo.c | 107 -- drivers/gpu/drm/nouveau/nouveau_bo.h | 3 + drivers/gpu/drm/nouveau/nouveau_drv.h | 2 + drivers/gpu/drm/nouveau/nouveau_ttm.c | 43 ++- 4 files changed, 131 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 81668104595f..acee054f77ed 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -137,6 +137,7 @@ nouveau_bo_del_ttm(struct ttm_buffer_object *bo) struct nouveau_bo *nvbo = nouveau_bo(bo); WARN_ON(nvbo->pin_refcnt > 0); + nouveau_bo_del_io_reserve_lru(bo); nv10_bo_put_tile_region(dev, nvbo->tile, NULL); /* @@ -304,6 +305,7 @@ nouveau_bo_init(struct nouveau_bo *nvbo, u64 size, int align, u32 flags, nvbo->bo.mem.num_pages = size >> PAGE_SHIFT; nouveau_bo_placement_set(nvbo, flags, 0); + INIT_LIST_HEAD(>io_reserve_lru); ret = ttm_bo_init(nvbo->bo.bdev, >bo, size, type, >placement, align >> PAGE_SHIFT, false, @@ -574,6 +576,26 @@ nouveau_bo_sync_for_cpu(struct nouveau_bo *nvbo) PAGE_SIZE, DMA_FROM_DEVICE); } +void nouveau_bo_add_io_reserve_lru(struct ttm_buffer_object *bo) +{ + struct nouveau_drm *drm = nouveau_bdev(bo->bdev); + struct nouveau_bo *nvbo = nouveau_bo(bo); + + mutex_lock(>ttm.io_reserve_mutex); + list_move_tail(>io_reserve_lru, >ttm.io_reserve_lru); + mutex_unlock(>ttm.io_reserve_mutex); +} + +void nouveau_bo_del_io_reserve_lru(struct ttm_buffer_object *bo) +{ + struct nouveau_drm *drm = nouveau_bdev(bo->bdev); + struct nouveau_bo *nvbo = nouveau_bo(bo); + + mutex_lock(>ttm.io_reserve_mutex); + list_del_init(>io_reserve_lru); + mutex_unlock(>ttm.io_reserve_mutex); +} + int nouveau_bo_validate(struct nouveau_bo *nvbo, bool interruptible, bool no_wait_gpu) @@ -675,8 +697,6 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type, } man->func = _vram_manager; - man->io_reserve_fastpath = false; - man->use_io_reserve_lru = true; } else { man->func =
[PATCH v4] drm/i915: dont retry stream management at seq_num_m roll over
When roll over detected for seq_num_m, we shouldn't continue with stream management with rolled over value. So we are terminating the stream management retry, on roll over of the seq_num_m. v2: using drm_dbg_kms instead of DRM_DEBUG_KMS [Anshuman] v3: dev_priv is used as i915 [JaniN] v4: roll over of seq_num_m detected at the start of stream management. Signed-off-by: Ramalingam C --- drivers/gpu/drm/i915/display/intel_hdcp.c | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index b24d12efae0a..f4b1dd1e62e7 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -1391,6 +1391,13 @@ int _hdcp2_propagate_stream_management_info(struct intel_connector *connector) const struct intel_hdcp_shim *shim = hdcp->shim; int ret; + /* +* seq_num_m roll over is possible only when dynamic update of +* content type is supported. But implementing as per the spec. +*/ + if (hdcp->seq_num_m > HDCP_2_2_SEQ_NUM_MAX) + return -1; + /* Prepare RepeaterAuth_Stream_Manage msg */ msgs.stream_manage.msg_id = HDCP_2_2_REP_STREAM_MANAGE; drm_hdcp_cpu_to_be24(msgs.stream_manage.seq_num_m, hdcp->seq_num_m); @@ -1419,11 +1426,6 @@ int _hdcp2_propagate_stream_management_info(struct intel_connector *connector) err_exit: hdcp->seq_num_m++; - if (hdcp->seq_num_m > HDCP_2_2_SEQ_NUM_MAX) { - DRM_DEBUG_KMS("seq_num_m roll over.\n"); - ret = -1; - } - return ret; } @@ -1618,8 +1620,11 @@ hdcp2_propagate_stream_management_info(struct intel_connector *connector) for (i = 0; i < tries; i++) { ret = _hdcp2_propagate_stream_management_info(connector); - if (!ret) + if (!ret || connector->hdcp.seq_num_m > HDCP_2_2_SEQ_NUM_MAX) { + if (connector->hdcp.seq_num_m > HDCP_2_2_SEQ_NUM_MAX) + drm_dbg_kms(>drm, "seq_num_m roll over.\n"); break; + } drm_dbg_kms(>drm, "HDCP2 stream management %d of %d Failed.(%d)\n", -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/pl111: Support Integrator IM-PD1 module
The last in-kernel user of the old framebuffer driver is the IM-PD1 module for the Integrator/AP. Let's implement support for this remaining user so we can migrate the last user over to DRM and delete the old FB driver. On the Integrator/AP the IM-PD1 system controller will exist alongside the common Integrator system controller so make sure to do a special lookup for the IM-PD1 syscon and make it take precedence if found. Tested on the Integrator/AP with the IM-PD1 mounted. Signed-off-by: Linus Walleij --- drivers/gpu/drm/pl111/pl111_versatile.c | 73 + 1 file changed, 73 insertions(+) diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c index 09aeaffb7660..4f325c410b5d 100644 --- a/drivers/gpu/drm/pl111/pl111_versatile.c +++ b/drivers/gpu/drm/pl111/pl111_versatile.c @@ -19,6 +19,7 @@ static struct regmap *versatile_syscon_map; * We detect the different syscon types from the compatible strings. */ enum versatile_clcd { + INTEGRATOR_IMPD1, INTEGRATOR_CLCD_CM, VERSATILE_CLCD, REALVIEW_CLCD_EB, @@ -65,6 +66,14 @@ static const struct of_device_id versatile_clcd_of_match[] = { {}, }; +static const struct of_device_id impd1_clcd_of_match[] = { + { + .compatible = "arm,im-pd1-syscon", + .data = (void *)INTEGRATOR_IMPD1, + }, + {}, +}; + /* * Core module CLCD control on the Integrator/CP, bits * 8 thru 19 of the CM_CONTROL register controls a bunch @@ -125,6 +134,36 @@ static void pl111_integrator_enable(struct drm_device *drm, u32 format) val); } +#define IMPD1_CTRL_OFFSET 0x18 +#define IMPD1_CTRL_DISP_LCD(0 << 0) +#define IMPD1_CTRL_DISP_VGA(1 << 0) +#define IMPD1_CTRL_DISP_LCD1 (2 << 0) +#define IMPD1_CTRL_DISP_ENABLE (1 << 2) +#define IMPD1_CTRL_DISP_MASK (7 << 0) + +static void pl111_impd1_enable(struct drm_device *drm, u32 format) +{ + u32 val; + + dev_info(drm->dev, "enable IM-PD1 CLCD connectors\n"); + val = IMPD1_CTRL_DISP_VGA | IMPD1_CTRL_DISP_ENABLE; + + regmap_update_bits(versatile_syscon_map, + IMPD1_CTRL_OFFSET, + IMPD1_CTRL_DISP_MASK, + val); +} + +static void pl111_impd1_disable(struct drm_device *drm) +{ + dev_info(drm->dev, "disable IM-PD1 CLCD connectors\n"); + + regmap_update_bits(versatile_syscon_map, + IMPD1_CTRL_OFFSET, + IMPD1_CTRL_DISP_MASK, + 0); +} + /* * This configuration register in the Versatile and RealView * family is uniformly present but appears more and more @@ -270,6 +309,20 @@ static const struct pl111_variant_data pl110_integrator = { .fb_bpp = 16, }; +/* + * The IM-PD1 variant is a PL110 with a bunch of broken, or not + * yet implemented features + */ +static const struct pl111_variant_data pl110_impd1 = { + .name = "PL110 IM-PD1", + .is_pl110 = true, + .broken_clockdivider = true, + .broken_vblank = true, + .formats = pl110_integrator_pixel_formats, + .nformats = ARRAY_SIZE(pl110_integrator_pixel_formats), + .fb_bpp = 16, +}; + /* * This is the in-between PL110 variant found in the ARM Versatile, * supporting RGB565/BGR565 @@ -322,8 +375,21 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv) /* Non-ARM reference designs, just bail out */ return 0; } + versatile_clcd_type = (enum versatile_clcd)clcd_id->data; + /* +* On the Integrator, check if we should use the IM-PD1 instead, +* if we find it, it will take precedence. This is on the Integrator/AP +* which only has this option for PL110 graphics. +*/ +if (versatile_clcd_type == INTEGRATOR_CLCD_CM) { + np = of_find_matching_node_and_match(NULL, impd1_clcd_of_match, +_id); + if (np) + versatile_clcd_type = (enum versatile_clcd)clcd_id->data; + } + /* Versatile Express special handling */ if (versatile_clcd_type == VEXPRESS_CLCD_V2M) { struct platform_device *pdev; @@ -367,6 +433,13 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv) priv->variant_display_enable = pl111_integrator_enable; dev_info(dev, "set up callbacks for Integrator PL110\n"); break; + case INTEGRATOR_IMPD1: + versatile_syscon_map = map; + priv->variant = _impd1; + priv->variant_display_enable = pl111_impd1_enable; + priv->variant_display_disable = pl111_impd1_disable; + dev_info(dev, "set up callbacks for IM-PD1 PL110\n"); + break; case VERSATILE_CLCD:
Re: [RFC PATCH 2/6] drm/radeon: don't use ttm bo->offset
On 2/13/20 1:18 PM, Christian König wrote: Looks like most of the patch is missing? We should have quite a number of uses of the BO offset in radeon or do all of those go through radeon_bo_gpu_offset? Compilation worked so I think all those(bo->offset) accesses went through radeon_bo_gpu_offset. If yes then the change is much smaller than I thought i needs to be. Christian. Regards, Nirmoy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 0/6] do not store GPU address in TTM
Am 13.02.20 um 13:01 schrieb Nirmoy Das: With this patch series I am trying to remove GPU address dependency in TTM and moving GPU address calculation to individual drm drivers. This is required[1] to continue the work started by Brian Welty to create struct drm_mem_region which can be leverage by DRM cgroup controller to manage memory limits. Nice work. I wouldn't say that it is necessary for [1], but it is certainly nice to have that cleaned up. Christian. I have only manage to test amdgpu driver as I only have GPU for that. I might be doing something really stupid while calculeting gpu offset for some of the drivers so please be patient and let me know how can I improve that. [1] https://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg272238.html Nirmoy Das (6): drm/amdgpu: move ttm bo->offset to amdgpu_bo drm/radeon: don't use ttm bo->offset drm/vmwgfx: don't use ttm bo->offset drm/nouveau: don't use ttm bo->offset drm/qxl: don't use ttm bo->offset drm/ttm: do not keep GPU dependent addresses drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 22 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 29 - drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 4 +-- drivers/gpu/drm/nouveau/dispnv04/crtc.c | 6 ++--- drivers/gpu/drm/nouveau/dispnv04/disp.c | 2 +- drivers/gpu/drm/nouveau/dispnv04/overlay.c | 6 ++--- drivers/gpu/drm/nouveau/dispnv50/base507c.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/core507d.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/ovly507e.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/wndw.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c | 2 +- drivers/gpu/drm/nouveau/nouveau_abi16.c | 8 +++--- drivers/gpu/drm/nouveau/nouveau_bo.c| 1 + drivers/gpu/drm/nouveau/nouveau_bo.h| 3 +++ drivers/gpu/drm/nouveau/nouveau_chan.c | 2 +- drivers/gpu/drm/nouveau/nouveau_fbcon.c | 2 +- drivers/gpu/drm/nouveau/nouveau_gem.c | 10 +++ drivers/gpu/drm/qxl/qxl_drv.h | 6 ++--- drivers/gpu/drm/qxl/qxl_kms.c | 3 +++ drivers/gpu/drm/qxl/qxl_object.h| 5 drivers/gpu/drm/qxl/qxl_ttm.c | 9 --- drivers/gpu/drm/radeon/radeon.h | 1 + drivers/gpu/drm/radeon/radeon_object.h | 16 +++- drivers/gpu/drm/radeon/radeon_ttm.c | 4 +-- drivers/gpu/drm/ttm/ttm_bo.c| 7 - drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 4 +-- drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c| 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 -- include/drm/ttm/ttm_bo_api.h| 2 -- include/drm/ttm/ttm_bo_driver.h | 1 - 33 files changed, 99 insertions(+), 72 deletions(-) -- 2.25.0 ___ amd-gfx mailing list amd-...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 2/6] drm/radeon: don't use ttm bo->offset
Looks like most of the patch is missing? We should have quite a number of uses of the BO offset in radeon or do all of those go through radeon_bo_gpu_offset? If yes then the change is much smaller than I thought i needs to be. Christian. Am 13.02.20 um 13:01 schrieb Nirmoy Das: Signed-off-by: Nirmoy Das --- drivers/gpu/drm/radeon/radeon.h| 1 + drivers/gpu/drm/radeon/radeon_object.h | 16 +++- drivers/gpu/drm/radeon/radeon_ttm.c| 4 +--- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index d59b004f6695..97cfcc2870af 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -2823,6 +2823,7 @@ extern void radeon_ttm_set_active_vram_size(struct radeon_device *rdev, u64 size extern void radeon_program_register_sequence(struct radeon_device *rdev, const u32 *registers, const u32 array_size); +struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev); /* * vm diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h index d23f2ed4126e..4d37571c7ff5 100644 --- a/drivers/gpu/drm/radeon/radeon_object.h +++ b/drivers/gpu/drm/radeon/radeon_object.h @@ -90,7 +90,21 @@ static inline void radeon_bo_unreserve(struct radeon_bo *bo) */ static inline u64 radeon_bo_gpu_offset(struct radeon_bo *bo) { - return bo->tbo.offset; + struct radeon_device *rdev; + u64 start = 0; + + rdev = radeon_get_rdev(bo->tbo.bdev); + + switch(bo->tbo.mem.mem_type) { + case TTM_PL_TT: + start = rdev->mc.gtt_start; + break; + case TTM_PL_VRAM: + start = rdev->mc.vram_start; + break; + } + + return (bo->tbo.mem.start << PAGE_SHIFT) + start; } static inline unsigned long radeon_bo_size(struct radeon_bo *bo) diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index 098bc9f40b98..b10654494262 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -56,7 +56,7 @@ static int radeon_ttm_debugfs_init(struct radeon_device *rdev); static void radeon_ttm_debugfs_fini(struct radeon_device *rdev); -static struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev) +struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev) { struct radeon_mman *mman; struct radeon_device *rdev; @@ -87,7 +87,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type, break; case TTM_PL_TT: man->func = _bo_manager_func; - man->gpu_offset = rdev->mc.gtt_start; man->available_caching = TTM_PL_MASK_CACHING; man->default_caching = TTM_PL_FLAG_CACHED; man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA; @@ -109,7 +108,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type, case TTM_PL_VRAM: /* "On-card" video ram */ man->func = _bo_manager_func; - man->gpu_offset = rdev->mc.vram_start; man->flags = TTM_MEMTYPE_FLAG_FIXED | TTM_MEMTYPE_FLAG_MAPPABLE; man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/tidss: dispc: Rewrite naive plane positioning code
On 13/02/2020 12:49, Tomi Valkeinen wrote: > On 13/02/2020 12:44, Jyri Sarha wrote: > >> + /* >> + * If a plane on a CRTC changes add all active planes on that >> + * CRTC to the atomic state. This is needed for updating the >> + * plane positions in tidss_crtc_position_planes() which is >> + * called from crtc_atomic_enable() and crtc_atomic_flush(). >> + * The update is needed for x,y-position changes too, so >> + * zpos_changed condition is not enough and we need this if >> + * planes_changed is true too. >> + */ >> + for_each_new_crtc_in_state(state, crtc, cstate, i) { >> + if (cstate->zpos_changed || cstate->planes_changed) { >> + ret = drm_atomic_add_affected_planes(state, crtc); >> + if (ret) >> + return ret; >> + } >> + } > > I think 99.99...% of the commits are such where only planes' fb address > is changed. I think "planes_changed" is true for these. I wonder if it > would be a sensible optimization to skip this for those 99.99...% cases. > Can we detect that easily? > Sure by looping all planes in the state through with for_each_oldnew_plane_in_state() and checking if crtc_x or crtc_y has changed. But then again writing the positions of max 4 planes is really not that heavy operation. There is more calculation to do and more registers to write when updating the fp, so I do not think avoiding the OVR update justifies the extra complexity. > Well, we haven't optimized for those 99.99...% cases anywhere else > either, so it's possible it doesn't matter. > -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v2 2/5] drm/hdcp: fix DRM_HDCP_2_KSV_COUNT_2_LSBITS
On 2020-02-12 at 15:59:39 +0530, Ramalingam C wrote: > Need to extract the 2 most significant bits from a byte for constructing > the revoked KSV count of the SRM. > > Signed-off-by: Ramalingam C > --- > include/drm/drm_hdcp.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h > index d512089b873f..c6bab4986a65 100644 > --- a/include/drm/drm_hdcp.h > +++ b/include/drm/drm_hdcp.h > @@ -276,7 +276,7 @@ void drm_hdcp_cpu_to_be24(u8 > seq_num[HDCP_2_2_SEQ_NUM_LEN], u32 val) > #define DRM_HDCP_2_VRL_LENGTH_SIZE 3 > #define DRM_HDCP_2_DCP_SIG_SIZE 384 > #define DRM_HDCP_2_NO_OF_DEV_PLUS_RESERVED_SZ4 > -#define DRM_HDCP_2_KSV_COUNT_2_LSBITS(byte) (((byte) & 0xC) >> 6) > +#define DRM_HDCP_2_KSV_COUNT_2_LSBITS(byte) (((byte) & 0xC0) >> 6) LGTM, verified wrt Table 5.1 HDCP HDMI specs page 64. Reviewed-by: Anshuman Gupta > > struct hdcp_srm_header { > u8 srm_id; > -- > 2.20.1 > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: replace dma_resv object on deleted BOs v3
> 2020年2月13日 18:01,Koenig, Christian 写道: > > Am 13.02.20 um 05:11 schrieb Pan, Xinhui: >> >> >>> 2020年2月12日 19:53,Christian König 写道: >>> >>> Am 12.02.20 um 07:23 schrieb Pan, Xinhui: > 2020年2月11日 23:43,Christian König 写道: > > When non-imported BOs are resurrected for delayed delete we replace > the dma_resv object to allow for easy reclaiming of the resources. > > v2: move that to ttm_bo_individualize_resv > v3: add a comment to explain what's going on > > Signed-off-by: Christian König > Reviewed-by: xinhui pan > --- > drivers/gpu/drm/ttm/ttm_bo.c | 14 +- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index bfc42a9e4fb4..8174603d390f 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -393,6 +393,18 @@ static int ttm_bo_individualize_resv(struct > ttm_buffer_object *bo) > > r = dma_resv_copy_fences(>base._resv, bo->base.resv); > dma_resv_unlock(>base._resv); > + if (r) > + return r; > + > + if (bo->type != ttm_bo_type_sg) { > + /* This works because the BO is about to be destroyed and nobody > + * reference it any more. The only tricky case is the trylock on > + * the resv object while holding the lru_lock. > + */ > + spin_lock(_bo_glob.lru_lock); > + bo->base.resv = >base._resv; > + spin_unlock(_bo_glob.lru_lock); > + } > how about something like that. the basic idea is to do the bo cleanup work in bo release first and avoid any race with evict. As in bo dieing progress, evict also just do bo cleanup work. If bo is busy, neither bo_release nor evict can do cleanupwork on it. For the bo release case, we just add bo back to lru list. So we can clean it up both in workqueue and shrinker as the past way did. @@ -405,8 +405,9 @@ static int ttm_bo_individualize_resv(struct ttm_buffer_object *bo) if (bo->type != ttm_bo_type_sg) { spin_lock(_bo_glob.lru_lock); - bo->base.resv = >base._resv; + ttm_bo_del_from_lru(bo); spin_unlock(_bo_glob.lru_lock); + bo->base.resv = >base._resv; } return r; @@ -606,10 +607,9 @@ static void ttm_bo_release(struct kref *kref) * shrinkers, now that they are queued for * destruction. */ - if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT) { + if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT) bo->mem.placement &= ~TTM_PL_FLAG_NO_EVICT; - ttm_bo_move_to_lru_tail(bo, NULL); - } + ttm_bo_add_mem_to_lru(bo, >mem); kref_init(>kref); list_add_tail(>ddestroy, >ddestroy); >>> Yeah, thought about that as well. But this has the major drawback that the >>> deleted BO moves to the end of the LRU, which is something we don't want. >> well, as the bo is busy, looks like it needs time to being idle. putting it >> to tail seems fair. > > No, see BOs should move to the tail of the LRU whenever they are used. > Freeing up a BO is basically the opposite of using it. > > So what would happen on the next memory contention is that the MM would evict > BOs which are still used and only after come to the delete BO which could > have been removed long ago. > >>> I think the real solution to this problem is to go a completely different >>> way and remove the delayed delete feature from TTM altogether. Instead this >>> should be part of some DRM domain handler component. >>> >> yes, completely agree. As long as we can shrink bos when OOM, the workqueue >> is not necessary, The workqueue does not help in a heavy workload case. >> >> Pls see my patches below, I remove the workqueue, and what’s more, we can >> clearup the bo without lru lock hold. >> That would reduce the lock contention. I run kfdtest and got a good >> performance result. > > No, that's an approach we had before as well and it also doesn't work that > well. > > See the problem is that we can only remove the BO from the LRU after it has > released the memory it references. Otherwise we run into the issue that some > threads can't wait for the memory to be freed any more and run into an OOM > situation. > ok, we can keep the workqueue at it is. Now I come up with another idea that evict and swap can touch the destroy list first, then lru list. Looks like putting a dieing bo in lru list is useless. thanks xinhui > Regards, > Christian. > >> >> >>> In other words it should not matter if a BO is evicted, moved or freed. >>> Whenever a piece of memory becomes available again we keep around a fence >>> which marks the end of using this piece of memory. >>> >>> When then
Re: [PATCH v2] drm/panfrost: Don't try to map on error faults
On 12/02/2020 20:22, Rob Herring wrote: > From: Tomeu Vizoso > > If the exception type isn't a translation fault, don't try to map and > instead go straight to a terminal fault. > > Otherwise, we can get flooded by kernel warnings and further faults. > > Signed-off-by: Tomeu Vizoso > Signed-off-by: Rob Herring > --- > I rewrote this some simplifying the code and somewhat following Steven's > suggested. Still not using defines though. No defines here was good > enough before IMO. Heresy! It's a good thing you're not writing kbase code - there you (seemingly) need to pick a #define which is as long as possible, but then still wrap the code to avoid the 80 character limit... For example[1]. Although shifting the code right might get you extra bonus points, deep nesting seems to be encouraged! ;) [1] https://gitlab.freedesktop.org/panfrost/mali_kbase/blob/master/driver/product/kernel/drivers/gpu/arm/midgard/backend/gpu/mali_kbase_js_backend.c#L156 > > Only compile tested. I've done some quick testing on a Firefly RK3288. But I don't have any (handy) tests to trigger non-TRANSLATION_FAULT MMU faults. Reviewed-by: Steven Price Thanks, Steve > > drivers/gpu/drm/panfrost/panfrost_mmu.c | 44 +++-- > 1 file changed, 19 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c > b/drivers/gpu/drm/panfrost/panfrost_mmu.c > index 763cfca886a7..4f2836bd9215 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c > +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c > @@ -596,33 +596,27 @@ static irqreturn_t panfrost_mmu_irq_handler_thread(int > irq, void *data) > source_id = (fault_status >> 16); > > /* Page fault only */ > - if ((status & mask) == BIT(i)) { > - WARN_ON(exception_type < 0xC1 || exception_type > 0xC4); > - > + ret = -1; > + if ((status & mask) == BIT(i) && (exception_type & 0xF8) == > 0xC0) > ret = panfrost_mmu_map_fault_addr(pfdev, i, addr); > - if (!ret) { > - mmu_write(pfdev, MMU_INT_CLEAR, BIT(i)); > - status &= ~mask; > - continue; > - } > - } > > - /* terminal fault, print info about the fault */ > - dev_err(pfdev->dev, > - "Unhandled Page fault in AS%d at VA 0x%016llX\n" > - "Reason: %s\n" > - "raw fault status: 0x%X\n" > - "decoded fault status: %s\n" > - "exception type 0x%X: %s\n" > - "access type 0x%X: %s\n" > - "source id 0x%X\n", > - i, addr, > - "TODO", > - fault_status, > - (fault_status & (1 << 10) ? "DECODER FAULT" : "SLAVE > FAULT"), > - exception_type, panfrost_exception_name(pfdev, > exception_type), > - access_type, access_type_name(pfdev, fault_status), > - source_id); > + if (ret) > + /* terminal fault, print info about the fault */ > + dev_err(pfdev->dev, > + "Unhandled Page fault in AS%d at VA 0x%016llX\n" > + "Reason: %s\n" > + "raw fault status: 0x%X\n" > + "decoded fault status: %s\n" > + "exception type 0x%X: %s\n" > + "access type 0x%X: %s\n" > + "source id 0x%X\n", > + i, addr, > + "TODO", > + fault_status, > + (fault_status & (1 << 10) ? "DECODER FAULT" : > "SLAVE FAULT"), > + exception_type, panfrost_exception_name(pfdev, > exception_type), > + access_type, access_type_name(pfdev, > fault_status), > + source_id); > > mmu_write(pfdev, MMU_INT_CLEAR, mask); > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/tidss: dispc: Rewrite naive plane positioning code
On 13/02/2020 12:44, Jyri Sarha wrote: + /* +* If a plane on a CRTC changes add all active planes on that +* CRTC to the atomic state. This is needed for updating the +* plane positions in tidss_crtc_position_planes() which is +* called from crtc_atomic_enable() and crtc_atomic_flush(). +* The update is needed for x,y-position changes too, so +* zpos_changed condition is not enough and we need this if +* planes_changed is true too. +*/ + for_each_new_crtc_in_state(state, crtc, cstate, i) { + if (cstate->zpos_changed || cstate->planes_changed) { + ret = drm_atomic_add_affected_planes(state, crtc); + if (ret) + return ret; + } + } I think 99.99...% of the commits are such where only planes' fb address is changed. I think "planes_changed" is true for these. I wonder if it would be a sensible optimization to skip this for those 99.99...% cases. Can we detect that easily? Well, we haven't optimized for those 99.99...% cases anywhere else either, so it's possible it doesn't matter. Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3] drm/tidss: dispc: Rewrite naive plane positioning code
The old implementation of placing planes on the CRTC while configuring the planes was naive and relied on the order in which the planes were configured, enabled, and disabled. The situation where a plane's zpos was changed on the fly was completely broken. The usual symptoms of this problem was scrambled display and a flood of sync lost errors, when a plane was active in two layers at the same time, or a missing plane, in case when a layer was accidentally disabled. The rewrite takes a more straight forward approach when HW is concerned. The plane positioning registers are in the CRTC (or actually OVR) register space and it is more natural to configure them in a one go when configuring the CRTC. To do this we need make sure we have all the planes on the updated CRTCs in the new atomic state. The untouched planes on changed CRTCs are added to the atomic state in tidss_atomic_check(). Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tidss/tidss_crtc.c | 52 +++ drivers/gpu/drm/tidss/tidss_dispc.c | 55 +++-- drivers/gpu/drm/tidss/tidss_dispc.h | 5 +++ drivers/gpu/drm/tidss/tidss_kms.c | 33 - 4 files changed, 109 insertions(+), 36 deletions(-) diff --git a/drivers/gpu/drm/tidss/tidss_crtc.c b/drivers/gpu/drm/tidss/tidss_crtc.c index 032c31ee2820..36478c54784b 100644 --- a/drivers/gpu/drm/tidss/tidss_crtc.c +++ b/drivers/gpu/drm/tidss/tidss_crtc.c @@ -17,6 +17,7 @@ #include "tidss_dispc.h" #include "tidss_drv.h" #include "tidss_irq.h" +#include "tidss_plane.h" /* Page flip and frame done IRQs */ @@ -111,6 +112,53 @@ static int tidss_crtc_atomic_check(struct drm_crtc *crtc, return dispc_vp_bus_check(dispc, hw_videoport, state); } +/* + * This needs all affected planes to be present in the atomic + * state. The untouched planes are added to the state in + * tidss_atomic_check(). + */ +static void tidss_crtc_position_planes(struct tidss_device *tidss, + struct drm_crtc *crtc, + struct drm_crtc_state *old_state, + bool newmodeset) +{ + struct drm_atomic_state *ostate = old_state->state; + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); + struct drm_crtc_state *cstate = crtc->state; + int zpos; + + if (!newmodeset && !cstate->zpos_changed && !cstate->planes_changed) + return; + + for (zpos = 0; zpos < tidss->feat->num_planes; zpos++) { + struct drm_plane_state *pstate; + struct drm_plane *plane; + bool zpos_taken = false; + int i; + + for_each_new_plane_in_state(ostate, plane, pstate, i) { + if (pstate->crtc != crtc || !pstate->visible) + continue; + + if (pstate->normalized_zpos == zpos) { + zpos_taken = true; + break; + } + } + + if (zpos_taken) { + struct tidss_plane *tplane = to_tidss_plane(plane); + + dispc_ovr_set_plane(tidss->dispc, tplane->hw_plane_id, + tcrtc->hw_videoport, + pstate->crtc_x, pstate->crtc_y, + zpos); + } + dispc_ovr_enable_layer(tidss->dispc, tcrtc->hw_videoport, zpos, + zpos_taken); + } +} + static void tidss_crtc_atomic_flush(struct drm_crtc *crtc, struct drm_crtc_state *old_crtc_state) { @@ -146,6 +194,9 @@ static void tidss_crtc_atomic_flush(struct drm_crtc *crtc, /* Write vp properties to HW if needed. */ dispc_vp_setup(tidss->dispc, tcrtc->hw_videoport, crtc->state, false); + /* Update plane positions if needed. */ + tidss_crtc_position_planes(tidss, crtc, old_crtc_state, false); + WARN_ON(drm_crtc_vblank_get(crtc) != 0); spin_lock_irqsave(>event_lock, flags); @@ -183,6 +234,7 @@ static void tidss_crtc_atomic_enable(struct drm_crtc *crtc, return; dispc_vp_setup(tidss->dispc, tcrtc->hw_videoport, crtc->state, true); + tidss_crtc_position_planes(tidss, crtc, old_state, true); /* Turn vertical blanking interrupt reporting on. */ drm_crtc_vblank_on(crtc); diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c index eeb160dc047b..e79dad246b1e 100644 --- a/drivers/gpu/drm/tidss/tidss_dispc.c +++ b/drivers/gpu/drm/tidss/tidss_dispc.c @@ -281,11 +281,6 @@ struct dss_vp_data { u32 *gamma_table; }; -struct dss_plane_data { - u32 zorder; - u32 hw_videoport; -}; - struct dispc_device { struct tidss_device *tidss; struct device *dev; @@ -307,8 +302,6 @@