Re: [Intel-gfx] [PATCH i-g-t v3 17/17] lib/kunit: Omit suite name prefix if the same as subtest name

2023-09-18 Thread Mauro Carvalho Chehab
On Mon, 18 Sep 2023 15:43:07 +0200
Janusz Krzysztofik  wrote:

> Kunit test modules usually contain one test suite, named after the module
> name with the trailing "_test" or "_kunit" suffix omitted.  Since we
> follow the same convention when we derive subtest names from module names,
> there is a great chance that those two names match.  Take this into
> account when composing names for IGT dynamic sub-subtest names and drop
> the leading test suite name component when it is the same as subtest name.
> 
> Signed-off-by: Janusz Krzysztofik 

Reviewed-by: Mauro Carvalho Chehab 

> ---
>  lib/igt_kmod.c | 11 ---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> index f6e0ab07ce..05c837031c 100644
> --- a/lib/igt_kmod.c
> +++ b/lib/igt_kmod.c
> @@ -863,7 +863,8 @@ static void kunit_result_free(struct igt_ktap_result *r,
>   *
>   * Returns: IGT default codes
>   */
> -static void __igt_kunit(struct igt_ktest *tst, const char *opts)
> +static void
> +__igt_kunit(struct igt_ktest *tst, const char *name, const char *opts)
>  {
>   struct modprobe_data modprobe = { pthread_self(), tst->kmod, opts, 0,
> PTHREAD_MUTEX_INITIALIZER, };
> @@ -902,7 +903,11 @@ static void __igt_kunit(struct igt_ktest *tst, const 
> char *opts)
>  
>   r = igt_list_first_entry(&results, r, link);
>  
> - igt_dynamic_f("%s-%s", r->suite_name, r->case_name) {
> + igt_dynamic_f("%s%s%s",
> +   strcmp(r->suite_name, name) ?  r->suite_name : "",
> +   strcmp(r->suite_name, name) ? "-" : "",
> +   r->case_name) {
> +
>   if (r->code == IGT_EXIT_INVALID) {
>   /* parametrized test case, get actual result */
>   kunit_result_free(r, &suite_name, &case_name);
> @@ -1011,7 +1016,7 @@ void igt_kunit(const char *module_name, const char 
> *name, const char *opts)
>* and for documentation.
>*/
>   igt_subtest_with_dynamic(name)
> - __igt_kunit(&tst, opts);
> + __igt_kunit(&tst, name, opts);
>  
>   igt_fixture
>   igt_ktest_end(&tst);


Re: [Intel-gfx] [PATCH i-g-t v3 07/17] lib/ktap: Drop checks for EINTR on read() failures

2023-09-18 Thread Mauro Carvalho Chehab
On Mon, 18 Sep 2023 15:42:57 +0200
Janusz Krzysztofik  wrote:

> While reading KTAP data from /dev/kmsg we now ignore EINTR failures that
> may occur during read() and we continue reading the data.  No explanation
> has been provided on what that could be needed for.
> 
> Since we use default SIGINT signal handler, read() should never fail with
> errno set to EINTR on user interrupt, only the whole process should be
> terminated.  Drop checks for errno == EINTR as not applicable.

This explanation makes sense to me.

Acked-by: Mauro Carvalho Chehab 


> 
> v2: Drop handling of EINTR completely, update commit message and
> descripion.
> 
> Signed-off-by: Janusz Krzysztofik 
> Cc: Mauro Carvalho Chehab 

> ---
>  lib/igt_ktap.c | 15 ---
>  1 file changed, 15 deletions(-)
> 
> diff --git a/lib/igt_ktap.c b/lib/igt_ktap.c
> index 84fb13218f..ce07f9aed7 100644
> --- a/lib/igt_ktap.c
> +++ b/lib/igt_ktap.c
> @@ -66,9 +66,6 @@ static int log_to_end(enum igt_log_level level, int fd,
>   return -2;
>   }
>  
> - if (errno == EINTR)
> - continue;
> -
>   if (errno == EPIPE) {
>   igt_warn("kmsg truncated: too many messages. 
> You may want to increase log_buf_len in kmcdline\n");
>   return -2;
> @@ -188,9 +185,6 @@ static int find_next_tap_subtest(int fd, char *record, 
> char *test_name, bool is_
>   return -2;
>   }
>  
> - if (errno == EINTR)
> - continue;
> -
>   if (errno == EPIPE) {
>   igt_warn("kmsg truncated: too many messages. 
> You may want to increase log_buf_len in kmcdline\n");
>   return -2;
> @@ -232,9 +226,6 @@ static int find_next_tap_subtest(int fd, char *record, 
> char *test_name, bool is_
>   return -2;
>   }
>  
> - if (errno == EINTR)
> - continue;
> -
>   if (errno == EPIPE) {
>   igt_warn("kmsg truncated: too many messages. 
> You may want to increase log_buf_len in kmcdline\n");
>   return -2;
> @@ -387,9 +378,6 @@ static int parse_tap_level(int fd, char *base_test_name, 
> int test_count, bool *f
>   return -1;
>   }
>  
> - if (errno == EINTR)
> - continue;
> -
>   if (errno == EAGAIN)
>   /* No records available */
>   continue;
> @@ -540,9 +528,6 @@ igt_ktap_parser_start:
>   /* No records available */
>   continue;
>  
> - if (errno == EINTR)
> - continue;
> -
>   if (errno == EPIPE) {
>   igt_warn("kmsg truncated: too many messages. You may 
> want to increase log_buf_len in kmcdline\n");
>   goto igt_ktap_parser_end;


Re: [Intel-gfx] [PATCH i-g-t v3 05/17] lib/kunit: Fix illegal igt_fail() calls inside subtest body

2023-09-18 Thread Mauro Carvalho Chehab
On Tue, 19 Sep 2023 08:25:22 +0200
Mauro Carvalho Chehab  wrote:

> On Mon, 18 Sep 2023 15:42:55 +0200
> Janusz Krzysztofik  wrote:
> 
> > In a body of a subtest with dynamic sub-subtests, it is illegal to call
> > igt_fail() and its variants from outside of a dynamic sub-subtest body.
> > On the other hand, it is perfectly legal to call either igt_skip() and
> > friends or __igt_abort() or its variant from there.
> > 
> > In the current implementation of igt_kunit(), there are several places
> > where igt_fail() is called despite being illegal.  Moreover, it is called
> > with IGT_EXIT_ABORT as an argument with no good reason for using such
> > aggressive method that forces CI to trigger system reboot (in most cases
> > igt_runner can decide if abort is required).
> > 
> > Follow igt_kselftests() pattern more closely, where similar setup and
> > cleanup operations are performed but their potential errors are processed
> > in a more friendly way.  Move common cleanup and their corresponding setup
> > steps out of the subtest body.  Place the latter as requirements in a
> > preceding igt_fixture section.  Replace remaining illegal igt_fail() calls
> > with more friendly skips.  Let igt_runner decide if abort is needed.
> > 
> > v2: Also call igt_skip() on igt_ktest_init() failure (Mauro), but then,
> > initialize local tst structure when declaring it to avoid freeing a
> > random pointer from igt_ktest_fini() when only listing subtests,
> >   - call igt_ktest_end() from igt_fixture so it is not unnecessarily
> > called when only listing subtests.
> > 
> > Signed-off-by: Janusz Krzysztofik 
> > Cc: Mauro Carvalho Chehab   
> 
> LGTM.
> 
> Acked-by: Mauro Carvalho Chehab 

Err... Please use another e-mail on my ack:

LGTM.

Acked-by: Mauro Carvalho Chehab 

> 
> > ---
> >  lib/igt_kmod.c | 76 +++---
> >  1 file changed, 23 insertions(+), 53 deletions(-)
> > 
> > diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> > index 1d1cd51170..063e4c12db 100644
> > --- a/lib/igt_kmod.c
> > +++ b/lib/igt_kmod.c
> > @@ -754,59 +754,27 @@ void igt_kselftest_get_tests(struct kmod_module *kmod,
> >   *
> >   * Returns: IGT default codes
> >   */
> > -static void __igt_kunit(const char *module_name, const char *opts)
> > +static void __igt_kunit(struct igt_ktest *tst, const char *opts)
> >  {
> > -   struct igt_ktest tst;
> > struct kmod_module *kunit_kmod;
> > bool is_builtin;
> > int ret;
> > struct ktap_test_results *results;
> > struct ktap_test_results_element *temp;
> > -   int skip = 0;
> > -   bool fail = false;
> > -
> > -   /* get normalized module name */
> > -   if (igt_ktest_init(&tst, module_name) != 0) {
> > -   igt_warn("Unable to initialize ktest for %s\n", module_name);
> > -   igt_fail(IGT_EXIT_ABORT);
> > -   }
> >  
> > -   if (igt_ktest_begin(&tst) != 0) {
> > -   igt_warn("Unable to begin ktest for %s\n", module_name);
> > -   igt_ktest_fini(&tst);
> > -   igt_fail(IGT_EXIT_ABORT);
> > -   }
> > +   igt_skip_on_f(tst->kmsg < 0, "Could not open /dev/kmsg\n");
> >  
> > -   if (tst.kmsg < 0) {
> > -   igt_warn("Could not open /dev/kmsg\n");
> > -   fail = true;
> > -   goto unload;
> > -   }
> > -
> > -   if (lseek(tst.kmsg, 0, SEEK_END)) {
> > -   igt_warn("Could not seek the end of /dev/kmsg\n");
> > -   fail = true;
> > -   goto unload;
> > -   }
> > -
> > -   ret = kmod_module_new_from_name(kmod_ctx(), "kunit", &kunit_kmod);
> > -   if (ret) {
> > -   igt_warn("Unable to load KUnit\n");
> > -   skip = ret;
> > -   goto unload;
> > -   }
> > +   igt_skip_on(lseek(tst->kmsg, 0, SEEK_END) < 0);
> >  
> > +   igt_skip_on(kmod_module_new_from_name(kmod_ctx(), "kunit", 
> > &kunit_kmod));
> > is_builtin = kmod_module_get_initstate(kunit_kmod) == 
> > KMOD_MODULE_BUILTIN;
> > kmod_module_unref(kunit_kmod);
> >  
> > -   results = ktap_parser_start(tst.kmsg, is_builtin);
> > +   results = ktap_parser_start(tst->kmsg, is_builtin);
> >  
> > -   ret = igt_kmod_load(module_name, opts);
> > -   if (ret) {
> > -   skip = ret;
> > -   igt_warn("Unable to load %s module\n", module_name);
> > -   ret = ktap_parser_stop();
> > -   goto unload;
> > +   if (igt_debug_on(igt_kmod_load(tst->module_name, opts) < 0)) {
> > +   igt_ignore_warn(ktap_parser_stop());
> > +   igt_skip("Unable to load %s module\n", tst->module_name);
> > }
> >  
> > while (READ_ONCE(results->still_running) || READ_ONCE(results->head) != 
> > NULL)
> > @@ -825,24 +793,21 @@ static void __igt_kunit(const char *module_name, 
> > const char *opts)
> > }
> > }
> >  
> > -unload:
> > -   igt_ktest_end(&tst);
> > -
> > -   igt_ktest_fini(&tst);
> > -
> > -   igt_skip_on_f(skip, "Skipping test, as probing KUnit module failed\n");
> > -
> > -   if (fail)
> > -   igt_fail(IGT_EXIT_ABORT);
>

Re: [Intel-gfx] [PATCH i-g-t v3 05/17] lib/kunit: Fix illegal igt_fail() calls inside subtest body

2023-09-18 Thread Mauro Carvalho Chehab
On Mon, 18 Sep 2023 15:42:55 +0200
Janusz Krzysztofik  wrote:

> In a body of a subtest with dynamic sub-subtests, it is illegal to call
> igt_fail() and its variants from outside of a dynamic sub-subtest body.
> On the other hand, it is perfectly legal to call either igt_skip() and
> friends or __igt_abort() or its variant from there.
> 
> In the current implementation of igt_kunit(), there are several places
> where igt_fail() is called despite being illegal.  Moreover, it is called
> with IGT_EXIT_ABORT as an argument with no good reason for using such
> aggressive method that forces CI to trigger system reboot (in most cases
> igt_runner can decide if abort is required).
> 
> Follow igt_kselftests() pattern more closely, where similar setup and
> cleanup operations are performed but their potential errors are processed
> in a more friendly way.  Move common cleanup and their corresponding setup
> steps out of the subtest body.  Place the latter as requirements in a
> preceding igt_fixture section.  Replace remaining illegal igt_fail() calls
> with more friendly skips.  Let igt_runner decide if abort is needed.
> 
> v2: Also call igt_skip() on igt_ktest_init() failure (Mauro), but then,
> initialize local tst structure when declaring it to avoid freeing a
> random pointer from igt_ktest_fini() when only listing subtests,
>   - call igt_ktest_end() from igt_fixture so it is not unnecessarily
> called when only listing subtests.
> 
> Signed-off-by: Janusz Krzysztofik 
> Cc: Mauro Carvalho Chehab 

LGTM.

Acked-by: Mauro Carvalho Chehab 

> ---
>  lib/igt_kmod.c | 76 +++---
>  1 file changed, 23 insertions(+), 53 deletions(-)
> 
> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> index 1d1cd51170..063e4c12db 100644
> --- a/lib/igt_kmod.c
> +++ b/lib/igt_kmod.c
> @@ -754,59 +754,27 @@ void igt_kselftest_get_tests(struct kmod_module *kmod,
>   *
>   * Returns: IGT default codes
>   */
> -static void __igt_kunit(const char *module_name, const char *opts)
> +static void __igt_kunit(struct igt_ktest *tst, const char *opts)
>  {
> - struct igt_ktest tst;
>   struct kmod_module *kunit_kmod;
>   bool is_builtin;
>   int ret;
>   struct ktap_test_results *results;
>   struct ktap_test_results_element *temp;
> - int skip = 0;
> - bool fail = false;
> -
> - /* get normalized module name */
> - if (igt_ktest_init(&tst, module_name) != 0) {
> - igt_warn("Unable to initialize ktest for %s\n", module_name);
> - igt_fail(IGT_EXIT_ABORT);
> - }
>  
> - if (igt_ktest_begin(&tst) != 0) {
> - igt_warn("Unable to begin ktest for %s\n", module_name);
> - igt_ktest_fini(&tst);
> - igt_fail(IGT_EXIT_ABORT);
> - }
> + igt_skip_on_f(tst->kmsg < 0, "Could not open /dev/kmsg\n");
>  
> - if (tst.kmsg < 0) {
> - igt_warn("Could not open /dev/kmsg\n");
> - fail = true;
> - goto unload;
> - }
> -
> - if (lseek(tst.kmsg, 0, SEEK_END)) {
> - igt_warn("Could not seek the end of /dev/kmsg\n");
> - fail = true;
> - goto unload;
> - }
> -
> - ret = kmod_module_new_from_name(kmod_ctx(), "kunit", &kunit_kmod);
> - if (ret) {
> - igt_warn("Unable to load KUnit\n");
> - skip = ret;
> - goto unload;
> - }
> + igt_skip_on(lseek(tst->kmsg, 0, SEEK_END) < 0);
>  
> + igt_skip_on(kmod_module_new_from_name(kmod_ctx(), "kunit", 
> &kunit_kmod));
>   is_builtin = kmod_module_get_initstate(kunit_kmod) == 
> KMOD_MODULE_BUILTIN;
>   kmod_module_unref(kunit_kmod);
>  
> - results = ktap_parser_start(tst.kmsg, is_builtin);
> + results = ktap_parser_start(tst->kmsg, is_builtin);
>  
> - ret = igt_kmod_load(module_name, opts);
> - if (ret) {
> - skip = ret;
> - igt_warn("Unable to load %s module\n", module_name);
> - ret = ktap_parser_stop();
> - goto unload;
> + if (igt_debug_on(igt_kmod_load(tst->module_name, opts) < 0)) {
> + igt_ignore_warn(ktap_parser_stop());
> + igt_skip("Unable to load %s module\n", tst->module_name);
>   }
>  
>   while (READ_ONCE(results->still_running) || READ_ONCE(results->head) != 
> NULL)
> @@ -825,24 +793,21 @@ static void __igt_kunit(const char *module_name, const 
> char *opts)
>   }
>   }
>  
> -unload:
> - igt_ktest_end(&tst);
> -
> - igt_ktest_fini(&tst);
> -
> - igt_skip_on_f(skip, "Skipping test, as probing KUnit module failed\n");
> -
> - if (fail)
> - igt_fail(IGT_EXIT_ABORT);
> -
>   ret = ktap_parser_stop();
>  
> - if (ret != 0)
> - igt_fail(IGT_EXIT_ABORT);
> + igt_skip_on_f(ret, "KTAP parser failed\n");
>  }
>  
>  void igt_kunit(const char *module_name, const char *name, const char *opts)
>  {
> + struct igt_ktest tst = { .kmsg = 

Re: [Intel-gfx] [PATCH i-g-t v3 10/17] lib/ktap: Read /dev/kmsg in blocking mode

2023-09-18 Thread Mauro Carvalho Chehab
On Mon, 18 Sep 2023 15:43:00 +0200
Janusz Krzysztofik  wrote:

> We obtain KTAP report from /dev/kmsg.  That file is now opened from
> igt_ktest_begin(), a function originally designed for i915 selftests and
> now reused with kunit tests.  The original intention of opening that file
> was to dump kernel messages to stderr on selftest error.  For that
> purpose, the file is now opened in non-blocking mode so we don't end up
> waiting for more kernel messages than already available.  Since our ktap
> parser code reuses the file descriptor, we now have to loop over
> EAGAIN responses, waiting for more KTAP data.  Since we have no sleeps
> inside those loops, extremely high CPU usage can be observed.
> 
> Simplify reading KTAP reports by first switching the file descriptor back
> to blocking mode.
> 
> While being at it, fix read errors other than EPIPE likely unintentionally
> ignored when reading first line of KTAP data.
> 
> v2: Drop EINTR handling as not applicable since SIGINT default signal
> handler kills the whole process anyway,
>   - update commit description to also mention read error handling fix.
> 
> Signed-off-by: Janusz Krzysztofik 
> Acked-by: Mauro Carvalho Chehab  # v1

LGTM.

Reviewed-by: Mauro Carvalho Chehab 


> ---
>  lib/igt_kmod.c |  7 +-
>  lib/igt_ktap.c | 66 +-
>  2 files changed, 28 insertions(+), 45 deletions(-)
> 
> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> index 7392276401..96240543a7 100644
> --- a/lib/igt_kmod.c
> +++ b/lib/igt_kmod.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include "igt_aux.h"
> @@ -758,12 +759,16 @@ static void __igt_kunit(struct igt_ktest *tst, const 
> char *opts)
>  {
>   struct kmod_module *kunit_kmod;
>   bool is_builtin;
> - int ret;
>   struct ktap_test_results *results;
>   struct ktap_test_results_element *temp;
> + int flags, ret;
>  
>   igt_skip_on_f(tst->kmsg < 0, "Could not open /dev/kmsg\n");
>  
> + flags = fcntl(tst->kmsg, F_GETFL, 0) & ~O_NONBLOCK;
> + igt_skip_on_f(fcntl(tst->kmsg, F_SETFL, flags) == -1,
> +   "Could not set /dev/kmsg to blocking mode\n");
> +
>   igt_skip_on(lseek(tst->kmsg, 0, SEEK_END) < 0);
>  
>   igt_skip_on(kmod_module_new_from_name(kmod_ctx(), "kunit", 
> &kunit_kmod));
> diff --git a/lib/igt_ktap.c b/lib/igt_ktap.c
> index 5bc5e003d7..282e44176e 100644
> --- a/lib/igt_ktap.c
> +++ b/lib/igt_ktap.c
> @@ -59,17 +59,12 @@ static int log_to_end(enum igt_log_level level, int fd,
>   while (*lend == '\0') {
>   igt_log(IGT_LOG_DOMAIN, level, "%s", record);
>  
> - while (read(fd, record, BUF_LEN) < 0) {
> - if (errno == EPIPE) {
> + if (read(fd, record, BUF_LEN) < 0) {
> + if (errno == EPIPE)
>   igt_warn("kmsg truncated: too many messages. 
> You may want to increase log_buf_len in kmcdline\n");
> - return -2;
> - }
> -
> - if (errno == EAGAIN)
> - /* No records available */
> - continue;
> + else
> + igt_warn("an error occurred while reading kmsg: 
> %m\n");
>  
> - igt_warn("kmsg truncated: unknown error (%m)\n");
>   return -2;
>   }
>  
> @@ -173,17 +168,12 @@ static int find_next_tap_subtest(int fd, char *record, 
> char *test_name, bool is_
>   return -1;
>  
>   if (is_builtin) {
> - while (read(fd, record, BUF_LEN) < 0) {
> - if (errno == EPIPE) {
> + if (read(fd, record, BUF_LEN) < 0) {
> + if (errno == EPIPE)
>   igt_warn("kmsg truncated: too many messages. 
> You may want to increase log_buf_len in kmcdline\n");
> - return -2;
> - }
> + else
> + igt_warn("an error occurred while reading kmsg: 
> %m\n");
>  
> - if (errno == EAGAIN)
> - /* No records available */
> - continue;
> -
> - igt_warn("kmsg truncated: unknown error (%m)\n");
>   return -2;
>   }
>   }
> @@ -209,17 +199,12 @@ static int find_next_tap_subtest(int fd, char *record, 
> char *test_name, bool is_
>   if (cutoff)
>   cutoff[0] = '\0';
>  
> - while (read(fd, record, BUF_LEN) < 0) {
> - if (errno == EPIPE) {
> + if (read(fd, record, BUF_LEN) < 0) {
> + if (errno == EPIPE)
>   igt_warn("kmsg truncated: too many messages. 
> You may want to increase log_buf_len in kmcdline\n");
> -   

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: refactor deprecated strncpy (rev2)

2023-09-18 Thread Patchwork
== Series Details ==

Series: drm/i915: refactor deprecated strncpy (rev2)
URL   : https://patchwork.freedesktop.org/series/123858/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13651 -> Patchwork_123858v2


Summary
---

  **SUCCESS**

  No regressions found.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123858v2/index.html

Participating hosts (38 -> 37)
--

  Missing(1): fi-snb-2520m 

Known issues


  Here are the changes found in Patchwork_123858v2 that come from known issues:

### IGT changes ###

 Issues hit 

  * igt@gem_exec_parallel@engines@fds:
- bat-mtlp-6: [PASS][1] -> [ABORT][2] ([i915#9262])
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13651/bat-mtlp-6/igt@gem_exec_parallel@engi...@fds.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123858v2/bat-mtlp-6/igt@gem_exec_parallel@engi...@fds.html

  * igt@i915_selftest@live@gt_heartbeat:
- fi-apl-guc: [PASS][3] -> [DMESG-FAIL][4] ([i915#5334])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13651/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123858v2/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence:
- bat-adlp-9: NOTRUN -> [SKIP][5] ([i915#3546]) +2 other tests skip
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123858v2/bat-adlp-9/igt@kms_pipe_crc_ba...@nonblocking-crc-frame-sequence.html
- bat-dg2-11: NOTRUN -> [SKIP][6] ([i915#1845]) +3 other tests skip
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123858v2/bat-dg2-11/igt@kms_pipe_crc_ba...@nonblocking-crc-frame-sequence.html

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1:
- bat-rplp-1: [PASS][7] -> [ABORT][8] ([i915#8668])
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13651/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-seque...@pipe-d-edp-1.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123858v2/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-seque...@pipe-d-edp-1.html

  
 Possible fixes 

  * igt@i915_selftest@live@execlists:
- fi-bsw-n3050:   [ABORT][9] ([i915#7913]) -> [PASS][10]
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13651/fi-bsw-n3050/igt@i915_selftest@l...@execlists.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123858v2/fi-bsw-n3050/igt@i915_selftest@l...@execlists.html

  * igt@kms_hdmi_inject@inject-audio:
- fi-kbl-guc: [FAIL][11] ([IGT#3] / [i915#6121]) -> [PASS][12]
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13651/fi-kbl-guc/igt@kms_hdmi_inj...@inject-audio.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123858v2/fi-kbl-guc/igt@kms_hdmi_inj...@inject-audio.html

  
  [IGT#3]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/3
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#6121]: https://gitlab.freedesktop.org/drm/intel/issues/6121
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#8668]: https://gitlab.freedesktop.org/drm/intel/issues/8668
  [i915#9262]: https://gitlab.freedesktop.org/drm/intel/issues/9262


Build changes
-

  * Linux: CI_DRM_13651 -> Patchwork_123858v2

  CI-20190529: 20190529
  CI_DRM_13651: 61b71c3f061a44a6ab1dcf756918886aa03a5480 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7493: 2517e42d612e0c1ca096acf8b5f6177f7ef4bce7 @ 
https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_123858v2: 61b71c3f061a44a6ab1dcf756918886aa03a5480 @ 
git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

42a2d801357d drm/i915: refactor deprecated strncpy

== Logs ==

For more details see: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123858v2/index.html


[Intel-gfx] [PATCH v2] drm/i915: refactor deprecated strncpy

2023-09-18 Thread Justin Stitt
`strncpy` is deprecated for use on NUL-terminated destination strings [1].

We should prefer more robust and less ambiguous string interfaces.

A suitable replacement is `strscpy` [2] due to the fact that it
guarantees NUL-termination on the destination buffer without
unnecessarily NUL-padding. `ctx` is zero allocated and as such strncpy's
NUL-padding behavior was strictly a performance hit which is now
resolved.

Link: 
https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
 [1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-harden...@vger.kernel.org
Signed-off-by: Justin Stitt 
---
Changes in v2:
- drop the `... - 1` (thanks Kees)
- Link to v1: 
https://lore.kernel.org/r/20230914-strncpy-drivers-gpu-drm-i915-gem-selftests-mock_context-c-v1-1-c3f92df94...@google.com
---
 drivers/gpu/drm/i915/gem/selftests/mock_context.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_context.c 
b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
index 8ac6726ec16b..e199d7dbb876 100644
--- a/drivers/gpu/drm/i915/gem/selftests/mock_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
@@ -36,7 +36,7 @@ mock_context(struct drm_i915_private *i915,
if (name) {
struct i915_ppgtt *ppgtt;
 
-   strncpy(ctx->name, name, sizeof(ctx->name) - 1);
+   strscpy(ctx->name, name, sizeof(ctx->name));
 
ppgtt = mock_ppgtt(i915, name);
if (!ppgtt)

---
base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec
change-id: 
20230914-strncpy-drivers-gpu-drm-i915-gem-selftests-mock_context-c-980c8ecc9142

Best regards,
--
Justin Stitt 



[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/pxp: Add drm_dbgs for critical PXP events. (rev3)

2023-09-18 Thread Patchwork
== Series Details ==

Series: drm/i915/pxp: Add drm_dbgs for critical PXP events. (rev3)
URL   : https://patchwork.freedesktop.org/series/123803/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13651 -> Patchwork_123803v3


Summary
---

  **SUCCESS**

  No regressions found.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123803v3/index.html

Participating hosts (38 -> 37)
--

  Missing(1): fi-snb-2520m 

Known issues


  Here are the changes found in Patchwork_123803v3 that come from known issues:

### IGT changes ###

 Issues hit 

  * igt@gem_exec_suspend@basic-s0@lmem0:
- bat-dg2-9:  [PASS][1] -> [INCOMPLETE][2] ([i915#9275])
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13651/bat-dg2-9/igt@gem_exec_suspend@basic...@lmem0.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123803v3/bat-dg2-9/igt@gem_exec_suspend@basic...@lmem0.html

  
 Possible fixes 

  * igt@i915_selftest@live@execlists:
- fi-bsw-n3050:   [ABORT][3] ([i915#7913]) -> [PASS][4]
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13651/fi-bsw-n3050/igt@i915_selftest@l...@execlists.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123803v3/fi-bsw-n3050/igt@i915_selftest@l...@execlists.html

  * igt@kms_hdmi_inject@inject-audio:
- fi-kbl-guc: [FAIL][5] ([IGT#3] / [i915#6121]) -> [PASS][6]
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13651/fi-kbl-guc/igt@kms_hdmi_inj...@inject-audio.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123803v3/fi-kbl-guc/igt@kms_hdmi_inj...@inject-audio.html

  
  [IGT#3]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/3
  [i915#6121]: https://gitlab.freedesktop.org/drm/intel/issues/6121
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#9275]: https://gitlab.freedesktop.org/drm/intel/issues/9275


Build changes
-

  * Linux: CI_DRM_13651 -> Patchwork_123803v3

  CI-20190529: 20190529
  CI_DRM_13651: 61b71c3f061a44a6ab1dcf756918886aa03a5480 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7493: 2517e42d612e0c1ca096acf8b5f6177f7ef4bce7 @ 
https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_123803v3: 61b71c3f061a44a6ab1dcf756918886aa03a5480 @ 
git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

6a9f9b406f68 drm/i915/pxp: Add drm_dbgs for critical PXP events.

== Logs ==

For more details see: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123803v3/index.html


[Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: Improve BW management on shared display links (rev9)

2023-09-18 Thread Patchwork
== Series Details ==

Series: drm/i915: Improve BW management on shared display links (rev9)
URL   : https://patchwork.freedesktop.org/series/122589/
State : failure

== Summary ==

Error: patch 
https://patchwork.freedesktop.org/api/1.0/series/122589/revisions/9/mbox/ not 
applied
Applying: drm/i915/dp: Factor out helpers to compute the link limits
Applying: drm/i915/dp: Track the pipe and link bpp limits separately
Applying: drm/i915/dp: Skip computing a non-DSC link config if DSC is needed
Applying: drm/i915/dp: Update the link bpp limits for DSC mode
Applying: drm/i915/dp: Limit the output link bpp in DSC mode
Applying: drm/i915: Add helper to modeset a set of pipes
Applying: drm/i915: Rename intel_modeset_all_pipes() to 
intel_modeset_all_pipes_late()
Applying: drm/i915: Factor out a helper to check/compute all the CRTC states
Applying: drm/i915: Add helpers for BW management on shared display links
Applying: drm/i915/fdi: Improve FDI BW sharing between pipe B and C
Applying: drm/i915/fdi: Recompute state for affected CRTCs on FDI links
Applying: drm/dp_mst: Fix fractional DSC bpp handling
Applying: drm/dp_mst: Add a way to calculate PBN values with FEC overhead
Applying: drm/dp_mst: Add helper to determine if an MST port is downstream of 
another port
Applying: drm/dp_mst: Factor out a helper to check the atomic state of a 
topology manager
Applying: drm/dp_mst: Swap the order of checking root vs. non-root port BW 
limitations
Applying: drm/i915/dp_mst: Fix PBN calculation with FEC overhead
Applying: drm/i915/dp_mst: Add atomic state for all streams on pre-tgl platforms
Applying: drm/i915/dp_mst: Program the DSC PPS SDP for each stream
Applying: drm/i915/dp: Make sure the DSC PPS SDP is disabled whenever DSC is 
disabled
Applying: drm/i915/dp_mst: Enable DSC decompression if any stream needs this
Applying: drm/i915/dp_mst: Add missing DSC compression disabling
Applying: drm/i915/dp_mst: Allow DSC only for sink ports of the first branch 
device
Applying: drm/i915/dp_mst: Improve BW sharing between MST streams
error: sha1 information is lacking or useless 
(drivers/gpu/drm/i915/display/intel_display.c).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0024 drm/i915/dp_mst: Improve BW sharing between MST streams
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Build failed, no error log produced




[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/pxp: Add drm_dbgs for critical PXP events. (rev3)

2023-09-18 Thread Patchwork
== Series Details ==

Series: drm/i915/pxp: Add drm_dbgs for critical PXP events. (rev3)
URL   : https://patchwork.freedesktop.org/series/123803/
State : warning

== Summary ==

Error: dim checkpatch failed
d3dc616d2344 drm/i915/pxp: Add drm_dbgs for critical PXP events.
-:7: WARNING:COMMIT_LOG_LONG_LINE: Prefer a maximum 75 chars per line (possible 
unwrapped commit description?)
#7: 
sequence of important events. Add drm_dbg into the most important PXP events.

-:15: ERROR:BAD_SIGN_OFF: Unrecognized email address: 'Balasubrawmanian, Vivaik 
'
#15: 
Reviewed-by: Balasubrawmanian, Vivaik 

-:96: WARNING:LONG_LINE: line length of 105 exceeds 100 columns
#96: FILE: drivers/gpu/drm/i915/pxp/intel_pxp_irq.c:43:
+   pxp->session_events |= PXP_TERMINATION_REQUEST | 
PXP_INVAL_REQUIRED | PXP_EVENT_TYPE_IRQ;

total: 1 errors, 2 warnings, 0 checks, 95 lines checked




[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/pxp/mtl: Update gsc-heci cmd submission to align with fw/hw spec (rev2)

2023-09-18 Thread Patchwork
== Series Details ==

Series: drm/i915/pxp/mtl: Update gsc-heci cmd submission to align with fw/hw 
spec (rev2)
URL   : https://patchwork.freedesktop.org/series/123830/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13651 -> Patchwork_123830v2


Summary
---

  **SUCCESS**

  No regressions found.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123830v2/index.html

Participating hosts (38 -> 39)
--

  Additional (2): fi-kbl-soraka bat-dg2-8 
  Missing(1): fi-snb-2520m 

Known issues


  Here are the changes found in Patchwork_123830v2 that come from known issues:

### CI changes ###

 Issues hit 

  * boot:
- fi-hsw-4770:[PASS][1] -> [FAIL][2] ([i915#8293])
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13651/fi-hsw-4770/boot.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123830v2/fi-hsw-4770/boot.html

  

### IGT changes ###

 Issues hit 

  * igt@gem_exec_suspend@basic-s3@smem:
- bat-dg2-8:  NOTRUN -> [INCOMPLETE][3] ([i915#8797] / [i915#9275])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123830v2/bat-dg2-8/igt@gem_exec_suspend@basic...@smem.html

  * igt@gem_huc_copy@huc-copy:
- fi-kbl-soraka:  NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#2190])
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123830v2/fi-kbl-soraka/igt@gem_huc_c...@huc-copy.html

  * igt@gem_lmem_swapping@basic:
- fi-kbl-soraka:  NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#4613]) +3 
other tests skip
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123830v2/fi-kbl-soraka/igt@gem_lmem_swapp...@basic.html

  * igt@gem_mmap@basic:
- bat-dg2-8:  NOTRUN -> [SKIP][6] ([i915#4083])
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123830v2/bat-dg2-8/igt@gem_m...@basic.html

  * igt@gem_mmap_gtt@basic:
- bat-dg2-8:  NOTRUN -> [SKIP][7] ([i915#4077]) +2 other tests skip
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123830v2/bat-dg2-8/igt@gem_mmap_...@basic.html

  * igt@gem_tiled_pread_basic:
- bat-dg2-8:  NOTRUN -> [SKIP][8] ([i915#4079]) +1 other test skip
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123830v2/bat-dg2-8/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_rps@basic-api:
- bat-dg2-8:  NOTRUN -> [SKIP][9] ([i915#6621])
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123830v2/bat-dg2-8/igt@i915_pm_...@basic-api.html

  * igt@i915_selftest@live@gt_heartbeat:
- fi-kbl-soraka:  NOTRUN -> [DMESG-FAIL][10] ([i915#5334] / [i915#7872])
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123830v2/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@gt_pm:
- fi-kbl-soraka:  NOTRUN -> [DMESG-FAIL][11] ([i915#1886] / [i915#7913])
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123830v2/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html

  * igt@i915_suspend@basic-s3-without-i915:
- bat-dg2-8:  NOTRUN -> [SKIP][12] ([i915#6645])
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123830v2/bat-dg2-8/igt@i915_susp...@basic-s3-without-i915.html

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
- bat-dg2-8:  NOTRUN -> [SKIP][13] ([i915#5190])
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123830v2/bat-dg2-8/igt@kms_addfb_ba...@addfb25-y-tiled-small-legacy.html

  * igt@kms_addfb_basic@basic-y-tiled-legacy:
- bat-dg2-8:  NOTRUN -> [SKIP][14] ([i915#4215] / [i915#5190])
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123830v2/bat-dg2-8/igt@kms_addfb_ba...@basic-y-tiled-legacy.html

  * igt@kms_addfb_basic@framebuffer-vs-set-tiling:
- bat-dg2-8:  NOTRUN -> [SKIP][15] ([i915#4212]) +6 other tests skip
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123830v2/bat-dg2-8/igt@kms_addfb_ba...@framebuffer-vs-set-tiling.html

  * igt@kms_addfb_basic@tile-pitch-mismatch:
- bat-dg2-8:  NOTRUN -> [SKIP][16] ([i915#4212] / [i915#5608])
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123830v2/bat-dg2-8/igt@kms_addfb_ba...@tile-pitch-mismatch.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
- bat-dg2-8:  NOTRUN -> [SKIP][17] ([i915#4103] / [i915#4213] / 
[i915#5608]) +1 other test skip
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123830v2/bat-dg2-8/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_dsc@dsc-basic:
- fi-kbl-soraka:  NOTRUN -> [SKIP][18] ([fdo#109271]) +9 other tests 
skip
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123830v2/fi-kbl-soraka/igt@kms_...@dsc-basic.html

  * igt@kms_force_connector_basic@force-load-detect:
- bat-dg2-8:  NOTRUN -> [SKIP][19] ([fdo#109285])
   [19]: 
https://intel-gfx-ci.01.org/tree

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915/pxp/mtl: Update gsc-heci cmd submission to align with fw/hw spec (rev2)

2023-09-18 Thread Patchwork
== Series Details ==

Series: drm/i915/pxp/mtl: Update gsc-heci cmd submission to align with fw/hw 
spec (rev2)
URL   : https://patchwork.freedesktop.org/series/123830/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
+./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:154:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:154:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:154:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:154:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:154:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:154:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:154:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:154:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:156:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:156:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:156:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:156:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:156:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:156:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:156:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:156:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:156:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:156:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:156:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:156:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:156:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:156:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:156:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:156:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:174:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:174:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:174:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:174:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:174:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:174:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:174:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:174:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:176:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:176:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:176:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:176:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:176:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:176:9: w

[Intel-gfx] ✓ Fi.CI.BAT: success for Update GGTT with MI_UPDATE_GTT on MTL (rev6)

2023-09-18 Thread Patchwork
== Series Details ==

Series: Update GGTT with MI_UPDATE_GTT on MTL (rev6)
URL   : https://patchwork.freedesktop.org/series/123329/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13651 -> Patchwork_123329v6


Summary
---

  **WARNING**

  Minor unknown changes coming with Patchwork_123329v6 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_123329v6, please notify your bug team 
(lgci.bug.fil...@intel.com) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123329v6/index.html

Participating hosts (38 -> 37)
--

  Additional (1): fi-kbl-soraka 
  Missing(2): bat-dg2-9 fi-snb-2520m 

Possible new issues
---

  Here are the unknown changes that may have been introduced in 
Patchwork_123329v6:

### IGT changes ###

 Warnings 

  * igt@gem_exec_suspend@basic-s3@lmem0:
- bat-atsm-1: [DMESG-WARN][1] ([i915#8504] / [i915#8841]) -> 
[ABORT][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13651/bat-atsm-1/igt@gem_exec_suspend@basic...@lmem0.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123329v6/bat-atsm-1/igt@gem_exec_suspend@basic...@lmem0.html

  
Known issues


  Here are the changes found in Patchwork_123329v6 that come from known issues:

### CI changes ###

 Issues hit 

  * boot:
- fi-hsw-4770:[PASS][3] -> [FAIL][4] ([i915#8293])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13651/fi-hsw-4770/boot.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123329v6/fi-hsw-4770/boot.html

  

### IGT changes ###

 Issues hit 

  * igt@gem_exec_suspend@basic-s0@smem:
- bat-jsl-1:  [PASS][5] -> [INCOMPLETE][6] ([i915#9275])
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13651/bat-jsl-1/igt@gem_exec_suspend@basic...@smem.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123329v6/bat-jsl-1/igt@gem_exec_suspend@basic...@smem.html

  * igt@gem_huc_copy@huc-copy:
- fi-kbl-soraka:  NOTRUN -> [SKIP][7] ([fdo#109271] / [i915#2190])
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123329v6/fi-kbl-soraka/igt@gem_huc_c...@huc-copy.html

  * igt@gem_lmem_swapping@basic:
- fi-kbl-soraka:  NOTRUN -> [SKIP][8] ([fdo#109271] / [i915#4613]) +3 
other tests skip
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123329v6/fi-kbl-soraka/igt@gem_lmem_swapp...@basic.html

  * igt@i915_selftest@live@gt_pm:
- fi-kbl-soraka:  NOTRUN -> [DMESG-FAIL][9] ([i915#1886] / [i915#7913])
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123329v6/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html

  * igt@i915_suspend@basic-s3-without-i915:
- bat-jsl-1:  [PASS][10] -> [FAIL][11] ([fdo#103375])
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13651/bat-jsl-1/igt@i915_susp...@basic-s3-without-i915.html
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123329v6/bat-jsl-1/igt@i915_susp...@basic-s3-without-i915.html

  * igt@kms_dsc@dsc-basic:
- fi-kbl-soraka:  NOTRUN -> [SKIP][12] ([fdo#109271]) +9 other tests 
skip
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123329v6/fi-kbl-soraka/igt@kms_...@dsc-basic.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence:
- bat-dg2-11: NOTRUN -> [SKIP][13] ([i915#1845]) +3 other tests skip
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123329v6/bat-dg2-11/igt@kms_pipe_crc_ba...@nonblocking-crc-frame-sequence.html

  
 Possible fixes 

  * igt@i915_selftest@live@execlists:
- fi-bsw-n3050:   [ABORT][14] ([i915#7913]) -> [PASS][15]
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13651/fi-bsw-n3050/igt@i915_selftest@l...@execlists.html
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123329v6/fi-bsw-n3050/igt@i915_selftest@l...@execlists.html

  
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#8293]: https://gitlab.freedesktop.org/drm/intel/issues/8293
  [i915#8504]: https://gitlab.freedesktop.org/drm/intel/issues/8504
  [i915#8841]: https://gitlab.freedesktop.org/drm/intel/issues/8841
  [i915#9275]: https://gitlab.freedesktop.org/drm/intel/issues/9275


Build changes
-

  * IGT: IGT_7493 -> IGTPW_9823
  * Linux: CI_DRM_13651 -> Patchwork_123329v6

  C

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for Update GGTT with MI_UPDATE_GTT on MTL (rev6)

2023-09-18 Thread Patchwork
== Series Details ==

Series: Update GGTT with MI_UPDATE_GTT on MTL (rev6)
URL   : https://patchwork.freedesktop.org/series/123329/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.




[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Update GGTT with MI_UPDATE_GTT on MTL (rev6)

2023-09-18 Thread Patchwork
== Series Details ==

Series: Update GGTT with MI_UPDATE_GTT on MTL (rev6)
URL   : https://patchwork.freedesktop.org/series/123329/
State : warning

== Summary ==

Error: dim checkpatch failed
00f4272cdb82 drm/i915: Lift runtime-pm acquire callbacks out of 
intel_wakeref.mutex
-:57: WARNING:AVOID_BUG: Do not crash the kernel unless it is absolutely 
unavoidable--use WARN_ON_ONCE() plus recovery code (if feasible) instead of 
BUG() or variants
#57: FILE: drivers/gpu/drm/i915/intel_wakeref.c:27:
+   INTEL_WAKEREF_BUG_ON(wf->wakeref);

-:73: WARNING:AVOID_BUG: Do not crash the kernel unless it is absolutely 
unavoidable--use WARN_ON_ONCE() plus recovery code (if feasible) instead of 
BUG() or variants
#73: FILE: drivers/gpu/drm/i915/intel_wakeref.c:40:
+   INTEL_WAKEREF_BUG_ON(atomic_read(&wf->count) <= 0);

-:96: WARNING:AVOID_BUG: Do not crash the kernel unless it is absolutely 
unavoidable--use WARN_ON_ONCE() plus recovery code (if feasible) instead of 
BUG() or variants
#96: FILE: drivers/gpu/drm/i915/intel_wakeref.c:60:
+   INTEL_WAKEREF_BUG_ON(!wf->wakeref);

total: 0 errors, 3 warnings, 0 checks, 83 lines checked
79086c93cfc2 drm/i915: Create a kernel context for GGTT updates
e6144a8b992e drm/i915: Implement for_each_sgt_daddr_next
-:40: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__iter' - possible 
side-effects?
#40: FILE: drivers/gpu/drm/i915/i915_scatterlist.h:100:
+#define __for_each_daddr_next(__dp, __iter, __step)  \
+   for (; ((__dp) = (__iter).dma + (__iter).curr), (__iter).sgp;   \
+(((__iter).curr += (__step)) >= (__iter).max) ?\
+(__iter) = __sgt_iter(__sg_next((__iter).sgp), true), 0 : 0)

total: 0 errors, 0 warnings, 1 checks, 25 lines checked
f2b35b1116f5 drm/i915: Parameterize binder context creation
523adfc41fed drm/i915: Implement GGTT update method with MI_UPDATE_GTT
-:69: WARNING:AVOID_BUG: Do not crash the kernel unless it is absolutely 
unavoidable--use WARN_ON_ONCE() plus recovery code (if feasible) instead of 
BUG() or variants
#69: FILE: drivers/gpu/drm/i915/gt/intel_ggtt.c:276:
+   GEM_BUG_ON(!ce);

-:239: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#239: FILE: drivers/gpu/drm/i915/gt/intel_ggtt.c:491:
+   if (!gen8_ggtt_bind_ptes(ggtt, start, vma_res->bi.pages,
+ vma_res->node_size / I915_GTT_PAGE_SIZE, pte_encode))

-:287: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#287: FILE: drivers/gpu/drm/i915/gt/intel_ggtt.c:553:
+   if (should_update_ggtt_with_bind(ggtt) && gen8_ggtt_bind_ptes(ggtt, 
first_entry,
+NULL, num_entries, scratch_pte))

total: 0 errors, 1 warnings, 2 checks, 283 lines checked
747df75fcbc2 drm/i915: Toggle binder context ready status
3f5824ab2d71 drm/i915: Enable GGTT updates with binder in MTL




[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: refactor deprecated strncpy

2023-09-18 Thread Patchwork
== Series Details ==

Series: drm/i915: refactor deprecated strncpy
URL   : https://patchwork.freedesktop.org/series/123858/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13651 -> Patchwork_123858v1


Summary
---

  **SUCCESS**

  No regressions found.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123858v1/index.html

Participating hosts (38 -> 38)
--

  Additional (1): fi-kbl-soraka 
  Missing(1): fi-snb-2520m 

Known issues


  Here are the changes found in Patchwork_123858v1 that come from known issues:

### IGT changes ###

 Issues hit 

  * igt@gem_huc_copy@huc-copy:
- fi-kbl-soraka:  NOTRUN -> [SKIP][1] ([fdo#109271] / [i915#2190])
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123858v1/fi-kbl-soraka/igt@gem_huc_c...@huc-copy.html

  * igt@gem_lmem_swapping@basic:
- fi-kbl-soraka:  NOTRUN -> [SKIP][2] ([fdo#109271] / [i915#4613]) +3 
other tests skip
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123858v1/fi-kbl-soraka/igt@gem_lmem_swapp...@basic.html

  * igt@i915_selftest@live@gt_pm:
- fi-kbl-soraka:  NOTRUN -> [DMESG-FAIL][3] ([i915#1886] / [i915#7913])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123858v1/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html

  * igt@kms_dsc@dsc-basic:
- fi-kbl-soraka:  NOTRUN -> [SKIP][4] ([fdo#109271]) +9 other tests skip
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123858v1/fi-kbl-soraka/igt@kms_...@dsc-basic.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence:
- bat-dg2-11: NOTRUN -> [SKIP][5] ([i915#1845]) +3 other tests skip
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123858v1/bat-dg2-11/igt@kms_pipe_crc_ba...@nonblocking-crc-frame-sequence.html

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1:
- bat-rplp-1: [PASS][6] -> [ABORT][7] ([i915#8668])
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13651/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-seque...@pipe-d-edp-1.html
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123858v1/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-seque...@pipe-d-edp-1.html

  
 Possible fixes 

  * igt@i915_selftest@live@execlists:
- fi-bsw-n3050:   [ABORT][8] ([i915#7913]) -> [PASS][9]
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13651/fi-bsw-n3050/igt@i915_selftest@l...@execlists.html
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123858v1/fi-bsw-n3050/igt@i915_selftest@l...@execlists.html

  * igt@kms_chamelium_edid@hdmi-edid-read:
- {bat-dg2-13}:   [DMESG-WARN][10] ([i915#7952]) -> [PASS][11]
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13651/bat-dg2-13/igt@kms_chamelium_e...@hdmi-edid-read.html
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123858v1/bat-dg2-13/igt@kms_chamelium_e...@hdmi-edid-read.html

  
  {name}: This element is suppressed. This means it is ignored when computing
  the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7952]: https://gitlab.freedesktop.org/drm/intel/issues/7952
  [i915#8668]: https://gitlab.freedesktop.org/drm/intel/issues/8668


Build changes
-

  * Linux: CI_DRM_13651 -> Patchwork_123858v1

  CI-20190529: 20190529
  CI_DRM_13651: 61b71c3f061a44a6ab1dcf756918886aa03a5480 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7493: 2517e42d612e0c1ca096acf8b5f6177f7ef4bce7 @ 
https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_123858v1: 61b71c3f061a44a6ab1dcf756918886aa03a5480 @ 
git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

fcf0c1103974 drm/i915: refactor deprecated strncpy

== Logs ==

For more details see: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123858v1/index.html


[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: refactor deprecated strncpy

2023-09-18 Thread Patchwork
== Series Details ==

Series: drm/i915: refactor deprecated strncpy
URL   : https://patchwork.freedesktop.org/series/123858/
State : warning

== Summary ==

Error: dim checkpatch failed
a3ff6333c019 drm/i915: refactor deprecated strncpy
-:11: WARNING:COMMIT_LOG_LONG_LINE: Prefer a maximum 75 chars per line 
(possible unwrapped commit description?)
#11: 
NUL-termination on the destination buffer without unnecessarily NUL-padding.

total: 0 errors, 1 warnings, 0 checks, 8 lines checked




Re: [Intel-gfx] [PATCH v2 12/12] drm/i915: Implement transcoder LRR for TGL+

2023-09-18 Thread Manasi Navare
Thanks Ville for the respin, the changes look good now.

Reviewed-by: Manasi Navare 

Manasi


On Fri, Sep 15, 2023 at 3:38 AM Ville Syrjala
 wrote:
>
> From: Ville Syrjälä 
>
> Implement low refresh rate (LRR) where we change the vblank
> length by hand as requested, but otherwise keep the timing
> generator running in non-VRR mode (ie. fixed refresh rate).
>
> The panel itself must support VRR for this to work, and
> only TGL+ has the double buffred TRANS_VTOTAL.VTOTAL that
> we need to make the switch properly. The double buffer
> latching happens at the start of transcoders undelayed
> vblank. The other thing that we change is
> TRANS_VBLANK.VBLANK_END but the hardware entirely ignores
> that in DP mode. But I decided to keep writing it anyway
> just to avoid more special cases in readout/state check.
>
> v2: Document that TRANS_VBLANK.VBLANK_END is ignored by
> the hardware
> v3: Reconcile with VRR fastset
> Adjust update_lrr flag behaviour
> Make sure timings stay within VRR range
> v4: Fix up update_m_n vs. update_lrr rebase fail (Manasi)
> Drop DOUBLE_BUFFER_VACTIVE define as it's not needed (Manasi)
>
> TODO: Hook LRR into the automatic DRRS downclocking stuff?
>
> Cc: Manasi Navare 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/display/intel_atomic.c   |  1 +
>  drivers/gpu/drm/i915/display/intel_crtc.c |  9 +--
>  drivers/gpu/drm/i915/display/intel_display.c  | 60 +--
>  .../drm/i915/display/intel_display_device.h   |  1 +
>  .../drm/i915/display/intel_display_types.h|  3 +-
>  drivers/gpu/drm/i915/display/intel_vrr.c  |  7 ++-
>  6 files changed, 70 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
> b/drivers/gpu/drm/i915/display/intel_atomic.c
> index aaddd8c0cfa0..5d18145da279 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -260,6 +260,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
>
> crtc_state->update_pipe = false;
> crtc_state->update_m_n = false;
> +   crtc_state->update_lrr = false;
> crtc_state->disable_lp_wm = false;
> crtc_state->disable_cxsr = false;
> crtc_state->update_wm_pre = false;
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c 
> b/drivers/gpu/drm/i915/display/intel_crtc.c
> index a39e31c1ca85..22e85fe7e8aa 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
> @@ -495,7 +495,7 @@ static void intel_crtc_vblank_evade_scanlines(struct 
> intel_atomic_state *state,
> if (crtc->mode_flags & I915_MODE_FLAG_VRR) {
> /* timing changes should happen with VRR disabled */
> drm_WARN_ON(state->base.dev, 
> intel_crtc_needs_modeset(new_crtc_state) ||
> -   new_crtc_state->update_m_n);
> +   new_crtc_state->update_m_n || 
> new_crtc_state->update_lrr);
>
> if (intel_vrr_is_push_sent(crtc_state))
> *vblank_start = 
> intel_vrr_vmin_vblank_start(crtc_state);
> @@ -511,10 +511,11 @@ static void intel_crtc_vblank_evade_scanlines(struct 
> intel_atomic_state *state,
> *max = *vblank_start - 1;
>
> /*
> -* M/N is double buffered on the transcoder's undelayed vblank,
> -* so with seamless M/N we must evade both vblanks.
> +* M/N and TRANS_VTOTAL are double buffered on the transcoder's
> +* undelayed vblank, so with seamless M/N and LRR we must evade
> +* both vblanks.
>  */
> -   if (new_crtc_state->update_m_n)
> +   if (new_crtc_state->update_m_n || new_crtc_state->update_lrr)
> *min -= adjusted_mode->crtc_vblank_start - 
> adjusted_mode->crtc_vdisplay;
>  }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
> b/drivers/gpu/drm/i915/display/intel_display.c
> index f0bb5c70ebfc..988558ccf794 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -928,7 +928,7 @@ static bool vrr_enabling(const struct intel_crtc_state 
> *old_crtc_state,
>  {
> return is_enabling(vrr.enable, old_crtc_state, new_crtc_state) ||
> (new_crtc_state->vrr.enable &&
> -(new_crtc_state->update_m_n ||
> +(new_crtc_state->update_m_n || new_crtc_state->update_lrr ||
>   vrr_params_changed(old_crtc_state, new_crtc_state)));
>  }
>
> @@ -937,7 +937,7 @@ static bool vrr_disabling(const struct intel_crtc_state 
> *old_crtc_state,
>  {
> return is_disabling(vrr.enable, old_crtc_state, new_crtc_state) ||
> (old_crtc_state->vrr.enable &&
> -(new_crtc_state->update_m_n ||
> +(new_crtc_state->update_m_n || new_crtc_state->update_lrr ||
>   vrr_params_changed(old_crtc_state, new_crtc_state)));
>  }
>
> @@ -2586,6 +2586,37 @@ st

Re: [Intel-gfx] [Intel-xe] [PATCH v4 10/30] drm/i915/display: Consolidate saved port bits in intel_digital_port

2023-09-18 Thread Matt Roper
On Mon, Sep 18, 2023 at 04:06:58PM -0500, Lucas De Marchi wrote:
> On Fri, Sep 15, 2023 at 12:50:41PM -0700, Matt Roper wrote:
> > On Fri, Sep 15, 2023 at 10:46:31AM -0700, Lucas De Marchi wrote:
> > > From: Clint Taylor 
> > > 
> > > We use multiple variables for HDMI and DisplayPort to store the value of
> > > DDI_BUF_CTL register (now called DDI_CTL_DE in the spec). Consolidate it
> > > to just one in struct intel_digital_port. This is a preparation step for
> > > future changes in D2D enable/disable sequence for xe2lpd that need to
> > > save some additional bits.
> > > 
> > > Cc: Anusha Srivatsa 
> > > Cc: Gustavo Sousa 
> > > Cc: Radhakrishna Sripada 
> > > Signed-off-by: Clint Taylor 
> > > Signed-off-by: Lucas De Marchi 
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_ddi.c | 36 +++-
> > >  drivers/gpu/drm/i915/display/intel_dp.c  |  2 +-
> > >  2 files changed, 18 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c 
> > > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > index 4668de45d6fe..29c9386659ff 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > @@ -325,26 +325,25 @@ static void intel_ddi_init_dp_buf_reg(struct 
> > > intel_encoder *encoder,
> > > const struct intel_crtc_state *crtc_state)
> > >  {
> > >   struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> > > - struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > >   struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> > >   enum phy phy = intel_port_to_phy(i915, encoder->port);
> > > 
> > >   /* DDI_BUF_CTL_ENABLE will be set by intel_ddi_prepare_link_retrain() 
> > > later */
> > > - intel_dp->DP = dig_port->saved_port_bits |
> > > + dig_port->saved_port_bits |=
> > 
> > Before this patch, saved_port_bits was a copy of DDI_BUF_PORT_REVERSAL
> > and DDI_A_4_LANES, either based on a value we readout from hardware at
> > startup, or based on VBT settings.  So it was a value of some
> > fundamental settings that we "saved" once at startup time and could then
> > just re-use thereafter.
> > 
> > If we're going to start saving per-modeset information (such as lane
> > count and link rate), then that's a pretty fundamental change to the
> > purpose of this field, and "saved_port_bits" doesn't really feel like an
> > appropriate name anymore.  We should probably rename it and add some
> > documentation on the field explaining exactly what its purpose is and
> > how/when it gets updated.
> 
> I will let Clint chime in as the original author here, but from what
> I can see this is basically a saved value of DDI_BUF_CTL(port), to be
> written when appropriate. We have more than just
> DDI_BUF_PORT_REVERSAL and DDI_A_4_LANES.

I think you grepped the wrong tree.  The output you pasted is where
saved_port_bits is being used after this LNL series gets applied.  The
usage of this field on today's drm-tip is just:

  drivers/gpu/drm/i915/display/intel_cx0_phy.c:2706:  bool lane_reversal = 
dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
  drivers/gpu/drm/i915/display/intel_display_types.h:1827:u32 
saved_port_bits;
  drivers/gpu/drm/i915/display/intel_tc.c:362:bool lane_reversal = 
dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
  drivers/gpu/drm/i915/display/intel_ddi.c:333:   intel_dp->DP = 
dig_port->saved_port_bits |
  drivers/gpu/drm/i915/display/intel_ddi.c:2259:  
dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
  drivers/gpu/drm/i915/display/intel_ddi.c:2389:  if (dig_port->saved_port_bits 
& DDI_BUF_PORT_REVERSAL)
  drivers/gpu/drm/i915/display/intel_ddi.c:3214:  buf_ctl = 
dig_port->saved_port_bits | DDI_BUF_CTL_ENABLE;
  drivers/gpu/drm/i915/display/intel_ddi.c:3221:  if 
(dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL)
  drivers/gpu/drm/i915/display/intel_ddi.c:4505:  if (dig_port->saved_port_bits 
& DDI_A_4_LANES)
  drivers/gpu/drm/i915/display/intel_ddi.c:4543:  
dig_port->saved_port_bits |= DDI_A_4_LANES;
  .--
  drivers/gpu/drm/i915/display/intel_ddi.c:4965:  
dig_port->saved_port_bits =
  drivers/gpu/drm/i915/display/intel_ddi.c-4966-  
intel_de_read(dev_priv, DDI_BUF_CTL(port))
  drivers/gpu/drm/i915/display/intel_ddi.c-4967-  & 
DDI_BUF_PORT_REVERSAL;
  .--
  drivers/gpu/drm/i915/display/intel_ddi.c:4969:  
dig_port->saved_port_bits =
  drivers/gpu/drm/i915/display/intel_ddi.c-4970-  
intel_de_read(dev_priv, DDI_BUF_CTL(port))
  drivers/gpu/drm/i915/display/intel_ddi.c-4971-  & 
(DDI_BUF_PORT_REVERSAL | DDI_A_4_LANES);
  .--
  drivers/gpu/drm/i915/display/intel_ddi.c:4974:  
dig_port->saved_port_bits |= DDI_BUF_PORT_REVERSAL;

I.e., using it as a cached copy of DDI_BUF_CTL is a large change from
the existing code; previously it was an unchanging stash of just those
two specific bits from startup

Re: [Intel-gfx] [Intel-xe] [PATCH v4 10/30] drm/i915/display: Consolidate saved port bits in intel_digital_port

2023-09-18 Thread Taylor, Clinton A
On Mon, 2023-09-18 at 16:06 -0500, Lucas De Marchi wrote:
> On Fri, Sep 15, 2023 at 12:50:41PM -0700, Matt Roper wrote:
> > On Fri, Sep 15, 2023 at 10:46:31AM -0700, Lucas De Marchi wrote:
> > > From: Clint Taylor 
> > > 
> > > We use multiple variables for HDMI and DisplayPort to store the value of
> > > DDI_BUF_CTL register (now called DDI_CTL_DE in the spec). Consolidate it
> > > to just one in struct intel_digital_port. This is a preparation step for
> > > future changes in D2D enable/disable sequence for xe2lpd that need to
> > > save some additional bits.
> > > 
> > > Cc: Anusha Srivatsa 
> > > Cc: Gustavo Sousa 
> > > Cc: Radhakrishna Sripada 
> > > Signed-off-by: Clint Taylor 
> > > Signed-off-by: Lucas De Marchi 
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_ddi.c | 36 +++-
> > >  drivers/gpu/drm/i915/display/intel_dp.c  |  2 +-
> > >  2 files changed, 18 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > index 4668de45d6fe..29c9386659ff 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > @@ -325,26 +325,25 @@ static void intel_ddi_init_dp_buf_reg(struct 
> > > intel_encoder
> > > *encoder,
> > > const struct intel_crtc_state *crtc_state)
> > >  {
> > >   struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> > > - struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > >   struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> > >   enum phy phy = intel_port_to_phy(i915, encoder->port);
> > > 
> > >   /* DDI_BUF_CTL_ENABLE will be set by intel_ddi_prepare_link_retrain() 
> > > later */
> > > - intel_dp->DP = dig_port->saved_port_bits |
> > > + dig_port->saved_port_bits |=
> > 
> > Before this patch, saved_port_bits was a copy of DDI_BUF_PORT_REVERSAL
> > and DDI_A_4_LANES, either based on a value we readout from hardware at
> > startup, or based on VBT settings.  So it was a value of some
> > fundamental settings that we "saved" once at startup time and could then
> > just re-use thereafter.
> > 
> > If we're going to start saving per-modeset information (such as lane
> > count and link rate), then that's a pretty fundamental change to the
> > purpose of this field, and "saved_port_bits" doesn't really feel like an
> > appropriate name anymore.  We should probably rename it and add some
> > documentation on the field explaining exactly what its purpose is and
> > how/when it gets updated.
> 
> I will let Clint chime in as the original author here, but from what
> I can see this is basically a saved value of DDI_BUF_CTL(port), to be
> written when appropriate. We have more than just
> DDI_BUF_PORT_REVERSAL and DDI_A_4_LANES.
> 
> drivers/gpu/drm/i915/display/intel_cx0_phy.c:   bool lane_reversal = dig_port-
> >saved_port_bits & DDI_BUF_PORT_REVERSAL;
> drivers/gpu/drm/i915/display/intel_ddi.c:   dig_port->saved_port_bits |=
> drivers/gpu/drm/i915/display/intel_ddi.c:   dig_port-
> >saved_port_bits |= DDI_BUF_PORT_DATA_40BIT;
> drivers/gpu/drm/i915/display/intel_ddi.c:   dig_port-
> >saved_port_bits |= DDI_BUF_PORT_DATA_10BIT;
> drivers/gpu/drm/i915/display/intel_ddi.c:   
> dig_port->saved_port_bits |=
> ddi_buf_phy_link_rate(crtc_state->port_clock);
> drivers/gpu/drm/i915/display/intel_ddi.c:   dig_port-
> >saved_port_bits |= DDI_BUF_CTL_TC_PHY_OWNERSHIP;
> drivers/gpu/drm/i915/display/intel_ddi.c:   dig_port->saved_port_bits &=
> ~DDI_BUF_EMP_MASK;
> drivers/gpu/drm/i915/display/intel_ddi.c:   dig_port->saved_port_bits |=
> signal_levels;
> drivers/gpu/drm/i915/display/intel_ddi.c:   intel_de_write(dev_priv,
> DDI_BUF_CTL(port), dig_port->saved_port_bits);
> drivers/gpu/drm/i915/display/intel_ddi.c:   dig_port-
> >saved_port_bits & DDI_BUF_PORT_REVERSAL;
> drivers/gpu/drm/i915/display/intel_ddi.c:   
> dig_port->saved_port_bits |=
> XE2LPD_DDI_BUF_D2D_LINK_ENABLE;
> drivers/gpu/drm/i915/display/intel_ddi.c:   if (dig_port->saved_port_bits 
> &
> DDI_BUF_PORT_REVERSAL)
> drivers/gpu/drm/i915/display/intel_ddi.c:   
> dig_port->saved_port_bits &=
> ~XE2LPD_DDI_BUF_D2D_LINK_ENABLE;
> drivers/gpu/drm/i915/display/intel_ddi.c:   dig_port->saved_port_bits |=
> DDI_BUF_CTL_ENABLE;
> drivers/gpu/drm/i915/display/intel_ddi.c:   if 
> (dig_port->saved_port_bits &
> DDI_BUF_PORT_REVERSAL)
> drivers/gpu/drm/i915/display/intel_ddi.c:   
> dig_port->saved_port_bits |=
> DDI_PORT_WIDTH(lane_count);
> drivers/gpu/drm/i915/display/intel_ddi.c:   
> dig_port->saved_port_bits |=
> DDI_BUF_CTL_TC_PHY_OWNERSHIP;
> drivers/gpu/drm/i915/display/intel_ddi.c:   intel_de_write(dev_priv,
> DDI_BUF_CTL(port), dig_port->saved_port_bits);
> drivers/gpu/drm/i915/display/intel_ddi.c:   dig_port->saved_po

Re: [Intel-gfx] [Intel-xe] [PATCH v4 10/30] drm/i915/display: Consolidate saved port bits in intel_digital_port

2023-09-18 Thread Lucas De Marchi

On Fri, Sep 15, 2023 at 12:50:41PM -0700, Matt Roper wrote:

On Fri, Sep 15, 2023 at 10:46:31AM -0700, Lucas De Marchi wrote:

From: Clint Taylor 

We use multiple variables for HDMI and DisplayPort to store the value of
DDI_BUF_CTL register (now called DDI_CTL_DE in the spec). Consolidate it
to just one in struct intel_digital_port. This is a preparation step for
future changes in D2D enable/disable sequence for xe2lpd that need to
save some additional bits.

Cc: Anusha Srivatsa 
Cc: Gustavo Sousa 
Cc: Radhakrishna Sripada 
Signed-off-by: Clint Taylor 
Signed-off-by: Lucas De Marchi 
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 36 +++-
 drivers/gpu/drm/i915/display/intel_dp.c  |  2 +-
 2 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c 
b/drivers/gpu/drm/i915/display/intel_ddi.c
index 4668de45d6fe..29c9386659ff 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -325,26 +325,25 @@ static void intel_ddi_init_dp_buf_reg(struct 
intel_encoder *encoder,
  const struct intel_crtc_state *crtc_state)
 {
struct drm_i915_private *i915 = to_i915(encoder->base.dev);
-   struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
enum phy phy = intel_port_to_phy(i915, encoder->port);

/* DDI_BUF_CTL_ENABLE will be set by intel_ddi_prepare_link_retrain() 
later */
-   intel_dp->DP = dig_port->saved_port_bits |
+   dig_port->saved_port_bits |=


Before this patch, saved_port_bits was a copy of DDI_BUF_PORT_REVERSAL
and DDI_A_4_LANES, either based on a value we readout from hardware at
startup, or based on VBT settings.  So it was a value of some
fundamental settings that we "saved" once at startup time and could then
just re-use thereafter.

If we're going to start saving per-modeset information (such as lane
count and link rate), then that's a pretty fundamental change to the
purpose of this field, and "saved_port_bits" doesn't really feel like an
appropriate name anymore.  We should probably rename it and add some
documentation on the field explaining exactly what its purpose is and
how/when it gets updated.


I will let Clint chime in as the original author here, but from what
I can see this is basically a saved value of DDI_BUF_CTL(port), to be
written when appropriate. We have more than just
DDI_BUF_PORT_REVERSAL and DDI_A_4_LANES.

drivers/gpu/drm/i915/display/intel_cx0_phy.c:   bool lane_reversal = 
dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
drivers/gpu/drm/i915/display/intel_ddi.c:   dig_port->saved_port_bits |=
drivers/gpu/drm/i915/display/intel_ddi.c:   
dig_port->saved_port_bits |= DDI_BUF_PORT_DATA_40BIT;
drivers/gpu/drm/i915/display/intel_ddi.c:   
dig_port->saved_port_bits |= DDI_BUF_PORT_DATA_10BIT;
drivers/gpu/drm/i915/display/intel_ddi.c:   dig_port->saved_port_bits 
|= ddi_buf_phy_link_rate(crtc_state->port_clock);
drivers/gpu/drm/i915/display/intel_ddi.c:   
dig_port->saved_port_bits |= DDI_BUF_CTL_TC_PHY_OWNERSHIP;
drivers/gpu/drm/i915/display/intel_ddi.c:   dig_port->saved_port_bits &= 
~DDI_BUF_EMP_MASK;
drivers/gpu/drm/i915/display/intel_ddi.c:   dig_port->saved_port_bits |= 
signal_levels;
drivers/gpu/drm/i915/display/intel_ddi.c:   intel_de_write(dev_priv, 
DDI_BUF_CTL(port), dig_port->saved_port_bits);
drivers/gpu/drm/i915/display/intel_ddi.c:   
dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
drivers/gpu/drm/i915/display/intel_ddi.c:   
dig_port->saved_port_bits |= XE2LPD_DDI_BUF_D2D_LINK_ENABLE;
drivers/gpu/drm/i915/display/intel_ddi.c:   if (dig_port->saved_port_bits & 
DDI_BUF_PORT_REVERSAL)
drivers/gpu/drm/i915/display/intel_ddi.c:   dig_port->saved_port_bits 
&= ~XE2LPD_DDI_BUF_D2D_LINK_ENABLE;
drivers/gpu/drm/i915/display/intel_ddi.c:   dig_port->saved_port_bits |= 
DDI_BUF_CTL_ENABLE;
drivers/gpu/drm/i915/display/intel_ddi.c:   if 
(dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL)
drivers/gpu/drm/i915/display/intel_ddi.c:   
dig_port->saved_port_bits |= DDI_PORT_WIDTH(lane_count);
drivers/gpu/drm/i915/display/intel_ddi.c:   
dig_port->saved_port_bits |= DDI_BUF_CTL_TC_PHY_OWNERSHIP;
drivers/gpu/drm/i915/display/intel_ddi.c:   intel_de_write(dev_priv, 
DDI_BUF_CTL(port), dig_port->saved_port_bits);
drivers/gpu/drm/i915/display/intel_ddi.c:   dig_port->saved_port_bits |= 
DDI_BUF_CTL_ENABLE;
drivers/gpu/drm/i915/display/intel_ddi.c:   intel_de_write(dev_priv, 
DDI_BUF_CTL(port), dig_port->saved_port_bits);
drivers/gpu/drm/i915/display/intel_ddi.c:   dig_port->saved_port_bits |= 
DDI_BUF_CTL_ENABLE;
drivers/gpu/drm/i915/display/intel_ddi.c:   intel_de_write(dev_priv, 
DDI_BUF_CTL(port), dig_port->saved_port_bits);
drive

[Intel-gfx] [PATCH i-g-t] tests/i915_pm_freq_api: Set min/max to expected values

2023-09-18 Thread Vinay Belgaumkar
A prior(rps) test leaves the system in a bad state causing failures
in the basic test. Set min/max to expected values before running it.
Test will restore values at the end.

Link: https://gitlab.freedesktop.org/drm/intel/-/issues/8670

Signed-off-by: Vinay Belgaumkar 
---
 tests/intel/i915_pm_freq_api.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tests/intel/i915_pm_freq_api.c b/tests/intel/i915_pm_freq_api.c
index 03bd0d05b..6018692a2 100644
--- a/tests/intel/i915_pm_freq_api.c
+++ b/tests/intel/i915_pm_freq_api.c
@@ -55,7 +55,11 @@ static void test_freq_basic_api(int dirfd, int gt)
rpn = get_freq(dirfd, RPS_RPn_FREQ_MHZ);
rp0 = get_freq(dirfd, RPS_RP0_FREQ_MHZ);
rpe = get_freq(dirfd, RPS_RP1_FREQ_MHZ);
-   igt_debug("GT: %d, RPn: %d, RPe: %d, RP0: %d", gt, rpn, rpe, rp0);
+   igt_debug("GT: %d, RPn: %d, RPe: %d, RP0: %d\n", gt, rpn, rpe, rp0);
+
+   /* Set min/max to RPn, RP0 for baseline behavior */
+   igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn) > 0);
+   igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rp0) > 0);
 
/*
 * Negative bound tests
@@ -170,7 +174,7 @@ igt_main
for_each_sysfs_gt_dirfd(i915, dirfd, gt) {
stash_min[gt] = get_freq(dirfd, RPS_MIN_FREQ_MHZ);
stash_max[gt] = get_freq(dirfd, RPS_MAX_FREQ_MHZ);
-   igt_debug("GT: %d, min: %d, max: %d", gt, 
stash_min[gt], stash_max[gt]);
+   igt_debug("GT: %d, min: %d, max: %d\n", gt, 
stash_min[gt], stash_max[gt]);
igt_pm_ignore_slpc_efficient_freq(i915, dirfd, true);
}
igt_install_exit_handler(restore_sysfs_freq);
-- 
2.38.1



[Intel-gfx] [PATCH v5 09/25] drm/i915: Add helpers for BW management on shared display links

2023-09-18 Thread Imre Deak
At the moment a modeset fails if the config computation of a pipe can't
fit its required BW to the available link BW even though the limitation
may be resolved by reducing the BW requirement of other pipes.

To improve the above this patch adds helper functions checking the
overall BW limits after all CRTC states have been computed. If the check
fails the maximum link bpp for a selected pipe will be reduced and all
the CRTC states will be recomputed until either the overall BW limit
check passes, or further bpp reduction is not possible (because all
pipes/encoders sharing the link BW reached their minimum link bpp).

Atm, the MST encoder allocates twice the required BW for YUV420 format
streams. A follow-up patchset will fix that, add a code comment about
this.

This change prepares for upcoming patches enabling the above BW
management on FDI and MST links.

v2:
- Rename intel_crtc_state::max_link_bpp to max_link_bpp_x16 and
  intel_link_bw_limits::max_bpp to max_bpp_x16. (Jani)
v3:
- Add the helper functions in a separate patch. (Ville)
- Add the functions to intel_link_bw.c instead of intel_atomic.c (Ville)
- Return -ENOSPC instead of -EINVAL to userspace in case of a link BW
  limit failure.
v4:
- Make intel_atomic_check_config() static.
v5: (Ville)
- Rename intel_link_bw_limits::min_bpp_pipes to min_bpp_reached_pipes
  and intel_link_bw_reset_pipe_limit_to_min() to
  intel_link_bw_set_min_bpp_for_pipe().
- Rename pipe_bpp to link_bpp in intel_link_bw_reduce_bpp().
- Add FIXME: comment about MST encoder's YUV420 BW allocation and
  tracking the link bpp limit accordingly.

Cc: Jani Nikula 
Cc: Ville Syrjälä 
Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/i915/Makefile |   1 +
 drivers/gpu/drm/i915/display/intel_crtc.c |   1 +
 drivers/gpu/drm/i915/display/intel_display.c  |  65 -
 .../drm/i915/display/intel_display_types.h|   3 +-
 drivers/gpu/drm/i915/display/intel_dp_mst.c   |   4 +
 drivers/gpu/drm/i915/display/intel_link_bw.c  | 232 ++
 drivers/gpu/drm/i915/display/intel_link_bw.h  |  38 +++
 7 files changed, 339 insertions(+), 5 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_link_bw.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_link_bw.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 1b2e02e9d92cb..de4967c141f00 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -268,6 +268,7 @@ i915-y += \
display/intel_hotplug.o \
display/intel_hotplug_irq.o \
display/intel_hti.o \
+   display/intel_link_bw.o \
display/intel_load_detect.o \
display/intel_lpe_audio.o \
display/intel_modeset_lock.o \
diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c 
b/drivers/gpu/drm/i915/display/intel_crtc.c
index 182c6dd64f47c..1eda6a9f19aa8 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc.c
@@ -175,6 +175,7 @@ void intel_crtc_state_reset(struct intel_crtc_state 
*crtc_state,
crtc_state->hsw_workaround_pipe = INVALID_PIPE;
crtc_state->scaler_state.scaler_id = -1;
crtc_state->mst_master_transcoder = INVALID_TRANSCODER;
+   crtc_state->max_link_bpp_x16 = INT_MAX;
 }
 
 static struct intel_crtc *intel_crtc_alloc(void)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 3bdc338a22e19..537884035304c 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -87,6 +87,7 @@
 #include "intel_frontbuffer.h"
 #include "intel_hdmi.h"
 #include "intel_hotplug.h"
+#include "intel_link_bw.h"
 #include "intel_lvds.h"
 #include "intel_lvds_regs.h"
 #include "intel_modeset_setup.h"
@@ -4596,7 +4597,8 @@ intel_crtc_prepare_cleared_state(struct 
intel_atomic_state *state,
 
 static int
 intel_modeset_pipe_config(struct intel_atomic_state *state,
- struct intel_crtc *crtc)
+ struct intel_crtc *crtc,
+ const struct intel_link_bw_limits *limits)
 {
struct drm_i915_private *i915 = to_i915(crtc->base.dev);
struct intel_crtc_state *crtc_state =
@@ -4628,6 +4630,15 @@ intel_modeset_pipe_config(struct intel_atomic_state 
*state,
if (ret)
return ret;
 
+   crtc_state->max_link_bpp_x16 = limits->max_bpp_x16[crtc->pipe];
+
+   if (crtc_state->pipe_bpp > to_bpp_int(crtc_state->max_link_bpp_x16)) {
+   drm_dbg_kms(&i915->drm,
+   "[CRTC:%d:%s] Link bpp limited to " BPP_X16_FMT 
"\n",
+   crtc->base.base.id, crtc->base.name,
+   BPP_X16_ARGS(crtc_state->max_link_bpp_x16));
+   }
+
base_bpp = crtc_state->pipe_bpp;
 
/*
@@ -6218,7 +6229,9 @@ static int intel_bigjoiner_add_affected_crtcs(struct 
intel_atomic_state *state)
return 0;
 }
 
-static int int

[Intel-gfx] [PATCH v5 10/25] drm/i915/fdi: Improve FDI BW sharing between pipe B and C

2023-09-18 Thread Imre Deak
At the moment modesetting pipe C on IVB will fail if pipe B uses 4 FDI
lanes. Make the BW sharing more dynamic by trying to reduce pipe B's
link bpp in this case, until pipe B uses only up to 2 FDI lanes.

For this instead of the encoder compute config retry loop - which
reduced link bpp only for the encoder's pipe - reduce the maximum link
bpp for pipe B/C as required after all CRTC states are computed and
recompute the CRTC states with the new bpp limit.

Atm, all FDI encoder's compute config function returns an error if a BW
constrain prevents increasing the pipe bpp value. SDVO is an exception
where this case is only handled in the outer config retry loop, failing
the modeset. For consistency handle this case already during compute
config similarly to the other encoders.

v2:
- Don't assume that a CRTC is already in the atomic state, while
  reducing its link bpp.
- Add DocBook description to intel_fdi_atomic_check_link().
v3:
- Enable BW management for FDI links in a separate patch. (Ville)
v4: (Ville)
- Fail the SDVO encoder config computation if it doesn't support the
  link bpp limit.
- Add TODO: comments about checking link_bpp_x16 instead of
  bw_constrained.

Cc: Ville Syrjälä 
Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/i915/display/g4x_hdmi.c  |  6 +-
 drivers/gpu/drm/i915/display/intel_crt.c |  8 ++
 drivers/gpu/drm/i915/display/intel_display.c | 14 +---
 drivers/gpu/drm/i915/display/intel_dp.c  |  3 +-
 drivers/gpu/drm/i915/display/intel_fdi.c | 87 +++-
 drivers/gpu/drm/i915/display/intel_fdi.h |  4 +
 drivers/gpu/drm/i915/display/intel_link_bw.c |  7 +-
 drivers/gpu/drm/i915/display/intel_lvds.c| 11 ++-
 drivers/gpu/drm/i915/display/intel_sdvo.c| 14 +++-
 9 files changed, 113 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/g4x_hdmi.c 
b/drivers/gpu/drm/i915/display/g4x_hdmi.c
index 634b14116d9dd..8b5d26cd3b85e 100644
--- a/drivers/gpu/drm/i915/display/g4x_hdmi.c
+++ b/drivers/gpu/drm/i915/display/g4x_hdmi.c
@@ -19,6 +19,7 @@
 #include "intel_fifo_underrun.h"
 #include "intel_hdmi.h"
 #include "intel_hotplug.h"
+#include "intel_link_bw.h"
 #include "intel_sdvo.h"
 #include "vlv_sideband.h"
 
@@ -133,8 +134,11 @@ static int g4x_hdmi_compute_config(struct intel_encoder 
*encoder,
struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
struct drm_i915_private *i915 = to_i915(encoder->base.dev);
 
-   if (HAS_PCH_SPLIT(i915))
+   if (HAS_PCH_SPLIT(i915)) {
crtc_state->has_pch_encoder = true;
+   if (!intel_link_bw_compute_pipe_bpp(crtc_state))
+   return -EINVAL;
+   }
 
if (IS_G4X(i915))
crtc_state->has_hdmi_sink = g4x_compute_has_hdmi_sink(state, 
crtc);
diff --git a/drivers/gpu/drm/i915/display/intel_crt.c 
b/drivers/gpu/drm/i915/display/intel_crt.c
index 595e65f1c13aa..1745c860a3db3 100644
--- a/drivers/gpu/drm/i915/display/intel_crt.c
+++ b/drivers/gpu/drm/i915/display/intel_crt.c
@@ -49,6 +49,7 @@
 #include "intel_gmbus.h"
 #include "intel_hotplug.h"
 #include "intel_hotplug_irq.h"
+#include "intel_link_bw.h"
 #include "intel_load_detect.h"
 #include "intel_pch_display.h"
 #include "intel_pch_refclk.h"
@@ -413,6 +414,9 @@ static int pch_crt_compute_config(struct intel_encoder 
*encoder,
return -EINVAL;
 
pipe_config->has_pch_encoder = true;
+   if (!intel_link_bw_compute_pipe_bpp(pipe_config))
+   return -EINVAL;
+
pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 
return 0;
@@ -435,10 +439,14 @@ static int hsw_crt_compute_config(struct intel_encoder 
*encoder,
return -EINVAL;
 
pipe_config->has_pch_encoder = true;
+   if (!intel_link_bw_compute_pipe_bpp(pipe_config))
+   return -EINVAL;
+
pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 
/* LPT FDI RX only supports 8bpc. */
if (HAS_PCH_LPT(dev_priv)) {
+   /* TODO: Check crtc_state->max_link_bpp_x16 instead of 
bw_constrained */
if (pipe_config->bw_constrained && pipe_config->pipe_bpp < 24) {
drm_dbg_kms(&dev_priv->drm,
"LPT only supports 24bpp\n");
diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 537884035304c..a6656d6c985e6 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4607,7 +4607,6 @@ intel_modeset_pipe_config(struct intel_atomic_state 
*state,
struct drm_connector_state *connector_state;
int pipe_src_w, pipe_src_h;
int base_bpp, ret, i;
-   bool retry = true;
 
crtc_state->cpu_transcoder = (enum transcoder) crtc->pipe;
 
@@ -4637,6 +4636,7 @@ intel_modeset_pipe_config(struct intel_atomic_state 
*state,
"[CRTC:%d:%s] Link bpp limited to " BPP_X16_FMT 

[Intel-gfx] [PATCH v5 07/25] drm/i915: Rename intel_modeset_all_pipes() to intel_modeset_all_pipes_late()

2023-09-18 Thread Imre Deak
Rename intel_modeset_all_pipes() to intel_modeset_all_pipes_late() to
clarify when the function can be called (vs.
intel_modeset_pipes_in_mask_early()).

Cc: Ville Syrjälä 
Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/i915/display/intel_cdclk.c   | 2 +-
 drivers/gpu/drm/i915/display/intel_display.c | 6 +++---
 drivers/gpu/drm/i915/display/intel_display.h | 4 ++--
 drivers/gpu/drm/i915/display/skl_watermark.c | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c 
b/drivers/gpu/drm/i915/display/intel_cdclk.c
index ad5251ba6fe13..a2e20b25d6361 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -3139,7 +3139,7 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state 
*state)
} else if (intel_cdclk_needs_modeset(&old_cdclk_state->actual,
 &new_cdclk_state->actual)) {
/* All pipes must be switched off while we change the cdclk. */
-   ret = intel_modeset_all_pipes(state, "CDCLK change");
+   ret = intel_modeset_all_pipes_late(state, "CDCLK change");
if (ret)
return ret;
 
diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 1c9a128f4af27..9a69b8848fee6 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5475,7 +5475,7 @@ int intel_modeset_pipes_in_mask_early(struct 
intel_atomic_state *state,
 }
 
 /**
- * intel_modeset_all_pipes - force a full modeset on all pipes
+ * intel_modeset_all_pipes_late - force a full modeset on all pipes
  * @state: intel atomic state
  * @reason: the reason for the full modeset
  *
@@ -5485,8 +5485,8 @@ int intel_modeset_pipes_in_mask_early(struct 
intel_atomic_state *state,
  *
  * Returns 0 in case of success, negative error code otherwise.
  */
-int intel_modeset_all_pipes(struct intel_atomic_state *state,
-   const char *reason)
+int intel_modeset_all_pipes_late(struct intel_atomic_state *state,
+const char *reason)
 {
struct drm_i915_private *dev_priv = to_i915(state->base.dev);
struct intel_crtc *crtc;
diff --git a/drivers/gpu/drm/i915/display/intel_display.h 
b/drivers/gpu/drm/i915/display/intel_display.h
index 3008e8b2b437e..64a5be7859331 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -515,8 +515,8 @@ void intel_update_watermarks(struct drm_i915_private *i915);
 /* modesetting */
 int intel_modeset_pipes_in_mask_early(struct intel_atomic_state *state,
  const char *reason, u8 pipe_mask);
-int intel_modeset_all_pipes(struct intel_atomic_state *state,
-   const char *reason);
+int intel_modeset_all_pipes_late(struct intel_atomic_state *state,
+const char *reason);
 void intel_modeset_get_crtc_power_domains(struct intel_crtc_state *crtc_state,
  struct intel_power_domain_mask 
*old_domains);
 void intel_modeset_put_crtc_power_domains(struct intel_crtc *crtc,
diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c 
b/drivers/gpu/drm/i915/display/skl_watermark.c
index 063929a42a42f..a29d9b717deed 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.c
+++ b/drivers/gpu/drm/i915/display/skl_watermark.c
@@ -2616,7 +2616,7 @@ skl_compute_ddb(struct intel_atomic_state *state)
 
if (old_dbuf_state->joined_mbus != new_dbuf_state->joined_mbus) 
{
/* TODO: Implement vblank synchronized MBUS joining 
changes */
-   ret = intel_modeset_all_pipes(state, "MBUS joining 
change");
+   ret = intel_modeset_all_pipes_late(state, "MBUS joining 
change");
if (ret)
return ret;
}
-- 
2.37.2



[Intel-gfx] [PATCH v5 06/25] drm/i915: Add helper to modeset a set of pipes

2023-09-18 Thread Imre Deak
Add intel_modeset_pipes_in_mask_early() to modeset a provided set of
pipes, used in a follow-up patch.

As opposed intel_modeset_all_pipes() which modesets only the active
pipes - others don't requiring programming the HW - modeset all enabled
pipes in intel_modeset_pipes_in_mask_early() which may need to recompute
their state even if they are not active (that is in the DPMS off state).

While at it add DocBook descriptions for the two exported functions.

v2:
- Add a flag controlling if active planes are force updated as well.
- Add DockBook descriptions.
v3:
- For clarity use _early/_late suffixes for the exported functions
  instead of the update_active_planes parameter. (Ville)
v4:
- In intel_modeset_pipes_in_mask_early() update only the crtc
  flags relevant to the early phase. (Ville)
- Rename intel_modeset_all_pipes() in a separate patch.

Cc: Ville Syrjälä 
Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/i915/display/intel_display.c | 102 +++
 drivers/gpu/drm/i915/display/intel_display.h |   2 +
 2 files changed, 83 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 6bbc9069754c4..1c9a128f4af27 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5407,16 +5407,90 @@ intel_verify_planes(struct intel_atomic_state *state)
 plane_state->uapi.visible);
 }
 
+static int intel_modeset_pipe(struct intel_atomic_state *state,
+ struct intel_crtc_state *crtc_state,
+ const char *reason)
+{
+   struct drm_i915_private *i915 = to_i915(state->base.dev);
+   struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+   int ret;
+
+   drm_dbg_kms(&i915->drm, "[CRTC:%d:%s] Full modeset due to %s\n",
+   crtc->base.base.id, crtc->base.name, reason);
+
+   ret = drm_atomic_add_affected_connectors(&state->base,
+&crtc->base);
+   if (ret)
+   return ret;
+
+   ret = intel_dp_mst_add_topology_state_for_crtc(state, crtc);
+   if (ret)
+   return ret;
+
+   ret = intel_atomic_add_affected_planes(state, crtc);
+   if (ret)
+   return ret;
+
+   crtc_state->uapi.mode_changed = true;
+
+   return 0;
+}
+
+/**
+ * intel_modeset_pipes_in_mask_early - force a full modeset on a set of pipes
+ * @state: intel atomic state
+ * @reason: the reason for the full modeset
+ * @mask: mask of pipes to modeset
+ *
+ * Add pipes in @mask to @state and force a full modeset on the enabled ones
+ * due to the description in @reason.
+ * This function can be called only before new plane states are computed.
+ *
+ * Returns 0 in case of success, negative error code otherwise.
+ */
+int intel_modeset_pipes_in_mask_early(struct intel_atomic_state *state,
+ const char *reason, u8 mask)
+{
+   struct drm_i915_private *i915 = to_i915(state->base.dev);
+   struct intel_crtc *crtc;
+
+   for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, mask) {
+   struct intel_crtc_state *crtc_state;
+   int ret;
+
+   crtc_state = intel_atomic_get_crtc_state(&state->base, crtc);
+   if (IS_ERR(crtc_state))
+   return PTR_ERR(crtc_state);
+
+   if (!crtc_state->hw.enable ||
+   intel_crtc_needs_modeset(crtc_state))
+   continue;
+
+   ret = intel_modeset_pipe(state, crtc_state, reason);
+   if (ret)
+   return ret;
+   }
+
+   return 0;
+}
+
+/**
+ * intel_modeset_all_pipes - force a full modeset on all pipes
+ * @state: intel atomic state
+ * @reason: the reason for the full modeset
+ *
+ * Add all pipes to @state and force a full modeset on the active ones due to
+ * the description in @reason.
+ * This function can be called only after new plane states are computed 
already.
+ *
+ * Returns 0 in case of success, negative error code otherwise.
+ */
 int intel_modeset_all_pipes(struct intel_atomic_state *state,
const char *reason)
 {
struct drm_i915_private *dev_priv = to_i915(state->base.dev);
struct intel_crtc *crtc;
 
-   /*
-* Add all pipes to the state, and force
-* a modeset on all the active ones.
-*/
for_each_intel_crtc(&dev_priv->drm, crtc) {
struct intel_crtc_state *crtc_state;
int ret;
@@ -5429,25 +5503,11 @@ int intel_modeset_all_pipes(struct intel_atomic_state 
*state,
intel_crtc_needs_modeset(crtc_state))
continue;
 
-   drm_dbg_kms(&dev_priv->drm, "[CRTC:%d:%s] Full modeset due to 
%s\n",
-   crtc->base.base.id, crtc->base.name, reason);
+

Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/pxp/mtl: Update gsc-heci cmd submission to align with fw/hw spec

2023-09-18 Thread Teres Alexis, Alan Previn
On Sun, 2023-09-17 at 22:04 +, Patchwork wrote:
> Patch Details
> Series: drm/i915/pxp/mtl: Update gsc-heci cmd submission to align with fw/hw 
> spec
> URL:https://patchwork.freedesktop.org/series/123830/
> 

alan:snip
Below issue it unrelated since this series only changes code paths in MTL-PXP 
only and in pxp-specific lrc code. (double checked). will retrigger test since 
it was a BAT failure.

> Possible new issues
> 
> Here are the unknown changes that may have been introduced in 
> Patchwork_123830v1:
> 
> IGT changes
> Possible regressions
> 
>   *   igt@gem_exec_suspend@basic-s3@lmem0:
>  *   bat-dg2-8: NOTRUN -> 
> INCOMPLETE
> 



Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/cx0: Add step for programming msgbus timer (rev2)

2023-09-18 Thread Matt Roper
On Wed, Sep 13, 2023 at 08:59:49AM -0300, Gustavo Sousa wrote:
> Quoting Patchwork (2023-09-12 19:24:10-03:00)
> >== Series Details ==
> >
> >Series: drm/i915/cx0: Add step for programming msgbus timer (rev2)
> >URL   : https://patchwork.freedesktop.org/series/123551/
> >State : failure
> >
> >== Summary ==
> >
> >CI Bug Log - changes from CI_DRM_13623_full -> Patchwork_123551v2_full
> >
> >
> >Summary
> >---
> >
> >  **FAILURE**
> >
> >  Serious unknown changes coming with Patchwork_123551v2_full absolutely 
> > need to be
> >  verified manually.
> >  
> >  If you think the reported changes have nothing to do with the changes
> >  introduced in Patchwork_123551v2_full, please notify your bug team 
> > (lgci.bug.fil...@intel.com) to allow them
> >  to document this new failure mode, which will reduce false positives in CI.
> >
> >  
> >
> >Participating hosts (9 -> 9)
> >--
> >
> >  No changes in participating hosts
> >
> >Possible new issues
> >---
> >
> >  Here are the unknown changes that may have been introduced in 
> > Patchwork_123551v2_full:
> >
> >### IGT changes ###
> >
> > Possible regressions 
> >
> >  * igt@gem_eio@in-flight-contexts-10ms:
> >- shard-snb:  [PASS][1] -> [FAIL][2]
> >   [1]: 
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13623/shard-snb6/igt@gem_...@in-flight-contexts-10ms.html
> >   [2]: 
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123551v2/shard-snb4/igt@gem_...@in-flight-contexts-10ms.html
> >
> >  * igt@gen9_exec_parse@allowed-single:
> >- shard-apl:  [PASS][3] -> [INCOMPLETE][4]
> >   [3]: 
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13623/shard-apl4/igt@gen9_exec_pa...@allowed-single.html
> >   [4]: 
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123551v2/shard-apl6/igt@gen9_exec_pa...@allowed-single.html
> >
> >  * igt@kms_rotation_crc@primary-rotation-270:
> >- shard-rkl:  [PASS][5] -> [INCOMPLETE][6] +1 other test 
> > incomplete
> >   [5]: 
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13623/shard-rkl-2/igt@kms_rotation_...@primary-rotation-270.html
> >   [6]: 
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123551v2/shard-rkl-1/igt@kms_rotation_...@primary-rotation-270.html
> >
> >  * igt@kms_universal_plane@cursor-fb-leak-pipe-a:
> >- shard-glk:  [PASS][7] -> [FAIL][8]
> >   [7]: 
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13623/shard-glk1/igt@kms_universal_pl...@cursor-fb-leak-pipe-a.html
> >   [8]: 
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123551v2/shard-glk3/igt@kms_universal_pl...@cursor-fb-leak-pipe-a.html
> >
> >  * igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend:
> >- shard-dg2:  [PASS][9] -> [INCOMPLETE][10]
> >   [9]: 
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13623/shard-dg2-1/igt@kms_vbl...@pipe-b-ts-continuation-dpms-suspend.html
> >   [10]: 
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123551v2/shard-dg2-5/igt@kms_vbl...@pipe-b-ts-continuation-dpms-suspend.html
> >
> 
> None of those should be related to this patch since those machines do not use
> the C10 or C20 PHYs.

Applied to drm-intel-next.  Thanks for the patch and review.


Matt


> 
> Gustavo Sousa
> 
> >  
> >Known issues
> >
> >
> >  Here are the changes found in Patchwork_123551v2_full that come from known 
> > issues:
> >
> >### IGT changes ###
> >
> > Issues hit 
> >
> >  * igt@api_intel_bb@object-reloc-keep-cache:
> >- shard-dg2:  NOTRUN -> [SKIP][11] ([i915#8411])
> >   [11]: 
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123551v2/shard-dg2-7/igt@api_intel...@object-reloc-keep-cache.html
> >
> >  * igt@api_intel_bb@object-reloc-purge-cache:
> >- shard-mtlp: NOTRUN -> [SKIP][12] ([i915#8411])
> >   [12]: 
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123551v2/shard-mtlp-4/igt@api_intel...@object-reloc-purge-cache.html
> >
> >  * igt@device_reset@cold-reset-bound:
> >- shard-dg2:  NOTRUN -> [SKIP][13] ([i915#7701])
> >   [13]: 
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123551v2/shard-dg2-7/igt@device_re...@cold-reset-bound.html
> >
> >  * igt@drm_fdinfo@most-busy-idle-check-all@vecs1:
> >- shard-dg2:  NOTRUN -> [SKIP][14] ([i915#8414]) +9 other tests 
> > skip
> >   [14]: 
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123551v2/shard-dg2-7/igt@drm_fdinfo@most-busy-idle-check-...@vecs1.html
> >
> >  * igt@feature_discovery@psr1:
> >- shard-dg2:  NOTRUN -> [SKIP][15] ([i915#658])
> >   [15]: 
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123551v2/shard-dg2-10/igt@feature_discov...@psr1.html
> >
> >  * igt@gem_close_race@multigpu-basic-process:
> >- shard-mtlp: NOTRUN -> [SKIP][16] ([i915#7697])
> >   [16]: 
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123551v2/shard-mtlp-4/igt@gem_close_r...

[Intel-gfx] [PATCH 4/7] drm/i915: Parameterize binder context creation

2023-09-18 Thread Nirmoy Das
Add i915_ggtt_require_binder() to indicate that i915
needs to create binder context which will be used
by subsequent patch to enable i915_address_space vfuncs
that will use GPU commands to update GGTT.

Signed-off-by: Nirmoy Das 
Reviewed-by: Oak Zeng 
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +-
 drivers/gpu/drm/i915/gt/intel_gtt.c   | 4 
 drivers/gpu/drm/i915/gt/intel_gtt.h   | 2 ++
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c 
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 0a58fe812ec5..9d44ca70fef5 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1477,7 +1477,7 @@ static int engine_init_common(struct intel_engine_cs 
*engine)
 * engines as well but BCS should be less busy engine so pick that for
 * GGTT updates.
 */
-   if (engine->id == BCS0) {
+   if (i915_ggtt_require_binder(engine->i915) && engine->id == BCS0) {
bce = create_ggtt_bind_context(engine);
if (IS_ERR(bce)) {
intel_engine_destroy_pinned_context(ce);
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c 
b/drivers/gpu/drm/i915/gt/intel_gtt.c
index 13944a14ea2d..4c89eb8d9af7 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -21,6 +21,10 @@
 #include "intel_gt_regs.h"
 #include "intel_gtt.h"
 
+bool i915_ggtt_require_binder(struct drm_i915_private *i915)
+{
+   return false;
+}
 
 static bool intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *i915)
 {
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h 
b/drivers/gpu/drm/i915/gt/intel_gtt.h
index 41e530d0a4e9..b471edac2699 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
@@ -691,4 +691,6 @@ static inline struct sgt_dma {
return (struct sgt_dma){ sg, addr, addr + sg_dma_len(sg) };
 }
 
+bool i915_ggtt_require_binder(struct drm_i915_private *i915);
+
 #endif
-- 
2.41.0



[Intel-gfx] [PATCH 5/7] drm/i915: Implement GGTT update method with MI_UPDATE_GTT

2023-09-18 Thread Nirmoy Das
Implement GGTT update method with blitter command, MI_UPDATE_GTT
and install those handlers if a platform requires that.

v2: Make sure we hold the GT wakeref and Blitter engine wakeref before
we call mutex_lock/intel_context_enter below. When GT/engine are not
awake, the intel_context_enter calls into some runtime pm function which
can end up with kmalloc/fs_reclaim. But trigger fs_reclaim holding a
mutex lock is not allowed because shrinker can also try to hold the same
mutex lock. It is a circular lock. So hold the GT/blitter engine wakeref
before calling mutex_lock, to fix the circular lock.

Signed-off-by: Nirmoy Das 
Signed-off-by: Oak Zeng 
Acked-by: Oak Zeng 
---
 drivers/gpu/drm/i915/gt/intel_ggtt.c | 235 +++
 1 file changed, 235 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c 
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index dd0ed941441a..b94de7cebfce 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -15,18 +15,23 @@
 #include "display/intel_display.h"
 #include "gem/i915_gem_lmem.h"
 
+#include "intel_context.h"
 #include "intel_ggtt_gmch.h"
+#include "intel_gpu_commands.h"
 #include "intel_gt.h"
 #include "intel_gt_regs.h"
 #include "intel_pci_config.h"
+#include "intel_ring.h"
 #include "i915_drv.h"
 #include "i915_pci.h"
+#include "i915_request.h"
 #include "i915_scatterlist.h"
 #include "i915_utils.h"
 #include "i915_vgpu.h"
 
 #include "intel_gtt.h"
 #include "gen8_ppgtt.h"
+#include "intel_engine_pm.h"
 
 static void i915_ggtt_color_adjust(const struct drm_mm_node *node,
   unsigned long color,
@@ -252,6 +257,145 @@ u64 gen8_ggtt_pte_encode(dma_addr_t addr,
return pte;
 }
 
+static bool should_update_ggtt_with_bind(struct i915_ggtt *ggtt)
+{
+   struct intel_gt *gt = ggtt->vm.gt;
+
+   return intel_gt_is_bind_context_ready(gt);
+}
+
+static struct intel_context *gen8_ggtt_bind_get_ce(struct i915_ggtt *ggtt)
+{
+   struct intel_context *ce;
+   struct intel_gt *gt = ggtt->vm.gt;
+
+   if (intel_gt_is_wedged(gt))
+   return NULL;
+
+   ce = gt->engine[BCS0]->bind_context;
+   GEM_BUG_ON(!ce);
+
+   /*
+* If the GT is not awake already at this stage then fallback
+* to pci based GGTT update otherwise __intel_wakeref_get_first()
+* would conflict with fs_reclaim trying to allocate memory while
+* doing rpm_resume().
+*/
+   if (!intel_gt_pm_get_if_awake(gt))
+   return NULL;
+
+   intel_engine_pm_get(ce->engine);
+
+   return ce;
+}
+
+static void gen8_ggtt_bind_put_ce(struct intel_context *ce)
+{
+   intel_engine_pm_put(ce->engine);
+   intel_gt_pm_put(ce->engine->gt);
+}
+
+static bool gen8_ggtt_bind_ptes(struct i915_ggtt *ggtt, u32 offset,
+   struct sg_table *pages, u32 num_entries,
+   const gen8_pte_t pte)
+{
+   struct i915_sched_attr attr = {};
+   struct intel_gt *gt = ggtt->vm.gt;
+   const gen8_pte_t scratch_pte = ggtt->vm.scratch[0]->encode;
+   struct sgt_iter iter;
+   struct i915_request *rq;
+   struct intel_context *ce;
+   u32 *cs;
+
+   if (!num_entries)
+   return true;
+
+   ce = gen8_ggtt_bind_get_ce(ggtt);
+   if (!ce)
+   return false;
+
+   if (pages)
+   iter = __sgt_iter(pages->sgl, true);
+
+   while (num_entries) {
+   int count = 0;
+   dma_addr_t addr;
+   /*
+* MI_UPDATE_GTT can update 512 entries in a single command but
+* that end up with engine reset, 511 works.
+*/
+   u32 n_ptes = min_t(u32, 511, num_entries);
+
+   if (mutex_lock_interruptible(&ce->timeline->mutex))
+   goto put_ce;
+
+   intel_context_enter(ce);
+   rq = __i915_request_create(ce, GFP_NOWAIT | GFP_ATOMIC);
+   intel_context_exit(ce);
+   if (IS_ERR(rq)) {
+   GT_TRACE(gt, "Failed to get bind request\n");
+   mutex_unlock(&ce->timeline->mutex);
+   goto put_ce;
+   }
+
+   cs = intel_ring_begin(rq, 2 * n_ptes + 2);
+   if (IS_ERR(cs)) {
+   GT_TRACE(gt, "Failed to ring space for GGTT bind\n");
+   i915_request_set_error_once(rq, PTR_ERR(cs));
+   /* once a request is created, it must be queued */
+   goto queue_err_rq;
+   }
+
+   *cs++ = MI_UPDATE_GTT | (2 * n_ptes);
+   *cs++ = offset << 12;
+
+   if (pages) {
+   for_each_sgt_daddr_next(addr, iter) {
+   if (count == n_ptes)
+   break;
+   *cs++ = lower_32_bits(pt

[Intel-gfx] [PATCH 7/7] drm/i915: Enable GGTT updates with binder in MTL

2023-09-18 Thread Nirmoy Das
MTL can hang because of a HW bug while parallel reading/writing
from/to LMEM/GTTMMADR BAR so try to reduce GGTT update
related pci transactions with blitter command as recommended
for Wa_13010847436 and Wa_14019519902.

Signed-off-by: Nirmoy Das 
Reviewed-by: Oak Zeng 
---
 drivers/gpu/drm/i915/gt/intel_gtt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c 
b/drivers/gpu/drm/i915/gt/intel_gtt.c
index 4c89eb8d9af7..4fbed27ef0ec 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -23,7 +23,8 @@
 
 bool i915_ggtt_require_binder(struct drm_i915_private *i915)
 {
-   return false;
+   /* Wa_13010847436 & Wa_14019519902 */
+   return MEDIA_VER_FULL(i915) == IP_VER(13, 0);
 }
 
 static bool intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *i915)
-- 
2.41.0



[Intel-gfx] [PATCH 6/7] drm/i915: Toggle binder context ready status

2023-09-18 Thread Nirmoy Das
Toggle binder context ready status when needed.

To issue gpu commands, the driver must be primed to receive
requests. Maintain binder-based GGTT update disablement until driver
probing completes. Moreover, implement a temporary disablement
of blitter prior to entering suspend, followed by re-enablement
post-resume. This is acceptable as those transition periods are
mostly single threaded.

Signed-off-by: Nirmoy Das 
Signed-off-by: Oak Zeng 
---
 drivers/gpu/drm/i915/i915_driver.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_driver.c 
b/drivers/gpu/drm/i915/i915_driver.c
index e5d693904123..07e95e9ab504 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -817,6 +817,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct 
pci_device_id *ent)
i915_welcome_messages(i915);
 
i915->do_release = true;
+   intel_gt_bind_context_set_ready(to_gt(i915), true);
 
return 0;
 
@@ -857,6 +858,7 @@ void i915_driver_remove(struct drm_i915_private *i915)
 {
intel_wakeref_t wakeref;
 
+   intel_gt_bind_context_set_ready(to_gt(i915), false);
wakeref = intel_runtime_pm_get(&i915->runtime_pm);
 
i915_driver_unregister(i915);
@@ -1079,6 +1081,8 @@ static int i915_drm_suspend(struct drm_device *dev)
struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
pci_power_t opregion_target_state;
 
+   intel_gt_bind_context_set_ready(to_gt(dev_priv), false);
+
disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
 
/* We do a lot of poking in a lot of registers, make sure they work
@@ -1266,6 +1270,7 @@ static int i915_drm_resume(struct drm_device *dev)
intel_gvt_resume(dev_priv);
 
enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
+   intel_gt_bind_context_set_ready(to_gt(dev_priv), true);
 
return 0;
 }
-- 
2.41.0



[Intel-gfx] [PATCH 3/7] drm/i915: Implement for_each_sgt_daddr_next

2023-09-18 Thread Nirmoy Das
Implement a way to iterate over sgt with pre-initialized
sgt_iter state.

Signed-off-by: Nirmoy Das 
Reviewed-by: Oak Zeng 
---
 drivers/gpu/drm/i915/gt/intel_gtt.h |  3 +++
 drivers/gpu/drm/i915/i915_scatterlist.h | 10 ++
 2 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h 
b/drivers/gpu/drm/i915/gt/intel_gtt.h
index 346ec8ec2edd..41e530d0a4e9 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
@@ -171,6 +171,9 @@ struct intel_gt;
 #define for_each_sgt_daddr(__dp, __iter, __sgt) \
__for_each_sgt_daddr(__dp, __iter, __sgt, I915_GTT_PAGE_SIZE)
 
+#define for_each_sgt_daddr_next(__dp, __iter) \
+   __for_each_daddr_next(__dp, __iter, I915_GTT_PAGE_SIZE)
+
 struct i915_page_table {
struct drm_i915_gem_object *base;
union {
diff --git a/drivers/gpu/drm/i915/i915_scatterlist.h 
b/drivers/gpu/drm/i915/i915_scatterlist.h
index 5a10c1a31183..6cf8a298849f 100644
--- a/drivers/gpu/drm/i915/i915_scatterlist.h
+++ b/drivers/gpu/drm/i915/i915_scatterlist.h
@@ -91,6 +91,16 @@ static inline struct scatterlist *__sg_next(struct 
scatterlist *sg)
 ((__dp) = (__iter).dma + (__iter).curr), (__iter).sgp; \
 (((__iter).curr += (__step)) >= (__iter).max) ?\
 (__iter) = __sgt_iter(__sg_next((__iter).sgp), true), 0 : 0)
+/**
+ * __for_each_daddr_next - iterates over the device addresses with 
pre-initialized iterator.
+ * @__dp:  Device address (output)
+ * @__iter:'struct sgt_iter' (iterator state, external)
+ * @__step:step size
+ */
+#define __for_each_daddr_next(__dp, __iter, __step)  \
+   for (; ((__dp) = (__iter).dma + (__iter).curr), (__iter).sgp;   \
+(((__iter).curr += (__step)) >= (__iter).max) ?\
+(__iter) = __sgt_iter(__sg_next((__iter).sgp), true), 0 : 0)
 
 /**
  * for_each_sgt_page - iterate over the pages of the given sg_table
-- 
2.41.0



[Intel-gfx] [PATCH 2/7] drm/i915: Create a kernel context for GGTT updates

2023-09-18 Thread Nirmoy Das
Create a separate kernel context if a platform requires
GGTT updates using MI_UPDATE_GTT blitter command.

Subsequent patch will introduce methods to update
GGTT using this bind context and MI_UPDATE_GTT blitter
command.

Signed-off-by: Nirmoy Das 
Reviewed-by: Oak Zeng 
---
 drivers/gpu/drm/i915/gt/intel_engine.h   |  2 ++
 drivers/gpu/drm/i915/gt/intel_engine_cs.c| 35 +++-
 drivers/gpu/drm/i915/gt/intel_engine_types.h |  3 ++
 drivers/gpu/drm/i915/gt/intel_gt.c   | 18 ++
 drivers/gpu/drm/i915/gt/intel_gt.h   |  2 ++
 5 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h 
b/drivers/gpu/drm/i915/gt/intel_engine.h
index b58c30ac8ef0..40269e4c1e31 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -170,6 +170,8 @@ intel_write_status_page(struct intel_engine_cs *engine, int 
reg, u32 value)
 #define I915_GEM_HWS_SEQNO 0x40
 #define I915_GEM_HWS_SEQNO_ADDR(I915_GEM_HWS_SEQNO * 
sizeof(u32))
 #define I915_GEM_HWS_MIGRATE   (0x42 * sizeof(u32))
+#define I915_GEM_HWS_GGTT_BIND 0x46
+#define I915_GEM_HWS_GGTT_BIND_ADDR(I915_GEM_HWS_GGTT_BIND * sizeof(u32))
 #define I915_GEM_HWS_PXP   0x60
 #define I915_GEM_HWS_PXP_ADDR  (I915_GEM_HWS_PXP * sizeof(u32))
 #define I915_GEM_HWS_GSC   0x62
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c 
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 84a75c95f3f7..0a58fe812ec5 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1418,6 +1418,20 @@ void intel_engine_destroy_pinned_context(struct 
intel_context *ce)
intel_context_put(ce);
 }
 
+static struct intel_context *
+create_ggtt_bind_context(struct intel_engine_cs *engine)
+{
+   static struct lock_class_key kernel;
+
+   /*
+* MI_UPDATE_GTT can insert up to 512 PTE entries and there could be 
multiple
+* bind requets at a time so get a bigger ring.
+*/
+   return intel_engine_create_pinned_context(engine, engine->gt->vm, 
SZ_512K,
+ I915_GEM_HWS_GGTT_BIND_ADDR,
+ &kernel, "ggtt_bind_context");
+}
+
 static struct intel_context *
 create_kernel_context(struct intel_engine_cs *engine)
 {
@@ -1441,7 +1455,7 @@ create_kernel_context(struct intel_engine_cs *engine)
  */
 static int engine_init_common(struct intel_engine_cs *engine)
 {
-   struct intel_context *ce;
+   struct intel_context *ce, *bce = NULL;
int ret;
 
engine->set_default_submission(engine);
@@ -1457,6 +1471,19 @@ static int engine_init_common(struct intel_engine_cs 
*engine)
ce = create_kernel_context(engine);
if (IS_ERR(ce))
return PTR_ERR(ce);
+   /*
+* Create a separate pinned context for GGTT update with blitter engine
+* if a platform require such service. MI_UPDATE_GTT works on other
+* engines as well but BCS should be less busy engine so pick that for
+* GGTT updates.
+*/
+   if (engine->id == BCS0) {
+   bce = create_ggtt_bind_context(engine);
+   if (IS_ERR(bce)) {
+   intel_engine_destroy_pinned_context(ce);
+   return PTR_ERR(bce);
+   }
+   }
 
ret = measure_breadcrumb_dw(ce);
if (ret < 0)
@@ -1464,11 +1491,14 @@ static int engine_init_common(struct intel_engine_cs 
*engine)
 
engine->emit_fini_breadcrumb_dw = ret;
engine->kernel_context = ce;
+   engine->bind_context = bce;
 
return 0;
 
 err_context:
intel_engine_destroy_pinned_context(ce);
+   if (bce)
+   intel_engine_destroy_pinned_context(ce);
return ret;
 }
 
@@ -1536,6 +1566,9 @@ void intel_engine_cleanup_common(struct intel_engine_cs 
*engine)
 
if (engine->kernel_context)
intel_engine_destroy_pinned_context(engine->kernel_context);
+   if (engine->bind_context)
+   intel_engine_destroy_pinned_context(engine->bind_context);
+
 
GEM_BUG_ON(!llist_empty(&engine->barrier_tasks));
cleanup_status_page(engine);
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h 
b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index a7e677598004..a8f527fab0f0 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -416,6 +416,9 @@ struct intel_engine_cs {
struct llist_head barrier_tasks;
 
struct intel_context *kernel_context; /* pinned */
+   struct intel_context *bind_context; /* pinned, only for BCS0 */
+   /* mark the bind context's availability status */
+   bool bind_context_ready;
 
/**
 * pinned_contexts_list: List of pinned contexts. This list is only
diff --git a/drivers/gpu/

[Intel-gfx] [PATCH 1/7] drm/i915: Lift runtime-pm acquire callbacks out of intel_wakeref.mutex

2023-09-18 Thread Nirmoy Das
From: Chris Wilson 

When runtime pm is first woken, it will synchronously call the
registered callbacks for the device. These callbacks
may pull in their own forest of locks, which we do not want to
conflate with the intel_wakeref.mutex. A second minor benefit to
reducing the coverage of the mutex, is that it will reduce
contention for frequent sleeps and wakes (such as when being used
for soft-rc6).

Signed-off-by: Chris Wilson 
Signed-off-by: Nirmoy Das 
Reviewed-by: Andi Shyti 
---
 drivers/gpu/drm/i915/intel_wakeref.c | 43 ++--
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_wakeref.c 
b/drivers/gpu/drm/i915/intel_wakeref.c
index 718f2f1b6174..af7b4cb5b4d7 100644
--- a/drivers/gpu/drm/i915/intel_wakeref.c
+++ b/drivers/gpu/drm/i915/intel_wakeref.c
@@ -10,21 +10,11 @@
 #include "intel_wakeref.h"
 #include "i915_drv.h"
 
-static void rpm_get(struct intel_wakeref *wf)
-{
-   wf->wakeref = intel_runtime_pm_get(&wf->i915->runtime_pm);
-}
-
-static void rpm_put(struct intel_wakeref *wf)
-{
-   intel_wakeref_t wakeref = fetch_and_zero(&wf->wakeref);
-
-   intel_runtime_pm_put(&wf->i915->runtime_pm, wakeref);
-   INTEL_WAKEREF_BUG_ON(!wakeref);
-}
-
 int __intel_wakeref_get_first(struct intel_wakeref *wf)
 {
+   intel_wakeref_t wakeref = intel_runtime_pm_get(&wf->i915->runtime_pm);
+   int err = 0;
+
/*
 * Treat get/put as different subclasses, as we may need to run
 * the put callback from under the shrinker and do not want to
@@ -32,41 +22,50 @@ int __intel_wakeref_get_first(struct intel_wakeref *wf)
 * upon acquiring the wakeref.
 */
mutex_lock_nested(&wf->mutex, SINGLE_DEPTH_NESTING);
-   if (!atomic_read(&wf->count)) {
-   int err;
 
-   rpm_get(wf);
+   if (likely(!atomic_read(&wf->count))) {
+   INTEL_WAKEREF_BUG_ON(wf->wakeref);
+   wf->wakeref = fetch_and_zero(&wakeref);
 
err = wf->ops->get(wf);
if (unlikely(err)) {
-   rpm_put(wf);
-   mutex_unlock(&wf->mutex);
-   return err;
+   wakeref = xchg(&wf->wakeref, 0);
+   wake_up_var(&wf->wakeref);
+   goto unlock;
}
 
smp_mb__before_atomic(); /* release wf->count */
}
atomic_inc(&wf->count);
+   INTEL_WAKEREF_BUG_ON(atomic_read(&wf->count) <= 0);
+
+unlock:
mutex_unlock(&wf->mutex);
+   if (unlikely(wakeref))
+   intel_runtime_pm_put(&wf->i915->runtime_pm, wakeref);
 
-   INTEL_WAKEREF_BUG_ON(atomic_read(&wf->count) <= 0);
-   return 0;
+   return err;
 }
 
 static void intel_wakeref_put_last(struct intel_wakeref *wf)
 {
+   intel_wakeref_t wakeref = 0;
+
INTEL_WAKEREF_BUG_ON(atomic_read(&wf->count) <= 0);
if (unlikely(!atomic_dec_and_test(&wf->count)))
goto unlock;
 
/* ops->put() must reschedule its own release on error/deferral */
if (likely(!wf->ops->put(wf))) {
-   rpm_put(wf);
+   INTEL_WAKEREF_BUG_ON(!wf->wakeref);
+   wakeref = xchg(&wf->wakeref, 0);
wake_up_var(&wf->wakeref);
}
 
 unlock:
mutex_unlock(&wf->mutex);
+   if (wakeref)
+   intel_runtime_pm_put(&wf->i915->runtime_pm, wakeref);
 }
 
 void __intel_wakeref_put_last(struct intel_wakeref *wf, unsigned long flags)
-- 
2.41.0



[Intel-gfx] [PATCH 0/7] Update GGTT with MI_UPDATE_GTT on MTL

2023-09-18 Thread Nirmoy Das
Implement a way to update GGTT using MI_UPDATE_GTT command 
when possible for MTL as a suggested work around for HW bugs,
Wa_13010847436 and Wa_14019519902.

v2: Fix lockdep warning related to GT wakeref vs 
blitter engine wakeref.

v3: Rearrange patches/diffs to fix code mixups(Andi)

v4: fix ce leak(Oak)

v5: rebase & resend with new "Test-with"

Test-with: 20230918164747.30171-1-nirmoy@intel.com

Chris Wilson (1):
  drm/i915: Lift runtime-pm acquire callbacks out of intel_wakeref.mutex

Nirmoy Das (6):
  drm/i915: Create a kernel context for GGTT updates
  drm/i915: Implement for_each_sgt_daddr_next
  drm/i915: Parameterize binder context creation
  drm/i915: Implement GGTT update method with MI_UPDATE_GTT
  drm/i915: Toggle binder context ready status
  drm/i915: Enable GGTT updates with binder in MTL

 drivers/gpu/drm/i915/gt/intel_engine.h   |   2 +
 drivers/gpu/drm/i915/gt/intel_engine_cs.c|  35 ++-
 drivers/gpu/drm/i915/gt/intel_engine_types.h |   3 +
 drivers/gpu/drm/i915/gt/intel_ggtt.c | 235 +++
 drivers/gpu/drm/i915/gt/intel_gt.c   |  18 ++
 drivers/gpu/drm/i915/gt/intel_gt.h   |   2 +
 drivers/gpu/drm/i915/gt/intel_gtt.c  |   5 +
 drivers/gpu/drm/i915/gt/intel_gtt.h  |   5 +
 drivers/gpu/drm/i915/i915_driver.c   |   5 +
 drivers/gpu/drm/i915/i915_scatterlist.h  |  10 +
 drivers/gpu/drm/i915/intel_wakeref.c |  43 ++--
 11 files changed, 340 insertions(+), 23 deletions(-)

-- 
2.41.0



Re: [Intel-gfx] [PATCH] drm/i915/gt: skip WA verfication for GEN7_MISCCPCTL on DG2

2023-09-18 Thread Andi Shyti
Hi Andrzej,

On Tue, Sep 12, 2023 at 09:35:21AM +0200, Andrzej Hajda wrote:
> Some DG2 firmware locks this register for modification. Using wa_add
> with read_mask 0 allows to skip checks of such registers.
> 
> Signed-off-by: Andrzej Hajda 

Reviewed-by: Andi Shyti  

Andi

> ---
>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c 
> b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 70071ead0659cc..1d1474ad945e0c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -1597,8 +1597,11 @@ dg2_gt_workarounds_init(struct intel_gt *gt, struct 
> i915_wa_list *wal)
>   /* Wa_14014830051:dg2 */
>   wa_mcr_write_clr(wal, SARB_CHICKEN1, COMP_CKN_IN);
>  
> - /* Wa_14015795083 */
> - wa_write_clr(wal, GEN7_MISCCPCTL, GEN12_DOP_CLOCK_GATE_RENDER_ENABLE);
> + /* Wa_14015795083
> +  * Skip verification for possibly locked register.
> +  */
> + wa_add(wal, GEN7_MISCCPCTL, GEN12_DOP_CLOCK_GATE_RENDER_ENABLE,
> +0, 0, false);
>  
>   /* Wa_18018781329 */
>   wa_mcr_write_or(wal, RENDER_MOD_CTRL, FORCE_MISS_FTLB);
> -- 
> 2.34.1


Re: [Intel-gfx] [PATCH 5/7] drm/i915: Implement GGTT update method with MI_UPDATE_GTT

2023-09-18 Thread Zeng, Oak


Thanks,
Oak

> -Original Message-
> From: Das, Nirmoy 
> Sent: Monday, September 18, 2023 6:26 AM
> To: Zeng, Oak ; intel-gfx@lists.freedesktop.org;
> Piorkowski, Piotr 
> Cc: chris.p.wil...@linux.intel.com; Shyti, Andi ; Mun,
> Gwan-gyeong ; Roper, Matthew D
> 
> Subject: Re: [PATCH 5/7] drm/i915: Implement GGTT update method with
> MI_UPDATE_GTT
> 
> 
> On 9/15/2023 5:56 PM, Zeng, Oak wrote:
> >
> > Thanks,
> > Oak
> >
> >> -Original Message-
> >> From: Das, Nirmoy 
> >> Sent: Friday, September 15, 2023 4:34 AM
> >> To: intel-gfx@lists.freedesktop.org
> >> Cc: Zeng, Oak ; chris.p.wil...@linux.intel.com;
> Piorkowski,
> >> Piotr ; Shyti, Andi ; 
> >> Mun,
> >> Gwan-gyeong ; Roper, Matthew D
> >> ; Das, Nirmoy 
> >> Subject: [PATCH 5/7] drm/i915: Implement GGTT update method with
> >> MI_UPDATE_GTT
> >>
> >> Implement GGTT update method with blitter command, MI_UPDATE_GTT
> >> and install those handlers if a platform requires that.
> >>
> >> v2: Make sure we hold the GT wakeref and Blitter engine wakeref before
> >> we call mutex_lock/intel_context_enter below. When GT/engine are not
> >> awake, the intel_context_enter calls into some runtime pm function which
> >> can end up with kmalloc/fs_reclaim. But trigger fs_reclaim holding a
> >> mutex lock is not allowed because shrinker can also try to hold the same
> >> mutex lock. It is a circular lock. So hold the GT/blitter engine wakeref
> >> before calling mutex_lock, to fix the circular lock.
> >>
> >> Signed-off-by: Nirmoy Das 
> >> Signed-off-by: Oak Zeng 
> >> ---
> >>   drivers/gpu/drm/i915/gt/intel_ggtt.c | 235
> +++
> >>   1 file changed, 235 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> >> b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> >> index dd0ed941441a..b94de7cebfce 100644
> >> --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> >> +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> >> @@ -15,18 +15,23 @@
> >>   #include "display/intel_display.h"
> >>   #include "gem/i915_gem_lmem.h"
> >>
> >> +#include "intel_context.h"
> >>   #include "intel_ggtt_gmch.h"
> >> +#include "intel_gpu_commands.h"
> >>   #include "intel_gt.h"
> >>   #include "intel_gt_regs.h"
> >>   #include "intel_pci_config.h"
> >> +#include "intel_ring.h"
> >>   #include "i915_drv.h"
> >>   #include "i915_pci.h"
> >> +#include "i915_request.h"
> >>   #include "i915_scatterlist.h"
> >>   #include "i915_utils.h"
> >>   #include "i915_vgpu.h"
> >>
> >>   #include "intel_gtt.h"
> >>   #include "gen8_ppgtt.h"
> >> +#include "intel_engine_pm.h"
> >>
> >>   static void i915_ggtt_color_adjust(const struct drm_mm_node *node,
> >>   unsigned long color,
> >> @@ -252,6 +257,145 @@ u64 gen8_ggtt_pte_encode(dma_addr_t addr,
> >>return pte;
> >>   }
> >>
> >> +static bool should_update_ggtt_with_bind(struct i915_ggtt *ggtt)
> >> +{
> >> +  struct intel_gt *gt = ggtt->vm.gt;
> >> +
> >> +  return intel_gt_is_bind_context_ready(gt);
> >> +}
> >> +
> >> +static struct intel_context *gen8_ggtt_bind_get_ce(struct i915_ggtt *ggtt)
> >> +{
> >> +  struct intel_context *ce;
> >> +  struct intel_gt *gt = ggtt->vm.gt;
> >> +
> >> +  if (intel_gt_is_wedged(gt))
> >> +  return NULL;
> >> +
> >> +  ce = gt->engine[BCS0]->bind_context;
> >> +  GEM_BUG_ON(!ce);
> >> +
> >> +  /*
> >> +   * If the GT is not awake already at this stage then fallback
> >> +   * to pci based GGTT update otherwise __intel_wakeref_get_first()
> >> +   * would conflict with fs_reclaim trying to allocate memory while
> >> +   * doing rpm_resume().
> >> +   */
> >> +  if (!intel_gt_pm_get_if_awake(gt))
> >> +  return NULL;
> >> +
> >> +  intel_engine_pm_get(ce->engine);
> >> +
> >> +  return ce;
> >> +}
> >> +
> >> +static void gen8_ggtt_bind_put_ce(struct intel_context *ce)
> >> +{
> >> +  intel_engine_pm_put(ce->engine);
> >> +  intel_gt_pm_put(ce->engine->gt);
> >> +}
> >> +
> >> +static bool gen8_ggtt_bind_ptes(struct i915_ggtt *ggtt, u32 offset,
> >> +  struct sg_table *pages, u32 num_entries,
> >> +  const gen8_pte_t pte)
> >> +{
> >> +  struct i915_sched_attr attr = {};
> >> +  struct intel_gt *gt = ggtt->vm.gt;
> >> +  const gen8_pte_t scratch_pte = ggtt->vm.scratch[0]->encode;
> >> +  struct sgt_iter iter;
> >> +  struct i915_request *rq;
> >> +  struct intel_context *ce;
> >> +  u32 *cs;
> >> +
> >> +  if (!num_entries)
> >> +  return true;
> >> +
> >> +  ce = gen8_ggtt_bind_get_ce(ggtt);
> >> +  if (!ce)
> >> +  return false;
> >> +
> >> +  if (pages)
> >> +  iter = __sgt_iter(pages->sgl, true);
> >> +
> >> +  while (num_entries) {
> >> +  int count = 0;
> >> +  dma_addr_t addr;
> >> +  /*
> >> +   * MI_UPDATE_GTT can update 512 entries in a single command
> >> but
> >> +   * that end up with engine reset, 511 works.
> >> +   */
> >> +  u32 n_ptes = min_t(u32, 511, num_entries);
> >> +
> >> +  

Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [1/4] drm/dp_mst: Fix NULL dereference during payload addition

2023-09-18 Thread Imre Deak
On Thu, Sep 14, 2023 at 06:01:49AM +, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [1/4] drm/dp_mst: Fix NULL dereference during 
> payload addition
> URL   : https://patchwork.freedesktop.org/series/123652/
> State : failure

Thanks for the reviews, patches 1-3 are pushed to drm-misc-next and
patch 4 to drm-intel-next.

All the failures below are unrelated, since none of the affected
platforms has MST sinks.

> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_13627_full -> Patchwork_123652v1_full
> 
> 
> Summary
> ---
> 
>   **FAILURE**
> 
>   Serious unknown changes coming with Patchwork_123652v1_full absolutely need 
> to be
>   verified manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in Patchwork_123652v1_full, please notify your bug team 
> (lgci.bug.fil...@intel.com) to allow them
>   to document this new failure mode, which will reduce false positives in CI.
> 
>   
> 
> Participating hosts (9 -> 10)
> --
> 
>   Additional (1): shard-rkl0 
> 
> Possible new issues
> ---
> 
>   Here are the unknown changes that may have been introduced in 
> Patchwork_123652v1_full:
> 
> ### IGT changes ###
> 
>  Possible regressions 
> 
>   * igt@gen9_exec_parse@allowed-all:
> - shard-apl:  [PASS][1] -> [INCOMPLETE][2]
>[1]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13627/shard-apl3/igt@gen9_exec_pa...@allowed-all.html
>[2]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123652v1/shard-apl6/igt@gen9_exec_pa...@allowed-all.html
> 
>   * igt@gen9_exec_parse@allowed-single:
> - shard-glk:  [PASS][3] -> [INCOMPLETE][4]
>[3]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13627/shard-glk7/igt@gen9_exec_pa...@allowed-single.html
>[4]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123652v1/shard-glk6/igt@gen9_exec_pa...@allowed-single.html
> 
>   * igt@kms_plane_cursor@overlay@pipe-b-edp-1-size-64:
> - shard-mtlp: [PASS][5] -> [FAIL][6]
>[5]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13627/shard-mtlp-6/igt@kms_plane_cursor@over...@pipe-b-edp-1-size-64.html
>[6]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123652v1/shard-mtlp-5/igt@kms_plane_cursor@over...@pipe-b-edp-1-size-64.html
> 
>   * igt@kms_rotation_crc@sprite-rotation-90:
> - shard-rkl:  [PASS][7] -> [INCOMPLETE][8] +1 other test 
> incomplete
>[7]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13627/shard-rkl-4/igt@kms_rotation_...@sprite-rotation-90.html
>[8]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123652v1/shard-rkl-7/igt@kms_rotation_...@sprite-rotation-90.html
> 
>   * igt@perf_pmu@rc6-all-gts:
> - shard-mtlp: NOTRUN -> [ABORT][9]
>[9]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123652v1/shard-mtlp-6/igt@perf_...@rc6-all-gts.html
> 
>   
>  Suppressed 
> 
>   The following results come from untrusted machines, tests, or statuses.
>   They do not affect the overall result.
> 
>   * {igt@kms_feature_discovery@display-4x}:
> - shard-mtlp: NOTRUN -> [SKIP][10]
>[10]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123652v1/shard-mtlp-6/igt@kms_feature_discov...@display-4x.html
> 
>   * {igt@kms_feature_discovery@dp-mst}:
> - shard-dg2:  NOTRUN -> [SKIP][11]
>[11]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123652v1/shard-dg2-11/igt@kms_feature_discov...@dp-mst.html
> 
>   
> New tests
> -
> 
>   New tests have been introduced between CI_DRM_13627_full and 
> Patchwork_123652v1_full:
> 
> ### New IGT tests (4) ###
> 
>   * 
> igt@kms_atomic_transition@plane-all-transition-nonblocking@pipe-a-hdmi-a-4:
> - Statuses : 1 pass(s)
> - Exec time: [0.0] s
> 
>   * 
> igt@kms_atomic_transition@plane-all-transition-nonblocking@pipe-b-hdmi-a-4:
> - Statuses : 1 pass(s)
> - Exec time: [0.0] s
> 
>   * igt@kms_cursor_crc@cursor-onscreen-128x128@pipe-a-hdmi-a-4:
> - Statuses : 1 pass(s)
> - Exec time: [0.0] s
> 
>   * igt@kms_cursor_crc@cursor-onscreen-128x128@pipe-d-hdmi-a-4:
> - Statuses : 1 pass(s)
> - Exec time: [0.0] s
> 
>   
> 
> Known issues
> 
> 
>   Here are the changes found in Patchwork_123652v1_full that come from known 
> issues:
> 
> ### IGT changes ###
> 
>  Issues hit 
> 
>   * igt@drm_fdinfo@most-busy-check-all@bcs0:
> - shard-dg2:  NOTRUN -> [SKIP][12] ([i915#8414]) +21 other tests 
> skip
>[12]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123652v1/shard-dg2-7/igt@drm_fdinfo@most-busy-check-...@bcs0.html
> 
>   * igt@drm_fdinfo@virtual-busy-hang:
> - shard-mtlp: NOTRUN -> [SKIP][13] ([i915#8414]) +1 other test 
> skip
>[13]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123652v1/shard-mtlp-1/igt@drm_fdi...@virtual-busy-hang.ht

[Intel-gfx] [PATCH i-g-t v3 17/17] lib/kunit: Omit suite name prefix if the same as subtest name

2023-09-18 Thread Janusz Krzysztofik
Kunit test modules usually contain one test suite, named after the module
name with the trailing "_test" or "_kunit" suffix omitted.  Since we
follow the same convention when we derive subtest names from module names,
there is a great chance that those two names match.  Take this into
account when composing names for IGT dynamic sub-subtest names and drop
the leading test suite name component when it is the same as subtest name.

Signed-off-by: Janusz Krzysztofik 
---
 lib/igt_kmod.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index f6e0ab07ce..05c837031c 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -863,7 +863,8 @@ static void kunit_result_free(struct igt_ktap_result *r,
  *
  * Returns: IGT default codes
  */
-static void __igt_kunit(struct igt_ktest *tst, const char *opts)
+static void
+__igt_kunit(struct igt_ktest *tst, const char *name, const char *opts)
 {
struct modprobe_data modprobe = { pthread_self(), tst->kmod, opts, 0,
  PTHREAD_MUTEX_INITIALIZER, };
@@ -902,7 +903,11 @@ static void __igt_kunit(struct igt_ktest *tst, const char 
*opts)
 
r = igt_list_first_entry(&results, r, link);
 
-   igt_dynamic_f("%s-%s", r->suite_name, r->case_name) {
+   igt_dynamic_f("%s%s%s",
+ strcmp(r->suite_name, name) ?  r->suite_name : "",
+ strcmp(r->suite_name, name) ? "-" : "",
+ r->case_name) {
+
if (r->code == IGT_EXIT_INVALID) {
/* parametrized test case, get actual result */
kunit_result_free(r, &suite_name, &case_name);
@@ -1011,7 +1016,7 @@ void igt_kunit(const char *module_name, const char *name, 
const char *opts)
 * and for documentation.
 */
igt_subtest_with_dynamic(name)
-   __igt_kunit(&tst, opts);
+   __igt_kunit(&tst, name, opts);
 
igt_fixture
igt_ktest_end(&tst);
-- 
2.41.0



[Intel-gfx] [PATCH i-g-t v3 16/17] lib/kunit: Strip "_test" or "_kunit" suffix from subtest names

2023-09-18 Thread Janusz Krzysztofik
If a user (an IGT test) doesn't provide a subtest name when calling
igt_kunit() then we now use the requested kernel module name as IGT
subtest name.  Since names of kunit test modules usually end with a
"_test" or "_kunit" suffix, those parts of the names don't carry any
useful information.  Strip those suffixes from IGT subtest names.

Signed-off-by: Janusz Krzysztofik 
Reviewed-by: Mauro Carvalho Chehab 
---
 lib/igt_kmod.c | 23 ---
 tests/drm_mm.c | 42 +-
 2 files changed, 41 insertions(+), 24 deletions(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 7fa5b4aa80..f6e0ab07ce 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -978,7 +978,27 @@ void igt_kunit(const char *module_name, const char *name, 
const char *opts)
struct igt_ktest tst = { .kmsg = -1, };
 
 
+   /*
+* If the caller (an IGT test) provides no subtest name then we
+* take the module name, drop the trailing "_test" or "_kunit"
+* suffix, if any, and use the result as our IGT subtest name.
+*/
+   if (!name) {
+   name = strdup(module_name);
+   if (name) {
+   char *suffix = strstr(name, "_test");
+
+   if (!suffix)
+   suffix = strstr(name, "_kunit");
+
+   if (suffix)
+   *suffix = '\0';
+   }
+   }
+
igt_fixture {
+   igt_require(name);
+
igt_skip_on(igt_ktest_init(&tst, module_name));
igt_skip_on(igt_ktest_begin(&tst));
}
@@ -990,9 +1010,6 @@ void igt_kunit(const char *module_name, const char *name, 
const char *opts)
 * proper namespace for dynamic subtests, with is required for CI
 * and for documentation.
 */
-   if (name == NULL)
-   name = module_name;
-
igt_subtest_with_dynamic(name)
__igt_kunit(&tst, opts);
 
diff --git a/tests/drm_mm.c b/tests/drm_mm.c
index 9a8b3f3fcb..e6ba224745 100644
--- a/tests/drm_mm.c
+++ b/tests/drm_mm.c
@@ -29,123 +29,123 @@
  * Feature: mapping
  * Run type: FULL
  *
- * SUBTEST: drm_mm_test
+ * SUBTEST: drm_mm
  *
- * SUBTEST: drm_mm_test@align
+ * SUBTEST: drm_mm@align
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: drm_mm_test@align32
+ * SUBTEST: drm_mm@align32
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: drm_mm_test@align64
+ * SUBTEST: drm_mm@align64
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: drm_mm_test@bottomup
+ * SUBTEST: drm_mm@bottomup
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: drm_mm_test@color
+ * SUBTEST: drm_mm@color
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: drm_mm_test@color_evict
+ * SUBTEST: drm_mm@color_evict
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: drm_mm_test@color_evict_range
+ * SUBTEST: drm_mm@color_evict_range
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: drm_mm_test@debug
+ * SUBTEST: drm_mm@debug
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: drm_mm_test@evict
+ * SUBTEST: drm_mm@evict
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: drm_mm_test@evict_range
+ * SUBTEST: drm_mm@evict_range
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: drm_mm_test@frag
+ * SUBTEST: drm_mm@frag
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: drm_mm_test@highest
+ * SUBTEST: drm_mm@highest
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: drm_mm_test@init
+ * SUBTEST: drm_mm@init
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality:

[Intel-gfx] [PATCH i-g-t v3 15/17] lib/kunit: Parse KTAP report from the main process thread

2023-09-18 Thread Janusz Krzysztofik
There was an attempt to parse KTAP reports in the background while a kunit
test module is loading.  However, since dynamic sub-subtests can be
executed only from the main thread, that attempt was not quite successful,
as IGT results from all executed kunit test cases were generated only
after loading of kunit test module completed.

Now that the parser maintains its state and we can call it separately for
each input line of a KTAP report, it is perfectly possible to call the
parser from the main thread while the module is loading in the background,
and convert results from kunit test cases immediately to results of IGT
dynamic sub-subtests by running an igt_dynamic() section for each result
as soon as returned by the parser.

Drop igt_ktap_parser() thread and execute igt_dynamic() for each kunit
result obtained from igt_ktap_parse() called from the main thread.

Also, drop no longer needed functions from igt_ktap soruces.

v2: Interrupt blocking read() on modprobe failure.

Signed-off-by: Janusz Krzysztofik 
---
 lib/igt_kmod.c | 208 ++
 lib/igt_ktap.c | 568 -
 lib/igt_ktap.h |  22 --
 3 files changed, 166 insertions(+), 632 deletions(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 8fbd274ccf..7fa5b4aa80 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -1,5 +1,5 @@
 /*
- * Copyright © 2016 Intel Corporation
+ * Copyright © 2016-2023 Intel Corporation
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),
@@ -26,7 +26,10 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
+#include 
 
 #include "igt_aux.h"
 #include "igt_core.h"
@@ -748,20 +751,109 @@ void igt_kselftest_get_tests(struct kmod_module *kmod,
 }
 
 struct modprobe_data {
+   pthread_t parent;
struct kmod_module *kmod;
const char *opts;
int err;
+   pthread_mutex_t lock;
+   pthread_t thread;
 };
 
 static void *modprobe_task(void *arg)
 {
struct modprobe_data *data = arg;
+   int err;
+
+   err = modprobe(data->kmod, data->opts);
 
-   data->err = modprobe(data->kmod, data->opts);
+   if (err) {
+   while (pthread_mutex_trylock(&data->lock) == EBUSY)
+   pthread_kill(data->parent, SIGCHLD);
+
+   data->err = err;
+   pthread_mutex_unlock(&data->lock);
+   }
 
return NULL;
 }
 
+static void kunit_sigchld_handler(int signal)
+{
+   return;
+}
+
+static int kunit_kmsg_result_get(struct igt_list_head *results,
+struct modprobe_data *modprobe,
+int fd, struct igt_ktap_results *ktap)
+{
+   struct sigaction sigchld = { .sa_handler = kunit_sigchld_handler, };
+   char record[BUF_LEN + 1], *buf;
+   unsigned long taints;
+   int ret;
+
+   do {
+   if (igt_kernel_tainted(&taints))
+   return -ENOTRECOVERABLE;
+
+   pthread_mutex_lock(&modprobe->lock);
+   if (!pthread_tryjoin_np(modprobe->thread, NULL) &&
+   modprobe->err) {
+   pthread_mutex_unlock(&modprobe->lock);
+   return modprobe->err;
+   }
+
+   sigaction(SIGCHLD, &sigchld, NULL);
+   ret = read(fd, record, BUF_LEN);
+   sigaction(SIGCHLD, NULL, NULL);
+   pthread_mutex_unlock(&modprobe->lock);
+
+   if (!ret)
+   return -ENODATA;
+   if (ret == -1)
+   return -errno;
+
+   igt_assert_lt(0, ret);
+
+   /* skip kmsg continuation lines */
+   if (igt_debug_on(*record == ' '))
+   continue;
+
+   /* NULL-terminate the record */
+   record[ret] = '\0';
+
+   /* detect start of log message, continue if not found */
+   buf = strchrnul(record, ';');
+   if (igt_debug_on(*buf == '\0'))
+   continue;
+   buf++;
+
+   ret = igt_ktap_parse(buf, ktap);
+   if (ret != -EINPROGRESS)
+   break;
+   } while (igt_list_empty(results));
+
+   return ret;
+}
+
+static void kunit_result_free(struct igt_ktap_result *r,
+ char **suite_name, char **case_name)
+{
+   igt_list_del(&r->link);
+
+   if (r->suite_name != *suite_name) {
+   free(*suite_name);
+   *suite_name = r->suite_name;
+   }
+
+   if (r->case_name != *case_name) {
+   free(*case_name);
+   *case_name = r->case_name;
+   }
+
+   free(r->msg);
+   free(r);
+}
+
 /**
  * igt_kunit:
  * @module_name: the name of the module
@@ -773,11 +865,12 @@ static void *modprobe_task(void *arg)
  */
 static void __igt_kunit(struct i

[Intel-gfx] [PATCH i-g-t v3 13/17] lib/ktap: Reimplement KTAP parser

2023-09-18 Thread Janusz Krzysztofik
Current implementation of KTAP parser suffers from several issues:
- works only with built-in kunit, can't parse KTAP output if modular,
- in most cases, kernel messages that are not part of KTAP output but
  happen to appear in between break the parser,
- results from parametrized test cases, not preceded with a "1..N" test
  plan, break the parser,
- skips are not supported, reported as success,
- IGT results from all 3 kunit test nesting levels, i.e., from
  parametrized subtests (should those were fixed to work correctly), test
  cases and test suites, are reported individually as if all those items
  were executed sequentially, all at the same level of nesting, which can
  be confusing to igt_runner,
- subtest names mostly consist of kunit suite name and kunit test case
  name but not always, sometimes the first component is omited,
- the parser is not only parsing the data, but also handles data input
  from a /dev/kmsg like source, which is integrated into it, making it
  hard if not impossible to feed KTAP data from different sources,
  including mock sources,
- since the parser has been designed for running it in a separate thread,
  it's not possible to use igt_skip() nor igt_fail() and friends
  immediately when a result is available, only pass it to the main thread
  over a buffer.  As a consequence, it is virtually impossible to
  synchronize IGT output with KTAP output.

Fixing the existing parser occurred more complicated than re-implementing
it from scratch.  This patch provides a new implementation.

Only results from kunit test cases are reported as results of IGT dynamic
sub-subtests.  Results from individual parametrized subtests have been
considered problematic since many of them provide multi-word descriptions
in place of single-word subtest names.  If a parametrized test case fails
then full KTAP output from its subtests, potentially mixed with
accompanying kernel messages, is available in dmesg for analysis so users
can still find out which individual subtests succeeded and which failed.

Results from test suites level are also omitted in faith that IGT can
handle aggregation of results from individual kunit test cases reported as
IGT dynamic sub-subtests and report those aggregated results correctly as
results from an IGT dynamic subtest.  That 1:1 mapping of kunit test
suites to IGT dynamic subtests now works perfectly for modules that
provide only one test suite, which is the case for all kunit test modules
now existing under drivers/gpu/drm, and the most common case among all
kunit test modules in the whole kernel tree.

New igt_ktap functions can be called directly from igt_kunit subtest body,
but for this patch, the old igt_ktap_parser() function that runs in a
separate thread has been preserved, only modified to use the new
implementation and translate results from those new functions to legacy
format.  Unlike the former implementation, translation of kunit names to
IGT names is handled outside the parser itself, though for now it is still
performed inside the legacy igt_ktap_parser() function.

For better readability of the patch, no longer used functions have been
left untouched, only tagged with __maybe_unused to shut up compiler
warnings / errors.  Kunit library functions will be modified to use the
new igt_ktap interface, and those old ktap functions removed by follow-
up patches.

A test with example subtests that feed igt_ktap_parse() function with some
example data and verifies correctness of their parsing is also provided.

v2: Fix incorrect and missing includes in the test source file,
  - add license and copyright clauses to the test source file.

Signed-off-by: Janusz Krzysztofik 
Acked-by: Mauro Carvalho Chehab 
---
 lib/igt_ktap.c  | 422 
 lib/igt_ktap.h  |  15 ++
 lib/tests/igt_ktap_parser.c | 246 +
 lib/tests/meson.build   |   1 +
 4 files changed, 645 insertions(+), 39 deletions(-)
 create mode 100644 lib/tests/igt_ktap_parser.c

diff --git a/lib/igt_ktap.c b/lib/igt_ktap.c
index c64323d9b4..5eac102417 100644
--- a/lib/igt_ktap.c
+++ b/lib/igt_ktap.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: MIT
 /*
  * Copyright © 2023 Isabella Basso do Amaral 
+ * Copyright © 2023 Intel Corporation
  */
 
 #include 
@@ -8,12 +9,310 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
+#include 
 
 #include "igt_aux.h"
 #include "igt_core.h"
 #include "igt_ktap.h"
 #include "igt_list.h"
 
+enum ktap_phase {
+   KTAP_START,
+   SUITE_COUNT,
+   SUITE_START,
+   SUITE_NAME,
+   CASE_COUNT,
+   CASE_NAME,
+   SUB_RESULT,
+   CASE_RESULT,
+   SUITE_RESULT,
+};
+
+struct igt_ktap_results {
+   enum ktap_phase expect;
+   unsigned int suite_count;
+   unsigned int suite_last;
+   char *suite_name;
+   unsigned int case_count;
+   unsigned int case_last;
+   char *case_name;
+   unsigned int sub_last;
+   stru

[Intel-gfx] [PATCH i-g-t v3 12/17] lib/ktap: Use IGT linked lists for storing KTAP results

2023-09-18 Thread Janusz Krzysztofik
For code simplicity and clarity, use existing IGT linked lists library
instead of open coding a custom implementation of a list of KTAP results.

While being at it, flatten the code by inverting a check for pending
results.

Signed-off-by: Janusz Krzysztofik 
Acked-by: Mauro Carvalho Chehab 
---
 lib/igt_kmod.c | 28 
 lib/igt_ktap.c | 25 +
 lib/igt_ktap.h |  6 --
 3 files changed, 25 insertions(+), 34 deletions(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 77fc971f8f..2941524bb4 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -760,7 +760,6 @@ static void __igt_kunit(struct igt_ktest *tst, const char 
*opts)
struct kmod_module *kunit_kmod;
bool is_builtin;
struct ktap_test_results *results;
-   struct ktap_test_results_element *temp;
unsigned long taints;
int flags, ret;
 
@@ -784,28 +783,33 @@ static void __igt_kunit(struct igt_ktest *tst, const char 
*opts)
igt_skip("Unable to load %s module\n", tst->module_name);
}
 
-   while (READ_ONCE(results->still_running) || READ_ONCE(results->head) != 
NULL)
+   while (READ_ONCE(results->still_running) || 
!igt_list_empty(&results->list))
{
+   struct ktap_test_results_element *result;
+
if (igt_kernel_tainted(&taints)) {
ktap_parser_cancel();
break;
}
 
-   if (READ_ONCE(results->head) != NULL) {
-   pthread_mutex_lock(&results->mutex);
+   pthread_mutex_lock(&results->mutex);
+   if (igt_list_empty(&results->list)) {
+   pthread_mutex_unlock(&results->mutex);
+   continue;
+   }
 
-   igt_dynamic(results->head->test_name) {
-   igt_assert(READ_ONCE(results->head->passed));
+   result = igt_list_first_entry(&results->list, result, link);
 
-   igt_fail_on(igt_kernel_tainted(&taints));
-   }
+   igt_list_del(&result->link);
+   pthread_mutex_unlock(&results->mutex);
 
-   temp = results->head;
-   results->head = results->head->next;
-   free(temp);
+   igt_dynamic(result->test_name) {
+   igt_assert(READ_ONCE(result->passed));
 
-   pthread_mutex_unlock(&results->mutex);
+   igt_fail_on(igt_kernel_tainted(&taints));
}
+
+   free(result);
}
 
ret = ktap_parser_stop();
diff --git a/lib/igt_ktap.c b/lib/igt_ktap.c
index 282e44176e..c64323d9b4 100644
--- a/lib/igt_ktap.c
+++ b/lib/igt_ktap.c
@@ -12,6 +12,7 @@
 #include "igt_aux.h"
 #include "igt_core.h"
 #include "igt_ktap.h"
+#include "igt_list.h"
 
 #define DELIMITER "-"
 
@@ -335,7 +336,7 @@ static int parse_tap_level(int fd, char *base_test_name, 
int test_count, bool *f
   bool *found_tests, bool is_builtin)
 {
char record[BUF_LEN + 1];
-   struct ktap_test_results_element *r, *temp;
+   struct ktap_test_results_element *r;
int internal_test_count;
char test_name[BUF_LEN + 1];
char base_test_name_for_next_level[BUF_LEN + 1];
@@ -403,17 +404,9 @@ static int parse_tap_level(int fd, char *base_test_name, 
int test_count, bool *f
r->test_name[BUF_LEN] = '\0';
 
r->passed = false;
-   r->next = NULL;
 
pthread_mutex_lock(&results.mutex);
-   if (results.head == NULL) {
-   results.head = r;
-   } else {
-   temp = results.head;
-   while (temp->next != NULL)
-   temp = temp->next;
-   temp->next = r;
-   }
+   igt_list_add_tail(&r->link, &results.list);
pthread_mutex_unlock(&results.mutex);
 
test_name[0] = '\0';
@@ -431,17 +424,9 @@ static int parse_tap_level(int fd, char *base_test_name, 
int test_count, bool *f
r->test_name[BUF_LEN] = '\0';
 
r->passed = true;
-   r->next = NULL;
 
pthread_mutex_lock(&results.mutex);
-   if (results.head == NULL) {
-   results.head = r;
-   } else {
-   temp = results.head;
-   while (temp->next != NULL)
-   temp = temp->next;
-   temp->next = r;
-   }
+   igt_list_add_tail(&r->link, &results.li

[Intel-gfx] [PATCH i-g-t v3 14/17] lib/kunit: Load test modules in background

2023-09-18 Thread Janusz Krzysztofik
For igt_runner to be able to correlate a stream of IGT results from
dynamic sub-subtests that correspond to individual kunit test cases, read
by the igt_runner from stdout / stderr of the test process, with a stream
of kernel messages read from /dev/kmsg, we need both result streams being
fed with data in parallel.  While our KTAP parser is currently started in
the background and reads KTAP results from /dev/kmsg in parallel with
execution of kunit tests performed by the kernel while we are loading a
kunit test module, results from the parser are then only stored as
intermediate data and not processed any further until the module loading
completes.  As a consequence, there is no synchronization between the two
streams.

Call the function that loads the kunit test module from a separate thread
and process the intermediate results immediately, as soon as available
from the background parser, without waiting for completion of module
loading.

Signed-off-by: Janusz Krzysztofik 
Acked-by: Mauro Carvalho Chehab 
---
 lib/igt_kmod.c | 35 +--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 2941524bb4..8fbd274ccf 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "igt_aux.h"
@@ -746,6 +747,21 @@ void igt_kselftest_get_tests(struct kmod_module *kmod,
kmod_module_info_free_list(pre);
 }
 
+struct modprobe_data {
+   struct kmod_module *kmod;
+   const char *opts;
+   int err;
+};
+
+static void *modprobe_task(void *arg)
+{
+   struct modprobe_data *data = arg;
+
+   data->err = modprobe(data->kmod, data->opts);
+
+   return NULL;
+}
+
 /**
  * igt_kunit:
  * @module_name: the name of the module
@@ -757,9 +773,11 @@ void igt_kselftest_get_tests(struct kmod_module *kmod,
  */
 static void __igt_kunit(struct igt_ktest *tst, const char *opts)
 {
+   struct modprobe_data modprobe = { tst->kmod, opts, 0, };
struct kmod_module *kunit_kmod;
bool is_builtin;
struct ktap_test_results *results;
+   pthread_t modprobe_thread;
unsigned long taints;
int flags, ret;
 
@@ -777,18 +795,25 @@ static void __igt_kunit(struct igt_ktest *tst, const char 
*opts)
 
results = ktap_parser_start(tst->kmsg, is_builtin);
 
-   if (igt_debug_on(igt_kmod_load(tst->module_name, opts) < 0)) {
+   if (igt_debug_on(pthread_create(&modprobe_thread, NULL,
+   modprobe_task, &modprobe))) {
ktap_parser_cancel();
igt_ignore_warn(ktap_parser_stop());
-   igt_skip("Unable to load %s module\n", tst->module_name);
+   igt_skip("Failed to create a modprobe thread\n");
}
 
while (READ_ONCE(results->still_running) || 
!igt_list_empty(&results->list))
{
struct ktap_test_results_element *result;
 
+   if (!pthread_tryjoin_np(modprobe_thread, NULL) && modprobe.err) 
{
+   ktap_parser_cancel();
+   break;
+   }
+
if (igt_kernel_tainted(&taints)) {
ktap_parser_cancel();
+   pthread_cancel(modprobe_thread);
break;
}
 
@@ -806,14 +831,20 @@ static void __igt_kunit(struct igt_ktest *tst, const char 
*opts)
igt_dynamic(result->test_name) {
igt_assert(READ_ONCE(result->passed));
 
+   if (!pthread_tryjoin_np(modprobe_thread, NULL))
+   igt_assert_eq(modprobe.err, 0);
+
igt_fail_on(igt_kernel_tainted(&taints));
}
 
free(result);
}
 
+   pthread_join(modprobe_thread, NULL);
+
ret = ktap_parser_stop();
 
+   igt_skip_on(modprobe.err);
igt_skip_on(igt_kernel_tainted(&taints));
igt_skip_on_f(ret, "KTAP parser failed\n");
 }
-- 
2.41.0



[Intel-gfx] [PATCH i-g-t v3 11/17] lib/kunit: Fail / skip on kernel taint

2023-09-18 Thread Janusz Krzysztofik
Similar to how igt_kselftest() handles kernel taints, fail current dynamic
sub-subtest and skip remaining ones when a kernel taint is detected during
execution of kunit test cases.

Signed-off-by: Janusz Krzysztofik 
Reviewed-by: Mauro Carvalho Chehab 
---
 lib/igt_kmod.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 96240543a7..77fc971f8f 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -761,6 +761,7 @@ static void __igt_kunit(struct igt_ktest *tst, const char 
*opts)
bool is_builtin;
struct ktap_test_results *results;
struct ktap_test_results_element *temp;
+   unsigned long taints;
int flags, ret;
 
igt_skip_on_f(tst->kmsg < 0, "Could not open /dev/kmsg\n");
@@ -785,12 +786,20 @@ static void __igt_kunit(struct igt_ktest *tst, const char 
*opts)
 
while (READ_ONCE(results->still_running) || READ_ONCE(results->head) != 
NULL)
{
+   if (igt_kernel_tainted(&taints)) {
+   ktap_parser_cancel();
+   break;
+   }
+
if (READ_ONCE(results->head) != NULL) {
pthread_mutex_lock(&results->mutex);
 
-   igt_dynamic(results->head->test_name)
+   igt_dynamic(results->head->test_name) {
igt_assert(READ_ONCE(results->head->passed));
 
+   igt_fail_on(igt_kernel_tainted(&taints));
+   }
+
temp = results->head;
results->head = results->head->next;
free(temp);
@@ -801,6 +810,7 @@ static void __igt_kunit(struct igt_ktest *tst, const char 
*opts)
 
ret = ktap_parser_stop();
 
+   igt_skip_on(igt_kernel_tainted(&taints));
igt_skip_on_f(ret, "KTAP parser failed\n");
 }
 
-- 
2.41.0



[Intel-gfx] [PATCH i-g-t v3 10/17] lib/ktap: Read /dev/kmsg in blocking mode

2023-09-18 Thread Janusz Krzysztofik
We obtain KTAP report from /dev/kmsg.  That file is now opened from
igt_ktest_begin(), a function originally designed for i915 selftests and
now reused with kunit tests.  The original intention of opening that file
was to dump kernel messages to stderr on selftest error.  For that
purpose, the file is now opened in non-blocking mode so we don't end up
waiting for more kernel messages than already available.  Since our ktap
parser code reuses the file descriptor, we now have to loop over
EAGAIN responses, waiting for more KTAP data.  Since we have no sleeps
inside those loops, extremely high CPU usage can be observed.

Simplify reading KTAP reports by first switching the file descriptor back
to blocking mode.

While being at it, fix read errors other than EPIPE likely unintentionally
ignored when reading first line of KTAP data.

v2: Drop EINTR handling as not applicable since SIGINT default signal
handler kills the whole process anyway,
  - update commit description to also mention read error handling fix.

Signed-off-by: Janusz Krzysztofik 
Acked-by: Mauro Carvalho Chehab  # v1
---
 lib/igt_kmod.c |  7 +-
 lib/igt_ktap.c | 66 +-
 2 files changed, 28 insertions(+), 45 deletions(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 7392276401..96240543a7 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "igt_aux.h"
@@ -758,12 +759,16 @@ static void __igt_kunit(struct igt_ktest *tst, const char 
*opts)
 {
struct kmod_module *kunit_kmod;
bool is_builtin;
-   int ret;
struct ktap_test_results *results;
struct ktap_test_results_element *temp;
+   int flags, ret;
 
igt_skip_on_f(tst->kmsg < 0, "Could not open /dev/kmsg\n");
 
+   flags = fcntl(tst->kmsg, F_GETFL, 0) & ~O_NONBLOCK;
+   igt_skip_on_f(fcntl(tst->kmsg, F_SETFL, flags) == -1,
+ "Could not set /dev/kmsg to blocking mode\n");
+
igt_skip_on(lseek(tst->kmsg, 0, SEEK_END) < 0);
 
igt_skip_on(kmod_module_new_from_name(kmod_ctx(), "kunit", 
&kunit_kmod));
diff --git a/lib/igt_ktap.c b/lib/igt_ktap.c
index 5bc5e003d7..282e44176e 100644
--- a/lib/igt_ktap.c
+++ b/lib/igt_ktap.c
@@ -59,17 +59,12 @@ static int log_to_end(enum igt_log_level level, int fd,
while (*lend == '\0') {
igt_log(IGT_LOG_DOMAIN, level, "%s", record);
 
-   while (read(fd, record, BUF_LEN) < 0) {
-   if (errno == EPIPE) {
+   if (read(fd, record, BUF_LEN) < 0) {
+   if (errno == EPIPE)
igt_warn("kmsg truncated: too many messages. 
You may want to increase log_buf_len in kmcdline\n");
-   return -2;
-   }
-
-   if (errno == EAGAIN)
-   /* No records available */
-   continue;
+   else
+   igt_warn("an error occurred while reading kmsg: 
%m\n");
 
-   igt_warn("kmsg truncated: unknown error (%m)\n");
return -2;
}
 
@@ -173,17 +168,12 @@ static int find_next_tap_subtest(int fd, char *record, 
char *test_name, bool is_
return -1;
 
if (is_builtin) {
-   while (read(fd, record, BUF_LEN) < 0) {
-   if (errno == EPIPE) {
+   if (read(fd, record, BUF_LEN) < 0) {
+   if (errno == EPIPE)
igt_warn("kmsg truncated: too many messages. 
You may want to increase log_buf_len in kmcdline\n");
-   return -2;
-   }
+   else
+   igt_warn("an error occurred while reading kmsg: 
%m\n");
 
-   if (errno == EAGAIN)
-   /* No records available */
-   continue;
-
-   igt_warn("kmsg truncated: unknown error (%m)\n");
return -2;
}
}
@@ -209,17 +199,12 @@ static int find_next_tap_subtest(int fd, char *record, 
char *test_name, bool is_
if (cutoff)
cutoff[0] = '\0';
 
-   while (read(fd, record, BUF_LEN) < 0) {
-   if (errno == EPIPE) {
+   if (read(fd, record, BUF_LEN) < 0) {
+   if (errno == EPIPE)
igt_warn("kmsg truncated: too many messages. 
You may want to increase log_buf_len in kmcdline\n");
-   return -2;
-   }
-
-   if (errno == EAGAIN)
-   /* No records available */
-   continue;
+   else
+  

[Intel-gfx] [PATCH i-g-t v3 09/17] lib/ktap: Drop is_running flag

2023-09-18 Thread Janusz Krzysztofik
Since we now call pthread_cancel() when we want to stop KTAP parser before
it completes, and we take care of returning failure in that case as a
result of KTAP parsing, we no longer need to check a flag that indicates
whether we should continue parsing or return a failure.  Drop that flag.

Signed-off-by: Janusz Krzysztofik 
Reviewed-by: Mauro Carvalho Chehab 
---
 lib/igt_ktap.c | 32 
 1 file changed, 32 deletions(-)

diff --git a/lib/igt_ktap.c b/lib/igt_ktap.c
index 0db42d1243..5bc5e003d7 100644
--- a/lib/igt_ktap.c
+++ b/lib/igt_ktap.c
@@ -18,7 +18,6 @@
 struct ktap_parser_args {
int fd;
bool is_builtin;
-   volatile bool is_running;
int ret;
 } ktap_args;
 
@@ -61,11 +60,6 @@ static int log_to_end(enum igt_log_level level, int fd,
igt_log(IGT_LOG_DOMAIN, level, "%s", record);
 
while (read(fd, record, BUF_LEN) < 0) {
-   if (!READ_ONCE(ktap_args.is_running)) {
-   igt_warn("ktap parser stopped\n");
-   return -2;
-   }
-
if (errno == EPIPE) {
igt_warn("kmsg truncated: too many messages. 
You may want to increase log_buf_len in kmcdline\n");
return -2;
@@ -180,11 +174,6 @@ static int find_next_tap_subtest(int fd, char *record, 
char *test_name, bool is_
 
if (is_builtin) {
while (read(fd, record, BUF_LEN) < 0) {
-   if (!READ_ONCE(ktap_args.is_running)) {
-   igt_warn("ktap parser stopped\n");
-   return -2;
-   }
-
if (errno == EPIPE) {
igt_warn("kmsg truncated: too many messages. 
You may want to increase log_buf_len in kmcdline\n");
return -2;
@@ -221,11 +210,6 @@ static int find_next_tap_subtest(int fd, char *record, 
char *test_name, bool is_
cutoff[0] = '\0';
 
while (read(fd, record, BUF_LEN) < 0) {
-   if (!READ_ONCE(ktap_args.is_running)) {
-   igt_warn("ktap parser stopped\n");
-   return -2;
-   }
-
if (errno == EPIPE) {
igt_warn("kmsg truncated: too many messages. 
You may want to increase log_buf_len in kmcdline\n");
return -2;
@@ -373,11 +357,6 @@ static int parse_tap_level(int fd, char *base_test_name, 
int test_count, bool *f
 
for (int i = 0; i < test_count; i++) {
while (read(fd, record, BUF_LEN) < 0) {
-   if (!READ_ONCE(ktap_args.is_running)) {
-   igt_warn("ktap parser stopped\n");
-   return -1;
-   }
-
if (errno == EAGAIN)
/* No records available */
continue;
@@ -511,19 +490,11 @@ void *igt_ktap_parser(void *unused)
failed_tests = false;
found_tests = false;
 
-   if (!READ_ONCE(ktap_args.is_running))
-   goto igt_ktap_parser_end;
-
 igt_ktap_parser_start:
test_name[0] = '\0';
test_name[BUF_LEN] = '\0';
 
while (read(fd, record, BUF_LEN) < 0) {
-   if (!READ_ONCE(ktap_args.is_running)) {
-   igt_warn("ktap parser stopped\n");
-   goto igt_ktap_parser_end;
-   }
-
if (errno == EAGAIN)
/* No records available */
continue;
@@ -580,7 +551,6 @@ struct ktap_test_results *ktap_parser_start(int fd, bool 
is_builtin)
 
ktap_args.fd = fd;
ktap_args.is_builtin = is_builtin;
-   ktap_args.is_running = true;
ktap_args.ret = IGT_EXIT_FAILURE;
pthread_create(&ktap_parser_thread, NULL, igt_ktap_parser, NULL);
 
@@ -589,13 +559,11 @@ struct ktap_test_results *ktap_parser_start(int fd, bool 
is_builtin)
 
 void ktap_parser_cancel(void)
 {
-   ktap_args.is_running = false;
pthread_cancel(ktap_parser_thread);
 }
 
 int ktap_parser_stop(void)
 {
-   ktap_args.is_running = false;
pthread_join(ktap_parser_thread, NULL);
return ktap_args.ret;
 }
-- 
2.41.0



[Intel-gfx] [PATCH i-g-t v3 08/17] lib/kunit: Cancel KTP parser on module load failure

2023-09-18 Thread Janusz Krzysztofik
For our KTAP parser to be running in parallel with kunit test module
loading, we now start it in a separate thread before we load the module.
If the module loading fails then we join the KTAP parser thread right
after that failure.  If the KTAP thread sleeps for some reason then we
can fail to break the test immediately.

Cancel the KTAP parser thread right after module load error and before
joining it.

Signed-off-by: Janusz Krzysztofik 
Reviewed-by: Mauro Carvalho Chehab 
---
 lib/igt_kmod.c | 1 +
 lib/igt_ktap.c | 6 ++
 lib/igt_ktap.h | 1 +
 3 files changed, 8 insertions(+)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 063e4c12db..7392276401 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -773,6 +773,7 @@ static void __igt_kunit(struct igt_ktest *tst, const char 
*opts)
results = ktap_parser_start(tst->kmsg, is_builtin);
 
if (igt_debug_on(igt_kmod_load(tst->module_name, opts) < 0)) {
+   ktap_parser_cancel();
igt_ignore_warn(ktap_parser_stop());
igt_skip("Unable to load %s module\n", tst->module_name);
}
diff --git a/lib/igt_ktap.c b/lib/igt_ktap.c
index ce07f9aed7..0db42d1243 100644
--- a/lib/igt_ktap.c
+++ b/lib/igt_ktap.c
@@ -587,6 +587,12 @@ struct ktap_test_results *ktap_parser_start(int fd, bool 
is_builtin)
return &results;
 }
 
+void ktap_parser_cancel(void)
+{
+   ktap_args.is_running = false;
+   pthread_cancel(ktap_parser_thread);
+}
+
 int ktap_parser_stop(void)
 {
ktap_args.is_running = false;
diff --git a/lib/igt_ktap.h b/lib/igt_ktap.h
index ea57c2bb9b..991800e912 100644
--- a/lib/igt_ktap.h
+++ b/lib/igt_ktap.h
@@ -45,6 +45,7 @@ struct ktap_test_results {
 
 
 struct ktap_test_results *ktap_parser_start(int fd, bool is_builtin);
+void ktap_parser_cancel(void);
 int ktap_parser_stop(void);
 
 #endif /* IGT_KTAP_H */
-- 
2.41.0



[Intel-gfx] [PATCH i-g-t v3 07/17] lib/ktap: Drop checks for EINTR on read() failures

2023-09-18 Thread Janusz Krzysztofik
While reading KTAP data from /dev/kmsg we now ignore EINTR failures that
may occur during read() and we continue reading the data.  No explanation
has been provided on what that could be needed for.

Since we use default SIGINT signal handler, read() should never fail with
errno set to EINTR on user interrupt, only the whole process should be
terminated.  Drop checks for errno == EINTR as not applicable.

v2: Drop handling of EINTR completely, update commit message and
descripion.

Signed-off-by: Janusz Krzysztofik 
Cc: Mauro Carvalho Chehab 
---
 lib/igt_ktap.c | 15 ---
 1 file changed, 15 deletions(-)

diff --git a/lib/igt_ktap.c b/lib/igt_ktap.c
index 84fb13218f..ce07f9aed7 100644
--- a/lib/igt_ktap.c
+++ b/lib/igt_ktap.c
@@ -66,9 +66,6 @@ static int log_to_end(enum igt_log_level level, int fd,
return -2;
}
 
-   if (errno == EINTR)
-   continue;
-
if (errno == EPIPE) {
igt_warn("kmsg truncated: too many messages. 
You may want to increase log_buf_len in kmcdline\n");
return -2;
@@ -188,9 +185,6 @@ static int find_next_tap_subtest(int fd, char *record, char 
*test_name, bool is_
return -2;
}
 
-   if (errno == EINTR)
-   continue;
-
if (errno == EPIPE) {
igt_warn("kmsg truncated: too many messages. 
You may want to increase log_buf_len in kmcdline\n");
return -2;
@@ -232,9 +226,6 @@ static int find_next_tap_subtest(int fd, char *record, char 
*test_name, bool is_
return -2;
}
 
-   if (errno == EINTR)
-   continue;
-
if (errno == EPIPE) {
igt_warn("kmsg truncated: too many messages. 
You may want to increase log_buf_len in kmcdline\n");
return -2;
@@ -387,9 +378,6 @@ static int parse_tap_level(int fd, char *base_test_name, 
int test_count, bool *f
return -1;
}
 
-   if (errno == EINTR)
-   continue;
-
if (errno == EAGAIN)
/* No records available */
continue;
@@ -540,9 +528,6 @@ igt_ktap_parser_start:
/* No records available */
continue;
 
-   if (errno == EINTR)
-   continue;
-
if (errno == EPIPE) {
igt_warn("kmsg truncated: too many messages. You may 
want to increase log_buf_len in kmcdline\n");
goto igt_ktap_parser_end;
-- 
2.41.0



[Intel-gfx] [PATCH i-g-t v3 06/17] lib/ktap: Make sure we fail on premature cancel

2023-09-18 Thread Janusz Krzysztofik
After loading a kunit test module that executes some kunit test cases, we
evaluate overall result of an IGT subtest that corresponds to that module
based on an error code returned by kunit_parser_stop() helper, obtained
from a .ret field of a ktap_args structure.  That code is now assigned to
that structure field right before completion of the KTAP parser thread
start routine.  If the thread is canceled for some reason then the return
code will be undefined.

Initialize the return code on KTAP parser startup with a value that
indicates a failure, then change it to success when so indicated by result
of KTAP parsing.

Signed-off-by: Janusz Krzysztofik 
Reviewed-by: Mauro Carvalho Chehab 
---
 lib/igt_ktap.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/igt_ktap.c b/lib/igt_ktap.c
index 123a40d183..84fb13218f 100644
--- a/lib/igt_ktap.c
+++ b/lib/igt_ktap.c
@@ -579,9 +579,7 @@ igt_ktap_parser_start:
 igt_ktap_parser_end:
results.still_running = false;
 
-   if (failed_tests || !found_tests)
-   ktap_args.ret = IGT_EXIT_FAILURE;
-   else
+   if (found_tests && !failed_tests)
ktap_args.ret = IGT_EXIT_SUCCESS;
 
return NULL;
@@ -598,6 +596,7 @@ struct ktap_test_results *ktap_parser_start(int fd, bool 
is_builtin)
ktap_args.fd = fd;
ktap_args.is_builtin = is_builtin;
ktap_args.is_running = true;
+   ktap_args.ret = IGT_EXIT_FAILURE;
pthread_create(&ktap_parser_thread, NULL, igt_ktap_parser, NULL);
 
return &results;
-- 
2.41.0



[Intel-gfx] [PATCH i-g-t v3 03/17] lib/kunit: Fix struct kmod_module kunit_kmod not freed

2023-09-18 Thread Janusz Krzysztofik
We obtain a kmod_module structure for kunit module in order to check if
it is modular or built-in, then we never release that structure.  Fix it.

Signed-off-by: Janusz Krzysztofik 
Reviewed-by: Mauro Carvalho Chehab 
---
 lib/igt_kmod.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index faf31afabc..34ddec3fad 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -797,6 +797,7 @@ static void __igt_kunit(const char *module_name, const char 
*opts)
}
 
is_builtin = kmod_module_get_initstate(kunit_kmod) == 
KMOD_MODULE_BUILTIN;
+   kmod_module_unref(kunit_kmod);
 
results = ktap_parser_start(tst.kmsg, is_builtin);
 
-- 
2.41.0



[Intel-gfx] [PATCH i-g-t v3 05/17] lib/kunit: Fix illegal igt_fail() calls inside subtest body

2023-09-18 Thread Janusz Krzysztofik
In a body of a subtest with dynamic sub-subtests, it is illegal to call
igt_fail() and its variants from outside of a dynamic sub-subtest body.
On the other hand, it is perfectly legal to call either igt_skip() and
friends or __igt_abort() or its variant from there.

In the current implementation of igt_kunit(), there are several places
where igt_fail() is called despite being illegal.  Moreover, it is called
with IGT_EXIT_ABORT as an argument with no good reason for using such
aggressive method that forces CI to trigger system reboot (in most cases
igt_runner can decide if abort is required).

Follow igt_kselftests() pattern more closely, where similar setup and
cleanup operations are performed but their potential errors are processed
in a more friendly way.  Move common cleanup and their corresponding setup
steps out of the subtest body.  Place the latter as requirements in a
preceding igt_fixture section.  Replace remaining illegal igt_fail() calls
with more friendly skips.  Let igt_runner decide if abort is needed.

v2: Also call igt_skip() on igt_ktest_init() failure (Mauro), but then,
initialize local tst structure when declaring it to avoid freeing a
random pointer from igt_ktest_fini() when only listing subtests,
  - call igt_ktest_end() from igt_fixture so it is not unnecessarily
called when only listing subtests.

Signed-off-by: Janusz Krzysztofik 
Cc: Mauro Carvalho Chehab 
---
 lib/igt_kmod.c | 76 +++---
 1 file changed, 23 insertions(+), 53 deletions(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 1d1cd51170..063e4c12db 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -754,59 +754,27 @@ void igt_kselftest_get_tests(struct kmod_module *kmod,
  *
  * Returns: IGT default codes
  */
-static void __igt_kunit(const char *module_name, const char *opts)
+static void __igt_kunit(struct igt_ktest *tst, const char *opts)
 {
-   struct igt_ktest tst;
struct kmod_module *kunit_kmod;
bool is_builtin;
int ret;
struct ktap_test_results *results;
struct ktap_test_results_element *temp;
-   int skip = 0;
-   bool fail = false;
-
-   /* get normalized module name */
-   if (igt_ktest_init(&tst, module_name) != 0) {
-   igt_warn("Unable to initialize ktest for %s\n", module_name);
-   igt_fail(IGT_EXIT_ABORT);
-   }
 
-   if (igt_ktest_begin(&tst) != 0) {
-   igt_warn("Unable to begin ktest for %s\n", module_name);
-   igt_ktest_fini(&tst);
-   igt_fail(IGT_EXIT_ABORT);
-   }
+   igt_skip_on_f(tst->kmsg < 0, "Could not open /dev/kmsg\n");
 
-   if (tst.kmsg < 0) {
-   igt_warn("Could not open /dev/kmsg\n");
-   fail = true;
-   goto unload;
-   }
-
-   if (lseek(tst.kmsg, 0, SEEK_END)) {
-   igt_warn("Could not seek the end of /dev/kmsg\n");
-   fail = true;
-   goto unload;
-   }
-
-   ret = kmod_module_new_from_name(kmod_ctx(), "kunit", &kunit_kmod);
-   if (ret) {
-   igt_warn("Unable to load KUnit\n");
-   skip = ret;
-   goto unload;
-   }
+   igt_skip_on(lseek(tst->kmsg, 0, SEEK_END) < 0);
 
+   igt_skip_on(kmod_module_new_from_name(kmod_ctx(), "kunit", 
&kunit_kmod));
is_builtin = kmod_module_get_initstate(kunit_kmod) == 
KMOD_MODULE_BUILTIN;
kmod_module_unref(kunit_kmod);
 
-   results = ktap_parser_start(tst.kmsg, is_builtin);
+   results = ktap_parser_start(tst->kmsg, is_builtin);
 
-   ret = igt_kmod_load(module_name, opts);
-   if (ret) {
-   skip = ret;
-   igt_warn("Unable to load %s module\n", module_name);
-   ret = ktap_parser_stop();
-   goto unload;
+   if (igt_debug_on(igt_kmod_load(tst->module_name, opts) < 0)) {
+   igt_ignore_warn(ktap_parser_stop());
+   igt_skip("Unable to load %s module\n", tst->module_name);
}
 
while (READ_ONCE(results->still_running) || READ_ONCE(results->head) != 
NULL)
@@ -825,24 +793,21 @@ static void __igt_kunit(const char *module_name, const 
char *opts)
}
}
 
-unload:
-   igt_ktest_end(&tst);
-
-   igt_ktest_fini(&tst);
-
-   igt_skip_on_f(skip, "Skipping test, as probing KUnit module failed\n");
-
-   if (fail)
-   igt_fail(IGT_EXIT_ABORT);
-
ret = ktap_parser_stop();
 
-   if (ret != 0)
-   igt_fail(IGT_EXIT_ABORT);
+   igt_skip_on_f(ret, "KTAP parser failed\n");
 }
 
 void igt_kunit(const char *module_name, const char *name, const char *opts)
 {
+   struct igt_ktest tst = { .kmsg = -1, };
+
+
+   igt_fixture {
+   igt_skip_on(igt_ktest_init(&tst, module_name));
+   igt_skip_on(igt_ktest_begin(&tst));
+   }
+
/*
 * We need to use igt_subtest here, as otherwise it may c

[Intel-gfx] [PATCH i-g-t v3 04/17] lib/kunit: Optimize calls to igt_success/skip/fail()

2023-09-18 Thread Janusz Krzysztofik
Calling igt_success() explicitly at the end of subtest body is not needed.
Other calls to igt_success() can be usually avoided by inverting test
result checks.  Optimize the code that now calls igt_success().

Moreover, using more advanced variants of igt_skip() and igt_fail() where
applicable makes the code more compact.  Go for it.

Signed-off-by: Janusz Krzysztofik 
Reviewed-by: Mauro Carvalho Chehab 
---
 lib/igt_kmod.c | 14 +++---
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 34ddec3fad..1d1cd51170 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -814,12 +814,8 @@ static void __igt_kunit(const char *module_name, const 
char *opts)
if (READ_ONCE(results->head) != NULL) {
pthread_mutex_lock(&results->mutex);
 
-   igt_dynamic(results->head->test_name) {
-   if (READ_ONCE(results->head->passed))
-   igt_success();
-   else
-   igt_fail(IGT_EXIT_FAILURE);
-   }
+   igt_dynamic(results->head->test_name)
+   igt_assert(READ_ONCE(results->head->passed));
 
temp = results->head;
results->head = results->head->next;
@@ -834,8 +830,7 @@ unload:
 
igt_ktest_fini(&tst);
 
-   if (skip)
-   igt_skip("Skipping test, as probing KUnit module returned %d", 
skip);
+   igt_skip_on_f(skip, "Skipping test, as probing KUnit module failed\n");
 
if (fail)
igt_fail(IGT_EXIT_ABORT);
@@ -844,9 +839,6 @@ unload:
 
if (ret != 0)
igt_fail(IGT_EXIT_ABORT);
-
-   if (ret == 0)
-   igt_success();
 }
 
 void igt_kunit(const char *module_name, const char *name, const char *opts)
-- 
2.41.0



[Intel-gfx] [PATCH i-g-t v3 02/17] lib/kunit: Stop loading kunit module explicitly

2023-09-18 Thread Janusz Krzysztofik
Since kmod functions we use for module loading can process module
dependencies, there is no need to load the "kunit" module explicitly
before loading a kunit test module.  For the same reason we already don't
unload the "kunit" module explicitly on cleanup.  Drop the unnecessary
operation.

Signed-off-by: Janusz Krzysztofik 
Acked-by: Mauro Carvalho Chehab 
---
 lib/igt_kmod.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 97667a896f..faf31afabc 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -789,12 +789,6 @@ static void __igt_kunit(const char *module_name, const 
char *opts)
goto unload;
}
 
-   /* The KUnit module is required for running any KUnit tests */
-   ret = igt_kmod_load("kunit", NULL);
-   if (ret) {
-   skip = ret;
-   goto unload;
-   }
ret = kmod_module_new_from_name(kmod_ctx(), "kunit", &kunit_kmod);
if (ret) {
igt_warn("Unable to load KUnit\n");
-- 
2.41.0



[Intel-gfx] [PATCH i-g-t v3 01/17] lib/kunit: Drop unused file stream

2023-09-18 Thread Janusz Krzysztofik
In the process of reviewing patches that introduced kunit support, I asked
once if we could use line buffered input instead of explicitly looking for
newlines in kmsg data.  While my idea was wrong because each read of raw
data from /dev/kmsg always returns one full log record that always ends
with a newline, conversion of /dev/kmsg file descriptor to a file stream
with freopen() was added to the code.  However, that file stream has never
been used for line buffered input.  While the file stream is passed to
functions that actually read the data, there it is converted back to a
file descriptor with fileno() and raw data is read with read().

Drop the unnecessary conversions and teach functions to accept and process
just the file descriptor of /dev/kmsg.

Signed-off-by: Janusz Krzysztofik 
Acked-by: Mauro Carvalho Chehab 
---
 lib/igt_kmod.c | 12 +-
 lib/igt_ktap.c | 62 +++---
 lib/igt_ktap.h |  2 +-
 3 files changed, 31 insertions(+), 45 deletions(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 6205871791..97667a896f 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -758,7 +758,6 @@ static void __igt_kunit(const char *module_name, const char 
*opts)
 {
struct igt_ktest tst;
struct kmod_module *kunit_kmod;
-   FILE *f;
bool is_builtin;
int ret;
struct ktap_test_results *results;
@@ -774,7 +773,6 @@ static void __igt_kunit(const char *module_name, const char 
*opts)
 
if (igt_ktest_begin(&tst) != 0) {
igt_warn("Unable to begin ktest for %s\n", module_name);
-
igt_ktest_fini(&tst);
igt_fail(IGT_EXIT_ABORT);
}
@@ -791,14 +789,6 @@ static void __igt_kunit(const char *module_name, const 
char *opts)
goto unload;
}
 
-   f = fdopen(tst.kmsg, "r");
-
-   if (f == NULL) {
-   igt_warn("Could not turn /dev/kmsg file descriptor into a FILE 
pointer\n");
-   fail = true;
-   goto unload;
-   }
-
/* The KUnit module is required for running any KUnit tests */
ret = igt_kmod_load("kunit", NULL);
if (ret) {
@@ -814,7 +804,7 @@ static void __igt_kunit(const char *module_name, const char 
*opts)
 
is_builtin = kmod_module_get_initstate(kunit_kmod) == 
KMOD_MODULE_BUILTIN;
 
-   results = ktap_parser_start(f, is_builtin);
+   results = ktap_parser_start(tst.kmsg, is_builtin);
 
ret = igt_kmod_load(module_name, opts);
if (ret) {
diff --git a/lib/igt_ktap.c b/lib/igt_ktap.c
index ecdcb8d83d..123a40d183 100644
--- a/lib/igt_ktap.c
+++ b/lib/igt_ktap.c
@@ -16,7 +16,7 @@
 #define DELIMITER "-"
 
 struct ktap_parser_args {
-   FILE *fp;
+   int fd;
bool is_builtin;
volatile bool is_running;
int ret;
@@ -24,7 +24,7 @@ struct ktap_parser_args {
 
 static struct ktap_test_results results;
 
-static int log_to_end(enum igt_log_level level, FILE *f,
+static int log_to_end(enum igt_log_level level, int fd,
  char *record, const char *format, ...) 
__attribute__((format(printf, 4, 5)));
 
 /**
@@ -39,12 +39,11 @@ static int log_to_end(enum igt_log_level level, FILE *f,
  *
  * Returns: 0 for success, or -2 if there's an error reading from the file
  */
-static int log_to_end(enum igt_log_level level, FILE *f,
+static int log_to_end(enum igt_log_level level, int fd,
  char *record, const char *format, ...)
 {
va_list args;
const char *lend;
-   int f_fd = fileno(f);
 
/* Cutoff after newline character, in order to not display garbage */
char *cutoff = strchr(record, '\n');
@@ -61,7 +60,7 @@ static int log_to_end(enum igt_log_level level, FILE *f,
while (*lend == '\0') {
igt_log(IGT_LOG_DOMAIN, level, "%s", record);
 
-   while (read(f_fd, record, BUF_LEN) < 0) {
+   while (read(fd, record, BUF_LEN) < 0) {
if (!READ_ONCE(ktap_args.is_running)) {
igt_warn("ktap parser stopped\n");
return -2;
@@ -157,8 +156,8 @@ static int tap_version_present(char* record, bool 
print_info)
 
 /**
  * find_next_tap_subtest:
- * @fp: FILE pointer
- * @record: buffer used to read fp
+ * @fd: file descriptor
+ * @record: buffer used to read fd
  * @is_builtin: whether KUnit is built-in or not
  *
  * Returns:
@@ -167,11 +166,10 @@ static int tap_version_present(char* record, bool 
print_info)
  * -2 if there are problems while reading the file.
  * any other value corresponds to the amount of cases of the next (sub)test
  */
-static int find_next_tap_subtest(FILE *fp, char *record, char *test_name, bool 
is_builtin)
+static int find_next_tap_subtest(int fd, char *record, char *test_name, bool 
is_builtin)
 {
const char *test_lookup_str, *subtest_lookup_str, *name_rptr;
long test_count;
-   int fp_fd = fileno(fp

[Intel-gfx] [PATCH i-g-t v3 00/17] Fix IGT Kunit implementation issues

2023-09-18 Thread Janusz Krzysztofik
v3: Also call igt_skip() on igt_ktest_init() failure (Mauro), but then,
initialize local tst structure when declaring it to avoid freeing a
random pointer from igt_ktest_fini() when only listing subtests,
  - call igt_ktest_end() from igt_fixture so it is not unnecessarily
called when only listing subtests,
  - drop EINTR handling completely as not applicable since SIGINT default
signal handler kills the whole process anyway,
  - update commit description to also mention read error handling fix,
  - interrupt blocking read() on modprobe failure.
v2: Fix incorrect and missing includes in the test source file,
  - add license and copyright clauses to the test source file.

Janusz Krzysztofik (17):
  lib/kunit: Drop unused file stream
  lib/kunit: Stop loading kunit module explicitly
  lib/kunit: Fix struct kmod_module kunit_kmod not freed
  lib/kunit: Optimize calls to igt_success/skip/fail()
  lib/kunit: Fix illegal igt_fail() calls inside subtest body
  lib/ktap: Make sure we fail on premature cancel
  lib/ktap: Drop checks for EINTR on read() failures
  lib/kunit: Cancel KTP parser on module load failure
  lib/ktap: Drop is_running flag
  lib/ktap: Read /dev/kmsg in blocking mode
  lib/kunit: Fail / skip on kernel taint
  lib/ktap: Use IGT linked lists for storing KTAP results
  lib/ktap: Reimplement KTAP parser
  lib/kunit: Load test modules in background
  lib/kunit: Parse KTAP report from the main process thread
  lib/kunit: Strip "_test" or "_kunit" suffix from subtest names
  lib/kunit: Omit suite name prefix if the same as subtest name

 lib/igt_kmod.c  | 320 ++
 lib/igt_ktap.c  | 833 
 lib/igt_ktap.h  |  28 +-
 lib/tests/igt_ktap_parser.c | 246 +++
 lib/tests/meson.build   |   1 +
 tests/drm_mm.c  |  42 +-
 6 files changed, 775 insertions(+), 695 deletions(-)
 create mode 100644 lib/tests/igt_ktap_parser.c

-- 
2.41.0



Re: [Intel-gfx] [PATCH 00/10] drm/i915/spi: spi access for discrete graphics

2023-09-18 Thread Miquel Raynal
Hi,

alexander.usys...@intel.com wrote on Tue, 12 Sep 2023 13:15:58 +:

> >   
> > > The spi controller on discreet graphics card is not visible to user-space.
> > > Spi access flows are supported by another hardware module and relevant  
> > registers are  
> > > available on graphics device memory bar.  
> > 
> > No SPI controllers are directly visible to userspace, some SPI devices
> > are selectively exposed but that needs to be explicitly requested and is
> > generally discouraged.  
> 
> What are the options here? Explicitly request exception is the one.
> Any other way to add access to flash memory connected in such way?

Register a spi controller with at least spi-mem ops, as suggested
previously, is the standard way I guess. If you're not willing to do
so, it must be justified, I guess?

Thanks,
Miquèl


Re: [Intel-gfx] [PATCH] drm/tests: Fix incorrect argument in drm_test_mm_insert_range

2023-09-18 Thread Maira Canal

On 9/15/23 11:17, Janusz Krzysztofik wrote:

Hi Maíra,

Thanks for review.

On Friday, 15 September 2023 16:01:31 CEST Maira Canal wrote:

Hi,

On 9/11/23 10:03, Janusz Krzysztofik wrote:

While drm_mm test was converted form igt selftest to kunit, unexpected
value of "end" argument equal "start" was introduced to one of calls to a
function that executes the drm_test_mm_insert_range for specific start/end
pair of arguments.  As a consequence, DRM_MM_BUG_ON(end <= start) is
triggered.  Fix it by restoring the original value.

Fixes: fc8d29e298cf ("drm: selftest: convert drm_mm selftest to KUnit")
Signed-off-by: Janusz Krzysztofik 


Reviewed-by: Maíra Canal 

Do you need me to push it to drm-misc-fixes?


Yes, please do if you can.


Pushed to drm-misc/drm-misc-fixes. Thanks!

Best Regards,
- Maíra



Thanks,
Janusz



Best Regards,
- Maíra


Cc: "Maíra Canal" 
Cc: Arthur Grillo 
Cc: Javier Martinez Canillas 
Cc: Daniel Latypov 
Cc: sta...@vger.kernel.org # v6.1+
---
   drivers/gpu/drm/tests/drm_mm_test.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tests/drm_mm_test.c 
b/drivers/gpu/drm/tests/drm_mm_test.c
index 186b28dc70380..05d5e7af6d250 100644
--- a/drivers/gpu/drm/tests/drm_mm_test.c
+++ b/drivers/gpu/drm/tests/drm_mm_test.c
@@ -939,7 +939,7 @@ static void drm_test_mm_insert_range(struct kunit *test)
KUNIT_ASSERT_FALSE(test, __drm_test_mm_insert_range(test, 
count, size, 0, max - 1));
KUNIT_ASSERT_FALSE(test, __drm_test_mm_insert_range(test, 
count, size, 0, max / 2));
KUNIT_ASSERT_FALSE(test, __drm_test_mm_insert_range(test, 
count, size,
-   max / 2, 
max / 2));
+   max / 2, 
max));
KUNIT_ASSERT_FALSE(test, __drm_test_mm_insert_range(test, 
count, size,
max / 4 + 
1, 3 * max / 4 - 1));
   









Re: [Intel-gfx] [PATCH] drm/tests: Fix incorrect argument in drm_test_mm_insert_range

2023-09-18 Thread Maira Canal

Hi,

On 9/11/23 10:03, Janusz Krzysztofik wrote:

While drm_mm test was converted form igt selftest to kunit, unexpected
value of "end" argument equal "start" was introduced to one of calls to a
function that executes the drm_test_mm_insert_range for specific start/end
pair of arguments.  As a consequence, DRM_MM_BUG_ON(end <= start) is
triggered.  Fix it by restoring the original value.

Fixes: fc8d29e298cf ("drm: selftest: convert drm_mm selftest to KUnit")
Signed-off-by: Janusz Krzysztofik 


Reviewed-by: Maíra Canal 

Do you need me to push it to drm-misc-fixes?

Best Regards,
- Maíra


Cc: "Maíra Canal" 
Cc: Arthur Grillo 
Cc: Javier Martinez Canillas 
Cc: Daniel Latypov 
Cc: sta...@vger.kernel.org # v6.1+
---
  drivers/gpu/drm/tests/drm_mm_test.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tests/drm_mm_test.c 
b/drivers/gpu/drm/tests/drm_mm_test.c
index 186b28dc70380..05d5e7af6d250 100644
--- a/drivers/gpu/drm/tests/drm_mm_test.c
+++ b/drivers/gpu/drm/tests/drm_mm_test.c
@@ -939,7 +939,7 @@ static void drm_test_mm_insert_range(struct kunit *test)
KUNIT_ASSERT_FALSE(test, __drm_test_mm_insert_range(test, 
count, size, 0, max - 1));
KUNIT_ASSERT_FALSE(test, __drm_test_mm_insert_range(test, 
count, size, 0, max / 2));
KUNIT_ASSERT_FALSE(test, __drm_test_mm_insert_range(test, 
count, size,
-   max / 2, 
max / 2));
+   max / 2, 
max));
KUNIT_ASSERT_FALSE(test, __drm_test_mm_insert_range(test, 
count, size,
max / 4 + 
1, 3 * max / 4 - 1));
  


Re: [Intel-gfx] [v3] drm/i915/display/lspcon: Increase LSPCON mode settle timeout

2023-09-18 Thread Joshua Pius
Yes, we've proposed this change before. The reasoning is still the
same. Added below to include in this thread as well. Is there a reason
the below explanation and test is not sufficient?

This issue affected several different CometLake-based Chrome OS device
designs. The details of the original report are in the Google partner
issue tracker (issue # 178169843), but I believe this requires a
Google partner account to access:
https://partnerissuetracker.corp.google.com/issues/178169843

The summary is that we were seeing these "*ERROR* LSPCON mode hasn't
settled" messages in the kernel logs followed by the display not
working at all. We increased the timeout to 500ms while investigation
continued and this reduced the number of occurrences of this issue:
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/7b2899fc1a6f9409e8075b3153baaf02c4d1fc75

The problem continued to occur on about 2% of devices even after
increasing the timeout to 500ms. The investigation continued in issue
# 188035814, with engineers from Parade and Intel involved.
Ultimately, the recommendation from Intel engineers was to increase
the timeout further:
https://partnerissuetracker.corp.google.com/issues/188035814

The timeout was then increased to 1000ms:
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/a16cfc2062e768c8e5ad8fa09b8ca127aa1ead9a

I recently ran 100 reboot trials on one device and found that the
median time for the LSPCON mode to settle was 440ms and the max was
444ms. But we know from the original reports that even after we set
the timeout to 500ms the issue continued to happen on some small
percentage of devices. So this is why I picked the larger value of
800ms.

>> This is to eliminate all cases of "*ERROR* LSPCON mode hasn't settled",
>> followed by link training errors. Intel engineers recommended increasing
>> this timeout and that does resolve the issue.
>>
>> On some CometLake-based device designs the Parade PS175 takes more than
>> 400ms to settle in PCON mode. 100 reboot trials on one device resulted
>> in a median settle time of 440ms and a maximum of 444ms. Even after
>> increasing the timeout to 500ms, 2% of devices still had this error. So
>> this increases the timeout to 800ms.
>>
>> Signed-off-by: Pablo Ceballos 
>
>I think we've been here before. Do you have a publicly available gitlab
>issue with the proper logs? If not, please file one at [1].
>
>BR,
>Jani.
>
>[1] https://gitlab.freedesktop.org/drm/intel/issues/new
>
>
>> ---
>>
>> V2: Added more details in the commit message
>> V3: Only apply the increased timeout if the vendor is Parade
>>
>> drivers/gpu/drm/i915/display/intel_lspcon.c | 21 -
>>  1 file changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.c 
>> b/drivers/gpu/drm/i915/display/intel_lspcon.c
>> index bb3b5355a0d9..b07eab84cc63 100644
>> --- a/drivers/gpu/drm/i915/display/intel_lspcon.c
>> +++ b/drivers/gpu/drm/i915/display/intel_lspcon.c
>> @@ -153,6 +153,24 @@ static enum drm_lspcon_mode 
>> lspcon_get_current_mode(struct intel_lspcon *lspcon)
>>   return current_mode;
>>  }
>>
>> +static u32 lspcon_get_mode_settle_timeout(struct intel_lspcon *lspcon)
>> +{
>> + u32 timeout_ms = 400;
>> +
>> + /*
>> + * On some CometLake-based device designs the Parade PS175 takes more
>> + * than 400ms to settle in PCON mode. 100 reboot trials on one device
>> + * resulted in a median settle time of 440ms and a maximum of 444ms.
>> + * Even after increasing the timeout to 500ms, 2% of devices still had
>> + * this error. So this sets the timeout to 800ms.
>> + */
>> + if (lspcon->vendor == LSPCON_VENDOR_PARADE)
>> + timeout_ms = 800;
>> +
>> + return timeout_ms;
>> +}
>> +
>> +
>>  static enum drm_lspcon_mode lspcon_wait_mode(struct intel_lspcon *lspcon,
>>   enum drm_lspcon_mode mode)
>>  {
>> @@ -167,7 +185,8 @@ static enum drm_lspcon_mode lspcon_wait_mode(struct 
>> intel_lspcon *lspcon,
>>   drm_dbg_kms(&i915->drm, "Waiting for LSPCON mode %s to settle\n",
>>  lspcon_mode_name(mode));
>>
>> - wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode, 400);
>> + wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode,
>> + lspcon_get_mode_settle_timeout(lspcon));
>>   if (current_mode != mode)
>>   drm_err(&i915->drm, "LSPCON mode hasn't settled\n");


[Intel-gfx] [PATCH] drm/i915: refactor deprecated strncpy

2023-09-18 Thread Justin Stitt
`strncpy` is deprecated for use on NUL-terminated destination strings [1].

We should prefer more robust and less ambiguous string interfaces.

A suitable replacement is `strscpy` [2] due to the fact that it guarantees
NUL-termination on the destination buffer without unnecessarily NUL-padding.

`ctx` is zero allocated and as such strncpy's NUL-padding behavior was
strictly a performance hit which should now be resolved. It should be
noted, however, that performance is not critical in these selftests,
especially by these margins.

Link: 
https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
 [1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-harden...@vger.kernel.org
Signed-off-by: Justin Stitt 
---
 drivers/gpu/drm/i915/gem/selftests/mock_context.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_context.c 
b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
index 8ac6726ec16b..025b9c773b93 100644
--- a/drivers/gpu/drm/i915/gem/selftests/mock_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
@@ -36,7 +36,7 @@ mock_context(struct drm_i915_private *i915,
if (name) {
struct i915_ppgtt *ppgtt;
 
-   strncpy(ctx->name, name, sizeof(ctx->name) - 1);
+   strscpy(ctx->name, name, sizeof(ctx->name) - 1);
 
ppgtt = mock_ppgtt(i915, name);
if (!ppgtt)

---
base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec
change-id: 
20230914-strncpy-drivers-gpu-drm-i915-gem-selftests-mock_context-c-980c8ecc9142

Best regards,
--
Justin Stitt 



Re: [Intel-gfx] PR for HuC v8.5.4 for MTL

2023-09-18 Thread Josh Boyer
On Thu, Sep 14, 2023 at 11:49 AM Daniele Ceraolo Spurio
 wrote:
>
> The following changes since commit dfa11466cf000120d1551146fd5bf78c44941eda:
>
>   Merge branch 'main' into 'main' (2023-09-07 11:36:57 +)
>
> are available in the Git repository at:
>
>   git://anongit.freedesktop.org/drm/drm-firmware mtl_huc_8.5.4

Pulled and pushed out.

josh

>
> for you to fetch changes up to a5dbe400f776b0dc2d0a402ba76b4c16c231b38e:
>
>   i915: update MTL HuC to version 8.5.4 (2023-09-14 08:34:08 -0700)
>
> 
> Daniele Ceraolo Spurio (1):
>   i915: update MTL HuC to version 8.5.4
>
>  WHENCE   |   2 +-
>  i915/mtl_huc_gsc.bin | Bin 569344 -> 561152 bytes
>  2 files changed, 1 insertion(+), 1 deletion(-)


Re: [Intel-gfx] [PATCH 5/7] drm/i915: Implement GGTT update method with MI_UPDATE_GTT

2023-09-18 Thread Nirmoy Das



On 9/15/2023 5:56 PM, Zeng, Oak wrote:


Thanks,
Oak


-Original Message-
From: Das, Nirmoy 
Sent: Friday, September 15, 2023 4:34 AM
To: intel-gfx@lists.freedesktop.org
Cc: Zeng, Oak ; chris.p.wil...@linux.intel.com; Piorkowski,
Piotr ; Shyti, Andi ; Mun,
Gwan-gyeong ; Roper, Matthew D
; Das, Nirmoy 
Subject: [PATCH 5/7] drm/i915: Implement GGTT update method with
MI_UPDATE_GTT

Implement GGTT update method with blitter command, MI_UPDATE_GTT
and install those handlers if a platform requires that.

v2: Make sure we hold the GT wakeref and Blitter engine wakeref before
we call mutex_lock/intel_context_enter below. When GT/engine are not
awake, the intel_context_enter calls into some runtime pm function which
can end up with kmalloc/fs_reclaim. But trigger fs_reclaim holding a
mutex lock is not allowed because shrinker can also try to hold the same
mutex lock. It is a circular lock. So hold the GT/blitter engine wakeref
before calling mutex_lock, to fix the circular lock.

Signed-off-by: Nirmoy Das 
Signed-off-by: Oak Zeng 
---
  drivers/gpu/drm/i915/gt/intel_ggtt.c | 235 +++
  1 file changed, 235 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index dd0ed941441a..b94de7cebfce 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -15,18 +15,23 @@
  #include "display/intel_display.h"
  #include "gem/i915_gem_lmem.h"

+#include "intel_context.h"
  #include "intel_ggtt_gmch.h"
+#include "intel_gpu_commands.h"
  #include "intel_gt.h"
  #include "intel_gt_regs.h"
  #include "intel_pci_config.h"
+#include "intel_ring.h"
  #include "i915_drv.h"
  #include "i915_pci.h"
+#include "i915_request.h"
  #include "i915_scatterlist.h"
  #include "i915_utils.h"
  #include "i915_vgpu.h"

  #include "intel_gtt.h"
  #include "gen8_ppgtt.h"
+#include "intel_engine_pm.h"

  static void i915_ggtt_color_adjust(const struct drm_mm_node *node,
   unsigned long color,
@@ -252,6 +257,145 @@ u64 gen8_ggtt_pte_encode(dma_addr_t addr,
return pte;
  }

+static bool should_update_ggtt_with_bind(struct i915_ggtt *ggtt)
+{
+   struct intel_gt *gt = ggtt->vm.gt;
+
+   return intel_gt_is_bind_context_ready(gt);
+}
+
+static struct intel_context *gen8_ggtt_bind_get_ce(struct i915_ggtt *ggtt)
+{
+   struct intel_context *ce;
+   struct intel_gt *gt = ggtt->vm.gt;
+
+   if (intel_gt_is_wedged(gt))
+   return NULL;
+
+   ce = gt->engine[BCS0]->bind_context;
+   GEM_BUG_ON(!ce);
+
+   /*
+* If the GT is not awake already at this stage then fallback
+* to pci based GGTT update otherwise __intel_wakeref_get_first()
+* would conflict with fs_reclaim trying to allocate memory while
+* doing rpm_resume().
+*/
+   if (!intel_gt_pm_get_if_awake(gt))
+   return NULL;
+
+   intel_engine_pm_get(ce->engine);
+
+   return ce;
+}
+
+static void gen8_ggtt_bind_put_ce(struct intel_context *ce)
+{
+   intel_engine_pm_put(ce->engine);
+   intel_gt_pm_put(ce->engine->gt);
+}
+
+static bool gen8_ggtt_bind_ptes(struct i915_ggtt *ggtt, u32 offset,
+   struct sg_table *pages, u32 num_entries,
+   const gen8_pte_t pte)
+{
+   struct i915_sched_attr attr = {};
+   struct intel_gt *gt = ggtt->vm.gt;
+   const gen8_pte_t scratch_pte = ggtt->vm.scratch[0]->encode;
+   struct sgt_iter iter;
+   struct i915_request *rq;
+   struct intel_context *ce;
+   u32 *cs;
+
+   if (!num_entries)
+   return true;
+
+   ce = gen8_ggtt_bind_get_ce(ggtt);
+   if (!ce)
+   return false;
+
+   if (pages)
+   iter = __sgt_iter(pages->sgl, true);
+
+   while (num_entries) {
+   int count = 0;
+   dma_addr_t addr;
+   /*
+* MI_UPDATE_GTT can update 512 entries in a single command
but
+* that end up with engine reset, 511 works.
+*/
+   u32 n_ptes = min_t(u32, 511, num_entries);
+
+   if (mutex_lock_interruptible(&ce->timeline->mutex))
+   goto put_ce;
+
+   intel_context_enter(ce);
+   rq = __i915_request_create(ce, GFP_NOWAIT | GFP_ATOMIC);
+   intel_context_exit(ce);
+   if (IS_ERR(rq)) {
+   GT_TRACE(gt, "Failed to get bind request\n");
+   mutex_unlock(&ce->timeline->mutex);
+   goto put_ce;
+   }
+
+   cs = intel_ring_begin(rq, 2 * n_ptes + 2);
+   if (IS_ERR(cs)) {
+   GT_TRACE(gt, "Failed to ring space for GGTT bind\n");
+   i915_request_set_error_once(rq, PTR_ERR(cs));
+   /* once a request is created, it must be queued */
+ 

Re: [Intel-gfx] [PATCH 0/4] drm/i915/vlv_dsi: Add quirks for x86 android tablets (v2)

2023-09-18 Thread Ville Syrjälä
On Sat, Sep 16, 2023 at 02:54:51PM +0200, Hans de Goede wrote:
> Hi All,
> 
> Some vlv/chv tablets ship with Android as factory OS. The factory OS
> BSP style kernel on these tablets does not use the normal x86 hw
> autodetection instead it hardcodes a whole bunch of things including
> using panel drivers instead of relying on VBT MIPI sequences to
> turn the panel/backlight on/off.
> 
> The normal i915 driver (which does not use panel drivers) mostly works
> since the VBT still needs to contain valid info for the GOP, but because
> of the Android kernel relying on panel drivers with various things
> hardcoded some DMI quirks are necessary to fix some issues on these
> devices.
> 
> Some of these issues also are related to which I2C bus to use for
> MIPI sequence elements which do I2C transfers. This series also
> includes a patch adding some extra debugging to mipi_exec_i2c() to
> help with debugging similar issues in the future.
> 
> These patches have been posted before but back then I did not get around
> to follow up on the series:
> https://lore.kernel.org/intel-gfx/20220225214934.383168-1-hdego...@redhat.com/
> 
> Changes compared to this old version:
> - Drop the changes how the I2C bus number is found, instead just have
>   the quirks set the right number directly where necessary. This should
>   avoid any chances of causing regressions on devices where the quirks
>   do not apply.
> 
> - New quirk for backlight control issues on Lenovo Yoga Tab 3
> 
> - Address Jani Nikula's remark about __func__ being redundant when using
>   drm_dbg_kms()
> 
> 
> Regards,
> 
> Hans
> 
> 
> 
> Hans de Goede (4):
>   drm/i915/vlv_dsi: Add DMI quirk for wrong panel modeline in BIOS on
> Asus TF103C (v2)
>   drm/i915/vlv_dsi: Add DMI quirk for wrong I2C bus and panel size on
> Lenovo Yoga Tablet 2 series (v2)
>   drm/i915/vlv_dsi: Add DMI quirk for backlight control issues on Lenovo
> Yoga Tab 3

Please file a bug for each of these and attach the usual drm.debug=0xe
dmesg + VBT + any other relevant information there. Otherwise a few
years from now I'll be cursing at these commits as well for not leaving
a decent papertrail...

>   drm/i915/dsi: Add some debug logging to mipi_exec_i2c (v2)
> 
>  drivers/gpu/drm/i915/display/intel_dsi_vbt.c |   3 +
>  drivers/gpu/drm/i915/display/vlv_dsi.c   | 124 +++
>  2 files changed, 127 insertions(+)
> 
> -- 
> 2.41.0

-- 
Ville Syrjälä
Intel