[intel-gvt-linux:gvt-staging 1991/2009] drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:4832:10: warning: assignment makes integer from pointer without a cast

Mario Kleiner mario.kleiner.de at gmail.com
Mon Mar 4 22:05:12 UTC 2019


Ok,

sorry for the late replies. My mail client somehow didn't show me all
the conversations around this patch over the weekend :-/.

I had a look at the various trees wrt. to the patch "drm/amd/display:
Use vrr friendly pageflip throttling in DC."

We apparently have 3 different versions of the patch, modified for
three different ways that amdgpu DC looks depending on 5.0/5.1/5.2 as
there has been a lot of refactoring, and then some mixups in which
patch needs to end up in which branch :-/

The patch in Linux 5.0.0 and current drm-fixes is correct, so the
released kernel is fine.

The patch in current drm-next (presumably pulled from
agd5f/drm-next-5.2-wip) and current agd5f/drm-next-5.2-wip and
drm-next-5.1-wip and drm-next-5.1 is *wrong*. Strangely it doesn't
correspond to the patch i sent out to the mailing list, and that was
reviewed-by and tested by Nicholas, which was correct.

This is how it looks in above branches:

+               target = (uint32_t)last_flip_vblank + wait_for_vblank;

This is how it should look:

+               target = (uint32_t)last_flip_vblank + *wait_for_vblank;

The patch in Alex current agd5f/amd-staging-drm-next is correct again.

It looks like the mistake comes from pulling the patch unmodified from
amd-staging-drm-next into drm-next-5.1-wip, drm-next-5.1,
drm-next-5.2-wip and then into drm-next. There's been big changes in
the surrounding code between 5.0 and 5.1/5.2-wip and then again a
smaller change in amd-staging-drm-next, so that the patch that is
correct in the context of amd-staging-drm-next is wrong in the context
of the 5.1/5.2/drm-next trees. The change for amd-staging-drm-next was
changing the function parameter bool *wait_for_vblank into bool
wait_for_vblank.

But then, the patch that will eventually hopefully go into 5.2
together with proper fixes for the vblank/pageflip timestampin in VRR
mode, looks again different from the one in amd-staging-drm-next, as
it needs to be adapted to the different trigger point for "vblank"
interrupts at start of the back-porch of vblank instead of start of
front-porch of vblank. This latest version is only on my disc driver
atm.

So i think we need above fix for Linux 5.1/drm-next, to be superseded
for the 5.2 merge window by what is now in
agd5f/amdgpu-staging-drm-next, which will get again replaced by the
newest patch asap. and hopefully for the 5.2 merge window.

Not sure what the proper flow for correcting this is? But my original
patch on the ML should be the correct resolution for Linux 5.1, as
that has:

+               target = (uint32_t)last_flip_vblank + *wait_for_vblank;

-mario

On Mon, Mar 4, 2019 at 10:55 AM kbuild test robot <lkp at intel.com> wrote:
>
> tree:   https://github.com/intel/gvt-linux gvt-staging
> head:   f3426c668cc5b6b94133ddb4fe60eb91061993c3
> commit: 289aabbf7bc83a0a268e6e4b24adb0cb9f75d780 [1991/2009] drm/amd/display: Use vrr friendly pageflip throttling in DC.
> config: i386-randconfig-m3-201909 (attached as .config)
> compiler: gcc-7 (Debian 7.4.0-5) 7.4.0
> reproduce:
>         git checkout 289aabbf7bc83a0a268e6e4b24adb0cb9f75d780
>         # save the attached .config to linux build tree
>         make ARCH=i386
>
> All warnings (new ones prefixed by >>):
>
>    drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In function 'amdgpu_dm_commit_planes':
> >> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:4832:10: warning: assignment makes integer from pointer without a cast [-Wint-conversion]
>       target = (uint32_t)last_flip_vblank + wait_for_vblank;
>              ^
>
> vim +4832 drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c
>
>   4625
>   4626  static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
>   4627                                      struct dc_state *dc_state,
>   4628                                      struct drm_device *dev,
>   4629                                      struct amdgpu_display_manager *dm,
>   4630                                      struct drm_crtc *pcrtc,
>   4631                                      bool *wait_for_vblank)
>   4632  {
>   4633          uint32_t i, r;
>   4634          uint64_t timestamp_ns;
>   4635          struct drm_plane *plane;
>   4636          struct drm_plane_state *old_plane_state, *new_plane_state;
>   4637          struct amdgpu_crtc *acrtc_attach = to_amdgpu_crtc(pcrtc);
>   4638          struct drm_crtc_state *new_pcrtc_state =
>   4639                          drm_atomic_get_new_crtc_state(state, pcrtc);
>   4640          struct dm_crtc_state *acrtc_state = to_dm_crtc_state(new_pcrtc_state);
>   4641          struct dm_crtc_state *dm_old_crtc_state =
>   4642                          to_dm_crtc_state(drm_atomic_get_old_crtc_state(state, pcrtc));
>   4643          int flip_count = 0, planes_count = 0, vpos, hpos;
>   4644          unsigned long flags;
>   4645          struct amdgpu_bo *abo;
>   4646          uint64_t tiling_flags, dcc_address;
>   4647          uint32_t target, target_vblank;
>   4648          uint64_t last_flip_vblank;
>   4649          bool vrr_active = acrtc_state->freesync_config.state == VRR_STATE_ACTIVE_VARIABLE;
>   4650
>   4651          struct {
>   4652                  struct dc_surface_update surface_updates[MAX_SURFACES];
>   4653                  struct dc_flip_addrs flip_addrs[MAX_SURFACES];
>   4654                  struct dc_stream_update stream_update;
>   4655          } *flip;
>   4656
>   4657          struct {
>   4658                  struct dc_surface_update surface_updates[MAX_SURFACES];
>   4659                  struct dc_plane_info plane_infos[MAX_SURFACES];
>   4660                  struct dc_scaling_info scaling_infos[MAX_SURFACES];
>   4661                  struct dc_stream_update stream_update;
>   4662          } *full;
>   4663
>   4664          flip = kzalloc(sizeof(*flip), GFP_KERNEL);
>   4665          full = kzalloc(sizeof(*full), GFP_KERNEL);
>   4666
>   4667          if (!flip || !full) {
>   4668                  dm_error("Failed to allocate update bundles\n");
>   4669                  goto cleanup;
>   4670          }
>   4671
>   4672          /* update planes when needed */
>   4673          for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) {
>   4674                  struct drm_crtc *crtc = new_plane_state->crtc;
>   4675                  struct drm_crtc_state *new_crtc_state;
>   4676                  struct drm_framebuffer *fb = new_plane_state->fb;
>   4677                  struct amdgpu_framebuffer *afb = to_amdgpu_framebuffer(fb);
>   4678                  bool pflip_needed;
>   4679                  struct dc_plane_state *dc_plane;
>   4680                  struct dm_plane_state *dm_new_plane_state = to_dm_plane_state(new_plane_state);
>   4681
>   4682                  /* Cursor plane is handled after stream updates */
>   4683                  if (plane->type == DRM_PLANE_TYPE_CURSOR)
>   4684                          continue;
>   4685
>   4686                  if (!fb || !crtc || pcrtc != crtc)
>   4687                          continue;
>   4688
>   4689                  new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
>   4690                  if (!new_crtc_state->active)
>   4691                          continue;
>   4692
>   4693                  pflip_needed = old_plane_state->fb &&
>   4694                          old_plane_state->fb != new_plane_state->fb;
>   4695
>   4696                  dc_plane = dm_new_plane_state->dc_state;
>   4697
>   4698                  if (pflip_needed) {
>   4699                          /*
>   4700                           * Assume even ONE crtc with immediate flip means
>   4701                           * entire can't wait for VBLANK
>   4702                           * TODO Check if it's correct
>   4703                           */
>   4704                          if (new_pcrtc_state->pageflip_flags & DRM_MODE_PAGE_FLIP_ASYNC)
>   4705                                  *wait_for_vblank = false;
>   4706
>   4707                          /*
>   4708                           * TODO This might fail and hence better not used, wait
>   4709                           * explicitly on fences instead
>   4710                           * and in general should be called for
>   4711                           * blocking commit to as per framework helpers
>   4712                           */
>   4713                          abo = gem_to_amdgpu_bo(fb->obj[0]);
>   4714                          r = amdgpu_bo_reserve(abo, true);
>   4715                          if (unlikely(r != 0))
>   4716                                  DRM_ERROR("failed to reserve buffer before flip\n");
>   4717
>   4718                          /*
>   4719                           * Wait for all fences on this FB. Do limited wait to avoid
>   4720                           * deadlock during GPU reset when this fence will not signal
>   4721                           * but we hold reservation lock for the BO.
>   4722                           */
>   4723                          r = reservation_object_wait_timeout_rcu(abo->tbo.resv,
>   4724                                                                  true, false,
>   4725                                                                  msecs_to_jiffies(5000));
>   4726                          if (unlikely(r == 0))
>   4727                                  DRM_ERROR("Waiting for fences timed out.");
>   4728
>   4729
>   4730
>   4731                          amdgpu_bo_get_tiling_flags(abo, &tiling_flags);
>   4732
>   4733                          amdgpu_bo_unreserve(abo);
>   4734
>   4735                          flip->flip_addrs[flip_count].address.grph.addr.low_part = lower_32_bits(afb->address);
>   4736                          flip->flip_addrs[flip_count].address.grph.addr.high_part = upper_32_bits(afb->address);
>   4737
>   4738                          dcc_address = get_dcc_address(afb->address, tiling_flags);
>   4739                          flip->flip_addrs[flip_count].address.grph.meta_addr.low_part = lower_32_bits(dcc_address);
>   4740                          flip->flip_addrs[flip_count].address.grph.meta_addr.high_part = upper_32_bits(dcc_address);
>   4741
>   4742                          flip->flip_addrs[flip_count].flip_immediate =
>   4743                                          (crtc->state->pageflip_flags & DRM_MODE_PAGE_FLIP_ASYNC) != 0;
>   4744
>   4745                          timestamp_ns = ktime_get_ns();
>   4746                          flip->flip_addrs[flip_count].flip_timestamp_in_us = div_u64(timestamp_ns, 1000);
>   4747                          flip->surface_updates[flip_count].flip_addr = &flip->flip_addrs[flip_count];
>   4748                          flip->surface_updates[flip_count].surface = dc_plane;
>   4749
>   4750                          if (!flip->surface_updates[flip_count].surface) {
>   4751                                  DRM_ERROR("No surface for CRTC: id=%d\n",
>   4752                                                  acrtc_attach->crtc_id);
>   4753                                  continue;
>   4754                          }
>   4755
>   4756                          if (plane == pcrtc->primary)
>   4757                                  update_freesync_state_on_stream(
>   4758                                          dm,
>   4759                                          acrtc_state,
>   4760                                          acrtc_state->stream,
>   4761                                          dc_plane,
>   4762                                          flip->flip_addrs[flip_count].flip_timestamp_in_us);
>   4763
>   4764                          DRM_DEBUG_DRIVER("%s Flipping to hi: 0x%x, low: 0x%x\n",
>   4765                                           __func__,
>   4766                                           flip->flip_addrs[flip_count].address.grph.addr.high_part,
>   4767                                           flip->flip_addrs[flip_count].address.grph.addr.low_part);
>   4768
>   4769                          flip_count += 1;
>   4770                  }
>   4771
>   4772                  full->surface_updates[planes_count].surface = dc_plane;
>   4773                  if (new_pcrtc_state->color_mgmt_changed) {
>   4774                          full->surface_updates[planes_count].gamma = dc_plane->gamma_correction;
>   4775                          full->surface_updates[planes_count].in_transfer_func = dc_plane->in_transfer_func;
>   4776                  }
>   4777
>   4778
>   4779                  full->scaling_infos[planes_count].scaling_quality = dc_plane->scaling_quality;
>   4780                  full->scaling_infos[planes_count].src_rect = dc_plane->src_rect;
>   4781                  full->scaling_infos[planes_count].dst_rect = dc_plane->dst_rect;
>   4782                  full->scaling_infos[planes_count].clip_rect = dc_plane->clip_rect;
>   4783                  full->surface_updates[planes_count].scaling_info = &full->scaling_infos[planes_count];
>   4784
>   4785
>   4786                  full->plane_infos[planes_count].color_space = dc_plane->color_space;
>   4787                  full->plane_infos[planes_count].format = dc_plane->format;
>   4788                  full->plane_infos[planes_count].plane_size = dc_plane->plane_size;
>   4789                  full->plane_infos[planes_count].rotation = dc_plane->rotation;
>   4790                  full->plane_infos[planes_count].horizontal_mirror = dc_plane->horizontal_mirror;
>   4791                  full->plane_infos[planes_count].stereo_format = dc_plane->stereo_format;
>   4792                  full->plane_infos[planes_count].tiling_info = dc_plane->tiling_info;
>   4793                  full->plane_infos[planes_count].visible = dc_plane->visible;
>   4794                  full->plane_infos[planes_count].per_pixel_alpha = dc_plane->per_pixel_alpha;
>   4795                  full->plane_infos[planes_count].dcc = dc_plane->dcc;
>   4796                  full->surface_updates[planes_count].plane_info = &full->plane_infos[planes_count];
>   4797
>   4798                  planes_count += 1;
>   4799
>   4800          }
>   4801
>   4802          /*
>   4803           * TODO: For proper atomic behaviour, we should be calling into DC once with
>   4804           * all the changes.  However, DC refuses to do pageflips and non-pageflip
>   4805           * changes in the same call.  Change DC to respect atomic behaviour,
>   4806           * hopefully eliminating dc_*_update structs in their entirety.
>   4807           */
>   4808          if (flip_count) {
>   4809                  if (!vrr_active) {
>   4810                          /* Use old throttling in non-vrr fixed refresh rate mode
>   4811                           * to keep flip scheduling based on target vblank counts
>   4812                           * working in a backwards compatible way, e.g., for
>   4813                           * clients using the GLX_OML_sync_control extension or
>   4814                           * DRI3/Present extension with defined target_msc.
>   4815                           */
>   4816                          last_flip_vblank = drm_crtc_vblank_count(pcrtc);
>   4817                  }
>   4818                  else {
>   4819                          /* For variable refresh rate mode only:
>   4820                           * Get vblank of last completed flip to avoid > 1 vrr
>   4821                           * flips per video frame by use of throttling, but allow
>   4822                           * flip programming anywhere in the possibly large
>   4823                           * variable vrr vblank interval for fine-grained flip
>   4824                           * timing control and more opportunity to avoid stutter
>   4825                           * on late submission of flips.
>   4826                           */
>   4827                          spin_lock_irqsave(&pcrtc->dev->event_lock, flags);
>   4828                          last_flip_vblank = acrtc_attach->last_flip_vblank;
>   4829                          spin_unlock_irqrestore(&pcrtc->dev->event_lock, flags);
>   4830                  }
>   4831
> > 4832                  target = (uint32_t)last_flip_vblank + wait_for_vblank;
>   4833
>   4834                  /* Prepare wait for target vblank early - before the fence-waits */
>   4835                  target_vblank = target - (uint32_t)drm_crtc_vblank_count(pcrtc) +
>   4836                                  amdgpu_get_vblank_counter_kms(pcrtc->dev, acrtc_attach->crtc_id);
>   4837
>   4838                  /*
>   4839                   * Wait until we're out of the vertical blank period before the one
>   4840                   * targeted by the flip
>   4841                   */
>   4842                  while ((acrtc_attach->enabled &&
>   4843                          (amdgpu_display_get_crtc_scanoutpos(dm->ddev, acrtc_attach->crtc_id,
>   4844                                                              0, &vpos, &hpos, NULL,
>   4845                                                              NULL, &pcrtc->hwmode)
>   4846                           & (DRM_SCANOUTPOS_VALID | DRM_SCANOUTPOS_IN_VBLANK)) ==
>   4847                          (DRM_SCANOUTPOS_VALID | DRM_SCANOUTPOS_IN_VBLANK) &&
>   4848                          (int)(target_vblank -
>   4849                            amdgpu_get_vblank_counter_kms(dm->ddev, acrtc_attach->crtc_id)) > 0)) {
>   4850                          usleep_range(1000, 1100);
>   4851                  }
>   4852
>   4853                  if (acrtc_attach->base.state->event) {
>   4854                          drm_crtc_vblank_get(pcrtc);
>   4855
>   4856                          spin_lock_irqsave(&pcrtc->dev->event_lock, flags);
>   4857
>   4858                          WARN_ON(acrtc_attach->pflip_status != AMDGPU_FLIP_NONE);
>   4859                          prepare_flip_isr(acrtc_attach);
>   4860
>   4861                          spin_unlock_irqrestore(&pcrtc->dev->event_lock, flags);
>   4862                  }
>   4863
>   4864                  if (acrtc_state->stream) {
>   4865
>   4866                          if (acrtc_state->freesync_timing_changed)
>   4867                                  flip->stream_update.adjust =
>   4868                                          &acrtc_state->stream->adjust;
>   4869
>   4870                          if (acrtc_state->freesync_vrr_info_changed)
>   4871                                  flip->stream_update.vrr_infopacket =
>   4872                                          &acrtc_state->stream->vrr_infopacket;
>   4873                  }
>   4874
>   4875                  mutex_lock(&dm->dc_lock);
>   4876                  dc_commit_updates_for_stream(dm->dc,
>   4877                                                       flip->surface_updates,
>   4878                                                       flip_count,
>   4879                                                       acrtc_state->stream,
>   4880                                                       &flip->stream_update,
>   4881                                                       dc_state);
>   4882                  mutex_unlock(&dm->dc_lock);
>   4883          }
>   4884
>   4885          if (planes_count) {
>   4886                  if (new_pcrtc_state->mode_changed) {
>   4887                          full->stream_update.src = acrtc_state->stream->src;
>   4888                          full->stream_update.dst = acrtc_state->stream->dst;
>   4889                  }
>   4890
>   4891                  if (new_pcrtc_state->color_mgmt_changed)
>   4892                          full->stream_update.out_transfer_func = acrtc_state->stream->out_transfer_func;
>   4893
>   4894                  acrtc_state->stream->abm_level = acrtc_state->abm_level;
>   4895                  if (acrtc_state->abm_level != dm_old_crtc_state->abm_level)
>   4896                          full->stream_update.abm_level = &acrtc_state->abm_level;
>   4897
>   4898                  mutex_lock(&dm->dc_lock);
>   4899                  dc_commit_updates_for_stream(dm->dc,
>   4900                                                       full->surface_updates,
>   4901                                                       planes_count,
>   4902                                                       acrtc_state->stream,
>   4903                                                       &full->stream_update,
>   4904                                                       dc_state);
>   4905                  mutex_unlock(&dm->dc_lock);
>   4906          }
>   4907
>   4908          for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i)
>   4909                  if (plane->type == DRM_PLANE_TYPE_CURSOR)
>   4910                          handle_cursor_update(plane, old_plane_state);
>   4911
>   4912  cleanup:
>   4913          kfree(flip);
>   4914          kfree(full);
>   4915  }
>   4916
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation


More information about the intel-gvt-dev mailing list