On Wed, Jun 2, 2010 at 9:30 PM, Matt Turner <mattst88 at gmail.com> wrote:
> 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.
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
>
