[Intel-gfx] [PATCH v1] drm/i915: Refactor setting dma info to a common helper

[Chris Wilson]

Quoting Ruhl, Michael J (2020-04-17 20:31:52)
> >-----Original Message-----
> >From: Chris Wilson <chris at chris-wilson.co.uk>
> >> diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
> >> index 3d42fc4290bc..4b34a5195c65 100644
> >> --- a/drivers/char/agp/intel-gtt.c
> >> +++ b/drivers/char/agp/intel-gtt.c
> >> @@ -1407,13 +1407,16 @@ int intel_gmch_probe(struct pci_dev
> >*bridge_pdev, struct pci_dev *gpu_pdev,
> >>
> >>         dev_info(&bridge_pdev->dev, "Intel %s Chipset\n",
> >intel_gtt_chipsets[i].name);
> >>
> >> -       mask = intel_private.driver->dma_mask_size;
> >> -       if (pci_set_dma_mask(intel_private.pcidev, DMA_BIT_MASK(mask)))
> >> -               dev_err(&intel_private.pcidev->dev,
> >> -                       "set gfx device dma mask %d-bit failed!\n", mask);
> >> -       else
> >> -               pci_set_consistent_dma_mask(intel_private.pcidev,
> >> -                                           DMA_BIT_MASK(mask));
> >> +       if (bridge) {
> >> +               mask = intel_private.driver->dma_mask_size;
> >> +               if (pci_set_dma_mask(intel_private.pcidev,
> >DMA_BIT_MASK(mask)))
> >> +                       dev_err(&intel_private.pcidev->dev,
> >> +                               "set gfx device dma mask %d-bit failed!\n",
> >> +                               mask);
> >> +               else
> >> +                       pci_set_consistent_dma_mask(intel_private.pcidev,
> >> +                                                   DMA_BIT_MASK(mask));
> >> +       }
> >
> >This could even go under IS_ENABLED(CONFIG_AGP_INTEL)
> 
> I was going to, but then I was going to have to add:
> 
> #if IS_ENABLED(CONFIG_AGP_INTEL)
>         int mask;
> #endif
> 
> as well...so I stopped while I was a ahead.  If you would like the #if I will
> add it.

Nope. If it ends up making the code much harder to read, then its not
worth the hassle.

> >> @@ -685,6 +698,7 @@ static const struct intel_device_info skl_gt4_info = {
> >>         .has_logical_ring_contexts = 1, \
> >>         .has_logical_ring_preemption = 1, \
> >>         .has_gt_uc = 1, \
> >> +       .dma_mask_size = 39, \
> >>         .ppgtt_type = INTEL_PPGTT_FULL, \
> >>         .ppgtt_size = 48, \
> >>         .has_reset_engine = 1, \
> >
> >Diff making a hash of this file again, but they all looked correct.
> >
> >> diff --git a/drivers/gpu/drm/i915/intel_device_info.h
> >b/drivers/gpu/drm/i915/intel_device_info.h
> >> index cce6a72c5ebc..69c9257c6c6a 100644
> >> --- a/drivers/gpu/drm/i915/intel_device_info.h
> >> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> >> @@ -158,6 +158,8 @@ struct intel_device_info {
> >>
> >>         enum intel_platform platform;
> >>
> >> +       unsigned int dma_mask_size; /* available DMA address bits */
> >
> >One day we should pack this struct and see how much .data we can save.
> 
> Hmm, this value could be a u8. However, placement for optimal usage is a
> bit more difficult...
> 
> I can update if you would like.

Rainy day task. To make any savings we need to look at all the fields
and pahole. Plus if we do more at once, it feels like we accomplished
something :)
-Chris