Re: [Intel-gfx] [PATCH 6/6] drm/i915: Migrate stolen objects before hibernation

2015-10-12 Thread Ankitprasad Sharma
On Thu, 2015-10-08 at 12:02 +0100, Chris Wilson wrote:
> On Thu, Oct 08, 2015 at 11:54:29AM +0530, ankitprasad.r.sha...@intel.com 
> wrote:
> > +   /* stolen objects are already pinned to prevent shrinkage */
> > +   memset(&node, 0, sizeof(node));
> > +   ret = drm_mm_insert_node_in_range_generic(&i915->gtt.base.mm,
> > + &node,
> > + 4096, 0, I915_CACHE_NONE,
> > + 0, i915->gtt.mappable_end,
> > + DRM_MM_SEARCH_DEFAULT,
> > + DRM_MM_CREATE_DEFAULT);
> > +   if (ret)
> > +   return ret;
> > +
> > +   i915->gtt.base.insert_entries(&i915->gtt.base, obj->pages,
> > + node.start, I915_CACHE_NONE, 0);
> 
> This was written using an insert_page() function you don't have. Either
> grab that as well, or you need to pin the entire object into the GGTT,
> i.e. i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE); note that to do so
> will also need to be very careful to handle the pinning of obj->pages
> and the introduction of a new GGTT vma.
> 
> > +
> > +   for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
> > +   struct page *page;
> > +   void *__iomem src;
> > +   void *dst;
> > +
> > +   page = shmem_read_mapping_page(mapping, i);
> > +   if (IS_ERR(page)) {
> > +   ret = PTR_ERR(page);
> > +   goto err_node;
> > +   }
> > +
> > +   src = io_mapping_map_atomic_wc(i915->gtt.mappable, node.start + 
> > PAGE_SIZE * i);
> > +   dst = kmap_atomic(page);
> > +   memcpy_fromio(dst, src, PAGE_SIZE);
> > +   kunmap_atomic(dst);
> > +   io_mapping_unmap_atomic(src);
> > +
> > +   page_cache_release(page);
> > +   }
> > +
> > +   wmb();
> > +   i915->gtt.base.clear_range(&i915->gtt.base,
> > +  node.start, node.size,
> > +  true);
> > +   drm_mm_remove_node(&node);
> > +
> > +swap_pages:
> > +   stolen_pages = obj->pages;
> > +   obj->pages = NULL;
> > +
> > +   obj->base.filp = file;
> > +   obj->base.read_domains = I915_GEM_DOMAIN_CPU;
> > +   obj->base.write_domain = I915_GEM_DOMAIN_CPU;
> > +
> > +   /* Recreate any pinned binding with pointers to the new storage */
> > +   if (!list_empty(&obj->vma_list)) {
> > +   ret = i915_gem_object_get_pages_gtt(obj);
> > +   if (ret) {
> > +   obj->pages = stolen_pages;
> > +   goto err_file;
> > +   }
> > +
> > +   ret = i915_gem_gtt_prepare_object(obj);
> > +   if (ret) {
> > +   i915_gem_object_put_pages_gtt(obj);
> > +   obj->pages = stolen_pages;
> > +   goto err_file;
> > +   }
> > +
> > +   ret = i915_gem_object_set_to_gtt_domain(obj, true);
> > +   if (ret) {
> > +   i915_gem_gtt_finish_object(obj);
> > +   i915_gem_object_put_pages_gtt(obj);
> > +   obj->pages = stolen_pages;
> > +   goto err_file;
> > +   }
> > +
> > +   obj->get_page.sg = obj->pages->sgl;
> > +   obj->get_page.last = 0;
> > +
> > +   list_for_each_entry(vma, &obj->vma_list, vma_link) {
> > +   if (!drm_mm_node_allocated(&vma->node))
> > +   continue;
> > +
> > +   WARN_ON(i915_vma_bind(vma,
> > + obj->cache_level,
> > + PIN_UPDATE));
> > +   }
> > +   } else
> > +   list_del(&obj->global_list);
> > +
> > +   /* drop the stolen pin and backing */
> > +   shmemfs_pages = obj->pages;
> > +   obj->pages = stolen_pages;
> > +
> > +   i915_gem_object_unpin_pages(obj);
> > +   obj->ops->put_pages(obj);
> > +   if (obj->ops->release)
> > +   obj->ops->release(obj);
> > +
> > +   obj->ops = &i915_gem_object_ops;
> > +   obj->pages = shmemfs_pages;
> > +
> > +   return 0;
> > +
> > +err_node:
> > +   wmb();
> > +   i915->gtt.base.clear_range(&i915->gtt.base,
> > +  node.start, node.size,
> > +  true);
> > +   drm_mm_remove_node(&node);
> > +err_file:
> > +   fput(file);
> > +   obj->base.filp = NULL;
> > +   return ret;
> > +}
> > +
> > +int
> > +i915_gem_freeze(struct drm_device *dev)
> > +{
> > +   /* Called before i915_gem_suspend() when hibernating */
> > +   struct drm_i915_private *i915 = to_i915(dev);
> > +   struct drm_i915_gem_object *obj, *tmp;
> > +   struct list_head *phase[] = {
> > +   &i915->mm.unbound_list, &i915->mm.bound_list, NULL
> > +   }, **p;
> > +
> > +   /* Across hibernation, the stolen area is not preserved.
> > +* Anything inside stolen must copied back to normal
> > +* memory if we wish t

Re: [Intel-gfx] [PATCH] drm/i915: Disable DC6 for now.

2015-10-12 Thread Hindman, Gavin
The current DC6 functionality is stable enough for development and is badly 
needed for working down other platform power issues.  I'm fine if you want to 
disable it by default, but please only do so in conjunction with i915 kernel 
override flags to reenable it at runtime.

Gavin Hindman
Senior Program Manager
SSG/OTC – Open Source Technology Center

-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of 
Rodrigo Vivi
Sent: Monday, October 12, 2015 8:43 AM
To: intel-gfx@lists.freedesktop.org
Cc: Vivi, Rodrigo 
Subject: [Intel-gfx] [PATCH] drm/i915: Disable DC6 for now.

There is an intermitent RAM corruption happening with DMC micro-controler when 
in DC6 transitioning to PC9/10. So the recoomendation is to use DC5 as the 
deeper DC state for now until this issue is being investigated at firmware 
level.

This macros must be re-worked in order to allow us to use module parameters to 
allow max DC states. However let's do this simple approach first before 
products out there start facing this corruption. Also this is the easiest one 
to be backported by products.

Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index ec010ee..7332cc0 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -49,8 +49,8 @@
  * present for a given platform.
  */
 
-#define GEN9_ENABLE_DC5(dev) 0
-#define SKL_ENABLE_DC6(dev) IS_SKYLAKE(dev)
+#define GEN9_ENABLE_DC5(dev) IS_SKYLAKE(dev) #define 
+SKL_ENABLE_DC6(dev) 0
 
 #define for_each_power_well(i, power_well, domain_mask, power_domains) \
for (i = 0; \
--
2.4.3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 09/20] i915: switch from acpi_os_ioremap to memremap

2015-10-12 Thread Williams, Dan J
On Mon, 2015-10-12 at 09:01 +0200, Daniel Vetter wrote:
> On Fri, Oct 09, 2015 at 06:16:25PM -0400, Dan Williams wrote:
> > i915 expects the OpRegion to be cached (i.e. not __iomem), so explicitly
> > map it with memremap rather than the implied cache setting of
> > acpi_os_ioremap().
> > 
> > Cc: Daniel Vetter 
> > Cc: Jani Nikula 
> > Cc: intel-gfx@lists.freedesktop.org
> > Cc: David Airlie 
> > Cc: dri-de...@lists.freedesktop.org
> > Signed-off-by: Dan Williams 
> 
> Assuming you've run sparse over this to make sure you've caught them all,
> and with the nit below addressed this is
> 
> Reviewed-by: Daniel Vetter 

Indeed, re-running sparse again found a few conversions of ioread* I
missed as well as moving the force casting out of validate_vbt() to
find_vbt().

> Feel free to pull v2 into whatever tree you think it's suitable for (but
> you can also resend and I'll pick it up).

Please pick up v2 below.

> 
> > diff --git a/drivers/gpu/drm/i915/intel_panel.c 
> > b/drivers/gpu/drm/i915/intel_panel.c
> > index e2ab3f6ed022..c8444d5f549f 100644
> > --- a/drivers/gpu/drm/i915/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > @@ -387,7 +387,7 @@ intel_panel_detect(struct drm_device *dev)
> >  
> > /* Assume that the BIOS does not lie through the OpRegion... */
> > if (!i915.panel_ignore_lid && dev_priv->opregion.lid_state) {
> > -   return ioread32(dev_priv->opregion.lid_state) & 0x1 ?
> > +   return *(dev_priv->opregion.lid_state) & 0x1 ?
> 
> () are redundant now here.

Yup, fixed.

8<
Subject: i915: switch from acpi_os_ioremap to memremap

From: Dan Williams 

i915 expects the OpRegion to be cached (i.e. not __iomem), so explicitly
map it with memremap rather than the implied cache setting of
acpi_os_ioremap().

Cc: Daniel Vetter 
Cc: Jani Nikula 
Cc: intel-gfx@lists.freedesktop.org
Cc: David Airlie 
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Dan Williams 
---
 drivers/gpu/drm/i915/i915_debugfs.c   |2 -
 drivers/gpu/drm/i915/i915_drv.h   |   12 ++---
 drivers/gpu/drm/i915/intel_bios.c |   25 +-
 drivers/gpu/drm/i915/intel_opregion.c |   83 -
 drivers/gpu/drm/i915/intel_panel.c|2 -
 5 files changed, 62 insertions(+), 62 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index e3ec9049081f..15989cc16e92 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1849,7 +1849,7 @@ static int i915_opregion(struct seq_file *m, void *unused)
goto out;
 
if (opregion->header) {
-   memcpy_fromio(data, opregion->header, OPREGION_SIZE);
+   memcpy(data, opregion->header, OPREGION_SIZE);
seq_write(m, data, OPREGION_SIZE);
}
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e1db8de52851..d8684634a31d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -444,14 +444,14 @@ struct opregion_swsci;
 struct opregion_asle;
 
 struct intel_opregion {
-   struct opregion_header __iomem *header;
-   struct opregion_acpi __iomem *acpi;
-   struct opregion_swsci __iomem *swsci;
+   struct opregion_header *header;
+   struct opregion_acpi *acpi;
+   struct opregion_swsci *swsci;
u32 swsci_gbda_sub_functions;
u32 swsci_sbcb_sub_functions;
-   struct opregion_asle __iomem *asle;
-   void __iomem *vbt;
-   u32 __iomem *lid_state;
+   struct opregion_asle *asle;
+   void *vbt;
+   u32 *lid_state;
struct work_struct asle_work;
 };
 #define OPREGION_SIZE(8*1024)
diff --git a/drivers/gpu/drm/i915/intel_bios.c 
b/drivers/gpu/drm/i915/intel_bios.c
index c19e669ffe50..f6762a5faee8 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1231,20 +1231,13 @@ static const struct dmi_system_id 
intel_no_opregion_vbt[] = {
{ }
 };
 
-static const struct bdb_header *validate_vbt(const void __iomem *_base,
+static const struct bdb_header *validate_vbt(const void *base,
 size_t size,
-const void __iomem *_vbt,
+const void *_vbt,
 const char *source)
 {
-   /*
-* This is the one place where we explicitly discard the address space
-* (__iomem) of the BIOS/VBT. (And this will cause a sparse complaint.)
-* From now on everything is based on 'base', and treated as regular
-* memory.
-*/
-   const void *base = (const void *) _base;
-   size_t offset = _vbt - _base;
-   const struct vbt_header *vbt = base + offset;
+   size_t offset = _vbt - base;
+   const struct vbt_header *vbt = _vbt;
const struct bdb_header *bdb;
 
if (offset + sizeof(struct 

Re: [Intel-gfx] [PATCH 6/7] drm/i915: use compute_page_offset() on SKL too

2015-10-12 Thread Ville Syrjälä
On Mon, Oct 12, 2015 at 06:19:52PM +, Hindman, Gavin wrote:
> What's the next step on this patch?

I think my branch is nearing usable state. But I haven't actually tested
on SKL yet, which means not tested 90/270 rotation either. That's pretty
much the next thing on my list.

> 
> Gavin Hindman
> Senior Program Manager
> SSG/OTC – Open Source Technology Center
> 
> -Original Message-
> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of 
> Ville Syrjälä
> Sent: Thursday, September 24, 2015 10:10 AM
> To: Zanoni, Paulo R 
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 6/7] drm/i915: use compute_page_offset() on 
> SKL too
> 
> On Wed, Sep 23, 2015 at 12:52:26PM -0300, Paulo Zanoni wrote:
> > The trick is not strictly necessary on SKL because the offset 
> > registers allow more bits. But for FBC, doing this changes how the 
> > hardware tracking works - it starts at the surface address we provide
> > - so there's a higher chance that the CRTC will be pointing to an area 
> > of the frontbuffer that is actually being covered by the hardware 
> > tracking mechanism. This fixes fbc-farfromfence on SKL.
> > 
> > Testcase: igt/kms_frontbuffer_tracking/fbc-farfromfence
> > Signed-off-by: Paulo Zanoni 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 7 +++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 24b8a72..d40ae71 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3031,6 +3031,7 @@ static void skylake_update_primary_plane(struct 
> > drm_crtc *crtc,
> > int src_x = 0, src_y = 0, src_w = 0, src_h = 0;
> > int dst_x = 0, dst_y = 0, dst_w = 0, dst_h = 0;
> > int scaler_id = -1;
> > +   int pixel_size;
> >  
> > plane_state = to_intel_plane_state(plane->state);
> >  
> > @@ -3079,6 +3080,12 @@ static void skylake_update_primary_plane(struct 
> > drm_crtc *crtc,
> > src_h = intel_crtc->config->pipe_src_h;
> > }
> >  
> > +   pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> > +   intel_crtc->dspaddr_offset = intel_gen4_compute_page_offset(dev_priv,
> > +   &x, &y, obj->tiling_mode,
> > +   pixel_size, fb->pitches[0]);
> > +   surf_addr += intel_crtc->dspaddr_offset;
> 
> It's not that simple. I have a branch that tries to make the required changes 
> to make it work properly:
> git://github.com/vsyrjala/linux.git tile_size
> 
> > +
> > if (intel_rotation_90_or_270(rotation)) {
> > /* stride = Surface height in tiles */
> > tile_height = intel_tile_height(dev, fb->pixel_format,
> > --
> > 2.5.1
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Ville Syrjälä
> Intel OTC
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 6/7] drm/i915: use compute_page_offset() on SKL too

2015-10-12 Thread Hindman, Gavin
What's the next step on this patch?

Gavin Hindman
Senior Program Manager
SSG/OTC – Open Source Technology Center

-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of 
Ville Syrjälä
Sent: Thursday, September 24, 2015 10:10 AM
To: Zanoni, Paulo R 
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 6/7] drm/i915: use compute_page_offset() on SKL 
too

On Wed, Sep 23, 2015 at 12:52:26PM -0300, Paulo Zanoni wrote:
> The trick is not strictly necessary on SKL because the offset 
> registers allow more bits. But for FBC, doing this changes how the 
> hardware tracking works - it starts at the surface address we provide
> - so there's a higher chance that the CRTC will be pointing to an area 
> of the frontbuffer that is actually being covered by the hardware 
> tracking mechanism. This fixes fbc-farfromfence on SKL.
> 
> Testcase: igt/kms_frontbuffer_tracking/fbc-farfromfence
> Signed-off-by: Paulo Zanoni 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 24b8a72..d40ae71 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3031,6 +3031,7 @@ static void skylake_update_primary_plane(struct 
> drm_crtc *crtc,
>   int src_x = 0, src_y = 0, src_w = 0, src_h = 0;
>   int dst_x = 0, dst_y = 0, dst_w = 0, dst_h = 0;
>   int scaler_id = -1;
> + int pixel_size;
>  
>   plane_state = to_intel_plane_state(plane->state);
>  
> @@ -3079,6 +3080,12 @@ static void skylake_update_primary_plane(struct 
> drm_crtc *crtc,
>   src_h = intel_crtc->config->pipe_src_h;
>   }
>  
> + pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> + intel_crtc->dspaddr_offset = intel_gen4_compute_page_offset(dev_priv,
> + &x, &y, obj->tiling_mode,
> + pixel_size, fb->pitches[0]);
> + surf_addr += intel_crtc->dspaddr_offset;

It's not that simple. I have a branch that tries to make the required changes 
to make it work properly:
git://github.com/vsyrjala/linux.git tile_size

> +
>   if (intel_rotation_90_or_270(rotation)) {
>   /* stride = Surface height in tiles */
>   tile_height = intel_tile_height(dev, fb->pixel_format,
> --
> 2.5.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 20/22] drm/i915: BDW: Load degamma correction values

2015-10-12 Thread Rob Bradford
On Sat, 2015-10-10 at 00:59 +0530, Shashank Sharma wrote:
> I915 color manager registers pipe degamma correction as palette
> correction before CTM, DRM property.
> 
> This patch adds the no of coefficients(65) for degamma correction
> as "num_samples_before_ctm" parameter in device info structures,
> for BDW and higher platforms.

Did you copy and paste this from the CHV version? The only constant you
add for degamma here is 512?

Rob

> 
> Signed-off-by: Shashank Sharma 
> Signed-off-by: Kausal Malladi 
> ---
>  drivers/gpu/drm/i915/i915_drv.c| 7 +++
>  drivers/gpu/drm/i915/intel_color_manager.h | 3 +++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c
> b/drivers/gpu/drm/i915/i915_drv.c
> index 4fa046f..ebf4910 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -303,6 +303,7 @@ static const struct intel_device_info
> intel_broadwell_d_info = {
>   .need_gfx_hws = 1, .has_hotplug = 1,
>   .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
>   .num_samples_after_ctm = BDW_SPLITGAMMA_MAX_VALS,
> + .num_samples_before_ctm = BDW_DEGAMMA_MAX_VALS,
>   .has_llc = 1,
>   .has_ddi = 1,
>   .has_fpga_dbg = 1,
> @@ -316,6 +317,7 @@ static const struct intel_device_info
> intel_broadwell_m_info = {
>   .need_gfx_hws = 1, .has_hotplug = 1,
>   .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
>   .num_samples_after_ctm = BDW_SPLITGAMMA_MAX_VALS,
> + .num_samples_before_ctm = BDW_DEGAMMA_MAX_VALS,
>   .has_llc = 1,
>   .has_ddi = 1,
>   .has_fpga_dbg = 1,
> @@ -329,6 +331,7 @@ static const struct intel_device_info
> intel_broadwell_gt3d_info = {
>   .need_gfx_hws = 1, .has_hotplug = 1,
>   .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING
> | BSD2_RING,
>   .num_samples_after_ctm = BDW_SPLITGAMMA_MAX_VALS,
> + .num_samples_before_ctm = BDW_DEGAMMA_MAX_VALS,
>   .has_llc = 1,
>   .has_ddi = 1,
>   .has_fpga_dbg = 1,
> @@ -342,6 +345,7 @@ static const struct intel_device_info
> intel_broadwell_gt3m_info = {
>   .need_gfx_hws = 1, .has_hotplug = 1,
>   .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING
> | BSD2_RING,
>   .num_samples_after_ctm = BDW_SPLITGAMMA_MAX_VALS,
> + .num_samples_before_ctm = BDW_DEGAMMA_MAX_VALS,
>   .has_llc = 1,
>   .has_ddi = 1,
>   .has_fpga_dbg = 1,
> @@ -368,6 +372,7 @@ static const struct intel_device_info
> intel_skylake_info = {
>   .need_gfx_hws = 1, .has_hotplug = 1,
>   .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
>   .num_samples_after_ctm = BDW_SPLITGAMMA_MAX_VALS,
> + .num_samples_before_ctm = BDW_DEGAMMA_MAX_VALS,
>   .has_llc = 1,
>   .has_ddi = 1,
>   .has_fpga_dbg = 1,
> @@ -382,6 +387,7 @@ static const struct intel_device_info
> intel_skylake_gt3_info = {
>   .need_gfx_hws = 1, .has_hotplug = 1,
>   .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING
> | BSD2_RING,
>   .num_samples_after_ctm = BDW_SPLITGAMMA_MAX_VALS,
> + .num_samples_before_ctm = BDW_DEGAMMA_MAX_VALS,
>   .has_llc = 1,
>   .has_ddi = 1,
>   .has_fpga_dbg = 1,
> @@ -396,6 +402,7 @@ static const struct intel_device_info
> intel_broxton_info = {
>   .need_gfx_hws = 1, .has_hotplug = 1,
>   .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
>   .num_samples_after_ctm = BDW_SPLITGAMMA_MAX_VALS,
> + .num_samples_before_ctm = BDW_DEGAMMA_MAX_VALS,
>   .num_pipes = 3,
>   .has_ddi = 1,
>   .has_fpga_dbg = 1,
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h
> b/drivers/gpu/drm/i915/intel_color_manager.h
> index 6c7cb08..e0c486e 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.h
> +++ b/drivers/gpu/drm/i915/intel_color_manager.h
> @@ -98,3 +98,6 @@
>  #define BDW_MAX_GAMMA ((1 << 24) - 1)
>  #define BDW_INDEX_AUTO_INCREMENT   (1 << 15)
>  #define BDW_INDEX_SPLIT_MODE   (1 << 31)
> +
> +/* Degamma on BDW */
> +#define BDW_DEGAMMA_MAX_VALS   512
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 19/22] drm/i915: BDW: Pipe level Gamma correction

2015-10-12 Thread Rob Bradford
On Sat, 2015-10-10 at 00:59 +0530, Shashank Sharma wrote:
> BDW/SKL/BXT platforms support various Gamma correction modes
> which are:
> 1. Legacy 8-bit mode
> 2. 10-bit Split Gamma mode
> 3. 12-bit mode
> 
> This patch does the following:
> 1. Adds the core function to program Gamma correction values
>for BDW/SKL/BXT platforms
> 2. Adds Gamma correction macros/defines
> 
> Signed-off-by: Shashank Sharma 
> Signed-off-by: Kausal Malladi 
> ---
>  drivers/gpu/drm/i915/i915_reg.h|  25 ++-
>  drivers/gpu/drm/i915/intel_color_manager.c | 248
> +
>  drivers/gpu/drm/i915/intel_color_manager.h |   6 +
>  3 files changed, 277 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 5825ab2..ed50f75 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5647,11 +5647,15 @@ enum skl_disp_power_wells {
>  /* legacy palette */
>  #define _LGC_PALETTE_A   0x4a000
>  #define _LGC_PALETTE_B   0x4a800
> -#define LGC_PALETTE(pipe, i) (_PIPE(pipe, _LGC_PALETTE_A,
> _LGC_PALETTE_B) + (i) * 4)
> +#define _LGC_PALETTE_C   0x4b000
> +#define LGC_PALETTE(pipe, i) (_PIPE3(pipe, _LGC_PALETTE_A,
> _LGC_PALETTE_B, \
> +  _LGC_PALETTE_C) + (i) * 4)
>  
>  #define _GAMMA_MODE_A0x4a480
>  #define _GAMMA_MODE_B0x4ac80
> -#define GAMMA_MODE(pipe) _PIPE(pipe, _GAMMA_MODE_A, _GAMMA_MODE_B)
> +#define _GAMMA_MODE_C0x4b480
> +#define GAMMA_MODE(pipe) \
> + _PIPE3(pipe, _GAMMA_MODE_A, _GAMMA_MODE_B, _GAMMA_MODE_C)
>  #define GAMMA_MODE_MODE_MASK (3 << 0)
>  #define GAMMA_MODE_MODE_8BIT (0 << 0)
>  #define GAMMA_MODE_MODE_10BIT(1 << 0)
> @@ -8062,6 +8066,23 @@ enum skl_disp_power_wells {
>  #define _PIPE_CSC_BASE(pipe) \
>   (_PIPE3(pipe, PIPEA_CGM_CSC, PIPEB_CGM_CSC, PIPEC_CGM_CSC))
>  
> +/* BDW gamma correction */
> +#define PAL_PREC_INDEX_A   0x4A400
> +#define PAL_PREC_INDEX_B   0x4AC00
> +#define PAL_PREC_INDEX_C   0x4B400
> +#define PAL_PREC_DATA_A0x4A404
> +#define PAL_PREC_DATA_B0x4AC04
> +#define PAL_PREC_DATA_C0x4B404
> +#define PAL_PREC_GCMAX_A 0x4A410
> +#define PAL_PREC_GCMAX_B 0x4AC10
> +#define PAL_PREC_GCMAX_C 0x4B410
> +
> +#define _PREC_PAL_INDEX(pipe) \
> + (_PIPE3(pipe, PAL_PREC_INDEX_A, PAL_PREC_INDEX_B,
> PAL_PREC_INDEX_C))
> +#define _PREC_PAL_DATA(pipe) \
> + (_PIPE3(pipe, PAL_PREC_DATA_A, PAL_PREC_DATA_B,
> PAL_PREC_DATA_C))
> +#define _PREC_PAL_GCMAX(pipe) \
> + (_PIPE3(pipe, PAL_PREC_GCMAX_A, PAL_PREC_GCMAX_B,
> PAL_PREC_GCMAX_C))
>  
>  
>  #endif /* _I915_REG_H_ */
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c
> b/drivers/gpu/drm/i915/intel_color_manager.c
> index d5315b2..74f8fc3 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.c
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -26,6 +26,252 @@
>   */
>  
>  #include "intel_color_manager.h"
> +static void bdw_write_10bit_gamma_precision(struct drm_device *dev,
> + struct drm_r32g32b32 *correction_values, u32 pal_prec_data,
> + u32 no_of_coeff)
> +{
> + u16 blue_fract, green_fract, red_fract;
> + u32 word = 0;
> + u32 count = 0;
> + u32 blue, green, red;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + while (count < no_of_coeff) {
> +
> + blue = correction_values[count].b32;
> + green = correction_values[count].g32;
> + red = correction_values[count].r32;
> +
> + /*
> + * Maximum possible gamma correction value supported
> + * for BDW is 0x, so clip the values
> accordingly
> + */

I think you mean clamp not clip.

> + if (blue >= BDW_MAX_GAMMA)
> + blue = BDW_MAX_GAMMA;
> + if (green >= BDW_MAX_GAMMA)
> + green = BDW_MAX_GAMMA;
> + if (red >= BDW_MAX_GAMMA)
> + red = BDW_MAX_GAMMA;

So this handles the issue that was raised before that 1.0 in 8.24
should map to 1023.

> +
> + /*
> + * Gamma correction values are sent in 8.24 format
> + * with 8 int and 24 fraction bits. BDW 10 bit gamma
> + * unit expects correction registers to be programmed
> in
> + * 0.10 format, with 0 int and 16 fraction bits. So
> take
> + * MSB 10 bit values(bits 23-14) from the fraction
> part and
> + * prepare the correction registers.
> + */
> + blue_fract = GET_BITS(blue, 14, 10);
> + green_fract = GET_BITS(green, 14, 10);
> + red_fract = GET_BITS(red, 14, 10);
> +

I think this should round to the nearest rather than floor.

> + 

Re: [Intel-gfx] [PATCH 21/22] drm/i915: BDW: Pipe level degamma correction

2015-10-12 Thread Rob Bradford
On Sat, 2015-10-10 at 00:59 +0530, Shashank Sharma wrote:
> BDW/SKL/BXT supports Degamma color correction feature, which
> linearizes the non-linearity due to gamma encoded color values.
> This will be applied before Color Transformation.
> 
> This patch does the following:
> 1. Adds the core function to program DeGamma correction values for
>BDW/SKL/BXT platform
> 2. Adds DeGamma correction macros/defines
> 
> Signed-off-by: Shashank Sharma 
> Signed-off-by: Kausal Malladi 
> ---
>  drivers/gpu/drm/i915/intel_color_manager.c | 59
> ++
>  1 file changed, 59 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c
> b/drivers/gpu/drm/i915/intel_color_manager.c
> index 74f8fc3..e659382 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.c
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -273,6 +273,63 @@ static int bdw_set_gamma(struct drm_device *dev,
> struct drm_property_blob *blob,
>   return 0;
>  }
>  
> +static int bdw_set_degamma(struct drm_device *dev,
> + struct drm_property_blob *blob, struct drm_crtc *crtc)
> +{
> + enum pipe pipe;
> + int num_samples, length;
> + u32 index, mode;
> + u32 pal_prec_index, pal_prec_data;
> + struct drm_palette *degamma_data;
> + struct drm_crtc_state *state = crtc->state;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_r32g32b32 *correction_values = NULL;
> +
> + if (WARN_ON(!blob))
> + return -EINVAL;
> +
> + degamma_data = (struct drm_palette *)blob->data;
> + pipe = to_intel_crtc(crtc)->pipe;
> + num_samples = degamma_data->num_samples;
> +
> + if (num_samples == GAMMA_DISABLE_VALS) {
> + /* Disable degamma on Pipe */
> + mode = I915_READ(GAMMA_MODE(pipe)) &
> ~GAMMA_MODE_MODE_MASK;
> + I915_WRITE(GAMMA_MODE(pipe), mode |
> GAMMA_MODE_MODE_8BIT);

When you turn off gamma you call bdw_reset_gamma() should you do the
same for degamma?

> +
> + state->palette_before_ctm_blob = NULL;
> + DRM_DEBUG_DRIVER("Disabling degamma on Pipe %c\n",
> + pipe_name(pipe));
> + return 0;
> + }
> +
> + if (num_samples != BDW_SPLITGAMMA_MAX_VALS) {
> + DRM_ERROR("Invalid number of samples\n");
> + return -EINVAL;
> + }
> +
> + length = num_samples * sizeof(struct drm_r32g32b32);

Uh, you calculate this length and don't use it anywhere? Was your
intention to check the blob length? And the length check should be in
the generic DRM code anyway...

I think it was suggested in the past that the number of samples could
be derived from the length of the data allowing the removal of the
struct member.

> + mode = I915_READ(GAMMA_MODE(pipe));

Move this closer to where you use it?

> + pal_prec_index = _PREC_PAL_INDEX(pipe);
> + pal_prec_data = _PREC_PAL_DATA(pipe);
> +
> + correction_values = (struct drm_r32g32b32 *)°amma_data
> ->lut;

Why do you need this cast?

> + index = I915_READ(pal_prec_index);
> + index |= BDW_INDEX_AUTO_INCREMENT | BDW_INDEX_SPLIT_MODE;
> + I915_WRITE(pal_prec_index, index);
> +
> + bdw_write_10bit_gamma_precision(dev, correction_values,
> + pal_prec_data, BDW_SPLITGAMMA_MAX_VALS);
> +
> + /* Enable DeGamma on Pipe */
> + mode &= ~GAMMA_MODE_MODE_MASK;
> + I915_WRITE(GAMMA_MODE(pipe), mode | GAMMA_MODE_MODE_SPLIT);
> + DRM_DEBUG_DRIVER("DeGamma correction enabled on Pipe %c\n",

Choose your capitalisation DeGamma or degamma. Pick one and use it
consistently to make it easier to grep through the code.

It also looks like you should check if the gamma mode is not something
other than split / off. Otherwise strange things could happen.
Similarly in the gamma code you shouldn't be able to program something
other than split if you have a degamma mode set.

> + pipe_name(pipe));
> +
> + return 0;
> +}
> +
>  static s16 chv_prepare_csc_coeff(s64 csc_value)
>  {
>   s32 csc_int_value;
> @@ -579,6 +636,8 @@ void intel_color_manager_crtc_commit(struct
> drm_device *dev,
>   /* Degamma correction */
>   if (IS_CHERRYVIEW(dev))
>   ret = chv_set_degamma(dev, blob, crtc);
> + else if (IS_BROADWELL(dev) || IS_GEN9(dev))
> + ret = bdw_set_degamma(dev, blob, crtc);
>  
>   if (ret)
>   DRM_ERROR("set degamma correction
> failed\n");

Rob
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe timing for BXT DSI

2015-10-12 Thread Ville Syrjälä
On Mon, Oct 12, 2015 at 10:55:02PM +0530, Uma Shankar wrote:
> For BXT DSI, vtotal, vactive, hactive registers are different.
> Making changes to intel_crtc_mode_get() and get_pipe_timings(),
> to read the correct registers for BXT DSI.
> 
> Signed-off-by: Uma Shankar 
> Signed-off-by: Vandana Kannan 
> ---
>  drivers/gpu/drm/i915/intel_display.c |   48 
> +++---
>  1 file changed, 45 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 75c60b8..2717823 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7708,6 +7708,7 @@ static void intel_get_pipe_timings(struct intel_crtc 
> *crtc,
>   struct drm_device *dev = crtc->base.dev;
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> + bool is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI);
>   uint32_t tmp;
>  
>   tmp = I915_READ(HTOTAL(cpu_transcoder));
> @@ -7736,6 +7737,26 @@ static void intel_get_pipe_timings(struct intel_crtc 
> *crtc,
>   pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
>   }
>  
> + if (IS_BROXTON(dev) && is_dsi) {
> + struct intel_encoder *encoder;
> +
> + for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
> + struct intel_dsi *intel_dsi =
> + enc_to_intel_dsi(&encoder->base);
> + enum port port;
> +
> + for_each_dsi_port(port, intel_dsi->ports) {
> + pipe_config->base.adjusted_mode.crtc_hdisplay =
> + I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
> + pipe_config->base.adjusted_mode.crtc_vdisplay =
> + I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
> + pipe_config->base.adjusted_mode.crtc_vtotal =
> + I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
> + }
> + }
> +
> + }

I think I already asked this earlier when this patch was posted; Don't
the normal pipe timing registers contain the same values? And if so, why
would you need to read them out from the DSI speciific registers?

> +
>   tmp = I915_READ(PIPESRC(crtc->pipe));
>   pipe_config->pipe_src_h = (tmp & 0x) + 1;
>   pipe_config->pipe_src_w = ((tmp >> 16) & 0x) + 1;
> @@ -10664,6 +10685,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct 
> drm_device *dev,
>   int vtot = I915_READ(VTOTAL(cpu_transcoder));
>   int vsync = I915_READ(VSYNC(cpu_transcoder));
>   enum pipe pipe = intel_crtc->pipe;
> + bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
>  
>   mode = kzalloc(sizeof(*mode), GFP_KERNEL);
>   if (!mode)
> @@ -10684,15 +10706,35 @@ struct drm_display_mode *intel_crtc_mode_get(struct 
> drm_device *dev,
>   i9xx_crtc_clock_get(intel_crtc, &pipe_config);
>  
>   mode->clock = pipe_config.port_clock / pipe_config.pixel_multiplier;
> - mode->hdisplay = (htot & 0x) + 1;
>   mode->htotal = ((htot & 0x) >> 16) + 1;
>   mode->hsync_start = (hsync & 0x) + 1;
>   mode->hsync_end = ((hsync & 0x) >> 16) + 1;
> - mode->vdisplay = (vtot & 0x) + 1;
> - mode->vtotal = ((vtot & 0x) >> 16) + 1;
>   mode->vsync_start = (vsync & 0x) + 1;
>   mode->vsync_end = ((vsync & 0x) >> 16) + 1;
>  
> + if (IS_BROXTON(dev) && is_dsi) {
> + struct intel_encoder *encoder;
> +
> + for_each_encoder_on_crtc(dev, &intel_crtc->base, encoder) {
> + struct intel_dsi *intel_dsi =
> + enc_to_intel_dsi(&encoder->base);
> + enum port port;
> +
> + for_each_dsi_port(port, intel_dsi->ports) {
> + mode->vtotal =
> + I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
> + mode->hdisplay =
> + I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
> + mode->vdisplay =
> + I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
> + }
> + }
> + } else {
> + mode->vtotal = ((vtot & 0x) >> 16) + 1;
> + mode->hdisplay = (htot & 0x) + 1;
> + mode->vdisplay = (vtot & 0x) + 1;
> + }
> +
>   drm_mode_set_name(mode);
>  
>   return mode;
> -- 
> 1.7.9.5
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Inte

[Intel-gfx] [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe timing for BXT DSI

2015-10-12 Thread Uma Shankar
For BXT DSI, vtotal, vactive, hactive registers are different.
Making changes to intel_crtc_mode_get() and get_pipe_timings(),
to read the correct registers for BXT DSI.

Signed-off-by: Uma Shankar 
Signed-off-by: Vandana Kannan 
---
 drivers/gpu/drm/i915/intel_display.c |   48 +++---
 1 file changed, 45 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 75c60b8..2717823 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7708,6 +7708,7 @@ static void intel_get_pipe_timings(struct intel_crtc 
*crtc,
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
+   bool is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI);
uint32_t tmp;
 
tmp = I915_READ(HTOTAL(cpu_transcoder));
@@ -7736,6 +7737,26 @@ static void intel_get_pipe_timings(struct intel_crtc 
*crtc,
pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
}
 
+   if (IS_BROXTON(dev) && is_dsi) {
+   struct intel_encoder *encoder;
+
+   for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
+   struct intel_dsi *intel_dsi =
+   enc_to_intel_dsi(&encoder->base);
+   enum port port;
+
+   for_each_dsi_port(port, intel_dsi->ports) {
+   pipe_config->base.adjusted_mode.crtc_hdisplay =
+   I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
+   pipe_config->base.adjusted_mode.crtc_vdisplay =
+   I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
+   pipe_config->base.adjusted_mode.crtc_vtotal =
+   I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
+   }
+   }
+
+   }
+
tmp = I915_READ(PIPESRC(crtc->pipe));
pipe_config->pipe_src_h = (tmp & 0x) + 1;
pipe_config->pipe_src_w = ((tmp >> 16) & 0x) + 1;
@@ -10664,6 +10685,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct 
drm_device *dev,
int vtot = I915_READ(VTOTAL(cpu_transcoder));
int vsync = I915_READ(VSYNC(cpu_transcoder));
enum pipe pipe = intel_crtc->pipe;
+   bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
 
mode = kzalloc(sizeof(*mode), GFP_KERNEL);
if (!mode)
@@ -10684,15 +10706,35 @@ struct drm_display_mode *intel_crtc_mode_get(struct 
drm_device *dev,
i9xx_crtc_clock_get(intel_crtc, &pipe_config);
 
mode->clock = pipe_config.port_clock / pipe_config.pixel_multiplier;
-   mode->hdisplay = (htot & 0x) + 1;
mode->htotal = ((htot & 0x) >> 16) + 1;
mode->hsync_start = (hsync & 0x) + 1;
mode->hsync_end = ((hsync & 0x) >> 16) + 1;
-   mode->vdisplay = (vtot & 0x) + 1;
-   mode->vtotal = ((vtot & 0x) >> 16) + 1;
mode->vsync_start = (vsync & 0x) + 1;
mode->vsync_end = ((vsync & 0x) >> 16) + 1;
 
+   if (IS_BROXTON(dev) && is_dsi) {
+   struct intel_encoder *encoder;
+
+   for_each_encoder_on_crtc(dev, &intel_crtc->base, encoder) {
+   struct intel_dsi *intel_dsi =
+   enc_to_intel_dsi(&encoder->base);
+   enum port port;
+
+   for_each_dsi_port(port, intel_dsi->ports) {
+   mode->vtotal =
+   I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
+   mode->hdisplay =
+   I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
+   mode->vdisplay =
+   I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
+   }
+   }
+   } else {
+   mode->vtotal = ((vtot & 0x) >> 16) + 1;
+   mode->hdisplay = (htot & 0x) + 1;
+   mode->vdisplay = (vtot & 0x) + 1;
+   }
+
drm_mode_set_name(mode);
 
return mode;
-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [BXT DSI timing fixes v1 3/3] drm/i915/bxt: Fixed dsi enc disable and blank at bootup

2015-10-12 Thread Uma Shankar
During bootup, mipi display was blanking in BXT. This is because during
driver load, though encoder, connector were active but crtc returned
inactive. This caused sanitize function to disable the DSI panel. In AOS,
this is fine since HWC will do a modeset and crtc, connector, encoder
mapping will be restored. But in Charging OS, no modeset is called, it
just calls DPMS ON/OFF. Hence display doesn't come in COS. This is needed
even for seamless booting to Android without a blank.

This is fine on BYT/CHT since transcoder is common b/w all encoders. But
for BXT, there is a separate mipi transcoder. Hence this needs special
handling for BXT DSI.

Signed-off-by: Uma Shankar 
---
 drivers/gpu/drm/i915/i915_drv.h  |3 +++
 drivers/gpu/drm/i915/intel_display.c |   27 +++
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bf14096..ae790ec 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1948,6 +1948,9 @@ struct drm_i915_private {
/* perform PHY state sanity checks? */
bool chv_phy_assert[2];
 
+   /* To check if DSI is initializing at bootup */
+   bool dsi_enumerating;
+
/*
 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
 * will be rejected. Instead look for a better place.
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 2717823..fe4f4f3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7711,6 +7711,9 @@ static void intel_get_pipe_timings(struct intel_crtc 
*crtc,
bool is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI);
uint32_t tmp;
 
+   if (dev_priv->dsi_enumerating && dev_priv->vbt.has_mipi)
+   is_dsi = true;
+
tmp = I915_READ(HTOTAL(cpu_transcoder));
pipe_config->base.adjusted_mode.crtc_hdisplay = (tmp & 0x) + 1;
pipe_config->base.adjusted_mode.crtc_htotal = ((tmp >> 16) & 0x) + 
1;
@@ -9825,10 +9828,20 @@ static bool haswell_get_pipe_config(struct intel_crtc 
*crtc,
is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI);
 
/*
-* Check if encoder is enabled or not
+* Check if encoder is enabled or not.
 * Separate implementation for DSI and DDI encoders.
+* For first time during driver init, encoder association
+* would not have happened and this function will return
+* false. This will cause encoder to be disabled causing
+* a blank, till user space does a modeset. In order to avoid
+* this, if DSI is enabled in VBT, for first iteration, this
+* will return true since BIOS would have initialized MIPI.
+* This is needed for seamless booting without blanking and
+* for Charging OS scenario. Since DSI is the first display in
+* setup_outputs, hence first crtc will be associated with mipi
+* display
 */
-   if (is_dsi) {
+   if (is_dsi || (dev_priv->dsi_enumerating && dev_priv->vbt.has_mipi)) {
struct intel_encoder *encoder;
 
for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
@@ -9852,8 +9865,11 @@ static bool haswell_get_pipe_config(struct intel_crtc 
*crtc,
if (dsi_enc_enabled)
break;
}
-   if (!dsi_enc_enabled)
-   return false;
+
+if (!dsi_enc_enabled) {
+   if (!dev_priv->dsi_enumerating)
+   return false;
+   }
} else {
 
tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
@@ -9921,6 +9937,8 @@ static bool haswell_get_pipe_config(struct intel_crtc 
*crtc,
pipe_config->pixel_multiplier = 1;
}
 
+   dev_priv->dsi_enumerating = false;
+
return true;
 }
 
@@ -14877,6 +14895,7 @@ void intel_modeset_init(struct drm_device *dev)
enum pipe pipe;
struct intel_crtc *crtc;
 
+   dev_priv->dsi_enumerating = true;
drm_mode_config_init(dev);
 
dev->mode_config.min_width = 0;
-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [BXT DSI timing fixes v1 1/3] drm/i915/: DSI mode setting fix

2015-10-12 Thread Uma Shankar
Fixed dsi crtc state. Updated the get config function
and handled the DSI and DDI encoder cases.

BXT DSI have to be handled differently from rest of the encoders.
Reading the port control register to determine if DSI is enabled.
Generalizing it for all existing platforms.

Signed-off-by: Uma Shankar 
Signed-off-by: Vandana Kannan 
---
 drivers/gpu/drm/i915/intel_display.c |   94 --
 1 file changed, 67 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index cddb0c6..75c60b8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -44,6 +44,7 @@
 #include 
 #include 
 #include 
+#include "intel_dsi.h"
 
 /* Primary plane formats for gen <= 3 */
 static const uint32_t i8xx_primary_formats[] = {
@@ -9788,46 +9789,85 @@ static bool haswell_get_pipe_config(struct intel_crtc 
*crtc,
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
enum intel_display_power_domain pfit_domain;
-   uint32_t tmp;
+   uint32_t tmp = 0;
+   bool is_dsi = false;
+   bool dsi_enc_enabled = false;
+   u32 port_ctrl = 0;
 
if (!intel_display_power_is_enabled(dev_priv,
-POWER_DOMAIN_PIPE(crtc->pipe)))
+   POWER_DOMAIN_PIPE(crtc->pipe)))
return false;
 
pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
pipe_config->shared_dpll = DPLL_ID_PRIVATE;
 
-   tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
-   if (tmp & TRANS_DDI_FUNC_ENABLE) {
-   enum pipe trans_edp_pipe;
-   switch (tmp & TRANS_DDI_EDP_INPUT_MASK) {
-   default:
-   WARN(1, "unknown pipe linked to edp transcoder\n");
-   case TRANS_DDI_EDP_INPUT_A_ONOFF:
-   case TRANS_DDI_EDP_INPUT_A_ON:
-   trans_edp_pipe = PIPE_A;
-   break;
-   case TRANS_DDI_EDP_INPUT_B_ONOFF:
-   trans_edp_pipe = PIPE_B;
-   break;
-   case TRANS_DDI_EDP_INPUT_C_ONOFF:
-   trans_edp_pipe = PIPE_C;
-   break;
+   is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI);
+
+   /*
+* Check if encoder is enabled or not
+* Separate implementation for DSI and DDI encoders.
+*/
+   if (is_dsi) {
+   struct intel_encoder *encoder;
+
+   for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
+   struct intel_dsi *intel_dsi =
+   enc_to_intel_dsi(&encoder->base);
+   enum port port;
+
+   for_each_dsi_port(port, intel_dsi->ports) {
+   if (IS_BROXTON(dev))
+   port_ctrl = BXT_MIPI_PORT_CTRL(port);
+   else if (IS_VALLEYVIEW(dev))
+   port_ctrl = MIPI_PORT_CTRL(port);
+
+   tmp = I915_READ(port_ctrl);
+   if (tmp & DPI_ENABLE) {
+   dsi_enc_enabled = true;
+   break;
+   }
+   }
+
+   if (dsi_enc_enabled)
+   break;
}
+   if (!dsi_enc_enabled)
+   return false;
+   } else {
 
-   if (trans_edp_pipe == crtc->pipe)
-   pipe_config->cpu_transcoder = TRANSCODER_EDP;
-   }
+   tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
+   if (tmp & TRANS_DDI_FUNC_ENABLE) {
+   enum pipe trans_edp_pipe;
 
-   if (!intel_display_power_is_enabled(dev_priv,
+   switch (tmp & TRANS_DDI_EDP_INPUT_MASK) {
+   default:
+   WARN(1, "unknown pipe linked to edp 
transcoder\n");
+   case TRANS_DDI_EDP_INPUT_A_ONOFF:
+   case TRANS_DDI_EDP_INPUT_A_ON:
+   trans_edp_pipe = PIPE_A;
+   break;
+   case TRANS_DDI_EDP_INPUT_B_ONOFF:
+   trans_edp_pipe = PIPE_B;
+   break;
+   case TRANS_DDI_EDP_INPUT_C_ONOFF:
+   trans_edp_pipe = PIPE_C;
+   break;
+   }
+
+   if (trans_edp_pipe == crtc->pipe)
+   pipe_config->cpu_transcoder = TRANSCODER_EDP;
+   }
+
+   if (!intel_display_power_is_enabled(dev_priv,
POWER_DOMAIN_TRANSCODER(pip

[Intel-gfx] [BXT DSI timing fixes v1 0/3] BXT DSI mode timing fixes

2015-10-12 Thread Uma Shankar
This patch series adds handling for getting mode and timing parameters
for BXT DSI.

MIPI DSI for BXT uses a separate trasncoder, hence special handling is
needed to address this. 

Floating version 1 of the patches to get the feedback and address the
BXT DSI HW devaition and corresponding SW design.

Uma Shankar (3):
  drm/i915/: DSI mode setting fix
  drm/i915/bxt: Get pipe timing for BXT DSI
  drm/i915/bxt: Fixed dsi enc disable and  blank at bootup

 drivers/gpu/drm/i915/i915_drv.h  |3 +
 drivers/gpu/drm/i915/intel_display.c |  161 +++---
 2 files changed, 134 insertions(+), 30 deletions(-)

-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 22/22] drm/i915: BDW: Pipe level CSC correction

2015-10-12 Thread Rob Bradford
On Sat, 2015-10-10 at 00:59 +0530, Shashank Sharma wrote:
> BDW/SKL/BXT support Color Space Conversion (CSC) using a 3x3 matrix
> that needs to be programmed into respective CSC registers.
> 
> This patch does the following:
> 1. Adds the core function to program CSC correction values for
>BDW/SKL/BXT platform
> 2. Adds CSC correction macros/defines
> 
> Signed-off-by: Shashank Sharma 
> Signed-off-by: Kausal Malladi 
> Signed-off-by: Kumar, Kiran S 
> ---
>  drivers/gpu/drm/i915/i915_reg.h|   7 ++
>  drivers/gpu/drm/i915/intel_color_manager.c | 114
> -
>  drivers/gpu/drm/i915/intel_color_manager.h |  12 ++-
>  3 files changed, 129 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index ed50f75..0e9d252 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8085,4 +8085,11 @@ enum skl_disp_power_wells {
>   (_PIPE3(pipe, PAL_PREC_GCMAX_A, PAL_PREC_GCMAX_B,
> PAL_PREC_GCMAX_C))
>  
>  
> +/* BDW CSC correction */
> +#define CSC_COEFF_A  0x49010
> +#define CSC_COEFF_B  0x49110
> +#define CSC_COEFF_C  0x49210
> +#define _PIPE_CSC_COEFF(pipe) \
> + (_PIPE3(pipe, CSC_COEFF_A, CSC_COEFF_B, CSC_COEFF_C))
> +
>  #endif /* _I915_REG_H_ */
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c
> b/drivers/gpu/drm/i915/intel_color_manager.c
> index e659382..0a6c00c 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.c
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -330,11 +330,119 @@ static int bdw_set_degamma(struct drm_device
> *dev,
>   return 0;
>  }
>  
> -static s16 chv_prepare_csc_coeff(s64 csc_value)
> +static uint32_t bdw_prepare_csc_coeff(int64_t coeff)
> +{
> + uint32_t reg_val, ls_bit_pos, exponent_bits, sign_bit = 0;
> + int32_t mantissa;
> + uint64_t abs_coeff;
> +
> + coeff = min_t(int64_t, coeff, BDW_CSC_COEFF_MAX_VAL);
> + coeff = max_t(int64_t, coeff, BDW_CSC_COEFF_MIN_VAL);
> +
> + abs_coeff = abs(coeff);
> + if (abs_coeff < (BDW_CSC_COEFF_UNITY_VAL >> 3)) {
> + /* abs_coeff < 0.125 */
> + exponent_bits = 3;
> + ls_bit_pos = 19;
> + } else if (abs_coeff >= (BDW_CSC_COEFF_UNITY_VAL >> 3) &&
> + abs_coeff < (BDW_CSC_COEFF_UNITY_VAL >> 2)) {
> + /* abs_coeff >= 0.125 && val < 0.25 */
> + exponent_bits = 2;
> + ls_bit_pos = 20;
> + } else if (abs_coeff >= (BDW_CSC_COEFF_UNITY_VAL >> 2)
> + && abs_coeff < (BDW_CSC_COEFF_UNITY_VAL >> 1)) {
> + /* abs_coeff >= 0.25 && val < 0.5 */
> + exponent_bits = 1;
> + ls_bit_pos = 21;
> + } else if (abs_coeff >= (BDW_CSC_COEFF_UNITY_VAL >> 1)
> + && abs_coeff < BDW_CSC_COEFF_UNITY_VAL) {
> + /* abs_coeff >= 0.5 && val < 1.0 */
> + exponent_bits = 0;
> + ls_bit_pos = 22;
> + } else if (abs_coeff >= BDW_CSC_COEFF_UNITY_VAL &&
> + abs_coeff < (BDW_CSC_COEFF_UNITY_VAL << 1)) {
> + /* abs_coeff >= 1.0 && val < 2.0 */
> + exponent_bits = 7;
> + ls_bit_pos = 23;
> + } else {
> + /* abs_coeff >= 2.0 && val < 4.0 */
> + exponent_bits = 6;
> + ls_bit_pos = 24;
> + }
> +
> + mantissa = GET_BITS_ROUNDOFF(abs_coeff, ls_bit_pos,
> CSC_MAX_VALS);

Is GET_BITS_ROUNDOFF safe for negative values?

> + if (coeff < 0) {
> + sign_bit = 1;
> + mantissa = -mantissa;
> + mantissa &= ((1 << CSC_MAX_VALS) - 1);

Oops. It looks like you are reusing the macro/define for the number of
coefficients for the size of the mantissa. Also you defined a macro for
this very purpose in your other patch.

> + }
> +
> + reg_val = 0;
> + SET_BITS(reg_val, exponent_bits, 12, 3);
> + SET_BITS(reg_val, mantissa, 3, 9);
> + SET_BITS(reg_val, sign_bit, 15, 1);
> + DRM_DEBUG_DRIVER("CSC: reg_val=0x%x\n", reg_val);

This debug message is superfluous as you can easily read register
values out with i-g-t.

> + return reg_val;
> +}
> +
> +int bdw_set_csc(struct drm_device *dev, struct drm_property_blob
> *blob,
> + struct drm_crtc *crtc)
> +{
> + enum pipe pipe;
> + enum plane plane;
> + int i, j, temp;
> + int word = 0;
> + u32 reg, plane_ctl, mode;
> + struct drm_ctm *csc_data;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + if (WARN_ON(!blob))
> + return -EINVAL;
> +
> + if (blob->length != sizeof(struct drm_ctm)) {
> + DRM_ERROR("Invalid length of data received\n");
> + return -EINVAL;
> + }
> +
> + csc_data = (struct drm_ctm *)blob->data;
> + pipe = to_intel_crtc(crtc)->pipe;
> + plane = to_intel_crtc(crtc)->plane;
> +
> + plane_ctl = I915_READ(PLANE_CTL(pipe, plane)

[Intel-gfx] [PATCH v2 23/43] drm/i915: Eliminate weird parameter inversion from BXT PPS registers

2015-10-12 Thread ville . syrjala
From: Ville Syrjälä 

v2: Keep using the same registers (PCH_*) instead of accidentally
starting to use the other ones (BXT_*)2) (Jesse)

Reviewed-by: Jesse Barnes 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/i915_reg.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3dad335..7bf11f5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6543,10 +6543,10 @@ enum skl_disp_power_wells {
 #define _BXT_PP_ON_DELAYS2 0xc7308
 #define _BXT_PP_OFF_DELAYS20xc730c
 
-#define BXT_PP_STATUS(n)   ((!n) ? PCH_PP_STATUS : _BXT_PP_STATUS2)
-#define BXT_PP_CONTROL(n)  ((!n) ? PCH_PP_CONTROL : _BXT_PP_CONTROL2)
-#define BXT_PP_ON_DELAYS(n)((!n) ? PCH_PP_ON_DELAYS : _BXT_PP_ON_DELAYS2)
-#define BXT_PP_OFF_DELAYS(n)   ((!n) ? PCH_PP_OFF_DELAYS : _BXT_PP_OFF_DELAYS2)
+#define BXT_PP_STATUS(n)   _PIPE(n, PCH_PP_STATUS, _BXT_PP_STATUS2)
+#define BXT_PP_CONTROL(n)  _PIPE(n, PCH_PP_CONTROL, _BXT_PP_CONTROL2)
+#define BXT_PP_ON_DELAYS(n)_PIPE(n, PCH_PP_ON_DELAYS, _BXT_PP_ON_DELAYS2)
+#define BXT_PP_OFF_DELAYS(n)   _PIPE(n, PCH_PP_OFF_DELAYS, _BXT_PP_OFF_DELAYS2)
 
 #define PCH_DP_B   0xe4100
 #define PCH_DPB_AUX_CH_CTL 0xe4110
-- 
2.4.9

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 30/43] drm/i915: Parametrize and fix SWF registers

2015-10-12 Thread Ville Syrjälä
On Mon, Oct 12, 2015 at 09:07:17AM -0700, Jesse Barnes wrote:
> On 09/18/2015 10:03 AM, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> > 
> > Parametrize the SWF registers. This also fixes the register offsets,
> > which were mostly garbage in the old defines.
> > 
> > Also save/restore only as many SWF registers that each platform has.
> > 
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h |  2 +-
> >  drivers/gpu/drm/i915/i915_reg.h | 28 +++
> >  drivers/gpu/drm/i915/i915_suspend.c | 45 
> > -
> >  3 files changed, 50 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 3bf8a9b..3e35e08 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1041,7 +1041,7 @@ struct i915_suspend_saved_registers {
> > u32 saveMI_ARB_STATE;
> > u32 saveSWF0[16];
> > u32 saveSWF1[16];
> > -   u32 saveSWF2[3];
> > +   u32 saveSWF3[3];
> > uint64_t saveFENCE[I915_MAX_NUM_FENCES];
> > u32 savePCH_PORT_HOTPLUG;
> > u16 saveGCDGMBUS;
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 0cc41e4b..57b9469 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4941,20 +4941,20 @@ enum skl_disp_power_wells {
> >  #define I915_LO_DISPBASE(val)  (val & ~DISP_BASEADDR_MASK)
> >  #define I915_HI_DISPBASE(val)  (val & DISP_BASEADDR_MASK)
> >  
> > -/* VBIOS flags */
> > -#define SWF00  (dev_priv->info.display_mmio_offset + 
> > 0x71410)
> > -#define SWF01  (dev_priv->info.display_mmio_offset + 
> > 0x71414)
> > -#define SWF02  (dev_priv->info.display_mmio_offset + 
> > 0x71418)
> > -#define SWF03  (dev_priv->info.display_mmio_offset + 
> > 0x7141c)
> > -#define SWF04  (dev_priv->info.display_mmio_offset + 
> > 0x71420)
> > -#define SWF05  (dev_priv->info.display_mmio_offset + 
> > 0x71424)
> > -#define SWF06  (dev_priv->info.display_mmio_offset + 
> > 0x71428)
> > -#define SWF10  (dev_priv->info.display_mmio_offset + 
> > 0x70410)
> > -#define SWF11  (dev_priv->info.display_mmio_offset + 
> > 0x70414)
> > -#define SWF14  (dev_priv->info.display_mmio_offset + 
> > 0x71420)
> > -#define SWF30  (dev_priv->info.display_mmio_offset + 
> > 0x72414)
> > -#define SWF31  (dev_priv->info.display_mmio_offset + 
> > 0x72418)
> > -#define SWF32  (dev_priv->info.display_mmio_offset + 
> > 0x7241c)
> > +/*
> > + * VBIOS flags
> > + * gen2:
> > + * [00:06] alm,mgm
> > + * [10:16] all
> > + * [30:32] alm,mgm
> > + * gen3+:
> > + * [00:0f] all
> > + * [10:1f] all
> > + * [30:32] all
> > + */
> > +#define SWF0(i)(dev_priv->info.display_mmio_offset + 0x70410 + (i) * 4)
> > +#define SWF1(i)(dev_priv->info.display_mmio_offset + 0x71410 + (i) * 4)
> > +#define SWF3(i)(dev_priv->info.display_mmio_offset + 0x72414 + (i) * 4)
> >  
> >  /* Pipe B */
> >  #define _PIPEBDSL  (dev_priv->info.display_mmio_offset + 0x71000)
> > diff --git a/drivers/gpu/drm/i915/i915_suspend.c 
> > b/drivers/gpu/drm/i915/i915_suspend.c
> > index 1ccac61..2d91821 100644
> > --- a/drivers/gpu/drm/i915/i915_suspend.c
> > +++ b/drivers/gpu/drm/i915/i915_suspend.c
> > @@ -122,12 +122,24 @@ int i915_save_state(struct drm_device *dev)
> > dev_priv->regfile.saveMI_ARB_STATE = I915_READ(MI_ARB_STATE);
> >  
> > /* Scratch space */
> > -   for (i = 0; i < 16; i++) {
> > -   dev_priv->regfile.saveSWF0[i] = I915_READ(SWF00 + (i << 2));
> > -   dev_priv->regfile.saveSWF1[i] = I915_READ(SWF10 + (i << 2));
> > +   if (IS_GEN2(dev_priv) && IS_MOBILE(dev_priv)) {
> > +   for (i = 0; i < 7; i++) {
> > +   dev_priv->regfile.saveSWF0[i] = I915_READ(SWF0(i));
> > +   dev_priv->regfile.saveSWF1[i] = I915_READ(SWF1(i));
> > +   }
> > +   for (i = 0; i < 3; i++)
> > +   dev_priv->regfile.saveSWF3[i] = I915_READ(SWF3(i));
> > +   } else if (IS_GEN2(dev_priv)) {
> > +   for (i = 0; i < 7; i++)
> > +   dev_priv->regfile.saveSWF1[i] = I915_READ(SWF1(i));
> > +   } else if (HAS_GMCH_DISPLAY(dev_priv)) {
> > +   for (i = 0; i < 16; i++) {
> > +   dev_priv->regfile.saveSWF0[i] = I915_READ(SWF0(i));
> > +   dev_priv->regfile.saveSWF1[i] = I915_READ(SWF1(i));
> > +   }
> > +   for (i = 0; i < 3; i++)
> > +   dev_priv->regfile.saveSWF3[i] = I915_READ(SWF3(i));
> > }
> > -   for (i = 0; i < 3; i++)
> > -   dev_priv->regfile.saveSWF2[i] = I915_READ(SWF30 + (i << 2));
> >  
> > mutex_unlock(&dev

Re: [Intel-gfx] [PATCH 24/43] drm/i915: Parametrize HSW video DIP data registers

2015-10-12 Thread Ville Syrjälä
On Mon, Oct 12, 2015 at 08:54:56AM -0700, Jesse Barnes wrote:
> On 09/18/2015 10:03 AM, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> > 
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h   | 16 
> >  drivers/gpu/drm/i915/intel_hdmi.c | 26 ++
> >  drivers/gpu/drm/i915/intel_psr.c  | 18 ++
> >  3 files changed, 32 insertions(+), 28 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 134b075..b35e24f 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6236,16 +6236,16 @@ enum skl_disp_power_wells {
> >  
> >  #define HSW_TVIDEO_DIP_CTL(trans) \
> >  _TRANSCODER2(trans, HSW_VIDEO_DIP_CTL_A)
> > -#define HSW_TVIDEO_DIP_AVI_DATA(trans) \
> > -_TRANSCODER2(trans, HSW_VIDEO_DIP_AVI_DATA_A)
> > -#define HSW_TVIDEO_DIP_VS_DATA(trans) \
> > -_TRANSCODER2(trans, HSW_VIDEO_DIP_VS_DATA_A)
> > -#define HSW_TVIDEO_DIP_SPD_DATA(trans) \
> > -_TRANSCODER2(trans, HSW_VIDEO_DIP_SPD_DATA_A)
> > +#define HSW_TVIDEO_DIP_AVI_DATA(trans, i) \
> > +   (_TRANSCODER2(trans, HSW_VIDEO_DIP_AVI_DATA_A) + (i) * 4)
> > +#define HSW_TVIDEO_DIP_VS_DATA(trans, i) \
> > +   (_TRANSCODER2(trans, HSW_VIDEO_DIP_VS_DATA_A) + (i) * 4)
> > +#define HSW_TVIDEO_DIP_SPD_DATA(trans, i) \
> > +   (_TRANSCODER2(trans, HSW_VIDEO_DIP_SPD_DATA_A) + (i) * 4)
> >  #define HSW_TVIDEO_DIP_GCP(trans) \
> > _TRANSCODER2(trans, HSW_VIDEO_DIP_GCP_A)
> > -#define HSW_TVIDEO_DIP_VSC_DATA(trans) \
> > -_TRANSCODER2(trans, HSW_VIDEO_DIP_VSC_DATA_A)
> > +#define HSW_TVIDEO_DIP_VSC_DATA(trans, i) \
> > +   (_TRANSCODER2(trans, HSW_VIDEO_DIP_VSC_DATA_A) + (i) * 4)
> >  
> >  #define HSW_STEREO_3D_CTL_A0x70020
> >  #define   S3D_ENABLE   (1<<31)
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
> > b/drivers/gpu/drm/i915/intel_hdmi.c
> > index e978c59..6b16292 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -113,17 +113,18 @@ static u32 hsw_infoframe_enable(enum 
> > hdmi_infoframe_type type)
> > }
> >  }
> >  
> > -static u32 hsw_infoframe_data_reg(enum hdmi_infoframe_type type,
> > - enum transcoder cpu_transcoder,
> > - struct drm_i915_private *dev_priv)
> > +static u32 hsw_dip_data_reg(struct drm_i915_private *dev_priv,
> > +   enum transcoder cpu_transcoder,
> > +   enum hdmi_infoframe_type type,
> > +   int i)
> >  {
> > switch (type) {
> > case HDMI_INFOFRAME_TYPE_AVI:
> > -   return HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder);
> > +   return HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder, i);
> > case HDMI_INFOFRAME_TYPE_SPD:
> > -   return HSW_TVIDEO_DIP_SPD_DATA(cpu_transcoder);
> > +   return HSW_TVIDEO_DIP_SPD_DATA(cpu_transcoder, i);
> > case HDMI_INFOFRAME_TYPE_VENDOR:
> > -   return HSW_TVIDEO_DIP_VS_DATA(cpu_transcoder);
> > +   return HSW_TVIDEO_DIP_VS_DATA(cpu_transcoder, i);
> > default:
> > DRM_DEBUG_DRIVER("unknown info frame type %d\n", type);
> > return 0;
> > @@ -365,14 +366,13 @@ static void hsw_write_infoframe(struct drm_encoder 
> > *encoder,
> > struct drm_device *dev = encoder->dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> > -   u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->config->cpu_transcoder);
> > +   enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> > +   u32 ctl_reg = HSW_TVIDEO_DIP_CTL(cpu_transcoder);
> > u32 data_reg;
> > int i;
> > u32 val = I915_READ(ctl_reg);
> >  
> > -   data_reg = hsw_infoframe_data_reg(type,
> > - intel_crtc->config->cpu_transcoder,
> > - dev_priv);
> > +   data_reg = hsw_dip_data_reg(dev_priv, cpu_transcoder, type, 0);
> > if (data_reg == 0)
> > return;
> >  
> > @@ -381,12 +381,14 @@ static void hsw_write_infoframe(struct drm_encoder 
> > *encoder,
> >  
> > mmiowb();
> > for (i = 0; i < len; i += 4) {
> > -   I915_WRITE(data_reg + i, *data);
> > +   I915_WRITE(hsw_dip_data_reg(dev_priv, cpu_transcoder,
> > +   type, i >> 2), *data);
> > data++;
> > }
> > /* Write every possible data byte to force correct ECC calculation. */
> > for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
> > -   I915_WRITE(data_reg + i, 0);
> > +   I915_WRITE(hsw_dip_data_reg(dev_priv, cpu_transcoder,
> > +   type, i >> 2), 0);
> > mmiowb();
> >  
> > val |= hsw_infoframe_enable(type);
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c 
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index a04

Re: [Intel-gfx] [PATCH 33/43] drm/i915: Remove dev_priv argument from NEEDS_FORCE_WAKE

2015-10-12 Thread Jesse Barnes
On 09/18/2015 10:03 AM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
> b/drivers/gpu/drm/i915/intel_uncore.c
> index 3294f63..197ca397 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -525,7 +525,7 @@ void assert_forcewakes_inactive(struct drm_i915_private 
> *dev_priv)
>  }
>  
>  /* We give fast paths for the really cool registers */
> -#define NEEDS_FORCE_WAKE(dev_priv, reg) \
> +#define NEEDS_FORCE_WAKE(reg) \
>((reg) < 0x4 && (reg) != FORCEWAKE)
>  
>  #define REG_RANGE(reg, start, end) ((reg) >= (start) && (reg) < (end))
> @@ -727,7 +727,7 @@ static u##x \
>  gen6_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
>   GEN6_READ_HEADER(x); \
>   hsw_unclaimed_reg_debug(dev_priv, reg, true, true); \
> - if (NEEDS_FORCE_WAKE((dev_priv), (reg))) \
> + if (NEEDS_FORCE_WAKE(reg)) \
>   __force_wake_get(dev_priv, FORCEWAKE_RENDER); \
>   val = __raw_i915_read##x(dev_priv, reg); \
>   hsw_unclaimed_reg_debug(dev_priv, reg, true, false); \
> @@ -761,7 +761,7 @@ chv_read##x(struct drm_i915_private *dev_priv, off_t reg, 
> bool trace) { \
>   GEN6_READ_FOOTER; \
>  }
>  
> -#define SKL_NEEDS_FORCE_WAKE(dev_priv, reg)  \
> +#define SKL_NEEDS_FORCE_WAKE(reg) \
>((reg) < 0x4 && !FORCEWAKE_GEN9_UNCORE_RANGE_OFFSET(reg))
>  
>  #define __gen9_read(x) \
> @@ -770,9 +770,9 @@ gen9_read##x(struct drm_i915_private *dev_priv, off_t 
> reg, bool trace) { \
>   enum forcewake_domains fw_engine; \
>   GEN6_READ_HEADER(x); \
>   hsw_unclaimed_reg_debug(dev_priv, reg, true, true); \
> - if (!SKL_NEEDS_FORCE_WAKE((dev_priv), (reg)))   \
> + if (!SKL_NEEDS_FORCE_WAKE(reg)) \
>   fw_engine = 0; \
> - else if (FORCEWAKE_GEN9_RENDER_RANGE_OFFSET(reg))   \
> + else if (FORCEWAKE_GEN9_RENDER_RANGE_OFFSET(reg)) \
>   fw_engine = FORCEWAKE_RENDER; \
>   else if (FORCEWAKE_GEN9_MEDIA_RANGE_OFFSET(reg)) \
>   fw_engine = FORCEWAKE_MEDIA; \
> @@ -868,7 +868,7 @@ static void \
>  gen6_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool 
> trace) { \
>   u32 __fifo_ret = 0; \
>   GEN6_WRITE_HEADER; \
> - if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
> + if (NEEDS_FORCE_WAKE(reg)) { \
>   __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
>   } \
>   __raw_i915_write##x(dev_priv, reg, val); \
> @@ -883,7 +883,7 @@ static void \
>  hsw_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool 
> trace) { \
>   u32 __fifo_ret = 0; \
>   GEN6_WRITE_HEADER; \
> - if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
> + if (NEEDS_FORCE_WAKE(reg)) { \
>   __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
>   } \
>   hsw_unclaimed_reg_debug(dev_priv, reg, false, true); \
> @@ -985,7 +985,7 @@ gen9_write##x(struct drm_i915_private *dev_priv, off_t 
> reg, u##x val, \
>   enum forcewake_domains fw_engine; \
>   GEN6_WRITE_HEADER; \
>   hsw_unclaimed_reg_debug(dev_priv, reg, false, true); \
> - if (!SKL_NEEDS_FORCE_WAKE((dev_priv), (reg)) || \
> + if (!SKL_NEEDS_FORCE_WAKE(reg) || \
>   is_gen9_shadowed(dev_priv, reg)) \
>   fw_engine = 0; \
>   else if (FORCEWAKE_GEN9_RENDER_RANGE_OFFSET(reg)) \
> 

Reviewed-by: Jesse Barnes 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 32/43] drm/i915: Clean up LVDS register handling

2015-10-12 Thread Jesse Barnes
On 09/18/2015 10:03 AM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Keep single 'lvds_reg' and 'lvds' variable around in
> intel_lvds_init(), and read it just once at the start.
> 
> Also intel_lvds_get_config() doesn't need to figure out which reg to use
> since it can just consult lvds_encoder->reg.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_lvds.c | 30 ++
>  1 file changed, 14 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c 
> b/drivers/gpu/drm/i915/intel_lvds.c
> index 2c2d1f0..35bad71 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -98,15 +98,11 @@ static void intel_lvds_get_config(struct intel_encoder 
> *encoder,
>  {
>   struct drm_device *dev = encoder->base.dev;
>   struct drm_i915_private *dev_priv = dev->dev_private;
> - u32 lvds_reg, tmp, flags = 0;
> + struct intel_lvds_encoder *lvds_encoder = 
> to_lvds_encoder(&encoder->base);
> + u32 tmp, flags = 0;
>   int dotclock;
>  
> - if (HAS_PCH_SPLIT(dev))
> - lvds_reg = PCH_LVDS;
> - else
> - lvds_reg = LVDS;
> -
> - tmp = I915_READ(lvds_reg);
> + tmp = I915_READ(lvds_encoder->reg);
>   if (tmp & LVDS_HSYNC_POLARITY)
>   flags |= DRM_MODE_FLAG_NHSYNC;
>   else
> @@ -944,6 +940,7 @@ void intel_lvds_init(struct drm_device *dev)
>   struct drm_display_mode *downclock_mode = NULL;
>   struct edid *edid;
>   struct drm_crtc *crtc;
> + u32 lvds_reg;
>   u32 lvds;
>   int pipe;
>   u8 pin;
> @@ -966,8 +963,15 @@ void intel_lvds_init(struct drm_device *dev)
>   if (dmi_check_system(intel_no_lvds))
>   return;
>  
> + if (HAS_PCH_SPLIT(dev))
> + lvds_reg = PCH_LVDS;
> + else
> + lvds_reg = LVDS;
> +
> + lvds = I915_READ(lvds_reg);
> +
>   if (HAS_PCH_SPLIT(dev)) {
> - if ((I915_READ(PCH_LVDS) & LVDS_DETECTED) == 0)
> + if ((lvds & LVDS_DETECTED) == 0)
>   return;
>   if (dev_priv->vbt.edp_support) {
>   DRM_DEBUG_KMS("disable LVDS for eDP support\n");
> @@ -977,8 +981,7 @@ void intel_lvds_init(struct drm_device *dev)
>  
>   pin = GMBUS_PIN_PANEL;
>   if (!lvds_is_present_in_vbt(dev, &pin)) {
> - u32 reg = HAS_PCH_SPLIT(dev) ? PCH_LVDS : LVDS;
> - if ((I915_READ(reg) & LVDS_PORT_EN) == 0) {
> + if ((lvds & LVDS_PORT_EN) == 0) {
>   DRM_DEBUG_KMS("LVDS is not present in VBT\n");
>   return;
>   }
> @@ -1055,11 +1058,7 @@ void intel_lvds_init(struct drm_device *dev)
>   connector->interlace_allowed = false;
>   connector->doublescan_allowed = false;
>  
> - if (HAS_PCH_SPLIT(dev)) {
> - lvds_encoder->reg = PCH_LVDS;
> - } else {
> - lvds_encoder->reg = LVDS;
> - }
> + lvds_encoder->reg = lvds_reg;
>  
>   /* create the scaling mode property */
>   drm_mode_create_scaling_mode_property(dev);
> @@ -1140,7 +1139,6 @@ void intel_lvds_init(struct drm_device *dev)
>   if (HAS_PCH_SPLIT(dev))
>   goto failed;
>  
> - lvds = I915_READ(LVDS);
>   pipe = (lvds & LVDS_PIPEB_SELECT) ? 1 : 0;
>   crtc = intel_get_crtc_for_pipe(dev, pipe);
>  
> 

Reviewed-by: Jesse Barnes 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2 31/43] drm/i915: Throw out some useless variables

2015-10-12 Thread Jesse Barnes
On 09/22/2015 09:50 AM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Drop some useless 'reg' variables when we only use them once.
> 
> v2: A few more, including a few variable moves
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_irq.c  |  3 +-
>  drivers/gpu/drm/i915/intel_display.c | 75 
> +++-
>  drivers/gpu/drm/i915/intel_dp.c  | 10 ++---
>  3 files changed, 28 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 4b61a42..d181dab 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -674,9 +674,8 @@ static u32 i915_get_vblank_counter(struct drm_device 
> *dev, int pipe)
>  static u32 g4x_get_vblank_counter(struct drm_device *dev, int pipe)
>  {
>   struct drm_i915_private *dev_priv = dev->dev_private;
> - int reg = PIPE_FRMCOUNT_G4X(pipe);
>  
> - return I915_READ(reg);
> + return I915_READ(PIPE_FRMCOUNT_G4X(pipe));
>  }
>  
>  /* raw reads, only for fast reads of display block, no need for forcewake 
> etc. */
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 6f538d5..dc24e22 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1110,12 +1110,10 @@ static const char *state_string(bool enabled)
>  void assert_pll(struct drm_i915_private *dev_priv,
>   enum pipe pipe, bool state)
>  {
> - int reg;
>   u32 val;
>   bool cur_state;
>  
> - reg = DPLL(pipe);
> - val = I915_READ(reg);
> + val = I915_READ(DPLL(pipe));
>   cur_state = !!(val & DPLL_VCO_ENABLE);
>   I915_STATE_WARN(cur_state != state,
>"PLL state assertion failure (expected %s, current %s)\n",
> @@ -1172,20 +1170,16 @@ void assert_shared_dpll(struct drm_i915_private 
> *dev_priv,
>  static void assert_fdi_tx(struct drm_i915_private *dev_priv,
> enum pipe pipe, bool state)
>  {
> - int reg;
> - u32 val;
>   bool cur_state;
>   enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
> pipe);
>  
>   if (HAS_DDI(dev_priv->dev)) {
>   /* DDI does not have a specific FDI_TX register */
> - reg = TRANS_DDI_FUNC_CTL(cpu_transcoder);
> - val = I915_READ(reg);
> + u32 val = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
>   cur_state = !!(val & TRANS_DDI_FUNC_ENABLE);
>   } else {
> - reg = FDI_TX_CTL(pipe);
> - val = I915_READ(reg);
> + u32 val = I915_READ(FDI_TX_CTL(pipe));
>   cur_state = !!(val & FDI_TX_ENABLE);
>   }
>   I915_STATE_WARN(cur_state != state,
> @@ -1198,12 +1192,10 @@ static void assert_fdi_tx(struct drm_i915_private 
> *dev_priv,
>  static void assert_fdi_rx(struct drm_i915_private *dev_priv,
> enum pipe pipe, bool state)
>  {
> - int reg;
>   u32 val;
>   bool cur_state;
>  
> - reg = FDI_RX_CTL(pipe);
> - val = I915_READ(reg);
> + val = I915_READ(FDI_RX_CTL(pipe));
>   cur_state = !!(val & FDI_RX_ENABLE);
>   I915_STATE_WARN(cur_state != state,
>"FDI RX state assertion failure (expected %s, current %s)\n",
> @@ -1215,7 +1207,6 @@ static void assert_fdi_rx(struct drm_i915_private 
> *dev_priv,
>  static void assert_fdi_tx_pll_enabled(struct drm_i915_private *dev_priv,
> enum pipe pipe)
>  {
> - int reg;
>   u32 val;
>  
>   /* ILK FDI PLL is always enabled */
> @@ -1226,20 +1217,17 @@ static void assert_fdi_tx_pll_enabled(struct 
> drm_i915_private *dev_priv,
>   if (HAS_DDI(dev_priv->dev))
>   return;
>  
> - reg = FDI_TX_CTL(pipe);
> - val = I915_READ(reg);
> + val = I915_READ(FDI_TX_CTL(pipe));
>   I915_STATE_WARN(!(val & FDI_TX_PLL_ENABLE), "FDI TX PLL assertion 
> failure, should be active but is disabled\n");
>  }
>  
>  void assert_fdi_rx_pll(struct drm_i915_private *dev_priv,
>  enum pipe pipe, bool state)
>  {
> - int reg;
>   u32 val;
>   bool cur_state;
>  
> - reg = FDI_RX_CTL(pipe);
> - val = I915_READ(reg);
> + val = I915_READ(FDI_RX_CTL(pipe));
>   cur_state = !!(val & FDI_RX_PLL_ENABLE);
>   I915_STATE_WARN(cur_state != state,
>"FDI RX PLL assertion failure (expected %s, current %s)\n",
> @@ -1309,8 +1297,6 @@ static void assert_cursor(struct drm_i915_private 
> *dev_priv,
>  void assert_pipe(struct drm_i915_private *dev_priv,
>enum pipe pipe, bool state)
>  {
> - int reg;
> - u32 val;
>   bool cur_state;
>   enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
> pipe);
> @@ -1324,8 +1310,7 

Re: [Intel-gfx] [PATCH 30/43] drm/i915: Parametrize and fix SWF registers

2015-10-12 Thread Jesse Barnes
On 09/18/2015 10:03 AM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Parametrize the SWF registers. This also fixes the register offsets,
> which were mostly garbage in the old defines.
> 
> Also save/restore only as many SWF registers that each platform has.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  2 +-
>  drivers/gpu/drm/i915/i915_reg.h | 28 +++
>  drivers/gpu/drm/i915/i915_suspend.c | 45 
> -
>  3 files changed, 50 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 3bf8a9b..3e35e08 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1041,7 +1041,7 @@ struct i915_suspend_saved_registers {
>   u32 saveMI_ARB_STATE;
>   u32 saveSWF0[16];
>   u32 saveSWF1[16];
> - u32 saveSWF2[3];
> + u32 saveSWF3[3];
>   uint64_t saveFENCE[I915_MAX_NUM_FENCES];
>   u32 savePCH_PORT_HOTPLUG;
>   u16 saveGCDGMBUS;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0cc41e4b..57b9469 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4941,20 +4941,20 @@ enum skl_disp_power_wells {
>  #define I915_LO_DISPBASE(val)(val & ~DISP_BASEADDR_MASK)
>  #define I915_HI_DISPBASE(val)(val & DISP_BASEADDR_MASK)
>  
> -/* VBIOS flags */
> -#define SWF00(dev_priv->info.display_mmio_offset + 
> 0x71410)
> -#define SWF01(dev_priv->info.display_mmio_offset + 
> 0x71414)
> -#define SWF02(dev_priv->info.display_mmio_offset + 
> 0x71418)
> -#define SWF03(dev_priv->info.display_mmio_offset + 
> 0x7141c)
> -#define SWF04(dev_priv->info.display_mmio_offset + 
> 0x71420)
> -#define SWF05(dev_priv->info.display_mmio_offset + 
> 0x71424)
> -#define SWF06(dev_priv->info.display_mmio_offset + 
> 0x71428)
> -#define SWF10(dev_priv->info.display_mmio_offset + 
> 0x70410)
> -#define SWF11(dev_priv->info.display_mmio_offset + 
> 0x70414)
> -#define SWF14(dev_priv->info.display_mmio_offset + 
> 0x71420)
> -#define SWF30(dev_priv->info.display_mmio_offset + 
> 0x72414)
> -#define SWF31(dev_priv->info.display_mmio_offset + 
> 0x72418)
> -#define SWF32(dev_priv->info.display_mmio_offset + 
> 0x7241c)
> +/*
> + * VBIOS flags
> + * gen2:
> + * [00:06] alm,mgm
> + * [10:16] all
> + * [30:32] alm,mgm
> + * gen3+:
> + * [00:0f] all
> + * [10:1f] all
> + * [30:32] all
> + */
> +#define SWF0(i)  (dev_priv->info.display_mmio_offset + 0x70410 + (i) * 4)
> +#define SWF1(i)  (dev_priv->info.display_mmio_offset + 0x71410 + (i) * 4)
> +#define SWF3(i)  (dev_priv->info.display_mmio_offset + 0x72414 + (i) * 4)
>  
>  /* Pipe B */
>  #define _PIPEBDSL(dev_priv->info.display_mmio_offset + 0x71000)
> diff --git a/drivers/gpu/drm/i915/i915_suspend.c 
> b/drivers/gpu/drm/i915/i915_suspend.c
> index 1ccac61..2d91821 100644
> --- a/drivers/gpu/drm/i915/i915_suspend.c
> +++ b/drivers/gpu/drm/i915/i915_suspend.c
> @@ -122,12 +122,24 @@ int i915_save_state(struct drm_device *dev)
>   dev_priv->regfile.saveMI_ARB_STATE = I915_READ(MI_ARB_STATE);
>  
>   /* Scratch space */
> - for (i = 0; i < 16; i++) {
> - dev_priv->regfile.saveSWF0[i] = I915_READ(SWF00 + (i << 2));
> - dev_priv->regfile.saveSWF1[i] = I915_READ(SWF10 + (i << 2));
> + if (IS_GEN2(dev_priv) && IS_MOBILE(dev_priv)) {
> + for (i = 0; i < 7; i++) {
> + dev_priv->regfile.saveSWF0[i] = I915_READ(SWF0(i));
> + dev_priv->regfile.saveSWF1[i] = I915_READ(SWF1(i));
> + }
> + for (i = 0; i < 3; i++)
> + dev_priv->regfile.saveSWF3[i] = I915_READ(SWF3(i));
> + } else if (IS_GEN2(dev_priv)) {
> + for (i = 0; i < 7; i++)
> + dev_priv->regfile.saveSWF1[i] = I915_READ(SWF1(i));
> + } else if (HAS_GMCH_DISPLAY(dev_priv)) {
> + for (i = 0; i < 16; i++) {
> + dev_priv->regfile.saveSWF0[i] = I915_READ(SWF0(i));
> + dev_priv->regfile.saveSWF1[i] = I915_READ(SWF1(i));
> + }
> + for (i = 0; i < 3; i++)
> + dev_priv->regfile.saveSWF3[i] = I915_READ(SWF3(i));
>   }
> - for (i = 0; i < 3; i++)
> - dev_priv->regfile.saveSWF2[i] = I915_READ(SWF30 + (i << 2));
>  
>   mutex_unlock(&dev->struct_mutex);
>  
> @@ -156,12 +168,25 @@ int i915_restore_state(struct drm_device *dev)
>   /* Memory arbitration state */
>   I915_WRITE(MI_ARB_STATE, dev_priv->regfile.saveMI_ARB_STA

Re: [Intel-gfx] [PATCH 29/43] drm/i915: s/PIPE_FRMCOUNT_GM45/PIPE_FRMCOUNT_G4X/ etc.

2015-10-12 Thread Jesse Barnes
On 09/18/2015 10:03 AM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> The PIPE_FRMCOUNT_GM45 and PIPE_FLIPCOUNT_GM45 names have bothered me
> for a long time. The work equally well for ELK and onwards, so let's
> s/GM45/G4X/.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_irq.c  |  6 +++---
>  drivers/gpu/drm/i915/i915_reg.h  | 12 ++--
>  drivers/gpu/drm/i915/intel_display.c |  4 ++--
>  3 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 24f68de..4b61a42 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -671,10 +671,10 @@ static u32 i915_get_vblank_counter(struct drm_device 
> *dev, int pipe)
>   return (((high1 << 8) | low) + (pixel >= vbl_start)) & 0xff;
>  }
>  
> -static u32 gm45_get_vblank_counter(struct drm_device *dev, int pipe)
> +static u32 g4x_get_vblank_counter(struct drm_device *dev, int pipe)
>  {
>   struct drm_i915_private *dev_priv = dev->dev_private;
> - int reg = PIPE_FRMCOUNT_GM45(pipe);
> + int reg = PIPE_FRMCOUNT_G4X(pipe);
>  
>   return I915_READ(reg);
>  }
> @@ -4311,7 +4311,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
>   dev->driver->get_vblank_counter = i8xx_get_vblank_counter;
>   } else if (IS_G4X(dev_priv) || INTEL_INFO(dev_priv)->gen >= 5) {
>   dev->max_vblank_count = 0x; /* full 32 bit counter */
> - dev->driver->get_vblank_counter = gm45_get_vblank_counter;
> + dev->driver->get_vblank_counter = g4x_get_vblank_counter;
>   } else {
>   dev->driver->get_vblank_counter = i915_get_vblank_counter;
>   dev->max_vblank_count = 0xff; /* only 24 bits of frame 
> count */
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 02f0935..0cc41e4b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4796,10 +4796,10 @@ enum skl_disp_power_wells {
>  #define   PIPE_PIXEL_MASK 0x00ff
>  #define   PIPE_PIXEL_SHIFT0
>  /* GM45+ just has to be different */
> -#define _PIPEA_FRMCOUNT_GM45 0x70040
> -#define _PIPEA_FLIPCOUNT_GM450x70044
> -#define PIPE_FRMCOUNT_GM45(pipe) _PIPE2(pipe, _PIPEA_FRMCOUNT_GM45)
> -#define PIPE_FLIPCOUNT_GM45(pipe) _PIPE2(pipe, _PIPEA_FLIPCOUNT_GM45)
> +#define _PIPEA_FRMCOUNT_G4X  0x70040
> +#define _PIPEA_FLIPCOUNT_G4X 0x70044
> +#define PIPE_FRMCOUNT_G4X(pipe) _PIPE2(pipe, _PIPEA_FRMCOUNT_G4X)
> +#define PIPE_FLIPCOUNT_G4X(pipe) _PIPE2(pipe, _PIPEA_FLIPCOUNT_G4X)
>  
>  /* Cursor A & B regs */
>  #define _CURACNTR0x70080
> @@ -4962,8 +4962,8 @@ enum skl_disp_power_wells {
>  #define _PIPEBSTAT   (dev_priv->info.display_mmio_offset + 0x71024)
>  #define _PIPEBFRAMEHIGH  0x71040
>  #define _PIPEBFRAMEPIXEL 0x71044
> -#define _PIPEB_FRMCOUNT_GM45 (dev_priv->info.display_mmio_offset + 0x71040)
> -#define _PIPEB_FLIPCOUNT_GM45(dev_priv->info.display_mmio_offset + 
> 0x71044)
> +#define _PIPEB_FRMCOUNT_G4X  (dev_priv->info.display_mmio_offset + 0x71040)
> +#define _PIPEB_FLIPCOUNT_G4X (dev_priv->info.display_mmio_offset + 0x71044)
>  
>  
>  /* Display B control */
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 92e624b..0074781 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10769,7 +10769,7 @@ static bool page_flip_finished(struct intel_crtc 
> *crtc)
>*/
>   return (I915_READ(DSPSURFLIVE(crtc->plane)) & ~0xfff) ==
>   crtc->unpin_work->gtt_offset &&
> - 
> g4x_flip_count_after_eq(I915_READ(PIPE_FLIPCOUNT_GM45(crtc->pipe)),
> + 
> g4x_flip_count_after_eq(I915_READ(PIPE_FLIPCOUNT_G4X(crtc->pipe)),
>   crtc->unpin_work->flip_count);
>  }
>  
> @@ -11374,7 +11374,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>   intel_crtc->reset_counter = 
> atomic_read(&dev_priv->gpu_error.reset_counter);
>  
>   if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev))
> - work->flip_count = I915_READ(PIPE_FLIPCOUNT_GM45(pipe)) + 1;
> + work->flip_count = I915_READ(PIPE_FLIPCOUNT_G4X(pipe)) + 1;
>  
>   if (IS_VALLEYVIEW(dev)) {
>   ring = &dev_priv->ring[BCS];
> 

Reviewed-by: Jesse Barnes 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 28/43] drm/i915: Turn GEN5_ASSERT_IIR_IS_ZERO() into a function

2015-10-12 Thread Jesse Barnes
On 09/18/2015 10:03 AM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 31 +--
>  1 file changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 16948b2..24f68de 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -139,27 +139,30 @@ static const u32 hpd_bxt[HPD_NUM_PINS] = {
>  /*
>   * We should clear IMR at preinstall/uninstall, and just check at 
> postinstall.
>   */
> -#define GEN5_ASSERT_IIR_IS_ZERO(reg) do { \
> - u32 val = I915_READ(reg); \
> - if (val) { \
> - WARN(1, "Interrupt register 0x%x is not zero: 0x%08x\n", \
> -  (reg), val); \
> - I915_WRITE((reg), 0x); \
> - POSTING_READ(reg); \
> - I915_WRITE((reg), 0x); \
> - POSTING_READ(reg); \
> - } \
> -} while (0)
> +static void gen5_assert_iir_is_zero(struct drm_i915_private *dev_priv, u32 
> reg)
> +{
> + u32 val = I915_READ(reg);
> +
> + if (val == 0)
> + return;
> +
> + WARN(1, "Interrupt register 0x%x is not zero: 0x%08x\n",
> +  reg, val);
> + I915_WRITE(reg, 0x);
> + POSTING_READ(reg);
> + I915_WRITE(reg, 0x);
> + POSTING_READ(reg);
> +}
>  
>  #define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) do { \
> - GEN5_ASSERT_IIR_IS_ZERO(GEN8_##type##_IIR(which)); \
> + gen5_assert_iir_is_zero(dev_priv, GEN8_##type##_IIR(which)); \
>   I915_WRITE(GEN8_##type##_IER(which), (ier_val)); \
>   I915_WRITE(GEN8_##type##_IMR(which), (imr_val)); \
>   POSTING_READ(GEN8_##type##_IMR(which)); \
>  } while (0)
>  
>  #define GEN5_IRQ_INIT(type, imr_val, ier_val) do { \
> - GEN5_ASSERT_IIR_IS_ZERO(type##IIR); \
> + gen5_assert_iir_is_zero(dev_priv, type##IIR); \
>   I915_WRITE(type##IER, (ier_val)); \
>   I915_WRITE(type##IMR, (imr_val)); \
>   POSTING_READ(type##IMR); \
> @@ -3276,7 +3279,7 @@ static void ibx_irq_postinstall(struct drm_device *dev)
>   else
>   mask = SDE_GMBUS_CPT | SDE_AUX_MASK_CPT;
>  
> - GEN5_ASSERT_IIR_IS_ZERO(SDEIIR);
> + gen5_assert_iir_is_zero(dev_priv, SDEIIR);
>   I915_WRITE(SDEIMR, ~mask);
>  }
>  
> 

Reviewed-by: Jesse Barnes 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 27/43] drm/i915: Fix a few bad hex numbers in register defines

2015-10-12 Thread Jesse Barnes
On 09/18/2015 10:03 AM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> A few register mask defines were missing the '0x' from hex numbers. Or
> at least I assume those were meant to be hex numbers. Put the '0x' in
> place.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 21d49e7..02f0935 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4234,7 +4234,7 @@ enum skl_disp_power_wells {
>  #define   DP_AUX_CH_CTL_PSR_DATA_AUX_REG_SKL (1 << 14)
>  #define   DP_AUX_CH_CTL_FS_DATA_AUX_REG_SKL  (1 << 13)
>  #define   DP_AUX_CH_CTL_GTC_DATA_AUX_REG_SKL (1 << 12)
> -#define   DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL_MASK (1f << 5)
> +#define   DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL_MASK (0x1f << 5)
>  #define   DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(c) (((c) - 1) << 5)
>  #define   DP_AUX_CH_CTL_SYNC_PULSE_SKL(c)   ((c) - 1)
>  
> @@ -7819,7 +7819,7 @@ enum skl_disp_power_wells {
>  #define  VIRTUAL_CHANNEL_SHIFT   6
>  #define  VIRTUAL_CHANNEL_MASK(3 << 6)
>  #define  DATA_TYPE_SHIFT 0
> -#define  DATA_TYPE_MASK  (3f << 0)
> +#define  DATA_TYPE_MASK  (0x3f << 0)
>  /* data type values, see include/video/mipi_display.h */
>  
>  #define _MIPIA_GEN_FIFO_STAT (dev_priv->mipi_mmio_base + 0xb074)
> 

Hah!  Maybe they're supposed to be floats though!  We should use more
floats in masks in general I believe.

Reviewed-by: Jesse Barnes 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 26/43] drm/i915: Protect register macro arguments

2015-10-12 Thread Jesse Barnes
On 09/18/2015 10:03 AM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Always put parens around macro argument evaluations.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 92 
> -
>  1 file changed, 46 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0216771..21d49e7 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -429,7 +429,7 @@
>  #define   ASYNC_FLIP(1<<22)
>  #define   DISPLAY_PLANE_A   (0<<20)
>  #define   DISPLAY_PLANE_B   (1<<20)
> -#define GFX_OP_PIPE_CONTROL(len) ((0x3<<29)|(0x3<<27)|(0x2<<24)|(len-2))
> +#define GFX_OP_PIPE_CONTROL(len) 
> ((0x3<<29)|(0x3<<27)|(0x2<<24)|((len)-2))
>  #define   PIPE_CONTROL_FLUSH_L3  (1<<27)
>  #define   PIPE_CONTROL_GLOBAL_GTT_IVB(1<<24) /* 
> gen7+ */
>  #define   PIPE_CONTROL_MMIO_WRITE(1<<23)
> @@ -1250,7 +1250,7 @@ enum skl_disp_power_wells {
>  #define  PORT_PLL_DCO_AMP_OVR_EN_H   (1<<27)
>  #define  PORT_PLL_DCO_AMP_DEFAULT15
>  #define  PORT_PLL_DCO_AMP_MASK   0x3c00
> -#define  PORT_PLL_DCO_AMP(x) (x<<10)
> +#define  PORT_PLL_DCO_AMP(x) ((x)<<10)
>  #define _PORT_PLL_BASE(port) _PORT3(port, _PORT_PLL_0_A, \
>   _PORT_PLL_0_B,  \
>   _PORT_PLL_0_C)
> @@ -1540,8 +1540,8 @@ enum skl_disp_power_wells {
>  #define RENDER_HWS_PGA_GEN7  (0x04080)
>  #define RING_FAULT_REG(ring) (0x4094 + 0x100*(ring)->id)
>  #define   RING_FAULT_GTTSEL_MASK (1<<11)
> -#define   RING_FAULT_SRCID(x)((x >> 3) & 0xff)
> -#define   RING_FAULT_FAULT_TYPE(x) ((x >> 1) & 0x3)
> +#define   RING_FAULT_SRCID(x)(((x) >> 3) & 0xff)
> +#define   RING_FAULT_FAULT_TYPE(x) (((x) >> 1) & 0x3)
>  #define   RING_FAULT_VALID   (1<<0)
>  #define DONE_REG 0x40b0
>  #define GEN8_PRIVATE_PAT_LO  0x40e0
> @@ -1626,9 +1626,9 @@ enum skl_disp_power_wells {
>  #define   ERR_INT_PIPE_CRC_DONE_B(1<<5)
>  #define   ERR_INT_FIFO_UNDERRUN_B(1<<3)
>  #define   ERR_INT_PIPE_CRC_DONE_A(1<<2)
> -#define   ERR_INT_PIPE_CRC_DONE(pipe)(1<<(2 + pipe*3))
> +#define   ERR_INT_PIPE_CRC_DONE(pipe)(1<<(2 + (pipe)*3))
>  #define   ERR_INT_FIFO_UNDERRUN_A(1<<0)
> -#define   ERR_INT_FIFO_UNDERRUN(pipe)(1<<(pipe*3))
> +#define   ERR_INT_FIFO_UNDERRUN(pipe)(1<<((pipe)*3))
>  
>  #define GEN8_FAULT_TLB_DATA0 0x04b10
>  #define GEN8_FAULT_TLB_DATA1 0x04b14
> @@ -1689,8 +1689,8 @@ enum skl_disp_power_wells {
>  #define   GEN6_WIZ_HASHING_16x4  
> GEN6_WIZ_HASHING(1, 0)
>  #define   GEN6_WIZ_HASHING_MASK  
> GEN6_WIZ_HASHING(1, 1)
>  #define   GEN6_TD_FOUR_ROW_DISPATCH_DISABLE  (1 << 5)
> -#define   GEN9_IZ_HASHING_MASK(slice)(0x3 << (slice 
> * 2))
> -#define   GEN9_IZ_HASHING(slice, val)((val) << 
> (slice * 2))
> +#define   GEN9_IZ_HASHING_MASK(slice)(0x3 << 
> ((slice) * 2))
> +#define   GEN9_IZ_HASHING(slice, val)((val) << 
> ((slice) * 2))
>  
>  #define GFX_MODE 0x02520
>  #define GFX_MODE_GEN70x0229c
> @@ -2828,21 +2828,21 @@ enum skl_disp_power_wells {
>   *   doesn't need saving on GT1
>   */
>  #define CXT_SIZE 0x21a0
> -#define GEN6_CXT_POWER_SIZE(cxt_reg) ((cxt_reg >> 24) & 0x3f)
> -#define GEN6_CXT_RING_SIZE(cxt_reg)  ((cxt_reg >> 18) & 0x3f)
> -#define GEN6_CXT_RENDER_SIZE(cxt_reg)((cxt_reg >> 12) & 0x3f)
> -#define GEN6_CXT_EXTENDED_SIZE(cxt_reg)  ((cxt_reg >> 6) & 0x3f)
> -#define GEN6_CXT_PIPELINE_SIZE(cxt_reg)  ((cxt_reg >> 0) & 0x3f)
> +#define GEN6_CXT_POWER_SIZE(cxt_reg) (((cxt_reg) >> 24) & 0x3f)
> +#define GEN6_CXT_RING_SIZE(cxt_reg)  (((cxt_reg) >> 18) & 0x3f)
> +#define GEN6_CXT_RENDER_SIZE(cxt_reg)(((cxt_reg) >> 12) & 0x3f)
> +#define GEN6_CXT_EXTENDED_SIZE(cxt_reg)  (((cxt_reg) >> 6) & 0x3f)
> +#define GEN6_CXT_PIPELINE_SIZE(cxt_reg)  (((cxt_reg) >> 0) & 0x3f)
>  #define GEN6_CXT_TOTAL_SIZE(cxt_reg) (GEN6_CXT_RING_SIZE(cxt_reg) + \
>   GEN6_CXT_EXTENDED_SIZE(cxt_reg) + \
>   GEN6_CXT_PIPELINE_SIZE(cxt_reg))
>  #define GEN7_CXT_SIZE0x21a8
> -#define GEN7_CXT_POWER_SIZE(ctx_reg) ((ctx_reg >> 25) & 0x7f)
> -#define GEN7_CXT_RING_SIZE(ctx_reg)  ((ctx_reg >> 22) & 0x7)
> -#define GEN7_CXT_RENDER_SIZE(ctx_reg)((ctx_reg >> 16) & 0x3f)
> -#define GEN7_CXT_EXTENDED_SIZE(ctx_reg)  ((ctx_reg >> 9) & 0x7f)
> -#define GEN7_CXT_GT1_SIZE(ctx_reg)   ((ctx_reg >> 6) & 0x7)
> -#define GEN7_CXT_VFSTATE_SIZE(ctx_reg)   ((ctx_reg >> 0) & 0x3f)
> +#define GEN7_CXT_POWER_SIZ

Re: [Intel-gfx] [PATCH 25/43] drm/i915: Include gpio_mmio_base in GMBUS reg defines

2015-10-12 Thread Jesse Barnes
On 09/18/2015 10:03 AM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_reg.h  | 12 -
>  drivers/gpu/drm/i915/intel_i2c.c | 54 
> +---
>  2 files changed, 29 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b35e24f..0216771 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2114,7 +2114,7 @@ enum skl_disp_power_wells {
>  # define GPIO_DATA_VAL_IN(1 << 12)
>  # define GPIO_DATA_PULLUP_DISABLE(1 << 13)
>  
> -#define GMBUS0   0x5100 /* clock/port select */
> +#define GMBUS0   (dev_priv->gpio_mmio_base + 0x5100) /* 
> clock/port select */
>  #define   GMBUS_RATE_100KHZ  (0<<8)
>  #define   GMBUS_RATE_50KHZ   (1<<8)
>  #define   GMBUS_RATE_400KHZ  (2<<8) /* reserved on Pineview */
> @@ -2133,7 +2133,7 @@ enum skl_disp_power_wells {
>  #define   GMBUS_PIN_2_BXT2
>  #define   GMBUS_PIN_3_BXT3
>  #define   GMBUS_NUM_PINS 7 /* including 0 */
> -#define GMBUS1   0x5104 /* command/status */
> +#define GMBUS1   (dev_priv->gpio_mmio_base + 0x5104) /* 
> command/status */
>  #define   GMBUS_SW_CLR_INT   (1<<31)
>  #define   GMBUS_SW_RDY   (1<<30)
>  #define   GMBUS_ENT  (1<<29) /* enable timeout */
> @@ -2147,7 +2147,7 @@ enum skl_disp_power_wells {
>  #define   GMBUS_SLAVE_ADDR_SHIFT 1
>  #define   GMBUS_SLAVE_READ   (1<<0)
>  #define   GMBUS_SLAVE_WRITE  (0<<0)
> -#define GMBUS2   0x5108 /* status */
> +#define GMBUS2   (dev_priv->gpio_mmio_base + 0x5108) /* 
> status */
>  #define   GMBUS_INUSE(1<<15)
>  #define   GMBUS_HW_WAIT_PHASE(1<<14)
>  #define   GMBUS_STALL_TIMEOUT(1<<13)
> @@ -2155,14 +2155,14 @@ enum skl_disp_power_wells {
>  #define   GMBUS_HW_RDY   (1<<11)
>  #define   GMBUS_SATOER   (1<<10)
>  #define   GMBUS_ACTIVE   (1<<9)
> -#define GMBUS3   0x510c /* data buffer bytes 3-0 */
> -#define GMBUS4   0x5110 /* interrupt mask (Pineview+) */
> +#define GMBUS3   (dev_priv->gpio_mmio_base + 0x510c) /* 
> data buffer bytes 3-0 */
> +#define GMBUS4   (dev_priv->gpio_mmio_base + 0x5110) /* 
> interrupt mask (Pineview+) */
>  #define   GMBUS_SLAVE_TIMEOUT_EN (1<<4)
>  #define   GMBUS_NAK_EN   (1<<3)
>  #define   GMBUS_IDLE_EN  (1<<2)
>  #define   GMBUS_HW_WAIT_EN   (1<<1)
>  #define   GMBUS_HW_RDY_EN(1<<0)
> -#define GMBUS5   0x5120 /* byte index */
> +#define GMBUS5   (dev_priv->gpio_mmio_base + 0x5120) /* 
> byte index */
>  #define   GMBUS_2BYTE_INDEX_EN   (1<<31)
>  
>  /*
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c 
> b/drivers/gpu/drm/i915/intel_i2c.c
> index a64f26c..1369fc4 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -114,8 +114,8 @@ intel_i2c_reset(struct drm_device *dev)
>  {
>   struct drm_i915_private *dev_priv = dev->dev_private;
>  
> - I915_WRITE(dev_priv->gpio_mmio_base + GMBUS0, 0);
> - I915_WRITE(dev_priv->gpio_mmio_base + GMBUS4, 0);
> + I915_WRITE(GMBUS0, 0);
> + I915_WRITE(GMBUS4, 0);
>  }
>  
>  static void intel_i2c_quirk_set(struct drm_i915_private *dev_priv, bool 
> enable)
> @@ -261,7 +261,6 @@ gmbus_wait_hw_status(struct drm_i915_private *dev_priv,
>u32 gmbus4_irq_en)
>  {
>   int i;
> - int reg_offset = dev_priv->gpio_mmio_base;
>   u32 gmbus2 = 0;
>   DEFINE_WAIT(wait);
>  
> @@ -271,13 +270,13 @@ gmbus_wait_hw_status(struct drm_i915_private *dev_priv,
>   /* Important: The hw handles only the first bit, so set only one! Since
>* we also need to check for NAKs besides the hw ready/idle signal, we
>* need to wake up periodically and check that ourselves. */
> - I915_WRITE(GMBUS4 + reg_offset, gmbus4_irq_en);
> + I915_WRITE(GMBUS4, gmbus4_irq_en);
>  
>   for (i = 0; i < msecs_to_jiffies_timeout(50); i++) {
>   prepare_to_wait(&dev_priv->gmbus_wait_queue, &wait,
>   TASK_UNINTERRUPTIBLE);
>  
> - gmbus2 = I915_READ_NOTRACE(GMBUS2 + reg_offset);
> + gmbus2 = I915_READ_NOTRACE(GMBUS2);
>   if (gmbus2 & (GMBUS_SATOER | gmbus2_status))
>   break;
>  
> @@ -285,7 +284,7 @@ gmbus_wait_hw_status(struct drm_i915_private *dev_priv,
>   }
>   finish_wait(&dev_priv->gmbus_wait_queue, &wait);
>  
> - I915_WRITE(GMBUS4 + reg_offset, 0);
> + I915_WRITE(GMBUS4, 0);
>  
>   if (gmbus2 & GMBUS_SATOER)
>   return -ENXIO;
> @@ -298,20 +297,19 @@ static int
>  gmbus_wait_idle(struct drm_i915_private *dev_priv)
>  {
>  

Re: [Intel-gfx] [PATCH 24/43] drm/i915: Parametrize HSW video DIP data registers

2015-10-12 Thread Jesse Barnes
On 09/18/2015 10:03 AM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_reg.h   | 16 
>  drivers/gpu/drm/i915/intel_hdmi.c | 26 ++
>  drivers/gpu/drm/i915/intel_psr.c  | 18 ++
>  3 files changed, 32 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 134b075..b35e24f 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6236,16 +6236,16 @@ enum skl_disp_power_wells {
>  
>  #define HSW_TVIDEO_DIP_CTL(trans) \
>_TRANSCODER2(trans, HSW_VIDEO_DIP_CTL_A)
> -#define HSW_TVIDEO_DIP_AVI_DATA(trans) \
> -  _TRANSCODER2(trans, HSW_VIDEO_DIP_AVI_DATA_A)
> -#define HSW_TVIDEO_DIP_VS_DATA(trans) \
> -  _TRANSCODER2(trans, HSW_VIDEO_DIP_VS_DATA_A)
> -#define HSW_TVIDEO_DIP_SPD_DATA(trans) \
> -  _TRANSCODER2(trans, HSW_VIDEO_DIP_SPD_DATA_A)
> +#define HSW_TVIDEO_DIP_AVI_DATA(trans, i) \
> + (_TRANSCODER2(trans, HSW_VIDEO_DIP_AVI_DATA_A) + (i) * 4)
> +#define HSW_TVIDEO_DIP_VS_DATA(trans, i) \
> + (_TRANSCODER2(trans, HSW_VIDEO_DIP_VS_DATA_A) + (i) * 4)
> +#define HSW_TVIDEO_DIP_SPD_DATA(trans, i) \
> + (_TRANSCODER2(trans, HSW_VIDEO_DIP_SPD_DATA_A) + (i) * 4)
>  #define HSW_TVIDEO_DIP_GCP(trans) \
>   _TRANSCODER2(trans, HSW_VIDEO_DIP_GCP_A)
> -#define HSW_TVIDEO_DIP_VSC_DATA(trans) \
> -  _TRANSCODER2(trans, HSW_VIDEO_DIP_VSC_DATA_A)
> +#define HSW_TVIDEO_DIP_VSC_DATA(trans, i) \
> + (_TRANSCODER2(trans, HSW_VIDEO_DIP_VSC_DATA_A) + (i) * 4)
>  
>  #define HSW_STEREO_3D_CTL_A  0x70020
>  #define   S3D_ENABLE (1<<31)
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index e978c59..6b16292 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -113,17 +113,18 @@ static u32 hsw_infoframe_enable(enum 
> hdmi_infoframe_type type)
>   }
>  }
>  
> -static u32 hsw_infoframe_data_reg(enum hdmi_infoframe_type type,
> -   enum transcoder cpu_transcoder,
> -   struct drm_i915_private *dev_priv)
> +static u32 hsw_dip_data_reg(struct drm_i915_private *dev_priv,
> + enum transcoder cpu_transcoder,
> + enum hdmi_infoframe_type type,
> + int i)
>  {
>   switch (type) {
>   case HDMI_INFOFRAME_TYPE_AVI:
> - return HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder);
> + return HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder, i);
>   case HDMI_INFOFRAME_TYPE_SPD:
> - return HSW_TVIDEO_DIP_SPD_DATA(cpu_transcoder);
> + return HSW_TVIDEO_DIP_SPD_DATA(cpu_transcoder, i);
>   case HDMI_INFOFRAME_TYPE_VENDOR:
> - return HSW_TVIDEO_DIP_VS_DATA(cpu_transcoder);
> + return HSW_TVIDEO_DIP_VS_DATA(cpu_transcoder, i);
>   default:
>   DRM_DEBUG_DRIVER("unknown info frame type %d\n", type);
>   return 0;
> @@ -365,14 +366,13 @@ static void hsw_write_infoframe(struct drm_encoder 
> *encoder,
>   struct drm_device *dev = encoder->dev;
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> - u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->config->cpu_transcoder);
> + enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> + u32 ctl_reg = HSW_TVIDEO_DIP_CTL(cpu_transcoder);
>   u32 data_reg;
>   int i;
>   u32 val = I915_READ(ctl_reg);
>  
> - data_reg = hsw_infoframe_data_reg(type,
> -   intel_crtc->config->cpu_transcoder,
> -   dev_priv);
> + data_reg = hsw_dip_data_reg(dev_priv, cpu_transcoder, type, 0);
>   if (data_reg == 0)
>   return;
>  
> @@ -381,12 +381,14 @@ static void hsw_write_infoframe(struct drm_encoder 
> *encoder,
>  
>   mmiowb();
>   for (i = 0; i < len; i += 4) {
> - I915_WRITE(data_reg + i, *data);
> + I915_WRITE(hsw_dip_data_reg(dev_priv, cpu_transcoder,
> + type, i >> 2), *data);
>   data++;
>   }
>   /* Write every possible data byte to force correct ECC calculation. */
>   for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
> - I915_WRITE(data_reg + i, 0);
> + I915_WRITE(hsw_dip_data_reg(dev_priv, cpu_transcoder,
> + type, i >> 2), 0);
>   mmiowb();
>  
>   val |= hsw_infoframe_enable(type);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c 
> b/drivers/gpu/drm/i915/intel_psr.c
> index a04b4dc..213581c 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -73,14 +73,14 @@ static bool vlv_is_psr_active_on_pipe(struct drm_device 
>

[Intel-gfx] [PATCH] drm/i915: Disable DC6 for now.

2015-10-12 Thread Rodrigo Vivi
There is an intermitent RAM corruption happening with DMC
micro-controler when in DC6 transitioning to PC9/10. So
the recoomendation is to use DC5 as the deeper DC state
for now until this issue is being investigated at firmware
level.

This macros must be re-worked in order to allow us to use
module parameters to allow max DC states. However let's do
this simple approach first before products out there start
facing this corruption. Also this is the easiest one to be
backported by products.

Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index ec010ee..7332cc0 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -49,8 +49,8 @@
  * present for a given platform.
  */
 
-#define GEN9_ENABLE_DC5(dev) 0
-#define SKL_ENABLE_DC6(dev) IS_SKYLAKE(dev)
+#define GEN9_ENABLE_DC5(dev) IS_SKYLAKE(dev)
+#define SKL_ENABLE_DC6(dev) 0
 
 #define for_each_power_well(i, power_well, domain_mask, power_domains) \
for (i = 0; \
-- 
2.4.3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [SKL-DMC-BUGFIX 3/5] drm/i915/skl: Do not disable cdclk PLL if csr firmware is present.

2015-10-12 Thread Imre Deak
On ma, 2015-10-12 at 16:46 +0200, Patrik Jakobsson wrote:
> On Mon, Oct 12, 2015 at 05:07:13PM +0300, Imre Deak wrote:
> > On ma, 2015-10-12 at 16:37 +0300, Imre Deak wrote:
> > > On ma, 2015-08-03 at 21:55 +0530, Animesh Manna wrote:
> > > > While display engine entering into low power state no need to disable
> > > > cdclk pll as CSR firmware of dmc will take care. If pll is already
> > > > enabled firmware execution sequence will be blocked. This is one
> > > > of the criteria for dmc to work properly.
> > > > 
> > > > Cc: Daniel Vetter 
> > > > Cc: Damien Lespiau 
> > > > Cc: Imre Deak 
> > > > Cc: Sunil Kamath 
> > > > Signed-off-by: Animesh Manna 
> > > > Signed-off-bt: Vathsala Nagaraju 
> > > > Signed-off-by: Rajneesh Bhardwaj 
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_display.c | 11 +++
> > > >  1 file changed, 7 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > > > b/drivers/gpu/drm/i915/intel_display.c
> > > > index af0bcfe..ef2ef4d 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -5675,10 +5675,13 @@ void skl_uninit_cdclk(struct drm_i915_private 
> > > > *dev_priv)
> > > > if (I915_READ(DBUF_CTL) & DBUF_POWER_STATE)
> > > > DRM_ERROR("DBuf power disable timeout\n");
> > > 
> > > My understanding is that DBUF_CTL is also handled by the firmware and so
> > > we shouldn't need to disable it either manually. I guess that could be
> > > addressed as a follow-up.
> > > 
> > > >  
> > > > -   /* disable DPLL0 */
> > > > -   I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) & 
> > > > ~LCPLL_PLL_ENABLE);
> > > > -   if (wait_for(!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK), 1))
> > > > -   DRM_ERROR("Couldn't disable DPLL0\n");
> > > > +   if (dev_priv->csr.dmc_payload) {
> > > > +   /* disable DPLL0 */
> > > > +   I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) &
> > > > +   ~LCPLL_PLL_ENABLE);
> > > > +   if (wait_for(!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK), 
> > > > 1))
> > > > +   DRM_ERROR("Couldn't disable DPLL0\n");
> > > > +   }
> > > >  
> > > > intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
> > 
> > Not introduced in this patch, but the above put looks incorrect. We get
> > here on the runtime suspend path, where all RPM and hence display power
> > domain references should be dropped already. So not sure how this can
> > even work atm. This is for someone to look into as a follow-up.
> 
> Hmm, I thought that was fixed already. This seems to be the last comment on
> Paulos attempt at fixing it:
> 
> http://lists.freedesktop.org/archives/intel-gfx/2015-August/073122.html

Yes, that would solve this issue. One note about it is that we only want
to manually toggle PW1 and Misc IO if DC6 is disabled (via a module
option for example). And if so, toggling of PW1 and Misc IO would be
part of the bigger
"Sequence for Software to Allow/Disallow Package C9-C10".

> > > >  }
> > > 
> > > 
> > > ___
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [SKL-DMC-BUGFIX 3/5] drm/i915/skl: Do not disable cdclk PLL if csr firmware is present.

2015-10-12 Thread Patrik Jakobsson
On Mon, Oct 12, 2015 at 05:07:13PM +0300, Imre Deak wrote:
> On ma, 2015-10-12 at 16:37 +0300, Imre Deak wrote:
> > On ma, 2015-08-03 at 21:55 +0530, Animesh Manna wrote:
> > > While display engine entering into low power state no need to disable
> > > cdclk pll as CSR firmware of dmc will take care. If pll is already
> > > enabled firmware execution sequence will be blocked. This is one
> > > of the criteria for dmc to work properly.
> > > 
> > > Cc: Daniel Vetter 
> > > Cc: Damien Lespiau 
> > > Cc: Imre Deak 
> > > Cc: Sunil Kamath 
> > > Signed-off-by: Animesh Manna 
> > > Signed-off-bt: Vathsala Nagaraju 
> > > Signed-off-by: Rajneesh Bhardwaj 
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 11 +++
> > >  1 file changed, 7 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index af0bcfe..ef2ef4d 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -5675,10 +5675,13 @@ void skl_uninit_cdclk(struct drm_i915_private 
> > > *dev_priv)
> > >   if (I915_READ(DBUF_CTL) & DBUF_POWER_STATE)
> > >   DRM_ERROR("DBuf power disable timeout\n");
> > 
> > My understanding is that DBUF_CTL is also handled by the firmware and so
> > we shouldn't need to disable it either manually. I guess that could be
> > addressed as a follow-up.
> > 
> > >  
> > > - /* disable DPLL0 */
> > > - I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) & ~LCPLL_PLL_ENABLE);
> > > - if (wait_for(!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK), 1))
> > > - DRM_ERROR("Couldn't disable DPLL0\n");
> > > + if (dev_priv->csr.dmc_payload) {
> > > + /* disable DPLL0 */
> > > + I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) &
> > > + ~LCPLL_PLL_ENABLE);
> > > + if (wait_for(!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK), 1))
> > > + DRM_ERROR("Couldn't disable DPLL0\n");
> > > + }
> > >  
> > >   intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
> 
> Not introduced in this patch, but the above put looks incorrect. We get
> here on the runtime suspend path, where all RPM and hence display power
> domain references should be dropped already. So not sure how this can
> even work atm. This is for someone to look into as a follow-up.

Hmm, I thought that was fixed already. This seems to be the last comment on
Paulos attempt at fixing it:

http://lists.freedesktop.org/archives/intel-gfx/2015-August/073122.html

> > >  }
> > 
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] As of kernel 4.3-rc1 system will not stay in S3 suspend [REGRESSION][BISECTED]

2015-10-12 Thread Jani Nikula
On Fri, 09 Oct 2015, Doug Smythies  wrote:
> This started somewhere between Kernel 4.2 and 4.3-rc1,
> but I only noticed it a day ago.

Thanks for the report, please let's follow up at [1].

Thanks,
Jani.


[1] https://bugs.freedesktop.org/show_bug.cgi?id=92414


-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [SKL-DMC-BUGFIX 3/5] drm/i915/skl: Do not disable cdclk PLL if csr firmware is present.

2015-10-12 Thread Imre Deak
On ma, 2015-10-12 at 16:37 +0300, Imre Deak wrote:
> On ma, 2015-08-03 at 21:55 +0530, Animesh Manna wrote:
> > While display engine entering into low power state no need to disable
> > cdclk pll as CSR firmware of dmc will take care. If pll is already
> > enabled firmware execution sequence will be blocked. This is one
> > of the criteria for dmc to work properly.
> > 
> > Cc: Daniel Vetter 
> > Cc: Damien Lespiau 
> > Cc: Imre Deak 
> > Cc: Sunil Kamath 
> > Signed-off-by: Animesh Manna 
> > Signed-off-bt: Vathsala Nagaraju 
> > Signed-off-by: Rajneesh Bhardwaj 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 11 +++
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index af0bcfe..ef2ef4d 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5675,10 +5675,13 @@ void skl_uninit_cdclk(struct drm_i915_private 
> > *dev_priv)
> > if (I915_READ(DBUF_CTL) & DBUF_POWER_STATE)
> > DRM_ERROR("DBuf power disable timeout\n");
> 
> My understanding is that DBUF_CTL is also handled by the firmware and so
> we shouldn't need to disable it either manually. I guess that could be
> addressed as a follow-up.
> 
> >  
> > -   /* disable DPLL0 */
> > -   I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) & ~LCPLL_PLL_ENABLE);
> > -   if (wait_for(!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK), 1))
> > -   DRM_ERROR("Couldn't disable DPLL0\n");
> > +   if (dev_priv->csr.dmc_payload) {
> > +   /* disable DPLL0 */
> > +   I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) &
> > +   ~LCPLL_PLL_ENABLE);
> > +   if (wait_for(!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK), 1))
> > +   DRM_ERROR("Couldn't disable DPLL0\n");
> > +   }
> >  
> > intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);

Not introduced in this patch, but the above put looks incorrect. We get
here on the runtime suspend path, where all RPM and hence display power
domain references should be dropped already. So not sure how this can
even work atm. This is for someone to look into as a follow-up.

> >  }
> 
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [SKL-DMC-BUGFIX 5/5] drm/i915/skl: Removed csr firmware load in resume path

2015-10-12 Thread Imre Deak
On ma, 2015-08-03 at 21:55 +0530, Animesh Manna wrote:
> As csr firmware is taking care of loading the firmware,
> so no need for driver to load again.

I'd think that it's some other (PUnit, PCode) firmware that would take
care of reprogramming the CSR(DMC) firmware, rather than the CSR
firmware itself.

> Cc: Daniel Vetter 
> Cc: Damien Lespiau 
> Cc: Imre Deak 
> Cc: Sunil Kamath 
> Signed-off-by: Animesh Manna 
> Signed-off-by: Vathsala Nagaraju 
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e1d0102..02019e9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1066,13 +1066,10 @@ static int bxt_resume_prepare(struct drm_i915_private 
> *dev_priv)
>  
>  static int skl_resume_prepare(struct drm_i915_private *dev_priv)
>  {
> - struct drm_device *dev = dev_priv->dev;
> -
>   if (intel_csr_load_status_get(dev_priv) == FW_LOADED)
>   skl_disable_dc6(dev_priv);
>  
>   skl_init_cdclk(dev_priv);
> - intel_csr_load_program(dev);

This also means that we don't reprogram the firmware during S3 and S4
resume. I can see how this would work during S3 resume, but I can't see
how it would work during S4 resume, especially if the kernel loading the
hibernation image doesn't load the firmware itself (possible if it
doesn't have the i915 driver). If you confirmed that we don't need to
reprogram the firmware ever, please add this to the commit message
mentioning explicitly the D3, S0ix, S3, S4 states.

>  
>   return 0;
>  }


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [SKL-DMC-BUGFIX 4/5] drm/i915/skl: Block disable call for pw1 if dmc firmware is present.

2015-10-12 Thread Imre Deak
On ma, 2015-08-03 at 21:55 +0530, Animesh Manna wrote:
> Another interesting criteria to work dmc as expected is pw1 to be
> enabled by driver and dmc will shut it off in its execution
> sequence. If already disabled by driver dmc will get confuse and
> behave differently than expected found during pc10 entry issue
> for skl.
> 
> So berfore we disable power-well 1, added check if dmc firmware is
> present and driver will not disable power well 1, but for any reason
> if firmware is not present of failed to load we can shut off the
> power well 1 which will save some power.
> 
> As skl is currently fully dependent on dmc to go in lowest possible
> power state (dc6) but the same is not applicable for bxt. Display
> engine can enter into dc9 without dmc, hence unblocking disable call.
> 
> v1: Initial version.
> 
> v2: Based on revire commnents from Sunil,
> - condition check for pw1 is moved in skl_set_power_well.
> 
> Cc: Daniel Vetter 
> Cc: Damien Lespiau 
> Cc: Imre Deak 
> Cc: Sunil Kamath 
> Signed-off-by: Animesh Manna 
> Signed-off-by: Vathsala Nagaraju 
> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index c660245..00cd4ff 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -636,9 +636,15 @@ static void skl_set_power_well(struct drm_i915_private 
> *dev_priv,
>   }
>   } else {
>   if (enable_requested) {
> - I915_WRITE(HSW_PWR_WELL_DRIVER, tmp & ~req_mask);
> - POSTING_READ(HSW_PWR_WELL_DRIVER);
> - DRM_DEBUG_KMS("Disabling %s\n", power_well->name);
> + if (IS_SKYLAKE(dev) &&
> + (power_well->data == SKL_DISP_PW_1) &&
> + (intel_csr_load_status_get(dev_priv) == 
> FW_LOADED))

We'll only ever get here with the firmware loaded, so no need to check
for it. 

> + DRM_DEBUG_KMS("Not Disabling PW1, dmc will 
> handle\n");
> + else {
> + I915_WRITE(HSW_PWR_WELL_DRIVER, tmp & 
> ~req_mask);
> + POSTING_READ(HSW_PWR_WELL_DRIVER);
> + DRM_DEBUG_KMS("Disabling %s\n", 
> power_well->name);
> + }

Not disabling PW1 here is a good solution for now imo, but ideally we
should move both PW1 enabling and disabling out from this code and do
these as part of the display init/uninit sequence. This could be done as
a follow-up to this patchset.

>  
>   if (GEN9_ENABLE_DC5(dev) &&
>   power_well->data == SKL_DISP_PW_2)


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [SKL-DMC-BUGFIX 3/5] drm/i915/skl: Do not disable cdclk PLL if csr firmware is present.

2015-10-12 Thread Imre Deak
On ma, 2015-08-03 at 21:55 +0530, Animesh Manna wrote:
> While display engine entering into low power state no need to disable
> cdclk pll as CSR firmware of dmc will take care. If pll is already
> enabled firmware execution sequence will be blocked. This is one
> of the criteria for dmc to work properly.
> 
> Cc: Daniel Vetter 
> Cc: Damien Lespiau 
> Cc: Imre Deak 
> Cc: Sunil Kamath 
> Signed-off-by: Animesh Manna 
> Signed-off-bt: Vathsala Nagaraju 
> Signed-off-by: Rajneesh Bhardwaj 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index af0bcfe..ef2ef4d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5675,10 +5675,13 @@ void skl_uninit_cdclk(struct drm_i915_private 
> *dev_priv)
>   if (I915_READ(DBUF_CTL) & DBUF_POWER_STATE)
>   DRM_ERROR("DBuf power disable timeout\n");

My understanding is that DBUF_CTL is also handled by the firmware and so
we shouldn't need to disable it either manually. I guess that could be
addressed as a follow-up.

>  
> - /* disable DPLL0 */
> - I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) & ~LCPLL_PLL_ENABLE);
> - if (wait_for(!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK), 1))
> - DRM_ERROR("Couldn't disable DPLL0\n");
> + if (dev_priv->csr.dmc_payload) {
> + /* disable DPLL0 */
> + I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) &
> + ~LCPLL_PLL_ENABLE);
> + if (wait_for(!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK), 1))
> + DRM_ERROR("Couldn't disable DPLL0\n");
> + }
>  
>   intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
>  }


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [SKL-DMC-BUGFIX 2/5] drm/i915/skl: Making DC6 entry is the last call in suspend flow.

2015-10-12 Thread Imre Deak
On ma, 2015-08-03 at 21:55 +0530, Animesh Manna wrote:
> Mmio register access after dc6/dc5 entry is not allowed when
> DC6 power states are enabled according to bspec (bspec-id 0527),
> so enabling dc6 as the last call in suspend flow.

The MMIO range BSpec-ID 0527 refers to is the DMC MMIO range. The driver
uses this MMIO range only to program the FW image, it doesn't access it
afterwards. So that's not a good justification to keep DC6 disabled.

That BSpec-ID also mentions that DC6 together with DC5 needs to be
disabled around modesets, which we need to address by disabling both
DC5/6, but only around modesets.

Later discussions with HW people revealed that there is an open issue
related to DC6, see [1]. The suggested workaround for that issue is to
keep DC6 disabled all the time and use a manual sequence to enable
deeper power states (see "Sequence for Software to Allow Package C9-C10"
in BSpec).

Based on this what we need to do in this patch is simply disable DC6. As
a follow-up to this patchset we need to:
- Add the manual PC9/10 sequence.
- Prevent DC5/6 during modesets (and DPAUX transfers).
- Move out the remaining parts of the display init/uninit sequence from
the suspend/resume path.
- Add a module options to select between enabling DC6 and running the
manual PC9/10 sequence. People still would like to experiment with DC6
and we may end up enabling it in the end if all the open issues get
resolved. For that case enabling DC6 should still happen at the place
where it happens now, that is after disabling PW2.

Please see my corresponding comments inlined below.

[1]
http://lists.freedesktop.org/archives/intel-gfx/2015-October/077669.html

> 
> v1: Initial version.
> 
> v2: commit message updated based on comment from Vathsala.
> 
> Cc: Daniel Vetter 
> Cc: Damien Lespiau 
> Cc: Imre Deak 
> Cc: Sunil Kamath 
> Signed-off-by: Animesh Manna 
> Signed-off-by: Vathsala Nagaraju 
> Signed-off-by: Rajneesh Bhardwaj 
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 12 ++--
>  drivers/gpu/drm/i915/intel_drv.h|  2 ++
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 33 
> -
>  3 files changed, 16 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 0d6775a..e1d0102 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1017,14 +1017,11 @@ static int skl_suspend_complete(struct 
> drm_i915_private *dev_priv)
>  {
>   /* Enabling DC6 is not a hard requirement to enter runtime D3 */
>  
> - /*
> -  * This is to ensure that CSR isn't identified as loaded before
> -  * CSR-loading program is called during runtime-resume.
> -  */
> - intel_csr_load_status_set(dev_priv, FW_UNINITIALIZED);
> -
>   skl_uninit_cdclk(dev_priv);
>  
> + if (intel_csr_load_status_get(dev_priv) == FW_LOADED)
> + skl_enable_dc6(dev_priv);
> +

Not needed, atm we shouldn't enable DC6 ever due to open issues under
investigation. Even if those investigations reveal that we can use DC6
it shouldn't be enabled here, rather at its current place after
disabling PW2.

>   return 0;
>  }
>  
> @@ -1071,6 +1068,9 @@ static int skl_resume_prepare(struct drm_i915_private 
> *dev_priv)
>  {
>   struct drm_device *dev = dev_priv->dev;
>  
> + if (intel_csr_load_status_get(dev_priv) == FW_LOADED)
> + skl_disable_dc6(dev_priv);
> +

Not needed based on the previous comment.

>   skl_init_cdclk(dev_priv);
>   intel_csr_load_program(dev);
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index 47cef0e..06f346f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1117,6 +1117,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
>  void bxt_disable_dc9(struct drm_i915_private *dev_priv);
>  void skl_init_cdclk(struct drm_i915_private *dev_priv);
>  void skl_uninit_cdclk(struct drm_i915_private *dev_priv);
> +void skl_enable_dc6(struct drm_i915_private *dev_priv);
> +void skl_disable_dc6(struct drm_i915_private *dev_priv);
>  void intel_dp_get_m_n(struct intel_crtc *crtc,
> struct intel_crtc_state *pipe_config);
>  void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n);
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 6393b76..c660245 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -532,7 +532,7 @@ static void assert_can_disable_dc6(struct 
> drm_i915_private *dev_priv)
>   "DC6 already programmed to be disabled.\n");
>  }
>  
> -static void skl_enable_dc6(struct drm_i915_private *dev_priv)
> +void skl_enable_dc6(struct drm_i915_private *dev_priv)
>  {
>   uint32_t val;
>  
> @@ -549,7 +549,7 @@ static void skl_enable_dc6(struct drm_i915_private 
> *dev_priv)
>   POSTING

Re: [Intel-gfx] [PATCH] drm/i915: Convert WARNs during userptr revoke to SIGBUS

2015-10-12 Thread Tvrtko Ursulin


On 12/10/15 11:10, Chris Wilson wrote:

On Mon, Oct 12, 2015 at 10:31:35AM +0100, Chris Wilson wrote:

We basically need to clone the obj, move the pages and vma over and so
as the vma die the pages are unreferenced. All new use will be forced to
call gup and be fine.

Ok, that smells ok, I'll see how doable that is.


Hmm. If we take the vma-centric driver as granted (i.e. using the vma as the
token when pinning, the vma holds fences etc), the tricky part if that we
don't hold a reference from the pinned vma to the object. pin_display to
the rescue!

Initial sketch:

static struct drm_i915_gem_object *steal_pages(struct drm_i915_gem_object *obj)
{
 struct drm_i915_gem_object *clone;
 struct i915_vma *vma;
 int i;

 BUG_ON(obj->stolen);

 clone = i915_gem_object_alloc(obj->base.dev);
 if (clone == NULL)
 return clone;

 drm_gem_private_object_init(obj->base.dev,
 &clone->base,
 obj->base.size);
 i915_gem_object_init(clone, obj->ops);

 list_splice_init(&obj->vma_list, &clone->vma_list);
 list_for_each_entry(vma, &clone->vma_list, obj_link)
 vma->obj = clone;

 if (obj->pin_display) {
 clone->pin_display = obj->pin_display;
 while (obj->pin_display--) {
 drm_gem_object_reference(&clone->base);
 drm_gem_object_unreference(&obj->base);
 }
 }

 clone->bind_count = obj->bind_count;
 obj->bind_count = 0;
 /* vma_ht / vma_hashed */

 for (i = 0; i < I915_NUM_RINGS; i++) {
 if (obj->last_read[i].request == NULL)
 continue;

 clone->last_read[i].request = obj->last_read[i].request;
 list_replace_init(&obj->last_read[i].link,
   &clone->last_read[i].link);
 clone->flags |= 1 << (i + I915_BO_ACTIVE_SHIFT);
 }
 if (obj->last_write.request) {
 clone->last_write.request = obj->last_write.request;
 list_replace_init(&obj->last_write.link,
   &clone->last_write.link);
 }

 clone->dirty = obj->dirty;
 obj->dirty = false;

 clone->tiling_mode = obj->tiling_mode;
 clone->stride = obj->stride;

 clone->pin_display = obj->pin_display;
 obj->pin_display = 0;

 clone->madv = I915_MADV_DONTNEED;
 clone->pages = obj->pages;
 clone->pages_pin_count = obj->pages_pin_count;
 clone->get_page = obj->get_page;
 clone->vmapping = obj->vmapping;
 obj->pages = NULL;
 obj->pages_pin_count = 0;
 obj->vmapping = NULL;

 clone->bit_17 = obj->bit_17;
 obj->bit_17 = NULL;

 i915_gem_release_mmap(obj);

 if (I915_BO_IS_ACTIVE(clone))
 clone->active_reference = true;
 else
 drm_gem_object_unreference(&clone->base);

 return clone;
}

It does have the presumption that we have either an active reference or a
pinned reference.


I get the general idea but the base is too far from the current code to 
properly evaluate. But in principle it sounds promising.


Regards,

Tvrtko





___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] 4.2-rc4 kernel warnings on HSW laptop [regression]

2015-10-12 Thread Takashi Iwai
On Mon, 12 Oct 2015 14:29:19 +0200,
Takashi Iwai wrote:
> 
> > > Then a warning when I start powertop:
> > > 
> > >  WARNING: CPU: 1 PID: 1674 at drivers/gpu/drm/drm_atomic.c:889 
> > > drm_atomic_get_property+0x232/0x2b0 [drm]()
> > >  CPU: 1 PID: 1674 Comm: powertop Tainted: GW   
> > > 4.3.0-rc4-test+ #93
> > >  Hardware name: Hewlett-Packard HP ProBook 430 G1/1946, BIOS L73 Ver. 
> > > 08.05 2013/03/15
> > >   a005b289 880059173ca8 81346fa9 
> > >   880059173ce0 81063232 8800372bc028 8800373f3740
> > >   8800372bc000 880062ad37c0 0001 880059173cf0
> > >  Call Trace:
> > >   [] dump_stack+0x4b/0x72
> > >   [] warn_slowpath_common+0x82/0xc0
> > >   [] warn_slowpath_null+0x1a/0x20
> > >   [] drm_atomic_get_property+0x232/0x2b0 [drm]
> > >   [] drm_object_property_get_value+0x6c/0x70 [drm]
> > >   [] dpms_show+0x2f/0x70 [drm]
> > >   [] dev_attr_show+0x20/0x50
> > >   [] sysfs_kf_seq_show+0xbc/0x130
> > >   [] kernfs_seq_show+0x23/0x30
> > >   [] seq_read+0xca/0x360
> > >   [] kernfs_fop_read+0x10a/0x160
> > >   [] __vfs_read+0x28/0xd0
> > >   [] ? security_file_permission+0xa0/0xc0
> > >   [] ? rw_verify_area+0x4f/0xe0
> > >   [] vfs_read+0x83/0x130
> > >   [] SyS_read+0x46/0xa0
> > >   [] ? syscall_return_slowpath+0x50/0x130
> > >   [] entry_SYSCALL_64_fastpath+0x16/0x75
> > > 
> > > 
> > > Both don't look like serious issues, but not sexy to see at each time.
> > 
> > This should be fixed with
> > 
> > commit 621bd0f6982badd6483acb191eb7b6226a578328
> > Author: Daniel Vetter 
> > Date:   Tue Sep 29 09:56:53 2015 +0200
> > 
> > drm: Fix locking for sysfs dpms file
> > 
> > If not please scream.
> 
> OK, I'll give a shot.

This one was confirmed to work.  Feel free to my tested-by tag if you
can still add.
  Tested-by: Takashi Iwai 


thanks,

Takashi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] 4.2-rc4 kernel warnings on HSW laptop [regression]

2015-10-12 Thread Takashi Iwai
On Mon, 12 Oct 2015 09:04:20 +0200,
Daniel Vetter wrote:
> 
> Another pile of regressions for Jairo to track ...
> 
> On Sat, Oct 10, 2015 at 11:46:29AM +0200, Takashi Iwai wrote:
> > Hi,
> > 
> > I noticed that a HSW laptop gets a few new warnings since 4.2-rc
> > kernels.  One error messages pops at each boot time:
> > 
> >  Console: switching to colour dummy device 80x25
> >  [drm] Replacing VGA console driver
> >  [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> >  [drm] Driver supports precise vblank timestamp query.
> >  vgaarb: device changed decodes: 
> > PCI::00:02.0,olddecodes=io+mem,decodes=io+mem:owns=io+mem
> >  [drm:drm_calc_timestamping_constants [drm]] *ERROR* crtc 21: Can't 
> > calculate constants, dotclock = 0!
> 
> There's patches for this one already, but I accidentally applied them for
> -next, not -fixes. They should land for -rc6.

Could you point commit ids?


> > Then a warning when I start powertop:
> > 
> >  WARNING: CPU: 1 PID: 1674 at drivers/gpu/drm/drm_atomic.c:889 
> > drm_atomic_get_property+0x232/0x2b0 [drm]()
> >  CPU: 1 PID: 1674 Comm: powertop Tainted: GW   4.3.0-rc4-test+ 
> > #93
> >  Hardware name: Hewlett-Packard HP ProBook 430 G1/1946, BIOS L73 Ver. 08.05 
> > 2013/03/15
> >   a005b289 880059173ca8 81346fa9 
> >   880059173ce0 81063232 8800372bc028 8800373f3740
> >   8800372bc000 880062ad37c0 0001 880059173cf0
> >  Call Trace:
> >   [] dump_stack+0x4b/0x72
> >   [] warn_slowpath_common+0x82/0xc0
> >   [] warn_slowpath_null+0x1a/0x20
> >   [] drm_atomic_get_property+0x232/0x2b0 [drm]
> >   [] drm_object_property_get_value+0x6c/0x70 [drm]
> >   [] dpms_show+0x2f/0x70 [drm]
> >   [] dev_attr_show+0x20/0x50
> >   [] sysfs_kf_seq_show+0xbc/0x130
> >   [] kernfs_seq_show+0x23/0x30
> >   [] seq_read+0xca/0x360
> >   [] kernfs_fop_read+0x10a/0x160
> >   [] __vfs_read+0x28/0xd0
> >   [] ? security_file_permission+0xa0/0xc0
> >   [] ? rw_verify_area+0x4f/0xe0
> >   [] vfs_read+0x83/0x130
> >   [] SyS_read+0x46/0xa0
> >   [] ? syscall_return_slowpath+0x50/0x130
> >   [] entry_SYSCALL_64_fastpath+0x16/0x75
> > 
> > 
> > Both don't look like serious issues, but not sexy to see at each time.
> 
> This should be fixed with
> 
> commit 621bd0f6982badd6483acb191eb7b6226a578328
> Author: Daniel Vetter 
> Date:   Tue Sep 29 09:56:53 2015 +0200
> 
> drm: Fix locking for sysfs dpms file
> 
> If not please scream.

OK, I'll give a shot.


thanks,

Takashi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases

2015-10-12 Thread Sharma, Shashank
We were debugging this issue, and we could find the root cause:
In function:
Intel_hpd_init()
|
encoder->hotplug()
|
display_power_get()
|
Intel_powe_well_enable()
|
power_well->ops->enable()
|
chv_pipe_power_well_enable()
|
vlv_display_power_well_init()
|
intel_hdp_init()

This function ends up calling intel_hpd_init, Which is causing the mutex 
deadlock due to recursion, as we are calling encoder->hotplug() from hpd_init().

Intel_hpd_init()
|
encoder->hotplug()
|
display_power_get()

There are two solutions here:
- remove hpd_init() from get_calls 
- remove encoder->hotplug() function call from hpd_init() and put it in some 
other place. We have added this function from encoder_init() for android trees, 
and it works well there. 

Regards
Shashank
-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of 
Ville Syrjälä
Sent: Thursday, October 08, 2015 7:06 PM
To: Jindal, Sonika
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915: Call encoder hotplug for init 
and resume cases

On Mon, Oct 05, 2015 at 04:43:14PM +0530, Sonika Jindal wrote:
> For all the encoders, call the hot_plug if it is registered.
> This is required for connected boot and resume cases to generate fake 
> hpd resulting in reading of edid.
> Removing the initial sdvo hot_plug call too so that it will be called 
> just once from this loop.
> 
> Signed-off-by: Sonika Jindal 
> ---
>  drivers/gpu/drm/i915/intel_hotplug.c |   11 +++
>  drivers/gpu/drm/i915/intel_sdvo.c|1 -
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c 
> b/drivers/gpu/drm/i915/intel_hotplug.c
> index 53c0173..eac4757 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -458,6 +458,7 @@ void intel_hpd_init(struct drm_i915_private 
> *dev_priv)  {
>   struct drm_device *dev = dev_priv->dev;
>   struct drm_mode_config *mode_config = &dev->mode_config;
> + struct intel_encoder *encoder;
>   struct drm_connector *connector;
>   int i;
>  
> @@ -482,6 +483,16 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
>   if (dev_priv->display.hpd_irq_setup)
>   dev_priv->display.hpd_irq_setup(dev);
>   spin_unlock_irq(&dev_priv->irq_lock);
> +
> + /*
> +  * Connected boot / resume scenarios can't generate new hot plug.
> +  * So, probe it manually.
> +  */
> + list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> + base.head) {
> + if (encoder->hot_plug)
> + encoder->hot_plug(encoder);
> + }


This breaks the world on CHV

[ 3187.575198] [drm:intel_hdmi_hot_plug] Live status not up!
[ 3187.585154] =
[ 3187.595010] [ INFO: possible recursive locking detected ]
[ 3187.604685] 4.3.0-rc4-bsw+ #2488 Tainted: G U  W  
[ 3187.614366] -
[ 3187.623892] Xorg/32212 is trying to acquire lock:
[ 3187.632635]  (&power_domains->lock){+.+...}, at: [] 
intel_display_power_get+0x38/0xcb [i915] [ 3187.647492] [ 3187.647492] but task 
is already holding lock:
[ 3187.661054]  (&power_domains->lock){+.+...}, at: [] 
intel_display_power_get+0x38/0xcb [i915] [ 3187.675960] [ 3187.675960] other 
info that might help us debug this:
[ 3187.690459]  Possible unsafe locking scenario:
[ 3187.690459] 
[ 3187.704224]CPU0
[ 3187.710485]
[ 3187.716711]   lock(&power_domains->lock);
[ 3187.724718]   lock(&power_domains->lock);
[ 3187.732663]
[ 3187.732663]  *** DEADLOCK ***
[ 3187.732663]
[ 3187.749460]  May be due to missing lock nesting notation [ 3187.749460] [ 
3187.763833] 5 locks held by Xorg/32212:
[ 3187.771523]  #0:  (drm_global_mutex){+.+.+.}, at: [] 
drm_release+0x3b/0x49b [drm] [ 3187.785216]  #1:  
(&dev->mode_config.mutex){+.+.+.}, at: [] 
drm_modeset_lock_all+0x54/0xcd [drm] [ 3187.800437]  #2:  
(crtc_ww_class_acquire){+.+.+.}, at: [] 
drm_modeset_lock_all+0x5e/0xcd [drm] [ 3187.815488]  #3:  
(crtc_ww_class_mutex){+.+.+.}, at: [] 
drm_modeset_lock+0x75/0xfc [drm] [ 3187.830094]  #4:  
(&power_domains->lock){+.+...}, at: [] 
intel_display_power_get+0x38/0xcb [i915] [ 3187.845534] [ 3187.845534] stack 
backtrace:
[ 3187.857685] CPU: 2 PID: 32212 Comm: Xorg Tainted: G U  W   
4.3.0-rc4-bsw+ #2488
[ 3187.870331] Hardware name: Intel Corporation CHERRYVIEW C0 PLATFORM/Braswell 
CRB, BIOS BRAS.X64.B085.R00.1509110553 09/11/2015 [ 3187.886827]  
 880175eff8e0 8128d59e 823f5ee0 [ 
3187.898904]  880175eff958 810a7a08  
880179d1c5d0 [ 3187.910954]  0004 0006 
45422a91588a4c3e 0005 [ 3187.923011] Call Trace:
[ 3187.929451]  [] dump_stack+0x4e/0x79 [ 3187.938912]  
[] __lock_acquire+0x7ab

Re: [Intel-gfx] [PATCH] drm/i915: Move skl/bxt gt specific workarounds to ring init

2015-10-12 Thread Chris Wilson
On Mon, Oct 12, 2015 at 01:20:59PM +0300, Mika Kuoppala wrote:
> Some registers are, naturally, lost in gpu reset/suspend cycle.
> And some registers, for example in display domain, are not subject
> to gpu reset so they retain their contents.
> 
> As hang recovery triggers a reset, recoverable gpu hang can currently
> flush out essential workarounds and cause havoc later on.
> 
> When register GEN8_GARBNTL is missing the WaEnableGapsTsvCreditFix:skl,
> it can cause random system hangs [1]. This workaround was added in:
> commit 245d96670d26 ("drm/i915:skl: Add WaEnableGapsTsvCreditFix")
> But another set of system hangs were observed and the failure pattern
> indicated that there was random gpu hang preceding the system hang [2].
> This lead to the realization that we lose this workaround and BDW_SCRATCH1
> on reset.
> 
> Add these workarounds setup in display init to skl/bxt ring init
> where LRI workarounds are also setup. This way their setup is not
> dependent on display side init.
> 
> References: [1] https://bugs.freedesktop.org/show_bug.cgi?id=90854
> References: [2] https://bugs.freedesktop.org/show_bug.cgi?id=92315
> Reported-by: Tomi Sarvela 
> Cc: Tomi Sarvela 
> Cc: Ville Syrjälä 
> Signed-off-by: Mika Kuoppala 

I had forgotten we had begun splitting out the GT registers from the
display registers for init_hw. These movements make sense so,
Reviewed-by: Chris Wilson 
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: Move skl/bxt gt specific workarounds to ring init

2015-10-12 Thread Mika Kuoppala
Some registers are, naturally, lost in gpu reset/suspend cycle.
And some registers, for example in display domain, are not subject
to gpu reset so they retain their contents.

As hang recovery triggers a reset, recoverable gpu hang can currently
flush out essential workarounds and cause havoc later on.

When register GEN8_GARBNTL is missing the WaEnableGapsTsvCreditFix:skl,
it can cause random system hangs [1]. This workaround was added in:
commit 245d96670d26 ("drm/i915:skl: Add WaEnableGapsTsvCreditFix")
But another set of system hangs were observed and the failure pattern
indicated that there was random gpu hang preceding the system hang [2].
This lead to the realization that we lose this workaround and BDW_SCRATCH1
on reset.

Add these workarounds setup in display init to skl/bxt ring init
where LRI workarounds are also setup. This way their setup is not
dependent on display side init.

References: [1] https://bugs.freedesktop.org/show_bug.cgi?id=90854
References: [2] https://bugs.freedesktop.org/show_bug.cgi?id=92315
Reported-by: Tomi Sarvela 
Cc: Tomi Sarvela 
Cc: Ville Syrjälä 
Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/intel_pm.c | 60 -
 drivers/gpu/drm/i915/intel_ringbuffer.c | 44 +++-
 2 files changed, 43 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3f9b3c0..2482cfd 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -52,56 +52,10 @@
 #define INTEL_RC6p_ENABLE  (1<<1)
 #define INTEL_RC6pp_ENABLE (1<<2)
 
-static void gen9_init_clock_gating(struct drm_device *dev)
-{
-   struct drm_i915_private *dev_priv = dev->dev_private;
-
-   /* WaEnableLbsSlaRetryTimerDecrement:skl */
-   I915_WRITE(BDW_SCRATCH1, I915_READ(BDW_SCRATCH1) |
-  GEN9_LBS_SLA_RETRY_TIMER_DECREMENT_ENABLE);
-
-   /* WaDisableKillLogic:bxt,skl */
-   I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) |
-  ECOCHK_DIS_TLB);
-}
-
-static void skl_init_clock_gating(struct drm_device *dev)
-{
-   struct drm_i915_private *dev_priv = dev->dev_private;
-
-   gen9_init_clock_gating(dev);
-
-   if (INTEL_REVID(dev) <= SKL_REVID_D0) {
-   /* WaDisableHDCInvalidation:skl */
-   I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) |
-  BDW_DISABLE_HDC_INVALIDATION);
-
-   /* WaDisableChickenBitTSGBarrierAckForFFSliceCS:skl */
-   I915_WRITE(FF_SLICE_CS_CHICKEN2,
-  _MASKED_BIT_ENABLE(GEN9_TSG_BARRIER_ACK_DISABLE));
-   }
-
-   /* GEN8_L3SQCREG4 has a dependency with WA batch so any new changes
-* involving this register should also be added to WA batch as required.
-*/
-   if (INTEL_REVID(dev) <= SKL_REVID_E0)
-   /* WaDisableLSQCROPERFforOCL:skl */
-   I915_WRITE(GEN8_L3SQCREG4, I915_READ(GEN8_L3SQCREG4) |
-  GEN8_LQSC_RO_PERF_DIS);
-
-   /* WaEnableGapsTsvCreditFix:skl */
-   if (IS_SKYLAKE(dev) && (INTEL_REVID(dev) >= SKL_REVID_C0)) {
-   I915_WRITE(GEN8_GARBCNTL, (I915_READ(GEN8_GARBCNTL) |
-  GEN9_GAPS_TSV_CREDIT_DISABLE));
-   }
-}
-
 static void bxt_init_clock_gating(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
 
-   gen9_init_clock_gating(dev);
-
/* WaDisableSDEUnitClockGating:bxt */
I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
   GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
@@ -112,17 +66,6 @@ static void bxt_init_clock_gating(struct drm_device *dev)
 */
I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
   GEN8_HDCUNIT_CLOCK_GATE_DISABLE_HDCREQ);
-
-   /* WaStoreMultiplePTEenable:bxt */
-   /* This is a requirement according to Hardware specification */
-   if (INTEL_REVID(dev) == BXT_REVID_A0)
-   I915_WRITE(TILECTL, I915_READ(TILECTL) | TILECTL_TLBPF);
-
-   /* WaSetClckGatingDisableMedia:bxt */
-   if (INTEL_REVID(dev) == BXT_REVID_A0) {
-   I915_WRITE(GEN7_MISCCPCTL, (I915_READ(GEN7_MISCCPCTL) &
-   ~GEN8_DOP_CLOCK_GATE_MEDIA_ENABLE));
-   }
 }
 
 static void i915_pineview_get_mem_freq(struct drm_device *dev)
@@ -6991,9 +6934,6 @@ void intel_init_pm(struct drm_device *dev)
if (IS_BROXTON(dev))
dev_priv->display.init_clock_gating =
bxt_init_clock_gating;
-   else if (IS_SKYLAKE(dev))
-   dev_priv->display.init_clock_gating =
-   skl_init_clock_gating;
dev_priv->display.update_wm = skl_update_wm;
} else if (HAS_PCH_SPLIT(dev)) {
ilk_setup_wm_latency(dev);
diff --git a/drivers/gp

Re: [Intel-gfx] [PATCH] drm/i915: Convert WARNs during userptr revoke to SIGBUS

2015-10-12 Thread Chris Wilson
On Mon, Oct 12, 2015 at 10:31:35AM +0100, Chris Wilson wrote:
> We basically need to clone the obj, move the pages and vma over and so
> as the vma die the pages are unreferenced. All new use will be forced to
> call gup and be fine.
> 
> Ok, that smells ok, I'll see how doable that is.

Hmm. If we take the vma-centric driver as granted (i.e. using the vma as the
token when pinning, the vma holds fences etc), the tricky part if that we
don't hold a reference from the pinned vma to the object. pin_display to
the rescue!

Initial sketch:

static struct drm_i915_gem_object *steal_pages(struct drm_i915_gem_object *obj)
{
struct drm_i915_gem_object *clone;
struct i915_vma *vma;
int i;

BUG_ON(obj->stolen);

clone = i915_gem_object_alloc(obj->base.dev);
if (clone == NULL)
return clone;

drm_gem_private_object_init(obj->base.dev,
&clone->base,
obj->base.size);
i915_gem_object_init(clone, obj->ops);

list_splice_init(&obj->vma_list, &clone->vma_list);
list_for_each_entry(vma, &clone->vma_list, obj_link)
vma->obj = clone;

if (obj->pin_display) {
clone->pin_display = obj->pin_display;
while (obj->pin_display--) {
drm_gem_object_reference(&clone->base);
drm_gem_object_unreference(&obj->base);
}
}

clone->bind_count = obj->bind_count;
obj->bind_count = 0;
/* vma_ht / vma_hashed */

for (i = 0; i < I915_NUM_RINGS; i++) {
if (obj->last_read[i].request == NULL)
continue;

clone->last_read[i].request = obj->last_read[i].request;
list_replace_init(&obj->last_read[i].link,
  &clone->last_read[i].link);
clone->flags |= 1 << (i + I915_BO_ACTIVE_SHIFT);
}
if (obj->last_write.request) {
clone->last_write.request = obj->last_write.request;
list_replace_init(&obj->last_write.link,
  &clone->last_write.link);
}

clone->dirty = obj->dirty;
obj->dirty = false;

clone->tiling_mode = obj->tiling_mode;
clone->stride = obj->stride;

clone->pin_display = obj->pin_display;
obj->pin_display = 0;

clone->madv = I915_MADV_DONTNEED;
clone->pages = obj->pages;
clone->pages_pin_count = obj->pages_pin_count;
clone->get_page = obj->get_page;
clone->vmapping = obj->vmapping;
obj->pages = NULL;
obj->pages_pin_count = 0;
obj->vmapping = NULL;

clone->bit_17 = obj->bit_17;
obj->bit_17 = NULL;

i915_gem_release_mmap(obj);

if (I915_BO_IS_ACTIVE(clone))
clone->active_reference = true;
else
drm_gem_object_unreference(&clone->base);

return clone;
}

It does have the presumption that we have either an active reference or a
pinned reference.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Convert WARNs during userptr revoke to SIGBUS

2015-10-12 Thread Chris Wilson
On Mon, Oct 12, 2015 at 10:06:23AM +0100, Tvrtko Ursulin wrote:
> 
> On 09/10/15 18:26, Chris Wilson wrote:
> >On Fri, Oct 09, 2015 at 07:14:02PM +0200, Daniel Vetter wrote:
> >>On Fri, Oct 09, 2015 at 10:03:14AM +0100, Tvrtko Ursulin wrote:
> >>>
> >>>On 09/10/15 09:55, Daniel Vetter wrote:
> On Fri, Oct 09, 2015 at 09:40:53AM +0100, Chris Wilson wrote:
> >On Fri, Oct 09, 2015 at 09:48:01AM +0200, Daniel Vetter wrote:
> >>On Thu, Oct 08, 2015 at 10:45:47AM +0100, Tvrtko Ursulin wrote:
> >>The concern is that this isn't how SIG_SEGV works, it's a signal the
> >>thread who made the invalid access gets directly. You never get a 
> >>SIG_SEGV
> >>for bad access someone else has made. So essentially it's new ABI.
> >
> >SIGBUS. For which the answer is yes, you can and do get SIGBUS for
> >actions taken by other processes.
> 
> Oh right I always forget that SIGBUS aliases with SIGIO. Anyway if
> userspace wants SIGIO we just need to provide it with a pollable fd and
> then it can use fcntl to make that happen. That's imo a much better api
> than unconditionally throwing around signals. Also we already have the
> reset stats ioctl to tell userspace that its gpu context is toats. If
> anyone wants that to be pollable (or even send SIGIO) I think we should
> extend that, with all the usual "needs userspace&igt" stuff on top.
> >>>
> >>>I don't see that this notification can be optional. Process is confused
> >>>about its memory map use so should die. :)
> >>>
> >>>This is not a GPU error/hang - this is the process doing stupid things.
> >>>
> >>>MMU notifiers do not support decision making otherwise we could say
> >>>-ETXTBUSY or something on munmap, but we can't. Not even sure that it would
> >>>help in all cases, would have to fail clone as well and who knows what.
> >>
> >>So what happens if the gpu just keeps using the memory? It'll all be
> >>horribly undefined behaviour and eventually it'll die on an -EFAULT in
> >>execbuf, but does anything else bad happen?
> >
> >We don't see an EFAULT unless a miracle occurs, and the stale pages
> >continue to be read/written by other processes (as well as the client).
> >Horribly undefined behaviour with a misinformation leak.
> 
> What other processes? Pages will still be referenced so won't be
> reused so there is not information leak across unrelated processes.
> Unless you meant ones involved in object sharing?

This client is trying to replace the userptr with a fresh set of pages.
The GPU and other processes continue to see the old pages i.e. old
information (misinformation :) leaks.
 
> But we could improve the revoke mechanism I suppose by marking the
> object and then revoking it at the next opportunity?

We basically need to clone the obj, move the pages and vma over and so
as the vma die the pages are unreferenced. All new use will be forced to
call gup and be fine.

Ok, that smells ok, I'll see how doable that is.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Convert WARNs during userptr revoke to SIGBUS

2015-10-12 Thread Tvrtko Ursulin


On 09/10/15 18:26, Chris Wilson wrote:

On Fri, Oct 09, 2015 at 07:14:02PM +0200, Daniel Vetter wrote:

On Fri, Oct 09, 2015 at 10:03:14AM +0100, Tvrtko Ursulin wrote:


On 09/10/15 09:55, Daniel Vetter wrote:

On Fri, Oct 09, 2015 at 09:40:53AM +0100, Chris Wilson wrote:

On Fri, Oct 09, 2015 at 09:48:01AM +0200, Daniel Vetter wrote:

On Thu, Oct 08, 2015 at 10:45:47AM +0100, Tvrtko Ursulin wrote:
The concern is that this isn't how SIG_SEGV works, it's a signal the
thread who made the invalid access gets directly. You never get a SIG_SEGV
for bad access someone else has made. So essentially it's new ABI.


SIGBUS. For which the answer is yes, you can and do get SIGBUS for
actions taken by other processes.


Oh right I always forget that SIGBUS aliases with SIGIO. Anyway if
userspace wants SIGIO we just need to provide it with a pollable fd and
then it can use fcntl to make that happen. That's imo a much better api
than unconditionally throwing around signals. Also we already have the
reset stats ioctl to tell userspace that its gpu context is toats. If
anyone wants that to be pollable (or even send SIGIO) I think we should
extend that, with all the usual "needs userspace&igt" stuff on top.


I don't see that this notification can be optional. Process is confused
about its memory map use so should die. :)

