[PATCH 1/7] drm/hisilicon:Add hisilicon hibmc master driver.

[Emil Velikov]

Hi Rongrong Zou,

With the next revision sent out, I've missed out that you had
questions in here :-)

>> On 29 February 2016 at 00:58, lijianhua <jueying0518 at gmail.com> wrote:
>>> +static struct drm_driver hibmc_driver = {

>>> +       .date                   = "20151218",
>>
>> Afaict this date is mostly unused, then again it's quite off :-)
>
> Do you mean i should better remove this property?
>
Just update it ? Then again many other drivers haven't updated theirs,
in a long time so I'm not sure how useful the .date field is to begin
with.
Ah, yes. It serves mostly to be send back to userspace via the
DRM_VERSION ioctl.

If we were to break the ABI, this is one of the things that will go :-)

>>> +static int hibmc_pm_resume(struct device *dev)
>>> +{
>>> +       struct pci_dev *pdev = to_pci_dev(dev);
>>> +       struct drm_device *drm_dev = pci_get_drvdata(pdev);
>>> +       struct hibmc_private *hiprivate = drm_dev->dev_private;
>>> +
>>> +       drm_helper_resume_force_mode(drm_dev);
>>> +
>>> +       if (hiprivate->fbdev.initialized) {
>>> +               console_lock();
>>> +               fb_set_suspend(hiprivate->fbdev.helper.fbdev, 0);
These should be using the drm fb helpers from Archit - see commit
0843010bbd6 "drm/virtio: Use new drm_fb_helper functions" and alike.

Which brings the question:

Archit,
Is it too much to ask to create a cocci script for the fb helper
refactoring ? One that warns/updates cases that are using the fb
functions directly, as opposed to the helper. Otherwise things are
bound to get confused. Like in the radeon driver, which uses both fb
and the helper.

I believe that one should even remove the relevant select statements
from the driver Kconfigs (FB_CFB* and FB_SYS*), correct ?

Just a humble request, thanks.

>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h

>>> +/* Vendor and Device id for HISILICON Graphics chip*/
>>> +#define PCI_VENDOR_ID_HIS 0x19e5
>>> +#define PCI_DEVID_HS_VGA 0x1711
>>> +
>
> I wonder where to place the 2 macros, any suggusetions, thanks.
>
include/linux/pci_ids.h looks like the place ?

Personally, I think that the above can be done with follow up patches.
Then again it's not my call to make.

Thanks
Emil