Re: question about drivers/gpu/drm/gma500/oaktrail_lvds.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 9 Jul 2012, Patrik Jakobsson wrote:

On Sun, Jul 8, 2012 at 10:16 PM, Alan Cox <alan@xxxxxxxxxxxxxxxxxxx> wrote:
On Sun, 8 Jul 2012 10:39:43 +0200 (CEST)
Julia Lawall <julia.lawall@xxxxxxx> wrote:

In the function oaktrail_lvds_mode_set, I don't think that the following
code makes any sense:

         /* Find the connector we're trying to set up */
         list_for_each_entry(connector, &mode_config->connector_list, head) {
                 if (!connector->encoder || connector->encoder->crtc != crtc)
                         continue;
         }

         if (!connector) {
                 DRM_ERROR("Couldn't find connector when setting mode");
                 return;
         }

         drm_connector_property_get_value(
                 connector,
                 dev->mode_config.scaling_mode_property,
                 &v);

The initial loop is a no-op, because it always continues.  The test
!connector can never be true, because at the end of a list_for_each_entry
connector points to the list head, and calling
drm_connector_property_get_value on the list head probably does not make
sense.

We test !connector->encoder rather than !connector ?

It seems we should break on :
if (connector->encoder && connector->encoder == encoder)

Then do a check after list iteration:
if (!connector || connector->encoder != encoder)
   DRM_ERROR("Couldn't find connector when setting mode");

Possible. The !connector is still not needed, but the overall logic seems better.

julia
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux