On Wed, Jan 24, 2018 at 9:53 AM, Harry Wentland <harry.wentland at amd.com> wrote:
> On 2018-01-23 05:10 PM, Alex Deucher wrote:
>> The atomic debugfs stuff gets created in drm_dev_alloc()
>> but this gets called before we've enumerated all of our
>> IPs, so move the DRIVER_ATOMIC flag setting to fix that.
>>
>> Since DRIVER_ATOMIC is a driver flag it's currently global
>> to the driver so setting it affects all GPUs driven by the
>> driver. Unfortunately, not all GPUs support atomic. Warn
>> the user if that is the case.
>>
>> This is the same as our current behavior, but at least the
>> atomic debugfs stuff gets created now.
>>
>> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 12 ++++++++++++
>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 --
>> 2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> index d1a695864793..367f331b4a54 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> @@ -578,6 +578,11 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
>> struct drm_device *dev;
>> unsigned long flags = ent->driver_data;
>> int ret, retry = 0;
>> + bool supports_atomic = false;
>> +
>> + if (!amdgpu_virtual_display &&
>> + amdgpu_device_asic_has_dc_support(flags & AMD_ASIC_MASK))
>> + supports_atomic = true;
>>
>> if ((flags & AMD_EXP_HW_SUPPORT) && !amdgpu_exp_hw_support) {
>> DRM_INFO("This hardware requires experimental hardware support.\n"
>> @@ -598,6 +603,13 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
>> if (ret)
>> return ret;
>>
>> + /* warn the user if they mix atomic and non-atomic capable GPUs */
>> + if ((kms_driver.driver_features & DRIVER_ATOMIC) && !supports_atomic)
>> + DRM_ERROR("Mixing atomic and non-atomic capable GPUs!\n");
>
> Would it make sense to make kms_driver non-static to deal with the DRIVER_ATOMIC flag correctly for multi-GPU?
We'd need two driver structs with the only difference being the ATOMIC
bit. Ideally this flag would be per device rather than per driver.
There are two issues here.
1. We need to set the ATOMIC bit early enough that the atomic debugfs
stuff gets created. We already set the flag when dc initializes
anyway so there is no change in behavior here other than that the
debugfs stuff gets created. So it seems like a net benefit to at
least set the flag early if dc is enabled.
2. Change the flag to be per device. This is a lot more involved
since we need to switch all the drivers. That part is not too bad
since most are either atomic or not. Where is gets tricky with amdgpu
(and possibly other drivers) is that we would need to fix up the
ordering of debugfs init and the drm load callbacks in
drm_dev_register() because the debugfs atomic files are added before
the driver load callback is called. I'm not sure how much other
drivers depend on that current ordering.
Alex
>
> Harry
>
>> + /* support atomic early so the atomic debugfs stuff gets created */
>> + if (supports_atomic)
>> + kms_driver.driver_features |= DRIVER_ATOMIC;
>> +
>> dev = drm_dev_alloc(&kms_driver, &pdev->dev);
>> if (IS_ERR(dev))
>> return PTR_ERR(dev);
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> index 63df977fc426..597302bb5f4d 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -1625,8 +1625,6 @@ static int dm_early_init(void *handle)
>> {
>> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>>
>> - adev->ddev->driver->driver_features |= DRIVER_ATOMIC;
>> -
>> switch (adev->asic_type) {
>> case CHIP_BONAIRE:
>> case CHIP_HAWAII:
>>
