2024-05-15 21:16:10

by Neil Armstrong

[permalink] [raw]
Subject: Re: [v7 3/7] arm64: defconfig: Enable HIMAX_HX83102 panel

Hi,

On 15/05/2024 03:46, Cong Yang wrote:
> DRM_PANEL_HIMAX_HX83102 is being split out from DRM_PANEL_BOE_TV101WUM_NL6.
> Since the arm64 defconfig had the BOE panel driver enabled, let's also
> enable the himax driver.
>
> Signed-off-by: Cong Yang <[email protected]>
> Reviewed-by: Douglas Anderson <[email protected]>
> ---
> arch/arm64/configs/defconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index 2c30d617e180..687c86ddaece 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -864,6 +864,7 @@ CONFIG_DRM_PANEL_BOE_TV101WUM_NL6=m
> CONFIG_DRM_PANEL_LVDS=m
> CONFIG_DRM_PANEL_SIMPLE=m
> CONFIG_DRM_PANEL_EDP=m
> +CONFIG_DRM_PANEL_HIMAX_HX83102=m
> CONFIG_DRM_PANEL_ILITEK_ILI9882T=m
> CONFIG_DRM_PANEL_MANTIX_MLAF057WE51=m
> CONFIG_DRM_PANEL_RAYDIUM_RM67191=m

You should probably sent this one separately since only an ARM SoC maintainer
can apply this, probably via the qcom tree.

Thanks,
Neil


2024-05-15 21:28:52

by Doug Anderson

[permalink] [raw]
Subject: Re: [v7 3/7] arm64: defconfig: Enable HIMAX_HX83102 panel

Hi,

On Wed, May 15, 2024 at 2:16 PM <[email protected]> wrote:
>
> Hi,
>
> On 15/05/2024 03:46, Cong Yang wrote:
> > DRM_PANEL_HIMAX_HX83102 is being split out from DRM_PANEL_BOE_TV101WUM_NL6.
> > Since the arm64 defconfig had the BOE panel driver enabled, let's also
> > enable the himax driver.
> >
> > Signed-off-by: Cong Yang <[email protected]>
> > Reviewed-by: Douglas Anderson <[email protected]>
> > ---
> > arch/arm64/configs/defconfig | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> > index 2c30d617e180..687c86ddaece 100644
> > --- a/arch/arm64/configs/defconfig
> > +++ b/arch/arm64/configs/defconfig
> > @@ -864,6 +864,7 @@ CONFIG_DRM_PANEL_BOE_TV101WUM_NL6=m
> > CONFIG_DRM_PANEL_LVDS=m
> > CONFIG_DRM_PANEL_SIMPLE=m
> > CONFIG_DRM_PANEL_EDP=m
> > +CONFIG_DRM_PANEL_HIMAX_HX83102=m
> > CONFIG_DRM_PANEL_ILITEK_ILI9882T=m
> > CONFIG_DRM_PANEL_MANTIX_MLAF057WE51=m
> > CONFIG_DRM_PANEL_RAYDIUM_RM67191=m
>
> You should probably sent this one separately since only an ARM SoC maintainer
> can apply this, probably via the qcom tree.

Really? I always kinda figured that this was a bit like MAINTAINERS
where it can come through a bunch of different trees. Certainly I've
landed changes to it before through the drm-misc tree. If that was
wrong then I'll certainly stop doing it, of course.

-Doug

2024-05-16 06:43:38

by cong yang

[permalink] [raw]
Subject: Re: [v7 3/7] arm64: defconfig: Enable HIMAX_HX83102 panel

Hi:

If it is determined that a separately patch needs to be sent, then I
will remove this patch in V8 series?

Doug Anderson <[email protected]> 于2024年5月16日周四 05:28写道:

>
> Hi,
>
> On Wed, May 15, 2024 at 2:16 PM <[email protected]> wrote:
> >
> > Hi,
> >
> > On 15/05/2024 03:46, Cong Yang wrote:
> > > DRM_PANEL_HIMAX_HX83102 is being split out from DRM_PANEL_BOE_TV101WUM_NL6.
> > > Since the arm64 defconfig had the BOE panel driver enabled, let's also
> > > enable the himax driver.
> > >
> > > Signed-off-by: Cong Yang <[email protected]>
> > > Reviewed-by: Douglas Anderson <[email protected]>
> > > ---
> > > arch/arm64/configs/defconfig | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> > > index 2c30d617e180..687c86ddaece 100644
> > > --- a/arch/arm64/configs/defconfig
> > > +++ b/arch/arm64/configs/defconfig
> > > @@ -864,6 +864,7 @@ CONFIG_DRM_PANEL_BOE_TV101WUM_NL6=m
> > > CONFIG_DRM_PANEL_LVDS=m
> > > CONFIG_DRM_PANEL_SIMPLE=m
> > > CONFIG_DRM_PANEL_EDP=m
> > > +CONFIG_DRM_PANEL_HIMAX_HX83102=m
> > > CONFIG_DRM_PANEL_ILITEK_ILI9882T=m
> > > CONFIG_DRM_PANEL_MANTIX_MLAF057WE51=m
> > > CONFIG_DRM_PANEL_RAYDIUM_RM67191=m
> >
> > You should probably sent this one separately since only an ARM SoC maintainer
> > can apply this, probably via the qcom tree.
>
> Really? I always kinda figured that this was a bit like MAINTAINERS
> where it can come through a bunch of different trees. Certainly I've
> landed changes to it before through the drm-misc tree. If that was
> wrong then I'll certainly stop doing it, of course.
>
> -Doug

2024-05-16 06:55:29

by Neil Armstrong

[permalink] [raw]
Subject: Re: [v7 3/7] arm64: defconfig: Enable HIMAX_HX83102 panel

On 16/05/2024 08:43, cong yang wrote:
> Hi:
>
> If it is determined that a separately patch needs to be sent, then I
> will remove this patch in V8 series?
>
> Doug Anderson <[email protected]> 于2024年5月16日周四 05:28写道:
>
>>
>> Hi,
>>
>> On Wed, May 15, 2024 at 2:16 PM <[email protected]> wrote:
>>>
>>> Hi,
>>>
>>> On 15/05/2024 03:46, Cong Yang wrote:
>>>> DRM_PANEL_HIMAX_HX83102 is being split out from DRM_PANEL_BOE_TV101WUM_NL6.
>>>> Since the arm64 defconfig had the BOE panel driver enabled, let's also
>>>> enable the himax driver.
>>>>
>>>> Signed-off-by: Cong Yang <[email protected]>
>>>> Reviewed-by: Douglas Anderson <[email protected]>
>>>> ---
>>>> arch/arm64/configs/defconfig | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
>>>> index 2c30d617e180..687c86ddaece 100644
>>>> --- a/arch/arm64/configs/defconfig
>>>> +++ b/arch/arm64/configs/defconfig
>>>> @@ -864,6 +864,7 @@ CONFIG_DRM_PANEL_BOE_TV101WUM_NL6=m
>>>> CONFIG_DRM_PANEL_LVDS=m
>>>> CONFIG_DRM_PANEL_SIMPLE=m
>>>> CONFIG_DRM_PANEL_EDP=m
>>>> +CONFIG_DRM_PANEL_HIMAX_HX83102=m
>>>> CONFIG_DRM_PANEL_ILITEK_ILI9882T=m
>>>> CONFIG_DRM_PANEL_MANTIX_MLAF057WE51=m
>>>> CONFIG_DRM_PANEL_RAYDIUM_RM67191=m
>>>
>>> You should probably sent this one separately since only an ARM SoC maintainer
>>> can apply this, probably via the qcom tree.
>>
>> Really? I always kinda figured that this was a bit like MAINTAINERS
>> where it can come through a bunch of different trees. Certainly I've
>> landed changes to it before through the drm-misc tree. If that was
>> wrong then I'll certainly stop doing it, of course.

Yeah we usually don't mess with arch specific defconfig from drm tree

>>
>> -Doug


2024-05-16 13:49:59

by Doug Anderson

[permalink] [raw]
Subject: Re: [v7 3/7] arm64: defconfig: Enable HIMAX_HX83102 panel

Hi,

