| Message ID | 1406031827-12432-4-git-send-email-boris.brezillon@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jul 22, 2014 at 02:23:45PM +0200, Boris BREZILLON wrote: > Add bus_formats and nbus_formats fields and > drm_display_info_set_bus_formats helper function to specify the bus > formats supported by a given display. > > This information can be used by display controller drivers to configure > the output interface appropriately (i.e. RGB565, RGB666 or RGB888 on raw > RGB or LVDS busses). > > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> > --- > drivers/gpu/drm/drm_crtc.c | 28 ++++++++++++++++++++++++++++ > include/drm/drm_crtc.h | 8 ++++++++ > 2 files changed, 36 insertions(+) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index c808a09..50c8395 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -825,6 +825,34 @@ static void drm_mode_remove(struct drm_connector *connector, > drm_mode_destroy(connector->dev, mode); > } > > +/* > + * drm_display_info_set_bus_formats - set the supported bus formats > + * @info: display info to store bus formats in > + * @fmts: array containing the supported bus formats > + * @nfmts: the number of entries in the fmts array > + * > + * Store the suppported bus formats in display info structure. > + */ > +int drm_display_info_set_bus_formats(struct drm_display_info *info, > + const enum video_bus_format *fmts, > + int nfmts) Can you make nfmts unsigned please? > +{ > + enum video_bus_format *formats = NULL; > + > + if (fmts && nfmts) { > + formats = kmemdup(fmts, sizeof(*fmts) * nfmts, GFP_KERNEL); > + if (!formats) > + return -ENOMEM; > + } > + > + kfree(info->bus_formats); > + info->bus_formats = formats; > + info->nbus_formats = formats ? nfmts : 0; And perhaps check for formats == NULL && nfmts != 0 since that's not a valid pair of values. Then you can simply assign this directly without relying on the value of formats. Also other variable names use "num_" as a prefix instead of "n", so if you're going to respin anyway might as well make the names more consistent. > + > + return 0; > +} > +EXPORT_SYMBOL(drm_display_info_set_bus_formats); > + > /** > * drm_connector_init - Init a preallocated connector > * @dev: DRM device > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index e529b68..957729b 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -31,6 +31,7 @@ > #include <linux/idr.h> > #include <linux/fb.h> > #include <linux/hdmi.h> > +#include <linux/video-bus-format.h> > #include <drm/drm_mode.h> > #include <drm/drm_fourcc.h> > #include <drm/drm_modeset_lock.h> > @@ -121,6 +122,9 @@ struct drm_display_info { > enum subpixel_order subpixel_order; > u32 color_formats; > > + const enum video_bus_format *bus_formats; > + int nbus_formats; unsigned int here too, please. Thierry
Hi Thierry, On Tue, 23 Sep 2014 16:04:40 +0200 Thierry Reding <thierry.reding@gmail.com> wrote: > On Tue, Jul 22, 2014 at 02:23:45PM +0200, Boris BREZILLON wrote: > > Add bus_formats and nbus_formats fields and > > drm_display_info_set_bus_formats helper function to specify the bus > > formats supported by a given display. > > > > This information can be used by display controller drivers to configure > > the output interface appropriately (i.e. RGB565, RGB666 or RGB888 on raw > > RGB or LVDS busses). > > > > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> > > --- > > drivers/gpu/drm/drm_crtc.c | 28 ++++++++++++++++++++++++++++ > > include/drm/drm_crtc.h | 8 ++++++++ > > 2 files changed, 36 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > > index c808a09..50c8395 100644 > > --- a/drivers/gpu/drm/drm_crtc.c > > +++ b/drivers/gpu/drm/drm_crtc.c > > @@ -825,6 +825,34 @@ static void drm_mode_remove(struct drm_connector *connector, > > drm_mode_destroy(connector->dev, mode); > > } > > > > +/* > > + * drm_display_info_set_bus_formats - set the supported bus formats > > + * @info: display info to store bus formats in > > + * @fmts: array containing the supported bus formats > > + * @nfmts: the number of entries in the fmts array > > + * > > + * Store the suppported bus formats in display info structure. > > + */ > > +int drm_display_info_set_bus_formats(struct drm_display_info *info, > > + const enum video_bus_format *fmts, > > + int nfmts) > > Can you make nfmts unsigned please? Sure. > > > +{ > > + enum video_bus_format *formats = NULL; > > + > > + if (fmts && nfmts) { > > + formats = kmemdup(fmts, sizeof(*fmts) * nfmts, GFP_KERNEL); > > + if (!formats) > > + return -ENOMEM; > > + } > > + > > + kfree(info->bus_formats); > > + info->bus_formats = formats; > > + info->nbus_formats = formats ? nfmts : 0; > > And perhaps check for formats == NULL && nfmts != 0 since that's not a > valid pair of values. Then you can simply assign this directly without > relying on the value of formats. > > Also other variable names use "num_" as a prefix instead of "n", so if > you're going to respin anyway might as well make the names more > consistent. I'll rename the field and variable and add the proper check before assigning values. Thanks for your review. Best Regards, Boris
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index c808a09..50c8395 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -825,6 +825,34 @@ static void drm_mode_remove(struct drm_connector *connector, drm_mode_destroy(connector->dev, mode); } +/* + * drm_display_info_set_bus_formats - set the supported bus formats + * @info: display info to store bus formats in + * @fmts: array containing the supported bus formats + * @nfmts: the number of entries in the fmts array + * + * Store the suppported bus formats in display info structure. + */ +int drm_display_info_set_bus_formats(struct drm_display_info *info, + const enum video_bus_format *fmts, + int nfmts) +{ + enum video_bus_format *formats = NULL; + + if (fmts && nfmts) { + formats = kmemdup(fmts, sizeof(*fmts) * nfmts, GFP_KERNEL); + if (!formats) + return -ENOMEM; + } + + kfree(info->bus_formats); + info->bus_formats = formats; + info->nbus_formats = formats ? nfmts : 0; + + return 0; +} +EXPORT_SYMBOL(drm_display_info_set_bus_formats); + /** * drm_connector_init - Init a preallocated connector * @dev: DRM device diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index e529b68..957729b 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -31,6 +31,7 @@ #include <linux/idr.h> #include <linux/fb.h> #include <linux/hdmi.h> +#include <linux/video-bus-format.h> #include <drm/drm_mode.h> #include <drm/drm_fourcc.h> #include <drm/drm_modeset_lock.h> @@ -121,6 +122,9 @@ struct drm_display_info { enum subpixel_order subpixel_order; u32 color_formats; + const enum video_bus_format *bus_formats; + int nbus_formats; + /* Mask of supported hdmi deep color modes */ u8 edid_hdmi_dc_modes; @@ -964,6 +968,10 @@ extern int drm_mode_connector_set_path_property(struct drm_connector *connector, extern int drm_mode_connector_update_edid_property(struct drm_connector *connector, struct edid *edid); +extern int drm_display_info_set_bus_formats(struct drm_display_info *info, + const enum video_bus_format *fmts, + int nfmts); + static inline bool drm_property_type_is(struct drm_property *property, uint32_t type) {
Add bus_formats and nbus_formats fields and drm_display_info_set_bus_formats helper function to specify the bus formats supported by a given display. This information can be used by display controller drivers to configure the output interface appropriately (i.e. RGB565, RGB666 or RGB888 on raw RGB or LVDS busses). Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> --- drivers/gpu/drm/drm_crtc.c | 28 ++++++++++++++++++++++++++++ include/drm/drm_crtc.h | 8 ++++++++ 2 files changed, 36 insertions(+)