All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Deucher, Alexander" <Alexander.Deucher-5C7GfCeVMHo@public.gmane.org>
To: "'Kai Wasserbäch'"
	<kai-1ZKVMVCtJ2dx9oSEVPI0kiST3g8Odh+X@public.gmane.org>,
	"Wentland, Harry" <Harry.Wentland-5C7GfCeVMHo@public.gmane.org>,
	"amd-gfx list"
	<amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Subject: RE: [PATCH v2] drm/amdgpu: Read vram width from integrated system info table
Date: Mon, 3 Apr 2017 15:43:52 +0000	[thread overview]
Message-ID: <BN6PR12MB1652CE82ACBFD409F3829D35F7080@BN6PR12MB1652.namprd12.prod.outlook.com> (raw)
In-Reply-To: <8623a5d4-1e10-fc46-aaa2-0777b500c3a2-1ZKVMVCtJ2dx9oSEVPI0kiST3g8Odh+X@public.gmane.org>

> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Kai Wasserbäch
> Sent: Monday, April 03, 2017 11:20 AM
> To: Wentland, Harry; amd-gfx list
> Subject: Re: [PATCH v2] drm/amdgpu: Read vram width from integrated
> system info table
> 
> Dear Harry,
> Harry Wentland wrote on 03.04.2017 17:01:
> > On KB, KV, CZ we should read the vram width from integrated system
> > table, if we can. The NOOFCHAN in MC_SHARED_CHMAP is not accurate.
> >
> > With this change we can enable two 4k displays on CZ again. This use
> > case was broken sometime in January when we started looking at
> > vram_width for bandwidth calculations instead of hardcoding this value.
> >
> > v2:
> >   Return 0 if integrated system info table is not available.
> >
> > Tested-by: Roman Li <roman.li@amd.com>
> > Signed-off-by: Harry Wentland <harry.wentland@amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 29 ++++++++++
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h |  2 +
> >  drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c        | 87 +++++++++++++++---
> ----------
> >  drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c        | 87 +++++++++++++++---
> ----------
> >  4 files changed, 123 insertions(+), 82 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> > index f52b1bf3d3d9..ad4329922f79 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> > @@ -754,6 +754,35 @@ union igp_info {
> >  	struct _ATOM_INTEGRATED_SYSTEM_INFO_V1_9 info_9;
> >  };
> >
> > +/*
> > + * Return vram width from integrated system info table, if available,
> > + * or 0 if not.
> > + */
> > +int amdgpu_atombios_get_vram_width(struct amdgpu_device *adev)
> > +{
> > +	struct amdgpu_mode_info *mode_info = &adev->mode_info;
> > +	int index = GetIndexIntoMasterTable(DATA, IntegratedSystemInfo);
> > +	u16 data_offset, size;
> > +	union igp_info *igp_info;
> > +	u8 frev, crev;
> > +
> > +	/* get any igp specific overrides */
> > +	if (amdgpu_atom_parse_data_header(mode_info->atom_context,
> index, &size,
> > +				   &frev, &crev, &data_offset)) {
> > +		igp_info = (union igp_info *)
> > +			(mode_info->atom_context->bios + data_offset);
> > +		switch (crev) {
> > +		case 8:
> > +		case 9:
> > +			return igp_info->info_8.ucUMAChannelNumber *
> 64;
> > +		default:
> > +			return 0;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static void amdgpu_atombios_get_igp_ss_overrides(struct
> amdgpu_device *adev,
> >  						 struct amdgpu_atom_ss *ss,
> >  						 int id)
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h
> > index 4e0f488487f3..38d0fe32e5cd 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h
> > @@ -148,6 +148,8 @@ int amdgpu_atombios_get_clock_info(struct
> amdgpu_device *adev);
> >
> >  int amdgpu_atombios_get_gfx_info(struct amdgpu_device *adev);
> >
> > +int amdgpu_atombios_get_vram_width(struct amdgpu_device *adev);
> > +
> >  bool amdgpu_atombios_get_asic_ss_info(struct amdgpu_device *adev,
> >  				      struct amdgpu_atom_ss *ss,
> >  				      int id, u32 clock);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> > index 0c0a6015cca5..78643a1baa5c 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> > @@ -37,6 +37,8 @@
> >  #include "oss/oss_2_0_d.h"
> >  #include "oss/oss_2_0_sh_mask.h"
> >
> > +#include "amdgpu_atombios.h"
> > +
> >  static void gmc_v7_0_set_gart_funcs(struct amdgpu_device *adev);
> >  static void gmc_v7_0_set_irq_funcs(struct amdgpu_device *adev);
> >  static int gmc_v7_0_wait_for_idle(void *handle);
> > @@ -325,48 +327,51 @@ static void gmc_v7_0_mc_program(struct
> amdgpu_device *adev)
> >   */
> >  static int gmc_v7_0_mc_init(struct amdgpu_device *adev)
> >  {
> > -	u32 tmp;
> > -	int chansize, numchan;
> > -
> > -	/* Get VRAM informations */
> > -	tmp = RREG32(mmMC_ARB_RAMCFG);
> > -	if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
> > -		chansize = 64;
> > -	} else {
> > -		chansize = 32;
> > -	}
> > -	tmp = RREG32(mmMC_SHARED_CHMAP);
> > -	switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP, NOOFCHAN)) {
> > -	case 0:
> > -	default:
> > -		numchan = 1;
> > -		break;
> > -	case 1:
> > -		numchan = 2;
> > -		break;
> > -	case 2:
> > -		numchan = 4;
> > -		break;
> > -	case 3:
> > -		numchan = 8;
> > -		break;
> > -	case 4:
> > -		numchan = 3;
> > -		break;
> > -	case 5:
> > -		numchan = 6;
> > -		break;
> > -	case 6:
> > -		numchan = 10;
> > -		break;
> > -	case 7:
> > -		numchan = 12;
> > -		break;
> > -	case 8:
> > -		numchan = 16;
> > -		break;
> > +	adev->mc.vram_width = amdgpu_atombios_get_vram_width(adev);
> > +	if (!adev->mc.vram_width) {
> > +		u32 tmp;
> > +		int chansize, numchan;
> > +
> > +		/* Get VRAM informations */
> > +		tmp = RREG32(mmMC_ARB_RAMCFG);
> > +		if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
> > +			chansize = 64;
> > +		} else {
> > +			chansize = 32;
> > +		}
> > +		tmp = RREG32(mmMC_SHARED_CHMAP);
> > +		switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP,
> NOOFCHAN)) {
> > +		case 0:
> > +		default:
> > +			numchan = 1;
> > +			break;
> > +		case 1:
> > +			numchan = 2;
> > +			break;
> > +		case 2:
> > +			numchan = 4;
> > +			break;
> > +		case 3:
> > +			numchan = 8;
> > +			break;
> > +		case 4:
> > +			numchan = 3;
> > +			break;
> > +		case 5:
> > +			numchan = 6;
> > +			break;
> > +		case 6:
> > +			numchan = 10;
> > +			break;
> > +		case 7:
> > +			numchan = 12;
> > +			break;
> > +		case 8:
> > +			numchan = 16;
> > +			break;
> > +		}
> > +		adev->mc.vram_width = numchan * chansize;
> >  	}
> > -	adev->mc.vram_width = numchan * chansize;
> >  	/* Could aper size report 0 ? */
> >  	adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> >  	adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > index d19d1c5e2847..42b2f357a799 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > @@ -38,6 +38,8 @@
> >  #include "vid.h"
> >  #include "vi.h"
> >
> > +#include "amdgpu_atombios.h"
> > +
> >
> >  static void gmc_v8_0_set_gart_funcs(struct amdgpu_device *adev);
> >  static void gmc_v8_0_set_irq_funcs(struct amdgpu_device *adev);
> > @@ -487,48 +489,51 @@ static void gmc_v8_0_mc_program(struct
> amdgpu_device *adev)
> >   */
> >  static int gmc_v8_0_mc_init(struct amdgpu_device *adev)
> >  {
> > -	u32 tmp;
> > -	int chansize, numchan;
> > -
> > -	/* Get VRAM informations */
> > -	tmp = RREG32(mmMC_ARB_RAMCFG);
> > -	if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
> > -		chansize = 64;
> > -	} else {
> > -		chansize = 32;
> > -	}
> > -	tmp = RREG32(mmMC_SHARED_CHMAP);
> > -	switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP, NOOFCHAN)) {
> > -	case 0:
> > -	default:
> > -		numchan = 1;
> > -		break;
> > -	case 1:
> > -		numchan = 2;
> > -		break;
> > -	case 2:
> > -		numchan = 4;
> > -		break;
> > -	case 3:
> > -		numchan = 8;
> > -		break;
> > -	case 4:
> > -		numchan = 3;
> > -		break;
> > -	case 5:
> > -		numchan = 6;
> > -		break;
> > -	case 6:
> > -		numchan = 10;
> > -		break;
> > -	case 7:
> > -		numchan = 12;
> > -		break;
> > -	case 8:
> > -		numchan = 16;
> > -		break;
> > +	adev->mc.vram_width = amdgpu_atombios_get_vram_width(adev);
> > +	if (!adev->mc.vram_width) {
> > +		u32 tmp;
> > +		int chansize, numchan;
> > +
> > +		/* Get VRAM informations */
> > +		tmp = RREG32(mmMC_ARB_RAMCFG);
> > +		if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
> > +			chansize = 64;
> > +		} else {
> > +			chansize = 32;
> > +		}
> > +		tmp = RREG32(mmMC_SHARED_CHMAP);
> > +		switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP,
> NOOFCHAN)) {
> > +		case 0:
> > +		default:
> > +			numchan = 1;
> > +			break;
> > +		case 1:
> > +			numchan = 2;
> > +			break;
> > +		case 2:
> > +			numchan = 4;
> > +			break;
> > +		case 3:
> > +			numchan = 8;
> > +			break;
> > +		case 4:
> > +			numchan = 3;
> > +			break;
> > +		case 5:
> > +			numchan = 6;
> > +			break;
> > +		case 6:
> > +			numchan = 10;
> > +			break;
> > +		case 7:
> > +			numchan = 12;
> > +			break;
> > +		case 8:
> > +			numchan = 16;
> > +			break;
> > +		}
> > +		adev->mc.vram_width = numchan * chansize;
> 
> I might miss something subtle here, but after reading this a couple of times,
> I've convinced myself this is exactly the same switch statement as in
> gmc_v7_0_mc_init() above, right? If so: why not move that part to common
> code as
> well?

