Re: [PATCH] drm/amd/display: remove unreachable code
Hi, On 12/08/2022 00:19, Jiapeng Chong wrote: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_util_32.c:1658 dml32_TruncToValidBPP() warn: ignoring unreachable code. Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=1894 Reported-by: Abaci Robot Signed-off-by: Jiapeng Chong --- .../drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c index 05fc14a47fba..0758e1da55a9 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c @@ -1654,10 +1654,6 @@ double dml32_TruncToValidBPP( else return DesiredBPP; } - - *RequiredSlots = dml_ceil(DesiredBPP / MaxLinkBPP * 64, 1); - - return BPP_INVALID; } // TruncToValidBPP double dml32_RequiredDTBCLK( Seems correct. Reviewed-by: Tales Aparecida I feel like RequiredSlots is not actually used anywhere in the code, just passed around dml32_TruncToValidBPP() and dml32_CalculateOutputLink(). I've looked for any mentions of it in the mailing list, but could not find anything that implied it's part of ground working. I wonder if it's something outside the Linux tree for other platforms or related to HW gospel.
Re: [PATCH 2/8] drm/amd/display: Introduce KUnit tests to the bw_fixed library
On 11/08/2022 04:34, David Gow wrote: On Thu, Aug 11, 2022 at 8:40 AM Tales Aparecida wrote: From: Maíra Canal KUnit unifies the test structure and provides helper tools that simplify the development of tests. Basic use case allows running tests as regular processes, which makes easier to run unit tests on a development machine and to integrate the tests in a CI system. This commit introduces a unit test to the bw_fixed library, which performs a lot of the mathematical operations involving fixed-point arithmetic and the conversion of integers to fixed-point representation inside the Display Mode Library. As fixed-point representation is the base foundation of the DML calcs operations, this unit tests intend to assure the proper functioning of the basic mathematical operations of fixed-point arithmetic, such as multiplication, conversion from fractional to fixed-point number, and more. You can run it with: ./tools/testing/kunit/kunit.py run \ --arch=x86_64 \ --kunitconfig=drivers/gpu/drm/amd/display/tests/ Signed-off-by: Maíra Canal Co-developed-by: Magali Lemes Signed-off-by: Magali Lemes Co-developed-by: Tales Aparecida Signed-off-by: Tales Aparecida --- Not directly related to this patch, but I get a whole stack of warnings about the definition of MIN_I64 causing integer overflow: ../drivers/gpu/drm/amd/amdgpu/../display/dc/dml/calcs/../../../tests/dc/dml/calcs/bw_fixed_test.c:214:31: note: in expansion of macro ‘MIN_I64’ 214 | KUNIT_EXPECT_EQ(test, MIN_I64 + 1, res.value); | ^~~ ../drivers/gpu/drm/amd/amdgpu/../display/dc/dml/calcs/bw_fixed.c:30:19: warning: integer overflow in expression ‘-9223372036854775808’ of type ‘long long int’ results in ‘-9223372036854775808’ [-Woverflow] 30 | (int64_t)(-(1LL << 63)) | ^ This seems to fix it (I'll re-send it out as a separate patch so gmail doesn't mangle it once I'm a bit more convinced it's the best implementation): Thanks! We were aware of this warnings, in fact there was a patch fixing this that got dropped in the last minute to expedite its review as a standalone patch, I'm sorry I didn't mention it in the cover letter. To make amends I've sent your approach to the mailing list, hope you don't mind! https://lore.kernel.org/all/20220811204327.411709-1-tales.aparec...@gmail.com/ --- 8< --- From 84e84664873dc9e98dff5ee9f74d95872e6cd423 Mon Sep 17 00:00:00 2001 From: David Gow Date: Thu, 11 Aug 2022 15:21:02 +0800 Subject: [PATCH] drm/amd/display: MIN_I64 definition causes overflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The definition of MIN_I64 in bw_fixed.c can cause gcc to whinge about integer overflow, because it is treated as a positive value, which is then negated. The temporary postive value is not necessarily representable. This causes the following warning: ../drivers/gpu/drm/amd/amdgpu/../display/dc/dml/calcs/bw_fixed.c:30:19: warning: integer overflow in expression ‘-9223372036854775808’ of type ‘long long int’ results in ‘-9223372036854775808’ [-Woverflow] 30 | (int64_t)(-(1LL << 63)) | ^ Writing out (INT_MIN - 1) - 1 works instead. Signed-off-by: David Gow --- drivers/gpu/drm/amd/display/dc/dml/calcs/bw_fixed.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml/calcs/bw_fixed.c b/drivers/gpu/drm/amd/display/dc/dml/calcs/bw_fixed.c index fbe8d0661396..3850f7f0f679 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/calcs/bw_fixed.c +++ b/drivers/gpu/drm/amd/display/dc/dml/calcs/bw_fixed.c @@ -26,12 +26,12 @@ #include "bw_fixed.h" -#define MIN_I64 \ - (int64_t)(-(1LL << 63)) - #define MAX_I64 \ (int64_t)((1ULL << 63) - 1) +#define MIN_I64 \ + (-MAX_I64 - 1) + #define FRACTIONAL_PART_MASK \ ((1ULL << BW_FIXED_BITS_PER_FRACTIONAL_PART) - 1) -- 2.37.1.595.g718a3a8f04-goog --- 8< --- Otherwise, this test seems to okay. I'll review it (and the series) more properly over then next few days. Cheers, -- David Thanks for reviewing, Tales
Re: [PATCH 4/4] Documentation/gpu/amdgpu/amdgpu_dm: add DM docs for pixel blend mode
On 16/07/2022 19:25, Melissa Wen wrote: AMD GPU display manager (DM) maps DRM pixel blend modes (None, Pre-multiplied, Coverage) to MPC hw blocks through blend configuration options. Describe relevant elements and how to set and test them to get the expected DRM blend mode on DCN hw. Signed-off-by: Melissa Wen --- .../gpu/amdgpu/display/display-manager.rst| 98 +++ Documentation/gpu/drm-kms.rst | 2 + 2 files changed, 100 insertions(+) diff --git a/Documentation/gpu/amdgpu/display/display-manager.rst b/Documentation/gpu/amdgpu/display/display-manager.rst index 8960a5f1fa66..7a495ed1f69e 100644 --- a/Documentation/gpu/amdgpu/display/display-manager.rst +++ b/Documentation/gpu/amdgpu/display/display-manager.rst @@ -84,3 +84,101 @@ families below. **DCN 3.0 family color caps and mapping** .. kernel-figure:: dcn3_cm_drm_current.svg + +Blend Mode Properties += + +Pixel blend mode is a DRM plane composition property of :c:type:`drm_plane` used to +describes how pixels from a foreground plane (fg) are composited with the +background plane (bg). Here, we present main concepts of DRM blend mode to help +to understand how this property is mapped to AMD DC interface. See more about +this DRM property and the alpha blending equations in :ref:`DRM Plane +Composition Properties `. + +Basically, a blend mode sets the alpha blending equation for plane +composition that fits the mode in which the alpha channel affects the state of +pixel color values and, therefore, the resulted pixel color. For +example, consider the following elements of the alpha blending equation: + +- *fg.rgb*: Each of the RGB component values from the foreground's pixel. +- *fg.alpha*: Alpha component value from the foreground's pixel. +- *bg.rgb*: Each of the RGB component values from the background. +- *plane_alpha*: Plane alpha value set by the **plane "alpha" property**, see + more in `DRM Plane Composition Properties `. You forgot to use :ref: in here. + +in the basic alpha blending equation:: + + out.rgb = alpha * fg.rgb + (1 - alpha) * bg.rgb + +the alpha channel value of each pixel in a plane is ignored and only the plane +alpha affects the resulted pixel color values. + +DRM has three blend mode to define the blend formula in the plane composition: + +* **None**: Blend formula that ignores the pixel alpha. + +* **Pre-multiplied**: Blend formula that assumes the pixel color values in a + plane was already pre-multiplied by its own alpha channel before storage. + +* **Coverage**: Blend formula that assumes the pixel color values were not + pre-multiplied with the alpha channel values. + +and pre-multiplied is the default pixel blend mode, that means, when no blend +mode property is created or defined, DRM considers the plane's pixels has +pre-multiplied color values. On IGT GPU tools, the kms_plane_alpha_blend test +provides a set of subtests to verify plane alpha and blend mode properties. + +The DRM blend mode and its elements are then mapped by AMDGPU display manager +(DM) to program the blending configuration of the Multiple Pipe/Plane Combined +(MPC), as follows: + +.. kernel-doc:: drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h + :doc: mpc-overview + +.. kernel-doc:: drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h + :functions: mpcc_blnd_cfg + +Therefore, the blending configuration for a single MPCC instance on the MPC +tree is defined by :c:type:`mpcc_blnd_cfg`, where +:c:type:`pre_multiplied_alpha` is the alpha pre-multiplied mode flag used to +set :c:type:`MPCC_ALPHA_MULTIPLIED_MODE`. It controls whether alpha is +multiplied (true/false), being only true for DRM pre-multiplied blend mode. +:c:type:`mpcc_alpha_blend_mode` defines the alpha blend mode regarding pixel +alpha and plane alpha values. It sets one of the three modes for +:c:type:`MPCC_ALPHA_BLND_MODE`, as described below. + +.. kernel-doc:: drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h + :functions: mpcc_alpha_blend_mode + +DM then maps the elements of `enum mpcc_alpha_blend_mode` to those in the DRM +blend formula, as follows: + +* *MPC pixel alpha* matches *DRM fg.alpha* as the alpha component value + from the plane's pixel +* *MPC global alpha* matches *DRM plane_alpha* when the pixel alpha should + be ignored and, therefore, pixel values are not pre-multiplied +* *MPC global gain* assumes *MPC global alpha* value when both *DRM + fg.alpha* and *DRM plane_alpha* participate in the blend equation + +In short, *fg.alpha* is ignored by selecting +:c:type:`MPCC_ALPHA_BLEND_MODE_GLOBAL_ALPHA`. On the other hand, (plane_alpha * +fg.alpha) component becomes available by selecting +:c:type:`MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN`. And the +:c:type:`MPCC_ALPHA_MULTIPLIED_MODE` defines if the pixel color values are +pre-multiplied by alpha or not. + +Blend configuration flow + + +The alpha blending equation is configured from DRM to DC interface by the
Re: [PATCH 3/4] drm/amd/display: add doc entries for MPC blending configuration
On 16/07/2022 19:25, Melissa Wen wrote:
Describe structs and enums used to set blend mode properties to MPC
blocks. Some pieces of information are already available as code
comments, and were just formatted. Others were collected and summarised
from discusssions on AMD issue tracker[1][2].
Typo in the commit message: discusssions -> discussions
[1] https://gitlab.freedesktop.org/drm/amd/-/issues/1734
[2] https://gitlab.freedesktop.org/drm/amd/-/issues/1769
Signed-off-by: Melissa Wen
---
drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h | 91 +
1 file changed, 77 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
b/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
index 5097037e3962..cf28b841c42d 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
@@ -22,6 +22,16 @@
*
*/
+/**
+ * DOC: mpc-overview
+ *
+ * Multiple Pipe/Plane Combined (MPC) is a component in the hardware pipeline
+ * that performs blending of multiple planes, using global and per-pixel alpha.
+ * It also performs post-blending color correction operations according to the
+ * hardware capabilities, such as color transformation matrix and gamma 1D and
+ * 3D LUT.
+ */
+
#ifndef __DC_MPCC_H__
#define __DC_MPCC_H__
@@ -48,14 +58,39 @@ enum mpcc_blend_mode {
MPCC_BLEND_MODE_TOP_BOT_BLENDING
};
+/**
+ * enum mpcc_alpha_blend_mode - define the alpha blend mode regarding pixel
+ * alpha and plane alpha values
+ */
enum mpcc_alpha_blend_mode {
+ /**
+* @MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA: per pixel alpha using DPP
+* alpha value
+*/
MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA,
+ /**
+* @MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN: per
+* pixel alpha using DPP alpha value multiplied by a global gain (plane
+* alpha)
+*/
MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN,
+ /**
+* @MPCC_ALPHA_BLEND_MODE_GLOBAL_ALPHA: global alpha value, ignores
+* pixel alpha and consider only plane alpha
+*/
MPCC_ALPHA_BLEND_MODE_GLOBAL_ALPHA
};
-/*
- * MPCC blending configuration
+/**
+ * struct mpcc_blnd_cfg - MPCC blending configuration
+ *
+ * @black_color: background color
+ * @alpha_mode: alpha blend mode (MPCC_ALPHA_BLND_MODE)
+ * @pre_multiplied_alpha: whether pixel color values were pre-multiplied by the
+ * alpha channel (MPCC_ALPHA_MULTIPLIED_MODE)
+ * @global_gain: used when blend mode considers both pixel alpha and plane
+ * alpha value and assumes the global alpha value.
+ * @global_alpha: plane alpha value
There's quite a few members missing definition. After reading the 4th
patch may I conclude that they weren't relevant for what's being
described about alpha blending?
./drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h:109: warning: Function
parameter or member 'overlap_only' not described in 'mpcc_blnd_cfg'
./drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h:109: warning: Function
parameter or member 'bottom_gain_mode' not described in 'mpcc_blnd_cfg'
./drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h:109: warning: Function
parameter or member 'background_color_bpc' not described in 'mpcc_blnd_cfg'
./drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h:109: warning: Function
parameter or member 'top_gain' not described in 'mpcc_blnd_cfg'
./drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h:109: warning: Function
parameter or member 'bottom_inside_gain' not described in 'mpcc_blnd_cfg'
./drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h:109: warning: Function
parameter or member 'bottom_outside_gain' not described in 'mpcc_blnd_cfg'
*/
struct mpcc_blnd_cfg {
struct tg_color black_color;/* background color */
@@ -107,8 +142,15 @@ struct mpc_dwb_flow_control {
int flow_ctrl_cnt1;
};
-/*
- * MPCC connection and blending configuration for a single MPCC instance.
+/**
+ * struct mpcc - MPCC connection and blending configuration for a single MPCC
instance.
Might be worth writing the definition of the abbreviation, if not here,
in the glossary... I couldn't find what the last "C" stands for, my
guess would be "context". hehehe
+ * @mpcc_id: MPCC physical instance
+ * @dpp_id: DPP input to this MPCC
+ * @mpcc_bot: pointer to bottom layer MPCC. NULL when not connected.
+ * @blnd_cfg: the blending configuration for this MPCC
+ * @sm_cfg: stereo mix setting for this MPCC
+ * @shared_bottom: if MPCC output to both OPP and DWB endpoints, true.
Othewise, false.
Typo Othewise -> Otherwise
+ *
* This struct is used as a node in an MPC tree.
*/
struct mpcc {
@@ -120,8 +162,12 @@ struct mpcc {
bool shared_bottom; /* TRUE if MPCC output to both OPP and
DWB endpoints, else FALSE */
};
-/*
- * MPC tree represents all MPCC connections for a pipe.
+/**
+ * struct mpc_tree - MPC tree represents all MPCC connections for a pipe.
+
Re: [PATCH 2/4] Documentation/amdgpu/display: add DC color caps info
On 16/07/2022 19:25, Melissa Wen wrote: Add details about color correction capabilities and explain a bit about differences between DC hw generations and also how they are mapped between DRM and DC interface. Two schemas for DCN 2.0 and 3.0 (converted to svg from the original png) is included to illustrate it. They were obtained from a discussion[1] in the amd-gfx mailing list. [1] https://lore.kernel.org/amd-gfx/20220422142811.dm6vtk6v64jcw...@mail.igalia.com/ v2: - remove redundant comments (Harry) - fix typo (Harry) Signed-off-by: Melissa Wen --- .../amdgpu/display/dcn2_cm_drm_current.svg| 1370 +++ .../amdgpu/display/dcn3_cm_drm_current.svg| 1529 + .../gpu/amdgpu/display/display-manager.rst| 35 + drivers/gpu/drm/amd/display/dc/dc.h | 74 +- 4 files changed, 2995 insertions(+), 13 deletions(-) create mode 100644 Documentation/gpu/amdgpu/display/dcn2_cm_drm_current.svg create mode 100644 Documentation/gpu/amdgpu/display/dcn3_cm_drm_current.svg diff --git a/Documentation/gpu/amdgpu/display/dcn2_cm_drm_current.svg b/Documentation/gpu/amdgpu/display/dcn2_cm_drm_current.svg new file mode 100644 index ..315ffc5a1a4b --- /dev/null +++ b/Documentation/gpu/amdgpu/display/dcn2_cm_drm_current.svg @@ -0,0 +1,1370 @@ + + + +http://www.inkscape.org/namespaces/inkscape; + xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd; + xmlns="http://www.w3.org/2000/svg; + xmlns:svg="http://www.w3.org/2000/svg;> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +Matrix +1D LUT +3D LUT +Unpacking +Other +drm_framebuffer +format +drm_plane +drm_crtc +Stream +MPC +DPP + +Blender +Degamma +CTM +Gamma +format +bias_and_scale +color space matrix +input_csc_color_matrix +in_transfer_func +hdr_mult +gamut_remap_matrix +in_shaper_func +lut3d_func +blend_tf +Blender +gamut_remap_matrix +func_shaper +lut3d_func +out_transfer_func +csc_color_matrix +bit_depth_param +clamping +output_color_space +Plane +Legend +DCN 2.0 +DC Interface +DRM Interface + +CNVC +Input CSC +DeGammaRAM and ROM(sRGB, BT2020 +HDR Multiply +Gamut Remap +Shaper LUTRAM +3D LUTRAM +Blend Gamma +Blender +GammaRAM +OCSC + + +color_encoding + +pixel_blend_mode + +color_range + + + + + + + + + + + + + + diff --git a/Documentation/gpu/amdgpu/display/dcn3_cm_drm_current.svg b/Documentation/gpu/amdgpu/display/dcn3_cm_drm_current.svg new file mode 100644 index ..7299ee9b6d64 --- /dev/null +++ b/Documentation/gpu/amdgpu/display/dcn3_cm_drm_current.svg @@ -0,0 +1,1529 @@ + + + +http://www.inkscape.org/namespaces/inkscape; + xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd; + xmlns="http://www.w3.org/2000/svg; + xmlns:svg="http://www.w3.org/2000/svg;> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +Matrix +1D LUT +3D LUT +Unpacking +Other +drm_framebuffer +format +drm_plane +drm_crtc +Stream +MPC +DPP + +Blender +Degamma +CTM +Gamma +format +bias_and_scale +color space matrix +input_csc_color_matrix +in_transfer_func +hdr_mult +gamut_remap_matrix +in_shaper_func +lut3d_func +blend_tf +Blender +gamut_remap_matrix +func_shaper +lut3d_func +out_transfer_func +csc_color_matrix +bit_depth_param +clamping +output_color_space +Plane +Legend +DCN 3.0 +DC Interface +DRM Interface + +CNVC +Input CSC +DeGammaROM(sRGB, BT2020, Gamma 2.2,PQ, HLG) +Post CSC +Gamma Correction +HDR Multiply +Gamut Remap +Shaper LUTRAM +3D LUTRAM +Blend Gamma +Blender +Gamut Remap +Shaper LUTRAM +3D LUTRAM +GammaRAM +
Re: [PATCH 1/4] Documentation/amdgpu_dm: Add DM color correction documentation
On 16/07/2022 19:25, Melissa Wen wrote:
AMDGPU DM maps DRM color management properties (degamma, ctm and gamma)
to DC color correction entities. Part of this mapping is already
documented as code comments and can be converted as kernel docs.
v2:
- rebase to amd-staging-drm-next
Reviewed-by: Harry Wentland
Signed-off-by: Melissa Wen
---
.../gpu/amdgpu/display/display-manager.rst| 9 ++
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 121 +-
2 files changed, 98 insertions(+), 32 deletions(-)
diff --git a/Documentation/gpu/amdgpu/display/display-manager.rst
b/Documentation/gpu/amdgpu/display/display-manager.rst
index 7ce31f89d9a0..b1b0f11aed83 100644
--- a/Documentation/gpu/amdgpu/display/display-manager.rst
+++ b/Documentation/gpu/amdgpu/display/display-manager.rst
@@ -40,3 +40,12 @@ Atomic Implementation
.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
:functions: amdgpu_dm_atomic_check amdgpu_dm_atomic_commit_tail
+
+Color Management Properties
+===
+
+.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
+ :doc: overview
+
+.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
+ :internal:
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
index a71177305bcd..93c813089bff 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
@@ -29,7 +29,9 @@
#include "modules/color/color_gamma.h"
#include "basics/conversion.h"
-/*
+/**
+ * DOC: overview
+ *
* The DC interface to HW gives us the following color management blocks
* per pipe (surface):
*
@@ -71,8 +73,8 @@
#define MAX_DRM_LUT_VALUE 0x
-/*
- * Initialize the color module.
+/**
+ * amdgpu_dm_init_color_mod - Initialize the color module.
*
* We're not using the full color module, only certain components.
* Only call setup functions for components that we need.
@@ -82,7 +84,14 @@ void amdgpu_dm_init_color_mod(void)
setup_x_points_distribution();
}
-/* Extracts the DRM lut and lut size from a blob. */
+/**
+ * __extract_blob_lut - Extracts the DRM lut and lut size from a blob.
+ * @blob: DRM color mgmt property blob
+ * @size: lut size
+ *
+ * Returns:
+ * DRM LUT or NULL
+ */
static const struct drm_color_lut *
__extract_blob_lut(const struct drm_property_blob *blob, uint32_t *size)
{
@@ -90,13 +99,18 @@ __extract_blob_lut(const struct drm_property_blob *blob,
uint32_t *size)
return blob ? (struct drm_color_lut *)blob->data : NULL;
}
I don't think everyone would approve using actual kernel-doc for these
static functions, but I can appreciate they being formatted as such.
Consider replacing /** with /*.
-/*
- * Return true if the given lut is a linear mapping of values, i.e. it acts
- * like a bypass LUT.
+/**
+ * __is_lut_linear - check if the given lut is a linear mapping of values
+ * @lut: given lut to check values
+ * @size: lut size
*
* It is considered linear if the lut represents:
- * f(a) = (0xFF00/MAX_COLOR_LUT_ENTRIES-1)a; for integer a in
- * [0, MAX_COLOR_LUT_ENTRIES)
+ * f(a) = (0xFF00/MAX_COLOR_LUT_ENTRIES-1)a; for integer a in [0,
+ * MAX_COLOR_LUT_ENTRIES)
+ *
+ * Returns:
+ * True if the given lut is a linear mapping of values, i.e. it acts like a
+ * bypass LUT. Otherwise, false.
*/
static bool __is_lut_linear(const struct drm_color_lut *lut, uint32_t size)
{
@@ -119,9 +133,13 @@ static bool __is_lut_linear(const struct drm_color_lut
*lut, uint32_t size)
return true;
}
-/*
- * Convert the drm_color_lut to dc_gamma. The conversion depends on the size
- * of the lut - whether or not it's legacy.
+/**
+ * __drm_lut_to_dc_gamma - convert the drm_color_lut to dc_gamma.
+ * @lut: DRM lookup table for color conversion
+ * @gamma: DC gamma to set entries
+ * @is_legacy: legacy or atomic gamma
+ *
+ * The conversion depends on the size of the lut - whether or not it's legacy.
*/
static void __drm_lut_to_dc_gamma(const struct drm_color_lut *lut,
struct dc_gamma *gamma, bool is_legacy)
@@ -154,8 +172,11 @@ static void __drm_lut_to_dc_gamma(const struct
drm_color_lut *lut,
}
}
-/*
- * Converts a DRM CTM to a DC CSC float matrix.
+/**
+ * __drm_ctm_to_dc_matrix - converts a DRM CTM to a DC CSC float matrix
+ * @ctm: DRM color transformation matrix
+ * @matrix: DC CSC float matrix
+ *
* The matrix needs to be a 3x4 (12 entry) matrix.
*/
static void __drm_ctm_to_dc_matrix(const struct drm_color_ctm *ctm,
@@ -189,7 +210,18 @@ static void __drm_ctm_to_dc_matrix(const struct
drm_color_ctm *ctm,
}
}
-/* Calculates the legacy transfer function - only for sRGB input space. */
+/**
+ * __set_legacy_tf - Calculates the legacy transfer function
+ * @func: transfer function
Re: [PATCH] Revert "drm/amdgpu: remove ctx->lock"
On 21/06/2022 11:42, Luben Tuikov wrote:
This reverts commit e68efb27647f2106d6b545667f35b2ea39746b57.
We see that the bo validate list gets corrupted, in
amdgpu_cs_list_validate(), the lobj->tv.bo is NULL. Then getting usermm on
the next line, references a NULL bo and we get a koops.
Bisecting leads to the commit being reverted as the cause of the list
corruption. See the link below for details of the corruption failure.
Cc: Christian König
Cc: Andrey Grodzovsky
Cc: Alex Deucher
Cc: Vitaly Prosyak
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2048#note_1427539
Signed-off-by: Luben Tuikov
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 16 +---
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 2 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 1 +
3 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 36ac1f1d11e6b4..e619031753b927 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -128,6 +128,8 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser
*p, union drm_amdgpu_cs
goto free_chunk;
}
+ mutex_lock(>ctx->lock);
+
/* skip guilty context job */
if (atomic_read(>ctx->guilty) == 1) {
ret = -ECANCELED;
@@ -585,16 +587,6 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
}
}
- /* Move fence waiting after getting reservation lock of
-* PD root. Then there is no need on a ctx mutex lock.
-*/
- r = amdgpu_ctx_wait_prev_fence(p->ctx, p->entity);
- if (unlikely(r != 0)) {
- if (r != -ERESTARTSYS)
- DRM_ERROR("amdgpu_ctx_wait_prev_fence failed.\n");
- goto error_validate;
- }
-
amdgpu_cs_get_threshold_for_moves(p->adev, >bytes_moved_threshold,
>bytes_moved_vis_threshold);
p->bytes_moved = 0;
@@ -722,6 +714,7 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser
*parser, int error,
dma_fence_put(parser->fence);
if (parser->ctx) {
+ mutex_unlock(>ctx->lock);
amdgpu_ctx_put(parser->ctx);
}
if (parser->bo_list)
@@ -965,7 +958,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev,
if (parser->job->uf_addr && ring->funcs->no_user_fence)
return -EINVAL;
- return 0;
+ return amdgpu_ctx_wait_prev_fence(parser->ctx, parser->entity);
}
static int amdgpu_cs_process_fence_dep(struct amdgpu_cs_parser *p,
@@ -1384,6 +1377,7 @@ int amdgpu_cs_ioctl(struct drm_device *dev, void *data,
struct drm_file *filp)
goto out;
r = amdgpu_cs_submit(, cs);
+
out:
amdgpu_cs_parser_fini(, r, reserved_buffers);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 53f9268366f29e..2ef5296216d64d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -286,6 +286,7 @@ static int amdgpu_ctx_init(struct amdgpu_ctx_mgr *mgr,
int32_t priority,
kref_init(>refcount);
ctx->mgr = mgr;
spin_lock_init(>ring_lock);
+ mutex_init(>lock);
ctx->reset_counter = atomic_read(>adev->gpu_reset_counter);
ctx->reset_counter_query = ctx->reset_counter;
@@ -400,6 +401,7 @@ static void amdgpu_ctx_fini(struct kref *ref)
drm_dev_exit(idx);
}
+ mutex_destroy(>lock);
kfree(ctx);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
index 0fa0e56daf67e9..cc7c8afff4144c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
@@ -53,6 +53,7 @@ struct amdgpu_ctx {
boolpreamble_presented;
int32_t init_priority;
int32_t override_priority;
+ struct mutexlock;
atomic_tguilty;
unsigned long ras_counter_ce;
unsigned long ras_counter_ue;
base-commit: f4b3c543a2a759ce657de4b6b7e88eaddee85ec2
Hello,
Applied on amd-staging-drm-next (with two additional commits from
torvalds/master to allow me to compile using GCC12: 52a9dab6, 82880283)
and tested with IGT using a RX580*; the errors on the "amd_cs_nop" tests
are gone!
* Advanced Micro Devices, Inc. [AMD/ATI] Ellesmere [Radeon RX
470/480/570/570X/580/580X/590] (rev e7)
The remaining IGT tests matching ".*amdgpu.*" were not affected. (FYI,
amdgpu/amd_plane, amdgpu/amd_bypass, amdgpu/amd_plane,
amdgpu/amd_vrr_range are failing in here, some should skip instead, but
that's another thread to come)
Tested-by: Tales Aparecida
Thanks for your time,
Tales
[PATCH 2/2] MAINTAINERS: add docs entry to AMDGPU
To make sure maintainers of amdgpu drivers are aware of any changes in their documentation, add its entry to MAINTAINERS. Signed-off-by: Tales Lelo da Aparecida --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index d54b9f15ffce..b3594b2a09de 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -16449,6 +16449,7 @@ S: Supported T: git https://gitlab.freedesktop.org/agd5f/linux.git B: https://gitlab.freedesktop.org/drm/amd/-/issues C: irc://irc.oftc.net/radeon +F: Documentation/gpu/amdgpu/ F: drivers/gpu/drm/amd/ F: drivers/gpu/drm/radeon/ F: include/uapi/drm/amdgpu_drm.h -- 2.35.1
[PATCH 1/2] Documentation/gpu: Add entries to amdgpu glossary
Add missing acronyms to the amdgppu glossary. Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/1939#note_1309737 Signed-off-by: Tales Lelo da Aparecida --- Documentation/gpu/amdgpu/amdgpu-glossary.rst | 13 + 1 file changed, 13 insertions(+) diff --git a/Documentation/gpu/amdgpu/amdgpu-glossary.rst b/Documentation/gpu/amdgpu/amdgpu-glossary.rst index 859dcec6c6f9..48829d097f40 100644 --- a/Documentation/gpu/amdgpu/amdgpu-glossary.rst +++ b/Documentation/gpu/amdgpu/amdgpu-glossary.rst @@ -8,12 +8,19 @@ we have a dedicated glossary for Display Core at .. glossary:: +active_cu_number + The number of CUs that are active on the system. The number of active + CUs may be less than SE * SH * CU depending on the board configuration. + CP Command Processor CPLIB Content Protection Library +CU + Compute unit + DFS Digital Frequency Synthesizer @@ -74,6 +81,12 @@ we have a dedicated glossary for Display Core at SDMA System DMA +SE + Shader Engine + +SH + SHader array + SMU System Management Unit -- 2.35.1
[PATCH 0/2] Update AMDGPU glossary and MAINTAINERS
I was handling the request from [0] and then I noticed that some AMD developers were missing from get_maintainers output due to the lack of a reference to their documentation in the MAINTAINERS file. [0] https://gitlab.freedesktop.org/drm/amd/-/issues/1939#note_1309737 Tales Lelo da Aparecida (2): Documentation/gpu: Add entries to amdgpu glossary MAINTAINERS: add docs entry to AMDGPU Documentation/gpu/amdgpu/amdgpu-glossary.rst | 13 + MAINTAINERS | 1 + 2 files changed, 14 insertions(+) -- 2.35.1
[PATCH] drm/amd/display: make hubp1_wait_pipe_read_start() static
It's a local function, let's make it static.
Signed-off-by: Tales Lelo da Aparecida
---
drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubp.c
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubp.c
index fbff6beb78be..3a7f76e2c598 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubp.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubp.c
@@ -1316,7 +1316,7 @@ void hubp1_set_flip_int(struct hubp *hubp)
*
* @hubp: hubp struct reference.
*/
-void hubp1_wait_pipe_read_start(struct hubp *hubp)
+static void hubp1_wait_pipe_read_start(struct hubp *hubp)
{
struct dcn10_hubp *hubp1 = TO_DCN10_HUBP(hubp);
--
2.35.1
