[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
David Airlie
airlied at redhat.com
Mon Mar 4 22:11:53 UTC 2019
On Tue, Mar 5, 2019 at 8:05 AM Mario Kleiner <mario.kleiner.de at gmail.com> wrote:
>
> 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.
Probably my fault, Alex said the patch in his drm-next-5.2-wip was the
one, I pulled it but didn't go back and check for any mailing list
discussion.
I'll make this change in drm-next, I'm trying to decide if it's worth
force pushing it.
Dave.
>
> 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