[PATCH v2 06/11] drm/i915/gt: Use pci_rebar_size_supported()

Ilpo Järvinen posted 11 patches 2 weeks, 3 days ago
[PATCH v2 06/11] drm/i915/gt: Use pci_rebar_size_supported()
Posted by Ilpo Järvinen 2 weeks, 3 days ago
PCI core provides pci_rebar_size_supported() that helps in checking if
a BAR Size is supported for the BAR or not. Use it in
i915_resize_lmem_bar() to simplify code.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Acked-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/i915/gt/intel_region_lmem.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
index 51bb27e10a4f..69c65fc8a72d 100644
--- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
+++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
@@ -61,16 +61,12 @@ static void i915_resize_lmem_bar(struct drm_i915_private *i915, resource_size_t
 	current_size = roundup_pow_of_two(pci_resource_len(pdev, GEN12_LMEM_BAR));
 
 	if (i915->params.lmem_bar_size) {
-		u32 bar_sizes;
-
-		rebar_size = i915->params.lmem_bar_size *
-			(resource_size_t)SZ_1M;
-		bar_sizes = pci_rebar_get_possible_sizes(pdev, GEN12_LMEM_BAR);
-
+		rebar_size = i915->params.lmem_bar_size * (resource_size_t)SZ_1M;
 		if (rebar_size == current_size)
 			return;
 
-		if (!(bar_sizes & BIT(pci_rebar_bytes_to_size(rebar_size))) ||
+		if (!pci_rebar_size_supported(pdev, GEN12_LMEM_BAR,
+					      pci_rebar_bytes_to_size(rebar_size)) ||
 		    rebar_size >= roundup_pow_of_two(lmem_size)) {
 			rebar_size = lmem_size;
 
-- 
2.39.5

Re: [PATCH v2 06/11] drm/i915/gt: Use pci_rebar_size_supported()
Posted by Andi Shyti 2 weeks, 2 days ago
Hi Ilpo,

On Mon, Sep 15, 2025 at 12:13:53PM +0300, Ilpo Järvinen wrote:
> PCI core provides pci_rebar_size_supported() that helps in checking if
> a BAR Size is supported for the BAR or not. Use it in
> i915_resize_lmem_bar() to simplify code.
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Acked-by: Christian König <christian.koenig@amd.com>

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>

Thanks,
Andi
Re: [PATCH v2 06/11] drm/i915/gt: Use pci_rebar_size_supported()
Posted by Jani Nikula 2 weeks, 3 days ago
On Mon, 15 Sep 2025, Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> PCI core provides pci_rebar_size_supported() that helps in checking if
> a BAR Size is supported for the BAR or not. Use it in
> i915_resize_lmem_bar() to simplify code.
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Acked-by: Christian König <christian.koenig@amd.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

and

Acked-by: Jani Nikula <jani.nikula@intel.com>

for merging via whichever tree is convenient.

> ---
>  drivers/gpu/drm/i915/gt/intel_region_lmem.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
> index 51bb27e10a4f..69c65fc8a72d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
> +++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
> @@ -61,16 +61,12 @@ static void i915_resize_lmem_bar(struct drm_i915_private *i915, resource_size_t
>  	current_size = roundup_pow_of_two(pci_resource_len(pdev, GEN12_LMEM_BAR));
>  
>  	if (i915->params.lmem_bar_size) {
> -		u32 bar_sizes;
> -
> -		rebar_size = i915->params.lmem_bar_size *
> -			(resource_size_t)SZ_1M;
> -		bar_sizes = pci_rebar_get_possible_sizes(pdev, GEN12_LMEM_BAR);
> -
> +		rebar_size = i915->params.lmem_bar_size * (resource_size_t)SZ_1M;
>  		if (rebar_size == current_size)
>  			return;
>  
> -		if (!(bar_sizes & BIT(pci_rebar_bytes_to_size(rebar_size))) ||
> +		if (!pci_rebar_size_supported(pdev, GEN12_LMEM_BAR,
> +					      pci_rebar_bytes_to_size(rebar_size)) ||
>  		    rebar_size >= roundup_pow_of_two(lmem_size)) {
>  			rebar_size = lmem_size;

-- 
Jani Nikula, Intel
Re: [PATCH v2 06/11] drm/i915/gt: Use pci_rebar_size_supported()
Posted by Andi Shyti 2 weeks, 2 days ago
Hi,

On Mon, Sep 15, 2025 at 03:42:23PM +0300, Jani Nikula wrote:
> On Mon, 15 Sep 2025, Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> > PCI core provides pci_rebar_size_supported() that helps in checking if
> > a BAR Size is supported for the BAR or not. Use it in
> > i915_resize_lmem_bar() to simplify code.
> >
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > Acked-by: Christian König <christian.koenig@amd.com>
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> and
> 
> Acked-by: Jani Nikula <jani.nikula@intel.com>

Just for some random noise on commit log's bureaucracy: why do we
need both Ack and R-b? I think R-b covers Ack making it
redundant. Right?

Andi
Re: [PATCH v2 06/11] drm/i915/gt: Use pci_rebar_size_supported()
Posted by Rodrigo Vivi 2 weeks, 2 days ago
On Mon, Sep 15, 2025 at 07:24:10PM +0200, Andi Shyti wrote:
> Hi,
> 
> On Mon, Sep 15, 2025 at 03:42:23PM +0300, Jani Nikula wrote:
> > On Mon, 15 Sep 2025, Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> > > PCI core provides pci_rebar_size_supported() that helps in checking if
> > > a BAR Size is supported for the BAR or not. Use it in
> > > i915_resize_lmem_bar() to simplify code.
> > >
> > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > > Acked-by: Christian König <christian.koenig@amd.com>
> > 
> > Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> > 
> > and
> > 
> > Acked-by: Jani Nikula <jani.nikula@intel.com>
> 
> Just for some random noise on commit log's bureaucracy: why do we
> need both Ack and R-b? I think R-b covers Ack making it
> redundant. Right?

reviewed-by is a more formal attestation of the entries in the
submitting-patches doc, saying that he carefully reviewed the work.

acked by is to state that from the maintainer perspective of that file
the file can be merged through any tree.

in the drm trees nowdays our tooling is enforcing acked-by tag if
the patch is touching domains outside that drm branch itself.

if a committer tries to push a patch without ack from the maintainer
of that domain it will be blocked.

So I believe it is a good idea to keep a separation of the meaning.
Carrying a technical review of the patch in question doesn't necessarily
mean that you, as maintainer, is okay of getting that patch merged
through other trees.

> 
> Andi
Re: [PATCH v2 06/11] drm/i915/gt: Use pci_rebar_size_supported()
Posted by Jani Nikula 2 weeks, 2 days ago
On Mon, 15 Sep 2025, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> On Mon, Sep 15, 2025 at 07:24:10PM +0200, Andi Shyti wrote:
>> Hi,
>> 
>> On Mon, Sep 15, 2025 at 03:42:23PM +0300, Jani Nikula wrote:
>> > On Mon, 15 Sep 2025, Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
>> > > PCI core provides pci_rebar_size_supported() that helps in checking if
>> > > a BAR Size is supported for the BAR or not. Use it in
>> > > i915_resize_lmem_bar() to simplify code.
>> > >
>> > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>> > > Acked-by: Christian König <christian.koenig@amd.com>
>> > 
>> > Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>> > 
>> > and
>> > 
>> > Acked-by: Jani Nikula <jani.nikula@intel.com>
>> 
>> Just for some random noise on commit log's bureaucracy: why do we
>> need both Ack and R-b? I think R-b covers Ack making it
>> redundant. Right?
>
> reviewed-by is a more formal attestation of the entries in the
> submitting-patches doc, saying that he carefully reviewed the work.
>
> acked by is to state that from the maintainer perspective of that file
> the file can be merged through any tree.
>
> in the drm trees nowdays our tooling is enforcing acked-by tag if
> the patch is touching domains outside that drm branch itself.
>
> if a committer tries to push a patch without ack from the maintainer
> of that domain it will be blocked.
>
> So I believe it is a good idea to keep a separation of the meaning.
> Carrying a technical review of the patch in question doesn't necessarily
> mean that you, as maintainer, is okay of getting that patch merged
> through other trees.

Yes, all of the above. I just wanted to be explicit to avoid the
follow-up questions "thanks for the review, but is it okay to merge via
pci" or "thanks for the ack, but does this need review also", and move
on from this whole thread. (Which is a nice cleanup, btw, thanks.)

BR,
Jani.

-- 
Jani Nikula, Intel
Re: [PATCH v2 06/11] drm/i915/gt: Use pci_rebar_size_supported()
Posted by Christian König 2 weeks, 2 days ago
On 16.09.25 10:12, Jani Nikula wrote:
> On Mon, 15 Sep 2025, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>> On Mon, Sep 15, 2025 at 07:24:10PM +0200, Andi Shyti wrote:
>>> Hi,
>>>
>>> On Mon, Sep 15, 2025 at 03:42:23PM +0300, Jani Nikula wrote:
>>>> On Mon, 15 Sep 2025, Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
>>>>> PCI core provides pci_rebar_size_supported() that helps in checking if
>>>>> a BAR Size is supported for the BAR or not. Use it in
>>>>> i915_resize_lmem_bar() to simplify code.
>>>>>
>>>>> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>>>>> Acked-by: Christian König <christian.koenig@amd.com>
>>>>
>>>> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>>>>
>>>> and
>>>>
>>>> Acked-by: Jani Nikula <jani.nikula@intel.com>
>>>
>>> Just for some random noise on commit log's bureaucracy: why do we
>>> need both Ack and R-b? I think R-b covers Ack making it
>>> redundant. Right?
>>
>> reviewed-by is a more formal attestation of the entries in the
>> submitting-patches doc, saying that he carefully reviewed the work.
>>
>> acked by is to state that from the maintainer perspective of that file
>> the file can be merged through any tree.
>>
>> in the drm trees nowdays our tooling is enforcing acked-by tag if
>> the patch is touching domains outside that drm branch itself.
>>
>> if a committer tries to push a patch without ack from the maintainer
>> of that domain it will be blocked.
>>
>> So I believe it is a good idea to keep a separation of the meaning.
>> Carrying a technical review of the patch in question doesn't necessarily
>> mean that you, as maintainer, is okay of getting that patch merged
>> through other trees.
> 
> Yes, all of the above. I just wanted to be explicit to avoid the
> follow-up questions "thanks for the review, but is it okay to merge via
> pci" or "thanks for the ack, but does this need review also", and move
> on from this whole thread. (Which is a nice cleanup, btw, thanks.)

Mhm, that's a really good point.

My understanding of an Acked-by by a maintainer is also "go a head and merge it through your tree", but I think we never formally documented that.

At least I can't find any reference to that in the "When to use Acked-by:, Cc:, and Co-developed-by:" section of Documentation/process/submitting-patches.rst.

Regards,
Christian.

> 
> BR,
> Jani.
> 

Re: [PATCH v2 06/11] drm/i915/gt: Use pci_rebar_size_supported()
Posted by Rodrigo Vivi 2 weeks, 1 day ago
On Tue, Sep 16, 2025 at 10:57:24AM +0200, Christian König wrote:
> On 16.09.25 10:12, Jani Nikula wrote:
> > On Mon, 15 Sep 2025, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> >> On Mon, Sep 15, 2025 at 07:24:10PM +0200, Andi Shyti wrote:
> >>> Hi,
> >>>
> >>> On Mon, Sep 15, 2025 at 03:42:23PM +0300, Jani Nikula wrote:
> >>>> On Mon, 15 Sep 2025, Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> >>>>> PCI core provides pci_rebar_size_supported() that helps in checking if
> >>>>> a BAR Size is supported for the BAR or not. Use it in
> >>>>> i915_resize_lmem_bar() to simplify code.
> >>>>>
> >>>>> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> >>>>> Acked-by: Christian König <christian.koenig@amd.com>
> >>>>
> >>>> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> >>>>
> >>>> and
> >>>>
> >>>> Acked-by: Jani Nikula <jani.nikula@intel.com>
> >>>
> >>> Just for some random noise on commit log's bureaucracy: why do we
> >>> need both Ack and R-b? I think R-b covers Ack making it
> >>> redundant. Right?
> >>
> >> reviewed-by is a more formal attestation of the entries in the
> >> submitting-patches doc, saying that he carefully reviewed the work.
> >>
> >> acked by is to state that from the maintainer perspective of that file
> >> the file can be merged through any tree.
> >>
> >> in the drm trees nowdays our tooling is enforcing acked-by tag if
> >> the patch is touching domains outside that drm branch itself.
> >>
> >> if a committer tries to push a patch without ack from the maintainer
> >> of that domain it will be blocked.
> >>
> >> So I believe it is a good idea to keep a separation of the meaning.
> >> Carrying a technical review of the patch in question doesn't necessarily
> >> mean that you, as maintainer, is okay of getting that patch merged
> >> through other trees.
> > 
> > Yes, all of the above. I just wanted to be explicit to avoid the
> > follow-up questions "thanks for the review, but is it okay to merge via
> > pci" or "thanks for the ack, but does this need review also", and move
> > on from this whole thread. (Which is a nice cleanup, btw, thanks.)
> 
> Mhm, that's a really good point.
> 
> My understanding of an Acked-by by a maintainer is also "go a head and merge it through your tree", but I think we never formally documented that.
> 
> At least I can't find any reference to that in the "When to use Acked-by:, Cc:, and Co-developed-by:" section of Documentation/process/submitting-patches.rst.

"Acked-by: is also less formal than Reviewed-by:. For instance, maintainers may
use it to signify that they are OK with a patch landing, but they may not have reviewed it..."

perhaps we should simply

s/patch landing/patch landing through any other tree/

> 
> Regards,
> Christian.
> 
> > 
> > BR,
> > Jani.
> > 
>