[PATCH] drm: Kconfig: Let all DRM_GEM_CMA_HELPER related macros depend on HAVE_DMA_ATTRS
After this fixing building patch, xtensa can pass allmodconfig. - There are still several warnings for it (I sent several patches for them, but not for all). - Xtensa gcc5 cross compiler has issues: it causes more than 10 broken areas with allmodconfig (but no issues with defconfig). I guess, in fact, it is only 1 real issue (which can cause all breaks), and I shall try to fix it within this month. Welcome any ideas, suggestions, or completions. Thanks. On 2/1/15 22:08, Chen Gang S wrote: > DRM_GEM_CMA_HELPER is depend on HAVE_DMA_ATTRS, or it will break the > building. The related error (with allmodconfig under xtensa): > > CC [M] drivers/gpu/drm/drm_gem_cma_helper.o > drivers/gpu/drm/drm_gem_cma_helper.c: In function 'drm_gem_cma_create': > drivers/gpu/drm/drm_gem_cma_helper.c:110:19: error: implicit declaration of > function 'dma_alloc_writecombine' [-Werror=implicit-function-declaration] > cma_obj->vaddr = dma_alloc_writecombine(drm->dev, size, > ^ > drivers/gpu/drm/drm_gem_cma_helper.c:110:17: warning: assignment makes > pointer from integer without a cast [-Wint-conversion] > cma_obj->vaddr = dma_alloc_writecombine(drm->dev, size, >^ > drivers/gpu/drm/drm_gem_cma_helper.c: In function 'drm_gem_cma_free_object': > drivers/gpu/drm/drm_gem_cma_helper.c:193:3: error: implicit declaration of > function 'dma_free_writecombine' [-Werror=implicit-function-declaration] > dma_free_writecombine(gem_obj->dev->dev, cma_obj->base.size, > ^ > drivers/gpu/drm/drm_gem_cma_helper.c: In function 'drm_gem_cma_mmap_obj': > drivers/gpu/drm/drm_gem_cma_helper.c:330:8: error: implicit declaration of > function 'dma_mmap_writecombine' [-Werror=implicit-function-declaration] > ret = dma_mmap_writecombine(cma_obj->base.dev->dev, vma, > ^ > > Signed-off-by: Chen Gang > --- > drivers/gpu/drm/Kconfig | 3 ++- > drivers/gpu/drm/atmel-hlcdc/Kconfig | 2 +- > drivers/gpu/drm/imx/Kconfig | 2 +- > drivers/gpu/drm/rcar-du/Kconfig | 2 +- > drivers/gpu/drm/shmobile/Kconfig| 2 +- > drivers/gpu/drm/sti/Kconfig | 2 +- > drivers/gpu/drm/tilcdc/Kconfig | 2 +- > 7 files changed, 8 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 308c104..151a050 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -62,12 +62,13 @@ config DRM_TTM > > config DRM_GEM_CMA_HELPER > bool > - depends on DRM > + depends on DRM && HAVE_DMA_ATTRS > help > Choose this if you need the GEM CMA helper functions > > config DRM_KMS_CMA_HELPER > bool > + depends on DRM && HAVE_DMA_ATTRS > select DRM_GEM_CMA_HELPER > select DRM_KMS_FB_HELPER > select FB_SYS_FILLRECT > diff --git a/drivers/gpu/drm/atmel-hlcdc/Kconfig > b/drivers/gpu/drm/atmel-hlcdc/Kconfig > index 1a08562..219fc87 100644 > --- a/drivers/gpu/drm/atmel-hlcdc/Kconfig > +++ b/drivers/gpu/drm/atmel-hlcdc/Kconfig > @@ -1,6 +1,6 @@ > config DRM_ATMEL_HLCDC > tristate "DRM Support for ATMEL HLCDC Display Controller" > - depends on DRM && OF && COMMON_CLK && MFD_ATMEL_HLCDC > + depends on DRM && OF && COMMON_CLK && MFD_ATMEL_HLCDC && HAVE_DMA_ATTRS > select DRM_GEM_CMA_HELPER > select DRM_KMS_HELPER > select DRM_KMS_FB_HELPER > diff --git a/drivers/gpu/drm/imx/Kconfig b/drivers/gpu/drm/imx/Kconfig > index 5d5e4092..33cdddf 100644 > --- a/drivers/gpu/drm/imx/Kconfig > +++ b/drivers/gpu/drm/imx/Kconfig > @@ -5,7 +5,7 @@ config DRM_IMX > select VIDEOMODE_HELPERS > select DRM_GEM_CMA_HELPER > select DRM_KMS_CMA_HELPER > - depends on DRM && (ARCH_MXC || ARCH_MULTIPLATFORM) > + depends on DRM && (ARCH_MXC || ARCH_MULTIPLATFORM) && HAVE_DMA_ATTRS > depends on IMX_IPUV3_CORE > help > enable i.MX graphics support > diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig > index 2324a52..11485a4 100644 > --- a/drivers/gpu/drm/rcar-du/Kconfig > +++ b/drivers/gpu/drm/rcar-du/Kconfig > @@ -1,6 +1,6 @@ > config DRM_RCAR_DU > tristate "DRM Support for R-Car Display Unit" > - depends on DRM && ARM > + depends on DRM && ARM && HAVE_DMA_ATTRS > depends on ARCH_SHMOBILE || COMPILE_TEST > select DRM_KMS_HELPER > select DRM_KMS_CMA_HELPER > diff --git a/drivers/gpu/drm/shmobile/Kconfig > b/drivers/gpu/drm/shmobile/Kconfig > index a50fe0e..7321c41 100644 > --- a/drivers/gpu/drm/shmobile/Kconfig > +++ b/drivers/gpu/drm/shmobile/Kconfig > @@ -1,6 +1,6 @@ > config DRM_SHMOBILE > tristate "DRM Support for SH Mobile" > - depends on DRM && ARM > + depends on DRM && ARM && HAVE_DMA_ATTRS > depends on ARCH_SHMOBILE || COMPILE_TEST > select BACKLIGHT_CLASS_DEVICE > select DRM_KMS_HELPER > diff --git a/drivers/gpu/drm/sti/Kconfig b/drivers/gpu/drm/sti/Kconfig > index d6d6b70..1fdb74
[Bug 73528] Deferred lighting in Second Life causes system hiccups and screen flickering
https://bugs.freedesktop.org/show_bug.cgi?id=73528 --- Comment #27 from MirceaKitsune --- I still check this every few days for news. I understand the developers have many issues to deal with, but could someone please commit a simple fix for this problem? It's the only reason why I can't safely use shaders in Second Life, Stuntrally, and potentially other engines... for about two years since the issue was introduced here. I confirmed a line in the code which fixes the problem, as well as the commit which introduced it, thanks to the suggestions in the comments above... so hopefully a solution isn't that difficult now. Any thoughts please? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/31fe5516/attachment.html>
[Bug 88877] Invisible Chrome windows on radeonsi with KWin compositing
https://bugs.freedesktop.org/show_bug.cgi?id=88877 --- Comment #10 from Michel Dänzer --- (In reply to darkbasic from comment #9) > Unfortunately the nine state tracker needs DRI3 support, so I have to use it. > Any ETA for proper present support in radeon? No, but in the meantime the modesetting driver from current xserver has proper DRI3 and Present support. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/6495bbbc/attachment-0001.html>
[Bug 88925] GPU lockup in XCOM with R270X
https://bugs.freedesktop.org/show_bug.cgi?id=88925 --- Comment #2 from Michel Dänzer --- Sounds like bug 80419. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/b81226a9/attachment.html>
[Intel-gfx] [RFC v3 1/4] drm: Add support to find drm_panel by name
On 01/27/2015 03:01 PM, Shobhit Kumar wrote:
> For scenarios where OF is not available, we can use panel identification by
> name.
Any body had a look at this ?
Regards
Shobhit
>
> Signed-off-by: Shobhit Kumar
> ---
> drivers/gpu/drm/drm_panel.c | 18 ++
> include/drm/drm_panel.h | 3 +++
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index 2ef988e..e1cb8cf 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -95,6 +95,24 @@ struct drm_panel *of_drm_find_panel(struct device_node *np)
> EXPORT_SYMBOL(of_drm_find_panel);
> #endif
>
> +struct drm_panel *drm_find_panel_by_name(const char *name)
> +{
> + struct drm_panel *panel;
> +
> + mutex_lock(&panel_lock);
> +
> + list_for_each_entry(panel, &panel_list, list) {
> + if (strcmp(panel->name, name) == 0) {
> + mutex_unlock(&panel_lock);
> + return panel;
> + }
> + }
> +
> + mutex_unlock(&panel_lock);
> + return NULL;
> +}
> +EXPORT_SYMBOL(drm_find_panel_by_name);
> +
> MODULE_AUTHOR("Thierry Reding ");
> MODULE_DESCRIPTION("DRM panel infrastructure");
> MODULE_LICENSE("GPL and additional rights");
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 1fbcc96..1ef9ff3 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -74,6 +74,7 @@ struct drm_panel {
> struct drm_device *drm;
> struct drm_connector *connector;
> struct device *dev;
> + char name[NAME_MAX];
>
> const struct drm_panel_funcs *funcs;
>
> @@ -137,4 +138,6 @@ static inline struct drm_panel *of_drm_find_panel(struct
> device_node *np)
> }
> #endif
>
> +struct drm_panel *drm_find_panel_by_name(const char *name);
> +
> #endif
>
[PATCH -v2] drm/exynos: do not disable hdmi clocks for exynos5420
Hi,
On 02/02/2015 11:26 PM, Gustavo Padovan wrote:
> From: Prathyush K
>
> When VPLL clock of less than 140 MHz was used and all the three clocks -
> hdmiphy, hdmi, sclk_hdmi are disabled, the system hangs during S2R when
> HDMI is connected. Since we want to use a vpll clock of 70.5 MHz, we
> cannot disable these 3 clocks before suspending. This patch add an extra
> clk enable/disable of hdmi and sclk_hdmi outside of the pm ops so these
> clocks are always enabled. Now system suspends and resumes with HDMI
> connected with VPLL set at 70.5 MHz.
>
> This is not the best solution, but it inhibits a crash while we figure out
> what is the correct solution for this issue.
>
> Signed-off-by: Prathyush K
> Signed-off-by: Andrew Bresticker
> Signed-off-by: Gustavo Padovan
> ---
> drivers/gpu/drm/exynos/exynos_hdmi.c | 20
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
> b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 6aa0d65..c6baf64 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -2254,6 +2254,20 @@ static int hdmi_resources_init(struct hdmi_context
> *hdata)
> } else
> res->reg_hdmi_en = NULL;
>
> + /*
> + * For these two clocks exynos5420 fails to suspend if VPLL clock of
> + * 70.5 MHz is used and these clocks are disabled before suspend. So
> + * enable them here.
> + * Note that this will keep the clocks enabled for the entire
> + * lifetime of the driver and waste energy when it is suspended.
> + * However it prevents the system crash until a better solution is
> + * found
> + */
> + if (of_device_is_compatible(dev->of_node, "samsung,exynos5420-hdmi")) {
> + clk_prepare_enable(res->sclk_hdmi);
> + clk_prepare_enable(res->hdmi);
> + }
> +
> return ret;
> fail:
> DRM_ERROR("HDMI resource init - failed\n");
> @@ -2516,6 +2530,12 @@ static int hdmi_remove(struct platform_device *pdev)
> if (hdata->res.reg_hdmi_en)
> regulator_disable(hdata->res.reg_hdmi_en);
>
> + if (of_device_is_compatible(hdata->dev->of_node,
> + "samsung,exynos5420-hdmi")) {
> + clk_disable_unprepare(hdata->res.sclk_hdmi);
> + clk_disable_unprepare(hdata->res.hdmi);
> + }
> +
> if (hdata->hdmiphy_port)
> put_device(&hdata->hdmiphy_port->dev);
> put_device(&hdata->ddc_adpt->dev);
>
Looks fine to me but i'm not sure any there are any side effects to
change parent clock of mout_hdmi clock.
Acked-by: Joonyoung Shim
Thanks.
[Bug 88758] Low FPS in settings on Dota2
https://bugs.freedesktop.org/show_bug.cgi?id=88758 --- Comment #29 from Michel Dänzer --- (In reply to Lorenzo Bona from comment #28) > I've probably messed up my .config. It's surprising though that something in .config could cause this. Can you attach the good and bad .config for comparison? > BTW since yesterday morning git updates, as you can see, I'm facing glitch > (or whatever they are). > > Should I open a new bug or I can edit the obj of this one? Please file a new report. Can you bisect? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/b0a9ff8e/attachment.html>
[Bug 79155] [Tesseract Game] Global Illumination: Medium Causes Color Distortion
https://bugs.freedesktop.org/show_bug.cgi?id=79155 Michel Dänzer changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #8 from Michel Dänzer --- Fixed with current Mesa and LLVM Git. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/36a5114b/attachment.html>
[Bug 78790] Game Tesseract: Crash on shaders and out of registers LLVM errors
https://bugs.freedesktop.org/show_bug.cgi?id=78790 Michel Dänzer changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #3 from Michel Dänzer --- Fixed with current Mesa and LLVM Git. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/f6270517/attachment.html>
[Bug 88758] Low FPS in settings on Dota2
https://bugs.freedesktop.org/show_bug.cgi?id=88758 --- Comment #30 from Lorenzo Bona --- (In reply to Michel Dänzer from comment #29) > (In reply to Lorenzo Bona from comment #28) > > I've probably messed up my .config. > > It's surprising though that something in .config could cause this. Can you > attach the good and bad .config for comparison? > > Yes, here you are the config-3.19.0-rc5-g16653db-dirty (bad one) and the config-3.19.0-rc5+ (good one). > > BTW since yesterday morning git updates, as you can see, I'm facing glitch > > (or whatever they are). > > > > Should I open a new bug or I can edit the obj of this one? > > Please file a new report. Can you bisect? Ok, sure. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/7f13d45e/attachment-0001.html>
[Bug 88758] Low FPS in settings on Dota2
https://bugs.freedesktop.org/show_bug.cgi?id=88758 --- Comment #31 from Lorenzo Bona --- Created attachment 113087 --> https://bugs.freedesktop.org/attachment.cgi?id=113087&action=edit bad_kernel_config -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/b494272f/attachment.html>
[Bug 88758] Low FPS in settings on Dota2
https://bugs.freedesktop.org/show_bug.cgi?id=88758 --- Comment #32 from Lorenzo Bona --- Created attachment 113088 --> https://bugs.freedesktop.org/attachment.cgi?id=113088&action=edit good_kernel_config -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/94a7329b/attachment.html>
[Bug 88658] (bisected) Slow video playback on Kabini
https://bugs.freedesktop.org/show_bug.cgi?id=88658 --- Comment #8 from Michel Dänzer --- (In reply to jtossenb from comment #2) > And the right video URL is: > https://dl-web.dropbox.com/get/hi10p_sample.mp4?w=AAC8qnXfS0xxWbd5Fc2bkvWxMxeqlUiSkL-Dxe5C5BDVyw&dl=1&_subject_uid=67600211 I get error 403 for that. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/c94dcdfb/attachment.html>
[Bug 88758] Low FPS in settings on Dota2
https://bugs.freedesktop.org/show_bug.cgi?id=88758 Michel Dänzer changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |INVALID --- Comment #33 from Michel Dänzer --- I guess it was most likely because CONFIG_X86_PAT was disabled. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/b4b839c0/attachment.html>
[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Mon, Feb 02, 2015 at 09:46:16PM +, Russell King - ARM Linux wrote: > On Mon, Feb 02, 2015 at 03:30:21PM -0500, Rob Clark wrote: > > On Mon, Feb 2, 2015 at 11:54 AM, Daniel Vetter wrote: > > >> My initial thought is for dma-buf to not try to prevent something than > > >> an exporter can actually do.. I think the scenario you describe could > > >> be handled by two sg-lists, if the exporter was clever enough. > > > > > > That's already needed, each attachment has it's own sg-list. After all > > > there's no array of dma_addr_t in the sg tables, so you can't use one sg > > > for more than one mapping. And due to different iommu different devices > > > can easily end up with different addresses. > > > > > > Well, to be fair it may not be explicitly stated, but currently one > > should assume the dma_addr_t's in the dmabuf sglist are bogus. With > > gpu's that implement per-process/context page tables, I'm not really > > sure that there is a sane way to actually do anything else.. > > That's incorrect - and goes dead against the design of scatterlists. > > Not only that, but it is entirely possible that you may get handed > memory via dmabufs for which there are no struct page's associated > with that memory - think about display systems which have their own > video memory which is accessible to the GPU, but it isn't system > memory. > > In those circumstances, you have to use the dma_addr_t's and not the > pages. Yeah exactly. At least with i915 we'd really want to be able to share stolen memory in some cases, and since that's stolen there's no struct pages for them. On top of that any cpu access is also blocked to that range in the memory controller, so the dma_addr_t is really the _only_ thing you can use to get at those memory ranges. And afaik the camera pipe on intel soc can get there - unfortunately that one doesn't have an upstream driver :( And just to clarify: All current dma-buf exporter that I've seen implement the sg mapping correctly and _do_ map the sg table into device address space with dma_map_sg. In other words: The dma_addr_t are all valid, it's just that e.g. with ttm no one has bothered to teach ttm a dma-buf correctly. The internal abstraction is all there, ttm-internal buffer object interface match what dma-buf exposes fairly closes (hey I didn't do shit when designing those interfaces ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Mon, Feb 02, 2015 at 03:30:21PM -0500, Rob Clark wrote: > On Mon, Feb 2, 2015 at 11:54 AM, Daniel Vetter wrote: > >> My initial thought is for dma-buf to not try to prevent something than > >> an exporter can actually do.. I think the scenario you describe could > >> be handled by two sg-lists, if the exporter was clever enough. > > > > That's already needed, each attachment has it's own sg-list. After all > > there's no array of dma_addr_t in the sg tables, so you can't use one sg > > for more than one mapping. And due to different iommu different devices > > can easily end up with different addresses. > > > Well, to be fair it may not be explicitly stated, but currently one > should assume the dma_addr_t's in the dmabuf sglist are bogus. With > gpu's that implement per-process/context page tables, I'm not really > sure that there is a sane way to actually do anything else.. Hm, what does per-process/context page tables have to do here? At least on i915 we have a two levels of page tables: - first level for vm/device isolation, used through dma api - 2nd level for per-gpu-context isolation and context switching, handled internally. Since atm the dma api doesn't have any context of contexts or different pagetables, I don't see who you could use that at all. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Mon, Feb 02, 2015 at 05:36:10PM -0500, Rob Clark wrote: > well, I guess anyways when it comes to sharing buffers, it won't be > the vram placement of the bo that gets shared ;-) Actually that's pretty much what I'd like to be able to do for i915. Except that vram is just a specially protected chunk of main memory. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Bug 88658] (bisected) Slow video playback on Kabini
https://bugs.freedesktop.org/show_bug.cgi?id=88658 --- Comment #9 from jtossenb --- Sorry. I hope it works. https://www.dropbox.com/s/3ronrw57r3n8hsn/hi10p_sample.mp4?dl=1 -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/22596b3e/attachment.html>
[PATCH] drm/modes: Print the mode status in human readable form
On Mon, Feb 02, 2015 at 12:56:26PM -0500, Alex Deucher wrote: > On Mon, Feb 2, 2015 at 12:13 PM, wrote: > > From: Ville Syrjälä > > > > Currently when a mode is rejected the reason is printed as a raw number. > > Having to manually decode that to a enum drm_mode_status value is > > tiresome. Have the code do the decoding instead and print the result > > in a human readable format. > > > > Just having an array of strings indexed with the mode status doesn't > > work since the enum includes negative values. So we offset the status > > by +3 which makes all the indexes non-negative. Also add a bit of > > paranoia into the code to catch out of bounds accesses in case > > someone adds more enum values but forgets to update the code. > > > > Signed-off-by: Ville Syrjälä > > Reviewed-by: Alex Deucher Merged to my drm-misc branch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH v3] dma-buf: cleanup dma_buf_export() to make it easily extensible
On 28/01/15 12:54, Sumit Semwal wrote:
> At present, dma_buf_export() takes a series of parameters, which
> makes it difficult to add any new parameters for exporters, if required.
>
> Make it simpler by moving all these parameters into a struct, and pass
> the struct * as parameter to dma_buf_export().
>
> While at it, unite dma_buf_export_named() with dma_buf_export(), and
> change all callers accordingly.
>
> Signed-off-by: Sumit Semwal
Sorry, a few more comments. Should have sent these before but at least
the are all related only to documentation. Once that is fixed then:
Reviewed-by: Daniel Thompson
> ---
> v3: Daniel Thompson caught the C99 warning issue w/ using {0}; using
> {.exp_name = xxx} instead.
>
> v2: add macro to zero out local struct, and fill KBUILD_MODNAME by default
>
> drivers/dma-buf/dma-buf.c | 47
> +-
> drivers/gpu/drm/armada/armada_gem.c| 10 --
> drivers/gpu/drm/drm_prime.c| 12 ---
> drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 9 +++--
> drivers/gpu/drm/i915/i915_gem_dmabuf.c | 10 --
> drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 9 -
> drivers/gpu/drm/tegra/gem.c| 10 --
> drivers/gpu/drm/ttm/ttm_object.c | 9 +++--
> drivers/gpu/drm/udl/udl_dmabuf.c | 9 -
> drivers/media/v4l2-core/videobuf2-dma-contig.c | 8 -
> drivers/media/v4l2-core/videobuf2-dma-sg.c | 8 -
> drivers/media/v4l2-core/videobuf2-vmalloc.c| 8 -
> drivers/staging/android/ion/ion.c | 9 +++--
> include/linux/dma-buf.h| 34 +++
Documentation/dma-buf-sharing.txt needs updating as a result of these
changes but its not in the diffstat.
> 14 files changed, 142 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 5be225c2ba98..6d3df3dd9310 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -265,7 +265,7 @@ static inline int is_dma_buf_file(struct file *file)
> }
>
> /**
> - * dma_buf_export_named - Creates a new dma_buf, and associates an anon file
> + * dma_buf_export - Creates a new dma_buf, and associates an anon file
> * with this buffer, so it can be exported.
> * Also connect the allocator specific data and ops to the buffer.
> * Additionally, provide a name string for exporter; useful in debugging.
> @@ -277,31 +277,32 @@ static inline int is_dma_buf_file(struct file *file)
> * @exp_name:[in]name of the exporting module - useful for
> debugging.
> * @resv:[in]reservation-object, NULL to allocate default one.
> *
> + * All the above info comes from struct dma_buf_export_info.
> + *
I'm not at all sure about this. Its a novel trick but won't this make
the HTML docs come out looking a bit weird? Is there any prior art for
double-documenting the structure members like this?
Daniel.
[PATCH] drm: Kconfig: Let all DRM_GEM_CMA_HELPER related macros depend on HAVE_DMA_ATTRS
Hi Chen, Thank you for the patch. On Sunday 01 February 2015 22:08:33 Chen Gang S wrote: > DRM_GEM_CMA_HELPER is depend on HAVE_DMA_ATTRS, or it will break the > building. The related error (with allmodconfig under xtensa): > > CC [M] drivers/gpu/drm/drm_gem_cma_helper.o > drivers/gpu/drm/drm_gem_cma_helper.c: In function 'drm_gem_cma_create': > drivers/gpu/drm/drm_gem_cma_helper.c:110:19: error: implicit declaration > of function 'dma_alloc_writecombine' > [-Werror=implicit-function-declaration] cma_obj->vaddr = > dma_alloc_writecombine(drm->dev, size, > ^ > drivers/gpu/drm/drm_gem_cma_helper.c:110:17: warning: assignment makes > pointer from integer without a cast [-Wint-conversion] cma_obj->vaddr = > dma_alloc_writecombine(drm->dev, size, >^ > drivers/gpu/drm/drm_gem_cma_helper.c: In function > 'drm_gem_cma_free_object': drivers/gpu/drm/drm_gem_cma_helper.c:193:3: > error: implicit declaration of function 'dma_free_writecombine' > [-Werror=implicit-function-declaration] > dma_free_writecombine(gem_obj->dev->dev, cma_obj->base.size, > ^ > drivers/gpu/drm/drm_gem_cma_helper.c: In function 'drm_gem_cma_mmap_obj': > drivers/gpu/drm/drm_gem_cma_helper.c:330:8: error: implicit declaration of > function 'dma_mmap_writecombine' [-Werror=implicit-function-declaration] > ret = dma_mmap_writecombine(cma_obj->base.dev->dev, vma, > ^ > > Signed-off-by: Chen Gang > --- > drivers/gpu/drm/Kconfig | 3 ++- > drivers/gpu/drm/atmel-hlcdc/Kconfig | 2 +- > drivers/gpu/drm/imx/Kconfig | 2 +- > drivers/gpu/drm/rcar-du/Kconfig | 2 +- > drivers/gpu/drm/shmobile/Kconfig| 2 +- > drivers/gpu/drm/sti/Kconfig | 2 +- > drivers/gpu/drm/tilcdc/Kconfig | 2 +- > 7 files changed, 8 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 308c104..151a050 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -62,12 +62,13 @@ config DRM_TTM > > config DRM_GEM_CMA_HELPER > bool > - depends on DRM > + depends on DRM && HAVE_DMA_ATTRS > help > Choose this if you need the GEM CMA helper functions > > config DRM_KMS_CMA_HELPER > bool > + depends on DRM && HAVE_DMA_ATTRS > select DRM_GEM_CMA_HELPER > select DRM_KMS_FB_HELPER > select FB_SYS_FILLRECT > diff --git a/drivers/gpu/drm/atmel-hlcdc/Kconfig > b/drivers/gpu/drm/atmel-hlcdc/Kconfig index 1a08562..219fc87 100644 > --- a/drivers/gpu/drm/atmel-hlcdc/Kconfig > +++ b/drivers/gpu/drm/atmel-hlcdc/Kconfig > @@ -1,6 +1,6 @@ > config DRM_ATMEL_HLCDC > tristate "DRM Support for ATMEL HLCDC Display Controller" > - depends on DRM && OF && COMMON_CLK && MFD_ATMEL_HLCDC > + depends on DRM && OF && COMMON_CLK && MFD_ATMEL_HLCDC && HAVE_DMA_ATTRS I don't like having to add the explicit dependency to all users of DRM_GEM_CMA_HELPER and DRM_KMS_CMA_HELPER, but there's no way around this given the select vs. depends issues in Kconfig. A better solution in the longer term would be to implement HAVE_DMA_ATTRS support for xtensa (and all the other architectures that miss it), but this patch looks fine to me as an interim solution. Acked-by: Laurent Pinchart > select DRM_GEM_CMA_HELPER > select DRM_KMS_HELPER > select DRM_KMS_FB_HELPER > diff --git a/drivers/gpu/drm/imx/Kconfig b/drivers/gpu/drm/imx/Kconfig > index 5d5e4092..33cdddf 100644 > --- a/drivers/gpu/drm/imx/Kconfig > +++ b/drivers/gpu/drm/imx/Kconfig > @@ -5,7 +5,7 @@ config DRM_IMX > select VIDEOMODE_HELPERS > select DRM_GEM_CMA_HELPER > select DRM_KMS_CMA_HELPER > - depends on DRM && (ARCH_MXC || ARCH_MULTIPLATFORM) > + depends on DRM && (ARCH_MXC || ARCH_MULTIPLATFORM) && HAVE_DMA_ATTRS > depends on IMX_IPUV3_CORE > help > enable i.MX graphics support > diff --git a/drivers/gpu/drm/rcar-du/Kconfig > b/drivers/gpu/drm/rcar-du/Kconfig index 2324a52..11485a4 100644 > --- a/drivers/gpu/drm/rcar-du/Kconfig > +++ b/drivers/gpu/drm/rcar-du/Kconfig > @@ -1,6 +1,6 @@ > config DRM_RCAR_DU > tristate "DRM Support for R-Car Display Unit" > - depends on DRM && ARM > + depends on DRM && ARM && HAVE_DMA_ATTRS > depends on ARCH_SHMOBILE || COMPILE_TEST > select DRM_KMS_HELPER > select DRM_KMS_CMA_HELPER > diff --git a/drivers/gpu/drm/shmobile/Kconfig > b/drivers/gpu/drm/shmobile/Kconfig index a50fe0e..7321c41 100644 > --- a/drivers/gpu/drm/shmobile/Kconfig > +++ b/drivers/gpu/drm/shmobile/Kconfig > @@ -1,6 +1,6 @@ > config DRM_SHMOBILE > tristate "DRM Support for SH Mobile" > - depends on DRM && ARM > + depends on DRM && ARM && HAVE_DMA_ATTRS > depends on ARCH_SHMOBILE || COMPILE_TEST > select BACKLIGHT_CLASS_DEVICE > select DRM_KMS_HELPER > diff --git a/drivers/gpu/drm/sti/Kconfig b/drivers/gpu/drm/sti/Kconfig > index d6d6b70..1fdb74b 100644 > --- a/drivers/gpu/
[PATCH 1/4] drm/irq: Add drm_crtc_vblank_reset
At driver load we need to tell the vblank code about the state of the
pipes, so that the logic around reject vblank_get when the pipe is off
works correctly.
Thus far i915 used drm_vblank_off, but one of the side-effects of it
is that it also saves the vblank counter. And for that it calls down
into the ->get_vblank_counter hook. Which isn't really a good idea
when the pipe is off for a few reasons:
- With runtime pm the register might not respond.
- If the pipe is off some datastructures might not be around or
unitialized.
The later is what blew up on gen3: We look at intel_crtc->config to
compute the vblank counter, and for a disabled pipe at boot-up that's
just not there. Thus far this was papered over by a check for
intel_crtc->active, but I want to get rid of that (since it's fairly
race, vblank hooks are called from all kinds of places).
So prep for that by adding a _reset functions which only does what we
really need to be done at driver load: Mark the vblank pipe as off,
but don't do any vblank counter saving or event flushing - neither of
that is required.
Cc: Laurent Pinchart
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_irq.c| 32
drivers/gpu/drm/i915/intel_display.c | 4 ++--
include/drm/drmP.h | 1 +
3 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 75647e7f012b..1e5fb1b994d7 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1226,6 +1226,38 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc)
EXPORT_SYMBOL(drm_crtc_vblank_off);
/**
+ * drm_crtc_vblank_reset - reset vblank state to off on a CRTC
+ * @crtc: CRTC in question
+ *
+ * Drivers can use this function to reset the vblank state to off at load time.
+ * Drivers should use this together with the drm_crtc_vblank_off() and
+ * drm_crtc_vblank_on() functions. The diffrence comparet to
+ * drm_crtc_vblank_off() is that this function doesn't save the vblank counter
+ * and hence doesn't need to call any driver hooks.
+ */
+void drm_crtc_vblank_reset(struct drm_crtc *drm_crtc)
+{
+ struct drm_device *dev = drm_crtc->dev;
+ unsigned long irqflags;
+ int crtc = drm_crtc_index(drm_crtc);
+ struct drm_vblank_crtc *vblank = &dev->vblank[crtc];
+
+ spin_lock_irqsave(&dev->vbl_lock, irqflags);
+ /*
+* Prevent subsequent drm_vblank_get() from enabling the vblank
+* interrupt by bumping the refcount.
+*/
+ if (!vblank->inmodeset) {
+ atomic_inc(&vblank->refcount);
+ vblank->inmodeset = 1;
+ }
+ spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
+
+ WARN_ON(!list_empty(&dev->vblank_event_list));
+}
+EXPORT_SYMBOL(drm_crtc_vblank_reset);
+
+/**
* drm_vblank_on - enable vblank events on a CRTC
* @dev: DRM device
* @crtc: CRTC in question
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 423ef959264d..f8871a184747 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13296,9 +13296,9 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
/* restore vblank interrupts to correct state */
if (crtc->active) {
update_scanline_offset(crtc);
- drm_vblank_on(dev, crtc->pipe);
+ drm_crtc_vblank_on(&crtc->base);
} else
- drm_vblank_off(dev, crtc->pipe);
+ drm_crtc_vblank_reset(&crtc->base);
/* We need to sanitize the plane -> pipe mapping first because this will
* disable the crtc (and hence change the state) if it is wrong. Note
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index e928625a9da0..54c6ea1e5866 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -922,6 +922,7 @@ extern void drm_crtc_wait_one_vblank(struct drm_crtc *crtc);
extern void drm_vblank_off(struct drm_device *dev, int crtc);
extern void drm_vblank_on(struct drm_device *dev, int crtc);
extern void drm_crtc_vblank_off(struct drm_crtc *crtc);
+extern void drm_crtc_vblank_reset(struct drm_crtc *crtc);
extern void drm_crtc_vblank_on(struct drm_crtc *crtc);
extern void drm_vblank_cleanup(struct drm_device *dev);
--
1.9.3
[PATCH 2/4] drm/i915: Drop pipe_enable checks in vblank funcs
With Ville's rework to use drm_crtc_vblank_on/off the core will take
care of rejecting drm_vblank_get calls when the pipe is off. Also the
core won't call the get_vblank_counter hooks in that case either. And
since we've dropped ums support recently we can now remove these
hacks, yay!
Noticed while trying to answer questions Laurent had about how the new
atomic helpers work.
Cc: Laurent Pinchart
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_irq.c | 52 -
1 file changed, 52 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 23bfe2232b6a..80f35dcffea4 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -492,31 +492,6 @@ static void i915_enable_asle_pipestat(struct drm_device
*dev)
spin_unlock_irq(&dev_priv->irq_lock);
}
-/**
- * i915_pipe_enabled - check if a pipe is enabled
- * @dev: DRM device
- * @pipe: pipe to check
- *
- * Reading certain registers when the pipe is disabled can hang the chip.
- * Use this routine to make sure the PLL is running and the pipe is active
- * before reading such registers if unsure.
- */
-static int
-i915_pipe_enabled(struct drm_device *dev, int pipe)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
-
- if (drm_core_check_feature(dev, DRIVER_MODESET)) {
- /* Locking is horribly broken here, but whatever. */
- struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
- return intel_crtc->active;
- } else {
- return I915_READ(PIPECONF(pipe)) & PIPECONF_ENABLE;
- }
-}
-
/*
* This timing diagram depicts the video signal in and
* around the vertical blanking period.
@@ -583,12 +558,6 @@ static u32 i915_get_vblank_counter(struct drm_device *dev,
int pipe)
unsigned long low_frame;
u32 high1, high2, low, pixel, vbl_start, hsync_start, htotal;
- if (!i915_pipe_enabled(dev, pipe)) {
- DRM_DEBUG_DRIVER("trying to get vblank count for disabled "
- "pipe %c\n", pipe_name(pipe));
- return 0;
- }
-
if (drm_core_check_feature(dev, DRIVER_MODESET)) {
struct intel_crtc *intel_crtc =
to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
@@ -648,12 +617,6 @@ static u32 gm45_get_vblank_counter(struct drm_device *dev,
int pipe)
struct drm_i915_private *dev_priv = dev->dev_private;
int reg = PIPE_FRMCOUNT_GM45(pipe);
- if (!i915_pipe_enabled(dev, pipe)) {
- DRM_DEBUG_DRIVER("trying to get vblank count for disabled "
-"pipe %c\n", pipe_name(pipe));
- return 0;
- }
-
return I915_READ(reg);
}
@@ -2660,9 +2623,6 @@ static int i915_enable_vblank(struct drm_device *dev, int
pipe)
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long irqflags;
- if (!i915_pipe_enabled(dev, pipe))
- return -EINVAL;
-
spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
if (INTEL_INFO(dev)->gen >= 4)
i915_enable_pipestat(dev_priv, pipe,
@@ -2682,9 +2642,6 @@ static int ironlake_enable_vblank(struct drm_device *dev,
int pipe)
uint32_t bit = (INTEL_INFO(dev)->gen >= 7) ? DE_PIPE_VBLANK_IVB(pipe) :
DE_PIPE_VBLANK(pipe);
- if (!i915_pipe_enabled(dev, pipe))
- return -EINVAL;
-
spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
ironlake_enable_display_irq(dev_priv, bit);
spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
@@ -2697,9 +2654,6 @@ static int valleyview_enable_vblank(struct drm_device
*dev, int pipe)
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long irqflags;
- if (!i915_pipe_enabled(dev, pipe))
- return -EINVAL;
-
spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
i915_enable_pipestat(dev_priv, pipe,
PIPE_START_VBLANK_INTERRUPT_STATUS);
@@ -2713,9 +2667,6 @@ static int gen8_enable_vblank(struct drm_device *dev, int
pipe)
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long irqflags;
- if (!i915_pipe_enabled(dev, pipe))
- return -EINVAL;
-
spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
dev_priv->de_irq_mask[pipe] &= ~GEN8_PIPE_VBLANK;
I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]);
@@ -2767,9 +2718,6 @@ static void gen8_disable_vblank(struct drm_device *dev,
int pipe)
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long irqflags;
- if (!i915_pipe_enabled(dev, pipe))
- return;
-
spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
[PATCH 3/4] drm/i915: Flatten DRIVER_MODESET checks in i915_irq.c
UMS is no more!
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_irq.c | 36 +++-
1 file changed, 11 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 80f35dcffea4..37189a25ca82 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -557,28 +557,16 @@ static u32 i915_get_vblank_counter(struct drm_device
*dev, int pipe)
unsigned long high_frame;
unsigned long low_frame;
u32 high1, high2, low, pixel, vbl_start, hsync_start, htotal;
+ struct intel_crtc *intel_crtc =
+ to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
+ const struct drm_display_mode *mode =
+ &intel_crtc->config->base.adjusted_mode;
- if (drm_core_check_feature(dev, DRIVER_MODESET)) {
- struct intel_crtc *intel_crtc =
- to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
- const struct drm_display_mode *mode =
- &intel_crtc->config->base.adjusted_mode;
-
- htotal = mode->crtc_htotal;
- hsync_start = mode->crtc_hsync_start;
- vbl_start = mode->crtc_vblank_start;
- if (mode->flags & DRM_MODE_FLAG_INTERLACE)
- vbl_start = DIV_ROUND_UP(vbl_start, 2);
- } else {
- enum transcoder cpu_transcoder = (enum transcoder) pipe;
-
- htotal = ((I915_READ(HTOTAL(cpu_transcoder)) >> 16) & 0x1fff) +
1;
- hsync_start = (I915_READ(HSYNC(cpu_transcoder)) & 0x1fff) + 1;
- vbl_start = (I915_READ(VBLANK(cpu_transcoder)) & 0x1fff) + 1;
- if ((I915_READ(PIPECONF(cpu_transcoder)) &
-PIPECONF_INTERLACE_MASK) != PIPECONF_PROGRESSIVE)
- vbl_start = DIV_ROUND_UP(vbl_start, 2);
- }
+ htotal = mode->crtc_htotal;
+ hsync_start = mode->crtc_hsync_start;
+ vbl_start = mode->crtc_vblank_start;
+ if (mode->flags & DRM_MODE_FLAG_INTERLACE)
+ vbl_start = DIV_ROUND_UP(vbl_start, 2);
/* Convert to pixel count */
vbl_start *= htotal;
@@ -4330,10 +4318,8 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
if (!IS_GEN2(dev_priv))
dev->vblank_disable_immediate = true;
- if (drm_core_check_feature(dev, DRIVER_MODESET)) {
- dev->driver->get_vblank_timestamp = i915_get_vblank_timestamp;
- dev->driver->get_scanout_position = i915_get_crtc_scanoutpos;
- }
+ dev->driver->get_vblank_timestamp = i915_get_vblank_timestamp;
+ dev->driver->get_scanout_position = i915_get_crtc_scanoutpos;
if (IS_CHERRYVIEW(dev_priv)) {
dev->driver->irq_handler = cherryview_irq_handler;
--
1.9.3
[PATCH 4/4] drm/i915: Switch to drm_crtc variants of vblank functions
Where possible right now. Just a small step towards nirvana ...
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
drivers/gpu/drm/i915/intel_display.c | 9 +
drivers/gpu/drm/i915/intel_sprite.c | 4 ++--
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 3b332a493674..479ea0825166 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -580,7 +580,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void
*data)
seq_printf(m, "Flip queued on frame %d, (was ready on
frame %d), now %d\n",
work->flip_queued_vblank,
work->flip_ready_vblank,
- drm_vblank_count(dev, crtc->pipe));
+ drm_crtc_vblank_count(dev, &crtc->base));
if (work->enable_stall_check)
seq_puts(m, "Stall check enabled, ");
else
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index f8871a184747..2c33ef5c5f68 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9674,10 +9674,10 @@ static bool __intel_pageflip_stall_check(struct
drm_device *dev,
!i915_gem_request_completed(work->flip_queued_req, true))
return false;
- work->flip_ready_vblank = drm_vblank_count(dev,
intel_crtc->pipe);
+ work->flip_ready_vblank = drm_crtc_vblank_count(dev, crtc);
}
- if (drm_vblank_count(dev, intel_crtc->pipe) - work->flip_ready_vblank <
3)
+ if (drm_crtc_vblank_count(dev, crtc) - work->flip_ready_vblank < 3)
return false;
/* Potential stall - if we see that the flip has happened,
@@ -9708,7 +9708,8 @@ void intel_check_page_flip(struct drm_device *dev, int
pipe)
spin_lock(&dev->event_lock);
if (intel_crtc->unpin_work && __intel_pageflip_stall_check(dev, crtc)) {
WARN_ONCE(1, "Kicking stuck page flip: queued at %d, now %d\n",
-intel_crtc->unpin_work->flip_queued_vblank,
drm_vblank_count(dev, pipe));
+intel_crtc->unpin_work->flip_queued_vblank,
+drm_vblank_count(dev, pipe));
page_flip_completed(intel_crtc);
}
spin_unlock(&dev->event_lock);
@@ -9849,7 +9850,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
intel_ring_get_request(ring));
}
- work->flip_queued_vblank = drm_vblank_count(dev, intel_crtc->pipe);
+ work->flip_queued_vblank = drm_crtc_vblank_count(dev, crtc);
work->enable_stall_check = true;
i915_gem_track_fb(work->old_fb_obj, obj,
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 0a52c44ad03d..0f499adfe2e0 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -98,7 +98,7 @@ bool intel_pipe_update_start(struct intel_crtc *crtc,
uint32_t *start_vbl_count)
if (min <= 0 || max <= 0)
return false;
- if (WARN_ON(drm_vblank_get(dev, pipe)))
+ if (WARN_ON(drm_crtc_vblank_get(dev, &crtc->base)))
return false;
local_irq_disable();
@@ -132,7 +132,7 @@ bool intel_pipe_update_start(struct intel_crtc *crtc,
uint32_t *start_vbl_count)
finish_wait(wq, &wait);
- drm_vblank_put(dev, pipe);
+ drm_crtc_vblank_put(dev, &crtc->base);
*start_vbl_count = dev->driver->get_vblank_counter(dev, pipe);
--
1.9.3
[PATCH 1/4] drm/irq: Add drm_crtc_vblank_reset
Hi Daniel,
Thank you for the patch.
On Tuesday 03 February 2015 11:30:11 Daniel Vetter wrote:
> At driver load we need to tell the vblank code about the state of the
> pipes, so that the logic around reject vblank_get when the pipe is off
> works correctly.
>
> Thus far i915 used drm_vblank_off, but one of the side-effects of it
> is that it also saves the vblank counter. And for that it calls down
> into the ->get_vblank_counter hook. Which isn't really a good idea
> when the pipe is off for a few reasons:
> - With runtime pm the register might not respond.
> - If the pipe is off some datastructures might not be around or
> unitialized.
>
> The later is what blew up on gen3: We look at intel_crtc->config to
> compute the vblank counter, and for a disabled pipe at boot-up that's
> just not there. Thus far this was papered over by a check for
> intel_crtc->active, but I want to get rid of that (since it's fairly
> race, vblank hooks are called from all kinds of places).
>
> So prep for that by adding a _reset functions which only does what we
> really need to be done at driver load: Mark the vblank pipe as off,
> but don't do any vblank counter saving or event flushing - neither of
> that is required.
(Thinking out loud)
In principle this looks good, but I find the naming pretty confusing. The
drm_crtc_vblank_* functions now have get, put, on, off and reset variants. The
fact that on is supposed to be called both at runtime and an init time while
off is replaced by reset at init time breaks the symmetry between on and off.
What would you think of a drm_crtc_vblank_init() function instead, used at
init time only, and that would take an enabled boolean argument ?
On the other hand, calling drm_crtc_vblank_on() at init time also makes sense,
as even if the CRTC is active the vblank interrupt should be off then, and an
explicit call to the function means "turn the vblank interrupt on". I'm thus
not totally opposed to keeping that as-is. Wouldn't it then be better to
modify the core to default to off, and let drivers call drm_crtc_vblank_on()
explicitly if the default isn't correct ? I think I like this solution better,
and it could be conditioned by a driver flag if we don't want to audit all
drivers for possible breakages.
> Cc: Laurent Pinchart
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_irq.c| 32
> drivers/gpu/drm/i915/intel_display.c | 4 ++--
> include/drm/drmP.h | 1 +
> 3 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 75647e7f012b..1e5fb1b994d7 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -1226,6 +1226,38 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc)
> EXPORT_SYMBOL(drm_crtc_vblank_off);
>
> /**
> + * drm_crtc_vblank_reset - reset vblank state to off on a CRTC
> + * @crtc: CRTC in question
> + *
> + * Drivers can use this function to reset the vblank state to off at load
> time.
> + * Drivers should use this together with the drm_crtc_vblank_off() and
> + * drm_crtc_vblank_on() functions. The diffrence comparet to
Typo: difference compared to
> + * drm_crtc_vblank_off() is that this function doesn't save the vblank
> counter
> + * and hence doesn't need to call any driver hooks.
> + */
> +void drm_crtc_vblank_reset(struct drm_crtc *drm_crtc)
> +{
> + struct drm_device *dev = drm_crtc->dev;
> + unsigned long irqflags;
> + int crtc = drm_crtc_index(drm_crtc);
> + struct drm_vblank_crtc *vblank = &dev->vblank[crtc];
> +
> + spin_lock_irqsave(&dev->vbl_lock, irqflags);
> + /*
> + * Prevent subsequent drm_vblank_get() from enabling the vblank
> + * interrupt by bumping the refcount.
> + */
> + if (!vblank->inmodeset) {
> + atomic_inc(&vblank->refcount);
> + vblank->inmodeset = 1;
> + }
> + spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
> +
> + WARN_ON(!list_empty(&dev->vblank_event_list));
> +}
> +EXPORT_SYMBOL(drm_crtc_vblank_reset);
> +
> +/**
> * drm_vblank_on - enable vblank events on a CRTC
> * @dev: DRM device
> * @crtc: CRTC in question
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c index 423ef959264d..f8871a184747
> 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13296,9 +13296,9 @@ static void intel_sanitize_crtc(struct intel_crtc
> *crtc) /* restore vblank interrupts to correct state */
> if (crtc->active) {
> update_scanline_offset(crtc);
> - drm_vblank_on(dev, crtc->pipe);
> + drm_crtc_vblank_on(&crtc->base);
> } else
> - drm_vblank_off(dev, crtc->pipe);
> + drm_crtc_vblank_reset(&crtc->base);
>
> /* We need to sanitize the plane -> pipe mapping first because this will
>* disable the crtc (and he
[Bug 88925] GPU lockup in XCOM with R270X
https://bugs.freedesktop.org/show_bug.cgi?id=88925 --- Comment #3 from dex+fdobugzilla at dragonslave.de --- (In reply to Michel Dänzer from comment #2) > Sounds like bug 80419. Possible. But the game worked fine until about start of the year. I think I may still have the packaged versions of llvm and mesa from then and perhaps I can confirm this. I strongly suspect LLVM being the culprit here. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/7fbb2968/attachment-0001.html>
[PULL] drm-amdkfd-next-fixes
Hi Dave, Two more fixes for 3.20 amdkfd code: - Fixing accounting of active queues - Preserving a register internal state Thanks, Oded The following changes since commit e4bf44b3b558742fb7c58b4d34e206c8942f07e6: drm/modes: Print the mode status in human readable form (2015-02-03 11:13:27 +1000) are available in the git repository at: git://people.freedesktop.org/~gabbayo/linux tags/drm-amdkfd-next-fixes-2015-02-03 for you to fetch changes up to d752f95e55ed36648fc2bcbca348f5113920c357: drm/amdkfd: Preserve CP_MQD_IQ_RPTR internal state (2015-01-19 11:47:34 -0600) Jay Cornwall (2): drm/amdkfd: Fix dqm->queue_count tracking drm/amdkfd: Preserve CP_MQD_IQ_RPTR internal state drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 9 ++--- drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c | 4 +++- 2 files changed, 9 insertions(+), 4 deletions(-)
[PATCH 1/4] drm/irq: Add drm_crtc_vblank_reset
On Tue, Feb 03, 2015 at 01:31:34PM +0200, Laurent Pinchart wrote:
> Hi Daniel,
>
> Thank you for the patch.
>
> On Tuesday 03 February 2015 11:30:11 Daniel Vetter wrote:
> > At driver load we need to tell the vblank code about the state of the
> > pipes, so that the logic around reject vblank_get when the pipe is off
> > works correctly.
> >
> > Thus far i915 used drm_vblank_off, but one of the side-effects of it
> > is that it also saves the vblank counter. And for that it calls down
> > into the ->get_vblank_counter hook. Which isn't really a good idea
> > when the pipe is off for a few reasons:
> > - With runtime pm the register might not respond.
> > - If the pipe is off some datastructures might not be around or
> > unitialized.
> >
> > The later is what blew up on gen3: We look at intel_crtc->config to
> > compute the vblank counter, and for a disabled pipe at boot-up that's
> > just not there. Thus far this was papered over by a check for
> > intel_crtc->active, but I want to get rid of that (since it's fairly
> > race, vblank hooks are called from all kinds of places).
> >
> > So prep for that by adding a _reset functions which only does what we
> > really need to be done at driver load: Mark the vblank pipe as off,
> > but don't do any vblank counter saving or event flushing - neither of
> > that is required.
>
> (Thinking out loud)
>
> In principle this looks good, but I find the naming pretty confusing. The
> drm_crtc_vblank_* functions now have get, put, on, off and reset variants.
> The
> fact that on is supposed to be called both at runtime and an init time while
> off is replaced by reset at init time breaks the symmetry between on and off.
> What would you think of a drm_crtc_vblank_init() function instead, used at
> init time only, and that would take an enabled boolean argument ?
>
> On the other hand, calling drm_crtc_vblank_on() at init time also makes
> sense,
> as even if the CRTC is active the vblank interrupt should be off then, and an
> explicit call to the function means "turn the vblank interrupt on". I'm thus
> not totally opposed to keeping that as-is. Wouldn't it then be better to
> modify the core to default to off, and let drivers call drm_crtc_vblank_on()
> explicitly if the default isn't correct ? I think I like this solution
> better,
> and it could be conditioned by a driver flag if we don't want to audit all
> drivers for possible breakages.
Yeah, that's been my thinking with sticking with on and only replacing the
off: For _on the environment exactly matches what we do when enabling the
crtc in a modeset:
- the pipe is actually running
- we want vblanks to start working
Totally different for _off, which assumes:
- pipe is still on
- vblanks should stop running and state should be saved
- any pending
Wheras _reset just disallows vblanks.
Long-term I wonder whether a drm_crtc_vblank_init would be useful, which:
- uses dev->mode_config.num_crtcs to set up the right amount of vblank
pipes.
- calls _reset directly
- in the future might even store the vblank state in the drm_crtc.
But before we can do that we need to split the vblank code into ums legacy
paths using int pipe and kms paths dealing in struct drm_crtc *. We're not
there yet.
>
> > Cc: Laurent Pinchart
> > Signed-off-by: Daniel Vetter
> > ---
> > drivers/gpu/drm/drm_irq.c| 32
> > drivers/gpu/drm/i915/intel_display.c | 4 ++--
> > include/drm/drmP.h | 1 +
> > 3 files changed, 35 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> > index 75647e7f012b..1e5fb1b994d7 100644
> > --- a/drivers/gpu/drm/drm_irq.c
> > +++ b/drivers/gpu/drm/drm_irq.c
> > @@ -1226,6 +1226,38 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc)
> > EXPORT_SYMBOL(drm_crtc_vblank_off);
> >
> > /**
> > + * drm_crtc_vblank_reset - reset vblank state to off on a CRTC
> > + * @crtc: CRTC in question
> > + *
> > + * Drivers can use this function to reset the vblank state to off at load
> > time.
> > + * Drivers should use this together with the drm_crtc_vblank_off() and
> > + * drm_crtc_vblank_on() functions. The diffrence comparet to
>
> Typo: difference compared to
>
> > + * drm_crtc_vblank_off() is that this function doesn't save the vblank
> > counter
> > + * and hence doesn't need to call any driver hooks.
> > + */
> > +void drm_crtc_vblank_reset(struct drm_crtc *drm_crtc)
> > +{
> > + struct drm_device *dev = drm_crtc->dev;
> > + unsigned long irqflags;
> > + int crtc = drm_crtc_index(drm_crtc);
> > + struct drm_vblank_crtc *vblank = &dev->vblank[crtc];
> > +
> > + spin_lock_irqsave(&dev->vbl_lock, irqflags);
> > + /*
> > +* Prevent subsequent drm_vblank_get() from enabling the vblank
> > +* interrupt by bumping the refcount.
> > +*/
> > + if (!vblank->inmodeset) {
> > + atomic_inc(&vblank->refcount);
> > + vblank->inmodeset = 1;
> > +
[PATCH 03/14] drm/bridge: make bridge registration independent of drm flow
On Fri, Jan 30, 2015 at 10:37:19AM -0500, Rob Clark wrote: > On Tue, Jan 20, 2015 at 11:38 AM, Ajay Kumar > wrote: > > Currently, third party bridge drivers(ptn3460) are dependent > > on the corresponding encoder driver init, since bridge driver > > needs a drm_device pointer to finish drm initializations. > > The encoder driver passes the drm_device pointer to the > > bridge driver. Because of this dependency, third party drivers > > like ptn3460 doesn't adhere to the driver model. > > > > In this patch, we reframe the bridge registration framework > > so that bridge initialization is split into 2 steps, and > > bridge registration happens independent of drm flow: > > --Step 1: gather all the bridge settings independent of drm and > > add the bridge onto a global list of bridges. > > --Step 2: when the encoder driver is probed, call drm_bridge_attach > > for the corresponding bridge so that the bridge receives > > drm_device pointer and continues with connector and other > > drm initializations. > > > > The old set of bridge helpers are removed, and a set of new helpers > > are added to accomplish the 2 step initialization. > > > > The bridge devices register themselves onto global list of bridges > > when they get probed by calling "drm_bridge_add". > > > > The parent encoder driver waits till the bridge is available > > in the lookup table(by calling "of_drm_find_bridge") and then > > continues with its initialization. > > > > The encoder driver should also call "drm_bridge_attach" to pass > > on the drm_device to the bridge object. > > > > drm_bridge_attach inturn calls "bridge->funcs->attach" so that > > bridge can continue with drm related initializations. > > ok, so I probably should have had a closer look at this before it > landed in drm-next, so if it is too late to revert (and deal w/ > untangling subsequent patches that depend on this) some of these > issues we'll just have to fix with follow-on patches. > > 1) needs headerdoc for new fxns in drm_bridge.c, and needs to be added > in drm.tmpl drm_panel.c is missing kerneldoc as well, though I have a local patch to add it. If nobody else steps up I'll take this. > 2) as far as I can tell, the new approach to cleaning up bridge > objects is to just let them leak !?! With this series bridges are no longer part of the DRM device and it's the driver that provides them that needs to free them (in ->remove()). It's not a completely perfect solution yet, but with the registry patch that I proposed a while back all the remaining issues should go away. Now if I could get anybody to look at that patch... Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/443d0324/attachment.sig>
[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Tue, Feb 03, 2015 at 08:48:56AM +0100, Daniel Vetter wrote: > On Mon, Feb 02, 2015 at 03:30:21PM -0500, Rob Clark wrote: > > On Mon, Feb 2, 2015 at 11:54 AM, Daniel Vetter wrote: > > >> My initial thought is for dma-buf to not try to prevent something than > > >> an exporter can actually do.. I think the scenario you describe could > > >> be handled by two sg-lists, if the exporter was clever enough. > > > > > > That's already needed, each attachment has it's own sg-list. After all > > > there's no array of dma_addr_t in the sg tables, so you can't use one sg > > > for more than one mapping. And due to different iommu different devices > > > can easily end up with different addresses. > > > > > > Well, to be fair it may not be explicitly stated, but currently one > > should assume the dma_addr_t's in the dmabuf sglist are bogus. With > > gpu's that implement per-process/context page tables, I'm not really > > sure that there is a sane way to actually do anything else.. > > Hm, what does per-process/context page tables have to do here? At least on > i915 we have a two levels of page tables: > - first level for vm/device isolation, used through dma api > - 2nd level for per-gpu-context isolation and context switching, handled > internally. > > Since atm the dma api doesn't have any context of contexts or different > pagetables, I don't see who you could use that at all. What I've found with *my* etnaviv drm implementation (not Christian's - I found it impossible to work with Christian, especially with the endless "msm doesn't do it that way, so we shouldn't" responses and his attitude towards cherry-picking my development work [*]) is that it's much easier to keep the GPU MMU local to the GPU and under the control of the DRM MM code, rather than attaching the IOMMU to the DMA API and handling it that way. There are several reasons for that: 1. DRM has a better idea about when the memory needs to be mapped to the GPU, and it can more effectively manage the GPU MMU. 2. The GPU MMU may have TLBs which can only be flushed via a command in the GPU command stream, so it's fundamentally necessary for the MMU to be managed by the GPU driver so that it knows when (and how) to insert the flushes. * - as a direct result of that, I've stopped all further development of etnaviv drm, and I'm intending to strip it out from my Xorg DDX driver as the etnaviv drm API which Christian wants is completely incompatible with the non-etnaviv drm, and that just creates far too much pain in the DDX driver. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
[Bug 84648] Lag/Pause When VRAM->GTT or GTT->VRAM transfer occur
https://bugs.freedesktop.org/show_bug.cgi?id=84648 --- Comment #7 from smoki --- (In reply to commiethebeastie from comment #6) > Created attachment 113077 [details] > ETS2 with GTT-usage:100 > > My 3Com U.S Robotics modem 56k works more faster than the GART memory. Yeah i can confirm that with ETS2, but also with any earlier 18 Wheels of Steel games (http://en.wikipedia.org/wiki/18_Wheels_of_Steel) played in wine (i have all of them :)) have same very worse stutter... try how it goes with reverting this mesa commit, it fixed that worse stutter in those games for me on Kabini APU... well and other issues: https://bugs.freedesktop.org/show_bug.cgi?id=88658#c6 And yeah please use at least kernel 3.19 or later if you test that -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/cb99ce00/attachment-0001.html>
[PATCH v3] drm/panel: add s6e3ha2 AMOLED panel driver
ulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> + if (ret < 0)
> + return ret;
> +
> + msleep(25);
> +
> + gpiod_set_value(ctx->panel_en_gpio, 0);
> + usleep_range(5000, 6000);
> + gpiod_set_value(ctx->panel_en_gpio, 1);
> +
> + gpiod_set_value(ctx->reset_gpio, 1);
> + usleep_range(5000, 6000);
> + gpiod_set_value(ctx->reset_gpio, 0);
> + usleep_range(5000, 6000);
> + gpiod_set_value(ctx->reset_gpio, 1);
I would expect the value after power on to be 0 for reset. Perhaps you
need GPIO_ACTIVE_LOW in your device tree? Also why do you toggle thrice?
I would assume that putting the peripheral into reset and taking it out
again would be enough.
> +static int s6e3ha2_prepare(struct drm_panel *panel)
> +{
> + struct s6e3ha2 *ctx = panel_to_s6e3ha2(panel);
> + int ret;
> +
> + ret = s6e3ha2_power_on(ctx);
> + if (ret < 0)
> + return ret;
> +
> + s6e3ha2_panel_init(ctx);
> + if (ctx->error < 0)
This is one of the reasons why ctx->error is a bad idea. It's completely
unintuitive to check ctx->error here because nobody's actually setting
it in this context.
> + s6e3ha2_unprepare(panel);
This is somewhat asymmetric. I would expect the s6e3ha2_panel_init() to
undo what it did on failure, so that you would only have to call
s6e3ha2_power_off() here and undo what you did in *this* function.
> +static int s6e3ha2_enable(struct drm_panel *panel)
> +{
> + return 0;
> +}
This is really where you're supposed to turn on the backlight or
similar. Where does that happen?
> +static int s6e3ha2_get_modes(struct drm_panel *panel)
> +{
> + struct drm_connector *connector = panel->connector;
> + struct s6e3ha2 *ctx = panel_to_s6e3ha2(panel);
> + struct drm_display_mode *mode;
> +
> + mode = drm_mode_create(connector->dev);
> + if (!mode) {
> + DRM_ERROR("failed to create a new display mode\n");
> + return 0;
> + }
> +
> + drm_display_mode_from_videomode(&ctx->vm, mode);
> + mode->width_mm = ctx->width_mm;
> + mode->height_mm = ctx->height_mm;
> + connector->display_info.width_mm = mode->width_mm;
> + connector->display_info.height_mm = mode->height_mm;
> +
> + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> + drm_mode_probed_add(connector, mode);
> +
> + return 1;
> +}
Like I said before, the mode is implied by the compatible value, so no
need to parse it from device tree.
> +static int s6e3ha2_probe(struct mipi_dsi_device *dsi)
> +{
> + struct device *dev = &dsi->dev;
> + struct s6e3ha2 *ctx;
> + int ret;
> +
> + ctx = devm_kzalloc(dev, sizeof(struct s6e3ha2), GFP_KERNEL);
sizeof(*ctx) is much shorter.
> + ctx->reset_gpio = devm_gpiod_get(dev, "reset");
> + if (IS_ERR(ctx->reset_gpio)) {
> + dev_err(dev, "cannot get reset-gpios %ld\n",
> + PTR_ERR(ctx->reset_gpio));
> + return PTR_ERR(ctx->reset_gpio);
> + }
> + ret = gpiod_direction_output(ctx->reset_gpio, 1);
For consistency the above two lines should be separated by a blank line.
> + if (ret < 0) {
> + dev_err(dev, "cannot configure reset-gpios %d\n", ret);
> + return ret;
> + }
> +
> + ctx->panel_en_gpio = devm_gpiod_get(dev, "panel-en");
> + if (IS_ERR(ctx->panel_en_gpio)) {
> + dev_err(dev, "cannot get panel-en-gpios %ld\n",
> + PTR_ERR(ctx->panel_en_gpio));
> + return PTR_ERR(ctx->panel_en_gpio);
> + }
> + ret = gpiod_direction_output(ctx->panel_en_gpio, 1);
Same here.
> + if (ret < 0) {
> + dev_err(dev, "cannot configure panel-en-gpios %d\n", ret);
> + return ret;
> + }
You seem to be turning on the panel here. That's wrong because you're
supposed to wait for the display driver to tell you to turn it on via
->prepare() and ->enable().
> +
> + drm_panel_init(&ctx->panel);
> + ctx->panel.dev = dev;
> + ctx->panel.funcs = &s6e3ha2_drm_funcs;
I don't see a use for the _drm in here.
> +static struct of_device_id s6e3ha2_of_match[] = {
static const, please.
> + { .compatible = "samsung,s6e3ha2" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, s6e3ha2_of_match);
> +
> +static struct mipi_dsi_driver s6e3ha2_driver = {
> + .probe = s6e3ha2_probe,
> + .remove = s6e3ha2_remove,
> + .driver = {
> + .name = "panel_s6e3ha2",
Please use a - instead of an _ here, for consistency with other drivers.
> + .owner = THIS_MODULE,
No need for this anymore.
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/e5c8aa2f/attachment.sig>
[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Tue, Feb 03, 2015 at 12:28:14PM +, Russell King - ARM Linux wrote: > On Tue, Feb 03, 2015 at 08:48:56AM +0100, Daniel Vetter wrote: > > On Mon, Feb 02, 2015 at 03:30:21PM -0500, Rob Clark wrote: > > > On Mon, Feb 2, 2015 at 11:54 AM, Daniel Vetter wrote: > > > >> My initial thought is for dma-buf to not try to prevent something than > > > >> an exporter can actually do.. I think the scenario you describe could > > > >> be handled by two sg-lists, if the exporter was clever enough. > > > > > > > > That's already needed, each attachment has it's own sg-list. After all > > > > there's no array of dma_addr_t in the sg tables, so you can't use one sg > > > > for more than one mapping. And due to different iommu different devices > > > > can easily end up with different addresses. > > > > > > > > > Well, to be fair it may not be explicitly stated, but currently one > > > should assume the dma_addr_t's in the dmabuf sglist are bogus. With > > > gpu's that implement per-process/context page tables, I'm not really > > > sure that there is a sane way to actually do anything else.. > > > > Hm, what does per-process/context page tables have to do here? At least on > > i915 we have a two levels of page tables: > > - first level for vm/device isolation, used through dma api > > - 2nd level for per-gpu-context isolation and context switching, handled > > internally. > > > > Since atm the dma api doesn't have any context of contexts or different > > pagetables, I don't see who you could use that at all. > > What I've found with *my* etnaviv drm implementation (not Christian's - I > found it impossible to work with Christian, especially with the endless > "msm doesn't do it that way, so we shouldn't" responses and his attitude > towards cherry-picking my development work [*]) is that it's much easier to > keep the GPU MMU local to the GPU and under the control of the DRM MM code, > rather than attaching the IOMMU to the DMA API and handling it that way. > > There are several reasons for that: > > 1. DRM has a better idea about when the memory needs to be mapped to the >GPU, and it can more effectively manage the GPU MMU. > > 2. The GPU MMU may have TLBs which can only be flushed via a command in >the GPU command stream, so it's fundamentally necessary for the MMU to >be managed by the GPU driver so that it knows when (and how) to insert >the flushes. 3. Switching between different address spaces (for per gpu context isolation) often requires intricate knowledge about the gpu and close coordination. Well maybe just a part of 2 really, but an important one. Fully agree and tbh I'm not sure whether the current push in arm to expose all gpu mmus as iommus is solid. Even for pasid (per-context iommu tables) which is a big official standard there's still a lot of open questions about how to do it properly. And it requires strict hw support so that the hw always knows which pasid it should use for a given dma access. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[RFC v3 1/4] drm: Add support to find drm_panel by name
On Wed, Jan 21, 2015 at 04:48:10PM +0530, Shobhit Kumar wrote:
> For scenarios where OF is not available, we can use panel identification by
> name.
>
> Signed-off-by: Shobhit Kumar
> ---
> drivers/gpu/drm/drm_panel.c | 18 ++
> include/drm/drm_panel.h | 3 +++
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index 2ef988e..e1cb8cf 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -95,6 +95,24 @@ struct drm_panel *of_drm_find_panel(struct device_node *np)
> EXPORT_SYMBOL(of_drm_find_panel);
> #endif
>
> +struct drm_panel *drm_find_panel_by_name(const char *name)
> +{
> + struct drm_panel *panel;
> +
> + mutex_lock(&panel_lock);
> +
> + list_for_each_entry(panel, &panel_list, list) {
> + if (strcmp(panel->name, name) == 0) {
> + mutex_unlock(&panel_lock);
> + return panel;
> + }
> + }
> +
> + mutex_unlock(&panel_lock);
> + return NULL;
> +}
> +EXPORT_SYMBOL(drm_find_panel_by_name);
> +
> MODULE_AUTHOR("Thierry Reding ");
> MODULE_DESCRIPTION("DRM panel infrastructure");
> MODULE_LICENSE("GPL and additional rights");
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 1fbcc96..1ef9ff3 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -74,6 +74,7 @@ struct drm_panel {
> struct drm_device *drm;
> struct drm_connector *connector;
> struct device *dev;
> + char name[NAME_MAX];
I thought I had said this before, but I can't find any record, so here
goes again: NAME_MAX is a pretty large number and it increases the size
of this structure a lot. Why not use dev_name(dev) to do the matching?
Or if that doesn't work for some reason, allocate a string of the right
size, or use a static one.
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/91aced16/attachment.sig>
[RFC v3 2/4] mfd: Add a new cell device for panel controlled by crystal cove pmic
On Wed, Jan 21, 2015 at 04:48:11PM +0530, Shobhit Kumar wrote:
> On BYT-T configuration, panel enable/disable signals are routed through
> PMIC. Add a cell device for the same.
>
> Signed-off-by: Shobhit Kumar
> ---
> drivers/mfd/intel_soc_pmic_crc.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/mfd/intel_soc_pmic_crc.c
> b/drivers/mfd/intel_soc_pmic_crc.c
> index c85e2ec..c8ccc24 100644
> --- a/drivers/mfd/intel_soc_pmic_crc.c
> +++ b/drivers/mfd/intel_soc_pmic_crc.c
> @@ -109,6 +109,9 @@ static struct mfd_cell crystal_cove_dev[] = {
> {
> .name = "crystal_cove_pmic",
> },
> + {
> + .name = "crystal_cove_panel",
> + },
> };
>
> static struct regmap_config crystal_cove_regmap_config = {
This doesn't look at all right. A PMIC doesn't typically contain a panel
so this likely is a wrong description of the hardware. Is the datasheet
for the Crystal Cove PMIC available somewhere? Google doesn't turn any-
thing useful up.
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/ae9ddb19/attachment-0001.sig>
[RFC v3 3/4] drm/i915: Add new panel driver based on crystal cove pmic
On Wed, Jan 21, 2015 at 04:48:12PM +0530, Shobhit Kumar wrote:
> This driver provides support for the "crystal_cove_panel" cell device.
> On BYT-T pmic has to be used to enable/disable panel.
>
> v2: Addressed Jani's comments
> - Moved inside i915
> - Correct licensing
> - Remove unused stuff
> - Do not initialize prepare/unprepare as they are not needed as of now
> - Correct backlight off delay
>
> Signed-off-by: Shobhit Kumar
> ---
> drivers/gpu/drm/i915/Kconfig | 12 ++
> drivers/gpu/drm/i915/Makefile | 3 +
> drivers/gpu/drm/i915/intel-panel-crystalcove.c | 159
> +
> 3 files changed, 174 insertions(+)
> create mode 100644 drivers/gpu/drm/i915/intel-panel-crystalcove.c
>
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index 4e39ab3..0510ef0 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -69,3 +69,15 @@ config DRM_I915_PRELIMINARY_HW_SUPPORT
> option changes the default for that module option.
>
> If in doubt, say "N".
> +
> +config DRM_I915_PANEL_CRYSTALCOVE_PMIC
> + bool "Enable drm panel for crystal cove pmic based control"
> + depends on DRM_I915
> + depends on DRM_PANEL
> + default n
n is the default default.
> diff --git a/drivers/gpu/drm/i915/intel-panel-crystalcove.c
> b/drivers/gpu/drm/i915/intel-panel-crystalcove.c
[...]
> +#define PMIC_PANEL_EN0x52
> +#define PMIC_PWM_EN 0x51
> +#define PMIC_BKL_EN 0x4B
> +#define PMIC_PWM_LEVEL 0x4E
These look like they should be GPIOs/regulators and a PWM instead. So I
think you'd need to further split up the MFD device to accomodate for
this.
> +static inline struct crystalcove_panel *to_crystalcove_panel(struct
> drm_panel *panel)
Please wrap this so it doesn't cross the 80-character boundary.
> +static int crystalcove_panel_disable(struct drm_panel *panel)
> +{
> + struct crystalcove_panel *p = to_crystalcove_panel(panel);
> +
> + if (!p->enabled)
> + return 0;
> +
> + DRM_DEBUG_KMS("\n");
> +
> + /* invoke the pmic driver */
> + regmap_write(p->regmap, PMIC_PANEL_EN, 0x00);
A datasheet would be really good to determine whether this is the
correct place to write this. There are ->prepare() and ->unprepare()
callbacks that get the panel into a state where you can communicate
with it. This being a DSI panel I would assume that you need to enable
the panel to some degree so you can send DSI commands. So most likely
this enable "GPIO" should be set in ->unprepare().
> +static int crystalcove_panel_enable(struct drm_panel *panel)
> +{
> + struct crystalcove_panel *p = to_crystalcove_panel(panel);
> +
> + if (p->enabled)
> + return 0;
> +
> + DRM_DEBUG_KMS("\n");
> +
> + /* invoke the pmic driver */
> + regmap_write(p->regmap, PMIC_PANEL_EN, 0x01);
Similarly I'd expect this to go into ->prepare() to make sure that you
can communicate with the panel after ->prepare().
> + /* Enable BKL as well */
> + regmap_write(p->regmap, PMIC_BKL_EN, 0xFF);
Writing 0xff to an "enable" register seems weird. Again the datasheet
would help in determining the right thing to do here.
> + regmap_write(p->regmap, PMIC_PWM_EN, 0x01);
> + msleep(20);
> + regmap_write(p->regmap, PMIC_PWM_LEVEL, 255);
This definitely looks like it should be a PWM driver. Also how do you
handle backlight brightness control? You only set things to either full
off or full on in this driver.
> +
> + drm_panel_init(&panel->base);
> + panel->base.dev = dev;
> + panel->base.funcs = &crystalcove_panel_funcs;
> +
> + drm_panel_add(&panel->base);
This function can theoretically fail, although it doesn't (at present),
so checking the error might be a good idea.
> +static int crystalcove_panel_remove(struct platform_device *pdev)
> +{
> + struct crystalcove_panel *panel = platform_get_drvdata(pdev);
> +
> + DRM_DEBUG_KMS("\n");
> +
> + drm_panel_detach(&panel->base);
> + drm_panel_remove(&panel->base);
> +
> + crystalcove_panel_disable(&panel->base);
> +
> + return 0;
> +}
> +
> +static struct platform_driver crystalcove_panel_driver = {
> + .probe = crystalcove_panel_probe,
> + .remove = crystalcove_panel_remove,
> + .driver = {
> + .name = "crystal_cove_panel",
> + },
> +};
> +
> +module_platform_driver(crystalcove_pan
[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
2015-02-03 13:28 GMT+01:00 Russell King - ARM Linux : > On Tue, Feb 03, 2015 at 08:48:56AM +0100, Daniel Vetter wrote: >> On Mon, Feb 02, 2015 at 03:30:21PM -0500, Rob Clark wrote: >> > On Mon, Feb 2, 2015 at 11:54 AM, Daniel Vetter wrote: >> > >> My initial thought is for dma-buf to not try to prevent something than >> > >> an exporter can actually do.. I think the scenario you describe could >> > >> be handled by two sg-lists, if the exporter was clever enough. >> > > >> > > That's already needed, each attachment has it's own sg-list. After all >> > > there's no array of dma_addr_t in the sg tables, so you can't use one sg >> > > for more than one mapping. And due to different iommu different devices >> > > can easily end up with different addresses. >> > >> > >> > Well, to be fair it may not be explicitly stated, but currently one >> > should assume the dma_addr_t's in the dmabuf sglist are bogus. With >> > gpu's that implement per-process/context page tables, I'm not really >> > sure that there is a sane way to actually do anything else.. >> >> Hm, what does per-process/context page tables have to do here? At least on >> i915 we have a two levels of page tables: >> - first level for vm/device isolation, used through dma api >> - 2nd level for per-gpu-context isolation and context switching, handled >> internally. >> >> Since atm the dma api doesn't have any context of contexts or different >> pagetables, I don't see who you could use that at all. > > What I've found with *my* etnaviv drm implementation (not Christian's - I > found it impossible to work with Christian, especially with the endless > "msm doesn't do it that way, so we shouldn't" responses and his attitude > towards cherry-picking my development work [*]) is that it's much easier to > keep the GPU MMU local to the GPU and under the control of the DRM MM code, > rather than attaching the IOMMU to the DMA API and handling it that way. > Keep in mind that I tried to reach you several times via mail and irc and you simply ignored me. Did you know that took almost all of your patches (with small changes)? And I needed to cherry pick you patches as they were a) wrong, b) solved in a different way or c) had "hack" in the subject. I am quite sorry that I ended that way, but it is not only my fault! > There are several reasons for that: > > 1. DRM has a better idea about when the memory needs to be mapped to the >GPU, and it can more effectively manage the GPU MMU. > > 2. The GPU MMU may have TLBs which can only be flushed via a command in >the GPU command stream, so it's fundamentally necessary for the MMU to >be managed by the GPU driver so that it knows when (and how) to insert >the flushes. > > > * - as a direct result of that, I've stopped all further development of > etnaviv drm, and I'm intending to strip it out from my Xorg DDX driver > as the etnaviv drm API which Christian wants is completely incompatible > with the non-etnaviv drm, and that just creates far too much pain in the > DDX driver. > That is bad, but life moves on. greets -- Christian Gmeiner, MSc https://soundcloud.com/christian-gmeiner
[PATCH 2/2] drm/exynos: solve plane on/off disharmory issue
Hi Joonyoung,
2015-01-29 Joonyoung Shim :
> The exynos_update_plane functions can be called from set_plane as well
> as set_crtc and pageflip. Currently the plane displayed by set_plane
> isn't called exynos_plane_on function and if plane is disabled, it calls
> exynos_plane_off, so it causes disharmory of plane on/off.
>
> This is caused from commit e7cd8041 ("drm/exynos: Don't touch DPMS
> when updating overlay planes").
>
> Make .update_plane function called only by set_plane and call
> exynos_plane_on in it.
>
> Signed-off-by: Joonyoung Shim
> ---
> drivers/gpu/drm/exynos/exynos_drm_crtc.c | 4 ++--
> drivers/gpu/drm/exynos/exynos_drm_plane.c | 21 -
> drivers/gpu/drm/exynos/exynos_drm_plane.h | 2 +-
> 3 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> index dac8f90..2765f7e 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> @@ -129,7 +129,7 @@ static int exynos_drm_crtc_mode_set_base(struct drm_crtc
> *crtc, int x, int y,
> crtc_w = fb->width - x;
> crtc_h = fb->height - y;
>
> - return exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
> + return exynos_plane_update(crtc->primary, crtc, fb, 0, 0,
> crtc_w, crtc_h, x, y, crtc_w, crtc_h);
This patch goes in the opposite direction of the clean up to support atomic
modesetting on exynos (see my patches for atomic modesetting here[0]) In my
latest series there was an effort to unify all places we update a plane under
exynos_update_plane() and this is a essential step for atomic modesetting.
My proposal to solve this issue would just be calling exynos_plane_dpms()
on exynos_drm_crtc_mode_set_base() and leave the rest as is. This would fix
the disharmory of plane on/off. I'll send a patch for this in a bit.
[0]
https://git.kernel.org/cgit/linux/kernel/git/padovan/drm-exynos.git/log/?h=atomic-next-2
Gustavo
[PATCH 1/3] drm/panel: Add display_timing support
On Thu, Dec 11, 2014 at 06:32:44PM +0100, Philipp Zabel wrote:
> Many panel data sheets additionally to typical values list allowed ranges for
> timings such as hsync/vsync lengths, porches, and the pixel clock rate. These
> can be stored in a struct display_timing, to be used by an encoder mode_fixup
> callback to clamp user provided timing values or to validate workarounds for
> clock source limitations.
>
> This patch adds a new drm_panel_funcs callback that returns the panels'
> available display_timing entries.
>
> Signed-off-by: Philipp Zabel
> ---
> include/drm/drm_panel.h | 5 +
> 1 file changed, 5 insertions(+)
Sorry for taking so long to get back on this. I generally like the idea,
though a couple of things are unclear to me.
In struct display_timing we have:
struct timing_entry hactive;
...
struct timing_entry vactive;
I wonder if that really makes sense. Are there really datasheets that
provide a valid range for the number of horizontal and vertical active
pixels?
>
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 1fbcc96..13ff44b 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -29,6 +29,7 @@
> struct drm_connector;
> struct drm_device;
> struct drm_panel;
> +struct display_timing;
>
> /**
> * struct drm_panel_funcs - perform operations on a given panel
> @@ -38,6 +39,8 @@ struct drm_panel;
> * @enable: enable panel (turn on back light, etc.)
> * @get_modes: add modes to the connector that the panel is attached to and
> * return the number of modes added
> + * @get_timings: copy display timings into the provided array and return
> + * the number of display timings available
> *
> * The .prepare() function is typically called before the display controller
> * starts to transmit video data. Panel drivers can use this to turn the
> panel
> @@ -68,6 +71,8 @@ struct drm_panel_funcs {
> int (*prepare)(struct drm_panel *panel);
> int (*enable)(struct drm_panel *panel);
> int (*get_modes)(struct drm_panel *panel);
> + int (*get_timings)(struct drm_panel *panel, unsigned int num_timings,
> +struct display_timing *timings);
Also are there even panels that support more than one set of timings?
I've never heard of panels that are actually able to do more than one
mode, so I'm wondering if num_timings isn't overdoing it a little here.
I guess it doesn't hurt all that much, though.
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/23f05c0b/attachment.sig>
[PATCH] drm/exynos: Fix disharmony when setting plane on/off
From: Gustavo Padovan
This issue was caused by the latest exynos_update_plane() clean up
that unified plane operations. After those changes the plane failed
to go the On state. This patch fix this problem by doing correct account
of exynos_crtc->enabled.
Signed-off-by: Gustavo Padovan
---
drivers/gpu/drm/exynos/exynos_drm_crtc.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index a85c451..1dbd0e3 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -119,6 +119,7 @@ static int exynos_drm_crtc_mode_set_base(struct drm_crtc
*crtc, int x, int y,
struct drm_framebuffer *fb = crtc->primary->fb;
unsigned int crtc_w;
unsigned int crtc_h;
+ int ret;
/* when framebuffer changing is requested, crtc's dpms should be on */
if (exynos_crtc->dpms > DRM_MODE_DPMS_ON) {
@@ -129,8 +130,15 @@ static int exynos_drm_crtc_mode_set_base(struct drm_crtc
*crtc, int x, int y,
crtc_w = fb->width - x;
crtc_h = fb->height - y;
- return exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
- crtc_w, crtc_h, x, y, crtc_w, crtc_h);
+ ret = exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
+ crtc_w, crtc_h, x, y, crtc_w, crtc_h);
+ if (!ret)
+ return ret;
+
+ exynos_plane_dpms(crtc->primary, DRM_MODE_DPMS_ON);
+
+ return 0;
+
}
static void exynos_drm_crtc_disable(struct drm_crtc *crtc)
--
1.9.3
[PATCH v3 2/3] drm/panel: add s6e63j0x03 LCD panel driver
On Mon, Jan 19, 2015 at 04:52:32PM +0900, Hyungwon Hwang wrote: > From: Inki Dae > > This patch adds MIPI-DSI based S6E63J0X03 AMOLED LCD panel driver > which uses mipi_dsi bus to communicate with panel. The panel has > 320Ã320 resolution in 1.63-inch physical panel. This panel is used in > Samsung Galaxy Gear 2. > > Signed-off-by: Inki Dae > Signed-off-by: Hyungwon Hwang > Acked-by: Kyungmin Park > --- > Changes for v2: > - Change the gamma table to 2-dimensional array > - Change the way to make index for brightness > - Make command functions to an array so that it can be called simply > - Change command id for reading device ID > - Change the way to handle the error condition > - Remove power variable, and use the same name variable in bl_dev > - Add the state FB_BLANK_NORMAL to represent the state which the panel > is working but blanked > - Miscellaneous changes to increase the readability and follow the > coding-style standard > Changes for v3: > - Add DT binding documentation > - Add the code getting DT properties of 'panel-width-mm' and 'panel-height-mm' > - Remove the code getting unnecessary DT properties flip-* > .../bindings/panel/samsung,s6e63j0x03.txt | 55 +++ > drivers/gpu/drm/panel/Kconfig | 6 + > drivers/gpu/drm/panel/Makefile | 1 + > drivers/gpu/drm/panel/panel-s6e63j0x03.c | 546 > + > 4 files changed, 608 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/panel/samsung,s6e63j0x03.txt > create mode 100644 drivers/gpu/drm/panel/panel-s6e63j0x03.c I have pretty much the same comments as for the SA6E3HA2 panel driver that I reviewed earlier. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/d0edcc9d/attachment-0001.sig>
[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Tue, Feb 3, 2015 at 2:48 AM, Daniel Vetter wrote: > On Mon, Feb 02, 2015 at 03:30:21PM -0500, Rob Clark wrote: >> On Mon, Feb 2, 2015 at 11:54 AM, Daniel Vetter wrote: >> >> My initial thought is for dma-buf to not try to prevent something than >> >> an exporter can actually do.. I think the scenario you describe could >> >> be handled by two sg-lists, if the exporter was clever enough. >> > >> > That's already needed, each attachment has it's own sg-list. After all >> > there's no array of dma_addr_t in the sg tables, so you can't use one sg >> > for more than one mapping. And due to different iommu different devices >> > can easily end up with different addresses. >> >> >> Well, to be fair it may not be explicitly stated, but currently one >> should assume the dma_addr_t's in the dmabuf sglist are bogus. With >> gpu's that implement per-process/context page tables, I'm not really >> sure that there is a sane way to actually do anything else.. > > Hm, what does per-process/context page tables have to do here? At least on > i915 we have a two levels of page tables: > - first level for vm/device isolation, used through dma api > - 2nd level for per-gpu-context isolation and context switching, handled > internally. > > Since atm the dma api doesn't have any context of contexts or different > pagetables, I don't see who you could use that at all. Since I'm stuck w/ an iommu, instead of built in mmu, my plan was to drop use of dma-mapping entirely (incl the current call to dma_map_sg, which I just need until we can use drm_cflush on arm), and attach/detach iommu domains directly to implement context switches. At that point, dma_addr_t really has no sensible meaning for me. BR, -R > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 6/6] drm/exynos: do not copy adjusted mode into mode during crtc mode_set
2015-02-02 Joonyoung Shim : > Hi, > > On 01/30/2015 11:44 PM, Gustavo Padovan wrote: > > Hi Joonyoung, > > > > 2015-01-30 Joonyoung Shim : > > > >> Hi, > >> > >> On 01/23/2015 09:43 PM, Gustavo Padovan wrote: > >>> From: Daniel Kurtz > >>> > >>> The 'mode' is the modeline information which specifies the ideal mode > >>> requested by the mode set initiator (usually userspace). > >>> The 'adjusted_mode' is the actual hardware mode after all the crtcs > >>> and encoders have had a chance to "fix it up". > >>> > >>> The adjusted_mode starts as a duplicate of the mode in > >>> drm_crtc_helper_set_mode(), and gets modified as required. There is no > >>> reason to touch the original requested mode. > >>> > >> > >> Agree, but is there any side effect after this commit? Should we save > >> adjusted_mode to other variable and use it? > > > > I haven't seen any. Tested on peach pi and snow. Do we have any reason to > > save > > it now? I don't we have a user for it now. > > > > Because current codes use values of adjusted_mode in exynos drm hw drivers. I fail to see where this happen. adjusted_mode is passed to to the mode_set() callback and drivers can use it from there as the hdmi one does for example. Gustavo
[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Tuesday 03 February 2015 09:04:03 Rob Clark wrote: > On Tue, Feb 3, 2015 at 2:48 AM, Daniel Vetter wrote: > > On Mon, Feb 02, 2015 at 03:30:21PM -0500, Rob Clark wrote: > >> On Mon, Feb 2, 2015 at 11:54 AM, Daniel Vetter wrote: > >> >> My initial thought is for dma-buf to not try to prevent something than > >> >> an exporter can actually do.. I think the scenario you describe could > >> >> be handled by two sg-lists, if the exporter was clever enough. > >> > > >> > That's already needed, each attachment has it's own sg-list. After all > >> > there's no array of dma_addr_t in the sg tables, so you can't use one sg > >> > for more than one mapping. And due to different iommu different devices > >> > can easily end up with different addresses. > >> > >> > >> Well, to be fair it may not be explicitly stated, but currently one > >> should assume the dma_addr_t's in the dmabuf sglist are bogus. With > >> gpu's that implement per-process/context page tables, I'm not really > >> sure that there is a sane way to actually do anything else.. > > > > Hm, what does per-process/context page tables have to do here? At least on > > i915 we have a two levels of page tables: > > - first level for vm/device isolation, used through dma api > > - 2nd level for per-gpu-context isolation and context switching, handled > > internally. > > > > Since atm the dma api doesn't have any context of contexts or different > > pagetables, I don't see who you could use that at all. > > Since I'm stuck w/ an iommu, instead of built in mmu, my plan was to > drop use of dma-mapping entirely (incl the current call to dma_map_sg, > which I just need until we can use drm_cflush on arm), and > attach/detach iommu domains directly to implement context switches. > At that point, dma_addr_t really has no sensible meaning for me. I think what you see here is a quite common hardware setup and we really lack the right abstraction for it at the moment. Everybody seems to work around it with a mix of the dma-mapping API and the iommu API. These are doing different things, and even though the dma-mapping API can be implemented on top of the iommu API, they are not really compatible. The drm_clflush helpers don't seem like the right solution to me, because all other devices outside of drm will face the same issue, and I suspect we should fill the missing gaps in the API in a more generic way. Arnd
[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Tue, Feb 3, 2015 at 7:28 AM, Russell King - ARM Linux wrote: > On Tue, Feb 03, 2015 at 08:48:56AM +0100, Daniel Vetter wrote: >> On Mon, Feb 02, 2015 at 03:30:21PM -0500, Rob Clark wrote: >> > On Mon, Feb 2, 2015 at 11:54 AM, Daniel Vetter wrote: >> > >> My initial thought is for dma-buf to not try to prevent something than >> > >> an exporter can actually do.. I think the scenario you describe could >> > >> be handled by two sg-lists, if the exporter was clever enough. >> > > >> > > That's already needed, each attachment has it's own sg-list. After all >> > > there's no array of dma_addr_t in the sg tables, so you can't use one sg >> > > for more than one mapping. And due to different iommu different devices >> > > can easily end up with different addresses. >> > >> > >> > Well, to be fair it may not be explicitly stated, but currently one >> > should assume the dma_addr_t's in the dmabuf sglist are bogus. With >> > gpu's that implement per-process/context page tables, I'm not really >> > sure that there is a sane way to actually do anything else.. >> >> Hm, what does per-process/context page tables have to do here? At least on >> i915 we have a two levels of page tables: >> - first level for vm/device isolation, used through dma api >> - 2nd level for per-gpu-context isolation and context switching, handled >> internally. >> >> Since atm the dma api doesn't have any context of contexts or different >> pagetables, I don't see who you could use that at all. > > What I've found with *my* etnaviv drm implementation (not Christian's - I > found it impossible to work with Christian, especially with the endless > "msm doesn't do it that way, so we shouldn't" responses and his attitude > towards cherry-picking my development work [*]) is that it's much easier to > keep the GPU MMU local to the GPU and under the control of the DRM MM code, > rather than attaching the IOMMU to the DMA API and handling it that way. > > There are several reasons for that: > > 1. DRM has a better idea about when the memory needs to be mapped to the >GPU, and it can more effectively manage the GPU MMU. > > 2. The GPU MMU may have TLBs which can only be flushed via a command in >the GPU command stream, so it's fundamentally necessary for the MMU to >be managed by the GPU driver so that it knows when (and how) to insert >the flushes. > If gpu mmu needs some/all updates to happen from command-stream then probably better to handle it internally.. That is a slightly different scenario from msm, where we have many instances of the same iommu[*] scattered through the SoC in front of various different devices. BR, -R [*] at least from iommu register layout, same driver is used for all instances.. but maybe the tlb+walker are maybe more tightly integrated to the gpu, but that is just speculation on implementation details based on some paper I found along the way > > * - as a direct result of that, I've stopped all further development of > etnaviv drm, and I'm intending to strip it out from my Xorg DDX driver > as the etnaviv drm API which Christian wants is completely incompatible > with the non-etnaviv drm, and that just creates far too much pain in the > DDX driver. > > -- > FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up > according to speedtest.net.
[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Tue, Feb 03, 2015 at 02:28:26PM +0100, Christian Gmeiner wrote: > 2015-02-03 13:28 GMT+01:00 Russell King - ARM Linux arm.linux.org.uk>: > > What I've found with *my* etnaviv drm implementation (not Christian's - I > > found it impossible to work with Christian, especially with the endless > > "msm doesn't do it that way, so we shouldn't" responses and his attitude > > towards cherry-picking my development work [*]) is that it's much easier to > > keep the GPU MMU local to the GPU and under the control of the DRM MM code, > > rather than attaching the IOMMU to the DMA API and handling it that way. > > > > Keep in mind that I tried to reach you several times via mail and irc > and you simply > ignored me. Did you know that took almost all of your patches (with > small changes)? > And I needed to cherry pick you patches as they were a) wrong, b) solved in a > different way or c) had "hack" in the subject. I am quite sorry that I > ended that > way, but it is not only my fault! Exactly - you *took* every patch that I published whether I was ready for you to take it or not. That's not how kernel development works. Kernel development works by people working on the code, and *pushing* patches or git trees upstream. It doesn't work by having people running around *taking* patches from people just because they feel like it. I asked you several times not to do that which means the only way I can control you is by *not* publishing my changes, thereby denying other people the ability to test my changes. Another result of you *taking* patches from me is that you totally *screwed* my ability to work with you. If you make this stuff unnecessary hard, you can expect people to walk away, and that's precisely what I've done. There's also the issue of you *taking* my patches and then applying them to your tree with your own modifications, again *screwing* my tree, and screwing my ability - again - to work with you. Many of my patches in your repository are also marked as you being the author of them... which _really_ is not nice. Your "review" comments of "based 1:1 on the MSM driver" were really crazy. Just because one DRM driver does something one way does not make it the only way, nor does it make it suitable for use everywhere, even if you modelled your driver on MSM. It certainly doesn't mean that the way the MSM driver does it is correct either. And frankly, you calling my patches "wrong" is laughable. I have a stable fully functional Xorg DDX driver here which works with my version of your etnaviv DRM across several Vivante GPUs - GC660 on Dove, and two revisions of GC320 on iMX6 (which are notoriously buggy). No kernel oopses, no GPU lockups. I've had machines with uptimes of a month here with it, with the driver being exercised frequently during that period. You refused to take things such as the DMA address monitoring for GPU hangups to stop it mis-firing. This _is_ a bug fix. Without that, your driver spits out random GPU hangups when there isn't actually any hangup at all. *Reliably* so. Your excuse for not taking it was "The current vivante driver doesn't do that." While that's true, the V2 Vivante drivers _do_ do it in their "guard thread" - and they do it because - as I already explained to you - it's entirely possible for the GPU to take a long time, longer than your hangcheck timeout, to render a series of 1080p operations. And again, not everything that the Vivante drivers do is correct either. Jon and myself know that very well having spent a long time debugging their GPL'd offerings. Even the "hack" patch was mostly correct - the reason that it is labelled as a "hack" is because - as the commit log said - it should really be done by the MMUv1 code, but that requires your entire IOMMU _bodge_ to be rewritten properly. Even the Vivante drivers use that region when they can. Then there's also the compatibility with the etnaviv library - which is an important thing if you want people to make use of your driver. You applied the patches for and then reverted which completely screws the Xorg DDX driver, making it impossible to support both etnaviv and etnadrm without having two almost identical copies of the same code. I don't want to maintain almost identical copies of that same code, and no one in their right mind would want that. Having some level of sane user compatibility between etnaviv and etnaviv drm will _gain_ you uses as it will allow people to write code which works on both platforms - and it's really not difficult to do. (In fact, I've proven it by writing a shim layer between the etnaviv API and the DRM interfaces in the DDX driver.) Then there's the round-robin IOMMU address space allocation, which is required to avoid corrupted pixmaps (which is something that _your_ driver does very very well - in fact, so well that it corrupts system memory), and the reaping of the IOMMU space when we run out of IOMMU space to map. Now, on to things that you do wrong. There'
[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Tue, Feb 03, 2015 at 09:04:03AM -0500, Rob Clark wrote: > Since I'm stuck w/ an iommu, instead of built in mmu, my plan was to > drop use of dma-mapping entirely (incl the current call to dma_map_sg, > which I just need until we can use drm_cflush on arm), and > attach/detach iommu domains directly to implement context switches. > At that point, dma_addr_t really has no sensible meaning for me. So how do you intend to import from a subsystem which only gives you the dma_addr_t? If you aren't passing system memory, you have no struct page. You can't fake up a struct page. What this means is that struct scatterlist can't represent it any other way. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Tue, Feb 03, 2015 at 03:17:27PM +0100, Arnd Bergmann wrote: > On Tuesday 03 February 2015 09:04:03 Rob Clark wrote: > > Since I'm stuck w/ an iommu, instead of built in mmu, my plan was to > > drop use of dma-mapping entirely (incl the current call to dma_map_sg, > > which I just need until we can use drm_cflush on arm), and > > attach/detach iommu domains directly to implement context switches. > > At that point, dma_addr_t really has no sensible meaning for me. > > I think what you see here is a quite common hardware setup and we really > lack the right abstraction for it at the moment. Everybody seems to > work around it with a mix of the dma-mapping API and the iommu API. > These are doing different things, and even though the dma-mapping API > can be implemented on top of the iommu API, they are not really compatible. I'd go as far as saying that the "DMA API on top of IOMMU" is more intended to be for a system IOMMU for the bus in question, rather than a device-level IOMMU. If an IOMMU is part of a device, then the device should handle it (maybe via an abstraction) and not via the DMA API. The DMA API should be handing the bus addresses to the device driver which the device's IOMMU would need to generate. (In other words, in this circumstance, the DMA API shouldn't give you the device internal address.) -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Tue, Feb 3, 2015 at 9:37 AM, Russell King - ARM Linux wrote: > On Tue, Feb 03, 2015 at 09:04:03AM -0500, Rob Clark wrote: >> Since I'm stuck w/ an iommu, instead of built in mmu, my plan was to >> drop use of dma-mapping entirely (incl the current call to dma_map_sg, >> which I just need until we can use drm_cflush on arm), and >> attach/detach iommu domains directly to implement context switches. >> At that point, dma_addr_t really has no sensible meaning for me. > > So how do you intend to import from a subsystem which only gives you > the dma_addr_t? > > If you aren't passing system memory, you have no struct page. You can't > fake up a struct page. What this means is that struct scatterlist can't > represent it any other way. Tell the exporter to stop using carveouts, and give me proper memory instead.. ;-) Well, at least on these SoC's, I think the only valid use for carveout memory is the bootloader splashscreen. And I was planning on just hanging on to that for myself for fbdev scanout buffer or other internal (non shared) usage.. BR, -R > -- > FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up > according to speedtest.net.
[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Tuesday 03 February 2015 14:41:09 Russell King - ARM Linux wrote: > On Tue, Feb 03, 2015 at 03:17:27PM +0100, Arnd Bergmann wrote: > > On Tuesday 03 February 2015 09:04:03 Rob Clark wrote: > > > Since I'm stuck w/ an iommu, instead of built in mmu, my plan was to > > > drop use of dma-mapping entirely (incl the current call to dma_map_sg, > > > which I just need until we can use drm_cflush on arm), and > > > attach/detach iommu domains directly to implement context switches. > > > At that point, dma_addr_t really has no sensible meaning for me. > > > > I think what you see here is a quite common hardware setup and we really > > lack the right abstraction for it at the moment. Everybody seems to > > work around it with a mix of the dma-mapping API and the iommu API. > > These are doing different things, and even though the dma-mapping API > > can be implemented on top of the iommu API, they are not really compatible. > > I'd go as far as saying that the "DMA API on top of IOMMU" is more > intended to be for a system IOMMU for the bus in question, rather > than a device-level IOMMU. > > If an IOMMU is part of a device, then the device should handle it > (maybe via an abstraction) and not via the DMA API. The DMA API should > be handing the bus addresses to the device driver which the device's > IOMMU would need to generate. (In other words, in this circumstance, > the DMA API shouldn't give you the device internal address.) Exactly. And the abstraction that people choose at the moment is the iommu API, for better or worse. It makes a lot of sense to use this API if the same iommu is used for other devices as well (which is the case on Tegra and probably a lot of others). Unfortunately the iommu API lacks support for cache management, and probably other things as well, because this was not an issue for the original use case (device assignment on KVM/x86). This could be done by adding explicit or implied cache management to the IOMMU mapping interfaces, or by extending the dma-mapping interfaces in a way that covers the use case of the device managing its own address space, in addition to the existing coherent and streaming interfaces. Arnd
[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Tue, Feb 03, 2015 at 09:44:57AM -0500, Rob Clark wrote: > On Tue, Feb 3, 2015 at 9:37 AM, Russell King - ARM Linux > wrote: > > On Tue, Feb 03, 2015 at 09:04:03AM -0500, Rob Clark wrote: > >> Since I'm stuck w/ an iommu, instead of built in mmu, my plan was to > >> drop use of dma-mapping entirely (incl the current call to dma_map_sg, > >> which I just need until we can use drm_cflush on arm), and > >> attach/detach iommu domains directly to implement context switches. > >> At that point, dma_addr_t really has no sensible meaning for me. > > > > So how do you intend to import from a subsystem which only gives you > > the dma_addr_t? > > > > If you aren't passing system memory, you have no struct page. You can't > > fake up a struct page. What this means is that struct scatterlist can't > > represent it any other way. > > Tell the exporter to stop using carveouts, and give me proper memory > instead.. ;-) > > Well, at least on these SoC's, I think the only valid use for carveout > memory is the bootloader splashscreen. And I was planning on just > hanging on to that for myself for fbdev scanout buffer or other > internal (non shared) usage.. I wasn't thinking about carveouts - as I already mentioned earlier in this thread, it may be memory which couldn't possibly ever be system memory - for example, a separate chunk of memory which is tightly coupled to the graphics system but not so to the CPU. In such a case, we wouldn't want to use that as normal system memory, but we would want to allocate framebuffers and the like from it, and maybe pass them around. While it may not be appropriate for MSM, it's still something that needs to be considered, because there may be (and I know there are) dmabuf users which do pass memory this way. So, what I'm saying is that for the purposes of the dmabuf API, we can't mandate that the scatterlists will contain a valid struct page pointer. It'd probably be a good idea for the importer to validate the scatterlist at import time if it has this requirement. However, thinking about this more, I think that from a generic design point of view, we really should limit the "struct page" usage to a special MSM-ism - something which should definitely not be copied by other drivers. As has been mentioned previously, if there is a system MMU which needs to be programmed to map system memory onto the bus, the struct page becomes absolutely useless, and the only thing that gives you the correct "handle" to that memory is the dma_addr_t. Finally, note that n_mapped = dma_map_sg(dev, sg, n_ent, dir) - n_mapped can be less than n_ent when there's the presence of an IOMMU, since an IOMMU is permitted to coalesce individual scatterlist entries if it so chooses, and when walking the scatterlist for DMA purposes, the scatterlist sg_dma_*() accessors should be used, and it should be iterated over from 0 to n_mapped, not 0 to n_ent. It's important to realise that in driver code, sg->length may not be the same as sg_dma_len(sg) for exactly this reason: #ifdef CONFIG_NEED_SG_DMA_LENGTH #define sg_dma_len(sg) ((sg)->dma_length) #else #define sg_dma_len(sg) ((sg)->length) #endif -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Tue, Feb 3, 2015 at 9:41 AM, Russell King - ARM Linux wrote: > On Tue, Feb 03, 2015 at 03:17:27PM +0100, Arnd Bergmann wrote: >> On Tuesday 03 February 2015 09:04:03 Rob Clark wrote: >> > Since I'm stuck w/ an iommu, instead of built in mmu, my plan was to >> > drop use of dma-mapping entirely (incl the current call to dma_map_sg, >> > which I just need until we can use drm_cflush on arm), and >> > attach/detach iommu domains directly to implement context switches. >> > At that point, dma_addr_t really has no sensible meaning for me. >> >> I think what you see here is a quite common hardware setup and we really >> lack the right abstraction for it at the moment. Everybody seems to >> work around it with a mix of the dma-mapping API and the iommu API. >> These are doing different things, and even though the dma-mapping API >> can be implemented on top of the iommu API, they are not really compatible. > > I'd go as far as saying that the "DMA API on top of IOMMU" is more > intended to be for a system IOMMU for the bus in question, rather > than a device-level IOMMU. > > If an IOMMU is part of a device, then the device should handle it > (maybe via an abstraction) and not via the DMA API. The DMA API should > be handing the bus addresses to the device driver which the device's > IOMMU would need to generate. (In other words, in this circumstance, > the DMA API shouldn't give you the device internal address.) if the dma_addr_t becomes the address upstream of the iommu (in practice, the phys addr), that would, I think, address my concerns about dma_addr_t BR, -R > -- > FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up > according to speedtest.net.
[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Tue, Feb 03, 2015 at 03:52:48PM +0100, Arnd Bergmann wrote: > On Tuesday 03 February 2015 14:41:09 Russell King - ARM Linux wrote: > > I'd go as far as saying that the "DMA API on top of IOMMU" is more > > intended to be for a system IOMMU for the bus in question, rather > > than a device-level IOMMU. > > > > If an IOMMU is part of a device, then the device should handle it > > (maybe via an abstraction) and not via the DMA API. The DMA API should > > be handing the bus addresses to the device driver which the device's > > IOMMU would need to generate. (In other words, in this circumstance, > > the DMA API shouldn't give you the device internal address.) > > Exactly. And the abstraction that people choose at the moment is the > iommu API, for better or worse. It makes a lot of sense to use this > API if the same iommu is used for other devices as well (which is > the case on Tegra and probably a lot of others). Unfortunately the > iommu API lacks support for cache management, and probably other things > as well, because this was not an issue for the original use case > (device assignment on KVM/x86). > > This could be done by adding explicit or implied cache management > to the IOMMU mapping interfaces, or by extending the dma-mapping > interfaces in a way that covers the use case of the device managing > its own address space, in addition to the existing coherent and > streaming interfaces. Don't we already have those in the DMA API? dma_sync_*() ? dma_map_sg() - sets up the system MMU and deals with initial cache coherency handling. Device IOMMU being the responsibility of the GPU driver. The GPU can then do dma_sync_*() on the scatterlist as is necessary to synchronise the cache coherency (while respecting the ownership rules - which are very important on ARM to follow as some sync()s are destructive to any dirty data in the CPU cache.) dma_unmap_sg() tears down the system MMU and deals with the final cache handling. Why do we need more DMA API interfaces? -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Tue, Feb 3, 2015 at 9:52 AM, Arnd Bergmann wrote: > On Tuesday 03 February 2015 14:41:09 Russell King - ARM Linux wrote: >> On Tue, Feb 03, 2015 at 03:17:27PM +0100, Arnd Bergmann wrote: >> > On Tuesday 03 February 2015 09:04:03 Rob Clark wrote: >> > > Since I'm stuck w/ an iommu, instead of built in mmu, my plan was to >> > > drop use of dma-mapping entirely (incl the current call to dma_map_sg, >> > > which I just need until we can use drm_cflush on arm), and >> > > attach/detach iommu domains directly to implement context switches. >> > > At that point, dma_addr_t really has no sensible meaning for me. >> > >> > I think what you see here is a quite common hardware setup and we really >> > lack the right abstraction for it at the moment. Everybody seems to >> > work around it with a mix of the dma-mapping API and the iommu API. >> > These are doing different things, and even though the dma-mapping API >> > can be implemented on top of the iommu API, they are not really compatible. >> >> I'd go as far as saying that the "DMA API on top of IOMMU" is more >> intended to be for a system IOMMU for the bus in question, rather >> than a device-level IOMMU. >> >> If an IOMMU is part of a device, then the device should handle it >> (maybe via an abstraction) and not via the DMA API. The DMA API should >> be handing the bus addresses to the device driver which the device's >> IOMMU would need to generate. (In other words, in this circumstance, >> the DMA API shouldn't give you the device internal address.) > > Exactly. And the abstraction that people choose at the moment is the > iommu API, for better or worse. It makes a lot of sense to use this > API if the same iommu is used for other devices as well (which is > the case on Tegra and probably a lot of others). Unfortunately the > iommu API lacks support for cache management, and probably other things > as well, because this was not an issue for the original use case > (device assignment on KVM/x86). > > This could be done by adding explicit or implied cache management > to the IOMMU mapping interfaces, or by extending the dma-mapping > interfaces in a way that covers the use case of the device managing > its own address space, in addition to the existing coherent and > streaming interfaces. I think for gpu's, we'd prefer explicit and less abstraction.. which is probably opposite of what every other driver would want In the end, my eventual goal is explicit control of tlb flush, and control of my address space. And in fact in some cases we are going to want to use the gpu to bang on iommu registers to do context switches and tlb flushes. (Which is obviously not the first step.. and something that is fairly difficult to get right/secure.. but the performance win seems significant so I'm not sure we can avoid it.) BR, -R > > Arnd
[Linaro-mm-sig] [RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Tuesday 03 February 2015 15:22:05 Russell King - ARM Linux wrote: > On Tue, Feb 03, 2015 at 03:52:48PM +0100, Arnd Bergmann wrote: > > On Tuesday 03 February 2015 14:41:09 Russell King - ARM Linux wrote: > > > I'd go as far as saying that the "DMA API on top of IOMMU" is more > > > intended to be for a system IOMMU for the bus in question, rather > > > than a device-level IOMMU. > > > > > > If an IOMMU is part of a device, then the device should handle it > > > (maybe via an abstraction) and not via the DMA API. The DMA API should > > > be handing the bus addresses to the device driver which the device's > > > IOMMU would need to generate. (In other words, in this circumstance, > > > the DMA API shouldn't give you the device internal address.) > > > > Exactly. And the abstraction that people choose at the moment is the > > iommu API, for better or worse. It makes a lot of sense to use this > > API if the same iommu is used for other devices as well (which is > > the case on Tegra and probably a lot of others). Unfortunately the > > iommu API lacks support for cache management, and probably other things > > as well, because this was not an issue for the original use case > > (device assignment on KVM/x86). > > > > This could be done by adding explicit or implied cache management > > to the IOMMU mapping interfaces, or by extending the dma-mapping > > interfaces in a way that covers the use case of the device managing > > its own address space, in addition to the existing coherent and > > streaming interfaces. > > Don't we already have those in the DMA API? dma_sync_*() ? > > dma_map_sg() - sets up the system MMU and deals with initial cache > coherency handling. Device IOMMU being the responsibility of the > GPU driver. dma_sync_*() works with whatever comes out of dma_map_*(), true, but this is not what they want to do here. > The GPU can then do dma_sync_*() on the scatterlist as is necessary > to synchronise the cache coherency (while respecting the ownership > rules - which are very important on ARM to follow as some sync()s are > destructive to any dirty data in the CPU cache.) > > dma_unmap_sg() tears down the system MMU and deals with the final cache > handling. > > Why do we need more DMA API interfaces? The dma_map_* interfaces assign the virtual addresses internally, using typically either a global address space for all devices, or one address space per device. There are multiple things that this cannot do, and that is why the drivers use the iommu API directly: - use one address space per 'struct mm' - map user memory with bus_address == user_address - map memory into the GPU without having a permanent kernel mapping - map memory first, and do the initial cache flushes later Arnd
[Bug 87023] Dota 2 crashed and hang on ATi Mobility Radeon HD 5650 (Acer 4745G)
https://bugs.freedesktop.org/show_bug.cgi?id=87023 Romy changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #14 from Romy --- After I'm playing around with installing various distros until Windows, not only Dota2 are crashed, but another games too, but not whole games. It's not overheat issues, nor drivers nor OS, but I have adressed it that the memory (RAM) causing this problem. I have 2 x 4GB 1600 Sodimm RAM, so that I'm disasembling one of them, leaving 1 x 4GB 1600 Sodimm. That configuration doesn't making my pc crash anymore so far. I mark this bug as RESOLVED. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/cbbe19b7/attachment.html>
[PATCH] RFC: drm: add support for tiled/compressed/etc modifier in addfb2
On Fri, Jan 30, 2015 at 05:08:23PM +0100, Daniel Vetter wrote:
> From: Rob Clark
>
> In DRM/KMS we are lacking a good way to deal with tiled/compressed
> formats. Especially in the case of dmabuf/prime buffer sharing, where
> we cannot always rely on under-the-hood flags passed to driver specific
> gem-create ioctl to pass around these extra flags.
>
> The proposal is to add a per-plane format modifier. This allows to, if
> necessary, use different tiling patters for sub-sampled planes, etc.
"patterns". Alternatively perhaps "modes", which is how I've heard it
referred to most commonly.
> The format modifiers are added at the end of the ioctl struct, so for
> legacy userspace it will be zero padded.
>
> v1: original
> v1.5: increase modifier to 64b
>
> v2: Incorporate review comments from the big thread, plus a few more.
>
> - Add a getcap so that userspace doesn't have to jump through hoops.
> - Allow modifiers only when a flag is set. That way drivers know when
> they're dealing with old userspace and need to fish out e.g. tiling
> from other information.
> - After rolling out checks for ->modifier to all drivers I've decided
> that this is way too fragile and needs an explicit opt-in flag. So
> do that instead.
> - Add a define (just for documentation really) for the "NONE"
> modifier. Imo we don't need to add mask #defines since drivers
> really should only do exact matches against values defined with
> fourcc_mod_code.
> - Drop the Samsung tiling modifier on Rob's request since he's not yet
> sure whether that one is accurate.
>
> v3:
> - Also add a new ->modifier[] array to struct drm_framebuffer and fill
> it in drm_helper_mode_fill_fb_struct. Requested by Tvrtko Uruslin.
> - Remove TODO in comment and add code comment that modifiers should be
> properly documented, requested by Rob.
>
> v4: Balance parens, spotted by Tvrtko.
>
> Cc: Rob Clark
> Cc: Tvrtko Ursulin
> Cc: Laurent Pinchart
> Cc: Daniel Stone
> Cc: Ville Syrjälä
> Cc: Michel Dänzer
> Signed-off-by: Rob Clark (v1.5)
> Reviewed-by: Rob Clark
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_crtc.c| 14 +-
> drivers/gpu/drm/drm_crtc_helper.c | 1 +
> drivers/gpu/drm/drm_ioctl.c | 3 +++
> include/drm/drm_crtc.h| 4
> include/uapi/drm/drm.h| 1 +
> include/uapi/drm/drm_fourcc.h | 32
> include/uapi/drm/drm_mode.h | 9 +
> 7 files changed, 63 insertions(+), 1 deletion(-)
Also as discussed on IRC, I think this would be better in a non-DRM
specific header so that we can have a central, cross-subsystem
authority.
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 646ae5f39f42..622109677747 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -132,4 +132,36 @@
> #define DRM_FORMAT_YUV444fourcc_code('Y', 'U', '2', '4') /*
> non-subsampled Cb (1) and Cr (2) planes */
> #define DRM_FORMAT_YVU444fourcc_code('Y', 'V', '2', '4') /*
> non-subsampled Cr (1) and Cb (2) planes */
>
> +
Possibly unintented extra blank line?
> +/*
> + * Format Modifiers:
> + *
> + * Format modifiers describe, typically, a re-ordering or modification
> + * of the data in a plane of an FB. This can be used to express tiled/
> + * swizzled formats, or compression, or a combination of the two.
> + *
> + * The upper 8 bits of the format modifier are a vendor-id as assigned
> + * below. The lower 56 bits are assigned as vendor sees fit.
> + */
> +
> +/* Vendor Ids: */
> +#define DRM_FORMAT_MOD_NONE 0
> +#define DRM_FORMAT_MOD_VENDOR_INTEL 0x01
> +#define DRM_FORMAT_MOD_VENDOR_AMD 0x02
> +#define DRM_FORMAT_MOD_VENDOR_NV 0x03
I think this should be NVIDIA for consistency with other naming in the
kernel, at least on Tegra.
Otherwise:
Reviewed-by: Thierry Reding
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/b14854ef/attachment-0001.sig>
Fwd: 3.18.3->4 : external monitor is black after wakeup of a docked system after s2ram
This commits breaks a working configuration here - reverting it let both version 3.18.4 and 3.18.5 working fine again at a hardened Gentoo Linux commit ddf6f9a4c9fb1c72ea2e8d196c9a580a8e914dbd Author: Dave Airlie Date: Mon Dec 8 13:23:37 2014 +1000 drm/i915: resume MST after reading back hw state commit e7d6f7d708290da1b7c92f533444b042c79412e0 upstream. Otherwise the MST resume paths can hit DPMS paths which hit state checker paths, which hit WARN_ON, because the state checker is inconsistent with the hw. This fixes a bunch of WARN_ON's on resume after undocking. Signed-off-by: Dave Airlie Reviewed-by: Daniel Vetter Signed-off-by: Jani Nikula Signed-off-by: Greg Kroah-Hartman On 02/02/2015 07:24 PM, Toralf Förster wrote: > In the mean while I have to correct it - it is reliable. > So something between 3.18.3 and 3.18.4 break a docking system configuration > in that manenr, that after resume the external (iand internal X11 screen) is > black. > > > Forwarded Message > Subject: 3.18.3->4 : external monitor is black after wakeup of a docked > system after s2ram > Date: Sat, 31 Jan 2015 16:47:45 +0100 > From: Toralf Förster > To: Linux Kernel > > Happened 2 times here (but not always) that a docked Lenovo T440s (intel > graphic) doesn't show anything at the external display (KDE 4.13.3). System > is a hardened stable 64 bit Gentoo Linux > > Nothing suspicious in the log however. > > Known issue ? > -- Toralf pgp key: 7B1A 07F4 EC82 0F90 D4C2 8936 872A E508 0076 E94E
[PATCH] drm: remove DRM_FORMAT_NV12MT
So this has been merged originally in commit 83052d4d5cd518332440bb4ee63d68bb5f744e0f Author: Seung-Woo Kim Date: Thu Dec 15 15:40:55 2011 +0900 drm: Add multi buffer plane pixel formats which hasn't seen a lot of review really. The problem is that it's not a real pixel format, but just a different way to lay out NV12 pixels in macroblocks, i.e. a tiling format. The new way of doing this is with the soon-to-be-merge fb modifiers. This was brough up in some long irc discussion around the entire topic, as an example of where things have gone wrong. Luckily we can correct the mistake: - The kms side support for NV12MT is all dead code because format_check in drm_crtc.c never accepted NV12MT. - The gem side for the gsc support doesn't look better: The code forgets to set the pixel format and makes a big mess with the tiling mode bits, inadvertedly setting them all. Conclusion: This never really worked (at least not in upstream) and hence we can savely correct our mistake here. Cc: Seung-Woo Kim Cc: Inki Dae Cc: Kyungmin Park Cc: Rob Clark Cc: Daniel Stone Cc: Damien Lespiau Signed-off-by: Daniel Vetter --- drivers/gpu/drm/exynos/exynos_drm_fimc.c | 14 ++ drivers/gpu/drm/exynos/exynos_drm_gsc.c | 6 -- drivers/gpu/drm/exynos/exynos_drm_plane.c | 1 - drivers/gpu/drm/exynos/exynos_mixer.c | 2 -- include/uapi/drm/drm_fourcc.h | 3 --- 5 files changed, 2 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c index 835b6af00970..842d6b8dc3c4 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c @@ -461,7 +461,6 @@ static int fimc_src_set_fmt_order(struct fimc_context *ctx, u32 fmt) cfg |= EXYNOS_MSCTRL_C_INT_IN_3PLANE; break; case DRM_FORMAT_NV12: - case DRM_FORMAT_NV12MT: case DRM_FORMAT_NV16: cfg |= (EXYNOS_MSCTRL_ORDER2P_LSB_CBCR | EXYNOS_MSCTRL_C_INT_IN_2PLANE); @@ -511,7 +510,6 @@ static int fimc_src_set_fmt(struct device *dev, u32 fmt) case DRM_FORMAT_YVU420: case DRM_FORMAT_NV12: case DRM_FORMAT_NV21: - case DRM_FORMAT_NV12MT: cfg |= EXYNOS_MSCTRL_INFORMAT_YCBCR420; break; default: @@ -524,10 +522,7 @@ static int fimc_src_set_fmt(struct device *dev, u32 fmt) cfg = fimc_read(ctx, EXYNOS_CIDMAPARAM); cfg &= ~EXYNOS_CIDMAPARAM_R_MODE_MASK; - if (fmt == DRM_FORMAT_NV12MT) - cfg |= EXYNOS_CIDMAPARAM_R_MODE_64X32; - else - cfg |= EXYNOS_CIDMAPARAM_R_MODE_LINEAR; + cfg |= EXYNOS_CIDMAPARAM_R_MODE_LINEAR; fimc_write(ctx, cfg, EXYNOS_CIDMAPARAM); @@ -812,7 +807,6 @@ static int fimc_dst_set_fmt_order(struct fimc_context *ctx, u32 fmt) cfg |= EXYNOS_CIOCTRL_YCBCR_3PLANE; break; case DRM_FORMAT_NV12: - case DRM_FORMAT_NV12MT: case DRM_FORMAT_NV16: cfg |= EXYNOS_CIOCTRL_ORDER2P_LSB_CBCR; cfg |= EXYNOS_CIOCTRL_YCBCR_2PLANE; @@ -867,7 +861,6 @@ static int fimc_dst_set_fmt(struct device *dev, u32 fmt) case DRM_FORMAT_YUV420: case DRM_FORMAT_YVU420: case DRM_FORMAT_NV12: - case DRM_FORMAT_NV12MT: case DRM_FORMAT_NV21: cfg |= EXYNOS_CITRGFMT_OUTFORMAT_YCBCR420; break; @@ -883,10 +876,7 @@ static int fimc_dst_set_fmt(struct device *dev, u32 fmt) cfg = fimc_read(ctx, EXYNOS_CIDMAPARAM); cfg &= ~EXYNOS_CIDMAPARAM_W_MODE_MASK; - if (fmt == DRM_FORMAT_NV12MT) - cfg |= EXYNOS_CIDMAPARAM_W_MODE_64X32; - else - cfg |= EXYNOS_CIDMAPARAM_W_MODE_LINEAR; + cfg |= EXYNOS_CIDMAPARAM_W_MODE_LINEAR; fimc_write(ctx, cfg, EXYNOS_CIDMAPARAM); diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c b/drivers/gpu/drm/exynos/exynos_drm_gsc.c index 0261468c8019..8040ed2a831f 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c @@ -542,9 +542,6 @@ static int gsc_src_set_fmt(struct device *dev, u32 fmt) cfg |= (GSC_IN_CHROMA_ORDER_CBCR | GSC_IN_YUV420_2P); break; - case DRM_FORMAT_NV12MT: - cfg |= (GSC_IN_TILE_C_16x8 | GSC_IN_TILE_MODE); - break; default: dev_err(ippdrv->dev, "inavlid target yuv order 0x%x.\n", fmt); return -EINVAL; @@ -809,9 +806,6 @@ static int gsc_dst_set_fmt(struct device *dev, u32 fmt) cfg |= (GSC_OUT_CHROMA_ORDER_CBCR | GSC_OUT_YUV420_2P); break; - case DRM_FORMAT_NV12MT: - cfg |= (GSC_OUT_TILE_C_16x8 | GSC_OUT_TILE_MODE); - break; default: dev
[Linaro-mm-sig] [RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Tue, Feb 03, 2015 at 04:31:13PM +0100, Arnd Bergmann wrote: > On Tuesday 03 February 2015 15:22:05 Russell King - ARM Linux wrote: > > Don't we already have those in the DMA API? dma_sync_*() ? > > > > dma_map_sg() - sets up the system MMU and deals with initial cache > > coherency handling. Device IOMMU being the responsibility of the > > GPU driver. > > dma_sync_*() works with whatever comes out of dma_map_*(), true, > but this is not what they want to do here. > > > The GPU can then do dma_sync_*() on the scatterlist as is necessary > > to synchronise the cache coherency (while respecting the ownership > > rules - which are very important on ARM to follow as some sync()s are > > destructive to any dirty data in the CPU cache.) > > > > dma_unmap_sg() tears down the system MMU and deals with the final cache > > handling. > > > > Why do we need more DMA API interfaces? > > The dma_map_* interfaces assign the virtual addresses internally, > using typically either a global address space for all devices, or one > address space per device. We shouldn't be doing one address space per device for precisely this reason. We should be doing one address space per *bus*. I did have a nice diagram to illustrate the point in my previous email, but I deleted it, I wish I hadn't... briefly: Fig. 1. +--+ |+-+ device | CPU--L1cache--L2cache--Memory--SysMMU---IOMMU--> | |+-+ | +--+ Fig.1 represents what I'd call the "GPU" issue that we're talking about in this thread. Fig. 2. CPU--L1cache--L2cache--Memory--SysMMU-IOMMU--device The DMA API should be responsible (at the very least) for everything on the left of "" in and should be providing a dma_addr_t which is representative of what the device (in Fig.1) as a whole sees. That's the "system" part. I believe this is the approach which is taken by x86 and similar platforms, simply because they tend not to have an IOMMU on individual devices (and if they did, eg, on a PCI card, it's clearly the responsibility of the device driver.) Whether the DMA API also handles the IOMMU in Fig.1 or 2 is questionable. For fig.2, it is entirely possible that the same device could appear without an IOMMU, and in that scenario, you would want the IOMMU to be handled transparently. However, by doing so for everything, you run into exactly the problem which is being discussed here - the need to separate out the cache coherency from the IOMMU aspects. You probably also have a setup very similar to fig.1 (which is certainly true of Vivante GPUs.) If you have the need to separately control both, then using the DMA API to encapsulate both does not make sense - at which point, the DMA API should be responsible for the minimum only - in other words, everything to the left of (so including the system MMU.) The control of the device IOMMU should be the responsibility of device driver in this case. So, dma_map_sg() would be responsible for dealing with the CPU cache coherency issues, and setting up the system MMU. dma_sync_*() would be responsible for the CPU cache coherency issues, and dma_unmap_sg() would (again) deal with the CPU cache and tear down the system MMU mappings. Meanwhile, the device driver has ultimate control over its IOMMU, the creation and destruction of mappings and context switches at the appropriate times. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
[Bug 77784] Certain mono based games hang the system with radeonsi (probably llvm backend)
https://bugs.freedesktop.org/show_bug.cgi?id=77784 --- Comment #7 from José Suárez --- Judging from my last testing of both Foosball: street edition and LITTLE racers street this problem does not show with mesa 10.5 git with llvm 3.6 (oibaf's ppa), so I guess it could be marked as solved. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/79effc58/attachment.html>
[Bug 77784] Certain mono based games hang the system with radeonsi (probably llvm backend)
https://bugs.freedesktop.org/show_bug.cgi?id=77784 Alex Deucher changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #8 from Alex Deucher --- You can mark it as resolved as well. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/ae3a0df0/attachment.html>
[Linaro-mm-sig] [RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Tuesday 03 February 2015 15:54:04 Russell King - ARM Linux wrote: > On Tue, Feb 03, 2015 at 04:31:13PM +0100, Arnd Bergmann wrote: > > The dma_map_* interfaces assign the virtual addresses internally, > > using typically either a global address space for all devices, or one > > address space per device. > > We shouldn't be doing one address space per device for precisely this > reason. We should be doing one address space per *bus*. I did have > a nice diagram to illustrate the point in my previous email, but I > deleted it, I wish I hadn't... briefly: > > Fig. 1. > +--+ > |+-+ device | > CPU--L1cache--L2cache--Memory--SysMMU---IOMMU--> | > |+-+ | > +--+ > > Fig.1 represents what I'd call the "GPU" issue that we're talking about > in this thread. > > Fig. 2. > CPU--L1cache--L2cache--Memory--SysMMU-IOMMU--device > > The DMA API should be responsible (at the very least) for everything on > the left of "" in and should be providing a dma_addr_t which is > representative of what the device (in Fig.1) as a whole sees. That's > the "system" part. > > I believe this is the approach which is taken by x86 and similar platforms, > simply because they tend not to have an IOMMU on individual devices (and > if they did, eg, on a PCI card, it's clearly the responsibility of the > device driver.) > > Whether the DMA API also handles the IOMMU in Fig.1 or 2 is questionable. > For fig.2, it is entirely possible that the same device could appear > without an IOMMU, and in that scenario, you would want the IOMMU to be > handled transparently. > > However, by doing so for everything, you run into exactly the problem > which is being discussed here - the need to separate out the cache > coherency from the IOMMU aspects. You probably also have a setup very > similar to fig.1 (which is certainly true of Vivante GPUs.) > > If you have the need to separately control both, then using the DMA API > to encapsulate both does not make sense - at which point, the DMA API > should be responsible for the minimum only - in other words, everything > to the left of (so including the system MMU.) The control of > the device IOMMU should be the responsibility of device driver in this > case. > > So, dma_map_sg() would be responsible for dealing with the CPU cache > coherency issues, and setting up the system MMU. dma_sync_*() would > be responsible for the CPU cache coherency issues, and dma_unmap_sg() > would (again) deal with the CPU cache and tear down the system MMU > mappings. > > Meanwhile, the device driver has ultimate control over its IOMMU, the > creation and destruction of mappings and context switches at the > appropriate times. I agree for the case you are describing here. From what I understood from Rob was that he is looking at something more like: Fig 3 CPU--L1cache--L2cache--Memory--IOMMU-device where the IOMMU controls one or more contexts per device, and is shared across GPU and non-GPU devices. Here, we need to use the dmap-mapping interface to set up the IO page table for any device that is unable to address all of system RAM, and we can use it for purposes like isolation of the devices. There are also cases where using the IOMMU is not optional. So unlike the scenario you describe, the driver cannot at the same time control the cache (using the dma-mapping API) and the I/O page tables (using the iommu API or some internal functions). Arnd
[PATCH] drm: remove DRM_FORMAT_NV12MT
2015-02-03 Daniel Vetter : > So this has been merged originally in > > commit 83052d4d5cd518332440bb4ee63d68bb5f744e0f > Author: Seung-Woo Kim > Date: Thu Dec 15 15:40:55 2011 +0900 > > drm: Add multi buffer plane pixel formats > > which hasn't seen a lot of review really. The problem is that it's not > a real pixel format, but just a different way to lay out NV12 pixels > in macroblocks, i.e. a tiling format. > > The new way of doing this is with the soon-to-be-merge fb modifiers. > > This was brough up in some long irc discussion around the entire > topic, as an example of where things have gone wrong. Luckily we can > correct the mistake: > - The kms side support for NV12MT is all dead code because > format_check in drm_crtc.c never accepted NV12MT. > - The gem side for the gsc support doesn't look better: The code > forgets to set the pixel format and makes a big mess with the tiling > mode bits, inadvertedly setting them all. > > Conclusion: This never really worked (at least not in upstream) and > hence we can savely correct our mistake here. > > Cc: Seung-Woo Kim > Cc: Inki Dae > Cc: Kyungmin Park > Cc: Rob Clark > Cc: Daniel Stone > Cc: Damien Lespiau > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/exynos/exynos_drm_fimc.c | 14 ++ > drivers/gpu/drm/exynos/exynos_drm_gsc.c | 6 -- > drivers/gpu/drm/exynos/exynos_drm_plane.c | 1 - > drivers/gpu/drm/exynos/exynos_mixer.c | 2 -- > include/uapi/drm/drm_fourcc.h | 3 --- > 5 files changed, 2 insertions(+), 24 deletions(-) That seems good to me. Reviewed-by: Gustavo Padovan Gustavo
[Linaro-mm-sig] [RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Tue, Feb 3, 2015 at 11:12 AM, Arnd Bergmann wrote: > I agree for the case you are describing here. From what I understood > from Rob was that he is looking at something more like: > > Fig 3 > CPU--L1cache--L2cache--Memory--IOMMU-device > > where the IOMMU controls one or more contexts per device, and is > shared across GPU and non-GPU devices. Here, we need to use the > dmap-mapping interface to set up the IO page table for any device > that is unable to address all of system RAM, and we can use it > for purposes like isolation of the devices. There are also cases > where using the IOMMU is not optional. Actually, just to clarify, the IOMMU instance is specific to the GPU.. not shared with other devices. Otherwise managing multiple contexts would go quite badly.. But other devices have their own instance of the same IOMMU.. so same driver could be used. BR, -R
[Linaro-mm-sig] [RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Tuesday 03 February 2015 11:22:01 Rob Clark wrote: > On Tue, Feb 3, 2015 at 11:12 AM, Arnd Bergmann wrote: > > I agree for the case you are describing here. From what I understood > > from Rob was that he is looking at something more like: > > > > Fig 3 > > CPU--L1cache--L2cache--Memory--IOMMU-device > > > > where the IOMMU controls one or more contexts per device, and is > > shared across GPU and non-GPU devices. Here, we need to use the > > dmap-mapping interface to set up the IO page table for any device > > that is unable to address all of system RAM, and we can use it > > for purposes like isolation of the devices. There are also cases > > where using the IOMMU is not optional. > > > Actually, just to clarify, the IOMMU instance is specific to the GPU.. > not shared with other devices. Otherwise managing multiple contexts > would go quite badly.. > > But other devices have their own instance of the same IOMMU.. so same > driver could be used. I think from the driver perspective, I'd view those two cases as identical. Not sure if Russell agrees with that. Arnd
[PATCH] drm/cirrus: Limit modes depending on bpp option
The commit [8975626ea35a: drm/cirrus: allow 32bpp framebuffers for
cirrus drm] broke X modesetting driver because cirrus driver still
provides the full list of modes up to 1280x1024 while the 32bpp can
support only up to 800x600.
We might be able to filter out the invalid modes in mode_valid
callback, but unfortunately the bpp in question can't be referred
there for now (let me know if there is a better way to retrieve the
bpp for the probed fb).
So, instead, this patch adds the bpp module option to specify the
maximal bpp explicitly and limits the resolutions in get_modes
depending on its value.
The default value is set to 24 so that the existing stuff keeps
working. If you need a new 32bpp feature, specify cirrus.bpp=32
option explicitly.
Fixes: 8975626ea35a ('drm/cirrus: allow 32bpp framebuffers for cirrus drm')
Signed-off-by: Takashi Iwai
---
Dave, this is a regression on 3.19-rc. It'd be great if you can take it
(or fix in a different form) before 3.19-final.
thanks,
Takashi
drivers/gpu/drm/cirrus/cirrus_drv.c | 3 +++
drivers/gpu/drm/cirrus/cirrus_drv.h | 3 +++
drivers/gpu/drm/cirrus/cirrus_main.c | 2 ++
drivers/gpu/drm/cirrus/cirrus_mode.c | 9 +++--
4 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.c
b/drivers/gpu/drm/cirrus/cirrus_drv.c
index c2a1cba1e984..b9140032962d 100644
--- a/drivers/gpu/drm/cirrus/cirrus_drv.c
+++ b/drivers/gpu/drm/cirrus/cirrus_drv.c
@@ -16,9 +16,12 @@
#include "cirrus_drv.h"
int cirrus_modeset = -1;
+int cirrus_bpp = 24;
MODULE_PARM_DESC(modeset, "Disable/Enable modesetting");
module_param_named(modeset, cirrus_modeset, int, 0400);
+MODULE_PARM_DESC(bpp, "Max bits-per-pixel (default:24)");
+module_param_named(bpp, cirrus_bpp, int, 0400);
/*
* This is the generic driver code. This binds the driver to the drm core,
diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.h
b/drivers/gpu/drm/cirrus/cirrus_drv.h
index 693a4565c4ff..705061537a27 100644
--- a/drivers/gpu/drm/cirrus/cirrus_drv.h
+++ b/drivers/gpu/drm/cirrus/cirrus_drv.h
@@ -262,4 +262,7 @@ static inline void cirrus_bo_unreserve(struct cirrus_bo *bo)
int cirrus_bo_push_sysram(struct cirrus_bo *bo);
int cirrus_bo_pin(struct cirrus_bo *bo, u32 pl_flag, u64 *gpu_addr);
+
+extern int cirrus_bpp;
+
#endif /* __CIRRUS_DRV_H__ */
diff --git a/drivers/gpu/drm/cirrus/cirrus_main.c
b/drivers/gpu/drm/cirrus/cirrus_main.c
index 4c2d68e9102d..e4b976658087 100644
--- a/drivers/gpu/drm/cirrus/cirrus_main.c
+++ b/drivers/gpu/drm/cirrus/cirrus_main.c
@@ -320,6 +320,8 @@ bool cirrus_check_framebuffer(struct cirrus_device *cdev,
int width, int height,
const int max_pitch = 0x1FF << 3; /* (4096 - 1) & ~111b bytes */
const int max_size = cdev->mc.vram_size;
+ if (bpp > cirrus_bpp)
+ return false;
if (bpp > 32)
return false;
diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c
b/drivers/gpu/drm/cirrus/cirrus_mode.c
index 99d4a74ffeaf..61385f2298bf 100644
--- a/drivers/gpu/drm/cirrus/cirrus_mode.c
+++ b/drivers/gpu/drm/cirrus/cirrus_mode.c
@@ -501,8 +501,13 @@ static int cirrus_vga_get_modes(struct drm_connector
*connector)
int count;
/* Just add a static list of modes */
- count = drm_add_modes_noedid(connector, 1280, 1024);
- drm_set_preferred_mode(connector, 1024, 768);
+ if (cirrus_bpp <= 24) {
+ count = drm_add_modes_noedid(connector, 1280, 1024);
+ drm_set_preferred_mode(connector, 1024, 768);
+ } else {
+ count = drm_add_modes_noedid(connector, 800, 600);
+ drm_set_preferred_mode(connector, 800, 600);
+ }
return count;
}
--
2.2.2
[PATCH 1/3] drm/panel: Add display_timing support
Hi Thierry,
Am Dienstag, den 03.02.2015, 14:30 +0100 schrieb Thierry Reding:
> On Thu, Dec 11, 2014 at 06:32:44PM +0100, Philipp Zabel wrote:
> > Many panel data sheets additionally to typical values list allowed ranges
> > for
> > timings such as hsync/vsync lengths, porches, and the pixel clock rate.
> > These
> > can be stored in a struct display_timing, to be used by an encoder
> > mode_fixup
> > callback to clamp user provided timing values or to validate workarounds for
> > clock source limitations.
> >
> > This patch adds a new drm_panel_funcs callback that returns the panels'
> > available display_timing entries.
> >
> > Signed-off-by: Philipp Zabel
> > ---
> > include/drm/drm_panel.h | 5 +
> > 1 file changed, 5 insertions(+)
>
> Sorry for taking so long to get back on this. I generally like the idea,
> though a couple of things are unclear to me.
>
> In struct display_timing we have:
>
> struct timing_entry hactive;
> ...
> struct timing_entry vactive;
>
> I wonder if that really makes sense. Are there really datasheets that
> provide a valid range for the number of horizontal and vertical active
> pixels?
I often see datasheets that give minimum, typical and maximum values
also for the horizontal and vertical active pixels, but those are
usually the same value. I don't know if there are any panels that really
have documented ranges here.
[...]
> > @@ -68,6 +71,8 @@ struct drm_panel_funcs {
> > int (*prepare)(struct drm_panel *panel);
> > int (*enable)(struct drm_panel *panel);
> > int (*get_modes)(struct drm_panel *panel);
> > + int (*get_timings)(struct drm_panel *panel, unsigned int num_timings,
> > + struct display_timing *timings);
>
> Also are there even panels that support more than one set of timings?
> I've never heard of panels that are actually able to do more than one
> mode, so I'm wondering if num_timings isn't overdoing it a little here.
> I guess it doesn't hurt all that much, though.
I have not seen any yet that reasonably should be driven by the
simple-panel driver. I was thinking about the Pixel Qi panel, but it
turns out those just have a single timing and use the subpixels in B/W
reflective mode.
regards
Philipp
[Linaro-mm-sig] [RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Tue, Feb 03, 2015 at 05:12:40PM +0100, Arnd Bergmann wrote: > On Tuesday 03 February 2015 15:54:04 Russell King - ARM Linux wrote: > > On Tue, Feb 03, 2015 at 04:31:13PM +0100, Arnd Bergmann wrote: > > > The dma_map_* interfaces assign the virtual addresses internally, > > > using typically either a global address space for all devices, or one > > > address space per device. > > > > We shouldn't be doing one address space per device for precisely this > > reason. We should be doing one address space per *bus*. I did have > > a nice diagram to illustrate the point in my previous email, but I > > deleted it, I wish I hadn't... briefly: > > > > Fig. 1. > > +--+ > > |+-+ device | > > CPU--L1cache--L2cache--Memory--SysMMU---IOMMU--> | > > |+-+ | > > +--+ > > > > Fig.1 represents what I'd call the "GPU" issue that we're talking about > > in this thread. > > > > Fig. 2. > > CPU--L1cache--L2cache--Memory--SysMMU-IOMMU--device > > > > The DMA API should be responsible (at the very least) for everything on > > the left of "" in and should be providing a dma_addr_t which is > > representative of what the device (in Fig.1) as a whole sees. That's > > the "system" part. > > > > I believe this is the approach which is taken by x86 and similar platforms, > > simply because they tend not to have an IOMMU on individual devices (and > > if they did, eg, on a PCI card, it's clearly the responsibility of the > > device driver.) > > > > Whether the DMA API also handles the IOMMU in Fig.1 or 2 is questionable. > > For fig.2, it is entirely possible that the same device could appear > > without an IOMMU, and in that scenario, you would want the IOMMU to be > > handled transparently. > > > > However, by doing so for everything, you run into exactly the problem > > which is being discussed here - the need to separate out the cache > > coherency from the IOMMU aspects. You probably also have a setup very > > similar to fig.1 (which is certainly true of Vivante GPUs.) > > > > If you have the need to separately control both, then using the DMA API > > to encapsulate both does not make sense - at which point, the DMA API > > should be responsible for the minimum only - in other words, everything > > to the left of (so including the system MMU.) The control of > > the device IOMMU should be the responsibility of device driver in this > > case. > > > > So, dma_map_sg() would be responsible for dealing with the CPU cache > > coherency issues, and setting up the system MMU. dma_sync_*() would > > be responsible for the CPU cache coherency issues, and dma_unmap_sg() > > would (again) deal with the CPU cache and tear down the system MMU > > mappings. > > > > Meanwhile, the device driver has ultimate control over its IOMMU, the > > creation and destruction of mappings and context switches at the > > appropriate times. > > I agree for the case you are describing here. From what I understood > from Rob was that he is looking at something more like: > > Fig 3 > CPU--L1cache--L2cache--Memory--IOMMU-device > > where the IOMMU controls one or more contexts per device, and is > shared across GPU and non-GPU devices. Here, we need to use the > dmap-mapping interface to set up the IO page table for any device > that is unable to address all of system RAM, and we can use it > for purposes like isolation of the devices. There are also cases > where using the IOMMU is not optional. Okay, but switching contexts is not something which the DMA API has any knowledge of (so it can't know which context to associate with which mapping.) While it knows which device, it has no knowledge (nor is there any way for it to gain knowledge) about contexts. My personal view is that extending the DMA API in this way feels quite dirty - it's a violation of the DMA API design, which is to (a) demark the buffer ownership between CPU and DMA agent, and (b) to translate buffer locations into a cookie which device drivers can use to instruct their device to access that memory. To see why, consider... that you map a buffer to a device in context A, and then you switch to context B, which means the dma_addr_t given previously is no longer valid. You then try to unmap it... which is normally done using the (now no longer valid) dma_addr_t. It seems to me that to support this at DMA API level, we would need to completely revamp the DMA API, which IMHO isn't going to be nice. (It would mean that we end up with three APIs - the original PCI DMA API, the existing DMA API, and some new DMA API.) Do we have any views on how common this feature is? -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
[Linaro-mm-sig] [RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Tue, Feb 03, 2015 at 11:22:01AM -0500, Rob Clark wrote: > On Tue, Feb 3, 2015 at 11:12 AM, Arnd Bergmann wrote: > > I agree for the case you are describing here. From what I understood > > from Rob was that he is looking at something more like: > > > > Fig 3 > > CPU--L1cache--L2cache--Memory--IOMMU-device > > > > where the IOMMU controls one or more contexts per device, and is > > shared across GPU and non-GPU devices. Here, we need to use the > > dmap-mapping interface to set up the IO page table for any device > > that is unable to address all of system RAM, and we can use it > > for purposes like isolation of the devices. There are also cases > > where using the IOMMU is not optional. > > > Actually, just to clarify, the IOMMU instance is specific to the GPU.. > not shared with other devices. Otherwise managing multiple contexts > would go quite badly.. > > But other devices have their own instance of the same IOMMU.. so same > driver could be used. Okay, so that is my Fig.2 case, and we don't have to worry about Fig.3. One thing I forgot in Fig.1/2 which my original did have were to mark the system MMU as optional. (Think an ARM64 with SMMU into a 32-bit peripheral bus.) Do we support stacked MMUs in the DMA API? We may need to if we keep IOMMUs in the DMA API. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
[Bug 88947] "Unknown" display appears with no external display connected
https://bugs.freedesktop.org/show_bug.cgi?id=88947 Bug ID: 88947 Summary: "Unknown" display appears with no external display connected Product: DRI Version: DRI git Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: General Assignee: dri-devel at lists.freedesktop.org Reporter: ypwong at gmail.com 1. boot to desktop with no external monitor connected 2. Open the "Display" utility, check whether there is an "Unknown" display. You will be able to move the cursor out of your screen, even open an application on the extended display. Tested with 2015-01-29 drm-intel-nightly. 00:02.0 VGA compatible controller [0300]: Intel Corporation 2nd Generation Core Processor Family Integrated Graphics Controller [8086:0106] (rev 09) (prog-if 00 [VGA controller]) 01:00.0 VGA compatible controller [0300]: NVIDIA Corporation GF108M [GeForce GT 630M] [10de:0de9] (rev a1) (prog-if 00 [VGA controller]) Original bug: https://bugs.launchpad.net/ubuntu/+source/linux-lts-trusty/+bug/1355562 -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150203/f77155e2/attachment.html>
[Linaro-mm-sig] [RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Tue, Feb 3, 2015 at 11:58 AM, Russell King - ARM Linux wrote: > > Okay, but switching contexts is not something which the DMA API has > any knowledge of (so it can't know which context to associate with > which mapping.) While it knows which device, it has no knowledge > (nor is there any way for it to gain knowledge) about contexts. > > My personal view is that extending the DMA API in this way feels quite > dirty - it's a violation of the DMA API design, which is to (a) demark > the buffer ownership between CPU and DMA agent, and (b) to translate > buffer locations into a cookie which device drivers can use to instruct > their device to access that memory. To see why, consider... that you > map a buffer to a device in context A, and then you switch to context B, > which means the dma_addr_t given previously is no longer valid. You > then try to unmap it... which is normally done using the (now no longer > valid) dma_addr_t. > > It seems to me that to support this at DMA API level, we would need to > completely revamp the DMA API, which IMHO isn't going to be nice. (It > would mean that we end up with three APIs - the original PCI DMA API, > the existing DMA API, and some new DMA API.) > > Do we have any views on how common this feature is? > I can't think of cases outside of GPU's.. if it were more common I'd be in favor of teaching dma api about multiple contexts, but right now I think that would just amount to forcing a lot of churn on everyone else for the benefit of GPU's. IMHO it makes more sense for GPU drivers to bypass the dma api if they need to. Plus, sooner or later, someone will discover that with some trick or optimization they can get moar fps, but the extra layer of abstraction will just be getting in the way. BR, -R
[PATCH v2 06/10] drm/imx: switch to use media bus formats
imx-drm internally misused the V4L2_PIX_FMT constants, which are supposed to
describe the pixel format of frame buffers in memory, to describe the pixel
format on the bus between the display controller and the encoder hardware
instead. Now that MEDIA_BUS_FMT constants are available to drm drivers, use
those instead.
Signed-off-by: Philipp Zabel
---
drivers/gpu/drm/imx/dw_hdmi-imx.c | 2 +-
drivers/gpu/drm/imx/imx-ldb.c | 6 +++---
drivers/gpu/drm/imx/imx-tve.c | 4 ++--
drivers/gpu/drm/imx/parallel-display.c | 8
drivers/gpu/ipu-v3/ipu-dc.c| 16
5 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c
b/drivers/gpu/drm/imx/dw_hdmi-imx.c
index 87fe8ed..61ef987 100644
--- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
+++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
@@ -123,7 +123,7 @@ static void dw_hdmi_imx_encoder_commit(struct drm_encoder
*encoder)
static void dw_hdmi_imx_encoder_prepare(struct drm_encoder *encoder)
{
- imx_drm_panel_format(encoder, V4L2_PIX_FMT_RGB24);
+ imx_drm_panel_format(encoder, MEDIA_BUS_FMT_RGB888_1X24);
}
static struct drm_encoder_helper_funcs dw_hdmi_imx_encoder_helper_funcs = {
diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
index 2d6dc94..f9ec17a 100644
--- a/drivers/gpu/drm/imx/imx-ldb.c
+++ b/drivers/gpu/drm/imx/imx-ldb.c
@@ -168,16 +168,16 @@ static void imx_ldb_encoder_prepare(struct drm_encoder
*encoder)
switch (imx_ldb_ch->chno) {
case 0:
pixel_fmt = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH0_24) ?
- V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_BGR666;
+ MEDIA_BUS_FMT_RGB888_1X24 : MEDIA_BUS_FMT_RGB666_1X18;
break;
case 1:
pixel_fmt = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH1_24) ?
- V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_BGR666;
+ MEDIA_BUS_FMT_RGB888_1X24 : MEDIA_BUS_FMT_RGB666_1X18;
break;
default:
dev_err(ldb->dev, "unable to config di%d panel format\n",
imx_ldb_ch->chno);
- pixel_fmt = V4L2_PIX_FMT_RGB24;
+ pixel_fmt = MEDIA_BUS_FMT_RGB888_1X24;
}
imx_drm_panel_format(encoder, pixel_fmt);
diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c
index b63601d..7b0f28b 100644
--- a/drivers/gpu/drm/imx/imx-tve.c
+++ b/drivers/gpu/drm/imx/imx-tve.c
@@ -297,11 +297,11 @@ static void imx_tve_encoder_prepare(struct drm_encoder
*encoder)
switch (tve->mode) {
case TVE_MODE_VGA:
- imx_drm_panel_format_pins(encoder, IPU_PIX_FMT_GBR24,
+ imx_drm_panel_format_pins(encoder, MEDIA_BUS_FMT_YUV8_1X24,
tve->hsync_pin, tve->vsync_pin);
break;
case TVE_MODE_TVOUT:
- imx_drm_panel_format(encoder, V4L2_PIX_FMT_YUV444);
+ imx_drm_panel_format(encoder, MEDIA_BUS_FMT_YUV8_1X24);
break;
}
}
diff --git a/drivers/gpu/drm/imx/parallel-display.c
b/drivers/gpu/drm/imx/parallel-display.c
index 5e83e00..40a9194 100644
--- a/drivers/gpu/drm/imx/parallel-display.c
+++ b/drivers/gpu/drm/imx/parallel-display.c
@@ -225,14 +225,14 @@ static int imx_pd_bind(struct device *dev, struct device
*master, void *data)
ret = of_property_read_string(np, "interface-pix-fmt", &fmt);
if (!ret) {
if (!strcmp(fmt, "rgb24"))
- imxpd->interface_pix_fmt = V4L2_PIX_FMT_RGB24;
+ imxpd->interface_pix_fmt = MEDIA_BUS_FMT_RGB888_1X24;
else if (!strcmp(fmt, "rgb565"))
- imxpd->interface_pix_fmt = V4L2_PIX_FMT_RGB565;
+ imxpd->interface_pix_fmt = MEDIA_BUS_FMT_RGB565_1X16;
else if (!strcmp(fmt, "bgr666"))
- imxpd->interface_pix_fmt = V4L2_PIX_FMT_BGR666;
+ imxpd->interface_pix_fmt = MEDIA_BUS_FMT_RGB666_1X18;
else if (!strcmp(fmt, "lvds666"))
imxpd->interface_pix_fmt =
- v4l2_fourcc('L', 'V', 'D', '6');
+ MEDIA_BUS_FMT_RGB666_1X24_CPADHI;
}
panel_node = of_parse_phandle(np, "fsl,panel", 0);
diff --git a/drivers/gpu/ipu-v3/ipu-dc.c b/drivers/gpu/ipu-v3/ipu-dc.c
index 323203d..da5a4b9 100644
--- a/drivers/gpu/ipu-v3/ipu-dc.c
+++ b/drivers/gpu/ipu-v3/ipu-dc.c
@@ -147,20 +147,20 @@ static void dc_write_tmpl(struct ipu_dc *dc, int word,
u32 opcode, u32 operand,
writel(reg2, priv->dc_tmpl_reg + word * 8 + 4);
}
-static int ipu_pixfmt_to_map(u32 fmt)
+static int ipu_bus_format_to_map(u32 fmt)
{
switch (fmt) {
- case V4L2_PIX_FMT_RGB24:
+ case MEDIA_BUS_FMT_RGB888_1X24:
return IPU_DC_MAP_RGB24;
- case V4L2_PIX_FMT_RGB
[PATCH v2 05/10] Add RGB666_1X24_CPADHI media bus format
Commit 9e74d2926a28 ("staging: imx-drm: add LVDS666 support for parallel
display") describes a 24-bit bus format where three 6-bit components each
take the lower part of 8 bits with the two high bits zero padded. Add a
component-wise padded media bus format RGB666_1X24_CPADHI to support this
connection.
Cc: Emil Renner Berthing
Signed-off-by: Philipp Zabel
---
Documentation/DocBook/media/v4l/subdev-formats.xml | 30 ++
include/uapi/linux/media-bus-format.h | 3 ++-
2 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/Documentation/DocBook/media/v4l/subdev-formats.xml
b/Documentation/DocBook/media/v4l/subdev-formats.xml
index 8d1f624..c02af7a 100644
--- a/Documentation/DocBook/media/v4l/subdev-formats.xml
+++ b/Documentation/DocBook/media/v4l/subdev-formats.xml
@@ -480,6 +480,36 @@ see .
b1
b0
+
+ MEDIA_BUS_FMT_RGB666_1X24_CPADHI
+ 0x1015
+
+ &dash-ent-8;
+ 0
+ 0
+ r5
+ r4
+ r3
+ r2
+ r1
+ r0
+ 0
+ 0
+ g5
+ g4
+ g3
+ g2
+ g1
+ g0
+ 0
+ 0
+ b5
+ b4
+ b3
+ b2
+ b1
+ b0
+
MEDIA_BUS_FMT_BGR888_1X24
0x1013
diff --git a/include/uapi/linux/media-bus-format.h
b/include/uapi/linux/media-bus-format.h
index 8dbf16c..83ea46f 100644
--- a/include/uapi/linux/media-bus-format.h
+++ b/include/uapi/linux/media-bus-format.h
@@ -33,7 +33,7 @@
#define MEDIA_BUS_FMT_FIXED0x0001
-/* RGB - next is 0x1015 */
+/* RGB - next is 0x1016 */
#define MEDIA_BUS_FMT_RGB444_1X12 0x100e
#define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE 0x1001
#define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE 0x1002
@@ -45,6 +45,7 @@
#define MEDIA_BUS_FMT_RGB565_2X8_BE0x1007
#define MEDIA_BUS_FMT_RGB565_2X8_LE0x1008
#define MEDIA_BUS_FMT_RGB666_1X18 0x1009
+#define MEDIA_BUS_FMT_RGB666_1X24_CPADHI 0x1015
#define MEDIA_BUS_FMT_RGB666_1X7X3_SPWG0x1010
#define MEDIA_BUS_FMT_BGR888_1X24 0x1013
#define MEDIA_BUS_FMT_GBR888_1X24 0x1014
--
2.1.4
[PATCH v2 01/10] Add RGB444_1X12 and RGB565_1X16 media bus formats
From: Boris Brezillion Add RGB444_1X12 and RGB565_1X16 format definitions and update the documentation. Signed-off-by: Boris Brezillon Acked-by: Mauro Carvalho Chehab Acked-by: Sakari Ailus Acked-by: Laurent Pinchart Signed-off-by: Philipp Zabel --- Changes since v1: - Fixed constant values in the documentation to correspond to the actual values. - Added Laurent's ack. --- Documentation/DocBook/media/v4l/subdev-formats.xml | 40 ++ include/uapi/linux/media-bus-format.h | 4 ++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/Documentation/DocBook/media/v4l/subdev-formats.xml b/Documentation/DocBook/media/v4l/subdev-formats.xml index c5ea868..29fe601 100644 --- a/Documentation/DocBook/media/v4l/subdev-formats.xml +++ b/Documentation/DocBook/media/v4l/subdev-formats.xml @@ -192,6 +192,24 @@ see . + + MEDIA_BUS_FMT_RGB444_1X12 + 0x100e + + &dash-ent-20; + r3 + r2 + r1 + r0 + g3 + g2 + g1 + g0 + b3 + b2 + b1 + b0 + MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE 0x1001 @@ -304,6 +322,28 @@ see . g4 g3 + + MEDIA_BUS_FMT_RGB565_1X16 + 0x100f + + &dash-ent-16; + r4 + r3 + r2 + r1 + r0 + g5 + g4 + g3 + g2 + g1 + g0 + b4 + b3 + b2 + b1 + b0 + MEDIA_BUS_FMT_BGR565_2X8_BE 0x1005 diff --git a/include/uapi/linux/media-bus-format.h b/include/uapi/linux/media-bus-format.h index 23b4090..37091c6 100644 --- a/include/uapi/linux/media-bus-format.h +++ b/include/uapi/linux/media-bus-format.h @@ -33,11 +33,13 @@ #define MEDIA_BUS_FMT_FIXED0x0001 -/* RGB - next is 0x100e */ +/* RGB - next is 0x1010 */ +#define MEDIA_BUS_FMT_RGB444_1X12 0x100e #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE 0x1001 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE 0x1002 #define MEDIA_BUS_FMT_RGB555_2X8_PADHI_BE 0x1003 #define MEDIA_BUS_FMT_RGB555_2X8_PADHI_LE 0x1004 +#define MEDIA_BUS_FMT_RGB565_1X16 0x100f #define MEDIA_BUS_FMT_BGR565_2X8_BE0x1005 #define MEDIA_BUS_FMT_BGR565_2X8_LE0x1006 #define MEDIA_BUS_FMT_RGB565_2X8_BE0x1007 -- 2.1.4
[PATCH v2 00/10] Use media bus formats in imx-drm and add drm panel support
Sorry about the noise, this time sent to the correct list: Currently the imx-drm driver misuses the V4L2_PIX_FMT constants to describe the pixel format on the parallel bus between display controllers and encoders. Now that MEDIA_BUS_FMT is available, use that instead. I'd like to get the V4L2 maintainers' acks for the four necessary media bus format patches to be merged through the drm tree. The two drm/imx core patches replace V4L2_PIX_FMT with MEDIA_BUS_FMT where appropriate and consolidate the variable names for clarification. The three LDB patches depend on this of-graph helper series: http://comments.gmane.org/gmane.linux.kernel/1871630 They allow to optionally use LVDS panels with drm_panel drivers, connected to the LDB encoder in the device tree via of-graph endpoint links. Changes since v1: - Fixed BGR888_1X24 and GBR888_1X24 constant values in the documentation and added Laurent's ack. - Fixed ldb drm_panel patch to use port at 2 as output on i.MX53 - Use correct mailing list regards Philipp Boris Brezillion (1): Add RGB444_1X12 and RGB565_1X16 media bus formats Philipp Zabel (9): Add LVDS RGB media bus formats Add BGR888_1X24 and GBR888_1X24 media bus formats Add YUV8_1X24 media bus format Add RGB666_1X24_CPADHI media bus format drm/imx: switch to use media bus formats drm/imx: consolidate bus format variable names drm/imx: imx-ldb: add drm_panel support drm/imx: imx-ldb: reset display clock input when disabling LVDS drm/imx: imx-ldb: allow to determine bus format from the connected panel Documentation/DocBook/media/v4l/subdev-formats.xml | 422 + Documentation/devicetree/bindings/drm/imx/ldb.txt | 51 ++- drivers/gpu/drm/imx/Kconfig| 1 + drivers/gpu/drm/imx/dw_hdmi-imx.c | 2 +- drivers/gpu/drm/imx/imx-drm-core.c | 14 +- drivers/gpu/drm/imx/imx-drm.h | 10 +- drivers/gpu/drm/imx/imx-ldb.c | 196 +++--- drivers/gpu/drm/imx/imx-tve.c | 6 +- drivers/gpu/drm/imx/ipuv3-crtc.c | 13 +- drivers/gpu/drm/imx/parallel-display.c | 13 +- drivers/gpu/ipu-v3/ipu-dc.c| 16 +- include/uapi/linux/media-bus-format.h | 13 +- 12 files changed, 642 insertions(+), 115 deletions(-) -- 2.1.4
[PATCH v2 08/10] drm/imx: imx-ldb: add drm_panel support
This patch allows to optionally attach the lvds-channel to a panel
supported by a drm_panel driver using of-graph bindings, instead of
supplying the modes via display-timings in the device tree.
This depends on of_graph_get_port_by_id and uses the OF graph to
link the optional DRM panel to the LDB lvds-channel. The output
port number is 1 on devices without the 4-port input multiplexer
(i.MX5) and 4 on devices with the mux (i.MX6).
Before:
ldb {
...
lvds-channel at 0 {
...
display-timings {
native-timing = <&timing1>;
timing1: etm0700g0dh6 {
hactive = <800>;
vactive = <480>;
clock-frequency = <3326>;
hsync-len = <128>;
hback-porch = <88>;
hfront-porch = <40>;
vsync-len = <2>;
vback-porch = <33>;
vfront-porch = <10>;
hsync-active = <0>;
vsync-active = <0>;
...
};
};
...
};
};
After:
ldb {
...
lvds-channel at 0 {
...
port at 4 {
reg = <4>;
lvds_out: endpoint {
remote_endpoint = <&panel_in>;
};
};
};
};
panel {
compatible = "edt,etm0700g0dh6", "simple-panel";
...
port {
panel_in: endpoint {
remote-endpoint = <&lvds_out>;
};
};
};
[Fixed build error due to missing select on DRM_PANEL --rmk]
Signed-off-by: Russell King
Signed-off-by: Philipp Zabel
---
Changes since v1:
- Fixed output port to be port at 2 on i.MX53.
---
Documentation/devicetree/bindings/drm/imx/ldb.txt | 51 ---
drivers/gpu/drm/imx/Kconfig | 1 +
drivers/gpu/drm/imx/imx-ldb.c | 48 -
3 files changed, 82 insertions(+), 18 deletions(-)
diff --git a/Documentation/devicetree/bindings/drm/imx/ldb.txt
b/Documentation/devicetree/bindings/drm/imx/ldb.txt
index 443bcb6..ee96dc6 100644
--- a/Documentation/devicetree/bindings/drm/imx/ldb.txt
+++ b/Documentation/devicetree/bindings/drm/imx/ldb.txt
@@ -44,23 +44,29 @@ Optional properties:
LVDS Channel
-Each LVDS Channel has to contain a display-timings node that describes the
-video timings for the connected LVDS display. For detailed information, also
-have a look at Documentation/devicetree/bindings/video/display-timing.txt.
+Each LVDS Channel must either be linked to a panel device tree node via an
+of-graph endpoint link on its output port, or it has to contain a node called
+display-timings that describes the video timings for the connected LVDS panel
+manally.
Required properties:
- reg : should be <0> or <1>
+ - port: Port nodes with endpoint definitions as defined in
+ Documentation/devicetree/bindings/media/video-interfaces.txt.
+ On i.MX5, the internal two-input-multiplexer is used. Due to limitations
+ in the clock tree, only one input port (port@[0,1]) can be used for each
+ channel (lvds-channel@[0,1], respectively). Here the output port is port at
2.
+ On i.MX6, there should be four input ports (port@[0-3]) that correspond
+ to the four LVDS multiplexer inputs. Here the output port is port at 4.
+ The output port is optional if display-timings are used instead.
+
+Optional properties (required if display-timings are used):
+ - display-timings : A node that describes the display timings as defined in
+ Documentation/devicetree/bindings/video/display-timing.txt.
- fsl,data-mapping : should be "spwg" or "jeida"
This describes how the color bits are laid out in the
serialized LVDS signal.
- fsl,data-width : should be <18> or <24>
- - port: A port node with endpoint definitions as defined in
- Documentation/devicetree/bindings/media/video-interfaces.txt.
- On i.MX5, the internal two-input-multiplexer is used.
- Due to hardware limitations, only one port (port@[0,1])
- can be used for each channel (lvds-channel@[0,1], respectively)
- On i.MX6, there should be four ports (port@[0-3]) that correspond
- to the four LVDS multiplexer inputs.
example:
@@ -80
[PATCH v2 02/10] Add LVDS RGB media bus formats
This patch adds three new RGB media bus formats that describe 18-bit or 24-bit samples transferred over an LVDS bus with three or four differential data pairs, serialized into 7 time slots, using standard SPWG/PSWG/VESA or JEIDA data ordering. Signed-off-by: Philipp Zabel Acked-by: Sakari Ailus --- Documentation/DocBook/media/v4l/subdev-formats.xml | 255 + include/uapi/linux/media-bus-format.h | 5 +- 2 files changed, 259 insertions(+), 1 deletion(-) diff --git a/Documentation/DocBook/media/v4l/subdev-formats.xml b/Documentation/DocBook/media/v4l/subdev-formats.xml index 29fe601..18449b3 100644 --- a/Documentation/DocBook/media/v4l/subdev-formats.xml +++ b/Documentation/DocBook/media/v4l/subdev-formats.xml @@ -622,6 +622,261 @@ see . + + On LVDS buses, usually each sample is transferred serialized in + seven time slots per pixel clock, on three (18-bit) or four (24-bit) + differential data pairs at the same time. The remaining bits are used for + control signals as defined by SPWG/PSWG/VESA or JEIDA standards. + The 24-bit RGB format serialized in seven time slots on four lanes using + JEIDA defined bit mapping will be named + MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, for example. + + + + LVDS RGB formats + + + + + + + + + + + + + Identifier + Code + + + Data organization + + + + + Timeslot + Lane + 3 + 2 + 1 + 0 + + + + + MEDIA_BUS_FMT_RGB666_1X7X3_SPWG + 0x1010 + 0 + + - + d + b1 + g0 + + + + + 1 + + - + d + b0 + r5 + + + + + 2 + + - + d + g5 + r4 + + + + + 3 + + - + b5 + g4 + r3 + + + + + 4 + + - + b4 + g3 + r2 + + + + + 5 + + - + b3 + g2 + r1 + + + + + 6 + + - + b2 + g1 + r0 + + + MEDIA_BUS_FMT_RGB888_1X7X4_SPWG + 0x1011 + 0 + + d + d + b1 + g0 + + + + + 1 + + b7 + d + b0 + r5 + + + + + 2 + + b6 + d + g5 + r4 + + + + + 3 + + g7 + b5 + g4 + r3 + + + + + 4 + + g6 + b4 + g3 + r2 + + + + + 5 + + r7 + b3 + g2 + r1 + + + + + 6 + + r6 + b2 + g1 + r0 + + + MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA + 0x1012 + 0 + + d + d + b3 + g2 + + + + + 1 + + b1 + d + b2 + r7 + + + + + 2 + + b0 + d + g7 + r6 + + + + + 3 + + g1 + b7 + g6 + r5 + + + + + 4 + + g0 + b6 + g5 + r4 + + + + + 5 + + r1 + b5 + g4 + r3 + + + +
[PATCH v2 09/10] drm/imx: imx-ldb: reset display clock input when disabling LVDS
The LDB driver changes the attached display interface's input clock mux
to the LDB_DI clock reference. Change it back again when disabling the
LVDS display. Changing back to the PLL5 for example allows to configure
the same DI for HDMI output later.
Signed-off-by: Philipp Zabel
---
drivers/gpu/drm/imx/imx-ldb.c | 26 ++
1 file changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
index 4286399..544282b 100644
--- a/drivers/gpu/drm/imx/imx-ldb.c
+++ b/drivers/gpu/drm/imx/imx-ldb.c
@@ -77,6 +77,7 @@ struct imx_ldb {
struct imx_ldb_channel channel[2];
struct clk *clk[2]; /* our own clock */
struct clk *clk_sel[4]; /* parent of display clock */
+ struct clk *clk_parent[4]; /* original parent of clk_sel */
struct clk *clk_pll[2]; /* upstream clock we can adjust */
u32 ldb_ctrl;
const struct bus_mux *lvds_mux;
@@ -287,6 +288,7 @@ static void imx_ldb_encoder_disable(struct drm_encoder
*encoder)
{
struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
struct imx_ldb *ldb = imx_ldb_ch->ldb;
+ int mux, ret;
/*
* imx_ldb_encoder_disable is called by
@@ -314,6 +316,28 @@ static void imx_ldb_encoder_disable(struct drm_encoder
*encoder)
clk_disable_unprepare(ldb->clk[1]);
}
+ if (ldb->lvds_mux) {
+ const struct bus_mux *lvds_mux = NULL;
+
+ if (imx_ldb_ch == &ldb->channel[0])
+ lvds_mux = &ldb->lvds_mux[0];
+ else if (imx_ldb_ch == &ldb->channel[1])
+ lvds_mux = &ldb->lvds_mux[1];
+
+ regmap_read(ldb->regmap, lvds_mux->reg, &mux);
+ mux &= lvds_mux->mask;
+ mux >>= lvds_mux->shift;
+ } else {
+ mux = (imx_ldb_ch == &ldb->channel[0]) ? 0 : 1;
+ }
+
+ /* set display clock mux back to original input clock */
+ ret = clk_set_parent(ldb->clk_sel[mux], ldb->clk_parent[mux]);
+ if (ret)
+ dev_err(ldb->dev,
+ "unable to set di%d parent clock to original parent\n",
+ mux);
+
drm_panel_unprepare(imx_ldb_ch->panel);
}
@@ -499,6 +523,8 @@ static int imx_ldb_bind(struct device *dev, struct device
*master, void *data)
imx_ldb->clk_sel[i] = NULL;
break;
}
+
+ imx_ldb->clk_parent[i] = clk_get_parent(imx_ldb->clk_sel[i]);
}
if (i == 0)
return ret;
--
2.1.4
[PATCH v2 10/10] drm/imx: imx-ldb: allow to determine bus format from the connected panel
This patch makes the fsl,data-width and fsl,data-mapping device tree
properties optional if a panel is connected to the ldb output port
via the of_graph bindings. The data mapping is determined from the
display_info.bus_format field provided by the panel.
Signed-off-by: Philipp Zabel
---
drivers/gpu/drm/imx/imx-ldb.c | 116 +++---
1 file changed, 63 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
index 544282b..abacc8f 100644
--- a/drivers/gpu/drm/imx/imx-ldb.c
+++ b/drivers/gpu/drm/imx/imx-ldb.c
@@ -63,6 +63,7 @@ struct imx_ldb_channel {
int edid_len;
struct drm_display_mode mode;
int mode_valid;
+ int bus_format;
};
struct bus_mux {
@@ -96,7 +97,11 @@ static int imx_ldb_connector_get_modes(struct drm_connector
*connector)
if (imx_ldb_ch->panel && imx_ldb_ch->panel->funcs &&
imx_ldb_ch->panel->funcs->get_modes) {
+ struct drm_display_info *di = &connector->display_info;
+
num_modes =
imx_ldb_ch->panel->funcs->get_modes(imx_ldb_ch->panel);
+ if (!imx_ldb_ch->bus_format && di->num_bus_formats)
+ imx_ldb_ch->bus_format = di->bus_formats[0];
if (num_modes > 0)
return num_modes;
}
@@ -173,21 +178,33 @@ static void imx_ldb_encoder_prepare(struct drm_encoder
*encoder)
{
struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
struct imx_ldb *ldb = imx_ldb_ch->ldb;
+ int dual = ldb->ldb_ctrl & LDB_SPLIT_MODE_EN;
u32 bus_format;
- switch (imx_ldb_ch->chno) {
- case 0:
- bus_format = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH0_24) ?
- MEDIA_BUS_FMT_RGB888_1X24 : MEDIA_BUS_FMT_RGB666_1X18;
+ switch (imx_ldb_ch->bus_format) {
+ default:
+ dev_warn(ldb->dev,
+"could not determine data mapping, default to 18-bit
\"spwg\"\n");
+ /* fallthrough */
+ case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
+ bus_format = MEDIA_BUS_FMT_RGB666_1X18;
break;
- case 1:
- bus_format = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH1_24) ?
- MEDIA_BUS_FMT_RGB888_1X24 : MEDIA_BUS_FMT_RGB666_1X18;
+ case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
+ bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+ if (imx_ldb_ch->chno == 0 || dual)
+ ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24;
+ if (imx_ldb_ch->chno == 1 || dual)
+ ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24;
break;
- default:
- dev_err(ldb->dev, "unable to config di%d panel format\n",
- imx_ldb_ch->chno);
+ case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+ if (imx_ldb_ch->chno == 0 || dual)
+ ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24 |
+LDB_BIT_MAP_CH0_JEIDA;
+ if (imx_ldb_ch->chno == 1 || dual)
+ ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24 |
+LDB_BIT_MAP_CH1_JEIDA;
+ break;
}
imx_drm_set_bus_format(encoder, bus_format);
@@ -426,25 +443,39 @@ enum {
LVDS_BIT_MAP_JEIDA
};
-static const char * const imx_ldb_bit_mappings[] = {
- [LVDS_BIT_MAP_SPWG] = "spwg",
- [LVDS_BIT_MAP_JEIDA] = "jeida",
+struct imx_ldb_bit_mapping {
+ u32 bus_format;
+ u32 datawidth;
+ const char * const mapping;
};
-static const int of_get_data_mapping(struct device_node *np)
+static const struct imx_ldb_bit_mapping imx_ldb_bit_mappings[] = {
+ { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, 18, "spwg" },
+ { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, 24, "spwg" },
+ { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, "jeida" },
+};
+
+static u32 of_get_bus_format(struct device *dev, struct device_node *np)
{
const char *bm;
+ u32 datawidth = 0;
int ret, i;
ret = of_property_read_string(np, "fsl,data-mapping", &bm);
if (ret < 0)
return ret;
- for (i = 0; i < ARRAY_SIZE(imx_ldb_bit_mappings); i++)
- if (!strcasecmp(bm, imx_ldb_bit_mappings[i]))
- return i;
+ of_property_read_u32(np, "fsl,data-width", &datawidth);
+
+ for (i = 0; i < ARRAY_SIZE(imx_ldb_bit_mappings); i++) {
+ if (!strcasecmp(bm, imx_ldb_bit_mappings[i].mapping) &&
+ datawidth == imx_ldb_bit_mappings[i].datawidth)
+ return imx_ldb_bit_mappings[i].bus_format;
+ }
+
+ dev_err(dev, "invalid data mapping: %d-bit \"%s\"\n", datawidth, bm);
- return -EINVAL;
+ return -ENOENT;
}
static struct bus_mux imx6q_lvds_mux[2] = {
@@ -481,8 +512,6 @@
[PATCH v2 04/10] Add YUV8_1X24 media bus format
This patch adds the media bus format for a 24-bit bus format with three 8-bit YUV components. Signed-off-by: Philipp Zabel Acked-by: Laurent Pinchart --- Documentation/DocBook/media/v4l/subdev-formats.xml | 37 ++ include/uapi/linux/media-bus-format.h | 3 +- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/Documentation/DocBook/media/v4l/subdev-formats.xml b/Documentation/DocBook/media/v4l/subdev-formats.xml index 805cbe1..8d1f624 100644 --- a/Documentation/DocBook/media/v4l/subdev-formats.xml +++ b/Documentation/DocBook/media/v4l/subdev-formats.xml @@ -3015,6 +3015,43 @@ see . u1 u0 + + MEDIA_BUS_FMT_YUV8_1X24 + 0x2024 + + - + - + - + - + - + - + - + - + y7 + y6 + y5 + y4 + y3 + y2 + y1 + y0 + u7 + u6 + u5 + u4 + u3 + u2 + u1 + u0 + v7 + v6 + v5 + v4 + v3 + v2 + v1 + v0 + MEDIA_BUS_FMT_YUV10_1X30 0x2016 diff --git a/include/uapi/linux/media-bus-format.h b/include/uapi/linux/media-bus-format.h index 6f6942e..8dbf16c 100644 --- a/include/uapi/linux/media-bus-format.h +++ b/include/uapi/linux/media-bus-format.h @@ -55,7 +55,7 @@ #define MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA 0x1012 #define MEDIA_BUS_FMT_ARGB_1X320x100d -/* YUV (including grey) - next is 0x2024 */ +/* YUV (including grey) - next is 0x2025 */ #define MEDIA_BUS_FMT_Y8_1X8 0x2001 #define MEDIA_BUS_FMT_UV8_1X8 0x2015 #define MEDIA_BUS_FMT_UYVY8_1_5X8 0x2002 @@ -81,6 +81,7 @@ #define MEDIA_BUS_FMT_VYUY10_1X20 0x201b #define MEDIA_BUS_FMT_YUYV10_1X20 0x200d #define MEDIA_BUS_FMT_YVYU10_1X20 0x200e +#define MEDIA_BUS_FMT_YUV8_1X240x2024 #define MEDIA_BUS_FMT_YUV10_1X30 0x2016 #define MEDIA_BUS_FMT_AYUV8_1X32 0x2017 #define MEDIA_BUS_FMT_UYVY12_2X12 0x201c -- 2.1.4
[PATCH v2 07/10] drm/imx: consolidate bus format variable names
This patch consolidates the different interface_pix_fmt, pixel_fmt, pix_fmt,
and pixfmt variables to a common name "bus_format" wherever they describe the
pixel format on the bus between display controller and encoder hardware.
At the same time, it renames imx_drm_panel_format to imx_drm_set_bus_format.
Signed-off-by: Philipp Zabel
---
drivers/gpu/drm/imx/dw_hdmi-imx.c | 2 +-
drivers/gpu/drm/imx/imx-drm-core.c | 14 +++---
drivers/gpu/drm/imx/imx-drm.h | 10 +-
drivers/gpu/drm/imx/imx-ldb.c | 10 +-
drivers/gpu/drm/imx/imx-tve.c | 6 +++---
drivers/gpu/drm/imx/ipuv3-crtc.c | 13 +
drivers/gpu/drm/imx/parallel-display.c | 13 ++---
7 files changed, 32 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c
b/drivers/gpu/drm/imx/dw_hdmi-imx.c
index 61ef987..1834ac8 100644
--- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
+++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
@@ -123,7 +123,7 @@ static void dw_hdmi_imx_encoder_commit(struct drm_encoder
*encoder)
static void dw_hdmi_imx_encoder_prepare(struct drm_encoder *encoder)
{
- imx_drm_panel_format(encoder, MEDIA_BUS_FMT_RGB888_1X24);
+ imx_drm_set_bus_format(encoder, MEDIA_BUS_FMT_RGB888_1X24);
}
static struct drm_encoder_helper_funcs dw_hdmi_imx_encoder_helper_funcs = {
diff --git a/drivers/gpu/drm/imx/imx-drm-core.c
b/drivers/gpu/drm/imx/imx-drm-core.c
index a002f53..c6f2c472 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -103,8 +103,8 @@ static struct imx_drm_crtc *imx_drm_find_crtc(struct
drm_crtc *crtc)
return NULL;
}
-int imx_drm_panel_format_pins(struct drm_encoder *encoder,
- u32 interface_pix_fmt, int hsync_pin, int vsync_pin)
+int imx_drm_set_bus_format_pins(struct drm_encoder *encoder, u32 bus_format,
+ int hsync_pin, int vsync_pin)
{
struct imx_drm_crtc_helper_funcs *helper;
struct imx_drm_crtc *imx_crtc;
@@ -116,16 +116,16 @@ int imx_drm_panel_format_pins(struct drm_encoder *encoder,
helper = &imx_crtc->imx_drm_helper_funcs;
if (helper->set_interface_pix_fmt)
return helper->set_interface_pix_fmt(encoder->crtc,
- interface_pix_fmt, hsync_pin, vsync_pin);
+ bus_format, hsync_pin, vsync_pin);
return 0;
}
-EXPORT_SYMBOL_GPL(imx_drm_panel_format_pins);
+EXPORT_SYMBOL_GPL(imx_drm_set_bus_format_pins);
-int imx_drm_panel_format(struct drm_encoder *encoder, u32 interface_pix_fmt)
+int imx_drm_set_bus_format(struct drm_encoder *encoder, u32 bus_format)
{
- return imx_drm_panel_format_pins(encoder, interface_pix_fmt, 2, 3);
+ return imx_drm_set_bus_format_pins(encoder, bus_format, 2, 3);
}
-EXPORT_SYMBOL_GPL(imx_drm_panel_format);
+EXPORT_SYMBOL_GPL(imx_drm_set_bus_format);
int imx_drm_crtc_vblank_get(struct imx_drm_crtc *imx_drm_crtc)
{
diff --git a/drivers/gpu/drm/imx/imx-drm.h b/drivers/gpu/drm/imx/imx-drm.h
index 3c559cc..28e776d 100644
--- a/drivers/gpu/drm/imx/imx-drm.h
+++ b/drivers/gpu/drm/imx/imx-drm.h
@@ -18,7 +18,7 @@ struct imx_drm_crtc_helper_funcs {
int (*enable_vblank)(struct drm_crtc *crtc);
void (*disable_vblank)(struct drm_crtc *crtc);
int (*set_interface_pix_fmt)(struct drm_crtc *crtc,
- u32 pix_fmt, int hsync_pin, int vsync_pin);
+ u32 bus_format, int hsync_pin, int vsync_pin);
const struct drm_crtc_helper_funcs *crtc_helper_funcs;
const struct drm_crtc_funcs *crtc_funcs;
};
@@ -40,10 +40,10 @@ void imx_drm_mode_config_init(struct drm_device *drm);
struct drm_gem_cma_object *imx_drm_fb_get_obj(struct drm_framebuffer *fb);
-int imx_drm_panel_format_pins(struct drm_encoder *encoder,
- u32 interface_pix_fmt, int hsync_pin, int vsync_pin);
-int imx_drm_panel_format(struct drm_encoder *encoder,
- u32 interface_pix_fmt);
+int imx_drm_set_bus_format_pins(struct drm_encoder *encoder,
+ u32 bus_format, int hsync_pin, int vsync_pin);
+int imx_drm_set_bus_format(struct drm_encoder *encoder,
+ u32 bus_format);
int imx_drm_encoder_get_mux_id(struct device_node *node,
struct drm_encoder *encoder);
diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
index f9ec17a..cd062b1 100644
--- a/drivers/gpu/drm/imx/imx-ldb.c
+++ b/drivers/gpu/drm/imx/imx-ldb.c
@@ -163,24 +163,24 @@ static void imx_ldb_encoder_prepare(struct drm_encoder
*encoder)
{
struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
struct imx_ldb *ldb = imx_ldb_ch->ldb;
- u32 pixel_fmt;
+ u32 bus_format;
switch (imx_ldb_ch->chno) {
case 0:
- pixel_fmt = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH0_24) ?
+ bus_format = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH0_24) ?
MEDIA_BUS_FMT_
[PATCH v2 03/10] Add BGR888_1X24 and GBR888_1X24 media bus formats
This patch adds two more 24-bit RGB formats. BGR888 is more or less common, GBR888 is used on the internal connection between the IPU display interface and the TVE (VGA DAC) on i.MX53 SoCs. Signed-off-by: Philipp Zabel Acked-by: Laurent Pinchart --- Documentation/DocBook/media/v4l/subdev-formats.xml | 60 ++ include/uapi/linux/media-bus-format.h | 4 +- 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/Documentation/DocBook/media/v4l/subdev-formats.xml b/Documentation/DocBook/media/v4l/subdev-formats.xml index 18449b3..805cbe1 100644 --- a/Documentation/DocBook/media/v4l/subdev-formats.xml +++ b/Documentation/DocBook/media/v4l/subdev-formats.xml @@ -480,6 +480,66 @@ see . b1 b0 + + MEDIA_BUS_FMT_BGR888_1X24 + 0x1013 + + &dash-ent-8; + b7 + b6 + b5 + b4 + b3 + b2 + b1 + b0 + g7 + g6 + g5 + g4 + g3 + g2 + g1 + g0 + r7 + r6 + r5 + r4 + r3 + r2 + r1 + r0 + + + MEDIA_BUS_FMT_GBR888_1X24 + 0x1014 + + &dash-ent-8; + g7 + g6 + g5 + g4 + g3 + g2 + g1 + g0 + b7 + b6 + b5 + b4 + b3 + b2 + b1 + b0 + r7 + r6 + r5 + r4 + r3 + r2 + r1 + r0 + MEDIA_BUS_FMT_RGB888_1X24 0x100a diff --git a/include/uapi/linux/media-bus-format.h b/include/uapi/linux/media-bus-format.h index 3fb9cbb..6f6942e 100644 --- a/include/uapi/linux/media-bus-format.h +++ b/include/uapi/linux/media-bus-format.h @@ -33,7 +33,7 @@ #define MEDIA_BUS_FMT_FIXED0x0001 -/* RGB - next is 0x1013 */ +/* RGB - next is 0x1015 */ #define MEDIA_BUS_FMT_RGB444_1X12 0x100e #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE 0x1001 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE 0x1002 @@ -46,6 +46,8 @@ #define MEDIA_BUS_FMT_RGB565_2X8_LE0x1008 #define MEDIA_BUS_FMT_RGB666_1X18 0x1009 #define MEDIA_BUS_FMT_RGB666_1X7X3_SPWG0x1010 +#define MEDIA_BUS_FMT_BGR888_1X24 0x1013 +#define MEDIA_BUS_FMT_GBR888_1X24 0x1014 #define MEDIA_BUS_FMT_RGB888_1X24 0x100a #define MEDIA_BUS_FMT_RGB888_2X12_BE 0x100b #define MEDIA_BUS_FMT_RGB888_2X12_LE 0x100c -- 2.1.4
[PATCH 00/14] drm/exynos: cleanups + atomic phases 1 and 2
From: Gustavo Padovan Hi, This series clean ups a few more paths from exynos-drm with the most important being the removal of the global page flip queue and the removal in driver internal data (struct *_win_data) that was replicating plane data. Following these patches comes the first step torwards atomic modesetting support on exynos. This series is applied on top of 3 patches[0][1][2] that were sent recently to dri-devel. Gustavo --- [0] http://www.spinics.net/lists/linux-samsung-soc/msg41867.html [1] http://lists.freedesktop.org/archives/dri-devel/2015-January/076504.html [2] http://lists.freedesktop.org/archives/dri-devel/2015-January/076505.html Daniel Kurtz (1): drm/exynos: do not copy adjusted mode into mode during crtc mode_set Gustavo Padovan (12): drm/exynos: Remove exynos_plane_dpms() call with no effect drm/exynos: remove leftover functions declarations drm/exynos: remove struct *_win_data abstraction on planes drm/exynos: atomic phase 1: use drm_plane_helper_update() drm/exynos: atomic phase 1: use drm_plane_helper_disable() drm/exynos: atomic phase 1: add atomic_begin()/atomic_flush() drm/exynos: atomic phase 1: add .mode_set_nofb() callback drm/exynos: atomic phase 2: wire up state reset(), duplicate() and destroy() drm/exynos: atomic phase 2: keep track of framebuffer pointer drm/exynos: make exynos_plane_mode_set() static drm/exynos: use correct pipe number on vblank event drm/exynos: remove exynos_disable_plane() Mandeep Singh Baines (1): drm/exynos: track vblank events on a per crtc basis drivers/gpu/drm/bridge/ptn3460.c | 4 + drivers/gpu/drm/exynos/exynos_dp_core.c | 4 + drivers/gpu/drm/exynos/exynos_drm_connector.c | 4 + drivers/gpu/drm/exynos/exynos_drm_crtc.c | 203 +++--- drivers/gpu/drm/exynos/exynos_drm_crtc.h | 7 +- drivers/gpu/drm/exynos/exynos_drm_dpi.c | 4 + drivers/gpu/drm/exynos/exynos_drm_drv.c | 29 +--- drivers/gpu/drm/exynos/exynos_drm_drv.h | 15 +- drivers/gpu/drm/exynos/exynos_drm_dsi.c | 4 + drivers/gpu/drm/exynos/exynos_drm_fb.c| 2 +- drivers/gpu/drm/exynos/exynos_drm_fimd.c | 235 -- drivers/gpu/drm/exynos/exynos_drm_plane.c | 99 ++- drivers/gpu/drm/exynos/exynos_drm_plane.h | 13 +- drivers/gpu/drm/exynos/exynos_drm_vidi.c | 127 -- drivers/gpu/drm/exynos/exynos_hdmi.c | 4 + drivers/gpu/drm/exynos/exynos_mixer.c | 212 --- 16 files changed, 408 insertions(+), 558 deletions(-) -- 1.9.3
[PATCH 01/14] drm/exynos: track vblank events on a per crtc basis
From: Mandeep Singh Baines
The goal of the change is to make sure we send the vblank event on the
current vblank. My hope is to fix any races that might be causing flicker.
After this change I only see a flicker in the transition plymouth and
X11.
Simplified the code by tracking vblank events on a per-crtc basis. This
allowed me to remove all error paths from the callback. It also allowed
me to remove the vblank wait from the callback.
Signed-off-by: Mandeep Singh Baines
Signed-off-by: Gustavo Padovan
---
drivers/gpu/drm/exynos/exynos_drm_crtc.c | 92 +++-
drivers/gpu/drm/exynos/exynos_drm_drv.c | 13 -
drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 +--
3 files changed, 44 insertions(+), 67 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index a85c451..b2a4b84 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -34,9 +34,8 @@ static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int
mode)
if (mode > DRM_MODE_DPMS_ON) {
/* wait for the completion of page flip. */
if (!wait_event_timeout(exynos_crtc->pending_flip_queue,
- !atomic_read(&exynos_crtc->pending_flip),
- HZ/20))
- atomic_set(&exynos_crtc->pending_flip, 0);
+ (exynos_crtc->event == NULL), HZ/20))
+ exynos_crtc->event = NULL;
drm_crtc_vblank_off(crtc);
}
@@ -166,11 +165,10 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc
*crtc,
uint32_t page_flip_flags)
{
struct drm_device *dev = crtc->dev;
- struct exynos_drm_private *dev_priv = dev->dev_private;
struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
struct drm_framebuffer *old_fb = crtc->primary->fb;
unsigned int crtc_w, crtc_h;
- int ret = -EINVAL;
+ int ret;
/* when the page flip is requested, crtc's dpms should be on */
if (exynos_crtc->dpms > DRM_MODE_DPMS_ON) {
@@ -178,48 +176,49 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc
*crtc,
return -EINVAL;
}
- mutex_lock(&dev->struct_mutex);
+ if (!event)
+ return -EINVAL;
- if (event) {
- /*
-* the pipe from user always is 0 so we can set pipe number
-* of current owner to event.
-*/
- event->pipe = exynos_crtc->pipe;
+ spin_lock_irq(&dev->event_lock);
+ if (exynos_crtc->event) {
+ ret = -EBUSY;
+ goto out;
+ }
- ret = drm_vblank_get(dev, exynos_crtc->pipe);
- if (ret) {
- DRM_DEBUG("failed to acquire vblank counter\n");
+ ret = drm_vblank_get(dev, exynos_crtc->pipe);
+ if (ret) {
+ DRM_DEBUG("failed to acquire vblank counter\n");
+ goto out;
+ }
- goto out;
- }
+ exynos_crtc->event = event;
+ spin_unlock_irq(&dev->event_lock);
+ /*
+* the pipe from user always is 0 so we can set pipe number
+* of current owner to event.
+*/
+ event->pipe = exynos_crtc->pipe;
+
+ crtc->primary->fb = fb;
+ crtc_w = fb->width - crtc->x;
+ crtc_h = fb->height - crtc->y;
+ ret = exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
+ crtc_w, crtc_h, crtc->x, crtc->y,
+ crtc_w, crtc_h);
+ if (ret) {
+ crtc->primary->fb = old_fb;
spin_lock_irq(&dev->event_lock);
- list_add_tail(&event->base.link,
- &dev_priv->pageflip_event_list);
- atomic_set(&exynos_crtc->pending_flip, 1);
+ exynos_crtc->event = NULL;
+ drm_vblank_put(dev, exynos_crtc->pipe);
spin_unlock_irq(&dev->event_lock);
-
- crtc->primary->fb = fb;
- crtc_w = fb->width - crtc->x;
- crtc_h = fb->height - crtc->y;
- ret = exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
- crtc_w, crtc_h, crtc->x, crtc->y,
- crtc_w, crtc_h);
- if (ret) {
- crtc->primary->fb = old_fb;
-
- spin_lock_irq(&dev->event_lock);
- drm_vblank_put(dev, exynos_crtc->pipe);
- list_del(&event->base.link);
- atomic_set(&exynos_crtc->pending_flip, 0);
- spin_unlock_irq(&dev->event_lock);
-
- goto out;
- }
+ return ret;
}
+
+ return 0;
+
out:
- mutex_
[PATCH 02/14] drm/exynos: Remove exynos_plane_dpms() call with no effect
From: Gustavo Padovan exynos_plane_dpms(DRM_MODE_DPMS_ON) calls the win_enable()'s callback from the underlying layer. However neither one of these layers implement win_enable() - FIMD, Mixer and VIDI. Thus the call to exynos_plane_dpms() is pointless. Signed-off-by: Gustavo Padovan --- drivers/gpu/drm/exynos/exynos_drm_crtc.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c index b2a4b84..ad675fb 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c @@ -65,8 +65,6 @@ static void exynos_drm_crtc_commit(struct drm_crtc *crtc) if (exynos_crtc->ops->commit) exynos_crtc->ops->commit(exynos_crtc); - - exynos_plane_dpms(crtc->primary, DRM_MODE_DPMS_ON); } static bool -- 1.9.3
[PATCH 03/14] drm/exynos: remove leftover functions declarations
From: Gustavo Padovan These functions were already removed by previous cleanup work, but these ones were left behind. Signed-off-by: Gustavo Padovan Acked-by: Joonyoung Shim --- drivers/gpu/drm/exynos/exynos_drm_crtc.h | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.h b/drivers/gpu/drm/exynos/exynos_drm_crtc.h index 6258b80..628b787 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.h +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.h @@ -27,12 +27,6 @@ void exynos_drm_crtc_disable_vblank(struct drm_device *dev, int pipe); void exynos_drm_crtc_finish_pageflip(struct drm_device *dev, int pipe); void exynos_drm_crtc_complete_scanout(struct drm_framebuffer *fb); -void exynos_drm_crtc_plane_mode_set(struct drm_crtc *crtc, - struct exynos_drm_plane *plane); -void exynos_drm_crtc_plane_commit(struct drm_crtc *crtc, int zpos); -void exynos_drm_crtc_plane_enable(struct drm_crtc *crtc, int zpos); -void exynos_drm_crtc_plane_disable(struct drm_crtc *crtc, int zpos); - /* This function gets pipe value to crtc device matched with out_type. */ int exynos_drm_crtc_get_pipe_from_type(struct drm_device *drm_dev, unsigned int out_type); -- 1.9.3
[PATCH 04/14] drm/exynos: remove struct *_win_data abstraction on planes
From: Gustavo Padovan
struct {fimd,mixer,vidi}_win_data was just keeping the same data
as struct exynos_drm_plane thus get ride of it and use exynos_drm_plane
directly.
It changes how planes are created and remove .win_mode_set() callback
that was only filling all *_win_data structs.
Signed-off-by: Gustavo Padovan
---
drivers/gpu/drm/exynos/exynos_drm_crtc.c | 9 +-
drivers/gpu/drm/exynos/exynos_drm_crtc.h | 1 +
drivers/gpu/drm/exynos/exynos_drm_drv.c | 14 --
drivers/gpu/drm/exynos/exynos_drm_drv.h | 5 +-
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 182 ++---
drivers/gpu/drm/exynos/exynos_drm_plane.c | 20 +--
drivers/gpu/drm/exynos/exynos_drm_plane.h | 6 +-
drivers/gpu/drm/exynos/exynos_drm_vidi.c | 123 +
drivers/gpu/drm/exynos/exynos_mixer.c | 212 +++---
9 files changed, 183 insertions(+), 389 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index ad675fb..d504f0b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -296,13 +296,13 @@ static void exynos_drm_crtc_attach_mode_property(struct
drm_crtc *crtc)
}
struct exynos_drm_crtc *exynos_drm_crtc_create(struct drm_device *drm_dev,
+ struct drm_plane *plane,
int pipe,
enum exynos_drm_output_type type,
struct exynos_drm_crtc_ops *ops,
void *ctx)
{
struct exynos_drm_crtc *exynos_crtc;
- struct drm_plane *plane;
struct exynos_drm_private *private = drm_dev->dev_private;
struct drm_crtc *crtc;
int ret;
@@ -318,12 +318,6 @@ struct exynos_drm_crtc *exynos_drm_crtc_create(struct
drm_device *drm_dev,
exynos_crtc->type = type;
exynos_crtc->ops = ops;
exynos_crtc->ctx = ctx;
- plane = exynos_plane_init(drm_dev, 1 << pipe,
- DRM_PLANE_TYPE_PRIMARY);
- if (IS_ERR(plane)) {
- ret = PTR_ERR(plane);
- goto err_plane;
- }
crtc = &exynos_crtc->base;
@@ -342,7 +336,6 @@ struct exynos_drm_crtc *exynos_drm_crtc_create(struct
drm_device *drm_dev,
err_crtc:
plane->funcs->destroy(plane);
-err_plane:
kfree(exynos_crtc);
return ERR_PTR(ret);
}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.h
b/drivers/gpu/drm/exynos/exynos_drm_crtc.h
index 628b787..0ecd8fc 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.h
@@ -18,6 +18,7 @@
#include "exynos_drm_drv.h"
struct exynos_drm_crtc *exynos_drm_crtc_create(struct drm_device *drm_dev,
+ struct drm_plane *plane,
int pipe,
enum exynos_drm_output_type type,
struct exynos_drm_crtc_ops *ops,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 737164d..778c91e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -55,7 +55,6 @@ static int exynos_drm_load(struct drm_device *dev, unsigned
long flags)
{
struct exynos_drm_private *private;
int ret;
- int nr;
private = kzalloc(sizeof(struct exynos_drm_private), GFP_KERNEL);
if (!private)
@@ -80,19 +79,6 @@ static int exynos_drm_load(struct drm_device *dev, unsigned
long flags)
exynos_drm_mode_config_init(dev);
- for (nr = 0; nr < MAX_PLANE; nr++) {
- struct drm_plane *plane;
- unsigned long possible_crtcs = (1 << MAX_CRTC) - 1;
-
- plane = exynos_plane_init(dev, possible_crtcs,
- DRM_PLANE_TYPE_OVERLAY);
- if (!IS_ERR(plane))
- continue;
-
- ret = PTR_ERR(plane);
- goto err_mode_config_cleanup;
- }
-
/* setup possible_clones. */
exynos_drm_encoder_setup(dev);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h
b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 7411af2..cad54e7 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -78,6 +78,7 @@ enum exynos_drm_output_type {
* @transparency: transparency on or off.
* @activated: activated or not.
* @enabled: enabled or not.
+ * @resume: to resume or not.
*
* this structure is common to exynos SoC and its contents would be copied
* to hardware specific overlay info.
@@ -112,6 +113,7 @@ struct exynos_drm_plane {
bool transparency:1;
bool activated:1;
bool enabled:1;
+ bool resume:1;
};
/*
@@
[PATCH 05/14] drm/exynos: do not copy adjusted mode into mode during crtc mode_set
From: Daniel Kurtz The 'mode' is the modeline information which specifies the ideal mode requested by the mode set initiator (usually userspace). The 'adjusted_mode' is the actual hardware mode after all the crtcs and encoders have had a chance to "fix it up". The adjusted_mode starts as a duplicate of the mode in drm_crtc_helper_set_mode(), and gets modified as required. There is no reason to touch the original requested mode. In fact, doing so will cause us to think a new mode is being requested whenever userspace tries to establish a new framebuffer with drmModeSetCrtc(), since userspace will still be using the ideal mode, but the crtc will be incorrectly comparing it against the adjusted_mode. Signed-off-by: Daniel Kurtz Signed-off-by: Gustavo Padovan --- drivers/gpu/drm/exynos/exynos_drm_crtc.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c index d504f0b..ecbcade 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c @@ -91,12 +91,6 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, unsigned int crtc_h; int ret; - /* -* copy the mode data adjusted by mode_fixup() into crtc->mode -* so that hardware can be seet to proper mode. -*/ - memcpy(&crtc->mode, adjusted_mode, sizeof(*adjusted_mode)); - ret = exynos_check_plane(crtc->primary, fb); if (ret < 0) return ret; -- 1.9.3
[PATCH 06/14] drm/exynos: atomic phase 1: use drm_plane_helper_update()
From: Gustavo Padovan
Rip out the check from exynos_update_plane() and create
exynos_check_plane() for the check phase enabling use to use
the atomic helpers to call our check and update phases when updating
planes.
Update all users of exynos_update_plane() accordingly to call
exynos_check_plane() before.
Signed-off-by: Gustavo Padovan
---
drivers/gpu/drm/exynos/exynos_drm_crtc.c | 29
drivers/gpu/drm/exynos/exynos_drm_plane.c | 37 ++-
drivers/gpu/drm/exynos/exynos_drm_plane.h | 2 +-
3 files changed, 43 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index ecbcade..09d4780 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -110,6 +110,7 @@ static int exynos_drm_crtc_mode_set_base(struct drm_crtc
*crtc, int x, int y,
struct drm_framebuffer *fb = crtc->primary->fb;
unsigned int crtc_w;
unsigned int crtc_h;
+ int ret;
/* when framebuffer changing is requested, crtc's dpms should be on */
if (exynos_crtc->dpms > DRM_MODE_DPMS_ON) {
@@ -117,11 +118,16 @@ static int exynos_drm_crtc_mode_set_base(struct drm_crtc
*crtc, int x, int y,
return -EPERM;
}
+ ret = exynos_check_plane(crtc->primary, fb);
+ if (ret)
+ return ret;
+
crtc_w = fb->width - x;
crtc_h = fb->height - y;
+ exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
+ crtc_w, crtc_h, x, y, crtc_w, crtc_h);
- return exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
- crtc_w, crtc_h, x, y, crtc_w, crtc_h);
+ return 0;
}
static void exynos_drm_crtc_disable(struct drm_crtc *crtc)
@@ -158,7 +164,6 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc,
{
struct drm_device *dev = crtc->dev;
struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
- struct drm_framebuffer *old_fb = crtc->primary->fb;
unsigned int crtc_w, crtc_h;
int ret;
@@ -177,6 +182,10 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc,
goto out;
}
+ ret = exynos_check_plane(crtc->primary, fb);
+ if (ret)
+ goto out;
+
ret = drm_vblank_get(dev, exynos_crtc->pipe);
if (ret) {
DRM_DEBUG("failed to acquire vblank counter\n");
@@ -195,17 +204,9 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc,
crtc->primary->fb = fb;
crtc_w = fb->width - crtc->x;
crtc_h = fb->height - crtc->y;
- ret = exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
- crtc_w, crtc_h, crtc->x, crtc->y,
- crtc_w, crtc_h);
- if (ret) {
- crtc->primary->fb = old_fb;
- spin_lock_irq(&dev->event_lock);
- exynos_crtc->event = NULL;
- drm_vblank_put(dev, exynos_crtc->pipe);
- spin_unlock_irq(&dev->event_lock);
- return ret;
- }
+ exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
+ crtc_w, crtc_h, crtc->x, crtc->y,
+ crtc_w, crtc_h);
return 0;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c
b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index dc13621..2c356b9 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -167,21 +167,15 @@ void exynos_plane_dpms(struct drm_plane *plane, int mode)
}
}
-int
+void
exynos_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
struct drm_framebuffer *fb, int crtc_x, int crtc_y,
unsigned int crtc_w, unsigned int crtc_h,
uint32_t src_x, uint32_t src_y,
uint32_t src_w, uint32_t src_h)
{
-
struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
- int ret;
-
- ret = exynos_check_plane(plane, fb);
- if (ret < 0)
- return ret;
exynos_plane_mode_set(plane, crtc, fb, crtc_x, crtc_y,
crtc_w, crtc_h, src_x >> 16, src_y >> 16,
@@ -189,8 +183,6 @@ exynos_update_plane(struct drm_plane *plane, struct
drm_crtc *crtc,
if (exynos_crtc->ops->win_commit)
exynos_crtc->ops->win_commit(exynos_crtc, exynos_plane->zpos);
-
- return 0;
}
static int exynos_disable_plane(struct drm_plane *plane)
@@ -226,12 +218,35 @@ static int exynos_plane_set_property(struct drm_plane
*plane,
}
static struct drm_plane_funcs exynos_plane_funcs = {
- .update_plane = exynos_update_plane,
+ .update_plane = drm_plane_helper_update,
.disable_plane = exynos_disable_plane,
.des
[PATCH 07/14] drm/exynos: atomic phase 1: use drm_plane_helper_disable()
From: Gustavo Padovan
The atomic helper to disable planes also uses the optional
.atomic_disable() helper. The unique operation it does is calling
.win_disable()
exynos_drm_fb_get_buf_cnt() needs a fb check too to avoid a null pointer.
Signed-off-by: Gustavo Padovan
---
drivers/gpu/drm/exynos/exynos_drm_fb.c| 2 +-
drivers/gpu/drm/exynos/exynos_drm_plane.c | 14 +-
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c
b/drivers/gpu/drm/exynos/exynos_drm_fb.c
index d346d1e..470456d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
@@ -136,7 +136,7 @@ unsigned int exynos_drm_fb_get_buf_cnt(struct
drm_framebuffer *fb)
exynos_fb = to_exynos_fb(fb);
- return exynos_fb->buf_cnt;
+ return exynos_fb ? exynos_fb->buf_cnt : 0;
}
struct drm_framebuffer *
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c
b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index 2c356b9..a3b0687 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -219,7 +219,7 @@ static int exynos_plane_set_property(struct drm_plane
*plane,
static struct drm_plane_funcs exynos_plane_funcs = {
.update_plane = drm_plane_helper_update,
- .disable_plane = exynos_disable_plane,
+ .disable_plane = drm_plane_helper_disable,
.destroy= exynos_plane_destroy,
.set_property = exynos_plane_set_property,
};
@@ -242,9 +242,21 @@ static void exynos_plane_atomic_update(struct drm_plane
*plane,
state->src_w >> 16, state->src_h >> 16);
}
+static void exynos_plane_atomic_disable(struct drm_plane *plane,
+ struct drm_plane_state *old_state)
+{
+ struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
+ struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(old_state->crtc);
+
+ if (exynos_crtc->ops->win_disable)
+ exynos_crtc->ops->win_disable(exynos_crtc,
+ exynos_plane->zpos);
+}
+
static const struct drm_plane_helper_funcs plane_helper_funcs = {
.atomic_check = exynos_plane_atomic_check,
.atomic_update = exynos_plane_atomic_update,
+ .atomic_disable = exynos_plane_atomic_disable,
};
static void exynos_plane_attach_zpos_property(struct drm_plane *plane)
--
1.9.3
[PATCH 08/14] drm/exynos: atomic phase 1: add atomic_begin()/atomic_flush()
From: Gustavo Padovan
Add CRTC callbacks .atomic_begin() .atomic_flush(). On exynos they
unprotect the windows before the commit and protects it after based on
a plane mask tha store which plane will be updated.
For that we create two new exynos_crtc callbacks: .win_protect() and
.win_unprotect(). The only driver that implement those now is FIMD.
Signed-off-by: Gustavo Padovan
---
drivers/gpu/drm/exynos/exynos_drm_crtc.c | 34 ++
drivers/gpu/drm/exynos/exynos_drm_drv.h | 4 +++
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 57 ++-
drivers/gpu/drm/exynos/exynos_drm_plane.c | 4 +++
4 files changed, 82 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 09d4780..be36cca 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -147,6 +147,38 @@ static void exynos_drm_crtc_disable(struct drm_crtc *crtc)
}
}
+static void exynos_crtc_atomic_begin(struct drm_crtc *crtc)
+{
+ struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
+ struct drm_plane *plane;
+ int index = 0;
+
+ list_for_each_entry(plane, &crtc->dev->mode_config.plane_list, head) {
+ if (exynos_crtc->ops->win_protect &&
+ exynos_crtc->plane_mask & (0x01 << index))
+ exynos_crtc->ops->win_protect(exynos_crtc, index);
+
+ index++;
+ }
+}
+
+static void exynos_crtc_atomic_flush(struct drm_crtc *crtc)
+{
+ struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
+ struct drm_plane *plane;
+ int index = 0;
+
+ list_for_each_entry(plane, &crtc->dev->mode_config.plane_list, head) {
+ if (exynos_crtc->ops->win_unprotect &&
+ exynos_crtc->plane_mask & (0x01 << index))
+ exynos_crtc->ops->win_unprotect(exynos_crtc, index);
+
+ index++;
+ }
+
+ exynos_crtc->plane_mask = 0;
+}
+
static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
.dpms = exynos_drm_crtc_dpms,
.prepare= exynos_drm_crtc_prepare,
@@ -155,6 +187,8 @@ static struct drm_crtc_helper_funcs
exynos_crtc_helper_funcs = {
.mode_set = exynos_drm_crtc_mode_set,
.mode_set_base = exynos_drm_crtc_mode_set_base,
.disable= exynos_drm_crtc_disable,
+ .atomic_begin = exynos_crtc_atomic_begin,
+ .atomic_flush = exynos_crtc_atomic_flush,
};
static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h
b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index cad54e7..43efd9f 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -194,6 +194,8 @@ struct exynos_drm_crtc_ops {
void (*win_enable)(struct exynos_drm_crtc *crtc, int zpos);
void (*win_disable)(struct exynos_drm_crtc *crtc, int zpos);
void (*te_handler)(struct exynos_drm_crtc *crtc);
+ void (*win_protect)(struct exynos_drm_crtc *crtc, int zpos);
+ void (*win_unprotect)(struct exynos_drm_crtc *crtc, int zpos);
};
enum exynos_crtc_mode {
@@ -214,6 +216,7 @@ enum exynos_crtc_mode {
* we can refer to the crtc to current hardware interrupt occurred through
* this pipe value.
* @dpms: store the crtc dpms value
+ * @plane_mask: store planes to be updated on atomic modesetting
* @mode: store the crtc mode value
* @event: vblank event that is currently queued for flip
* @ops: pointer to callbacks for exynos drm specific functionality
@@ -224,6 +227,7 @@ struct exynos_drm_crtc {
enum exynos_drm_output_type type;
unsigned intpipe;
unsigned intdpms;
+ unsigned intplane_mask;
enum exynos_crtc_mode mode;
wait_queue_head_t pending_flip_queue;
struct drm_pending_vblank_event *event;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index ebb4cdc..f498d86 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -585,6 +585,16 @@ static void fimd_shadow_protect_win(struct fimd_context
*ctx,
{
u32 reg, bits, val;
+ /*
+* SHADOWCON/PRTCON register is used for enabling timing.
+*
+* for example, once only width value of a register is set,
+* if the dma is started then fimd hardware could malfunction so
+* with protect window setting, the register fields with prefix '_F'
+* wouldn't be updated at vsync also but updated once unprotect window
+* is set.
+*/
+
if (ctx->driver_data->has_shadowcon) {
reg = SHADOWCON;
bits = SHADOWCON_WINx_PROTECT(win);
@@ -627,20 +637,6 @@ static void fim
[PATCH 09/14] drm/exynos: atomic phase 1: add .mode_set_nofb() callback
From: Gustavo Padovan
The new atomic infrastructure needs the .mode_set_nofb() callback to
update CRTC timings before setting any plane.
Signed-off-by: Gustavo Padovan
Conflicts:
drivers/gpu/drm/exynos/exynos_drm_crtc.c
---
drivers/gpu/drm/exynos/exynos_drm_crtc.c | 55 +++-
1 file changed, 11 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index be36cca..17b64f8 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -81,53 +81,19 @@ exynos_drm_crtc_mode_fixup(struct drm_crtc *crtc,
return true;
}
-static int
-exynos_drm_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode, int x, int y,
- struct drm_framebuffer *old_fb)
-{
- struct drm_framebuffer *fb = crtc->primary->fb;
- unsigned int crtc_w;
- unsigned int crtc_h;
- int ret;
-
- ret = exynos_check_plane(crtc->primary, fb);
- if (ret < 0)
- return ret;
-
- crtc_w = fb->width - x;
- crtc_h = fb->height - y;
- exynos_plane_mode_set(crtc->primary, crtc, fb, 0, 0,
- crtc_w, crtc_h, x, y, crtc_w, crtc_h);
-
- return 0;
-}
-
-static int exynos_drm_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
- struct drm_framebuffer *old_fb)
+static void
+exynos_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
{
struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
- struct drm_framebuffer *fb = crtc->primary->fb;
- unsigned int crtc_w;
- unsigned int crtc_h;
- int ret;
+ struct drm_display_mode *adjusted_mode;
- /* when framebuffer changing is requested, crtc's dpms should be on */
- if (exynos_crtc->dpms > DRM_MODE_DPMS_ON) {
- DRM_ERROR("failed framebuffer changing request.\n");
- return -EPERM;
- }
-
- ret = exynos_check_plane(crtc->primary, fb);
- if (ret)
- return ret;
+ if (WARN_ON(!crtc->state))
+ return;
- crtc_w = fb->width - x;
- crtc_h = fb->height - y;
- exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
- crtc_w, crtc_h, x, y, crtc_w, crtc_h);
+ adjusted_mode = &crtc->state->adjusted_mode;
- return 0;
+ if (exynos_crtc->ops->commit)
+ exynos_crtc->ops->commit(exynos_crtc);
}
static void exynos_drm_crtc_disable(struct drm_crtc *crtc)
@@ -184,8 +150,9 @@ static struct drm_crtc_helper_funcs
exynos_crtc_helper_funcs = {
.prepare= exynos_drm_crtc_prepare,
.commit = exynos_drm_crtc_commit,
.mode_fixup = exynos_drm_crtc_mode_fixup,
- .mode_set = exynos_drm_crtc_mode_set,
- .mode_set_base = exynos_drm_crtc_mode_set_base,
+ .mode_set = drm_helper_crtc_mode_set,
+ .mode_set_nofb = exynos_drm_crtc_mode_set_nofb,
+ .mode_set_base = drm_helper_crtc_mode_set_base,
.disable= exynos_drm_crtc_disable,
.atomic_begin = exynos_crtc_atomic_begin,
.atomic_flush = exynos_crtc_atomic_flush,
--
1.9.3
[PATCH 10/14] drm/exynos: atomic phase 2: wire up state reset(), duplicate() and destroy()
From: Gustavo Padovan
Set CRTC, planes and connectors to use the default implementations from
the atomic helper library. The helpers will work to keep track of state
for each DRM object.
Signed-off-by: Gustavo Padovan
---
drivers/gpu/drm/bridge/ptn3460.c | 4
drivers/gpu/drm/exynos/exynos_dp_core.c | 4
drivers/gpu/drm/exynos/exynos_drm_connector.c | 4
drivers/gpu/drm/exynos/exynos_drm_crtc.c | 5 +
drivers/gpu/drm/exynos/exynos_drm_dpi.c | 4
drivers/gpu/drm/exynos/exynos_drm_drv.c | 2 ++
drivers/gpu/drm/exynos/exynos_drm_dsi.c | 4
drivers/gpu/drm/exynos/exynos_drm_plane.c | 4
drivers/gpu/drm/exynos/exynos_drm_vidi.c | 4
drivers/gpu/drm/exynos/exynos_hdmi.c | 4
10 files changed, 39 insertions(+)
diff --git a/drivers/gpu/drm/bridge/ptn3460.c b/drivers/gpu/drm/bridge/ptn3460.c
index 826833e..30da10c 100644
--- a/drivers/gpu/drm/bridge/ptn3460.c
+++ b/drivers/gpu/drm/bridge/ptn3460.c
@@ -27,6 +27,7 @@
#include "drm_crtc.h"
#include "drm_crtc_helper.h"
+#include "drm_atomic_helper.h"
#include "drm_edid.h"
#include "drmP.h"
@@ -263,6 +264,9 @@ static struct drm_connector_funcs ptn3460_connector_funcs =
{
.fill_modes = drm_helper_probe_single_connector_modes,
.detect = ptn3460_detect,
.destroy = ptn3460_connector_destroy,
+ .reset = drm_atomic_helper_connector_reset,
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
};
int ptn3460_bridge_attach(struct drm_bridge *bridge)
diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c
b/drivers/gpu/drm/exynos/exynos_dp_core.c
index bf17a60..6704d5c 100644
--- a/drivers/gpu/drm/exynos/exynos_dp_core.c
+++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
#include
#include
@@ -952,6 +953,9 @@ static struct drm_connector_funcs exynos_dp_connector_funcs
= {
.fill_modes = drm_helper_probe_single_connector_modes,
.detect = exynos_dp_detect,
.destroy = exynos_dp_connector_destroy,
+ .reset = drm_atomic_helper_connector_reset,
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
};
static int exynos_dp_get_modes(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_connector.c
b/drivers/gpu/drm/exynos/exynos_drm_connector.c
index ba9b3d5..980b085 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_connector.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_connector.c
@@ -13,6 +13,7 @@
#include
#include
+#include
#include
#include "exynos_drm_drv.h"
@@ -182,6 +183,9 @@ static struct drm_connector_funcs exynos_connector_funcs = {
.fill_modes = exynos_drm_connector_fill_modes,
.detect = exynos_drm_connector_detect,
.destroy= exynos_drm_connector_destroy,
+ .reset = drm_atomic_helper_connector_reset,
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
};
struct drm_connector *exynos_drm_connector_create(struct drm_device *dev,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 17b64f8..660ad64 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -14,6 +14,8 @@
#include
#include
+#include
+#include
#include "exynos_drm_crtc.h"
#include "exynos_drm_drv.h"
@@ -265,6 +267,9 @@ static struct drm_crtc_funcs exynos_crtc_funcs = {
.page_flip = exynos_drm_crtc_page_flip,
.destroy= exynos_drm_crtc_destroy,
.set_property = exynos_drm_crtc_set_property,
+ .reset = drm_atomic_helper_crtc_reset,
+ .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
};
static const struct drm_prop_enum_list mode_names[] = {
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c
b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
index 37678cf..ced5c23 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include
@@ -63,6 +64,9 @@ static struct drm_connector_funcs exynos_dpi_connector_funcs
= {
.detect = exynos_dpi_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
.destroy = exynos_dpi_connector_destroy,
+ .reset = drm_atomic_helper_connector_reset,
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
};
static int exynos_dpi_get_modes(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/exynos
[PATCH 11/14] drm/exynos: atomic phase 2: keep track of framebuffer pointer
From: Gustavo Padovan Use drm_atomic_set_fb_for_plane() in the legacy page_flip path to keep track of the framebuffer pointer and reference. Signed-off-by: Gustavo Padovan --- drivers/gpu/drm/exynos/exynos_drm_crtc.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c index 660ad64..2edc73c 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c @@ -211,6 +211,9 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc, crtc_w, crtc_h, crtc->x, crtc->y, crtc_w, crtc_h); + if (crtc->primary->state) + drm_atomic_set_fb_for_plane(crtc->primary->state, fb); + return 0; out: -- 1.9.3
[PATCH 12/14] drm/exynos: make exynos_plane_mode_set() static
From: Gustavo Padovan
It is not used outside of the plane scope anymore.
Signed-off-by: Gustavo Padovan
---
drivers/gpu/drm/exynos/exynos_drm_plane.c | 11 ++-
drivers/gpu/drm/exynos/exynos_drm_plane.h | 5 -
2 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c
b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index 0980c84..c0c53d6 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -91,11 +91,12 @@ int exynos_check_plane(struct drm_plane *plane, struct
drm_framebuffer *fb)
return 0;
}
-void exynos_plane_mode_set(struct drm_plane *plane, struct drm_crtc *crtc,
- struct drm_framebuffer *fb, int crtc_x, int crtc_y,
- unsigned int crtc_w, unsigned int crtc_h,
- uint32_t src_x, uint32_t src_y,
- uint32_t src_w, uint32_t src_h)
+static void exynos_plane_mode_set(struct drm_plane *plane, struct drm_crtc
*crtc,
+ struct drm_framebuffer *fb,
+ int crtc_x, int crtc_y,
+ unsigned int crtc_w, unsigned int crtc_h,
+ uint32_t src_x, uint32_t src_y,
+ uint32_t src_w, uint32_t src_h)
{
struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
unsigned int actual_w;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.h
b/drivers/gpu/drm/exynos/exynos_drm_plane.h
index 8361606..8cdca0d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.h
@@ -10,11 +10,6 @@
*/
int exynos_check_plane(struct drm_plane *plane, struct drm_framebuffer *fb);
-void exynos_plane_mode_set(struct drm_plane *plane, struct drm_crtc *crtc,
- struct drm_framebuffer *fb, int crtc_x, int crtc_y,
- unsigned int crtc_w, unsigned int crtc_h,
- uint32_t src_x, uint32_t src_y,
- uint32_t src_w, uint32_t src_h);
void exynos_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
struct drm_framebuffer *fb, int crtc_x, int crtc_y,
unsigned int crtc_w, unsigned int crtc_h,
--
1.9.3
[PATCH 13/14] drm/exynos: use correct pipe number on vblank event
From: Gustavo Padovan
Use the pipe var received instead of using always -1 as pipe value.
Signed-off-by: Gustavo Padovan
---
drivers/gpu/drm/exynos/exynos_drm_crtc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 2edc73c..0f2b789 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -382,7 +382,7 @@ void exynos_drm_crtc_finish_pageflip(struct drm_device
*dev, int pipe)
spin_lock_irqsave(&dev->event_lock, flags);
if (exynos_crtc->event) {
- drm_send_vblank_event(dev, -1, exynos_crtc->event);
+ drm_send_vblank_event(dev, pipe, exynos_crtc->event);
drm_vblank_put(dev, pipe);
wake_up(&exynos_crtc->pending_flip_queue);
--
1.9.3
[PATCH 14/14] drm/exynos: remove exynos_disable_plane()
From: Gustavo Padovan
exynos_disable_plane() is used only once now thus remove it and call
exynos_plane_dpms() directly.
Signed-off-by: Gustavo Padovan
---
drivers/gpu/drm/exynos/exynos_drm_plane.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c
b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index c0c53d6..34dcf17b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -191,18 +191,11 @@ exynos_update_plane(struct drm_plane *plane, struct
drm_crtc *crtc,
exynos_crtc->ops->win_commit(exynos_crtc, exynos_plane->zpos);
}
-static int exynos_disable_plane(struct drm_plane *plane)
-{
- exynos_plane_dpms(plane, DRM_MODE_DPMS_OFF);
-
- return 0;
-}
-
static void exynos_plane_destroy(struct drm_plane *plane)
{
struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
- exynos_disable_plane(plane);
+ exynos_plane_dpms(plane, DRM_MODE_DPMS_OFF);
drm_plane_cleanup(plane);
kfree(exynos_plane);
}
--
1.9.3
[Linaro-mm-sig] [RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms
On Tue, Feb 03, 2015 at 05:36:59PM +0100, Arnd Bergmann wrote: > On Tuesday 03 February 2015 11:22:01 Rob Clark wrote: > > On Tue, Feb 3, 2015 at 11:12 AM, Arnd Bergmann wrote: > > > I agree for the case you are describing here. From what I understood > > > from Rob was that he is looking at something more like: > > > > > > Fig 3 > > > CPU--L1cache--L2cache--Memory--IOMMU-device > > > > > > where the IOMMU controls one or more contexts per device, and is > > > shared across GPU and non-GPU devices. Here, we need to use the > > > dmap-mapping interface to set up the IO page table for any device > > > that is unable to address all of system RAM, and we can use it > > > for purposes like isolation of the devices. There are also cases > > > where using the IOMMU is not optional. > > > > > > Actually, just to clarify, the IOMMU instance is specific to the GPU.. > > not shared with other devices. Otherwise managing multiple contexts > > would go quite badly.. > > > > But other devices have their own instance of the same IOMMU.. so same > > driver could be used. > > I think from the driver perspective, I'd view those two cases as > identical. Not sure if Russell agrees with that. Imo whether the iommu is private to the device and required for gpu functionality like context switching or shared across a bunch of devices is fairly important. Assuming I understand this discussion correctly we have two different things pulling in opposite directions: - From a gpu functionality perspective we want to give the gpu driver full control over the device-private iommu, pushing it out of the control of the dma api. dma_map_sg would just map to whatever bus addresses that iommu would need to use for generating access cycles. This is the design used by every gpu driver we have in upstream thus far (where you always have some on-gpu iommu/pagetable walker thing), on top of whatever system iommu that might be there or not (which is then managed by the dma apis). - On many soc people love to reuse iommus with the same or similar interface all over the place. The solution thus far adopted on arm platforms is to write an iommu driver for those and then implement the dma-api on top of this iommu. But if we unconditionally do this then we rob the gpu driver's ability to control its private iommu like it wants to, because a lot of the functionality is lost behind the dma api abstraction. Again assuming I'm not confused can't we just solve this by pushing the dma api abstraction down one layer for just the gpu, and let it use its private iommmu directly? Steps for binding a buffer would be: 1. dma_map_sg 2. Noodle the dma_addr_t out of the sg table and feed those into a 2nd level mapping set up through the iommu api for the gpu-private mmu. Again, this is what i915 and all the ttm based drivers already do, except that we don't use the generic iommu interfaces but have our own (i915 has its interface in i915_gem_gtt.c, ttm just calls them tt for translation tables ...). Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