This is not a GPU error/hang - this is the process doing stupid things.

MMU notifiers do not support decision making otherwise we could say
-ETXTBUSY or something on munmap, but we can't. Not even sure that it would
help in all cases, would have to fail clone as well and who knows what.


So what happens if the gpu just keeps using the memory? It'll all be
horribly undefined behaviour and eventually it'll die on an -EFAULT in
execbuf, but does anything else bad happen?


We don't see an EFAULT unless a miracle occurs, and the stale pages
continue to be read/written by other processes (as well as the client).
Horribly undefined behaviour with a misinformation leak.


What other processes? Pages will still be referenced so won't be reused 
so there is not information leak across unrelated processes. Unless you 
meant ones involved in object sharing?


But we could improve the revoke mechanism I suppose by marking the 
object and then revoking it at the next opportunity?


Regards,

Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 0/7] Enable SVM for Intel VT-d

2015-10-12 Thread Oded Gabbay
On Sat, Oct 10, 2015 at 4:17 PM, David Woodhouse  wrote:
>
> On Fri, 2015-10-09 at 00:50 +0100, David Woodhouse wrote:
> > This patch set enables PASID support for the Intel IOMMU, along with
> > page request support.
> >
> > Like its AMD counterpart, it exposes an IOMMU-specific API. I believe
> > we'll have a session at the Kernel Summit later this month in which we
> > can work out a generic API which will cover the two (now) existing
> > implementations as well as upcoming ARM (and other?) versions.
> >
> > For the time being, however, exposing an Intel-specific API is good
> > enough, especially as we don't have the required TLP prefix support on
> > our PCIe root ports and we *can't* support discrete PCIe devices with
> > PASID support. It's purely on-chip stuff right now, which is basically
> > only Intel graphics.
> >
> > The AMD implementation allows a per-device PASID space, and managing
> > the PASID space is left entirely to the device driver. In contrast,
> > this implementation maintains a per-IOMMU PASID space, and drivers
> > calling intel_svm_bind_mm() will be *given* the PASID that they are to
> > use. In general we seem to be converging on using a single PASID space
> > across *all* IOMMUs in the system, and this will support that mode of
> > operation.
>
> The other noticeable difference is the lifetime management of the mm.
> My code takes a reference on it, and will only do the mmput() when the
> driver unbinds the PASID. So the mmu_notifier's .release() method won't
> get called before that.
>
> The AMD version doesn't take that refcount, and its .release() method
> therefore needs to actually call back into the device driver and ensure
> that all access to the mm, including pending page faults, is flushed.
> The locking issues there scare me a little, especially if page faults
> are currently outstanding.
>
> In the i915 case we have an open file descriptor associated with the
> gfx context. When the process dies, the fd is closed and the driver can
> go and clean up after it.
>
> The amdkfd driver, on the other hand, keeps the device-side job running
> even after the process has closed its file descriptor. So it *needs*
> the .release() call to happen when the process exits, as it otherwise
> doesn't know when to clean up.
>
> I am somewhat dubious about that as a design decision. If we're moving
> to a more explicit management of off-cpu tasks with mm access, as is to
> be discussed at the Kernel Summit, then hopefully we can fix that. It's
> a *lot* simpler if we just pin the mm while the device context has
> access to it.
>
> --
> dwmw2
>

