Re: [Intel-gfx] Request Linux Graphic Driver for Intel GMA 3150

[Matt Turner]

On Wed, Sep 2, 2015 at 9:01 PM, David Ho  wrote:
> Dear Rodrigo,
>
> Thank you for your help.
>
> I'll take a look at the PCI ID.
>
> However,
> when I search GMA 3150 at 01.org, I came across this page:
> https://01.org/linuxgraphics/downloads/2013q1-intel-graphics-stack-release
>
> and also this page at Intel.com
> http://www.intel.com/support/graphics/sb/cs-010512.htm
>
> it seems that the driver of GMA 3150 for older Ubuntu version is available 
> (supported by open source driver).

Wikipedia says GMA 3150 is just a "Pineview" [0]

If so, it's supported by both the i915 kernel and Mesa drivers. But
grep Mesa's include/pci_ids/ to be sure that's what you have.

[0] https://en.wikipedia.org/wiki/Intel_GMA#GMA_3150
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] Request Linux Graphic Driver for Intel GMA 3150

[David Ho]

Dear Rodrigo,

Thank you for your help.

I'll take a look at the PCI ID.

However,
when I search GMA 3150 at 01.org, I came across this page:
https://01.org/linuxgraphics/downloads/2013q1-intel-graphics-stack-release

and also this page at Intel.com
http://www.intel.com/support/graphics/sb/cs-010512.htm

it seems that the driver of GMA 3150 for older Ubuntu version is available 
(supported by open source driver).

Regards,
David


-Original Message-
From: Vivi, Rodrigo [mailto:rodrigo.v...@intel.com] 
Sent: 03 September 2015 9:50
To: matts...@gmail.com
Cc: intel-gfx@lists.freedesktop.org; jani.nik...@linux.intel.com; 
hupernikao...@gmail.com
Subject: Re: [Intel-gfx] Request Linux Graphic Driver for Intel GMA 3150

On Wed, 2015-09-02 at 18:09 -0700, Matt Turner wrote:
> On Wed, Sep 2, 2015 at 10:37 AM, Vivi, Rodrigo < 
> rodrigo.v...@intel.com> wrote:
> > On Wed, 2015-09-02 at 18:16 +0700, David Ho wrote:
> > > Dear Rodrigo,
> > 
> > Hi David,
> > 
> > I just paid attention to the subject and notice you are looking for 
> > driver for GMA 3150. I'm not sure, but I'm afraid this platform 
> > doesn't have the GPU supported by our open source driver.
> > 
> > Probably the GMA 3150 will be in the same bucket as GMA 3600 series 
> > and the official statement is at 
> > https://01.org/linuxgraphics/documentation/faq
> 
> The link in the FAQ
> (http://download.meego.com/live/MeeGo:/1.2.0:/CedarTrail:/non-oss)
> doesn't even work, so that's no good.

So, FAQ updated without any link.

Thanks Matt.

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

Re: [Intel-gfx] Request Linux Graphic Driver for Intel GMA 3150

[Vivi, Rodrigo]

On Wed, 2015-09-02 at 18:09 -0700, Matt Turner wrote:
> On Wed, Sep 2, 2015 at 10:37 AM, Vivi, Rodrigo <
> rodrigo.v...@intel.com> wrote:
> > On Wed, 2015-09-02 at 18:16 +0700, David Ho wrote:
> > > Dear Rodrigo,
> > 
> > Hi David,
> > 
> > I just paid attention to the subject and notice you are looking for
> > driver for GMA 3150. I'm not sure, but I'm afraid this platform
> > doesn't
> > have the GPU supported by our open source driver.
> > 
> > Probably the GMA 3150 will be in the same bucket as GMA 3600 series
> > and
> > the official statement is at
> > https://01.org/linuxgraphics/documentation/faq
> 
> The link in the FAQ
> (http://download.meego.com/live/MeeGo:/1.2.0:/CedarTrail:/non-oss)
> doesn't even work, so that's no good.

So, FAQ updated without any link.

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

Re: [Intel-gfx] Request Linux Graphic Driver for Intel GMA 3150

[Matt Turner]

On Wed, Sep 2, 2015 at 10:37 AM, Vivi, Rodrigo  wrote:
> On Wed, 2015-09-02 at 18:16 +0700, David Ho wrote:
>> Dear Rodrigo,
>
> Hi David,
>
> I just paid attention to the subject and notice you are looking for
> driver for GMA 3150. I'm not sure, but I'm afraid this platform doesn't
> have the GPU supported by our open source driver.
>
> Probably the GMA 3150 will be in the same bucket as GMA 3600 series and
> the official statement is at
> https://01.org/linuxgraphics/documentation/faq

The link in the FAQ
(http://download.meego.com/live/MeeGo:/1.2.0:/CedarTrail:/non-oss)
doesn't even work, so that's no good.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] linux-next: build failure after merge of the drm-misc tree

[Stephen Rothwell]

Hi all,

After merging the drm-misc tree, today's linux-next build (arm
multi_v7_defconfig) failed like this:

drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c: In function 'mdp5_plane_cleanup_fb':
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c:272:26: error: 'fb' redeclared as 
different kind of symbol
  struct drm_framebuffer *fb = old_state->fb;
  ^
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c:267:27: note: previous definition of 
'fb' was here
   struct drm_framebuffer *fb,
   ^
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c: At top level:
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c:383:3: warning: initialization from 
incompatible pointer type
   .cleanup_fb = mdp5_plane_cleanup_fb,
   ^
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c:383:3: warning: (near initialization 
for 'mdp5_plane_helper_funcs.cleanup_fb')

Caused by commit

  a317290af0db ("drm/atomic: Make prepare_fb/cleanup_fb only take state, v3")

Well, that was clearly never build tested :-(

I have used the drm-misc tree from next-20150902 for today.

-- 
Cheers,
Stephen Rothwells...@canb.auug.org.au
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Add GuC css header parser

[yu . dai]

From: Alex Dai 

By using information from GuC css header, we can eliminate some
hard code w.r.t size of some components of firmware.

Signed-off-by: Alex Dai 
---
 drivers/gpu/drm/i915/intel_guc.h|  2 +-
 drivers/gpu/drm/i915/intel_guc_fwif.h   | 36 +
 drivers/gpu/drm/i915/intel_guc_loader.c | 91 ++---
 3 files changed, 98 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 4ec2d27..e1389fc 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -71,6 +71,7 @@ struct intel_guc_fw {
struct drm_i915_gem_object *guc_fw_obj;
enum intel_guc_fw_statusguc_fw_fetch_status;
enum intel_guc_fw_statusguc_fw_load_status;
+   struct guc_css_header   guc_fw_header;
 
uint16_tguc_fw_major_wanted;
uint16_tguc_fw_minor_wanted;
@@ -80,7 +81,6 @@ struct intel_guc_fw {
 
 struct intel_guc {
struct intel_guc_fw guc_fw;
-
uint32_t log_flags;
struct drm_i915_gem_object *log_obj;
 
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index e1f47ba..d6cb4e8 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -122,6 +122,42 @@
 
 #define GUC_CTL_MAX_DWORDS (GUC_CTL_RSRVD + 1)
 
+struct guc_css_header {
+   uint32_t module_type;
+   uint32_t header_len; /* header length plus size of all other keys */
+   uint32_t header_version;
+   uint32_t module_id;
+   uint32_t module_vendor;
+   union {
+   struct {
+   uint8_t day;
+   uint8_t month;
+   uint16_t year;
+   };
+   uint32_t date;
+   };
+   uint32_t size; /* uCode size plus header_len */
+   uint32_t key_size;
+   uint32_t modulus_size;
+   uint32_t exponent_size;
+   union {
+   struct {
+   uint8_t hour;
+   uint8_t min;
+   uint16_t sec;
+   };
+   uint32_t time;
+   };
+
+   char username[8];
+   char buildnumber[12];
+   uint32_t device_id;
+   uint32_t guc_sw_version;
+   uint32_t prod_preprod_fw;
+   uint32_t reserved[12];
+   uint32_t header_info;
+};
+
 struct guc_doorbell_info {
u32 db_status;
u32 cookie;
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
b/drivers/gpu/drm/i915/intel_guc_loader.c
index 5823615..96826ae 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -215,18 +215,24 @@ static inline bool guc_ucode_response(struct 
drm_i915_private *dev_priv,
((val & GS_MIA_CORE_STATE) && uk_val == GS_UKERNEL_LAPIC_DONE));
 }
 
-/*
+/**
  * Transfer the firmware image to RAM for execution by the microcontroller.
  *
  * GuC Firmware layout:
- * +---+  
- * |  CSS header   |  128B
+ * +---+
+ * |guc_css_header |
  * | contains major/minor version  |
- * +---+  
+ * +---+
  * | uCode |
- * +---+  
- * | RSA signature |  256B
- * +---+  
+ * +---+
+ * | RSA signature |
+ * +---+
+ * |  modulus key  |
+ * +---+
+ * |  exponent val |
+ * +---+
+ * The firmware may or may not have modulus key and exponent data. The header,
+ * uCode and RSA signature are must-have components that will be used by 
driver.
  *
  * Architecturally, the DMA engine is bidirectional, and can potentially even
  * transfer between GTT locations. This functionality is left out of the API
@@ -236,30 +242,39 @@ static inline bool guc_ucode_response(struct 
drm_i915_private *dev_priv,
  * DMA engine in one operation, whereas the RSA signature is loaded via MMIO.
  */
 
-#define UOS_CSS_HEADER_OFFSET  0
-#define UOS_VER_MINOR_OFFSET   0x44
-#define UOS_VER_MAJOR_OFFSET   0x46
-#define UOS_CSS_HEADER_SIZE0x80
-#define UOS_RSA_SIG_SIZE   0x100
-
 static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv)
 {
struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
+   struct guc_css_header *header = &guc_fw->guc_fw_header;
struct drm_i915_gem_object *fw_obj = guc_fw->guc_fw_obj;
unsigned long offset;
struct sg_table *sg = fw_obj->pages;
-   u32 status, ucode_size, rsa[UOS_RSA_SIG_SIZE / sizeof(u32)];
+   u32 status, header_size, rsa_size, ucode_size, *rsa;
int i, ret

[Intel-gfx] [PATCH] drm/i915: Fix a bug in GuC status check

[yu . dai]

From: Alex Dai 

Bit 16 of GuC status indicates resuming from RC6. The LAPIC_DONE
status is a reliable readiness flag only when resuming from RC6.
This fix a racing issue that allocation of doorbell fails whilst
GuC init is not finished.

Signed-off-by: Alex Dai 
---
 drivers/gpu/drm/i915/i915_guc_reg.h | 1 +
 drivers/gpu/drm/i915/intel_guc_loader.c | 5 +++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_reg.h 
b/drivers/gpu/drm/i915/i915_guc_reg.h
index 8c8e574..dd0e1e8 100644
--- a/drivers/gpu/drm/i915/i915_guc_reg.h
+++ b/drivers/gpu/drm/i915/i915_guc_reg.h
@@ -37,6 +37,7 @@
 #define   GS_UKERNEL_READY   (0xF0 << GS_UKERNEL_SHIFT)
 #define   GS_MIA_SHIFT 16
 #define   GS_MIA_MASK(0x07 << GS_MIA_SHIFT)
+#define   GS_MIA_CORE_STATE  (1 << GS_MIA_SHIFT)
 
 #define SOFT_SCRATCH(n)(0xc180 + ((n) * 4))
 
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
b/drivers/gpu/drm/i915/intel_guc_loader.c
index 5eafd31..5823615 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -209,9 +209,10 @@ static inline bool guc_ucode_response(struct 
drm_i915_private *dev_priv,
  u32 *status)
 {
u32 val = I915_READ(GUC_STATUS);
+   u32 uk_val = val & GS_UKERNEL_MASK;
*status = val;
-   return ((val & GS_UKERNEL_MASK) == GS_UKERNEL_READY ||
-   (val & GS_UKERNEL_MASK) == GS_UKERNEL_LAPIC_DONE);
+   return (uk_val == GS_UKERNEL_READY ||
+   ((val & GS_MIA_CORE_STATE) && uk_val == GS_UKERNEL_LAPIC_DONE));
 }
 
 /*
-- 
1.9.1

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

[Intel-gfx] Water mark update need to wait for next VSYNC?

[Xie, William]

Hi all,
 Can anyone educate me if water mark update need to wait for next VSYNC?
In other words, if we flip a frame to overlay for the first time,
it will be showed in the next VBlank as water mark update needs to wait for 
that?

Is this true or a bug?

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

[Intel-gfx] watermark problem in kernel 3.14

[Xie, William]

Hi all,
We suspect watermark has problem in kernel 3.14,
Does anyone have a new watermark patch for 3.14 similar as below patch:
http://patchwork.freedesktop.org/bundle/anderco/matt-watermarks/


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

[Intel-gfx] [PATCH 2/3] drm/i915: Future proof uncore_init.

[Rodrigo Vivi]

Unless future specs tells otherwise we can assume future gens
inherit some stuff from the previous so let's handle
missed cases when we know tehy should't be there and assume
default equals newest one.

No functional changes.

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

diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
b/drivers/gpu/drm/i915/intel_uncore.c
index dec20d6..e633d36 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1202,8 +1202,6 @@ void intel_uncore_init(struct drm_device *dev)
 
switch (INTEL_INFO(dev)->gen) {
default:
-   MISSING_CASE(INTEL_INFO(dev)->gen);
-   return;
case 9:
ASSIGN_WRITE_MMIO_VFUNCS(gen9);
ASSIGN_READ_MMIO_VFUNCS(gen9);
@@ -1242,6 +1240,10 @@ void intel_uncore_init(struct drm_device *dev)
ASSIGN_WRITE_MMIO_VFUNCS(gen2);
ASSIGN_READ_MMIO_VFUNCS(gen2);
break;
+   case 1:
+   case 0:
+   MISSING_CASE(INTEL_INFO(dev)->gen);
+   return;
}
 
if (intel_vgpu_active(dev)) {
-- 
2.4.3

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

[Intel-gfx] [PATCH 1/3] drm/i915: Future proof interrupt handler.

[Rodrigo Vivi]

These functions are already being called for gen >= 9,
so let's be sure when this happens we use whatever is
there already for the latest platform.

No functional change.

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

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 2063279..90bc6c2 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2154,7 +2154,7 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
if (!intel_irqs_enabled(dev_priv))
return IRQ_NONE;
 
-   if (IS_GEN9(dev))
+   if (INTEL_INFO(dev_priv)->gen >= 9)
aux_mask |=  GEN9_AUX_CHANNEL_B | GEN9_AUX_CHANNEL_C |
GEN9_AUX_CHANNEL_D;
 
@@ -2237,7 +2237,7 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
intel_pipe_handle_vblank(dev, pipe))
intel_check_page_flip(dev, pipe);
 
-   if (IS_GEN9(dev))
+   if (INTEL_INFO(dev_priv)->gen >= 9)
flip_done = pipe_iir & 
GEN9_PIPE_PLANE1_FLIP_DONE;
else
flip_done = pipe_iir & 
GEN8_PIPE_PRIMARY_FLIP_DONE;
@@ -2255,7 +2255,7 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
pipe);
 
 
-   if (IS_GEN9(dev))
+   if (INTEL_INFO(dev_priv)->gen >= 9)
fault_errors = pipe_iir & 
GEN9_DE_PIPE_IRQ_FAULT_ERRORS;
else
fault_errors = pipe_iir & 
GEN8_DE_PIPE_IRQ_FAULT_ERRORS;
@@ -3538,7 +3538,7 @@ static void gen8_de_irq_postinstall(struct 
drm_i915_private *dev_priv)
u32 de_port_enables;
enum pipe pipe;
 
-   if (IS_GEN9(dev_priv)) {
+   if (INTEL_INFO(dev_priv)->gen >= 9) {
de_pipe_masked |= GEN9_PIPE_PLANE1_FLIP_DONE |
  GEN9_DE_PIPE_IRQ_FAULT_ERRORS;
de_port_masked |= GEN9_AUX_CHANNEL_B | GEN9_AUX_CHANNEL_C |
-- 
2.4.3

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

[Intel-gfx] [PATCH 3/3] drm/i915: Future proof panel fitter.

[Rodrigo Vivi]

This is another case where we can consider the default is the
newest available and not actually a missed case.

No functional change.

Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/intel_display.c | 18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 0377520..5c62511 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4915,12 +4915,10 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 
intel_ddi_enable_pipe_clock(intel_crtc);
 
-   if (INTEL_INFO(dev)->gen == 9)
+   if (INTEL_INFO(dev)->gen >= 9)
skylake_pfit_enable(intel_crtc);
-   else if (INTEL_INFO(dev)->gen < 9)
-   ironlake_pfit_enable(intel_crtc);
else
-   MISSING_CASE(INTEL_INFO(dev)->gen);
+   ironlake_pfit_enable(intel_crtc);
 
/*
 * On ILK+ LUT must be loaded before the pipe is running but with
@@ -5052,12 +5050,10 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 
intel_ddi_disable_transcoder_func(dev_priv, cpu_transcoder);
 
-   if (INTEL_INFO(dev)->gen == 9)
+   if (INTEL_INFO(dev)->gen >= 9)
skylake_scaler_disable(intel_crtc);
-   else if (INTEL_INFO(dev)->gen < 9)
-   ironlake_pfit_disable(intel_crtc);
else
-   MISSING_CASE(INTEL_INFO(dev)->gen);
+   ironlake_pfit_disable(intel_crtc);
 
intel_ddi_disable_pipe_clock(intel_crtc);
 
@@ -9780,12 +9776,10 @@ static bool haswell_get_pipe_config(struct intel_crtc 
*crtc,
}
 
if (intel_display_power_is_enabled(dev_priv, pfit_domain)) {
-   if (INTEL_INFO(dev)->gen == 9)
+   if (INTEL_INFO(dev)->gen >= 9)
skylake_get_pfit_config(crtc, pipe_config);
-   else if (INTEL_INFO(dev)->gen < 9)
-   ironlake_get_pfit_config(crtc, pipe_config);
else
-   MISSING_CASE(INTEL_INFO(dev)->gen);
+   ironlake_get_pfit_config(crtc, pipe_config);
}
 
if (IS_HASWELL(dev))
-- 
2.4.3

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

Re: [Intel-gfx] [PATCH 2/2] drm/i915: restrict unclaimed register checking

[Paulo Zanoni]

2015-09-02 17:53 GMT-03:00 ch...@chris-wilson.co.uk :
> On Wed, Sep 02, 2015 at 08:20:52PM +, Zanoni, Paulo R wrote:
>> Em Qua, 2015-08-26 às 08:44 +0100, Chris Wilson escreveu:
>> > On Tue, Aug 25, 2015 at 07:03:42PM -0300, Paulo Zanoni wrote:
>> > > The unclaimed register bit is only triggered when someone touches
>> > > the
>> > > specified register range.
>> > >
>> > > For the normal use case (with i915.mmio_debug=0), this commit will
>> > > avoid the extra __raw_i915_read32() call for every register outside
>> > > the specified range, at the expense of a few additional "if"
>> > > statements.
>> > >
>> > > Cc: Chris Wilson 
>> > > Signed-off-by: Paulo Zanoni 
>> >
>> >
>> > > @@ -612,7 +614,7 @@ hsw_unclaimed_reg_debug(struct drm_i915_private
>> > > *dev_priv, u32 reg, bool read,
>> > >   const char *op = read ? "reading" : "writing to";
>> > >   const char *when = before ? "before" : "after";
>> > >
>> > > - if (!i915.mmio_debug)
>> > > + if (!i915.mmio_debug || !UNCLAIMED_CHECK_RANGE(reg))
>> > >   return;
>> >
>> > Place the register check on the preceding line so that it is not
>> > confused with checking the debug-enabled status. (Also you can argue
>> > that reg will be register/cache-hot and so a cheaper test to do first
>> > than loading a module parameter.)
>>
>> That makes sense, I'll do it.
>>
>> >
>> > >   if (__raw_i915_read32(dev_priv, FPGA_DBG) &
>> > > FPGA_DBG_RM_NOCLAIM) {
>> > > @@ -624,11 +626,11 @@ hsw_unclaimed_reg_debug(struct
>> > > drm_i915_private *dev_priv, u32 reg, bool read,
>> > >  }
>> > >
>> > >  static void
>> > > -hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv)
>> > > +hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv, u32
>> > > reg)
>> > >  {
>> > >   static bool mmio_debug_once = true;
>> > >
>> > > - if (i915.mmio_debug || !mmio_debug_once)
>> > > + if (i915.mmio_debug || !UNCLAIMED_CHECK_RANGE(reg) ||
>> > > !mmio_debug_once)
>> > >   return;
>> >
>> > The use is wrong here though because you never reviewed my patch to
>> > enable the single-shot debugging from the interrupt handler to catch
>> > invalid usage from elsewhere.
>>
>> If you're talking about intel_uncore_check_errors(), I think we can
>> just kill it now. I'll submit a patch with the arguments, so we can
>> continue this topic there.
>>
>> Moving back to the main subject:
>>
>> In the last time you sent the patch to do the unclaimed checks only on
>> forcewake code, I started the conversations with the HW guys about the
>> range of registers relevant by FPGA_DBG (and CCd you on these
>> conversations). My plan was to write this patch at that time, but
>> priorities happened and it got postponed :(. I also hoped that maybe
>> you would eventually end up writing it and I would just have to review
>> it :)
>>
>> My main argument was that by doing unclaimed register checking only at
>> forcewake time we would be running a check that is only relevant to
>> display code during non-display code. So my idea is that if we restrict
>> the unclaimed check to the actual range of registers that can be caught
>> we basically remove unclaimed register checking for most (all?) of the
>> performance-sensitive code.
>>
>> Since we have multiple solutions, I decided to do some experiments.
>> First of all, since I was not really seeing hsw_unclaimed_reg_detect()
>> on perf, I decided to patch it so it would do the read/write check
>> around FPGA_DBG 50 times per call instead of just one. With this, by
>> running "perf record glxgears" for a few seconds I could see
>> hsw_unclaimed_reg_detect() taking about 4.6% of the total time.
>
> Something is very suspect with your system if you are not observing much
> hsw_unclaimed_reg_check during trivial workloads.

Ok, I should have said "almost not seeing" instead of "not really
seeing": my bad. I see about 0.24% on perf with plain
drm-intel-nightly. The problem is that any of the next patches reduces
this down to 0.01% or 0.02%, so it's hard to compare the optimized
patches. By doing 50 reg reads on detect() I can make the numbers of
the optimized patches a little bigger, so easier to compare.

>
>> Then I applied just this patch, and the time went down to 0.13%. I also
>> applied your patch on top of it all, and it went up to 0.52% (I guess
>> the extra checks at forcewake time cost a little bit). Also notice that
>> since I add a "reg" argument to hsw_unclaimed_reg_detect(), I changed
>> the forcewake call to use a register in the display range.
>> I also tested your patch without my patch, and the measured result was
>> about 0.56%.
>>
>> Now this isn't a super relevant experiment: just glxgears with a
>> modified hsw_unclaimed_reg_detect(), but I thought it would be useful
>> information, and maybe you could provide me suggestions for better
>> workloads.
>>
>> So I'd like to know if you're ok with proceeding with just this patch
>> (considering I implement your suggestions), or if you think we should
>> go with just your patch o

Re: [Intel-gfx] [PATCH 2/2] drm/i915: restrict unclaimed register checking

[Paulo Zanoni]

2015-09-02 17:53 GMT-03:00 ch...@chris-wilson.co.uk :
> On Wed, Sep 02, 2015 at 08:20:52PM +, Zanoni, Paulo R wrote:
>> Em Qua, 2015-08-26 às 08:44 +0100, Chris Wilson escreveu:
>> > On Tue, Aug 25, 2015 at 07:03:42PM -0300, Paulo Zanoni wrote:
>> > > The unclaimed register bit is only triggered when someone touches
>> > > the
>> > > specified register range.
>> > >
>> > > For the normal use case (with i915.mmio_debug=0), this commit will
>> > > avoid the extra __raw_i915_read32() call for every register outside
>> > > the specified range, at the expense of a few additional "if"
>> > > statements.
>> > >
>> > > Cc: Chris Wilson 
>> > > Signed-off-by: Paulo Zanoni 
>> >
>> >
>> > > @@ -612,7 +614,7 @@ hsw_unclaimed_reg_debug(struct drm_i915_private
>> > > *dev_priv, u32 reg, bool read,
>> > >   const char *op = read ? "reading" : "writing to";
>> > >   const char *when = before ? "before" : "after";
>> > >
>> > > - if (!i915.mmio_debug)
>> > > + if (!i915.mmio_debug || !UNCLAIMED_CHECK_RANGE(reg))
>> > >   return;
>> >
>> > Place the register check on the preceding line so that it is not
>> > confused with checking the debug-enabled status. (Also you can argue
>> > that reg will be register/cache-hot and so a cheaper test to do first
>> > than loading a module parameter.)
>>
>> That makes sense, I'll do it.
>>
>> >
>> > >   if (__raw_i915_read32(dev_priv, FPGA_DBG) &
>> > > FPGA_DBG_RM_NOCLAIM) {
>> > > @@ -624,11 +626,11 @@ hsw_unclaimed_reg_debug(struct
>> > > drm_i915_private *dev_priv, u32 reg, bool read,
>> > >  }
>> > >
>> > >  static void
>> > > -hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv)
>> > > +hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv, u32
>> > > reg)
>> > >  {
>> > >   static bool mmio_debug_once = true;
>> > >
>> > > - if (i915.mmio_debug || !mmio_debug_once)
>> > > + if (i915.mmio_debug || !UNCLAIMED_CHECK_RANGE(reg) ||
>> > > !mmio_debug_once)
>> > >   return;
>> >
>> > The use is wrong here though because you never reviewed my patch to
>> > enable the single-shot debugging from the interrupt handler to catch
>> > invalid usage from elsewhere.
>>
>> If you're talking about intel_uncore_check_errors(), I think we can
>> just kill it now. I'll submit a patch with the arguments, so we can
>> continue this topic there.
>>
>> Moving back to the main subject:
>>
>> In the last time you sent the patch to do the unclaimed checks only on
>> forcewake code, I started the conversations with the HW guys about the
>> range of registers relevant by FPGA_DBG (and CCd you on these
>> conversations). My plan was to write this patch at that time, but
>> priorities happened and it got postponed :(. I also hoped that maybe
>> you would eventually end up writing it and I would just have to review
>> it :)
>>
>> My main argument was that by doing unclaimed register checking only at
>> forcewake time we would be running a check that is only relevant to
>> display code during non-display code. So my idea is that if we restrict
>> the unclaimed check to the actual range of registers that can be caught
>> we basically remove unclaimed register checking for most (all?) of the
>> performance-sensitive code.
>>
>> Since we have multiple solutions, I decided to do some experiments.
>> First of all, since I was not really seeing hsw_unclaimed_reg_detect()
>> on perf, I decided to patch it so it would do the read/write check
>> around FPGA_DBG 50 times per call instead of just one. With this, by
>> running "perf record glxgears" for a few seconds I could see
>> hsw_unclaimed_reg_detect() taking about 4.6% of the total time.
>
> Something is very suspect with your system if you are not observing much
> hsw_unclaimed_reg_check during trivial workloads.
>
>> Then I applied just this patch, and the time went down to 0.13%. I also
>> applied your patch on top of it all, and it went up to 0.52% (I guess
>> the extra checks at forcewake time cost a little bit). Also notice that
>> since I add a "reg" argument to hsw_unclaimed_reg_detect(), I changed
>> the forcewake call to use a register in the display range.
>> I also tested your patch without my patch, and the measured result was
>> about 0.56%.
>>
>> Now this isn't a super relevant experiment: just glxgears with a
>> modified hsw_unclaimed_reg_detect(), but I thought it would be useful
>> information, and maybe you could provide me suggestions for better
>> workloads.
>>
>> So I'd like to know if you're ok with proceeding with just this patch
>> (considering I implement your suggestions), or if you think we should
>> go with just your patch or both or none.
>
> If you really wanted to, you could combine approaches a check in the
> forcewake handler as demonstrated is an order of magnitude less frequent
> than the register accesses. The key point is that we *need* the checking
> against other users, not just our known register access.

I'm not sure I fully understood your point. We are still going to
check against errors made

Re: [Intel-gfx] [PATCH 2/2] drm/i915: restrict unclaimed register checking

[ch...@chris-wilson.co.uk]

On Wed, Sep 02, 2015 at 08:20:52PM +, Zanoni, Paulo R wrote:
> Em Qua, 2015-08-26 às 08:44 +0100, Chris Wilson escreveu:
> > On Tue, Aug 25, 2015 at 07:03:42PM -0300, Paulo Zanoni wrote:
> > > The unclaimed register bit is only triggered when someone touches 
> > > the
> > > specified register range.
> > > 
> > > For the normal use case (with i915.mmio_debug=0), this commit will
> > > avoid the extra __raw_i915_read32() call for every register outside
> > > the specified range, at the expense of a few additional "if"
> > > statements.
> > > 
> > > Cc: Chris Wilson 
> > > Signed-off-by: Paulo Zanoni 
> > 
> > 
> > > @@ -612,7 +614,7 @@ hsw_unclaimed_reg_debug(struct drm_i915_private 
> > > *dev_priv, u32 reg, bool read,
> > >   const char *op = read ? "reading" : "writing to";
> > >   const char *when = before ? "before" : "after";
> > >  
> > > - if (!i915.mmio_debug)
> > > + if (!i915.mmio_debug || !UNCLAIMED_CHECK_RANGE(reg))
> > >   return;
> > 
> > Place the register check on the preceding line so that it is not
> > confused with checking the debug-enabled status. (Also you can argue
> > that reg will be register/cache-hot and so a cheaper test to do first
> > than loading a module parameter.)
> 
> That makes sense, I'll do it.
> 
> > 
> > >   if (__raw_i915_read32(dev_priv, FPGA_DBG) & 
> > > FPGA_DBG_RM_NOCLAIM) {
> > > @@ -624,11 +626,11 @@ hsw_unclaimed_reg_debug(struct 
> > > drm_i915_private *dev_priv, u32 reg, bool read,
> > >  }
> > >  
> > >  static void
> > > -hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv)
> > > +hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv, u32 
> > > reg)
> > >  {
> > >   static bool mmio_debug_once = true;
> > >  
> > > - if (i915.mmio_debug || !mmio_debug_once)
> > > + if (i915.mmio_debug || !UNCLAIMED_CHECK_RANGE(reg) || 
> > > !mmio_debug_once)
> > >   return;
> > 
> > The use is wrong here though because you never reviewed my patch to
> > enable the single-shot debugging from the interrupt handler to catch
> > invalid usage from elsewhere.
> 
> If you're talking about intel_uncore_check_errors(), I think we can
> just kill it now. I'll submit a patch with the arguments, so we can
> continue this topic there.
> 
> Moving back to the main subject:
> 
> In the last time you sent the patch to do the unclaimed checks only on
> forcewake code, I started the conversations with the HW guys about the
> range of registers relevant by FPGA_DBG (and CCd you on these
> conversations). My plan was to write this patch at that time, but
> priorities happened and it got postponed :(. I also hoped that maybe
> you would eventually end up writing it and I would just have to review
> it :)
> 
> My main argument was that by doing unclaimed register checking only at
> forcewake time we would be running a check that is only relevant to
> display code during non-display code. So my idea is that if we restrict
> the unclaimed check to the actual range of registers that can be caught
> we basically remove unclaimed register checking for most (all?) of the
> performance-sensitive code.
> 
> Since we have multiple solutions, I decided to do some experiments.
> First of all, since I was not really seeing hsw_unclaimed_reg_detect()
> on perf, I decided to patch it so it would do the read/write check
> around FPGA_DBG 50 times per call instead of just one. With this, by
> running "perf record glxgears" for a few seconds I could see
> hsw_unclaimed_reg_detect() taking about 4.6% of the total time.

Something is very suspect with your system if you are not observing much
hsw_unclaimed_reg_check during trivial workloads.

> Then I applied just this patch, and the time went down to 0.13%. I also
> applied your patch on top of it all, and it went up to 0.52% (I guess
> the extra checks at forcewake time cost a little bit). Also notice that
> since I add a "reg" argument to hsw_unclaimed_reg_detect(), I changed
> the forcewake call to use a register in the display range.
> I also tested your patch without my patch, and the measured result was
> about 0.56%.
> 
> Now this isn't a super relevant experiment: just glxgears with a
> modified hsw_unclaimed_reg_detect(), but I thought it would be useful
> information, and maybe you could provide me suggestions for better
> workloads.
> 
> So I'd like to know if you're ok with proceeding with just this patch
> (considering I implement your suggestions), or if you think we should
> go with just your patch or both or none.

If you really wanted to, you could combine approaches a check in the
forcewake handler as demonstrated is an order of magnitude less frequent
than the register accesses. The key point is that we *need* the checking
against other users, not just our known register access. Checking our own
access basically amounts to detecting invalid registers, which your
approach more or less erradicates (since it is a flag we add to known good
registers, we may as well just compile it and only turn it on during hw

Re: [Intel-gfx] 4.1->4.2 regression : intel_display.c:12324 check_crtc_state warning

[Konduru, Chandra]

>> Sep  2 18:10:26 t44 kernel: [drm:check_crtc_state [i915]] *ERROR* mismatch 
>> in ips_enabled (expected 1, found 0)

It is due to ips_enabled mismatch in crtc_state. 
I can't think how below patch is triggering mismatch in ips_enabled.


> -Original Message-
> From: Toralf Förster [mailto:toralf.foers...@gmx.de]
> Sent: Wednesday, September 02, 2015 1:18 PM
> To: Konduru, Chandra; Roper, Matthew D; daniel.vet...@ffwll.ch
> Cc: intel-gfx
> Subject: 4.1->4.2 regression : intel_display.c:12324 check_crtc_state warning
> 
> The following commit was bisected and tested to be the first bad commit which
> causes the warning as seen below at a ThinkPad T40s:
> 
> 
> 
> commit a1b2278e4dfcd2dbea85e319ebf73a6b7b2f180b
> Author: Chandra Konduru 
> Date:   Tue Apr 7 15:28:45 2015 -0700
> 
> drm/i915: skylake panel fitting using shared scalers
> 
> Enabling skylake panel fitting feature using shared scalers
> 
> v2:
> -added force detach parameter for pfit disable purpose (me)
> -read crtc scaler state from hw state (Daniel)
> -replaced both skylake_pfit_enable and disable with skylake_pfit_update 
> (me)
> -added scaler id check to intel_pipe_config_compare (Daniel)
> 
> v3:
> -updated function header to kerneldoc format (Matt)
> -dropped need_scaling checks (Matt)
> 
> v4:
> -move clearing of scaler id from commit path to check path (Matt)
> -updated colorkey checks based on recent updates (me)
> -squashed scaler check while enabling colorkey to here (me)
> -use values in plane_state->src as regular integers (me)
> -changes made not to modify state in commit path (Matt)
> 
> v5:
> -squashed helper function to update scaler users to here (Matt)
> -squashed helper function to detach scaler to here (Matt, me)
> -changes to align with updated scaler structures (Matt, me)
> 
> Signed-off-by: Chandra Konduru 
> Reviewed-by: Matt Roper 
> Signed-off-by: Daniel Vetter 
> 
> 
> 
> 
> 
> 
> From the syslog:
> 
> Sep  2 18:10:17 t44 kernel: [ cut here ]
> Sep  2 18:10:17 t44 kernel: WARNING: CPU: 2 PID: 1252 at
> drivers/gpu/drm/i915/intel_display.c:12324 check_crtc_state+0x7fa/0xf50
> [i915]()
> Sep  2 18:10:17 t44 kernel: pipe state doesn't match!
> Sep  2 18:10:17 t44 kernel: Modules linked in: nf_conntrack_ipv6 
> nf_defrag_ipv6
> ip6table_filter ip6_tables ipt_MASQUERADE nf_nat_masquerade_ipv4
> nf_log_ipv4 nf_log_common xt_LOG xt_limit ipt_REJECT nf_reject_ipv4
> xt_recent xt_tcpudp xt_conntrack iptable_raw iptable_nat nf_conntrack_ipv4
> nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_filter ip_tables 
> x_tables
> nfsd auth_rpcgss oid_registry lockd grace ctr ccm sunrpc af_packet bridge stp 
> llc
> tun rc_dib0700_rc5 hid_generic hid_cherry usbhid hid dib7000p
> dvb_usb_dib0700 dib3000mc dvb_usb dvb_core dib0070 dib7000m
> dibx000_common dib0090 rc_core snd_hda_codec_generic arc4 coretemp
> x86_pkg_temp_thermal kvm_intel kvm rtsx_pci_sdmmc mmc_core evdev
> jitterentropy_rng hmac drbg aesni_intel aes_x86_64 glue_helper lrw gf128mul
> psmouse ablk_helper cryptd atkbd pcspkr iwlmvm
> Sep  2 18:10:17 t44 kernel:  wmi mac80211 i915 fbcon bitblit cfbfillrect 
> cfbimgblt
> softcursor font i2c_algo_bit cfbcopyarea snd_hda_intel snd_hda_codec battery
> thinkpad_acpi tpm_tis ehci_pci drm_kms_helper ac nvram snd_hda_core iwlwifi
> hwmon i2c_i801 snd_pcm tpm cfg80211 drm video rtsx_pci rfkill intel_gtt
> xhci_pci thermal xhci_hcd agpgart ehci_hcd button fb usbcore snd_timer
> i2c_core e1000e fbdev lpc_ich snd usb_common mfd_core soundcore processor
> Sep  2 18:10:17 t44 kernel: CPU: 2 PID: 1252 Comm: X Tainted: GW
> 4.2.0+ #4
> Sep  2 18:10:17 t44 kernel: Hardware name: LENOVO
> 20AQCTO1WW/20AQCTO1WW, BIOS GJET83WW (2.33 ) 03/09/2015
> Sep  2 18:10:17 t44 kernel:  c05f0f18 88032a6ab828 
> 814cef01
> 88032a6ab870
> Sep  2 18:10:17 t44 kernel:  88032a6ab860 810503b6 
> 88032fdcec00
> 8800ad66f800
> Sep  2 18:10:17 t44 kernel:  8800ad04 8800ad66fc98
> 8800adfeb000 88032a6ab8c0
> Sep  2 18:10:17 t44 kernel: Call Trace:
> Sep  2 18:10:17 t44 kernel:  [] dump_stack+0x44/0x55
> Sep  2 18:10:17 t44 kernel:  []
> warn_slowpath_common+0x86/0xc0
> Sep  2 18:10:17 t44 kernel:  [] warn_slowpath_fmt+0x4c/0x50
> Sep  2 18:10:17 t44 kernel:  [] check_crtc_state+0x7fa/0xf50
> [i915]
> Sep  2 18:10:17 t44 kernel:  []
> intel_modeset_check_state+0x28d/0xae0 [i915]
> Sep  2 18:10:17 t44 kernel:  [] ?
> __intel_set_mode+0x906/0xb30 [i915]
> Sep  2 18:10:17 t44 kernel:  [] ?
> intel_modeset_compute_config+0x362/0xb60 [i915]
> Sep  2 18:10:17 t44 kernel:  []
> intel_crtc_set_config+0x3ed/0x5a0 [i915]
> Sep  2 18:10:17 t44 kernel:  []
> drm_mode_set_config_internal+0x67/0x110 [drm]
> Sep  2 18:10:17 t44 kernel:  [] drm_mode_setcrtc+0xdd/0x500
> [drm]
> Sep  2 18:10:17 t44 kernel:  [] drm_ioctl+0x363/0x680 [drm]
> Sep  2 18:10:17 t44 kernel:  [] ?
> drm

Re: [Intel-gfx] [PATCH 2/2] drm/i915: restrict unclaimed register checking

[Zanoni, Paulo R]

Em Qua, 2015-08-26 às 08:44 +0100, Chris Wilson escreveu:
> On Tue, Aug 25, 2015 at 07:03:42PM -0300, Paulo Zanoni wrote:
> > The unclaimed register bit is only triggered when someone touches 
> > the
> > specified register range.
> > 
> > For the normal use case (with i915.mmio_debug=0), this commit will
> > avoid the extra __raw_i915_read32() call for every register outside
> > the specified range, at the expense of a few additional "if"
> > statements.
> > 
> > Cc: Chris Wilson 
> > Signed-off-by: Paulo Zanoni 
> 
> 
> > @@ -612,7 +614,7 @@ hsw_unclaimed_reg_debug(struct drm_i915_private 
> > *dev_priv, u32 reg, bool read,
> > const char *op = read ? "reading" : "writing to";
> > const char *when = before ? "before" : "after";
> >  
> > -   if (!i915.mmio_debug)
> > +   if (!i915.mmio_debug || !UNCLAIMED_CHECK_RANGE(reg))
> > return;
> 
> Place the register check on the preceding line so that it is not
> confused with checking the debug-enabled status. (Also you can argue
> that reg will be register/cache-hot and so a cheaper test to do first
> than loading a module parameter.)

That makes sense, I'll do it.

> 
> > if (__raw_i915_read32(dev_priv, FPGA_DBG) & 
> > FPGA_DBG_RM_NOCLAIM) {
> > @@ -624,11 +626,11 @@ hsw_unclaimed_reg_debug(struct 
> > drm_i915_private *dev_priv, u32 reg, bool read,
> >  }
> >  
> >  static void
> > -hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv)
> > +hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv, u32 
> > reg)
> >  {
> > static bool mmio_debug_once = true;
> >  
> > -   if (i915.mmio_debug || !mmio_debug_once)
> > +   if (i915.mmio_debug || !UNCLAIMED_CHECK_RANGE(reg) || 
> > !mmio_debug_once)
> > return;
> 
> The use is wrong here though because you never reviewed my patch to
> enable the single-shot debugging from the interrupt handler to catch
> invalid usage from elsewhere.

If you're talking about intel_uncore_check_errors(), I think we can
just kill it now. I'll submit a patch with the arguments, so we can
continue this topic there.

Moving back to the main subject:

In the last time you sent the patch to do the unclaimed checks only on
forcewake code, I started the conversations with the HW guys about the
range of registers relevant by FPGA_DBG (and CCd you on these
conversations). My plan was to write this patch at that time, but
priorities happened and it got postponed :(. I also hoped that maybe
you would eventually end up writing it and I would just have to review
it :)

My main argument was that by doing unclaimed register checking only at
forcewake time we would be running a check that is only relevant to
display code during non-display code. So my idea is that if we restrict
the unclaimed check to the actual range of registers that can be caught
we basically remove unclaimed register checking for most (all?) of the
performance-sensitive code.

Since we have multiple solutions, I decided to do some experiments.
First of all, since I was not really seeing hsw_unclaimed_reg_detect()
on perf, I decided to patch it so it would do the read/write check
around FPGA_DBG 50 times per call instead of just one. With this, by
running "perf record glxgears" for a few seconds I could see
hsw_unclaimed_reg_detect() taking about 4.6% of the total time.

Then I applied just this patch, and the time went down to 0.13%. I also
applied your patch on top of it all, and it went up to 0.52% (I guess
the extra checks at forcewake time cost a little bit). Also notice that
since I add a "reg" argument to hsw_unclaimed_reg_detect(), I changed
the forcewake call to use a register in the display range.
I also tested your patch without my patch, and the measured result was
about 0.56%.

Now this isn't a super relevant experiment: just glxgears with a
modified hsw_unclaimed_reg_detect(), but I thought it would be useful
information, and maybe you could provide me suggestions for better
workloads.

So I'd like to know if you're ok with proceeding with just this patch
(considering I implement your suggestions), or if you think we should
go with just your patch or both or none.

Thanks,
Paulo

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

[Intel-gfx] 4.1->4.2 regression : intel_display.c:12324 check_crtc_state warning

[Toralf Förster]

The following commit was bisected and tested to be the first bad commit which 
causes the warning as seen below at a ThinkPad T40s:



commit a1b2278e4dfcd2dbea85e319ebf73a6b7b2f180b
Author: Chandra Konduru 
Date:   Tue Apr 7 15:28:45 2015 -0700

drm/i915: skylake panel fitting using shared scalers

Enabling skylake panel fitting feature using shared scalers

v2:
-added force detach parameter for pfit disable purpose (me)
-read crtc scaler state from hw state (Daniel)
-replaced both skylake_pfit_enable and disable with skylake_pfit_update (me)
-added scaler id check to intel_pipe_config_compare (Daniel)

v3:
-updated function header to kerneldoc format (Matt)
-dropped need_scaling checks (Matt)

v4:
-move clearing of scaler id from commit path to check path (Matt)
-updated colorkey checks based on recent updates (me)
-squashed scaler check while enabling colorkey to here (me)
-use values in plane_state->src as regular integers (me)
-changes made not to modify state in commit path (Matt)

v5:
-squashed helper function to update scaler users to here (Matt)
-squashed helper function to detach scaler to here (Matt, me)
-changes to align with updated scaler structures (Matt, me)

Signed-off-by: Chandra Konduru 
Reviewed-by: Matt Roper 
Signed-off-by: Daniel Vetter 






From the syslog:

Sep  2 18:10:17 t44 kernel: [ cut here ]
Sep  2 18:10:17 t44 kernel: WARNING: CPU: 2 PID: 1252 at 
drivers/gpu/drm/i915/intel_display.c:12324 check_crtc_state+0x7fa/0xf50 [i915]()
Sep  2 18:10:17 t44 kernel: pipe state doesn't match!
Sep  2 18:10:17 t44 kernel: Modules linked in: nf_conntrack_ipv6 nf_defrag_ipv6 
ip6table_filter ip6_tables ipt_MASQUERADE nf_nat_masquerade_ipv4 nf_log_ipv4 
nf_log_common xt_LOG xt_limit ipt_REJECT nf_reject_ipv4 xt_recent xt_tcpudp 
xt_conntrack iptable_raw iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 
nf_nat_ipv4 nf_nat nf_conntrack iptable_filter ip_tables x_tables nfsd 
auth_rpcgss oid_registry lockd grace ctr ccm sunrpc af_packet bridge stp llc 
tun rc_dib0700_rc5 hid_generic hid_cherry usbhid hid dib7000p dvb_usb_dib0700 
dib3000mc dvb_usb dvb_core dib0070 dib7000m dibx000_common dib0090 rc_core 
snd_hda_codec_generic arc4 coretemp x86_pkg_temp_thermal kvm_intel kvm 
rtsx_pci_sdmmc mmc_core evdev jitterentropy_rng hmac drbg aesni_intel 
aes_x86_64 glue_helper lrw gf128mul psmouse ablk_helper cryptd atkbd pcspkr 
iwlmvm
Sep  2 18:10:17 t44 kernel:  wmi mac80211 i915 fbcon bitblit cfbfillrect 
cfbimgblt softcursor font i2c_algo_bit cfbcopyarea snd_hda_intel snd_hda_codec 
battery thinkpad_acpi tpm_tis ehci_pci drm_kms_helper ac nvram snd_hda_core 
iwlwifi hwmon i2c_i801 snd_pcm tpm cfg80211 drm video rtsx_pci rfkill intel_gtt 
xhci_pci thermal xhci_hcd agpgart ehci_hcd button fb usbcore snd_timer i2c_core 
e1000e fbdev lpc_ich snd usb_common mfd_core soundcore processor
Sep  2 18:10:17 t44 kernel: CPU: 2 PID: 1252 Comm: X Tainted: GW   
4.2.0+ #4
Sep  2 18:10:17 t44 kernel: Hardware name: LENOVO 20AQCTO1WW/20AQCTO1WW, BIOS 
GJET83WW (2.33 ) 03/09/2015
Sep  2 18:10:17 t44 kernel:  c05f0f18 88032a6ab828 814cef01 
88032a6ab870
Sep  2 18:10:17 t44 kernel:  88032a6ab860 810503b6 88032fdcec00 
8800ad66f800
Sep  2 18:10:17 t44 kernel:  8800ad04 8800ad66fc98 8800adfeb000 
88032a6ab8c0
Sep  2 18:10:17 t44 kernel: Call Trace:
Sep  2 18:10:17 t44 kernel:  [] dump_stack+0x44/0x55
Sep  2 18:10:17 t44 kernel:  [] warn_slowpath_common+0x86/0xc0
Sep  2 18:10:17 t44 kernel:  [] warn_slowpath_fmt+0x4c/0x50
Sep  2 18:10:17 t44 kernel:  [] check_crtc_state+0x7fa/0xf50 
[i915]
Sep  2 18:10:17 t44 kernel:  [] 
intel_modeset_check_state+0x28d/0xae0 [i915]
Sep  2 18:10:17 t44 kernel:  [] ? 
__intel_set_mode+0x906/0xb30 [i915]
Sep  2 18:10:17 t44 kernel:  [] ? 
intel_modeset_compute_config+0x362/0xb60 [i915]
Sep  2 18:10:17 t44 kernel:  [] 
intel_crtc_set_config+0x3ed/0x5a0 [i915]
Sep  2 18:10:17 t44 kernel:  [] 
drm_mode_set_config_internal+0x67/0x110 [drm]
Sep  2 18:10:17 t44 kernel:  [] drm_mode_setcrtc+0xdd/0x500 
[drm]
Sep  2 18:10:17 t44 kernel:  [] drm_ioctl+0x363/0x680 [drm]
Sep  2 18:10:17 t44 kernel:  [] ? 
drm_mode_setplane+0x1c0/0x1c0 [drm]
Sep  2 18:10:17 t44 kernel:  [] ? 
ieee80211_rx_irqsafe+0x32/0x50 [mac80211]
Sep  2 18:10:17 t44 kernel:  [] do_vfs_ioctl+0x2cd/0x4c0
Sep  2 18:10:17 t44 kernel:  [] ? 
__audit_syscall_entry+0xaf/0x100
Sep  2 18:10:17 t44 kernel:  [] ? 
do_audit_syscall_entry+0x66/0x70
Sep  2 18:10:17 t44 kernel:  [] ? 
syscall_trace_enter_phase1+0x126/0x140
Sep  2 18:10:17 t44 kernel:  [] SyS_ioctl+0x41/0x70
Sep  2 18:10:17 t44 kernel:  [] ? 
syscall_return_slowpath+0x45/0x110
Sep  2 18:10:17 t44 kernel:  [] 
entry_SYSCALL_64_fastpath+0x12/0x6a
Sep  2 18:10:17 t44 kernel: ---[ end trace 310af98b8519434e ]---
Sep  2 18:10:24 t44 logger: /home/tfoerste/workspace/bin/monitor.sh: started 
with int
S

Re: [Intel-gfx] [PATCH] drm/i915: access the PP_ON_DELAYS/PP_OFF_DELAYS regs only pre GEN5

[Ville Syrjälä]

On Wed, Sep 02, 2015 at 06:36:35PM +0300, Imre Deak wrote:
> These registers exist only before GEN5, so currently we may access
> undefined registers on VLV/CHV and BXT. Apply the workaround only pre
> GEN5.
> 
> This triggered an unclaimed register access warning on BXT.
> 
> Signed-off-by: Imre Deak 
> ---
>  drivers/gpu/drm/i915/intel_bios.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.c 
> b/drivers/gpu/drm/i915/intel_bios.c
> index b3e437b..8969fe6 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1349,7 +1349,7 @@ void intel_setup_bios(struct drm_device *dev)
>   struct drm_i915_private *dev_priv = dev->dev_private;
>  
>/* Set the Panel Power On/Off timings if uninitialized. */
> - if (!HAS_PCH_SPLIT(dev) &&
> + if (INTEL_INFO(dev_priv)->gen < 5 &&
>   I915_READ(PP_ON_DELAYS) == 0 && I915_READ(PP_OFF_DELAYS) == 0) {
>   /* Set T2 to 40ms and T5 to 200ms */
>   I915_WRITE(PP_ON_DELAYS, 0x019007d0);

What a nasty place to hide it. Could you move this somewhere into the
LVDS init code so that it's less well hidden? And maybe toss in a debug
message indicating we had to pull the panel delays from thin air.

-- 
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] Request Linux Graphic Driver for Intel GMA 3150

[Vivi, Rodrigo]

On Wed, 2015-09-02 at 18:16 +0700, David Ho wrote:
> Dear Rodrigo,

Hi David,

I just paid attention to the subject and notice you are looking for
driver for GMA 3150. I'm not sure, but I'm afraid this platform doesn't
have the GPU supported by our open source driver.

Probably the GMA 3150 will be in the same bucket as GMA 3600 series and
the official statement is at 
https://01.org/linuxgraphics/documentation/faq

But to confirm if your gpu is supported by our open source driver, grab
the pci id using

lspci -ns :00:02.0 | awk -F: '{ print $4 }'

and check if these 4 number code is present at:
http://cgit.freedesktop.org/drm-intel/tree/include/drm/i915_pciids.h


> 
> Is it possible to obtain the "raw" driver and to install it manually, 
> without installing it through the Installer?

There is no generic "raw" driver you could just install.

But if your platform is really supported by our driver you have some
alternatives:

1. Get stable source code from https://01.org/linuxgraphics/downloads/2
015q2-intel-graphics-stack-release

and build your self following:
https://01.org/linuxgraphics/documentation/build-guide-0

2. Get development code from
https://01.org/linuxgraphics/documentation/development/source-code

and build your self following:
https://01.org/linuxgraphics/documentation/build-guide-0

3. At your own risk get a community development packaging 
http://kernel.ubuntu.com/~kernel-ppa/mainline/drm-intel-nightly/


> 
> Regards,
> David

Thanks,
Rodrigo.

> 
> 
> -Original Message-
> From: Vivi, Rodrigo [mailto:rodrigo.v...@intel.com] 
> Sent: 01 September 2015 23:04
> To: intel-gfx@lists.freedesktop.org; jani.nik...@linux.intel.com; 
> hupernikao...@gmail.com
> Subject: Re: [Intel-gfx] Request Linux Graphic Driver for Intel GMA 
> 3150
> 
> On Tue, 2015-09-01 at 09:59 +0300, Jani Nikula wrote:
> > On Sat, 22 Aug 2015, David Ho  wrote:
> > > REQUEST
> > > 
> > > May I please request support for driver of Intel GMA 3150 for 
> > > Ubuntu 
> > > 14.04.3
> > > 32 bit (Trusty Tahr)?
> > > 
> > > I installed "Intel Graphic Installer for Linux" from 01.org, but 
> > > it 
> > > stops at the very first step saying "Distribution not supported".
> > 
> > Rodrigo (Cc'd) knows this stuff better, but I don't think it's 
> > likely 
> > old (from upstream POV) distros will be supported. In that regard, 
> > you're probably better off asking help from your distro vendor.
> 
> Unfortunately Installer aims to support just latest versions of 
> Ubuntu and Fedora when it is being prepared for release, regardless 
> distros LTS.
> In the past we supported LTS, but with time official updates 
> conflicted with installer packages and caused more issues to users.
> 
> > > BACKGROUND
> > > 
> > > After fresh install of 14.04, the mouse pointer is not showing up 
> > > and the display change (when scrolling, moving between windows, 
> > > etc) 
> > > is very slow (even only for regular application, never tried to 
> > > watch video yet). I concluded that this is the driver issue.
> > 
> > Does it work on a newer Ubuntu install? Or can you try a new 
> > kernel? 
> > If
> > you can reproduce this on a new kernel, please file a bug at [1].
> 
> > BR,
> > Jani.
> > 
> > 
> > 
> > [1] 
> > https://bugs.freedesktop.org/enter_bug.cgi?product=DRI&component=
> > DRM/Intel
> > 
> > 
> > > 
> > > I must install it for around 20 PCs.
> > > 
> > > Please help me to get the correct driver.
> > > 
> > >  
> > > 
> > > Thank you.
> > > 
> > >  
> > > 
> > > Regards,
> > > 
> > > David Ho
> > > 
> > >  
> > > 
> > >  
> > > 
> > > -Original Message-
> > > From: Chacn Limn, DanielX [mailto:danielx.chacn.l...@intel.com]
> > > Sent: 21 Agustus 2015 22:05
> > > To: hupernikao...@gmail.com
> > > Cc: Becerra Ruiz, Lilia; Flores Perez, Jimena; Diaz, Victor H
> > > Subject: RE: [Contact] Intel GMA 3150 for Ubuntu 14.04.3Trusty 
> > > Tahr 
> > > 32bit
> > > 
> > >  
> > > 
> > > Hi David,
> > > 
> > > Thank you for contacting us.
> > > 
> > >  
> > > 
> > > For help or more information about Linux Graphics drivers please 
> > > contact the Team in charge through their mailing list:
> > > 
> > >  
> > > intel-gfx@lists.freedesktop.org
> > > 
> > >  
> > > 
> > > Regards,
> > > 
> > > Daniel.
> > > 
> > >  
> > > 
> > > [.MESSAGES TRUNCATED]
> > > 
> > > ___
> > > 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] drm/i915/skl: Don't clear all watermarks when updating. (v2)

[Bob Paauwe]

Damien,

You reviewed v1 and then went on vacation for v2. Any chance you can
review v2?

Thanks,
Bob

 On Tue, 21 Jul 2015 10:42:53 -0700
Bob Paauwe  wrote:

> Clearing the watermarks for all pipes/planes when updating the
> watermarks for a single CRTC change seems like the wrong thing to
> do here. As is, this code will ony update any pipe/plane watermarks
> that need updating and leave the remaining set to zero.  Later, the
> watermark checks in check_wm_state() will flag these zero'd out pipe/plane
> watermarks and throw errors.
> 
> By clearing only the watermark values associated with the specific crtc
> the other watermark values may remain unchanged.
> 
> v2: Make sure all the dirty flags are cleared. Damien
> Clear all values assoicated with crtc/pipe being updated.  Damien
> 
> Signed-off-by: Bob Paauwe 
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 26 +-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a1d92b7..27c3126 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3670,6 +3670,26 @@ static void skl_update_other_pipe_wm(struct drm_device 
> *dev,
>   }
>  }
>  
> +static void skl_clear_wm(struct skl_wm_values *watermarks, enum pipe pipe)
> +{
> + watermarks->wm_linetime[pipe] = 0;
> + memset(watermarks->plane[pipe], 0,
> +sizeof(uint32_t) * 8 * I915_MAX_PLANES);
> + memset(watermarks->cursor[pipe], 0, sizeof(uint32_t) * 8);
> + memset(watermarks->plane_trans[pipe],
> +0, sizeof(uint32_t) * I915_MAX_PLANES);
> + watermarks->cursor_trans[pipe] = 0;
> +
> + /* Clear ddb entries for pipe */
> + memset(&watermarks->ddb.pipe[pipe], 0, sizeof(struct skl_ddb_entry));
> + memset(&watermarks->ddb.plane[pipe], 0,
> +sizeof(struct skl_ddb_entry) * I915_MAX_PLANES);
> + memset(&watermarks->ddb.y_plane[pipe], 0,
> +sizeof(struct skl_ddb_entry) * I915_MAX_PLANES);
> + memset(&watermarks->ddb.cursor[pipe], 0, sizeof(struct skl_ddb_entry));
> +
> +}
> +
>  static void skl_update_wm(struct drm_crtc *crtc)
>  {
>   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> @@ -3680,7 +3700,11 @@ static void skl_update_wm(struct drm_crtc *crtc)
>   struct skl_pipe_wm pipe_wm = {};
>   struct intel_wm_config config = {};
>  
> - memset(results, 0, sizeof(*results));
> +
> + /* Clear all dirty flags */
> + memset(results->dirty, 0, sizeof(bool) * I915_MAX_PIPES);
> +
> + skl_clear_wm(results, intel_crtc->pipe);
>  
>   skl_compute_wm_global_parameters(dev, &config);
>  



-- 
--
Bob Paauwe  
bob.j.paa...@intel.com
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193

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

Re: [Intel-gfx] [PATCH v4] drm/i915/gtt: Avoid calling kcalloc in a loop when allocating temp bitmaps

[Chris Wilson]

On Wed, Sep 02, 2015 at 05:46:38PM +0200, Michał Winiarski wrote:
> + pts = kcalloc(pdpes * BITS_TO_LONGS(I915_PDES),
> + sizeof(unsigned long), GFP_TEMPORARY);
> + if (!pts)
> + goto err_out;

This is the oddly aligned bracket!
-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 v4] drm/i915/gtt: Avoid calling kcalloc in a loop when allocating temp bitmaps

[Michał Winiarski]

On each call to gen8_alloc_va_range_3lvl we're allocating temporary
bitmaps needed for error handling. Unfortunately, when we increase
address space size (48b ppgtt) we do additional (512 - 4) calls to
kcalloc, increasing latency between exec and actual start of execution
on the GPU. Let's just do a single kcalloc, we can also drop the size
from free_gen8_temp_bitmaps since it's no longer used.

v2: Use GFP_TEMPORARY to make the allocations reclaimable.
v3: Drop the 2D array, just allocate a single block.
v4: Rebase to handle gen8_preallocate_top_level_pdps.

Cc: Chris Wilson 
Cc: Mika Kuoppala 
Cc: Michel Thierry 
Signed-off-by: Michał Winiarski 
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 49 ++---
 1 file changed, 18 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index bdb7adc..8885ceb 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1164,13 +1164,8 @@ unwind_out:
 }
 
 static void
-free_gen8_temp_bitmaps(unsigned long *new_pds, unsigned long **new_pts,
-  uint32_t pdpes)
+free_gen8_temp_bitmaps(unsigned long *new_pds, unsigned long *new_pts)
 {
-   int i;
-
-   for (i = 0; i < pdpes; i++)
-   kfree(new_pts[i]);
kfree(new_pts);
kfree(new_pds);
 }
@@ -1180,29 +1175,20 @@ free_gen8_temp_bitmaps(unsigned long *new_pds, unsigned 
long **new_pts,
  */
 static
 int __must_check alloc_gen8_temp_bitmaps(unsigned long **new_pds,
-unsigned long ***new_pts,
+unsigned long **new_pts,
 uint32_t pdpes)
 {
-   int i;
unsigned long *pds;
-   unsigned long **pts;
+   unsigned long *pts;
 
-   pds = kcalloc(BITS_TO_LONGS(pdpes), sizeof(unsigned long), GFP_KERNEL);
+   pds = kcalloc(BITS_TO_LONGS(pdpes), sizeof(unsigned long), 
GFP_TEMPORARY);
if (!pds)
return -ENOMEM;
 
-   pts = kcalloc(pdpes, sizeof(unsigned long *), GFP_KERNEL);
-   if (!pts) {
-   kfree(pds);
-   return -ENOMEM;
-   }
-
-   for (i = 0; i < pdpes; i++) {
-   pts[i] = kcalloc(BITS_TO_LONGS(I915_PDES),
-sizeof(unsigned long), GFP_KERNEL);
-   if (!pts[i])
-   goto err_out;
-   }
+   pts = kcalloc(pdpes * BITS_TO_LONGS(I915_PDES),
+   sizeof(unsigned long), GFP_TEMPORARY);
+   if (!pts)
+   goto err_out;
 
*new_pds = pds;
*new_pts = pts;
@@ -1210,7 +1196,7 @@ int __must_check alloc_gen8_temp_bitmaps(unsigned long 
**new_pds,
return 0;
 
 err_out:
-   free_gen8_temp_bitmaps(pds, pts, pdpes);
+   free_gen8_temp_bitmaps(pds, pts);
return -ENOMEM;
 }
 
@@ -1231,7 +1217,7 @@ static int gen8_alloc_va_range_3lvl(struct 
i915_address_space *vm,
 {
struct i915_hw_ppgtt *ppgtt =
container_of(vm, struct i915_hw_ppgtt, base);
-   unsigned long *new_page_dirs, **new_page_tables;
+   unsigned long *new_page_dirs, *new_page_tables;
struct drm_device *dev = vm->dev;
struct i915_page_directory *pd;
const uint64_t orig_start = start;
@@ -1258,14 +1244,14 @@ static int gen8_alloc_va_range_3lvl(struct 
i915_address_space *vm,
ret = gen8_ppgtt_alloc_page_directories(vm, pdp, start, length,
new_page_dirs);
if (ret) {
-   free_gen8_temp_bitmaps(new_page_dirs, new_page_tables, pdpes);
+   free_gen8_temp_bitmaps(new_page_dirs, new_page_tables);
return ret;
}
 
/* For every page directory referenced, allocate page tables */
gen8_for_each_pdpe(pd, pdp, start, length, temp, pdpe) {
ret = gen8_ppgtt_alloc_pagetabs(vm, pd, start, length,
-   new_page_tables[pdpe]);
+   new_page_tables + pdpe * 
BITS_TO_LONGS(I915_PDES));
if (ret)
goto err_out;
}
@@ -1316,20 +1302,21 @@ static int gen8_alloc_va_range_3lvl(struct 
i915_address_space *vm,
gen8_setup_page_directory(ppgtt, pdp, pd, pdpe);
}
 
-   free_gen8_temp_bitmaps(new_page_dirs, new_page_tables, pdpes);
+   free_gen8_temp_bitmaps(new_page_dirs, new_page_tables);
mark_tlbs_dirty(ppgtt);
return 0;
 
 err_out:
while (pdpe--) {
-   for_each_set_bit(temp, new_page_tables[pdpe], I915_PDES)
+   for_each_set_bit(temp, new_page_tables + pdpe *
+   BITS_TO_LONGS(I915_PDES), I915_PDES)
free_pt(dev, 
pdp->page_directory[pdpe]->page_table[temp]);
}
 
for_each_set_bit(pdpe, new_page_dirs, pdpes)
   

[Intel-gfx] [PATCH] drm/i915: access the PP_ON_DELAYS/PP_OFF_DELAYS regs only pre GEN5

[Imre Deak]

These registers exist only before GEN5, so currently we may access
undefined registers on VLV/CHV and BXT. Apply the workaround only pre
GEN5.

This triggered an unclaimed register access warning on BXT.

Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/i915/intel_bios.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c 
b/drivers/gpu/drm/i915/intel_bios.c
index b3e437b..8969fe6 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1349,7 +1349,7 @@ void intel_setup_bios(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
 
 /* Set the Panel Power On/Off timings if uninitialized. */
-   if (!HAS_PCH_SPLIT(dev) &&
+   if (INTEL_INFO(dev_priv)->gen < 5 &&
I915_READ(PP_ON_DELAYS) == 0 && I915_READ(PP_OFF_DELAYS) == 0) {
/* Set T2 to 40ms and T5 to 200ms */
I915_WRITE(PP_ON_DELAYS, 0x019007d0);
-- 
2.1.4

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

Re: [Intel-gfx] [PATCH v6 4/4] drm/i915: set proper N/CTS in modeset

[Takashi Iwai]

On Wed, 02 Sep 2015 17:22:01 +0200,
Daniel Vetter wrote:
> 
> On Wed, Sep 02, 2015 at 03:46:40PM +0200, Takashi Iwai wrote:
> > On Wed, 02 Sep 2015 15:44:34 +0200,
> > Jani Nikula wrote:
> > > 
> > > On Wed, 02 Sep 2015, Takashi Iwai  wrote:
> > > > On Wed, 02 Sep 2015 11:02:42 +0200,
> > > > Jani Nikula wrote:
> > > >> 
> > > >> >> Nitpick. I'd prefer some sharing with the similar blocks from the
> > > >> >> earlier patch. Also a debug message on n == 0 would be nice; you
> > > >> >> probably didn't notice your audio_config_get_rate() wasn't working
> > > >> >> right
> > > >> >> because this silently fell back to the automatic mode here.
> > > >> >
> > > >> > OK, I will add the msg. As you and Ville are insisting on
> > > >> > sharing code, I will do it in next version.
> > > >> 
> > > >> Well, really, I'm fine with having that part duplicated as-is for now,
> > > >> we can fix it later. More important to focus on getting
> > > >> audio_config_get_rate() right.
> > > >> 
> > > >> I don't know if you're still targeting v4.3 with this (up to Takashi I
> > > >> guess) we'll really need to wrap this up soon.
> > > >
> > > > I'm in favor of merging this into 4.3, so it'd be appreciated if Libin
> > > > can prepare the fixed version soonish, indeed.
> > > 
> > > IIUC patches 1-3 would be useful on their own already, and a fixed
> > > version of patch 4 could follow. Just a thought.
> > 
> > That's good, then I can take the first three patches.
> > 
> > Daniel, would you like to review these three before I merge them?
> > I assumed your previous mail as a kind of ack to this series, too.
> 
> fwiw ack on that plan, but given how much I made a mess out of these two
> series and mixed them up I wouldn't trust me anyway ;-)
> 
> Once the code settles I'll rebase/backmerge so that I can apply the
> follow-up documentation/kerneldoc work that's still getting done.

FYI, I've pushed a branch including Libin's patchset (but only patches
1-3) just for checking.  Let me know if this looks OK.


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

Re: [Intel-gfx] [RFC 1/3] drm/i915: Roll intel_crtc->atomic into intel_crtc_state

[Ville Syrjälä]

On Wed, Sep 02, 2015 at 04:22:56PM +0200, Maarten Lankhorst wrote:
> Op 02-09-15 om 13:15 schreef Ville Syrjälä:
> > On Wed, Sep 02, 2015 at 01:08:31PM +0200, Maarten Lankhorst wrote:
> >> Op 02-09-15 om 12:35 schreef Ville Syrjälä:
> >>> On Wed, Sep 02, 2015 at 07:15:25AM +0200, Maarten Lankhorst wrote:
>  Op 01-09-15 om 17:48 schreef Ville Syrjälä:
> > On Tue, Sep 01, 2015 at 08:30:05AM -0700, Matt Roper wrote:
> >> On Tue, Sep 01, 2015 at 07:24:19AM +0200, Maarten Lankhorst wrote:
> >>> Op 29-08-15 om 01:57 schreef Matt Roper:
>  Way back at the beginning of i915's atomic conversion I added
>  intel_crtc->atomic as a temporary dumping ground for "stuff to do
>  outside vblank evasion" flags since CRTC states weren't properly 
>  wired
>  up and tracked at that time.  We've had proper CRTC state tracking 
>  for a
>  while now, so there's really no reason for this hack to continue to
>  exist.  Moving forward we want to store intermediate crtc/plane state
>  data for modesets in addition to the final state, so moving these 
>  fields
>  into the proper state object allows us to properly compute them for 
>  both
>  the intermediate and final state.
> 
>  Signed-off-by: Matt Roper 
>  ---
> >>> Can I shoot this patch down? It's better to add a field 'wm_changed'
> >>> to the crtc_state, which gets reset to false for each crtc_state
> >>> duplication. It's needed for checking if a cs pageflip can be done for
> >>> atomic. It would remove the duplication of some checks there.
> >>>
> >>> The other atomic state members will die soon. I already have some
> >>> patches to achieve that. :)
> >>>
> >>> I'm not sure if an intermediate state is a good idea. Any code that
> >>> disables a crtc should only be looking at the old state.
> >>> pre_plane_update runs all stuff in preparation for disabling planes,
> >>> while post_plane_update runs everything needed for enabling planes. So
> >>> no need to split it up I think, maybe put in some intermediate
> >>> watermarks in intel_atomic_state, but no need for a full crtc_state.
> >> Well, the intermediate state stuff was requested by Ville in response 
> >> to
> >> my watermark series, so I posted these patches as an RFC to make sure I
> >> was understanding what he was looking for properly.
> >>
> >> Ville, can you comment?
> > My opinion is that the current "disable is special" way of doing things
> > is quite horrible. For one it makes it really hard to reason about what
> > happens to a plane or crtc during the modeset. It's not just off->on,
> > on->off, or same->same, but can be on->off->on. With the intermediate
> > state in place, there can only be one transition, so really easy to
> > think about what's going on.
>  pre_plane_update deals with all stuff related to disabling planes, while 
>  post_plane_update deals with changes after enabling.
> 
>  If the crtc goes from on -> off only you could just hammer in the final 
>  values after the disable.
> 
>  While for off->on or on->off->on you can put in the final values in 
>  .crtc_enable before lighting the pipe. I don't see why wm's would need 
>  more transitions.
> >>> One special case after another. Yuck. Not to mention that the plane
> >>> disable isn't even atomic in the current code, which can look ugly.
> >> That's easily fixed by adding a pipe_update_start/end pair.
> > It'll also mean don't have to sprinkle silly wm update calls all over
> > the modeset path. They will just get updated in response to the plane
> > state changes. Same for IPS/FBC etc.
>  IPS and FBC are already calculated correctly in response to modesets.
> >>> Correctly perhaps, but not in an obvious way.
> >> It will become more obvious again when pre_plane_update and 
> >> post_plane_update are loops
> >> instead of being precalculated from intel_plane_atomic_calc_changes.
> > It'll never be obvious as long as the on->off->on case exists.
> >
> But On -> off will always be a special case because any enable might depend 
> on the disable, for example taking over the pll or cdclk changes.
> It can never be the same, so why pretend it is?

I don't understand what you're saying. If we had the intermediate atomic
state, plane code wouldn't need to know at all what's happening to the
pipe. And for pipes there can only be an on->off or off->on transition.

-- 
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] [RFC] Docs: drm: Move KMS properties table out to source files

[Daniel Vetter]

On Wed, Sep 02, 2015 at 02:50:52PM +0100, Graham Whaley wrote:
> (RFC/test - not for merging)
> The below is a test of moving the large HTML KMS properties table out
> to markdown style in the appropriate files.
> In the test we only use the first few rows of the existing KMS table
> an example.
> We use a fixed width table as the other styles of table supported by
> pandoc markdown do not support multi-column cells.
> 
> The test shows a couple of issues:
>  1) double quote characters are being expanded in the fixed width table
>  which then breaks the table alignment and leaves html style  tags
>  in the text
> 
>  2) Cramming the seven columns into the apprx 80ish column width makes
>  some entries fairly impractical - one word per row. For reference,
>  pdfdocs rendering clips the fixed text tables at around 80 characters.
> 
> If we can:
>  a) Resolve (1) above
> and
>  b) Agree that we are OK with comment fields extending beyond the 80th
>  column significantly
> 
> then maybe we can continue looking at moving the KMS properties table out
> of drm.tmpl and into markdown format fragments in the source files.
> If not, then maybe we go back to considering the conversion of the KMS
> table to docbook CALS format.

Another option would be to split up the table into sub-tables - the first
1-2 rows are really just headings for sub-tables imo. If we split them up
we have tables without spans and that could use the markdown table layout
instead of fixed-width text.

But then we'd need to do that part-by-part ...
-Daniel

> ---
>  Documentation/DocBook/drm.tmpl | 925 
> +
>  drivers/gpu/drm/drm_crtc.c |  16 +
>  2 files changed, 17 insertions(+), 924 deletions(-)
> 
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 66bc646..ecfd084 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -2573,930 +2573,7 @@ void intel_crt_init(struct drm_device *dev)
>   The following table gives description of drm properties exposed by 
> various
>   modules/drivers.
>   
> - 
> - 
> - 
> - Owner Module/Drivers
> - Group
> - Property Name
> - Type
> - Property Values
> - Object attached
> - Description/Restrictions
> - 
> - 
> - DRM
> - Generic
> - “rotation”
> - BITMASK
> - { 0, "rotate-0" },
> - { 1, "rotate-90" },
> - { 2, "rotate-180" },
> - { 3, "rotate-270" },
> - { 4, "reflect-x" },
> - { 5, "reflect-y" }
> - CRTC, Plane
> - rotate-(degrees) rotates the image by the specified 
> amount in degrees
> - in counter clockwise direction. reflect-x and reflect-y reflects the
> - image along the specified axis prior to rotation
> - 
> - 
> - Connector
> - “EDID”
> - BLOB | IMMUTABLE
> - 0
> - Connector
> - Contains id of edid blob ptr object.
> - 
> - 
> - “DPMS”
> - ENUM
> - { “On”, “Standby”, “Suspend”, “Off” }
> - Connector
> - Contains DPMS operation mode value.
> - 
> - 
> - “PATH”
> - BLOB | IMMUTABLE
> - 0
> - Connector
> - Contains topology path to a connector.
> - 
> - 
> - “TILE”
> - BLOB | IMMUTABLE
> - 0
> - Connector
> - Contains tiling information for a connector.
> - 
> - 
> - “CRTC_ID”
> - OBJECT
> - DRM_MODE_OBJECT_CRTC
> - Connector
> - CRTC that connector is attached to (atomic)
> - 
> - 
> - Plane
> - “type”
> - ENUM | IMMUTABLE
> - { "Overlay", "Primary", "Cursor" }
> - Plane
> - Plane type
> - 
> - 
> - “SRC_X”
> - RANGE
> - Min=0, Max=UINT_MAX
> - Plane
> - Scanout source x coordinate in 16.16 fixed point 
> (atomic)
> - 
> - 
> - “SRC_Y”
> - RANGE
> - Min=0, Max=UINT_MAX
> - Plane
> - Scanout source y coordinate in 16.16 fixed point 
> (atomic)
> - 
> - 
> - “SRC_W”
> - RANGE
> - Min=0, Max=UINT_MAX
> - Plane
> - Scanout source width in 16.16 fixed point 
> (atomic)
> - 
> - 
> - “SRC_H”
> - RANGE
> - Min=0, Max=UINT_MAX
> - Plane
> - Scanout source height in 16.16 fixed point 
> (atomic)
> - 
> - 
> - “CRTC_X”
> - SIGNED_RANGE
> - Min=INT_MIN, Max=INT_MAX
> - Plane
> - Scanout CRTC (destination) x coordinate (atomic)
> - 
> - 
> - “CRTC_Y”
> - SIGNED_RANGE
> - Min=INT_MIN, Max=INT_MAX
> - Plane
> - Scanout CRTC (destination) y coordinate (atomic)
> - 
> - 
> - “CRTC_W”
> - RANGE
> - Min=0, Max=UINT_MAX
> - Plane
> - Scanout CRTC (destination) width (atomic)
> - 
> - 
> - “CRTC_H”
> - RANGE
> - Min=0, Max=UINT_MAX
> - Plane
> - Scanout CRTC (destination) height (atomic)
> - 
> - 
> - “FB_ID”
> - OBJECT
> - DRM_MODE_OBJECT_FB
> - Plane
> - Scanout framebuff

Re: [Intel-gfx] [RFC] Docs: drm: Move KMS properties table out to source files

[Daniel Vetter]

On Wed, Sep 02, 2015 at 05:14:35PM +0300, Jani Nikula wrote:
> On Wed, 02 Sep 2015, Graham Whaley  wrote:
> >  Documentation/DocBook/drm.tmpl | 925 
> > +
> >  drivers/gpu/drm/drm_crtc.c |  16 +
> >  2 files changed, 17 insertions(+), 924 deletions(-)
> 
> I like this already.

Well the patch removes the entire table but only adds the entry for
rotation property.
-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

Re: [Intel-gfx] [PATCH v3] drm/i915/gtt: Avoid calling kcalloc in a loop when allocating temp bitmaps

[Daniel Vetter]

On Wed, Sep 02, 2015 at 05:13:29PM +0200, Daniel Vetter wrote:
> On Wed, Sep 02, 2015 at 02:46:41PM +0100, Chris Wilson wrote:
> > On Wed, Sep 02, 2015 at 02:40:03PM +0100, Michel Thierry wrote:
> > > On 9/1/2015 10:06 AM, Michał Winiarski wrote:
> > > >On each call to gen8_alloc_va_range_3lvl we're allocating temporary
> > > >bitmaps needed for error handling. Unfortunately, when we increase
> > > >address space size (48b ppgtt) we do additional (512 - 4) calls to
> > > >kcalloc, increasing latency between exec and actual start of execution
> > > >on the GPU. Let's just do a single kcalloc, we can also drop the size
> > > >from free_gen8_temp_bitmaps since it's no longer used.
> > > >
> > > >v2: Use GFP_TEMPORARY to make the allocations reclaimable.
> > > >v3: Drop the 2D array, just allocate a single block.
> > > >
> > > >Cc: Chris Wilson 
> > > >Cc: Mika Kuoppala 
> > > >Cc: Michel Thierry 
> > > >Signed-off-by: Michał Winiarski 
> > > 
> > > Unless Chris thinks otherwise, I see Michał already addressed his 
> > > comments.
> > 
> > I think I spotted a misaligned bracket... ;)
> 
> checkpatch didn't spot it and neither me ...
> 
> > Reviewed-by: Chris Wilson 
> 
> Queued for -next, thanks for the patch.

Strike that, patch doesn't compile too well on plain dinq. What's going on
here? And there doesn't seem to be anything in -nightly which isn't in
dinq ...
-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

Re: [Intel-gfx] [PATCH 1/2] drm/atomic: Make prepare_fb/cleanup_fb only take state, v3.

[Daniel Vetter]

On Wed, Sep 02, 2015 at 03:36:33PM +0100, Daniel Stone wrote:
> On 2 September 2015 at 09:42, Maarten Lankhorst
>  wrote:
> > This removes the need to separately track fb changes i915.
> > That will be done as a separate commit, however.
> >
> > Changes since v1:
> > - Add dri-devel to cc.
> > - Fix a check in intel's prepare and cleanup fb to take rotation
> >   into account.
> > Changes since v2:
> > - Split out i915 changes to a separate commit.
> >
> > Cc: dri-de...@lists.freedesktop.org
> > Signed-off-by: Maarten Lankhorst 
> 
> I'd probably prefer to see the change to call them unconditionally
> (regardless of fb != NULL) in a separate patch, but with or without:
> Reviewed-by: Daniel Stone 

Applied to drm-misc, 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

Re: [Intel-gfx] [PATCH v6 4/4] drm/i915: set proper N/CTS in modeset

[Daniel Vetter]

On Wed, Sep 02, 2015 at 03:46:40PM +0200, Takashi Iwai wrote:
> On Wed, 02 Sep 2015 15:44:34 +0200,
> Jani Nikula wrote:
> > 
> > On Wed, 02 Sep 2015, Takashi Iwai  wrote:
> > > On Wed, 02 Sep 2015 11:02:42 +0200,
> > > Jani Nikula wrote:
> > >> 
> > >> >> Nitpick. I'd prefer some sharing with the similar blocks from the
> > >> >> earlier patch. Also a debug message on n == 0 would be nice; you
> > >> >> probably didn't notice your audio_config_get_rate() wasn't working
> > >> >> right
> > >> >> because this silently fell back to the automatic mode here.
> > >> >
> > >> > OK, I will add the msg. As you and Ville are insisting on
> > >> > sharing code, I will do it in next version.
> > >> 
> > >> Well, really, I'm fine with having that part duplicated as-is for now,
> > >> we can fix it later. More important to focus on getting
> > >> audio_config_get_rate() right.
> > >> 
> > >> I don't know if you're still targeting v4.3 with this (up to Takashi I
> > >> guess) we'll really need to wrap this up soon.
> > >
> > > I'm in favor of merging this into 4.3, so it'd be appreciated if Libin
> > > can prepare the fixed version soonish, indeed.
> > 
> > IIUC patches 1-3 would be useful on their own already, and a fixed
> > version of patch 4 could follow. Just a thought.
> 
> That's good, then I can take the first three patches.
> 
> Daniel, would you like to review these three before I merge them?
> I assumed your previous mail as a kind of ack to this series, too.

fwiw ack on that plan, but given how much I made a mess out of these two
series and mixed them up I wouldn't trust me anyway ;-)

Once the code settles I'll rebase/backmerge so that I can apply the
follow-up documentation/kerneldoc work that's still getting done.

Cheers, 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

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt

[Egbert Eich]

Daniel Vetter writes:
 > On Wed, Sep 02, 2015 at 04:19:00PM +0200, Egbert Eich wrote:
 > 
 > Hm I missed that this same register is also accessed by the irq handler
 > code, and it's not just that touching these bits can cause interrupts. So
 > yeah we need your patch, but it needs to be clearer in the commit message
 > that there's also trouble with concurrent register access to
 > PORT_HOTPLUG_EN.
 > 
 > Also I think a commen in the code why we grab that spinlock would be good.
 > For that extracting a small helper to manipulate the register (like we do
 > with other irq mask registers with functions like ilk_update_gt_irq) would
 > be good - then we have just one place to put that commment.

OK, I will come up with a suggestion.

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

Re: [Intel-gfx] [PATCH v3] drm/i915/gtt: Avoid calling kcalloc in a loop when allocating temp bitmaps

[Daniel Vetter]

On Wed, Sep 02, 2015 at 02:46:41PM +0100, Chris Wilson wrote:
> On Wed, Sep 02, 2015 at 02:40:03PM +0100, Michel Thierry wrote:
> > On 9/1/2015 10:06 AM, Michał Winiarski wrote:
> > >On each call to gen8_alloc_va_range_3lvl we're allocating temporary
> > >bitmaps needed for error handling. Unfortunately, when we increase
> > >address space size (48b ppgtt) we do additional (512 - 4) calls to
> > >kcalloc, increasing latency between exec and actual start of execution
> > >on the GPU. Let's just do a single kcalloc, we can also drop the size
> > >from free_gen8_temp_bitmaps since it's no longer used.
> > >
> > >v2: Use GFP_TEMPORARY to make the allocations reclaimable.
> > >v3: Drop the 2D array, just allocate a single block.
> > >
> > >Cc: Chris Wilson 
> > >Cc: Mika Kuoppala 
> > >Cc: Michel Thierry 
> > >Signed-off-by: Michał Winiarski 
> > 
> > Unless Chris thinks otherwise, I see Michał already addressed his comments.
> 
> I think I spotted a misaligned bracket... ;)

checkpatch didn't spot it and neither me ...

> Reviewed-by: Chris Wilson 

Queued for -next, thanks for the patch.
-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

Re: [Intel-gfx] [PATCH 18/17] drm/i915: Don't call intel_get_hpd_pins() when there's no hotplug interrupt

[Daniel Vetter]

On Fri, Aug 28, 2015 at 07:15:15PM -0300, Paulo Zanoni wrote:
> 2015-08-28 16:59 GMT-03:00  :
> > From: Ville Syrjälä 
> >
> > On GMCH plaforms we are now getting the following spew on aux
> > interrupts:
> > [drm:intel_get_hpd_pins] hotplug event received, stat 0x, dig 
> > 0x, pins 0x
> > [drm:intel_get_hpd_pins] hotplug event received, stat 0x, dig 
> > 0x, pins 0x
> > [drm:intel_get_hpd_pins] hotplug event received, stat 0x, dig 
> > 0x, pins 0x
> > [drm:intel_get_hpd_pins] hotplug event received, stat 0x, dig 
> > 0x, pins 0x
> > [drm:intel_get_hpd_pins] hotplug event received, stat 0x, dig 
> > 0x, pins 0x
> > [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x71450064
> >
> > Prevent it by not calling intel_get_hpd_pins() unless one of the HPD
> > interrupt bits are actually set.
> >
> > I already fixed similar annoyance once with
> > 4bca26d0a6518d51a9abe64fbde4b12f04c74053 drm/i915: Use 
> > HOTPLUG_INT_STATUS_G4X on VLV/CHV
> >
> > but another source for it got added in
> > fd63e2a972c670887e5e8a08440111d3812c0996 drm/i915: combine 
> > i9xx_get_hpd_pins and pch_get_hpd_pins
> >
> > due to pch_get_hpd_pins() being chosen over i9xx_get_hpd_pins() to
> > serve as the new unified piece of code. pch_get_hpd_pins() had the debug
> > print, and i9xx_get_hpd_pins() didn't.
> >
> > Cc: Imre Deak 
> > Signed-off-by: Ville Syrjälä 
> 
> Reviewed-by: Paulo Zanoni 

All applied to dinq except for one patch that Jani picked up to -fixes.
-Daniel

> 
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 22 ++
> >  1 file changed, 14 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c 
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index 610d301..07e539d 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1639,20 +1639,26 @@ static void i9xx_hpd_irq_handler(struct drm_device 
> > *dev)
> > if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
> > u32 hotplug_trigger = hotplug_status & 
> > HOTPLUG_INT_STATUS_G4X;
> >
> > -   intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> > -  hotplug_trigger, hpd_status_g4x,
> > -  i9xx_port_hotplug_long_detect);
> > -   intel_hpd_irq_handler(dev, pin_mask, long_mask);
> > +   if (hotplug_trigger) {
> > +   intel_get_hpd_pins(&pin_mask, &long_mask, 
> > hotplug_trigger,
> > +  hotplug_trigger, hpd_status_g4x,
> > +  i9xx_port_hotplug_long_detect);
> > +
> > +   intel_hpd_irq_handler(dev, pin_mask, long_mask);
> > +   }
> >
> > if (hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X)
> > dp_aux_irq_handler(dev);
> > } else {
> > u32 hotplug_trigger = hotplug_status & 
> > HOTPLUG_INT_STATUS_I915;
> >
> > -   intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> > -  hotplug_trigger, hpd_status_i915,
> > -  i9xx_port_hotplug_long_detect);
> > -   intel_hpd_irq_handler(dev, pin_mask, long_mask);
> > +   if (hotplug_trigger) {
> > +   intel_get_hpd_pins(&pin_mask, &long_mask, 
> > hotplug_trigger,
> > +  hotplug_trigger, hpd_status_i915,
> > +  i9xx_port_hotplug_long_detect);
> > +
> > +   intel_hpd_irq_handler(dev, pin_mask, long_mask);
> > +   }
> > }
> >  }
> >
> > --
> > 2.4.6
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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] [PATCH v2] drm/i915/bdw: Check for slice, subslice and EU count for BDW

[Arun Siluvery]

On 02/09/2015 16:47, Łukasz Daniluk wrote:

Added checks for available slices, subslices and EUs for Broadwell. This
information is filled in intel_device_info and is available to user with
GET_PARAM.
Added checks for enabled slices, subslices and EU for Broadwell. This
information is based on available counts but takes power gated slices
into account. It can be read in debugfs.
Introduce new register defines that contain information on slices on
Broadwell.

v2:
- Introduce GT_SLICE_INFO register
- Change Broadwell sseu_device_status function to use GT_SLICE_INFO
   register instead of RPCS register
- Undo removal of dev_priv variables in Cherryview and Gen9
   sseu_device_satus functions

Cc: Jeff Mcgee 
Signed-off-by: Łukasz Daniluk 
---
  drivers/gpu/drm/i915/i915_debugfs.c | 29 +++-
  drivers/gpu/drm/i915/i915_dma.c | 89 +
  drivers/gpu/drm/i915/i915_reg.h | 22 -
  3 files changed, 137 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 23a69307..e8a62ef 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4932,13 +4932,38 @@ static void gen9_sseu_device_status(struct drm_device 
*dev,
}
  }

+static void broadwell_sseu_device_status(struct drm_device *dev,
+struct sseu_dev_status *stat)
+{
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   int s;
+   u32 slice_info = I915_READ(GEN8_GT_SLICE_INFO);

I don't see this register (0x138064 as defined below) in bdw spec.


+
+   stat->slice_total =
+   hweight32(slice_info & GEN8_LSLICESTAT_MASK);


why not in a single line, seems to fit under 80 chars.


+
+   if (stat->slice_total)
+   {
+   stat->subslice_per_slice = INTEL_INFO(dev)->subslice_per_slice;
+   stat->eu_per_subslice = INTEL_INFO(dev)->eu_per_subslice;
+   stat->subslice_total = stat->slice_total * 
stat->subslice_per_slice;
+   stat->eu_total = stat->eu_per_subslice * stat->subslice_total;
+
Please reorder such that all subslice values are populated first 
followed by EUs to follow an order.



+   /* subtract fused off EU(s) from enabled slice(s) */
+   for (s = 0; s < stat->slice_total; s++) {
+   u8 subslice_7eu = INTEL_INFO(dev)->subslice_7eu[s];
+   stat->eu_total -= hweight8(subslice_7eu);
+   }
+   }
+}
+
  static int i915_sseu_status(struct seq_file *m, void *unused)
  {
struct drm_info_node *node = (struct drm_info_node *) m->private;
struct drm_device *dev = node->minor->dev;
struct sseu_dev_status stat;

-   if ((INTEL_INFO(dev)->gen < 8) || IS_BROADWELL(dev))
+   if (INTEL_INFO(dev)->gen < 8)
return -ENODEV;

seq_puts(m, "SSEU Device Info\n");
@@ -4963,6 +4988,8 @@ static int i915_sseu_status(struct seq_file *m, void 
*unused)
memset(&stat, 0, sizeof(stat));
if (IS_CHERRYVIEW(dev)) {
cherryview_sseu_device_status(dev, &stat);
+   } else if (IS_BROADWELL(dev)) {
+   broadwell_sseu_device_status(dev, &stat);
} else if (INTEL_INFO(dev)->gen >= 9) {
gen9_sseu_device_status(dev, &stat);
}
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index ab37d11..2d52b1e 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -705,6 +705,93 @@ static void gen9_sseu_info_init(struct drm_device *dev)
info->has_eu_pg = (info->eu_per_subslice > 2);
  }

+static void broadwell_sseu_info_init(struct drm_device *dev)
+{
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_device_info *info;
+   const int s_max = 3, ss_max = 3, eu_max = 8;

I only see max of 2 slices for bdw in spec.


+   int s, ss;
+   u32 fuse2, eu_disable[s_max], s_enable, ss_disable;
+
+   fuse2 = I915_READ(GEN8_FUSE2);
+   s_enable = (fuse2 & GEN8_F2_S_ENA_MASK) >>
+   GEN8_F2_S_ENA_SHIFT;
+   ss_disable = (fuse2 & GEN8_F2_SS_DIS_MASK) >>
+   GEN8_F2_SS_DIS_SHIFT;
+
+   eu_disable[0] = I915_READ(GEN8_EU_DISABLE0) &
+   GEN8_EU_DIS0_S0_MASK;
+   eu_disable[1] = (I915_READ(GEN8_EU_DISABLE0) >>
+   GEN8_EU_DIS0_S1_SHIFT) |
+   ((I915_READ(GEN8_EU_DISABLE1) &
+ GEN8_EU_DIS1_S1_MASK) <<
+(32 - GEN8_EU_DIS0_S1_SHIFT));
+   eu_disable[2] = (I915_READ(GEN8_EU_DISABLE1) >>
+   GEN8_EU_DIS1_S2_SHIFT) |
+   ((I915_READ(GEN8_EU_DISABLE2) &
+ GEN8_EU_DIS2_S2_MASK) <<
+(32 - GEN8_EU_DIS1_S2_SHIFT));
+


GEN9_EU_DISABLE(slice) is already available, since the register 
addresses are same maybe it can be reused after renaming it to 

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt

[Egbert Eich]

Jani Nikula writes:
 > On Wed, 02 Sep 2015, Egbert Eich  wrote:
 > > This is exactly the scenatio I'm getting here. I get HPD interrupts at an 
 > > order of 10^4 / sec.
 > 
 > Makes you wonder if either you have faulty hardware or we are
 > configuring the hardware wrong (we overlook some configuration about
 > some voltage/duration threshold maybe, or get irqs from a line that's
 > floating, or something).

It is faulty hardware. But it is not a single machine that broke.
It is an entire series. IMHO due to bad signal routing and poor shilding
there is crosstalk on the SDVO lines signaling the plug status.
Since SDVO uses PCIe lines it is AC coupled, if I recall correctly
from reading the specs long time ago, one status is signalled by a
10MHz signal, the other by 20MHz.

At the time when I implemented this I've seen other reports from systems 
which showed similar problems under certain conditions(*) - although not 
quite as bad, therefore I thought of a general solution to get rid of 
this once and for all. If this had only been one system with this problem, 
I would just have blacklisted it.

(*) It seems that this somewhat depends on the video mode set (supports
the crosstalk theory) but I also had a report where this occurred at
certain charging levels and whether a power supply was connected or
not.

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

Re: [Intel-gfx] [PATCH] drm/i915: Fix broken mst get_hw_state.

[Daniel Vetter]

On Wed, Sep 02, 2015 at 04:14:27PM +0200, Maarten Lankhorst wrote:
> Op 02-09-15 om 16:07 schreef Ander Conselvan De Oliveira:
> > On Thu, 2015-08-27 at 13:13 +0200, Maarten Lankhorst wrote:
> >> connector->encoder is initialized as NULL. Fix this by setting it in
> >> during pre enable. MST connectors are not read out during initial hw
> >> readout, and have no fixed encoder mappings. So it's harmless to
> >> return false when the connector has never been assigned to an encoder.
> >>
> >> Signed-off-by: Maarten Lankhorst 
> >> ---
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> >> b/drivers/gpu/drm/i915/intel_display.c
> >> index 738178a0ac96..e3922d973db0 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -6283,7 +6283,7 @@ static void intel_connector_check_state(struct 
> >> intel_connector *connector)
> >>  connector->base.name);
> >>  
> >>if (connector->get_hw_state(connector)) {
> >> -  struct drm_encoder *encoder = &connector->encoder->base;
> >> +  struct intel_encoder *encoder = connector->encoder;
> >>struct drm_connector_state *conn_state = connector->base.state;
> >>  
> >>I915_STATE_WARN(!crtc,
> >> @@ -6295,13 +6295,13 @@ static void intel_connector_check_state(struct 
> >> intel_connector *connector)
> >>I915_STATE_WARN(!crtc->state->active,
> >>  "connector is active, but attached crtc isn't\n");
> >>  
> >> -  if (!encoder)
> >> +  if (!encoder || encoder->type == INTEL_OUTPUT_DP_MST)
> >>return;
> >>  
> >> -  I915_STATE_WARN(conn_state->best_encoder != encoder,
> >> +  I915_STATE_WARN(conn_state->best_encoder != &encoder->base,
> >>"atomic encoder doesn't match attached encoder\n");
> >>  
> >> -  I915_STATE_WARN(conn_state->crtc != encoder->crtc,
> >> +  I915_STATE_WARN(conn_state->crtc != encoder->base.crtc,
> >>"attached encoder crtc differs from connector crtc\n");
> >>} else {
> >>I915_STATE_WARN(crtc && crtc->state->active,
> >> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
> >> b/drivers/gpu/drm/i915/intel_dp_mst.c
> >> index ebf205462631..d606721b1788 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> >> @@ -159,6 +159,11 @@ static void intel_mst_pre_enable_dp(struct 
> >> intel_encoder *encoder)
> >>return;
> >>}
> >>  
> >> +  /* MST encoders are bound to a crtc, not to a connector,
> >> +   * force the mapping here for get_hw_state.
> >> +   */
> >> +  found->encoder = encoder;
> >> +
> >>DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
> >>intel_mst->port = found->port;
> >>  
> >> @@ -392,7 +397,7 @@ static const struct drm_encoder_funcs 
> >> intel_dp_mst_enc_funcs = {
> >>  
> >>  static bool intel_dp_mst_get_hw_state(struct intel_connector *connector)
> >>  {
> >> -  if (connector->encoder) {
> >> +  if (connector->encoder && connector->base.state->crtc) {
> > Is this here to cover the case where we disable this connector and assign 
> > the encoder to a different
> > connector? I guess this should work since if we disable the connector than 
> > crtc will be null, and
> > we'll assign a proper encoder when re-enabling. But now I'm wondering if it 
> > would make sense to set
> > connector->encoder back to NULL in post_disable.
> >
> Yes, it's harmless to keep it non-null if conn_state->crtc is checked though. 
> Saves another pointless loop.

Hm in get_hw_state functions we really shouldn't look at state pointers
like connector->encoder or state->crtc at all. It /should/ be all
free-standing for fastboot to actually work.

But I guess that's a lot more than just one patch ;-)
-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

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt

[Daniel Vetter]

On Wed, Sep 02, 2015 at 04:19:00PM +0200, Egbert Eich wrote:
> Daniel Vetter writes:
>  > On Tue, Sep 01, 2015 at 10:21:35PM +0200, Egbert Eich wrote:
>  > > A HPD interrupt may fire during intel_crt_detect_hotplug() - especially
>  > > when HPD interrupt storms occur.
>  > > Since the interrupt handler changes the enabled interrupt lines when it
>  > > detects a storm this races with intel_crt_detect_hotplug().
>  > > To avoid this, shiled the rmw cycles with IRQ save spinlocks.
>  > > 
>  > > Signed-off-by: Egbert Eich 
>  > 
>  > I think this only reduces one source of such races, but fundamentally we
>  > can't avoid them. E.g. if you're _very_ unlucky you might cause a real hpd
>  > right when the strom code frobs around. Plugging the race with this known
> 
> This is exactly the scenatio I'm getting here. I get HPD interrupts at an 
> order of 10^4 / sec.
> 
>  > interrupt source here doesn't fix the fundamental issue.
>  > 
>  > Also I think the actual interrupt deliver is fairly asynchronous, both in
>  > the hardware and in the sw handling. E.g. spin_lock_irq does not
>  > synchronize with the interrupt handler on SMP systems, it only guarantees
>  > that it's not running concurrently on the same cpu (which would deadlock).
>  > 
>  > I think fundamentally this race is unfixable.
> 
> 
> There is one important race we avoid with this patch: It is that
> we change the PORT_HOTPLUG_EN concurrently in the interrupt handler
> (thru xxx_hpd_irq_setup() and in intel_crt_detect_hotplug()).
> 
> With the test system that I have here the old version of the code
> easily runs into this as the time spent inside intel_crt_detect_hotplug() 
> is quite long - especially when no CRT is connected.
> 
> What happens intel_crt_detect_hotplug() reads the value of PORT_HOTPLUG_EN
> at entry, then frobs around for ages, during this time several HPD interrupts
> occur, the storm is detected, the bit related to the stormy line is unset
> then after ages intel_crt_detect_hotplug() decides to give up and restores
> the value it read on entry.
> 
> To remedy this this patch reads the value of PORT_HOTPLUG_EN whenever 
> it is going to change it and adds the spin locks to make sure the 
> read-modify-write cycles don't happen concurrently.
> 
> PORT_HOTPLUG_EN is only touched in two places: in xxx_hpd_irq_setup()
> and in intel_crt_detect_hotplug(), the former can be called from an
> interrupt handler.
> 
> Not sure why you see a problem here.

Hm I missed that this same register is also accessed by the irq handler
code, and it's not just that touching these bits can cause interrupts. So
yeah we need your patch, but it needs to be clearer in the commit message
that there's also trouble with concurrent register access to
PORT_HOTPLUG_EN.

Also I think a commen in the code why we grab that spinlock would be good.
For that extracting a small helper to manipulate the register (like we do
with other irq mask registers with functions like ilk_update_gt_irq) would
be good - then we have just one place to put that commment.
-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

Re: [Intel-gfx] [PATCH] drm/i915: Update comments around base bpp

[Daniel Vetter]

On Wed, Aug 26, 2015 at 06:57:26PM +0200, Daniel Vetter wrote:
> Forgot to do that in
> 
> commit d328c9d78d64ca11e744fe227096990430a88477
> Author: Daniel Vetter 
> Date:   Fri Apr 10 16:22:37 2015 +0200
> 
> drm/i915: Select starting pipe bpp irrespective or the primary plane
> 
> and it's confusing. Fix it.
> 
> Cc: Jesse Barnes 
> Signed-off-by: Daniel Vetter 

Merged to dinq with Jesse's irc ack.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 384c575ae65f..957a3680381c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12112,10 +12112,6 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> (DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NVSYNC)))
>   pipe_config->base.adjusted_mode.flags |= DRM_MODE_FLAG_NVSYNC;
>  
> - /* Compute a starting value for pipe_config->pipe_bpp taking the source
> -  * plane pixel format and any sink constraints into account. Returns the
> -  * source plane bpp so that dithering can be selected on mismatches
> -  * after encoders and crtc also have had their say. */
>   base_bpp = compute_baseline_pipe_bpp(to_intel_crtc(crtc),
>pipe_config);
>   if (base_bpp < 0)
> @@ -12184,7 +12180,7 @@ encoder_retry:
>   /* Dithering seems to not pass-through bits correctly when it should, so
>* only enable it on 6bpc panels. */
>   pipe_config->dither = pipe_config->pipe_bpp == 6*3;
> - DRM_DEBUG_KMS("plane bpp: %i, pipe bpp: %i, dithering: %i\n",
> + DRM_DEBUG_KMS("hw max bpp: %i, pipe bpp: %i, dithering: %i\n",
> base_bpp, pipe_config->pipe_bpp, pipe_config->dither);
>  
>  fail:
> -- 
> 2.5.0
> 

-- 
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] [PATCH 1/2] drm/atomic: Make prepare_fb/cleanup_fb only take state, v3.

[Daniel Stone]

On 2 September 2015 at 09:42, Maarten Lankhorst
 wrote:
> This removes the need to separately track fb changes i915.
> That will be done as a separate commit, however.
>
> Changes since v1:
> - Add dri-devel to cc.
> - Fix a check in intel's prepare and cleanup fb to take rotation
>   into account.
> Changes since v2:
> - Split out i915 changes to a separate commit.
>
> Cc: dri-de...@lists.freedesktop.org
> Signed-off-by: Maarten Lankhorst 

I'd probably prefer to see the change to call them unconditionally
(regardless of fb != NULL) in a separate patch, but with or without:
Reviewed-by: Daniel Stone 

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

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt

[Jani Nikula]

On Wed, 02 Sep 2015, Egbert Eich  wrote:
> This is exactly the scenatio I'm getting here. I get HPD interrupts at an 
> order of 10^4 / sec.

Makes you wonder if either you have faulty hardware or we are
configuring the hardware wrong (we overlook some configuration about
some voltage/duration threshold maybe, or get irqs from a line that's
floating, or something).

BR,
Jani.


-- 
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] [RFC 1/3] drm/i915: Roll intel_crtc->atomic into intel_crtc_state

[Maarten Lankhorst]

Op 02-09-15 om 13:15 schreef Ville Syrjälä:
> On Wed, Sep 02, 2015 at 01:08:31PM +0200, Maarten Lankhorst wrote:
>> Op 02-09-15 om 12:35 schreef Ville Syrjälä:
>>> On Wed, Sep 02, 2015 at 07:15:25AM +0200, Maarten Lankhorst wrote:
 Op 01-09-15 om 17:48 schreef Ville Syrjälä:
> On Tue, Sep 01, 2015 at 08:30:05AM -0700, Matt Roper wrote:
>> On Tue, Sep 01, 2015 at 07:24:19AM +0200, Maarten Lankhorst wrote:
>>> Op 29-08-15 om 01:57 schreef Matt Roper:
 Way back at the beginning of i915's atomic conversion I added
 intel_crtc->atomic as a temporary dumping ground for "stuff to do
 outside vblank evasion" flags since CRTC states weren't properly wired
 up and tracked at that time.  We've had proper CRTC state tracking for 
 a
 while now, so there's really no reason for this hack to continue to
 exist.  Moving forward we want to store intermediate crtc/plane state
 data for modesets in addition to the final state, so moving these 
 fields
 into the proper state object allows us to properly compute them for 
 both
 the intermediate and final state.

 Signed-off-by: Matt Roper 
 ---
>>> Can I shoot this patch down? It's better to add a field 'wm_changed'
>>> to the crtc_state, which gets reset to false for each crtc_state
>>> duplication. It's needed for checking if a cs pageflip can be done for
>>> atomic. It would remove the duplication of some checks there.
>>>
>>> The other atomic state members will die soon. I already have some
>>> patches to achieve that. :)
>>>
>>> I'm not sure if an intermediate state is a good idea. Any code that
>>> disables a crtc should only be looking at the old state.
>>> pre_plane_update runs all stuff in preparation for disabling planes,
>>> while post_plane_update runs everything needed for enabling planes. So
>>> no need to split it up I think, maybe put in some intermediate
>>> watermarks in intel_atomic_state, but no need for a full crtc_state.
>> Well, the intermediate state stuff was requested by Ville in response to
>> my watermark series, so I posted these patches as an RFC to make sure I
>> was understanding what he was looking for properly.
>>
>> Ville, can you comment?
> My opinion is that the current "disable is special" way of doing things
> is quite horrible. For one it makes it really hard to reason about what
> happens to a plane or crtc during the modeset. It's not just off->on,
> on->off, or same->same, but can be on->off->on. With the intermediate
> state in place, there can only be one transition, so really easy to
> think about what's going on.
 pre_plane_update deals with all stuff related to disabling planes, while 
 post_plane_update deals with changes after enabling.

 If the crtc goes from on -> off only you could just hammer in the final 
 values after the disable.

 While for off->on or on->off->on you can put in the final values in 
 .crtc_enable before lighting the pipe. I don't see why wm's would need 
 more transitions.
>>> One special case after another. Yuck. Not to mention that the plane
>>> disable isn't even atomic in the current code, which can look ugly.
>> That's easily fixed by adding a pipe_update_start/end pair.
> It'll also mean don't have to sprinkle silly wm update calls all over
> the modeset path. They will just get updated in response to the plane
> state changes. Same for IPS/FBC etc.
 IPS and FBC are already calculated correctly in response to modesets.
>>> Correctly perhaps, but not in an obvious way.
>> It will become more obvious again when pre_plane_update and 
>> post_plane_update are loops
>> instead of being precalculated from intel_plane_atomic_calc_changes.
> It'll never be obvious as long as the on->off->on case exists.
>
But On -> off will always be a special case because any enable might depend on 
the disable, for example taking over the pll or cdclk changes.
It can never be the same, so why pretend it is?
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] scripts/kernel-doc: Improve Markdown results

[Jonathan Corbet]

On Tue, 1 Sep 2015 14:57:33 -0300
Danilo Cesar Lemes de Paula  wrote:

> Did you find time to check this patch? As you mentioned that you applied
> the Markdown support for the linux-next tree, this patch might be needed
> (maybe "wanted" is a better word).

Not quite what I said...I said I'd apply it right after the merge window
so it can sit in linux-next through the full cycle.  It's a bit early to
be pushing 4.4 stuff into linux-next now...

Beyond that, I wasn't sure where things stand with fixes... Can you send
me a new patch set with this fix (and any others that might
exist) integrated in?

Thanks,

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

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt

[Egbert Eich]

Daniel Vetter writes:
 > On Tue, Sep 01, 2015 at 10:21:35PM +0200, Egbert Eich wrote:
 > > A HPD interrupt may fire during intel_crt_detect_hotplug() - especially
 > > when HPD interrupt storms occur.
 > > Since the interrupt handler changes the enabled interrupt lines when it
 > > detects a storm this races with intel_crt_detect_hotplug().
 > > To avoid this, shiled the rmw cycles with IRQ save spinlocks.
 > > 
 > > Signed-off-by: Egbert Eich 
 > 
 > I think this only reduces one source of such races, but fundamentally we
 > can't avoid them. E.g. if you're _very_ unlucky you might cause a real hpd
 > right when the strom code frobs around. Plugging the race with this known

This is exactly the scenatio I'm getting here. I get HPD interrupts at an 
order of 10^4 / sec.

 > interrupt source here doesn't fix the fundamental issue.
 > 
 > Also I think the actual interrupt deliver is fairly asynchronous, both in
 > the hardware and in the sw handling. E.g. spin_lock_irq does not
 > synchronize with the interrupt handler on SMP systems, it only guarantees
 > that it's not running concurrently on the same cpu (which would deadlock).
 > 
 > I think fundamentally this race is unfixable.


There is one important race we avoid with this patch: It is that
we change the PORT_HOTPLUG_EN concurrently in the interrupt handler
(thru xxx_hpd_irq_setup() and in intel_crt_detect_hotplug()).

With the test system that I have here the old version of the code
easily runs into this as the time spent inside intel_crt_detect_hotplug() 
is quite long - especially when no CRT is connected.

What happens intel_crt_detect_hotplug() reads the value of PORT_HOTPLUG_EN
at entry, then frobs around for ages, during this time several HPD interrupts
occur, the storm is detected, the bit related to the stormy line is unset
then after ages intel_crt_detect_hotplug() decides to give up and restores
the value it read on entry.

To remedy this this patch reads the value of PORT_HOTPLUG_EN whenever 
it is going to change it and adds the spin locks to make sure the 
read-modify-write cycles don't happen concurrently.

PORT_HOTPLUG_EN is only touched in two places: in xxx_hpd_irq_setup()
and in intel_crt_detect_hotplug(), the former can be called from an
interrupt handler.

Not sure why you see a problem here.

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

Re: [Intel-gfx] [PATCH] drm/i915: Fix broken mst get_hw_state.

[Maarten Lankhorst]

Op 02-09-15 om 16:07 schreef Ander Conselvan De Oliveira:
> On Thu, 2015-08-27 at 13:13 +0200, Maarten Lankhorst wrote:
>> connector->encoder is initialized as NULL. Fix this by setting it in
>> during pre enable. MST connectors are not read out during initial hw
>> readout, and have no fixed encoder mappings. So it's harmless to
>> return false when the connector has never been assigned to an encoder.
>>
>> Signed-off-by: Maarten Lankhorst 
>> ---
>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 738178a0ac96..e3922d973db0 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -6283,7 +6283,7 @@ static void intel_connector_check_state(struct 
>> intel_connector *connector)
>>connector->base.name);
>>  
>>  if (connector->get_hw_state(connector)) {
>> -struct drm_encoder *encoder = &connector->encoder->base;
>> +struct intel_encoder *encoder = connector->encoder;
>>  struct drm_connector_state *conn_state = connector->base.state;
>>  
>>  I915_STATE_WARN(!crtc,
>> @@ -6295,13 +6295,13 @@ static void intel_connector_check_state(struct 
>> intel_connector *connector)
>>  I915_STATE_WARN(!crtc->state->active,
>>"connector is active, but attached crtc isn't\n");
>>  
>> -if (!encoder)
>> +if (!encoder || encoder->type == INTEL_OUTPUT_DP_MST)
>>  return;
>>  
>> -I915_STATE_WARN(conn_state->best_encoder != encoder,
>> +I915_STATE_WARN(conn_state->best_encoder != &encoder->base,
>>  "atomic encoder doesn't match attached encoder\n");
>>  
>> -I915_STATE_WARN(conn_state->crtc != encoder->crtc,
>> +I915_STATE_WARN(conn_state->crtc != encoder->base.crtc,
>>  "attached encoder crtc differs from connector crtc\n");
>>  } else {
>>  I915_STATE_WARN(crtc && crtc->state->active,
>> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
>> b/drivers/gpu/drm/i915/intel_dp_mst.c
>> index ebf205462631..d606721b1788 100644
>> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
>> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
>> @@ -159,6 +159,11 @@ static void intel_mst_pre_enable_dp(struct 
>> intel_encoder *encoder)
>>  return;
>>  }
>>  
>> +/* MST encoders are bound to a crtc, not to a connector,
>> + * force the mapping here for get_hw_state.
>> + */
>> +found->encoder = encoder;
>> +
>>  DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
>>  intel_mst->port = found->port;
>>  
>> @@ -392,7 +397,7 @@ static const struct drm_encoder_funcs 
>> intel_dp_mst_enc_funcs = {
>>  
>>  static bool intel_dp_mst_get_hw_state(struct intel_connector *connector)
>>  {
>> -if (connector->encoder) {
>> +if (connector->encoder && connector->base.state->crtc) {
> Is this here to cover the case where we disable this connector and assign the 
> encoder to a different
> connector? I guess this should work since if we disable the connector than 
> crtc will be null, and
> we'll assign a proper encoder when re-enabling. But now I'm wondering if it 
> would make sense to set
> connector->encoder back to NULL in post_disable.
>
Yes, it's harmless to keep it non-null if conn_state->crtc is checked though. 
Saves another pointless loop.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC] Docs: drm: Move KMS properties table out to source files

[Jani Nikula]

On Wed, 02 Sep 2015, Graham Whaley  wrote:
>  Documentation/DocBook/drm.tmpl | 925 
> +
>  drivers/gpu/drm/drm_crtc.c |  16 +
>  2 files changed, 17 insertions(+), 924 deletions(-)

I like this already.

BR,
Jani.

-- 
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] [RFC PATCH] drm/i915: Use universal planes for cursor on skylake.

[Maarten Lankhorst]

Op 02-09-15 om 11:15 schreef Daniel Vetter:
> On Thu, Aug 27, 2015 at 12:08:30PM +0200, Maarten Lankhorst wrote:
>> This appears to make all the cursor tests really slow because of the many 
>> calls to skl_update_wm
>> when the cursor plane visibility is changed. It performs does 3 vblanks each 
>> time it's called, and
>> it's probably called more than once on each update.
> On all other platforms wm updates (right now at least) don't do any vblank
> waits, which means changing cursors actually _does_ cause tons of
> underruns. Can we perhaps add a hack in skl to do the same (maybe just for
> cursors) so that we can get this in? There's lots other work that really
> wants proper universal planes ...
>
Well this is easily fixed by moving it to unpin_work, so it runs after the next 
vblank interrupt.

If that patch series is too far out annotate wm changed,
kill all intel_wait_vblank calls inside post_plane_update and run that function 
after drm_atomic_helper_wait_for_vblanks.

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

Re: [Intel-gfx] [PATCH] drm/i915: Fix broken mst get_hw_state.

[Ander Conselvan De Oliveira]

On Thu, 2015-08-27 at 13:13 +0200, Maarten Lankhorst wrote:
> connector->encoder is initialized as NULL. Fix this by setting it in
> during pre enable. MST connectors are not read out during initial hw
> readout, and have no fixed encoder mappings. So it's harmless to
> return false when the connector has never been assigned to an encoder.
>
> Signed-off-by: Maarten Lankhorst 
> ---
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 738178a0ac96..e3922d973db0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6283,7 +6283,7 @@ static void intel_connector_check_state(struct 
> intel_connector *connector)
> connector->base.name);
>  
>   if (connector->get_hw_state(connector)) {
> - struct drm_encoder *encoder = &connector->encoder->base;
> + struct intel_encoder *encoder = connector->encoder;
>   struct drm_connector_state *conn_state = connector->base.state;
>  
>   I915_STATE_WARN(!crtc,
> @@ -6295,13 +6295,13 @@ static void intel_connector_check_state(struct 
> intel_connector *connector)
>   I915_STATE_WARN(!crtc->state->active,
> "connector is active, but attached crtc isn't\n");
>  
> - if (!encoder)
> + if (!encoder || encoder->type == INTEL_OUTPUT_DP_MST)
>   return;
>  
> - I915_STATE_WARN(conn_state->best_encoder != encoder,
> + I915_STATE_WARN(conn_state->best_encoder != &encoder->base,
>   "atomic encoder doesn't match attached encoder\n");
>  
> - I915_STATE_WARN(conn_state->crtc != encoder->crtc,
> + I915_STATE_WARN(conn_state->crtc != encoder->base.crtc,
>   "attached encoder crtc differs from connector crtc\n");
>   } else {
>   I915_STATE_WARN(crtc && crtc->state->active,
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
> b/drivers/gpu/drm/i915/intel_dp_mst.c
> index ebf205462631..d606721b1788 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -159,6 +159,11 @@ static void intel_mst_pre_enable_dp(struct intel_encoder 
> *encoder)
>   return;
>   }
>  
> + /* MST encoders are bound to a crtc, not to a connector,
> +  * force the mapping here for get_hw_state.
> +  */
> + found->encoder = encoder;
> +
>   DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
>   intel_mst->port = found->port;
>  
> @@ -392,7 +397,7 @@ static const struct drm_encoder_funcs 
> intel_dp_mst_enc_funcs = {
>  
>  static bool intel_dp_mst_get_hw_state(struct intel_connector *connector)
>  {
> - if (connector->encoder) {
> + if (connector->encoder && connector->base.state->crtc) {

Is this here to cover the case where we disable this connector and assign the 
encoder to a different
connector? I guess this should work since if we disable the connector than crtc 
will be null, and
we'll assign a proper encoder when re-enabling. But now I'm wondering if it 
would make sense to set
connector->encoder back to NULL in post_disable.

Ander

>   enum pipe pipe;
>   if (!connector->encoder->get_hw_state(connector->encoder, 
> &pipe))
>   return false;
> 
> ___
> 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 4/5] Documentation: drm: Convert KMS Properties HTML table to CALS

[Graham Whaley]

On Tue, 2015-09-01 at 14:56 -0300, Danilo Cesar Lemes de Paula wrote:
> On 08/25/2015 01:10 PM, Graham Whaley wrote:
> > On Tue, 2015-08-25 at 16:29 +0200, Daniel Vetter wrote:
> > > On Tue, Aug 25, 2015 at 10:26:44AM +0100, Graham Whaley wrote:
> > > > The KMS Properties table is in HTML format, which is not
> > > > supported
> > > > for building pdfdocs, resulting in the following types of
> > > > errors:
> > > > 
> > > >  jade:/Documentation/DocBook/drm.xml:34413:15:E: there is no
> > > > attribute
> > > >  "border"
> > > >  jade:/Documentation/DocBook/drm.xml:34413:31:E: there is no
> > > > attribute
> > > >  "cellpadding"
> > > >  jade:/Documentation/DocBook/drm.xml:34413:47:E: there is no
> > > > attribute
> > > >  "cellspacing"
> > > >  jade:/Documentation/DocBook/drm.xml:34414:7:E: document type
> > > > does
> > > > not
> > > >  allow element "tbody" here
> > > > 
> > > > Convert the table over to a CALS format table
> > > 
> > > Hm, long-term plan was to move this table into DOC: comments in
> > > the
> > > source-code using markdown, which we now have (at least in
> > > drm-intel-nightly and also planned to be merged into 4.4). Since
> > > this
> > > is
> > > both a lot of churn I'd like to get there in just 1 step ...
> > > -Daniel
> > First - I've just noted an erroneous debug comment (or two) left in
> > this patch as well, so looks like I will have to re-issue the
> > series
> > anyway.
> > 
> > OK. I guess this comes down to a matter of timing...
> > From Danilos patch of: f6d6913 (drm/doc: Convert to markdown)
> > we can see markdown does not natively support tables, and we'd have
> > to
> > make this a fixed width layout like the one in that patch I
> > suspect.
> > Danilo - any advice on how you did that other table conversion? I
> > just
> > did a pandoc docbook->markdown_github and it looks some way there -
> > but
> > of course seems to have not honored the multi-column items, of
> > which
> > there are a few. It's probably not too bad to fix up by hand - I'll
> > see
> > if I can get that to work...
> 
> Hi Graham,
> 
> To be honest I didn't have to do any conversion as that table was
> already in the header file. I just added 4 spaces so it would be
> transformed into fixed width.
> 
> However, there's tool you can use to help you: http://pandoc.org/try/
> I did a lot of translation there. If your table doesn't have any
> spancells, you can put the HTML code there and get the Markdown for
> free.
> 
> Danilo

Hi,
 following this email should be an [RFC] patch with the subject:
  [RFC] Docs: drm: Move KMS properties table out to source files

This is not a fix - it is an example to show that maybe this table does
not migrate well to markdown. Danilo, if you have any thoughts about
the 'quote expansion' detailed in the patch I'd be interested. Daniel,
we should probably consider if moving this table to markdown will work
out given the width of the table and the restrictions of having to use
fixed width for multi-row markdown table representation.

 Graham




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

Re: [Intel-gfx] [PATCH v2] drm/i915/bdw: Check for slice, subslice and EU count for BDW

[Chris Wilson]

On Wed, Sep 02, 2015 at 05:47:58PM +0200, Łukasz Daniluk wrote:
> +static void broadwell_sseu_device_status(struct drm_device *dev,

Why pass in dev if you only use dev_priv (and commit the sin of
repeatedly retrieving dev->dev_priv)?

> +  struct sseu_dev_status *stat)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + int s;
> + u32 slice_info = I915_READ(GEN8_GT_SLICE_INFO);
> +
> + stat->slice_total =
> + hweight32(slice_info & GEN8_LSLICESTAT_MASK);
> +
> + if (stat->slice_total)
> + {
Missed.

> + stat->subslice_per_slice = INTEL_INFO(dev)->subslice_per_slice;
> + stat->eu_per_subslice = INTEL_INFO(dev)->eu_per_subslice;
> + stat->subslice_total = stat->slice_total * 
> stat->subslice_per_slice;
> + stat->eu_total = stat->eu_per_subslice * stat->subslice_total;
> +
> + /* subtract fused off EU(s) from enabled slice(s) */
> + for (s = 0; s < stat->slice_total; s++) {
> + u8 subslice_7eu = INTEL_INFO(dev)->subslice_7eu[s];
> + stat->eu_total -= hweight8(subslice_7eu);
> + }

So why are we computing this twice? Is the debugfs to show the hw
registers or the sw tracking? Where do I see what we actually report? If
it is for the hw registers, report them. A perfect function here would
report the actual register values (possibly decoding them) and show what
the internal values are.
-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 v2] drm/i915/bdw: Check for slice, subslice and EU count for BDW

[Łukasz Daniluk]

Added checks for available slices, subslices and EUs for Broadwell. This
information is filled in intel_device_info and is available to user with
GET_PARAM.
Added checks for enabled slices, subslices and EU for Broadwell. This
information is based on available counts but takes power gated slices
into account. It can be read in debugfs.
Introduce new register defines that contain information on slices on
Broadwell.

v2:
- Introduce GT_SLICE_INFO register
- Change Broadwell sseu_device_status function to use GT_SLICE_INFO
  register instead of RPCS register
- Undo removal of dev_priv variables in Cherryview and Gen9
  sseu_device_satus functions

Cc: Jeff Mcgee 
Signed-off-by: Łukasz Daniluk 
---
 drivers/gpu/drm/i915/i915_debugfs.c | 29 +++-
 drivers/gpu/drm/i915/i915_dma.c | 89 +
 drivers/gpu/drm/i915/i915_reg.h | 22 -
 3 files changed, 137 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 23a69307..e8a62ef 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4932,13 +4932,38 @@ static void gen9_sseu_device_status(struct drm_device 
*dev,
}
 }
 
+static void broadwell_sseu_device_status(struct drm_device *dev,
+struct sseu_dev_status *stat)
+{
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   int s;
+   u32 slice_info = I915_READ(GEN8_GT_SLICE_INFO);
+
+   stat->slice_total =
+   hweight32(slice_info & GEN8_LSLICESTAT_MASK);
+
+   if (stat->slice_total)
+   {
+   stat->subslice_per_slice = INTEL_INFO(dev)->subslice_per_slice;
+   stat->eu_per_subslice = INTEL_INFO(dev)->eu_per_subslice;
+   stat->subslice_total = stat->slice_total * 
stat->subslice_per_slice;
+   stat->eu_total = stat->eu_per_subslice * stat->subslice_total;
+
+   /* subtract fused off EU(s) from enabled slice(s) */
+   for (s = 0; s < stat->slice_total; s++) {
+   u8 subslice_7eu = INTEL_INFO(dev)->subslice_7eu[s];
+   stat->eu_total -= hweight8(subslice_7eu);
+   }
+   }
+}
+
 static int i915_sseu_status(struct seq_file *m, void *unused)
 {
struct drm_info_node *node = (struct drm_info_node *) m->private;
struct drm_device *dev = node->minor->dev;
struct sseu_dev_status stat;
 
-   if ((INTEL_INFO(dev)->gen < 8) || IS_BROADWELL(dev))
+   if (INTEL_INFO(dev)->gen < 8)
return -ENODEV;
 
seq_puts(m, "SSEU Device Info\n");
@@ -4963,6 +4988,8 @@ static int i915_sseu_status(struct seq_file *m, void 
*unused)
memset(&stat, 0, sizeof(stat));
if (IS_CHERRYVIEW(dev)) {
cherryview_sseu_device_status(dev, &stat);
+   } else if (IS_BROADWELL(dev)) {
+   broadwell_sseu_device_status(dev, &stat);
} else if (INTEL_INFO(dev)->gen >= 9) {
gen9_sseu_device_status(dev, &stat);
}
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index ab37d11..2d52b1e 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -705,6 +705,93 @@ static void gen9_sseu_info_init(struct drm_device *dev)
info->has_eu_pg = (info->eu_per_subslice > 2);
 }
 
+static void broadwell_sseu_info_init(struct drm_device *dev)
+{
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_device_info *info;
+   const int s_max = 3, ss_max = 3, eu_max = 8;
+   int s, ss;
+   u32 fuse2, eu_disable[s_max], s_enable, ss_disable;
+
+   fuse2 = I915_READ(GEN8_FUSE2);
+   s_enable = (fuse2 & GEN8_F2_S_ENA_MASK) >>
+   GEN8_F2_S_ENA_SHIFT;
+   ss_disable = (fuse2 & GEN8_F2_SS_DIS_MASK) >>
+   GEN8_F2_SS_DIS_SHIFT;
+
+   eu_disable[0] = I915_READ(GEN8_EU_DISABLE0) &
+   GEN8_EU_DIS0_S0_MASK;
+   eu_disable[1] = (I915_READ(GEN8_EU_DISABLE0) >>
+   GEN8_EU_DIS0_S1_SHIFT) |
+   ((I915_READ(GEN8_EU_DISABLE1) &
+ GEN8_EU_DIS1_S1_MASK) <<
+(32 - GEN8_EU_DIS0_S1_SHIFT));
+   eu_disable[2] = (I915_READ(GEN8_EU_DISABLE1) >>
+   GEN8_EU_DIS1_S2_SHIFT) |
+   ((I915_READ(GEN8_EU_DISABLE2) &
+ GEN8_EU_DIS2_S2_MASK) <<
+(32 - GEN8_EU_DIS1_S2_SHIFT));
+
+
+   info = (struct intel_device_info *)&dev_priv->info;
+   info->slice_total = hweight32(s_enable);
+
+   /*
+* The subslice disable field is global, i.e. it applies
+* to each of the enabled slices.
+*/
+   info->subslice_per_slice = ss_max - hweight32(ss_disable);
+   info->subslice_total = info->slice_total *
+   info->subslice_per_slice;
+
+   /*
+* Iterate through enabled slices and 

[Intel-gfx] [RFC] Docs: drm: Move KMS properties table out to source files

[Graham Whaley]

(RFC/test - not for merging)
The below is a test of moving the large HTML KMS properties table out
to markdown style in the appropriate files.
In the test we only use the first few rows of the existing KMS table
an example.
We use a fixed width table as the other styles of table supported by
pandoc markdown do not support multi-column cells.

The test shows a couple of issues:
 1) double quote characters are being expanded in the fixed width table
 which then breaks the table alignment and leaves html style  tags
 in the text

 2) Cramming the seven columns into the apprx 80ish column width makes
 some entries fairly impractical - one word per row. For reference,
 pdfdocs rendering clips the fixed text tables at around 80 characters.

If we can:
 a) Resolve (1) above
and
 b) Agree that we are OK with comment fields extending beyond the 80th
 column significantly

then maybe we can continue looking at moving the KMS properties table out
of drm.tmpl and into markdown format fragments in the source files.
If not, then maybe we go back to considering the conversion of the KMS
table to docbook CALS format.
---
 Documentation/DocBook/drm.tmpl | 925 +
 drivers/gpu/drm/drm_crtc.c |  16 +
 2 files changed, 17 insertions(+), 924 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 66bc646..ecfd084 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2573,930 +2573,7 @@ void intel_crt_init(struct drm_device *dev)
The following table gives description of drm properties exposed by 
various
modules/drivers.

-   
-   
-   
-   Owner Module/Drivers
-   Group
-   Property Name
-   Type
-   Property Values
-   Object attached
-   Description/Restrictions
-   
-   
-   DRM
-   Generic
-   “rotation”
-   BITMASK
-   { 0, "rotate-0" },
-   { 1, "rotate-90" },
-   { 2, "rotate-180" },
-   { 3, "rotate-270" },
-   { 4, "reflect-x" },
-   { 5, "reflect-y" }
-   CRTC, Plane
-   rotate-(degrees) rotates the image by the specified 
amount in degrees
-   in counter clockwise direction. reflect-x and reflect-y reflects the
-   image along the specified axis prior to rotation
-   
-   
-   Connector
-   “EDID”
-   BLOB | IMMUTABLE
-   0
-   Connector
-   Contains id of edid blob ptr object.
-   
-   
-   “DPMS”
-   ENUM
-   { “On”, “Standby”, “Suspend”, “Off” }
-   Connector
-   Contains DPMS operation mode value.
-   
-   
-   “PATH”
-   BLOB | IMMUTABLE
-   0
-   Connector
-   Contains topology path to a connector.
-   
-   
-   “TILE”
-   BLOB | IMMUTABLE
-   0
-   Connector
-   Contains tiling information for a connector.
-   
-   
-   “CRTC_ID”
-   OBJECT
-   DRM_MODE_OBJECT_CRTC
-   Connector
-   CRTC that connector is attached to (atomic)
-   
-   
-   Plane
-   “type”
-   ENUM | IMMUTABLE
-   { "Overlay", "Primary", "Cursor" }
-   Plane
-   Plane type
-   
-   
-   “SRC_X”
-   RANGE
-   Min=0, Max=UINT_MAX
-   Plane
-   Scanout source x coordinate in 16.16 fixed point 
(atomic)
-   
-   
-   “SRC_Y”
-   RANGE
-   Min=0, Max=UINT_MAX
-   Plane
-   Scanout source y coordinate in 16.16 fixed point 
(atomic)
-   
-   
-   “SRC_W”
-   RANGE
-   Min=0, Max=UINT_MAX
-   Plane
-   Scanout source width in 16.16 fixed point 
(atomic)
-   
-   
-   “SRC_H”
-   RANGE
-   Min=0, Max=UINT_MAX
-   Plane
-   Scanout source height in 16.16 fixed point 
(atomic)
-   
-   
-   “CRTC_X”
-   SIGNED_RANGE
-   Min=INT_MIN, Max=INT_MAX
-   Plane
-   Scanout CRTC (destination) x coordinate (atomic)
-   
-   
-   “CRTC_Y”
-   SIGNED_RANGE
-   Min=INT_MIN, Max=INT_MAX
-   Plane
-   Scanout CRTC (destination) y coordinate (atomic)
-   
-   
-   “CRTC_W”
-   RANGE
-   Min=0, Max=UINT_MAX
-   Plane
-   Scanout CRTC (destination) width (atomic)
-   
-   
-   “CRTC_H”
-   RANGE
-   Min=0, Max=UINT_MAX
-   Plane
-   Scanout CRTC (destination) height (atomic)
-   
-   
-   “FB_ID”
-   OBJECT
-   DRM_MODE_OBJECT_FB
-   Plane
-   Scanout framebuffer (atomic)
-   
-   
-   “CRTC_ID”
-   OBJECT
-   DRM_MODE_OBJECT_CRTC
-   Plane
-   CRTC that plane is attached to (atomic)
-   
-   
-   DVI-I
-   “subconnector”
-   ENUM
-   { “Unknown”, “DVI-D”, “DVI-A” }
-   Connector
-   TBD
-   
-   
-   “select subconnector”
-   ENUM
-   { “Automatic”, “DVI-D”, “DVI-A” }
-   Connector
-   TBD
-   
-   
-   TV
-  

Re: [Intel-gfx] [PATCH v3] drm/i915/gtt: Avoid calling kcalloc in a loop when allocating temp bitmaps

[Chris Wilson]

On Wed, Sep 02, 2015 at 02:40:03PM +0100, Michel Thierry wrote:
> On 9/1/2015 10:06 AM, Michał Winiarski wrote:
> >On each call to gen8_alloc_va_range_3lvl we're allocating temporary
> >bitmaps needed for error handling. Unfortunately, when we increase
> >address space size (48b ppgtt) we do additional (512 - 4) calls to
> >kcalloc, increasing latency between exec and actual start of execution
> >on the GPU. Let's just do a single kcalloc, we can also drop the size
> >from free_gen8_temp_bitmaps since it's no longer used.
> >
> >v2: Use GFP_TEMPORARY to make the allocations reclaimable.
> >v3: Drop the 2D array, just allocate a single block.
> >
> >Cc: Chris Wilson 
> >Cc: Mika Kuoppala 
> >Cc: Michel Thierry 
> >Signed-off-by: Michał Winiarski 
> 
> Unless Chris thinks otherwise, I see Michał already addressed his comments.

I think I spotted a misaligned bracket... ;)
Reviewed-by: Chris Wilson 
-Chri

-- 
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 v6 4/4] drm/i915: set proper N/CTS in modeset

[Takashi Iwai]

On Wed, 02 Sep 2015 15:44:34 +0200,
Jani Nikula wrote:
> 
> On Wed, 02 Sep 2015, Takashi Iwai  wrote:
> > On Wed, 02 Sep 2015 11:02:42 +0200,
> > Jani Nikula wrote:
> >> 
> >> >> Nitpick. I'd prefer some sharing with the similar blocks from the
> >> >> earlier patch. Also a debug message on n == 0 would be nice; you
> >> >> probably didn't notice your audio_config_get_rate() wasn't working
> >> >> right
> >> >> because this silently fell back to the automatic mode here.
> >> >
> >> > OK, I will add the msg. As you and Ville are insisting on
> >> > sharing code, I will do it in next version.
> >> 
> >> Well, really, I'm fine with having that part duplicated as-is for now,
> >> we can fix it later. More important to focus on getting
> >> audio_config_get_rate() right.
> >> 
> >> I don't know if you're still targeting v4.3 with this (up to Takashi I
> >> guess) we'll really need to wrap this up soon.
> >
> > I'm in favor of merging this into 4.3, so it'd be appreciated if Libin
> > can prepare the fixed version soonish, indeed.
> 
> IIUC patches 1-3 would be useful on their own already, and a fixed
> version of patch 4 could follow. Just a thought.

That's good, then I can take the first three patches.

Daniel, would you like to review these three before I merge them?
I assumed your previous mail as a kind of ack to this series, too.


thanks,

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

Re: [Intel-gfx] [PATCH v6 4/4] drm/i915: set proper N/CTS in modeset

[Jani Nikula]

On Wed, 02 Sep 2015, Takashi Iwai  wrote:
> On Wed, 02 Sep 2015 11:02:42 +0200,
> Jani Nikula wrote:
>> 
>> >> Nitpick. I'd prefer some sharing with the similar blocks from the
>> >> earlier patch. Also a debug message on n == 0 would be nice; you
>> >> probably didn't notice your audio_config_get_rate() wasn't working
>> >> right
>> >> because this silently fell back to the automatic mode here.
>> >
>> > OK, I will add the msg. As you and Ville are insisting on
>> > sharing code, I will do it in next version.
>> 
>> Well, really, I'm fine with having that part duplicated as-is for now,
>> we can fix it later. More important to focus on getting
>> audio_config_get_rate() right.
>> 
>> I don't know if you're still targeting v4.3 with this (up to Takashi I
>> guess) we'll really need to wrap this up soon.
>
> I'm in favor of merging this into 4.3, so it'd be appreciated if Libin
> can prepare the fixed version soonish, indeed.

IIUC patches 1-3 would be useful on their own already, and a fixed
version of patch 4 could follow. Just a thought.

BR,
Jani.

>
>
> thanks,
>
> Takashi

-- 
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] [PATCH v3] drm/i915/gtt: Avoid calling kcalloc in a loop when allocating temp bitmaps

[Michel Thierry]

On 9/1/2015 10:06 AM, Michał Winiarski wrote:

On each call to gen8_alloc_va_range_3lvl we're allocating temporary
bitmaps needed for error handling. Unfortunately, when we increase
address space size (48b ppgtt) we do additional (512 - 4) calls to
kcalloc, increasing latency between exec and actual start of execution
on the GPU. Let's just do a single kcalloc, we can also drop the size
from free_gen8_temp_bitmaps since it's no longer used.

v2: Use GFP_TEMPORARY to make the allocations reclaimable.
v3: Drop the 2D array, just allocate a single block.

Cc: Chris Wilson 
Cc: Mika Kuoppala 
Cc: Michel Thierry 
Signed-off-by: Michał Winiarski 


Unless Chris thinks otherwise, I see Michał already addressed his comments.

Reviewed-by: Michel Thierry 


---
  drivers/gpu/drm/i915/i915_gem_gtt.c | 45 +
  1 file changed, 16 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 4a76807..f810762 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1126,13 +1126,8 @@ unwind_out:
  }

  static void
-free_gen8_temp_bitmaps(unsigned long *new_pds, unsigned long **new_pts,
-  uint32_t pdpes)
+free_gen8_temp_bitmaps(unsigned long *new_pds, unsigned long *new_pts)
  {
-   int i;
-
-   for (i = 0; i < pdpes; i++)
-   kfree(new_pts[i]);
kfree(new_pts);
kfree(new_pds);
  }
@@ -1142,29 +1137,20 @@ free_gen8_temp_bitmaps(unsigned long *new_pds, unsigned 
long **new_pts,
   */
  static
  int __must_check alloc_gen8_temp_bitmaps(unsigned long **new_pds,
-unsigned long ***new_pts,
+unsigned long **new_pts,
 uint32_t pdpes)
  {
-   int i;
unsigned long *pds;
-   unsigned long **pts;
+   unsigned long *pts;

-   pds = kcalloc(BITS_TO_LONGS(pdpes), sizeof(unsigned long), GFP_KERNEL);
+   pds = kcalloc(BITS_TO_LONGS(pdpes), sizeof(unsigned long), 
GFP_TEMPORARY);
if (!pds)
return -ENOMEM;

-   pts = kcalloc(pdpes, sizeof(unsigned long *), GFP_KERNEL);
-   if (!pts) {
-   kfree(pds);
-   return -ENOMEM;
-   }
-
-   for (i = 0; i < pdpes; i++) {
-   pts[i] = kcalloc(BITS_TO_LONGS(I915_PDES),
-sizeof(unsigned long), GFP_KERNEL);
-   if (!pts[i])
-   goto err_out;
-   }
+   pts = kcalloc(pdpes * BITS_TO_LONGS(I915_PDES),
+   sizeof(unsigned long), GFP_TEMPORARY);
+   if (!pts)
+   goto err_out;

*new_pds = pds;
*new_pts = pts;
@@ -1172,7 +1158,7 @@ int __must_check alloc_gen8_temp_bitmaps(unsigned long 
**new_pds,
return 0;

  err_out:
-   free_gen8_temp_bitmaps(pds, pts, pdpes);
+   free_gen8_temp_bitmaps(pds, pts);
return -ENOMEM;
  }

@@ -1193,7 +1179,7 @@ static int gen8_alloc_va_range_3lvl(struct 
i915_address_space *vm,
  {
struct i915_hw_ppgtt *ppgtt =
container_of(vm, struct i915_hw_ppgtt, base);
-   unsigned long *new_page_dirs, **new_page_tables;
+   unsigned long *new_page_dirs, *new_page_tables;
struct drm_device *dev = vm->dev;
struct i915_page_directory *pd;
const uint64_t orig_start = start;
@@ -1220,14 +1206,14 @@ static int gen8_alloc_va_range_3lvl(struct 
i915_address_space *vm,
ret = gen8_ppgtt_alloc_page_directories(vm, pdp, start, length,
new_page_dirs);
if (ret) {
-   free_gen8_temp_bitmaps(new_page_dirs, new_page_tables, pdpes);
+   free_gen8_temp_bitmaps(new_page_dirs, new_page_tables);
return ret;
}

/* For every page directory referenced, allocate page tables */
gen8_for_each_pdpe(pd, pdp, start, length, temp, pdpe) {
ret = gen8_ppgtt_alloc_pagetabs(vm, pd, start, length,
-   new_page_tables[pdpe]);
+   new_page_tables + pdpe * 
BITS_TO_LONGS(I915_PDES));
if (ret)
goto err_out;
}
@@ -1278,20 +1264,21 @@ static int gen8_alloc_va_range_3lvl(struct 
i915_address_space *vm,
gen8_setup_page_directory(ppgtt, pdp, pd, pdpe);
}

-   free_gen8_temp_bitmaps(new_page_dirs, new_page_tables, pdpes);
+   free_gen8_temp_bitmaps(new_page_dirs, new_page_tables);
mark_tlbs_dirty(ppgtt);
return 0;

  err_out:
while (pdpe--) {
-   for_each_set_bit(temp, new_page_tables[pdpe], I915_PDES)
+   for_each_set_bit(temp, new_page_tables + pdpe *
+   BITS_TO_LONGS(I915_PDES), I915_PDES)
free_pt(dev, 
pdp->page_dir

Re: [Intel-gfx] [PATCH 3/4] drm/i915: Use the correct hpd_status list for non-G4xx/VLV

[Jani Nikula]

On Wed, 02 Sep 2015, Imre Deak  wrote:
> On ke, 2015-09-02 at 14:00 +0200, Daniel Vetter wrote:
>> On Tue, Sep 01, 2015 at 10:21:34PM +0200, Egbert Eich wrote:
>> > This copy-and-past error was introduced in:
>> > 
>> > commit fd63e2a972c670887e5e8a08440111d3812c0996
>> > Author: Imre Deak 
>> > Date:   Tue Jul 21 15:32:44 2015 -0700
>> > 
>> > drm/i915: combine i9xx_get_hpd_pins and pch_get_hpd_pins
>> > 
>> > Signed-off-by: Egbert Eich 
>> 
>> I recommend to cc: the authors/reviewers of the patch which broke things
>> so they have a better chance to spot your patch and make amends by quickly
>> reviewing your fix. Anyway Reviewed-by: Daniel Vetter 
>> 
>
> Thanks for catching this. Ville also noticed it and has sent already a
> fix for it:
> http://lists.freedesktop.org/archives/intel-gfx/2015-August/074676.html

I pushed that one as it came first.

BR,
Jani.


>
> --Imre
>
>> 
>> Jani, this all seems to be for you.
>> -Daniel
>> > ---
>> >  drivers/gpu/drm/i915/i915_irq.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/i915_irq.c 
>> > b/drivers/gpu/drm/i915/i915_irq.c
>> > index 8485bea..ad99dae 100644
>> > --- a/drivers/gpu/drm/i915/i915_irq.c
>> > +++ b/drivers/gpu/drm/i915/i915_irq.c
>> > @@ -1559,7 +1559,7 @@ static void i9xx_hpd_irq_handler(struct drm_device 
>> > *dev)
>> >u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915;
>> >  
>> >intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
>> > - hotplug_trigger, hpd_status_g4x,
>> > + hotplug_trigger, hpd_status_i915,
>> >   i9xx_port_hotplug_long_detect);
>> >intel_hpd_irq_handler(dev, pin_mask, long_mask);
>> >}
>> > -- 
>> > 1.8.4.5
>> > 
>> 
>
>

-- 
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] [PATCH 17/17] drm/i915: Pass hpd_status_i915[] to intel_get_hpd_pins() in pre-g4x

[Jani Nikula]

On Sat, 29 Aug 2015, Paulo Zanoni  wrote:
> 2015-08-27 17:56 GMT-03:00  :
>> From: Ville Syrjälä 
>>
>> Pass the correct hpd[] array to intel_get_hpd_pins() on pre-g4x
>> platforms.
>>
>> This got broken in the following commit:
>> commit fd63e2a972c670887e5e8a08440111d3812c0996
>> Author: Imre Deak 
>> Date:   Tue Jul 21 15:32:44 2015 -0700
>>
>> drm/i915: combine i9xx_get_hpd_pins and pch_get_hpd_pins
>
> The good & old "copy, paste, then adjust only one of the two variable
> names" error.
>
> Reviewed-by: Paulo Zanoni 

Pushed this one patch to drm-intel-next-fixes, thanks for the patch and
review.

I took the liberty of adding Reviewed-by also from Egbert and Daniel, as
Egbert submitted an identical patch [1] which Daniel reviewed. Patch
selection was purely on a first come first applied basis.

BR,
Jani.

[1] http://mid.gmane.org/1441138895-23732-4-git-send-email-e...@suse.de


>
>>
>> Cc: Imre Deak 
>> Signed-off-by: Ville Syrjälä 
>> ---
>>  drivers/gpu/drm/i915/i915_irq.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c 
>> b/drivers/gpu/drm/i915/i915_irq.c
>> index 1a29dfd..9866739 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -1644,7 +1644,7 @@ static void i9xx_hpd_irq_handler(struct drm_device 
>> *dev)
>> u32 hotplug_trigger = hotplug_status & 
>> HOTPLUG_INT_STATUS_I915;
>>
>> intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
>> -  hotplug_trigger, hpd_status_g4x,
>> +  hotplug_trigger, hpd_status_i915,
>>i9xx_port_hotplug_long_detect);
>> intel_hpd_irq_handler(dev, pin_mask, long_mask);
>> }
>> --
>> 2.4.6
>>
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> -- 
> Paulo Zanoni
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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] [PATCH v6 4/4] drm/i915: set proper N/CTS in modeset

[Takashi Iwai]

On Wed, 02 Sep 2015 11:02:42 +0200,
Jani Nikula wrote:
> 
> >> Nitpick. I'd prefer some sharing with the similar blocks from the
> >> earlier patch. Also a debug message on n == 0 would be nice; you
> >> probably didn't notice your audio_config_get_rate() wasn't working
> >> right
> >> because this silently fell back to the automatic mode here.
> >
> > OK, I will add the msg. As you and Ville are insisting on
> > sharing code, I will do it in next version.
> 
> Well, really, I'm fine with having that part duplicated as-is for now,
> we can fix it later. More important to focus on getting
> audio_config_get_rate() right.
> 
> I don't know if you're still targeting v4.3 with this (up to Takashi I
> guess) we'll really need to wrap this up soon.

I'm in favor of merging this into 4.3, so it'd be appreciated if Libin
can prepare the fixed version soonish, indeed.


thanks,

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

Re: [Intel-gfx] [PATCH] drm/i915: Always mark the object as dirty when used by the GPU

[Jani Nikula]

On Wed, 02 Sep 2015, Daniel Vetter  wrote:
> On Mon, Aug 31, 2015 at 03:10:39PM +0100, Chris Wilson wrote:
>> There have been many hard to track down bugs whereby userspace forgot to
>> flag a write buffer and then cause graphics corruption or a hung GPU
>> when that buffer was later purged under memory pressure (as the buffer
>> appeared clean, its pages would have been evicted rather than preserved
>> and any changes more recent than in the backing storage would be lost).
>> In retrospect this is a rare optimisation against memory pressure,
>> already the slow path. If we always mark the buffer as dirty when
>> accessed by the GPU, anything not used can still be evicted cheaply
>> (ideal behaviour for mark-and-sweep eviction) but we do not run the risk
>> of corruption. For correct read serialisation, userspace still has to
>> notify when the GPU writes to an object. However, there are certain
>> situations under which userspace may wish to tell white lies to the
>> kernel...
>> 
>> Signed-off-by: Chris Wilson 
>> Cc: Daniel Vetter 
>> Cc: Kristian Høgsberg 
>> Cc: Jesse Barnes 
>> Cc: "Goel, Akash" 
>> Cc: Michał Winiarski 
>> Cc: sta...@vger.kernel.org
>
> Reviewed-by: Daniel Vetter 

Pushed to drm-intel-next-fixes, thanks for the patch and review.

BR,
Jani.

>
>> ---
>>  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
>> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>> index 923a3c4bf0b7..a953d4975b8c 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>> @@ -1032,6 +1032,7 @@ i915_gem_execbuffer_move_to_active(struct list_head 
>> *vmas,
>>  u32 old_read = obj->base.read_domains;
>>  u32 old_write = obj->base.write_domain;
>>  
>> +obj->dirty = 1; /* be paranoid  */
>>  obj->base.write_domain = obj->base.pending_write_domain;
>>  if (obj->base.write_domain == 0)
>>  obj->base.pending_read_domains |= 
>> obj->base.read_domains;
>> @@ -1039,7 +1040,6 @@ i915_gem_execbuffer_move_to_active(struct list_head 
>> *vmas,
>>  
>>  i915_vma_move_to_active(vma, req);
>>  if (obj->base.write_domain) {
>> -obj->dirty = 1;
>>  i915_gem_request_assign(&obj->last_write_req, req);
>>  
>>  intel_fb_obj_invalidate(obj, ORIGIN_CS);
>> -- 
>> 2.5.1
>> 
>
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
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] [PATCH] uapi/drm/i915_drm.h: fix userspace compilation.

[Jani Nikula]

On Wed, 02 Sep 2015, Daniel Vetter  wrote:
> On Wed, Sep 02, 2015 at 02:52:19PM +0300, Ville Syrjälä wrote:
>> On Wed, Sep 02, 2015 at 01:41:18PM +0200, Artem Savkov wrote:
>> > Patch "drm/i915: Use expcitly fixed type in compat32 structs" changed the 
>> > type
>> > of param field in drm_i915_getparam from int to s32. This header is 
>> > exported to
>> > userspace and needs to use userspace type __s32 instead.
>> > 
>> > This fixes userspace compilation errors like the following:
>> > include/drm/i915_drm.h:361:2: error: unknown type name 's32'
>> >   s32 param;
>> > 
>> > Signed-off-by: Artem Savkov 
>> > ---
>> >  include/uapi/drm/i915_drm.h | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > 
>> > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>> > index dbd16a2..fd5aa47 100644
>> > --- a/include/uapi/drm/i915_drm.h
>> > +++ b/include/uapi/drm/i915_drm.h
>> > @@ -358,7 +358,7 @@ typedef struct drm_i915_irq_wait {
>> >  #define I915_PARAM_HAS_RESOURCE_STREAMER 36
>> >  
>> >  typedef struct drm_i915_getparam {
>> > -  s32 param;
>> > +  __s32 param;
>> 
>> Hmm. I don't understand why this one in particular got changed to s32
>> when there are other ioctl structs still using int.
>
> Mostly me being incompetent.
>
> Reviewed-by: Daniel Vetter  on this one, not that
> it seems to be worth much ...

Pushed to drm-intel-next-fixes, thanks for the patch and review.

BR,
Jani.


> -Daniel
>
>> 
>> >/*
>> > * WARNING: Using pointers instead of fixed-size u64 means we need to 
>> > write
>> > * compat32 code. Don't repeat this mistake.
>> > -- 
>> > 2.1.0
>> > 
>> > ___
>> > 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
>
> -- 
> 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

-- 
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] [PATCH 3/4] drm/i915: Use the correct hpd_status list for non-G4xx/VLV

[Imre Deak]

On ke, 2015-09-02 at 14:00 +0200, Daniel Vetter wrote:
> On Tue, Sep 01, 2015 at 10:21:34PM +0200, Egbert Eich wrote:
> > This copy-and-past error was introduced in:
> > 
> > commit fd63e2a972c670887e5e8a08440111d3812c0996
> > Author: Imre Deak 
> > Date:   Tue Jul 21 15:32:44 2015 -0700
> > 
> > drm/i915: combine i9xx_get_hpd_pins and pch_get_hpd_pins
> > 
> > Signed-off-by: Egbert Eich 
> 
> I recommend to cc: the authors/reviewers of the patch which broke things
> so they have a better chance to spot your patch and make amends by quickly
> reviewing your fix. Anyway Reviewed-by: Daniel Vetter 

Thanks for catching this. Ville also noticed it and has sent already a
fix for it:
http://lists.freedesktop.org/archives/intel-gfx/2015-August/074676.html

--Imre

> 
> Jani, this all seems to be for you.
> -Daniel
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c 
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index 8485bea..ad99dae 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1559,7 +1559,7 @@ static void i9xx_hpd_irq_handler(struct drm_device 
> > *dev)
> > u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915;
> >  
> > intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> > -  hotplug_trigger, hpd_status_g4x,
> > +  hotplug_trigger, hpd_status_i915,
> >i9xx_port_hotplug_long_detect);
> > intel_hpd_irq_handler(dev, pin_mask, long_mask);
> > }
> > -- 
> > 1.8.4.5
> > 
> 


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

Re: [Intel-gfx] [PATCH] uapi/drm/i915_drm.h: fix userspace compilation.

[Daniel Vetter]

On Wed, Sep 02, 2015 at 02:52:19PM +0300, Ville Syrjälä wrote:
> On Wed, Sep 02, 2015 at 01:41:18PM +0200, Artem Savkov wrote:
> > Patch "drm/i915: Use expcitly fixed type in compat32 structs" changed the 
> > type
> > of param field in drm_i915_getparam from int to s32. This header is 
> > exported to
> > userspace and needs to use userspace type __s32 instead.
> > 
> > This fixes userspace compilation errors like the following:
> > include/drm/i915_drm.h:361:2: error: unknown type name 's32'
> >   s32 param;
> > 
> > Signed-off-by: Artem Savkov 
> > ---
> >  include/uapi/drm/i915_drm.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> > index dbd16a2..fd5aa47 100644
> > --- a/include/uapi/drm/i915_drm.h
> > +++ b/include/uapi/drm/i915_drm.h
> > @@ -358,7 +358,7 @@ typedef struct drm_i915_irq_wait {
> >  #define I915_PARAM_HAS_RESOURCE_STREAMER 36
> >  
> >  typedef struct drm_i915_getparam {
> > -   s32 param;
> > +   __s32 param;
> 
> Hmm. I don't understand why this one in particular got changed to s32
> when there are other ioctl structs still using int.

Mostly me being incompetent.

Reviewed-by: Daniel Vetter  on this one, not that
it seems to be worth much ...
-Daniel

> 
> > /*
> >  * WARNING: Using pointers instead of fixed-size u64 means we need to 
> > write
> >  * compat32 code. Don't repeat this mistake.
> > -- 
> > 2.1.0
> > 
> > ___
> > 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

-- 
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] [PATCH] drm/i915: add kerneldoc for i915_audio_component

[Daniel Vetter]

On Wed, Sep 02, 2015 at 02:12:24PM +0800, libin.y...@intel.com wrote:
> From: Libin Yang 
> 
> Add the kerneldoc for i915_audio_component in i915_component.h
> 
> Signed-off-by: Libin Yang 
> ---
>  include/drm/i915_component.h | 39 ---
>  1 file changed, 24 insertions(+), 15 deletions(-)
> 
> diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
> index 8ad6f1b..187acc8 100644
> --- a/include/drm/i915_component.h
> +++ b/include/drm/i915_component.h
> @@ -24,23 +24,32 @@
>  #ifndef _I915_COMPONENT_H_
>  #define _I915_COMPONENT_H_
>  
> +/**
> + * struct i915_audio_component_ops - callbacks defined in gfx driver
> + * @owner: the module owner
> + * @get_power: get the POWER_DOMAIN_AUDIO power well
> + * @put_power: put the POWER_DOMAIN_AUDIO power well
> + * @codec_wake_override: Enable/Disable generating the codec wake signal
> + * @get_cdclk_freq: get the Core Display Clock in KHz
> + * @sync_audio_rate: set n/cts based on the sample rate
> + */
> +struct i915_audio_component_ops {
> + struct module *owner;

New kerneldoc in 4.3 allows you to split structure documentation up into
per-member comments. Especially with vtables I think this makes a lot of
sense, since then you have enough space to document where and how exactly
a given hook is called (looks, place in the overall sequence).

Also please include your stancas in the drm.tmpl docbook template,
otherwise it won't be included in the html docs. And finally please add a
DOC: overview section which explains at a high level how i915 and
hda-intel corporate for hdmi/dp audio.

Thanks, Daniel

> + void (*get_power)(struct device *);
> + void (*put_power)(struct device *);
> + void (*codec_wake_override)(struct device *, bool enable);
> + int (*get_cdclk_freq)(struct device *);
> + int (*sync_audio_rate)(struct device *, int port, int rate);
> +};
> +
> +/**
> + * struct i915_audio_component - used for audio video interaction
> + * @dev: the device from gfx driver
> + * @ops: callback for audio driver calling
> + */
>  struct i915_audio_component {
>   struct device *dev;
> -
> - 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_ops *ops;
>  };
>  
>  #endif /* _I915_COMPONENT_H_ */
> -- 
> 1.9.1
> 

-- 
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] [PATCH 0/4 v5] i915 to call hda driver on HDMI plug/unplug

[Daniel Vetter]

On Wed, Sep 02, 2015 at 01:59:30PM +0200, Takashi Iwai wrote:
> On Wed, 02 Sep 2015 13:45:03 +0200,
> Daniel Vetter wrote:
> > 
> > On Fri, Aug 28, 2015 at 08:14:48PM +0300, Jani Nikula wrote:
> > > On Fri, 28 Aug 2015, David Henningsson  
> > > wrote:
> > > > Hopefully last version? :-)
> > > >
> > > >  * Added commit text about duplicate events (patch 4/4)
> > > >  * Added locks in bind/unbind on i915 side (patch 2/4)
> > > >  * Fixed docbook comments in i915 struct (patch 1/4)
> > > >  * Removed port_mst_streaming parameter (patch 1/4)
> > > >  * Added Reviewed-by - 1 & 2 by Jani, 3 & 4 by Takashi.
> > > >Hopefully this was correct, otherwise feel free to adjust
> > > >yourself before committing.
> > > >
> > > > Both Jani and Takashi seem okay with this going into 4.3. 
> > > > Nobody has said which tree you prefer to take it through, so
> > > > how about Takashi merging it?
> > > 
> > > I think there's a slight conflict with Libin's series [1], so both
> > > should probably go through the same tree. I'm fine with either.
> > 
> > Yeah makes sense. Takashi can you please pull in this entire series into
> > your tree too?
> 
> Do you mean Libin's series or David's?  I already applied David's
> patches (including drm/i915 changes) to for-next of sound git tree
> now.  Shall I merge Libin's patches to my branch, too?

I was a few days behind on mails and made a bit a mess between these two
patch series. I thought both David's and Libin's series is ready, but
let's ask Jani to confirm ;-)

> The branch is supposed to be stable, so feel free to pull into your
> testing branch.

Ok. I'll ping you if I do pull something which isn't in upstream just in.
-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

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt

[Daniel Vetter]

On Tue, Sep 01, 2015 at 10:21:35PM +0200, Egbert Eich wrote:
> A HPD interrupt may fire during intel_crt_detect_hotplug() - especially
> when HPD interrupt storms occur.
> Since the interrupt handler changes the enabled interrupt lines when it
> detects a storm this races with intel_crt_detect_hotplug().
> To avoid this, shiled the rmw cycles with IRQ save spinlocks.
> 
> Signed-off-by: Egbert Eich 

I think this only reduces one source of such races, but fundamentally we
can't avoid them. E.g. if you're _very_ unlucky you might cause a real hpd
right when the strom code frobs around. Plugging the race with this known
interrupt source here doesn't fix the fundamental issue.

Also I think the actual interrupt deliver is fairly asynchronous, both in
the hardware and in the sw handling. E.g. spin_lock_irq does not
synchronize with the interrupt handler on SMP systems, it only guarantees
that it's not running concurrently on the same cpu (which would deadlock).

I think fundamentally this race is unfixable.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_crt.c | 17 -
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_crt.c 
> b/drivers/gpu/drm/i915/intel_crt.c
> index af5e43b..685f3de 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -376,9 +376,10 @@ static bool intel_crt_detect_hotplug(struct 
> drm_connector *connector)
>  {
>   struct drm_device *dev = connector->dev;
>   struct drm_i915_private *dev_priv = dev->dev_private;
> - u32 hotplug_en, orig, stat;
> + u32 hotplug_en, stat;
>   bool ret = false;
>   int i, tries = 0;
> + unsigned long irqflags;
>  
>   if (HAS_PCH_SPLIT(dev))
>   return intel_ironlake_crt_detect_hotplug(connector);
> @@ -395,12 +396,14 @@ static bool intel_crt_detect_hotplug(struct 
> drm_connector *connector)
>   tries = 2;
>   else
>   tries = 1;
> - hotplug_en = orig = I915_READ(PORT_HOTPLUG_EN);
> - hotplug_en |= CRT_HOTPLUG_FORCE_DETECT;
>  
>   for (i = 0; i < tries ; i++) {
>   /* turn on the FORCE_DETECT */
> - I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);
> + spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> + hotplug_en = I915_READ(PORT_HOTPLUG_EN);
> + I915_WRITE(PORT_HOTPLUG_EN, hotplug_en |
> +CRT_HOTPLUG_FORCE_DETECT);
> + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>   /* wait for FORCE_DETECT to go off */
>   if (wait_for((I915_READ(PORT_HOTPLUG_EN) &
> CRT_HOTPLUG_FORCE_DETECT) == 0,
> @@ -416,7 +419,11 @@ static bool intel_crt_detect_hotplug(struct 
> drm_connector *connector)
>   I915_WRITE(PORT_HOTPLUG_STAT, CRT_HOTPLUG_INT_STATUS);
>  
>   /* and put the bits back */
> - I915_WRITE(PORT_HOTPLUG_EN, orig);
> + spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> + hotplug_en = I915_READ(PORT_HOTPLUG_EN);
> + hotplug_en &= ~CRT_HOTPLUG_FORCE_DETECT;
> + I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);
> + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>  
>   return ret;
>  }
> -- 
> 1.8.4.5
> 

-- 
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] [PATCH 3/4] drm/i915: Use the correct hpd_status list for non-G4xx/VLV

[Daniel Vetter]

On Tue, Sep 01, 2015 at 10:21:34PM +0200, Egbert Eich wrote:
> This copy-and-past error was introduced in:
> 
> commit fd63e2a972c670887e5e8a08440111d3812c0996
> Author: Imre Deak 
> Date:   Tue Jul 21 15:32:44 2015 -0700
> 
> drm/i915: combine i9xx_get_hpd_pins and pch_get_hpd_pins
> 
> Signed-off-by: Egbert Eich 

I recommend to cc: the authors/reviewers of the patch which broke things
so they have a better chance to spot your patch and make amends by quickly
reviewing your fix. Anyway Reviewed-by: Daniel Vetter 

Jani, this all seems to be for you.
-Daniel
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 8485bea..ad99dae 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1559,7 +1559,7 @@ static void i9xx_hpd_irq_handler(struct drm_device *dev)
>   u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915;
>  
>   intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> -hotplug_trigger, hpd_status_g4x,
> +hotplug_trigger, hpd_status_i915,
>  i9xx_port_hotplug_long_detect);
>   intel_hpd_irq_handler(dev, pin_mask, long_mask);
>   }
> -- 
> 1.8.4.5
> 

-- 
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] [PATCH] drm/i915/bxt: Fix irq_port for eDP

[Jindal, Sonika]

When hpd occurs on port_b, due to the below assignment, it calls 
digital_port_wok_func first.
There it tries to check the connector status for port B (actually checking port 
A's HPD pin due to the BXT WA).
It finds it connected and continues to read the dpcd. Since this port was only 
initialized as HDMI and not initialized as DP, we haven't initialized the aux 
and causes crash in the case when the port is only enumerated as HDMI alone.
So, this part needs to be moved under the init_dp check itself.
I will be posting Durga's original patch along with hdmi patches because this 
is required for hdmi to work on bxt.

Regards,
Sonika

-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Wednesday, September 2, 2015 5:21 PM
To: Jindal, Sonika
Cc: Daniel Vetter; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915/bxt: Fix irq_port for eDP

On Wed, Sep 02, 2015 at 11:48:36AM +, Jindal, Sonika wrote:
> :( This had a hole..
> Please drop this patch..

What kind of hole? It sounds like we need this to avoid blowing up when we 
handle the edp hpd interrupt everywhere? Please explain so I can understand 
what I've missed here ...

Thanks, Daniel

> 
> I am going to send another patch tested with hdmi optimization series for bxt.
> 
> Regards,
> Sonika
> 
> -Original Message-
> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of 
> Daniel Vetter
> Sent: Wednesday, September 2, 2015 5:17 PM
> To: Jindal, Sonika
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/bxt: Fix irq_port for eDP
> 
> On Mon, Aug 31, 2015 at 02:35:32PM +0530, Sonika Jindal wrote:
> > From: Durgadoss R 
> > 
> > Currently, HDMI hotplug with eDP as local panel is failing because 
> > the HDMI hpd is detected as a long hpd for eDP; and is thus 
> > rightfully ignored. But, it should really be handled as an interrupt 
> > on port B for HDMI (due to BXT A1 platform having HPD pins A and B swapped).
> > This patch sets the irq_port[PORT_A] to NULL in case eDP is on port 
> > A so that irq handler does not treat it as a 'dig_port' interrupt.
> > 
> > v2 (Sonika): Moving the setting of irq_port for BXT WA outside so 
> > that this can be set for both hdmi or dp ports. For HDMI this is 
> > required because we get interrupts for portB on the hpd line of 
> > portA for BXT A0/A1.
> > This issue occurred because hpd on edp was not disabled which was 
> > done as part of "drm/i915: Dont enable hpd for eDP" from the series:
> > http://lists.freedesktop.org/archives/intel-gfx/2015-August/073266.h
> > tm
> > l
> > 
> > This patch can be squashed to :
> > commit cf1d58833f07afbb4534b15caa3fd48baa313b2c
> > Author: Sonika Jindal 
> > Date:   Mon Aug 10 10:35:36 2015 +0530
> > 
> > drm/i915/bxt: WA for swapped HPD pins in A stepping
> > 
> > Signed-off-by: Durgadoss R 
> > Signed-off-by: Sonika Jindal 
> 
> Queued for -next, thanks for the patch.
> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c |   21 -
> >  1 file changed, 12 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index 56d778f..bba0cb6 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -3242,15 +3242,7 @@ void intel_ddi_init(struct drm_device *dev, enum 
> > port port)
> > goto err;
> >  
> > intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> > -   /*
> > -* On BXT A0/A1, sw needs to activate DDIA HPD logic and
> > -* interrupts to check the external panel connection.
> > -*/
> > -   if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)
> > -&& port == PORT_B)
> > -   dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> > -   else
> > -   dev_priv->hotplug.irq_port[port] = intel_dig_port;
> > +   dev_priv->hotplug.irq_port[port] = intel_dig_port;
> > }
> >  
> > /* In theory we don't need the encoder->type check, but leave it 
> > just in @@ -3259,6 +3251,17 @@ void intel_ddi_init(struct drm_device *dev, 
> > enum port port)
> > if (!intel_ddi_init_hdmi_connector(intel_dig_port))
> > goto err;
> > }
> > +   /*
> > +* On BXT A0/A1, sw needs to activate DDIA HPD logic and
> > +* interrupts to check the external panel connection.
> > +*/
> > +   if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)) {
> > +   if (port == PORT_B) {
> > +   dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> > +   intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> > +   } else if (intel_encoder->type == INTEL_OUTPUT_EDP)
> > +   dev_priv->hotplug.irq_port[port] = NULL;
> > +   }
> >  
> > return;
> >  
> > --
> > 1.7.10.4

Re: [Intel-gfx] [PATCH 0/4 v5] i915 to call hda driver on HDMI plug/unplug

[Takashi Iwai]

On Wed, 02 Sep 2015 13:45:03 +0200,
Daniel Vetter wrote:
> 
> On Fri, Aug 28, 2015 at 08:14:48PM +0300, Jani Nikula wrote:
> > On Fri, 28 Aug 2015, David Henningsson  
> > wrote:
> > > Hopefully last version? :-)
> > >
> > >  * Added commit text about duplicate events (patch 4/4)
> > >  * Added locks in bind/unbind on i915 side (patch 2/4)
> > >  * Fixed docbook comments in i915 struct (patch 1/4)
> > >  * Removed port_mst_streaming parameter (patch 1/4)
> > >  * Added Reviewed-by - 1 & 2 by Jani, 3 & 4 by Takashi.
> > >Hopefully this was correct, otherwise feel free to adjust
> > >yourself before committing.
> > >
> > > Both Jani and Takashi seem okay with this going into 4.3. 
> > > Nobody has said which tree you prefer to take it through, so
> > > how about Takashi merging it?
> > 
> > I think there's a slight conflict with Libin's series [1], so both
> > should probably go through the same tree. I'm fine with either.
> 
> Yeah makes sense. Takashi can you please pull in this entire series into
> your tree too?

Do you mean Libin's series or David's?  I already applied David's
patches (including drm/i915 changes) to for-next of sound git tree
now.  Shall I merge Libin's patches to my branch, too?

The branch is supposed to be stable, so feel free to pull into your
testing branch.


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

Re: [Intel-gfx] [PATCH 2/4] drm/i915: Call non-locking version of drm_kms_helper_poll_enable()

[Daniel Vetter]

On Tue, Sep 01, 2015 at 10:21:33PM +0200, Egbert Eich wrote:
> drm_kms_helper_poll_enable() is called from a context in
> intel_hpd_irq_storm_disable() where the the mode_config mutex is
> already locked.
> When this function was converted to lock this mutex in:
> 
> commit 8c4ccc4ab6f64e859d4ff8d7c02c2ed2e956e07f
> Author: Daniel Vetter 
> Date:   Thu Jul 9 23:44:26 2015 +0200
> 
> drm/probe-helper: Grab mode_config.mutex in poll_init/enable
> 
> a deadlock occurred.
> Call the newly implemented non-locking version of this function.
> 
> Signed-off-by: Egbert Eich 

Oops. Reviewed-by: Daniel Vetter  if you do the
s/_no_lock/_locked/ for consistency on both patch 1&2.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_hotplug.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c 
> b/drivers/gpu/drm/i915/intel_hotplug.c
> index 53c0173..77dd5b6 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -180,7 +180,7 @@ static void intel_hpd_irq_storm_disable(struct 
> drm_i915_private *dev_priv)
>  
>   /* Enable polling and queue hotplug re-enabling. */
>   if (hpd_disabled) {
> - drm_kms_helper_poll_enable(dev);
> + drm_kms_helper_poll_enable_no_lock(dev);
>   mod_delayed_work(system_wq, &dev_priv->hotplug.reenable_work,
>msecs_to_jiffies(HPD_STORM_REENABLE_DELAY));
>   }
> -- 
> 1.8.4.5
> 

-- 
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] [PATCH 1/4] drm: Add a non-locking version of drm_kms_helper_poll_enable().

[Daniel Vetter]

On Tue, Sep 01, 2015 at 10:21:32PM +0200, Egbert Eich wrote:
> drm_kms_helper_poll_enable() was converted to lock the mode_config
> mutex in commit 8c4ccc4ab6f64e859d4ff8d7c02c2ed2e956e07f
> ("drm/probe-helper: Grab mode_config.mutex in poll_init/enable").
> 
> This disregarded the cases where this function is called from a context
> where this mutex is already locked.
> 
> Add a non-locking version as well.
> 
> Signed-off-by: Egbert Eich 
> ---
>  drivers/gpu/drm/drm_probe_helper.c | 19 ---
>  include/drm/drm_crtc_helper.h  |  1 +
>  2 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c 
> b/drivers/gpu/drm/drm_probe_helper.c
> index d734780..a93ad1b 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -93,8 +93,19 @@ static int drm_helper_probe_add_cmdline_mode(struct 
> drm_connector *connector)
>   return 1;
>  }
>  
> +/**
> + * drm_kms_helper_poll_enable_no_lock - re-enable output polling.
> + * @dev: drm_device
> + *
> + * This function re-enables the output polling work without
> + * locking the mode_config mutex.
> + *
> + * This is like drm_kms_helper_poll_enable() however it is to be
> + * called from a context where the mode_config mutex is locked
> + * already.
> + */
>  #define DRM_OUTPUT_POLL_PERIOD (10*HZ)
> -static void __drm_kms_helper_poll_enable(struct drm_device *dev)
> +void drm_kms_helper_poll_enable_no_lock(struct drm_device *dev)

Please use _locked to stay consistent with other such functions. With that
address this lgtm.
-Daniel

>  {
>   bool poll = false;
>   struct drm_connector *connector;
> @@ -113,6 +124,8 @@ static void __drm_kms_helper_poll_enable(struct 
> drm_device *dev)
>   if (poll)
>   schedule_delayed_work(&dev->mode_config.output_poll_work, 
> DRM_OUTPUT_POLL_PERIOD);
>  }
> +EXPORT_SYMBOL(drm_kms_helper_poll_enable_no_lock);
> +
>  
>  static int drm_helper_probe_single_connector_modes_merge_bits(struct 
> drm_connector *connector,
> uint32_t maxX, 
> uint32_t maxY, bool merge_type_bits)
> @@ -174,7 +187,7 @@ static int 
> drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect
>  
>   /* Re-enable polling in case the global poll config changed. */
>   if (drm_kms_helper_poll != dev->mode_config.poll_running)
> - __drm_kms_helper_poll_enable(dev);
> + drm_kms_helper_poll_enable_no_lock(dev);
>  
>   dev->mode_config.poll_running = drm_kms_helper_poll;
>  
> @@ -428,7 +441,7 @@ EXPORT_SYMBOL(drm_kms_helper_poll_disable);
>  void drm_kms_helper_poll_enable(struct drm_device *dev)
>  {
>   mutex_lock(&dev->mode_config.mutex);
> - __drm_kms_helper_poll_enable(dev);
> + drm_kms_helper_poll_enable_no_lock(dev);
>   mutex_unlock(&dev->mode_config.mutex);
>  }
>  EXPORT_SYMBOL(drm_kms_helper_poll_enable);
> diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
> index 2a747a9..f96703d 100644
> --- a/include/drm/drm_crtc_helper.h
> +++ b/include/drm/drm_crtc_helper.h
> @@ -240,5 +240,6 @@ extern void drm_kms_helper_hotplug_event(struct 
> drm_device *dev);
>  
>  extern void drm_kms_helper_poll_disable(struct drm_device *dev);
>  extern void drm_kms_helper_poll_enable(struct drm_device *dev);
> +extern void drm_kms_helper_poll_enable_no_lock(struct drm_device *dev);
>  
>  #endif
> -- 
> 1.8.4.5
> 

-- 
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] [PATCH] uapi/drm/i915_drm.h: fix userspace compilation.

[Ville Syrjälä]

On Wed, Sep 02, 2015 at 01:41:18PM +0200, Artem Savkov wrote:
> Patch "drm/i915: Use expcitly fixed type in compat32 structs" changed the type
> of param field in drm_i915_getparam from int to s32. This header is exported 
> to
> userspace and needs to use userspace type __s32 instead.
> 
> This fixes userspace compilation errors like the following:
> include/drm/i915_drm.h:361:2: error: unknown type name 's32'
>   s32 param;
> 
> Signed-off-by: Artem Savkov 
> ---
>  include/uapi/drm/i915_drm.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index dbd16a2..fd5aa47 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -358,7 +358,7 @@ typedef struct drm_i915_irq_wait {
>  #define I915_PARAM_HAS_RESOURCE_STREAMER 36
>  
>  typedef struct drm_i915_getparam {
> - s32 param;
> + __s32 param;

Hmm. I don't understand why this one in particular got changed to s32
when there are other ioctl structs still using int.

>   /*
>* WARNING: Using pointers instead of fixed-size u64 means we need to 
> write
>* compat32 code. Don't repeat this mistake.
> -- 
> 2.1.0
> 
> ___
> 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] drm/i915: Always mark the object as dirty when used by the GPU

[Daniel Vetter]

On Mon, Aug 31, 2015 at 03:10:39PM +0100, Chris Wilson wrote:
> There have been many hard to track down bugs whereby userspace forgot to
> flag a write buffer and then cause graphics corruption or a hung GPU
> when that buffer was later purged under memory pressure (as the buffer
> appeared clean, its pages would have been evicted rather than preserved
> and any changes more recent than in the backing storage would be lost).
> In retrospect this is a rare optimisation against memory pressure,
> already the slow path. If we always mark the buffer as dirty when
> accessed by the GPU, anything not used can still be evicted cheaply
> (ideal behaviour for mark-and-sweep eviction) but we do not run the risk
> of corruption. For correct read serialisation, userspace still has to
> notify when the GPU writes to an object. However, there are certain
> situations under which userspace may wish to tell white lies to the
> kernel...
> 
> Signed-off-by: Chris Wilson 
> Cc: Daniel Vetter 
> Cc: Kristian Høgsberg 
> Cc: Jesse Barnes 
> Cc: "Goel, Akash" 
> Cc: Michał Winiarski 
> Cc: sta...@vger.kernel.org

Reviewed-by: Daniel Vetter 

> ---
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 923a3c4bf0b7..a953d4975b8c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1032,6 +1032,7 @@ i915_gem_execbuffer_move_to_active(struct list_head 
> *vmas,
>   u32 old_read = obj->base.read_domains;
>   u32 old_write = obj->base.write_domain;
>  
> + obj->dirty = 1; /* be paranoid  */
>   obj->base.write_domain = obj->base.pending_write_domain;
>   if (obj->base.write_domain == 0)
>   obj->base.pending_read_domains |= 
> obj->base.read_domains;
> @@ -1039,7 +1040,6 @@ i915_gem_execbuffer_move_to_active(struct list_head 
> *vmas,
>  
>   i915_vma_move_to_active(vma, req);
>   if (obj->base.write_domain) {
> - obj->dirty = 1;
>   i915_gem_request_assign(&obj->last_write_req, req);
>  
>   intel_fb_obj_invalidate(obj, ORIGIN_CS);
> -- 
> 2.5.1
> 

-- 
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] [PATCH v2 2/2] drm/i915: Also record time difference if vblank evasion fails, v2.

[Daniel Vetter]

On Tue, Sep 01, 2015 at 12:15:33PM +0200, Maarten Lankhorst wrote:
> This makes the error message slightly more useful.
> 
> Changes since v1:
> - Use ktime_get() while irqs are still disabled. (vsyrjala)
> 
> Signed-off-by: Maarten Lankhorst 
> Reviewed-by: Ville Syrjälä 

Both applied to dinq, thanks.
-Daniel

> ---
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index 63f0d5850cdf..b0c3a3af7ed2 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -563,6 +563,8 @@ struct intel_crtc {
>   int scanline_offset;
>  
>   unsigned start_vbl_count;
> + ktime_t start_vbl_time;
> +
>   struct intel_crtc_atomic_commit atomic;
>  
>   /* scalers available on this crtc */
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 63fba42be2cf..9387af5ef1b8 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -134,6 +134,7 @@ void intel_pipe_update_start(struct intel_crtc *crtc)
>  
>   drm_crtc_vblank_put(&crtc->base);
>  
> + crtc->start_vbl_time = ktime_get();
>   crtc->start_vbl_count = dev->driver->get_vblank_counter(dev, pipe);
>  
>   trace_i915_pipe_update_vblank_evaded(crtc, min, max,
> @@ -154,14 +155,16 @@ void intel_pipe_update_end(struct intel_crtc *crtc)
>   struct drm_device *dev = crtc->base.dev;
>   enum pipe pipe = crtc->pipe;
>   u32 end_vbl_count = dev->driver->get_vblank_counter(dev, pipe);
> + ktime_t end_vbl_time = ktime_get();
>  
>   trace_i915_pipe_update_end(crtc, end_vbl_count);
>  
>   local_irq_enable();
>  
>   if (crtc->start_vbl_count && crtc->start_vbl_count != end_vbl_count)
> - DRM_ERROR("Atomic update failure on pipe %c (start=%u 
> end=%u)\n",
> -   pipe_name(pipe), crtc->start_vbl_count, 
> end_vbl_count);
> + DRM_ERROR("Atomic update failure on pipe %c (start=%u end=%u) 
> time %lld us\n",
> +   pipe_name(pipe), crtc->start_vbl_count, end_vbl_count,
> +   ktime_us_delta(end_vbl_time, crtc->start_vbl_time));
>  }
>  
>  static void
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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] [PATCH] drm/i915/bxt: Fix irq_port for eDP

[Daniel Vetter]

On Wed, Sep 02, 2015 at 11:48:36AM +, Jindal, Sonika wrote:
> :( This had a hole..
> Please drop this patch..

What kind of hole? It sounds like we need this to avoid blowing up when we
handle the edp hpd interrupt everywhere? Please explain so I can
understand what I've missed here ...

Thanks, Daniel

> 
> I am going to send another patch tested with hdmi optimization series for bxt.
> 
> Regards,
> Sonika
> 
> -Original Message-
> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Wednesday, September 2, 2015 5:17 PM
> To: Jindal, Sonika
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/bxt: Fix irq_port for eDP
> 
> On Mon, Aug 31, 2015 at 02:35:32PM +0530, Sonika Jindal wrote:
> > From: Durgadoss R 
> > 
> > Currently, HDMI hotplug with eDP as local panel is failing because the 
> > HDMI hpd is detected as a long hpd for eDP; and is thus rightfully 
> > ignored. But, it should really be handled as an interrupt on port B 
> > for HDMI (due to BXT A1 platform having HPD pins A and B swapped). 
> > This patch sets the irq_port[PORT_A] to NULL in case eDP is on port A 
> > so that irq handler does not treat it as a 'dig_port' interrupt.
> > 
> > v2 (Sonika): Moving the setting of irq_port for BXT WA outside so that 
> > this can be set for both hdmi or dp ports. For HDMI this is required 
> > because we get interrupts for portB on the hpd line of portA for BXT 
> > A0/A1.
> > This issue occurred because hpd on edp was not disabled which was done 
> > as part of "drm/i915: Dont enable hpd for eDP" from the series:
> > http://lists.freedesktop.org/archives/intel-gfx/2015-August/073266.htm
> > l
> > 
> > This patch can be squashed to :
> > commit cf1d58833f07afbb4534b15caa3fd48baa313b2c
> > Author: Sonika Jindal 
> > Date:   Mon Aug 10 10:35:36 2015 +0530
> > 
> > drm/i915/bxt: WA for swapped HPD pins in A stepping
> > 
> > Signed-off-by: Durgadoss R 
> > Signed-off-by: Sonika Jindal 
> 
> Queued for -next, thanks for the patch.
> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c |   21 -
> >  1 file changed, 12 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index 56d778f..bba0cb6 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -3242,15 +3242,7 @@ void intel_ddi_init(struct drm_device *dev, enum 
> > port port)
> > goto err;
> >  
> > intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> > -   /*
> > -* On BXT A0/A1, sw needs to activate DDIA HPD logic and
> > -* interrupts to check the external panel connection.
> > -*/
> > -   if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)
> > -&& port == PORT_B)
> > -   dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> > -   else
> > -   dev_priv->hotplug.irq_port[port] = intel_dig_port;
> > +   dev_priv->hotplug.irq_port[port] = intel_dig_port;
> > }
> >  
> > /* In theory we don't need the encoder->type check, but leave it 
> > just in @@ -3259,6 +3251,17 @@ void intel_ddi_init(struct drm_device *dev, 
> > enum port port)
> > if (!intel_ddi_init_hdmi_connector(intel_dig_port))
> > goto err;
> > }
> > +   /*
> > +* On BXT A0/A1, sw needs to activate DDIA HPD logic and
> > +* interrupts to check the external panel connection.
> > +*/
> > +   if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)) {
> > +   if (port == PORT_B) {
> > +   dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> > +   intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> > +   } else if (intel_encoder->type == INTEL_OUTPUT_EDP)
> > +   dev_priv->hotplug.irq_port[port] = NULL;
> > +   }
> >  
> > return;
> >  
> > --
> > 1.7.10.4
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
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] [PATCH] drm/i915/bxt: Fix irq_port for eDP

[Jindal, Sonika]

:( This had a hole..
Please drop this patch..

I am going to send another patch tested with hdmi optimization series for bxt.

Regards,
Sonika

-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Wednesday, September 2, 2015 5:17 PM
To: Jindal, Sonika
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915/bxt: Fix irq_port for eDP

On Mon, Aug 31, 2015 at 02:35:32PM +0530, Sonika Jindal wrote:
> From: Durgadoss R 
> 
> Currently, HDMI hotplug with eDP as local panel is failing because the 
> HDMI hpd is detected as a long hpd for eDP; and is thus rightfully 
> ignored. But, it should really be handled as an interrupt on port B 
> for HDMI (due to BXT A1 platform having HPD pins A and B swapped). 
> This patch sets the irq_port[PORT_A] to NULL in case eDP is on port A 
> so that irq handler does not treat it as a 'dig_port' interrupt.
> 
> v2 (Sonika): Moving the setting of irq_port for BXT WA outside so that 
> this can be set for both hdmi or dp ports. For HDMI this is required 
> because we get interrupts for portB on the hpd line of portA for BXT 
> A0/A1.
> This issue occurred because hpd on edp was not disabled which was done 
> as part of "drm/i915: Dont enable hpd for eDP" from the series:
> http://lists.freedesktop.org/archives/intel-gfx/2015-August/073266.htm
> l
> 
> This patch can be squashed to :
> commit cf1d58833f07afbb4534b15caa3fd48baa313b2c
> Author: Sonika Jindal 
> Date:   Mon Aug 10 10:35:36 2015 +0530
> 
> drm/i915/bxt: WA for swapped HPD pins in A stepping
> 
> Signed-off-by: Durgadoss R 
> Signed-off-by: Sonika Jindal 

Queued for -next, thanks for the patch.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_ddi.c |   21 -
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 56d778f..bba0cb6 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3242,15 +3242,7 @@ void intel_ddi_init(struct drm_device *dev, enum port 
> port)
>   goto err;
>  
>   intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> - /*
> -  * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> -  * interrupts to check the external panel connection.
> -  */
> - if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)
> -  && port == PORT_B)
> - dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> - else
> - dev_priv->hotplug.irq_port[port] = intel_dig_port;
> + dev_priv->hotplug.irq_port[port] = intel_dig_port;
>   }
>  
>   /* In theory we don't need the encoder->type check, but leave it 
> just in @@ -3259,6 +3251,17 @@ void intel_ddi_init(struct drm_device *dev, 
> enum port port)
>   if (!intel_ddi_init_hdmi_connector(intel_dig_port))
>   goto err;
>   }
> + /*
> +  * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> +  * interrupts to check the external panel connection.
> +  */
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)) {
> + if (port == PORT_B) {
> + dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> + intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> + } else if (intel_encoder->type == INTEL_OUTPUT_EDP)
> + dev_priv->hotplug.irq_port[port] = NULL;
> + }
>  
>   return;
>  
> --
> 1.7.10.4
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
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] [PATCH] drm/i915/bxt: Fix irq_port for eDP

[Daniel Vetter]

On Mon, Aug 31, 2015 at 02:35:32PM +0530, Sonika Jindal wrote:
> From: Durgadoss R 
> 
> Currently, HDMI hotplug with eDP as local panel is failing
> because the HDMI hpd is detected as a long hpd for eDP; and is
> thus rightfully ignored. But, it should really be handled as
> an interrupt on port B for HDMI (due to BXT A1 platform having
> HPD pins A and B swapped). This patch sets the irq_port[PORT_A]
> to NULL in case eDP is on port A so that irq handler does not
> treat it as a 'dig_port' interrupt.
> 
> v2 (Sonika): Moving the setting of irq_port for BXT WA outside so that this
> can be set for both hdmi or dp ports. For HDMI this is required
> because we get interrupts for portB on the hpd line of portA for
> BXT A0/A1.
> This issue occurred because hpd on edp was not disabled
> which was done as part of "drm/i915: Dont enable hpd for eDP" from
> the series:
> http://lists.freedesktop.org/archives/intel-gfx/2015-August/073266.html
> 
> This patch can be squashed to :
> commit cf1d58833f07afbb4534b15caa3fd48baa313b2c
> Author: Sonika Jindal 
> Date:   Mon Aug 10 10:35:36 2015 +0530
> 
> drm/i915/bxt: WA for swapped HPD pins in A stepping
> 
> Signed-off-by: Durgadoss R 
> Signed-off-by: Sonika Jindal 

