Re: [PATCH 2/2] drm_print: Remove deprecated DRM_DEBUG_KMS_RATELIMITED()

2023-01-17 Thread Christian König




Am 17.01.23 um 19:12 schrieb Das, Nirmoy:

Hi Alex,

On 1/17/2023 7:06 PM, Alex Deucher wrote:

On Tue, Jan 17, 2023 at 1:05 PM Nirmoy Das  wrote:

There are no current users of DRM_DEBUG_KMS_RATELIMITED()
so remove it.

Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Thomas Zimmermann 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Sam Ravnborg 

Signed-off-by: Nirmoy Das 
Reviewed-by: Sam Ravnborg 

Series is:
Reviewed-by: Alex Deucher 

Feel free to take the patches through whatever tree you want.



Please help me with this, I don't have committer rights for any tree.


Going to push that into drm-misc-next later today.

Thanks,
Christian.




Nirmoy




Alex


---
  include/drm/drm_print.h | 3 ---
  1 file changed, 3 deletions(-)

diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index a44fb7ef257f..c3753da97c4e 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -605,9 +605,6 @@ void __drm_err(const char *format, ...);
  #define drm_dbg_kms_ratelimited(drm, fmt, ...) \
 __DRM_DEFINE_DBG_RATELIMITED(KMS, drm, fmt, ## __VA_ARGS__)

-/* NOTE: this is deprecated in favor of 
drm_dbg_kms_ratelimited(NULL, ...). */
-#define DRM_DEBUG_KMS_RATELIMITED(fmt, ...) 
drm_dbg_kms_ratelimited(NULL, fmt, ## __VA_ARGS__)

-
  /*
   * struct drm_device based WARNs
   *
--
2.39.0





RE: [PATCH 1/4] drm/amdgpu/nv: don't expose AV1 if VCN0 is harvested

2023-01-17 Thread Liu, Leo
[AMD Official Use Only - General]

The series are:

Reviewed-by: Leo Liu 


-Original Message-
From: amd-gfx  On Behalf Of Alex Deucher
Sent: January 17, 2023 3:00 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander 
Subject: [PATCH 1/4] drm/amdgpu/nv: don't expose AV1 if VCN0 is harvested

Only VCN0 supports AV1.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/nv.c | 101 +---
 1 file changed, 81 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c 
index 6853b93ac82e..d972025f0d20 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -98,7 +98,7 @@ static const struct amdgpu_video_codecs 
nv_video_codecs_decode =  };

 /* Sienna Cichlid */
-static const struct amdgpu_video_codec_info sc_video_codecs_decode_array[] =
+static const struct amdgpu_video_codec_info
+sc_video_codecs_decode_array_vcn0[] =
 {
{codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG2, 4096, 4096, 
3)},
{codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4, 4096, 4096, 
5)}, @@ -110,10 +110,27 @@ static const struct amdgpu_video_codec_info 
sc_video_codecs_decode_array[] =
{codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_AV1, 8192, 4352, 
0)},  };

-static const struct amdgpu_video_codecs sc_video_codecs_decode =
+static const struct amdgpu_video_codec_info
+sc_video_codecs_decode_array_vcn1[] =
 {
-   .codec_count = ARRAY_SIZE(sc_video_codecs_decode_array),
-   .codec_array = sc_video_codecs_decode_array,
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG2, 4096, 4096, 
3)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4, 4096, 4096, 
5)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4_AVC, 4096, 
4096, 52)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_VC1, 4096, 4096, 4)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_HEVC, 8192, 4352, 
186)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_JPEG, 4096, 4096, 
0)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_VP9, 8192, 4352,
+0)}, };
+
+static const struct amdgpu_video_codecs sc_video_codecs_decode_vcn0 = {
+   .codec_count = ARRAY_SIZE(sc_video_codecs_decode_array_vcn0),
+   .codec_array = sc_video_codecs_decode_array_vcn0,
+};
+
+static const struct amdgpu_video_codecs sc_video_codecs_decode_vcn1 = {
+   .codec_count = ARRAY_SIZE(sc_video_codecs_decode_array_vcn1),
+   .codec_array = sc_video_codecs_decode_array_vcn1,
 };

 /* SRIOV Sienna Cichlid, not const since data is controlled by host */ @@ 
-123,7 +140,7 @@ static struct amdgpu_video_codec_info 
sriov_sc_video_codecs_encode_array[] =
{codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_HEVC, 4096, 2304, 
0)},  };

-static struct amdgpu_video_codec_info sriov_sc_video_codecs_decode_array[] =
+static struct amdgpu_video_codec_info
+sriov_sc_video_codecs_decode_array_vcn0[] =
 {
{codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG2, 4096, 4096, 
3)},
{codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4, 4096, 4096, 
5)}, @@ -135,16 +152,33 @@ static struct amdgpu_video_codec_info 
sriov_sc_video_codecs_decode_array[] =
{codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_AV1, 8192, 4352, 
0)},  };

+static struct amdgpu_video_codec_info
+sriov_sc_video_codecs_decode_array_vcn1[] = {
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG2, 4096, 4096, 
3)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4, 4096, 4096, 
5)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4_AVC, 4096, 
4096, 52)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_VC1, 4096, 4096, 4)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_HEVC, 8192, 4352, 
186)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_JPEG, 4096, 4096, 
0)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_VP9, 8192, 4352,
+0)}, };
+
 static struct amdgpu_video_codecs sriov_sc_video_codecs_encode =  {
.codec_count = ARRAY_SIZE(sriov_sc_video_codecs_encode_array),
.codec_array = sriov_sc_video_codecs_encode_array,
 };

-static struct amdgpu_video_codecs sriov_sc_video_codecs_decode =
+static struct amdgpu_video_codecs sriov_sc_video_codecs_decode_vcn0 =
 {
-   .codec_count = ARRAY_SIZE(sriov_sc_video_codecs_decode_array),
-   .codec_array = sriov_sc_video_codecs_decode_array,
+   .codec_count = ARRAY_SIZE(sriov_sc_video_codecs_decode_array_vcn0),
+   .codec_array = sriov_sc_video_codecs_decode_array_vcn0,
+};
+
+static struct amdgpu_video_codecs sriov_sc_video_codecs_decode_vcn1 = {
+   .codec_count = ARRAY_SIZE(sriov_sc_video_codecs_decode_array_vcn1),
+   .codec_array = sriov_sc_video_codecs_decode_array_vcn1,
 };

 /* Beige Goby*/