Hi David,

There was a whole debate about this issue (amdkfd binding to mm struct
lifespan instead of to fd) when we upstreamed amdkfd, with good
arguments for and against. If you want to understand the reasons, I
suggest reading the following email thread:

https://lists.linuxfoundation.org/pipermail/iommu/2014-July/009005.html

TL;DR, IIRC, the bottom line was that (over-simplified):

1. HSA/amdkfd is not a "classic" device driver, is it performs
operations in context of a process working on multiple devices and
doesn't contain an "instance per device". It's conceptually more like
a subsystem/system call interface then a device driver.

2. It is not a one-of-a-kind in the kernel, as there are other drivers
which use this method.

Oded
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] v4.3-rc4: i915: ThinkPad Yoga 12: *ERROR* The master control interrupt lied (SDE)!

2015-10-12 Thread Darren Hart
The Debian 3.16.0 kernel does not emit the error, but I have not attempted a
bisection.

The warning was added by:
38cc46d drm/i915/bdw: Ack interrupts before handling them (GEN8)
 2014-06-18 (1 year, 4 months ago), Oscar Mateo 

Follows: v3.15-rc8
Preceedes: 3.17-rc1

This is not present in v3.16, so perhaps not present in the Debian kernel and
this is not a regression, but just reporting the condition as intended.

