diff mbox series

[v1,4/4] drm/panel: ili9806e: Break some CMDS into helper functions

Message ID 20240710084715.1119935-5-yangcong5@huaqin.corp-partner.google.com (mailing list archive)
State New, archived
Headers show
Series Break some CMDS into helper functions | expand

Commit Message

cong yang July 10, 2024, 8:47 a.m. UTC
Break select page cmds into helper function.

Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
---
 drivers/gpu/drm/panel/panel-ilitek-ili9806e.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

Comments

Michael Walle July 10, 2024, 9:02 a.m. UTC | #1
On Wed Jul 10, 2024 at 10:47 AM CEST, Cong Yang wrote:
> Break select page cmds into helper function.

Why though? I don't find that anything easier to read. In fact, I
deliberately chose not to factor that out into a function. It's just
a sequence of magic commands, taken straight from the datasheet. So,
I'd like to keep it that way.

-michael

> Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
> ---
>  drivers/gpu/drm/panel/panel-ilitek-ili9806e.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c b/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c
> index e4a44cd26c4d..68fb9a1a4d80 100644
> --- a/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c
> +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c
> @@ -35,6 +35,12 @@ struct ili9806e_panel {
>  	enum drm_panel_orientation orientation;
>  };
>  
> +#define ILI9806E_DCS_SWITCH_PAGE	0xff
> +
> +#define ili9806e_switch_page(ctx, page) \
> +	mipi_dsi_dcs_write_seq_multi(ctx, ILI9806E_DCS_SWITCH_PAGE, \
> +				     0xff, 0x98, 0x06, 0x04, (page))
> +
>  static const char * const regulator_names[] = {
>  	"vdd",
>  	"vccio",
> @@ -227,7 +233,7 @@ static void ili9806e_dsi_remove(struct mipi_dsi_device *dsi)
>  static void com35h3p70ulc_init(struct mipi_dsi_multi_context *ctx)
>  {
>  	/* Switch to page 1 */
> -	mipi_dsi_dcs_write_seq_multi(ctx, 0xff, 0xff, 0x98, 0x06, 0x04, 0x01);
> +	ili9806e_switch_page(ctx, 0x01);
>  	/* Interface Settings */
>  	mipi_dsi_dcs_write_seq_multi(ctx, 0x08, 0x18);
>  	mipi_dsi_dcs_write_seq_multi(ctx, 0x21, 0x01);
> @@ -285,14 +291,14 @@ static void com35h3p70ulc_init(struct mipi_dsi_multi_context *ctx)
>  	mipi_dsi_dcs_write_seq_multi(ctx, 0xcf, 0x0a);
>  
>  	/* Switch to page 7 */
> -	mipi_dsi_dcs_write_seq_multi(ctx, 0xff, 0xff, 0x98, 0x06, 0x04, 0x07);
> +	ili9806e_switch_page(ctx, 0x07);
>  	/* Power Control */
>  	mipi_dsi_dcs_write_seq_multi(ctx, 0x06, 0x00);
>  	mipi_dsi_dcs_write_seq_multi(ctx, 0x18, 0x1d);
>  	mipi_dsi_dcs_write_seq_multi(ctx, 0x17, 0x32);
>  
>  	/* Switch to page 6 */
> -	mipi_dsi_dcs_write_seq_multi(ctx, 0xff, 0xff, 0x98, 0x06, 0x04, 0x06);
> +	ili9806e_switch_page(ctx, 0x06);
>  	/* GIP settings */
>  	mipi_dsi_dcs_write_seq_multi(ctx, 0x00, 0x20);
>  	mipi_dsi_dcs_write_seq_multi(ctx, 0x01, 0x02);
> @@ -352,7 +358,7 @@ static void com35h3p70ulc_init(struct mipi_dsi_multi_context *ctx)
>  	mipi_dsi_dcs_write_seq_multi(ctx, 0x53, 0x12);
>  
>  	/* Switch to page 0 */
> -	mipi_dsi_dcs_write_seq_multi(ctx, 0xff, 0xff, 0x98, 0x06, 0x04, 0x00);
> +	ili9806e_switch_page(ctx, 0x00);
>  	/* Interface Pixel format */
>  	mipi_dsi_dcs_write_seq_multi(ctx, 0x3a, 0x60);
>  };
Doug Anderson July 10, 2024, 7:12 p.m. UTC | #2
Hi,

On Wed, Jul 10, 2024 at 2:02 AM Michael Walle <mwalle@kernel.org> wrote:
>
> On Wed Jul 10, 2024 at 10:47 AM CEST, Cong Yang wrote:
> > Break select page cmds into helper function.
>
> Why though? I don't find that anything easier to read. In fact, I
> deliberately chose not to factor that out into a function. It's just
> a sequence of magic commands, taken straight from the datasheet. So,
> I'd like to keep it that way.

The consensus of previous discussion on the lists was that folks
agreed that we should, where possible, make it more obvious what these
magic sequences of commands were doing. IMO separating out the page
switch command helps. Certainly I'm always happy to hear other
opinions, though.


> -michael
>
> > Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
> > ---
> >  drivers/gpu/drm/panel/panel-ilitek-ili9806e.c | 14 ++++++++++----
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c b/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c
> > index e4a44cd26c4d..68fb9a1a4d80 100644
> > --- a/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c
> > +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c
> > @@ -35,6 +35,12 @@ struct ili9806e_panel {
> >       enum drm_panel_orientation orientation;
> >  };
> >
> > +#define ILI9806E_DCS_SWITCH_PAGE     0xff
> > +
> > +#define ili9806e_switch_page(ctx, page) \
> > +     mipi_dsi_dcs_write_seq_multi(ctx, ILI9806E_DCS_SWITCH_PAGE, \
> > +                                  0xff, 0x98, 0x06, 0x04, (page))
> > +
> >  static const char * const regulator_names[] = {
> >       "vdd",
> >       "vccio",
> > @@ -227,7 +233,7 @@ static void ili9806e_dsi_remove(struct mipi_dsi_device *dsi)
> >  static void com35h3p70ulc_init(struct mipi_dsi_multi_context *ctx)
> >  {
> >       /* Switch to page 1 */
> > -     mipi_dsi_dcs_write_seq_multi(ctx, 0xff, 0xff, 0x98, 0x06, 0x04, 0x01);
> > +     ili9806e_switch_page(ctx, 0x01);

I think with your change you should remove the "Switch to page X"
comments since they're now obvious. Other than that, I'm happy with:

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Michael Walle July 10, 2024, 7:38 p.m. UTC | #3
On Wed Jul 10, 2024 at 9:12 PM CEST, Doug Anderson wrote:
> Hi,
>
> On Wed, Jul 10, 2024 at 2:02 AM Michael Walle <mwalle@kernel.org> wrote:
> >
> > On Wed Jul 10, 2024 at 10:47 AM CEST, Cong Yang wrote:
> > > Break select page cmds into helper function.
> >
> > Why though? I don't find that anything easier to read. In fact, I
> > deliberately chose not to factor that out into a function. It's just
> > a sequence of magic commands, taken straight from the datasheet. So,
> > I'd like to keep it that way.
>
> The consensus of previous discussion on the lists was that folks
> agreed that we should, where possible, make it more obvious what these
> magic sequences of commands were doing. IMO separating out the page
> switch command helps. Certainly I'm always happy to hear other
> opinions, though.

Fair enough, but in that case, one should take the datasheet (which
you can find online) and replace all the magic numbers with the
correct command names from it. E.g. 0xff is the ENEXTC register. To
be clear, I'm not just talking about the "switch page command".

As patch stands, I don't see much value, TBH. On the contrary, you
make it harder to compare it with the Ortustech panel datasheet.

just my 2c,
-michael

> > -michael
> >
> > > Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
> > > ---
> > >  drivers/gpu/drm/panel/panel-ilitek-ili9806e.c | 14 ++++++++++----
> > >  1 file changed, 10 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c b/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c
> > > index e4a44cd26c4d..68fb9a1a4d80 100644
> > > --- a/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c
> > > +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c
> > > @@ -35,6 +35,12 @@ struct ili9806e_panel {
> > >       enum drm_panel_orientation orientation;
> > >  };
> > >
> > > +#define ILI9806E_DCS_SWITCH_PAGE     0xff
> > > +
> > > +#define ili9806e_switch_page(ctx, page) \
> > > +     mipi_dsi_dcs_write_seq_multi(ctx, ILI9806E_DCS_SWITCH_PAGE, \
> > > +                                  0xff, 0x98, 0x06, 0x04, (page))
> > > +
> > >  static const char * const regulator_names[] = {
> > >       "vdd",
> > >       "vccio",
> > > @@ -227,7 +233,7 @@ static void ili9806e_dsi_remove(struct mipi_dsi_device *dsi)
> > >  static void com35h3p70ulc_init(struct mipi_dsi_multi_context *ctx)
> > >  {
> > >       /* Switch to page 1 */
> > > -     mipi_dsi_dcs_write_seq_multi(ctx, 0xff, 0xff, 0x98, 0x06, 0x04, 0x01);
> > > +     ili9806e_switch_page(ctx, 0x01);
>
> I think with your change you should remove the "Switch to page X"
> comments since they're now obvious. Other than that, I'm happy with:
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
cong yang July 11, 2024, 1:08 a.m. UTC | #4
Hi,

Michael Walle <mwalle@kernel.org> 于2024年7月11日周四 03:38写道:
>
> On Wed Jul 10, 2024 at 9:12 PM CEST, Doug Anderson wrote:
> > Hi,
> >
> > On Wed, Jul 10, 2024 at 2:02 AM Michael Walle <mwalle@kernel.org> wrote:
> > >
> > > On Wed Jul 10, 2024 at 10:47 AM CEST, Cong Yang wrote:
> > > > Break select page cmds into helper function.
> > >
> > > Why though? I don't find that anything easier to read. In fact, I
> > > deliberately chose not to factor that out into a function. It's just
> > > a sequence of magic commands, taken straight from the datasheet. So,
> > > I'd like to keep it that way.
> >
> > The consensus of previous discussion on the lists was that folks
> > agreed that we should, where possible, make it more obvious what these
> > magic sequences of commands were doing. IMO separating out the page
> > switch command helps. Certainly I'm always happy to hear other
> > opinions, though.
>
> Fair enough, but in that case, one should take the datasheet (which
> you can find online) and replace all the magic numbers with the
> correct command names from it. E.g. 0xff is the ENEXTC register. To
> be clear, I'm not just talking about the "switch page command".
>
> As patch stands, I don't see much value, TBH. On the contrary, you
> make it harder to compare it with the Ortustech panel datasheet.
>
> just my 2c,
> -michael

If all drivers replace all the magic numbers with the correct command names,
it will be a huge amount of work (assuming that the datasheet can be found).
 I am afraid I don't have enough time to complete it.  Thanks.

>
> > > -michael
> > >
> > > > Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
> > > > ---
> > > >  drivers/gpu/drm/panel/panel-ilitek-ili9806e.c | 14 ++++++++++----
> > > >  1 file changed, 10 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c b/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c
> > > > index e4a44cd26c4d..68fb9a1a4d80 100644
> > > > --- a/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c
> > > > +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c
> > > > @@ -35,6 +35,12 @@ struct ili9806e_panel {
> > > >       enum drm_panel_orientation orientation;
> > > >  };
> > > >
> > > > +#define ILI9806E_DCS_SWITCH_PAGE     0xff
> > > > +
> > > > +#define ili9806e_switch_page(ctx, page) \
> > > > +     mipi_dsi_dcs_write_seq_multi(ctx, ILI9806E_DCS_SWITCH_PAGE, \
> > > > +                                  0xff, 0x98, 0x06, 0x04, (page))
> > > > +
> > > >  static const char * const regulator_names[] = {
> > > >       "vdd",
> > > >       "vccio",
> > > > @@ -227,7 +233,7 @@ static void ili9806e_dsi_remove(struct mipi_dsi_device *dsi)
> > > >  static void com35h3p70ulc_init(struct mipi_dsi_multi_context *ctx)
> > > >  {
> > > >       /* Switch to page 1 */
> > > > -     mipi_dsi_dcs_write_seq_multi(ctx, 0xff, 0xff, 0x98, 0x06, 0x04, 0x01);
> > > > +     ili9806e_switch_page(ctx, 0x01);
> >
> > I think with your change you should remove the "Switch to page X"
> > comments since they're now obvious. Other than that, I'm happy with:
> >
> > Reviewed-by: Douglas Anderson <dianders@chromium.org>
>
Doug Anderson July 11, 2024, 7:36 p.m. UTC | #5
Hi,

On Wed, Jul 10, 2024 at 6:09 PM cong yang
<yangcong5@huaqin.corp-partner.google.com> wrote:
>
> Hi,
>
> Michael Walle <mwalle@kernel.org> 于2024年7月11日周四 03:38写道:
> >
> > On Wed Jul 10, 2024 at 9:12 PM CEST, Doug Anderson wrote:
> > > Hi,
> > >
> > > On Wed, Jul 10, 2024 at 2:02 AM Michael Walle <mwalle@kernel.org> wrote:
> > > >
> > > > On Wed Jul 10, 2024 at 10:47 AM CEST, Cong Yang wrote:
> > > > > Break select page cmds into helper function.
> > > >
> > > > Why though? I don't find that anything easier to read. In fact, I
> > > > deliberately chose not to factor that out into a function. It's just
> > > > a sequence of magic commands, taken straight from the datasheet. So,
> > > > I'd like to keep it that way.
> > >
> > > The consensus of previous discussion on the lists was that folks
> > > agreed that we should, where possible, make it more obvious what these
> > > magic sequences of commands were doing. IMO separating out the page
> > > switch command helps. Certainly I'm always happy to hear other
> > > opinions, though.
> >
> > Fair enough, but in that case, one should take the datasheet (which
> > you can find online) and replace all the magic numbers with the
> > correct command names from it. E.g. 0xff is the ENEXTC register. To
> > be clear, I'm not just talking about the "switch page command".
> >
> > As patch stands, I don't see much value, TBH. On the contrary, you
> > make it harder to compare it with the Ortustech panel datasheet.
> >
> > just my 2c,
> > -michael
>
> If all drivers replace all the magic numbers with the correct command names,
> it will be a huge amount of work (assuming that the datasheet can be found).
>  I am afraid I don't have enough time to complete it.  Thanks.

Makes sense. I'd be interested in hearing the opinion of others in the
DRM community about whether they'd prefer to land something long this
patch as-is or drop it.

-Doug
Neil Armstrong July 12, 2024, 2:56 p.m. UTC | #6
On 11/07/2024 21:36, Doug Anderson wrote:
> Hi,
> 
> On Wed, Jul 10, 2024 at 6:09 PM cong yang
> <yangcong5@huaqin.corp-partner.google.com> wrote:
>>
>> Hi,
>>
>> Michael Walle <mwalle@kernel.org> 于2024年7月11日周四 03:38写道:
>>>
>>> On Wed Jul 10, 2024 at 9:12 PM CEST, Doug Anderson wrote:
>>>> Hi,
>>>>
>>>> On Wed, Jul 10, 2024 at 2:02 AM Michael Walle <mwalle@kernel.org> wrote:
>>>>>
>>>>> On Wed Jul 10, 2024 at 10:47 AM CEST, Cong Yang wrote:
>>>>>> Break select page cmds into helper function.
>>>>>
>>>>> Why though? I don't find that anything easier to read. In fact, I
>>>>> deliberately chose not to factor that out into a function. It's just
>>>>> a sequence of magic commands, taken straight from the datasheet. So,
>>>>> I'd like to keep it that way.
>>>>
>>>> The consensus of previous discussion on the lists was that folks
>>>> agreed that we should, where possible, make it more obvious what these
>>>> magic sequences of commands were doing. IMO separating out the page
>>>> switch command helps. Certainly I'm always happy to hear other
>>>> opinions, though.
>>>
>>> Fair enough, but in that case, one should take the datasheet (which
>>> you can find online) and replace all the magic numbers with the
>>> correct command names from it. E.g. 0xff is the ENEXTC register. To
>>> be clear, I'm not just talking about the "switch page command".
>>>
>>> As patch stands, I don't see much value, TBH. On the contrary, you
>>> make it harder to compare it with the Ortustech panel datasheet.
>>>
>>> just my 2c,
>>> -michael
>>
>> If all drivers replace all the magic numbers with the correct command names,
>> it will be a huge amount of work (assuming that the datasheet can be found).
>>   I am afraid I don't have enough time to complete it.  Thanks.
> 
> Makes sense. I'd be interested in hearing the opinion of others in the
> DRM community about whether they'd prefer to land something long this
> patch as-is or drop it.

I don't have a strong opinion, but I think only changing the switch
page operations doesn't make a lot of sense by itself.

Having a much larger register coverage would be preferred.

Neil

> 
> -Doug
Doug Anderson July 12, 2024, 3:50 p.m. UTC | #7
Hi,

On Fri, Jul 12, 2024 at 7:56 AM <neil.armstrong@linaro.org> wrote:
>
> On 11/07/2024 21:36, Doug Anderson wrote:
> > Hi,
> >
> > On Wed, Jul 10, 2024 at 6:09 PM cong yang
> > <yangcong5@huaqin.corp-partner.google.com> wrote:
> >>
> >> Hi,
> >>
> >> Michael Walle <mwalle@kernel.org> 于2024年7月11日周四 03:38写道:
> >>>
> >>> On Wed Jul 10, 2024 at 9:12 PM CEST, Doug Anderson wrote:
> >>>> Hi,
> >>>>
> >>>> On Wed, Jul 10, 2024 at 2:02 AM Michael Walle <mwalle@kernel.org> wrote:
> >>>>>
> >>>>> On Wed Jul 10, 2024 at 10:47 AM CEST, Cong Yang wrote:
> >>>>>> Break select page cmds into helper function.
> >>>>>
> >>>>> Why though? I don't find that anything easier to read. In fact, I
> >>>>> deliberately chose not to factor that out into a function. It's just
> >>>>> a sequence of magic commands, taken straight from the datasheet. So,
> >>>>> I'd like to keep it that way.
> >>>>
> >>>> The consensus of previous discussion on the lists was that folks
> >>>> agreed that we should, where possible, make it more obvious what these
> >>>> magic sequences of commands were doing. IMO separating out the page
> >>>> switch command helps. Certainly I'm always happy to hear other
> >>>> opinions, though.
> >>>
> >>> Fair enough, but in that case, one should take the datasheet (which
> >>> you can find online) and replace all the magic numbers with the
> >>> correct command names from it. E.g. 0xff is the ENEXTC register. To
> >>> be clear, I'm not just talking about the "switch page command".
> >>>
> >>> As patch stands, I don't see much value, TBH. On the contrary, you
> >>> make it harder to compare it with the Ortustech panel datasheet.
> >>>
> >>> just my 2c,
> >>> -michael
> >>
> >> If all drivers replace all the magic numbers with the correct command names,
> >> it will be a huge amount of work (assuming that the datasheet can be found).
> >>   I am afraid I don't have enough time to complete it.  Thanks.
> >
> > Makes sense. I'd be interested in hearing the opinion of others in the
> > DRM community about whether they'd prefer to land something long this
> > patch as-is or drop it.
>
> I don't have a strong opinion, but I think only changing the switch
> page operations doesn't make a lot of sense by itself.

Does that mean you think we should drop this whole series? For the
"panel-ilitek-ili9806e.c" driver dropping seems fine since the switch
page command (and many of the other blocks of commands) is commented,
but for the other panels in this series IMO even just getting the
switch page adds to the readability... I'm happy to just apply patches
#1-#3 or just drop the series.

-Doug
Neil Armstrong July 24, 2024, 7:59 a.m. UTC | #8
On 12/07/2024 17:50, Doug Anderson wrote:
> Hi,
> 
> On Fri, Jul 12, 2024 at 7:56 AM <neil.armstrong@linaro.org> wrote:
>>
>> On 11/07/2024 21:36, Doug Anderson wrote:
>>> Hi,
>>>
>>> On Wed, Jul 10, 2024 at 6:09 PM cong yang
>>> <yangcong5@huaqin.corp-partner.google.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> Michael Walle <mwalle@kernel.org> 于2024年7月11日周四 03:38写道:
>>>>>
>>>>> On Wed Jul 10, 2024 at 9:12 PM CEST, Doug Anderson wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On Wed, Jul 10, 2024 at 2:02 AM Michael Walle <mwalle@kernel.org> wrote:
>>>>>>>
>>>>>>> On Wed Jul 10, 2024 at 10:47 AM CEST, Cong Yang wrote:
>>>>>>>> Break select page cmds into helper function.
>>>>>>>
>>>>>>> Why though? I don't find that anything easier to read. In fact, I
>>>>>>> deliberately chose not to factor that out into a function. It's just
>>>>>>> a sequence of magic commands, taken straight from the datasheet. So,
>>>>>>> I'd like to keep it that way.
>>>>>>
>>>>>> The consensus of previous discussion on the lists was that folks
>>>>>> agreed that we should, where possible, make it more obvious what these
>>>>>> magic sequences of commands were doing. IMO separating out the page
>>>>>> switch command helps. Certainly I'm always happy to hear other
>>>>>> opinions, though.
>>>>>
>>>>> Fair enough, but in that case, one should take the datasheet (which
>>>>> you can find online) and replace all the magic numbers with the
>>>>> correct command names from it. E.g. 0xff is the ENEXTC register. To
>>>>> be clear, I'm not just talking about the "switch page command".
>>>>>
>>>>> As patch stands, I don't see much value, TBH. On the contrary, you
>>>>> make it harder to compare it with the Ortustech panel datasheet.
>>>>>
>>>>> just my 2c,
>>>>> -michael
>>>>
>>>> If all drivers replace all the magic numbers with the correct command names,
>>>> it will be a huge amount of work (assuming that the datasheet can be found).
>>>>    I am afraid I don't have enough time to complete it.  Thanks.
>>>
>>> Makes sense. I'd be interested in hearing the opinion of others in the
>>> DRM community about whether they'd prefer to land something long this
>>> patch as-is or drop it.
>>
>> I don't have a strong opinion, but I think only changing the switch
>> page operations doesn't make a lot of sense by itself.
> 
> Does that mean you think we should drop this whole series? For the
> "panel-ilitek-ili9806e.c" driver dropping seems fine since the switch
> page command (and many of the other blocks of commands) is commented,
> but for the other panels in this series IMO even just getting the
> switch page adds to the readability... I'm happy to just apply patches
> #1-#3 or just drop the series.

Well since we already have the changes, clean and reviewed, let's apply them!

Neil

> 
> -Doug
diff mbox series

Patch

diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c b/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c
index e4a44cd26c4d..68fb9a1a4d80 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c
@@ -35,6 +35,12 @@  struct ili9806e_panel {
 	enum drm_panel_orientation orientation;
 };
 
+#define ILI9806E_DCS_SWITCH_PAGE	0xff
+
+#define ili9806e_switch_page(ctx, page) \
+	mipi_dsi_dcs_write_seq_multi(ctx, ILI9806E_DCS_SWITCH_PAGE, \
+				     0xff, 0x98, 0x06, 0x04, (page))
+
 static const char * const regulator_names[] = {
 	"vdd",
 	"vccio",
@@ -227,7 +233,7 @@  static void ili9806e_dsi_remove(struct mipi_dsi_device *dsi)
 static void com35h3p70ulc_init(struct mipi_dsi_multi_context *ctx)
 {
 	/* Switch to page 1 */
-	mipi_dsi_dcs_write_seq_multi(ctx, 0xff, 0xff, 0x98, 0x06, 0x04, 0x01);
+	ili9806e_switch_page(ctx, 0x01);
 	/* Interface Settings */
 	mipi_dsi_dcs_write_seq_multi(ctx, 0x08, 0x18);
 	mipi_dsi_dcs_write_seq_multi(ctx, 0x21, 0x01);
@@ -285,14 +291,14 @@  static void com35h3p70ulc_init(struct mipi_dsi_multi_context *ctx)
 	mipi_dsi_dcs_write_seq_multi(ctx, 0xcf, 0x0a);
 
 	/* Switch to page 7 */
-	mipi_dsi_dcs_write_seq_multi(ctx, 0xff, 0xff, 0x98, 0x06, 0x04, 0x07);
+	ili9806e_switch_page(ctx, 0x07);
 	/* Power Control */
 	mipi_dsi_dcs_write_seq_multi(ctx, 0x06, 0x00);
 	mipi_dsi_dcs_write_seq_multi(ctx, 0x18, 0x1d);
 	mipi_dsi_dcs_write_seq_multi(ctx, 0x17, 0x32);
 
 	/* Switch to page 6 */
-	mipi_dsi_dcs_write_seq_multi(ctx, 0xff, 0xff, 0x98, 0x06, 0x04, 0x06);
+	ili9806e_switch_page(ctx, 0x06);
 	/* GIP settings */
 	mipi_dsi_dcs_write_seq_multi(ctx, 0x00, 0x20);
 	mipi_dsi_dcs_write_seq_multi(ctx, 0x01, 0x02);
@@ -352,7 +358,7 @@  static void com35h3p70ulc_init(struct mipi_dsi_multi_context *ctx)
 	mipi_dsi_dcs_write_seq_multi(ctx, 0x53, 0x12);
 
 	/* Switch to page 0 */
-	mipi_dsi_dcs_write_seq_multi(ctx, 0xff, 0xff, 0x98, 0x06, 0x04, 0x00);
+	ili9806e_switch_page(ctx, 0x00);
 	/* Interface Pixel format */
 	mipi_dsi_dcs_write_seq_multi(ctx, 0x3a, 0x60);
 };