| Message ID | VI1PR03MB4206740285A775280063E303AC1C0@VI1PR03MB4206.eurprd03.prod.outlook.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge: dw-hdmi: Add support for HDR metadata | expand |
On 26.05.2019 23:20, Jonas Karlman wrote: > This patch enables Dynamic Range and Mastering InfoFrame on H6. > > Cc: Maxime Ripard <maxime.ripard@bootlin.com> > Cc: Jernej Skrabec <jernej.skrabec@siol.net> > Signed-off-by: Jonas Karlman <jonas@kwiboo.se> > --- > drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 2 ++ > drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 1 + > 2 files changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > index 39d8509d96a0..b80164dd8ad8 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > @@ -189,6 +189,7 @@ static int sun8i_dw_hdmi_bind(struct device *dev, struct device *master, > sun8i_hdmi_phy_init(hdmi->phy); > > plat_data->mode_valid = hdmi->quirks->mode_valid; > + plat_data->drm_infoframe = hdmi->quirks->drm_infoframe; > sun8i_hdmi_phy_set_ops(hdmi->phy, plat_data); > > platform_set_drvdata(pdev, hdmi); > @@ -255,6 +256,7 @@ static const struct sun8i_dw_hdmi_quirks sun8i_a83t_quirks = { > > static const struct sun8i_dw_hdmi_quirks sun50i_h6_quirks = { > .mode_valid = sun8i_dw_hdmi_mode_valid_h6, > + .drm_infoframe = true, > }; > > static const struct of_device_id sun8i_dw_hdmi_dt_ids[] = { > diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > index 720c5aa8adc1..2a0ec08ee236 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > @@ -178,6 +178,7 @@ struct sun8i_dw_hdmi_quirks { > enum drm_mode_status (*mode_valid)(struct drm_connector *connector, > const struct drm_display_mode *mode); > unsigned int set_rate : 1; > + unsigned int drm_infoframe : 1; Again, drm_infoframe suggests it contains inforframe, but in fact it just informs infoframe can be used, so again my suggestion use_drm_infoframe. Moreover bool type seems more appropriate here. Beside this: Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> -- Regards Andrzej > }; > > struct sun8i_dw_hdmi {
Dne ponedeljek, 24. junij 2019 ob 17:03:31 CEST je Andrzej Hajda napisal(a): > On 26.05.2019 23:20, Jonas Karlman wrote: > > This patch enables Dynamic Range and Mastering InfoFrame on H6. > > > > Cc: Maxime Ripard <maxime.ripard@bootlin.com> > > Cc: Jernej Skrabec <jernej.skrabec@siol.net> > > Signed-off-by: Jonas Karlman <jonas@kwiboo.se> > > --- > > > > drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 2 ++ > > drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 1 + > > 2 files changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > > b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c index 39d8509d96a0..b80164dd8ad8 > > 100644 > > --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > > +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > > @@ -189,6 +189,7 @@ static int sun8i_dw_hdmi_bind(struct device *dev, > > struct device *master,> > > sun8i_hdmi_phy_init(hdmi->phy); > > > > plat_data->mode_valid = hdmi->quirks->mode_valid; > > > > + plat_data->drm_infoframe = hdmi->quirks->drm_infoframe; > > > > sun8i_hdmi_phy_set_ops(hdmi->phy, plat_data); > > > > platform_set_drvdata(pdev, hdmi); > > > > @@ -255,6 +256,7 @@ static const struct sun8i_dw_hdmi_quirks > > sun8i_a83t_quirks = {> > > static const struct sun8i_dw_hdmi_quirks sun50i_h6_quirks = { > > > > .mode_valid = sun8i_dw_hdmi_mode_valid_h6, > > > > + .drm_infoframe = true, > > > > }; > > > > static const struct of_device_id sun8i_dw_hdmi_dt_ids[] = { > > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > > b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index 720c5aa8adc1..2a0ec08ee236 > > 100644 > > --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > > +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > > @@ -178,6 +178,7 @@ struct sun8i_dw_hdmi_quirks { > > > > enum drm_mode_status (*mode_valid)(struct drm_connector *connector, > > > > const struct drm_display_mode *mode); > > > > unsigned int set_rate : 1; > > > > + unsigned int drm_infoframe : 1; > > Again, drm_infoframe suggests it contains inforframe, but in fact it > just informs infoframe can be used, so again my suggestion > use_drm_infoframe. > > Moreover bool type seems more appropriate here. checkpatch will give warning if bool is used. Best regards, Jernej > > Beside this: > > Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> > > -- > Regards > Andrzej > > > }; > > > > struct sun8i_dw_hdmi {
On 24.06.2019 17:05, Jernej Škrabec wrote: > Dne ponedeljek, 24. junij 2019 ob 17:03:31 CEST je Andrzej Hajda napisal(a): >> On 26.05.2019 23:20, Jonas Karlman wrote: >>> This patch enables Dynamic Range and Mastering InfoFrame on H6. >>> >>> Cc: Maxime Ripard <maxime.ripard@bootlin.com> >>> Cc: Jernej Skrabec <jernej.skrabec@siol.net> >>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se> >>> --- >>> >>> drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 2 ++ >>> drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 1 + >>> 2 files changed, 3 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c >>> b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c index 39d8509d96a0..b80164dd8ad8 >>> 100644 >>> --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c >>> +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c >>> @@ -189,6 +189,7 @@ static int sun8i_dw_hdmi_bind(struct device *dev, >>> struct device *master,> >>> sun8i_hdmi_phy_init(hdmi->phy); >>> >>> plat_data->mode_valid = hdmi->quirks->mode_valid; >>> >>> + plat_data->drm_infoframe = hdmi->quirks->drm_infoframe; >>> >>> sun8i_hdmi_phy_set_ops(hdmi->phy, plat_data); >>> >>> platform_set_drvdata(pdev, hdmi); >>> >>> @@ -255,6 +256,7 @@ static const struct sun8i_dw_hdmi_quirks >>> sun8i_a83t_quirks = {> >>> static const struct sun8i_dw_hdmi_quirks sun50i_h6_quirks = { >>> >>> .mode_valid = sun8i_dw_hdmi_mode_valid_h6, >>> >>> + .drm_infoframe = true, >>> >>> }; >>> >>> static const struct of_device_id sun8i_dw_hdmi_dt_ids[] = { >>> >>> diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h >>> b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index 720c5aa8adc1..2a0ec08ee236 >>> 100644 >>> --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h >>> +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h >>> @@ -178,6 +178,7 @@ struct sun8i_dw_hdmi_quirks { >>> >>> enum drm_mode_status (*mode_valid)(struct drm_connector > *connector, >>> >>> const struct > drm_display_mode *mode); >>> >>> unsigned int set_rate : 1; >>> >>> + unsigned int drm_infoframe : 1; >> Again, drm_infoframe suggests it contains inforframe, but in fact it >> just informs infoframe can be used, so again my suggestion >> use_drm_infoframe. >> >> Moreover bool type seems more appropriate here. > checkpatch will give warning if bool is used. Then I would say "fix/ignore checkpatch" :) But maybe there is a reason. Anyway I've tested and I do not see the warning, could you elaborate it. Regards Andrzej > > Best regards, > Jernej > >> Beside this: >> >> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> >> >> -- >> Regards >> Andrzej >> >>> }; >>> >>> struct sun8i_dw_hdmi { > > > > >
On Mon, Jun 24, 2019 at 11:49 PM Andrzej Hajda <a.hajda@samsung.com> wrote: > > On 24.06.2019 17:05, Jernej Škrabec wrote: > > Dne ponedeljek, 24. junij 2019 ob 17:03:31 CEST je Andrzej Hajda napisal(a): > >> On 26.05.2019 23:20, Jonas Karlman wrote: > >>> This patch enables Dynamic Range and Mastering InfoFrame on H6. > >>> > >>> Cc: Maxime Ripard <maxime.ripard@bootlin.com> > >>> Cc: Jernej Skrabec <jernej.skrabec@siol.net> > >>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se> > >>> --- > >>> > >>> drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 2 ++ > >>> drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 1 + > >>> 2 files changed, 3 insertions(+) > >>> > >>> diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > >>> b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c index 39d8509d96a0..b80164dd8ad8 > >>> 100644 > >>> --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > >>> +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > >>> @@ -189,6 +189,7 @@ static int sun8i_dw_hdmi_bind(struct device *dev, > >>> struct device *master,> > >>> sun8i_hdmi_phy_init(hdmi->phy); > >>> > >>> plat_data->mode_valid = hdmi->quirks->mode_valid; > >>> > >>> + plat_data->drm_infoframe = hdmi->quirks->drm_infoframe; > >>> > >>> sun8i_hdmi_phy_set_ops(hdmi->phy, plat_data); > >>> > >>> platform_set_drvdata(pdev, hdmi); > >>> > >>> @@ -255,6 +256,7 @@ static const struct sun8i_dw_hdmi_quirks > >>> sun8i_a83t_quirks = {> > >>> static const struct sun8i_dw_hdmi_quirks sun50i_h6_quirks = { > >>> > >>> .mode_valid = sun8i_dw_hdmi_mode_valid_h6, > >>> > >>> + .drm_infoframe = true, > >>> > >>> }; > >>> > >>> static const struct of_device_id sun8i_dw_hdmi_dt_ids[] = { > >>> > >>> diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > >>> b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index 720c5aa8adc1..2a0ec08ee236 > >>> 100644 > >>> --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > >>> +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > >>> @@ -178,6 +178,7 @@ struct sun8i_dw_hdmi_quirks { > >>> > >>> enum drm_mode_status (*mode_valid)(struct drm_connector > > *connector, > >>> > >>> const struct > > drm_display_mode *mode); > >>> > >>> unsigned int set_rate : 1; > >>> > >>> + unsigned int drm_infoframe : 1; > >> Again, drm_infoframe suggests it contains inforframe, but in fact it > >> just informs infoframe can be used, so again my suggestion > >> use_drm_infoframe. > >> > >> Moreover bool type seems more appropriate here. > > checkpatch will give warning if bool is used. > > > Then I would say "fix/ignore checkpatch" :) > > But maybe there is a reason. Here's an old one from Linus: https://lkml.org/lkml/2013/9/1/154 I'd say that bool in a struct is a waste of space compared to a 1 bit bitfield, especially when there already are other bitfields in the same struct. > Anyway I've tested and I do not see the warning, could you elaborate it. Maybe checkpatch.pl --strict? ChenYu
Dne ponedeljek, 24. junij 2019 ob 17:56:30 CEST je Chen-Yu Tsai napisal(a): > On Mon, Jun 24, 2019 at 11:49 PM Andrzej Hajda <a.hajda@samsung.com> wrote: > > On 24.06.2019 17:05, Jernej Škrabec wrote: > > > Dne ponedeljek, 24. junij 2019 ob 17:03:31 CEST je Andrzej Hajda napisal(a): > > >> On 26.05.2019 23:20, Jonas Karlman wrote: > > >>> This patch enables Dynamic Range and Mastering InfoFrame on H6. > > >>> > > >>> Cc: Maxime Ripard <maxime.ripard@bootlin.com> > > >>> Cc: Jernej Skrabec <jernej.skrabec@siol.net> > > >>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se> > > >>> --- > > >>> > > >>> drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 2 ++ > > >>> drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 1 + > > >>> 2 files changed, 3 insertions(+) > > >>> > > >>> diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > > >>> b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c index > > >>> 39d8509d96a0..b80164dd8ad8 > > >>> 100644 > > >>> --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > > >>> +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > > >>> @@ -189,6 +189,7 @@ static int sun8i_dw_hdmi_bind(struct device *dev, > > >>> struct device *master,> > > >>> > > >>> sun8i_hdmi_phy_init(hdmi->phy); > > >>> > > >>> plat_data->mode_valid = hdmi->quirks->mode_valid; > > >>> > > >>> + plat_data->drm_infoframe = hdmi->quirks->drm_infoframe; > > >>> > > >>> sun8i_hdmi_phy_set_ops(hdmi->phy, plat_data); > > >>> > > >>> platform_set_drvdata(pdev, hdmi); > > >>> > > >>> @@ -255,6 +256,7 @@ static const struct sun8i_dw_hdmi_quirks > > >>> sun8i_a83t_quirks = {> > > >>> > > >>> static const struct sun8i_dw_hdmi_quirks sun50i_h6_quirks = { > > >>> > > >>> .mode_valid = sun8i_dw_hdmi_mode_valid_h6, > > >>> > > >>> + .drm_infoframe = true, > > >>> > > >>> }; > > >>> > > >>> static const struct of_device_id sun8i_dw_hdmi_dt_ids[] = { > > >>> > > >>> diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > > >>> b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index > > >>> 720c5aa8adc1..2a0ec08ee236 > > >>> 100644 > > >>> --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > > >>> +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > > >>> @@ -178,6 +178,7 @@ struct sun8i_dw_hdmi_quirks { > > >>> > > >>> enum drm_mode_status (*mode_valid)(struct drm_connector > > > > > > *connector, > > > > > >>> const struct > > > > > > drm_display_mode *mode); > > > > > >>> unsigned int set_rate : 1; > > >>> > > >>> + unsigned int drm_infoframe : 1; > > >> > > >> Again, drm_infoframe suggests it contains inforframe, but in fact it > > >> just informs infoframe can be used, so again my suggestion > > >> use_drm_infoframe. > > >> > > >> Moreover bool type seems more appropriate here. > > > > > > checkpatch will give warning if bool is used. > > > > Then I would say "fix/ignore checkpatch" :) > > > > But maybe there is a reason. > > Here's an old one from Linus: https://lkml.org/lkml/2013/9/1/154 > > I'd say that bool in a struct is a waste of space compared to a 1 bit > bitfield, especially when there already are other bitfields in the same > struct. > > Anyway I've tested and I do not see the warning, could you elaborate it. > > Maybe checkpatch.pl --strict? It seems they removed that check: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/? id=7967656ffbfa493f5546c0f1 After reading that block of text, I'm not sure what would be prefered way for this case. Best regards, Jernej
On Tue, Jun 25, 2019 at 12:03 AM Jernej Škrabec <jernej.skrabec@siol.net> wrote: > > Dne ponedeljek, 24. junij 2019 ob 17:56:30 CEST je Chen-Yu Tsai napisal(a): > > On Mon, Jun 24, 2019 at 11:49 PM Andrzej Hajda <a.hajda@samsung.com> wrote: > > > On 24.06.2019 17:05, Jernej Škrabec wrote: > > > > Dne ponedeljek, 24. junij 2019 ob 17:03:31 CEST je Andrzej Hajda > napisal(a): > > > >> On 26.05.2019 23:20, Jonas Karlman wrote: > > > >>> This patch enables Dynamic Range and Mastering InfoFrame on H6. > > > >>> > > > >>> Cc: Maxime Ripard <maxime.ripard@bootlin.com> > > > >>> Cc: Jernej Skrabec <jernej.skrabec@siol.net> > > > >>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se> > > > >>> --- > > > >>> > > > >>> drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 2 ++ > > > >>> drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 1 + > > > >>> 2 files changed, 3 insertions(+) > > > >>> > > > >>> diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > > > >>> b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c index > > > >>> 39d8509d96a0..b80164dd8ad8 > > > >>> 100644 > > > >>> --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > > > >>> +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > > > >>> @@ -189,6 +189,7 @@ static int sun8i_dw_hdmi_bind(struct device *dev, > > > >>> struct device *master,> > > > >>> > > > >>> sun8i_hdmi_phy_init(hdmi->phy); > > > >>> > > > >>> plat_data->mode_valid = hdmi->quirks->mode_valid; > > > >>> > > > >>> + plat_data->drm_infoframe = hdmi->quirks->drm_infoframe; > > > >>> > > > >>> sun8i_hdmi_phy_set_ops(hdmi->phy, plat_data); > > > >>> > > > >>> platform_set_drvdata(pdev, hdmi); > > > >>> > > > >>> @@ -255,6 +256,7 @@ static const struct sun8i_dw_hdmi_quirks > > > >>> sun8i_a83t_quirks = {> > > > >>> > > > >>> static const struct sun8i_dw_hdmi_quirks sun50i_h6_quirks = { > > > >>> > > > >>> .mode_valid = sun8i_dw_hdmi_mode_valid_h6, > > > >>> > > > >>> + .drm_infoframe = true, > > > >>> > > > >>> }; > > > >>> > > > >>> static const struct of_device_id sun8i_dw_hdmi_dt_ids[] = { > > > >>> > > > >>> diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > > > >>> b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index > > > >>> 720c5aa8adc1..2a0ec08ee236 > > > >>> 100644 > > > >>> --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > > > >>> +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > > > >>> @@ -178,6 +178,7 @@ struct sun8i_dw_hdmi_quirks { > > > >>> > > > >>> enum drm_mode_status (*mode_valid)(struct drm_connector > > > > > > > > *connector, > > > > > > > >>> const struct > > > > > > > > drm_display_mode *mode); > > > > > > > >>> unsigned int set_rate : 1; > > > >>> > > > >>> + unsigned int drm_infoframe : 1; > > > >> > > > >> Again, drm_infoframe suggests it contains inforframe, but in fact it > > > >> just informs infoframe can be used, so again my suggestion > > > >> use_drm_infoframe. > > > >> > > > >> Moreover bool type seems more appropriate here. > > > > > > > > checkpatch will give warning if bool is used. > > > > > > Then I would say "fix/ignore checkpatch" :) > > > > > > But maybe there is a reason. > > > > Here's an old one from Linus: https://lkml.org/lkml/2013/9/1/154 > > > > I'd say that bool in a struct is a waste of space compared to a 1 bit > > bitfield, especially when there already are other bitfields in the same > > struct. > > > Anyway I've tested and I do not see the warning, could you elaborate it. > > > > Maybe checkpatch.pl --strict? > > It seems they removed that check: > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/? > id=7967656ffbfa493f5546c0f1 > > After reading that block of text, I'm not sure what would be prefered way for > this case. This: +If a structure has many true/false values, consider consolidating them into a +bitfield with 1 bit members, or using an appropriate fixed width type, such as +u8. would suggest using a bitfield, or flags within a fixed width type? ChenYu
On 24.06.2019 18:07, Chen-Yu Tsai wrote: > On Tue, Jun 25, 2019 at 12:03 AM Jernej Škrabec <jernej.skrabec@siol.net> wrote: >> Dne ponedeljek, 24. junij 2019 ob 17:56:30 CEST je Chen-Yu Tsai napisal(a): >>> On Mon, Jun 24, 2019 at 11:49 PM Andrzej Hajda <a.hajda@samsung.com> wrote: >>>> On 24.06.2019 17:05, Jernej Škrabec wrote: >>>>> Dne ponedeljek, 24. junij 2019 ob 17:03:31 CEST je Andrzej Hajda >> napisal(a): >>>>>> On 26.05.2019 23:20, Jonas Karlman wrote: >>>>>>> This patch enables Dynamic Range and Mastering InfoFrame on H6. >>>>>>> >>>>>>> Cc: Maxime Ripard <maxime.ripard@bootlin.com> >>>>>>> Cc: Jernej Skrabec <jernej.skrabec@siol.net> >>>>>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se> >>>>>>> --- >>>>>>> >>>>>>> drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 2 ++ >>>>>>> drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 1 + >>>>>>> 2 files changed, 3 insertions(+) >>>>>>> >>>>>>> diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c >>>>>>> b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c index >>>>>>> 39d8509d96a0..b80164dd8ad8 >>>>>>> 100644 >>>>>>> --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c >>>>>>> +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c >>>>>>> @@ -189,6 +189,7 @@ static int sun8i_dw_hdmi_bind(struct device *dev, >>>>>>> struct device *master,> >>>>>>> >>>>>>> sun8i_hdmi_phy_init(hdmi->phy); >>>>>>> >>>>>>> plat_data->mode_valid = hdmi->quirks->mode_valid; >>>>>>> >>>>>>> + plat_data->drm_infoframe = hdmi->quirks->drm_infoframe; >>>>>>> >>>>>>> sun8i_hdmi_phy_set_ops(hdmi->phy, plat_data); >>>>>>> >>>>>>> platform_set_drvdata(pdev, hdmi); >>>>>>> >>>>>>> @@ -255,6 +256,7 @@ static const struct sun8i_dw_hdmi_quirks >>>>>>> sun8i_a83t_quirks = {> >>>>>>> >>>>>>> static const struct sun8i_dw_hdmi_quirks sun50i_h6_quirks = { >>>>>>> >>>>>>> .mode_valid = sun8i_dw_hdmi_mode_valid_h6, >>>>>>> >>>>>>> + .drm_infoframe = true, >>>>>>> >>>>>>> }; >>>>>>> >>>>>>> static const struct of_device_id sun8i_dw_hdmi_dt_ids[] = { >>>>>>> >>>>>>> diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h >>>>>>> b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index >>>>>>> 720c5aa8adc1..2a0ec08ee236 >>>>>>> 100644 >>>>>>> --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h >>>>>>> +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h >>>>>>> @@ -178,6 +178,7 @@ struct sun8i_dw_hdmi_quirks { >>>>>>> >>>>>>> enum drm_mode_status (*mode_valid)(struct drm_connector >>>>> *connector, >>>>> >>>>>>> const struct >>>>> drm_display_mode *mode); >>>>> >>>>>>> unsigned int set_rate : 1; >>>>>>> >>>>>>> + unsigned int drm_infoframe : 1; >>>>>> Again, drm_infoframe suggests it contains inforframe, but in fact it >>>>>> just informs infoframe can be used, so again my suggestion >>>>>> use_drm_infoframe. >>>>>> >>>>>> Moreover bool type seems more appropriate here. >>>>> checkpatch will give warning if bool is used. >>>> Then I would say "fix/ignore checkpatch" :) >>>> >>>> But maybe there is a reason. >>> Here's an old one from Linus: https://lkml.org/lkml/2013/9/1/154 >>> >>> I'd say that bool in a struct is a waste of space compared to a 1 bit >>> bitfield, especially when there already are other bitfields in the same >>> struct. >>>> Anyway I've tested and I do not see the warning, could you elaborate it. >>> Maybe checkpatch.pl --strict? >> It seems they removed that check: >> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/? >> id=7967656ffbfa493f5546c0f1 >> >> After reading that block of text, I'm not sure what would be prefered way for >> this case. > This: > > +If a structure has many true/false values, consider consolidating them into a > +bitfield with 1 bit members, or using an appropriate fixed width type, such as > +u8. > > would suggest using a bitfield, or flags within a fixed width type? OK, I have also guessed what kind of warning Jernej was writing about. And IMO it rather does not fit here: - no concurrent writes, - no need for size/cache optimizations. But since there are some controversies about bool in struct and file has already convention of bitfield I do not insist on it, you can keep it as is. Regards Andrzej > > ChenYu > >
diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c index 39d8509d96a0..b80164dd8ad8 100644 --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c @@ -189,6 +189,7 @@ static int sun8i_dw_hdmi_bind(struct device *dev, struct device *master, sun8i_hdmi_phy_init(hdmi->phy); plat_data->mode_valid = hdmi->quirks->mode_valid; + plat_data->drm_infoframe = hdmi->quirks->drm_infoframe; sun8i_hdmi_phy_set_ops(hdmi->phy, plat_data); platform_set_drvdata(pdev, hdmi); @@ -255,6 +256,7 @@ static const struct sun8i_dw_hdmi_quirks sun8i_a83t_quirks = { static const struct sun8i_dw_hdmi_quirks sun50i_h6_quirks = { .mode_valid = sun8i_dw_hdmi_mode_valid_h6, + .drm_infoframe = true, }; static const struct of_device_id sun8i_dw_hdmi_dt_ids[] = { diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index 720c5aa8adc1..2a0ec08ee236 100644 --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h @@ -178,6 +178,7 @@ struct sun8i_dw_hdmi_quirks { enum drm_mode_status (*mode_valid)(struct drm_connector *connector, const struct drm_display_mode *mode); unsigned int set_rate : 1; + unsigned int drm_infoframe : 1; }; struct sun8i_dw_hdmi {
This patch enables Dynamic Range and Mastering InfoFrame on H6. Cc: Maxime Ripard <maxime.ripard@bootlin.com> Cc: Jernej Skrabec <jernej.skrabec@siol.net> Signed-off-by: Jonas Karlman <jonas@kwiboo.se> --- drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 2 ++ drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 1 + 2 files changed, 3 insertions(+)