Re: [PATCH] drm/dsc: Add kernel documentation for DRM DP DSC helpers

2019-02-05 Thread Sean Paul
On Mon, Feb 04, 2019 at 03:40:22PM -0800, Manasi Navare wrote:
> This patch adds appropiate kernel documentation for DRM DP helpers
> used for enabling Display Stream compression functionality in
> drm_dp_helper.h and drm_dp_helper.c as well as for the DSC spec
> related structure definitions and helpers in drm_dsc.c and drm_dsc.h
> Also add links between the functions and structures in the documentation.
> 
> Suggested-by: Daniel Vetter 
> Suggested-by: Sean Paul 
> Cc: Daniel Vetter 
> Cc: Sean Paul 
> Signed-off-by: Manasi Navare 

Build warnings are gone with this, so once Daniel's suggestions are
incorporated:

Acked-by: Sean Paul 

> ---
>  drivers/gpu/drm/drm_dp_helper.c |  42 +-
>  drivers/gpu/drm/drm_dsc.c   |  13 ++-
>  include/drm/drm_dp_helper.h |  15 +++-
>  include/drm/drm_dsc.h   | 138 ++--
>  4 files changed, 142 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 54120b6319e7..e9e0233f5b2f 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1360,7 +1360,20 @@ int drm_dp_read_desc(struct drm_dp_aux *aux, struct 
> drm_dp_desc *desc,
>  EXPORT_SYMBOL(drm_dp_read_desc);
>  
>  /**
> - * DRM DP Helpers for DSC
> + * drm_dp_dsc_sink_max_slice_count() - Get the max slice count
> + * supported by the DSC sink.
> + * @dsc_dpcd: DSC capabilities from DPCD
> + * @is_edp: true if its eDP, false for DP
> + *
> + * Read the slice capabilities DPCD register from DSC sink to get
> + * the maximum slice count supported. This is used to populate
> + * the DSC parameters in the  drm_dsc_config by the driver.
> + * Driver creates an infoframe using these parameters to populate
> + *  drm_dsc_pps_infoframe. These are sent to the sink using DSC
> + * infoframe using the helper function drm_dsc_pps_infoframe_pack()
> + *
> + * Returns:
> + * Maximum slice count supported by DSC sink or 0 its invalid
>   */
>  u8 drm_dp_dsc_sink_max_slice_count(const u8 
> dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
>  bool is_edp)
> @@ -1405,6 +1418,19 @@ u8 drm_dp_dsc_sink_max_slice_count(const u8 
> dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
>  }
>  EXPORT_SYMBOL(drm_dp_dsc_sink_max_slice_count);
>  
> +/**
> + * drm_dp_dsc_sink_line_buf_depth() - Get the line buffer depth in bits 
> which is
> + * number of bits of precision within the decoder line buffer supported by
> + * the DSC sink. This is used to populate the DSC parameters in the
> + *  drm_dsc_config by the driver.
> + * Driver creates an infoframe using these parameters to populate
> + *  drm_dsc_pps_infoframe. These are sent to the sink using DSC
> + * infoframe using the helper function drm_dsc_pps_infoframe_pack()
> + * @dsc_dpcd: DSC capabilities from DPCD
> + *
> + * Returns:
> + * Line buffer depth supported by DSC panel or 0 its invalid
> + */
>  u8 drm_dp_dsc_sink_line_buf_depth(const u8 
> dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
>  {
>   u8 line_buf_depth = dsc_dpcd[DP_DSC_LINE_BUF_BIT_DEPTH - 
> DP_DSC_SUPPORT];
> @@ -1434,6 +1460,20 @@ u8 drm_dp_dsc_sink_line_buf_depth(const u8 
> dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
>  }
>  EXPORT_SYMBOL(drm_dp_dsc_sink_line_buf_depth);
>  
> +/**
> + * drm_dp_dsc_sink_supported_input_bpcs() - Get all the input bits per 
> component
> + * values supported by the DSC sink. This is used to populate the DSC 
> parameters
> + * in the  drm_dsc_config by the driver.
> + * Driver creates an infoframe using these parameters to populate
> + *  drm_dsc_pps_infoframe. These are sent to the sink using DSC
> + * infoframe using the helper function drm_dsc_pps_infoframe_pack()
> + * @dsc_dpcd: DSC capabilities from DPCD
> + * @dsc_bpc: An array to be filled by this helper with supported
> + *   input bpcs.
> + *
> + * Returns:
> + * Number of input BPC values parsed from the DPCD
> + */
>  int drm_dp_dsc_sink_supported_input_bpcs(const u8 
> dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
>u8 dsc_bpc[3])
>  {
> diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
> index bc2b23adb072..0fd56fbdf9b4 100644
> --- a/drivers/gpu/drm/drm_dsc.c
> +++ b/drivers/gpu/drm/drm_dsc.c
> @@ -17,6 +17,12 @@
>  /**
>   * DOC: dsc helpers
>   *
> + * VESA specification for DP 1.4 adds a new feature called Display Stream
> + * Compression (DSC) used to compress the pixel bits before sending it on
> + * DP/eDP/MIPI DSI interface. DSC is required to be enabled so that the 
> existing
> + * display interfaces can support high resolutions at higher frames rates 
> uisng
> + * the maximum available link capacity of these interfaces.
> + *
>   * These functions contain some common logic and helpers to deal with VESA
>   * Display Stream Compression standard required for DSC on Display Port/eDP 
> or
>   * MIPI display interfaces.
> @@ -26,6 +32,7 @@
>   * drm_dsc_dp_pps_header_init() - Initializes the PPS Header
>   * for 

Re: [PATCH] drm/dsc: Add kernel documentation for DRM DP DSC helpers

2019-02-05 Thread Manasi Navare
On Tue, Feb 05, 2019 at 10:55:12AM +0100, Daniel Vetter wrote:
> On Mon, Feb 04, 2019 at 03:40:22PM -0800, Manasi Navare wrote:
> > This patch adds appropiate kernel documentation for DRM DP helpers
> > used for enabling Display Stream compression functionality in
> > drm_dp_helper.h and drm_dp_helper.c as well as for the DSC spec
> > related structure definitions and helpers in drm_dsc.c and drm_dsc.h
> > Also add links between the functions and structures in the documentation.
> > 
> > Suggested-by: Daniel Vetter 
> > Suggested-by: Sean Paul 
> > Cc: Daniel Vetter 
> > Cc: Sean Paul 
> > Signed-off-by: Manasi Navare 
> 
> Awesome, more docs!
> 
> > ---
> >  drivers/gpu/drm/drm_dp_helper.c |  42 +-
> >  drivers/gpu/drm/drm_dsc.c   |  13 ++-
> >  include/drm/drm_dp_helper.h |  15 +++-
> >  include/drm/drm_dsc.h   | 138 ++--
> >  4 files changed, 142 insertions(+), 66 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_helper.c 
> > b/drivers/gpu/drm/drm_dp_helper.c
> > index 54120b6319e7..e9e0233f5b2f 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -1360,7 +1360,20 @@ int drm_dp_read_desc(struct drm_dp_aux *aux, struct 
> > drm_dp_desc *desc,
> >  EXPORT_SYMBOL(drm_dp_read_desc);
> >  
> >  /**
> > - * DRM DP Helpers for DSC
> > + * drm_dp_dsc_sink_max_slice_count() - Get the max slice count
> > + * supported by the DSC sink.
> > + * @dsc_dpcd: DSC capabilities from DPCD
> > + * @is_edp: true if its eDP, false for DP
> > + *
> > + * Read the slice capabilities DPCD register from DSC sink to get
> > + * the maximum slice count supported. This is used to populate
> > + * the DSC parameters in the  drm_dsc_config by the driver.
> > + * Driver creates an infoframe using these parameters to populate
> > + *  drm_dsc_pps_infoframe. These are sent to the sink using DSC
> > + * infoframe using the helper function drm_dsc_pps_infoframe_pack()
> > + *
> > + * Returns:
> > + * Maximum slice count supported by DSC sink or 0 its invalid
> >   */
> >  u8 drm_dp_dsc_sink_max_slice_count(const u8 
> > dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
> >bool is_edp)
> > @@ -1405,6 +1418,19 @@ u8 drm_dp_dsc_sink_max_slice_count(const u8 
> > dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
> >  }
> >  EXPORT_SYMBOL(drm_dp_dsc_sink_max_slice_count);
> >  
> > +/**
> > + * drm_dp_dsc_sink_line_buf_depth() - Get the line buffer depth in bits 
> > which is
> > + * number of bits of precision within the decoder line buffer supported by
> > + * the DSC sink. This is used to populate the DSC parameters in the
> > + *  drm_dsc_config by the driver.
> > + * Driver creates an infoframe using these parameters to populate
> > + *  drm_dsc_pps_infoframe. These are sent to the sink using DSC
> > + * infoframe using the helper function drm_dsc_pps_infoframe_pack()
> 
> The entire above block ends up being used as the "one-line" summary, and
> no description. I guess you wanted to split it up like the one above?
> 
> The description starts after the first empty newline, everything before
> that is the summary.

Yes I will split this as summary and description. I tested this in html docs
but my eyes failed to catch this. Thanks for pointing out.

> 
> > + * @dsc_dpcd: DSC capabilities from DPCD
> > + *
> > + * Returns:
> > + * Line buffer depth supported by DSC panel or 0 its invalid
> > + */
> >  u8 drm_dp_dsc_sink_line_buf_depth(const u8 
> > dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> >  {
> > u8 line_buf_depth = dsc_dpcd[DP_DSC_LINE_BUF_BIT_DEPTH - 
> > DP_DSC_SUPPORT];
> > @@ -1434,6 +1460,20 @@ u8 drm_dp_dsc_sink_line_buf_depth(const u8 
> > dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> >  }
> >  EXPORT_SYMBOL(drm_dp_dsc_sink_line_buf_depth);
> >  
> > +/**
> > + * drm_dp_dsc_sink_supported_input_bpcs() - Get all the input bits per 
> > component
> > + * values supported by the DSC sink. This is used to populate the DSC 
> > parameters
> > + * in the  drm_dsc_config by the driver.
> > + * Driver creates an infoframe using these parameters to populate
> > + *  drm_dsc_pps_infoframe. These are sent to the sink using DSC
> > + * infoframe using the helper function drm_dsc_pps_infoframe_pack()
> > + * @dsc_dpcd: DSC capabilities from DPCD
> > + * @dsc_bpc: An array to be filled by this helper with supported
> > + *   input bpcs.
> > + *
> > + * Returns:
> > + * Number of input BPC values parsed from the DPCD
> > + */
> >  int drm_dp_dsc_sink_supported_input_bpcs(const u8 
> > dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
> >  u8 dsc_bpc[3])
> >  {
> > diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
> > index bc2b23adb072..0fd56fbdf9b4 100644
> > --- a/drivers/gpu/drm/drm_dsc.c
> > +++ b/drivers/gpu/drm/drm_dsc.c
> > @@ -17,6 +17,12 @@
> >  /**
> >   * DOC: dsc helpers
> >   *
> > + * VESA specification for DP 1.4 adds a new feature called Display Stream
> > + * Compression (DSC) used 

Re: [PATCH] drm/dsc: Add kernel documentation for DRM DP DSC helpers

2019-02-05 Thread Daniel Vetter
On Mon, Feb 04, 2019 at 03:40:22PM -0800, Manasi Navare wrote:
> This patch adds appropiate kernel documentation for DRM DP helpers
> used for enabling Display Stream compression functionality in
> drm_dp_helper.h and drm_dp_helper.c as well as for the DSC spec
> related structure definitions and helpers in drm_dsc.c and drm_dsc.h
> Also add links between the functions and structures in the documentation.
> 
> Suggested-by: Daniel Vetter 
> Suggested-by: Sean Paul 
> Cc: Daniel Vetter 
> Cc: Sean Paul 
> Signed-off-by: Manasi Navare 

Awesome, more docs!

> ---
>  drivers/gpu/drm/drm_dp_helper.c |  42 +-
>  drivers/gpu/drm/drm_dsc.c   |  13 ++-
>  include/drm/drm_dp_helper.h |  15 +++-
>  include/drm/drm_dsc.h   | 138 ++--
>  4 files changed, 142 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 54120b6319e7..e9e0233f5b2f 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1360,7 +1360,20 @@ int drm_dp_read_desc(struct drm_dp_aux *aux, struct 
> drm_dp_desc *desc,
>  EXPORT_SYMBOL(drm_dp_read_desc);
>  
>  /**
> - * DRM DP Helpers for DSC
> + * drm_dp_dsc_sink_max_slice_count() - Get the max slice count
> + * supported by the DSC sink.
> + * @dsc_dpcd: DSC capabilities from DPCD
> + * @is_edp: true if its eDP, false for DP
> + *
> + * Read the slice capabilities DPCD register from DSC sink to get
> + * the maximum slice count supported. This is used to populate
> + * the DSC parameters in the  drm_dsc_config by the driver.
> + * Driver creates an infoframe using these parameters to populate
> + *  drm_dsc_pps_infoframe. These are sent to the sink using DSC
> + * infoframe using the helper function drm_dsc_pps_infoframe_pack()
> + *
> + * Returns:
> + * Maximum slice count supported by DSC sink or 0 its invalid
>   */
>  u8 drm_dp_dsc_sink_max_slice_count(const u8 
> dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
>  bool is_edp)
> @@ -1405,6 +1418,19 @@ u8 drm_dp_dsc_sink_max_slice_count(const u8 
> dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
>  }
>  EXPORT_SYMBOL(drm_dp_dsc_sink_max_slice_count);
>  
> +/**
> + * drm_dp_dsc_sink_line_buf_depth() - Get the line buffer depth in bits 
> which is
> + * number of bits of precision within the decoder line buffer supported by
> + * the DSC sink. This is used to populate the DSC parameters in the
> + *  drm_dsc_config by the driver.
> + * Driver creates an infoframe using these parameters to populate
> + *  drm_dsc_pps_infoframe. These are sent to the sink using DSC
> + * infoframe using the helper function drm_dsc_pps_infoframe_pack()

The entire above block ends up being used as the "one-line" summary, and
no description. I guess you wanted to split it up like the one above?

The description starts after the first empty newline, everything before
that is the summary.

> + * @dsc_dpcd: DSC capabilities from DPCD
> + *
> + * Returns:
> + * Line buffer depth supported by DSC panel or 0 its invalid
> + */
>  u8 drm_dp_dsc_sink_line_buf_depth(const u8 
> dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
>  {
>   u8 line_buf_depth = dsc_dpcd[DP_DSC_LINE_BUF_BIT_DEPTH - 
> DP_DSC_SUPPORT];
> @@ -1434,6 +1460,20 @@ u8 drm_dp_dsc_sink_line_buf_depth(const u8 
> dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
>  }
>  EXPORT_SYMBOL(drm_dp_dsc_sink_line_buf_depth);
>  
> +/**
> + * drm_dp_dsc_sink_supported_input_bpcs() - Get all the input bits per 
> component
> + * values supported by the DSC sink. This is used to populate the DSC 
> parameters
> + * in the  drm_dsc_config by the driver.
> + * Driver creates an infoframe using these parameters to populate
> + *  drm_dsc_pps_infoframe. These are sent to the sink using DSC
> + * infoframe using the helper function drm_dsc_pps_infoframe_pack()
> + * @dsc_dpcd: DSC capabilities from DPCD
> + * @dsc_bpc: An array to be filled by this helper with supported
> + *   input bpcs.
> + *
> + * Returns:
> + * Number of input BPC values parsed from the DPCD
> + */
>  int drm_dp_dsc_sink_supported_input_bpcs(const u8 
> dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
>u8 dsc_bpc[3])
>  {
> diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
> index bc2b23adb072..0fd56fbdf9b4 100644
> --- a/drivers/gpu/drm/drm_dsc.c
> +++ b/drivers/gpu/drm/drm_dsc.c
> @@ -17,6 +17,12 @@
>  /**
>   * DOC: dsc helpers
>   *
> + * VESA specification for DP 1.4 adds a new feature called Display Stream
> + * Compression (DSC) used to compress the pixel bits before sending it on
> + * DP/eDP/MIPI DSI interface. DSC is required to be enabled so that the 
> existing
> + * display interfaces can support high resolutions at higher frames rates 
> uisng
> + * the maximum available link capacity of these interfaces.
> + *
>   * These functions contain some common logic and helpers to deal with VESA
>   * Display Stream Compression standard required for DSC on