Re: [PATCH] backlight: adp8860: Mark expected switch fall-through
On Mon, 09 Jul 2018, Gustavo A. R. Silva wrote: > In preparation to enabling -Wimplicit-fallthrough, mark switch cases > where we are expecting to fall through. > > Signed-off-by: Gustavo A. R. Silva > --- > drivers/video/backlight/adp8860_bl.c | 1 + > 1 file changed, 1 insertion(+) Looks good to me, but Dan should still review. https://gcc.gnu.org/onlinedocs/gcc-8.1.0/gcc.pdf#page=86 Acked-by: Lee Jones -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fb: fix lost console when the user unplugs a USB adapter
On Wed, 4 Jul 2018, Noralf Trønnes wrote: > AFAIU calling unregister_framebuffer() with open fd's is just fine as > long as fb_info with buffers stay intact. All it does is to remove the > fbX from userspace. Cleanup can be done in fb_ops->fb_destroy. > > I have been working on generic fbdev emulation for DRM [1] and I did a > test now to see what would happen if I did unbind the driver from the > device. It worked as expected if I didn't have another fbdev present, > but if there is an fb0 and I remove fb1 with a console on it, I would > sometimes get crashes, often with a call to cursor_timer_handler() in > the traceback. > > I think there's index mixup in fbcon_fb_unbind(), at least this change > seems to solve the immediate problem: > > diff --git a/drivers/video/fbdev/core/fbcon.c > b/drivers/video/fbdev/core/fbcon.c > index 5fb156bdcf4e..271b9b988b73 100644 > --- a/drivers/video/fbdev/core/fbcon.c > +++ b/drivers/video/fbdev/core/fbcon.c > @@ -3066,7 +3072,7 @@ static int fbcon_fb_unbind(int idx) > for (i = first_fb_vc; i <= last_fb_vc; i++) { > if (con2fb_map[i] != idx && > con2fb_map[i] != -1) { > - new_idx = i; > + new_idx = con2fb_map[i]; > break; > } > } Reviewed-by: Mikulas Patocka Yes - that's a good point. i is virtual console index and it is assigned to new_idx and new_idx is interpreted as a framebuffer device index. > I haven't got time to follow up on this now, but making sure DRM generic > fbdev emulation device unplug works is on my TODO. > > BTW, I believe the udl drm driver should be able to use the generic fbdev > emulation if it had a drm_driver->gem_prime_vmap hook. It uses a shadow > buffer which would also make fbdev mmap work for udl. (shmem buffers and > fbdev deferred I/O doesn't work together since they both use > page->lru/mapping) I have sent patch for this: https://patchwork.kernel.org/patch/10445393/ Mikulas___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fb: fix lost console when the user unplugs a USB adapter
On Wed, 4 Jul 2018, Bartlomiej Zolnierkiewicz wrote: > On Tuesday, July 03, 2018 01:18:57 PM Mikulas Patocka wrote: > > > > On Tue, 3 Jul 2018, Bartlomiej Zolnierkiewicz wrote: > > > > > Hi, > > > > > > On Sunday, June 03, 2018 11:46:29 AM Mikulas Patocka wrote: > > > > I have a USB display adapter using the udlfb driver and I use it on an > > > > ARM > > > > board that doesn't have any graphics card. When I plug the adapter in, > > > > the > > > > console is properly displayed, however when I unplug and re-plug the > > > > adapter, the console is not displayed and I can't access it until I > > > > reboot > > > > the board. > > > > > > > > The reason is this: > > > > When the adapter is unplugged, dlfb_usb_disconnect calls > > > > unlink_framebuffer, then it waits until the reference count drops to > > > > zero > > > > and then it deallocates the framebuffer. However, the console that is > > > > attached to the framebuffer device keeps the reference count non-zero, > > > > so > > > > the framebuffer device is never destroyed. When the USB adapter is > > > > plugged > > > > again, it creates a new device /dev/fb1 and the console is not attached > > > > to > > > > it. > > > > > > > > This patch fixes the bug by unbinding the console from > > > > unlink_framebuffer. > > > > The code to unbind the console is moved from do_unregister_framebuffer > > > > to > > > > a function unbind_console. When the console is unbound, the reference > > > > count drops to zero and the udlfb driver frees the framebuffer. When the > > > > adapter is plugged back, a new framebuffer is created and the console is > > > > attached to it. > > > > > > > > Signed-off-by: Mikulas Patocka > > > > Cc: sta...@vger.kernel.org > > > > > > After this change unbind_console() will be called twice in the standard > > > framebuffer unregister path: > > > > > > - first time, directly by do_unregister_framebuffer() > > > > > > - second time, indirectly by > > > do_unregister_framebuffer()->unlink_framebuffer() > > > > > > This doesn't look correctly. > > > > unbind_console calls the FB_EVENT_FB_UNBIND notifier, FB_EVENT_FB_UNBIND > > goes to the function fbcon_fb_unbind and fbcon_fb_unbind checks if the > > console is bound to the framebuffer for which unbind is requested. So a > > double call won't cause any trouble. > > Even if it works okay currently it is not a best design to send duplicate > events - especially since this can be easily avoided (for non-udlfb users) > by: > > - renaming "vanilla" unlink_framebuffer() to __unlink_framebuffer() > > - converting do_unregister_framebuffer() to use __unlink_framebuffer() > > - adding "new" unlink_framebuffer() that will also call unbind_console() > > > > Also why can't udlfb just use unregister_framebuffer() like all other > > > drivers (it uses unlink_framebuffer() and it is the only user of this > > > helper)? > > > > It uses unregister_framebuffer() - but - unregister_framebuffer() may only > > be called when the open count of the framebuffer is zero. So, the udlfb > > driver waits until the open count drops to zero and then calls > > unregister_framebuffer(). > > > > But the console subsystem keeps the framebuffer open - which means that if > > user use unplugs the USB adapter, the open count won't drop to zero > > (because the console is bound to it) - which means that > > unregister_framebuffer() will not be called. > > Is it a really the console subsystem and not the user-space keeping > /dev/fb0 (with console binded to fb0) opened after the USB device > vanishes? Yes - I unplugged the adapter without Xserver running and without any other framebuffer application running - the console keeps it open. > After re-plugging the USB device /dev/fb0 stays and /dev/fb1 > appears, right? The file /dev/fb0 is deleted (because dlfb_usb_disconnect calls unlink_framebuffer), but it is kept in the kernel. When I re-plug the adapter, /dev/fb1 is created but the console is still bound to /dev/fb0. When the adapter is re-plugged, it shows just a green screen. > I also mean that unregister_framebuffer() should be called instead > unlink_framebuffer(), not additionally some time later as it is done > currently. Can unregister_framebuffer() be called when /dev/fb0 is open as a file handle and/or mapped to some process? > Moreover the dlfb <-> fb_info locking scheme seems to be reversed > (+racy) as it is dlfb that should control lifetime of fb_info, then > in dlfb_free() we should just call framebuffer_release() etc. How should in your opinion framebuffer destruction work? Should the driver count the number of users and call unregister_framebuffer() when it drops to zero? Or should the driver call unregister_framebuffer() unconditionally when the device is unplugged and destroy the device in the "fb_destroy" callback? (fb_destroy seems to be called by the framebuffer subsystem when the open count reaches zero) If I grep the kernel for fb_destroy, very
[git pull] drm fixes for 4.18-rc5
Hey Linus, School holidays here, and I'm not sure if I'll be inclined to work until next week, (I might and if so there might be another fixes), but thought it best to just dequeue the bits I had now. Otherwise the rest of the fixes for rc5 might arrive just before rc5 or just after. This just contains some etnaviv fixes and a MAINTAINERS update for the new drm tree locations. Dave. drm-fixes-2018-07-10: etnaviv fixes and MAINTAINERS patch The following changes since commit 1e4b044d22517cae7047c99038abb23243ca: Linux 4.18-rc4 (2018-07-08 16:34:02 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2018-07-10 for you to fetch changes up to dc81aab1be9fac2e11f31fe7538a50705eba08cf: MAINTAINERS: update drm tree (2018-07-10 10:59:58 +1000) etnaviv fixes and MAINTAINERS patch Daniel Vetter (1): MAINTAINERS: update drm tree Dave Airlie (1): Merge branch 'etnaviv/fixes' of https://git.pengutronix.de/git/lst/linux into drm-fixes Fabio Estevam (2): drm/etnaviv: Check for platform_device_register_simple() failure drm/etnaviv: Fix driver unregistering Lucas Stach (1): drm/etnaviv: bring back progress check in job timeout handler MAINTAINERS | 4 ++-- drivers/gpu/drm/etnaviv/etnaviv_drv.c | 24 drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 3 +++ drivers/gpu/drm/etnaviv/etnaviv_sched.c | 24 4 files changed, 49 insertions(+), 6 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] kernel.h: Add for_each_if()
On Mon, 9 Jul 2018 18:25:09 +0200 Daniel Vetter wrote: > To avoid compilers complainig about ambigious else blocks when putting > an if condition into a for_each macro one needs to invert the > condition and add a dummy else. We have a nice little convenience > macro for that in drm headers, let's move it out. Subsequent patches > will roll it out to other places. > > The issue the compilers complain about are nested if with an else > block and no {} to disambiguate which if the else belongs to. The C > standard is clear, but in practice people forget: > > if (foo) > if (bar) > /* something */ > else > /* something else um, yeah, don't do that. Kernel coding style is very much to do if (foo) { if (bar) /* something */ else /* something else } And if not doing that generates a warning then, well, do that. > The same can happen in a for_each macro when it also contains an if > condition at the end, except the compiler message is now really > confusing since there's only 1 if: > > for_each_something() > if (bar) > /* something */ > else > /* something else > > The for_each_if() macro, by inverting the condition and adding an > else, avoids the compiler warning. Ditto. > Motivated by a discussion with Andy and Yisheng, who want to add > another for_each_macro which would benefit from for_each_if() instead > of hand-rolling it. Ditto. > v2: Explain a bit better what this is good for, after the discussion > with Peter Z. Presumably the above was discussed in whatever-thread-that-was. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 10/12] pci: use for_each_if
On Mon, Jul 09, 2018 at 10:36:48AM +0200, Daniel Vetter wrote: > Avoids the inverted condition compared to the open-coded version. > > Signed-off-by: Daniel Vetter > Cc: Bjorn Helgaas > Cc: linux-...@vger.kernel.org Acked-by: Bjorn Helgaas I assume you'll merge this with the rest of the series. > --- > include/linux/pci.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 340029b2fb38..1484471ed048 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -601,7 +601,7 @@ static inline bool pci_is_bridge(struct pci_dev *dev) > > #define for_each_pci_bridge(dev, bus)\ > list_for_each_entry(dev, >devices, bus_list) \ > - if (!pci_is_bridge(dev)) {} else > + for_each_if (pci_is_bridge(dev)) > > static inline struct pci_dev *pci_upstream_bridge(struct pci_dev *dev) > { > -- > 2.18.0 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] cpufreq: use for_each_if
On Mon, Jul 9, 2018 at 6:11 PM, Daniel Vetter wrote: > Avoids the inverted condition compared to the open coded version. > > Signed-off-by: Daniel Vetter > Cc: "Rafael J. Wysocki" > Cc: Viresh Kumar > Cc: linux...@vger.kernel.org > Cc: Eric Engestrom > -- > v2: Fix the logic fumble in the 2nd hunk, spotted by Eric. Acked-by: Rafael J. Wysocki Or do you want me to apply it? > --- > include/linux/cpufreq.h | 8 ++-- > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h > index 882a9b9e34bc..3a774f523d00 100644 > --- a/include/linux/cpufreq.h > +++ b/include/linux/cpufreq.h > @@ -649,9 +649,7 @@ static inline void dev_pm_opp_free_cpufreq_table(struct > device *dev, > > #define cpufreq_for_each_valid_entry(pos, table) \ > for (pos = table; pos->frequency != CPUFREQ_TABLE_END; pos++) \ > - if (pos->frequency == CPUFREQ_ENTRY_INVALID)\ > - continue; \ > - else > + for_each_if (pos->frequency != CPUFREQ_ENTRY_INVALID) > > /* > * cpufreq_for_each_valid_entry_idx - iterate with index over a cpufreq > @@ -663,9 +661,7 @@ static inline void dev_pm_opp_free_cpufreq_table(struct > device *dev, > > #define cpufreq_for_each_valid_entry_idx(pos, table, idx) \ > cpufreq_for_each_entry_idx(pos, table, idx) \ > - if (pos->frequency == CPUFREQ_ENTRY_INVALID)\ > - continue; \ > - else > + for_each_if (pos->frequency != CPUFREQ_ENTRY_INVALID) > > > int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy, > -- > 2.18.0 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 20/40] drm/i915: Check HDCP 1.4 and 2.2 link on CP_IRQ
On Wed, Jun 27, 2018 at 02:10:09PM +0530, Ramalingam C wrote: > On DP HDCP1.4 and 2.2, when CP_IRQ is received, start the link > integrity check for the HDCP version that is enabled. > > v2: > Rebased. Function name is changed. > v3: > No Changes. > v4: > No Changes. > v5: > No Changes. > > Signed-off-by: Ramalingam C > cc: Sean Paul > --- > drivers/gpu/drm/i915/intel_dp.c | 2 +- > drivers/gpu/drm/i915/intel_drv.h | 2 +- > drivers/gpu/drm/i915/intel_hdcp.c | 31 ++- > 3 files changed, 32 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 7467e7b3f2df..a6ba27ef20ae 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4484,7 +4484,7 @@ intel_dp_short_pulse(struct intel_dp *intel_dp) > if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST) > intel_dp_handle_test_request(intel_dp); > if (sink_irq_vector & DP_CP_IRQ) > - intel_hdcp_check_link(intel_dp->attached_connector); > + intel_hdcp_handle_cp_irq(intel_dp->attached_connector); > if (sink_irq_vector & DP_SINK_SPECIFIC_IRQ) > DRM_DEBUG_DRIVER("Sink specific irq unhandled\n"); > } > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index 7624388eecd5..875657fd7d3c 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1965,8 +1965,8 @@ int intel_hdcp_init(struct intel_connector *connector, > bool hdcp2_supported); > int intel_hdcp_enable(struct intel_connector *connector); > int intel_hdcp_disable(struct intel_connector *connector); > -int intel_hdcp_check_link(struct intel_connector *connector); > bool is_hdcp_supported(struct drm_i915_private *dev_priv, enum port port); > +void intel_hdcp_handle_cp_irq(struct intel_connector *connector); > > /* intel_psr.c */ > #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv->psr.sink_support) > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c > b/drivers/gpu/drm/i915/intel_hdcp.c > index 790f4a9f4793..b035274785c8 100644 > --- a/drivers/gpu/drm/i915/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/intel_hdcp.c > @@ -32,6 +32,7 @@ int intel_hdcp_read_valid_bksv(struct intel_digital_port > *intel_dig_port, > const struct intel_hdcp_shim *shim, u8 *bksv); > static > struct intel_digital_port *conn_to_dig_port(struct intel_connector > *connector); > +static int intel_hdcp_check_link(struct intel_connector *connector); > > static bool panel_supports_hdcp(struct intel_connector *connector) > { > @@ -80,6 +81,26 @@ static inline bool intel_hdcp2_capable(struct > intel_connector *connector) > panel_supports_hdcp2(connector)); > } > > +static inline bool intel_hdcp_in_force(struct intel_connector *connector) nit: I'd use _in_use instead of in_force With that fixed, Reviewed-by: Sean Paul > +{ > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > + enum port port = connector->encoder->port; > + u32 reg; > + > + reg = I915_READ(PORT_HDCP_STATUS(port)); > + return reg & (HDCP_STATUS_AUTH | HDCP_STATUS_ENC); > +} > + > +static inline bool intel_hdcp2_in_force(struct intel_connector *connector) > +{ > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > + enum port port = connector->encoder->port; > + u32 reg; > + > + reg = I915_READ(HDCP2_STATUS_DDI(port)); > + return reg & (LINK_ENCRYPTION_STATUS | LINK_AUTH_STATUS); > +} > + > static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port > *intel_dig_port, > const struct intel_hdcp_shim *shim) > { > @@ -939,7 +960,7 @@ void intel_hdcp_atomic_check(struct drm_connector > *connector, > } > > /* Implements Part 3 of the HDCP authorization procedure */ > -int intel_hdcp_check_link(struct intel_connector *connector) > +static int intel_hdcp_check_link(struct intel_connector *connector) > { > struct intel_hdcp *hdcp = >hdcp; > struct drm_i915_private *dev_priv = connector->base.dev->dev_private; > @@ -2011,3 +2032,11 @@ static void intel_hdcp2_check_work(struct work_struct > *work) > schedule_delayed_work(>hdcp2_check_work, > DRM_HDCP2_CHECK_PERIOD_MS); > } > + > +void intel_hdcp_handle_cp_irq(struct intel_connector *connector) > +{ > + if (intel_hdcp_in_force(connector)) > + intel_hdcp_check_link(connector); > + else if (intel_hdcp2_in_force(connector)) > + intel_hdcp2_check_link(connector); > +} > -- > 2.7.4 > -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 19/40] drm/i915: hdcp_check_link only on CP_IRQ
On Wed, Jun 27, 2018 at 02:10:08PM +0530, Ramalingam C wrote: > HDCP check link is invoked only on CP_IRQ detection, instead of all > short pulses. > > v3: > No Changes. > v4: > Added sean in cc and collected the reviewed-by received. > v5: > No Change. > > Signed-off-by: Ramalingam C Reviewed-by: Sean Paul > cc: Sean Paul > Reviewed-by: Uma Shankar > --- > drivers/gpu/drm/i915/intel_dp.c | 9 - > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 6bcc52766ea3..7467e7b3f2df 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4483,8 +4483,10 @@ intel_dp_short_pulse(struct intel_dp *intel_dp) > > if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST) > intel_dp_handle_test_request(intel_dp); > - if (sink_irq_vector & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ)) > - DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n"); > + if (sink_irq_vector & DP_CP_IRQ) > + intel_hdcp_check_link(intel_dp->attached_connector); > + if (sink_irq_vector & DP_SINK_SPECIFIC_IRQ) > + DRM_DEBUG_DRIVER("Sink specific irq unhandled\n"); > } > > /* defer to the hotplug work for link retraining if needed */ > @@ -5454,9 +5456,6 @@ intel_dp_hpd_pulse(struct intel_digital_port > *intel_dig_port, bool long_hpd) > > handled = intel_dp_short_pulse(intel_dp); > > - /* Short pulse can signify loss of hdcp authentication */ > - intel_hdcp_check_link(intel_dp->attached_connector); > - > if (!handled) { > intel_dp->detect_done = false; > goto put_power; > -- > 2.7.4 > -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v5 13/40] drm/i915: Implement HDCP2.2 Enable and Disable
On Wed, Jun 27, 2018 at 02:10:02PM +0530, Ramalingam C wrote: > Implements a sequence of enabling and disabling the HDCP2.2 > (auth and encryption). This is really hard to review, since all I see are stubs. I'd much rather have each patch do something useful, instead of just call stubs. That said, I don't have a vested interest in HDCP2.2 on intel, so if others are fine with it, I am too. Sean > > v2: > Rebased. > v3: > No Changes. > v4: > No Changes. > v5: > Rebased as part of the patch reordering. > HDCP2 encryption status is tracked. > > Signed-off-by: Ramalingam C > --- > drivers/gpu/drm/i915/intel_hdcp.c | 105 > +- > 1 file changed, 104 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c > b/drivers/gpu/drm/i915/intel_hdcp.c > index 34bafc2025f7..f72684488bc7 100644 > --- a/drivers/gpu/drm/i915/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/intel_hdcp.c > @@ -994,14 +994,117 @@ int intel_hdcp_check_link(struct intel_connector > *connector) > return ret; > } > > +static int hdcp2_close_mei_session(struct intel_connector *connector) > +{ > + struct mei_hdcp_data *data = >hdcp.mei_data; > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > + struct i915_hdcp_component *comp = dev_priv->hdcp_comp; > + int ret; > + > + if (!comp) > + return -EINVAL; > + > + mutex_lock(>mutex); > + if (!comp->ops || !comp->mei_cldev || data->port == INVALID_PORT) { > + mutex_unlock(>mutex); > + return -EINVAL; > + } > + ret = comp->ops->close_hdcp_session(comp->mei_cldev, data); > + mutex_unlock(>mutex); > + > + return ret; > +} > + > +static int hdcp2_deauthenticate_port(struct intel_connector *connector) > +{ > + return hdcp2_close_mei_session(connector); > +} > + > +static int hdcp2_authenticate_sink(struct intel_connector *connector) > +{ > + return 0; > +} > + > +static int hdcp2_enable_encryption(struct intel_connector *connector) > +{ > + return 0; > +} > + > +static int hdcp2_disable_encryption(struct intel_connector *connector) > +{ > + return 0; > +} > + > +static int hdcp2_authenticate_and_encrypt(struct intel_connector *connector) > +{ > + int ret, i, tries = 3; > + > + for (i = 0; i < tries; i++) { > + ret = hdcp2_authenticate_sink(connector); > + if (!ret) > + break; > + > + /* Clearing the mei hdcp session */ > + hdcp2_deauthenticate_port(connector); > + DRM_DEBUG_KMS("HDCP2.2 Auth %d of %d Failed.(%d)\n", > + i + 1, tries, ret); > + } > + > + if (i != tries) { > + /* > + * Ensuring the required 200mSec min time interval between > + * Session Key Exchange and encryption. > + */ > + msleep(HDCP_2_2_DELAY_BEFORE_ENCRYPTION_EN); > + ret = hdcp2_enable_encryption(connector); > + if (ret < 0) { > + DRM_DEBUG_KMS("Encryption Enable Failed.(%d)\n", ret); > + hdcp2_deauthenticate_port(connector); > + } > + } > + > + return ret; > +} > + > static int _intel_hdcp2_enable(struct intel_connector *connector) > { > + struct intel_hdcp *hdcp = >hdcp; > + int ret; > + > + DRM_DEBUG_KMS("[%s:%d] HDCP2.2 is being enabled. Type: %d\n", > + connector->base.name, connector->base.base.id, > + hdcp->content_type); > + > + ret = hdcp2_authenticate_and_encrypt(connector); > + if (ret) { > + DRM_ERROR("HDCP2 Type%d Enabling Failed. (%d)\n", > + hdcp->content_type, ret); > + return ret; > + } > + > + DRM_DEBUG_KMS("[%s:%d] HDCP2.2 is enabled. Type %d\n", > + connector->base.name, connector->base.base.id, > + hdcp->content_type); > + > + hdcp->hdcp2_in_use = true; > + hdcp->hdcp_value = DRM_MODE_CONTENT_PROTECTION_ENABLED; > + schedule_work(>hdcp_prop_work); > return 0; > } > > static int _intel_hdcp2_disable(struct intel_connector *connector) > { > - return 0; > + int ret; > + > + DRM_DEBUG_KMS("[%s:%d] HDCP2.2 is being Disabled\n", > + connector->base.name, connector->base.base.id); > + > + ret = hdcp2_disable_encryption(connector); > + > + hdcp2_deauthenticate_port(connector); > + connector->hdcp.hdcp2_in_use = false; > + > + return ret; > } > > static int i915_hdcp_component_master_bind(struct device *dev) > -- > 2.7.4 > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org
Re: [Intel-gfx] [PATCH v5 12/40] drm/i915: Enable HDCP1.4 incase of HDCP2.2 failure
On Wed, Jun 27, 2018 at 02:10:01PM +0530, Ramalingam C wrote: > When HDCP2.2 enabling fails and HDCP1.4 is supported, HDCP1.4 is > enabled. > Just squash this into patch 11, no need for a separate patch. > v2: > Rebased. > v3: > No Changes. > v4: > Reviewed-by is collected. > v5: > No Change. > > Signed-off-by: Ramalingam C > Reviewed-by: Uma Shankar > --- > drivers/gpu/drm/i915/intel_hdcp.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c > b/drivers/gpu/drm/i915/intel_hdcp.c > index b34e3b1587d6..34bafc2025f7 100644 > --- a/drivers/gpu/drm/i915/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/intel_hdcp.c > @@ -863,7 +863,9 @@ int intel_hdcp_enable(struct intel_connector *connector) >*/ > if (intel_hdcp2_capable(connector)) > ret = _intel_hdcp2_enable(connector); > - else if (intel_hdcp_capable(connector)) > + > + /* When HDCP2.2 fails, HDCP1.4 will be attempted */ > + if (ret && intel_hdcp_capable(connector)) > ret = _intel_hdcp_enable(connector); > > if (!ret) { > -- > 2.7.4 > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 11/40] drm/i915: Enable superior HDCP ver that is capable
On Wed, Jun 27, 2018 at 02:10:00PM +0530, Ramalingam C wrote: > Considering that HDCP2.2 is more secure than HDCP1.4, When a setup > supports HDCP2.2 and HDCP1.4, HDCP2.2 will be enabled. > > v2: > Included few optimization suggestions [Chris Wilson] > Commit message is updated as per the rebased version. > v3: > No changes. > v4: > Extra comment added and Style issue fixed [Uma] > v5: > Rebased as part of patch reordering. > Flag is added for tracking hdcp2.2 encryption status. > > Signed-off-by: Ramalingam C > --- > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_hdcp.c | 90 > +++ > 2 files changed, 83 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index 2eeb82b04953..7624388eecd5 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -414,6 +414,7 @@ struct intel_hdcp { > > /* HDCP2.2 related definitions */ > bool hdcp2_supported; > + bool hdcp2_in_use; > > /* >* Content Stream Type defined by content owner. TYPE0(0x0) content can > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c > b/drivers/gpu/drm/i915/intel_hdcp.c > index 32a1a3f39b65..b34e3b1587d6 100644 > --- a/drivers/gpu/drm/i915/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/intel_hdcp.c > @@ -21,6 +21,60 @@ >(enum hdcp_physical_port)(port)) > > static int intel_hdcp2_init(struct intel_connector *connector); > +static int _intel_hdcp2_enable(struct intel_connector *connector); > +static int _intel_hdcp2_disable(struct intel_connector *connector); > +static > +int intel_hdcp_read_valid_bksv(struct intel_digital_port *intel_dig_port, > +const struct intel_hdcp_shim *shim, u8 *bksv); > +static > +struct intel_digital_port *conn_to_dig_port(struct intel_connector > *connector); Please avoid forward declarations of static functions. Just place things appropriately in the file. > + > +static bool panel_supports_hdcp(struct intel_connector *connector) > +{ > + struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector); > + struct intel_hdcp *hdcp = >hdcp; > + bool capable = false; > + u8 bksv[5]; > + > + if (hdcp->hdcp_shim) { This function can only be called from intel_hdcp_enable() -> intel_hdcp_capable() -> panel_supports_hdcp() Both of those preceding functions check if hdcp_shim is NULL. > + if (hdcp->hdcp_shim->hdcp_capable) { > + hdcp->hdcp_shim->hdcp_capable(intel_dig_port, ); > + } else { > + if (!intel_hdcp_read_valid_bksv(intel_dig_port, > + hdcp->hdcp_shim, bksv)) > + capable = true; > + } > + } > + > + return capable; > +} > + > +static inline > +bool panel_supports_hdcp2(struct intel_connector *connector) > +{ > + struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector); > + struct intel_hdcp *hdcp = >hdcp; > + bool capable = false; > + > + /* Check the panel's hdcp2.2 compliance if platform supports it. */ > + if (hdcp->hdcp2_supported) > + hdcp->hdcp_shim->hdcp_2_2_capable(intel_dig_port, ); > + > + return capable; > +} > + > +/* Is HDCP1.4 capable on Platform and Panel */ > +static inline bool intel_hdcp_capable(struct intel_connector *connector) > +{ > + return (connector->hdcp.hdcp_shim && panel_supports_hdcp(connector)); As mentioned, the hdcp_shim check is already covered by the caller. The way things are setup, the shim checks only need to exist at the entry points (enable/disable/check_link). > +} > + > +/* Is HDCP2.2 capable on Platform and Panel */ > +static inline bool intel_hdcp2_capable(struct intel_connector *connector) > +{ > + return (connector->hdcp.hdcp2_supported && > + panel_supports_hdcp2(connector)); > +} The panel_supports_* functions don't seem necessary, just do the work in the intel_hdcp*_capable functions. > > static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port > *intel_dig_port, > const struct intel_hdcp_shim *shim) > @@ -796,20 +850,27 @@ int intel_hdcp_init(struct intel_connector *connector, > int intel_hdcp_enable(struct intel_connector *connector) > { > struct intel_hdcp *hdcp = >hdcp; > - int ret; > + int ret = -EINVAL; > > if (!hdcp->hdcp_shim) > return -ENOENT; > > mutex_lock(>hdcp_mutex); > > - ret = _intel_hdcp_enable(connector); > - if (ret) > - goto out; > + /* > + * Considering that HDCP2.2 is more secure than HDCP1.4, If the setup > + * is capable of HDCP2.2, it is preferred to use HDCP2.2. > + */ > + if (intel_hdcp2_capable(connector)) > + ret = _intel_hdcp2_enable(connector); > + else
Re: [Intel-gfx] [PATCH v5 10/40] drm/i915: Pullout the bksv read and validation
On Wed, Jun 27, 2018 at 02:09:59PM +0530, Ramalingam C wrote: > For reusability purpose, this patch implements the hdcp1.4 bksv's > read and validation as a functions. > > For detecting the HDMI panel's HDCP capability this fucntions will be > used. > > v2: > Rebased. > v3: > No Changes. > v4: > inline tag is removed with modified error msg. > v5: > No Changes. > > Signed-off-by: Ramalingam C Reviewed-by: Sean Paul > --- > drivers/gpu/drm/i915/intel_hdcp.c | 37 + > 1 file changed, 25 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c > b/drivers/gpu/drm/i915/intel_hdcp.c > index 4bff74b3bed0..32a1a3f39b65 100644 > --- a/drivers/gpu/drm/i915/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/intel_hdcp.c > @@ -400,6 +400,28 @@ int intel_hdcp_validate_v_prime(struct > intel_digital_port *intel_dig_port, > return 0; > } > > +static > +int intel_hdcp_read_valid_bksv(struct intel_digital_port *intel_dig_port, > +const struct intel_hdcp_shim *shim, u8 *bksv) > +{ > + int ret, i, tries = 2; > + > + /* HDCP spec states that we must retry the bksv if it is invalid */ > + for (i = 0; i < tries; i++) { > + ret = shim->read_bksv(intel_dig_port, bksv); > + if (ret) > + return ret; > + if (intel_hdcp_is_ksv_valid(bksv)) > + break; > + } > + if (i == tries) { > + DRM_ERROR("Bksv is invalid\n"); > + return -ENODEV; > + } > + > + return 0; > +} > + > /* Implements Part 2 of the HDCP authorization procedure */ > static > int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port, > @@ -533,18 +555,9 @@ static int intel_hdcp_auth(struct intel_digital_port > *intel_dig_port, > > memset(, 0, sizeof(bksv)); > > - /* HDCP spec states that we must retry the bksv if it is invalid */ > - for (i = 0; i < tries; i++) { > - ret = shim->read_bksv(intel_dig_port, bksv.shim); > - if (ret) > - return ret; > - if (intel_hdcp_is_ksv_valid(bksv.shim)) > - break; > - } > - if (i == tries) { > - DRM_ERROR("HDCP failed, Bksv is invalid\n"); > - return -ENODEV; > - } > + ret = intel_hdcp_read_valid_bksv(intel_dig_port, shim, bksv.shim); > + if (ret < 0) > + return ret; > > I915_WRITE(PORT_HDCP_BKSVLO(port), bksv.reg[0]); > I915_WRITE(PORT_HDCP_BKSVHI(port), bksv.reg[1]); > -- > 2.7.4 > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 09/40] drm/i915: Schedule hdcp_check_link in _intel_hdcp_enable
On Wed, Jun 27, 2018 at 02:09:58PM +0530, Ramalingam C wrote: > As a preparation for making the intel_hdcp_enable as common function > for both HDCP1.4 and HDCP2.2, HDCP1.4 check_link scheduling is moved > into _intel_hdcp_enable() function. > > v3: > No Changes. > v4: > Style fix. > v5: > No Change. > > Signed-off-by: Ramalingam C > Reviewed-by: Uma Shankar > --- > drivers/gpu/drm/i915/intel_hdcp.c | 12 > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c > b/drivers/gpu/drm/i915/intel_hdcp.c > index 769560591aa8..4bff74b3bed0 100644 > --- a/drivers/gpu/drm/i915/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/intel_hdcp.c > @@ -688,7 +688,7 @@ static int _intel_hdcp_enable(struct intel_connector > *connector) > ret = intel_hdcp_auth(conn_to_dig_port(connector), > hdcp->hdcp_shim); > if (!ret) > - return 0; > + break; > > DRM_DEBUG_KMS("HDCP Auth failure (%d)\n", ret); > > @@ -696,7 +696,13 @@ static int _intel_hdcp_enable(struct intel_connector > *connector) > _intel_hdcp_disable(connector); > } > > - DRM_ERROR("HDCP authentication failed (%d tries/%d)\n", tries, ret); > + if (i != tries) > + schedule_delayed_work(>hdcp_check_work, > + DRM_HDCP_CHECK_PERIOD_MS); At best, this results in a duplicate scheduling when called from intel_hdcp_check_link(). At worst, it schedules a check when it's not supposed to (see the condition in intel_hdcp_check_work). Sean > + else > + DRM_ERROR("HDCP authentication failed (%d tries/%d)\n", > + tries, ret); > + > return ret; > } > > @@ -790,8 +796,6 @@ int intel_hdcp_enable(struct intel_connector *connector) > > hdcp->hdcp_value = DRM_MODE_CONTENT_PROTECTION_ENABLED; > schedule_work(>hdcp_prop_work); > - schedule_delayed_work(>hdcp_check_work, > - DRM_HDCP_CHECK_PERIOD_MS); > out: > mutex_unlock(>hdcp_mutex); > return ret; > -- > 2.7.4 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v5 07/40] drm/i915: Define Intel HDCP2.2 registers
On Wed, Jun 27, 2018 at 02:09:56PM +0530, Ramalingam C wrote: > Intel HDCP2.2 registers are defined with addr offsets and bit details. > > v2: > Replaced the arith calc with _PICK [Sean Paul] > v3: > No changes. > v4: > %s/HDCP2_CTR_DDI/HDCP2_CTL_DDI [Uma] > v5: > Added parentheses for the parameters of macro. > > Signed-off-by: Ramalingam C Reviewed-by: Sean Paul > --- > drivers/gpu/drm/i915/i915_reg.h | 32 > 1 file changed, 32 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index caad19f5f557..822fee56931e 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -8697,6 +8697,38 @@ enum skl_power_gate { > #define HDCP_STATUS_CIPHER BIT(16) > #define HDCP_STATUS_FRAME_CNT(x)(((x) >> 8) & 0xff) > > +/* HDCP2.2 Registers */ > +#define _PORTA_HDCP2_BASE0x66800 > +#define _PORTB_HDCP2_BASE0x66500 > +#define _PORTC_HDCP2_BASE0x66600 > +#define _PORTD_HDCP2_BASE0x66700 > +#define _PORTE_HDCP2_BASE0x66A00 > +#define _PORTF_HDCP2_BASE0x66900 > +#define _PORT_HDCP2_BASE(port, x)_MMIO(_PICK((port), \ > + _PORTA_HDCP2_BASE, \ > + _PORTB_HDCP2_BASE, \ > + _PORTC_HDCP2_BASE, \ > + _PORTD_HDCP2_BASE, \ > + _PORTE_HDCP2_BASE, \ > + _PORTF_HDCP2_BASE) + (x)) > + > +#define HDCP2_AUTH_DDI(port) _PORT_HDCP2_BASE(port, 0x98) > +#define AUTH_LINK_AUTHENTICATEDBIT(31) > +#define AUTH_LINK_TYPE BIT(30) > +#define AUTH_FORCE_CLR_INPUTCTRBIT(19) > +#define AUTH_CLR_KEYS BIT(18) > + > +#define HDCP2_CTL_DDI(port) _PORT_HDCP2_BASE(port, 0xB0) > +#define CTL_LINK_ENCRYPTION_REQBIT(31) > + > +#define HDCP2_STATUS_DDI(port) _PORT_HDCP2_BASE(port, 0xB4) > +#define STREAM_ENCRYPTION_STATUS_A BIT(31) > +#define STREAM_ENCRYPTION_STATUS_B BIT(30) > +#define STREAM_ENCRYPTION_STATUS_C BIT(29) > +#define LINK_TYPE_STATUS BIT(22) > +#define LINK_AUTH_STATUS BIT(21) > +#define LINK_ENCRYPTION_STATUS BIT(20) > + > /* Per-pipe DDI Function Control */ > #define _TRANS_DDI_FUNC_CTL_A0x60400 > #define _TRANS_DDI_FUNC_CTL_B0x61400 > -- > 2.7.4 > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v5 06/40] drm/i915: Define HDCP2.2 related variables
On Wed, Jun 27, 2018 at 02:09:55PM +0530, Ramalingam C wrote: > For upcoming implementation of HDCP2.2 in I915, important variable > required for HDCP2.2 are defined. Please just introduce them when you use them. I can't provide useful review on this patch unless I can see how the variables are used. This will also reduce the series size, which is an added bonus for reviewers :-) Sean > > HDCP_shim is extended to support encoder specific HDCP2.2 flows. > > v2: > 1.4 shim is extended to support hdcp2.2. [Sean Paul] > platform's/panel's hdcp ver capability is removed. [Sean Paul] > mei references in i915_private are moved to later patches. [Chris Wilson] > v3: > mei_cl_device ref is moved into intel_hdcp > v4: > Extra * in comment is removed [Uma] > v5: > No Change. > > Signed-off-by: Ramalingam C > --- > drivers/gpu/drm/i915/intel_drv.h | 61 > > 1 file changed, 61 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index eb480574a92e..b615ea4a44c3 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -29,6 +29,7 @@ > #include > #include > #include > +#include > #include > #include "i915_drv.h" > #include > @@ -375,6 +376,32 @@ struct intel_hdcp_shim { > /* Detects panel's hdcp capability. This is optional for HDMI. */ > int (*hdcp_capable)(struct intel_digital_port *intel_dig_port, > bool *hdcp_capable); > + > + /* Write HDCP2.2 messages */ > + int (*write_2_2_msg)(struct intel_digital_port *intel_dig_port, > + void *buf, size_t size); > + > + /* Read HDCP2.2 messages */ > + int (*read_2_2_msg)(struct intel_digital_port *intel_dig_port, > + uint8_t msg_id, void *buf, size_t size); > + > + /* > + * Implementation of DP HDCP2.2 Errata for the communication of stream > + * type to Receivers. In DP HDCP2.2 Stream type is one of the input to > + * the HDCP2.2 Chiper for En/De-Cryption. Not applicable for HDMI. > + */ > + int (*config_stream_type)(struct intel_digital_port *intel_dig_port, > + void *buf, size_t size); > + > + /* HDCP2.2 Link Integrity Check */ > + int (*check_2_2_link)(struct intel_digital_port *intel_dig_port); > + > + /* Detects whether Panel is HDCP2.2 capable */ > + int (*hdcp_2_2_capable)(struct intel_digital_port *intel_dig_port, > + bool *capable); > + > + /* Detects the HDCP protocol(DP/HDMI) required on the port */ > + enum hdcp_protocol (*hdcp_protocol)(void); > }; > > struct intel_hdcp { > @@ -384,6 +411,40 @@ struct intel_hdcp { > uint64_t hdcp_value; > struct delayed_work hdcp_check_work; > struct work_struct hdcp_prop_work; > + > + /* HDCP2.2 related definitions */ > + bool hdcp2_supported; > + > + /* > + * Content Stream Type defined by content owner. TYPE0(0x0) content can > + * flow in the link protected by HDCP2.2 or HDCP1.4, where as TYPE1(0x1) > + * content can flow only through a link protected by HDCP2.2. > + */ > + u8 content_type; > + > + bool is_paired; > + bool is_repeater; > + > + /* > + * Count of ReceiverID_List received. Initialized to 0 at AKE_INIT. > + * Incremented after processing the RepeaterAuth_Send_ReceiverID_List. > + * When it rolls over re-auth has to be triggered. > + */ > + uint32_t seq_num_v; > + > + /* > + * Count of RepeaterAuth_Stream_Manage msg propagated. > + * Initialized to 0 on AKE_INIT. Incremented after every successful > + * transmission of RepeaterAuth_Stream_Manage message. When it rolls > + * over re-Auth has to be triggered. > + */ > + uint32_t seq_num_m; > + > + /* mei interface related information */ > + struct mei_cl_device *cldev; > + struct mei_hdcp_data mei_data; > + > + struct delayed_work hdcp2_check_work; > }; > > struct intel_connector { > -- > 2.7.4 > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 05/40] drm/i915: wrapping all hdcp var into intel_hdcp
On Wed, Jun 27, 2018 at 02:09:54PM +0530, Ramalingam C wrote: > Considering significant number of HDCP specific variables, it will > be clean to have separate struct for HDCP. > > New structure called intel_hdcp is added within intel_connector. > > v2: > struct hdcp statically allocated. [Sean Paul] > enable and disable function parameters are retained.[Sean Paul] > v3: > No Changes. > v4: > Commit msg is rephrased [Uma] > v5: > Comment for mutex definition. > > Signed-off-by: Ramalingam C > --- > drivers/gpu/drm/i915/intel_display.c | 7 +-- > drivers/gpu/drm/i915/intel_drv.h | 15 -- > drivers/gpu/drm/i915/intel_hdcp.c| 94 > > 3 files changed, 67 insertions(+), 49 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 3d849ec17f5c..ef09bb89d2ca 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -15856,9 +15856,10 @@ static void intel_hpd_poll_fini(struct drm_device > *dev) > for_each_intel_connector_iter(connector, _iter) { > if (connector->modeset_retry_work.func) > cancel_work_sync(>modeset_retry_work); > - if (connector->hdcp_shim) { > - cancel_delayed_work_sync(>hdcp_check_work); > - cancel_work_sync(>hdcp_prop_work); > + if (connector->hdcp.hdcp_shim) { > + cancel_delayed_work_sync( > + >hdcp.hdcp_check_work); > + cancel_work_sync(>hdcp.hdcp_prop_work); > } > } > drm_connector_list_iter_end(_iter); > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index 578346b8d7e2..eb480574a92e 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -377,6 +377,15 @@ struct intel_hdcp_shim { > bool *hdcp_capable); > }; > > +struct intel_hdcp { > + const struct intel_hdcp_shim *hdcp_shim; > + /* Mutex for hdcp state of the connector */ > + struct mutex hdcp_mutex; > + uint64_t hdcp_value; > + struct delayed_work hdcp_check_work; > + struct work_struct hdcp_prop_work; Now that they're in their own struct, you can probably drop the hdcp_* prefix from the struct members. > +}; > + > struct intel_connector { > struct drm_connector base; > /* > @@ -409,11 +418,7 @@ struct intel_connector { > /* Work struct to schedule a uevent on link train failure */ > struct work_struct modeset_retry_work; > > - const struct intel_hdcp_shim *hdcp_shim; > - struct mutex hdcp_mutex; > - uint64_t hdcp_value; /* protected by hdcp_mutex */ > - struct delayed_work hdcp_check_work; > - struct work_struct hdcp_prop_work; > + struct intel_hdcp hdcp; > }; > > struct intel_digital_connector_state { > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c > b/drivers/gpu/drm/i915/intel_hdcp.c > index 0cc6a861bcf8..65bbe5874eee 100644 > --- a/drivers/gpu/drm/i915/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/intel_hdcp.c > @@ -626,6 +626,7 @@ struct intel_digital_port *conn_to_dig_port(struct > intel_connector *connector) > > static int _intel_hdcp_disable(struct intel_connector *connector) > { > + struct intel_hdcp *hdcp = >hdcp; > struct drm_i915_private *dev_priv = connector->base.dev->dev_private; > struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector); > enum port port = intel_dig_port->base.port; > @@ -641,7 +642,7 @@ static int _intel_hdcp_disable(struct intel_connector > *connector) > return -ETIMEDOUT; > } > > - ret = connector->hdcp_shim->toggle_signalling(intel_dig_port, false); > + ret = hdcp->hdcp_shim->toggle_signalling(intel_dig_port, false); > if (ret) { > DRM_ERROR("Failed to disable HDCP signalling\n"); > return ret; > @@ -653,6 +654,7 @@ static int _intel_hdcp_disable(struct intel_connector > *connector) > > static int _intel_hdcp_enable(struct intel_connector *connector) > { > + struct intel_hdcp *hdcp = >hdcp; > struct drm_i915_private *dev_priv = connector->base.dev->dev_private; > int i, ret, tries = 3; > > @@ -678,7 +680,7 @@ static int _intel_hdcp_enable(struct intel_connector > *connector) > /* Incase of authentication failures, HDCP spec expects reauth. */ > for (i = 0; i < tries; i++) { > ret = intel_hdcp_auth(conn_to_dig_port(connector), > - connector->hdcp_shim); > + hdcp->hdcp_shim); > if (!ret) > return 0; > > @@ -694,36 +696,42 @@ static int _intel_hdcp_enable(struct intel_connector > *connector) > > static void intel_hdcp_check_work(struct work_struct *work) > { > - struct intel_connector
Re: [Intel-gfx] [PATCH v5 02/40] drm: HDMI and DP specific HDCP2.2 defines
On Wed, Jun 27, 2018 at 02:09:51PM +0530, Ramalingam C wrote: > This patch adds HDCP register definitions for HDMI and DP HDCP > adaptations. > > HDMI specific HDCP2.2 register definitions are added into drm_hdcp.h, > where as HDCP2.2 register offsets in DPCD offsets are defined at > drm_dp_helper.h. > > v2: > bit_field definitions are replaced by macros. [Tomas and Jani] > v3: > No Changes. > v4: > Comments style and typos are fixed [Uma] > v5: > Fix for macros. > > Signed-off-by: Ramalingam C > --- > include/drm/drm_dp_helper.h | 51 > + > include/drm/drm_hdcp.h | 30 ++ > 2 files changed, 81 insertions(+) > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > index c01564991a9f..17e0889d6aaa 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -904,6 +904,57 @@ > #define DP_AUX_HDCP_KSV_FIFO 0x6802C > #define DP_AUX_HDCP_AINFO0x6803B > > +/* DP HDCP2.2 parameter offsets in DPCD address space */ > +#define DP_HDCP_2_2_REG_RTX_OFFSET 0x69000 > +#define DP_HDCP_2_2_REG_TXCAPS_OFFSET0x69008 > +#define DP_HDCP_2_2_REG_CERT_RX_OFFSET 0x6900B > +#define DP_HDCP_2_2_REG_RRX_OFFSET 0x69215 > +#define DP_HDCP_2_2_REG_RX_CAPS_OFFSET 0x6921D > +#define DP_HDCP_2_2_REG_EKPUB_KM_OFFSET 0x69220 > +#define DP_HDCP_2_2_REG_EKH_KM_OFFSET0x692A0 > +#define DP_HDCP_2_2_REG_M_OFFSET 0x692B0 > +#define DP_HDCP_2_2_REG_HPRIME_OFFSET0x692C0 > +#define DP_HDCP_2_2_REG_EKH_KM_RD_OFFSET 0x692E0 > +#define DP_HDCP_2_2_REG_RN_OFFSET0x692F0 > +#define DP_HDCP_2_2_REG_LPRIME_OFFSET0x692F8 > +#define DP_HDCP_2_2_REG_EDKEY_KS_OFFSET 0x69318 > +#define DP_HDCP_2_2_REG_RIV_OFFSET 0x69328 > +#define DP_HDCP_2_2_REG_RXINFO_OFFSET0x69330 > +#define DP_HDCP_2_2_REG_SEQ_NUM_V_OFFSET 0x69332 > +#define DP_HDCP_2_2_REG_VPRIME_OFFSET0x69335 > +#define DP_HDCP_2_2_REG_RECV_ID_LIST_OFFSET 0x69345 > +#define DP_HDCP_2_2_REG_V_OFFSET 0x693E0 > +#define DP_HDCP_2_2_REG_SEQ_NUM_M_OFFSET 0x693F0 > +#define DP_HDCP_2_2_REG_K_OFFSET 0x693F3 > +#define DP_HDCP_2_2_REG_STREAM_ID_TYPE_OFFSET0x693F5 > +#define DP_HDCP_2_2_REG_MPRIME_OFFSET0x69473 > +#define DP_HDCP_2_2_REG_RXSTATUS_OFFSET 0x69493 > +#define DP_HDCP_2_2_REG_STREAM_TYPE_OFFSET 0x69494 > +#define DP_HDCP_2_2_REG_DBG_OFFSET 0x69518 > + > +/* DP HDCP message start offsets in DPCD address space */ > +#define DP_HDCP_2_2_AKE_INIT_OFFSET DP_HDCP_2_2_REG_RTX_OFFSET > +#define DP_HDCP_2_2_AKE_SEND_CERT_OFFSET DP_HDCP_2_2_REG_CERT_RX_OFFSET > +#define DP_HDCP_2_2_AKE_NO_STORED_KM_OFFSET DP_HDCP_2_2_REG_EKPUB_KM_OFFSET > +#define DP_HDCP_2_2_AKE_STORED_KM_OFFSET DP_HDCP_2_2_REG_EKH_KM_OFFSET > +#define DP_HDCP_2_2_AKE_SEND_HPRIME_OFFSET DP_HDCP_2_2_REG_HPRIME_OFFSET > +#define DP_HDCP_2_2_AKE_SEND_PAIRING_INFO_OFFSET \ > + DP_HDCP_2_2_REG_EKH_KM_RD_OFFSET > +#define DP_HDCP_2_2_LC_INIT_OFFSET DP_HDCP_2_2_REG_RN_OFFSET > +#define DP_HDCP_2_2_LC_SEND_LPRIME_OFFSETDP_HDCP_2_2_REG_LPRIME_OFFSET > +#define DP_HDCP_2_2_SKE_SEND_EKS_OFFSET > DP_HDCP_2_2_REG_EDKEY_KS_OFFSET > +#define DP_HDCP_2_2_REP_SEND_RECVID_LIST_OFFSET > DP_HDCP_2_2_REG_RXINFO_OFFSET > +#define DP_HDCP_2_2_REP_SEND_ACK_OFFSET DP_HDCP_2_2_REG_V_OFFSET > +#define DP_HDCP_2_2_REP_STREAM_MANAGE_OFFSET DP_HDCP_2_2_REG_SEQ_NUM_M_OFFSET > +#define DP_HDCP_2_2_REP_STREAM_READY_OFFSET DP_HDCP_2_2_REG_MPRIME_OFFSET > + > +#define HDCP_2_2_DP_RXSTATUS_LEN 1 > +#define HDCP_2_2_DP_RXSTATUS_READY(x)((x) & BIT(0)) > +#define HDCP_2_2_DP_RXSTATUS_H_PRIME(x) ((x) & BIT(1)) > +#define HDCP_2_2_DP_RXSTATUS_PAIRING(x) ((x) & BIT(2)) > +#define HDCP_2_2_DP_RXSTATUS_REAUTH_REQ(x) ((x) & BIT(3)) > +#define HDCP_2_2_DP_RXSTATUS_LINK_FAILED(x) ((x) & BIT(4)) > + > /* DP 1.2 Sideband message defines */ > /* peer device type - DP 1.2a Table 2-92 */ > #define DP_PEER_DEVICE_NONE 0x0 > diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h > index 3e963c5d04b2..2fc6311dc060 100644 > --- a/include/drm/drm_hdcp.h > +++ b/include/drm/drm_hdcp.h > @@ -217,4 +217,34 @@ struct hdcp2_dp_errata_stream_type { > uint8_t stream_type; > } __packed; > > +/* HDCP2.2 TIMEOUTs in mSec */ Minor nit: it's usually better to add _MS postfix to the var names so it's obvious at the point of use what unit the #define is in. Otherwise, Reviewed-by: Sean Paul > +#define HDCP_2_2_CERT_TIMEOUT100 > +#define HDCP_2_2_HPRIME_NO_PAIRED_TIMEOUT1000 > +#define HDCP_2_2_HPRIME_PAIRED_TIMEOUT
Re: [Intel-gfx] [PATCH v5 01/40] drm: hdcp2.2 authentication msg definitions
On Wed, Jun 27, 2018 at 02:09:50PM +0530, Ramalingam C wrote: > This patch defines the hdcp2.2 protocol messages for authentication. > > v2: > bit_fields are removed. Instead bitmasking used. [Tomas and Jani] > prefix HDCP_2_2_ is added to the macros. [Tomas] > v3: > No Changes. > v4: > Style and spellings are fixed [Uma] > v5: > Fix for macros. > > Signed-off-by: Ramalingam C > --- > include/drm/drm_hdcp.h | 179 > + > 1 file changed, 179 insertions(+) > > diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h > index 98e63d870139..3e963c5d04b2 100644 > --- a/include/drm/drm_hdcp.h > +++ b/include/drm/drm_hdcp.h > @@ -38,4 +38,183 @@ > #define DRM_HDCP_DDC_BSTATUS 0x41 > #define DRM_HDCP_DDC_KSV_FIFO0x43 > > +#define DRM_HDCP_1_4_SRM_ID 0x8 > +#define DRM_HDCP_1_4_VRL_LENGTH_SIZE 3 > +#define DRM_HDCP_1_4_DCP_SIG_SIZE40 These don't seem to be related to the patch? > + > +/* Protocol message definition for HDCP2.2 specification */ > +#define HDCP_STREAM_TYPE00x00 > +#define HDCP_STREAM_TYPE10x01 Why not HDCP_2_2 prefix? > + > +/* HDCP2.2 Msg IDs */ > +#define HDCP_2_2_NULL_MSG1 > +#define HDCP_2_2_AKE_INIT2 > +#define HDCP_2_2_AKE_SEND_CERT 3 > +#define HDCP_2_2_AKE_NO_STORED_KM4 > +#define HDCP_2_2_AKE_STORED_KM 5 > +#define HDCP_2_2_AKE_SEND_HPRIME 7 > +#define HDCP_2_2_AKE_SEND_PAIRING_INFO 8 > +#define HDCP_2_2_LC_INIT 9 > +#define HDCP_2_2_LC_SEND_LPRIME 10 > +#define HDCP_2_2_SKE_SEND_EKS11 > +#define HDCP_2_2_REP_SEND_RECVID_LIST12 > +#define HDCP_2_2_REP_SEND_ACK15 > +#define HDCP_2_2_REP_STREAM_MANAGE 16 > +#define HDCP_2_2_REP_STREAM_READY17 > +#define HDCP_2_2_ERRATA_DP_STREAM_TYPE 50 > + > +#define HDCP_2_2_RTX_LEN 8 > +#define HDCP_2_2_RRX_LEN 8 > + > +#define HDCP_2_2_K_PUB_RX_MOD_N_LEN 128 > +#define HDCP_2_2_K_PUB_RX_EXP_E_LEN 3 > +#define HDCP_2_2_K_PUB_RX_LEN > (HDCP_2_2_K_PUB_RX_MOD_N_LEN + \ > + HDCP_2_2_K_PUB_RX_EXP_E_LEN) > + > +#define HDCP_2_2_DCP_LLC_SIG_LEN 384 > + > +#define HDCP_2_2_E_KPUB_KM_LEN 128 > +#define HDCP_2_2_E_KH_KM_M_LEN (16 + 16) > +#define HDCP_2_2_H_PRIME_LEN 32 > +#define HDCP_2_2_E_KH_KM_LEN 16 > +#define HDCP_2_2_RN_LEN 8 > +#define HDCP_2_2_L_PRIME_LEN 32 > +#define HDCP_2_2_E_DKEY_KS_LEN 16 > +#define HDCP_2_2_RIV_LEN 8 > +#define HDCP_2_2_SEQ_NUM_LEN 3 > +#define HDCP_2_2_LPRIME_HALF_LEN (HDCP_2_2_L_PRIME_LEN / 2) > +#define HDCP_2_2_RECEIVER_ID_LEN DRM_HDCP_KSV_LEN > +#define HDCP_2_2_MAX_DEVICE_COUNT31 > +#define HDCP_2_2_RECEIVER_IDS_MAX_LEN > (HDCP_2_2_RECEIVER_ID_LEN * \ > + HDCP_2_2_MAX_DEVICE_COUNT) > +#define HDCP_2_2_MPRIME_LEN 32 > + > +/* Following Macros take a byte at a time for bit(s) masking */ > +/* > + * TODO: This has to be changed for DP MST, as multiple stream on > + * same port is possible. > + * For HDCP2.2 on HDMI and DP SST this value is always 1. > + */ > +#define HDCP_2_2_MAX_CONTENT_STREAMS_CNT 1 > +#define HDCP_2_2_TXCAP_MASK_LEN 2 > +#define HDCP_2_2_RXCAPS_LEN 3 > +#define HDCP_2_2_RX_REPEATER(x) ((x) & BIT(0)) > +#define HDCP_2_2_DP_HDCP_CAPABLE(x) ((x) & BIT(1)) > +#define HDCP_2_2_RXINFO_LEN 2 > + > +/* HDCP1.x compliant device in downstream */ > +#define HDCP_2_2_HDCP1_DEVICE_CONNECTED(x) ((x) & BIT(0)) > + > +/* HDCP2.0 Compliant repeater in downstream */ > +#define HDCP_2_2_HDCP_2_0_REP_CONNECTED(x) ((x) & BIT(1)) > +#define HDCP_2_2_MAX_CASCADE_EXCEEDED(x) ((x) & BIT(2)) > +#define HDCP_2_2_MAX_DEVS_EXCEEDED(x)((x) & BIT(3)) > +#define HDCP_2_2_DEV_COUNT_LO(x) (((x) & (0xF << 4)) >> 4) > +#define HDCP_2_2_DEV_COUNT_HI(x) ((x) & BIT(0)) > +#define HDCP_2_2_DEPTH(x)(((x) & (0x7 << 1)) >> 1) > + > +struct hdcp2_cert_rx { > + uint8_t receiver_id[HDCP_2_2_RECEIVER_ID_LEN]; > + uint8_t kpub_rx[HDCP_2_2_K_PUB_RX_LEN]; > + uint8_t reserved[2]; > + uint8_t dcp_signature[HDCP_2_2_DCP_LLC_SIG_LEN]; > +} __packed; > + > +struct hdcp2_streamid_type { > + uint8_t stream_id; > + uint8_t stream_type; > +} __packed; > + > +/* > + * The TxCaps field specified in the HDCP HDMI, DP specs
Re: [PATCH 21/21] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file
On Mon, Jul 9, 2018 at 2:35 PM, Sean Paul wrote: > On Mon, Jul 09, 2018 at 12:07:11PM -0600, Rob Herring wrote: >> On Mon, Jul 9, 2018 at 11:40 AM Sean Paul wrote: >> > >> > Signed-off-by: Sean Paul >> > --- >> > arch/arm64/boot/dts/qcom/sdm845.dtsi | 194 +++ >> > 1 file changed, 194 insertions(+) >> > >> > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi >> > b/arch/arm64/boot/dts/qcom/sdm845.dtsi >> > index cdaabeb3c995..339afed856de 100644 >> > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi >> > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi >> > @@ -5,6 +5,8 @@ >> > * Copyright (c) 2018, The Linux Foundation. All rights reserved. >> > */ >> > >> > +#include >> > +#include >> > #include >> > >> > / { >> > @@ -221,6 +223,198 @@ >> > #interrupt-cells = <2>; >> > }; >> > >> > + mdss: mdss@ae0 { >> > + compatible = "qcom,dpu-mdss"; >> > + reg = <0xae0 0x1000>; >> > + reg-names = "mdss_phys"; >> > + >> > + power-domains = < 0>; >> > + >> > + clocks = < GCC_DISP_AHB_CLK>, >> > +< GCC_DISP_AXI_CLK>, >> > +< DISP_CC_MDSS_MDP_CLK>; >> > + clock-names = "iface", "bus", "core"; >> > + clock-frequency = <0 0 3>; >> > + >> > + interrupts = ; >> > + interrupt-controller; >> > + #interrupt-cells = <1>; >> > + >> > + /* iommus = <_iommu 0>; */ >> > + >> > + #address-cells = <1>; >> > + #size-cells = <1>; >> > + ranges; >> > + >> > + mdss_mdp: mdp@ae01000 { >> > + compatible = "qcom,dpu"; >> > > Hi Rob, > Thanks for the quick turnaround! In addition to below, I'll also beef up the > commit message, since I forgot to add any description of the change. > > >> Needs an SoC specific compatible. Did this binding get reviewed? >> > > No, it's part of this set ([PATCH 19/21] dt-bindings: msm/disp: Add bindings > for > Snapdragon 845 DPU). note that for display controller and DSI we've been able to reliably read the version of the block from hardware. BR, -R > >> > + reg = <0x0ae01000 0x8f000>, >> > + <0x0aeb 0x2008>; >> > + reg-names = "mdp_phys", "vbif_phys"; >> > + >> > + clocks = < DISP_CC_MDSS_AHB_CLK>, >> > +< DISP_CC_MDSS_AXI_CLK>, >> > +< DISP_CC_MDSS_MDP_CLK>, >> > +< DISP_CC_MDSS_VSYNC_CLK>; >> > + clock-names = "iface", "bus", "core", >> > "vsync"; >> > + clock-frequency = <0 0 3 1920>; >> >> That's abusing clock-frequency which is generally 1 value. Use >> assigned-clock-rates instead. >> > > Thanks, will change. > >> > + >> > + interrupt-parent = <>; >> > + interrupts = <0 IRQ_TYPE_LEVEL_HIGH>; >> > + >> > + ports { >> > + #address-cells = <1>; >> > + #size-cells = <0>; >> > + >> > + port@0 { >> > + reg = <0>; >> > + dpu_intf1_out: endpoint { >> > + remote-endpoint = >> > <_in>; >> > + }; >> > + }; >> > + >> > + port@1 { >> > + reg = <1>; >> > + dpu_intf2_out: endpoint { >> > + remote-endpoint = >> > <_in>; >> > + }; >> > + }; >> > + }; >> > + }; >> > + >> > + dsi0: dsi@ae94000 { >> > + compatible = "qcom,mdss-dsi-ctrl"; >> >> Needs an SoC specific compatible. >> > > Ok, will add. > >> > + reg = <0xae94000 0x400>; >> > + reg-names = "dsi_ctrl"; >> > + >> > + interrupt-parent = <>; >> > + interrupts = <4 0>; >> > + >> > + clocks = < DISP_CC_MDSS_BYTE0_CLK>, >> > +< >> > DISP_CC_MDSS_BYTE0_INTF_CLK>,
Re: [PATCH 21/21] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file
On Mon, Jul 09, 2018 at 12:07:11PM -0600, Rob Herring wrote: > On Mon, Jul 9, 2018 at 11:40 AM Sean Paul wrote: > > > > Signed-off-by: Sean Paul > > --- > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 194 +++ > > 1 file changed, 194 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi > > b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > index cdaabeb3c995..339afed856de 100644 > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > @@ -5,6 +5,8 @@ > > * Copyright (c) 2018, The Linux Foundation. All rights reserved. > > */ > > > > +#include > > +#include > > #include > > > > / { > > @@ -221,6 +223,198 @@ > > #interrupt-cells = <2>; > > }; > > > > + mdss: mdss@ae0 { > > + compatible = "qcom,dpu-mdss"; > > + reg = <0xae0 0x1000>; > > + reg-names = "mdss_phys"; > > + > > + power-domains = < 0>; > > + > > + clocks = < GCC_DISP_AHB_CLK>, > > +< GCC_DISP_AXI_CLK>, > > +< DISP_CC_MDSS_MDP_CLK>; > > + clock-names = "iface", "bus", "core"; > > + clock-frequency = <0 0 3>; > > + > > + interrupts = ; > > + interrupt-controller; > > + #interrupt-cells = <1>; > > + > > + /* iommus = <_iommu 0>; */ > > + > > + #address-cells = <1>; > > + #size-cells = <1>; > > + ranges; > > + > > + mdss_mdp: mdp@ae01000 { > > + compatible = "qcom,dpu"; > Hi Rob, Thanks for the quick turnaround! In addition to below, I'll also beef up the commit message, since I forgot to add any description of the change. > Needs an SoC specific compatible. Did this binding get reviewed? > No, it's part of this set ([PATCH 19/21] dt-bindings: msm/disp: Add bindings for Snapdragon 845 DPU). > > + reg = <0x0ae01000 0x8f000>, > > + <0x0aeb 0x2008>; > > + reg-names = "mdp_phys", "vbif_phys"; > > + > > + clocks = < DISP_CC_MDSS_AHB_CLK>, > > +< DISP_CC_MDSS_AXI_CLK>, > > +< DISP_CC_MDSS_MDP_CLK>, > > +< DISP_CC_MDSS_VSYNC_CLK>; > > + clock-names = "iface", "bus", "core", > > "vsync"; > > + clock-frequency = <0 0 3 1920>; > > That's abusing clock-frequency which is generally 1 value. Use > assigned-clock-rates instead. > Thanks, will change. > > + > > + interrupt-parent = <>; > > + interrupts = <0 IRQ_TYPE_LEVEL_HIGH>; > > + > > + ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + port@0 { > > + reg = <0>; > > + dpu_intf1_out: endpoint { > > + remote-endpoint = > > <_in>; > > + }; > > + }; > > + > > + port@1 { > > + reg = <1>; > > + dpu_intf2_out: endpoint { > > + remote-endpoint = > > <_in>; > > + }; > > + }; > > + }; > > + }; > > + > > + dsi0: dsi@ae94000 { > > + compatible = "qcom,mdss-dsi-ctrl"; > > Needs an SoC specific compatible. > Ok, will add. > > + reg = <0xae94000 0x400>; > > + reg-names = "dsi_ctrl"; > > + > > + interrupt-parent = <>; > > + interrupts = <4 0>; > > + > > + clocks = < DISP_CC_MDSS_BYTE0_CLK>, > > +< > > DISP_CC_MDSS_BYTE0_INTF_CLK>, > > +< DISP_CC_MDSS_PCLK0_CLK>, > > +< DISP_CC_MDSS_ESC0_CLK>, > > +< DISP_CC_MDSS_AHB_CLK>, > > +< DISP_CC_MDSS_AXI_CLK>; > > +
[Bug 107065] "BUG: unable to handle kernel paging request at 0000000000002000" in amdgpu_vm_cpu_set_ptes at amdgpu_vm.c:921
https://bugs.freedesktop.org/show_bug.cgi?id=107065 --- Comment #18 from Andrey Grodzovsky --- (In reply to dwagner from comment #17) > Interesting observation: If I first switch from the X11 display to the > console display (with Alt-F2), and then enter "echo mem >/sys/power/state" > on the console, above described crashes upon S3 resume do not occur, and I > do not see the "[TTM] Buffer eviction failed" in the kernel log, neither > with vm_update_mode=0, nor with vm_update_mode=3. > > Switching back to the X11 display after a successful S3 resume to the > console also works fine. > > What could be the relevant difference here? Well, there is no acceleration involved when in console mode. So maybe this has something to do with it. Anyway, i am sidetracked a bit by an internal requirement but once i finish I will get back to this issue especially because I got another report with the same failure as you describe. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm/i915/intel_dsi: Read back pclk set by GOP and use that as pclk
On Sat, Jul 07, 2018 at 08:32:16AM +0200, Hans de Goede wrote: > Hi, > > On 07/06/2018 04:16 PM, Ville Syrjälä wrote: > > On Tue, Jun 19, 2018 at 10:18:27PM +0200, Hans de Goede wrote: > >> On BYT and CHT the GOP sometimes initializes the pclk at a (slightly) > >> different frequency then the pclk which we've calculated. > >> > >> This commit makes the DSI code read-back the pclk set by the GOP and > >> if that is within a reasonable margin of the calculated pclk, uses > >> that instead. > >> > >> This fixes the first modeset being a full modeset instead of a > >> fast modeset on systems where the GOP pclk is different. > >> > >> Signed-off-by: Hans de Goede > >> --- > >> drivers/gpu/drm/i915/intel_dsi_vbt.c | 14 ++ > >> 1 file changed, 14 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c > >> b/drivers/gpu/drm/i915/intel_dsi_vbt.c > >> index 4d6ffa7b3e7b..d4cc6099012c 100644 > >> --- a/drivers/gpu/drm/i915/intel_dsi_vbt.c > >> +++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c > >> @@ -517,6 +517,7 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, > >> u16 panel_id) > >>u32 mul; > >>u16 burst_mode_ratio; > >>enum port port; > >> + enum pipe pipe; > >> > >>DRM_DEBUG_KMS("\n"); > >> > >> @@ -583,6 +584,19 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, > >> u16 panel_id) > >>} else > >>burst_mode_ratio = 100; > >> > >> + /* > >> + * On BYT / CRC the GOP sometimes picks a slightly different pclk, > >> + * read back the GOP configured pclk and prefer it over ours. > >> + */ > >> + if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) && > >> + intel_dsi_get_hw_state(_dsi->base, )) { > >> + u32 gop = intel_dsi_get_pclk(_dsi->base, bpp, NULL); > >> + > >> + DRM_DEBUG_KMS("Calculated pclk %d GOP %d\n", pclk, gop); > >> + if (gop >= (pclk * 9 / 10) && gop <= (pclk * 11 / 10)) > >> + pclk = gop; > >> + } > > > > Is the gop acually picking a different clock that what we pick in the > > end, or is it just that the value in the vbt doesn't quite match what we > > (and the gop) end up using on account of limitations of the pll? > > I *think* the GOP is picking a different clock, IIRC (*) it is something like > 90Mhz for the GOP and the VBT says 87Mhz (and our calculations leave it > unmodified. With this patch which puts pclk at 90Mhz on the specific > tablet I developed this on, the PLL settings calculated by our PLL code > end up being exactly the same as the once chosen by the GOP once we have > the pclk set to 90MHz. > > Note I've seen these small (and sometimes somewhat bigger) differences > between GOP and VBT pclk on a lot of devices, not just the one tablet > I developed it on. Since submitting this I've run this on at least > 5 different CHT/BYT devices and it works as advertised so far. > > > For that particular problem I think I had patches long ago to go through > > the pll computation during init so that we basically fix up the slightly > > bogus clock from the vbt. > > We do end up with a slightly different clock then the 87MHhz when going > though the PLL calculations, something like 86.33MHz or some such from > the top of my head, but the problem is not the pclk not matching the > intel_pipe_config_compare() function does not look at it, it looks at > dsi_pll.ctrl dsi_pll.div and those don't match, where as they do match > if we fixup the VBT clock to be the one confgured by the GOP. > > > Any kind of hack that involves reading out the hardware state should go > > into something like intel_sanitize_encoder(). Actually by that time we > > have already read out the hw state, so it shouldn't require any > > modifications to the existing dsi code itself. > > I do not think that intel_sanitize encoder is the right place to do this: > > * I don't want to modify the read-back state, I want to modify our >calculated "new/ideal" state to match the read-back state I wasn't suggesting that. What I meant is that you already have the state there to look so you don't have to hack the readout functions to function without a state being around. That said, we do already have intel_encoder_current_mode() which is doing something similar to what you're proposing. So probably should just try to reuse that. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107153] 4.18-rc3 crash on hdmi (0010:dm_update_crtcs_state+0x41e/0x4a0)
https://bugs.freedesktop.org/show_bug.cgi?id=107153 --- Comment #5 from Patrik Kullman --- For reference, they seem to reference the same line and is vanilla compared to Ubuntu-kernels: https://github.com/torvalds/linux/blob/v4.18-rc3/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c#L4784 https://github.com/torvalds/linux/blob/v4.18-rc4/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c#L4829 (According to the bug line references) -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 21/21] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file
On Mon, Jul 9, 2018 at 11:40 AM Sean Paul wrote: > > Signed-off-by: Sean Paul > --- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 194 +++ > 1 file changed, 194 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi > b/arch/arm64/boot/dts/qcom/sdm845.dtsi > index cdaabeb3c995..339afed856de 100644 > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > @@ -5,6 +5,8 @@ > * Copyright (c) 2018, The Linux Foundation. All rights reserved. > */ > > +#include > +#include > #include > > / { > @@ -221,6 +223,198 @@ > #interrupt-cells = <2>; > }; > > + mdss: mdss@ae0 { > + compatible = "qcom,dpu-mdss"; > + reg = <0xae0 0x1000>; > + reg-names = "mdss_phys"; > + > + power-domains = < 0>; > + > + clocks = < GCC_DISP_AHB_CLK>, > +< GCC_DISP_AXI_CLK>, > +< DISP_CC_MDSS_MDP_CLK>; > + clock-names = "iface", "bus", "core"; > + clock-frequency = <0 0 3>; > + > + interrupts = ; > + interrupt-controller; > + #interrupt-cells = <1>; > + > + /* iommus = <_iommu 0>; */ > + > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + > + mdss_mdp: mdp@ae01000 { > + compatible = "qcom,dpu"; Needs an SoC specific compatible. Did this binding get reviewed? > + reg = <0x0ae01000 0x8f000>, > + <0x0aeb 0x2008>; > + reg-names = "mdp_phys", "vbif_phys"; > + > + clocks = < DISP_CC_MDSS_AHB_CLK>, > +< DISP_CC_MDSS_AXI_CLK>, > +< DISP_CC_MDSS_MDP_CLK>, > +< DISP_CC_MDSS_VSYNC_CLK>; > + clock-names = "iface", "bus", "core", "vsync"; > + clock-frequency = <0 0 3 1920>; That's abusing clock-frequency which is generally 1 value. Use assigned-clock-rates instead. > + > + interrupt-parent = <>; > + interrupts = <0 IRQ_TYPE_LEVEL_HIGH>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + dpu_intf1_out: endpoint { > + remote-endpoint = > <_in>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + dpu_intf2_out: endpoint { > + remote-endpoint = > <_in>; > + }; > + }; > + }; > + }; > + > + dsi0: dsi@ae94000 { > + compatible = "qcom,mdss-dsi-ctrl"; Needs an SoC specific compatible. > + reg = <0xae94000 0x400>; > + reg-names = "dsi_ctrl"; > + > + interrupt-parent = <>; > + interrupts = <4 0>; > + > + clocks = < DISP_CC_MDSS_BYTE0_CLK>, > +< > DISP_CC_MDSS_BYTE0_INTF_CLK>, > +< DISP_CC_MDSS_PCLK0_CLK>, > +< DISP_CC_MDSS_ESC0_CLK>, > +< DISP_CC_MDSS_AHB_CLK>, > +< DISP_CC_MDSS_AXI_CLK>; > + clock-names = "byte_clk", > + "byte_intf_clk", > + "pixel_clk", > + "core_clk", > + "iface_clk", > + "bus_clk"; Should have found this in binding review, but the "_clk" part is redundant. > + > + phys = <_phy>; > + phy-names = "dsi-phy"; > + > + #address-cells =
Re: [Intel-gfx] [PATCH 11/12] sched: use for_each_if in topology.h
On Mon, Jul 9, 2018 at 6:12 PM, Mark Rutland wrote: > On Mon, Jul 09, 2018 at 06:03:42PM +0200, Peter Zijlstra wrote: >> On Mon, Jul 09, 2018 at 05:52:04PM +0200, Daniel Vetter wrote: >> > for_each_something(foo) >> > if (foo->bla) >> > call_bla(foo); >> > else >> > call_default(foo); >> > >> > Totally contrived, but this complains. Liberally sprinkling {} also shuts >> > up the compiler, but it's a bit confusing given that a plain for {;;} is >> > totally fine. And it's confusing since at first glance the compiler >> > complaining about nested if and ambigous else doesn't make sense since >> > clearly there's only 1 if there. >> >> Ah, so the pattern the compiler tries to warn about is: >> >> if (foo) >> if (bar) >> /* stmts1 */ >> else >> /* stmts2 * >> >> Because it might not be immediately obvious with which if the else goes. >> Which is fair enough I suppose. >> >> OK, ACK. > > Just to bikeshed, there could be macros other than for_each_*() macros > that will want to use this internally, so perhaps it would be worth the > generic version being named something like if_noelse(). > > We could always add that as/when required, though. I think a better name would be really good, but both when we added it for i915 and when we move it to drm headers we drew a blank. if_noelse() describes pretty good what it does, but kinda fails on the "where should I use it" departement. If there's some consensus I can sed the patches quickly. -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: [PATCH 4/4] drm/i915/intel_dsi: Read back pclk set by GOP and use that as pclk
Hi, On 07/09/2018 07:37 PM, Rodrigo Vivi wrote: On Sat, Jul 07, 2018 at 08:32:16AM +0200, Hans de Goede wrote: Hi, On 07/06/2018 04:16 PM, Ville Syrjälä wrote: On Tue, Jun 19, 2018 at 10:18:27PM +0200, Hans de Goede wrote: On BYT and CHT the GOP sometimes initializes the pclk at a (slightly) different frequency then the pclk which we've calculated. This commit makes the DSI code read-back the pclk set by the GOP and if that is within a reasonable margin of the calculated pclk, uses that instead. This fixes the first modeset being a full modeset instead of a fast modeset on systems where the GOP pclk is different. Signed-off-by: Hans de Goede --- drivers/gpu/drm/i915/intel_dsi_vbt.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c b/drivers/gpu/drm/i915/intel_dsi_vbt.c index 4d6ffa7b3e7b..d4cc6099012c 100644 --- a/drivers/gpu/drm/i915/intel_dsi_vbt.c +++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c @@ -517,6 +517,7 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id) u32 mul; u16 burst_mode_ratio; enum port port; + enum pipe pipe; DRM_DEBUG_KMS("\n"); @@ -583,6 +584,19 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id) } else burst_mode_ratio = 100; + /* +* On BYT / CRC the GOP sometimes picks a slightly different pclk, +* read back the GOP configured pclk and prefer it over ours. +*/ + if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) && + intel_dsi_get_hw_state(_dsi->base, )) { + u32 gop = intel_dsi_get_pclk(_dsi->base, bpp, NULL); + + DRM_DEBUG_KMS("Calculated pclk %d GOP %d\n", pclk, gop); + if (gop >= (pclk * 9 / 10) && gop <= (pclk * 11 / 10)) + pclk = gop; + } Is the gop acually picking a different clock that what we pick in the end, or is it just that the value in the vbt doesn't quite match what we (and the gop) end up using on account of limitations of the pll? I *think* the GOP is picking a different clock, IIRC (*) it is something like 90Mhz for the GOP and the VBT says 87Mhz (and our calculations leave it unmodified. With this patch which puts pclk at 90Mhz on the specific tablet I developed this on, the PLL settings calculated by our PLL code end up being exactly the same as the once chosen by the GOP once we have the pclk set to 90MHz. Note I've seen these small (and sometimes somewhat bigger) differences between GOP and VBT pclk on a lot of devices, not just the one tablet I developed it on. Since submitting this I've run this on at least 5 different CHT/BYT devices and it works as advertised so far. It seems GOP is just not respecting VBT and using the whatever it judge the safest option?! Yes, that or it is using some unknown rounding mechanism, to get to a pclk which the pll can represent exactly. If I don't fixup the pclk in the VBT the actually set pclk is somewhat different then the one originally in the VBT as the PLL can not generate an exact match. Probably not optimal, but I believe we should stay on the safest side as well, if this is the case. Agreed. Regards, Hans For that particular problem I think I had patches long ago to go through the pll computation during init so that we basically fix up the slightly bogus clock from the vbt. We do end up with a slightly different clock then the 87MHhz when going though the PLL calculations, something like 86.33MHz or some such from the top of my head, but the problem is not the pclk not matching the intel_pipe_config_compare() function does not look at it, it looks at dsi_pll.ctrl dsi_pll.div and those don't match, where as they do match if we fixup the VBT clock to be the one confgured by the GOP. Any kind of hack that involves reading out the hardware state should go into something like intel_sanitize_encoder(). Actually by that time we have already read out the hw state, so it shouldn't require any modifications to the existing dsi code itself. I do not think that intel_sanitize encoder is the right place to do this: * I don't want to modify the read-back state, I want to modify our calculated "new/ideal" state to match the read-back state * I don't want to directly modify our calculated new/ideal state, instead I want to cleanup / sanitize the values we got from the VBT so that the initial-modeset *and* any future modesets will use the GOP pclk. I believe it is important that if we're going to use the GOP pclk we use it for all modesets consistently. * I only want to do this once, at boot when we are sure the mode was set by the GOP and not after suspend/resume when we don't know if the GOP will have touched things or not. * It is DSI specific, whereas sofar intel_sanitize_encoder seems to not contain any output specific code. Regards, Hans + intel_dsi->burst_mode_ratio =
[PATCH 21/21] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file
Signed-off-by: Sean Paul --- arch/arm64/boot/dts/qcom/sdm845.dtsi | 194 +++ 1 file changed, 194 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index cdaabeb3c995..339afed856de 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -5,6 +5,8 @@ * Copyright (c) 2018, The Linux Foundation. All rights reserved. */ +#include +#include #include / { @@ -221,6 +223,198 @@ #interrupt-cells = <2>; }; + mdss: mdss@ae0 { + compatible = "qcom,dpu-mdss"; + reg = <0xae0 0x1000>; + reg-names = "mdss_phys"; + + power-domains = < 0>; + + clocks = < GCC_DISP_AHB_CLK>, +< GCC_DISP_AXI_CLK>, +< DISP_CC_MDSS_MDP_CLK>; + clock-names = "iface", "bus", "core"; + clock-frequency = <0 0 3>; + + interrupts = ; + interrupt-controller; + #interrupt-cells = <1>; + + /* iommus = <_iommu 0>; */ + + #address-cells = <1>; + #size-cells = <1>; + ranges; + + mdss_mdp: mdp@ae01000 { + compatible = "qcom,dpu"; + reg = <0x0ae01000 0x8f000>, + <0x0aeb 0x2008>; + reg-names = "mdp_phys", "vbif_phys"; + + clocks = < DISP_CC_MDSS_AHB_CLK>, +< DISP_CC_MDSS_AXI_CLK>, +< DISP_CC_MDSS_MDP_CLK>, +< DISP_CC_MDSS_VSYNC_CLK>; + clock-names = "iface", "bus", "core", "vsync"; + clock-frequency = <0 0 3 1920>; + + interrupt-parent = <>; + interrupts = <0 IRQ_TYPE_LEVEL_HIGH>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dpu_intf1_out: endpoint { + remote-endpoint = <_in>; + }; + }; + + port@1 { + reg = <1>; + dpu_intf2_out: endpoint { + remote-endpoint = <_in>; + }; + }; + }; + }; + + dsi0: dsi@ae94000 { + compatible = "qcom,mdss-dsi-ctrl"; + reg = <0xae94000 0x400>; + reg-names = "dsi_ctrl"; + + interrupt-parent = <>; + interrupts = <4 0>; + + clocks = < DISP_CC_MDSS_BYTE0_CLK>, +< DISP_CC_MDSS_BYTE0_INTF_CLK>, +< DISP_CC_MDSS_PCLK0_CLK>, +< DISP_CC_MDSS_ESC0_CLK>, +< DISP_CC_MDSS_AHB_CLK>, +< DISP_CC_MDSS_AXI_CLK>; + clock-names = "byte_clk", + "byte_intf_clk", + "pixel_clk", + "core_clk", + "iface_clk", + "bus_clk"; + + phys = <_phy>; + phy-names = "dsi-phy"; + + #address-cells = <1>; + #size-cells = <0>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dsi0_in: endpoint { + remote-endpoint = <_intf1_out>; + }; +
Re: [PATCH 4/4] drm/i915/intel_dsi: Read back pclk set by GOP and use that as pclk
On Sat, Jul 07, 2018 at 08:32:16AM +0200, Hans de Goede wrote: > Hi, > > On 07/06/2018 04:16 PM, Ville Syrjälä wrote: > > On Tue, Jun 19, 2018 at 10:18:27PM +0200, Hans de Goede wrote: > > > On BYT and CHT the GOP sometimes initializes the pclk at a (slightly) > > > different frequency then the pclk which we've calculated. > > > > > > This commit makes the DSI code read-back the pclk set by the GOP and > > > if that is within a reasonable margin of the calculated pclk, uses > > > that instead. > > > > > > This fixes the first modeset being a full modeset instead of a > > > fast modeset on systems where the GOP pclk is different. > > > > > > Signed-off-by: Hans de Goede > > > --- > > > drivers/gpu/drm/i915/intel_dsi_vbt.c | 14 ++ > > > 1 file changed, 14 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c > > > b/drivers/gpu/drm/i915/intel_dsi_vbt.c > > > index 4d6ffa7b3e7b..d4cc6099012c 100644 > > > --- a/drivers/gpu/drm/i915/intel_dsi_vbt.c > > > +++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c > > > @@ -517,6 +517,7 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, > > > u16 panel_id) > > > u32 mul; > > > u16 burst_mode_ratio; > > > enum port port; > > > + enum pipe pipe; > > > DRM_DEBUG_KMS("\n"); > > > @@ -583,6 +584,19 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, > > > u16 panel_id) > > > } else > > > burst_mode_ratio = 100; > > > + /* > > > + * On BYT / CRC the GOP sometimes picks a slightly different pclk, > > > + * read back the GOP configured pclk and prefer it over ours. > > > + */ > > > + if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) && > > > + intel_dsi_get_hw_state(_dsi->base, )) { > > > + u32 gop = intel_dsi_get_pclk(_dsi->base, bpp, NULL); > > > + > > > + DRM_DEBUG_KMS("Calculated pclk %d GOP %d\n", pclk, gop); > > > + if (gop >= (pclk * 9 / 10) && gop <= (pclk * 11 / 10)) > > > + pclk = gop; > > > + } > > > > Is the gop acually picking a different clock that what we pick in the > > end, or is it just that the value in the vbt doesn't quite match what we > > (and the gop) end up using on account of limitations of the pll? > > I *think* the GOP is picking a different clock, IIRC (*) it is something like > 90Mhz for the GOP and the VBT says 87Mhz (and our calculations leave it > unmodified. With this patch which puts pclk at 90Mhz on the specific > tablet I developed this on, the PLL settings calculated by our PLL code > end up being exactly the same as the once chosen by the GOP once we have > the pclk set to 90MHz. > > Note I've seen these small (and sometimes somewhat bigger) differences > between GOP and VBT pclk on a lot of devices, not just the one tablet > I developed it on. Since submitting this I've run this on at least > 5 different CHT/BYT devices and it works as advertised so far. It seems GOP is just not respecting VBT and using the whatever it judge the safest option?! Probably not optimal, but I believe we should stay on the safest side as well, if this is the case. > > > For that particular problem I think I had patches long ago to go through > > the pll computation during init so that we basically fix up the slightly > > bogus clock from the vbt. > > We do end up with a slightly different clock then the 87MHhz when going > though the PLL calculations, something like 86.33MHz or some such from > the top of my head, but the problem is not the pclk not matching the > intel_pipe_config_compare() function does not look at it, it looks at > dsi_pll.ctrl dsi_pll.div and those don't match, where as they do match > if we fixup the VBT clock to be the one confgured by the GOP. > > > Any kind of hack that involves reading out the hardware state should go > > into something like intel_sanitize_encoder(). Actually by that time we > > have already read out the hw state, so it shouldn't require any > > modifications to the existing dsi code itself. > > I do not think that intel_sanitize encoder is the right place to do this: > > * I don't want to modify the read-back state, I want to modify our > calculated "new/ideal" state to match the read-back state > * I don't want to directly modify our calculated new/ideal state, > instead I want to cleanup / sanitize the values we got from the VBT > so that the initial-modeset *and* any future modesets will use the > GOP pclk. I believe it is important that if we're going to use the > GOP pclk we use it for all modesets consistently. > * I only want to do this once, at boot when we are sure the mode was > set by the GOP and not after suspend/resume when we don't know if the > GOP will have touched things or not. > * It is DSI specific, whereas sofar intel_sanitize_encoder seems to > not contain any output specific code. > > Regards, > > Hans > > > > > > > > + > > > intel_dsi->burst_mode_ratio = burst_mode_ratio; > > >
[PATCH 19/21] dt-bindings: msm/disp: Add bindings for Snapdragon 845 DPU
From: Jeykumar Sankaran Adds bindings for Snapdragon 845 display processing unit Signed-off-by: Jeykumar Sankaran Signed-off-by: Rajesh Yadav Signed-off-by: Sean Paul --- .../devicetree/bindings/display/msm/dpu.txt | 128 ++ 1 file changed, 128 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/msm/dpu.txt diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt new file mode 100644 index ..080fb77624a3 --- /dev/null +++ b/Documentation/devicetree/bindings/display/msm/dpu.txt @@ -0,0 +1,128 @@ +Qualcomm Technologies, Inc. DPU KMS + +Description: + +Device tree bindings for MSM Mobile Display Subsytem(MDSS) that encapsulates +sub-blocks like DPU display controller, DSI and DP interfaces etc. +The DPU display controller is found in SDM845 SoC. + +MDSS: +Required properties: +- compatible: "qcom,dpu-mdss" +- reg: physical base address and length of contoller's registers. +- reg-names: register region names. The following region is required: + * "mdss_phys" +- power-domains: a power domain consumer specifier according to + Documentation/devicetree/bindings/power/power_domain.txt +- clocks: list of phandles for clock device nodes needed by the device. +- clock-names: device clock names, must be in same order as clocks property. + The following clocks are required: + * "iface" + * "bus" + * "core" +- interrupts: interrupt signal from MDSS. +- interrupt-controller: identifies the node as an interrupt controller. +- #interrupt-cells: specifies the number of cells needed to encode an interrupt + source, should be 1. +- iommus: phandle of iommu device node. +- #address-cells: number of address cells for the MDSS children. Should be 1. +- #size-cells: Should be 1. +- ranges: parent bus address space is the same as the child bus address space. + +Optional properties: +- clock-frequency: list of clock frequencies sorted in the same order as the + clocks property. + +MDP: +Required properties: +- compatible: "qcom,dpu" +- reg: physical base address and length of controller's registers. +- reg-names : register region names. The following region is required: + * "mdp_phys" + * "vbif_phys" +- clocks: list of phandles for clock device nodes needed by the device. +- clock-names: device clock names, must be in same order as clocks property. + The following clocks are required. + * "bus" + * "iface" + * "core" + * "vsync" +- interrupt-parent: phandle to MDSS block. +- interrupts: interrupt line from DPU to MDSS. +- ports: contains the list of output ports from DPU device. These ports connect + to interfaces that are external to the DPU hardware, such as DSI, DP etc. + + Each output port contains an endpoint that describes how it is connected to an + external interface. These are described by the standard properties documented + here: + Documentation/devicetree/bindings/graph.txt + Documentation/devicetree/bindings/media/video-interfaces.txt + + Port 0 -> DPU_INTF1 (DSI1) + Port 1 -> DPU_INTF2 (DSI2) + +Optional properties: +- clock-frequency: list of clock frequencies sorted in the same order as the + clocks property. + +Example: + + mdss: mdss@ae0 { + compatible = "qcom,dpu-mdss"; + reg = <0xae0 0x1000>; + reg-names = "mdss_phys"; + + power-domains = <_dispcc 0>; + + clocks = < GCC_DISP_AHB_CLK>, +< GCC_DISP_AXI_CLK>, +<_dispcc DISP_CC_MDSS_MDP_CLK>; + clock-names = "iface", "bus", "core"; + clock-frequency = <0 0 3>; + + interrupts = ; + interrupt-controller; + #interrupt-cells = <1>; + + iommus = <_iommu 0>; + + #address-cells = <1>; + #size-cells = <1>; + ranges; + + mdss_mdp: mdp@ae01000 { + compatible = "qcom,dpu"; + reg = <0x0ae01000 0x8f000>, + <0x0aeb 0x2008>; + reg-names = "mdp_phys", "vbif_phys"; + + clocks = <_dispcc DISP_CC_MDSS_AHB_CLK>, +<_dispcc DISP_CC_MDSS_AXI_CLK>, +<_dispcc DISP_CC_MDSS_MDP_CLK>, +<_dispcc DISP_CC_MDSS_VSYNC_CLK>; + clock-names = "iface", "bus", "core", "vsync"; + clock-frequency = <0 0 3 1920>; + + interrupt-parent = <>; + interrupts = <0 IRQ_TYPE_LEVEL_HIGH>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; +
[PATCH 17/21] drm/msm: Add preclose kms hook
From: Jeykumar Sankaran This is needed by the dpu driver Signed-off-by: Jeykumar Sankaran [seanpaul split from the dpu megapatch] Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/msm_drv.c | 9 + drivers/gpu/drm/msm/msm_kms.h | 1 + 2 files changed, 10 insertions(+) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 8bd9fe831968..ed6efebabc38 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -540,6 +540,14 @@ static void context_close(struct msm_file_private *ctx) kfree(ctx); } +static void msm_preclose(struct drm_device *dev, struct drm_file *file) +{ + struct msm_drm_private *priv = dev->dev_private; + struct msm_kms *kms = priv->kms; + + if (kms && kms->funcs && kms->funcs->preclose) + kms->funcs->preclose(kms, file); +} static void msm_postclose(struct drm_device *dev, struct drm_file *file) { struct msm_drm_private *priv = dev->dev_private; @@ -860,6 +868,7 @@ static struct drm_driver msm_driver = { DRIVER_ATOMIC | DRIVER_MODESET, .open = msm_open, + .preclose = msm_preclose, .postclose = msm_postclose, .lastclose = drm_fb_helper_lastclose, .irq_handler= msm_irq, diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h index 761bb07cd7bf..9cd7223febcf 100644 --- a/drivers/gpu/drm/msm/msm_kms.h +++ b/drivers/gpu/drm/msm/msm_kms.h @@ -58,6 +58,7 @@ struct msm_kms_funcs { struct drm_encoder *encoder, struct drm_encoder *slave_encoder, bool is_cmd_mode); + void (*preclose)(struct msm_kms *kms, struct drm_file *file); void (*set_encoder_mode)(struct msm_kms *kms, struct drm_encoder *encoder, bool cmd_mode); -- 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 18/21] drm/msm: Add pm_suspend/resume callbacks to msm_kms
From: Jeykumar Sankaran Used by the dpu driver for custom suspend/resume. Signed-off-by: Jeykumar Sankaran [seanpaul split this out of the megapatch] Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/msm_drv.c | 10 ++ drivers/gpu/drm/msm/msm_kms.h | 3 +++ 2 files changed, 13 insertions(+) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index ed6efebabc38..cd0959783203 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -912,6 +912,11 @@ static int msm_pm_suspend(struct device *dev) { struct drm_device *ddev = dev_get_drvdata(dev); struct msm_drm_private *priv = ddev->dev_private; + struct msm_kms *kms = priv->kms; + + /* TODO: Use atomic helper suspend/resume */ + if (kms && kms->funcs && kms->funcs->pm_suspend) + return kms->funcs->pm_suspend(dev); drm_kms_helper_poll_disable(ddev); @@ -928,6 +933,11 @@ static int msm_pm_resume(struct device *dev) { struct drm_device *ddev = dev_get_drvdata(dev); struct msm_drm_private *priv = ddev->dev_private; + struct msm_kms *kms = priv->kms; + + /* TODO: Use atomic helper suspend/resume */ + if (kms && kms->funcs && kms->funcs->pm_resume) + return kms->funcs->pm_resume(dev); drm_atomic_helper_resume(ddev, priv->pm_state); drm_kms_helper_poll_enable(ddev); diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h index 9cd7223febcf..36201f43fa31 100644 --- a/drivers/gpu/drm/msm/msm_kms.h +++ b/drivers/gpu/drm/msm/msm_kms.h @@ -62,6 +62,9 @@ struct msm_kms_funcs { void (*set_encoder_mode)(struct msm_kms *kms, struct drm_encoder *encoder, bool cmd_mode); + /* pm suspend/resume hooks */ + int (*pm_suspend)(struct device *dev); + int (*pm_resume)(struct device *dev); /* cleanup: */ void (*destroy)(struct msm_kms *kms); #ifdef CONFIG_DEBUG_FS -- 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 16/21] drm/msm: Add .commit() callback to msm_kms functions
From: Jeykumar Sankaran Called right before wait_for_commit_done() to perform kickoff for active crtcs. Signed-off-by: Jeykumar Sankaran [seanpaul split this out of the megapatch] Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/msm_atomic.c | 5 + drivers/gpu/drm/msm/msm_kms.h| 1 + 2 files changed, 6 insertions(+) diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c index e6f1e25c60af..c1f1779c980f 100644 --- a/drivers/gpu/drm/msm/msm_atomic.c +++ b/drivers/gpu/drm/msm/msm_atomic.c @@ -71,6 +71,11 @@ void msm_atomic_commit_tail(struct drm_atomic_state *state) drm_atomic_helper_commit_modeset_enables(dev, state); + if (kms->funcs->commit) { + DRM_DEBUG_ATOMIC("triggering commit\n"); + kms->funcs->commit(kms, state); + } + msm_atomic_wait_for_commit_done(dev, state); kms->funcs->complete_commit(kms, state); diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h index 76c14221ffdf..761bb07cd7bf 100644 --- a/drivers/gpu/drm/msm/msm_kms.h +++ b/drivers/gpu/drm/msm/msm_kms.h @@ -42,6 +42,7 @@ struct msm_kms_funcs { void (*disable_vblank)(struct msm_kms *kms, struct drm_crtc *crtc); /* modeset, bracketing atomic_commit(): */ void (*prepare_commit)(struct msm_kms *kms, struct drm_atomic_state *state); + void (*commit)(struct msm_kms *kms, struct drm_atomic_state *state); void (*complete_commit)(struct msm_kms *kms, struct drm_atomic_state *state); /* functions to wait for atomic commit completed on each CRTC */ void (*wait_for_crtc_commit_done)(struct msm_kms *kms, -- 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 15/21] drm/msm: #define MAX_ in msm_drv.h
From: Jeykumar Sankaran dpu uses these elsewhere in the driver (in addition to increasing MAX_PLANES, that'll come later), so pull them out into #define. Signed-off-by: Jeykumar Sankaran [seanpaul pulled this out of the dpu megapatch] Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/msm_drv.h | 16 +++- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index fa0376b0f42b..3b206ae6423f 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -54,6 +54,12 @@ struct msm_fence_context; struct msm_gem_address_space; struct msm_gem_vma; +#define MAX_CRTCS 8 +#define MAX_PLANES 16 +#define MAX_ENCODERS 8 +#define MAX_BRIDGES8 +#define MAX_CONNECTORS 8 + struct msm_file_private { rwlock_t queuelock; struct list_head submitqueues; @@ -117,19 +123,19 @@ struct msm_drm_private { struct workqueue_struct *wq; unsigned int num_planes; - struct drm_plane *planes[16]; + struct drm_plane *planes[MAX_PLANES]; unsigned int num_crtcs; - struct drm_crtc *crtcs[8]; + struct drm_crtc *crtcs[MAX_CRTCS]; unsigned int num_encoders; - struct drm_encoder *encoders[8]; + struct drm_encoder *encoders[MAX_ENCODERS]; unsigned int num_bridges; - struct drm_bridge *bridges[8]; + struct drm_bridge *bridges[MAX_BRIDGES]; unsigned int num_connectors; - struct drm_connector *connectors[8]; + struct drm_connector *connectors[MAX_CONNECTORS]; /* Properties */ struct drm_property *plane_property[PLANE_PROP_MAX_NUM]; -- 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 14/21] drm/msm: Use labels for unwinding in the error path
From: Jeykumar Sankaran This simplifies cleanup, to make sure nothing drops out in case of error. Signed-off-by: Jeykumar Sankaran [seanpaul split out of dpu megapatch and renamed labels] Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/msm_drv.c | 44 +-- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 67816543a0d7..8bd9fe831968 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -372,19 +372,16 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) priv = kzalloc(sizeof(*priv), GFP_KERNEL); if (!priv) { - drm_dev_unref(ddev); - return -ENOMEM; + ret = -ENOMEM; + goto err_unref_drm_dev; } ddev->dev_private = priv; priv->dev = ddev; ret = mdp5_mdss_init(ddev); - if (ret) { - kfree(priv); - drm_dev_unref(ddev); - return ret; - } + if (ret) + goto err_free_priv; mdss = priv->mdss; @@ -399,17 +396,12 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) /* Bind all our sub-components: */ ret = component_bind_all(dev, ddev); - if (ret) { - if (mdss && mdss->funcs) - mdss->funcs->destroy(ddev); - kfree(priv); - drm_dev_unref(ddev); - return ret; - } + if (ret) + goto err_destroy_mdss; ret = msm_init_vram(ddev); if (ret) - goto fail; + goto err_msm_uninit; msm_gem_shrinker_init(ddev); @@ -435,7 +427,7 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) */ dev_err(dev, "failed to load kms\n"); ret = PTR_ERR(kms); - goto fail; + goto err_msm_uninit; } /* Enable normalization of plane zpos */ @@ -445,7 +437,7 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) ret = kms->funcs->hw_init(kms); if (ret) { dev_err(dev, "kms hw init failed: %d\n", ret); - goto fail; + goto err_msm_uninit; } } @@ -455,7 +447,7 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) ret = drm_vblank_init(ddev, priv->num_crtcs); if (ret < 0) { dev_err(dev, "failed to initialize vblank\n"); - goto fail; + goto err_msm_uninit; } if (kms) { @@ -464,13 +456,13 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) pm_runtime_put_sync(dev); if (ret < 0) { dev_err(dev, "failed to install IRQ handler\n"); - goto fail; + goto err_msm_uninit; } } ret = drm_dev_register(ddev, 0); if (ret) - goto fail; + goto err_msm_uninit; drm_mode_config_reset(ddev); @@ -481,15 +473,23 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) ret = msm_debugfs_late_init(ddev); if (ret) - goto fail; + goto err_msm_uninit; drm_kms_helper_poll_init(ddev); return 0; -fail: +err_msm_uninit: msm_drm_uninit(dev); return ret; +err_destroy_mdss: + if (mdss && mdss->funcs) + mdss->funcs->destroy(ddev); +err_free_priv: + kfree(priv); +err_unref_drm_dev: + drm_dev_unref(ddev); + return ret; } /* -- 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 11/21] drm/msm: higher values of pclk can exceed 32 bits when multiplied by a factor
From: Abhinav Kumar Make the pclk_rate u64 to accommodate higher pixel clock rates. Changes in v4: - fixed commit message Signed-off-by: Abhinav Kumar Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/dsi/dsi_host.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 671039b7b75b..73587e731a23 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -669,7 +669,8 @@ static int dsi_calc_clk_rate(struct msm_dsi_host *msm_host, bool is_dual_dsi) const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd; u8 lanes = msm_host->lanes; u32 bpp = dsi_get_bpp(msm_host->format); - u32 pclk_rate; + u64 pclk_rate; + u64 pclk_bpp; if (!mode) { pr_err("%s: mode not set\n", __func__); @@ -689,13 +690,15 @@ static int dsi_calc_clk_rate(struct msm_dsi_host *msm_host, bool is_dual_dsi) if (is_dual_dsi) pclk_rate /= 2; + pclk_bpp = pclk_rate * bpp; if (lanes > 0) { - msm_host->byte_clk_rate = (pclk_rate * bpp) / (8 * lanes); + do_div(pclk_bpp, (8 * lanes)); } else { pr_err("%s: forcing mdss_dsi lanes to 1\n", __func__); - msm_host->byte_clk_rate = (pclk_rate * bpp) / 8; + do_div(pclk_bpp, 8); } msm_host->pixel_clk_rate = pclk_rate; + msm_host->byte_clk_rate = pclk_bpp; DBG("pclk=%d, bclk=%d", msm_host->pixel_clk_rate, msm_host->byte_clk_rate); -- 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 13/21] drm/msm: #define MDP version numbers
From: Jeykumar Sankaran Useful for incoming DPU support Signed-off-by: Jeykumar Sankaran [seanpaul split this from the dpu megapatch] Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/msm_drv.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index b73acdd52931..67816543a0d7 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -267,6 +267,9 @@ static int msm_drm_uninit(struct device *dev) return 0; } +#define KMS_MDP4 4 +#define KMS_MDP5 5 + static int get_mdp_ver(struct platform_device *pdev) { struct device *dev = >dev; @@ -411,11 +414,11 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) msm_gem_shrinker_init(ddev); switch (get_mdp_ver(pdev)) { - case 4: + case KMS_MDP4: kms = mdp4_kms_init(ddev); priv->kms = kms; break; - case 5: + case KMS_MDP5: kms = mdp5_kms_init(ddev); break; default: @@ -1162,8 +1165,8 @@ static int msm_pdev_remove(struct platform_device *pdev) } static const struct of_device_id dt_match[] = { - { .compatible = "qcom,mdp4", .data = (void *)4 }, /* MDP4 */ - { .compatible = "qcom,mdss", .data = (void *)5 }, /* MDP5 MDSS */ + { .compatible = "qcom,mdp4", .data = (void *)KMS_MDP4 }, + { .compatible = "qcom,mdss", .data = (void *)KMS_MDP5 }, {} }; MODULE_DEVICE_TABLE(of, dt_match); -- 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 12/21] drm/msm: Clean up dangling atomic_wq
I missed this during the atomic conversion Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/msm_drv.c | 4 drivers/gpu/drm/msm/msm_drv.h | 1 - 2 files changed, 5 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 9c760cee5156..b73acdd52931 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -244,9 +244,6 @@ static int msm_drm_uninit(struct device *dev) flush_workqueue(priv->wq); destroy_workqueue(priv->wq); - flush_workqueue(priv->atomic_wq); - destroy_workqueue(priv->atomic_wq); - if (kms && kms->funcs) kms->funcs->destroy(kms); @@ -389,7 +386,6 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) mdss = priv->mdss; priv->wq = alloc_ordered_workqueue("msm", 0); - priv->atomic_wq = alloc_ordered_workqueue("msm:atomic", 0); INIT_LIST_HEAD(>inactive_list); INIT_LIST_HEAD(>vblank_ctrl.event_list); diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index 17cefca1d566..fa0376b0f42b 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -115,7 +115,6 @@ struct msm_drm_private { struct list_head inactive_list; struct workqueue_struct *wq; - struct workqueue_struct *atomic_wq; unsigned int num_planes; struct drm_plane *planes[16]; -- 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 09/21] drm/msm/mdp5: subclass msm_mdss for mdp5
From: Rajesh Yadav SoCs having mdp5 or dpu have identical tree like device hierarchy where MDSS top level wrapper manages common power resources for all child devices. Subclass msm_mdss so that msm_mdss includes common defines and mdp5/dpu mdss derivations to include any extensions. Add mdss helper interface (msm_mdss_funcs) to msm_mdss base for mdp5/dpu mdss specific implementation calls. This change subclasses msm_mdss for mdp5, dpu specific changes will be done separately. Changes in v3: - none Changes in v2: - fixed indentation for irq_domain_add_linear call (Sean Paul) Signed-off-by: Rajesh Yadav Reviewed-by: Sean Paul [seanpaul rebased on msm-next and resolved conflicts] Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/disp/mdp5/mdp5_mdss.c | 154 -- drivers/gpu/drm/msm/msm_drv.c | 22 +++- drivers/gpu/drm/msm/msm_kms.h | 17 ++- 3 files changed, 109 insertions(+), 84 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_mdss.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_mdss.c index f2a0db7a8a03..1cc4e57f0226 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_mdss.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_mdss.c @@ -20,12 +20,10 @@ #include "msm_drv.h" #include "mdp5_kms.h" -/* - * If needed, this can become more specific: something like struct mdp5_mdss, - * which contains a 'struct msm_mdss base' member. - */ -struct msm_mdss { - struct drm_device *dev; +#define to_mdp5_mdss(x) container_of(x, struct mdp5_mdss, base) + +struct mdp5_mdss { + struct msm_mdss base; void __iomem *mmio, *vbif; @@ -41,22 +39,22 @@ struct msm_mdss { } irqcontroller; }; -static inline void mdss_write(struct msm_mdss *mdss, u32 reg, u32 data) +static inline void mdss_write(struct mdp5_mdss *mdp5_mdss, u32 reg, u32 data) { - msm_writel(data, mdss->mmio + reg); + msm_writel(data, mdp5_mdss->mmio + reg); } -static inline u32 mdss_read(struct msm_mdss *mdss, u32 reg) +static inline u32 mdss_read(struct mdp5_mdss *mdp5_mdss, u32 reg) { - return msm_readl(mdss->mmio + reg); + return msm_readl(mdp5_mdss->mmio + reg); } static irqreturn_t mdss_irq(int irq, void *arg) { - struct msm_mdss *mdss = arg; + struct mdp5_mdss *mdp5_mdss = arg; u32 intr; - intr = mdss_read(mdss, REG_MDSS_HW_INTR_STATUS); + intr = mdss_read(mdp5_mdss, REG_MDSS_HW_INTR_STATUS); VERB("intr=%08x", intr); @@ -64,7 +62,7 @@ static irqreturn_t mdss_irq(int irq, void *arg) irq_hw_number_t hwirq = fls(intr) - 1; generic_handle_irq(irq_find_mapping( - mdss->irqcontroller.domain, hwirq)); + mdp5_mdss->irqcontroller.domain, hwirq)); intr &= ~(1 << hwirq); } @@ -84,19 +82,19 @@ static irqreturn_t mdss_irq(int irq, void *arg) static void mdss_hw_mask_irq(struct irq_data *irqd) { - struct msm_mdss *mdss = irq_data_get_irq_chip_data(irqd); + struct mdp5_mdss *mdp5_mdss = irq_data_get_irq_chip_data(irqd); smp_mb__before_atomic(); - clear_bit(irqd->hwirq, >irqcontroller.enabled_mask); + clear_bit(irqd->hwirq, _mdss->irqcontroller.enabled_mask); smp_mb__after_atomic(); } static void mdss_hw_unmask_irq(struct irq_data *irqd) { - struct msm_mdss *mdss = irq_data_get_irq_chip_data(irqd); + struct mdp5_mdss *mdp5_mdss = irq_data_get_irq_chip_data(irqd); smp_mb__before_atomic(); - set_bit(irqd->hwirq, >irqcontroller.enabled_mask); + set_bit(irqd->hwirq, _mdss->irqcontroller.enabled_mask); smp_mb__after_atomic(); } @@ -109,13 +107,13 @@ static struct irq_chip mdss_hw_irq_chip = { static int mdss_hw_irqdomain_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hwirq) { - struct msm_mdss *mdss = d->host_data; + struct mdp5_mdss *mdp5_mdss = d->host_data; if (!(VALID_IRQS & (1 << hwirq))) return -EPERM; irq_set_chip_and_handler(irq, _hw_irq_chip, handle_level_irq); - irq_set_chip_data(irq, mdss); + irq_set_chip_data(irq, mdp5_mdss); return 0; } @@ -126,90 +124,99 @@ static const struct irq_domain_ops mdss_hw_irqdomain_ops = { }; -static int mdss_irq_domain_init(struct msm_mdss *mdss) +static int mdss_irq_domain_init(struct mdp5_mdss *mdp5_mdss) { - struct device *dev = mdss->dev->dev; + struct device *dev = mdp5_mdss->base.dev->dev; struct irq_domain *d; d = irq_domain_add_linear(dev->of_node, 32, _hw_irqdomain_ops, - mdss); + mdp5_mdss); if (!d) { dev_err(dev, "mdss irq domain add failed\n"); return -ENXIO; } - mdss->irqcontroller.enabled_mask = 0; - mdss->irqcontroller.domain = d; +
[PATCH 10/21] drm/msm: enable zpos normalization
From: Jeykumar Sankaran Enable drm core zpos normalization for planes. changes in v2: - none changes in v3: - rebased on https://gitlab.freedesktop.org/seanpaul/ dpu-staging/commit/481d29d31cd629fd216381b53de5695f645465d5 Signed-off-by: Jeykumar Sankaran Reviewed-by: Sean Paul Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/msm_drv.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 2608d3f77956..9c760cee5156 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -439,6 +439,9 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) goto fail; } + /* Enable normalization of plane zpos */ + ddev->mode_config.normalize_zpos = true; + if (kms) { ret = kms->funcs->hw_init(kms); if (ret) { -- 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 07/21] drm/msm/dsi: initialize postdiv_lock before use for 10nm pll
From: Rajesh Yadav postdiv_lock spinlock was used before initialization for 10nm pll. It causes following spin_bug: "BUG: spinlock bad magic on CPU#0". Initialize spinlock before its usage. Signed-off-by: Rajesh Yadav Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c index c4c37a7df637..4c03f0b7343e 100644 --- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c +++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c @@ -798,6 +798,8 @@ struct msm_dsi_pll *msm_dsi_pll_10nm_init(struct platform_device *pdev, int id) return ERR_PTR(-ENOMEM); } + spin_lock_init(_10nm->postdiv_lock); + pll = _10nm->base; pll->min_rate = 10UL; pll->max_rate = 35UL; -- 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 06/21] drm/msm/dsi: Use one connector for dual DSI mode
From: Chandan Uddaraju Current DSI driver uses two connectors for dual DSI case even though we only have one panel. Fix this by implementing one connector/bridge for dual DSI use case. Use master DSI controllers to register one connector/bridge. Changes in V2: -Removed Change-Id from the commit text tags. -Remove extra parentheses Changes in V3: -None Reviewed-by: Archit Taneja Signed-off-by: Chandan Uddaraju [seanpaul removed unused local var causing a build warning] Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/dsi/dsi.c | 3 + drivers/gpu/drm/msm/dsi/dsi.h | 1 + drivers/gpu/drm/msm/dsi/dsi_manager.c | 112 ++ 3 files changed, 30 insertions(+), 86 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c index b744bcc7d8ad..ff8164cc6738 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.c +++ b/drivers/gpu/drm/msm/dsi/dsi.c @@ -208,6 +208,9 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev, goto fail; } + if (!msm_dsi_manager_validate_current_config(msm_dsi->id)) + goto fail; + msm_dsi->encoder = encoder; msm_dsi->bridge = msm_dsi_manager_bridge_init(msm_dsi->id); diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h index 01c38f67d699..c858e8e1a5bd 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.h +++ b/drivers/gpu/drm/msm/dsi/dsi.h @@ -100,6 +100,7 @@ bool msm_dsi_manager_cmd_xfer_trigger(int id, u32 dma_base, u32 len); void msm_dsi_manager_attach_dsi_device(int id, u32 device_flags); int msm_dsi_manager_register(struct msm_dsi *msm_dsi); void msm_dsi_manager_unregister(struct msm_dsi *msm_dsi); +bool msm_dsi_manager_validate_current_config(u8 id); /* msm dsi */ static inline bool msm_dsi_device_connected(struct msm_dsi *msm_dsi) diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c index 3bb506b44a4b..000721fe5ab4 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c @@ -306,102 +306,25 @@ static void dsi_mgr_connector_destroy(struct drm_connector *connector) kfree(dsi_connector); } -static void dsi_dual_connector_fix_modes(struct drm_connector *connector) -{ - struct drm_display_mode *mode, *m; - - /* Only support left-right mode */ - list_for_each_entry_safe(mode, m, >probed_modes, head) { - mode->clock >>= 1; - mode->hdisplay >>= 1; - mode->hsync_start >>= 1; - mode->hsync_end >>= 1; - mode->htotal >>= 1; - drm_mode_set_name(mode); - } -} - -static int dsi_dual_connector_tile_init( - struct drm_connector *connector, int id) -{ - struct drm_display_mode *mode; - /* Fake topology id */ - char topo_id[8] = {'M', 'S', 'M', 'D', 'U', 'D', 'S', 'I'}; - - if (connector->tile_group) { - DBG("Tile property has been initialized"); - return 0; - } - - /* Use the first mode only for now */ - mode = list_first_entry(>probed_modes, - struct drm_display_mode, - head); - if (!mode) - return -EINVAL; - - connector->tile_group = drm_mode_get_tile_group( - connector->dev, topo_id); - if (!connector->tile_group) - connector->tile_group = drm_mode_create_tile_group( - connector->dev, topo_id); - if (!connector->tile_group) { - pr_err("%s: failed to create tile group\n", __func__); - return -ENOMEM; - } - - connector->has_tile = true; - connector->tile_is_single_monitor = true; - - /* mode has been fixed */ - connector->tile_h_size = mode->hdisplay; - connector->tile_v_size = mode->vdisplay; - - /* Only support left-right mode */ - connector->num_h_tile = 2; - connector->num_v_tile = 1; - - connector->tile_v_loc = 0; - connector->tile_h_loc = (id == DSI_RIGHT) ? 1 : 0; - - return 0; -} - static int dsi_mgr_connector_get_modes(struct drm_connector *connector) { int id = dsi_mgr_connector_get_id(connector); struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id); struct drm_panel *panel = msm_dsi->panel; - int ret, num; + int num; if (!panel) return 0; - /* Since we have 2 connectors, but only 1 drm_panel in dual DSI mode, -* panel should not attach to any connector. -* Only temporarily attach panel to the current connector here, -* to let panel set mode to this connector. + /* +* In dual DSI mode, we have one connector that can be +* attached to the drm_panel. */ drm_panel_attach(panel, connector); num =
[PATCH 08/21] drm/msm: Move wait_for_vblanks into mdp complete_commit() hooks
DPU doesn't use this, so push it into the mdp drivers. Signed-off-by: Sean Paul Signed-off-by: Rajesh Yadav --- drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 2 ++ drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 2 ++ drivers/gpu/drm/msm/msm_atomic.c | 2 -- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c index 4b646bf9c214..44d1cda56974 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c @@ -125,6 +125,8 @@ static void mdp4_complete_commit(struct msm_kms *kms, struct drm_atomic_state *s struct drm_crtc *crtc; struct drm_crtc_state *crtc_state; + drm_atomic_helper_wait_for_vblanks(mdp4_kms->dev, state); + /* see 119ecb7fd */ for_each_new_crtc_in_state(state, crtc, crtc_state, i) drm_crtc_vblank_put(crtc); diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c index 6e12e275deba..bddd625ab91b 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c @@ -170,6 +170,8 @@ static void mdp5_complete_commit(struct msm_kms *kms, struct drm_atomic_state *s struct device *dev = _kms->pdev->dev; struct mdp5_global_state *global_state; + drm_atomic_helper_wait_for_vblanks(mdp5_kms->dev, state); + global_state = mdp5_get_existing_global_state(mdp5_kms); if (mdp5_kms->smp) diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c index f0635c3da7f4..e6f1e25c60af 100644 --- a/drivers/gpu/drm/msm/msm_atomic.c +++ b/drivers/gpu/drm/msm/msm_atomic.c @@ -75,8 +75,6 @@ void msm_atomic_commit_tail(struct drm_atomic_state *state) kms->funcs->complete_commit(kms, state); - drm_atomic_helper_wait_for_vblanks(dev, state); - drm_atomic_helper_commit_hw_done(state); drm_atomic_helper_cleanup_planes(dev, state); -- 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 05/21] drm/msm/dsi: adjust dsi timing for dual dsi mode
From: Chandan Uddaraju For dual dsi mode, the horizontal timing needs to be divided by half since both the dsi controllers will be driving this panel. Adjust the pixel clock and DSI timing accordingly. Changes in V2: --Removed Change-Id from the commit text tags. Changes in V3: --Instead of adjusting the DRM mode structure, divide the clocks and horizontal timings in DSI host just before configuring the values. Signed-off-by: Chandan Uddaraju Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/dsi/dsi.h | 6 ++- drivers/gpu/drm/msm/dsi/dsi_host.c| 55 +-- drivers/gpu/drm/msm/dsi/dsi_manager.c | 7 ++-- 3 files changed, 52 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h index 70d9a9a47acd..01c38f67d699 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.h +++ b/drivers/gpu/drm/msm/dsi/dsi.h @@ -162,7 +162,8 @@ void msm_dsi_host_cmd_xfer_commit(struct mipi_dsi_host *host, int msm_dsi_host_enable(struct mipi_dsi_host *host); int msm_dsi_host_disable(struct mipi_dsi_host *host); int msm_dsi_host_power_on(struct mipi_dsi_host *host, - struct msm_dsi_phy_shared_timings *phy_shared_timings); + struct msm_dsi_phy_shared_timings *phy_shared_timings, + bool is_dual_dsi); int msm_dsi_host_power_off(struct mipi_dsi_host *host); int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host, struct drm_display_mode *mode); @@ -175,7 +176,8 @@ int msm_dsi_host_set_src_pll(struct mipi_dsi_host *host, struct msm_dsi_pll *src_pll); void msm_dsi_host_reset_phy(struct mipi_dsi_host *host); void msm_dsi_host_get_phy_clk_req(struct mipi_dsi_host *host, - struct msm_dsi_phy_clk_request *clk_req); + struct msm_dsi_phy_clk_request *clk_req, + bool is_dual_dsi); void msm_dsi_host_destroy(struct mipi_dsi_host *host); int msm_dsi_host_modeset_init(struct mipi_dsi_host *host, struct drm_device *dev); diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 2f1a2780658a..671039b7b75b 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -118,6 +118,7 @@ struct msm_dsi_host { struct clk *byte_intf_clk; u32 byte_clk_rate; + u32 pixel_clk_rate; u32 esc_clk_rate; /* DSI v2 specific clocks */ @@ -511,7 +512,7 @@ static int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host) goto error; } - ret = clk_set_rate(msm_host->pixel_clk, msm_host->mode->clock * 1000); + ret = clk_set_rate(msm_host->pixel_clk, msm_host->pixel_clk_rate); if (ret) { pr_err("%s: Failed to set rate pixel clk, %d\n", __func__, ret); goto error; @@ -592,7 +593,7 @@ static int dsi_link_clk_enable_v2(struct msm_dsi_host *msm_host) goto error; } - ret = clk_set_rate(msm_host->pixel_clk, msm_host->mode->clock * 1000); + ret = clk_set_rate(msm_host->pixel_clk, msm_host->pixel_clk_rate); if (ret) { pr_err("%s: Failed to set rate pixel clk, %d\n", __func__, ret); goto error; @@ -662,7 +663,7 @@ static void dsi_link_clk_disable(struct msm_dsi_host *msm_host) } } -static int dsi_calc_clk_rate(struct msm_dsi_host *msm_host) +static int dsi_calc_clk_rate(struct msm_dsi_host *msm_host, bool is_dual_dsi) { struct drm_display_mode *mode = msm_host->mode; const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd; @@ -676,14 +677,28 @@ static int dsi_calc_clk_rate(struct msm_dsi_host *msm_host) } pclk_rate = mode->clock * 1000; + + /* +* For dual DSI mode, the current DRM mode has +* the complete width of the panel. Since, the complete +* panel is driven by two DSI controllers, the +* the clock rates have to be split between +* the two dsi controllers. Adjust the byte and +* pixel clock rates for each dsi host accordingly. +*/ + if (is_dual_dsi) + pclk_rate /= 2; + if (lanes > 0) { msm_host->byte_clk_rate = (pclk_rate * bpp) / (8 * lanes); } else { pr_err("%s: forcing mdss_dsi lanes to 1\n", __func__); msm_host->byte_clk_rate = (pclk_rate * bpp) / 8; } + msm_host->pixel_clk_rate = pclk_rate; - DBG("pclk=%d, bclk=%d", pclk_rate, msm_host->byte_clk_rate); + DBG("pclk=%d, bclk=%d", msm_host->pixel_clk_rate, + msm_host->byte_clk_rate); msm_host->esc_clk_rate = clk_get_rate(msm_host->esc_clk); @@ -885,7 +900,7 @@ static void dsi_ctrl_config(struct msm_dsi_host *msm_host, bool enable, dsi_write(msm_host, REG_DSI_CTRL, data); } -static void dsi_timing_setup(struct
[PATCH 04/21] drm: add msm compressed format modifiers
From: Jeykumar Sankaran Qualcomm Snapdragon chipsets uses compressed format to optimize BW across multiple IP's. This change adds needed modifier support in drm for a simple 4x4 tile based compressed variants of base formats. Signed-off-by: Jeykumar Sankaran Signed-off-by: Sean Paul --- include/uapi/drm/drm_fourcc.h | 45 +++ 1 file changed, 45 insertions(+) diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index e04613d30a13..9a97405a3d2a 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -298,6 +298,38 @@ extern "C" { */ #define DRM_FORMAT_MOD_SAMSUNG_64_32_TILE fourcc_mod_code(SAMSUNG, 1) +/* + * Qualcomm Compressed Format + * + * Refers to a compressed variant of the base format that is compressed. + * Implementation may be platform and base-format specific. + */ +#define DRM_FORMAT_MOD_QCOM_COMPRESSED fourcc_mod_code(QCOM, 1) + +/* + * QTI DX Format + * + * Refers to a DX variant of the base format. + * Implementation may be platform and base-format specific. + */ +#define DRM_FORMAT_MOD_QCOM_DX fourcc_mod_code(QCOM, 0x2) + +/* + * QTI Tight Format + * + * Refers to a tightly packed variant of the base format. + * Implementation may be platform and base-format specific. + */ +#define DRM_FORMAT_MOD_QCOM_TIGHT fourcc_mod_code(QCOM, 0x4) + +/* + * QTI Tile Format + * + * Refers to a tile variant of the base format. + * Implementation may be platform and base-format specific. + */ +#define DRM_FORMAT_MOD_QCOM_TILE fourcc_mod_code(QCOM, 0x8) + /* Vivante framebuffer modifiers */ /* @@ -405,6 +437,19 @@ extern "C" { */ #define DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED fourcc_mod_code(BROADCOM, 1) +/* + * MSM compressed format + * + * Refers to the compressed variant of a base format. + * Implementation may be platform and base-format specific. + * + * Each macrotile consists of m x n (mostly 4 x 4) tiles. + * Pixel data pitch/stride is aligned with macrotile width. + * Pixel data height is aligned with macrotile height. + * Entire pixel data buffer is aligned with 4k(bytes). + */ +#define DRM_FORMAT_MOD_QCOM_COMPRESSED fourcc_mod_code(QCOM, 1) + #if defined(__cplusplus) } #endif -- 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 03/21] drm: Add support for pps and compression mode command packet
From: vkorjani After enabling DSC we need to send compression mode command packet and pps data packet, for which 2 new data types are added 07h Compression Mode Data Type Write , short write, 2 parameters 0Ah PPS Long Write (word count determines number of bytes) This patch adds support to send these packets. Cc: David Airlie Cc: Jean-Christophe Plagniol-Villard Cc: Tomi Valkeinen Cc: dri-devel@lists.freedesktop.org Cc: linux-ker...@vger.kernel.org Cc: linux-fb...@vger.kernel.org Signed-off-by: vkorjani [seanpaul removed pps_write_buffer fn, added types to packet_format helpers] Signed-off-by: Sean Paul --- drivers/gpu/drm/drm_mipi_dsi.c | 2 ++ include/video/mipi_display.h | 3 +++ 2 files changed, 5 insertions(+) diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c index bc73b7f5b9fc..80b75501f5c6 100644 --- a/drivers/gpu/drm/drm_mipi_dsi.c +++ b/drivers/gpu/drm/drm_mipi_dsi.c @@ -392,6 +392,7 @@ bool mipi_dsi_packet_format_is_short(u8 type) case MIPI_DSI_DCS_SHORT_WRITE: case MIPI_DSI_DCS_SHORT_WRITE_PARAM: case MIPI_DSI_DCS_READ: + case MIPI_DSI_DCS_COMPRESSION_MODE: case MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE: return true; } @@ -410,6 +411,7 @@ EXPORT_SYMBOL(mipi_dsi_packet_format_is_short); bool mipi_dsi_packet_format_is_long(u8 type) { switch (type) { + case MIPI_DSI_PPS_LONG_WRITE: case MIPI_DSI_NULL_PACKET: case MIPI_DSI_BLANKING_PACKET: case MIPI_DSI_GENERIC_LONG_WRITE: diff --git a/include/video/mipi_display.h b/include/video/mipi_display.h index 19aa65a35546..49a53ef8da96 100644 --- a/include/video/mipi_display.h +++ b/include/video/mipi_display.h @@ -38,6 +38,9 @@ enum { MIPI_DSI_DCS_READ = 0x06, + MIPI_DSI_DCS_COMPRESSION_MODE = 0x07, + MIPI_DSI_PPS_LONG_WRITE = 0x0A, + MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE = 0x37, MIPI_DSI_END_OF_TRANSMISSION= 0x08, -- 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 02/21] dt-bindings: clock: Introduce QCOM Display clock bindings
From: Taniya Das Add device tree bindings for display clock controller for Qualcomm Technology Inc's SDM845 SoCs. Signed-off-by: Taniya Das Reviewed-by: Rob Herring Signed-off-by: Sean Paul --- .../devicetree/bindings/clock/qcom,dispcc.txt | 19 .../dt-bindings/clock/qcom,dispcc-sdm845.h| 45 +++ 2 files changed, 64 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc.txt create mode 100644 include/dt-bindings/clock/qcom,dispcc-sdm845.h diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc.txt b/Documentation/devicetree/bindings/clock/qcom,dispcc.txt new file mode 100644 index ..d639e18d0b85 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc.txt @@ -0,0 +1,19 @@ +Qualcomm Technologies, Inc. Display Clock Controller Binding + + +Required properties : + +- compatible : shall contain "qcom,sdm845-dispcc" +- reg : shall contain base register location and length. +- #clock-cells : from common clock binding, shall contain 1. +- #reset-cells : from common reset binding, shall contain 1. +- #power-domain-cells : from generic power domain binding, shall contain 1. + +Example: + dispcc: clock-controller@af0 { + compatible = "qcom,sdm845-dispcc"; + reg = <0xaf0 0x10>; + #clock-cells = <1>; + #reset-cells = <1>; + #power-domain-cells = <1>; + }; diff --git a/include/dt-bindings/clock/qcom,dispcc-sdm845.h b/include/dt-bindings/clock/qcom,dispcc-sdm845.h new file mode 100644 index ..11eed4bc9646 --- /dev/null +++ b/include/dt-bindings/clock/qcom,dispcc-sdm845.h @@ -0,0 +1,45 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (c) 2018, The Linux Foundation. All rights reserved. + */ + +#ifndef _DT_BINDINGS_CLK_SDM_DISP_CC_SDM845_H +#define _DT_BINDINGS_CLK_SDM_DISP_CC_SDM845_H + +/* DISP_CC clock registers */ +#define DISP_CC_MDSS_AHB_CLK 0 +#define DISP_CC_MDSS_AXI_CLK 1 +#define DISP_CC_MDSS_BYTE0_CLK 2 +#define DISP_CC_MDSS_BYTE0_CLK_SRC 3 +#define DISP_CC_MDSS_BYTE0_INTF_CLK4 +#define DISP_CC_MDSS_BYTE1_CLK 5 +#define DISP_CC_MDSS_BYTE1_CLK_SRC 6 +#define DISP_CC_MDSS_BYTE1_INTF_CLK7 +#define DISP_CC_MDSS_ESC0_CLK 8 +#define DISP_CC_MDSS_ESC0_CLK_SRC 9 +#define DISP_CC_MDSS_ESC1_CLK 10 +#define DISP_CC_MDSS_ESC1_CLK_SRC 11 +#define DISP_CC_MDSS_MDP_CLK 12 +#define DISP_CC_MDSS_MDP_CLK_SRC 13 +#define DISP_CC_MDSS_MDP_LUT_CLK 14 +#define DISP_CC_MDSS_PCLK0_CLK 15 +#define DISP_CC_MDSS_PCLK0_CLK_SRC 16 +#define DISP_CC_MDSS_PCLK1_CLK 17 +#define DISP_CC_MDSS_PCLK1_CLK_SRC 18 +#define DISP_CC_MDSS_ROT_CLK 19 +#define DISP_CC_MDSS_ROT_CLK_SRC 20 +#define DISP_CC_MDSS_RSCC_AHB_CLK 21 +#define DISP_CC_MDSS_RSCC_VSYNC_CLK22 +#define DISP_CC_MDSS_VSYNC_CLK 23 +#define DISP_CC_MDSS_VSYNC_CLK_SRC 24 +#define DISP_CC_PLL0 25 +#define DISP_CC_MDSS_BYTE0_DIV_CLK_SRC 26 +#define DISP_CC_MDSS_BYTE1_DIV_CLK_SRC 27 + +/* DISP_CC Reset */ +#define DISP_CC_MDSS_RSCC_BCR 0 + +/* DISP_CC GDSCR */ +#define MDSS_GDSC 0 + +#endif -- 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 01/21] dt-bindings: msm/dsi: Add mdp transfer time to msm dsi binding
From: Jeykumar Sankaran Adds mdp transfer time to msm dsi binding Signed-off-by: Jeykumar Sankaran Signed-off-by: Rajesh Yadav Signed-off-by: Sean Paul --- .../devicetree/bindings/display/msm/dsi.txt | 16 1 file changed, 16 insertions(+) diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt index 518e9cdf0d4b..d22237a88eae 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi.txt +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt @@ -121,6 +121,20 @@ Required properties: Optional properties: - qcom,dsi-phy-regulator-ldo-mode: Boolean value indicating if the LDO mode PHY regulator is wanted. +- qcom,mdss-mdp-transfer-time-us: Specifies the dsi transfer time for command mode + panels in microseconds. Driver uses this number to adjust + the clock rate according to the expected transfer time. + Increasing this value would slow down the mdp processing + and can result in slower performance. + Decreasing this value can speed up the mdp processing, + but this can also impact power consumption. + As a rule this time should not be higher than the time + that would be expected with the processing at the + dsi link rate since anyways this would be the maximum + transfer time that could be achieved. + If ping pong split is enabled, this time should not be higher + than two times the dsi link rate time. + If the property is not specified, then the default value is 14000 us. [1] Documentation/devicetree/bindings/clock/clock-bindings.txt [2] Documentation/devicetree/bindings/graph.txt @@ -171,6 +185,8 @@ Example: qcom,master-dsi; qcom,sync-dual-dsi; + qcom,mdss-mdp-transfer-time-us = <12000>; + pinctrl-names = "default", "sleep"; pinctrl-0 = <_active>; pinctrl-1 = <_suspend>; -- 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 00/21] drm/msm: Add support for SDM845 Display Processing Unit (DPU)
Hello again, Well, here's the driver for QC SDM845 DPU support that I sent out in February, this time without the RFC safety net. We've been busy since then, here's what we've been up to! We've seen 184 unique patches from 8 people sent to the list to prepare the driver, here are the diffstat diffs: Diffstat from February: 236 files changed, 110234 insertions(+), 159 deletions(-) Diffstat from present day: 81 files changed, 32900 insertions(+), 237 deletions(-) So now you're wondering what was removed to get here, the breakdown of patches is listed below. The highlights are: - Remove dsi-staging driver, use upstream msm/dsi - Remove dp driver, will introduce soon - Remove writeback support, will re-introduce using upstream UAPI - Remove dpu_connector abstraction layer - Remove color processing/adaptive display features - Remove hdcp support, will re-introduce using upstream UAPI - Remove custom dpu ioctls and properties - A _bunch_ of other stuff that really adds up We've also de-duplicated a bunch of functionality: - dpu EDID parsing vs. drm_edid - dpu_rect vs. drm_rect - DPU_TRACE vs. tracepoints Finally, we've improved upstream msm in the following ways: - Converted to atomic helpers - Added dual-dsi support to msm/dsi - Added a few kms callbacks to allow finer grained control - Added mdss subclassing to share infrastructure between dpu/mdp5 The work is obviously not done, in addition to the re-introduction of features above, we still have a fair amount of clean up and polishing to do. All of that clean up (aside from the disp/event kthreads) are isolated to the dpu portion of the msm driver. We're also getting to the point where we want to add functionality back to the driver. For these reasons, I think now is a good time to land dpu in msm-next and work on the rest there. One note: "dt-bindings: clock: Introduce QCOM Display clock bindings" is included here since the DPU bindings depend on it. It has been posted and reviewed elsewhere, so it is simply here for completion. If you would like this in the form of a git tree, it is hosted at https://gitlab.freedesktop.org/seanpaul/dpu-staging/tree/for-next Finally, huge thanks to the display team at Qualcomm for diving into this headfirst, they've been awesome to work with. We've still got a long road ahead, but we've come far in little time. I'm looking forward to their continued contributions. Thanks for your consideration, Sean --- Breakdown of patches squashed into the "Add SDM845 DPU Support" patch: Abhinav Kumar (12): drm/msm/dsi: add only dsi nodes with a valid device to list drm/msm/dsi: check return value for video done waits drm/msm/dsi: check video mode engine status before waiting drm/msm/dsi: configure VCO rate for 10nm PLL driver drm/msm/dsi: implement auto PHY timing calculator for 10nm PHY drm/msm/dsi: set encoder mode for DRM bridge explicitly drm/msm: higher values of pclk can exceed 32 bits when multiplied by a factor drm/msm: make pclk_rate u64 to avoid truncation drm/msm: remove support for seamless modes drm/panel: Add support for Truly NT35597 panel drm/panel: add backlight control support for truly panel dt-bindings: Add Truly NT35597 panel bindings Archit Taneja (5): drm/msm/mdp5: Add global state as a private atomic object drm/msm/mdp5: Use the new private_obj state drm/msm: Don't subclass drm_atomic_state anymore drm/panel: Add Truly Dual DSI video mode panel drm/panel: Add Truly NT35597 panel Chandan Uddaraju (6): ARM: dts: msm: Add DPU node as a child of MDSS for SDM845 ARM: dts: msm: disable MDP/DSI driver present in "SDM845-dpu" ARM: dts: msm: fix panel gpio/regulator settings for SDM845 Fix GPIO/Regulator settings in DT for truly panel drm/msm/dsi: Use one connector for dual DSI mode drm/msm/dsi: adjust dsi timing for dual dsi mode Jeykumar Sankaran (35): Remove dpu crtc custom properties and its handlers drm/fourcc: add msm compressed format modifiers drm/msm/dpu: Fix writeback compile macros drm/msm/dpu: add atomic private object to dpu kms drm/msm/dpu: avoid querying for hw intf before assignment drm/msm/dpu: clean up dpu crtc custom properties drm/msm/dpu: clean up dpu plane custom properties drm/msm/dpu: iterate for assigned hw ctl in virtual encoder drm/msm/dpu: move hw resource tracking to crtc state drm/msm/dpu: program master-slave encoders explicitly drm/msm/dpu: remove display H_TILE from encoder drm/msm/dpu: remove ping pong split topology variables drm/msm/dpu: remove resource pool manager drm/msm/dpu: remove scalar config definitions drm/msm/dpu: remove stale encoder code drm/msm/dpu: remove topology name drm/msm/dpu: rename hw_ctl to lm_ctl drm/msm/dpu: switch to drm zpos property drm/msm/dpu: use kms stored hw mdp block
Re: [PATCH v3 0/2] gpu: drm/panel: Add DLC DLC0700YZG-1 support
On Wed, May 23, 2018 at 11:25:02AM +0200, Marco Felsch wrote: > This serie adds support for the DLC Display Co. DLC0700YZG-1 7.0" WSVGA > TFT LCD panel. The customer isn't listed as vendor so we have to add the > vendor prefix too. > > Philipp Zabel (2): > dt-bindings: Add vendor prefix for DLC Display Co., Ltd. > gpu: drm/panel: Add DLC DLC0700YZG-1 panel > > .../display/panel/dlc,dlc0700yzg-1.txt| 7 > .../devicetree/bindings/vendor-prefixes.txt | 1 + > drivers/gpu/drm/panel/panel-simple.c | 32 +++ > 3 files changed, 40 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/dlc,dlc0700yzg-1.txt Applied, thanks. Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] dt-bindings: display: Document the EDT et* displays in one file.
On Tue, Jun 19, 2018 at 11:55:43AM +0200, jan.tu...@emtrion.com wrote: > From: Jan Tuerk > > Document the Emerging Display Technology Corp. (EDT) using the > simple-panel binding in one single file. > > Reviewed-by: Rob Herring > Signed-off-by: Jan Tuerk > --- > .../bindings/display/panel/edt,et-series.txt | 39 +++ > .../display/panel/edt,et057090dhu.txt | 7 > .../display/panel/edt,et070080dh6.txt | 10 - > .../display/panel/edt,etm0700g0dh6.txt| 10 - > 4 files changed, 39 insertions(+), 27 deletions(-) > create mode 100644 > Documentation/devicetree/bindings/display/panel/edt,et-series.txt > delete mode 100644 > Documentation/devicetree/bindings/display/panel/edt,et057090dhu.txt > delete mode 100644 > Documentation/devicetree/bindings/display/panel/edt,et070080dh6.txt > delete mode 100644 > Documentation/devicetree/bindings/display/panel/edt,etm0700g0dh6.txt > > [Changes from v4 of emCON patch-series] > - rebased to mainline/master > - split as requested by Shawn Guo > > As requested by Sha All three patches applied, thanks. Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/4] drm/panel: simple: Add support for Rocktech RK070ER9427 LCD panel
On Thu, Jun 07, 2018 at 07:16:48PM +0530, Jagan Teki wrote: > This adds support for the Rocktech Display Ltd. RK070ER9427 > 800(RGB)x480 TFT LCD panel, which can be supported by the > simple panel driver. > > Signed-off-by: Jagan Teki > Reviewed-by: Rob Herring > --- > Changes for v2: > - collect Rob r-w-b tag > > .../display/panel/rocktech,rk070er9427.txt | 25 > drivers/gpu/drm/panel/panel-simple.c | 33 > ++ > 2 files changed, 58 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/rocktech,rk070er9427.txt checkpatch warns about the rocktech vendor prefix not being documented. Can you send a patch which adds it to Documentation/devicetree/bindings/vendor-prefixes.txt Thanks, Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: 答复: [alsa-devel] 答复: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id
On Mon, 09 Jul 2018 18:05:09 +0200, Qu, Jim wrote: > > Hi Takashi, > > Not intel, but it is AMD APU+ AMD GFX, the APU has a local HDMI port for > extension. And dGPU is only for offloading render via PRIME. > > Originally, the HDA driver before v4.17, there is a bug, that all the audio > is set to CLIENT_DIS, so the when the dGPU suspend, the audio which is bound > to iGPU will also be suspend. > > Now, after Lukas' patches. The audio will auto suspend. It make ubuntu audio > setting could not detect HDMI audio, even if HDMI has light up. OK, and it has all necessary patches including the one Lukas suggested, right? If so, figure out what you're actually seeing as "PA could not detect HDMI audio". For example, is it the HDMI (jack) detection (giving false even if it's connected)? Or is it an error at opening the given device? Does the driver give the proper ELD bytes as well as the jack state? Takashi > > Thanks > JimQu > > -邮件原件- > 发件人: Takashi Iwai > 发送时间: 2018年7月9日 23:58 > 收件人: Daniel Vetter > 抄送: Alex Deucher ; alsa-de...@alsa-project.org; > amd-...@lists.freedesktop.org; Qu, Jim ; > dri-devel@lists.freedesktop.org; Deucher, Alexander > > 主题: Re: [alsa-devel] 答复: [PATCH] vgaswitchroo: set audio client id according > to bound gpu client id > > On Mon, 09 Jul 2018 17:56:43 +0200, > Daniel Vetter wrote: > > > > On Mon, Jul 09, 2018 at 05:04:22PM +0200, Takashi Iwai wrote: > > > On Mon, 09 Jul 2018 15:58:51 +0200, > > > Alex Deucher wrote: > > > > > > > > On Mon, Jul 9, 2018 at 6:16 AM, Qu, Jim wrote: > > > > > Hi Lukas, > > > > > > > > > > Thanks to your explanation, and see comments in line. > > > > > > > > > > > > > > > Do you need to runtime resume the HDA controller even if user > > > > > space isn't streaming audio? Why, and in which situation exactly? > > > > > > > > > > Jim: OEM system uses pactl to queiry audio card and audio output > > > > > sink, since the audio has power down by runtime pm, so the audio card > > > > > and output sink are all unavailable. they could not select the > > > > > available HDMI audio for audio playing. > > > > > > > > > > You're saying above that the HDA controller isn't runtime > > > > > resumed on hotplug of a display. Is that necessary to retrieve ELD > > > > > or something? > > > > > I'm not sure if there's code in the HDA driver to acquire a > > > > > runtime PM ref on HPD, but maybe it's necessary. Or maybe the > > > > > code is there but somehow no HPD interrupt is received by the HDA > > > > > driver? > > > > > > > > > > Jim: So far, I do not find any code about audio response HPD in > > > > > kernel. the HPD interrupt will sent to user mode via uevent, not sure > > > > > whether audio user mode driver can receive the event or not. > > > > > > > > On the gfx side at least, we can get a hotplug event via ACPI > > > > (depending on the OEM design) if displays are attached/detached > > > > while the dGPU is powered down. I suppose the gfx driver could > > > > call into the audio driver during one of those events. On the gfx > > > > side at least we just generate the gfx hotplug event and let userspace > > > > deal with it. > > > > > > IMO, a more proper way would be to have the direct communication > > > between the graphics driver and the audio driver like i915 driver > > > does. Then the audio driver can get plug/unplug event at more > > > accurate timing without races. > > > > > > (Though, it will cause another mess wrt the weak module dependency, > > > but it's another story :) > > > > Just don't do what snd-hda-intel did and instead implement the > > component stuff properly :-) > > A patch is welcome :) > > > thanks, > > Takashi > > > -Daniel > > > > > > > > > > > thanks, > > > > > > Takashi > > > > > > > > > > > > > > Alex > > > > > > > > > > > > > > Thanks > > > > > JimQu > > > > > > > > > > > > > > > 发件人: Lukas Wunner > > > > > 发送时间: 2018年7月9日 17:27 > > > > > 收件人: Qu, Jim > > > > > 抄送: alsa-de...@alsa-project.org; > > > > > dri-devel@lists.freedesktop.org; Deucher, Alexander; > > > > > amd-...@lists.freedesktop.org > > > > > 主题: Re: [PATCH] vgaswitchroo: set audio client id according to > > > > > bound gpu client id > > > > > > > > > > On Mon, Jul 09, 2018 at 08:52:33AM +, Qu, Jim wrote: > > > > >> Now, I found the audio device will auto suspend even if the gpu > > > > >> is active, and if I plug in a HDMI device it also do not resume back. > > > > >> > > > > >> 1. Did you encounter similar issue before? > > > > >> 2. audio will auto suspend as default at beginning of boot, is > > > > >> it expect behaviour? > > > > > > > > > > Yes, that's expected. Once you start streaming audio to > > > > > attached displays, user space opens the codec device and this > > > > > causes the HDA controller to runtime resume. If the discrete > > > > > GPU is suspended at that moment, it will be powered on and kept > > > > > powered on
Re: 答复: [alsa-devel] ??????: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id
On Mon, 09 Jul 2018 18:03:48 +0200, Alex Deucher wrote: > > On Mon, Jul 9, 2018 at 11:57 AM, Takashi Iwai wrote: > > On Mon, 09 Jul 2018 17:53:19 +0200, > > Qu, Jim wrote: > >> > >> Hi All, > >> > >> Here, I want to clarify the audio device is bound to iGPU. There is no > >> audio codec for dGPU. > > > > I'm confused. So you mean that the HDMI detection on Intel GPU > > doesn't work with the hybrid GPUs? And no audio codec on discrete > > GPU...? Why do we need vga_switcheroo at all, then? > > The original problem is the audio codec is getting associated with > dGPU rather than the iGPU so switcheroo tries to power down the audio > codec when it powers down the dGPU. We got a bit sidetracked in the > discussion however. Ah, thanks, that clarifies better, but still I'm lost about what's going on... Can anyone summarize the problem and the results with the recent (>= 4.17) Linux kernel...? thanks, Takashi > Alex > > > > > > > Takashi > > > > > >> > >> Thanks > >> JimQu > >> > >> -邮件原件- > >> 发件人: Lukas Wunner > >> 发送时间: 2018年7月9日 23:48 > >> 收件人: Takashi Iwai > >> 抄送: Alex Deucher ; alsa-de...@alsa-project.org; > >> amd-...@lists.freedesktop.org; Qu, Jim ; > >> dri-devel@lists.freedesktop.org; Deucher, Alexander > >> > >> 主题: Re: [alsa-devel] ??: [PATCH] vgaswitchroo: set audio client id > >> according to bound gpu client id > >> > >> On Mon, Jul 09, 2018 at 05:04:22PM +0200, Takashi Iwai wrote: > >> > On Mon, 09 Jul 2018 15:58:51 +0200, Alex Deucher wrote: > >> > > On Mon, Jul 9, 2018 at 6:16 AM, Qu, Jim wrote: > >> > > > > You're saying above that the HDA controller isn't runtime > >> > > > > resumed on hotplug of a display. Is that necessary to retrieve > >> > > > > ELD or something? > >> > > > > I'm not sure if there's code in the HDA driver to acquire a > >> > > > > runtime PM ref on HPD, but maybe it's necessary. Or maybe the > >> > > > > code is there but somehow no HPD interrupt is received by the HDA > >> > > > > driver? > >> > > > > >> > > > So far, I do not find any code about audio response HPD in kernel. > >> > > > the HPD interrupt will sent to user mode via uevent, not sure > >> > > > whether audio user mode driver can receive the event or not. > >> > > > >> > > On the gfx side at least, we can get a hotplug event via ACPI > >> > > (depending on the OEM design) if displays are attached/detached > >> > > while the dGPU is powered down. I suppose the gfx driver could call > >> > > into the audio driver during one of those events. On the gfx side > >> > > at least we just generate the gfx hotplug event and let userspace deal > >> > > with it. > >> > > >> > IMO, a more proper way would be to have the direct communication > >> > between the graphics driver and the audio driver like i915 driver > >> > does. Then the audio driver can get plug/unplug event at more > >> > accurate timing without races. > >> > >> Since v4.17, every time the GPU is powered up, the HDA controller is > >> runtime resumed to PCI_D0. (See the call to pci_wakeup_bus() in > >> vga_switcheroo_runtime_resume() added by dcac86b7d0.) > >> > >> If the HDA controller can't detect presence of an HDMI display even if > >> it's in PCI_D0, then I suppose that needs to be adressed in the HDA > >> driver. Thus so far I don't see the need to call from amdgpu into the HDA > >> driver. > >> > >> Thanks, > >> > >> Lukas > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [alsa-devel] ??????: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id
On Mon, 09 Jul 2018 18:15:32 +0200, Alex Deucher wrote: > > On Mon, Jul 9, 2018 at 12:06 PM, Lukas Wunner wrote: > > On Mon, Jul 09, 2018 at 05:52:49PM +0200, Takashi Iwai wrote: > >> On Mon, 09 Jul 2018 17:47:34 +0200, Lukas Wunner wrote: > >> > Since v4.17, every time the GPU is powered up, the HDA controller is > >> > runtime resumed to PCI_D0. (See the call to pci_wakeup_bus() in > >> > vga_switcheroo_runtime_resume() added by dcac86b7d0.) > >> > > >> > If the HDA controller can't detect presence of an HDMI display even > >> > if it's in PCI_D0, then I suppose that needs to be adressed in the HDA > >> > driver. Thus so far I don't see the need to call from amdgpu into the > >> > HDA driver. > >> > >> It's not about the PCI power state, but the problem is that the > >> detection of the HDMI and its ELD read is somewhat asynchronous. > >> Basically it's handled via the hardware unsolicited event emitted over > >> HD-audio bus, which was originally generated by GPU at dealing with > >> the hotplug/unplug. So, this part is racy and not 100% reliable -- > >> although usually it shouldn't be a big problem. > >> > >> That said, it's not the exact "need" but it would make things more > >> reliable and accurate (even consumes less power as we don't need to > >> power up/down just for the HDMI detection). > > > > Okay. If amdgpu triggers the event on the HDA bus, it should call > > pm_runtime_get_sync() on the HDA device beforehand. Which device > > needs to be resumed exactly to ensure ELD reception, the PCI one > > or the codec? > > I don't think it makes sense to power up anything until the user wants > to do something with the event. On AMD hardware, the ELD is not > actually used. The gpu passes the ELD info internally via shared > registers. So as soon as the GPU driver parses the EDID and updates > the shared registers, the audio driver will know the state just by > reading back the shared registers. That said, there no reason to act > on the event unless the user actually wants to do something with it. > E.g., the desktop manager sees the event and decides to call down to > the kernel to query the display topology (or not). At which point the > hw can be powered up, etc. It needs to power up to read even the shared registers; they are accessed via HD-audio verbs, so we need the whole runtime resume of the bus & codec just for reading ELD information that are stored by GPU. And, ELD info is mandatory for informing the hotplug information to user-space, not only the connection/disconnection state. If ELD and hoptlug info are passed directly from GPU by other means, we can avoid the power up/down in the audio side, indeed. That's what Intel driver serves, after all. Takashi ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: vkms: select DRM_KMS_HELPER
On Mon, Jul 09, 2018 at 05:48:18PM +0200, Arnd Bergmann wrote: > Without this, we get link errors during randconfig build: > > drivers/gpu/drm/vkms/vkms_drv.o:(.rodata+0xa0): undefined reference to > `drm_atomic_helper_check' > drivers/gpu/drm/vkms/vkms_drv.o:(.rodata+0xa8): undefined reference to > `drm_atomic_helper_commit' > drivers/gpu/drm/vkms/vkms_plane.o:(.rodata+0x0): undefined reference to > `drm_atomic_helper_update_plane' > drivers/gpu/drm/vkms/vkms_plane.o:(.rodata+0x8): undefined reference to > `drm_atomic_helper_disable_plane' > drivers/gpu/drm/vkms/vkms_plane.o:(.rodata+0x18): undefined reference to > `drm_atomic_helper_plane_reset' > drivers/gpu/drm/vkms/vkms_plane.o:(.rodata+0x28): undefined reference to > `drm_atomic_helper_plane_duplicate_state' > drivers/gpu/drm/vkms/vkms_plane.o:(.rodata+0x30): undefined reference to > `drm_atomic_helper_plane_destroy_state' > drivers/gpu/drm/vkms/vkms_output.o:(.rodata+0x1c0): undefined reference to > `drm_helper_probe_single_connector_modes' > drivers/gpu/drm/vkms/vkms_crtc.o:(.rodata+0x40): undefined reference to > `drm_atomic_helper_crtc_reset' > drivers/gpu/drm/vkms/vkms_crtc.o:(.rodata+0x70): undefined reference to > `drm_atomic_helper_set_config' > drivers/gpu/drm/vkms/vkms_crtc.o:(.rodata+0x78): undefined reference to > `drm_atomic_helper_page_flip' > drivers/gpu/drm/vkms/vkms_crtc.o:(.rodata+0x90): undefined reference to > `drm_atomic_helper_crtc_duplicate_state' > drivers/gpu/drm/vkms/vkms_crtc.o:(.rodata+0x98): undefined reference to > `drm_atomic_helper_crtc_destroy_state' > > Fixes: 854502fa0a38 ("drm/vkms: Add basic CRTC initialization") > Fixes: 1c7c5fd916a0 ("drm/vkms: Introduce basic VKMS driver") > Signed-off-by: Arnd Bergmann Oops. Thanks for your patch, applied. -Daniel > --- > drivers/gpu/drm/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 516a8f5d1ac6..e527772f5ca6 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -214,6 +214,7 @@ config DRM_VGEM > config DRM_VKMS > tristate "Virtual KMS (EXPERIMENTAL)" > depends on DRM > + select DRM_KMS_HELPER > default n > help > Virtual Kernel Mode-Setting (VKMS) is used for testing or for > -- > 2.9.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107168] Allow to load firmware during run-time (after initialization)
https://bugs.freedesktop.org/show_bug.cgi?id=107168 --- Comment #5 from Alex Deucher --- (In reply to Paul Menzel from comment #4) > (In reply to Alex Deucher from comment #3) > > (In reply to Paul Menzel from comment #2) > > > (In reply to Alex Deucher from comment #1) > > > > You need the firmware for initialization. > > > > > > What’s the technical reason for this. Why can’t certain parts of the > > > hardware be initialized later on? > > > > You can't do anything other than very limited modesetting until the > > firmwares are loaded. > > That would be enough for me for entering the LUKS password. > > > You might as well just use the efifb driver and then load the driver later > > after the filesystem is available. > > There are several problems with that. > > 1. No UEFI based firmware is used, but a coreboot based one. > 2. To boot fast, I like to load the Radeon/AMDGPU driver directly, and not > spent precious milliseconds loading other display drivers (efifb, > corebootfb). Not having to include these makes the Linux kernel image a > little smaller and loading it faster. > 3. Currently loading `radeon` on an ASRock E350M1 and Asus F2A85-M takes > roughly two seconds, which is longer then the OS needs to start. So it’d be > great to be able to load the firmware while GDM for example starts. That's a different problem, nothing to do with when the firmware loads or whether it's available at driver load time. You still have to do the initialization at some point and that takes time. You are just moving the when that init happens. So rather than a delay boot, you'll get a delay when starting gdm. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: adv7511: Reset registers on hotplug
On Wed, Jul 04, 2018 at 06:53:02PM +0530, Archit Taneja wrote: > > > On Wednesday 04 July 2018 12:28 AM, Rob Clark wrote: > > On Tue, Jul 3, 2018 at 12:56 PM, Sean Paul wrote: > > > The bridge loses its hw state when the cable is unplugged. If we detect > > > this case in the hpd handler, reset its state. > > > > > > Reported-by: Rob Clark > > > Signed-off-by: Sean Paul > > > > Tested-by: Rob Clark > > > > > --- > > > This is the follow-up to "drm/msm: Disable mdp5 crtc when there are no > > > active planes". I incorrectly assumed the modeset was required from the > > > msm side, but Rob pointed out that he thought it might be a bridge > > > issue. > > > > To elaborate, this is an atomic userspace (weston), when it sees the > > display disconnected, it removes the planes from the CRTC but does not > > disable the CRTC. So drm/msm sets up the LM to scanout a solid color, > > and leaves the encoder and dsi cranking along happily. When > > replugging the display, the atomic helpers see the CRTC is still > > active and (correctly) doesn't do a full modeset. So the bridge is > > not re-configured/re-enabled. > > The adv7511 connector's detect() op should have re-configured the > registers. I'm assuming it was never called after the cable is > plugged in again in the wetson usecase? Right. detect() isn't guaranteed to be called on connection (it just usually is). > > With this patch, in the case of fb emulation/X, I'm wondering if > we will end up trying to re-enable ADV7511 twice. First, in the new code > in adv7511_hpd_work(), and later in adv7511_detect() (i.e, the > connector's detect op). > > I'm guessing the 'hpd' variable in the check in adv7511_detect() below > should ideally be false, and avoid us trying to configure the registers > again, but I'm not entirely sure. It's entirely possible to call detect() multiple times per connection (ie, run modetest a bunch), so that hpd check is pretty critical for even non-hotplug cases. > > if (status == connector_status_connected && hpd && adv7511->powered) { > regcache_mark_dirty(adv7511->regmap); > ... > > In any case: > > Reviewed-by: Archit Taneja Thanks for your review, I'll apply to -misc-fixes. Sean > > > > > Arguably this perhaps isn't what weston *wanted* to do, but in the end > > the bug is in the bridge. > > > > We could possibly set the connector link_status to BAD as an > > alternative. But at least the build of weston I'm using atm doesn't > > handle the link_status propery, this approach requires each atomic > > userspace to handle this case. Compared to Sean's approach which is > > transparent to userspace, which seems attractive. > > > > BR, > > -R > > > > > > > > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 12 > > > 1 file changed, 12 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > > index 73021b388e12..dd3ff2f2cdce 100644 > > > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > > @@ -429,6 +429,18 @@ static void adv7511_hpd_work(struct work_struct > > > *work) > > > else > > > status = connector_status_disconnected; > > > > > > + /* > > > +* The bridge resets its registers on unplug. So when we get a > > > plug > > > +* event and we're already supposed to be powered, cycle the > > > bridge to > > > +* restore its state. > > > +*/ > > > + if (status == connector_status_connected && > > > + adv7511->connector.status == connector_status_disconnected && > > > + adv7511->powered) { > > > + regcache_mark_dirty(adv7511->regmap); > > > + adv7511_power_on(adv7511); > > > + } > > > + > > > if (adv7511->connector.status != status) { > > > adv7511->connector.status = status; > > > if (status == connector_status_disconnected) > > > -- > > > Sean Paul, Software Engineer, Google / Chromium OS > > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in > > the body of a message to majord...@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107168] Allow to load firmware during run-time (after initialization)
https://bugs.freedesktop.org/show_bug.cgi?id=107168 --- Comment #4 from Paul Menzel --- (In reply to Alex Deucher from comment #3) > (In reply to Paul Menzel from comment #2) > > (In reply to Alex Deucher from comment #1) > > > You need the firmware for initialization. > > > > What’s the technical reason for this. Why can’t certain parts of the > > hardware be initialized later on? > > You can't do anything other than very limited modesetting until the > firmwares are loaded. That would be enough for me for entering the LUKS password. > You might as well just use the efifb driver and then load the driver later > after the filesystem is available. There are several problems with that. 1. No UEFI based firmware is used, but a coreboot based one. 2. To boot fast, I like to load the Radeon/AMDGPU driver directly, and not spent precious milliseconds loading other display drivers (efifb, corebootfb). Not having to include these makes the Linux kernel image a little smaller and loading it faster. 3. Currently loading `radeon` on an ASRock E350M1 and Asus F2A85-M takes roughly two seconds, which is longer then the OS needs to start. So it’d be great to be able to load the firmware while GDM for example starts. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 11/12] sched: use for_each_if in topology.h
On Mon, Jul 09, 2018 at 05:52:04PM +0200, Daniel Vetter wrote: > for_each_something(foo) > if (foo->bla) > call_bla(foo); > else > call_default(foo); Note that the kernel coding style 'discourages' this style and would like you to write: for_each_something(foo) { if (foo->bla) call_bla(foo); else call_default(foo); } Which avoids the entire problem. See CodingStyle-3: Also, use braces when a loop contains more than a single simple statement: .. code-block:: c while (condition) { if (test) do_something(); } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107154] [drm] GPU recovery disabled.
https://bugs.freedesktop.org/show_bug.cgi?id=107154 --- Comment #8 from freedesktop@nentwig.biz --- Created attachment 140528 --> https://bugs.freedesktop.org/attachment.cgi?id=140528=edit dmesg 4.14 LTS Sorry, forgot about the requested 4.14 dmesg log. Attached as well. This is: boot, login (to KDE this time), do stuff, remember, sleep, wakeup. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107168] Allow to load firmware during run-time (after initialization)
https://bugs.freedesktop.org/show_bug.cgi?id=107168 --- Comment #3 from Alex Deucher --- (In reply to Paul Menzel from comment #2) > (In reply to Alex Deucher from comment #1) > > You need the firmware for initialization. > > What’s the technical reason for this. Why can’t certain parts of the > hardware be initialized later on? You can't do anything other than very limited modesetting until the firmwares are loaded. You might as well just use the efifb driver and then load the driver later after the filesystem is available. There's not really anything you can initialize without firmware. MC and SMC firmware are required to adjust the clocks which is required for performance and for bandwidth requirements for high res display configurations. You can't use the SDMA hardware without firmware so that means no GPU memory management or GPU VM updates and you are limited to 256 MB of vram (PCI BAR size) on most platforms. You can't use the gfx/compute engines without the CP firmware. You can't use the multi-media engines without firmware. Etc. Not to mention you now have multiple code paths to validate at the hw and sw level. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] kernel.h: Add for_each_if()
To avoid compilers complainig about ambigious else blocks when putting an if condition into a for_each macro one needs to invert the condition and add a dummy else. We have a nice little convenience macro for that in drm headers, let's move it out. Subsequent patches will roll it out to other places. The issue the compilers complain about are nested if with an else block and no {} to disambiguate which if the else belongs to. The C standard is clear, but in practice people forget: if (foo) if (bar) /* something */ else /* something else The same can happen in a for_each macro when it also contains an if condition at the end, except the compiler message is now really confusing since there's only 1 if: for_each_something() if (bar) /* something */ else /* something else The for_each_if() macro, by inverting the condition and adding an else, avoids the compiler warning. Motivated by a discussion with Andy and Yisheng, who want to add another for_each_macro which would benefit from for_each_if() instead of hand-rolling it. Cc: Gustavo Padovan Cc: Maarten Lankhorst Cc: Sean Paul Cc: David Airlie Cc: Andrew Morton Cc: Kees Cook Cc: Ingo Molnar Cc: Greg Kroah-Hartman Cc: NeilBrown Cc: Wei Wang Cc: Stefan Agner Cc: Andrei Vagin Cc: Randy Dunlap Cc: Andy Shevchenko Cc: Yisheng Xie Cc: Peter Zijlstra Reviewed-by: Andy Shevchenko Signed-off-by: Daniel Vetter -- v2: Explain a bit better what this is good for, after the discussion with Peter Z. --- include/drm/drmP.h | 3 --- include/linux/kernel.h | 3 +++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/drm/drmP.h b/include/drm/drmP.h index f7a19c2a7a80..05350424a4d3 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -110,7 +110,4 @@ static inline bool drm_can_sleep(void) return true; } -/* helper for handling conditionals in various for_each macros */ -#define for_each_if(condition) if (!(condition)) {} else - #endif diff --git a/include/linux/kernel.h b/include/linux/kernel.h index 941dc0a5a877..4cb95ab9a5bc 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -71,6 +71,9 @@ */ #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr)) +/* helper for handling conditionals in various for_each macros */ +#define for_each_if(condition) if (!(condition)) {} else + #define u64_to_user_ptr(x) ( \ { \ typecheck(u64, x); \ -- 2.18.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [alsa-devel] ??????: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id
On Mon, Jul 9, 2018 at 12:06 PM, Lukas Wunner wrote: > On Mon, Jul 09, 2018 at 05:52:49PM +0200, Takashi Iwai wrote: >> On Mon, 09 Jul 2018 17:47:34 +0200, Lukas Wunner wrote: >> > Since v4.17, every time the GPU is powered up, the HDA controller is >> > runtime resumed to PCI_D0. (See the call to pci_wakeup_bus() in >> > vga_switcheroo_runtime_resume() added by dcac86b7d0.) >> > >> > If the HDA controller can't detect presence of an HDMI display even >> > if it's in PCI_D0, then I suppose that needs to be adressed in the HDA >> > driver. Thus so far I don't see the need to call from amdgpu into the >> > HDA driver. >> >> It's not about the PCI power state, but the problem is that the >> detection of the HDMI and its ELD read is somewhat asynchronous. >> Basically it's handled via the hardware unsolicited event emitted over >> HD-audio bus, which was originally generated by GPU at dealing with >> the hotplug/unplug. So, this part is racy and not 100% reliable -- >> although usually it shouldn't be a big problem. >> >> That said, it's not the exact "need" but it would make things more >> reliable and accurate (even consumes less power as we don't need to >> power up/down just for the HDMI detection). > > Okay. If amdgpu triggers the event on the HDA bus, it should call > pm_runtime_get_sync() on the HDA device beforehand. Which device > needs to be resumed exactly to ensure ELD reception, the PCI one > or the codec? I don't think it makes sense to power up anything until the user wants to do something with the event. On AMD hardware, the ELD is not actually used. The gpu passes the ELD info internally via shared registers. So as soon as the GPU driver parses the EDID and updates the shared registers, the audio driver will know the state just by reading back the shared registers. That said, there no reason to act on the event unless the user actually wants to do something with it. E.g., the desktop manager sees the event and decides to call down to the kernel to query the display topology (or not). At which point the hw can be powered up, etc. Alex ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107154] [drm] GPU recovery disabled.
https://bugs.freedesktop.org/show_bug.cgi?id=107154 --- Comment #7 from freedesktop@nentwig.biz --- Sure, attached. AMD staging kernel. I don't know how to tell whether DC=1 is really enabled, so I did two runs: one with amdgpu.dc=1 as boot parameter and one with /etc/modprobe.d/ on top of that. Procedure was the same both times: - boot - X login - switch to console - sleep, wakeup - switch to X The drm/amdgpu lines appear already in the console right after waking up, prior to switching to X. This time "only" X crashed (could still move the pointer); at times the complete machine is dead, no switching to console and and no SSH. (as a side note: is is normal that waking up on ryzen takes something on the order of 10-30s? I'm used to split second wakeups on Intel.) HTH -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] cpufreq: use for_each_if
Avoids the inverted condition compared to the open coded version. Signed-off-by: Daniel Vetter Cc: "Rafael J. Wysocki" Cc: Viresh Kumar Cc: linux...@vger.kernel.org Cc: Eric Engestrom -- v2: Fix the logic fumble in the 2nd hunk, spotted by Eric. --- include/linux/cpufreq.h | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index 882a9b9e34bc..3a774f523d00 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -649,9 +649,7 @@ static inline void dev_pm_opp_free_cpufreq_table(struct device *dev, #define cpufreq_for_each_valid_entry(pos, table) \ for (pos = table; pos->frequency != CPUFREQ_TABLE_END; pos++) \ - if (pos->frequency == CPUFREQ_ENTRY_INVALID)\ - continue; \ - else + for_each_if (pos->frequency != CPUFREQ_ENTRY_INVALID) /* * cpufreq_for_each_valid_entry_idx - iterate with index over a cpufreq @@ -663,9 +661,7 @@ static inline void dev_pm_opp_free_cpufreq_table(struct device *dev, #define cpufreq_for_each_valid_entry_idx(pos, table, idx) \ cpufreq_for_each_entry_idx(pos, table, idx) \ - if (pos->frequency == CPUFREQ_ENTRY_INVALID)\ - continue; \ - else + for_each_if (pos->frequency != CPUFREQ_ENTRY_INVALID) int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy, -- 2.18.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [alsa-devel] ??????: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id
On Mon, Jul 09, 2018 at 05:52:49PM +0200, Takashi Iwai wrote: > On Mon, 09 Jul 2018 17:47:34 +0200, Lukas Wunner wrote: > > Since v4.17, every time the GPU is powered up, the HDA controller is > > runtime resumed to PCI_D0. (See the call to pci_wakeup_bus() in > > vga_switcheroo_runtime_resume() added by dcac86b7d0.) > > > > If the HDA controller can't detect presence of an HDMI display even > > if it's in PCI_D0, then I suppose that needs to be adressed in the HDA > > driver. Thus so far I don't see the need to call from amdgpu into the > > HDA driver. > > It's not about the PCI power state, but the problem is that the > detection of the HDMI and its ELD read is somewhat asynchronous. > Basically it's handled via the hardware unsolicited event emitted over > HD-audio bus, which was originally generated by GPU at dealing with > the hotplug/unplug. So, this part is racy and not 100% reliable -- > although usually it shouldn't be a big problem. > > That said, it's not the exact "need" but it would make things more > reliable and accurate (even consumes less power as we don't need to > power up/down just for the HDMI detection). Okay. If amdgpu triggers the event on the HDA bus, it should call pm_runtime_get_sync() on the HDA device beforehand. Which device needs to be resumed exactly to ensure ELD reception, the PCI one or the codec? Thanks, Lukas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] [v2] drm/sun4i: fix build failure with CONFIG_DRM_SUN8I_MIXER=m
Having DRM_SUN4I built-in but DRM_SUN8I_MIXER as a loadable module results in a link error, as we try to access a symbol from the sun8i_tcon_top.ko module: ERROR: "sun8i_tcon_top_of_table" [drivers/gpu/drm/sun4i/sun8i-drm-hdmi.ko] undefined! ERROR: "sun8i_tcon_top_of_table" [drivers/gpu/drm/sun4i/sun4i-drm.ko] undefined! This solves the problem by adding a silent symbol for the tcon_top module, building it as a separate module in exactly the cases that we need it, but in a way that it is reachable by the other modules. Fixes: 57e23de02f48 ("drm/sun4i: DW HDMI: Expand algorithm for possible crtcs") Fixes: ef0cf6441fbb ("drm/sun4i: Add support for traversing graph with TCON TOP") Signed-off-by: Arnd Bergmann --- v2: link the tcon top module separately instead of into the mixer --- drivers/gpu/drm/sun4i/Kconfig | 7 +++ drivers/gpu/drm/sun4i/Makefile| 3 ++- drivers/gpu/drm/sun4i/sun4i_drv.c | 3 ++- drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 3 ++- 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig index 156a865c3e6d..c2c042287c19 100644 --- a/drivers/gpu/drm/sun4i/Kconfig +++ b/drivers/gpu/drm/sun4i/Kconfig @@ -68,4 +68,11 @@ config DRM_SUN8I_MIXER graphics mixture and feed graphics to TCON, If M is selected the module will be called sun8i-mixer. +config DRM_SUN8I_TCON_TOP + tristate + default DRM_SUN4I if DRM_SUN8I_MIXER!=n + help + TCON TOP is responsible for configuring display pipeline for + HTMI, TVE and LCD. + endif diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile index cd27d02c94e2..0eb38ac8e86e 100644 --- a/drivers/gpu/drm/sun4i/Makefile +++ b/drivers/gpu/drm/sun4i/Makefile @@ -39,4 +39,5 @@ endif obj-$(CONFIG_DRM_SUN4I_HDMI) += sun4i-drm-hdmi.o obj-$(CONFIG_DRM_SUN6I_DSI)+= sun6i-dsi.o obj-$(CONFIG_DRM_SUN8I_DW_HDMI)+= sun8i-drm-hdmi.o -obj-$(CONFIG_DRM_SUN8I_MIXER) += sun8i-mixer.o sun8i_tcon_top.o +obj-$(CONFIG_DRM_SUN8I_MIXER) += sun8i-mixer.o +obj-$(CONFIG_DRM_SUN8I_TCON_TOP) += sun8i_tcon_top.o diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c index 6ddf4eaccb40..495d0fc41297 100644 --- a/drivers/gpu/drm/sun4i/sun4i_drv.c +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c @@ -216,7 +216,8 @@ static bool sun4i_drv_node_is_tcon_with_ch0(struct device_node *node) static bool sun4i_drv_node_is_tcon_top(struct device_node *node) { - return !!of_match_node(sun8i_tcon_top_of_table, node); + return IS_ENABLED(CONFIG_DRM_SUN8I_TCON_TOP) && + !!of_match_node(sun8i_tcon_top_of_table, node); } static int compare_of(struct device *dev, void *data) diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c index 3459b9ec56c9..3704d95874c2 100644 --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c @@ -44,7 +44,8 @@ sun8i_dw_hdmi_mode_valid(struct drm_connector *connector, static bool sun8i_dw_hdmi_node_is_tcon_top(struct device_node *node) { - return !!of_match_node(sun8i_tcon_top_of_table, node); + return IS_ENABLED(CONFIG_DRM_SUN8I_TCON_TOP) && + !!of_match_node(sun8i_tcon_top_of_table, node); } static u32 sun8i_dw_hdmi_find_possible_crtcs(struct drm_device *drm, -- 2.9.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 11/12] sched: use for_each_if in topology.h
On Mon, Jul 9, 2018 at 6:03 PM, Peter Zijlstra wrote: > On Mon, Jul 09, 2018 at 05:52:04PM +0200, Daniel Vetter wrote: >> for_each_something(foo) >> if (foo->bla) >> call_bla(foo); >> else >> call_default(foo); >> >> Totally contrived, but this complains. Liberally sprinkling {} also shuts >> up the compiler, but it's a bit confusing given that a plain for {;;} is >> totally fine. And it's confusing since at first glance the compiler >> complaining about nested if and ambigous else doesn't make sense since >> clearly there's only 1 if there. > > Ah, so the pattern the compiler tries to warn about is: > > if (foo) > if (bar) > /* stmts1 */ > else > /* stmts2 * > > Because it might not be immediately obvious with which if the else goes. > Which is fair enough I suppose. Yup. I'll augment the commit message of patch 1 to include this as example, and why it's confusing in context of a for_each_foo macro containing an if(). > OK, ACK. Thanks, 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: [amdgpu][tahiti xt] missing firmware
On Mon, Jul 09, 2018 at 10:24:19AM -0400, Alex Deucher wrote: > On Sun, Jul 8, 2018 at 10:31 AM, wrote: > > Hi, > > > > The firmware paths were changed from 'radeon/' to 'amdgpu/' in > > amd-staging-drm-next, but master of > > git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git is > > missing > > the files in amdgpu/ for tahiti gpus. > > I'm pushing out updated firmware this week. In the meantime, you can > just copy or symlink the firmware from the radeon directory. Of course, thanks. regards, -- Sylvain ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
答复: [alsa-devel] 答复: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id
Hi Takashi, Not intel, but it is AMD APU+ AMD GFX, the APU has a local HDMI port for extension. And dGPU is only for offloading render via PRIME. Originally, the HDA driver before v4.17, there is a bug, that all the audio is set to CLIENT_DIS, so the when the dGPU suspend, the audio which is bound to iGPU will also be suspend. Now, after Lukas' patches. The audio will auto suspend. It make ubuntu audio setting could not detect HDMI audio, even if HDMI has light up. Thanks JimQu -邮件原件- 发件人: Takashi Iwai 发送时间: 2018年7月9日 23:58 收件人: Daniel Vetter 抄送: Alex Deucher ; alsa-de...@alsa-project.org; amd-...@lists.freedesktop.org; Qu, Jim ; dri-devel@lists.freedesktop.org; Deucher, Alexander 主题: Re: [alsa-devel] 答复: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id On Mon, 09 Jul 2018 17:56:43 +0200, Daniel Vetter wrote: > > On Mon, Jul 09, 2018 at 05:04:22PM +0200, Takashi Iwai wrote: > > On Mon, 09 Jul 2018 15:58:51 +0200, > > Alex Deucher wrote: > > > > > > On Mon, Jul 9, 2018 at 6:16 AM, Qu, Jim wrote: > > > > Hi Lukas, > > > > > > > > Thanks to your explanation, and see comments in line. > > > > > > > > > > > > Do you need to runtime resume the HDA controller even if user > > > > space isn't streaming audio? Why, and in which situation exactly? > > > > > > > > Jim: OEM system uses pactl to queiry audio card and audio output sink, > > > > since the audio has power down by runtime pm, so the audio card and > > > > output sink are all unavailable. they could not select the available > > > > HDMI audio for audio playing. > > > > > > > > You're saying above that the HDA controller isn't runtime > > > > resumed on hotplug of a display. Is that necessary to retrieve ELD or > > > > something? > > > > I'm not sure if there's code in the HDA driver to acquire a > > > > runtime PM ref on HPD, but maybe it's necessary. Or maybe the > > > > code is there but somehow no HPD interrupt is received by the HDA > > > > driver? > > > > > > > > Jim: So far, I do not find any code about audio response HPD in kernel. > > > > the HPD interrupt will sent to user mode via uevent, not sure whether > > > > audio user mode driver can receive the event or not. > > > > > > On the gfx side at least, we can get a hotplug event via ACPI > > > (depending on the OEM design) if displays are attached/detached > > > while the dGPU is powered down. I suppose the gfx driver could > > > call into the audio driver during one of those events. On the gfx > > > side at least we just generate the gfx hotplug event and let userspace > > > deal with it. > > > > IMO, a more proper way would be to have the direct communication > > between the graphics driver and the audio driver like i915 driver > > does. Then the audio driver can get plug/unplug event at more > > accurate timing without races. > > > > (Though, it will cause another mess wrt the weak module dependency, > > but it's another story :) > > Just don't do what snd-hda-intel did and instead implement the > component stuff properly :-) A patch is welcome :) thanks, Takashi > -Daniel > > > > > > > thanks, > > > > Takashi > > > > > > > > > > Alex > > > > > > > > > > > Thanks > > > > JimQu > > > > > > > > > > > > 发件人: Lukas Wunner > > > > 发送时间: 2018年7月9日 17:27 > > > > 收件人: Qu, Jim > > > > 抄送: alsa-de...@alsa-project.org; > > > > dri-devel@lists.freedesktop.org; Deucher, Alexander; > > > > amd-...@lists.freedesktop.org > > > > 主题: Re: [PATCH] vgaswitchroo: set audio client id according to > > > > bound gpu client id > > > > > > > > On Mon, Jul 09, 2018 at 08:52:33AM +, Qu, Jim wrote: > > > >> Now, I found the audio device will auto suspend even if the gpu > > > >> is active, and if I plug in a HDMI device it also do not resume back. > > > >> > > > >> 1. Did you encounter similar issue before? > > > >> 2. audio will auto suspend as default at beginning of boot, is > > > >> it expect behaviour? > > > > > > > > Yes, that's expected. Once you start streaming audio to > > > > attached displays, user space opens the codec device and this > > > > causes the HDA controller to runtime resume. If the discrete > > > > GPU is suspended at that moment, it will be powered on and kept powered > > > > on as long as user space is streaming audio. > > > > > > > > Do you need to runtime resume the HDA controller even if user > > > > space isn't streaming audio? Why, and in which situation exactly? > > > > > > > > You're saying above that the HDA controller isn't runtime > > > > resumed on hotplug of a display. Is that necessary to retrieve ELD or > > > > something? > > > > I'm not sure if there's code in the HDA driver to acquire a > > > > runtime PM ref on HPD, but maybe it's necessary. Or maybe the > > > > code is there but somehow no HPD interrupt is received by the HDA > > > > driver? > > > > > > > > Thanks, > > > > > > > > Lukas > > > >
[Bug 107154] [drm] GPU recovery disabled.
https://bugs.freedesktop.org/show_bug.cgi?id=107154 --- Comment #6 from freedesktop@nentwig.biz --- Created attachment 140526 --> https://bugs.freedesktop.org/attachment.cgi?id=140526=edit dmesg /etc/modprobe.d/ Booted with amdgpu.dc=1 in /etc/modprobe.d/ -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 11/12] sched: use for_each_if in topology.h
On Mon, Jul 09, 2018 at 05:52:04PM +0200, Daniel Vetter wrote: > for_each_something(foo) > if (foo->bla) > call_bla(foo); > else > call_default(foo); > > Totally contrived, but this complains. Liberally sprinkling {} also shuts > up the compiler, but it's a bit confusing given that a plain for {;;} is > totally fine. And it's confusing since at first glance the compiler > complaining about nested if and ambigous else doesn't make sense since > clearly there's only 1 if there. Ah, so the pattern the compiler tries to warn about is: if (foo) if (bar) /* stmts1 */ else /* stmts2 * Because it might not be immediately obvious with which if the else goes. Which is fair enough I suppose. OK, ACK. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: 答复: [alsa-devel] ??????: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id
On Mon, Jul 9, 2018 at 11:57 AM, Takashi Iwai wrote: > On Mon, 09 Jul 2018 17:53:19 +0200, > Qu, Jim wrote: >> >> Hi All, >> >> Here, I want to clarify the audio device is bound to iGPU. There is no audio >> codec for dGPU. > > I'm confused. So you mean that the HDMI detection on Intel GPU > doesn't work with the hybrid GPUs? And no audio codec on discrete > GPU...? Why do we need vga_switcheroo at all, then? The original problem is the audio codec is getting associated with dGPU rather than the iGPU so switcheroo tries to power down the audio codec when it powers down the dGPU. We got a bit sidetracked in the discussion however. Alex > > > Takashi > > >> >> Thanks >> JimQu >> >> -邮件原件- >> 发件人: Lukas Wunner >> 发送时间: 2018年7月9日 23:48 >> 收件人: Takashi Iwai >> 抄送: Alex Deucher ; alsa-de...@alsa-project.org; >> amd-...@lists.freedesktop.org; Qu, Jim ; >> dri-devel@lists.freedesktop.org; Deucher, Alexander >> >> 主题: Re: [alsa-devel] ??: [PATCH] vgaswitchroo: set audio client id >> according to bound gpu client id >> >> On Mon, Jul 09, 2018 at 05:04:22PM +0200, Takashi Iwai wrote: >> > On Mon, 09 Jul 2018 15:58:51 +0200, Alex Deucher wrote: >> > > On Mon, Jul 9, 2018 at 6:16 AM, Qu, Jim wrote: >> > > > > You're saying above that the HDA controller isn't runtime >> > > > > resumed on hotplug of a display. Is that necessary to retrieve ELD >> > > > > or something? >> > > > > I'm not sure if there's code in the HDA driver to acquire a >> > > > > runtime PM ref on HPD, but maybe it's necessary. Or maybe the >> > > > > code is there but somehow no HPD interrupt is received by the HDA >> > > > > driver? >> > > > >> > > > So far, I do not find any code about audio response HPD in kernel. >> > > > the HPD interrupt will sent to user mode via uevent, not sure >> > > > whether audio user mode driver can receive the event or not. >> > > >> > > On the gfx side at least, we can get a hotplug event via ACPI >> > > (depending on the OEM design) if displays are attached/detached >> > > while the dGPU is powered down. I suppose the gfx driver could call >> > > into the audio driver during one of those events. On the gfx side >> > > at least we just generate the gfx hotplug event and let userspace deal >> > > with it. >> > >> > IMO, a more proper way would be to have the direct communication >> > between the graphics driver and the audio driver like i915 driver >> > does. Then the audio driver can get plug/unplug event at more >> > accurate timing without races. >> >> Since v4.17, every time the GPU is powered up, the HDA controller is runtime >> resumed to PCI_D0. (See the call to pci_wakeup_bus() in >> vga_switcheroo_runtime_resume() added by dcac86b7d0.) >> >> If the HDA controller can't detect presence of an HDMI display even if it's >> in PCI_D0, then I suppose that needs to be adressed in the HDA driver. Thus >> so far I don't see the need to call from amdgpu into the HDA driver. >> >> Thanks, >> >> Lukas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107154] [drm] GPU recovery disabled.
https://bugs.freedesktop.org/show_bug.cgi?id=107154 --- Comment #5 from freedesktop@nentwig.biz --- Created attachment 140525 --> https://bugs.freedesktop.org/attachment.cgi?id=140525=edit dmesg amdgpu.dc=1 Booted with amdgpu.dc=1. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 7/8] drm/bridge/synopsys: dsi: add dual-dsi support
On 09.07.2018 15:45, Heiko Stuebner wrote: > Am Dienstag, 3. Juli 2018, 19:07:02 CEST schrieb Andrzej Hajda: >> On 18.06.2018 12:28, Heiko Stuebner wrote: >>> From: Nickey Yang >>> >>> Allow to also drive a slave dw-mipi-dsi controller in a dual-dsi >>> setup. This will require additional implementation-specific >>> code to look up the slave instance and do specific setup. >>> Also will probably need code in the specific crtcs as dual-dsi >>> does not equal two separate dsi outputs. >>> >>> To activate, the implementation-specific code should set the slave >>> using dw_mipi_dsi_set_slave() before calling __dw_mipi_dsi_bind(). >>> >>> v2: >>> - expect real interface number of lanes >>> - keep links to both master and slave >> I did not see the whole driver/pipeline, but it seems the point of this >> patch is to perform the same work on the slave as on the master in case >> of dual mode. I think DSI should not be a place for it, >> DSI masters usually are stupid devices from display stack PoV, they just >> convert video streams, in dual mode also. In this case the panel and/or >> crtc adds complications so they should be responsible for handling it. >> Panel should: >> - register its both mipi interfaces with proper mode_flags (maybe some >> dual-mode indication flags should be added if necessary), >> - register drm_panel for both interfaces (it requires change in >> drm_panel api), and provide video mode timings. >> - in case it needs perform transfers perform it to master/slave/both >> interfaces according to its needs, >> >> I am not sure about DRM pipeline, it should model, maybe it could be >> done this way: >> CRTC -->ENCODER0(dsi master) --> CONNECTOR0 (panel interface 0) >> |---> ENCODER1(dsi slave) --> CONNECTOR1 (panel interface 1) >> >> But I am not sure if it is not reserved only for mirroring. >> >> For me more tempting solution is to create meta-encoder-connector let's >> call it dual-encoder (maybe it could be even generic), which is visible >> to userspace as single pipeline and encapsulates both dsi bridges/panel >> inputs. So its every callback will be translated usually to sequence of >> callbacks to 1st and 2nd dsi, or in case of get_modes it should return >> mode which represent sum of modes in both panels. >> Maybe it looks more complicated, but it can be more universal - you can >> use it with different bridges/panels even two single-panels if necessary. >> >> Of course I do not see the whole picture, or I can be just wrong, or >> just freaking purist :). If there are arguments against my vision please >> provide them. >> I am also not strongly against your solution, I just want to show >> alternatives, which could be better/more generic. > I cannot really claim to know the correct way to go forward, as these > drm-internals are still very much unknown land for me, but current > thought points I had on this: > > - strawman argument ;-) : this is the same way tegra handles dual-dsi > in its ganged mode ... also with regards to panel handling > With Thierry helming both panels and the tegra dual-dsi change Yes, and I remember our disagreement on the subject :) > > - while the panel may expose two DSI data interfaces, there is still > only one power-sequence and also only one init-command sequence. As in case of almost all devices exposing two, or more interfaces. > So I don't think you can really handle a dual-dsi panels as two fully > separate panels, creating instead the need for both panel-instances > to work together The question is if they can work separately? Ie is it possible to use only one interface, and have picture only on left/top side, even/odd lines? Or to have connected interfaces to two independent display controllers (of course via some dsi bridges)? Btw I have not seen bindings/driver for the panel, what is the panel name? The answer for above questions would be helpful. If the only way of work is with both dsi interfaces, exposing one drm_panel maybe could be OK. The point is if the panel has two interfaces it should be his responsibility for handling it, not the poor DSI host, which is just for converting native video signal (RGB I guess) to DSI. The other responsible for the mess is CRTC which actually should negotiate with panel how the data is split between both channels (left/right, top/bottom, odd/even, ). DSI-host here serves only as data transport, in fact it does not need to know if it works in dual mode or not (from HW point of view) - dual mode is just something which only CRTC and panel should be really aware. See what will happen to your solution if in next version of hardware vendor: 1. replace dual-panel with two panels glued together. 2. replace DSI bridges with different ones. 3. replace only one DSI briges - ie two different bridges working in dual mode. > > - Right now we have two data-points on dual-something voodoo > (tegra+rockchip) > So for me it sounds hard to design something generic that survives > the
[Bug 107168] Allow to load firmware during run-time (after initialization)
https://bugs.freedesktop.org/show_bug.cgi?id=107168 --- Comment #2 from Paul Menzel --- (In reply to Alex Deucher from comment #1) > You need the firmware for initialization. What’s the technical reason for this. Why can’t certain parts of the hardware be initialized later on? > Even if the firmware were open source, it would still needs to be loaded so > you'd still need firmware images. Understood. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [alsa-devel] ??????: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id
On Mon, 09 Jul 2018 17:59:00 +0200, Alex Deucher wrote: > > On Mon, Jul 9, 2018 at 11:52 AM, Takashi Iwai wrote: > > On Mon, 09 Jul 2018 17:47:34 +0200, > > Lukas Wunner wrote: > >> > >> On Mon, Jul 09, 2018 at 05:04:22PM +0200, Takashi Iwai wrote: > >> > On Mon, 09 Jul 2018 15:58:51 +0200, Alex Deucher wrote: > >> > > On Mon, Jul 9, 2018 at 6:16 AM, Qu, Jim wrote: > >> > > > > You're saying above that the HDA controller isn't runtime resumed > >> > > > > on > >> > > > > hotplug of a display. Is that necessary to retrieve ELD or > >> > > > > something? > >> > > > > I'm not sure if there's code in the HDA driver to acquire a > >> > > > > runtime PM > >> > > > > ref on HPD, but maybe it's necessary. Or maybe the code is there > >> > > > > but > >> > > > > somehow no HPD interrupt is received by the HDA driver? > >> > > > > >> > > > So far, I do not find any code about audio response HPD in kernel. > >> > > > the HPD interrupt will sent to user mode via uevent, not sure whether > >> > > > audio user mode driver can receive the event or not. > >> > > > >> > > On the gfx side at least, we can get a hotplug event via ACPI > >> > > (depending on the OEM design) if displays are attached/detached while > >> > > the dGPU is powered down. I suppose the gfx driver could call into > >> > > the audio driver during one of those events. On the gfx side at least > >> > > we just generate the gfx hotplug event and let userspace deal with it. > >> > > >> > IMO, a more proper way would be to have the direct communication > >> > between the graphics driver and the audio driver like i915 driver > >> > does. Then the audio driver can get plug/unplug event at more > >> > accurate timing without races. > >> > >> Since v4.17, every time the GPU is powered up, the HDA controller is > >> runtime resumed to PCI_D0. (See the call to pci_wakeup_bus() in > >> vga_switcheroo_runtime_resume() added by dcac86b7d0.) > >> > >> If the HDA controller can't detect presence of an HDMI display even > >> if it's in PCI_D0, then I suppose that needs to be adressed in the HDA > >> driver. Thus so far I don't see the need to call from amdgpu into the > >> HDA driver. > > > > It's not about the PCI power state, but the problem is that the > > detection of the HDMI and its ELD read is somewhat asynchronous. > > Basically it's handled via the hardware unsolicited event emitted over > > HD-audio bus, which was originally generated by GPU at dealing with > > the hotplug/unplug. So, this part is racy and not 100% reliable -- > > although usually it shouldn't be a big problem. > > > > That said, it's not the exact "need" but it would make things more > > reliable and accurate (even consumes less power as we don't need to > > power up/down just for the HDMI detection). > > Well for display detection, we don't actually have to wake the GPU, we > get the notification from the embedded controller via ACPI if the GPU > is powered down, so in the GPU driver we just pass the event along to > userspace if the GPU is asleep. I think if we wanted to pass the info > along to the audio driver, we'd need to wake both the GPU and the > audio to update the audio connect status. I guess that It's not enough to have an ACPI event but we need to read ELD upon hotplugging (not at unplugging, of course, though). So a GPU wakeup would be mandatory, if I'm not mistaken. Takashi ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [alsa-devel] ??????: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id
On Mon, Jul 9, 2018 at 11:52 AM, Takashi Iwai wrote: > On Mon, 09 Jul 2018 17:47:34 +0200, > Lukas Wunner wrote: >> >> On Mon, Jul 09, 2018 at 05:04:22PM +0200, Takashi Iwai wrote: >> > On Mon, 09 Jul 2018 15:58:51 +0200, Alex Deucher wrote: >> > > On Mon, Jul 9, 2018 at 6:16 AM, Qu, Jim wrote: >> > > > > You're saying above that the HDA controller isn't runtime resumed on >> > > > > hotplug of a display. Is that necessary to retrieve ELD or >> > > > > something? >> > > > > I'm not sure if there's code in the HDA driver to acquire a runtime >> > > > > PM >> > > > > ref on HPD, but maybe it's necessary. Or maybe the code is there but >> > > > > somehow no HPD interrupt is received by the HDA driver? >> > > > >> > > > So far, I do not find any code about audio response HPD in kernel. >> > > > the HPD interrupt will sent to user mode via uevent, not sure whether >> > > > audio user mode driver can receive the event or not. >> > > >> > > On the gfx side at least, we can get a hotplug event via ACPI >> > > (depending on the OEM design) if displays are attached/detached while >> > > the dGPU is powered down. I suppose the gfx driver could call into >> > > the audio driver during one of those events. On the gfx side at least >> > > we just generate the gfx hotplug event and let userspace deal with it. >> > >> > IMO, a more proper way would be to have the direct communication >> > between the graphics driver and the audio driver like i915 driver >> > does. Then the audio driver can get plug/unplug event at more >> > accurate timing without races. >> >> Since v4.17, every time the GPU is powered up, the HDA controller is >> runtime resumed to PCI_D0. (See the call to pci_wakeup_bus() in >> vga_switcheroo_runtime_resume() added by dcac86b7d0.) >> >> If the HDA controller can't detect presence of an HDMI display even >> if it's in PCI_D0, then I suppose that needs to be adressed in the HDA >> driver. Thus so far I don't see the need to call from amdgpu into the >> HDA driver. > > It's not about the PCI power state, but the problem is that the > detection of the HDMI and its ELD read is somewhat asynchronous. > Basically it's handled via the hardware unsolicited event emitted over > HD-audio bus, which was originally generated by GPU at dealing with > the hotplug/unplug. So, this part is racy and not 100% reliable -- > although usually it shouldn't be a big problem. > > That said, it's not the exact "need" but it would make things more > reliable and accurate (even consumes less power as we don't need to > power up/down just for the HDMI detection). Well for display detection, we don't actually have to wake the GPU, we get the notification from the embedded controller via ACPI if the GPU is powered down, so in the GPU driver we just pass the event along to userspace if the GPU is asleep. I think if we wanted to pass the info along to the audio driver, we'd need to wake both the GPU and the audio to update the audio connect status. Alex ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [alsa-devel] 答复: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id
On Mon, 09 Jul 2018 17:56:43 +0200, Daniel Vetter wrote: > > On Mon, Jul 09, 2018 at 05:04:22PM +0200, Takashi Iwai wrote: > > On Mon, 09 Jul 2018 15:58:51 +0200, > > Alex Deucher wrote: > > > > > > On Mon, Jul 9, 2018 at 6:16 AM, Qu, Jim wrote: > > > > Hi Lukas, > > > > > > > > Thanks to your explanation, and see comments in line. > > > > > > > > > > > > Do you need to runtime resume the HDA controller even if user space > > > > isn't > > > > streaming audio? Why, and in which situation exactly? > > > > > > > > Jim: OEM system uses pactl to queiry audio card and audio output sink, > > > > since the audio has power down by runtime pm, so the audio card and > > > > output sink are all unavailable. they could not select the available > > > > HDMI audio for audio playing. > > > > > > > > You're saying above that the HDA controller isn't runtime resumed on > > > > hotplug of a display. Is that necessary to retrieve ELD or something? > > > > I'm not sure if there's code in the HDA driver to acquire a runtime PM > > > > ref on HPD, but maybe it's necessary. Or maybe the code is there but > > > > somehow no HPD interrupt is received by the HDA driver? > > > > > > > > Jim: So far, I do not find any code about audio response HPD in kernel. > > > > the HPD interrupt will sent to user mode via uevent, not sure whether > > > > audio user mode driver can receive the event or not. > > > > > > On the gfx side at least, we can get a hotplug event via ACPI > > > (depending on the OEM design) if displays are attached/detached while > > > the dGPU is powered down. I suppose the gfx driver could call into > > > the audio driver during one of those events. On the gfx side at least > > > we just generate the gfx hotplug event and let userspace deal with it. > > > > IMO, a more proper way would be to have the direct communication > > between the graphics driver and the audio driver like i915 driver > > does. Then the audio driver can get plug/unplug event at more > > accurate timing without races. > > > > (Though, it will cause another mess wrt the weak module dependency, > > but it's another story :) > > Just don't do what snd-hda-intel did and instead implement the component > stuff properly :-) A patch is welcome :) thanks, Takashi > -Daniel > > > > > > > thanks, > > > > Takashi > > > > > > > > > > Alex > > > > > > > > > > > Thanks > > > > JimQu > > > > > > > > > > > > 发件人: Lukas Wunner > > > > 发送时间: 2018年7月9日 17:27 > > > > 收件人: Qu, Jim > > > > 抄送: alsa-de...@alsa-project.org; dri-devel@lists.freedesktop.org; > > > > Deucher, Alexander; amd-...@lists.freedesktop.org > > > > 主题: Re: [PATCH] vgaswitchroo: set audio client id according to bound > > > > gpu client id > > > > > > > > On Mon, Jul 09, 2018 at 08:52:33AM +, Qu, Jim wrote: > > > >> Now, I found the audio device will auto suspend even if the gpu is > > > >> active, > > > >> and if I plug in a HDMI device it also do not resume back. > > > >> > > > >> 1. Did you encounter similar issue before? > > > >> 2. audio will auto suspend as default at beginning of boot, is it > > > >> expect > > > >> behaviour? > > > > > > > > Yes, that's expected. Once you start streaming audio to attached > > > > displays, > > > > user space opens the codec device and this causes the HDA controller to > > > > runtime resume. If the discrete GPU is suspended at that moment, it > > > > will > > > > be powered on and kept powered on as long as user space is streaming > > > > audio. > > > > > > > > Do you need to runtime resume the HDA controller even if user space > > > > isn't > > > > streaming audio? Why, and in which situation exactly? > > > > > > > > You're saying above that the HDA controller isn't runtime resumed on > > > > hotplug of a display. Is that necessary to retrieve ELD or something? > > > > I'm not sure if there's code in the HDA driver to acquire a runtime PM > > > > ref on HPD, but maybe it's necessary. Or maybe the code is there but > > > > somehow no HPD interrupt is received by the HDA driver? > > > > > > > > Thanks, > > > > > > > > Lukas > > > > ___ > > > > dri-devel mailing list > > > > dri-devel@lists.freedesktop.org > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > ___ > > > Alsa-devel mailing list > > > alsa-de...@alsa-project.org > > > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: 答复: [alsa-devel] ??????: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id
On Mon, 09 Jul 2018 17:53:19 +0200, Qu, Jim wrote: > > Hi All, > > Here, I want to clarify the audio device is bound to iGPU. There is no audio > codec for dGPU. I'm confused. So you mean that the HDMI detection on Intel GPU doesn't work with the hybrid GPUs? And no audio codec on discrete GPU...? Why do we need vga_switcheroo at all, then? Takashi > > Thanks > JimQu > > -邮件原件- > 发件人: Lukas Wunner > 发送时间: 2018年7月9日 23:48 > 收件人: Takashi Iwai > 抄送: Alex Deucher ; alsa-de...@alsa-project.org; > amd-...@lists.freedesktop.org; Qu, Jim ; > dri-devel@lists.freedesktop.org; Deucher, Alexander > > 主题: Re: [alsa-devel] ??: [PATCH] vgaswitchroo: set audio client id > according to bound gpu client id > > On Mon, Jul 09, 2018 at 05:04:22PM +0200, Takashi Iwai wrote: > > On Mon, 09 Jul 2018 15:58:51 +0200, Alex Deucher wrote: > > > On Mon, Jul 9, 2018 at 6:16 AM, Qu, Jim wrote: > > > > > You're saying above that the HDA controller isn't runtime > > > > > resumed on hotplug of a display. Is that necessary to retrieve ELD > > > > > or something? > > > > > I'm not sure if there's code in the HDA driver to acquire a > > > > > runtime PM ref on HPD, but maybe it's necessary. Or maybe the > > > > > code is there but somehow no HPD interrupt is received by the HDA > > > > > driver? > > > > > > > > So far, I do not find any code about audio response HPD in kernel. > > > > the HPD interrupt will sent to user mode via uevent, not sure > > > > whether audio user mode driver can receive the event or not. > > > > > > On the gfx side at least, we can get a hotplug event via ACPI > > > (depending on the OEM design) if displays are attached/detached > > > while the dGPU is powered down. I suppose the gfx driver could call > > > into the audio driver during one of those events. On the gfx side > > > at least we just generate the gfx hotplug event and let userspace deal > > > with it. > > > > IMO, a more proper way would be to have the direct communication > > between the graphics driver and the audio driver like i915 driver > > does. Then the audio driver can get plug/unplug event at more > > accurate timing without races. > > Since v4.17, every time the GPU is powered up, the HDA controller is runtime > resumed to PCI_D0. (See the call to pci_wakeup_bus() in > vga_switcheroo_runtime_resume() added by dcac86b7d0.) > > If the HDA controller can't detect presence of an HDMI display even if it's > in PCI_D0, then I suppose that needs to be adressed in the HDA driver. Thus > so far I don't see the need to call from amdgpu into the HDA driver. > > Thanks, > > Lukas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [alsa-devel] 答复: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id
On Mon, Jul 09, 2018 at 05:04:22PM +0200, Takashi Iwai wrote: > On Mon, 09 Jul 2018 15:58:51 +0200, > Alex Deucher wrote: > > > > On Mon, Jul 9, 2018 at 6:16 AM, Qu, Jim wrote: > > > Hi Lukas, > > > > > > Thanks to your explanation, and see comments in line. > > > > > > > > > Do you need to runtime resume the HDA controller even if user space isn't > > > streaming audio? Why, and in which situation exactly? > > > > > > Jim: OEM system uses pactl to queiry audio card and audio output sink, > > > since the audio has power down by runtime pm, so the audio card and > > > output sink are all unavailable. they could not select the available HDMI > > > audio for audio playing. > > > > > > You're saying above that the HDA controller isn't runtime resumed on > > > hotplug of a display. Is that necessary to retrieve ELD or something? > > > I'm not sure if there's code in the HDA driver to acquire a runtime PM > > > ref on HPD, but maybe it's necessary. Or maybe the code is there but > > > somehow no HPD interrupt is received by the HDA driver? > > > > > > Jim: So far, I do not find any code about audio response HPD in kernel. > > > the HPD interrupt will sent to user mode via uevent, not sure whether > > > audio user mode driver can receive the event or not. > > > > On the gfx side at least, we can get a hotplug event via ACPI > > (depending on the OEM design) if displays are attached/detached while > > the dGPU is powered down. I suppose the gfx driver could call into > > the audio driver during one of those events. On the gfx side at least > > we just generate the gfx hotplug event and let userspace deal with it. > > IMO, a more proper way would be to have the direct communication > between the graphics driver and the audio driver like i915 driver > does. Then the audio driver can get plug/unplug event at more > accurate timing without races. > > (Though, it will cause another mess wrt the weak module dependency, > but it's another story :) Just don't do what snd-hda-intel did and instead implement the component stuff properly :-) -Daniel > > > thanks, > > Takashi > > > > > > Alex > > > > > > > > Thanks > > > JimQu > > > > > > > > > 发件人: Lukas Wunner > > > 发送时间: 2018年7月9日 17:27 > > > 收件人: Qu, Jim > > > 抄送: alsa-de...@alsa-project.org; dri-devel@lists.freedesktop.org; > > > Deucher, Alexander; amd-...@lists.freedesktop.org > > > 主题: Re: [PATCH] vgaswitchroo: set audio client id according to bound gpu > > > client id > > > > > > On Mon, Jul 09, 2018 at 08:52:33AM +, Qu, Jim wrote: > > >> Now, I found the audio device will auto suspend even if the gpu is > > >> active, > > >> and if I plug in a HDMI device it also do not resume back. > > >> > > >> 1. Did you encounter similar issue before? > > >> 2. audio will auto suspend as default at beginning of boot, is it expect > > >> behaviour? > > > > > > Yes, that's expected. Once you start streaming audio to attached > > > displays, > > > user space opens the codec device and this causes the HDA controller to > > > runtime resume. If the discrete GPU is suspended at that moment, it will > > > be powered on and kept powered on as long as user space is streaming > > > audio. > > > > > > Do you need to runtime resume the HDA controller even if user space isn't > > > streaming audio? Why, and in which situation exactly? > > > > > > You're saying above that the HDA controller isn't runtime resumed on > > > hotplug of a display. Is that necessary to retrieve ELD or something? > > > I'm not sure if there's code in the HDA driver to acquire a runtime PM > > > ref on HPD, but maybe it's necessary. Or maybe the code is there but > > > somehow no HPD interrupt is received by the HDA driver? > > > > > > Thanks, > > > > > > Lukas > > > ___ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > ___ > > Alsa-devel mailing list > > alsa-de...@alsa-project.org > > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107168] Allow to load firmware during run-time (after initialization)
https://bugs.freedesktop.org/show_bug.cgi?id=107168 Alex Deucher changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |WONTFIX --- Comment #1 from Alex Deucher --- You need the firmware for initialization. Even if the firmware were open source, it would still needs to be loaded so you'd still need firmware images. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
答复: [alsa-devel] ??????: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id
Hi All, Here, I want to clarify the audio device is bound to iGPU. There is no audio codec for dGPU. Thanks JimQu -邮件原件- 发件人: Lukas Wunner 发送时间: 2018年7月9日 23:48 收件人: Takashi Iwai 抄送: Alex Deucher ; alsa-de...@alsa-project.org; amd-...@lists.freedesktop.org; Qu, Jim ; dri-devel@lists.freedesktop.org; Deucher, Alexander 主题: Re: [alsa-devel] ??: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id On Mon, Jul 09, 2018 at 05:04:22PM +0200, Takashi Iwai wrote: > On Mon, 09 Jul 2018 15:58:51 +0200, Alex Deucher wrote: > > On Mon, Jul 9, 2018 at 6:16 AM, Qu, Jim wrote: > > > > You're saying above that the HDA controller isn't runtime > > > > resumed on hotplug of a display. Is that necessary to retrieve ELD or > > > > something? > > > > I'm not sure if there's code in the HDA driver to acquire a > > > > runtime PM ref on HPD, but maybe it's necessary. Or maybe the > > > > code is there but somehow no HPD interrupt is received by the HDA > > > > driver? > > > > > > So far, I do not find any code about audio response HPD in kernel. > > > the HPD interrupt will sent to user mode via uevent, not sure > > > whether audio user mode driver can receive the event or not. > > > > On the gfx side at least, we can get a hotplug event via ACPI > > (depending on the OEM design) if displays are attached/detached > > while the dGPU is powered down. I suppose the gfx driver could call > > into the audio driver during one of those events. On the gfx side > > at least we just generate the gfx hotplug event and let userspace deal with > > it. > > IMO, a more proper way would be to have the direct communication > between the graphics driver and the audio driver like i915 driver > does. Then the audio driver can get plug/unplug event at more > accurate timing without races. Since v4.17, every time the GPU is powered up, the HDA controller is runtime resumed to PCI_D0. (See the call to pci_wakeup_bus() in vga_switcheroo_runtime_resume() added by dcac86b7d0.) If the HDA controller can't detect presence of an HDMI display even if it's in PCI_D0, then I suppose that needs to be adressed in the HDA driver. Thus so far I don't see the need to call from amdgpu into the HDA driver. Thanks, Lukas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [alsa-devel] ??????: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id
On Mon, 09 Jul 2018 17:47:34 +0200, Lukas Wunner wrote: > > On Mon, Jul 09, 2018 at 05:04:22PM +0200, Takashi Iwai wrote: > > On Mon, 09 Jul 2018 15:58:51 +0200, Alex Deucher wrote: > > > On Mon, Jul 9, 2018 at 6:16 AM, Qu, Jim wrote: > > > > > You're saying above that the HDA controller isn't runtime resumed on > > > > > hotplug of a display. Is that necessary to retrieve ELD or something? > > > > > I'm not sure if there's code in the HDA driver to acquire a runtime PM > > > > > ref on HPD, but maybe it's necessary. Or maybe the code is there but > > > > > somehow no HPD interrupt is received by the HDA driver? > > > > > > > > So far, I do not find any code about audio response HPD in kernel. > > > > the HPD interrupt will sent to user mode via uevent, not sure whether > > > > audio user mode driver can receive the event or not. > > > > > > On the gfx side at least, we can get a hotplug event via ACPI > > > (depending on the OEM design) if displays are attached/detached while > > > the dGPU is powered down. I suppose the gfx driver could call into > > > the audio driver during one of those events. On the gfx side at least > > > we just generate the gfx hotplug event and let userspace deal with it. > > > > IMO, a more proper way would be to have the direct communication > > between the graphics driver and the audio driver like i915 driver > > does. Then the audio driver can get plug/unplug event at more > > accurate timing without races. > > Since v4.17, every time the GPU is powered up, the HDA controller is > runtime resumed to PCI_D0. (See the call to pci_wakeup_bus() in > vga_switcheroo_runtime_resume() added by dcac86b7d0.) > > If the HDA controller can't detect presence of an HDMI display even > if it's in PCI_D0, then I suppose that needs to be adressed in the HDA > driver. Thus so far I don't see the need to call from amdgpu into the > HDA driver. It's not about the PCI power state, but the problem is that the detection of the HDMI and its ELD read is somewhat asynchronous. Basically it's handled via the hardware unsolicited event emitted over HD-audio bus, which was originally generated by GPU at dealing with the hotplug/unplug. So, this part is racy and not 100% reliable -- although usually it shouldn't be a big problem. That said, it's not the exact "need" but it would make things more reliable and accurate (even consumes less power as we don't need to power up/down just for the HDMI detection). Takashi ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 11/12] sched: use for_each_if in topology.h
On Mon, Jul 09, 2018 at 05:12:58PM +0200, Peter Zijlstra wrote: > On Mon, Jul 09, 2018 at 05:00:07PM +0200, Daniel Vetter wrote: > > On Mon, Jul 9, 2018 at 12:36 PM, Peter Zijlstra > > wrote: > > > On Mon, Jul 09, 2018 at 10:36:49AM +0200, Daniel Vetter wrote: > > > >> #define for_each_node_with_cpus(node)\ > > >> for_each_online_node(node) \ > > >> - if (nr_cpus_node(node)) > > >> + for_each_if (nr_cpus_node(node)) > > > > > > Not having gotten any of the other patches, I'm not really sure what > > > this does and such, but improve readability it does not :/ > > > > Patch 1 in this series, which I dumped onto lkml as a whole: > > > > https://lkml.org/lkml/2018/7/9/179 > > Right, so while I don't object to being Cc'ed to the whole series, I do > mind not being Cc'ed to at least the generic bits required to understand > the patch I do have to look at. > > > Imo it does improve readability for the if (!cond) {} else pattern. > > And (assuming my grep fu isn't too badly wrong) most places in the > > kernel do use this pattern in for_each macros, so I guess its a real > > thing. We've definitely hit it plenty in drm iterators (but we seem to > > like if() checks in iterator macros maybe a bit too much). > > > > I'm happy to drop this patch tough if you deem it offensive. > > I'd just like to understand it better; what compiler complains about > this and is the warning otherwise useful? These things don't seem > mentioned in that initial patch either. > > IOW I suppose I'm asking for the justification of this churn. If it's > really needed and useful so be it, but so far I'm not seeing any. > > At a while guess I'd say this is something new in gcc-8 (and while I > have that installed on some machines, it doesn't seem to be the default, > and so I've not actually seen its output). But is the warning actually > useful, should we not just kill the warning like we tend to do some > really silly ones. for_each_something(foo) if (foo->bla) call_bla(foo); else call_default(foo); Totally contrived, but this complains. Liberally sprinkling {} also shuts up the compiler, but it's a bit confusing given that a plain for {;;} is totally fine. And it's confusing since at first glance the compiler complaining about nested if and ambigous else doesn't make sense since clearly there's only 1 if there. Wrt this being useful or not: We've had it for a while in drm, and Andy and Yishen where rolling yet another open coded version of this on a patch that flew past me on dri-devel. So I pointed them at the for_each_if() we have and typed this series to move it to kernel.h. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107168] Allow to load firmware during run-time (after initialization)
https://bugs.freedesktop.org/show_bug.cgi?id=107168 Bug ID: 107168 Summary: Allow to load firmware during run-time (after initialization) Product: DRI Version: DRI git Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: DRM/Radeon Assignee: dri-devel@lists.freedesktop.org Reporter: pmenzel+bugs.freedesk...@molgen.mpg.de Building the radeon driver into the Linux kernel, that means not as a module, *without* an initrd image, the firmware is not loaded. The firmware is present on the filesystem in `/lib/firmware/`, but it has not been mounted yet. The firmware can be added using `EXTRA_FIRMWARE`, but that makes it difficult if the Linux kernel should be shared between different systems using different hardware generations. (The best solution would be to not require firmware at all.) It’d be great if the driver supported to load firmware after the initialization phase. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: vkms: select DRM_KMS_HELPER
Without this, we get link errors during randconfig build: drivers/gpu/drm/vkms/vkms_drv.o:(.rodata+0xa0): undefined reference to `drm_atomic_helper_check' drivers/gpu/drm/vkms/vkms_drv.o:(.rodata+0xa8): undefined reference to `drm_atomic_helper_commit' drivers/gpu/drm/vkms/vkms_plane.o:(.rodata+0x0): undefined reference to `drm_atomic_helper_update_plane' drivers/gpu/drm/vkms/vkms_plane.o:(.rodata+0x8): undefined reference to `drm_atomic_helper_disable_plane' drivers/gpu/drm/vkms/vkms_plane.o:(.rodata+0x18): undefined reference to `drm_atomic_helper_plane_reset' drivers/gpu/drm/vkms/vkms_plane.o:(.rodata+0x28): undefined reference to `drm_atomic_helper_plane_duplicate_state' drivers/gpu/drm/vkms/vkms_plane.o:(.rodata+0x30): undefined reference to `drm_atomic_helper_plane_destroy_state' drivers/gpu/drm/vkms/vkms_output.o:(.rodata+0x1c0): undefined reference to `drm_helper_probe_single_connector_modes' drivers/gpu/drm/vkms/vkms_crtc.o:(.rodata+0x40): undefined reference to `drm_atomic_helper_crtc_reset' drivers/gpu/drm/vkms/vkms_crtc.o:(.rodata+0x70): undefined reference to `drm_atomic_helper_set_config' drivers/gpu/drm/vkms/vkms_crtc.o:(.rodata+0x78): undefined reference to `drm_atomic_helper_page_flip' drivers/gpu/drm/vkms/vkms_crtc.o:(.rodata+0x90): undefined reference to `drm_atomic_helper_crtc_duplicate_state' drivers/gpu/drm/vkms/vkms_crtc.o:(.rodata+0x98): undefined reference to `drm_atomic_helper_crtc_destroy_state' Fixes: 854502fa0a38 ("drm/vkms: Add basic CRTC initialization") Fixes: 1c7c5fd916a0 ("drm/vkms: Introduce basic VKMS driver") Signed-off-by: Arnd Bergmann --- drivers/gpu/drm/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 516a8f5d1ac6..e527772f5ca6 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -214,6 +214,7 @@ config DRM_VGEM config DRM_VKMS tristate "Virtual KMS (EXPERIMENTAL)" depends on DRM + select DRM_KMS_HELPER default n help Virtual Kernel Mode-Setting (VKMS) is used for testing or for -- 2.9.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [alsa-devel] ??????: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id
On Mon, Jul 09, 2018 at 05:04:22PM +0200, Takashi Iwai wrote: > On Mon, 09 Jul 2018 15:58:51 +0200, Alex Deucher wrote: > > On Mon, Jul 9, 2018 at 6:16 AM, Qu, Jim wrote: > > > > You're saying above that the HDA controller isn't runtime resumed on > > > > hotplug of a display. Is that necessary to retrieve ELD or something? > > > > I'm not sure if there's code in the HDA driver to acquire a runtime PM > > > > ref on HPD, but maybe it's necessary. Or maybe the code is there but > > > > somehow no HPD interrupt is received by the HDA driver? > > > > > > So far, I do not find any code about audio response HPD in kernel. > > > the HPD interrupt will sent to user mode via uevent, not sure whether > > > audio user mode driver can receive the event or not. > > > > On the gfx side at least, we can get a hotplug event via ACPI > > (depending on the OEM design) if displays are attached/detached while > > the dGPU is powered down. I suppose the gfx driver could call into > > the audio driver during one of those events. On the gfx side at least > > we just generate the gfx hotplug event and let userspace deal with it. > > IMO, a more proper way would be to have the direct communication > between the graphics driver and the audio driver like i915 driver > does. Then the audio driver can get plug/unplug event at more > accurate timing without races. Since v4.17, every time the GPU is powered up, the HDA controller is runtime resumed to PCI_D0. (See the call to pci_wakeup_bus() in vga_switcheroo_runtime_resume() added by dcac86b7d0.) If the HDA controller can't detect presence of an HDMI display even if it's in PCI_D0, then I suppose that needs to be adressed in the HDA driver. Thus so far I don't see the need to call from amdgpu into the HDA driver. Thanks, Lukas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC COMP I/F] drm/i915: Initialize HDCP2.2 and its MEI interface
On Monday 09 July 2018 08:40 PM, Daniel Vetter wrote: On Mon, Jul 09, 2018 at 07:01:09PM +0530, Ramalingam C wrote: Initialize HDCP2.2 support. This includes the mei interface initialization along with required component registration. v2: mei interface handle is protected with mutex. [Chris Wilson] v3: Notifiers are used for the mei interface state. v4: Poll for mei client device state Error msg for out of mem [Uma] Inline req for init function removed [Uma] v5: Rebase as Part of reordering. Component is used for the I915 and MEI_HDCP interface [Daniel] v6: I915 registration is moved into hdcp component bind [Daniel] Signed-off-by: Ramalingam C --- drivers/gpu/drm/i915/i915_drv.c | 33 +-- drivers/gpu/drm/i915/i915_drv.h | 3 + drivers/gpu/drm/i915/intel_dp.c | 3 +- drivers/gpu/drm/i915/intel_drv.h | 6 +- drivers/gpu/drm/i915/intel_hdcp.c | 193 +- drivers/gpu/drm/i915/intel_hdmi.c | 2 +- include/drm/i915_component.h | 88 + 7 files changed, 314 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 0db3c83cce29..e6dc0f409a6e 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1310,6 +1310,19 @@ static void i915_welcome_messages(struct drm_i915_private *dev_priv) DRM_INFO("DRM_I915_DEBUG_GEM enabled\n"); } +void i915_driver_load_tail(struct drm_i915_private *dev_priv) +{ This isn't quite enough, we also must delay anything that might do a modeset. For almost everything this happens when we call i915_driver_register, but the fbdev emulation gets set up before that. We need to also include that (and everything after that point) in the load_tail function here. Ok. I will check that. + i915_driver_register(dev_priv); + + intel_runtime_pm_enable(dev_priv); + + intel_init_ipc(dev_priv); + + intel_runtime_pm_put(dev_priv); + + i915_welcome_messages(dev_priv); +} + /** * i915_driver_load - setup chip and create an initial config * @pdev: PCI device @@ -1389,16 +1402,18 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret < 0) goto out_cleanup_hw; - i915_driver_register(dev_priv); - - intel_runtime_pm_enable(dev_priv); - - intel_init_ipc(dev_priv); - - intel_runtime_pm_put(dev_priv); - - i915_welcome_messages(dev_priv); + if (is_hdcp2_supported(dev_priv)) { + ret = intel_hdcp_component_init(dev_priv); + if (ret) { + DRM_DEV_ERROR(>dev, "HDCP comp init failed %d\n", + ret); + goto out_cleanup_hw; + } + DRM_DEV_INFO(>dev, "I915 waits for MEI HDCP bind.\n"); + return 0; + } + i915_driver_load_tail(dev_priv); Yup, this is roughly what I had in mind. The only slightly annoying thing is that this sprinkles is_hdcp2_supported() checks all over the codebase. What I had in mind instead to have slightly more polished code, and make this componentn business more useful in general for us: - Put the component master into i915_dev_priv instead of hiding it in the hdcp structure. Even now component is in dev_priv only. - Create a 2nd component that fires when driver loading is complete (i.e. right here). This can be a really dumb one with nothing as callbacks. You mean register another client component in i915_hdcp_master_comp bind which will trigger a bind for master component that is used to call load tail? - Put the driver_load_tail call unconditionally into the master_bind, and put the component_master_add_with_match() call also here. - Don't move the hdcp init around since that's no longer necessary. return 0; out_cleanup_hw: diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 09ab12458244..febb8c8dc457 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2119,6 +2119,8 @@ struct drm_i915_private { struct i915_pmu pmu; + struct i915_hdcp_component *hdcp_comp; + /* * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch * will be rejected. Instead look for a better place. @@ -2732,6 +2734,7 @@ int intel_engines_init_mmio(struct drm_i915_private *dev_priv); int intel_engines_init(struct drm_i915_private *dev_priv); u32 intel_calculate_mcr_s_ss_select(struct drm_i915_private *dev_priv); +void i915_driver_load_tail(struct drm_i915_private *dev_priv); /* intel_hotplug.c */ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv, diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 5be07e1d816d..12eb5bd33b7e 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -6406,7
[PATCH] drm/tinydrm: add backlight dependency for ili9341
This tinydrm driver fails to link without the backlight support: drivers/gpu/drm/tinydrm/ili9341.o: In function `ili9341_probe': ili9341.c:(.text+0x578): undefined reference to `devm_of_find_backlight' Fixes: 3fa0e8f6f960 ("drm/tinydrm: new driver for ILI9341 display panels") Signed-off-by: Arnd Bergmann --- drivers/gpu/drm/tinydrm/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/tinydrm/Kconfig b/drivers/gpu/drm/tinydrm/Kconfig index 7a8008b0783f..16f4b5c91f1b 100644 --- a/drivers/gpu/drm/tinydrm/Kconfig +++ b/drivers/gpu/drm/tinydrm/Kconfig @@ -23,6 +23,7 @@ config TINYDRM_ILI9225 config TINYDRM_ILI9341 tristate "DRM support for ILI9341 display panels" depends on DRM_TINYDRM && SPI + depends on BACKLIGHT_CLASS_DEVICE select TINYDRM_MIPI_DBI help DRM driver for the following Ilitek ILI9341 panels: -- 2.9.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 11/12] sched: use for_each_if in topology.h
On Mon, Jul 09, 2018 at 05:00:07PM +0200, Daniel Vetter wrote: > On Mon, Jul 9, 2018 at 12:36 PM, Peter Zijlstra wrote: > > On Mon, Jul 09, 2018 at 10:36:49AM +0200, Daniel Vetter wrote: > >> #define for_each_node_with_cpus(node)\ > >> for_each_online_node(node) \ > >> - if (nr_cpus_node(node)) > >> + for_each_if (nr_cpus_node(node)) > > > > Not having gotten any of the other patches, I'm not really sure what > > this does and such, but improve readability it does not :/ > > Patch 1 in this series, which I dumped onto lkml as a whole: > > https://lkml.org/lkml/2018/7/9/179 Right, so while I don't object to being Cc'ed to the whole series, I do mind not being Cc'ed to at least the generic bits required to understand the patch I do have to look at. > Imo it does improve readability for the if (!cond) {} else pattern. > And (assuming my grep fu isn't too badly wrong) most places in the > kernel do use this pattern in for_each macros, so I guess its a real > thing. We've definitely hit it plenty in drm iterators (but we seem to > like if() checks in iterator macros maybe a bit too much). > > I'm happy to drop this patch tough if you deem it offensive. I'd just like to understand it better; what compiler complains about this and is the warning otherwise useful? These things don't seem mentioned in that initial patch either. IOW I suppose I'm asking for the justification of this churn. If it's really needed and useful so be it, but so far I'm not seeing any. At a while guess I'd say this is something new in gcc-8 (and while I have that installed on some machines, it doesn't seem to be the default, and so I've not actually seen its output). But is the warning actually useful, should we not just kill the warning like we tend to do some really silly ones. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC COMP I/F] drm/i915: Initialize HDCP2.2 and its MEI interface
On Mon, Jul 09, 2018 at 07:01:09PM +0530, Ramalingam C wrote: > Initialize HDCP2.2 support. This includes the mei interface > initialization along with required component registration. > > v2: > mei interface handle is protected with mutex. [Chris Wilson] > v3: > Notifiers are used for the mei interface state. > v4: > Poll for mei client device state > Error msg for out of mem [Uma] > Inline req for init function removed [Uma] > v5: > Rebase as Part of reordering. > Component is used for the I915 and MEI_HDCP interface [Daniel] > v6: > I915 registration is moved into hdcp component bind [Daniel] > > Signed-off-by: Ramalingam C > --- > drivers/gpu/drm/i915/i915_drv.c | 33 +-- > drivers/gpu/drm/i915/i915_drv.h | 3 + > drivers/gpu/drm/i915/intel_dp.c | 3 +- > drivers/gpu/drm/i915/intel_drv.h | 6 +- > drivers/gpu/drm/i915/intel_hdcp.c | 193 > +- > drivers/gpu/drm/i915/intel_hdmi.c | 2 +- > include/drm/i915_component.h | 88 + > 7 files changed, 314 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 0db3c83cce29..e6dc0f409a6e 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1310,6 +1310,19 @@ static void i915_welcome_messages(struct > drm_i915_private *dev_priv) > DRM_INFO("DRM_I915_DEBUG_GEM enabled\n"); > } > > +void i915_driver_load_tail(struct drm_i915_private *dev_priv) > +{ This isn't quite enough, we also must delay anything that might do a modeset. For almost everything this happens when we call i915_driver_register, but the fbdev emulation gets set up before that. We need to also include that (and everything after that point) in the load_tail function here. > + i915_driver_register(dev_priv); > + > + intel_runtime_pm_enable(dev_priv); > + > + intel_init_ipc(dev_priv); > + > + intel_runtime_pm_put(dev_priv); > + > + i915_welcome_messages(dev_priv); > +} > + > /** > * i915_driver_load - setup chip and create an initial config > * @pdev: PCI device > @@ -1389,16 +1402,18 @@ int i915_driver_load(struct pci_dev *pdev, const > struct pci_device_id *ent) > if (ret < 0) > goto out_cleanup_hw; > > - i915_driver_register(dev_priv); > - > - intel_runtime_pm_enable(dev_priv); > - > - intel_init_ipc(dev_priv); > - > - intel_runtime_pm_put(dev_priv); > - > - i915_welcome_messages(dev_priv); > + if (is_hdcp2_supported(dev_priv)) { > + ret = intel_hdcp_component_init(dev_priv); > + if (ret) { > + DRM_DEV_ERROR(>dev, "HDCP comp init failed %d\n", > + ret); > + goto out_cleanup_hw; > + } > + DRM_DEV_INFO(>dev, "I915 waits for MEI HDCP bind.\n"); > + return 0; > + } > > + i915_driver_load_tail(dev_priv); Yup, this is roughly what I had in mind. The only slightly annoying thing is that this sprinkles is_hdcp2_supported() checks all over the codebase. What I had in mind instead to have slightly more polished code, and make this componentn business more useful in general for us: - Put the component master into i915_dev_priv instead of hiding it in the hdcp structure. - Create a 2nd component that fires when driver loading is complete (i.e. right here). This can be a really dumb one with nothing as callbacks. - Put the driver_load_tail call unconditionally into the master_bind, and put the component_master_add_with_match() call also here. - Don't move the hdcp init around since that's no longer necessary. > return 0; > > out_cleanup_hw: > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 09ab12458244..febb8c8dc457 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2119,6 +2119,8 @@ struct drm_i915_private { > > struct i915_pmu pmu; > > + struct i915_hdcp_component *hdcp_comp; > + > /* >* NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch >* will be rejected. Instead look for a better place. > @@ -2732,6 +2734,7 @@ int intel_engines_init_mmio(struct drm_i915_private > *dev_priv); > int intel_engines_init(struct drm_i915_private *dev_priv); > > u32 intel_calculate_mcr_s_ss_select(struct drm_i915_private *dev_priv); > +void i915_driver_load_tail(struct drm_i915_private *dev_priv); > > /* intel_hotplug.c */ > void intel_hpd_irq_handler(struct drm_i915_private *dev_priv, > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 5be07e1d816d..12eb5bd33b7e 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -6406,7 +6406,8 @@ intel_dp_init_connector(struct intel_digital_port > *intel_dig_port, > intel_dp_add_properties(intel_dp, connector); > >
Re: [alsa-devel] 答复: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id
On Mon, 09 Jul 2018 16:02:48 +0200, Alex Deucher wrote: > > On Mon, Jul 9, 2018 at 6:16 AM, Qu, Jim wrote: > > Hi Lukas, > > > > Thanks to your explanation, and see comments in line. > > > > > > Do you need to runtime resume the HDA controller even if user space isn't > > streaming audio? Why, and in which situation exactly? > > > > Jim: OEM system uses pactl to queiry audio card and audio output sink, > > since the audio has power down by runtime pm, so the audio card and output > > sink are all unavailable. they could not select the available HDMI audio > > for audio playing. > > > Sounds like a bug in the HDA driver. The HDA driver should cache what > it needs or power it back up when there is an ioctl/etc. that requires > something that can't be cached. Actually I'm not sure whether the analysis above is correct. If PA shows it's unavailable, it implies rather that the HDMI is seen as unconnected. PA doesn't matter about the runtime PM state. If a runtime PM fails, it'd result in a "failure", not "unavailable". Takashi > > Alex > > > > > You're saying above that the HDA controller isn't runtime resumed on > > hotplug of a display. Is that necessary to retrieve ELD or something? > > I'm not sure if there's code in the HDA driver to acquire a runtime PM > > ref on HPD, but maybe it's necessary. Or maybe the code is there but > > somehow no HPD interrupt is received by the HDA driver? > > > > Jim: So far, I do not find any code about audio response HPD in kernel. the > > HPD interrupt will sent to user mode via uevent, not sure whether audio > > user mode driver can receive the event or not. > > > > Thanks > > JimQu > > > > > > 发件人: Lukas Wunner > > 发送时间: 2018年7月9日 17:27 > > 收件人: Qu, Jim > > 抄送: alsa-de...@alsa-project.org; dri-devel@lists.freedesktop.org; Deucher, > > Alexander; amd-...@lists.freedesktop.org > > 主题: Re: [PATCH] vgaswitchroo: set audio client id according to bound gpu > > client id > > > > On Mon, Jul 09, 2018 at 08:52:33AM +, Qu, Jim wrote: > >> Now, I found the audio device will auto suspend even if the gpu is active, > >> and if I plug in a HDMI device it also do not resume back. > >> > >> 1. Did you encounter similar issue before? > >> 2. audio will auto suspend as default at beginning of boot, is it expect > >> behaviour? > > > > Yes, that's expected. Once you start streaming audio to attached displays, > > user space opens the codec device and this causes the HDA controller to > > runtime resume. If the discrete GPU is suspended at that moment, it will > > be powered on and kept powered on as long as user space is streaming audio. > > > > Do you need to runtime resume the HDA controller even if user space isn't > > streaming audio? Why, and in which situation exactly? > > > > You're saying above that the HDA controller isn't runtime resumed on > > hotplug of a display. Is that necessary to retrieve ELD or something? > > I'm not sure if there's code in the HDA driver to acquire a runtime PM > > ref on HPD, but maybe it's necessary. Or maybe the code is there but > > somehow no HPD interrupt is received by the HDA driver? > > > > Thanks, > > > > Lukas > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ > Alsa-devel mailing list > alsa-de...@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [alsa-devel] 答复: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id
On Mon, 09 Jul 2018 15:58:51 +0200, Alex Deucher wrote: > > On Mon, Jul 9, 2018 at 6:16 AM, Qu, Jim wrote: > > Hi Lukas, > > > > Thanks to your explanation, and see comments in line. > > > > > > Do you need to runtime resume the HDA controller even if user space isn't > > streaming audio? Why, and in which situation exactly? > > > > Jim: OEM system uses pactl to queiry audio card and audio output sink, > > since the audio has power down by runtime pm, so the audio card and output > > sink are all unavailable. they could not select the available HDMI audio > > for audio playing. > > > > You're saying above that the HDA controller isn't runtime resumed on > > hotplug of a display. Is that necessary to retrieve ELD or something? > > I'm not sure if there's code in the HDA driver to acquire a runtime PM > > ref on HPD, but maybe it's necessary. Or maybe the code is there but > > somehow no HPD interrupt is received by the HDA driver? > > > > Jim: So far, I do not find any code about audio response HPD in kernel. the > > HPD interrupt will sent to user mode via uevent, not sure whether audio > > user mode driver can receive the event or not. > > On the gfx side at least, we can get a hotplug event via ACPI > (depending on the OEM design) if displays are attached/detached while > the dGPU is powered down. I suppose the gfx driver could call into > the audio driver during one of those events. On the gfx side at least > we just generate the gfx hotplug event and let userspace deal with it. IMO, a more proper way would be to have the direct communication between the graphics driver and the audio driver like i915 driver does. Then the audio driver can get plug/unplug event at more accurate timing without races. (Though, it will cause another mess wrt the weak module dependency, but it's another story :) thanks, Takashi > > Alex > > > > > Thanks > > JimQu > > > > > > 发件人: Lukas Wunner > > 发送时间: 2018年7月9日 17:27 > > 收件人: Qu, Jim > > 抄送: alsa-de...@alsa-project.org; dri-devel@lists.freedesktop.org; Deucher, > > Alexander; amd-...@lists.freedesktop.org > > 主题: Re: [PATCH] vgaswitchroo: set audio client id according to bound gpu > > client id > > > > On Mon, Jul 09, 2018 at 08:52:33AM +, Qu, Jim wrote: > >> Now, I found the audio device will auto suspend even if the gpu is active, > >> and if I plug in a HDMI device it also do not resume back. > >> > >> 1. Did you encounter similar issue before? > >> 2. audio will auto suspend as default at beginning of boot, is it expect > >> behaviour? > > > > Yes, that's expected. Once you start streaming audio to attached displays, > > user space opens the codec device and this causes the HDA controller to > > runtime resume. If the discrete GPU is suspended at that moment, it will > > be powered on and kept powered on as long as user space is streaming audio. > > > > Do you need to runtime resume the HDA controller even if user space isn't > > streaming audio? Why, and in which situation exactly? > > > > You're saying above that the HDA controller isn't runtime resumed on > > hotplug of a display. Is that necessary to retrieve ELD or something? > > I'm not sure if there's code in the HDA driver to acquire a runtime PM > > ref on HPD, but maybe it's necessary. Or maybe the code is there but > > somehow no HPD interrupt is received by the HDA driver? > > > > Thanks, > > > > Lukas > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ > Alsa-devel mailing list > alsa-de...@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 11/12] sched: use for_each_if in topology.h
On Mon, Jul 9, 2018 at 12:36 PM, Peter Zijlstra wrote: > On Mon, Jul 09, 2018 at 10:36:49AM +0200, Daniel Vetter wrote: >> Avoids complaints from gcc about ambiguous else clauses. > > Is that a new thing? I'm fairly sure I've never seen it do that, > >> Signed-off-by: Daniel Vetter >> Cc: Andrew Morton >> Cc: Peter Zijlstra >> --- >> include/linux/topology.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/include/linux/topology.h b/include/linux/topology.h >> index cb0775e1ee4b..4fba5a5b148d 100644 >> --- a/include/linux/topology.h >> +++ b/include/linux/topology.h >> @@ -40,7 +40,7 @@ >> >> #define for_each_node_with_cpus(node)\ >> for_each_online_node(node) \ >> - if (nr_cpus_node(node)) >> + for_each_if (nr_cpus_node(node)) > > Not having gotten any of the other patches, I'm not really sure what > this does and such, but improve readability it does not :/ Patch 1 in this series, which I dumped onto lkml as a whole: https://lkml.org/lkml/2018/7/9/179 Imo it does improve readability for the if (!cond) {} else pattern. And (assuming my grep fu isn't too badly wrong) most places in the kernel do use this pattern in for_each macros, so I guess its a real thing. We've definitely hit it plenty in drm iterators (but we seem to like if() checks in iterator macros maybe a bit too much). I'm happy to drop this patch tough if you deem it offensive. -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
[Bug 102322] System crashes after "[drm] IP block:gmc_v8_0 is hung!" / [drm] IP block:sdma_v3_0 is hung!
https://bugs.freedesktop.org/show_bug.cgi?id=102322 --- Comment #29 from Andrey Grodzovsky --- (In reply to dwagner from comment #28) > (In reply to Michel Dänzer from comment #27) > > That could be a Mesa issue, anyway it should probably be tracked separately > > from this report. > > Created separate bug report > https://bugs.freedesktop.org/show_bug.cgi?id=107152 > > (If that is a Mesa issue, no more than user processes / X11 should have > crashed - but not the kernel amdgpu driver... right?) Not exactly, MESA could create a bad request (faulty GPU address) which would lead to this. It can even be triggered on purpose using a debug flag from MESA. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [amdgpu][tahiti xt] missing firmware
On Sun, Jul 8, 2018 at 10:31 AM, wrote: > Hi, > > The firmware paths were changed from 'radeon/' to 'amdgpu/' in > amd-staging-drm-next, but master of > git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git is missing > the files in amdgpu/ for tahiti gpus. I'm pushing out updated firmware this week. In the meantime, you can just copy or symlink the firmware from the radeon directory. Alex > > regards, > > -- > Sylvain > ___ > 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: [PATCH 06/17] dt-bindings: display: sun4i-drm: Add R40 TV TCON description
On Fri, Jul 6, 2018 at 2:46 PM Jernej Škrabec wrote: > > Dne petek, 06. julij 2018 ob 22:40:52 CEST je Rob Herring napisal(a): > > On Fri, Jul 06, 2018 at 07:51:02PM +0200, Jernej Skrabec wrote: > > > TCON description is expanded with R40 TV TCON compatible. It is a bit > > > special, because it is connected to TCON TOP instead directly to mixer > > > and it needs special handling. > > > > > > Signed-off-by: Jernej Skrabec > > > --- > > > > > > Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > Please add acks/reviews when posting new versions. > > This is not exactly the same version you give ack for, so I didn't include it. Then you need to state what changed (in this patch) and why you didn't add the ack. > I'm not sure where the line for that is. > > In contrast to previous version, it doesn't introduce additional clock. Sounds trivial enough to keep the ack. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 7/9] drm/bridge: tc358764: Add DSI to LVDS bridge driver
On 19.06.2018 10:19, Maciej Purski wrote: > From: Andrzej Hajda > > Add a drm_bridge driver for the Toshiba TC358764 DSI to LVDS bridge. > > Signed-off-by: Andrzej Hajda > Signed-off-by: Maciej Purski > --- > drivers/gpu/drm/bridge/Kconfig| 8 + > drivers/gpu/drm/bridge/Makefile | 1 + > drivers/gpu/drm/bridge/tc358764.c | 521 > ++ > 3 files changed, 530 insertions(+) > create mode 100644 drivers/gpu/drm/bridge/tc358764.c > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > index fa2c799..f3da8a7 100644 > --- a/drivers/gpu/drm/bridge/Kconfig > +++ b/drivers/gpu/drm/bridge/Kconfig > @@ -110,6 +110,14 @@ config DRM_THINE_THC63LVD1024 > ---help--- > Thine THC63LVD1024 LVDS/parallel converter driver. > > +config DRM_TOSHIBA_TC358764 > + tristate "TC358764 DSI/LVDS bridge" > + depends on DRM && DRM_PANEL > + depends on OF > + select DRM_MIPI_DSI > + help > + Toshiba TC358764 DSI/LVDS bridge driver. > + > config DRM_TOSHIBA_TC358767 > tristate "Toshiba TC358767 eDP bridge" > depends on OF > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile > index 35f88d4..bf7c0ce 100644 > --- a/drivers/gpu/drm/bridge/Makefile > +++ b/drivers/gpu/drm/bridge/Makefile > @@ -10,6 +10,7 @@ obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o > obj-$(CONFIG_DRM_SII902X) += sii902x.o > obj-$(CONFIG_DRM_SII9234) += sii9234.o > obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o > +obj-$(CONFIG_DRM_TOSHIBA_TC358764) += tc358764.o > obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o > obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/ > obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/ > diff --git a/drivers/gpu/drm/bridge/tc358764.c > b/drivers/gpu/drm/bridge/tc358764.c > new file mode 100644 > index 000..0aee155 > --- /dev/null > +++ b/drivers/gpu/drm/bridge/tc358764.c > @@ -0,0 +1,521 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2018 Samsung Electronics Co., Ltd > + * > + * Authors: > + * Andrzej Hajda > + * Maciej Purski > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. You should drop license blob if SPDX identifer provided, see for example drivers/gpu/drm/i915/intel_hdcp.c > + * > + */ > + > +#include > + > +#include > +#include > +#include > + > +#include > +#include > + > +#include > +#include > +#include > + > +#include You can order it alphabetically. > + > +#define FLD_MASK(start, end)(((1 << ((start) - (end) + 1)) - 1) << (end)) > +#define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end)) > + > +/* PPI layer registers */ > +#define PPI_STARTPPI 0x0104 /* START control bit */ > +#define PPI_LPTXTIMECNT 0x0114 /* LPTX timing signal */ > +#define PPI_LANEENABLE 0x0134 /* Enables each lane */ > +#define PPI_TX_RX_TA 0x013C /* BTA timing parameters */ > +#define PPI_D0S_CLRSIPOCOUNT 0x0164 /* Assertion timer for Lane 0 */ > +#define PPI_D1S_CLRSIPOCOUNT 0x0168 /* Assertion timer for Lane 1 */ > +#define PPI_D2S_CLRSIPOCOUNT 0x016C /* Assertion timer for Lane 2 */ > +#define PPI_D3S_CLRSIPOCOUNT 0x0170 /* Assertion timer for Lane 3 */ > +#define PPI_START_FUNCTION 1 > + > +/* DSI layer registers */ > +#define DSI_STARTDSI 0x0204 /* START control bit of DSI-TX */ > +#define DSI_LANEENABLE 0x0210 /* Enables each lane */ > +#define DSI_RX_START 1 > + > +/* Video path registers */ > +#define VP_CTRL 0x0450 /* Video Path Control */ > +#define VP_CTRL_MSF(v) FLD_VAL(v, 0, 0) /* Magic square in > RGB666 */ > +#define VP_CTRL_VTGEN(v) FLD_VAL(v, 4, 4) /* Use chip clock for timing */ > +#define VP_CTRL_EVTMODE(v) FLD_VAL(v, 5, 5) /* Event mode */ > +#define VP_CTRL_RGB888(v)FLD_VAL(v, 8, 8) /* RGB888 mode */ > +#define VP_CTRL_VSDELAY(v) FLD_VAL(v, 31, 20) /* VSYNC delay */ > +#define VP_CTRL_HSPOLBIT(17) /* Polarity of HSYNC signal */ > +#define VP_CTRL_DEPOLBIT(18) /* Polarity of DE signal */ > +#define VP_CTRL_VSPOLBIT(19) /* Polarity of VSYNC signal */ > +#define VP_HTIM1 0x0454 /* Horizontal Timing Control 1 */ > +#define VP_HTIM1_HBP(v) FLD_VAL(v, 24, 16) > +#define VP_HTIM1_HSYNC(v)FLD_VAL(v, 8, 0) > +#define VP_HTIM2 0x0458 /* Horizontal Timing Control 2 */ > +#define VP_HTIM2_HFP(v)