@@ -181,20 +215,37 @@ static const struct amdgpu_video_codecs 
yc_video_codecs_decode = {  

[PATCH 2/2] drm/amd/display: Remove unused display_content_support

2023-01-17 Thread Joshua Ashton
This was never filled in and thus never truly used.

Checking the EDID for content_type support is not required for sending
the avi infoframe packet.

Signed-off-by: Joshua Ashton 
---
 drivers/gpu/drm/amd/display/dc/dc_stream.h |  1 -
 drivers/gpu/drm/amd/display/dc/dc_types.h  | 14 --
 2 files changed, 15 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dc_stream.h 
b/drivers/gpu/drm/amd/display/dc/dc_stream.h
index 51dc30706e43..a499c0952ea0 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_stream.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_stream.h
@@ -182,7 +182,6 @@ struct dc_stream_state {
 */
struct link_encoder *link_enc;
struct dc_panel_patch sink_patches;
-   union display_content_support content_support;
struct dc_crtc_timing timing;
struct dc_crtc_timing_adjust adjust;
struct dc_info_packet vrr_infopacket;
diff --git a/drivers/gpu/drm/amd/display/dc/dc_types.h 
b/drivers/gpu/drm/amd/display/dc/dc_types.h
index c73a655bd687..862af36027e7 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_types.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_types.h
@@ -175,18 +175,6 @@ struct dc_edid {
 
 #define AUDIO_INFO_DISPLAY_NAME_SIZE_IN_CHARS 20
 
-union display_content_support {
-   unsigned int raw;
-   struct {
-   unsigned int valid_content_type :1;
-   unsigned int game_content :1;
-   unsigned int cinema_content :1;
-   unsigned int photo_content :1;
-   unsigned int graphics_content :1;
-   unsigned int reserved :27;
-   } bits;
-};
-
 struct dc_panel_patch {
unsigned int dppowerup_delay;
unsigned int extra_t12_ms;
@@ -219,8 +207,6 @@ struct dc_edid_caps {
uint32_t audio_latency;
uint32_t video_latency;
 
-   union display_content_support content_support;
-
uint8_t qs_bit;
uint8_t qy_bit;
 
-- 
2.39.0



[PATCH 1/2] drm/amd/display: Hook up 'content type' property for HDMI

2023-01-17 Thread Joshua Ashton
Implements the 'content type' property for HDMI connectors.
Verified by checking the avi infoframe on a connected TV.

This also simplifies a lot of the code in that area as well, there were
a lot of temp variables doing very little and unnecessary logic
that was quite confusing.

It is not necessary to check for support in the EDID before sending a
'content type' value in the avi infoframe also.

Signed-off-by: Joshua Ashton 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 24 +++
 .../gpu/drm/amd/display/dc/core/dc_resource.c | 69 ++-
 drivers/gpu/drm/amd/display/dc/dc_stream.h|  1 +
 3 files changed, 46 insertions(+), 48 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 9547037857b6..65fe3de9 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5216,6 +5216,24 @@ get_output_color_space(const struct dc_crtc_timing 
*dc_crtc_timing)
return color_space;
 }
 
+static enum display_content_type
+get_output_content_type(const struct drm_connector_state *connector_state)
+{
+   switch (connector_state->content_type) {
+   default:
+   case DRM_MODE_CONTENT_TYPE_NO_DATA:
+   return DISPLAY_CONTENT_TYPE_NO_DATA;
+   case DRM_MODE_CONTENT_TYPE_GRAPHICS:
+   return DISPLAY_CONTENT_TYPE_GRAPHICS;
+   case DRM_MODE_CONTENT_TYPE_PHOTO:
+   return DISPLAY_CONTENT_TYPE_PHOTO;
+   case DRM_MODE_CONTENT_TYPE_CINEMA:
+   return DISPLAY_CONTENT_TYPE_CINEMA;
+   case DRM_MODE_CONTENT_TYPE_GAME:
+   return DISPLAY_CONTENT_TYPE_GAME;
+   }
+}
+
 static bool adjust_colour_depth_from_display_info(
struct dc_crtc_timing *timing_out,
const struct drm_display_info *info)
@@ -5349,6 +5367,7 @@ static void fill_stream_properties_from_drm_display_mode(
}
 
stream->output_color_space = get_output_color_space(timing_out);
+   stream->content_type = get_output_content_type(connector_state);
 }
 
 static void fill_audio_info(struct audio_info *audio_info,
@@ -7123,6 +7142,11 @@ void amdgpu_dm_connector_init_helper(struct 
amdgpu_display_manager *dm,
adev->mode_info.abm_level_property, 0);
}
 
+   if (connector_type == DRM_MODE_CONNECTOR_HDMIA) {
+   /* Content Type is currently only implemented for HDMI. */
+   drm_connector_attach_content_type_property(>base);
+   }
+
if (connector_type == DRM_MODE_CONNECTOR_HDMIA ||
connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
connector_type == DRM_MODE_CONNECTOR_eDP) {
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index a5b5f8592c1b..39ceccdb6586 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -2944,14 +2944,9 @@ static void set_avi_info_frame(
uint32_t pixel_encoding = 0;
enum scanning_type scan_type = SCANNING_TYPE_NODATA;
enum dc_aspect_ratio aspect = ASPECT_RATIO_NO_DATA;
-   bool itc = false;
-   uint8_t itc_value = 0;
-   uint8_t cn0_cn1 = 0;
-   unsigned int cn0_cn1_value = 0;
uint8_t *check_sum = NULL;
uint8_t byte_index = 0;
union hdmi_info_packet hdmi_info;
-   union display_content_support support = {0};
unsigned int vic = pipe_ctx->stream->timing.vic;
unsigned int rid = pipe_ctx->stream->timing.rid;
unsigned int fr_ind = pipe_ctx->stream->timing.fr_index;
@@ -3055,49 +3050,27 @@ static void set_avi_info_frame(
/* Active Format Aspect ratio - same as Picture Aspect Ratio. */
hdmi_info.bits.R0_R3 = ACTIVE_FORMAT_ASPECT_RATIO_SAME_AS_PICTURE;
 
-   /* TODO: un-hardcode cn0_cn1 and itc */
-
-   cn0_cn1 = 0;
-   cn0_cn1_value = 0;
-
-   itc = true;
-   itc_value = 1;
-
-   support = stream->content_support;
-
-   if (itc) {
-   if (!support.bits.valid_content_type) {
-   cn0_cn1_value = 0;
-   } else {
-   if (cn0_cn1 == DISPLAY_CONTENT_TYPE_GRAPHICS) {
-   if (support.bits.graphics_content == 1) {
-   cn0_cn1_value = 0;
-   }
-   } else if (cn0_cn1 == DISPLAY_CONTENT_TYPE_PHOTO) {
-   if (support.bits.photo_content == 1) {
-   cn0_cn1_value = 1;
-   } else {
-   cn0_cn1_value = 0;
-   itc_value = 0;
-   }
-   } else if (cn0_cn1 == DISPLAY_CONTENT_TYPE_CINEMA) {
-   if 

Re: [PATCH] drm/amd/display: fix issues with driver unload

2023-01-17 Thread Alex Deucher
On Tue, Jan 17, 2023 at 3:55 PM Hamza Mahfooz  wrote:
>
> Currently, we run into a number of WARN()s when attempting to unload the
> amdgpu driver (e.g. using "modprobe -r amdgpu"). These all stem from
> calling drm_encoder_cleanup() too early. So, to fix this we can stop
> calling drm_encoder_cleanup() in amdgpu_dm_fini() and instead have it be
> called from amdgpu_dm_encoder_destroy(). Also, we don't need to free in
> amdgpu_dm_encoder_destroy() since mst_encoders[] isn't explicitly
> allocated by the slab allocater.
>
> Fixes: f74367e492ba ("drm/amdgpu/display: create fake mst encoders ahead of 
> time (v4)")
> Signed-off-by: Hamza Mahfooz 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   | 4 
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 1 -
>  2 files changed, 5 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 9547037857b6..5cc14ed2e93e 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1733,10 +1733,6 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev)
> adev->dm.vblank_control_workqueue = NULL;
> }
>
> -   for (i = 0; i < adev->dm.display_indexes_num; i++) {
> -   drm_encoder_cleanup(>dm.mst_encoders[i].base);
> -   }
> -
> amdgpu_dm_destroy_drm_device(>dm);
>
>  #if defined(CONFIG_DRM_AMD_SECURE_DISPLAY)
> 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 bb7c5d7c..5fa9bab95038 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
> @@ -492,7 +492,6 @@ static const struct drm_connector_helper_funcs 
> dm_dp_mst_connector_helper_funcs
>  static void amdgpu_dm_encoder_destroy(struct drm_encoder *encoder)
>  {
> drm_encoder_cleanup(encoder);
> -   kfree(encoder);
>  }
>
>  static const struct drm_encoder_funcs amdgpu_dm_encoder_funcs = {
> --
> 2.39.0
>


[PATCH] drm/amd/display: fix issues with driver unload

2023-01-17 Thread Hamza Mahfooz
Currently, we run into a number of WARN()s when attempting to unload the
amdgpu driver (e.g. using "modprobe -r amdgpu"). These all stem from
calling drm_encoder_cleanup() too early. So, to fix this we can stop
calling drm_encoder_cleanup() in amdgpu_dm_fini() and instead have it be
called from amdgpu_dm_encoder_destroy(). Also, we don't need to free in
amdgpu_dm_encoder_destroy() since mst_encoders[] isn't explicitly
allocated by the slab allocater.

Fixes: f74367e492ba ("drm/amdgpu/display: create fake mst encoders ahead of 
time (v4)")
Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   | 4 
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 1 -
 2 files changed, 5 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 9547037857b6..5cc14ed2e93e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1733,10 +1733,6 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev)
adev->dm.vblank_control_workqueue = NULL;
}
 
-   for (i = 0; i < adev->dm.display_indexes_num; i++) {
-   drm_encoder_cleanup(>dm.mst_encoders[i].base);
-   }
-
amdgpu_dm_destroy_drm_device(>dm);
 
 #if defined(CONFIG_DRM_AMD_SECURE_DISPLAY)
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 bb7c5d7c..5fa9bab95038 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
@@ -492,7 +492,6 @@ static const struct drm_connector_helper_funcs 
dm_dp_mst_connector_helper_funcs
 static void amdgpu_dm_encoder_destroy(struct drm_encoder *encoder)
 {
drm_encoder_cleanup(encoder);
-   kfree(encoder);
 }
 
 static const struct drm_encoder_funcs amdgpu_dm_encoder_funcs = {
-- 
2.39.0



Re: Calculating array sizes in C - was: Re: Build regressions/improvements in v6.2-rc1

2023-01-17 Thread John Paul Adrian Glaubitz

Hi!

On 1/17/23 21:05, Geert Uytterhoeven wrote:

Isn't this supposed to be caught by this check:

 a, __same_type(a, NULL)

?


Yeah, but gcc thinks it is smarter than us...
Probably it drops the test, assuming UB cannot happen.


Hmm, sounds like a GGC bug to me then. Not sure how to fix this then.

Adrian

--
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913



Re: Calculating array sizes in C - was: Re: Build regressions/improvements in v6.2-rc1

2023-01-17 Thread Geert Uytterhoeven
Hi Adrian,

On Tue, Jan 17, 2023 at 6:06 PM John Paul Adrian Glaubitz
 wrote:
> On 1/17/23 18:01, Geert Uytterhoeven wrote:
> > The issue is that some of the parameters are not arrays, but
> > NULL. E.g.:
> >
> > arch/sh/kernel/cpu/sh2/setup-sh7619.c:static
> > DECLARE_INTC_DESC(intc_desc, "sh7619", vectors, NULL,
> > arch/sh/kernel/cpu/sh2/setup-sh7619.c-   NULL,
> > prio_registers, NULL);
>
> Isn't this supposed to be caught by this check:
>
> a, __same_type(a, NULL)
>
> ?

Yeah, but gcc thinks it is smarter than us...
Probably it drops the test, assuming UB cannot happen.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


[PATCH 4/4] drm/amdgpu/vcn4: fail to schedule IB for AV1 if VCN0 is harvested

2023-01-17 Thread Alex Deucher
Only VCN0 supports AV1.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
index a79b6088374b..efb22d0975b3 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
@@ -1632,6 +1632,10 @@ static int vcn_v4_0_limit_sched(struct amdgpu_cs_parser 
*p,
if (atomic_read(>base.entity->fence_seq))
return -EINVAL;
 
+   /* if VCN0 is harvested, we can't support AV1 */
+   if (p->adev->vcn.harvest_config & AMDGPU_VCN_HARVEST_VCN0)
+   return -EINVAL;
+
scheds = p->adev->gpu_sched[AMDGPU_HW_IP_VCN_ENC]
[AMDGPU_RING_PRIO_0].sched;
drm_sched_entity_modify_sched(job->base.entity, scheds, 1);
-- 
2.39.0



[PATCH 2/4] drm/amdgpu/vcn3: fail to schedule IB for AV1 if VCN0 is harvested

2023-01-17 Thread Alex Deucher
Only VCN0 supports AV1.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
index bd228512424a..66439388faee 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
@@ -1771,6 +1771,10 @@ static int vcn_v3_0_limit_sched(struct amdgpu_cs_parser 
*p,
if (atomic_read(>base.entity->fence_seq))
return -EINVAL;
 
+   /* if VCN0 is harvested, we can't support AV1 */
+   if (p->adev->vcn.harvest_config & AMDGPU_VCN_HARVEST_VCN0)
+   return -EINVAL;
+
scheds = p->adev->gpu_sched[AMDGPU_HW_IP_VCN_DEC]
[AMDGPU_RING_PRIO_DEFAULT].sched;
drm_sched_entity_modify_sched(job->base.entity, scheds, 1);
-- 
2.39.0



[PATCH 3/4] drm/amdgpu/soc21: don't expose AV1 if VCN0 is harvested

2023-01-17 Thread Alex Deucher
Only VCN0 supports AV1.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/soc21.c | 61 +++---
 1 file changed, 48 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c 
b/drivers/gpu/drm/amd/amdgpu/soc21.c
index bea6b499568a..e03cf7f766c5 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc21.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
@@ -48,20 +48,32 @@
 static const struct amd_ip_funcs soc21_common_ip_funcs;
 
 /* SOC21 */
-static const struct amdgpu_video_codec_info 
vcn_4_0_0_video_codecs_encode_array[] =
+static const struct amdgpu_video_codec_info 
vcn_4_0_0_video_codecs_encode_array_vcn0[] =
 {
{codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4_AVC, 4096, 
2304, 0)},
{codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_HEVC, 4096, 2304, 
0)},
{codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_AV1, 8192, 4352, 0)},
 };
 
-static const struct amdgpu_video_codecs vcn_4_0_0_video_codecs_encode =
+static const struct amdgpu_video_codec_info 
vcn_4_0_0_video_codecs_encode_array_vcn1[] =
 {
-   .codec_count = ARRAY_SIZE(vcn_4_0_0_video_codecs_encode_array),
-   .codec_array = vcn_4_0_0_video_codecs_encode_array,
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4_AVC, 4096, 
2304, 0)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_HEVC, 4096, 2304, 
0)},
+};
+
+static const struct amdgpu_video_codecs vcn_4_0_0_video_codecs_encode_vcn0 =
+{
+   .codec_count = ARRAY_SIZE(vcn_4_0_0_video_codecs_encode_array_vcn0),
+   .codec_array = vcn_4_0_0_video_codecs_encode_array_vcn0,
 };
 
-static const struct amdgpu_video_codec_info 
vcn_4_0_0_video_codecs_decode_array[] =
+static const struct amdgpu_video_codecs vcn_4_0_0_video_codecs_encode_vcn1 =
+{
+   .codec_count = ARRAY_SIZE(vcn_4_0_0_video_codecs_encode_array_vcn1),
+   .codec_array = vcn_4_0_0_video_codecs_encode_array_vcn1,
+};
+
+static const struct amdgpu_video_codec_info 
vcn_4_0_0_video_codecs_decode_array_vcn0[] =
 {
{codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4_AVC, 4096, 
4096, 52)},
{codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_HEVC, 8192, 4352, 
186)},
@@ -70,23 +82,46 @@ static const struct amdgpu_video_codec_info 
vcn_4_0_0_video_codecs_decode_array[
{codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_AV1, 8192, 4352, 0)},
 };
 
-static const struct amdgpu_video_codecs vcn_4_0_0_video_codecs_decode =
+static const struct amdgpu_video_codec_info 
vcn_4_0_0_video_codecs_decode_array_vcn1[] =
+{
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4_AVC, 4096, 
4096, 52)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_HEVC, 8192, 4352, 
186)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_JPEG, 4096, 4096, 
0)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_VP9, 8192, 4352, 0)},
+};
+
+static const struct amdgpu_video_codecs vcn_4_0_0_video_codecs_decode_vcn0 =
 {
-   .codec_count = ARRAY_SIZE(vcn_4_0_0_video_codecs_decode_array),
-   .codec_array = vcn_4_0_0_video_codecs_decode_array,
+   .codec_count = ARRAY_SIZE(vcn_4_0_0_video_codecs_decode_array_vcn0),
+   .codec_array = vcn_4_0_0_video_codecs_decode_array_vcn0,
+};
+
+static const struct amdgpu_video_codecs vcn_4_0_0_video_codecs_decode_vcn1 =
+{
+   .codec_count = ARRAY_SIZE(vcn_4_0_0_video_codecs_decode_array_vcn1),
+   .codec_array = vcn_4_0_0_video_codecs_decode_array_vcn1,
 };
 
 static int soc21_query_video_codecs(struct amdgpu_device *adev, bool encode,
 const struct amdgpu_video_codecs **codecs)
 {
-   switch (adev->ip_versions[UVD_HWIP][0]) {
+   if (adev->vcn.num_vcn_inst == hweight8(adev->vcn.harvest_config))
+   return -EINVAL;
 
+   switch (adev->ip_versions[UVD_HWIP][0]) {
case IP_VERSION(4, 0, 0):
case IP_VERSION(4, 0, 2):
-   if (encode)
-   *codecs = _4_0_0_video_codecs_encode;
-   else
-   *codecs = _4_0_0_video_codecs_decode;
+   if (adev->vcn.harvest_config & AMDGPU_VCN_HARVEST_VCN0) {
+   if (encode)
+   *codecs = _4_0_0_video_codecs_encode_vcn1;
+   else
+   *codecs = _4_0_0_video_codecs_decode_vcn1;
+   } else {
+   if (encode)
+   *codecs = _4_0_0_video_codecs_encode_vcn0;
+   else
+   *codecs = _4_0_0_video_codecs_decode_vcn0;
+   }
return 0;
default:
return -EINVAL;
-- 
2.39.0



[PATCH 1/4] drm/amdgpu/nv: don't expose AV1 if VCN0 is harvested

2023-01-17 Thread Alex Deucher
Only VCN0 supports AV1.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/nv.c | 101 +---
 1 file changed, 81 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
index 6853b93ac82e..d972025f0d20 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -98,7 +98,7 @@ static const struct amdgpu_video_codecs 
nv_video_codecs_decode =
 };
 
 /* Sienna Cichlid */
-static const struct amdgpu_video_codec_info sc_video_codecs_decode_array[] =
+static const struct amdgpu_video_codec_info 
sc_video_codecs_decode_array_vcn0[] =
 {
{codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG2, 4096, 4096, 
3)},
{codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4, 4096, 4096, 
5)},
@@ -110,10 +110,27 @@ static const struct amdgpu_video_codec_info 
sc_video_codecs_decode_array[] =
{codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_AV1, 8192, 4352, 0)},
 };
 
-static const struct amdgpu_video_codecs sc_video_codecs_decode =
+static const struct amdgpu_video_codec_info 
sc_video_codecs_decode_array_vcn1[] =
 {
-   .codec_count = ARRAY_SIZE(sc_video_codecs_decode_array),
-   .codec_array = sc_video_codecs_decode_array,
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG2, 4096, 4096, 
3)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4, 4096, 4096, 
5)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4_AVC, 4096, 
4096, 52)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_VC1, 4096, 4096, 4)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_HEVC, 8192, 4352, 
186)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_JPEG, 4096, 4096, 
0)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_VP9, 8192, 4352, 0)},
+};
+
+static const struct amdgpu_video_codecs sc_video_codecs_decode_vcn0 =
+{
+   .codec_count = ARRAY_SIZE(sc_video_codecs_decode_array_vcn0),
+   .codec_array = sc_video_codecs_decode_array_vcn0,
+};
+
+static const struct amdgpu_video_codecs sc_video_codecs_decode_vcn1 =
+{
+   .codec_count = ARRAY_SIZE(sc_video_codecs_decode_array_vcn1),
+   .codec_array = sc_video_codecs_decode_array_vcn1,
 };
 
 /* SRIOV Sienna Cichlid, not const since data is controlled by host */
@@ -123,7 +140,7 @@ static struct amdgpu_video_codec_info 
sriov_sc_video_codecs_encode_array[] =
{codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_HEVC, 4096, 2304, 
0)},
 };
 
-static struct amdgpu_video_codec_info sriov_sc_video_codecs_decode_array[] =
+static struct amdgpu_video_codec_info 
sriov_sc_video_codecs_decode_array_vcn0[] =
 {
{codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG2, 4096, 4096, 
3)},
{codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4, 4096, 4096, 
5)},
@@ -135,16 +152,33 @@ static struct amdgpu_video_codec_info 
sriov_sc_video_codecs_decode_array[] =
{codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_AV1, 8192, 4352, 0)},
 };
 
+static struct amdgpu_video_codec_info 
sriov_sc_video_codecs_decode_array_vcn1[] =
+{
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG2, 4096, 4096, 
3)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4, 4096, 4096, 
5)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4_AVC, 4096, 
4096, 52)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_VC1, 4096, 4096, 4)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_HEVC, 8192, 4352, 
186)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_JPEG, 4096, 4096, 
0)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_VP9, 8192, 4352, 0)},
+};
+
 static struct amdgpu_video_codecs sriov_sc_video_codecs_encode =
 {
.codec_count = ARRAY_SIZE(sriov_sc_video_codecs_encode_array),
.codec_array = sriov_sc_video_codecs_encode_array,
 };
 
-static struct amdgpu_video_codecs sriov_sc_video_codecs_decode =
+static struct amdgpu_video_codecs sriov_sc_video_codecs_decode_vcn0 =
 {
-   .codec_count = ARRAY_SIZE(sriov_sc_video_codecs_decode_array),
-   .codec_array = sriov_sc_video_codecs_decode_array,
+   .codec_count = ARRAY_SIZE(sriov_sc_video_codecs_decode_array_vcn0),
+   .codec_array = sriov_sc_video_codecs_decode_array_vcn0,
+};
+
+static struct amdgpu_video_codecs sriov_sc_video_codecs_decode_vcn1 =
+{
+   .codec_count = ARRAY_SIZE(sriov_sc_video_codecs_decode_array_vcn1),
+   .codec_array = sriov_sc_video_codecs_decode_array_vcn1,
 };
 
 /* Beige Goby*/
