RE: [PATCH v2] drm/amdgpu: set fb_modifiers_not_supported in vkms

[Huang, Tim]

[Public]

Reviewed-by: Tim Huang 

Best Regards,
Tim Huang



-Original Message-
From: amd-gfx  On Behalf Of Yifan Zhang
Sent: Monday, October 24, 2022 12:51 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Zhang, Yifan 
; Wentland, Harry ; Koenig, 
Christian ; Chen, Guchun 
Subject: [PATCH v2] drm/amdgpu: set fb_modifiers_not_supported in vkms

This patch to fix the gdm3 start failure with virual display:

/usr/libexec/gdm-x-session[1711]: (II) AMDGPU(0): Setting screen physical size 
to 270 x 203
/usr/libexec/gdm-x-session[1711]: (EE) AMDGPU(0): Failed to make import prime 
FD as pixmap: 22
/usr/libexec/gdm-x-session[1711]: (EE) AMDGPU(0): failed to set mode: Invalid 
argument
/usr/libexec/gdm-x-session[1711]: (WW) AMDGPU(0): Failed to set mode on CRTC 0
/usr/libexec/gdm-x-session[1711]: (EE) AMDGPU(0): Failed to enable any CRTC
gnome-shell[1840]: Running GNOME Shell (using mutter 42.2) as a X11 window and 
compositing manager
/usr/libexec/gdm-x-session[1711]: (EE) AMDGPU(0): failed to set mode: Invalid 
argument

vkms doesn't have modifiers support, set fb_modifiers_not_supported to bring 
the gdm back.

Signed-off-by: Yifan Zhang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
index 576849e95296..f69827aefb57 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
@@ -500,6 +500,8 @@ static int amdgpu_vkms_sw_init(void *handle)

adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base;

+   adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true;
+
r = amdgpu_display_modeset_create_props(adev);
if (r)
return r;
--
2.37.3

Re: [PATCH v2] kset: fix memory leak when kset_register() returns error

[Luben Tuikov]

On 2022-10-24 08:19, Yang Yingliang wrote:
> Inject fault while loading module, kset_register() may fail.
> If it fails, the name allocated by kobject_set_name() which
> is called before kset_register() is leaked, because refcount
> of kobject is hold in kset_init().
> 
> As a kset may be embedded in a larger structure which needs
> be freed in release() function or error path in callers, we
> can not call kset_put() in kset_register(), or it will cause
> double free, so just call kfree_const() to free the name and
> set it to NULL.
> 
> With this fix, the callers don't need to care about the name
> freeing and call an extra kset_put() if kset_register() fails.
> 
> Suggested-by: Luben Tuikov 
> Signed-off-by: Yang Yingliang 
> ---
> v1 -> v2:
>   Free name inside of kset_register() instead of calling kset_put()
>   in drivers.
> ---
>  lib/kobject.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/kobject.c b/lib/kobject.c
> index a0b2dbfcfa23..3409a89c81e5 100644
> --- a/lib/kobject.c
> +++ b/lib/kobject.c
> @@ -834,6 +834,9 @@ EXPORT_SYMBOL_GPL(kobj_sysfs_ops);
>  /**
>   * kset_register() - Initialize and add a kset.
>   * @k: kset.
> + *
> + * NOTE: On error, the kset.kobj.name allocated by() kobj_set_name()
> + * which is called before kset_register() in caller need be freed.
>   */

The "need be freed" is confusing here because it is not clear if the user
needs to do this or if it is done by the code. Since it is the latter,
it should read "_is_ freed". Like this (no "NOTE"):

"On error, the kset.kobj.name allocated by kobj_set_name(),
 which must be called before kset_register() is called, is freed
 by this function."

Regards,
Luben

>  int kset_register(struct kset *k)
>  {
> @@ -844,8 +847,11 @@ int kset_register(struct kset *k)
>  
>   kset_init(k);
>   err = kobject_add_internal(&k->kobj);
> - if (err)
> + if (err) {
> + kfree_const(k->kobj.name);
> + k->kobj.name = NULL;
>   return err;
> + }
>   kobject_uevent(&k->kobj, KOBJ_ADD);
>   return 0;
>  }

RE: [PATCH v2] drm/amdgpu: disallow gfxoff until GC IP blocks complete s2idle resume

[Liang, Prike]

[Public]

-Original Message-
From: Quan, Evan 
Sent: Monday, October 24, 2022 2:31 PM
To: Liang, Prike ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Liang, Prike 

Subject: RE: [PATCH v2] drm/amdgpu: disallow gfxoff until GC IP blocks complete 
s2idle resume

[AMD Official Use Only - General]



> -Original Message-
> From: amd-gfx  On Behalf Of
> Prike Liang
> Sent: Friday, October 21, 2022 10:47 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander ; Liang, Prike
> 
> Subject: [PATCH v2] drm/amdgpu: disallow gfxoff until GC IP blocks
> complete s2idle resume
>
> In the S2idle suspend/resume phase the gfxoff is keeping functional so
> some IP blocks will be likely to reinitialize at gfxoff entry and that
> will result in failing to program GC registers.Therefore, let disallow
> gfxoff until AMDGPU IPs reinitialized completely.
[Quan, Evan] It seems the issue described here has nothing related with 
suspend. Instead it happened during resuming only. Right?
If so, I would suggest to drop the confusing "suspend" from the description 
part.
Other than that, the patch is reviewed-by: Evan Quan 

Evan

[Prike] Yes this problem only observed on the s2idle resume period, but that's 
true the s2idle suspend and resume keep the gfxoff being functional.

>
> Signed-off-by: Prike Liang 
> ---
> -v2: Move the operation of exiting gfxoff from smu to higer layer in
> amdgpu_device.c.
>
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 5b8362727226..36c44625932e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -3210,6 +3210,12 @@ static int
> amdgpu_device_ip_resume_phase2(struct amdgpu_device *adev)
>   return r;
>   }
>   adev->ip_blocks[i].status.hw = true;
> +
> + if (adev->in_s0ix && adev->ip_blocks[i].version->type ==
> AMD_IP_BLOCK_TYPE_SMC) {
> + amdgpu_gfx_off_ctrl(adev, false);
> + DRM_DEBUG("will disable gfxoff for re-initializing
> other blocks\n");
> + }
> +
>   }
>
>   return 0;
> @@ -4185,6 +4191,10 @@ int amdgpu_device_resume(struct drm_device
> *dev, bool fbcon)
>   /* Make sure IB tests flushed */
>   flush_delayed_work(&adev->delayed_init_work);
>
> + if (adev->in_s0ix) {
> + amdgpu_gfx_off_ctrl(adev, true);
> + DRM_DEBUG("will enable gfxoff for the mission mode\n");
> + }
>   if (fbcon)
>
>   drm_fb_helper_set_suspend_unlocked(adev_to_drm(adev)-
> >fb_helper, false);
>
> --
> 2.25.1

Re: [linux-next:master] BUILD SUCCESS WITH WARNING 76cf65d1377f733af1e2a55233e3353ffa577f54

[Jakub Kicinski]

On Tue, 25 Oct 2022 00:58:57 +0800 kernel test robot wrote:
> drivers/net/phy/phylink.c:588 phylink_validate_mask_caps() warn: variable 
> dereferenced before check 'state' (see line 583)

Hi Russell, I think the warning is semi-legit. Your commit f392a1846489
("net: phylink: provide phylink_validate_mask_caps() helper") added an 
if (state) before defer'ing state but it's already deref'ed higher up
so can't be null.

Re: [PATCH v2] kset: fix memory leak when kset_register() returns error

[Luben Tuikov]

On 2022-10-24 17:06, Luben Tuikov wrote:
> On 2022-10-24 08:19, Yang Yingliang wrote:
>> Inject fault while loading module, kset_register() may fail.
>> If it fails, the name allocated by kobject_set_name() which
>> is called before kset_register() is leaked, because refcount
>> of kobject is hold in kset_init().
> 
> "is hold" --> "was set".
> 
> Also, I'd say "which must be called" instead of "is", since
> we cannot register kobj/kset without a name--the kobj code crashes,
> and we want to make this clear. IOW, a novice user may wonder
> where "is" it called, as opposed to learning that they "must"
> call it to allocate/set a name, before calling kset_register().
> 
> So, I'd say this:
> 
> "If it fails, the name allocated by kobject_set_name() which must
>  be called before a call to kset_regsiter() is leaked, since
>  refcount of kobj was set in kset_init()."

Actually, to be a bit more clear:

"If kset_register() fails, the name allocated by kobject_set_name(),
 namely kset.kobj.name, which must be called before a call to kset_register(),
 may be leaked, if the caller doesn't explicitly free it, say by calling 
kset_put().

 To mitigate this, we free the name in kset_register() when an error is 
encountered,
 i.e. when kset_register() returns an error."

> 
>>
>> As a kset may be embedded in a larger structure which needs
>> be freed in release() function or error path in callers, we
> 
> Drop "As", start with "A kset". "which needs _to_ be".
> Also please specify that the release is part of the ktype,
> like this:
> 
> "A kset may be embedded in a larger structure which needs to be
>  freed in ktype.release() or error path in callers, we ..."
> 
>> can not call kset_put() in kset_register(), or it will cause
>> double free, so just call kfree_const() to free the name and
>> set it to NULL.
>>
>> With this fix, the callers don't need to care about the name
>> freeing and call an extra kset_put() if kset_register() fails.
> 
> This is unclear because you're *missing* a verb:
> "and call an extra kset_put()".
> Please add the proper verb _between_ "and call", something like,
> 
> "With this fix, the callers don't need to care about freeing
>  the name of the kset, and _can_ call kset_put() if kset_register() fails."
> 
> Choose a proper verb here: can, should, cannot, should not, etc.
> 
> We can do this because you set "kset.kobj.name to NULL, and this
> is checked for in kobject_cleanup(). We just need to stipulate
> whether they should/shouldn't have to call kset_put(), or can free the kset
> and/or the embedding object themselves. This really depends
> on how we want kset_register() to behave in the future, and on
> user's own ktype.release implementation...

Forgot "may", "may not".

So, do we want to say "may call kset_put()", like:

"With this fix, the callers need not care about freeing
 the name of the kset, and _may_ call kset_put() if kset_register() fails."

Or do we want to say "should" or even "must"--it really depends on
what else is (would be) going on in kobj registration.

Although, the user may have additional work to be done in the ktype.release()
callback for the embedding object. It would be good to give them the freedom,
i.e. "may", to call kset_put(). If that's not the case, this must be explicitly
stipulated with the proper verb.

Regards,
Luben

Re: [PATCH v2] kset: fix memory leak when kset_register() returns error

[Luben Tuikov]

On 2022-10-24 08:19, Yang Yingliang wrote:
> Inject fault while loading module, kset_register() may fail.
> If it fails, the name allocated by kobject_set_name() which
> is called before kset_register() is leaked, because refcount
> of kobject is hold in kset_init().

"is hold" --> "was set".

Also, I'd say "which must be called" instead of "is", since
we cannot register kobj/kset without a name--the kobj code crashes,
and we want to make this clear. IOW, a novice user may wonder
where "is" it called, as opposed to learning that they "must"
call it to allocate/set a name, before calling kset_register().

So, I'd say this:

"If it fails, the name allocated by kobject_set_name() which must
 be called before a call to kset_regsiter() is leaked, since
 refcount of kobj was set in kset_init()."

> 
> As a kset may be embedded in a larger structure which needs
> be freed in release() function or error path in callers, we

Drop "As", start with "A kset". "which needs _to_ be".
Also please specify that the release is part of the ktype,
like this:

"A kset may be embedded in a larger structure which needs to be
 freed in ktype.release() or error path in callers, we ..."

> can not call kset_put() in kset_register(), or it will cause
> double free, so just call kfree_const() to free the name and
> set it to NULL.
> 
> With this fix, the callers don't need to care about the name
> freeing and call an extra kset_put() if kset_register() fails.

This is unclear because you're *missing* a verb:
"and call an extra kset_put()".
Please add the proper verb _between_ "and call", something like,

"With this fix, the callers don't need to care about freeing
 the name of the kset, and _can_ call kset_put() if kset_register() fails."

Choose a proper verb here: can, should, cannot, should not, etc.

We can do this because you set "kset.kobj.name to NULL, and this
is checked for in kobject_cleanup(). We just need to stipulate
whether they should/shouldn't have to call kset_put(), or can free the kset
and/or the embedding object themselves. This really depends
on how we want kset_register() to behave in the future, and on
user's own ktype.release implementation...

