dri-devel June 2015

dri-devel@lists.freedesktop.org

188 participants
1140 discussions

[PATCH 8/9] [media] drm/exynos: Convert g2d_userptr_get_dma_addr() to use get_vaddr_frames()
by Mauro Carvalho Chehab 10 Jun '15

10 Jun '15

From: Jan Kara <jack(a)suse.cz> Convert g2d_userptr_get_dma_addr() to pin pages using get_vaddr_frames(). This removes the knowledge about vmas and mmap_sem locking from exynos driver. Also it fixes a problem that the function has been mapping user provided address without holding mmap_sem. Signed-off-by: Jan Kara <jack(a)suse.cz> Signed-off-by: Hans Verkuil <hans.verkuil(a)cisco.com> Signed-off-by: Mauro Carvalho Chehab <mchehab(a)osg.samsung.com> diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c index 81a250830808..810e1ee7c07d 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c +++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c @@ -190,10 +190,8 @@ struct g2d_cmdlist_userptr { dma_addr_t dma_addr; unsigned long userptr; unsigned long size; - struct page **pages; - unsigned int npages; + struct frame_vector *vec; struct sg_table *sgt; - struct vm_area_struct *vma; atomic_t refcount; bool in_pool; bool out_of_list; @@ -363,6 +361,7 @@ static void g2d_userptr_put_dma_addr(struct drm_device *drm_dev, { struct g2d_cmdlist_userptr *g2d_userptr = (struct g2d_cmdlist_userptr *)obj; + struct page **pages; if (!obj) return; @@ -382,19 +381,21 @@ out: exynos_gem_unmap_sgt_from_dma(drm_dev, g2d_userptr->sgt, DMA_BIDIRECTIONAL); - exynos_gem_put_pages_to_userptr(g2d_userptr->pages, - g2d_userptr->npages, - g2d_userptr->vma); + pages = frame_vector_pages(g2d_userptr->vec); + if (!IS_ERR(pages)) { + int i; - exynos_gem_put_vma(g2d_userptr->vma); + for (i = 0; i < frame_vector_count(g2d_userptr->vec); i++) + set_page_dirty_lock(pages[i]); + } + put_vaddr_frames(g2d_userptr->vec); + frame_vector_destroy(g2d_userptr->vec); if (!g2d_userptr->out_of_list) list_del_init(&g2d_userptr->list); sg_free_table(g2d_userptr->sgt); kfree(g2d_userptr->sgt); - - drm_free_large(g2d_userptr->pages); kfree(g2d_userptr); } @@ -408,9 +409,7 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct drm_device *drm_dev, struct exynos_drm_g2d_private *g2d_priv = file_priv->g2d_priv; struct g2d_cmdlist_userptr *g2d_userptr; struct g2d_data *g2d; - struct page **pages; struct sg_table *sgt; - struct vm_area_struct *vma; unsigned long start, end; unsigned int npages, offset; int ret; @@ -456,65 +455,38 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct drm_device *drm_dev, return ERR_PTR(-ENOMEM); atomic_set(&g2d_userptr->refcount, 1); + g2d_userptr->size = size; start = userptr & PAGE_MASK; offset = userptr & ~PAGE_MASK; end = PAGE_ALIGN(userptr + size); npages = (end - start) >> PAGE_SHIFT; - g2d_userptr->npages = npages; - - pages = drm_calloc_large(npages, sizeof(struct page *)); - if (!pages) { - DRM_ERROR("failed to allocate pages.\n"); - ret = -ENOMEM; + g2d_userptr->vec = frame_vector_create(npages); + if (!vec) goto err_free; - } - down_read(&current->mm->mmap_sem); - vma = find_vma(current->mm, userptr); - if (!vma) { - up_read(&current->mm->mmap_sem); - DRM_ERROR("failed to get vm region.\n"); - ret = -EFAULT; - goto err_free_pages; - } - - if (vma->vm_end < userptr + size) { - up_read(&current->mm->mmap_sem); - DRM_ERROR("vma is too small.\n"); - ret = -EFAULT; - goto err_free_pages; - } - - g2d_userptr->vma = exynos_gem_get_vma(vma); - if (!g2d_userptr->vma) { - up_read(&current->mm->mmap_sem); - DRM_ERROR("failed to copy vma.\n"); - ret = -ENOMEM; - goto err_free_pages; - } - - g2d_userptr->size = size; - - ret = exynos_gem_get_pages_from_userptr(start & PAGE_MASK, - npages, pages, vma); - if (ret < 0) { - up_read(&current->mm->mmap_sem); + ret = get_vaddr_frames(start, npages, true, true, g2d_userptr->vec); + if (ret != npages) { DRM_ERROR("failed to get user pages from userptr.\n"); - goto err_put_vma; + if (ret < 0) + goto err_destroy_framevec; + ret = -EFAULT; + goto err_put_framevec; + } + if (frame_vector_to_pages(g2d_userptr->vec) < 0) { + ret = -EFAULT; + goto err_put_framevec; } - - up_read(&current->mm->mmap_sem); - g2d_userptr->pages = pages; sgt = kzalloc(sizeof(*sgt), GFP_KERNEL); if (!sgt) { ret = -ENOMEM; - goto err_free_userptr; + goto err_put_framevec; } - ret = sg_alloc_table_from_pages(sgt, pages, npages, offset, - size, GFP_KERNEL); + ret = sg_alloc_table_from_pages(sgt, + frame_vector_pages(g2d_userptr->vec), + npages, offset, size, GFP_KERNEL); if (ret < 0) { DRM_ERROR("failed to get sgt from pages.\n"); goto err_free_sgt; @@ -549,16 +521,11 @@ err_sg_free_table: err_free_sgt: kfree(sgt); -err_free_userptr: - exynos_gem_put_pages_to_userptr(g2d_userptr->pages, - g2d_userptr->npages, - g2d_userptr->vma); +err_put_framevec: + put_vaddr_frames(g2d_userptr->vec); -err_put_vma: - exynos_gem_put_vma(g2d_userptr->vma); - -err_free_pages: - drm_free_large(pages); +err_destroy_framevec: + frame_vector_destroy(g2d_userptr->vec); err_free: kfree(g2d_userptr); diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c index 0d5b9698d384..47068ae44ced 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c @@ -378,103 +378,6 @@ int exynos_drm_gem_get_ioctl(struct drm_device *dev, void *data, return 0; } -struct vm_area_struct *exynos_gem_get_vma(struct vm_area_struct *vma) -{ - struct vm_area_struct *vma_copy; - - vma_copy = kmalloc(sizeof(*vma_copy), GFP_KERNEL); - if (!vma_copy) - return NULL; - - if (vma->vm_ops && vma->vm_ops->open) - vma->vm_ops->open(vma); - - if (vma->vm_file) - get_file(vma->vm_file); - - memcpy(vma_copy, vma, sizeof(*vma)); - - vma_copy->vm_mm = NULL; - vma_copy->vm_next = NULL; - vma_copy->vm_prev = NULL; - - return vma_copy; -} - -void exynos_gem_put_vma(struct vm_area_struct *vma) -{ - if (!vma) - return; - - if (vma->vm_ops && vma->vm_ops->close) - vma->vm_ops->close(vma); - - if (vma->vm_file) - fput(vma->vm_file); - - kfree(vma); -} - -int exynos_gem_get_pages_from_userptr(unsigned long start, - unsigned int npages, - struct page **pages, - struct vm_area_struct *vma) -{ - int get_npages; - - /* the memory region mmaped with VM_PFNMAP. */ - if (vma_is_io(vma)) { - unsigned int i; - - for (i = 0; i < npages; ++i, start += PAGE_SIZE) { - unsigned long pfn; - int ret = follow_pfn(vma, start, &pfn); - if (ret) - return ret; - - pages[i] = pfn_to_page(pfn); - } - - if (i != npages) { - DRM_ERROR("failed to get user_pages.\n"); - return -EINVAL; - } - - return 0; - } - - get_npages = get_user_pages(current, current->mm, start, - npages, 1, 1, pages, NULL); - get_npages = max(get_npages, 0); - if (get_npages != npages) { - DRM_ERROR("failed to get user_pages.\n"); - while (get_npages) - put_page(pages[--get_npages]); - return -EFAULT; - } - - return 0; -} - -void exynos_gem_put_pages_to_userptr(struct page **pages, - unsigned int npages, - struct vm_area_struct *vma) -{ - if (!vma_is_io(vma)) { - unsigned int i; - - for (i = 0; i < npages; i++) { - set_page_dirty_lock(pages[i]); - - /* - * undo the reference we took when populating - * the table. - */ - put_page(pages[i]); - } - } -} - int exynos_gem_map_sgt_with_dma(struct drm_device *drm_dev, struct sg_table *sgt, enum dma_data_direction dir) -- 2.4.2

1 0

[PATCH 0/9 v5] Helper to abstract vma handling in media layer
by Jan Kara 10 Jun '15

10 Jun '15

Hello, I'm sending the fifth version of my patch series to abstract vma handling from the various media drivers. The patches got some review from mm people and testing from device driver guys so unless someone objects, patches will be queued in media tree for the next merge window. After this patch set drivers have to know much less details about vmas, their types, and locking. Also quite some code is removed from them. As a bonus drivers get automatically VM_FAULT_RETRY handling. The primary motivation for this series is to remove knowledge about mmap_sem locking from as many places a possible so that we can change it with reasonable effort. The core of the series is the new helper get_vaddr_frames() which is given a virtual address and it fills in PFNs / struct page pointers (depending on VMA type) into the provided array. If PFNs correspond to normal pages it also grabs references to these pages. The difference from get_user_pages() is that this function can also deal with pfnmap, and io mappings which is what the media drivers need. I have tested the patches with vivid driver so at least vb2 code got some exposure. Conversion of other drivers was just compile-tested (for x86 so e.g. exynos driver which is only for Samsung platform is completely untested). Honza Changes since v4: * Minor cleanups and fixes pointed out by Mel and Vlasta * Added Acked-by tags Changes since v3: * Added include <linux/vmalloc.h> into mm/gup.c as it's needed for some archs * Fixed error path for exynos driver Changes since v2: * Renamed functions and structures as Mel suggested * Other minor changes suggested by Mel * Rebased on top of 4.1-rc2 * Changed functions to get pointer to array of pages / pfns to perform conversion if necessary. This fixes possible issue in the omap I may have introduced in v2 and generally makes the API less errorprone.

4 20

[Bug 99651] New: third monitor connected via dvi does not work on radeonhd 7770
by bugzilla-daemon＠bugzilla.kernel.org 10 Jun '15

10 Jun '15

https://bugzilla.kernel.org/show_bug.cgi?id=99651 Bug ID: 99651 Summary: third monitor connected via dvi does not work on radeonhd 7770 Product: Drivers Version: 2.5 Kernel Version: 4.0.5 and higher (tested on 4.1-rc7 also) Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: high Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-dri(a)kernel-bugs.osdl.org Reporter: sss123next(a)list.ru Regression: No Created attachment 179171 --> https://bugzilla.kernel.org/attachment.cgi?id=179171&action=edit dmesg with some drm errors after kernel update to 4.0.5 my third monitor stopped working (black screen with blinking standby indicator), but console resolution is adjusted to lowest monitor resolution, the one which is not working, so i guess driver see it and trying to use, look in dmesg in attachment -- You are receiving this mail because: You are watching the assignee of the bug.

1 7

[Bug 90889] Radeon driver with radeon.dpm=1 causes lockout
by bugzilla-daemon＠freedesktop.org 09 Jun '15

09 Jun '15

https://bugs.freedesktop.org/show_bug.cgi?id=90889 --- Comment #2 from lolwoot1234(a)gmail.com --- Lockout as in no systemd services working, can't ssh into it or switch tty/vt and getting a blank screen. Here is the systemd journal log when booting with the default radeon.dpm=1: http://pastebin.com/raw.php?i=SRpX7QLK If by microcode you mean the early KMS/initramfs, yes, it is installed correctly and works with radeon.dpm=0. -- You are receiving this mail because: You are the assignee for the bug.

1 0

[PATCH 06/15] gpu/drm: kill off set_irq_flags usage
by Rob Herring 09 Jun '15

09 Jun '15

set_irq_flags is ARM specific with custom flags which have genirq equivalents. Convert drivers to use the genirq interfaces directly, so we can kill off set_irq_flags. The translation of flags is as follows: IRQF_VALID -> !IRQ_NOREQUEST IRQF_PROBE -> !IRQ_NOPROBE IRQF_NOAUTOEN -> IRQ_NOAUTOEN For IRQs managed by an irqdomain, the irqdomain core code handles clearing and setting IRQ_NOREQUEST already, so there is no need to do this in .map() functions and we can simply remove the set_irq_flags calls. Some users also set IRQ_NOPROBE and this has been maintained although it is not clear that is really needed. There appears to be a great deal of blind copy and paste of this code. Signed-off-by: Rob Herring <robh(a)kernel.org> Cc: David Airlie <airlied(a)linux.ie> Cc: dri-devel(a)lists.freedesktop.org --- drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c | 1 - drivers/gpu/ipu-v3/ipu-common.c | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c index 33bd4c6..9a6a747 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c @@ -165,7 +165,6 @@ static int mdp5_hw_irqdomain_map(struct irq_domain *d, irq_set_chip_and_handler(irq, &mdp5_hw_irq_chip, handle_level_irq); irq_set_chip_data(irq, mdp5_kms); - set_irq_flags(irq, IRQF_VALID); return 0; } diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c index 67bab5c..eb95c94 100644 --- a/drivers/gpu/ipu-v3/ipu-common.c +++ b/drivers/gpu/ipu-v3/ipu-common.c @@ -1099,8 +1099,7 @@ static int ipu_irq_init(struct ipu_soc *ipu) } ret = irq_alloc_domain_generic_chips(ipu->domain, 32, 1, "IPU", - handle_level_irq, 0, - IRQF_VALID, 0); + handle_level_irq, 0, 0, 0); if (ret < 0) { dev_err(ipu->dev, "failed to alloc generic irq chips\n"); irq_domain_remove(ipu->domain); -- 2.1.0

1 0

[PATCH] drm/radeon: fix freeze for laptop with Turks/Thames GPU.
by j.glisse＠gmail.com 09 Jun '15

09 Jun '15

From: Jérôme Glisse <jglisse(a)redhat.com> Laptop with Turks/Thames GPU will freeze if dpm is enabled. It seems the SMC engine is relying on some state inside the CP engine. CP needs to chew at least one packet for it to get in good state for dynamic power management. This patch simply disabled and re-enable DPM after the ring test which is enough to avoid the freeze. Signed-off-by: Jérôme Glisse <jglisse(a)redhat.com> Cc: stable(a)vger.kernel.org --- drivers/gpu/drm/radeon/radeon_device.c | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index b7ca4c5..a7fdfa4 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -1463,6 +1463,21 @@ int radeon_device_init(struct radeon_device *rdev, if (r) DRM_ERROR("ib ring test failed (%d).\n", r); + /* + * Turks/Thames GPU will freeze whole laptop if DPM is not restarted + * after the CP ring have chew one packet at least. Hence here we stop + * and restart DPM after the radeon_ib_ring_tests(). + */ + if (rdev->pm.dpm_enabled && + (rdev->pm.pm_method == PM_METHOD_DPM) && + (rdev->family == CHIP_TURKS) && + (rdev->flags & RADEON_IS_MOBILITY)) { + mutex_lock(&rdev->pm.mutex); + radeon_dpm_disable(rdev); + radeon_dpm_enable(rdev); + mutex_unlock(&rdev->pm.mutex); + } + if ((radeon_testing & 1)) { if (rdev->accel_working) radeon_test_moves(rdev); -- 1.7.1

2 1

Re: [PATCH v2 08/10] drm: Add CSC correction structure
by Damien Lespiau 09 Jun '15

09 Jun '15

On Thu, Jun 04, 2015 at 07:12:39PM +0530, Kausal Malladi wrote: > From: Kausal Malladi <Kausal.Malladi(a)intel.com> > > This patch adds a new structure in DRM layer for CSC color correction. > This structure will be used by all user space agents to configure > appropriate CSC format and CSC level. And another little one: > /* Color Management structure for CSC */ > struct drm_intel_csc { > __u32 csc_level; > (The csc_level indicates whether the CSC to be applied at > pipe/plane level) > __u32 csc_format; > (The csc_format indicates the supported correction value \ > formats by underlying hardware) > __u32 reserved; > __s32 csc_matrix[9]; > (Raw CSC correction matrix) You haven't defined if that data represents a row-major or column-major matrix. > }; -- Damien

1 0

Re: [PATCH v2 01/10] drm/i915: Initialize Color Manager
by Damien Lespiau 09 Jun '15

09 Jun '15

On Thu, Jun 04, 2015 at 07:12:32PM +0530, Kausal Malladi wrote: > From: Kausal Malladi <Kausal.Malladi(a)intel.com> > > Color Manager is an extension in i915 driver to handle color correction > and enhancements across various Intel platforms. > > This patch initializes color manager framework by : > 1. Adding two new files, intel_color_manager(.c/.h) > 2. Introducing new pointers in DRM mode_config structure to > carry CSC and Gamma color correction properties. > 3. Creating these DRM properties in Color Manager initialization > sequence. > > v2: Addressing Sonika's review comment. > 1. Made intel_color_manager_init void > 2. Moved init after NUM_PIPES check > > Signed-off-by: Shashank Sharma <shashank.sharma(a)intel.com> > Signed-off-by: Kausal Malladi <Kausal.Malladi(a)intel.com> > --- > drivers/gpu/drm/i915/Makefile | 3 ++ > drivers/gpu/drm/i915/intel_color_manager.c | 48 ++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_color_manager.h | 32 ++++++++++++++++++++ > drivers/gpu/drm/i915/intel_display.c | 3 ++ > include/drm/drm_crtc.h | 4 +++ > 5 files changed, 90 insertions(+) > create mode 100644 drivers/gpu/drm/i915/intel_color_manager.c > create mode 100644 drivers/gpu/drm/i915/intel_color_manager.h > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index b7ddf48..c62d048 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -89,6 +89,9 @@ i915-y += i915_vgpu.o > # legacy horrors > i915-y += i915_dma.o > > +# Color Management > +i915-y += intel_color_manager.o > + FWIW, I'd put this in the "# modesetting core code" section. The objects are somewhat sorted by big categories, having a category saying that "intel_color_manager.c" is for "Color Management" is, well, not that useful :) > obj-$(CONFIG_DRM_I915) += i915.o > > CFLAGS_i915_trace_points.o := -I$(src) > diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c > new file mode 100644 > index 0000000..f7e2380 > --- /dev/null > +++ b/drivers/gpu/drm/i915/intel_color_manager.c > @@ -0,0 +1,48 @@ > +/* > + * Copyright © 2015 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS > + * IN THE SOFTWARE. > + * > + * Authors: > + * Shashank Sharma <shashank.sharma(a)intel.com> > + * Kausal Malladi <Kausal.Malladi(a)intel.com> > + */ > + > +#include "intel_color_manager.h" > + > +void intel_color_manager_init(struct drm_device *dev) This function create "generic" properties (more on that in the cover letter) in drm_mode_config. It looks like it should be part of the DRM core, not i915? > +{ > + struct drm_mode_config *config = &dev->mode_config; > + > + /* Create Gamma and CSC properties */ > + config->gamma_property = drm_property_create(dev, > + DRM_MODE_PROP_BLOB, "gamma_property", 0); You are creating properties with the names "gamma_property", "csc_property". While we do have (and that's quite unfortunate) a variety of naming schemes for properties none of them include the _property suffix. Just "gamma" and "csc" would be more appropriate. See the current list of properties: https://www.kernel.org/doc/htmldocs/drm/drm-kms-properties.html#idp13803344 Speaking of which, you also need to add the property to the documentation (Documentation/DocBook/drm.xml): > + if (!config->gamma_property) > + DRM_ERROR("Gamma property creation failed\n"); > + else > + DRM_DEBUG_DRIVER("Created Gamma property\n"); That's really too verbose on the logs, I don't think you need any of them. It's easy enough to check if the property was created, it's part of the API. > + > + config->csc_property = drm_property_create(dev, > + DRM_MODE_PROP_BLOB, "csc_property", 0); "CSC" is quite specific to how Intel calls the unit doing the 3x3 matrix transform on colors, maybe we could be more generic there? "color-matrix"? This also depends if we want to try to create somewhat generic properties or not (if not, maybe we should prefix the properties with "i915-") > + if (!config->csc_property) > + DRM_ERROR("CSC property creation failed\n"); > + else > + DRM_DEBUG_DRIVER("Created CSC property\n"); > +} > diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h > new file mode 100644 > index 0000000..154bf16 > --- /dev/null > +++ b/drivers/gpu/drm/i915/intel_color_manager.h > @@ -0,0 +1,32 @@ > +/* > + * Copyright © 2015 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS > + * IN THE SOFTWARE. > + * > + * Authors: > + * Shashank Sharma <shashank.sharma(a)intel.com> > + * Kausal Malladi <Kausal.Malladi(a)intel.com> > + */ > + > +#include <drm/drmP.h> > +#include <drm/drm_crtc_helper.h> > + > +/* Generic Function prototypes */ > +void intel_color_manager_init(struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 067b1de..2322dee 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -44,6 +44,7 @@ > #include <drm/drm_plane_helper.h> > #include <drm/drm_rect.h> > #include <linux/dma_remapping.h> > +#include "intel_color_manager.h" > > /* Primary plane formats for gen <= 3 */ > static const uint32_t i8xx_primary_formats[] = { > @@ -14673,6 +14674,8 @@ void intel_modeset_init(struct drm_device *dev) > if (INTEL_INFO(dev)->num_pipes == 0) > return; > > + intel_color_manager_init(dev); > + > intel_init_display(dev); > intel_init_audio(dev); > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index 3b4d8a4..2a75d7d 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -1176,6 +1176,10 @@ struct drm_mode_config { > struct drm_property *suggested_x_property; > struct drm_property *suggested_y_property; > > + /* Color Management Properties */ > + struct drm_property *gamma_property; > + struct drm_property *csc_property; > + > /* dumb ioctl parameters */ > uint32_t preferred_depth, prefer_shadow; > > -- > 2.4.2 >

1 1

Re: [PATCH v2 07/10] drm/i915: Add pipe level Gamma correction for CHV/BSW
by Damien Lespiau 09 Jun '15

09 Jun '15

On Thu, Jun 04, 2015 at 07:12:38PM +0530, Kausal Malladi wrote: > From: Kausal Malladi <Kausal.Malladi(a)intel.com> > > This patch does the following: > 1. Adds the core function to program Gamma correction values for CHV/BSW > platform A couple of things I forgot (they are probably others): - This patch doesn't actually add anything for CHV despite the claim - The properties are exposed everywhere, but return -EINVAL on !IS_CHERRYVIEW. We only expose the properties on a platform when we do have code that can make this property work for that platform. This way userspace can easily know if the feature is supported or not by querying if the property exists. -- Damien

1 0

Re: [PATCH v2 10/10] drm/i915: Add CSC support for CHV/BSW
by Damien Lespiau 09 Jun '15

09 Jun '15

On Thu, Jun 04, 2015 at 07:12:41PM +0530, Kausal Malladi wrote: > From: Kausal Malladi <Kausal.Malladi(a)intel.com> > > This patch does the following: > 1. Adds the core function to program CSC correction values for > CHV/BSW platform > 2. Adds CSC correction macros/defines > 3. Adds a pointer to hold blob for CSC property in drm_crtc > > Signed-off-by: Shashank Sharma <shashank.sharma(a)intel.com> > Signed-off-by: Kausal Malladi <Kausal.Malladi(a)intel.com> > --- > drivers/gpu/drm/i915/intel_atomic.c | 3 +- > drivers/gpu/drm/i915/intel_color_manager.c | 115 +++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_color_manager.h | 26 +++++++ > 3 files changed, 143 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c > index 4726847..dcf4694 100644 > --- a/drivers/gpu/drm/i915/intel_atomic.c > +++ b/drivers/gpu/drm/i915/intel_atomic.c > @@ -433,7 +433,8 @@ intel_crtc_atomic_set_property(struct drm_crtc *crtc, > > if (property == config->gamma_property) > return intel_color_manager_set_gamma(dev, &crtc->base, val); > + if (property == config->csc_property) > + return intel_color_manager_set_csc(dev, &crtc->base, val); > > - DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name); > return -EINVAL; > } > diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c > index 421c267..d904050 100644 > --- a/drivers/gpu/drm/i915/intel_color_manager.c > +++ b/drivers/gpu/drm/i915/intel_color_manager.c > @@ -27,6 +27,108 @@ > > #include "intel_color_manager.h" > > +int chv_set_csc(struct drm_device *dev, uint64_t blob_id, > + struct drm_crtc *crtc) > +{ > + struct drm_csc *csc_data; > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct drm_property_blob *blob; > + struct drm_mode_config *config = &dev->mode_config; > + u32 reg; > + enum pipe pipe; > + s16 csc_value; > + s32 word, temp; > + int ret, count = 0; > + > + blob = drm_property_lookup_blob(dev, blob_id); > + if (!blob) { > + DRM_ERROR("Invalid Blob ID\n"); > + return -EINVAL; > + } > + > + csc_data = (struct drm_csc *)blob->data; > + pipe = to_intel_crtc(crtc)->pipe; > + > + if (csc_data->csc_format == I915_CSC_COEFF_FORMAT_UNKNOWN) { > + > + /* Disable CSC functionality */ > + reg = _PIPE_CGM_CONTROL(pipe); > + I915_WRITE(reg, I915_READ(reg) & (~CGM_CSC_EN)); > + > + DRM_DEBUG_DRIVER("Disabled CSC Functionality on Pipe %c\n", > + pipe_name(pipe)); > + ret = 0; Same remarks here I did for the gamma property. i915 specific defines for an interface that looks like it want to be generic, UNKNOWN Vs DISABLED, DRM_ERROR, ... And I haven't actually looked at the CHV details in this pass. > + } else if (csc_data->csc_format == I915_CSC_COEFF_FORMAT_S2_29) { > + > + /* Disable CSC functionality in case it was set earlier */ > + reg = _PIPE_CGM_CONTROL(pipe); > + I915_WRITE(reg, I915_READ(reg) & (~CGM_CSC_EN)); > + > + DRM_DEBUG_DRIVER("Disabled CSC Functionality on Pipe %c\n", > + pipe_name(pipe)); > + > + reg = _PIPE_CSC_BASE(pipe); > + while (count < CSC_MAX_VALS) { > + > + /* Rounding off, to decrease loss of precision */ > + if (csc_data->csc_matrix[count] < 0) { > + temp = csc_data->csc_matrix[count]; > + temp -= CHV_CSC_ROUNDOFF; > + if (temp < CHV_CSC_MIN) > + temp = CHV_CSC_MIN; > + } else { > + temp = csc_data->csc_matrix[count]; > + temp += CHV_CSC_ROUNDOFF; > + if (temp > CHV_CSC_MAX) > + temp = CHV_CSC_MIN; > + } > + csc_value = temp >> S2_29_CSC_COEFF_SHIFT; > + word = csc_value; > + > + /* > + * Last value to be written in 1 register. > + * Otherwise, each pair of CSC values go > + * into 1 register > + */ > + if (count != (CSC_MAX_VALS - 1)) { > + count++; > + csc_value = temp >> S2_29_CSC_COEFF_SHIFT; > + temp = csc_value; > + temp <<= CHV_CSC_SHIFT; > + word |= temp; > + } > + I915_WRITE(reg, word); > + reg += 4; > + count++; > + } > + > + DRM_DEBUG_DRIVER("All CSC values written to registers\n"); > + > + /* Enable CSC functionality */ > + reg = _PIPE_CGM_CONTROL(pipe); > + I915_WRITE(reg, I915_READ(reg) | CGM_CSC_EN); > + DRM_DEBUG_DRIVER("CSC enabled on Pipe %c\n", > + pipe_name(pipe)); > + ret = 0; > + } else { > + DRM_ERROR("Invalid CSC COEFF Format\n"); > + return -EINVAL; > + } > + > + ret = drm_mode_crtc_update_color_property(&blob, > + sizeof(struct drm_csc), (void *) csc_data, > + &crtc->base, config->csc_property); > + if (ret) { > + DRM_ERROR("Error updating CSC blob\n"); > + crtc->csc_blob_id = INVALID_BLOB_ID; > + return -EFAULT; > + } > + > + /* Save blob ID for future reference */ > + crtc->csc_blob_id = blob->base.id; > + return ret; > +} > + > int chv_set_gamma(struct drm_device *dev, uint32_t blob_id, > struct drm_crtc *crtc) > { > @@ -175,6 +277,19 @@ int chv_set_gamma(struct drm_device *dev, uint32_t blob_id, > return ret; > } > > +int intel_color_manager_set_csc(struct drm_device *dev, > + struct drm_mode_object *obj, uint64_t blob_id) > +{ > + struct drm_crtc *crtc = obj_to_crtc(obj); > + > + DRM_DEBUG_DRIVER("\n"); > + > + if (IS_CHERRYVIEW(dev)) > + return chv_set_csc(dev, blob_id, crtc); > + > + return -EINVAL; > +} > + > int intel_color_manager_set_gamma(struct drm_device *dev, > struct drm_mode_object *obj, uint32_t blob_id) > { > diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h > index 0acf8e9..8a3e76a 100644 > --- a/drivers/gpu/drm/i915/intel_color_manager.h > +++ b/drivers/gpu/drm/i915/intel_color_manager.h > @@ -41,6 +41,14 @@ > #define I915_GAMMA_PRECISION_14BIT (1 << 3) > #define I915_GAMMA_PRECISION_16BIT (1 << 4) > > +/* Color Management macros for CSC */ > +#define I915_PIPE_CSC (1 << 0) > +#define I915_PLANE_CSC (1 << 1) > +#define I915_CSC_COEFF_FORMAT_UNKNOWN 0 > +#define I915_CSC_COEFF_FORMAT_CURRENT 0xFFFFFFFF > +#define I915_CSC_COEFF_FORMAT_S1_30 (1 << 0) > +#define I915_CSC_COEFF_FORMAT_S2_29 (1 << 1) > + > #define CHV_MAX_PIPES 3 > #define CHV_DISPLAY_BASE 0x180000 > #define INVALID_BLOB_ID 9999 > @@ -52,6 +60,8 @@ struct rgb_pixel { > }; > > /* CHV CGM Block */ > +/* Bit 1 to be enabled */ > +#define CGM_CSC_EN 2 > /* Bit 2 to be enabled in CGM block for CHV */ > #define CGM_GAMMA_EN 4 > > @@ -74,15 +84,27 @@ struct rgb_pixel { > #define CHV_8BIT_GAMMA_SHIFT_GREEN_REG 8 > #define CHV_8BIT_GAMMA_SHIFT_RED_REG 16 > > +/* CSC */ > +#define CSC_MAX_VALS 9 > +#define CHV_CSC_SHIFT 16 > +#define CHV_CSC_ROUNDOFF (1 << 15) > +#define CHV_CSC_MAX 0x7FFF > +#define CHV_CSC_MIN 0x8000 > +#define S2_29_CSC_COEFF_SHIFT 16 > + > /* CGM Registers */ > #define CGM_OFFSET 0x2000 > #define GAMMA_OFFSET 0x2000 > +#define CGM_CSC_OFFSET 0x2000 > #define PIPEA_CGM_CONTROL (CHV_DISPLAY_BASE + 0x67A00) > #define PIPEA_CGM_GAMMA_MIN (CHV_DISPLAY_BASE + 0x67000) > +#define PIPEA_CGM_CSC_MIN (CHV_DISPLAY_BASE + 0x67900) > #define _PIPE_CGM_CONTROL(pipe) \ > (PIPEA_CGM_CONTROL + (pipe * CGM_OFFSET)) > #define _PIPE_GAMMA_BASE(pipe) \ > (PIPEA_CGM_GAMMA_MIN + (pipe * GAMMA_OFFSET)) > +#define _PIPE_CSC_BASE(pipe) \ > + (PIPEA_CGM_CSC_MIN + (pipe * CGM_CSC_OFFSET)) > > /* Generic Function prototypes */ > void intel_color_manager_init(struct drm_device *dev); > @@ -90,7 +112,11 @@ void intel_color_manager_attach(struct drm_device *dev, > struct drm_mode_object *mode_obj); > extern int intel_color_manager_set_gamma(struct drm_device *dev, > struct drm_mode_object *obj, uint32_t blob_id); > +extern int intel_color_manager_set_csc(struct drm_device *dev, > + struct drm_mode_object *obj, uint64_t blob_id); > > /* Platform specific function prototypes */ > extern int chv_set_gamma(struct drm_device *dev, > uint32_t blob_id, struct drm_crtc *crtc); > +extern int chv_set_csc(struct drm_device *dev, > + uint64_t blob_id, struct drm_crtc *crtc); > -- > 2.4.2 >

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

dri-devel June 2015