Linux Version: v4.3-rc4
Platform: Lenovo ThinkPad Yoga 12
OS: Debian 8.2

$ dmesg | grep -i drm
[   10.918367] [drm] Initialized drm 1.1.0 20060810
[   11.235005] [drm] Memory usable by graphics device = 4096M
[   11.235009] fb: switching to inteldrmfb from simple
[   11.235093] [drm] Replacing VGA console driver
[   11.241160] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[   11.241162] [drm] Driver supports precise vblank timestamp query.
[   11.256249] [drm:drm_calc_timestamping_constants [drm]] *ERROR* crtc 21: 
Can't calculate constants, dotclock = 0!
[   11.326946] [drm] GMBUS [i915 gmbus dpb] timed out, falling back to bit 
banging on pin 5
[   11.344097] [drm] Initialized i915 1.6.0 20150731 for :00:02.0 on minor 0
[   11.344913] fbcon: inteldrmfb (fb0) is primary device
[   12.462509] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
interrupt lied (SDE)!
[   12.466498] i915 :00:02.0: fb0: inteldrmfb frame buffer device
[   12.794359] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
interrupt lied (SDE)!
[   13.869733] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
interrupt lied (SDE)!
[   13.869894] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
interrupt lied (SDE)!
[   13.870058] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
interrupt lied (SDE)!
[   22.656986] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
interrupt lied (SDE)!
[  257.506246] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
interrupt lied (SDE)!
[  257.506415] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
interrupt lied (SDE)!
[  257.506584] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
interrupt lied (SDE)!
[  257.506746] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
interrupt lied (SDE)!
[  278.722570] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
interrupt lied (SDE)!
[  278.722744] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
interrupt lied (SDE)!
[  278.722908] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
interrupt lied (SDE)!
[ 1857.776390] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
interrupt lied (SDE)!
[ 1857.776549] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
interrupt lied (SDE)!
[ 1857.776710] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
interrupt lied (SDE)!
[ 4057.592685] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
interrupt lied (SDE)!

