From: Dan Carpenter <dan.carpenter@oracle.com>
To: contact@emersion.fr
Cc: amd-gfx@lists.freedesktop.org
Subject: [bug report] drm/amd/display: use FB pitch to fill dc_cursor_attributes
Date: Tue, 26 Jul 2022 18:20:13 +0300 [thread overview]
Message-ID: <YuAGLYvbtrrgBH0U@kili> (raw)
Hello Simon Ser,
The patch 03a663673063: "drm/amd/display: use FB pitch to fill
dc_cursor_attributes" from Dec 2, 2020, leads to the following Smatch
static checker warning:
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_plane.c:1271 handle_cursor_update()
error: we previously assumed 'afb' could be null (see line 1230)
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_plane.c
1222 void handle_cursor_update(struct drm_plane *plane,
1223 struct drm_plane_state *old_plane_state)
1224 {
1225 struct amdgpu_device *adev = drm_to_adev(plane->dev);
1226 struct amdgpu_framebuffer *afb = to_amdgpu_framebuffer(plane->state->fb);
1227 struct drm_crtc *crtc = afb ? plane->state->crtc : old_plane_state->crtc;
afb is container_of() but it's basically a cast in this case. I would
always prefer to check "plane->state->fb" for NULL instead of the
container_of(), but that's sort of a style debate, I guess. Some people
really like checking the returned pointer and add build time asserts to
ensure that the container_of() is a no-op.
1228 struct dm_crtc_state *crtc_state = crtc ? to_dm_crtc_state(crtc->state) : NULL;
1229 struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
1230 uint64_t address = afb ? afb->address : 0;
1231 struct dc_cursor_position position = {0};
1232 struct dc_cursor_attributes attributes;
1233 int ret;
1234
1235 if (!plane->state->fb && !old_plane_state->fb)
1236 return;
1237
1238 DC_LOG_CURSOR("%s: crtc_id=%d with size %d to %d\n",
1239 __func__,
1240 amdgpu_crtc->crtc_id,
1241 plane->state->crtc_w,
1242 plane->state->crtc_h);
1243
1244 ret = get_cursor_position(plane, crtc, &position);
1245 if (ret)
1246 return;
1247
1248 if (!position.enable) {
1249 /* turn off cursor */
1250 if (crtc_state && crtc_state->stream) {
1251 mutex_lock(&adev->dm.dc_lock);
1252 dc_stream_set_cursor_position(crtc_state->stream,
1253 &position);
1254 mutex_unlock(&adev->dm.dc_lock);
1255 }
1256 return;
1257 }
1258
1259 amdgpu_crtc->cursor_width = plane->state->crtc_w;
1260 amdgpu_crtc->cursor_height = plane->state->crtc_h;
1261
1262 memset(&attributes, 0, sizeof(attributes));
1263 attributes.address.high_part = upper_32_bits(address);
1264 attributes.address.low_part = lower_32_bits(address);
1265 attributes.width = plane->state->crtc_w;
1266 attributes.height = plane->state->crtc_h;
1267 attributes.color_format = CURSOR_MODE_COLOR_PRE_MULTIPLIED_ALPHA;
1268 attributes.rotation_angle = 0;
1269 attributes.attribute_flags.value = 0;
1270
--> 1271 attributes.pitch = afb->base.pitches[0] / afb->base.format->cpp[0];
^^^^^ ^^^^^
Unchecked dereferences.
1272
1273 if (crtc_state->stream) {
1274 mutex_lock(&adev->dm.dc_lock);
1275 if (!dc_stream_set_cursor_attributes(crtc_state->stream,
1276 &attributes))
1277 DRM_ERROR("DC failed to set cursor attributes\n");
1278
1279 if (!dc_stream_set_cursor_position(crtc_state->stream,
1280 &position))
1281 DRM_ERROR("DC failed to set cursor position\n");
1282 mutex_unlock(&adev->dm.dc_lock);
1283 }
1284 }
regards,
dan carpenter
next reply other threads:[~2022-07-26 15:20 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-26 15:20 Dan Carpenter [this message]
2022-07-26 15:27 ` [bug report] drm/amd/display: use FB pitch to fill dc_cursor_attributes Simon Ser
2022-07-26 15:47 ` Dan Carpenter
2022-07-26 17:16 ` Simon Ser
-- strict thread matches above, loose matches on Subject: below --
2020-12-07 14:51 Dan Carpenter
2020-12-07 14:53 ` Simon Ser
2020-12-07 17:51 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YuAGLYvbtrrgBH0U@kili \
--to=dan.carpenter@oracle.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=contact@emersion.fr \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.