On Wed, Jun 2, 2010 at 9:30 PM, Matt Turner <mattst88@xxxxxxxxx> wrote: > On Wed, Jun 2, 2010 at 3:16 PM, Alex Deucher <alexdeucher@xxxxxxxxx> wrote: >> On Wed, Jun 2, 2010 at 2:32 PM, Matt Turner <mattst88@xxxxxxxxx> wrote: >>> On Wed, Jun 2, 2010 at 1:53 PM, Alex Deucher <alexdeucher@xxxxxxxxx> wrote: >>>> Fixes: >>>> https://bugs.freedesktop.org/show_bug.cgi?id=28327 >>>> >>>> Signed-off-by: Alex Deucher <alexdeucher@xxxxxxxxx> >>>> --- >>>> 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. Care to send a patch? Alex > > 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 > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
