[Mesa-dev] [PATCH v4] winsys/radeon: fix nop packet padding for hawaii

[Andreas Boll]

2014-08-04 13:40 GMT+02:00 Christian König <christian.koenig at amd.com>:
> Am 04.08.2014 um 12:48 schrieb Andreas Boll:
>>
>> The initial firmware for hawaii does not support type3 nop packet.
>>
>> Detect the new hawaii firmware with query RADEON_INFO_ACCEL_WORKING2.
>> If the returned value is 3, then the new firmware is used.
>>
>> This patch uses type2 for the old firmware and type3 for the new firmware.
>
>
> AFAIK the type2 nop packet is deprecated because it was found to not work
> correctly in some situations. The hack with the type3 nop packet and the
> strange size field was added to work around this.
>
> I won't advise using any stack that still uses the old nop packet and
> firmware, that might not be very not reliable. So we should print at least a
> big warning if that's the case.
>
> Regards,
> Christian.
>

Would a warning in Xorg log be sufficient or do you want an error message in
the winsys init code?

Andreas.

>>
>> It fixes the cases when the old firmware is used and the user wants to
>> manually enable acceleration.
>> The two possible scenarios are:
>>   - the kernel has no support for the new firmware.
>>   - the kernel has support for the new firmware but only the old firmware
>>     is available.
>>
>> Additionaly this patch disables GPU acceleration on hawaii if the kernel
>> returns a value < 2. In this case the kernel hasn't the required fixes
>> for proper acceleration.
>>
>> v2:
>>   - Fix indentation
>>   - Use private struct radeon_drm_winsys instead of public struct
>> radeon_info
>>   - Rename r600_accel_working2 to accel_working2
>>
>> v3:
>>   - Use type2 nop packet for returned value < 3
>>
>> v4:
>>   - Fail to initialize winsys for returned value < 2
>>
>> Cc: mesa-stable at lists.freedesktop.org
>> Cc: Alex Deucher <alexander.deucher at amd.com>
>> Cc: Jérôme Glisse <jglisse at redhat.com>
>> Cc: Marek Olšák <marek.olsak at amd.com>
>> Cc: Michel Dänzer <michel.daenzer at amd.com>
>> Signed-off-by: Andreas Boll <andreas.boll.dev at gmail.com>
>> ---
>>
>> Unfortunately I can't test this patch myself since I don't own a hawaii
>> card.
>> So I'd need someone to test this patch on kernel >= 3.16-rc7 + these
>> patches
>> [1-2].
>>
>> This patch would bring us one step further for hawaii acceleration on
>> kernel
>> 3.16.
>>
>> Finally we can enable hawaii acceleration if the query returns > 2 [3].
>>
>> Andreas.
>>
>> [1]
>> http://lists.freedesktop.org/archives/dri-devel/2014-August/065305.html
>> [2]
>> http://lists.freedesktop.org/archives/dri-devel/2014-August/065306.html
>> [3] http://lists.x.org/archives/xorg-driver-ati/2014-August/026534.html
>>
>>   src/gallium/winsys/radeon/drm/radeon_drm_cs.c     |  6 +++++-
>>   src/gallium/winsys/radeon/drm/radeon_drm_winsys.c | 10 ++++++++++
>>   src/gallium/winsys/radeon/drm/radeon_drm_winsys.h |  1 +
>>   3 files changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_cs.c
>> b/src/gallium/winsys/radeon/drm/radeon_drm_cs.c
>> index a06ecb2..dd109af 100644
>> --- a/src/gallium/winsys/radeon/drm/radeon_drm_cs.c
>> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_cs.c
>> @@ -446,8 +446,12 @@ static void radeon_drm_cs_flush(struct
>> radeon_winsys_cs *rcs,
>>       case RING_GFX:
>>           /* pad DMA ring to 8 DWs to meet CP fetch alignment requirements
>>            * r6xx, requires at least 4 dw alignment to avoid a hw bug.
>> +         * hawaii with old firmware needs type2 nop packet.
>> +         * accel_working2 with value 2 indicates the new firmware.
>>            */
>> -        if (cs->ws->info.chip_class <= SI) {
>> +        if (cs->ws->info.chip_class <= SI ||
>> +            (cs->ws->info.family == CHIP_HAWAII &&
>> +             cs->ws->accel_working2 < 3)) {
>>               while (rcs->cdw & 7)
>>                   OUT_CS(&cs->base, 0x80000000); /* type2 nop packet */
>>           } else {
>> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
>> b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
>> index 910d06b..ecff0e7 100644
>> --- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
>> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
>> @@ -395,6 +395,16 @@ static boolean do_winsys_init(struct
>> radeon_drm_winsys *ws)
>>       radeon_get_drm_value(ws->fd, RADEON_INFO_MAX_SH_PER_SE, NULL,
>>                            &ws->info.max_sh_per_se);
>>   +    radeon_get_drm_value(ws->fd, RADEON_INFO_ACCEL_WORKING2, NULL,
>> +                         &ws->accel_working2);
>> +    if (ws->info.family == CHIP_HAWAII && ws->accel_working2 < 2) {
>> +        fprintf(stderr, "radeon: GPU acceleration for Hawaii disabled, "
>> +                "returned accel_working2 value %u is smaller than 2. "
>> +                "Please install a newer kernel.\n",
>> +                ws->accel_working2);
>> +        return FALSE;
>> +    }
>> +
>>       if (radeon_get_drm_value(ws->fd, RADEON_INFO_SI_TILE_MODE_ARRAY,
>> NULL,
>>                                ws->info.si_tile_mode_array)) {
>>           ws->info.si_tile_mode_array_valid = TRUE;
>> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h
>> b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h
>> index ea6f7f0..aebc391 100644
>> --- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h
>> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h
>> @@ -55,6 +55,7 @@ struct radeon_drm_winsys {
>>       enum radeon_generation gen;
>>       struct radeon_info info;
>>       uint32_t va_start;
>> +    uint32_t accel_working2;
>>         struct pb_manager *kman;
>>       struct pb_manager *cman_vram;
>
>