Queued for -next, thanks for the patch.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_ddi.c |   21 -
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 56d778f..bba0cb6 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3242,15 +3242,7 @@ void intel_ddi_init(struct drm_device *dev, enum port 
> port)
>   goto err;
>  
>   intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> - /*
> -  * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> -  * interrupts to check the external panel connection.
> -  */
> - if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)
> -  && port == PORT_B)
> - dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> - else
> - dev_priv->hotplug.irq_port[port] = intel_dig_port;
> + dev_priv->hotplug.irq_port[port] = intel_dig_port;
>   }
>  
>   /* In theory we don't need the encoder->type check, but leave it just in
> @@ -3259,6 +3251,17 @@ void intel_ddi_init(struct drm_device *dev, enum port 
> port)
>   if (!intel_ddi_init_hdmi_connector(intel_dig_port))
>   goto err;
>   }
> + /*
> +  * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> +  * interrupts to check the external panel connection.
> +  */
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)) {
> + if (port == PORT_B) {
> + dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> + intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> + } else if (intel_encoder->type == INTEL_OUTPUT_EDP)
> + dev_priv->hotplug.irq_port[port] = NULL;
> + }
>  
>   return;
>  
> -- 
> 1.7.10.4
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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

[Intel-gfx] [PULL] drm-intel-next-fixes