On Wed, May 15, 2024 at 11:55 PM <[email protected]> wrote:
>
> On 16/05/2024 08:43, cong yang wrote:
> > Hi:
> >
> > If it is determined that a separately patch needs to be sent, then I
> > will remove this patch in V8 series?
> >
> > Doug Anderson <[email protected]> 于2024年5月16日周四 05:28写道:
> >
> >>
> >> Hi,
> >>
> >> On Wed, May 15, 2024 at 2:16 PM <[email protected]> wrote:
> >>>
> >>> Hi,
> >>>
> >>> On 15/05/2024 03:46, Cong Yang wrote:
> >>>> DRM_PANEL_HIMAX_HX83102 is being split out from DRM_PANEL_BOE_TV101WUM_NL6.
> >>>> Since the arm64 defconfig had the BOE panel driver enabled, let's also
> >>>> enable the himax driver.
> >>>>
> >>>> Signed-off-by: Cong Yang <[email protected]>
> >>>> Reviewed-by: Douglas Anderson <[email protected]>
> >>>> ---
> >>>> arch/arm64/configs/defconfig | 1 +
> >>>> 1 file changed, 1 insertion(+)
> >>>>
> >>>> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> >>>> index 2c30d617e180..687c86ddaece 100644
> >>>> --- a/arch/arm64/configs/defconfig
> >>>> +++ b/arch/arm64/configs/defconfig
> >>>> @@ -864,6 +864,7 @@ CONFIG_DRM_PANEL_BOE_TV101WUM_NL6=m
> >>>> CONFIG_DRM_PANEL_LVDS=m
> >>>> CONFIG_DRM_PANEL_SIMPLE=m
> >>>> CONFIG_DRM_PANEL_EDP=m
> >>>> +CONFIG_DRM_PANEL_HIMAX_HX83102=m
> >>>> CONFIG_DRM_PANEL_ILITEK_ILI9882T=m
> >>>> CONFIG_DRM_PANEL_MANTIX_MLAF057WE51=m
> >>>> CONFIG_DRM_PANEL_RAYDIUM_RM67191=m
> >>>
> >>> You should probably sent this one separately since only an ARM SoC maintainer
> >>> can apply this, probably via the qcom tree.
> >>
> >> Really? I always kinda figured that this was a bit like MAINTAINERS
> >> where it can come through a bunch of different trees. Certainly I've
> >> landed changes to it before through the drm-misc tree. If that was
> >> wrong then I'll certainly stop doing it, of course.
>
> Yeah we usually don't mess with arch specific defconfig from drm tree

In general I agree that makes sense. In this case, though, the new
config symbol was introduced in the previous patch and split off an
existing symbol. Updating "all" of the configs (AKA just arm64) that
had the old symbol to also have the new symbol seems like the nice
thing to do and it feels like it makes sense to land in the same tree
that did the "split" just to cause the least confusion to anyone
affected.

In any case, if it's going to land in some other tree then I guess the
question is whether it needs to wait a few revisions to land there or
if it should land right away. Nobody would get a compile error if it
landed in a different tree right away since unknown config symbols are
silently ignored, but it feels a little weird to me.

..of course, I'm also OK just dropping the config patch. I personally
don't use the upstream "defconfig". It just seemed courteous to update
it for those who do.

-Doug

2024-05-16 14:12:19

by Doug Anderson

[permalink] [raw]
Subject: Re: [v7 3/7] arm64: defconfig: Enable HIMAX_HX83102 panel

Hi,

On Thu, May 16, 2024 at 6:43 AM Doug Anderson <[email protected]> wrote:
>
> Hi,
>
> On Wed, May 15, 2024 at 11:55 PM <[email protected]> wrote:
> >
> > On 16/05/2024 08:43, cong yang wrote:
> > > Hi:
> > >
> > > If it is determined that a separately patch needs to be sent, then I
> > > will remove this patch in V8 series?
> > >
> > > Doug Anderson <[email protected]> 于2024年5月16日周四 05:28写道:
> > >
> > >>
> > >> Hi,
> > >>
> > >> On Wed, May 15, 2024 at 2:16 PM <[email protected]> wrote:
> > >>>
> > >>> Hi,
> > >>>
> > >>> On 15/05/2024 03:46, Cong Yang wrote:
> > >>>> DRM_PANEL_HIMAX_HX83102 is being split out from DRM_PANEL_BOE_TV101WUM_NL6.
> > >>>> Since the arm64 defconfig had the BOE panel driver enabled, let's also
> > >>>> enable the himax driver.
> > >>>>
> > >>>> Signed-off-by: Cong Yang <[email protected]>
> > >>>> Reviewed-by: Douglas Anderson <[email protected]>
> > >>>> ---
> > >>>> arch/arm64/configs/defconfig | 1 +
> > >>>> 1 file changed, 1 insertion(+)
> > >>>>
> > >>>> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> > >>>> index 2c30d617e180..687c86ddaece 100644
> > >>>> --- a/arch/arm64/configs/defconfig
> > >>>> +++ b/arch/arm64/configs/defconfig
> > >>>> @@ -864,6 +864,7 @@ CONFIG_DRM_PANEL_BOE_TV101WUM_NL6=m
> > >>>> CONFIG_DRM_PANEL_LVDS=m
> > >>>> CONFIG_DRM_PANEL_SIMPLE=m
> > >>>> CONFIG_DRM_PANEL_EDP=m
> > >>>> +CONFIG_DRM_PANEL_HIMAX_HX83102=m
> > >>>> CONFIG_DRM_PANEL_ILITEK_ILI9882T=m
> > >>>> CONFIG_DRM_PANEL_MANTIX_MLAF057WE51=m
> > >>>> CONFIG_DRM_PANEL_RAYDIUM_RM67191=m
> > >>>
> > >>> You should probably sent this one separately since only an ARM SoC maintainer
> > >>> can apply this, probably via the qcom tree.
> > >>
> > >> Really? I always kinda figured that this was a bit like MAINTAINERS
> > >> where it can come through a bunch of different trees. Certainly I've
> > >> landed changes to it before through the drm-misc tree. If that was
> > >> wrong then I'll certainly stop doing it, of course.
> >
> > Yeah we usually don't mess with arch specific defconfig from drm tree
>
> In general I agree that makes sense. In this case, though, the new
> config symbol was introduced in the previous patch and split off an
> existing symbol. Updating "all" of the configs (AKA just arm64) that
> had the old symbol to also have the new symbol seems like the nice
> thing to do and it feels like it makes sense to land in the same tree
> that did the "split" just to cause the least confusion to anyone
> affected.
>
> In any case, if it's going to land in some other tree then I guess the
> question is whether it needs to wait a few revisions to land there or
> if it should land right away. Nobody would get a compile error if it
> landed in a different tree right away since unknown config symbols are
> silently ignored, but it feels a little weird to me.
>
> ...of course, I'm also OK just dropping the config patch. I personally
> don't use the upstream "defconfig". It just seemed courteous to update
> it for those who do.