> 
> Suggested-by: Luben Tuikov 
> Signed-off-by: Yang Yingliang 
> ---
> v1 -> v2:
>   Free name inside of kset_register() instead of calling kset_put()
>   in drivers.
> ---
>  lib/kobject.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/kobject.c b/lib/kobject.c
> index a0b2dbfcfa23..3409a89c81e5 100644
> --- a/lib/kobject.c
> +++ b/lib/kobject.c
> @@ -834,6 +834,9 @@ EXPORT_SYMBOL_GPL(kobj_sysfs_ops);
>  /**
>   * kset_register() - Initialize and add a kset.
>   * @k: kset.
> + *
> + * NOTE: On error, the kset.kobj.name allocated by() kobj_set_name()
> + * which is called before kset_register() in caller need be freed.
>   */
>  int kset_register(struct kset *k)
>  {
> @@ -844,8 +847,11 @@ int kset_register(struct kset *k)
>  
>   kset_init(k);
>   err = kobject_add_internal(&k->kobj);
> - if (err)
> + if (err) {
> + kfree_const(k->kobj.name);
> + k->kobj.name = NULL;
>   return err;
> + }

This looks good. It's good you set kset.kobj.name to NULL, so that
recovery/free paths don't get confused. Waiting for v3.

(I guess this is no different than what we currently do in kobject_cleanup(),
 so I see it as safe, no-surprises implementation.)

Regards,
Luben

Re: [PATCH 2/2] drm/amdkfd: Fix the warning of array-index-out-of-bounds

[Felix Kuehling]

On 2022-10-24 07:26, Ma Jun wrote:

For some GPUs with more CUs, the original sibling_map[32]
in struct crat_subtype_cache is not enough
to save the cache information when create the VCRAT table,
so fill the cache info into struct kfd_cache_properties_ext
directly to fix the problem.

At the same time, a new directory
"/sys/class/kfd/kfd/topology/nodes/*nodes_num*/caches_ext"
is created for cache information showing.


Is this necessary because existing user mode cannot handle the larger 
sibling map? If so, you'll also need to update the Thunk to parse the 
information from caches_ext. Do you have a link to that patch?


If user mode can handle a larger siblingmap in the existing sysfs 
interface, we should not create new one.


Another alternative is to add more lines to the existing cache info 
sysfs entries. Older user mode will probably ignore the ones it doesn't 
know about.


Regards,
  Felix




The original directory "cache" is reserved for GPU which using real CRAT
table.

Signed-off-by: Ma Jun 
---
  drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 1229 +---
  drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 1246 -
  drivers/gpu/drm/amd/amdkfd/kfd_topology.h |   21 +
  3 files changed, 1261 insertions(+), 1235 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
index 4857ec5b9f46..e6928c60338e 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
@@ -30,799 +30,6 @@
  #include "amdgpu.h"
  #include "amdgpu_amdkfd.h"
  
-/* Static table to describe GPU Cache information */

-struct kfd_gpu_cache_info {
-   uint32_tcache_size;
-   uint32_tcache_level;
-   uint32_tflags;
-   /* Indicates how many Compute Units share this cache
-* within a SA. Value = 1 indicates the cache is not shared
-*/
-   uint32_tnum_cu_shared;
-};
-
-static struct kfd_gpu_cache_info kaveri_cache_info[] = {
-   {
-   /* TCP L1 Cache per CU */
-   .cache_size = 16,
-   .cache_level = 1,
-   .flags = (CRAT_CACHE_FLAGS_ENABLED |
-   CRAT_CACHE_FLAGS_DATA_CACHE |
-   CRAT_CACHE_FLAGS_SIMD_CACHE),
-   .num_cu_shared = 1,
-   },
-   {
-   /* Scalar L1 Instruction Cache (in SQC module) per bank */
-   .cache_size = 16,
-   .cache_level = 1,
-   .flags = (CRAT_CACHE_FLAGS_ENABLED |
-   CRAT_CACHE_FLAGS_INST_CACHE |
-   CRAT_CACHE_FLAGS_SIMD_CACHE),
-   .num_cu_shared = 2,
-   },
-   {
-   /* Scalar L1 Data Cache (in SQC module) per bank */
-   .cache_size = 8,
-   .cache_level = 1,
-   .flags = (CRAT_CACHE_FLAGS_ENABLED |
-   CRAT_CACHE_FLAGS_DATA_CACHE |
-   CRAT_CACHE_FLAGS_SIMD_CACHE),
-   .num_cu_shared = 2,
-   },
-
-   /* TODO: Add L2 Cache information */
-};
-
-
-static struct kfd_gpu_cache_info carrizo_cache_info[] = {
-   {
-   /* TCP L1 Cache per CU */
-   .cache_size = 16,
-   .cache_level = 1,
-   .flags = (CRAT_CACHE_FLAGS_ENABLED |
-   CRAT_CACHE_FLAGS_DATA_CACHE |
-   CRAT_CACHE_FLAGS_SIMD_CACHE),
-   .num_cu_shared = 1,
-   },
-   {
-   /* Scalar L1 Instruction Cache (in SQC module) per bank */
-   .cache_size = 8,
-   .cache_level = 1,
-   .flags = (CRAT_CACHE_FLAGS_ENABLED |
-   CRAT_CACHE_FLAGS_INST_CACHE |
-   CRAT_CACHE_FLAGS_SIMD_CACHE),
-   .num_cu_shared = 4,
-   },
-   {
-   /* Scalar L1 Data Cache (in SQC module) per bank. */
-   .cache_size = 4,
-   .cache_level = 1,
-   .flags = (CRAT_CACHE_FLAGS_ENABLED |
-   CRAT_CACHE_FLAGS_DATA_CACHE |
-   CRAT_CACHE_FLAGS_SIMD_CACHE),
-   .num_cu_shared = 4,
-   },
-
-   /* TODO: Add L2 Cache information */
-};
-
-#define hawaii_cache_info kaveri_cache_info
-#define tonga_cache_info carrizo_cache_info
-#define fiji_cache_info  carrizo_cache_info
-#define polaris10_cache_info carrizo_cache_info
-#define polaris11_cache_info carrizo_cache_info
-#define polaris12_cache_info carrizo_cache_info
-#define vegam_cache_info carrizo_cache_info
-
-/* NOTE: L1 cache information has been updated and L2/L3
- * cache information has been added for Vega10 and
- * newer ASICs. The unit for cache_size is KiB.
- * In future,  check & update cache details
- * for every new ASIC is required.
- */
-
-static struct kfd_gpu_cache_info vega10_cache_info[] = {
-   

Re: [PATCH 1/2] drm/amdkfd: Init the base cu processor id

[Felix Kuehling]

On 2022-10-24 07:26, Ma Jun wrote:

Init and save the base cu processor id for later use

Signed-off-by: Ma Jun 
---
  drivers/gpu/drm/amd/amdkfd/kfd_crat.c   | 24 +
  drivers/gpu/drm/amd/amdkfd/kfd_device.c | 28 +
  drivers/gpu/drm/amd/amdkfd/kfd_priv.h   |  3 +++
  3 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
index d25ac9cbe5b2..4857ec5b9f46 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
@@ -30,26 +30,6 @@
  #include "amdgpu.h"
  #include "amdgpu_amdkfd.h"
  
-/* GPU Processor ID base for dGPUs for which VCRAT needs to be created.

- * GPU processor ID are expressed with Bit[31]=1.
- * The base is set to 0x8000_ + 0x1000 to avoid collision with GPU IDs
- * used in the CRAT.
- */
-static uint32_t gpu_processor_id_low = 0x80001000;
-
-/* Return the next available gpu_processor_id and increment it for next GPU
- * @total_cu_count - Total CUs present in the GPU including ones
- *   masked off
- */
-static inline unsigned int get_and_inc_gpu_processor_id(
-   unsigned int total_cu_count)
-{
-   int current_id = gpu_processor_id_low;
-
-   gpu_processor_id_low += total_cu_count;
-   return current_id;
-}
-
  /* Static table to describe GPU Cache information */
  struct kfd_gpu_cache_info {
uint32_tcache_size;
@@ -2223,7 +2203,6 @@ static int kfd_create_vcrat_image_gpu(void *pcrat_image,
struct crat_subtype_computeunit *cu;
struct kfd_cu_info cu_info;
int avail_size = *size;
-   uint32_t total_num_of_cu;
int num_of_cache_entries = 0;
int cache_mem_filled = 0;
uint32_t nid = 0;
@@ -2275,8 +2254,7 @@ static int kfd_create_vcrat_image_gpu(void *pcrat_image,
cu->wave_front_size = cu_info.wave_front_size;
cu->array_count = cu_info.num_shader_arrays_per_engine *
cu_info.num_shader_engines;
-   total_num_of_cu = (cu->array_count * cu_info.num_cu_per_sh);
-   cu->processor_id_low = get_and_inc_gpu_processor_id(total_num_of_cu);
+   cu->processor_id_low = kdev->processor_id_low;


I don't understand why you can't leave the call to get_and_inc here. You 
could set kdev->processor_id_low here. Or set it when parsing the CRAT 
table later. Setting it in kdev before creating the topology seems 
backwards.




cu->num_cu_per_array = cu_info.num_cu_per_sh;
cu->max_slots_scatch_cu = cu_info.max_scratch_slots_per_cu;
cu->num_banks = cu_info.num_shader_engines;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index ae1f0be3f3a1..099df4598a42 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -38,6 +38,32 @@
  
  #define MQD_SIZE_ALIGNED 768
  
+/* GPU Processor ID base for dGPUs for which VCRAT needs to be created.

+ * GPU processor ID are expressed with Bit[31]=1.
+ * The base is set to 0x8000_ + 0x1000 to avoid collision with GPU IDs
+ * used in the CRAT.
+ */
+static uint32_t gpu_processor_id_low = 0x80001000;
+
+/* Return the next available gpu_processor_id and increment it for next GPU
+ * @total_cu_count - Total CUs present in the GPU including ones
+ *   masked off
+ */
+static inline unsigned int get_and_inc_gpu_processor_id(struct kfd_dev *kfd)
+{
+   struct amdgpu_device *adev = kfd->adev;
+   unsigned int array_count = 0;
+   unsigned int total_cu_count = 0;
+
+   kfd->processor_id_low = gpu_processor_id_low;
+
+   array_count = adev->gfx.config.max_sh_per_se * 
adev->gfx.config.max_shader_engines;
+   total_cu_count = array_count * adev->gfx.config.max_cu_per_sh;
+
+   gpu_processor_id_low += total_cu_count;


Can this function fun on two threads concurrently? If yes, you may need 
a lock here.




+   return 0;


If this function cannot fail, you should make it return void.



+}
+
  /*
   * kfd_locked is used to lock the kfd driver during suspend or reset
   * once locked, kfd driver will stop any further GPU execution.
@@ -656,6 +682,8 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
  
  	amdgpu_amdkfd_get_local_mem_info(kfd->adev, &kfd->local_mem_info);
  
+	get_and_inc_gpu_processor_id(kfd);


You're ignoring the return value. Since the function cannot fail, make 
it void. The name "get_..." implies that it should return something, 
though, so maybe change the name. E.g. assign_gpu_processor_id.


Regards,
  Felix



+
if (kfd_topology_add_device(kfd)) {
dev_err(kfd_device, "Error adding device to topology\n");
goto kfd_topology_add_device_error;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 182eb67edbc5..4c06b233472f 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/dr

[PATCH] drm/amdgpu: Disable GPU reset on SRIOV before remove pci.

[Gavin Wan]

  The change of the commit f5c7e7797060 ("Adjust removal control
  flow for smu v13_0_2") brought a bug on SRIOV envrionment. It
  caused unloading amdgpu failed on Guest VM. The reason is that
  the VF FLR was requested while unloading amdgpu driver, but the
  VF FLR of SRIOV sequence is wrong while removing PCI device.

  Fixes: f5c7e7797060 ("drm/amdgpu: Adjust removal control flow
 for smu v13_0_2")

Acked-by: Alex Deucher 
Signed-off-by: Gavin Wan 
Change-Id: I1ff8dcbffd85d7f3d8267d660fd8292423d2f70f
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 16f6a313335e..ab0c856c13b0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2187,7 +2187,8 @@ amdgpu_pci_remove(struct pci_dev *pdev)
pm_runtime_forbid(dev->dev);
}
 
-   if (adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0, 2)) {
+   if ((adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0, 2)) &&
+   !amdgpu_sriov_vf(adev)) {
bool need_to_reset_gpu = false;
 
if (adev->gmc.xgmi.num_physical_nodes > 1) {
-- 
2.34.1

Re: [PATCH] drm/amdgpu: Disable GPU reset on SRIOV before remove pci.

[Alex Deucher]

On Mon, Oct 24, 2022 at 4:03 PM Gavin Wan  wrote:
>
>   Fixes: f5c7e7797060 ("drm/amdgpu: Adjust removal control flow
>  for smu v13_0_2")
>
>   The change of the commit f5c7e7797060 ("Adjust removal control
>   flow for smu v13_0_2") brought a bug on SRIOV envrionment. It
>   caused unloading amdgpu failed on Guest VM. The reason is that
>   the VF FLR was requested while unloading amdgpu driver, but the
>   VF FLR of SRIOV sequence is wrong while removing PCI device.

Please move the Fixes line down below the patch description and above
the Signed-off-by/Reviewed-by/Acked-by/etc. tags.  Feel free to
add:
Acked-by: Alex Deucher 

Thanks,

Alex

>
> Signed-off-by: Gavin Wan 
> Change-Id: I1ff8dcbffd85d7f3d8267d660fd8292423d2f70f
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 16f6a313335e..ab0c856c13b0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2187,7 +2187,8 @@ amdgpu_pci_remove(struct pci_dev *pdev)
> pm_runtime_forbid(dev->dev);
> }
>
> -   if (adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0, 2)) {
> +   if ((adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0, 2)) &&
> +   !amdgpu_sriov_vf(adev)) {
> bool need_to_reset_gpu = false;
>
> if (adev->gmc.xgmi.num_physical_nodes > 1) {
> --
> 2.34.1
>

[PATCH] drm/amdgpu: Disable GPU reset on SRIOV before remove pci.

[Gavin Wan]

  Fixes: f5c7e7797060 ("drm/amdgpu: Adjust removal control flow
 for smu v13_0_2")

  The change of the commit f5c7e7797060 ("Adjust removal control
  flow for smu v13_0_2") brought a bug on SRIOV envrionment. It
  caused unloading amdgpu failed on Guest VM. The reason is that
  the VF FLR was requested while unloading amdgpu driver, but the
  VF FLR of SRIOV sequence is wrong while removing PCI device.

Signed-off-by: Gavin Wan 
Change-Id: I1ff8dcbffd85d7f3d8267d660fd8292423d2f70f
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 16f6a313335e..ab0c856c13b0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2187,7 +2187,8 @@ amdgpu_pci_remove(struct pci_dev *pdev)
pm_runtime_forbid(dev->dev);
}
 
-   if (adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0, 2)) {
+   if ((adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0, 2)) &&
+   !amdgpu_sriov_vf(adev)) {
bool need_to_reset_gpu = false;
 
if (adev->gmc.xgmi.num_physical_nodes > 1) {
-- 
2.34.1

Re: [PATCH] drm/amdgpu: Disable GPU reset on SRIOV before remove pci.

[Alex Deucher]

On Mon, Oct 24, 2022 at 3:45 PM Gavin Wan  wrote:
>
>   The change "Adjust removal control flow for smu v13_0_2"

commit f5c7e7797060 ("drm/amdgpu: Adjust removal control flow for smu v13_0_2")

>   brought a bug on SRIOV envrionment. It caused unloading
>   amdgpu failed on Guest VM. The reason is that the VF FLR was
>   requested while unloading amdgpu driver, but VF FLR of SRIOV
>   sequence is wrong while removing PCI device.
>
> Signed-off-by: Gavin Wan 

Please add:
Fixes: f5c7e7797060 ("drm/amdgpu: Adjust removal control flow for smu v13_0_2")

With that,
Acked-by: Alex Deucher 

> Change-Id: I1ff8dcbffd85d7f3d8267d660fd8292423d2f70f
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 16f6a313335e..ab0c856c13b0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2187,7 +2187,8 @@ amdgpu_pci_remove(struct pci_dev *pdev)
> pm_runtime_forbid(dev->dev);
> }
>
> -   if (adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0, 2)) {
> +   if ((adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0, 2)) &&
> +   !amdgpu_sriov_vf(adev)) {
> bool need_to_reset_gpu = false;
>
> if (adev->gmc.xgmi.num_physical_nodes > 1) {
> --
> 2.34.1
>

[PATCH] drm/amdgpu: Disable GPU reset on SRIOV before remove pci.

[Gavin Wan]

  The change "Adjust removal control flow for smu v13_0_2"
  brought a bug on SRIOV envrionment. It caused unloading
  amdgpu failed on Guest VM. The reason is that the VF FLR was
  requested while unloading amdgpu driver, but VF FLR of SRIOV
  sequence is wrong while removing PCI device.

Signed-off-by: Gavin Wan 
Change-Id: I1ff8dcbffd85d7f3d8267d660fd8292423d2f70f
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 16f6a313335e..ab0c856c13b0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2187,7 +2187,8 @@ amdgpu_pci_remove(struct pci_dev *pdev)
pm_runtime_forbid(dev->dev);
}
 
-   if (adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0, 2)) {
+   if ((adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0, 2)) &&
+   !amdgpu_sriov_vf(adev)) {
bool need_to_reset_gpu = false;
 
if (adev->gmc.xgmi.num_physical_nodes > 1) {
-- 
2.34.1

[PATCH 3/3] Revert "drm/amd/display: Limit max DSC target bpp for specific monitors"

[Hamza Mahfooz]

This reverts commit 55eea8ef98641f6e1e1c202bd3a49a57c1dd4059.

This quirk is now handled in the DRM core, so we can drop all of
the internal code that was added to handle it.

Signed-off-by: Hamza Mahfooz 
---
 .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 35 ---
 1 file changed, 35 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index 4956a0118215..a21e2ba77ddb 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -41,39 +41,6 @@
 #include "dm_helpers.h"
 #include "ddc_service_types.h"
 
-struct monitor_patch_info {
-   unsigned int manufacturer_id;
-   unsigned int product_id;
-   void (*patch_func)(struct dc_edid_caps *edid_caps, unsigned int param);
-   unsigned int patch_param;
-};
-static void set_max_dsc_bpp_limit(struct dc_edid_caps *edid_caps, unsigned int 
param);
-
-static const struct monitor_patch_info monitor_patch_table[] = {
-{0x6D1E, 0x5BBF, set_max_dsc_bpp_limit, 15},
-{0x6D1E, 0x5B9A, set_max_dsc_bpp_limit, 15},
-};
-
-static void set_max_dsc_bpp_limit(struct dc_edid_caps *edid_caps, unsigned int 
param)
-{
-   if (edid_caps)
-   edid_caps->panel_patch.max_dsc_target_bpp_limit = param;
-}
-
-static int amdgpu_dm_patch_edid_caps(struct dc_edid_caps *edid_caps)
-{
-   int i, ret = 0;
-
-   for (i = 0; i < ARRAY_SIZE(monitor_patch_table); i++)
-   if ((edid_caps->manufacturer_id == 
monitor_patch_table[i].manufacturer_id)
-   &&  (edid_caps->product_id == 
monitor_patch_table[i].product_id)) {
-   monitor_patch_table[i].patch_func(edid_caps, 
monitor_patch_table[i].patch_param);
-   ret++;
-   }
-
-   return ret;
-}
-
 /* dm_helpers_parse_edid_caps
  *
  * Parse edid caps
@@ -148,8 +115,6 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
kfree(sads);
kfree(sadb);
 
-   amdgpu_dm_patch_edid_caps(edid_caps);
-
return result;
 }
 
-- 
2.38.0

[PATCH 2/3] drm/amd/display: use max_dsc_bpp in amdgpu_dm

[Hamza Mahfooz]

Since, the quirk is handled in the DRM core now, we can use that value
instead of the internal value.

Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  6 ++
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c   | 11 +--
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 63f076a46260..9b9cca8cb71a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5618,16 +5618,14 @@ static void apply_dsc_policy_for_stream(struct 
amdgpu_dm_connector *aconnector,
 {
struct drm_connector *drm_connector = &aconnector->base;
uint32_t link_bandwidth_kbps;
-   uint32_t max_dsc_target_bpp_limit_override = 0;
struct dc *dc = sink->ctx->dc;
uint32_t max_supported_bw_in_kbps, timing_bw_in_kbps;
uint32_t dsc_max_supported_bw_in_kbps;
+   uint32_t max_dsc_target_bpp_limit_override =
+   drm_connector->display_info.max_dsc_bpp;
 
link_bandwidth_kbps = dc_link_bandwidth_kbps(aconnector->dc_link,

dc_link_get_link_cap(aconnector->dc_link));
-   if (stream->link && stream->link->local_sink)
-   max_dsc_target_bpp_limit_override =
-   
stream->link->local_sink->edid_caps.panel_patch.max_dsc_target_bpp_limit;
 
/* Set DSC policy according to dsc_clock_en */
dc_dsc_policy_set_enable_dsc_when_not_needed(
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index ce6929224a6e..eb42c0e21a28 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -670,15 +670,18 @@ static void set_dsc_configs_from_fairness_vars(struct 
dsc_mst_fairness_params *p
int count,
int k)
 {
+   struct drm_connector *drm_connector;
int i;
 
for (i = 0; i < count; i++) {
+   drm_connector = ¶ms[i].aconnector->base;
+
memset(¶ms[i].timing->dsc_cfg, 0, 
sizeof(params[i].timing->dsc_cfg));
if (vars[i + k].dsc_enabled && dc_dsc_compute_config(

params[i].sink->ctx->dc->res_pool->dscs[0],
¶ms[i].sink->dsc_caps.dsc_dec_caps,

params[i].sink->ctx->dc->debug.dsc_min_slice_height_override,
-   
params[i].sink->edid_caps.panel_patch.max_dsc_target_bpp_limit,
+   drm_connector->display_info.max_dsc_bpp,
0,
params[i].timing,
¶ms[i].timing->dsc_cfg)) {
@@ -720,12 +723,16 @@ static int bpp_x16_from_pbn(struct 
dsc_mst_fairness_params param, int pbn)
struct dc_dsc_config dsc_config;
u64 kbps;
 
+   struct drm_connector *drm_connector = ¶m.aconnector->base;
+   uint32_t max_dsc_target_bpp_limit_override =
+   drm_connector->display_info.max_dsc_bpp;
+
kbps = div_u64((u64)pbn * 994 * 8 * 54, 64);
dc_dsc_compute_config(
param.sink->ctx->dc->res_pool->dscs[0],
¶m.sink->dsc_caps.dsc_dec_caps,

param.sink->ctx->dc->debug.dsc_min_slice_height_override,
-   
param.sink->edid_caps.panel_patch.max_dsc_target_bpp_limit,
+   max_dsc_target_bpp_limit_override,
(int) kbps, param.timing, &dsc_config);
 
return dsc_config.bits_per_pixel;
-- 
2.38.0

[PATCH 1/3] drm/edid: add a quirk for two LG monitors to get them to work on 10bpc

[Hamza Mahfooz]

The LG 27GP950 and LG 27GN950 have visible display corruption when
trying to use 10bpc modes. So, to fix this, cap their maximum DSC
target bitrate to 15bpp.

Suggested-by: Roman Li 
Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/drm_edid.c  | 12 
 include/drm/drm_connector.h |  6 ++
 2 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index bc43e1b32092..f4f96115dce7 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -91,6 +91,8 @@ static int oui(u8 first, u8 second, u8 third)
 #define EDID_QUIRK_FORCE_10BPC (1 << 11)
 /* Non desktop display (i.e. HMD) */
 #define EDID_QUIRK_NON_DESKTOP (1 << 12)
+/* Cap the DSC target bitrate to 15bpp */
+#define EDID_QUIRK_CAP_DSC_15BPP   (1 << 13)
 
 #define MICROSOFT_IEEE_OUI 0xca125c
 
@@ -151,6 +153,12 @@ static const struct edid_quirk {
EDID_QUIRK('F', 'C', 'M', 13600, EDID_QUIRK_PREFER_LARGE_75 |
   EDID_QUIRK_DETAILED_IN_CM),
 
+   /* LG 27GP950 */
+   EDID_QUIRK('G', 'S', 'M', 0x5bbf, EDID_QUIRK_CAP_DSC_15BPP),
+
+   /* LG 27GN950 */
+   EDID_QUIRK('G', 'S', 'M', 0x5b9a, EDID_QUIRK_CAP_DSC_15BPP),
+
/* LGD panel of HP zBook 17 G2, eDP 10 bpc, but reports unknown bpc */
EDID_QUIRK('L', 'G', 'D', 764, EDID_QUIRK_FORCE_10BPC),
 
@@ -5511,6 +5519,7 @@ drm_reset_display_info(struct drm_connector *connector)
 
info->mso_stream_count = 0;
info->mso_pixel_overlap = 0;
+   info->max_dsc_bpp = 0;
 }
 
 u32 drm_add_display_info(struct drm_connector *connector, const struct edid 
*edid)
@@ -5595,6 +5604,9 @@ u32 drm_add_display_info(struct drm_connector *connector, 
const struct edid *edi
info->non_desktop = true;
}
 
+   if (quirks & EDID_QUIRK_CAP_DSC_15BPP)
+   info->max_dsc_bpp = 15;
+
return quirks;
 }
 
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 3ac4bf87f257..7a8fb486b6ab 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -634,6 +634,12 @@ struct drm_display_info {
 * @mso_pixel_overlap: eDP MSO segment pixel overlap, 0-8 pixels.
 */
u8 mso_pixel_overlap;
+
+   /**
+* @max_dsc_bpp: Maximum DSC target bitrate, if it is set to 0 the
+* monitor's default value is used instead.
+*/
+   u32 max_dsc_bpp;
 };
 
 int drm_display_info_set_bus_formats(struct drm_display_info *info,
-- 
2.38.0

Re: [PATCH v2] drm/amd/display: Revert logic for plane modifiers

[Alex Deucher]

Applied.  Thanks!

Alex

On Mon, Oct 24, 2022 at 9:17 AM Joaquín Ignacio Aramendía
 wrote:
>
> This file was split in commit 5d945cbcd4b16a29d6470a80dfb19738f9a4319f
> ("drm/amd/display: Create a file dedicated to planes") and the logic in
> dm_plane_format_mod_supported() function got changed by a switch logic.
> That change broke drm_plane modifiers setting on series 5000 APUs
> (tested on OXP mini AMD 5800U and HP Dev One 5850U PRO)
> leading to Gamescope not working as reported on GitHub[1]
>
> To reproduce the issue, enter a TTY and run:
>
> $ gamescope -- vkcube
>
> With said commit applied it will abort. This one restores the old logic,
> fixing the issue that affects Gamescope.
>
> [1](https://github.com/Plagman/gamescope/issues/624)
>
> Cc:  # 6.0.x
> Signed-off-by: Joaquín Ignacio Aramendía 
> Reviewed-by: Bas Nieuwenhuizen 
> ---
> Removed asic_id and excess newlines. Resend with correct Cc line.
> ---
>  .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 50 +++
>  1 file changed, 7 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> index dfd3be49eac8..e6854f7270a6 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> @@ -1369,7 +1369,7 @@ static bool dm_plane_format_mod_supported(struct 
> drm_plane *plane,
>  {
> struct amdgpu_device *adev = drm_to_adev(plane->dev);
> const struct drm_format_info *info = drm_format_info(format);
> -   struct hw_asic_id asic_id = adev->dm.dc->ctx->asic_id;
> +   int i;
>
> enum dm_micro_swizzle microtile = 
> modifier_gfx9_swizzle_mode(modifier) & 3;
>
> @@ -1386,49 +1386,13 @@ static bool dm_plane_format_mod_supported(struct 
> drm_plane *plane,
> return true;
> }
>
> -   /* check if swizzle mode is supported by this version of DCN */
> -   switch (asic_id.chip_family) {
> -   case FAMILY_SI:
> -   case FAMILY_CI:
> -   case FAMILY_KV:
> -   case FAMILY_CZ:
> -   case FAMILY_VI:
> -   /* asics before AI does not have modifier support */
> -   return false;
> -   case FAMILY_AI:
> -   case FAMILY_RV:
> -   case FAMILY_NV:
> -   case FAMILY_VGH:
> -   case FAMILY_YELLOW_CARP:
> -   case AMDGPU_FAMILY_GC_10_3_6:
> -   case AMDGPU_FAMILY_GC_10_3_7:
> -   switch (AMD_FMT_MOD_GET(TILE, modifier)) {
> -   case AMD_FMT_MOD_TILE_GFX9_64K_R_X:
> -   case AMD_FMT_MOD_TILE_GFX9_64K_D_X:
> -   case AMD_FMT_MOD_TILE_GFX9_64K_S_X:
> -   case AMD_FMT_MOD_TILE_GFX9_64K_D:
> -   return true;
> -   default:
> -   return false;
> -   }
> -   break;
> -   case AMDGPU_FAMILY_GC_11_0_0:
> -   case AMDGPU_FAMILY_GC_11_0_1:
> -   switch (AMD_FMT_MOD_GET(TILE, modifier)) {
> -   case AMD_FMT_MOD_TILE_GFX11_256K_R_X:
> -   case AMD_FMT_MOD_TILE_GFX9_64K_R_X:
> -   case AMD_FMT_MOD_TILE_GFX9_64K_D_X:
> -   case AMD_FMT_MOD_TILE_GFX9_64K_S_X:
> -   case AMD_FMT_MOD_TILE_GFX9_64K_D:
> -   return true;
> -   default:
> -   return false;
> -   }
> -   break;
> -   default:
> -   ASSERT(0); /* Unknown asic */
> -   break;
> +   /* Check that the modifier is on the list of the plane's supported 
> modifiers. */
> +   for (i = 0; i < plane->modifier_count; i++) {
> +   if (modifier == plane->modifiers[i])
> +   break;
> }
> +   if (i == plane->modifier_count)
> +   return false;
>
> /*
>  * For D swizzle the canonical modifier depends on the bpp, so check
> --
> 2.38.1
>

[PATCH] drm/amd/display: fix documentation warning

[Alex Deucher]

build htmldocs produced this warning:
drivers/gpu/drm/amd/display/dc/dc.h:1275: warning: cannot understand function 
prototype: 'struct dc_validation_set '
The word "struct" was left out of the comment.

Fixes: f4a59996c408 ("drm/amd/display: Include surface of unaffected streams")
Reported-by: Stephen Rothwell 
Signed-off-by: Alex Deucher 
Cc: Rodrigo Siqueira 
---
 drivers/gpu/drm/amd/display/dc/dc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dc.h 
b/drivers/gpu/drm/amd/display/dc/dc.h
index bd7a896fab49..e4e41f2e5054 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -1269,7 +1269,7 @@ void dc_post_update_surfaces_to_stream(
 #include "dc_stream.h"
 
 /**
- * dc_validation_set - Struct to store surface/stream associations for 
validation
+ * struct dc_validation_set - Struct to store surface/stream associations for 
validation
  */
 struct dc_validation_set {
/**
-- 
2.37.3

[linux-next:master] BUILD SUCCESS WITH WARNING 76cf65d1377f733af1e2a55233e3353ffa577f54

[kernel test robot]

tree/branch: 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
branch HEAD: 76cf65d1377f733af1e2a55233e3353ffa577f54  Add linux-next specific 
files for 20221024

Warning reports:

https://lore.kernel.org/linux-mm/202210090954.ptr6m6rj-...@intel.com
https://lore.kernel.org/linux-mm/202210110857.9s0txvnn-...@intel.com
https://lore.kernel.org/linux-mm/202210111318.mbufyhps-...@intel.com
https://lore.kernel.org/linux-mm/202210240729.zs46cfzo-...@intel.com

Warning: (recently discovered and may have been fixed)

arch/parisc/kernel/setup.c:78 setup_cmdline() warn: curly braces intended?
drivers/net/phy/phylink.c:588 phylink_validate_mask_caps() warn: variable 
dereferenced before check 'state' (see line 583)

Warning ids grouped by kconfigs:

gcc_recent_errors
|-- i386-randconfig-s041-20221024
|   |-- fs-ntfs3-index.c:sparse:sparse:restricted-__le32-degrades-to-integer
|   |-- 
fs-ntfs3-namei.c:sparse:sparse:incorrect-type-in-argument-(different-base-types)-expected-restricted-__le16-const-usertype-s1-got-unsigned-short
|   `-- 
fs-ntfs3-namei.c:sparse:sparse:incorrect-type-in-argument-(different-base-types)-expected-restricted-__le16-const-usertype-s2-got-unsigned-short
|-- microblaze-randconfig-m031-20221023
|   `-- 
drivers-net-phy-phylink.c-phylink_validate_mask_caps()-warn:variable-dereferenced-before-check-state-(see-line-)
|-- parisc-randconfig-m031-20221023
|   `-- arch-parisc-kernel-setup.c-setup_cmdline()-warn:curly-braces-intended
`-- x86_64-randconfig-m001
`-- 
arch-x86-kernel-apic-apic.c-generic_processor_info()-warn:always-true-condition-(num_processors-()-)-(-u32max-)
clang_recent_errors
|-- hexagon-randconfig-r045-20221023
|   |-- 
drivers-phy-mediatek-phy-mtk-hdmi-mt2701.c:warning:result-of-comparison-of-constant-with-expression-of-type-typeof-(_Generic((mask_)-char:(unsigned-char)-unsigned-char:(unsigned-char)-signed-char:(uns
|   |-- 
drivers-phy-mediatek-phy-mtk-hdmi-mt8173.c:warning:result-of-comparison-of-constant-with-expression-of-type-typeof-(_Generic((mask_)-char:(unsigned-char)-unsigned-char:(unsigned-char)-signed-char:(uns
|   |-- 
drivers-phy-mediatek-phy-mtk-mipi-dsi-mt8173.c:warning:result-of-comparison-of-constant-with-expression-of-type-typeof-(_Generic((mask_)-char:(unsigned-char)-unsigned-char:(unsigned-char)-signed-char:
|   |-- 
drivers-phy-mediatek-phy-mtk-mipi-dsi-mt8183.c:warning:result-of-comparison-of-constant-with-expression-of-type-typeof-(_Generic((mask_)-char:(unsigned-char)-unsigned-char:(unsigned-char)-signed-char:
|   |-- 
drivers-phy-mediatek-phy-mtk-tphy.c:warning:result-of-comparison-of-constant-with-expression-of-type-typeof-(_Generic((mask_)-char:(unsigned-char)-unsigned-char:(unsigned-char)-signed-char:(unsigned-c
|   `-- 
fs-ntfs3-namei.c:warning:variable-uni1-is-used-uninitialized-whenever-if-condition-is-true
|-- i386-randconfig-a001-20221024
|   `-- 
fs-ntfs3-namei.c:warning:variable-uni1-is-used-uninitialized-whenever-if-condition-is-true
|-- i386-randconfig-a011
|   `-- 
fs-ntfs3-namei.c:warning:variable-uni1-is-used-uninitialized-whenever-if-condition-is-true
|-- i386-randconfig-a015
|   `-- 
fs-ntfs3-namei.c:warning:variable-uni1-is-used-uninitialized-whenever-if-condition-is-true
|-- powerpc-randconfig-r015-20221023
|   |-- 
drivers-gpu-drm-amd-amdgpu-mmhub_v2_0.c:warning:variable-data-is-uninitialized-when-used-here
|   |-- 
drivers-phy-mediatek-phy-mtk-tphy.c:warning:result-of-comparison-of-constant-with-expression-of-type-typeof-(_Generic((mask_)-char:(unsigned-char)-unsigned-char:(unsigned-char)-signed-char:(unsigned-c
|   `-- 
fs-ntfs3-namei.c:warning:variable-uni1-is-used-uninitialized-whenever-if-condition-is-true
|-- riscv-randconfig-r042-20221023
|   `-- ld.lld:error:undefined-symbol:dax_holder_notify_failure
|-- s390-randconfig-r022-20221023
|   `-- 
fs-ntfs3-namei.c:warning:variable-uni1-is-used-uninitialized-whenever-if-condition-is-true
|-- s390-randconfig-r044-20221023
|   `-- 
fs-ntfs3-namei.c:warning:variable-uni1-is-used-uninitialized-whenever-if-condition-is-true
|-- x86_64-randconfig-a002-20221024
|   `-- 
fs-ntfs3-namei.c:warning:variable-uni1-is-used-uninitialized-whenever-if-condition-is-true
|-- x86_64-randconfig-a004-20221024
|   `-- 
fs-ntfs3-namei.c:warning:variable-uni1-is-used-uninitialized-whenever-if-condition-is-true
`-- x86_64-randconfig-a005-20221024
`-- 
fs-ntfs3-namei.c:warning:variable-uni1-is-used-uninitialized-whenever-if-condition-is-true

elapsed time: 726m

configs tested: 91
configs skipped: 3

gcc tested configs:
um i386_defconfig
um   x86_64_defconfig
x86_64rhel-8.3-kselftests
x86_64  defconfig
x86_64  rhel-8.3-func
i386defconfig
x86_64   randconfig-a013-20221024
x86_64   rhel-8.3
x86_64   randconfig-a012-20221024
x86_64   allyescon

Re: [PATCH] drm/amdgpu: don't call drm_fb_helper_lastclose in lastclose()

[Alex Deucher]

On Mon, Oct 24, 2022 at 3:33 AM Thomas Zimmermann  wrote:
>
> Hi
>
> Am 24.10.22 um 08:20 schrieb Quan, Evan:
> > [AMD Official Use Only - General]
> >
> > Reviewed-by: Evan Quan 
> >
> >> -Original Message-
> >> From: amd-gfx  On Behalf Of Alex
> >> Deucher
> >> Sent: Thursday, October 20, 2022 10:36 PM
> >> To: amd-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
> >> Cc: Deucher, Alexander ; Thomas
> >> Zimmermann 
> >> Subject: [PATCH] drm/amdgpu: don't call drm_fb_helper_lastclose in
> >> lastclose()
> >>
> >> It's used to restore the fbdev console, but as amdgpu uses
> >> generic fbdev emulation, the console is being restored by the
> >> DRM client helpers already. See the call to drm_client_dev_restore()
> >> in drm_lastclose().
> >>
> >> Fixes: 087451f372bf76 ("drm/amdgpu: use generic fb helpers instead of
> >> setting up AMD own's.")
> >> Cc: Thomas Zimmermann 
> >> Signed-off-by: Alex Deucher 
> >> ---
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 1 -
> >>   1 file changed, 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >> index fe23e09eec98..474b9f40f792 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >> @@ -1106,7 +1106,6 @@ int amdgpu_info_ioctl(struct drm_device *dev,
> >> void *data, struct drm_file *filp)
> >>*/
> >>   void amdgpu_driver_lastclose_kms(struct drm_device *dev)
> >>   {
> >> -drm_fb_helper_lastclose(dev);
> >>  vga_switcheroo_process_delayed_switch();
> >>   }
>
> Without the call to drm_fb_helper_lastclose(), the console emulation
> will be restored by drm_client_dev_restore() from drm_lastclose(). [1]
> It means that it's now changing order with the call to
> vga_switcheroo_process_delay_switch(). Can this become a problem?
>
> I looked at the other callers of that function. Most restore the console
> before doing the switcheroo. Nouveau doesn't seem to care about the
> console at all.

I don't know off hand.  I suppose if the switch powered down the GPU
and then we tried to restore it's console state that would be a
problem, but it looks like vga_switchto_stage2() just powers down the
GPU without going through suspend so I'm not sure if this actually
worked correctly?  What are the potential problems with calling
drm_fb_helper_lastclose twice?

Alex

>
> Best regards
> Thomas
>
> [1]
> https://elixir.bootlin.com/linux/v6.0.3/source/drivers/gpu/drm/drm_file.c#L467
>
> >>
> >> --
> >> 2.37.3
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev

Re: [PATCH] drm/amdgpu: Fix uninitialized warning in mmhub_v2_0_get_clockgating()

[Alex Deucher]

Applied.  Thanks!

Alex

On Mon, Oct 24, 2022 at 11:20 AM Nathan Chancellor  wrote:
>
> Clang warns:
>
>   drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c:686:3: error: variable 'data' is 
> uninitialized when used here [-Werror,-Wuninitialized]
>   data |= MM_ATC_L2_MISC_CG__ENABLE_MASK;
>   ^~~~
>   drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c:674:10: note: initialize the 
> variable 'data' to silence this warning
>   int data, data1;
>   ^
>   = 0
>   1 error generated.
>
> This clearly should have just been a regular '=', as there was no prior
> assignment.
>
> Fixes: 7a4fad619819 ("drm/amdgpu: Remove ATC L2 access for MMHUB 2.1.x")
> Link: https://github.com/ClangBuiltLinux/linux/issues/1748
> Signed-off-by: Nathan Chancellor 
> ---
>  drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c 
> b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
> index 5ec6d17fed09..998b5d17b271 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
> @@ -683,7 +683,7 @@ static void mmhub_v2_0_get_clockgating(struct 
> amdgpu_device *adev, u64 *flags)
> /* There is no ATCL2 in MMHUB for 2.1.x. Keep the status
>  * based on DAGB
>  */
> -   data |= MM_ATC_L2_MISC_CG__ENABLE_MASK;
> +   data = MM_ATC_L2_MISC_CG__ENABLE_MASK;
> data1 = RREG32_SOC15(MMHUB, 0, 
> mmDAGB0_CNTL_MISC2_Sienna_Cichlid);
> break;
> default:
>
> base-commit: fb5e487f910e1105019b883e8ed25e36e4bfd657
> --
> 2.38.1
>

[PATCH] drm/amdgpu: Fix uninitialized warning in mmhub_v2_0_get_clockgating()

[Nathan Chancellor]

Clang warns:

  drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c:686:3: error: variable 'data' is 
uninitialized when used here [-Werror,-Wuninitialized]
  data |= MM_ATC_L2_MISC_CG__ENABLE_MASK;
  ^~~~
  drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c:674:10: note: initialize the variable 
'data' to silence this warning
  int data, data1;
  ^
  = 0
  1 error generated.

This clearly should have just been a regular '=', as there was no prior
assignment.

Fixes: 7a4fad619819 ("drm/amdgpu: Remove ATC L2 access for MMHUB 2.1.x")
Link: https://github.com/ClangBuiltLinux/linux/issues/1748
Signed-off-by: Nathan Chancellor 
---
 drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c 
b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
index 5ec6d17fed09..998b5d17b271 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
@@ -683,7 +683,7 @@ static void mmhub_v2_0_get_clockgating(struct amdgpu_device 
*adev, u64 *flags)
/* There is no ATCL2 in MMHUB for 2.1.x. Keep the status
 * based on DAGB
 */
-   data |= MM_ATC_L2_MISC_CG__ENABLE_MASK;
+   data = MM_ATC_L2_MISC_CG__ENABLE_MASK;
data1 = RREG32_SOC15(MMHUB, 0, 
mmDAGB0_CNTL_MISC2_Sienna_Cichlid);
break;
default:

base-commit: fb5e487f910e1105019b883e8ed25e36e4bfd657
-- 
2.38.1

Re: [PATCH] drm/radeon: fix repeated words in comments

[Alex Deucher]

Applied.  Thanks!

Alex

On Sat, Oct 22, 2022 at 2:06 AM wangjianli  wrote:
>
> Delete the redundant word 'the'.
>
> Signed-off-by: wangjianli 
> ---
>  drivers/gpu/drm/radeon/radeon_device.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_device.c 
> b/drivers/gpu/drm/radeon/radeon_device.c
> index 15692cb241fc..ff52b5f4c1e6 100644
> --- a/drivers/gpu/drm/radeon/radeon_device.c
> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> @@ -1206,7 +1206,7 @@ static void radeon_check_arguments(struct radeon_device 
> *rdev)
>   * @pdev: pci dev pointer
>   * @state: vga_switcheroo state
>   *
> - * Callback for the switcheroo driver.  Suspends or resumes the
> + * Callback for the switcheroo driver.  Suspends or resumes
>   * the asics before or after it is powered up using ACPI methods.
>   */
>  static void radeon_switcheroo_set_state(struct pci_dev *pdev, enum 
> vga_switcheroo_state state)
> --
> 2.36.1
>

Re: [PATCH] amd/amdgpu: fix repeated words in comments

[Alex Deucher]

On Sat, Oct 22, 2022 at 2:02 AM wangjianli  wrote:
>
> Delete the redundant word 'the'.
>
> Signed-off-by: wangjianli 

Applied.  Thanks!

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index e9155dc1c30c..3de990bbbf2f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1565,7 +1565,7 @@ static int amdgpu_device_check_arguments(struct 
> amdgpu_device *adev)
>   * @pdev: pci dev pointer
>   * @state: vga_switcheroo state
>   *
> - * Callback for the switcheroo driver.  Suspends or resumes the
> + * Callback for the switcheroo driver.  Suspends or resumes
>   * the asics before or after it is powered up using ACPI methods.
>   */
>  static void amdgpu_switcheroo_set_state(struct pci_dev *pdev,
> --
> 2.36.1
>

Re: [PATCH v2] kset: fix memory leak when kset_register() returns error

[Yang Yingliang]

On 2022/10/24 22:53, Greg KH wrote:

On Mon, Oct 24, 2022 at 10:39:44PM +0800, Yang Yingliang wrote:

On 2022/10/24 21:52, Greg KH wrote:

On Mon, Oct 24, 2022 at 08:19:10PM +0800, Yang Yingliang wrote:

Inject fault while loading module, kset_register() may fail.
If it fails, the name allocated by kobject_set_name() which
is called before kset_register() is leaked, because refcount
of kobject is hold in kset_init().

As a kset may be embedded in a larger structure which needs
be freed in release() function or error path in callers, we
can not call kset_put() in kset_register(), or it will cause
double free, so just call kfree_const() to free the name and
set it to NULL.

With this fix, the callers don't need to care about the name
freeing and call an extra kset_put() if kset_register() fails.

Suggested-by: Luben Tuikov 
Signed-off-by: Yang Yingliang 
---
v1 -> v2:
Free name inside of kset_register() instead of calling kset_put()
in drivers.
---
   lib/kobject.c | 8 +++-
   1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/kobject.c b/lib/kobject.c
index a0b2dbfcfa23..3409a89c81e5 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -834,6 +834,9 @@ EXPORT_SYMBOL_GPL(kobj_sysfs_ops);
   /**
* kset_register() - Initialize and add a kset.
* @k: kset.
+ *
+ * NOTE: On error, the kset.kobj.name allocated by() kobj_set_name()
+ * which is called before kset_register() in caller need be freed.

This comment doesn't make any sense anymore.  No caller needs to worry
about this, right?

With this fix, the name is freed inside of kset_register(), it can not be
accessed,

Agreed.


if it allocated dynamically, but callers don't know this if no comment here,
they may use it in error path (something like to print error message with
it),
so how about comment like this to tell callers not to use the name:

NOTE: On error, the kset.kobj.name allocated by() kobj_set_name()
is freed, it can not be used any more.

Sure, that's a better way to word it.


*/
   int kset_register(struct kset *k)
   {
@@ -844,8 +847,11 @@ int kset_register(struct kset *k)
kset_init(k);
err = kobject_add_internal(&k->kobj);
-   if (err)
+   if (err) {
+   kfree_const(k->kobj.name);
+   k->kobj.name = NULL;

Why are you setting the name here to NULL?

I set it to NULL to avoid accessing bad pointer in callers,
if callers use it in error path, current callers won't use this
name pointer in error path, so we can remove this assignment?

Ah, I didn't think about using it on error paths.  Ideally that would
never happen, but that's good to set just to make it obvious.  How about
adding a small comment here saying why you are setting it so we all
remember it in 5 years when we look at the code again.

OK, I can add it in v3.

Thanks,
Yang


thanks,

greg k-h
.

Re: [PATCH v2] kset: fix memory leak when kset_register() returns error

[Greg KH]

On Mon, Oct 24, 2022 at 10:39:44PM +0800, Yang Yingliang wrote:
> 
> On 2022/10/24 21:52, Greg KH wrote:
> > On Mon, Oct 24, 2022 at 08:19:10PM +0800, Yang Yingliang wrote:
> > > Inject fault while loading module, kset_register() may fail.
> > > If it fails, the name allocated by kobject_set_name() which
> > > is called before kset_register() is leaked, because refcount
> > > of kobject is hold in kset_init().
> > > 
> > > As a kset may be embedded in a larger structure which needs
> > > be freed in release() function or error path in callers, we
> > > can not call kset_put() in kset_register(), or it will cause
> > > double free, so just call kfree_const() to free the name and
> > > set it to NULL.
> > > 
> > > With this fix, the callers don't need to care about the name
> > > freeing and call an extra kset_put() if kset_register() fails.
> > > 
> > > Suggested-by: Luben Tuikov 
> > > Signed-off-by: Yang Yingliang 
> > > ---
> > > v1 -> v2:
> > >Free name inside of kset_register() instead of calling kset_put()
> > >in drivers.
> > > ---
> > >   lib/kobject.c | 8 +++-
> > >   1 file changed, 7 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/lib/kobject.c b/lib/kobject.c
> > > index a0b2dbfcfa23..3409a89c81e5 100644
> > > --- a/lib/kobject.c
> > > +++ b/lib/kobject.c
> > > @@ -834,6 +834,9 @@ EXPORT_SYMBOL_GPL(kobj_sysfs_ops);
> > >   /**
> > >* kset_register() - Initialize and add a kset.
> > >* @k: kset.
> > > + *
> > > + * NOTE: On error, the kset.kobj.name allocated by() kobj_set_name()
> > > + * which is called before kset_register() in caller need be freed.
> > This comment doesn't make any sense anymore.  No caller needs to worry
> > about this, right?
> With this fix, the name is freed inside of kset_register(), it can not be
> accessed,

Agreed.

> if it allocated dynamically, but callers don't know this if no comment here,
> they may use it in error path (something like to print error message with
> it),
> so how about comment like this to tell callers not to use the name:
> 
> NOTE: On error, the kset.kobj.name allocated by() kobj_set_name()
> is freed, it can not be used any more.

Sure, that's a better way to word it.

> > >*/
> > >   int kset_register(struct kset *k)
> > >   {
> > > @@ -844,8 +847,11 @@ int kset_register(struct kset *k)
> > >   kset_init(k);
> > >   err = kobject_add_internal(&k->kobj);
> > > - if (err)
> > > + if (err) {
> > > + kfree_const(k->kobj.name);
> > > + k->kobj.name = NULL;
> > Why are you setting the name here to NULL?
> I set it to NULL to avoid accessing bad pointer in callers,
> if callers use it in error path, current callers won't use this
> name pointer in error path, so we can remove this assignment?

Ah, I didn't think about using it on error paths.  Ideally that would
never happen, but that's good to set just to make it obvious.  How about
adding a small comment here saying why you are setting it so we all
remember it in 5 years when we look at the code again.

thanks,

greg k-h

Re: [PATCH v2] kset: fix memory leak when kset_register() returns error

[Yang Yingliang]

On 2022/10/24 21:52, Greg KH wrote:

On Mon, Oct 24, 2022 at 08:19:10PM +0800, Yang Yingliang wrote:

Inject fault while loading module, kset_register() may fail.
If it fails, the name allocated by kobject_set_name() which
is called before kset_register() is leaked, because refcount
of kobject is hold in kset_init().

As a kset may be embedded in a larger structure which needs
be freed in release() function or error path in callers, we
can not call kset_put() in kset_register(), or it will cause
double free, so just call kfree_const() to free the name and
set it to NULL.

With this fix, the callers don't need to care about the name
freeing and call an extra kset_put() if kset_register() fails.

Suggested-by: Luben Tuikov 
Signed-off-by: Yang Yingliang 
---
v1 -> v2:
   Free name inside of kset_register() instead of calling kset_put()
   in drivers.
---
  lib/kobject.c | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/kobject.c b/lib/kobject.c
index a0b2dbfcfa23..3409a89c81e5 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -834,6 +834,9 @@ EXPORT_SYMBOL_GPL(kobj_sysfs_ops);
  /**
   * kset_register() - Initialize and add a kset.
   * @k: kset.
+ *
+ * NOTE: On error, the kset.kobj.name allocated by() kobj_set_name()
+ * which is called before kset_register() in caller need be freed.

This comment doesn't make any sense anymore.  No caller needs to worry
about this, right?
With this fix, the name is freed inside of kset_register(), it can not 
be accessed,

if it allocated dynamically, but callers don't know this if no comment here,
they may use it in error path (something like to print error message 
with it),

so how about comment like this to tell callers not to use the name:

NOTE: On error, the kset.kobj.name allocated by() kobj_set_name()
is freed, it can not be used any more.



   */
  int kset_register(struct kset *k)
  {
@@ -844,8 +847,11 @@ int kset_register(struct kset *k)
  
  	kset_init(k);

err = kobject_add_internal(&k->kobj);
-   if (err)
+   if (err) {
+   kfree_const(k->kobj.name);
+   k->kobj.name = NULL;

Why are you setting the name here to NULL?

I set it to NULL to avoid accessing bad pointer in callers,
if callers use it in error path, current callers won't use this
name pointer in error path, so we can remove this assignment?

Thanks,
Yang


thanks,

greg k-h
.

Re: [PATCH v2] kset: fix memory leak when kset_register() returns error

[Greg KH]

On Mon, Oct 24, 2022 at 08:19:10PM +0800, Yang Yingliang wrote:
> Inject fault while loading module, kset_register() may fail.
> If it fails, the name allocated by kobject_set_name() which
> is called before kset_register() is leaked, because refcount
> of kobject is hold in kset_init().
> 
> As a kset may be embedded in a larger structure which needs
> be freed in release() function or error path in callers, we
> can not call kset_put() in kset_register(), or it will cause
> double free, so just call kfree_const() to free the name and
> set it to NULL.
> 
> With this fix, the callers don't need to care about the name
> freeing and call an extra kset_put() if kset_register() fails.
> 
> Suggested-by: Luben Tuikov 
> Signed-off-by: Yang Yingliang 
> ---
> v1 -> v2:
>   Free name inside of kset_register() instead of calling kset_put()
>   in drivers.
> ---
>  lib/kobject.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/kobject.c b/lib/kobject.c
> index a0b2dbfcfa23..3409a89c81e5 100644
> --- a/lib/kobject.c
> +++ b/lib/kobject.c
> @@ -834,6 +834,9 @@ EXPORT_SYMBOL_GPL(kobj_sysfs_ops);
>  /**
>   * kset_register() - Initialize and add a kset.
>   * @k: kset.
> + *
> + * NOTE: On error, the kset.kobj.name allocated by() kobj_set_name()
> + * which is called before kset_register() in caller need be freed.

This comment doesn't make any sense anymore.  No caller needs to worry
about this, right?

>   */
>  int kset_register(struct kset *k)
>  {
> @@ -844,8 +847,11 @@ int kset_register(struct kset *k)
>  
>   kset_init(k);
>   err = kobject_add_internal(&k->kobj);
> - if (err)
> + if (err) {
> + kfree_const(k->kobj.name);
> + k->kobj.name = NULL;

Why are you setting the name here to NULL?

thanks,

greg k-h

[PATCH v2] kset: fix memory leak when kset_register() returns error

[Yang Yingliang]

Inject fault while loading module, kset_register() may fail.
If it fails, the name allocated by kobject_set_name() which
is called before kset_register() is leaked, because refcount
of kobject is hold in kset_init().

As a kset may be embedded in a larger structure which needs
be freed in release() function or error path in callers, we
can not call kset_put() in kset_register(), or it will cause
double free, so just call kfree_const() to free the name and
set it to NULL.

With this fix, the callers don't need to care about the name
freeing and call an extra kset_put() if kset_register() fails.

Suggested-by: Luben Tuikov 
Signed-off-by: Yang Yingliang 
---
v1 -> v2:
  Free name inside of kset_register() instead of calling kset_put()
  in drivers.
---
 lib/kobject.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/kobject.c b/lib/kobject.c
index a0b2dbfcfa23..3409a89c81e5 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -834,6 +834,9 @@ EXPORT_SYMBOL_GPL(kobj_sysfs_ops);
 /**
  * kset_register() - Initialize and add a kset.
  * @k: kset.
+ *
+ * NOTE: On error, the kset.kobj.name allocated by() kobj_set_name()
+ * which is called before kset_register() in caller need be freed.
  */
 int kset_register(struct kset *k)
 {
@@ -844,8 +847,11 @@ int kset_register(struct kset *k)
 
kset_init(k);
err = kobject_add_internal(&k->kobj);
-   if (err)
+   if (err) {
+   kfree_const(k->kobj.name);
+   k->kobj.name = NULL;
return err;
+   }
kobject_uevent(&k->kobj, KOBJ_ADD);
return 0;
 }
-- 
2.25.1

[PATCH v2] drm/amd/display: Revert logic for plane modifiers

[Joaquín Ignacio Aramendía]

This file was split in commit 5d945cbcd4b16a29d6470a80dfb19738f9a4319f
("drm/amd/display: Create a file dedicated to planes") and the logic in
dm_plane_format_mod_supported() function got changed by a switch logic.
That change broke drm_plane modifiers setting on series 5000 APUs
(tested on OXP mini AMD 5800U and HP Dev One 5850U PRO)
leading to Gamescope not working as reported on GitHub[1]

To reproduce the issue, enter a TTY and run:

$ gamescope -- vkcube

With said commit applied it will abort. This one restores the old logic,
fixing the issue that affects Gamescope.

[1](https://github.com/Plagman/gamescope/issues/624)

Cc:  # 6.0.x
Signed-off-by: Joaquín Ignacio Aramendía 
Reviewed-by: Bas Nieuwenhuizen 
---
Removed asic_id and excess newlines. Resend with correct Cc line.
---
 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 50 +++
 1 file changed, 7 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index dfd3be49eac8..e6854f7270a6 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -1369,7 +1369,7 @@ static bool dm_plane_format_mod_supported(struct 
drm_plane *plane,
 {
struct amdgpu_device *adev = drm_to_adev(plane->dev);
const struct drm_format_info *info = drm_format_info(format);
-   struct hw_asic_id asic_id = adev->dm.dc->ctx->asic_id;
+   int i;

enum dm_micro_swizzle microtile = modifier_gfx9_swizzle_mode(modifier) 
& 3;

@@ -1386,49 +1386,13 @@ static bool dm_plane_format_mod_supported(struct 
drm_plane *plane,
return true;
}

-   /* check if swizzle mode is supported by this version of DCN */
-   switch (asic_id.chip_family) {
-   case FAMILY_SI:
-   case FAMILY_CI:
-   case FAMILY_KV:
-   case FAMILY_CZ:
-   case FAMILY_VI:
-   /* asics before AI does not have modifier support */
-   return false;
-   case FAMILY_AI:
-   case FAMILY_RV:
-   case FAMILY_NV:
-   case FAMILY_VGH:
-   case FAMILY_YELLOW_CARP:
-   case AMDGPU_FAMILY_GC_10_3_6:
-   case AMDGPU_FAMILY_GC_10_3_7:
-   switch (AMD_FMT_MOD_GET(TILE, modifier)) {
-   case AMD_FMT_MOD_TILE_GFX9_64K_R_X:
-   case AMD_FMT_MOD_TILE_GFX9_64K_D_X:
-   case AMD_FMT_MOD_TILE_GFX9_64K_S_X:
-   case AMD_FMT_MOD_TILE_GFX9_64K_D:
-   return true;
-   default:
-   return false;
-   }
-   break;
-   case AMDGPU_FAMILY_GC_11_0_0:
-   case AMDGPU_FAMILY_GC_11_0_1:
-   switch (AMD_FMT_MOD_GET(TILE, modifier)) {
-   case AMD_FMT_MOD_TILE_GFX11_256K_R_X:
-   case AMD_FMT_MOD_TILE_GFX9_64K_R_X:
-   case AMD_FMT_MOD_TILE_GFX9_64K_D_X:
-   case AMD_FMT_MOD_TILE_GFX9_64K_S_X:
-   case AMD_FMT_MOD_TILE_GFX9_64K_D:
-   return true;
-   default:
-   return false;
-   }
-   break;
-   default:
-   ASSERT(0); /* Unknown asic */
-   break;
+   /* Check that the modifier is on the list of the plane's supported 
modifiers. */
+   for (i = 0; i < plane->modifier_count; i++) {
+   if (modifier == plane->modifiers[i])
+   break;
}
+   if (i == plane->modifier_count)
+   return false;

/*
 * For D swizzle the canonical modifier depends on the bpp, so check
--
2.38.1

Re: Fwd: amdgpu: update from 5.10.145 to 5.10.146...149 breaks boot on Ryzen based computers

[Greg Kroah-Hartman]

On Sun, Oct 23, 2022 at 10:23:53AM -0700, Linus Torvalds wrote:
> This was sent to me, but should have gone to other people.
> 
> It may already be fixed, but note how the report is about -stable
> kernels, including apparently the current 5.10 stable version (149(.
> 
>   Linus
> 
> -- Forwarded message -
> From: Kevin Torkelson 
> Date: Thu, Oct 20, 2022 at 8:09 AM
> Subject: amdgpu: update from 5.10.145 to 5.10.146...149 breaks boot on
> Ryzen based computers
> To: 
> 
> 
> Linus,
> 
> --- Possibly Important ---
> I know several people who are crashing with Debian Bullseye (stable)
> with the most current kernel put out by the distribution. AMD put out
> a fix that seems like it might be related here:
> https://gitlab.freedesktop.org/drm/amd/-/issues/2216

We have fixes queued up for this in the stable tree for 5.10 already,
thanks.

greg k-h

[PATCH v2 03/16] drm/amd/display: stop using connector->override_edid

[Jani Nikula]

The connector->override_edid flag is strictly for EDID override debugfs
management, and drivers have no business using it.

Cc: Alex Deucher 
Cc: Christian König 
Cc: Xinhui Pan 
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Jani Nikula 
Reviewed-by: Harry Wentland 
Acked-by: Alex Deucher 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 0db2a88cd4d7..3c072754738d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6108,7 +6108,6 @@ static void create_eml_sink(struct amdgpu_dm_connector 
*aconnector)
aconnector->base.name);
 
aconnector->base.force = DRM_FORCE_OFF;
-   aconnector->base.override_edid = false;
return;
}
 
@@ -6143,8 +6142,6 @@ static void handle_edid_mgmt(struct amdgpu_dm_connector 
*aconnector)
link->verified_link_cap.link_rate = LINK_RATE_HIGH2;
}
 
-
-   aconnector->base.override_edid = true;
create_eml_sink(aconnector);
 }
 
-- 
2.34.1

RE: [PATCH 1/2] drm/amdkfd: introduce dummy cache info for property asic

[Liang, Prike]

[Public]

-Original Message-
From: Kuehling, Felix 
Sent: Saturday, October 22, 2022 4:53 AM
To: Liang, Prike ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Zhang, Yifan 
; Huang, Ray ; Liu, Aaron 

Subject: Re: [PATCH 1/2] drm/amdkfd: introduce dummy cache info for property 
asic

On 2022-10-21 09:05, Liang, Prike wrote:
> [Public]
>
> -Original Message-
> From: Kuehling, Felix 
> Sent: Friday, October 21, 2022 1:11 PM
> To: Liang, Prike ; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander ; Zhang, Yifan
> ; Huang, Ray ; Liu, Aaron
> 
> Subject: Re: [PATCH 1/2] drm/amdkfd: introduce dummy cache info for
> property asic
>
> Am 2022-10-20 um 21:50 schrieb Liang, Prike:
>> [Public]
>>
>> -Original Message-
>> From: Kuehling, Felix 
>> Sent: Friday, October 21, 2022 12:03 AM
>> To: Liang, Prike ; amd-gfx@lists.freedesktop.org
>> Cc: Deucher, Alexander ; Zhang, Yifan
>> ; Huang, Ray ; Liu, Aaron
>> 
>> Subject: Re: [PATCH 1/2] drm/amdkfd: introduce dummy cache info for
>> property asic
>>
>>
>> Am 2022-10-20 um 05:15 schrieb Prike Liang:
>>> This dummy cache info will enable kfd base function support.
>>>
>>> Signed-off-by: Prike Liang 
>>> ---
>>> drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 55 +--
>>> 1 file changed, 52 insertions(+), 3 deletions(-)
>>>
[snip]
>>> @@ -1528,7 +1574,10 @@ static int kfd_fill_gpu_cache_info(struct kfd_dev 
>>> *kdev,
>>> 
>>> kfd_fill_gpu_cache_info_from_gfx_config(kdev, pcache_info);
>>> break;
>>> default:
>>> - return -EINVAL;
>>> + pcache_info = dummy_cache_info;
>>> + num_of_cache_types = ARRAY_SIZE(dummy_cache_info);
>>> + pr_warn("dummy cache info is used temporarily and 
>>> real cache info need update later.\n");
>>> + break;
>> Could we make this return an error if the amdgpu.exp_hw_support module 
>> parameter is not set?
>>
>> Regards,
>>  Felix
>>
>> [Prike] It's fine to protect this dummy info by checking the parameter 
>> amdgpu_exp_hw_support, but it may not friendly to end user by adding the 
>> parameter and some guys will still report KFD not enabled for this parameter 
>> setting problem. The original idea is the end user will not aware the dummy 
>> cache info and only alert the warning message to developer.
> I thought the intention was to simplify bring-up but make sure that valid 
> cache info is available by the time a chip goes into production.
> Therefore, normal end users should never need to set the 
> amdgpu_exp_hw_support option. I think you're saying that we would go to 
> production with dummy info. That seems like a bad idea to me.
>
> Regards,
> Felix
>
> [Prike]  Sorry for the confusion. In fact, this dummy cache info only used 
> before production and meanwhile needn't require any parameter setting for CQE 
> do KFD test. Anyway if you still have concern on this solution I will add the 
> condition of checking amdgpu_exp_hw_support.

The idea to control this with a module parameter was to cause a more obvious 
failure if we don't have correct cache info before going to production. Just a 
warning in the log file is too easy to miss or ignore. Of course, if QA gets 
into the habit of testing with amdgpu_exp_hw_support, then this may not solve 
the problem. You need to have at least one test pass without 
amdgpu_exp_hw_support to catch missing cache info.

Regards,
   Felix

Get your point. As to the KFD support on a NPI will be tracked by a ticket 
which make sure the real cache info update later after this dummy cache info 
assigned in the early BU phase.

Thanks,
Prike

[PATCH 2/2] drm/amdkfd: Fix the warning of array-index-out-of-bounds

[Ma Jun]

For some GPUs with more CUs, the original sibling_map[32]
in struct crat_subtype_cache is not enough
to save the cache information when create the VCRAT table,
so fill the cache info into struct kfd_cache_properties_ext
directly to fix the problem.

At the same time, a new directory
"/sys/class/kfd/kfd/topology/nodes/*nodes_num*/caches_ext"
is created for cache information showing.

The original directory "cache" is reserved for GPU which using real CRAT
table.

Signed-off-by: Ma Jun 
---
 drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 1229 +---
 drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 1246 -
 drivers/gpu/drm/amd/amdkfd/kfd_topology.h |   21 +
 3 files changed, 1261 insertions(+), 1235 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
index 4857ec5b9f46..e6928c60338e 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
@@ -30,799 +30,6 @@
 #include "amdgpu.h"
 #include "amdgpu_amdkfd.h"
 
-/* Static table to describe GPU Cache information */
-struct kfd_gpu_cache_info {
-   uint32_tcache_size;
-   uint32_tcache_level;
-   uint32_tflags;
-   /* Indicates how many Compute Units share this cache
-* within a SA. Value = 1 indicates the cache is not shared
-*/
-   uint32_tnum_cu_shared;
-};
-
-static struct kfd_gpu_cache_info kaveri_cache_info[] = {
-   {
-   /* TCP L1 Cache per CU */
-   .cache_size = 16,
-   .cache_level = 1,
-   .flags = (CRAT_CACHE_FLAGS_ENABLED |
-   CRAT_CACHE_FLAGS_DATA_CACHE |
-   CRAT_CACHE_FLAGS_SIMD_CACHE),
-   .num_cu_shared = 1,
-   },
-   {
-   /* Scalar L1 Instruction Cache (in SQC module) per bank */
-   .cache_size = 16,
-   .cache_level = 1,
-   .flags = (CRAT_CACHE_FLAGS_ENABLED |
-   CRAT_CACHE_FLAGS_INST_CACHE |
-   CRAT_CACHE_FLAGS_SIMD_CACHE),
-   .num_cu_shared = 2,
-   },
-   {
-   /* Scalar L1 Data Cache (in SQC module) per bank */
-   .cache_size = 8,
-   .cache_level = 1,
-   .flags = (CRAT_CACHE_FLAGS_ENABLED |
-   CRAT_CACHE_FLAGS_DATA_CACHE |
-   CRAT_CACHE_FLAGS_SIMD_CACHE),
-   .num_cu_shared = 2,
-   },
-
-   /* TODO: Add L2 Cache information */
-};
-
-
-static struct kfd_gpu_cache_info carrizo_cache_info[] = {
-   {
-   /* TCP L1 Cache per CU */
-   .cache_size = 16,
-   .cache_level = 1,
-   .flags = (CRAT_CACHE_FLAGS_ENABLED |
-   CRAT_CACHE_FLAGS_DATA_CACHE |
-   CRAT_CACHE_FLAGS_SIMD_CACHE),
-   .num_cu_shared = 1,
-   },
-   {
-   /* Scalar L1 Instruction Cache (in SQC module) per bank */
-   .cache_size = 8,
-   .cache_level = 1,
-   .flags = (CRAT_CACHE_FLAGS_ENABLED |
-   CRAT_CACHE_FLAGS_INST_CACHE |
-   CRAT_CACHE_FLAGS_SIMD_CACHE),
-   .num_cu_shared = 4,
-   },
-   {
-   /* Scalar L1 Data Cache (in SQC module) per bank. */
-   .cache_size = 4,
-   .cache_level = 1,
-   .flags = (CRAT_CACHE_FLAGS_ENABLED |
-   CRAT_CACHE_FLAGS_DATA_CACHE |
-   CRAT_CACHE_FLAGS_SIMD_CACHE),
-   .num_cu_shared = 4,
-   },
-
-   /* TODO: Add L2 Cache information */
-};
-
-#define hawaii_cache_info kaveri_cache_info
-#define tonga_cache_info carrizo_cache_info
-#define fiji_cache_info  carrizo_cache_info
-#define polaris10_cache_info carrizo_cache_info
-#define polaris11_cache_info carrizo_cache_info
-#define polaris12_cache_info carrizo_cache_info
-#define vegam_cache_info carrizo_cache_info
-
-/* NOTE: L1 cache information has been updated and L2/L3
- * cache information has been added for Vega10 and
- * newer ASICs. The unit for cache_size is KiB.
- * In future,  check & update cache details
- * for every new ASIC is required.
- */
-
-static struct kfd_gpu_cache_info vega10_cache_info[] = {
-   {
-   /* TCP L1 Cache per CU */
-   .cache_size = 16,
-   .cache_level = 1,
-   .flags = (CRAT_CACHE_FLAGS_ENABLED |
-   CRAT_CACHE_FLAGS_DATA_CACHE |
-   CRAT_CACHE_FLAGS_SIMD_CACHE),
-   .num_cu_shared = 1,
-   },
-   {
-   /* Scalar L1 Instruction Cache per SQC */
-   .cache_size = 32,
-   .cache_level = 1,
-   .flags = (CRAT_CACHE_FLAGS_ENABLED |
-

[PATCH 1/2] drm/amdkfd: Init the base cu processor id

[Ma Jun]

Init and save the base cu processor id for later use

Signed-off-by: Ma Jun 
---
 drivers/gpu/drm/amd/amdkfd/kfd_crat.c   | 24 +
 drivers/gpu/drm/amd/amdkfd/kfd_device.c | 28 +
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h   |  3 +++
 3 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
index d25ac9cbe5b2..4857ec5b9f46 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
@@ -30,26 +30,6 @@
 #include "amdgpu.h"
 #include "amdgpu_amdkfd.h"
 
-/* GPU Processor ID base for dGPUs for which VCRAT needs to be created.
- * GPU processor ID are expressed with Bit[31]=1.
- * The base is set to 0x8000_ + 0x1000 to avoid collision with GPU IDs
- * used in the CRAT.
- */
-static uint32_t gpu_processor_id_low = 0x80001000;
-
-/* Return the next available gpu_processor_id and increment it for next GPU
- * @total_cu_count - Total CUs present in the GPU including ones
- *   masked off
- */
-static inline unsigned int get_and_inc_gpu_processor_id(
-   unsigned int total_cu_count)
-{
-   int current_id = gpu_processor_id_low;
-
-   gpu_processor_id_low += total_cu_count;
-   return current_id;
-}
-
 /* Static table to describe GPU Cache information */
 struct kfd_gpu_cache_info {
uint32_tcache_size;
@@ -2223,7 +2203,6 @@ static int kfd_create_vcrat_image_gpu(void *pcrat_image,
struct crat_subtype_computeunit *cu;
struct kfd_cu_info cu_info;
int avail_size = *size;
-   uint32_t total_num_of_cu;
int num_of_cache_entries = 0;
int cache_mem_filled = 0;
uint32_t nid = 0;
@@ -2275,8 +2254,7 @@ static int kfd_create_vcrat_image_gpu(void *pcrat_image,
cu->wave_front_size = cu_info.wave_front_size;
cu->array_count = cu_info.num_shader_arrays_per_engine *
cu_info.num_shader_engines;
-   total_num_of_cu = (cu->array_count * cu_info.num_cu_per_sh);
-   cu->processor_id_low = get_and_inc_gpu_processor_id(total_num_of_cu);
+   cu->processor_id_low = kdev->processor_id_low;
cu->num_cu_per_array = cu_info.num_cu_per_sh;
cu->max_slots_scatch_cu = cu_info.max_scratch_slots_per_cu;
cu->num_banks = cu_info.num_shader_engines;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index ae1f0be3f3a1..099df4598a42 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -38,6 +38,32 @@
 
 #define MQD_SIZE_ALIGNED 768
 
+/* GPU Processor ID base for dGPUs for which VCRAT needs to be created.
+ * GPU processor ID are expressed with Bit[31]=1.
+ * The base is set to 0x8000_ + 0x1000 to avoid collision with GPU IDs
+ * used in the CRAT.
+ */
+static uint32_t gpu_processor_id_low = 0x80001000;
+
+/* Return the next available gpu_processor_id and increment it for next GPU
+ * @total_cu_count - Total CUs present in the GPU including ones
+ *   masked off
+ */
+static inline unsigned int get_and_inc_gpu_processor_id(struct kfd_dev *kfd)
+{
+   struct amdgpu_device *adev = kfd->adev;
+   unsigned int array_count = 0;
+   unsigned int total_cu_count = 0;
+
+   kfd->processor_id_low = gpu_processor_id_low;
+
+   array_count = adev->gfx.config.max_sh_per_se * 
adev->gfx.config.max_shader_engines;
+   total_cu_count = array_count * adev->gfx.config.max_cu_per_sh;
+
+   gpu_processor_id_low += total_cu_count;
+   return 0;
+}
+
 /*
  * kfd_locked is used to lock the kfd driver during suspend or reset
  * once locked, kfd driver will stop any further GPU execution.
@@ -656,6 +682,8 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
 
amdgpu_amdkfd_get_local_mem_info(kfd->adev, &kfd->local_mem_info);
 
+   get_and_inc_gpu_processor_id(kfd);
+
if (kfd_topology_add_device(kfd)) {
dev_err(kfd_device, "Error adding device to topology\n");
goto kfd_topology_add_device_error;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 182eb67edbc5..4c06b233472f 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -370,6 +370,9 @@ struct kfd_dev {
 
/* Track per device allocated watch points. */
uint32_t alloc_watch_ids;
+
+   /* cu processor id base */
+   unsigned intprocessor_id_low;
 };
 
 struct kfd_ipc_obj;
-- 
2.25.1

[PATCH v2 19/21] drm/fb-helper: Always initialize generic fbdev emulation

[Thomas Zimmermann]

Initialize the generic fbdev emulation even if it has been disabled
on the kernel command line. The hotplug and mode initialization will
fail accordingly.

The kernel parameter can still be changed at runtime and the emulation
will initialize after hotplugging the connector.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/drm_fb_helper.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index fbc5c5445fdb0..d1afb420c6e06 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -526,11 +526,6 @@ int drm_fb_helper_init(struct drm_device *dev,
 {
int ret;
 
-   if (!drm_fbdev_emulation) {
-   dev->fb_helper = fb_helper;
-   return 0;
-   }
-
/*
 * If this is not the generic fbdev client, initialize a drm_client
 * without callbacks so we can use the modesets.
@@ -2716,9 +2711,6 @@ void drm_fbdev_generic_setup(struct drm_device *dev,
drm_WARN(dev, !dev->registered, "Device has not been registered.\n");
drm_WARN(dev, dev->fb_helper, "fb_helper is already set!\n");
 
-   if (!drm_fbdev_emulation)
-   return;
-
fb_helper = kzalloc(sizeof(*fb_helper), GFP_KERNEL);
if (!fb_helper) {
drm_err(dev, "Failed to allocate fb_helper\n");
-- 
2.38.0

[PATCH v2 20/21] drm/fb-helper: Move generic fbdev emulation into separate source file

[Thomas Zimmermann]

Move the generic fbdev implementation into its own source and header
file. Adapt drivers. No functonal changes, but some of the internal
helpers have been renamed to fit into the drm_fbdev_ naming scheme.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/Makefile  |   2 +-
 .../gpu/drm/arm/display/komeda/komeda_drv.c   |   2 +-
 drivers/gpu/drm/arm/hdlcd_drv.c   |   2 +-
 drivers/gpu/drm/arm/malidp_drv.c  |   2 +-
 drivers/gpu/drm/aspeed/aspeed_gfx_drv.c   |   2 +-
 drivers/gpu/drm/ast/ast_drv.c |   1 +
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c  |   2 +-
 drivers/gpu/drm/drm_fb_helper.c   | 517 +-
 drivers/gpu/drm/drm_fbdev.c   | 512 +
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c |   2 +-
 drivers/gpu/drm/gud/gud_drv.c |   2 +-
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   |   1 +
 .../gpu/drm/hisilicon/kirin/kirin_drm_drv.c   |   2 +-
 drivers/gpu/drm/hyperv/hyperv_drm_drv.c   |   2 +-
 drivers/gpu/drm/imx/dcss/dcss-kms.c   |   2 +-
 drivers/gpu/drm/imx/imx-drm-core.c|   2 +-
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c |   2 +-
 drivers/gpu/drm/kmb/kmb_drv.c |   2 +-
 drivers/gpu/drm/logicvc/logicvc_drm.c |   2 +-
 drivers/gpu/drm/mcde/mcde_drv.c   |   2 +-
 drivers/gpu/drm/mediatek/mtk_drm_drv.c|   2 +-
 drivers/gpu/drm/meson/meson_drv.c |   2 +-
 drivers/gpu/drm/mgag200/mgag200_drv.c |   1 +
 drivers/gpu/drm/mxsfb/lcdif_drv.c |   2 +-
 drivers/gpu/drm/mxsfb/mxsfb_drv.c |   2 +-
 drivers/gpu/drm/panel/panel-ilitek-ili9341.c  |   2 +-
 drivers/gpu/drm/pl111/pl111_drv.c |   2 +-
 drivers/gpu/drm/qxl/qxl_drv.c |   1 +
 drivers/gpu/drm/rcar-du/rcar_du_drv.c |   2 +-
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c   |   2 +-
 drivers/gpu/drm/solomon/ssd130x.c |   2 +-
 drivers/gpu/drm/sti/sti_drv.c |   2 +-
 drivers/gpu/drm/stm/drv.c |   2 +-
 drivers/gpu/drm/sun4i/sun4i_drv.c |   2 +-
 drivers/gpu/drm/tidss/tidss_drv.c |   2 +-
 drivers/gpu/drm/tilcdc/tilcdc_drv.c   |   2 +-
 drivers/gpu/drm/tiny/arcpgu.c |   2 +-
 drivers/gpu/drm/tiny/bochs.c  |   2 +-
 drivers/gpu/drm/tiny/cirrus.c |   2 +-
 drivers/gpu/drm/tiny/gm12u320.c   |   2 +-
 drivers/gpu/drm/tiny/hx8357d.c|   2 +-
 drivers/gpu/drm/tiny/ili9163.c|   2 +-
 drivers/gpu/drm/tiny/ili9225.c|   2 +-
 drivers/gpu/drm/tiny/ili9341.c|   2 +-
 drivers/gpu/drm/tiny/ili9486.c|   2 +-
 drivers/gpu/drm/tiny/mi0283qt.c   |   2 +-
 drivers/gpu/drm/tiny/ofdrm.c  |   2 +-
 drivers/gpu/drm/tiny/panel-mipi-dbi.c |   2 +-
 drivers/gpu/drm/tiny/repaper.c|   2 +-
 drivers/gpu/drm/tiny/simpledrm.c  |   2 +-
 drivers/gpu/drm/tiny/st7586.c |   2 +-
 drivers/gpu/drm/tiny/st7735r.c|   2 +-
 drivers/gpu/drm/tve200/tve200_drv.c   |   2 +-
 drivers/gpu/drm/udl/udl_drv.c |   2 +-
 drivers/gpu/drm/vboxvideo/vbox_drv.c  |   2 +-
 drivers/gpu/drm/vc4/vc4_drv.c |   2 +-
 drivers/gpu/drm/virtio/virtgpu_drv.c  |   1 +
 drivers/gpu/drm/vkms/vkms_drv.c   |   2 +-
 drivers/gpu/drm/xlnx/zynqmp_dpsub.c   |   2 +-
 include/drm/drm_fb_helper.h   |   9 -
 include/drm/drm_fbdev.h   |  15 +
 61 files changed, 586 insertions(+), 576 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_fbdev.c
 create mode 100644 include/drm/drm_fbdev.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 6e55c47288e42..b1c3d31128094 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -117,7 +117,7 @@ drm_kms_helper-y := \
drm_self_refresh_helper.o \
drm_simple_kms_helper.o
 drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
-drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
+drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fbdev.o drm_fb_helper.o
 obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
 
 #
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c 
b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
index 9fce4239d4ad4..9124d9e3f4e71 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
@@ -9,7 +9,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include "komeda_dev.h"
diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index a032003c340cc..fddcef0c373e0 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -26,7 +26,7 @@
 #include 
 #include 
 #include 
-#

[PATCH v2 17/21] drm/fb-helper: Perform all fbdev I/O with the same implementation

[Thomas Zimmermann]

Implement the fbdev's read/write helpers with the same functions. Use
the generic fbdev's code as template. Convert all drivers.

DRM's fb helpers must implement regular I/O functionality in struct
fb_ops and possibly perform a damage update. Handle all this in the
same functions and convert drivers. The functionality has been used
as part of the generic fbdev code for some time. The drivers don't
set struct drm_fb_helper.fb_dirty, so they will not be affected by
damage handling.

For I/O memory, fb helpers now provide drm_fb_helper_cfb_read() and
drm_fb_helper_cfb_write(). Several drivers require these. Until now
tegra used I/O read and write, although the memory buffer appears to
be in system memory. So use _sys_ helpers now.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/armada/armada_fbdev.c  |   2 +
 drivers/gpu/drm/drm_fb_helper.c| 383 -
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c  |   2 +
 drivers/gpu/drm/gma500/framebuffer.c   |   2 +
 drivers/gpu/drm/i915/display/intel_fbdev.c |   2 +
 drivers/gpu/drm/radeon/radeon_fb.c |   2 +
 drivers/gpu/drm/tegra/fb.c |   2 +
 drivers/gpu/drm/vmwgfx/vmwgfx_fb.c |   3 +
 include/drm/drm_fb_helper.h|  17 +
 9 files changed, 257 insertions(+), 158 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_fbdev.c 
b/drivers/gpu/drm/armada/armada_fbdev.c
index f02f6a5ba8320..584cee123bd8e 100644
--- a/drivers/gpu/drm/armada/armada_fbdev.c
+++ b/drivers/gpu/drm/armada/armada_fbdev.c
@@ -19,6 +19,8 @@
 static const struct fb_ops armada_fb_ops = {
.owner  = THIS_MODULE,
DRM_FB_HELPER_DEFAULT_OPS,
+   .fb_read= drm_fb_helper_cfb_read,
+   .fb_write   = drm_fb_helper_cfb_write,
.fb_fillrect= drm_fb_helper_cfb_fillrect,
.fb_copyarea= drm_fb_helper_cfb_copyarea,
.fb_imageblit   = drm_fb_helper_cfb_imageblit,
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 379e0d2f67198..836523aef6a27 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -747,30 +747,132 @@ void drm_fb_helper_deferred_io(struct fb_info *info, 
struct list_head *pagerefli
 }
 EXPORT_SYMBOL(drm_fb_helper_deferred_io);
 
+typedef ssize_t (*drm_fb_helper_read_screen)(struct fb_info *info, char __user 
*buf,
+size_t count, loff_t pos);
+
+static ssize_t __drm_fb_helper_read(struct fb_info *info, char __user *buf, 
size_t count,
+   loff_t *ppos, drm_fb_helper_read_screen 
read_screen)
+{
+   loff_t pos = *ppos;
+   size_t total_size;
+   ssize_t ret;
+
+   if (info->screen_size)
+   total_size = info->screen_size;
+   else
+   total_size = info->fix.smem_len;
+
+   if (pos >= total_size)
+   return 0;
+   if (count >= total_size)
+   count = total_size;
+   if (total_size - count < pos)
+   count = total_size - pos;
+
+   if (info->fbops->fb_sync)
+   info->fbops->fb_sync(info);
+
+   ret = read_screen(info, buf, count, pos);
+   if (ret > 0)
+   *ppos += ret;
+
+   return ret;
+}
+
+typedef ssize_t (*drm_fb_helper_write_screen)(struct fb_info *info, const char 
__user *buf,
+ size_t count, loff_t pos);
+
+static ssize_t __drm_fb_helper_write(struct fb_info *info, const char __user 
*buf, size_t count,
+loff_t *ppos, drm_fb_helper_write_screen 
write_screen)
+{
+   loff_t pos = *ppos;
+   size_t total_size;
+   ssize_t ret;
+   int err = 0;
+
+   if (info->screen_size)
+   total_size = info->screen_size;
+   else
+   total_size = info->fix.smem_len;
+
+   if (pos > total_size)
+   return -EFBIG;
+   if (count > total_size) {
+   err = -EFBIG;
+   count = total_size;
+   }
+   if (total_size - count < pos) {
+   if (!err)
+   err = -ENOSPC;
+   count = total_size - pos;
+   }
+
+   if (info->fbops->fb_sync)
+   info->fbops->fb_sync(info);
+
+   /*
+* Copy to framebuffer even if we already logged an error. Emulates
+* the behavior of the original fbdev implementation.
+*/
+   ret = write_screen(info, buf, count, pos);
+   if (ret < 0)
+   return ret; /* return last error, if any */
+   else if (!ret)
+   return err; /* return previous error, if any */
+
+   *ppos += ret;
+
+   return ret;
+}
+
+static ssize_t drm_fb_helper_read_screen_buffer(struct fb_info *info, char 
__user *buf,
+   size_t count, loff_t pos)
+{
+   const char *src = info->screen_buffer + pos;
+
+   if (copy_to_user(buf, src, count))
+ 

[PATCH v2 10/21] drm/tve200: Include

[Thomas Zimmermann]

Include  for of_match_ptr().

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/tve200/tve200_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/tve200/tve200_drv.c 
b/drivers/gpu/drm/tve200/tve200_drv.c
index 04db72e3fa9c2..611785e097576 100644
--- a/drivers/gpu/drm/tve200/tve200_drv.c
+++ b/drivers/gpu/drm/tve200/tve200_drv.c
@@ -32,6 +32,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
-- 
2.38.0

[PATCH v2 08/21] drm/rockchip: Don't set struct drm_driver.output_poll_changed

[Thomas Zimmermann]

Don't set struct drm_driver.output_poll_changed. It's used to restore
the fbdev console. But as rockchip uses generic fbdev emulation, the
console is being restored by the DRM client helpers already. See the
functions drm_kms_helper_hotplug_event() and
drm_kms_helper_connector_hotplug_event() in drm_probe_helper.c.

v2:
* fix commit description (Christian)

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index 092bf863110b7..7de64b0ad047f 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -9,7 +9,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -101,7 +100,6 @@ rockchip_fb_create(struct drm_device *dev, struct drm_file 
*file,
 
 static const struct drm_mode_config_funcs rockchip_drm_mode_config_funcs = {
.fb_create = rockchip_fb_create,
-   .output_poll_changed = drm_fb_helper_output_poll_changed,
.atomic_check = drm_atomic_helper_check,
.atomic_commit = drm_atomic_helper_commit,
 };
-- 
2.38.0

[PATCH v2 09/21] drm/panel-ili9341: Include

[Thomas Zimmermann]

Include  for devm_of_find_backlight().

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c 
b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
index 39dc40cf681f0..b59472c29a40d 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
@@ -18,6 +18,7 @@
  * Copyright 2018 David Lechner 
  */
 
+#include 
 #include 
 #include 
 #include 
-- 
2.38.0

[PATCH v2 18/21] drm/fb_helper: Minimize damage-helper overhead

[Thomas Zimmermann]

Pull the test for fb_dirty into the caller to avoid extra work
if no callback has been set. In this case no damage handling is
required and no damage area needs to be computed. Print a warning
if the damage worker runs without getting an fb_dirty callback.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/drm_fb_helper.c | 90 ++---
 1 file changed, 60 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 836523aef6a27..fbc5c5445fdb0 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -449,12 +449,13 @@ static int drm_fb_helper_damage_blit(struct drm_fb_helper 
*fb_helper,
 static void drm_fb_helper_damage_work(struct work_struct *work)
 {
struct drm_fb_helper *helper = container_of(work, struct drm_fb_helper, 
damage_work);
+   struct drm_device *dev = helper->dev;
struct drm_clip_rect *clip = &helper->damage_clip;
struct drm_clip_rect clip_copy;
unsigned long flags;
int ret;
 
-   if (!helper->funcs->fb_dirty)
+   if (drm_WARN_ON_ONCE(dev, !helper->funcs->fb_dirty))
return;
 
spin_lock_irqsave(&helper->damage_lock, flags);
@@ -659,16 +660,12 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
 }
 EXPORT_SYMBOL(drm_fb_helper_fini);
 
-static void drm_fb_helper_damage(struct fb_info *info, u32 x, u32 y,
+static void drm_fb_helper_damage(struct drm_fb_helper *helper, u32 x, u32 y,
 u32 width, u32 height)
 {
-   struct drm_fb_helper *helper = info->par;
struct drm_clip_rect *clip = &helper->damage_clip;
unsigned long flags;
 
-   if (!helper->funcs->fb_dirty)
-   return;
-
spin_lock_irqsave(&helper->damage_lock, flags);
clip->x1 = min_t(u32, clip->x1, x);
clip->y1 = min_t(u32, clip->y1, y);
@@ -718,6 +715,7 @@ static void drm_fb_helper_memory_range_to_clip(struct 
fb_info *info, off_t off,
  */
 void drm_fb_helper_deferred_io(struct fb_info *info, struct list_head 
*pagereflist)
 {
+   struct drm_fb_helper *helper = info->par;
unsigned long start, end, min_off, max_off;
struct fb_deferred_io_pageref *pageref;
struct drm_rect damage_area;
@@ -733,17 +731,19 @@ void drm_fb_helper_deferred_io(struct fb_info *info, 
struct list_head *pagerefli
if (min_off >= max_off)
return;
 
-   /*
-* As we can only track pages, we might reach beyond the end
-* of the screen and account for non-existing scanlines. Hence,
-* keep the covered memory area within the screen buffer.
-*/
-   max_off = min(max_off, info->screen_size);
+   if (helper->funcs->fb_dirty) {
+   /*
+* As we can only track pages, we might reach beyond the end
+* of the screen and account for non-existing scanlines. Hence,
+* keep the covered memory area within the screen buffer.
+*/
+   max_off = min(max_off, info->screen_size);
 
-   drm_fb_helper_memory_range_to_clip(info, min_off, max_off - min_off, 
&damage_area);
-   drm_fb_helper_damage(info, damage_area.x1, damage_area.y1,
-drm_rect_width(&damage_area),
-drm_rect_height(&damage_area));
+   drm_fb_helper_memory_range_to_clip(info, min_off, max_off - 
min_off, &damage_area);
+   drm_fb_helper_damage(helper, damage_area.x1, damage_area.y1,
+drm_rect_width(&damage_area),
+drm_rect_height(&damage_area));
+   }
 }
 EXPORT_SYMBOL(drm_fb_helper_deferred_io);
 
@@ -877,6 +877,7 @@ static ssize_t drm_fb_helper_write_screen_buffer(struct 
fb_info *info, const cha
 ssize_t drm_fb_helper_sys_write(struct fb_info *info, const char __user *buf,
size_t count, loff_t *ppos)
 {
+   struct drm_fb_helper *helper = info->par;
loff_t pos = *ppos;
ssize_t ret;
struct drm_rect damage_area;
@@ -885,10 +886,12 @@ ssize_t drm_fb_helper_sys_write(struct fb_info *info, 
const char __user *buf,
if (ret <= 0)
return ret;
 
-   drm_fb_helper_memory_range_to_clip(info, pos, ret, &damage_area);
-   drm_fb_helper_damage(info, damage_area.x1, damage_area.y1,
-drm_rect_width(&damage_area),
-drm_rect_height(&damage_area));
+   if (helper->funcs->fb_dirty) {
+   drm_fb_helper_memory_range_to_clip(info, pos, ret, 
&damage_area);
+   drm_fb_helper_damage(helper, damage_area.x1, damage_area.y1,
+drm_rect_width(&damage_area),
+drm_rect_height(&damage_area));
+   }
 
return ret;
 }
@@ -904,8 +907,12 @@ EXPORT_SYMBOL(drm_fb_helper_sys_write);
 void

[PATCH v2 13/21] drm/fb-helper: Rename drm_fb_helper_alloc_fbi() to use _info postfix

[Thomas Zimmermann]

Rename drm_fb_helper_alloc_fbi() to drm_fb_helper_alloc_info() as
part of unifying the naming within fbdev helpers. Adapt drivers. No
functional changes.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/armada/armada_fbdev.c  | 2 +-
 drivers/gpu/drm/drm_fb_helper.c| 8 
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c  | 2 +-
 drivers/gpu/drm/gma500/framebuffer.c   | 2 +-
 drivers/gpu/drm/i915/display/intel_fbdev.c | 2 +-
 drivers/gpu/drm/msm/msm_fbdev.c| 2 +-
 drivers/gpu/drm/nouveau/nouveau_fbcon.c| 2 +-
 drivers/gpu/drm/omapdrm/omap_fbdev.c   | 2 +-
 drivers/gpu/drm/radeon/radeon_fb.c | 2 +-
 drivers/gpu/drm/tegra/fb.c | 2 +-
 include/drm/drm_fb_helper.h| 4 ++--
 11 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_fbdev.c 
b/drivers/gpu/drm/armada/armada_fbdev.c
index 38f5170c0fea6..eaae98d9377ae 100644
--- a/drivers/gpu/drm/armada/armada_fbdev.c
+++ b/drivers/gpu/drm/armada/armada_fbdev.c
@@ -72,7 +72,7 @@ static int armada_fbdev_create(struct drm_fb_helper *fbh,
if (IS_ERR(dfb))
return PTR_ERR(dfb);
 
-   info = drm_fb_helper_alloc_fbi(fbh);
+   info = drm_fb_helper_alloc_info(fbh);
if (IS_ERR(info)) {
ret = PTR_ERR(info);
goto err_fballoc;
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 480bf4f568b7b..881e6a04fa706 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -558,7 +558,7 @@ int drm_fb_helper_init(struct drm_device *dev,
 EXPORT_SYMBOL(drm_fb_helper_init);
 
 /**
- * drm_fb_helper_alloc_fbi - allocate fb_info and some of its members
+ * drm_fb_helper_alloc_info - allocate fb_info and some of its members
  * @fb_helper: driver-allocated fbdev helper
  *
  * A helper to alloc fb_info and the members cmap and apertures. Called
@@ -570,7 +570,7 @@ EXPORT_SYMBOL(drm_fb_helper_init);
  * fb_info pointer if things went okay, pointer containing error code
  * otherwise
  */
-struct fb_info *drm_fb_helper_alloc_fbi(struct drm_fb_helper *fb_helper)
+struct fb_info *drm_fb_helper_alloc_info(struct drm_fb_helper *fb_helper)
 {
struct device *dev = fb_helper->dev->dev;
struct fb_info *info;
@@ -609,7 +609,7 @@ struct fb_info *drm_fb_helper_alloc_fbi(struct 
drm_fb_helper *fb_helper)
framebuffer_release(info);
return ERR_PTR(ret);
 }
-EXPORT_SYMBOL(drm_fb_helper_alloc_fbi);
+EXPORT_SYMBOL(drm_fb_helper_alloc_info);
 
 /**
  * drm_fb_helper_unregister_fbi - unregister fb_info framebuffer device
@@ -2440,7 +2440,7 @@ static int drm_fb_helper_generic_probe(struct 
drm_fb_helper *fb_helper,
fb_helper->fb = buffer->fb;
fb = buffer->fb;
 
-   fbi = drm_fb_helper_alloc_fbi(fb_helper);
+   fbi = drm_fb_helper_alloc_info(fb_helper);
if (IS_ERR(fbi))
return PTR_ERR(fbi);
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c 
b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index 767afd2bfa822..8741eb0b1b604 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -63,7 +63,7 @@ static int exynos_drm_fbdev_update(struct drm_fb_helper 
*helper,
unsigned int size = fb->width * fb->height * fb->format->cpp[0];
unsigned long offset;
 
-   fbi = drm_fb_helper_alloc_fbi(helper);
+   fbi = drm_fb_helper_alloc_info(helper);
if (IS_ERR(fbi)) {
DRM_DEV_ERROR(to_dma_dev(helper->dev),
  "failed to allocate fb info.\n");
diff --git a/drivers/gpu/drm/gma500/framebuffer.c 
b/drivers/gpu/drm/gma500/framebuffer.c
index 5f502a0048ab8..6d0e3bf6435ee 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -268,7 +268,7 @@ static int psbfb_create(struct drm_fb_helper *fb_helper,
 
memset(dev_priv->vram_addr + backing->offset, 0, size);
 
-   info = drm_fb_helper_alloc_fbi(fb_helper);
+   info = drm_fb_helper_alloc_info(fb_helper);
if (IS_ERR(info)) {
ret = PTR_ERR(info);
goto err_drm_gem_object_put;
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c 
b/drivers/gpu/drm/i915/display/intel_fbdev.c
index d533ecd451025..05b841343ea3e 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -254,7 +254,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
goto out_unlock;
}
 
-   info = drm_fb_helper_alloc_fbi(helper);
+   info = drm_fb_helper_alloc_info(helper);
if (IS_ERR(info)) {
drm_err(&dev_priv->drm, "Failed to allocate fb_info (%pe)\n", 
info);
ret = PTR_ERR(info);
diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
index b373e30003203..4d9a0fcbf95b6 100644
--- a/drivers/gpu/drm/msm/msm_fbdev.c
+++ b/drivers/gpu/dr

[PATCH v2 14/21] drm/fb-helper: Rename drm_fb_helper_unregister_fbi() to use _info postfix

[Thomas Zimmermann]

Rename drm_fb_helper_unregister_fbi() to drm_fb_helper_unregister_info()
as part of unifying the naming within fbdev helpers. Adapt drivers. No
functional changes.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/armada/armada_fbdev.c  | 2 +-
 drivers/gpu/drm/drm_fb_helper.c| 8 
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c  | 2 +-
 drivers/gpu/drm/gma500/framebuffer.c   | 2 +-
 drivers/gpu/drm/i915/display/intel_fbdev.c | 2 +-
 drivers/gpu/drm/msm/msm_fbdev.c| 2 +-
 drivers/gpu/drm/nouveau/nouveau_fbcon.c| 2 +-
 drivers/gpu/drm/omapdrm/omap_fbdev.c   | 2 +-
 drivers/gpu/drm/radeon/radeon_fb.c | 2 +-
 drivers/gpu/drm/tegra/fb.c | 2 +-
 include/drm/drm_fb_helper.h| 4 ++--
 11 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_fbdev.c 
b/drivers/gpu/drm/armada/armada_fbdev.c
index eaae98d9377ae..f02f6a5ba8320 100644
--- a/drivers/gpu/drm/armada/armada_fbdev.c
+++ b/drivers/gpu/drm/armada/armada_fbdev.c
@@ -155,7 +155,7 @@ void armada_fbdev_fini(struct drm_device *dev)
struct drm_fb_helper *fbh = priv->fbdev;
 
if (fbh) {
-   drm_fb_helper_unregister_fbi(fbh);
+   drm_fb_helper_unregister_info(fbh);
 
drm_fb_helper_fini(fbh);
 
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 881e6a04fa706..bfbb2af144060 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -612,19 +612,19 @@ struct fb_info *drm_fb_helper_alloc_info(struct 
drm_fb_helper *fb_helper)
 EXPORT_SYMBOL(drm_fb_helper_alloc_info);
 
 /**
- * drm_fb_helper_unregister_fbi - unregister fb_info framebuffer device
+ * drm_fb_helper_unregister_info - unregister fb_info framebuffer device
  * @fb_helper: driver-allocated fbdev helper, can be NULL
  *
  * A wrapper around unregister_framebuffer, to release the fb_info
  * framebuffer device. This must be called before releasing all resources for
  * @fb_helper by calling drm_fb_helper_fini().
  */
-void drm_fb_helper_unregister_fbi(struct drm_fb_helper *fb_helper)
+void drm_fb_helper_unregister_info(struct drm_fb_helper *fb_helper)
 {
if (fb_helper && fb_helper->info)
unregister_framebuffer(fb_helper->info);
 }
-EXPORT_SYMBOL(drm_fb_helper_unregister_fbi);
+EXPORT_SYMBOL(drm_fb_helper_unregister_info);
 
 /**
  * drm_fb_helper_fini - finialize a &struct drm_fb_helper
@@ -2497,7 +2497,7 @@ static void drm_fbdev_client_unregister(struct 
drm_client_dev *client)
 
if (fb_helper->info)
/* drm_fbdev_fb_destroy() takes care of cleanup */
-   drm_fb_helper_unregister_fbi(fb_helper);
+   drm_fb_helper_unregister_info(fb_helper);
else
drm_fbdev_release(fb_helper);
 }
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c 
b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index 8741eb0b1b604..86c489d945849 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -201,7 +201,7 @@ static void exynos_drm_fbdev_destroy(struct drm_device *dev,
drm_framebuffer_remove(fb);
}
 
-   drm_fb_helper_unregister_fbi(fb_helper);
+   drm_fb_helper_unregister_info(fb_helper);
 
drm_fb_helper_fini(fb_helper);
 }
diff --git a/drivers/gpu/drm/gma500/framebuffer.c 
b/drivers/gpu/drm/gma500/framebuffer.c
index 6d0e3bf6435ee..6098d936e44b6 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -383,7 +383,7 @@ static int psb_fbdev_destroy(struct drm_device *dev,
 {
struct drm_framebuffer *fb = fb_helper->fb;
 
-   drm_fb_helper_unregister_fbi(fb_helper);
+   drm_fb_helper_unregister_info(fb_helper);
 
drm_fb_helper_fini(fb_helper);
drm_framebuffer_unregister_private(fb);
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c 
b/drivers/gpu/drm/i915/display/intel_fbdev.c
index 05b841343ea3e..1b576c859837b 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -584,7 +584,7 @@ void intel_fbdev_unregister(struct drm_i915_private 
*dev_priv)
if (!current_is_async())
intel_fbdev_sync(ifbdev);
 
-   drm_fb_helper_unregister_fbi(&ifbdev->helper);
+   drm_fb_helper_unregister_info(&ifbdev->helper);
 }
 
 void intel_fbdev_fini(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
index 4d9a0fcbf95b6..31e1e30cb52a2 100644
--- a/drivers/gpu/drm/msm/msm_fbdev.c
+++ b/drivers/gpu/drm/msm/msm_fbdev.c
@@ -182,7 +182,7 @@ void msm_fbdev_free(struct drm_device *dev)
 
DBG();
 
-   drm_fb_helper_unregister_fbi(helper);
+   drm_fb_helper_unregister_info(helper);
 
drm_fb_helper_fini(helper);
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c 
b/drivers/gpu/drm/nouveau/nouveau_fbc

[PATCH v2 21/21] drm/fb-helper: Remove unnecessary include statements

[Thomas Zimmermann]

Remove include statements for  where it is not
required (i.e., most of them). In a few places include other header
files that are required by the source code.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c  | 1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h| 1 -
 drivers/gpu/drm/arm/hdlcd_crtc.c| 1 -
 drivers/gpu/drm/ast/ast_drv.h   | 1 -
 drivers/gpu/drm/bridge/tc358762.c   | 2 +-
 drivers/gpu/drm/drm_crtc_helper.c   | 1 -
 drivers/gpu/drm/drm_gem_framebuffer_helper.c| 1 -
 drivers/gpu/drm/drm_probe_helper.c  | 1 -
 drivers/gpu/drm/etnaviv/etnaviv_drv.h   | 3 ++-
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 1 -
 drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 1 -
 drivers/gpu/drm/imx/imx-ldb.c   | 2 +-
 drivers/gpu/drm/imx/imx-tve.c   | 1 -
 drivers/gpu/drm/imx/parallel-display.c  | 2 +-
 drivers/gpu/drm/kmb/kmb_plane.c | 1 -
 drivers/gpu/drm/mgag200/mgag200_drv.h   | 1 -
 drivers/gpu/drm/qxl/qxl_drv.h   | 1 -
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 2 +-
 drivers/gpu/drm/tidss/tidss_kms.c   | 1 -
 drivers/gpu/drm/v3d/v3d_drv.c   | 1 -
 drivers/gpu/drm/vboxvideo/vbox_main.c   | 1 -
 drivers/gpu/drm/virtio/virtgpu_drv.h| 1 -
 drivers/gpu/drm/xen/xen_drm_front_gem.c | 1 -
 24 files changed, 6 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index 491d4846fc02c..e1320edfc5274 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -26,7 +26,6 @@
 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include "amdgpu.h"
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index dd6f9ae6fbe9f..b32b387599b5c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -40,7 +40,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index 37322550d7508..8a39300b1a845 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -36,7 +36,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 7030339fa2323..ddbe1dd2d44ef 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -19,7 +19,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 74f41282444f6..d51b81fea9c80 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -38,7 +38,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #define DRIVER_AUTHOR  "Dave Airlie"
 
diff --git a/drivers/gpu/drm/bridge/tc358762.c 
b/drivers/gpu/drm/bridge/tc358762.c
index 7f4fce1aa9988..0b6a284368859 100644
--- a/drivers/gpu/drm/bridge/tc358762.c
+++ b/drivers/gpu/drm/bridge/tc358762.c
@@ -11,6 +11,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -19,7 +20,6 @@
 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/drivers/gpu/drm/drm_crtc_helper.c 
b/drivers/gpu/drm/drm_crtc_helper.c
index f5fb22e0d0337..a209659a996c7 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -43,7 +43,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c 
b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index e35e224e6303a..e93533b86037f 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -9,7 +9,6 @@
 #include 
 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/drivers/gpu/drm/drm_probe_helper.c 
b/drivers/gpu/drm/drm_probe_helper.c
index 69b0b2b9cc1c5..ef2b41b2eb7b8 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -36,7 +36,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h 
b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index f32f4771dada7..2bb4c25565dcb 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -6,13 +6,14 @@
 #ifndef __ETNAVIV_DRV_H__
 #define __ETNAVIV_DRV_H__
 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
 
-#include 
+#include 
 #include 
 #include 
 #include 
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h 
b/drivers/gpu/drm

[PATCH v2 11/21] drm/fb-helper: Cleanup include statements in header file

[Thomas Zimmermann]

Only include what we have to.

Signed-off-by: Thomas Zimmermann 
---
 include/drm/drm_fb_helper.h | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index fddd0d1af6891..e923089522896 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -32,11 +32,9 @@
 
 struct drm_fb_helper;
 
-#include 
-#include 
-#include 
 #include 
-#include 
+
+#include 
 
 enum mode_set_atomic {
LEAVE_ATOMIC_MODE_SET,
-- 
2.38.0

[PATCH v2 15/21] drm/fb-helper: Disconnect damage worker from update logic

[Thomas Zimmermann]

The fbdev helpers implement a damage worker that forwards fbdev
updates to the DRM driver. The worker's update logic depends on
the generic fbdev emulation. Separate the two via function pointer.

The generic fbdev emulation sets struct drm_fb_helper_funcs.fb_dirty,
a new callback that hides the update logic from the damage worker.
It's not possible to use the generic logic with other fbdev emulation,
because it contains additional code for the shadow buffering that
the generic emulation employs.

DRM drivers with internal fbdev emulation can set fb_dirty to their
own implementation if they require damage handling; although no such
drivers currently exist.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/drm_fb_helper.c | 75 -
 include/drm/drm_fb_helper.h | 15 +++
 2 files changed, 61 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index bfbb2af144060..f6d22cc4cd876 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -448,35 +448,24 @@ static int drm_fb_helper_damage_blit(struct drm_fb_helper 
*fb_helper,
 
 static void drm_fb_helper_damage_work(struct work_struct *work)
 {
-   struct drm_fb_helper *helper = container_of(work, struct drm_fb_helper,
-   damage_work);
-   struct drm_device *dev = helper->dev;
+   struct drm_fb_helper *helper = container_of(work, struct drm_fb_helper, 
damage_work);
struct drm_clip_rect *clip = &helper->damage_clip;
struct drm_clip_rect clip_copy;
unsigned long flags;
int ret;
 
+   if (!helper->funcs->fb_dirty)
+   return;
+
spin_lock_irqsave(&helper->damage_lock, flags);
clip_copy = *clip;
clip->x1 = clip->y1 = ~0;
clip->x2 = clip->y2 = 0;
spin_unlock_irqrestore(&helper->damage_lock, flags);
 
-   /* Call damage handlers only if necessary */
-   if (!(clip_copy.x1 < clip_copy.x2 && clip_copy.y1 < clip_copy.y2))
-   return;
-
-   if (helper->buffer) {
-   ret = drm_fb_helper_damage_blit(helper, &clip_copy);
-   if (drm_WARN_ONCE(dev, ret, "Damage blitter failed: ret=%d\n", 
ret))
-   goto err;
-   }
-
-   if (helper->fb->funcs->dirty) {
-   ret = helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, 
&clip_copy, 1);
-   if (drm_WARN_ONCE(dev, ret, "Dirty helper failed: ret=%d\n", 
ret))
-   goto err;
-   }
+   ret = helper->funcs->fb_dirty(helper, &clip_copy);
+   if (ret)
+   goto err;
 
return;
 
@@ -670,16 +659,6 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
 }
 EXPORT_SYMBOL(drm_fb_helper_fini);
 
-static bool drm_fbdev_use_shadow_fb(struct drm_fb_helper *fb_helper)
-{
-   struct drm_device *dev = fb_helper->dev;
-   struct drm_framebuffer *fb = fb_helper->fb;
-
-   return dev->mode_config.prefer_shadow_fbdev ||
-  dev->mode_config.prefer_shadow ||
-  fb->funcs->dirty;
-}
-
 static void drm_fb_helper_damage(struct fb_info *info, u32 x, u32 y,
 u32 width, u32 height)
 {
@@ -687,7 +666,7 @@ static void drm_fb_helper_damage(struct fb_info *info, u32 
x, u32 y,
struct drm_clip_rect *clip = &helper->damage_clip;
unsigned long flags;
 
-   if (!drm_fbdev_use_shadow_fb(helper))
+   if (!helper->funcs->fb_dirty)
return;
 
spin_lock_irqsave(&helper->damage_lock, flags);
@@ -2111,6 +2090,16 @@ void drm_fb_helper_output_poll_changed(struct drm_device 
*dev)
 }
 EXPORT_SYMBOL(drm_fb_helper_output_poll_changed);
 
+static bool drm_fbdev_use_shadow_fb(struct drm_fb_helper *fb_helper)
+{
+   struct drm_device *dev = fb_helper->dev;
+   struct drm_framebuffer *fb = fb_helper->fb;
+
+   return dev->mode_config.prefer_shadow_fbdev ||
+  dev->mode_config.prefer_shadow ||
+  fb->funcs->dirty;
+}
+
 /* @user: 1=userspace, 0=fbcon */
 static int drm_fbdev_fb_open(struct fb_info *info, int user)
 {
@@ -2487,8 +2476,36 @@ static int drm_fb_helper_generic_probe(struct 
drm_fb_helper *fb_helper,
return 0;
 }
 
+static int drm_fbdev_fb_dirty(struct drm_fb_helper *helper, struct 
drm_clip_rect *clip)
+{
+   struct drm_device *dev = helper->dev;
+   int ret;
+
+   if (!drm_fbdev_use_shadow_fb(helper))
+   return 0;
+
+   /* Call damage handlers only if necessary */
+   if (!(clip->x1 < clip->x2 && clip->y1 < clip->y2))
+   return 0;
+
+   if (helper->buffer) {
+   ret = drm_fb_helper_damage_blit(helper, clip);
+   if (drm_WARN_ONCE(dev, ret, "Damage blitter failed: ret=%d\n", 
ret))
+   return ret;
+   }
+
+   if (helper->fb->funcs->dirty) {
+   ret = helper->fb->funcs->dirty(helper->fb, 

[PATCH v2 06/21] drm/ingenic: Don't set struct drm_driver.output_poll_changed

[Thomas Zimmermann]

Don't set struct drm_driver.output_poll_changed. It's used to restore
the fbdev console. But as ingenic uses generic fbdev emulation, the
console is being restored by the DRM client helpers already. See the
functions drm_kms_helper_hotplug_event() and
drm_kms_helper_connector_hotplug_event() in drm_probe_helper.c.

v2:
* fix commit description (Christian, Sergey)

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c 
b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index ab0515d2c420a..99f86f1ba8bee 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -1018,7 +1018,6 @@ static const struct drm_bridge_funcs 
ingenic_drm_bridge_funcs = {
 
 static const struct drm_mode_config_funcs ingenic_drm_mode_config_funcs = {
.fb_create  = ingenic_drm_gem_fb_create,
-   .output_poll_changed= drm_fb_helper_output_poll_changed,
.atomic_check   = drm_atomic_helper_check,
.atomic_commit  = drm_atomic_helper_commit,
 };
-- 
2.38.0

[PATCH v2 05/21] drm/imx/dcss: Don't set struct drm_driver.output_poll_changed

[Thomas Zimmermann]

Don't set struct drm_driver.output_poll_changed. It's used to restore
the fbdev console. But as DCSS uses generic fbdev emulation, the
console is being restored by the DRM client helpers already. See the
functions drm_kms_helper_hotplug_event() and
drm_kms_helper_connector_hotplug_event() in drm_probe_helper.c.

v2:
* fix commit description (Christian)

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/imx/dcss/dcss-kms.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/imx/dcss/dcss-kms.c 
b/drivers/gpu/drm/imx/dcss/dcss-kms.c
index b4f82ebca5325..1defd6a40f11d 100644
--- a/drivers/gpu/drm/imx/dcss/dcss-kms.c
+++ b/drivers/gpu/drm/imx/dcss/dcss-kms.c
@@ -21,7 +21,6 @@ DEFINE_DRM_GEM_DMA_FOPS(dcss_cma_fops);
 
 static const struct drm_mode_config_funcs dcss_drm_mode_config_funcs = {
.fb_create = drm_gem_fb_create,
-   .output_poll_changed = drm_fb_helper_output_poll_changed,
.atomic_check = drm_atomic_helper_check,
.atomic_commit = drm_atomic_helper_commit,
 };
-- 
2.38.0

[PATCH v2 16/21] drm/fb-helper: Call fb_sync in I/O functions

[Thomas Zimmermann]

Call struct fb_ops.fb_sync in drm_fbdev_{read,write}() to mimic the
behavior of fbdev. Fbdev implementations of fb_read and fb_write in
struct fb_ops invoke fb_sync to synchronize with outstanding operations
before I/O. Doing the same in DRM implementations will allow us to use
them throughout DRM drivers.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/drm_fb_helper.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index f6d22cc4cd876..379e0d2f67198 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2246,6 +2246,9 @@ static ssize_t drm_fbdev_fb_read(struct fb_info *info, 
char __user *buf,
if (total_size - count < pos)
count = total_size - pos;
 
+   if (info->fbops->fb_sync)
+   info->fbops->fb_sync(info);
+
if (drm_fbdev_use_iomem(info))
ret = fb_read_screen_base(info, buf, count, pos);
else
@@ -2327,6 +2330,9 @@ static ssize_t drm_fbdev_fb_write(struct fb_info *info, 
const char __user *buf,
count = total_size - pos;
}
 
+   if (info->fbops->fb_sync)
+   info->fbops->fb_sync(info);
+
/*
 * Copy to framebuffer even if we already logged an error. Emulates
 * the behavior of the original fbdev implementation.
-- 
2.38.0

[PATCH v2 07/21] drm/logicvc: Don't set struct drm_driver.output_poll_changed

[Thomas Zimmermann]

Don't set struct drm_driver.output_poll_changed. It's used to restore
the fbdev console. But as logicvc uses generic fbdev emulation, the
console is being restored by the DRM client helpers already. See the
functions drm_kms_helper_hotplug_event() and
drm_kms_helper_connector_hotplug_event() in drm_probe_helper.c.

v2:
* fix commit description (Christian)

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/logicvc/logicvc_mode.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/logicvc/logicvc_mode.c 
b/drivers/gpu/drm/logicvc/logicvc_mode.c
index d8207ffda1af9..9971950ebd4ee 100644
--- a/drivers/gpu/drm/logicvc/logicvc_mode.c
+++ b/drivers/gpu/drm/logicvc/logicvc_mode.c
@@ -10,7 +10,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -26,7 +25,6 @@
 
 static const struct drm_mode_config_funcs logicvc_mode_config_funcs = {
.fb_create  = drm_gem_fb_create,
-   .output_poll_changed= drm_fb_helper_output_poll_changed,
.atomic_check   = drm_atomic_helper_check,
.atomic_commit  = drm_atomic_helper_commit,
 };
-- 
2.38.0

[PATCH v2 03/21] drm/vboxvideo: Don't set struct drm_driver.lastclose

[Thomas Zimmermann]

Don't set struct drm_driver.lastclose. It's used to restore the
fbdev console. But as vboxvideo uses generic fbdev emulation, the
console is being restored by the DRM client helpers already. See
the call to drm_client_dev_restore() in drm_lastclose().

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/vboxvideo/vbox_drv.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c 
b/drivers/gpu/drm/vboxvideo/vbox_drv.c
index f4f2bd79a7cb6..1cd716eb17a1c 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_drv.c
+++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c
@@ -178,8 +178,6 @@ static const struct drm_driver driver = {
.driver_features =
DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC,
 
-   .lastclose = drm_fb_helper_lastclose,
-
.fops = &vbox_fops,
.name = DRIVER_NAME,
.desc = DRIVER_DESC,
-- 
2.38.0

[PATCH v2 04/21] drm/amdgpu: Don't set struct drm_driver.output_poll_changed

[Thomas Zimmermann]

Don't set struct drm_driver.output_poll_changed. It's used to restore
the fbdev console. But as amdgpu uses generic fbdev emulation, the
console is being restored by the DRM client helpers already. See the
functions drm_kms_helper_hotplug_event() and
drm_kms_helper_connector_hotplug_event() in drm_probe_helper.c.

v2:
* fix commit description (Christian)

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   | 1 -
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 --
 2 files changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 1a06b8d724f39..dd6f9ae6fbe9f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -1214,7 +1214,6 @@ amdgpu_display_user_framebuffer_create(struct drm_device 
*dev,
 
 const struct drm_mode_config_funcs amdgpu_mode_funcs = {
.fb_create = amdgpu_display_user_framebuffer_create,
-   .output_poll_changed = drm_fb_helper_output_poll_changed,
 };
 
 static const struct drm_prop_enum_list amdgpu_underscan_enum_list[] =
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 0db2a88cd4d7b..528b8be516ff6 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -82,7 +82,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -2810,7 +2809,6 @@ const struct amdgpu_ip_block_version dm_ip_block =
 static const struct drm_mode_config_funcs amdgpu_dm_mode_funcs = {
.fb_create = amdgpu_display_user_framebuffer_create,
.get_format_info = amd_get_format_info,
-   .output_poll_changed = drm_fb_helper_output_poll_changed,
.atomic_check = amdgpu_dm_atomic_check,
.atomic_commit = drm_atomic_helper_commit,
 };
-- 
2.38.0

[PATCH v2 12/21] drm/fb_helper: Rename field fbdev to info in struct drm_fb_helper

[Thomas Zimmermann]

Rename struct drm_fb_helper.fbdev to info. The current name is
misleading as it overlaps with generic fbdev naming conventions.
Adapt to the usual naming in fbdev drivers by calling the field
'info'. No functional changes.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/drm_fb_helper.c| 40 +++---
 drivers/gpu/drm/i915/display/intel_fbdev.c |  2 +-
 drivers/gpu/drm/nouveau/nouveau_fbcon.c| 23 ++---
 drivers/gpu/drm/omapdrm/omap_fbdev.c   |  2 +-
 drivers/gpu/drm/tegra/fb.c |  2 +-
 include/drm/drm_fb_helper.h|  4 +--
 6 files changed, 36 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 71edb80fe0fb9..480bf4f568b7b 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -368,7 +368,7 @@ static void drm_fb_helper_resume_worker(struct work_struct 
*work)
resume_work);
 
console_lock();
-   fb_set_suspend(helper->fbdev, 0);
+   fb_set_suspend(helper->info, 0);
console_unlock();
 }
 
@@ -401,7 +401,7 @@ static void drm_fb_helper_damage_blit_real(struct 
drm_fb_helper *fb_helper,
break;
}
 
-   src = fb_helper->fbdev->screen_buffer + offset;
+   src = fb_helper->info->screen_buffer + offset;
iosys_map_incr(dst, offset); /* go to first pixel within clip rect */
 
for (y = clip->y1; y < clip->y2; y++) {
@@ -598,7 +598,7 @@ struct fb_info *drm_fb_helper_alloc_fbi(struct 
drm_fb_helper *fb_helper)
goto err_free_cmap;
}
 
-   fb_helper->fbdev = info;
+   fb_helper->info = info;
info->skip_vt_switch = true;
 
return info;
@@ -621,8 +621,8 @@ EXPORT_SYMBOL(drm_fb_helper_alloc_fbi);
  */
 void drm_fb_helper_unregister_fbi(struct drm_fb_helper *fb_helper)
 {
-   if (fb_helper && fb_helper->fbdev)
-   unregister_framebuffer(fb_helper->fbdev);
+   if (fb_helper && fb_helper->info)
+   unregister_framebuffer(fb_helper->info);
 }
 EXPORT_SYMBOL(drm_fb_helper_unregister_fbi);
 
@@ -647,13 +647,13 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
cancel_work_sync(&fb_helper->resume_work);
cancel_work_sync(&fb_helper->damage_work);
 
-   info = fb_helper->fbdev;
+   info = fb_helper->info;
if (info) {
if (info->cmap.len)
fb_dealloc_cmap(&info->cmap);
framebuffer_release(info);
}
-   fb_helper->fbdev = NULL;
+   fb_helper->info = NULL;
 
mutex_lock(&kernel_fb_helper_lock);
if (!list_empty(&fb_helper->kernel_fb_list)) {
@@ -914,8 +914,8 @@ EXPORT_SYMBOL(drm_fb_helper_cfb_imageblit);
  */
 void drm_fb_helper_set_suspend(struct drm_fb_helper *fb_helper, bool suspend)
 {
-   if (fb_helper && fb_helper->fbdev)
-   fb_set_suspend(fb_helper->fbdev, suspend);
+   if (fb_helper && fb_helper->info)
+   fb_set_suspend(fb_helper->info, suspend);
 }
 EXPORT_SYMBOL(drm_fb_helper_set_suspend);
 
@@ -938,20 +938,20 @@ EXPORT_SYMBOL(drm_fb_helper_set_suspend);
 void drm_fb_helper_set_suspend_unlocked(struct drm_fb_helper *fb_helper,
bool suspend)
 {
-   if (!fb_helper || !fb_helper->fbdev)
+   if (!fb_helper || !fb_helper->info)
return;
 
/* make sure there's no pending/ongoing resume */
flush_work(&fb_helper->resume_work);
 
if (suspend) {
-   if (fb_helper->fbdev->state != FBINFO_STATE_RUNNING)
+   if (fb_helper->info->state != FBINFO_STATE_RUNNING)
return;
 
console_lock();
 
} else {
-   if (fb_helper->fbdev->state == FBINFO_STATE_RUNNING)
+   if (fb_helper->info->state == FBINFO_STATE_RUNNING)
return;
 
if (!console_trylock()) {
@@ -960,7 +960,7 @@ void drm_fb_helper_set_suspend_unlocked(struct 
drm_fb_helper *fb_helper,
}
}
 
-   fb_set_suspend(fb_helper->fbdev, suspend);
+   fb_set_suspend(fb_helper->info, suspend);
console_unlock();
 }
 EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
@@ -1850,7 +1850,7 @@ EXPORT_SYMBOL(drm_fb_helper_fill_info);
 /*
  * This is a continuation of drm_setup_crtcs() that sets up anything related
  * to the framebuffer. During initialization, drm_setup_crtcs() is called 
before
- * the framebuffer has been allocated (fb_helper->fb and fb_helper->fbdev).
+ * the framebuffer has been allocated (fb_helper->fb and fb_helper->info).
  * So, any setup that touches those fields needs to be done here instead of in
  * drm_setup_crtcs().
  */
@@ -1858,7 +1858,7 @@ static void drm_setup_crtcs_fb(struct drm_fb_helper 
*fb_helper)
 {
struct drm_client_dev *client = &fb_helper->client;
struct drm_connector_l

[PATCH v2 02/21] drm/mcde: Don't set struct drm_driver.lastclose

[Thomas Zimmermann]

Don't set struct drm_driver.lastclose. It's used to restore the
fbdev console. But as mcde uses generic fbdev emulation, the
console is being restored by the DRM client helpers already. See
the call to drm_client_dev_restore() in drm_lastclose().

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/mcde/mcde_drv.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/mcde/mcde_drv.c b/drivers/gpu/drm/mcde/mcde_drv.c
index 1c4482ad507d9..38c3907bb151a 100644
--- a/drivers/gpu/drm/mcde/mcde_drv.c
+++ b/drivers/gpu/drm/mcde/mcde_drv.c
@@ -203,7 +203,6 @@ DEFINE_DRM_GEM_DMA_FOPS(drm_fops);
 static const struct drm_driver mcde_drm_driver = {
.driver_features =
DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC,
-   .lastclose = drm_fb_helper_lastclose,
.ioctls = NULL,
.fops = &drm_fops,
.name = "mcde",
-- 
2.38.0

[PATCH v2 01/21] drm/komeda: Don't set struct drm_driver.lastclose

[Thomas Zimmermann]

Don't set struct drm_driver.lastclose. It's used to restore the
fbdev console. But as komeda uses generic fbdev emulation, the
console is being restored by the DRM client helpers already. See
the call to drm_client_dev_restore() in drm_lastclose().

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c 
b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
index 451746ebbe713..62dc64550793e 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
@@ -10,7 +10,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -59,7 +58,6 @@ static irqreturn_t komeda_kms_irq_handler(int irq, void *data)
 
 static const struct drm_driver komeda_kms_driver = {
.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
-   .lastclose  = drm_fb_helper_lastclose,
DRM_GEM_DMA_DRIVER_OPS_WITH_DUMB_CREATE(komeda_gem_dma_dumb_create),
.fops = &komeda_cma_fops,
.name = "komeda",
-- 
2.38.0

[PATCH v2 00/21] drm/fb-helper: Untangle fbdev emulation and helpers

[Thomas Zimmermann]

Separate generic fbdev emulation from the helper code that is shared
among the various fbdev implementations within DRM. Affects many drivers.

It has become apparent that our fully generic fbdev emulation will
never produce optimal results for all drivers. In its current form,
it is also hard to maintain. The goal of this patchset is to improve
readability and streamline the fbdev helper code within DRM. In the
long term, we want to get to a point where drivers or memory managers
can pick and combine the various helpers for optimal fbdev support.

Patches 1 to 8 start by preparing drivers. Setting struct drm_driver's
lastclose and output_poll_changed is not required by generic fbdev
emulation.

Two drivers depend on fb helpers implicitly including other Linux header
files. Fixing this in patches 9 and 10 allows to remove unnecesary include
statements from the fb-helper header in patch 11.

Do some renaming in patches 12 to 14.

There are currently various implementation of the fbdev I/O helpers
with varying feature sets. The fb helpers for fbdev I/O should all call
fb_sync, which is what fbdev's internal implementation does. For DRM,
damage handling needs to be performed after updating a framebuffer. The
damage worker is part of the fb helpers, but the actual update logic only
works with generic fbdev emulation. Separate the two, which also gives
other drivers an option to set their own damage handling if neccessary.
The full-featured I/O helpers can be moved under a shared implementation
and called by all drivers. Patches 15 to 18 resolve these issues.

Patch 19 changes fbdev disablement to work at the level of display
detection. If disabled, generic fbdev emulation will be initialized,
but no display will be detected. It can later be enabled by changing
the parameter in sysfs and plugging in a connector.

Patches 20 and 21 move the generic fbdev emulation into their own source
and header files and clean up the include statements throughout DRM. Many
drivers only call drm_fbdev_generic_setup() and can avoid including other
Linux header files.

Built on x86-64, aarch64, arm, ppc64le. Tested with various combinations
of bochs, i915, simpledrm.

v2:
* fixed commit descriptions (Christian, Sergey)

Thomas Zimmermann (21):
  drm/komeda: Don't set struct drm_driver.lastclose
  drm/mcde: Don't set struct drm_driver.lastclose
  drm/vboxvideo: Don't set struct drm_driver.lastclose
  drm/amdgpu: Don't set struct drm_driver.output_poll_changed
  drm/imx/dcss: Don't set struct drm_driver.output_poll_changed
  drm/ingenic: Don't set struct drm_driver.output_poll_changed
  drm/logicvc: Don't set struct drm_driver.output_poll_changed
  drm/rockchip: Don't set struct drm_driver.output_poll_changed
  drm/panel-ili9341: Include 
  drm/tve200: Include 
  drm/fb-helper: Cleanup include statements in header file
  drm/fb_helper: Rename field fbdev to info in struct drm_fb_helper
  drm/fb-helper: Rename drm_fb_helper_alloc_fbi() to use _info postfix
  drm/fb-helper: Rename drm_fb_helper_unregister_fbi() to use _info
postfix
  drm/fb-helper: Disconnect damage worker from update logic
  drm/fb-helper: Call fb_sync in I/O functions
  drm/fb-helper: Perform all fbdev I/O with the same implementation
  drm/fb_helper: Minimize damage-helper overhead
  drm/fb-helper: Always initialize generic fbdev emulation
  drm/fb-helper: Move generic fbdev emulation into separate source file
  drm/fb-helper: Remove unnecessary include statements

 drivers/gpu/drm/Makefile  |2 +-
 .../gpu/drm/amd/amdgpu/amdgpu_connectors.c|1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   |2 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |1 -
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |2 -
 .../gpu/drm/arm/display/komeda/komeda_drv.c   |2 +-
 .../gpu/drm/arm/display/komeda/komeda_kms.c   |2 -
 drivers/gpu/drm/arm/hdlcd_crtc.c  |1 -
 drivers/gpu/drm/arm/hdlcd_drv.c   |2 +-
 drivers/gpu/drm/arm/malidp_drv.c  |2 +-
 drivers/gpu/drm/armada/armada_fbdev.c |6 +-
 drivers/gpu/drm/aspeed/aspeed_gfx_drv.c   |2 +-
 drivers/gpu/drm/ast/ast_drv.c |1 +
 drivers/gpu/drm/ast/ast_drv.h |1 -
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c  |2 +-
 drivers/gpu/drm/bridge/tc358762.c |2 +-
 drivers/gpu/drm/drm_crtc_helper.c |1 -
 drivers/gpu/drm/drm_fb_helper.c   | 1081 ++---
 drivers/gpu/drm/drm_fbdev.c   |  512 
 drivers/gpu/drm/drm_gem_framebuffer_helper.c  |1 -
 drivers/gpu/drm/drm_probe_helper.c|1 -
 drivers/gpu/drm/etnaviv/etnaviv_drv.h |3 +-
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c |6 +-
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c |2 +-
 drivers/gpu/drm/gma500/framebuffer.c  |6 +-
 drivers/gpu/drm/gud/gud_drv.c |2 +-
 .../gpu/drm/hisilicon/h

Re: [PATCH 05/13] drm/amdgpu: drop amdgpu_sync from amdgpu_vmid_grab

[Christian König]

Am 23.10.22 um 03:25 schrieb Luben Tuikov:

On 2022-10-14 04:46, Christian König wrote:

[SNIP]
@@ -254,12 +254,10 @@ static struct dma_fence *amdgpu_job_dependency(struct 
drm_sched_job *sched_job,
DRM_ERROR("Error adding fence (%d)\n", r);
}
  
-	while (fence == NULL && vm && !job->vmid) {

-   r = amdgpu_vmid_grab(vm, ring, &job->sync, job);
+   while (fence == NULL && job->vm && !job->vmid) {

In preliminary application of the patch series, checkpatch.pl complains about 
comparison to NULL,
and wants !fence, instead:

while (!fence && job->vm && !job->vmid) {

I can see that it had been like this before... and I know it's out of the scope 
of this series,
but we should fix this at some point in time.


Thanks for pointing that out. I try to fix it whenever I encounter 
something like this, but sometimes just forget to double check.


Thanks,
Christian.



Regards,
Luben

Re: [PATCH] drm/amd/display: don't print messages that contain %f in dml

[Christian König]

Am 21.10.22 um 18:34 schrieb Hamza Mahfooz:

Unfortunately, printk() doesn't currently support the printing of %f
entries. So, print statements that contain "%f" should be removed.
However, since DC is used on other OSes that can still benefit from the
additional debugging information, we should instead remove the
problematic print statements at compile time.

Reported-by: Jim Cromie 
Signed-off-by: Hamza Mahfooz 
---
  drivers/gpu/drm/amd/display/include/logger_types.h | 11 +--
  1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/include/logger_types.h 
b/drivers/gpu/drm/amd/display/include/logger_types.h
index 3bf08a60c45c..f80630adb5f0 100644
--- a/drivers/gpu/drm/amd/display/include/logger_types.h
+++ b/drivers/gpu/drm/amd/display/include/logger_types.h
@@ -30,6 +30,12 @@
  
  #define MAX_NAME_LEN 32
  
+#define __DC_LOG_IGNORE_FLOATS(fmt, args...)	\

+do {   \
+   if (!strstr((fmt), "%f")) \
+   pr_debug(fmt, ##args);  \
+} while (0)


This is absolutely not sufficient.

Linux drivers must be made for Linux, see the documentation about 
porting drivers.


So just disabling the debug messages won't work in this case. Either 
completely remove or properly fix them.


Regards,
Christian.



+
  #define DC_LOG_ERROR(...) DRM_ERROR(__VA_ARGS__)
  #define DC_LOG_WARNING(...) DRM_WARN(__VA_ARGS__)
  #define DC_LOG_DEBUG(...) DRM_DEBUG_KMS(__VA_ARGS__)
@@ -48,7 +54,8 @@
  #define DC_LOG_MST(...) DRM_DEBUG_KMS(__VA_ARGS__)
  #define DC_LOG_SCALER(...) pr_debug("[SCALER]:"__VA_ARGS__)
  #define DC_LOG_BIOS(...) pr_debug("[BIOS]:"__VA_ARGS__)
-#define DC_LOG_BANDWIDTH_CALCS(...) pr_debug("[BANDWIDTH_CALCS]:"__VA_ARGS__)
+#define DC_LOG_BANDWIDTH_CALCS(args...) \
+   __DC_LOG_IGNORE_FLOATS("[BANDWIDTH_CALCS]:" args)
  #define DC_LOG_BANDWIDTH_VALIDATION(...) DRM_DEBUG_KMS(__VA_ARGS__)
  #define DC_LOG_I2C_AUX(...) DRM_DEBUG_KMS(__VA_ARGS__)
  #define DC_LOG_SYNC(...) DRM_DEBUG_KMS(__VA_ARGS__)
@@ -57,7 +64,7 @@
  #define DC_LOG_DETECTION_EDID_PARSER(...) DRM_DEBUG_KMS(__VA_ARGS__)
  #define DC_LOG_DETECTION_DP_CAPS(...) DRM_DEBUG_KMS(__VA_ARGS__)
  #define DC_LOG_RESOURCE(...) DRM_DEBUG_KMS(__VA_ARGS__)
-#define DC_LOG_DML(...) pr_debug("[DML]:"__VA_ARGS__)
+#define DC_LOG_DML(args...) __DC_LOG_IGNORE_FLOATS("[DML]:" args)
  #define DC_LOG_EVENT_MODE_SET(...) DRM_DEBUG_KMS(__VA_ARGS__)
  #define DC_LOG_EVENT_DETECTION(...) DRM_DEBUG_KMS(__VA_ARGS__)
  #define DC_LOG_EVENT_LINK_TRAINING(...) DRM_DEBUG_KMS(__VA_ARGS__)

Re: [PATCH v7 13/21] media: videobuf2: Prepare to dynamic dma-buf locking specification

[Hans Verkuil]

On 10/17/22 19:22, Dmitry Osipenko wrote:
> Prepare V4L2 memory allocators to the common dynamic dma-buf locking
> convention by starting to use the unlocked versions of dma-buf API
> functions.
> 
> Acked-by: Tomasz Figa 

Acked-by: Hans Verkuil 

Thanks!

Hans

> Acked-by: Christian König 
> Signed-off-by: Dmitry Osipenko 
> ---
>  drivers/media/common/videobuf2/videobuf2-dma-contig.c | 11 ++-
>  drivers/media/common/videobuf2/videobuf2-dma-sg.c |  8 
>  drivers/media/common/videobuf2/videobuf2-vmalloc.c|  6 +++---
>  3 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c 
> b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> index 678b359717c4..79f4d8301fbb 100644
> --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> @@ -101,7 +101,7 @@ static void *vb2_dc_vaddr(struct vb2_buffer *vb, void 
> *buf_priv)
>   if (buf->db_attach) {
>   struct iosys_map map;
>  
> - if (!dma_buf_vmap(buf->db_attach->dmabuf, &map))
> + if (!dma_buf_vmap_unlocked(buf->db_attach->dmabuf, &map))
>   buf->vaddr = map.vaddr;
>  
>   return buf->vaddr;
> @@ -711,7 +711,7 @@ static int vb2_dc_map_dmabuf(void *mem_priv)
>   }
>  
>   /* get the associated scatterlist for this buffer */
> - sgt = dma_buf_map_attachment(buf->db_attach, buf->dma_dir);
> + sgt = dma_buf_map_attachment_unlocked(buf->db_attach, buf->dma_dir);
>   if (IS_ERR(sgt)) {
>   pr_err("Error getting dmabuf scatterlist\n");
>   return -EINVAL;
> @@ -722,7 +722,8 @@ static int vb2_dc_map_dmabuf(void *mem_priv)
>   if (contig_size < buf->size) {
>   pr_err("contiguous chunk is too small %lu/%lu\n",
>  contig_size, buf->size);
> - dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir);
> + dma_buf_unmap_attachment_unlocked(buf->db_attach, sgt,
> +   buf->dma_dir);
>   return -EFAULT;
>   }
>  
> @@ -750,10 +751,10 @@ static void vb2_dc_unmap_dmabuf(void *mem_priv)
>   }
>  
>   if (buf->vaddr) {
> - dma_buf_vunmap(buf->db_attach->dmabuf, &map);
> + dma_buf_vunmap_unlocked(buf->db_attach->dmabuf, &map);
>   buf->vaddr = NULL;
>   }
> - dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir);
> + dma_buf_unmap_attachment_unlocked(buf->db_attach, sgt, buf->dma_dir);
>  
>   buf->dma_addr = 0;
>   buf->dma_sgt = NULL;
> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c 
> b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> index fa69158a65b1..36ecdea8d707 100644
> --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> @@ -309,7 +309,7 @@ static void *vb2_dma_sg_vaddr(struct vb2_buffer *vb, void 
> *buf_priv)
>  
>   if (!buf->vaddr) {
>   if (buf->db_attach) {
> - ret = dma_buf_vmap(buf->db_attach->dmabuf, &map);
> + ret = dma_buf_vmap_unlocked(buf->db_attach->dmabuf, 
> &map);
>   buf->vaddr = ret ? NULL : map.vaddr;
>   } else {
>   buf->vaddr = vm_map_ram(buf->pages, buf->num_pages, -1);
> @@ -565,7 +565,7 @@ static int vb2_dma_sg_map_dmabuf(void *mem_priv)
>   }
>  
>   /* get the associated scatterlist for this buffer */
> - sgt = dma_buf_map_attachment(buf->db_attach, buf->dma_dir);
> + sgt = dma_buf_map_attachment_unlocked(buf->db_attach, buf->dma_dir);
>   if (IS_ERR(sgt)) {
>   pr_err("Error getting dmabuf scatterlist\n");
>   return -EINVAL;
> @@ -594,10 +594,10 @@ static void vb2_dma_sg_unmap_dmabuf(void *mem_priv)
>   }
>  
>   if (buf->vaddr) {
> - dma_buf_vunmap(buf->db_attach->dmabuf, &map);
> + dma_buf_vunmap_unlocked(buf->db_attach->dmabuf, &map);
>   buf->vaddr = NULL;
>   }
> - dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir);
> + dma_buf_unmap_attachment_unlocked(buf->db_attach, sgt, buf->dma_dir);
>  
>   buf->dma_sgt = NULL;
>  }
> diff --git a/drivers/media/common/videobuf2/videobuf2-vmalloc.c 
> b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
> index 948152f1596b..7831bf545874 100644
> --- a/drivers/media/common/videobuf2/videobuf2-vmalloc.c
> +++ b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
> @@ -376,7 +376,7 @@ static int vb2_vmalloc_map_dmabuf(void *mem_priv)
>   struct iosys_map map;
>   int ret;
>  
> - ret = dma_buf_vmap(buf->dbuf, &map);
> + ret = dma_buf_vmap_unlocked(buf->dbuf, &map);
>   if (ret)
>   return -EFAULT;
>   buf->vaddr = map.vaddr;
> @@ -389,7 +389,7 @@ static void vb2_vmalloc_unmap_dmabuf(void *mem_priv)

Re: [PATCH v7 14/21] media: tegra-vde: Prepare to dynamic dma-buf locking specification

[Hans Verkuil]

On 10/17/22 19:22, Dmitry Osipenko wrote:
> Prepare Tegra video decoder driver to the common dynamic dma-buf
> locking convention by starting to use the unlocked versions of dma-buf
> API functions.
> 
> Acked-by: Christian König 

Acked-by: Hans Verkuil 

Thanks!

Hans

> Signed-off-by: Dmitry Osipenko 
> ---
>  drivers/media/platform/nvidia/tegra-vde/dmabuf-cache.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/nvidia/tegra-vde/dmabuf-cache.c 
> b/drivers/media/platform/nvidia/tegra-vde/dmabuf-cache.c
> index 69c346148070..1c5b94989aec 100644
> --- a/drivers/media/platform/nvidia/tegra-vde/dmabuf-cache.c
> +++ b/drivers/media/platform/nvidia/tegra-vde/dmabuf-cache.c
> @@ -38,7 +38,7 @@ static void tegra_vde_release_entry(struct 
> tegra_vde_cache_entry *entry)
>   if (entry->vde->domain)
>   tegra_vde_iommu_unmap(entry->vde, entry->iova);
>  
> - dma_buf_unmap_attachment(entry->a, entry->sgt, entry->dma_dir);
> + dma_buf_unmap_attachment_unlocked(entry->a, entry->sgt, entry->dma_dir);
>   dma_buf_detach(dmabuf, entry->a);
>   dma_buf_put(dmabuf);
>  
> @@ -102,7 +102,7 @@ int tegra_vde_dmabuf_cache_map(struct tegra_vde *vde,
>   goto err_unlock;
>   }
>  
> - sgt = dma_buf_map_attachment(attachment, dma_dir);
> + sgt = dma_buf_map_attachment_unlocked(attachment, dma_dir);
>   if (IS_ERR(sgt)) {
>   dev_err(dev, "Failed to get dmabufs sg_table\n");
>   err = PTR_ERR(sgt);
> @@ -152,7 +152,7 @@ int tegra_vde_dmabuf_cache_map(struct tegra_vde *vde,
>  err_free:
>   kfree(entry);
>  err_unmap:
> - dma_buf_unmap_attachment(attachment, sgt, dma_dir);
> + dma_buf_unmap_attachment_unlocked(attachment, sgt, dma_dir);
>  err_detach:
>   dma_buf_detach(dmabuf, attachment);
>  err_unlock:

Re: [PATCH] drm/amdgpu: don't call drm_fb_helper_lastclose in lastclose()

[Thomas Zimmermann]

Hi

Am 24.10.22 um 08:20 schrieb Quan, Evan:

[AMD Official Use Only - General]

Reviewed-by: Evan Quan 


-Original Message-
From: amd-gfx  On Behalf Of Alex
Deucher
Sent: Thursday, October 20, 2022 10:36 PM
To: amd-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
Cc: Deucher, Alexander ; Thomas
Zimmermann 
Subject: [PATCH] drm/amdgpu: don't call drm_fb_helper_lastclose in
lastclose()

It's used to restore the fbdev console, but as amdgpu uses
generic fbdev emulation, the console is being restored by the
DRM client helpers already. See the call to drm_client_dev_restore()
in drm_lastclose().

Fixes: 087451f372bf76 ("drm/amdgpu: use generic fb helpers instead of
setting up AMD own's.")
Cc: Thomas Zimmermann 
Signed-off-by: Alex Deucher 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index fe23e09eec98..474b9f40f792 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -1106,7 +1106,6 @@ int amdgpu_info_ioctl(struct drm_device *dev,
void *data, struct drm_file *filp)
   */
  void amdgpu_driver_lastclose_kms(struct drm_device *dev)
  {
-   drm_fb_helper_lastclose(dev);
vga_switcheroo_process_delayed_switch();
  }


Without the call to drm_fb_helper_lastclose(), the console emulation 
will be restored by drm_client_dev_restore() from drm_lastclose(). [1] 
It means that it's now changing order with the call to 
vga_switcheroo_process_delay_switch(). Can this become a problem?


I looked at the other callers of that function. Most restore the console 
before doing the switcheroo. Nouveau doesn't seem to care about the 
console at all.


Best regards
Thomas

[1] 
https://elixir.bootlin.com/linux/v6.0.3/source/drivers/gpu/drm/drm_file.c#L467




--
2.37.3


--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH 2/2] drm/amdgpu: use DRM_SCHED_FENCE_DONT_PIPELINE for VM updates

[Luben Tuikov]

On 2022-10-17 02:27, Christian König wrote:
> Am 17.10.22 um 07:29 schrieb Luben Tuikov:
>> Hi Christian,
>>
>> On 2022-10-14 04:15, Christian König wrote:
>>> Make sure that we always have a CPU round trip to let the submission
>>> code correctly decide if a TLB flush is necessary or not.
>>>
>>> Signed-off-by: Christian König 
>>> CC: sta...@vger.kernel.org # 5.19+
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 9 -
>>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
>>> index 2b0669c464f6..69e105fa41f6 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
>>> @@ -116,8 +116,15 @@ static int amdgpu_vm_sdma_commit(struct 
>>> amdgpu_vm_update_params *p,
>>>DMA_RESV_USAGE_BOOKKEEP);
>>> }
>>>   
>>> -   if (fence && !p->immediate)
>>> +   if (fence && !p->immediate) {
>>> +   /*
>>> +* Most hw generations now have a separate queue for page table
>>> +* updates, but when the queue is shared with userspace we need
>>> +* the extra CPU round trip to correctly flush the TLB.
>>> +*/
>>> +   set_bit(DRM_SCHED_FENCE_DONT_PIPELINE, &f->flags);
>>> swap(*fence, f);
>>> +   }
>> Do you ever turn that bit off?
> 
> No, I just rely on the fact that the flags are zero initialized.

Acked-by: Luben Tuikov 

Regards,
Luben

Re: [PATCH 1/2] drm/sched: add DRM_SCHED_FENCE_DONT_PIPELINE flag

[Luben Tuikov]

On 2022-10-14 04:15, Christian König wrote:
> Setting this flag on a scheduler fence prevents pipelining of jobs
> depending on this fence. In other words we always insert a full CPU
> round trip before dependen jobs are pushed to the pipeline.

"dependent"

> 
> Signed-off-by: Christian König 
> CC: sta...@vger.kernel.org # 5.19+
> ---
>  drivers/gpu/drm/scheduler/sched_entity.c | 3 ++-
>  include/drm/gpu_scheduler.h  | 9 +
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c 
> b/drivers/gpu/drm/scheduler/sched_entity.c
> index 191c56064f19..43d337d8b153 100644
> --- a/drivers/gpu/drm/scheduler/sched_entity.c
> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
> @@ -385,7 +385,8 @@ static bool drm_sched_entity_add_dependency_cb(struct 
> drm_sched_entity *entity)
>   }
>  
>   s_fence = to_drm_sched_fence(fence);
> - if (s_fence && s_fence->sched == sched) {
> + if (s_fence && s_fence->sched == sched &&
> + !test_bit(DRM_SCHED_FENCE_DONT_PIPELINE, &fence->flags)) {
>  
>   /*
>* Fence is from the same scheduler, only need to wait for
> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> index 0fca8f38bee4..f01d14b231ed 100644
> --- a/include/drm/gpu_scheduler.h
> +++ b/include/drm/gpu_scheduler.h
> @@ -32,6 +32,15 @@
>  
>  #define MAX_WAIT_SCHED_ENTITY_Q_EMPTY msecs_to_jiffies(1000)
>  
> +/**
> + * DRM_SCHED_FENCE_DONT_PIPELINE - Prefent dependency pipelining

"Prevent"

> + *
> + * Setting this flag on a scheduler fence prevents pipelining of jobs 
> depending
> + * on this fence. In other words we always insert a full CPU round trip 
> before
> + * dependen jobs are pushed to the hw queue.

"dependent"

> + */
> +#define DRM_SCHED_FENCE_DONT_PIPELINEDMA_FENCE_FLAG_USER_BITS
> +
>  struct drm_gem_object;
>  
>  struct drm_gpu_scheduler;

With those corrections,

Acked-by: Luben Tuikov 

Regards,
Luben

Re: Fixing rts5208 driver code style

[Uros Milojkovic]

Hi I am new to kernel dev and am very passionate, even if this fix isn't
formatted correctly, could you please give me specific advice on how to do
so going forward?
Best,
Uros

On Sun, Oct 23, 2022 at 3:32 PM uroshm  wrote:

>
> diff --git a/drivers/staging/rts5208/general.c
> b/drivers/staging/rts5208/general.c
> index 0f912b011064..4694593af4d9 100644
> --- a/drivers/staging/rts5208/general.c
> +++ b/drivers/staging/rts5208/general.c
> @@ -9,7 +9,7 @@
>*   Micky Ching (micky_ch...@realsil.com.cn)
>*/
>
> -#include "general.h"
> + #include "general.h"
>
>   int bit1cnt_long(u32 data)
>   {
>
>
> Signed-off-by: Uros Milojkovic 
>
>

Re: [6.1][regression] after commit dd80d9c8eecac8c516da5b240d01a35660ba6cb6 some games (Cyberpunk 2077, Forza Horizon 4/5) hang at start #forregzbot

[Thorsten Leemhuis]

[Note: this mail is primarily send for documentation purposes and/or for
regzbot, my Linux kernel regression tracking bot. That's why I removed
most or all folks from the list of recipients, but left any that looked
like a mailing lists. These mails usually contain '#forregzbot' in the
subject, to make them easy to spot and filter out.]

[TLDR: I'm adding this regression report to the list of tracked
regressions; all text from me you find below is based on a few templates
paragraphs you might have encountered already already in similar form.]

Hi, this is your Linux kernel regression tracker. CCing the regression
mailing list, as it should be in the loop for all regressions, as
explained here:
https://www.kernel.org/doc/html/latest/admin-guide/reporting-issues.html


On 21.10.22 10:08, Mikhail Gavrilov wrote:
> Hi!
> I found that some games (Cyberpunk 2077, Forza Horizon 4/5) hang at
> start after commit dd80d9c8eecac8c516da5b240d01a35660ba6cb6.

Thanks for the report. To be sure below issue doesn't fall through the
cracks unnoticed, I'm adding it to regzbot, my Linux kernel regression
tracking bot:

#regzbot ^introduced dd80d9c8eecac
#regzbot title drm: amdgpu: some games (Cyberpunk 2077, Forza Horizon
4/5) hang at start
#regzbot ignore-activity

This isn't a regression? This issue or a fix for it are already
discussed somewhere else? It was fixed already? You want to clarify when
the regression started to happen? Or point out I got the title or
something else totally wrong? Then just reply -- ideally with also
telling regzbot about it, as explained here:
https://linux-regtracking.leemhuis.info/tracked-regression/

Reminder for developers: When fixing the issue, add 'Link:' tags
pointing to the report (the mail this one replies to), as explained for
in the Linux kernel's documentation; above webpage explains why this is
important for tracked regressions.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I deal with a lot of
reports and sometimes miss something important when writing mails like
this. If that's the case here, don't hesitate to tell me in a public
reply, it's in everyone's interest to set the public record straight.

> dd80d9c8eecac8c516da5b240d01a35660ba6cb6 is the first bad commit
> commit dd80d9c8eecac8c516da5b240d01a35660ba6cb6
> Author: Christian König 
> Date:   Thu Jul 14 10:23:38 2022 +0200
> 
> drm/amdgpu: revert "partial revert "remove ctx->lock" v2"
> 
> This reverts commit 94f4c4965e5513ba624488f4b601d6b385635aec.
> 
> We found that the bo_list is missing a protection for its list entries.
> Since that is fixed now this workaround can be removed again.
> 
> Signed-off-by: Christian König 
> Reviewed-by: Alex Deucher 
> Signed-off-by: Alex Deucher 
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  | 21 ++---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c |  2 --
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h |  1 -
>  3 files changed, 6 insertions(+), 18 deletions(-)
> 
> 
> And when it happening in kernel log appears a such backtrace:
> [  231.331210] [ cut here ]
> [  231.331262] WARNING: CPU: 11 PID: 6555 at
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c:675
> amdgpu_ttm_tt_get_user_pages+0x14c/0x190 [amdgpu]
> [  231.331424] Modules linked in: uinput rfcomm snd_seq_dummy
> snd_hrtimer nft_objref nf_conntrack_netbios_ns nf_conntrack_broadcast
> nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet
> nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat nf_nat
> nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set nf_tables nfnetlink
> qrtr bnep intel_rapl_msr intel_rapl_common snd_sof_amd_renoir
> snd_sof_amd_acp snd_sof_pci snd_hda_codec_realtek snd_sof
> snd_hda_codec_generic snd_hda_codec_hdmi snd_sof_utils mt7921e
> snd_hda_intel sunrpc snd_intel_dspcfg mt7921_common binfmt_misc
> snd_intel_sdw_acpi snd_hda_codec mt76_connac_lib edac_mce_amd btusb
> snd_soc_core mt76 snd_hda_core btrtl snd_hwdep snd_compress kvm_amd
> ac97_bus snd_seq btbcm snd_pcm_dmaengine btintel snd_rpl_pci_acp6x
> mac80211 btmtk snd_pci_acp6x kvm snd_seq_device snd_pcm snd_pci_acp5x
> libarc4 irqbypass bluetooth snd_rn_pci_acp3x snd_timer pcspkr
> asus_nb_wmi rapl joydev wmi_bmof snd_acp_config cfg80211 snd_soc_acpi
> vfat snd
> [  231.331490]  snd_pci_acp3x i2c_piix4 soundcore fat k10temp amd_pmc
> asus_wireless zram amdgpu drm_ttm_helper ttm hid_asus asus_wmi
> iommu_v2 crct10dif_pclmul crc32_pclmul gpu_sched crc32c_intel
> ledtrig_audio sparse_keymap polyval_clmulni platform_profile drm_buddy
> polyval_generic hid_multitouch drm_display_helper rfkill nvme
> ucsi_acpi ghash_clmulni_intel nvme_core video typec_ucsi serio_raw ccp
> sha512_ssse3 sp5100_tco r8169 cec nvme_common typec wmi i2c_hid_acpi
> i2c_hid ip6_tables ip_tables fuse
> [  231.331532] CPU: 11 PID: 6555 Comm: GameThread Tainted: GW
>   L---  ---
> 6.1.0-0.rc1.20221019gitaae703b02f92.17.

[PATCH v2] drm/amd/display: Revert logic for plane modifiers

[Joaquín Ignacio Aramendía]

This file was split in commit 5d945cbcd4b16a29d6470a80dfb19738f9a4319f
("drm/amd/display: Create a file dedicated to planes") and the logic in
dm_plane_format_mod_supported() function got changed by a switch logic.
That change broke drm_plane modifiers setting on series 5000 APUs
(tested on OXP mini AMD 5800U and HP Dev One 5850U PRO)
leading to Gamescope not working as reported on GitHub[1]

To reproduce the issue, enter a TTY and run:

$ gamescope -- vkcube

With said commit applied it will abort. This one restores the old logic,
fixing the issue that affects Gamescope.

[1](https://github.com/Plagman/gamescope/issues/624)

Signed-off-by: Joaquín Ignacio Aramendía 
Reviewed-by: Bas Nieuwenhuizen 
---
Removed asic_id and excess newlines
---
 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 50 +++
 1 file changed, 7 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index dfd3be49eac8..e6854f7270a6 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -1369,7 +1369,7 @@ static bool dm_plane_format_mod_supported(struct 
drm_plane *plane,
 {
struct amdgpu_device *adev = drm_to_adev(plane->dev);
const struct drm_format_info *info = drm_format_info(format);
-   struct hw_asic_id asic_id = adev->dm.dc->ctx->asic_id;
+   int i;

enum dm_micro_swizzle microtile = modifier_gfx9_swizzle_mode(modifier) 
& 3;

@@ -1386,49 +1386,13 @@ static bool dm_plane_format_mod_supported(struct 
drm_plane *plane,
return true;
}

-   /* check if swizzle mode is supported by this version of DCN */
-   switch (asic_id.chip_family) {
-   case FAMILY_SI:
-   case FAMILY_CI:
-   case FAMILY_KV:
-   case FAMILY_CZ:
-   case FAMILY_VI:
-   /* asics before AI does not have modifier support */
-   return false;
-   case FAMILY_AI:
-   case FAMILY_RV:
-   case FAMILY_NV:
-   case FAMILY_VGH:
-   case FAMILY_YELLOW_CARP:
-   case AMDGPU_FAMILY_GC_10_3_6:
-   case AMDGPU_FAMILY_GC_10_3_7:
-   switch (AMD_FMT_MOD_GET(TILE, modifier)) {
-   case AMD_FMT_MOD_TILE_GFX9_64K_R_X:
-   case AMD_FMT_MOD_TILE_GFX9_64K_D_X:
-   case AMD_FMT_MOD_TILE_GFX9_64K_S_X:
-   case AMD_FMT_MOD_TILE_GFX9_64K_D:
-   return true;
-   default:
-   return false;
-   }
-   break;
-   case AMDGPU_FAMILY_GC_11_0_0:
-   case AMDGPU_FAMILY_GC_11_0_1:
-   switch (AMD_FMT_MOD_GET(TILE, modifier)) {
-   case AMD_FMT_MOD_TILE_GFX11_256K_R_X:
-   case AMD_FMT_MOD_TILE_GFX9_64K_R_X:
-   case AMD_FMT_MOD_TILE_GFX9_64K_D_X:
-   case AMD_FMT_MOD_TILE_GFX9_64K_S_X:
-   case AMD_FMT_MOD_TILE_GFX9_64K_D:
-   return true;
-   default:
-   return false;
-   }
-   break;
-   default:
-   ASSERT(0); /* Unknown asic */
-   break;
+   /* Check that the modifier is on the list of the plane's supported 
modifiers. */
+   for (i = 0; i < plane->modifier_count; i++) {
+   if (modifier == plane->modifiers[i])
+   break;
}
+   if (i == plane->modifier_count)
+   return false;

/*
 * For D swizzle the canonical modifier depends on the bpp, so check
--
2.38.1

Fwd: amdgpu: update from 5.10.145 to 5.10.146...149 breaks boot on Ryzen based computers

[Linus Torvalds]

This was sent to me, but should have gone to other people.

It may already be fixed, but note how the report is about -stable
kernels, including apparently the current 5.10 stable version (149(.

  Linus

-- Forwarded message -
From: Kevin Torkelson 
Date: Thu, Oct 20, 2022 at 8:09 AM
Subject: amdgpu: update from 5.10.145 to 5.10.146...149 breaks boot on
Ryzen based computers
To: 


Linus,

--- Possibly Important ---
I know several people who are crashing with Debian Bullseye (stable)
with the most current kernel put out by the distribution. AMD put out
a fix that seems like it might be related here:
https://gitlab.freedesktop.org/drm/amd/-/issues/2216

Re: Fixes for scheduler hang when killing a process

[Luben Tuikov]

On 2022-10-14 04:46, Christian König wrote:
> Hi guys,
> 
> rebased those patches on top of amd-staging-drm-next since the
> amdgpu changes are quite substencial.
> 
> Please review and comment,
> Christian.

Hi Christian,

The changes are good and I think asynchronous is the way to go. I'm also 
running with those changes live.

The series is:

Reviewed-by: Luben Tuikov 

Regards,
Luben