@@ -181,20 +215,37 @@ static const struct amdgpu_video_codecs 
yc_video_codecs_decode = {
 static int nv_query_video_codecs(struct amdgpu_device *adev, bool encode,
 const struct amdgpu_video_codecs **codecs)
 {
+   if (adev->vcn.num_vcn_inst == hweight8(adev->vcn.harvest_config))
+   return -EINVAL;
+
switch 

[PATCH] drm/amdgpu: update wave data type to 3 for gfx11

2023-01-17 Thread Graham Sider
SQ_WAVE_INST_DW0 isn't present on gfx11 compared to gfx10, so update
wave data type to signify a difference.

Signed-off-by: Graham Sider 
---
 drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
index f98c67d07a9b..f821309f48c9 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
@@ -754,8 +754,8 @@ static void gfx_v11_0_read_wave_data(struct amdgpu_device 
*adev, uint32_t simd,
 * zero here */
WARN_ON(simd != 0);
 
-   /* type 2 wave data */
-   dst[(*no_fields)++] = 2;
+   /* type 3 wave data */
+   dst[(*no_fields)++] = 3;
dst[(*no_fields)++] = wave_read_ind(adev, wave, ixSQ_WAVE_STATUS);
dst[(*no_fields)++] = wave_read_ind(adev, wave, ixSQ_WAVE_PC_LO);
dst[(*no_fields)++] = wave_read_ind(adev, wave, ixSQ_WAVE_PC_HI);
-- 
2.25.1



Re: [PATCH 1/2] drm/amdgpu: return the PCIe gen and lanes from the INFO

2023-01-17 Thread Alex Deucher
Looks good to me.  It might be good to add a comment above pcie_gen in
amdgpu_drm.h to specify that this is the max common pcie gen supported
by both the GPU and the slot.  With that fixed, the patch is:
Reviewed-by: Alex Deucher 

On Fri, Jan 13, 2023 at 6:33 PM Marek Olšák  wrote:
>
> There is no hole on 32-bit unfortunately. It looks like the hole on 64-bit is 
> now ABI.
>
> I moved the field to replace _pad1. The patch is attached (with your Rb).
>
> Marek
>
> On Fri, Jan 13, 2023 at 4:20 PM Alex Deucher  wrote:
>>
>> On Fri, Jan 13, 2023 at 4:02 PM Marek Olšák  wrote:
>> >
>> > i've added the comments and indeed pahole shows the hole as expected.
>>
>> What about on 32-bit?
>>
>> Alex
>>
>> >
>> > Marek
>> >
>> > On Thu, Jan 12, 2023 at 11:44 AM Alex Deucher  
>> > wrote:
>> >>
>> >> On Thu, Jan 12, 2023 at 6:50 AM Christian König
>> >>  wrote:
>> >> >
>> >> > Am 11.01.23 um 21:48 schrieb Alex Deucher:
>> >> > > On Wed, Jan 4, 2023 at 3:17 PM Marek Olšák  wrote:
>> >> > >> Yes, it's meant to be like a spec sheet. We are not interested in 
>> >> > >> the current bandwidth utilization.
>> >> > > After chatting with Marek on IRC and thinking about this more, I think
>> >> > > this patch is fine.  It's not really meant for bandwidth per se, but
>> >> > > rather as a limit to determine what the driver should do in certain
>> >> > > cases (i.e., when does it make sense to copy to vram vs not).  It's
>> >> > > not straightforward for userspace to parse the full topology to
>> >> > > determine what links may be slow.  I guess one potential pitfall would
>> >> > > be that if you pass the device into a VM, the driver may report the
>> >> > > wrong values.  Generally in a VM the VM doesn't get the full view up
>> >> > > to the root port.  I don't know if the hypervisors report properly for
>> >> > > pcie_bandwidth_available() in a VM or if it just shows the info about
>> >> > > the endpoint in the VM.
>> >> >
>> >> > So this basically doesn't return the gen and lanes of the device, but
>> >> > rather what was negotiated between the device and the upstream root 
>> >> > port?
>> >>
>> >> Correct. It exposes the max gen and lanes of the slowest link between
>> >> the device and the root port.
>> >>
>> >> >
>> >> > If I got that correctly then we should probably document that cause
>> >> > otherwise somebody will try to "fix" it at some time.
>> >>
>> >> Good point.
>> >>
>> >> Alex
>> >>
>> >> >
>> >> > Christian.
>> >> >
>> >> > >
>> >> > > Reviewed-by: Alex Deucher 
>> >> > >
>> >> > > Alex
>> >> > >
>> >> > >> Marek
>> >> > >>
>> >> > >> On Wed, Jan 4, 2023 at 10:33 AM Lazar, Lijo  
>> >> > >> wrote:
>> >> > >>> [AMD Official Use Only - General]
>> >> > >>>
>> >> > >>>
>> >> > >>> To clarify, with DPM in place, the current bandwidth will be 
>> >> > >>> changing based on the load.
>> >> > >>>
>> >> > >>> If apps/umd already has a way to know the current bandwidth 
>> >> > >>> utilisation, then possible maximum also could be part of the same 
>> >> > >>> API. Otherwise, this only looks like duplicate information. We have 
>> >> > >>> the same information in sysfs DPM nodes.
>> >> > >>>
>> >> > >>> BTW, I don't know to what extent app/umd really makes use of this. 
>> >> > >>> Take that memory frequency as an example (I'm reading it as 16GHz). 
>> >> > >>> It only looks like a spec sheet.
>> >> > >>>
>> >> > >>> Thanks,
>> >> > >>> Lijo
>> >> > >>> 
>> >> > >>> From: Marek Olšák 
>> >> > >>> Sent: Wednesday, January 4, 2023 8:40:00 PM
>> >> > >>> To: Lazar, Lijo 
>> >> > >>> Cc: amd-gfx@lists.freedesktop.org 
>> >> > >>> Subject: Re: [PATCH 1/2] drm/amdgpu: return the PCIe gen and lanes 
>> >> > >>> from the INFO
>> >> > >>>
>> >> > >>> On Wed, Jan 4, 2023 at 9:19 AM Lazar, Lijo  
>> >> > >>> wrote:
>> >> > >>>
>> >> > >>>
>> >> > >>>
>> >> > >>> On 1/4/2023 7:43 PM, Marek Olšák wrote:
>> >> >  On Wed, Jan 4, 2023 at 6:50 AM Lazar, Lijo > >> >  > wrote:
>> >> > 
>> >> > 
>> >> > 
>> >> >   On 1/4/2023 4:11 AM, Marek Olšák wrote:
>> >> >    > I see. Well, those sysfs files are not usable, and I don't 
>> >> >  think it
>> >> >    > would be important even if they were usable, but for 
>> >> >  completeness:
>> >> >    >
>> >> >    > The ioctl returns:
>> >> >    >  pcie_gen = 1
>> >> >    >  pcie_num_lanes = 16
>> >> >    >
>> >> >    > Theoretical bandwidth from those values: 4.0 GB/s
>> >> >    > My DMA test shows this write bandwidth: 3.5 GB/s
>> >> >    > It matches the expectation.
>> >> >    >
>> >> >    > Let's see the devices (there is only 1 GPU Navi21 in the 
>> >> >  system):
>> >> >    > $ lspci |egrep '(PCI|VGA).*Navi'
>> >> >    > 0a:00.0 PCI bridge: Advanced Micro Devices, Inc. [AMD/ATI] 
>> >> >  Navi
>> >> >   10 XL
>> >> >    > Upstream Port of PCI Express 

Re: [PATCH v3 2/2] drm/amdgpu/vcn: Remove redundant indirect SRAM HW model check

2023-01-17 Thread Alex Deucher
Applied the series.  Thanks!

Alex

On Tue, Jan 17, 2023 at 1:15 PM Guilherme G. Piccoli
 wrote:
>
> The HW model validation that guards the indirect SRAM checking in the
> VCN code path is redundant - there's no model that's not included in the
> switch, making it useless in practice [0].
>
> So, let's remove this switch statement for good.
>
> [0] 
> lore.kernel.org/amd-gfx/mn0pr12mb61013d20b8a2263b22ae1bcfe2...@mn0pr12mb6101.namprd12.prod.outlook.com
>
> Suggested-by: Alex Deucher 
> Reviewed-by: Mario Limonciello 
> Cc: James Zhu 
> Cc: Lazar Lijo 
> Cc: Leo Liu 
> Cc: Sonny Jiang 
> Signed-off-by: Guilherme G. Piccoli 
> ---
>
>
> V3:
> * Added Mario's review tag and Alex's suggested tag - thanks
> for the reminder Mario!
>
> V2:
> * Changed the approach after ML discussion- instead of cleaning up
> the switch statement, removed it entirely - special thanks to Alex
> and Mario for the feedback!
>
>
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 81 +
>  1 file changed, 3 insertions(+), 78 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index 1b1a3c9e1863..02d428ddf2f8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -110,84 +110,9 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
> for (i = 0; i < adev->vcn.num_vcn_inst; i++)
> atomic_set(>vcn.inst[i].dpg_enc_submission_cnt, 0);
>
> -   switch (adev->ip_versions[UVD_HWIP][0]) {
> -   case IP_VERSION(1, 0, 0):
> -   case IP_VERSION(1, 0, 1):
> -   case IP_VERSION(2, 5, 0):
> -   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> -   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> -   adev->vcn.indirect_sram = true;
> -   break;
> -   case IP_VERSION(2, 2, 0):
> -   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> -   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> -   adev->vcn.indirect_sram = true;
> -   break;
> -   case IP_VERSION(2, 6, 0):
> -   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> -   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> -   adev->vcn.indirect_sram = true;
> -   break;
> -   case IP_VERSION(2, 0, 0):
> -   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> -   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> -   adev->vcn.indirect_sram = true;
> -   break;
> -   case IP_VERSION(2, 0, 2):
> -   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> -   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> -   adev->vcn.indirect_sram = true;
> -   break;
> -   case IP_VERSION(3, 0, 0):
> -   case IP_VERSION(3, 0, 64):
> -   case IP_VERSION(3, 0, 192):
> -   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> -   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> -   adev->vcn.indirect_sram = true;
> -   break;
> -   case IP_VERSION(3, 0, 2):
> -   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> -   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> -   adev->vcn.indirect_sram = true;
> -   break;
> -   case IP_VERSION(3, 0, 16):
> -   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> -   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> -   adev->vcn.indirect_sram = true;
> -   break;
> -   case IP_VERSION(3, 0, 33):
> -   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> -   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> -   adev->vcn.indirect_sram = true;
> -   break;
> -   case IP_VERSION(3, 1, 1):
> -   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> -   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> -   adev->vcn.indirect_sram = true;
> -   break;
> -   case IP_VERSION(3, 1, 2):
> -   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> -   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> -   adev->vcn.indirect_sram = true;
> -   break;
> -   case IP_VERSION(4, 0, 0):
> -   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> -   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> -   adev->vcn.indirect_sram = true;
> -   break;
> -   case IP_VERSION(4, 0, 2):
> -   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> -   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> -   

Re: [PATCH v2 2/2] drm/amdgpu/vcn: Remove redundant indirect SRAM HW model check

2023-01-17 Thread Guilherme G. Piccoli
On 17/01/2023 15:14, Limonciello, Mario wrote:
> [Public]
> 
> 
> 
>> -Original Message-
>> From: Guilherme G. Piccoli 
>> Sent: Tuesday, January 17, 2023 12:14
>> To: Limonciello, Mario ; amd-
>> g...@lists.freedesktop.org; Deucher, Alexander
>> 
>> Cc: dri-de...@lists.freedesktop.org; Koenig, Christian
>> ; Pan, Xinhui ;
>> ker...@gpiccoli.net; kernel-...@igalia.com; Zhu, James
>> ; Lazar, Lijo ; Liu, Leo
>> ; Jiang, Sonny 
>> Subject: Re: [PATCH v2 2/2] drm/amdgpu/vcn: Remove redundant indirect
>> SRAM HW model check
>>
>> On 17/01/2023 15:08, Limonciello, Mario wrote:
>>> [...]
>>>
>>> Should have added this tag too:
>>> Suggested-by: Alexander Deucher 
>>>
>>> Looks good to me, thanks!
>>> Reviewed-by: Mario Limonciello 
>>>
>>
>> You're totally right, thanks for the reminder and apologies for missing
>> that! Just sending V3 heheh
>>
>> Ah, thanks for the reviews and prompt responses.
>> Cheers,
>>
>>
>> Guilherme
> 
> No need to resend.  Patchwork will embed the tags when we pick this up.

Already did, but thanks again for the info - learning a lot in this
thread =)


Re: [PATCH 2/2] drm_print: Remove deprecated DRM_DEBUG_KMS_RATELIMITED()

2023-01-17 Thread Das, Nirmoy

Hi Alex,

On 1/17/2023 7:06 PM, Alex Deucher wrote:

On Tue, Jan 17, 2023 at 1:05 PM Nirmoy Das  wrote:

There are no current users of DRM_DEBUG_KMS_RATELIMITED()
so remove it.

Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Thomas Zimmermann 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Sam Ravnborg 

Signed-off-by: Nirmoy Das 
Reviewed-by: Sam Ravnborg 

Series is:
Reviewed-by: Alex Deucher 

Feel free to take the patches through whatever tree you want.



Please help me with this, I don't have committer rights for any tree.


Nirmoy




Alex


---
  include/drm/drm_print.h | 3 ---
  1 file changed, 3 deletions(-)

diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index a44fb7ef257f..c3753da97c4e 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -605,9 +605,6 @@ void __drm_err(const char *format, ...);
  #define drm_dbg_kms_ratelimited(drm, fmt, ...) \
 __DRM_DEFINE_DBG_RATELIMITED(KMS, drm, fmt, ## __VA_ARGS__)

-/* NOTE: this is deprecated in favor of drm_dbg_kms_ratelimited(NULL, ...). */
-#define DRM_DEBUG_KMS_RATELIMITED(fmt, ...) drm_dbg_kms_ratelimited(NULL, fmt, 
## __VA_ARGS__)
-
  /*
   * struct drm_device based WARNs
   *
--
2.39.0



[PATCH v3 2/2] drm/amdgpu/vcn: Remove redundant indirect SRAM HW model check

2023-01-17 Thread Guilherme G. Piccoli
The HW model validation that guards the indirect SRAM checking in the
VCN code path is redundant - there's no model that's not included in the
switch, making it useless in practice [0].

So, let's remove this switch statement for good.

[0] 
lore.kernel.org/amd-gfx/mn0pr12mb61013d20b8a2263b22ae1bcfe2...@mn0pr12mb6101.namprd12.prod.outlook.com

Suggested-by: Alex Deucher 
Reviewed-by: Mario Limonciello 
Cc: James Zhu 
Cc: Lazar Lijo 
Cc: Leo Liu 
Cc: Sonny Jiang 
Signed-off-by: Guilherme G. Piccoli 
---


V3:
* Added Mario's review tag and Alex's suggested tag - thanks
for the reminder Mario!

V2:
* Changed the approach after ML discussion- instead of cleaning up
the switch statement, removed it entirely - special thanks to Alex
and Mario for the feedback!


 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 81 +
 1 file changed, 3 insertions(+), 78 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 1b1a3c9e1863..02d428ddf2f8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -110,84 +110,9 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
for (i = 0; i < adev->vcn.num_vcn_inst; i++)
atomic_set(>vcn.inst[i].dpg_enc_submission_cnt, 0);
 
-   switch (adev->ip_versions[UVD_HWIP][0]) {
-   case IP_VERSION(1, 0, 0):
-   case IP_VERSION(1, 0, 1):
-   case IP_VERSION(2, 5, 0):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(2, 2, 0):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(2, 6, 0):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(2, 0, 0):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(2, 0, 2):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(3, 0, 0):
-   case IP_VERSION(3, 0, 64):
-   case IP_VERSION(3, 0, 192):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(3, 0, 2):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(3, 0, 16):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(3, 0, 33):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(3, 1, 1):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(3, 1, 2):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(4, 0, 0):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(4, 0, 2):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(4, 0, 4):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   

[PATCH v3 1/2] drm/amdgpu/vcn: Adjust firmware names indentation

2023-01-17 Thread Guilherme G. Piccoli
This is an incredibly trivial fix, just for the sake of
"aesthetical" organization of the defines. Some were space based,
most were tab based and there was a lack of "alignment", now it's
all the same and aligned.

Cc: James Zhu 
Cc: Lazar Lijo 
Cc: Leo Liu 
Cc: Mario Limonciello 
Cc: Sonny Jiang 
Reviewed-by: Alex Deucher 
Signed-off-by: Guilherme G. Piccoli 
---


V2/V3:
* Added Alex's review tag - thanks!


 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 38 -
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index f8397d993f23..1b1a3c9e1863 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -36,26 +36,26 @@
 #include "soc15d.h"
 
 /* Firmware Names */
-#define FIRMWARE_RAVEN "amdgpu/raven_vcn.bin"
-#define FIRMWARE_PICASSO   "amdgpu/picasso_vcn.bin"
-#define FIRMWARE_RAVEN2"amdgpu/raven2_vcn.bin"
-#define FIRMWARE_ARCTURUS  "amdgpu/arcturus_vcn.bin"
-#define FIRMWARE_RENOIR"amdgpu/renoir_vcn.bin"
-#define FIRMWARE_GREEN_SARDINE "amdgpu/green_sardine_vcn.bin"
-#define FIRMWARE_NAVI10"amdgpu/navi10_vcn.bin"
-#define FIRMWARE_NAVI14"amdgpu/navi14_vcn.bin"
-#define FIRMWARE_NAVI12"amdgpu/navi12_vcn.bin"
-#define FIRMWARE_SIENNA_CICHLID"amdgpu/sienna_cichlid_vcn.bin"
-#define FIRMWARE_NAVY_FLOUNDER "amdgpu/navy_flounder_vcn.bin"
-#define FIRMWARE_VANGOGH   "amdgpu/vangogh_vcn.bin"
+#define FIRMWARE_RAVEN "amdgpu/raven_vcn.bin"
+#define FIRMWARE_PICASSO   "amdgpu/picasso_vcn.bin"
+#define FIRMWARE_RAVEN2"amdgpu/raven2_vcn.bin"
+#define FIRMWARE_ARCTURUS  "amdgpu/arcturus_vcn.bin"
+#define FIRMWARE_RENOIR"amdgpu/renoir_vcn.bin"
+#define FIRMWARE_GREEN_SARDINE "amdgpu/green_sardine_vcn.bin"
+#define FIRMWARE_NAVI10"amdgpu/navi10_vcn.bin"
+#define FIRMWARE_NAVI14"amdgpu/navi14_vcn.bin"
+#define FIRMWARE_NAVI12"amdgpu/navi12_vcn.bin"
+#define FIRMWARE_SIENNA_CICHLID"amdgpu/sienna_cichlid_vcn.bin"
+#define FIRMWARE_NAVY_FLOUNDER "amdgpu/navy_flounder_vcn.bin"
+#define FIRMWARE_VANGOGH   "amdgpu/vangogh_vcn.bin"
 #define FIRMWARE_DIMGREY_CAVEFISH  "amdgpu/dimgrey_cavefish_vcn.bin"
-#define FIRMWARE_ALDEBARAN "amdgpu/aldebaran_vcn.bin"
-#define FIRMWARE_BEIGE_GOBY"amdgpu/beige_goby_vcn.bin"
-#define FIRMWARE_YELLOW_CARP   "amdgpu/yellow_carp_vcn.bin"
-#define FIRMWARE_VCN_3_1_2 "amdgpu/vcn_3_1_2.bin"
-#define FIRMWARE_VCN4_0_0  "amdgpu/vcn_4_0_0.bin"
-#define FIRMWARE_VCN4_0_2  "amdgpu/vcn_4_0_2.bin"
-#define FIRMWARE_VCN4_0_4  "amdgpu/vcn_4_0_4.bin"
+#define FIRMWARE_ALDEBARAN "amdgpu/aldebaran_vcn.bin"
+#define FIRMWARE_BEIGE_GOBY"amdgpu/beige_goby_vcn.bin"
+#define FIRMWARE_YELLOW_CARP   "amdgpu/yellow_carp_vcn.bin"
+#define FIRMWARE_VCN_3_1_2 "amdgpu/vcn_3_1_2.bin"
+#define FIRMWARE_VCN4_0_0  "amdgpu/vcn_4_0_0.bin"
+#define FIRMWARE_VCN4_0_2  "amdgpu/vcn_4_0_2.bin"
+#define FIRMWARE_VCN4_0_4  "amdgpu/vcn_4_0_4.bin"
 
 MODULE_FIRMWARE(FIRMWARE_RAVEN);
 MODULE_FIRMWARE(FIRMWARE_PICASSO);
-- 
2.39.0



Re: [PATCH v2 2/2] drm/amdgpu/vcn: Remove redundant indirect SRAM HW model check

2023-01-17 Thread Guilherme G. Piccoli
On 17/01/2023 15:08, Limonciello, Mario wrote:
> [...]
> 
> Should have added this tag too:
> Suggested-by: Alexander Deucher 
> 
> Looks good to me, thanks!
> Reviewed-by: Mario Limonciello 
> 

You're totally right, thanks for the reminder and apologies for missing
that! Just sending V3 heheh

Ah, thanks for the reviews and prompt responses.
Cheers,


Guilherme


RE: [PATCH v2 2/2] drm/amdgpu/vcn: Remove redundant indirect SRAM HW model check

2023-01-17 Thread Limonciello, Mario
[Public]



> -Original Message-
> From: Guilherme G. Piccoli 
> Sent: Tuesday, January 17, 2023 12:14
> To: Limonciello, Mario ; amd-
> g...@lists.freedesktop.org; Deucher, Alexander
> 
> Cc: dri-de...@lists.freedesktop.org; Koenig, Christian
> ; Pan, Xinhui ;
> ker...@gpiccoli.net; kernel-...@igalia.com; Zhu, James
> ; Lazar, Lijo ; Liu, Leo
> ; Jiang, Sonny 
> Subject: Re: [PATCH v2 2/2] drm/amdgpu/vcn: Remove redundant indirect
> SRAM HW model check
> 
> On 17/01/2023 15:08, Limonciello, Mario wrote:
> > [...]
> >
> > Should have added this tag too:
> > Suggested-by: Alexander Deucher 
> >
> > Looks good to me, thanks!
> > Reviewed-by: Mario Limonciello 
> >
> 
> You're totally right, thanks for the reminder and apologies for missing
> that! Just sending V3 heheh
> 
> Ah, thanks for the reviews and prompt responses.
> Cheers,
> 
> 
> Guilherme

No need to resend.  Patchwork will embed the tags when we pick this up.


RE: [PATCH v2 2/2] drm/amdgpu/vcn: Remove redundant indirect SRAM HW model check

2023-01-17 Thread Limonciello, Mario
[Public]



> -Original Message-
> From: Guilherme G. Piccoli 
> Sent: Tuesday, January 17, 2023 11:59
> To: amd-gfx@lists.freedesktop.org
> Cc: dri-de...@lists.freedesktop.org; Deucher, Alexander
> ; Koenig, Christian
> ; Pan, Xinhui ;
> ker...@gpiccoli.net; kernel-...@igalia.com; Guilherme G. Piccoli
> ; Zhu, James ; Lazar, Lijo
> ; Liu, Leo ; Limonciello, Mario
> ; Jiang, Sonny 
> Subject: [PATCH v2 2/2] drm/amdgpu/vcn: Remove redundant indirect SRAM
> HW model check
> 
> The HW model validation that guards the indirect SRAM checking in the
> VCN code path is redundant - there's no model that's not included in the
> switch, making it useless in practice [0].
> 
> So, let's remove this switch statement for good.
> 
> [0] lore.kernel.org/amd-
> gfx/mn0pr12mb61013d20b8a2263b22ae1bcfe2...@mn0pr12mb6101.na
> mprd12.prod.outlook.com
> 
> Cc: James Zhu 
> Cc: Lazar Lijo 
> Cc: Leo Liu 
> Cc: Mario Limonciello 
> Cc: Sonny Jiang 
> Signed-off-by: Guilherme G. Piccoli 

Should have added this tag too:
Suggested-by: Alexander Deucher 

Looks good to me, thanks!
Reviewed-by: Mario Limonciello 

> ---
> 
> 
> V2:
> * Changed the approach after ML discussion- instead of cleaning up
> the switch statement, removed it entirely - special thanks to Alex
> and Mario for the feedback!
> 
> Notice that patch 3 was dropped from this series after reviews.
> 
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 81 +
>  1 file changed, 3 insertions(+), 78 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index 1b1a3c9e1863..02d428ddf2f8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -110,84 +110,9 @@ int amdgpu_vcn_sw_init(struct amdgpu_device
> *adev)
>   for (i = 0; i < adev->vcn.num_vcn_inst; i++)
>   atomic_set(>vcn.inst[i].dpg_enc_submission_cnt, 0);
> 
> - switch (adev->ip_versions[UVD_HWIP][0]) {
> - case IP_VERSION(1, 0, 0):
> - case IP_VERSION(1, 0, 1):
> - case IP_VERSION(2, 5, 0):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP)
> &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> - case IP_VERSION(2, 2, 0):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP)
> &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> - case IP_VERSION(2, 6, 0):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP)
> &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> - case IP_VERSION(2, 0, 0):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP)
> &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> - case IP_VERSION(2, 0, 2):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP)
> &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> - case IP_VERSION(3, 0, 0):
> - case IP_VERSION(3, 0, 64):
> - case IP_VERSION(3, 0, 192):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP)
> &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> - case IP_VERSION(3, 0, 2):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP)
> &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> - case IP_VERSION(3, 0, 16):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP)
> &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> - case IP_VERSION(3, 0, 33):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP)
> &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> - case IP_VERSION(3, 1, 1):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP)
> &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> - case IP_VERSION(3, 1, 2):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP)
> &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> - case IP_VERSION(4, 0, 0):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP)
> &&
> -

