Re: [PATCH 3/4] drm/amd/display: add doc entries for MPC blending configuration

2022-07-20 Thread Melissa Wen
On 07/17, Tales Lelo da Aparecida wrote:
> 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?

Hi Tales,

although they aren't changed for DRM blend modes programming, it would
be nice if someone can describe them and also avoid those warnings. I
wasn't able to identify how they behave for MPC programming (hope
someone from AMD can help on documenting them).

> 
> ./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 c

Re: [PATCH 3/4] drm/amd/display: add doc entries for MPC blending configuration

2022-07-17 Thread Tales Lelo da Aparecida

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.
+ *

[PATCH 3/4] drm/amd/display: add doc entries for MPC blending configuration

2022-07-16 Thread Melissa Wen
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].

[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
  */
 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.
+ * @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.
+ *
  * 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.
+ *
+ * @opp_id: the OPP instance that owns this MPC tree
+ * @opp_list: the top MPCC layer of the MPC tree that outputs to OPP endpoint
+ *
  */
 struct mpc_tree {
int opp_id; /* The OPP instance that owns this MPC 
tree */
@@ -149,13 +195,18 @@ struct mpcc_state {
uint32_t busy;
 };
 
+/**
+ * struct mpc_funcs - funcs
+ */
 struct mpc_funcs {
void (*read_mpcc_state)(
struct mpc *mpc,
int mpcc_inst,
struct mpcc_state *s);
 
-   /*
+   /**
+* @insert_plane:
+*
 * Insert DPP into MPC tree based on specified blending position.
 * Only used for planes that are part of blending chain for OPP output
 *
@@ -180,7 +231,9 @@ struct mpc_funcs {
int dpp_id,
int mpcc_id);
 
-   /*
+   /**
+* @remove_mpcc:
+*
 * Remove a specified MPCC from the MPC tree.
 *
 * Parameters:
@@ -195,7 +248,9 @@ struct mpc_funcs {
struct mpc_tree *tree,
struct mpcc *mpcc);
 
-   /*
+   /**
+* @mpc_init:
+*
 * Reset the MPCC HW status by disconnecting all muxes.
 *
 * Parameters:
@@ -208,7 +263,9 @@ struct mpc_funcs {