> -----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