Re: [PATCH 2/2] drm_print: Remove deprecated DRM_DEBUG_KMS_RATELIMITED()

2023-01-17 Thread Alex Deucher
On Tue, Jan 17, 2023 at 1:05 PM Nirmoy Das  wrote:
>
> There are no current users of DRM_DEBUG_KMS_RATELIMITED()
> so remove it.
>
> Cc: Maarten Lankhorst 
> Cc: Maxime Ripard 
> Cc: Thomas Zimmermann 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: Sam Ravnborg 
>
> Signed-off-by: Nirmoy Das 
> Reviewed-by: Sam Ravnborg 

Series is:
Reviewed-by: Alex Deucher 

Feel free to take the patches through whatever tree you want.

Alex

> ---
>  include/drm/drm_print.h | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index a44fb7ef257f..c3753da97c4e 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -605,9 +605,6 @@ void __drm_err(const char *format, ...);
>  #define drm_dbg_kms_ratelimited(drm, fmt, ...) \
> __DRM_DEFINE_DBG_RATELIMITED(KMS, drm, fmt, ## __VA_ARGS__)
>
> -/* NOTE: this is deprecated in favor of drm_dbg_kms_ratelimited(NULL, ...). 
> */
> -#define DRM_DEBUG_KMS_RATELIMITED(fmt, ...) drm_dbg_kms_ratelimited(NULL, 
> fmt, ## __VA_ARGS__)
> -
>  /*
>   * struct drm_device based WARNs
>   *
> --
> 2.39.0
>


[PATCH v2] drm/radeon: Do not use deprecated drm log API

2023-01-17 Thread Nirmoy Das
Replace deprecated DRM_DEBUG_KMS_RATELIMITED() and DRM_ERROR()
with proper APIs.

v2: replace pr_err with dev_err(Alex).

Cc: Alex Deucher 
Cc: Christian König 

Signed-off-by: Nirmoy Das 
---
 drivers/gpu/drm/radeon/radeon_dp_auxch.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_dp_auxch.c 
b/drivers/gpu/drm/radeon/radeon_dp_auxch.c
index 69379b95146e..1e5b6baf76a1 100644
--- a/drivers/gpu/drm/radeon/radeon_dp_auxch.c
+++ b/drivers/gpu/drm/radeon/radeon_dp_auxch.c
@@ -158,7 +158,7 @@ radeon_dp_aux_transfer_native(struct drm_dp_aux *aux, 
struct drm_dp_aux_msg *msg
} while (retry_count++ < 1000);
 
if (retry_count >= 1000) {
-   DRM_ERROR("auxch hw never signalled completion, error %08x\n", 
tmp);
+   dev_err(rdev->dev, "auxch hw never signalled completion, error 
%08x\n", tmp);
ret = -EIO;
goto done;
}
@@ -168,8 +168,7 @@ radeon_dp_aux_transfer_native(struct drm_dp_aux *aux, 
struct drm_dp_aux_msg *msg
goto done;
}
if (tmp & AUX_RX_ERROR_FLAGS) {
-   DRM_DEBUG_KMS_RATELIMITED("dp_aux_ch flags not zero: %08x\n",
- tmp);
+   drm_dbg_kms_ratelimited(dev, "dp_aux_ch flags not zero: 
%08x\n", tmp);
ret = -EIO;
goto done;
}
-- 
2.39.0



[PATCH 2/2] drm_print: Remove deprecated DRM_DEBUG_KMS_RATELIMITED()

2023-01-17 Thread Nirmoy Das
There are no current users of DRM_DEBUG_KMS_RATELIMITED()
so remove it.

Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Thomas Zimmermann 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Sam Ravnborg 

Signed-off-by: Nirmoy Das 
Reviewed-by: Sam Ravnborg 
---
 include/drm/drm_print.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index a44fb7ef257f..c3753da97c4e 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -605,9 +605,6 @@ void __drm_err(const char *format, ...);
 #define drm_dbg_kms_ratelimited(drm, fmt, ...) \
__DRM_DEFINE_DBG_RATELIMITED(KMS, drm, fmt, ## __VA_ARGS__)
 
-/* NOTE: this is deprecated in favor of drm_dbg_kms_ratelimited(NULL, ...). */
-#define DRM_DEBUG_KMS_RATELIMITED(fmt, ...) drm_dbg_kms_ratelimited(NULL, fmt, 
## __VA_ARGS__)
-
 /*
  * struct drm_device based WARNs
  *
-- 
2.39.0



[PATCH v2 2/2] drm/amdgpu/vcn: Remove redundant indirect SRAM HW model check

2023-01-17 Thread Guilherme G. Piccoli
The HW model validation that guards the indirect SRAM checking in the
VCN code path is redundant - there's no model that's not included in the
switch, making it useless in practice [0].

So, let's remove this switch statement for good.

[0] 
lore.kernel.org/amd-gfx/mn0pr12mb61013d20b8a2263b22ae1bcfe2...@mn0pr12mb6101.namprd12.prod.outlook.com

Cc: James Zhu 
Cc: Lazar Lijo 
Cc: Leo Liu 
Cc: Mario Limonciello 
Cc: Sonny Jiang 
Signed-off-by: Guilherme G. Piccoli 
---


V2:
* Changed the approach after ML discussion- instead of cleaning up
the switch statement, removed it entirely - special thanks to Alex
and Mario for the feedback!

Notice that patch 3 was dropped from this series after reviews.


 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 81 +
 1 file changed, 3 insertions(+), 78 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 1b1a3c9e1863..02d428ddf2f8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -110,84 +110,9 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
for (i = 0; i < adev->vcn.num_vcn_inst; i++)
atomic_set(>vcn.inst[i].dpg_enc_submission_cnt, 0);
 
-   switch (adev->ip_versions[UVD_HWIP][0]) {
-   case IP_VERSION(1, 0, 0):
-   case IP_VERSION(1, 0, 1):
-   case IP_VERSION(2, 5, 0):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(2, 2, 0):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(2, 6, 0):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(2, 0, 0):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(2, 0, 2):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(3, 0, 0):
-   case IP_VERSION(3, 0, 64):
-   case IP_VERSION(3, 0, 192):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(3, 0, 2):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(3, 0, 16):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(3, 0, 33):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(3, 1, 1):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(3, 1, 2):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(4, 0, 0):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(4, 0, 2):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   case IP_VERSION(4, 0, 4):
-   if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
-   adev->vcn.indirect_sram = true;
-   break;
-   default:
-   return -EINVAL;
-   }
+   if 

[PATCH v2 1/2] drm/amdgpu/vcn: Adjust firmware names indentation

2023-01-17 Thread Guilherme G. Piccoli
This is an incredibly trivial fix, just for the sake of
"aesthetical" organization of the defines. Some were space based,
most were tab based and there was a lack of "alignment", now it's
all the same and aligned.

Cc: James Zhu 
Cc: Lazar Lijo 
Cc: Leo Liu 
Cc: Mario Limonciello 
Cc: Sonny Jiang 
Reviewed-by: Alex Deucher 
Signed-off-by: Guilherme G. Piccoli 
---


V2:
* Added Alex's review tag - thanks!


 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 38 -
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index f8397d993f23..1b1a3c9e1863 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -36,26 +36,26 @@
 #include "soc15d.h"
 
 /* Firmware Names */
-#define FIRMWARE_RAVEN "amdgpu/raven_vcn.bin"
-#define FIRMWARE_PICASSO   "amdgpu/picasso_vcn.bin"
-#define FIRMWARE_RAVEN2"amdgpu/raven2_vcn.bin"
-#define FIRMWARE_ARCTURUS  "amdgpu/arcturus_vcn.bin"
-#define FIRMWARE_RENOIR"amdgpu/renoir_vcn.bin"
-#define FIRMWARE_GREEN_SARDINE "amdgpu/green_sardine_vcn.bin"
-#define FIRMWARE_NAVI10"amdgpu/navi10_vcn.bin"
-#define FIRMWARE_NAVI14"amdgpu/navi14_vcn.bin"
-#define FIRMWARE_NAVI12"amdgpu/navi12_vcn.bin"
-#define FIRMWARE_SIENNA_CICHLID"amdgpu/sienna_cichlid_vcn.bin"
-#define FIRMWARE_NAVY_FLOUNDER "amdgpu/navy_flounder_vcn.bin"
-#define FIRMWARE_VANGOGH   "amdgpu/vangogh_vcn.bin"
+#define FIRMWARE_RAVEN "amdgpu/raven_vcn.bin"
+#define FIRMWARE_PICASSO   "amdgpu/picasso_vcn.bin"
+#define FIRMWARE_RAVEN2"amdgpu/raven2_vcn.bin"
+#define FIRMWARE_ARCTURUS  "amdgpu/arcturus_vcn.bin"
+#define FIRMWARE_RENOIR"amdgpu/renoir_vcn.bin"
+#define FIRMWARE_GREEN_SARDINE "amdgpu/green_sardine_vcn.bin"
+#define FIRMWARE_NAVI10"amdgpu/navi10_vcn.bin"
+#define FIRMWARE_NAVI14"amdgpu/navi14_vcn.bin"
+#define FIRMWARE_NAVI12"amdgpu/navi12_vcn.bin"
+#define FIRMWARE_SIENNA_CICHLID"amdgpu/sienna_cichlid_vcn.bin"
+#define FIRMWARE_NAVY_FLOUNDER "amdgpu/navy_flounder_vcn.bin"
+#define FIRMWARE_VANGOGH   "amdgpu/vangogh_vcn.bin"
 #define FIRMWARE_DIMGREY_CAVEFISH  "amdgpu/dimgrey_cavefish_vcn.bin"
-#define FIRMWARE_ALDEBARAN "amdgpu/aldebaran_vcn.bin"
-#define FIRMWARE_BEIGE_GOBY"amdgpu/beige_goby_vcn.bin"
-#define FIRMWARE_YELLOW_CARP   "amdgpu/yellow_carp_vcn.bin"
-#define FIRMWARE_VCN_3_1_2 "amdgpu/vcn_3_1_2.bin"
-#define FIRMWARE_VCN4_0_0  "amdgpu/vcn_4_0_0.bin"
-#define FIRMWARE_VCN4_0_2  "amdgpu/vcn_4_0_2.bin"
-#define FIRMWARE_VCN4_0_4  "amdgpu/vcn_4_0_4.bin"
+#define FIRMWARE_ALDEBARAN "amdgpu/aldebaran_vcn.bin"
+#define FIRMWARE_BEIGE_GOBY"amdgpu/beige_goby_vcn.bin"
+#define FIRMWARE_YELLOW_CARP   "amdgpu/yellow_carp_vcn.bin"
+#define FIRMWARE_VCN_3_1_2 "amdgpu/vcn_3_1_2.bin"
+#define FIRMWARE_VCN4_0_0  "amdgpu/vcn_4_0_0.bin"
+#define FIRMWARE_VCN4_0_2  "amdgpu/vcn_4_0_2.bin"
+#define FIRMWARE_VCN4_0_4  "amdgpu/vcn_4_0_4.bin"
 
 MODULE_FIRMWARE(FIRMWARE_RAVEN);
 MODULE_FIRMWARE(FIRMWARE_PICASSO);
-- 
2.39.0



Re: [PATCH 2/2] drm_print: Remove deprecated DRM_DEBUG_KMS_RATELIMITED()

2023-01-17 Thread Sam Ravnborg
On Tue, Jan 17, 2023 at 06:44:47PM +0100, Nirmoy Das wrote:
> There are no current users of DRM_DEBUG_KMS_RATELIMITED()
> so remove it.
Thanks
> 
> Cc: Maarten Lankhorst 
> Cc: Maxime Ripard 
> Cc: Thomas Zimmermann 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: Sam Ravnborg 
> 
> Signed-off-by: Nirmoy Das 
Reviewed-by: Sam Ravnborg 

> ---
>  include/drm/drm_print.h | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index a44fb7ef257f..c3753da97c4e 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -605,9 +605,6 @@ void __drm_err(const char *format, ...);
>  #define drm_dbg_kms_ratelimited(drm, fmt, ...) \
>   __DRM_DEFINE_DBG_RATELIMITED(KMS, drm, fmt, ## __VA_ARGS__)
>  
> -/* NOTE: this is deprecated in favor of drm_dbg_kms_ratelimited(NULL, ...). 
> */
> -#define DRM_DEBUG_KMS_RATELIMITED(fmt, ...) drm_dbg_kms_ratelimited(NULL, 
> fmt, ## __VA_ARGS__)
> -
>  /*
>   * struct drm_device based WARNs
>   *
> -- 
> 2.39.0


Re: [PATCH 1/2] drm/radeon: Do not use deprecated drm log API

2023-01-17 Thread Das, Nirmoy



On 1/17/2023 6:48 PM, Alex Deucher wrote:

On Tue, Jan 17, 2023 at 12:45 PM Nirmoy Das  wrote:

Replace deprecated DRM_DEBUG_KMS_RATELIMITED() and DRM_ERROR()
with proper APIs.

Cc: Alex Deucher 
Cc: Christian König 

Signed-off-by: Nirmoy Das 
---
  drivers/gpu/drm/radeon/radeon_dp_auxch.c | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_dp_auxch.c 
b/drivers/gpu/drm/radeon/radeon_dp_auxch.c
index 69379b95146e..76ce66efb5f8 100644
--- a/drivers/gpu/drm/radeon/radeon_dp_auxch.c
+++ b/drivers/gpu/drm/radeon/radeon_dp_auxch.c
@@ -158,7 +158,7 @@ radeon_dp_aux_transfer_native(struct drm_dp_aux *aux, 
struct drm_dp_aux_msg *msg
 } while (retry_count++ < 1000);

 if (retry_count >= 1000) {
-   DRM_ERROR("auxch hw never signalled completion, error %08x\n", 
tmp);
+   pr_err("auxch hw never signalled completion, error %08x\n", 
tmp);

Please use dev_err() instead so we get device identification on error
messages.  Makes it much easier when you have multiple GPUs in a
system.



Thanks for your quick review, Alex. I will resend with dev_err().


Nirmoy



Alex


 ret = -EIO;
 goto done;
 }
@@ -168,8 +168,7 @@ radeon_dp_aux_transfer_native(struct drm_dp_aux *aux, 
struct drm_dp_aux_msg *msg
 goto done;
 }
 if (tmp & AUX_RX_ERROR_FLAGS) {
-   DRM_DEBUG_KMS_RATELIMITED("dp_aux_ch flags not zero: %08x\n",
- tmp);
+   drm_dbg_kms_ratelimited(dev, "dp_aux_ch flags not zero: 
%08x\n", tmp);
 ret = -EIO;
 goto done;
 }
--
2.39.0



[PATCH 1/2] drm/radeon: Do not use deprecated drm log API

2023-01-17 Thread Nirmoy Das
Replace deprecated DRM_DEBUG_KMS_RATELIMITED() and DRM_ERROR()
with proper APIs.

Cc: Alex Deucher 
Cc: Christian König 

Signed-off-by: Nirmoy Das 
---
 drivers/gpu/drm/radeon/radeon_dp_auxch.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_dp_auxch.c 
b/drivers/gpu/drm/radeon/radeon_dp_auxch.c
index 69379b95146e..76ce66efb5f8 100644
--- a/drivers/gpu/drm/radeon/radeon_dp_auxch.c
+++ b/drivers/gpu/drm/radeon/radeon_dp_auxch.c
@@ -158,7 +158,7 @@ radeon_dp_aux_transfer_native(struct drm_dp_aux *aux, 
struct drm_dp_aux_msg *msg
} while (retry_count++ < 1000);
 
if (retry_count >= 1000) {
-   DRM_ERROR("auxch hw never signalled completion, error %08x\n", 
tmp);
+   pr_err("auxch hw never signalled completion, error %08x\n", 
tmp);
ret = -EIO;
goto done;
}
@@ -168,8 +168,7 @@ radeon_dp_aux_transfer_native(struct drm_dp_aux *aux, 
struct drm_dp_aux_msg *msg
goto done;
}
if (tmp & AUX_RX_ERROR_FLAGS) {
-   DRM_DEBUG_KMS_RATELIMITED("dp_aux_ch flags not zero: %08x\n",
- tmp);
+   drm_dbg_kms_ratelimited(dev, "dp_aux_ch flags not zero: 
%08x\n", tmp);
ret = -EIO;
goto done;
}
-- 
2.39.0



[PATCH 2/2] drm_print: Remove deprecated DRM_DEBUG_KMS_RATELIMITED()

2023-01-17 Thread Nirmoy Das
There are no current users of DRM_DEBUG_KMS_RATELIMITED()
so remove it.

Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Thomas Zimmermann 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Sam Ravnborg 

Signed-off-by: Nirmoy Das 
---
 include/drm/drm_print.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index a44fb7ef257f..c3753da97c4e 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -605,9 +605,6 @@ void __drm_err(const char *format, ...);
 #define drm_dbg_kms_ratelimited(drm, fmt, ...) \
__DRM_DEFINE_DBG_RATELIMITED(KMS, drm, fmt, ## __VA_ARGS__)
 
-/* NOTE: this is deprecated in favor of drm_dbg_kms_ratelimited(NULL, ...). */
-#define DRM_DEBUG_KMS_RATELIMITED(fmt, ...) drm_dbg_kms_ratelimited(NULL, fmt, 
## __VA_ARGS__)
-
 /*
  * struct drm_device based WARNs
  *
-- 
2.39.0



Re: [PATCH 1/2] drm/radeon: Do not use deprecated drm log API

2023-01-17 Thread Alex Deucher
On Tue, Jan 17, 2023 at 12:45 PM Nirmoy Das  wrote:
>
> Replace deprecated DRM_DEBUG_KMS_RATELIMITED() and DRM_ERROR()
> with proper APIs.
>
> Cc: Alex Deucher 
> Cc: Christian König 
>
> Signed-off-by: Nirmoy Das 
> ---
>  drivers/gpu/drm/radeon/radeon_dp_auxch.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_dp_auxch.c 
> b/drivers/gpu/drm/radeon/radeon_dp_auxch.c
> index 69379b95146e..76ce66efb5f8 100644
> --- a/drivers/gpu/drm/radeon/radeon_dp_auxch.c
> +++ b/drivers/gpu/drm/radeon/radeon_dp_auxch.c
> @@ -158,7 +158,7 @@ radeon_dp_aux_transfer_native(struct drm_dp_aux *aux, 
> struct drm_dp_aux_msg *msg
> } while (retry_count++ < 1000);
>
> if (retry_count >= 1000) {
> -   DRM_ERROR("auxch hw never signalled completion, error 
> %08x\n", tmp);
> +   pr_err("auxch hw never signalled completion, error %08x\n", 
> tmp);

Please use dev_err() instead so we get device identification on error
messages.  Makes it much easier when you have multiple GPUs in a
system.

Alex

> ret = -EIO;
> goto done;
> }
> @@ -168,8 +168,7 @@ radeon_dp_aux_transfer_native(struct drm_dp_aux *aux, 
> struct drm_dp_aux_msg *msg
> goto done;
> }
> if (tmp & AUX_RX_ERROR_FLAGS) {
> -   DRM_DEBUG_KMS_RATELIMITED("dp_aux_ch flags not zero: %08x\n",
> - tmp);
> +   drm_dbg_kms_ratelimited(dev, "dp_aux_ch flags not zero: 
> %08x\n", tmp);
> ret = -EIO;
> goto done;
> }
> --
> 2.39.0
>


[PATCH] [v2] drm/amd/display: fix dp_retrieve_lttpr_cap return code

2023-01-17 Thread Arnd Bergmann
From: Arnd Bergmann 

The dp_retrieve_lttpr_cap() return type changed from 'bool'
to 'enum dc_status', so now the early 'return' uses the wrong
type:

drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c: In function 
'dp_retrieve_lttpr_cap':
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:5075:24: error: 
implicit conversion from 'enum ' to 'enum dc_status' 
[-Werror=enum-conversion]
 5075 | return false;
  |^

Convert it to return 'DC_ERROR_UNEXPECTED', which was apparently set
as a default return code here but never used.

Fixes: b473bd5fc333 ("drm/amd/display: refine wake up aux in retrieve link 
caps")
Signed-off-by: Arnd Bergmann 
---
v2 changes:
 - use DC_ERROR_UNEXPECTED instead of DC_OK
 - remove bogus initializers
---
 drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index 9edfcdf3db3b..cf512362b4f1 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -5088,14 +5088,14 @@ static bool dpcd_read_sink_ext_caps(struct dc_link 
*link)
 enum dc_status dp_retrieve_lttpr_cap(struct dc_link *link)
 {
uint8_t lttpr_dpcd_data[8];
-   enum dc_status status = DC_ERROR_UNEXPECTED;
-   bool is_lttpr_present = false;
+   enum dc_status status;
+   bool is_lttpr_present;
 
/* Logic to determine LTTPR support*/
bool vbios_lttpr_interop = link->dc->caps.vbios_lttpr_aware;
 
if (!vbios_lttpr_interop || 
!link->dc->caps.extended_aux_timeout_support)
-   return false;
+   return DC_ERROR_UNEXPECTED;
 
/* By reading LTTPR capability, RX assumes that we will enable
 * LTTPR extended aux timeout if LTTPR is present.
-- 
2.39.0



Calculating array sizes in C - was: Re: Build regressions/improvements in v6.2-rc1

2023-01-17 Thread John Paul Adrian Glaubitz

Hi Geert!

On 1/6/23 16:17, Geert Uytterhoeven wrote:

I'm not seeing this one, but I am getting this one instead:

In file included from ./arch/sh/include/asm/hw_irq.h:6,
   from ./include/linux/irq.h:596,
   from ./include/asm-generic/hardirq.h:17,
   from ./arch/sh/include/asm/hardirq.h:9,
   from ./include/linux/hardirq.h:11,
   from ./include/linux/interrupt.h:11,
   from ./include/linux/serial_core.h:13,
   from ./include/linux/serial_sci.h:6,
   from arch/sh/kernel/cpu/sh2/setup-sh7619.c:11:
./include/linux/sh_intc.h:100:63: error: division 'sizeof (void *) / sizeof 
(void)' does not compute the number of array elements 
[-Werror=sizeof-pointer-div]
100 | #define _INTC_ARRAY(a) a, __same_type(a, NULL) ? 0 : 
sizeof(a)/sizeof(*a)
|   ^
./include/linux/sh_intc.h:105:31: note: in expansion of macro '_INTC_ARRAY'
105 | _INTC_ARRAY(vectors), _INTC_ARRAY(groups),  \
|   ^~~


The easiest fix for the latter is to disable CONFIG_WERROR.
Unfortunately I don't know a simple solution to get rid of the warning.


I did some research and it seems that what the macro _INT_ARRAY() does with 
"sizeof(a)/sizeof(*a)"
is a commonly used way to calculate array sizes and the kernel has even its own 
macro for that
called ARRAY_SIZE() which Linus asks people to use here [1].

So, I replaced _INTC_ARRAY() with ARRAY_SIZE() (see below), however the 
kernel's own ARRAY_SIZE()
macro triggers the same compiler warning. I'm CC'ing Michael Karcher who has 
more knowledge on
writing proper C code than me and maybe an idea how to fix this warning.

Thanks,
Adrian


[1] https://lkml.org/lkml/2015/9/3/428


diff --git a/include/linux/sh_intc.h b/include/linux/sh_intc.h
index c255273b0281..07a187686a84 100644
--- a/include/linux/sh_intc.h
+++ b/include/linux/sh_intc.h
@@ -97,14 +97,12 @@ struct intc_hw_desc {
unsigned int nr_subgroups;
 };
 
-#define _INTC_ARRAY(a) a, __same_type(a, NULL) ? 0 : sizeof(a)/sizeof(*a)

-
 #define INTC_HW_DESC(vectors, groups, mask_regs,   \
 prio_regs, sense_regs, ack_regs)   \
 {  \
-   _INTC_ARRAY(vectors), _INTC_ARRAY(groups),  \
-   _INTC_ARRAY(mask_regs), _INTC_ARRAY(prio_regs), \
-   _INTC_ARRAY(sense_regs), _INTC_ARRAY(ack_regs), \
+   ARRAY_SIZE(vectors), ARRAY_SIZE(groups),\
+   ARRAY_SIZE(mask_regs), ARRAY_SIZE(prio_regs),   \
+   ARRAY_SIZE(sense_regs), ARRAY_SIZE(ack_regs),   \
 }
 
 struct intc_desc {


--
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913



Re: Calculating array sizes in C - was: Re: Build regressions/improvements in v6.2-rc1

2023-01-17 Thread Geert Uytterhoeven
Hi Adrian,

On Tue, Jan 17, 2023 at 5:42 PM John Paul Adrian Glaubitz
 wrote:
> On 1/6/23 16:17, Geert Uytterhoeven wrote:
> >> I'm not seeing this one, but I am getting this one instead:
> >>
> >> In file included from ./arch/sh/include/asm/hw_irq.h:6,
> >>from ./include/linux/irq.h:596,
> >>from ./include/asm-generic/hardirq.h:17,
> >>from ./arch/sh/include/asm/hardirq.h:9,
> >>from ./include/linux/hardirq.h:11,
> >>from ./include/linux/interrupt.h:11,
> >>from ./include/linux/serial_core.h:13,
> >>from ./include/linux/serial_sci.h:6,
> >>from arch/sh/kernel/cpu/sh2/setup-sh7619.c:11:
> >> ./include/linux/sh_intc.h:100:63: error: division 'sizeof (void *) / 
> >> sizeof (void)' does not compute the number of array elements 
> >> [-Werror=sizeof-pointer-div]
> >> 100 | #define _INTC_ARRAY(a) a, __same_type(a, NULL) ? 0 : 
> >> sizeof(a)/sizeof(*a)
> >> |   ^
> >> ./include/linux/sh_intc.h:105:31: note: in expansion of macro '_INTC_ARRAY'
> >> 105 | _INTC_ARRAY(vectors), _INTC_ARRAY(groups),  \
> >> |   ^~~
> >
> > The easiest fix for the latter is to disable CONFIG_WERROR.
> > Unfortunately I don't know a simple solution to get rid of the warning.
>
> I did some research and it seems that what the macro _INT_ARRAY() does with 
> "sizeof(a)/sizeof(*a)"
> is a commonly used way to calculate array sizes and the kernel has even its 
> own macro for that
> called ARRAY_SIZE() which Linus asks people to use here [1].
>
> So, I replaced _INTC_ARRAY() with ARRAY_SIZE() (see below), however the 
> kernel's own ARRAY_SIZE()
> macro triggers the same compiler warning. I'm CC'ing Michael Karcher who has 
> more knowledge on
> writing proper C code than me and maybe an idea how to fix this warning.
>
> Thanks,
> Adrian
>
> > [1] https://lkml.org/lkml/2015/9/3/428
>
> diff --git a/include/linux/sh_intc.h b/include/linux/sh_intc.h
> index c255273b0281..07a187686a84 100644
> --- a/include/linux/sh_intc.h
> +++ b/include/linux/sh_intc.h
> @@ -97,14 +97,12 @@ struct intc_hw_desc {
>  unsigned int nr_subgroups;
>   };
>
> -#define _INTC_ARRAY(a) a, __same_type(a, NULL) ? 0 : sizeof(a)/sizeof(*a)
> -
>   #define INTC_HW_DESC(vectors, groups, mask_regs,   \
>   prio_regs, sense_regs, ack_regs)   \
>   {  \
> -   _INTC_ARRAY(vectors), _INTC_ARRAY(groups),  \
> -   _INTC_ARRAY(mask_regs), _INTC_ARRAY(prio_regs), \
> -   _INTC_ARRAY(sense_regs), _INTC_ARRAY(ack_regs), \
> +   ARRAY_SIZE(vectors), ARRAY_SIZE(groups),\
> +   ARRAY_SIZE(mask_regs), ARRAY_SIZE(prio_regs),   \
> +   ARRAY_SIZE(sense_regs), ARRAY_SIZE(ack_regs),   \
>   }

The issue is that some of the parameters are not arrays, but
NULL. E.g.:

arch/sh/kernel/cpu/sh2/setup-sh7619.c:static
DECLARE_INTC_DESC(intc_desc, "sh7619", vectors, NULL,
arch/sh/kernel/cpu/sh2/setup-sh7619.c-   NULL,
prio_registers, NULL);
--

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: Calculating array sizes in C - was: Re: Build regressions/improvements in v6.2-rc1

2023-01-17 Thread John Paul Adrian Glaubitz

Hi!

On 1/17/23 18:01, Geert Uytterhoeven wrote:

The issue is that some of the parameters are not arrays, but
NULL. E.g.:

arch/sh/kernel/cpu/sh2/setup-sh7619.c:static
DECLARE_INTC_DESC(intc_desc, "sh7619", vectors, NULL,
arch/sh/kernel/cpu/sh2/setup-sh7619.c-   NULL,
prio_registers, NULL);


Isn't this supposed to be caught by this check:

a, __same_type(a, NULL)

?

Adrian

--
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913



Re: [PATCH 3/3] drm/amdgpu/vcn: Add parameter to force (en/dis)abling indirect SRAM mode

2023-01-17 Thread Guilherme G. Piccoli
On 17/01/2023 13:24, Limonciello, Mario wrote:
> [...]
>>> Though I see two problems with that: first, I'm not sure what's the
>>> impact in the GPU functioning when I disable some IP block.
>>>
> 
> It depends on the individual block what the impact is.  For example
> if you don't have VCN, then you can't do any accelerated video playback.
> 
>>> Second, the parameter is a bit hard to figure - we need to clear a bit
>>> for the IP block we want to disable, and the doc suggest to read on
>>> dmesg to get this information (it seems it changes depending on the HW
>>> model), but I couldn't parse the proper bit from dmesg. Needed to
>>> instrument the kernel to find the proper bit heh
>>>
> 
> Isn't it this stuff (taken from a CZN system):
> 
> [7.797779] [drm] add ip block number 0 
> [7.797781] [drm] add ip block number 1 
> [7.797782] [drm] add ip block number 2 
> [7.797783] [drm] add ip block number 3 
> [7.797783] [drm] add ip block number 4 
> [7.797784] [drm] add ip block number 5 
> [7.797785] [drm] add ip block number 6 
> [7.797786] [drm] add ip block number 7 
> [7.797787] [drm] add ip block number 8 
> [7.797788] [drm] add ip block number 9 
> 
> So for that system it would be bit 8 to disable vcn.
> 
> In terms of how debugging would work:
> I would expect when you get your failure it will have been the previous
> block # that failed, and so you can reboot with that block masked and
> see if you get further.
> 

Thanks Mario, much appreciated! You're totally right and I messed up not
seeing these obvious messages...

So, I'll just drop the parameter on V2.
Cheers,


Guilherme


RE: [PATCH 3/3] drm/amdgpu/vcn: Add parameter to force (en/dis)abling indirect SRAM mode

2023-01-17 Thread Limonciello, Mario
[Public]



> -Original Message-
> From: Alex Deucher 
> Sent: Tuesday, January 17, 2023 09:11
> To: Guilherme G. Piccoli 
> Cc: Limonciello, Mario ; Liu, Leo
> ; amd-gfx@lists.freedesktop.org; Jiang, Sonny
> ; ker...@gpiccoli.net; Pan, Xinhui
> ; dri-de...@lists.freedesktop.org; Lazar, Lijo
> ; kernel-...@igalia.com; Deucher, Alexander
> ; Zhu, James ;
> Koenig, Christian ; Pierre-Loup Griffais
> 
> Subject: Re: [PATCH 3/3] drm/amdgpu/vcn: Add parameter to force
> (en/dis)abling indirect SRAM mode
> 
> On Tue, Jan 17, 2023 at 9:33 AM Guilherme G. Piccoli
>  wrote:
> >
> > On 16/01/2023 23:33, Limonciello, Mario wrote:
> > > [...]
> > >
> > > For debugging these type of problems, I think an effective debugging
> > > tactic would have been to mask the IP block (amdgpu.ip_block_mask).
> >
> > Thank you, it worked indeed - nice suggestion!
> >
> > Though I see two problems with that: first, I'm not sure what's the
> > impact in the GPU functioning when I disable some IP block.
> >

It depends on the individual block what the impact is.  For example
if you don't have VCN, then you can't do any accelerated video playback.

> > Second, the parameter is a bit hard to figure - we need to clear a bit
> > for the IP block we want to disable, and the doc suggest to read on
> > dmesg to get this information (it seems it changes depending on the HW
> > model), but I couldn't parse the proper bit from dmesg. Needed to
> > instrument the kernel to find the proper bit heh
> >

Isn't it this stuff (taken from a CZN system):

[7.797779] [drm] add ip block number 0 
[7.797781] [drm] add ip block number 1 
[7.797782] [drm] add ip block number 2 
[7.797783] [drm] add ip block number 3 
[7.797783] [drm] add ip block number 4 
[7.797784] [drm] add ip block number 5 
[7.797785] [drm] add ip block number 6 
[7.797786] [drm] add ip block number 7 
[7.797787] [drm] add ip block number 8 
[7.797788] [drm] add ip block number 9 

So for that system it would be bit 8 to disable vcn.

In terms of how debugging would work:
I would expect when you get your failure it will have been the previous
block # that failed, and so you can reboot with that block masked and
see if you get further.

> > The second part is easy to improve (we can just show this bit in
> > dmesg!), I might do that instead of proposing this parameter, which
> > seems didn't raise much excitement after all heheh
> >
> > Finally, I'm still curious on why Deck was working fine with the
> > indirect SRAM mode disabled (by mistake) in many kernels - was it in
> > practice the same as disabling the VCN IP block?
> 
> IIRC, it depends on the fuse recipe for the particular ASIC.
> 
> Alex
> 
> 
> >
> > Thanks,
> >
> >
> > Guilherme
> >


Re: [PATCH 3/3] drm/amdgpu/vcn: Add parameter to force (en/dis)abling indirect SRAM mode

2023-01-17 Thread Alex Deucher
On Tue, Jan 17, 2023 at 9:33 AM Guilherme G. Piccoli
 wrote:
>
> On 16/01/2023 23:33, Limonciello, Mario wrote:
> > [...]
> >
> > For debugging these type of problems, I think an effective debugging
> > tactic would have been to mask the IP block (amdgpu.ip_block_mask).
>
> Thank you, it worked indeed - nice suggestion!
>
> Though I see two problems with that: first, I'm not sure what's the
> impact in the GPU functioning when I disable some IP block.
>
> Second, the parameter is a bit hard to figure - we need to clear a bit
> for the IP block we want to disable, and the doc suggest to read on
> dmesg to get this information (it seems it changes depending on the HW
> model), but I couldn't parse the proper bit from dmesg. Needed to
> instrument the kernel to find the proper bit heh
>
> The second part is easy to improve (we can just show this bit in
> dmesg!), I might do that instead of proposing this parameter, which
> seems didn't raise much excitement after all heheh
>
> Finally, I'm still curious on why Deck was working fine with the
> indirect SRAM mode disabled (by mistake) in many kernels - was it in
> practice the same as disabling the VCN IP block?

IIRC, it depends on the fuse recipe for the particular ASIC.

Alex


>
> Thanks,
>
>
> Guilherme
>


Re: [PATCH 3/3] drm/amdgpu/vcn: Add parameter to force (en/dis)abling indirect SRAM mode

2023-01-17 Thread Guilherme G. Piccoli
On 16/01/2023 23:33, Limonciello, Mario wrote:
> [...]
> 
> For debugging these type of problems, I think an effective debugging 
> tactic would have been to mask the IP block (amdgpu.ip_block_mask).

Thank you, it worked indeed - nice suggestion!

Though I see two problems with that: first, I'm not sure what's the
impact in the GPU functioning when I disable some IP block.

Second, the parameter is a bit hard to figure - we need to clear a bit
for the IP block we want to disable, and the doc suggest to read on
dmesg to get this information (it seems it changes depending on the HW
model), but I couldn't parse the proper bit from dmesg. Needed to
instrument the kernel to find the proper bit heh

The second part is easy to improve (we can just show this bit in
dmesg!), I might do that instead of proposing this parameter, which
seems didn't raise much excitement after all heheh

Finally, I'm still curious on why Deck was working fine with the
indirect SRAM mode disabled (by mistake) in many kernels - was it in
practice the same as disabling the VCN IP block?

Thanks,


Guilherme



Re: [PATCH v3 2/3] drm/amdgpu: Remove redundant framebuffer format check

2023-01-17 Thread Maíra Canal

Hi Simon,

On 1/13/23 14:06, Simon Ser wrote:

Hm, unfortunately I think we need to keep the check in amdgpu for the
same reason as i915: amdgpu will pick a modifier if user-space didn't
supply one on GFX9+.

I wonder if that also applies to vmwgfx? Maybe that would be a reason
to have the check in framebuffer_init()? (Not sure!)


I tried to move the check to framebuffer_init(), but it ended up causing
problems in the i915 driver (the kernel was emitting warnings when running
the IGT tests). I was thinking of going back to the drm_gem_fb_create()
approach [1], as it would make the other drivers return EINVAL in the case of
a bad modifier and it wouldn't change the current behavior of i915 and amdgpu.

[1] 
https://lore.kernel.org/dri-devel/20230103125322.855089-1-mca...@igalia.com/T/

Best Regards,
- Maíra Canal


Re: [PATCH] drm/amdgpu/smu: skip pptable init under sriov

2023-01-17 Thread Alex Deucher
On Mon, Jan 16, 2023 at 10:05 PM Jane Jian  wrote:
>
> sriov does not need to init pptable from amdgpu driver
> we finish it from PF
>
> Signed-off-by: Jane Jian 

Acked-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c 
> b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> index d0cdc578344d..7d711861b90e 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> @@ -407,6 +407,9 @@ static int smu_v13_0_0_setup_pptable(struct smu_context 
> *smu)
> struct amdgpu_device *adev = smu->adev;
> int ret = 0;
>
> +   if (amdgpu_sriov_vf(smu->adev))
> +   return 0;
> +
> ret = smu_v13_0_0_get_pptable_from_pmfw(smu,
> _table->power_play_table,
> 
> _table->power_play_table_size);
> @@ -1257,6 +1260,9 @@ static int 
> smu_v13_0_0_get_thermal_temperature_range(struct smu_context *smu,
> table_context->power_play_table;
> PPTable_t *pptable = smu->smu_table.driver_pptable;
>
> +   if (amdgpu_sriov_vf(smu->adev))
> +   return 0;
> +
> if (!range)
> return -EINVAL;
>
> --
> 2.17.1
>


Re: [PATCH 2/2] drm/amdgpu: simplify amdgpu_uvd_send_msg

2023-01-17 Thread Alex Deucher
Series is:
Reviewed-by: Alex Deucher 

On Tue, Jan 17, 2023 at 3:12 AM Christian König
 wrote:
>
> We only need one offset and not an array of it.
>
> Signed-off-by: Christian König 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 21 +
>  1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> index dd0894c9740d..229419c0c031 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> @@ -1118,29 +1118,26 @@ static int amdgpu_uvd_send_msg(struct amdgpu_ring 
> *ring, struct amdgpu_bo *bo,
>  {
> struct amdgpu_device *adev = ring->adev;
> struct dma_fence *f = NULL;
> +   uint32_t offset, data[4];
> struct amdgpu_job *job;
> struct amdgpu_ib *ib;
> -   uint32_t data[4];
> uint64_t addr;
> int i, r;
> -   unsigned offset_idx = 0;
> -   unsigned offset[3] = { UVD_BASE_SI, 0, 0 };
>
> r = amdgpu_job_alloc_with_ib(adev, 64, direct ? AMDGPU_IB_POOL_DIRECT 
> :
>  AMDGPU_IB_POOL_DELAYED, );
> if (r)
> return r;
>
> -   if (adev->asic_type >= CHIP_VEGA10) {
> -   offset_idx = 1 + ring->me;
> -   offset[1] = adev->reg_offset[UVD_HWIP][0][1];
> -   offset[2] = adev->reg_offset[UVD_HWIP][1][1];
> -   }
> +   if (adev->asic_type >= CHIP_VEGA10)
> +   offset = adev->reg_offset[UVD_HWIP][ring->me][1];
> +   else
> +   offset = UVD_BASE_SI;
>
> -   data[0] = PACKET0(offset[offset_idx] + UVD_GPCOM_VCPU_DATA0, 0);
> -   data[1] = PACKET0(offset[offset_idx] + UVD_GPCOM_VCPU_DATA1, 0);
> -   data[2] = PACKET0(offset[offset_idx] + UVD_GPCOM_VCPU_CMD, 0);
> -   data[3] = PACKET0(offset[offset_idx] + UVD_NO_OP, 0);
> +   data[0] = PACKET0(offset + UVD_GPCOM_VCPU_DATA0, 0);
> +   data[1] = PACKET0(offset + UVD_GPCOM_VCPU_DATA1, 0);
> +   data[2] = PACKET0(offset + UVD_GPCOM_VCPU_CMD, 0);
> +   data[3] = PACKET0(offset + UVD_NO_OP, 0);
>
> ib = >ibs[0];
> addr = amdgpu_bo_gpu_offset(bo);
> --
> 2.34.1
>


Re: [PATCH v2 20/20] jump_label: RFC - tolerate toggled state

2023-01-17 Thread Peter Zijlstra
On Fri, Jan 13, 2023 at 12:30:16PM -0700, Jim Cromie wrote:
> __jump_label_patch currently will "crash the box" if it finds a
> jump_entry not as expected.  ISTM this overly harsh; it doesn't
> distinguish between "alternate/opposite" state, and truly
> "insane/corrupted".
> 
> The "opposite" (but well-formed) state is a milder mis-initialization
> problem, and some less severe mitigation seems practical.  ATM this
> just warns about it; a range/enum of outcomes: warn, crash, silence,
> ok, fixup-continue, etc, are possible on a case-by-case basis.
> 
> Ive managed to create this mis-initialization condition with
> test_dynamic_debug.ko & _submod.ko.  These replicate DRM's regression
> on DRM_USE_DYNAMIC_DEBUG=y; drm.debug callsites in drivers/helpers
> (dependent modules) are not enabled along with those in drm.ko itself.
> 

> Ive hit this case a few times, but havent been able to isolate the
> when and why.
> 
> warn-only is something of a punt, and I'm still left with remaining
> bugs which are likely related; I'm able to toggle the p-flag on
> callsites in the submod, but their enablement still doesn't yield
> logging activity.

Right; having been in this is state is bad since it will generate
inconsistent code-flow. Full on panic *might* not be warranted (as it
does for corrupted text) but it is still a fairly bad situation -- so
I'm not convinced we want to warn and carry on.

It would be really good to figure out why the site was skipped over and
got out of skew.

Given it's all module stuff, the 'obvious' case would be something like
a race between adding the new sites and flipping it, but I'm not seeing
how -- things are rather crudely serialized by jump_label_mutex.

The only other option I can come up with is that somehow the update
condition in jump_label_add_module() is somehow wrong.



Re: [PATCH v2 3/3] drm: Call vga_switcheroo_process_delayed_switch() in drm_lastclose

2023-01-17 Thread Thomas Zimmermann

Hi

Am 17.01.23 um 10:44 schrieb Lukas Wunner:

On Thu, Jan 12, 2023 at 09:11:56PM +0100, Thomas Zimmermann wrote:

Several lastclose helpers call vga_switcheroo_process_delayed_switch().
It's better to call the helper from drm_lastclose() after the kernel
client's screen has been restored. This way, all drivers can benefit
without having to implement their own lastclose helper. For drivers
without vga-switcheroo, vga_switcheroo_process_delayed_switch() does
nothing.

[...]

--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -38,6 +38,7 @@
  #include 
  #include 
  #include 
+#include 
  
  #include 

  #include 
@@ -460,6 +461,8 @@ void drm_lastclose(struct drm_device * dev)
drm_legacy_dev_reinit(dev);
  
  	drm_client_dev_restore(dev);

+
+   vga_switcheroo_process_delayed_switch();
  }


Hm, this looks like a case of midlayer fallacy:

https://blog.ffwll.ch/2016/12/midlayers-once-more-with-feeling.html

It is a departure from the opt-in library approach we've had so far.

For switcheroo-aware EDID retrieval, there's a drm_get_edid_switcheroo()
helper.  How about introducing a switcheroo-aware lastclose helper which
drivers can reference?


Console emulation is a mess. I'm working on it, but it takes time. 
Therefore a nice solution is probably not possible ATM.


We could have something like drm_fb_helper_lastclose_switcheroo(), which 
does drm_fb_helper_lastclose() + 
vga_switcheroo_process_delayed_switch(). i915 and radeon could use that 
as-is. amdgpu and nouveau have already switched to generic fbdev 
emulation. We could use drm_fb_helper_lastclose_switcheroo() from the 
generic code as well, but it will still be somewhat midlayerish.


But with all that, it's probably better to not land patch 3 at all. The 
hook drm_driver.lastclose is just for the old fbdev emulation. For the 
new fbdev emulation, we have better data structures (see drm_client_dev).


The correct design would be to switch amdgpu and nouveau back to their 
own fbdev emulation and handle switcheroo there. But their old fbdev 
code needed an overhaul, so just reverting isn't an option.


Best regards
Thomas



Thanks,

Lukas


--
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 v2 3/3] drm: Call vga_switcheroo_process_delayed_switch() in drm_lastclose

2023-01-17 Thread Lukas Wunner
On Thu, Jan 12, 2023 at 09:11:56PM +0100, Thomas Zimmermann wrote:
> Several lastclose helpers call vga_switcheroo_process_delayed_switch().
> It's better to call the helper from drm_lastclose() after the kernel
> client's screen has been restored. This way, all drivers can benefit
> without having to implement their own lastclose helper. For drivers
> without vga-switcheroo, vga_switcheroo_process_delayed_switch() does
> nothing.
[...]
> --- a/drivers/gpu/drm/drm_file.c
> +++ b/drivers/gpu/drm/drm_file.c
> @@ -38,6 +38,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -460,6 +461,8 @@ void drm_lastclose(struct drm_device * dev)
>   drm_legacy_dev_reinit(dev);
>  
>   drm_client_dev_restore(dev);
> +
> + vga_switcheroo_process_delayed_switch();
>  }

Hm, this looks like a case of midlayer fallacy:

https://blog.ffwll.ch/2016/12/midlayers-once-more-with-feeling.html

It is a departure from the opt-in library approach we've had so far.

For switcheroo-aware EDID retrieval, there's a drm_get_edid_switcheroo()
helper.  How about introducing a switcheroo-aware lastclose helper which
drivers can reference?

Thanks,

Lukas


Re: [PATCH 00/22] drm: Remove includes for drm_crtc_helper.h

2023-01-17 Thread Thomas Zimmermann

Hi

Am 16.01.23 um 21:47 schrieb Sam Ravnborg:

Hi Thomas.

On Mon, Jan 16, 2023 at 02:12:13PM +0100, Thomas Zimmermann wrote:

A lot of source files include drm_crtc_helper.h for its contained
include statements. This leads to excessive compile-time dependencies.

Where possible, remove the include statements for drm_crtc_helper.h
and include the required source files directly. Also remove the
include statements from drm_crtc_helper.h itself, which doesn't need
most of them.

With this patchset drm_crtc_helper usage is reduced from 85 places to 35
places. And the 35 places is only .c files.
This is a very nice reduction of bloat! I hope this has a measureable
effect on building times.


I cannot say what the effect is for a single header, but if we do this 
for all header files, the effect is measureable.




I was working on something similar, but that approach only added missing
includes, and did not kill all the unnessesary includes - which I think
is the biggest win here.

All patches are:
Reviewed-by: Sam Ravnborg 


Thank you.



For a few of them the r-b is conditional, see the specific comments
posted.


I fixed all your comments, including the minor complaint about the 
commit messages.


I don't intent to send out a v2 here, but the fixed patches should soon 
show up in drm-misc-next.


Best regards
Thomas




I did a build check here with the archs and config I verifies with.
This covers "alpha arm arm64 sparc64 i386 x86 powerpc s390 riscv sh"
and everything was fine. I have a few specific configs to pull in
drivers that need a bit extra to be built.
So I consider build coverage OK for applying, but it would be nice to
wait a few days for the bots to verify too.

My own work on slimming drm_atomic_helper.h and drm_print.h will be
rebased on top of your work before I continue it.
I need to look into removing unused includes too.

Sam



I built this patchset on x86-64, aarch64 and arm. Hopefully I found
all include dependencies.

Thanks to Sam Ravnborg for bringing this to my attention.

Thomas Zimmermann (22):
   drm/amdgpu: Fix coding style
   drm: Remove unnecessary include statements for drm_crtc_helper.h
   drm/amdgpu: Remove unnecessary include statements for
 drm_crtc_helper.h
   drm/arm/komeda: Remove unnecessary include statements for
 drm_crtc_helper.h
   drm/aspeed: Remove unnecessary include statements for
 drm_crtc_helper.h
   drm/ast: Remove unnecessary include statements for drm_crtc_helper.h
   drm/bridge: Remove unnecessary include statements for
 drm_crtc_helper.h
   drm/gma500: Remove unnecessary include statements for
 drm_crtc_helper.h
   drm/i2c/ch7006: Remove unnecessary include statements for
 drm_crtc_helper.h
   drm/ingenic: Remove unnecessary include statements for
 drm_crtc_helper.h
   drm/kmb: Remove unnecessary include statements for drm_crtc_helper.h
   drm/logicvc: Remove unnecessary include statements for
 drm_crtc_helper.h
   drm/nouveau: Remove unnecessary include statements for
 drm_crtc_helper.h
   drm/radeon: Remove unnecessary include statements for
 drm_crtc_helper.h
   drm/rockchip: Remove unnecessary include statements for
 drm_crtc_helper.h
   drm/shmobile: Remove unnecessary include statements for
 drm_crtc_helper.h
   drm/sprd: Remove unnecessary include statements for drm_crtc_helper.h
   drm/sun4i: Remove unnecessary include statements for drm_crtc_helper.h
   drm/tidss: Remove unnecessary include statements for drm_crtc_helper.h
   drm/udl: Remove unnecessary include statements for drm_crtc_helper.h
   drm/vboxvideo: Remove unnecessary include statements for
 drm_crtc_helper.h
   drm/crtc-helper: Remove most include statements from drm_crtc_helper.h

  drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c |  2 ++
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  5 +++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c|  1 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c   |  1 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c|  1 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h   |  1 -
  drivers/gpu/drm/amd/amdgpu/atombios_crtc.c |  1 -
  drivers/gpu/drm/amd/amdgpu/atombios_encoders.c |  1 -
  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c |  2 ++
  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c |  2 ++
  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c  |  2 ++
  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c  |  2 ++
  drivers/gpu/drm/arm/display/komeda/komeda_crtc.c   |  1 -
  drivers/gpu/drm/arm/display/komeda/komeda_kms.h|  1 -
  drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c   |  1 -
  drivers/gpu/drm/aspeed/aspeed_gfx_drv.c|  1 -
  drivers/gpu/drm/aspeed/aspeed_gfx_out.c|  1 -
  drivers/gpu/drm/ast/ast_drv.c  |  1 -
  drivers/gpu/drm/ast/ast_main.c |  1 -
  drivers/gpu/drm/ast/ast_mode.c |  1 -
  drivers/gpu/drm/bridge/analogix/analogix-anx6345.c 

Re: [PATCH 00/22] drm: Remove includes for drm_crtc_helper.h

2023-01-17 Thread Thomas Zimmermann

Hi

Am 16.01.23 um 19:37 schrieb Alex Deucher:

On Mon, Jan 16, 2023 at 11:20 AM Jani Nikula
 wrote:


On Mon, 16 Jan 2023, Thomas Zimmermann  wrote:

A lot of source files include drm_crtc_helper.h for its contained
include statements. This leads to excessive compile-time dependencies.

Where possible, remove the include statements for drm_crtc_helper.h
and include the required source files directly. Also remove the
include statements from drm_crtc_helper.h itself, which doesn't need
most of them.

I built this patchset on x86-64, aarch64 and arm. Hopefully I found
all include dependencies.


I think this is the right direction and I support this. Personally I
think it's enough to build test and fix any fallout afterwards.

To that end, I did build test this myself with my config, and it was
fine, though that probably doesn't add much coverage.

FWIW,

Acked-by: Jani Nikula 


Agreed.  I applied 1/22 since it was an unrelated cleanup, but the
rest of the series is:
Acked-by: Alex Deucher 


Thanks to both of you. I'll leave out the first patch when merging the 
series.


Best regards
Thomas






--
Jani Nikula, Intel Open Source Graphics Center


--
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


[PATCH 2/3] drm/amd/display: Remove the unused variable value0

2023-01-17 Thread Jiapeng Chong
Variable value0 is not effectively used, so delete it.

drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_link_encoder.c:1222:11: 
warning: variable ‘value0’ set but not used.

Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=3783
Reported-by: Abaci Robot 
Signed-off-by: Jiapeng Chong 
---
 .../gpu/drm/amd/display/dc/dcn10/dcn10_link_encoder.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_link_encoder.c 
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_link_encoder.c
index c4287147b853..4c067f8a19b4 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_link_encoder.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_link_encoder.c
@@ -1219,7 +1219,6 @@ void 
dcn10_link_encoder_update_mst_stream_allocation_table(
const struct link_mst_stream_allocation_table *table)
 {
struct dcn10_link_encoder *enc10 = TO_DCN10_LINK_ENC(enc);
-   uint32_t value0 = 0;
uint32_t value1 = 0;
uint32_t value2 = 0;
uint32_t slots = 0;
@@ -1321,13 +1320,9 @@ void 
dcn10_link_encoder_update_mst_stream_allocation_table(
do {
udelay(10);
 
-   value0 = REG_READ(DP_MSE_SAT_UPDATE);
-
-   REG_GET(DP_MSE_SAT_UPDATE,
-   DP_MSE_SAT_UPDATE, );
-
-   REG_GET(DP_MSE_SAT_UPDATE,
-   DP_MSE_16_MTP_KEEPOUT, );
+   REG_READ(DP_MSE_SAT_UPDATE);
+   REG_GET(DP_MSE_SAT_UPDATE, DP_MSE_SAT_UPDATE, );
+   REG_GET(DP_MSE_SAT_UPDATE, DP_MSE_16_MTP_KEEPOUT, );
 
/* bit field DP_MSE_SAT_UPDATE is set to 1 already */
if (!value1 && !value2)
-- 
2.20.1.7.g153144c



Re: [PATCH 1/6] drm/amdgpu: Generalize KFD dmabuf import

2023-01-17 Thread Dmitry Osipenko
16.01.2023 18:11, Christian König пишет:
> 
>
>> mmapping the memory with that new offset should still work. The
>> imported BO is created with ttm_bo_type_sg, and AFAICT ttm_bo_vm.c
>> supports mapping of SG BOs.
>
> Actually it shouldn't. This can go boom really easily.

 OK. I don't think we're doing this, but after Xiaogang raised the
 question I went looking through the code whether it's theoretically
 possible. I didn't find anything in the code that says that mmapping
 imported dmabufs would be prohibited or even dangerous. On the
 contrary, I found that ttm_bo_vm explicitly supports mmapping SG BOs.


>
> When you have imported a BO the only correct way of to mmap() it is
> to do so on the original exporter.

 That seems sensible, and this is what we do today. That said, if
 mmapping an imported BO is dangerous, I'm missing a mechanism to
 protect against this. It could be as simple as setting
 AMDGPU_GEM_CREATE_NO_CPU_ACCESS in amdgpu_dma_buf_create_obj.
>>>
>>> At least for the GEM mmap() handler this is double checked very early
>>> by looking at obj->import_attach and then either rejecting it or
>>> redirecting the request to the DMA-buf file instead.
>>
>> Can you point me at where this check is? I see a check for
>> obj->import_attach in drm_gem_dumb_map_offset. But I can't see how
>> this function is called in amdgpu. I don't think it is used at all.
> 
> Uff, good question! @Thomas and @Dmitry: I clearly remember that one of
> you guys was involved in the DRM/GEM mmap cleanup and DMA-buf with
> workarounds for the KFD and DMA-buf.
> 
> What was the final solution to this? I can't find it of hand any more.

I was looking at it. The AMDGPU indeed allows to map imported GEMs, but
then touching the mapped area by CPU results in a bus fault. You,
Christian, suggested that this an AMDGPU bug that should be fixed by
prohibiting the mapping in the first place and I was going to fix it,
but then the plan changed from prohibiting the mapping into fixing it.

The first proposal was to make DRM core to handle the dma-buf mappings
for all drivers universally [1]. Then we decided that will be better to
prohibit mapping of imported GEMs [2]. In the end, Rob Clark argued that
better to implement the [1], otherwise current userspace (Android) will
be broken if mapping will be prohibited.

The last question was about the cache syncing of imported dma-bufs, how
to ensure that drivers will do the cache maintenance/syncing properly.
Rob suggested that it should be a problem for drivers and not for DRM core.

I was going to re-send the [1], but other things were getting priority.
It's good that you reminded me about it :) I may re-send it sometime
soon if there are no new objections.

[1] https://patchwork.freedesktop.org/patch/487481/

[2]
https://lore.kernel.org/all/20220701090240.1896131-1-dmitry.osipe...@collabora.com/



[PATCH 3/3] drm/amd/display: Clean up some inconsistent indenting

2023-01-17 Thread Jiapeng Chong
No functional modification involved.

drivers/gpu/drm/amd/amdgpu/../display/dc/link/link_hpd.c:140 get_hpd_line() 
warn: inconsistent indenting.

Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=3787
Reported-by: Abaci Robot 
Signed-off-by: Jiapeng Chong 
---
 drivers/gpu/drm/amd/display/dc/link/link_hpd.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/link/link_hpd.c 
b/drivers/gpu/drm/amd/display/dc/link/link_hpd.c
index 5f39dfe06e9a..4746ea623354 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_hpd.c
+++ b/drivers/gpu/drm/amd/display/dc/link/link_hpd.c
@@ -137,10 +137,9 @@ enum hpd_source_id get_hpd_line(struct dc_link *link)
struct gpio *hpd;
enum hpd_source_id hpd_id;
 
-   hpd_id = HPD_SOURCEID_UNKNOWN;
-
+   hpd_id = HPD_SOURCEID_UNKNOWN;
hpd = link_get_hpd_gpio(link->ctx->dc_bios, link->link_id,
-  link->ctx->gpio_service);
+   link->ctx->gpio_service);
 
if (hpd) {
switch (dal_irq_get_source(hpd)) {
-- 
2.20.1.7.g153144c



[PATCH 1/3] drm/amd/display: Remove the unused variable optc

2023-01-17 Thread Jiapeng Chong
Variable optc is not effectively used, so delete it.

drivers/gpu/drm/amd/amdgpu/../display/dc/dcn30/dcn30_hwseq.c:325:27: warning: 
variable ‘optc’ set but not used.

Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=3782
Reported-by: Abaci Robot 
Signed-off-by: Jiapeng Chong 
---
 drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c 
b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
index 7360b3ce4283..bae6e3b52546 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
@@ -322,13 +322,11 @@ void dcn30_enable_writeback(
 {
struct dwbc *dwb;
struct mcif_wb *mcif_wb;
-   struct timing_generator *optc;
 
dwb = dc->res_pool->dwbc[wb_info->dwb_pipe_inst];
mcif_wb = dc->res_pool->mcif_wb[wb_info->dwb_pipe_inst];
 
/* set the OPTC source mux */
-   optc = dc->res_pool->timing_generators[dwb->otg_inst];
DC_LOG_DWB("%s dwb_pipe_inst = %d, mpcc_inst = %d",\
__func__, wb_info->dwb_pipe_inst,\
wb_info->mpcc_inst);
-- 
2.20.1.7.g153144c



Re: [PATCH 3/3] drm/amdgpu/vcn: Add parameter to force (en/dis)abling indirect SRAM mode

2023-01-17 Thread Guilherme G. Piccoli
On 16/01/2023 20:00, Alex Deucher wrote:
> [...]
> 
> It's not clear to me when this would be used.  We only disable it
> briefly during new asic bring up, after that we never touch it again.
> No end user on production hardware should ever have to change it and
> doing so would break VCN on their system.
> 
> Alex

[+ Pierre-Loup]

Steam Deck is facing a pretty weird scenario then heheh

Commit 82132ecc543 ("drm/amdgpu: enable Vangogh VCN indirect sram mode")
corrected a long-term issue in which the indirect SRAM mode wasn't
enabled for Vangogh - and Deck GPU architecture is Vangogh, so it was
working perfectly with that disabled.

Happens that a bad FW candidate seems to have broken something - it was
a bit convoluted to debug, but we proved that disabling indirect SRAM is
a good workaround so far, it "restored the behavior" pre-82132ecc543.

Hence my proposal - this parameter would've made life so much easier,
and we're start using it "downstream" now. While I understand that of
course the FW should be fixed, meanwhile this is a cheap solution to
allow further debug and real use of the system.

Thanks,


Guilherme


[PATCH 2/2] drm/amdgpu: simplify amdgpu_uvd_send_msg

2023-01-17 Thread Christian König
We only need one offset and not an array of it.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 21 +
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
index dd0894c9740d..229419c0c031 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
@@ -1118,29 +1118,26 @@ static int amdgpu_uvd_send_msg(struct amdgpu_ring 
*ring, struct amdgpu_bo *bo,
 {
struct amdgpu_device *adev = ring->adev;
struct dma_fence *f = NULL;
+   uint32_t offset, data[4];
struct amdgpu_job *job;
struct amdgpu_ib *ib;
-   uint32_t data[4];
uint64_t addr;
int i, r;
-   unsigned offset_idx = 0;
-   unsigned offset[3] = { UVD_BASE_SI, 0, 0 };
 
r = amdgpu_job_alloc_with_ib(adev, 64, direct ? AMDGPU_IB_POOL_DIRECT :
 AMDGPU_IB_POOL_DELAYED, );
if (r)
return r;
 
-   if (adev->asic_type >= CHIP_VEGA10) {
-   offset_idx = 1 + ring->me;
-   offset[1] = adev->reg_offset[UVD_HWIP][0][1];
-   offset[2] = adev->reg_offset[UVD_HWIP][1][1];
-   }
+   if (adev->asic_type >= CHIP_VEGA10)
+   offset = adev->reg_offset[UVD_HWIP][ring->me][1];
+   else
+   offset = UVD_BASE_SI;
 
-   data[0] = PACKET0(offset[offset_idx] + UVD_GPCOM_VCPU_DATA0, 0);
-   data[1] = PACKET0(offset[offset_idx] + UVD_GPCOM_VCPU_DATA1, 0);
-   data[2] = PACKET0(offset[offset_idx] + UVD_GPCOM_VCPU_CMD, 0);
-   data[3] = PACKET0(offset[offset_idx] + UVD_NO_OP, 0);
+   data[0] = PACKET0(offset + UVD_GPCOM_VCPU_DATA0, 0);
+   data[1] = PACKET0(offset + UVD_GPCOM_VCPU_DATA1, 0);
+   data[2] = PACKET0(offset + UVD_GPCOM_VCPU_CMD, 0);
+   data[3] = PACKET0(offset + UVD_NO_OP, 0);
 
ib = >ibs[0];
addr = amdgpu_bo_gpu_offset(bo);
-- 
2.34.1



[PATCH 1/2] drm/amdgpu: stop waiting in amdgpu_uvd_send_msg

2023-01-17 Thread Christian König
We have a wait in the amdgpu_bo_kmap() code for quite a while now, so
waiting here isn't needed any more.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 17 +
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
index b67a5fb2ff3e..dd0894c9740d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
@@ -1122,8 +1122,7 @@ static int amdgpu_uvd_send_msg(struct amdgpu_ring *ring, 
struct amdgpu_bo *bo,
struct amdgpu_ib *ib;
uint32_t data[4];
uint64_t addr;
-   long r;
-   int i;
+   int i, r;
unsigned offset_idx = 0;
unsigned offset[3] = { UVD_BASE_SI, 0, 0 };
 
@@ -1158,24 +1157,10 @@ static int amdgpu_uvd_send_msg(struct amdgpu_ring 
*ring, struct amdgpu_bo *bo,
ib->length_dw = 16;
 
if (direct) {
-   r = dma_resv_wait_timeout(bo->tbo.base.resv,
- DMA_RESV_USAGE_KERNEL, false,
- msecs_to_jiffies(10));
-   if (r == 0)
-   r = -ETIMEDOUT;
-   if (r < 0)
-   goto err_free;
-
r = amdgpu_job_submit_direct(job, ring, );
if (r)
goto err_free;
} else {
-   r = amdgpu_sync_resv(adev, >sync, bo->tbo.base.resv,
-AMDGPU_SYNC_ALWAYS,
-AMDGPU_FENCE_OWNER_UNDEFINED);
-   if (r)
-   goto err_free;
-
r = amdgpu_job_submit(job, >uvd.entity,
  AMDGPU_FENCE_OWNER_UNDEFINED, );
if (r)
-- 
2.34.1