All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alex Deucher <alexdeucher@gmail.com>
To: "Christian König" <deathsimple@vodafone.de>
Cc: Maling list - DRI developers <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH 2/3] drm/radeon: add readonly flag to radeon_gart_set_page
Date: Wed, 9 Jul 2014 16:01:11 -0400	[thread overview]
Message-ID: <CADnq5_OdSMPzqNcq20_M2M_Veu8Sn1EhV1s4XuuKBjcgUg3pDg@mail.gmail.com> (raw)
In-Reply-To: <1404929744-8630-2-git-send-email-deathsimple@vodafone.de>

On Wed, Jul 9, 2014 at 2:15 PM, Christian König <deathsimple@vodafone.de> wrote:
> From: Christian König <christian.koenig@amd.com>
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/radeon/r100.c        | 2 +-
>  drivers/gpu/drm/radeon/r300.c        | 8 ++++++--
>  drivers/gpu/drm/radeon/radeon.h      | 8 ++++----
>  drivers/gpu/drm/radeon/radeon_asic.h | 8 ++++----
>  drivers/gpu/drm/radeon/radeon_gart.c | 4 ++--
>  drivers/gpu/drm/radeon/rs400.c       | 9 +++++++--
>  drivers/gpu/drm/radeon/rs600.c       | 8 ++++++--
>  7 files changed, 30 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
> index ed1c53e..bffbb14 100644
> --- a/drivers/gpu/drm/radeon/r100.c
> +++ b/drivers/gpu/drm/radeon/r100.c
> @@ -682,7 +682,7 @@ void r100_pci_gart_disable(struct radeon_device *rdev)
>  }
>
>  void r100_pci_gart_set_page(struct radeon_device *rdev, unsigned i,
> -                           uint64_t addr)
> +                           uint64_t addr, bool readonly)
>  {
>         u32 *gtt = rdev->gart.ptr;
>         gtt[i] = cpu_to_le32(lower_32_bits(addr));
> diff --git a/drivers/gpu/drm/radeon/r300.c b/drivers/gpu/drm/radeon/r300.c
> index 8d14e66..2ea3d29 100644
> --- a/drivers/gpu/drm/radeon/r300.c
> +++ b/drivers/gpu/drm/radeon/r300.c
> @@ -73,13 +73,17 @@ void rv370_pcie_gart_tlb_flush(struct radeon_device *rdev)
>  #define R300_PTE_READABLE  (1 << 3)
>
>  void rv370_pcie_gart_set_page(struct radeon_device *rdev, unsigned i,
> -                             uint64_t addr)
> +                             uint64_t addr, bool readonly)
>  {
>         void __iomem *ptr = rdev->gart.ptr;
>
>         addr = (lower_32_bits(addr) >> 8) |
>                ((upper_32_bits(addr) & 0xff) << 24) |
> -              R300_PTE_WRITEABLE | R300_PTE_READABLE;
> +              R300_PTE_READABLE;
> +
> +       if (!readonly)
> +               addr |= R300_PTE_WRITEABLE;
> +
>         /* on x86 we want this to be CPU endian, on powerpc
>          * on powerpc without HW swappers, it'll get swapped on way
>          * into VRAM - so no need for cpu_to_le32 on VRAM tables */
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index c16652a..283b496 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -855,8 +855,8 @@ struct radeon_mec {
>  #define R600_PTE_FRAG_256KB    (6 << 7)
>
>  /* flags used for GART page table entries on R600+ */
> -#define R600_PTE_GART  ( R600_PTE_VALID | R600_PTE_SYSTEM | R600_PTE_SNOOPED \
> -                       | R600_PTE_READABLE | R600_PTE_WRITEABLE)
> +#define R600_PTE_GART  ( R600_PTE_VALID | R600_PTE_SYSTEM | \
> +                         R600_PTE_SNOOPED | R600_PTE_READABLE )
>
>  struct radeon_vm_pt {
>         struct radeon_bo                *bo;
> @@ -1776,7 +1776,7 @@ struct radeon_asic {
>         struct {
>                 void (*tlb_flush)(struct radeon_device *rdev);
>                 void (*set_page)(struct radeon_device *rdev, unsigned i,
> -                                uint64_t addr);
> +                                uint64_t addr, bool readonly);


I think it would be better to add a page_flags parameter rather than a
boolean for each attribute.  At some point we may want write-only or
non-snooped.

Alex

>         } gart;
>         struct {
>                 int (*init)(struct radeon_device *rdev);
> @@ -2703,7 +2703,7 @@ void radeon_ring_write(struct radeon_ring *ring, uint32_t v);
>  #define radeon_vga_set_state(rdev, state) (rdev)->asic->vga_set_state((rdev), (state))
>  #define radeon_asic_reset(rdev) (rdev)->asic->asic_reset((rdev))
>  #define radeon_gart_tlb_flush(rdev) (rdev)->asic->gart.tlb_flush((rdev))
> -#define radeon_gart_set_page(rdev, i, p) (rdev)->asic->gart.set_page((rdev), (i), (p))
> +#define radeon_gart_set_page(rdev, i, p, r) (rdev)->asic->gart.set_page((rdev), (i), (p), (r))
>  #define radeon_asic_vm_init(rdev) (rdev)->asic->vm.init((rdev))
>  #define radeon_asic_vm_fini(rdev) (rdev)->asic->vm.fini((rdev))
>  #define radeon_asic_vm_set_page(rdev, ib, pe, addr, count, incr, flags) ((rdev)->asic->vm.set_page((rdev), (ib), (pe), (addr), (count), (incr), (flags)))
> diff --git a/drivers/gpu/drm/radeon/radeon_asic.h b/drivers/gpu/drm/radeon/radeon_asic.h
> index 01e7c0a..1e0cb0c2 100644
> --- a/drivers/gpu/drm/radeon/radeon_asic.h
> +++ b/drivers/gpu/drm/radeon/radeon_asic.h
> @@ -68,7 +68,7 @@ int r100_asic_reset(struct radeon_device *rdev);
>  u32 r100_get_vblank_counter(struct radeon_device *rdev, int crtc);
>  void r100_pci_gart_tlb_flush(struct radeon_device *rdev);
>  void r100_pci_gart_set_page(struct radeon_device *rdev, unsigned i,
> -                           uint64_t addr);
> +                           uint64_t addr, bool readonly);
>  void r100_ring_start(struct radeon_device *rdev, struct radeon_ring *ring);
>  int r100_irq_set(struct radeon_device *rdev);
>  int r100_irq_process(struct radeon_device *rdev);
> @@ -173,7 +173,7 @@ extern void r300_fence_ring_emit(struct radeon_device *rdev,
>  extern int r300_cs_parse(struct radeon_cs_parser *p);
>  extern void rv370_pcie_gart_tlb_flush(struct radeon_device *rdev);
>  extern void rv370_pcie_gart_set_page(struct radeon_device *rdev, unsigned i,
> -                                    uint64_t addr);
> +                                    uint64_t addr, bool readonly);
>  extern void rv370_set_pcie_lanes(struct radeon_device *rdev, int lanes);
>  extern int rv370_get_pcie_lanes(struct radeon_device *rdev);
>  extern void r300_set_reg_safe(struct radeon_device *rdev);
> @@ -209,7 +209,7 @@ extern int rs400_suspend(struct radeon_device *rdev);
>  extern int rs400_resume(struct radeon_device *rdev);
>  void rs400_gart_tlb_flush(struct radeon_device *rdev);
>  void rs400_gart_set_page(struct radeon_device *rdev, unsigned i,
> -                        uint64_t addr);
> +                        uint64_t addr, bool readonly);
>  uint32_t rs400_mc_rreg(struct radeon_device *rdev, uint32_t reg);
>  void rs400_mc_wreg(struct radeon_device *rdev, uint32_t reg, uint32_t v);
>  int rs400_gart_init(struct radeon_device *rdev);
> @@ -233,7 +233,7 @@ void rs600_irq_disable(struct radeon_device *rdev);
>  u32 rs600_get_vblank_counter(struct radeon_device *rdev, int crtc);
>  void rs600_gart_tlb_flush(struct radeon_device *rdev);
>  void rs600_gart_set_page(struct radeon_device *rdev, unsigned i,
> -                        uint64_t addr);
> +                        uint64_t addr, bool readonly);
>  uint32_t rs600_mc_rreg(struct radeon_device *rdev, uint32_t reg);
>  void rs600_mc_wreg(struct radeon_device *rdev, uint32_t reg, uint32_t v);
>  void rs600_bandwidth_update(struct radeon_device *rdev);
> diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c
> index b7d3e84..3b80eac 100644
> --- a/drivers/gpu/drm/radeon/radeon_gart.c
> +++ b/drivers/gpu/drm/radeon/radeon_gart.c
> @@ -243,7 +243,7 @@ void radeon_gart_unbind(struct radeon_device *rdev, unsigned offset,
>                         page_base = rdev->gart.pages_addr[p];
>                         for (j = 0; j < (PAGE_SIZE / RADEON_GPU_PAGE_SIZE); j++, t++) {
>                                 if (rdev->gart.ptr) {
> -                                       radeon_gart_set_page(rdev, t, page_base);
> +                                       radeon_gart_set_page(rdev, t, page_base, false);
>                                 }
>                                 page_base += RADEON_GPU_PAGE_SIZE;
>                         }
> @@ -287,7 +287,7 @@ int radeon_gart_bind(struct radeon_device *rdev, unsigned offset,
>                 if (rdev->gart.ptr) {
>                         page_base = rdev->gart.pages_addr[p];
>                         for (j = 0; j < (PAGE_SIZE / RADEON_GPU_PAGE_SIZE); j++, t++) {
> -                               radeon_gart_set_page(rdev, t, page_base);
> +                               radeon_gart_set_page(rdev, t, page_base, false);
>                                 page_base += RADEON_GPU_PAGE_SIZE;
>                         }
>                 }
> diff --git a/drivers/gpu/drm/radeon/rs400.c b/drivers/gpu/drm/radeon/rs400.c
> index 4519f9c..9dfb617 100644
> --- a/drivers/gpu/drm/radeon/rs400.c
> +++ b/drivers/gpu/drm/radeon/rs400.c
> @@ -211,14 +211,19 @@ void rs400_gart_fini(struct radeon_device *rdev)
>  #define RS400_PTE_WRITEABLE (1 << 2)
>  #define RS400_PTE_READABLE  (1 << 3)
>
> -void rs400_gart_set_page(struct radeon_device *rdev, unsigned i, uint64_t addr)
> +void rs400_gart_set_page(struct radeon_device *rdev, unsigned i,
> +                        uint64_t addr, bool readonly)
>  {
>         uint32_t entry;
>         u32 *gtt = rdev->gart.ptr;
>
>         entry = (lower_32_bits(addr) & PAGE_MASK) |
>                 ((upper_32_bits(addr) & 0xff) << 4) |
> -               RS400_PTE_WRITEABLE | RS400_PTE_READABLE;
> +               RS400_PTE_READABLE;
> +
> +       if (!readonly)
> +               entry |= RS400_PTE_WRITEABLE;
> +
>         entry = cpu_to_le32(entry);
>         gtt[i] = entry;
>  }
> diff --git a/drivers/gpu/drm/radeon/rs600.c b/drivers/gpu/drm/radeon/rs600.c
> index 27a56ad..0bbb1ab 100644
> --- a/drivers/gpu/drm/radeon/rs600.c
> +++ b/drivers/gpu/drm/radeon/rs600.c
> @@ -625,15 +625,19 @@ static void rs600_gart_fini(struct radeon_device *rdev)
>         radeon_gart_table_vram_free(rdev);
>  }
>
> -void rs600_gart_set_page(struct radeon_device *rdev, unsigned i, uint64_t addr)
> +void rs600_gart_set_page(struct radeon_device *rdev, unsigned i,
> +                        uint64_t addr, bool readonly)
>  {
>         void __iomem *ptr = (void *)rdev->gart.ptr;
>
>         addr = addr & 0xFFFFFFFFFFFFF000ULL;
>         if (addr == rdev->dummy_page.addr)
>                 addr |= R600_PTE_SYSTEM | R600_PTE_SNOOPED;
> -       else
> +       else {
>                 addr |= R600_PTE_GART;
> +               if (!readonly)
> +                       addr |= R600_PTE_WRITEABLE;
> +       }
>         writeq(addr, ptr + (i * 8));
>  }
>
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2014-07-09 20:01 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-09 18:15 [PATCH 1/3] drm/radeon: Remove radeon_gart_restore() Christian König
2014-07-09 18:15 ` [PATCH 2/3] drm/radeon: add readonly flag to radeon_gart_set_page Christian König
2014-07-09 20:01   ` Alex Deucher [this message]
2014-07-10 10:59     ` Christian König
2014-07-09 18:15 ` [PATCH 3/3] drm/radeon: add user pointer support v3 Christian König
2014-07-09 19:58 ` [PATCH 1/3] drm/radeon: Remove radeon_gart_restore() Alex Deucher

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CADnq5_OdSMPzqNcq20_M2M_Veu8Sn1EhV1s4XuuKBjcgUg3pDg@mail.gmail.com \
    --to=alexdeucher@gmail.com \
    --cc=deathsimple@vodafone.de \
    --cc=dri-devel@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.