The register offsets and bitfields may change across different IP revisions.  The actual switch statement itself could be common I guess (minus the registers), but I'm not sure if it's worth the effort.

Alex

> 
> Cheers,
> Kai
> 
> 
> >  	}
> > -	adev->mc.vram_width = numchan * chansize;
> >  	/* Could aper size report 0 ? */
> >  	adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> >  	adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

  parent reply	other threads:[~2017-04-03 15:43 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-03 15:01 [PATCH v2] drm/amdgpu: Read vram width from integrated system info table Harry Wentland
     [not found] ` <20170403150107.13253-1-harry.wentland-5C7GfCeVMHo@public.gmane.org>
2017-04-03 15:20   ` Kai Wasserbäch
     [not found]     ` <8623a5d4-1e10-fc46-aaa2-0777b500c3a2-1ZKVMVCtJ2dx9oSEVPI0kiST3g8Odh+X@public.gmane.org>
2017-04-03 15:29       ` Harry Wentland
2017-04-03 15:43       ` Deucher, Alexander [this message]
     [not found]         ` <BN6PR12MB1652CE82ACBFD409F3829D35F7080-/b2+HYfkarQqUD6E6FAiowdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2017-04-03 16:29           ` Kai Wasserbäch
2017-04-03 15:37   ` Deucher, Alexander

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=BN6PR12MB1652CE82ACBFD409F3829D35F7080@BN6PR12MB1652.namprd12.prod.outlook.com \
    --to=alexander.deucher-5c7gfcevmho@public.gmane.org \
    --cc=Harry.Wentland-5C7GfCeVMHo@public.gmane.org \
    --cc=amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
    --cc=kai-1ZKVMVCtJ2dx9oSEVPI0kiST3g8Odh+X@public.gmane.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.