Re: [Intel-gfx] [PATCH v3 i-g-t] tests/kms: increase max threshold time for edid read
With this tweak the test pass:
--- a/tests/kms_sysfs_edid_timing.c
+++ b/tests/kms_sysfs_edid_timing.c
@@ -99,7 +99,7 @@ int main(int argc, char **argv)
edid_size = read(fd_edid, edid, 512);
threshold = THRESHOLD_PER_EDID_BLOCK * (edid_size / 128);
- if (lspcon_present || (edid_size > 128 &&
+ if (lspcon_present || (edid_size >= 128 &&
!strncmp(de->d_name, "card0-HDMI", 10))) {
threshold *= HDMI_THRESHOLD_MULTIPLIER;
}
> -Original Message-
> From: Lofstedt, Marta
> Sent: Thursday, August 10, 2017 9:41 AM
> To: Taylor, Clinton A ; intel-
> g...@lists.freedesktop.org
> Cc: Vetter, Daniel
> Subject: RE: [PATCH v3 i-g-t] tests/kms: increase max threshold time for edid
> read
>
> Sorry Clinton I fail the test again on my BDW NUCi5 with this V3.
>
> Check: https://bugs.freedesktop.org/show_bug.cgi?id=100047
> For details.
>
> /Marta
>
> > -Original Message-
> > From: Taylor, Clinton A
> > Sent: Thursday, August 10, 2017 1:52 AM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Taylor, Clinton A ; Vetter, Daniel
> > ; Lofstedt, Marta
> > Subject: [PATCH v3 i-g-t] tests/kms: increase max threshold time for
> > edid read
> >
> > From: Clint Taylor
> >
> > Current 50ms max threshold timing for an EDID read is very close to
> > the actual time for a 2 block HDMI EDID read. Adjust the timings base
> > on connector type as DP reads are at 1 MBit and HDMI at 100K bit. If
> > an LSPcon is connected to device under test the -l option should be
> > passed to update the threshold timing to allow the LSPcon to read the EDID
> at the HDMI timing.
> > The -l option should be used when LSPcon is on the motherboard or if a
> > USB_C->HDMI converter is present
> >
> > V2: Adjust timings based on connector type, EDID size, and Add an
> > option to specify if an LSPcon is present.
> > V3: added bugzilla to commit message
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100047
> >
> > Cc: Daniel Vetter
> > Cc: Marta Lofstedt
> > Signed-off-by: Clint Taylor
> > ---
> > tests/kms_sysfs_edid_timing.c | 76
> > +++
> > 1 file changed, 62 insertions(+), 14 deletions(-)
> >
> > diff --git a/tests/kms_sysfs_edid_timing.c
> > b/tests/kms_sysfs_edid_timing.c index 1201388..441dfee 100644
> > --- a/tests/kms_sysfs_edid_timing.c
> > +++ b/tests/kms_sysfs_edid_timing.c
> > @@ -26,21 +26,46 @@
> > #include
> > #include
> >
> > -#define THRESHOLD_PER_CONNECTOR10
> > -#define THRESHOLD_TOTAL50
> > -#define CHECK_TIMES15
> > +#define THRESHOLD_FOR_EMPTY_CONNECTOR 10
> > +#define THRESHOLD_PER_EDID_BLOCK 5
> > +#define HDMI_THRESHOLD_MULTIPLIER 10
> > +#define CHECK_TIMES10
> >
> > IGT_TEST_DESCRIPTION("This check the time we take to read the content
> > of all "
> > "the possible connectors. Without the edid -
> ENXIO patch "
> >
> > "(http://permalink.gmane.org/gmane.comp.video.dri.devel/62083), "
> > -"we sometimes take a *really* long time. "
> > -"So let's just check for some reasonable
> > timing here");
> > +"we sometimes take a *really* long time. So
> > let's check "
> > +"an approximate time per edid block based on
> > connector "
> > +"type. The -l option adjusts DP timing to
> > reflect HDMI read "
> > +"timings from LSPcon.");
> > +
> > +/* The -l option has been added to correctly handle timings when an
> > +LSPcon is
> > + * present. This could be on the platform itself or in a USB_C->HDMI
> > converter.
> > + * With LSPCon EDID read timing will need to change from the 1 Mbit
> > +AUX
> > + * bus speed to the 100 Kbit HDMI DDC bus speed */ bool
> > +lspcon_present;
> >
> > +static int opt_handler(int opt, int opt_index, void *data) {
> > + if (opt == 'l') {
> > + lspcon_present = true;
> > + igt_info("set LSPcon present on DP ports\n");
> > + }
> >
> > -igt_simple_main
> > + return 0;
> > +}
> > +
> > +int main(int argc, char **argv)
> > {
> > DIR *dirp;
> > struct dirent *de;
> > + lspcon_present = false;
> > +
> > + igt_simple_init_parse_opts(&argc, argv, "l", NULL, NULL,
> > + opt_handler,
> > NULL);
> > +
> > + igt_skip_on_simulation();
> >
> > dirp = opendir("/sys/class/drm");
> > igt_assert(dirp != NULL);
> > @@ -49,17 +74,36 @@ igt_simple_main
> > struct igt_mean mean = {};
> > struct stat st;
> > char path[PATH_MAX];
> > - int i;
> > + char edid_path[PATH_MAX];
> > + char edid[512]; /* enough for 4 block edid */
> > + unsigned long edid_size = 0;
> > + int i, fd_edid;
> > + unsigned int threshold = 0;
> >
> > if (*de->d_name == '.')
> >
Re: [Intel-gfx] [maintainer-tools PATCH 5/5] doc: use Sphinx bizstyle builtin html theme
On Thu, 10 Aug 2017, Rodrigo Vivi wrote: > On Wed, Aug 9, 2017 at 2:08 PM, Jani Nikula wrote: >> Matter of taste, but looks better than the default alabaster, > > not sure which one I preferred... > alabaster goes better with 01.org... but it indeed is not very good... > >> and is >> less trouble than requiring some external themes (such as the sphinx rtd >> theme used in the kernel). > > less trouble is good, but since we are already making changes now and > forcing new packages I believe we could force to use same style as > kernel so we keep in sync... Looks like sphinx_rtd_theme is builtin since Sphinx 1.3, and I think we could easily require that. The trouble is that the sphinx_rtd_theme forces a fairly narrow layout horizontally, without horizontal scroll bars, and the wavedrom timeline overflows the content box and is clipped at the window border. Fail. I peeked at the Sphinx wavedrom extension, and incorporated their raw html hack to provide scrollbars for overflowing content. With hack [1], without hack [2]. What do people think? (Perhaps the right answer is to use something other than wavedrom altogether, but I don't have the time and I don't want it to block the Sphinx conversion.) BR, Jani. [1] https://people.freedesktop.org/~jani/html/drm-intel.html#merge-timeline [2] https://people.freedesktop.org/~jani/html/drm-misc.html#merge-timeline > >> >> Signed-off-by: Jani Nikula >> --- >> conf.py | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/conf.py b/conf.py >> index 2e7acb8e76ca..7293d9ddab80 100644 >> --- a/conf.py >> +++ b/conf.py >> @@ -119,7 +119,7 @@ todo_include_todos = False >> # The theme to use for HTML and HTML Help pages. See the documentation for >> # a list of builtin themes. >> # >> -html_theme = 'alabaster' >> +html_theme = 'bizstyle' >> >> # Theme options are theme-specific and customize the look and feel of a >> theme >> # further. For a list of options available for each theme, see the >> -- >> 2.11.0 >> >> ___ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915/perf: Drop lockdep assert for i915_oa_init_reg_state()
On 10/08/17 00:13, Chris Wilson wrote:
Quoting Lionel Landwerlin (2017-08-09 23:47:20)
On 09/08/17 16:38, Chris Wilson wrote:
This is called from execlist context init which we need to be unlocked.
Commit f89823c21224 ("drm/i915/perf: Implement
I915_PERF_ADD/REMOVE_CONFIG interface") added a lockdep assert to this
path for unclear reasons, remove it again!
Fixes: 701f8231a2fe ("drm/i915/perf: prune OA configs")
Signed-off-by: Chris Wilson
Cc: Lionel Landwerlin
Cc: Matthew Auld
---
drivers/gpu/drm/i915/i915_perf.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 1be355d14e8a..3bdf53faae24 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -2173,8 +2173,6 @@ void i915_oa_init_reg_state(struct intel_engine_cs
*engine,
struct drm_i915_private *dev_priv = engine->i915;
struct i915_perf_stream *stream = dev_priv->perf.oa.exclusive_stream;
I was trying to avoid adding a new lock for exclusive_stream.
If we can't rely on dev_priv->drm.struct_mutex to update
exclusive_stream, I believe we need to add a new lock.
Or maybe some other mechanism?
Doesn't changing the exclusive_stream require serialization against the
requests? Therefore we would be safe here for reset as the existence of
the incomplete request that we are altering is the barrier.
We serialize the requests to make sure that our modifications to the
context image have been applied.
So that when we return the perf stream fd, all work that was submitted
before the context image was modified has retired.
When new contexts are created, there is a need to set the OA parameters
in their image. But that has to read from the currently opened stream.
So there must be some kind of synchronization there.
I can make the access to exclusive_stream go through an srcu. Does that
sound like the right way to do it?
Wait... You don't have that barrier anymore? So you never change the
context image for the exclusive stream on setting and force the reload
of the image? I expected to see something like
gen8_configure_all_contexts(), but only needing to change the old/new
exclusive_streams.
At any rate, just wrapping exclusive_stream = bah inside struct_mutex is
not the barrier you intended, or at least not the barrier I think you need
wrt to request construction and concurrent execution thereof.
Longer term, I don't plan for the context allocation/initialisation to
be under the struct_mutex, but we can still expect it to be part of the
request construction, so would still be serialized by a future semaphore
between oa and execbuf.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915/perf: Drop lockdep assert for i915_oa_init_reg_state()
On 10/08/17 00:13, Chris Wilson wrote:
Quoting Lionel Landwerlin (2017-08-09 23:47:20)
On 09/08/17 16:38, Chris Wilson wrote:
This is called from execlist context init which we need to be unlocked.
Commit f89823c21224 ("drm/i915/perf: Implement
I915_PERF_ADD/REMOVE_CONFIG interface") added a lockdep assert to this
path for unclear reasons, remove it again!
Fixes: 701f8231a2fe ("drm/i915/perf: prune OA configs")
Signed-off-by: Chris Wilson
Cc: Lionel Landwerlin
Cc: Matthew Auld
---
drivers/gpu/drm/i915/i915_perf.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 1be355d14e8a..3bdf53faae24 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -2173,8 +2173,6 @@ void i915_oa_init_reg_state(struct intel_engine_cs
*engine,
struct drm_i915_private *dev_priv = engine->i915;
struct i915_perf_stream *stream = dev_priv->perf.oa.exclusive_stream;
I was trying to avoid adding a new lock for exclusive_stream.
If we can't rely on dev_priv->drm.struct_mutex to update
exclusive_stream, I believe we need to add a new lock.
Or maybe some other mechanism?
Doesn't changing the exclusive_stream require serialization against the
requests? Therefore we would be safe here for reset as the existence of
the incomplete request that we are altering is the barrier.
Wait... You don't have that barrier anymore? So you never change the
context image for the exclusive stream on setting and force the reload
of the image? I expected to see something like
gen8_configure_all_contexts(), but only needing to change the old/new
exclusive_streams.
We serialize the requests to make sure that our modifications to the
context image have been applied.
So that when we return the perf stream fd, all work that was submitted
before the context image was modified has retired.
When new contexts are created, there is a need to set the OA parameters
in their images. But that has to read configuration from the currently
opened stream.
So there must be some kind of synchronization there.
I can make the access to exclusive_stream go through a srcu. Do that
sound right?
At any rate, just wrapping exclusive_stream = bah inside struct_mutex is
not the barrier you intended, or at least not the barrier I think you need
wrt to request construction and concurrent execution thereof.
Longer term, I don't plan for the context allocation/initialisation to
be under the struct_mutex, but we can still expect it to be part of the
request construction, so would still be serialized by a future semaphore
between oa and execbuf.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 1/2] Add support for subtest-specific documentation
On Wed, Aug 09, 2017 at 01:18:55PM -0700, Belgaumkar, Vinay wrote:
> > @@ -710,6 +721,8 @@ static int common_init(int *argc, char **argv,
> > static struct option long_options[] = {
> > {"list-subtests", 0, 0, OPT_LIST_SUBTESTS},
> > {"run-subtest", 1, 0, OPT_RUN_SUBTEST},
> > + {"document-all-subtests", 0, 0, OPT_DOC_SUBTESTS},
> > + {"document-subtest", 1, 0, OPT_DOC_SINGLE_SUBTEST},
>
> Can we add these options to the README file as well?
>
Sending another series soon with README updated.
> > {"help-description", 0, 0, OPT_DESCRIPTION},
> > {"debug", optional_argument, 0, OPT_DEBUG},
> > {"interactive-debug", optional_argument, 0,
> > OPT_INTERACTIVE_DEBUG},
> > @@ -800,12 +813,24 @@ static int common_init(int *argc, char **argv,
> > igt_log_domain_filter = strdup(optarg);
> > break;
> > case OPT_LIST_SUBTESTS:
> > - if (!run_single_subtest)
> > - list_subtests = true;
> > + if (runmode == EXECUTE_ALL)
> > + runmode = LIST_SUBTESTS;
> > break;
> > case OPT_RUN_SUBTEST:
> > - if (!list_subtests)
> > - run_single_subtest = strdup(optarg);
> > + if (runmode == EXECUTE_ALL) {
> > + runmode = EXECUTE_SINGLE;
> > + single_subtest = strdup(optarg);
> > + }
> > + break;
> > + case OPT_DOC_SUBTESTS:
> > + if (runmode == EXECUTE_ALL)
> > + runmode = DOCUMENT;
> > + break;
> > + case OPT_DOC_SINGLE_SUBTEST:
> > + if (runmode == EXECUTE_ALL) {
> > + runmode = DOCUMENT_SINGLE;
> > + single_subtest = strdup(optarg);
> > + }
> > break;
> > case OPT_DESCRIPTION:
> > print_test_description();
> > @@ -837,11 +862,11 @@ out:
> > /* exit immediately if this test has no subtests and a subtest or the
> > * list of subtests has been requested */
> > if (!test_with_subtests) {
> > - if (run_single_subtest) {
> > - igt_warn("Unknown subtest: %s\n", run_single_subtest);
> > + if (runmode == EXECUTE_SINGLE || runmode == DOCUMENT_SINGLE) {
> > + igt_warn("Unknown subtest: %s\n", single_subtest);
> > exit(IGT_EXIT_INVALID);
> > }
> > - if (list_subtests)
> > + if (runmode == LIST_SUBTESTS || runmode == DOCUMENT)
> > exit(IGT_EXIT_INVALID);
> > }
> > @@ -849,7 +874,7 @@ out:
> > /* exit with no error for -h/--help */
> > exit(ret == -1 ? 0 : IGT_EXIT_INVALID);
> > - if (!list_subtests) {
> > + if (!igt_only_collect_data()) {
> > kick_fbcon(false);
> > kmsg(KERN_INFO "[IGT] %s: executing\n", command_str);
> > print_version();
> > @@ -957,16 +982,37 @@ bool __igt_run_subtest(const char *subtest_name)
> > igt_exit();
> > }
> > - if (list_subtests) {
> > + if (runmode == LIST_SUBTESTS) {
> > printf("%s\n", subtest_name);
> > return false;
> > }
> > - if (run_single_subtest) {
> > - if (uwildmat(subtest_name, run_single_subtest) == 0)
> > + if (runmode == DOCUMENT) {
> > + if (current_subtest_documentation) {
> > + printf("%s:\n\n", subtest_name);
> > + printf("%s", current_subtest_documentation);
> > + free(current_subtest_documentation);
> > + current_subtest_documentation = NULL;
> > + }
> > + return false;
> > + }
> > +
> > + if (runmode == EXECUTE_SINGLE || runmode == DOCUMENT_SINGLE) {
> > + if (uwildmat(subtest_name, single_subtest) == 0)
> > return false;
> > - else
> > - run_single_subtest_found = true;
> > + else {
> > + single_subtest_found = true;
> > +
> > + if (runmode == DOCUMENT_SINGLE) {
> > + if (current_subtest_documentation) {
> > + printf("%s",
> > current_subtest_documentation);
> > + free(current_subtest_documentation);
> > + current_subtest_documentation = NULL;
> > + }
> > +
> > + return false;
> > + }
> > + }
> > }
> > if (skip_subtests_henceforth) {
> > @@ -983,10 +1029,51 @@ bool __igt_run_subtest(const char *subtest_name)
> > _igt_log_buffer_reset();
> > gettime(&subtest_
Re: [Intel-gfx] [RFC i-g-t] tests/gem_exec_basic: Documentation for subtests
On Wed, Aug 09, 2017 at 10:00:16AM -0700, Belgaumkar, Vinay wrote: > > > On 8/9/2017 7:32 AM, Arkadiusz Hiler wrote: > > On Tue, Aug 08, 2017 at 03:09:00PM -0700, Vinay Belgaumkar wrote: > > > This is an RFC for adding documentation to IGT subtests. Each subtest can > > > have > > > something similar to a WHAT - explaining what the subtest actually does, > > > and a WHY - which explains a use case, if applicable. Additionally, > > > include comments for anything in the subtest code which can help > > > explain HOW the test has been implemented. We don't actually need the WHAT > > > and WHY tags in the documentation. > > > > > > These comments will not be linked to gtkdoc as of now, since we do not > > > have a > > > mechanism to link it to every subtest name. > > Hey Vinay, > > > > I get similar feelings towards this RFC as Lukasz and Radek do. > > > > Was your intention to propose format of the comments? Or maybe force > > people to comment more on the code? Or just pointing out that we could > > use some subtest documentation? > > > > You are not documenting subtests, you are documenting arbitrary > > functions that may or may not be used as a subtest. > > > > I cannot help but feel lost here. > > > > Being explicit as of your intention and coming up with more abstract or > > better examples would also help, as current ones are detracting from the > > idea itself. > > > > I do not get this RFC and it's purpose but I am looking forward to > > seeing revised version that is clearer on your intentions and easier to > > grasp. > > > > Hi Arek, > The purpose of this RFC is to complement Petri's subtest documentation > patch. That patch will give us > an ability to add a line of documentation per subtest, which is definitely > useful. However, what I noticed is > that when you actually start debugging a test issue and step into the > subtest code, it is very hard to understand > what the purpose of certain commands are. My intention was to provide a text > only documentation in the test source > to allow test developers to understand the code better. It's hard to explain > the how and and why all in a single sentence. > > If we provided an ability/guideline to test developers for mentioning the > same at the beginning of the actual subtest code, The ability is already there - the comments. What we need is the drive to do the commenting. IMO the issue is that one thing may be obvious to some and at the same time look arcane to others - people are generally biased and treat themselves as the baseline. This is not solvable by guideline or a RFC imposing a commenting format. This should be a joint effort: * authors trying to be more empathetic - hard to do and impossible to enforce * reviewers should point out things that are hard to understand and ask for clarification, preferably in a form of a slight rewrite of the code so it's more manageable, or as a comment if the former is not possible * people debugging such arcane parts (or asked the author) should take the effort and either make it more human readable or provide such commentary to save time of those who will come there after them In case you haven't noticed - recently we started requiring that all commits go through the mailing list, so everything is reviewed. Feel free to participate in the discussion. It's okay even if you ask for clarifications post-merge. They can be done in follow-up series, and those don't need to be done by the original author. As of guideline, having Petri's approach would already help a lot. I would prefer to see how it does, and after it proves (or not) take the further steps. > it can make debugging a lot more simpler rather than having to jump back to > the main function to figure out what the subtest > is supposed to do. I have also included some comments inside the test > function to explain why we use certain system calls. > Idea was not to define the system call, but explain why it is being used. Since you are "debugging" the subtest I don't think it's this much of an effort to do --list-subtest-with-doc or do jump in the code once or twice. IMO it's better to have the information provided only once. It's already hard to keep doc/comments up to date not dealing with such redundancy. > Again, I did not mean to duplicate the subtest documentation effort. My > initial plan was to send out an > RFC using Petri's patch as well, so that the intention is more clear. I can > do that with the second version of the patch if needed. > However, the main aim is to agree on a convention to add more documentation > to the subtest code so that it simplifies > debugging and helps with understanding of the aim behind writing the > particular subtest. Waiting for v2 then :-) -- Cheers, Arek ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [maintainer-tools PATCH 3/5] doc: use window.onload to call WaveDrom.ProcessAll()
On Thu, 10 Aug 2017, Rodrigo Vivi wrote:
> I don't know WaveDrom much... but it works so
>
> Acked-by: Rodrigo Vivi
Thanks, pushed the first three.
BR,
Jani.
>
> On Wed, Aug 9, 2017 at 2:08 PM, Jani Nikula wrote:
>> Simplify the build by doing the WaveDrom processing from the rst file
>> (albeit raw html block) instead of post-processing the output html.
>>
>> Signed-off-by: Jani Nikula
>> ---
>> Makefile | 6 --
>> drm-intel-timeline.rst | 4
>> drm-misc-timeline.rst | 4
>> 3 files changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 8bbabae56741..7059eec42720 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -12,15 +12,9 @@ all: drm-intel.html dim.html drm-misc.html
>> %.html: %.rst
>> rst2html $< > $@
>>
>> -# the sed bit here is a hack to make wavedrom process the timeline
>> drm-intel.html: drm-intel.rst drm-intel-flow.svg drm-intel-timeline.rst
>> drm-intel-timeline.json
>> - rst2html $< > $@
>> - sed -i 's/>
>> -# the sed bit here is a hack to make wavedrom process the timeline
>> drm-misc.html: drm-misc.rst drm-misc-timeline.rst drm-misc-timeline.json
>> drm-misc-commit-flow.svg
>> - rst2html $< > $@
>> - sed -i 's/>
>> dim.html: dim.rst
>>
>> diff --git a/drm-intel-timeline.rst b/drm-intel-timeline.rst
>> index e1766a5df98b..3ab39afd5788 100644
>> --- a/drm-intel-timeline.rst
>> +++ b/drm-intel-timeline.rst
>> @@ -17,6 +17,10 @@
>>
>> .. raw:: html
>>
>> + function init() {
>> + WaveDrom.ProcessAll();
>> + }
>> + window.onload = init;
>>
>>
>>
>> diff --git a/drm-misc-timeline.rst b/drm-misc-timeline.rst
>> index 69724362..a9a80d6a4cfb 100644
>> --- a/drm-misc-timeline.rst
>> +++ b/drm-misc-timeline.rst
>> @@ -8,6 +8,10 @@
>>
>> .. raw:: html
>>
>> + function init() {
>> + WaveDrom.ProcessAll();
>> + }
>> + window.onload = init;
>>
>>
[Intel-gfx] [PATCH i-g-t 1/5] Add support for subtest-specific documentation
The current documentation for tests is limited to a single string per
test binary. This patch adds support for documenting individual
subtests.
The syntax for subtest documentation is:
igt_document_subtest("Frob knobs to see if one of the "
"crossbeams will go out of skew on the "
"treadle.\n");
igt_subtest("knob-frobbing-askew")
test_frob();
or with a format string:
for_example_loop(e) {
igt_document_subtest_f("Frob %s to see if one of the "
"crossbeams will go out of skew on the "
"treadle.\n", e->readable_name);
igt_subtest_f("%s-frob-askew", e->name)
test_frob(e);
}
The documentation cannot be extracted from just comments, because
associating them with the correct subtest name will then require doing
pattern matching in the documentation generator, for subtests where
the name is generated at runtime using igt_subtest_f.
v2: Rebase, change function name in commit message to match code
v3: Fix current documentation string tracking, add missing va_end (Vinay)
Signed-off-by: Petri Latvala
Acked-by: Leo Li
Acked-by: Arkadiusz Hiler
---
lib/igt_aux.c | 8 ++--
lib/igt_core.c | 148 +
lib/igt_core.h | 6 ++-
3 files changed, 127 insertions(+), 35 deletions(-)
diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index f428f15..d56f41f 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -311,7 +311,7 @@ static void sig_handler(int i)
*/
void igt_fork_signal_helper(void)
{
- if (igt_only_list_subtests())
+ if (igt_only_collect_data())
return;
/* We pick SIGCONT as it is a "safe" signal - if we send SIGCONT to
@@ -344,7 +344,7 @@ void igt_fork_signal_helper(void)
*/
void igt_stop_signal_helper(void)
{
- if (igt_only_list_subtests())
+ if (igt_only_collect_data())
return;
igt_stop_helper(&signal_helper);
@@ -375,7 +375,7 @@ static void __attribute__((noreturn))
shrink_helper_process(int fd, pid_t pid)
*/
void igt_fork_shrink_helper(int drm_fd)
{
- assert(!igt_only_list_subtests());
+ assert(!igt_only_collect_data());
igt_require(igt_drop_caches_has(drm_fd, DROP_SHRINK_ALL));
igt_fork_helper(&shrink_helper)
shrink_helper_process(drm_fd, getppid());
@@ -473,7 +473,7 @@ void igt_stop_hang_detector(void)
#else
void igt_fork_hang_detector(int fd)
{
- if (igt_only_list_subtests())
+ if (igt_only_collect_data())
return;
}
diff --git a/lib/igt_core.c b/lib/igt_core.c
index c0488e9..7da9340 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -99,7 +99,7 @@
*
* To allow this i-g-t provides #igt_fixture code blocks for setup code outside
* of subtests and automatically skips the subtest code blocks themselves. For
- * special cases igt_only_list_subtests() is also provided. For setup code only
+ * special cases igt_only_collect_data() is also provided. For setup code only
* shared by a group of subtest encapsulate the #igt_fixture block and all the
* subtestest in a #igt_subtest_group block.
*
@@ -253,9 +253,9 @@ static unsigned int exit_handler_count;
const char *igt_interactive_debug;
/* subtests helpers */
-static bool list_subtests = false;
-static char *run_single_subtest = NULL;
-static bool run_single_subtest_found = false;
+static char *single_subtest = NULL;
+static bool single_subtest_found = false;
+static char *current_subtest_documentation = NULL;
static const char *in_subtest = NULL;
static struct timespec subtest_time;
static clockid_t igt_clock = (clockid_t)-1;
@@ -265,6 +265,13 @@ static bool in_atexit_handler = false;
static enum {
CONT = 0, SKIP, FAIL
} skip_subtests_henceforth = CONT;
+static enum {
+ EXECUTE_ALL,
+ EXECUTE_SINGLE,
+ LIST_SUBTESTS,
+ DOCUMENT,
+ DOCUMENT_SINGLE
+} runmode = EXECUTE_ALL;
bool __igt_plain_output = false;
@@ -277,6 +284,8 @@ bool test_child;
enum {
OPT_LIST_SUBTESTS,
OPT_RUN_SUBTEST,
+ OPT_DOC_SUBTESTS,
+ OPT_DOC_SINGLE_SUBTEST,
OPT_DESCRIPTION,
OPT_DEBUG,
OPT_INTERACTIVE_DEBUG,
@@ -469,7 +478,7 @@ bool __igt_fixture(void)
{
assert(!in_fixture);
- if (igt_only_list_subtests())
+ if (igt_only_collect_data())
return false;
if (skip_subtests_henceforth)
@@ -563,7 +572,7 @@ static void low_mem_killer_disable(bool disable)
bool igt_exit_called;
static void common_exit_handler(int sig)
{
- if (!igt_only_list_subtests()) {
+ if (!igt_only_collect_data()) {
low_mem_killer_disable(false);
kick_fbcon(true);
}
@@ -583,7 +592,7 @@ static void print_version(void)
{
struct utsname uts;
- if (list_subtests)
+ if (igt_only_collect_data())
return;
uname(&uts);
@@ -599,6 +608,8 @@ static void print_usage(const char
[Intel-gfx] [PATCH i-g-t 2/5] README: Describe the subtest documentation command line flags
Signed-off-by: Petri Latvala --- README | 5 + 1 file changed, 5 insertions(+) diff --git a/README b/README index d1ea952..5b6cb00 100644 --- a/README +++ b/README @@ -32,6 +32,11 @@ tests/ be run. Some tests have futher options and these are detailed by using the --help option. + Tests with subtests will print a documentation string (if any) + with the --document-subtest option. The option + --document-all-subtests will print the subtest documentation + for all subtests that have it. + The test suite can be run using the run-tests.sh script available in the scripts directory. Piglit is used to run the tests and can either be installed from your distribution (if available), or can be -- 2.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v3 0/5] Subtest documentation
New ever-growing series with some fixes and additions. Petri Latvala (5): Add support for subtest-specific documentation README: Describe the subtest documentation command line flags docs: Include subtest documentation testdisplay: Handle subtest documentation flags igt_command_line.sh: Test subtest documentation flag handling README | 5 + docs/reference/intel-gpu-tools/Makefile.am | 6 ++ lib/igt_aux.c | 8 +- lib/igt_core.c | 148 +++-- lib/igt_core.h | 6 +- tests/igt_command_line.sh | 12 ++- tests/testdisplay.c| 2 + 7 files changed, 151 insertions(+), 36 deletions(-) -- 2.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 4/5] testdisplay: Handle subtest documentation flags
testdisplay has no subtests so hook them to existing subtest handling.
Signed-off-by: Petri Latvala
---
TODO: Make testdisplay use igt_simple_init_parse_opts...
tests/testdisplay.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/testdisplay.c b/tests/testdisplay.c
index f2a41fa..85b118f 100644
--- a/tests/testdisplay.c
+++ b/tests/testdisplay.c
@@ -623,6 +623,8 @@ int main(int argc, char **argv)
struct option long_opts[] = {
{"list-subtests", 0, 0, SUBTEST_OPTS},
{"run-subtest", 1, 0, SUBTEST_OPTS},
+ {"document-all-subtests", 0, 0, SUBTEST_OPTS},
+ {"document-subtest", 1, 0, SUBTEST_OPTS},
{"help-description", 0, 0, HELP_DESCRIPTION},
{"help", 0, 0, 'h'},
{"yb", 0, 0, Yb_OPT},
--
2.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 5/5] igt_command_line.sh: Test subtest documentation flag handling
Signed-off-by: Petri Latvala --- tests/igt_command_line.sh | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/igt_command_line.sh b/tests/igt_command_line.sh index 7f80fc8..b3a794c 100755 --- a/tests/igt_command_line.sh +++ b/tests/igt_command_line.sh @@ -102,7 +102,17 @@ for test in $TESTLIST; do fi fi + # check --document-all-subtests handling + echo " Checking subtest documentation..." + DOCS=`./$test --document-all-subtests` + RET=$? + + if [ $RET -ne 0 -a $RET -ne 79 ]; then + fail $test + fi + # check invalid subtest handling echo " Checking invalid subtest handling..." - ./$test --run-subtest invalid-subtest > /dev/null 2>&1 && fail $test + ./$test --run-subtest invalid-subtest > /dev/null 2>&1 && fail "$test --run-subtest invalid" + ./$test --document-subtest invalid-subtest > /dev/null 2>&1 && fail "$test --document-subtest invalid" done -- 2.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 3/5] docs: Include subtest documentation
A simple and naive format: Double newline denotes paragraph change, otherwise insert subtest documentation into the generated docs as-is. Signed-off-by: Petri Latvala --- Still not sure whether inside is kosher. docs/reference/intel-gpu-tools/Makefile.am | 6 ++ 1 file changed, 6 insertions(+) diff --git a/docs/reference/intel-gpu-tools/Makefile.am b/docs/reference/intel-gpu-tools/Makefile.am index ee1e900..2407e37 100644 --- a/docs/reference/intel-gpu-tools/Makefile.am +++ b/docs/reference/intel-gpu-tools/Makefile.am @@ -56,6 +56,12 @@ xml/igt_test_programs_%_description.xml: $(TESTLISTS) for subtest in $$subtest_list; do \ echo "" >> $@; \ echo "$$subtest" | perl -pe 's/\b$(KEYWORDS)\b/\1<\/acronym>/g' >> $@; \ + subtest_doc=`./$$testprog --document-subtest $$subtest`; \ + if [ -n "$$subtest_doc" ]; then \ + echo "" >> $@; \ + fi; \ echo "" >> $@; \ done; \ echo "" >> $@; \ -- 2.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 8/8] drm/i915/skl+: debugfs entry to control IPC
Hi,
On Wednesday 09 August 2017 03:16 PM, Maarten Lankhorst wrote:
Op 18-07-17 om 14:49 schreef Mahesh Kumar:
From: "Kumar, Mahesh"
This patch creates an entry in debugfs to check the status of IPC.
This can also be used to enable/disable IPC in supported platforms.
Signed-off-by: Mahesh Kumar
---
drivers/gpu/drm/i915/i915_debugfs.c | 73 -
1 file changed, 72 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 2ef75c1a6119..368f64de0fdc 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3592,6 +3592,76 @@ static int i915_wa_registers(struct seq_file *m, void
*unused)
return 0;
}
+static int i915_ipc_status_show(struct seq_file *m, void *data)
+{
+ struct drm_i915_private *dev_priv = m->private;
+
+ seq_printf(m, "Isochronous Priority Control: %s\n",
+ enableddisabled(dev_priv->ipc_enabled));
+ return 0;
+}
+
+static int i915_ipc_status_open(struct inode *inode, struct file *file)
+{
+ struct drm_i915_private *dev_priv = inode->i_private;
+
+ if (HAS_IPC(dev_priv))
+ return -ENODEV;
+
+ return single_open(file, i915_ipc_status_show, dev_priv);
+}
+
+static ssize_t i915_ipc_status_write(struct file *file, const char __user
*ubuf,
+size_t len, loff_t *offp)
+{
+ struct seq_file *m = file->private_data;
+ struct drm_i915_private *dev_priv = m->private;
+ char *newline;
+ char tmp[16];
+ bool enable;
+
+ if (HAS_IPC(dev_priv))
+ return -ENODEV;
+
+ if (len >= sizeof(tmp))
+ return -EINVAL;
+
+ if (copy_from_user(tmp, ubuf, len))
+ return -EFAULT;
+
+ tmp[len] = '\0';
+
+ /* Strip newline, if any */
+ newline = strchr(tmp, '\n');
+ if (newline)
+ *newline = '\0';
+
+ if (strcmp(tmp, "0") == 0 || strcmp(tmp, "disable") == 0 ||
+ strcmp(tmp, "off") == 0 || strcmp(tmp, "dis") == 0)
+ enable = false;
+ else if (strcmp(tmp, "1") == 0 || strcmp(tmp, "enable") == 0 ||
+ strcmp(tmp, "on") == 0 || strcmp(tmp, "en") == 0)
+ enable = true;
+ else
+ return -EINVAL;
+
+ intel_runtime_pm_get(dev_priv);
+ dev_priv->ipc_enabled = enable;
+ intel_enable_ipc(dev_priv);
+ intel_runtime_pm_put(dev_priv);
Hm, intel_enable_ipc should take a bool on whether to enable ipc.
intel_enable_ipc takes decision to enable/disable ipc based on
dev_priv->ipc_enabled value. which is anyway input to function.
Forcefully enabling IPC here might give weird effects until the next plane
update. The transition watermarks are not updated yet until the next commit.
This could probably be handled here, but that would be overkill..
yes, agree, but I don't wanted to call a commit internally. Will add a
drm_info when changing IPC disable => enable as suggested.
-Mahesh
Perhaps add a drm_info or something about it when setting enable = true, when
it was previously false?
+
+ return len;
+}
+
+static const struct file_operations i915_ipc_status_fops = {
+ .owner = THIS_MODULE,
+ .open = i915_ipc_status_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+ .write = i915_ipc_status_write
+};
+
static int i915_ddb_info(struct seq_file *m, void *unused)
{
struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -4929,7 +4999,8 @@ static const struct i915_debugfs_files {
{"i915_dp_test_type", &i915_displayport_test_type_fops},
{"i915_dp_test_active", &i915_displayport_test_active_fops},
{"i915_guc_log_control", &i915_guc_log_control_fops},
- {"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops}
+ {"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops},
+ {"i915_ipc_status", &i915_ipc_status_fops}
};
int i915_debugfs_register(struct drm_i915_private *dev_priv)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH libdrm] drm: Remove create_handle() drm_framebuffer "virtual".
On Wed, Aug 09, 2017 at 04:13:05PM -0700, Joe Kniss wrote:
> On Wed, Aug 9, 2017 at 12:14 PM, Noralf Trønnes wrote:
> >
> > Den 09.08.2017 01.42, skrev Joe Kniss:
> >>
> >> Because all drivers currently use gem objects for framebuffer planes,
> >> the virtual create_handle() is not required. This change adds a
> >> struct drm_gem_object *gems[4] field to drm_framebuffer and removes
> >> create_handle() function pointer from drm_framebuffer_funcs. The
> >> corresponding *_create_handle() function is removed from each driver.
> >>
> >> In many cases this change eliminates a struct *_framebuffer object,
> >> as the only need for the derived struct is the addition of the gem
> >> object pointer.
> >>
> >> TESTED: compiled: allyesconfig ARCH=x86,arm platforms:i915, rockchip
> >>
> >> Signed-off-by: Joe Kniss
> >> ---
> >
> >
> > Hi Joe,
> >
> > I'm also looking into adding gem objs to drm_framebuffer in this patch:
> > [PATCH v2 01/22] drm: Add GEM backed framebuffer library
> > https://lists.freedesktop.org/archives/dri-devel/2017-August/149782.html
> >
>
> Great. There's only minimal overlap here. I'll rebase this change on yours
> once it's in.
Even better than waiting: Reviewing each another's patches. Noralf has
commit rights and knows how driver-wide refactoring works, so you're all
covered to get this landed, if you provide a bit of review for the review
market :-)
Cheers, Daniel
>
> > [...]
> >
> >> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> >> b/drivers/gpu/drm/drm_fb_cma_helper.c
> >> index ade319d10e70..f5f011b910b1 100644
> >> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> >> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> >> @@ -31,14 +31,9 @@
> >> #define DEFAULT_FBDEFIO_DELAY_MS 50
> >> -struct drm_fb_cma {
> >> - struct drm_framebuffer fb;
> >> - struct drm_gem_cma_object *obj[4];
> >> -};
> >> -
> >> struct drm_fbdev_cma {
> >> struct drm_fb_helperfb_helper;
> >> - struct drm_fb_cma *fb;
> >> + struct drm_framebuffer *fb;
> >
> >
> > This fb pointer isn't necessary, since fb_helper already has one.
> >
>
> I'll remove it... but I am sure when I look deeper there will be more
> of these in the various drivers too.
>
> > Noralf.
> >
> >
>
> -joe
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://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
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for Subtest documentation
== Series Details == Series: Subtest documentation URL : https://patchwork.freedesktop.org/series/28605/ State : failure == Summary == IGT patchset tested on top of latest successful build c129026622accef6f54c0cfb0dc55e930cfa60b5 igt: add syncobj_basic. with latest DRM-Tip kernel build CI_DRM_2942 2d0288b5b28c drm-tip: 2017y-08m-09d-18h-09m-54s UTC integration manifest Test drv_hangman: Subgroup error-state-basic: pass -> INCOMPLETE (fi-bxt-j4205) Test gem_busy: Subgroup basic-busy-default: pass -> INCOMPLETE (fi-ilk-650) pass -> INCOMPLETE (fi-snb-2520m) pass -> INCOMPLETE (fi-snb-2600) pass -> INCOMPLETE (fi-ivb-3520m) pass -> INCOMPLETE (fi-ivb-3770) pass -> INCOMPLETE (fi-byt-j1900) pass -> INCOMPLETE (fi-hsw-4770) pass -> INCOMPLETE (fi-hsw-4770r) pass -> INCOMPLETE (fi-bdw-5557u) pass -> INCOMPLETE (fi-bdw-gvtdvm) pass -> INCOMPLETE (fi-bsw-n3050) pass -> INCOMPLETE (fi-skl-6260u) pass -> INCOMPLETE (fi-skl-6700k) pass -> INCOMPLETE (fi-skl-6770hq) pass -> INCOMPLETE (fi-skl-gvtdvm) pass -> INCOMPLETE (fi-skl-x1585l) pass -> INCOMPLETE (fi-kbl-7500u) pass -> INCOMPLETE (fi-kbl-7560u) pass -> INCOMPLETE (fi-kbl-r) pass -> INCOMPLETE (fi-glk-2a) Test gem_close_race: Subgroup basic-process: pass -> INCOMPLETE (fi-blb-e6850) pass -> INCOMPLETE (fi-pnv-d510) fi-bdw-5557u total:10 pass:9dwarn:0 dfail:0 fail:0 skip:0 fi-bdw-gvtdvmtotal:10 pass:9dwarn:0 dfail:0 fail:0 skip:0 fi-blb-e6850 total:12 pass:11 dwarn:0 dfail:0 fail:0 skip:0 fi-bsw-n3050 total:10 pass:9dwarn:0 dfail:0 fail:0 skip:0 fi-bxt-j4205 total:6pass:5dwarn:0 dfail:0 fail:0 skip:0 fi-byt-j1900 total:10 pass:9dwarn:0 dfail:0 fail:0 skip:0 fi-glk-2atotal:10 pass:9dwarn:0 dfail:0 fail:0 skip:0 fi-hsw-4770 total:10 pass:9dwarn:0 dfail:0 fail:0 skip:0 fi-hsw-4770r total:10 pass:9dwarn:0 dfail:0 fail:0 skip:0 fi-ilk-650 total:10 pass:9dwarn:0 dfail:0 fail:0 skip:0 fi-ivb-3520m total:10 pass:9dwarn:0 dfail:0 fail:0 skip:0 fi-ivb-3770 total:10 pass:9dwarn:0 dfail:0 fail:0 skip:0 fi-kbl-7500u total:10 pass:9dwarn:0 dfail:0 fail:0 skip:0 fi-kbl-7560u total:10 pass:9dwarn:0 dfail:0 fail:0 skip:0 fi-kbl-r total:10 pass:9dwarn:0 dfail:0 fail:0 skip:0 fi-pnv-d510 total:12 pass:11 dwarn:0 dfail:0 fail:0 skip:0 fi-skl-6260u total:10 pass:9dwarn:0 dfail:0 fail:0 skip:0 fi-skl-6700k total:10 pass:9dwarn:0 dfail:0 fail:0 skip:0 fi-skl-6770hqtotal:10 pass:9dwarn:0 dfail:0 fail:0 skip:0 fi-skl-gvtdvmtotal:10 pass:9dwarn:0 dfail:0 fail:0 skip:0 fi-skl-x1585ltotal:10 pass:9dwarn:0 dfail:0 fail:0 skip:0 fi-snb-2520m total:10 pass:9dwarn:0 dfail:0 fail:0 skip:0 fi-snb-2600 total:10 pass:9dwarn:0 dfail:0 fail:0 skip:0 == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_51/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 4/4] tests/syncobj: Add some wait and reset tests (v5)
Quoting Jason Ekstrand (2017-08-10 06:35:43)
> +igt_main
> +{
> + int fd;
> +
> + igt_fixture {
> + fd = drm_open_driver(DRIVER_ANY);
DRIVER_ANY | DRIVER_VGEM
(ANY really means KMS)
> + igt_require(has_syncobj_wait(fd));
> + igt_require_sw_sync();
We should add something like
igt_require(has_timer_resolution(CLOCK_MONOTONIC, SHORT_TIME_NSEC / 2));
static bool has_timer_resolution(clockid_t clk, uint64_t target_ns)
{
struct timespec res;
if (clock_getres(clk, &res))
return false; /* clock unavailable */
return res.tv_sec * NSEC_PER_SEC + res.tv_nsec <= target_ns;
}
That'll rule out testing on pnv most likely.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 8/8] drm/i915/skl+: debugfs entry to control IPC
Op 10-08-17 om 10:56 schreef Mahesh Kumar:
> Hi,
>
>
> On Wednesday 09 August 2017 03:16 PM, Maarten Lankhorst wrote:
>> Op 18-07-17 om 14:49 schreef Mahesh Kumar:
>>> From: "Kumar, Mahesh"
>>>
>>> This patch creates an entry in debugfs to check the status of IPC.
>>> This can also be used to enable/disable IPC in supported platforms.
>>>
>>> Signed-off-by: Mahesh Kumar
>>> ---
>>> drivers/gpu/drm/i915/i915_debugfs.c | 73
>>> -
>>> 1 file changed, 72 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>>> b/drivers/gpu/drm/i915/i915_debugfs.c
>>> index 2ef75c1a6119..368f64de0fdc 100644
>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>>> @@ -3592,6 +3592,76 @@ static int i915_wa_registers(struct seq_file *m,
>>> void *unused)
>>> return 0;
>>> }
>>> +static int i915_ipc_status_show(struct seq_file *m, void *data)
>>> +{
>>> +struct drm_i915_private *dev_priv = m->private;
>>> +
>>> +seq_printf(m, "Isochronous Priority Control: %s\n",
>>> +enableddisabled(dev_priv->ipc_enabled));
>>> +return 0;
>>> +}
>>> +
>>> +static int i915_ipc_status_open(struct inode *inode, struct file *file)
>>> +{
>>> +struct drm_i915_private *dev_priv = inode->i_private;
>>> +
>>> +if (HAS_IPC(dev_priv))
>>> +return -ENODEV;
>>> +
>>> +return single_open(file, i915_ipc_status_show, dev_priv);
>>> +}
>>> +
>>> +static ssize_t i915_ipc_status_write(struct file *file, const char __user
>>> *ubuf,
>>> + size_t len, loff_t *offp)
>>> +{
>>> +struct seq_file *m = file->private_data;
>>> +struct drm_i915_private *dev_priv = m->private;
>>> +char *newline;
>>> +char tmp[16];
>>> +bool enable;
>>> +
>>> +if (HAS_IPC(dev_priv))
>>> +return -ENODEV;
>>> +
>>> +if (len >= sizeof(tmp))
>>> +return -EINVAL;
>>> +
>>> +if (copy_from_user(tmp, ubuf, len))
>>> +return -EFAULT;
>>> +
>>> +tmp[len] = '\0';
>>> +
>>> +/* Strip newline, if any */
>>> +newline = strchr(tmp, '\n');
>>> +if (newline)
>>> +*newline = '\0';
>>> +
>>> +if (strcmp(tmp, "0") == 0 || strcmp(tmp, "disable") == 0 ||
>>> +strcmp(tmp, "off") == 0 || strcmp(tmp, "dis") == 0)
>>> +enable = false;
>>> +else if (strcmp(tmp, "1") == 0 || strcmp(tmp, "enable") == 0 ||
>>> +strcmp(tmp, "on") == 0 || strcmp(tmp, "en") == 0)
>>> +enable = true;
>>> +else
>>> +return -EINVAL;
>>> +
>>> +intel_runtime_pm_get(dev_priv);
>>> +dev_priv->ipc_enabled = enable;
>>> +intel_enable_ipc(dev_priv);
>>> +intel_runtime_pm_put(dev_priv);
>> Hm, intel_enable_ipc should take a bool on whether to enable ipc.
> intel_enable_ipc takes decision to enable/disable ipc based on
> dev_priv->ipc_enabled value. which is anyway input to function.
>>
>> Forcefully enabling IPC here might give weird effects until the next plane
>> update. The transition watermarks are not updated yet until the next commit.
>> This could probably be handled here, but that would be overkill..
> yes, agree, but I don't wanted to call a commit internally. Will add a
> drm_info when changing IPC disable => enable as suggested.
True, perhaps we could set dev_priv->wm.distrust_bios_wm to force recalculation
next time?
~Maarten
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/8] dmi: Move memdev_dmi_entry definition to dmi.h (v2)
On mer., 2017-08-09 at 09:18 -0700, Matt Roper wrote: > On Mon, Jul 31, 2017 at 10:36:05AM +0200, Jean Delvare wrote: > > > > Hi Matt, Mauro, > > > > On Thu, 17 Mar 2016 15:18:20 +0100, Jean Delvare wrote: > > > > > > On Tue, 8 Mar 2016 10:32:37 -0800, Matt Roper wrote: > > > > > > > > A couple of the EDAC drivers have a nice memdev_dmi_entry structure for > > > > decoding DMI memory device entries. Move the structure definition to > > > > dmi.h so that it can be shared between those drivers and also other > > > > parts of the kernel; the i915 graphics driver is going to need to use > > > > this structure soon as well. As part of this move we rename the > > > > structure s/memdev_dmi_entry/dmi_entry_memdev/ to ensure it has a proper > > > > 'dmi' prefix. > > > > > > > > v2: > > > > - Rename structure to dmi_entry_memdev. (Jean) > > > > - Use __packed instead of __attribute__((__packed__)) for consistency > > > >with the rest of the dmi.h header. (Jean) > > > > > > Looks better. (...) > > > > What happened to this patch? I never received v3. Is it sill needed? > > We ended up going a different direction in the graphics driver and wound > up not needing access to this structure. If there's still interest in > the general refactoring here, let me know and I can incorporate your > last feedback and respin a v3. No, if you don't need it anymore I'll just drop it. Thanks, -- Jean Delvare SUSE L3 Support ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Add SW_SYNC to our recommend testing Kconfig
Since we do use the SW_SYNC in igt for validating dma-fence and sync_file, and wish to expand usage to cover driver independent portions of syncobj interaction, ensure SW_SYNC is included in our testing Kconfig. Signed-off-by: Chris Wilson Cc: Jason Ekstrand Cc: Joonas Lahtinen --- drivers/gpu/drm/i915/Kconfig.debug | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug index 78c5c049a347..aed7d207ea84 100644 --- a/drivers/gpu/drm/i915/Kconfig.debug +++ b/drivers/gpu/drm/i915/Kconfig.debug @@ -25,6 +25,7 @@ config DRM_I915_DEBUG select DRM_VGEM # used by igt/prime_vgem (dmabuf interop checks) select DRM_DEBUG_MM if DRM=y select DRM_DEBUG_MM_SELFTEST + select SW_SYNC # signaling validation framework (igt/syncobj*) select DRM_I915_SW_FENCE_DEBUG_OBJECTS select DRM_I915_SELFTEST default n -- 2.13.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 4/4] tests/syncobj: Add some wait and reset tests (v5)
Quoting Jason Ekstrand (2017-08-10 06:35:43)
> This adds both trivial error-checking tests as well as more complex
> tests which actually test whether or not waits do what they're supposed
> to do. They only currently work on i915 but it should be simple to hook
> them up for other drivers by simply implementing the little function
> pointer hook provided at the top for triggering a syncobj.
>
> v2:
> - Actually add the reset tests.
> v3:
> - Only do one execbuf for trigger
> - Use do_ioctl and do_ioctl_err
> - Better check for syncobj support
> - Add local_/LOCAL_ defines of things
> - Use a timer instead of a pthread
> v4:
> - Use ioctl wrappers
> - Use VGEM instead of i915
> - Combine a bunch of the simple tests into one function
> v5:
> - Combinatorially generate basic tests
> - Use sw_sync instead of using vgem directly
Aye, sw_sync looks to be quite useful here - a completely driver
agnostic method for signaling syncobj. Nice.
> +static int
> +syncobj_attach_sw_sync(int fd, uint32_t handle)
> +{
> + struct drm_syncobj_handle;
> + int timeline, fence;
> +
> + timeline = sw_sync_timeline_create();
> + fence = sw_sync_timeline_create_fence(timeline, 1);
> + syncobj_import_sync_file(fd, handle, fence);
> + close(fence);
> +
> + return timeline;
> +}
> +
> +static void
> +syncobj_trigger(int fd, uint32_t handle)
> +{
> + int timeline = syncobj_attach_sw_sync(fd, handle);
> + sw_sync_timeline_inc(timeline, 1);
> +}
> +
> +struct delayed_trigger {
> + int fd;
> + uint32_t *syncobjs;
> + int count;
> + uint64_t nsec;
> +};
> +
> +static void
> +trigger_syncobj_delayed_func(union sigval sigval)
> +{
> + struct delayed_trigger *trigger = sigval.sival_ptr;
> + int i;
> +
> + for (i = 0; i < trigger->count; i++)
> + syncobj_trigger(trigger->fd, trigger->syncobjs[i]);
> + free(trigger);
> +}
> +
> +static timer_t
> +trigger_syncobj_delayed(int fd, uint32_t *syncobjs, int count, uint64_t nsec)
> +{
> + struct delayed_trigger *trigger;
> +timer_t timer;
> +struct sigevent sev;
> +struct itimerspec its;
> +
> + trigger = malloc(sizeof(*trigger));
> + trigger->fd = fd;
> + trigger->syncobjs = syncobjs;
> + trigger->count = count;
> + trigger->nsec = nsec;
> +
> +memset(&sev, 0, sizeof(sev));
> +sev.sigev_notify = SIGEV_THREAD;
> +sev.sigev_value.sival_ptr = trigger;
> +sev.sigev_notify_function = trigger_syncobj_delayed_func;
> +igt_assert(timer_create(CLOCK_MONOTONIC, &sev, &timer) == 0);
> +
> +memset(&its, 0, sizeof(its));
> +its.it_value.tv_sec = nsec / NSEC_PER_SEC;
> +its.it_value.tv_nsec = nsec % NSEC_PER_SEC;
> +igt_assert(timer_settime(timer, 0, &its, NULL) == 0);
> +
> + return timer;
> +}
static void
trigger_syncobj_delayed_func(union sigval sigval)
{
int timeline = (intptr_t)sigval.sival_ptr;
sw_sync_timeline_inc(timeline, 1);
close(timeline);
}
static timer_t
trigger_syncobj_delayed(int fd, uint32_t *syncobjs, int count, uint64_t nsec)
{
struct itimerspec its;
struct sigevent sev;
int timeline, fence;
timer_t timer;
timeline = sw_sync_timeline_create();
fence = sw_sync_timeline_create_fence(timeline, 1);
for (int i = 0; i < count; i++)
syncobj_import_sync_file(fd, syncobjs[i], fence);
close(fence);
memset(&sev, 0, sizeof(sev));
sev.sigev_notify = SIGEV_THREAD;
sev.sigev_value.sival_ptr = (intptr_t)timeline;
sev.sigev_notify_function = trigger_syncobj_delayed_func;
igt_assert(timer_create(CLOCK_MONOTONIC, &sev, &timer) == 0);
memset(&its, 0, sizeof(its));
its.it_value.tv_sec = nsec / NSEC_PER_SEC;
its.it_value.tv_nsec = nsec % NSEC_PER_SEC;
igt_assert(timer_settime(timer, 0, &its, NULL) == 0);
return timer;
}
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 4/4] tests/syncobj: Add some wait and reset tests (v5)
Quoting Jason Ekstrand (2017-08-10 06:35:43)
> +static void
> +test_single_wait(int fd, uint32_t test_flags, int expect)
> +{
> + uint32_t syncobj = syncobj_create(fd);
> + uint32_t flags = 0;
> + int timeline;
> +
> + if (test_flags & WAIT_FOR_SUBMIT)
> + flags |= LOCAL_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT;
> +
> + if (test_flags & WAIT_ALL)
> + flags |= LOCAL_SYNCOBJ_WAIT_FLAGS_WAIT_ALL;
> +
> + if (test_flags & (WAIT_SUBMITTED | WAIT_SIGNALED))
> + timeline = syncobj_attach_sw_sync(fd, syncobj);
> +
> + if (test_flags & WAIT_SIGNALED)
> + sw_sync_timeline_inc(timeline, 1);
> +
> + igt_assert_eq(syncobj_wait_err(fd, &syncobj, 1, 0, flags), expect);
> +
> + igt_assert_eq(syncobj_wait_err(fd, &syncobj, 1, short_timeout(),
> + flags), expect);
> +
> + if (expect != -ETIME) {
> + igt_assert_eq(syncobj_wait_err(fd, &syncobj, 1, UINT64_MAX,
> + flags), expect);
Won't this fail in 6000 years time... Perhaps not an issue, but
maximum duration is gettime_ns() + INT64_MAX (and these tests serve as
part of the ABI guide / documentation as well as checking the contract).
We don't have an encoding for an infinite wait, I guess several millennia
will just have to do.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [maintainer-tools PATCH v2 1/4] doc: load WaveDrom scripts directly from CDN instead of bundling
Way back when the WaveDrom stuff was added, the scripts could only be accessed over http. This caused issues with sites served over https and modern browsers rightly complaining about mixed content. This was worked around by downloading the WaveDrom scripts over http at build time, and bundling them inline into the http. Now that WaveDrom is available over https, simplify the hackery, and let the user's browser load the scripts directly at page load time. Signed-off-by: Jani Nikula --- drm-intel-timeline.rst | 20 +++- drm-misc-timeline.rst | 21 +++-- 2 files changed, 6 insertions(+), 35 deletions(-) diff --git a/drm-intel-timeline.rst b/drm-intel-timeline.rst index 3ab39afd5788..fe69fd374cf6 100644 --- a/drm-intel-timeline.rst +++ b/drm-intel-timeline.rst @@ -1,22 +1,8 @@ .. raw:: html - - /* Embedded WaveDrom skin from http://wavedrom.com/skins/default.js */ - -.. raw:: html - :url: http://wavedrom.com/skins/default.js - -.. raw:: html - - - - /* Embedded WaveDrom engine from http://wavedrom.com/WaveDrom.js */ - -.. raw:: html - :url: http://wavedrom.com/WaveDrom.js - -.. raw:: html - +