[PATCH] drm/radeon/kms: fix regressions in CS checker due to r6xx tiling changes

Matt Turner mattst88 at gmail.com
Wed Jun 2 18:30:37 PDT 2010


On Wed, Jun 2, 2010 at 3:16 PM, Alex Deucher <alexdeucher at gmail.com> wrote:
> On Wed, Jun 2, 2010 at 2:32 PM, Matt Turner <mattst88 at gmail.com> wrote:
>> On Wed, Jun 2, 2010 at 1:53 PM, Alex Deucher <alexdeucher at gmail.com> wrote:
>>> Fixes:
>>> https://bugs.freedesktop.org/show_bug.cgi?id=28327
>>>
>>> Signed-off-by: Alex Deucher <alexdeucher at gmail.com>
>>> ---
>>>  drivers/gpu/drm/radeon/r600_cs.c |   43 ++++++++++++++-----------------------
>>>  1 files changed, 16 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/radeon/r600_cs.c b/drivers/gpu/drm/radeon/r600_cs.c
>>> index 133f0da..99c8f40 100644
>>> --- a/drivers/gpu/drm/radeon/r600_cs.c
>>> +++ b/drivers/gpu/drm/radeon/r600_cs.c
>>> @@ -184,28 +184,16 @@ static inline int r600_cs_track_validate_cb(struct radeon_cs_parser *p, int i)
>>>        /* pitch is the number of 8x8 tiles per row */
>>>        pitch = G_028060_PITCH_TILE_MAX(track->cb_color_size[i]) + 1;
>>>        slice_tile_max = G_028060_SLICE_TILE_MAX(track->cb_color_size[i]) + 1;
>>> -       if (!pitch) {
>>> -               dev_warn(p->dev, "%s:%d cb pitch (%d) for %d invalid (0x%08X)\n",
>>> -                       __func__, __LINE__, pitch, i, track->cb_color_size[i]);
>>> -               return -EINVAL;
>>> -       }
>>>        height = size / (pitch * 8 * bpe);
>>>        if (height > 8192)
>>>                height = 8192;
>>> -       height &= ~0x7;
>>> -       if (!height)
>>> -               height = 8;
>>>        switch (G_0280A0_ARRAY_MODE(track->cb_color_info[i])) {
>>>        case V_0280A0_ARRAY_LINEAR_GENERAL:
>>> -               if (height & 0x7) {
>>> -                       dev_warn(p->dev, "%s:%d cb height (%d) invalid\n",
>>> -                                __func__, __LINE__, height);
>>> -                       return -EINVAL;
>>> -               }
>>> +               /* technically height & 0x7 */
>>>                break;
>>>        case V_0280A0_ARRAY_LINEAR_ALIGNED:
>>> -               pitch_align = (max((u32)64, (u32)(track->group_size / bpe)) / 8) - 1;
>>> -               if (pitch & pitch_align) {
>>> +               pitch_align = max((u32)64, (u32)(track->group_size / bpe)) / 8;
>>> +               if (pitch & (pitch_align - 1)) {
>>
>> Maybe we should use the kernel's ALIGN macro here? ALIGN(pitch,
>> pitch_align). I don't know, it might just make it unclear.
>
> We don't want to align the pitch, we want to check if it is aligned.

Ah, I meant IS_ALIGNED, in linux/kernel.h:

#define IS_ALIGNED(x, a)        (((x) & ((typeof(x))(a) - 1)) == 0)

Should definitely be using it here, instead.

Matt

> Alex
>
>>
>>>                        dev_warn(p->dev, "%s:%d cb pitch (%d) invalid\n",
>>>                                 __func__, __LINE__, pitch);
>>>                        return -EINVAL;
>>> @@ -217,8 +205,8 @@ static inline int r600_cs_track_validate_cb(struct radeon_cs_parser *p, int i)
>>>                }
>>>                break;
>>>        case V_0280A0_ARRAY_1D_TILED_THIN1:
>>> -               pitch_align = (max((u32)8, (u32)(track->group_size / (8 * bpe * track->nsamples))) / 8) - 1;
>>> -               if (pitch & pitch_align) {
>>> +               pitch_align = max((u32)8, (u32)(track->group_size / (8 * bpe * track->nsamples))) / 8;
>>> +               if (pitch & (pitch_align - 1)) {
>>>                        dev_warn(p->dev, "%s:%d cb pitch (%d) invalid\n",
>>>                                 __func__, __LINE__, pitch);
>>>                        return -EINVAL;
>>> @@ -231,8 +219,8 @@ static inline int r600_cs_track_validate_cb(struct radeon_cs_parser *p, int i)
>>>                break;
>>>        case V_0280A0_ARRAY_2D_TILED_THIN1:
>>>                pitch_align = max((u32)track->nbanks,
>>> -                                 (u32)(((track->group_size / 8) / (bpe * track->nsamples)) * track->nbanks)) - 1;
>>> -               if (pitch & pitch_align) {
>>> +                                 (u32)(((track->group_size / 8) / (bpe * track->nsamples)) * track->nbanks));
>>> +               if (pitch & (pitch_align - 1)) {
>>>                        dev_warn(p->dev, "%s:%d cb pitch (%d) invalid\n",
>>>                                __func__, __LINE__, pitch);
>>>                        return -EINVAL;
>>> @@ -250,9 +238,9 @@ static inline int r600_cs_track_validate_cb(struct radeon_cs_parser *p, int i)
>>>                return -EINVAL;
>>>        }
>>>        /* check offset */
>>> -       tmp = height * pitch * 8;
>>> +       tmp = height * pitch * 8 * bpe;
>>>        if ((tmp + track->cb_color_bo_offset[i]) > radeon_bo_size(track->cb_color_bo[i])) {
>>> -               dev_warn(p->dev, "%s offset[%d] %d to big\n", __func__, i, track->cb_color_bo_offset[i]);
>>> +               dev_warn(p->dev, "%s offset[%d] %d too big\n", __func__, i, track->cb_color_bo_offset[i]);
>>>                return -EINVAL;
>>>        }
>>>        if (track->cb_color_bo_offset[i] & (track->group_size - 1)) {
>>> @@ -1130,11 +1118,12 @@ static inline int r600_check_texture_resource(struct radeon_cs_parser *p,  u32 i
>>>        pitch = G_038000_PITCH(word0) + 1;
>>>        switch (G_038000_TILE_MODE(word0)) {
>>>        case V_038000_ARRAY_LINEAR_GENERAL:
>>> +               pitch_align = 1;
>>>                /* XXX check height align */
>>>                break;
>>>        case V_038000_ARRAY_LINEAR_ALIGNED:
>>> -               pitch_align = (max((u32)64, (u32)(track->group_size / bpe)) / 8) - 1;
>>> -               if (pitch & pitch_align) {
>>> +               pitch_align = max((u32)64, (u32)(track->group_size / bpe)) / 8;
>>> +               if (pitch & (pitch_align - 1)) {
>>>                        dev_warn(p->dev, "%s:%d tex pitch (%d) invalid\n",
>>>                                 __func__, __LINE__, pitch);
>>>                        return -EINVAL;
>>> @@ -1142,8 +1131,8 @@ static inline int r600_check_texture_resource(struct radeon_cs_parser *p,  u32 i
>>>                /* XXX check height align */
>>>                break;
>>>        case V_038000_ARRAY_1D_TILED_THIN1:
>>> -               pitch_align = (max((u32)8, (u32)(track->group_size / (8 * bpe))) / 8) - 1;
>>> -               if (pitch & pitch_align) {
>>> +               pitch_align = max((u32)8, (u32)(track->group_size / (8 * bpe))) / 8;
>>> +               if (pitch & (pitch_align - 1)) {
>>>                        dev_warn(p->dev, "%s:%d tex pitch (%d) invalid\n",
>>>                                 __func__, __LINE__, pitch);
>>>                        return -EINVAL;
>>> @@ -1152,8 +1141,8 @@ static inline int r600_check_texture_resource(struct radeon_cs_parser *p,  u32 i
>>>                break;
>>>        case V_038000_ARRAY_2D_TILED_THIN1:
>>>                pitch_align = max((u32)track->nbanks,
>>> -                                 (u32)(((track->group_size / 8) / bpe) * track->nbanks)) - 1;
>>> -               if (pitch & pitch_align) {
>>> +                                 (u32)(((track->group_size / 8) / bpe) * track->nbanks));
>>> +               if (pitch & (pitch_align - 1)) {
>>>                        dev_warn(p->dev, "%s:%d tex pitch (%d) invalid\n",
>>>                                __func__, __LINE__, pitch);
>>>                        return -EINVAL;
>>> --
>>
>> Matt


More information about the dri-devel mailing list