| Message ID | 1397201015-2807-5-git-send-email-archit@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 11/04/14 10:23, Archit Taneja wrote: > The drm ioctl DRM_IOCTL_MODE_ADDFB2 doesn't let us allocate buffers which are > greater than what is specified in the driver through dev->mode_config. > > Create helpers for DISPC which return the max manager width and height supported > by the device. The maximum width for a framebuffer is set to the combined width > of the all the crtcs, assuming they are arranged horizontally. > > Signed-off-by: Archit Taneja <archit@ti.com> > --- > drivers/gpu/drm/omapdrm/omap_drv.c | 10 ++++++---- > drivers/video/omap2/dss/dispc.c | 12 ++++++++++++ > include/video/omapdss.h | 2 ++ > 3 files changed, 20 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c > index c8270e4..55ec575 100644 > --- a/drivers/gpu/drm/omapdrm/omap_drv.c > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c > @@ -306,11 +306,13 @@ static int omap_modeset_init(struct drm_device *dev) > dev->mode_config.min_width = 32; > dev->mode_config.min_height = 32; > > - /* note: eventually will need some cpu_is_omapXYZ() type stuff here > - * to fill in these limits properly on different OMAP generations.. > + /* > + * Note: the maximum width is set to the combined width of all the > + * crtcs. We could assume the same for the maximum height too, but > + * we generally don't use such a configuration. > */ > - dev->mode_config.max_width = 2048; > - dev->mode_config.max_height = 2048; > + dev->mode_config.max_width = num_crtcs * dispc_mgr_max_width(); > + dev->mode_config.max_height = dispc_mgr_max_height(); This looks very strange. If the max size is supposed to be the maximum output size we support, then multiplying with num_crtcs doesn't make sense. If, on the other hand, it tells the possible maximum size of the framebuffer in the memory, of which only small part is shown (where's the max size of that "part" defined, then?), then there should be no limits as the only limit is the size of the memory. Tomi
On Monday 14 April 2014 02:55 PM, Tomi Valkeinen wrote: > On 11/04/14 10:23, Archit Taneja wrote: >> The drm ioctl DRM_IOCTL_MODE_ADDFB2 doesn't let us allocate buffers which are >> greater than what is specified in the driver through dev->mode_config. >> >> Create helpers for DISPC which return the max manager width and height supported >> by the device. The maximum width for a framebuffer is set to the combined width >> of the all the crtcs, assuming they are arranged horizontally. >> >> Signed-off-by: Archit Taneja <archit@ti.com> >> --- >> drivers/gpu/drm/omapdrm/omap_drv.c | 10 ++++++---- >> drivers/video/omap2/dss/dispc.c | 12 ++++++++++++ >> include/video/omapdss.h | 2 ++ >> 3 files changed, 20 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c >> index c8270e4..55ec575 100644 >> --- a/drivers/gpu/drm/omapdrm/omap_drv.c >> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c >> @@ -306,11 +306,13 @@ static int omap_modeset_init(struct drm_device *dev) >> dev->mode_config.min_width = 32; >> dev->mode_config.min_height = 32; >> >> - /* note: eventually will need some cpu_is_omapXYZ() type stuff here >> - * to fill in these limits properly on different OMAP generations.. >> + /* >> + * Note: the maximum width is set to the combined width of all the >> + * crtcs. We could assume the same for the maximum height too, but >> + * we generally don't use such a configuration. >> */ >> - dev->mode_config.max_width = 2048; >> - dev->mode_config.max_height = 2048; >> + dev->mode_config.max_width = num_crtcs * dispc_mgr_max_width(); >> + dev->mode_config.max_height = dispc_mgr_max_height(); > > This looks very strange. > > If the max size is supposed to be the maximum output size we support, > then multiplying with num_crtcs doesn't make sense. > > If, on the other hand, it tells the possible maximum size of the > framebuffer in the memory, of which only small part is shown (where's > the max size of that "part" defined, then?), then there should be no > limits as the only limit is the size of the memory. These parameters are used to tell the max size of framebuffer we can allocate in memory. I'm not sure why there is a limit in the first place, but if we have a really low minimum(like 2048 pixels right now), we can't have multiple displays showing different regions of the same buffer. Archit
On 14/04/14 13:18, Archit Taneja wrote: > On Monday 14 April 2014 02:55 PM, Tomi Valkeinen wrote: >> On 11/04/14 10:23, Archit Taneja wrote: >>> The drm ioctl DRM_IOCTL_MODE_ADDFB2 doesn't let us allocate buffers >>> which are >>> greater than what is specified in the driver through dev->mode_config. >>> >>> Create helpers for DISPC which return the max manager width and >>> height supported >>> by the device. The maximum width for a framebuffer is set to the >>> combined width >>> of the all the crtcs, assuming they are arranged horizontally. >>> >>> Signed-off-by: Archit Taneja <archit@ti.com> >>> --- >>> drivers/gpu/drm/omapdrm/omap_drv.c | 10 ++++++---- >>> drivers/video/omap2/dss/dispc.c | 12 ++++++++++++ >>> include/video/omapdss.h | 2 ++ >>> 3 files changed, 20 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c >>> b/drivers/gpu/drm/omapdrm/omap_drv.c >>> index c8270e4..55ec575 100644 >>> --- a/drivers/gpu/drm/omapdrm/omap_drv.c >>> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c >>> @@ -306,11 +306,13 @@ static int omap_modeset_init(struct drm_device >>> *dev) >>> dev->mode_config.min_width = 32; >>> dev->mode_config.min_height = 32; >>> >>> - /* note: eventually will need some cpu_is_omapXYZ() type stuff here >>> - * to fill in these limits properly on different OMAP generations.. >>> + /* >>> + * Note: the maximum width is set to the combined width of all the >>> + * crtcs. We could assume the same for the maximum height too, but >>> + * we generally don't use such a configuration. >>> */ >>> - dev->mode_config.max_width = 2048; >>> - dev->mode_config.max_height = 2048; >>> + dev->mode_config.max_width = num_crtcs * dispc_mgr_max_width(); >>> + dev->mode_config.max_height = dispc_mgr_max_height(); >> >> This looks very strange. >> >> If the max size is supposed to be the maximum output size we support, >> then multiplying with num_crtcs doesn't make sense. >> >> If, on the other hand, it tells the possible maximum size of the >> framebuffer in the memory, of which only small part is shown (where's >> the max size of that "part" defined, then?), then there should be no >> limits as the only limit is the size of the memory. > > These parameters are used to tell the max size of framebuffer we can > allocate in memory. > > I'm not sure why there is a limit in the first place, but if we have a > really low minimum(like 2048 pixels right now), we can't have multiple > displays showing different regions of the same buffer. In that case shouldn't the limit be 0 (if that's allowed) or some surely high enough value, say, 32767. Why calculate it at all based on the dispc's maximum, if it has no relevance? Tomi
On Fri, Apr 11, 2014 at 3:23 AM, Archit Taneja <archit@ti.com> wrote: > The drm ioctl DRM_IOCTL_MODE_ADDFB2 doesn't let us allocate buffers which are > greater than what is specified in the driver through dev->mode_config. > > Create helpers for DISPC which return the max manager width and height supported > by the device. The maximum width for a framebuffer is set to the combined width > of the all the crtcs, assuming they are arranged horizontally. > > Signed-off-by: Archit Taneja <archit@ti.com> > --- > drivers/gpu/drm/omapdrm/omap_drv.c | 10 ++++++---- > drivers/video/omap2/dss/dispc.c | 12 ++++++++++++ > include/video/omapdss.h | 2 ++ > 3 files changed, 20 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c > index c8270e4..55ec575 100644 > --- a/drivers/gpu/drm/omapdrm/omap_drv.c > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c > @@ -306,11 +306,13 @@ static int omap_modeset_init(struct drm_device *dev) > dev->mode_config.min_width = 32; > dev->mode_config.min_height = 32; > > - /* note: eventually will need some cpu_is_omapXYZ() type stuff here > - * to fill in these limits properly on different OMAP generations.. > + /* > + * Note: the maximum width is set to the combined width of all the > + * crtcs. We could assume the same for the maximum height too, but > + * we generally don't use such a configuration. > */ > - dev->mode_config.max_width = 2048; > - dev->mode_config.max_height = 2048; > + dev->mode_config.max_width = num_crtcs * dispc_mgr_max_width(); > + dev->mode_config.max_height = dispc_mgr_max_height(); the max_width should probably actually be the max_pitch.. BR, -R > dev->mode_config.funcs = &omap_mode_config_funcs; > > diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c > index 77d6221..47f9829 100644 > --- a/drivers/video/omap2/dss/dispc.c > +++ b/drivers/video/omap2/dss/dispc.c > @@ -2999,6 +2999,18 @@ void dispc_mgr_set_timings(enum omap_channel channel, > } > EXPORT_SYMBOL(dispc_mgr_set_timings); > > +u16 dispc_mgr_max_width(void) > +{ > + return dispc.feat->mgr_width_max; > +} > +EXPORT_SYMBOL(dispc_mgr_max_width); > + > +u16 dispc_mgr_max_height(void) > +{ > + return dispc.feat->mgr_height_max; > +} > +EXPORT_SYMBOL(dispc_mgr_max_height); > + > static void dispc_mgr_set_lcd_divisor(enum omap_channel channel, u16 lck_div, > u16 pck_div) > { > diff --git a/include/video/omapdss.h b/include/video/omapdss.h > index 3d7c51a..53637ac 100644 > --- a/include/video/omapdss.h > +++ b/include/video/omapdss.h > @@ -949,6 +949,8 @@ void dispc_mgr_set_lcd_config(enum omap_channel channel, > const struct dss_lcd_mgr_config *config); > void dispc_mgr_set_timings(enum omap_channel channel, > const struct omap_video_timings *timings); > +u16 dispc_mgr_max_width(void); > +u16 dispc_mgr_max_height(void); > void dispc_mgr_setup(enum omap_channel channel, > const struct omap_overlay_manager_info *info); > > -- > 1.8.3.2 >
On Monday 14 April 2014 04:00 PM, Tomi Valkeinen wrote: > On 14/04/14 13:18, Archit Taneja wrote: >> On Monday 14 April 2014 02:55 PM, Tomi Valkeinen wrote: >>> On 11/04/14 10:23, Archit Taneja wrote: >>>> The drm ioctl DRM_IOCTL_MODE_ADDFB2 doesn't let us allocate buffers >>>> which are >>>> greater than what is specified in the driver through dev->mode_config. >>>> >>>> Create helpers for DISPC which return the max manager width and >>>> height supported >>>> by the device. The maximum width for a framebuffer is set to the >>>> combined width >>>> of the all the crtcs, assuming they are arranged horizontally. >>>> >>>> Signed-off-by: Archit Taneja <archit@ti.com> >>>> --- >>>> drivers/gpu/drm/omapdrm/omap_drv.c | 10 ++++++---- >>>> drivers/video/omap2/dss/dispc.c | 12 ++++++++++++ >>>> include/video/omapdss.h | 2 ++ >>>> 3 files changed, 20 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c >>>> b/drivers/gpu/drm/omapdrm/omap_drv.c >>>> index c8270e4..55ec575 100644 >>>> --- a/drivers/gpu/drm/omapdrm/omap_drv.c >>>> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c >>>> @@ -306,11 +306,13 @@ static int omap_modeset_init(struct drm_device >>>> *dev) >>>> dev->mode_config.min_width = 32; >>>> dev->mode_config.min_height = 32; >>>> >>>> - /* note: eventually will need some cpu_is_omapXYZ() type stuff here >>>> - * to fill in these limits properly on different OMAP generations.. >>>> + /* >>>> + * Note: the maximum width is set to the combined width of all the >>>> + * crtcs. We could assume the same for the maximum height too, but >>>> + * we generally don't use such a configuration. >>>> */ >>>> - dev->mode_config.max_width = 2048; >>>> - dev->mode_config.max_height = 2048; >>>> + dev->mode_config.max_width = num_crtcs * dispc_mgr_max_width(); >>>> + dev->mode_config.max_height = dispc_mgr_max_height(); >>> >>> This looks very strange. >>> >>> If the max size is supposed to be the maximum output size we support, >>> then multiplying with num_crtcs doesn't make sense. >>> >>> If, on the other hand, it tells the possible maximum size of the >>> framebuffer in the memory, of which only small part is shown (where's >>> the max size of that "part" defined, then?), then there should be no >>> limits as the only limit is the size of the memory. >> >> These parameters are used to tell the max size of framebuffer we can >> allocate in memory. >> >> I'm not sure why there is a limit in the first place, but if we have a >> really low minimum(like 2048 pixels right now), we can't have multiple >> displays showing different regions of the same buffer. > > In that case shouldn't the limit be 0 (if that's allowed) or some surely > high enough value, say, 32767. Why calculate it at all based on the > dispc's maximum, if it has no relevance? I understand your point. When I wrote the patch, I thought more like: "I have 3 displays arranged horizontally, if I want a buffer large enough to fill the 3 displays, what should be it's width/height?" Archit
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c index c8270e4..55ec575 100644 --- a/drivers/gpu/drm/omapdrm/omap_drv.c +++ b/drivers/gpu/drm/omapdrm/omap_drv.c @@ -306,11 +306,13 @@ static int omap_modeset_init(struct drm_device *dev) dev->mode_config.min_width = 32; dev->mode_config.min_height = 32; - /* note: eventually will need some cpu_is_omapXYZ() type stuff here - * to fill in these limits properly on different OMAP generations.. + /* + * Note: the maximum width is set to the combined width of all the + * crtcs. We could assume the same for the maximum height too, but + * we generally don't use such a configuration. */ - dev->mode_config.max_width = 2048; - dev->mode_config.max_height = 2048; + dev->mode_config.max_width = num_crtcs * dispc_mgr_max_width(); + dev->mode_config.max_height = dispc_mgr_max_height(); dev->mode_config.funcs = &omap_mode_config_funcs; diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c index 77d6221..47f9829 100644 --- a/drivers/video/omap2/dss/dispc.c +++ b/drivers/video/omap2/dss/dispc.c @@ -2999,6 +2999,18 @@ void dispc_mgr_set_timings(enum omap_channel channel, } EXPORT_SYMBOL(dispc_mgr_set_timings); +u16 dispc_mgr_max_width(void) +{ + return dispc.feat->mgr_width_max; +} +EXPORT_SYMBOL(dispc_mgr_max_width); + +u16 dispc_mgr_max_height(void) +{ + return dispc.feat->mgr_height_max; +} +EXPORT_SYMBOL(dispc_mgr_max_height); + static void dispc_mgr_set_lcd_divisor(enum omap_channel channel, u16 lck_div, u16 pck_div) { diff --git a/include/video/omapdss.h b/include/video/omapdss.h index 3d7c51a..53637ac 100644 --- a/include/video/omapdss.h +++ b/include/video/omapdss.h @@ -949,6 +949,8 @@ void dispc_mgr_set_lcd_config(enum omap_channel channel, const struct dss_lcd_mgr_config *config); void dispc_mgr_set_timings(enum omap_channel channel, const struct omap_video_timings *timings); +u16 dispc_mgr_max_width(void); +u16 dispc_mgr_max_height(void); void dispc_mgr_setup(enum omap_channel channel, const struct omap_overlay_manager_info *info);
The drm ioctl DRM_IOCTL_MODE_ADDFB2 doesn't let us allocate buffers which are greater than what is specified in the driver through dev->mode_config. Create helpers for DISPC which return the max manager width and height supported by the device. The maximum width for a framebuffer is set to the combined width of the all the crtcs, assuming they are arranged horizontally. Signed-off-by: Archit Taneja <archit@ti.com> --- drivers/gpu/drm/omapdrm/omap_drv.c | 10 ++++++---- drivers/video/omap2/dss/dispc.c | 12 ++++++++++++ include/video/omapdss.h | 2 ++ 3 files changed, 20 insertions(+), 4 deletions(-)