Hi Sebastian,
On Monday, 10 December 2018 00:26:28 EET Sebastian Reichel wrote:
> On Wed, Dec 05, 2018 at 05:00:18PM +0200, Laurent Pinchart wrote:
> > The omap_dss_device .check_timings() and .set_timings() operations
> > operate on struct videomode, while the DRM API operates on struct
> > drm_display_mode. This forces conversion from to videomode in the
> > callers. While that's not a problem per se, it creates a difference with
> > the drm_bridge API.
> >
> > Replace the videomode parameter to the .check_timings() and
> > .set_timings() operations with a drm_display_mode. This pushed the
> > conversion to videomode down to the DSS devices in some cases. If needed
> > they will be converted to operate on drm_display_mode natively.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> > ---
> > [...]
> > diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c
> > b/drivers/gpu/drm/omapdrm/omap_connector.c index
> > 9882a4b1402b..487603c56b08 100644
> > --- a/drivers/gpu/drm/omapdrm/omap_connector.c
> > +++ b/drivers/gpu/drm/omapdrm/omap_connector.c
> > @@ -245,23 +245,19 @@ enum drm_mode_status
> > omap_connector_mode_fixup(struct omap_dss_device *dssdev,
> > const struct drm_display_mode *mode,
> > struct drm_display_mode *adjusted_mode)
> {
> > - struct videomode vm = { 0 };
> > int ret;
> >
> > - drm_display_mode_to_videomode(mode, &vm);
> > + drm_mode_copy(adjusted_mode, mode);
>
> This will explode if adjusted_mode is NULL?
Correct, but this function should never (and is never as far as I can tell)
called with adjusted_mode equal to NULL.
> > for (; dssdev; dssdev = dssdev->next) {
> > if (!dssdev->ops->check_timings)
> > continue;
> >
> > - ret = dssdev->ops->check_timings(dssdev, &vm);
> > + ret = dssdev->ops->check_timings(dssdev, adjusted_mode);
> >
> > if (ret)
> > return MODE_BAD;
> > }
> >
> > - if (adjusted_mode)
> > - drm_display_mode_from_videomode(&vm, adjusted_mode);
> > -
> > return MODE_OK;
> > }
>
> Old code assumed, that adjusted_mode may be NULL. If adjusted_mode
> cannot be NULL, it should be mentioned in the patch description.
I will instead remove the check in the previous patch where it was needlessly
introduced.
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
