> -----Original Message----- > From: linux-omap-owner@xxxxxxxxxxxxxxx [mailto:linux-omap- > owner@xxxxxxxxxxxxxxx] On Behalf Of K, Mythri P > Sent: Wednesday, March 09, 2011 5:15 PM > To: linux-omap@xxxxxxxxxxxxxxx; Valkeinen, Tomi > Cc: K, Mythri P > Subject: [PATCH v4 3/9] OMAP4 : DSS2 : HDMI: HDMI dispc gamma table > disable. [sp] HDMI seems repetitive in the subject in this and some of the subsequent Patches. > > Adding changes to set gamma table bit for TV interface to make sure it is > disabled [sp] Is this bit expected to be "enabled" by default? If no, how can/would this bit get enabled. The subject and description of the patch doesn't match the contents. The patch is adding a function that can *both* enable/disable gamma table - not just disable. > > Signed-off-by: Mythri P K <mythripk@xxxxxx> > --- > drivers/video/omap2/dss/dispc.c | 10 ++++++++++ > drivers/video/omap2/dss/dss.h | 1 + > 2 files changed, 11 insertions(+), 0 deletions(-) > > diff --git a/drivers/video/omap2/dss/dispc.c > b/drivers/video/omap2/dss/dispc.c > index 4a368c4..0defe16 100644 > --- a/drivers/video/omap2/dss/dispc.c > +++ b/drivers/video/omap2/dss/dispc.c > @@ -1008,6 +1008,16 @@ void dispc_set_burst_size(enum omap_plane plane, > enable_clocks(0); > } > > +void dispc_enable_gamma_table(bool enable) > +{ > + /* This is partially implemented to support only > + * disabling of the gamma table. > + */ > + BUG_ON(enable); [sp] Is BUG_ON really needed. Can't this be a WARN or equiv? > + > + REG_FLD_MOD(DISPC_CONFIG, enable, 9, 9); [sp] I don't understand HDMI much, but if enabling gamma causes BUG_ON, then why would we need implementation to disable it. Instead, shouldn't the function be empty - if it can't be avoided altogether? > +} > + > static void _dispc_set_vid_color_conv(enum omap_plane plane, bool enable) > { > u32 val; > diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h > index 29f31fd..008cbf4 100644 > --- a/drivers/video/omap2/dss/dss.h > +++ b/drivers/video/omap2/dss/dss.h > @@ -363,6 +363,7 @@ void dispc_set_plane_size(enum omap_plane plane, u16 > width, u16 height); > void dispc_set_channel_out(enum omap_plane plane, > enum omap_channel channel_out); > > +void dispc_enable_gamma_table(bool enable); [sp] Is this patch really standalone? I suggest merging it with the patch where the API is actually used. In effect, nothing gets disabled in this patch. > int dispc_setup_plane(enum omap_plane plane, > u32 paddr, u16 screen_width, > u16 pos_x, u16 pos_y, > -- > 1.5.6.3 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