[Jani Nikula]

Hi Dave -

i915 display fixes headed for v4.3. Mostly SKL, but some regression
fixes too.

BR,
Jani.

The following changes since commit 26951caf55d73ceb1967b0bf12f6d0b96853508e:

  drm/i915/skl: enable DDI-E hotplug (2015-08-26 10:24:25 +0300)

are available in the git repository at:

  git://anongit.freedesktop.org/drm-intel tags/drm-intel-next-fixes-2015-09-02

for you to fetch changes up to 6fa2d197936ba0b8936e813d0adecefac160062b:

  i915: Set ddi_pll_sel in DP MST path (2015-09-01 12:42:27 +0300)


Ander Conselvan de Oliveira (1):
  i915: Set ddi_pll_sel in DP MST path

Gary Wang (1):
  drm/i915: set CDCLK if DPLL0 enabled during resuming from S3

Imre Deak (1):
  drm/i915: apply the PCI_D0/D3 hibernation workaround everywhere on pre 
GEN6

Lukas Wunner (1):
  drm/i915: Preserve SSC earlier

Rodrigo Vivi (2):
  drm/i915/skl: Enable DDI-E
  drm/i915: eDP can be present on DDI-E

Ville Syrjälä (2):
  drm/i915: Check DP link status on long hpd too
  drm/i915: Don't use link_bw for PLL setup

Xiong Zhang (2):
  drm/i915: Enable HDMI on DDI-E
  drm/i915/skl: Adding DDI_E power well domain

 drivers/gpu/drm/i915/i915_debugfs.c |  2 +
 drivers/gpu/drm/i915/i915_drv.c | 15 +---
 drivers/gpu/drm/i915/i915_drv.h |  6 +++
 drivers/gpu/drm/i915/intel_bios.c   | 39 +--
 drivers/gpu/drm/i915/intel_bios.h   |  7 +---
 drivers/gpu/drm/i915/intel_ddi.c| 11 ++
 drivers/gpu/drm/i915/intel_display.c| 54 +--
 drivers/gpu/drm/i915/intel_dp.c | 66 -
 drivers/gpu/drm/i915/intel_dp_mst.c |  5 +++
 drivers/gpu/drm/i915/intel_drv.h|  1 +
 drivers/gpu/drm/i915/intel_hdmi.c   | 21 +++
 drivers/gpu/drm/i915/intel_runtime_pm.c |  2 +
 12 files changed, 147 insertions(+), 82 deletions(-)

-- 
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] [PATCH 0/4 v5] i915 to call hda driver on HDMI plug/unplug

[Daniel Vetter]

On Fri, Aug 28, 2015 at 08:14:48PM +0300, Jani Nikula wrote:
> On Fri, 28 Aug 2015, David Henningsson  
> wrote:
> > Hopefully last version? :-)
> >
> >  * Added commit text about duplicate events (patch 4/4)
> >  * Added locks in bind/unbind on i915 side (patch 2/4)
> >  * Fixed docbook comments in i915 struct (patch 1/4)
> >  * Removed port_mst_streaming parameter (patch 1/4)
> >  * Added Reviewed-by - 1 & 2 by Jani, 3 & 4 by Takashi.
> >Hopefully this was correct, otherwise feel free to adjust
> >yourself before committing.
> >
> > Both Jani and Takashi seem okay with this going into 4.3. 
> > Nobody has said which tree you prefer to take it through, so
> > how about Takashi merging it?
> 
> I think there's a slight conflict with Libin's series [1], so both
> should probably go through the same tree. I'm fine with either.

Yeah makes sense. Takashi can you please pull in this entire series into
your tree too?

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

Re: [Intel-gfx] [PATCH 1/4] drm/i915: Add audio pin sense / ELD callback

[Daniel Vetter]

On Fri, Aug 28, 2015 at 07:02:27PM +0200, David Henningsson wrote:
> This callback will be called by the i915 driver to notify the hda
> driver that its HDMI information needs to be refreshed, i e,
> that audio output is now available (or unavailable) - usually as a
> result of a monitor being plugged in (or unplugged).
> 
> Reviewed-by: Jani Nikula 
> Signed-off-by: David Henningsson 
> ---
>  include/drm/i915_component.h | 16 
>  1 file changed, 16 insertions(+)
> 
> diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
> index c9a8b64..c0f4d45 100644
> --- a/include/drm/i915_component.h
> +++ b/include/drm/i915_component.h
> @@ -34,6 +34,22 @@ struct i915_audio_component {
>   void (*codec_wake_override)(struct device *, bool enable);
>   int (*get_cdclk_freq)(struct device *);
>   } *ops;
> +
> + const struct i915_audio_component_audio_ops {
> + /**
> +  * @audio_ptr:
> +  *
> +  * Pointer to pass when calling pin_eld_notify.
> +  */
> + void *audio_ptr;
> + /**
> +  * @pin_eld_notify:
> +  *
> +  * Called from i915 driver, notifying the HDA driver that
> +  * pin sense and/or ELD information has changed.
> +  */
> + void (*pin_eld_notify)(void *audio_ptr, int port);

Kerneldoc looks good now, but it won't be of much use with an include
directive into the drm.tmpl docbook. Can you please coordinate with Libin
on that?

Thanks, Daniel

> + } *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

-- 
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

[Intel-gfx] [PATCH] uapi/drm/i915_drm.h: fix userspace compilation.

[Artem Savkov]

Patch "drm/i915: Use expcitly fixed type in compat32 structs" changed the type
of param field in drm_i915_getparam from int to s32. This header is exported to
userspace and needs to use userspace type __s32 instead.

This fixes userspace compilation errors like the following:
include/drm/i915_drm.h:361:2: error: unknown type name 's32'
  s32 param;

Signed-off-by: Artem Savkov 
---
 include/uapi/drm/i915_drm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index dbd16a2..fd5aa47 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -358,7 +358,7 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_HAS_RESOURCE_STREAMER 36
 
 typedef struct drm_i915_getparam {
-   s32 param;
+   __s32 param;
/*
 * WARNING: Using pointers instead of fixed-size u64 means we need to 
write
 * compat32 code. Don't repeat this mistake.
-- 
2.1.0

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

[Intel-gfx] [PATCH] drm/i915: Fix cmdparser STORE/LOAD command descriptors

[Chris Wilson]

Fixes regression from
commit f1afe24f0e736b9d7f2275e2b1504af3fe612f2a
Author: Arun Siluvery 
Date:   Tue Aug 4 16:22:20 2015 +0100

drm/i915: Change SRM, LRM instructions to use correct length

which forgot to account for the length bias when declaring the fixed
length.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91844
Reported-by: Andreas Reis 
Signed-off-by: Chris Wilson 
Cc: Dave Gordon 
Cc: Arun Siluvery 
Cc: Mika Kuoppala 
Cc: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_cmd_parser.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c 
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index ad7d7ab76d3f..09932cab1a3f 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -124,14 +124,14 @@ static const struct drm_i915_cmd_descriptor common_cmds[] 
= {
CMD(  MI_STORE_DWORD_INDEX, SMI,   !F,  0xFF,   R  ),
CMD(  MI_LOAD_REGISTER_IMM(1),  SMI,   !F,  0xFF,   W,
  .reg = { .offset = 1, .mask = 0x007C, .step = 2 }),
-   CMD(  MI_STORE_REGISTER_MEM,SMI,F,  1, W | B,
+   CMD(  MI_STORE_REGISTER_MEM,SMI,F,  3, W | B,
  .reg = { .offset = 1, .mask = 0x007C },
  .bits = {{
.offset = 0,
.mask = MI_GLOBAL_GTT,
.expected = 0,
  }},  ),
-   CMD(  MI_LOAD_REGISTER_MEM, SMI,F,  1, W | B,
+   CMD(  MI_LOAD_REGISTER_MEM, SMI,F,  3, W | B,
  .reg = { .offset = 1, .mask = 0x007C },
  .bits = {{
.offset = 0,
-- 
2.5.1

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

Re: [Intel-gfx] [PATCH] drm/i915: Update ring space correctly on lrc context reset

[Arun Siluvery]

On 20/08/2015 16:27, Chris Wilson wrote:

On Thu, Aug 20, 2015 at 05:34:59PM +0300, Mika Kuoppala wrote:

If we leave the last_retired_head to pre-reset value, we might
end up in a situation where intel_ring_space() returns wrong
value on next hardware init.


http://patchwork.freedesktop.org/patch/46612/
and earlier
-Chris


Hi Chris,

I see the warning even with below batch,

[Intel-gfx] [PATCH 50/70] drm/i915: The argument for postfix is redundant
http://patchwork.freedesktop.org/patch/46601/

the following patch need to be updated as it uses olr,
[Intel-gfx] [PATCH 51/70] drm/i915: Record the position of the start of 
the request

http://patchwork.freedesktop.org/patch/46612/

Do we need some of the previous patches in series as well?

This patch is fixing the issue in the current code, do you think we can 
get this in its current state?


regards
Arun

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

Re: [Intel-gfx] Request Linux Graphic Driver for Intel GMA 3150

[David Ho]

Dear Rodrigo,

Is it possible to obtain the "raw" driver and to install it manually, without 
installing it through the Installer?

Regards,
David


-Original Message-
From: Vivi, Rodrigo [mailto:rodrigo.v...@intel.com] 
Sent: 01 September 2015 23:04
To: intel-gfx@lists.freedesktop.org; jani.nik...@linux.intel.com; 
hupernikao...@gmail.com
Subject: Re: [Intel-gfx] Request Linux Graphic Driver for Intel GMA 3150

On Tue, 2015-09-01 at 09:59 +0300, Jani Nikula wrote:
> On Sat, 22 Aug 2015, David Ho  wrote:
> > REQUEST
> > 
> > May I please request support for driver of Intel GMA 3150 for Ubuntu 
> > 14.04.3
> > 32 bit (Trusty Tahr)?
> > 
> > I installed "Intel Graphic Installer for Linux" from 01.org, but it 
> > stops at the very first step saying "Distribution not supported".
> 
> Rodrigo (Cc'd) knows this stuff better, but I don't think it's likely 
> old (from upstream POV) distros will be supported. In that regard, 
> you're probably better off asking help from your distro vendor.

Unfortunately Installer aims to support just latest versions of Ubuntu and 
Fedora when it is being prepared for release, regardless distros LTS.
In the past we supported LTS, but with time official updates conflicted with 
installer packages and caused more issues to users.

> > BACKGROUND
> > 
> > After fresh install of 14.04, the mouse pointer is not showing up 
> > and the display change (when scrolling, moving between windows, etc) 
> > is very slow (even only for regular application, never tried to 
> > watch video yet). I concluded that this is the driver issue.
> 
> Does it work on a newer Ubuntu install? Or can you try a new kernel? 
> If
> you can reproduce this on a new kernel, please file a bug at [1].

> BR,
> Jani.
> 
> 
> 
> [1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI&component=
> DRM/Intel
> 
> 
> > 
> > I must install it for around 20 PCs.
> > 
> > Please help me to get the correct driver.
> > 
> >  
> > 
> > Thank you.
> > 
> >  
> > 
> > Regards,
> > 
> > David Ho
> > 
> >  
> > 
> >  
> > 
> > -Original Message-
> > From: Chacn Limn, DanielX [mailto:danielx.chacn.l...@intel.com]
> > Sent: 21 Agustus 2015 22:05
> > To: hupernikao...@gmail.com
> > Cc: Becerra Ruiz, Lilia; Flores Perez, Jimena; Diaz, Victor H
> > Subject: RE: [Contact] Intel GMA 3150 for Ubuntu 14.04.3Trusty Tahr 
> > 32bit
> > 
> >  
> > 
> > Hi David,
> > 
> > Thank you for contacting us.
> > 
> >  
> > 
> > For help or more information about Linux Graphics drivers please 
> > contact the Team in charge through their mailing list:
> > 
> >  
> > intel-gfx@lists.freedesktop.org
> > 
> >  
> > 
> > Regards,
> > 
> > Daniel.
> > 
> >  
> > 
> > [.MESSAGES TRUNCATED]
> > 
> > ___
> > 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] [RFC 1/3] drm/i915: Roll intel_crtc->atomic into intel_crtc_state

[Ville Syrjälä]

On Wed, Sep 02, 2015 at 01:08:31PM +0200, Maarten Lankhorst wrote:
> Op 02-09-15 om 12:35 schreef Ville Syrjälä:
> > On Wed, Sep 02, 2015 at 07:15:25AM +0200, Maarten Lankhorst wrote:
> >> Op 01-09-15 om 17:48 schreef Ville Syrjälä:
> >>> On Tue, Sep 01, 2015 at 08:30:05AM -0700, Matt Roper wrote:
>  On Tue, Sep 01, 2015 at 07:24:19AM +0200, Maarten Lankhorst wrote:
> > Op 29-08-15 om 01:57 schreef Matt Roper:
> >> Way back at the beginning of i915's atomic conversion I added
> >> intel_crtc->atomic as a temporary dumping ground for "stuff to do
> >> outside vblank evasion" flags since CRTC states weren't properly wired
> >> up and tracked at that time.  We've had proper CRTC state tracking for 
> >> a
> >> while now, so there's really no reason for this hack to continue to
> >> exist.  Moving forward we want to store intermediate crtc/plane state
> >> data for modesets in addition to the final state, so moving these 
> >> fields
> >> into the proper state object allows us to properly compute them for 
> >> both
> >> the intermediate and final state.
> >>
> >> Signed-off-by: Matt Roper 
> >> ---
> > Can I shoot this patch down? It's better to add a field 'wm_changed'
> > to the crtc_state, which gets reset to false for each crtc_state
> > duplication. It's needed for checking if a cs pageflip can be done for
> > atomic. It would remove the duplication of some checks there.
> >
> > The other atomic state members will die soon. I already have some
> > patches to achieve that. :)
> >
> > I'm not sure if an intermediate state is a good idea. Any code that
> > disables a crtc should only be looking at the old state.
> > pre_plane_update runs all stuff in preparation for disabling planes,
> > while post_plane_update runs everything needed for enabling planes. So
> > no need to split it up I think, maybe put in some intermediate
> > watermarks in intel_atomic_state, but no need for a full crtc_state.
>  Well, the intermediate state stuff was requested by Ville in response to
>  my watermark series, so I posted these patches as an RFC to make sure I
>  was understanding what he was looking for properly.
> 
>  Ville, can you comment?
> >>> My opinion is that the current "disable is special" way of doing things
> >>> is quite horrible. For one it makes it really hard to reason about what
> >>> happens to a plane or crtc during the modeset. It's not just off->on,
> >>> on->off, or same->same, but can be on->off->on. With the intermediate
> >>> state in place, there can only be one transition, so really easy to
> >>> think about what's going on.
> >> pre_plane_update deals with all stuff related to disabling planes, while 
> >> post_plane_update deals with changes after enabling.
> >>
> >> If the crtc goes from on -> off only you could just hammer in the final 
> >> values after the disable.
> >>
> >> While for off->on or on->off->on you can put in the final values in 
> >> .crtc_enable before lighting the pipe. I don't see why wm's would need 
> >> more transitions.
> > One special case after another. Yuck. Not to mention that the plane
> > disable isn't even atomic in the current code, which can look ugly.
> That's easily fixed by adding a pipe_update_start/end pair.
> >>> It'll also mean don't have to sprinkle silly wm update calls all over
> >>> the modeset path. They will just get updated in response to the plane
> >>> state changes. Same for IPS/FBC etc.
> >> IPS and FBC are already calculated correctly in response to modesets.
> > Correctly perhaps, but not in an obvious way.
> It will become more obvious again when pre_plane_update and post_plane_update 
> are loops
> instead of being precalculated from intel_plane_atomic_calc_changes.

It'll never be obvious as long as the on->off->on case exists.

> 
> But if you can precalculate fb_bits and know of wm changed post commit then 
> post_plane_update
> only cares about primary plane state.
> 
> ~Maarten

-- 
Ville Syrjälä
Intel OTC
-
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

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

Re: [Intel-gfx] [RFC 1/3] drm/i915: Roll intel_crtc->atomic into intel_crtc_state

[Maarten Lankhorst]

Op 02-09-15 om 12:35 schreef Ville Syrjälä:
> On Wed, Sep 02, 2015 at 07:15:25AM +0200, Maarten Lankhorst wrote:
>> Op 01-09-15 om 17:48 schreef Ville Syrjälä:
>>> On Tue, Sep 01, 2015 at 08:30:05AM -0700, Matt Roper wrote:
 On Tue, Sep 01, 2015 at 07:24:19AM +0200, Maarten Lankhorst wrote:
> Op 29-08-15 om 01:57 schreef Matt Roper:
>> Way back at the beginning of i915's atomic conversion I added
>> intel_crtc->atomic as a temporary dumping ground for "stuff to do
>> outside vblank evasion" flags since CRTC states weren't properly wired
>> up and tracked at that time.  We've had proper CRTC state tracking for a
>> while now, so there's really no reason for this hack to continue to
>> exist.  Moving forward we want to store intermediate crtc/plane state
>> data for modesets in addition to the final state, so moving these fields
>> into the proper state object allows us to properly compute them for both
>> the intermediate and final state.
>>
>> Signed-off-by: Matt Roper 
>> ---
> Can I shoot this patch down? It's better to add a field 'wm_changed'
> to the crtc_state, which gets reset to false for each crtc_state
> duplication. It's needed for checking if a cs pageflip can be done for
> atomic. It would remove the duplication of some checks there.
>
> The other atomic state members will die soon. I already have some
> patches to achieve that. :)
>
> I'm not sure if an intermediate state is a good idea. Any code that
> disables a crtc should only be looking at the old state.
> pre_plane_update runs all stuff in preparation for disabling planes,
> while post_plane_update runs everything needed for enabling planes. So
> no need to split it up I think, maybe put in some intermediate
> watermarks in intel_atomic_state, but no need for a full crtc_state.
 Well, the intermediate state stuff was requested by Ville in response to
 my watermark series, so I posted these patches as an RFC to make sure I
 was understanding what he was looking for properly.

 Ville, can you comment?
>>> My opinion is that the current "disable is special" way of doing things
>>> is quite horrible. For one it makes it really hard to reason about what
>>> happens to a plane or crtc during the modeset. It's not just off->on,
>>> on->off, or same->same, but can be on->off->on. With the intermediate
>>> state in place, there can only be one transition, so really easy to
>>> think about what's going on.
>> pre_plane_update deals with all stuff related to disabling planes, while 
>> post_plane_update deals with changes after enabling.
>>
>> If the crtc goes from on -> off only you could just hammer in the final 
>> values after the disable.
>>
>> While for off->on or on->off->on you can put in the final values in 
>> .crtc_enable before lighting the pipe. I don't see why wm's would need more 
>> transitions.
> One special case after another. Yuck. Not to mention that the plane
> disable isn't even atomic in the current code, which can look ugly.
That's easily fixed by adding a pipe_update_start/end pair.
>>> It'll also mean don't have to sprinkle silly wm update calls all over
>>> the modeset path. They will just get updated in response to the plane
>>> state changes. Same for IPS/FBC etc.
>> IPS and FBC are already calculated correctly in response to modesets.
> Correctly perhaps, but not in an obvious way.
It will become more obvious again when pre_plane_update and post_plane_update 
are loops
instead of being precalculated from intel_plane_atomic_calc_changes.

But if you can precalculate fb_bits and know of wm changed post commit then 
post_plane_update
only cares about primary plane state.

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

Re: [Intel-gfx] [RFC 1/3] drm/i915: Roll intel_crtc->atomic into intel_crtc_state

[Ville Syrjälä]

On Wed, Sep 02, 2015 at 07:15:25AM +0200, Maarten Lankhorst wrote:
> Op 01-09-15 om 17:48 schreef Ville Syrjälä:
> > On Tue, Sep 01, 2015 at 08:30:05AM -0700, Matt Roper wrote:
> >> On Tue, Sep 01, 2015 at 07:24:19AM +0200, Maarten Lankhorst wrote:
> >>> Op 29-08-15 om 01:57 schreef Matt Roper:
>  Way back at the beginning of i915's atomic conversion I added
>  intel_crtc->atomic as a temporary dumping ground for "stuff to do
>  outside vblank evasion" flags since CRTC states weren't properly wired
>  up and tracked at that time.  We've had proper CRTC state tracking for a
>  while now, so there's really no reason for this hack to continue to
>  exist.  Moving forward we want to store intermediate crtc/plane state
>  data for modesets in addition to the final state, so moving these fields
>  into the proper state object allows us to properly compute them for both
>  the intermediate and final state.
> 
>  Signed-off-by: Matt Roper 
>  ---
> >>> Can I shoot this patch down? It's better to add a field 'wm_changed'
> >>> to the crtc_state, which gets reset to false for each crtc_state
> >>> duplication. It's needed for checking if a cs pageflip can be done for
> >>> atomic. It would remove the duplication of some checks there.
> >>>
> >>> The other atomic state members will die soon. I already have some
> >>> patches to achieve that. :)
> >>>
> >>> I'm not sure if an intermediate state is a good idea. Any code that
> >>> disables a crtc should only be looking at the old state.
> >>> pre_plane_update runs all stuff in preparation for disabling planes,
> >>> while post_plane_update runs everything needed for enabling planes. So
> >>> no need to split it up I think, maybe put in some intermediate
> >>> watermarks in intel_atomic_state, but no need for a full crtc_state.
> >> Well, the intermediate state stuff was requested by Ville in response to
> >> my watermark series, so I posted these patches as an RFC to make sure I
> >> was understanding what he was looking for properly.
> >>
> >> Ville, can you comment?
> > My opinion is that the current "disable is special" way of doing things
> > is quite horrible. For one it makes it really hard to reason about what
> > happens to a plane or crtc during the modeset. It's not just off->on,
> > on->off, or same->same, but can be on->off->on. With the intermediate
> > state in place, there can only be one transition, so really easy to
> > think about what's going on.
> pre_plane_update deals with all stuff related to disabling planes, while 
> post_plane_update deals with changes after enabling.
> 
> If the crtc goes from on -> off only you could just hammer in the final 
> values after the disable.
> 
> While for off->on or on->off->on you can put in the final values in 
> .crtc_enable before lighting the pipe. I don't see why wm's would need more 
> transitions.

One special case after another. Yuck. Not to mention that the plane
disable isn't even atomic in the current code, which can look ugly.

> > It'll also mean don't have to sprinkle silly wm update calls all over
> > the modeset path. They will just get updated in response to the plane
> > state changes. Same for IPS/FBC etc.
> IPS and FBC are already calculated correctly in response to modesets.

Correctly perhaps, but not in an obvious way.

-- 
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] drm/i915: Detect virtual south bridge

[Daniel Vetter]

On Fri, Aug 28, 2015 at 01:10:22PM +0100, robert.beck...@intel.com wrote:
> From: Robert Beckett 
> 
> Virtualized systems often use a virtual P2X4 south bridge.
> Detect this in intel_detect_pch and make a best guess as to which PCH
> we should be using.
> 
> This was seen on vmware esxi hypervisor. When passing the graphics device
> through to a guest, it can not pass through the PCH. Instead it simulates
> a P2X4 southbridge.
> 
> Signed-off-by: Robert Beckett 

What changed here compared to your earlier submission a day before?
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.c |   30 ++
>  drivers/gpu/drm/i915/i915_drv.h |1 +
>  2 files changed, 31 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index ce3bd0c..8e158b3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -443,6 +443,34 @@ static const struct pci_device_id pciidlist[] = {
> /* aka */
>  
>  MODULE_DEVICE_TABLE(pci, pciidlist);
>  
> +static enum intel_pch intel_virt_detect_pch(struct drm_device *dev)
> +{
> + enum intel_pch ret = PCH_NOP;
> +
> + /*
> +  * In a virtualized passthrough environment we can be in a
> +  * setup where the ISA bridge is not able to be passed through.
> +  * In this case, a south bridge can be emulated and we have to
> +  * make an educated guess as to which PCH is really there.
> +  */
> +
> + if (IS_GEN5(dev)) {
> + ret = PCH_IBX;
> + DRM_DEBUG_KMS("Assuming Ibex Peak PCH\n");
> + } else if (IS_GEN6(dev) || IS_IVYBRIDGE(dev)) {
> + ret = PCH_CPT;
> + DRM_DEBUG_KMS("Assuming CouarPoint PCH\n");
> + } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> + ret = PCH_LPT;
> + DRM_DEBUG_KMS("Assuming LynxPoint PCH\n");
> + } else if (IS_SKYLAKE(dev)) {
> + ret = PCH_SPT;
> + DRM_DEBUG_KMS("Assuming SunrisePoint PCH\n");
> + }
> +
> + return ret;
> +}
> +
>  void intel_detect_pch(struct drm_device *dev)
>  {
>   struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -503,6 +531,8 @@ void intel_detect_pch(struct drm_device *dev)
>   dev_priv->pch_type = PCH_SPT;
>   DRM_DEBUG_KMS("Found SunrisePoint LP PCH\n");
>   WARN_ON(!IS_SKYLAKE(dev));
> + } else if (id == INTEL_PCH_P2X_DEVICE_ID_TYPE) {
> + dev_priv->pch_type = intel_virt_detect_pch(dev);
>   } else
>   continue;
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8c93845..6eb0230 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2584,6 +2584,7 @@ struct drm_i915_cmd_table {
>  #define INTEL_PCH_LPT_LP_DEVICE_ID_TYPE  0x9c00
>  #define INTEL_PCH_SPT_DEVICE_ID_TYPE 0xA100
>  #define INTEL_PCH_SPT_LP_DEVICE_ID_TYPE  0x9D00
> +#define INTEL_PCH_P2X_DEVICE_ID_TYPE 0x7100
>  
>  #define INTEL_PCH_TYPE(dev) (__I915__(dev)->pch_type)
>  #define HAS_PCH_SPT(dev) (INTEL_PCH_TYPE(dev) == PCH_SPT)
> -- 
> 1.7.9.5
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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] [PATCH v2 6/6] drm/i915: Allow Broadwell guest with Intel GVT-g

[Daniel Vetter]

On Fri, Aug 28, 2015 at 03:41:19PM +0800, Zhiyuan Lv wrote:
> I915 Broadwell guest driver is now supported to run inside a VM with
> Intel GVT-g
> 
> v2:
> - Introduce HAS_VGPU macro (Zhenyu Wang)
> 
> Signed-off-by: Zhiyuan Lv 
> Signed-off-by: Zhi Wang 
> Reviewed-by: Joonas Lahtinen 

I'll hold of on this one for the polished version of patch 2, but all
others are merged to dinq.

Thanks, Daniel

> ---
>  drivers/gpu/drm/i915/i915_vgpu.c | 2 +-
>  drivers/gpu/drm/i915/i915_vgpu.h | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.c 
> b/drivers/gpu/drm/i915/i915_vgpu.c
> index 5eee75b..f98a979 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.c
> +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> @@ -66,7 +66,7 @@ void i915_check_vgpu(struct drm_device *dev)
>  
>   BUILD_BUG_ON(sizeof(struct vgt_if) != VGT_PVINFO_SIZE);
>  
> - if (!IS_HASWELL(dev))
> + if (!HAS_VGPU(dev))
>   return;
>  
>   magic = readq(dev_priv->regs + vgtif_reg(magic));
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.h 
> b/drivers/gpu/drm/i915/i915_vgpu.h
> index 21c97f4..9a9eb57 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.h
> +++ b/drivers/gpu/drm/i915/i915_vgpu.h
> @@ -114,6 +114,8 @@ struct vgt_if {
>  #define VGT_DRV_DISPLAY_NOT_READY 0
>  #define VGT_DRV_DISPLAY_READY 1  /* ready for display switch */
>  
> +#define HAS_VGPU(dev)(IS_HASWELL(dev) || IS_BROADWELL(dev))
> +
>  extern void i915_check_vgpu(struct drm_device *dev);
>  extern int intel_vgt_balloon(struct drm_device *dev);
>  extern void intel_vgt_deballoon(void);
> -- 
> 1.9.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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] [PATCH v2 2/6] drm/i915: Enable full ppgtt for vgpu on Broadwell

[Daniel Vetter]

On Mon, Aug 31, 2015 at 03:55:58PM +0300, Joonas Lahtinen wrote:
> On pe, 2015-08-28 at 15:41 +0800, Zhiyuan Lv wrote:
> > The full ppgtt is supported now in Intel GVT-g device model.
> > Broadwell
> > is allowed to use it in virtual machines.
> > 
> > v2:
> > - Keep backward compatibility on HSW with old device model (daniel)
> > 
> > Signed-off-by: Zhiyuan Lv 
> > Signed-off-by: Zhi Wang 
> > Reviewed-by: Joonas Lahtinen 
> 
> It's a good idea to add the version reviewed after Reviewed-by, when
> adding a new revision. This is not to make it look like the new
> revision had already been reviewed too.
> 
> I this case:
> 
> Reviewed-by: Joonas Lahtinen  (v1)
> 
> Would have been appropriate.
> 
> But you can now leave it as it is, as this patch seems fine, too. Maybe
> could still add a comment in the code what makes Haswell special.
> 
> Regards, Joonas
> 
> > ---
> >  drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index ed10e77..56cc8e8 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -108,8 +108,8 @@ static int sanitize_enable_ppgtt(struct
> > drm_device *dev, int enable_ppgtt)
> > has_aliasing_ppgtt = INTEL_INFO(dev)->gen >= 6;
> > has_full_ppgtt = INTEL_INFO(dev)->gen >= 7;
> >  
> > -   if (intel_vgpu_active(dev))
> > -   has_full_ppgtt = false; /* emulation is too hard */
> > +   if (intel_vgpu_active(dev) && (IS_HASWELL(dev)))
> > +   has_full_ppgtt = false;

I'd say the real check here should be for INTEL_INFO(dev)->gen < 8. Only
checking for hsw is a bit confusing since then people wonder why hsw is
special. But the only reason is that vgpu isn't supported on pre-hsw.

Using the gen check instead will make it clear that this is a generic
issue with pre-gen8 hw (no execlists) and imo be less confusing. Maybe
even add a comment like:

/* virtualizing ppgtt with execlists is too hard */
> >  
> > /*
> >  * We don't allow disabling PPGTT for gen9+ as it's a
> > requirement for
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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] [PATCH igt] dma-buf mmap

[Daniel Vetter]

On Thu, Aug 27, 2015 at 07:48:49PM -0300, Tiago Vignatti wrote:
> Hi,
> 
> Here's the igt side of the work I sent yesterday to dri-devel:
> 
> http://lists.freedesktop.org/archives/dri-devel/2015-August/089263.html
> 
> I've addressed all the commentaries made in the previous igt patchset and I
> believe that all tests now run fine in older kernels that don't support
> dma-buf mmap functionality -- hint hint could be applied right now in igt main
> repo hint hint :)

Merge order is:
0. Full stack is ready&reviewed, from kernel to userspace including tests
and all.
1. Kernel patches go in (for ioctl allocations).
2. Then and only then userspace gets merged.

Otherwise ioctl allocation clashes and that's no fun.
-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

Re: [Intel-gfx] About the iGVT-g's requirement to pin guest contexts in VM

[Daniel Vetter]

On Wed, Sep 02, 2015 at 05:20:34PM +0800, Zhiyuan Lv wrote:
> Hi Daniel,
> 
> Thanks for the comments! And my reply in line:
> 
> On Wed, Sep 02, 2015 at 10:19:03AM +0200, Daniel Vetter wrote:
> > > > 
> > > > Also you obviously have to complete the copying from shadow->guest ctx
> > > > before you send the irq to the guest to signal ctx completion. Which 
> > > > means
> > > > there's really no overall problem here from a design pov, the only thing
> > > 
> > > Right. We cannot control when guest driver sees seqno change, but we
> > > can control when guest sees context interrupts. The guest CSB update
> > > and interrupt injection will be after we finish writing guest
> > > contexts.
> > > 
> > > So right now we have two options of context shadowing: one is to track
> > > the whole life-cycle of guest context, and another is to do the shadow
> > > work in context schedule-in/schedule-out time. Zhi draws a nice
> > > picture of them.
> > > 
> > > Currently we do not have concrete performance comparison of the two
> > > approaches. We will have a try and see. And about this patchset, I
> > > will remove the "context notification" part and send out an updated
> > > version. Thanks!
> > > 
> > > > you have to do is fix up bugs in the host code (probably you should just
> > > > write through the ggtt).
> > > 
> > > Sorry could you elaborate a little more about this? Guest context may
> > > not always be in aperture right?
> > 
> > Yeah the high-level problem is that global gtt is contended (we already
> > have trouble with that on xengt and there's the ongoing but unfished
> > partial mmap support for that). And permanently pinning execlist contexts
> > will cause lots of troubles.
> > 
> > Windows can do this because it segments the global gtt into different
> > parts (at least last time I looked at their memory manager), which means
> > execlist will never sit in the middle of the range used for mmaps. But
> > linux has a unified memory manager, which means execlist can sit anywhere,
> > and therefore badly fragment the global gtt. If we pin them then that will
> > cause trouble after long system uptime. And afaiui xengt is mostly aimed
> > at servers, where the uptime assumption should be "runs forever".
> 
> In server side, we do not expect host to run much graphics workloads
> (all should be in virtual machines with shorter uptime). But yeah, gm
> fragmentation is still an issue. Thanks for the explanation!

Fragmentation is a lot worse on the guest side (due to the reduce global
gtt space due to ballooning). Host isn't really any different.

> > Compounding factor is that despite that I raised this in the original
> > review execlist is still not yet using the active list in upstream and
> > instead does short-time pinning. It's better than pinning forever but
> > still breaks the eviction logic.
> > 
> > What Chris Wilson and I talked about forever is adding an object-specific
> > global_gtt_unbind hook. The idea is that this would be called when
> > unbinding/evicting a special object (e.g. hw context), and you could use
> > that to do the host signalling. That would be the perfect combination of
> > both approaches:
> 
> Thanks for the suggestion! That sounds great! Right now in XenGT part
> we still plan to try the shadow context work in context
> schedule-in/out time. With this way, we do not need to pin context as
> well as the host notification. We will collect some performance data
> to see how it works.
> 
> I sent out v2 patch set which has removed the pin/unpin of execlist
> contexts. The patchset addressed review comments from you, Chris and
> Joonas(Sorry I forgot to add CC to related people). Is that patch set
> OK to be merged? Thanks!

I'll defer to detailed reviewers, but if the static pinning is gone then
I'm ok. We can add the optimization I described here later on.
-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

Re: [Intel-gfx] [PATCH 4/4] ALSA: hda - Wake the codec up on pin/ELD notify events

[Takashi Iwai]

On Wed, 02 Sep 2015 10:32:34 +0200,
Daniel Vetter wrote:
> 
> On Wed, Sep 02, 2015 at 10:03:55AM +0200, Takashi Iwai wrote:
> > On Wed, 02 Sep 2015 10:00:44 +0200,
> > Daniel Vetter wrote:
> > > 
> > > On Fri, Aug 28, 2015 at 04:10:36PM +0300, Jani Nikula wrote:
> > > > On Thu, 20 Aug 2015, Takashi Iwai  wrote:
> > > > > On Thu, 20 Aug 2015 11:41:42 +0200,
> > > > > David Henningsson wrote:
> > > > >> 
> > > > >> 
> > > > >> 
> > > > >> On 2015-08-20 11:28, Takashi Iwai wrote:
> > > > >> > On Wed, 19 Aug 2015 10:48:58 +0200,
> > > > >> > David Henningsson wrote:
> > > > >> >>
> > > > >> >> Whenever there is an event from the i915 driver, wake the codec
> > > > >> >> and recheck plug/unplug + ELD status.
> > > > >> >>
> > > > >> >> This fixes the issue with lost unsol events in power save mode,
> > > > >> >> the codec and controller can now sleep in D3 and still know when
> > > > >> >> the HDMI monitor has been connected.
> > > > >> >>
> > > > >> >> Signed-off-by: David Henningsson 
> > > > >> >
> > > > >> > This addition looks fine, but then we'll get double notification 
> > > > >> > for
> > > > >> > the normal hotplug/unplug, one via component ops and another via 
> > > > >> > unsol
> > > > >> > event?
> > > > >> 
> > > > >> Right, in case the unsol event actually works...
> > > > >> 
> > > > >> I would argue that the normal case would be that the controller and 
> > > > >> codec is in D3 which means that the unsol event never gets through - 
> > > > >> due 
> > > > >> to hw limitations - which is what triggered this patch set in the 
> > > > >> first 
> > > > >> place.
> > > > >> 
> > > > >> But yes, in some case we might get double notification, but this 
> > > > >> should 
> > > > >> not cause any trouble in practice. The unsol event could be turned 
> > > > >> off, 
> > > > >> but would it be okay to save that for a later patch set (so I don't 
> > > > >> miss 
> > > > >> the upcoming merge window)?
> > > > >
> > > > > In that case, it should be mentioned in the changelog at least.
> > > > >
> > > > > This series came a bit too late for the merge window, so I'm not sure
> > > > > whether this can get in.  I personally find it OK, so take my ack for
> > > > > ALSA parts (patch 3/4), but the rest need review and ack from i915
> > > > > guys.  And we don't know who to merge this, if any.  The changes are
> > > > > almost even to i915 and hda.  I don't mind either way, via drm or
> > > > > sound tree.
> > > > 
> > > > Personally I'm fine with this still going for v4.3 since to me it looks
> > > > like any regressions this might cause will be on your side. *grin*.
> > > 
> > > The only bit I want is some decent kerneldoc for the ad-hoc growing
> > > i915/hda-intel interfaces. But that can be done in follow-up patches and
> > > needs to be coordinated with the audio rate handling series anyway. So ack
> > > from my side for all getting merged through alsa trees too.
> > 
> > Are you going to merge Libin's patchset?  If yes, it's better to align
> > both patchsets through a single tree.  It'll make merges easier, as
> > both touch the similar place.
> 
> Oh that was an ack for merging it all through your tree. If you punt it to
> 4.4 I might ask you for a stable tree to pull in in case I get conflicts.

OK, then I'll merge David's patchset now for the upcoming pull request
for 4.3-rc1.


thanks,

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

Re: [Intel-gfx] About the iGVT-g's requirement to pin guest contexts in VM

[Zhiyuan Lv]

Hi Daniel,

Thanks for the comments! And my reply in line:

On Wed, Sep 02, 2015 at 10:19:03AM +0200, Daniel Vetter wrote:
> > > 
> > > Also you obviously have to complete the copying from shadow->guest ctx
> > > before you send the irq to the guest to signal ctx completion. Which means
> > > there's really no overall problem here from a design pov, the only thing
> > 
> > Right. We cannot control when guest driver sees seqno change, but we
> > can control when guest sees context interrupts. The guest CSB update
> > and interrupt injection will be after we finish writing guest
> > contexts.
> > 
> > So right now we have two options of context shadowing: one is to track
> > the whole life-cycle of guest context, and another is to do the shadow
> > work in context schedule-in/schedule-out time. Zhi draws a nice
> > picture of them.
> > 
> > Currently we do not have concrete performance comparison of the two
> > approaches. We will have a try and see. And about this patchset, I
> > will remove the "context notification" part and send out an updated
> > version. Thanks!
> > 
> > > you have to do is fix up bugs in the host code (probably you should just
> > > write through the ggtt).
> > 
> > Sorry could you elaborate a little more about this? Guest context may
> > not always be in aperture right?
> 
> Yeah the high-level problem is that global gtt is contended (we already
> have trouble with that on xengt and there's the ongoing but unfished
> partial mmap support for that). And permanently pinning execlist contexts
> will cause lots of troubles.
> 
> Windows can do this because it segments the global gtt into different
> parts (at least last time I looked at their memory manager), which means
> execlist will never sit in the middle of the range used for mmaps. But
> linux has a unified memory manager, which means execlist can sit anywhere,
> and therefore badly fragment the global gtt. If we pin them then that will
> cause trouble after long system uptime. And afaiui xengt is mostly aimed
> at servers, where the uptime assumption should be "runs forever".

In server side, we do not expect host to run much graphics workloads
(all should be in virtual machines with shorter uptime). But yeah, gm
fragmentation is still an issue. Thanks for the explanation!

> 
> Compounding factor is that despite that I raised this in the original
> review execlist is still not yet using the active list in upstream and
> instead does short-time pinning. It's better than pinning forever but
> still breaks the eviction logic.
> 
> What Chris Wilson and I talked about forever is adding an object-specific
> global_gtt_unbind hook. The idea is that this would be called when
> unbinding/evicting a special object (e.g. hw context), and you could use
> that to do the host signalling. That would be the perfect combination of
> both approaches:

Thanks for the suggestion! That sounds great! Right now in XenGT part
we still plan to try the shadow context work in context
schedule-in/out time. With this way, we do not need to pin context as
well as the host notification. We will collect some performance data
to see how it works.

I sent out v2 patch set which has removed the pin/unpin of execlist
contexts. The patchset addressed review comments from you, Chris and
Joonas(Sorry I forgot to add CC to related people). Is that patch set
OK to be merged? Thanks!

Regards,
-Zhiyuan

> 
> - Fast: host signalling (and therefore shadow context recreation) would
>   only be done when the execlist context has actually moved around. That
>   almost never happens, and hence per-execbuf overhead would be as low as
>   with your pinning solution.
> 
> - Flexible: The i915 memory manager is still in full control since we
>   don't pin any objects unecessarily.
> 
> Cheers, 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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/5] Documentation: drm: Convert KMS Properties HTML table to CALS

[Graham Whaley]

On Tue, 2015-09-01 at 14:56 -0300, Danilo Cesar Lemes de Paula wrote:
> On 08/25/2015 01:10 PM, Graham Whaley wrote:
> > On Tue, 2015-08-25 at 16:29 +0200, Daniel Vetter wrote:
> > > On Tue, Aug 25, 2015 at 10:26:44AM +0100, Graham Whaley wrote:
> > > > The KMS Properties table is in HTML format, which is not
> > > > supported
> > > > for building pdfdocs, resulting in the following types of
> > > > errors:
> > > > 
> > > >  jade:/Documentation/DocBook/drm.xml:34413:15:E: there is no
> > > > attribute
> > > >  "border"
> > > >  jade:/Documentation/DocBook/drm.xml:34413:31:E: there is no
> > > > attribute
> > > >  "cellpadding"
> > > >  jade:/Documentation/DocBook/drm.xml:34413:47:E: there is no
> > > > attribute
> > > >  "cellspacing"
> > > >  jade:/Documentation/DocBook/drm.xml:34414:7:E: document type
> > > > does
> > > > not
> > > >  allow element "tbody" here
> > > > 
> > > > Convert the table over to a CALS format table
> > > 
> > > Hm, long-term plan was to move this table into DOC: comments in
> > > the
> > > source-code using markdown, which we now have (at least in
> > > drm-intel-nightly and also planned to be merged into 4.4). Since
> > > this
> > > is
> > > both a lot of churn I'd like to get there in just 1 step ...
> > > -Daniel
> > First - I've just noted an erroneous debug comment (or two) left in
> > this patch as well, so looks like I will have to re-issue the
> > series
> > anyway.
> > 
> > OK. I guess this comes down to a matter of timing...
> > From Danilos patch of: f6d6913 (drm/doc: Convert to markdown)
> > we can see markdown does not natively support tables, and we'd have
> > to
> > make this a fixed width layout like the one in that patch I
> > suspect.
> > Danilo - any advice on how you did that other table conversion? I
> > just
> > did a pandoc docbook->markdown_github and it looks some way there -
> > but
> > of course seems to have not honored the multi-column items, of
> > which
> > there are a few. It's probably not too bad to fix up by hand - I'll
> > see
> > if I can get that to work...
> 
> Hi Graham,
> 
> To be honest I didn't have to do any conversion as that table was
> already in the header file. I just added 4 spaces so it would be
> transformed into fixed width.
> 
> However, there's tool you can use to help you: http://pandoc.org/try/
> I did a lot of translation there. If your table doesn't have any
> spancells, you can put the HTML code there and get the Markdown for
> free.
> 
> Danilo
Thanks,
 I got to have a look at this yesterday. I did a text render from the
html using 'links' that worked surprisingly well, but the table has
many spancells (both vertical and horizontal), and some other issues
arose. I'll do an email later with some details of what I've found, but
right now I'm not hopeful that it will be practical to move that large
KMS Properties table to markdown. More later.

 Graham

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

Re: [Intel-gfx] [PATCH] drm/atomic: Make sure lock is held in trylock contexts.

[Daniel Vetter]

On Thu, Aug 27, 2015 at 01:58:09PM +0200, Maarten Lankhorst wrote:
> This will make sure we get a lockdep spat in all cases
> even if the context is a complete garbage pointer.
> 
> Signed-off-by: Maarten Lankhorst 

Applied to drm-misc, thanks.
-Daniel

> ---
> diff --git a/drivers/gpu/drm/drm_modeset_lock.c 
> b/drivers/gpu/drm/drm_modeset_lock.c
> index 9abee87c1501..7c9ca2381d78 100644
> --- a/drivers/gpu/drm/drm_modeset_lock.c
> +++ b/drivers/gpu/drm/drm_modeset_lock.c
> @@ -305,6 +305,8 @@ static inline int modeset_lock(struct drm_modeset_lock 
> *lock,
>   WARN_ON(ctx->contended);
>  
>   if (ctx->trylock_only) {
> + lockdep_assert_held(&ctx->ww_ctx);
> +
>   if (!ww_mutex_trylock(&lock->mutex))
>   return -EBUSY;
>   else
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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] [PATCH 2/3] drm/i915: add yesno utility function

[Daniel Vetter]

On Mon, Aug 31, 2015 at 03:33:13PM +0100, Chris Wilson wrote:
> On Mon, Aug 31, 2015 at 05:23:27PM +0300, Jani Nikula wrote:
> > On Thu, 27 Aug 2015, Jani Nikula  wrote:
> > > On Thu, 27 Aug 2015, Chris Wilson  wrote:
> > >> On Thu, Aug 27, 2015 at 04:23:30PM +0300, Jani Nikula wrote:
> > >>> Add a common function to return "yes" or "no" string based on the
> > >>> argument, and drop the local versions of it.
> > >>
> > >> Purely out of curiosity, gcc is able to amalgamate the constant strings
> > >> (I remember reading that it is intelligent enough to do so), right? i.e.
> > >> size i915.ko doesn't change (at least .data, we may see .text
> > >> differences for gcc having different ideas about inlines)?
> > >
> > > I admit to giving GCC the benefit of the doubt. I may be naïve that way,
> > > trusting the tools to do what seems like the obviously right thing to
> > > do.
> > >
> > > If GCC lets us down, we could try something like the yesno version in
> > > drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c. GCC not doing the
> > > right thing with that would be violating the standard.
> > 
> > AFAICT GCC does the right thing with the patch.
> 
> Fwiw, I didn't see any harm in the series, so
> Reviewed-by: Chris Wilson 

Merged just this patch (due to conflict fun for now) to dinq, 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

Re: [Intel-gfx] [RFC PATCH] drm/i915: Use universal planes for cursor on skylake.

[Daniel Vetter]

On Thu, Aug 27, 2015 at 12:08:30PM +0200, Maarten Lankhorst wrote:
> This appears to make all the cursor tests really slow because of the many 
> calls to skl_update_wm
> when the cursor plane visibility is changed. It performs does 3 vblanks each 
> time it's called, and
> it's probably called more than once on each update.

On all other platforms wm updates (right now at least) don't do any vblank
waits, which means changing cursors actually _does_ cause tons of
underruns. Can we perhaps add a hack in skl to do the same (maybe just for
cursors) so that we can get this in? There's lots other work that really
wants proper universal planes ...
-Daniel

> 
> Smarter watermark updates will hopefully fix this..
> 
> This patch is tested with Xorg and kms_cursor_crc, and is just a RFC. :)
> 
> Signed-off-by: Maarten Lankhorst 
> ---
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 05523491ad6f..bf5f814f72ae 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -263,11 +263,11 @@ struct i915_hotplug {
>   for ((__p) = 0; (__p) < INTEL_INFO(__dev_priv)->num_pipes; (__p)++)
>  #define for_each_plane(__dev_priv, __pipe, __p)  
> \
>   for ((__p) = 0; \
> -  (__p) < INTEL_INFO(__dev_priv)->num_sprites[(__pipe)] + 1; \
> +  (__p) < INTEL_INFO(__dev_priv)->num_sprites[(__pipe)] + 1 + 
> IS_GEN9(__dev_priv);   \
>(__p)++)
>  #define for_each_sprite(__dev_priv, __p, __s)
> \
>   for ((__s) = 0; \
> -  (__s) < INTEL_INFO(__dev_priv)->num_sprites[(__p)];\
> +  (__s) < INTEL_INFO(__dev_priv)->num_sprites[(__p)] + 
> IS_GEN9(__dev_priv);  \
>(__s)++)
>  
>  #define for_each_crtc(dev, crtc) \
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 92e3f074ae30..307d1374daa5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1402,7 +1402,7 @@ static void assert_sprites_disabled(struct 
> drm_i915_private *dev_priv,
>  
>   if (INTEL_INFO(dev)->gen >= 9) {
>   for_each_sprite(dev_priv, pipe, sprite) {
> - val = I915_READ(PLANE_CTL(pipe, sprite));
> + val = I915_READ(PLANE_CTL(pipe, sprite + 1));
>   I915_STATE_WARN(val & PLANE_CTL_ENABLE,
>"plane %d assertion failure, should be off on pipe 
> %c but is still active\n",
>sprite, pipe_name(pipe));
> @@ -11563,12 +11563,15 @@ int intel_plane_atomic_calc_changes(struct 
> drm_crtc_state *crtc_state,
>   bool mode_changed = needs_modeset(crtc_state);
>   bool was_crtc_enabled = crtc->state->active;
>   bool is_crtc_enabled = crtc_state->active;
> -
> + enum drm_plane_type plane_type = plane->type;
>   bool turn_off, turn_on, visible, was_visible;
>   struct drm_framebuffer *fb = plane_state->fb;
>  
> + if (IS_GEN9(dev) && plane->type == DRM_PLANE_TYPE_CURSOR)
> + plane_type = DRM_PLANE_TYPE_OVERLAY;
> +
>   if (crtc_state && INTEL_INFO(dev)->gen >= 9 &&
> - plane->type != DRM_PLANE_TYPE_CURSOR) {
> + plane_type != DRM_PLANE_TYPE_CURSOR) {
>   ret = skl_update_scaler_plane(
>   to_intel_crtc_state(crtc_state),
>   to_intel_plane_state(plane_state));
> @@ -11601,7 +11604,7 @@ int intel_plane_atomic_calc_changes(struct 
> drm_crtc_state *crtc_state,
>   if (turn_on) {
>   intel_crtc->atomic.update_wm_pre = true;
>   /* must disable cxsr around plane enable/disable */
> - if (plane->type != DRM_PLANE_TYPE_CURSOR) {
> + if (plane_type != DRM_PLANE_TYPE_CURSOR) {
>   intel_crtc->atomic.disable_cxsr = true;
>   /* to potentially re-enable cxsr */
>   intel_crtc->atomic.wait_vblank = true;
> @@ -11610,7 +11613,7 @@ int intel_plane_atomic_calc_changes(struct 
> drm_crtc_state *crtc_state,
>   } else if (turn_off) {
>   intel_crtc->atomic.update_wm_post = true;
>   /* must disable cxsr around plane enable/disable */
> - if (plane->type != DRM_PLANE_TYPE_CURSOR) {
> + if (plane_type != DRM_PLANE_TYPE_CURSOR) {
>   if (is_crtc_enabled)
>   intel_crtc->atomic.wait_vblank = true;
>   intel_crtc->atomic.disable_cxsr = true;
> @@ -11623,7 +11626,7 @@ int intel_plane_atomic_calc_changes(struct 
> drm_crtc_state *crtc_state,
>   intel_crtc->atomic.fb_bits |=
>   to_intel_plane(plane)->frontbuffer_bit;
>  
> - switch (plane->type) {
> + switch (plane_type) {
>   case DRM_PLANE_TYPE_PRIMARY

Re: [Intel-gfx] [PATCH 4/4] drm/i915: force full detect on sink count change

[Daniel Vetter]

On Thu, Aug 27, 2015 at 02:18:32PM +0530, Sivakumar Thulasimani wrote:
> From: "Thulasimani,Sivakumar" 
> 
> This patch checks for changes in sink count between short pulse
> hpds and forces full detect when there is a change.
> 
> This will allow both detection of hotplug and unplug of panels
> through dongles that give only short pulse for such events.
> 
> v2: changed variable type from u8 to bool (Jani)
> return immediately if perform_full_detect is set(Siva)
> 
> Signed-off-by: Sivakumar Thulasimani 

That's still incomplete since in the hotplug work function we check
whether ->detect status has changed. If that doesn't happen (e.g. sink
count changes from 1->2 or whatever) then we'll fail to generate the
required uevent.

I suspect the only clean way to fix this is to merge all the dp hpd
handling together into one place and stop using ->detect for some parts of
it. Then we would have one place only that decides whether anything
changed, and whether we need to send anything to userspace or not.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_dp.c |   27 ++-
>  1 file changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 834f513..279e52c 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4370,26 +4370,39 @@ go_again:
>   *  4. Check link status on receipt of hot-plug interrupt
>   */
>  static void
> -intel_dp_check_link_status(struct intel_dp *intel_dp)
> +intel_dp_check_link_status(struct intel_dp *intel_dp, bool 
> *perform_full_detect)
>  {
>   struct drm_device *dev = intel_dp_to_dev(intel_dp);
>   struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
>   struct intel_crtc *crtc =
>   to_intel_crtc(dp_to_dig_port(intel_dp)->base.base.crtc);
>   u8 sink_irq_vector;
> + u8 old_sink_count = intel_dp->sink_count;
> + bool ret;
>   u8 link_status[DP_LINK_STATUS_SIZE];
>  
>   WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>  
> + *perform_full_detect = false;
> +
>   /* Try to read receiver status if the link appears to be up */
>   if (!intel_dp_get_link_status(intel_dp, link_status)) {
>   return;
>   }
>  
> - /* Now read the DPCD to see if it's actually running */
> - if (!intel_dp_get_dpcd(intel_dp)) {
> + /* Now read the DPCD to see if it's actually running
> +  * Don't return immediately if dpcd read failed,
> +  * if sink count was 1 and dpcd read failed we need
> +  * to do full detection
> +  */
> + ret = intel_dp_get_dpcd(intel_dp);
> +
> + if (old_sink_count != intel_dp->sink_count)
> + *perform_full_detect = true;
> +
> + /* No need to proceed if we are going to do full detect */
> + if (!ret || *perform_full_detect)
>   return;
> - }
>  
>   if (!intel_encoder->base.crtc)
>   return;
> @@ -5031,13 +5044,17 @@ intel_dp_hpd_pulse(struct intel_digital_port 
> *intel_dig_port, bool long_hpd)
>   }
>  
>   if (!intel_dp->is_mst) {
> + bool full_detect;
>   /*
>* we'll check the link status via the normal hot plug 
> path later -
>* but for short hpds we should check it now
>*/
>   drm_modeset_lock(&dev->mode_config.connection_mutex, 
> NULL);
> - intel_dp_check_link_status(intel_dp);
> + intel_dp_check_link_status(intel_dp, &full_detect);
>   drm_modeset_unlock(&dev->mode_config.connection_mutex);
> +
> + if (full_detect)
> + goto put_power;
>   }
>   }
>  
> -- 
> 1.7.9.5
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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] [PATCH 1/4] drm/i915: read dpcd 0 - 12 & link_status always

[Daniel Vetter]

On Tue, Sep 01, 2015 at 01:16:49PM +0300, Jani Nikula wrote:
> On Thu, 27 Aug 2015, Sivakumar Thulasimani  
> wrote:
> > From: "Thulasimani,Sivakumar" 
> >
> > Compliance requires the driver to read dpcd register 0 to 12 and
> > registers 0x200 to 0x205 to be read always.
> > Current code performs dpcd read for short pulse interrupts only
> > if the sink is enabled. This patch forces read for link status
> > and registers 0 to 12.
> 
> While this seems a bit silly from the driver perspective, I don't see it
> doing much harm either.

I don't think this is silly, but fixing it like this might be - currently
we don't do _any_ detection of sink ports, so if you have a hub with two
outputs and then plug in another one and plug out the first userspace
won't reprobe. So probably what we should be doing is not just read the
dpcd, but actually look at it, figure out whether something has changed
and make sure we send out the uevent even if the hpd line stays unchanged
on short pulses to make userspace aware of the changes.

Punting on this for now since I suspect there's a bigger story to be had
here ...
-Daniel

> 
> Note that We do read dpcd 0 to 14 no matter what the spec says.
> 
> Reviewed-by: Jani Nikula 
> 
> >
> > Signed-off-by: Sivakumar Thulasimani 
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c |   12 ++--
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 8a66a44..76561e0 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4374,12 +4374,6 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
> >  
> > WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
> >  
> > -   if (!intel_encoder->base.crtc)
> > -   return;
> > -
> > -   if (!to_intel_crtc(intel_encoder->base.crtc)->active)
> > -   return;
> > -
> > /* Try to read receiver status if the link appears to be up */
> > if (!intel_dp_get_link_status(intel_dp, link_status)) {
> > return;
> > @@ -4390,6 +4384,12 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
> > return;
> > }
> >  
> > +   if (!intel_encoder->base.crtc)
> > +   return;
> > +
> > +   if (!to_intel_crtc(intel_encoder->base.crtc)->active)
> > +   return;
> > +
> > /* Try to read the source of the interrupt */
> > if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
> > intel_dp_get_sink_irq(intel_dp, &sink_irq_vector)) {
> > -- 
> > 1.7.9.5
> >
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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