-- 
Darren Hart
Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2] drm/i915: Improve kernel-doc for i915_audio_component struct

2015-10-12 Thread David Henningsson



On 2015-10-12 10:07, David Henningsson wrote:

To make kernel-doc happy, the i915_audio_component_audio_ops struct
cannot be nested.

Signed-off-by: David Henningsson 
---


Changes since v1:

 * Added a notice about when pin_eld_notify is called

 * Uses new inline struct member style

Verified with "make htmldocs", looks fine to me.

Also, applies to Takashi's tree (master branch), maybe we should take it 
through Takashi's tree to avoid conflicts, especially as there might be 
more stuff coming that way (dp mst support from Mengdong, and perhaps 
info retrieval from me).




  Documentation/DocBook/drm.tmpl |  1 +
  include/drm/i915_component.h   | 92 ++
  2 files changed, 67 insertions(+), 26 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 9ddf8c6..f16e4e8 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -4051,6 +4051,7 @@ int num_ioctls;
High Definition Audio
  !Pdrivers/gpu/drm/i915/intel_audio.c High Definition Audio over HDMI and 
Display Port
  !Idrivers/gpu/drm/i915/intel_audio.c
+!Iinclude/drm/i915_component.h


Panel Self Refresh PSR (PSR/SRD)
diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
index 89dc7d6..76c10c8 100644
--- a/include/drm/i915_component.h
+++ b/include/drm/i915_component.h
@@ -30,38 +30,78 @@
   */
  #define MAX_PORTS 5

