Re: [Intel-gfx] [PATCH 1/2] drm/i915: Adding break for one case

[Jani Nikula]

On Thu, 13 Aug 2015, Xiong Zhang  wrote:
> Signed-off-by: Xiong Zhang 

Even for a small patch like this, your commit message is inadequate.

First, it's obvious from the code that you're adding a break for one
case. Instead, you should explain what bug you fix. If someone hits this
bug and is looking for a fix, he's not going to find your patch with
this title.

Second, that break was omitted from or removed by some commit, and you
should find out which one it was, and reference it in your commit
message. It's helpful for everyone, and particularly for maintainers and
backporters who need to find that out anyway to decide where this fix
should be applied.

Thanks,
Jani.

> ---
>  drivers/gpu/drm/i915/intel_display.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 65cc5b1..801187c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1100,6 +1100,7 @@ bool ibx_digital_port_connected(struct drm_i915_private 
> *dev_priv,
>   break;
>   case PORT_E:
>   bit = SDE_PORTE_HOTPLUG_SPT;
> + break;
>   default:
>   return true;
>   }
> -- 
> 2.1.4
>
> ___
> 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 1/2] Revert "drm/i915: Add eDP intermediate frequencies for CHV"

[Sivakumar Thulasimani]

On 8/14/2015 12:29 PM, Jani Nikula wrote:

On Wed, 12 Aug 2015, Daniel Vetter  wrote:

On Wed, Aug 12, 2015 at 04:02:17PM +0300, Ville Syrjälä wrote:

On Wed, Aug 12, 2015 at 05:31:55PM +0530, Sivakumar Thulasimani wrote:


On 8/12/2015 5:02 PM, Ville Syrjälä wrote:

On Fri, Jul 31, 2015 at 11:32:52AM +0530, Sivakumar Thulasimani wrote:

From: "Thulasimani,Sivakumar" 

This reverts
commit fe51bfb95c996733150c44d21e1c9f4b6322a326.
Author: Ville Syrjälä 
Date:   Thu Mar 12 17:10:38 2015 +0200

CHV does not support intermediate frequencies so reverting the
patch that added it in the first place

My docs still say it does. Is there some undocumented problem with the
PLL or is this just a marketing decision?

I don't think so, i too have just one document that shows the phy values
for each of
the link rates but have not found any where else to say it is supported .

Display cluster HAS says they're supported. And since the spreadsheet
has them all in green I assume someone actually figured that they ought
to work.


Also this is not tested by anyone including us from product team so i
highly doubt
   it will even work.

Well if no one has tested them I guess we shouldn't try to use them. Not
a big loss in any case I suppose.

So based on that reasoning I can give an ack for this patch:
Acked-by: Ville Syrjälä 


regarding HBR2, it was supposed to land on a future stepping that never
happened
so it is not supported at all.

Hmm. Display Cluster HAS listed it as a stretch goal for early steppings. Apart
from that there isn't much else to go by. Excepth the training pattern 3
support, but now that I look again the new bit for that has disappeared
from the DP register in the spec. Or maybe I was looking at the k0 spec
when I added it. It's still in the k0 spec but as you say that's been
nuked.

In light of this, I think dropping HBR2 is reasonable. HBR is anyway
enough for 4k@30 with 4 lanes, so it's not like we really need HBR2.

And could you also cook up a patch to kill the training pattern 3
support for CHV? Should be mostly a revert of my original patch that
added it, but you probably need to adjust the use_tps3 condition a bit
too.

Can we please grill the people responsible for this docs mess some more?

Patch itself is for Jani.

There was some suggestions from Ville [1] to patch 1/2 that I haven't
seen a reply to.

BR,
Jani.

[1] http://mid.gmane.org/20150812131205.gw5...@intel.com


yes, i can make that change. i was assuming that since Daniel's reply 
had message
"patch itself is for Jani" that you would pick it up :), will check once 
before waiting

next time.

-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


--
regards,
Sivakumar

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

Re: [Intel-gfx] [PATCH] drm/i915: fix typo causing bad memory access in ring init

[Chris Wilson]

