[PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



And exactly that's the problematical assumption.

This doesn't print only when the module is loaded, but rather when a 
framebuffer object is created.

And depending on the use case that can even be many many times per second.

Please remove that printing, addition to that I agree with Michel that 
the module parameter is overkill.

Regards,
Christian.

Am 06.03.2018 um 18:09 schrieb Li, Samuel:
> This information is kind of important, and it only prints once typically when module is loaded.
>
> Regards,
> Samuel Li
>
>
>> -----Original Message-----
>> From: Alex Deucher [mailto:alexdeucher at gmail.com]
>> Sent: Tuesday, March 06, 2018 12:04 PM
>> To: Li, Samuel <Samuel.Li at amd.com>
>> Cc: Koenig, Christian <Christian.Koenig at amd.com>; amd-gfx list <amd-
>> gfx at lists.freedesktop.org>
>> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
>>
>> On Tue, Mar 6, 2018 at 11:49 AM, Samuel Li <samuel.li at amd.com> wrote:
>>>>>        domain = amdgpu_display_framebuffer_domains(adev);
>>>>> +    if (domain == AMDGPU_GEM_DOMAIN_GTT) {
>>>>> +        DRM_INFO("Scatter gather display: enabled\n");
>>>>> +    } else if (domain & AMDGPU_GEM_DOMAIN_GTT) {
>>>>> +        DRM_INFO("Scatter gather display: auto\n");
>>>>> +    }
>>>> Dito and printing that here is not a good idea as far as I can see.
>>>>
>>>> Christian.
>>>
>>> The intention here is to print out when fb is created. You have other
>> suggestions?
>>
>> Make it DRM_DEBUG?  otherwise you'll spam the logs.
>>
>>> Sam
>>>
>>>
>>> On 2018-03-03 08:41 AM, Christian König wrote:
>>>> Am 03.03.2018 um 00:25 schrieb Samuel Li:
>>>>> It's enabled by default. -1 is auto, to allow both vram and gtt
>>>>> memory be available, for testing purpose only.
>>>>> ---
>>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu.h         | 1 +
>>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 +++++++--
>>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c     | 4 ++++
>>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c      | 5 +++++
>>>>>    4 files changed, 17 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> index 292c7e7..6b0ee34 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> @@ -129,6 +129,7 @@ extern int amdgpu_lbpw;
>>>>>    extern int amdgpu_compute_multipipe;
>>>>>    extern int amdgpu_gpu_recovery;
>>>>>    extern int amdgpu_emu_mode;
>>>>> +extern int amdgpu_sg_display;
>>>>>      #ifdef CONFIG_DRM_AMDGPU_SI
>>>>>    extern int amdgpu_si_support;
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>>>> index 5495b29..dfa11b1 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>>>> @@ -513,8 +513,13 @@ uint32_t
>> amdgpu_display_framebuffer_domains(struct amdgpu_device *adev)
>>>>>    #if defined(CONFIG_DRM_AMD_DC)
>>>>>        if (adev->asic_type >= CHIP_CARRIZO && adev->asic_type <
>> CHIP_RAVEN &&
>>>>>            adev->flags & AMD_IS_APU &&
>>>>> -        amdgpu_device_asic_has_dc_support(adev->asic_type))
>>>>> -        domain |= AMDGPU_GEM_DOMAIN_GTT;
>>>>> +        amdgpu_device_asic_has_dc_support(adev->asic_type)) {
>>>>> +        if (amdgpu_sg_display == 1) {
>>>>> +            domain = AMDGPU_GEM_DOMAIN_GTT;
>>>>> +        } else if (amdgpu_sg_display == -1) {
>>>>> +            domain |= AMDGPU_GEM_DOMAIN_GTT;
>>>>> +        }
>>>> Coding style, that if shouldn't use "{" and "}".
>>>>
>>>>> +    }
>>>>>    #endif
>>>>>          return domain;
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>>> index e670936..f0ada24 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>>> @@ -132,6 +132,7 @@ int amdgpu_lbpw = -1;
>>>>>    int amdgpu_compute_multipipe = -1;
>>>>>    int amdgpu_gpu_recovery = -1; /* auto */
>>>>>    int amdgpu_emu_mode = 0;
>>>>> +int amdgpu_sg_display = 1;
>>>>>      MODULE_PARM_DESC(vramlimit, "Restrict VRAM for testing, in
>> megabytes");
>>>>>    module_param_named(vramlimit, amdgpu_vram_limit, int, 0600); @@
>>>>> -290,6 +291,9 @@ module_param_named(gpu_recovery,
>> amdgpu_gpu_recovery, int, 0444);
>>>>>    MODULE_PARM_DESC(emu_mode, "Emulation mode, (1 = enable, 0 =
>> disable)");
>>>>>    module_param_named(emu_mode, amdgpu_emu_mode, int, 0444);
>>>>>    +MODULE_PARM_DESC(sg_display, "Enable scatter gather display, (1 =
>>>>> enable, 0 = disable, -1 = auto");
>>>>> +module_param_named(sg_display, amdgpu_sg_display, int, 0444);
>>>>> +
>>>>>    #ifdef CONFIG_DRM_AMDGPU_SI
>>>>>      #if defined(CONFIG_DRM_RADEON) ||
>>>>> defined(CONFIG_DRM_RADEON_MODULE) diff --git
>>>>> a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>>>>> index 1206301..10f1f4f 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>>>>> @@ -138,6 +138,11 @@ static int
>> amdgpufb_create_pinned_object(struct amdgpu_fbdev *rfbdev,
>>>>>        mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd-
>>> width, cpp,
>>>>>                              fb_tiled);
>>>>>        domain = amdgpu_display_framebuffer_domains(adev);
>>>>> +    if (domain == AMDGPU_GEM_DOMAIN_GTT) {
>>>>> +        DRM_INFO("Scatter gather display: enabled\n");
>>>>> +    } else if (domain & AMDGPU_GEM_DOMAIN_GTT) {
>>>>> +        DRM_INFO("Scatter gather display: auto\n");
>>>>> +    }
>>>> Dito and printing that here is not a good idea as far as I can see.
>>>>
>>>> Christian.
>>>>
>>>>>          height = ALIGN(mode_cmd->height, 8);
>>>>>        size = mode_cmd->pitches[0] * height;
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux