| Message ID | 1479498793-31021-21-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Nov 18, 2016 at 09:52:56PM +0200, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Populate the fb->format to point at the correct format information > structure. > > Cc: linux-graphics-maintainer@vmware.com > Cc: Sinclair Yeh <syeh@vmware.com> > Cc: Thomas Hellstrom <thellstrom@vmware.com> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > index 5788913ca8f9..77f936e3056e 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > @@ -589,6 +589,7 @@ static int vmw_kms_new_framebuffer_surface(struct vmw_private *dev_priv, > > vfbs->base.base.dev = dev; > /* XXX get the first 4 from the surface info */ > + vfbs->base.base.format = drm_format_info(pixel_format); > vfbs->base.base.pixel_format = pixel_format; > vfbs->base.base.bits_per_pixel = mode_cmd->bpp; > vfbs->base.base.pitches[0] = mode_cmd->pitch; > @@ -897,6 +898,7 @@ static int vmw_kms_new_framebuffer_dmabuf(struct vmw_private *dev_priv, > } > > vfbd->base.base.dev = dev; > + vfbd->base.base.format = drm_format_info(pixel_format); Why not use drm_mode_fill_fb_struct here? This just seems to be bad copypasta in both cases ... -Daniel > vfbd->base.base.pixel_format = pixel_format; > vfbd->base.base.bits_per_pixel = mode_cmd->bpp; > vfbd->base.base.pitches[0] = mode_cmd->pitch; > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, Nov 18, 2016 at 09:52:56PM +0200, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Populate the fb->format to point at the correct format information > structure. > > Cc: linux-graphics-maintainer@vmware.com > Cc: Sinclair Yeh <syeh@vmware.com> > Cc: Thomas Hellstrom <thellstrom@vmware.com> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > index 5788913ca8f9..77f936e3056e 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > @@ -589,6 +589,7 @@ static int vmw_kms_new_framebuffer_surface(struct vmw_private *dev_priv, > > vfbs->base.base.dev = dev; > /* XXX get the first 4 from the surface info */ > + vfbs->base.base.format = drm_format_info(pixel_format); > vfbs->base.base.pixel_format = pixel_format; > vfbs->base.base.bits_per_pixel = mode_cmd->bpp; > vfbs->base.base.pitches[0] = mode_cmd->pitch; If you change the earlier patch to just use fill_fb_struct this one here isn't needed. -Daniel > @@ -897,6 +898,7 @@ static int vmw_kms_new_framebuffer_dmabuf(struct vmw_private *dev_priv, > } > > vfbd->base.base.dev = dev; > + vfbd->base.base.format = drm_format_info(pixel_format); > vfbd->base.base.pixel_format = pixel_format; > vfbd->base.base.bits_per_pixel = mode_cmd->bpp; > vfbd->base.base.pitches[0] = mode_cmd->pitch; > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Wed, Nov 30, 2016 at 04:40:27PM +0100, Daniel Vetter wrote: > On Fri, Nov 18, 2016 at 09:52:56PM +0200, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Populate the fb->format to point at the correct format information > > structure. > > > > Cc: linux-graphics-maintainer@vmware.com > > Cc: Sinclair Yeh <syeh@vmware.com> > > Cc: Thomas Hellstrom <thellstrom@vmware.com> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > > index 5788913ca8f9..77f936e3056e 100644 > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > > @@ -589,6 +589,7 @@ static int vmw_kms_new_framebuffer_surface(struct vmw_private *dev_priv, > > > > vfbs->base.base.dev = dev; > > /* XXX get the first 4 from the surface info */ > > + vfbs->base.base.format = drm_format_info(pixel_format); > > vfbs->base.base.pixel_format = pixel_format; > > vfbs->base.base.bits_per_pixel = mode_cmd->bpp; > > vfbs->base.base.pitches[0] = mode_cmd->pitch; > > @@ -897,6 +898,7 @@ static int vmw_kms_new_framebuffer_dmabuf(struct vmw_private *dev_priv, > > } > > > > vfbd->base.base.dev = dev; > > + vfbd->base.base.format = drm_format_info(pixel_format); > > Why not use drm_mode_fill_fb_struct here? This just seems to be bad > copypasta in both cases ... That it is. I was hoping I'd trick the vmware folks into fixing it by posting this series. One issue is that this code still used the old mode_cmd struct rather than the new mode_cmd2 struct. So the fix may not be entirely trivial, and hence I was hoping people who care about the driver would do it. > -Daniel > > > vfbd->base.base.pixel_format = pixel_format; > > vfbd->base.base.bits_per_pixel = mode_cmd->bpp; > > vfbd->base.base.pitches[0] = mode_cmd->pitch; > > -- > > 2.7.4 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
Hello, (Thomas, there's a question for you below) On Wednesday 30 Nov 2016 18:03:30 Ville Syrjälä wrote: > On Wed, Nov 30, 2016 at 04:40:27PM +0100, Daniel Vetter wrote: > > On Fri, Nov 18, 2016 at 09:52:56PM +0200, ville.syrjala@linux.intel.com wrote: > >> From: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> > >> Populate the fb->format to point at the correct format information > >> structure. > >> > >> Cc: linux-graphics-maintainer@vmware.com > >> Cc: Sinclair Yeh <syeh@vmware.com> > >> Cc: Thomas Hellstrom <thellstrom@vmware.com> > >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> --- > >> > >> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > >> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index 5788913ca8f9..77f936e3056e > >> 100644 > >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > >> @@ -589,6 +589,7 @@ static int vmw_kms_new_framebuffer_surface(struct > >> vmw_private *dev_priv, > >> vfbs->base.base.dev = dev; > >> /* XXX get the first 4 from the surface info */ > >> > >> + vfbs->base.base.format = drm_format_info(pixel_format); > >> vfbs->base.base.pixel_format = pixel_format; > >> vfbs->base.base.bits_per_pixel = mode_cmd->bpp; > >> vfbs->base.base.pitches[0] = mode_cmd->pitch; > >> @@ -897,6 +898,7 @@ static int vmw_kms_new_framebuffer_dmabuf(struct > >> vmw_private *dev_priv, > >> } > >> > >> vfbd->base.base.dev = dev; > >> + vfbd->base.base.format = drm_format_info(pixel_format); > > > > Why not use drm_mode_fill_fb_struct here? This just seems to be bad > > copypasta in both cases ... > > That it is. I was hoping I'd trick the vmware folks into fixing it by > posting this series. One issue is that this code still used the old > mode_cmd struct rather than the new mode_cmd2 struct. So the fix may > not be entirely trivial, and hence I was hoping people who care > about the driver would do it. I gave it a try and it required touching too much code for my limited (or rather non-existing) knowledge of the driver. Pretty please to the vmwgfx maintainers, could you fix this ? vmwgfx is the last driver still relying on the old mode_cmd structure. > >> vfbd->base.base.pixel_format = pixel_format; > >> vfbd->base.base.bits_per_pixel = mode_cmd->bpp; > >> vfbd->base.base.pitches[0] = mode_cmd->pitch;
On Wed, Nov 30, 2016 at 06:09:02PM +0200, Laurent Pinchart wrote: > Hello, > > (Thomas, there's a question for you below) > > On Wednesday 30 Nov 2016 18:03:30 Ville Syrjälä wrote: > > On Wed, Nov 30, 2016 at 04:40:27PM +0100, Daniel Vetter wrote: > > > On Fri, Nov 18, 2016 at 09:52:56PM +0200, ville.syrjala@linux.intel.com > wrote: > > >> From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > >> > > >> Populate the fb->format to point at the correct format information > > >> structure. > > >> > > >> Cc: linux-graphics-maintainer@vmware.com > > >> Cc: Sinclair Yeh <syeh@vmware.com> > > >> Cc: Thomas Hellstrom <thellstrom@vmware.com> > > >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > >> --- > > >> > > >> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 ++ > > >> 1 file changed, 2 insertions(+) > > >> > > >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > > >> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index 5788913ca8f9..77f936e3056e > > >> 100644 > > >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > > >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > > >> @@ -589,6 +589,7 @@ static int vmw_kms_new_framebuffer_surface(struct > > >> vmw_private *dev_priv, > > >> vfbs->base.base.dev = dev; > > >> /* XXX get the first 4 from the surface info */ > > >> > > >> + vfbs->base.base.format = drm_format_info(pixel_format); > > >> vfbs->base.base.pixel_format = pixel_format; > > >> vfbs->base.base.bits_per_pixel = mode_cmd->bpp; > > >> vfbs->base.base.pitches[0] = mode_cmd->pitch; > > >> @@ -897,6 +898,7 @@ static int vmw_kms_new_framebuffer_dmabuf(struct > > >> vmw_private *dev_priv, > > >> } > > >> > > >> vfbd->base.base.dev = dev; > > >> + vfbd->base.base.format = drm_format_info(pixel_format); > > > > > > Why not use drm_mode_fill_fb_struct here? This just seems to be bad > > > copypasta in both cases ... > > > > That it is. I was hoping I'd trick the vmware folks into fixing it by > > posting this series. One issue is that this code still used the old > > mode_cmd struct rather than the new mode_cmd2 struct. So the fix may > > not be entirely trivial, and hence I was hoping people who care > > about the driver would do it. > > I gave it a try and it required touching too much code for my limited (or > rather non-existing) knowledge of the driver. Pretty please to the vmwgfx > maintainers, could you fix this ? vmwgfx is the last driver still relying on > the old mode_cmd structure. Well you all managed to trick me into typing it, and it didn't seem too bad. Pretty much just mechanically switching everything from mode_cmd to mode_cmd2. Maybe I've touched too many drm drivers for big refactorings and I have too low standards for what's involved ;-) Patch submitted, review very much appreciated. And I'll try to ping Sinclair on irc once he shows up for testing. -Daniel
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index 5788913ca8f9..77f936e3056e 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -589,6 +589,7 @@ static int vmw_kms_new_framebuffer_surface(struct vmw_private *dev_priv, vfbs->base.base.dev = dev; /* XXX get the first 4 from the surface info */ + vfbs->base.base.format = drm_format_info(pixel_format); vfbs->base.base.pixel_format = pixel_format; vfbs->base.base.bits_per_pixel = mode_cmd->bpp; vfbs->base.base.pitches[0] = mode_cmd->pitch; @@ -897,6 +898,7 @@ static int vmw_kms_new_framebuffer_dmabuf(struct vmw_private *dev_priv, } vfbd->base.base.dev = dev; + vfbd->base.base.format = drm_format_info(pixel_format); vfbd->base.base.pixel_format = pixel_format; vfbd->base.base.bits_per_pixel = mode_cmd->bpp; vfbd->base.base.pitches[0] = mode_cmd->pitch;