Am 26.06.20 um 12:39 schrieb Dan Carpenter:
> These if statements are supposed to be true if we ended the
> list_for_each_entry() loops without hitting a break statement but they
> don't work.
>
> In the first loop, we increment "i" after the "if (i == unit)" condition
> so we don't necessarily know that "i" is not equal to unit at the end of
> the loop.
So, if I understand this right, this would only really be a problem if
there's no list entries at all, right? That is i == unit == 0.
Not sure if that can actually happen, but in any case the fix looks correct.
>
> In the second loop we exit when mode is not pointing to a valid
> drm_display_mode struct so it doesn't make sense to check "mode->type".
Looks good to me too, condition order seems fine to me as well, though I
wouldn't particularly care.
Applied to vmwgfx-next as well, thanks.
Roland
>
> Fixes: a278724aa23c ("drm/vmwgfx: Implement fbdev on kms v2")
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> ---
> I reversed the second condition as well, just because I was copy and
> pasting the exit condition. Plus I always feel like error handling is
> better than success handling. If anyone feel strongly, then I can send
> a v2.
>
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 3c97654b5a43..44168a7d7b44 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -2576,7 +2576,7 @@ int vmw_kms_fbdev_init_data(struct vmw_private *dev_priv,
> ++i;
> }
>
> - if (i != unit) {
> + if (&con->head == &dev_priv->dev->mode_config.connector_list) {
> DRM_ERROR("Could not find initial display unit.\n");
> ret = -EINVAL;
> goto out_unlock;
> @@ -2600,13 +2600,13 @@ int vmw_kms_fbdev_init_data(struct vmw_private *dev_priv,
> break;
> }
>
> - if (mode->type & DRM_MODE_TYPE_PREFERRED)
> - *p_mode = mode;
> - else {
> + if (&mode->head == &con->modes) {
> WARN_ONCE(true, "Could not find initial preferred mode.\n");
> *p_mode = list_first_entry(&con->modes,
> struct drm_display_mode,
> head);
> + } else {
> + *p_mode = mode;
> }
>
> out_unlock:
>