+/**
+ * struct i915_audio_component_ops - Ops implemented by i915 driver, called by 
hda driver
+ */
+struct i915_audio_component_ops {
+   /**
+* @owner: i915 module
+*/
+   struct module *owner;
+   /**
+* @get_power: Request that power well is to be turned on
+*/
+   void (*get_power)(struct device *);
+   /**
+* @put_power: Allow the power well to be turned off
+*/
+   void (*put_power)(struct device *);
+   /**
+* @codec_wake_override: Force the audio codec to stay awake
+*/
+   void (*codec_wake_override)(struct device *, bool enable);
+   /**
+* @get_cdclk_freq: Query the i915 driver about the current cdclk 
frequency
+*/
+   int (*get_cdclk_freq)(struct device *);
+   /**
+* @sync_audio_rate: set n/cts based on the sample rate
+*
+* Called from audio driver. After audio driver sets the
+* sample rate, it will call this function to set n/cts
+*/
+   int (*sync_audio_rate)(struct device *, int port, int rate);
+};
+
+/**
+ * struct i915_audio_component_audio_ops - Ops implemented by hda driver, 
called by i915 driver
+ */
+struct i915_audio_component_audio_ops {
+   /**
+* @audio_ptr: Pointer to be used in call to pin_eld_notify
+*/
+   void *audio_ptr;
+   /**
+* @pin_eld_notify: Notify the HDA driver that pin sense and/or ELD 
information has changed
+*
+* Called when the i915 driver has set up audio pipeline or has just
+* begun to tear it down. This allows the HDA driver to update its
+* status accordingly (even when the HDA controller is in power save
+* mode).
+*/
+   void (*pin_eld_notify)(void *audio_ptr, int port);
+};
+
+/**
+ * struct i915_audio_component - Used for direct communication between i915 
and hda drivers
+ */
  struct i915_audio_component {
+   /**
+* @dev: i915 device, used as parameter for ops
+*/
struct device *dev;
/**
 * @aud_sample_rate: the array of audio sample rate per port
 */
int aud_sample_rate[MAX_PORTS];
-
-   const struct i915_audio_component_ops {
-   struct module *owner;
-   void (*get_power)(struct device *);
-   void (*put_power)(struct device *);
-   void (*codec_wake_override)(struct device *, bool enable);
-   int (*get_cdclk_freq)(struct device *);
-   /**
-* @sync_audio_rate: set n/cts based on the sample rate
-*
-* Called from audio driver. After audio driver sets the
-* sample rate, it will call this function to set n/cts
-*/
-   int (*sync_audio_rate)(struct device *, int port, int rate);
-   } *ops;
-
-   const struct i915_audio_component_audio_ops {
-   void *audio_ptr;
-   /**
-* Call from i915 driver, notifying the HDA driver that
-* pin sense and/or ELD information has changed.
-* @audio_ptr:  HDA driver object
-* @port:   Which port has changed (PORTA / PORTB / 
PORTC etc)
-*/
-   void (*pin_eld_notify)(void *audio_ptr, int port);
-   } *audio_ops;
+   /**
+* @ops: Ops implemented by i915 driver, called by hda driver
+*/
+   const struct i915_audio_component_ops *ops;
+  

[Intel-gfx] [PATCH v2] drm/i915: Improve kernel-doc for i915_audio_component struct

2015-10-12 Thread David Henningsson
To make kernel-doc happy, the i915_audio_component_audio_ops struct
cannot be nested.

Signed-off-by: David Henningsson 
---
 Documentation/DocBook/drm.tmpl |  1 +
 include/drm/i915_component.h   | 92 ++
 2 files changed, 67 insertions(+), 26 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 9ddf8c6..f16e4e8 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -4051,6 +4051,7 @@ int num_ioctls;
High Definition Audio
 !Pdrivers/gpu/drm/i915/intel_audio.c High Definition Audio over HDMI and 
Display Port
 !Idrivers/gpu/drm/i915/intel_audio.c
+!Iinclude/drm/i915_component.h
   
   
Panel Self Refresh PSR (PSR/SRD)
diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
index 89dc7d6..76c10c8 100644
--- a/include/drm/i915_component.h
+++ b/include/drm/i915_component.h
@@ -30,38 +30,78 @@
  */
 #define MAX_PORTS 5
 
+/**
+ * struct i915_audio_component_ops - Ops implemented by i915 driver, called by 
hda driver
+ */
+struct i915_audio_component_ops {
+   /**
+* @owner: i915 module
+*/
+   struct module *owner;
+   /**
+* @get_power: Request that power well is to be turned on
+*/
+   void (*get_power)(struct device *);
+   /**
+* @put_power: Allow the power well to be turned off
+*/
+   void (*put_power)(struct device *);
+   /**
+* @codec_wake_override: Force the audio codec to stay awake
+*/
+   void (*codec_wake_override)(struct device *, bool enable);
+   /**
+* @get_cdclk_freq: Query the i915 driver about the current cdclk 
frequency
+*/
+   int (*get_cdclk_freq)(struct device *);
+   /**
+* @sync_audio_rate: set n/cts based on the sample rate
+*
+* Called from audio driver. After audio driver sets the
+* sample rate, it will call this function to set n/cts
+*/
+   int (*sync_audio_rate)(struct device *, int port, int rate);
+};
+
+/**
+ * struct i915_audio_component_audio_ops - Ops implemented by hda driver, 
called by i915 driver
+ */
+struct i915_audio_component_audio_ops {
+   /**
+* @audio_ptr: Pointer to be used in call to pin_eld_notify
+*/
+   void *audio_ptr;
+   /**
+* @pin_eld_notify: Notify the HDA driver that pin sense and/or ELD 
information has changed
+*
+* Called when the i915 driver has set up audio pipeline or has just
+* begun to tear it down. This allows the HDA driver to update its
+* status accordingly (even when the HDA controller is in power save
+* mode).
+*/
+   void (*pin_eld_notify)(void *audio_ptr, int port);
+};
+
+/**
+ * struct i915_audio_component - Used for direct communication between i915 
and hda drivers
+ */
 struct i915_audio_component {
+   /**
+* @dev: i915 device, used as parameter for ops
+*/
struct device *dev;
/**
 * @aud_sample_rate: the array of audio sample rate per port
 */
int aud_sample_rate[MAX_PORTS];
-
-   const struct i915_audio_component_ops {
-   struct module *owner;
-   void (*get_power)(struct device *);
-   void (*put_power)(struct device *);
-   void (*codec_wake_override)(struct device *, bool enable);
-   int (*get_cdclk_freq)(struct device *);
-   /**
-* @sync_audio_rate: set n/cts based on the sample rate
-*
-* Called from audio driver. After audio driver sets the
-* sample rate, it will call this function to set n/cts
-*/
-   int (*sync_audio_rate)(struct device *, int port, int rate);
-   } *ops;
-
-   const struct i915_audio_component_audio_ops {
-   void *audio_ptr;
-   /**
-* Call from i915 driver, notifying the HDA driver that
-* pin sense and/or ELD information has changed.
-* @audio_ptr:  HDA driver object
-* @port:   Which port has changed (PORTA / PORTB / 
PORTC etc)
-*/
-   void (*pin_eld_notify)(void *audio_ptr, int port);
-   } *audio_ops;
+   /**
+* @ops: Ops implemented by i915 driver, called by hda driver
+*/
+   const struct i915_audio_component_ops *ops;
+   /**
+* @audio_ops: Ops implemented by hda driver, called by i915 driver
+*/
+   const struct i915_audio_component_audio_ops *audio_ops;
 };
 
 #endif /* _I915_COMPONENT_H_ */
-- 
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] v4.3-rc4: i915: ThinkPad Yoga 12: *ERROR* The master control interrupt lied (SDE)! [regression]