Hmmm, probably should have put Arnd on this thread. Added now in case
he has any opinions. I also did manage to find when this last came up
where I was involved. At that time Will Deacon (who get_maintainer.pl
reports is the official maintainer of this file) said [1]:

> But yes, although there are a few things I really care about
> in defconfig (e.g. things like page size!), generally speaking we don't
> need to Ack everything that changes in there.

[1] https://lore.kernel.org/linux-arm-kernel/[email protected]/T/

2024-05-17 06:41:53

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [v7 3/7] arm64: defconfig: Enable HIMAX_HX83102 panel

On Thu, May 16, 2024, at 14:09, Doug Anderson wrote:
> On Thu, May 16, 2024 at 6:43 AM Doug Anderson <[email protected]> wrote:
>> On Wed, May 15, 2024 at 11:55 PM <[email protected]> wrote:
>> > On 16/05/2024 08:43, cong yang wrote:
>> >
>> > Yeah we usually don't mess with arch specific defconfig from drm tree
>>
>> In general I agree that makes sense. In this case, though, the new
>> config symbol was introduced in the previous patch and split off an
>> existing symbol. Updating "all" of the configs (AKA just arm64) that
>> had the old symbol to also have the new symbol seems like the nice
>> thing to do and it feels like it makes sense to land in the same tree
>> that did the "split" just to cause the least confusion to anyone
>> affected.
>>
>> In any case, if it's going to land in some other tree then I guess the
>> question is whether it needs to wait a few revisions to land there or
>> if it should land right away. Nobody would get a compile error if it
>> landed in a different tree right away since unknown config symbols are
>> silently ignored, but it feels a little weird to me.
>>
>> ...of course, I'm also OK just dropping the config patch. I personally
>> don't use the upstream "defconfig". It just seemed courteous to update
>> it for those who do.
>
> Hmmm, probably should have put Arnd on this thread. Added now in case
> he has any opinions. I also did manage to find when this last came up
> where I was involved. At that time Will Deacon (who get_maintainer.pl
> reports is the official maintainer of this file) said [1]:
>
>> But yes, although there are a few things I really care about
>> in defconfig (e.g. things like page size!), generally speaking we don't
>> need to Ack everything that changes in there.
>

My preferred way of getting arm/arm64 defconfig updates is to have
them picked up by the platform maintainer, the same way we handle
updates to dts files. The platform maintainers are familiar with the
process and will send the patches on to me for integration through
the soc tree.

If a change is not specific to any particular platform, I recommend
to send it to:[email protected], cc:lakml. This makes it show up in
my patchwork, so I will eventually get around to picking it up.

When you do this, it's helpful to me if you include an explanation
(after the --- line) why this patch does not get picked up by
a platform maintainer, and it also helps me to include whether
I should include it in the current (6.10) fixes or queue it for
the next merge window.

Arnd