On Tue, Oct 19, 2021 at 11:02:45AM +0300, Jani Nikula wrote:
> 
> From patch 1:
> 
> static bool check_modifier_display_ver(const struct intel_modifier_desc *md,
>                                      u8 display_ver)
> {
>       return display_ver >= md->display_ver.from &&
>              display_ver <= md->display_ver.until;
> }
> 
> On Fri, 15 Oct 2021, Imre Deak <[email protected]> wrote:
> > +static bool check_modifier_display_ver_range(const struct 
> > intel_modifier_desc *md,
> > +                                        u8 display_ver_from, u8 
> > display_ver_until)
> > +{
> > +   return check_modifier_display_ver(md, display_ver_from) &&
> > +          check_modifier_display_ver(md, display_ver_until);
> > +}
> > +
> 
> ...
> 
> > +/**
> > + * intel_fb_is_gen12_ccs_aux_plane: Check if a framebuffer color plane is 
> > a GEN12 CCS AUX plane
> > + * @fb: Framebuffer
> > + * @color_plane: color plane index to check
> > + *
> > + * Returns:
> > + * Returns %true if @fb's color plane at index @color_plane is a GEN12 CCS 
> > AUX plane.
> > + */
> > +static bool intel_fb_is_gen12_ccs_aux_plane(const struct drm_framebuffer 
> > *fb, int color_plane)
> >  {
> > -   return is_gen12_ccs_modifier(fb->modifier) && is_ccs_plane(fb, plane);
> > +   const struct intel_modifier_desc *md = lookup_modifier(fb->modifier);
> > +
> > +   return check_modifier_display_ver_range(md, 12, 13) &&
> > +          ccs_aux_plane_mask(md, fb->format) & BIT(color_plane);
> >  }
> 
> check_modifier_display_ver_range(md, 12, 13)
> 
> ==>
> 
> check_modifier_display_ver(md, 12) &&
> check_modifier_display_ver(md, 13)
> 
> ==>
> 
> 12 >= md->display_ver.from &&
> 12 <= md->display_ver.until &&
> 13 >= md->display_ver.from &&
> 13 <= md->display_ver.until
> 
> ==>
> 
> Always false.

If md->display_ver.from=12, md->display_ver.until=13

12 >= 12 &&
12 <= 13 &&
13 >= 12 &&
13 <= 13

not false.

But yes, check_modifier_display_ver_range() is bogus for the
md->display_ver.from == md->display_ver.until case, and should be

md->display_ver.from >= display_ver_from &&
md->display_ver.until <= disaply_ver_until

Thanks for catching this, will fix it.

> BR,
> Jani.
> 
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

Reply via email to