2015-10-12 Thread Daniel Vetter
Another regression for Jairo to track.
-Daniel

On Sat, Oct 10, 2015 at 12:08:43PM -0700, Darren Hart wrote:
> The Debian 3.16.0 kernel does not emit the error, but I have not attempted a
> bisection.
> 
> The warning was added by:
> 38cc46d drm/i915/bdw: Ack interrupts before handling them (GEN8)
>  2014-06-18 (1 year, 4 months ago), Oscar Mateo 
> 
> Follows: v3.15-rc8
> Preceedes: 3.17-rc1
> 
> This is not present in v3.16, so perhaps not present in the Debian kernel and
> this is not a regression, but just reporting the condition as intended.
> 
> Linux Version: v4.3-rc4
> Platform: Lenovo ThinkPad Yoga 12
> OS: Debian 8.2
> 
> $ dmesg | grep -i drm
> [   10.918367] [drm] Initialized drm 1.1.0 20060810
> [   11.235005] [drm] Memory usable by graphics device = 4096M
> [   11.235009] fb: switching to inteldrmfb from simple
> [   11.235093] [drm] Replacing VGA console driver
> [   11.241160] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [   11.241162] [drm] Driver supports precise vblank timestamp query.
> [   11.256249] [drm:drm_calc_timestamping_constants [drm]] *ERROR* crtc 21: 
> Can't calculate constants, dotclock = 0!
> [   11.326946] [drm] GMBUS [i915 gmbus dpb] timed out, falling back to bit 
> banging on pin 5
> [   11.344097] [drm] Initialized i915 1.6.0 20150731 for :00:02.0 on 
> minor 0
> [   11.344913] fbcon: inteldrmfb (fb0) is primary device
> [   12.462509] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [   12.466498] i915 :00:02.0: fb0: inteldrmfb frame buffer device
> [   12.794359] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [   13.869733] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [   13.869894] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [   13.870058] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [   22.656986] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [  257.506246] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [  257.506415] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [  257.506584] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [  257.506746] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [  278.722570] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [  278.722744] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [  278.722908] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [ 1857.776390] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [ 1857.776549] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [ 1857.776710] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [ 4057.592685] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> 
> -- 
> Darren Hart
> Intel Open Source Technology Center

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] v4.3-rc4: i915: ThinkPad Yoga 12: *ERROR* The master control interrupt lied (SDE)!