On Fri, Aug 14, 2015 at 12:13:04PM +1000, Dave Airlie wrote:
> From: Dave Airlie 
> 
> This is validating from the wrong index.
> 
> testing with KASAN found it.
> 
> Reported-by: Dave Jones 
> Signed-off-by: Dave Airlie 
> ---
>  drivers/gpu/drm/i915/i915_cmd_parser.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c 
> b/drivers/gpu/drm/i915/i915_cmd_parser.c
> index 61ae8ff..59f85e2 100644
> --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> @@ -534,7 +534,7 @@ static bool validate_cmds_sorted(struct intel_engine_cs 
> *ring,
>  
>   for (j = 0; j < table->count; j++) {
>   const struct drm_i915_cmd_descriptor *desc =
> - &table->table[i];
> + &table->table[j];
>   u32 curr = desc->cmd.value & desc->cmd.mask;

Which uncovered another problem that the tables weren't sorted and
triggered a nasty BUG().

Daniel should have already sent the pair of fixes onwards.
-Chris

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

[Intel-gfx] [PATCH] drm/i915/skl: WaIgnoreDDIAStrap is forever, always init DDI A

[Jani Nikula]

There is currently conflicting documentation on which steppings the
workaround is needed, up to C vs. forever. However there is post-C
stepping hardware that doesn't report port presence on DDI A, leading to
black screen on eDP. Assume the strap isn't connected, and try to enable
DDI A on these machines. (We'll still check the VBT for the info in DDI
init.)

Cc: Ville Syrjälä 
Cc: Mika Westerberg 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/intel_display.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 21aa745caed1..aa208a2cfafc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13949,8 +13949,7 @@ static void intel_setup_outputs(struct drm_device *dev)
 */
found = I915_READ(DDI_BUF_CTL_A) & DDI_INIT_DISPLAY_DETECTED;
/* WaIgnoreDDIAStrap: skl */
-   if (found ||
-   (IS_SKYLAKE(dev) && INTEL_REVID(dev) < SKL_REVID_D0))
+   if (found || IS_SKYLAKE(dev))
intel_ddi_init(dev, PORT_A);
 
/* DDI B, C and D detection is indicated by the SFUSE_STRAP
-- 
2.1.4

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

Re: [Intel-gfx] [PATCH 1/2 v2 addendum v2] drm/i915: Allow parsing of variable size child device entries from VBT

[Timo Aaltonen]

On 13.08.2015 16:14, David Weinehall wrote:
> On Wed, Aug 12, 2015 at 05:19:35PM +0300, Jani Nikula wrote:
>> On Wed, 12 Aug 2015, David Weinehall  wrote:
>>> Some more fixup is needed; the bits from Antti's patch
>>> that actually expanded the struct to fully fit the newer
>>> versions of the child_device_config was part of the second
>>> patch; since that patch hasn't been merged yet we need this bit:
>>>
>>> This applies on top of the patch you already merged
>>> (the Iboost patch will need corresponding adjustment to
>>>  remove the changes I split out):
>>>
>>> Expand common_child_dev_config to be able to fit all information
>>> defined by the latest VBT specification.
>>>
>>> Signed-off-by: David Weinehall 
>>> CC: Antti Koskipaa 
>>> ---
>>>  intel_bios.c |7 ++-
>>>  intel_bios.h |4 
>>>  2 files changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_bios.c 
>>> b/drivers/gpu/drm/i915/intel_bios.c
>>> index 990acc20771a..40e2cc4e7419 100644
>>> --- a/drivers/gpu/drm/i915/intel_bios.c
>>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>>> @@ -1038,6 +1038,10 @@ parse_device_mapping(struct drm_i915_private 
>>> *dev_priv,
>>> DRM_DEBUG_KMS("No general definition block is found, no devices 
>>> defined.\n");
>>> return;
>>> }
>>> +   /* Remember to keep this in sync with child_device_config;
>>> +* whenever a new feature is added to BDB that causes that
>>> +* struct to grow this needs to be updated too
>>> +*/
>>> if (bdb->version < 195) {
>>> expected_size = 33;
>>> } else if (bdb->version == 195) {
>>> @@ -1051,7 +1055,8 @@ parse_device_mapping(struct drm_i915_private 
>>> *dev_priv,
>>> }
>>>  
>>> if (expected_size > sizeof(*p_child)) {
>>> -   DRM_ERROR("child_device_config cannot fit in p_child\n");
>>> +   DRM_ERROR("child_device_config (size %u) cannot fit in p_child 
>>> (size %u); bdb->version: %u\n",
>>> + expected_size, sizeof(*p_child), bdb->version);
>>
>> drivers/gpu/drm/i915/intel_bios.c: In function ‘parse_device_mapping’:
>> drivers/gpu/drm/i915/intel_bios.c:1058:3: warning: format ‘%u’ expects 
>> argument of type ‘unsigned int’, but argument 3 has type ‘long unsigned int’ 
>> [-Wformat=]
>>DRM_ERROR("child_device_config (size %u) cannot fit in p_child (size %u); 
>> bdb->version: %u\n",
>>^
>>   CC [M]  drivers/gpu/drm/i915/intel_fifo_underrun.o
> 
> Well spotted.  Or rather, stupid of me to forget that sizeof yields
> size_t as type, thus necessitating %zu as conversion specifier.
> 
> Fixed version:
> 
> Expand common_child_dev_config to be able to fit all information
> defined by the latest VBT specification.
> 
> v2: Use proper conversion specifier for size_t (%zu)
> 
> Signed-off-by: David Weinehall 
> CC: Antti Koskipaa 
> 
>  intel_bios.c |7 ++-
>  intel_bios.h |4 
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.c 
> b/drivers/gpu/drm/i915/intel_bios.c
> index 990acc20771a..5673ed53797b 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1038,6 +1038,10 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
>   DRM_DEBUG_KMS("No general definition block is found, no devices 
> defined.\n");
>   return;
>   }
> + /* Remember to keep this in sync with child_device_config;
> +  * whenever a new feature is added to BDB that causes that
> +  * struct to grow this needs to be updated too
> +  */
>   if (bdb->version < 195) {
>   expected_size = 33;
>   } else if (bdb->version == 195) {
> @@ -1051,7 +1055,8 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
>   }
>  
>   if (expected_size > sizeof(*p_child)) {
> - DRM_ERROR("child_device_config cannot fit in p_child\n");
> + DRM_ERROR("child_device_config (size %u) cannot fit in p_child 
> (size %zu); bdb->version: %u\n",
> +   expected_size, sizeof(*p_child), bdb->version);
>   return;
>   }
>  
> diff --git a/drivers/gpu/drm/i915/intel_bios.h 
> b/drivers/gpu/drm/i915/intel_bios.h
> index f7ad6a585129..71cb96f77870 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -239,6 +239,10 @@ 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;
>  } __packed;
>  
>  /* This field changes depending on the BDB version, so the most reliable way 
> to

Please apply this, fixes issues on some machine(s) here. Probably also
cc:stable since it applies to BSW as well as SKL.

and the upstream bug could be referenced too, it's
https://bugs.freedesktop.org/show_bug.cgi?id=91613

-- 
t
___
Intel-gfx mailing list
Inte

Re: [Intel-gfx] [PATCH] drm/i915: fix typo causing bad memory access in ring init

[Daniel Vetter]

On Fri, Aug 14, 2015 at 08:40:47AM +0100, Chris Wilson wrote:
> On Fri, Aug 14, 2015 at 12:13:04PM +1000, Dave Airlie wrote:
> > From: Dave Airlie 
> > 
> > This is validating from the wrong index.
> > 
> > testing with KASAN found it.
> > 
> > Reported-by: Dave Jones 
> > Signed-off-by: Dave Airlie 
> > ---
> >  drivers/gpu/drm/i915/i915_cmd_parser.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c 
> > b/drivers/gpu/drm/i915/i915_cmd_parser.c
> > index 61ae8ff..59f85e2 100644
> > --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> > +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> > @@ -534,7 +534,7 @@ static bool validate_cmds_sorted(struct intel_engine_cs 
> > *ring,
> >  
> > for (j = 0; j < table->count; j++) {
> > const struct drm_i915_cmd_descriptor *desc =
> > -   &table->table[i];
> > +   &table->table[j];
> > u32 curr = desc->cmd.value & desc->cmd.mask;
> 
> Which uncovered another problem that the tables weren't sorted and
> triggered a nasty BUG().
> 
> Daniel should have already sent the pair of fixes onwards.

Yeah if you want this fixed asap you need 9f58582c7ad64f025e7f and
8453580cb8834dedffda from next for other it'll BUG.
-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/18] gitignore: ignore more files

[Daniel Vetter]

On Thu, Aug 13, 2015 at 01:31:41PM -0700, Jesse Barnes wrote:

git clean fixes this all, at least over here git status is clean.
-Daniel

> ---
>  .gitignore   |  3 +++
>  tests/.gitignore | 13 +
>  tools/.gitignore |  8 
>  3 files changed, 24 insertions(+)
> 
> diff --git a/.gitignore b/.gitignore
> index a438c1c..533f6f1 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -90,3 +90,6 @@ gtk-doc.m4
>  
>  piglit
>  results
> +
> +*.orig
> +version.h
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 31d35a5..e45cbb7 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -99,6 +99,7 @@ gem_set_tiling_vs_blt
>  gem_set_tiling_vs_gtt
>  gem_set_tiling_vs_pwrite
>  gem_storedw_batches_loop
> +gem_storedw_loop
>  gem_storedw_loop_blt
>  gem_storedw_loop_bsd
>  gem_storedw_loop_render
> @@ -169,3 +170,15 @@ prime_udl
>  template
>  test-list.txt
>  testdisplay
> +ddi_compute_wrpll
> +gem_vmap_blits
> +gem_wait_render_timeout
> +igt_fork_helper
> +igt_list_only
> +igt_no_exit
> +igt_no_exit_list_only
> +igt_no_subtest
> +igt_simulation
> +pm_pc8
> +pm_psr
> +
> diff --git a/tools/.gitignore b/tools/.gitignore
> index 49bd24f..7bf91e3 100644
> --- a/tools/.gitignore
> +++ b/tools/.gitignore
> @@ -37,3 +37,11 @@ intel_vga_write
>  intel_watermark
>  skl_compute_wrpll
>  skl_ddb_allocation
> +forcewaked
> +intel_dpio_read
> +intel_dpio_write
> +intel_gpu_dump
> +intel_nc_read
> +intel_nc_write
> +intel_punit_read
> +intel_punit_write
> -- 
> 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] sna: Fix the reduction of xy reflection onto rotations.

[Chris Wilson]

On Thu, Aug 13, 2015 at 04:51:37PM -0700, Bob Paauwe wrote:
> When reducing a xy reflection to a 180 degree rotation, make sure
> only one rotation bit is set.  Also by rotating the bit left, we
> can support cases where xy reflection happens with 90/270 degree
> rotation.
> 
> Signed-off-by: Bob Paauwe 

Nice. Took me a few moments to verify the shifting works as expected, so
I left a couple of comments there for my future self.

Thanks, pushed
-Chris

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

Re: [Intel-gfx] [PATCH 0/7] drm/i915: Move DP link parameters out from intel_dp

[Daniel Vetter]

On Wed, Aug 12, 2015 at 07:04:22PM +0300, Ville Syrjälä wrote:
> On Mon, Jul 06, 2015 at 03:09:59PM +0300, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> > 
> > While working on CHV DPIO powergating I relized DP .compute_config() was
> > clobbering lane_count etc. stored in intel_dp. This could cause problems
> > if we do the .compute_config() but later fail the modeset for some reason.
> > Any subsequent link re-training might then fail if intel_dp->lane_count
> > etc. got changed.
> > 
> > The reason I ran into this during the DPIO powergating work was that I may
> > need to know which lanes he active when shutting down the link. However
> > .compute_config() already clobbered that information by the time I need it.
> > By moving it to the pipe config we avoid that problem as well.
> > 
> > I also cleaned up the limited color range handling a bit while I was
> > in the neighborhood.
> > 
> 
> >   drm/i915: Clean up DP/HDMI limited color range handling
> >   drm/i915: Move intel_dp->lane_count into pipe_config
> 
> These two are still lacking a r-b. Would be nice to get these in so that
> they don't end up blocking the CHV DPIO powergating stuff once that gets
> reviewed.
> 
> Sivakumar, any chance you'd like to review those as well? Or do we have
> anyone else interested?

All merged 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] [4.2-rc4] acpi|drm|i915: circular locking dependency: acpi_video_get_backlight_type

[Hans de Goede]

Hi,

On 13-08-15 16:33, Hans de Goede wrote:

Hi,

On 12-08-15 21:26, Ville Syrjälä wrote:

On Mon, Aug 10, 2015 at 08:29:00PM +0200, Sedat Dilek wrote:

On Sat, Aug 1, 2015 at 2:23 PM, Sedat Dilek  wrote:

On Mon, Jul 27, 2015 at 12:33 AM, Sedat Dilek  wrote:

Hi,

this my first build of a 4.2-rcN Linux-kernel and I see this...



Just FYI:

I am *not* seeing this with drm-intel-nightly from below url.

Also, I plan to test Linux v4.2-rc5.



[ CC Linus ]

Knock Knock Knock.

This issue still remains here (with CONFIG_DRM_I915=m)...

[   18.269792] ==
[   18.269798] [ INFO: possible circular locking dependency detected ]
[   18.269805] 4.2.0-rc6-1-iniza-small #1 Not tainted
[   18.269810] ---
[   18.269816] modprobe/727 is trying to acquire lock:
[   18.269822]  (init_mutex){+.+.+.}, at: []
acpi_video_get_backlight_type+0x17/0x164 [video]
[   18.269840]
[   18.269840] but task is already holding lock:
[   18.269848]  (&(&backlight_notifier)->rwsem){..}, at:
[] __blocking_notifier_call_chain+0x39/0x70
[   18.269864]
[   18.269864] which lock already depends on the new lock.
[   18.269864]
[   18.269875]
[   18.269875] the existing dependency chain (in reverse order) is:
[   18.269884]
...

Full dmesg log and kernel-config attached.

Shall I add Rusty and modules/modprobe folks?


Just got back from vacation and was greeted by this same lockdep splat.

On a hunch I reverted

commit 93a291dfaf9c328ca5a9cea1733af1a128efe890
Author: Hans de Goede 
Date:   Tue Jun 16 16:27:52 2015 +0200

 ACPI / video: Move backlight notifier to video_detect.c

and the problem seems to be gone. Hans, any thoughts?


Looking into this atm, lockdep clearly is right.

Sorry about this I have put a lot of thinking into avoiding
these kind of issues with this patch-set, but I did not realize
there was another lock "hiding" inside the notifier-chain.

Further analysis shows that the lock inside the notifier-chain
causes similar problems vs register_count_mutex from
drivers/acpi/acpi_video.c. I'm working on a fix for this
atm.


Heh, look at what I just found (I'm shifting my work focus
in the direction of nouveau) :

https://bugzilla.redhat.com/show_bug.cgi?id=1152876

So it looks like we have had the root cause of this issue
for a long time already, maybe my recent backlight selection
logic cleanup / rework has made it easier to trigger the
lockdep warning for this though.

Anyhow assuming people are ok with the fix I submitted
yesterday we've a fix for this now.

Regards,

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

Re: [Intel-gfx] [PATCH] drm/i915: fix checksum write for automated test reply

[Jindal, Sonika]

Looks good to me.
Reviewed-by: Sonika Jindal 

-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of 
Sivakumar Thulasimani
Sent: Friday, August 7, 2015 3:15 PM
To: dan...@ffwll.ch; intel-gfx@lists.freedesktop.org
Subject: [Intel-gfx] [PATCH] drm/i915: fix checksum write for automated test 
reply

From: "Thulasimani,Sivakumar" 

DP spec requires the checksum of the last block read to be written when 
replying to TEST_EDID_READ. This patch fixes the current code to do the same.

v2: removed loop for jumping blocks and performed direct addition as 
recommended by Daniel

Signed-off-by: Sivakumar Thulasimani 
---
 drivers/gpu/drm/i915/intel_dp.c |9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c 
index f1b9f93..fa6e202 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4090,9 +4090,16 @@ static uint8_t intel_dp_autotest_edid(struct intel_dp 
*intel_dp)
  intel_dp->aux.i2c_defer_count);
intel_dp->compliance_test_data = INTEL_DP_RESOLUTION_FAILSAFE;
} else {
+   struct edid *block = intel_connector->detect_edid;
+
+   /* We have to write the checksum
+* of the last block read
+*/
+   block += intel_connector->detect_edid->extensions;
+
if (!drm_dp_dpcd_write(&intel_dp->aux,
DP_TEST_EDID_CHECKSUM,
-   &intel_connector->detect_edid->checksum,
+   &block->checksum,
1))
DRM_DEBUG_KMS("Failed to write EDID checksum\n");
 
--
1.7.9.5

___
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 v3 03/11] drm/i915: HDMI pixel clock check

[Daniel Vetter]

On Wed, Aug 12, 2015 at 09:34:54PM +0300, Ville Syrjälä wrote:
> On Fri, Jul 31, 2015 at 03:13:52PM +0300, Mika Kahola wrote:
> > It is possible the we request to have a mode that has
> > higher pixel clock than our HW can support. This patch
> > checks if requested pixel clock is lower than the one
> > supported by the HW. The requested mode is discarded
> > if we cannot support the requested pixel clock.
> > 
> > This patch applies to HDMI.
> > 
> > V2:
> > - removed computation for max dot clock
> > 
> > V3:
> > - cleanup by removing unnecessary lines
> > 
> > Signed-off-by: Mika Kahola 
> > ---
> >  drivers/gpu/drm/i915/intel_hdmi.c | 4 
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
> > b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 70bad5b..3149e5f 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -1193,10 +1193,14 @@ intel_hdmi_mode_valid(struct drm_connector 
> > *connector,
> > struct drm_device *dev = intel_hdmi_to_dev(hdmi);
> > enum drm_mode_status status;
> > int clock;
> > +   int max_pixclk = to_i915(connector->dev)->max_dotclk;
> >  
> > if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> > return MODE_NO_DBLESCAN;
> >  
> > +   if (mode->clock > max_pixclk)
> > +   return MODE_CLOCK_HIGH;
> > +
> > clock = mode->clock;
> 
> I believe we should do something like this here:
> 
>   clock = mode->clock;
> 
>   if ((mode->flags & DRM_MODE_FLAG_3D_MASK) == 
> DRM_MODE_FLAG_3D_FRAME_PACKING)
>   clock *= 2;

That's already done in drm_mode_set_crtcinfo, i915 uses the STEREO_DOUBLE
mode of that function.
-Daniel

> 
>   if (clock > max_pixclk)
>   return MODE_CLOCK_HIGH;
> 
> > if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> > clock *= 2;
> 
> The stero handling should probably be in a separate patch, but in
> preparation for it you could already put the dotclk check between the
> clock= assigment and the DBCLK doubling (since DBLCLK only affects the
> port_clock and not pixel clock).
> 
> > -- 
> > 1.9.1
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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 08/11] drm/i915: TV pixel clock check

[Daniel Vetter]

On Wed, Aug 12, 2015 at 09:24:01PM +0300, Ville Syrjälä wrote:
> On Fri, Jul 31, 2015 at 03:13:57PM +0300, Mika Kahola wrote:
> > It is possible the we request to have a mode that has
> > higher pixel clock than our HW can support. This patch
> > checks if requested pixel clock is lower than the one
> > supported by the HW. The requested mode is discarded
> > if we cannot support the requested pixel clock.
> > 
> > This patch applies to TV.
> > 
> > V2:
> > - removed computation for max pixel clock
> > 
> > V3:
> > - cleanup by removing unnecessary lines
> > 
> > Signed-off-by: Mika Kahola 
> > ---
> >  drivers/gpu/drm/i915/intel_tv.c | 4 
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_tv.c 
> > b/drivers/gpu/drm/i915/intel_tv.c
> > index 8b9d325..beeed25 100644
> > --- a/drivers/gpu/drm/i915/intel_tv.c
> > +++ b/drivers/gpu/drm/i915/intel_tv.c
> > @@ -897,6 +897,10 @@ intel_tv_mode_valid(struct drm_connector *connector,
> >  {
> > struct intel_tv *intel_tv = intel_attached_tv(connector);
> > const struct tv_mode *tv_mode = intel_tv_mode_find(intel_tv);
> > +   int max_pixclk = to_i915(connector->dev)->max_dotclk;
> > +
> > +   if (mode->clock > max_pixclk)
> > +   return MODE_CLOCK_HIGH;
> 
> The mode handling in intel_tv.c seems to be a mess. But I suppose this
> should be OK.

We set a fake mode (which is the one userspace asks for) and the hw
internally munges it to the actual tv mode with a scaler. Except that the
frame-start/dotclock are fed from the actual TV mode (which is why
kms_flip has a check for TV modes since the timings are all wrong).

Yup, giant hairball indeed ;-)
-Daniel

> 
> Reviewed-by: Ville Syrjälä 
> 
> >  
> > /* Ensure TV refresh is close to desired refresh */
> > if (tv_mode && abs(tv_mode->refresh - drm_mode_vrefresh(mode) * 1000)
> > -- 
> > 1.9.1
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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 09/11] drm/i915: DisplayPort-MST pixel clock check

[Daniel Vetter]

On Wed, Aug 12, 2015 at 09:49:12PM +0300, Ville Syrjälä wrote:
> On Fri, Jul 31, 2015 at 03:13:58PM +0300, Mika Kahola wrote:
> > It is possible the we request to have a mode that has
> > higher pixel clock than our HW can support. This patch
> > checks if requested pixel clock is lower than the one
> > supported by the HW. The requested mode is discarded
> > if we cannot support the requested pixel clock.
> > 
> > This patch applies to DisplayPort MST.
> > 
> > V2:
> > - removed computation for max pixel clock
> > 
> > V3:
> > - cleanup by removing unnecessary lines
> > 
> > Signed-off-by: Mika Kahola 
> > ---
> >  drivers/gpu/drm/i915/intel_dp_mst.c | 5 +
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
> > b/drivers/gpu/drm/i915/intel_dp_mst.c
> > index 585f0a4..fcf03d0 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > @@ -347,6 +347,8 @@ static enum drm_mode_status
> >  intel_dp_mst_mode_valid(struct drm_connector *connector,
> > struct drm_display_mode *mode)
> >  {
> > +   int max_pixclk = to_i915(connector->dev)->max_dotclk;
> 
> The pixclk vs. dotclk in every patch is tickling my ocd nerves. I'd say
> pick one and stick to it everywhere. I guess I'm probably to blame here
> since I've been using dotclock in the .get_config() paths, and pixclk
> in the cdclk calculations. With grep I can't say which one is winning,
> so I guess you could pick whichever seems more awesome.

+1 from me for a follow-up series to standardize on one and then maybe
even add a small DOC: kerneldoc comment explaining what all the different
clocks are that we have. We have much improved kerneldoc support now.
-Daniel

> 
> Apart from that this patch looks good so:
> Reviewed-by: Ville Syrjälä 
> 
> > +
> > /* TODO - validate mode against available PBN for link */
> > if (mode->clock < 1)
> > return MODE_CLOCK_LOW;
> > @@ -354,6 +356,9 @@ intel_dp_mst_mode_valid(struct drm_connector *connector,
> > if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> > return MODE_H_ILLEGAL;
> >  
> > +   if (mode->clock > max_pixclk)
> > +   return MODE_CLOCK_HIGH;
> > +
> > return MODE_OK;
> >  }
> >  
> > -- 
> > 1.9.1
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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 v2 addendum v2] drm/i915: Allow parsing of variable size child device entries from VBT

[Jani Nikula]

On Thu, 13 Aug 2015, David Weinehall  wrote:
> On Wed, Aug 12, 2015 at 05:19:35PM +0300, Jani Nikula wrote:
>> On Wed, 12 Aug 2015, David Weinehall  wrote:
>> > Some more fixup is needed; the bits from Antti's patch
>> > that actually expanded the struct to fully fit the newer
>> > versions of the child_device_config was part of the second
>> > patch; since that patch hasn't been merged yet we need this bit:
>> >
>> > This applies on top of the patch you already merged
>> > (the Iboost patch will need corresponding adjustment to
>> >  remove the changes I split out):
>> >
>> > Expand common_child_dev_config to be able to fit all information
>> > defined by the latest VBT specification.
>> >
>> > Signed-off-by: David Weinehall 
>> > CC: Antti Koskipaa 
>> > ---
>> >  intel_bios.c |7 ++-
>> >  intel_bios.h |4 
>> >  2 files changed, 10 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_bios.c 
>> > b/drivers/gpu/drm/i915/intel_bios.c
>> > index 990acc20771a..40e2cc4e7419 100644
>> > --- a/drivers/gpu/drm/i915/intel_bios.c
>> > +++ b/drivers/gpu/drm/i915/intel_bios.c
>> > @@ -1038,6 +1038,10 @@ parse_device_mapping(struct drm_i915_private 
>> > *dev_priv,
>> >DRM_DEBUG_KMS("No general definition block is found, no devices 
>> > defined.\n");
>> >return;
>> >}
>> > +  /* Remember to keep this in sync with child_device_config;
>> > +   * whenever a new feature is added to BDB that causes that
>> > +   * struct to grow this needs to be updated too
>> > +   */
>> >if (bdb->version < 195) {
>> >expected_size = 33;
>> >} else if (bdb->version == 195) {
>> > @@ -1051,7 +1055,8 @@ parse_device_mapping(struct drm_i915_private 
>> > *dev_priv,
>> >}
>> >  
>> >if (expected_size > sizeof(*p_child)) {
>> > -  DRM_ERROR("child_device_config cannot fit in p_child\n");
>> > +  DRM_ERROR("child_device_config (size %u) cannot fit in p_child 
>> > (size %u); bdb->version: %u\n",
>> > +expected_size, sizeof(*p_child), bdb->version);
>> 
>> drivers/gpu/drm/i915/intel_bios.c: In function ‘parse_device_mapping’:
>> drivers/gpu/drm/i915/intel_bios.c:1058:3: warning: format ‘%u’ expects 
>> argument of type ‘unsigned int’, but argument 3 has type ‘long unsigned int’ 
>> [-Wformat=]
>>DRM_ERROR("child_device_config (size %u) cannot fit in p_child (size %u); 
>> bdb->version: %u\n",
>>^
>>   CC [M]  drivers/gpu/drm/i915/intel_fifo_underrun.o
>
> Well spotted.  Or rather, stupid of me to forget that sizeof yields
> size_t as type, thus necessitating %zu as conversion specifier.
>
> Fixed version:
>
> Expand common_child_dev_config to be able to fit all information
> defined by the latest VBT specification.
>
> v2: Use proper conversion specifier for size_t (%zu)
>
> Signed-off-by: David Weinehall 
> CC: Antti Koskipaa 
>
>  intel_bios.c |7 ++-
>  intel_bios.h |4 
>  2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c 
> b/drivers/gpu/drm/i915/intel_bios.c
> index 990acc20771a..5673ed53797b 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1038,6 +1038,10 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
>   DRM_DEBUG_KMS("No general definition block is found, no devices 
> defined.\n");
>   return;
>   }
> + /* Remember to keep this in sync with child_device_config;
> +  * whenever a new feature is added to BDB that causes that
> +  * struct to grow this needs to be updated too
> +  */
>   if (bdb->version < 195) {
>   expected_size = 33;
>   } else if (bdb->version == 195) {
> @@ -1051,7 +1055,8 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
>   }
>  
>   if (expected_size > sizeof(*p_child)) {
> - DRM_ERROR("child_device_config cannot fit in p_child\n");
> + DRM_ERROR("child_device_config (size %u) cannot fit in p_child 
> (size %zu); bdb->version: %u\n",
> +   expected_size, sizeof(*p_child), bdb->version);
>   return;
>   }
>  
> diff --git a/drivers/gpu/drm/i915/intel_bios.h 
> b/drivers/gpu/drm/i915/intel_bios.h
> index f7ad6a585129..71cb96f77870 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -239,6 +239,10 @@ 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;

How does this impact the if (p_defs->child_dev_size != sizeof(*p_child))
check in parse_sdvo_device_mapping()?

BR,
Jani.

>  } __packed;
>  
>  /* This field changes depending on the BDB version, so the most reliable way 
> to

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org

Re: [Intel-gfx] [PATCH 6/6 v3] drm/i915: Enable HDMI on DDI-E

[Daniel Vetter]

On Thu, Aug 13, 2015 at 02:57:38AM +, Zhang, Xiong Y wrote:
> > On Wed, Aug 12, 2015 at 06:39:34PM +0800, Xiong Zhang wrote:
> > > DDI-E doesn't have the correspondent GMBUS pin.
> > >
> > > We rely on VBT to tell us which one it being used instead.
> > >
> > > The DVI/HDMI on shared port couldn't exist.
> > >
> > > This patch isn't tested without hardware wchich has HDMI on DDI-E.
> > >
> > > v2: fix trailing whitespace
> > > v3: WARN() take place of BUG()
> > 
> > I asked for MISSING_CASE, not WARN.
> > -Daniel
> [Zhang, Xiong Y] Because DDI-E on SKL doesn't have correspondent GMBUS pin, 
> it should share such pin with DDI-B/C/D. We don't know the default GMBUS pin 
> for DDI-E if VBT doesn't tell us. 
> If VBT don't tell us or give us an invalid GMBUS pin, driver will fail in 
> getting HDMI info. In such case, we really need a warn which tell us why 
> driver couldn't read EDID info for HDMI on DDI-E.

Have you bothered to look at the MISSING_CASE macro?! It does boil down to
a WARN, but I prefer it much over a WARN_ON since it's nicely
standardized.

Thanks, Daniel

> 
> thanks
> > 
> > >
> > > Signed-off-by: Xiong Zhang 
> > > Reviewed-by: Rodrigo Vivi 
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.h   |  5 +
> > >  drivers/gpu/drm/i915/intel_bios.c | 25 +
> > > drivers/gpu/drm/i915/intel_hdmi.c | 18 ++
> > >  3 files changed, 44 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h index 6d93334..5d8e7fe 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -1414,6 +1414,10 @@ enum modeset_restore {  #define DP_AUX_C
> > 0x20
> > > #define DP_AUX_D 0x30
> > >
> > > +#define DDC_PIN_B  0x05
> > > +#define DDC_PIN_C  0x04
> > > +#define DDC_PIN_D  0x06
> > > +
> > >  struct ddi_vbt_port_info {
> > >   /*
> > >* This is an index in the HDMI/DVI DDI buffer translation table.
> > > @@ -1428,6 +1432,7 @@ struct ddi_vbt_port_info {
> > >   uint8_t supports_dp:1;
> > >
> > >   uint8_t alternate_aux_channel;
> > > + uint8_t alternate_ddc_pin;
> > >  };
> > >
> > >  enum psr_lines_to_wait {
> > > diff --git a/drivers/gpu/drm/i915/intel_bios.c
> > > b/drivers/gpu/drm/i915/intel_bios.c
> > > index 16cdf17..8140531 100644
> > > --- a/drivers/gpu/drm/i915/intel_bios.c
> > > +++ b/drivers/gpu/drm/i915/intel_bios.c
> > > @@ -894,7 +894,7 @@ static void parse_ddi_port(struct drm_i915_private
> > *dev_priv, enum port port,
> > >   uint8_t hdmi_level_shift;
> > >   int i, j;
> > >   bool is_dvi, is_hdmi, is_dp, is_edp, is_crt;
> > > - uint8_t aux_channel;
> > > + uint8_t aux_channel, ddc_pin;
> > >   /* Each DDI port can have more than one value on the "DVO Port" field,
> > >* so look for all the possible values for each port and abort if more
> > >* than one is found. */
> > > @@ -928,6 +928,7 @@ static void parse_ddi_port(struct drm_i915_private
> > *dev_priv, enum port port,
> > >   return;
> > >
> > >   aux_channel = child->raw[25];
> > > + ddc_pin = child->common.ddc_pin;
> > >
> > >   is_dvi = child->common.device_type &
> > DEVICE_TYPE_TMDS_DVI_SIGNALING;
> > >   is_dp = child->common.device_type &
> > DEVICE_TYPE_DISPLAYPORT_OUTPUT;
> > > @@ -959,11 +960,27 @@ static void parse_ddi_port(struct
> > drm_i915_private *dev_priv, enum port port,
> > >   DRM_DEBUG_KMS("Port %c is internal DP\n", port_name(port));
> > >
> > >   if (is_dvi) {
> > > - if (child->common.ddc_pin == 0x05 && port != PORT_B)
> > > + if (port == PORT_E) {
> > > + info->alternate_ddc_pin = ddc_pin;
> > > + /* if DDIE share ddc pin with other port, then
> > > +  * dvi/hdmi couldn't exist on the shared port.
> > > +  * Otherwise they share the same ddc bin and system
> > > +  * couldn't communicate with them seperately. */
> > > + if (ddc_pin == DDC_PIN_B) {
> > > + 
> > > dev_priv->vbt.ddi_port_info[PORT_B].supports_dvi = 0;
> > > + 
> > > dev_priv->vbt.ddi_port_info[PORT_B].supports_hdmi = 0;
> > > + } else if (ddc_pin == DDC_PIN_C) {
> > > + 
> > > dev_priv->vbt.ddi_port_info[PORT_C].supports_dvi = 0;
> > > + 
> > > dev_priv->vbt.ddi_port_info[PORT_C].supports_hdmi = 0;
> > > + } else if (ddc_pin == DDC_PIN_D) {
> > > + 
> > > dev_priv->vbt.ddi_port_info[PORT_D].supports_dvi = 0;
> > > + 
> > > dev_priv->vbt.ddi_port_info[PORT_D].supports_hdmi = 0;
> > > + }
> > > + } else if (ddc_pin == DDC_PIN_B && port != PORT_B)
> > >   DRM_DEBUG_KMS("Unexpected DDC pin for port B\n");
> > > - if (child->common.ddc_pin == 0x04 && port != PORT_C)
> > > + else if (ddc_pin == DDC_PIN_C && port != PORT_C)
> > > 

Re: [Intel-gfx] [PATCH v1 1/2] drm/i915: Contain the WA_REG macro

[Daniel Vetter]

On Wed, Aug 12, 2015 at 04:40:03PM +0100, Dave Gordon wrote:
> On 11/08/15 15:44, Arun Siluvery wrote:
> >From: Mika Kuoppala 
> >
> >Prevent leaking the if scoping by containing the WA_REG
> >macro inside its own scope.
> >
> >Reported-by: Arun Siluvery 
> >Signed-off-by: Mika Kuoppala 
> >---
> >  drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
> >b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >index 1c14233..cf61262 100644
> >--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> >+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >@@ -780,11 +780,11 @@ static int wa_add(struct drm_i915_private *dev_priv,
> > return 0;
> >  }
> >
> >-#define WA_REG(addr, mask, val) { \
> >+#define WA_REG(addr, mask, val) do { \
> > const int r = wa_add(dev_priv, (addr), (mask), (val)); \
> > if (r) \
> > return r; \
> >-}
> >+} while(0)
> >
> >  #define WA_SET_BIT_MASKED(addr, mask) \
> > WA_REG(addr, (mask), _MASKED_BIT_ENABLE(mask))
> 
> On the one hand, yes, this definitely needs the do-while wrapper.
> 
> OTOH, hiding a conditional 'return' inside a macro is an abomination  :( At
> least it's only local to this file ...
> 
> So, on the grounds that this makes it more correct if no less ugly:
> 
> Reviewed-by: Dave Gordon 

Queued for -next, thanks for the patch. And I fixed the checkpatch ERROR,
tsk ;-)
-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 v1 2/2] drm/i915/gen9: Disable gather at set shader bit

[Daniel Vetter]

On Wed, Aug 12, 2015 at 04:41:13PM +0100, Dave Gordon wrote:
> On 11/08/15 15:44, Arun Siluvery wrote:
> > From Gen9, Push constant instruction parsing behaviour varies according to
> >whether set shader is enabled or not. If we want legacy behaviour then it
> >can be achieved by disabling set shader.
> >
> >Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89959
> >
> >Cc: Ben Widawsky 
> >Cc: Joonas Lahtinen 
> >Cc: Mika Kuoppala 
> >Signed-off-by: Arun Siluvery 
> >---
> >  drivers/gpu/drm/i915/i915_reg.h |  5 +
> >  drivers/gpu/drm/i915/intel_ringbuffer.c | 10 ++
> >  2 files changed, 15 insertions(+)
> 
> [snip]
> 
> >diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
> >b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >index cf61262..7d284ed 100644
> >--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> >+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >@@ -983,6 +983,16 @@ static int gen9_init_workarounds(struct intel_engine_cs 
> >*ring)
> > tmp |= HDC_FORCE_CSR_NON_COHERENT_OVR_DISABLE;
> > WA_SET_BIT_MASKED(HDC_CHICKEN0, tmp);
> >
> >+/* Chicken bits to disable set shader is in multiple places,
> >+ * set bits in all required registers to disable it correctly
> >+ */
> >+WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2, 
> >GEN9_DISABLE_GATHER_SET_SHADER_SLICE);
> >+if ((IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_D0) ||
> >+(IS_BROXTON(dev) && INTEL_REVID(dev) == BXT_REVID_A0))
> >+WA_SET_BIT_MASKED(RS_CHICKEN, 
> >RS_CHICKEN_DISABLE_GATHER_AT_SHADER);
> >+else
> >+WA_SET_BIT_MASKED(CS_RCS_BE, CS_RCS_DISABLE_GATHER_AT_SHADER);
> >+
> > return 0;
> >  }
> 
> This workaround isn't tagged with a specific /* WaXyz:chip */ comment.
> Also, the style isn't consistent with the other paragraphs earlier in this
> function: those have braces round the body part even when there's only one
> line of code, possibly to make it clear where the WA comment
> applies (of course, this is why the buggy WA_REG() macro wasn't spotted
> earlier).

Imo if we want to bikeshed this is should be

/*
 * WaBlaFoo:pft
 * extended comment if need
 */
if (IS_FOO(dev))
WA_SET_();

i.e. follow the style in this patch here for all of them.
-Daniel
> 
> So, maybe prettify this a bit, if possible? The code actually looks correct,
> just ugly.
> 
> Oh, and keep patch 1 even if you decide to abandon this one!
> 
> .Dave.
> 
> ___
> 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] lib/rendercopy_gen9: Setup Push constant pointer before sending BTP commands

[Daniel Vetter]

On Thu, Aug 13, 2015 at 03:49:35PM -0700, Ben Widawsky wrote:
> On Thu, Aug 13, 2015 at 10:33:00AM +0300, Joonas Lahtinen wrote:
> > Hi,
> > 
> > On ke, 2015-08-12 at 18:35 -0700, Ben Widawsky wrote:
> > > On Wed, Aug 12, 2015 at 03:10:18PM +0300, Joonas Lahtinen wrote:
> > > > On ke, 2015-08-12 at 12:26 +0100, Arun Siluvery wrote:
> > > > > From Gen9, by default push constant command is not committed to 
> > > > > the 
> > > > > shader unit
> > > > > untill the corresponding shader's BTP_* command is parsed. This 
> > > > > is 
> > > > > the
> > > > > behaviour when set shader is enabled. This patch updates the 
> > > > > batch to 
> > > > > follow
> > > > > this requirement otherwise it results in gpu hang.
> > > > > 
> > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89959
> > > > > 
> > > > > Set shader need to be disabled if legacy behaviour is required.
> > > > > 
> > > > > Cc: Ben Widawsky 
> > > > > Cc: Joonas Lahtinen 
> > > > > Cc: Mika Kuoppala 
> > > > > Signed-off-by: Arun Siluvery 
> > > > 
> > > > Reviewed-by: Joonas Lahtinen 
> > > > 
> > > 
> > > Repeating what I said on the mesa thread:
> > > Does anyone understand why this actually causes a hang on the IGT 
> > > test? I
> > > certainly don't. The docs are pretty clear that the constant command 
> > > is not
> > > committed until the BTP command, but I can't make any sense of how it 
> > > related to
> > > a GPU hang.
> > > 
> > 
> > Changing the order makes the hang go away and come back for sure, we've
> > all been experiencing that. System validation said it is a programming
> > restriction, so I'm not sure how relevant it is what goes wrong if it's
> > not followed. And the legacy mode bits were added to support the old
> > behavior of having the order like it has been previously, so I do not
> > see why question it without visibility to the actual RTL. And enabling
> > the legacy bits makes the hang go away, too.
> > 
> > If I had the RTL sources, then it would be more relevant to take
> > educated guesses as to why a set of hundreds of thousands of
> > transistors doesn't work as it should :) Without that, if it gets
> > stuck, it gets stuck.
> > 
> > Regards, Joonas
> > 
> 
> Let me start by saying I do believe that questioning this shouldn't prevent
> merging the patch.
> 
> 
> I absolutely disagree with you and think we should question these kind of 
> things
> and get out of the mindset of, "well, it fixes a hang, let's move on."
> Understanding these kind of things is critical to writing stable drivers.  If
> the programming guide/SV team said it can lead to a hang, that's one thing, 
> but
> AFAICT, we do not understand why it is hanging nor does any of the 
> documentation
> we do have suggest it should hang. Without clarification, next time we have a
> similar hang signature we're going to be right back here where we started.
> 
> It was one thing when there were a handful of us working on the stuff and we
> didn't have time to get to the bottom of bugs like this. I'm guilty of patches
> like this myself. I really do not see any excuse for this any more though.
> 
> 
> Could you send me the reference for where SV said it was a "programming
> restriction"? To me it all sounds very much like an implementation detail, and
> I'd like to try to understand what I am missing.

Fully agree, we can't just ignore hangs but have to bottom out on them.
And in the past we've had piles of examples where we discovered something
and didn't chase it correctly, then a few months later massive panic in
intel. At least this must be reflected in bspec.

Joonas, can you please work together with Ben to chase this down? If you
don't get traction please escalate the shit out of this,  we really must
at least get docs to accurately reflect reality.

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/9 v6] drm/i915: GuC-specific firmware loader

[Daniel Vetter]

On Wed, Aug 12, 2015 at 03:43:36PM +0100, Dave Gordon wrote:
> From: Alex Dai 
> 
> This fetches the required firmware image from the filesystem,
> then loads it into the GuC's memory via a dedicated DMA engine.
> 
> This patch is derived from GuC loading work originally done by
> Vinit Azad and Ben Widawsky.
> 
> v2:
> Various improvements per review comments by Chris Wilson
> 
> v3:
> Removed 'wait' parameter to intel_guc_ucode_load() as firmware
> prefetch is no longer supported in the common firmware loader,
>   per Daniel Vetter's request.
> Firmware checker callback fn now returns errno rather than bool.
> 
> v4:
> Squash uC-independent code into GuC-specifc loader [Daniel Vetter]
> Don't keep the driver working (by falling back to execlist mode)
> if GuC firmware loading fails [Daniel Vetter]
> 
> v5:
> Clarify WOPCM-related #defines [Tom O'Rourke]
> Delete obsolete code no longer required with current h/w & f/w
> [Tom O'Rourke]
> Move the call to intel_guc_ucode_init() later, so that it can
> allocate GEM objects, and have it fetch the firmware; then
>   intel_guc_ucode_load() doesn't need to fetch it later.
> [Daniel Vetter].
> 
> v6:
> Update comment describing intel_guc_ucode_load() [Tom O'Rourke]
> 
> Issue: VIZ-4884
> Signed-off-by: Alex Dai 
> Signed-off-by: Dave Gordon 

Checkpatch was pretty unhappy with long lines and alignment of
continuations. But didn't look too bad overall so pulled it in.

> +/*
> + * Transfer the firmware image to RAM for execution by the microcontroller.
> + *
> + * GuC Firmware layout:
> + * +---+  
> + * |  CSS header   |  128B
> + * | contains major/minor version  |
> + * +---+  
> + * | uCode |
> + * +---+  
> + * | RSA signature |  256B
> + * +---+  
> + * | RSA public Key|  256B
> + * +---+  
> + * |   Public key modulus  |4B
> + * +---+  
> + *
> + * Architecturally, the DMA engine is bidirectional, and can potentially even
> + * transfer between GTT locations. This functionality is left out of the API
> + * for now as there is no need for it.
> + *
> + * Note that GuC needs the CSS header plus uKernel code to be copied by the
> + * DMA engine in one operation, whereas the RSA signature is loaded via MMIO.

Imo this should be included in the kerneldoc too, especially now that we
can handle tables with markdown.
-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 0/9 v6] Batch submission via GuC

[Daniel Vetter]

On Wed, Aug 12, 2015 at 07:35:10PM -0700, O'Rourke, Tom wrote:
> On Wed, Aug 12, 2015 at 07:57:37AM -0700, Gordon, David S wrote:
> > On 12/08/15 15:43, Dave Gordon wrote:
> > > This patch series enables command submission via the GuC. In this mode,
> > > instead of the host CPU driving the execlist port directly, it hands
> > > over work items to the GuC, using a doorbell mechanism to tell the GuC
> > > that new items have been added to its work queue. The GuC then dispatches
> > > contexts to the various GPU engines, and manages the resulting context-
> > > switch interrupts. Completion of a batch is however still signalled to
> > > the CPU; the GuC is not involved in handling user interrupts.
> > >
> > > There are two subsequences within the patch series:
> > >
> > >drm/i915: GuC-specific firmware loader
> > >drm/i915: Debugfs interface to read GuC load status
> > >
> > > These two patches provide the GuC loader and a debugfs interface to
> > > verify the resulting state.  At this point in the sequence we can load
> > > and activate the GuC firmware, but not submit any batches through it.
> > > (This is nonetheless a potentially useful state, as the GuC could do
> > > other useful work even when not handling batch submissions).
> > >
> > >drm/i915: Expose one LRC function for GuC submission mode
> > >drm/i915: Prepare for GuC-based command submission
> > >drm/i915: Enable GuC firmware log
> > >drm/i915: Implementation of GuC submission client
> > >drm/i915: Interrupt routing for GuC submission
> > >drm/i915: Integrate GuC-based command submission
> > >drm/i915: Debugfs interface for GuC submission statistics
> > >
> > > In this second section, we implement the GuC submission mechanism, and
> > > link it into the (execlist-based) submission path. At this stage, it is
> > > not yet enabled by default; it is activated only if the kernel command
> > > line explicitly switches it on.
> > >
> > > The GuC firmware itself is not included in this patchset; it is or will
> > > be available for download from https://01.org/linuxgraphics/downloads/
> > > This driver works with and requires GuC firmware revision 3.x. It will
> > > not work with any firmware version 1.x, as the GuC protocol in those
> > > revisions was incompatible and is no longer supported.
> > >
> > > On platforms where there is no GuC, or where GuC submission is not
> > > specifically enabled, batch submission will continue to use the execlist
> > > (or ringbuffer) mechanisms.  On the other hand, if GuC submission is
> > > requested but the GuC firmware cannot be found or is invalid, the GPU
> > > will be unusable.
> > >
> > > It is expected that a subsequent patch will enable GuC submission by
> > > default once the signed firmware is published on 01.org.
> > >
> > > Ben Widawsky (0):
> > > Vinit Azad (0):
> > > Michael H. Nguyen (0):
> > >created the original versions on which some of these patches are based.
> > >
> > > Alex Dai (5):
> > >drm/i915: GuC-specific firmware loader
> > >drm/i915: Debugfs interface to read GuC load status
> > >drm/i915: Prepare for GuC-based command submission
> > >drm/i915: Enable GuC firmware log
> > >drm/i915: Integrate GuC-based command submission
> > >
> > > Dave Gordon (4):
> > >drm/i915: Expose one LRC function for GuC submission mode
> > >drm/i915: Implementation of GuC submission client
> > >drm/i915: Interrupt routing for GuC submission
> > >drm/i915: Debugfs interface for GuC submission statistics
> > >
> > >   Documentation/DocBook/drm.tmpl |  14 +
> > >   drivers/gpu/drm/i915/Makefile  |   4 +
> > >   drivers/gpu/drm/i915/i915_debugfs.c| 146 -
> > >   drivers/gpu/drm/i915/i915_dma.c|   9 +
> > >   drivers/gpu/drm/i915/i915_drv.h|  11 +
> > >   drivers/gpu/drm/i915/i915_gem.c|  16 +
> > >   drivers/gpu/drm/i915/i915_gpu_error.c  |  14 +-
> > >   drivers/gpu/drm/i915/i915_guc_reg.h|  17 +-
> > >   drivers/gpu/drm/i915/i915_guc_submission.c | 901 
> > > +
> > >   drivers/gpu/drm/i915/i915_reg.h|  15 +-
> > >   drivers/gpu/drm/i915/intel_guc.h   | 122 
> > >   drivers/gpu/drm/i915/intel_guc_fwif.h  |   9 +-
> > >   drivers/gpu/drm/i915/intel_guc_loader.c| 611 +++
> > >   drivers/gpu/drm/i915/intel_lrc.c   |  65 ++-
> > >   drivers/gpu/drm/i915/intel_lrc.h   |   8 +
> > >   15 files changed, 1918 insertions(+), 44 deletions(-)
> > >   create mode 100644 drivers/gpu/drm/i915/i915_guc_submission.c
> > >   create mode 100644 drivers/gpu/drm/i915/intel_guc.h
> > >   create mode 100644 drivers/gpu/drm/i915/intel_guc_loader.c
> > 
> > Tom O'Rourke has already R-B'ed the previous [v5] versions of these, and 
> > there are no substantive changes to patches 2, 3, 4, 5, 7 and 8, so we 
> > can carry that over; also, the change in patch 1 is just an update to a 
> > comme

Re: [Intel-gfx] [PATCH] ACPI / video: Fix circular lock dependency issue in the video-detect code

[Rafael J. Wysocki]

On Thursday, August 13, 2015 10:39:08 PM Sedat Dilek wrote:
> 
> --f46d04447e7fc2306e051d3753a5
> Content-Type: text/plain; charset=UTF-8
> 
> On Thu, Aug 13, 2015 at 6:53 PM, Hans de Goede  wrote:
> > Before this commit, the following would happen:
> >
> > a) acpi_video_get_backlight_type() gets called
> > b) acpi_video_get_backlight_type() calls acpi_video_init_backlight_type()
> > c) acpi_video_init_backlight_type() locks its function static init_mutex
> > d) acpi_video_init_backlight_type() calls backlight_register_notifier()
> > e) backlight_register_notifier() takes its notifier-chain lock
> >
> > And when the backlight notifier chain gets called we've:
> >
> > 1) blocking_notifier_call_chain() gets called
> > 2) blocking_notifier_call_chain() takes the notifier-chain lock
> > 3) blocking_notifier_call_chain() calls acpi_video_backlight_notify()
> > 4) acpi_video_backlight_notify() calls acpi_video_get_backlight_type()
> > 5) acpi_video_get_backlight_type() calls acpi_video_init_backlight_type()
> > 6) acpi_video_init_backlight_type() locks its function static init_mutex
> >
> > So in the first call sequence we have:
> >
> > a) init_mutex gets locked
> > b) notifier-chain gets locked
> >
> > and in the second call sequence we have:
> >
> > 1) notifier-chain gets locked
> > 2) init_mutex gets locked
> >
> > And we've a circular locking dependency. This specific locking dependency
> > is fixable without using the big hammer otherwise known as a workqueue,
> > but further analysis shows a similar problem with the backlight notifier
> > chain lock vs register_count_mutex from drivers/acpi/acpi_video.c,
> > and fixing that becomes problematic.
> >
> > So this commit simply fixes this with the big hammer, performance
> > wise this is a non issue as we expect the work to get scheduled
> > exactly zero or one times during normal system use.
> >
> 
> This patch on top of Linux v4.2-rc6 fixes the issue for me.
> 
> Feel free to add my...
> 
>Reported-by: Sedat Dilek 
>Tested-by: Sedat Dilek 

Applied, thanks!

Rafael

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

Re: [Intel-gfx] [PATCH] drm/i915: fix checksum write for automated test reply

[Daniel Vetter]

On Fri, Aug 14, 2015 at 08:27:45AM +, Jindal, Sonika wrote:
> Looks good to me.
> Reviewed-by: Sonika Jindal 

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

> 
> -Original Message-
> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of 
> Sivakumar Thulasimani
> Sent: Friday, August 7, 2015 3:15 PM
> To: dan...@ffwll.ch; intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH] drm/i915: fix checksum write for automated test 
> reply
> 
> From: "Thulasimani,Sivakumar" 
> 
> DP spec requires the checksum of the last block read to be written when 
> replying to TEST_EDID_READ. This patch fixes the current code to do the same.
> 
> v2: removed loop for jumping blocks and performed direct addition as 
> recommended by Daniel
> 
> Signed-off-by: Sivakumar Thulasimani 
> ---
>  drivers/gpu/drm/i915/intel_dp.c |9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> b/drivers/gpu/drm/i915/intel_dp.c index f1b9f93..fa6e202 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4090,9 +4090,16 @@ static uint8_t intel_dp_autotest_edid(struct intel_dp 
> *intel_dp)
> intel_dp->aux.i2c_defer_count);
>   intel_dp->compliance_test_data = INTEL_DP_RESOLUTION_FAILSAFE;
>   } else {
> + struct edid *block = intel_connector->detect_edid;
> +
> + /* We have to write the checksum
> +  * of the last block read
> +  */
> + block += intel_connector->detect_edid->extensions;
> +
>   if (!drm_dp_dpcd_write(&intel_dp->aux,
>   DP_TEST_EDID_CHECKSUM,
> - &intel_connector->detect_edid->checksum,
> + &block->checksum,
>   1))
>   DRM_DEBUG_KMS("Failed to write EDID checksum\n");
>  
> --
> 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 08/11] drm/i915: Add port A HPD support for IVB/HSW

[Daniel Vetter]

On Wed, Aug 12, 2015 at 06:44:17PM +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> As with ILK/SNB wire up the port A HPD on IVB/HSW.
> 
> This might be more important on HSW with PSR. BSpec tells us that if the
> automagic link training performed by the hardware fails for some reason,
> we're going to get a short HPD and are supposed to re-train the link
> manyally.

Rodrigo, could this be the cause behind our frozen screens? If we get out
of PSR and the hw link train fails then the screen will indeed freeze ...
-Daniel

> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 34 +++---
>  1 file changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 152be8b..d994b80 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -49,6 +49,10 @@ static const u32 hpd_ilk[HPD_NUM_PINS] = {
>   [HPD_PORT_A] = DE_DP_A_HOTPLUG,
>  };
>  
> +static const u32 hpd_ivb[HPD_NUM_PINS] = {
> + [HPD_PORT_A] = DE_DP_A_HOTPLUG_IVB,
> +};
> +
>  static const u32 hpd_ibx[HPD_NUM_PINS] = {
>   [HPD_CRT] = SDE_CRT_HOTPLUG,
>   [HPD_SDVO_B] = SDE_SDVOB_HOTPLUG,
> @@ -1940,6 +1944,19 @@ static void ivb_display_irq_handler(struct drm_device 
> *dev, u32 de_iir)
>  {
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   enum pipe pipe;
> + u32 hotplug_trigger = de_iir & DE_DP_A_HOTPLUG_IVB;
> +
> + if (hotplug_trigger) {
> + u32 dig_hotplug_reg, pin_mask, long_mask;
> +
> + dig_hotplug_reg = I915_READ(DIGITAL_PORT_HOTPLUG_CNTRL);
> + I915_WRITE(DIGITAL_PORT_HOTPLUG_CNTRL, dig_hotplug_reg);
> +
> + intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> +dig_hotplug_reg, hpd_ivb,
> +ilk_port_hotplug_long_detect);
> + intel_hpd_irq_handler(dev, pin_mask, long_mask);
> + }
>  
>   if (de_iir & DE_ERR_INT_IVB)
>   ivb_err_int_handler(dev);
> @@ -3137,8 +3154,13 @@ static void ilk_hpd_irq_setup(struct drm_device *dev)
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   u32 hotplug_irqs, hotplug, enabled_irqs;
>  
> - hotplug_irqs = DE_DP_A_HOTPLUG;
> - enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ilk);
> + if (INTEL_INFO(dev)->gen >= 7) {
> + hotplug_irqs = DE_DP_A_HOTPLUG_IVB;
> + enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ivb);
> + } else {
> + hotplug_irqs = DE_DP_A_HOTPLUG;
> + enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ilk);
> + }
>  
>   ilk_update_display_irq(dev_priv, hotplug_irqs, enabled_irqs);
>  
> @@ -3245,7 +3267,8 @@ static int ironlake_irq_postinstall(struct drm_device 
> *dev)
>   DE_PLANEB_FLIP_DONE_IVB |
>   DE_PLANEA_FLIP_DONE_IVB | DE_AUX_CHANNEL_A_IVB);
>   extra_mask = (DE_PIPEC_VBLANK_IVB | DE_PIPEB_VBLANK_IVB |
> -   DE_PIPEA_VBLANK_IVB | DE_ERR_INT_IVB);
> +   DE_PIPEA_VBLANK_IVB | DE_ERR_INT_IVB |
> +   DE_DP_A_HOTPLUG_IVB);
>   } else {
>   display_mask = (DE_MASTER_IRQ_CONTROL | DE_GSE | DE_PCH_EVENT |
>   DE_PLANEA_FLIP_DONE | DE_PLANEB_FLIP_DONE |
> @@ -4270,10 +4293,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
>   dev->driver->irq_uninstall = ironlake_irq_uninstall;
>   dev->driver->enable_vblank = ironlake_enable_vblank;
>   dev->driver->disable_vblank = ironlake_disable_vblank;
> - if (INTEL_INFO(dev)->gen >= 7)
> - dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
> - else
> - dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
> + dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
>   } else {
>   if (INTEL_INFO(dev_priv)->gen == 2) {
>   dev->driver->irq_preinstall = i8xx_irq_preinstall;
> -- 
> 2.4.6
> 
> ___
> 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] lib/igt_draw: add support for RGB565 and XRGB2101010

[Daniel Vetter]

On Wed, Aug 12, 2015 at 06:33:55PM -0300, Paulo Zanoni wrote:
> We need to test those pixel formats on the FBC code, so let's make
> sure the drawing library works on them first.
> 
> Signed-off-by: Paulo Zanoni 

gtkdoc update seems to be missing for the igt_draw_rect change. Even more
so that there's a line in the overview section stating that this library
assumes everything is XRGB ;-)
-Daniel

> ---
>  lib/igt_draw.c   | 162 
> +++
>  lib/igt_draw.h   |   2 +-
>  tests/kms_draw_crc.c | 113 +--
>  tests/kms_frontbuffer_tracking.c |   2 +-
>  4 files changed, 206 insertions(+), 73 deletions(-)
> 
> diff --git a/lib/igt_draw.c b/lib/igt_draw.c
> index 2210f77..afb937f 100644
> --- a/lib/igt_draw.c
> +++ b/lib/igt_draw.c
> @@ -57,6 +57,7 @@ struct buf_data {
>   uint32_t handle;
>   uint32_t size;
>   uint32_t stride;
> + int bpp;
>  };
>  
>  struct rect {
> @@ -133,27 +134,27 @@ static int swizzle_addr(int addr, int swizzle)
>  
>  /* It's all in "pixel coordinates", so make sure you multiply/divide by the 
> bpp
>   * if you need to. */
> -static int linear_x_y_to_tiled_pos(int x, int y, uint32_t stride, int 
> swizzle)
> +static int linear_x_y_to_tiled_pos(int x, int y, uint32_t stride, int 
> swizzle,
> +int bpp)
>  {
>   int x_tile_size, y_tile_size;
>   int x_tile_n, y_tile_n, x_tile_off, y_tile_off;
>   int line_size, tile_size;
>   int tile_n, tile_off;
>   int tiled_pos, tiles_per_line;
> - int bpp;
> + int pixel_size = bpp / 8;
>  
>   line_size = stride;
>   x_tile_size = 512;
>   y_tile_size = 8;
>   tile_size = x_tile_size * y_tile_size;
>   tiles_per_line = line_size / x_tile_size;
> - bpp = sizeof(uint32_t);
>  
>   y_tile_n = y / y_tile_size;
>   y_tile_off = y % y_tile_size;
>  
> - x_tile_n = (x * bpp) / x_tile_size;
> - x_tile_off = (x * bpp) % x_tile_size;
> + x_tile_n = (x * pixel_size) / x_tile_size;
> + x_tile_off = (x * pixel_size) % x_tile_size;
>  
>   tile_n = y_tile_n * tiles_per_line + x_tile_n;
>   tile_off = y_tile_off * x_tile_size + x_tile_off;
> @@ -161,19 +162,19 @@ static int linear_x_y_to_tiled_pos(int x, int y, 
> uint32_t stride, int swizzle)
>  
>   tiled_pos = swizzle_addr(tiled_pos, swizzle);
>  
> - return tiled_pos / bpp;
> + return tiled_pos / pixel_size;
>  }
>  
>  /* It's all in "pixel coordinates", so make sure you multiply/divide by the 
> bpp
>   * if you need to. */
>  static void tiled_pos_to_x_y_linear(int tiled_pos, uint32_t stride,
> - int swizzle, int *x, int *y)
> + int swizzle, int bpp, int *x, int *y)
>  {
>   int tile_n, tile_off, tiles_per_line, line_size;
>   int x_tile_off, y_tile_off;
>   int x_tile_n, y_tile_n;
>   int x_tile_size, y_tile_size, tile_size;
> - int bpp;
> + int pixel_size = bpp / 8;
>  
>   tiled_pos = swizzle_addr(tiled_pos, swizzle);
>  
> @@ -182,7 +183,6 @@ static void tiled_pos_to_x_y_linear(int tiled_pos, 
> uint32_t stride,
>   y_tile_size = 8;
>   tile_size = x_tile_size * y_tile_size;
>   tiles_per_line = line_size / x_tile_size;
> - bpp = sizeof(uint32_t);
>  
>   tile_n = tiled_pos / tile_size;
>   tile_off = tiled_pos % tile_size;
> @@ -193,32 +193,45 @@ static void tiled_pos_to_x_y_linear(int tiled_pos, 
> uint32_t stride,
>   x_tile_n = tile_n % tiles_per_line;
>   y_tile_n = tile_n / tiles_per_line;
>  
> - *x = (x_tile_n * x_tile_size + x_tile_off) / bpp;
> + *x = (x_tile_n * x_tile_size + x_tile_off) / pixel_size;
>   *y = y_tile_n * y_tile_size + y_tile_off;
>  }
>  
> -static void draw_rect_ptr_linear(uint32_t *ptr, uint32_t stride,
> -   struct rect *rect, uint32_t color)
> +static void set_pixel(void *_ptr, int index, uint32_t color, int bpp)
> +{
> + if (bpp == 16) {
> + uint16_t *ptr = _ptr;
> + ptr[index] = color;
> + } else if (bpp == 32) {
> + uint32_t *ptr = _ptr;
> + ptr[index] = color;
> + } else {
> + igt_assert_f(false, "bpp: %d\n", bpp);
> + }
> +}
> +
> +static void draw_rect_ptr_linear(void *ptr, uint32_t stride,
> +  struct rect *rect, uint32_t color, int bpp)
>  {
>   int x, y, line_begin;
>  
>   for (y = rect->y; y < rect->y + rect->h; y++) {
> - line_begin = y * stride / sizeof(uint32_t);
> + line_begin = y * stride / (bpp / 8);
>   for (x = rect->x; x < rect->x + rect->w; x++)
> - ptr[line_begin + x] = color;
> + set_pixel(ptr, line_begin + x, color, bpp);
>   }
> -
>  }
>  
> -static void draw_rect_ptr_tiled(uint32_t *ptr, uint32_t stride, int swizzle,
> -  stru

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Adding break for one case

[Daniel Vetter]

On Thu, Aug 13, 2015 at 01:37:49PM +0300, Timo Aaltonen wrote:
> On 13.08.2015 13:36, Timo Aaltonen wrote:
> > On 13.08.2015 13:00, Xiong Zhang wrote:
> >> Signed-off-by: Xiong Zhang 
> >> ---
> >>  drivers/gpu/drm/i915/intel_display.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> >> b/drivers/gpu/drm/i915/intel_display.c
> >> index 65cc5b1..801187c 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -1100,6 +1100,7 @@ bool ibx_digital_port_connected(struct 
> >> drm_i915_private *dev_priv,
> >>break;
> >>case PORT_E:
> >>bit = SDE_PORTE_HOTPLUG_SPT;
> >> +  break;
> >>default:
> >>return true;
> >>}
> >>
> > 
> > shouldn't this belong to [5/6]?
> 
> Nevermind, I see now that it got merged already.

I dropped that patch again so that we can rectify this properly. Jani's
complaint about the sub-par commit message still holds though, like why
was this not caught in testing?
-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 v4] mmap on the dma-buf directly

[Thomas Hellstrom]

On 08/13/2015 01:29 AM, Tiago Vignatti wrote:
> Hi,
>
> The idea is to create a GEM bo in one process and pass the prime handle of the
> it to another process, which in turn uses the handle only to map and write.
> This could be useful for Chrome OS architecture, where the Web content
> ("unpriviledged process") maps and CPU-draws a buffer, which was previously
> allocated in the GPU process ("priviledged process").
>
> In v2, I've added a patch that Daniel kindly drafted to allow the
> unpriviledged process flush through a prime fd. In v3, I've fixed a few
> concerns and then added end_cpu_access to i915. In v4, I fixed Sumit Semwal's
> concerns about dma-duf documentation and the FIXME missing in that same patch,
> and also removed WARN in i915 dma-buf mmap (pointed by Chris). PTAL.
>
> Best Regards,
>
> Tiago
Tiago,

I take it, this is intended to be a generic interface used mostly for 2D
rendering.

In that case, using SYNC is crucial for performance of incoherent
architectures and I'd rather see it mandatory than an option. It could
perhaps be made mandatory preferrably using an error or a one-time
kernel warning. If nobody uses the SYNC interface, it is of little use.

Also I think the syncing needs to be extended to two dimensions. A long
time ago when this was brought up people argued why we should limit it
to two dimensions, but I believe two dimensions addresses most
performance-problematic use-cases. A default implementation of
twodimensional sync can easily be made using the one-dimensional API.

Thanks,
Thomas






>
>
> Daniel Thompson (1):
>   drm: prime: Honour O_RDWR during prime-handle-to-fd
>
> Daniel Vetter (1):
>   dma-buf: Add ioctls to allow userspace to flush
>
> Tiago Vignatti (2):
>   drm/i915: Implement end_cpu_access
>   drm/i915: Use CPU mapping for userspace dma-buf mmap()
>
>  Documentation/dma-buf-sharing.txt  | 12 
>  drivers/dma-buf/dma-buf.c  | 50 
> ++
>  drivers/gpu/drm/drm_prime.c| 10 ++-
>  drivers/gpu/drm/i915/i915_gem_dmabuf.c | 28 ++-
>  include/uapi/drm/drm.h |  1 +
>  include/uapi/linux/dma-buf.h   | 43 +
>  6 files changed, 136 insertions(+), 8 deletions(-)
>  create mode 100644 include/uapi/linux/dma-buf.h
>


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

Re: [Intel-gfx] [PATCH] drm/i915/skl: WaIgnoreDDIAStrap is forever, always init DDI A

[Daniel Vetter]

On Fri, Aug 14, 2015 at 10:53:17AM +0300, Jani Nikula wrote:
> There is currently conflicting documentation on which steppings the
> workaround is needed, up to C vs. forever. However there is post-C
> stepping hardware that doesn't report port presence on DDI A, leading to
> black screen on eDP. Assume the strap isn't connected, and try to enable
> DDI A on these machines. (We'll still check the VBT for the info in DDI
> init.)
> 
> Cc: Ville Syrjälä 
> Cc: Mika Westerberg 
> Signed-off-by: Jani Nikula 

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

> ---
>  drivers/gpu/drm/i915/intel_display.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 21aa745caed1..aa208a2cfafc 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13949,8 +13949,7 @@ static void intel_setup_outputs(struct drm_device 
> *dev)
>*/
>   found = I915_READ(DDI_BUF_CTL_A) & DDI_INIT_DISPLAY_DETECTED;
>   /* WaIgnoreDDIAStrap: skl */
> - if (found ||
> - (IS_SKYLAKE(dev) && INTEL_REVID(dev) < SKL_REVID_D0))
> + if (found || IS_SKYLAKE(dev))
>   intel_ddi_init(dev, PORT_A);
>  
>   /* DDI B, C and D detection is indicated by the SFUSE_STRAP
> -- 
> 2.1.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] [4.2-rc4] acpi|drm|i915: circular locking dependency: acpi_video_get_backlight_type

[Sedat Dilek]

On Fri, Aug 14, 2015 at 10:24 AM, Hans de Goede  wrote:
> Hi,
>
>
> On 13-08-15 16:33, Hans de Goede wrote:
>>
>> Hi,
>>
>> On 12-08-15 21:26, Ville Syrjälä wrote:
>>>
>>> On Mon, Aug 10, 2015 at 08:29:00PM +0200, Sedat Dilek wrote:

 On Sat, Aug 1, 2015 at 2:23 PM, Sedat Dilek 
 wrote:
>
> On Mon, Jul 27, 2015 at 12:33 AM, Sedat Dilek 
> wrote:
>>
>> Hi,
>>
>> this my first build of a 4.2-rcN Linux-kernel and I see this...
>>
>
> Just FYI:
>
> I am *not* seeing this with drm-intel-nightly from below url.
>
> Also, I plan to test Linux v4.2-rc5.
>

 [ CC Linus ]

 Knock Knock Knock.

 This issue still remains here (with CONFIG_DRM_I915=m)...

 [   18.269792] ==
 [   18.269798] [ INFO: possible circular locking dependency detected ]
 [   18.269805] 4.2.0-rc6-1-iniza-small #1 Not tainted
 [   18.269810] ---
 [   18.269816] modprobe/727 is trying to acquire lock:
 [   18.269822]  (init_mutex){+.+.+.}, at: []
 acpi_video_get_backlight_type+0x17/0x164 [video]
 [   18.269840]
 [   18.269840] but task is already holding lock:
 [   18.269848]  (&(&backlight_notifier)->rwsem){..}, at:
 [] __blocking_notifier_call_chain+0x39/0x70
 [   18.269864]
 [   18.269864] which lock already depends on the new lock.
 [   18.269864]
 [   18.269875]
 [   18.269875] the existing dependency chain (in reverse order) is:
 [   18.269884]
 ...

 Full dmesg log and kernel-config attached.

 Shall I add Rusty and modules/modprobe folks?
>>>
>>>
>>> Just got back from vacation and was greeted by this same lockdep splat.
>>>
>>> On a hunch I reverted
>>>
>>> commit 93a291dfaf9c328ca5a9cea1733af1a128efe890
>>> Author: Hans de Goede 
>>> Date:   Tue Jun 16 16:27:52 2015 +0200
>>>
>>>  ACPI / video: Move backlight notifier to video_detect.c
>>>
>>> and the problem seems to be gone. Hans, any thoughts?
>>
>>
>> Looking into this atm, lockdep clearly is right.
>>
>> Sorry about this I have put a lot of thinking into avoiding
>> these kind of issues with this patch-set, but I did not realize
>> there was another lock "hiding" inside the notifier-chain.
>>
>> Further analysis shows that the lock inside the notifier-chain
>> causes similar problems vs register_count_mutex from
>> drivers/acpi/acpi_video.c. I'm working on a fix for this
>> atm.
>
>
> Heh, look at what I just found (I'm shifting my work focus
> in the direction of nouveau) :
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1152876
>
> So it looks like we have had the root cause of this issue
> for a long time already, maybe my recent backlight selection
> logic cleanup / rework has made it easier to trigger the
> lockdep warning for this though.
>
> Anyhow assuming people are ok with the fix I submitted
> yesterday we've a fix for this now.
>

Yay :-)

- sed@ -

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

Re: [Intel-gfx] [PATCH] ACPI / video: Fix circular lock dependency issue in the video-detect code

[Sedat Dilek]

On Fri, Aug 14, 2015 at 11:39 AM, Rafael J. Wysocki  wrote:
> On Thursday, August 13, 2015 10:39:08 PM Sedat Dilek wrote:
>>
>> --f46d04447e7fc2306e051d3753a5
>> Content-Type: text/plain; charset=UTF-8
>>
>> On Thu, Aug 13, 2015 at 6:53 PM, Hans de Goede  wrote:
>> > Before this commit, the following would happen:
>> >
>> > a) acpi_video_get_backlight_type() gets called
>> > b) acpi_video_get_backlight_type() calls acpi_video_init_backlight_type()
>> > c) acpi_video_init_backlight_type() locks its function static init_mutex
>> > d) acpi_video_init_backlight_type() calls backlight_register_notifier()
>> > e) backlight_register_notifier() takes its notifier-chain lock
>> >
>> > And when the backlight notifier chain gets called we've:
>> >
>> > 1) blocking_notifier_call_chain() gets called
>> > 2) blocking_notifier_call_chain() takes the notifier-chain lock
>> > 3) blocking_notifier_call_chain() calls acpi_video_backlight_notify()
>> > 4) acpi_video_backlight_notify() calls acpi_video_get_backlight_type()
>> > 5) acpi_video_get_backlight_type() calls acpi_video_init_backlight_type()
>> > 6) acpi_video_init_backlight_type() locks its function static init_mutex
>> >
>> > So in the first call sequence we have:
>> >
>> > a) init_mutex gets locked
>> > b) notifier-chain gets locked
>> >
>> > and in the second call sequence we have:
>> >
>> > 1) notifier-chain gets locked
>> > 2) init_mutex gets locked
>> >
>> > And we've a circular locking dependency. This specific locking dependency
>> > is fixable without using the big hammer otherwise known as a workqueue,
>> > but further analysis shows a similar problem with the backlight notifier
>> > chain lock vs register_count_mutex from drivers/acpi/acpi_video.c,
>> > and fixing that becomes problematic.
>> >
>> > So this commit simply fixes this with the big hammer, performance
>> > wise this is a non issue as we expect the work to get scheduled
>> > exactly zero or one times during normal system use.
>> >
>>
>> This patch on top of Linux v4.2-rc6 fixes the issue for me.
>>
>> Feel free to add my...
>>
>>Reported-by: Sedat Dilek 
>>Tested-by: Sedat Dilek 
>
> Applied, thanks!
>

Thanks for carrying this one.
Isn't this a for-4.2/acpi-video-fixes?
Why did you apply this in your linux-next Git branch?

- sed@ -

[1] 
http://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=7231ed1a813e0a9d249bbbe58e66ca058aee83e1

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

Re: [Intel-gfx] [PATCH 1/2 v2 addendum v2] drm/i915: Allow parsing of variable size child device entries from VBT

[Jani Nikula]

On Fri, 14 Aug 2015, Jani Nikula  wrote:
> On Thu, 13 Aug 2015, David Weinehall  wrote:
>> On Wed, Aug 12, 2015 at 05:19:35PM +0300, Jani Nikula wrote:
>>> On Wed, 12 Aug 2015, David Weinehall  
>>> wrote:
>>> > Some more fixup is needed; the bits from Antti's patch
>>> > that actually expanded the struct to fully fit the newer
>>> > versions of the child_device_config was part of the second
>>> > patch; since that patch hasn't been merged yet we need this bit:
>>> >
>>> > This applies on top of the patch you already merged
>>> > (the Iboost patch will need corresponding adjustment to
>>> >  remove the changes I split out):
>>> >
>>> > Expand common_child_dev_config to be able to fit all information
>>> > defined by the latest VBT specification.
>>> >
>>> > Signed-off-by: David Weinehall 
>>> > CC: Antti Koskipaa 
>>> > ---
>>> >  intel_bios.c |7 ++-
>>> >  intel_bios.h |4 
>>> >  2 files changed, 10 insertions(+), 1 deletion(-)
>>> >
>>> > diff --git a/drivers/gpu/drm/i915/intel_bios.c 
>>> > b/drivers/gpu/drm/i915/intel_bios.c
>>> > index 990acc20771a..40e2cc4e7419 100644
>>> > --- a/drivers/gpu/drm/i915/intel_bios.c
>>> > +++ b/drivers/gpu/drm/i915/intel_bios.c
>>> > @@ -1038,6 +1038,10 @@ parse_device_mapping(struct drm_i915_private 
>>> > *dev_priv,
>>> >   DRM_DEBUG_KMS("No general definition block is found, no devices 
>>> > defined.\n");
>>> >   return;
>>> >   }
>>> > + /* Remember to keep this in sync with child_device_config;
>>> > +  * whenever a new feature is added to BDB that causes that
>>> > +  * struct to grow this needs to be updated too
>>> > +  */
>>> >   if (bdb->version < 195) {
>>> >   expected_size = 33;
>>> >   } else if (bdb->version == 195) {
>>> > @@ -1051,7 +1055,8 @@ parse_device_mapping(struct drm_i915_private 
>>> > *dev_priv,
>>> >   }
>>> >  
>>> >   if (expected_size > sizeof(*p_child)) {
>>> > - DRM_ERROR("child_device_config cannot fit in p_child\n");
>>> > + DRM_ERROR("child_device_config (size %u) cannot fit in p_child 
>>> > (size %u); bdb->version: %u\n",
>>> > +   expected_size, sizeof(*p_child), bdb->version);
>>> 
>>> drivers/gpu/drm/i915/intel_bios.c: In function ‘parse_device_mapping’:
>>> drivers/gpu/drm/i915/intel_bios.c:1058:3: warning: format ‘%u’ expects 
>>> argument of type ‘unsigned int’, but argument 3 has type ‘long unsigned 
>>> int’ [-Wformat=]
>>>DRM_ERROR("child_device_config (size %u) cannot fit in p_child (size 
>>> %u); bdb->version: %u\n",
>>>^
>>>   CC [M]  drivers/gpu/drm/i915/intel_fifo_underrun.o
>>
>> Well spotted.  Or rather, stupid of me to forget that sizeof yields
>> size_t as type, thus necessitating %zu as conversion specifier.
>>
>> Fixed version:
>>
>> Expand common_child_dev_config to be able to fit all information
>> defined by the latest VBT specification.
>>
>> v2: Use proper conversion specifier for size_t (%zu)
>>
>> Signed-off-by: David Weinehall 
>> CC: Antti Koskipaa 
>>
>>  intel_bios.c |7 ++-
>>  intel_bios.h |4 
>>  2 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c 
>> b/drivers/gpu/drm/i915/intel_bios.c
>> index 990acc20771a..5673ed53797b 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -1038,6 +1038,10 @@ parse_device_mapping(struct drm_i915_private 
>> *dev_priv,
>>  DRM_DEBUG_KMS("No general definition block is found, no devices 
>> defined.\n");
>>  return;
>>  }
>> +/* Remember to keep this in sync with child_device_config;
>> + * whenever a new feature is added to BDB that causes that
>> + * struct to grow this needs to be updated too
>> + */
>>  if (bdb->version < 195) {
>>  expected_size = 33;
>>  } else if (bdb->version == 195) {
>> @@ -1051,7 +1055,8 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
>>  }
>>  
>>  if (expected_size > sizeof(*p_child)) {
>> -DRM_ERROR("child_device_config cannot fit in p_child\n");
>> +DRM_ERROR("child_device_config (size %u) cannot fit in p_child 
>> (size %zu); bdb->version: %u\n",
>> +  expected_size, sizeof(*p_child), bdb->version);
>>  return;
>>  }
>>  
>> diff --git a/drivers/gpu/drm/i915/intel_bios.h 
>> b/drivers/gpu/drm/i915/intel_bios.h
>> index f7ad6a585129..71cb96f77870 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.h
>> +++ b/drivers/gpu/drm/i915/intel_bios.h
>> @@ -239,6 +239,10 @@ 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;
>
> How does this impact the if (p_defs->child_dev_size != sizeof(*p_child))
> check in parse_sdvo_device_mapping()?

Ugh. What a mess the whole child device parsing is.

What's really broken in the current code is *assuming* some expected
size for future v

Re: [Intel-gfx] [PATCH] drm/i915: Treat foreign dma-buf imports as uncached

[shuang . he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: 
shuang...@intel.com)
Task id: 7140
-Summary-
Platform  Delta  drm-intel-nightly  Series Applied
ILK  302/302  302/302
SNB  315/315  315/315
IVB  336/336  336/336
BYT  283/283  283/283
HSW  378/378  378/378
-Detailed-
Platform  Testdrm-intel-nightly  Series 
Applied
Note: You need to pay more attention to line start with '*'
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] ACPI / video: Fix circular lock dependency issue in the video-detect code

[Rafael J. Wysocki]

On Friday, August 14, 2015 11:36:41 AM Sedat Dilek wrote:
> On Fri, Aug 14, 2015 at 11:39 AM, Rafael J. Wysocki  
> wrote:
> > On Thursday, August 13, 2015 10:39:08 PM Sedat Dilek wrote:
> >>
> >> --f46d04447e7fc2306e051d3753a5
> >> Content-Type: text/plain; charset=UTF-8
> >>
> >> On Thu, Aug 13, 2015 at 6:53 PM, Hans de Goede  wrote:
> >> > Before this commit, the following would happen:
> >> >
> >> > a) acpi_video_get_backlight_type() gets called
> >> > b) acpi_video_get_backlight_type() calls acpi_video_init_backlight_type()
> >> > c) acpi_video_init_backlight_type() locks its function static init_mutex
> >> > d) acpi_video_init_backlight_type() calls backlight_register_notifier()
> >> > e) backlight_register_notifier() takes its notifier-chain lock
> >> >
> >> > And when the backlight notifier chain gets called we've:
> >> >
> >> > 1) blocking_notifier_call_chain() gets called
> >> > 2) blocking_notifier_call_chain() takes the notifier-chain lock
> >> > 3) blocking_notifier_call_chain() calls acpi_video_backlight_notify()
> >> > 4) acpi_video_backlight_notify() calls acpi_video_get_backlight_type()
> >> > 5) acpi_video_get_backlight_type() calls acpi_video_init_backlight_type()
> >> > 6) acpi_video_init_backlight_type() locks its function static init_mutex
> >> >
> >> > So in the first call sequence we have:
> >> >
> >> > a) init_mutex gets locked
> >> > b) notifier-chain gets locked
> >> >
> >> > and in the second call sequence we have:
> >> >
> >> > 1) notifier-chain gets locked
> >> > 2) init_mutex gets locked
> >> >
> >> > And we've a circular locking dependency. This specific locking dependency
> >> > is fixable without using the big hammer otherwise known as a workqueue,
> >> > but further analysis shows a similar problem with the backlight notifier
> >> > chain lock vs register_count_mutex from drivers/acpi/acpi_video.c,
> >> > and fixing that becomes problematic.
> >> >
> >> > So this commit simply fixes this with the big hammer, performance
> >> > wise this is a non issue as we expect the work to get scheduled
> >> > exactly zero or one times during normal system use.
> >> >
> >>
> >> This patch on top of Linux v4.2-rc6 fixes the issue for me.
> >>
> >> Feel free to add my...
> >>
> >>Reported-by: Sedat Dilek 
> >>Tested-by: Sedat Dilek 
> >
> > Applied, thanks!
> >
> 
> Thanks for carrying this one.
> Isn't this a for-4.2/acpi-video-fixes?

I have no branch like that.

> Why did you apply this in your linux-next Git branch?

Because that's what I do normally with patches I'm going to push to Linus.

Thanks,
Rafael

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

Re: [Intel-gfx] [PATCH] ACPI / video: Fix circular lock dependency issue in the video-detect code

[Sedat Dilek]

On Fri, Aug 14, 2015 at 12:16 PM, Rafael J. Wysocki  wrote:
> On Friday, August 14, 2015 11:36:41 AM Sedat Dilek wrote:
>> On Fri, Aug 14, 2015 at 11:39 AM, Rafael J. Wysocki  
>> wrote:
>> > On Thursday, August 13, 2015 10:39:08 PM Sedat Dilek wrote:
>> >>
>> >> --f46d04447e7fc2306e051d3753a5
>> >> Content-Type: text/plain; charset=UTF-8
>> >>
>> >> On Thu, Aug 13, 2015 at 6:53 PM, Hans de Goede  
>> >> wrote:
>> >> > Before this commit, the following would happen:
>> >> >
>> >> > a) acpi_video_get_backlight_type() gets called
>> >> > b) acpi_video_get_backlight_type() calls 
>> >> > acpi_video_init_backlight_type()
>> >> > c) acpi_video_init_backlight_type() locks its function static init_mutex
>> >> > d) acpi_video_init_backlight_type() calls backlight_register_notifier()
>> >> > e) backlight_register_notifier() takes its notifier-chain lock
>> >> >
>> >> > And when the backlight notifier chain gets called we've:
>> >> >
>> >> > 1) blocking_notifier_call_chain() gets called
>> >> > 2) blocking_notifier_call_chain() takes the notifier-chain lock
>> >> > 3) blocking_notifier_call_chain() calls acpi_video_backlight_notify()
>> >> > 4) acpi_video_backlight_notify() calls acpi_video_get_backlight_type()
>> >> > 5) acpi_video_get_backlight_type() calls 
>> >> > acpi_video_init_backlight_type()
>> >> > 6) acpi_video_init_backlight_type() locks its function static init_mutex
>> >> >
>> >> > So in the first call sequence we have:
>> >> >
>> >> > a) init_mutex gets locked
>> >> > b) notifier-chain gets locked
>> >> >
>> >> > and in the second call sequence we have:
>> >> >
>> >> > 1) notifier-chain gets locked
>> >> > 2) init_mutex gets locked
>> >> >
>> >> > And we've a circular locking dependency. This specific locking 
>> >> > dependency
>> >> > is fixable without using the big hammer otherwise known as a workqueue,
>> >> > but further analysis shows a similar problem with the backlight notifier
>> >> > chain lock vs register_count_mutex from drivers/acpi/acpi_video.c,
>> >> > and fixing that becomes problematic.
>> >> >
>> >> > So this commit simply fixes this with the big hammer, performance
>> >> > wise this is a non issue as we expect the work to get scheduled
>> >> > exactly zero or one times during normal system use.
>> >> >
>> >>
>> >> This patch on top of Linux v4.2-rc6 fixes the issue for me.
>> >>
>> >> Feel free to add my...
>> >>
>> >>Reported-by: Sedat Dilek 
>> >>Tested-by: Sedat Dilek 
>> >
>> > Applied, thanks!
>> >
>>
>> Thanks for carrying this one.
>> Isn't this a for-4.2/acpi-video-fixes?
>
> I have no branch like that.
>

I wanted to say it's "for 4.2-rc7".

>> Why did you apply this in your linux-next Git branch?
>
> Because that's what I do normally with patches I'm going to push to Linus.
>

OK, thanks.

- sed@ -

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

[Intel-gfx] [PULL] topic/drm-fixes

[Jani Nikula]

Hi Dave, here's a DP MST fix from Maarten.

BR,
Jani.

The following changes since commit 5677d67ae3949f09f57357241b88222d49b8c782:

  drm: Stop resetting connector state to unknown (2015-07-22 14:52:26 +0200)

are available in the git repository at:

  git://anongit.freedesktop.org/drm-intel tags/topic/drm-fixes-2015-08-14

for you to fetch changes up to 4772ff03df8094fd99d28de5fcf5df3a3e9c68bb:

  drm/dp/mst: Remove port after removing connector. (2015-08-11 12:30:38 +0300)


Maarten Lankhorst (1):
  drm/dp/mst: Remove port after removing connector.

 drivers/gpu/drm/drm_dp_mst_topology.c | 19 +--
 include/drm/drm_crtc.h|  2 --
 2 files changed, 13 insertions(+), 8 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

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

[Jani Nikula]

Hi Dave, some i915 display fixes. A couple of backports by Maarten from
our -next to v4.2, and a dithering fix from Daniel.

BR,
Jani.

The following changes since commit f7644cbfcdf03528f0f450f3940c4985b2291f49:

  Linux 4.2-rc6 (2015-08-09 15:54:30 -0400)

are available in the git repository at:

  git://anongit.freedesktop.org/drm-intel tags/drm-intel-fixes-2015-08-14

for you to fetch changes up to d2944cf21305c754fa8b2d6c1eea05ad5dad7944:

  drm/i915: Commit planes on each crtc separately. (2015-08-13 12:09:18 +0300)


Daniel Vetter (1):
  drm/i915: Only dither on 6bpc panels

Maarten Lankhorst (2):
  drm/i915: calculate primary visibility changes instead of calling from 
set_config
  drm/i915: Commit planes on each crtc separately.

 drivers/gpu/drm/i915/intel_atomic.c  | 45 +--
 drivers/gpu/drm/i915/intel_display.c | 59 +---
 2 files changed, 23 insertions(+), 81 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

[Intel-gfx] [PATCH v4 01/11] drm/i915: Store max dotclock

[Mika Kahola]

Store max dotclock into dev_priv structure so we are able
to filter out the modes that are not supported by our
platforms.

V2:
- limit the max dot clock frequency to max CD clock frequency
  for the gen9 and above
- limit the max dot clock frequency to 90% of the max CD clock
  frequency for the older gens
- for Cherryview the max dot clock frequency is limited to 95%
  of the max CD clock frequency
- for gen2 and gen3 the max dot clock limit is set to 90% of the
  2X max CD clock frequency

V3:
- max_dotclk variable renamed as max_dotclk_freq in i915_drv.h
- in intel_compute_max_dotclk() the rounding method changed from
  round up to round down when computing max dotclock

