Hi,

On Tue, 24 May 2016 16:16:40 +0200 Philipp Zabel wrote:
> Hi Lothar,
> 
> thank you for rebasing. I have applied the other two patches.
> With this one, I'd like to avoid the duplicated code. See below:
> 
> Am Dienstag, den 24.05.2016, 08:31 +0200 schrieb Lothar Waßmann:
> > Currently these flags are lost in the call
> > drm_display_mode_from_videomode()
> > 
> > Signed-off-by: Lothar Waßmann <LW at KARO-electronics.de>
> > ---
> >  drivers/gpu/drm/imx/imx-ldb.c          | 37 
> > ++++++++++++++++++++++++++--------
> >  drivers/gpu/drm/imx/parallel-display.c | 31 ++++++++++++++++++++++++----
> >  2 files changed, 56 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
> > index b2dc4df..b17a246 100644
> > --- a/drivers/gpu/drm/imx/imx-ldb.c
> > +++ b/drivers/gpu/drm/imx/imx-ldb.c
> [...]
> > @@ -601,10 +606,26 @@ static int imx_ldb_bind(struct device *dev, struct 
> > device *master, void *data)
> >                     channel->edid = kmemdup(edidp, channel->edid_len,
> >                                             GFP_KERNEL);
> >             } else if (!channel->panel) {
> > -                   ret = of_get_drm_display_mode(child, &channel->mode,
> > -                                           OF_USE_NATIVE_MODE);
> > -                   if (!ret)
> > -                           channel->mode_valid = 1;
> > +                   struct videomode vm;
> > +
> > +                   ret = of_get_videomode(child, &vm, OF_USE_NATIVE_MODE);
> > +                   if (ret)
> > +                           return ret;
> > +                   drm_display_mode_from_videomode(&vm, &channel->mode);
> > +
> > +                   if (vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> > +                           channel->bus_flags |= 
> > DRM_BUS_FLAG_PIXDATA_POSEDGE;
> > +                   if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> > +                           channel->bus_flags |= 
> > DRM_BUS_FLAG_PIXDATA_NEGEDGE;
> > +
> > +                   if (vm.flags & DISPLAY_FLAGS_DE_LOW)
> > +                           channel->bus_flags |= DRM_BUS_FLAG_DE_LOW;
> > +                   if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
> > +                           channel->bus_flags |= DRM_BUS_FLAG_DE_HIGH;
> > +
> > +                   drm_mode_debug_printmodeline(&channel->mode);
> > +                   channel->mode_valid = 1;
> [...]
> > @@ -81,10 +87,27 @@ static int imx_pd_connector_get_modes(struct 
> > drm_connector *connector)
> >  
> >     if (np) {
> >             struct drm_display_mode *mode = drm_mode_create(connector->dev);
> > +           struct videomode vm;
> > +           int ret;
> >  
> >             if (!mode)
> >                     return -EINVAL;
> > -           of_get_drm_display_mode(np, &imxpd->mode, OF_USE_NATIVE_MODE);
> > +
> > +           ret = of_get_videomode(np, &vm, OF_USE_NATIVE_MODE);
> > +           if (ret)
> > +                   return ret;
> > +           drm_display_mode_from_videomode(&vm, &imxpd->mode);
> > +           drm_mode_debug_printmodeline(&imxpd->mode);
> > +
> > +           if (vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> > +                   imxpd->bus_flags |= DRM_BUS_FLAG_PIXDATA_POSEDGE;
> > +           if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> > +                   imxpd->bus_flags |= DRM_BUS_FLAG_PIXDATA_NEGEDGE;
> > +
> > +           if (vm.flags & DISPLAY_FLAGS_DE_LOW)
> > +                   imxpd->bus_flags |= DRM_BUS_FLAG_DE_LOW;
> > +           if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
> > +                   imxpd->bus_flags |= DRM_BUS_FLAG_DE_HIGH;
> 
> This could be shared between ldb and parallel drivers if you extended
> of_get_drm_display_mode to also return bus_flags, for example. The flags
> translation could be put into a drm_bus_flags_from_videomode helper.
> 
I had that in mind also, but hesitated to change the generic
of_get_drm_display_mode() to implement some i.MX specific functionality.


Lothar Waßmann

Reply via email to