| Message ID | 1490706496-4959-5-git-send-email-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Tomi, Thank you for the patch. On Tuesday 28 Mar 2017 16:07:50 Tomi Valkeinen wrote: > mode_config's min_width and min_height are both set to 32, which is > overly restrictive. > > The real limits depend on whether we're configuring a crtc or a plane, > but a limit of 8x2 is safe for both cases. Strictly speaking it's planes in all cases, you could talk about primary or overlay planes instead. Apart from that, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > drivers/gpu/drm/omapdrm/omap_drv.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c > b/drivers/gpu/drm/omapdrm/omap_drv.c index 79a4aad35e0f..fe83efbbf127 > 100644 > --- a/drivers/gpu/drm/omapdrm/omap_drv.c > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c > @@ -488,8 +488,8 @@ static int omap_modeset_init(struct drm_device *dev) > priv->num_planes, priv->num_crtcs, priv->num_encoders, > priv->num_connectors); > > - dev->mode_config.min_width = 32; > - dev->mode_config.min_height = 32; > + dev->mode_config.min_width = 8; > + dev->mode_config.min_height = 2; > > /* note: eventually will need some cpu_is_omapXYZ() type stuff here > * to fill in these limits properly on different OMAP generations..
On 29/03/17 11:13, Laurent Pinchart wrote: > Hi Tomi, > > Thank you for the patch. > > On Tuesday 28 Mar 2017 16:07:50 Tomi Valkeinen wrote: >> mode_config's min_width and min_height are both set to 32, which is >> overly restrictive. >> >> The real limits depend on whether we're configuring a crtc or a plane, >> but a limit of 8x2 is safe for both cases. > > Strictly speaking it's planes in all cases, you could talk about primary or > overlay planes instead. Apart from that, Hmm, well, on the HW they are different. We have CRTC limits, and we have plane limits. In our HW, I think we can have 1x1 planes (at least with RGB). But CRTC minimum depends on the display used. In some cases CRTC minimum is 1x1, but usually it's 8x1 I think. And talking about primary or overlay planes is so legacy... With atomic, we just have planes! =) Tomi
On Wednesday 29 Mar 2017 11:23:00 Tomi Valkeinen wrote: > On 29/03/17 11:13, Laurent Pinchart wrote: > > Hi Tomi, > > > > Thank you for the patch. > > > > On Tuesday 28 Mar 2017 16:07:50 Tomi Valkeinen wrote: > >> mode_config's min_width and min_height are both set to 32, which is > >> overly restrictive. > >> > >> The real limits depend on whether we're configuring a crtc or a plane, > >> but a limit of 8x2 is safe for both cases. > > > > Strictly speaking it's planes in all cases, you could talk about primary > > or overlay planes instead. Apart from that, > > Hmm, well, on the HW they are different. We have CRTC limits, and we > have plane limits. > > In our HW, I think we can have 1x1 planes (at least with RGB). But CRTC > minimum depends on the display used. In some cases CRTC minimum is 1x1, > but usually it's 8x1 I think. > > And talking about primary or overlay planes is so legacy... With atomic, > we just have planes! =) CRTCs and planes are KMS concepts, at the hardware level we have background and overlay planes, don't we ?
On 29/03/17 11:24, Laurent Pinchart wrote: > On Wednesday 29 Mar 2017 11:23:00 Tomi Valkeinen wrote: >> On 29/03/17 11:13, Laurent Pinchart wrote: >>> Hi Tomi, >>> >>> Thank you for the patch. >>> >>> On Tuesday 28 Mar 2017 16:07:50 Tomi Valkeinen wrote: >>>> mode_config's min_width and min_height are both set to 32, which is >>>> overly restrictive. >>>> >>>> The real limits depend on whether we're configuring a crtc or a plane, >>>> but a limit of 8x2 is safe for both cases. >>> >>> Strictly speaking it's planes in all cases, you could talk about primary >>> or overlay planes instead. Apart from that, >> >> Hmm, well, on the HW they are different. We have CRTC limits, and we >> have plane limits. >> >> In our HW, I think we can have 1x1 planes (at least with RGB). But CRTC >> minimum depends on the display used. In some cases CRTC minimum is 1x1, >> but usually it's 8x1 I think. >> >> And talking about primary or overlay planes is so legacy... With atomic, >> we just have planes! =) > > CRTCs and planes are KMS concepts, at the hardware level we have background > and overlay planes, don't we ? No, on the HW level we have... I don't know what's the proper name, but whatever matches the CRTC. The dss driver calls it overlay manager, but that's a silly name too. It's the video timing generator, plus overlay composition. It does have a default background color, though. And then we have overlays, i.e. planes. You can have the CRTC enabled without any planes. Then you'll see the background color. If you have planes, they can be smaller than the CRTC. Tomi
Hi Tomi, On Wednesday 29 Mar 2017 11:26:45 Tomi Valkeinen wrote: > On 29/03/17 11:24, Laurent Pinchart wrote: > > On Wednesday 29 Mar 2017 11:23:00 Tomi Valkeinen wrote: > >> On 29/03/17 11:13, Laurent Pinchart wrote: > >>> On Tuesday 28 Mar 2017 16:07:50 Tomi Valkeinen wrote: > >>>> mode_config's min_width and min_height are both set to 32, which is > >>>> overly restrictive. > >>>> > >>>> The real limits depend on whether we're configuring a crtc or a plane, > >>>> but a limit of 8x2 is safe for both cases. > >>> > >>> Strictly speaking it's planes in all cases, you could talk about primary > >>> or overlay planes instead. Apart from that, > >> > >> Hmm, well, on the HW they are different. We have CRTC limits, and we > >> have plane limits. > >> > >> In our HW, I think we can have 1x1 planes (at least with RGB). But CRTC > >> minimum depends on the display used. In some cases CRTC minimum is 1x1, > >> but usually it's 8x1 I think. > >> > >> And talking about primary or overlay planes is so legacy... With atomic, > >> we just have planes! =) > > > > CRTCs and planes are KMS concepts, at the hardware level we have > > background and overlay planes, don't we ? > > No, on the HW level we have... I don't know what's the proper name, but > whatever matches the CRTC. The dss driver calls it overlay manager, but > that's a silly name too. It's the video timing generator, plus overlay > composition. It does have a default background color, though. > > And then we have overlays, i.e. planes. > > You can have the CRTC enabled without any planes. Then you'll see the > background color. If you have planes, they can be smaller than the CRTC. The mode setting min/max width and height are used by the core when creating a framebuffer to validate its size. It's thus only related to planes, not to the CRTC.
On 29/03/17 11:30, Laurent Pinchart wrote: > The mode setting min/max width and height are used by the core when creating a > framebuffer to validate its size. It's thus only related to planes, not to the > CRTC. It's also set to drm_mode_card_res, and fb's min & max are not necessarily the same as plane's min and max. But yes, I think you're right, it's not related to crtc as such. With legacy the fb min defines the crtc min (I think), but possibly with atomic that's not true. It's a bit messed up, really. We should have separate limits for crtcs, planes and fbs. For now the mode_config is all we have. Tomi
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c index 79a4aad35e0f..fe83efbbf127 100644 --- a/drivers/gpu/drm/omapdrm/omap_drv.c +++ b/drivers/gpu/drm/omapdrm/omap_drv.c @@ -488,8 +488,8 @@ static int omap_modeset_init(struct drm_device *dev) priv->num_planes, priv->num_crtcs, priv->num_encoders, priv->num_connectors); - dev->mode_config.min_width = 32; - dev->mode_config.min_height = 32; + dev->mode_config.min_width = 8; + dev->mode_config.min_height = 2; /* note: eventually will need some cpu_is_omapXYZ() type stuff here * to fill in these limits properly on different OMAP generations..
mode_config's min_width and min_height are both set to 32, which is overly restrictive. The real limits depend on whether we're configuring a crtc or a plane, but a limit of 8x2 is safe for both cases. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> --- drivers/gpu/drm/omapdrm/omap_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)