Re: [Intel-gfx] [PATCH v3 19/37] drm/i915: stop using kernel-doc markups for something else

2022-09-26 Thread Mauro Carvalho Chehab
Em Mon, 12 Sep 2022 10:19:04 -0700
Matt Roper  escreveu:

> > Those *appear* to be kernel-doc markups, but they aren't, because
> > the structs themselves are not properly marked. See, for instance
> > struct intel_context.
> > 
> > scripts/kerneldoc will *only* consider what's there as a proper
> > comment if you add:
> > 
> > /**
> >  * struct intel_context - describes an i915 context
> >  * 
> >  */
> > struct intel_context {
> > union {
> > /** @ref: a kernel object reference */
> > struct kref ref; /* no kref_get_unless_zero()! */
> > /** @rcu: a rcu header */
> > struct rcu_head rcu;
> > };
> > ...
> > }
> > 
> > Describing all fields inside the struct. Just placing
> > /** something */
> > on some random places in the middle doesn't make it a kernel-doc.  
> 
> Right, what we have today is incomplete/incorrect.  But I think we
> should be fixing that by adding the missing documentation on the
> structure, rather than giving up and removing the kerneldoc.  The end
> goal should be to have proper generated documentation, not just silence
> the warnings while leaving the actual output incomplete.

The end goal is to have *real* kernel-doc markups, not fake ones.
We're aiming towards there, where the first step is to get rid of the
ones that just pretend to be kernel-doc without really being validated
in order to check if they produce a valid content.

See, what we have so far are just comments. Some examples from this
patch:

/** Powers down the TV out block, and DAC0-3 */
 #define CH7017_TV_POWER_DOWN_EN(1 << 5)

Currently, kernel-doc doesn't have any markup for not-function defines.

What we do to document this at kernel-doc is to either:

1. convert to an enum;
2. embed it inside some struct (or function) definition.

Other examples: this is not a Kernel-doc markup:

/** @file
  * driver for the Chrontel 7xxx DVI chip over DVO.
  */

Neither this:

/**
 * active: Active tracker for the rq activity (inc. external) on this
 * intel_context object.
 */
 
In summary, what happens is that those "/**" tags removed on this patch are
just pretending to be Kernel-doc, but they aren't anything. See, when a newbie
starts programming in C code, without having a compiler, lots of syntax
issues will happen. When they try to compile their code, hundreds or errors
and warnings happen. That's basically what it is happening here.

See, the very basic syntax thing is missing: just like a C file requires
that all codes to be inside functions, a kernel-doc field description 
requires a kernel-doc markup for the struct/function/enum/union that
contains it.

-

Now, I fully agree that the end goal is to have proper kernel-doc markups.

Adding those require a lot of archaeological work, looking at the git
commits which introduced the changes. Patch 34/37, for instance, does
that for struct drm_i915_gem_object:


https://lists.freedesktop.org/archives/intel-gfx/2022-September/305736.html

See, besides adding a real Kernel-doc markup for the struct:

+/**
+ * struct drm_i915_gem_object - describes an i915 GEM object
+ */
 struct drm_i915_gem_object {

It had to fix several sintax and semantic mistakes:

Typos:

/**
-* @shared_resv_from: The object shares the resv from this vm.
+* @shares_resv_from: The object shares the resv from this vm.
 */

Invalid kernel-doc markups:

/**
-* @mem_flags - Mutable placement-related flags
+* @mem_flags: Mutable placement-related flags

Kernel markups that miss that they're on an embedded struct inside
the main one (thus those are also invalid kernel-doc markups):

/**
-* @shrink_pin: Prevents the pages from being made visible to
+* @mm.shrink_pin: Prevents the pages from being made visible to


Etc.

Thanks,
Mauro


Re: [PATCH v3 19/37] drm/i915: stop using kernel-doc markups for something else

2022-09-12 Thread Matt Roper
On Mon, Sep 12, 2022 at 06:47:56PM +0200, Mauro Carvalho Chehab wrote:
> Hi Matt,
> 
> Em Mon, 12 Sep 2022 08:09:57 -0700
> Matt Roper  escreveu:
> 
> > > --- a/drivers/gpu/drm/i915/gt/intel_context_types.h
> > > +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h  
> > 
> > Several of the comments in this file do appear to be kerneldoc (in fact
> > kerneldoc that was specifically requested during code review at
> > https://patchwork.freedesktop.org/patch/448456/#comment_804252) and this
> > file is included from Documentation/gpu/i915.rst, so I think some of
> > these changes might be moving in the wrong direction.  Should we instead
> > focus on fixing up the comments that aren't quite formatted properly?
> 
> Those *appear* to be kernel-doc markups, but they aren't, because
> the structs themselves are not properly marked. See, for instance
> struct intel_context.
> 
> scripts/kerneldoc will *only* consider what's there as a proper
> comment if you add:
> 
>   /**
>* struct intel_context - describes an i915 context
>* 
>*/
>   struct intel_context {
>   union {
>   /** @ref: a kernel object reference */
>   struct kref ref; /* no kref_get_unless_zero()! */
>   /** @rcu: a rcu header */
>   struct rcu_head rcu;
>   };
>   ...
>   }
> 
> Describing all fields inside the struct. Just placing
>   /** something */
> on some random places in the middle doesn't make it a kernel-doc.

Right, what we have today is incomplete/incorrect.  But I think we
should be fixing that by adding the missing documentation on the
structure, rather than giving up and removing the kerneldoc.  The end
goal should be to have proper generated documentation, not just silence
the warnings while leaving the actual output incomplete.


Matt

> 
> If you actually run kernel-doc in Werror mode:
> 
>   ./scripts/kernel-doc -Werror -sphinx-version 2.4.4 
> drivers/gpu/drm/i915/gt/intel_context_types.h | echo "ERROR!"
>   ERROR!
>   drivers/gpu/drm/i915/gt/intel_context_types.h:1: warning: no structured 
> comments found
>   1 warnings as Errors
> 
> you'll see that this is currently broken.
> 
> Thanks,
> Mauro

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation


Re: [PATCH v3 19/37] drm/i915: stop using kernel-doc markups for something else

2022-09-12 Thread Mauro Carvalho Chehab
Hi Matt,

Em Mon, 12 Sep 2022 08:09:57 -0700
Matt Roper  escreveu:

> > --- a/drivers/gpu/drm/i915/gt/intel_context_types.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h  
> 
> Several of the comments in this file do appear to be kerneldoc (in fact
> kerneldoc that was specifically requested during code review at
> https://patchwork.freedesktop.org/patch/448456/#comment_804252) and this
> file is included from Documentation/gpu/i915.rst, so I think some of
> these changes might be moving in the wrong direction.  Should we instead
> focus on fixing up the comments that aren't quite formatted properly?

Those *appear* to be kernel-doc markups, but they aren't, because
the structs themselves are not properly marked. See, for instance
struct intel_context.

scripts/kerneldoc will *only* consider what's there as a proper
comment if you add:

/**
 * struct intel_context - describes an i915 context
 * 
 */
struct intel_context {
union {
/** @ref: a kernel object reference */
struct kref ref; /* no kref_get_unless_zero()! */
/** @rcu: a rcu header */
struct rcu_head rcu;
};
...
}

Describing all fields inside the struct. Just placing
/** something */
on some random places in the middle doesn't make it a kernel-doc.

If you actually run kernel-doc in Werror mode:

./scripts/kernel-doc -Werror -sphinx-version 2.4.4 
drivers/gpu/drm/i915/gt/intel_context_types.h | echo "ERROR!"
ERROR!
drivers/gpu/drm/i915/gt/intel_context_types.h:1: warning: no structured 
comments found
1 warnings as Errors

you'll see that this is currently broken.

Thanks,
Mauro


Re: [PATCH v3 19/37] drm/i915: stop using kernel-doc markups for something else

2022-09-12 Thread Matt Roper
On Fri, Sep 09, 2022 at 09:34:26AM +0200, Mauro Carvalho Chehab wrote:
> There are some occurrences of "/**" that aren't actually part of
> a kernel-doc markup. Replace them by "/*", in order to make easier
> to identify what i915 files contain kernel-doc markups.
> 
> Reviewed-by: Rodrigo Vivi 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
> 
> To avoid mailbombing on a large number of people, only mailing lists were C/C 
> on the cover.
> See [PATCH v3 00/37] at: 
> https://lore.kernel.org/all/cover.1662708705.git.mche...@kernel.org/
> 
>  drivers/gpu/drm/i915/display/dvo_ch7017.c | 26 +++
>  drivers/gpu/drm/i915/display/dvo_ch7xxx.c |  6 +-
>  .../drm/i915/display/intel_display_types.h|  2 +-
>  drivers/gpu/drm/i915/display/intel_dvo_dev.h  |  6 +-
>  drivers/gpu/drm/i915/display/intel_sdvo.c |  4 +-
>  drivers/gpu/drm/i915/display/intel_tv.c   |  2 +-
>  drivers/gpu/drm/i915/gt/intel_context_types.h | 69 +--
>  drivers/gpu/drm/i915/gt/intel_ggtt_fencing.h  |  2 +-
>  drivers/gpu/drm/i915/gt/intel_gt_types.h  | 12 ++--
>  drivers/gpu/drm/i915/gt/intel_reset_types.h   |  4 +-
>  .../gpu/drm/i915/gt/intel_timeline_types.h|  6 +-
>  .../drm/i915/gt/shaders/clear_kernel/hsw.asm  |  4 +-
>  .../drm/i915/gt/shaders/clear_kernel/ivb.asm  |  4 +-
>  drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h | 10 +--
>  drivers/gpu/drm/i915/i915_drm_client.h|  2 +-
>  drivers/gpu/drm/i915/i915_drv.h   | 24 +++
>  drivers/gpu/drm/i915/i915_file_private.h  |  8 +--
>  drivers/gpu/drm/i915/i915_gpu_error.h |  4 +-
>  drivers/gpu/drm/i915/i915_pmu.h   | 32 -
>  drivers/gpu/drm/i915/intel_uncore.h   |  4 +-
>  20 files changed, 115 insertions(+), 116 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/dvo_ch7017.c 
> b/drivers/gpu/drm/i915/display/dvo_ch7017.c
> index 0589994dde11..581e29ab77e4 100644
> --- a/drivers/gpu/drm/i915/display/dvo_ch7017.c
> +++ b/drivers/gpu/drm/i915/display/dvo_ch7017.c
> @@ -55,13 +55,13 @@
>  #define CH7017_TEST_PATTERN  0x48
>  
>  #define CH7017_POWER_MANAGEMENT  0x49
> -/** Enables the TV output path. */
> +/* Enables the TV output path. */
>  #define CH7017_TV_EN (1 << 0)
>  #define CH7017_DAC0_POWER_DOWN   (1 << 1)
>  #define CH7017_DAC1_POWER_DOWN   (1 << 2)
>  #define CH7017_DAC2_POWER_DOWN   (1 << 3)
>  #define CH7017_DAC3_POWER_DOWN   (1 << 4)
> -/** Powers down the TV out block, and DAC0-3 */
> +/* Powers down the TV out block, and DAC0-3 */
>  #define CH7017_TV_POWER_DOWN_EN  (1 << 5)
>  
>  #define CH7017_VERSION_ID0x4a
> @@ -84,26 +84,26 @@
>  #define CH7017_UP_SCALER_HORIZONTAL_INC_10x5e
>  
>  #define CH7017_HORIZONTAL_ACTIVE_PIXEL_INPUT 0x5f
> -/**< Low bits of horizontal active pixel input */
> +/* Low bits of horizontal active pixel input */
>  
>  #define CH7017_ACTIVE_INPUT_LINE_OUTPUT  0x60
> -/** High bits of horizontal active pixel input */
> +/* High bits of horizontal active pixel input */
>  #define CH7017_LVDS_HAP_INPUT_MASK   (0x7 << 0)
> -/** High bits of vertical active line output */
> +/* High bits of vertical active line output */
>  #define CH7017_LVDS_VAL_HIGH_MASK(0x7 << 3)
>  
>  #define CH7017_VERTICAL_ACTIVE_LINE_OUTPUT   0x61
> -/**< Low bits of vertical active line output */
> +/* Low bits of vertical active line output */
>  
>  #define CH7017_HORIZONTAL_ACTIVE_PIXEL_OUTPUT0x62
> -/**< Low bits of horizontal active pixel output */
> +/* Low bits of horizontal active pixel output */
>  
>  #define CH7017_LVDS_POWER_DOWN   0x63
> -/** High bits of horizontal active pixel output */
> +/* High bits of horizontal active pixel output */
>  #define CH7017_LVDS_HAP_HIGH_MASK(0x7 << 0)
> -/** Enables the LVDS power down state transition */
> +/* Enables the LVDS power down state transition */
>  #define CH7017_LVDS_POWER_DOWN_EN(1 << 6)
> -/** Enables the LVDS upscaler */
> +/* Enables the LVDS upscaler */
>  #define CH7017_LVDS_UPSCALER_EN  (1 << 7)
>  #define CH7017_LVDS_POWER_DOWN_DEFAULT_RESERVED 0x08
>  
> @@ -116,9 +116,9 @@
>  #define CH7017_LVDS_ENCODING_2   0x65
>  
>  #define CH7017_LVDS_PLL_CONTROL  0x66
> -/** Enables the LVDS panel output path */
> +/* Enables the LVDS panel output path */
>  #define CH7017_LVDS_PANEN(1 << 0)
> -/** Enables the LVDS panel backlight */
> +/* Enables the LVDS panel backlight */
>  #define CH7017_LVDS_BKLEN(1 << 3)
>  
>  #define CH7017_POWER_SEQUENCING_T1   0x67
> @@ -197,7 +197,7 @@ static bool ch7017_write(struct intel_dvo_device *dvo, u8 
> addr, u8 val)
>   return i2c_transfer(dvo->i2c_bus, , 1) == 1;
>  }
>  
> -/** Probes for a CH7017 on the given bus and slave address. */
> +/* Probes for a CH7017 on the given bus and slave address. */
>  static bool ch7017_init(struct intel_dvo_device 

Re: [PATCH v3 19/37] drm/i915: stop using kernel-doc markups for something else

2022-09-11 Thread Andi Shyti
Hi Mauro,

On Fri, Sep 09, 2022 at 09:34:26AM +0200, Mauro Carvalho Chehab wrote:
> There are some occurrences of "/**" that aren't actually part of
> a kernel-doc markup. Replace them by "/*", in order to make easier
> to identify what i915 files contain kernel-doc markups.
> 
> Reviewed-by: Rodrigo Vivi 
> Signed-off-by: Mauro Carvalho Chehab 

nice cleanup!

Reviewed-by: Andi Shyti 

Thanks,
Andi

> ---
> 
> To avoid mailbombing on a large number of people, only mailing lists were C/C 
> on the cover.
> See [PATCH v3 00/37] at: 
> https://lore.kernel.org/all/cover.1662708705.git.mche...@kernel.org/
> 
>  drivers/gpu/drm/i915/display/dvo_ch7017.c | 26 +++
>  drivers/gpu/drm/i915/display/dvo_ch7xxx.c |  6 +-
>  .../drm/i915/display/intel_display_types.h|  2 +-
>  drivers/gpu/drm/i915/display/intel_dvo_dev.h  |  6 +-
>  drivers/gpu/drm/i915/display/intel_sdvo.c |  4 +-
>  drivers/gpu/drm/i915/display/intel_tv.c   |  2 +-
>  drivers/gpu/drm/i915/gt/intel_context_types.h | 69 +--
>  drivers/gpu/drm/i915/gt/intel_ggtt_fencing.h  |  2 +-
>  drivers/gpu/drm/i915/gt/intel_gt_types.h  | 12 ++--
>  drivers/gpu/drm/i915/gt/intel_reset_types.h   |  4 +-
>  .../gpu/drm/i915/gt/intel_timeline_types.h|  6 +-
>  .../drm/i915/gt/shaders/clear_kernel/hsw.asm  |  4 +-
>  .../drm/i915/gt/shaders/clear_kernel/ivb.asm  |  4 +-
>  drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h | 10 +--
>  drivers/gpu/drm/i915/i915_drm_client.h|  2 +-
>  drivers/gpu/drm/i915/i915_drv.h   | 24 +++
>  drivers/gpu/drm/i915/i915_file_private.h  |  8 +--
>  drivers/gpu/drm/i915/i915_gpu_error.h |  4 +-
>  drivers/gpu/drm/i915/i915_pmu.h   | 32 -
>  drivers/gpu/drm/i915/intel_uncore.h   |  4 +-
>  20 files changed, 115 insertions(+), 116 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/dvo_ch7017.c 
> b/drivers/gpu/drm/i915/display/dvo_ch7017.c
> index 0589994dde11..581e29ab77e4 100644
> --- a/drivers/gpu/drm/i915/display/dvo_ch7017.c
> +++ b/drivers/gpu/drm/i915/display/dvo_ch7017.c
> @@ -55,13 +55,13 @@
>  #define CH7017_TEST_PATTERN  0x48
>  
>  #define CH7017_POWER_MANAGEMENT  0x49
> -/** Enables the TV output path. */
> +/* Enables the TV output path. */
>  #define CH7017_TV_EN (1 << 0)
>  #define CH7017_DAC0_POWER_DOWN   (1 << 1)
>  #define CH7017_DAC1_POWER_DOWN   (1 << 2)
>  #define CH7017_DAC2_POWER_DOWN   (1 << 3)
>  #define CH7017_DAC3_POWER_DOWN   (1 << 4)
> -/** Powers down the TV out block, and DAC0-3 */
> +/* Powers down the TV out block, and DAC0-3 */
>  #define CH7017_TV_POWER_DOWN_EN  (1 << 5)
>  
>  #define CH7017_VERSION_ID0x4a
> @@ -84,26 +84,26 @@
>  #define CH7017_UP_SCALER_HORIZONTAL_INC_10x5e
>  
>  #define CH7017_HORIZONTAL_ACTIVE_PIXEL_INPUT 0x5f
> -/**< Low bits of horizontal active pixel input */
> +/* Low bits of horizontal active pixel input */
>  
>  #define CH7017_ACTIVE_INPUT_LINE_OUTPUT  0x60
> -/** High bits of horizontal active pixel input */
> +/* High bits of horizontal active pixel input */
>  #define CH7017_LVDS_HAP_INPUT_MASK   (0x7 << 0)
> -/** High bits of vertical active line output */
> +/* High bits of vertical active line output */
>  #define CH7017_LVDS_VAL_HIGH_MASK(0x7 << 3)
>  
>  #define CH7017_VERTICAL_ACTIVE_LINE_OUTPUT   0x61
> -/**< Low bits of vertical active line output */
> +/* Low bits of vertical active line output */
>  
>  #define CH7017_HORIZONTAL_ACTIVE_PIXEL_OUTPUT0x62
> -/**< Low bits of horizontal active pixel output */
> +/* Low bits of horizontal active pixel output */
>  
>  #define CH7017_LVDS_POWER_DOWN   0x63
> -/** High bits of horizontal active pixel output */
> +/* High bits of horizontal active pixel output */
>  #define CH7017_LVDS_HAP_HIGH_MASK(0x7 << 0)
> -/** Enables the LVDS power down state transition */
> +/* Enables the LVDS power down state transition */
>  #define CH7017_LVDS_POWER_DOWN_EN(1 << 6)
> -/** Enables the LVDS upscaler */
> +/* Enables the LVDS upscaler */
>  #define CH7017_LVDS_UPSCALER_EN  (1 << 7)
>  #define CH7017_LVDS_POWER_DOWN_DEFAULT_RESERVED 0x08
>  
> @@ -116,9 +116,9 @@
>  #define CH7017_LVDS_ENCODING_2   0x65
>  
>  #define CH7017_LVDS_PLL_CONTROL  0x66
> -/** Enables the LVDS panel output path */
> +/* Enables the LVDS panel output path */
>  #define CH7017_LVDS_PANEN(1 << 0)
> -/** Enables the LVDS panel backlight */
> +/* Enables the LVDS panel backlight */
>  #define CH7017_LVDS_BKLEN(1 << 3)
>  
>  #define CH7017_POWER_SEQUENCING_T1   0x67
> @@ -197,7 +197,7 @@ static bool ch7017_write(struct intel_dvo_device *dvo, u8 
> addr, u8 val)
>   return i2c_transfer(dvo->i2c_bus, , 1) == 1;
>  }
>  
> -/** Probes for a CH7017 on the given bus and slave address. */
> +/* Probes for a CH7017 on the given bus and 

[PATCH v3 19/37] drm/i915: stop using kernel-doc markups for something else

2022-09-09 Thread Mauro Carvalho Chehab
There are some occurrences of "/**" that aren't actually part of
a kernel-doc markup. Replace them by "/*", in order to make easier
to identify what i915 files contain kernel-doc markups.

Reviewed-by: Rodrigo Vivi 
Signed-off-by: Mauro Carvalho Chehab 
---

To avoid mailbombing on a large number of people, only mailing lists were C/C 
on the cover.
See [PATCH v3 00/37] at: 
https://lore.kernel.org/all/cover.1662708705.git.mche...@kernel.org/

 drivers/gpu/drm/i915/display/dvo_ch7017.c | 26 +++
 drivers/gpu/drm/i915/display/dvo_ch7xxx.c |  6 +-
 .../drm/i915/display/intel_display_types.h|  2 +-
 drivers/gpu/drm/i915/display/intel_dvo_dev.h  |  6 +-
 drivers/gpu/drm/i915/display/intel_sdvo.c |  4 +-
 drivers/gpu/drm/i915/display/intel_tv.c   |  2 +-
 drivers/gpu/drm/i915/gt/intel_context_types.h | 69 +--
 drivers/gpu/drm/i915/gt/intel_ggtt_fencing.h  |  2 +-
 drivers/gpu/drm/i915/gt/intel_gt_types.h  | 12 ++--
 drivers/gpu/drm/i915/gt/intel_reset_types.h   |  4 +-
 .../gpu/drm/i915/gt/intel_timeline_types.h|  6 +-
 .../drm/i915/gt/shaders/clear_kernel/hsw.asm  |  4 +-
 .../drm/i915/gt/shaders/clear_kernel/ivb.asm  |  4 +-
 drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h | 10 +--
 drivers/gpu/drm/i915/i915_drm_client.h|  2 +-
 drivers/gpu/drm/i915/i915_drv.h   | 24 +++
 drivers/gpu/drm/i915/i915_file_private.h  |  8 +--
 drivers/gpu/drm/i915/i915_gpu_error.h |  4 +-
 drivers/gpu/drm/i915/i915_pmu.h   | 32 -
 drivers/gpu/drm/i915/intel_uncore.h   |  4 +-
 20 files changed, 115 insertions(+), 116 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/dvo_ch7017.c 
b/drivers/gpu/drm/i915/display/dvo_ch7017.c
index 0589994dde11..581e29ab77e4 100644
--- a/drivers/gpu/drm/i915/display/dvo_ch7017.c
+++ b/drivers/gpu/drm/i915/display/dvo_ch7017.c
@@ -55,13 +55,13 @@
 #define CH7017_TEST_PATTERN0x48
 
 #define CH7017_POWER_MANAGEMENT0x49
-/** Enables the TV output path. */
+/* Enables the TV output path. */
 #define CH7017_TV_EN   (1 << 0)
 #define CH7017_DAC0_POWER_DOWN (1 << 1)
 #define CH7017_DAC1_POWER_DOWN (1 << 2)
 #define CH7017_DAC2_POWER_DOWN (1 << 3)
 #define CH7017_DAC3_POWER_DOWN (1 << 4)
-/** Powers down the TV out block, and DAC0-3 */
+/* Powers down the TV out block, and DAC0-3 */
 #define CH7017_TV_POWER_DOWN_EN(1 << 5)
 
 #define CH7017_VERSION_ID  0x4a
@@ -84,26 +84,26 @@
 #define CH7017_UP_SCALER_HORIZONTAL_INC_1  0x5e
 
 #define CH7017_HORIZONTAL_ACTIVE_PIXEL_INPUT   0x5f
-/**< Low bits of horizontal active pixel input */
+/* Low bits of horizontal active pixel input */
 
 #define CH7017_ACTIVE_INPUT_LINE_OUTPUT0x60
-/** High bits of horizontal active pixel input */
+/* High bits of horizontal active pixel input */
 #define CH7017_LVDS_HAP_INPUT_MASK (0x7 << 0)
-/** High bits of vertical active line output */
+/* High bits of vertical active line output */
 #define CH7017_LVDS_VAL_HIGH_MASK  (0x7 << 3)
 
 #define CH7017_VERTICAL_ACTIVE_LINE_OUTPUT 0x61
-/**< Low bits of vertical active line output */
+/* Low bits of vertical active line output */
 
 #define CH7017_HORIZONTAL_ACTIVE_PIXEL_OUTPUT  0x62
-/**< Low bits of horizontal active pixel output */
+/* Low bits of horizontal active pixel output */
 
 #define CH7017_LVDS_POWER_DOWN 0x63
-/** High bits of horizontal active pixel output */
+/* High bits of horizontal active pixel output */
 #define CH7017_LVDS_HAP_HIGH_MASK  (0x7 << 0)
-/** Enables the LVDS power down state transition */
+/* Enables the LVDS power down state transition */
 #define CH7017_LVDS_POWER_DOWN_EN  (1 << 6)
-/** Enables the LVDS upscaler */
+/* Enables the LVDS upscaler */
 #define CH7017_LVDS_UPSCALER_EN(1 << 7)
 #define CH7017_LVDS_POWER_DOWN_DEFAULT_RESERVED 0x08
 
@@ -116,9 +116,9 @@
 #define CH7017_LVDS_ENCODING_2 0x65
 
 #define CH7017_LVDS_PLL_CONTROL0x66
-/** Enables the LVDS panel output path */
+/* Enables the LVDS panel output path */
 #define CH7017_LVDS_PANEN  (1 << 0)
-/** Enables the LVDS panel backlight */
+/* Enables the LVDS panel backlight */
 #define CH7017_LVDS_BKLEN  (1 << 3)
 
 #define CH7017_POWER_SEQUENCING_T1 0x67
@@ -197,7 +197,7 @@ static bool ch7017_write(struct intel_dvo_device *dvo, u8 
addr, u8 val)
return i2c_transfer(dvo->i2c_bus, , 1) == 1;
 }
 
-/** Probes for a CH7017 on the given bus and slave address. */
+/* Probes for a CH7017 on the given bus and slave address. */
 static bool ch7017_init(struct intel_dvo_device *dvo,
struct i2c_adapter *adapter)
 {
diff --git a/drivers/gpu/drm/i915/display/dvo_ch7xxx.c 
b/drivers/gpu/drm/i915/display/dvo_ch7xxx.c
index 54f58ba44b9f..1c1fe1f29675 100644
--- a/drivers/gpu/drm/i915/display/dvo_ch7xxx.c
+++ b/drivers/gpu/drm/i915/display/dvo_ch7xxx.c