[PATCH v2] drm/amd/amdgpu: Fix warnings in amdgpu/amdgpu_display.c

Alex Deucher alexdeucher at gmail.com
Thu Jul 20 16:12:05 UTC 2023


On Wed, Jul 19, 2023 at 10:42 PM Srinivasan Shanmugam
<srinivasan.shanmugam at amd.com> wrote:
>
> Fixes the below checkpatch.pl warnings:
>
> WARNING: Block comments use * on subsequent lines
> WARNING: Block comments use a trailing */ on a separate line
> WARNING: suspect code indent for conditional statements (8, 12)
> WARNING: braces {} are not necessary for single statement blocks
>
> Cc: Christian König <christian.koenig at amd.com>
> Cc: Alex Deucher <alexander.deucher at amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 42 ++++++++++++---------
>  1 file changed, 25 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index b702f499f5fb..9a74eae50ad4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -124,7 +124,7 @@ static void amdgpu_display_flip_work_func(struct work_struct *__work)
>
>         struct drm_crtc *crtc = &amdgpu_crtc->base;
>         unsigned long flags;
> -       unsigned i;
> +       unsigned int i;
>         int vpos, hpos;
>
>         for (i = 0; i < work->shared_count; ++i)
> @@ -201,7 +201,7 @@ int amdgpu_display_crtc_page_flip_target(struct drm_crtc *crtc,
>         u64 tiling_flags;
>         int i, r;
>
> -       work = kzalloc(sizeof *work, GFP_KERNEL);
> +       work = kzalloc(sizeof(*work), GFP_KERNEL);
>         if (work == NULL)
>                 return -ENOMEM;
>
> @@ -332,13 +332,15 @@ int amdgpu_display_crtc_set_config(struct drm_mode_set *set,
>
>         adev = drm_to_adev(dev);
>         /* if we have active crtcs and we don't have a power ref,
> -          take the current one */
> +        * take the current one
> +        */
>         if (active && !adev->have_disp_power_ref) {
>                 adev->have_disp_power_ref = true;
>                 return ret;
>         }
>         /* if we have no active crtcs, then drop the power ref
> -          we got before */
> +        * we got before
> +        */
>         if (!active && adev->have_disp_power_ref) {
>                 pm_runtime_put_autosuspend(dev->dev);
>                 adev->have_disp_power_ref = false;
> @@ -507,11 +509,10 @@ bool amdgpu_display_ddc_probe(struct amdgpu_connector *amdgpu_connector,
>         if (amdgpu_connector->router.ddc_valid)
>                 amdgpu_i2c_router_select_ddc_port(amdgpu_connector);
>
> -       if (use_aux) {
> +       if (use_aux)
>                 ret = i2c_transfer(&amdgpu_connector->ddc_bus->aux.ddc, msgs, 2);
> -       } else {
> +       else
>                 ret = i2c_transfer(&amdgpu_connector->ddc_bus->adapter, msgs, 2);
> -       }
>
>         if (ret != 2)
>                 /* Couldn't find an accessible DDC on this connector */
> @@ -520,10 +521,12 @@ bool amdgpu_display_ddc_probe(struct amdgpu_connector *amdgpu_connector,
>          * EDID header starts with:
>          * 0x00,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0x00.
>          * Only the first 6 bytes must be valid as
> -        * drm_edid_block_valid() can fix the last 2 bytes */
> +        * drm_edid_block_valid() can fix the last 2 bytes
> +        */
>         if (drm_edid_header_is_valid(buf) < 6) {
>                 /* Couldn't find an accessible EDID on this
> -                * connector */
> +                * connector
> +                */
>                 return false;
>         }
>         return true;
> @@ -1216,8 +1219,10 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,
>
>         obj = drm_gem_object_lookup(file_priv, mode_cmd->handles[0]);
>         if (obj ==  NULL) {
> -               drm_dbg_kms(dev, "No GEM object associated to handle 0x%08X, "
> -                           "can't create framebuffer\n", mode_cmd->handles[0]);
> +               drm_dbg_kms(dev, "No GEM object associated to handle 0x%08X\n",
> +                           mode_cmd->handles[0]);
> +               drm_dbg_kms(dev, "  - Can't create framebuffer\n");

Can you rework this to be a single debug statement?  Maybe
               drm_dbg_kms(dev,
                           "No GEM object associated to handle 0x%08X,
can't create framebuffer\n"
                           mode_cmd->handles[0]);

With that fixed,
Reviewed-by: Alex Deucher <alexander.deucher at amd.com>


> +
>                 return ERR_PTR(-ENOENT);
>         }
>
> @@ -1410,6 +1415,7 @@ bool amdgpu_display_crtc_scaling_mode_fixup(struct drm_crtc *crtc,
>         }
>         if (amdgpu_crtc->rmx_type != RMX_OFF) {
>                 fixed20_12 a, b;
> +
>                 a.full = dfixed_const(src_v);
>                 b.full = dfixed_const(dst_v);
>                 amdgpu_crtc->vsc.full = dfixed_div(a, b);
> @@ -1429,7 +1435,7 @@ bool amdgpu_display_crtc_scaling_mode_fixup(struct drm_crtc *crtc,
>   *
>   * \param dev Device to query.
>   * \param pipe Crtc to query.
> - * \param flags Flags from caller (DRM_CALLED_FROM_VBLIRQ or 0).
> + * \param flags from caller (DRM_CALLED_FROM_VBLIRQ or 0).
>   *              For driver internal use only also supports these flags:
>   *
>   *              USE_REAL_VBLANKSTART to use the real start of vblank instead
> @@ -1504,8 +1510,8 @@ int amdgpu_display_get_crtc_scanoutpos(struct drm_device *dev,
>
>         /* Called from driver internal vblank counter query code? */
>         if (flags & GET_DISTANCE_TO_VBLANKSTART) {
> -           /* Caller wants distance from real vbl_start in *hpos */
> -           *hpos = *vpos - vbl_start;
> +               /* Caller wants distance from real vbl_start in *hpos */
> +               *hpos = *vpos - vbl_start;
>         }
>
>         /* Fudge vblank to start a few scanlines earlier to handle the
> @@ -1527,7 +1533,7 @@ int amdgpu_display_get_crtc_scanoutpos(struct drm_device *dev,
>
>         /* In vblank? */
>         if (in_vbl)
> -           ret |= DRM_SCANOUTPOS_IN_VBLANK;
> +               ret |= DRM_SCANOUTPOS_IN_VBLANK;
>
>         /* Called from driver internal vblank counter query code? */
>         if (flags & GET_DISTANCE_TO_VBLANKSTART) {
> @@ -1635,6 +1641,7 @@ int amdgpu_display_suspend_helper(struct amdgpu_device *adev)
>
>                 if (amdgpu_crtc->cursor_bo && !adev->enable_virtual_display) {
>                         struct amdgpu_bo *aobj = gem_to_amdgpu_bo(amdgpu_crtc->cursor_bo);
> +
>                         r = amdgpu_bo_reserve(aobj, true);
>                         if (r == 0) {
>                                 amdgpu_bo_unpin(aobj);
> @@ -1642,9 +1649,9 @@ int amdgpu_display_suspend_helper(struct amdgpu_device *adev)
>                         }
>                 }
>
> -               if (fb == NULL || fb->obj[0] == NULL) {
> +               if (!fb || !fb->obj[0])
>                         continue;
> -               }
> +
>                 robj = gem_to_amdgpu_bo(fb->obj[0]);
>                 if (!amdgpu_display_robj_is_fb(adev, robj)) {
>                         r = amdgpu_bo_reserve(robj, true);
> @@ -1671,6 +1678,7 @@ int amdgpu_display_resume_helper(struct amdgpu_device *adev)
>
>                 if (amdgpu_crtc->cursor_bo && !adev->enable_virtual_display) {
>                         struct amdgpu_bo *aobj = gem_to_amdgpu_bo(amdgpu_crtc->cursor_bo);
> +
>                         r = amdgpu_bo_reserve(aobj, true);
>                         if (r == 0) {
>                                 r = amdgpu_bo_pin(aobj, AMDGPU_GEM_DOMAIN_VRAM);
> --
> 2.25.1
>


More information about the amd-gfx mailing list