Signed-off-by: Mika Kahola 
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_display.c | 19 +++
 2 files changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 55611d8..e1910ec 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1787,6 +1787,7 @@ struct drm_i915_private {
unsigned int fsb_freq, mem_freq, is_ddr3;
unsigned int skl_boot_cdclk;
unsigned int cdclk_freq, max_cdclk_freq;
+   unsigned int max_dotclk_freq;
unsigned int hpll_freq;
 
/**
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 21aa745..e8d8860 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5275,6 +5275,20 @@ static void modeset_update_crtc_power_domains(struct 
drm_atomic_state *state)
modeset_put_power_domains(dev_priv, put_domains[i]);
 }
 
+static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
+{
+   int max_cdclk_freq = dev_priv->max_cdclk_freq;
+
+   if (INTEL_INFO(dev_priv)->gen >= 9)
+   return max_cdclk_freq;
+   else if (IS_CHERRYVIEW(dev_priv))
+   return max_cdclk_freq*95/100;
+   else if (INTEL_INFO(dev_priv)->gen < 4)
+   return 2*max_cdclk_freq*90/100;
+   else
+   return max_cdclk_freq*90/100;
+}
+
 static void intel_update_max_cdclk(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -5314,8 +5328,13 @@ static void intel_update_max_cdclk(struct drm_device 
*dev)
dev_priv->max_cdclk_freq = dev_priv->cdclk_freq;
}
 
+   dev_priv->max_dotclk_freq = intel_compute_max_dotclk(dev_priv);
+
DRM_DEBUG_DRIVER("Max CD clock rate: %d kHz\n",
 dev_priv->max_cdclk_freq);
+
+   DRM_DEBUG_DRIVER("Max dotclock rate: %d kHz\n",
+dev_priv->max_dotclk_freq);
 }
 
 static void intel_update_cdclk(struct drm_device *dev)
-- 
1.9.1

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

[Intel-gfx] [PATCH v4 02/11] drm/i915: DisplayPort pixel clock check

[Mika Kahola]

It is possible the we request to have a mode that has
higher pixel clock than our HW can support. This patch
checks if requested pixel clock is lower than the one
supported by the HW. The requested mode is discarded
if we cannot support the requested pixel clock.

This patch applies to DisplayPort.

V2:
- removed computation for max DOT clock

V3:
- cleanup by removing unnecessary lines

V4:
- max_pixclk renamed as max_dotclk

Signed-off-by: Mika Kahola 
---
 drivers/gpu/drm/i915/intel_dp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a1dac9c..f4fad4c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -206,6 +206,7 @@ intel_dp_mode_valid(struct drm_connector *connector,
struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
int target_clock = mode->clock;
int max_rate, mode_rate, max_lanes, max_link_clock;
+   int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
 
if (is_edp(intel_dp) && fixed_mode) {
if (mode->hdisplay > fixed_mode->hdisplay)
@@ -223,7 +224,7 @@ intel_dp_mode_valid(struct drm_connector *connector,
max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes);
mode_rate = intel_dp_link_required(target_clock, 18);
 
-   if (mode_rate > max_rate)
+   if (mode_rate > max_rate || target_clock > max_dotclk)
return MODE_CLOCK_HIGH;
 
if (mode->clock < 1)
-- 
1.9.1

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

[Intel-gfx] [PATCH v4 06/11] drm/i915: DSI pixel clock check

[Mika Kahola]

It is possible the we request to have a mode that has
higher pixel clock than our HW can support. This patch
checks if requested pixel clock is lower than the one
supported by the HW. The requested mode is discarded
if we cannot support the requested pixel clock.

This patch applies to DSI.

V2:
- removed computation for max pixel clock

V3:
- cleanup by removing unnecessary lines

V4:
- max_pixclk variable renamed as max_dotclk
- moved dot clock checking inside 'if (fixed_mode)'

Signed-off-by: Mika Kahola 
---
 drivers/gpu/drm/i915/intel_dsi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 4a601cf..00c6b1f 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -654,6 +654,7 @@ intel_dsi_mode_valid(struct drm_connector *connector,
 {
struct intel_connector *intel_connector = to_intel_connector(connector);
struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
+   int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
 
DRM_DEBUG_KMS("\n");
 
@@ -667,6 +668,8 @@ intel_dsi_mode_valid(struct drm_connector *connector,
return MODE_PANEL;
if (mode->vdisplay > fixed_mode->vdisplay)
return MODE_PANEL;
+   if (mode->clock > max_dotclk)
+   return MODE_CLOCK_HIGH;
}
 
return MODE_OK;
-- 
1.9.1

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

[Intel-gfx] [PATCH v4 04/11] drm/i915: LVDS pixel clock check

[Mika Kahola]

It is possible the we request to have a mode that has
higher pixel clock than our HW can support. This patch
checks if requested pixel clock is lower than the one
supported by the HW. The requested mode is discarded
if we cannot support the requested pixel clock.

This patch applies to LVDS.

V2:
- removed computation for max pixel clock

V3:
- cleanup by removing unnecessary lines

V4:
- moved supported dotclock check from mode_valid() to intel_lvds_init()

Signed-off-by: Mika Kahola 
---
 drivers/gpu/drm/i915/intel_lvds.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c 
b/drivers/gpu/drm/i915/intel_lvds.c
index 881b5d1..06b9d1b 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -1098,8 +1098,11 @@ void intel_lvds_init(struct drm_device *dev)
drm_mode_debug_printmodeline(scan);
 
fixed_mode = drm_mode_duplicate(dev, scan);
-   if (fixed_mode)
-   goto out;
+   if (fixed_mode) {
+   if (fixed_mode->clock <= 
dev_priv->max_dotclk_freq)
+   goto out;
+   }
+   fixed_mode = NULL;
}
}
 
@@ -,8 +1114,10 @@ void intel_lvds_init(struct drm_device *dev)
fixed_mode = drm_mode_duplicate(dev, 
dev_priv->vbt.lfp_lvds_vbt_mode);
if (fixed_mode) {
fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
-   goto out;
+   if (fixed_mode->clock <= dev_priv->max_dotclk_freq)
+   goto out;
}
+   fixed_mode = NULL;
}
 
/*
@@ -1135,8 +1140,10 @@ void intel_lvds_init(struct drm_device *dev)
DRM_DEBUG_KMS("using current (BIOS) mode: ");
drm_mode_debug_printmodeline(fixed_mode);
fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
-   goto out;
+   if (fixed_mode->clock <= dev_priv->max_dotclk_freq)
+   goto out;
}
+   fixed_mode = NULL;
}
 
/* If we still don't have a mode after all that, give up. */
-- 
1.9.1

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

[Intel-gfx] [PATCH v4 03/11] drm/i915: HDMI pixel clock check

[Mika Kahola]

It is possible the we request to have a mode that has
higher pixel clock than our HW can support. This patch
checks if requested pixel clock is lower than the one
supported by the HW. The requested mode is discarded
if we cannot support the requested pixel clock.

This patch applies to HDMI.

V2:
- removed computation for max dot clock

V3:
- cleanup by removing unnecessary lines

V4:
- max_pixclk variable renamed as max_dotclk
- check for stereo mode added

Signed-off-by: Mika Kahola 
---
 drivers/gpu/drm/i915/intel_hdmi.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index 0cfbe85..e4b35dc 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1193,11 +1193,19 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
struct drm_device *dev = intel_hdmi_to_dev(hdmi);
enum drm_mode_status status;
int clock;
+   int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
 
if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
return MODE_NO_DBLESCAN;
 
clock = mode->clock;
+
+   if ((mode->flags & DRM_MODE_FLAG_3D_MASK) == 
DRM_MODE_FLAG_3D_FRAME_PACKING)
+   clock *= 2;
+
+   if (clock > max_dotclk)
+   return MODE_CLOCK_HIGH;
+
if (mode->flags & DRM_MODE_FLAG_DBLCLK)
clock *= 2;
 
-- 
1.9.1

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

[Intel-gfx] [PATCH v4 00/11] Check pixel clock when setting mode

[Mika Kahola]

From EDID we can read and request higher pixel clock than
our HW can support. This set of patches add checks if
requested pixel clock is lower than the one supported by the HW.
The requested mode is discarded if we cannot support the requested
pixel clock. For example for Cherryview

'cvt 2560 1600 60' gives

# 2560x1600 59.99 Hz (CVT 4.10MA) hsync: 99.46 kHz; pclk: 348.50 MHz
Modeline "2560x1600_60.00"  348.50  2560 2760 3032 3504  1600 1603 1609 1658 
-hsync +vsync

where pixel clock 348.50 MHz is higher than the supported 304 MHz.

The checks are implemented for DisplayPort, HDMI, LVDS, DVO, SDVO, DSI,
CRT, TV, and DP-MST.

V2:
- The maximum DOT clock frequency is added to debugfs i915_frequency_info.
- max dotclock cached in dev_priv structure
- moved computation of max dotclock to 'intel_display.c'

V3:
- intel_update_max_dotclk() renamed as intel_compute_max_dotclk()
- for GEN9 and above the max dotclock frequency is equal to CD clock
  frequency
- for older generations the dot clock frequency is limited to 90% of the
  CD clock frequency
- For Cherryview the dot clock is limited to 95% of CD clock frequency
- for GEN2/3 the maximum dot clock frequency is limited to 90% of the
  2X CD clock frequency as we have on option to use double wide mode
- cleanup

V4:
- renaming of max_dotclk as max_dotclk_freq in dev_priv (i915_drv.h)
  caused changes to all patches in my series even though some of them has
  been r-b'd by Ville
- for consistency the max_pixclk variable is renamed as max_dotclk throughout
  the whole series

Mika Kahola (11):
  drm/i915: Store max dotclock
  drm/i915: DisplayPort pixel clock check
  drm/i915: HDMI pixel clock check
  drm/i915: LVDS pixel clock check
  drm/i915: SDVO pixel clock check
  drm/i915: DSI pixel clock check
  drm/i915: CRT pixel clock check
  drm/i915: TV pixel clock check
  drm/i915: DisplayPort-MST pixel clock check
  drm/i915: DVO pixel clock check
  drm/i915: Max DOT clock frequency to debugfs

 drivers/gpu/drm/i915/i915_debugfs.c  |  2 ++
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_crt.c |  4 
 drivers/gpu/drm/i915/intel_display.c | 19 +++
 drivers/gpu/drm/i915/intel_dp.c  |  3 ++-
 drivers/gpu/drm/i915/intel_dp_mst.c  |  5 +
 drivers/gpu/drm/i915/intel_dsi.c |  3 +++
 drivers/gpu/drm/i915/intel_dvo.c |  3 +++
 drivers/gpu/drm/i915/intel_hdmi.c|  8 
 drivers/gpu/drm/i915/intel_lvds.c| 15 +++
 drivers/gpu/drm/i915/intel_sdvo.c|  4 
 drivers/gpu/drm/i915/intel_tv.c  |  4 
 12 files changed, 66 insertions(+), 5 deletions(-)

-- 
1.9.1

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

[Intel-gfx] [PATCH v4 07/11] drm/i915: CRT pixel clock check

[Mika Kahola]

It is possible the we request to have a mode that has
higher pixel clock than our HW can support. This patch
checks if requested pixel clock is lower than the one
supported by the HW. The requested mode is discarded
if we cannot support the requested pixel clock.

This patch applies to CRT.

V2:
- removed computation for max pixel clock

V3:
- cleanup by removing unnecessary lines

V4:
- max_pixclk variable renamed as max_dotclk

Signed-off-by: Mika Kahola 
---
 drivers/gpu/drm/i915/intel_crt.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index af5e43b..016ca46 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -241,6 +241,7 @@ intel_crt_mode_valid(struct drm_connector *connector,
 struct drm_display_mode *mode)
 {
struct drm_device *dev = connector->dev;
+   int max_dotclk = to_i915(dev)->max_dotclk_freq;
 
int max_clock = 0;
if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
@@ -256,6 +257,9 @@ intel_crt_mode_valid(struct drm_connector *connector,
if (mode->clock > max_clock)
return MODE_CLOCK_HIGH;
 
+   if (mode->clock > max_dotclk)
+   return MODE_CLOCK_HIGH;
+
/* The FDI receiver on LPT only supports 8bpc and only has 2 lanes. */
if (HAS_PCH_LPT(dev) &&
(ironlake_get_lanes_required(mode->clock, 27, 24) > 2))
-- 
1.9.1

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

[Intel-gfx] [PATCH v4 11/11] drm/i915: Max DOT clock frequency to debugfs

[Mika Kahola]

Information on maximum supported DOT clock frequency to
i915_frequency_info.

Signed-off-by: Mika Kahola 
---
 drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 86734be..3df7492 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1314,6 +1314,8 @@ static int i915_frequency_info(struct seq_file *m, void 
*unused)
seq_puts(m, "no P-state info available\n");
}
 
+   seq_printf(m, "Max pixel clock frequency: %dkHz\n", 
dev_priv->max_dotclk_freq);
+
 out:
intel_runtime_pm_put(dev_priv);
return ret;
-- 
1.9.1

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

[Intel-gfx] [PATCH v4 05/11] drm/i915: SDVO pixel clock check

[Mika Kahola]

It is possible the we request to have a mode that has
higher pixel clock than our HW can support. This patch
checks if requested pixel clock is lower than the one
supported by the HW. The requested mode is discarded
if we cannot support the requested pixel clock.

This patch applies to SDVO.

V2:
- removed computation for max pixel clock

V3:
- cleanup by removing unnecessary lines

V4:
- max_pixclk variable renamed as max_dotclk

Signed-off-by: Mika Kahola 
---
 drivers/gpu/drm/i915/intel_sdvo.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c 
b/drivers/gpu/drm/i915/intel_sdvo.c
index c98098e..ba00be8 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1513,6 +1513,7 @@ intel_sdvo_mode_valid(struct drm_connector *connector,
  struct drm_display_mode *mode)
 {
struct intel_sdvo *intel_sdvo = intel_attached_sdvo(connector);
+   int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
 
if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
return MODE_NO_DBLESCAN;
@@ -1523,6 +1524,9 @@ intel_sdvo_mode_valid(struct drm_connector *connector,
if (intel_sdvo->pixel_clock_max < mode->clock)
return MODE_CLOCK_HIGH;
 
+   if (mode->clock > max_dotclk)
+   return MODE_CLOCK_HIGH;
+
if (intel_sdvo->is_lvds) {
if (mode->hdisplay > intel_sdvo->sdvo_lvds_fixed_mode->hdisplay)
return MODE_PANEL;
-- 
1.9.1

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

[Intel-gfx] [PATCH v4 08/11] drm/i915: TV pixel clock check

[Mika Kahola]

It is possible the we request to have a mode that has
higher pixel clock than our HW can support. This patch
checks if requested pixel clock is lower than the one
supported by the HW. The requested mode is discarded
if we cannot support the requested pixel clock.

This patch applies to TV.

V2:
- removed computation for max pixel clock

V3:
- cleanup by removing unnecessary lines

V4:
- max_pixclk variable renamed as max_dotclk

Signed-off-by: Mika Kahola 
---
 drivers/gpu/drm/i915/intel_tv.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 0568ae6..62ae1f1 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -897,6 +897,10 @@ intel_tv_mode_valid(struct drm_connector *connector,
 {
struct intel_tv *intel_tv = intel_attached_tv(connector);
const struct tv_mode *tv_mode = intel_tv_mode_find(intel_tv);
+   int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
+
+   if (mode->clock > max_dotclk)
+   return MODE_CLOCK_HIGH;
 
/* Ensure TV refresh is close to desired refresh */
if (tv_mode && abs(tv_mode->refresh - drm_mode_vrefresh(mode) * 1000)
-- 
1.9.1

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

[Intel-gfx] [PATCH v4 10/11] drm/i915: DVO pixel clock check

[Mika Kahola]

It is possible the we request to have a mode that has
higher pixel clock than our HW can support. This patch
checks if requested pixel clock is lower than the one
supported by the HW. The requested mode is discarded
if we cannot support the requested pixel clock.

This patch applies to DVO.

V2:
- removed computation for max pixel clock

V3:
- cleanup by removing unnecessary lines

V4:
- clock check against max dotclock moved inside 'if (fixed_mode)'

Signed-off-by: Mika Kahola 
---
 drivers/gpu/drm/i915/intel_dvo.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index dc532bb..924b724 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -201,6 +201,7 @@ intel_dvo_mode_valid(struct drm_connector *connector,
 struct drm_display_mode *mode)
 {
struct intel_dvo *intel_dvo = intel_attached_dvo(connector);
+   int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
 
if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
return MODE_NO_DBLESCAN;
@@ -212,6 +213,8 @@ intel_dvo_mode_valid(struct drm_connector *connector,
return MODE_PANEL;
if (mode->vdisplay > intel_dvo->panel_fixed_mode->vdisplay)
return MODE_PANEL;
+   if (mode->clock > max_dotclk)
+   return MODE_CLOCK_HIGH;
}
 
return intel_dvo->dev.dev_ops->mode_valid(&intel_dvo->dev, mode);
-- 
1.9.1

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

[Intel-gfx] [PATCH v4 09/11] drm/i915: DisplayPort-MST pixel clock check

[Mika Kahola]

It is possible the we request to have a mode that has
higher pixel clock than our HW can support. This patch
checks if requested pixel clock is lower than the one
supported by the HW. The requested mode is discarded
if we cannot support the requested pixel clock.

This patch applies to DisplayPort MST.

V2:
- removed computation for max pixel clock

V3:
- cleanup by removing unnecessary lines

V4:
- max_pixclk variable renamed as max_dotclk

Signed-off-by: Mika Kahola 
---
 drivers/gpu/drm/i915/intel_dp_mst.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
b/drivers/gpu/drm/i915/intel_dp_mst.c
index 369f8b6..d84a837 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -347,6 +347,8 @@ static enum drm_mode_status
 intel_dp_mst_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode)
 {
+   int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
+
/* TODO - validate mode against available PBN for link */
if (mode->clock < 1)
return MODE_CLOCK_LOW;
@@ -354,6 +356,9 @@ intel_dp_mst_mode_valid(struct drm_connector *connector,
if (mode->flags & DRM_MODE_FLAG_DBLCLK)
return MODE_H_ILLEGAL;
 
+   if (mode->clock > max_dotclk)
+   return MODE_CLOCK_HIGH;
+
return MODE_OK;
 }
 
-- 
1.9.1

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

[Intel-gfx] [PATCH] drm/i915: Fix build warning on 32-bit

[Thierry Reding]

From: Thierry Reding 

The gtt.stolen_size field is of type size_t, and so should be printed
using %zu to avoid build warnings on either 32-bit and 64-bit builds.

Signed-off-by: Thierry Reding 
---
 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 a36cb95ec798..f361c4a56995 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -348,7 +348,7 @@ int i915_gem_init_stolen(struct drm_device *dev)
 * memory, so just consider the start. */
reserved_total = stolen_top - reserved_base;
 
-   DRM_DEBUG_KMS("Memory reserved for graphics device: %luK, usable: 
%luK\n",
+   DRM_DEBUG_KMS("Memory reserved for graphics device: %zuK, usable: 
%luK\n",
  dev_priv->gtt.stolen_size >> 10,
  (dev_priv->gtt.stolen_size - reserved_total) >> 10);
 
-- 
2.4.5

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

Re: [Intel-gfx] [PATCH 6/6 v3] drm/i915: Enable HDMI on DDI-E

[Zhang, Xiong Y]

> On Thu, Aug 13, 2015 at 02:57:38AM +, Zhang, Xiong Y wrote:
> > > On Wed, Aug 12, 2015 at 06:39:34PM +0800, Xiong Zhang wrote:
> > > > DDI-E doesn't have the correspondent GMBUS pin.
> > > >
> > > > We rely on VBT to tell us which one it being used instead.
> > > >
> > > > The DVI/HDMI on shared port couldn't exist.
> > > >
> > > > This patch isn't tested without hardware wchich has HDMI on DDI-E.
> > > >
> > > > v2: fix trailing whitespace
> > > > v3: WARN() take place of BUG()
> > >
> > > I asked for MISSING_CASE, not WARN.
> > > -Daniel
> > [Zhang, Xiong Y] Because DDI-E on SKL doesn't have correspondent GMBUS
> pin, it should share such pin with DDI-B/C/D. We don't know the default
> GMBUS pin for DDI-E if VBT doesn't tell us.
> > If VBT don't tell us or give us an invalid GMBUS pin, driver will fail in 
> > getting
> HDMI info. In such case, we really need a warn which tell us why driver
> couldn't read EDID info for HDMI on DDI-E.
> 
> Have you bothered to look at the MISSING_CASE macro?! It does boil down to
> a WARN, but I prefer it much over a WARN_ON since it's nicely standardized.
> 
[Zhang, Xiong Y] Please forgive my innocence! I misunderstand MISSING_CASE at 
the beginning.

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

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Adding break for one case

[Zhang, Xiong Y]

> On Thu, Aug 13, 2015 at 01:37:49PM +0300, Timo Aaltonen wrote:
> > On 13.08.2015 13:36, Timo Aaltonen wrote:
> > > On 13.08.2015 13:00, Xiong Zhang wrote:
> > >> Signed-off-by: Xiong Zhang 
> > >> ---
> > >>  drivers/gpu/drm/i915/intel_display.c | 1 +
> > >>  1 file changed, 1 insertion(+)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/intel_display.c
> > >> b/drivers/gpu/drm/i915/intel_display.c
> > >> index 65cc5b1..801187c 100644
> > >> --- a/drivers/gpu/drm/i915/intel_display.c
> > >> +++ b/drivers/gpu/drm/i915/intel_display.c
> > >> @@ -1100,6 +1100,7 @@ bool ibx_digital_port_connected(struct
> drm_i915_private *dev_priv,
> > >>  break;
> > >>  case PORT_E:
> > >>  bit = SDE_PORTE_HOTPLUG_SPT;
> > >> +break;
> > >>  default:
> > >>  return true;
> > >>  }
> > >>
> > >
> > > shouldn't this belong to [5/6]?
> >
> > Nevermind, I see now that it got merged already.
> 
> I dropped that patch again so that we can rectify this properly. Jani's 
> complaint
> about the sub-par commit message still holds though, like why was this not
> caught in testing?
[Zhang, Xiong Y] Yes, it's better to drop it. I will explain it the commit 
message and resent the patch.
> -Daniel

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

Re: [Intel-gfx] [PATCH] drm/i915: Fix build warning on 32-bit

[Chris Wilson]

On Fri, Aug 14, 2015 at 12:35:23PM +0200, Thierry Reding wrote:
> From: Thierry Reding 
> 
> The gtt.stolen_size field is of type size_t, and so should be printed
> using %zu to avoid build warnings on either 32-bit and 64-bit builds.

Or better would be to convert stolen.size to u32 so that we know that it
is correctly sized for the hardware irrespective of the compilation
environment.
-Chris

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

[Intel-gfx] [PATCH] drm/i915: Flag the execlists context object as dirty after every use

[Chris Wilson]

Everytime we use the logical context with execlists it becomes dirty (as
the hardware will write the new register values afterwards, as well as
the GPU state that will be used). We need to then flag the context as
dirty everytime since after a swap-out/swap-in cycle the dirty flag will
be cleared, and a further swap-out cycle will then loose the most recent
GPU state.

Signed-off-by: Chris Wilson 
Cc: sta...@vger.kernel.org
---
 drivers/gpu/drm/i915/intel_lrc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 138964afd187..41cfa6fa909d 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1013,6 +1013,8 @@ static int intel_lr_context_pin(struct 
drm_i915_gem_request *rq)
ret = intel_pin_and_map_ringbuffer_obj(ring->dev, ringbuf);
if (ret)
goto unpin_ctx_obj;
+
+   ctx_obj->dirty = true;
}
 
return ret;
-- 
2.5.0

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

Re: [Intel-gfx] [PATCH] drm/i915: Flag the execlists context object as dirty after every use

[Daniel Vetter]

On Fri, Aug 14, 2015 at 12:59:19PM +0100, Chris Wilson wrote:
> Everytime we use the logical context with execlists it becomes dirty (as
> the hardware will write the new register values afterwards, as well as
> the GPU state that will be used). We need to then flag the context as
> dirty everytime since after a swap-out/swap-in cycle the dirty flag will
> be cleared, and a further swap-out cycle will then loose the most recent
> GPU state.
> 
> Signed-off-by: Chris Wilson 
> Cc: sta...@vger.kernel.org

Yay for reinventing active tracking I guess, legacy hw ctx has this
already. Reviewed-by: Daniel Vetter 
-Daniel
> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
> b/drivers/gpu/drm/i915/intel_lrc.c
> index 138964afd187..41cfa6fa909d 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1013,6 +1013,8 @@ static int intel_lr_context_pin(struct 
> drm_i915_gem_request *rq)
>   ret = intel_pin_and_map_ringbuffer_obj(ring->dev, ringbuf);
>   if (ret)
>   goto unpin_ctx_obj;
> +
> + ctx_obj->dirty = true;
>   }
>  
>   return ret;
> -- 
> 2.5.0
> 
> ___
> 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 01/18] tests/gem_storedw_loop: add new store_dword test to unify per-ring ones

[Daniel Vetter]

On Thu, Aug 13, 2015 at 01:31:24PM -0700, Jesse Barnes wrote:
> There was a lot of duplication going on...  Mark as basic while we're at
> it as these should never fail.
> 
> Signed-off-by: Jesse Barnes 
> ---
>  tests/Makefile.sources   |   1 +
>  tests/gem_storedw_loop.c | 181 
> +++
>  2 files changed, 182 insertions(+)
>  create mode 100644 tests/gem_storedw_loop.c
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index b9a4cb4..cdcee33 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -138,6 +138,7 @@ TESTS_progs = \
>   gem_seqno_wrap \
>   gem_set_tiling_vs_gtt \
>   gem_set_tiling_vs_pwrite \
> + gem_storedw_loop \
>   gem_storedw_loop_blt \
>   gem_storedw_loop_bsd \
>   gem_storedw_loop_render \

Why not remove the old ones while at it? This just means more gunk in the
overall igt set. Also please update .gitignore here for these ...

> diff --git a/tests/gem_storedw_loop.c b/tests/gem_storedw_loop.c
> new file mode 100644
> index 000..e4ed35d
> --- /dev/null
> +++ b/tests/gem_storedw_loop.c
> @@ -0,0 +1,181 @@
> +/*
> + * Copyright © 2009 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *Eric Anholt 
> + *Jesse Barnes  (based on gem_bad_blit.c)
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "drm.h"
> +#include "ioctl_wrappers.h"
> +#include "drmtest.h"
> +#include "intel_bufmgr.h"
> +#include "intel_batchbuffer.h"
> +#include "intel_io.h"
> +#include "intel_chipset.h"
> +
> +IGT_TEST_DESCRIPTION("Basic CS check using MI_STORE_DATA_IMM.");
> +
> +#define LOCAL_I915_EXEC_VEBOX (4<<0)
> +
> +static drm_intel_bufmgr *bufmgr;
> +struct intel_batchbuffer *batch;
> +static drm_intel_bo *target_buffer;
> +static int has_ppgtt = 0;
> +static int devid;
> +
> +/*
> + * Testcase: Basic bsd MI check using MI_STORE_DATA_IMM
> + */
> +
> +static void
> +emit_store_dword_imm(drm_intel_bo *dest, uint32_t val)
> +{
> + int cmd;
> + cmd = MI_STORE_DWORD_IMM;
> + if (!has_ppgtt)
> + cmd |= MI_MEM_VIRTUAL;
> +
> + BEGIN_BATCH(4, 0);
> + OUT_BATCH(cmd);
> + if (batch->gen >= 8) {
> + OUT_RELOC(dest, I915_GEM_DOMAIN_INSTRUCTION,
> +   I915_GEM_DOMAIN_INSTRUCTION, 0);
> + OUT_BATCH(val);
> + } else {
> + OUT_BATCH(0); /* reserved */
> + OUT_RELOC(dest, I915_GEM_DOMAIN_INSTRUCTION,
> +   I915_GEM_DOMAIN_INSTRUCTION, 0);
> + OUT_BATCH(val);
> + }
> + ADVANCE_BATCH();
> +}
> +
> +static void
> +store_dword_loop(int ring, int count, int divider)
> +{
> + int i, val = 0;
> + uint32_t *buf;
> +
> + igt_info("running storedw loop on render with stall every %i batch\n", 
> divider);
> +
> + for (i = 0; i < SLOW_QUICK(0x2000, 0x10); i++) {
> + emit_store_dword_imm(target_buffer, val);
> + intel_batchbuffer_flush_on_ring(batch, ring);
> +
> + if (i % divider != 0)
> + goto cont;
> +
> + drm_intel_bo_map(target_buffer, 0);
> +
> + buf = target_buffer->virtual;
> + igt_assert_f(buf[0] == val,
> +  "value mismatch: cur 0x%08x, stored 0x%08x\n",
> +  buf[0], val);
> +
> + drm_intel_bo_unmap(target_buffer);
> +
> +cont:
> + val++;
> + }
> +
> + drm_intel_bo_map(target_buffer, 0);
> + buf = target_buffer->virtual;
> +
> + igt_info("completed %d writes successfully, current value: 0x%08x\n", i,
> + buf[0]);
> + drm_intel_bo_unmap(target_buffer);
> +}
> +
> +static void
> +store_test(int ring, int count)
> +{
> + drm_inte

Re: [Intel-gfx] [PATCH 02/18] tests/drv_module_reload: rename drv_module_reload to include in BATs

[Daniel Vetter]

On Thu, Aug 13, 2015 at 01:31:25PM -0700, Jesse Barnes wrote:
> Signed-off-by: Jesse Barnes 

Absolutely-Acked-by: Daniel Vetter 

btw this will cause lots of hilarity because of lockdep and vt and shit
like that. We probably need to grill gregkh a bit more with patches he's
not merging ...
-Daniel

> ---
>  tests/Makefile.sources|  2 +-
>  tests/drv_module_reload   | 60 
> ---
>  tests/drv_module_reload_basic | 60 
> +++
>  3 files changed, 61 insertions(+), 61 deletions(-)
>  delete mode 100755 tests/drv_module_reload
>  create mode 100755 tests/drv_module_reload_basic
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index cdcee33..4fe7d4c 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -178,7 +178,7 @@ TESTS_scripts = \
>   debugfs_emon_crash \
>   drv_debugfs_reader \
>   drv_missed_irq_hang \
> - drv_module_reload \
> + drv_module_reload_basic \
>   kms_sysfs_edid_timing \
>   sysfs_l3_parity \
>   test_rte_check \
> diff --git a/tests/drv_module_reload b/tests/drv_module_reload
> deleted file mode 100755
> index bb29a64..000
> --- a/tests/drv_module_reload
> +++ /dev/null
> @@ -1,60 +0,0 @@
> -#!/bin/bash
> -#
> -# Testcase: Reload the drm module
> -#
> -# ... we've broken this way too often :(
> -#
> -
> -SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
> -. $SOURCE_DIR/drm_lib.sh
> -
> -# no other drm service should be running, so we can just unbind
> -
> -# we must kick away fbcon (but only fbcon)
> -for vtcon in /sys/class/vtconsole/vtcon*/ ; do
> - if grep "frame buffer device" $vtcon/name > /dev/null ; then
> - echo unbinding $vtcon: `cat $vtcon/name`
> - echo 0 > $vtcon/bind
> - fi
> -done
> -
> -# The sound driver uses our power well
> -pkill alsactl
> -rmmod snd_hda_intel &> /dev/null
> -
> -#ignore errors in ips - gen5 only
> -rmmod intel_ips &> /dev/null
> -rmmod i915
> -#ignore errors in intel-gtt, often built-in
> -rmmod intel-gtt &> /dev/null
> -# drm may be used by other devices (nouveau, radeon, udl, etc)
> -rmmod drm_kms_helper &> /dev/null
> -rmmod drm &> /dev/null
> -
> -if lsmod | grep i915 &> /dev/null ; then
> - echo WARNING: i915.ko still loaded!
> - exit 1
> -else
> - echo module successfully unloaded
> -fi
> -
> -modprobe i915
> -echo 1 > /sys/class/vtconsole/vtcon1/bind
> -
> -modprobe snd_hda_intel
> -
> -# does the device exist?
> -if $SOURCE_DIR/gem_alive > /dev/null ; then
> - echo "module successfully loaded again"
> -else
> - echo "failed to reload module successfully"
> - exit 2
> -fi
> -
> -# then try to run something
> -if ! $SOURCE_DIR/gem_exec_nop > /dev/null ; then
> - echo "failed to execute a simple batch after reload"
> - exit 3
> -fi
> -
> -exit 0
> diff --git a/tests/drv_module_reload_basic b/tests/drv_module_reload_basic
> new file mode 100755
> index 000..bb29a64
> --- /dev/null
> +++ b/tests/drv_module_reload_basic
> @@ -0,0 +1,60 @@
> +#!/bin/bash
> +#
> +# Testcase: Reload the drm module
> +#
> +# ... we've broken this way too often :(
> +#
> +
> +SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
> +. $SOURCE_DIR/drm_lib.sh
> +
> +# no other drm service should be running, so we can just unbind
> +
> +# we must kick away fbcon (but only fbcon)
> +for vtcon in /sys/class/vtconsole/vtcon*/ ; do
> + if grep "frame buffer device" $vtcon/name > /dev/null ; then
> + echo unbinding $vtcon: `cat $vtcon/name`
> + echo 0 > $vtcon/bind
> + fi
> +done
> +
> +# The sound driver uses our power well
> +pkill alsactl
> +rmmod snd_hda_intel &> /dev/null
> +
> +#ignore errors in ips - gen5 only
> +rmmod intel_ips &> /dev/null
> +rmmod i915
> +#ignore errors in intel-gtt, often built-in
> +rmmod intel-gtt &> /dev/null
> +# drm may be used by other devices (nouveau, radeon, udl, etc)
> +rmmod drm_kms_helper &> /dev/null
> +rmmod drm &> /dev/null
> +
> +if lsmod | grep i915 &> /dev/null ; then
> + echo WARNING: i915.ko still loaded!
> + exit 1
> +else
> + echo module successfully unloaded
> +fi
> +
> +modprobe i915
> +echo 1 > /sys/class/vtconsole/vtcon1/bind
> +
> +modprobe snd_hda_intel
> +
> +# does the device exist?
> +if $SOURCE_DIR/gem_alive > /dev/null ; then
> + echo "module successfully loaded again"
> +else
> + echo "failed to reload module successfully"
> + exit 2
> +fi
> +
> +# then try to run something
> +if ! $SOURCE_DIR/gem_exec_nop > /dev/null ; then
> + echo "failed to execute a simple batch after reload"
> + exit 3
> +fi
> +
> +exit 0
> -- 
> 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
Inte

Re: [Intel-gfx] [PATCH 03/18] tests/drv_module_reload_basic: use linear_blits after module_reload for sanity check

[Daniel Vetter]

On Thu, Aug 13, 2015 at 01:31:26PM -0700, Jesse Barnes wrote:
> Reduces runtime a lot...
> 
> Signed-off-by: Jesse Barnes 
> ---
>  tests/drv_module_reload_basic | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/drv_module_reload_basic b/tests/drv_module_reload_basic
> index bb29a64..cf7f2b8 100755
> --- a/tests/drv_module_reload_basic
> +++ b/tests/drv_module_reload_basic
> @@ -52,7 +52,7 @@ else
>  fi
>  
>  # then try to run something
> -if ! $SOURCE_DIR/gem_exec_nop > /dev/null ; then
> +if ! $SOURCE_DIR/gem_linear_blits --r basic > /dev/null ; then

Please use the full-lenght run-subtest to avoid lots of wtf when we add
another option starting with r eventually. With that fixed:

Reviewed-by: Daniel Vetter 

>   echo "failed to execute a simple batch after reload"
>   exit 3
>  fi
> -- 
> 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 04/18] tests/drm_import_export: mark flink and prime tests as basic

[Daniel Vetter]

On Thu, Aug 13, 2015 at 01:31:27PM -0700, Jesse Barnes wrote:
> They're testing basic functionality and don't involve stress or race
> induction.
> 
> Signed-off-by: Jesse Barnes 

These are more stress-tests in nature I think. For basic testscases of
prime I'd recommend instead

prime_self_import: Everything not marked with *-race

gem_flink: all of them.

instead of this patch here.
-Daniel

> ---
>  tests/drm_import_export.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/drm_import_export.c b/tests/drm_import_export.c
> index e24e0df..6a63ef9 100644
> --- a/tests/drm_import_export.c
> +++ b/tests/drm_import_export.c
> @@ -265,7 +265,7 @@ igt_main {
>   test_import_close_race();
>   }
>  
> - igt_subtest("flink") {
> + igt_subtest("basic-flink") {
>   use_flink = true;
>  
>   pthread_create(&test_thread_id1, NULL, test_thread, NULL);
> @@ -279,7 +279,7 @@ igt_main {
>   pthread_join(test_thread_id4, NULL);
>   }
>  
> - igt_subtest("prime") {
> + igt_subtest("basic-prime") {
>   use_flink = false;
>  
>   pthread_create(&test_thread_id1, NULL, test_thread, NULL);
> -- 
> 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 05/18] tests/drv_getparams: mark EU and subslice fetch as basic

[Daniel Vetter]

On Thu, Aug 13, 2015 at 01:31:28PM -0700, Jesse Barnes wrote:
> Fundamental and simple functionality.
> 
> Signed-off-by: Jesse Barnes 

Mark entire testcase as basic instead to catch future extensions?
getparams should always complete super-fast I think.
-Daniel

> ---
>  tests/drv_getparams.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/drv_getparams.c b/tests/drv_getparams.c
> index 31382e9..5a5c8bb 100644
> --- a/tests/drv_getparams.c
> +++ b/tests/drv_getparams.c
> @@ -157,9 +157,9 @@ igt_main
>   init();
>   }
>  
> - igt_subtest("subslice-total")
> + igt_subtest("basic-subslice-total")
>   subslice_total();
>  
> - igt_subtest("eu-total")
> + igt_subtest("basic-eu-total")
>   eu_total();
>  }
> -- 
> 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 06/18] tests/drv_suspend: mark sysfs tests as basic

[Daniel Vetter]

On Thu, Aug 13, 2015 at 01:31:29PM -0700, Jesse Barnes wrote:
> debugfs may not be mounted, but sysfs should always be restored after
> suspend or hibernate.
> 
> Signed-off-by: Jesse Barnes 

We already have a suspend/resume testcase in kms_pipc_crc_basic. Do we
have enough budget for this one?
> ---
>  tests/drv_suspend.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/drv_suspend.c b/tests/drv_suspend.c
> index d67a794..60ca8e3 100644
> --- a/tests/drv_suspend.c
> +++ b/tests/drv_suspend.c
> @@ -193,7 +193,7 @@ igt_main
>   igt_subtest("debugfs-reader")
>   test_debugfs_reader(false);
>  
> - igt_subtest("sysfs-reader")
> + igt_subtest("basic-sysfs-reader")
>   test_sysfs_reader(false);
>  
>   igt_subtest("forcewake")
> @@ -208,7 +208,7 @@ igt_main
>   igt_subtest("debugfs-reader-hibernate")
>   test_debugfs_reader(true);
>  
> - igt_subtest("sysfs-reader-hibernate")
> + igt_subtest("basic-sysfs-reader-hibernate")
>   test_sysfs_reader(true);

Hibernate is a giantic can of worms. We have a big pile of issues here
that we never fixed. Otoh not testing hibernate is definitely an
oversight. Assuming we have the testing budget:

Acked-by: Daniel Vetter 
>  
>   igt_subtest("forcewake-hibernate")
> -- 
> 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 07/18] tests/gem_ctx_exec: mark lrc lite restore as basic

[Daniel Vetter]

On Thu, Aug 13, 2015 at 01:31:30PM -0700, Jesse Barnes wrote:
> Need some LRC tests in the 'basic' subset, and this is a good simple
> one.
> 
> Signed-off-by: Jesse Barnes 

This is just a testcase for a very specific lrc corner case. We do already
exercise lrc with all the other execbuf testcases. Imo we're covered
enough already with what we have in the basic testset - testing for all 3
billion cornercases will make it grow out of scope I fear. I'd just drop
this one here as not needed for BAT.

If you want to extend execbuffer scope a bit then we should add a
concurrency test, i.e. one of the gem_concurrent_blt testcases as basic
ones. Unfortunately to be able to reliable trigger race conditions those
all take a few seconds. But inter-batch sync is a _big_ gap across all
archs, and something which is even more tricky with lrc (and scheduler).
Imo that would be a lot more useful than this test here.
-Daniel

> ---
>  tests/gem_ctx_exec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
> index 3df939c..ea0fb7f 100644
> --- a/tests/gem_ctx_exec.c
> +++ b/tests/gem_ctx_exec.c
> @@ -216,7 +216,7 @@ igt_main
>   gem_context_destroy(fd, ctx_id);
>   }
>  
> - igt_subtest("lrc-lite-restore") {
> + igt_subtest("basic-lrc-lite-restore") {
>   int i, j;
>  
>   /*
> -- 
> 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 08/18] tests/gem_mmap: mark basic object creation tests as basic

[Daniel Vetter]

On Thu, Aug 13, 2015 at 01:31:31PM -0700, Jesse Barnes wrote:
> We should be able to create small and moderate sized objects quickly and
> without errors.
> 
> Signed-off-by: Jesse Barnes 

They're all super-fast basic testcases for api cornercases. I'd vote to
rename the entire testcase to gem_mmap_basic.
-Daniel

> ---
>  tests/gem_mmap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/gem_mmap.c b/tests/gem_mmap.c
> index 095f5b9..dd66ad6 100644
> --- a/tests/gem_mmap.c
> +++ b/tests/gem_mmap.c
> @@ -118,7 +118,7 @@ igt_main
>   igt_assert(ret == -1 && errno == ENOENT);
>   }
>  
> - igt_subtest("new-object") {
> + igt_subtest("basic") {
>   arg.handle = gem_create(fd, OBJECT_SIZE);
>   arg.offset = 0;
>   arg.size = OBJECT_SIZE;
> @@ -154,7 +154,7 @@ igt_main
>   gem_close(fd, arg.handle);
>   }
>  
> - igt_subtest("small-bo")
> + igt_subtest("basic-small-bo")
>   test_huge_bo(fd, -1);
>   igt_subtest("big-bo")
>   test_huge_bo(fd, 0);
> -- 
> 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 09/18] tests/gem_mmap_gtt: mark basic access and copy tests as basic

[Daniel Vetter]

On Thu, Aug 13, 2015 at 01:31:32PM -0700, Jesse Barnes wrote:
> These ones should always pass and are fairly quick.
> 
> Signed-off-by: Jesse Barnes 
> ---
>  tests/gem_mmap_gtt.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/gem_mmap_gtt.c b/tests/gem_mmap_gtt.c
> index a95b98a..f964b39 100644
> --- a/tests/gem_mmap_gtt.c
> +++ b/tests/gem_mmap_gtt.c
> @@ -525,7 +525,7 @@ igt_main
>   igt_fixture
>   fd = drm_open_any();
>  
> - igt_subtest("access")
> + igt_subtest("basic")
>   test_access(fd);
>   igt_subtest("short")
>   test_short(fd);
> @@ -556,11 +556,11 @@ igt_main
>   igt_subtest("write-cpu-read-gtt")
>   test_write_cpu_read_gtt(fd);
>  
> - igt_subtest("small-bo")
> + igt_subtest("basic-small-bo")
>   test_huge_bo(fd, -1, I915_TILING_NONE);
> - igt_subtest("small-bo-tiledX")
> + igt_subtest("basic-small-bo-tiledX")
>   test_huge_bo(fd, -1, I915_TILING_X);
> - igt_subtest("small-bo-tiledY")
> + igt_subtest("basic-small-bo-tiledY")
>   test_huge_bo(fd, -1, I915_TILING_Y);

All the testcases up to this one are super-basic sanity checks and
coherency checks. They should also run really fast with the exception of
fault-concurrent. I'd mark them all as basic, except fault-concurrent
since that's more a stress-test.
-Daniel

>  
>   igt_subtest("big-bo")
> -- 
> 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 10/18] tests/gem_pread/pwrite: mark normal tests as basic

[Daniel Vetter]

On Thu, Aug 13, 2015 at 01:31:33PM -0700, Jesse Barnes wrote:
> These should always pass.
> 
> Signed-off-by: Jesse Barnes 

If they're fast enough ... Acked-by: Daniel Vetter 
> ---
>  tests/gem_pread.c  | 2 +-
>  tests/gem_pwrite.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/gem_pread.c b/tests/gem_pread.c
> index cc83948..3ec5fb1 100644
> --- a/tests/gem_pread.c
> +++ b/tests/gem_pread.c
> @@ -108,7 +108,7 @@ int main(int argc, char **argv)
>   src = malloc(object_size);
>   }
>  
> - igt_subtest("normal") {
> + igt_subtest("basic") {
>   for (count = 1; count <= 1<<17; count <<= 1) {
>   struct timeval start, end;
>  
> diff --git a/tests/gem_pwrite.c b/tests/gem_pwrite.c
> index 5b6a77f..1d867e7 100644
> --- a/tests/gem_pwrite.c
> +++ b/tests/gem_pwrite.c
> @@ -169,7 +169,7 @@ int main(int argc, char **argv)
>   src = malloc(object_size);
>   }
>  
> - igt_subtest("normal") {
> + igt_subtest("basic") {
>   for (count = 1; count <= 1<<17; count <<= 1) {
>   struct timeval start, end;
>  
> -- 
> 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 08/18] tests/gem_mmap: mark basic object creation tests as basic

[Chris Wilson]

On Fri, Aug 14, 2015 at 02:33:25PM +0200, Daniel Vetter wrote:
> On Thu, Aug 13, 2015 at 01:31:31PM -0700, Jesse Barnes wrote:
> > We should be able to create small and moderate sized objects quickly and
> > without errors.
> > 
> > Signed-off-by: Jesse Barnes 
> 
> They're all super-fast basic testcases for api cornercases. I'd vote to
> rename the entire testcase to gem_mmap_basic.

Hmm. Time to add another test, huger-bo.
-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] [drm-intel:drm-intel-next-queued 543/545] drivers/gpu/drm/i915/i915_debugfs.c:2449:57: sparse: Using plain integer as NULL pointer

[kbuild test robot]

tree:   git://anongit.freedesktop.org/drm-intel drm-intel-next-queued
head:   41615b7ab6113248748c5734981009f1a0ee499b
commit: 9a9cb6512e140a84b589a6e99f4e71b0397b6685 [543/545] drm/i915: Debugfs 
interface for GuC submission statistics
reproduce:
  # apt-get install sparse
  git checkout 9a9cb6512e140a84b589a6e99f4e71b0397b6685
  make ARCH=x86_64 allmodconfig
  make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/gpu/drm/i915/i915_debugfs.c:2449:57: sparse: Using plain integer as 
>> NULL pointer
   drivers/gpu/drm/i915/i915_debugfs.c:4953:18: sparse: Variable length array 
is used.
   drivers/gpu/drm/i915/i915_debugfs.c:4953:32: sparse: Variable length array 
is used.

vim +2449 drivers/gpu/drm/i915/i915_debugfs.c

  2433  
  2434  for_each_ring(ring, dev_priv, i) {
  2435  seq_printf(m, "\tSubmissions: %llu %s\n",
  2436  client->submissions[i],
  2437  ring->name);
  2438  tot += client->submissions[i];
  2439  }
  2440  seq_printf(m, "\tTotal: %llu\n", tot);
  2441  }
  2442  
  2443  static int i915_guc_info(struct seq_file *m, void *data)
  2444  {
  2445  struct drm_info_node *node = m->private;
  2446  struct drm_device *dev = node->minor->dev;
  2447  struct drm_i915_private *dev_priv = dev->dev_private;
  2448  struct intel_guc guc;
> 2449  struct i915_guc_client client = { .client_obj = 0 };
  2450  struct intel_engine_cs *ring;
  2451  enum intel_ring_id i;
  2452  u64 total = 0;
  2453  
  2454  if (!HAS_GUC_SCHED(dev_priv->dev))
  2455  return 0;
  2456  
  2457  /* Take a local copy of the GuC data, so we can dump it at 
leisure */

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 1/2] drm/i915/bxt: work around HW coherency issue when accessing GPU seqno

[Imre Deak]

By running igt/store_dword_loop_render on BXT we can hit a coherency
problem where the seqno written at GPU command completion time is not
seen by the CPU. This results in __i915_wait_request seeing the stale
seqno and not completing the request (not considering the lost
interrupt/GPU reset mechanism). I also verified that this isn't a case
of a lost interrupt, or that the command didn't complete somehow: when
the coherency issue occured I read the seqno via an uncached GTT mapping
too. While the cached version of the seqno still showed the stale value
the one read via the uncached mapping was the correct one.

Work around this issue by clflushing the corresponding CPU cacheline
following any store of the seqno and preceding any reading of it. When
reading it do this only when the caller expects a coherent view.

v2:
- fix using the proper logical && instead of a bitwise & (Jani, Mika)
- limit the workaround to A stepping, on later steppings this HW issue
  is fixed

Testcase: igt/store_dword_loop_render
Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/i915/intel_lrc.c| 20 
 drivers/gpu/drm/i915/intel_ringbuffer.h |  7 +++
 2 files changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 138964a..46f2be0 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1691,12 +1691,32 @@ static int gen8_emit_flush_render(struct 
drm_i915_gem_request *request,
 
 static u32 gen8_get_seqno(struct intel_engine_cs *ring, bool lazy_coherency)
 {
+
+   /*
+* On BXT A steppings there is a HW coherency issue whereby the
+* MI_STORE_DATA_IMM storing the completed request's seqno
+* occasionally doesn't invalidate the CPU cache. Work around this by
+* clflushing the corresponding cacheline whenever the caller wants
+* the coherency to be guaranteed. Note that this cacheline is known
+* to be clean at this point, since we only write it in
+* gen8_set_seqno(), where we also do a clflush after the write. So
+* this clflush in practice becomes an invalidate operation.
+*/
+
+   if (IS_BROXTON(ring->dev) && INTEL_REVID(ring->dev) < BXT_REVID_B0 &&
+   !lazy_coherency)
+   intel_flush_status_page(ring, I915_GEM_HWS_INDEX);
+
return intel_read_status_page(ring, I915_GEM_HWS_INDEX);
 }
 
 static void gen8_set_seqno(struct intel_engine_cs *ring, u32 seqno)
 {
intel_write_status_page(ring, I915_GEM_HWS_INDEX, seqno);
+
+   /* See gen8_get_seqno() explaining the reason for the clflush. */
+   if (IS_BROXTON(ring->dev) && INTEL_REVID(ring->dev) < BXT_REVID_B0)
+   intel_flush_status_page(ring, I915_GEM_HWS_INDEX);
 }
 
 static int gen8_emit_request(struct drm_i915_gem_request *request)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h 
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 2e85fda..95b0b4b 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -377,6 +377,13 @@ intel_ring_sync_index(struct intel_engine_cs *ring,
return idx;
 }
 
+static inline void
+intel_flush_status_page(struct intel_engine_cs *ring, int reg)
+{
+   drm_clflush_virt_range(&ring->status_page.page_addr[reg],
+  sizeof(uint32_t));
+}
+
 static inline u32
 intel_read_status_page(struct intel_engine_cs *ring,
   int reg)
-- 
2.1.4

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

[Intel-gfx] [PATCH v2 2/2] drm/i915/bxt: work around HW coherency issue for cached GEM mappings

[Imre Deak]

Due to a coherency issue on BXT A steppings we can't guarantee a
coherent view of cached GPU mappings, so fall back to uncached mappings.
Note that this still won't fix cases where userspace expects a coherent
view without synchronizing (via a set domain call). It still makes sense
to limit the kernel's notion of the mapping to be uncached, for example
for relocations to work properly during execbuffer time. Also in case
user space does synchronize the buffer, this will still guarantee that
we'll do the proper clflushing for the buffer.

v2:
- limit the WA to A steppings, on later stepping this HW issue is fixed

Testcast: igt/gem_store_dword_batches_loop/cached-mapping
Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/i915/i915_gem.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 407b6b3..987ffa8 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3742,7 +3742,16 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev, 
void *data,
level = I915_CACHE_NONE;
break;
case I915_CACHING_CACHED:
-   level = I915_CACHE_LLC;
+   /*
+* Due to a HW issue on BXT A stepping, GPU stores via a
+* snooped mapping may leave stale data in a corresponding CPU
+* cacheline, whereas normally such cachelines would get
+* invalidated. As a workaround assume that these stores are
+* not coherent, which means we'll flush the CPU cache manually
+* whenever doing a CPU/GPU sync operation.
+*/
+   level = IS_BROXTON(dev) && INTEL_REVID(dev) < BXT_REVID_B0 ?
+   I915_CACHE_NONE : I915_CACHE_LLC;
break;
case I915_CACHING_DISPLAY:
level = HAS_WT(dev) ? I915_CACHE_WT : I915_CACHE_NONE;
-- 
2.1.4

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

[Intel-gfx] [PATCH v2 0/2] drm/i915/bxt: work around HW coherency issue

[Imre Deak]

This is a v2 of [1]. Since v1 the HW team confirmed that there is an
HW issue in A steppings with the GPU/CPU snoop logic, which explains why
we need this workaround.

In v2 I fixed a typo and limited the workaround to A steppings, since
in later steppings this problem is fixed.

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

Imre Deak (2):
  drm/i915/bxt: work around HW coherency issue when accessing GPU seqno
  drm/i915/bxt: work around HW coherency issue for cached GEM mappings

 drivers/gpu/drm/i915/i915_gem.c | 11 ++-
 drivers/gpu/drm/i915/intel_lrc.c| 20 
 drivers/gpu/drm/i915/intel_ringbuffer.h |  7 +++
 3 files changed, 37 insertions(+), 1 deletion(-)

-- 
2.1.4

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

Re: [Intel-gfx] [PATCH 11/18] tests/gem_tiled_pread/pwrite: mark normal tests as basic

[Daniel Vetter]

On Thu, Aug 13, 2015 at 01:31:34PM -0700, Jesse Barnes wrote:
> These simple tests should always pass.
> 
> Signed-off-by: Jesse Barnes 

Imo shouldn't be part of the basic set, they thrash the machine quite
badly. Especially gem_tiled_pread_pwrite thrashes all of memory, so nack
on that one from me. At least until we've implemented the speedup with
memlock that's been in JIRA since years ...

For gem_tiled_pread, why not just rename to gem_tiled_pread_basic?
-Daniel

> ---
>  tests/gem_tiled_pread.c| 167 
> +
>  tests/gem_tiled_pread_pwrite.c |  48 ++--
>  2 files changed, 112 insertions(+), 103 deletions(-)
> 
> diff --git a/tests/gem_tiled_pread.c b/tests/gem_tiled_pread.c
> index fdc5173..92bb649 100644
> --- a/tests/gem_tiled_pread.c
> +++ b/tests/gem_tiled_pread.c
> @@ -112,7 +112,7 @@ calculate_expected(int offset)
>   return (base_y + tile_y) * WIDTH + base_x + tile_x;
>  }
>  
> -igt_simple_main
> +igt_main
>  {
>   int fd;
>   int i, iter = 100;
> @@ -120,96 +120,101 @@ igt_simple_main
>   uint32_t handle;
>   uint32_t devid;
>  
> - fd = drm_open_any();
> + igt_fixture {
> + fd = drm_open_any();
>  
> - handle = create_bo(fd);
> - gem_get_tiling(fd, handle, &tiling, &swizzle);
> + handle = create_bo(fd);
> + gem_get_tiling(fd, handle, &tiling, &swizzle);
>  
> - devid = intel_get_drm_devid(fd);
> -
> - if (IS_GEN2(devid)) {
> - tile_height = 16;
> - tile_width = 128;
> - tile_size = 2048;
> - } else {
> - tile_height = 8;
> - tile_width = 512;
> - tile_size = PAGE_SIZE;
> + devid = intel_get_drm_devid(fd);
>   }
>  
> - /* Read a bunch of random subsets of the data and check that they come
> -  * out right.
> -  */
> - for (i = 0; i < iter; i++) {
> - int size = WIDTH * HEIGHT * 4;
> - int offset = (random() % size) & ~3;
> - int len = (random() % size) & ~3;
> - int j;
> + igt_subtest("basic") {
>  
> - if (len == 0)
> - len = 4;
> + if (IS_GEN2(devid)) {
> + tile_height = 16;
> + tile_width = 128;
> + tile_size = 2048;
> + } else {
> + tile_height = 8;
> + tile_width = 512;
> + tile_size = PAGE_SIZE;
> + }
>  
> - if (offset + len > size)
> - len = size - offset;
> + /* Read a bunch of random subsets of the data and check that 
> they come
> +  * out right.
> +  */
> + for (i = 0; i < iter; i++) {
> + int size = WIDTH * HEIGHT * 4;
> + int offset = (random() % size) & ~3;
> + int len = (random() % size) & ~3;
> + int j;
>  
> - if (i == 0) {
> - offset = 0;
> - len = size;
> - }
> + if (len == 0)
> + len = 4;
>  
> - gem_read(fd, handle, offset, linear, len);
> + if (offset + len > size)
> + len = size - offset;
>  
> - /* Translate from offsets in the read buffer to the swizzled
> -  * address that it corresponds to.  This is the opposite of
> -  * what Mesa does (calculate offset to be read given the linear
> -  * offset it's looking for).
> -  */
> - for (j = offset; j < offset + len; j += 4) {
> - uint32_t expected_val, found_val;
> - int swizzled_offset;
> - const char *swizzle_str;
> -
> - switch (swizzle) {
> - case I915_BIT_6_SWIZZLE_NONE:
> - swizzled_offset = j;
> - swizzle_str = "none";
> - break;
> - case I915_BIT_6_SWIZZLE_9:
> - swizzled_offset = j ^
> - swizzle_bit(9, j);
> - swizzle_str = "bit9";
> - break;
> - case I915_BIT_6_SWIZZLE_9_10:
> - swizzled_offset = j ^
> - swizzle_bit(9, j) ^
> - swizzle_bit(10, j);
> - swizzle_str = "bit9^10";
> - break;
> - case I915_BIT_6_SWIZZLE_9_11:
> - swizzled_offset = j ^
> - swizzle_bit(9, j) ^
> - swizzle_bit(11, j);
> - swizzle_s

[Intel-gfx] [drm-intel:drm-intel-next-queued 542/545] DockBook: docproc: drivers/gpu/drm/i915/intel_guc_submission.c: No such file or directory

[kbuild test robot]

tree:   git://anongit.freedesktop.org/drm-intel drm-intel-next-queued
head:   41615b7ab6113248748c5734981009f1a0ee499b
commit: a08d99201d3133c4a59f9f04554fe03dc039631d [542/545] drm/i915: Integrate 
GuC-based command submission
reproduce: make htmldocs

All error/warnings (new ones prefixed by >>):

>> docproc: drivers/gpu/drm/i915/intel_guc_submission.c: No such file or 
>> directory

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
#
# Automatically generated file; DO NOT EDIT.
# Linux/i386 4.2.0-rc3 Kernel Configuration
#
# CONFIG_64BIT is not set
CONFIG_X86_32=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_OUTPUT_FORMAT="elf32-i386"
CONFIG_ARCH_DEFCONFIG="arch/x86/configs/i386_defconfig"
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_HAVE_LATENCYTOP_SUPPORT=y
CONFIG_MMU=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y
CONFIG_ARCH_WANT_GENERAL_HUGETLB=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
CONFIG_X86_32_LAZY_GS=y
CONFIG_ARCH_HWEIGHT_CFLAGS="-fcall-saved-ecx -fcall-saved-edx"
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_FIX_EARLYCON_MEM=y
CONFIG_PGTABLE_LEVELS=2
CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y

#
# General setup
#
CONFIG_BROKEN_ON_SMP=y
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=""
# CONFIG_COMPILE_TEST is not set
CONFIG_LOCALVERSION=""
# CONFIG_LOCALVERSION_AUTO is not set
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_BZIP2=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_HAVE_KERNEL_LZ4=y
CONFIG_KERNEL_GZIP=y
# CONFIG_KERNEL_BZIP2 is not set
# CONFIG_KERNEL_LZMA is not set
# CONFIG_KERNEL_XZ is not set
# CONFIG_KERNEL_LZO is not set
# CONFIG_KERNEL_LZ4 is not set
CONFIG_DEFAULT_HOSTNAME="(none)"
# CONFIG_SYSVIPC is not set
# CONFIG_CROSS_MEMORY_ATTACH is not set
# CONFIG_FHANDLE is not set
# CONFIG_USELIB is not set
CONFIG_HAVE_ARCH_AUDITSYSCALL=y

#
# IRQ subsystem
#
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
CONFIG_CLOCKSOURCE_WATCHDOG=y
CONFIG_ARCH_CLOCKSOURCE_DATA=y
CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE=y
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y
CONFIG_GENERIC_CMOS_UPDATE=y

#
# Timers subsystem
#
CONFIG_HZ_PERIODIC=y
# CONFIG_NO_HZ_IDLE is not set
# CONFIG_NO_HZ is not set
# CONFIG_HIGH_RES_TIMERS is not set

#
# CPU/Task time and stats accounting
#
CONFIG_TICK_CPU_ACCOUNTING=y
# CONFIG_IRQ_TIME_ACCOUNTING is not set

#
# RCU Subsystem
#
CONFIG_TINY_RCU=y
# CONFIG_RCU_EXPERT is not set
CONFIG_SRCU=y
# CONFIG_TASKS_RCU is not set
# CONFIG_RCU_STALL_COMMON is not set
# CONFIG_TREE_RCU_TRACE is not set
# CONFIG_RCU_EXPEDITE_BOOT is not set
# CONFIG_BUILD_BIN2C is not set
# CONFIG_IKCONFIG is not set
CONFIG_HAVE_UNSTABLE_SCHED_CLOCK=y
# CONFIG_CGROUPS is not set
# CONFIG_CHECKPOINT_RESTORE is not set
# CONFIG_SCHED_AUTOGROUP is not set
# CONFIG_RELAY is not set
# CONFIG_BLK_DEV_INITRD is not set
# CONFIG_CC_OPTIMIZE_FOR_SIZE is not set
CONFIG_ANON_INODES=y
CONFIG_HAVE_UID16=y
CONFIG_SYSCTL_EXCEPTION_TRACE=y
CONFIG_HAVE_PCSPKR_PLATFORM=y
CONFIG_EXPERT=y
# CONFIG_MULTIUSER is not set
# CONFIG_SGETMASK_SYSCALL is not set
# CONFIG_SYSFS_SYSCALL is not set
# CONFIG_KALLSYMS is not set
# CONFIG_PRINTK is not set
# CONFIG_BUG is not set
# CONFIG_PCSPKR_PLATFORM is not set
# CONFIG_BASE_FULL is not set
# CONFIG_FUTEX is not set
# CONFIG_EPOLL is not set
# CONFIG_SIGNALFD is not set
# CONFIG_TIMERFD is not set
# CONFIG_EVENTFD is not set
# CONFIG_BPF_SYSCALL is not set
# CONFIG_SHMEM is not set
# CONFIG_AIO is not set
# CONFIG_ADVISE_SYSCALLS is not set
CONFIG_EMBEDDED=y
CONFIG_HAVE_PERF_EVENTS=y

#
# Kernel Performance Events And Counters
#
CONFIG_PERF_EVENTS=y
# CONFIG_DEBUG_PERF_USE_VMALLOC is not set
# CONFIG_VM_EVENT_COUNTERS is not set
# CONFIG_COMPAT_BRK is not set
# CONFIG_SLAB is not set
CONFIG_SLUB=y
# CONFIG_SLOB is not set
# CONFIG_PROFILING is not set
CONFIG_HAVE_OPROFILE=y
CONFIG_OPROFILE_NMI_TIMER=y
# CONFIG_JUMP_LABEL is not set
# CONFIG_UPROBES is not set
# CONFIG_HAVE_64BIT_ALIGNED_ACCESS is not set
CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y
CONFIG_ARCH_USE_BUILTIN_BSWAP=y
CONFIG_HAVE_IOREMAP_PROT=y
CONFIG_HAVE_KPROBES=y
CONFIG_HAVE_KRETPROBES=y
CONFIG_HAVE_OPTPROBES=y
CONFIG_HAVE_KPROBES_ON_FTRACE=y
CONFIG_HAVE_ARCH_TRACEHOOK=y
CONFIG_HAVE_DMA_ATTRS=y
CONFIG_HAVE_DMA_CONTIGUOUS=y
CONFIG_GENERIC_SMP_IDLE_

Re: [Intel-gfx] [PATCH 12/18] tests/kms_addfb: mark simple fb creation tests as basic

[Daniel Vetter]

On Thu, Aug 13, 2015 at 01:31:35PM -0700, Jesse Barnes wrote:
> We should always be able to create simple and tiled objects.
> 
> Signed-off-by: Jesse Barnes 

There's are all super-fast abi tests which don't even do a modeset. Imo
add them all by renaming the testcase to kms_addfb_basic. The real
functional testcases are somewhere else (x/y-tiled flipping, rotation,
stuff like that).
-Daniel

> ---
>  tests/kms_addfb.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/kms_addfb.c b/tests/kms_addfb.c
> index 42ee632..f10e12b 100644
> --- a/tests/kms_addfb.c
> +++ b/tests/kms_addfb.c
> @@ -121,7 +121,7 @@ static void pitch_tests(int fd)
>   }
>  
>   f.handles[0] = gem_bo;
> - igt_subtest("normal") {
> + igt_subtest("basic") {
>   igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f) == 0);
>   igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
>   f.fb_id = 0;
> @@ -139,7 +139,7 @@ static void pitch_tests(int fd)
>   gem_set_tiling(fd, gem_bo, I915_TILING_X, 1024*4);
>   f.pitches[0] = 1024*4;
>  
> - igt_subtest("X-tiled") {
> + igt_subtest("basic-X-tiled") {
>   igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f) == 0);
>   igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
>   f.fb_id = 0;
> @@ -162,7 +162,7 @@ static void pitch_tests(int fd)
>   igt_fixture
>   gem_set_tiling(fd, gem_bo, I915_TILING_Y, 1024*4);
>   f.pitches[0] = 1024*4;
> - igt_subtest("Y-tiled") {
> + igt_subtest("basic-Y-tiled") {
>   igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f) == -1 &&
>  errno == EINVAL);
>   }
> -- 
> 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 13/18] tests/kms_vblank: mark accuracy test as basic

[Daniel Vetter]

On Thu, Aug 13, 2015 at 01:31:36PM -0700, Jesse Barnes wrote:
> Need some simple vblank coverage in the BAT list.
> 
> Signed-off-by: Jesse Barnes 

This testcase relies upon fbcon to have enabled pipe 0, which means it
spuriously skips (when fbcon is disabled or when it's blanked the screen)
until that's fixed. Same problem happens with drm_read.

Until that's addressed imo Nack for basic igt testcase list. I'll do a
jira about this issue.
-Daniel
> ---
>  tests/kms_vblank.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
> index 6946177..ef2f7ca 100644
> --- a/tests/kms_vblank.c
> +++ b/tests/kms_vblank.c
> @@ -188,7 +188,7 @@ igt_main
>   igt_require(crtc0_active(fd));
>   }
>  
> - igt_subtest("accuracy")
> + igt_subtest("basic-accuracy")
>   accuracy(fd);
>  
>   igt_subtest("query-idle")
> -- 
> 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 13/18] tests/kms_vblank: mark accuracy test as basic

[Daniel Vetter]

On Fri, Aug 14, 2015 at 02:44:14PM +0200, Daniel Vetter wrote:
> On Thu, Aug 13, 2015 at 01:31:36PM -0700, Jesse Barnes wrote:
> > Need some simple vblank coverage in the BAT list.
> > 
> > Signed-off-by: Jesse Barnes 
> 
> This testcase relies upon fbcon to have enabled pipe 0, which means it
> spuriously skips (when fbcon is disabled or when it's blanked the screen)
> until that's fixed. Same problem happens with drm_read.
> 
> Until that's addressed imo Nack for basic igt testcase list. I'll do a
> jira about this issue.

Meanwhile for a basic vblank testcase I think we should pick one of the
kms_flip ones.
-Daniel

> -Daniel
> > ---
> >  tests/kms_vblank.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
> > index 6946177..ef2f7ca 100644
> > --- a/tests/kms_vblank.c
> > +++ b/tests/kms_vblank.c
> > @@ -188,7 +188,7 @@ igt_main
> > igt_require(crtc0_active(fd));
> > }
> >  
> > -   igt_subtest("accuracy")
> > +   igt_subtest("basic-accuracy")
> > accuracy(fd);
> >  
> > igt_subtest("query-idle")
> > -- 
> > 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

-- 
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 14/18] tests/pm_backlight: mark simple test as basic

[Daniel Vetter]

On Thu, Aug 13, 2015 at 01:31:37PM -0700, Jesse Barnes wrote:
> We should be able to adjust the backlight and observe changes in sysfs.
> 
> Signed-off-by: Jesse Barnes 

Reviewed-by: Daniel Vetter 

> ---
>  tests/pm_backlight.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/pm_backlight.c b/tests/pm_backlight.c
> index d02336d..cff2694 100644
> --- a/tests/pm_backlight.c
> +++ b/tests/pm_backlight.c
> @@ -157,7 +157,7 @@ igt_main
>   igt_assert(backlight_read(&max, "max_brightness") > -1);
>   }
>  
> - igt_subtest("brightness")
> + igt_subtest("basic-brightness")
>   test_brightness(max);
>   igt_subtest("bad-brightness")
>   test_bad_brightness(max);
> -- 
> 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 15/18] tests/pm_rpm: mark RTE and D3 tests as basic

[Daniel Vetter]

On Thu, Aug 13, 2015 at 01:31:38PM -0700, Jesse Barnes wrote:
> These always need to pass for basic PM functionality.
> 
> Signed-off-by: Jesse Barnes 
> ---
>  tests/pm_rpm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
> index d509fa8..7ae5806 100644
> --- a/tests/pm_rpm.c
> +++ b/tests/pm_rpm.c
> @@ -1832,11 +1832,11 @@ int main(int argc, char *argv[])
>   stay_subtest();
>  
>   /* Essential things */
> - igt_subtest("rte")
> + igt_subtest("basic-rte")

I thought our BAT criteria would be basic|rte?

rte is runtime environment check and more along the lines of "did QA set
up their machine correctly", so imo good to keep separate.
-Daniel

>   basic_subtest();
>   igt_subtest("drm-resources-equal")
>   drm_resources_equal_subtest();
> - igt_subtest("pci-d3-state")
> + igt_subtest("basic-pci-d3-state")
>   pci_d3_state_subtest();
>  
>   /* Basic modeset */
> -- 
> 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 0/2] drm/i915/bxt: work around HW coherency issue

[Chris Wilson]

On Fri, Aug 14, 2015 at 03:38:55PM +0300, Imre Deak wrote:
> This is a v2 of [1]. Since v1 the HW team confirmed that there is an
> HW issue in A steppings with the GPU/CPU snoop logic, which explains why
> we need this workaround.

I've been testing this extensively, and I do believe we can use clflush
for all gen - it is a measurable improvement over using the
heavyweight/locked read of ACTHD, and none of the coherency tests have
thrown any warnings (over a period of a couple of months now).
-Chris

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

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

[Thierry Reding]

On Fri, Aug 14, 2015 at 03:35:32PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the drm-misc tree, today's linux-next build (arm
> multi_v7_defconfig) produced these warnings:
> 
> drivers/gpu/drm/exynos/exynos_drm_drv.c:290:2: warning: initialization from 
> incompatible pointer type
>   .get_vblank_counter = drm_vblank_count,
>   ^
> drivers/gpu/drm/exynos/exynos_drm_drv.c:290:2: warning: (near initialization 
> for 'exynos_drm_driver.get_vblank_counter')
> drivers/gpu/drm/rcar-du/rcar_du_drv.c:262:2: warning: initialization from 
> incompatible pointer type
>   .get_vblank_counter = drm_vblank_count,
>   ^
> drivers/gpu/drm/rcar-du/rcar_du_drv.c:262:2: warning: (near initialization 
> for 'rcar_du_driver.get_vblank_counter')
> drivers/gpu/drm/msm/msm_drv.c:981:2: warning: initialization from 
> incompatible pointer type
>   .get_vblank_counter = drm_vblank_count,
>   ^
> drivers/gpu/drm/msm/msm_drv.c:981:2: warning: (near initialization for 
> 'msm_driver.get_vblank_counter')
> 
> Introduced by commit
> 
>   b90180b057f7 ("drm/irq: More pipe/crtc consistency cleanups")

Yes, that patch turned out to be incomplete. Daniel suggested to remove
it from drm-misc for now and I'll address this fully in a larger patch
for 4.4.

Thierry


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

Re: [Intel-gfx] [PATCH 08/18] tests/gem_mmap: mark basic object creation tests as basic

[Chris Wilson]

On Fri, Aug 14, 2015 at 01:37:46PM +0100, Chris Wilson wrote:
> On Fri, Aug 14, 2015 at 02:33:25PM +0200, Daniel Vetter wrote:
> > On Thu, Aug 13, 2015 at 01:31:31PM -0700, Jesse Barnes wrote:
> > > We should be able to create small and moderate sized objects quickly and
> > > without errors.
> > > 
> > > Signed-off-by: Jesse Barnes 
> > 
> > They're all super-fast basic testcases for api cornercases. I'd vote to
> > rename the entire testcase to gem_mmap_basic.
> 
> Hmm. Time to add another test, huger-bo.

Actually, the problem is that I broke the test...
-Chris

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

Re: [Intel-gfx] [PATCH v4 01/11] drm/i915: Store max dotclock

[Ville Syrjälä]

On Fri, Aug 14, 2015 at 01:03:21PM +0300, Mika Kahola wrote:
> Store max dotclock into dev_priv structure so we are able
> to filter out the modes that are not supported by our
> platforms.
> 
> V2:
> - limit the max dot clock frequency to max CD clock frequency
>   for the gen9 and above
> - limit the max dot clock frequency to 90% of the max CD clock
>   frequency for the older gens
> - for Cherryview the max dot clock frequency is limited to 95%
>   of the max CD clock frequency
> - for gen2 and gen3 the max dot clock limit is set to 90% of the
>   2X max CD clock frequency
> 
> V3:
> - max_dotclk variable renamed as max_dotclk_freq in i915_drv.h
> - in intel_compute_max_dotclk() the rounding method changed from
>   round up to round down when computing max dotclock
> 
> Signed-off-by: Mika Kahola 
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>  drivers/gpu/drm/i915/intel_display.c | 19 +++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 55611d8..e1910ec 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1787,6 +1787,7 @@ struct drm_i915_private {
>   unsigned int fsb_freq, mem_freq, is_ddr3;
>   unsigned int skl_boot_cdclk;
>   unsigned int cdclk_freq, max_cdclk_freq;
> + unsigned int max_dotclk_freq;
>   unsigned int hpll_freq;
>  
>   /**
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 21aa745..e8d8860 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5275,6 +5275,20 @@ static void modeset_update_crtc_power_domains(struct 
> drm_atomic_state *state)
>   modeset_put_power_domains(dev_priv, put_domains[i]);
>  }
>  
> +static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
> +{
> + int max_cdclk_freq = dev_priv->max_cdclk_freq;
> +
> + if (INTEL_INFO(dev_priv)->gen >= 9)

Sorry, I missed this the last time. The conditiona should
actually be (HSW || BDW || gen >= 9) or something to that effect.

Otherwise looks good, so with that fixed:
Reviewed-by: Ville Syrjälä 

> + return max_cdclk_freq;
> + else if (IS_CHERRYVIEW(dev_priv))
> + return max_cdclk_freq*95/100;
> + else if (INTEL_INFO(dev_priv)->gen < 4)
> + return 2*max_cdclk_freq*90/100;
> + else
> + return max_cdclk_freq*90/100;
> +}
> +
>  static void intel_update_max_cdclk(struct drm_device *dev)
>  {
>   struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -5314,8 +5328,13 @@ static void intel_update_max_cdclk(struct drm_device 
> *dev)
>   dev_priv->max_cdclk_freq = dev_priv->cdclk_freq;
>   }
>  
> + dev_priv->max_dotclk_freq = intel_compute_max_dotclk(dev_priv);
> +
>   DRM_DEBUG_DRIVER("Max CD clock rate: %d kHz\n",
>dev_priv->max_cdclk_freq);
> +
> + DRM_DEBUG_DRIVER("Max dotclock rate: %d kHz\n",
> +  dev_priv->max_dotclk_freq);
>  }
>  
>  static void intel_update_cdclk(struct drm_device *dev)
> -- 
> 1.9.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] [PATCH 16/18] tests/kms_flip: add basic tests for flip, flip vs dpms, and flip modeset

[Daniel Vetter]

On Thu, Aug 13, 2015 at 01:31:39PM -0700, Jesse Barnes wrote:
> Simple variants that don't do multiple output or interruptible testing.
> 
> Signed-off-by: Jesse Barnes 
> ---
>  tests/kms_flip.c | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> index a595d9f..a0e4112 100644
> --- a/tests/kms_flip.c
> +++ b/tests/kms_flip.c
> @@ -80,6 +80,7 @@
>  #define TEST_TS_CONT (1 << 27)
>  #define TEST_BO_TOOBIG   (1 << 28)
>  #define TEST_HANG_ONCE   (1 << 29)
> +#define TEST_BASIC   (1 << 30)
>  
>  #define EVENT_FLIP   (1 << 0)
>  #define EVENT_VBLANK (1 << 1)
> @@ -1649,7 +1650,7 @@ int main(int argc, char **argv)
>   "blt-wf_vblank-vs-modeset" },
>   { 60,  TEST_VBLANK | TEST_MODESET | TEST_WITH_DUMMY_RCS,
>   "rcs-wf_vblank-vs-modeset" },
> -
> + { 30, TEST_FLIP | TEST_BASIC, "basic-plain-flip" },

I think for better coverage we should pick the timestamp checking ones
here. Also we need one to cover vblank since kms_vblank can't be used. And
we can do that all in one testcases to save testing time!

>   { 30, TEST_FLIP , "plain-flip" },
>   { 30, TEST_FLIP | TEST_EBUSY , "busy-flip" },
>   { 30, TEST_FLIP | TEST_FENCE_STRESS , "flip-vs-fences" },
> @@ -1657,12 +1658,14 @@ int main(int argc, char **argv)
>   { 30, TEST_FLIP | TEST_CHECK_TS | TEST_FB_RECREATE,
>   "plain-flip-fb-recreate" },
>   { 30, TEST_FLIP | TEST_RMFB | TEST_MODESET , "flip-vs-rmfb" },
> + { 60, TEST_FLIP | TEST_DPMS | TEST_EINVAL | TEST_BASIC, 
> "basic-flip-vs-dpms" },
>   { 60, TEST_FLIP | TEST_DPMS | TEST_EINVAL, "flip-vs-dpms" },
>   { 60, TEST_FLIP | TEST_DPMS | TEST_WITH_DUMMY_BCS, 
> "blt-flip-vs-dpms" },
>   { 60, TEST_FLIP | TEST_DPMS | TEST_WITH_DUMMY_RCS, 
> "render-flip-vs-dpms" },
>   { 30,  TEST_FLIP | TEST_PAN, "flip-vs-panning" },
>   { 60, TEST_FLIP | TEST_PAN | TEST_WITH_DUMMY_BCS, 
> "blt-flip-vs-panning" },
>   { 60, TEST_FLIP | TEST_PAN | TEST_WITH_DUMMY_RCS, 
> "render-flip-vs-panning" },
> + { 60, TEST_FLIP | TEST_MODESET | TEST_EINVAL | TEST_BASIC, 
> "basic-flip-vs-modeset" },
>   { 60, TEST_FLIP | TEST_MODESET | TEST_EINVAL, "flip-vs-modeset" 
> },
>   { 60, TEST_FLIP | TEST_MODESET | TEST_WITH_DUMMY_BCS, 
> "blt-flip-vs-modeset" },
>   { 60, TEST_FLIP | TEST_MODESET | TEST_WITH_DUMMY_RCS, 
> "render-flip-vs-modeset" },
> @@ -1721,6 +1724,9 @@ int main(int argc, char **argv)
>   igt_subtest(tests[i].name)
>   run_test(tests[i].duration, tests[i].flags);
>  
> + if (tests[i].flags & TEST_BASIC)
> + continue;

This means you remove the tests you marked as BASIC from the 2x and
interruptible sets. What about instead adding the basic prefix at runtime,
i.e. for all my requests:
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index a595d9f1d69f..aaf03b07df87 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -80,6 +80,7 @@
 #define TEST_TS_CONT   (1 << 27)
 #define TEST_BO_TOOBIG (1 << 28)
 #define TEST_HANG_ONCE (1 << 29)
+#define TEST_BASIC (1 << 31)
 
 #define EVENT_FLIP (1 << 0)
 #define EVENT_VBLANK   (1 << 1)
@@ -1650,20 +1651,20 @@ int main(int argc, char **argv)
{ 60,  TEST_VBLANK | TEST_MODESET | TEST_WITH_DUMMY_RCS,
"rcs-wf_vblank-vs-modeset" },
 
-   { 30, TEST_FLIP , "plain-flip" },
+   { 30, TEST_FLIP, "plain-flip" },
{ 30, TEST_FLIP | TEST_EBUSY , "busy-flip" },
{ 30, TEST_FLIP | TEST_FENCE_STRESS , "flip-vs-fences" },
{ 30, TEST_FLIP | TEST_CHECK_TS, "plain-flip-ts-check" },
{ 30, TEST_FLIP | TEST_CHECK_TS | TEST_FB_RECREATE,
"plain-flip-fb-recreate" },
{ 30, TEST_FLIP | TEST_RMFB | TEST_MODESET , "flip-vs-rmfb" },
-   { 60, TEST_FLIP | TEST_DPMS | TEST_EINVAL, "flip-vs-dpms" },
+   { 60, TEST_FLIP | TEST_DPMS | TEST_EINVAL | TEST_BASIC, 
"flip-vs-dpms" },
{ 60, TEST_FLIP | TEST_DPMS | TEST_WITH_DUMMY_BCS, 
"blt-flip-vs-dpms" },
{ 60, TEST_FLIP | TEST_DPMS | TEST_WITH_DUMMY_RCS, 
"render-flip-vs-dpms" },
{ 30,  TEST_FLIP | TEST_PAN, "flip-vs-panning" },
{ 60, TEST_FLIP | TEST_PAN | TEST_WITH_DUMMY_BCS, 
"blt-flip-vs-panning" },
{ 60, TEST_FLIP | TEST_PAN | TEST_WITH_DUMMY_RCS, 
"render-flip-vs-panning" },
-   { 60, TEST_FLIP | TEST_MODESET | TEST_EINVAL, "flip-vs-modeset" 
},
+   { 60, TEST_FLIP | TEST_MODESET | TEST_EINVAL | TEST_BASIC, 
"flip-

[Intel-gfx] [PATCH] drm/i915: remove excessive scaler debugging messages

[Jani Nikula]

There's so much scaler debugging messages that it makes other debugging
hard. Remove them.

Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/intel_atomic.c  | 3 ---
 drivers/gpu/drm/i915/intel_display.c | 2 --
 2 files changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
b/drivers/gpu/drm/i915/intel_atomic.c
index e2531cf59266..9336e8030980 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -149,9 +149,6 @@ int intel_atomic_setup_scalers(struct drm_device *dev,
int i, j;
 
num_scalers_need = hweight32(scaler_state->scaler_users);
-   DRM_DEBUG_KMS("crtc_state = %p need = %d avail = %d scaler_users = 
0x%x\n",
-   crtc_state, num_scalers_need, intel_crtc->num_scalers,
-   scaler_state->scaler_users);
 
/*
 * High level flow:
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 21aa745caed1..52720ff9f769 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2935,8 +2935,6 @@ static void skl_detach_scaler(struct intel_crtc 
*intel_crtc, int id)
I915_WRITE(SKL_PS_CTRL(intel_crtc->pipe, id), 0);
I915_WRITE(SKL_PS_WIN_POS(intel_crtc->pipe, id), 0);
I915_WRITE(SKL_PS_WIN_SZ(intel_crtc->pipe, id), 0);
-   DRM_DEBUG_KMS("CRTC:%d Disabled scaler id %u.%u\n",
-   intel_crtc->base.base.id, intel_crtc->pipe, id);
 }
 
 /*
-- 
2.1.4

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

Re: [Intel-gfx] [PATCH v4 06/11] drm/i915: DSI pixel clock check

[Ville Syrjälä]

On Fri, Aug 14, 2015 at 01:03:26PM +0300, Mika Kahola wrote:
> It is possible the we request to have a mode that has
> higher pixel clock than our HW can support. This patch
> checks if requested pixel clock is lower than the one
> supported by the HW. The requested mode is discarded
> if we cannot support the requested pixel clock.
> 
> This patch applies to DSI.
> 
> V2:
> - removed computation for max pixel clock
> 
> V3:
> - cleanup by removing unnecessary lines
> 
> V4:
> - max_pixclk variable renamed as max_dotclk
> - moved dot clock checking inside 'if (fixed_mode)'
> 
> Signed-off-by: Mika Kahola 
> ---
>  drivers/gpu/drm/i915/intel_dsi.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c 
> b/drivers/gpu/drm/i915/intel_dsi.c
> index 4a601cf..00c6b1f 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -654,6 +654,7 @@ intel_dsi_mode_valid(struct drm_connector *connector,
>  {
>   struct intel_connector *intel_connector = to_intel_connector(connector);
>   struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
> + int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
>  
>   DRM_DEBUG_KMS("\n");
>  
> @@ -667,6 +668,8 @@ intel_dsi_mode_valid(struct drm_connector *connector,
>   return MODE_PANEL;
>   if (mode->vdisplay > fixed_mode->vdisplay)
>   return MODE_PANEL;
> + if (mode->clock > max_dotclk)
> + return MODE_CLOCK_HIGH;

Should be fixed_mode->clock

With that fixed:
Reviewed-by: Ville Syrjälä 

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

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

Re: [Intel-gfx] [PATCH 17/18] tests/kms_setmode: mark simple clone test as basic

[Daniel Vetter]

On Thu, Aug 13, 2015 at 01:31:40PM -0700, Jesse Barnes wrote:
> Should cover simple, single CRTC mode sets.
> 
> Signed-off-by: Jesse Barnes 
> ---
>  tests/kms_setmode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
> index 82769ab..2c0fe45 100644
> --- a/tests/kms_setmode.c
> +++ b/tests/kms_setmode.c
> @@ -685,7 +685,7 @@ int main(int argc, char **argv)
>   const char *name;
>   } tests[] = {
>   { TEST_CLONE | TEST_SINGLE_CRTC_CLONE,
> - "clone-single-crtc" },
> + "basic-clone-single-crtc" },

This isn't all that simple, it covers cloning. But since kms_flip doesn't
do that yet definitely makes sense to include (assuming we still have
budget for kms tests).

Acked-by: Daniel Vetter 

>   { TEST_INVALID | TEST_CLONE | TEST_SINGLE_CRTC_CLONE,
>   "invalid-clone-single-crtc" },
>   { TEST_INVALID | TEST_CLONE | TEST_EXCLUSIVE_CRTC_CLONE,
> -- 
> 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 v4 01/11] drm/i915: Store max dotclock

[Mika Kahola]

On Fri, 2015-08-14 at 15:55 +0300, Ville Syrjälä wrote:
> On Fri, Aug 14, 2015 at 01:03:21PM +0300, Mika Kahola wrote:
> > Store max dotclock into dev_priv structure so we are able
> > to filter out the modes that are not supported by our
> > platforms.
> > 
> > V2:
> > - limit the max dot clock frequency to max CD clock frequency
> >   for the gen9 and above
> > - limit the max dot clock frequency to 90% of the max CD clock
> >   frequency for the older gens
> > - for Cherryview the max dot clock frequency is limited to 95%
> >   of the max CD clock frequency
> > - for gen2 and gen3 the max dot clock limit is set to 90% of the
> >   2X max CD clock frequency
> > 
> > V3:
> > - max_dotclk variable renamed as max_dotclk_freq in i915_drv.h
> > - in intel_compute_max_dotclk() the rounding method changed from
> >   round up to round down when computing max dotclock
> > 
> > Signed-off-by: Mika Kahola 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h  |  1 +
> >  drivers/gpu/drm/i915/intel_display.c | 19 +++
> >  2 files changed, 20 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 55611d8..e1910ec 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1787,6 +1787,7 @@ struct drm_i915_private {
> > unsigned int fsb_freq, mem_freq, is_ddr3;
> > unsigned int skl_boot_cdclk;
> > unsigned int cdclk_freq, max_cdclk_freq;
> > +   unsigned int max_dotclk_freq;
> > unsigned int hpll_freq;
> >  
> > /**
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 21aa745..e8d8860 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5275,6 +5275,20 @@ static void modeset_update_crtc_power_domains(struct 
> > drm_atomic_state *state)
> > modeset_put_power_domains(dev_priv, put_domains[i]);
> >  }
> >  
> > +static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
> > +{
> > +   int max_cdclk_freq = dev_priv->max_cdclk_freq;
> > +
> > +   if (INTEL_INFO(dev_priv)->gen >= 9)
> 
> Sorry, I missed this the last time. The conditiona should
> actually be (HSW || BDW || gen >= 9) or something to that effect.

Allright. I add Haswell and Broadwell to this condition.

-Mika-

> Otherwise looks good, so with that fixed:
> Reviewed-by: Ville Syrjälä 
> 
> > +   return max_cdclk_freq;
> > +   else if (IS_CHERRYVIEW(dev_priv))
> > +   return max_cdclk_freq*95/100;
> > +   else if (INTEL_INFO(dev_priv)->gen < 4)
> > +   return 2*max_cdclk_freq*90/100;
> > +   else
> > +   return max_cdclk_freq*90/100;
> > +}
> > +
> >  static void intel_update_max_cdclk(struct drm_device *dev)
> >  {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -5314,8 +5328,13 @@ static void intel_update_max_cdclk(struct drm_device 
> > *dev)
> > dev_priv->max_cdclk_freq = dev_priv->cdclk_freq;
> > }
> >  
> > +   dev_priv->max_dotclk_freq = intel_compute_max_dotclk(dev_priv);
> > +
> > DRM_DEBUG_DRIVER("Max CD clock rate: %d kHz\n",
> >  dev_priv->max_cdclk_freq);
> > +
> > +   DRM_DEBUG_DRIVER("Max dotclock rate: %d kHz\n",
> > +dev_priv->max_dotclk_freq);
> >  }
> >  
> >  static void intel_update_cdclk(struct drm_device *dev)
> > -- 
> > 1.9.1
> > 
> > ___
> > 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 v3 03/11] drm/i915: HDMI pixel clock check

[Ville Syrjälä]

On Fri, Aug 14, 2015 at 10:30:26AM +0200, Daniel Vetter wrote:
> On Wed, Aug 12, 2015 at 09:34:54PM +0300, Ville Syrjälä wrote:
> > On Fri, Jul 31, 2015 at 03:13:52PM +0300, Mika Kahola wrote:
> > > It is possible the we request to have a mode that has
> > > higher pixel clock than our HW can support. This patch
> > > checks if requested pixel clock is lower than the one
> > > supported by the HW. The requested mode is discarded
> > > if we cannot support the requested pixel clock.
> > > 
> > > This patch applies to HDMI.
> > > 
> > > V2:
> > > - removed computation for max dot clock
> > > 
> > > V3:
> > > - cleanup by removing unnecessary lines
> > > 
> > > Signed-off-by: Mika Kahola 
> > > ---
> > >  drivers/gpu/drm/i915/intel_hdmi.c | 4 
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
> > > b/drivers/gpu/drm/i915/intel_hdmi.c
> > > index 70bad5b..3149e5f 100644
> > > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > > @@ -1193,10 +1193,14 @@ intel_hdmi_mode_valid(struct drm_connector 
> > > *connector,
> > >   struct drm_device *dev = intel_hdmi_to_dev(hdmi);
> > >   enum drm_mode_status status;
> > >   int clock;
> > > + int max_pixclk = to_i915(connector->dev)->max_dotclk;
> > >  
> > >   if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> > >   return MODE_NO_DBLESCAN;
> > >  
> > > + if (mode->clock > max_pixclk)
> > > + return MODE_CLOCK_HIGH;
> > > +
> > >   clock = mode->clock;
> > 
> > I believe we should do something like this here:
> > 
> > clock = mode->clock;
> > 
> > if ((mode->flags & DRM_MODE_FLAG_3D_MASK) == 
> > DRM_MODE_FLAG_3D_FRAME_PACKING)
> > clock *= 2;
> 
> That's already done in drm_mode_set_crtcinfo, i915 uses the STEREO_DOUBLE
> mode of that function.

Only for modesets. .mode_valid() only deals with the results of
.get_modes() etc., so the timings won't have been doubled. And even then
we only double the .crtc_ timings, which is where I previously gave up
trying to use the same mode validation code during modeset and .fill_modes().
I suppose we can make it happen if we really try, but in the meantime I
think it's at least an improvement if we stop advertising modes we for
sure can't use.

> -Daniel
> 
> > 
> > if (clock > max_pixclk)
> > return MODE_CLOCK_HIGH;
> > 
> > >   if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> > >   clock *= 2;
> > 
> > The stero handling should probably be in a separate patch, but in
> > preparation for it you could already put the dotclk check between the
> > clock= assigment and the DBCLK doubling (since DBLCLK only affects the
> > port_clock and not pixel clock).
> > 
> > > -- 
> > > 1.9.1
> > > 
> > > ___
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Ville Syrjälä
> > Intel OTC
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
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 v4 04/11] drm/i915: LVDS pixel clock check

[Ville Syrjälä]

On Fri, Aug 14, 2015 at 01:03:24PM +0300, Mika Kahola wrote:
> It is possible the we request to have a mode that has
> higher pixel clock than our HW can support. This patch
> checks if requested pixel clock is lower than the one
> supported by the HW. The requested mode is discarded
> if we cannot support the requested pixel clock.
> 
> This patch applies to LVDS.
> 
> V2:
> - removed computation for max pixel clock
> 
> V3:
> - cleanup by removing unnecessary lines
> 
> V4:
> - moved supported dotclock check from mode_valid() to intel_lvds_init()
> 
> Signed-off-by: Mika Kahola 
> ---
>  drivers/gpu/drm/i915/intel_lvds.c | 15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c 
> b/drivers/gpu/drm/i915/intel_lvds.c
> index 881b5d1..06b9d1b 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -1098,8 +1098,11 @@ void intel_lvds_init(struct drm_device *dev)
>   drm_mode_debug_printmodeline(scan);
>  
>   fixed_mode = drm_mode_duplicate(dev, scan);
> - if (fixed_mode)
> - goto out;
> + if (fixed_mode) {
> + if (fixed_mode->clock <= 
> dev_priv->max_dotclk_freq)
> + goto out;
> + }
> + fixed_mode = NULL;
>   }
>   }
>  
> @@ -,8 +1114,10 @@ void intel_lvds_init(struct drm_device *dev)
>   fixed_mode = drm_mode_duplicate(dev, 
> dev_priv->vbt.lfp_lvds_vbt_mode);
>   if (fixed_mode) {
>   fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
> - goto out;
> + if (fixed_mode->clock <= dev_priv->max_dotclk_freq)
> + goto out;
>   }
> + fixed_mode = NULL;
>   }
>  
>   /*
> @@ -1135,8 +1140,10 @@ void intel_lvds_init(struct drm_device *dev)
>   DRM_DEBUG_KMS("using current (BIOS) mode: ");
>   drm_mode_debug_printmodeline(fixed_mode);
>   fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
> - goto out;
> + if (fixed_mode->clock <= dev_priv->max_dotclk_freq)
> + goto out;
>   }
> + fixed_mode = NULL;

I don't think we want to special case just LVDS this way. Whatever we do
with fixed_mode validation should be done for all connector types that
have it. For now I think we can more or less ignore the issue.

So in that light, your previous patch was OK except it should have just
checked fixed_mode->clock instead of mode->clock. Sorry if my ramblings
confused you too much.

>   }
>  
>   /* If we still don't have a mode after all that, give up. */
> -- 
> 1.9.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] [PATCH 15/18] tests/pm_rpm: mark RTE and D3 tests as basic

[Paulo Zanoni]

2015-08-14 9:50 GMT-03:00 Daniel Vetter :
> On Thu, Aug 13, 2015 at 01:31:38PM -0700, Jesse Barnes wrote:
>> These always need to pass for basic PM functionality.
>>
>> Signed-off-by: Jesse Barnes 
>> ---
>>  tests/pm_rpm.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
>> index d509fa8..7ae5806 100644
>> --- a/tests/pm_rpm.c
>> +++ b/tests/pm_rpm.c
>> @@ -1832,11 +1832,11 @@ int main(int argc, char *argv[])
>>   stay_subtest();
>>
>>   /* Essential things */
>> - igt_subtest("rte")
>> + igt_subtest("basic-rte")
>
> I thought our BAT criteria would be basic|rte?
>
> rte is runtime environment check and more along the lines of "did QA set
> up their machine correctly", so imo good to keep separate.

When I originally named the test, I thought the test would catch bad
machine setups more often than the other case. But after QA managed to
properly setup their machines, we got tons and tons of Kernel
regressions that broke pm_rpm/rte. So "basic-rte" wouldn't be a bad
name, since we can actually catch both bad setups and fundamental
regressions here.

> -Daniel
>
>>   basic_subtest();
>>   igt_subtest("drm-resources-equal")
>>   drm_resources_equal_subtest();
>> - igt_subtest("pci-d3-state")
>> + igt_subtest("basic-pci-d3-state")
>>   pci_d3_state_subtest();
>>
>>   /* Basic modeset */
>> --
>> 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



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

Re: [Intel-gfx] [PATCH v4 10/11] drm/i915: DVO pixel clock check

[Ville Syrjälä]

On Fri, Aug 14, 2015 at 01:03:30PM +0300, Mika Kahola wrote:
> It is possible the we request to have a mode that has
> higher pixel clock than our HW can support. This patch
> checks if requested pixel clock is lower than the one
> supported by the HW. The requested mode is discarded
> if we cannot support the requested pixel clock.
> 
> This patch applies to DVO.
> 
> V2:
> - removed computation for max pixel clock
> 
> V3:
> - cleanup by removing unnecessary lines
> 
> V4:
> - clock check against max dotclock moved inside 'if (fixed_mode)'
> 
> Signed-off-by: Mika Kahola 
> ---
>  drivers/gpu/drm/i915/intel_dvo.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c 
> b/drivers/gpu/drm/i915/intel_dvo.c
> index dc532bb..924b724 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -201,6 +201,7 @@ intel_dvo_mode_valid(struct drm_connector *connector,
>struct drm_display_mode *mode)
>  {
>   struct intel_dvo *intel_dvo = intel_attached_dvo(connector);
> + int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
>  
>   if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
>   return MODE_NO_DBLESCAN;
> @@ -212,6 +213,8 @@ intel_dvo_mode_valid(struct drm_connector *connector,
>   return MODE_PANEL;
>   if (mode->vdisplay > intel_dvo->panel_fixed_mode->vdisplay)
>   return MODE_PANEL;
> + if (mode->clock > max_dotclk)
> + return MODE_CLOCK_HIGH;
>   }

What we want here is duplicate the intel_dp.c logic exactly, ie. check
against fixed_mode->clock when it's around, otherwise check against
mode->clock.

>  
>   return intel_dvo->dev.dev_ops->mode_valid(&intel_dvo->dev, mode);
> -- 
> 1.9.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] [PATCH v2 2/2] drm/i915/bxt: work around HW coherency issue for cached GEM mappings

[Chris Wilson]

On Fri, Aug 14, 2015 at 03:38:57PM +0300, Imre Deak wrote:
> Due to a coherency issue on BXT A steppings we can't guarantee a
> coherent view of cached GPU mappings, so fall back to uncached mappings.
> Note that this still won't fix cases where userspace expects a coherent
> view without synchronizing (via a set domain call). It still makes sense
> to limit the kernel's notion of the mapping to be uncached, for example
> for relocations to work properly during execbuffer time. Also in case
> user space does synchronize the buffer, this will still guarantee that
> we'll do the proper clflushing for the buffer.
> 
> v2:
> - limit the WA to A steppings, on later stepping this HW issue is fixed

This has to report the failure, ENODEV otherwise userspace will be
terribly confused (it will try to CPU coherent access assuming it will
be fast, when it is better to use alternative paths).
-Chris

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

Re: [Intel-gfx] [PATCH v3 03/11] drm/i915: HDMI pixel clock check

[Daniel Vetter]

On Fri, Aug 14, 2015 at 04:03:55PM +0300, Ville Syrjälä wrote:
> On Fri, Aug 14, 2015 at 10:30:26AM +0200, Daniel Vetter wrote:
> > On Wed, Aug 12, 2015 at 09:34:54PM +0300, Ville Syrjälä wrote:
> > > On Fri, Jul 31, 2015 at 03:13:52PM +0300, Mika Kahola wrote:
> > > > It is possible the we request to have a mode that has
> > > > higher pixel clock than our HW can support. This patch
> > > > checks if requested pixel clock is lower than the one
> > > > supported by the HW. The requested mode is discarded
> > > > if we cannot support the requested pixel clock.
> > > > 
> > > > This patch applies to HDMI.
> > > > 
> > > > V2:
> > > > - removed computation for max dot clock
> > > > 
> > > > V3:
> > > > - cleanup by removing unnecessary lines
> > > > 
> > > > Signed-off-by: Mika Kahola 
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_hdmi.c | 4 
> > > >  1 file changed, 4 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
> > > > b/drivers/gpu/drm/i915/intel_hdmi.c
> > > > index 70bad5b..3149e5f 100644
> > > > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > > > @@ -1193,10 +1193,14 @@ intel_hdmi_mode_valid(struct drm_connector 
> > > > *connector,
> > > > struct drm_device *dev = intel_hdmi_to_dev(hdmi);
> > > > enum drm_mode_status status;
> > > > int clock;
> > > > +   int max_pixclk = to_i915(connector->dev)->max_dotclk;
> > > >  
> > > > if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> > > > return MODE_NO_DBLESCAN;
> > > >  
> > > > +   if (mode->clock > max_pixclk)
> > > > +   return MODE_CLOCK_HIGH;
> > > > +
> > > > clock = mode->clock;
> > > 
> > > I believe we should do something like this here:
> > > 
> > >   clock = mode->clock;
> > > 
> > >   if ((mode->flags & DRM_MODE_FLAG_3D_MASK) == 
> > > DRM_MODE_FLAG_3D_FRAME_PACKING)
> > >   clock *= 2;
> > 
> > That's already done in drm_mode_set_crtcinfo, i915 uses the STEREO_DOUBLE
> > mode of that function.
> 
> Only for modesets. .mode_valid() only deals with the results of
> .get_modes() etc., so the timings won't have been doubled. And even then
> we only double the .crtc_ timings, which is where I previously gave up
> trying to use the same mode validation code during modeset and .fill_modes().
> I suppose we can make it happen if we really try, but in the meantime I
> think it's at least an improvement if we stop advertising modes we for
> sure can't use.

Oh right totally mixed things up, you're correct.
-Daniel

> 
> > -Daniel
> > 
> > > 
> > >   if (clock > max_pixclk)
> > >   return MODE_CLOCK_HIGH;
> > > 
> > > > if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> > > > clock *= 2;
> > > 
> > > The stero handling should probably be in a separate patch, but in
> > > preparation for it you could already put the dotclk check between the
> > > clock= assigment and the DBCLK doubling (since DBLCLK only affects the
> > > port_clock and not pixel clock).
> > > 
> > > > -- 
> > > > 1.9.1
> > > > 
> > > > ___
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > 
> > > -- 
> > > Ville Syrjälä
> > > Intel OTC
> > > ___
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
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 1/2] drm/i915/bxt: work around HW coherency issue when accessing GPU seqno

[Chris Wilson]

On Fri, Aug 14, 2015 at 03:38:56PM +0300, Imre Deak wrote:
> By running igt/store_dword_loop_render on BXT we can hit a coherency
> problem where the seqno written at GPU command completion time is not
> seen by the CPU. This results in __i915_wait_request seeing the stale
> seqno and not completing the request (not considering the lost
> interrupt/GPU reset mechanism). I also verified that this isn't a case
> of a lost interrupt, or that the command didn't complete somehow: when
> the coherency issue occured I read the seqno via an uncached GTT mapping
> too. While the cached version of the seqno still showed the stale value
> the one read via the uncached mapping was the correct one.
> 
> Work around this issue by clflushing the corresponding CPU cacheline
> following any store of the seqno and preceding any reading of it. When
> reading it do this only when the caller expects a coherent view.
> 
> v2:
> - fix using the proper logical && instead of a bitwise & (Jani, Mika)
> - limit the workaround to A stepping, on later steppings this HW issue
>   is fixed

We have vfuncs in order to avoid the pointer dance (and boy is it a
pretty and quite convoluted dance).
-Chris

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

Re: [Intel-gfx] [PATCH v4 00/11] Check pixel clock when setting mode

[Daniel Vetter]

On Fri, Aug 14, 2015 at 01:03:20PM +0300, Mika Kahola wrote:
> From EDID we can read and request higher pixel clock than
> our HW can support. This set of patches add checks if
> requested pixel clock is lower than the one supported by the HW.
> The requested mode is discarded if we cannot support the requested
> pixel clock. For example for Cherryview
> 
> 'cvt 2560 1600 60' gives
> 
> # 2560x1600 59.99 Hz (CVT 4.10MA) hsync: 99.46 kHz; pclk: 348.50 MHz
> Modeline "2560x1600_60.00"  348.50  2560 2760 3032 3504  1600 1603 1609 1658 
> -hsync +vsync
> 
> where pixel clock 348.50 MHz is higher than the supported 304 MHz.
> 
> The checks are implemented for DisplayPort, HDMI, LVDS, DVO, SDVO, DSI,
> CRT, TV, and DP-MST.
> 
> V2:
> - The maximum DOT clock frequency is added to debugfs i915_frequency_info.
> - max dotclock cached in dev_priv structure
> - moved computation of max dotclock to 'intel_display.c'
> 
> V3:
> - intel_update_max_dotclk() renamed as intel_compute_max_dotclk()
> - for GEN9 and above the max dotclock frequency is equal to CD clock
>   frequency
> - for older generations the dot clock frequency is limited to 90% of the
>   CD clock frequency
> - For Cherryview the dot clock is limited to 95% of CD clock frequency
> - for GEN2/3 the maximum dot clock frequency is limited to 90% of the
>   2X CD clock frequency as we have on option to use double wide mode
> - cleanup
> 
> V4:
> - renaming of max_dotclk as max_dotclk_freq in dev_priv (i915_drv.h)
>   caused changes to all patches in my series even though some of them has
>   been r-b'd by Ville
> - for consistency the max_pixclk variable is renamed as max_dotclk throughout
>   the whole series

One thing that was completely missed here is testcases? Do they exist?
-Daniel

> 
> Mika Kahola (11):
>   drm/i915: Store max dotclock
>   drm/i915: DisplayPort pixel clock check
>   drm/i915: HDMI pixel clock check
>   drm/i915: LVDS pixel clock check
>   drm/i915: SDVO pixel clock check
>   drm/i915: DSI pixel clock check
>   drm/i915: CRT pixel clock check
>   drm/i915: TV pixel clock check
>   drm/i915: DisplayPort-MST pixel clock check
>   drm/i915: DVO pixel clock check
>   drm/i915: Max DOT clock frequency to debugfs
> 
>  drivers/gpu/drm/i915/i915_debugfs.c  |  2 ++
>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>  drivers/gpu/drm/i915/intel_crt.c |  4 
>  drivers/gpu/drm/i915/intel_display.c | 19 +++
>  drivers/gpu/drm/i915/intel_dp.c  |  3 ++-
>  drivers/gpu/drm/i915/intel_dp_mst.c  |  5 +
>  drivers/gpu/drm/i915/intel_dsi.c |  3 +++
>  drivers/gpu/drm/i915/intel_dvo.c |  3 +++
>  drivers/gpu/drm/i915/intel_hdmi.c|  8 
>  drivers/gpu/drm/i915/intel_lvds.c| 15 +++
>  drivers/gpu/drm/i915/intel_sdvo.c|  4 
>  drivers/gpu/drm/i915/intel_tv.c  |  4 
>  12 files changed, 66 insertions(+), 5 deletions(-)
> 
> -- 
> 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] drm/i915: remove excessive scaler debugging messages

[Daniel Vetter]

On Fri, Aug 14, 2015 at 03:59:53PM +0300, Jani Nikula wrote:
> There's so much scaler debugging messages that it makes other debugging
> hard. Remove them.
> 
> Signed-off-by: Jani Nikula 

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

> ---
>  drivers/gpu/drm/i915/intel_atomic.c  | 3 ---
>  drivers/gpu/drm/i915/intel_display.c | 2 --
>  2 files changed, 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
> b/drivers/gpu/drm/i915/intel_atomic.c
> index e2531cf59266..9336e8030980 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -149,9 +149,6 @@ int intel_atomic_setup_scalers(struct drm_device *dev,
>   int i, j;
>  
>   num_scalers_need = hweight32(scaler_state->scaler_users);
> - DRM_DEBUG_KMS("crtc_state = %p need = %d avail = %d scaler_users = 
> 0x%x\n",
> - crtc_state, num_scalers_need, intel_crtc->num_scalers,
> - scaler_state->scaler_users);
>  
>   /*
>* High level flow:
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 21aa745caed1..52720ff9f769 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2935,8 +2935,6 @@ static void skl_detach_scaler(struct intel_crtc 
> *intel_crtc, int id)
>   I915_WRITE(SKL_PS_CTRL(intel_crtc->pipe, id), 0);
>   I915_WRITE(SKL_PS_WIN_POS(intel_crtc->pipe, id), 0);
>   I915_WRITE(SKL_PS_WIN_SZ(intel_crtc->pipe, id), 0);
> - DRM_DEBUG_KMS("CRTC:%d Disabled scaler id %u.%u\n",
> - intel_crtc->base.base.id, intel_crtc->pipe, id);
>  }
>  
>  /*
> -- 
> 2.1.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 v4 04/11] drm/i915: LVDS pixel clock check

[Mika Kahola]

On Fri, 2015-08-14 at 16:09 +0300, Ville Syrjälä wrote:
> On Fri, Aug 14, 2015 at 01:03:24PM +0300, Mika Kahola wrote:
> > It is possible the we request to have a mode that has
> > higher pixel clock than our HW can support. This patch
> > checks if requested pixel clock is lower than the one
> > supported by the HW. The requested mode is discarded
> > if we cannot support the requested pixel clock.
> > 
> > This patch applies to LVDS.
> > 
> > V2:
> > - removed computation for max pixel clock
> > 
> > V3:
> > - cleanup by removing unnecessary lines
> > 
> > V4:
> > - moved supported dotclock check from mode_valid() to intel_lvds_init()
> > 
> > Signed-off-by: Mika Kahola 
> > ---
> >  drivers/gpu/drm/i915/intel_lvds.c | 15 +++
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_lvds.c 
> > b/drivers/gpu/drm/i915/intel_lvds.c
> > index 881b5d1..06b9d1b 100644
> > --- a/drivers/gpu/drm/i915/intel_lvds.c
> > +++ b/drivers/gpu/drm/i915/intel_lvds.c
> > @@ -1098,8 +1098,11 @@ void intel_lvds_init(struct drm_device *dev)
> > drm_mode_debug_printmodeline(scan);
> >  
> > fixed_mode = drm_mode_duplicate(dev, scan);
> > -   if (fixed_mode)
> > -   goto out;
> > +   if (fixed_mode) {
> > +   if (fixed_mode->clock <= 
> > dev_priv->max_dotclk_freq)
> > +   goto out;
> > +   }
> > +   fixed_mode = NULL;
> > }
> > }
> >  
> > @@ -,8 +1114,10 @@ void intel_lvds_init(struct drm_device *dev)
> > fixed_mode = drm_mode_duplicate(dev, 
> > dev_priv->vbt.lfp_lvds_vbt_mode);
> > if (fixed_mode) {
> > fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
> > -   goto out;
> > +   if (fixed_mode->clock <= dev_priv->max_dotclk_freq)
> > +   goto out;
> > }
> > +   fixed_mode = NULL;
> > }
> >  
> > /*
> > @@ -1135,8 +1140,10 @@ void intel_lvds_init(struct drm_device *dev)
> > DRM_DEBUG_KMS("using current (BIOS) mode: ");
> > drm_mode_debug_printmodeline(fixed_mode);
> > fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
> > -   goto out;
> > +   if (fixed_mode->clock <= dev_priv->max_dotclk_freq)
> > +   goto out;
> > }
> > +   fixed_mode = NULL;
> 
> I don't think we want to special case just LVDS this way. Whatever we do
> with fixed_mode validation should be done for all connector types that
> have it. For now I think we can more or less ignore the issue.
> 
> So in that light, your previous patch was OK except it should have just
> checked fixed_mode->clock instead of mode->clock. Sorry if my ramblings
> confused you too much.
> 
No worries. I was a bit unsure if this check should have been done in
init anyway. I'll take a step back and modify the previous patch.

-Mika-
> > }
> >  
> > /* If we still don't have a mode after all that, give up. */
> > -- 
> > 1.9.1
> > 
> > ___
> > 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 v2 0/2] drm/i915/bxt: work around HW coherency issue

[Imre Deak]

On pe, 2015-08-14 at 13:49 +0100, Chris Wilson wrote:
> On Fri, Aug 14, 2015 at 03:38:55PM +0300, Imre Deak wrote:
> > This is a v2 of [1]. Since v1 the HW team confirmed that there is an
> > HW issue in A steppings with the GPU/CPU snoop logic, which explains why
> > we need this workaround.
> 
> I've been testing this extensively, and I do believe we can use clflush
> for all gen - it is a measurable improvement over using the
> heavyweight/locked read of ACTHD, and none of the coherency tests have
> thrown any warnings (over a period of a couple of months now).

Ok. Do you mean only places where there is already the ACTHD w/a, or to
extend it also to other platforms?

Imo doing this as a follow-up to this patchset would make sense, to keep
platform specific changes separate.

--Imre

___
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/bxt: work around HW coherency issue for cached GEM mappings

[Imre Deak]

On pe, 2015-08-14 at 14:11 +0100, Chris Wilson wrote:
> On Fri, Aug 14, 2015 at 03:38:57PM +0300, Imre Deak wrote:
> > Due to a coherency issue on BXT A steppings we can't guarantee a
> > coherent view of cached GPU mappings, so fall back to uncached mappings.
> > Note that this still won't fix cases where userspace expects a coherent
> > view without synchronizing (via a set domain call). It still makes sense
> > to limit the kernel's notion of the mapping to be uncached, for example
> > for relocations to work properly during execbuffer time. Also in case
> > user space does synchronize the buffer, this will still guarantee that
> > we'll do the proper clflushing for the buffer.
> > 
> > v2:
> > - limit the WA to A steppings, on later stepping this HW issue is fixed
> 
> This has to report the failure, ENODEV otherwise userspace will be
> terribly confused (it will try to CPU coherent access assuming it will
> be fast, when it is better to use alternative paths).

Ok, I was not sure how existing user space would handle the failure, but
if it has the fall-back logic then ENODEV is the better solution. Will
change this.

> -Chris
> 


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

Re: [Intel-gfx] [PATCH v2 1/2] drm/i915/bxt: work around HW coherency issue when accessing GPU seqno

[Imre Deak]

On pe, 2015-08-14 at 14:12 +0100, Chris Wilson wrote:
> On Fri, Aug 14, 2015 at 03:38:56PM +0300, Imre Deak wrote:
> > By running igt/store_dword_loop_render on BXT we can hit a coherency
> > problem where the seqno written at GPU command completion time is not
> > seen by the CPU. This results in __i915_wait_request seeing the stale
> > seqno and not completing the request (not considering the lost
> > interrupt/GPU reset mechanism). I also verified that this isn't a case
> > of a lost interrupt, or that the command didn't complete somehow: when
> > the coherency issue occured I read the seqno via an uncached GTT mapping
> > too. While the cached version of the seqno still showed the stale value
> > the one read via the uncached mapping was the correct one.
> > 
> > Work around this issue by clflushing the corresponding CPU cacheline
> > following any store of the seqno and preceding any reading of it. When
> > reading it do this only when the caller expects a coherent view.
> > 
> > v2:
> > - fix using the proper logical && instead of a bitwise & (Jani, Mika)
> > - limit the workaround to A stepping, on later steppings this HW issue
> >   is fixed
> 
> We have vfuncs in order to avoid the pointer dance (and boy is it a
> pretty and quite convoluted dance).

Ok, I'll add new get_seqno/set_seqno vfuncs.

> -Chris
> 


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

Re: [Intel-gfx] [PATCH v2 0/2] drm/i915/bxt: work around HW coherency issue

[Chris Wilson]

On Fri, Aug 14, 2015 at 04:26:29PM +0300, Imre Deak wrote:
> On pe, 2015-08-14 at 13:49 +0100, Chris Wilson wrote:
> > On Fri, Aug 14, 2015 at 03:38:55PM +0300, Imre Deak wrote:
> > > This is a v2 of [1]. Since v1 the HW team confirmed that there is an
> > > HW issue in A steppings with the GPU/CPU snoop logic, which explains why
> > > we need this workaround.
> > 
> > I've been testing this extensively, and I do believe we can use clflush
> > for all gen - it is a measurable improvement over using the
> > heavyweight/locked read of ACTHD, and none of the coherency tests have
> > thrown any warnings (over a period of a couple of months now).
> 
> Ok. Do you mean only places where there is already the ACTHD w/a, or to
> extend it also to other platforms?

I mean replace the current ACTHD w/a.
 
> Imo doing this as a follow-up to this patchset would make sense, to keep
> platform specific changes separate.

Agreed. Get the w/a in place, then do the conversion so that we have a
safe fallback.
-Chris

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

Re: [Intel-gfx] [PATCH] drm/dp/mst: Remove port after removing connector.

[shuang . he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: 
shuang...@intel.com)
Task id: 7141
-Summary-
Platform  Delta  drm-intel-nightly  Series Applied
ILK  302/302  302/302
SNB  315/315  315/315
IVB  336/336  336/336
BYT  283/283  283/283
HSW  378/378  378/378
-Detailed-
Platform  Testdrm-intel-nightly  Series 
Applied
Note: You need to pay more attention to line start with '*'
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx