Re: [RFC PATCH v2 0/5] Add vblank hooks to struct drm_crtc_funcs
On Tue, Jan 24, 2017 at 08:55:35AM +0100, Daniel Vetter wrote: > On Sun, Jan 22, 2017 at 02:09:01PM +0800, Shawn Guo wrote: > > From: Shawn Guo> > > > The vblank is mostly CRTC specific and implemented as part of CRTC > > driver. The first patch adds 3 vblank core-driver hooks into struct > > drm_crtc_funcs, and wraps around core vblank handling code to use the > > new hooks for modern MODESET drivers and the ones in struct drm_driver > > as fallback for legacy drivers. > > > > The other patches in the series are to demonstrate how the new hooks > > are going to influence the driver code. There are more drivers than > > the ones included here can be converted. But before doing that, I would > > like to get some feedbacks first, expecially on how .get_vblank_counter > > should be converted when it's being drm_vblank_no_hw_counter(). > > > > .get_vblank_counter = drm_vblank_no_hw_counter > > I dropped some suggestions about this onto patch 3. Thanks for doing this, > I think it looks rather pretty. Just to check: Do you plan to resend this series with my comments on the first patch addressed? I really like this, so I guess you could drop the RFC part and then we give maintainers a week or two for reviews and land it into drm-misc for 4.12 ... -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 99687] Radeon RV790 Choppy cursor and framerates under xwayland
https://bugs.freedesktop.org/show_bug.cgi?id=99687 --- Comment #3 from Michel Dänzer--- FWIW, I did some more testing, and there does seem to be an issue in Xwayland, see bug 99702. I'm not sure if that explains all the issues you're seeing though. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/atmel-hlcdc: Simplify the HLCDC layer logic
On Mon, Feb 06, 2017 at 07:27:07PM +0100, Boris Brezillon wrote: > An HLCDC layers in Atmel's nomenclature is either a DRM plane or a 'Post > Processing Layer' which can be used to output the results of the HLCDC > composition in a memory buffer. > > atmel_hlcdc_layer.c was designed to be generic enough to be re-usable in > both cases, but we're not exposing the post-processing layer yet, and > even if we were, I'm not sure the code would provide the necessary tools > to manipulate this kind of layer. > > Moreover, the code in atmel_hlcdc_{plane,layer}.c was designed before the > atomic modesetting API, and was trying solve the > check-setting/commit-if-ok/rollback-otherwise problem, which is now > entirely solved by the existing core infrastructure. > > And finally, the code in atmel_hlcdc_layer.c in over-complicated compared > to what we really need. This rework is a good excuse to simplify it. Note > that this rework solves an existing resource leak (leading to a -EBUSY > error) which I failed to clearly identify. > > Signed-off-by: Boris Brezillon> --- > Hi Daniel, > > You might not remember, but this is something you asked me to do a while > ago, and it's finally there. > This patch reworks the Atmel HLCDC plane logic to get rid of all the > complexity in atmel_hlcdc_layer.c and this includes getting rid of > drm_flip_work, which you were trying to kill IIRC. [snip] > @@ -76,6 +77,27 @@ drm_plane_state_to_atmel_hlcdc_plane_state(struct > drm_plane_state *s) > return container_of(s, struct atmel_hlcdc_plane_state, base); > } > > +/** > + * Atmel HLCDC Plane. > + * > + * @base: base DRM plane structure > + * @desc: HLCDC layer desc structure > + * @properties: pointer to the property definitions structure > + * @regmap: HLCDC regmap > + */ > +struct atmel_hlcdc_plane { > + struct drm_plane base; > + const struct atmel_hlcdc_layer_desc *desc; So I'm not 100%, but it looks like you have a 1:1 relationship between atmel_hlcdc_plane and atmel_hlcdc_layer. I't go one step further even and merge these two structures together (or embed one into the other). Only reason against that would be if eventually you need to dynamically assign layers to planes (e.g. 2 layers for nv12 or something like that), which would mean you need to keep the pointer (and for dynamic assignement, move it into the plane_state). I didn't check the details of your patch really, but in general removing abstractions when we don't need them is a good idea. Acked-by: Daniel Vetter -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/7] drm/tinydrm: Add helper functions
On Mon, Feb 06, 2017 at 04:55:55PM -0600, Rob Herring wrote: > On Mon, Feb 6, 2017 at 5:08 AM, Thierry Reding> wrote: > > On Mon, Feb 06, 2017 at 11:07:42AM +0100, Daniel Vetter wrote: > >> On Mon, Feb 6, 2017 at 10:35 AM, Thierry Reding > >> wrote: > >> > >> > > > > +EXPORT_SYMBOL(tinydrm_disable_backlight); > >> > > > > +#endif > >> > > > > >> > > > These look like they really should be part of the backlight > >> > > > subsystem. > >> > I > >> > > > don't see anything DRM specific about them. Well, except for the > >> > > > error > >> > > > messages. > >> > > > >> > > So this is a bit an unpopular opinion with some folks, but I don't > >> > require > >> > > anyone to submit new code to subsystems outside of drm for new drivers. > >> > > Simply because it takes months to get stuff landed, and in general it's > >> > > not worth the trouble. > >> > > >> > "Not worth the trouble" is very subjective. If you look at the Linux > >> > kernel in general, one of the reasons why it works so well is because > >> > the changes we make apply to the kernel as a whole. Yes, sometimes that > >> > makes things more difficult and time-consuming, but it also means that > >> > the end result will be much more widely usable and therefore benefits > >> > everyone else in return. In my opinion that's a large part of why the > >> > kernel is so successful. > >> > > >> > > We have piles of stuff in drm and drm drivers that should be in core > >> > > but > >> > > isn't. > >> > > > >> > > Imo the only reasonable way is to merge as-is, then follow-up with a > >> > patch > >> > > series to move the helper into the right subsystem. Most often > >> > > unfortunately that follow-up patch series will just die. > >> > > >> > Of course follow-up series die. That's because nobody cares to follow-up > >> > once their code has been merged. > >> > > >> > Collecting our own helpers or variants of subsystems is a great way of > >> > isolating ourselves from the rest of the community. I don't think that's > >> > a good solution in the long run at all. > >> > > >> > >> We have a bunch of patch series that we resubmit for months and they go > >> exactly nowhere. They don't die because we stop caring, they die because > >> they die. Some of them we even need to constantly rebase and carry around > >> in drm-tip since our CI would Oops or spew WARNIGs all over the place. > >> There's simply some areas of the kernel which seem overloaded under patches > >> and no one is willing or able to fix things, and I can't fix the entire > >> kernel. Nor expect contributors (who have much less political weight to > >> throw around than me) to do that and succeed. And we don't end up with > >> worse code in the drm subsystem, since we can still do the refactoring > >> within drm helpers and end up with clean drivers. > >> > >> I fully agree that it's not great for the kernel's future, but when I'm > >> stuck with the option to get shit done or burning out playing the > >> upstreaming game, the choice is easy. And in the end I care about open > >> source gfx much more than the kernel, and I think for open source gfx's > >> success it's crucial that we're welcoming to new contributors and don't > >> throw up massive roadblocks. Open source gfx is tiny and still far away > >> from world domination, we need _lots_ more people. If that means routing > >> around other subsystems for them, I'm all for it. > > > > I can't say I fully agree with that sentiment. I do see how routing > > around subsystems can be useful occasionally. If nobody will merge the > > code, or if nobody cares, then by all means, let's make them DRM- > > specific helpers. > > In this case, these backlight helpers aren't even common across DRM. > They are tinydrm specific, but only in name and location. They look > like they'd be helpful to panel-simple.c and other panel drivers, too. > :) > > Who's to blame for duplication within DRM then? If only I had 1 > location of OF graph code to clean-up... I get new DT functions all > the time that other subsystems want, so I don't think the problem lies > there. > > > But I think we need to at least try to do the right thing. If only to > > teach people what the right way is. If we start accepting such things > > by default, how can we expect contributors to even try? > > > > I also think that contributors will often end up contributing not only > > to DRM but to the kernel as a whole. As such it should be part of our > > mentoring to teach them about how the process works as a rule, even if > > the occasional exception is necessary to get things done. > > Yes, it's important for contributors to learn to avoid "the platform > problem"[1]. Stuff Noralf has done over the past few months to get tinydrm merged: - proper deferred io support in fbdev helpers - refactoring all drivers to use the same that rolled their own - bunch of refactoring all around in drm core - remove the debugfs cleanup
[GIT PULL] exynos-drm-next
Hi Dave, Summary: - Add UHD support on TM2/TM2E boards. . adding interlace mode support and 297MHz pixel clock support for UHD mode, setting sysreg register in case of HW trigger mode, and adding SiI8620 MHL bridge device support. - Fix trigger mode issue on Rinato board. . On Rinato board, HW trigger mode doesn't work so fix it. - Some fixup and cleanup. There is one patch series[1] being reviewed yet but seems more review is required. Please kindly let me know if there is any problem. [1] http://www.spinics.net/lists/devicetree/msg162681.html Thanks, Inki Dae. The following changes since commit 99743ae4c5f52f8f8ceb17783056fcc9b4f8b64c: Merge branch 'drm-etnaviv-next' of https://git.pengutronix.de/git/lst/linux into drm-next (2017-02-03 05:41:58 +1000) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos exynos-drm-next for you to fetch changes up to 7ff093d09f8f027c317282e18b566473df53e8a1: drm/exynos: fimd: Do not use HW trigger for exynos3250 (2017-02-07 13:54:01 +0900) Andrzej Hajda (7): drm/exynos/decon5433: configure sysreg in case of hardware trigger drm/exynos/hdmi: add 297MHz pixel clock support drm/exynos/hdmi: fix VSI infoframe registers drm/exynos/hdmi: fix PLL for 27MHz settings drm/exynos/decon5433: add support for interlace modes drm/exynos/decon5433: signal vblank only on odd fields drm/exynos/hdmi: add bridge support Hoegeun Kwon (1): drm/exynos: fimd: Do not use HW trigger for exynos3250 Tobias Jakobi (1): drm/exynos: g2d: change platform driver name to 'exynos-drm-g2d' drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 92 +-- drivers/gpu/drm/exynos/exynos_drm_fimd.c | 2 - drivers/gpu/drm/exynos/exynos_drm_g2d.c | 2 +- drivers/gpu/drm/exynos/exynos_hdmi.c | 80 ++- include/video/exynos5433_decon.h | 2 + 5 files changed, 137 insertions(+), 41 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/7] drm/tinydrm: Add helper functions
On Tue, Feb 07, 2017 at 08:28:16AM +1000, Dave Airlie wrote: > > > > I definitely don't want that we don't attempt this. But brought from years > > of experience, I recommend to merge first (with pre-refactoring already > > applied, but helpers only extracted, not yet at the right spot), and then > > follow up with. Because on average, there's way too many trees with > > overloaded maintainers who maybe look at your patch once per kernel > > release cycle. > > > > If you know that backlight and spi isn't one of these areas (anything that > > goes through takashi/sound is a similar good experience for us on the i915 > > side), then I guess we can try. But then Noralf has already written a few > > months worth of really great refactoring, and I'm seriously starting to > > feel guilty for volunteering him for all of this. Even though he seems to > > be really good at it, and seems to not mind, it's getting a bit silly. > > Given that I'd say up to Noralf. > > > > In short, there's always a balance. > > I don't think we can make a rule for this, it will always depend on the > code. There is always going to be stuff we put in drm that should go > elsewhere, and stuff that is elsewhere that drm should use. > > I think however if we do add stuff like this, someone should keep track > of them and try to make them get further into the kernel. In this case > I don't think the patches are too insane to keep in drm and refactor up > later, in other cases I'm sure it'll be lot more obvious (i.e. we could > make the same argument for chunks of DAL :-) DAL's not under this exception, since DAL is all about using drm stuff more (and maybe fixing a few things in drm helpers). My concern here is only about going outside of drm, where at least sometimes it's just utter pain to get even the most trivial bits merged. And holding up an entire driver for that seems silly, hence merge first, try to further refactor later. For DAL amd has started to work on things properly, and since Alex is now drm-misc committer that should all progress reasonably quickly. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 1/7] drm: Add DRM support for tiny LCD displays
On Mon, Feb 06, 2017 at 08:23:36PM +0100, Noralf Trønnes wrote: > > Den 06.02.2017 10.17, skrev Thierry Reding: > > On Tue, Jan 31, 2017 at 05:03:13PM +0100, Noralf Trønnes wrote: > > > tinydrm provides helpers for very simple displays that can use > > > CMA backed framebuffers and need flushing on changes. > > > > > > Signed-off-by: Noralf Trønnes> > > Acked-by: Daniel Vetter > > > --- > > > > > > Changes since version 2: > > > - Remove fbdev after drm unregister, not before. > > > > > > Changes since version 1: > > > - Add tinydrm.rst > > > - Set tdev->fbdev_cma=NULL on unregister (lastclose is called after that). > > > - Remove some DRM_DEBUG*() > > > > > > Documentation/gpu/index.rst | 1 + > > > Documentation/gpu/tinydrm.rst | 21 ++ > > > drivers/gpu/drm/Kconfig | 2 + > > > drivers/gpu/drm/Makefile| 1 + > > > drivers/gpu/drm/tinydrm/Kconfig | 8 + > > > drivers/gpu/drm/tinydrm/Makefile| 1 + > > > drivers/gpu/drm/tinydrm/core/Makefile | 3 + > > > drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 377 > > > > > > drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c | 234 + > > > include/drm/tinydrm/tinydrm.h | 115 + > > > 10 files changed, 763 insertions(+) > > > create mode 100644 Documentation/gpu/tinydrm.rst > > > create mode 100644 drivers/gpu/drm/tinydrm/Kconfig > > > create mode 100644 drivers/gpu/drm/tinydrm/Makefile > > > create mode 100644 drivers/gpu/drm/tinydrm/core/Makefile > > > create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-core.c > > > create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c > > > create mode 100644 include/drm/tinydrm/tinydrm.h > > I realize this is totally subjective, but this feels somewhat too much > > of a separation. Given the helper nature of TinyDRM, I think it would be > > more appropriate to move the helpers themselves into drm_tiny.[ch] and > > then maybe add a subdirectory drivers/gpu/drm/tiny that contains all the > > drivers that use the helpers. > > > > The separation above further shows in subsequent patches where helpers > > are added to tinydrm that aren't specific to TinyDRM. So this make the > > new helpers appear as more of a subsystem in DRM rather than a helper > > library. It also makes things somewhat inconsistent with existing > > infrastructure. > > What I have done with tinydrm is to do as little as possible in the > core helper. The minimum required to pull together > drm_simple_display_pipe, cma helper + fbdev and framebuffer flushing. > > Then I have added a set of functions that ease the writing of drivers > for RGB565 displays with optional backlight and regulator. > > Added to that is a controller library that handles register access (will > use regmap for non-mipi) and framebuffer flushing (set the windowing > registers and write the buffer to the pixel register). > > And at last there's the display driver that initializes the controller > to match a particular panel. > > Maybe I should narrow tinydrm to _just_ support "RGB565 displays with > optional backlight and regulator". It looks like I split it up to much, > unless someone sees a need for the core of tinydrm elsewhere. > > I think it's hard to avoid the subsystem smell here. These displays are > really simple, fbdev is more than enough to cover their needs. > But fbdev is closed. > > I'm glad you pick on this, as getting the architecture right will save > me maintenance down the line. I did paint me into a corner with > staging/fbtft and I'm not keen on repeating that (to my defence it was > my first C code since university and I had 2 displays that had some > similarities which ended up as fbtft). tbh I'm not sure either whether the tinydrm midlayer is good or not. But then it is what it says on the tin, i.e. tiny, so not much work to demidlayer if we notice a clear need for that change. I wouldn't worry about this for now, but good to keep in mind. In the end, good design is design that can be changed, because you'll only get it right until the next unforseen thing happens :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] drm/rockchip: Do not use DMA mapping API if attached to IOMMU domain
Hi Mark, Thanks for reviving this series and sorry for not taking care of it myself. Please see some comments inline. On Tue, Feb 7, 2017 at 3:09 PM, Mark Yao wrote: > From: Tomasz Figa> > 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 > Signed-off-by: Shunqian Zheng > Acked-by: Mark Yao I believe you need your Signed-off-by here. > --- > drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 4 +- > drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 217 > ++-- > drivers/gpu/drm/rockchip/rockchip_drm_gem.h | 8 + > 3 files changed, 219 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h > b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h > index fb6226c..7c123d9 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. > @@ -60,7 +61,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; > struct list_head psr_list; > spinlock_t psr_list_lock; > }; > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > index b70f942..5209392 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > @@ -16,11 +16,135 @@ > #include > #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_gem_object *rk_obj) > +{ > + struct drm_device *drm = rk_obj->base.dev; > + struct rockchip_drm_private *private = drm->dev_private; > + int prot = IOMMU_READ | IOMMU_WRITE; > + ssize_t ret; > + > + ret = drm_mm_insert_node_generic(>mm, _obj->mm, > +rk_obj->base.size, PAGE_SIZE, > +0, 0); > + if (ret < 0) { > + DRM_ERROR("out of I/O virtual memory: %zd\n", ret); > + return ret; > + } > + > + 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(_obj->mm); > + > + return ret; > +} > + > +static int rockchip_gem_iommu_unmap(struct rockchip_gem_object *rk_obj) > +{ > + struct drm_device *drm = rk_obj->base.dev; > + struct rockchip_drm_private *private = drm->dev_private; > + > + iommu_unmap(private->domain, rk_obj->dma_addr, rk_obj->size); > + drm_mm_remove_node(_obj->mm); > + > + return 0; > +} > + > +static int rockchip_gem_get_pages(struct rockchip_gem_object *rk_obj) > +{ > + struct drm_device *drm = rk_obj->base.dev; > + int ret, i; > + struct scatterlist *s; > + > + rk_obj->pages = drm_gem_get_pages(_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_clflush_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,
[Bug 37724] r300g with HyperZ/Z compression: occlusion queries are messed up in ut2004 (regression, bisected)
https://bugs.freedesktop.org/show_bug.cgi?id=37724 --- Comment #17 from Pavel Ondračka--- (In reply to cosiekvfj from comment #16) > Is it enabled by default now? IIRC the HyperZ should be enabled by default on r500 but was never enabled on r300 and r400 due to lack of testing. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/4] drm/rockchip: Use common IOMMU API to attach devices
From: Shunqian ZhengRockchip 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 Acked-by: Mark Yao --- 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 c30d649..7a610e9 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c @@ -14,19 +14,19 @@ * GNU General Public License for more details. */ -#include - #include #include #include #include #include #include +#include #include #include #include #include #include +#include #include "rockchip_drm_drv.h" #include "rockchip_drm_fb.h" @@ -50,28 +50,31 @@ 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, @@ -123,11 +126,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(_bus_type); + if (!private->domain) + return -ENOMEM; + + geometry = >domain->geometry; + start = geometry->aperture_start; + end = geometry->aperture_end; + + DRM_DEBUG("IOMMU context initialized (aperture: %#llx-%#llx)\n", + start, end); + drm_mm_init(>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(>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(_drm_driver, dev); @@ -151,38 +188,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(_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;
[PATCH 4/4] drm/rockchip: gem: fixup iommu_map_sg error path
The return value of iommu_map_sg is size_t, it's unsigned, So check ret < 0 is wrong. And if iommu_map_sg is error, it's return value is zero, but rockchip_gem_iommu_map feel the zero return value is success, bug happen: [5.227458] [drm:rockchip_gem_iommu_map] *ERROR* failed to map buffer: 0 [ 12.291590] WARNING: at drivers/gpu/drm/drm_mm.c:369 [ 12.291611] Modules linked in: [ 12.291634] [ 12.291658] CPU: 4 PID: 338 Comm: cameraserver Not tainted 4.4.41 #196 [ 12.291680] Hardware name: rockchip,rk3399-mid (DT) [ 12.291703] task: ffc0e5a23100 ti: ffc0e5a64000 task.ti: ffc0e5a64000 [ 12.291739] PC is at drm_mm_remove_node+0xc/0xf8 [ 12.291766] LR is at rockchip_gem_iommu_unmap+0x3c/0x54 [ 12.303799] [] drm_mm_remove_node+0xc/0xf8 [ 12.303827] [] rockchip_gem_free_object+0x98/0x168 [ 12.303854] [] drm_gem_object_free+0x2c/0x34 [ 12.303878] [] drm_gem_dmabuf_release+0x90/0xa4 [ 12.303904] [] dma_buf_release+0x64/0x15c [ 12.303929] [] __fput+0xe0/0x1a4 [ 12.303950] [] fput+0xc/0x14 [ 12.303977] [] task_work_run+0xa0/0xc0 [ 12.304004] [] do_notify_resume+0x40/0x54 [ 12.304026] [] work_pending+0x10/0x14 Change-Id: Id79c052691270553c1c60086f9926f39a5296354 Signed-off-by: Mark Yao --- drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c index 8d27965..cc48673 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c @@ -44,8 +44,10 @@ static int rockchip_gem_iommu_map(struct rockchip_gem_object *rk_obj) 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); + if (ret < rk_obj->base.size) { + DRM_ERROR("failed to map buffer: size=%zd request_size=%zd\n", + ret, rk_obj->base.size); + ret = -ENOMEM; goto err_remove_node; } -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/4] drm/rockchip: gem: add mutex lock for drm mm
drm_mm_insert_node_generic and drm_mm_remove_node may access same resource with list ops, it's not threads safe, so protect this context with mutex lock. Fix bug: [49451.856244] == [49451.856350] BUG: KASAN: wild-memory-access on address dead0108 [49451.856379] Write of size 8 by task Binder:218_4/683 [49451.856417] CPU: 2 PID: 683 Comm: Binder:218_4 Not tainted 4.4.36 #62 [49451.856443] Hardware name: Rockchip RK3399 Excavator Board edp (Android) (DT) [49451.856469] Call trace: [49451.856519] [] dump_backtrace+0x0/0x230 [49451.856556] [] show_stack+0x14/0x1c [49451.856592] [] dump_stack+0xa0/0xc8 [49451.856633] [] kasan_report+0x110/0x4dc [49451.856670] [] __asan_store8+0x24/0x7c [49451.856715] [] drm_mm_insert_node_generic+0x2dc/0x464 [49451.856760] [] rockchip_gem_iommu_map+0x60/0x158 [49451.856794] [] rockchip_gem_create_object+0x278/0x488 [49451.856827] [] rockchip_gem_create_with_handle+0x24/0x10c [49451.856862] [] rockchip_gem_create_ioctl+0x3c/0x50 [49451.856896] [] drm_ioctl+0x354/0x52c [49451.856939] [] do_vfs_ioctl+0x670/0x78c [49451.856976] [] SyS_ioctl+0x60/0x88 [49451.857009] [] el0_svc_naked+0x24/0x28 Change-Id: I2ea377aa9ca24f70c59e2d86f2a6ad5ccb9c0891 Signed-off-by: Mark Yao --- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 1 + drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 2 ++ drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 9 + 3 files changed, 12 insertions(+) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c index 7a610e9..b360e62 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c @@ -146,6 +146,7 @@ static int rockchip_drm_init_iommu(struct drm_device *drm_dev) DRM_DEBUG("IOMMU context initialized (aperture: %#llx-%#llx)\n", start, end); drm_mm_init(>mm, start, end - start + 1); + mutex_init(>mm_lock); return 0; } diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h index 7c123d9..adc3930 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h @@ -62,6 +62,8 @@ struct rockchip_drm_private { const struct rockchip_crtc_funcs *crtc_funcs[ROCKCHIP_MAX_CRTC]; struct drm_atomic_state *state; struct iommu_domain *domain; + /* protect drm_mm on multi-threads */ + struct mutex mm_lock; struct drm_mm mm; struct list_head psr_list; spinlock_t psr_list_lock; diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c index 5209392..8d27965 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c @@ -28,9 +28,13 @@ static int rockchip_gem_iommu_map(struct rockchip_gem_object *rk_obj) int prot = IOMMU_READ | IOMMU_WRITE; ssize_t ret; + mutex_lock(>mm_lock); + ret = drm_mm_insert_node_generic(>mm, _obj->mm, rk_obj->base.size, PAGE_SIZE, 0, 0); + + mutex_unlock(>mm_lock); if (ret < 0) { DRM_ERROR("out of I/O virtual memory: %zd\n", ret); return ret; @@ -61,8 +65,13 @@ static int rockchip_gem_iommu_unmap(struct rockchip_gem_object *rk_obj) struct rockchip_drm_private *private = drm->dev_private; iommu_unmap(private->domain, rk_obj->dma_addr, rk_obj->size); + + mutex_lock(>mm_lock); + drm_mm_remove_node(_obj->mm); + mutex_unlock(>mm_lock); + return 0; } -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/4] drm/rockchip: switch to drm_mm for support arm64 iommu
Some iommu patches on the series[0] "iommu/rockchip: Fix bugs and enable on ARM64" already landed, So drm/rockchip related patches [1] and [2] ready to landed, this series just rebase them to lastest drm-next. And fix some bugs for drm/rockchip drm_mm [0]: http://www.spinics.net/lists/arm-kernel/msg513781.html [1]: https://patchwork.kernel.org/patch/9196367 [2]: https://patchwork.kernel.org/patch/9196369 Mark Yao (2): drm/rockchip: gem: add mutex lock for drm mm drm/rockchip: gem: fixup iommu_map_sg error path Shunqian Zheng (1): drm/rockchip: Use common IOMMU API to attach devices Tomasz Figa (1): drm/rockchip: Do not use DMA mapping API if attached to IOMMU domain drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 101 ++-- drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 6 +- drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 228 ++-- drivers/gpu/drm/rockchip/rockchip_drm_gem.h | 8 + 4 files changed, 286 insertions(+), 57 deletions(-) -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/4] drm/rockchip: Do not use DMA mapping API if attached to IOMMU domain
From: Tomasz FigaThe 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 Signed-off-by: Shunqian Zheng Acked-by: Mark Yao --- drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 4 +- drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 217 ++-- drivers/gpu/drm/rockchip/rockchip_drm_gem.h | 8 + 3 files changed, 219 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h index fb6226c..7c123d9 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. @@ -60,7 +61,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; struct list_head psr_list; spinlock_t psr_list_lock; }; diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c index b70f942..5209392 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c @@ -16,11 +16,135 @@ #include #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_gem_object *rk_obj) +{ + struct drm_device *drm = rk_obj->base.dev; + struct rockchip_drm_private *private = drm->dev_private; + int prot = IOMMU_READ | IOMMU_WRITE; + ssize_t ret; + + ret = drm_mm_insert_node_generic(>mm, _obj->mm, +rk_obj->base.size, PAGE_SIZE, +0, 0); + if (ret < 0) { + DRM_ERROR("out of I/O virtual memory: %zd\n", ret); + return ret; + } + + 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(_obj->mm); + + return ret; +} + +static int rockchip_gem_iommu_unmap(struct rockchip_gem_object *rk_obj) +{ + struct drm_device *drm = rk_obj->base.dev; + struct rockchip_drm_private *private = drm->dev_private; + + iommu_unmap(private->domain, rk_obj->dma_addr, rk_obj->size); + drm_mm_remove_node(_obj->mm); + + return 0; +} + +static int rockchip_gem_get_pages(struct rockchip_gem_object *rk_obj) +{ + struct drm_device *drm = rk_obj->base.dev; + int ret, i; + struct scatterlist *s; + + rk_obj->pages = drm_gem_get_pages(_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_clflush_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(_obj->base, rk_obj->pages, false, false); + return ret; +} + +static void rockchip_gem_put_pages(struct rockchip_gem_object *rk_obj) +{ + sg_free_table(rk_obj->sgt); + kfree(rk_obj->sgt); + drm_gem_put_pages(_obj->base, rk_obj->pages, false, false); +} + +static int rockchip_gem_alloc_iommu(struct rockchip_gem_object *rk_obj, +
Re: [GIT PULL] drm/rockchip: add cdn-dp support and some fixes
On 2017年02月07日 09:05, Dave Airlie wrote: On 5 February 2017 at 18:40, Mark yao wrote: Hi Dave drm/rockchip cdn-dp driver is v17 now, I think it's ready to land it. Thanks. The following changes since commit 99743ae4c5f52f8f8ceb17783056fcc9b4f8b64c: Merge branch 'drm-etnaviv-next' of https://git.pengutronix.de/git/lst/linux into drm-next (2017-02-03 05:41:58 +1000) CC [M] drivers/gpu/drm/rockchip/rockchip_vop_reg.o CC [M] drivers/gpu/drm/rockchip/rockchip_drm_vop.o /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/rockchip/cdn-dp-reg.c: In function ‘cdn_dp_config_video’: /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/rockchip/cdn-dp-reg.c:632:24: warning: ‘val[1]’ may be used uninitialized in this function [-Wmaybe-uninitialized] msa_misc = 2 * val[0] + 32 * val[1] + ~~~^ /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/rockchip/cdn-dp-reg.c:595:5: note: ‘val[1]’ was declared here u8 val[2]; You might want to send me a follow up pull request to fix that. Dave. Hi Dave Sorry for the compile warning, following patch fix the compile warning with tiny modify. Thanks. The following changes since commit 26d7f34cae7aad9600cd40ce07ec3fbe8606a567: Merge branch 'msm-next' of git://people.freedesktop.org/~robclark/linux into drm-next (2017-02-07 11:05:42 +1000) are available in the git repository at: https://github.com/markyzq/kernel-drm-rockchip.git drm-rockchip-next-2017-02-07 for you to fetch changes up to 213c4b96639818a2dc8750d1b7a37672a9c9eaeb: drm/rockchip: cdn-dp: fix cdn-dp complie warning (2017-02-07 11:06:01 +0800) Mark Yao (1): drm/rockchip: cdn-dp: fix cdn-dp complie warning drivers/gpu/drm/rockchip/cdn-dp-reg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- Mark Yao ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Do no reset the max link parameters when userspace reprobes modes
Hi, I am working on a solution for handling link failures during or after the modeset. In case of link failure, the max link rate/lane count values are updated to lower link rate/lane count as per the spec and uevent is sent to the userspace with link-status BAD. The userspace is expected to first retry the existing mode and do a a full reprobe if this fails. When userspace reprobes the modes, it calls drmModeGetConnector() IOCTL, that eventually calls drm_helper_probe_single_connector_modes(). This helper function calls the connector->func->detect() which in case of i915 calls intel_dp_detect() which calls intel_dp_long_pulse() that resets the max_link_rate and max_link_lane_count values. Now during the link training retry, it uses the reset values of link rate/lane count instead of using the lowered link rate/lane count values from link failure. For this to work properly, we should not reset the link rate/lane count values during the fill_modes() operation from userspace. However the current driver calls the long pulse handler again during fill_modes and resets all the values as per DPCD reads which fails to retrain the link at lower rate. How can we handle this situation? Do we need to call long pulse handler again during fill_modes()? Any ideas/suggestions? Regards Manasi ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/rockchip: cdn-dp: fix cdn-dp complie warning
fix warning: drivers/gpu/drm/rockchip/cdn-dp-reg.c:632:24: warning: 'val[1]' may be used uninitialized in this function [-Wmaybe-uninitialized] msa_misc = 2 * val[0] + 32 * val[1] + Signed-off-by: Mark Yao --- drivers/gpu/drm/rockchip/cdn-dp-reg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.c b/drivers/gpu/drm/rockchip/cdn-dp-reg.c index 3a5b8a4..319dbba 100644 --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.c +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.c @@ -592,7 +592,7 @@ static int cdn_dp_get_msa_misc(struct video_info *video, struct drm_display_mode *mode) { u32 msa_misc; - u8 val[2]; + u8 val[2] = {0}; switch (video->color_fmt) { case PXL_RGB: -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [GIT PULL] drm/rockchip: add cdn-dp support and some fixes
On 5 February 2017 at 18:40, Mark yao wrote: > Hi Dave > > drm/rockchip cdn-dp driver is v17 now, I think it's ready to land it. > > Thanks. > > The following changes since commit 99743ae4c5f52f8f8ceb17783056fcc9b4f8b64c: > > Merge branch 'drm-etnaviv-next' of > https://git.pengutronix.de/git/lst/linux into drm-next (2017-02-03 05:41:58 > +1000) > CC [M] drivers/gpu/drm/rockchip/rockchip_vop_reg.o CC [M] drivers/gpu/drm/rockchip/rockchip_drm_vop.o /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/rockchip/cdn-dp-reg.c: In function ‘cdn_dp_config_video’: /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/rockchip/cdn-dp-reg.c:632:24: warning: ‘val[1]’ may be used uninitialized in this function [-Wmaybe-uninitialized] msa_misc = 2 * val[0] + 32 * val[1] + ~~~^ /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/rockchip/cdn-dp-reg.c:595:5: note: ‘val[1]’ was declared here u8 val[2]; You might want to send me a follow up pull request to fix that. Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next] drm: mxsfb: fix error return code in mxsfb_load()
On 02/05/2017 05:00 PM, Wei Yongjun wrote: > From: Wei Yongjun> > Fix to return a negative error code from the error handling > case instead of 0, as done elsewhere in this function. > > Signed-off-by: Wei Yongjun Acked-by: Marek Vasut > --- > drivers/gpu/drm/mxsfb/mxsfb_drv.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c > b/drivers/gpu/drm/mxsfb/mxsfb_drv.c > index cdfbe02..28a75cb 100644 > --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c > +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c > @@ -221,6 +221,7 @@ static int mxsfb_load(struct drm_device *drm, unsigned > long flags) > mxsfb->fbdev = drm_fbdev_cma_init(drm, 32, > drm->mode_config.num_connector); > if (IS_ERR(mxsfb->fbdev)) { > + ret = PTR_ERR(mxsfb->fbdev); > mxsfb->fbdev = NULL; > dev_err(drm->dev, "Failed to init FB CMA area\n"); > goto err_cma; > > > -- Best regards, Marek Vasut ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/5] drm: of: introduce drm_of_find_panel_or_bridge
On Mon, 2017-02-06 at 10:53 -0600, Rob Herring wrote: > On Mon, Feb 06, 2017 at 11:42:48AM +0100, Philipp Zabel wrote: > > On Fri, 2017-02-03 at 21:36 -0600, Rob Herring wrote: > > > Many drivers have a common pattern of searching the OF graph for either an > > > attached panel or bridge and then finding the DRM struct for the panel > > > or bridge. Also, most drivers need to handle deferred probing when the > > > DRM device is not yet instantiated. Create a common function, > > > drm_of_find_panel_or_bridge, to find the connected node and the > > > associated DRM panel or bridge device. > > [...] > > > > +int drm_of_find_panel_or_bridge(const struct device_node *np, > > > + int port, int endpoint, > > > + struct drm_panel **panel, > > > + struct drm_bridge **bridge) > > > +{ > > > + int ret = -ENODEV; > > > > This is only returned if !panel && !bridge. I'd consider this invalid > > usage of this function, so maybe use -EINVAL? > > Yes. > > > > + struct device_node *remote; > > > + > > > + remote = of_graph_get_remote_node(np, port, endpoint); > > > + if (!remote) > > > + return -ENODEV; > > > + > > > + if (bridge) > > > + *bridge = NULL; > > > > I would move this ^ ... > > > > > + if (panel) { > > > + *panel = of_drm_find_panel(remote); > > > + if (*panel) { > > > > ... here. > > Okay. > > > > + ret = 0; > > > + goto out_put; > > > + } > > > + ret = -EPROBE_DEFER; > > > + } > > > + > > > + if (bridge) { > > > + *bridge = of_drm_find_bridge(remote); > > > + if (*bridge) > > > + ret = 0; > > > + else > > > + ret = -EPROBE_DEFER; > > > + } > > > +out_put: > > > + of_node_put(remote); > > > + return ret; > > > +} > > I've ended up re-writing things a bit getting rid of the goto and the > result looks like this: Looks good to me. > int drm_of_find_panel_or_bridge(const struct device_node *np, > int port, int endpoint, > struct drm_panel **panel, > struct drm_bridge **bridge) > { > int ret = -EPROBE_DEFER; > struct device_node *remote; > > if (!panel && !bridge) > return -EINVAL; > > remote = of_graph_get_remote_node(np, port, endpoint); > if (!remote) > return -ENODEV; > > if (panel) { > *panel = of_drm_find_panel(remote); > if (*panel) { > if (bridge) > *bridge = NULL; With the goto out_put gone, I'm conflicted whether I find this clearer here, or ... > ret = 0; > } > } > > /* No panel found yet, check for a bridge next. */ > if (ret && bridge) { > *bridge = of_drm_find_bridge(remote); > if (*bridge) > ret = 0; > } ... even down here: if (bridge) { if (ret) { /* No panel found yet, check for a bridge next. */ *bridge = of_drm_find_bridge(remote) if (*bridge) ret = 0; } else { *bridge = NULL; } } That way bridge doesn't have to be checked twice and all the modification of *bridge is in the same block. > > of_node_put(remote); > return ret; > } Either way, Acked-by: Philipp Zabelregards Philipp ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node
On Mon, Feb 06, 2017 at 05:34:07PM +, Russell King - ARM Linux wrote: > On Mon, Feb 06, 2017 at 05:23:06PM +, Liviu Dudau wrote: > > On Mon, Feb 06, 2017 at 11:09:49AM -0600, Rob Herring wrote: > > > On Mon, Feb 06, 2017 at 10:29:33AM +, Liviu Dudau wrote: > > > > On Fri, Feb 03, 2017 at 09:36:33PM -0600, Rob Herring wrote: > > > > > - /* add the remote encoder port as component */ > > > > > - port = of_graph_get_remote_port_parent(ep); > > > > > - of_node_put(ep); > > > > > - if (!port || !of_device_is_available(port)) { > > > > > - of_node_put(port); > > > > > - return -EAGAIN; > > > > > > > > The HDLCD change looks reasonable except for this -EAGAIN business. > > > > I'll have to > > > > test your changes on my setup to see how this affects having the > > > > encoder as a module. > > > > > > What are you expecting to happen with -EAGAIN? This one was a bit of an > > > oddball. > > > > When both the HDLCD and the TDA998x drivers are compiled as modules, the > > order in which they are inserted can be somewhat random (due to testing). > > Not really "due to testing" but if you run a real distro, they tend to > have a multi-threaded behaviour when loading kernel modules at boot. Yeah, a lot of times I'm using a toy "distribution" (buildroot) as it boots faster under ARM models than a "real" (read systemd-based) distro would. > > > It is at that time when you want the probe of HDLCD to be retried on the > > insmod-ing of the tda998x.ko rather than fail entirely. > > -EAGAIN doesn't get you that, and in any case, solving that problem is > exactly why the component API exists - so that DRM only comes up once > all the necessary components are available. > > -EAGAIN also doesn't get you that from inside a probe function - such > an error will be reported in the kernel log, and no further action > will be taken (the device driver probe will be failed, and not > automatically retried. I stand corrected on the behaviour of the driver then. That was the original intent, to generate a re-probe of the driver. > > The only case that we automatically retry is if a driver returns > -EPROBE_DEFER. Everything else causes a probe failure. OK, I will fix the driver if Rob's patch still requires it. Best regards, Liviu > > -- > RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ > FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up > according to speedtest.net. -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node
On Mon, Feb 06, 2017 at 05:55:33PM +, Liviu Dudau wrote: > OK, I will fix the driver if Rob's patch still requires it. I don't think you ever needed it. As Rob says, what you're testing won't ever change unless you're using overlays - it's certainly not dependent on the tda998x module being loaded or not, or even the tda998x driver being bound or not. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node
On Mon, Feb 6, 2017 at 11:23 AM, Liviu Dudauwrote: > On Mon, Feb 06, 2017 at 11:09:49AM -0600, Rob Herring wrote: >> On Mon, Feb 06, 2017 at 10:29:33AM +, Liviu Dudau wrote: >> > On Fri, Feb 03, 2017 at 09:36:33PM -0600, Rob Herring wrote: >> > > Convert drivers to use the new of_graph_get_remote_node() helper >> > > instead of parsing the endpoint node and then getting the remote device >> > > node. Now drivers can just specify the device node and which >> > > port/endpoint and get back the connected remote device node. The details >> > > of the graph binding are nicely abstracted into the core OF graph code. >> > > >> > > This changes some error messages to debug messages (in the graph core). >> > > Graph connections are often "no connects" depending on the particular >> > > board, so we want to avoid spurious messages. Plus the kernel is not a >> > > DT validator. [...] >> > > - /* add the remote encoder port as component */ >> > > - port = of_graph_get_remote_port_parent(ep); >> > > - of_node_put(ep); >> > > - if (!port || !of_device_is_available(port)) { >> > > - of_node_put(port); >> > > - return -EAGAIN; >> > >> > The HDLCD change looks reasonable except for this -EAGAIN business. I'll >> > have to >> > test your changes on my setup to see how this affects having the encoder >> > as a module. >> >> What are you expecting to happen with -EAGAIN? This one was a bit of an >> oddball. > > When both the HDLCD and the TDA998x drivers are compiled as modules, the > order in which > they are inserted can be somewhat random (due to testing). It is at that time > when you > want the probe of HDLCD to be retried on the insmod-ing of the tda998x.ko > rather than > fail entirely. > >> >> This condition would only change if you had an overlay. That's a use >> case that needs to be handled in a common way ('cause I don't want to >> clean-up every driver doing overlays in their own way latter). Just >> having "status" changing at runtime would have all sorts of implications >> in the kernel. > > Hmm, not sure what you mean here with overlays. Are you thinking that the > remote port is initially disabled and then re-enabled by an overlay? That is > not the only way of_device_is_available() can fail, see above regarding > modules. Russell pretty much answered most of this, but specifically for of_device_is_available, the only way of_device_is_available() can change is a DT change with "status" changing. The only way of_graph_get_remote_port_parent changes is also from a DT change. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 98405] No signal on HDMI output after upgrade to kernel 4.8.3-1
https://bugs.freedesktop.org/show_bug.cgi?id=98405 pomachanged: What|Removed |Added See Also||https://bugzilla.suse.com/s ||how_bug.cgi?id=1006420 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 98405] No signal on HDMI output after upgrade to kernel 4.8.3-1
https://bugs.freedesktop.org/show_bug.cgi?id=98405 pomachanged: What|Removed |Added See Also||https://bugzilla.redhat.com ||/show_bug.cgi?id=1385394 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 37724] r300g with HyperZ/Z compression: occlusion queries are messed up in ut2004 (regression, bisected)
https://bugs.freedesktop.org/show_bug.cgi?id=37724 --- Comment #16 from cosiek...@o2.pl --- Is it enabled by default now? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/7] drm/tinydrm: Add helper functions
On Mon, Feb 6, 2017 at 5:08 AM, Thierry Redingwrote: > On Mon, Feb 06, 2017 at 11:07:42AM +0100, Daniel Vetter wrote: >> On Mon, Feb 6, 2017 at 10:35 AM, Thierry Reding >> wrote: >> >> > > > > +EXPORT_SYMBOL(tinydrm_disable_backlight); >> > > > > +#endif >> > > > >> > > > These look like they really should be part of the backlight subsystem. >> > I >> > > > don't see anything DRM specific about them. Well, except for the error >> > > > messages. >> > > >> > > So this is a bit an unpopular opinion with some folks, but I don't >> > require >> > > anyone to submit new code to subsystems outside of drm for new drivers. >> > > Simply because it takes months to get stuff landed, and in general it's >> > > not worth the trouble. >> > >> > "Not worth the trouble" is very subjective. If you look at the Linux >> > kernel in general, one of the reasons why it works so well is because >> > the changes we make apply to the kernel as a whole. Yes, sometimes that >> > makes things more difficult and time-consuming, but it also means that >> > the end result will be much more widely usable and therefore benefits >> > everyone else in return. In my opinion that's a large part of why the >> > kernel is so successful. >> > >> > > We have piles of stuff in drm and drm drivers that should be in core but >> > > isn't. >> > > >> > > Imo the only reasonable way is to merge as-is, then follow-up with a >> > patch >> > > series to move the helper into the right subsystem. Most often >> > > unfortunately that follow-up patch series will just die. >> > >> > Of course follow-up series die. That's because nobody cares to follow-up >> > once their code has been merged. >> > >> > Collecting our own helpers or variants of subsystems is a great way of >> > isolating ourselves from the rest of the community. I don't think that's >> > a good solution in the long run at all. >> > >> >> We have a bunch of patch series that we resubmit for months and they go >> exactly nowhere. They don't die because we stop caring, they die because >> they die. Some of them we even need to constantly rebase and carry around >> in drm-tip since our CI would Oops or spew WARNIGs all over the place. >> There's simply some areas of the kernel which seem overloaded under patches >> and no one is willing or able to fix things, and I can't fix the entire >> kernel. Nor expect contributors (who have much less political weight to >> throw around than me) to do that and succeed. And we don't end up with >> worse code in the drm subsystem, since we can still do the refactoring >> within drm helpers and end up with clean drivers. >> >> I fully agree that it's not great for the kernel's future, but when I'm >> stuck with the option to get shit done or burning out playing the >> upstreaming game, the choice is easy. And in the end I care about open >> source gfx much more than the kernel, and I think for open source gfx's >> success it's crucial that we're welcoming to new contributors and don't >> throw up massive roadblocks. Open source gfx is tiny and still far away >> from world domination, we need _lots_ more people. If that means routing >> around other subsystems for them, I'm all for it. > > I can't say I fully agree with that sentiment. I do see how routing > around subsystems can be useful occasionally. If nobody will merge the > code, or if nobody cares, then by all means, let's make them DRM- > specific helpers. In this case, these backlight helpers aren't even common across DRM. They are tinydrm specific, but only in name and location. They look like they'd be helpful to panel-simple.c and other panel drivers, too. :) Who's to blame for duplication within DRM then? If only I had 1 location of OF graph code to clean-up... I get new DT functions all the time that other subsystems want, so I don't think the problem lies there. > But I think we need to at least try to do the right thing. If only to > teach people what the right way is. If we start accepting such things > by default, how can we expect contributors to even try? > > I also think that contributors will often end up contributing not only > to DRM but to the kernel as a whole. As such it should be part of our > mentoring to teach them about how the process works as a rule, even if > the occasional exception is necessary to get things done. Yes, it's important for contributors to learn to avoid "the platform problem"[1]. Rob [1] https://lwn.net/Articles/443531/ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/7] drm/tinydrm: Add helper functions
> > I definitely don't want that we don't attempt this. But brought from years > of experience, I recommend to merge first (with pre-refactoring already > applied, but helpers only extracted, not yet at the right spot), and then > follow up with. Because on average, there's way too many trees with > overloaded maintainers who maybe look at your patch once per kernel > release cycle. > > If you know that backlight and spi isn't one of these areas (anything that > goes through takashi/sound is a similar good experience for us on the i915 > side), then I guess we can try. But then Noralf has already written a few > months worth of really great refactoring, and I'm seriously starting to > feel guilty for volunteering him for all of this. Even though he seems to > be really good at it, and seems to not mind, it's getting a bit silly. > Given that I'd say up to Noralf. > > In short, there's always a balance. I don't think we can make a rule for this, it will always depend on the code. There is always going to be stuff we put in drm that should go elsewhere, and stuff that is elsewhere that drm should use. I think however if we do add stuff like this, someone should keep track of them and try to make them get further into the kernel. In this case I don't think the patches are too insane to keep in drm and refactor up later, in other cases I'm sure it'll be lot more obvious (i.e. we could make the same argument for chunks of DAL :-) Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/7] drm/tinydrm: Add helper functions
Den 06.02.2017 16.53, skrev Daniel Vetter: On Mon, Feb 06, 2017 at 12:08:47PM +0100, Thierry Reding wrote: On Mon, Feb 06, 2017 at 11:07:42AM +0100, Daniel Vetter wrote: On Mon, Feb 6, 2017 at 10:35 AM, Thierry Redingwrote: +EXPORT_SYMBOL(tinydrm_disable_backlight); +#endif These look like they really should be part of the backlight subsystem. I don't see anything DRM specific about them. Well, except for the error messages. So this is a bit an unpopular opinion with some folks, but I don't require anyone to submit new code to subsystems outside of drm for new drivers. Simply because it takes months to get stuff landed, and in general it's not worth the trouble. "Not worth the trouble" is very subjective. If you look at the Linux kernel in general, one of the reasons why it works so well is because the changes we make apply to the kernel as a whole. Yes, sometimes that makes things more difficult and time-consuming, but it also means that the end result will be much more widely usable and therefore benefits everyone else in return. In my opinion that's a large part of why the kernel is so successful. We have piles of stuff in drm and drm drivers that should be in core but isn't. Imo the only reasonable way is to merge as-is, then follow-up with a patch series to move the helper into the right subsystem. Most often unfortunately that follow-up patch series will just die. Of course follow-up series die. That's because nobody cares to follow-up once their code has been merged. Collecting our own helpers or variants of subsystems is a great way of isolating ourselves from the rest of the community. I don't think that's a good solution in the long run at all. We have a bunch of patch series that we resubmit for months and they go exactly nowhere. They don't die because we stop caring, they die because they die. Some of them we even need to constantly rebase and carry around in drm-tip since our CI would Oops or spew WARNIGs all over the place. There's simply some areas of the kernel which seem overloaded under patches and no one is willing or able to fix things, and I can't fix the entire kernel. Nor expect contributors (who have much less political weight to throw around than me) to do that and succeed. And we don't end up with worse code in the drm subsystem, since we can still do the refactoring within drm helpers and end up with clean drivers. I fully agree that it's not great for the kernel's future, but when I'm stuck with the option to get shit done or burning out playing the upstreaming game, the choice is easy. And in the end I care about open source gfx much more than the kernel, and I think for open source gfx's success it's crucial that we're welcoming to new contributors and don't throw up massive roadblocks. Open source gfx is tiny and still far away from world domination, we need _lots_ more people. If that means routing around other subsystems for them, I'm all for it. I can't say I fully agree with that sentiment. I do see how routing around subsystems can be useful occasionally. If nobody will merge the code, or if nobody cares, then by all means, let's make them DRM- specific helpers. But I think we need to at least try to do the right thing. If only to teach people what the right way is. If we start accepting such things by default, how can we expect contributors to even try? I also think that contributors will often end up contributing not only to DRM but to the kernel as a whole. As such it should be part of our mentoring to teach them about how the process works as a rule, even if the occasional exception is necessary to get things done. In this particular case, I know for a fact that both backlight and SPI maintainers are very responsive, so that's not a good excuse. I definitely don't want that we don't attempt this. But brought from years of experience, I recommend to merge first (with pre-refactoring already applied, but helpers only extracted, not yet at the right spot), and then follow up with. Because on average, there's way too many trees with overloaded maintainers who maybe look at your patch once per kernel release cycle. If you know that backlight and spi isn't one of these areas (anything that goes through takashi/sound is a similar good experience for us on the i915 side), then I guess we can try. But then Noralf has already written a few months worth of really great refactoring, and I'm seriously starting to feel guilty for volunteering him for all of this. Even though he seems to be really good at it, and seems to not mind, it's getting a bit silly. Given that I'd say up to Noralf. In short, there's always a balance. Yes, it's a balance between the perfect and not's so perfect, the professinal and the amateur, and the drm expert and the newbie. If I knew how much time I would have to spend on tinydrm to get it merged, then I would never have started it. I wondered, a couple of months back, if I should just cut
[Bug 43698] On PPC, OpenGL programs use incorrect texture colors.
https://bugs.freedesktop.org/show_bug.cgi?id=43698 cosiek...@o2.pl changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #25 from cosiek...@o2.pl --- Fix for this bug result in regression: https://bugs.freedesktop.org/show_bug.cgi?id=98869 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 98869] Electronic Super Joy graphic artefacts (regression)
https://bugs.freedesktop.org/show_bug.cgi?id=98869 cosiek...@o2.pl changed: What|Removed |Added Priority|high|medium Severity|major |normal -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 71789] [r300g] Visuals not found in (default) depth = 24
https://bugs.freedesktop.org/show_bug.cgi?id=71789 --- Comment #34 from cosiek...@o2.pl --- Fix for this bug result in regression: https://bugs.freedesktop.org/show_bug.cgi?id=98869 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 98869] Electronic Super Joy graphic artefacts (regression)
https://bugs.freedesktop.org/show_bug.cgi?id=98869 --- Comment #17 from cosiek...@o2.pl --- 172bfdaa9e80342ade3f023f72d455d76713b866 Is cause of the problem! https://cgit.freedesktop.org/mesa/mesa/commit/?h=12.0=172bfdaa9e80342ade3f023f72d455d76713b866 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 03/11] drm/msm: Add hint to DRM_IOCTL_MSM_GEM_INFO to return an object IOVA
On Mon, Feb 6, 2017 at 3:24 PM, Emil Velikovwrote: > On 6 February 2017 at 19:57, Rob Clark wrote: >> On Mon, Feb 6, 2017 at 2:20 PM, Emil Velikov >> wrote: >>> Hi Jordan, >>> >>> On 6 February 2017 at 17:39, Jordan Crouse wrote: Modify the 'pad' member of struct drm_msm_gem_info to 'hint'. If the user sets 'hint' to non-zero it means that they want a IOVA for the GEM object instead of a mmap() offset. Return the iova in the 'offset' member. Signed-off-by: Jordan Crouse --- drivers/gpu/drm/msm/msm_drv.c | 29 + include/uapi/drm/msm_drm.h| 4 ++-- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index e29bb66..1e4e022 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -677,6 +677,17 @@ static int msm_ioctl_gem_cpu_fini(struct drm_device *dev, void *data, return ret; } +static int msm_ioctl_gem_info_iova(struct drm_device *dev, + struct drm_gem_object *obj, uint64_t *iova) +{ + struct msm_drm_private *priv = dev->dev_private; + + if (!priv->gpu) + return -EINVAL; + >>> Not too familiar with msm so perhaps a silly question: how can we trigger >>> this ? >> >> if gpu has not loaded (for example, missing firmware, or kernel does >> not have iommu, etc) >> > Thanks Rob. I was under the impression that in such cases the driver > will/should fail to load. radeon/nouveau/i915 will all, iirc, fail to load. I made drm/msm defer to loading gpu until first open() since having to constantly rebuild initrd seemed annoying ;-) + __u64 offset; /* out, mmap() offset if hint is 0, iova if 1 */ >>> Other drivers have used anonymous unions to improve the naming, in >>> such situations. >>> >>> struct drm_msm_gem_info { >>> __u32 handle; /* in */ >>> __u32 hint; /* in */ >>> union { /* out */ >>> __u64 offset; /* offset if hint is FOO */ >>> __u64 iova; /* iova if hint is BAR */ >>> }; >>> }; >> >> is anon union legit for uabi? I was under the impression that for >> some reason it was not. But I could be wrong. >> > Haven't seen any wording against it and we do have a few instances in > DRM UABI land. > Either way it was just an idea. hmm, ok, if we are already using it in uabi (and not just ancient ioctls) then maybe I am wrong.. BR, -R > Regards, > Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 03/11] drm/msm: Add hint to DRM_IOCTL_MSM_GEM_INFO to return an object IOVA
On 6 February 2017 at 19:57, Rob Clarkwrote: > On Mon, Feb 6, 2017 at 2:20 PM, Emil Velikov wrote: >> Hi Jordan, >> >> On 6 February 2017 at 17:39, Jordan Crouse wrote: >>> Modify the 'pad' member of struct drm_msm_gem_info to 'hint'. If the >>> user sets 'hint' to non-zero it means that they want a IOVA for the >>> GEM object instead of a mmap() offset. Return the iova in the 'offset' >>> member. >>> >>> Signed-off-by: Jordan Crouse >>> --- >>> drivers/gpu/drm/msm/msm_drv.c | 29 + >>> include/uapi/drm/msm_drm.h| 4 ++-- >>> 2 files changed, 27 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c >>> index e29bb66..1e4e022 100644 >>> --- a/drivers/gpu/drm/msm/msm_drv.c >>> +++ b/drivers/gpu/drm/msm/msm_drv.c >>> @@ -677,6 +677,17 @@ static int msm_ioctl_gem_cpu_fini(struct drm_device >>> *dev, void *data, >>> return ret; >>> } >>> >>> +static int msm_ioctl_gem_info_iova(struct drm_device *dev, >>> + struct drm_gem_object *obj, uint64_t *iova) >>> +{ >>> + struct msm_drm_private *priv = dev->dev_private; >>> + >>> + if (!priv->gpu) >>> + return -EINVAL; >>> + >> Not too familiar with msm so perhaps a silly question: how can we trigger >> this ? > > if gpu has not loaded (for example, missing firmware, or kernel does > not have iommu, etc) > Thanks Rob. I was under the impression that in such cases the driver will/should fail to load. >>> + __u64 offset; /* out, mmap() offset if hint is 0, iova if 1 >>> */ >> Other drivers have used anonymous unions to improve the naming, in >> such situations. >> >> struct drm_msm_gem_info { >> __u32 handle; /* in */ >> __u32 hint; /* in */ >> union { /* out */ >> __u64 offset; /* offset if hint is FOO */ >> __u64 iova; /* iova if hint is BAR */ >> }; >> }; > > is anon union legit for uabi? I was under the impression that for > some reason it was not. But I could be wrong. > Haven't seen any wording against it and we do have a few instances in DRM UABI land. Either way it was just an idea. Regards, Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/radeon: avoid kernel segfault in vce when gpu fails to resume
From: Jérôme GlisseWhen GPU fails to resume we can not trust that value we write to GPU memory will post and we might get garbage (more like 0x on x86) when reading them back. This trigger out of range memory access in the kernel inside the vce resume code path. This patch use canonical value to compute offset instead of reading back value from GPU memory. Signed-off-by: Jérôme Glisse --- drivers/gpu/drm/radeon/vce_v1_0.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/radeon/vce_v1_0.c b/drivers/gpu/drm/radeon/vce_v1_0.c index a01efe3..f541a4b 100644 --- a/drivers/gpu/drm/radeon/vce_v1_0.c +++ b/drivers/gpu/drm/radeon/vce_v1_0.c @@ -196,7 +196,7 @@ int vce_v1_0_load_fw(struct radeon_device *rdev, uint32_t *data) memset([5], 0, 44); memcpy([16], [1], rdev->vce_fw->size - sizeof(*sign)); - data += le32_to_cpu(data[4]) / 4; + data += (le32_to_cpu(sign->len) + 64) / 4; data[0] = sign->val[i].sigval[0]; data[1] = sign->val[i].sigval[1]; data[2] = sign->val[i].sigval[2]; -- 2.9.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 03/11] drm/msm: Add hint to DRM_IOCTL_MSM_GEM_INFO to return an object IOVA
On Mon, Feb 6, 2017 at 2:20 PM, Emil Velikovwrote: > Hi Jordan, > > On 6 February 2017 at 17:39, Jordan Crouse wrote: >> Modify the 'pad' member of struct drm_msm_gem_info to 'hint'. If the >> user sets 'hint' to non-zero it means that they want a IOVA for the >> GEM object instead of a mmap() offset. Return the iova in the 'offset' >> member. >> >> Signed-off-by: Jordan Crouse >> --- >> drivers/gpu/drm/msm/msm_drv.c | 29 + >> include/uapi/drm/msm_drm.h| 4 ++-- >> 2 files changed, 27 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c >> index e29bb66..1e4e022 100644 >> --- a/drivers/gpu/drm/msm/msm_drv.c >> +++ b/drivers/gpu/drm/msm/msm_drv.c >> @@ -677,6 +677,17 @@ static int msm_ioctl_gem_cpu_fini(struct drm_device >> *dev, void *data, >> return ret; >> } >> >> +static int msm_ioctl_gem_info_iova(struct drm_device *dev, >> + struct drm_gem_object *obj, uint64_t *iova) >> +{ >> + struct msm_drm_private *priv = dev->dev_private; >> + >> + if (!priv->gpu) >> + return -EINVAL; >> + > Not too familiar with msm so perhaps a silly question: how can we trigger > this ? if gpu has not loaded (for example, missing firmware, or kernel does not have iommu, etc) >> + return msm_gem_get_iova(obj, priv->gpu->aspace, iova); >> +} >> + >> static int msm_ioctl_gem_info(struct drm_device *dev, void *data, >> struct drm_file *file) >> { >> @@ -684,14 +695,24 @@ static int msm_ioctl_gem_info(struct drm_device *dev, >> void *data, >> struct drm_gem_object *obj; >> int ret = 0; >> >> - if (args->pad) >> - return -EINVAL; >> - > Please keep the input validation before doing any work (the lookup below). +1 for making this args->flags and checking against GEM_INFO_VALID_FLAGS up front. We may want to use some of those other bits some day >> obj = drm_gem_object_lookup(file, args->handle); >> if (!obj) >> return -ENOENT; >> >> - args->offset = msm_gem_mmap_offset(obj); >> + /* >> +* If the hint variable is set, it means that the user wants a IOVA >> for >> +* this buffer. Return the address from the GPU because that is >> +* probably what it is looking for >> +*/ >> + if (args->hint) { > One could also rename hint to flags. Regardless of the name you can > use hint/flags as a bitmask. > >> + uint64_t iova; >> + >> + ret = msm_ioctl_gem_info_iova(dev, obj, ); >> + if (!ret) >> + args->offset = iova; >> + } else { >> + args->offset = msm_gem_mmap_offset(obj); >> + } >> >> drm_gem_object_unreference_unlocked(obj); >> >> diff --git a/include/uapi/drm/msm_drm.h b/include/uapi/drm/msm_drm.h >> index 4d5d6a2..045ad20 100644 >> --- a/include/uapi/drm/msm_drm.h >> +++ b/include/uapi/drm/msm_drm.h >> @@ -105,8 +105,8 @@ struct drm_msm_gem_new { >> >> struct drm_msm_gem_info { >> __u32 handle; /* in */ >> - __u32 pad; >> - __u64 offset; /* out, offset to pass to mmap() */ >> + __u32 hint; /* in */ > Please add explicit #define for the currently valid hints/flags. > >> + __u64 offset; /* out, mmap() offset if hint is 0, iova if 1 >> */ > Other drivers have used anonymous unions to improve the naming, in > such situations. > > struct drm_msm_gem_info { > __u32 handle; /* in */ > __u32 hint; /* in */ > union { /* out */ > __u64 offset; /* offset if hint is FOO */ > __u64 iova; /* iova if hint is BAR */ > }; > }; is anon union legit for uabi? I was under the impression that for some reason it was not. But I could be wrong. BR, -R > > Thanks > Emil > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 1/7] drm: Add DRM support for tiny LCD displays
Den 06.02.2017 10.17, skrev Thierry Reding: On Tue, Jan 31, 2017 at 05:03:13PM +0100, Noralf Trønnes wrote: tinydrm provides helpers for very simple displays that can use CMA backed framebuffers and need flushing on changes. Signed-off-by: Noralf TrønnesAcked-by: Daniel Vetter --- Changes since version 2: - Remove fbdev after drm unregister, not before. Changes since version 1: - Add tinydrm.rst - Set tdev->fbdev_cma=NULL on unregister (lastclose is called after that). - Remove some DRM_DEBUG*() Documentation/gpu/index.rst | 1 + Documentation/gpu/tinydrm.rst | 21 ++ drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile| 1 + drivers/gpu/drm/tinydrm/Kconfig | 8 + drivers/gpu/drm/tinydrm/Makefile| 1 + drivers/gpu/drm/tinydrm/core/Makefile | 3 + drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 377 drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c | 234 + include/drm/tinydrm/tinydrm.h | 115 + 10 files changed, 763 insertions(+) create mode 100644 Documentation/gpu/tinydrm.rst create mode 100644 drivers/gpu/drm/tinydrm/Kconfig create mode 100644 drivers/gpu/drm/tinydrm/Makefile create mode 100644 drivers/gpu/drm/tinydrm/core/Makefile create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-core.c create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c create mode 100644 include/drm/tinydrm/tinydrm.h I realize this is totally subjective, but this feels somewhat too much of a separation. Given the helper nature of TinyDRM, I think it would be more appropriate to move the helpers themselves into drm_tiny.[ch] and then maybe add a subdirectory drivers/gpu/drm/tiny that contains all the drivers that use the helpers. The separation above further shows in subsequent patches where helpers are added to tinydrm that aren't specific to TinyDRM. So this make the new helpers appear as more of a subsystem in DRM rather than a helper library. It also makes things somewhat inconsistent with existing infrastructure. What I have done with tinydrm is to do as little as possible in the core helper. The minimum required to pull together drm_simple_display_pipe, cma helper + fbdev and framebuffer flushing. Then I have added a set of functions that ease the writing of drivers for RGB565 displays with optional backlight and regulator. Added to that is a controller library that handles register access (will use regmap for non-mipi) and framebuffer flushing (set the windowing registers and write the buffer to the pixel register). And at last there's the display driver that initializes the controller to match a particular panel. Maybe I should narrow tinydrm to _just_ support "RGB565 displays with optional backlight and regulator". It looks like I split it up to much, unless someone sees a need for the core of tinydrm elsewhere. I think it's hard to avoid the subsystem smell here. These displays are really simple, fbdev is more than enough to cover their needs. But fbdev is closed. I'm glad you pick on this, as getting the architecture right will save me maintenance down the line. I did paint me into a corner with staging/fbtft and I'm not keen on repeating that (to my defence it was my first C code since university and I had 2 displays that had some similarities which ended up as fbtft). Noralf. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 03/11] drm/msm: Add hint to DRM_IOCTL_MSM_GEM_INFO to return an object IOVA
Hi Jordan, On 6 February 2017 at 17:39, Jordan Crousewrote: > Modify the 'pad' member of struct drm_msm_gem_info to 'hint'. If the > user sets 'hint' to non-zero it means that they want a IOVA for the > GEM object instead of a mmap() offset. Return the iova in the 'offset' > member. > > Signed-off-by: Jordan Crouse > --- > drivers/gpu/drm/msm/msm_drv.c | 29 + > include/uapi/drm/msm_drm.h| 4 ++-- > 2 files changed, 27 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c > index e29bb66..1e4e022 100644 > --- a/drivers/gpu/drm/msm/msm_drv.c > +++ b/drivers/gpu/drm/msm/msm_drv.c > @@ -677,6 +677,17 @@ static int msm_ioctl_gem_cpu_fini(struct drm_device > *dev, void *data, > return ret; > } > > +static int msm_ioctl_gem_info_iova(struct drm_device *dev, > + struct drm_gem_object *obj, uint64_t *iova) > +{ > + struct msm_drm_private *priv = dev->dev_private; > + > + if (!priv->gpu) > + return -EINVAL; > + Not too familiar with msm so perhaps a silly question: how can we trigger this ? > + return msm_gem_get_iova(obj, priv->gpu->aspace, iova); > +} > + > static int msm_ioctl_gem_info(struct drm_device *dev, void *data, > struct drm_file *file) > { > @@ -684,14 +695,24 @@ static int msm_ioctl_gem_info(struct drm_device *dev, > void *data, > struct drm_gem_object *obj; > int ret = 0; > > - if (args->pad) > - return -EINVAL; > - Please keep the input validation before doing any work (the lookup below). > obj = drm_gem_object_lookup(file, args->handle); > if (!obj) > return -ENOENT; > > - args->offset = msm_gem_mmap_offset(obj); > + /* > +* If the hint variable is set, it means that the user wants a IOVA > for > +* this buffer. Return the address from the GPU because that is > +* probably what it is looking for > +*/ > + if (args->hint) { One could also rename hint to flags. Regardless of the name you can use hint/flags as a bitmask. > + uint64_t iova; > + > + ret = msm_ioctl_gem_info_iova(dev, obj, ); > + if (!ret) > + args->offset = iova; > + } else { > + args->offset = msm_gem_mmap_offset(obj); > + } > > drm_gem_object_unreference_unlocked(obj); > > diff --git a/include/uapi/drm/msm_drm.h b/include/uapi/drm/msm_drm.h > index 4d5d6a2..045ad20 100644 > --- a/include/uapi/drm/msm_drm.h > +++ b/include/uapi/drm/msm_drm.h > @@ -105,8 +105,8 @@ struct drm_msm_gem_new { > > struct drm_msm_gem_info { > __u32 handle; /* in */ > - __u32 pad; > - __u64 offset; /* out, offset to pass to mmap() */ > + __u32 hint; /* in */ Please add explicit #define for the currently valid hints/flags. > + __u64 offset; /* out, mmap() offset if hint is 0, iova if 1 */ Other drivers have used anonymous unions to improve the naming, in such situations. struct drm_msm_gem_info { __u32 handle; /* in */ __u32 hint; /* in */ union { /* out */ __u64 offset; /* offset if hint is FOO */ __u64 iova; /* iova if hint is BAR */ }; }; Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 00/11] drm/msm: A5XX preemption
On Mon, Feb 6, 2017 at 1:23 PM, Daniel Stonewrote: > Hi, > > On 6 February 2017 at 17:59, Daniel Vetter wrote: >> On Mon, Feb 06, 2017 at 10:39:28AM -0700, Jordan Crouse wrote: >>> This initial series implements 4 ringbuffers to give sufficient coverage >>> for the >>> range of priority levels requested by the GLES and compute extensions. The >>> targeted ringbuffer is specified in the command submission flags. The >>> default >>> ring is 0 (lowest priority). >> >> Link to userspace part that implements these extensions? Also which >> gles/compute extensions are you talking about? Asking not just because of >> the open source userspace requirement, but also because we want to >> upstream a scheduler on the i915 side. Getting alignment on that across >> drm drivers would be sweet. > > Assuming he meant EGL rather than GLES, this is the usual one: > https://www.khronos.org/registry/EGL/extensions/IMG/EGL_IMG_context_priority.txt > There was an RFC for this from Chris.. not sure if it landed yet, but that is what I was planning to use from the userspace side. Probably first step would be to enable this without preemption points. Although I think I have a reasonable idea how the preemption points work.. BR, -R ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/11] drm/msm: A5XX preemption
On Mon, Feb 6, 2017 at 12:59 PM, Daniel Vetterwrote: > On Mon, Feb 06, 2017 at 10:39:28AM -0700, Jordan Crouse wrote: >> This series of patches implements multiple ringbuffers and preemption for >> Adreno >> A5XX targets. Preemption allows a command to be interrupted at specific >> preemption points and execution switched to a different ringbuffer. >> >> The software alogrithm uses preemption to enforce quality of service for >> priority levels - commands to a certain ring preempt the rings of lower >> priority. Note that priority is a software construct; the driver chooses a >> ring >> to switch to and the hardware executes. This is important because it shows >> that >> preemption can be used for things other than priority (timeslices for >> quality of >> service for example). >> >> This initial series implements 4 ringbuffers to give sufficient coverage for >> the >> range of priority levels requested by the GLES and compute extensions. The >> targeted ringbuffer is specified in the command submission flags. The default >> ring is 0 (lowest priority). > > Link to userspace part that implements these extensions? Also which > gles/compute extensions are you talking about? Asking not just because of > the open source userspace requirement, but also because we want to > upstream a scheduler on the i915 side. Getting alignment on that across > drm drivers would be sweet. FWIW, we have had a GPU scheduler in amdgpu for several years now. We purposely tried to keep it largely separate from our driver so others could leverage it if they wanted to. See drivers/gpu/drm/amd/scheduler in the kernel. Alex > > Adding intel-gfx, I'll poke the folks working on this too. > -Daniel > >> >> Jordan >> >> Jordan Crouse (11): >> drm/msm: Make sure to detach the MMU during GPU cleanup >> drm/msm: Improve the zap shader >> drm/msm: Add hint to DRM_IOCTL_MSM_GEM_INFO to return an object IOVA >> drm/msm: Remove idle function hook >> drm/msm: get an iova from the address space instead of an id >> drm/msm: Add a struct to pass configuration to msm_gpu_init() >> drm/msm: Remove memptrs->wptr >> drm/msm: Support multiple ringbuffers >> drm/msm: Shadow current pointer in the ring until command is complete >> drm/msm: Make the value of RB_CNTL (almost) generic >> drm/msm: Implement preemption for A5XX targets >> >> drivers/gpu/drm/msm/Makefile | 1 + >> drivers/gpu/drm/msm/adreno/a3xx_gpu.c | 13 +- >> drivers/gpu/drm/msm/adreno/a4xx_gpu.c | 13 +- >> drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 278 +- >> drivers/gpu/drm/msm/adreno/a5xx_gpu.h | 106 + >> drivers/gpu/drm/msm/adreno/a5xx_power.c | 11 +- >> drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 367 >> ++ >> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 215 +++-- >> drivers/gpu/drm/msm/adreno/adreno_gpu.h | 42 ++-- >> drivers/gpu/drm/msm/dsi/dsi_host.c| 15 +- >> drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c | 8 +- >> drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 18 +- >> drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.h | 3 - >> drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c | 13 +- >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 5 +- >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 11 +- >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 4 - >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 13 +- >> drivers/gpu/drm/msm/msm_drv.c | 43 ++-- >> drivers/gpu/drm/msm/msm_drv.h | 27 ++- >> drivers/gpu/drm/msm/msm_fb.c | 15 +- >> drivers/gpu/drm/msm/msm_fbdev.c | 10 +- >> drivers/gpu/drm/msm/msm_fence.c | 85 +-- >> drivers/gpu/drm/msm/msm_fence.h | 13 +- >> drivers/gpu/drm/msm/msm_gem.c | 124 +++--- >> drivers/gpu/drm/msm/msm_gem.h | 5 +- >> drivers/gpu/drm/msm/msm_gem_submit.c | 14 +- >> drivers/gpu/drm/msm/msm_gpu.c | 140 +++- >> drivers/gpu/drm/msm/msm_gpu.h | 54 - >> drivers/gpu/drm/msm/msm_kms.h | 3 + >> drivers/gpu/drm/msm/msm_ringbuffer.c | 14 +- >> drivers/gpu/drm/msm/msm_ringbuffer.h | 21 +- >> include/uapi/drm/msm_drm.h| 9 +- >> 33 files changed, 1324 insertions(+), 389 deletions(-) >> create mode 100644 drivers/gpu/drm/msm/adreno/a5xx_preempt.c >> >> -- >> 1.9.1 >> >> ___ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org
[PATCH] drm/atmel-hlcdc: Simplify the HLCDC layer logic
An HLCDC layers in Atmel's nomenclature is either a DRM plane or a 'Post Processing Layer' which can be used to output the results of the HLCDC composition in a memory buffer. atmel_hlcdc_layer.c was designed to be generic enough to be re-usable in both cases, but we're not exposing the post-processing layer yet, and even if we were, I'm not sure the code would provide the necessary tools to manipulate this kind of layer. Moreover, the code in atmel_hlcdc_{plane,layer}.c was designed before the atomic modesetting API, and was trying solve the check-setting/commit-if-ok/rollback-otherwise problem, which is now entirely solved by the existing core infrastructure. And finally, the code in atmel_hlcdc_layer.c in over-complicated compared to what we really need. This rework is a good excuse to simplify it. Note that this rework solves an existing resource leak (leading to a -EBUSY error) which I failed to clearly identify. Signed-off-by: Boris Brezillon--- Hi Daniel, You might not remember, but this is something you asked me to do a while ago, and it's finally there. This patch reworks the Atmel HLCDC plane logic to get rid of all the complexity in atmel_hlcdc_layer.c and this includes getting rid of drm_flip_work, which you were trying to kill IIRC. Regards, Boris --- drivers/gpu/drm/atmel-hlcdc/Makefile| 1 - drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 32 +- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c| 81 +-- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h| 62 +-- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.c | 666 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.h | 334 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 618 +++--- 7 files changed, 522 insertions(+), 1272 deletions(-) delete mode 100644 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.c diff --git a/drivers/gpu/drm/atmel-hlcdc/Makefile b/drivers/gpu/drm/atmel-hlcdc/Makefile index 10ae426e60bd..bb5f8507a8ce 100644 --- a/drivers/gpu/drm/atmel-hlcdc/Makefile +++ b/drivers/gpu/drm/atmel-hlcdc/Makefile @@ -1,6 +1,5 @@ atmel-hlcdc-dc-y := atmel_hlcdc_crtc.o \ atmel_hlcdc_dc.o \ - atmel_hlcdc_layer.o \ atmel_hlcdc_output.o \ atmel_hlcdc_plane.o diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c index 9b17a66cf0e1..cdf8aa2b7a8d 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c @@ -445,8 +445,8 @@ static const struct drm_crtc_funcs atmel_hlcdc_crtc_funcs = { int atmel_hlcdc_crtc_create(struct drm_device *dev) { + struct drm_plane *primary = NULL, *cursor = NULL; struct atmel_hlcdc_dc *dc = dev->dev_private; - struct atmel_hlcdc_planes *planes = dc->planes; struct atmel_hlcdc_crtc *crtc; int ret; int i; @@ -457,20 +457,32 @@ int atmel_hlcdc_crtc_create(struct drm_device *dev) crtc->dc = dc; - ret = drm_crtc_init_with_planes(dev, >base, - >primary->base, - planes->cursor ? >cursor->base : NULL, - _hlcdc_crtc_funcs, NULL); + for (i = 0; i < ATMEL_HLCDC_MAX_LAYERS; i++) { + switch (dc->layers[i].type) { + case ATMEL_HLCDC_BASE_LAYER: + primary = dc->layers[i].plane; + break; + + case ATMEL_HLCDC_CURSOR_LAYER: + cursor = dc->layers[i].plane; + break; + + default: + break; + } + } + + ret = drm_crtc_init_with_planes(dev, >base, primary, cursor, + _hlcdc_crtc_funcs, NULL); if (ret < 0) goto fail; crtc->id = drm_crtc_index(>base); - if (planes->cursor) - planes->cursor->base.possible_crtcs = 1 << crtc->id; - - for (i = 0; i < planes->noverlays; i++) - planes->overlays[i]->base.possible_crtcs = 1 << crtc->id; + for (i = 0; i < ATMEL_HLCDC_MAX_LAYERS; i++) { + if (dc->layers[i].type == ATMEL_HLCDC_OVERLAY_LAYER) + dc->layers[i].plane->possible_crtcs = 1 << crtc->id; + } drm_crtc_helper_add(>base, _crtc_helper_funcs); drm_crtc_vblank_reset(>base); diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c index 0bf32d6ac39b..5e7ba6de1777 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c @@ -36,7 +36,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9n12_layers[] = { .regs_offset = 0x40, .id = 0, .type = ATMEL_HLCDC_BASE_LAYER, - .nconfigs =
Re: [PATCH 00/11] drm/msm: A5XX preemption
Hi, On 6 February 2017 at 17:59, Daniel Vetterwrote: > On Mon, Feb 06, 2017 at 10:39:28AM -0700, Jordan Crouse wrote: >> This initial series implements 4 ringbuffers to give sufficient coverage for >> the >> range of priority levels requested by the GLES and compute extensions. The >> targeted ringbuffer is specified in the command submission flags. The default >> ring is 0 (lowest priority). > > Link to userspace part that implements these extensions? Also which > gles/compute extensions are you talking about? Asking not just because of > the open source userspace requirement, but also because we want to > upstream a scheduler on the i915 side. Getting alignment on that across > drm drivers would be sweet. Assuming he meant EGL rather than GLES, this is the usual one: https://www.khronos.org/registry/EGL/extensions/IMG/EGL_IMG_context_priority.txt Cheers, Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/11] drm/msm: A5XX preemption
On Mon, Feb 06, 2017 at 10:39:28AM -0700, Jordan Crouse wrote: > This series of patches implements multiple ringbuffers and preemption for > Adreno > A5XX targets. Preemption allows a command to be interrupted at specific > preemption points and execution switched to a different ringbuffer. > > The software alogrithm uses preemption to enforce quality of service for > priority levels - commands to a certain ring preempt the rings of lower > priority. Note that priority is a software construct; the driver chooses a > ring > to switch to and the hardware executes. This is important because it shows > that > preemption can be used for things other than priority (timeslices for quality > of > service for example). > > This initial series implements 4 ringbuffers to give sufficient coverage for > the > range of priority levels requested by the GLES and compute extensions. The > targeted ringbuffer is specified in the command submission flags. The default > ring is 0 (lowest priority). Link to userspace part that implements these extensions? Also which gles/compute extensions are you talking about? Asking not just because of the open source userspace requirement, but also because we want to upstream a scheduler on the i915 side. Getting alignment on that across drm drivers would be sweet. Adding intel-gfx, I'll poke the folks working on this too. -Daniel > > Jordan > > Jordan Crouse (11): > drm/msm: Make sure to detach the MMU during GPU cleanup > drm/msm: Improve the zap shader > drm/msm: Add hint to DRM_IOCTL_MSM_GEM_INFO to return an object IOVA > drm/msm: Remove idle function hook > drm/msm: get an iova from the address space instead of an id > drm/msm: Add a struct to pass configuration to msm_gpu_init() > drm/msm: Remove memptrs->wptr > drm/msm: Support multiple ringbuffers > drm/msm: Shadow current pointer in the ring until command is complete > drm/msm: Make the value of RB_CNTL (almost) generic > drm/msm: Implement preemption for A5XX targets > > drivers/gpu/drm/msm/Makefile | 1 + > drivers/gpu/drm/msm/adreno/a3xx_gpu.c | 13 +- > drivers/gpu/drm/msm/adreno/a4xx_gpu.c | 13 +- > drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 278 +- > drivers/gpu/drm/msm/adreno/a5xx_gpu.h | 106 + > drivers/gpu/drm/msm/adreno/a5xx_power.c | 11 +- > drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 367 > ++ > drivers/gpu/drm/msm/adreno/adreno_gpu.c | 215 +++-- > drivers/gpu/drm/msm/adreno/adreno_gpu.h | 42 ++-- > drivers/gpu/drm/msm/dsi/dsi_host.c| 15 +- > drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c | 8 +- > drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 18 +- > drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.h | 3 - > drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c | 13 +- > drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 5 +- > drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 11 +- > drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 4 - > drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 13 +- > drivers/gpu/drm/msm/msm_drv.c | 43 ++-- > drivers/gpu/drm/msm/msm_drv.h | 27 ++- > drivers/gpu/drm/msm/msm_fb.c | 15 +- > drivers/gpu/drm/msm/msm_fbdev.c | 10 +- > drivers/gpu/drm/msm/msm_fence.c | 85 +-- > drivers/gpu/drm/msm/msm_fence.h | 13 +- > drivers/gpu/drm/msm/msm_gem.c | 124 +++--- > drivers/gpu/drm/msm/msm_gem.h | 5 +- > drivers/gpu/drm/msm/msm_gem_submit.c | 14 +- > drivers/gpu/drm/msm/msm_gpu.c | 140 +++- > drivers/gpu/drm/msm/msm_gpu.h | 54 - > drivers/gpu/drm/msm/msm_kms.h | 3 + > drivers/gpu/drm/msm/msm_ringbuffer.c | 14 +- > drivers/gpu/drm/msm/msm_ringbuffer.h | 21 +- > include/uapi/drm/msm_drm.h| 9 +- > 33 files changed, 1324 insertions(+), 389 deletions(-) > create mode 100644 drivers/gpu/drm/msm/adreno/a5xx_preempt.c > > -- > 1.9.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[pull] drm/msm: msm-next for 4.11
Hi Dave, The big things this time around are: 1) support for hw cursor on newer mdp5 devices (snapdragon 820+, tested on db820c) 2) dsi encoder cleanup 3) gpu dt bindings cleanup so we can get the gpu nodes merged upstream The following changes since commit 99743ae4c5f52f8f8ceb17783056fcc9b4f8b64c: Merge branch 'drm-etnaviv-next' of https://git.pengutronix.de/git/lst/linux into drm-next (2017-02-03 05:41:58 +1000) are available in the git repository at: git://people.freedesktop.org/~robclark/linux msm-next for you to fetch changes up to 21c42da18ef128ca8fb4cc4ead888f5c61e3916a: drm/msm: return -EFAULT if copy_from_user() fails (2017-02-06 11:28:45 -0500) Archit Taneja (22): drm/msm/mdp5: cfg: Add pipe_cursor block drm/msm/mdp5: Update generated headers drm/msm/dsi: Update generated headers drm/msm/dsi: Set msm_dsi->encoders before initializing bridge drm/msm: Construct only one encoder for DSI drm/msm: Set encoder's mode of operation using a kms func drm/msm/mdp5: Prepare for merging video and command encoders drm/msm/mdp5: Create single encoder per interface (INTF) drm/msm/mdp5: cfg: Change count to unsigned int drm/msm/mdp5: Create only as many CRTCs as we need drm/msm/mdp5: Prepare CRTC/LM for empty stages drm/msm/mdp5: Use plane helpers to configure src/dst rectangles drm/msm/mdp5: Configure COLOR3_OUT propagation drm/msm/mdp5: Misc cursor plane bits drm/msm/mdp5: Add cursor planes drm/msm/mdp5: Refactor mdp5_plane_atomic_check drm/msm/mdp5: Add support for legacy cursor updates drm/msm/dsi: Don't error if a DSI host doesn't have a device connected drm/msm/dsi: Add 8x96 info in dsi_cfg drm/msm/dsi: Add a PHY op that initializes version specific stuff drm/msm/dsi: Reset both PHYs before clock operation for dual DSI drm/msm/dsi: Add PHY/PLL for 8x96 Dan Carpenter (1): drm/msm: return -EFAULT if copy_from_user() fails Hai Li (4): drm/msm/dsi: Return more timings from PHY to host drm/msm/dsi: Pass down use case to PHY drm/msm/dsi: Move PHY operations out of host drm/msm/dsi: Add new method to calculate 14nm PHY timings Rob Clark (5): drm/msm: remove qcom,gpu-pwrlevels bindings drm/msm: drop qcom,chipid drm/msm: drop quirks binding drm/msm: drop _clk suffix from clk names drm/msm: let gpu wire up it's own fault handler .../devicetree/bindings/display/msm/gpu.txt| 38 +- drivers/gpu/drm/msm/Kconfig|7 + drivers/gpu/drm/msm/Makefile |2 + drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 21 +- drivers/gpu/drm/msm/adreno/adreno_device.c | 62 +- drivers/gpu/drm/msm/adreno/adreno_gpu.c|1 - drivers/gpu/drm/msm/adreno/adreno_gpu.h|4 +- drivers/gpu/drm/msm/dsi/dsi.c | 18 +- drivers/gpu/drm/msm/dsi/dsi.h | 51 +- drivers/gpu/drm/msm/dsi/dsi.xml.h | 269 - drivers/gpu/drm/msm/dsi/dsi_cfg.c | 25 + drivers/gpu/drm/msm/dsi/dsi_cfg.h |1 + drivers/gpu/drm/msm/dsi/dsi_host.c | 97 +- drivers/gpu/drm/msm/dsi/dsi_manager.c | 254 +++-- drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 239 - drivers/gpu/drm/msm/dsi/phy/dsi_phy.h | 20 +- drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 169 +++ drivers/gpu/drm/msm/dsi/phy/dsi_phy_20nm.c |5 +- drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c |6 +- drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c|5 +- drivers/gpu/drm/msm/dsi/pll/dsi_pll.c | 12 + drivers/gpu/drm/msm/dsi/pll/dsi_pll.h | 11 + drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c | 1104 drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c| 28 +- drivers/gpu/drm/msm/mdp/mdp5/mdp5.xml.h| 48 +- drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c| 10 + drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h|3 +- drivers/gpu/drm/msm/mdp/mdp5/mdp5_cmd_encoder.c| 135 +-- drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 73 +- drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.c| 14 +- drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.h|4 +- drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c| 77 +- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c| 123 ++- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h| 45 +- drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c |8 + drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 181 +++- drivers/gpu/drm/msm/mdp/mdp_kms.h |1 + drivers/gpu/drm/msm/msm_atomic.c | 26 +- drivers/gpu/drm/msm/msm_drv.c | 20 +
[PATCH 02/11] drm/msm: Improve the zap shader
Simply the code, use snprintf correctly and make sure that we memset the rest of the segment if the memory size in the ELF file is larger than the file size. Signed-off-by: Jordan Crouse--- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 60 +-- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c index 5f8b368..23eeed2 100644 --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c @@ -31,11 +31,11 @@ static inline bool _check_segment(const struct elf32_phdr *phdr) phdr->p_memsz); } -static int __pil_tz_load_image(struct platform_device *pdev, +static int zap_load_segments(struct platform_device *pdev, const struct firmware *mdt, const char *fwname, void *fwptr, size_t fw_size, unsigned long fw_min_addr) { - char str[64] = { 0 }; + char filename[64]; const struct elf32_hdr *ehdr = (struct elf32_hdr *) mdt->data; const struct elf32_phdr *phdrs = (struct elf32_phdr *) (ehdr + 1); const struct firmware *fw; @@ -53,16 +53,18 @@ static int __pil_tz_load_image(struct platform_device *pdev, offset = (phdr->p_paddr - fw_min_addr); /* Request the file containing the segment */ - snprintf(str, sizeof(str) - 1, "%s.b%02d", fwname, i); + snprintf(filename, sizeof(filename), "%s.b%02d", fwname, i); - ret = request_firmware(, str, >dev); + ret = request_firmware(, filename, >dev); if (ret) { - dev_err(>dev, "Failed to load segment %s\n", str); + dev_err(>dev, "Failed to load segment %s\n", + filename); break; } if (offset + fw->size > fw_size) { - dev_err(>dev, "Segment %s is too big\n", str); + dev_err(>dev, "Segment %s is too big\n", + filename); ret = -EINVAL; release_firmware(fw); break; @@ -70,15 +72,19 @@ static int __pil_tz_load_image(struct platform_device *pdev, /* Copy the segment into place */ memcpy(fwptr + offset, fw->data, fw->size); + + if (phdr->p_memsz > phdr->p_filesz) + memset(fwptr + fw->size, 0, + phdr->p_memsz - phdr->p_filesz); release_firmware(fw); } return ret; } -static int _pil_tz_load_image(struct platform_device *pdev) +static int zap_load_mdt(struct platform_device *pdev) { - char str[64] = { 0 }; + char filename[64]; const char *fwname; const struct elf32_hdr *ehdr; const struct elf32_phdr *phdrs; @@ -86,7 +92,6 @@ static int _pil_tz_load_image(struct platform_device *pdev) phys_addr_t fw_min_addr, fw_max_addr; dma_addr_t fw_phys; size_t fw_size; - u32 pas_id; void *ptr; int i, ret; @@ -95,11 +100,10 @@ static int _pil_tz_load_image(struct platform_device *pdev) if (!qcom_scm_is_available()) { dev_err(>dev, "SCM is not available\n"); - return -EINVAL; + return -EPROBE_DEFER; } ret = of_reserved_mem_device_init(>dev); - if (ret) { dev_err(>dev, "Unable to set up the reserved memory\n"); return ret; @@ -112,17 +116,12 @@ static int _pil_tz_load_image(struct platform_device *pdev) return -EINVAL; } - if (of_property_read_u32(pdev->dev.of_node, "qcom,pas-id", _id)) { - dev_err(>dev, "Could not read the pas ID\n"); - return -EINVAL; - } - - snprintf(str, sizeof(str) - 1, "%s.mdt", fwname); + snprintf(filename, sizeof(filename), "%s.mdt", fwname); /* Request the MDT file for the firmware */ - ret = request_firmware(, str, >dev); + ret = request_firmware(, filename, >dev); if (ret) { - dev_err(>dev, "Unable to load %s\n", str); + dev_err(>dev, "Unable to load %s\n", filename); return ret; } @@ -151,7 +150,7 @@ static int _pil_tz_load_image(struct platform_device *pdev) fw_size = (size_t) (fw_max_addr - fw_min_addr); /* Verify the MDT header */ - ret = qcom_scm_pas_init_image(pas_id, mdt->data, mdt->size); + ret = qcom_scm_pas_init_image(13, mdt->data, mdt->size); if (ret) { dev_err(>dev, "Invalid firmware metadata\n"); goto out; @@ -163,18 +162,19 @@ static int _pil_tz_load_image(struct platform_device *pdev) goto out; /* Set up the newly allocated memory region */ - ret
Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node
On Mon, Feb 6, 2017 at 4:52 AM, Philipp Zabelwrote: > On Fri, 2017-02-03 at 21:36 -0600, Rob Herring wrote: >> Convert drivers to use the new of_graph_get_remote_node() helper >> instead of parsing the endpoint node and then getting the remote device >> node. Now drivers can just specify the device node and which >> port/endpoint and get back the connected remote device node. The details >> of the graph binding are nicely abstracted into the core OF graph code. >> >> This changes some error messages to debug messages (in the graph core). >> Graph connections are often "no connects" depending on the particular >> board, so we want to avoid spurious messages. Plus the kernel is not a >> DT validator. >> >> Signed-off-by: Rob Herring >> --- >> drivers/gpu/drm/arm/hdlcd_drv.c | 22 ++--- >> drivers/gpu/drm/arm/malidp_drv.c| 29 ++- >> drivers/gpu/drm/bridge/adv7511/adv7533.c| 12 + >> drivers/gpu/drm/bridge/dumb-vga-dac.c | 15 ++ >> drivers/gpu/drm/bridge/ti-tfp410.c | 15 ++ >> drivers/gpu/drm/exynos/exynos_drm_dpi.c | 16 +- >> drivers/gpu/drm/exynos/exynos_drm_dsi.c | 13 ++--- >> drivers/gpu/drm/exynos/exynos_drm_mic.c | 27 +- >> drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c| 26 ++ >> drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 30 +-- >> drivers/gpu/drm/mediatek/mtk_dpi.c | 12 ++--- >> drivers/gpu/drm/mediatek/mtk_hdmi.c | 26 ++ >> drivers/gpu/drm/meson/meson_drv.c | 12 ++--- >> drivers/gpu/drm/meson/meson_venc_cvbs.c | 19 ++- >> drivers/gpu/drm/msm/dsi/dsi_host.c | 3 +- >> drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 28 +-- >> drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 18 +++ >> drivers/gpu/drm/tilcdc/tilcdc_crtc.c| 11 + >> drivers/gpu/drm/tilcdc/tilcdc_external.c| 66 >> +++-- >> drivers/gpu/drm/vc4/vc4_dpi.c | 15 ++ >> 20 files changed, 64 insertions(+), 351 deletions(-) >> > [...] >> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c >> b/drivers/gpu/drm/mediatek/mtk_dpi.c >> index 90fb831ef031..dbd554c09a39 100644 >> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c >> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c >> @@ -661,7 +661,7 @@ static int mtk_dpi_probe(struct platform_device *pdev) >> struct device *dev = >dev; >> struct mtk_dpi *dpi; >> struct resource *mem; >> - struct device_node *ep, *bridge_node = NULL; >> + struct device_node *bridge_node; >> int comp_id; >> int ret; >> >> @@ -706,15 +706,9 @@ static int mtk_dpi_probe(struct platform_device *pdev) >> return -EINVAL; >> } >> >> - ep = of_graph_get_next_endpoint(dev->of_node, NULL); >> - if (ep) { >> - bridge_node = of_graph_get_remote_port_parent(ep); >> - of_node_put(ep); >> - } >> - if (!bridge_node) { >> - dev_err(dev, "Failed to find bridge node\n"); >> + bridge_node = of_graph_get_remote_node(dev->of_node, 0, 0); > > Note that before this change, of_graph_get_next_endpoint would just > choose the first port/endpoint without restrictions to their reg > properties, whereas the new code requires reg to be either not set or > set to zero. > As the former is the case for the mt8173 dpi->hdmi link, which is the > only use of this driver, this should be fine. Yes, that is on purpose. A given node should have known port numbering and know what type of port each port is. The binding doc should outline the ports and endpoints. > > [...] >> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c >> b/drivers/gpu/drm/mediatek/mtk_hdmi.c >> index 0e8c4d9af340..f14e472812ce 100644 >> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c >> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c >> @@ -1433,7 +1433,7 @@ static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi >> *hdmi, >> { >> struct device *dev = >dev; >> struct device_node *np = dev->of_node; >> - struct device_node *cec_np, *port, *ep, *remote, *i2c_np; >> + struct device_node *cec_np, *remote, *i2c_np; >> struct platform_device *cec_pdev; >> struct regmap *regmap; >> struct resource *mem; >> @@ -1485,29 +1485,9 @@ static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi >> *hdmi, >> if (IS_ERR(hdmi->regs)) >> return PTR_ERR(hdmi->regs); >> >> - port = of_graph_get_port_by_id(np, 1); >> - if (!port) { >> - dev_err(dev, "Missing output port node\n"); >> + remote = of_graph_get_remote_node(np, 1, 0); >> + if (!remote) >> return -EINVAL; >> - } >> - >> - ep = of_get_child_by_name(port, "endpoint"); >> - if (!ep) { >> - dev_err(dev, "Missing endpoint node in port %s\n", >> - port->full_name); >> -
[PATCH 08/11] drm/msm: Support multiple ringbuffers
Add the infrastructure to support the idea of multiple ringbuffers. Assign each ringbuffer an id and use that as an index for the various ring specific operations. The biggest delta is to support legacy fences. Each fence gets its own sequence number but the legacy functions expect to use a unique integer. To handle this we return a unique identifer for each submission but map it to a specific ring/sequence under the covers. Newer users use a dma_fence pointer anyway so they don't care about the actual sequence ID or ring. The actual mechanics for multiple ringbuffers are very target specific so this code just allows for the possibility but still only defines one ringbuffer for each target family. Signed-off-by: Jordan Crouse--- drivers/gpu/drm/msm/adreno/a3xx_gpu.c | 9 +- drivers/gpu/drm/msm/adreno/a4xx_gpu.c | 9 +- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 45 +- drivers/gpu/drm/msm/adreno/a5xx_gpu.h | 2 +- drivers/gpu/drm/msm/adreno/a5xx_power.c | 6 +- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 155 +--- drivers/gpu/drm/msm/adreno/adreno_gpu.h | 36 +--- drivers/gpu/drm/msm/msm_drv.h | 2 + drivers/gpu/drm/msm/msm_fence.c | 85 +- drivers/gpu/drm/msm/msm_fence.h | 13 +-- drivers/gpu/drm/msm/msm_gem.h | 1 + drivers/gpu/drm/msm/msm_gem_submit.c| 10 ++- drivers/gpu/drm/msm/msm_gpu.c | 122 - drivers/gpu/drm/msm/msm_gpu.h | 38 ++-- drivers/gpu/drm/msm/msm_ringbuffer.c| 13 ++- drivers/gpu/drm/msm/msm_ringbuffer.h| 8 +- include/uapi/drm/msm_drm.h | 5 ++ 17 files changed, 373 insertions(+), 186 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c index fc4fd2d..2f72848 100644 --- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c @@ -44,7 +44,7 @@ static bool a3xx_me_init(struct msm_gpu *gpu) { - struct msm_ringbuffer *ring = gpu->rb; + struct msm_ringbuffer *ring = gpu->rb[0]; OUT_PKT3(ring, CP_ME_INIT, 17); OUT_RING(ring, 0x03f7); @@ -65,7 +65,7 @@ static bool a3xx_me_init(struct msm_gpu *gpu) OUT_RING(ring, 0x); OUT_RING(ring, 0x); - gpu->funcs->flush(gpu); + gpu->funcs->flush(gpu, ring); return a3xx_idle(gpu); } @@ -339,7 +339,7 @@ static void a3xx_destroy(struct msm_gpu *gpu) static bool a3xx_idle(struct msm_gpu *gpu) { /* wait for ringbuffer to drain: */ - if (!adreno_idle(gpu)) + if (!adreno_idle(gpu, gpu->rb[0])) return false; /* then wait for GPU to finish: */ @@ -449,6 +449,7 @@ static void a3xx_dump(struct msm_gpu *gpu) .last_fence = adreno_last_fence, .submit = adreno_submit, .flush = adreno_flush, + .active_ring = adreno_active_ring, .irq = a3xx_irq, .destroy = a3xx_destroy, #ifdef CONFIG_DEBUG_FS @@ -496,7 +497,7 @@ struct msm_gpu *a3xx_gpu_init(struct drm_device *dev) adreno_gpu->registers = a3xx_registers; adreno_gpu->reg_offsets = a3xx_register_offsets; - ret = adreno_gpu_init(dev, pdev, adreno_gpu, ); + ret = adreno_gpu_init(dev, pdev, adreno_gpu, , 1); if (ret) goto fail; diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c index 6bc948b..bdd2a24 100644 --- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c @@ -116,7 +116,7 @@ static void a4xx_enable_hwcg(struct msm_gpu *gpu) static bool a4xx_me_init(struct msm_gpu *gpu) { - struct msm_ringbuffer *ring = gpu->rb; + struct msm_ringbuffer *ring = gpu->rb[0]; OUT_PKT3(ring, CP_ME_INIT, 17); OUT_RING(ring, 0x03f7); @@ -137,7 +137,7 @@ static bool a4xx_me_init(struct msm_gpu *gpu) OUT_RING(ring, 0x); OUT_RING(ring, 0x); - gpu->funcs->flush(gpu); + gpu->funcs->flush(gpu, ring); return a4xx_idle(gpu); } @@ -337,7 +337,7 @@ static void a4xx_destroy(struct msm_gpu *gpu) static bool a4xx_idle(struct msm_gpu *gpu) { /* wait for ringbuffer to drain: */ - if (!adreno_idle(gpu)) + if (!adreno_idle(gpu, gpu->rb[0])) return false; /* then wait for GPU to finish: */ @@ -539,6 +539,7 @@ static int a4xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value) .last_fence = adreno_last_fence, .submit = adreno_submit, .flush = adreno_flush, + .active_ring = adreno_active_ring, .irq = a4xx_irq, .destroy = a4xx_destroy, #ifdef CONFIG_DEBUG_FS @@ -580,7 +581,7 @@ struct msm_gpu *a4xx_gpu_init(struct drm_device *dev) adreno_gpu->registers =
[PATCH 0/5] Fix the parse_dt of exynos dsi
This patch fixed the parse_dt function of exynos dsi and fixed the related exynos3250, exynos4210, exynos4412, exynos5433 dts. Hoegeun Kwon (5): drm/exynos: dsi: Fix the parse_dt function arm64: dts: exynos: Move the clock-frequency property arm: dts: Move the clock-frequency property for exynos3250 dts arm: dts: Move the clock-frequency property for exynos4412 dts arm: dts: Move the clock-frequency property for exynos4210 dts arch/arm/boot/dts/exynos3250-rinato.dts| 23 ++-- arch/arm/boot/dts/exynos4210-trats.dts | 23 ++-- arch/arm/boot/dts/exynos4412-trats2.dts| 23 ++-- .../boot/dts/exynos/exynos5433-tm2-common.dtsi | 16 ++- drivers/gpu/drm/exynos/exynos_drm_dsi.c| 32 ++ 5 files changed, 16 insertions(+), 101 deletions(-) -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node
On Mon, Feb 06, 2017 at 10:29:33AM +, Liviu Dudau wrote: > On Fri, Feb 03, 2017 at 09:36:33PM -0600, Rob Herring wrote: > > Convert drivers to use the new of_graph_get_remote_node() helper > > instead of parsing the endpoint node and then getting the remote device > > node. Now drivers can just specify the device node and which > > port/endpoint and get back the connected remote device node. The details > > of the graph binding are nicely abstracted into the core OF graph code. > > > > This changes some error messages to debug messages (in the graph core). > > Graph connections are often "no connects" depending on the particular > > board, so we want to avoid spurious messages. Plus the kernel is not a > > DT validator. > > > > Signed-off-by: Rob Herring> > --- > > drivers/gpu/drm/arm/hdlcd_drv.c | 22 ++--- > > drivers/gpu/drm/arm/malidp_drv.c| 29 ++- > > drivers/gpu/drm/bridge/adv7511/adv7533.c| 12 + > > drivers/gpu/drm/bridge/dumb-vga-dac.c | 15 ++ > > drivers/gpu/drm/bridge/ti-tfp410.c | 15 ++ > > drivers/gpu/drm/exynos/exynos_drm_dpi.c | 16 +- > > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 13 ++--- > > drivers/gpu/drm/exynos/exynos_drm_mic.c | 27 +- > > drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c| 26 ++ > > drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 30 +-- > > drivers/gpu/drm/mediatek/mtk_dpi.c | 12 ++--- > > drivers/gpu/drm/mediatek/mtk_hdmi.c | 26 ++ > > drivers/gpu/drm/meson/meson_drv.c | 12 ++--- > > drivers/gpu/drm/meson/meson_venc_cvbs.c | 19 ++- > > drivers/gpu/drm/msm/dsi/dsi_host.c | 3 +- > > drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 28 +-- > > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 18 +++ > > drivers/gpu/drm/tilcdc/tilcdc_crtc.c| 11 + > > drivers/gpu/drm/tilcdc/tilcdc_external.c| 66 > > +++-- > > drivers/gpu/drm/vc4/vc4_dpi.c | 15 ++ > > 20 files changed, 64 insertions(+), 351 deletions(-) > > > > diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c > > b/drivers/gpu/drm/arm/hdlcd_drv.c > > index e5f4f4a6546d..0f70f5fe9970 100644 > > --- a/drivers/gpu/drm/arm/hdlcd_drv.c > > +++ b/drivers/gpu/drm/arm/hdlcd_drv.c > > @@ -430,29 +430,13 @@ static int compare_dev(struct device *dev, void *data) > > > > static int hdlcd_probe(struct platform_device *pdev) > > { > > - struct device_node *port, *ep; > > + struct device_node *port; > > struct component_match *match = NULL; > > > > - if (!pdev->dev.of_node) > > - return -ENODEV; > > - > > /* there is only one output port inside each device, find it */ > > - ep = of_graph_get_next_endpoint(pdev->dev.of_node, NULL); > > - if (!ep) > > - return -ENODEV; > > - > > - if (!of_device_is_available(ep)) { > > - of_node_put(ep); > > + port = of_graph_get_remote_node(pdev->dev.of_node, 0, 0); > > + if (!port) > > return -ENODEV; > > - } > > - > > - /* add the remote encoder port as component */ > > - port = of_graph_get_remote_port_parent(ep); > > - of_node_put(ep); > > - if (!port || !of_device_is_available(port)) { > > - of_node_put(port); > > - return -EAGAIN; > > The HDLCD change looks reasonable except for this -EAGAIN business. I'll have > to > test your changes on my setup to see how this affects having the encoder as a > module. What are you expecting to happen with -EAGAIN? This one was a bit of an oddball. This condition would only change if you had an overlay. That's a use case that needs to be handled in a common way ('cause I don't want to clean-up every driver doing overlays in their own way latter). Just having "status" changing at runtime would have all sorts of implications in the kernel. > > > - } > > > > drm_of_component_match_add(>dev, , compare_dev, port); > > of_node_put(port); > > diff --git a/drivers/gpu/drm/arm/malidp_drv.c > > b/drivers/gpu/drm/arm/malidp_drv.c > > index 32f746e31379..bfa04be7f5de 100644 > > --- a/drivers/gpu/drm/arm/malidp_drv.c > > +++ b/drivers/gpu/drm/arm/malidp_drv.c > > @@ -262,7 +262,6 @@ static int malidp_bind(struct device *dev) > > { > > struct resource *res; > > struct drm_device *drm; > > - struct device_node *ep; > > struct malidp_drm *malidp; > > struct malidp_hw_device *hwdev; > > struct platform_device *pdev = to_platform_device(dev); > > @@ -360,12 +359,7 @@ static int malidp_bind(struct device *dev) > > goto init_fail; > > > > /* Set the CRTC's port so that the encoder component can find it */ > > - ep = of_graph_get_next_endpoint(dev->of_node, NULL); > > - if (!ep) { > > - ret = -EINVAL; > > - goto port_fail; > > - } > > - malidp->crtc.port =
[PATCH 04/11] drm/msm: Remove idle function hook
There isn't any generic code that uses ->idle so remove it. Signed-off-by: Jordan Crouse--- drivers/gpu/drm/msm/adreno/a3xx_gpu.c | 4 ++-- drivers/gpu/drm/msm/adreno/a4xx_gpu.c | 4 ++-- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 9 - drivers/gpu/drm/msm/adreno/a5xx_gpu.h | 1 + drivers/gpu/drm/msm/adreno/a5xx_power.c | 2 +- drivers/gpu/drm/msm/msm_gpu.h | 1 - 6 files changed, 10 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c index b999349..fc4fd2d 100644 --- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c @@ -40,6 +40,7 @@ extern bool hang_debug; static void a3xx_dump(struct msm_gpu *gpu); +static bool a3xx_idle(struct msm_gpu *gpu); static bool a3xx_me_init(struct msm_gpu *gpu) { @@ -65,7 +66,7 @@ static bool a3xx_me_init(struct msm_gpu *gpu) OUT_RING(ring, 0x); gpu->funcs->flush(gpu); - return gpu->funcs->idle(gpu); + return a3xx_idle(gpu); } static int a3xx_hw_init(struct msm_gpu *gpu) @@ -448,7 +449,6 @@ static void a3xx_dump(struct msm_gpu *gpu) .last_fence = adreno_last_fence, .submit = adreno_submit, .flush = adreno_flush, - .idle = a3xx_idle, .irq = a3xx_irq, .destroy = a3xx_destroy, #ifdef CONFIG_DEBUG_FS diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c index 511bc85..6bc948b 100644 --- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c @@ -31,6 +31,7 @@ extern bool hang_debug; static void a4xx_dump(struct msm_gpu *gpu); +static bool a4xx_idle(struct msm_gpu *gpu); /* * a4xx_enable_hwcg() - Program the clock control registers @@ -137,7 +138,7 @@ static bool a4xx_me_init(struct msm_gpu *gpu) OUT_RING(ring, 0x); gpu->funcs->flush(gpu); - return gpu->funcs->idle(gpu); + return a4xx_idle(gpu); } static int a4xx_hw_init(struct msm_gpu *gpu) @@ -538,7 +539,6 @@ static int a4xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value) .last_fence = adreno_last_fence, .submit = adreno_submit, .flush = adreno_flush, - .idle = a4xx_idle, .irq = a4xx_irq, .destroy = a4xx_destroy, #ifdef CONFIG_DEBUG_FS diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c index 23eeed2..2074f64 100644 --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c @@ -391,7 +391,7 @@ static int a5xx_me_init(struct msm_gpu *gpu) gpu->funcs->flush(gpu); - return gpu->funcs->idle(gpu) ? 0 : -EINVAL; + return a5xx_idle(gpu) ? 0 : -EINVAL; } static struct drm_gem_object *a5xx_ucode_load_bo(struct msm_gpu *gpu, @@ -699,7 +699,7 @@ static int a5xx_hw_init(struct msm_gpu *gpu) OUT_RING(gpu->rb, 0x0F); gpu->funcs->flush(gpu); - if (!gpu->funcs->idle(gpu)) + if (!a5xx_idle(gpu)) return -EINVAL; } @@ -716,7 +716,7 @@ static int a5xx_hw_init(struct msm_gpu *gpu) OUT_RING(gpu->rb, 0x); gpu->funcs->flush(gpu); - if (!gpu->funcs->idle(gpu)) + if (!a5xx_idle(gpu)) return -EINVAL; } else { /* Print a warning so if we die, we know why */ @@ -790,7 +790,7 @@ static inline bool _a5xx_check_idle(struct msm_gpu *gpu) A5XX_RBBM_INT_0_MASK_MISC_HANG_DETECT); } -static bool a5xx_idle(struct msm_gpu *gpu) +bool a5xx_idle(struct msm_gpu *gpu) { /* wait for CP to drain ringbuffer: */ if (!adreno_idle(gpu)) @@ -1091,7 +1091,6 @@ static void a5xx_show(struct msm_gpu *gpu, struct seq_file *m) .last_fence = adreno_last_fence, .submit = a5xx_submit, .flush = adreno_flush, - .idle = a5xx_idle, .irq = a5xx_irq, .destroy = a5xx_destroy, .show = a5xx_show, diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.h b/drivers/gpu/drm/msm/adreno/a5xx_gpu.h index 1590f84..6b20f28 100644 --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.h +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.h @@ -56,5 +56,6 @@ static inline int spin_usecs(struct msm_gpu *gpu, uint32_t usecs, return -ETIMEDOUT; } +bool a5xx_idle(struct msm_gpu *gpu); #endif /* __A5XX_GPU_H__ */ diff --git a/drivers/gpu/drm/msm/adreno/a5xx_power.c b/drivers/gpu/drm/msm/adreno/a5xx_power.c index 72d52c7..ed0802e 100644 --- a/drivers/gpu/drm/msm/adreno/a5xx_power.c +++ b/drivers/gpu/drm/msm/adreno/a5xx_power.c @@ -194,7 +194,7 @@ static int a5xx_gpmu_init(struct msm_gpu *gpu) gpu->funcs->flush(gpu); - if (!gpu->funcs->idle(gpu)) { +
[PATCH 00/11] drm/msm: A5XX preemption
This series of patches implements multiple ringbuffers and preemption for Adreno A5XX targets. Preemption allows a command to be interrupted at specific preemption points and execution switched to a different ringbuffer. The software alogrithm uses preemption to enforce quality of service for priority levels - commands to a certain ring preempt the rings of lower priority. Note that priority is a software construct; the driver chooses a ring to switch to and the hardware executes. This is important because it shows that preemption can be used for things other than priority (timeslices for quality of service for example). This initial series implements 4 ringbuffers to give sufficient coverage for the range of priority levels requested by the GLES and compute extensions. The targeted ringbuffer is specified in the command submission flags. The default ring is 0 (lowest priority). Jordan Jordan Crouse (11): drm/msm: Make sure to detach the MMU during GPU cleanup drm/msm: Improve the zap shader drm/msm: Add hint to DRM_IOCTL_MSM_GEM_INFO to return an object IOVA drm/msm: Remove idle function hook drm/msm: get an iova from the address space instead of an id drm/msm: Add a struct to pass configuration to msm_gpu_init() drm/msm: Remove memptrs->wptr drm/msm: Support multiple ringbuffers drm/msm: Shadow current pointer in the ring until command is complete drm/msm: Make the value of RB_CNTL (almost) generic drm/msm: Implement preemption for A5XX targets drivers/gpu/drm/msm/Makefile | 1 + drivers/gpu/drm/msm/adreno/a3xx_gpu.c | 13 +- drivers/gpu/drm/msm/adreno/a4xx_gpu.c | 13 +- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 278 +- drivers/gpu/drm/msm/adreno/a5xx_gpu.h | 106 + drivers/gpu/drm/msm/adreno/a5xx_power.c | 11 +- drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 367 ++ drivers/gpu/drm/msm/adreno/adreno_gpu.c | 215 +++-- drivers/gpu/drm/msm/adreno/adreno_gpu.h | 42 ++-- drivers/gpu/drm/msm/dsi/dsi_host.c| 15 +- drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c | 8 +- drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 18 +- drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.h | 3 - drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c | 13 +- drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 5 +- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 11 +- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 4 - drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 13 +- drivers/gpu/drm/msm/msm_drv.c | 43 ++-- drivers/gpu/drm/msm/msm_drv.h | 27 ++- drivers/gpu/drm/msm/msm_fb.c | 15 +- drivers/gpu/drm/msm/msm_fbdev.c | 10 +- drivers/gpu/drm/msm/msm_fence.c | 85 +-- drivers/gpu/drm/msm/msm_fence.h | 13 +- drivers/gpu/drm/msm/msm_gem.c | 124 +++--- drivers/gpu/drm/msm/msm_gem.h | 5 +- drivers/gpu/drm/msm/msm_gem_submit.c | 14 +- drivers/gpu/drm/msm/msm_gpu.c | 140 +++- drivers/gpu/drm/msm/msm_gpu.h | 54 - drivers/gpu/drm/msm/msm_kms.h | 3 + drivers/gpu/drm/msm/msm_ringbuffer.c | 14 +- drivers/gpu/drm/msm/msm_ringbuffer.h | 21 +- include/uapi/drm/msm_drm.h| 9 +- 33 files changed, 1324 insertions(+), 389 deletions(-) create mode 100644 drivers/gpu/drm/msm/adreno/a5xx_preempt.c -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/5] of: introduce of_graph_get_remote_node
Hi Rob, thanks for this clean-up series! I was not aware how far the duplication has spread over time. On Fri, 2017-02-03 at 21:36 -0600, Rob Herring wrote: > The OF graph API leaves too much of the graph walking to clients when > in many cases the driver doesn't care about accessing the port or > endpoint nodes. The drivers typically just want the device connected via > a particular graph connection. of_graph_get_remote_node provides this > functionality. > > Signed-off-by: Rob Herring> --- > drivers/of/base.c| 28 > include/linux/of_graph.h | 8 > 2 files changed, 36 insertions(+) > > diff --git a/drivers/of/base.c b/drivers/of/base.c > index d4bea3c797d6..ea18ab16b92c 100644 > --- a/drivers/of/base.c > +++ b/drivers/of/base.c > @@ -2469,3 +2469,31 @@ struct device_node *of_graph_get_remote_port(const > struct device_node *node) > return of_get_next_parent(np); > } > EXPORT_SYMBOL(of_graph_get_remote_port); > + > +struct device_node *of_graph_get_remote_node(const struct device_node *node, > + int port, int endpoint) I think this should have a documentation comment, similar to the of_graph_get_endpoint_by_regs one, as it is not really clear from the function name that the returned device node is the parent (or grandparent) device node containing the remote port to the specified node & port & endpoint. Also it might be interesting to the user that -1 is a wildcard value for port / endpoint. > +{ > + struct device_node *endpoint_node, *remote; > + > + endpoint_node = of_graph_get_endpoint_by_regs(node, port, endpoint); > + if (!endpoint_node) { > + pr_debug("no valid endpoint (%d, %d) for node %s\n", > + port, endpoint, node->full_name); > + return NULL; > + } > + > + remote = of_graph_get_remote_port_parent(endpoint_node); > + of_node_put(endpoint); Vladimir pointed this out already. With that fixed and the missing doc comment added, Acked-by: Philipp Zabel > + if (!remote) { > + pr_debug("no valid remote node\n"); > + return NULL; > + } > + > + if (!of_device_is_available(remote)) { > + pr_debug("not available for remote node\n"); > + return NULL; > + } > + > + return remote; > +} > +EXPORT_SYMBOL(of_graph_get_remote_node); > diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h > index bb3a5a2cd570..7b71d3e09209 100644 > --- a/include/linux/of_graph.h > +++ b/include/linux/of_graph.h > @@ -51,6 +51,8 @@ struct device_node *of_graph_get_endpoint_by_regs( > struct device_node *of_graph_get_remote_port_parent( > const struct device_node *node); > struct device_node *of_graph_get_remote_port(const struct device_node *node); > +struct device_node *of_graph_get_remote_node(const struct device_node *node, > + int port, int endpoint); > #else > > static inline int of_graph_parse_endpoint(const struct device_node *node, > @@ -89,6 +91,12 @@ static inline struct device_node *of_graph_get_remote_port( > { > return NULL; > } > +static inline struct device_node *of_graph_get_remote_node( > + const struct device_node *node, > + int port, int endpoint) > +{ > + return NULL; > +} > > #endif /* CONFIG_OF */ regards Philipp ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 07/11] drm/msm: Remove memptrs->wptr
memptrs->wptr seems to be unused. Remove it to avoid confusing the upcoming preemption code. Signed-off-by: Jordan Crouse--- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 3 --- drivers/gpu/drm/msm/adreno/adreno_gpu.h | 1 - 2 files changed, 4 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index 53f9dea..4c3e9b3 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -123,7 +123,6 @@ void adreno_recover(struct msm_gpu *gpu) /* reset completed fence seqno: */ adreno_gpu->memptrs->fence = gpu->fctx->completed_fence; adreno_gpu->memptrs->rptr = 0; - adreno_gpu->memptrs->wptr = 0; gpu->funcs->pm_resume(gpu); @@ -256,7 +255,6 @@ void adreno_show(struct msm_gpu *gpu, struct seq_file *m) seq_printf(m, "fence:%d/%d\n", adreno_gpu->memptrs->fence, gpu->fctx->last_fence); seq_printf(m, "rptr: %d\n", get_rptr(adreno_gpu)); - seq_printf(m, "wptr: %d\n", adreno_gpu->memptrs->wptr); seq_printf(m, "rb wptr: %d\n", get_wptr(gpu->rb)); gpu->funcs->pm_resume(gpu); @@ -296,7 +294,6 @@ void adreno_dump_info(struct msm_gpu *gpu) printk("fence:%d/%d\n", adreno_gpu->memptrs->fence, gpu->fctx->last_fence); printk("rptr: %d\n", get_rptr(adreno_gpu)); - printk("wptr: %d\n", adreno_gpu->memptrs->wptr); printk("rb wptr: %d\n", get_wptr(gpu->rb)); } diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h index e8d55b0..fdf4ef3 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h @@ -85,7 +85,6 @@ struct adreno_info { struct adreno_rbmemptrs { volatile uint32_t rptr; - volatile uint32_t wptr; volatile uint32_t fence; }; -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 11/11] drm/msm: Implement preemption for A5XX targets
Implement preemption for A5XX targets - this allows multiple ringbuffers for different priorities with automatic preemption of a lower priority ringbuffer if a higher one is ready. Signed-off-by: Jordan Crouse--- drivers/gpu/drm/msm/Makefile | 1 + drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 172 +- drivers/gpu/drm/msm/adreno/a5xx_gpu.h | 105 + drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 367 ++ drivers/gpu/drm/msm/adreno/adreno_gpu.c | 11 +- drivers/gpu/drm/msm/adreno/adreno_gpu.h | 5 + drivers/gpu/drm/msm/msm_drv.h | 2 +- drivers/gpu/drm/msm/msm_ringbuffer.c | 2 + drivers/gpu/drm/msm/msm_ringbuffer.h | 1 + 9 files changed, 654 insertions(+), 12 deletions(-) create mode 100644 drivers/gpu/drm/msm/adreno/a5xx_preempt.c diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile index 028c24d..8a3d74e 100644 --- a/drivers/gpu/drm/msm/Makefile +++ b/drivers/gpu/drm/msm/Makefile @@ -8,6 +8,7 @@ msm-y := \ adreno/a4xx_gpu.o \ adreno/a5xx_gpu.o \ adreno/a5xx_power.o \ + adreno/a5xx_preempt.o \ hdmi/hdmi.o \ hdmi/hdmi_audio.o \ hdmi/hdmi_bridge.o \ diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c index 5f02ff3..b7c6158 100644 --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c @@ -184,14 +184,66 @@ static int zap_load_mdt(struct platform_device *pdev) return ret; } +static void a5xx_flush(struct msm_gpu *gpu, struct msm_ringbuffer *ring) +{ + struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); + struct a5xx_gpu *a5xx_gpu = to_a5xx_gpu(adreno_gpu); + uint32_t wptr; + unsigned long flags; + + spin_lock_irqsave(>lock, flags); + + /* Copy the shadow to the actual register */ + ring->cur = ring->next; + + /* Make sure to wrap wptr if we need to */ + wptr = (ring->cur - ring->start) % (MSM_GPU_RINGBUFFER_SZ >> 2); + + spin_unlock_irqrestore(>lock, flags); + + /* Make sure everything is posted before making a decision */ + mb(); + + /* Update HW if this is the current ring and we are not in preempt */ + if (a5xx_gpu->cur_ring == ring && !a5xx_in_preempt(a5xx_gpu)) + gpu_write(gpu, REG_A5XX_CP_RB_WPTR, wptr); +} + static void a5xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit, struct msm_file_private *ctx) { struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); + struct a5xx_gpu *a5xx_gpu = to_a5xx_gpu(adreno_gpu); + struct msm_drm_private *priv = gpu->dev->dev_private; struct msm_ringbuffer *ring = submit->ring; unsigned int i, ibs = 0; + OUT_PKT7(ring, CP_PREEMPT_ENABLE_GLOBAL, 1); + OUT_RING(ring, 0x02); + + /* Turn off protected mode to write to special registers */ + OUT_PKT7(ring, CP_SET_PROTECTED_MODE, 1); + OUT_RING(ring, 0); + + /* Set the save preemption record for the ring/command */ + OUT_PKT4(ring, REG_A5XX_CP_CONTEXT_SWITCH_SAVE_ADDR_LO, 2); + OUT_RING(ring, lower_32_bits(a5xx_gpu->preempt_iova[submit->ring->id])); + OUT_RING(ring, upper_32_bits(a5xx_gpu->preempt_iova[submit->ring->id])); + + /* Turn back on protected mode */ + OUT_PKT7(ring, CP_SET_PROTECTED_MODE, 1); + OUT_RING(ring, 1); + + /* Enable local preemption for finegrain preemption */ + OUT_PKT7(ring, CP_PREEMPT_ENABLE_GLOBAL, 1); + OUT_RING(ring, 0x02); + + /* Allow CP_CONTEXT_SWITCH_YIELD packets in the IB2 */ + OUT_PKT7(ring, CP_YIELD_ENABLE, 1); + OUT_RING(ring, 0x02); + + /* Submit the commands */ for (i = 0; i < submit->nr_cmds; i++) { switch (submit->cmd[i].type) { case MSM_SUBMIT_CMD_IB_TARGET_BUF: @@ -209,16 +261,54 @@ static void a5xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit, } } + /* +* Write the render mode to NULL (0) to indicate to the CP that the IBs +* are done rendering - otherwise a lucky preemption would start +* replaying from the last checkpoint +*/ + OUT_PKT7(ring, CP_SET_RENDER_MODE, 5); + OUT_RING(ring, 0); + OUT_RING(ring, 0); + OUT_RING(ring, 0); + OUT_RING(ring, 0); + OUT_RING(ring, 0); + + /* Turn off IB level preemptions */ + OUT_PKT7(ring, CP_YIELD_ENABLE, 1); + OUT_RING(ring, 0x01); + + /* Write the fence to the scratch register */ OUT_PKT4(ring, REG_A5XX_CP_SCRATCH_REG(2), 1); OUT_RING(ring, submit->fence->seqno); + /* +* Execute a CACHE_FLUSH_TS event. This will ensure that the +* timestamp is written to the memory and then triggers the interrupt +*/ OUT_PKT7(ring, CP_EVENT_WRITE, 4); OUT_RING(ring,
[PATCH 10/11] drm/msm: Make the value of RB_CNTL (almost) generic
We use a global ringbuffer size and block size for all targets and at least for 5XX preemption we need to know the value the RB_CNTL in several locations so it makes sense to caculate it once and use it everywhere. The only monkey wrench is that we need to disable the RPTR shadow for A430 targets but that only needs to be done once and doesn't affect A5XX so we can or in the value at init time. Signed-off-by: Jordan Crouse--- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 12 +++- drivers/gpu/drm/msm/msm_gpu.h | 5 + 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index 44a95ea..aca1fc3 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -21,7 +21,6 @@ #include "msm_gem.h" #include "msm_mmu.h" -#define RB_BLKSIZE 32 int adreno_get_param(struct msm_gpu *gpu, uint32_t param, uint64_t *value) { @@ -71,11 +70,14 @@ int adreno_hw_init(struct msm_gpu *gpu) } } - /* Setup REG_CP_RB_CNTL: */ + /* +* Setup REG_CP_RB_CNTL. The same value is used across targets (with +* the excpetion of A430 that disables the RPTR shadow) - the cacluation +* for the ringbuffer size and block size is moved to msm_gpu.h for the +* pre-processor to deal with and the A430 variant is ORed in here +*/ adreno_gpu_write(adreno_gpu, REG_ADRENO_CP_RB_CNTL, - /* size is log2(quad-words): */ - AXXX_CP_RB_CNTL_BUFSZ(ilog2(MSM_GPU_RINGBUFFER_SZ / 8)) | - AXXX_CP_RB_CNTL_BLKSZ(ilog2(RB_BLKSIZE / 8)) | + MSM_GPU_RB_CNTL_DEFAULT | (adreno_is_a430(adreno_gpu) ? AXXX_CP_RB_CNTL_NO_UPDATE : 0)); /* Setup ringbuffer address - use ringbuffer[0] for GPU init */ diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h index 38d826a..50fef27 100644 --- a/drivers/gpu/drm/msm/msm_gpu.h +++ b/drivers/gpu/drm/msm/msm_gpu.h @@ -135,6 +135,11 @@ struct msm_gpu { /* It turns out that all targets use the same ringbuffer size */ #define MSM_GPU_RINGBUFFER_SZ SZ_32K +#define MSM_GPU_RINGBUFFER_BLKSIZE 32 + +#define MSM_GPU_RB_CNTL_DEFAULT \ + (AXXX_CP_RB_CNTL_BUFSZ(ilog2(MSM_GPU_RINGBUFFER_SZ / 8)) | \ + AXXX_CP_RB_CNTL_BLKSZ(ilog2(MSM_GPU_RINGBUFFER_BLKSIZE / 8))) static inline struct msm_ringbuffer *__get_ring(struct msm_gpu *gpu, int index) { -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 05/11] drm/msm: get an iova from the address space instead of an id
In the future we won't have a fixed set of addresses spaces. Instead of going through the effort of assigning a ID for each address space just use the address space itself as a token for getting / putting an iova. This forces a few changes in the gem object however: instead of using a simple index into a list of domains, we need to maintain a list of them. Luckily the list will be pretty small; even with dynamic address spaces we wouldn't ever see more than two or three. Signed-off-by: Jordan Crouse--- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 8 +- drivers/gpu/drm/msm/adreno/a5xx_power.c | 5 +- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 6 +- drivers/gpu/drm/msm/dsi/dsi_host.c| 15 +++- drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c | 8 +- drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 18 ++--- drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.h | 3 - drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c | 13 ++-- drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 5 +- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 11 +-- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 4 - drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 13 ++-- drivers/gpu/drm/msm/msm_drv.c | 14 drivers/gpu/drm/msm/msm_drv.h | 25 +++--- drivers/gpu/drm/msm/msm_fb.c | 15 ++-- drivers/gpu/drm/msm/msm_fbdev.c | 10 ++- drivers/gpu/drm/msm/msm_gem.c | 124 +- drivers/gpu/drm/msm/msm_gem.h | 4 +- drivers/gpu/drm/msm/msm_gem_submit.c | 4 +- drivers/gpu/drm/msm/msm_gpu.c | 8 +- drivers/gpu/drm/msm/msm_gpu.h | 1 - drivers/gpu/drm/msm/msm_kms.h | 3 + 22 files changed, 184 insertions(+), 133 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c index 2074f64..546280c 100644 --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c @@ -415,7 +415,7 @@ static struct drm_gem_object *a5xx_ucode_load_bo(struct msm_gpu *gpu, } if (iova) { - int ret = msm_gem_get_iova(bo, gpu->id, iova); + int ret = msm_gem_get_iova(bo, gpu->aspace, iova); if (ret) { drm_gem_object_unreference_unlocked(bo); @@ -757,19 +757,19 @@ static void a5xx_destroy(struct msm_gpu *gpu) if (a5xx_gpu->pm4_bo) { if (a5xx_gpu->pm4_iova) - msm_gem_put_iova(a5xx_gpu->pm4_bo, gpu->id); + msm_gem_put_iova(a5xx_gpu->pm4_bo, gpu->aspace); drm_gem_object_unreference_unlocked(a5xx_gpu->pm4_bo); } if (a5xx_gpu->pfp_bo) { if (a5xx_gpu->pfp_iova) - msm_gem_put_iova(a5xx_gpu->pfp_bo, gpu->id); + msm_gem_put_iova(a5xx_gpu->pfp_bo, gpu->aspace); drm_gem_object_unreference_unlocked(a5xx_gpu->pfp_bo); } if (a5xx_gpu->gpmu_bo) { if (a5xx_gpu->gpmu_bo) - msm_gem_put_iova(a5xx_gpu->gpmu_bo, gpu->id); + msm_gem_put_iova(a5xx_gpu->gpmu_bo, gpu->aspace); drm_gem_object_unreference_unlocked(a5xx_gpu->gpmu_bo); } diff --git a/drivers/gpu/drm/msm/adreno/a5xx_power.c b/drivers/gpu/drm/msm/adreno/a5xx_power.c index ed0802e..2fdee44 100644 --- a/drivers/gpu/drm/msm/adreno/a5xx_power.c +++ b/drivers/gpu/drm/msm/adreno/a5xx_power.c @@ -301,7 +301,8 @@ void a5xx_gpmu_ucode_init(struct msm_gpu *gpu) if (IS_ERR(a5xx_gpu->gpmu_bo)) goto err; - if (msm_gem_get_iova(a5xx_gpu->gpmu_bo, gpu->id, _gpu->gpmu_iova)) + if (msm_gem_get_iova(a5xx_gpu->gpmu_bo, gpu->aspace, + _gpu->gpmu_iova)) goto err; ptr = msm_gem_get_vaddr(a5xx_gpu->gpmu_bo); @@ -330,7 +331,7 @@ void a5xx_gpmu_ucode_init(struct msm_gpu *gpu) err: if (a5xx_gpu->gpmu_iova) - msm_gem_put_iova(a5xx_gpu->gpmu_bo, gpu->id); + msm_gem_put_iova(a5xx_gpu->gpmu_bo, gpu->aspace); if (a5xx_gpu->gpmu_bo) drm_gem_object_unreference_unlocked(a5xx_gpu->gpmu_bo); diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index acb685a..247f017 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -61,7 +61,7 @@ int adreno_hw_init(struct msm_gpu *gpu) DBG("%s", gpu->name); - ret = msm_gem_get_iova(gpu->rb->bo, gpu->id, >rb_iova); + ret = msm_gem_get_iova(gpu->rb->bo, gpu->aspace, >rb_iova); if (ret) { gpu->rb_iova = 0; dev_err(gpu->dev->dev, "could not map ringbuffer: %d\n", ret); @@ -411,7 +411,7 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, return -ENOMEM; } - ret =
Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node
On 02/04/2017 04:36 AM, Rob Herring wrote: > Convert drivers to use the new of_graph_get_remote_node() helper > instead of parsing the endpoint node and then getting the remote device > node. Now drivers can just specify the device node and which > port/endpoint and get back the connected remote device node. The details > of the graph binding are nicely abstracted into the core OF graph code. > > This changes some error messages to debug messages (in the graph core). > Graph connections are often "no connects" depending on the particular > board, so we want to avoid spurious messages. Plus the kernel is not a > DT validator. > > Signed-off-by: Rob Herring> --- > drivers/gpu/drm/arm/hdlcd_drv.c | 22 ++--- > drivers/gpu/drm/arm/malidp_drv.c| 29 ++- > drivers/gpu/drm/bridge/adv7511/adv7533.c| 12 + > drivers/gpu/drm/bridge/dumb-vga-dac.c | 15 ++ > drivers/gpu/drm/bridge/ti-tfp410.c | 15 ++ > drivers/gpu/drm/exynos/exynos_drm_dpi.c | 16 +- > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 13 ++--- > drivers/gpu/drm/exynos/exynos_drm_mic.c | 27 +- > drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c| 26 ++ > drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 30 +-- > drivers/gpu/drm/mediatek/mtk_dpi.c | 12 ++--- > drivers/gpu/drm/mediatek/mtk_hdmi.c | 26 ++ > drivers/gpu/drm/meson/meson_drv.c | 12 ++--- > drivers/gpu/drm/meson/meson_venc_cvbs.c | 19 ++- > drivers/gpu/drm/msm/dsi/dsi_host.c | 3 +- > drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 28 +-- > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 18 +++ > drivers/gpu/drm/tilcdc/tilcdc_crtc.c| 11 + > drivers/gpu/drm/tilcdc/tilcdc_external.c| 66 > +++-- > drivers/gpu/drm/vc4/vc4_dpi.c | 15 ++ > 20 files changed, 64 insertions(+), 351 deletions(-) > [...] > diff --git a/drivers/gpu/drm/meson/meson_drv.c > b/drivers/gpu/drm/meson/meson_drv.c > index ff1f6019b97b..37cb9c755ed7 100644 > --- a/drivers/gpu/drm/meson/meson_drv.c > +++ b/drivers/gpu/drm/meson/meson_drv.c > @@ -163,14 +163,14 @@ static struct drm_driver meson_driver = { > > static bool meson_vpu_has_available_connectors(struct device *dev) > { > - struct device_node *ep, *remote; > + struct device_node *remote; > + int i; > > - /* Parses each endpoint and check if remote exists */ > - for_each_endpoint_of_node(dev->of_node, ep) { > - /* If the endpoint node exists, consider it enabled */ > - remote = of_graph_get_remote_port(ep); > - if (remote) > + for_each_of_graph_remote_node(dev->of_node, remote, i, 2) { > + if (remote) { > + of_node_put(remote); > return true; > + } > } > > return false; > diff --git a/drivers/gpu/drm/meson/meson_venc_cvbs.c > b/drivers/gpu/drm/meson/meson_venc_cvbs.c > index a2bcc70a03ef..8566de2edb62 100644 > --- a/drivers/gpu/drm/meson/meson_venc_cvbs.c > +++ b/drivers/gpu/drm/meson/meson_venc_cvbs.c > @@ -217,25 +217,14 @@ static const struct drm_encoder_helper_funcs > > static bool meson_venc_cvbs_connector_is_available(struct meson_drm *priv) > { > - struct device_node *ep, *remote; > + struct device_node *remote; > > - /* CVBS VDAC output is on the first port, first endpoint */ > - ep = of_graph_get_endpoint_by_regs(priv->dev->of_node, 0, 0); > - if (!ep) > + remote = of_graph_get_remote_node(priv->dev->of_node, 0, 0); > + if (!remote) > return false; > > - > - /* If the endpoint node exists, consider it enabled */ > - remote = of_graph_get_remote_port(ep); > - if (remote) { > - of_node_put(ep); > - return true; > - } > - > - of_node_put(ep); > of_node_put(remote); > - > - return false; > + return true; > } > > int meson_venc_cvbs_create(struct meson_drm *priv) [...] Hi Rob, For the meson changes : Acked-by: Neil Armstrong Thanks ! Neil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node
Thanks Rob, for nice cleanup, but ... On 02/04/17 05:36, Rob Herring wrote: > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > index 6dfdb145f3bb..e74cc236a79b 100644 > --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > @@ -1013,16 +1013,7 @@ int tilcdc_crtc_create(struct drm_device *dev) > drm_crtc_helper_add(crtc, _crtc_helper_funcs); > > if (priv->is_componentized) { > - struct device_node *ports = > - of_get_child_by_name(dev->dev->of_node, "ports"); > - > - if (ports) { > - crtc->port = of_get_child_by_name(ports, "port"); > - of_node_put(ports); > - } else { > - crtc->port = > - of_get_child_by_name(dev->dev->of_node, "port"); > - } > + crtc->port = of_graph_get_port_by_id(dev->dev->of_node, 0, 0); > if (!crtc->port) { /* This should never happen */ > dev_err(dev->dev, "Port node not found in %s\n", > dev->dev->of_node->full_name); > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.c > b/drivers/gpu/drm/tilcdc/tilcdc_external.c > index c67d7cd7d57e..b7523dce4e8a 100644 > --- a/drivers/gpu/drm/tilcdc/tilcdc_external.c > +++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c > @@ -187,39 +187,6 @@ int tilcdc_attach_bridge(struct drm_device *ddev, struct > drm_bridge *bridge) > return ret; > } > > -static int tilcdc_node_has_port(struct device_node *dev_node) > -{ > - struct device_node *node; > - > - node = of_get_child_by_name(dev_node, "ports"); > - if (!node) > - node = of_get_child_by_name(dev_node, "port"); > - if (!node) > - return 0; > - of_node_put(node); > - > - return 1; > -} > - > -static > -struct device_node *tilcdc_get_remote_node(struct device_node *node) > -{ > - struct device_node *ep; > - struct device_node *parent; > - > - if (!tilcdc_node_has_port(node)) > - return NULL; > - > - ep = of_graph_get_next_endpoint(node, NULL); > - if (!ep) > - return NULL; > - > - parent = of_graph_get_remote_port_parent(ep); > - of_node_put(ep); > - > - return parent; > -} > - > int tilcdc_attach_external_device(struct drm_device *ddev) > { > struct tilcdc_drm_private *priv = ddev->dev_private; > @@ -227,7 +194,7 @@ int tilcdc_attach_external_device(struct drm_device *ddev) > struct drm_bridge *bridge; > int ret; > > - remote_node = tilcdc_get_remote_node(ddev->dev->of_node); > + remote_node = of_graph_get_remote_node(ddev->dev->of_node, 0, 0); > if (!remote_node) > return 0; > > @@ -266,35 +233,18 @@ int tilcdc_get_external_components(struct device *dev, > struct component_match **match) > { > struct device_node *node; > - struct device_node *ep = NULL; > - int count = 0; > - int ret = 0; > > - if (!tilcdc_node_has_port(dev->of_node)) > + if (!match) > return 0; This will break tilcdc on setups that use the old tilcdc internal panel- or tfp410- support. The driver uses tilcdc_get_external_components() with match == NULL to check if the setup uses graph binding or legacy internal panel/tfp410 support. My intention is to get rid off the legacy internal encoder and connector support (I even did that once already, but DRM had lot of infrastructure changes in that area and my changes became obsolete), but we still need it. > > - while ((ep = of_graph_get_next_endpoint(dev->of_node, ep))) { > - node = of_graph_get_remote_port_parent(ep); > - if (!node || !of_device_is_available(node)) { > - of_node_put(node); > - continue; > - } > - > - dev_dbg(dev, "Subdevice node '%s' found\n", node->name); > - > - if (of_device_is_compatible(node, "nxp,tda998x")) { > - if (match) > - drm_of_component_match_add(dev, match, > -dev_match_of, node); > - ret = 1; > - } > + node = of_graph_get_remote_node(dev->of_node, 0, 0); > > + if (!of_device_is_compatible(node, "nxp,tda998x")) { > of_node_put(node); > - if (count++ > 1) { > - dev_err(dev, "Only one port is supported\n"); > - return -EINVAL; > - } > + return 0; > } > > - return ret; Simply remove the above mentioned if statement add and this here: if (match) > + drm_of_component_match_add(dev, match, dev_match_of, node); > + of_node_put(node); > + return 1; > } Best regards, Jyri ___ dri-devel mailing
Re: [PATCH 1/5] of: introduce of_graph_get_remote_node
On Mon, 2017-02-06 at 07:54 -0600, Rob Herring wrote: > On Mon, Feb 6, 2017 at 4:32 AM, Philipp Zabelwrote: > > Hi Rob, > > > > thanks for this clean-up series! I was not aware how far the duplication > > has spread over time. > > > > On Fri, 2017-02-03 at 21:36 -0600, Rob Herring wrote: > >> The OF graph API leaves too much of the graph walking to clients when > >> in many cases the driver doesn't care about accessing the port or > >> endpoint nodes. The drivers typically just want the device connected via > >> a particular graph connection. of_graph_get_remote_node provides this > >> functionality. > >> > >> Signed-off-by: Rob Herring > >> --- > >> drivers/of/base.c| 28 > >> include/linux/of_graph.h | 8 > >> 2 files changed, 36 insertions(+) > >> > >> diff --git a/drivers/of/base.c b/drivers/of/base.c > >> index d4bea3c797d6..ea18ab16b92c 100644 > >> --- a/drivers/of/base.c > >> +++ b/drivers/of/base.c > >> @@ -2469,3 +2469,31 @@ struct device_node *of_graph_get_remote_port(const > >> struct device_node *node) > >> return of_get_next_parent(np); > >> } > >> EXPORT_SYMBOL(of_graph_get_remote_port); > >> + > >> +struct device_node *of_graph_get_remote_node(const struct device_node > >> *node, > >> + int port, int endpoint) > > > > I think this should have a documentation comment, similar to the > > of_graph_get_endpoint_by_regs one, as it is not really clear from the > > function name that the returned device node is the parent (or > > grandparent) device node containing the remote port to the specified > > node & port & endpoint. > > Also it might be interesting to the user that -1 is a wildcard value for > > port / endpoint. > > I really want to not allow using a wildcard here. Drivers should know > what port they want (or iterate over all of them). It didn't look like > any drivers were depending on the wildcard, but were just using -1 for > "no reg property" when really that should 0. Of course, I may have > missed something. > > I guess I could enforce port/endpoint > 0 here as there's no existing users. That sounds reasonable. If it works for all users, enforcing >= 0 should be fine, but in that case I'd change the parameters to be unsigned. regards Philipp ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 09/11] drm/msm: Shadow current pointer in the ring until command is complete
Add a shadow pointer to track the current command being written into the ring. Don't commit it as 'cur' until the command is submitted. Because 'cur' is used to construct the software copy of the wptr this ensures that somebody peeking in on the ring doesn't assume that a command is inflight while it is being written. This isn't a huge deal with a single ring (though technically the hangcheck could assume the system is prematurely busy when it isn't) but it will be rather important for preemption where the decision to preempt is based on a non-empty ringbuffer. Without a shadow an aggressive preemption scheme could assume that the ringbuffer is non empty and switch to it before the CPU is done writing the command and boom. Even though preemption won't be supported for all targets because of the way the code is organized it is simpler to make this generic for all targets. The extra load for non-preemption targets should be minimal. Signed-off-by: Jordan Crouse--- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 9 +++-- drivers/gpu/drm/msm/msm_ringbuffer.c| 1 + drivers/gpu/drm/msm/msm_ringbuffer.h| 12 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index f5c2bad..44a95ea 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -144,6 +144,7 @@ void adreno_recover(struct msm_gpu *gpu) continue; ring->cur = ring->start; + ring->next = ring->start; /* reset completed fence seqno, discard anything pending: */ adreno_gpu->memptrs->fence[ring->id] = ring->completed_fence; @@ -240,12 +241,15 @@ void adreno_flush(struct msm_gpu *gpu, struct msm_ringbuffer *ring) struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); uint32_t wptr; + /* Copy the shadow to the actual register */ + ring->cur = ring->next; + /* * Mask wptr value that we calculate to fit in the HW range. This is * to account for the possibility that the last command fit exactly into * the ringbuffer and rb->next hasn't wrapped to zero yet */ - wptr = get_wptr(ring) % (MSM_GPU_RINGBUFFER_SZ >> 2); + wptr = (ring->cur - ring->start) % (MSM_GPU_RINGBUFFER_SZ >> 2); /* ensure writes to ringbuffer have hit system memory: */ mb(); @@ -366,7 +370,8 @@ static uint32_t ring_freewords(struct msm_ringbuffer *ring) { struct adreno_gpu *adreno_gpu = to_adreno_gpu(ring->gpu); uint32_t size = MSM_GPU_RINGBUFFER_SZ >> 2; - uint32_t wptr = get_wptr(ring); + /* Use ring->next to calculate free size */ + uint32_t wptr = ring->next - ring->start; uint32_t rptr = get_rptr(adreno_gpu, ring); return (rptr + (size - 1) - wptr) % size; } diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.c b/drivers/gpu/drm/msm/msm_ringbuffer.c index 2ab31c7..b885979 100644 --- a/drivers/gpu/drm/msm/msm_ringbuffer.c +++ b/drivers/gpu/drm/msm/msm_ringbuffer.c @@ -47,6 +47,7 @@ struct msm_ringbuffer *msm_ringbuffer_new(struct msm_gpu *gpu, int id) goto fail; } ring->end = ring->start + (MSM_GPU_RINGBUFFER_SZ >> 2); + ring->next = ring->start; ring->cur = ring->start; return ring; diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.h b/drivers/gpu/drm/msm/msm_ringbuffer.h index 4eb05fe..865b21a 100644 --- a/drivers/gpu/drm/msm/msm_ringbuffer.h +++ b/drivers/gpu/drm/msm/msm_ringbuffer.h @@ -24,7 +24,7 @@ struct msm_ringbuffer { struct msm_gpu *gpu; int id; struct drm_gem_object *bo; - uint32_t *start, *end, *cur; + uint32_t *start, *end, *cur, *next; uint64_t iova; /* last_fence == completed_fence --> no pending work */ uint32_t last_fence; @@ -39,9 +39,13 @@ struct msm_ringbuffer { static inline void OUT_RING(struct msm_ringbuffer *ring, uint32_t data) { - if (ring->cur == ring->end) - ring->cur = ring->start; - *(ring->cur++) = data; + /* +* ring->next points to the current command being written - it won't be +* committed as ring->cur until the flush +*/ + if (ring->next == ring->end) + ring->next = ring->start; + *(ring->next++) = data; } #endif /* __MSM_RINGBUFFER_H__ */ -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] drm: convert drivers to use drm_of_find_panel_or_bridge
On Fri, 2017-02-03 at 21:36 -0600, Rob Herring wrote: > Similar to the previous commit, convert drivers open coding OF graph > parsing to use drm_of_find_panel_or_bridge instead. > > This changes some error messages to debug messages (in the graph core). > Graph connections are often "no connects" depending on the particular > board, so we want to avoid spurious messages. Plus the kernel is not a > DT validator. > > Signed-off-by: Rob Herring> --- > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 64 - > drivers/gpu/drm/bridge/nxp-ptn3460.c | 16 ++--- > drivers/gpu/drm/bridge/parade-ps8622.c | 16 ++--- > drivers/gpu/drm/bridge/tc358767.c| 27 +-- > drivers/gpu/drm/exynos/exynos_dp.c | 35 - > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c| 49 - > drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 5 +- > drivers/gpu/drm/imx/imx-ldb.c| 28 ++-- > drivers/gpu/drm/imx/parallel-display.c | 35 ++--- > drivers/gpu/drm/mediatek/mtk_dsi.c | 23 ++ > drivers/gpu/drm/mxsfb/mxsfb_out.c| 36 ++ > drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 26 ++- > drivers/gpu/drm/sun4i/sun4i_rgb.c| 17 ++--- > drivers/gpu/drm/sun4i/sun4i_tcon.c | 90 > ++-- > 14 files changed, 88 insertions(+), 379 deletions(-) > [...] > diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c > index 516d06490465..e670993906b8 100644 > --- a/drivers/gpu/drm/imx/imx-ldb.c > +++ b/drivers/gpu/drm/imx/imx-ldb.c > @@ -649,7 +649,7 @@ static int imx_ldb_bind(struct device *dev, struct device > *master, void *data) > > for_each_child_of_node(np, child) { > struct imx_ldb_channel *channel; > - struct device_node *ep; > + struct device_node *remote; > int bus_format; > > ret = of_property_read_u32(child, "reg", ); > @@ -673,27 +673,11 @@ static int imx_ldb_bind(struct device *dev, struct > device *master, void *data) >* The output port is port@4 with an external 4-port mux or >* port@2 with the internal 2-port mux. >*/ > - ep = of_graph_get_endpoint_by_regs(child, > -imx_ldb->lvds_mux ? 4 : 2, > --1); > - if (ep) { > - struct device_node *remote; > - > - remote = of_graph_get_remote_port_parent(ep); > - of_node_put(ep); > - if (remote) { > - channel->panel = of_drm_find_panel(remote); > - channel->bridge = of_drm_find_bridge(remote); > - } else > - return -EPROBE_DEFER; > - of_node_put(remote); > - > - if (!channel->panel && !channel->bridge) { > - dev_err(dev, "panel/bridge not found: %s\n", > - remote->full_name); > - return -EPROBE_DEFER; > - } > - } > + ret = drm_of_find_panel_or_bridge(fsl_dev->np, s/fsl_dev->np/child/ > + imx_ldb->lvds_mux ? 4 : 2, 0, Same comment as for the mediatek changes, this now requires that the endpoints have no reg property set or reg = <0>. This is fine for LVDS links, and I am not aware of any device trees that set the reg propertiy in their LVDS output endpoints, so that should be fine. > + >panel, > >bridge); > + if (ret) > + return ret; > > /* panel ddc only if there is no bridge */ > if (!channel->bridge) { > diff --git a/drivers/gpu/drm/imx/parallel-display.c > b/drivers/gpu/drm/imx/parallel-display.c > index 8582a83c0d9b..eb3a0201853a 100644 > --- a/drivers/gpu/drm/imx/parallel-display.c > +++ b/drivers/gpu/drm/imx/parallel-display.c > @@ -210,7 +210,7 @@ static int imx_pd_bind(struct device *dev, struct device > *master, void *data) > { > struct drm_device *drm = data; > struct device_node *np = dev->of_node; > - struct device_node *ep; > + struct device_node *remote; > const u8 *edidp; > struct imx_parallel_display *imxpd; > int ret; > @@ -239,36 +239,9 @@ static int imx_pd_bind(struct device *dev, struct device > *master, void *data) > imxpd->bus_format = bus_format; > > /* port@1 is the output port */ > - ep = of_graph_get_endpoint_by_regs(np, 1, -1); > - if (ep) { > - struct device_node *remote; > - > - remote = of_graph_get_remote_port_parent(ep); > - if (!remote) { > -
Re: [PATCH 4/5] drm: convert drivers to use drm_of_find_panel_or_bridge
Hi Rob, On Fri, Feb 03, 2017 at 09:36:34PM -0600, Rob Herring wrote: > Similar to the previous commit, convert drivers open coding OF graph > parsing to use drm_of_find_panel_or_bridge instead. > > This changes some error messages to debug messages (in the graph core). > Graph connections are often "no connects" depending on the particular > board, so we want to avoid spurious messages. Plus the kernel is not a > DT validator. > > Signed-off-by: Rob Herring> --- [..] > diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c > b/drivers/gpu/drm/sun4i/sun4i_rgb.c > index f5e86fe7750e..4720725b0fb0 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_rgb.c > +++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c > @@ -15,6 +15,7 @@ > #include > #include > #include > +#include > #include > > #include "sun4i_drv.h" > @@ -217,12 +218,10 @@ int sun4i_rgb_init(struct drm_device *drm) > rgb->drv = drv; > encoder = >encoder; > > - tcon->panel = sun4i_tcon_find_panel(tcon->dev->of_node); > - encoder->bridge = sun4i_tcon_find_bridge(tcon->dev->of_node); > - if (IS_ERR(tcon->panel) && IS_ERR(encoder->bridge)) { > - dev_info(drm->dev, "No panel or bridge found... RGB output > disabled\n"); > - return 0; > - } > + ret = drm_of_find_panel_or_bridge(tcon->dev->of_node, 1, 0, > + >panel, >bridge); > + if (ret) > + return ret; It used to ignore the error if it couldn't find the bridge. This will break the probe. > > drm_encoder_helper_add(>encoder, > _rgb_enc_helper_funcs); > @@ -239,7 +238,7 @@ int sun4i_rgb_init(struct drm_device *drm) > /* The RGB encoder can only work with the TCON channel 0 */ > rgb->encoder.possible_crtcs = BIT(0); > > - if (!IS_ERR(tcon->panel)) { > + if (tcon->panel) { > drm_connector_helper_add(>connector, >_rgb_con_helper_funcs); > ret = drm_connector_init(drm, >connector, > @@ -260,7 +259,7 @@ int sun4i_rgb_init(struct drm_device *drm) > } > } > > - if (!IS_ERR(encoder->bridge)) { > + if (encoder->bridge) { > encoder->bridge->encoder = >encoder; > > ret = drm_bridge_attach(drm, encoder->bridge); > @@ -268,8 +267,6 @@ int sun4i_rgb_init(struct drm_device *drm) > dev_err(drm->dev, "Couldn't attach our bridge\n"); > goto err_cleanup_connector; > } > - } else { > - encoder->bridge = NULL; > } > > return 0; > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c > b/drivers/gpu/drm/sun4i/sun4i_tcon.c > index ea2906f87cb9..2e4e365cecf9 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > @@ -15,13 +15,12 @@ > #include > #include > #include > -#include > +#include > > #include > #include > #include > #include > -#include > #include > #include > #include > @@ -405,74 +404,6 @@ static int sun4i_tcon_init_regmap(struct device *dev, > return 0; > } > > -struct drm_panel *sun4i_tcon_find_panel(struct device_node *node) > -{ > - struct device_node *port, *remote, *child; > - struct device_node *end_node = NULL; > - > - /* Inputs are listed first, then outputs */ > - port = of_graph_get_port_by_id(node, 1); > - > - /* > - * Our first output is the RGB interface where the panel will > - * be connected. > - */ > - for_each_child_of_node(port, child) { > - u32 reg; > - > - of_property_read_u32(child, "reg", ); > - if (reg == 0) > - end_node = child; > - } > - > - if (!end_node) { > - DRM_DEBUG_DRIVER("Missing panel endpoint\n"); > - return ERR_PTR(-ENODEV); > - } > - > - remote = of_graph_get_remote_port_parent(end_node); > - if (!remote) { > - DRM_DEBUG_DRIVER("Unable to parse remote node\n"); > - return ERR_PTR(-EINVAL); > - } > - > - return of_drm_find_panel(remote) ?: ERR_PTR(-EPROBE_DEFER); And the panel is only one of our endpoints, which is optional, while other endpoints are mandatory. This means that we might very well have an endpoint that is not a panel or a bridge. In this case, I think your function will return an error and will be treated as such, while it's really the expected behaviour. I think it's better to leave this driver alone for now, it's not as trivial as it looks, and will require some testing to get things right. I'll try to get my head around how to use your new (very welcome) helpers. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org
Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node
On Fri, Feb 03, 2017 at 09:36:33PM -0600, Rob Herring wrote: > Convert drivers to use the new of_graph_get_remote_node() helper > instead of parsing the endpoint node and then getting the remote device > node. Now drivers can just specify the device node and which > port/endpoint and get back the connected remote device node. The details > of the graph binding are nicely abstracted into the core OF graph code. > > This changes some error messages to debug messages (in the graph core). > Graph connections are often "no connects" depending on the particular > board, so we want to avoid spurious messages. Plus the kernel is not a > DT validator. > > Signed-off-by: Rob Herring> --- > drivers/gpu/drm/arm/hdlcd_drv.c | 22 ++--- > drivers/gpu/drm/arm/malidp_drv.c| 29 ++- > drivers/gpu/drm/bridge/adv7511/adv7533.c| 12 + > drivers/gpu/drm/bridge/dumb-vga-dac.c | 15 ++ > drivers/gpu/drm/bridge/ti-tfp410.c | 15 ++ > drivers/gpu/drm/exynos/exynos_drm_dpi.c | 16 +- > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 13 ++--- > drivers/gpu/drm/exynos/exynos_drm_mic.c | 27 +- > drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c| 26 ++ > drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 30 +-- > drivers/gpu/drm/mediatek/mtk_dpi.c | 12 ++--- > drivers/gpu/drm/mediatek/mtk_hdmi.c | 26 ++ > drivers/gpu/drm/meson/meson_drv.c | 12 ++--- > drivers/gpu/drm/meson/meson_venc_cvbs.c | 19 ++- > drivers/gpu/drm/msm/dsi/dsi_host.c | 3 +- > drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 28 +-- > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 18 +++ > drivers/gpu/drm/tilcdc/tilcdc_crtc.c| 11 + > drivers/gpu/drm/tilcdc/tilcdc_external.c| 66 > +++-- > drivers/gpu/drm/vc4/vc4_dpi.c | 15 ++ > 20 files changed, 64 insertions(+), 351 deletions(-) > > diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c > index e5f4f4a6546d..0f70f5fe9970 100644 > --- a/drivers/gpu/drm/arm/hdlcd_drv.c > +++ b/drivers/gpu/drm/arm/hdlcd_drv.c > @@ -430,29 +430,13 @@ static int compare_dev(struct device *dev, void *data) > > static int hdlcd_probe(struct platform_device *pdev) > { > - struct device_node *port, *ep; > + struct device_node *port; > struct component_match *match = NULL; > > - if (!pdev->dev.of_node) > - return -ENODEV; > - > /* there is only one output port inside each device, find it */ > - ep = of_graph_get_next_endpoint(pdev->dev.of_node, NULL); > - if (!ep) > - return -ENODEV; > - > - if (!of_device_is_available(ep)) { > - of_node_put(ep); > + port = of_graph_get_remote_node(pdev->dev.of_node, 0, 0); > + if (!port) > return -ENODEV; > - } > - > - /* add the remote encoder port as component */ > - port = of_graph_get_remote_port_parent(ep); > - of_node_put(ep); > - if (!port || !of_device_is_available(port)) { > - of_node_put(port); > - return -EAGAIN; The HDLCD change looks reasonable except for this -EAGAIN business. I'll have to test your changes on my setup to see how this affects having the encoder as a module. > - } > > drm_of_component_match_add(>dev, , compare_dev, port); > of_node_put(port); > diff --git a/drivers/gpu/drm/arm/malidp_drv.c > b/drivers/gpu/drm/arm/malidp_drv.c > index 32f746e31379..bfa04be7f5de 100644 > --- a/drivers/gpu/drm/arm/malidp_drv.c > +++ b/drivers/gpu/drm/arm/malidp_drv.c > @@ -262,7 +262,6 @@ static int malidp_bind(struct device *dev) > { > struct resource *res; > struct drm_device *drm; > - struct device_node *ep; > struct malidp_drm *malidp; > struct malidp_hw_device *hwdev; > struct platform_device *pdev = to_platform_device(dev); > @@ -360,12 +359,7 @@ static int malidp_bind(struct device *dev) > goto init_fail; > > /* Set the CRTC's port so that the encoder component can find it */ > - ep = of_graph_get_next_endpoint(dev->of_node, NULL); > - if (!ep) { > - ret = -EINVAL; > - goto port_fail; > - } > - malidp->crtc.port = of_get_next_parent(ep); > + malidp->crtc.port = of_graph_get_port_by_id(dev->of_node, 0); > > ret = component_bind_all(dev, drm); > if (ret) { > @@ -418,9 +412,7 @@ static int malidp_bind(struct device *dev) > irq_init_fail: > component_unbind_all(dev, drm); > bind_fail: > - of_node_put(malidp->crtc.port); Why removing this line? AFAICT this is still needed, according to of_graph_get_port_by_id() documentation. > malidp->crtc.port = NULL; > -port_fail: > malidp_fini(drm); > init_fail: > drm->dev_private = NULL; > @@ -478,29 +470,16
Re: [PATCH 1/5] of: introduce of_graph_get_remote_node
On Fri, Feb 03, 2017 at 09:36:31PM -0600, Rob Herring wrote: > The OF graph API leaves too much of the graph walking to clients when > in many cases the driver doesn't care about accessing the port or > endpoint nodes. The drivers typically just want the device connected via > a particular graph connection. of_graph_get_remote_node provides this > functionality. > > Signed-off-by: Rob HerringJust a quick procedural comment: drm-misc for 4.11 is closed already, but if we can get this core patch into 4.11 still then that would avoid cross-tree sync pains in 4.12 ... -Daniel > --- > drivers/of/base.c| 28 > include/linux/of_graph.h | 8 > 2 files changed, 36 insertions(+) > > diff --git a/drivers/of/base.c b/drivers/of/base.c > index d4bea3c797d6..ea18ab16b92c 100644 > --- a/drivers/of/base.c > +++ b/drivers/of/base.c > @@ -2469,3 +2469,31 @@ struct device_node *of_graph_get_remote_port(const > struct device_node *node) > return of_get_next_parent(np); > } > EXPORT_SYMBOL(of_graph_get_remote_port); > + > +struct device_node *of_graph_get_remote_node(const struct device_node *node, > + int port, int endpoint) > +{ > + struct device_node *endpoint_node, *remote; > + > + endpoint_node = of_graph_get_endpoint_by_regs(node, port, endpoint); > + if (!endpoint_node) { > + pr_debug("no valid endpoint (%d, %d) for node %s\n", > + port, endpoint, node->full_name); > + return NULL; > + } > + > + remote = of_graph_get_remote_port_parent(endpoint_node); > + of_node_put(endpoint); > + if (!remote) { > + pr_debug("no valid remote node\n"); > + return NULL; > + } > + > + if (!of_device_is_available(remote)) { > + pr_debug("not available for remote node\n"); > + return NULL; > + } > + > + return remote; > +} > +EXPORT_SYMBOL(of_graph_get_remote_node); > diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h > index bb3a5a2cd570..7b71d3e09209 100644 > --- a/include/linux/of_graph.h > +++ b/include/linux/of_graph.h > @@ -51,6 +51,8 @@ struct device_node *of_graph_get_endpoint_by_regs( > struct device_node *of_graph_get_remote_port_parent( > const struct device_node *node); > struct device_node *of_graph_get_remote_port(const struct device_node *node); > +struct device_node *of_graph_get_remote_node(const struct device_node *node, > + int port, int endpoint); > #else > > static inline int of_graph_parse_endpoint(const struct device_node *node, > @@ -89,6 +91,12 @@ static inline struct device_node *of_graph_get_remote_port( > { > return NULL; > } > +static inline struct device_node *of_graph_get_remote_node( > + const struct device_node *node, > + int port, int endpoint) > +{ > + return NULL; > +} > > #endif /* CONFIG_OF */ > > -- > 2.10.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/5] drm: of: introduce drm_of_find_panel_or_bridge
On Fri, 2017-02-03 at 21:36 -0600, Rob Herring wrote: > Many drivers have a common pattern of searching the OF graph for either an > attached panel or bridge and then finding the DRM struct for the panel > or bridge. Also, most drivers need to handle deferred probing when the > DRM device is not yet instantiated. Create a common function, > drm_of_find_panel_or_bridge, to find the connected node and the > associated DRM panel or bridge device. > > Signed-off-by: Rob Herring> --- > drivers/gpu/drm/drm_of.c | 50 > > include/drm/drm_of.h | 13 + > 2 files changed, 63 insertions(+) > > diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c > index 47848ed8ca48..b29ce2f52113 100644 > --- a/drivers/gpu/drm/drm_of.c > +++ b/drivers/gpu/drm/drm_of.c > @@ -3,7 +3,9 @@ > #include > #include > #include > +#include > #include > +#include > #include > > static void drm_release_of(struct device *dev, void *data) > @@ -207,3 +209,51 @@ int drm_of_encoder_active_endpoint(struct device_node > *node, > return -EINVAL; > } > EXPORT_SYMBOL_GPL(drm_of_encoder_active_endpoint); > + > +/* > + * drm_of_find_panel_or_bridge - return connected panel or bridge device > + * @np: device tree node containing encoder input ports > + * @panel: pointer to hold returned drm_panel > + * @bridge: pointer to hold returned drm_bridge > + * > + * Given a DT node's port and endpoint number, find the connected node and > + * return either the associated struct drm_panel or drm_bridge device. Add a commend that at least one of panel, bridge must be set? > + * Returns zero if successful, or one of the standard error codes if it > fails. > + */ > +int drm_of_find_panel_or_bridge(const struct device_node *np, > + int port, int endpoint, > + struct drm_panel **panel, > + struct drm_bridge **bridge) > +{ > + int ret = -ENODEV; This is only returned if !panel && !bridge. I'd consider this invalid usage of this function, so maybe use -EINVAL? > + struct device_node *remote; > + > + remote = of_graph_get_remote_node(np, port, endpoint); > + if (!remote) > + return -ENODEV; > + > + if (bridge) > + *bridge = NULL; I would move this ^ ... > + if (panel) { > + *panel = of_drm_find_panel(remote); > + if (*panel) { ... here. > + ret = 0; > + goto out_put; > + } > + ret = -EPROBE_DEFER; > + } > + > + if (bridge) { > + *bridge = of_drm_find_bridge(remote); > + if (*bridge) > + ret = 0; > + else > + ret = -EPROBE_DEFER; > + } > +out_put: > + of_node_put(remote); > + return ret; > +} > +EXPORT_SYMBOL_GPL(drm_of_find_panel_or_bridge); > diff --git a/include/drm/drm_of.h b/include/drm/drm_of.h > index 26a64805cc15..f86507f0599b 100644 > --- a/include/drm/drm_of.h > +++ b/include/drm/drm_of.h > @@ -8,6 +8,8 @@ struct component_match; > struct device; > struct drm_device; > struct drm_encoder; > +struct drm_panel; > +struct drm_bridge; > struct device_node; > > #ifdef CONFIG_OF > @@ -23,6 +25,10 @@ extern int drm_of_component_probe(struct device *dev, > extern int drm_of_encoder_active_endpoint(struct device_node *node, > struct drm_encoder *encoder, > struct of_endpoint *endpoint); > +extern int drm_of_find_panel_or_bridge(const struct device_node *np, > +int port, int endpoint, > +struct drm_panel **panel, > +struct drm_bridge **bridge); > #else > static inline uint32_t drm_of_find_possible_crtcs(struct drm_device *dev, > struct device_node *port) > @@ -52,6 +58,13 @@ static inline int drm_of_encoder_active_endpoint(struct > device_node *node, > { > return -EINVAL; > } > +static inline int drm_of_find_panel_or_bridge(const struct device_node *np, > + int port, int endpoint, > + struct drm_panel **panel, > + struct drm_bridge **bridge) > +{ > + return -EINVAL; > +} > #endif > > static inline int drm_of_encoder_active_endpoint_id(struct device_node *node, regards Philipp ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 03/11] drm/msm: Add hint to DRM_IOCTL_MSM_GEM_INFO to return an object IOVA
Modify the 'pad' member of struct drm_msm_gem_info to 'hint'. If the user sets 'hint' to non-zero it means that they want a IOVA for the GEM object instead of a mmap() offset. Return the iova in the 'offset' member. Signed-off-by: Jordan Crouse--- drivers/gpu/drm/msm/msm_drv.c | 29 + include/uapi/drm/msm_drm.h| 4 ++-- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index e29bb66..1e4e022 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -677,6 +677,17 @@ static int msm_ioctl_gem_cpu_fini(struct drm_device *dev, void *data, return ret; } +static int msm_ioctl_gem_info_iova(struct drm_device *dev, + struct drm_gem_object *obj, uint64_t *iova) +{ + struct msm_drm_private *priv = dev->dev_private; + + if (!priv->gpu) + return -EINVAL; + + return msm_gem_get_iova(obj, priv->gpu->aspace, iova); +} + static int msm_ioctl_gem_info(struct drm_device *dev, void *data, struct drm_file *file) { @@ -684,14 +695,24 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data, struct drm_gem_object *obj; int ret = 0; - if (args->pad) - return -EINVAL; - obj = drm_gem_object_lookup(file, args->handle); if (!obj) return -ENOENT; - args->offset = msm_gem_mmap_offset(obj); + /* +* If the hint variable is set, it means that the user wants a IOVA for +* this buffer. Return the address from the GPU because that is +* probably what it is looking for +*/ + if (args->hint) { + uint64_t iova; + + ret = msm_ioctl_gem_info_iova(dev, obj, ); + if (!ret) + args->offset = iova; + } else { + args->offset = msm_gem_mmap_offset(obj); + } drm_gem_object_unreference_unlocked(obj); diff --git a/include/uapi/drm/msm_drm.h b/include/uapi/drm/msm_drm.h index 4d5d6a2..045ad20 100644 --- a/include/uapi/drm/msm_drm.h +++ b/include/uapi/drm/msm_drm.h @@ -105,8 +105,8 @@ struct drm_msm_gem_new { struct drm_msm_gem_info { __u32 handle; /* in */ - __u32 pad; - __u64 offset; /* out, offset to pass to mmap() */ + __u32 hint; /* in */ + __u64 offset; /* out, mmap() offset if hint is 0, iova if 1 */ }; #define MSM_PREP_READ0x01 -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] drm: convert drivers to use drm_of_find_panel_or_bridge
On Sun, Feb 5, 2017 at 10:01 PM, Fabio Estevamwrote: > > The tda998x_probe() returns success, but tda998x_bind() is never called. > > I am trying to understand what needs to be done in > drivers/gpu/drm/mxsfb/ so that it can bind the tda998x driver. Looks like that converting to component style and adding component_bind_all() does the trick: [1.323772] tda998x 2-0070: found TDA19988 [1.342381] mxsfb 222.lcdif: bound 2-0070 (ops tda998x_ops) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 01/11] drm/msm: Make sure to detach the MMU during GPU cleanup
We should be detaching the MMU before destroying the address space. To do this cleanly, the detach has to happen in adreno_gpu_cleanup() because it needs access to structs in adreno_gpu.c. Plus it is better symmetry to have the attach and detach at the same code level. Signed-off-by: Jordan Crouse--- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 29 +++-- drivers/gpu/drm/msm/msm_gpu.c | 3 --- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index bc2224b..acb685a 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -421,18 +421,27 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, return 0; } -void adreno_gpu_cleanup(struct adreno_gpu *gpu) +void adreno_gpu_cleanup(struct adreno_gpu *adreno_gpu) { - if (gpu->memptrs_bo) { - if (gpu->memptrs) - msm_gem_put_vaddr(gpu->memptrs_bo); + struct msm_gpu *gpu = _gpu->base; + + if (adreno_gpu->memptrs_bo) { + if (adreno_gpu->memptrs) + msm_gem_put_vaddr(adreno_gpu->memptrs_bo); + + if (adreno_gpu->memptrs_iova) + msm_gem_put_iova(adreno_gpu->memptrs_bo, gpu->id); + + drm_gem_object_unreference_unlocked(adreno_gpu->memptrs_bo); + } + release_firmware(adreno_gpu->pm4); + release_firmware(adreno_gpu->pfp); - if (gpu->memptrs_iova) - msm_gem_put_iova(gpu->memptrs_bo, gpu->base.id); + msm_gpu_cleanup(gpu); - drm_gem_object_unreference_unlocked(gpu->memptrs_bo); + if (gpu->aspace) { + gpu->aspace->mmu->funcs->detach(gpu->aspace->mmu, + iommu_ports, ARRAY_SIZE(iommu_ports)); + msm_gem_address_space_destroy(gpu->aspace); } - release_firmware(gpu->pm4); - release_firmware(gpu->pfp); - msm_gpu_cleanup(>base); } diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index d8420be..403cca1 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -711,9 +711,6 @@ void msm_gpu_cleanup(struct msm_gpu *gpu) msm_ringbuffer_destroy(gpu->rb); } - if (gpu->aspace) - msm_gem_address_space_destroy(gpu->aspace); - if (gpu->fctx) msm_fence_context_free(gpu->fctx); } -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/5] drm: of: introduce drm_of_find_panel_or_bridge
On Fri, Feb 03, 2017 at 09:36:32PM -0600, Rob Herring wrote: > Many drivers have a common pattern of searching the OF graph for either an > attached panel or bridge and then finding the DRM struct for the panel > or bridge. Also, most drivers need to handle deferred probing when the > DRM device is not yet instantiated. Create a common function, > drm_of_find_panel_or_bridge, to find the connected node and the > associated DRM panel or bridge device. > > Signed-off-by: Rob Herring> --- > drivers/gpu/drm/drm_of.c | 50 > > include/drm/drm_of.h | 13 + > 2 files changed, 63 insertions(+) > > diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c > index 47848ed8ca48..b29ce2f52113 100644 > --- a/drivers/gpu/drm/drm_of.c > +++ b/drivers/gpu/drm/drm_of.c > @@ -3,7 +3,9 @@ > #include > #include > #include > +#include > #include > +#include > #include > > static void drm_release_of(struct device *dev, void *data) > @@ -207,3 +209,51 @@ int drm_of_encoder_active_endpoint(struct device_node > *node, > return -EINVAL; > } > EXPORT_SYMBOL_GPL(drm_of_encoder_active_endpoint); > + > +/* > + * drm_of_find_panel_or_bridge - return connected panel or bridge device > + * @np: device tree node containing encoder input ports > + * @panel: pointer to hold returned drm_panel > + * @bridge: pointer to hold returned drm_bridge > + * > + * Given a DT node's port and endpoint number, find the connected node and > + * return either the associated struct drm_panel or drm_bridge device. > + * > + * Returns zero if successful, or one of the standard error codes if it > fails. > + */ > +int drm_of_find_panel_or_bridge(const struct device_node *np, > + int port, int endpoint, > + struct drm_panel **panel, > + struct drm_bridge **bridge) > +{ > + int ret = -ENODEV; Given that you never return 'ret' with -ENODEV can I suggest that you initialise 'ret' with -EPROBE_DEFER and then you can get rid of the two assignments further down? Otherwise, looks good to me. Best regards, Liviu > + struct device_node *remote; > + > + remote = of_graph_get_remote_node(np, port, endpoint); > + if (!remote) > + return -ENODEV; > + > + if (bridge) > + *bridge = NULL; > + > + if (panel) { > + *panel = of_drm_find_panel(remote); > + if (*panel) { > + ret = 0; > + goto out_put; > + } > + ret = -EPROBE_DEFER; > + } > + > + if (bridge) { > + *bridge = of_drm_find_bridge(remote); > + if (*bridge) > + ret = 0; > + else > + ret = -EPROBE_DEFER; > + } > +out_put: > + of_node_put(remote); > + return ret; > +} > +EXPORT_SYMBOL_GPL(drm_of_find_panel_or_bridge); > diff --git a/include/drm/drm_of.h b/include/drm/drm_of.h > index 26a64805cc15..f86507f0599b 100644 > --- a/include/drm/drm_of.h > +++ b/include/drm/drm_of.h > @@ -8,6 +8,8 @@ struct component_match; > struct device; > struct drm_device; > struct drm_encoder; > +struct drm_panel; > +struct drm_bridge; > struct device_node; > > #ifdef CONFIG_OF > @@ -23,6 +25,10 @@ extern int drm_of_component_probe(struct device *dev, > extern int drm_of_encoder_active_endpoint(struct device_node *node, > struct drm_encoder *encoder, > struct of_endpoint *endpoint); > +extern int drm_of_find_panel_or_bridge(const struct device_node *np, > +int port, int endpoint, > +struct drm_panel **panel, > +struct drm_bridge **bridge); > #else > static inline uint32_t drm_of_find_possible_crtcs(struct drm_device *dev, > struct device_node *port) > @@ -52,6 +58,13 @@ static inline int drm_of_encoder_active_endpoint(struct > device_node *node, > { > return -EINVAL; > } > +static inline int drm_of_find_panel_or_bridge(const struct device_node *np, > + int port, int endpoint, > + struct drm_panel **panel, > + struct drm_bridge **bridge) > +{ > + return -EINVAL; > +} > #endif > > static inline int drm_of_encoder_active_endpoint_id(struct device_node *node, > -- > 2.10.1 > -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/5] drm/exynos: dsi: Fix the parse_dt function
The bridge_node may or may not be required. For example, mic and dsi are connected to OF graph, but fimd and dsi are not connected. Also the OF graph is not needed because the panel is a child of dsi. So not have to go to the endpoint and parse the burst, esc clock-frequency. Signed-off-by: Hoegeun Kwon--- drivers/gpu/drm/exynos/exynos_drm_dsi.c | 32 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c index e07cb1f..214d486 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c @@ -1652,39 +1652,23 @@ static int exynos_dsi_parse_dt(struct exynos_dsi *dsi) if (ret < 0) return ret; - ep = of_graph_get_endpoint_by_regs(node, DSI_PORT_OUT, 0); - if (!ep) { - dev_err(dev, "no output port with endpoint specified\n"); - return -EINVAL; - } - - ret = exynos_dsi_of_read_u32(ep, "samsung,burst-clock-frequency", + ret = exynos_dsi_of_read_u32(node, "samsung,burst-clock-frequency", >burst_clk_rate); if (ret < 0) - goto end; + return ret; - ret = exynos_dsi_of_read_u32(ep, "samsung,esc-clock-frequency", + ret = exynos_dsi_of_read_u32(node, "samsung,esc-clock-frequency", >esc_clk_rate); if (ret < 0) - goto end; - - of_node_put(ep); + return ret; ep = of_graph_get_next_endpoint(node, NULL); - if (!ep) { - ret = -EINVAL; - goto end; - } - - dsi->bridge_node = of_graph_get_remote_port_parent(ep); - if (!dsi->bridge_node) { - ret = -EINVAL; - goto end; + if (ep) { + dsi->bridge_node = of_graph_get_remote_port_parent(ep); + of_node_put(ep); } -end: - of_node_put(ep); - return ret; + return 0; } static int exynos_dsi_bind(struct device *dev, struct device *master, -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node
On Mon, Feb 06, 2017 at 05:23:06PM +, Liviu Dudau wrote: > On Mon, Feb 06, 2017 at 11:09:49AM -0600, Rob Herring wrote: > > On Mon, Feb 06, 2017 at 10:29:33AM +, Liviu Dudau wrote: > > > On Fri, Feb 03, 2017 at 09:36:33PM -0600, Rob Herring wrote: > > > > - /* add the remote encoder port as component */ > > > > - port = of_graph_get_remote_port_parent(ep); > > > > - of_node_put(ep); > > > > - if (!port || !of_device_is_available(port)) { > > > > - of_node_put(port); > > > > - return -EAGAIN; > > > > > > The HDLCD change looks reasonable except for this -EAGAIN business. I'll > > > have to > > > test your changes on my setup to see how this affects having the encoder > > > as a module. > > > > What are you expecting to happen with -EAGAIN? This one was a bit of an > > oddball. > > When both the HDLCD and the TDA998x drivers are compiled as modules, the > order in which they are inserted can be somewhat random (due to testing). Not really "due to testing" but if you run a real distro, they tend to have a multi-threaded behaviour when loading kernel modules at boot. > It is at that time when you want the probe of HDLCD to be retried on the > insmod-ing of the tda998x.ko rather than fail entirely. -EAGAIN doesn't get you that, and in any case, solving that problem is exactly why the component API exists - so that DRM only comes up once all the necessary components are available. -EAGAIN also doesn't get you that from inside a probe function - such an error will be reported in the kernel log, and no further action will be taken (the device driver probe will be failed, and not automatically retried. The only case that we automatically retry is if a driver returns -EPROBE_DEFER. Everything else causes a probe failure. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 06/11] drm/msm: Add a struct to pass configuration to msm_gpu_init()
The amount of information that we need to pass into msm_gpu_init() is steadily increasing, so add a new struct to stabilize the function call and make it easier to add new configuration down the line. Signed-off-by: Jordan Crouse--- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 12 ++-- drivers/gpu/drm/msm/msm_gpu.c | 13 ++--- drivers/gpu/drm/msm/msm_gpu.h | 11 ++- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index 247f017..53f9dea 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -344,6 +344,7 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, struct adreno_gpu *adreno_gpu, const struct adreno_gpu_funcs *funcs) { struct adreno_platform_config *config = pdev->dev.platform_data; + struct msm_gpu_config adreno_gpu_config = { 0 }; struct msm_gpu *gpu = _gpu->base; int ret; @@ -364,9 +365,16 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, DBG("fast_rate=%u, slow_rate=%u, bus_freq=%u", gpu->fast_rate, gpu->slow_rate, gpu->bus_freq); + adreno_gpu_config.ioname = "kgsl_3d0_reg_memory"; + adreno_gpu_config.irqname = "kgsl_3d0_irq"; + + adreno_gpu_config.va_start = SZ_16M; + adreno_gpu_config.va_end = 0x; + + adreno_gpu_config.ringsz = RB_SIZE; + ret = msm_gpu_init(drm, pdev, _gpu->base, >base, - adreno_gpu->info->name, "kgsl_3d0_reg_memory", "kgsl_3d0_irq", - RB_SIZE); + adreno_gpu->info->name, _gpu_config); if (ret) return ret; diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index d336c24..bc75425 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -570,7 +570,7 @@ static irqreturn_t irq_handler(int irq, void *data) int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev, struct msm_gpu *gpu, const struct msm_gpu_funcs *funcs, - const char *name, const char *ioname, const char *irqname, int ringsz) + const char *name, struct msm_gpu_config *config) { struct iommu_domain *iommu; int i, ret; @@ -606,14 +606,14 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev, BUG_ON(ARRAY_SIZE(clk_names) != ARRAY_SIZE(gpu->grp_clks)); /* Map registers: */ - gpu->mmio = msm_ioremap(pdev, ioname, name); + gpu->mmio = msm_ioremap(pdev, config->ioname, name); if (IS_ERR(gpu->mmio)) { ret = PTR_ERR(gpu->mmio); goto fail; } /* Get Interrupt: */ - gpu->irq = platform_get_irq_byname(pdev, irqname); + gpu->irq = platform_get_irq_byname(pdev, config->irqname); if (gpu->irq < 0) { ret = gpu->irq; dev_err(drm->dev, "failed to get irq: %d\n", ret); @@ -657,9 +657,8 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev, */ iommu = iommu_domain_alloc(_bus_type); if (iommu) { - /* TODO 32b vs 64b address space.. */ - iommu->geometry.aperture_start = SZ_16M; - iommu->geometry.aperture_end = 0x; + iommu->geometry.aperture_start = config->va_start; + iommu->geometry.aperture_end = config->va_end; dev_info(drm->dev, "%s: using IOMMU\n", name); gpu->aspace = msm_gem_address_space_create(>dev, @@ -678,7 +677,7 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev, /* Create ringbuffer: */ mutex_lock(>struct_mutex); - gpu->rb = msm_ringbuffer_new(gpu, ringsz); + gpu->rb = msm_ringbuffer_new(gpu, config->ringsz); mutex_unlock(>struct_mutex); if (IS_ERR(gpu->rb)) { ret = PTR_ERR(gpu->rb); diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h index ad6d13a..cc6530f 100644 --- a/drivers/gpu/drm/msm/msm_gpu.h +++ b/drivers/gpu/drm/msm/msm_gpu.h @@ -28,6 +28,14 @@ struct msm_gem_submit; struct msm_gpu_perfcntr; +struct msm_gpu_config { + const char *ioname; + const char *irqname; + uint64_t va_start; + uint64_t va_end; + unsigned int ringsz; +}; + /* So far, with hardware that I've seen to date, we can have: * + zero, one, or two z180 2d cores * + a3xx or a2xx 3d core, which share a common CP (the firmware @@ -205,7 +213,8 @@ void msm_gpu_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit, int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev, struct msm_gpu *gpu, const struct msm_gpu_funcs *funcs, - const char
Re: [PATCH 2/5] drm: of: introduce drm_of_find_panel_or_bridge
On Mon, Feb 06, 2017 at 11:42:48AM +0100, Philipp Zabel wrote: > On Fri, 2017-02-03 at 21:36 -0600, Rob Herring wrote: > > Many drivers have a common pattern of searching the OF graph for either an > > attached panel or bridge and then finding the DRM struct for the panel > > or bridge. Also, most drivers need to handle deferred probing when the > > DRM device is not yet instantiated. Create a common function, > > drm_of_find_panel_or_bridge, to find the connected node and the > > associated DRM panel or bridge device. [...] > > +int drm_of_find_panel_or_bridge(const struct device_node *np, > > + int port, int endpoint, > > + struct drm_panel **panel, > > + struct drm_bridge **bridge) > > +{ > > + int ret = -ENODEV; > > This is only returned if !panel && !bridge. I'd consider this invalid > usage of this function, so maybe use -EINVAL? Yes. > > + struct device_node *remote; > > + > > + remote = of_graph_get_remote_node(np, port, endpoint); > > + if (!remote) > > + return -ENODEV; > > + > > + if (bridge) > > + *bridge = NULL; > > I would move this ^ ... > > > + if (panel) { > > + *panel = of_drm_find_panel(remote); > > + if (*panel) { > > ... here. Okay. > > + ret = 0; > > + goto out_put; > > + } > > + ret = -EPROBE_DEFER; > > + } > > + > > + if (bridge) { > > + *bridge = of_drm_find_bridge(remote); > > + if (*bridge) > > + ret = 0; > > + else > > + ret = -EPROBE_DEFER; > > + } > > +out_put: > > + of_node_put(remote); > > + return ret; > > +} I've ended up re-writing things a bit getting rid of the goto and the result looks like this: int drm_of_find_panel_or_bridge(const struct device_node *np, int port, int endpoint, struct drm_panel **panel, struct drm_bridge **bridge) { int ret = -EPROBE_DEFER; struct device_node *remote; if (!panel && !bridge) return -EINVAL; remote = of_graph_get_remote_node(np, port, endpoint); if (!remote) return -ENODEV; if (panel) { *panel = of_drm_find_panel(remote); if (*panel) { if (bridge) *bridge = NULL; ret = 0; } } /* No panel found yet, check for a bridge next. */ if (ret && bridge) { *bridge = of_drm_find_bridge(remote); if (*bridge) ret = 0; } of_node_put(remote); return ret; } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V7 3/4] drm/bridge: Add driver for GE B850v3 LVDS/DP++ Bridge
On Fri, Feb 03, 2017 at 12:25:24PM +, Emil Velikov wrote: > On 3 February 2017 at 08:00, Daniel Vetterwrote: > > On Thu, Feb 02, 2017 at 12:37:21PM +, Emil Velikov wrote: > >> - Daniel, gents - is drm-misc aimed at devs with limited (no?) > >> review/commit history in the area and/or the kernel in general ? > >> In this case, Peter have quite noticeable experience [in kernel > >> development] with little-to no in DRM. > >> Should one draw a line in the sand somewhere ? > > > > We're fairly lenient with accepting new drivers already, and for drivers > > in drm-misc we require some checks and peer review to make sure new folks > > learn faster. But it's an expirement, I fully expect some fallout and then > > some learnign and fine tuning from that, and maybe we even need to crank > > requirements back up to a much higher level of experience. > > > > But for drivers I'm honestly not too concerned: As long as there's some > > process in place to foster learning (which I think will be > > better with this) I really don't see much harm in merging non-perfect > > driver code. > > > Thank you Daniel, this is very well said. Being lenient to begin with > and adjusting where/if needed is the more welcoming approach, indeed. > Doubt there'll be (m)any cases of (ab|mis)use but even if so, nobody > can see the future. Yeah, the big idea behind drm-misc is also to make drm more welcoming for new folks, and I think starting with a small driver or a few driver patches for hw you have lying around for hacking is a great way. And every once in a while we can "trick" one of these newbies into doing some cool core refactoring or cleanup, and maybe even to stick around for a while. All for world domination and other evil purposes ofc :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node
On Fri, 2017-02-03 at 21:36 -0600, Rob Herring wrote: > Convert drivers to use the new of_graph_get_remote_node() helper > instead of parsing the endpoint node and then getting the remote device > node. Now drivers can just specify the device node and which > port/endpoint and get back the connected remote device node. The details > of the graph binding are nicely abstracted into the core OF graph code. > > This changes some error messages to debug messages (in the graph core). > Graph connections are often "no connects" depending on the particular > board, so we want to avoid spurious messages. Plus the kernel is not a > DT validator. > > Signed-off-by: Rob Herring> --- > drivers/gpu/drm/arm/hdlcd_drv.c | 22 ++--- > drivers/gpu/drm/arm/malidp_drv.c| 29 ++- > drivers/gpu/drm/bridge/adv7511/adv7533.c| 12 + > drivers/gpu/drm/bridge/dumb-vga-dac.c | 15 ++ > drivers/gpu/drm/bridge/ti-tfp410.c | 15 ++ > drivers/gpu/drm/exynos/exynos_drm_dpi.c | 16 +- > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 13 ++--- > drivers/gpu/drm/exynos/exynos_drm_mic.c | 27 +- > drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c| 26 ++ > drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 30 +-- > drivers/gpu/drm/mediatek/mtk_dpi.c | 12 ++--- > drivers/gpu/drm/mediatek/mtk_hdmi.c | 26 ++ > drivers/gpu/drm/meson/meson_drv.c | 12 ++--- > drivers/gpu/drm/meson/meson_venc_cvbs.c | 19 ++- > drivers/gpu/drm/msm/dsi/dsi_host.c | 3 +- > drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 28 +-- > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 18 +++ > drivers/gpu/drm/tilcdc/tilcdc_crtc.c| 11 + > drivers/gpu/drm/tilcdc/tilcdc_external.c| 66 > +++-- > drivers/gpu/drm/vc4/vc4_dpi.c | 15 ++ > 20 files changed, 64 insertions(+), 351 deletions(-) > [...] > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c > b/drivers/gpu/drm/mediatek/mtk_dpi.c > index 90fb831ef031..dbd554c09a39 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dpi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c > @@ -661,7 +661,7 @@ static int mtk_dpi_probe(struct platform_device *pdev) > struct device *dev = >dev; > struct mtk_dpi *dpi; > struct resource *mem; > - struct device_node *ep, *bridge_node = NULL; > + struct device_node *bridge_node; > int comp_id; > int ret; > > @@ -706,15 +706,9 @@ static int mtk_dpi_probe(struct platform_device *pdev) > return -EINVAL; > } > > - ep = of_graph_get_next_endpoint(dev->of_node, NULL); > - if (ep) { > - bridge_node = of_graph_get_remote_port_parent(ep); > - of_node_put(ep); > - } > - if (!bridge_node) { > - dev_err(dev, "Failed to find bridge node\n"); > + bridge_node = of_graph_get_remote_node(dev->of_node, 0, 0); Note that before this change, of_graph_get_next_endpoint would just choose the first port/endpoint without restrictions to their reg properties, whereas the new code requires reg to be either not set or set to zero. As the former is the case for the mt8173 dpi->hdmi link, which is the only use of this driver, this should be fine. [...] > diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c > b/drivers/gpu/drm/mediatek/mtk_hdmi.c > index 0e8c4d9af340..f14e472812ce 100644 > --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c > +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c > @@ -1433,7 +1433,7 @@ static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi > *hdmi, > { > struct device *dev = >dev; > struct device_node *np = dev->of_node; > - struct device_node *cec_np, *port, *ep, *remote, *i2c_np; > + struct device_node *cec_np, *remote, *i2c_np; > struct platform_device *cec_pdev; > struct regmap *regmap; > struct resource *mem; > @@ -1485,29 +1485,9 @@ static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi > *hdmi, > if (IS_ERR(hdmi->regs)) > return PTR_ERR(hdmi->regs); > > - port = of_graph_get_port_by_id(np, 1); > - if (!port) { > - dev_err(dev, "Missing output port node\n"); > + remote = of_graph_get_remote_node(np, 1, 0); > + if (!remote) > return -EINVAL; > - } > - > - ep = of_get_child_by_name(port, "endpoint"); > - if (!ep) { > - dev_err(dev, "Missing endpoint node in port %s\n", > - port->full_name); > - of_node_put(port); > - return -EINVAL; > - } > - of_node_put(port); > - > - remote = of_graph_get_remote_port_parent(ep); > - if (!remote) { > - dev_err(dev, "Missing connector/bridge node for endpoint %s\n", > - ep->full_name); > - of_node_put(ep); > - return -EINVAL; > -
[PATCH 3/5] arm: dts: Move the clock-frequency property for exynos3250 dts
The OF graph is not needed because the panel is a child of dsi. So removed the ports and moved burst, esc clock-frequency property to the top. Signed-off-by: Hoegeun Kwon--- arch/arm/boot/dts/exynos3250-rinato.dts | 23 ++- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/arch/arm/boot/dts/exynos3250-rinato.dts b/arch/arm/boot/dts/exynos3250-rinato.dts index 548413e..82e676a 100644 --- a/arch/arm/boot/dts/exynos3250-rinato.dts +++ b/arch/arm/boot/dts/exynos3250-rinato.dts @@ -215,24 +215,11 @@ _0 { vddcore-supply = <_reg>; vddio-supply = <_reg>; + samsung,burst-clock-frequency = <25000>; + samsung,esc-clock-frequency = <2000>; samsung,pll-clock-frequency = <2400>; status = "okay"; - ports { - #address-cells = <1>; - #size-cells = <0>; - - port@1 { - reg = <1>; - - dsi_out: endpoint { - remote-endpoint = <_in>; - samsung,burst-clock-frequency = <25000>; - samsung,esc-clock-frequency = <2000>; - }; - }; - }; - panel@0 { compatible = "samsung,s6e63j0x03"; reg = <0>; @@ -262,12 +249,6 @@ vsync-len = <2>; }; }; - - port { - dsi_in: endpoint { - remote-endpoint = <_out>; - }; - }; }; }; -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/5] of: introduce of_graph_get_remote_node
On Mon, Feb 6, 2017 at 4:32 AM, Philipp Zabelwrote: > Hi Rob, > > thanks for this clean-up series! I was not aware how far the duplication > has spread over time. > > On Fri, 2017-02-03 at 21:36 -0600, Rob Herring wrote: >> The OF graph API leaves too much of the graph walking to clients when >> in many cases the driver doesn't care about accessing the port or >> endpoint nodes. The drivers typically just want the device connected via >> a particular graph connection. of_graph_get_remote_node provides this >> functionality. >> >> Signed-off-by: Rob Herring >> --- >> drivers/of/base.c| 28 >> include/linux/of_graph.h | 8 >> 2 files changed, 36 insertions(+) >> >> diff --git a/drivers/of/base.c b/drivers/of/base.c >> index d4bea3c797d6..ea18ab16b92c 100644 >> --- a/drivers/of/base.c >> +++ b/drivers/of/base.c >> @@ -2469,3 +2469,31 @@ struct device_node *of_graph_get_remote_port(const >> struct device_node *node) >> return of_get_next_parent(np); >> } >> EXPORT_SYMBOL(of_graph_get_remote_port); >> + >> +struct device_node *of_graph_get_remote_node(const struct device_node *node, >> + int port, int endpoint) > > I think this should have a documentation comment, similar to the > of_graph_get_endpoint_by_regs one, as it is not really clear from the > function name that the returned device node is the parent (or > grandparent) device node containing the remote port to the specified > node & port & endpoint. > Also it might be interesting to the user that -1 is a wildcard value for > port / endpoint. I really want to not allow using a wildcard here. Drivers should know what port they want (or iterate over all of them). It didn't look like any drivers were depending on the wildcard, but were just using -1 for "no reg property" when really that should 0. Of course, I may have missed something. I guess I could enforce port/endpoint > 0 here as there's no existing users. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/5] arm: dts: Move the clock-frequency property for exynos4210 dts
The OF graph is not needed because the panel is a child of dsi. So removed the ports and moved burst, esc clock-frequency property to the top. Signed-off-by: Hoegeun Kwon--- arch/arm/boot/dts/exynos4210-trats.dts | 23 ++- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/arch/arm/boot/dts/exynos4210-trats.dts b/arch/arm/boot/dts/exynos4210-trats.dts index 0ca1b4d..9452bed 100644 --- a/arch/arm/boot/dts/exynos4210-trats.dts +++ b/arch/arm/boot/dts/exynos4210-trats.dts @@ -197,24 +197,11 @@ _0 { vddcore-supply = <_reg>; vddio-supply = <_reg>; + samsung,burst-clock-frequency = <5>; + samsung,esc-clock-frequency = <2000>; samsung,pll-clock-frequency = <2400>; status = "okay"; - ports { - #address-cells = <1>; - #size-cells = <0>; - - port@1 { - reg = <1>; - - dsi_out: endpoint { - remote-endpoint = <_in>; - samsung,burst-clock-frequency = <5>; - samsung,esc-clock-frequency = <2000>; - }; - }; - }; - panel@0 { reg = <0>; compatible = "samsung,s6e8aa0"; @@ -242,12 +229,6 @@ vsync-len = <2>; }; }; - - port { - dsi_in: endpoint { - remote-endpoint = <_out>; - }; - }; }; }; -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/5] arm64: dts: exynos: Move the clock-frequency property
The OF graph is not needed because the panel is a child of dsi. So removed the ports and moved burst, esc clock-frequency property to the top. Signed-off-by: Hoegeun Kwon--- arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 16 ++-- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi index 6ce93a3..77ba238 100644 --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi @@ -298,23 +298,11 @@ status = "okay"; vddcore-supply = <_reg>; vddio-supply = <_reg>; + samsung,burst-clock-frequency = <51200>; + samsung,esc-clock-frequency = <1600>; samsung,pll-clock-frequency = <2400>; pinctrl-names = "default"; pinctrl-0 = <_irq>; - - ports { - #address-cells = <1>; - #size-cells = <0>; - - port@1 { - reg = <1>; - - dsi_out: endpoint { - samsung,burst-clock-frequency = <51200>; - samsung,esc-clock-frequency = <1600>; - }; - }; - }; }; { -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/5] arm: dts: Move the clock-frequency property for exynos4412 dts
The OF graph is not needed because the panel is a child of dsi. So removed the ports and moved burst, esc clock-frequency property to the top. Signed-off-by: Hoegeun Kwon--- arch/arm/boot/dts/exynos4412-trats2.dts | 23 ++- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts b/arch/arm/boot/dts/exynos4412-trats2.dts index 41ecd6d..86ce5e5 100644 --- a/arch/arm/boot/dts/exynos4412-trats2.dts +++ b/arch/arm/boot/dts/exynos4412-trats2.dts @@ -385,24 +385,11 @@ _0 { vddcore-supply = <_reg>; vddio-supply = <_reg>; + samsung,burst-clock-frequency = <5>; + samsung,esc-clock-frequency = <2000>; samsung,pll-clock-frequency = <2400>; status = "okay"; - ports { - #address-cells = <1>; - #size-cells = <0>; - - port@1 { - reg = <1>; - - dsi_out: endpoint { - remote-endpoint = <_in>; - samsung,burst-clock-frequency = <5>; - samsung,esc-clock-frequency = <2000>; - }; - }; - }; - panel@0 { compatible = "samsung,s6e8aa0"; reg = <0>; @@ -430,12 +417,6 @@ vsync-len = <2>; }; }; - - port { - dsi_in: endpoint { - remote-endpoint = <_out>; - }; - }; }; }; -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node
On Mon, Feb 06, 2017 at 11:09:49AM -0600, Rob Herring wrote: > On Mon, Feb 06, 2017 at 10:29:33AM +, Liviu Dudau wrote: > > On Fri, Feb 03, 2017 at 09:36:33PM -0600, Rob Herring wrote: > > > Convert drivers to use the new of_graph_get_remote_node() helper > > > instead of parsing the endpoint node and then getting the remote device > > > node. Now drivers can just specify the device node and which > > > port/endpoint and get back the connected remote device node. The details > > > of the graph binding are nicely abstracted into the core OF graph code. > > > > > > This changes some error messages to debug messages (in the graph core). > > > Graph connections are often "no connects" depending on the particular > > > board, so we want to avoid spurious messages. Plus the kernel is not a > > > DT validator. > > > > > > Signed-off-by: Rob Herring> > > --- > > > drivers/gpu/drm/arm/hdlcd_drv.c | 22 ++--- > > > drivers/gpu/drm/arm/malidp_drv.c| 29 ++- > > > drivers/gpu/drm/bridge/adv7511/adv7533.c| 12 + > > > drivers/gpu/drm/bridge/dumb-vga-dac.c | 15 ++ > > > drivers/gpu/drm/bridge/ti-tfp410.c | 15 ++ > > > drivers/gpu/drm/exynos/exynos_drm_dpi.c | 16 +- > > > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 13 ++--- > > > drivers/gpu/drm/exynos/exynos_drm_mic.c | 27 +- > > > drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c| 26 ++ > > > drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 30 +-- > > > drivers/gpu/drm/mediatek/mtk_dpi.c | 12 ++--- > > > drivers/gpu/drm/mediatek/mtk_hdmi.c | 26 ++ > > > drivers/gpu/drm/meson/meson_drv.c | 12 ++--- > > > drivers/gpu/drm/meson/meson_venc_cvbs.c | 19 ++- > > > drivers/gpu/drm/msm/dsi/dsi_host.c | 3 +- > > > drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 28 +-- > > > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 18 +++ > > > drivers/gpu/drm/tilcdc/tilcdc_crtc.c| 11 + > > > drivers/gpu/drm/tilcdc/tilcdc_external.c| 66 > > > +++-- > > > drivers/gpu/drm/vc4/vc4_dpi.c | 15 ++ > > > 20 files changed, 64 insertions(+), 351 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c > > > b/drivers/gpu/drm/arm/hdlcd_drv.c > > > index e5f4f4a6546d..0f70f5fe9970 100644 > > > --- a/drivers/gpu/drm/arm/hdlcd_drv.c > > > +++ b/drivers/gpu/drm/arm/hdlcd_drv.c > > > @@ -430,29 +430,13 @@ static int compare_dev(struct device *dev, void > > > *data) > > > > > > static int hdlcd_probe(struct platform_device *pdev) > > > { > > > - struct device_node *port, *ep; > > > + struct device_node *port; > > > struct component_match *match = NULL; > > > > > > - if (!pdev->dev.of_node) > > > - return -ENODEV; > > > - > > > /* there is only one output port inside each device, find it */ > > > - ep = of_graph_get_next_endpoint(pdev->dev.of_node, NULL); > > > - if (!ep) > > > - return -ENODEV; > > > - > > > - if (!of_device_is_available(ep)) { > > > - of_node_put(ep); > > > + port = of_graph_get_remote_node(pdev->dev.of_node, 0, 0); > > > + if (!port) > > > return -ENODEV; > > > - } > > > - > > > - /* add the remote encoder port as component */ > > > - port = of_graph_get_remote_port_parent(ep); > > > - of_node_put(ep); > > > - if (!port || !of_device_is_available(port)) { > > > - of_node_put(port); > > > - return -EAGAIN; > > > > The HDLCD change looks reasonable except for this -EAGAIN business. I'll > > have to > > test your changes on my setup to see how this affects having the encoder as > > a module. > > What are you expecting to happen with -EAGAIN? This one was a bit of an > oddball. When both the HDLCD and the TDA998x drivers are compiled as modules, the order in which they are inserted can be somewhat random (due to testing). It is at that time when you want the probe of HDLCD to be retried on the insmod-ing of the tda998x.ko rather than fail entirely. > > This condition would only change if you had an overlay. That's a use > case that needs to be handled in a common way ('cause I don't want to > clean-up every driver doing overlays in their own way latter). Just > having "status" changing at runtime would have all sorts of implications > in the kernel. Hmm, not sure what you mean here with overlays. Are you thinking that the remote port is initially disabled and then re-enabled by an overlay? That is not the only way of_device_is_available() can fail, see above regarding modules. Best regards, Liviu > > > > > > - } > > > > > > drm_of_component_match_add(>dev, , compare_dev, port); > > > of_node_put(port); > > > diff --git a/drivers/gpu/drm/arm/malidp_drv.c > > > b/drivers/gpu/drm/arm/malidp_drv.c > > > index 32f746e31379..bfa04be7f5de 100644 > > > ---
Re: [PATCH 2/5] drm: of: introduce drm_of_find_panel_or_bridge
On Mon, Feb 6, 2017 at 4:18 AM, Liviu Dudauwrote: > On Fri, Feb 03, 2017 at 09:36:32PM -0600, Rob Herring wrote: >> Many drivers have a common pattern of searching the OF graph for either an >> attached panel or bridge and then finding the DRM struct for the panel >> or bridge. Also, most drivers need to handle deferred probing when the >> DRM device is not yet instantiated. Create a common function, >> drm_of_find_panel_or_bridge, to find the connected node and the >> associated DRM panel or bridge device. >> >> Signed-off-by: Rob Herring >> --- >> drivers/gpu/drm/drm_of.c | 50 >> >> include/drm/drm_of.h | 13 + >> 2 files changed, 63 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c >> index 47848ed8ca48..b29ce2f52113 100644 >> --- a/drivers/gpu/drm/drm_of.c >> +++ b/drivers/gpu/drm/drm_of.c >> @@ -3,7 +3,9 @@ >> #include >> #include >> #include >> +#include >> #include >> +#include >> #include >> >> static void drm_release_of(struct device *dev, void *data) >> @@ -207,3 +209,51 @@ int drm_of_encoder_active_endpoint(struct device_node >> *node, >> return -EINVAL; >> } >> EXPORT_SYMBOL_GPL(drm_of_encoder_active_endpoint); >> + >> +/* >> + * drm_of_find_panel_or_bridge - return connected panel or bridge device >> + * @np: device tree node containing encoder input ports >> + * @panel: pointer to hold returned drm_panel >> + * @bridge: pointer to hold returned drm_bridge >> + * >> + * Given a DT node's port and endpoint number, find the connected node and >> + * return either the associated struct drm_panel or drm_bridge device. >> + * >> + * Returns zero if successful, or one of the standard error codes if it >> fails. >> + */ >> +int drm_of_find_panel_or_bridge(const struct device_node *np, >> + int port, int endpoint, >> + struct drm_panel **panel, >> + struct drm_bridge **bridge) >> +{ >> + int ret = -ENODEV; > > Given that you never return 'ret' with -ENODEV can I suggest that you > initialise 'ret' > with -EPROBE_DEFER and then you can get rid of the two assignments further > down? But I can if both panel and bridge are NULL. I'm going with Philipp's suggestion of -EINVAL here. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] drm: convert drivers to use drm_of_find_panel_or_bridge
On Mon, Feb 06, 2017 at 11:03:01AM +0100, Maxime Ripard wrote: > Hi Rob, > > On Fri, Feb 03, 2017 at 09:36:34PM -0600, Rob Herring wrote: > > Similar to the previous commit, convert drivers open coding OF graph > > parsing to use drm_of_find_panel_or_bridge instead. > > > > This changes some error messages to debug messages (in the graph core). > > Graph connections are often "no connects" depending on the particular > > board, so we want to avoid spurious messages. Plus the kernel is not a > > DT validator. > > > > Signed-off-by: Rob Herring> > --- > > [..] > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c > > b/drivers/gpu/drm/sun4i/sun4i_rgb.c > > index f5e86fe7750e..4720725b0fb0 100644 > > --- a/drivers/gpu/drm/sun4i/sun4i_rgb.c > > +++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c > > @@ -15,6 +15,7 @@ > > #include > > #include > > #include > > +#include > > #include > > > > #include "sun4i_drv.h" > > @@ -217,12 +218,10 @@ int sun4i_rgb_init(struct drm_device *drm) > > rgb->drv = drv; > > encoder = >encoder; > > > > - tcon->panel = sun4i_tcon_find_panel(tcon->dev->of_node); > > - encoder->bridge = sun4i_tcon_find_bridge(tcon->dev->of_node); > > - if (IS_ERR(tcon->panel) && IS_ERR(encoder->bridge)) { > > - dev_info(drm->dev, "No panel or bridge found... RGB output > > disabled\n"); > > - return 0; > > - } > > + ret = drm_of_find_panel_or_bridge(tcon->dev->of_node, 1, 0, > > + >panel, >bridge); > > + if (ret) > > + return ret; > > It used to ignore the error if it couldn't find the bridge. This will > break the probe. Well, I got it half right. :) The probe does that, but this needs to too. > > > > drm_encoder_helper_add(>encoder, > >_rgb_enc_helper_funcs); > > @@ -239,7 +238,7 @@ int sun4i_rgb_init(struct drm_device *drm) > > /* The RGB encoder can only work with the TCON channel 0 */ > > rgb->encoder.possible_crtcs = BIT(0); > > > > - if (!IS_ERR(tcon->panel)) { > > + if (tcon->panel) { > > drm_connector_helper_add(>connector, > > _rgb_con_helper_funcs); > > ret = drm_connector_init(drm, >connector, > > @@ -260,7 +259,7 @@ int sun4i_rgb_init(struct drm_device *drm) > > } > > } > > > > - if (!IS_ERR(encoder->bridge)) { > > + if (encoder->bridge) { > > encoder->bridge->encoder = >encoder; > > > > ret = drm_bridge_attach(drm, encoder->bridge); > > @@ -268,8 +267,6 @@ int sun4i_rgb_init(struct drm_device *drm) > > dev_err(drm->dev, "Couldn't attach our bridge\n"); > > goto err_cleanup_connector; > > } > > - } else { > > - encoder->bridge = NULL; > > } > > > > return 0; > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c > > index ea2906f87cb9..2e4e365cecf9 100644 > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > > @@ -15,13 +15,12 @@ > > #include > > #include > > #include > > -#include > > +#include > > > > #include > > #include > > #include > > #include > > -#include > > #include > > #include > > #include > > @@ -405,74 +404,6 @@ static int sun4i_tcon_init_regmap(struct device *dev, > > return 0; > > } > > > > -struct drm_panel *sun4i_tcon_find_panel(struct device_node *node) > > -{ > > - struct device_node *port, *remote, *child; > > - struct device_node *end_node = NULL; > > - > > - /* Inputs are listed first, then outputs */ > > - port = of_graph_get_port_by_id(node, 1); > > - > > - /* > > -* Our first output is the RGB interface where the panel will > > -* be connected. > > -*/ > > - for_each_child_of_node(port, child) { > > - u32 reg; > > - > > - of_property_read_u32(child, "reg", ); > > - if (reg == 0) > > - end_node = child; > > - } > > - > > - if (!end_node) { > > - DRM_DEBUG_DRIVER("Missing panel endpoint\n"); > > - return ERR_PTR(-ENODEV); > > - } > > - > > - remote = of_graph_get_remote_port_parent(end_node); > > - if (!remote) { > > - DRM_DEBUG_DRIVER("Unable to parse remote node\n"); > > - return ERR_PTR(-EINVAL); > > - } > > - > > - return of_drm_find_panel(remote) ?: ERR_PTR(-EPROBE_DEFER); > > And the panel is only one of our endpoints, which is optional, while > other endpoints are mandatory. This means that we might very well have > an endpoint that is not a panel or a bridge. In this case, I think > your function will return an error and will be treated as such, while > it's really the expected behaviour. > > I think it's better to leave this driver alone for now, it's not as > trivial as it looks, and will require some testing to get things > right. I'll try to get my head around how to use your
Re: [PATCH 1/5] of: introduce of_graph_get_remote_node
On Mon, Feb 6, 2017 at 2:50 AM, Daniel Vetterwrote: > On Fri, Feb 03, 2017 at 09:36:31PM -0600, Rob Herring wrote: >> The OF graph API leaves too much of the graph walking to clients when >> in many cases the driver doesn't care about accessing the port or >> endpoint nodes. The drivers typically just want the device connected via >> a particular graph connection. of_graph_get_remote_node provides this >> functionality. >> >> Signed-off-by: Rob Herring > > Just a quick procedural comment: drm-misc for 4.11 is closed already, but > if we can get this core patch into 4.11 still then that would avoid > cross-tree sync pains in 4.12 ... Right, I was thinking the same thing. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] dim: use annotated tags everywhere
Kinda no reason only to use them for drm-intel-next ... Acked-by: Jani NikulaSigned-off-by: Daniel Vetter --- dim | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dim b/dim index 20eac8c58c58..457bf93d8a11 100755 --- a/dim +++ b/dim @@ -1232,9 +1232,9 @@ function dim_pull_request repo="drm-intel" else tag=$branch-$today - $DRY git tag -f $tag $branch@{upstream} + $DRY git tag -f -a $tag $branch@{upstream} $DRY git push -f $remote $tag - prep_pull_mail + prep_pull_mail $tag repo=`branch_to_repo $branch` fi -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/panel: simple: ensure Sharp lq123p1jx31 isn't turned off too soon
Hi, On Mon, Feb 6, 2017 at 4:03 AM, Thierry Redingwrote: > On Thu, Feb 02, 2017 at 03:38:53PM -0800, Douglas Anderson wrote: >> The Sharp lq123p1jx31 has a requirement that the VDD is on for at >> least 300 ms before being turned off. At the moment nothing anywhere >> in the kernel is ensuring this. > > Looks to me like .delay.prepare would be a better fit. That enables the > regulator and setting it to 300 ms would make sure that the regulator > will always get enabled for at least 300 ms. This would also ensure that the 300 ms wasn't violated, right. > Anyway, your commit message says nothing about why this is important. > What are the side-effects if the above isn't true? Does the display hang > in some way? See my own response where I NAKed my own patch because I realized that it wasn't fixing the root cause of the problem I was seeing (it just happened to push timing around). Basically the problem was that the backlight was getting turned on while the panel was off and the panel didn't like that. This appears to already be addressed with upstream patches, so picking those into my tree fixed the problem. It might be nice long term to address some of these "the panel needs to be on for at least 300 ms" or "the panel needs to be off for at least 500 ms" in a way that doesn't require a hardcoded delay. I suppose it could be done today with a custom panel driver, so I guess it depends on how common those types of requirements are in the spec and how important they are to actually follow. -Doug ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Micro-optimise drm_mm_for_each_node_in_range()
On Mon, Feb 06, 2017 at 12:21:48PM +0200, Joonas Lahtinen wrote: > On la, 2017-02-04 at 11:19 +, Chris Wilson wrote: > > As we require valid start/end parameters, we can replace the initial > > potential NULL with a pointer to the drm_mm.head_node and so reduce the > > test on every iteration from a NULL + address comparison to just an > > address comparison. > > > > add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-26 (-26) > > function old new delta > > i915_gem_evict_for_node 719 693 -26 > > > > (No other users outside of the test harness.) > > > > Signed-off-by: Chris Wilson> > Cc: Joonas Lahtinen > > Slightly confused by the mixing of [start, end] and [start, end). > > Reviewed-by: Joonas Lahtinen Applied to drm-misc-next for 4.12. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/7] drm/tinydrm: Add helper functions
On Mon, Feb 06, 2017 at 12:08:47PM +0100, Thierry Reding wrote: > On Mon, Feb 06, 2017 at 11:07:42AM +0100, Daniel Vetter wrote: > > On Mon, Feb 6, 2017 at 10:35 AM, Thierry Reding> > wrote: > > > > > > > > +EXPORT_SYMBOL(tinydrm_disable_backlight); > > > > > > +#endif > > > > > > > > > > These look like they really should be part of the backlight subsystem. > > > I > > > > > don't see anything DRM specific about them. Well, except for the error > > > > > messages. > > > > > > > > So this is a bit an unpopular opinion with some folks, but I don't > > > require > > > > anyone to submit new code to subsystems outside of drm for new drivers. > > > > Simply because it takes months to get stuff landed, and in general it's > > > > not worth the trouble. > > > > > > "Not worth the trouble" is very subjective. If you look at the Linux > > > kernel in general, one of the reasons why it works so well is because > > > the changes we make apply to the kernel as a whole. Yes, sometimes that > > > makes things more difficult and time-consuming, but it also means that > > > the end result will be much more widely usable and therefore benefits > > > everyone else in return. In my opinion that's a large part of why the > > > kernel is so successful. > > > > > > > We have piles of stuff in drm and drm drivers that should be in core but > > > > isn't. > > > > > > > > Imo the only reasonable way is to merge as-is, then follow-up with a > > > patch > > > > series to move the helper into the right subsystem. Most often > > > > unfortunately that follow-up patch series will just die. > > > > > > Of course follow-up series die. That's because nobody cares to follow-up > > > once their code has been merged. > > > > > > Collecting our own helpers or variants of subsystems is a great way of > > > isolating ourselves from the rest of the community. I don't think that's > > > a good solution in the long run at all. > > > > > > > We have a bunch of patch series that we resubmit for months and they go > > exactly nowhere. They don't die because we stop caring, they die because > > they die. Some of them we even need to constantly rebase and carry around > > in drm-tip since our CI would Oops or spew WARNIGs all over the place. > > There's simply some areas of the kernel which seem overloaded under patches > > and no one is willing or able to fix things, and I can't fix the entire > > kernel. Nor expect contributors (who have much less political weight to > > throw around than me) to do that and succeed. And we don't end up with > > worse code in the drm subsystem, since we can still do the refactoring > > within drm helpers and end up with clean drivers. > > > > I fully agree that it's not great for the kernel's future, but when I'm > > stuck with the option to get shit done or burning out playing the > > upstreaming game, the choice is easy. And in the end I care about open > > source gfx much more than the kernel, and I think for open source gfx's > > success it's crucial that we're welcoming to new contributors and don't > > throw up massive roadblocks. Open source gfx is tiny and still far away > > from world domination, we need _lots_ more people. If that means routing > > around other subsystems for them, I'm all for it. > > I can't say I fully agree with that sentiment. I do see how routing > around subsystems can be useful occasionally. If nobody will merge the > code, or if nobody cares, then by all means, let's make them DRM- > specific helpers. > > But I think we need to at least try to do the right thing. If only to > teach people what the right way is. If we start accepting such things > by default, how can we expect contributors to even try? > > I also think that contributors will often end up contributing not only > to DRM but to the kernel as a whole. As such it should be part of our > mentoring to teach them about how the process works as a rule, even if > the occasional exception is necessary to get things done. > > In this particular case, I know for a fact that both backlight and SPI > maintainers are very responsive, so that's not a good excuse. I definitely don't want that we don't attempt this. But brought from years of experience, I recommend to merge first (with pre-refactoring already applied, but helpers only extracted, not yet at the right spot), and then follow up with. Because on average, there's way too many trees with overloaded maintainers who maybe look at your patch once per kernel release cycle. If you know that backlight and spi isn't one of these areas (anything that goes through takashi/sound is a similar good experience for us on the i915 side), then I guess we can try. But then Noralf has already written a few months worth of really great refactoring, and I'm seriously starting to feel guilty for volunteering him for all of this. Even though he seems to be really good at it, and seems to not mind, it's getting a bit silly. Given that I'd say up to Noralf. In
Re: [RFC PATCH xserver] modesetting: re-set the crtc's mode when link-status goes BAD
On 03/02/17 10:04, Daniel Vetter wrote: On Fri, Feb 03, 2017 at 01:30:14AM +0200, Martin Peres wrote: On 01/02/17 22:05, Manasi Navare wrote: On Wed, Feb 01, 2017 at 11:58:16AM -0800, Eric Anholt wrote: Jani Nikulawrites: On Tue, 31 Jan 2017, Eric Anholt wrote: Martin Peres writes: Despite all the careful planing of the kernel, a link may become insufficient to handle the currently-set mode. At this point, the kernel should mark this particular configuration as being broken and potentially prune the mode before setting the offending connector's link-status to BAD and send the userspace a hotplug event. This may happen right after a modeset or later on. When available, we should use the link-status information to reset the wanted mode. Signed-off-by: Martin Peres If I understand this right, there are two failure modes being handled: 1) A mode that won't actually work because the link isn't good enough. 2) A mode that should work, but link parameters were too optimistic and if we just ask the kernel to set the mode again it'll use more conservative parameters that work. This patch seems good for 2). For 1), the drmmode_set_mode_major is going to set our old mode back. Won't the modeset then fail to link train again, and bring us back into this loop? The only escape that I see would be some other userspace responding to the advertised mode list changing, and then asking X to modeset to something new. To avoid that failure busy loop, should we re-fetching modes at this point, and only re-setting if our mode still exists? Disclaimer: I don't know anything about the internals of the modesetting driver. Perhaps we can identify the two cases now, but I'd put this more generally: if the link status has gone bad, it's an indicator to userspace that the circumstances may have changed, and userspace should query the kernel for the list of available modes again. It should no longer trust information obtained prior to getting the bad link status, including the current mode. But specifically, I think you're right, and AFAICT asking for the list of modes again is the only way for the userspace to distinguish between the two cases. I don't think there's a shortcut for deciding the current mode is still valid. To avoid the busy-loop problem, I think I'd like this patch to re-query the kernel to ask about the current mode list, and only try to re-set the mode if our mode is still there. If the mode isn't there, then it's up to the DE to take action in response to the notification of new modes. If you don't have a DE to take appropriate action, you're kind of out of luck. As far as the ABI goes, this seems fine to me. The only concern I had about ABI was having to walk all the connectors on every uevent to see if any had gone bad -- couldn't we have a flag of some sort about what the uevent indicates? But uevents should be super rare, so I'd say the kernel could go ahead with the current plan. Yes I agree. The kernel sets the link status BAD as soona s link training fails but does not prune the modes until a new modelist is requested by the userspace. So this patch should re query the mode list as soon as it sees the link status BAD in order for the kernel to validate the modes again based on new link paarmeters and send a new mode list back. Seems like a bad behaviour from the kernel, isn't it? It should return immediatly if the mode is gonna be pruned :s The mode list pruning isn't relevant for modeesets, the kernel doesn't validate requested modes against that. The mode list is purely for userspace's information. Which means updating it only when userspace asks for it is fine. Hmm, ok. Fair enough! I also thought some more about the loop when we're stuck on BAD, and I think it shouldn't happen: When the link goes BAD we update the link parameter values to the new limits, and the kernel will reject any mode that won't fit anymore. Of course you might be unlucky, and the new link limits are also not working, but eventually you're stuck at the "you're link is broken, sry" stage, where the kernel rejects everything :-) So I think the busy-loop has strictly a limited amount of time until it runs out of steam. OK, I have given it more thoughts and discussed with Ville and Chris IRL or on IRC. My current proposal is based on the idea that the kernel should reject a mode it knows it cannot set. This is already largely tested in real life: Try setting 4K@60Hz on an HDMI 1.x monitor, it should immediately fail on prepare(). For this proposal to work, we would need to put in the ABI that a driver that sets the link-status to BAD should also make sure that whatever the userspace does, no infinite loop should be created (by changing the maximum link characteristics before setting the link-status property). Re-probing the list of modes and checking if the mode is still in there is
Re: [PATCH v3 3/7] drm/tinydrm: Add MIPI DBI support
(Adding Maxime) Den 06.02.2017 13.34, skrev Andrzej Hajda: On 06.02.2017 12:53, Thierry Reding wrote: On Mon, Feb 06, 2017 at 01:30:09PM +0200, Jani Nikula wrote: On Mon, 06 Feb 2017, Thierry Redingwrote: On Tue, Jan 31, 2017 at 05:03:15PM +0100, Noralf Trønnes wrote: Add support for MIPI DBI compatible controllers. Interface type C option 1 and 3 are supported (SPI). Signed-off-by: Noralf Trønnes --- Documentation/gpu/tinydrm.rst | 12 + drivers/gpu/drm/tinydrm/Kconfig|3 + drivers/gpu/drm/tinydrm/Makefile |3 + drivers/gpu/drm/tinydrm/mipi-dbi.c | 1005 include/drm/tinydrm/mipi-dbi.h | 107 5 files changed, 1130 insertions(+) create mode 100644 drivers/gpu/drm/tinydrm/mipi-dbi.c create mode 100644 include/drm/tinydrm/mipi-dbi.h Any reason why this is in the tinydrm subdirectory? Looks like this could be useful to drivers outside of it. I did consider having it outside, but I couldn't find any users in drm that could benefit from it (there is one backlight driver). But now there's Maxime's panel driver. How about something like this: (I have not included the framebuffer dirty function since it will only be used in tinydrm anyway) include/drm/drm_mipi_dbi.h: struct mipi_dbi_device; /** * mipi_dbi_dcs_write - MIPI DCS command with optional parameter(s) * @dbi: MIPI DBI structure * @cmd: Command * @seq...: Optional parameter(s) * * Send MIPI DCS command to the controller. Use mipi_dbi_dcs_read_buffer() for * get/read commands. * * Returns: * Zero on success, negative error code on failure. */ #define mipi_dbi_dcs_write(dbi, cmd, seq...) \ ({ \ const u8 d[] = { seq }; \ BUILD_BUG_ON_MSG(ARRAY_SIZE(d) > 64, "DCS sequence too big for stack");\ mipi_dbi_dcs_write_buffer(dbi, cmd, d, ARRAY_SIZE(d)); \ }) ... drivers/gpu/drm/drm_mipi_dbi.c: struct mipi_dbi_device { struct mutex lock; int (*write)(struct mipi_dbi_device *dbi, u8 cmd, const u8 *par, size_t num); int (*read)(struct mipi_dbi_device *dbi, u8 cmd, u8 *par, size_t num); bool swap16; }; /* MIPI DBI Type C options 1 and 3 */ struct mipi_dbi_spi_device { struct mipi_dbi_device dbi; struct spi_device *spi; struct gpio_desc *dc; }; /* * MIPI DBI Type B - Intel 8080 type parallel bus * I need this to fully convert staging/fbtft to drm */ struct mipi_dbi_i80_device { struct mipi_dbi_device dbi; struct device *dev; struct gpio_desc *cs; struct gpio_desc *dc; struct gpio_desc *wr; struct gpio_descs *db; }; /** * mipi_dbi_get_swap16 - Is byteswapping 16-bit pixel data needed? * @dbi: MIPI DBI device * * Byte swapping 16-bit pixel data is necessary if the bus can't support 16-bit * big endian transfers (e.g. if SPI can only do 8-bit and the machine is * little endian). This applies to the MIPI_DCS_WRITE_MEMORY_START command. * * Returns: * True if it's neccesary to swap bytes, false otherwise. */ bool mipi_dbi_get_swap16(struct mipi_dbi_device *dbi) { return dbi->swap16; } /** * mipi_dbi_spi_init - Initialize MIPI DBI SPI device * @spi: SPI device * @dc: D/C gpio (optional) * @writeonly: True if it's not possible to read from the controller. * * If @dc is set, a Type C Option 3 interface is assumed, if not * Type C Option 1 (9-bit). * * If the SPI master driver doesn't support the necessary bits per word, * the following transformation is used: * * - 9-bit: reorder buffer as 9x 8-bit words, padded with no-op command. * - 16-bit (pixel data): if machine is big endian send as 8-bit, if little * endian the user is responsible for swapping the bytes. * See mipi_dbi_get_swap_pixel_bytes(). * * Returns: * Pointer to _dbi_device on success, ERR_PTR on failure. */ struct mipi_dbi_device *mipi_dbi_spi_init(struct spi_device *spi, struct gpio_desc *dc, bool writeonly) { ... } struct mipi_dbi_device *mipi_dbi_i80_init(...) Noralf. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 99637] VLC video has corrupted colors when using VDPAU output on Radeon SI
https://bugs.freedesktop.org/show_bug.cgi?id=99637 Nayan Deshmukhchanged: What|Removed |Added Status|REOPENED|RESOLVED Resolution|--- |FIXED --- Comment #5 from Nayan Deshmukh --- The patch has landed on 17.0.0-rc3. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 99683] Powerplay features request: manually set GPU (Fiji) frequency beyond default limits and per application
https://bugs.freedesktop.org/show_bug.cgi?id=99683 Alex Deucherchanged: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |WORKSFORME --- Comment #1 from Alex Deucher --- This is already supported. You can echo the percentage (0-20) you want to overclock to the pp_sclk_od and pp_mclk_od files in sysfs. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 98869] Electronic Super Joy graphic artefacts (regression)
https://bugs.freedesktop.org/show_bug.cgi?id=98869 --- Comment #16 from cosiek...@o2.pl --- Hi. I was searching for r300 in 11.2 and 12.0 branch. And it's look like this five patches are in 12 and not in 11.2. It is likely one of them that breaks the game? https://cgit.freedesktop.org/mesa/mesa/commit/?h=12.0=b7da8fa11d5c6ec71113350eed1959191a7d5990 https://cgit.freedesktop.org/mesa/mesa/commit/?h=12.0=84b961dd53a0509a6865d8417301838b34a40096 https://cgit.freedesktop.org/mesa/mesa/commit/?h=12.0=172bfdaa9e80342ade3f023f72d455d76713b866 https://cgit.freedesktop.org/mesa/mesa/commit/?h=12.0=e382bc649b95aa2ab6e86d60b0520236b2bf2947 https://cgit.freedesktop.org/mesa/mesa/commit/?h=12.0=f75a1084349f2332aa080d77acc175ddbe0ab886 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 98869] Electronic Super Joy graphic artefacts (regression)
https://bugs.freedesktop.org/show_bug.cgi?id=98869 cosiek...@o2.pl changed: What|Removed |Added Version|12.0|13.0 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 4/6] drm: scrambling support in drm layer
HDMI 2.0 spec mandates scrambling for modes with pixel clock higher than 340 MHz. This patch adds few new functions in drm layer for core drivers to enable/disable scrambling. This patch adds: - A function to detect scrambling support parsing HF-VSDB - A function to check scrambling status runtime using SCDC read. - Two functions to enable/disable scrambling using SCDC read/write. - Few new bools to reflect scrambling support and status. V2: Addressed review comments from Thierry, Ville and Dhinakaran Thierry: - Mhz -> MHz in comments and commit message. - i2c -> I2C in comments. - Fix the function documentations, keep in sync with drm_scdc_helper.c - drm_connector -> DRM connector. - Create structure for SCDC, and save scrambling status inside that, in a sub-structure. - Call this sub-structure scrambling instead of scr_info. - low_rates -> low_clocks in scrambling status structure. - Store the return value of I2C read/write and print the error code in case of failure. Thierry and Ville: - Move the scrambling enable/disable/query functions in drm_scdc_helper.c file. - Add drm_SCDC prefix for the functions. - Optimize the return statement from function drm_SCDC_check_scrambling_status. Ville: - Dont overwrite saved max TMDS clock value in display_info, if max tmds clock from HF-VSDB is not > 340 MHz. - drm_detect_hdmi_scrambling -> drm_parse_hdmi_forum_vsdb. - Remove dynamic tracking of SCDC status from DRM layer, force bool. - Program clock ratio bit also, while enabling scrambling. Dhinakaran: - Add a comment about *5000 while extracting max clock supported. Signed-off-by: Shashank Sharma--- drivers/gpu/drm/drm_edid.c| 33 - drivers/gpu/drm/drm_scdc_helper.c | 100 ++ include/drm/drm_connector.h | 19 include/drm/drm_edid.h| 6 ++- include/drm/drm_scdc_helper.h | 20 5 files changed, 176 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index a487b80..dc85eb9 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -37,6 +37,7 @@ #include #include #include +#include #define version_greater(edid, maj, min) \ (((edid)->version > (maj)) || \ @@ -3805,13 +3806,43 @@ enum hdmi_quantization_range static void drm_parse_hdmi_forum_vsdb(struct drm_connector *connector, const u8 *hf_vsdb) { - struct drm_hdmi_info *hdmi = >display_info.hdmi; + struct drm_display_info *display = >display_info; + struct drm_hdmi_info *hdmi = >hdmi; if (hf_vsdb[6] & 0x80) { hdmi->scdc.supported = true; if (hf_vsdb[6] & 0x40) hdmi->scdc.read_request = true; } + + /* +* All HDMI 2.0 monitors must support scrambling at rates > 340 MHz. +* And as per the spec, three factors confirm this: +* * Availability of a HF-VSDB block in EDID (check) +* * Non zero Max_TMDS_Char_Rate filed in HF-VSDB (let's check) +* * SCDC support available (let's check) +* Lets check it out. +*/ + + if (hf_vsdb[5]) { + /* max clock is 5000 KHz times block value */ + u32 max_tmds_clock = hf_vsdb[5] * 5000; + struct drm_scdc *scdc = >scdc; + + if (max_tmds_clock > 34) { + display->max_tmds_clock = max_tmds_clock; + DRM_DEBUG_KMS("HF-VSDB: max TMDS clock %d kHz\n", + display->max_tmds_clock); + } + + if (scdc->supported) { + scdc->scrambling.supported = true; + + /* Few sinks support scrambling for cloks < 340M */ + if ((hf_vsdb[6] & 0x8)) + scdc->scrambling.low_rates = true; + } + } } static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, diff --git a/drivers/gpu/drm/drm_scdc_helper.c b/drivers/gpu/drm/drm_scdc_helper.c index fe0e121..311f62e 100644 --- a/drivers/gpu/drm/drm_scdc_helper.c +++ b/drivers/gpu/drm/drm_scdc_helper.c @@ -22,8 +22,10 @@ */ #include +#include #include +#include /** * DOC: scdc helpers @@ -109,3 +111,101 @@ ssize_t drm_scdc_write(struct i2c_adapter *adapter, u8 offset, return err; } EXPORT_SYMBOL(drm_scdc_write); + +/** + * drm_scdc_check_scrambling_status - what is status of scrambling? + * @adapter: I2C adapter for DDC channel + * + * Reads the scrambler status over SCDC, and checks the + * scrambling status. + * + * Returns: + * True if the scrambling is enabled, false otherwise. + */ + +bool drm_scdc_check_scrambling_status(struct i2c_adapter *adapter) +{ + u8 status; + int ret; + + ret = drm_scdc_readb(adapter, SCDC_SCRAMBLER_STATUS, ); + if (ret < 0) { +
[PATCH v2 6/6] drm/i915: allow HDMI 2.0 clock rates
Geminilake has a native HDMI 2.0 controller, which is capable of driving clocks upto 594Mhz. This patch updates the max tmds clock limit for the same. V2: rebase Cc: AnderSigned-off-by: Shashank Sharma --- drivers/gpu/drm/i915/intel_hdmi.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 41d3309..b7277a3 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1210,6 +1210,8 @@ static int intel_hdmi_source_max_tmds_clock(struct drm_i915_private *dev_priv) { if (IS_G4X(dev_priv)) return 165000; + else if (IS_GEMINILAKE(dev_priv)) + return 594000; else if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8) return 30; else -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 5/6] drm/i915: enable scrambling
Geminilake platform sports a native HDMI 2.0 controller, and is capable of driving pixel-clocks upto 594Mhz. HDMI 2.0 spec mendates scrambling for these higher clocks, for reduced RF footprint. This patch checks if the monitor supports scrambling, and if required, enables it during the modeset. V2: Addressed review comments from Ville: - Do not track scrambling status in DRM layer, track somewhere in driver like in intel_crtc_state. - Don't talk to monitor at such a low layer, set monitor scrambling in intel_enable_ddi() before enabling the port. Signed-off-by: Shashank Sharma--- drivers/gpu/drm/i915/i915_reg.h | 2 ++ drivers/gpu/drm/i915/intel_ddi.c | 26 +++ drivers/gpu/drm/i915/intel_drv.h | 11 ++ drivers/gpu/drm/i915/intel_hdmi.c | 70 +++ 4 files changed, 109 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 495b789..cc85892 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7807,6 +7807,8 @@ enum { #define TRANS_DDI_EDP_INPUT_C_ONOFF (6<<12) #define TRANS_DDI_DP_VC_PAYLOAD_ALLOC (1<<8) #define TRANS_DDI_BFI_ENABLE (1<<4) +#define TRANS_DDI_HIGH_TMDS_CHAR_RATE (1<<4) +#define TRANS_DDI_HDMI_SCRAMBLING (1<<0) /* DisplayPort Transport Control */ #define _DP_TP_CTL_A 0x64040 diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 9a9a670..cc7e091 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1278,6 +1278,11 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc) temp |= TRANS_DDI_MODE_SELECT_HDMI; else temp |= TRANS_DDI_MODE_SELECT_DVI; + + if (IS_GEMINILAKE(dev_priv)) + temp = intel_hdmi_handle_source_scrambling( + intel_encoder, + _crtc->config->base.adjusted_mode, temp); } else if (type == INTEL_OUTPUT_ANALOG) { temp |= TRANS_DDI_MODE_SELECT_FDI; temp |= (intel_crtc->config->fdi_lanes - 1) << 1; @@ -1845,6 +1850,21 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder, struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder); + if (IS_GEMINILAKE(dev_priv)) { + /* +* GLK sports a native HDMI 2.0 controller. If required +* clock rate is > 340 Mhz && scrambling is supported +* by monitor, enable scrambling before enabling the +* HDMI 2.0 port. The sink can choose to disable the +* scrambling if it doesn't detect a scrambled within +* 100 ms. +*/ + intel_hdmi_handle_monitor_scrambling(intel_encoder, + conn_state->connector, + intel_crtc->config, + true); + } + /* In HDMI/DVI mode, the port width, and swing/emphasis values * are ignored so nothing special needs to be done besides * enabling the port. @@ -1885,6 +1905,12 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder, intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO); } + if (type == INTEL_OUTPUT_HDMI) { + intel_hdmi_handle_monitor_scrambling(intel_encoder, + old_conn_state->connector, + intel_crtc->config, false); + } + if (type == INTEL_OUTPUT_EDP) { struct intel_dp *intel_dp = enc_to_intel_dp(encoder); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 393f243..300353c 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -681,6 +681,9 @@ struct intel_crtc_state { /* Gamma mode programmed on the pipe */ uint32_t gamma_mode; + + /* HDMI scrambling status (monitor) */ + bool scrambling; }; struct vlv_wm_state { @@ -1588,6 +1591,14 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, bool intel_hdmi_compute_config(struct intel_encoder *encoder, struct intel_crtc_state *pipe_config, struct drm_connector_state *conn_state); +uint32_t +intel_hdmi_handle_source_scrambling(struct intel_encoder *intel_encoder, + struct drm_display_mode *mode, + uint32_t config); +void
[PATCH v2 1/6] drm: Add SCDC helpers
From: Thierry RedingSCDC is a mechanism defined in the HDMI 2.0 specification that allows the source and sink devices to communicate. This commit introduces helpers to access the SCDC and provides the symbolic names for the various registers defined in the specification. Signed-off-by: Thierry Reding Signed-off-by: Shashank Sharma --- Documentation/gpu/drm-kms-helpers.rst | 12 drivers/gpu/drm/Makefile | 3 +- drivers/gpu/drm/drm_scdc_helper.c | 111 include/drm/drm_scdc_helper.h | 132 ++ 4 files changed, 257 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/drm_scdc_helper.c create mode 100644 include/drm/drm_scdc_helper.h diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst index 03040aa..7592756 100644 --- a/Documentation/gpu/drm-kms-helpers.rst +++ b/Documentation/gpu/drm-kms-helpers.rst @@ -217,6 +217,18 @@ EDID Helper Functions Reference .. kernel-doc:: drivers/gpu/drm/drm_edid.c :export: +SCDC Helper Functions Reference +=== + +.. kernel-doc:: drivers/gpu/drm/drm_scdc_helper.c + :doc: scdc helpers + +.. kernel-doc:: include/drm/drm_scdc_helper.h + :internal: + +.. kernel-doc:: drivers/gpu/drm/drm_scdc_helper.c + :export: + Rectangle Utilities Reference = diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 92de399..ad91cd9 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -31,7 +31,8 @@ drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \ drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \ drm_kms_helper_common.o drm_dp_dual_mode_helper.o \ - drm_simple_kms_helper.o drm_modeset_helper.o + drm_simple_kms_helper.o drm_modeset_helper.o \ + drm_scdc_helper.o drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o diff --git a/drivers/gpu/drm/drm_scdc_helper.c b/drivers/gpu/drm/drm_scdc_helper.c new file mode 100644 index 000..fe0e121 --- /dev/null +++ b/drivers/gpu/drm/drm_scdc_helper.c @@ -0,0 +1,111 @@ +/* + * Copyright (c) 2015 NVIDIA Corporation. All rights reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sub license, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the + * next paragraph) shall be included in all copies or substantial portions + * of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + */ + +#include + +#include + +/** + * DOC: scdc helpers + * + * Status and Control Data Channel (SCDC) is a mechanism introduced by the + * HDMI 2.0 specification. It is a point-to-point protocol that allows the + * HDMI source and HDMI sink to exchange data. The same I2C interface that + * is used to access EDID serves as the transport mechanism for SCDC. + */ + +#define SCDC_I2C_SLAVE_ADDRESS 0x54 + +/** + * drm_scdc_read - read a block of data from SCDC + * @adapter: I2C controller + * @offset: start offset of block to read + * @buffer: return location for the block to read + * @size: size of the block to read + * + * Reads a block of data from SCDC, starting at a given offset. + * + * Returns: + * The number of bytes read from SCDC or a negative error code on failure. + */ +ssize_t drm_scdc_read(struct i2c_adapter *adapter, u8 offset, void *buffer, + size_t size) +{ + struct i2c_msg msgs[2] = { + { + .addr = SCDC_I2C_SLAVE_ADDRESS, + .flags = 0, + .len = 1, + .buf = , + }, { + .addr = SCDC_I2C_SLAVE_ADDRESS, + .flags = I2C_M_RD, + .len = size, + .buf = buffer, + } + }; + + return
[PATCH v2 0/6] HDMI 2.0: Scrambling in DRM layer
HDMI 2.0 spec defines a method to reduce the RF footprint while operating at higher pixel clocks, which is called Scrambling. Scrambling can be controlled over a new set of I2C registers which are accessible over existing DDC I2C lines, called SCDC register set. This patch series contains 6 patches: - First two patches add generic drm helper functions to read and write into SCDC registers. These patches are written by Thierry, in a patch series published here: https://patchwork.kernel.org/patch/9459051/ Minor changes were done to map the patches into this series. - Next two patches add functions for scrambling detection and scrambling control. - Next two patches use this infrastructure in DRM layer From I915 driver, to enable scrambling on a GLK deivce which sports a native HDMI 2.0 controller. V2: - addressed review comments from Thierry, Ville and Dhinakaran - added signed-off-by:self in first two patches(Jani N) Shashank Sharma (4): drm/edid: detect SCDC support in HF-VSDB drm: scrambling support in drm layer drm/i915: enable scrambling drm/i915: allow HDMI 2.0 clock rates Thierry Reding (2): drm: Add SCDC helpers drm/edid: check for HF-VSDB block Documentation/gpu/drm-kms-helpers.rst | 12 ++ drivers/gpu/drm/Makefile | 3 +- drivers/gpu/drm/drm_edid.c| 60 ++ drivers/gpu/drm/drm_scdc_helper.c | 211 ++ drivers/gpu/drm/i915/i915_reg.h | 2 + drivers/gpu/drm/i915/intel_ddi.c | 26 + drivers/gpu/drm/i915/intel_drv.h | 11 ++ drivers/gpu/drm/i915/intel_hdmi.c | 72 include/drm/drm_connector.h | 52 + include/drm/drm_edid.h| 6 +- include/drm/drm_scdc_helper.h | 152 include/linux/hdmi.h | 1 + 12 files changed, 606 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/drm_scdc_helper.c create mode 100644 include/drm/drm_scdc_helper.h -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel