Re: [Intel-gfx] [PATCH 58/71] drm/i915/chv: Register port D encoders and connectors

2014-04-25 Thread Antti Koskipää
For 50-58, with Jani's coding style fix:

Reviewed-by: Antti Koskipää 

-- 
- Antti

On 04/09/2014 01:28 PM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_reg.h  | 1 +
>  drivers/gpu/drm/i915/intel_display.c | 9 +
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a3957c7..4c0edb8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2435,6 +2435,7 @@ enum punit_power_well {
>  #define GEN3_SDVOC   0x61160
>  #define GEN4_HDMIB   GEN3_SDVOB
>  #define GEN4_HDMIC   GEN3_SDVOC
> +#define CHV_HDMID0x6116C
>  #define PCH_SDVOB0xe1140
>  #define PCH_HDMIBPCH_SDVOB
>  #define PCH_HDMIC0xe1150
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 798d91f..bf64e9d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11050,6 +11050,15 @@ static void intel_setup_outputs(struct drm_device 
> *dev)
>   intel_dp_init(dev, VLV_DISPLAY_BASE + DP_C, 
> PORT_C);
>   }
>  
> + if (IS_CHERRYVIEW(dev)) {
> + if (I915_READ(VLV_DISPLAY_BASE + CHV_HDMID) & 
> SDVO_DETECTED) {
> + intel_hdmi_init(dev, VLV_DISPLAY_BASE + 
> CHV_HDMID,
> + PORT_D);
> + if (I915_READ(VLV_DISPLAY_BASE + DP_D) & 
> DP_DETECTED)
> + intel_dp_init(dev, VLV_DISPLAY_BASE + 
> DP_D, PORT_D);
> + }
> + }
> +
>   intel_dsi_init(dev);
>   } else if (SUPPORTS_DIGITAL_OUTPUTS(dev)) {
>   bool found = false;
> 

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


Re: [Intel-gfx] [PATCH v2] drm/i915: Always apply cursor width changes

2014-06-02 Thread Antti Koskipää
On 06/02/2014 11:49 AM, Daniel Vetter wrote:
> On Fri, May 30, 2014 at 04:35:26PM +0300, Chris Wilson wrote:
>> It is possible for userspace to create a big object large enough for a
>> 256x256, and then switch over to using it as a 64x64 cursor. This
>> requires the cursor update routines to check for a change in width on
>> every update, rather than just when the cursor is originally enabled.
>>
>> This also fixes an issue with 845g/865g which cannot change the base
>> address of the cursor whilst it is active.
>>
>> Signed-off-by: Chris Wilson 
>> [Antti:rebased, adjusted macro names and moved some lines, no functional
>> changes]
>> Reviewed-by: Antti Koskipaa 
>> Tested-by: Antti Koskipaa 
> 
> Still missing the igt testcase. Is Antti still working on that one?

The testcase was just posted. It just needed some cleanup.

> And I
> guess now we also need an Cc: sta...@vger.kernel.org on this one here.
> -Daniel
> 
>> ---
>>  drivers/gpu/drm/i915/i915_debugfs.c  |   2 +-
>>  drivers/gpu/drm/i915/intel_display.c | 106 
>> +--
>>  drivers/gpu/drm/i915/intel_drv.h |   3 +-
>>  3 files changed, 53 insertions(+), 58 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 2e5f76a..04d0b7c 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -2353,7 +2353,7 @@ static int i915_display_info(struct seq_file *m, void 
>> *unused)
>>  
>>  active = cursor_position(dev, crtc->pipe, &x, &y);
>>  seq_printf(m, "\tcursor visible? %s, position (%d, %d), 
>> addr 0x%08x, active? %s\n",
>> -   yesno(crtc->cursor_visible),
>> +   yesno(crtc->cursor_base),
>> x, y, crtc->cursor_addr,
>> yesno(active));
>>  }
>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 731cd01..955f92d 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -7872,29 +7872,33 @@ static void i845_update_cursor(struct drm_crtc 
>> *crtc, u32 base)
>>  struct drm_device *dev = crtc->dev;
>>  struct drm_i915_private *dev_priv = dev->dev_private;
>>  struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> -bool visible = base != 0;
>> -u32 cntl;
>> +uint32_t cntl;
>>  
>> -if (intel_crtc->cursor_visible == visible)
>> -return;
>> -
>> -cntl = I915_READ(_CURACNTR);
>> -if (visible) {
>> +if (base != intel_crtc->cursor_base) {
>>  /* On these chipsets we can only modify the base whilst
>>   * the cursor is disabled.
>>   */
>> +if (intel_crtc->cursor_cntl) {
>> +I915_WRITE(_CURACNTR, 0);
>> +POSTING_READ(_CURACNTR);
>> +intel_crtc->cursor_cntl = 0;
>> +}
>> +
>>  I915_WRITE(_CURABASE, base);
>> +POSTING_READ(_CURABASE);
>> +}
>>  
>> -cntl &= ~(CURSOR_FORMAT_MASK);
>> -/* XXX width must be 64, stride 256 => 0x00 << 28 */
>> -cntl |= CURSOR_ENABLE |
>> +/* XXX width must be 64, stride 256 => 0x00 << 28 */
>> +cntl = 0;
>> +if (base)
>> +cntl = (CURSOR_ENABLE |
>>  CURSOR_GAMMA_ENABLE |
>> -CURSOR_FORMAT_ARGB;
>> -} else
>> -cntl &= ~(CURSOR_ENABLE | CURSOR_GAMMA_ENABLE);
>> -I915_WRITE(_CURACNTR, cntl);
>> -
>> -intel_crtc->cursor_visible = visible;
>> +CURSOR_FORMAT_ARGB);
>> +if (intel_crtc->cursor_cntl != cntl) {
>> +I915_WRITE(_CURACNTR, cntl);
>> +POSTING_READ(_CURACNTR);
>> +intel_crtc->cursor_cntl = cntl;
>> +}
>>  }
>>  
>>  static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
>> @@ -7903,16 +7907,12 @@ static void i9xx_update_cursor(struct drm_crtc 
>> *crtc, u32 base)
>>  struct drm_i915_private *dev_priv = dev->dev_private;
>>  struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>>  int pipe = intel_crtc->pipe;
>> -bool visible = base != 0;
>> -
>> -if (intel_crtc->cursor_visible != visible) {
>> -int16_t width = intel_crtc->cursor_width;
>> -uint32_t cntl = I915_READ(CURCNTR(pipe));
>> -if (base) {
>> -cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
>> -cntl |= MCURSOR_GAMMA_ENABLE;
>> +uint32_t cntl;
>>  
>> -switch (width) {
>> +cntl = 0;
>> +if (base) {
>> +cntl = MCURSOR_GAMMA_ENABLE;
>> +switch (intel_crtc->cursor_width) {
>>  case 64:
>>  cntl |= CURSOR_MODE_64_ARGB_AX;
>>  break;
>> @@ -7925,18 +7925,16 @@ stati

Re: [Intel-gfx] [PATCH v2 0/4] drm/i915: dp: fix order of dp aux i2c device cleanup

2014-02-13 Thread Antti Koskipää
On 02/11/2014 05:12 PM, Imre Deak wrote:
> The main point of this patchset is to fix a driver unload bug caused by
> incorrect order of dp aux i2c cleanup wrt. destroying the corresponding
> encoder/connector objects, see the second patch for details.
> 
> Tested on vlv/dp.
> 
> v2: move all sysfs removal bits to the new connector->unregister
> callback (Daniel)
> 
> Imre Deak (4):
>   drm/i915: add unregister callback to connector
>   drm/i915: dp: fix order of dp aux i2c device cleanup
>   drm/i915: sdvo: fix error path in sdvo_connector_init
>   drm/i915: sdvo: add i2c sysfs symlink to the connector's directory
> 
>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>  drivers/gpu/drm/i915/intel_crt.c |  1 +
>  drivers/gpu/drm/i915/intel_display.c | 14 ++--
>  drivers/gpu/drm/i915/intel_dp.c  | 23 +++-
>  drivers/gpu/drm/i915/intel_drv.h |  8 +
>  drivers/gpu/drm/i915/intel_dsi.c |  1 +
>  drivers/gpu/drm/i915/intel_dvo.c |  1 +
>  drivers/gpu/drm/i915/intel_hdmi.c|  1 +
>  drivers/gpu/drm/i915/intel_lvds.c|  1 +
>  drivers/gpu/drm/i915/intel_sdvo.c| 70 
> +++-
>  drivers/gpu/drm/i915/intel_tv.c  |  1 +
>  11 files changed, 110 insertions(+), 12 deletions(-)
>
> Signed-off-by: Imre Deak 

Reviewed-by: Antti Koskipää 

-- 
- Antti




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


Re: [Intel-gfx] [PATCH 0/7] drm/i915: Some more w/a'ish stuff

2014-02-27 Thread Antti Koskipää
On 02/04/2014 09:59 PM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> So I accidentally looked at gen6_init_clock_gating() and noticed a few
> weird things that should have gotten cleaned up years ago. So I did that.
> 
> While doing that I also noticed the WIZ hashing bits, and the fact that
> we weren't following the BSpec recommendation. After doing a few tests
> on IVB and HSW it looks 16x4 is in fact the best option, 8x4 was second
> best and 8x8 (our current default for !SNB) was the worst. I didn't see
> much (if any) difference w/o MSAA, but w/ MSAA 4x there was some change.
> I used xonotic as my benchmark.
> 
> Ville Syrjälä (7):
>   drm/i915: Fix SNB GT_MODE register setup
>   drm/i915: Assume we implement
> WaStripsFansDisableFastClipPerformanceFix:snb
>   drm/i915: There's no need to mask all 3D_CHICKEN bits on SNB
>   drm/i915: Disable SF pipelined attribute fetch for SNB
>   drm/i915: Change IVB WIZ hashing mode to 16x4
>   drm/i915: Change HSW WIZ hashing mode to 16x4
>   drm/i915: Change BDW WIZ hashing mode to 16x4
> 
>  drivers/gpu/drm/i915/i915_reg.h | 10 +++--
>  drivers/gpu/drm/i915/intel_pm.c | 47 
> ++---
>  2 files changed, 47 insertions(+), 10 deletions(-)

For everything except 4/7, which was reviewed by Ken:

Reviewed-by: Antti Koskipää 

-- 
- Antti


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


Re: [Intel-gfx] [PATCH 0/2] drm/i915: vlv: preparation for enabling RPM

2014-03-28 Thread Antti Koskipää
On 03/27/2014 05:45 PM, Imre Deak wrote:
> These are dependency patches for enabling RPM on VLV. I'm sending them
> in advance because they could be applied independently and the second
> one might fix some eDP issues.
> 
> Imre Deak (2):
>   drm/i915: vlv: cache current CD clock rate
>   drm/i915: vlv: get power domain for eDP vdd
> 
>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>  drivers/gpu/drm/i915/intel_display.c | 16 +++-
>  drivers/gpu/drm/i915/intel_dp.c  | 20 +---
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  drivers/gpu/drm/i915/intel_pm.c  |  4 
>  5 files changed, 30 insertions(+), 12 deletions(-)

For the whole series,

Reviewed-by: Antti Koskipää 

-- 
- Antti



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


Re: [Intel-gfx] [PATCH i-g-t 5/7] kms_cursor_crc: Add reference software rendering

2014-04-02 Thread Antti Koskipää
On 04/02/2014 02:21 PM, Ville Syrjälä wrote:
> On Wed, Apr 02, 2014 at 02:06:28PM +0300, Antti Koskipaa wrote:
> 
>> @@ -184,9 +192,6 @@ static void test_crc_offscreen(test_data_t *test_data)
>>  do_test(test_data, left - (cursor_w+512), right + (cursor_w+512), top   
>>   , bottom );
>>  do_test(test_data, left , right , top - 
>> (cursor_h+512), bottom + (cursor_h+512));
>>  do_test(test_data, left - (cursor_w+512), right + (cursor_w+512), top - 
>> (cursor_h+512), bottom + (cursor_h+512));
>> -
>> -/* go nuts */
>> -do_test(test_data, INT_MIN, INT_MAX, INT_MIN, INT_MAX);
> 
> Why are you dropping the go nuts test? We had an actual bug in the
> driver that this managed to hit. We should keep this test to avoid
> regressions.

Link please. Cairo went nuts with that one, but if there is a case for
the test, I'll work around Cairo to keep it.

-- 
- Antti


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


Re: [Intel-gfx] [PATCH] drm/i915: Clean up display pipe register accesses

2014-01-24 Thread Antti Koskipää
On 01/22/14 08:50, Barbalho, Rafael wrote:

> BCLRPAT is in the transcoder, not the pipe.

> PIPERSRC is in the transcoder not in the pipe.

> PIPECONF is in the pipe not the transcoder.

> Missing from that patch is also all the DSP* registers (DSPCNTR,DSPADDR, 
> etc...) those should use the new _PIPE2 macro.

Yes, you are correct. Seems that some of the original macros were wrong.
V2 coming up.

-- 
- Antti


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


Re: [Intel-gfx] [PATCH v2] drm/i915: Clean up display pipe register accesses

2014-01-27 Thread Antti Koskipää
On 01/24/14 14:52, Ville Syrjälä wrote:
> On Fri, Jan 24, 2014 at 02:13:14PM +0200, Antti Koskipaa wrote:
>> +#define PIPE_A_OFFSET   0x7
>> +#define PIPE_B_OFFSET   0x71000
>> +#define PIPE_C_OFFSET   0x72000
> 
> I'd like a comment here to explain what PIPE_EDP_OFFSET
> actually means. Eg.:
> 
> /*
>  * There's actually no pipe EDP. Some pipe registers have
>  * simply shifted from the pipe to the transcoder, while
>  * keeping their original offset. Thus we need PIPE_EDP_OFFSET
>  * to access such registers in transcoder EDP.
>  */

Ok.

>>  #define TRANS_DDI_FUNC_CTL_B0x61400
>>  #define TRANS_DDI_FUNC_CTL_C0x62400
>>  #define TRANS_DDI_FUNC_CTL_EDP  0x6F400
>> -#define TRANS_DDI_FUNC_CTL(tran) _TRANSCODER(tran, TRANS_DDI_FUNC_CTL_A, \
>> -   TRANS_DDI_FUNC_CTL_B)
>> +#define TRANS_DDI_FUNC_CTL(tran) 
>> (dev_priv->info->trans_ddi_func_offsets[tran])
>> +
> 
> Why is there a separate offset for these? They're all within the
> normal transcoder range, aren't they?

Yes, they are. Will use _TRANSCODER2 for those too.

>>  #define  TRANS_DDI_FUNC_ENABLE  (1<<31)
>>  /* Those bits are ignored by pipe EDP since it can only connect to DDI A */
>>  #define  TRANS_DDI_PORT_MASK(7<<28)
>> @@ -5343,10 +5362,12 @@
>>  #define  TRANS_CLK_SEL_DISABLED (0x0<<29)
>>  #define  TRANS_CLK_SEL_PORT(x)  ((x+1)<<29)
>>  
>> -#define _TRANSA_MSA_MISC0x60410
>> -#define _TRANSB_MSA_MISC0x61410
>> -#define TRANS_MSA_MISC(tran) _TRANSCODER(tran, _TRANSA_MSA_MISC, \
>> -   _TRANSB_MSA_MISC)
>> +#define TRANSA_MSA_MISC 0x60410
>> +#define TRANSB_MSA_MISC 0x61410
>> +#define TRANSC_MSA_MISC 0x62410
>> +#define TRANS_EDP_MSA_MISC  0x6f410
>> +#define TRANS_MSA_MISC(tran) (dev_priv->info->trans_msa_misc_offsets[tran])
> 
> ditto

Ditto.

-- 
- Antti

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


Re: [Intel-gfx] [PATCH] drm/i915: Clean up display pipe register accesses

2014-01-27 Thread Antti Koskipää
Sorry, sent the wrong file. Ignore this one.

-- 
- Antti

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


Re: [Intel-gfx] [PATCH] drm/i915: Clean up display pipe register accesses

2014-01-27 Thread Antti Koskipää
On 01/27/14 13:21, Chris Wilson wrote:
> On Mon, Jan 27, 2014 at 01:17:25PM +0200, Antti Koskipaa wrote:
>> RFCv2: Reorganize array indexing so that full offsets can be used as
>> is. It makes grepping for registers in i915_reg.h much easier. Also
>> move offset arrays to intel_device_info.
>>
>> PATCHv1: Fixed offsets for VLV
>>
>> Upcoming hardware will not have the various display pipe register
>> ranges evenly spaced in memory. Change register address calculations
>> into array lookups.
>>
>> Tested on SandyBridge and Valleyview
> 
> Please also report changes in object size. If we have people complaining
> that i915.ko is too big, it helps to point why.
> -Chris

Do you want this to be reported in the commit message, or where?

bloat-o-meter says:
add/remove: 0/0 grow/shrink: 88/10 up/down: 3326/-518 (2808)

-- 
- Antti



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


Re: [Intel-gfx] [PATCH v4] drm/i915: Reorganize display pipe register accesses

2014-01-28 Thread Antti Koskipää
On 01/27/14 15:31, Ville Syrjälä wrote:
> On Mon, Jan 27, 2014 at 03:09:34PM +0200, Antti Koskipaa wrote:
>> +.dpll_offsets = { DPLL_A_OFFSET, DPLL_B_OFFSET }, \
>> +.dpll_md_offsets = { DPLL_A_MD_OFFSET, DPLL_B_MD_OFFSET }, \
>> +.palette_offsets = { PALETTE_A_OFFSET, PALETTE_B_OFFSET }, \
> 
> Please drop the last comma here ...
> 
>> +
>> +
>>  static const struct intel_device_info intel_i830_info = {
>>  .gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 2,
>>  .has_overlay = 1, .overlay_needs_physical = 1,
>>  .ring_mask = RENDER_RING,
>> +GEN_DEFAULT_PIPEOFFSETS
> 
> ... and place it here
> 

Done.

>> +/*
>> + * There's actually no pipe EDP. Some pipe registers have
>> + * simply shifted from the pipe to the transcoder, while
>> + * keeping their original offset. Thus we need PIPE_EDP_OFFSET
>> + * to access such registers in transcoder EDP.
>> + */
>> +#define PIPE_EDP_OFFSET 0x7f000
> 
> I just realized that you're not using this actually. But I think we need
> to use it to make *future* stuff work.
> 
> For the same reason PIPECONF() needs to use _PIPE2() macro, BCLRPAT needs
> to use _TRANSCODER2() macro. Ie. the choice of which we use needs to be
> based strictly on the offset range where the register lives. 
>  0x6 -> _TRANSCODER2()
>  0x7 -> _PIPE2()
> 
> PIPESRC() seems to be where it needs to be, ie. using _TRANCODER2().

Read it again. The old code has PIPECONF and BCLRPAT using the wrong
macro. This patch fixes it.

> So this also means we need the pipe_offsets[] array to have space for
> PIPE_EDP_OFFSET.

Done.

-- 
- Antti

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


Re: [Intel-gfx] [RFC] drm/i915: Clean up display pipe register accesses

2013-10-31 Thread Antti Koskipää
On 10/31/13 09:32, Jani Nikula wrote:
> On Wed, 30 Oct 2013, Antti Koskipaa  wrote:
>> Upcoming hardware will not have the various display pipe register
>> ranges evenly spaced in memory. Change register address calculations
>> into array lookups.
>>
>> Tested on SandyBridge.
>>
>> I left the UMS cruft untouched.
>>
>> Signed-off-by: Antti Koskipaa 
>> ---
>>  drivers/gpu/drm/i915/dvo_ns2501.c |  6 ++--
>>  drivers/gpu/drm/i915/i915_dma.c   | 16 +++
>>  drivers/gpu/drm/i915/i915_drv.h   | 10 ++-
>>  drivers/gpu/drm/i915/i915_reg.h   | 59 
>> +++
>>  4 files changed, 69 insertions(+), 22 deletions(-)
> 
> [snip]
> 
>> +/* Pipe timing regs */
>> +#define HTOTAL(trans) (dev_priv->trans_offsets[trans])
>> +#define HBLANK(trans) (dev_priv->trans_offsets[trans] + 0x04)
>> +#define HSYNC(trans) (dev_priv->trans_offsets[trans] + 0x08)
>> +#define VTOTAL(trans) (dev_priv->trans_offsets[trans] + 0x0c)
>> +#define VBLANK(trans) (dev_priv->trans_offsets[trans] + 0x10)
>> +#define VSYNC(trans) (dev_priv->trans_offsets[trans] + 0x14)
>> +#define BCLRPAT(trans) (dev_priv->trans_offsets[trans] + 0x20)
>> +#define VSYNCSHIFT(trans) (dev_priv->trans_offsets[trans] + 0x28)
>> +#define PIPESRC(pipe) (dev_priv->trans_offsets[pipe] + 0x1c)
> 
> This is the part that gives me the creeps about this approach, in many
> different ways. First, I don't know if we have guarantees that the
> offsets between registers remain the same; maybe they do, maybe they
> don't.

Probably they do, since it's usually the same IP block that's just
replicated n times.

> Second, I find myself searching for the register addresses in
> this file quite often. With this the "reverse lookup" becomes
> hard.

Why do you search by address? The registers have names. And because the
UMS stuff had to be left in, the original definitions w/addresses are
still there.

> Third, an unsubstanciated claim, it just *feels* like too many
> levels of indirection to be comfortable.

See below for stack usage of your approach.

> Here's an alternative approach. How about we change the defines for
> _PIPE(), _TRANSCODER(), etc. to require the the register addresses for
> *every* pipe, transcoder, etc. as parameters. It may be a bit tedious to
> change all uses of the macros, but at least it's straightforward, with,

What do you mean "change all the uses of the macros"? Nobody uses _PIPE
and _TRANSCODER directly. If they did, we'd have to change them *every
time* we added a display pipe, thus negating the whole purpose of this
patch.

> I think, fewer chances for bugs.
> 
> Here's an idea how to do it neatly:
> 
> #define _OFFSET(index, ...) (((int[]){ __VA_ARGS__ })[index])
> 
> #define _PIPE(pipe, a, b, c)  _OFFSET(pipe, a, b, c)
> #define _TRANSCODER(trans, a, b, c, edp)  _OFFSET(trans, a, b, c, edp)

This helps with your grepping for addresses, but it just hides the array
on the local variable storage area on the stack, replicated in the
machine code as many times as there are functions that call these macros.

> I did not look at the assembly produced by that vs. the table lookup.

I did, for _TRANSCODER() above:

mov DWORD PTR [rsp-24], 0x6
mov DWORD PTR [rsp-20], 0x61000
mov DWORD PTR [rsp-16], 0x63000
mov DWORD PTR [rsp-12], 0x6f000
mov eax, DWORD PTR [rsp-24+rdi*4]

> 
> BR,
> Jani.
> 
> 
> PS. And don't just go ahead and do what I suggested; more bikeshedding
> might be required! ;)
> 
> 
>> +
>> +
>>  /* Pipe A timing regs */
>>  #define _HTOTAL_A   (dev_priv->info->display_mmio_offset + 0x6)
>>  #define _HBLANK_A   (dev_priv->info->display_mmio_offset + 0x60004)
>> @@ -1941,15 +1969,6 @@
>>  #define _BCLRPAT_B  (dev_priv->info->display_mmio_offset + 0x61020)
>>  #define _VSYNCSHIFT_B   (dev_priv->info->display_mmio_offset + 0x61028)
>>  
>> -#define HTOTAL(trans) _TRANSCODER(trans, _HTOTAL_A, _HTOTAL_B)
>> -#define HBLANK(trans) _TRANSCODER(trans, _HBLANK_A, _HBLANK_B)
>> -#define HSYNC(trans) _TRANSCODER(trans, _HSYNC_A, _HSYNC_B)
>> -#define VTOTAL(trans) _TRANSCODER(trans, _VTOTAL_A, _VTOTAL_B)
>> -#define VBLANK(trans) _TRANSCODER(trans, _VBLANK_A, _VBLANK_B)
>> -#define VSYNC(trans) _TRANSCODER(trans, _VSYNC_A, _VSYNC_B)
>> -#define BCLRPAT(pipe) _PIPE(pipe, _BCLRPAT_A, _BCLRPAT_B)
>> -#define VSYNCSHIFT(trans) _TRANSCODER(trans, _VSYNCSHIFT_A, _VSYNCSHIFT_B)
>> -
>>  /* HSW eDP PSR registers */
>>  #define EDP_PSR_BASE(dev)   0x64800
>>  #define EDP_PSR_CTL(dev)(EDP_PSR_BASE(dev) + 0)
>> @@ -3211,12 +3230,16 @@
>>  #define   PIPE_VBLANK_INTERRUPT_STATUS  (1UL<<1)
>>  #define   PIPE_OVERLAY_UPDATED_STATUS   (1UL<<0)
>>  
>> -#define PIPESRC(pipe) _PIPE(pipe, _PIPEASRC, _PIPEBSRC)
>> -#define PIPECONF(tran) _TRANSCODER(tran, _PIPEACONF, _PIPEBCONF)
>> -#define PIPEDSL(pipe)  _PIPE(pipe, _PIPEADSL, _PIPEBDSL)
>> -#define PIPEFRAME(pipe) _PIPE(pipe, _PIPEAFRAMEHIGH, _PIPEBFRAMEHIGH

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Add ERR_INT to gen7 error state

2012-08-22 Thread Antti Koskipää
Both patches look ok.

Reviewed-by: Antti Koskipaa 

On 08/21/12 02:15, Ben Widawsky wrote:
> ERR_INT can generate interrupts. However since most of the conditions seem
> quite fatal the patch opts to simply report it in error state instead of
> adding more complexity to the interrupt handler for little gain (the
> bits are sticky anyway).
> 
> Signed-off-by: Ben Widawsky 
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 3 +++
>  drivers/gpu/drm/i915/i915_drv.h | 1 +
>  drivers/gpu/drm/i915/i915_irq.c | 3 +++
>  drivers/gpu/drm/i915/i915_reg.h | 1 +
>  4 files changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 0e8f14d..02405c7 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -702,6 +702,9 @@ static int i915_error_state(struct seq_file *m, void 
> *unused)
>   seq_printf(m, "DONE_REG: 0x%08x\n", error->done_reg);
>   }
>  
> + if (INTEL_INFO(dev)->gen == 7)
> + seq_printf(m, "ERR_INT: 0x%08x\n", error->err_int);
> +
>   for_each_ring(ring, dev_priv, i)
>   i915_ring_error_state(m, dev, error, i);
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 33f19eb..c61fc48 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -196,6 +196,7 @@ struct drm_i915_error_state {
>   u32 cpu_ring_head[I915_NUM_RINGS];
>   u32 cpu_ring_tail[I915_NUM_RINGS];
>   u32 error; /* gen6+ */
> + u32 err_int; /* gen7 */
>   u32 instpm[I915_NUM_RINGS];
>   u32 instps[I915_NUM_RINGS];
>   u32 instdone1;
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 0c37101..021207c 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1211,6 +1211,9 @@ static void i915_capture_error_state(struct drm_device 
> *dev)
>   error->done_reg = I915_READ(DONE_REG);
>   }
>  
> + if (INTEL_INFO(dev)->gen == 7)
> + error->err_int = I915_READ(GEN7_ERR_INT);
> +
>   i915_gem_record_fences(dev, error);
>   i915_gem_record_rings(dev, error);
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2f7b688..d4a7d73 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -507,6 +507,7 @@
>  #define DMA_FADD_I8XX0x020d0
>  
>  #define ERROR_GEN6   0x040a0
> +#define GEN7_ERR_INT 0x44040
>  
>  /* GM45+ chicken bits -- debug workaround bits that may be required
>   * for various sorts of correct behavior.  The top 16 bits of each are

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


Re: [Intel-gfx] [PATCH v2] drm/i915: fix color order for BGR formats on IVB

2012-08-22 Thread Antti Koskipää
Hi,

On 08/22/12 12:17, Vijay Purushothaman wrote:
> This is already fixed for ILK and SNB in the below commit but somehow
> IVB is missed.
> 
> commit ab2f9df10dd955f1fc0a8650e377588c98f1c029
> Author: Jesse Barnes 
> Date:   Mon Feb 27 12:40:10 2012 -0800
> 
> drm/i915: fix color order for BGR formats on SNB
> 
> Had the wrong bits and field definitions.
> 
> Signed-off-by: Vijay Purushothaman 
> Signed-off-by: Ben Lin 
> ---
>  drivers/gpu/drm/i915/intel_sprite.c |6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
> b/drivers/gpu/drm/i915/intel_sprite.c
> index cc8df4d..6045a01 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -54,17 +54,17 @@ ivb_update_plane(struct drm_plane *plane, struct 
> drm_framebuffer *fb,
>  
>   /* Mask out pixel format bits in case we change it */
>   sprctl &= ~SPRITE_PIXFORMAT_MASK;
> - sprctl &= ~SPRITE_RGB_ORDER_RGBX;
> + sprctl &= ~SPRITE_RGB_ORDER_XBGR;
Are you sure about this? Where is the #define for _XBGR?

>   sprctl &= ~SPRITE_YUV_BYTE_ORDER_MASK;
>   sprctl &= ~SPRITE_TILED;
>  
>   switch (fb->pixel_format) {
>   case DRM_FORMAT_XBGR:
> - sprctl |= SPRITE_FORMAT_RGBX888;
> + sprctl |= SPRITE_FORMAT_RGBX888 | SPRITE_RGB_ORDER_RGBX;
>   pixel_size = 4;
>   break;
>   case DRM_FORMAT_XRGB:
> - sprctl |= SPRITE_FORMAT_RGBX888 | SPRITE_RGB_ORDER_RGBX;
> + sprctl |= SPRITE_FORMAT_RGBX888;
>   pixel_size = 4;
>   break;
>   case DRM_FORMAT_YUYV:

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


Re: [Intel-gfx] [PATCH 4/8] drm/i915: implement WaDisableVLVClockGating_VBIIssue on VLV

2012-11-01 Thread Antti Koskipää
Hi,

On 10/25/12 22:15, Jesse Barnes wrote:
> This allows us to get the right vblank interrupt frequency.
> 
> v2: pull in register definition
> 
> Signed-off-by: Jesse Barnes 
> ---
>  drivers/gpu/drm/i915/i915_reg.h |2 ++
>  drivers/gpu/drm/i915/intel_pm.c |7 +++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 6464eaa..4aec0a3 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -556,6 +556,8 @@
>  #define IIR  0x020a4
>  #define IMR  0x020a8
>  #define ISR  0x020ac
> +#define VLV_GUNIT_CLOCK_GATE 0x182060

Where did you pull this offset from? It's not in the Bspec.


> +#define   GCFG_DIS   (1<<8)
>  #define VLV_IIR_RW   0x182084
>  #define VLV_IER  0x1820a0
>  #define VLV_IIR  0x1820a4
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d04e87f..88c154c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3713,6 +3713,13 @@ static void valleyview_init_clock_gating(struct 
> drm_device *dev)
>  PIPEA_HLINE_INT_EN | PIPEA_VBLANK_INT_EN |
>  SPRITEB_FLIPDONE_INT_EN | SPRITEA_FLIPDONE_INT_EN |
>  PLANEA_FLIPDONE_INT_EN);
> +
> + /*
> +  * WaDisableVLVClockGating_VBIIssue
> +  * Disable clock gating on th GCFG unit to prevent a delay
> +  * in the reporting of vblank events.
> +  */
> + I915_WRITE(VLV_GUNIT_CLOCK_GATE, GCFG_DIS);
>  }
>  
>  static void g4x_init_clock_gating(struct drm_device *dev)

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


Re: [Intel-gfx] [PATCH 4/8] drm/i915: implement WaDisableVLVClockGating_VBIIssue on VLV

2012-11-01 Thread Antti Koskipää
On 11/01/12 16:50, Jesse Barnes wrote:
> No, it's in the gunit spec.  I'm still working on getting that one
> opened up.

In that case, for the whole lot:
Reviewed-by: Antti Koskipää 

-- 
- Antti




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


Re: [Intel-gfx] how to disable dpst on linux?

2012-11-22 Thread Antti Koskipää
On 11/20/12 20:24, Роман Мельник wrote:
> Good day!
> 
> I've sent already mail, but got no response, so trying again.
> Please advise how to disable dpst on linux? The module is i915. As I
> see, windows driver has such checkbox for this. Can I do the same on linux?

The linux driver does not support DPST at all.

-- 
- Antti

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


Re: [Intel-gfx] how to disable dpst on linux?

2012-11-22 Thread Antti Koskipää
On 11/22/12 16:36, РоманМельник wrote:
> Antti Koskipää  linux.intel.com> writes:
> 
>>
>> On 11/20/12 20:24, Роман Мельник wrote:
>>> Good day!
>>>
>>> I've sent already mail, but got no response, so trying again.
>>> Please advise how to disable dpst on linux? The module is i915. As I
>>> see, windows driver has such checkbox for this. Can I do the same on linux?
>>
>> The linux driver does not support DPST at all.
>>
> 
> But I definitely see that when the laptop works from battery, it changes
> dynamically the contrast of the screen depends on what the summary brightness 
> of
> the screen (the more dark pixels - the more screen has less contrast). If this
> is not DPST, so that what can it be and how to disable this? Thanks!!

It could be CABC (Content Adaptive Backlight Control) which is a similar
technology but is implemented fully in hardware, in the panel. How you
can disable (and IF you can disable it) depends completely on the hardware.

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


[Intel-gfx] Bug team status

2012-12-21 Thread Antti Koskipää
Another bug team week gone...

Antti:

Went through bugzilla, checking that work is still being done, general
stuff.

Mika:

Fixed https://bugs.freedesktop.org/show_bug.cgi?id=58230

Tried to verify a fix (drm/i915: disable shrinker lock
stealing for create_mmap_offset) for a problem i reported
but didn't manage to get ./gem_tiled_swapping to break anymore on
top of clean of clean drm-intel-fixes.

Rodrigo:

Worked on https://bugs.freedesktop.org/show_bug.cgi?id=51146
and https://bugs.freedesktop.org/show_bug.cgi?id=54964 and may have
found a VGA workaround for the latter one.


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


Re: [Intel-gfx] [PATCH 01/49] drm/i915/bxt: Add BXT PCI ids

2015-03-23 Thread Antti Koskipää
Reviewed-by: Antti Koskipää 

On 03/17/2015 11:39 AM, Imre Deak wrote:
> From: Damien Lespiau 
> 
> v2: Switch to info->ring_mask and add VEBOX support.
> v3: Fold in update from Damien.
> v4: Add GEN_DEFAULT_PIPEOFFSETS and IVB_CURSOR_OFFSETS
> v5: set no-LLC (imre)
> 
> Signed-off-by: Damien Lespiau  (v1,v4)
> Signed-off-by: Daniel Vetter  (v4)
> Signed-off-by: Imre Deak 
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 14 +-
>  include/drm/i915_pciids.h   |  6 ++
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 82f8be4..4d50785 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -381,6 +381,17 @@ static const struct intel_device_info 
> intel_skylake_gt3_info = {
>   IVB_CURSOR_OFFSETS,
>  };
>  
> +static const struct intel_device_info intel_broxton_info = {
> + .is_preliminary = 1,
> + .gen = 9,
> + .need_gfx_hws = 1, .has_hotplug = 1,
> + .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> + .num_pipes = 3,
> + .has_ddi = 1,
> + GEN_DEFAULT_PIPEOFFSETS,
> + IVB_CURSOR_OFFSETS,
> +};
> +
>  /*
>   * Make sure any device matches here are from most specific to most
>   * general.  For example, since the Quanta match is based on the subsystem
> @@ -420,7 +431,8 @@ static const struct intel_device_info 
> intel_skylake_gt3_info = {
>   INTEL_CHV_IDS(&intel_cherryview_info),  \
>   INTEL_SKL_GT1_IDS(&intel_skylake_info), \
>   INTEL_SKL_GT2_IDS(&intel_skylake_info), \
> - INTEL_SKL_GT3_IDS(&intel_skylake_gt3_info)  \
> + INTEL_SKL_GT3_IDS(&intel_skylake_gt3_info), \
> + INTEL_BXT_IDS(&intel_broxton_info)
>  
>  static const struct pci_device_id pciidlist[] = {/* aka */
>   INTEL_PCI_IDS,
> diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> index 6133723..bd0d644 100644
> --- a/include/drm/i915_pciids.h
> +++ b/include/drm/i915_pciids.h
> @@ -287,4 +287,10 @@
>   INTEL_SKL_GT3_IDS(info)
>  
>  
> +#define INTEL_BXT_IDS(info) \
> + INTEL_VGA_DEVICE(0x0A84, info), \
> + INTEL_VGA_DEVICE(0x0A85, info), \
> + INTEL_VGA_DEVICE(0x0A86, info), \
> + INTEL_VGA_DEVICE(0x0A87, info)
> +
>  #endif /* _I915_PCIIDS_H */
> 

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


Re: [Intel-gfx] [PATCH 05/49] drm/i915/bxt: Enable PTE encoding

2015-03-23 Thread Antti Koskipää
Reviewed-by: Antti Koskipää 

On 03/17/2015 11:39 AM, Imre Deak wrote:
> From: Sumit Singh 
> 
> The caching options for page table entries have remained the same as
> Cherryview. This patch fixes it so the right code path is taken on BXT.
> 
> v2: Fix up commit message (Mike)
> 
> Signed-off-by: Sumit Singh 
> Signed-off-by: Damien Lespiau 
> ---
>  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 f1b9ea6..4311292 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1506,7 +1506,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device 
> *dev)
>  
>  
>   if (INTEL_INFO(dev)->gen >= 8) {
> - if (IS_CHERRYVIEW(dev))
> + if (IS_CHERRYVIEW(dev) || IS_BROXTON(dev))
>   chv_setup_private_ppat(dev_priv);
>   else
>   bdw_setup_private_ppat(dev_priv);
> @@ -2187,7 +2187,7 @@ static int gen8_gmch_probe(struct drm_device *dev,
>  
>   *gtt_total = (gtt_size / sizeof(gen8_gtt_pte_t)) << PAGE_SHIFT;
>  
> - if (IS_CHERRYVIEW(dev))
> + if (IS_CHERRYVIEW(dev) || IS_BROXTON(dev))
>   chv_setup_private_ppat(dev_priv);
>   else
>   bdw_setup_private_ppat(dev_priv);
> 

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


Re: [Intel-gfx] [PATCH 04/49] drm/i915/bxt: Broxton uses the same GMS values as Skylake

2015-03-23 Thread Antti Koskipää
Reviewed-by: Antti Koskipää 

On 03/17/2015 11:39 AM, Imre Deak wrote:
> From: Damien Lespiau 
> 
> v2: Rebase on top of the early-quirks rework from Ville.
> 
> Signed-off-by: Damien Lespiau  (v1)
> Signed-off-by: Daniel Vetter 
> ---
>  arch/x86/kernel/early-quirks.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
> index fe9f0b7..ab470e4 100644
> --- a/arch/x86/kernel/early-quirks.c
> +++ b/arch/x86/kernel/early-quirks.c
> @@ -546,6 +546,7 @@ static const struct pci_device_id intel_stolen_ids[] 
> __initconst = {
>   INTEL_BDW_D_IDS(&gen8_stolen_funcs),
>   INTEL_CHV_IDS(&chv_stolen_funcs),
>   INTEL_SKL_IDS(&gen9_stolen_funcs),
> + INTEL_BXT_IDS(&gen9_stolen_funcs),
>  };
>  
>  static void __init intel_graphics_stolen(int num, int slot, int func)
> 

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


Re: [Intel-gfx] [PATCH 08/49] drm/i915/bxt: Broxton DDB is 512 blocks

2015-03-23 Thread Antti Koskipää
Reviewed-by: Antti Koskipää 

On 03/17/2015 11:39 AM, Imre Deak wrote:
> From: Damien Lespiau 
> 
> Signed-off-by: Damien Lespiau 
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 288c9d2..b89ab4d 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2538,6 +2538,7 @@ static bool ilk_disable_lp_wm(struct drm_device *dev)
>   */
>  
>  #define SKL_DDB_SIZE 896 /* in blocks */
> +#define BXT_DDB_SIZE 512
>  
>  static void
>  skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
> @@ -2556,7 +2557,10 @@ skl_ddb_get_pipe_allocation_limits(struct drm_device 
> *dev,
>   return;
>   }
>  
> - ddb_size = SKL_DDB_SIZE;
> + if (IS_BROXTON(dev))
> + ddb_size = BXT_DDB_SIZE;
> + else
> + ddb_size = SKL_DDB_SIZE;
>  
>   ddb_size -= 4; /* 4 blocks for bypass path allocation */
>  
> 

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


Re: [Intel-gfx] [PATCH 09/49] drm/i915/bxt: Broxton raises the maximum number of planes to 4

2015-03-23 Thread Antti Koskipää
Reviewed-by: Antti Koskipää 

On 03/17/2015 11:39 AM, Imre Deak wrote:
> From: Damien Lespiau 
> 
> Pipe A and b have 4 planes.
> 
> Signed-off-by: Damien Lespiau 
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index eba53c3..8fb7cc0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -130,7 +130,7 @@ enum transcoder {
>   *
>   * This value doesn't count the cursor plane.
>   */
> -#define I915_MAX_PLANES  3
> +#define I915_MAX_PLANES  4
>  
>  enum plane {
>   PLANE_A = 0,
> 

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


Re: [Intel-gfx] [PATCH 07/49] drm/i915/bxt: Add the plane4 related interrupt definitions

2015-03-23 Thread Antti Koskipää
Reviewed-by: Antti Koskipää 

On 03/17/2015 11:39 AM, Imre Deak wrote:
> From: Damien Lespiau 
> 
> Signed-off-by: Damien Lespiau 
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index cc8ebab..3369a11 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5216,9 +5216,11 @@ enum skl_disp_power_wells {
>  #define  GEN8_PIPE_VSYNC (1 << 1)
>  #define  GEN8_PIPE_VBLANK(1 << 0)
>  #define  GEN9_PIPE_CURSOR_FAULT  (1 << 11)
> +#define  GEN9_PIPE_PLANE4_FAULT  (1 << 10)
>  #define  GEN9_PIPE_PLANE3_FAULT  (1 << 9)
>  #define  GEN9_PIPE_PLANE2_FAULT  (1 << 8)
>  #define  GEN9_PIPE_PLANE1_FAULT  (1 << 7)
> +#define  GEN9_PIPE_PLANE4_FLIP_DONE  (1 << 6)
>  #define  GEN9_PIPE_PLANE3_FLIP_DONE  (1 << 5)
>  #define  GEN9_PIPE_PLANE2_FLIP_DONE  (1 << 4)
>  #define  GEN9_PIPE_PLANE1_FLIP_DONE  (1 << 3)
> @@ -5229,6 +5231,7 @@ enum skl_disp_power_wells {
>GEN8_PIPE_PRIMARY_FAULT)
>  #define GEN9_DE_PIPE_IRQ_FAULT_ERRORS \
>   (GEN9_PIPE_CURSOR_FAULT | \
> +  GEN9_PIPE_PLANE4_FAULT | \
>GEN9_PIPE_PLANE3_FAULT | \
>GEN9_PIPE_PLANE2_FAULT | \
>GEN9_PIPE_PLANE1_FAULT)
> 

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


Re: [Intel-gfx] [PATCH 06/49] drm/i915/bxt: Broxton has 3 sprite planes on pipe A/B, 2 on pipe C

2015-03-23 Thread Antti Koskipää
Reviewed-by: Antti Koskipää 

On 03/17/2015 11:39 AM, Imre Deak wrote:
> From: Damien Lespiau 
> 
> v2: Rebase on top of the for_each_pipe() change adding dev_priv as first
> argument.
> 
> Signed-off-by: Damien Lespiau 
> ---
>  drivers/gpu/drm/i915/i915_dma.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index d49ed68..a94a970 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -585,7 +585,11 @@ static void intel_device_info_runtime_init(struct 
> drm_device *dev)
>  
>   info = (struct intel_device_info *)&dev_priv->info;
>  
> - if (IS_VALLEYVIEW(dev) || INTEL_INFO(dev)->gen == 9)
> + if (IS_BROXTON(dev)) {
> + info->num_sprites[PIPE_A] = 3;
> + info->num_sprites[PIPE_B] = 3;
> + info->num_sprites[PIPE_C] = 2;
> + } else if (IS_VALLEYVIEW(dev) || INTEL_INFO(dev)->gen == 9)
>   for_each_pipe(dev_priv, pipe)
>   info->num_sprites[pipe] = 2;
>   else
> 

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


Re: [Intel-gfx] [PATCH v2] drm/i915/bxt: map GTT as uncached

2015-03-30 Thread Antti Koskipää
Reviewed-by: Antti Koskipää 

On 03/27/2015 01:07 PM, Imre Deak wrote:
> On Broxton per specification the GTT has to be mapped as uncached.
> This was caught by the PTE write readback warning, which showed a
> corrupted PTE value with using the current write-combine mapping.
> 
> v2:
> - add comment explaining how the problem with WC mapping manifests
>   (Daniel)
> 
> Signed-off-by: Imre Deak 
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 12 +++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index e33b121..4d15237 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2256,7 +2256,17 @@ static int ggtt_probe_common(struct drm_device *dev,
>   gtt_phys_addr = pci_resource_start(dev->pdev, 0) +
>   (pci_resource_len(dev->pdev, 0) / 2);
>  
> - dev_priv->gtt.gsm = ioremap_wc(gtt_phys_addr, gtt_size);
> + /*
> +  * On BXT writes larger than 64 bit to the GTT pagetable range will be
> +  * dropped. For WC mappings in general we have 64 byte burst writes
> +  * when the WC buffer is flushed, so we can't use it, but have to
> +  * resort to an uncached mapping. The WC issue is easily caught by the
> +  * readback check when writing GTT PTE entries.
> +  */
> + if (IS_BROXTON(dev))
> + dev_priv->gtt.gsm = ioremap_nocache(gtt_phys_addr, gtt_size);
> + else
> + dev_priv->gtt.gsm = ioremap_wc(gtt_phys_addr, gtt_size);
>   if (!dev_priv->gtt.gsm) {
>   DRM_ERROR("Failed to map the gtt page table\n");
>   return -ENOMEM;
> 

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


Re: [Intel-gfx] [PATCH 02/49] drm/i915/bxt: BXT FBC enablement

2015-03-30 Thread Antti Koskipää
Reviewed-by: Antti Koskipää 

On 03/17/2015 11:39 AM, Imre Deak wrote:
> From: Daisy Sun 
> 
> Enable FBC feature on Broxton
> 
> Issue: VIZ-3784
> Signed-off-by: Daisy Sun 
> Signed-off-by: Damien Lespiau 
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 4d50785..48434cb6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -388,6 +388,7 @@ static const struct intel_device_info intel_broxton_info 
> = {
>   .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
>   .num_pipes = 3,
>   .has_ddi = 1,
> + .has_fbc = 1,
>   GEN_DEFAULT_PIPEOFFSETS,
>   IVB_CURSOR_OFFSETS,
>  };
> 

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


Re: [Intel-gfx] [PATCH 02/49] drm/i915/bxt: BXT FBC enablement

2015-03-30 Thread Antti Koskipää
Reviewed-by: Antti Koskipää 

On 03/17/2015 11:39 AM, Imre Deak wrote:
> From: Daisy Sun 
> 
> Enable FBC feature on Broxton
> 
> Issue: VIZ-3784
> Signed-off-by: Daisy Sun 
> Signed-off-by: Damien Lespiau 
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 4d50785..48434cb6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -388,6 +388,7 @@ static const struct intel_device_info intel_broxton_info 
> = {
>   .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
>   .num_pipes = 3,
>   .has_ddi = 1,
> + .has_fbc = 1,
>   GEN_DEFAULT_PIPEOFFSETS,
>   IVB_CURSOR_OFFSETS,
>  };
> 

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


Re: [Intel-gfx] [PATCH 02.1/49] drm/i915: use proper FBC base register on all new platforms

2015-03-30 Thread Antti Koskipää
Reviewed-by: Antti Koskipää 

On 03/26/2015 05:35 PM, Imre Deak wrote:
> Starting from GEN5 the FBC base register is the same on all platforms.
> GEN>=5 is the same condition as HAS_PCH_SPLIT except on BXT, so make
> things work on BXT as well.
> 
> Motivated by Rodrigo's request to check FBC support on BXT.
> 
> Signed-off-by: Imre Deak 
> ---
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
> b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index f8da716..348ed5a 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -209,7 +209,7 @@ static int i915_setup_compression(struct drm_device *dev, 
> int size, int fb_cpp)
>  
>   dev_priv->fbc.threshold = ret;
>  
> - if (HAS_PCH_SPLIT(dev))
> + if (INTEL_INFO(dev_priv)->gen >= 5)
>   I915_WRITE(ILK_DPFC_CB_BASE, dev_priv->fbc.compressed_fb.start);
>   else if (IS_GM45(dev)) {
>   I915_WRITE(DPFC_CB_BASE, dev_priv->fbc.compressed_fb.start);
> 

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


Re: [Intel-gfx] [PATCH] drm/i915: Per-DDI I_boost override

2015-06-18 Thread Antti Koskipää
Just FYI, this patch depends on David Weinehall's Buffer translation
improvements patch from earlier today.

-- 
- Antti

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


Re: [Intel-gfx] [PATCH v2] drm/i915/skl: Buffer translation improvements

2015-06-29 Thread Antti Koskipää
Looks fine to me.

Reviewed-by: Antti Koskipää 


On 06/25/2015 11:11 AM, David Weinehall wrote:
> This patch adds support for 0.85V VccIO on Skylake Y,
> separate buffer translation tables for Skylake U,
> and support for I_boost for the entries that needs this.
> 
> Changes in v2:
> * Refactored the code a bit to move all DDI signal level setup to
>   intel_ddi.c
> 
> Issue: VIZ-5677
> Signed-off-by: David Weinehall 
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |   8 +
>  drivers/gpu/drm/i915/i915_reg.h  |  12 +
>  drivers/gpu/drm/i915/intel_ddi.c | 507 
> ++-
>  drivers/gpu/drm/i915/intel_dp.c  | 104 +---
>  drivers/gpu/drm/i915/intel_drv.h |   3 +-
>  5 files changed, 421 insertions(+), 213 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 491ef0cfcb0b..09a57a584f5f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2415,6 +2415,14 @@ struct drm_i915_cmd_table {
>  /* ULX machines are also considered ULT. */
>  #define IS_HSW_ULX(dev)  (INTEL_DEVID(dev) == 0x0A0E || \
>INTEL_DEVID(dev) == 0x0A1E)
> +#define IS_SKL_ULT(dev)  (INTEL_DEVID(dev) == 0x1906 || \
> +  INTEL_DEVID(dev) == 0x1913 || \
> +  INTEL_DEVID(dev) == 0x1916 || \
> +  INTEL_DEVID(dev) == 0x1921 || \
> +  INTEL_DEVID(dev) == 0x1926)
> +#define IS_SKL_ULX(dev)  (INTEL_DEVID(dev) == 0x190E || \
> +  INTEL_DEVID(dev) == 0x1915 || \
> +  INTEL_DEVID(dev) == 0x191E)
>  #define IS_PRELIMINARY_HW(intel_info) ((intel_info)->is_preliminary)
>  
>  #define SKL_REVID_A0 (0x0)
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0b979ad16d41..fb63ead2b8eb 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1382,6 +1382,18 @@ enum skl_disp_power_wells {
>   _PORT_TX_DW14_LN0_C) + \
>_BXT_LANE_OFFSET(lane))
>  
> +/* UAIMI scratch pad register 1 */
> +#define UAIMI_SPR1   0x4F074
> +/* SKL VccIO mask */
> +#define SKL_VCCIO_MASK   0x1
> +/* SKL balance leg register */
> +#define DISPIO_CR_TX_BMU_CR0 0x6C00C
> +/* I_boost values */
> +#define BALANCE_LEG_SHIFT(port)  (8+3*(port))
> +#define BALANCE_LEG_MASK(port)   (7<<(8+3*(port)))
> +/* Balance leg disable bits */
> +#define BALANCE_LEG_DISABLE_SHIFT23
> +
>  /*
>   * Fence registers
>   */
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 31b29e8781ac..08f89299b572 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -31,6 +31,7 @@
>  struct ddi_buf_trans {
>   u32 trans1; /* balance leg enable, de-emph level */
>   u32 trans2; /* vref sel, vswing */
> + u8 i_boost; /* SKL: I_boost; valid: 0x0, 0x1, 0x3, 0x7 */
>  };
>  
>  /* HDMI/DVI modes ignore everything but the last 2 items. So we share
> @@ -38,134 +39,213 @@ struct ddi_buf_trans {
>   * automatically adapt to HDMI connections as well
>   */
>  static const struct ddi_buf_trans hsw_ddi_translations_dp[] = {
> - { 0x00FF, 0x0006000E },
> - { 0x00D75FFF, 0x0005000A },
> - { 0x00C30FFF, 0x00040006 },
> - { 0x80AAAFFF, 0x000B },
> - { 0x00FF, 0x0005000A },
> - { 0x00D75FFF, 0x000C0004 },
> - { 0x80C30FFF, 0x000B },
> - { 0x00FF, 0x00040006 },
> - { 0x80D75FFF, 0x000B },
> + { 0x00FF, 0x0006000E, 0x0 },
> + { 0x00D75FFF, 0x0005000A, 0x0 },
> + { 0x00C30FFF, 0x00040006, 0x0 },
> + { 0x80AAAFFF, 0x000B, 0x0 },
> + { 0x00FF, 0x0005000A, 0x0 },
> + { 0x00D75FFF, 0x000C0004, 0x0 },
> + { 0x80C30FFF, 0x000B, 0x0 },
> + { 0x00FF, 0x00040006, 0x0 },
> + { 0x80D75FFF, 0x000B, 0x0 },
>  };
>  
>  static const struct ddi_buf_trans hsw_ddi_translations_fdi[] = {
> - { 0x00FF, 0x0007000E },
> - { 0x00D75FFF, 0x000F000A },
> - { 0x00C30FFF, 0x00060006 },
> - { 0x00AAAFFF, 0x001E },
> - { 0x00FF, 0x000F000A },
> - { 0x00D75FFF, 0x00160004 },
> - { 0x00C30FFF, 0x001E },
> - { 0x00FF, 0x00060006 },
> - { 0x00D75FFF, 0x001E },
> + { 0x00FF, 0x0007000E, 0x0 },
> + { 0x00D75FFF, 0x000F000A, 0x0 },
> + { 0x00C30

Re: [Intel-gfx] [PATCH v2] drm/i915: Per-DDI I_boost override

2015-07-03 Thread Antti Koskipää
On 07/03/2015 06:09 PM, Paulo Zanoni wrote:
> 2015-07-03 8:28 GMT-03:00 Antti Koskipaa :
>> An OEM may request increased I_boost beyond the recommended values
>> by specifying an I_boost value to be applied to all swing entries for
>> a port. These override values are specified in VBT.
>>
>> v2: rebase and remove unused iboost_bit variable
>>
>> Issue: VIZ-5676
>> Signed-off-by: Antti Koskipaa 
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h   |  3 +++
>>  drivers/gpu/drm/i915/intel_bios.c | 21 +
>>  drivers/gpu/drm/i915/intel_bios.h |  9 +
>>  drivers/gpu/drm/i915/intel_ddi.c  | 38 
>> ++
>>  4 files changed, 63 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h 
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index 1dbd957..6aa8083 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1410,6 +1410,9 @@ struct ddi_vbt_port_info {
>> uint8_t supports_dvi:1;
>> uint8_t supports_hdmi:1;
>> uint8_t supports_dp:1;
>> +
>> +   uint8_t dp_boost_level;
>> +   uint8_t hdmi_boost_level;
>>  };
>>
>>  enum psr_lines_to_wait {
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c 
>> b/drivers/gpu/drm/i915/intel_bios.c
>> index 2ff9eb0..76e12f5 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -886,6 +886,17 @@ err:
>> memset(dev_priv->vbt.dsi.sequence, 0, 
>> sizeof(dev_priv->vbt.dsi.sequence));
>>  }
>>
>> +static u8 translate_iboost(u8 val)
>> +{
>> +   static const u8 mapping[] = { 1, 3, 7 }; /* See VBT spec */
>> +
>> +   if (val >= ARRAY_SIZE(mapping)) {
>> +   DRM_DEBUG_KMS("Unsupported I_boost value found in VBT (%d), 
>> display may not work properly\n", val);
>> +   return 0;
>> +   }
>> +   return mapping[val];
>> +}
>> +
>>  static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port 
>> port,
>>const struct bdb_header *bdb)
>>  {
>> @@ -986,6 +997,16 @@ static void parse_ddi_port(struct drm_i915_private 
>> *dev_priv, enum port port,
>>   hdmi_level_shift);
>> info->hdmi_level_shift = hdmi_level_shift;
>> }
>> +
>> +   /* Parse the I_boost config for SKL and above */
>> +   if (bdb->version >= 196 && (child->common.flags_1 & IBOOST_ENABLE)) {
>> +   info->dp_boost_level = 
>> translate_iboost(child->common.iboost_level & 0xF);
>> +   DRM_DEBUG_KMS("VBT (e)DP boost level for port %c: %d\n",
>> + port_name(port), info->dp_boost_level);
>> +   info->hdmi_boost_level = 
>> translate_iboost(child->common.iboost_level >> 4);
>> +   DRM_DEBUG_KMS("VBT HDMI boost level for port %c: %d\n",
>> + port_name(port), info->hdmi_boost_level);
>> +   }
>>  }
>>
>>  static void parse_ddi_ports(struct drm_i915_private *dev_priv,
>> diff --git a/drivers/gpu/drm/i915/intel_bios.h 
>> b/drivers/gpu/drm/i915/intel_bios.h
>> index af0b476..8edd75c 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.h
>> +++ b/drivers/gpu/drm/i915/intel_bios.h
>> @@ -231,6 +231,10 @@ struct old_child_dev_config {
>>  /* This one contains field offsets that are known to be common for all BDB
>>   * versions. Notice that the meaning of the contents contents may still 
>> change,
>>   * but at least the offsets are consistent. */
>> +
>> +/* Definitions for flags_1 */
>> +#define IBOOST_ENABLE (1<<3)
>> +
>>  struct common_child_dev_config {
>> u16 handle;
>> u16 device_type;
>> @@ -239,8 +243,13 @@ struct common_child_dev_config {
>> u8 not_common2[2];
>> u8 ddc_pin;
>> u16 edid_ptr;
>> +   u8 obsolete;
>> +   u8 flags_1;
>> +   u8 not_common3[13];
>> +   u8 iboost_level;
> 
> In the old VBT spec I have, each child_dev_config is supposed to have
> only 33 bytes. But in this patch you're increasing it to 38. I believe
> this is what's causing the errors I see when I boot my BDW.
> 
> Are you sure they increased the VBT's ChildDevInfo to more than 33
> bytes? I don't have access to your VBT spec right now, so I can't do a
> proper review or a suggestion on how to fix the problem.

The VBT spec I coded for does increase the size. You need to look at
version >= 196 of the spec.

>>  } __packed;
>>
>> +
>>  /* This field changes depending on the BDB version, so the most reliable 
>> way to
>>   * read it is by checking the BDB version and reading the raw pointer. */
>>  union child_device_config {
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
>> b/drivers/gpu/drm/i915/intel_ddi.c
>> index 15fc66a..c83f15f 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -440,6 +440,7 @@ static void intel_prepare_ddi_buffers(struct drm_device 
>> *dev, enum port port,
>>  {
>> struct drm_i915_private *dev_priv = 

Re: [Intel-gfx] [PATCH i-g-t] tests/pm_backlight: Add backlight test

2015-05-27 Thread Antti Koskipää
On 05/26/2015 02:37 PM, Jani Nikula wrote:
> On Sat, 23 May 2015, Antti Koskipaa  wrote:
>> This is a basic sanity test of the backlight sysfs interface.
>>
>> Issue: VIZ-3377
>> Signed-off-by: Antti Koskipaa 
>> ---
>>  tests/.gitignore   |   1 +
>>  tests/Makefile.sources |   1 +
>>  tests/pm_backlight.c   | 144 
>> +
>>  3 files changed, 146 insertions(+)
>>  create mode 100644 tests/pm_backlight.c
>>
>> diff --git a/tests/.gitignore b/tests/.gitignore
>> index a3f3143..f816ded 100644
>> --- a/tests/.gitignore
>> +++ b/tests/.gitignore
>> @@ -150,6 +150,7 @@ kms_vblank
>>  kms_crtc_background_color
>>  kms_plane_scaling
>>  kms_panel_fitting
>> +pm_backlight
>>  pm_lpsp
>>  pm_rc6_residency
>>  pm_rpm
>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
>> index 994c31b..d2a44e8 100644
>> --- a/tests/Makefile.sources
>> +++ b/tests/Makefile.sources
>> @@ -80,6 +80,7 @@ TESTS_progs_M = \
>>  kms_crtc_background_color \
>>  kms_plane_scaling \
>>  kms_panel_fitting \
>> +pm_backlight \
>>  pm_lpsp \
>>  pm_rpm \
>>  pm_rps \
>> diff --git a/tests/pm_backlight.c b/tests/pm_backlight.c
>> new file mode 100644
>> index 000..eb2dcf5
>> --- /dev/null
>> +++ b/tests/pm_backlight.c
>> @@ -0,0 +1,144 @@
>> +/*
>> + * Copyright © 2015 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the 
>> "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the next
>> + * paragraph) shall be included in all copies or substantial portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
>> OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
>> OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
>> DEALINGS
>> + * IN THE SOFTWARE.
>> + *
>> + * Author:
>> + *Antti Koskipaa 
>> + *
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include "igt_core.h"
>> +
>> +#define TOLERANCE 5 /* percent */
>> +#define BACKLIGHT_PATH "/sys/class/backlight/intel_backlight"
>> +
>> +IGT_TEST_DESCRIPTION("Basic backlight sysfs test");
>> +
>> +static int backlight_read(int *result, const char *fname)
>> +{
>> +int fd;
>> +char full[PATH_MAX];
>> +char dst[64];
>> +int r, e;
>> +
>> +igt_assert(snprintf(full, PATH_MAX, "%s/%s", BACKLIGHT_PATH, fname) < 
>> PATH_MAX);
>> +
>> +fd = open(full, O_RDONLY);
>> +if (fd == -1)
>> +return -errno;
>> +
>> +r = read(fd, dst, sizeof(dst));
>> +e = errno;
>> +close(fd);
>> +
>> +if (r < 0)
>> +return -e;
>> +
>> +errno = 0;
>> +*result = strtol(dst, NULL, 10);
>> +return errno;
>> +}
>> +
>> +static int backlight_write(int value, const char *fname)
>> +{
>> +int fd;
>> +char full[PATH_MAX];
>> +char src[64];
>> +int len;
>> +
>> +igt_assert(snprintf(full, PATH_MAX, "%s/%s", BACKLIGHT_PATH, fname) < 
>> PATH_MAX);
>> +fd = open(full, O_WRONLY);
>> +if (fd == -1)
>> +return -errno;
>> +
>> +len = snprintf(src, sizeof(src), "%i", value);
>> +len = write(fd, src, len);
>> +close(fd);
>> +
>> +if (len < 0)
>> +return len;
>> +
>> +return 0;
>> +}
>> +
>> +static void test_and_verify(int val, int max)
>> +{
>> +int result;
>> +igt_assert(backlight_write(val, "brightness") == 0);
>> +igt_assert(backlight_read(&result, "actual_brightness") == 0);
>> +/* Some rounding may happen depending on hw. Just check that it's close 
>> enough. */
>> +igt_assert(result <= val + val * TOLERANCE / 100 && result >= val - val 
>> * TOLERANCE / 100);
> 
> This could additionally read "brightness" and assert it is equal to what
> was just written (i.e. without the tolerance check).

v2 coming soon.

> There are no guarantees that the result read back from
> "actual_brightness" would be within the tolerance, in fact it could be
> changed by ACPI behind our backs, at least on some platforms... But I
> guess this is good enough.

It should be, unless the tester does something stupid like slam the lid
shut in the middle of the test or something.

>> +}
>> +
>> +static void test_brightness

Re: [Intel-gfx] [PATCH 10/14] drm/i915: Track which port is using which pipe's power sequencer

2014-09-01 Thread Antti Koskipää
Reviewed-by: Antti Koskipaa 

-- 
- Antti

On 08/18/2014 10:16 PM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> VLV/CHV have a per-pipe panel power sequencer which locks onto the
> port once used. We need to keep track wich power sequencers are
> locked to which ports.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 185 
> ++-
>  drivers/gpu/drm/i915/intel_drv.h |   6 ++
>  2 files changed, 168 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 556e4de..4614e6e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -294,28 +294,99 @@ static enum pipe
>  vlv_power_sequencer_pipe(struct intel_dp *intel_dp)
>  {
>   struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> - struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
>   struct drm_device *dev = intel_dig_port->base.base.dev;
>   struct drm_i915_private *dev_priv = dev->dev_private;
> - enum port port = intel_dig_port->port;
> - enum pipe pipe;
> + struct intel_encoder *encoder;
> + unsigned int pipes = (1 << PIPE_A) | (1 << PIPE_B);
> + struct edp_power_seq power_seq;
>  
>   lockdep_assert_held(&dev_priv->pps_mutex);
>  
> - /* modeset should have pipe */
> - if (crtc)
> - return to_intel_crtc(crtc)->pipe;
> + if (intel_dp->pps_pipe != INVALID_PIPE)
> + return intel_dp->pps_pipe;
> +
> + /*
> +  * We don't have power sequencer currently.
> +  * Pick one that's not used by other ports.
> +  */
> + list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) 
> {
> + struct intel_dp *tmp;
> +
> + if (encoder->type != INTEL_OUTPUT_EDP)
> + continue;
> +
> + tmp = enc_to_intel_dp(&encoder->base);
> +
> + if (tmp->pps_pipe != INVALID_PIPE)
> + pipes &= ~(1 << tmp->pps_pipe);
> + }
> +
> + /*
> +  * Didn't find one. This should not happen since there
> +  * are two power sequencers and up to two eDP ports.
> +  */
> + if (WARN_ON(pipes == 0))
> + return PIPE_A;
> +
> + intel_dp->pps_pipe = ffs(pipes) - 1;
> +
> + DRM_DEBUG_KMS("picked pipe %c power sequencer for port %c\n",
> +   pipe_name(intel_dp->pps_pipe), 
> port_name(intel_dig_port->port));
> +
> + /* init power sequencer on this pipe and port */
> + intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq);
> + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp,
> +   &power_seq);
> +
> + return intel_dp->pps_pipe;
> +}
> +
> +static enum pipe
> +vlv_initial_power_sequencer_pipe(struct drm_i915_private *dev_priv,
> +  enum port port)
> +{
> + enum pipe pipe;
>  
> - /* init time, try to find a pipe with this port selected */
>   for (pipe = PIPE_A; pipe <= PIPE_B; pipe++) {
>   u32 port_sel = I915_READ(VLV_PIPE_PP_ON_DELAYS(pipe)) &
>   PANEL_PORT_SELECT_MASK;
> - if (port_sel == PANEL_PORT_SELECT_VLV(port))
> - return pipe;
> +
> + if (port_sel != PANEL_PORT_SELECT_VLV(port))
> + continue;
> +
> + return pipe;
> + }
> +
> + return INVALID_PIPE;
> +}
> +
> +static void
> +vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_device *dev = intel_dig_port->base.base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct edp_power_seq power_seq;
> + enum port port = intel_dig_port->port;
> +
> + lockdep_assert_held(&dev_priv->pps_mutex);
> +
> + /* try to find a pipe with this port selected */
> + intel_dp->pps_pipe = vlv_initial_power_sequencer_pipe(dev_priv, port);
> +
> + /* didn't find one? just let vlv_power_sequencer_pipe() pick one when 
> needed */
> + if (intel_dp->pps_pipe == INVALID_PIPE) {
> + DRM_DEBUG_KMS("no initial power sequencer for port %c\n",
> +   port_name(port));
> + return;
>   }
>  
> - /* shrug */
> - return PIPE_A;
> + DRM_DEBUG_KMS("initial power sequencer for port %c: pipe %c\n",
> +   port_name(port), pipe_name(intel_dp->pps_pipe));
> +
> + intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq);
> + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp,
> +   &power_seq);
>  }
>  
>  static u32 _pp_ctrl_reg(struct intel_dp *intel_dp)
> @@ -2209,6 +2280,76 @@ static void g4x_pre_enable_dp(struct intel_encoder 
> *encoder)
>   }
>  }
>  
> +static void vlv_steal_power_sequenc