2015-10-12 Thread Jani Nikula
On Sat, 10 Oct 2015, Darren Hart  wrote:
> The Debian 3.16.0 kernel does not emit the error, but I have not attempted a
> bisection.
>
> The warning was added by:
> 38cc46d drm/i915/bdw: Ack interrupts before handling them (GEN8)
>  2014-06-18 (1 year, 4 months ago), Oscar Mateo 

But we don't start hitting the warning right away with that commit, do
we? There's a bug about this with a bisected bad commit [1], please
let's track this there.

Thanks,
Jani.


[1] https://bugs.freedesktop.org/show_bug.cgi?id=92084


>
> Follows: v3.15-rc8
> Preceedes: 3.17-rc1
>
> This is not present in v3.16, so perhaps not present in the Debian kernel and
> this is not a regression, but just reporting the condition as intended.
>
> Linux Version: v4.3-rc4
> Platform: Lenovo ThinkPad Yoga 12
> OS: Debian 8.2
>
> $ dmesg | grep -i drm
> [   10.918367] [drm] Initialized drm 1.1.0 20060810
> [   11.235005] [drm] Memory usable by graphics device = 4096M
> [   11.235009] fb: switching to inteldrmfb from simple
> [   11.235093] [drm] Replacing VGA console driver
> [   11.241160] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [   11.241162] [drm] Driver supports precise vblank timestamp query.
> [   11.256249] [drm:drm_calc_timestamping_constants [drm]] *ERROR* crtc 21: 
> Can't calculate constants, dotclock = 0!
> [   11.326946] [drm] GMBUS [i915 gmbus dpb] timed out, falling back to bit 
> banging on pin 5
> [   11.344097] [drm] Initialized i915 1.6.0 20150731 for :00:02.0 on 
> minor 0
> [   11.344913] fbcon: inteldrmfb (fb0) is primary device
> [   12.462509] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [   12.466498] i915 :00:02.0: fb0: inteldrmfb frame buffer device
> [   12.794359] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [   13.869733] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [   13.869894] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [   13.870058] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [   22.656986] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [  257.506246] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [  257.506415] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [  257.506584] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [  257.506746] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [  278.722570] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [  278.722744] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [  278.722908] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [ 1857.776390] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [ 1857.776549] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [ 1857.776710] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
> [ 4057.592685] [drm:gen8_irq_handler [i915]] *ERROR* The master control 
> interrupt lied (SDE)!
>
> -- 
> Darren Hart
> Intel Open Source Technology Center

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] 4.2-rc4 kernel warnings on HSW laptop [regression]

2015-10-12 Thread Daniel Vetter
Another pile of regressions for Jairo to track ...

On Sat, Oct 10, 2015 at 11:46:29AM +0200, Takashi Iwai wrote:
> Hi,
> 
> I noticed that a HSW laptop gets a few new warnings since 4.2-rc
> kernels.  One error messages pops at each boot time:
> 
>  Console: switching to colour dummy device 80x25
>  [drm] Replacing VGA console driver
>  [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
>  [drm] Driver supports precise vblank timestamp query.
>  vgaarb: device changed decodes: 
> PCI::00:02.0,olddecodes=io+mem,decodes=io+mem:owns=io+mem
>  [drm:drm_calc_timestamping_constants [drm]] *ERROR* crtc 21: Can't calculate 
> constants, dotclock = 0!

There's patches for this one already, but I accidentally applied them for
-next, not -fixes. They should land for -rc6.

> Then a warning when I start powertop:
> 
>  WARNING: CPU: 1 PID: 1674 at drivers/gpu/drm/drm_atomic.c:889 
> drm_atomic_get_property+0x232/0x2b0 [drm]()
>  CPU: 1 PID: 1674 Comm: powertop Tainted: GW   4.3.0-rc4-test+ #93
>  Hardware name: Hewlett-Packard HP ProBook 430 G1/1946, BIOS L73 Ver. 08.05 
> 2013/03/15
>   a005b289 880059173ca8 81346fa9 
>   880059173ce0 81063232 8800372bc028 8800373f3740
>   8800372bc000 880062ad37c0 0001 880059173cf0
>  Call Trace:
>   [] dump_stack+0x4b/0x72
>   [] warn_slowpath_common+0x82/0xc0
>   [] warn_slowpath_null+0x1a/0x20
>   [] drm_atomic_get_property+0x232/0x2b0 [drm]
>   [] drm_object_property_get_value+0x6c/0x70 [drm]
>   [] dpms_show+0x2f/0x70 [drm]
>   [] dev_attr_show+0x20/0x50
>   [] sysfs_kf_seq_show+0xbc/0x130
>   [] kernfs_seq_show+0x23/0x30
>   [] seq_read+0xca/0x360
>   [] kernfs_fop_read+0x10a/0x160
>   [] __vfs_read+0x28/0xd0
>   [] ? security_file_permission+0xa0/0xc0
>   [] ? rw_verify_area+0x4f/0xe0
>   [] vfs_read+0x83/0x130
>   [] SyS_read+0x46/0xa0
>   [] ? syscall_return_slowpath+0x50/0x130
>   [] entry_SYSCALL_64_fastpath+0x16/0x75
> 
> 
> Both don't look like serious issues, but not sexy to see at each time.

This should be fixed with

commit 621bd0f6982badd6483acb191eb7b6226a578328
Author: Daniel Vetter 
Date:   Tue Sep 29 09:56:53 2015 +0200

drm: Fix locking for sysfs dpms file

If not please scream.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx