[PATCH] drm/amdgpu: Change the virtual_display type from int to char*.

[Deng, Emily]

> -----Original Message-----
> From: Michel Dänzer [mailto:michel at daenzer.net]
> Sent: Tuesday, August 09, 2016 2:29 PM
> To: Deng, Emily <Emily.Deng at amd.com>
> Cc: amd-gfx at lists.freedesktop.org
> Subject: Re: [PATCH] drm/amdgpu: Change the virtual_display type from int to
> char*.
> 
> On 09/08/16 02:43 PM, Emily Deng wrote:
> > For virtual display feature, as there may be mutiple GPUs, for user
> > could choose whiche GPU need to enable this feature, change the type
> > of virtual_display from int to char*. The variable will be set like
> > this virtual_display="xx:xx.x;xx:xx.x;".
> >
> > Signed-off-by: Emily Deng <Emily.Deng at amd.com>
> 
> When sending updated patches, it's nice to include a changelog describing what
> changed between revisions. This can be added either in the commit log itself or
> after the --- marker in the e-mail generated by git format-patch.
> 
> 
> > @@ -1182,11 +1183,38 @@ int amdgpu_ip_block_version_cmp(struct
> amdgpu_device *adev,
> >  	return 1;
> >  }
> >
> > +static void amdgpu_whether_enable_virtual_display(struct
> > +amdgpu_device *adev) {
> > +	adev->enable_virtual_display = 0;
> > +
> > +	if (amdgpu_virtual_display) {
> > +		char pci_address_name[8];
> > +		char *pciaddstr, *pciaddstr_tmp, *pciaddname;
> > +		struct drm_device *ddev = adev->ddev;
> > +
> > +		sprintf(pci_address_name, "%02x:%02x.%d", ddev->pdev-
> >bus->number,
> > +			PCI_SLOT(ddev->pdev->devfn), PCI_FUNC(ddev-
> >pdev->devfn));
> 
> Some systems have multiple PCI domains. The domain number needs to be
> included, before the bus number.
[[EmilyD]] Ok, I will add.
> 
> OTOH I'm not sure the PCI function number needs to be included. Do we ever
> do anything with a non-0 PCI function?
> 
[[EmilyD]] Yes, for SRIOV case, the PCI function may be non-0.
> 
> > +		pciaddstr = kstrdup(amdgpu_virtual_display, GFP_KERNEL);
> > +		pciaddstr_tmp = pciaddstr;
> > +		while ((pciaddname = strsep(&pciaddstr_tmp, ";"))) {
> > +			if (!strncmp(pci_address_name, pciaddname,
> > +sizeof(pci_address_name))) {
> 
> Please use strcmp instead of strncmp here, otherwise it will match different
> strings which contain the PCI address as a prefix.
> 
[[EmilyD]] Ok, I will modify it.
> 
> Other than that, looks good to me.
> 
> 
> --
> Earthling Michel Dänzer               |               http://www.amd.com
> Libre software enthusiast             |             Mesa and X developer