[Bug 96625] GPU lockup when using r600g VDPAU on powerpc
https://bugs.freedesktop.org/show_bug.cgi?id=96625 Bug ID: 96625 Summary: GPU lockup when using r600g VDPAU on powerpc Product: DRI Version: unspecified Hardware: PowerPC OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: DRM/Radeon Assignee: dri-devel at lists.freedesktop.org Reporter: j.ribeirovega at outlook.com Created attachment 124649 --> https://bugs.freedesktop.org/attachment.cgi?id=124649&action=edit dmesg I installed a Radeon HD6670 on a PowerMac G5 Quad, hoping to use VDPAU to play videos. When using mpv with opengl and vdpau hwdec and testing with a 720p sample from samplevideos the gpu locks up and I am forced to reboot. Attached dmesg after crash. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160621/d6fb0c18/attachment.html>
[PATCH 01/10] drm/amd-kfd: Clean up inline handling
On Tue, Jun 21, 2016 at 12:10 PM, Daniel Vetter wrote: > - inline functions need to be static inline, otherwise gcc can opt to > not inline and the linker gets unhappy. > - no forward decls for inline functions, just include the right headers. > > Cc: Oded Gabbay > Cc: Ben Goz > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h | 4 ++-- > drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 3 --- > 2 files changed, 2 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h > b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h > index ec4036a09f3e..a625b9137da2 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h > @@ -187,12 +187,12 @@ int init_pipelines(struct device_queue_manager *dqm, > unsigned int get_first_pipe(struct device_queue_manager *dqm); > unsigned int get_pipes_num(struct device_queue_manager *dqm); > > -extern inline unsigned int get_sh_mem_bases_32(struct kfd_process_device > *pdd) > +static inline unsigned int get_sh_mem_bases_32(struct kfd_process_device > *pdd) > { > return (pdd->lds_base >> 16) & 0xFF; > } > > -extern inline unsigned int > +static inline unsigned int > get_sh_mem_bases_nybble_64(struct kfd_process_device *pdd) > { > return (pdd->lds_base >> 60) & 0x0E; > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > index d0d5f4baf72d..80113c335966 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > @@ -617,10 +617,7 @@ int kgd2kfd_resume(struct kfd_dev *kfd); > int kfd_init_apertures(struct kfd_process *process); > > /* Queue Context Management */ > -inline uint32_t lower_32(uint64_t x); > -inline uint32_t upper_32(uint64_t x); > struct cik_sdma_rlc_registers *get_sdma_mqd(void *mqd); > -inline uint32_t get_sdma_base_addr(struct cik_sdma_rlc_registers *m); > > int init_queue(struct queue **q, struct queue_properties properties); > void uninit_queue(struct queue *q); > -- > 2.8.1 > Hi Daniel, Minor comment, please change the commit message title to "drm/amdkfd: ..." (without the "-" between amd and kfd), to make this patch consistent with all amdkfd patches. With that change, this patch is: Reviewed-by: Oded Gabbay
[PATCH 2/3] drm/vgem: Fix mmaping
On Sat, Jun 18, 2016 at 04:20:48PM +0100, Chris Wilson wrote: > The vGEM mmap code has bitrotted slightly and now immediately BUGs. > Since vGEM was last updated, there are new core GEM facilities to > provide more common functions, so let's use those here. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96603 > Testcase: igt/vgem_basic/mmap > Signed-off-by: Chris Wilson Tested-by: Humberto Israel Perez Rodriguez -Chris -- Chris Wilson, Intel Open Source Technology Centre
[PATCH v4 0/8] iommu/rockchip: Fix bugs and enable on ARM64
Hi Joerg, On Tue, Jun 21, 2016 at 6:17 PM, Joerg Roedel wrote: > > On Tue, Jun 21, 2016 at 01:34:33PM +0900, Tomasz Figa wrote: > > This series intends mostly to enable support for ARM64 architecture > > in the rockchip-iommu driver. On the way to do so, some bugs are also > > fixed. > > > > The most important changes here are: > > - making the Rockchip IOMMU driver use DMA API for managing cache > >coherency of page tables, > > - making the Rockchip DRM driver not use DMA API on behalf of a virtual > >device (behind a virtual IOMMU) to allocate and map buffers, but > >instead proper DRM helpers and IOMMU API directly. > > Are these two parts dependent on each other or can the IOMMU and the DRM > part merged independently? In simple words, DRM patches depend on IOMMU patches. More precisely: The IOMMU patches alone are supposed to not break anything. Same goes for the first DRM patch (7/8). Only second DRM patch (8/8) depends on changes introduced by its predecessors. Best regards, Tomasz
[PATCH v3 3/3] drm: sti: rework init sequence
On Tue, Jun 21, 2016 at 03:09:40PM +0200, Benjamin Gaignard wrote: > Use drm_dev_alloc() and drm_dev_register() instead of .load() > To simplify init sequence only create fbdev when requested > in output_poll_changed(). > > version 2: > remove call to drm_connector_unregister_all() and > drm_dev_set_unique() > > Signed-off-by: Benjamin Gaignard > --- > drivers/gpu/drm/sti/sti_drv.c | 137 > + > drivers/gpu/drm/sti/sti_drv.h | 1 + > drivers/gpu/drm/sti/sti_dvo.c | 7 --- > drivers/gpu/drm/sti/sti_hda.c | 8 +-- > drivers/gpu/drm/sti/sti_hdmi.c | 21 +-- > 5 files changed, 100 insertions(+), 74 deletions(-) > > diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c > index 26aa85d..96bd3d0 100644 > --- a/drivers/gpu/drm/sti/sti_drv.c > +++ b/drivers/gpu/drm/sti/sti_drv.c > @@ -226,8 +226,28 @@ static int sti_atomic_commit(struct drm_device *drm, > return 0; > } > > +static void sti_output_poll_changed(struct drm_device *ddev) > +{ > + struct sti_private *private = ddev->dev_private; > + > + if (!ddev->mode_config.num_connector) > + return; > + > + if (private->fbdev) { > + drm_fbdev_cma_hotplug_event(private->fbdev); > + return; > + } > + > + private->fbdev = drm_fbdev_cma_init(ddev, 32, > + ddev->mode_config.num_crtc, > + ddev->mode_config.num_connector); > + if (IS_ERR(private->fbdev)) > + private->fbdev = NULL; > +} This creates another copy of the copy-pasted deferred fbdev init code. Thierry Redding is working to clean that up, but since you've just created more work for him here I think you've volunteered to review Thierry's patches ;-) All three patches merged to drm-misc, thanks. -Daniel > + > static const struct drm_mode_config_funcs sti_mode_config_funcs = { > .fb_create = drm_fb_cma_create, > + .output_poll_changed = sti_output_poll_changed, > .atomic_check = drm_atomic_helper_check, > .atomic_commit = sti_atomic_commit, > }; > @@ -248,44 +268,6 @@ static void sti_mode_config_init(struct drm_device *dev) > dev->mode_config.funcs = &sti_mode_config_funcs; > } > > -static int sti_load(struct drm_device *dev, unsigned long flags) > -{ > - struct sti_private *private; > - int ret; > - > - private = kzalloc(sizeof(*private), GFP_KERNEL); > - if (!private) { > - DRM_ERROR("Failed to allocate private\n"); > - return -ENOMEM; > - } > - dev->dev_private = (void *)private; > - private->drm_dev = dev; > - > - mutex_init(&private->commit.lock); > - INIT_WORK(&private->commit.work, sti_atomic_work); > - > - drm_mode_config_init(dev); > - drm_kms_helper_poll_init(dev); > - > - sti_mode_config_init(dev); > - > - ret = component_bind_all(dev->dev, dev); > - if (ret) { > - drm_kms_helper_poll_fini(dev); > - drm_mode_config_cleanup(dev); > - kfree(private); > - return ret; > - } > - > - drm_mode_config_reset(dev); > - > - drm_fbdev_cma_init(dev, 32, > -dev->mode_config.num_crtc, > -dev->mode_config.num_connector); > - > - return 0; > -} > - > static const struct file_operations sti_driver_fops = { > .owner = THIS_MODULE, > .open = drm_open, > @@ -302,7 +284,6 @@ static const struct file_operations sti_driver_fops = { > static struct drm_driver sti_driver = { > .driver_features = DRIVER_HAVE_IRQ | DRIVER_MODESET | > DRIVER_GEM | DRIVER_PRIME | DRIVER_ATOMIC, > - .load = sti_load, > .gem_free_object_unlocked = drm_gem_cma_free_object, > .gem_vm_ops = &drm_gem_cma_vm_ops, > .dumb_create = drm_gem_cma_dumb_create, > @@ -339,14 +320,88 @@ static int compare_of(struct device *dev, void *data) > return dev->of_node == data; > } > > +static int sti_init(struct drm_device *ddev) > +{ > + struct sti_private *private; > + > + private = kzalloc(sizeof(*private), GFP_KERNEL); > + if (!private) > + return -ENOMEM; > + > + ddev->dev_private = (void *)private; > + dev_set_drvdata(ddev->dev, ddev); > + private->drm_dev = ddev; > + > + mutex_init(&private->commit.lock); > + INIT_WORK(&private->commit.work, sti_atomic_work); > + > + drm_mode_config_init(ddev); > + > + sti_mode_config_init(ddev); > + > + drm_kms_helper_poll_init(ddev); > + > + return 0; > +} > + > +static void sti_cleanup(struct drm_device *ddev) > +{ > + struct sti_private *private = ddev->dev_private; > + > + if (private->fbdev) { > + drm_fbdev_cma_fini(private->fbdev); > + private->fbdev = NULL; > + } > + > + drm_kms_helper_poll_fini(ddev); > + drm_vblank_cleanup(ddev); > + kfree(private); > + ddev->dev_private = NULL; > +} > + > st
[PATCH v4 4/4] drm/i915: Enable polling when we don't have hpd
On Tue, Jun 21, 2016 at 02:18:35PM -0400, Lyude wrote: > Unfortunately, there's two situations where we lose hpd right now: > - Runtime suspend > - When we've shut off all of the power wells on Valleyview/Cherryview > > While it would be nice if this didn't cause issues, this has the > ability to get us in some awkward states where a user won't be able to > get their display to turn on. For instance; if we boot a Valleyview > system without any monitors connected, it won't need any of it's power > wells and thus shut them off. Since this causes us to lose HPD, this > means that unless the user knows how to ssh into their machine and do a > manual reprobe for monitors, none of the monitors they connect after > booting will actually work. > > Eventually we should come up with a better fix then having to enable > polling for this, since this makes rpm a lot less useful, but for now > the infrastructure in i915 just isn't there yet to get hpd in these > situations. > > Changes since v1: > - Add comment explaining the addition of the if >(!mode_config->poll_running) in intel_hpd_init() > - Remove unneeded if (!dev->mode_config.poll_enabled) in >i915_hpd_poll_init_work() > - Call to drm_helper_hpd_irq_event() after we disable polling > - Add cancel_work_sync() call to intel_hpd_cancel_work() > > Changes since v2: > - Apparently dev->mode_config.poll_running doesn't actually reflect >whether or not a poll is currently in progress, and is actually used >for dynamic module paramter enabling/disabling. So now we instead >keep track of our own poll_running variable in dev_priv->hotplug > - Clean i915_hpd_poll_init_work() a little bit > > Changes since v3: > - Remove the now-redundant connector loop in intel_hpd_init(), just >rely on intel_hpd_poll_enable() for setting connector->polled >correctly on each connector > - Get rid of poll_running > - Don't assign enabled in i915_hpd_poll_init_work before we actually >lock dev->mode_config.mutex > - Wrap enabled assignment in i915_hpd_poll_init_work() in READ_ONCE() >for doc purposes > - Do the same for dev_priv->hotplug.poll_enabled with WRITE_ONCE in >intel_hpd_poll_enable() > - Add some comments about racing not mattering in intel_hpd_poll_enable > > Cc: stable at vger.kernel.org > Cc: Ville Syrjälä > Cc: Daniel Vetter > Signed-off-by: Lyude > --- > drivers/gpu/drm/i915/i915_drv.c | 7 ++- > drivers/gpu/drm/i915/i915_drv.h | 3 ++ > drivers/gpu/drm/i915/intel_drv.h| 2 + > drivers/gpu/drm/i915/intel_hotplug.c| 76 > ++--- > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 ++ > 5 files changed, 75 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 3eb47fb..5381d9e 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1612,6 +1612,9 @@ static int intel_runtime_suspend(struct device *device) > > assert_forcewakes_inactive(dev_priv); > > + if (!IS_VALLEYVIEW(dev_priv) || !IS_CHERRYVIEW(dev_priv)) > + intel_hpd_poll_enable(dev_priv, true); > + > DRM_DEBUG_KMS("Device suspended\n"); > return 0; > } > @@ -1667,8 +1670,10 @@ static int intel_runtime_resume(struct device *device) >* power well, so hpd is reinitialized from there. For >* everyone else do it here. >*/ > - if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv)) > + if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv)) { > intel_hpd_init(dev_priv); > + intel_hpd_poll_enable(dev_priv, false); > + } > > intel_enable_gt_powersave(dev_priv); > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 239a3ec..944c0d0 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -283,6 +283,9 @@ struct i915_hotplug { > u32 short_port_mask; > struct work_struct dig_port_work; > > + struct work_struct poll_enable_work; > + bool poll_enabled; > + > /* >* if we get a HPD irq from DP and a HPD irq from non-DP >* the non-DP HPD could block the workqueue on a mode config > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index dcbfdde..cedac13 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1385,6 +1385,8 @@ int intel_dsi_dcs_init_backlight_funcs(struct > intel_connector *intel_connector); > > /* intel_dvo.c */ > void intel_dvo_init(struct drm_device *dev); > +/* intel_hotplug.c */ > +void intel_hpd_poll_enable(struct drm_i915_private *dev_priv, bool enabled); > > > /* legacy fbdev emulation in intel_fbdev.c */ > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c > b/drivers/gpu/drm/i915/intel_hotplug.c > index ec3285f..f23330d 100644 > --- a/drivers/gpu/drm/i915/intel_hotplug.c > +++ b/dri
[PATCH 01/10] drm/amd-kfd: Clean up inline handling
On Tue, Jun 21, 2016 at 10:11:13PM +0300, Oded Gabbay wrote: > On Tue, Jun 21, 2016 at 12:10 PM, Daniel Vetter > wrote: > > - inline functions need to be static inline, otherwise gcc can opt to > > not inline and the linker gets unhappy. > > - no forward decls for inline functions, just include the right headers. > > > > Cc: Oded Gabbay > > Cc: Ben Goz > > Signed-off-by: Daniel Vetter > > --- > > drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h | 4 ++-- > > drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 3 --- > > 2 files changed, 2 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h > > b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h > > index ec4036a09f3e..a625b9137da2 100644 > > --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h > > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h > > @@ -187,12 +187,12 @@ int init_pipelines(struct device_queue_manager *dqm, > > unsigned int get_first_pipe(struct device_queue_manager *dqm); > > unsigned int get_pipes_num(struct device_queue_manager *dqm); > > > > -extern inline unsigned int get_sh_mem_bases_32(struct kfd_process_device > > *pdd) > > +static inline unsigned int get_sh_mem_bases_32(struct kfd_process_device > > *pdd) > > { > > return (pdd->lds_base >> 16) & 0xFF; > > } > > > > -extern inline unsigned int > > +static inline unsigned int > > get_sh_mem_bases_nybble_64(struct kfd_process_device *pdd) > > { > > return (pdd->lds_base >> 60) & 0x0E; > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > > b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > > index d0d5f4baf72d..80113c335966 100644 > > --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > > @@ -617,10 +617,7 @@ int kgd2kfd_resume(struct kfd_dev *kfd); > > int kfd_init_apertures(struct kfd_process *process); > > > > /* Queue Context Management */ > > -inline uint32_t lower_32(uint64_t x); > > -inline uint32_t upper_32(uint64_t x); > > struct cik_sdma_rlc_registers *get_sdma_mqd(void *mqd); > > -inline uint32_t get_sdma_base_addr(struct cik_sdma_rlc_registers *m); > > > > int init_queue(struct queue **q, struct queue_properties properties); > > void uninit_queue(struct queue *q); > > -- > > 2.8.1 > > > > Hi Daniel, > Minor comment, please change the commit message title to "drm/amdkfd: > ..." (without the "-" between amd and kfd), to make this patch > consistent with all amdkfd patches. > > With that change, this patch is: > Reviewed-by: Oded Gabbay Fixed up an applied to drm-misc, thanks for the review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[RFC PATCH 1/2] drm: bridge: anx7688: Add anx7688 bridge driver support.
Hi, On 6/20/2016 12:44 PM, Nicolas Boichat wrote: > ANX7688 is a HDMI to DP converter (as well as USB-C port controller), > that has an internal microcontroller. > > The only reason a Linux kernel driver is necessary is to reject > resolutions that require more bandwidth than what is available on > the DP side. DP bandwidth and lane count are reported by the bridge > via 2 registers on I2C. How does the chip know when to enable/disable itself? Does it shutoff itself if there isn't anything on the HDMI link? > > Signed-off-by: Nicolas Boichat > --- > > Hi, > > I tested this driver using the Mediatek HDMI controller (MT8173) upstream > of the ANX bridge chip (Phillip sent a PULL request on June 13: > git://git.pengutronix.de/git/pza/linux.git tags/mediatek-drm-2016-06-13 > ). > > I have 2 concerns, that I'm not sure how to address within the kernel DRM > framework: > 1. All other bridge drivers also have a connector attached to it. However, > in > this case, we cannot read the monitor EDID directly, so I'm not sure what > I could put in a "get_modes" function. Instead, ANX7688 provides a I2C > passthru/repeater, so the EDID is read on the Mediatek HDMI controller > side. > > That leads to a somewhat strange layout, where we have: > - MTK HDMI bridge/encoder >- MTK connector (HDMI) >- ANX7688 bridge > - No connector You should ideally have one DP connector at the end of the chain: - MTK HDMI bridge/encoder - ANX7688 bridge - Connector (DP) In the dt-bindings for this board, hdmi's output port shouldn't be connected to a hdmi connector, but the input port of the ANX7688 DP bridge. The output port of the bridge should be the one that connects to the DP connector. One way I can think of fixing this is to make make the MTK hdmi encoder driver a bit smarter by observing the DT connections. If it's output port is connected to just a hdmi-connector, then things should be as before. If the output is connected to the DP bridge, then it should create a DP connector. The connector ops for the DP connector can still be the same as that of the HDMI connector before, but the phandle to the DDC bus would be in the DP device node in this case. This way, you can get around having the correct layout. Ideally, a bridge driver shouldn't be the one that creates a connector. It may contain some of the connector functionality, but the connector creation should be managed by the kms driver. Almost all bridge drivers creating a connector in their .attach callbacks since they own some of the connector functionality (like reading EDID). That's something we're trying to fix by providing some more bridge api that kms drivers can use. Since this bridge driver doesn't have any connector functionality anyway, you should be okay. > > Resolution filtering works fine though, thanks to mode_fixup callback on > the > bridge. It also helps that Mediatek HDMI bridge calls mode_fixup from its > connector mode_valid callback, so that invalid modes are not even > presented > to userspace. > > 2. In the bandwidth computation, I hard-code 8-bit per channel (bpc). bpc > does > not seem to be included in the mode setting itself. We could possibly > iterate > over connectors on the DRM device, but then, IIUC, > connector->display_info.bpc > indicates the _maximum_ bpc supported by the monitor. I'm not clear about this either. Some drivers set a bus format on the connector via drm_display_info_set_bus_formats in their get_modes connector op, and then retrieve it later. > > Any pointers? Thanks! > > Best, > > Nicolas > > drivers/gpu/drm/bridge/Kconfig| 9 ++ > drivers/gpu/drm/bridge/Makefile | 1 + > drivers/gpu/drm/bridge/analogix-anx7688.c | 227 > ++ > 3 files changed, 237 insertions(+) > create mode 100644 drivers/gpu/drm/bridge/analogix-anx7688.c > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > index 8f7423f..0c1eb41 100644 > --- a/drivers/gpu/drm/bridge/Kconfig > +++ b/drivers/gpu/drm/bridge/Kconfig > @@ -7,6 +7,15 @@ config DRM_BRIDGE > menu "Display Interface Bridges" > depends on DRM && DRM_BRIDGE > > +config DRM_ANALOGIX_ANX7688 > + tristate "Analogix ANX7688 bridge" > + depends on DRM > + select DRM_KMS_HELPER > + ---help--- > + ANX7688 is a transmitter to support DisplayPort over USB-C for > + smartphone and tablets. > + This driver only supports the HDMI to DP component of the chip. > + > config DRM_ANALOGIX_ANX78XX > tristate "Analogix ANX78XX bridge" > select DRM_KMS_HELPER > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile > index 96b13b3..d744c6c 100644 > --- a/drivers/gpu/drm/bridge/Makefile > +++ b/drivers/gpu/drm/bridge/Makefile > @@ -1,5 +1,6 @@ > ccflags-y := -Iinclude/drm > > +obj-$(CONFIG_DRM_ANALOGIX_ANX768
[PATCH v3 0/10]
On 6/14/2016 5:15 PM, Yakir Yang wrote: > RK3399 and RK3288 shared the same eDP IP controller, only some light > difference with VOP configure and GRF configure. > > Also same misc fix to analogix_dp driver: > - Hotplug invalid which report by Dan Carpenter > - Make panel detect to an optional action > - correct the register bit define error in ANALOGIX_DP_PLL_REG_1 > > > Changes in v3: > - Correct the misspell of "marcos" in commit message (Dominik, reviewed at > Google Gerrit) > [https://chromium-review.googlesource.com/#/c/346312/9//COMMIT_MSG at 9] > - Add reviewed flag from Stéphane. > [https://chromium-review.googlesource.com/#/c/346312/16] > - Add tested flag from Javier. > - Write a kerneldoc-style comment explaining the chips data fields (Tomasz, > reviewed at Google Gerrit) > > [https://chromium-review.googlesource.com/#/c/346313/10/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c > at 39] > - Drop the '.lcdcsel_mask' number in chips data field (Tomasz, reviewed at > Google Gerrit) > > [https://chromium-review.googlesource.com/#/c/346313/10/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c > at 382] > - Add acked flag from Mark. > - Add reviewed flag from Tomasz. > [https://chromium-review.googlesource.com/#/c/346315/15] > - Add tested flag from Javier > - Make this hack code more clear (Tomasz, reviewed at Google Gerrit) >reg = ~reg & REF_CLK_MASK; ---> reg ^= REF_CLK_MASK; > > [https://chromium-review.googlesource.com/#/c/346852/7/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c > at 80] > - Add tested flag from Javier > - Give the "rk3399-edp" a separate line for clarity in document (Tomasz, > reviewed at Google Gerrit) > > [https://chromium-review.googlesource.com/#/c/346314/10/Documentation/devicetree/bindings/display/rockchip/analogix_dp-rockchip.txt > at 5] > - Move 'output_type' setting before the return statement (Tomasz, reviewed at > Google Gerrit) > > [https://chromium-review.googlesource.com/#/c/346314/10/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c > at 154] > - Add the acked flag from Mark. > - Add the acked flag from Mark. > - Avoid to change any internal driver state in .mode_valid interface. > (Tomasz, reviewed at Google Gerrit) > > [https://chromium-review.googlesource.com/#/c/346318/10/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c > at 113] > - Hook the connector's color_formats in .get_modes directly. (Tomasz, > reviewed at Google Gerrit) > [https://chromium-review.googlesource.com/#/c/346317/15] > - Add the acked flag from Mark. > - Add the reviewed flag from Tomasz. > [https://chromium-review.googlesource.com/#/c/346853/12] > - Add the acked flag from Mark. > - Add reviewed flag from Stéphane. > [https://chromium-review.googlesource.com/#/c/346319/15] > - Add tested flag from Javier > > Changes in v2: > - new patch in v2 > - rebase with drm-next, fix some conflicts > - new patch in v2 > > Yakir Yang (10): >drm/bridge: analogix_dp: rename RK3288_DP to ROCKCHIP_DP >drm/rockchip: analogix_dp: split the lcdc select setting into device > data >drm/bridge: analogix_dp: correct the register bit define error in > ANALOGIX_DP_PLL_REG_1 >drm/bridge: analogix_dp: some rockchip chips need to flip REF_CLK bit > setting >drm/rockchip: analogix_dp: add rk3399 eDP support >drm/rockchip: analogix_dp: make panel detect to an optional action >drm/bridge: analogix_dp: passing the connector as an argument in > .get_modes() >drm/rockchip: analogix_dp: correct the connector display color format > and bpc >drm/rockchip: analogix_dp: update the comments about why need to > hardcode VOP output mode >drm/bridge: analogix_dp: fix no drm hpd event when panel plug in Is the plan to take all the bridge+rockchip stuff via the rockchip pull request? Thanks, Archit > > .../bindings/display/bridge/analogix_dp.txt| 1 + > .../display/rockchip/analogix_dp-rockchip.txt | 3 +- > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 6 +- > drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 8 +- > drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 12 +- > drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h | 5 +- > drivers/gpu/drm/exynos/exynos_dp.c | 4 +- > drivers/gpu/drm/rockchip/analogix_dp-rockchip.c| 158 > ++--- > include/drm/bridge/analogix_dp.h | 9 +- > 9 files changed, 141 insertions(+), 65 deletions(-) > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
[Bug 96622] [radeonsi] "Dreamfall Chapters: The longest Journey" shows visual corruption
https://bugs.freedesktop.org/show_bug.cgi?id=96622 Bug ID: 96622 Summary: [radeonsi] "Dreamfall Chapters: The longest Journey" shows visual corruption Product: Mesa Version: git Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: Drivers/Gallium/radeonsi Assignee: dri-devel at lists.freedesktop.org Reporter: kai at dev.carbon-project.org QA Contact: dri-devel at lists.freedesktop.org Blocks: 77449 Created attachment 124647 --> https://bugs.freedesktop.org/attachment.cgi?id=124647&action=edit Screenshot showing visual corruption with game version 5.1.1 on radeonsi Since the recent release of book five for "Dreamfall Chapters: The longest Journey" and the games usage of a OpenGL 4.1+ context I'm seeing minor visual corruption in almost all scenes: usually it's just a few pixels â mostly along borders/edges of models â with wrong colours, like a bright green or yellow. During the epilogue there is a scene with more heavy corruption in the background. The attached screenshot shows that scene. The game version is the currently latest 5.1.1 and was run on the following stack (a fully updated Debian testing as a base): GPU: Hawaii PRO [Radeon R9 290] (ChipID = 0x67b1) Mesa: Git:master/5a64549f54 libdrm: 2.4.68-1 LLVM: SVN:trunk/r273281 (3.9 devel) + <http://reviews.llvm.org/D21551?id=61349&download=true> X.Org: 2:1.18.3-1 Linux: 4.6.2 Firmware: firmware-amd-graphics/20160110-1 libclc: Git:master/20d977a3e6 DDX: 1:7.7.0-1 Please let me know, if you need anything else. Referenced Bugs: https://bugs.freedesktop.org/show_bug.cgi?id=77449 [Bug 77449] Tracker bug for all bugs related to Steam titles -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160621/82a96ca0/attachment-0001.html>
[Bug 96602] [radeonsi] Dreamfall Chapters: one shader fails to compile, minor visual corruption
https://bugs.freedesktop.org/show_bug.cgi?id=96602 Kai changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #3 from Kai --- (In reply to Nicolai Hähnle from comment #2) > With the patch at http://reviews.llvm.org/D21551 for LLVM, the shader in the > provided log file compiles without error. Let us know if there are any other > problems with the game. Thanks for the fast reply and patch! I can confirm, that with the stack detailed below I'm no longer seeing the shader compilation failure. The minor visual corruption is still there, but I'll open a separate bug for that, since it seems unconnected. You can have my: Tested-by: Kai Wasserbäch for <http://reviews.llvm.org/D21551?id=61349&download=true> The stack used was (Debian testing, fully updated, as a base): GPU: Hawaii PRO [Radeon R9 290] (ChipID = 0x67b1) Mesa: Git:master/5a64549f54 libdrm: 2.4.68-1 LLVM: SVN:trunk/r273281 (3.9 devel) + <http://reviews.llvm.org/D21551?id=61349&download=true> X.Org: 2:1.18.3-1 Linux: 4.6.2 Firmware: firmware-amd-graphics/20160110-1 libclc: Git:master/20d977a3e6 DDX: 1:7.7.0-1 -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160621/c33187bf/attachment.html>
[Intel-gfx] Bad flicker on skylake HQD due to code in the 4.7 merge window
On Tue, Jun 21, 2016 at 09:53:15AM -0400, James Bottomley wrote: > On Mon, 2016-06-20 at 11:03 +0300, Jani Nikula wrote: > > Cc: Ville > > > > On Mon, 20 Jun 2016, James Bottomley < > > James.Bottomley at HansenPartnership.com> wrote: > > > OK, my candidate bad commit is this one: > > > > > > commit a05628195a0d9f3173dd9aa76f482aef692e46ee > > > Author: Ville Syrjälä > > > Date: Mon Apr 11 10:23:51 2016 +0300 > > > > > > drm/i915: Get panel_type from OpRegion panel details > > > > > > After being more careful about waiting to identify flicker, this > > > one seems to be the one the bisect finds. I'm now running v4.7-rc3 > > > with this one reverted and am currently seeing no flicker problems. > > > It is, however, early days because the flicker can hide for long > > > periods, so I 'll wait until Monday evening and a few reboots > > > before declaring victory. > > > > If that turns out to be the bad commit, it doesn't really surprise > > me, and that in itself is depressing. > > As far as I can tell, after running for a day with this reverted, this > is the problem. The flicker hasn't appeared with it reverted. It's > pretty noticeable with this commit included. Hmm. The only difference I can see is low vs. normal vswing. Panel 0 has low, panel 2 has normal. So either the VBT or opregion is telling utter lies, or there's some other bug in our low vswing support. To confirm it's really a vswing issue, you should be able to run with i915.edp_vswing=2 without flickers on the broken kernel. -- Ville Syrjälä Intel OTC
[Bug 94231] Problems compiling libdrm since glibc 2.23
https://bugs.freedesktop.org/show_bug.cgi?id=94231 --- Comment #23 from Mike Frysinger --- Created attachment 124643 --> https://bugs.freedesktop.org/attachment.cgi?id=124643&action=edit using AC_CHECK_HEADERS -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160621/a2f9984c/attachment.html>
[Bug 94231] Problems compiling libdrm since glibc 2.23
https://bugs.freedesktop.org/show_bug.cgi?id=94231 --- Comment #22 from Mike Frysinger --- Created attachment 124642 --> https://bugs.freedesktop.org/attachment.cgi?id=124642&action=edit using AC_CHECK_HEADERS() -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160621/bccb20cb/attachment-0001.html>
[PATCH 1/2] drm/i915/dp: Revert "drm/i915/dp: fall back to 18 bpp when sink capability is unknown"
Any news on this one? Thanks a bunch, -mario On 06/14/2016 04:12 PM, Mario Kleiner wrote: > On 06/14/2016 01:05 PM, Daniel Vetter wrote: >> On Thu, May 26, 2016 at 4:39 PM, Mario Kleiner >> wrote: >>> This reverts commit 013dd9e03872 >>> ("drm/i915/dp: fall back to 18 bpp when sink capability is unknown") >>> >>> This commit introduced a regression into stable kernels, >>> as it reduces output color depth to 6 bpc for any video >>> sink connected to a Displayport connector if that sink >>> doesn't report a specific color depth via EDID, or if >>> our EDID parser doesn't actually recognize the proper >>> bpc from EDID. >>> >>> Affected are active DisplayPort->VGA converters and >>> active DisplayPort->DVI converters. Both should be >>> able to handle 8 bpc, but are degraded to 6 bpc with >>> this patch. >>> >>> The reverted commit was meant to fix >>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=105331 >>> >>> A followup patch implements a fix for that specific bug, >>> which is caused by a faulty EDID of the affected DP panel >>> by adding a new EDID quirk for that panel. >>> >>> DP 18 bpp fallback handling and other improvements to >>> DP sink bpc detection will be handled for future >>> kernels in a separate series of patches. >>> >>> Please backport to stable. >>> >>> Signed-off-by: Mario Kleiner >>> Acked-by: Jani Nikula >>> Cc: stable at vger.kernel.org >>> Cc: Ville Syrjälä >>> Cc: Daniel Vetter >> >> I wonder whether we shouldn't just move this into the DP code, and >> instead of looking at the edid (which is just pass-through for dp->vga >> dongles) we should only look at dpcd values? Or maybe only look at the >> edid value if the sink is native DP, and not when it's a dongle. >> >> That would probably also avoid the quirk, and that quirk seems a bit >> fishy. >> -Daniel >> > > This patch is just a simple fix for the color depth regression which > affects stable kernels. It can be back-ported easily to affected stable > kernels, as Jani advised me. > > I wanted to clean up and resubmit that DP helper function which looks at > dpcd values and might be a bit too much for stable, once this fix is in. > > -mario
[PATCH v1 2/3] drm: Add API for capturing frame CRCs
t; +} Do we really want these for all minors? Is ->primary not enough? It certainly seems completely misplaced in ->render, and I don't think anything really uses ->control anymore. Also I think you need to export this symbol. > +void drm_debugfs_crtc_remove(struct drm_crtc *crtc) > +{ > + int i; unsigned int > + > + for (i = 0; i < DRM_MINOR_CNT; i++) { > + debugfs_remove_recursive(crtc->crc.debugfs_entries[i]); > + crtc->crc.debugfs_entries[i] = NULL; > + } > +} This also needs an export, I think. > + > +void drm_crtc_add_crc_entry(struct drm_crtc *crtc, uint32_t frame, > + uint32_t crc0, uint32_t crc1, uint32_t crc2, > + uint32_t crc3, uint32_t crc4) Perhaps allow passing the CRC as an array with a count parameter? I can imagine that a lot of hardware will only give you a single uint32_t for the CRC, in which case you could do: drm_crtc_add_crc_entry(crtc, frame, &crc, 1); instead of: drm_crtc_add_crc_entry(crtc, frame, crc, 0, 0, 0, 0); It would probably save poor users of the interface, such as myself, a lot of headaches because they can't remember how many uint32_t:s the function needs. > +{ > + struct drm_crtc_crc *crc = &crtc->crc; > + struct drm_crtc_crc_entry *entry; > + int head, tail; unsigned int > + > + spin_lock(&crc->lock); > + > + head = crc->head; > + tail = crc->tail; > + > + if (CIRC_SPACE(head, tail, DRM_CRTC_CRC_ENTRIES_NR) < 1) { Perhaps "== 0"? > + spin_unlock(&crc->lock); > + DRM_ERROR("Overflow of CRC buffer, userspace reads too > slow.\n"); > + return; > + } Maybe return an error here? And perhaps use some sort of printk rate limiting here to avoid this from spamming logs? > + > + entry = &crc->entries[head]; > + > + entry->frame = frame; > + entry->crc[0] = crc0; > + entry->crc[1] = crc1; > + entry->crc[2] = crc2; > + entry->crc[3] = crc3; > + entry->crc[4] = crc4; > + > + head = (head + 1) & (DRM_CRTC_CRC_ENTRIES_NR - 1); > + crc->head = head; > + > + spin_unlock(&crc->lock); > + > + wake_up_interruptible(&crc->wq); > +} > + > +#endif /* CONFIG_DEBUG_FS */ > diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h > index 38401d406532..e5b124d937f5 100644 > --- a/drivers/gpu/drm/drm_internal.h > +++ b/drivers/gpu/drm/drm_internal.h > @@ -100,6 +100,8 @@ int drm_debugfs_init(struct drm_minor *minor, int > minor_id, > int drm_debugfs_cleanup(struct drm_minor *minor); > int drm_debugfs_connector_add(struct drm_connector *connector); > void drm_debugfs_connector_remove(struct drm_connector *connector); > +int drm_debugfs_crtc_add(struct drm_crtc *crtc); > +void drm_debugfs_crtc_remove(struct drm_crtc *crtc); Oh... this isn't something that drivers are supposed to call? > #else > static inline int drm_debugfs_init(struct drm_minor *minor, int minor_id, > struct dentry *root) > @@ -119,4 +121,12 @@ static inline int drm_debugfs_connector_add(struct > drm_connector *connector) > static inline void drm_debugfs_connector_remove(struct drm_connector > *connector) > { > } > + > +static inline int drm_debugfs_crtc_add(struct drm_crtc *crtc) > +{ > + return 0; > +} > +static inline void drm_debugfs_crtc_remove(struct drm_crtc *crtc) > +{ > +} > #endif > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > index 084fd141e8bf..ec2f91c8b7cd 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -1142,6 +1142,11 @@ static __inline__ bool drm_can_sleep(void) > return true; > } > > +#if defined(CONFIG_DEBUG_FS) > +extern const struct file_operations drm_crc_control_fops; > +extern const struct file_operations drm_crtc_crc_fops; > +#endif > + > /* helper for handling conditionals in various for_each macros */ > #define for_each_if(condition) if (!(condition)) {} else > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index c2734979f164..141335a3c647 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -376,6 +376,22 @@ struct drm_crtc_state { > struct drm_atomic_state *state; > }; > > +struct drm_crtc_crc_entry { > + uint32_t frame; > + uint32_t crc[5]; > +}; > + > +#define DRM_CRTC_CRC_ENTRIES_NR 128 > +struct drm_crtc_crc { > + spinlock_t lock; > + const char *source; > + bool opened;/* exclusive access to the result file */ You could probably have done this with an atomic and avoid the spin lock for exclusive access, but it's probably not worth it. > + struct drm_crtc_crc_entry *entries; > + int head, tail; unsigned int? > + wait_queue_head_t wq; > + struct dentry **debugfs_entries; > +}; > + > /** > * struct drm_crtc_funcs - control CRTCs for a given device > * > @@ -704,6 +720,29 @@ struct drm_crtc_funcs { > const struct drm_crtc_state *state, > struct drm_property *property, > uint64_t *val); > + > + /** > + * @set_crc_source: > + * > + * Changes the source of CRC checksums of frames at the request of > + * userspace, typically for testing purposes. The sources available are > + * specific of each driver and a %NULL value indicates that CRC > + * generation is to be switched off. Perhaps also mention that "none" is an alias for NULL? > + * > + * When CRC generation is enabled, the driver should call > + * drm_crtc_add_crc_entry() at each frame, providing any information > + * that characterizes the frame contents in the crcN arguments, as > + * provided from the configured source. Drivers should accept a "auto" > + * source name that will select a default source for this CRTC. Would it be useful to provide some more aliases? "enable" and "on" for "auto", "disable" and "off" for "none"? Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160621/b8bdf25e/attachment-0001.sig>
[PATCH v6 4/4] drm/i915: Enable polling when we don't have hpd
Unfortunately, there's two situations where we lose hpd right now: - Runtime suspend - When we've shut off all of the power wells on Valleyview/Cherryview While it would be nice if this didn't cause issues, this has the ability to get us in some awkward states where a user won't be able to get their display to turn on. For instance; if we boot a Valleyview system without any monitors connected, it won't need any of it's power wells and thus shut them off. Since this causes us to lose HPD, this means that unless the user knows how to ssh into their machine and do a manual reprobe for monitors, none of the monitors they connect after booting will actually work. Eventually we should come up with a better fix then having to enable polling for this, since this makes rpm a lot less useful, but for now the infrastructure in i915 just isn't there yet to get hpd in these situations. Changes since v1: - Add comment explaining the addition of the if (!mode_config->poll_running) in intel_hpd_init() - Remove unneeded if (!dev->mode_config.poll_enabled) in i915_hpd_poll_init_work() - Call to drm_helper_hpd_irq_event() after we disable polling - Add cancel_work_sync() call to intel_hpd_cancel_work() Changes since v2: - Apparently dev->mode_config.poll_running doesn't actually reflect whether or not a poll is currently in progress, and is actually used for dynamic module paramter enabling/disabling. So now we instead keep track of our own poll_running variable in dev_priv->hotplug - Clean i915_hpd_poll_init_work() a little bit Changes since v3: - Remove the now-redundant connector loop in intel_hpd_init(), just rely on intel_hpd_poll_enable() for setting connector->polled correctly on each connector - Get rid of poll_running - Don't assign enabled in i915_hpd_poll_init_work before we actually lock dev->mode_config.mutex - Wrap enabled assignment in i915_hpd_poll_init_work() in READ_ONCE() for doc purposes - Do the same for dev_priv->hotplug.poll_enabled with WRITE_ONCE in intel_hpd_poll_enable() - Add some comments about racing not mattering in intel_hpd_poll_enable Changes since v4: - Rename intel_hpd_poll_enable() to intel_hpd_poll_init() - Drop the bool argument from intel_hpd_poll_init() - Remove redundant calls to intel_hpd_poll_init() - Rename poll_enable_work to poll_init_work - Add some kerneldoc for intel_hpd_poll_init() - Cross-reference intel_hpd_poll_init() in intel_hpd_init() - Just copy the loop from intel_hpd_init() in intel_hpd_poll_init() Changes since v5: - Minor kerneldoc nitpicks Cc: stable at vger.kernel.org Cc: Ville Syrjälä Reviewed-by: Daniel Vetter Signed-off-by: Lyude --- drivers/gpu/drm/i915/i915_drv.c | 3 ++ drivers/gpu/drm/i915/i915_drv.h | 3 ++ drivers/gpu/drm/i915/intel_drv.h| 2 + drivers/gpu/drm/i915/intel_hotplug.c| 90 - drivers/gpu/drm/i915/intel_runtime_pm.c | 2 + 5 files changed, 88 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 3eb47fb..86ff8a7 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1612,6 +1612,9 @@ static int intel_runtime_suspend(struct device *device) assert_forcewakes_inactive(dev_priv); + if (!IS_VALLEYVIEW(dev_priv) || !IS_CHERRYVIEW(dev_priv)) + intel_hpd_poll_init(dev_priv); + DRM_DEBUG_KMS("Device suspended\n"); return 0; } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 239a3ec..bbcd625 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -283,6 +283,9 @@ struct i915_hotplug { u32 short_port_mask; struct work_struct dig_port_work; + struct work_struct poll_init_work; + bool poll_enabled; + /* * if we get a HPD irq from DP and a HPD irq from non-DP * the non-DP HPD could block the workqueue on a mode config diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index dcbfdde..8af0668 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1385,6 +1385,8 @@ int intel_dsi_dcs_init_backlight_funcs(struct intel_connector *intel_connector); /* intel_dvo.c */ void intel_dvo_init(struct drm_device *dev); +/* intel_hotplug.c */ +void intel_hpd_poll_init(struct drm_i915_private *dev_priv); /* legacy fbdev emulation in intel_fbdev.c */ diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c index ec3285f..816c761 100644 --- a/drivers/gpu/drm/i915/intel_hotplug.c +++ b/drivers/gpu/drm/i915/intel_hotplug.c @@ -452,20 +452,47 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv, * * This is a separate step from interrupt enabling to simplify the locking rules * in the driver load and resume code. + * + * Also see: intel_hpd_poll_init(), which enables connec
[PATCH v6 3/4] drm/i915/vlv: Disable HPD in valleyview_crt_detect_hotplug()
One of the things preventing us from using polling is the fact that calling valleyview_crt_detect_hotplug() when there's a VGA cable connected results in sending another hotplug. With polling enabled when HPD is disabled, this results in a scenario like this: - We enable power wells and reset the ADPA - output_poll_exec does force probe on VGA, triggering a hpd - HPD handler waits for poll to unlock dev->mode_config.mutex - output_poll_exec shuts off the ADPA, unlocks dev->mode_config.mutex - HPD handler runs, resets ADPA and brings us back to the start This results in an endless irq storm getting sent from the ADPA whenever a VGA connector gets detected in the middle of polling. Somewhat based off of the "drm/i915: Disable CRT HPD around force trigger" patch Ville Syrjälä sent a while back Cc: stable at vger.kernel.org Cc: Ville Syrjälä Signed-off-by: Lyude --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/intel_crt.c | 18 ++ drivers/gpu/drm/i915/intel_hotplug.c | 27 +++ 3 files changed, 47 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a988a95..239a3ec 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2933,6 +2933,8 @@ void intel_hpd_init(struct drm_i915_private *dev_priv); void intel_hpd_init_work(struct drm_i915_private *dev_priv); void intel_hpd_cancel_work(struct drm_i915_private *dev_priv); bool intel_hpd_pin_to_port(enum hpd_pin pin, enum port *port); +bool intel_hpd_disable(struct drm_i915_private *dev_priv, enum hpd_pin pin); +void intel_hpd_enable(struct drm_i915_private *dev_priv, enum hpd_pin pin); /* i915_irq.c */ void i915_queue_hangcheck(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index 220be7a..a626793 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -327,10 +327,25 @@ static bool valleyview_crt_detect_hotplug(struct drm_connector *connector) struct drm_device *dev = connector->dev; struct intel_crt *crt = intel_attached_crt(connector); struct drm_i915_private *dev_priv = dev->dev_private; + bool reenable_hpd; u32 adpa; bool ret; u32 save_adpa; + /* +* Doing a force trigger causes a hpd interrupt to get sent, which can +* get us stuck in a loop if we're polling: +* - We enable power wells and reset the ADPA +* - output_poll_exec does force probe on VGA, triggering a hpd +* - HPD handler waits for poll to unlock dev->mode_config.mutex +* - output_poll_exec shuts off the ADPA, unlocks +*dev->mode_config.mutex +* - HPD handler runs, resets ADPA and brings us back to the start +* +* Just disable HPD interrupts here to prevent this +*/ + reenable_hpd = intel_hpd_disable(dev_priv, crt->base.hpd_pin); + save_adpa = adpa = I915_READ(crt->adpa_reg); DRM_DEBUG_KMS("trigger hotplug detect cycle: adpa=0x%x\n", adpa); @@ -353,6 +368,9 @@ static bool valleyview_crt_detect_hotplug(struct drm_connector *connector) DRM_DEBUG_KMS("valleyview hotplug adpa=0x%x, result %d\n", adpa, ret); + if (reenable_hpd) + intel_hpd_enable(dev_priv, crt->base.hpd_pin); + return ret; } diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c index 38eeca7..ec3285f 100644 --- a/drivers/gpu/drm/i915/intel_hotplug.c +++ b/drivers/gpu/drm/i915/intel_hotplug.c @@ -510,3 +510,30 @@ void intel_hpd_cancel_work(struct drm_i915_private *dev_priv) cancel_work_sync(&dev_priv->hotplug.hotplug_work); cancel_delayed_work_sync(&dev_priv->hotplug.reenable_work); } + +bool intel_hpd_disable(struct drm_i915_private *dev_priv, enum hpd_pin pin) +{ + bool ret = false; + + if (pin == HPD_NONE) + return false; + + spin_lock_irq(&dev_priv->irq_lock); + if (dev_priv->hotplug.stats[pin].state == HPD_ENABLED) { + dev_priv->hotplug.stats[pin].state = HPD_DISABLED; + ret = true; + } + spin_unlock_irq(&dev_priv->irq_lock); + + return ret; +} + +void intel_hpd_enable(struct drm_i915_private *dev_priv, enum hpd_pin pin) +{ + if (pin == HPD_NONE) + return; + + spin_lock_irq(&dev_priv->irq_lock); + dev_priv->hotplug.stats[pin].state = HPD_ENABLED; + spin_unlock_irq(&dev_priv->irq_lock); +} -- 2.5.5
[PATCH v6 2/4] drm/i915/vlv: Reset the ADPA in vlv_display_power_well_init()
While VGA hotplugging worked(ish) before, it looks like that was mainly because we'd unintentionally enable it in valleyview_crt_detect_hotplug() when we did a force trigger. This doesn't work reliably enough because whenever the display powerwell on vlv gets disabled, the values set in VLV_ADPA get cleared and consequently VGA hotplugging gets disabled. This causes bugs such as one we found on an Intel NUC, where doing the following sequence of hotplugs: - Disconnect all monitors - Connect VGA - Disconnect VGA - Connect HDMI Would result in VGA hotplugging becoming disabled, due to the powerwells getting toggled in the process of connecting HDMI. Changes since v3: - Expose intel_crt_reset() through intel_drv.h and call that in vlv_display_power_well_init() instead of encoder->base.funcs->reset(&encoder->base); Changes since v2: - Use intel_encoder structs instead of drm_encoder structs Changes since v1: - Instead of handling the register writes ourself, we just reuse intel_crt_detect() - Instead of resetting the ADPA during display IRQ installation, we now reset them in vlv_display_power_well_init() Cc: stable at vger.kernel.org Acked-by: Daniel Vetter Signed-off-by: Lyude Reviewed-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_crt.c| 2 +- drivers/gpu/drm/i915/intel_drv.h| 2 +- drivers/gpu/drm/i915/intel_runtime_pm.c | 7 +++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index dfcd718..220be7a 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -713,7 +713,7 @@ static int intel_crt_set_property(struct drm_connector *connector, return 0; } -static void intel_crt_reset(struct drm_encoder *encoder) +void intel_crt_reset(struct drm_encoder *encoder) { struct drm_device *dev = encoder->dev; struct drm_i915_private *dev_priv = dev->dev_private; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 496c962..dcbfdde 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1075,7 +1075,7 @@ void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv, /* intel_crt.c */ void intel_crt_init(struct drm_device *dev); - +void intel_crt_reset(struct drm_encoder *encoder); /* intel_ddi.c */ void intel_ddi_clk_select(struct intel_encoder *encoder, diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index e856d49..f88ef76 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -1065,6 +1065,7 @@ static void vlv_init_display_clock_gating(struct drm_i915_private *dev_priv) static void vlv_display_power_well_init(struct drm_i915_private *dev_priv) { + struct intel_encoder *encoder; enum pipe pipe; /* @@ -1100,6 +1101,12 @@ static void vlv_display_power_well_init(struct drm_i915_private *dev_priv) intel_hpd_init(dev_priv); + /* Re-enable the ADPA, if we have one */ + for_each_intel_encoder(dev_priv->dev, encoder) { + if (encoder->type == INTEL_OUTPUT_ANALOG) + intel_crt_reset(&encoder->base); + } + i915_redisable_vga_power_on(dev_priv->dev); } -- 2.5.5
[PATCH v6 1/4] drm/i915/vlv: Make intel_crt_reset() per-encoder
This lets call intel_crt_reset() in contexts where IRQs are disabled and as such, can't hold the locks required to work with the connectors. Cc: stable at vger.kernel.org Cc: Ville Syrjälä Acked-by: Daniel Vetter Signed-off-by: Lyude --- drivers/gpu/drm/i915/intel_crt.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index e115bcc..dfcd718 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -713,11 +713,11 @@ static int intel_crt_set_property(struct drm_connector *connector, return 0; } -static void intel_crt_reset(struct drm_connector *connector) +static void intel_crt_reset(struct drm_encoder *encoder) { - struct drm_device *dev = connector->dev; + struct drm_device *dev = encoder->dev; struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crt *crt = intel_attached_crt(connector); + struct intel_crt *crt = intel_encoder_to_crt(to_intel_encoder(encoder)); if (INTEL_INFO(dev)->gen >= 5) { u32 adpa; @@ -739,7 +739,6 @@ static void intel_crt_reset(struct drm_connector *connector) */ static const struct drm_connector_funcs intel_crt_connector_funcs = { - .reset = intel_crt_reset, .dpms = drm_atomic_helper_connector_dpms, .detect = intel_crt_detect, .fill_modes = drm_helper_probe_single_connector_modes, @@ -757,6 +756,7 @@ static const struct drm_connector_helper_funcs intel_crt_connector_helper_funcs }; static const struct drm_encoder_funcs intel_crt_enc_funcs = { + .reset = intel_crt_reset, .destroy = intel_encoder_destroy, }; @@ -901,5 +901,5 @@ void intel_crt_init(struct drm_device *dev) dev_priv->fdi_rx_config = I915_READ(FDI_RX_CTL(PIPE_A)) & fdi_config; } - intel_crt_reset(connector); + intel_crt_reset(&crt->base.base); } -- 2.5.5
[PATCH v6 0/4] Fixes for HPD
Latest version of: https://lists.freedesktop.org/archives/intel-gfx/2016-June/098787.html The only patch that's changed here is 4/4, the rest are just being sent so that they can be in one thread to make things easier for reviewers Lyude (4): drm/i915/vlv: Make intel_crt_reset() per-encoder drm/i915/vlv: Reset the ADPA in vlv_display_power_well_init() drm/i915/vlv: Disable HPD in valleyview_crt_detect_hotplug() drm/i915: Enable polling when we don't have hpd drivers/gpu/drm/i915/i915_drv.c | 3 + drivers/gpu/drm/i915/i915_drv.h | 5 ++ drivers/gpu/drm/i915/intel_crt.c| 28 ++-- drivers/gpu/drm/i915/intel_drv.h| 4 +- drivers/gpu/drm/i915/intel_hotplug.c| 117 drivers/gpu/drm/i915/intel_runtime_pm.c | 9 +++ 6 files changed, 148 insertions(+), 18 deletions(-) -- 2.5.5
[Intel-gfx] Bad flicker on skylake HQD due to code in the 4.7 merge window
On Tue, 2016-06-21 at 18:44 +0300, Ville Syrjälä wrote: > On Tue, Jun 21, 2016 at 09:53:15AM -0400, James Bottomley wrote: > > On Mon, 2016-06-20 at 11:03 +0300, Jani Nikula wrote: > > > Cc: Ville > > > > > > On Mon, 20 Jun 2016, James Bottomley < > > > James.Bottomley at HansenPartnership.com> wrote: > > > > OK, my candidate bad commit is this one: > > > > > > > > commit a05628195a0d9f3173dd9aa76f482aef692e46ee > > > > Author: Ville Syrjälä > > > > Date: Mon Apr 11 10:23:51 2016 +0300 > > > > > > > > drm/i915: Get panel_type from OpRegion panel details > > > > > > > > After being more careful about waiting to identify flicker, > > > > this one seems to be the one the bisect finds. I'm now running > > > > v4.7-rc3 with this one reverted and am currently seeing no > > > > flicker problems. It is, however, early days because the > > > > flicker can hide for long periods, so I 'll wait until Monday > > > > evening and a few reboots before declaring victory. > > > > > > If that turns out to be the bad commit, it doesn't really > > > surprise me, and that in itself is depressing. > > > > As far as I can tell, after running for a day with this reverted, > > this is the problem. The flicker hasn't appeared with it reverted. > > It's pretty noticeable with this commit included. > > Hmm. The only difference I can see is low vs. normal vswing. Panel 0 > has low, panel 2 has normal. So either the VBT or opregion is telling > utter lies, or there's some other bug in our low vswing support. > > To confirm it's really a vswing issue, you should be able to run with > i915.edp_vswing=2 without flickers on the broken kernel. Preliminary boot indicates no flicker with the bad commit included and this option, but I'll have to run for quite a bit longer to verify, since it can sometimes be elusive. James
[PATCH 1/2] drm/omap: dsi: Remove unused variable
Hi Thierry, On 21/06/16 16:56, Thierry Reding wrote: > From: Thierry Reding > > Commit 973999aa0140 ("drm/omap: Remove regulator API abuse") removed the > only user of the local 'r' variable, which thus became unused. > > Signed-off-by: Thierry Reding > --- > drivers/gpu/drm/omapdrm/dss/dsi.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c > b/drivers/gpu/drm/omapdrm/dss/dsi.c > index 6f45e9d00b41..e1be5e795cd8 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dsi.c > +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c > @@ -1167,7 +1167,6 @@ static int dsi_regulator_init(struct platform_device > *dsidev) > { > struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev); > struct regulator *vdds_dsi; > - int r; > > if (dsi->vdds_dsi_reg != NULL) > return 0; > Thanks, but there's already 85332739628fe4beafecdb713438c7cb1454c2f5 ("drm/omap: fix unused variable warning in dsi & hdmi") in the mainline (-rc3). Tomi -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: OpenPGP digital signature URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160621/d60c4892/attachment-0001.sig>
[PATCH v4 1/3] drm: Add callbacks for late registering
Like what has been done for connectors add callbacks on encoder, crtc and plane to let driver do actions after drm device registration. Correspondingly, add callbacks called before unregister drm device. version 2: add drm_modeset_register_all() and drm_modeset_unregister_all() to centralize all calls version 3: in error case unwind registers in drm_modeset_register_all fix uninitialed return value inverse order of unregistration in drm_modeset_unregister_all version 4: move function definitions in drm_crtc_internal.h remove not needed documentation Signed-off-by: Benjamin Gaignard --- drivers/gpu/drm/drm_crtc.c | 115 drivers/gpu/drm/drm_crtc_internal.h | 2 + drivers/gpu/drm/drm_drv.c | 5 +- include/drm/drm_crtc.h | 77 4 files changed, 197 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index e7c862b..a717238 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -608,6 +608,31 @@ static unsigned int drm_num_crtcs(struct drm_device *dev) return num; } +static int drm_crtc_register_all(struct drm_device *dev) +{ + struct drm_crtc *crtc; + int ret = 0; + + drm_for_each_crtc(crtc, dev) { + if (crtc->funcs->late_register) + ret = crtc->funcs->late_register(crtc); + if (ret) + return ret; + } + + return 0; +} + +static void drm_crtc_unregister_all(struct drm_device *dev) +{ + struct drm_crtc *crtc; + + drm_for_each_crtc(crtc, dev) { + if (crtc->funcs->early_unregister) + crtc->funcs->early_unregister(crtc); + } +} + /** * drm_crtc_init_with_planes - Initialise a new CRTC object with *specified primary and cursor planes. @@ -1102,6 +1127,31 @@ void drm_connector_unregister_all(struct drm_device *dev) } EXPORT_SYMBOL(drm_connector_unregister_all); +static int drm_encoder_register_all(struct drm_device *dev) +{ + struct drm_encoder *encoder; + int ret = 0; + + drm_for_each_encoder(encoder, dev) { + if (encoder->funcs->late_register) + ret = encoder->funcs->late_register(encoder); + if (ret) + return ret; + } + + return 0; +} + +static void drm_encoder_unregister_all(struct drm_device *dev) +{ + struct drm_encoder *encoder; + + drm_for_each_encoder(encoder, dev) { + if (encoder->funcs->early_unregister) + encoder->funcs->early_unregister(encoder); + } +} + /** * drm_encoder_init - Init a preallocated encoder * @dev: drm device @@ -1290,6 +1340,31 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane, } EXPORT_SYMBOL(drm_universal_plane_init); +static int drm_plane_register_all(struct drm_device *dev) +{ + struct drm_plane *plane; + int ret = 0; + + drm_for_each_plane(plane, dev) { + if (plane->funcs->late_register) + ret = plane->funcs->late_register(plane); + if (ret) + return ret; + } + + return 0; +} + +static void drm_plane_unregister_all(struct drm_device *dev) +{ + struct drm_plane *plane; + + drm_for_each_plane(plane, dev) { + if (plane->funcs->early_unregister) + plane->funcs->early_unregister(plane); + } +} + /** * drm_plane_init - Initialize a legacy plane * @dev: DRM device @@ -1412,6 +1487,46 @@ void drm_plane_force_disable(struct drm_plane *plane) } EXPORT_SYMBOL(drm_plane_force_disable); +int drm_modeset_register_all(struct drm_device *dev) +{ + int ret; + + ret = drm_plane_register_all(dev); + if (ret) + goto err_plane; + + ret = drm_crtc_register_all(dev); + if (ret) + goto err_crtc; + + ret = drm_encoder_register_all(dev); + if (ret) + goto err_encoder; + + ret = drm_connector_register_all(dev); + if (ret) + goto err_connector; + + return 0; + +err_connector: + drm_encoder_unregister_all(dev); +err_encoder: + drm_crtc_unregister_all(dev); +err_crtc: + drm_plane_unregister_all(dev); +err_plane: + return ret; +} + +void drm_modeset_unregister_all(struct drm_device *dev) +{ + drm_connector_unregister_all(dev); + drm_encoder_unregister_all(dev); + drm_crtc_unregister_all(dev); + drm_plane_unregister_all(dev); +} + static int drm_mode_create_standard_properties(struct drm_device *dev) { struct drm_property *prop; diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h index a78c138..a3b0fbc 100644 --- a/drivers/gpu/drm/drm_crtc_internal.h +++ b/drivers/gpu/drm/drm_crtc_internal.h @@ -42,3 +42,5 @@
[PATCH v4 1/3] drm: Add callbacks for late registering
On Tue, Jun 21, 2016 at 04:37:09PM +0200, Benjamin Gaignard wrote: > Like what has been done for connectors add callbacks on encoder, > crtc and plane to let driver do actions after drm device registration. > > Correspondingly, add callbacks called before unregister drm device. > > version 2: > add drm_modeset_register_all() and drm_modeset_unregister_all() > to centralize all calls > > version 3: > in error case unwind registers in drm_modeset_register_all > fix uninitialed return value > inverse order of unregistration in drm_modeset_unregister_all > > version 4: > move function definitions in drm_crtc_internal.h > remove not needed documentation > > Signed-off-by: Benjamin Gaignard Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre
[PATCH 1/2] drm/omap: dsi: Remove unused variable
On Tue, Jun 21, 2016 at 05:00:15PM +0300, Tomi Valkeinen wrote: > Hi Thierry, > > On 21/06/16 16:56, Thierry Reding wrote: > > From: Thierry Reding > > > > Commit 973999aa0140 ("drm/omap: Remove regulator API abuse") removed the > > only user of the local 'r' variable, which thus became unused. > > > > Signed-off-by: Thierry Reding > > --- > > drivers/gpu/drm/omapdrm/dss/dsi.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c > > b/drivers/gpu/drm/omapdrm/dss/dsi.c > > index 6f45e9d00b41..e1be5e795cd8 100644 > > --- a/drivers/gpu/drm/omapdrm/dss/dsi.c > > +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c > > @@ -1167,7 +1167,6 @@ static int dsi_regulator_init(struct platform_device > > *dsidev) > > { > > struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev); > > struct regulator *vdds_dsi; > > - int r; > > > > if (dsi->vdds_dsi_reg != NULL) > > return 0; > > > > Thanks, but there's already 85332739628fe4beafecdb713438c7cb1454c2f5 > ("drm/omap: fix unused variable warning in dsi & hdmi") in the mainline > (-rc3). Bah... that's what I get for not generating patches against linux-next. Thanks for letting me know. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160621/78fda562/attachment.sig>
[PATCH 2/2] drm/omap: hdmi: Remove unused variable
From: Thierry Reding Commit 973999aa0140 ("drm/omap: Remove regulator API abuse") removed the only user of the local 'r' variable, which thus became unused. Signed-off-by: Thierry Reding --- drivers/gpu/drm/omapdrm/dss/hdmi5.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5.c b/drivers/gpu/drm/omapdrm/dss/hdmi5.c index 061f9bab4c9b..0c0a5139a301 100644 --- a/drivers/gpu/drm/omapdrm/dss/hdmi5.c +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5.c @@ -120,7 +120,6 @@ static irqreturn_t hdmi_irq_handler(int irq, void *data) static int hdmi_init_regulator(void) { - int r; struct regulator *reg; if (hdmi.vdda_reg != NULL) -- 2.8.3
[PATCH 1/2] drm/omap: dsi: Remove unused variable
From: Thierry Reding Commit 973999aa0140 ("drm/omap: Remove regulator API abuse") removed the only user of the local 'r' variable, which thus became unused. Signed-off-by: Thierry Reding --- drivers/gpu/drm/omapdrm/dss/dsi.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c index 6f45e9d00b41..e1be5e795cd8 100644 --- a/drivers/gpu/drm/omapdrm/dss/dsi.c +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c @@ -1167,7 +1167,6 @@ static int dsi_regulator_init(struct platform_device *dsidev) { struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev); struct regulator *vdds_dsi; - int r; if (dsi->vdds_dsi_reg != NULL) return 0; -- 2.8.3
[PATCH] drm/fsl-dcu: use drm_mode_config_cleanup on initialization errors
On Sat, Jun 18, 2016 at 07:15:43PM -0700, Stefan Agner wrote: > Commit 7566e247672d ("drm/fsl-dcu: handle initialization errors properly") > introduced error handling during initialization, but with a wrong cleanup > order. > > Replace the error handling with the generic cleanup function > drm_mode_config_cleanup. > > Signed-off-by: Stefan Agner > --- > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c | 13 ++--- > 1 file changed, 6 insertions(+), 7 deletions(-) Applied to topic/drm-misc, thanks. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160621/fa2c4b1a/attachment.sig>
[PATCH v4 0/8] iommu/rockchip: Fix bugs and enable on ARM64
On Tue, Jun 21, 2016 at 03:18:49PM +0200, Heiko Stübner wrote: > Am Dienstag, 21. Juni 2016, 14:54:35 schrieb Joerg Roedel: > > Hi Tomasz, > > > > On Tue, Jun 21, 2016 at 09:42:16PM +0900, Tomasz Figa wrote: > > > In simple words, DRM patches depend on IOMMU patches. > > > > > > More precisely: The IOMMU patches alone are supposed to not break > > > anything. Same goes for the first DRM patch (7/8). Only second DRM > > > patch (8/8) depends on changes introduced by its predecessors. > > > > The first DRM patch is 6/7, so it is 7/8 with the iommu dependency, > > right? Anyway, I think the best is I take the iommu patches when Heiko > > is ok with them and then the DRM tree can merge that branch in to apply > > the DRM patches. > > > > But first Heiko should have a look at the patches. > > I think from all his previous work on the rockchip iommus Tomasz is a lot > more > qualified to judge them - which I guess he did when picking up the ones from > Rockchip devs :-) . In that case you guys should probably co-maintain that driver? > > >From a style-side, please don't carry the Reviewed-on gerrit tags over to > mainline patches (patches 1 and 2). Okay. Joerg
[PATCH v3 1/3] drm: Add callbacks for late registering
On Tue, Jun 21, 2016 at 03:09:38PM +0200, Benjamin Gaignard wrote: > Like what has been done for connectors add callbacks on encoder, > crtc and plane to let driver do actions after drm device registration. > > Correspondingly, add callbacks called before unregister drm device. > > version 2: > add drm_modeset_register_all() and drm_modeset_unregister_all() > to centralize all calls > > version 3: > in error case unwind registers in drm_modeset_register_all > fix uninitialed return value > inverse order of unregistration in drm_modeset_unregister_all > > Signed-off-by: Benjamin Gaignard > --- > drivers/gpu/drm/drm_crtc.c | 132 > + > drivers/gpu/drm/drm_drv.c | 4 +- > include/drm/drm_crtc.h | 79 +++ > 3 files changed, 213 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index e7c862b..c078bc4 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -608,6 +608,31 @@ static unsigned int drm_num_crtcs(struct drm_device *dev) > return num; > } > > +static int drm_crtc_register_all(struct drm_device *dev) > +{ > + struct drm_crtc *crtc; > + int ret = 0; > + > + drm_for_each_crtc(crtc, dev) { > + if (crtc->funcs->late_register) > + ret = crtc->funcs->late_register(crtc); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > +static void drm_crtc_unregister_all(struct drm_device *dev) > +{ > + struct drm_crtc *crtc; > + > + drm_for_each_crtc(crtc, dev) { > + if (crtc->funcs->early_unregister) > + crtc->funcs->early_unregister(crtc); > + } > +} > + > /** > * drm_crtc_init_with_planes - Initialise a new CRTC object with > *specified primary and cursor planes. > @@ -1102,6 +1127,31 @@ void drm_connector_unregister_all(struct drm_device > *dev) > } > EXPORT_SYMBOL(drm_connector_unregister_all); > > +static int drm_encoder_register_all(struct drm_device *dev) > +{ > + struct drm_encoder *encoder; > + int ret = 0; > + > + drm_for_each_encoder(encoder, dev) { > + if (encoder->funcs->late_register) > + ret = encoder->funcs->late_register(encoder); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > +static void drm_encoder_unregister_all(struct drm_device *dev) > +{ > + struct drm_encoder *encoder; > + > + drm_for_each_encoder(encoder, dev) { > + if (encoder->funcs->early_unregister) > + encoder->funcs->early_unregister(encoder); > + } > +} > + > /** > * drm_encoder_init - Init a preallocated encoder > * @dev: drm device > @@ -1290,6 +1340,31 @@ int drm_universal_plane_init(struct drm_device *dev, > struct drm_plane *plane, > } > EXPORT_SYMBOL(drm_universal_plane_init); > > +static int drm_plane_register_all(struct drm_device *dev) > +{ > + struct drm_plane *plane; > + int ret = 0; > + > + drm_for_each_plane(plane, dev) { > + if (plane->funcs->late_register) > + ret = plane->funcs->late_register(plane); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > +static void drm_plane_unregister_all(struct drm_device *dev) > +{ > + struct drm_plane *plane; > + > + drm_for_each_plane(plane, dev) { > + if (plane->funcs->early_unregister) > + plane->funcs->early_unregister(plane); > + } > +} > + > /** > * drm_plane_init - Initialize a legacy plane > * @dev: DRM device > @@ -1412,6 +1487,63 @@ void drm_plane_force_disable(struct drm_plane *plane) > } > EXPORT_SYMBOL(drm_plane_force_disable); > > +/** > + * drm_modeset_register_all - do late registration > + * @dev: drm device > + * > + * This function call late_register callback for all planes, > + * crcts, encoders and connectors > + * > + * Returns: > + * Zero on success, erro code on failure > + */ We don't document drm-internal functions, it just confuses the docs (which are aimed at driver writers). Since the comment doesn't explain much of what the code does, please remove. Same for unregister below. > +int drm_modeset_register_all(struct drm_device *dev) > +{ > + int ret; > + > + ret = drm_plane_register_all(dev); > + if (ret) > + goto err_plane; > + > + ret = drm_crtc_register_all(dev); > + if (ret) > + goto err_crtc; > + > + ret = drm_encoder_register_all(dev); > + if (ret) > + goto err_encoder; > + > + ret = drm_connector_register_all(dev); > + if (ret) > + goto err_connector; > + > + return 0; > + > +err_connector: > + drm_encoder_unregister_all(dev); > +err_encoder: > + drm_crtc_unregister_all(dev); > +err_crtc: > + drm_plane_unregister_all(dev); > +err_
[PATCH] drm/hisilicon: add select HISI_KIRIN_DW_DSI
On Mon, Jun 20, 2016 at 11:59:03AM +0800, Xinliang Liu wrote: > From: Guodong Xu > > Add select HISI_KIRIN_DW_DSI to Kconfig. > The DRM driver depends on dsi sub-driver. > > Signed-off-by: Zoltan Kuscsik > --- > drivers/gpu/drm/hisilicon/kirin/Kconfig | 1 + > 1 file changed, 1 insertion(+) You've got the Signed-off-by area messed up. If Guodong wrote this patch that his Signed-off-by needs to be added. Your Signed-off by should also be added because you forwarded the patch to the mailing list. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160621/4934490a/attachment-0001.sig>
[PATCH] drm: mediatek: remove IOMMU_DMA select
On Wed, May 11, 2016 at 10:15:20PM +0200, Arnd Bergmann wrote: > On Wednesday 11 May 2016 22:11:07 Arnd Bergmann wrote: > > We get a harmless build warning when trying to use the mediatek > > DRM driver with IOMMU support disabled: > > > > warning: (DRM_MEDIATEK) selects IOMMU_DMA which has unmet direct > > dependencies (IOMMU_SUPPORT) > > > > However, the IOMMU_DMA symbol is not meant to be used by drivers > > at all, and this driver doesn't seem to have a strict dependency > > on it other than using the mediatek IOMMU driver that does. > > > > Since we also want to be able to do compile tests with the > > driver on other platforms, the IOMMU_DMA symbol should not > > be selected here. > > > > Signed-off-by: Arnd Bergmann > > --- > > If someone has a better explanation about why the 'select' is here, > > let me know, it certainly seems out of place. > > Sorry, I didn't mean to send this as a reply to "More build fixes for > omapdrm in current -next", I copied the wrong command line. > > I'll resend it if necessary. That's probably the reason why patchwork didn't include the commit message in the downloaded mbox. I've pieced it together manually and applied this to topic/drm-misc. Thanks, Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160621/563249f9/attachment.sig>
[PATCH v4 0/8] iommu/rockchip: Fix bugs and enable on ARM64
Am Dienstag, 21. Juni 2016, 14:54:35 schrieb Joerg Roedel: > Hi Tomasz, > > On Tue, Jun 21, 2016 at 09:42:16PM +0900, Tomasz Figa wrote: > > In simple words, DRM patches depend on IOMMU patches. > > > > More precisely: The IOMMU patches alone are supposed to not break > > anything. Same goes for the first DRM patch (7/8). Only second DRM > > patch (8/8) depends on changes introduced by its predecessors. > > The first DRM patch is 6/7, so it is 7/8 with the iommu dependency, > right? Anyway, I think the best is I take the iommu patches when Heiko > is ok with them and then the DRM tree can merge that branch in to apply > the DRM patches. > > But first Heiko should have a look at the patches. I think from all his previous work on the rockchip iommus Tomasz is a lot more qualified to judge them - which I guess he did when picking up the ones from Rockchip devs :-) . >From a style-side, please don't carry the Reviewed-on gerrit tags over to mainline patches (patches 1 and 2). Other than that, I didn't see anything jump out and it looks all pretty nice. Heiko
[PATCH v3 3/3] drm: sti: rework init sequence
Use drm_dev_alloc() and drm_dev_register() instead of .load() To simplify init sequence only create fbdev when requested in output_poll_changed(). version 2: remove call to drm_connector_unregister_all() and drm_dev_set_unique() Signed-off-by: Benjamin Gaignard --- drivers/gpu/drm/sti/sti_drv.c | 137 + drivers/gpu/drm/sti/sti_drv.h | 1 + drivers/gpu/drm/sti/sti_dvo.c | 7 --- drivers/gpu/drm/sti/sti_hda.c | 8 +-- drivers/gpu/drm/sti/sti_hdmi.c | 21 +-- 5 files changed, 100 insertions(+), 74 deletions(-) diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c index 26aa85d..96bd3d0 100644 --- a/drivers/gpu/drm/sti/sti_drv.c +++ b/drivers/gpu/drm/sti/sti_drv.c @@ -226,8 +226,28 @@ static int sti_atomic_commit(struct drm_device *drm, return 0; } +static void sti_output_poll_changed(struct drm_device *ddev) +{ + struct sti_private *private = ddev->dev_private; + + if (!ddev->mode_config.num_connector) + return; + + if (private->fbdev) { + drm_fbdev_cma_hotplug_event(private->fbdev); + return; + } + + private->fbdev = drm_fbdev_cma_init(ddev, 32, + ddev->mode_config.num_crtc, + ddev->mode_config.num_connector); + if (IS_ERR(private->fbdev)) + private->fbdev = NULL; +} + static const struct drm_mode_config_funcs sti_mode_config_funcs = { .fb_create = drm_fb_cma_create, + .output_poll_changed = sti_output_poll_changed, .atomic_check = drm_atomic_helper_check, .atomic_commit = sti_atomic_commit, }; @@ -248,44 +268,6 @@ static void sti_mode_config_init(struct drm_device *dev) dev->mode_config.funcs = &sti_mode_config_funcs; } -static int sti_load(struct drm_device *dev, unsigned long flags) -{ - struct sti_private *private; - int ret; - - private = kzalloc(sizeof(*private), GFP_KERNEL); - if (!private) { - DRM_ERROR("Failed to allocate private\n"); - return -ENOMEM; - } - dev->dev_private = (void *)private; - private->drm_dev = dev; - - mutex_init(&private->commit.lock); - INIT_WORK(&private->commit.work, sti_atomic_work); - - drm_mode_config_init(dev); - drm_kms_helper_poll_init(dev); - - sti_mode_config_init(dev); - - ret = component_bind_all(dev->dev, dev); - if (ret) { - drm_kms_helper_poll_fini(dev); - drm_mode_config_cleanup(dev); - kfree(private); - return ret; - } - - drm_mode_config_reset(dev); - - drm_fbdev_cma_init(dev, 32, - dev->mode_config.num_crtc, - dev->mode_config.num_connector); - - return 0; -} - static const struct file_operations sti_driver_fops = { .owner = THIS_MODULE, .open = drm_open, @@ -302,7 +284,6 @@ static const struct file_operations sti_driver_fops = { static struct drm_driver sti_driver = { .driver_features = DRIVER_HAVE_IRQ | DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | DRIVER_ATOMIC, - .load = sti_load, .gem_free_object_unlocked = drm_gem_cma_free_object, .gem_vm_ops = &drm_gem_cma_vm_ops, .dumb_create = drm_gem_cma_dumb_create, @@ -339,14 +320,88 @@ static int compare_of(struct device *dev, void *data) return dev->of_node == data; } +static int sti_init(struct drm_device *ddev) +{ + struct sti_private *private; + + private = kzalloc(sizeof(*private), GFP_KERNEL); + if (!private) + return -ENOMEM; + + ddev->dev_private = (void *)private; + dev_set_drvdata(ddev->dev, ddev); + private->drm_dev = ddev; + + mutex_init(&private->commit.lock); + INIT_WORK(&private->commit.work, sti_atomic_work); + + drm_mode_config_init(ddev); + + sti_mode_config_init(ddev); + + drm_kms_helper_poll_init(ddev); + + return 0; +} + +static void sti_cleanup(struct drm_device *ddev) +{ + struct sti_private *private = ddev->dev_private; + + if (private->fbdev) { + drm_fbdev_cma_fini(private->fbdev); + private->fbdev = NULL; + } + + drm_kms_helper_poll_fini(ddev); + drm_vblank_cleanup(ddev); + kfree(private); + ddev->dev_private = NULL; +} + static int sti_bind(struct device *dev) { - return drm_platform_init(&sti_driver, to_platform_device(dev)); + struct drm_device *ddev; + int ret; + + ddev = drm_dev_alloc(&sti_driver, dev); + if (!ddev) + return -ENOMEM; + + ddev->platformdev = to_platform_device(dev); + + ret = sti_init(ddev); + if (ret) + goto err_drm_dev_unref; + + ret = component_bind_all(ddev->dev, ddev); + if (ret) + goto err
[PATCH v3 2/3] drm: sti: use late_register and early_unregister callbacks
Make sti driver use register callback to move debugfs initialization out of sub-components creation. This will allow to convert driver .load() to drm_dev_alloc() and drm_dev_register(). sti_compositor bring up 2 crtc but only one debugfs init is needed so use drm_crtc_index to do it on the first one. This can't be done in sti_drv because only sti_compositor have access to the devices. It is almost the same for sti_encoder which handle multiple encoder while one only debugfs entry is needed so add a boolean to avoid multiple debugfs initialization Signed-off-by: Benjamin Gaignard --- drivers/gpu/drm/sti/sti_compositor.c | 20 drivers/gpu/drm/sti/sti_compositor.h | 3 +++ drivers/gpu/drm/sti/sti_crtc.c | 12 drivers/gpu/drm/sti/sti_cursor.c | 32 drivers/gpu/drm/sti/sti_dvo.c| 18 ++ drivers/gpu/drm/sti/sti_gdp.c| 32 drivers/gpu/drm/sti/sti_hda.c| 18 ++ drivers/gpu/drm/sti/sti_hdmi.c | 19 +++ drivers/gpu/drm/sti/sti_hqvdp.c | 32 drivers/gpu/drm/sti/sti_mixer.c | 5 + drivers/gpu/drm/sti/sti_mixer.h | 2 ++ drivers/gpu/drm/sti/sti_plane.c | 24 +++- drivers/gpu/drm/sti/sti_plane.h | 7 +-- drivers/gpu/drm/sti/sti_tvout.c | 36 ++-- drivers/gpu/drm/sti/sti_vid.c| 5 + drivers/gpu/drm/sti/sti_vid.h| 2 ++ 16 files changed, 198 insertions(+), 69 deletions(-) diff --git a/drivers/gpu/drm/sti/sti_compositor.c b/drivers/gpu/drm/sti/sti_compositor.c index 3d2fa3a..794148f 100644 --- a/drivers/gpu/drm/sti/sti_compositor.c +++ b/drivers/gpu/drm/sti/sti_compositor.c @@ -55,6 +55,26 @@ struct sti_compositor_data stih416_compositor_data = { }, }; +int sti_compositor_debufs_init(struct sti_compositor *compo, + struct drm_minor *minor) +{ + int ret = 0, i; + + for (i = 0; compo->vid[i]; i++) { + ret = vid_debugfs_init(compo->vid[i], minor); + if (ret) + return ret; + } + + for (i = 0; compo->mixer[i]; i++) { + ret = sti_mixer_debugfs_init(compo->mixer[i], minor); + if (ret) + return ret; + } + + return 0; +} + static int sti_compositor_bind(struct device *dev, struct device *master, void *data) diff --git a/drivers/gpu/drm/sti/sti_compositor.h b/drivers/gpu/drm/sti/sti_compositor.h index 1a4a73d..2ef 100644 --- a/drivers/gpu/drm/sti/sti_compositor.h +++ b/drivers/gpu/drm/sti/sti_compositor.h @@ -81,4 +81,7 @@ struct sti_compositor { struct notifier_block vtg_vblank_nb; }; +int sti_compositor_debufs_init(struct sti_compositor *compo, + struct drm_minor *minor); + #endif diff --git a/drivers/gpu/drm/sti/sti_crtc.c b/drivers/gpu/drm/sti/sti_crtc.c index e04deed..7fab3af 100644 --- a/drivers/gpu/drm/sti/sti_crtc.c +++ b/drivers/gpu/drm/sti/sti_crtc.c @@ -331,6 +331,17 @@ void sti_crtc_disable_vblank(struct drm_device *drm_dev, unsigned int pipe) } } +static int sti_crtc_late_register(struct drm_crtc *crtc) +{ + struct sti_mixer *mixer = to_sti_mixer(crtc); + struct sti_compositor *compo = dev_get_drvdata(mixer->dev); + + if (drm_crtc_index(crtc) == 0) + return sti_compositor_debufs_init(compo, crtc->dev->primary); + + return 0; +} + static const struct drm_crtc_funcs sti_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, @@ -339,6 +350,7 @@ static const struct drm_crtc_funcs sti_crtc_funcs = { .reset = drm_atomic_helper_crtc_reset, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, + .late_register = sti_crtc_late_register, }; bool sti_crtc_is_main(struct drm_crtc *crtc) diff --git a/drivers/gpu/drm/sti/sti_cursor.c b/drivers/gpu/drm/sti/sti_cursor.c index 53aa002..a263bbb 100644 --- a/drivers/gpu/drm/sti/sti_cursor.c +++ b/drivers/gpu/drm/sti/sti_cursor.c @@ -329,6 +329,33 @@ static const struct drm_plane_helper_funcs sti_cursor_helpers_funcs = { .atomic_disable = sti_cursor_atomic_disable, }; +static void sti_cursor_destroy(struct drm_plane *drm_plane) +{ + DRM_DEBUG_DRIVER("\n"); + + drm_plane_helper_disable(drm_plane); + drm_plane_cleanup(drm_plane); +} + +static int sti_cursor_late_register(struct drm_plane *drm_plane) +{ + struct sti_plane *plane = to_sti_plane(drm_plane); + struct sti_cursor *cursor = to_sti_cursor(plane); + + return cursor_debugfs_init(cursor, drm_plane->dev->primary); +} + +struct drm_plane_funcs sti_cursor_plane_he
[PATCH v3 1/3] drm: Add callbacks for late registering
Like what has been done for connectors add callbacks on encoder, crtc and plane to let driver do actions after drm device registration. Correspondingly, add callbacks called before unregister drm device. version 2: add drm_modeset_register_all() and drm_modeset_unregister_all() to centralize all calls version 3: in error case unwind registers in drm_modeset_register_all fix uninitialed return value inverse order of unregistration in drm_modeset_unregister_all Signed-off-by: Benjamin Gaignard --- drivers/gpu/drm/drm_crtc.c | 132 + drivers/gpu/drm/drm_drv.c | 4 +- include/drm/drm_crtc.h | 79 +++ 3 files changed, 213 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index e7c862b..c078bc4 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -608,6 +608,31 @@ static unsigned int drm_num_crtcs(struct drm_device *dev) return num; } +static int drm_crtc_register_all(struct drm_device *dev) +{ + struct drm_crtc *crtc; + int ret = 0; + + drm_for_each_crtc(crtc, dev) { + if (crtc->funcs->late_register) + ret = crtc->funcs->late_register(crtc); + if (ret) + return ret; + } + + return 0; +} + +static void drm_crtc_unregister_all(struct drm_device *dev) +{ + struct drm_crtc *crtc; + + drm_for_each_crtc(crtc, dev) { + if (crtc->funcs->early_unregister) + crtc->funcs->early_unregister(crtc); + } +} + /** * drm_crtc_init_with_planes - Initialise a new CRTC object with *specified primary and cursor planes. @@ -1102,6 +1127,31 @@ void drm_connector_unregister_all(struct drm_device *dev) } EXPORT_SYMBOL(drm_connector_unregister_all); +static int drm_encoder_register_all(struct drm_device *dev) +{ + struct drm_encoder *encoder; + int ret = 0; + + drm_for_each_encoder(encoder, dev) { + if (encoder->funcs->late_register) + ret = encoder->funcs->late_register(encoder); + if (ret) + return ret; + } + + return 0; +} + +static void drm_encoder_unregister_all(struct drm_device *dev) +{ + struct drm_encoder *encoder; + + drm_for_each_encoder(encoder, dev) { + if (encoder->funcs->early_unregister) + encoder->funcs->early_unregister(encoder); + } +} + /** * drm_encoder_init - Init a preallocated encoder * @dev: drm device @@ -1290,6 +1340,31 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane, } EXPORT_SYMBOL(drm_universal_plane_init); +static int drm_plane_register_all(struct drm_device *dev) +{ + struct drm_plane *plane; + int ret = 0; + + drm_for_each_plane(plane, dev) { + if (plane->funcs->late_register) + ret = plane->funcs->late_register(plane); + if (ret) + return ret; + } + + return 0; +} + +static void drm_plane_unregister_all(struct drm_device *dev) +{ + struct drm_plane *plane; + + drm_for_each_plane(plane, dev) { + if (plane->funcs->early_unregister) + plane->funcs->early_unregister(plane); + } +} + /** * drm_plane_init - Initialize a legacy plane * @dev: DRM device @@ -1412,6 +1487,63 @@ void drm_plane_force_disable(struct drm_plane *plane) } EXPORT_SYMBOL(drm_plane_force_disable); +/** + * drm_modeset_register_all - do late registration + * @dev: drm device + * + * This function call late_register callback for all planes, + * crcts, encoders and connectors + * + * Returns: + * Zero on success, erro code on failure + */ +int drm_modeset_register_all(struct drm_device *dev) +{ + int ret; + + ret = drm_plane_register_all(dev); + if (ret) + goto err_plane; + + ret = drm_crtc_register_all(dev); + if (ret) + goto err_crtc; + + ret = drm_encoder_register_all(dev); + if (ret) + goto err_encoder; + + ret = drm_connector_register_all(dev); + if (ret) + goto err_connector; + + return 0; + +err_connector: + drm_encoder_unregister_all(dev); +err_encoder: + drm_crtc_unregister_all(dev); +err_crtc: + drm_plane_unregister_all(dev); +err_plane: + return ret; +} + +/** + * drm_modeset_unregister_all - do early unregistration + * @dev: drm device + * + * This function call early_unregister callbacks for all + * connectors, encoders, crtcs and planes + */ +void drm_modeset_unregister_all(struct drm_device *dev) +{ + drm_connector_unregister_all(dev); + drm_encoder_unregister_all(dev); + drm_crtc_unregister_all(dev); + drm_plane_unregister_all(dev); +} + static int drm_mode_create_standard_properties
[PATCH v3 0/3] Add callbacks for late registering
version 3: Dont export functions if not needed. Fix uninitialized return variable. In case of error while calling late_register unwind what was aldeay done. drm_modeset_unregister_all() call callbacks in reverse order compare to drm_modeset_register_all() version 2: create functions drm_modeset_register_all and drm_modeset_unregister_all to regroup all callbacks calls to avoid loops into drm_dev_register and drm_dev_unregister. Call order is now planes, crtcs, encoders and connectors Fix sti driver to not call drm_connector_register_all and drm_dev_set_unique version 1: late_register() and early_unregister() callbacks have been introduced for connectors, let do the same for encoders, crcts and planes. Like for the previously introduced functions they are called right after drm device registration and before unregistration. Those new callbacks allow to defer actions after drm_dev_register(). For example sti driver use them to initialize debugfs because it require to have register a drm device to get the root debug file. Benjamin Gaignard (3): drm: Add callbacks for late registering drm: sti: use late_register and early_unregister callbacks drm: sti: rework init sequence drivers/gpu/drm/drm_crtc.c | 132 + drivers/gpu/drm/drm_drv.c| 4 +- drivers/gpu/drm/sti/sti_compositor.c | 20 + drivers/gpu/drm/sti/sti_compositor.h | 3 + drivers/gpu/drm/sti/sti_crtc.c | 12 +++ drivers/gpu/drm/sti/sti_cursor.c | 32 +++- drivers/gpu/drm/sti/sti_drv.c| 137 --- drivers/gpu/drm/sti/sti_drv.h| 1 + drivers/gpu/drm/sti/sti_dvo.c| 25 +++ drivers/gpu/drm/sti/sti_gdp.c| 32 +++- drivers/gpu/drm/sti/sti_hda.c| 26 +++ drivers/gpu/drm/sti/sti_hdmi.c | 40 +- drivers/gpu/drm/sti/sti_hqvdp.c | 32 +++- drivers/gpu/drm/sti/sti_mixer.c | 5 +- drivers/gpu/drm/sti/sti_mixer.h | 2 + drivers/gpu/drm/sti/sti_plane.c | 24 +- drivers/gpu/drm/sti/sti_plane.h | 7 +- drivers/gpu/drm/sti/sti_tvout.c | 36 +++-- drivers/gpu/drm/sti/sti_vid.c| 5 +- drivers/gpu/drm/sti/sti_vid.h| 2 + include/drm/drm_crtc.h | 79 21 files changed, 511 insertions(+), 145 deletions(-) -- 1.9.1
[PATCH 1/3] RFC: drm: Restrict vblank ioctl to master
Hi, On 21 June 2016 at 14:57, Rainer Hochecker wrote: > Are you saying that this is outdated: > https://wayland.freedesktop.org/faq.html#heading_toc_j_12 > > A more subtle point is that libGL.so includes the GLX symbols, so linking to > that library will pull in all the X dependencies. This means that we can't > link to full GL without pulling in the client side of X, so we're using > GLES2 for now. Longer term, we'll need a way to use full GL under Wayland. Badly worded, really. libGL.so includes the GLX API entrypoints, so your libGL will link to X11. For that reason - and because there's no need for it to use full GL - Weston uses GLES2 for its own composition. For clients, if you don't care about this, then you can use libGL + EGL (this has always worked), or there's also libglvnd's libOpenGL (this is new). Given that, it should be reworded. Cheers, Daniel
[PATCH v4 0/8] iommu/rockchip: Fix bugs and enable on ARM64
Hi Tomasz, On Tue, Jun 21, 2016 at 09:42:16PM +0900, Tomasz Figa wrote: > In simple words, DRM patches depend on IOMMU patches. > > More precisely: The IOMMU patches alone are supposed to not break > anything. Same goes for the first DRM patch (7/8). Only second DRM > patch (8/8) depends on changes introduced by its predecessors. The first DRM patch is 6/7, so it is 7/8 with the iommu dependency, right? Anyway, I think the best is I take the iommu patches when Heiko is ok with them and then the DRM tree can merge that branch in to apply the DRM patches. But first Heiko should have a look at the patches. Thanks, Joerg
[RFC PATCH v2] drm/nouveau/fb/nv50: set DMA mask before mapping scratch page
The 100c08 scratch page is mapped using dma_map_page() before the TTM layer has had a chance to set the DMA mask. This means we are still running with the default of 32 when this code executes, and this causes problems for platforms with no memory below 4 GB (such as AMD Seattle) So move the dma_map_page() to the .init hook, and set the streaming DMA mask based on the MMU subdev parameters before performing the call. Signed-off-by: Ard Biesheuvel --- I am sure there is a much better way to address this, but this fixes the problem I get on AMD Seattle with a GeForce 210 PCIe card: nouveau :02:00.0: enabling device ( -> 0003) nouveau :02:00.0: NVIDIA GT218 (0a8280b1) nouveau :02:00.0: bios: version 70.18.a6.00.00 nouveau :02:00.0: fb ctor failed, -14 nouveau: probe of :02:00.0 failed with error -14 v2: replace incorrect comparison of dma_addr_t type var against NULL drivers/gpu/drm/nouveau/nvkm/subdev/fb/nv50.c | 37 ++-- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/nv50.c b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/nv50.c index 1b5fb02eab2a..033ca0effb7e 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/nv50.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/nv50.c @@ -216,11 +216,30 @@ nv50_fb_init(struct nvkm_fb *base) struct nv50_fb *fb = nv50_fb(base); struct nvkm_device *device = fb->base.subdev.device; + if (!fb->r100c08) { + /* +* We are calling the DMA api way before the TTM layer sets the +* DMA mask based on the MMU subdev parameters. This means we +* are using the default DMA mask of 32, which may cause +* problems on systems with no RAM below the 4 GB mark. So set +* the streaming DMA mask here as well. +*/ + dma_set_mask(device->dev, DMA_BIT_MASK(device->mmu->dma_bits)); + + fb->r100c08 = dma_map_page(device->dev, fb->r100c08_page, 0, + PAGE_SIZE, DMA_BIDIRECTIONAL); + if (dma_mapping_error(device->dev, fb->r100c08)) { + nvkm_warn(&fb->base.subdev, + "dma_map_page() failed on 100c08 page\n"); + } + } + /* Not a clue what this is exactly. Without pointing it at a * scratch page, VRAM->GART blits with M2MF (as in DDX DFS) * cause IOMMU "read from address 0" errors (rh#561267) */ - nvkm_wr32(device, 0x100c08, fb->r100c08 >> 8); + if (fb->r100c08 != DMA_ERROR_CODE) + nvkm_wr32(device, 0x100c08, fb->r100c08 >> 8); /* This is needed to get meaningful information from 100c90 * on traps. No idea what these values mean exactly. */ @@ -233,11 +252,11 @@ nv50_fb_dtor(struct nvkm_fb *base) struct nv50_fb *fb = nv50_fb(base); struct nvkm_device *device = fb->base.subdev.device; - if (fb->r100c08_page) { + if (fb->r100c08 && fb->r100c08 != DMA_ERROR_CODE) dma_unmap_page(device->dev, fb->r100c08, PAGE_SIZE, DMA_BIDIRECTIONAL); - __free_page(fb->r100c08_page); - } + + __free_page(fb->r100c08_page); return fb; } @@ -264,13 +283,9 @@ nv50_fb_new_(const struct nv50_fb_func *func, struct nvkm_device *device, *pfb = &fb->base; fb->r100c08_page = alloc_page(GFP_KERNEL | __GFP_ZERO); - if (fb->r100c08_page) { - fb->r100c08 = dma_map_page(device->dev, fb->r100c08_page, 0, - PAGE_SIZE, DMA_BIDIRECTIONAL); - if (dma_mapping_error(device->dev, fb->r100c08)) - return -EFAULT; - } else { - nvkm_warn(&fb->base.subdev, "failed 100c08 page alloc\n"); + if (!fb->r100c08_page) { + nvkm_error(&fb->base.subdev, "failed 100c08 page alloc\n"); + return -ENOMEM; } return 0; -- 2.7.4
[PATCH v2 7/7] Documentation/DocBook: remove gpu.tmpl
The gpu documentation has now been converted to reStructuredText files under Documentation/gpu. Remove the obsolete DocBook template. Also remove it from MAINTAINERS. Good riddance. Signed-off-by: Jani Nikula --- Documentation/DocBook/Makefile |2 +- Documentation/DocBook/gpu.tmpl | 3528 MAINTAINERS|1 - 3 files changed, 1 insertion(+), 3530 deletions(-) delete mode 100644 Documentation/DocBook/gpu.tmpl diff --git a/Documentation/DocBook/Makefile b/Documentation/DocBook/Makefile index e0c7e1e0590b..f4482f9b221f 100644 --- a/Documentation/DocBook/Makefile +++ b/Documentation/DocBook/Makefile @@ -14,7 +14,7 @@ DOCBOOKS := z8530book.xml device-drivers.xml \ genericirq.xml s390-drivers.xml uio-howto.xml scsi.xml \ 80211.xml debugobjects.xml sh.xml regulator.xml \ alsa-driver-api.xml writing-an-alsa-driver.xml \ - tracepoint.xml gpu.xml media_api.xml w1.xml \ + tracepoint.xml media_api.xml w1.xml \ writing_musb_glue_layer.xml crypto-API.xml iio.xml include Documentation/DocBook/media/Makefile diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl deleted file mode 100644 index d09536c91717.. --- a/Documentation/DocBook/gpu.tmpl +++ /dev/null @@ -1,3528 +0,0 @@ - -http://www.oasis-open.org/docbook/xml/4.1.2/docbookx.dtd"; []> - - - -Linux GPU Driver Developer's Guide - - - - Jesse - Barnes - Initial version - - Intel Corporation - - jesse.barnes at intel.com - - - - - Laurent - Pinchart - Driver internals - - Ideas on board SPRL - - laurent.pinchart at ideasonboard.com - - - - - Daniel - Vetter - Contributions all over the place - - Intel Corporation - - daniel.vetter at ffwll.ch - - - - - Lukas - Wunner - vga_switcheroo documentation - - - lukas at wunner.de - - - - - - - 2008-2009 - 2013-2014 - Intel Corporation - - - 2012 - Laurent Pinchart - - - 2015 - Lukas Wunner - - - - - The contents of this file may be used under the terms of the GNU - General Public License version 2 (the "GPL") as distributed in - the kernel source COPYING file. - - - - - - - 1.0 - 2012-07-13 - LP - Added extensive documentation about driver internals. - - - - 1.1 - 2015-10-11 - LW - Added vga_switcheroo documentation. - - - - - - - - - DRM Core - - - This first part of the GPU Driver Developer's Guide documents core DRM - code, helper libraries for writing drivers and generic userspace - interfaces exposed by DRM drivers. - - - - -Introduction - - The Linux DRM layer contains code intended to support the needs - of complex graphics devices, usually containing programmable - pipelines well suited to 3D graphics acceleration. Graphics - drivers in the kernel may make use of DRM functions to make - tasks like memory management, interrupt handling and DMA easier, - and provide a uniform interface to applications. - - - A note on versions: this guide covers features found in the DRM - tree, including the TTM memory manager, output configuration and - mode setting, and the new vblank internals, in addition to all - the regular features found in current kernels. - - - [Insert diagram of typical DRM stack here] - - -Style Guidelines - - For consistency this documentation uses American English. Abbreviations - are written as all-uppercase, for example: DRM, KMS, IOCTL, CRTC, and so - on. To aid in reading, documentations make full use of the markup - characters kerneldoc provides: @parameter for function parameters, @member - for structure members, &structure to reference structures and - function() for functions. These all get automatically hyperlinked if - kerneldoc for the referenced objects exists. When referencing entries in - function vtables please use ->vfunc(). Note that kerneldoc does - not support referencing struct members directly, so please add a reference - to the vtable struct somewhere in the same paragraph or at least section. - - - Except in special situations (to separate locked from unlocked variants) - locking requirements for functions aren't documented in the kerneldoc. - Instead locking should be check at runtime using e.g. - WARN_ON(!mutex_is_locked(...));. Since it's much easier to - ignore documentation than runtime no
[PATCH v2 6/7] Documentation/gpu: split up mm, kms and kms-helpers from internals
Make the documents more manageable. Signed-off-by: Jani Nikula --- Documentation/gpu/drm-internals.rst | 1379 + Documentation/gpu/drm-kms-helpers.rst | 260 +++ Documentation/gpu/drm-kms.rst | 656 Documentation/gpu/drm-mm.rst | 454 +++ Documentation/gpu/index.rst |3 + 5 files changed, 1379 insertions(+), 1373 deletions(-) create mode 100644 Documentation/gpu/drm-kms-helpers.rst create mode 100644 Documentation/gpu/drm-kms.rst create mode 100644 Documentation/gpu/drm-mm.rst diff --git a/Documentation/gpu/drm-internals.rst b/Documentation/gpu/drm-internals.rst index a7f117653033..4f7176576feb 100644 --- a/Documentation/gpu/drm-internals.rst +++ b/Documentation/gpu/drm-internals.rst @@ -40,7 +40,7 @@ Driver Information -- Driver Features -^^^ +~~~ Drivers inform the DRM core about their requirements and supported features by setting appropriate flags in the driver_features field. @@ -96,7 +96,7 @@ DRIVER_ATOMIC modeset objects with driver specific properties. Major, Minor and Patchlevel -^^^ +~~~ int major; int minor; int patchlevel; The DRM core identifies driver versions by a major, minor and patch @@ -114,7 +114,7 @@ return an error. Otherwise the driver's set_version() method will be called with the requested version. Name, Description and Date -^^ +~~ char \*name; char \*desc; char \*date; The driver name is printed to the kernel log at initialization time, @@ -144,7 +144,7 @@ Driver Load --- IRQ Registration - + The DRM core tries to facilitate IRQ handler registration and unregistration by providing :c:func:`drm_irq_install()` and @@ -198,7 +198,7 @@ registration of the IRQs, and clear it to 0 after unregistering the IRQs. Memory Manager Initialization -^ +~ Every DRM driver requires a memory manager which must be initialized at load time. DRM currently contains two memory managers, the Translation @@ -207,7 +207,7 @@ document describes the use of the GEM memory manager only. See ? for details. Miscellaneous Device Configuration -^^ +~~ Another task that may be necessary for PCI devices during configuration is mapping the video BIOS. On many devices, the VBIOS describes device @@ -236,1373 +236,6 @@ drivers. .. kernel-doc:: drivers/gpu/drm/drm_platform.c :export: -Memory management -= - -Modern Linux systems require large amount of graphics memory to store -frame buffers, textures, vertices and other graphics-related data. Given -the very dynamic nature of many of that data, managing graphics memory -efficiently is thus crucial for the graphics stack and plays a central -role in the DRM infrastructure. - -The DRM core includes two memory managers, namely Translation Table Maps -(TTM) and Graphics Execution Manager (GEM). TTM was the first DRM memory -manager to be developed and tried to be a one-size-fits-them all -solution. It provides a single userspace API to accommodate the need of -all hardware, supporting both Unified Memory Architecture (UMA) devices -and devices with dedicated video RAM (i.e. most discrete video cards). -This resulted in a large, complex piece of code that turned out to be -hard to use for driver development. - -GEM started as an Intel-sponsored project in reaction to TTM's -complexity. Its design philosophy is completely different: instead of -providing a solution to every graphics memory-related problems, GEM -identified common code between drivers and created a support library to -share it. GEM has simpler initialization and execution requirements than -TTM, but has no video RAM management capabilities and is thus limited to -UMA devices. - -The Translation Table Manager (TTM) - -TTM design background and information belongs here. - -TTM initialization -^^ - -**Warning** - -This section is outdated. - -Drivers wishing to support TTM must fill out a drm_bo_driver -structure. The structure contains several fields with function pointers -for initializing the TTM, allocating and freeing memory, waiting for -command completion and fence synchronization, and memory migration. See -the radeon_ttm.c file for an example of usage. - -The ttm_global_reference structure is made up of several fields: - -:: - - struct ttm_global_reference { - enum ttm_global_types global_type; - size_t size; - void *object; - int (*init) (struct ttm_global_reference *); - void (*release) (struct ttm_global_reference *); - }; - - -There should be one global
[PATCH v2 5/7] Documentation/gpu: convert the KMS properties table to CSV
Pandoc really did a bad job of converting the big KMS properties table to RST. Instead, put the properties into a separate plain text CSV file, and include it in the RST file. The generated output isn't very pretty, but at least the information is there, and it's stored in a format that's easier to process and improve upon at a later time. The CSV file was generated by copy-pasting the table from the HTML generated by the DocBook toolchain into LibreOffice Calc, and then saved as CSV, unmodified. Signed-off-by: Jani Nikula --- Documentation/gpu/drm-internals.rst | 260 +-- Documentation/gpu/kms-properties.csv | 128 + 2 files changed, 131 insertions(+), 257 deletions(-) create mode 100644 Documentation/gpu/kms-properties.csv diff --git a/Documentation/gpu/drm-internals.rst b/Documentation/gpu/drm-internals.rst index ee01a4fbd657..a7f117653033 100644 --- a/Documentation/gpu/drm-internals.rst +++ b/Documentation/gpu/drm-internals.rst @@ -1532,263 +1532,9 @@ Existing KMS Properties The following table gives description of drm properties exposed by various modules/drivers. -+---+--++---++---++ -| Owner Module/Drivers | Group| Property Name | Type | Property Values | Object attached | Description/Restrictions | -+---+--++---++---++ -| DRM | Generic | ârotationâ | BITMASK | { 0, "rotate-0" }, { 1, "rotate-90" }, { 2, "rotate-180" }, { 3, "rotate-270" }, { 4, "reflect-x" }, { 5, "reflect-y" } | CRTC, Plane | rotate-(degrees) rotates the image by the specified amount in degrees in counter clockwise direction. reflect-x and reflect-y reflects the image along the specified
[PATCH v2 4/7] Documentation/gpu: use recommended order of heading markers
While splitting the document up, the headings "shifted" from what pandoc generated. Use the following order for headings for consistency: == Document title == First = Second -- Third ~ Leave the lower level headings as they are; I think those are less important. Although RST doesn't mandate a specific order ("Rather than imposing a fixed number and order of section title adornment styles, the order enforced will be the order as encountered."), having the higher levels the same overall makes it easier to follow the documents. [I'm sort of kind of writing the recommendation for docs-next in the mean time, but this order seems sensible, and is what I'm proposing.] Signed-off-by: Jani Nikula --- Documentation/gpu/drm-internals.rst | 127 ++-- Documentation/gpu/drm-uapi.rst | 5 +- Documentation/gpu/i915.rst | 81 +++ Documentation/gpu/introduction.rst | 3 +- 4 files changed, 110 insertions(+), 106 deletions(-) diff --git a/Documentation/gpu/drm-internals.rst b/Documentation/gpu/drm-internals.rst index 8b8257891396..ee01a4fbd657 100644 --- a/Documentation/gpu/drm-internals.rst +++ b/Documentation/gpu/drm-internals.rst @@ -1,3 +1,4 @@ += DRM Internals = @@ -18,7 +19,7 @@ management, command submission & fencing, suspend/resume support, and DMA services. Driver Initialization -- += At the core of every DRM driver is a :c:type:`struct drm_driver ` structure. Drivers typically statically initialize @@ -36,7 +37,7 @@ then describe individual operations in details as they get used in later sections. Driver Information -~~ +-- Driver Features ^^^ @@ -131,7 +132,7 @@ kernel log at initialization time and passes it to userspace through the DRM_IOCTL_VERSION ioctl. Device Instance and Driver Handling -~~~ +--- .. kernel-doc:: drivers/gpu/drm/drm_drv.c :doc: driver instance overview @@ -140,7 +141,7 @@ Device Instance and Driver Handling :export: Driver Load -~~~ +--- IRQ Registration @@ -221,7 +222,7 @@ other BARs, so leaving it mapped could cause undesired behaviour like hangs or memory corruption. Bus-specific Device Registration and PCI Support - + A number of functions are provided to help with device registration. The functions deal with PCI and platform devices respectively and are only @@ -236,7 +237,7 @@ drivers. :export: Memory management -- += Modern Linux systems require large amount of graphics memory to store frame buffers, textures, vertices and other graphics-related data. Given @@ -262,7 +263,7 @@ TTM, but has no video RAM management capabilities and is thus limited to UMA devices. The Translation Table Manager (TTM) -~~~ +--- TTM design background and information belongs here. @@ -313,7 +314,7 @@ object, ttm_global_item_ref() is used to create an initial reference count for the TTM, which will call your initialization function. The Graphics Execution Manager (GEM) - + The GEM design approach has resulted in a memory manager that doesn't provide full coverage of all (or even all common) use cases in its @@ -576,7 +577,7 @@ available to the client. Such resource management should be abstracted from the client in libdrm. GEM Function Reference -~~ +-- .. kernel-doc:: drivers/gpu/drm/drm_gem.c :export: @@ -585,7 +586,7 @@ GEM Function Reference :internal: VMA Offset Manager -~~ +-- .. kernel-doc:: drivers/gpu/drm/drm_vma_manager.c :doc: vma offset manager @@ -597,7 +598,7 @@ VMA Offset Manager :internal: PRIME Buffer Sharing - + PRIME is the cross device buffer sharing framework in drm, originally created for the OPTIMUS range of multi-gpu platforms. To userspace PRIME @@ -648,13 +649,13 @@ PRIME Helper Functions :doc: PRIME Helpers PRIME Function References -~ +- .. kernel-doc:: drivers/gpu/drm/drm_prime.c :export: DRM MM Range Allocator -~~ +-- Overview @@ -669,7 +670,7 @@ LRU Scan/Eviction Support :doc: lru scan roaster DRM MM Range Allocator Function References -~~ +-- .. kernel-doc:: drivers/gpu/drm/drm_mm.c :export: @@ -678,7 +679,7 @@ DRM MM Range Allocator Function References :internal: CMA Helper Functions Re
[PATCH v2 3/7] MAINTAINERS: add Documentation/gpu and Documentation/gpu/i915.rst
We'll want to keep an eye on what's going on in these files. Signed-off-by: Jani Nikula --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index cb88f724e07c..ce9c23dd02c6 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3857,6 +3857,7 @@ F:drivers/gpu/vga/ F: Documentation/devicetree/bindings/display/ F: Documentation/devicetree/bindings/gpu/ F: Documentation/devicetree/bindings/video/ +F: Documentation/gpu/ F: Documentation/DocBook/gpu.* F: include/drm/ F: include/uapi/drm/ @@ -3909,6 +3910,7 @@ S:Supported F: drivers/gpu/drm/i915/ F: include/drm/i915* F: include/uapi/drm/i915_drm.h +F: Documentation/gpu/i915.rst DRM DRIVERS FOR ATMEL HLCDC M: Boris Brezillon -- 2.1.4
[PATCH v2 2/7] Documentation/gpu: split up the gpu documentation
Make the gpu documentation easier to manage by splitting to separate files. Again, this is just the split, no real edits. Signed-off-by: Jani Nikula --- Documentation/gpu/drm-internals.rst | 1998 ++ Documentation/gpu/drm-uapi.rst | 91 ++ Documentation/gpu/i915.rst | 346 + Documentation/gpu/index.rst | 2602 +- Documentation/gpu/introduction.rst | 50 + Documentation/gpu/vga-switcheroo.rst | 102 ++ 6 files changed, 2593 insertions(+), 2596 deletions(-) create mode 100644 Documentation/gpu/drm-internals.rst create mode 100644 Documentation/gpu/drm-uapi.rst create mode 100644 Documentation/gpu/i915.rst create mode 100644 Documentation/gpu/introduction.rst create mode 100644 Documentation/gpu/vga-switcheroo.rst diff --git a/Documentation/gpu/drm-internals.rst b/Documentation/gpu/drm-internals.rst new file mode 100644 index ..8b8257891396 --- /dev/null +++ b/Documentation/gpu/drm-internals.rst @@ -0,0 +1,1998 @@ +DRM Internals += + +This chapter documents DRM internals relevant to driver authors and +developers working to add support for the latest features to existing +drivers. + +First, we go over some typical driver initialization requirements, like +setting up command buffers, creating an initial output configuration, +and initializing core services. Subsequent sections cover core internals +in more detail, providing implementation notes and examples. + +The DRM layer provides several services to graphics drivers, many of +them driven by the application interfaces it provides through libdrm, +the library that wraps most of the DRM ioctls. These include vblank +event handling, memory management, output management, framebuffer +management, command submission & fencing, suspend/resume support, and +DMA services. + +Driver Initialization +- + +At the core of every DRM driver is a :c:type:`struct drm_driver +` structure. Drivers typically statically initialize +a drm_driver structure, and then pass it to +:c:func:`drm_dev_alloc()` to allocate a device instance. After the +device instance is fully initialized it can be registered (which makes +it accessible from userspace) using :c:func:`drm_dev_register()`. + +The :c:type:`struct drm_driver ` structure +contains static information that describes the driver and features it +supports, and pointers to methods that the DRM core will call to +implement the DRM API. We will first go through the :c:type:`struct +drm_driver ` static information fields, and will +then describe individual operations in details as they get used in later +sections. + +Driver Information +~~ + +Driver Features +^^^ + +Drivers inform the DRM core about their requirements and supported +features by setting appropriate flags in the driver_features field. +Since those flags influence the DRM core behaviour since registration +time, most of them must be set to registering the :c:type:`struct +drm_driver ` instance. + +u32 driver_features; + +DRIVER_USE_AGP +Driver uses AGP interface, the DRM core will manage AGP resources. + +DRIVER_REQUIRE_AGP +Driver needs AGP interface to function. AGP initialization failure +will become a fatal error. + +DRIVER_PCI_DMA +Driver is capable of PCI DMA, mapping of PCI DMA buffers to +userspace will be enabled. Deprecated. + +DRIVER_SG +Driver can perform scatter/gather DMA, allocation and mapping of +scatter/gather buffers will be enabled. Deprecated. + +DRIVER_HAVE_DMA +Driver supports DMA, the userspace DMA API will be supported. +Deprecated. + +DRIVER_HAVE_IRQ; DRIVER_IRQ_SHARED +DRIVER_HAVE_IRQ indicates whether the driver has an IRQ handler +managed by the DRM Core. The core will support simple IRQ handler +installation when the flag is set. The installation process is +described in ?. + +DRIVER_IRQ_SHARED indicates whether the device & handler support +shared IRQs (note that this is required of PCI drivers). + +DRIVER_GEM +Driver use the GEM memory manager. + +DRIVER_MODESET +Driver supports mode setting interfaces (KMS). + +DRIVER_PRIME +Driver implements DRM PRIME buffer sharing. + +DRIVER_RENDER +Driver supports dedicated render nodes. + +DRIVER_ATOMIC +Driver supports atomic properties. In this case the driver must +implement appropriate obj->atomic_get_property() vfuncs for any +modeset objects with driver specific properties. + +Major, Minor and Patchlevel +^^^ + +int major; int minor; int patchlevel; +The DRM core identifies driver versions by a major, minor and patch +level triplet. The information is printed to the kernel log at +initialization time and passed to userspace through the +DRM_IOCTL_VERSION ioctl. + +The major and minor numbers are also used to verify the requested driver +API version passed to DRM_IOCTL_SET_VERSION. When the driver API +changes bet
[PATCH v2 1/7] Documentation/gpu: add new gpu.rst converted from DocBook gpu.tmpl
This is the first step towards converting the DocBook gpu.tmpl to Sphinx and reStructuredText, the new kernel documentation tool and markup. Use Jon's "cheesy conversion script" in Documentation/sphinx/tmplcvt to do the rough conversion. Do the manual edits in follow-up patches. Add a new Documentation/gpu directories for the graphics related documentation. (Hooray, now we can have directories based on topics rather than tools under Documentation.) We also won't remove the DocBook gpu.tmpl yet so it's easier to build both and compare the results for parity. Signed-off-by: Jani Nikula --- Documentation/gpu/index.rst | 2601 +++ Documentation/index.rst |1 + 2 files changed, 2602 insertions(+) create mode 100644 Documentation/gpu/index.rst diff --git a/Documentation/gpu/index.rst b/Documentation/gpu/index.rst new file mode 100644 index ..795a5ec309e2 --- /dev/null +++ b/Documentation/gpu/index.rst @@ -0,0 +1,2601 @@ +== +Linux GPU Driver Developer's Guide +== + +:Author: Jesse Barnes Initial version +:Author: Laurent Pinchart Driver internals +:Author: Daniel Vetter Contributions all over the place +:Author: Lukas Wunner vga_switcheroo documentation +:Date: 2015-10-11 + +This first part of the GPU Driver Developer's Guide documents core DRM +code, helper libraries for writing drivers and generic userspace +interfaces exposed by DRM drivers. + +Introduction + + +The Linux DRM layer contains code intended to support the needs of +complex graphics devices, usually containing programmable pipelines well +suited to 3D graphics acceleration. Graphics drivers in the kernel may +make use of DRM functions to make tasks like memory management, +interrupt handling and DMA easier, and provide a uniform interface to +applications. + +A note on versions: this guide covers features found in the DRM tree, +including the TTM memory manager, output configuration and mode setting, +and the new vblank internals, in addition to all the regular features +found in current kernels. + +[Insert diagram of typical DRM stack here] + +Style Guidelines + + +For consistency this documentation uses American English. Abbreviations +are written as all-uppercase, for example: DRM, KMS, IOCTL, CRTC, and so +on. To aid in reading, documentations make full use of the markup +characters kerneldoc provides: @parameter for function parameters, + at member for structure members, &structure to reference structures and +function() for functions. These all get automatically hyperlinked if +kerneldoc for the referenced objects exists. When referencing entries in +function vtables please use ->vfunc(). Note that kerneldoc does not +support referencing struct members directly, so please add a reference +to the vtable struct somewhere in the same paragraph or at least +section. + +Except in special situations (to separate locked from unlocked variants) +locking requirements for functions aren't documented in the kerneldoc. +Instead locking should be check at runtime using e.g. +``WARN_ON(!mutex_is_locked(...));``. Since it's much easier to ignore +documentation than runtime noise this provides more value. And on top of +that runtime checks do need to be updated when the locking rules change, +increasing the chances that they're correct. Within the documentation +the locking rules should be explained in the relevant structures: Either +in the comment for the lock explaining what it protects, or data fields +need a note about which lock protects them, or both. + +Functions which have a non-\ ``void`` return value should have a section +called "Returns" explaining the expected return values in different +cases and their meanings. Currently there's no consensus whether that +section name should be all upper-case or not, and whether it should end +in a colon or not. Go with the file-local style. Other common section +names are "Notes" with information for dangerous or tricky corner cases, +and "FIXME" where the interface could be cleaned up. + +DRM Internals += + +This chapter documents DRM internals relevant to driver authors and +developers working to add support for the latest features to existing +drivers. + +First, we go over some typical driver initialization requirements, like +setting up command buffers, creating an initial output configuration, +and initializing core services. Subsequent sections cover core internals +in more detail, providing implementation notes and examples. + +The DRM layer provides several services to graphics drivers, many of +them driven by the application interfaces it provides through libdrm, +the library that wraps most of the DRM ioctls. These include vblank +event handling, memory management, output management, framebuffer +management, command submission & fencing, suspend/resume support, and +DMA services. + +Driver Initialization +-
[PATCH v2 0/7] Documentation: convert DocBook gpu.tmpl to reStructuredText
Take two of [1], see the cover letter there. I've renamed drm-userland-interfaces.rst to drm-uapi.rst and added a further split-up patch of drm internals, extracting mm, kms and kms helpers to separate documents. BR, Jani. [1] http://mid.gmane.org/cover.1466434348.git.jani.nikula at intel.com Jani Nikula (7): Documentation/gpu: add new gpu.rst converted from DocBook gpu.tmpl Documentation/gpu: split up the gpu documentation MAINTAINERS: add Documentation/gpu and Documentation/gpu/i915.rst Documentation/gpu: use recommended order of heading markers Documentation/gpu: convert the KMS properties table to CSV Documentation/gpu: split up mm, kms and kms-helpers from internals Documentation/DocBook: remove gpu.tmpl Documentation/DocBook/Makefile|2 +- Documentation/DocBook/gpu.tmpl| 3528 - Documentation/gpu/drm-internals.rst | 378 Documentation/gpu/drm-kms-helpers.rst | 260 +++ Documentation/gpu/drm-kms.rst | 656 ++ Documentation/gpu/drm-mm.rst | 454 + Documentation/gpu/drm-uapi.rst| 92 + Documentation/gpu/i915.rst| 347 Documentation/gpu/index.rst | 14 + Documentation/gpu/introduction.rst| 51 + Documentation/gpu/kms-properties.csv | 128 ++ Documentation/gpu/vga-switcheroo.rst | 102 + Documentation/index.rst |1 + MAINTAINERS |3 +- 14 files changed, 2486 insertions(+), 3530 deletions(-) delete mode 100644 Documentation/DocBook/gpu.tmpl create mode 100644 Documentation/gpu/drm-internals.rst create mode 100644 Documentation/gpu/drm-kms-helpers.rst create mode 100644 Documentation/gpu/drm-kms.rst create mode 100644 Documentation/gpu/drm-mm.rst create mode 100644 Documentation/gpu/drm-uapi.rst create mode 100644 Documentation/gpu/i915.rst create mode 100644 Documentation/gpu/index.rst create mode 100644 Documentation/gpu/introduction.rst create mode 100644 Documentation/gpu/kms-properties.csv create mode 100644 Documentation/gpu/vga-switcheroo.rst -- 2.1.4
[PATCH] drm: Drop mode_config.mutex from get_resources ioctl
The only thing this protected is the connector_list, which is now protected differently. v2: Also remove comment (Chris). Cc: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 13c3bacc6b51..f8931b82865a 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1904,9 +1904,6 @@ int drm_mode_getresources(struct drm_device *dev, void *data, card_res->count_fbs = fb_count; mutex_unlock(&file_priv->fbs_lock); - /* mode_config.mutex protects the connector list against e.g. DP MST -* connector hot-adding. CRTC/Plane lists are invariant. */ - mutex_lock(&dev->mode_config.mutex); card_res->max_height = dev->mode_config.max_height; card_res->min_height = dev->mode_config.min_height; card_res->max_width = dev->mode_config.max_width; @@ -1961,7 +1958,6 @@ int drm_mode_getresources(struct drm_device *dev, void *data, card_res->count_connectors = connector_count; out: - mutex_unlock(&dev->mode_config.mutex); return ret; } -- 2.8.1
[PATCH] drm: Don't compute obj counts expensively in get_resources
Looping when we keep track of this is silly. Only thing we have to be careful is with sampling the connector count. To avoid inconsisten results due to gcc re-computing this, use READ_ONCE. And to avoid surprising userspace, make sure we don't copy more connectors than planned, and report the actual number of connectors copied. That way any racing hot-add/remove will be handled. v2: Actually try to not blow up, somehow I lost the hunk that checks we don't copy too much. Noticed by Chris. Cc: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c | 19 +++ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 28c109ff7330..59c5261a309c 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1842,10 +1842,10 @@ int drm_mode_getresources(struct drm_device *dev, void *data, struct drm_crtc *crtc; struct drm_encoder *encoder; int ret = 0; - int connector_count = 0; - int crtc_count = 0; + int connector_count = READ_ONCE(dev->mode_config.num_connector); + int crtc_count = dev->mode_config.num_crtc; int fb_count = 0; - int encoder_count = 0; + int encoder_count = dev->mode_config.num_encoder; int copied = 0; uint32_t __user *fb_id; uint32_t __user *crtc_id; @@ -1883,15 +1883,6 @@ int drm_mode_getresources(struct drm_device *dev, void *data, /* mode_config.mutex protects the connector list against e.g. DP MST * connector hot-adding. CRTC/Plane lists are invariant. */ mutex_lock(&dev->mode_config.mutex); - drm_for_each_crtc(crtc, dev) - crtc_count++; - - drm_for_each_connector(connector, dev) - connector_count++; - - drm_for_each_encoder(encoder, dev) - encoder_count++; - card_res->max_height = dev->mode_config.max_height; card_res->min_height = dev->mode_config.min_height; card_res->max_width = dev->mode_config.max_width; @@ -1931,6 +1922,9 @@ int drm_mode_getresources(struct drm_device *dev, void *data, copied = 0; connector_id = (uint32_t __user *)(unsigned long)card_res->connector_id_ptr; drm_for_each_connector(connector, dev) { + if (copied >= connector_count) + break; + if (put_user(connector->base.id, connector_id + copied)) { ret = -EFAULT; @@ -1938,6 +1932,7 @@ int drm_mode_getresources(struct drm_device *dev, void *data, } copied++; } + connector_count = copied; } card_res->count_connectors = connector_count; -- 2.8.1
[PULL] drm: atmel-hlcdc: Fixes for 4.7-rc5
Hi Dave, Here is a PR containing two fixes for 4.7. Regards, Boris The following changes since commit 58a2ab3af722550b2e4e8155eb08660e16c20ee6: drm: atmel-hlcdc: fix a NULL check (2016-06-01 15:59:36 +0200) are available in the git repository at: git at github.com:bbrezillon/linux-at91.git tags/drm-atmel-hlcdc-fixes/for-4.7-rc5 for you to fetch changes up to 0b1e1eb76220afa043b2733dfe61f5927cf0e458: drm: atmel-hlcdc: Fix OF graph parsing (2016-06-21 14:15:45 +0200) Two bug fixes for the atmel-hlcdc driver. Boris Brezillon (2): drm: atmel-hlcdc: actually disable scaling when no scaling is required drm: atmel-hlcdc: Fix OF graph parsing drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 10 ++ drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 2 ++ 2 files changed, 8 insertions(+), 4 deletions(-)
[PATCH 02/10] drm: Don't compute obj counts expensively in get_resources
On Tue, Jun 21, 2016 at 10:48:08AM +0100, Chris Wilson wrote: > On Tue, Jun 21, 2016 at 11:10:27AM +0200, Daniel Vetter wrote: > > And to avoid surprising userspace, make sure we don't copy more > > connectors than planned, and report the actual number of connectors > > copied. That way any racing hot-add/remove will be handled. > > > @@ -1938,6 +1929,7 @@ int drm_mode_getresources(struct drm_device *dev, > > void *data, > > } > > copied++; > > } > > + connector_count = copied; > > You forgot to actually make sure we don't copy more conectors than > planned. Indeed, must have lost that hunk somewhere. At least I'm sure I've typed it ;-) Will resend. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH] drm: Refactor drop/set master code a bit
File open/set_maseter ioctl and file close/drop_master ioctl share the same master handling code. Extract it. Note that vmwgfx's master_set callback needs to know whether the master is a new one or has been used already, so thread this through. On the close/drop side a similar parameter existed, but wasnt used. Drop it to simplify the flow. v2: Try to make it not leak so much (Emil). v3: Send out the right version ... Cc: Emil Velikov Cc: Chris Wilson Cc: Thomas Hellstrom Reviewed-by: Chris Wilson Reviewed-by: Emil Velikov Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_auth.c | 80 +++-- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 3 +- include/drm/drmP.h | 3 +- 3 files changed, 44 insertions(+), 42 deletions(-) diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index 6bba6b9e9dcc..2794a4f3a105 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -110,6 +110,24 @@ static struct drm_master *drm_master_create(struct drm_device *dev) return master; } +static int drm_set_master(struct drm_device *dev, struct drm_file *fpriv, + bool new_master) +{ + int ret = 0; + + dev->master = drm_master_get(fpriv->master); + fpriv->is_master = 1; + if (dev->driver->master_set) { + ret = dev->driver->master_set(dev, fpriv, new_master); + if (unlikely(ret != 0)) { + fpriv->is_master = 0; + drm_master_put(&dev->master); + } + } + + return ret; +} + /* * drm_new_set_master - Allocate a new master object and become master for the * associated master realm. @@ -127,37 +145,32 @@ static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv) lockdep_assert_held_once(&dev->master_mutex); - /* create a new master */ - dev->master = drm_master_create(dev); - if (!dev->master) - return -ENOMEM; - - /* take another reference for the copy in the local file priv */ old_master = fpriv->master; - fpriv->master = drm_master_get(dev->master); + fpriv->master = drm_master_create(dev); + if (!fpriv->master) { + fpriv->master = old_master; + return -ENOMEM; + } if (dev->driver->master_create) { ret = dev->driver->master_create(dev, fpriv->master); if (ret) goto out_err; } - if (dev->driver->master_set) { - ret = dev->driver->master_set(dev, fpriv, true); - if (ret) - goto out_err; - } - - fpriv->is_master = 1; fpriv->allowed_master = 1; fpriv->authenticated = 1; + + ret = drm_set_master(dev, fpriv, true); + if (ret) + goto out_err; + if (old_master) drm_master_put(&old_master); return 0; out_err: - /* drop both references and restore old master on failure */ - drm_master_put(&dev->master); + /* drop references and restore old master on failure */ drm_master_put(&fpriv->master); fpriv->master = old_master; @@ -188,21 +201,21 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data, goto out_unlock; } - dev->master = drm_master_get(file_priv->master); - file_priv->is_master = 1; - if (dev->driver->master_set) { - ret = dev->driver->master_set(dev, file_priv, false); - if (unlikely(ret != 0)) { - file_priv->is_master = 0; - drm_master_put(&dev->master); - } - } - + ret = drm_set_master(dev, file_priv, false); out_unlock: mutex_unlock(&dev->master_mutex); return ret; } +static void drm_drop_master(struct drm_device *dev, + struct drm_file *fpriv) +{ + if (dev->driver->master_drop) + dev->driver->master_drop(dev, fpriv); + drm_master_put(&dev->master); + fpriv->is_master = 0; +} + int drm_dropmaster_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) { @@ -216,11 +229,7 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data, goto out_unlock; ret = 0; - if (dev->driver->master_drop) - dev->driver->master_drop(dev, file_priv, false); - drm_master_put(&dev->master); - file_priv->is_master = 0; - + drm_drop_master(dev, file_priv); out_unlock: mutex_unlock(&dev->master_mutex); return ret; @@ -271,17 +280,12 @@ void drm_master_release(struct drm_file *file_priv) mutex_unlock(&dev->struct_mutex); } - if (dev->master == file_priv->master) { - /* drop the reference held my the minor */ - if (dev->driver->mast
[PATCH v3 1/3] drm: Add callbacks for late registering
On Tue, Jun 21, 2016 at 03:09:38PM +0200, Benjamin Gaignard wrote: > Like what has been done for connectors add callbacks on encoder, > crtc and plane to let driver do actions after drm device registration. > > Correspondingly, add callbacks called before unregister drm device. > > version 2: > add drm_modeset_register_all() and drm_modeset_unregister_all() > to centralize all calls > > version 3: > in error case unwind registers in drm_modeset_register_all > fix uninitialed return value > inverse order of unregistration in drm_modeset_unregister_all > > Signed-off-by: Benjamin Gaignard > --- > drivers/gpu/drm/drm_crtc.c | 132 > + > drivers/gpu/drm/drm_drv.c | 4 +- > include/drm/drm_crtc.h | 79 +++ > 3 files changed, 213 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index e7c862b..c078bc4 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -608,6 +608,31 @@ static unsigned int drm_num_crtcs(struct drm_device *dev) > return num; > } > > +static int drm_crtc_register_all(struct drm_device *dev) > +{ > + struct drm_crtc *crtc; > + int ret = 0; > + > + drm_for_each_crtc(crtc, dev) { > + if (crtc->funcs->late_register) > + ret = crtc->funcs->late_register(crtc); > + if (ret) What happened to the unwind here? > + return ret; > + } > + > + return 0; > +} > +/** > + * drm_modeset_register_all - do late registration > + * @dev: drm device > + * > + * This function call late_register callback for all planes, > + * crcts, encoders and connectors > + * > + * Returns: > + * Zero on success, erro code on failure > + */ > +int drm_modeset_register_all(struct drm_device *dev) > +{ > + int ret; > + > + ret = drm_plane_register_all(dev); > + if (ret) > + goto err_plane; > + > + ret = drm_crtc_register_all(dev); > + if (ret) > + goto err_crtc; > + > + ret = drm_encoder_register_all(dev); > + if (ret) > + goto err_encoder; > + > + ret = drm_connector_register_all(dev); > + if (ret) > + goto err_connector; > + > + return 0; > + > +err_connector: > + drm_encoder_unregister_all(dev); The name here should be what we are about to free. That just makes it a bit easier to change the sequence later (if adding new stags). Looks good enough though, Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre
[PATCH v4 4/4] drm/i915: Enable polling when we don't have hpd
Unfortunately, there's two situations where we lose hpd right now: - Runtime suspend - When we've shut off all of the power wells on Valleyview/Cherryview While it would be nice if this didn't cause issues, this has the ability to get us in some awkward states where a user won't be able to get their display to turn on. For instance; if we boot a Valleyview system without any monitors connected, it won't need any of it's power wells and thus shut them off. Since this causes us to lose HPD, this means that unless the user knows how to ssh into their machine and do a manual reprobe for monitors, none of the monitors they connect after booting will actually work. Eventually we should come up with a better fix then having to enable polling for this, since this makes rpm a lot less useful, but for now the infrastructure in i915 just isn't there yet to get hpd in these situations. Changes since v1: - Add comment explaining the addition of the if (!mode_config->poll_running) in intel_hpd_init() - Remove unneeded if (!dev->mode_config.poll_enabled) in i915_hpd_poll_init_work() - Call to drm_helper_hpd_irq_event() after we disable polling - Add cancel_work_sync() call to intel_hpd_cancel_work() Changes since v2: - Apparently dev->mode_config.poll_running doesn't actually reflect whether or not a poll is currently in progress, and is actually used for dynamic module paramter enabling/disabling. So now we instead keep track of our own poll_running variable in dev_priv->hotplug - Clean i915_hpd_poll_init_work() a little bit Changes since v3: - Remove the now-redundant connector loop in intel_hpd_init(), just rely on intel_hpd_poll_enable() for setting connector->polled correctly on each connector - Get rid of poll_running - Don't assign enabled in i915_hpd_poll_init_work before we actually lock dev->mode_config.mutex - Wrap enabled assignment in i915_hpd_poll_init_work() in READ_ONCE() for doc purposes - Do the same for dev_priv->hotplug.poll_enabled with WRITE_ONCE in intel_hpd_poll_enable() - Add some comments about racing not mattering in intel_hpd_poll_enable Cc: stable at vger.kernel.org Cc: Ville Syrjälä Cc: Daniel Vetter Signed-off-by: Lyude --- drivers/gpu/drm/i915/i915_drv.c | 7 ++- drivers/gpu/drm/i915/i915_drv.h | 3 ++ drivers/gpu/drm/i915/intel_drv.h| 2 + drivers/gpu/drm/i915/intel_hotplug.c| 76 ++--- drivers/gpu/drm/i915/intel_runtime_pm.c | 3 ++ 5 files changed, 75 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 3eb47fb..5381d9e 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1612,6 +1612,9 @@ static int intel_runtime_suspend(struct device *device) assert_forcewakes_inactive(dev_priv); + if (!IS_VALLEYVIEW(dev_priv) || !IS_CHERRYVIEW(dev_priv)) + intel_hpd_poll_enable(dev_priv, true); + DRM_DEBUG_KMS("Device suspended\n"); return 0; } @@ -1667,8 +1670,10 @@ static int intel_runtime_resume(struct device *device) * power well, so hpd is reinitialized from there. For * everyone else do it here. */ - if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv)) + if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv)) { intel_hpd_init(dev_priv); + intel_hpd_poll_enable(dev_priv, false); + } intel_enable_gt_powersave(dev_priv); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 239a3ec..944c0d0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -283,6 +283,9 @@ struct i915_hotplug { u32 short_port_mask; struct work_struct dig_port_work; + struct work_struct poll_enable_work; + bool poll_enabled; + /* * if we get a HPD irq from DP and a HPD irq from non-DP * the non-DP HPD could block the workqueue on a mode config diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index dcbfdde..cedac13 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1385,6 +1385,8 @@ int intel_dsi_dcs_init_backlight_funcs(struct intel_connector *intel_connector); /* intel_dvo.c */ void intel_dvo_init(struct drm_device *dev); +/* intel_hotplug.c */ +void intel_hpd_poll_enable(struct drm_i915_private *dev_priv, bool enabled); /* legacy fbdev emulation in intel_fbdev.c */ diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c index ec3285f..f23330d 100644 --- a/drivers/gpu/drm/i915/intel_hotplug.c +++ b/drivers/gpu/drm/i915/intel_hotplug.c @@ -455,28 +455,14 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv, */ void intel_hpd_init(struct drm_i915_private *dev_priv) { - struct drm_device *dev = dev_priv->dev; - struct drm_mode_config
[PATCH v2 0/7] Documentation: convert DocBook gpu.tmpl to reStructuredText
On Tue, Jun 21, 2016 at 02:48:56PM +0300, Jani Nikula wrote: > Take two of [1], see the cover letter there. > > I've renamed drm-userland-interfaces.rst to drm-uapi.rst and added a > further split-up patch of drm internals, extracting mm, kms and kms > helpers to separate documents. All applied to drm-misc. Thanks a lot for all your work on improving the doc toolchain! Cheers, Daniel > > BR, > Jani. > > [1] http://mid.gmane.org/cover.1466434348.git.jani.nikula at intel.com > > > Jani Nikula (7): > Documentation/gpu: add new gpu.rst converted from DocBook gpu.tmpl > Documentation/gpu: split up the gpu documentation > MAINTAINERS: add Documentation/gpu and Documentation/gpu/i915.rst > Documentation/gpu: use recommended order of heading markers > Documentation/gpu: convert the KMS properties table to CSV > Documentation/gpu: split up mm, kms and kms-helpers from internals > Documentation/DocBook: remove gpu.tmpl > > Documentation/DocBook/Makefile|2 +- > Documentation/DocBook/gpu.tmpl| 3528 > - > Documentation/gpu/drm-internals.rst | 378 > Documentation/gpu/drm-kms-helpers.rst | 260 +++ > Documentation/gpu/drm-kms.rst | 656 ++ > Documentation/gpu/drm-mm.rst | 454 + > Documentation/gpu/drm-uapi.rst| 92 + > Documentation/gpu/i915.rst| 347 > Documentation/gpu/index.rst | 14 + > Documentation/gpu/introduction.rst| 51 + > Documentation/gpu/kms-properties.csv | 128 ++ > Documentation/gpu/vga-switcheroo.rst | 102 + > Documentation/index.rst |1 + > MAINTAINERS |3 +- > 14 files changed, 2486 insertions(+), 3530 deletions(-) > delete mode 100644 Documentation/DocBook/gpu.tmpl > create mode 100644 Documentation/gpu/drm-internals.rst > create mode 100644 Documentation/gpu/drm-kms-helpers.rst > create mode 100644 Documentation/gpu/drm-kms.rst > create mode 100644 Documentation/gpu/drm-mm.rst > create mode 100644 Documentation/gpu/drm-uapi.rst > create mode 100644 Documentation/gpu/i915.rst > create mode 100644 Documentation/gpu/index.rst > create mode 100644 Documentation/gpu/introduction.rst > create mode 100644 Documentation/gpu/kms-properties.csv > create mode 100644 Documentation/gpu/vga-switcheroo.rst > > -- > 2.1.4 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH] drm: Lobotomize set_busid nonsense for !pci drivers
We already have a fallback in place to fill out the unique from dev->unique, which is set to something reasonable in drm_dev_alloc. Which means we only need to have a special set_busid for pci devices, to be able to care the backwards compat code for drm 1.1 around, which libdrm still needs. While developing and testing this patch things blew up in really interesting ways, and the code is rather confusing in naming things between the kernel code, ioctl #defines and libdrm. For the next brave dragon slayer, document all this madness properly in the userspace interface section of gpu.tmpl. v2: Make drm_dev_set_unique static and update kerneldoc. v3: Entire rewrite, plus document what's going on for posterity in the gpu docbook uapi section. v4: Drop accidental amdgpu hunk (Emil). v5: Drop accidental omapdrm vblank counter change (Emil). Cc: Gustavo Padovan Cc: Emil Velikov Tested-by: Gustavo Padovan (virt_gpu) Reviewed-by: Emil Velikov Signed-off-by: Daniel Vetter --- Documentation/DocBook/gpu.tmpl | 4 ++ drivers/gpu/drm/armada/armada_drv.c | 1 - drivers/gpu/drm/drm_ioctl.c | 58 + drivers/gpu/drm/drm_platform.c | 18 drivers/gpu/drm/etnaviv/etnaviv_drv.c | 1 - drivers/gpu/drm/exynos/exynos_drm_drv.c | 1 - drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 1 - drivers/gpu/drm/imx/imx-drm-core.c | 1 - drivers/gpu/drm/msm/msm_drv.c | 1 - drivers/gpu/drm/nouveau/nouveau_drm.c | 1 - drivers/gpu/drm/omapdrm/omap_drv.c | 1 - drivers/gpu/drm/shmobile/shmob_drm_drv.c| 1 - drivers/gpu/drm/tilcdc/tilcdc_drv.c | 1 - drivers/gpu/drm/virtio/virtgpu_drm_bus.c| 10 - drivers/gpu/drm/virtio/virtgpu_drv.c| 1 - drivers/gpu/drm/virtio/virtgpu_drv.h| 1 - include/drm/drmP.h | 1 - 17 files changed, 62 insertions(+), 41 deletions(-) diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl index a836a8bcd289..94c6bdee8080 100644 --- a/Documentation/DocBook/gpu.tmpl +++ b/Documentation/DocBook/gpu.tmpl @@ -3099,6 +3099,10 @@ int num_ioctls; + libdrm Device Lookup +!Pdrivers/gpu/drm/drm_vma_manager.c getunique and setversion story + + Render nodes DRM core provides multiple character-devices for user-space to use. diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c index cb21c0b6374a..f5ebdd681445 100644 --- a/drivers/gpu/drm/armada/armada_drv.c +++ b/drivers/gpu/drm/armada/armada_drv.c @@ -189,7 +189,6 @@ static struct drm_driver armada_drm_driver = { .load = armada_drm_load, .lastclose = armada_drm_lastclose, .unload = armada_drm_unload, - .set_busid = drm_platform_set_busid, .get_vblank_counter = drm_vblank_no_hw_counter, .enable_vblank = armada_drm_enable_vblank, .disable_vblank = armada_drm_disable_vblank, diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 1fa7619face3..063775f7946e 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -37,6 +37,64 @@ #include #include +/** + * DOC: getunique and setversion story + * + * BEWARE THE DRAGONS! MIND THE TRAPDOORS! + * + * In and attempt to warn anyone else who's trying to figure out what's going + * on here, I'll try to summarize the story. First things first, let's clear up + * the names, because the kernel internals, libdrm and the ioctls are all named + * differently: + * + * - GET_UNIQUE ioctl, implemented by drm_getunique is wrapped up in libdrm + *through the drmGetBusid function. + * - The libdrm drmSetBusid function is backed by the SET_UNIQUE ioctl. All + *that code is nerved in the kernel with drm_invalid_op(). + * - The internal set_busid kernel functions and driver callbacks are + *exclusively use by the SET_VERSION ioctl, because only drm 1.0 (which is + *nerved) allowed userspace to set the busid through the above ioctl. + * - Other ioctls and functions involved are named consistently. + * + * For anyone wondering what's the difference between drm 1.1 and 1.4: Correctly + * handling pci domains in the busid on ppc. Doing this correctly was only + * implemented in libdrm in 2010, hence can't be nerved yet. No one knows what's + * special with drm 1.2 and 1.3. + * + * Now the actual horror story of how device lookup in drm works. At large, + * there's 2 different ways, either by busid, or by device driver name. + * + * Opening by busid is fairly simple: + * + * 1. First call SET_VERSION to make sure pci domains are handled properly. As a + *side-effect this fills out the unique name in the master structure. + * 2. Call GET_UNIQUE to read out the unique name from th
[PATCH] drm: atmel-hlcdc: Fix OF graph parsing
Le 03/06/2016 09:26, Boris Brezillon a écrit : > atmel_hlcdc_create_outputs() iterates over OF graph nodes and releases > the node (using of_node_put()) after each iteration, which is wrong > since for_each_endpoint_of_node() is already taking care of that. > > Move the of_node_put() call in the error path. > > Signed-off-by: Boris Brezillon > Fixes: 17a8e03e7e97 ("drm: atmel-hlcdc: rework the output code to support drm > bridges") Reviewed-by: Nicolas Ferre > --- > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 10 ++ > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c > b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c > index 39802c0..3d34fc4 100644 > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c > @@ -266,9 +266,10 @@ int atmel_hlcdc_create_outputs(struct drm_device *dev) > if (!ret) > ret = atmel_hlcdc_check_endpoint(dev, &ep); > > - of_node_put(ep_np); > - if (ret) > + if (ret) { > + of_node_put(ep_np); > return ret; > + } > } > > for_each_endpoint_of_node(dev->dev->of_node, ep_np) { > @@ -276,9 +277,10 @@ int atmel_hlcdc_create_outputs(struct drm_device *dev) > if (!ret) > ret = atmel_hlcdc_attach_endpoint(dev, &ep); > > - of_node_put(ep_np); > - if (ret) > + if (ret) { > + of_node_put(ep_np); > return ret; > + } > } > > return 0; > -- Nicolas Ferre
[PATCH] drm: Don't compute obj counts expensively in get_resources
On Tue, Jun 21, 2016 at 02:29:48PM +0200, Daniel Vetter wrote: > Looping when we keep track of this is silly. Only thing we have to > be careful is with sampling the connector count. To avoid inconsisten > results due to gcc re-computing this, use READ_ONCE. > > And to avoid surprising userspace, make sure we don't copy more > connectors than planned, and report the actual number of connectors > copied. That way any racing hot-add/remove will be handled. > > v2: Actually try to not blow up, somehow I lost the hunk that checks > we don't copy too much. Noticed by Chris. > > Cc: Chris Wilson > Signed-off-by: Daniel Vetter The defacto API is call once with count_connectors=0 then with again with the previous value of count_connectors. On the second pass, the user is expecting no more then count_connectors to be copied, and is not expecting to be told about new ones. The expected ABI looks unchanged. > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 28c109ff7330..59c5261a309c 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -1842,10 +1842,10 @@ int drm_mode_getresources(struct drm_device *dev, > void *data, > struct drm_crtc *crtc; > struct drm_encoder *encoder; > int ret = 0; > - int connector_count = 0; > - int crtc_count = 0; > + int connector_count = READ_ONCE(dev->mode_config.num_connector); Ok, since in the future num_connector is not guarded by mode_conf.mutex, moving the read underneath that lock doesn't obviate the need for READ_ONCE. Still it reduces the window and how far one has too look back if it were set just before the connector loop. Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre
[Bug 96588] [regression] [amdgpu] Errors scheduling IBs
https://bugs.freedesktop.org/show_bug.cgi?id=96588 --- Comment #12 from Alex Deucher --- (In reply to Christian König from comment #11) > I'm running out of ideas. Does that have any other negative results except > for the error message? > > Alex any idea what else could cause an eviction during switching of the dGPU? powering up/down the dGPU should hit the same code as resume/suspend. Are you seeing similar issues with suspend and resume? Maybe the scheduler isn't getting stopped properly on suspend? We recently fixed something like this for gpu reset. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160621/03aaf363/attachment.html>
[PATCH v4 8/8] iommu/rockchip: Enable Rockchip IOMMU on ARM64
From: Simon Xue This patch makes it possible to compile the rockchip-iommu driver on ARM64, so that it can be used with 64-bit SoCs equipped with this type of IOMMU. Signed-off-by: Simon Xue Signed-off-by: Shunqian Zheng Signed-off-by: Tomasz Figa --- drivers/iommu/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index ad08603..5572621 100644 --- a/drivers/iommu/Kconfig +++ b/drivers/iommu/Kconfig @@ -218,7 +218,7 @@ config OMAP_IOMMU_DEBUG config ROCKCHIP_IOMMU bool "Rockchip IOMMU Support" - depends on ARM + depends on ARM || ARM64 depends on ARCH_ROCKCHIP || COMPILE_TEST select IOMMU_API select ARM_DMA_USE_IOMMU -- 2.8.0.rc3.226.g39d4020
[PATCH v4 7/8] drm/rockchip: Use common IOMMU API to attach devices
From: Shunqian Zheng Rockchip DRM used the arm special API, arm_iommu_*(), to attach iommu for ARM32 SoCs. This patch convert to common iommu API so it would support ARM64 like RK3399. Since previous patch added support for direct IOMMU address space management, there is no need to use DMA API anymore and this patch wires things to use the new method. Signed-off-by: Shunqian Zheng Signed-off-by: Tomasz Figa --- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 100 +++- 1 file changed, 53 insertions(+), 47 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c index e2c31d3..2793ac9 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c @@ -14,18 +14,18 @@ * GNU General Public License for more details. */ -#include - #include #include #include #include #include +#include #include #include #include #include #include +#include #include "rockchip_drm_drv.h" #include "rockchip_drm_fb.h" @@ -49,28 +49,31 @@ static struct drm_driver rockchip_drm_driver; int rockchip_drm_dma_attach_device(struct drm_device *drm_dev, struct device *dev) { - struct dma_iommu_mapping *mapping = drm_dev->dev->archdata.mapping; + struct rockchip_drm_private *private = drm_dev->dev_private; int ret; if (!is_support_iommu) return 0; - ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32)); - if (ret) + ret = iommu_attach_device(private->domain, dev); + if (ret) { + dev_err(dev, "Failed to attach iommu device\n"); return ret; + } - dma_set_max_seg_size(dev, DMA_BIT_MASK(32)); - - return arm_iommu_attach_device(dev, mapping); + return 0; } void rockchip_drm_dma_detach_device(struct drm_device *drm_dev, struct device *dev) { + struct rockchip_drm_private *private = drm_dev->dev_private; + struct iommu_domain *domain = private->domain; + if (!is_support_iommu) return; - arm_iommu_detach_device(dev); + iommu_detach_device(domain, dev); } int rockchip_register_crtc_funcs(struct drm_crtc *crtc, @@ -135,11 +138,45 @@ static void rockchip_drm_crtc_disable_vblank(struct drm_device *dev, priv->crtc_funcs[pipe]->disable_vblank(crtc); } +static int rockchip_drm_init_iommu(struct drm_device *drm_dev) +{ + struct rockchip_drm_private *private = drm_dev->dev_private; + struct iommu_domain_geometry *geometry; + u64 start, end; + + if (!is_support_iommu) + return 0; + + private->domain = iommu_domain_alloc(&platform_bus_type); + if (!private->domain) + return -ENOMEM; + + geometry = &private->domain->geometry; + start = geometry->aperture_start; + end = geometry->aperture_end; + + DRM_DEBUG("IOMMU context initialized (aperture: %#llx-%#llx)\n", + start, end); + drm_mm_init(&private->mm, start, end - start + 1); + + return 0; +} + +static void rockchip_iommu_cleanup(struct drm_device *drm_dev) +{ + struct rockchip_drm_private *private = drm_dev->dev_private; + + if (!is_support_iommu) + return; + + drm_mm_takedown(&private->mm); + iommu_domain_free(private->domain); +} + static int rockchip_drm_bind(struct device *dev) { struct drm_device *drm_dev; struct rockchip_drm_private *private; - struct dma_iommu_mapping *mapping = NULL; int ret; drm_dev = drm_dev_alloc(&rockchip_drm_driver, dev); @@ -160,38 +197,14 @@ static int rockchip_drm_bind(struct device *dev) rockchip_drm_mode_config_init(drm_dev); - dev->dma_parms = devm_kzalloc(dev, sizeof(*dev->dma_parms), - GFP_KERNEL); - if (!dev->dma_parms) { - ret = -ENOMEM; + ret = rockchip_drm_init_iommu(drm_dev); + if (ret) goto err_config_cleanup; - } - - if (is_support_iommu) { - /* TODO(djkurtz): fetch the mapping start/size from somewhere */ - mapping = arm_iommu_create_mapping(&platform_bus_type, - 0x, - SZ_2G); - if (IS_ERR(mapping)) { - ret = PTR_ERR(mapping); - goto err_config_cleanup; - } - - ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32)); - if (ret) - goto err_release_mapping; - - dma_set_max_seg_size(dev, DMA_BIT_MASK(32)); - - ret = arm_iommu_attach_device(dev, mapping); - if (ret) - goto err_release_mapping; - } /* Try to bind al
[PATCH v4 6/8] drm/rockchip: Do not use DMA mapping API if attached to IOMMU domain
The API is not suitable for subsystems consisting of multiple devices and requires severe hacks to use it. To mitigate this, this patch implements allocation and address space management locally by using helpers provided by DRM framework, like other DRM drivers do, e.g. Tegra. This patch should not introduce any functional changes until the driver is made to attach subdevices into an IOMMU domain with the generic IOMMU API, which will happen in following patch. Based heavily on GEM implementation of Tegra DRM driver. Signed-off-by: Tomasz Figa --- drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 3 + drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 236 ++-- drivers/gpu/drm/rockchip/rockchip_drm_gem.h | 9 ++ 3 files changed, 237 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h index ea39329..5ab1223 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h @@ -30,6 +30,7 @@ struct drm_device; struct drm_connector; +struct iommu_domain; /* * Rockchip drm private crtc funcs. @@ -61,6 +62,8 @@ struct rockchip_drm_private { struct drm_gem_object *fbdev_bo; const struct rockchip_crtc_funcs *crtc_funcs[ROCKCHIP_MAX_CRTC]; struct drm_atomic_state *state; + struct iommu_domain *domain; + struct drm_mm mm; }; int rockchip_register_crtc_funcs(struct drm_crtc *crtc, diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c index 059e902..fc0e6c1 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c @@ -18,11 +18,147 @@ #include #include +#include #include "rockchip_drm_drv.h" #include "rockchip_drm_gem.h" -static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj, +static int rockchip_gem_iommu_map(struct rockchip_drm_private *private, + struct rockchip_gem_object *rk_obj) +{ + int prot = IOMMU_READ | IOMMU_WRITE; + ssize_t ret; + + if (rk_obj->mm) + return -EBUSY; + + rk_obj->mm = kzalloc(sizeof(*rk_obj->mm), GFP_KERNEL); + if (!rk_obj->mm) + return -ENOMEM; + + ret = drm_mm_insert_node_generic(&private->mm, rk_obj->mm, +rk_obj->base.size, PAGE_SIZE, +0, 0, 0); + if (ret < 0) { + DRM_ERROR("out of I/O virtual memory: %zd\n", ret); + goto err_free_mm; + } + + rk_obj->dma_addr = rk_obj->mm->start; + + ret = iommu_map_sg(private->domain, rk_obj->dma_addr, rk_obj->sgt->sgl, + rk_obj->sgt->nents, prot); + if (ret < 0) { + DRM_ERROR("failed to map buffer: %zd\n", ret); + goto err_remove_node; + } + + rk_obj->size = ret; + + return 0; + +err_remove_node: + drm_mm_remove_node(rk_obj->mm); +err_free_mm: + kfree(rk_obj->mm); + return ret; +} + +static int rockchip_gem_iommu_unmap(struct rockchip_drm_private *private, + struct rockchip_gem_object *rk_obj) +{ + if (!rk_obj->mm) + return 0; + + iommu_unmap(private->domain, rk_obj->dma_addr, rk_obj->size); + drm_mm_remove_node(rk_obj->mm); + kfree(rk_obj->mm); + + return 0; +} + +static int rockchip_gem_get_pages(struct drm_device *drm, + struct rockchip_gem_object *rk_obj) +{ + int ret, i; + struct scatterlist *s; + + rk_obj->pages = drm_gem_get_pages(&rk_obj->base); + if (IS_ERR(rk_obj->pages)) + return PTR_ERR(rk_obj->pages); + + rk_obj->num_pages = rk_obj->base.size >> PAGE_SHIFT; + + rk_obj->sgt = drm_prime_pages_to_sg(rk_obj->pages, rk_obj->num_pages); + if (IS_ERR(rk_obj->sgt)) { + ret = PTR_ERR(rk_obj->sgt); + goto err_put_pages; + } + + /* +* Fake up the SG table so that dma_sync_sg_for_device() can be used +* to flush the pages associated with it. +* +* TODO: Replace this by drm_clflash_sg() once it can be implemented +* without relying on symbols that are not exported. +*/ + for_each_sg(rk_obj->sgt->sgl, s, rk_obj->sgt->nents, i) + sg_dma_address(s) = sg_phys(s); + + dma_sync_sg_for_device(drm->dev, rk_obj->sgt->sgl, rk_obj->sgt->nents, + DMA_TO_DEVICE); + + return 0; + +err_put_pages: + drm_gem_put_pages(&rk_obj->base, rk_obj->pages, false, false); + return ret; +} + +static void rockchip_gem_put_pages(struct drm_device *drm, + struct rockchip_gem_object *rk_obj) +{ + sg_free_table(rk_obj->sgt); + kfree(rk_obj->sgt); + drm_gem_put_pages(&rk_obj->base, rk_obj
[PATCH v4 5/8] iommu/rockchip: Prepare to support generic DMA mapping
From: Shunqian Zheng Set geometry for allocated domains and fix .domain_alloc() callback to work with IOMMU_DOMAIN_DMA domain type, which is used for implicit domains on ARM64. Signed-off-by: Shunqian Zheng Signed-off-by: Tomasz Figa --- drivers/iommu/rockchip-iommu.c | 16 +++- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c index 0551146..d5fd074 100644 --- a/drivers/iommu/rockchip-iommu.c +++ b/drivers/iommu/rockchip-iommu.c @@ -888,7 +888,7 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type) struct platform_device *pdev; struct device *iommu_dev; - if (type != IOMMU_DOMAIN_UNMANAGED) + if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA) return NULL; /* Register a pdev per domain, so DMA API can base on this *dev @@ -905,8 +905,8 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type) rk_domain->pdev = pdev; - /* To init the iovad which is required by iommu_dma_init_domain() */ - if (iommu_get_dma_cookie(&rk_domain->domain)) + if (type == IOMMU_DOMAIN_DMA && + iommu_get_dma_cookie(&rk_domain->domain)) goto err_unreg_pdev; /* @@ -932,12 +932,17 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type) spin_lock_init(&rk_domain->dt_lock); INIT_LIST_HEAD(&rk_domain->iommus); + rk_domain->domain.geometry.aperture_start = 0; + rk_domain->domain.geometry.aperture_end = DMA_BIT_MASK(32); + rk_domain->domain.geometry.force_aperture = true; + return &rk_domain->domain; err_free_dt: free_page((unsigned long)rk_domain->dt); err_put_cookie: - iommu_put_dma_cookie(&rk_domain->domain); + if (type == IOMMU_DOMAIN_DMA) + iommu_put_dma_cookie(&rk_domain->domain); err_unreg_pdev: platform_device_unregister(pdev); @@ -966,7 +971,8 @@ static void rk_iommu_domain_free(struct iommu_domain *domain) SPAGE_SIZE, DMA_TO_DEVICE); free_page((unsigned long)rk_domain->dt); - iommu_put_dma_cookie(&rk_domain->domain); + if (domain->type == IOMMU_DOMAIN_DMA) + iommu_put_dma_cookie(&rk_domain->domain); platform_device_unregister(rk_domain->pdev); } -- 2.8.0.rc3.226.g39d4020
[PATCH v4 4/8] iommu/rockchip: Use DMA API to manage coherency
From: Shunqian Zheng Use DMA API instead of architecture internal functions like __cpuc_flush_dcache_area() etc. The biggest difficulty here is that dma_map and _sync calls require some struct device, while there is no real 1:1 relation between an IOMMU domain and some device. To overcome this, a simple platform device is registered for each allocated IOMMU domain. With this patch, this driver can be used on both ARM and ARM64 platforms, such as RK3288 and RK3399 respectively. Signed-off-by: Shunqian Zheng Signed-off-by: Tomasz Figa --- drivers/iommu/rockchip-iommu.c | 161 +++-- 1 file changed, 122 insertions(+), 39 deletions(-) diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c index 8a5bac7..0551146 100644 --- a/drivers/iommu/rockchip-iommu.c +++ b/drivers/iommu/rockchip-iommu.c @@ -4,11 +4,10 @@ * published by the Free Software Foundation. */ -#include -#include #include #include #include +#include #include #include #include @@ -77,7 +76,9 @@ struct rk_iommu_domain { struct list_head iommus; + struct platform_device *pdev; u32 *dt; /* page directory table */ + dma_addr_t dt_dma; spinlock_t iommus_lock; /* lock for iommus list */ spinlock_t dt_lock; /* lock for modifying page directory table */ @@ -93,14 +94,12 @@ struct rk_iommu { struct iommu_domain *domain; /* domain to which iommu is attached */ }; -static inline void rk_table_flush(u32 *va, unsigned int count) +static inline void rk_table_flush(struct rk_iommu_domain *dom, dma_addr_t dma, + unsigned int count) { - phys_addr_t pa_start = virt_to_phys(va); - phys_addr_t pa_end = virt_to_phys(va + count); - size_t size = pa_end - pa_start; + size_t size = count * 4; /* count of entry, 4 bytes per entry */ - __cpuc_flush_dcache_area(va, size); - outer_flush_range(pa_start, pa_end); + dma_sync_single_for_device(&dom->pdev->dev, dma, size, DMA_TO_DEVICE); } static struct rk_iommu_domain *to_rk_domain(struct iommu_domain *dom) @@ -183,10 +182,9 @@ static inline bool rk_dte_is_pt_valid(u32 dte) return dte & RK_DTE_PT_VALID; } -static u32 rk_mk_dte(u32 *pt) +static inline u32 rk_mk_dte(dma_addr_t pt_dma) { - phys_addr_t pt_phys = virt_to_phys(pt); - return (pt_phys & RK_DTE_PT_ADDRESS_MASK) | RK_DTE_PT_VALID; + return (pt_dma & RK_DTE_PT_ADDRESS_MASK) | RK_DTE_PT_VALID; } /* @@ -603,13 +601,16 @@ static void rk_iommu_zap_iova_first_last(struct rk_iommu_domain *rk_domain, static u32 *rk_dte_get_page_table(struct rk_iommu_domain *rk_domain, dma_addr_t iova) { + struct device *dev = &rk_domain->pdev->dev; u32 *page_table, *dte_addr; - u32 dte; + u32 dte_index, dte; phys_addr_t pt_phys; + dma_addr_t pt_dma; assert_spin_locked(&rk_domain->dt_lock); - dte_addr = &rk_domain->dt[rk_iova_dte_index(iova)]; + dte_index = rk_iova_dte_index(iova); + dte_addr = &rk_domain->dt[dte_index]; dte = *dte_addr; if (rk_dte_is_pt_valid(dte)) goto done; @@ -618,19 +619,26 @@ static u32 *rk_dte_get_page_table(struct rk_iommu_domain *rk_domain, if (!page_table) return ERR_PTR(-ENOMEM); - dte = rk_mk_dte(page_table); - *dte_addr = dte; + pt_dma = dma_map_single(dev, page_table, SPAGE_SIZE, DMA_TO_DEVICE); + if (dma_mapping_error(dev, pt_dma)) { + dev_err(dev, "DMA mapping error while allocating page table\n"); + free_page((unsigned long)page_table); + return ERR_PTR(-ENOMEM); + } - rk_table_flush(page_table, NUM_PT_ENTRIES); - rk_table_flush(dte_addr, 1); + dte = rk_mk_dte(pt_dma); + *dte_addr = dte; + rk_table_flush(rk_domain, pt_dma, NUM_PT_ENTRIES); + rk_table_flush(rk_domain, rk_domain->dt_dma + dte_index * 4, 1); done: pt_phys = rk_dte_pt_address(dte); return (u32 *)phys_to_virt(pt_phys); } static size_t rk_iommu_unmap_iova(struct rk_iommu_domain *rk_domain, - u32 *pte_addr, dma_addr_t iova, size_t size) + u32 *pte_addr, dma_addr_t pte_dma, + size_t size) { unsigned int pte_count; unsigned int pte_total = size / SPAGE_SIZE; @@ -645,14 +653,14 @@ static size_t rk_iommu_unmap_iova(struct rk_iommu_domain *rk_domain, pte_addr[pte_count] = rk_mk_pte_invalid(pte); } - rk_table_flush(pte_addr, pte_count); + rk_table_flush(rk_domain, pte_dma, pte_count); return pte_count * SPAGE_SIZE; } static int rk_iommu_map_iova(struct rk_iommu_domain *rk_domain, u32 *pte_addr, -dma_addr_t iova, phys_addr_t paddr, size_t size, -int prot) +
[PATCH v4 3/8] iommu/rockchip: Fix allocation of bases array in driver probe
From: Shunqian Zheng In .probe(), devm_kzalloc() is called with size == 0 and works only by luck, due to internal behavior of the allocator and the fact that the proper allocation size is small. Let's use proper value for calculating the size. Fixes: cd6438c5f844 ("iommu/rockchip: Reconstruct to support multi slaves") Signed-off-by: Shunqian Zheng Signed-off-by: Tomasz Figa --- drivers/iommu/rockchip-iommu.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c index 53fa0d9..8a5bac7 100644 --- a/drivers/iommu/rockchip-iommu.c +++ b/drivers/iommu/rockchip-iommu.c @@ -1034,6 +1034,7 @@ static int rk_iommu_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct rk_iommu *iommu; struct resource *res; + int num_res = pdev->num_resources; int i; iommu = devm_kzalloc(dev, sizeof(*iommu), GFP_KERNEL); @@ -1043,12 +1044,13 @@ static int rk_iommu_probe(struct platform_device *pdev) platform_set_drvdata(pdev, iommu); iommu->dev = dev; iommu->num_mmu = 0; - iommu->bases = devm_kzalloc(dev, sizeof(*iommu->bases) * iommu->num_mmu, + + iommu->bases = devm_kzalloc(dev, sizeof(*iommu->bases) * num_res, GFP_KERNEL); if (!iommu->bases) return -ENOMEM; - for (i = 0; i < pdev->num_resources; i++) { + for (i = 0; i < num_res; i++) { res = platform_get_resource(pdev, IORESOURCE_MEM, i); if (!res) continue; -- 2.8.0.rc3.226.g39d4020
[PATCH v4 2/8] iommu/rockchip: Add map_sg callback for rk_iommu_ops
From: Simon Xue The iommu_dma_alloc() in iommu/dma-iommu.c calls iommu_map_sg() that requires the callback iommu_ops .map_sg(). Adding the default_iommu_map_sg() to Rockchip IOMMU accordingly. Signed-off-by: Simon Xue Signed-off-by: Shunqian Zheng Reviewed-on: https://chromium-review.googlesource.com/346326 Reviewed-by: Douglas Anderson Signed-off-by: Tomasz Figa --- drivers/iommu/rockchip-iommu.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c index 5a9659a..53fa0d9 100644 --- a/drivers/iommu/rockchip-iommu.c +++ b/drivers/iommu/rockchip-iommu.c @@ -1022,6 +1022,7 @@ static const struct iommu_ops rk_iommu_ops = { .detach_dev = rk_iommu_detach_device, .map = rk_iommu_map, .unmap = rk_iommu_unmap, + .map_sg = default_iommu_map_sg, .add_device = rk_iommu_add_device, .remove_device = rk_iommu_remove_device, .iova_to_phys = rk_iommu_iova_to_phys, -- 2.8.0.rc3.226.g39d4020
[PATCH v4 1/8] iommu/rockchip: Fix devm_{request,free}_irq parameter
From: Simon Xue Even though the IOMMU shares IRQ with its master, the struct device passed to {request,free}_irq is supposed to represent the device that is signalling the interrupt. This patch makes the driver use IOMMU device instead of master's device to make things clear. Signed-off-by: Simon Xue Signed-off-by: Shunqian Zheng Reviewed-on: https://chromium-review.googlesource.com/346325 Reviewed-by: Douglas Anderson Signed-off-by: Tomasz Figa --- drivers/iommu/rockchip-iommu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c index 25b4627..5a9659a 100644 --- a/drivers/iommu/rockchip-iommu.c +++ b/drivers/iommu/rockchip-iommu.c @@ -807,7 +807,7 @@ static int rk_iommu_attach_device(struct iommu_domain *domain, iommu->domain = domain; - ret = devm_request_irq(dev, iommu->irq, rk_iommu_irq, + ret = devm_request_irq(iommu->dev, iommu->irq, rk_iommu_irq, IRQF_SHARED, dev_name(dev), iommu); if (ret) return ret; @@ -860,7 +860,7 @@ static void rk_iommu_detach_device(struct iommu_domain *domain, } rk_iommu_disable_stall(iommu); - devm_free_irq(dev, iommu->irq, iommu); + devm_free_irq(iommu->dev, iommu->irq, iommu); iommu->domain = NULL; -- 2.8.0.rc3.226.g39d4020
[PATCH v4 0/8] iommu/rockchip: Fix bugs and enable on ARM64
This series intends mostly to enable support for ARM64 architecture in the rockchip-iommu driver. On the way to do so, some bugs are also fixed. The most important changes here are: - making the Rockchip IOMMU driver use DMA API for managing cache coherency of page tables, - making the Rockchip DRM driver not use DMA API on behalf of a virtual device (behind a virtual IOMMU) to allocate and map buffers, but instead proper DRM helpers and IOMMU API directly. Changes since v3: - Drop the idea of virtual IOMMU. Instead replace hacky allocation code in DRM driver, with proper management of IOMMU domain. - Add one more fix for allocation of IOMMU register base addresses. Changes since v2: - Instead of registering virtual IOMMU from DTS, create it when attaching. - Fix some bugs found in internal review. Shunqian Zheng (4): iommu/rockchip: Fix allocation of bases array in driver probe iommu/rockchip: Use DMA API to manage coherency iommu/rockchip: Prepare to support generic DMA mapping drm/rockchip: Use common IOMMU API to attach devices Simon Xue (3): iommu/rockchip: Fix devm_{request,free}_irq parameter iommu/rockchip: Add map_sg callback for rk_iommu_ops iommu/rockchip: Enable Rockchip IOMMU on ARM64 Tomasz Figa (1): drm/rockchip: Do not use DMA mapping API if attached to IOMMU domain drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 100 ++-- drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 3 + drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 236 ++-- drivers/gpu/drm/rockchip/rockchip_drm_gem.h | 9 ++ drivers/iommu/Kconfig | 2 +- drivers/iommu/rockchip-iommu.c | 180 +++-- 6 files changed, 427 insertions(+), 103 deletions(-) -- 2.8.0.rc3.226.g39d4020
[PATCH] drm/rockchip: Finish initialization before registering DRM device
Currently the driver calls drm_dev_register() directly after allocating the DRM device and then continues with further initialization. This is incorrect, because drm_dev_register() is supposed to be called after all initialization is done. This problem was masked by the fact that drm_dev_register() did not use to do anything special before, but recently it started to call drm_connector_register_all(), which leads to a crash if the driver is not fully initialized. This patch fixes the problem by moving the call to drm_dev_register() to the end of the initialization sequence and also removing the, now unnecessary, call to drm_connector_register_all() from driver code. Fixes: f706974a69b6 ("drm/rockchip: Drop drm_driver.load/unload callbacks") Signed-off-by: Tomasz Figa --- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 18 +++--- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c index 0d28455..e2c31d3 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c @@ -146,16 +146,12 @@ static int rockchip_drm_bind(struct device *dev) if (!drm_dev) return -ENOMEM; - ret = drm_dev_register(drm_dev, 0); - if (ret) - goto err_free; - dev_set_drvdata(dev, drm_dev); private = devm_kzalloc(drm_dev->dev, sizeof(*private), GFP_KERNEL); if (!private) { ret = -ENOMEM; - goto err_unregister; + goto err_free; } drm_dev->dev_private = private; @@ -197,12 +193,6 @@ static int rockchip_drm_bind(struct device *dev) if (ret) goto err_detach_device; - ret = drm_connector_register_all(drm_dev); - if (ret) { - dev_err(dev, "failed to register connectors\n"); - goto err_unbind; - } - /* init kms poll for handling hpd */ drm_kms_helper_poll_init(drm_dev); @@ -222,9 +212,15 @@ static int rockchip_drm_bind(struct device *dev) if (ret) goto err_vblank_cleanup; + ret = drm_dev_register(drm_dev, 0); + if (ret) + goto err_fbdev_fini; + if (is_support_iommu) arm_iommu_release_mapping(mapping); return 0; +err_fbdev_fini: + rockchip_drm_fbdev_fini(drm_dev); err_vblank_cleanup: drm_vblank_cleanup(drm_dev); err_kms_helper_poll_fini: -- 2.8.0.rc3.226.g39d4020
[PATCH v2 1/3] drm: Add callbacks for late registering
2016-06-21 12:31 GMT+02:00 Chris Wilson : > On Tue, Jun 21, 2016 at 11:31:34AM +0200, Benjamin Gaignard wrote: >> Like what has been done for connectors add callbacks on encoder, >> crtc and plane to let driver do actions after drm device registration. >> >> Correspondingly, add callbacks called before unregister drm device. >> >> version 2: >> add drm_modeset_register_all() and drm_modeset_unregister_all() >> to centralize all calls >> >> Signed-off-by: Benjamin Gaignard >> --- >> drivers/gpu/drm/drm_crtc.c | 122 >> + >> drivers/gpu/drm/drm_drv.c | 4 +- >> include/drm/drm_crtc.h | 79 + >> 3 files changed, 203 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c >> index e7c862b..b393feb 100644 >> --- a/drivers/gpu/drm/drm_crtc.c >> +++ b/drivers/gpu/drm/drm_crtc.c >> @@ -608,6 +608,31 @@ static unsigned int drm_num_crtcs(struct drm_device >> *dev) >> return num; >> } >> >> +static int drm_crtc_register_all(struct drm_device *dev) >> +{ >> + struct drm_crtc *crtc; >> + int ret; >> + >> + drm_for_each_crtc(crtc, dev) { >> + if (crtc->funcs->late_register) >> + ret = crtc->funcs->late_register(crtc); >> + if (ret) > > ret is uninitialised. OK I fix that for v3 > > It is a good question (probably for another series) on what exactly to > do on registration failure? At the very least we need to unwind on the > error path, or we just ignore errors (other than a DRM_ERROR to > userspace explaining why one object is not available, but otherwise let > the driver load). > >> +int drm_modeset_register_all(struct drm_device *dev) >> +{ >> + int ret; >> + >> + ret = drm_plane_register_all(dev); >> + if (ret) >> + return ret; >> + >> + ret = drm_crtc_register_all(dev); >> + if (ret) >> + return ret; >> + >> + ret = drm_encoder_register_all(dev); >> + if (ret) >> + return ret; >> + >> + ret = drm_connector_register_all(dev); > > Might as well continue on with > > ret = <> > if (ret) > return ret; > > for a consistent style. Along with the comment about how should we be > handling errors? If we report an error, everything up to that point > should be unwound. > OK I will add goto for each case. >> + return ret; >> +} >> +EXPORT_SYMBOL(drm_modeset_register_all); >> + >> +/** >> + * drm_modeset_unregister_all - do early unregistration >> + * @dev: drm device >> + * >> + * This function call early_unregister callbakc for all planes, >> + * crtcs, encoders and connectors >> + */ >> +void drm_modeset_unregister_all(struct drm_device *dev) >> +{ >> + drm_plane_unregister_all(dev); >> + drm_crtc_unregister_all(dev); >> + drm_encoder_unregister_all(dev); >> + drm_connector_unregister_all(dev); > > Unregister should be in the opposite order. OK for v3 >> +} >> +EXPORT_SYMBOL(drm_modeset_unregister_all); > > I think the plan is not to export these symbols, > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre -- Benjamin Gaignard Graphic Working Group Linaro.org â Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
[PATCH v1 3/3] drm/i915: Use new CRC debugfs API
The core provides now an ABI to userspace for generation of CRCs that is compatible with the one currently in i915. So remove the code that is now duplicated and implement instead the ->set_crc_source() callback to start and end CRC generation. We still register files in the old debugfs paths so the current testsuite still passes. Signed-off-by: Tomeu Vizoso --- drivers/gpu/drm/i915/i915_debugfs.c | 8 +- drivers/gpu/drm/i915/i915_dma.c | 2 - drivers/gpu/drm/i915/i915_drv.h | 21 -- drivers/gpu/drm/i915/i915_irq.c | 39 +-- drivers/gpu/drm/i915/intel_display.c | 3 + drivers/gpu/drm/i915/intel_drv.h | 4 +- drivers/gpu/drm/i915/intel_pipe_crc.c | 511 -- 7 files changed, 67 insertions(+), 521 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index ece88a1966b2..f82e66609ace 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -4620,7 +4620,7 @@ static const struct i915_debugfs_files { {"i915_gem_drop_caches", &i915_drop_caches_fops}, {"i915_error_state", &i915_error_state_fops}, {"i915_next_seqno", &i915_next_seqno_fops}, - {"i915_display_crc_ctl", &i915_display_crc_ctl_fops}, + {"i915_display_crc_ctl", &drm_crc_control_fops}, {"i915_pri_wm_latency", &i915_pri_wm_latency_fops}, {"i915_spr_wm_latency", &i915_spr_wm_latency_fops}, {"i915_cur_wm_latency", &i915_cur_wm_latency_fops}, @@ -4638,10 +4638,6 @@ int i915_debugfs_init(struct drm_minor *minor) if (ret) return ret; - ret = intel_pipe_crc_create(minor); - if (ret) - return ret; - for (i = 0; i < ARRAY_SIZE(i915_debugfs_files); i++) { ret = i915_debugfs_create(minor->debugfs_root, minor, i915_debugfs_files[i].name, @@ -4665,8 +4661,6 @@ void i915_debugfs_cleanup(struct drm_minor *minor) drm_debugfs_remove_files((struct drm_info_list *) &i915_forcewake_fops, 1, minor); - intel_pipe_crc_cleanup(minor); - for (i = 0; i < ARRAY_SIZE(i915_debugfs_files); i++) { struct drm_info_list *info_list = (struct drm_info_list *) i915_debugfs_files[i].fops; diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index d15a461fa84a..bf8fc274f067 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1136,8 +1136,6 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv, intel_init_audio_hooks(dev_priv); i915_gem_load_init(dev); - intel_display_crc_init(dev); - i915_dump_device_info(dev_priv); /* Not all pre-production machines fall into this category, only the diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 7c81757f3a3f..a2d8e4eca502 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1661,21 +1661,6 @@ enum intel_pipe_crc_source { INTEL_PIPE_CRC_SOURCE_MAX, }; -struct intel_pipe_crc_entry { - uint32_t frame; - uint32_t crc[5]; -}; - -#define INTEL_PIPE_CRC_ENTRIES_NR 128 -struct intel_pipe_crc { - spinlock_t lock; - bool opened;/* exclusive access to the result file */ - struct intel_pipe_crc_entry *entries; - enum intel_pipe_crc_source source; - int head, tail; - wait_queue_head_t wq; -}; - struct i915_frontbuffer_tracking { struct mutex lock; @@ -1881,10 +1866,6 @@ struct drm_i915_private { struct drm_crtc *pipe_to_crtc_mapping[I915_MAX_PIPES]; wait_queue_head_t pending_flip_queue; -#ifdef CONFIG_DEBUG_FS - struct intel_pipe_crc pipe_crc[I915_MAX_PIPES]; -#endif - /* dpll and cdclk state is protected by connection_mutex */ int num_shared_dpll; struct intel_shared_dpll shared_dplls[I915_NUM_PLLS]; @@ -3600,11 +3581,9 @@ int i915_debugfs_init(struct drm_minor *minor); void i915_debugfs_cleanup(struct drm_minor *minor); #ifdef CONFIG_DEBUG_FS int i915_debugfs_connector_add(struct drm_connector *connector); -void intel_display_crc_init(struct drm_device *dev); #else static inline int i915_debugfs_connector_add(struct drm_connector *connector) { return 0; } -static inline void intel_display_crc_init(struct drm_device *dev) {} #endif /* i915_gpu_error.c */ diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 4378a659d962..7eb57b9658e4 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1503,43 +1503,12 @@ static void display_pipe_crc_irq_handler(struct drm_i915_private *dev_priv, uint32_t crc2, uint32_t crc3, uint32_t crc4) { - struct intel_pipe_crc *pipe_crc = &dev_priv
[PATCH v1 2/3] drm: Add API for capturing frame CRCs
Adds a per-device debugfile "drm_crc_control" that allows selecting a source for frame checksums in each CRTC that supports them. The checksums for each subsequent frame can be read from the per-CRTC file "drm_crtc_N_crc". The code is taken from the i915 driver and other drivers can now provide frame CRCs by implementing the set_crc_source callback in drm_crtc_funcs. Signed-off-by: Tomeu Vizoso --- drivers/gpu/drm/drm_crtc.c | 28 ++- drivers/gpu/drm/drm_debugfs.c | 506 - drivers/gpu/drm/drm_internal.h | 10 + include/drm/drmP.h | 5 + include/drm/drm_crtc.h | 72 ++ 5 files changed, 611 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index e7c862bd2f19..4dae42b122d9 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -657,8 +657,8 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, drm_num_crtcs(dev)); } if (!crtc->name) { - drm_mode_object_unregister(dev, &crtc->base); - return -ENOMEM; + ret = -ENOMEM; + goto err_unregister; } crtc->base.properties = &crtc->properties; @@ -673,12 +673,30 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, if (cursor) cursor->possible_crtcs = 1 << drm_crtc_index(crtc); +#ifdef CONFIG_DEBUG_FS + spin_lock_init(&crtc->crc.lock); + init_waitqueue_head(&crtc->crc.wq); + crtc->crc.debugfs_entries = kmalloc_array(DRM_MINOR_CNT, + sizeof(*crtc->crc.debugfs_entries), + GFP_KERNEL); + + ret = drm_debugfs_crtc_add(crtc); + if (ret) + goto err_free_name; +#endif + if (drm_core_check_feature(dev, DRIVER_ATOMIC)) { drm_object_attach_property(&crtc->base, config->prop_active, 0); drm_object_attach_property(&crtc->base, config->prop_mode_id, 0); } return 0; + +err_free_name: + kfree(crtc->name); +err_unregister: + drm_mode_object_unregister(dev, &crtc->base); + return ret; } EXPORT_SYMBOL(drm_crtc_init_with_planes); @@ -699,6 +717,12 @@ void drm_crtc_cleanup(struct drm_crtc *crtc) * the indices on the drm_crtc after us in the crtc_list. */ +#ifdef CONFIG_DEBUG_FS + drm_debugfs_crtc_remove(crtc); + kfree(crtc->crc.debugfs_entries); + kfree(crtc->crc.source); +#endif + kfree(crtc->gamma_store); crtc->gamma_store = NULL; diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c index fa10cef2ba37..cdc8836bc22a 100644 --- a/drivers/gpu/drm/drm_debugfs.c +++ b/drivers/gpu/drm/drm_debugfs.c @@ -30,6 +30,8 @@ * OTHER DEALINGS IN THE SOFTWARE. */ +#include +#include #include #include #include @@ -127,6 +129,259 @@ fail: } EXPORT_SYMBOL(drm_debugfs_create_files); +static int +drm_add_fake_info_node(struct drm_minor *minor, + struct dentry *ent, + const void *key) +{ + struct drm_info_node *node; + + node = kmalloc(sizeof(*node), GFP_KERNEL); + if (node == NULL) { + debugfs_remove(ent); + return -ENOMEM; + } + + node->minor = minor; + node->dent = ent; + node->info_ent = (void *) key; + + mutex_lock(&minor->debugfs_lock); + list_add(&node->list, &minor->debugfs_list); + mutex_unlock(&minor->debugfs_lock); + + return 0; +} + +static int crc_control_show(struct seq_file *m, void *data) +{ + struct drm_device *dev = m->private; + struct drm_crtc *crtc; + + drm_for_each_crtc(crtc, dev) + seq_printf(m, "crtc %d %s\n", crtc->index, + crtc->crc.source ? crtc->crc.source : "none"); + + return 0; +} + +static int crc_control_open(struct inode *inode, struct file *file) +{ + struct drm_device *dev = inode->i_private; + + return single_open(file, crc_control_show, dev); +} + +static int crc_control_update_crtc(struct drm_crtc *crtc, const char *source) +{ + struct drm_crtc_crc *crc = &crtc->crc; + struct drm_crtc_crc_entry *entries; + int ret; + + if (!strcmp(source, "none")) + source = NULL; + + if (!crc->source && !source) + return 0; + + if (crc->source && source && !strcmp(crc->source, source)) + return 0; + + /* Forbid changing the source without going back to "none". */ + if (crc->source && source) + return -EINVAL; + + if (!crtc->funcs->set_crc_source) + return -ENOTSUPP; + + if (source) { + entries = kcalloc(DRM_CRTC_CRC_ENTRIES_NR, + sizeof(crc->entries[0]), +
[PATCH v1 1/3] drm/i915/debugfs: Move out pipe CRC code
In preparation to using a generic API in the DRM core for continuous CRC generation, move the related code out of i915_debugfs.c into a new file. Eventually, only the Intel-specific code will remain in this new file. Signed-off-by: Tomeu Vizoso --- drivers/gpu/drm/i915/Makefile | 2 +- drivers/gpu/drm/i915/i915_debugfs.c | 892 +-- drivers/gpu/drm/i915/intel_drv.h | 5 + drivers/gpu/drm/i915/intel_pipe_crc.c | 953 ++ 4 files changed, 963 insertions(+), 889 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_pipe_crc.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 276abf1cac2b..2c14612dbdb5 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -17,7 +17,7 @@ i915-y := i915_drv.o \ intel_runtime_pm.o i915-$(CONFIG_COMPAT) += i915_ioc32.o -i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o +i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o intel_pipe_crc.o # GEM code i915-y += i915_cmd_parser.o \ diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 5b7526697838..ece88a1966b2 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -26,19 +26,9 @@ * */ -#include -#include -#include #include -#include -#include #include -#include -#include #include "intel_drv.h" -#include "intel_ringbuffer.h" -#include -#include "i915_drv.h" enum { ACTIVE_LIST, @@ -3480,12 +3470,6 @@ static int i915_drrs_status(struct seq_file *m, void *unused) return 0; } -struct pipe_crc_info { - const char *name; - struct drm_device *dev; - enum pipe pipe; -}; - static int i915_dp_mst_info(struct seq_file *m, void *unused) { struct drm_info_node *node = (struct drm_info_node *) m->private; @@ -3509,853 +3493,6 @@ static int i915_dp_mst_info(struct seq_file *m, void *unused) return 0; } -static int i915_pipe_crc_open(struct inode *inode, struct file *filep) -{ - struct pipe_crc_info *info = inode->i_private; - struct drm_i915_private *dev_priv = info->dev->dev_private; - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe]; - - if (info->pipe >= INTEL_INFO(info->dev)->num_pipes) - return -ENODEV; - - spin_lock_irq(&pipe_crc->lock); - - if (pipe_crc->opened) { - spin_unlock_irq(&pipe_crc->lock); - return -EBUSY; /* already open */ - } - - pipe_crc->opened = true; - filep->private_data = inode->i_private; - - spin_unlock_irq(&pipe_crc->lock); - - return 0; -} - -static int i915_pipe_crc_release(struct inode *inode, struct file *filep) -{ - struct pipe_crc_info *info = inode->i_private; - struct drm_i915_private *dev_priv = info->dev->dev_private; - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe]; - - spin_lock_irq(&pipe_crc->lock); - pipe_crc->opened = false; - spin_unlock_irq(&pipe_crc->lock); - - return 0; -} - -/* (6 fields, 8 chars each, space separated (5) + '\n') */ -#define PIPE_CRC_LINE_LEN (6 * 8 + 5 + 1) -/* account for \'0' */ -#define PIPE_CRC_BUFFER_LEN(PIPE_CRC_LINE_LEN + 1) - -static int pipe_crc_data_count(struct intel_pipe_crc *pipe_crc) -{ - assert_spin_locked(&pipe_crc->lock); - return CIRC_CNT(pipe_crc->head, pipe_crc->tail, - INTEL_PIPE_CRC_ENTRIES_NR); -} - -static ssize_t -i915_pipe_crc_read(struct file *filep, char __user *user_buf, size_t count, - loff_t *pos) -{ - struct pipe_crc_info *info = filep->private_data; - struct drm_device *dev = info->dev; - struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe]; - char buf[PIPE_CRC_BUFFER_LEN]; - int n_entries; - ssize_t bytes_read; - - /* -* Don't allow user space to provide buffers not big enough to hold -* a line of data. -*/ - if (count < PIPE_CRC_LINE_LEN) - return -EINVAL; - - if (pipe_crc->source == INTEL_PIPE_CRC_SOURCE_NONE) - return 0; - - /* nothing to read */ - spin_lock_irq(&pipe_crc->lock); - while (pipe_crc_data_count(pipe_crc) == 0) { - int ret; - - if (filep->f_flags & O_NONBLOCK) { - spin_unlock_irq(&pipe_crc->lock); - return -EAGAIN; - } - - ret = wait_event_interruptible_lock_irq(pipe_crc->wq, - pipe_crc_data_count(pipe_crc), pipe_crc->lock); - if (ret) { - spin_unlock_irq(&pipe_crc->lock); - return ret; - } - } - - /* We now have one or more entries to read */ - n_entries = count / PIPE_CRC_LINE_LEN;
[PATCH v1 0/3] New debugfs API for capturing CRC of frames
Hi, this series basically takes the facility for continuously capturing CRCs of frames from the i915 driver and into the DRM core. The idea is that test suites such as IGT use this information to check that frames that are expected to be identical, also have identical CRC values. Other drivers for hardware that can provide frame CRCs (including eDP panels that support self-refresh) can easily implement the new callback and provide userspace with the CRC values. Thanks, Tomeu Tomeu Vizoso (3): drm/i915/debugfs: Move out pipe CRC code drm: Add API for capturing frame CRCs drm/i915: Use new CRC debugfs API drivers/gpu/drm/drm_crtc.c| 28 +- drivers/gpu/drm/drm_debugfs.c | 506 ++- drivers/gpu/drm/drm_internal.h| 10 + drivers/gpu/drm/i915/Makefile | 2 +- drivers/gpu/drm/i915/i915_debugfs.c | 892 +- drivers/gpu/drm/i915/i915_dma.c | 2 - drivers/gpu/drm/i915/i915_drv.h | 21 - drivers/gpu/drm/i915/i915_irq.c | 39 +- drivers/gpu/drm/i915/intel_display.c | 3 + drivers/gpu/drm/i915/intel_drv.h | 5 + drivers/gpu/drm/i915/intel_pipe_crc.c | 556 + include/drm/drmP.h| 5 + include/drm/drm_crtc.h| 72 +++ 13 files changed, 1181 insertions(+), 960 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_pipe_crc.c -- 2.5.5
[PATCH v3 4/4] drm/i915: Enable polling when we don't have hpd
Unfortunately, there's two situations where we lose hpd right now: - Runtime suspend - When we've shut off all of the power wells on Valleyview/Cherryview While it would be nice if this didn't cause issues, this has the ability to get us in some awkward states where a user won't be able to get their display to turn on. For instance; if we boot a Valleyview system without any monitors connected, it won't need any of it's power wells and thus shut them off. Since this causes us to lose HPD, this means that unless the user knows how to ssh into their machine and do a manual reprobe for monitors, none of the monitors they connect after booting will actually work. Eventually we should come up with a better fix then having to enable polling for this, since this makes rpm a lot less useful, but for now the infrastructure in i915 just isn't there yet to get hpd in these situations. Changes since v1: - Add comment explaining the addition of the if (!mode_config->poll_running) in intel_hpd_init() - Remove unneeded if (!dev->mode_config.poll_enabled) in i915_hpd_poll_init_work() - Call to drm_helper_hpd_irq_event() after we disable polling - Add cancel_work_sync() call to intel_hpd_cancel_work() Changes since v2: - Apparently dev->mode_config.poll_running doesn't actually reflect whether or not a poll is currently in progress, and is actually used for dynamic module paramter enabling/disabling. So now we instead keep track of our own poll_running variable in dev_priv->hotplug - Clean i915_hpd_poll_init_work() a little bit Cc: stable at vger.kernel.org Cc: Ville Syrjälä Cc: Daniel Vetter Signed-off-by: Lyude --- drivers/gpu/drm/i915/i915_drv.c | 7 ++- drivers/gpu/drm/i915/i915_drv.h | 4 ++ drivers/gpu/drm/i915/intel_drv.h| 2 + drivers/gpu/drm/i915/intel_hotplug.c| 85 + drivers/gpu/drm/i915/intel_runtime_pm.c | 3 ++ 5 files changed, 90 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 3eb47fb..5381d9e 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1612,6 +1612,9 @@ static int intel_runtime_suspend(struct device *device) assert_forcewakes_inactive(dev_priv); + if (!IS_VALLEYVIEW(dev_priv) || !IS_CHERRYVIEW(dev_priv)) + intel_hpd_poll_enable(dev_priv, true); + DRM_DEBUG_KMS("Device suspended\n"); return 0; } @@ -1667,8 +1670,10 @@ static int intel_runtime_resume(struct device *device) * power well, so hpd is reinitialized from there. For * everyone else do it here. */ - if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv)) + if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv)) { intel_hpd_init(dev_priv); + intel_hpd_poll_enable(dev_priv, false); + } intel_enable_gt_powersave(dev_priv); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 239a3ec..36a9d90 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -283,6 +283,10 @@ struct i915_hotplug { u32 short_port_mask; struct work_struct dig_port_work; + struct work_struct poll_enable_work; + bool poll_enabled; + bool poll_running; + /* * if we get a HPD irq from DP and a HPD irq from non-DP * the non-DP HPD could block the workqueue on a mode config diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index dcbfdde..cedac13 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1385,6 +1385,8 @@ int intel_dsi_dcs_init_backlight_funcs(struct intel_connector *intel_connector); /* intel_dvo.c */ void intel_dvo_init(struct drm_device *dev); +/* intel_hotplug.c */ +void intel_hpd_poll_enable(struct drm_i915_private *dev_priv, bool enabled); /* legacy fbdev emulation in intel_fbdev.c */ diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c index ec3285f..1fc36a5 100644 --- a/drivers/gpu/drm/i915/intel_hotplug.c +++ b/drivers/gpu/drm/i915/intel_hotplug.c @@ -464,20 +464,34 @@ void intel_hpd_init(struct drm_i915_private *dev_priv) dev_priv->hotplug.stats[i].count = 0; dev_priv->hotplug.stats[i].state = HPD_ENABLED; } - list_for_each_entry(connector, &mode_config->connector_list, head) { - struct intel_connector *intel_connector = to_intel_connector(connector); - connector->polled = intel_connector->polled; - /* MST has a dynamic intel_connector->encoder and it's reprobing -* is all handled by the MST helpers. */ - if (intel_connector->mst_port) - continue; + /* +* When we're in the midst of doing connector polling in situations +* where we don't have
[PATCH v3 3/4] drm/i915/vlv: Disable HPD in valleyview_crt_detect_hotplug()
One of the things preventing us from using polling is the fact that calling valleyview_crt_detect_hotplug() when there's a VGA cable connected results in sending another hotplug. With polling enabled when HPD is disabled, this results in a scenario like this: - We enable power wells and reset the ADPA - output_poll_exec does force probe on VGA, triggering a hpd - HPD handler waits for poll to unlock dev->mode_config.mutex - output_poll_exec shuts off the ADPA, unlocks dev->mode_config.mutex - HPD handler runs, resets ADPA and brings us back to the start This results in an endless irq storm getting sent from the ADPA whenever a VGA connector gets detected in the middle of polling. Somewhat based off of the "drm/i915: Disable CRT HPD around force trigger" patch Ville Syrjälä sent a while back Cc: stable at vger.kernel.org Cc: Ville Syrjälä Signed-off-by: Lyude --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/intel_crt.c | 18 ++ drivers/gpu/drm/i915/intel_hotplug.c | 27 +++ 3 files changed, 47 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a988a95..239a3ec 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2933,6 +2933,8 @@ void intel_hpd_init(struct drm_i915_private *dev_priv); void intel_hpd_init_work(struct drm_i915_private *dev_priv); void intel_hpd_cancel_work(struct drm_i915_private *dev_priv); bool intel_hpd_pin_to_port(enum hpd_pin pin, enum port *port); +bool intel_hpd_disable(struct drm_i915_private *dev_priv, enum hpd_pin pin); +void intel_hpd_enable(struct drm_i915_private *dev_priv, enum hpd_pin pin); /* i915_irq.c */ void i915_queue_hangcheck(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index 220be7a..a626793 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -327,10 +327,25 @@ static bool valleyview_crt_detect_hotplug(struct drm_connector *connector) struct drm_device *dev = connector->dev; struct intel_crt *crt = intel_attached_crt(connector); struct drm_i915_private *dev_priv = dev->dev_private; + bool reenable_hpd; u32 adpa; bool ret; u32 save_adpa; + /* +* Doing a force trigger causes a hpd interrupt to get sent, which can +* get us stuck in a loop if we're polling: +* - We enable power wells and reset the ADPA +* - output_poll_exec does force probe on VGA, triggering a hpd +* - HPD handler waits for poll to unlock dev->mode_config.mutex +* - output_poll_exec shuts off the ADPA, unlocks +*dev->mode_config.mutex +* - HPD handler runs, resets ADPA and brings us back to the start +* +* Just disable HPD interrupts here to prevent this +*/ + reenable_hpd = intel_hpd_disable(dev_priv, crt->base.hpd_pin); + save_adpa = adpa = I915_READ(crt->adpa_reg); DRM_DEBUG_KMS("trigger hotplug detect cycle: adpa=0x%x\n", adpa); @@ -353,6 +368,9 @@ static bool valleyview_crt_detect_hotplug(struct drm_connector *connector) DRM_DEBUG_KMS("valleyview hotplug adpa=0x%x, result %d\n", adpa, ret); + if (reenable_hpd) + intel_hpd_enable(dev_priv, crt->base.hpd_pin); + return ret; } diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c index 38eeca7..ec3285f 100644 --- a/drivers/gpu/drm/i915/intel_hotplug.c +++ b/drivers/gpu/drm/i915/intel_hotplug.c @@ -510,3 +510,30 @@ void intel_hpd_cancel_work(struct drm_i915_private *dev_priv) cancel_work_sync(&dev_priv->hotplug.hotplug_work); cancel_delayed_work_sync(&dev_priv->hotplug.reenable_work); } + +bool intel_hpd_disable(struct drm_i915_private *dev_priv, enum hpd_pin pin) +{ + bool ret = false; + + if (pin == HPD_NONE) + return false; + + spin_lock_irq(&dev_priv->irq_lock); + if (dev_priv->hotplug.stats[pin].state == HPD_ENABLED) { + dev_priv->hotplug.stats[pin].state = HPD_DISABLED; + ret = true; + } + spin_unlock_irq(&dev_priv->irq_lock); + + return ret; +} + +void intel_hpd_enable(struct drm_i915_private *dev_priv, enum hpd_pin pin) +{ + if (pin == HPD_NONE) + return; + + spin_lock_irq(&dev_priv->irq_lock); + dev_priv->hotplug.stats[pin].state = HPD_ENABLED; + spin_unlock_irq(&dev_priv->irq_lock); +} -- 2.5.5
[PATCH v3 2/4] drm/i915/vlv: Reset the ADPA in vlv_display_power_well_init()
While VGA hotplugging worked(ish) before, it looks like that was mainly because we'd unintentionally enable it in valleyview_crt_detect_hotplug() when we did a force trigger. This doesn't work reliably enough because whenever the display powerwell on vlv gets disabled, the values set in VLV_ADPA get cleared and consequently VGA hotplugging gets disabled. This causes bugs such as one we found on an Intel NUC, where doing the following sequence of hotplugs: - Disconnect all monitors - Connect VGA - Disconnect VGA - Connect HDMI Would result in VGA hotplugging becoming disabled, due to the powerwells getting toggled in the process of connecting HDMI. Changes since v3: - Expose intel_crt_reset() through intel_drv.h and call that in vlv_display_power_well_init() instead of encoder->base.funcs->reset(&encoder->base); Changes since v2: - Use intel_encoder structs instead of drm_encoder structs Changes since v1: - Instead of handling the register writes ourself, we just reuse intel_crt_detect() - Instead of resetting the ADPA during display IRQ installation, we now reset them in vlv_display_power_well_init() Cc: stable at vger.kernel.org Acked-by: Daniel Vetter Signed-off-by: Lyude Reviewed-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_crt.c| 2 +- drivers/gpu/drm/i915/intel_drv.h| 2 +- drivers/gpu/drm/i915/intel_runtime_pm.c | 7 +++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index dfcd718..220be7a 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -713,7 +713,7 @@ static int intel_crt_set_property(struct drm_connector *connector, return 0; } -static void intel_crt_reset(struct drm_encoder *encoder) +void intel_crt_reset(struct drm_encoder *encoder) { struct drm_device *dev = encoder->dev; struct drm_i915_private *dev_priv = dev->dev_private; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 496c962..dcbfdde 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1075,7 +1075,7 @@ void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv, /* intel_crt.c */ void intel_crt_init(struct drm_device *dev); - +void intel_crt_reset(struct drm_encoder *encoder); /* intel_ddi.c */ void intel_ddi_clk_select(struct intel_encoder *encoder, diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index e856d49..f88ef76 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -1065,6 +1065,7 @@ static void vlv_init_display_clock_gating(struct drm_i915_private *dev_priv) static void vlv_display_power_well_init(struct drm_i915_private *dev_priv) { + struct intel_encoder *encoder; enum pipe pipe; /* @@ -1100,6 +1101,12 @@ static void vlv_display_power_well_init(struct drm_i915_private *dev_priv) intel_hpd_init(dev_priv); + /* Re-enable the ADPA, if we have one */ + for_each_intel_encoder(dev_priv->dev, encoder) { + if (encoder->type == INTEL_OUTPUT_ANALOG) + intel_crt_reset(&encoder->base); + } + i915_redisable_vga_power_on(dev_priv->dev); } -- 2.5.5
[PATCH v3 1/4] drm/i915/vlv: Make intel_crt_reset() per-encoder
This lets call intel_crt_reset() in contexts where IRQs are disabled and as such, can't hold the locks required to work with the connectors. Cc: stable at vger.kernel.org Cc: Ville Syrjälä Acked-by: Daniel Vetter Signed-off-by: Lyude --- drivers/gpu/drm/i915/intel_crt.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index e115bcc..dfcd718 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -713,11 +713,11 @@ static int intel_crt_set_property(struct drm_connector *connector, return 0; } -static void intel_crt_reset(struct drm_connector *connector) +static void intel_crt_reset(struct drm_encoder *encoder) { - struct drm_device *dev = connector->dev; + struct drm_device *dev = encoder->dev; struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crt *crt = intel_attached_crt(connector); + struct intel_crt *crt = intel_encoder_to_crt(to_intel_encoder(encoder)); if (INTEL_INFO(dev)->gen >= 5) { u32 adpa; @@ -739,7 +739,6 @@ static void intel_crt_reset(struct drm_connector *connector) */ static const struct drm_connector_funcs intel_crt_connector_funcs = { - .reset = intel_crt_reset, .dpms = drm_atomic_helper_connector_dpms, .detect = intel_crt_detect, .fill_modes = drm_helper_probe_single_connector_modes, @@ -757,6 +756,7 @@ static const struct drm_connector_helper_funcs intel_crt_connector_helper_funcs }; static const struct drm_encoder_funcs intel_crt_enc_funcs = { + .reset = intel_crt_reset, .destroy = intel_encoder_destroy, }; @@ -901,5 +901,5 @@ void intel_crt_init(struct drm_device *dev) dev_priv->fdi_rx_config = I915_READ(FDI_RX_CTL(PIPE_A)) & fdi_config; } - intel_crt_reset(connector); + intel_crt_reset(&crt->base.base); } -- 2.5.5
[PATCH v3 0/4] Fixes for HPD
This is a revised version of the patchset: https://lists.freedesktop.org/archives/intel-gfx/2016-June/098787.html This patchset is intended to fix the issue of not having HPD when we're in runtime suspend, or on Valleyview/Cherryview systems when we don't have any power wells enabled. While this isn't the best for RPM, until we have the infrastructure to provide hpd while in runtime suspend this will have to suffice since not having HPD at all times means a user could potentially boot their system with no displays connected, then never have any displays they connect afterwards get detected. Unlike the previous patchset, this patchset includes a fix for the VGA detect loops we were seeing on Valleyview. On my setup, this fix seems to have fixed the loop entirely. As it turns out, I think our original understanding of this issue wasn't entirely accurate. Originally it had been assumed that the HPD signals we were getting from the ADPA were a result of turning the power wells on and off. As it turns out, the HPD signals actually don't get fired off until we do the force detect on the ADPA, not when we reset it. This means this problem can easily be solved just by temporarily disabling HPD on the ADPA whenever we do a force detect. As such, "drm/i915/vlv: Disable HPD in valleyview_crt_detect_hotplug()" deprecates Ville's previous two fixes Cc: stable at vger.kernel.org Cc: Ville Syrjälä Cc: Daniel Vetter Lyude (4): drm/i915/vlv: Make intel_crt_reset() per-encoder drm/i915/vlv: Reset the ADPA in vlv_display_power_well_init() drm/i915/vlv: Disable HPD in valleyview_crt_detect_hotplug() drm/i915: Enable polling when we don't have hpd drivers/gpu/drm/i915/i915_drv.c | 7 +- drivers/gpu/drm/i915/i915_drv.h | 6 ++ drivers/gpu/drm/i915/intel_crt.c| 28 ++-- drivers/gpu/drm/i915/intel_drv.h| 4 +- drivers/gpu/drm/i915/intel_hotplug.c| 112 +--- drivers/gpu/drm/i915/intel_runtime_pm.c | 10 +++ 6 files changed, 150 insertions(+), 17 deletions(-) -- 2.5.5
[PATCH 7/7] drm/rcar-du: Remove redundant calls to drm_connector_register_all()
Hi Chris, Thank you for the patch. On Friday 17 Jun 2016 09:25:23 Chris Wilson wrote: > Up to now, the recommendation was for drivers to call drm_dev_register() > followed by drm_connector_register_all(). Now that > drm_connector_register() is safe against multiple invocations, we can > move drm_connector_register_all() to drm_dev_register() and not suffer > from any backwards compatibility issues with drivers not following the > more rigorous init ordering. > > Signed-off-by: Chris Wilson > Cc: Daniel Vetter > Cc: Laurent Pinchart > Cc: David Airlie > Cc: dri-devel at lists.freedesktop.org > Cc: linux-renesas-soc at vger.kernel.org Reviewed-by: Laurent Pinchart > --- > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 5 - > 1 file changed, 5 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 48ec4b6e8b26..36d390093c92 > 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > @@ -278,7 +278,6 @@ static int rcar_du_remove(struct platform_device *pdev) > struct rcar_du_device *rcdu = platform_get_drvdata(pdev); > struct drm_device *ddev = rcdu->ddev; > > - drm_connector_unregister_all(ddev); > drm_dev_unregister(ddev); > > if (rcdu->fbdev) > @@ -360,10 +359,6 @@ static int rcar_du_probe(struct platform_device *pdev) > if (ret) > goto error; > > - ret = drm_connector_register_all(ddev); > - if (ret < 0) > - goto error; > - > DRM_INFO("Device %s probed\n", dev_name(&pdev->dev)); > > return 0; -- Regards, Laurent Pinchart
[PATCH 26/27] drm/crtc-helper: disable_unused_functions really isn't for atomic
Hi Daniel, On Thursday 09 Jun 2016 10:26:44 Daniel Vetter wrote: > On Thu, Jun 09, 2016 at 01:36:30AM +0300, Laurent Pinchart wrote: > > On Wednesday 08 Jun 2016 14:19:18 Daniel Vetter wrote: > >> Rockchip just blew up here on testing, because I removed some "is this > >> crtc already disabled/enabled" state tracking from callbacks (not needed > >> with atomic). Turns out that was needed to work around rockchip still > >> calling legacy helper code. > >> > >> Since me explaining on irc/mailing-list plus kerneldoc isn't enough, > >> be more verbose and add dmesg output. Not that anyone actually reads > >> that, > >> either. > > > > How about also removing it from atomic drivers then ? At least omapdrm > > calls this function. > > I tried, and thought I caught them all. See preceeding sti patch. Not sure > why I missed it, will remedy. With the call removed from drivers, Acked-by: Laurent Pinchart > >> Cc: Tomeu Vizoso > >> Signed-off-by: Daniel Vetter > >> --- > >> > >> drivers/gpu/drm/drm_crtc_helper.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/drm_crtc_helper.c > >> b/drivers/gpu/drm/drm_crtc_helper.c index a6e42433ef0e..b47ec24939a0 > >> 100644 > >> --- a/drivers/gpu/drm/drm_crtc_helper.c > >> +++ b/drivers/gpu/drm/drm_crtc_helper.c > >> @@ -232,6 +232,9 @@ static void > >> __drm_helper_disable_unused_functions(struct drm_device *dev) */ > >> > >> void drm_helper_disable_unused_functions(struct drm_device *dev) > >> { > >> + if (drm_core_check_feature(dev, DRIVER_ATOMIC)) > >> + DRM_ERROR("Called for atomic driver, this is not what you > >> want.\n"); > >> + > >>drm_modeset_lock_all(dev); > >>__drm_helper_disable_unused_functions(dev); > >>drm_modeset_unlock_all(dev); -- Regards, Laurent Pinchart
[PATCH 2/3] drm/vc4: Remove open-coded drm_connector_register_all()
On Tue, Jun 21, 2016 at 10:28:02AM +0100, Chris Wilson wrote: > drm_dev_register() will now register all known connectors, so we no > longer have to do so manually. > > Signed-off-by: Chris Wilson > Cc: Eric Anholt > Cc: David Airlie > Cc: Daniel Vetter > Cc: dri-devel at lists.freedesktop.org > --- > drivers/gpu/drm/vc4/vc4_drv.c | 12 > 1 file changed, 12 deletions(-) > > diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c > index 98cf2c4d622e..1cd6b7b36241 100644 > --- a/drivers/gpu/drm/vc4/vc4_drv.c > +++ b/drivers/gpu/drm/vc4/vc4_drv.c > @@ -211,22 +211,10 @@ static int vc4_drm_bind(struct device *dev) > if (ret < 0) > goto unbind_all; > > - /* Connector registration has to occur after DRM device > - * registration, because it creates sysfs entries based on the > - * DRM device. > - */ > - list_for_each_entry(connector, &drm->mode_config.connector_list, head) { > - ret = drm_connector_register(connector); > - if (ret) > - goto unregister; > - } > - > vc4_kms_load(drm); Semi-related: This should be called before drm_dev_register, with this ordering userspace might be able to peak at a not-yet set up drm device. -Daniel > > return 0; > > -unregister: > - drm_dev_unregister(drm); > unbind_all: > component_unbind_all(dev, drm); > gem_destroy: > -- > 2.8.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 26/27] drm/crtc-helper: disable_unused_functions really isn't for atomic
On Tue, Jun 21, 2016 at 12:12:54PM +0300, Laurent Pinchart wrote: > Hi Daniel, > > On Thursday 09 Jun 2016 10:26:44 Daniel Vetter wrote: > > On Thu, Jun 09, 2016 at 01:36:30AM +0300, Laurent Pinchart wrote: > > > On Wednesday 08 Jun 2016 14:19:18 Daniel Vetter wrote: > > >> Rockchip just blew up here on testing, because I removed some "is this > > >> crtc already disabled/enabled" state tracking from callbacks (not needed > > >> with atomic). Turns out that was needed to work around rockchip still > > >> calling legacy helper code. > > >> > > >> Since me explaining on irc/mailing-list plus kerneldoc isn't enough, > > >> be more verbose and add dmesg output. Not that anyone actually reads > > >> that, > > >> either. > > > > > > How about also removing it from atomic drivers then ? At least omapdrm > > > calls this function. > > > > I tried, and thought I caught them all. See preceeding sti patch. Not sure > > why I missed it, will remedy. > > With the call removed from drivers, > > Acked-by: Laurent Pinchart Applied to drm-misc, thanks for double-checking. -Daniel > > > >> Cc: Tomeu Vizoso > > >> Signed-off-by: Daniel Vetter > > >> --- > > >> > > >> drivers/gpu/drm/drm_crtc_helper.c | 3 +++ > > >> 1 file changed, 3 insertions(+) > > >> > > >> diff --git a/drivers/gpu/drm/drm_crtc_helper.c > > >> b/drivers/gpu/drm/drm_crtc_helper.c index a6e42433ef0e..b47ec24939a0 > > >> 100644 > > >> --- a/drivers/gpu/drm/drm_crtc_helper.c > > >> +++ b/drivers/gpu/drm/drm_crtc_helper.c > > >> @@ -232,6 +232,9 @@ static void > > >> __drm_helper_disable_unused_functions(struct drm_device *dev) */ > > >> > > >> void drm_helper_disable_unused_functions(struct drm_device *dev) > > >> { > > >> +if (drm_core_check_feature(dev, DRIVER_ATOMIC)) > > >> +DRM_ERROR("Called for atomic driver, this is not what > > >> you > > >> want.\n"); > > >> + > > >> drm_modeset_lock_all(dev); > > >> __drm_helper_disable_unused_functions(dev); > > >> drm_modeset_unlock_all(dev); > > -- > Regards, > > Laurent Pinchart > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH] arm64: dts: Add display subsystem DT nodes for hi6220-hikey
Add ade and dsi DT nodes for hikey board. The binding docs were acked by Rob Herring in this thread: https://lists.freedesktop.org/archives/dri-devel/2016-March/102135.html Signed-off-by: Xinliang Liu --- arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 8 arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 54 ++ 2 files changed, 62 insertions(+) diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts index e92a30c87a82..90e77380f073 100644 --- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts +++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts @@ -229,3 +229,11 @@ &uart3 { label = "LS-UART1"; }; + +&ade { + status = "ok"; +}; + +&dsi { + status = "ok"; +}; diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi index 0fb84814ded2..c53c9db94248 100644 --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi @@ -269,6 +269,11 @@ mboxes = <&mailbox 1 0 11>; }; + medianoc_ade: medianoc_ade at f452 { + compatible = "syscon"; + reg = <0x0 0xf452 0x0 0x4000>; + }; + uart0: uart at f8015000 { /* console */ compatible = "arm,pl011", "arm,primecell"; reg = <0x0 0xf8015000 0x0 0x1000>; @@ -833,5 +838,54 @@ }; }; }; + + ade: ade at f410 { + compatible = "hisilicon,hi6220-ade"; + reg = <0x0 0xf410 0x0 0x7800>; + reg-names = "ade_base"; + hisilicon,noc-syscon = <&medianoc_ade>; + resets = <&media_ctrl MEDIA_ADE>; + interrupts = <0 115 4>; /* ldi interrupt */ + + clocks = <&media_ctrl HI6220_ADE_CORE>, +<&media_ctrl HI6220_CODEC_JPEG>, +<&media_ctrl HI6220_ADE_PIX_SRC>; + /*clock name*/ + clock-names = "clk_ade_core", + "clk_codec_jpeg", + "clk_ade_pix"; + + assigned-clocks = <&media_ctrl HI6220_ADE_CORE>, + <&media_ctrl HI6220_CODEC_JPEG>; + assigned-clock-rates = <36000>, <28800>; + status = "disabled"; + + port { + ade_out: endpoint { + remote-endpoint = <&dsi_in>; + }; + }; + }; + + dsi: dsi at f4107800 { + compatible = "hisilicon,hi6220-dsi"; + reg = <0x0 0xf4107800 0x0 0x100>; + clocks = <&media_ctrl HI6220_DSI_PCLK>; + clock-names = "pclk"; + status = "disabled"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + /* 0 for input port */ + port at 0 { + reg = <0>; + dsi_in: endpoint { + remote-endpoint = <&ade_out>; + }; + }; + }; + }; }; }; -- 2.8.3
i915 and Intel NUC DN2820FYK
On Mon, 20 Jun 2016, Aristeu Rozanski wrote: > while trying to use the NUC DN2820FYK [1], screen goes black as soon > i915 is loaded. The NUC is connected by HDMI to a 1920x1080 display. > Works normally while on the BIOS (upgraded to the latest one just in > case today) and while on grub. Here's what I tried: Please file a bug at [1], drop all the extra debug params, but add drm.debug=14, and attach dmesg all the way from boot. BR, Jani. [1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI&component=DRM/Intel -- Jani Nikula, Intel Open Source Technology Center
[PATCH v2 1/3] drm: Add callbacks for late registering
On Tue, Jun 21, 2016 at 11:31:34AM +0200, Benjamin Gaignard wrote: > Like what has been done for connectors add callbacks on encoder, > crtc and plane to let driver do actions after drm device registration. > > Correspondingly, add callbacks called before unregister drm device. > > version 2: > add drm_modeset_register_all() and drm_modeset_unregister_all() > to centralize all calls > > Signed-off-by: Benjamin Gaignard > --- > drivers/gpu/drm/drm_crtc.c | 122 > + > drivers/gpu/drm/drm_drv.c | 4 +- > include/drm/drm_crtc.h | 79 + > 3 files changed, 203 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index e7c862b..b393feb 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -608,6 +608,31 @@ static unsigned int drm_num_crtcs(struct drm_device *dev) > return num; > } > > +static int drm_crtc_register_all(struct drm_device *dev) > +{ > + struct drm_crtc *crtc; > + int ret; > + > + drm_for_each_crtc(crtc, dev) { > + if (crtc->funcs->late_register) > + ret = crtc->funcs->late_register(crtc); > + if (ret) ret is uninitialised. It is a good question (probably for another series) on what exactly to do on registration failure? At the very least we need to unwind on the error path, or we just ignore errors (other than a DRM_ERROR to userspace explaining why one object is not available, but otherwise let the driver load). > +int drm_modeset_register_all(struct drm_device *dev) > +{ > + int ret; > + > + ret = drm_plane_register_all(dev); > + if (ret) > + return ret; > + > + ret = drm_crtc_register_all(dev); > + if (ret) > + return ret; > + > + ret = drm_encoder_register_all(dev); > + if (ret) > + return ret; > + > + ret = drm_connector_register_all(dev); Might as well continue on with ret = <> if (ret) return ret; for a consistent style. Along with the comment about how should we be handling errors? If we report an error, everything up to that point should be unwound. > + return ret; > +} > +EXPORT_SYMBOL(drm_modeset_register_all); > + > +/** > + * drm_modeset_unregister_all - do early unregistration > + * @dev: drm device > + * > + * This function call early_unregister callbakc for all planes, > + * crtcs, encoders and connectors > + */ > +void drm_modeset_unregister_all(struct drm_device *dev) > +{ > + drm_plane_unregister_all(dev); > + drm_crtc_unregister_all(dev); > + drm_encoder_unregister_all(dev); > + drm_connector_unregister_all(dev); Unregister should be in the opposite order. > +} > +EXPORT_SYMBOL(drm_modeset_unregister_all); I think the plan is not to export these symbols, -Chris -- Chris Wilson, Intel Open Source Technology Centre
[PATCH v2 3/3] drm: sti: rework init sequence
Use drm_dev_alloc() and drm_dev_register() instead of .load() To simplify init sequence only create fbdev when requested in output_poll_changed(). version 2: remove call to drm_connector_unregister_all() and drm_dev_set_unique() Signed-off-by: Benjamin Gaignard --- drivers/gpu/drm/sti/sti_drv.c | 137 + drivers/gpu/drm/sti/sti_drv.h | 1 + drivers/gpu/drm/sti/sti_dvo.c | 7 --- drivers/gpu/drm/sti/sti_hda.c | 8 +-- drivers/gpu/drm/sti/sti_hdmi.c | 21 +-- 5 files changed, 100 insertions(+), 74 deletions(-) diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c index 26aa85d..96bd3d0 100644 --- a/drivers/gpu/drm/sti/sti_drv.c +++ b/drivers/gpu/drm/sti/sti_drv.c @@ -226,8 +226,28 @@ static int sti_atomic_commit(struct drm_device *drm, return 0; } +static void sti_output_poll_changed(struct drm_device *ddev) +{ + struct sti_private *private = ddev->dev_private; + + if (!ddev->mode_config.num_connector) + return; + + if (private->fbdev) { + drm_fbdev_cma_hotplug_event(private->fbdev); + return; + } + + private->fbdev = drm_fbdev_cma_init(ddev, 32, + ddev->mode_config.num_crtc, + ddev->mode_config.num_connector); + if (IS_ERR(private->fbdev)) + private->fbdev = NULL; +} + static const struct drm_mode_config_funcs sti_mode_config_funcs = { .fb_create = drm_fb_cma_create, + .output_poll_changed = sti_output_poll_changed, .atomic_check = drm_atomic_helper_check, .atomic_commit = sti_atomic_commit, }; @@ -248,44 +268,6 @@ static void sti_mode_config_init(struct drm_device *dev) dev->mode_config.funcs = &sti_mode_config_funcs; } -static int sti_load(struct drm_device *dev, unsigned long flags) -{ - struct sti_private *private; - int ret; - - private = kzalloc(sizeof(*private), GFP_KERNEL); - if (!private) { - DRM_ERROR("Failed to allocate private\n"); - return -ENOMEM; - } - dev->dev_private = (void *)private; - private->drm_dev = dev; - - mutex_init(&private->commit.lock); - INIT_WORK(&private->commit.work, sti_atomic_work); - - drm_mode_config_init(dev); - drm_kms_helper_poll_init(dev); - - sti_mode_config_init(dev); - - ret = component_bind_all(dev->dev, dev); - if (ret) { - drm_kms_helper_poll_fini(dev); - drm_mode_config_cleanup(dev); - kfree(private); - return ret; - } - - drm_mode_config_reset(dev); - - drm_fbdev_cma_init(dev, 32, - dev->mode_config.num_crtc, - dev->mode_config.num_connector); - - return 0; -} - static const struct file_operations sti_driver_fops = { .owner = THIS_MODULE, .open = drm_open, @@ -302,7 +284,6 @@ static const struct file_operations sti_driver_fops = { static struct drm_driver sti_driver = { .driver_features = DRIVER_HAVE_IRQ | DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | DRIVER_ATOMIC, - .load = sti_load, .gem_free_object_unlocked = drm_gem_cma_free_object, .gem_vm_ops = &drm_gem_cma_vm_ops, .dumb_create = drm_gem_cma_dumb_create, @@ -339,14 +320,88 @@ static int compare_of(struct device *dev, void *data) return dev->of_node == data; } +static int sti_init(struct drm_device *ddev) +{ + struct sti_private *private; + + private = kzalloc(sizeof(*private), GFP_KERNEL); + if (!private) + return -ENOMEM; + + ddev->dev_private = (void *)private; + dev_set_drvdata(ddev->dev, ddev); + private->drm_dev = ddev; + + mutex_init(&private->commit.lock); + INIT_WORK(&private->commit.work, sti_atomic_work); + + drm_mode_config_init(ddev); + + sti_mode_config_init(ddev); + + drm_kms_helper_poll_init(ddev); + + return 0; +} + +static void sti_cleanup(struct drm_device *ddev) +{ + struct sti_private *private = ddev->dev_private; + + if (private->fbdev) { + drm_fbdev_cma_fini(private->fbdev); + private->fbdev = NULL; + } + + drm_kms_helper_poll_fini(ddev); + drm_vblank_cleanup(ddev); + kfree(private); + ddev->dev_private = NULL; +} + static int sti_bind(struct device *dev) { - return drm_platform_init(&sti_driver, to_platform_device(dev)); + struct drm_device *ddev; + int ret; + + ddev = drm_dev_alloc(&sti_driver, dev); + if (!ddev) + return -ENOMEM; + + ddev->platformdev = to_platform_device(dev); + + ret = sti_init(ddev); + if (ret) + goto err_drm_dev_unref; + + ret = component_bind_all(ddev->dev, ddev); + if (ret) + goto err
[PATCH v2 2/3] drm: sti: use late_register and early_unregister callbacks
Make sti driver use register callback to move debugfs initialization out of sub-components creation. This will allow to convert driver .load() to drm_dev_alloc() and drm_dev_register(). sti_compositor bring up 2 crtc but only one debugfs init is needed so use drm_crtc_index to do it on the first one. This can't be done in sti_drv because only sti_compositor have access to the devices. It is almost the same for sti_encoder which handle multiple encoder while one only debugfs entry is needed so add a boolean to avoid multiple debugfs initialization Signed-off-by: Benjamin Gaignard --- drivers/gpu/drm/sti/sti_compositor.c | 20 drivers/gpu/drm/sti/sti_compositor.h | 3 +++ drivers/gpu/drm/sti/sti_crtc.c | 12 drivers/gpu/drm/sti/sti_cursor.c | 32 drivers/gpu/drm/sti/sti_dvo.c| 18 ++ drivers/gpu/drm/sti/sti_gdp.c| 32 drivers/gpu/drm/sti/sti_hda.c| 18 ++ drivers/gpu/drm/sti/sti_hdmi.c | 19 +++ drivers/gpu/drm/sti/sti_hqvdp.c | 32 drivers/gpu/drm/sti/sti_mixer.c | 5 + drivers/gpu/drm/sti/sti_mixer.h | 2 ++ drivers/gpu/drm/sti/sti_plane.c | 24 +++- drivers/gpu/drm/sti/sti_plane.h | 7 +-- drivers/gpu/drm/sti/sti_tvout.c | 36 ++-- drivers/gpu/drm/sti/sti_vid.c| 5 + drivers/gpu/drm/sti/sti_vid.h| 2 ++ 16 files changed, 198 insertions(+), 69 deletions(-) diff --git a/drivers/gpu/drm/sti/sti_compositor.c b/drivers/gpu/drm/sti/sti_compositor.c index 3d2fa3a..794148f 100644 --- a/drivers/gpu/drm/sti/sti_compositor.c +++ b/drivers/gpu/drm/sti/sti_compositor.c @@ -55,6 +55,26 @@ struct sti_compositor_data stih416_compositor_data = { }, }; +int sti_compositor_debufs_init(struct sti_compositor *compo, + struct drm_minor *minor) +{ + int ret = 0, i; + + for (i = 0; compo->vid[i]; i++) { + ret = vid_debugfs_init(compo->vid[i], minor); + if (ret) + return ret; + } + + for (i = 0; compo->mixer[i]; i++) { + ret = sti_mixer_debugfs_init(compo->mixer[i], minor); + if (ret) + return ret; + } + + return 0; +} + static int sti_compositor_bind(struct device *dev, struct device *master, void *data) diff --git a/drivers/gpu/drm/sti/sti_compositor.h b/drivers/gpu/drm/sti/sti_compositor.h index 1a4a73d..2ef 100644 --- a/drivers/gpu/drm/sti/sti_compositor.h +++ b/drivers/gpu/drm/sti/sti_compositor.h @@ -81,4 +81,7 @@ struct sti_compositor { struct notifier_block vtg_vblank_nb; }; +int sti_compositor_debufs_init(struct sti_compositor *compo, + struct drm_minor *minor); + #endif diff --git a/drivers/gpu/drm/sti/sti_crtc.c b/drivers/gpu/drm/sti/sti_crtc.c index e04deed..7fab3af 100644 --- a/drivers/gpu/drm/sti/sti_crtc.c +++ b/drivers/gpu/drm/sti/sti_crtc.c @@ -331,6 +331,17 @@ void sti_crtc_disable_vblank(struct drm_device *drm_dev, unsigned int pipe) } } +static int sti_crtc_late_register(struct drm_crtc *crtc) +{ + struct sti_mixer *mixer = to_sti_mixer(crtc); + struct sti_compositor *compo = dev_get_drvdata(mixer->dev); + + if (drm_crtc_index(crtc) == 0) + return sti_compositor_debufs_init(compo, crtc->dev->primary); + + return 0; +} + static const struct drm_crtc_funcs sti_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, @@ -339,6 +350,7 @@ static const struct drm_crtc_funcs sti_crtc_funcs = { .reset = drm_atomic_helper_crtc_reset, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, + .late_register = sti_crtc_late_register, }; bool sti_crtc_is_main(struct drm_crtc *crtc) diff --git a/drivers/gpu/drm/sti/sti_cursor.c b/drivers/gpu/drm/sti/sti_cursor.c index 53aa002..a263bbb 100644 --- a/drivers/gpu/drm/sti/sti_cursor.c +++ b/drivers/gpu/drm/sti/sti_cursor.c @@ -329,6 +329,33 @@ static const struct drm_plane_helper_funcs sti_cursor_helpers_funcs = { .atomic_disable = sti_cursor_atomic_disable, }; +static void sti_cursor_destroy(struct drm_plane *drm_plane) +{ + DRM_DEBUG_DRIVER("\n"); + + drm_plane_helper_disable(drm_plane); + drm_plane_cleanup(drm_plane); +} + +static int sti_cursor_late_register(struct drm_plane *drm_plane) +{ + struct sti_plane *plane = to_sti_plane(drm_plane); + struct sti_cursor *cursor = to_sti_cursor(plane); + + return cursor_debugfs_init(cursor, drm_plane->dev->primary); +} + +struct drm_plane_funcs sti_cursor_plane_he
[PATCH v2 1/3] drm: Add callbacks for late registering
Like what has been done for connectors add callbacks on encoder, crtc and plane to let driver do actions after drm device registration. Correspondingly, add callbacks called before unregister drm device. version 2: add drm_modeset_register_all() and drm_modeset_unregister_all() to centralize all calls Signed-off-by: Benjamin Gaignard --- drivers/gpu/drm/drm_crtc.c | 122 + drivers/gpu/drm/drm_drv.c | 4 +- include/drm/drm_crtc.h | 79 + 3 files changed, 203 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index e7c862b..b393feb 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -608,6 +608,31 @@ static unsigned int drm_num_crtcs(struct drm_device *dev) return num; } +static int drm_crtc_register_all(struct drm_device *dev) +{ + struct drm_crtc *crtc; + int ret; + + drm_for_each_crtc(crtc, dev) { + if (crtc->funcs->late_register) + ret = crtc->funcs->late_register(crtc); + if (ret) + return ret; + } + + return 0; +} + +static void drm_crtc_unregister_all(struct drm_device *dev) +{ + struct drm_crtc *crtc; + + drm_for_each_crtc(crtc, dev) { + if (crtc->funcs->early_unregister) + crtc->funcs->early_unregister(crtc); + } +} + /** * drm_crtc_init_with_planes - Initialise a new CRTC object with *specified primary and cursor planes. @@ -1102,6 +1127,31 @@ void drm_connector_unregister_all(struct drm_device *dev) } EXPORT_SYMBOL(drm_connector_unregister_all); +static int drm_encoder_register_all(struct drm_device *dev) +{ + struct drm_encoder *encoder; + int ret; + + drm_for_each_encoder(encoder, dev) { + if (encoder->funcs->late_register) + ret = encoder->funcs->late_register(encoder); + if (ret) + return ret; + } + + return 0; +} + +static void drm_encoder_unregister_all(struct drm_device *dev) +{ + struct drm_encoder *encoder; + + drm_for_each_encoder(encoder, dev) { + if (encoder->funcs->early_unregister) + encoder->funcs->early_unregister(encoder); + } +} + /** * drm_encoder_init - Init a preallocated encoder * @dev: drm device @@ -1290,6 +1340,31 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane, } EXPORT_SYMBOL(drm_universal_plane_init); +static int drm_plane_register_all(struct drm_device *dev) +{ + struct drm_plane *plane; + int ret; + + drm_for_each_plane(plane, dev) { + if (plane->funcs->late_register) + ret = plane->funcs->late_register(plane); + if (ret) + return ret; + } + + return 0; +} + +static void drm_plane_unregister_all(struct drm_device *dev) +{ + struct drm_plane *plane; + + drm_for_each_plane(plane, dev) { + if (plane->funcs->early_unregister) + plane->funcs->early_unregister(plane); + } +} + /** * drm_plane_init - Initialize a legacy plane * @dev: DRM device @@ -1412,6 +1487,53 @@ void drm_plane_force_disable(struct drm_plane *plane) } EXPORT_SYMBOL(drm_plane_force_disable); +/** + * drm_modeset_register_all - do late registration + * @dev: drm device + * + * This function call late_register callback for all planes, + * crcts, encoders and connectors + * + * Returns: + * Zero on success, erro code on failure + */ +int drm_modeset_register_all(struct drm_device *dev) +{ + int ret; + + ret = drm_plane_register_all(dev); + if (ret) + return ret; + + ret = drm_crtc_register_all(dev); + if (ret) + return ret; + + ret = drm_encoder_register_all(dev); + if (ret) + return ret; + + ret = drm_connector_register_all(dev); + return ret; +} +EXPORT_SYMBOL(drm_modeset_register_all); + +/** + * drm_modeset_unregister_all - do early unregistration + * @dev: drm device + * + * This function call early_unregister callbakc for all planes, + * crtcs, encoders and connectors + */ +void drm_modeset_unregister_all(struct drm_device *dev) +{ + drm_plane_unregister_all(dev); + drm_crtc_unregister_all(dev); + drm_encoder_unregister_all(dev); + drm_connector_unregister_all(dev); +} +EXPORT_SYMBOL(drm_modeset_unregister_all); + static int drm_mode_create_standard_properties(struct drm_device *dev) { struct drm_property *prop; diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index c7101c0..53eadca 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -688,7 +688,7 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) } if (drm_core_che
[PATCH v2 0/3] Add callbacks for late registering
version 2: create functions drm_modeset_register_all and drm_modeset_unregister_all to regroup all callbacks calls to avoid loops into drm_dev_register and drm_dev_unregister. Call order is now planes, crtcs, encoders and connectors Fix sti driver to not call drm_connector_register_all and drm_dev_set_unique version 1: late_register() and early_unregister() callbacks have been introduced for connectors, let do the same for encoders, crcts and planes. Like for the previously introduced functions they are called right after drm device registration and before unregistration. Those new callbacks allow to defer actions after drm_dev_register(). For example sti driver use them to initialize debugfs because it require to have register a drm device to get the root debug file. Benjamin Gaignard (3): drm: Add callbacks for late registering drm: sti: use late_register and early_unregister callbacks drm: sti: rework init sequence drivers/gpu/drm/drm_crtc.c | 122 +++ drivers/gpu/drm/drm_drv.c| 4 +- drivers/gpu/drm/sti/sti_compositor.c | 20 + drivers/gpu/drm/sti/sti_compositor.h | 3 + drivers/gpu/drm/sti/sti_crtc.c | 12 +++ drivers/gpu/drm/sti/sti_cursor.c | 32 +++- drivers/gpu/drm/sti/sti_drv.c| 137 --- drivers/gpu/drm/sti/sti_drv.h| 1 + drivers/gpu/drm/sti/sti_dvo.c| 25 +++ drivers/gpu/drm/sti/sti_gdp.c| 32 +++- drivers/gpu/drm/sti/sti_hda.c| 26 +++ drivers/gpu/drm/sti/sti_hdmi.c | 40 +- drivers/gpu/drm/sti/sti_hqvdp.c | 32 +++- drivers/gpu/drm/sti/sti_mixer.c | 5 +- drivers/gpu/drm/sti/sti_mixer.h | 2 + drivers/gpu/drm/sti/sti_plane.c | 24 +- drivers/gpu/drm/sti/sti_plane.h | 7 +- drivers/gpu/drm/sti/sti_tvout.c | 36 +++-- drivers/gpu/drm/sti/sti_vid.c| 5 +- drivers/gpu/drm/sti/sti_vid.h| 2 + include/drm/drm_crtc.h | 79 21 files changed, 501 insertions(+), 145 deletions(-) -- 1.9.1
[PATCH 02/20] clk: multiplier: Prevent the multiplier from under / over flowing
On Mon, Jun 20, 2016 at 01:50:30PM -0700, Michael Turquette wrote: > Quoting Maxime Ripard (2016-05-16 05:47:02) > > In the current multiplier base clock implementation, if the > > CLK_SET_RATE_PARENT flag isn't set, the code will not make sure that the > > multiplier computed remains within the boundaries of our clock. > > > > This means that if the clock we want to reach is below the parent rate, > > or if the multiplier is above the maximum that we can reach, we will end up > > with a completely bogus one that the clock cannot achieve. > > > > Fixes: f2e0a53271a4 ("clk: Add a basic multiplier clock") > > Signed-off-by: Maxime Ripard > > Applied. Thanks, but apparently you merged it in clk-next, while it should go in clk-fixes, shouldn't it? Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160621/d2365af8/attachment.sig>
[PATCH v4 0/8] iommu/rockchip: Fix bugs and enable on ARM64
On Tue, Jun 21, 2016 at 01:34:33PM +0900, Tomasz Figa wrote: > This series intends mostly to enable support for ARM64 architecture > in the rockchip-iommu driver. On the way to do so, some bugs are also > fixed. > > The most important changes here are: > - making the Rockchip IOMMU driver use DMA API for managing cache >coherency of page tables, > - making the Rockchip DRM driver not use DMA API on behalf of a virtual >device (behind a virtual IOMMU) to allocate and map buffers, but >instead proper DRM helpers and IOMMU API directly. Are these two parts dependent on each other or can the IOMMU and the DRM part merged independently? Joerg
[PATCH 10/10] drm: Drop mode_config.mutex from _reset()
Again, we now have a safe way to iterate over the connector list and can remove the locking. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 0a678cfd9920..1064a41ed38b 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -5416,11 +5416,9 @@ void drm_mode_config_reset(struct drm_device *dev) if (encoder->funcs->reset) encoder->funcs->reset(encoder); - mutex_lock(&dev->mode_config.mutex); drm_for_each_connector(connector, dev, iter) if (connector->funcs->reset) connector->funcs->reset(connector); - mutex_unlock(&dev->mode_config.mutex); } EXPORT_SYMBOL(drm_mode_config_reset); -- 2.8.1
[PATCH 09/10] drm: Drop mode_config.mutex from get_resources ioctl
The only thing this protected is the connector_list, which is now protected differently. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index d104717cab6b..0a678cfd9920 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1906,7 +1906,6 @@ int drm_mode_getresources(struct drm_device *dev, void *data, /* mode_config.mutex protects the connector list against e.g. DP MST * connector hot-adding. CRTC/Plane lists are invariant. */ - mutex_lock(&dev->mode_config.mutex); card_res->max_height = dev->mode_config.max_height; card_res->min_height = dev->mode_config.min_height; card_res->max_width = dev->mode_config.max_width; @@ -1958,7 +1957,6 @@ int drm_mode_getresources(struct drm_device *dev, void *data, card_res->count_connectors = connector_count; out: - mutex_unlock(&dev->mode_config.mutex); return ret; } -- 2.8.1
[PATCH 08/10] drm: Drop mode_config.mutex from connector_register_all
With the reworked connector_list locking we don't need this any more. Also, we can remove the FIXME comment from the unregister function, too, by using drm_for_each_connector. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index dc22c0bfbe99..d104717cab6b 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1107,20 +1107,15 @@ int drm_connector_register_all(struct drm_device *dev) struct drm_connector_iter iter; int ret; - mutex_lock(&dev->mode_config.mutex); - drm_for_each_connector(connector, dev, iter) { ret = drm_connector_register(connector); if (ret) goto err; } - mutex_unlock(&dev->mode_config.mutex); - return 0; err: - mutex_unlock(&dev->mode_config.mutex); drm_connector_unregister_all(dev); return ret; } @@ -1139,9 +1134,9 @@ EXPORT_SYMBOL(drm_connector_register_all); void drm_connector_unregister_all(struct drm_device *dev) { struct drm_connector *connector; + struct drm_connector_iter iter; - /* FIXME: taking the mode config mutex ends up in a clash with sysfs */ - list_for_each_entry(connector, &dev->mode_config.connector_list, head) + drm_for_each_connector(connector, dev, iter) drm_connector_unregister(connector); } EXPORT_SYMBOL(drm_connector_unregister_all); -- 2.8.1
[PATCH 07/10] drm: Revamp connector_list protection
This is a pretty good horror show, but I think it's the best tradeoff: - Thanks to srcu and delayed freeing the locking doesn't leak out to callers, hence no added headaches with locking inversions. - For core and drivers which hot-remove connectors all the connector list walking details are hidden in a macro. - Other drivers which don't care about hot-removing can simply keep on using the normal list walking macros. The new design is: - New dev->mode_config.connector_list_lock to protect the connector_list, and the connector_ida (since that's also unprotected), plus num_connectors. This is a pure leaf lock, nothing is allowed to nest within, nothing outside of connector init/cleanup will ever need it. - Protect connector_list itself with srcu. This allows sleeping and anything else. The only thing which is not allowed is calling synchronize_srcu, or grabbing any locks or waiting on waitqueues/completions/whatever which might call that. To make this 100% safe we opt to not have any calls to synchronize_srcu. - Protect against use-after-free of connectors using call_srcu, to delay the kfree enough. - To protect against zombie connectors which are in progress of final destruction use kref_get_unless_zero. This is safe since srcu prevents against use-after-free, and that's the only guarantee we need. For this demo I only bothered converting i915, and there also not everything - I left the connector loops in the modeset code unchanged since those will all be converted over to drm_for_each_connector_in_state to make them work correctly for nonblocking atomic commits. Only loops outside of atomic commits should be converted to drm_for_each_connector. Note that the i915 DP MST code still uses drm_modeset_lock_all(). But that's not because of drm core connector lifetime issues, but because the fbdev helper reuses core locks to mange its own lists and data structures. Thierry Reding has a patch series to gift fbdev its own lock, which will remedy this. v2: Totally rewrite everything to pack it up in a neat iterator macro, with init/check/next extracted into helpers. v3: Tiny rebase conflict. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c | 57 +- drivers/gpu/drm/i915/intel_dp_mst.c | 2 +- include/drm/drm_crtc.h | 82 +++-- 3 files changed, 109 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 6a8f91e8821b..dc22c0bfbe99 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -44,6 +44,9 @@ #include "drm_crtc_internal.h" #include "drm_internal.h" +DEFINE_SRCU(drm_connector_list_srcu); +EXPORT_SYMBOL(drm_connector_list_srcu); + static struct drm_framebuffer * internal_framebuffer_create(struct drm_device *dev, const struct drm_mode_fb_cmd2 *r, @@ -825,7 +828,7 @@ static void drm_connector_get_cmdline_mode(struct drm_connector *connector) mode->interlace ? " interlaced" : ""); } -static void drm_connector_free(struct kref *kref) +static void drm_connector_kref_release(struct kref *kref) { struct drm_connector *connector = container_of(kref, struct drm_connector, base.refcount); @@ -858,11 +861,10 @@ int drm_connector_init(struct drm_device *dev, struct ida *connector_ida = &drm_connector_enum_list[connector_type].ida; - drm_modeset_lock_all(dev); - + mutex_lock(&dev->mode_config.connector_list_lock); ret = drm_mode_object_get_reg(dev, &connector->base, DRM_MODE_OBJECT_CONNECTOR, - false, drm_connector_free); + false, drm_connector_kref_release); if (ret) goto out_unlock; @@ -899,11 +901,6 @@ int drm_connector_init(struct drm_device *dev, drm_connector_get_cmdline_mode(connector); - /* We should add connectors at the end to avoid upsetting the connector -* index too much. */ - list_add_tail(&connector->head, &config->connector_list); - config->num_connector++; - if (connector_type != DRM_MODE_CONNECTOR_VIRTUAL) drm_object_attach_property(&connector->base, config->edid_property, @@ -917,6 +914,11 @@ int drm_connector_init(struct drm_device *dev, } connector->debugfs_entry = NULL; + + /* We must add the connector to the list last. */ + list_add_tail_rcu(&connector->head, &config->connector_list); + config->num_connector++; + out_put_type_id: if (ret) ida_remove(connector_ida, connector->connector_type_id); @@ -928,7 +930,7 @@ out_put: drm_mode_object_unregister(dev, &connector->base); out_unlock: - drm_modeset_unlock_all(dev); + mutex_unlock(&dev->mode_config.connect
[PATCH 06/10] drm: Drop cargo-culted modeset_lock_all from encoder/plane init/cleanup
We can't hotplug encoders/planes, there's no point in that locking. It was also inconsistent because lacking from plane_init. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c | 11 +-- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 2a927488ec26..6a8f91e8821b 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1131,11 +1131,9 @@ int drm_encoder_init(struct drm_device *dev, { int ret; - drm_modeset_lock_all(dev); - ret = drm_mode_object_get(dev, &encoder->base, DRM_MODE_OBJECT_ENCODER); if (ret) - goto out_unlock; + return ret; encoder->dev = dev; encoder->encoder_type = encoder_type; @@ -1163,9 +1161,6 @@ out_put: if (ret) drm_mode_object_unregister(dev, &encoder->base); -out_unlock: - drm_modeset_unlock_all(dev); - return ret; } EXPORT_SYMBOL(drm_encoder_init); @@ -1185,12 +1180,10 @@ void drm_encoder_cleanup(struct drm_encoder *encoder) * the indices on the drm_encoder after us in the encoder_list. */ - drm_modeset_lock_all(dev); drm_mode_object_unregister(dev, &encoder->base); kfree(encoder->name); list_del(&encoder->head); dev->mode_config.num_encoder--; - drm_modeset_unlock_all(dev); memset(encoder, 0, sizeof(*encoder)); } @@ -1341,7 +1334,6 @@ void drm_plane_cleanup(struct drm_plane *plane) { struct drm_device *dev = plane->dev; - drm_modeset_lock_all(dev); kfree(plane->format_types); drm_mode_object_unregister(dev, &plane->base); @@ -1356,7 +1348,6 @@ void drm_plane_cleanup(struct drm_plane *plane) dev->mode_config.num_total_plane--; if (plane->type == DRM_PLANE_TYPE_OVERLAY) dev->mode_config.num_overlay_plane--; - drm_modeset_unlock_all(dev); WARN_ON(plane->state && !plane->funcs->atomic_destroy_state); if (plane->state && plane->funcs->atomic_destroy_state) -- 2.8.1
[PATCH 05/10] drm/i915: Roll out drm_for_each_connector in intel_hotplug.c
Just prep work to for the revamped connector_list locking. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_hotplug.c | 16 +--- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c index 38eeca7a6e72..e537c03ef6ee 100644 --- a/drivers/gpu/drm/i915/intel_hotplug.c +++ b/drivers/gpu/drm/i915/intel_hotplug.c @@ -145,16 +145,16 @@ static bool intel_hpd_irq_storm_detect(struct drm_i915_private *dev_priv, static void intel_hpd_irq_storm_disable(struct drm_i915_private *dev_priv) { struct drm_device *dev = dev_priv->dev; - struct drm_mode_config *mode_config = &dev->mode_config; struct intel_connector *intel_connector; struct intel_encoder *intel_encoder; struct drm_connector *connector; enum hpd_pin pin; bool hpd_disabled = false; + struct drm_connector_iter iter; assert_spin_locked(&dev_priv->irq_lock); - list_for_each_entry(connector, &mode_config->connector_list, head) { + drm_for_each_connector(connector, dev, iter) { if (connector->polled != DRM_CONNECTOR_POLL_HPD) continue; @@ -192,7 +192,7 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work) container_of(work, typeof(*dev_priv), hotplug.reenable_work.work); struct drm_device *dev = dev_priv->dev; - struct drm_mode_config *mode_config = &dev->mode_config; + struct drm_connector_iter iter; int i; intel_runtime_pm_get(dev_priv); @@ -206,7 +206,7 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work) dev_priv->hotplug.stats[i].state = HPD_ENABLED; - list_for_each_entry(connector, &mode_config->connector_list, head) { + drm_for_each_connector(connector, dev, iter) { struct intel_connector *intel_connector = to_intel_connector(connector); if (intel_connector->encoder->hpd_pin == i) { @@ -309,6 +309,7 @@ static void i915_hotplug_work_func(struct work_struct *work) struct drm_connector *connector; bool changed = false; u32 hpd_event_bits; + struct drm_connector_iter iter; mutex_lock(&mode_config->mutex); DRM_DEBUG_KMS("running encoder hotplug functions\n"); @@ -323,7 +324,7 @@ static void i915_hotplug_work_func(struct work_struct *work) spin_unlock_irq(&dev_priv->irq_lock); - list_for_each_entry(connector, &mode_config->connector_list, head) { + drm_for_each_connector(connector, dev, iter) { intel_connector = to_intel_connector(connector); if (!intel_connector->encoder) continue; @@ -456,15 +457,16 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv, void intel_hpd_init(struct drm_i915_private *dev_priv) { struct drm_device *dev = dev_priv->dev; - struct drm_mode_config *mode_config = &dev->mode_config; struct drm_connector *connector; + struct drm_connector_iter iter; int i; for_each_hpd_pin(i) { dev_priv->hotplug.stats[i].count = 0; dev_priv->hotplug.stats[i].state = HPD_ENABLED; } - list_for_each_entry(connector, &mode_config->connector_list, head) { + + drm_for_each_connector(connector, dev, iter) { struct intel_connector *intel_connector = to_intel_connector(connector); connector->polled = intel_connector->polled; -- 2.8.1