From mboxrd@z Thu Jan 1 00:00:00 1970 From: lars@metafoo.de (Lars-Peter Clausen) Date: Wed, 23 May 2012 10:37:58 +0200 Subject: [PATCH 1/2] DRM: add Freescale i.MX LCDC driver In-Reply-To: <20120523074710.GP30400@pengutronix.de> References: <1337344032-25431-1-git-send-email-s.hauer@pengutronix.de> <1337344032-25431-2-git-send-email-s.hauer@pengutronix.de> <20120523074710.GP30400@pengutronix.de> Message-ID: <4FBCA1E6.9050604@metafoo.de> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org [...] >>> + >>> +static int imx_drm_gem_mmap_buffer(struct file *filp, >>> + struct vm_area_struct *vma) >>> +{ >>> + struct drm_gem_object *obj = filp->private_data; >>> + struct imx_drm_gem_obj *imx_drm_gem_obj = to_imx_drm_gem_obj(obj); >>> + struct imx_drm_buf_entry *entry; >>> + unsigned long pfn, vm_size; >>> + >>> + vma->vm_flags |= VM_IO | VM_RESERVED; >>> + >>> + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); >> >> pgprot_writecombine()? > > copied from the exynos driver. The exynos driver recently gained support > for different cache attribute flags and I could do the same. I would > prefer not to even have to think about it by using some generic code > here instead of duplicating other peoples bugs. > > Do you think it's possible to share this code as suggested by Lars? > Every SoC not having a IOMMU could share the same code here, it's just > not clear to me how we can put this in a form that is acceptable > upstream. I may have missed this in the previous discussion. But why can't we put the gem handling code in the toplevel drm folder, give it a config symbol and let drivers which want to use the code select the config symbol? I think the main concern was about introducing a new intermediate layer, but the "simple" gem support would really just a set of helper functions. Thanks, - Lars From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lars-Peter Clausen Subject: Re: [PATCH 1/2] DRM: add Freescale i.MX LCDC driver Date: Wed, 23 May 2012 10:37:58 +0200 Message-ID: <4FBCA1E6.9050604@metafoo.de> References: <1337344032-25431-1-git-send-email-s.hauer@pengutronix.de> <1337344032-25431-2-git-send-email-s.hauer@pengutronix.de> <20120523074710.GP30400@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from smtp-out-085.synserver.de (smtp-out-085.synserver.de [212.40.185.85]) by gabe.freedesktop.org (Postfix) with ESMTP id 70328A094A for ; Wed, 23 May 2012 01:34:52 -0700 (PDT) In-Reply-To: <20120523074710.GP30400@pengutronix.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Sascha Hauer Cc: dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, Rob Clark List-Id: dri-devel@lists.freedesktop.org [...] >>> + >>> +static int imx_drm_gem_mmap_buffer(struct file *filp, >>> + struct vm_area_struct *vma) >>> +{ >>> + struct drm_gem_object *obj = filp->private_data; >>> + struct imx_drm_gem_obj *imx_drm_gem_obj = to_imx_drm_gem_obj(obj); >>> + struct imx_drm_buf_entry *entry; >>> + unsigned long pfn, vm_size; >>> + >>> + vma->vm_flags |= VM_IO | VM_RESERVED; >>> + >>> + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); >> >> pgprot_writecombine()? > > copied from the exynos driver. The exynos driver recently gained support > for different cache attribute flags and I could do the same. I would > prefer not to even have to think about it by using some generic code > here instead of duplicating other peoples bugs. > > Do you think it's possible to share this code as suggested by Lars? > Every SoC not having a IOMMU could share the same code here, it's just > not clear to me how we can put this in a form that is acceptable > upstream. I may have missed this in the previous discussion. But why can't we put the gem handling code in the toplevel drm folder, give it a config symbol and let drivers which want to use the code select the config symbol? I think the main concern was about introducing a new intermediate layer, but the "simple" gem support would really just a set of helper functions. Thanks, - Lars